roots.revision() judges a checkout clean from git status, which cannot see ignored, hidden-untracked or sparse-excluded records

[realmarcin]

Found in the adversarial review of #120.

'+dirty' misses records that are counted but not committed (gitignored files, or status.showUntrackedFiles=no)

Where: scripts/fleet/roots.py:107

revision() decides whether a checkout is clean with a bare git status --porcelain. That command never lists ignored files, and it lists no untracked files when the repo or the user's config sets status.showUntrackedFiles=no. record_paths() counts both kinds. So the census and the stats can include files that are not in the named commit while revision() still returns a clean 40-hex SHA. All four RefreshProvenanceTests then pass, because both scripts count the same wrong set at the same 'clean' revision. This is the #88 failure (2,957 vs 2,951 MediaIngredientMech records from backup files), and the new guard would not have caught it.

Failure scenario. MediaIngredientMech's save_yaml writes a backup next to the record it overwrites, at path.parent / "backups" (src/mediaingredientmech/utils/yaml_handler.py:110 at dfce1c9). Its .gitignore:100 ignores every data/ingredients/*/backups/, but roots.EXCLUDE_DIRS only excludes data/ingredients/mapped/backups/. Saving any record under data/ingredients/unmapped/ creates an ignored unmapped/backups/*.yaml. The glob data/ingredients/**/*.yaml counts it and revision() reports clean. The page then overstates the count, and tests and CI stay green.

Evidence. Scratch sparse clone of MediaIngredientMech at the pin, with MECHS_ROOT pointed at it. Before: MediaIngredientMech 2953 dfce1c9342ca7aec41b50d0f6db8adb950bb72fe. After copying one record into data/ingredients/unmapped/backups/: MediaIngredientMech 2954 dfce1c9342ca7aec41b50d0f6db8adb950bb72fe (no +dirty), and git status --porcelain | wc -l printed 0. With git config status.showUntrackedFiles no and a new untracked kb/communities/zz_new_record.yaml in CommunityMech: CommunityMech 427 8505a56... (clean); after unsetting the option it read ...+dirty. The live MediaIngredientMech checkout already holds 6 ignored yaml under data/ingredients/mapped/backups/ (git ls-files -o -i --exclude-standard -- data/ingredients). The committed counts in this PR are correct: all ten equal a git ls-tree -r glob count at the pinned SHAs.

Verifier (confirmed, low). Every technical claim reproduces. A bare git status --porcelain never reports ignored files, and it hides untracked ones when status.showUntrackedFiles=no. record_paths() globs the filesystem and counts both kinds. revision() therefore returns a clean 40-hex SHA for a record set that is not that commit's, which contradicts its docstring ("A checkout with uncommitted changes is marked, since its records are not that commit's") and the test's failure message. MediaIngredientMech's own save path writes ignored backups under any data/ingredients/

/backups/, and EXCLUDE_DIRS covers only mapped/. Both scripts share record_paths(), so the census and the stats agree on the wrong count and every RefreshProvenanceTest passes. This is the #88 mechanism, and the new code guard would not flag it.

I downgraded severity from medium to low for three reasons. First, the defect is latent: no live checkout has a counted ignored file today, and the committed numbers in this PR are correct. Second, the refresh procedure this PR introduces (SKILL.md step 3) reads from fresh sparse clones and explicitly compares record_paths() against a NUL-separated git ls-tree at the pin, which catches this whole class of error; the finding omits that check. Third, the global showUntrackedFiles config is not set on this machine.

It is still a real gap. The guard is documented as more than it does, and the README's pipeline section runs against the shared working trees with the default MECHS_ROOT, so there nothing but revision() stands between a stray backup file and an overstated count on the page. The cheapest fix is to add --untracked-files=all --ignored=matching to the status call. The robust fix is to compare record_paths() against git ls-files -z or ls-tree -z inside roots.py, making the skill's step 3 an automated check.

Suggested fix. Stop inferring cleanliness from status. Compare record_paths(name) (relative, NFC-normalized) to git ls-files -z (or git ls-tree -r -z --name-only HEAD) filtered by the same globs and EXCLUDE_DIRS. Mark or fail on any difference. As a minimum, run git status --porcelain --untracked-files=all --ignored=matching -- <glob base dirs>.

A sparse checkout missing a record directory undercounts under a clean revision, and the skill's step-3 check cannot catch it

Where: scripts/fleet/roots.py:89

