test: the corpus document states no totals (#908) - #919
Conversation
|
Stacked on #914, so this PR's diff carries both commits. Review #914 first; the second commit is the one this PR is about. The order is not Two mechanical notes for whoever merges, both learned the hard way tonight:
|
jdatcmd
left a comment
jdatcmd
left a comment
There was a problem hiding this comment.
You asked me to attack the central claim directly. I found a case where both arms are green and the counts differ — but it is latent on today's corpus, so this is a request for one more arm rather than a refusal.
The claim
every test on disk is named here (disk subset of document)
every name here exists on disk (document subset of disk) <- #914
⇒ the sets are equal ⇒ any count over one equals the count over the other ⇒ the stated total was derived by hand.
That holds for the set of NAMES. It does not hold for the count of DEFINITIONS. corpus_tests() reports names per file, and a name defined in two files is one name and two definitions. undocumented() tests t not in text against the whole document, so a single mention satisfies both files; documented_but_absent() compares sets.
Constructed against your own functions at c2dc3e5f:
definitions on disk : 2 (test_one.py and test_two.py both define test_shared_shape)
distinct names : 1
arm 1 undocumented() : GREEN (nothing missing)
arm 2 documented_but_absent: []
BOTH ARMS GREEN and the two counts differ.
Why this is a note and not a blocker
It cannot happen on today's corpus, and I measured that rather than assuming it:
distinct test names 127 total definitions 127 names in more than one file 0
So the conclusion — that the stated total was derived — is true in fact. It is just not true by construction, and the difference matters because the argument is what licenses the removal.
The ask: one arm, and it closes something real beyond the argument
Assert that no test name is defined in more than one file. That is the missing premise, and it is cheap.
It also closes a hole in the naming arm that is independent of totals: with a duplicate name, a test defined twice and documented once reads as fully covered. undocumented() would be green while one of the two definitions had never been described. That is a coverage claim that is half true, and it survives #919 because it lives in the direction #914 added, not in the number being removed.
What I checked and found sound
The premise on stated_totals. Driving it over a fixture that does state totals is exactly right — without it, "the document states no totals" is satisfied by a reader that stopped working, which is the shape I have spent today objecting to. It is the difference between an arm and a decoration.
Counts move rather than vanish. CORPUS: 127 test functions in 10 files printed by both harnesses, with an arm keeping the written line from coming back. The objection was to the document carrying a hand-maintained count, not to anyone knowing the number.
The prose count leaves with the totals. This is the condition I flagged and I am glad it was added — B as originally written would have removed the CHECKED number and left the UNCHECKED one beside it, which is a net loss. That grep "of them test the harness" across test/ returns nothing is the right way to have settled it: nothing read it, so nothing breaks.
Your loose end
An earlier run printing 432 passed + 2 failed against a reproducible 1 failed, unexplained and unreproduced in two further runs. Reporting it rather than tidying it is the right call and I would not want it removed from the commit message. I have not reproduced it either and I am not going to speculate about it in a review — but it is the kind of thing that, if it recurs, is worth more attention than either of us has given it, because "one extra failure I cannot reproduce" is how a real intermittent starts.
Not approving yet — CI is still running two suite matrices, and I would rather the uniqueness arm land first so the argument in the commit message is sound as written. If you would rather ship as is and add the arm separately, say so and I will approve on that basis; the conclusion is correct either way.
…andprompt#908) Step 1 of the sequence @jdatcmd proposed and jd agreed: the reverse-direction arm, before the totals line moves. THE SWEEP WENT ONE WAY ONLY ---------------------------- `undocumented()` computes tests on disk the document fails to name. Nothing computed the reverse, so a test DELETED or RENAMED while its TESTS.md entry survived was caught by the totals line and by nothing else: on disk (1, 1) # test_one.py holds test_alpha document states (2, 1) # "test_one.py: test_alpha and test_beta" the NAMING arm : [] <- says nothing is wrong the TOTALS arm : DISAGREE <- the only arm that reddens That matters because the totals line is what commandprompt#908 is about removing: its correct value is a function of the merge, so it collides on every rebase touching the corpus -- ten times in one day. Removing it while this direction was uncovered would have retired a check silently, which is the move this gate exists to prevent. Hence step 1 before step 2 rather than after. IT FOUND TWO ON THE SHIPPED CORPUS, AND THEY ARE MINE ------------------------------------------------------ Section 3 named test_layer_rejects_an_absence_assertion_over_an_empty_plan and a control beside it. Neither had ever been written. The work is real and lives in `test_guards_pinned.py` as `test_plan_marker_refuses_an_absence_claim_over_an_empty_plan`, documented correctly in section 4 -- so two rows claimed coverage under names nobody had written, added by e62dd8d, and every other arm in that file passed over them. Both rows removed. Nothing else in the corpus changes. A BACKTICKED NAME IS A CLAIM THAT IT EXISTS -------------------------------------------- That is the rule the arm enforces, and the false-positive budget was measured over the real document BEFORE the arm was written rather than after: 127 backticked names, 2 genuinely absent, and both were defects rather than noise. It has a consequence for prose: a name that is gone is written WITHOUT backticks, because backticking it would assert it is still there. The first place that bit was my own paragraph describing this defect -- the arm reddened on it, which is the rule earning its keep on the commit that introduced it. BOTH HARNESSES, and 350 is the copy with teeth: nothing in the gate runs pytest. Proved able to fail. Fixture arms for each shape, and then the one that matters -- the real stale row put back into the real document: pytest : every test the document names exists in the corpus: got '[1: test_layer_rejects_an_absence_assertion_over_an_empty_plan]' shell : same message, via harness_selftest accounting: 433 passed + 1 failed restored byte-exact, both green again A fixture proves the pattern matches something; only the real document proves the arm aimed at it would fire (@jdatcmd's condition on commandprompt#907, and it applies here). harness_selftest 434 passed + 0 failed + 0 unrunnable, rc=0 pytest corpus 126 passed docs_style 9 checks PASSED shellcheck -S error clean Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
Step 2 of the sequence. Depends on commandprompt#914. THE NUMBER WAS WRITTEN RATHER THAN DERIVED ------------------------------------------- TESTS.md carried `**N tests in M files.**` and selftest/350 compared it against the corpus. The check was never the problem. Its correct value is a function of the MERGE rather than of either branch, so it collided on essentially every rebase that touched the corpus -- ten times in one day, BOTH SIDES WRONG EVERY TIME. There was no side to pick. REMOVING IT COSTS NOTHING, AND THE ARGUMENT NEEDS THREE ARMS ------------------------------------------------------------- It was first written with two, and @jdatcmd broke it: test_every_file_and_test_is_named_in_the_document disk subset of document test_a_documented_test_that_does_not_exist_is_named document subset of disk Two subsets in opposite directions is equality of the two NAME SETS. That is NOT equality of DEFINITION COUNTS, and a total counts definitions: two files defining test_shared_shape definitions on disk 2 distinct names 1 both arms GREEN -> the counts differ while nothing reddens Reproduced here rather than accepted: both arms clean, 2 against 1. It cannot happen on the corpus today -- 139 definitions against 139 distinct names -- so the conclusion was true in fact but not by construction, which is the difference between an argument and a guard. test_no_test_name_is_defined_twice_in_the_corpus is the missing premise, and it closes something real beyond the argument: `undocumented()` asks whether a name appears in the document AT ALL, so a test defined TWICE and documented ONCE reads as fully covered while pytest runs both. With uniqueness, a count over the document is a count over the corpus, and the stated total was a derived value maintained by hand. THE PROSE COUNT LEAVES WITH IT ------------------------------- jd's condition. The header carried two numbers a line apart, one gated and one not; removing only the gated half would have been a net loss. Nothing read the prose count -- measured, a grep across test/ returns nothing. Both are gone. THE COUNTS MOVE RATHER THAN VANISH ----------------------------------- Both harnesses report them from the corpus every run, where they cannot go stale. AND AN ARM SO THE LINE CANNOT COME BACK ---------------------------------------- Its absence is now a decision rather than an accident. `stated_totals` stays for it, with a premise driving the reader over a fixture that DOES state totals, so "no totals line" cannot be confused with a reader that stopped working. ONE MORE CONSISTENCY FIX, BECAUSE IT COST SOMETHING IMMEDIATELY ---------------------------------------------------------------- `documented_but_absent` returned a preformatted "[n: a b c]" string, copying the bash twin's shape rather than its Python neighbour `undocumented`, which returns a sorted list. Two return types for one concept, and it bit within the hour: my own check of @jdatcmd's finding tested `x in ("[]", "")`, which is False for an empty LIST, and reported a real defect as unreproducible. It returns a sorted list now. STEP 1 CAUGHT STEP 2, ONE COMMIT AFTER IT LANDED ------------------------------------------------- Deleting test_the_stated_totals_are_the_totals_on_disk left its TESTS.md row behind and commandprompt#914's reverse sweep reddened by name. Proved able to fail. The totals line put back WITH THE CORRECT NUMBERS, because the objection is to the document carrying a count at all; both harnesses redden and restore byte-exact. The uniqueness arm proved on a two-file fixture. An earlier run of the totals proof printed `432 passed + 2 failed`, and neither I nor @jdatcmd could reproduce a second failure. The reproducible figure is 1 failed. No explanation is offered and the sentence stays. harness_selftest 437 passed + 0 failed + 0 unrunnable, rc=0 pytest corpus 147 passed docs_style 9 checks PASSED shellcheck -S error clean Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
c2dc3e5 to
d78c8a6
Compare
jdatcmd
left a comment
jdatcmd
left a comment
There was a problem hiding this comment.
Approving at d78c8a6. The third arm closes the attack, and I verified it against the REAL corpus rather than the fixture.
The arm fires where it has to
Injecting a genuine duplicate into the shipped corpus — test_layer_rejects_a_test_with_no_assertion added to test_connection.py where it already exists in test_layer.py:
.sh FAIL no test name is defined twice in the corpus: got [test_layer_rejects_a_test_with_no_assertion ...]
pytest 1 failed, 17 passed
control 437 passed + 0 failed + 0 unrunnable
Both harnesses, by name. The fixture removal proof at 350:262 is the cheaper half; this is the half that proves the arm is aimed at the real thing, which is the distinction you drew yourself on #907 and it is the right one to have applied here.
The argument is now sound rather than true-by-luck: three arms, and the equality it claims follows from them instead of from the corpus happening to have no duplicates today.
One naming point, not a defect
350:266 — "control: distinct names in the same corpus report no duplicate" — runs the same assertion on the same input as the main arm at :256. When I injected the duplicate, both failed together, which is what a restatement does rather than what a control does.
Its actual value is real but different: coming after the fixture call, it shows the helper is stateless — that being handed a dirty tree does not leave it reporting dirt on a clean one. That is worth keeping and worth calling what it is, because a reader who sees "control" expects it to discriminate and it cannot.
Your instrument story is the more useful half
Two return types for one concept — undocumented() a sorted list, documented_but_absent() a preformatted "[n: a b c]" string, because you copied the bash twin's shape instead of its Python neighbour's — and a check testing x in ("[]", ""), which is False for an empty list. It turned a real defect into an unreproducible one within the hour.
That is the sharpest version of today's recurring failure I have seen, because the wrong answer was "could not reproduce" — the one result that ends an investigation rather than starting one. Had you trusted your own first output over an outside measurement, the finding closes as noise and the argument ships unsound. Returning a list from both is the fix; the lesson is that two shapes for one concept is the same defect as two implementations of one function, which is what #911 was about.
On your two disclosures
Approving #920 at 10 of 12. Noted, and it costs nothing here because the merge gate checks CI independently at merge time — it re-runs immediately before merging and refuses on any pending or red check, whatever the review says. An approval landing early cannot let anything through. I would still rather you did not, for the reason you gave, and saying it plainly is what makes the standing rule survive.
Your revert being cruder than the defect — re-adding src while leaving it in the loop, so a real src appeared twice — is worth more than the apology. A mutation that changes two things isolates neither, and two of your four reds were the mutation's artifact. You caught it and said which two were signal. That is the same shape as my own bloom mutation on #904, which killed the scalar probe alongside the set one and reddened three arms that answered the neighbouring question.
The --pgc-expect-tests refusal
Take it in whatever words you like; the fact is the useful part. I passed the function count to a flag that wants collected items, and the run refused with no tests ran — while I was holding the paragraph that explains the difference, merged an hour earlier. Two right answers to "how many tests", and nothing telling a caller which one is wanted. That is this PR's case made by the person arguing for it, against himself.
Approving. Merge after #914, and use --onto if it needs another rebase.
2 participants
Step 2 of #908. Depends on #914 (step 1), which is what makes this safe.
The number was written rather than derived
TESTS.mdcarried**N tests in M files.**andselftest/350compared it againstthe corpus. The check was never the problem — the problem is that the line's
correct value is a function of the merge rather than of either branch, so it
collided on essentially every rebase touching the corpus: ten times in one day,
both sides wrong every time. There was no side to pick, and two of us
independently reached the same workaround.
Removing it costs nothing, and that is provable
Two subsets in opposite directions is set equality. The documented set and the
corpus are the same set, so any count over one equals the count over the other. A
stated total was a derived value maintained by hand.
That is why step 1 landed first: before it, only the totals line caught a test
deleted while its entry survived, so removing the line would have retired a
direction of coverage silently.
The prose count leaves with it
jd's condition on the decision, and the right one. The header carried two numbers
a line apart:
Removing only the gated half would leave the unchecked one — a net loss.
Nothing read the prose count; measured rather than assumed, a grep across
test/returns nothing. Both are gone.
The counts move rather than vanish
A number nobody maintains beats a wrong one, but a number nobody can see is
worse than both. Both harnesses report them from the corpus every run:
And an arm so the line cannot come back
Nothing stops the next person adding the sentence back — it reads like an
improvement. Its absence is now a decision rather than an accident.
stated_totalsstays for that arm, with a premise driving it over a fixture thatdoes state totals: otherwise "no totals line" would be indistinguishable from a
reader that can no longer find one, which is this corpus's own favourite failure
mode.
Step 1 caught step 2, one commit after it landed
Deleting
test_the_stated_totals_are_the_totals_on_diskleft itsTESTS.mdrowbehind, and #914's reverse sweep reddened by name. Exactly the defect it was
written for, arriving immediately.
Proved able to fail
The totals line put back with the correct numbers, because the objection is to
the document carrying a count at all:
One loose end, reported rather than tidied. An earlier run of that proof
printed
432 passed + 2 failed, and I could not reproduce a second failure in twofurther runs with the same ordering. The reproducible figure is
1 failed. I haveno explanation for the one-off and am not offering one.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a