Skip to content

The provenance tests do not cover build_subsets.py or build_data.py output #126

Description

@realmarcin

Found in the adversarial review of #120.

RefreshProvenanceTests does not cover build_subsets/build_data output, which is the partial rerun the skill says it catches

Where: tests/test_fleet_page.py:403

Only prefix_census.json, mech_stats.json and site_audit.json carry revisions. build_subsets.py writes assets/fleet/{edges,cells}/*.json and subsets_summary.json with no revision, and no test checks that fleet_data.json matches its two inputs. So a partial rerun of the overlap/record-list stage, or a skipped build_data.py, passes every gate. The skill says 'A partial rerun is the failure this exists to catch' (SKILL.md:37). It also names overlaps as the reason 'a single re-pin is a partial rerun' (SKILL.md:217). The test docstring (test_fleet_page.py:410) makes the same claim.

Failure scenario. CellStructureMech drifts, as it did at this merge (542 pinned, 545 live). Someone re-pins that one Mech, reruns prefix_census.py and mech_stats.py and updates its site_audit sha, but does not rerun build_subsets.py (4 min over 429k files). The census, stats and audit all agree, so all 33 tests, assemble_page --check and CI pass. Meanwhile the overlap chords, edge weights and clickable record lists on /mechs/ still come from the old revision. The same holds if build_subsets.py alone is rerun against another checkout, or if the census is rerun without build_data.py (stale heatmap).

Evidence. Isolated scratch copy /private/tmp/claude-501/review-pr120/tg-f66c2125/repo, harness mutate.py, full suite (python3 -m unittest discover -s tests) after each mutation:
M11 subsets_summary cells +11 and edge n +3, fleet_data not rebuilt -> OK
M12 build_subsets+build_data+assemble rerun 'at another revision', census/stats/audit untouched -> OK
M13 census prefixes changed (CultureMech CHEBI +500, TraitMech METPO=1), build_data not rerun -> OK
M14 census+stats+audit re-pinned for CommunityMech (sha and files 431), build_subsets not rerun -> OK
For contrast, M1-M8 (census-only rerun, stats-only rerun, stale audit sha, dirty census, all-dirty, Mech dropped from audit, missing source_revision, census without _revisions) each fail the intended test. The committed fleet_data.json is currently byte-equal to build_data.build(subsets_summary, census), so a guard for it would pass today.

Verifier (partly, low). The coverage gap is real. build_subsets.py writes subsets_summary.json and assets/fleet/{edges,cells}/*.json without recording any revision, and no test or gate checks that fleet_data.json equals build_data.build(subsets_summary, prefix_census). So all 33 tests, assemble_page --check and CI stay green in each of these cases:

  • the overlap/record-list stage is rerun against a different checkout;
  • build_subsets.py is skipped after one Mech is re-pinned in census, stats and audit;
  • build_data.py is skipped after a census rerun that changes prefix counts only.

In each case the overlap chords, edge weights, record lists or heatmap on /mechs/ can come from a different revision than the census and stats, and nothing notices. That is the class of inconsistency #85 describes, and this PR closes #85.

The finding overstates what the docs claim, though:

  • The PR body scopes the test accurately to census, stats and site_audit.
  • SKILL.md:35-37 and the test docstring name those three files in the sentence before the "partial rerun" line.
  • SKILL.md:217 gives the overlaps as the reason not to re-pin one Mech. It does not say the test would catch such a re-pin.

What actually misleads is narrower: the unqualified "A partial rerun is the failure this exists to catch" (SKILL.md:37) and "a partial rerun shows up here" (docstring), which a reader could take as covering the whole derived layer. The committed data is consistent today: fleet_data.json matches its inputs.

Suggested fix. Have build_subsets.py write _revisions = {m: roots.revision(m) for m in ORDER} into subsets_summary.json, and assert it equals census['_revisions'] in RefreshProvenanceTests. Add a test that json.loads(fleet_data.json) == build_data.build(subsets_summary, prefix_census). Or narrow the 'partial rerun' wording in SKILL.md:37 and the test docstring to the three files actually covered.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions