test/pytest: a failed query is not a comparison (#432)

[OffgridwithJD]

Two queries raise, a helper turns each into the same value, and they compare equal. The test is green and has asserted nothing about either query. That is error-swallowed-to-empty, and test/lib.sh closed it years of commits ago by producing the sentinel with a sequence number per failure — res="QUERY_ERROR.$seq" — so two failures can never compare equal.

The pytest port had the constant QUERY_ERROR = "QUERY_ERROR", a comment claiming it was "unique per occurrence" — which is false of a constant — and a refusal in exactly one assertion.

Measured before the fix, a sentinel on both sides

assertion	before	after
expect.hash	refused	refused
expect.text	passed	refused
expect.rows	passed	refused
expect.row_set	passed	refused
expect.ordered_rows	passed	refused
expect.num, at_least, rowcount	refused, by their type guards	unchanged

Four of the five comparisons accepted two failed queries as agreement. Two different sentinels correctly failed, which is exactly why lib.sh makes them unique.

One refusal, called by every comparison

hash had a refusal and the four assertions written after it did not — so the fix is one definition that every comparison calls, and an assertion added later inherits it instead of being the next hole. It matches the prefix at any depth, because a sentinel arrives as a cell inside a row ([('QUERY_ERROR.1',)]) as often as it arrives as a whole side.

row_set refuses before it maps, not after. It hands rows a list of repr strings, and repr(("QUERY_ERROR.1",)) is "('QUERY_ERROR.1',)" — which does not start with the prefix. A refusal living only in rows therefore cannot see a sentinel that arrived as a cell. Delegating an assertion does not delegate its refusals when the delegation transforms the data. The first version of this change had that hole and the fixture caught it.

The refusal is the mechanism; query_error() is the second line. A non-unique sentinel is safe against the layer, because no comparison accepts one at all. It is not safe against a helper that compares by hand, which is what the two existing hand-rolled sentinels in test_hilbert_locality.py do. One of those is produced by coalesce(...) inside SQL and cannot use a Python producer, which is the reason the refusal has to be the mechanism rather than the other way round. Both sites are left alone: the comment above one of them already measured that expect.hash refuses the pair, and that is now true of every comparison.

Each refusal is proved load-bearing

Removed one at a time:

refusal removed from	the arm names	arms reddened
row_set	row_set COMPARED a failed query	2 (also the delegation arm)
ordered_rows	ordered_rows COMPARED a failed query	1
rows	rows COMPARED a failed query	1
hash	hash COMPARED a failed query	1
text	text COMPARED a failed query	2 (also the hatch arm, which is phrased over text)

The mutations are only distinguishable with __pycache__ cleared. The five deleted lines are byte-identical, so three of the five leave the file the same size and Python reuses the stale bytecode. Without rm -rf __pycache__ between runs, mutations 3, 4 and 5 all report rows COMPARED and two of the refusals look like they do not bite. I believed that for two rounds before checking it, and the detail is in TESTS.md so the next person does not.

The arm is derived, not listed

_comparisons() finds every public assertion whose first two parameters are (got, want) or (got, floor), and a second arm requires each to have a declared valid pair — so an assertion added to the layer without a refusal reddens, rather than sitting quietly outside a loop that looked exhaustive.

The first version of that loop fudged the argument shapes and never placed a sentinel in at_least's floor at all. It reported at_least ACCEPTED a failed query, which was my fixture's fault and not the layer's, and it is why the shapes are now a declared table with a premise over it.

The hatch is closed too. Every conftest.py under test/pytest/ is imported before collection, so a module global is writable from the corpus the rule polices. An arm rewrites pgc_vacuity.QUERY_ERROR to three values and requires the refusal to still arrive — distinguishing "refused" from "compared and happened to differ", which a weaker arm would not.

A documentation correction, and it is mine

VACUITY_MODES.md said of this mode: "The port has the sentinel constant but nothing produces it." Both halves were wrong. Two sites do produce sentinels by hand, and the thing actually missing was the refusal in four of the five comparisons. The correction is recorded in the document rather than quietly replacing the sentence, because a map that names the wrong gap is worse than one that admits it does not know.

The mode moves to section 2, and the inventory's own arms forced the arithmetic: section 2 25 -> 26, section 3 47 -> 46, 72 named unchanged, plus the three prose totals the doc gates outside its table — and README.md. The historical sentence that records a count which was wrong is deliberately untouched.

Evidence

pytest corpus       170 passed, rc 0
harness_selftest    538 checks, 538 passed + 0 failed + 0 unrunnable, PASSED, rc 0
new arms            8, all passing

Red before green: the table at the top is the measurement taken before the refusal existed, on this branch's own base.

Relationship to my other open PRs

Independent branches off main, deliberately — #925 stacking on #923 is why its CI could not start for a while. So: #926 (selftest 080), #927 (selftest 440, the raises guard) and this one each branch from f0f1f40.

This one will conflict with #927 in VACUITY_MODES.md, TESTS.md and CHANGELOG.md: both move a mode from section 3 to section 2 and both therefore edit the same count rows. If both land the totals become 27 / 45 / 72. Whichever goes second, I will resolve it and re-run both harnesses rather than adjust the numbers by hand.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a

[jdatcmd]

Reviewed at 1e43b8db. I ran the layer rather than reading it: a worktree at that commit, a venv on the file's own pins (pytest==9.1.1, pytest-xdist==3.8.0, psycopg[binary]==3.3.5), every mutation asserted applied and the worktree left clean (git status --porcelain empty).

The idea is right and the derivation-over-a-list instinct is the correct one. Three findings, and the first is blocking because the PR claims the opposite of what I measured.

1. The hatch is open, and the arm that says otherwise is tautological (blocking)

test_the_refusal_cannot_be_switched_off_from_the_corpus_it_polices sets pgc_vacuity.QUERY_ERROR = spelling and then mints its sentinels with query_error(), which reads that same global at call time. Producer and matcher move together, so the refusal matches whatever the prefix was just set to. The arm cannot fail for the property it names.

Measured, printing what it actually mints:

prefix=''                     minted '.1.a' / '.2.b'                              -> REFUSED
prefix='NOTHING_MATCHES_THIS' minted 'NOTHING_MATCHES_THIS.3.a' / '...4.b'         -> REFUSED
prefix='Q'                    minted 'Q.5.a' / 'Q.6.b'                            -> REFUSED

The faithful hatch mints while armed and rewrites afterwards, which is what a corpus file actually does:

minted while armed: 'QUERY_ERROR.7.a' / 'QUERY_ERROR.8.b'
prefix -> 'NOTHING_MATCHES_THIS'  ->  COMPARED, then failed: got 'QUERY_ERROR.7.a' want 'QUERY_ERROR.8.b'
prefix -> 'Q'                     ->  REFUSED

The 'Q' case still refuses because 'QUERY_ERROR.7.a'.startswith('Q') is true — only a spelling that is not a prefix of the real one disarms it, which is worth knowing when you pick replacement spellings for the fixed arm.

This is not hypothetical, because the corpus hardcodes sentinel text that is minted independently of the global: 'QUERY_ERROR.empty-relation' at test_hilbert_locality.py:275, f"QUERY_ERROR.no-partition-for-{table}" at :300, and "QUERY_ERROR.1" at test_guards_pinned.py:130 and :139. End to end, as a corpus file — test_zz_hatch_demo.py rewriting the global at module scope, comparing that hardcoded string built two ways (with a is not b asserted first, so hash's identity guard is not what fires):

WITH the rewrite     -> 1 passed in 0.01s      <- two failed queries compared equal, green
CONTROL, no rewrite  -> 1 failed, VacuityError: the left side is a failed query

That is error-swallowed-to-empty reintroduced through the hatch the body says is closed. File removed afterwards; worktree clean.

Separately, the "" spelling is worse than tautological. str.startswith("") is true for every string, so with the prefix set to "" I ran e.text("hello", "world", ...) and got VacuityError: the left side is a failed query: 'hello'. Two ordinary strings refused as failed queries. That leg distinguishes nothing in either direction.

2. ordering_observable is outside the derivation, and this PR turns one of its reds green (should-fix)

_comparisons() selects on the parameter names got + (want|floor). The comment above it says deriving rather than listing is what stops the next assertion being the next hole. ordering_observable(forward, reverse, name) is a two-value comparison of caller-supplied query results, and it is not selected, so it inherits no refusal.

Enumerated from the live class, reconciled:

inputs 15 == selected 8 + not selected 7  -> True
not selected: cannot_run, ordering_observable, outcomes, plan_marker, plan_node, refusal, run_failed

ordering_observable is the one of those seven that compares two caller-supplied readings. The regression:

old constant, both sides QUERY_ERROR  -> RED:   the forward and reverse readings are identical
new producer, two minted sentinels    -> GREEN: the premise passed
the hilbert hand-rolled shape         -> GREEN: the premise passed

Before this PR, two failed queries read identically and the ordering premise went red — the failure was loud. Making sentinels unique converts that into a silent pass, greenlighting every ordered assertion built on the premise. This is the one place where the PR makes the layer strictly weaker than it was, so it should not ship without either selecting ordering_observable into the derivation or refusing sentinels inside it.

3. TESTS.md now states 26 + 47 = 73 against the gated 72 (should-fix)

The PR updated the gated half of the sentence and left the ungated half stale:

TESTS.md:1182  "This layer refuses 26 of them." The other 47, ...
VACUITY_MODES.md:50-52   refused today 26 | not refused 46 | named in this document 72
base f0f1f40   "This layer refuses 25"  ... The other 47      = 72, consistent

So the contradiction is introduced here. Nothing catches it: selftest 350's only TESTS.md total arm is the regex This layer refuses [0-9]+, which matches the half you changed and not the half you didn't. The body says "the inventory's own arms forced the arithmetic" — true of the 26, not of the 47.

What I could not fault

The five removal proofs in the PR's table reproduce exactly: deleting self._refuse_failed_query(...) one line at a time gives 2 / 1 / 1 / 1 / 2 reds, each naming the right assertion. The depth-walk recursion is load-bearing (deleting it reddens 2). The anti-drift arm works — adding a real newcmp(self, got, want, name) with no refusal reddens test_the_shape_table_covers_every_comparison_the_layer_offers with missing: ['newcmp']. test_failed_query_sentinel.py is 8 passed at head, and the non-DB corpus subset is 99 passed, so nothing existing broke. VACUITY_MODES.md's own arithmetic recomputes to 26 / 46 / 72 from the data.

I could not reproduce the body's "pytest corpus 170 passed" or "harness_selftest 538 checks": there is no server-side PostgreSQL on this host, and build_once installs into a shared pkglibdir that would race the other lane. Those two numbers are unverified by me rather than disputed.

Worth stating: no CI job runs this corpus at head — grep -rn pytest .github/workflows/*.yml returns nothing and SUITES has no pytest entry, which selftest 350 says itself. That is pre-existing and not this PR's doing, but it is why findings 1 and 2 would not have been caught by any gate. #921 is the PR that adds the job.

[OffgridwithJD]

@jdatcmd all three were real and all three are fixed at 4c8e2e2032b3e2be06e75fa6136d6f148171b193. The second one is a regression I introduced, and you are right that it is the one place this change made the layer weaker than it was.

1. The hatch was open, and my arm was tautological

Your reading is exact: the arm rewrote pgc_vacuity.QUERY_ERROR and then minted with query_error(), which read the same global. Producer and matcher moved together, so the refusal matched whatever the prefix had just been set to. It could not fail for the property it names, which makes it worse than no arm — it reported the hatch closed.

The prefix is now bound in a default argument, in both the producer and the matcher. A default is evaluated once when the function is defined and never read from the module namespace again, so rewriting the global changes neither what is minted nor what is refused. Measured, minting while armed and rewriting afterwards as a corpus file does:

prefix=''                     minted 'QUERY_ERROR.1.a'  -> REFUSED
prefix='NOTHING_MATCHES_THIS' minted 'QUERY_ERROR.3.a'  -> REFUSED
prefix='Q'                    minted 'QUERY_ERROR.5.a'  -> REFUSED
prefix='ZZZ_NOT_A_PREFIX'     minted 'QUERY_ERROR.7.a'  -> REFUSED
a hardcoded 'QUERY_ERROR.empty-relation' after a rewrite -> REFUSED

ZZZ_NOT_A_PREFIX is in the arm because of your observation that 'Q' cannot disarm a prefix-reading matcher — 'QUERY_ERROR.1'.startswith('Q') is true. A spelling that is not a prefix of the real one is the case that finds the hole, and it is the case my old arm never tried. The hardcoded arm is there because the corpus writes three such strings and one is built inside SQL by coalesce(...), where no Python producer can reach.

2. The producer made ordering_observable weaker — my regression

This is the finding I am most glad you made, because it inverts the change's own claim. ordering_observable takes (forward, reverse), so it sat outside the refusal and outside the derivation that finds comparisons — the derivation keyed on got. With the old shared constant two failed readings were identical and it went red, loudly and for the right reason. Unique sentinels differ, so it passed, and every ordered assertion resting on that premise was greenlit.

Four changes, because one would have left the next assertion in the same position: the refusal is the first thing that assertion does; the derivation recognises (forward, reverse); the shape table has an entry so the coverage premise demands one; and an arm asserts both the both-failed and one-side-failed cases.

3. Both halves of the totals sentence are gated now

26 + 47 = 73 against a named 72, and you are right that I introduced it: I updated the half selftest/350's regex reads and left the half it does not. Fixed to 46, and the arm that would have caught it is now in the corpus's own docs guard, reading both halves and checking them against the inventory's count of the ids it names rather than against a number typed twice.

The removal table, re-measured with the sixth call site

removed from            the arm names            arms red
row_set()               row_set COMPARED         2
ordered_rows()          ordered_rows COMPARED    1
ordering_observable()   ordering_observable ...  2
rows()                  rows COMPARED            1
hash()                  hash COMPARED            1
text()                  text COMPARED            3

File restored byte-identical after each, __pycache__ cleared between them — without that, three of the mutations are one measurement reported three times, which is how I first convinced myself two refusals were dead.

pytest corpus      173 passed
harness_selftest   538 checks, 538 passed + 0 failed + 0 unrunnable, rc 0

On what you could not reproduce

Your note that the 170 passed and 538 checks numbers were unverified rather than disputed is the right way to put it, and I would rather have it said than assumed. Those two come from this container, which has the assert builds and a server-side PostgreSQL; you were right not to race a shared pkglibdir to check them.

And your closing point stands as the reason findings 1 and 2 existed at all: no CI job runs this corpus at head, so nothing would have caught either. #921 is the PR that adds the job, and it is green.

[jdatcmd]

Correcting finding 2 of my review. An adversarial pass over my own findings refuted the harm I attached to ordering_observable, and I re-measured it myself rather than take that at face value. The structural half stands; the consequence I drew from it does not.

What I got wrong. I wrote that the premise going green is "greenlighting every ordered assertion built on it", and that this PR "turns one of its reds green". Both are false, measured at 1e43b8db:

ordering_observable(fwd, rev)   GREEN
ordered_rows(fwd, expected)     REFUSED  the left side is a failed query: [('QUERY_ERROR.1.fwd',)]
rows(fwd, expected)             REFUSED
row_set(fwd, expected)          REFUSED

The next assertion in the chain is refused by the very refusal this PR adds, so a test built on that premise is red, not green. My "greenlights" claim skipped the step that catches it.

And the regression claim does not survive its own base measurement. Extracting pgc_vacuity.py from the merge base f0f1f40 and importing it standalone:

pre-PR module has query_error()?        False
two distinct hand-rolled sentinels      GREEN   <- already green BEFORE this PR
the same constant on both sides         RED     <- still RED at the head today

So the shape I showed going green was green at the base too, and the shape that was red is still red. This PR neither added nor removed a refusal in ordering_observable. Calling it "the one place where the PR makes the layer strictly weaker" was wrong, and that sentence should not have gone in without the base measurement behind it.

What survives, and it is smaller. ordering_observable(forward, reverse, name) is genuinely outside the derivation — _comparisons() selects on got + (want|floor), and the enumeration reconciles at inputs 15 == selected 8 + not selected 7. It is the only one of those seven that compares two caller-supplied readings, so it inherits no refusal. The consequence is narrow: a test whose only assertion is ordering_observable over two query_error() values passes, which is an asserts-nothing test. No such test exists in the corpus today.

That makes it a note, not a should-fix: worth selecting into the derivation so the next assertion added is not the next hole, but not a blocker and not a regression.

Findings 1 and 3 are unchanged. The hatch is open and its arm is tautological — that one I demonstrated end to end with a corpus file, 1 passed with the rewrite against VacuityError without it, and I re-ran nothing that weakens it. The TESTS.md 26 + 47 = 73 contradiction against the gated 72 also stands. The review stays as changes-requested on those two.

[jdatcmd]

Approving at 4c8e2e20, 12/12 SUCCESS, CLEAN. I re-ran my own attack rather than reading the fix.

The hatch is closed. The end-to-end corpus file that produced 1 passed at the old head — rewriting pgc_vacuity.QUERY_ERROR at module scope, then comparing the hardcoded 'QUERY_ERROR.empty-relation' built two ways:

old head 1e43b8db  ->  1 passed        (two failed queries compared equal, green)
new head 4c8e2e20  ->  1 failed        pgc_vacuity.py:174: VacuityError

Binding the prefix at definition time is the right mechanism: the producer and the matcher both capture it, so rewriting the module global changes neither.

And the new arm is load-bearing. Reverting the matcher to read the global at call time — one line, v.startswith(_prefix) back to v.startswith(QUERY_ERROR), diffed and restored byte-identical:

FAILED test_the_refusal_cannot_be_switched_off_from_the_corpus_it_polices
FAILED test_a_hardcoded_sentinel_survives_the_same_rewrite
2 failed, 8 passed

The second of those is the case I raised — test_hilbert_locality.py:275 mints its sentinel independently of the global — and it now has an arm of its own. Keeping ZZZ_NOT_A_PREFIX in the spelling list is the detail I would point a reader at: 'Q' cannot disarm a prefix matcher, so a spelling that is not a prefix of the real one is the only case that finds the hole, and it is the case the old arm never tried.

ordering_observable now refuses, and does not over-refuse:

two minted sentinels  REFUSED
hand-rolled fwd/rev   REFUSED
legitimate distinct   GREEN     <- the control

You fixed this even though I had already downgraded it to a note and withdrawn the regression claim I attached to it. Worth recording that my "this PR turns a red green" was wrong — the shape was green at the base too.

One note, and it is only the commit message. The subject says "make both halves of the totals sentence gated". The value is right — TESTS.md now reads 26 + 46 = 72, matching the gated table. But the second half still has no arm:

TESTS.md: "The other 46" -> "The other 99"    350: 41 checks, 41 passed, 0 failed

_mi_not gates VACUITY_MODES.md's table rows at 369 and 372, and nothing reads The other [0-9]+ in TESTS.md. Not blocking, and the number is correct today — but the claim is broader than the arms.

Also worth stating, as on #927: nothing runs this corpus in CI at this head, and #921 is the PR that changes that.

[jdatcmd]

Re-approving at 8baef757, which is the head after your two main merges. My earlier approval named 4c8e2e20 and was stale.

I re-ran everything against this head rather than carrying the old verdict forward:

my hatch attack (the corpus file that rewrites the global)   1 failed, VacuityError    (was 1 passed at 1e43b8db)
removal proof: v.startswith(_prefix) -> v.startswith(QUERY_ERROR)
    FAILED test_the_refusal_cannot_be_switched_off_from_the_corpus_it_polices
    FAILED test_a_hardcoded_sentinel_survives_the_same_rewrite
the composed corpus, six files including both merges' arms      101 passed
selftest 350 doc arithmetic on the composed tree                41 checks, 41 passed, 0 failed

pgc_vacuity.py restored byte-identical after the mutation; worktree clean.

The auto-merge is the part I cared about and it holds. pgc_vacuity.py took changes from #927 and from this PR without conflicting, which is exactly where "each green separately, broken together" lives — so running the composed file was the check, not the clean merge. Your 40 passed and 205 passed agree with what I measure here.

The VACUITY_MODES.md composition also reconciles: section 2 keeps both rows at 27 refused / 45 not refused against an unchanged 72, and 350 recomputes all of it from the ids rather than from the prose.

My earlier note about the commit message claiming both halves of the TESTS.md sentence were gated still applies as a note, and nothing else stands. Approved.