The skill mandates sparse-checkout set --cone <record dirs> src snapshots. In a sparse checkout, git status reports nothing for paths outside the cone, so revision() cannot tell a full checkout of the pin from a partial one. record_paths only raises when every glob comes back empty. For a Mech with several globs, or with a record dir left partly outside the cone, the scripts silently count a subset under the pinned SHA. test_census_and_stats_count_the_same_records ('Same globs, same revision, so the same files') passes because both scripts see the same subset. The skill's step 3 says it catches 'a sparse set missing a directory', but the check it describes is one-directional (every returned path is tracked), and a subset satisfies it. Step 4's 'files equals the step 3 count' compares record_paths with itself.

Failure scenario. The next refresh builds CommunityMech's sparse set as kb/communities src and forgets data/isolates. The census and the stats both record 422 at 8505a56 with no +dirty, and the tests pass. CommunityMech's card follows its site, which counts only kb/communities (422), so even the card cross-check agrees. The heatmap and the census silently drop the isolates.

Evidence. Scratch probe: git clone --shared --no-checkout CommunityMech, sparse-checkout set --cone kb/communities src, detach at 8505a56d. That printed CommunityMech 422 8505a56d644b02fe87f776be9d667db4cf3f5c5d. After adding data/isolates it printed CommunityMech 426 8505a56.... git ls-tree -r HEAD -- kb/communities data/isolates | grep -c yaml = 426 (data/isolates = 4). The SKILL.md step 3 bullet is at line 96 and the claim at line 101.

Verifier (confirmed, low). The finding is accurate as stated. The one addition is that PR #120's own outputs are correct: all nine census counts equal the glob matches in git ls-tree at the pinned SHAs. The defect is a latent gap in the skill's verification step (SKILL.md:96-102, 119), plus the misleading docstring of roots.revision(). It is not a wrong figure on the page.

Suggested fix. Tie the count to the commit rather than the working tree: in revision() or record_paths, compare the glob matches against git ls-tree -r --name-only HEAD and fail on missing paths. Or refuse when core.sparseCheckout is set and a glob's base dir is not in git sparse-checkout list. Change skill step 3 to require set equality with the tree, not a subset.

'+dirty' is repo-wide, so untracked files unrelated to records fail the provenance test with a misleading message

Where: tests/test_fleet_page.py:420

The status check covers the whole repository, not the record globs (plus src/ for the schema). Any untracked scratch file marks the revision +dirty. test_every_count_names_a_clean_revision then fails with 'read from a checkout with uncommitted changes' even though every counted record matches the commit. Taken with the first finding, the marker misses files that are counted and flags files that are not.

