Back to report index

iolabs-common (shared config layer) dc43ccb: AI3D-379 Review fixes: unwrap nested Annotated in coercion, reject non-mapping raw/overrides, alias names in value errors

Miroslav Simko <ms@iolabs.ch> 2026-09-02T08:56:50+02:00

Commit #3 ยท 14 snippets

 src/iolabs/common/config_loader.py | 16 ++++++++-
 src/iolabs/common/config_model.py  | 17 ++++++++++
 tests/test_config_model.py         | 66 +++++++++++++++++++++++++++++++++++++-
 3 files changed, 97 insertions(+), 2 deletions(-)
Importance #1: src/iolabs/common/config_model.py @@ -44,8 +44,13 @@
44 context="foo config",44 context="foo config",
45 error_cls=FooConfigError,45 error_cls=FooConfigError,
46 ).model_dump()46 ).model_dump()
4747
48Nest sections as `ConfigModel` subclasses, not stdlib dataclasses: a dataclass
49section is coerced, but its errors do not carry the section path. Writers that
50serialise a config (run stats, provenance) should use ``model_dump(mode="json")``
51so tuples, paths and enums become JSON-native values.
52
48Unknown keys are rejected (``extra="forbid"``) with the same message shape as53Unknown keys are rejected (``extra="forbid"``) with the same message shape as
49:func:`iolabs.common.config_loader.validate_allowed_keys`; instances are frozen,54:func:`iolabs.common.config_loader.validate_allowed_keys`; instances are frozen,
50so no field can be rebound after construction. Freezing is shallow, as in55so no field can be rebound after construction. Freezing is shallow, as in
51pydantic itself: a ``list``-valued field is still a mutable list, so prefer56pydantic itself: a ``list``-valued field is still a mutable list, so prefer
Importance #2: src/iolabs/common/config_model.py @@ -147,8 +152,10 @@
147 Raises:152 Raises:
148 error_cls: *raw* holds an unknown key, misses a required key, or holds153 error_cls: *raw* holds an unknown key, misses a required key, or holds
149 a value that is not valid for its declared field type.154 a value that is not valid for its declared field type.
150 """155 """
156 if not isinstance(raw, Mapping):
157 raise error_cls(f"{context} must be a mapping, got {type(raw).__name__}")
151 try:158 try:
152 return model_cls.model_validate(dict(raw))159 return model_cls.model_validate(dict(raw))
153 except pydantic.ValidationError as exc:160 except pydantic.ValidationError as exc:
154 raise error_cls(161 raise error_cls(
Importance #3: src/iolabs/common/config_model.py @@ -197,8 +204,12 @@
197 logger.debug("Loaded %s from %s", context, config_path)204 logger.debug("Loaded %s from %s", context, config_path)
198 else:205 else:
199 raw = _load_packaged(package, filename, error_cls=error_cls)206 raw = _load_packaged(package, filename, error_cls=error_cls)
200 logger.debug("Loaded %s defaults from %s:%s", context, package, filename)207 logger.debug("Loaded %s defaults from %s:%s", context, package, filename)
208 if overrides is not None and not isinstance(overrides, Mapping):
209 raise error_cls(
210 f"{context} overrides must be a mapping, got {type(overrides).__name__}"
211 )
201 if overrides:212 if overrides:
202 raw = config_loader.deep_merge_dicts(raw, dict(overrides))213 raw = config_loader.deep_merge_dicts(raw, dict(overrides))
203 return validate_config(model_cls, raw, context=context, error_cls=error_cls)214 return validate_config(model_cls, raw, context=context, error_cls=error_cls)
204215
Importance #4: src/iolabs/common/config_model.py @@ -298,8 +309,14 @@
298 head, _, tail = message.partition(" for '")309 head, _, tail = message.partition(" for '")
299 name, quote, rest = tail.partition("': ")310 name, quote, rest = tail.partition("': ")
300 if not head.startswith("Invalid ") or not quote or "'" in name:311 if not head.startswith("Invalid ") or not quote or "'" in name:
301 return None312 return None
313 # The coercion names the Python field (plus any ``[index]`` suffix); the
314 # config key may differ when the field is aliased, so prefer the key
315 # pydantic reports unless the message carries an item suffix.
316 key = str(loc[-1])
317 if not name.startswith(key):
318 name = key
302 dotted = ".".join([*(str(part) for part in loc[:-1]), name])319 dotted = ".".join([*(str(part) for part in loc[:-1]), name])
303 return f"{head} for '{dotted}': {rest}"320 return f"{head} for '{dotted}': {rest}"
304321
305322
Importance #5: src/iolabs/common/config_loader.py @@ -58,9 +58,18 @@
58from dataclasses import fields, is_dataclass58from dataclasses import fields, is_dataclass
59from importlib import resources59from importlib import resources
60from pathlib import Path60from pathlib import Path
61from types import UnionType61from types import UnionType
62from typing import Any, Literal, TypeVar, Union, get_args, get_origin, get_type_hints62from typing import (
63 Annotated,
64 Any,
65 Literal,
66 TypeVar,
67 Union,
68 get_args,
69 get_origin,
70 get_type_hints,
71)
6372
64logger = logging.getLogger(__name__)73logger = logging.getLogger(__name__)
6574
66_PYDANTIC_EXPORTS = frozenset(75_PYDANTIC_EXPORTS = frozenset(
Importance #6: src/iolabs/common/config_loader.py @@ -362,8 +371,13 @@
362 error_cls: *value* is not valid for *declared*.371 error_cls: *value* is not valid for *declared*.
363 """372 """
364 if isinstance(declared, str):373 if isinstance(declared, str):
365 declared = _SCALAR_ALIASES.get(declared, declared)374 declared = _SCALAR_ALIASES.get(declared, declared)
375 # pydantic strips only the outermost ``Annotated`` from a field annotation;
376 # one nested in ``Optional[...]``/``list[...]``/``tuple[...]`` still carries
377 # its metadata here and must be unwrapped so the matrix applies to it.
378 while get_origin(declared) is Annotated:
379 declared = get_args(declared)[0]
366 origin = get_origin(declared)380 origin = get_origin(declared)
367 if origin is Literal:381 if origin is Literal:
368 return _coerce_literal(name, value, get_args(declared), error_cls=error_cls)382 return _coerce_literal(name, value, get_args(declared), error_cls=error_cls)
369 if origin is Union or origin is UnionType:383 if origin is Union or origin is UnionType:
Importance #7: tests/test_config_model.py @@ -7,9 +7,9 @@
7import json7import json
8import math8import math
9import warnings9import warnings
10from pathlib import Path10from pathlib import Path
11from typing import Any, Literal11from typing import Annotated, Any, Literal
12from unittest.mock import patch12from unittest.mock import patch
1313
14import pydantic14import pydantic
15import pytest15import pytest
Importance #8: tests/test_config_model.py @@ -622,4 +622,68 @@
622 config_loader.validate_config(Pair, {"nope": 1}, context="pair config")622 config_loader.validate_config(Pair, {"nope": 1}, context="pair config")
623 assert str(excinfo.value) == (623 assert str(excinfo.value) == (
624 "Unknown pair config key(s): nope. Allowed keys: alpha, beta"624 "Unknown pair config key(s): nope. Allowed keys: alpha, beta"
625 )625 )
626
627
628def test_annotated_inside_optional_and_sequences_uses_fleet_matrix() -> None:
629 """``Annotated`` nested in Optional/list/tuple still gets the coercion matrix."""
630
631 class Nested(config_loader.ConfigModel):
632 maybe: Annotated[int, pydantic.Field(ge=0)] | None = None
633 items: list[Annotated[float, pydantic.Field(gt=0)]] = [1.0]
634 pair: tuple[Annotated[int, pydantic.Field(ge=0)], Annotated[int, pydantic.Field(ge=0)]] = (
635 1,
636 2,
637 )
638
639 assert Nested(maybe="1e3").maybe == 1000
640 assert Nested(items=["2.5"]).items == [2.5]
641 assert Nested(pair=[3.0, "4"]).pair == (3, 4)
642 for bad in ({"maybe": True}, {"items": [True]}, {"pair": [True, 1]}):
643 with pytest.raises(pydantic.ValidationError):
644 Nested(**bad)
645 with pytest.raises(pydantic.ValidationError):
646 Nested(maybe=-1)
647
648
649def test_validate_config_rejects_non_mapping_input() -> None:
650 """A non-mapping raw config raises the config error, not a bare TypeError."""
651
652 class Simple(config_loader.ConfigModel):
653 count: int = 1
654
655 for raw in (None, "count", [("count", 1)]):
656 with pytest.raises(config_loader.ConfigError, match="must be a mapping"):
657 config_loader.validate_config(Simple, raw, context="simple config") # type: ignore[arg-type]
658
659
660def test_load_config_rejects_non_mapping_overrides(tmp_path: Path) -> None:
661 """String overrides (an unparsed --set) raise the config error."""
662
663 class Simple(config_loader.ConfigModel):
664 count: int = 1
665
666 config_file = tmp_path / "simple.json"
667 config_file.write_text(json.dumps({"count": 2}), encoding="utf-8")
668 with pytest.raises(config_loader.ConfigError, match="overrides must be a mapping"):
669 config_loader.load_config(
670 Simple,
671 package="iolabs.common",
672 filename="unused.json",
673 overrides="count=3", # type: ignore[arg-type]
674 config_path=config_file,
675 context="simple config",
676 )
677
678
679def test_value_error_names_the_alias_of_an_aliased_field() -> None:
680 """Coercion errors name the config key (alias), matching unknown-key messages."""
681
682 class Aliased(config_loader.ConfigModel):
683 internal_name: int = pydantic.Field(default=1, alias="external-name")
684
685 with pytest.raises(config_loader.ConfigError) as excinfo:
686 config_loader.validate_config(
687 Aliased, {"external-name": "abc"}, context="aliased config"
688 )
689 assert str(excinfo.value).startswith("Invalid int for 'external-name': 'abc'")
Importance #9: src/iolabs/common/config_model.py @@ -44,8 +44,13 @@
44 context="foo config",44 context="foo config",
45 error_cls=FooConfigError,45 error_cls=FooConfigError,
46 ).model_dump()46 ).model_dump()
4747
48Nest sections as `ConfigModel` subclasses, not stdlib dataclasses: a dataclass
49section is coerced, but its errors do not carry the section path. Writers that
50serialise a config (run stats, provenance) should use ``model_dump(mode="json")``
51so tuples, paths and enums become JSON-native values.
52
48Unknown keys are rejected (``extra="forbid"``) with the same message shape as53Unknown keys are rejected (``extra="forbid"``) with the same message shape as
49:func:`iolabs.common.config_loader.validate_allowed_keys`; instances are frozen,54:func:`iolabs.common.config_loader.validate_allowed_keys`; instances are frozen,
50so no field can be rebound after construction. Freezing is shallow, as in55so no field can be rebound after construction. Freezing is shallow, as in
51pydantic itself: a ``list``-valued field is still a mutable list, so prefer56pydantic itself: a ``list``-valued field is still a mutable list, so prefer
Importance #10: src/iolabs/common/config_model.py @@ -147,8 +152,10 @@
147 Raises:152 Raises:
148 error_cls: *raw* holds an unknown key, misses a required key, or holds153 error_cls: *raw* holds an unknown key, misses a required key, or holds
149 a value that is not valid for its declared field type.154 a value that is not valid for its declared field type.
150 """155 """
156 if not isinstance(raw, Mapping):
157 raise error_cls(f"{context} must be a mapping, got {type(raw).__name__}")
151 try:158 try:
152 return model_cls.model_validate(dict(raw))159 return model_cls.model_validate(dict(raw))
153 except pydantic.ValidationError as exc:160 except pydantic.ValidationError as exc:
154 raise error_cls(161 raise error_cls(
Importance #11: src/iolabs/common/config_model.py @@ -197,8 +204,12 @@
197 logger.debug("Loaded %s from %s", context, config_path)204 logger.debug("Loaded %s from %s", context, config_path)
198 else:205 else:
199 raw = _load_packaged(package, filename, error_cls=error_cls)206 raw = _load_packaged(package, filename, error_cls=error_cls)
200 logger.debug("Loaded %s defaults from %s:%s", context, package, filename)207 logger.debug("Loaded %s defaults from %s:%s", context, package, filename)
208 if overrides is not None and not isinstance(overrides, Mapping):
209 raise error_cls(
210 f"{context} overrides must be a mapping, got {type(overrides).__name__}"
211 )
201 if overrides:212 if overrides:
202 raw = config_loader.deep_merge_dicts(raw, dict(overrides))213 raw = config_loader.deep_merge_dicts(raw, dict(overrides))
203 return validate_config(model_cls, raw, context=context, error_cls=error_cls)214 return validate_config(model_cls, raw, context=context, error_cls=error_cls)
204215
Importance #12: src/iolabs/common/config_model.py @@ -298,8 +309,14 @@
298 head, _, tail = message.partition(" for '")309 head, _, tail = message.partition(" for '")
299 name, quote, rest = tail.partition("': ")310 name, quote, rest = tail.partition("': ")
300 if not head.startswith("Invalid ") or not quote or "'" in name:311 if not head.startswith("Invalid ") or not quote or "'" in name:
301 return None312 return None
313 # The coercion names the Python field (plus any ``[index]`` suffix); the
314 # config key may differ when the field is aliased, so prefer the key
315 # pydantic reports unless the message carries an item suffix.
316 key = str(loc[-1])
317 if not name.startswith(key):
318 name = key
302 dotted = ".".join([*(str(part) for part in loc[:-1]), name])319 dotted = ".".join([*(str(part) for part in loc[:-1]), name])
303 return f"{head} for '{dotted}': {rest}"320 return f"{head} for '{dotted}': {rest}"
304321
305322
Importance #13: tests/test_config_model.py @@ -7,9 +7,9 @@
7import json7import json
8import math8import math
9import warnings9import warnings
10from pathlib import Path10from pathlib import Path
11from typing import Any, Literal11from typing import Annotated, Any, Literal
12from unittest.mock import patch12from unittest.mock import patch
1313
14import pydantic14import pydantic
15import pytest15import pytest
Importance #14: tests/test_config_model.py @@ -622,4 +622,68 @@
622 config_loader.validate_config(Pair, {"nope": 1}, context="pair config")622 config_loader.validate_config(Pair, {"nope": 1}, context="pair config")
623 assert str(excinfo.value) == (623 assert str(excinfo.value) == (
624 "Unknown pair config key(s): nope. Allowed keys: alpha, beta"624 "Unknown pair config key(s): nope. Allowed keys: alpha, beta"
625 )625 )
626
627
628def test_annotated_inside_optional_and_sequences_uses_fleet_matrix() -> None:
629 """``Annotated`` nested in Optional/list/tuple still gets the coercion matrix."""
630
631 class Nested(config_loader.ConfigModel):
632 maybe: Annotated[int, pydantic.Field(ge=0)] | None = None
633 items: list[Annotated[float, pydantic.Field(gt=0)]] = [1.0]
634 pair: tuple[Annotated[int, pydantic.Field(ge=0)], Annotated[int, pydantic.Field(ge=0)]] = (
635 1,
636 2,
637 )
638
639 assert Nested(maybe="1e3").maybe == 1000
640 assert Nested(items=["2.5"]).items == [2.5]
641 assert Nested(pair=[3.0, "4"]).pair == (3, 4)
642 for bad in ({"maybe": True}, {"items": [True]}, {"pair": [True, 1]}):
643 with pytest.raises(pydantic.ValidationError):
644 Nested(**bad)
645 with pytest.raises(pydantic.ValidationError):
646 Nested(maybe=-1)
647
648
649def test_validate_config_rejects_non_mapping_input() -> None:
650 """A non-mapping raw config raises the config error, not a bare TypeError."""
651
652 class Simple(config_loader.ConfigModel):
653 count: int = 1
654
655 for raw in (None, "count", [("count", 1)]):
656 with pytest.raises(config_loader.ConfigError, match="must be a mapping"):
657 config_loader.validate_config(Simple, raw, context="simple config") # type: ignore[arg-type]
658
659
660def test_load_config_rejects_non_mapping_overrides(tmp_path: Path) -> None:
661 """String overrides (an unparsed --set) raise the config error."""
662
663 class Simple(config_loader.ConfigModel):
664 count: int = 1
665
666 config_file = tmp_path / "simple.json"
667 config_file.write_text(json.dumps({"count": 2}), encoding="utf-8")
668 with pytest.raises(config_loader.ConfigError, match="overrides must be a mapping"):
669 config_loader.load_config(
670 Simple,
671 package="iolabs.common",
672 filename="unused.json",
673 overrides="count=3", # type: ignore[arg-type]
674 config_path=config_file,
675 context="simple config",
676 )
677
678
679def test_value_error_names_the_alias_of_an_aliased_field() -> None:
680 """Coercion errors name the config key (alias), matching unknown-key messages."""
681
682 class Aliased(config_loader.ConfigModel):
683 internal_name: int = pydantic.Field(default=1, alias="external-name")
684
685 with pytest.raises(config_loader.ConfigError) as excinfo:
686 config_loader.validate_config(
687 Aliased, {"external-name": "abc"}, context="aliased config"
688 )
689 assert str(excinfo.value).startswith("Invalid int for 'external-name': 'abc'")