Skip to content

A refresh replaced both artifacts wholesale and nothing said what it moved - #71

Merged
ChelseaKR merged 1 commit into
mainfrom
feat/artifact-diff
Sep 7, 2026
Merged

ChelseaKR merged 1 commit into
mainfrom
feat/artifact-diff

Conversation

@ChelseaKR

Copy link
Copy Markdown
Owner

Closes #58.

What was wrong

A refresh of the pinned retrievals replaces site/data/perimeters-coverage.json and
site/data/dins-coverage.json wholesale, and the only account of what changed was
git diff over a large JSON document. That answers a different question: it reports
lines, so a reordered list reads as hundreds of changes and a single count that moved
reads as two. The README promises the figures "move only when those retrievals are
deliberately refreshed", and a deliberate refresh should be able to say what it moved.

What changed

python -m perimeter.diff OLD NEW walks every leaf of both documents and reports the
value at its path (/fields[3]/present) with both sides. Exit 0 no change, 1 changes
reported, 2 a refused removal or an unreadable input. --json writes the rows sorted by
path, byte-identical on repeat, so a refresh can cite a comparison in PROVENANCE.md.
make diff OLD=… NEW=… wraps it, and the Makefile records that make collapses every
recipe failure to its own exit 2
, so a script needing the three codes must call the
module directly.

Three distinctions decide the design:

  • A key that stops being published is not a value moving. It is refused outright
    unless ALLOW_REMOVALS=1 names it deliberate.
  • A number becoming null is a change to absence, never a removal. ADR-0010 writes a
    domain the layer stopped publishing as null rather than dropping it, so collapsing the
    two would lose the distinction the artifact exists to carry.
  • 1000 and 1000.0 are a type change. Every percentage here is a *_tenths_pct
    integer precisely so that no float decides an equality; nothing converts, rounds or
    tolerances a value.

make site-check now prints the leaf comparison before its byte-for-byte check decides.
The comparison is the report; diff -r is still the gate. Its exit status is discarded
deliberately and that is commented in the recipe, so the reporting step can add detail and
can never turn a red target green.

Refusals, per ADR-0004: a missing file, an empty file, an unparseable one, and a JSON
document whose top level is not an object are each refused rather than parsed into an
empty document. Two empty documents compare equal, and "no change" about two artifacts
that were never read is exactly the failure this repository is organised against.

Done-when, checked

From the issue Result
Diffing an artifact against itself exits 0 and prints "no change" ✅ and it states the population: no change: 56241 leaves compared, none differ, none added, none removed
Removing duplicate_signals exits 2 without --allow-removals, 1 with it, naming the key ✅ both, key named in the output
A records count change reported with both values; JSON output byte-identical on repeat /records: 132522 -> 999; two runs compared equal in-test
make site-check prints the leaf-level diff ✅ wired ahead of diff -r (that target needs data/raw/, so it is not exercisable in CI)

How it was verified

822 passed (full suite less the two Node/browser a11y gates); coverage 99.27% total,
95% on the new module, floor 90; mypy --strict clean over 13 source files;
ruff check / ruff format --check clean.

Negative controls, each with the sabotage asserted present in the file before the
result was read:

Sabotage in diff.py Result
removals always empty 3 tests FAILED (…is_a_removal_not_a_change, both exit-code tests)
emptiness check deleted 1 test FAILED

The second control initially passed, and that was a real finding in this PR's own test.
With the emptiness check gone, the empty file still failed as unparseable JSON, and
pytest.raises(match="empty") was satisfied by the fixture's filename, empty.json. The
assertion was passing for the wrong reason. The fixture is renamed nothing-here.json and
the pattern now matches the reason; the control then failed as it should. A control that
passes for the wrong reason is a control that is not there.

Out of scope, as the issue says

Diffing the pages (they are rendered from the artifacts) and automatic refreshes.

Prepared with AI assistance; reviewed before submission.

@ChelseaKR

Copy link
Copy Markdown
Owner Author

verify is red here, and not because of this branch. Every other check passes
(secret-scan, sast, zizmor, codeql). The five failures are all in
tests/test_release_claims.py, which this branch does not touch, and they all say the
same thing:

AssertionError: the tag list here cannot be trusted: this is a shallow clone.
actions/checkout fetches no tags at the default depth ... Check out with fetch-depth: 0

ci.yml does set fetch-depth: 0 on the verify job, and this branch does not modify
ci.yml (git diff origin/main origin/<head> -- .github/workflows/ci.yml is empty).

What was ruled out

Control Result
main's own CI, re-run just now (push event) passes
power-content-check PR #54, whose test_release_claims.py uses byte-identical shallow detection passes on a pull_request event, and has since merged
release/version-tag-honesty (#68's branch, whose tree contains this same test), re-run just now passes on a pull_request event
this run, re-run twice fails both times, so it is not transient

The only tree difference between #68's passing branch and main is the concurrency block
from #69. tests/test_release_claims.py is byte-identical between them.

What the failure actually is

The checkout is not truncated. In the same job, on the same checkout, the new
tests/test_ruleset_evidence.py runs git cat-file -t d4f533f and
git rev-list --count d4f533f and passes, reading a 66-commit ancestry. All five failures
are in test_release_claims.py; none is in the module that walks history.

So the checkout has full history and git rev-parse --is-shallow-repository returns
true at the same time. That predicate reports whether a .git/shallow marker exists, not
whether anything is actually truncated, and in this repository's pull_request-event
checkouts today those two have come apart.

That is this portfolio's dominant defect class pointed the other way: a state marker read
as a measurement
. The gate is failing closed, which is the correct direction and
exactly what it was built to do, but it is refusing to answer a question it can in fact
answer here.

What I did not do

I did not touch that gate. Weakening or re-baselining a version-honesty check that landed
an hour ago is not a call to make from inside an unrelated pull request, and "make it
green" is the wrong instinct for a check that is being conservative. Recording the
diagnosis instead, with the controls that produced it, so whoever fixes it starts from
evidence rather than from a red tick.

The fix is not a looser assertion: it is to make the predicate ask what the message
claims it asks, or to stop the PR-event checkout leaving a shallow marker behind. Both are
changes to test_release_claims.py or ci.yml, and both belong in their own pull request
where the negative control can be run against a genuinely truncated clone.

Prepared with AI assistance; reviewed before submission.

@ChelseaKR

Copy link
Copy Markdown
Owner Author

This PR's red verify job is not about this diff.

tests/test_release_claims.py refuses to read the tag list because git rev-parse --is-shallow-repository reports true. The message blames actions/checkout, but fetch-depth: 0 is set on the verify job and the logged fetch carries no --depth — the same refspec reproduced locally on git 2.55.0 produces a clone that is not shallow.

Measured on a runner via a throwaway diagnostic branch (now deleted): .git/shallow is absent after checkout, absent after make lock-check sync node-sync, absent after make browser-sync, and appears the moment tests/test_a11y_browser_gate.py runs — holding d4f533f, the pull request's base SHA. The cause is Playwright's git-info plugin: gitDiff opens with git fetch origin <pr base sha> --depth=1 ... (node_modules/playwright/lib/runner/index.js), and the harness's working directory is inside this work tree, so the fetch shallows the repository.

It presented as flake because it is a race: whether it fired depended on whether pytest -n auto scheduled the browser gate before the release-claims tests. Pushes to main were unaffected — with no PR base SHA there is nothing to fetch — so main stayed green while pull requests failed at random.

PR #74 turns the capture off (captureGitInfo: { commit: false, diff: false }) and strips the CI-identifying environment names the plugin reads, with negative controls for both halves. It is green.

Once #74 is on main, this PR needs no rebase: its merge ref is recomputed against the new base, so re-running the ci workflow here should be enough to get a real verdict.

Prepared with AI assistance; reviewed before submission.

@ChelseaKR ChelseaKR closed this Sep 7, 2026
@ChelseaKR ChelseaKR reopened this Sep 7, 2026
@ChelseaKR ChelseaKR closed this Sep 7, 2026
@ChelseaKR ChelseaKR reopened this Sep 7, 2026
…moved

The figures on these pages move only when the pinned retrievals are deliberately
refreshed, and a refresh rewrites site/data/perimeters-coverage.json and
dins-coverage.json in full. The only account of what changed was git diff over a large
JSON document, which answers a different question: it reports lines, so a reordered list
reads as hundreds of changes and one count that moved reads as two.

python -m perimeter.diff OLD NEW walks every leaf of both documents and reports the value
at its path with both sides. Exit 0 no change, 1 changes reported, 2 a refused removal or
an unreadable input. --json writes the rows sorted by path, byte-identical on repeat, so
a refresh can cite a comparison in PROVENANCE.md. make diff wraps it for a terminal, and
records in the Makefile that make collapses every recipe failure to its own exit 2, so a
script that needs the three codes calls the module.

Three distinctions decide the design. A key the later artifact stops publishing is not a
value moving: it is refused unless ALLOW_REMOVALS names it deliberate. A number becoming
null is a change to absence and never a removal, because ADR-0010 writes a domain the
layer stopped publishing as null rather than dropping it. And 1000 and 1000.0 are a type
change, because every percentage here is a *_tenths_pct integer precisely so that no
float decides an equality.

make site-check now prints the leaf comparison before its byte-for-byte check decides.
The comparison is the report and diff -r is still the gate: the report's exit status is
discarded deliberately, so it can add detail and can never turn a red target green.

Refusals per ADR-0004: a missing file, an empty file, an unparseable one and a JSON
document whose top level is not an object are each refused rather than parsed into an
empty document, because two empty documents compare equal and would report no change
about two artifacts that were never read.

Running the tests against the faults they exist to catch found a weak assertion in this
change's own test file. With the emptiness check deleted, an empty file still failed as
unparseable JSON and match='empty' was satisfied by the fixture filename empty.json. The
fixture is renamed and the pattern now matches the reason.

Closes #58
@ChelseaKR
ChelseaKR merged commit dbef2c2 into main Sep 7, 2026
6 checks passed
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.

Add an artifact diff command so a deliberate refresh is compared value by value

1 participant