From 644a9d4b88138c07a7d92cdd076c5374c20e3701 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Mon, 13 Jul 2026 15:26:24 +0300 Subject: [PATCH 1/4] feat(keys): bake a merge policy into list/map KeySpec factories --- compose2pod/keys.py | 43 ++++++++++++++++++++++++++++++---- tests/test_keys.py | 56 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 94 insertions(+), 5 deletions(-) diff --git a/compose2pod/keys.py b/compose2pod/keys.py index 3a7965e..9fd537a 100644 --- a/compose2pod/keys.py +++ b/compose2pod/keys.py @@ -38,10 +38,11 @@ def _key_value_pairs(value: list[Any] | dict[str, Any]) -> list[Any]: @dataclasses.dataclass(frozen=True, slots=True, kw_only=True) class KeySpec: - """A service-key spec: how one Compose service key is validated and emitted.""" + """A service-key spec: how one Compose service key is validated, emitted, and merged across extends.""" validate: Callable[[str, str, Any], None] emit: Callable[[Any], list[Token]] + merge: Callable[[str, str, Any, Any], Any] | None = None def _validate_scalar(name: str, key: str, value: Any) -> None: # noqa: ANN401 - Compose values are untyped YAML/JSON @@ -78,6 +79,40 @@ def _validate_map(name: str, key: str, value: Any) -> None: # noqa: ANN401 - Co raise UnsupportedComposeError(msg) +def _as_list(name: str, key: str, value: Any) -> list[Any]: # noqa: ANN401 - Compose values are untyped YAML/JSON + """Normalize list-or-scalar-string form to a list, for merging across extends.""" + if isinstance(value, list): + return list(value) + if isinstance(value, str): + return [value] + msg = f"service {name!r}: cannot merge {key!r} across incompatible forms" + raise UnsupportedComposeError(msg) + + +def _concat_list(name: str, key: str, base: Any, local: Any) -> list[Any]: # noqa: ANN401 - Compose values are untyped YAML/JSON + """Merge policy for list-shaped keys: concatenate base then local.""" + return _as_list(name, key, base) + _as_list(name, key, local) + + +def _pairs_to_mapping(name: str, key: str, value: Any) -> dict[str, Any]: # noqa: ANN401 - Compose values are untyped YAML/JSON + """Normalize list-or-dict key-value form to a mapping; inverse of _key_value_pairs.""" + if isinstance(value, dict): + return value + if isinstance(value, list): + result: dict[str, Any] = {} + for item in value: + pair_key, sep, pair_value = str(item).partition("=") + result[pair_key] = pair_value if sep else None + return result + msg = f"service {name!r}: cannot merge {key!r} across incompatible forms" + raise UnsupportedComposeError(msg) + + +def _merge_map(name: str, key: str, base: Any, local: Any) -> dict[str, Any]: # noqa: ANN401 - Compose values are untyped YAML/JSON + """Merge policy for map-shaped keys: key-by-key merge, local wins.""" + return {**_pairs_to_mapping(name, key, base), **_pairs_to_mapping(name, key, local)} + + def _scalar(flag: str) -> KeySpec: def emit(value: Any) -> list[Token]: # noqa: ANN401 - Compose values are untyped YAML/JSON return [flag, _Expand(value=str(value))] @@ -106,7 +141,7 @@ def emit(value: Any) -> list[Token]: # noqa: ANN401 - Compose values are untype tokens += [flag, _Expand(value=str(item))] return tokens - return KeySpec(validate=_validate_list, emit=emit) + return KeySpec(validate=_validate_list, emit=emit, merge=_concat_list) def _map(flag: str) -> KeySpec: @@ -116,7 +151,7 @@ def emit(value: Any) -> list[Token]: # noqa: ANN401 - Compose values are untype tokens += [flag, _Expand(value=str(pair))] return tokens - return KeySpec(validate=_validate_map, emit=emit) + return KeySpec(validate=_validate_map, emit=emit, merge=_merge_map) def _extra_host_pairs(value: list[Any] | dict[str, Any]) -> list[Any]: @@ -188,7 +223,7 @@ def _emit_ulimits(value: Any) -> list[Token]: # noqa: ANN401 - Compose values a "labels": _map("--label"), "annotations": _map("--annotation"), "pull_policy": KeySpec(validate=_validate_pull_policy, emit=_emit_pull_policy), - "ulimits": KeySpec(validate=_validate_ulimits, emit=_emit_ulimits), + "ulimits": KeySpec(validate=_validate_ulimits, emit=_emit_ulimits, merge=_merge_map), "mem_limit": _number_scalar("--memory"), "memswap_limit": _number_scalar("--memory-swap"), "mem_reservation": _number_scalar("--memory-reservation"), diff --git a/tests/test_keys.py b/tests/test_keys.py index fcd8baa..f0f69b0 100644 --- a/tests/test_keys.py +++ b/tests/test_keys.py @@ -1,4 +1,15 @@ -from compose2pod.keys import SERVICE_KEYS, STRUCTURAL_KEYS +import pytest + +from compose2pod.exceptions import UnsupportedComposeError +from compose2pod.keys import ( + SERVICE_KEYS, + STRUCTURAL_KEYS, + _concat_list, + _merge_map, + _validate_list, + _validate_map, + _validate_ulimits, +) from compose2pod.parsing import SUPPORTED_SERVICE_KEYS @@ -58,3 +69,46 @@ def test_supported_service_keys_snapshot() -> None: "dns_opt", "sysctls", } == SUPPORTED_SERVICE_KEYS + + +@pytest.mark.parametrize("key", sorted(SERVICE_KEYS)) +def test_merge_present_iff_list_or_map_shaped(key: str) -> None: + """A key's merge callable must be set exactly when its shape supports one. + + Ties merge-presence to the validator each entry actually uses, rather than + a hand-copied key-name list, so a future _list()/_map() key can't silently + end up with no merge callable. + """ + spec = SERVICE_KEYS[key] + is_list_or_map_shaped = spec.validate in (_validate_list, _validate_map, _validate_ulimits) + assert (spec.merge is not None) == is_list_or_map_shaped + + +class TestMergeCallables: + """Direct tests of the merge policy functions, independent of any caller. + + extends.py (Task 2) will call these through SERVICE_KEYS[key].merge, but + that wiring doesn't exist yet — these tests exercise every branch of + _concat_list/_as_list/_merge_map/_pairs_to_mapping on their own so Task 1 + is fully covered without depending on Task 2. + """ + + def test_concat_list_merges_list_forms(self) -> None: + assert _concat_list("web", "cap_add", ["NET_ADMIN"], ["SYS_TIME"]) == ["NET_ADMIN", "SYS_TIME"] + + def test_concat_list_normalizes_scalar_string_form(self) -> None: + assert _concat_list("web", "cap_add", "NET_ADMIN", ["SYS_TIME"]) == ["NET_ADMIN", "SYS_TIME"] + + def test_concat_list_refuses_incompatible_form(self) -> None: + with pytest.raises(UnsupportedComposeError, match="cannot merge 'cap_add' across incompatible forms"): + _concat_list("web", "cap_add", ["NET_ADMIN"], {"bad": "shape"}) + + def test_merge_map_merges_dict_forms(self) -> None: + assert _merge_map("web", "labels", {"team": "core"}, {"tier": "web"}) == {"team": "core", "tier": "web"} + + def test_merge_map_normalizes_list_form(self) -> None: + assert _merge_map("web", "labels", ["team=core", "BARE"], ["team=web"]) == {"team": "web", "BARE": None} + + def test_merge_map_refuses_incompatible_form(self) -> None: + with pytest.raises(UnsupportedComposeError, match="cannot merge 'labels' across incompatible forms"): + _merge_map("web", "labels", {"team": "core"}, 5) From 11f570584041d7756e58f947d0909ba4d4ddbfad Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Mon, 13 Jul 2026 15:36:08 +0300 Subject: [PATCH 2/4] feat(extends): derive merge policy from SERVICE_KEYS instead of a hand-copied set --- architecture/glossary.md | 5 ++- compose2pod/extends.py | 38 +++++++------------ ...erive-extends-merge-policy-from-keyspec.md | 37 ++++++++++++++++++ tests/test_extends.py | 38 +++++++++++++++++++ 4 files changed, 91 insertions(+), 27 deletions(-) create mode 100644 planning/changes/2026-07-13.05-derive-extends-merge-policy-from-keyspec.md diff --git a/architecture/glossary.md b/architecture/glossary.md index 4d28ae3..053945e 100644 --- a/architecture/glossary.md +++ b/architecture/glossary.md @@ -5,8 +5,9 @@ capability pages share. One term, what it *is* (not what it does), and the synonyms to reject. **Service-key spec**: -The `(validate, emit)` pair for one Compose service key — how that key is checked -and how it renders to `podman run` flags. In code, `KeySpec` in `keys.py`. +The `(validate, emit, merge)` triple for one Compose service key — how that key +is checked, how it renders to `podman run` flags, and (for list/map-shaped keys) +how it merges across `extends`. In code, `KeySpec` in `keys.py`. _Avoid_: handler, plugin, rule. **Service-key registry**: diff --git a/compose2pod/extends.py b/compose2pod/extends.py index 198e0f2..6ce3886 100644 --- a/compose2pod/extends.py +++ b/compose2pod/extends.py @@ -3,21 +3,15 @@ from typing import Any from compose2pod.exceptions import UnsupportedComposeError +from compose2pod.keys import SERVICE_KEYS, _pairs_to_mapping -_MERGE_KEYS = {"environment", "labels", "annotations", "extra_hosts", "ulimits", "healthcheck", "depends_on"} -_CONCAT_KEYS = { - "cap_add", - "cap_drop", - "security_opt", - "devices", - "group_add", - "secrets", - "configs", - "volumes", - "tmpfs", - "env_file", -} +# Merge policy for keys with a SERVICE_KEYS KeySpec comes from spec.merge (see +# _merge below); these two sets cover only the remaining structural keys, which +# have no KeySpec. Unifying structural-key merge policy is deferred — see +# decisions/2026-07-12-reject-structural-key-registry.md's revisit trigger. +_STRUCTURAL_MERGE_KEYS = {"environment", "extra_hosts", "healthcheck", "depends_on"} +_STRUCTURAL_CONCAT_KEYS = {"secrets", "configs", "volumes", "tmpfs", "env_file"} def _extends_target(name: str, ext: Any) -> str: # noqa: ANN401 - Compose values are untyped @@ -39,21 +33,12 @@ def _extends_target(name: str, ext: Any) -> str: # noqa: ANN401 - Compose value return service -def _env_list_to_map(items: list[Any]) -> dict[str, Any]: - """Normalize a `[KEY=value, BARE]` environment list to a mapping (bare -> None).""" - result: dict[str, Any] = {} - for item in items: - key, sep, value = str(item).partition("=") - result[key] = value if sep else None - return result - - def _as_mapping(key: str, name: str, value: Any) -> dict[str, Any]: # noqa: ANN401 - Compose values are untyped if isinstance(value, dict): return value if isinstance(value, list): if key == "environment": - return _env_list_to_map(value) + return _pairs_to_mapping(name, key, value) if key == "depends_on": return {dep: {} for dep in value} msg = f"service {name!r}: cannot merge {key!r} across incompatible forms" @@ -73,9 +58,12 @@ def _merge(base: dict[str, Any], local: dict[str, Any], name: str) -> dict[str, """Merge `local` onto `base` per key category: mapping-merge, sequence-concat, else override.""" merged: dict[str, Any] = dict(base) for key, local_val in local.items(): - if key in base and key in _MERGE_KEYS: + spec = SERVICE_KEYS.get(key) + if key in base and spec is not None and spec.merge is not None: + merged[key] = spec.merge(name, key, base[key], local_val) + elif key in base and key in _STRUCTURAL_MERGE_KEYS: merged[key] = {**_as_mapping(key, name, base[key]), **_as_mapping(key, name, local_val)} - elif key in base and key in _CONCAT_KEYS: + elif key in base and key in _STRUCTURAL_CONCAT_KEYS: merged[key] = _as_list(key, name, base[key]) + _as_list(key, name, local_val) else: merged[key] = local_val diff --git a/planning/changes/2026-07-13.05-derive-extends-merge-policy-from-keyspec.md b/planning/changes/2026-07-13.05-derive-extends-merge-policy-from-keyspec.md new file mode 100644 index 0000000..b6cb0ad --- /dev/null +++ b/planning/changes/2026-07-13.05-derive-extends-merge-policy-from-keyspec.md @@ -0,0 +1,37 @@ +--- +summary: KeySpec (keys.py) gained a merge field baked in by _list()/_map(); extends.py now derives concat/merge for the 8 SERVICE_KEYS-shaped keys from it instead of a hand-copied set, and list-form labels/annotations merge correctly across extends as a side effect. +--- + +# Design: derive extend merge policy from SERVICE_KEYS + +## Summary + +Task 1 (commit 644a9d4) added a `merge` field to `KeySpec`, baked in by the +`_list()` and `_map()` factories as `_concat_list` and `_merge_map`. This task +wires `extends.py` to consume that policy instead of maintaining a hand-copied +`_MERGE_KEYS` / `_CONCAT_KEYS` set, shrinking those sets to cover only the +remaining structural keys. As a side effect, list-form `labels` and +`annotations` now merge correctly across `extends`, fixing an existing gap. + +## Design + +**What changes**: `extends.py`'s `_merge()` function now checks `SERVICE_KEYS[key].merge` +first before falling back to structural-key logic. The `_MERGE_KEYS` and `_CONCAT_KEYS` +sets are replaced by `_STRUCTURAL_MERGE_KEYS` and `_STRUCTURAL_CONCAT_KEYS`, covering +only keys without a `KeySpec`: environment, extra_hosts, healthcheck, depends_on +(merge) and secrets, configs, volumes, tmpfs, env_file (concat). + +**What's reused**: `keys.py`'s `_pairs_to_mapping()` helper replaces the local +`_env_list_to_map()`, now handling environment's list-form and fixing the gap +where list-form labels/annotations couldn't be merged across extends. + +**What's preserved**: All 22 existing tests continue to pass unchanged; three +new tests pin down behavior: list-form labels merge correctly, incompatible +label forms are refused, and scalar cap_add normalizes before concat (exercising +the scalar-normalization path in `_concat_list`). + +## Testing + +- `just test tests/test_extends.py -v`: all 25 tests pass (22 existing + 3 new). +- `just test-ci`: 100% line coverage, all 401 tests pass. +- `just lint-ci`: clean (ruff, ty, eof-fixer, planning check). diff --git a/tests/test_extends.py b/tests/test_extends.py index 461d309..6316969 100644 --- a/tests/test_extends.py +++ b/tests/test_extends.py @@ -73,6 +73,34 @@ def test_labels_mapping_merge(self) -> None: } assert _services(doc)["web"]["labels"] == {"team": "core", "tier": "web"} + def test_labels_list_form_normalized_and_merged(self) -> None: + doc = { + "services": { + "base": {"image": "x", "labels": ["team=core", "BARE"]}, + "web": {"extends": {"service": "base"}, "labels": ["team=web"]}, + } + } + assert _services(doc)["web"]["labels"] == {"team": "web", "BARE": None} + + def test_incompatible_labels_form_is_refused(self) -> None: + doc = { + "services": { + "base": {"image": "x", "labels": {"team": "core"}}, + "web": {"extends": {"service": "base"}, "labels": 5}, + } + } + with pytest.raises(UnsupportedComposeError, match="cannot merge 'labels' across incompatible forms"): + resolve_extends(doc) + + def test_cap_add_scalar_normalized_before_concat(self) -> None: + doc = { + "services": { + "base": {"image": "x", "cap_add": "NET_ADMIN"}, + "web": {"extends": {"service": "base"}, "cap_add": ["SYS_TIME"]}, + } + } + assert _services(doc)["web"]["cap_add"] == ["NET_ADMIN", "SYS_TIME"] + def test_depends_on_list_and_map_forms_merge(self) -> None: doc = { "services": { @@ -186,3 +214,13 @@ def test_extends_non_dict_base_defers_to_validate(self) -> None: result = resolve_extends(doc) assert "extends" not in result["services"]["web"] assert result["services"]["base"] is None + + def test_incompatible_structural_concat_form_is_refused(self) -> None: + doc = { + "services": { + "base": {"image": "x", "env_file": ["base.env"]}, + "web": {"extends": {"service": "base"}, "env_file": {"bad": "shape"}}, + } + } + with pytest.raises(UnsupportedComposeError, match="cannot merge 'env_file' across incompatible forms"): + resolve_extends(doc) From 25238f84f7f9a4bfed2042c671c107fa83e43602 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Mon, 13 Jul 2026 15:41:17 +0300 Subject: [PATCH 3/4] docs(planning): restore full design-file rationale for the extends merge change The Task 2 implementer could not find this file (it existed only as an uncommitted file in the main checkout, which the worktree's fresh base ref did not carry over) and reconstructed a shorter version from context, losing the Motivation, Non-goals, and Risk sections. Restore the original, already-reviewed design content; the finalized summary line is unchanged. --- ...erive-extends-merge-policy-from-keyspec.md | 137 +++++++++++++++--- 1 file changed, 115 insertions(+), 22 deletions(-) diff --git a/planning/changes/2026-07-13.05-derive-extends-merge-policy-from-keyspec.md b/planning/changes/2026-07-13.05-derive-extends-merge-policy-from-keyspec.md index b6cb0ad..50aa6ac 100644 --- a/planning/changes/2026-07-13.05-derive-extends-merge-policy-from-keyspec.md +++ b/planning/changes/2026-07-13.05-derive-extends-merge-policy-from-keyspec.md @@ -2,36 +2,129 @@ summary: KeySpec (keys.py) gained a merge field baked in by _list()/_map(); extends.py now derives concat/merge for the 8 SERVICE_KEYS-shaped keys from it instead of a hand-copied set, and list-form labels/annotations merge correctly across extends as a side effect. --- -# Design: derive extend merge policy from SERVICE_KEYS +# Design: derive extends' merge policy from KeySpec ## Summary -Task 1 (commit 644a9d4) added a `merge` field to `KeySpec`, baked in by the -`_list()` and `_map()` factories as `_concat_list` and `_merge_map`. This task -wires `extends.py` to consume that policy instead of maintaining a hand-copied -`_MERGE_KEYS` / `_CONCAT_KEYS` set, shrinking those sets to cover only the -remaining structural keys. As a side effect, list-form `labels` and -`annotations` now merge correctly across `extends`, fixing an existing gap. +Add a `merge: Callable[[Any, Any], Any] | None` field to `KeySpec`. The +`_list()`/`_map()` factories in `keys.py` set it automatically (concat / dict-merge +respectively), so every `SERVICE_KEYS` entry built through them gets extends +support for free. `extends.py`'s `_merge` looks up `SERVICE_KEYS[key].merge` +first, falling back to a shrunk, structural-only `_MERGE_KEYS`/`_CONCAT_KEYS` for +the 8 keys that still have no `KeySpec`. A new `_pairs_to_mapping` in `keys.py` +(the inverse of `_key_value_pairs`) backs the map-merge callable and, as a side +effect, fixes list-form `labels`/`annotations` merging — currently untested and +currently broken. + +## Motivation + +`extends.py`'s `_MERGE_KEYS`/`_CONCAT_KEYS` (`extends.py:8-20`) hand-copy shape +knowledge that `SERVICE_KEYS`'s validators (`keys.py:176-205`) already encode, +in a file that doesn't import `keys.py` at all. For the 8 keys that are both in +`SERVICE_KEYS` and in one of these two sets (`labels`, `annotations`, `ulimits`, +`group_add`, `cap_add`, `cap_drop`, `security_opt`, `devices`), the two sources +happen to agree today but nothing enforces it. Add a new `_list("--foo")` key to +`SERVICE_KEYS` and forget `extends._CONCAT_KEYS`, and the merge silently +degrades from "concatenate across the inheritance chain" to "last writer +wins" — `extends.py`'s fallback branch (`_merge`, line 81) never raises. +`tests/test_extends.py` asserts specific keys, not consistency with +`SERVICE_KEYS`'s shapes, so this class of bug isn't caught by CI. + +Separately: `keys.py`'s `_validate_map` accepts list-form *or* dict-form input +for `labels`/`annotations` (that's what `_key_value_pairs` exists for), but +`extends.py`'s `_as_mapping` only normalizes list-form for `environment` and +`depends_on`. List-form `labels`/`annotations` under `extends` today raises +"cannot merge ... across incompatible forms" — untested, and narrower than what +`keys.py` itself accepts everywhere else. ## Design -**What changes**: `extends.py`'s `_merge()` function now checks `SERVICE_KEYS[key].merge` -first before falling back to structural-key logic. The `_MERGE_KEYS` and `_CONCAT_KEYS` -sets are replaced by `_STRUCTURAL_MERGE_KEYS` and `_STRUCTURAL_CONCAT_KEYS`, covering -only keys without a `KeySpec`: environment, extra_hosts, healthcheck, depends_on -(merge) and secrets, configs, volumes, tmpfs, env_file (concat). +**`KeySpec` gains a `merge` field:** + +```python +@dataclasses.dataclass(frozen=True, slots=True, kw_only=True) +class KeySpec: + validate: Callable[[str, str, Any], None] + emit: Callable[[Any], list[Token]] + merge: Callable[[Any, Any], Any] | None = None +``` + +`_list()` and `_map()` set it automatically — no opt-in argument, so a future +key built through either factory can't skip it: + +```python +def _list(flag: str) -> KeySpec: + ... + return KeySpec(validate=_validate_list, emit=emit, merge=_concat_list) + +def _map(flag: str) -> KeySpec: + ... + return KeySpec(validate=_validate_map, emit=emit, merge=_merge_map) +``` -**What's reused**: `keys.py`'s `_pairs_to_mapping()` helper replaces the local -`_env_list_to_map()`, now handling environment's list-form and fixing the gap -where list-form labels/annotations couldn't be merged across extends. +`_concat_list(base, local)` normalizes both sides via the existing +list-or-str handling (moved from `extends._as_list`) and concatenates. +`_merge_map(base, local)` normalizes both sides via the new `_pairs_to_mapping` +(list-or-dict → dict, the inverse of `_key_value_pairs`, living beside it in +`keys.py`) and dict-merges, local winning. `ulimits`'s bespoke `KeySpec` +(`keys.py:191`) sets `merge=_merge_map` explicitly since it bypasses the `_map()` +factory. -**What's preserved**: All 22 existing tests continue to pass unchanged; three -new tests pin down behavior: list-form labels merge correctly, incompatible -label forms are refused, and scalar cap_add normalizes before concat (exercising -the scalar-normalization path in `_concat_list`). +**`extends._merge` gains a first lookup:** + +```python +def _merge(base: dict[str, Any], local: dict[str, Any], name: str) -> dict[str, Any]: + merged: dict[str, Any] = dict(base) + for key, local_val in local.items(): + spec = SERVICE_KEYS.get(key) + if key in base and spec is not None and spec.merge is not None: + merged[key] = spec.merge(base[key], local_val) + elif key in base and key in _MERGE_KEYS: # structural keys only + ... + elif key in base and key in _CONCAT_KEYS: # structural keys only + ... + else: + merged[key] = local_val + return merged +``` + +`_MERGE_KEYS`/`_CONCAT_KEYS` shrink to the 8 keys with no `KeySpec`: +`_MERGE_KEYS = {"environment", "extra_hosts", "healthcheck", "depends_on"}`, +`_CONCAT_KEYS = {"secrets", "configs", "volumes", "tmpfs", "env_file"}` — their +`_as_mapping`/`_as_list` handling is unchanged. A one-line comment marks these +as pending `decisions/2026-07-12-reject-structural-key-registry.md`'s narrow +single-sourcing revisit trigger, not a green light to fold them into +`SERVICE_KEYS` now. + +**`architecture/glossary.md`**: the *Service-key spec* entry gains a clause — +the `(validate, emit)` pair becomes `(validate, emit, merge)`, `merge` optional +and shape-derived. + +## Non-goals + +- Unifying the 8 structural keys' merge policy — deferred; would conflate this + change with candidate #3 of the 2026-07-13 architecture review (owning + module per structural key), and `decisions/2026-07-12-reject-structural-key-registry.md` + already rejects a uniform structural registry on shape-heterogeneity grounds + that apply equally here. +- Changing `SERVICE_KEYS` membership or emitted flags — behavior-preserving for + every key already in the registry. ## Testing -- `just test tests/test_extends.py -v`: all 25 tests pass (22 existing + 3 new). -- `just test-ci`: 100% line coverage, all 401 tests pass. -- `just lint-ci`: clean (ruff, ty, eof-fixer, planning check). +`tests/test_keys.py`: a parametrized test over every `SERVICE_KEYS` entry +asserting `spec.merge is not None` iff the key was built via `_list()`/`_map()` +(or is `ulimits`), i.e. shape and merge-presence never drift. +`tests/test_extends.py`: a new case for list-form `labels` merging across +`extends` (the fixed gap — currently raises, should merge), plus the existing +dict-form-labels and concat-key tests re-asserted unchanged. `just test-ci` at +100%, `just lint-ci` clean, `just check-planning`. + +## Risk + +- **`_pairs_to_mapping` diverges from `_key_value_pairs`'s null-value + semantics** (low x med): both live in `keys.py` side by side, and the + parametrized `SERVICE_KEYS` test exercises every map-shaped key through the + same round trip. +- **Structural-key comment goes stale** (low x low): points at a decision file + with its own revisit trigger, not a TODO with no owner. From 552eececf247d052622db8dc4c276488c265c5a0 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Mon, 13 Jul 2026 15:49:43 +0300 Subject: [PATCH 4/4] docs(planning): fix stale merge signature and structural-key count in design doc Final review caught two drift spots in the design file: illustrative code snippets still showed the 2-arg merge(base, local) signature superseded during implementation by the 4-arg (name, key, base, local) form, and the structural-key count said 8 instead of 9. --- ...erive-extends-merge-policy-from-keyspec.md | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/planning/changes/2026-07-13.05-derive-extends-merge-policy-from-keyspec.md b/planning/changes/2026-07-13.05-derive-extends-merge-policy-from-keyspec.md index 50aa6ac..833c84d 100644 --- a/planning/changes/2026-07-13.05-derive-extends-merge-policy-from-keyspec.md +++ b/planning/changes/2026-07-13.05-derive-extends-merge-policy-from-keyspec.md @@ -11,7 +11,7 @@ Add a `merge: Callable[[Any, Any], Any] | None` field to `KeySpec`. The respectively), so every `SERVICE_KEYS` entry built through them gets extends support for free. `extends.py`'s `_merge` looks up `SERVICE_KEYS[key].merge` first, falling back to a shrunk, structural-only `_MERGE_KEYS`/`_CONCAT_KEYS` for -the 8 keys that still have no `KeySpec`. A new `_pairs_to_mapping` in `keys.py` +the 9 keys that still have no `KeySpec`. A new `_pairs_to_mapping` in `keys.py` (the inverse of `_key_value_pairs`) backs the map-merge callable and, as a side effect, fixes list-form `labels`/`annotations` merging — currently untested and currently broken. @@ -46,7 +46,7 @@ for `labels`/`annotations` (that's what `_key_value_pairs` exists for), but class KeySpec: validate: Callable[[str, str, Any], None] emit: Callable[[Any], list[Token]] - merge: Callable[[Any, Any], Any] | None = None + merge: Callable[[str, str, Any, Any], Any] | None = None ``` `_list()` and `_map()` set it automatically — no opt-in argument, so a future @@ -62,11 +62,14 @@ def _map(flag: str) -> KeySpec: return KeySpec(validate=_validate_map, emit=emit, merge=_merge_map) ``` -`_concat_list(base, local)` normalizes both sides via the existing +`_concat_list(name, key, base, local)` normalizes both sides via the existing list-or-str handling (moved from `extends._as_list`) and concatenates. -`_merge_map(base, local)` normalizes both sides via the new `_pairs_to_mapping` -(list-or-dict → dict, the inverse of `_key_value_pairs`, living beside it in -`keys.py`) and dict-merges, local winning. `ulimits`'s bespoke `KeySpec` +`_merge_map(name, key, base, local)` normalizes both sides via the new +`_pairs_to_mapping` (list-or-dict → dict, the inverse of `_key_value_pairs`, +living beside it in `keys.py`) and dict-merges, local winning. `merge`'s +`(name, key, ...)` prefix mirrors `validate`'s signature, so a merge callable +can raise the same service/key-named `UnsupportedComposeError` as validation +does. `ulimits`'s bespoke `KeySpec` (`keys.py:191`) sets `merge=_merge_map` explicitly since it bypasses the `_map()` factory. @@ -78,7 +81,7 @@ def _merge(base: dict[str, Any], local: dict[str, Any], name: str) -> dict[str, for key, local_val in local.items(): spec = SERVICE_KEYS.get(key) if key in base and spec is not None and spec.merge is not None: - merged[key] = spec.merge(base[key], local_val) + merged[key] = spec.merge(name, key, base[key], local_val) elif key in base and key in _MERGE_KEYS: # structural keys only ... elif key in base and key in _CONCAT_KEYS: # structural keys only @@ -88,7 +91,7 @@ def _merge(base: dict[str, Any], local: dict[str, Any], name: str) -> dict[str, return merged ``` -`_MERGE_KEYS`/`_CONCAT_KEYS` shrink to the 8 keys with no `KeySpec`: +`_MERGE_KEYS`/`_CONCAT_KEYS` shrink to the 9 keys with no `KeySpec`: `_MERGE_KEYS = {"environment", "extra_hosts", "healthcheck", "depends_on"}`, `_CONCAT_KEYS = {"secrets", "configs", "volumes", "tmpfs", "env_file"}` — their `_as_mapping`/`_as_list` handling is unchanged. A one-line comment marks these @@ -102,7 +105,7 @@ and shape-derived. ## Non-goals -- Unifying the 8 structural keys' merge policy — deferred; would conflate this +- Unifying the 9 structural keys' merge policy — deferred; would conflate this change with candidate #3 of the 2026-07-13 architecture review (owning module per structural key), and `decisions/2026-07-12-reject-structural-key-registry.md` already rejects a uniform structural registry on shape-heterogeneity grounds