Skip to content

refactor: collapse extends' duplicate list helper onto keys.py#56

Merged
lesnik512 merged 1 commit into
mainfrom
refactor/extends-keys-dedup
Jul 14, 2026
Merged

refactor: collapse extends' duplicate list helper onto keys.py#56
lesnik512 merged 1 commit into
mainfrom
refactor/extends-keys-dedup

Conversation

@lesnik512

Copy link
Copy Markdown
Member

Last of the changes spawned by the audit (finding D4, #52). Design: planning/changes/2026-07-14.03-extends-keys-dedup.md.

The footgun

extends.py carried a private _as_list that was byte-identical to keys.py's _as_list — same body, same error message — with the first two parameters swapped:

# extends.py
def _as_list(key: str, name: str, value: Any) -> list[Any]: ...
# keys.py
def _as_list(name: str, key: str, value: Any) -> list[Any]: ...

Each call site was correct only because it happened to match its own module's local signature. A transposition would have rendered service 'volumes': cannot merge 'app' — names and keys swapped — and nothing would have caught it. That sits in the extends merge path, which runs ahead of the gate (cli.py calls resolve_extends() before validate()) — precisely where this codebase has already been bitten.

The change

keys._as_list/_concat_list are promoted to public as_list/concat_list, joining the module's existing cross-module primitives (key_value_pairs, pairs_to_mapping, validate_map, require_string_keys, extra_host_pairs, is_number) on the same (name, key, value) signature. extends.py drops its copy and its open-coded concat. One definition, one parameter order.

extends._as_mapping deliberately stays. It is not duplication — it is stricter than pairs_to_mapping, accepting list form only for environment and depends_on and refusing it for extra_hosts/healthcheck. Collapsing it would silently start coercing list-form extra_hosts on a merged side: a behavior change wearing a dedup's clothes. Its parameter order is corrected to match, which removes the other half of the footgun.

No structural-key registry — decisions/2026-07-12-reject-structural-key-registry.md stands. This moves two helpers; it does not build a dispatch table.

Proof it's behavior-preserving

The emitted script for a document exercising every merge path (mapping-merge, sequence-concat, registry merge, structural concat, override) is byte-identical to main. Review independently diffed resolve_extends() output across 20 documents covering every merge path and all five error paths — identical on both revisions.

Every existing test passes unedited; a pure refactor that needed its expectations changed would not be pure. Two tests added, pinning the name/key order in the merge error so a future transposition fails loudly instead of producing a garbled message.

551 tests at 100% coverage.

🤖 Generated with Claude Code

extends._as_list was byte-identical to keys._as_list -- same body, same
error message -- with the first two parameters swapped. Each call site was
correct only because it matched its own module's local signature; a
transposition would have produced a garbled message and nothing would have
caught it.

keys.py's _as_list/_concat_list join its public cross-module primitives as
as_list/concat_list, on the same (name, key, value) signature as the rest.
extends drops its copy and its inline concat.

extends._as_mapping stays: it is deliberately stricter than
pairs_to_mapping, accepting list form only for environment and depends_on,
so collapsing it would silently start coercing list-form extra_hosts. Its
parameter order is corrected to match.

Behavior-preserving: the emitted script for a document exercising every
merge path is byte-identical.
@lesnik512 lesnik512 merged commit 274ae59 into main Jul 14, 2026
7 checks passed
@lesnik512 lesnik512 deleted the refactor/extends-keys-dedup branch July 14, 2026 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant