Found in the adversarial review of #120.
#107 is closed with no regression test; reverting the fix leaves CI green
Where: scripts/fleet/build_subsets.py:192
The PR closes #107 with correct sort fixes, but adds no test. The only guard is the skill's manual 'run build_subsets.py twice and compare bytes'. Every other pipeline issue in this file (#88, #95, #97, #98, #102, #104) has a regression test. Reintroducing dict(byp.most_common()) or unsorted cell iteration passes all 33 tests, and churn returns silently on the next refresh.
Failure scenario. A later refactor restores most_common() (the idiomatic spelling). CI passes, and the next refresh commits tie-order flips across dozens of edge files, which is the noise #107 set out to remove.
Evidence. Copy /private/tmp/claude-501/review-pr120/tg-f66c2125/repo_mut with both #107 hunks reverted (git diff --stat: build_subsets.py 2+/2-) -> 'OK' for the full suite. The fix itself holds: in the isolated snapshot at the pins, PYTHONHASHSEED=1 and =2 runs were byte-identical to each other and to the committed assets/fleet and subsets_summary.json ('RUNS IDENTICAL', 'changed vs committed: 0').
Verifier (confirmed, low). The core claim holds. The PR says it closes #107, and the fix itself is correct: seed-dependent tie order is replaced with a total order. But the PR adds no test for it. Reverting all three edits leaves the whole 33-test suite passing, and the PR CI job would pass too. The PR body's determinism claim rests on a one-off manual run, and the skill's "run twice and compare bytes" step is the only ongoing guard.
This is a test-coverage gap, not a functional defect. The shipped code and committed assets are correct, and #107 never asked for a test. If most_common() comes back, the result is noisy diffs on the next refresh, not wrong numbers.
One side claim is overstated. "Every other pipeline issue in this file (#88, #95, #97, #98, #102, #104) has a regression test" is loose. #88 (backup copies counted as MediaIngredientMech records) is about record-path filtering in roots.py. #95 and #102 are prefix_census.py issues, and #98 is the census guard test, so it lives in the suite itself. #104 (card drift) is about check_cards.py. Only #97 is a build_subsets.py issue. The point still stands that each of those issues has a tagged regression test and #107 does not.
The suggested fix is feasible. Such a test would cover both of the PR's leaks, because they are the only hash-seed-dependent spots in the file. Severity stays low.
Suggested fix. Add a test that runs build_subsets.main() in a subprocess over a tiny fixture MECHS_ROOT, under two PYTHONHASHSEED values, with OUT/DATA redirected to a temp tree. The fixture needs tied prefix counts on one edge and several prefixes per Mech. Assert the outputs are byte-identical.
Found in the adversarial review of #120.
#107 is closed with no regression test; reverting the fix leaves CI green
Where:
scripts/fleet/build_subsets.py:192The PR closes #107 with correct sort fixes, but adds no test. The only guard is the skill's manual 'run build_subsets.py twice and compare bytes'. Every other pipeline issue in this file (#88, #95, #97, #98, #102, #104) has a regression test. Reintroducing
dict(byp.most_common())or unsorted cell iteration passes all 33 tests, and churn returns silently on the next refresh.Failure scenario. A later refactor restores most_common() (the idiomatic spelling). CI passes, and the next refresh commits tie-order flips across dozens of edge files, which is the noise #107 set out to remove.
Evidence. Copy /private/tmp/claude-501/review-pr120/tg-f66c2125/repo_mut with both #107 hunks reverted (git diff --stat: build_subsets.py 2+/2-) -> 'OK' for the full suite. The fix itself holds: in the isolated snapshot at the pins, PYTHONHASHSEED=1 and =2 runs were byte-identical to each other and to the committed assets/fleet and subsets_summary.json ('RUNS IDENTICAL', 'changed vs committed: 0').
Verifier (confirmed, low). The core claim holds. The PR says it closes #107, and the fix itself is correct: seed-dependent tie order is replaced with a total order. But the PR adds no test for it. Reverting all three edits leaves the whole 33-test suite passing, and the PR CI job would pass too. The PR body's determinism claim rests on a one-off manual run, and the skill's "run twice and compare bytes" step is the only ongoing guard.
This is a test-coverage gap, not a functional defect. The shipped code and committed assets are correct, and #107 never asked for a test. If most_common() comes back, the result is noisy diffs on the next refresh, not wrong numbers.
One side claim is overstated. "Every other pipeline issue in this file (#88, #95, #97, #98, #102, #104) has a regression test" is loose. #88 (backup copies counted as MediaIngredientMech records) is about record-path filtering in roots.py. #95 and #102 are prefix_census.py issues, and #98 is the census guard test, so it lives in the suite itself. #104 (card drift) is about check_cards.py. Only #97 is a build_subsets.py issue. The point still stands that each of those issues has a tagged regression test and #107 does not.
The suggested fix is feasible. Such a test would cover both of the PR's leaks, because they are the only hash-seed-dependent spots in the file. Severity stays low.
Suggested fix. Add a test that runs build_subsets.main() in a subprocess over a tiny fixture MECHS_ROOT, under two PYTHONHASHSEED values, with OUT/DATA redirected to a temp tree. The fixture needs tied prefix counts on one edge and several prefixes per Mech. Assert the outputs are byte-identical.