Failure scenario. The README's documented default run over the live checkouts. CommunityMech's checkout holds 10 untracked references_cache/*.md files and 1 untracked file under reports/, none of them records. revision('CommunityMech') returns '+dirty', and the test fails. The operator is told the counts came from uncommitted changes, which is false for the records.

Evidence. GIT_OPTIONAL_LOCKS=0 git -C .../Mechs/CommunityMech status --porcelain: ?? references_cache/PMID_34111782.md ..., broken down as 10 references_cache and 1 reports. git ls-files -o --exclude-standard -- kb/communities data/isolates has 0 yaml.

Verifier (confirmed, low). The core claim reproduces exactly. revision() marks a checkout +dirty whenever git status --porcelain prints anything anywhere in the repository, not just under the record globs or src/. The live CommunityMech checkout has 11 untracked non-record files (10 in references_cache/, 1 in reports/) and no untracked or modified files under kb/communities, data/isolates or src. So revision('CommunityMech') returns '+dirty', and test_every_count_names_a_clean_revision fails for CommunityMech even though every counted record matches the commit. The path that hits this is the README's own pipeline block, which this PR extended with mech_stats.py. It runs with the default MECHS_ROOT, which points at the shared live checkouts.

Three corrections limit the impact, which is why it stays low:

• Only untracked files that are not gitignored trigger it. --porcelain leaves out ignored files, so "any untracked scratch file" overstates it.
• The committed data is clean and all four provenance tests pass today. The defect only shows on a future rerun against live checkouts. The skill's documented procedure (sparse clones at the pins, with a step-3 check that revision() is not +dirty) never hits it.
• The failure message is literally true about the checkout, which does have uncommitted files. It misleads only by implying the counts are tainted. A false positive is also fail-safe: it pushes the operator toward the clean-snapshot workflow the README and skill already recommend. For ProteinTraitsMech, whose live checkout has ~44,940 modified data files, the flag would be correct.

The suggested fix, scoping status or ls-files to the RECORD_GLOBS base dirs plus src/, is reasonable. I did not verify the secondary claim that the marker misses counted files ("with the first finding"), since that belongs to a separate finding. The ignored-file behavior shown above is consistent with it.

Suggested fix. Scope the check to the paths the scripts read: the RECORD_GLOBS base dirs and src/ for mech_stats' schema lookup. The ls-files comparison suggested in the first finding does this naturally.

The 'clean revision' guard cannot see gitignored record files (the #88 case), and revision() itself is untested

Where: scripts/fleet/roots.py:107

revision() marks a checkout +dirty only if git status --porcelain is non-empty. That output omits gitignored files, but record_paths() globs the filesystem and counts them. So a checkout whose record set includes ignored files reports a clean 40-hex sha, and test_every_count_names_a_clean_revision passes. Nothing checks that the globbed files are the commit's tracked files; that check exists only as a manual step (SKILL.md step 3). A sparse clone missing a record directory is also 'clean'. The revision is also taken after the multi-minute scan (prefix_census.py:40), not when each corpus is read. And no test calls revision() at all: deleting the +dirty branch leaves the suite green.

Failure scenario. MediaIngredientMech's .gitignore ignores data/ingredients/*/backups/. EXCLUDE_DIRS removes only mapped/backups/. A checkout with unmapped/backups/x.yaml (same timestamped-copy habit that produced #88) is counted as a record. revision() returns a clean sha, census and stats both count one extra record, and all provenance tests pass. The shared checkout today holds 6 such ignored files under mapped/backups.

Evidence. Fixture at /private/tmp/claude-501/review-pr120/ignored_demo: a repo with .gitignore 'data/ingredients/*/backups/', 2 tracked records plus an untracked data/ingredients/unmapped/backups/b_20260924.yaml.
record_paths -> ['data/ingredients/mapped/a.yaml','data/ingredients/unmapped/b.yaml','data/ingredients/unmapped/backups/b_20260924.yaml']
revision() -> '62e2a3dbb0a045a117d39601d0cb9591e2e730bc' (no +dirty)
ls-tree HEAD -> only the 2 records.
git -C Mechs/MediaIngredientMech ls-files --others --ignored --exclude-standard -- data/ingredients -> 6 files under mapped/backups/.
Mutation: replacing line 107 with return sha in a copy -> 'Ran 33 tests ... OK'.

Verifier (confirmed, low). Every factual part of the finding reproduces. revision() checks only git status --porcelain. That output leaves out ignored files and skip-worktree (sparse) paths, while record_paths() globs the filesystem. A checkout can therefore count records that are not in HEAD, or miss records that are, and still get a clean 40-hex sha that passes test_every_count_names_a_clean_revision. The census-vs-stats test cannot catch this either, because both scripts glob the same files.

That contradicts the docstring's promise ("A checkout with uncommitted changes is marked, since its records are not that commit's"), the test's name, and the PR's line that revision() "names the commit each count was read at". revision() itself has no test at all. Removing the +dirty branch leaves all 33 tests green. The PR's "each invariant was mutation-tested" covers mutations of the data files, not of this function. The revision is also stamped after the scan rather than when each corpus is read.

Why low: the gap is latent. The documented procedure uses fresh sparse clones at the pins, which cannot contain ignored files, and SKILL.md step 3 checks by hand that the globbed paths match ls-tree -r -z at the pin. That manual check is the real guard for exactly this case. I confirmed the committed counts match the tracked trees, so this PR's numbers are unaffected. The failure appears only if the pipeline runs against a working checkout (the default MECHS_ROOT is the shared checkouts, and _fleet/README.md's pipeline section describes running with 'the Mech checkouts available locally'), or if a sparse set drops one of several record dirs and step 3 is skipped. In that case the automated provenance tests give false assurance. The finding's '#88 case' label needs one qualification: the actual #88 files (mapped/backups) are already excluded. The live risk is a new backups/ under unmapped/, which the Mech's own exporter documents creating.

Suggested fix. In revision(), also compare the globbed paths against git ls-files -z (or ls-tree -r -z --name-only HEAD, NFC-normalized) for the record globs. Mark the result dirty, or raise, on any untracked or ignored record or any tracked record missing from the checkout. Take the revision before each Mech's scan and re-check it after. Add a RecordPathTests-style unit test with a throwaway repo covering untracked, ignored and modified files.