test/pytest: every counted assertion is a record, and the count is derived from them (#937)

[jdatcmd]

First phase of #937. Self-contained, and deliberately not the whole issue.

The property, and why it is not a port of the shell's

The shell harness makes pgc_record count and record in one call, so no path can do either alone, then reconciles the totals. Copying that format into pytest would be the harness-independence violation that killed #923's file. The property is what has to be parallel, not the implementation.

Python can reach it more strongly, because it can remove the possibility rather than police it:

@property
def count(self):
    return len(self._records)

A count with no setter cannot drift from the stream it counts. The shell needs a reconciliation because in bash those must be two variables; here they need not be.

What was there before

_counted() call sites                  15
readers of the counter                  1   (pytest_runtest_call, `rec.count == 0`)
per-assertion records                   0

The verdict is not passed in, and that is the design decision

_counted() is called before the comparison at all 15 sites, so the call site does not know the outcome. Passing a verdict would split one operation back into two — the exact shape this removes.

It does not need to. Assertions in a body are sequential and a raise ends the test, so a failed test's failing assertion is the last record and every earlier one passed. Resolving the verdict from the exception is phase 2; every refusal message is byte-identical here, which matters because test_guards_pinned.py pins them.

Two outcomes are settled now because they are call-site facts rather than exception facts:

• cannot_run() records UNRUN, not a pass — an assertion that declined to run is an outcome like any other.
• A refused assertion records nothing: a VacuityError means the assertion never ran, so the stream is outcomes and not a log of attempts. That one matters for phase 5 — a refused assertion leaving a record would make the totals disagree with what the run reported.

Removal proofs

MUTATION                                        RESULT
the record is never appended                    228 red -- the entire corpus
a faithful, writable second counter               1 red, and only one

The second is the one that matters. It is not a broken implementation: it is a real self.count initialised to 0 and incremented in _record, kept perfectly in step. Every arm accepts it except test_the_count_cannot_be_moved_without_a_record, which is the point — an arm checking only that the count agrees with the records would pass on exactly that, and the agreement would then be maintained rather than structural.

The arm that cannot fail today, and why it is in

test_a_name_carrying_a_separator_survives_the_record puts a tab and a newline in a check name. There is no separator in an object, so it cannot fail against this implementation.

It is in because the shell measured what happens when there is one: a tabbed name gave a record of four fields, a newlined name gave two lines. The moment somebody formats these records into a line that class returns, and without this arm nothing would say so. Stated in TESTS.md as well, so it does not read as redundant to whoever prunes next.

Gate

pytest, driver-free (the CI job's file list)   228 passed
pytest, full corpus, live PG16 cluster         326 passed
harness_selftest.sh, PG16                      803 passed, 0 failed, 0 unrunnable
docs_style.sh                                  9 checks, PASSED

No ledger or budget change: the ledger's rows are shell suites, and this adds no shell check.

test_check_records.py is declared in NO_CLUSTER and the classifier in test_harness_deps.py decides that membership independently — the fifth time that arm has settled a membership rather than been told one.

Relationship to #964

#964 (for #924) adds a binding snapshot taken from globals(), so it picks up _Record and _record automatically when both land; there is no coordination needed and no textual overlap. This branch is off main, not off #964.

What is not here

Phases 2 through 5: the verdict resolved from the exception, and a session reconciliation that can fail. The issue is explicit that a reconciliation which cannot fail has been shipped twice on the shell side, so that arm gets attacked before it is believed rather than landing quietly beside this.

🤖 Generated with Claude Code

https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw

[OffgridwithJD]

Attacked both claims as asked. The construction claim survives — I tried to break it and could not. The phase-2 argument has a hole, and there is a live counter-example in the corpus.

Attack 1: I could not find a third implementation

You invited one that passes all six arms and still lets the count drift. My candidate was the one that attacks the invariant rather than the counter: leave count as an unwritable property over len(self._records), and make _record append twice. The count then cannot be written and still drifts from "number of counted assertions".

six arms, unmutated                           6 passed
six arms, _record appends twice               5 failed, 1 passed
whole corpus, _record appends twice           6 failed, 320 passed

test_each_counted_assertion_appends_exactly_one_record is exactly the arm that catches it, and four more go with it. So the invariant is pinned, not just the counter, and your claim is stronger than I expected — "a count that cannot be written cannot drift" holds because a second arm pins the one-record-per-assertion relation that the property rests on. I withdraw the doubt.

Attack 2: the phase-2 argument has a hole, and it is already in the corpus

Your argument:

assertions in a body are sequential and a raise ends the test, so a failed test's failing assertion is the LAST record and every earlier one passed

A raise ends the test only if nothing catches it. The corpus catches it deliberately, in five places, because that is how you prove a guard refuses:

test_ordered.py:243                 except AssertionError:  -> records a pass, test continues
test_failed_query_sentinel.py:236, :326, :357, :382

test_ordered.py:241 is the clearest:

try:
    expect.ordered_rows(FWD, REV, "ordered oracle sees order")   # records, then RAISES
except AssertionError:
    expect.num(1, 1, "the ordered oracle refused the reversed sequence")

and the outer arm asserts that inner test passes (passed=2).

Driven on your branch, the record stream for that shape:

count before/mid/after: 0 / 1 / 2
  record 0: name='this comparison must fail'   verdict='PASS'    <- this one RAISED
  record 1: name='and the test continues'      verdict='PASS'
1 passed

The failing assertion is record 0, the test passed, and no exception ever reaches pytest_runtest_call. So phase 2 cannot correct it: it resolves verdicts from the exception, and there is no exception. A genuinely failed assertion stays recorded as PASS, permanently, in a passing test.

That is worse for phase 2 than "the last record is not the failing one", which at least leaves something to correct. And it is not an exotic shape — it is how this corpus tests its own guards, five times.

It does not touch #966. Phase 1's claim is that count == len(_records) and cannot be written, and that is untouched: the count above is 2 and correct. Only the verdict is wrong, and phase 1 does not claim verdicts. So this is a finding for the next PR rather than a change wanted here.

If it helps, the two shapes I would think about for phase 2: record the verdict at the point the comparison returns or raises (which you rejected, correctly, as splitting one operation into two), or have the comparison itself mark its own record on the way out — the finally of the comparison rather than the caller, which keeps it one operation and does not need the exception to propagate.

The arm you flagged as unable to fail

The tab-and-newline round-trip. Agreed it cannot fail today and I am glad it is labelled in both the PR and TESTS.md rather than counted as coverage. It is the right kind of arm to keep: it costs nothing and fires the moment somebody formats these into a line, which is precisely when the shell's four-fields-for-a-tab measurement becomes relevant again.

Gate on my box

test_check_records.py    6 passed
full corpus            326 passed
driver-free            228 passed

Matching your numbers. Five files, pytest-only, no ledger — checked against real main 9cfa32d8 this time, having learned that lesson twice today.

CI is early (2 of 13 reported). I will approve when all 13 are in, and the approval is for phase 1 on its own terms; the phase-2 hole above is not a reason to hold it.

[OffgridwithJD]

Head moved c223bf61 -> 3a3d1e68 for the comment correction, so I checked whether my review still applies rather than assuming it.

It is docstring-only, verified by comparing the code itself rather than the diff. Parsed both versions, stripped every docstring, and hashed the resulting AST:

c223bf61 code skeleton: b09ce3e33dfa
3a3d1e68 code skeleton: b09ce3e33dfa      IDENTICAL
files changed:          test/pytest/pgc_vacuity.py only

So the executable content is unchanged and my review of c223bf61 carries on evidence. I mention the method because "the diff only shows comments" is a weaker claim than "the parsed code is the same object" — a stray change inside a long docstring edit is exactly the thing a diff invites you to skim past.

On the correction itself

This is the right repair, and it is better than what I suggested. I offered the hole; you wrote down the measurement, the five sites that make it true, and the consequence — a genuinely failed assertion stays PASS, in a passing test, and nothing reaches pytest_runtest_call to correct it. Stating that in the docstring of the function that will have to fix it is worth more than an issue comment, because the next person to implement phase 2 reads the former and may not find the latter.

And you kept the part that matters for this PR intact: what IS claimed here is the count, which the probe above shows is 2 and correct. The count was never in question — my attack on it failed outright, with 5 of 6 arms catching the double-append.

One note on the conclusion you drew: the verdict therefore has to be set on the comparison's own path, where the outcome is known and no propagation is needed. I agree, and I would add the reason it survives the attack I used on the counter — a finally in the comparison records the outcome whether or not the exception escapes, so catching it in a test no longer hides the verdict. That is the property the exception-based version lacked.

CI restarted on the new head and is still running. I will approve once 13 report, with the head re-checked.

[jdatcmd]

@OffgridwithJD's attack 2 landed, and I have pushed the correction rather than arguing with it. 3a3d1e6.

What was wrong

_record's comment argued that a later phase could resolve each verdict from the exception, because assertions are sequential and a raise ends the test, so the failing assertion would be the last record.

False here, and this corpus is what makes it false: proving a guard refuses means catching the AssertionError, which five tests do — test_ordered.py:243, test_failed_query_sentinel.py:236, :326, :357, :382. Reproduced on the branch rather than taken on report:

count before/mid/after: 0 / 1 / 2
  record 0  'this comparison must fail'   verdict PASS   <- this one RAISED
  record 1  'and the test continues'      verdict PASS
1 passed

A genuinely failed assertion stays PASS, in a passing test, and nothing reaches pytest_runtest_call to correct it.

Nothing this PR claims is affected — the count above is 2 and correct, and phase 1 claims the count and its underivability, not verdicts. But a comment arguing for a guarantee the code does not provide is worse than no comment, because it stops the next person checking. That is the reason pgc_write_source_stamp carries the same warning, and it applied to me.

The comment now carries the measurement and says the verdict has to be set on the comparison's own path — @OffgridwithJD's suggestion, and the right one, since it needs no propagation.

Comment only; 228 driver-free still green.

The merged tree, not the branch

main moved to 3f0aa0f (#964). GitHub says MERGEABLE, but two PRs can each be green and merge-clean without composing, so I composed them locally (37e211a):

driver-free corpus, composed          234 passed   (228 from main + 6 here)
_Record present                       True
binding guard present                 True

And the interaction is better than either PR claimed on its own. #964 said names added later are covered without being listed anywhere; _Record is a name added later, by a different PR, and it is covered:

pgc_vacuity._Record = None      REFUSED rc=4, naming _Record

That is #964's central claim demonstrated by something written after it, which is stronger than the arm it shipped with.

And composing them found a defect neither PR has — #967

Checking whether #964's snapshot covered _record showed that it does not, because _record is a method, not a module binding. Following that:

pgc_vacuity.Expect.num = lambda self, got, want, name: self._record(name)

makes a test asserting 1 == 2 print 1 passed. Strictly worse than #924 — those hatches switched off meta-rules; this switches off the assertion itself.

It is not a defect in this PR or in #964. #964 catches pgc_vacuity.Expect = <subclass> wholesale; what it cannot see is a mutation of the object a binding points at. Third instance of one pattern — datum, then reader, then attribute-of-a-bound-object — each fix moving the hatch one frame out. Filed with the surface (18 methods) and a probe, and deliberately without a proposed fix, because "document where the floor is" is genuinely one of the options.

[OffgridwithJD]

Approving at 3a3d1e68 on full green CI (13 of 13).

Phase 1 survived the attack I was invited to make. My candidate went after the invariant rather than the counter — leave count an unwritable property over len(self._records), make _record append twice, so the count still cannot be written and still drifts from "number of counted assertions":

six arms, unmutated                      6 passed
six arms, _record appends twice          5 failed
whole corpus, _record appends twice      6 failed, 320 passed

test_each_counted_assertion_appends_exactly_one_record catches it directly. So the property rests on a pinned relation rather than on itself, and the construction claim is stronger than I expected. I withdraw the doubt, and I would not have known which arm was carrying that weight without trying to break it.

The phase-2 hole I found is fixed in the right place. Not the code — phase 1 never claimed verdicts — but the docstring of the function that will have to implement it, now carrying the measurement, the five catch sites, and the consequence: a genuinely failed assertion stays PASS, in a passing test, and nothing reaches pytest_runtest_call to correct it. That is better than my issue comment would have been, because whoever writes phase 2 reads the former.

The head moved during review and I checked rather than assumed. Parsed both versions, stripped every docstring, hashed the AST:

c223bf61 code skeleton: b09ce3e33dfa
3a3d1e68 code skeleton: b09ce3e33dfa     IDENTICAL, one file

"The diff only shows comments" is a weaker claim than "the parsed code is the same object".

Gate on my box: test_check_records.py 6 passed, full corpus 326, driver-free 228 — matching yours. Five files, pytest-only, no ledger, checked against real main.

Two notes, neither blocking:

The tab-and-newline arm cannot fail today and is labelled as such in both the PR and TESTS.md. Right call — it costs nothing and fires the moment these are formatted into a line, which is when the shell's four-fields-for-a-tab measurement becomes relevant again.

And #967 does not bear on landing this. I measured the Expect.num hatch on main at 3f0aa0f6 with a stub written against main's own API: 1 passed there too. #966 changes the name the stub has to call and nothing else about the hatch, so there is nothing to hold for.