Skip to content

S1 — Amend ADR-002's io/ layering claim and the physical-architecture standard (closes C-82) (Epic #240) #241

Description

@Polichinel

Part of epic #240. Nothing blocks this. Must be done before S2 — both stories edit
docs/standards/physical_architecture_standard.md, and S2's rewrite sits under the lines this story
corrects.

The problem

ADR-002 §Decision, intra-package bullet 3, says:

io/ (io/npz, io/arrow) sits at the top, imports the frames to serialize them, and changes for
its own reasons (a new disk format), not when a frame's schema changes. Nothing lower may import
io/. A frame must not know how it is serialized.

Its §Layering Principle repeats it with a worked example (io/arrow → prediction_frame → index), and
its §Forbidden Patterns lists:

index.py or _validation.py importing a frame, or a frame importing io/

docs/standards/physical_architecture_standard.md:48-49 restates the claim verbatim, and :67 turns
it into a Circular Dependency Guard: "no core module may import io/."

The code is the inverse. Verified with grimp over all three packages (36 modules, 89
dependencies, zero cycles):

  • src/views_frames/io/npz.py and io/arrow.py import only json, pathlib, typing, numpy,
    pyarrow and views_frames._typing. Neither imports a frame.
  • src/views_frames/feature_frame.py:21, prediction_frame.py:20 and target_frame.py:18 each do
    from views_frames.io import npz, and call npz.save(...) / npz.load(...)
    (prediction_frame.py:156,168).

So the forbidden pattern is used in three of the leaf's core files, and the layer assignment is
backwards.

Why the code is right and the documents are wrong

Three reasons, and the story should say all three in the amendment text:

  1. Persistable puts persistence on the frame. src/views_frames/protocols.py:62 declares
    save(directory) and load(directory, mmap) as frame methods. Once persistence is a method the
    frame owns, the frame must reach the serializer. The arrow only points one way from there.
  2. C-09 is the origin. Resolved 2026-06-21 (v0.1.0): "io/npz operates on a generic frame state
    dict (values + identifiers + a JSON header); the I/O layer carries no per-frame schema."
    That
    resolution is what inverted the dependency. Neither topology document was amended.
  3. The code satisfies ADR-002's stated goal better than its own prescription. The goal is that
    io/ "changes for its own reasons, not when a frame's schema changes." io/ never importing a
    frame is stronger decoupling than io/ importing three of them. npz.save takes primitives
    (values, time, unit, level, metadata) and never sees a frame type.

Correcting the code is not available. save/load are frozen v1 surface (ADR-018 §In scope), so
removing them is a MAJOR bump with a cross-repo merge train — to fix a documentation error.

Amend, do not supersede

ADR-000 §Status says a changed decision is superseded, not erased — but the decision here is not
changing. Acyclic, one-way dependency direction still holds; only the ADR's factual claim about which
way io/ runs is wrong. That is an amendment.

Follow the established pattern: ADR-019 carries **Status:** Accepted (amended 2026-06-24 — see *Amendments* below) with numbered > **Amendment N (date, register C-xx).** blockquotes, and
ADR-013 has an > **As-built amendment (2026-07-02, ...).** note. Use that form.

Work

  • docs/ADRs/002_topology_and_dependency_rules.md:
    • Rewrite the intra-package bullet: io/ is a low-level array codec the frames call, not a
      top layer that imports them. Name all ten leaf modules, not four — the current lowest-layer
      list omits _typing (fan-in 8, the most-depended-on module in the leaf), metadata, io and
      conformance.
    • Fix the §Layering Principle worked example — io/arrow → prediction_frame → index runs the
      wrong way.
    • Delete the "or a frame importing io/" clause from §Forbidden Patterns. Keep the
      index.py/_validation.py clause; that half is true.
    • Add an **Amendment (2026-08, register C-82).** blockquote near the top recording what
      changed, why (reasons 1–3 above), and that C-09 caused it. Update **Status:** to
      Accepted (amended 2026-08-xx — see *Amendment* below).
  • docs/standards/physical_architecture_standard.md: correct the layering paragraph at :48-49
    and the Circular Dependency Guard at :67. Leave the directory tree alone — that is S2.
  • docs/CICs/Protocols.md: add one line under Persistable stating that placing save/load on
    the frame is why the frames depend on io/ and not the reverse. Bump **Last reviewed:**
    (currently 2026-06-24, the oldest CIC — and this protocol is the causal reason for the layering).
  • Update C-82 in reports/technical_risk_register.md to RESOLVED, with a resolution that
    states what was verified, not what was intended (register C-77).

How we know it is done

  • grep -rn "sits at the top\|imports the frames to serialize\|a frame importing" docs/ returns
    nothing except inside the new amendment text explaining the correction.
  • ADR-002's module list names all ten: index, spatial_level, protocols, _validation,
    _typing, metadata, io, conformance, the three frames. Cross-check against
    find src/views_frames -name '*.py'.
  • The amendment says why — a reader who only reads the amendment understands that Persistable
    forces the direction and that C-09 caused the drift.
  • bash docs/validate_docs.sh passes.
  • The pyproject.toml [tool.importlinter] comment that currently reads "ADR-002 is out of date, not
    the code; see the PR that added this file's contracts"
    is updated to point at the amended ADR
    instead. Do not leave it saying the ADR is out of date after this story fixes it.

Checks to run before committing

bash docs/validate_docs.sh
uv run lint-imports          # contract 2 encodes the direction this story documents
uv run pytest -q --cov --cov-fail-under=100

Relevant files

docs/ADRs/002_topology_and_dependency_rules.md · docs/standards/physical_architecture_standard.md
docs/CICs/Protocols.md · pyproject.toml ([tool.importlinter] comment) ·
reports/technical_risk_register.md (C-82) · src/views_frames/protocols.py:62 (evidence) ·
src/views_frames/io/ (evidence)

Metadata

Metadata

Assignees

No one assigned

    Labels

    adrArchitecture Decision RecorddocumentationImprovements or additions to documentationriskTechnical risk registerstoryA single story within an epic

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions