compass(gates): join the README's exclude-list counts to the list - #310
Conversation
scripts/compass/README.md states cpu_gate_exclude.txt's counts (29 total, 28 GENERATED, 1 MANUAL), and nothing checked them. An entry added to the MANUAL section reddened the in-file pin and left the README wrong in silence. Add test_the_readme_states_the_counts_the_list_holds, parametrised over the three numbers in the README row. Each one is compared with the count derived from the list, the same shape gate_gpu.sh uses for BASE_FAILED against gpu_gate_known_failures.txt. A disagreement names the README, the stated number, the list and the entries it holds. A missing or reworded row fails by name rather than passing vacuously. Nothing under scripts/compass/ changes. Closes #213 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
| ids=["total", "generated", "manual"], | ||
| ) | ||
| def test_the_readme_states_the_counts_the_list_holds(stated, derive): | ||
| # README.md restates this file's counts, and the pin above holds the MANUAL |
There was a problem hiding this comment.
Non-blocking (principles 3 and 8). The PR body says the MANUAL pin (test_the_manual_section_holds_the_entries_it_is_pinned_to_hold, L147-168) is kept because "the join does not replace it". I measured that claim, and it does not hold. The pin's stated job is to stop an emptied MANUAL section from passing silently, with its parametrisation collecting nothing. This join does that job already.
| tree | edit (line count preserved) | result, tests/compass/test_cpu_gate_exclude.py only |
|---|---|---|
a821914ad = head + MANUAL emptied |
cpu_gate_exclude.txt:51 commented out |
3 failed, 68 passed, 1 skipped: the pin, [total], [manual] |
a833627b9 = the same + pin neutralised |
also assert len(entries) == 1 -> >= 0 |
2 failed, 69 passed, 1 skipped: test_the_readme_states_the_counts_the_list_holds[total] and [manual] (states **1 MANUAL** but ... holds 0: []) |
After this PR, the MANUAL count is stated twice by hand: **1 MANUAL** in the README and == 1 in the pin. Both are joined to the list, and every edit that fails one fails the other. Reaching zero on purpose now takes two edits instead of one.
Either choice is fine:
- Delete the pin (about -24 lines). Then point the control test's docstring (L172) and the module docstring (L28-30) at this join.
- Keep the pin with a reason that holds. The only real difference is locality: the pin's number sits in the file the list is tested from. The PR body's sentence should then say that, not that the join cannot replace it.
| entries = derive() | ||
| assert int(found[1]) == len(entries), ( | ||
| f"{README.name} states {found[0]} but {EXCLUDE.name} holds {len(entries)}: " | ||
| f"{entries}. Update the README row to match the list." |
There was a problem hiding this comment.
Non-blocking (principle 6). This sentence picks a side, which the comment at L206 says the join does not do ("a disagreement names both rather than picking one"). gate_gpu.sh's message is the model this test copies, and it only states the two numbers: BASE_FAILED=%s but %s names %s node-id(s).
The side it picks is wrong in exactly the case the MANUAL pin exists for. Emptying the MANUAL section by accident (mutation a821914ad: cpu_gate_exclude.txt:51 commented out) fails [manual] with README.md states **1 MANUAL** but cpu_gate_exclude.txt holds 0: []. Update the README row to match the list. Here the list is the wrong side, and the message tells the reader to make the README agree with it. For GENERATED the list usually is right, because a regenerator wrote it. For MANUAL it is a hand edit, and either side can be the mistake.
Suggested fix: end the message at {entries}., or replace the last sentence with a neutral one such as One of the two is wrong; fix that one. This matters more if the pin goes (see the comment on L202), because the pin's "Add or remove one deliberately" wording is then no longer printed beside it.
|
This review is agent-authored. I read the eight design principles in Verdict: APPROVE
There are two inline comments, both non-blocking:
Rulings1. Assert rather than remove (principle 3): keeping the numbers and pinning them is the right call.
3. Parser robustness (principle 6): it refuses on every format change I tried, and never falls back.
Named result and mutationsNode 18, Each run covered
M1-M4 are my own. The developer's table reproduces exactly, with my own commit objects. Gate: the tree that will land
5225 is the number the history predicts:
The three new ids are in the merged run's
Other checks
ponytail-reviewThe added 28 lines themselves are the minimum: one parametrised test, a row lookup, a regex, and an assert. The three cases are worth their node ids, since M2′ and M3′ each redden a different subset. net: -24 lines possible. |
…peats Each count that scripts/compass/ restated away from its source is now either held to that source by a test, or no longer restated. Joined, in the shape #310 used for the exclusion list's counts: - the README's trigger-path count, against gpu_gate_triggers.txt, as a `triggers` case of test_the_readme_states_the_counts_the_list_holds; - the README's GPU baseline (passed, failed, commit), against gate_gpu.sh's BASE_PASSED / BASE_FAILED / BASE_COMMIT, in test_the_readme_states_the_baseline_the_gate_holds; - gate_gpu.sh's worked `4779 + 0 - 49 = 4730`, which a test pinned as the literal "4730". It is now derived from BASE_PASSED and BASE_COMPASS_TESTS, so a rebaseline that leaves the comment behind reddens. No longer restated. These either moved with most tasks, were already wrong at the tip, or repeated a figure that is now joined: - "130 of 189 test files" in the README and gate_cpu.sh. At 2565b5f it is 166 of 225; 130/189 was fada742's census; - gate_cpu.sh's 30 plugin files and 29 / 28 / 1 exclusions; - "(130 files)" on two Baselines rows. At 186d128 the tier was 144 files; - the README's second "30 source paths", the two "five failing node-ids", and the numbers in "What the GPU gate expects", now given as constant names; - "190 of this tree's indented imports", in the README and in regen_gpu_gate_triggers.sh. It is 205 at the tip, so it is now pinned to fada742, where it was measured; - "removes no path from this tree: 30 triggers with it, 30 without", in the regenerator's header template and in the file it wrote. That was measured at 236abfd, yet every regeneration stamped it as "this tree". It is now pinned to 236abfd; - "All five" in two gate_gpu.sh comments, and "All five callers" in _lib.sh. Every scripts/compass/ edit is to a comment or the README. The one change to gpu_gate_triggers.txt is to a `#` line, which gate_cpu.sh skips. Closes #311 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…peats (#327) * compass(gates): join or stop restating the counts scripts/compass/ repeats Each count that scripts/compass/ restated away from its source is now either held to that source by a test, or no longer restated. Joined, in the shape #310 used for the exclusion list's counts: - the README's trigger-path count, against gpu_gate_triggers.txt, as a `triggers` case of test_the_readme_states_the_counts_the_list_holds; - the README's GPU baseline (passed, failed, commit), against gate_gpu.sh's BASE_PASSED / BASE_FAILED / BASE_COMMIT, in test_the_readme_states_the_baseline_the_gate_holds; - gate_gpu.sh's worked `4779 + 0 - 49 = 4730`, which a test pinned as the literal "4730". It is now derived from BASE_PASSED and BASE_COMPASS_TESTS, so a rebaseline that leaves the comment behind reddens. No longer restated. These either moved with most tasks, were already wrong at the tip, or repeated a figure that is now joined: - "130 of 189 test files" in the README and gate_cpu.sh. At 2565b5f it is 166 of 225; 130/189 was fada742's census; - gate_cpu.sh's 30 plugin files and 29 / 28 / 1 exclusions; - "(130 files)" on two Baselines rows. At 186d128 the tier was 144 files; - the README's second "30 source paths", the two "five failing node-ids", and the numbers in "What the GPU gate expects", now given as constant names; - "190 of this tree's indented imports", in the README and in regen_gpu_gate_triggers.sh. It is 205 at the tip, so it is now pinned to fada742, where it was measured; - "removes no path from this tree: 30 triggers with it, 30 without", in the regenerator's header template and in the file it wrote. That was measured at 236abfd, yet every regeneration stamped it as "this tree". It is now pinned to 236abfd; - "All five" in two gate_gpu.sh comments, and "All five callers" in _lib.sh. Every scripts/compass/ edit is to a comment or the README. The one change to gpu_gate_triggers.txt is to a `#` line, which gate_cpu.sh skips. Closes #311 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * compass(gates): drop the undated gate timings and pin what "this tree" meant Answers review cycle 1 on #327. Comments, README and one test only; no executable line of any script changes. - gate_cpu.sh and the README's gate_cpu.sh row drop "~30 s" / "~31 s". Nothing reads either figure, and the merged tree's gate took 183.82 s of pytest. _lib.sh drops "~1 s beside the superset's 72 s" for the same reason. - gate_gpu.sh's 4779 + 0 - 49 = 4730 is now about "such a tree", not the integration branch, which has carried tests/compass/ since 4c16792. The README, _lib.sh and the surplus test's docstring said the same false thing. - The regenerator's comment on the header no longer claims every number came from this run: it names which counts do, and the template's line citations now name 236abfd, where they were measured. - "The collection probe removes no path" is pinned to 236abfd in the README and the regenerator's header instead of "currently" and "this tree". - The README's 186d128 row is labelled by its commit, not "current". - The undated "106-file" subtree count is dropped from both places. - test_the_readme_states_the_baseline_the_gate_holds reads the README row with one named-group regex. The three node ids are unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Closes #213
What changed
tests/compass/test_cpu_gate_exclude.pygets one test,test_the_readme_states_the_counts_the_list_holds, parametrised as[total],[generated]and[manual]. Each case reads its number from thecpu_gate_exclude.txtrow ofscripts/compass/README.mdand asserts that it equals the count derived from the list. On a disagreement the message names the README, the stated number, the list and the entries it holds.A missing row or a reworded number also fails by name. It does not pass vacuously.
Nothing under
scripts/compass/changes. The directory's tree object isba78568c8at both the tip and the head, so the gate below is the same instrument on both sides.Lines: 0 production, 28 test (+28/-0). The estimate was ~35.
Staleness check at
f84b4d778scripts/compass/README.md:20still restates 29 / 28 GENERATED / 1 MANUAL, and nothing joins it to the list.The choice: assert the README against the list, rather than stop restating
gate_gpu.sh:111-122settles the shape. It keepsBASE_FAILED=5as a stated number, countsgpu_gate_known_failures.txt, and on a disagreement refuses, naming both. The README's own row for that file describes the same shape: "two statements of one fact;gate_gpu.shrefuses to run if they disagree". This PR copies that shape instead of inventing a second one.On simplicity: dropping the numbers from the README would be fewer lines. But the row's 29 is the size of the CPU tier's blind spot, and that figure is the reason a reader opens the row. That is also why
BASE_FAILEDis kept beside its list rather than removed.The join also costs nothing the gate executes. The alternative, editing the README, would have moved the
scripts/compasstree object, and that object is the instrument every open gate delta is measured with.The existing MANUAL pin (
== 1) is kept as it is. It is the in-file statement that separates a chosen empty section from one that arrived, and the join does not replace it.Named result: node 18,
xiaobizh_n18_cpu, one module per treeEach mutation is a line-count-preserving edit built with
git commit-tree, so every run names a commit.atom.__file__resolved under each staged root in every run.f84b4d778(null control)3fb992d2c(null control)678c85a2a= tip + manual entrycpu_gate_exclude.txt:49, a comment line becomestests/test_gc_utils.pytest_the_manual_section_holds_the_entries_it_is_pinned_to_hold. The README stays wrong in silence.c17909d47= head + the same entrytest_the_readme_states_the_counts_the_list_holds[total]andtest_the_readme_states_the_counts_the_list_holds[manual]197c307ec= tip + README says 2 MANUALREADME.md:20,**1 MANUAL**becomes**2 MANUAL**038cee2c3= head + README says 2 MANUALtest_the_readme_states_the_counts_the_list_holds[manual]The join's assertion text at
c17909d47,[manual]:The added entry was chosen to break nothing else.
tests/test_gc_utils.pyexists, sorts before the existing entry, keeps a comment line directly above it, and is not in the GENERATED section. Only the counts move.I also fired the two refusal paths on the head tree, both with line-count-preserving README edits:
**29** excludedreworded to29 excluded: 1 failed,[total], with "README.md's cpu_gate_exclude.txt row no longer states /…/".One note from the brief, stated rather than left to happen quietly: this PR does not raise the MANUAL count, so
test_the_manual_section_is_sorted_and_uniqueis still trivially true at n=1. At n=2 in the mutation above it had signal and passed, because the added entry was placed in sorted order.Gate 1: ATOM's suite, unmodified, as a delta against a measured control
Node 18,
xiaobizh_n18_cpu. Each tree was staged withgit archiveanddocker exec -i … tar -xinto/tmp/i213gates/<side>/ATOM, with.compass-commit/.compass-changedwritten from the same sha. Content digests matched on both ends. Each tree ran its ownscripts/compass/gate_cpu.sh, unpiped, undertimeout -k 10, one at a time.GATE_CPU_RCf84b4d778f84b4d778 (stamp), gpu: not required3fb992d2c3fb992d2c (stamp), gpu: not requiredNode-id delta (from
--junitxml): 0 ids only in the control, and 3 only in the head, all passed:tests/compass/test_cpu_gate_exclude.py::test_the_readme_states_the_counts_the_list_holds[total]…[generated]…[manual]No flake-listed test moved.
git merge-tree --write-tree f84b4d778 3fb992d2cgives65f9f1fedbcd067c9f7d375cabc8820f3aa8434a, which equals3fb992d2c^{tree}.Gate 2: CPU-only tests
The three ids above run in the CPU tier. They exercise
scripts/compass/README.mdandscripts/compass/cpu_gate_exclude.txt, neither of which this PR adds or edits.ruff checkandruff format --checkon the file: clean.Not done, and left for a successor (filed as #311)
scripts/compass/README.mdrestates other counts with no join: 30 trigger paths on line 21 (still right at the tip), and4779 passed / 5 failedplus "five" known failures on lines 15 and 22.regen_gpu_gate_triggers.shrewrites the file wholesale, and the README would not follow.atom/compass/design/16_execution_plan.md:171also says "29 entries are 28 GENERATED + 1 MANUAL". It is a dated record of what one commit produced ("From this commit…"), not a live statement, so it is not joined.🤖 Generated with Claude Code