compass(tests): the reply-contract guard states its reach and pins each audit check by name - #343
Conversation
…ch check by name The guard compares the paragraph's names and numbers with ATOM's source, and nothing else. The docstring now says so: an assertion with no number and no name is outside its reach, including the replaced rule put back in place of the sentence that corrects it with every number left alone. Each drift now names the checks it must raise, compared as a set, and three new drifts isolate the total, the arity, and which names go unwaited with the count unchanged. Deleting any one of audit's six checks now fails a named drift. REVERTED is pinned by SHA-256 to the pre-fix paragraph as git show prints it. Closes #239 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
| That is the whole reach: an assertion that carries no number and no name is | ||
| outside it. Put the rule this paragraph replaced back in place of the sentence | ||
| that corrects it, with every number left alone, and nothing here fails. The | ||
| source facts behind two such assertions are checked below; that the prose still | ||
| makes them is not. |
There was a problem hiding this comment.
Blocking: the reach is drawn along the wrong axis, and the sentence after it is false as written.
Principle 8: "Every claim carries its measurement. A number without a source is a defect." This is the one paragraph the PR exists to get right.
I measured this on node 18 (xiaobizh_n18_cpu), on head 163c4171d staged with git archive, with atom.__file__ = /tmp/pr343r1gates/head/ATOM/atom/__init__.py. Each edit is one doc edit that keeps the line count (521 → 521), run against tests/compass/test_design_rpc_reply_contract.py:
| edit to the design paragraph | what it carries | module |
|---|---|---|
in the parent, at `engine_core.py:149`, not in the worker. → in the worker, … not in the parent. |
a number and a name | 11 passed |
`call_func`'s `outputs_queue.get()` takes no timeout → takes a timeout |
two names | 11 passed |
`async_proc.py:236-250` → `async_proc.py:1-2` |
numbers | 11 passed |
- "an assertion that carries no number and no name is outside it" is literally true. But it follows "That is the whole reach", so it reads as its complement: a claim that carries a number or a name is inside. All three rows carry a number or a name, and all three are outside. The reach is the six values
auditcompares, not "numbers and names". - "The source facts behind two such assertions are checked below". Here such means carrying no number and no name. But the two claims that
test_the_two_claims_the_paragraph_makes_without_a_numbercovers are rows 1 and 2. One sits onengine_core.py:149, and the other namescall_funcandoutputs_queue.get(). They are not "such" assertions. They are the counterexamples to the sentence before.
A reader who trusts this sentence would take the parent/worker claim as guarded, because it rides on a cited line number. Flipping that claim leaves the module green. That is the misreading #239 was filed to prevent.
One possible wording. It is about the same length and asserts no prose verbatim:
That is the whole reach: the six values
auditcompares. Every other assertion is outside it, including one that sits beside a compared name or number. Flip where the unpack runs, or whether the wait has a timeout, and nothing here fails; nor does putting the rule this paragraph replaced back in place of the sentence that corrects it, with every compared value left alone. The source facts behind those two assertions are checked below; that the prose still makes them is not.
There was a problem hiding this comment.
Fixed in 3874b22e1. I used your wording, with one addition: the third edit, the dispatch citation, is named too.
- The reach is now "the six values
auditcompares". The paragraph above it now lists all six: the names, how many there are, how many are waited on, which are not, the cited line, and the arity. The old list left out the total. - It says every other assertion is outside the reach, "including one that sits beside a compared name or number".
- It names all three of your edits as outside the reach. "The first two" (where the unpack runs, and the timeout) are the claims whose source facts
test_the_two_claims_the_paragraph_makes_without_a_numberchecks. I dropped "such".
Re-measured at 3874b22e1 on node 18 (xiaobizh_n18_cpu), staged with git archive, with atom.__file__ = /tmp/xiaobizh_pr343r2/head/ATOM/atom/__init__.py. Each edit is to the doc and keeps its 521 lines:
| edit | module | named by the new sentence as |
|---|---|---|
parent ↔ worker around engine_core.py:149 |
11 passed | "Flip where the unpack runs" |
takes no timeout → takes a timeout |
11 passed | "or whether the wait has a timeout" |
async_proc.py:236-250 → 1-2 |
11 passed | "or cite other lines for the dispatch" |
All three are still green, and the docstring now declares all three.
| ) | ||
|
|
||
| # The checks each drift must raise and no others, so a check deleted from | ||
| # `audit` fails the drift that isolates it, by name. |
There was a problem hiding this comment.
Non-blocking: "fails the drift that isolates it" overclaims for one check.
Principle 8: "Every claim carries its measurement."
No drift isolates "how many are waited on". Deleting that check at head (line count held at 282) gives 2 failed / 9 passed:
test_the_guard_fires_when_one_side_moves_alone[surface]fails on{'which are not waited on'} == {'how many ar...ot waited on'};[reverted]fails too.
So it is still red by name. The PR body's "each of audit's six checks is pinned by name" holds: deleting the enumeration check is also red, with [enumeration] at 1 failed / 10 passed. But this comment promises an isolating drift for every check, and there is none for this one. Something like "so a check deleted from audit fails, by name, every drift that raises it" is true of all six.
There was a problem hiding this comment.
Fixed in 3874b22e1. The comment now reads: "so a check deleted from audit fails, by name, every drift that raises it."
I checked it for all six deletions at 3874b22e1 on node 18. Each deletion replaces the lines with comment lines and keeps the file at 286 lines. Every red set is exactly the drifts whose FIRES entry holds that check:
| check deleted | red | FIRES entries holding it |
|---|---|---|
| the names it enumerates | 1 failed: [enumeration] |
enumeration |
| how many there are | 2 failed: [reverted], [total] |
reverted, total |
| how many are waited on | 2 failed: [reverted], [surface] |
reverted, surface |
| which are not waited on | 3 failed: [reverted], [surface], [unwaited] |
reverted, surface, unwaited |
| the cited line | 2 failed: [reverted], [citation] |
reverted, citation |
| the arity | 1 failed: [arity] |
arity |
| def test_reverted_is_the_pre_fix_paragraph_byte_for_byte(): | ||
| """A paraphrase would still fire the guard; this keeps `REVERTED` the text | ||
| `git show` prints for that paragraph at the commit named above it.""" | ||
| digest = "36da9cc1ef3f9f1fa8bdf192023dc2e332d6c42dc5dce1ca318a5fb0f8c0a932" | ||
| assert hashlib.sha256(REVERTED.encode("utf-8")).hexdigest() == digest |
There was a problem hiding this comment.
Non-blocking: the digest is correct and taken over the right bytes, but nothing says how to reproduce it.
Principle 8: "A number without a source is a defect."
What I measured from git history, in the container:
- Which bytes. At
05880556e^and atcddda00b50, the doc has 0 CR bytes. The paragraph, split on blank lines, with no trailing newline and LF endings, hashes to36da9cc1…0a932. That matches the pin, and both commits give the same digest. The same text with a trailing\nhashes to53910a45…, and with CRLF to4ffde4ef…. - Line endings in the test file. The literal does not depend on them. With the whole test file converted to CRLF, the module is 11 passed, because Python normalises newlines in source.
- The pin fires. Paraphrasing
killed→crashedfailstest_reverted_is_the_pre_fix_paragraph_byte_for_byte: 1 failed / 10 passed, on'e52d4332…' == '36da9cc1…'.
What is missing is a way to recompute the 64-hex constant. The docstring says "the text git show prints for that paragraph". That leaves the trailing newline open, and that choice alone changes the digest. One comment line would settle it, for example: sha256 of the paragraph at cddda00, first ** to last **, no trailing newline.
It would also help to say that the digest never has a legitimate reason to change. It pins history, not the live document, so the only valid edit is moving the anchor commit.
There was a problem hiding this comment.
Fixed in 3874b22e1. The test's docstring now gives the recipe: "sha256 over that paragraph alone: the document split on blank lines, LF endings, no trailing newline, UTF-8." It also says the digest pins history, "so the only legitimate change is a new anchor commit."
I recomputed it by that recipe from git show in the container:
| commit | CR bytes | recipe | with a trailing \n |
|---|---|---|---|
cddda00b50 |
0 | 36da9cc1…0a932 (= the pin) |
53910a45… |
05880556e^ |
0 | 36da9cc1…0a932 (= the pin) |
53910a45… |
On node 18, the digest test is still green at 3874b22e1. The killed → crashed paraphrase still reddens it: 1 failed / 10 passed, 'e52d4332…' == '36da9cc1…'. The digest literal is unchanged.
|
This review is agent-authored. Review cycle 1 of PR #343 (issue #239). Verdict: REQUEST_CHANGES on head There is one blocking finding: a sentence in the new docstring boundary is false as written. The pins are sound. All six Before reviewing I read the eight design principles in Findings
1. Reproducing the named result on node 18 (
|
| mutant | head 163c4171d |
tip 86d70df49 |
|---|---|---|
| baseline | 11 passed | 7 passed |
| A4: the corrective "wrong shape … traceback" sentence replaced by the false rule | 11 passed | 7 passed |
| delete "how many there are" | 2 failed / 9 passed: …moves_alone[total] (set() == {'how many there are'}) and [reverted] |
7 passed |
| delete the arity check | 1 failed / 10 passed: …[arity] (set() == {'the values ...unpacks into'}) |
7 passed |
| delete "which are not waited on" | 3 failed / 8 passed: …[unwaited] (set() == {'which are not waited on'}), [surface], [reverted] |
7 passed |
| delete the citation check (positive control) | 2 failed / 9 passed: …[citation], [reverted] |
1 failed / 6 passed: …[citation] |
REVERTED paraphrased (killed → crashed) |
1 failed / 10 passed: test_reverted_is_the_pre_fix_paragraph_byte_for_byte ('e52d4332…' == '36da9cc1…') |
7 passed |
| null control (one comment reworded) | 11 passed | 7 passed |
I also deleted the two checks the PR does not claim, to test its "each of the six pinned by name":
| mutant (head only) | result |
|---|---|
| delete "how many are waited on" | 2 failed / 9 passed: [surface] and [reverted] (finding 2) |
| delete "the names it enumerates" | 1 failed / 10 passed: [enumeration] |
So all six checks are red by name at head. Three of them (total, arity, unwaited) are green at tip, which is the gap #239 reported, now closed.
A4 through the full gate, on the merged tree:
| tree | passed | skipped | xfailed | GATE_CPU_RC |
|---|---|---|---|---|
merged 6610672bc |
5257 | 155 | 3 | 0 |
merged + A4 980928649 |
5257 | 155 | 3 | 0 |
The two are bit-identical, and the docstring declares A4 out of reach. The named result reproduces.
2. FIRES is exact in both directions (head, module)
A check added to a declared set. Each of the 7 mutants fails 1 failed / 10 passed, on exactly that drift's node id:
| drift | check added |
|---|---|
[reverted] |
enumeration |
[surface] |
TOTAL |
[citation] |
WAITED |
[enumeration] |
TOTAL |
[total] |
CITE ({'how many there are'} == {'how many th...graph unpack'}) |
[arity] |
TOTAL |
[unwaited] |
WAITED |
A check removed from a declared set. Each of the 7 mutants fails 1 failed / 10 passed, on exactly that drift's node id:
| drift | declared set becomes |
|---|---|
[reverted] |
CITE dropped |
[surface] |
UNWAITED dropped |
[citation], [enumeration], [total], [arity], [unwaited] |
set() (e.g. {'how many there are'} == set()) |
A drift that raises an extra, undeclared check:
| mutant | result |
|---|---|
total drift also turns three values into two values |
1 failed / 10 passed, [total] ({'how many th...unpacks into'} == {'how many there are'}) |
unwaited drift also sets process_kvconnector_output=True, so the waiter count moves |
1 failed / 10 passed, [unwaited] ({'how many ar...ot waited on'} == {'which are not waited on'}) |
None of the 16 mutants is inert.
3. The SHA pin on REVERTED
- The digest is taken over the paragraph only. At
05880556e^and atcddda00b50, the doc has 0 CR bytes. The paragraph (split on blank lines, no trailing newline, LF) hashes to36da9cc1…0a932. That matches the pin at both commits. - Line endings are settled for the test file. With the whole test file as CRLF, the module is 11 passed, because Python normalises source newlines.
- Line endings are not settled in the documentation. A trailing
\ngives53910a45…and CRLF gives4ffde4ef…. Nothing in the file says which bytes were hashed (finding 3). - There is no update procedure, and none is needed. The digest pins a fixed historical paragraph, so its only legitimate change is a new anchor commit. That should be said beside it.
- The pin fires on a paraphrase. See the table in section 1.
4. Are the new docstring sentences true?
| sentence | measurement | verdict |
|---|---|---|
| "Put the rule … back in place of the sentence that corrects it, with every number left alone, and nothing here fails." | A4: 11 passed at head; the gate is 5257/155/3 on both merged and merged+A4 | true |
| "an assertion that carries no number and no name is outside it" (after "That is the whole reach") | See the three edits below. Each carries a number or a name, and each gives 11 passed. | literally true, but it misstates the reach |
| "The source facts behind two such assertions are checked below; that the prose still makes them is not." | The second half is true: the parent/worker flip and the timeout flip both give 11 passed. But those two assertions carry engine_core.py:149, call_func and outputs_queue.get(), so they are not "such". |
false as written (finding 1) |
"The last three move one claim alone" (the comment above DRIFTS) |
FIRES holds singletons for total, arity and unwaited, and the add/remove battery confirms them |
true |
The three edits behind rows 2 and 3:
- parent ↔ worker around
engine_core.py:149; takes no timeout→takes a timeout;async_proc.py:236-250→1-2.
The design paragraph itself is untouched, and its test, source and surface files are identical between 265036deb and 86d70df49.
ruff check on the file gives 0 findings (RUFF_RC=0). ruff format --check would reformat three asserts, all on lines this PR does not touch. There are no design-doc references in the file.
5. ponytail-review
- The PR is 1.5x its estimate, mostly because of
FIRES. I tested the obvious cut, foldingFIRESintoDRIFTSas a third tuple element. Unformatted, it is 282 → 266 lines. Afterruff formatat the default 88 columns it is 286 lines against 282, because five entries wrap. So the separate table is the shorter form. - The label constants (
TOTAL,CITE,WAITED,UNWAITED) are each used two or three times, and inlining them wraps therevertedrow. hashlibplus a six-line test is the minimum for the pin.
Lean already. Ship.
6. Gate: merged tree on the current tip
- I re-read the tip immediately before posting:
86d70df49. git merge-tree --write-tree 86d70df49 163c4171d=af32c7b5bf3fb1ddda93883e657e4dbdde9a0192, with no conflict. It is not163c4171d^{tree}(a10bdacf6…), and it differs from the tip only in this PR's file.- I gated it once as
commit-treecommit6610672bc. The gate printedcommit: 6610672bc (stamp), andatom.__file__=/tmp/pr343r1gates/merged/ATOM/atom/__init__.py.
| tree | passed | failed | skipped | xfailed | GATE_CPU_RC |
|---|---|---|---|---|---|
control, tip 86d70df49 |
5253 | 0 | 155 | 3 | 0 |
merged af32c7b5b |
5257 | 0 | 155 | 3 | 0 |
--collect-only gives 5390 → 5394: +4, −0. All four are in this file: [total], [arity], [unwaited] and test_reverted_is_the_pre_fix_paragraph_byte_for_byte. No timing-class test failed on any run, so nothing needed a re-run.
To reach APPROVE
- Fix finding 1. State the reach as "the six values
auditcompares", name the beside-a-number case, and drop "such". The inline comment has a suggested wording. - Findings 2 and 3 are one comment line each. They are optional, but cheap to fix in the same commit.
…s as its reach The docstring drew the guard's reach as "numbers and names", but three edits that carry a number or a name leave the module green: swapping parent and worker around the cited unpack line, giving the wait a timeout, and changing the dispatch citation. The reach is the six values audit compares; the docstring now says so, lists all six, and names those edits as outside it. The FIRES comment no longer promises an isolating drift for every check (none isolates the waiter count); it says a deleted check fails every drift that raises it. The SHA pin states its recipe and that the only legitimate change is a new anchor commit. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Developer round 2 on PR #343: new head This answers the cycle-1 REQUEST_CHANGES with one new commit. Nothing was amended or force-pushed. Before starting I read the eight design principles in No blocking issues remain open on my side. All three findings are fixed:
Named result, node 18 (
|
| mutant | module | red node ids (test_the_guard_fires_when_one_side_moves_alone[...] unless stated) |
|---|---|---|
| baseline | 11 passed | — |
| parent ↔ worker | 11 passed | — (now declared) |
takes a timeout |
11 passed | — (now declared) |
async_proc.py:1-2 |
11 passed | — (now declared) |
| delete enumeration check | 1 failed / 10 passed | [enumeration] |
| delete total check | 2 failed / 9 passed | [reverted], [total] |
| delete waited-count check | 2 failed / 9 passed | [reverted], [surface] |
| delete unwaited check | 3 failed / 8 passed | [reverted], [surface], [unwaited] |
| delete cited-line check | 2 failed / 9 passed | [reverted], [citation] |
| delete arity check | 1 failed / 10 passed | [arity] |
| null control | 11 passed | — |
REVERTED paraphrase |
1 failed / 10 passed | test_reverted_is_the_pre_fix_paragraph_byte_for_byte ('e52d4332…' == '36da9cc1…') |
The digest is unchanged. The literal is still 36da9cc1ef3f…0a932, and its test passes at the new head.
Gate: merged tree on the current tip
- The tip moved while I worked, from
86d70df49tobdd244c57(compass(docs): stop restating the CPU gate census in CLAUDE.md, and pin two scripts/compass figures #339:CLAUDE.mdandscripts/compass/only). I re-read it after pushing. git merge-tree --write-tree bdd244c57 3874b22e1=b19f4d3569caed4acc563bdcc6782e50c6a802c4, with no conflict. It differs from the tip only in this file.- I gated it once as
commit-treecommit0ad586250. The gate printedcommit: 0ad586250 (stamp), andatom.__file__=/tmp/xiaobizh_pr343r2/merged2/ATOM/atom/__init__.py.
| tree | passed | failed | skipped | xfailed | GATE_CPU_RC |
|---|---|---|---|---|---|
control, tip bdd244c57 |
5253 | 0 | 155 | 3 | 0 |
merged b19f4d356 |
5257 | 0 | 155 | 3 | 0 |
--collect-onlygives 5390 → 5394: +4, −0. All four are in this file.- Against the earlier tip
86d70df49(merged tree82508847f), the counts were the same: 5257 against 5253, both withGATE_CPU_RC=0. - No timing-class test failed on any run.
ruff checkon the file passes (RUFF_RC=0). The file has no design-doc references.
Lines this round: +14 / −10, all in tests/compass/test_design_rpc_reply_contract.py, all docstrings or comments. Against the brief's 10–25, the PR total is +42 / −4, about 1.7x and under the 2x line.
Next: a delta review of 163c4171d..3874b22e1.
|
This review is agent-authored. Review cycle 2 of PR #343 (issue #239). It is a delta review of Verdict: APPROVE on head There are no blocking issues, and no non-blocking ones. All three cycle-1 findings are fixed, and every sentence the delta rewords is true as measured below. No inline comments, because nothing points at a line. Before reviewing I read the eight design principles in
1. The delta is comments and docstrings onlyChecked in the container, with
The other hunk, the 2. Is every reworded sentence true?All runs are on node 18 (
a. "the six values
That is six on each side, with none missing and none extra. The old list's gap (no total) is closed. b. "Every other assertion is outside it, including one that sits beside a compared name or number" (L24–30):
All five are green, and the first four are exactly the edits the docstring names. "The source facts behind the first two are checked below" is also true. I reinstated each source-side break in
c. "a check deleted from
"By name" holds in both senses:
d. Controls:
3. The SHA recipe, followed literallyThe docstring says: "sha256 over that paragraph alone: the document split on blank lines, LF endings, no trailing newline, UTF-8". "The commit named above it" is
The pin reproduces under all three readings of "split on blank lines", so the recipe is not ambiguous for this paragraph. The selected paragraph is byte-equal to the 4. ponytail-review (delta)The delta is 14 lines of docstring and comment.
Both additions were requested in cycle 1, and each carries a measurement the other lines depend on. Nothing restates something already stated nearby, so no line can be cut without dropping a checked claim.
5. Gate: merged tree on the current tip
NextThe approval covers tree |
1 participant
Closes #239
What changed
This touches one file,
tests/compass/test_design_rpc_reply_contract.py, adding 42 lines and removing 4, all test code. No production code changed. The design paragraph is unchanged: I checked each of its sentences againstasync_proc.pyandengine_core.pyat265036deb, and none is false.The boundary is stated. The module docstring says that the guard's whole reach is the six values
auditcompares: the names the paragraph enumerates and how many there are, how many are waited on, which are not, and the site and arity it cites for the unpack. It says that every other assertion is outside that reach, including one that sits beside a compared name or number. It names four such edits that fail nothing here:It also says that
test_the_two_claims_the_paragraph_makes_without_a_numberchecks the source facts behind the first two, not whether the prose still makes them. The docstring asserts no prose verbatim. (Round 2 rewrote this, after review found "numbers and names" was the wrong boundary.)Each of
audit's six checks is pinned by name. Each drift now names the exact set of checks it must raise (FIRES), compared as a set, so a check deleted fromauditfails, by name, every drift that raises it. Three drifts are new:total: twelve becomes eleven, which moves only "how many there are".arity: three values becomes two values, which moves only the arity check.unwaited: a surface swap (exitwaits,dummy_executiondoes not), which moves only "which are not waited on". The waiter count stays at ten.REVERTEDis pinned byte-for-byte. It is pinned by SHA-256,36da9cc1…0a932, to the paragraph atcddda00b50, which is the same as at05880556e^. The recipe is written beside the digest: that paragraph alone, split on blank lines, LF endings, no trailing newline, UTF-8. The docstring also says the digest pins history, so the only legitimate change to it is a new anchor commit.REVERTEDis test data, so this puts no verbatim assertion on the live document.Named result (node 18,
xiaobizh_n18_cpu), at head3874b22e1Setup.
git archiveplus stamps. The tar md5 was49accf0af8b0258b75de586b12f025fcon both ends.atom.__file__=/tmp/xiaobizh_pr343r2/head/ATOM/atom/__init__.py.The runs are against
tests/compass/test_design_rpc_reply_contract.py.engine_core.py:149takes no timeout→takes a timeoutasync_proc.py:236-250→1-2…moves_alone[enumeration][reverted],[total][reverted],[surface][reverted],[surface],[unwaited][reverted],[citation][arity]REVERTEDparaphrased (killed→crashed)test_reverted_is_the_pre_fix_paragraph_byte_for_byte('e52d4332…' == '36da9cc1…')At the tip, before this PR, deleting the total, arity or unwaited check left the module green (round 1, and the review reproduced it). That is the gap #239 reported.
A4 is the issue's mutant: the corrective "wrong shape … traceback" sentence is replaced by the false rule, and every compared value is unchanged. In round 1 it was measured bit-identical to the baseline through the full gate, and the review reproduced that on the merged tree (5257 / 155 / 3 on both). The docstring declares A4 out of reach.
Gate 1: merged tree on the current tip
bdd244c57(compass(docs): stop restating the CPU gate census in CLAUDE.md, and pin two scripts/compass figures #339, which changes onlyCLAUDE.mdandscripts/compass/). I re-read it after pushing.git merge-tree --write-tree bdd244c57 3874b22e1=b19f4d3569caed4acc563bdcc6782e50c6a802c4, with no conflict. It differs from the tip only in this file.commit-treecommit0ad586250. The gate printedcommit: 0ad586250 (stamp).GATE_CPU_RCbdd244c57b19f4d356--collect-onlygives 5390 → 5394: +4, −0. All four are in this file:[total],[arity],[unwaited]andtest_reverted_is_the_pre_fix_paragraph_byte_for_byte. No timing-class test failed, so nothing needed a re-run.Lines: production +0 / −0. Tests +42 / −4 (
git diff --numstat 265036deb 3874b22e1); round 2 was +14 / −10 of that. The brief estimated 10–25 lines, so this is about 1.7x, under the 2x escalation line. Most of the extra isFIRES.Dev record
exit=True, dummy_execution=False, moves the names alone.assert audit(...), passed if any check fired. Every drift now asserts the exact set of checks.[surface]and[reverted]. TheFIREScomment now claims only "every drift that raises it", which holds for all six checks.🤖 Generated with Claude Code