test: refuse an assertion that discards its measurement (#1164)

[jdatcmd]

Follows #1165, which repaired the 26 pytest sites. This refuses the next one.

expect.num(1 if abs(on - off) <= 5 else 0, 1, ...) reduces four measured buffer counts to a boolean before the assert, so the failure reads got 0 want 1 and carries none of them.

The measured cost. A PG 15 leg reddened one arm of sorted_pathkeys, and two sessions could not say whether the ORACLE (|on - off| <= 5, a six-buffer spurious gap) or the CONTROL (order_on > order_off + 5, a fourteen-buffer collapse) had failed. Different defects, different owners. Four further experiments were aimed at a target whose identity is still unknown, and the honest ceiling on that investigation is "we could not reproduce it".

Two halves, deliberately asymmetric

pytest   asserts ZERO                  that corpus reaches zero after #1165
shell    asserts a tracked list         that corpus holds 60 arms across 35 suites

A shell guard asserting zero would go red on arrival with 60 offenders, which is how a rule gets switched off within a week. So the debt is tracked in test/lossy_arms.tsv and the gate refuses both directions: an unlisted offender fails by name, and a repaired arm fails until its row is removed, so the list may only shrink — by construction, with no number for anyone to maintain.

The asymmetry is the honest answer. The two corpora are in different states, and a guard asserting zero over both would claim a property this tree does not have.

The predicate was run over the tree before the guard was written

expect.*(<IfExp>, ...)              190
  + both branches constant          148
    + a LOSSY test                   26

Three drafts over-reported and each was narrowed by reading what it selected, not by trusting the count. is None, in, == and nothing-versus-something (x > 0, n >= 1) are determinate — the boolean already names the failing state.

Nothing-versus-something is determinate whatever the left side looks like. An earlier draft keyed on the operand being a bare Name and flagged (st['ret'] or 0) > 0; the or 0 is a None-guard, not a measurement, and refusing it was syntax standing in for semantics.

The one exception, and why the carve-out cannot be "the comparator is 0":

'yes' if min(t_proj_run, l_proj_run, t_base_run, l_base_run) > 0 else 'no'

The comparator IS 0 and the boolean still hides which of four measurements went non-positive.

Three bugs, none of which one instrument could find

generator required `|| echo`     missed ten `&& echo ok` arms
sweep searched anywhere for a    read `grep -c .` as a file test, because `c` is in
  unary operator                   the class -- excused five real offenders
line continuations unfolded      four arms that DO carry their value, across a
                                   `\` newline, read as though they discard it

The first two were found only by the two implementations disagreeing — 10 one way, 5 the other, neither half able to find either bug alone. The third was found because a reviewer asked an unrelated question about empty want values. 65 → 61 → 60.

Keyed by (suite, name), and the collision is live

Keyed by name alone the corpus gives 54 distinct keys for 55 rows: premise: the install script declares MODULE_PATHNAME symbols at all is carried by both entry_point_privilege and hilbert_cluster, so one key for both would let a repair in one be satisfied by the other. Never file:line — a line number moves under any edit above it.

Verified

harness_selftest.sh          1086 passed + 0 failed   (1081 on main, +5)
pytest guard leg              388 passed, want 388     (382 on main, +6)
pytest cluster leg            442 passed, want 442     unmoved, as expected

Both directions proved by mutation against wrapped arms specifically:

planted a wrapped arm that DISCARDS
  FAIL  a new arm that cannot say what it measured is refused by name:
        got [autovacuum: planted: a wrapped arm that discards its numbers;]
planted a wrapped arm that CARRIES
  1086 passed + 0 failed          <- correctly not caught
repaired a real listed arm
  FAIL  a repaired arm must have its row removed, so the list only shrinks:
        got [autovacuum: premise: av_off starts with dead rows above the threshold;]

61 -> 60 in this PR is #1165 repairing native_fetch_coalesce: a wide index fetch does not pin once per column. The row went because the sweep stopped finding it, not because anyone edited the file.

Known false positives, published rather than claimed away

determinate by a NEIGHBOURING assertion   collation_pinned:72 -- `len(names) >= 2`
                                          preceded by `at_least(len(names), 1)`.
                                          No static rule can see it.
a drift guard over two constants          nothing MEASURED is lost, and it still
                                          needs to name what the constant became
check_num "$X" "$X"                       correct through that helper, which
                                          validates both sides are numbers first

The list is generated by the sweep that checks it, which is circular by construction — so the controls in 540 validate it, not the list. That is stated in the header rather than left for someone to notice.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK

[OffgridwithJD]

You asked me to try to plant a shape the controls accept and the sweep mishandles. I found
five. Two of them I recommend you do NOT close, and that half is the more useful half.

Every result below is from running your collapsing_arms against a planted fixture, then
checking the real corpus for whether the shape is live or theoretical.

Both controls behave

a real lossy arm in args[0]     -> n=1
an arm whose branch carries     -> n=0

1. A chained comparison is not examined — LATENT, and it has live near-misses

expect.num(1 if 0 < sel < 20000 else 0, 1, 'n')     ->  MISSED

_comparison_is_lossy opens with if not (isinstance(test, ast.Compare) and len(test.ops) == 1): return False, so a two-op comparison is scored determinate. 0 < sel < 20000
hides sel and has many failing states.

Two chained comparisons exist in the corpus today:

test_projection_update.py:65   'partial' if 0 < sel < 20000 else f'degenerate ({sel})'
test_scan_decode_cost.py:133   'yes' if wa and wt and (0 < wa < wt) else f'no (a={wa} t1={wt})'

Both are saved by the f-string on the other branch, so _branch_carries returns first and
neither is a live miss. But the guard is not what is protecting them. Remove either
f-string and the arm becomes undiagnosable with the guard silent. Since _branch_carries
runs first, fixing this costs nothing in false positives on today's tree.

2. min over a single non-constant argument — theoretical, one line

expect.text('yes' if min(runs) > 0 else 'no', 'yes', 'n')     ->  MISSED

_aggregates requires len(node.args) > 1, so min(runs) over a list of four
measurements is excused while min(a, b, c, d) is caught. Same defect, same reasoning as
the exception you documented. Zero live sites, so take it or leave it — but your own
control min(a) > 0 is correct for a different reason than the code gives: it is
determinate because there is one operand, not because there is one argument.

3. The shell scope excludes a directory on a justification that covers one file

test/selftest/ is excluded because "this file's own explanation of the rule, and the
control fixture below, both contain the shape they exist to describe." That is exactly
right for 540. It is not a property of the directory.

test/selftest/410-a-check-must-have-been-red.sh:423 is a normal selftest part, and it
carries a genuinely lossy arm by your own rule:

check "and defines it BEFORE the first call, since bash reads top to bottom" \
	"$([ "$(grep -n '^pgc_stamp_log()' "$_rv" | cut -d: -f1)" \
	     -lt "$(grep -n 'pgc_stamp_log "' "$_rv" | sed -n 2p | cut -d: -f1)" ] \
	   && echo before || echo after)" "before"

Two computed line numbers, -lt, both hidden — and the comment above it says the real
failure was "definition at line 1128, called at 762". Those are precisely the two numbers
got [after] want [before] throws away.

Narrowing the exclusion to the file that describes the rule, rather than the directory
containing it, is the change I would make. Selftest 260's mistake was sweeping the enforcer
for instances of the rule; the enforcer is one file.

4 and 5: holes I recommend leaving, with the evidence

_branch_carries treats any non-Constant as carrying, so a named constant slips:

KEPT, HALVED = 'io-kept', 'halved'
expect.text(KEPT if ratio < 1.35 else HALVED, KEPT, 'n')      ->  MISSED

Do not tighten it. All four Name/Attribute branches in the corpus genuinely carry
the measurement — bounds, old, proj_err.sqlstate, plain_err.sqlstate — so requiring
an f-string would produce four false positives on correct code. The rule "a branch that is
not a bare constant carries" is right for this tree; the constant-alias case is a shape
nobody writes and would cost more to exclude than to tolerate.

Only args[0] is examined:

expect.num(1, 1 if abs(on - off) <= 5 else 0, 'n')            ->  MISSED

Also leave it. All three conditional expressions in later argument positions are
conditional check names, not measured values — test_projection_privilege.py:185 and
:191 build the name from which function is under test. Scanning args[1:] would flag
three legitimate sites to catch a shape that does not occur.

On the circularity you flagged

The list being generated by the sweep that checks it is real, and the controls are load
bearing rather than decorative — I could not get a planted fixture past
test_a_lossy_arm_is_caught or the carve-out driver. What the controls do not cover is
the sweep's reach: every control is a file the sweep is pointed at, so a shape it never
looks at (a chained comparison, a directory not globbed) passes every control while the
population is wrong. Findings 1 and 3 are both that shape. A control proves the matcher
sees what it is shown; only the population argument covers what it is not shown.

What I checked and found clean

• _counted excluding cannot_run is right — it declares a test unrunnable rather than
  concluding anything.
• The (suite, name) key: I verified the collision is live, 54 keys for 55 rows.
• The coverage premise counting classified arms rather than rejected ones is the
  d41d8cd98f00 defence and it is in the right place.
• Folding continuations before sweeping, after four arms read as discarding across a \.

Findings 1 and 3 I would fix before merge. 2 is optional. 4 and 5 are documented reasons
not to act, and I would put them in the file so the next reviewer does not re-derive them.

[OffgridwithJD]

Re-ran my six plants against 585806e. Findings 1 and 2 are closed — the chained
comparison and min over one argument are both caught, and 4 and 5 still slip exactly as
documented, which is the intended behaviour.

Your sharpening of finding 2 is right and I verified the premise rather than taking it:

min(5)      -> TypeError: 'int' object is not iterable
min([3,1,2]) -> 1

So a one-ARGUMENT call is an iterable by construction, len(args) > 1 was wrong rather
than conservative, and your control was passing while asserting something that cannot
happen. I had the observation and stopped one step short of the consequence.

A second round found two more, and one is created by the fix

_is_lossy handles BoolOp and Compare and returns False for every other node
type. That is a fail-open default, and it means any comparison wrapped in something else
is scored determinate:

expect.text('yes' if any(r <= 0 for r in runs) else 'no', 'no', 'n')   ->  MISSED
expect.text('yes' if all(r > t for r in runs) else 'no', 'yes', 'n')   ->  MISSED
expect.num(1 if not abs(on - off) > 5 else 0, 1, 'n')                  ->  MISSED

Your new chain handling and the walrus case both behave:

1 if 0 <= idx < len(xs) else 0        -> caught
1 if (g := abs(on - off)) <= 5 else 0 -> caught
1 if a is not None is not b else 0    -> not caught   (determinate chain, correct)

any(r <= 0 for r in runs) is the natural rewrite of min(runs) > 0. So the arm you
just taught the guard to refuse has an escape hatch one keystroke away, and an author told
"your min arm is refused" is fairly likely to reach for it. That is the one I would close
on those grounds alone, independent of whether it is live.

Neither is live, and I checked rather than assumed

any()/all() inside an expect conditional   3 sites
  encode_post_codec.py:169   `!=` inside, so determinate anyway -- correctly not flagged
  the other two carry their value

a negated subexpression                    8 sites
  six carry; the two that do not wrap an emptiness test, not an ordering comparison

So zero live misses. The risk is the evasion path, not the current tree.

The false-positive budget of closing it, measured

I implemented the recursion — descend through any node rather than defaulting to
determinate — and ran both over the corpus:

as shipped (585806e):  0 offenders
recursing into all:    0 offenders
FALSE-POSITIVE COST:   0

Zero, because the only live any() uses != inside and != is already determinate. So
this is a strictly-safe tightening on today's tree, which is the argument I could not make
for findings 4 and 5 — those cost four and three false positives respectively, which is why
I said leave them and still would.

The change is three lines:

def _is_lossy(test):
    if isinstance(test, ast.BoolOp):
        return any(_is_lossy(v) for v in test.values)
    if isinstance(test, ast.Compare):
        return _comparison_is_lossy(test)
    return any(_is_lossy(c) for c in ast.iter_child_nodes(test))

with a driver for any(), not, and a determinate control (any(e != X for e in ...)
must stay unflagged, and it is the real shape from encode_post_codec:169).

Your call whether it is worth another round. The guard is a clear improvement as it
stands, the hole is not live, and I would not hold the PR for it — but "the fix teaches the
evasion" is the kind of thing that is much cheaper to close now than after somebody uses it.

On scope, and what I would add to the header

77 rows across 52 suites, with 410's two computed line numbers in the list, is the right
answer and 17 rows is a real cost to have found.

One sentence I would add beside "a control proves the matcher sees what it is shown":
the default for an unrecognised shape decides which way the guard fails. _is_lossy
defaults to determinate, so every shape it has not been taught is silently permitted —
which is how BoolOp was missed in one draft, how chained comparisons were missed in the
next, and how any() is missed now. Three instances of one default. A guard that defaulted
the other way would have been noisy and wrong in a way somebody would have fixed on day one.

[jdatcmd]

The two suites legs were the ledger gate, not the guard. The guard itself passed all five arms on both majors — the log shows PASS for each — and then the gate refused them:

not in the ledger: harness_selftest  540-an-arm-must-carry-its-measurement
                   a new arm that cannot say what it measured is refused by name  (on major 17)
... and four more

harness_selftest is one of the 13 covered suites, and the gate refuses a check the ledger has never seen in a suite it already covers. Adding five checks to a covered suite is exactly the case that loses.

This is the gap my own notes describe and I walked into it anyway: harness_selftest green locally does not reach the ledger gate, because the gate is a matrix-level step and the suite is not.

The major set was derived, not copied

A ledger row is a matrix-wide claim. Rather than copy 15;16;17;18;19 from the neighbouring rows, harness_selftest was built and run against each major and the five logs merged together:

PG15  15.18      1086 passed + 0 failed
PG16  16.14      1086 passed + 0 failed
PG17  17.10      1086 passed + 0 failed
PG18  18.4       1086 passed + 0 failed
PG19  19beta2    1086 passed + 0 failed

ledger: rows=1468 | runs=5, distinct checks this merge=1086
  majors: uniform, all 1468 rows carry 15;16;17;18;19
  rows 1468 = sum of buckets printed 1468

Verified against the property the gate actually enforces, rather than by re-running the gate with a reconstructed argument list: every one of the 1086 RESULT records in the run now has a ledger row, 0 missing.

census        1439 -> 1444   re-counted with awk, never by adding five
suites_not_covered     249   unmoved -- the suite was already covered, which is
                             precisely why these five needed seeding

One thing the merge did that I undid, and it is worth a look

pgc_ledger.py merge also rewrote the DETAIL field of two unrelated index_am_support rows — reordering their ;-joined segments and adding a leading space — so the raw merge was +7 / -2 rather than +5 / -0. The verdict and major columns were untouched; only the prose moved.

I restored both, so this change adds five rows and touches nothing else. But that rewrite would land in every PR that merges a run, as unrelated churn a reviewer has to read past to find the real rows. Worth someone looking at separately; I have not filed it because I have not established whether the reordering is deliberate.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK

[jdatcmd]

Correcting my previous comment: I was wrong about the merge, and I established it instead of leaving the flag hanging.

I said pgc_ledger.py merge "rewrote the DETAIL field of two unrelated rows" and called it churn worth someone looking at. I then restored those rows so my diff stayed at five.

That was the wrong call on both counts. The tool documents the column:

mutations is -, or a ;-separated SET of the mutations that have reddened this check — accumulated, not overwritten

and writes it as ";".join(sorted(muts)). The reordering is the canonical form, not a side effect. Restoring the unsorted rows preserved a non-canonical state that the next merge would have corrected anyway, so my "clean five-row diff" was tidiness bought with a worse file.

The leading space was the real defect, and it is data rather than the tool. One element of that set literally begins with a space:

docs/features.md qualifies the claim inside its own bullet: 'builds ...
 the whitelist refuses it for being a form the suite cannot read$
docs/features.md stops naming any index access method, so the extractor ...

Sorting puts it first, which is what made the rewrite look like reordering for its own sake. The row is mine, recorded under #1147.

Both rows are now sorted with each element stripped, and nothing anywhere in the ledger starts an element with whitespace. The file's delta against main:

+5   new rows for 540's arms, majors derived from five runs
 2   rows canonicalised, one stray leading space removed

I would rather carry two lines of justified canonicalisation than a clean diff that leaves the file wrong — and I should not have implied a defect in someone else's tool before reading what it says it does.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK

[OffgridwithJD]

Re-ran every prior plant against 4a19940. Rounds 1 and 2 are fully closed — the
chained comparison, min over one argument, any(), all() and the negated comparison
are all caught, the determinate chain is correctly not caught, and C/D/E still slip
exactly as documented.

The direction regression you found is fixed and I drove it both ways:

r > 0   not caught     r <= 0  caught
r >= 1  not caught     r < 1   caught
                       r <= 1  caught

Round 3 found two more. Neither is live in the corpus — I checked before reporting, 0
instances of each — but they are different kinds of theoretical and I would treat them
differently.

1. A false positive on correct code: the swapped form

expect.num(1 if 0 < r else 0, 1, 'n')     ->  CAUGHT, and should not be

0 < r is r > 0 with the operands written the other way round. It is determinate — it
fails at exactly one value — but the carve-out keys on the COMPARATOR being 0 or 1, and
here the constant is on the LEFT, so the nothing-versus-something test never fires.

This is the one I would fix, and not because it occurs today. A guard that refuses
correct code is the failure mode that gets a guard switched off, and then the rule it
enforced is gone too — which is the repository's own stated reason for the false-positive
budget. 0 < x is ordinary Python, and 0 < x < y is idiomatic enough that two arms in
this corpus already use the chained form.

The fix is symmetric rather than another special case: when the left side is the constant
and the right is not, mirror the comparison — swap the operands and invert the operator —
then apply the existing rule unchanged. 0 < r becomes r > 0, 1 <= r becomes
r >= 1, and both land in the carve-out that already exists. It also keeps the direction
logic you just fixed honest under either spelling, which a second special case would not.

Checked against the mirrors that must stay lossy: 1 < r mirrors to r > 1, comparator 1
with >, which is outside the carve-out and stays caught. Correct.

2. An aggregate that is not spelled min

expect.text('yes' if sorted(runs)[0] > 0 else 'no', 'yes', 'n')   ->  MISSED
expect.text('yes' if max(runs) > 0 else 'no', 'yes', 'n')         ->  caught

sorted(runs)[0] is min(runs) written differently, and min(runs) > 0 is caught. So the
aggregate rule is spelling-sensitive.

This is the same shape as any(r <= 0 for r in runs) from round 2 — the natural rewrite of
the thing the guard just refused — but it is a much less natural rewrite than any() was,
and closing it means either enumerating more names or reasoning about what a subscript of a
sort means. I would leave it, and record it beside C, D and E as a known gap rather
than chase it. The argument that made any() worth closing was that an author told "your
min arm is refused" would reach for it; nobody reaches for sorted(runs)[0].

What I checked and found clean

• Both directions of the compound case: x > 0 and len(ys) > 0 is not caught.
• The nested case where the outer comparison is determinate and an inner one hides:
  len([r for r in runs if r <= 0]) == 0 IS caught, which is right — the == 0 says
  nothing about which r.
• max(runs) > 0 caught, so the one-argument aggregate fix from round 2 generalised
  rather than special-casing min.

Three rounds, and what they cost each time

Round 1: two holes, both closed. Round 2: three more, closed, and closing them exposed a
regression introduced in round 1's fix. Round 3: one false positive and one gap, neither
live.

The trend is the useful signal — the findings are getting less severe and less live, which
is what a converging review looks like. I would fix finding 1, record finding 2 with C, D
and E, and stop. I have no further attacks queued.