Skip to content

test/pytest: a loop that never ran asserted nothing, and half of it was already refused (#432) - #951

Merged
jdatcmd merged 4 commits into
commandprompt:mainfrom
OffgridwithJD:audit/432-a-loop-that-never-ran
Sep 11, 2026
Merged

test/pytest: a loop that never ran asserted nothing, and half of it was already refused (#432)#951
jdatcmd merged 4 commits into
commandprompt:mainfrom
OffgridwithJD:audit/432-a-loop-that-never-ran

Conversation

@OffgridwithJD

Copy link
Copy Markdown
Collaborator

Stacked on #950 — it adds section 5's entry 6, which only exists because #950 made that list checkable. Merge #950 first; this is the first use of the rule it added.

assert-inside-a-loop-over-zero-rows in VACUITY_MODES.md 3.5 is two shapes, and the layer already refused one of them without anyone recording that it did.

The half already refused

When a loop's body holds a test's only counted assertions, a zero-trip loop leaves the count at 0 and pytest_runtest_call raises VacuityError. Measured on a planted test rather than read off the hook:

only assertion inside a zero-trip loop   VacuityError: made no counted assertion
the same loop with one row               1 passed

So that half needs nothing, and this PR does not claim it.

The half that was open

When the test also asserts outside the loop, the count is non-zero, the test passes, and the loop's assertions simply never ran. Nothing noticed. That is the shape a query returning no rows produces, and the shape a glob matching nothing produces.

The population, measured before the arm was written

shape loops state
non-empty by construction (literal, range, local literal) 20 cannot be zero-trip
derived, loop holds the only assertions 0 already refused
derived, with assertions outside the loop 2 at risk

Both at-risk loops already carried a premise, so the arm is green on arrival. That is the point rather than a weakness: the property was true of the corpus and nothing was holding it there, so what this catches is the third one. It is not insurance against an imagined shape — it has a population of two and locks it in.

Both at-risk loops are ones I wrote, which is its own small argument for the arm.

It narrows rather than closes, and 3.5 names the residual

The honest requirement is a premise bounding the cardinality of this iterable. What is enforced is a counted assertion outside the loop taking len(...) of something.

test_harness_deps.py's loop iterates sorted(found) while its premise bounds len(files)found is built from files in a preceding loop. A rule demanding the names match would reject correct code, which is how a guard gets switched off. So the residual is a loop whose premise bounds the wrong collection: a reviewer catches that, a sweep does not.

And a defect in my own declaration, found by a count that did not reconcile

NO_CLUSTER ended up naming the new file twice. My edit script ran a second time during an unrelated revert, its anchor was still present, so the count == 1 guard passed and it double-applied.

membership_report could not see it. It compares set(declared) against the corpus, and a set has no cardinality to disagree with. The tell was the declaration saying 13 while job_runnable said 12 with both set differences empty — only possible if a name appears twice.

test_the_declaration_names_each_file_once now asserts the cardinality separately. It discriminates: re-add the duplicate and it fails naming the file; restored byte-exact.

And the docstring records the measured consequence rather than the assumed one: 12 entries and 13 entries both give 215 passed, because pytest deduplicates identical paths. So the cost is the job's own printed file count, not a double run. My first version of that docstring claimed the double run and was wrong.

A set comparison hiding a duplicate is the same shape as a check-name collision folding two ledger rows into one. Whenever the mechanism compares sets, the cardinality needs its own arm.

Gate

pg18a (assert):

docs_style.sh           9 checks, 0 FAIL
harness_selftest.sh     757 checks run, 0 FAIL
driver-free pytest      215 passed across 12 files
full pytest corpus      301 passed

No shell check changed, so check_ledger.tsv and its census are untouched.

Based on #950 at 3e57ee54.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a

OffgridwithJD and others added 4 commits September 11, 2026 03:23
… and it was wrong

VACUITY_MODES.md sections 1a, 2 and 3 are all compared against the mode ids on
disk. Section 5 -- the ordered list of what to build next -- was prose, and entry 1
still said "the constant exists and nothing writes it" long after `query_error()`
existed and `test_failed_query_sentinel.py` carried ten arms over it.

THAT IS THE MOST EXPENSIVE PLACE IN THE DOCUMENT FOR A STALE SENTENCE, because its
only reader is someone about to build something. The near-miss one document over is
what it costs: a bad enumeration of `test/selftest/340` made an existing block look
like a coverage gap, and the duplicate was written and proven to discriminate
against a mutation before the duplication was noticed.

WHAT IS CHECKABLE IS THE ANCHOR, NOT THE WORK. Entry 1's work landed under four test
names, none of them the one the entry proposed, so asking whether the NAMED test
exists would have passed and said nothing. Two arms hold the list instead:

  * every entry names at least one mode id, so it is tied to the inventory at all.
    Entry 1 named none, which is exactly how it stayed wrong.
  * no UN-STRUCK entry names an id section 2 already claims as refused. An entry
    whose id has reached section 2 is done by the document's own accounting,
    whatever the test ended up being called.

Entry 1 is struck and anchored to `error-swallowed-to-empty`, and it records what
the entry got wrong rather than replacing it: both halves of "the constant exists
and nothing writes it" were false -- two sites already minted sentinels by hand, one
from inside SQL, and the thing missing was the REFUSAL in four of the five
comparisons.

FIXING IT MADE THE SECOND ARM VACUOUS ON THIS DOCUMENT, because with every entry
struck there is nothing left to refuse. The planted fixture beside it is therefore
the whole of its evidence, and section 5 says so rather than leaving it implied: a
two-entry fixture where moving the open entry's id into section 2 must be named,
with the clean control beside it. An arm that is vacuous today and load-bearing
after the next edit should say which it is.

The totals do not move -- 28 refused, 44 not refused, 72 named, all three still
matching what 1a states -- because the mode was already counted as refused. Only the
list that tells the next person what to do was wrong.

One instrument defect of mine, caught by that fixture: the parser matched the
SECTION HEADING as an entry. `re.split(r"^## ")` leaves a chunk beginning "5. What
to add next", which the numbered-item pattern also matches, inventing an entry 5
that is the title and colliding with the real entry 5. The fixture reported 3
entries in a two-entry document, which is how it was found.

Verified on pg18a: selftest 757 checks, 0 FAIL; driver-free subset 210 passed, which
is 207 plus these three; the full corpus 295 passed. Section 1a's three totals
reconcile against the ids on disk.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
…idence-led

Section 5's new rule is that an entry must name a mode id. That makes WHICH id
worth measuring rather than guessing, so four of 3.6's were counted by AST scan
over the whole corpus before anything was built on them:

    truthy-cursor-from-execute        7 sites, ALL BENIGN. Every one is
                                      `x = cur.execute(...)`, which is idiomatic
                                      psycopg3 -- execute returns the cursor. ZERO
                                      branch on it (if/while/assert), which is the
                                      dangerous form.
    empty-query-string-succeeds       0 sites.
    multistatement-execute-...        1 site, a three-statement SETUP that fetches
                                      nothing; the arm after it asserts
                                      count(*) = 40000, so the INSERT is proven.
    server-cursor-rowcount-is-not-... 2 sites, BOTH LEGITIMATE: a DELETE's rowcount
                                      with an at_least premise, and a stub field.

SO ALL FOUR ARE PROSPECTIVE. A guard for any of them is insurance against a shape
the corpus has not written yet, not a closure of one it has, and it should say so --
the way the plan_marker ordering arm does.

That is not an argument against writing them. It is an argument against writing one
and calling the mode closed: 1a counts section 2 as "refused today", and a refusal
with no population has not refused anything. The cheapest of the four is named, with
the reason: the dangerous spelling of the first is distinct from the benign one, so a
planted fixture separates them in two lines.

Verified on pg18a: driver-free subset 210 passed, the full corpus 295 passed, and the
doc arms 31 passed. Documentation only; no check changed, so the ledger is untouched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
`docs_style` refuses an em or en dash in CHANGELOG.md and the two commits before
this added four, one line carrying two. Main has none, so all four arrive here.
Replaced with the house ` -- `. Reported by @jdatcmd.

I should have run `docs_style` myself: the change edits CHANGELOG.md and that suite
is the one whose subject is CHANGELOG.md. Deriving the gate list from what a change
touches is the rule, and I skipped it for a documentation-only commit, which is
exactly where it is easiest to skip.

AND THE FIRST FIX WAS TOO BROAD, caught before it was pushed. A sweep over every file
this branch touches replaced 147 dashes in TESTS.md, VACUITY_MODES.md and
test_docs_cover_the_corpus.py as well. Those are pre-existing house style: the check
names CHANGELOG.md and its own comment says "CHANGELOG.md: dash characters only. See
the scope note above." Rewriting 147 lines nobody asked about would have buried four
real ones in a diff no reviewer could read. Reverted, and only CHANGELOG.md changed.

Verified: `docs_style.sh` 9 checks, 0 FAIL, including the arm that caught this.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
…as already refused

`assert-inside-a-loop-over-zero-rows` in VACUITY_MODES.md 3.5 is two shapes, and the
layer already refused one of them without anyone recording that it did.

ALREADY REFUSED. When a loop's body holds a test's ONLY counted assertions, a
zero-trip loop leaves the count at 0 and `pytest_runtest_call` raises `VacuityError`.
Measured on a planted test rather than read off the hook:

    only assertion inside a zero-trip loop   VacuityError: made no counted assertion
    the same loop with one row               1 passed

WAS OPEN. When the test ALSO asserts outside the loop, the count is non-zero, the test
passes, and the loop's assertions simply never ran. That is the shape a query returning
no rows produces, and the shape a glob matching nothing produces.

THE POPULATION, measured over the whole corpus before the arm was written:

    non-empty by construction (literal, range, local literal)   20  cannot be zero-trip
    derived, loop holds the only assertions                      0  already refused
    derived, WITH assertions outside the loop                    2  at risk

Both at-risk loops already carried a premise, so the arm is green on arrival. That is
the point rather than a weakness: the property was true of the corpus and nothing was
holding it there, so what this catches is the third one.

IT NARROWS RATHER THAN CLOSES. The honest requirement is a premise bounding the
cardinality of THIS iterable; what is enforced is a counted assertion outside the loop
taking `len(...)` of something. `test_harness_deps.py`'s loop iterates `sorted(found)`
while its premise bounds `len(files)`, because `found` is built from `files` in a
preceding loop, so a rule demanding the names match would reject correct code -- which
is how a guard gets switched off. The residual is a loop whose premise bounds the wrong
collection: a reviewer catches it, a sweep does not. 3.5 names it.

Section 5 gains entry 6, struck and anchored to its mode id, which is the first use of
the rule the previous commits added.

AND A DEFECT IN MY OWN DECLARATION, found by a count that did not reconcile.
`NO_CLUSTER` ended up naming the new file TWICE, because the edit script ran a second
time during an unrelated revert and its anchor was still present, so its `count == 1`
guard passed and it double-applied. `membership_report` could not see it: it compares
`set(declared)` against the corpus, and a set has no cardinality to disagree with. The
tell was the declaration saying 13 while `job_runnable` said 12 with both set
differences empty, which is only possible if a name appears twice.

`test_the_declaration_names_each_file_once` now asserts the cardinality separately, and
the docstring records the measured consequence rather than the assumed one: 12 entries
and 13 entries both give `215 passed`, because pytest deduplicates identical paths. So
the cost is the job's own printed file count, not a double run. The first version of
that docstring claimed the double run and was wrong.

Verified on pg18a: `docs_style` 9 checks 0 FAIL; selftest 757 checks 0 FAIL;
driver-free subset 215 passed across 12 files; the full corpus 301 passed. No shell
check changed, so the ledger is untouched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a

@jdatcmd jdatcmd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving at a2e189d7, 13/13 green. One finding about the PR's base, not its content.

The already-refused half, verified by planting one

I did not take the hook reading. Planted inside the real corpus:

def test_only_assertion_inside_a_zero_trip_loop(expect):
    for x in []:
        expect.num(x, x, "unreachable")
def test_control_a_real_assertion(expect):
    expect.num(1, 1, "this one counts")
VacuityError: vacuity guard: test_only_assertion_inside_a_zero_trip_loop made no
counted assertion. A test that concludes nothing must not report a pass.
1 failed, 1 passed

So the layer already refuses the shape where a loop holds a test's only counted assertions, and the control passes beside it — which is what makes the failure about the zero trip rather than about the fixture. Splitting the mode into the half that is already closed and the half that needs a cardinality premise is the right reading, and it is the difference between adding a guard and discovering you already had one.

I challenged your population count and I was wrong

Worth recording because the correction is mine. I measured the corpus on main independently and got the total exactly right — 22 loops — then split them 17 / 1 / 4 against your 20 / 0 / 2 and came here to report a discrepancy.

Two of my four "at risk" iterate .items() on a local dict literal:

for shape, text in shapes.items():      # test_suite_accounting.py:119
for label, text in cases.items():       # test_mutation_ledger.py:328

Non-empty by construction. My detector only recognised a bare Name as the iterable, so it could not see through the method call. Your classification is right and mine was the cruder instrument — the third time today my sweep disagreed with yours and the fault was at my end.

The agreeing total is what makes that legible: same 22 inputs, different buckets, so the disagreement had to be classification rather than population. inputs == sum(buckets) earned its keep on both sides.

Green on arrival is the point, and you are right to say so

Both at-risk loops already carried a premise, so the arm passes today. A population of two that is already compliant, locked in, is a different thing from insurance against an imagined shape — what it catches is the third one. Measuring the population before writing the arm is what lets you say that rather than hope it.

The finding: this PR's base

#951 is based on main but contains #950 — I confirmed 3e57ee54 is an ancestor of a2e189d7, and the diff against main carries #950's files.

So merging #951 alone would land #950's work under #951's number: reviewed in one place, attributed in another, and #950's own approval never exercised. Either retarget this to #950's branch so its diff shows only its own change, or land #950 first. You already said #951 must follow #950 — this is the same instruction, with the reason that it is not merely preferable.

Noted

Your NO_CLUSTER double-entry is the cleanest instance yet of the set/cardinality family: membership_report compares set(declared), and a set has no cardinality to disagree with, so the duplicate was invisible to the thing whose job is to see it. The tell — declared=13 against job_runnable=12 with both set differences empty — is only possible if a name repeats. And correcting your own claim about the consequence, that 12 and 13 both give 215 passed because pytest deduplicates identical paths, is the part most people would have left alone once the fix was in.

Whenever a mechanism compares sets, the cardinality needs its own arm. Agreed, and it generalises past this repo.

@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

⚠ Merge #950 first — this branch CONTAINS it

Raised by @jdatcmd and it is a real hazard I created: this PR's GitHub base is main, but the branch is built on #950's head (3e57ee54), so merging this alone would land #950's three commits under this PR's number. The content would arrive either way; the attribution and the review trail would not.

I cannot retarget the base — the parent is a branch in my fork and GitHub will not take a fork branch as the base of a cross-repo PR — so the dependency has to be respected rather than enforced. Making it unmissable here rather than only in the body.

git log --oneline main..HEAD
  a2e189d7  a loop that never ran asserted nothing        <- this PR
  3e57ee54  docs: spell the dashes the way CHANGELOG does \
  a6dc3e22  record the measured populations                 > #950
  9dd248fe  the "what to add next" list was wrong         /

Why it is genuinely stacked rather than lazily branched: this PR adds section 5's entry 6, and entry 6 is only checkable because #950 added the two arms that hold entries to the inventory. Rebasing this onto bare main would drop the paragraph entry 6 is inserted before, and the arms that give it meaning would not exist. It is a dependent change, not a convenience.

#950 is approved at 3e57ee54. Land it, and this becomes a single-commit PR against main with no further action from me.

@linuxhikerpm linuxhikerpm left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving exact head a2e189d7f5d5. MERGEABLE, CLEAN, 13/13 on this SHA. Stacked on #950 as the PR says; merge #950 first.

The mode really is two shapes, and the planted VacuityError measurement is the right way to claim the already-refused half rather than reading the hook. The open half — assertions inside a derived loop that also asserts outside — has a measured population of 2, both already carrying a premise, so the arm is green on arrival on purpose. The residual (premise bounds the wrong collection) is named rather than papered over. NO_CLUSTER duplicate-name arm is the cardinality check a set comparison cannot see.

Non-blocking: #945 already added TESTS.md section 24 for test_join_runtime_filter.py. This PR also inserts a section 24 for test_loop_coverage_premise.py. Landing either first makes the other rebase and renumber.

@jdatcmd
jdatcmd merged commit e030c15 into commandprompt:main Sep 11, 2026
13 checks passed
jdatcmd added a commit to OffgridwithJD/pgcolumnar that referenced this pull request Sep 11, 2026
Merging main after commandprompt#942, commandprompt#947, commandprompt#949, commandprompt#950 and commandprompt#951 landed. Two conflicts and one
of them could not be resolved by reading the diff.

CHANGELOG.md -- union, both entries kept.

check_ledger_budget.txt -- THE NUMBER CAME FROM A RUN, not from arithmetic. This
branch carried 769 off main's old 756 baseline; commandprompt#947 has since landed 762; the
composed ledger takes both row sets and holds 775. None of 769, 762, or any sum
of deltas is the answer, because the rows are the source and the census is a
measurement of them.

    selftest on the composed tree, census left stale on purpose
        776 checks, 775 passed + 1 failed
        FAIL the committed census matches the committed ledger: got [762] want [775]

That arm naming 775 is the derivation; the confirmation is a second run:

    census set to 775, derived from the ledger
        776 checks, 776 passed + 0 failed

CHECKED BEFORE TRUSTING THE LOG, because a reconciling log can still be evidence
about the environment rather than the code (commandprompt#946):

    checks in the run absent from the ledger    0
    rows in the ledger absent from the run      2, both pre-existing conditional
                                                premises in part 330 that also sit
                                                in main's ledger
    FAIL records in the run                     1, the stale census arm itself

So the ledger's auto-merge was correct and the only thing wrong was the number
describing it. Ceiling untouched at 250: adding rows to an already-covered suite
cannot move it.

Third time this artifact has needed re-deriving on a merge. That is now a property
rather than an accident -- the census is a measurement of the tree, and every
merge invalidates it. Tracked in commandprompt#952.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants