Repository navigation
test(gaul): C-43 forward-check as a standing guard + fail-loud lookup build - #141
Merged
Merged
Conversation
… build ADR-011 swapped runtime geopandas mapping for a precomputed lookup, and the plan's required old-vs-new value diff (#20 Stage 2 / #23) was signed off without being produced. The comparator was then deleted in PR #42, so the backward check is unrecoverable — and run-0 shipped 64,742 cells to FAO on 2026-07-27 with no gate anywhere asserting that a *present* GAUL value was the *right* one. The check that remains available runs forwards, against the producer. Run this session, zero mismatches on all four links: gid->lat/lon formula vs PRIO-GRID priogrid_cell.dbf, all 259,200 cells -> max abs error 0.00e+00 lookup values vs the 7 datafactory gaul_admin parquets -> 0 mismatches / 64,742 cells lookup gid set vs land_gaul_pgids.json -> exactly equal, all unique delivered run-0 sidecar bytes vs the lookup -> 0 mismatches, sha256 matches the manifest The leading hypothesis going in — a flipped or off-by-one coordinate formula putting wrong-but-non-null coordinates on every cell, invisible to every gate — was falsified. The formula is exact across the entire global grid. This commit turns that session result into a standing guarantee. 18 tests split by dependency: always-on checks on the committed artifact (key uniqueness, no nulls, no -1 sentinels, region count, coordinate formula against a committed 219-cell PRIO-GRID ground-truth sample) and skipif checks against the views-datafactory sibling (all 7 columns, full 259,200-cell formula sweep). build_gaul_lookup.py: the three bare `assert`s become explicit LookupBuildError raises (python -O silently strips asserts, writing an unvalidated artifact that looks identical), plus a new index-uniqueness guard. Rebuild verified value-identical to the committed artifact. Two entries I registered earlier today are corrected here on mutation evidence rather than left overstated: - C-59 tier 2 -> 3. A duplicate cannot reach the artifact on the production path: Index.intersection de-duplicates first. The guard is reachable only via --region all. Kept anyway — that de-duplication is accidental pandas behaviour, not a declared guard, and silently absorbing upstream duplication hides a producer defect. - C-61 exposure corrected. The primary protection against -1 is the completeness filter, which is plain code and survives -O; the strippable-assert argument never applied to the -1 case. The real defect was stating invariants in a form the interpreter can delete. C-43 scope stated explicitly and deliberately: this proves transcription fidelity, not assignment correctness. If views-datafactory's area-majority join puts a cell in the wrong country, every test here still passes. That is filed as views-datafactory#387 — it had been resolved here on 2026-06-24 as "tracked there" and was never actually opened there, leaving it untracked platform-wide for five weeks. Register: 61 concerns, 24 open. Cluster K added. ruff clean; 296 passed (5 known local pyarrow byte-parity failures, CI authoritative). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Polichinel
added a commit
that referenced
this pull request
Aug 1, 2026
…and make closing conditions machine-checkable (#194) C-43, C-59 and C-61 were filed under Open with their own stated closing conditions already met. C-43's body said it plainly — "C-43 closes when tests/test_gaul_lookup_fidelity.py is committed and green" — and that file was committed in #141 with 18 tests passing. C-59 and C-61 both recorded "Mitigation — landed 2026-07-31" and named four guards that exist and pass. Nothing was outstanding. The entries simply never moved. All three moved to Resolved, each citing the test rather than the session, which is what C-43 instructed. Its transcription-fidelity-vs-assignment-correctness scope split is preserved verbatim: this closes faithful carriage of the producer's answer, NOT whether that answer is right. The latter is views-datafactory#387 and is not this repo's to close. The guard this adds is the point of the story. test_register_integrity.py could already catch a heading that SAYS resolved; it could not catch an entry saying open while the thing it waited for had arrived — which turned out to be this register's actual failure mode, six times in two days. Two new checks: - an Open entry naming a "closes when `tests/…`" artifact that exists - an Open entry recording "Mitigation — landed" without "Partial mitigation" Mutation-tested against the real 2026-08-01 register: both fail, naming 'C-43 -> tests/test_gaul_lookup_fidelity.py' and ['C-59','C-61']. Restored, all 10 pass. Deliberately narrow, and narrowed further in review. _ARTIFACT_ROOTS covers tests/ and scripts/ only. For an executable artifact, existence is close to the condition because the suite then runs it; for docs/ the two come apart entirely — an ADR file has existed since July while the section an entry waits on may never have been written. Including doc paths would fire on correctly-open entries, and the guard would be deleted rather than fixed. That is the failure this check exists to avoid, so it must not commit it. Cluster K carries a closure note: one test file discharged three entries exactly as its fix strategy predicted, and the cost was not the geography bug it was opened for — it was that the engineering took one session and the record took two more days. C-43's own residual said the forward-check needed attaching to something the interpreter runs; the entries then made the identical error one level up. A guarantee needs a check, including the register's own. Header 25 -> 22 open, 49 -> 52 resolved. Cluster M heading corrected to six. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Closes the last correctness question hanging over run-0.
The problem
ADR-011 replaced runtime geopandas mapping with a precomputed lookup. The plan's required old-vs-new value diff (#20 Stage 2 / #23) was signed off without being produced, and the comparator was deleted in PR #42 — so the backward check is unrecoverable.
Meanwhile every gate in the delivery chain checks presence, never value:
_validateandassert_metadata_completecount nulls,coverage.pycounts cells,parity.pycompares gid sets. A wrong-but-non-null GAUL label passes all of them. Run-0 shipped 64,742 cells to FAO on 2026-07-27 on that basis.The check that is available
Not backwards against the deleted mapper — forwards, against the producer. Run this session:
priogrid_cell.dbf, all 259,200 cellsgaul_adminparquetsland_gaul_pgids.jsonMy leading hypothesis going in — that the coordinate formula might be transposed or off-by-one, putting plausible wrong coordinates on every cell, invisible to every gate — was falsified. It is exact across the whole global grid.
What this PR adds
tests/test_gaul_lookup_fidelity.py— 18 tests turning that one-off into a standing guarantee, split by dependency:-1sentinels, region count vs thecoverage.pypin, excluded-cell absence, schema contract, provenance stamp resolves, and the coordinate formula against a committed 219-cell PRIO-GRID ground-truth fixture (corners, row boundaries, equator crossing, even spread — so a transposition or off-by-one can't pass by coincidence)scripts/build_gaul_lookup.py— three bareasserts → explicitLookupBuildErrorraises (python -Ostrips asserts, writing an unvalidated artifact that looks identical on disk), plus a new index-uniqueness guard. Rebuild verified value-identical to the committed artifact.Two corrections to entries I registered earlier today
Mutation-testing the builder proved me wrong twice, and the register now says so rather than carrying overstated entries:
Index.intersectionde-duplicates first. The guard is reachable only via--region all. Kept anyway: that de-duplication is accidental pandas behaviour, not a declared guard, and silently absorbing upstream duplication hides a producer defect.-1is the completeness filter — plain code, survives-O. The strippable-assert argument never applied to-1. The real defect was stating invariants in a form the interpreter can delete.I also dropped a first attempt at a
-1guard test that monkeypatched pandas'__ne__; a test that fragile is worse than the invariant it guards. What's pinned instead is the behaviour that actually protects FAO — the sentinel cell is excluded, so it fails loud as absent rather than shipping as wrong-but-non-null.Scope, stated deliberately
This proves transcription fidelity, not assignment correctness. If views-datafactory's area-majority join puts a cell in the wrong country, every test here still passes.
That is views-datafactory#387, filed today. It had been resolved here on 2026-06-24 as "tracked there, not here" — and was never opened there. Untracked platform-wide for five weeks, while run-0 shipped the affected high-latitude cells. The register convention is amended: relocation isn't complete until the destination issue exists and is cited by number.
Verification
ruffclean. 296 passed (up from 278), 41 xfailed. The 5 failures are the known local pyarrow byte-parity tests (local 23.0.1 vs pinned 16.1.0) — CI authoritative.Register: 61 concerns, 24 open, Cluster K added, integrity guard green.
🤖 Generated with Claude Code