S1 — Amend ADR-002: io/ is a codec the frames call, not a layer above them (closes C-82) - #252
Merged
Merged
Conversation
…r above them (closes C-82) S1 of epic #240, closing register C-82. ADR-002 said `io/` "sits at the top, imports the frames to serialize them", that "nothing lower may import io/", and listed "a frame importing io/" under Forbidden Patterns. The code runs the other way: `io/npz` and `io/arrow` import only `_typing`, and all three frames import `io` and call `npz.save`/`npz.load`. The decision was never wrong — direction is one-way and acyclic, verified with grimp across 36 modules and 89 dependencies, zero cycles. What was wrong was the ADR's factual claim about which way `io/` runs. Three things settle it: - `Persistable` puts `save`/`load` on the frame, so the frame is what reaches the serializer. Those methods are frozen v1 surface under ADR-018. - C-09 is the origin. Resolving it in v0.1.0 moved `io/npz` onto a generic frame-state contract so the I/O layer would carry no per-frame schema. That inverted the dependency; the ADR was never amended to match. - The result serves the ADR's own stated goal better than its prescription did. It wanted `io/` to change for its own reasons, not when a frame's schema changes. `io/` never importing a frame guarantees that more strongly than `io/` importing three of them. Correcting the code was not available: removing `save`/`load` from the frames is a MAJOR bump with a cross-repo merge train, to fix a documentation error. Changed: - ADR-002 — an Amendment blockquote recording all of the above; the intra-package section rewritten to name all eleven leaf modules (it named four, omitting `_typing`, which has the highest fan-in in the leaf, plus `metadata`, `io` and `conformance`); the Layering Principle example reversed; the Forbidden Patterns clause corrected; the Context passage that named the wrong failure mode fixed. Status is now "Accepted (amended 2026-08-17)", following ADR-019's precedent. - `docs/standards/physical_architecture_standard.md` — the layering paragraph and the Circular Dependency Guard, which restated the same claim. The directory tree in that file is stale in other ways; that is C-84 and S2. - `docs/CICs/Protocols.md` — records that `Persistable` is *why* the frames depend on `io/`. Last reviewed moved from 2026-06-24, the oldest CIC, and the protocol at the centre of this. - `pyproject.toml` — the `[tool.importlinter]` comment no longer says the ADR is out of date, now that it isn't. Verified: the only remaining occurrences of the old claim anywhere under `docs/` are inside the amendment text explaining the correction; all eleven modules checked against `find src/views_frames -name '*.py'`; lint-imports 2 kept 0 broken; validate_docs.sh passed; suite green at 100%. Closes #241. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…own list Four findings from /review-diff on this branch. The intra-package section opened with "Ten modules, lowest layer first" and then listed eleven. The commit message, PR body and register resolution all said eleven, verified against `find src/views_frames -name '*.py'`; only the ADR said ten. In a story whose whole purpose is making a document state true things, that is the wrong defect to ship — and it is C-85's failure mode in miniature, a hard-coded enumeration count that nothing checks. Dropped the number rather than correcting it: the list is the enumeration, and a count beside it can only drift. The line now says so explicitly, so the next editor does not re-add one. Also: - Full dates everywhere. Two places said "2026-08" where the Amendment and the standard said "2026-08-17"; month precision invites the question of whether they refer to the same event. - Trimmed the inline "why the frames call io/" note. It restated the Amendment's first reason in similar words forty lines later. The note now carries the standing rule and points at the Amendment for the history. The fourth finding needed no code change: `physical_architecture_standard.md` is more internally inconsistent after S1 than before it, because the corrected layering paragraph names `_typing` and `metadata` while the directory tree ten lines above still omits them. That tree is C-84 / #242, blocked on this PR because both stories edit this file. Fixing it here would reopen the conflict the split exists to prevent, so it is declared in the PR body instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…st resolution /code-review found the correction incomplete and, more usefully, found that this entry's own verification did not support the claim it made. Two files still asserted the inverted direction, both inside the search path the C-82 resolution grepped: - `README.md` §layout rules: "I/O adapters live under `io/`, import the frame". Different wording from the four literal phrases the check searched for, and wrapped across two lines. README is the design bible — CLAUDE.md says if code and README disagree, reconcile before merging — so leaving it meant the next reader of the layout rules would conclude the frames' `npz` imports are the violation. Rewritten to say what is actually true: the codecs take raw arrays, never import a frame, and a frame's `save`/`load` delegate down into them. - `docs/ADRs/README.md:23` summarised ADR-002 as "(index/spatial_level/protocols/_validation → frames → io)" — the direction this epic amends away, and the pre-amendment four-module lowest layer. The index is what an agent or contributor reads before opening the ADR, so the corrected ADR was fronted by an uncorrected summary of itself. The CHANGELOG needed the same treatment: its `[Unreleased]` section still said "Correcting the ADR is left to a separate change" while this *is* that change, in the same unreleased section. Added a `Changed — architecture record` subsection and made the earlier paragraph past-tense. The C-82 resolution now records why its first verification passed while two occurrences survived, and replaces the four fixed phrases with a check that matches any line pairing `io` with a frame in an import or arrow relation. The lesson is worth keeping: C-77's discipline is to paste evidence rather than describe it, and this entry did paste real output from a real command — the command was the weak part. Pasting evidence is necessary, not sufficient; the check has to be capable of failing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The C-82 resolution followed C-77 exactly — ran a check, pasted the real output of a real command rather than describing what was done — and still asserted something false. The check was a grep for four fixed literal phrases, and two occurrences of the inverted claim survived inside its own search path: README said "I/O adapters live under `io/`, import the frame" (different wording, wrapped across two lines), and the ADR index carried a pre-amendment one-line summary of the very ADR being corrected. So the rule needs a second half. Pasting output proves a command ran; it does not prove the command could have failed on the thing being claimed. A check built from fixed phrases only finds the phrasings its author already thought of — which are, by construction, the ones they just finished editing. Where a resolution claims absence, prefer a check that matches the shape of the thing rather than its wording, and mutate something to confirm the check goes red before trusting it green. Cross-referenced to C-67, which is the same shape in code: the published conformance suites once reported green under `python -O` while checking nothing. The import contracts in #239 were mutation-tested for exactly this reason; the documentation check was not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
S1 of epic #240. Closes #241 and register C-82. Documentation only —
src/is untouched, no public API changes,CONFORMANCE_FLOORstays1.0.0.The problem
ADR-002 §Decision said
io/"sits at the top, imports the frames to serialize them" and "Nothing lower may importio/", and §Forbidden Patterns listed "a frame importingio/" as an architectural violation.docs/standards/physical_architecture_standard.md:48-49restated it;:67made it an operational Circular Dependency Guard.The code is the inverse.
io/npz.pyandio/arrow.pyimport onlyjson,pathlib,typing,numpy,pyarrowandviews_frames._typing— neither imports a frame.feature_frame.py:21,prediction_frame.py:20andtarget_frame.py:18each dofrom views_frames.io import npzand callnpz.save(...)/npz.load(...). The pattern the ADR forbade was used in three of the leaf's core files.Why the documents move and not the code
Persistableputs persistence on the frame.protocols.py:62declaressave(directory)/load(directory, mmap)as frame methods. Once persistence is a frame method, the frame must reach the serializer.io/npzoperates on a generic frame state dict; the I/O layer carries no per-frame schema." That resolution inverted the dependency. Neither topology document was amended — fourteen months, until Run the import contracts as a gate, and register what writing them turned up (#238) #239's import contracts forced the question.io/change "for its own reasons, not when a frame's schema changes."io/never importing a frame guarantees that more strongly thanio/importing three of them.save/loadare frozen v1 surface (ADR-018), so correcting the code would be a MAJOR bump with a cross-repo merge train — to fix a documentation error.Amended, not superseded
ADR-000 says a changed decision is superseded, not erased — but the decision did not change. Direction is still one-way and acyclic (
grimp: 36 modules, 89 dependencies, zero cycles). Only the factual claim about which wayio/runs was wrong. That is an amendment, following ADR-019's precedent (**Status:** Accepted (amended … — see *Amendments* below)).What changed
docs/ADRs/002_topology_and_dependency_rules.md— an Amendment blockquote recording the above; the intra-package section rewritten to name all eleven leaf modules (it named four, omitting_typing— the highest fan-in in the leaf at 8 — plusmetadata,ioandconformance); the §Layering Principle worked example reversed; the §Forbidden Patterns clause corrected; the §Context passage that named the wrong failure mode fixed.docs/standards/physical_architecture_standard.md— the layering paragraph and Circular Dependency Guard. The directory tree in that file is stale in other ways and is deliberately untouched here — that is C-84 / S2 (S2 — Rewrite the physical-architecture standard's directory tree (closes C-84) (Epic #240) #242), which is blocked on this PR precisely because both edit this file.docs/CICs/Protocols.md— records thatPersistableis why the frames depend onio/.Last reviewedmoved from 2026-06-24 (the oldest CIC, and the protocol at the centre of this).pyproject.toml— the[tool.importlinter]comment no longer says the ADR is out of date, now that it isn't.reports/technical_risk_register.md— C-82 closed and moved to Resolved; open 18 → 17, resolved 64 → 65.Verification
Per register C-77's discipline, the resolution pastes evidence rather than describing it.
Every stale claim is gone — the only remaining occurrences anywhere under
docs/are inside the amendment text explaining the correction:All eleven modules checked against
find src/views_frames -name '*.py'—_typing,metadata,spatial_level,_validation,io,index,protocols,feature_frame,prediction_frame,target_frame,conformance: all present, none missing.⚠ One deliberate intermediate state
physical_architecture_standard.mdis more internally inconsistent after this PR than before it, and that is intentional. The corrected layering paragraph now names_typingandmetadataas the lowest layer — neither appears in the directory tree ten lines above, because that tree still omits them.The tree is C-84 / #242, the next story, and it is blocked on this PR precisely because both edit this file. Fixing the tree here would reopen the conflict the split exists to prevent. Flagging it so a reviewer reads the mismatch as scheduled rather than missed.
Not in this PR
The rest of the physical-architecture standard's staleness — a tree showing one of three packages, omitting
metadata.py/_typing.py/conformance/, and listing two files that never shipped — is C-84 / #242, the next story.🤖 Generated with Claude Code