Skip to content

refactor(unfao): S3 — one representation seam; store metadata gets its own home - #161

Merged
Polichinel merged 1 commit into
developmentfrom
feat/s3-collapse-extraction-seams
Jul 31, 2026
Merged

refactor(unfao): S3 — one representation seam; store metadata gets its own home#161
Polichinel merged 1 commit into
developmentfrom
feat/s3-collapse-extraction-seams

Conversation

@Polichinel

Copy link
Copy Markdown
Collaborator

Closes #151 · Epic #148 · Register C-65 closed

What extraction.py actually was

Two unrelated things in one module:

fate
cells_of, months_of, drop_months_above, unmapped_cell_count deleted — unreachable since #149, already implemented frame-natively in frame_extraction.py
file_metadata moved — it unpacks a store document and touches no representation

They shared a module only because both "unpack something", which is not a reason to change together (CCP). ADR-012 called extraction.py "the single pandas-aware module" — the representation change had already landed beside it.

file_metadata moves to unfao/store_metadata.py, sibling of source_metadata.py, so the pair reads producer facts / store facts and each module carries one concept.

Tests followed the code — none were dropped

  • test_extraction.pytest_store_metadata.py, keeping the file_metadata cases.
  • test_frame_extraction.py was a parity suite proving both seams returned identical primitives — the right test while both existed. With one seam gone parity has nothing to compare against, but the absolute expectations were already written into the assertions ({1,2,3}, [100,101]), so they are now asserted directly. Values unchanged; they simply no longer route through a deleted module to be checked.
  • test_input_integrity_e2e.py now feeds the invariants primitives, which is how the delivery actually calls them. It had been building synthetic pandas frames and pushing them through the pandas seam to reach representation-free rules — testing a chain the delivery no longer has, and holding the file's last pandas import hostage to it. Its docstring now names where each representation-specific half is covered (test_frame_extraction, test_historical_builder, test_store_metadata).
  • test_input_integrity_design_contract.py ① named extraction.py as "the seam"; now names frame_extraction.py and asserts both halves — the seam exists and its retired pandas sibling has not come back, because two seams for one concept is the CRP violation D-11 predicted.

One note worth keeping

That design-contract file recorded ② "no premature Extractor Protocol (YAGNI/ISP) — a migration, not a coexistence." It is now marked vindicated: the coexistence ended, the second implementation was deleted rather than abstracted over, and a Protocol introduced at the time would have outlived the thing it existed to unify. That is WET-before-DRY paying out, and it is worth having in writing for the next seam.

Verification

ruff clean · 289 passed, 41 xfailed · the 5 failures are the known local pyarrow byte-parity tests; CI authoritative · no file under wire/ or fixtures/ touched.

Register: 71 concerns, 31 open, 40 resolved. Integrity guard green.

🤖 Generated with Claude Code

…s own home

Closes #151; closes register C-65. Epic #148.

extraction.py held two unrelated things. Its four frame readers (cells_of,
months_of, drop_months_above, unmapped_cell_count) became unreachable when #149
retired the pandas delivery, and frame_extraction.py already implements all four
frame-natively — the WET siblings D-11 sanctioned for the migration, now one
implementation too many. Deleted.

Its one survivor, file_metadata, was never extraction at all: it unpacks a STORE
DOCUMENT and touches no representation. It shared a module with the frame readers
only because both "unpack something", which is not a reason to change together.
It moves to unfao/store_metadata.py — sibling of source_metadata.py, so the pair
reads producer facts / store facts and each module carries one concept.

Tests followed the code rather than being deleted:

- test_extraction.py -> test_store_metadata.py, keeping the file_metadata cases.
- test_frame_extraction.py was a PARITY suite proving both seams returned
  identical primitives. With one seam gone parity has nothing to compare against,
  but the absolute expectations were already written into the assertions, so they
  are asserted directly. Values unchanged; they just no longer route through a
  deleted module to be checked.
- test_input_integrity_e2e.py now feeds the invariants PRIMITIVES, which is how
  the delivery actually calls them. It had been building synthetic pandas frames
  and pushing them through the pandas seam to reach representation-free rules —
  testing a chain the delivery no longer has, and holding the file's last pandas
  import hostage to it. Its docstring names where each representation-specific
  half is covered instead.
- test_input_integrity_design_contract.py ① named extraction.py as "the seam";
  updated to frame_extraction.py and now asserting BOTH halves — the seam exists
  and its retired pandas sibling has not come back. Its ② note ("no premature
  Extractor Protocol — a migration, not a coexistence") is marked vindicated: the
  coexistence ended, and a Protocol would have outlived the second implementation.

ruff clean; 289 passed, 41 xfailed, 5 known local pyarrow byte-parity failures
(CI authoritative). No file under wire/ or fixtures/ touched.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Polichinel
Polichinel merged commit cdf4a42 into development Jul 31, 2026
4 checks passed
@Polichinel
Polichinel deleted the feat/s3-collapse-extraction-seams branch July 31, 2026 17:31
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