fix(test): the intake audit could clear intake over an empty comparison (BACKLOG #1292) - #599
Open
wshallwshall wants to merge 17 commits into
Open
fix(test): the intake audit could clear intake over an empty comparison (BACKLOG #1292)#599wshallwshall wants to merge 17 commits into
wshallwshall wants to merge 17 commits into
Conversation
…E URGENT STOP NOT A FINISHED CHANGE AND NOT REVIEWED. Committed mid-flight because the usage ladder reached URGENT STOP, where the instruction is to commit whatever exists rather than risk losing a tree. A partial commit beats a lost one; an unreviewed commit on an unpushed branch costs nothing and can be amended. WHAT THIS IS: the BACKLOG #1292 store-side discriminator, being built to separate a harness reconciliation race from a real count-and-log intake loss. Its design stage returned confidence NOT-ESTABLISHED, which was the honest answer and the one I told the agent was acceptable -- the deliverable is an instrument that ATTRIBUTES the next occurrence, not a verdict on which branch is real. STATE, stated plainly so nobody mistakes this for finished work: - the implement stage was STILL RUNNING when this was committed - the ADVERSARIAL REVIEW STAGE HAD NOT RUN AT ALL - I have NOT verified any of it: no red-first, no ruff, no mypy, no test run by me - the two new modules (intake_audit.py, test_connscale_intake_audit.py) may be incomplete DO NOT LAND THIS. It needs the review stage, then my own verification, then almost certainly a correction commit -- the reviews of my last two commits each found a real defect, one of which let a genuine FD collapse pass. Assume the same here until shown otherwise. Also carried, unrelated and equally unverified: whatever the agent touched in driver.py, profile.py, report.py, runner.py, sender.py, docs/LOAD-TESTING.md and harness/load/__init__.py. The ledger is deliberately untouched, as it has been in every commit on this branch.
…on, and blamed the gauge by guessing
The adversarial review the wip commit said this needed has now run: six dimensions, each finding
handed to a refuter. 17 findings raised, 8 survived, 9 refuted as overstated or wrong. Then a
quality pass found a further defect in my own fix for one of them. Every fix below is verified
red-first -- the guard is broken, the test is watched to fail, the guard restored.
THE HIGH ONE. judge()'s vacuity guard tested `ledger.total` (confirmed + unconfirmed) while every
finding branch consumes only `ledger.confirmed`. A step where nothing was ever ACK-confirmed
therefore passed all six blindness guards and returned a CONCLUSIVE verdict -- "the shortfall is
not in intake" -- computed over a ZERO-ELEMENT comparison, with a green SLO row and all four smoke
assertions passing. The existing positive control guards the STORE side; nothing guarded the
LEDGER side. It is reachable on the harness's own headline fault, because the runner's excusal
clamps `excused` to 0 over budget. This is the blind-but-green direction the whole ordering exists
to prevent, and it cleared intake on precisely the step the reconcile calls a possible
accepted-and-dropped.
THE ONE I GOT WRONG FIRST, RECORDED BECAUSE THE SHAPE MATTERS. SAMPLING_LAG named the `engine_read`
gauge by pure elimination. My first correction inferred the missing state inside judge() by asking
`read_short <= len(ledger.unconfirmed)`. That inference holds only OVER budget. IN budget
`excused == unconfirmed`, so the unconfirmed sends are ALREADY subtracted out of `read_short` and
any residue is confirmed sends the gauge did not count -- a real finding my predicate silenced. And
over-budget needs `timeouts > 3/4 sent`, so the case I broke is the COMMON one. Measured against the
real producer arithmetic (sent=100, timeouts=5, read=93): old predicate `0 < 2 <= 5` fires and
silences; corrected predicate names the gauge. The fix is at the producer instead --
`_unexplained_shortfall` computes `sent - unconfirmed - read`, which means the same thing in both
worlds, and judge() consumes it rather than reconstructing budget arithmetic it does not own. An
absent value falls back to naming the gauge, so the unsafe direction is never the default.
ALSO FIXED, all confirmed by the review:
- engine_suspect now requires the POST-MORTEM moment. A live sweep pages over a store still being
written and can manufacture INVARIANT_SUSPECT; the prose already hedged that, but the machine
surface did not, and only a distant call site's choice of field kept it honest.
- the live moment reports an absent row WITHOUT the engine conclusion (`_conclusion`), so a
console line or JSON artifact cannot be quoted as an invariant violation the post-mortem
does not support.
- the live NOT_RUN reason said "no intake shortfall to attribute" when the audit was merely
DISABLED -- a false statement about the run, in the field whose only job is attribution, on
exactly the step someone opens the artifact to read.
- two docstrings promised full control ids "stay in the harness log line". Nothing logs one. False
in both directions: a reader finds none, and a maintainer reconciling prose against code would
make it true by logging exactly what the PHI rule exists to keep out.
TWO TESTS THAT PASSED WITH THE FEATURE BROKEN, which is the defect class this branch has now been
corrected for three times. The overflow test asserted only the verdict against an empty snapshot, so
a neighbouring guard produced the same PROBE_UNUSABLE and it passed with `if ledger.overflow:`
deleted outright. CORRELATION_SUSPECT had a positive test but no negative control, so dropping the
store-membership half of its predicate kept the suite green while turning every ordinary NAK into a
standing finding. Both now assert the discriminating detail and both go red when broken.
BEHAVIOUR CHANGE, STATED SO IT IS NOT A SURPRISE: a step where nothing was ever confirmed now
returns PROBE_UNUSABLE, which is not `conclusive`, so `_assert_intake_audit` FAILS there. It
previously passed with a vacuous clean verdict. That is the intended direction -- the item's own
text says a probe that cannot answer is a defect to classify deliberately, not a gap to excuse --
but it does mean this test can now red where it was falsely green.
Quality pass also collapsed three near-identical `_build` call sites into a `_matched` closure
beside the existing `_unusable` one, hoisted the duplicated counts, and gave the two audit moments
one spelling of the disabled reason so they cannot drift into disagreeing about why nothing ran.
VERIFIED: ruff check + format, mypy strict, 33 intake-audit tests, 241 connscale tests, and the
connscale end-to-end smoke (2 passed) -- which exercises the new verdict semantics for real rather
than at unit level. Red-first on all six new or repaired guards, including one that reproduces the
predicate bug I introduced. Glyph scan clean over 403 added lines against a 736-hit positive
control. The ledger stays untouched, as on every commit on this branch.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
wshallwshall
enabled auto-merge (squash)
August 25, 2026 21:17
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The store-side discriminator that separates a harness reconciliation race from a real count-and-log
intake loss (an engine that accept-ACKed a message and then has no committed-store row for it).
Adversarial review found the blocking defect: `judge()`'s vacuity guard tested `ledger.total`
(confirmed + unconfirmed) while every finding branch consumes only `ledger.confirmed`. A step where
nothing was ever ACK-confirmed passed all six blindness guards and returned a CONCLUSIVE verdict --
"the shortfall is not in intake" -- computed over a zero-element comparison, with a green SLO row and
all smoke assertions passing. The existing positive control guarded the store side; nothing guarded
the ledger side, and it's reachable on the harness's own headline fault (the runner's excusal clamps
`excused` to 0 over budget).
Also fixed, all confirmed by the review: a predicate that inferred the missing-gauge state only held
over-budget and silenced a real finding in the common in-budget case (corrected at the producer,
where `_unexplained_shortfall` computes the same quantity in both worlds); `engine_suspect` now
requires the post-mortem moment rather than a live sweep that can manufacture a false positive; the
live NOT_RUN reason no longer claims "no shortfall to attribute" when the audit was simply disabled;
and two docstrings that promised control ids "stay in the harness log line" when nothing logs one.
Two tests that passed with the feature broken were corrected to assert the discriminating detail
instead of just the verdict shape, so both now go red when broken rather than passing by
coincidence.
This landing required a hand-merge, not a clean cherry-pick: main has moved since this branch was
cut, and `tests/test_connscale_smoke.py` was independently restructured by #1331's landed work
(the monolithic test split into one-name-per-property functions with a shared module-scoped
fixture). Reconciled by giving the intake-audit assertion its own named test function
(`test_no_accept_acked_message_is_absent_from_the_stopped_engines_store`) rather than folding it
into an existing one, consistent with #1331's own "one name per property" rationale. Verified the
merge itself, not just each side independently: all 10 tests in the smoke module pass against real
spawned engine subprocesses, including the new function.
Verified locally: 33/33 in `test_connscale_intake_audit.py`, 21/21 in `test_connscale_cpu_probe.py`,
10/10 in the hand-merged `test_connscale_smoke.py`, ruff check + format clean, mypy strict clean on
the new module.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com