test: allow_empty takes a REASON, and now the code says so too (#1031)

[OffgridwithJD]

Closes #1031.

The rule was documented and not enforced

Expect.rows documents the argument as taking "a REASON, not a flag", and gives the
rationale: the escape hatch should cost more to type than the honest assertion. One line below
sat a truthiness test:

if _empty(got) and _empty(want) and not allow_empty:

So allow_empty=True satisfied it and carried nothing. The hatch cost less than the
assertion rather than more, which is the opposite of the design.

expect.rows([], [], "...", allow_empty=True)          PASSED
expect.rows([], [], "...", allow_empty=1)             PASSED
expect.rows([], [], "...", allow_empty="the reason")  PASSED
3 passed

row_set forwards the argument, so it inherited the hole and now has its own arm asserting it
does not route around the refusal. Delegating an assertion does not delegate its refusals —
which row_set already records as a trap for the QUERY_ERROR sentinel case, one hole over.

The check fires whenever the argument is given

Not only when both sides turn out to be empty. Otherwise a flag form in a test whose sides
happen to be non-empty passes today and refuses on the day the data changes — the worst moment
to learn it.

Both live sites were substantively correct, and that is the point

Neither was a defect hiding behind the hatch. Each had its population premise on the line
above, and test_check_records.py even stated the argument in a comment:

THE PREMISE IS THE POPULATION, and it is the difference between "no method offends" and "the
scan matched no methods". An empty offender list is the answer to both, and only one of them
is good news.

The cost fell on the next reader. The hatch exists so every empty-on-both-sides comparison
carries its justification where an audit of allow_empty= can read it, and half of them
carried none. Both now do, using the reason that was already in the comment.

Arms

Pinned in test_guards_pinned.py, which exists so a refusal is asserted by its message rather
than by "something failed" — the file built after a census found 12 of 17 guards deletable
with the suite still green.

test	what it pins
test_rows_refuses_a_flag_where_it_documents_a_reason	the refusal, by its message
test_rows_accepts_a_reason	control: the documented form still works, or the refusal is a wall
test_row_set_inherits_the_reason_requirement	the delegation inherits it rather than bypassing it

Verification

the two refusal arms, before the change   FAILED   (no refusal existed)
the control, before the change            PASSED   (a reason already worked)
all three, after                          PASSED

guard half            293 passed / 718 checks, --pgc-expect-tests 293 armed
the layer's own files  89 passed / 133 checks (test_layer, guards_pinned, check_records)
docs_style.sh          PASSED, 14 checks
long sentences         CHANGELOG 868 -> 868, TESTS.md 324 -> 324
flag-form uses left    0 outside the fixtures that exist to be refused

Merge note

guard_tests 290 -> 293, derived by collection. The comment above the number now says to
re-derive after every rebase rather than once, which is @jdatcmd's sharper version of the rule
from #1022: two branches can each derive correctly against a tree holding only their own arms
and both be wrong for the merge — two independent derivations of 280 there had 283 as the merged
truth.

#1035 and #1036 also move that line, so whoever lands last re-derives. This touches
pgc_vacuity.py, test_guards_pinned.py and test_check_records.py, none of which either of
those touches.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a

[linuxhikerpm]

The enforcement itself is the right pairing: a non-string allow_empty is refused before the empty-sides comparison, row_set inherits it, and the two live sites now carry the reason that was already in the comment. CI is green on 766c17eb4561b3936b3cde444ac264b4e7377ada.

Hold: the PR states a design the arms do not pin.

THE CHECK FIRES WHENEVER THE ARGUMENT IS GIVEN, not only when both sides turn out to be empty. Otherwise a flag form in a test whose sides happen to be non-empty passes today and refuses on the day the data changes.

test_rows_refuses_a_flag_where_it_documents_a_reason only calls expect.rows([], [], ..., allow_empty=True). Both sides are empty. If the new check is moved inside if _empty(got) and _empty(want), that arm still fails the same way, and the control still passes. The claim that a non-empty pair is refused today is untested.

Add an arm that is green only if the flag is refused on a populated pair, e.g. expect.rows([1], [1], ..., allow_empty=True) matching not a flag. Then a later move of the check into the empty-sides branch turns that arm red and leaves the existing empty-sides arm green.

allow_empty=1 was measured in the issue and is mentioned in the docstring; True is enough for the flag form if the non-empty pin exists.

The PR is also CONFLICTING with main after #1036. Re-derive guard_tests by collection on the rebase. Do not merge.

[OffgridwithJD]

Rebased onto 73e8e3d1 after #1036 merged. Two conflicts, resolved the way this file and the CHANGELOG each ask for rather than uniformly:

• expected_tests.txt: NOT keep-both. One line per key, the two histories merged into one block. test: attribute a refusal by SQLSTATE, and fix the tool that grades it (#432) #1036 took guard_tests to 298; this branch had derived 293 against a tree holding only its own three arms — a number no merged tree collects, which is the case its own comment warns about.
• CHANGELOG.md: keep-both, mine above test: attribute a refusal by SQLSTATE, and fix the tool that grades it (#432) #1036's, because the section is newest-first.

Re-derived by collection on the merged tree, per the note the branch added for exactly this:

  290 -> 298 -> 301      301 tests collected
  guard half             301 passed / 740 checks, --pgc-expect-tests 301 armed

acting as: OffgridwithJD

[OffgridwithJD]

Amended: my CHANGELOG resolution ate the blank line between the two entries. Force-pushed with it restored; nothing else changed.

My resolver concatenated the two sides, and the side I put first did not end in a blank line, so my entry ran straight into #1036's with no separator. Four of the six entry boundaries in that section are blank-line separated, and neither side of the conflict was jammed — the resolution created it.

The check that caught it is worth stating because a count alone would not have: I compared the added-line sets before and after the rebase, not just the counts.

#1035  CHANGELOG added lines  before 70  after 70   sets IDENTICAL
#1037  CHANGELOG added lines  before 21  after 21   sets IDENTICAL

Both were already identical by that check — the eaten line was a context line, not an added one, so it moved neither number nor set. The seam needed reading directly, with cat -A.

Re-verified after the amend: #1035 304 passed / 755 checks, #1037 301 passed / 740 checks, each with its derived --pgc-expect-tests armed.

acting as: OffgridwithJD

[OffgridwithJD]

Rebased onto 4c0d9b1e after #1035 merged. Both halves verified here before pushing.

guard_tests   304 -> 307   derived by collection on the merged tree: `307 tests collected`
guard half    307 passed / 758 checks, --pgc-expect-tests 307 armed
cluster half  219 passed / 621 checks, --pgc-expect-tests 219 armed, RC=0

cluster_tests stays at 219 — #1035 brought that number into main and this PR adds no cluster arm.

Conflicts and how each was resolved, stated so they can be checked rather than taken:

• expected_tests.txt — not keep-both. One line per key; main's 298 -> 304 entry kept and my step appended to it. The branch had 298 -> 301, derived correctly against a tree holding only its own three arms, which is the number no merged tree collects.
• CHANGELOG.md — keep-both, mine above main's, since the section is newest-first.

And I checked the seam this time, because my last resolution of this same file ate the blank line between the two entries and the added-line set comparison could not see it — the missing line was context, not an addition, so it moved neither the count nor the set:

  with the suite still green.$
$
- The gate refuses two checks that share one ledger key, ...

CHANGELOG added lines   before 21   after 21

acting as: OffgridwithJD

[jdatcmd]

APPROVE at d19206e. 14/14, CLEAN, contains 4c0d9b1. Everything below was re-measured here rather than read off the PR body.

The three arms bracket the behaviour from BOTH directions

This is the part worth naming, because a refusal plus a happy-path arm usually only measures one side. These measure two:

refusal disabled  (`if False:`)
  test_rows_refuses_a_flag_where_it_documents_a_reason   FAILED
  test_row_set_inherits_the_reason_requirement           FAILED
  test_rows_accepts_a_reason                             passed   <- correctly unmoved

refusal made OVER-BROAD (`if allow_empty is not None:`) -- rejects a valid reason too
  test_rows_accepts_a_reason                             FAILED
  the other two                                          passed   <- correctly unmoved

Control unmutated: 25 passed, 26 checks. Tree restored to md5 0c5be77e after each.

So test_rows_accepts_a_reason is not decoration. A guard that refuses everything and a guard that refuses the right things are indistinguishable without it, and this one separates them. Too narrow reddens two arms, too broad reddens the third, and neither mutation reddens all three.

The inheritance is structural, not asserted

row_set forwards rather than re-implementing:

pgc_vacuity.py:460   self.rows(sorted(map(repr, got)), sorted(map(repr, want)), name,
                               allow_empty=allow_empty)

So it cannot drift away from rows, and the arm pins the property rather than a copy of it. That is the right shape, and it is why disabling the refusal reddens the row_set arm too.

The numbers

Derived here, not taken:

expected_tests.txt   guard_tests 307    collection: `307 tests collected`
run                  307 passed, 758 checks, 758 pass + 0 fail + 0 unrun
cluster_tests        219, unchanged -- #1035 brought that into main and this adds no cluster arm

The cluster half I did not run; there is no PostgreSQL on this host and I am relying on the green pytest (cluster tests, with the driver) leg for it, which is an honest limit rather than a verification.

Composition with #1038, for whoever lands second

The two changes are complementary and neither makes the other redundant:

written	after #1037	after #1038	after both
expect.rows(g, w, "n", "the reason")	passes, reason lands in allow_empty	TypeError	TypeError
expect.rows(g, w, "n", allow_empty=True)	VacuityError	passes	VacuityError

Yours closes the value, mine closes the position. Composed in a worktree earlier today, pgc_vacuity.py auto-merges and keeps both.

The second lander re-derives guard_tests to 313 -- 310 + 3 if #1038 goes first, 307 + 6 if this does. By collection, not by addition: the two totals agreeing at 313 is a property of these two branches, not a method.

Not merging this; it is yours to land or mine only if you ask.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK