Aggregate all metric config bounds into a BoundsSet

RELEASE_NOTES.md

@@ -137,6 +137,7 @@
 
     * `frequenz.client.common.metrics.BoundsSet` — a normalized union of `Bounds` with an efficient `value in bounds_set` membership test. Overlapping and touching bounds are merged on construction, and the empty set is the unbounded set (it contains every value and is falsy).
     * `frequenz.client.common.metrics.InvalidBoundsSet` — a set built from bounds that included at least one `InvalidBounds`; it preserves all the raw bounds unmerged and provides no membership test.
+    * `frequenz.client.common.metrics.proto.v1alpha8.bounds_set_from_proto` conversion function returning `BoundsSet | InvalidBoundsSet`. It converts a `repeated Bounds` field into a single bounds set.
 
 * Added a new `frequenz.client.common.metrics.MetricSample.bounds_set` field, typed `BoundsSet | InvalidBoundsSet`, replacing the deprecated `bounds` list (see Upgrading). Malformed wire bounds are preserved as an `InvalidBoundsSet` instead of being dropped. Use `get_bounds_set()` to resolve it to a valid `BoundsSet` or a clear `InvalidBoundsSetError`.
 
@@ -148,8 +149,6 @@
 
     The class of a component is its identity; components don't carry category or type attributes. The only exceptions are the error-recovery classes `UnrecognizedElectricalComponent` and `MismatchedCategoryElectricalComponent` (with a raw protobuf `category` value) and `UnrecognizedBattery`, `UnrecognizedInverter` and `UnrecognizedEvCharger` (with a raw protobuf `type` value), which preserve the raw protobuf values received from the protocol version used to load them.
 
-    `ElectricalComponent.metric_config_bounds` is typed `Mapping[Metric | int, Bounds | InvalidBounds]`: malformed wire entries are preserved as `InvalidBounds` instead of being silently dropped, and entries that named a metric but carried no (or an empty) `config_bounds` submessage load as an unbounded `Bounds()` (a `Bounds` with neither bound set imposes no limit in either direction). Use `get_metric_config_bounds()` to resolve an entry to a valid `Bounds` or a clear `InvalidBoundsError`; it mimics `dict.get()`, returning an unbounded `Bounds()` (or a caller-supplied `default`) for absent metrics.
-
 * Added a new `frequenz.client.common.microgrid.Microgrid` type with a raising `is_active()` method, together with the `frequenz.client.common.microgrid.proto.v1alpha8.microgrid_from_proto` conversion function.
 
 * Added `frequenz.client.common.FloatInt`, a type alias for `float | int`.

src/frequenz/client/common/metrics/proto/v1alpha8/__init__.py

@@ -7,6 +7,7 @@
     bounds_from_proto,
     bounds_from_proto2,
     bounds_from_proto_with_issues,
+    bounds_set_from_proto,
 )
 from ._metric import metric_from_proto, metric_to_proto
 from ._metric_connection_category import (
@@ -24,6 +25,7 @@
     "bounds_from_proto",
     "bounds_from_proto2",
     "bounds_from_proto_with_issues",
+    "bounds_set_from_proto",
     "metric_connection_category_from_proto",
     "metric_connection_category_to_proto",
     "metric_connection_from_proto_with_issues",

src/frequenz/client/common/metrics/proto/v1alpha8/_bounds.py

@@ -3,10 +3,13 @@
 
 """Loading of Bounds objects from protobuf messages."""
 
+from collections.abc import Sequence
+from typing import assert_never
+
 from frequenz.api.common.v1alpha8.metrics import bounds_pb2
 from typing_extensions import deprecated
 
-from ..._bounds import Bounds, InvalidBounds
+from ..._bounds import Bounds, BoundsSet, InvalidBounds, InvalidBoundsSet
 
 
 @deprecated(
@@ -60,6 +63,43 @@ def bounds_from_proto2(
     return InvalidBounds(lower=lower, upper=upper)
 
 
+def bounds_set_from_proto(
+    messages: Sequence[bounds_pb2.Bounds],
+) -> BoundsSet | InvalidBoundsSet:
+    """Convert a sequence of bounds messages into a single bounds set.
+
+    This is the multi-bound counterpart of
+    [`bounds_from_proto2`][..bounds_from_proto2]: it converts each message and
+    combines the results.
+
+    Args:
+        messages: The bounds messages to convert.
+
+    Returns:
+        A [`BoundsSet`][....BoundsSet] (the union of the bounds) when every
+            message is well-formed, or an
+            [`InvalidBoundsSet`][....InvalidBoundsSet] preserving all the raw
+            bounds in order when any message is malformed.
+    """
+    valid: list[Bounds] = []
+    raw: list[Bounds | InvalidBounds] = []
+    has_invalid = False
+    for pb_bound in messages:
+        match bounds_from_proto2(pb_bound):
+            case Bounds() as bound:
+                valid.append(bound)
+                raw.append(bound)
+            case InvalidBounds() as bound:
+                has_invalid = True
+                raw.append(bound)
+            case unknown:
+                assert_never(unknown)
+
+    if has_invalid:
+        return InvalidBoundsSet(bounds=tuple(raw))
+    return BoundsSet(bounds=tuple(valid))
+
+
 @deprecated(
     "`bounds_from_proto_with_issues` is deprecated; use "
     "`bounds_from_proto2` (returns `Bounds | InvalidBounds`) and inspect "

src/frequenz/client/common/metrics/proto/v1alpha8/_sample.py

@@ -3,21 +3,17 @@
 
 """Loading of MetricSample and AggregatedMetricValue objects from protobuf messages."""
 
-from collections.abc import Sequence
-from typing import assert_never
-
-from frequenz.api.common.v1alpha8.metrics import bounds_pb2, metrics_pb2
+from frequenz.api.common.v1alpha8.metrics import metrics_pb2
 
 from ....proto import datetime_from_proto
-from ..._bounds import Bounds, BoundsSet, InvalidBounds, InvalidBoundsSet
 from ..._metric import Metric
 from ..._sample import (
     AggregatedMetricValue,
     MetricConnection,
     MetricConnectionCategory,
     MetricSample,
 )
-from ._bounds import bounds_from_proto2
+from ._bounds import bounds_set_from_proto
 from ._metric import metric_from_proto
 from ._metric_connection_category import metric_connection_category_from_proto
 
@@ -106,7 +102,7 @@ def metric_sample_from_proto_with_issues(
                     message.value.aggregated_metric
                 )
 
-    bounds_set = _bounds_set_from_proto(message.bounds)
+    bounds_set = bounds_set_from_proto(message.bounds)
 
     connection = None
     if message.HasField("connection"):
@@ -121,35 +117,3 @@ def metric_sample_from_proto_with_issues(
         bounds_set=bounds_set,
         connection=connection,
     )
-
-
-def _bounds_set_from_proto(
-    messages: Sequence[bounds_pb2.Bounds],
-) -> BoundsSet | InvalidBoundsSet:
-    """Convert a sequence of bounds messages to a bounds set.
-
-    Args:
-        messages: The sequence of bounds messages.
-
-    Returns:
-        A [`BoundsSet`][....BoundsSet] when every bound is well-formed, or an
-            [`InvalidBoundsSet`][....InvalidBoundsSet] preserving all the raw
-            bounds when any bound is malformed.
-    """
-    valid: list[Bounds] = []
-    raw: list[Bounds | InvalidBounds] = []
-    has_invalid = False
-    for pb_bound in messages:
-        match bounds_from_proto2(pb_bound):
-            case Bounds() as bound:
-                valid.append(bound)
-                raw.append(bound)
-            case InvalidBounds() as bound:
-                has_invalid = True
-                raw.append(bound)
-            case unknown:
-                assert_never(unknown)
-
-    if has_invalid:
-        return InvalidBoundsSet(bounds=tuple(raw))
-    return BoundsSet(bounds=tuple(valid))

src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py

@@ -9,7 +9,7 @@
 from typing import Any, Self, TypeVar, assert_never, overload
 
 from ..._exception import UnrecognizedEnumValueError, UnspecifiedEnumValueError
-from ...metrics import Bounds, InvalidBounds, InvalidBoundsError, Metric
+from ...metrics import BoundsSet, InvalidBoundsSet, InvalidBoundsSetError, Metric
 from .. import MicrogridId
 from .._lifetime import InvalidLifetime, InvalidLifetimeError, Lifetime
 from ._category_specific_info import CategorySpecificInfo
@@ -76,7 +76,7 @@ class ElectricalComponent:  # pylint: disable=too-many-instance-attributes
     )
     """Internal guard allowing construction only via the `*_from_proto` converters."""
 
-    metric_config_bounds: Mapping[Metric | int, Bounds | InvalidBounds] = (
+    metric_config_bounds: Mapping[Metric | int, BoundsSet | InvalidBoundsSet] = (
         dataclasses.field(
             default_factory=dict,
             # dict is not hashable, so we don't use this field to calculate the hash.
@@ -91,17 +91,18 @@ class ElectricalComponent:  # pylint: disable=too-many-instance-attributes
     These bounds may be derived from the component configuration, manufacturer
     limits, or limits of other devices.
 
-    Malformed bounds received from the wire are preserved as
-    [`InvalidBounds`][.....metrics.InvalidBounds] instances so callers can
-    inspect the raw values without accidentally using them for range checks.
+    Each metric maps to the aggregate of all the bounds configured for it: a
+    [`BoundsSet`][.....metrics.BoundsSet] when every one is well-formed, or an
+    [`InvalidBoundsSet`][.....metrics.InvalidBoundsSet] preserving all the raw
+    bounds when any is malformed.
 
     If an unspecified metric is received, it is stored as the plain `int` key `0` when
     loading from protobuf. Metrics unknown to this client version may also appear
     as plain `int` keys for forward-compatibility.
 
     Tip:
         Prefer [`get_metric_config_bounds()`][..get_metric_config_bounds]
-        when a valid [`Bounds`][.....metrics.Bounds] is required.
+        when a valid [`BoundsSet`][.....metrics.BoundsSet] is required.
     """
 
     category_specific_info: CategorySpecificInfo | None = None
@@ -203,21 +204,21 @@ def accepts_control(self) -> bool:
                 assert_never(unknown)
 
     @overload
-    def get_metric_config_bounds(self, metric: Metric) -> Bounds: ...
+    def get_metric_config_bounds(self, metric: Metric) -> BoundsSet: ...
 
     @overload
     def get_metric_config_bounds(
         self, metric: Metric, *, default: DefaultT
-    ) -> Bounds | DefaultT: ...
+    ) -> BoundsSet | DefaultT: ...
 
     def get_metric_config_bounds(
-        self, metric: Metric, *, default: object = Bounds()
+        self, metric: Metric, *, default: object = BoundsSet()
     ) -> object:
-        """Return the configured bounds for a metric as a valid `Bounds`.
+        """Return the configured bounds for a metric as a valid `BoundsSet`.
 
         An absent entry returns an unbounded metric, so when no bounds are
         configured for `metric` this returns an unbounded
-        [`Bounds`][frequenz.client.common.metrics.Bounds] by default. Pass
+        [`BoundsSet`][frequenz.client.common.metrics.BoundsSet] by default. Pass
         `default` to return a different value for absent entries instead,
         mimicking [`dict.get()`][dict.get].
 
@@ -241,25 +242,25 @@ def get_metric_config_bounds(
                 `metric`.
 
         Returns:
-            The valid [`Bounds`][.....metrics.Bounds] configured for `metric`,
-                or `default` when there is no entry for `metric`.
+            The valid [`BoundsSet`][.....metrics.BoundsSet] configured for
+                `metric`, or `default` when there is no entry for `metric`.
 
         Raises:
-            InvalidBoundsError: If the bounds configured for `metric` are
+            InvalidBoundsSetError: If the bounds configured for `metric` are
                 malformed. The offending instance is available on the
-                exception's `bounds` attribute.
+                exception's `bounds_set` attribute.
         """
         match self.metric_config_bounds.get(metric):
             case None:
                 return default
-            case InvalidBounds() as invalid:
-                raise InvalidBoundsError(
+            case InvalidBoundsSet() as invalid:
+                raise InvalidBoundsSetError(
                     self,
                     "metric_config_bounds",
                     invalid,
-                    f"invalid bounds {invalid} for metric {metric} in {self}",
+                    f"invalid bounds set {invalid} for metric {metric} in {self}",
                 )
-            case Bounds() as valid:
+            case BoundsSet() as valid:
                 return valid
             case unknown:
                 assert_never(unknown)

src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py

@@ -8,13 +8,14 @@
 from collections.abc import Mapping, Sequence
 from typing import Final, NamedTuple, TypeAlias, assert_never, overload
 
+from frequenz.api.common.v1alpha8.metrics import bounds_pb2
 from frequenz.api.common.v1alpha8.microgrid.electrical_components import (
     electrical_components_pb2,
 )
 from google.protobuf.json_format import MessageToDict
 
-from .....metrics import Bounds, InvalidBounds, Metric
-from .....metrics.proto.v1alpha8 import bounds_from_proto2
+from .....metrics import BoundsSet, InvalidBoundsSet, Metric
+from .....metrics.proto.v1alpha8._bounds import bounds_set_from_proto
 from .....proto import enum_from_proto
 from ...._ids import MicrogridId
 from ...._lifetime import InvalidLifetime, Lifetime
@@ -892,13 +893,16 @@ class _ElectricalComponentBaseData(NamedTuple):
     lifetime: Lifetime | InvalidLifetime
     """The operational lifetime of the electrical component."""
 
-    metric_config_bounds: dict[Metric | int, Bounds | InvalidBounds]
+    metric_config_bounds: dict[Metric | int, BoundsSet | InvalidBoundsSet]
     """The metric configuration bounds extracted from the protobuf message.
 
-    Malformed entries are preserved as
-    [`InvalidBounds`][frequenz.client.common.metrics.InvalidBounds]; entries
-    whose `config_bounds` field was not set load as an unbounded
-    [`Bounds`][frequenz.client.common.metrics.Bounds].
+    Each metric maps to the aggregate of every entry it had on the wire: a
+    [`BoundsSet`][frequenz.client.common.metrics.BoundsSet] when all are
+    well-formed, or an
+    [`InvalidBoundsSet`][frequenz.client.common.metrics.InvalidBoundsSet]
+    preserving all the raw bounds when any is malformed. A metric with no
+    configured limits maps to the empty, unbounded
+    [`BoundsSet`][frequenz.client.common.metrics.BoundsSet].
     """
 
     category_specific_info: CategorySpecificInfo | None
@@ -1284,42 +1288,44 @@ def electrical_component_from_proto_with_issues(
 
 def _metric_config_bounds_from_proto(
     message: Sequence[electrical_components_pb2.MetricConfigBounds],
-) -> dict[Metric | int, Bounds | InvalidBounds]:
-    """Convert a `MetricConfigBounds` message to a dictionary mapping `Metric` to bounds.
+) -> dict[Metric | int, BoundsSet | InvalidBoundsSet]:
+    """Convert `MetricConfigBounds` messages to a mapping of metric to bounds set.
 
     The keys of the result map are
     [`Metric`][frequenz.client.common.metrics.Metric] enum members (or `int` for
-    unrecognized values). Values are
-    [`Bounds`][frequenz.client.common.metrics.Bounds] for well-formed entries and
-    [`InvalidBounds`][frequenz.client.common.metrics.InvalidBounds] for entries
-    that carried bound values violating `lower <= upper`.
+    unrecognized values, and `0` for the unspecified metric). Each value
+    aggregates *every* entry that named the metric into a single set: a
+    [`BoundsSet`][frequenz.client.common.metrics.BoundsSet] (their union) when
+    all of the metric's `config_bounds` are well-formed, or an
+    [`InvalidBoundsSet`][frequenz.client.common.metrics.InvalidBoundsSet]
+    preserving all the raw bounds in wire order when any is malformed.
 
     An entry with no configured limits — its `config_bounds` submessage absent,
-    or present but empty — loads as an unbounded
-    [`Bounds`][frequenz.client.common.metrics.Bounds]: a `Bounds` with neither
-    `lower` nor `upper` set imposes no limit in either direction. Absence and a
+    or present but empty — contributes an unbounded `Bounds()` (a `Bounds` with
+    neither `lower` nor `upper` set imposes no limit in either direction), which
+    normalizes into the empty, unbounded
+    [`BoundsSet`][frequenz.client.common.metrics.BoundsSet]. Absence and a
     present-but-empty submessage are intentionally treated the same.
 
-    Duplicated metrics on the wire follow proto3 map semantics: the last entry
-    wins silently.
-
     Args:
-        message: The `MetricConfigBounds` message.
+        message: The `MetricConfigBounds` messages.
 
     Returns:
-        The resulting dictionary mapping metrics to their bounds.
+        A mapping from each metric to the set of its configured bounds.
     """
-    bounds: dict[Metric | int, Bounds | InvalidBounds] = {}
+    grouped: dict[Metric | int, list[bounds_pb2.Bounds]] = {}
     for metric_bound in message:
         with warnings.catch_warnings():
             warnings.filterwarnings("ignore", category=DeprecationWarning)
             metric = enum_from_proto(metric_bound.metric, Metric)
             if metric is Metric.UNSPECIFIED:
                 metric = metric.value
 
-        bounds[metric] = bounds_from_proto2(metric_bound.config_bounds)
+        grouped.setdefault(metric, []).append(metric_bound.config_bounds)
 
-    return bounds
+    return {
+        metric: bounds_set_from_proto(configs) for metric, configs in grouped.items()
+    }
 
 
 def _get_operational_lifetime_from_proto(