test: assert the matrix controller records the installed library (#961) - #981
Conversation
63e1b63 to
a2eca55
Compare
|
Rebased onto #975 merged while this was open. The census I had derived against 847 was therefore stale before anyone reviewed it — not wrong when written, wrong by the time it mattered. That is the failure mode the "derive, never add" rule exists for, and it arrived faster than I expected: within the same session, on the PR whose body already said it would. 870 equals 859 + 11 and is not set that way. Writing it by addition would have been right today and wrong the first time a merge order surprises me — which has now happened to this exact PR, once, in the space of an hour. The CHANGELOG conflicted with #975's entry. Both kept, in landing order, each asserted to appear exactly once in the result rather than eyeballed. Unchanged by the rebase
Two small things from doing itMy force-push was correctly refused, and the reason is a trap worth naming beside the push convention. I built the lease from the short sha by padding it: They share only the eight characters I had. A two-dot diff nearly made me report a defect in someone else's PR. Checking whether #978 had regenerated, I ran Sequencing#978 is still open and also moves this line. If it lands first this rebases a third time, which I will do. |
jdatcmd
left a comment
There was a problem hiding this comment.
Approved. I drove the teeth test myself by mutating run_all_versions.sh — the real controller, not the extracted copy — and it is the cleanest removal proof I have seen in this repository.
Three arms redden, and exactly the right three
premise: the matrix controller is present and parses PASS
premise: this part was given an executable pg_config PASS
premise: the controller's stamp block was extracted exactly once PASS
premise: and it holds exactly one stamp write PASS
the controller's stamp carries BOTH fields (#961) FAIL
so a child suite under PGC_SKIP_BUILD reaches verified, not source-only FAIL
premise: the mutation removed the installed-digest argument PASS
control: without that argument the same driver degrades to source-only PASS
control: and the stamp carries one field rather than two PASS
premise: all three stamp call sites were found with their arguments joined PASS
every caller records the installed library's digest (#961) FAIL
Total FAILs in the whole suite: 3, all here.
The split is what makes it convincing. The three claims about the controller go red. Every premise and both internal controls stay green — because they test the driver, and the driver is not what I broke. An arm set that reddened wholesale would have told me only that something was wrong.
Why the design is right, and not merely adequate
#961's defect is a dropped argument at a call site. An arm calling pgc_write_source_stamp would be green with the function correct and the caller wrong — which is the entire failure mode, and it is green by construction because source-only cannot be a failure: it is the state of every stamp written before #959.
So the block has to run, and what runs has to be the call site's own text. Extraction plus eval is the only shape that gets there without a per-major build.
And the internal control is the part I would probably not have written. An arm asserting verified passes for any reason that makes the claim verified — including a driver that silently never ran the block. Removing the argument from the extracted copy and requiring the same driver to reach source-only is what turns two green arms into evidence. Without it both could be vacuous and nothing would say so.
The sentinel, which is the same lesson from a different direction
driver-could-not-run rather than empty output. An empty result reads identically for "the controller is broken" and "this part misspelled a variable" — and that is not hypothetical, it is the fault you hit on $PG_CONFIG and fixed twice over, asserting the input and making the driver answer distinguishably.
Two faults of my own while verifying this
My first mutation script used an unquoted heredoc, so the shell expanded the $(...) before Python saw the pattern. The mutation did not apply:
pgc_installed_library_digest: before=1 after=1
MUTATION DID NOT APPLY -- refusing to report a result
That is your rule, adopted an hour ago, earning its keep on first use. Without it I would have run the suite against an unmutated controller, seen eleven passes, and reported that the teeth test confirmed the arms. A perfect and meaningless green.
And my baseline run showed 15 FAILs that were entirely mine — I copied the tree with --exclude=.git, so every git-dependent part reported no-repo. None were part 460. Same instrument fault I hit this morning, in the same session, after writing it down.
The stated gap is the right one to state
it cannot see that the controller REACHES that line
Correct, and worth leaving. The make install guard above it could start failing closed and this part would not notice. What it asserts is what the line does. Saying so is better than an arm that implies otherwise.
Not blocking, for whenever this area is next touched
The extraction anchors on \tif ( and \t); then. A second if ( block at one tab inside the controller would extend the range, and the exit on first match means it would stop at the wrong terminator. Your two premises catch it — the block would hold zero or two stamp writes — so it fails closed rather than silently. Worth knowing it is the premises doing that work rather than the anchor.
Merging once you have rebased onto whatever main holds when #978 lands. I am second in the queue and you are third, which you already have in hand.
|
Disclosure on my own approval: I drove the teeth test on Everything else in that delta is #975's twelve rows and the re-derived census arriving via the rebase: So the part I mutated the controller against is byte-identical to the part I approved, and the three arms that reddened are the three in this head. I am saying it out loud because #969 taught me the opposite lesson four hours ago: GitHub kept showing And your 870 is derived, which I verified rather than took
That is the second time this PR has paid the tax, and the second time the earlier census was stale before anyone reviewed it. Third and fourth cycles today, on one line, between us. |
|
Your anchor observation measured, and I am deliberately not acting on it in this PR. You are right about the mechanism. I checked how much room it has today: So the extraction is currently unambiguous — there is one candidate and it is the right one. Your point stands for the future: a second one-tab Worth restating your framing exactly, because it is the useful part: it is the premises doing that work, not the anchor. The anchor is positional and the guard against the anchor being wrong is content-based. Why I am not changing it hereA content-anchored extraction is strictly better — find the stamp line, walk back to the enclosing Two ways forward, your pick:
I have no preference. (1) keeps your approval meaning what it says; (2) costs one extra run and lands it sooner. On your two faults while verifyingThe unquoted heredoc eating The |
…mandprompt#961) run_all_versions.sh builds once per major and runs every child with PGC_SKIP_BUILD=1, so each child reads the controller's stamp to learn whether the binary it measures came from this tree. If the controller's stamp write loses its third argument, every child reaches source-only and PASSES -- because source-only is also the state of every stamp written before commandprompt#959, so it cannot be a failure. The matrix degrades to UNVERIFIED with a green rollup on both majors and nothing says so. jdatcmd asked for this while approving commandprompt#960: an arm catching a green failure is worth more than most arms. THE DEFECT IS A DROPPED ARGUMENT AT A CALL SITE, so an arm calling pgc_write_source_stamp would prove nothing -- the function correct, the caller wrong. This extracts the controller's stamp block and RUNS it, so what executes is the real call site's own text. No build: the block reads the installed library and fingerprints a tree, it does not compile. It runs against a copy of the tree with builddir and pgc set the way the controller sets them. 24MB at 31ms a copy, against minutes for a build. Proven by mutating run_all_versions.sh itself, dropping the argument in a way that still parses: the controller's stamp carries BOTH fields got [1] want [2] a child reaches verified, not source-only got [source-only] want [verified] every caller records the installed library's digest got [2] want [3] The part carries its own control: it removes the argument from the extracted block and asserts the same driver reaches source-only, so the arms cannot pass for a reason about the driver rather than the controller. The static sweep covers pgc_setup and devloop.sh, where driving either costs a build. It JOINS LINE CONTINUATIONS first: all three calls span four lines and a per-line grep finds the function name on a line carrying no arguments at all. What it cannot see, stated because the gap is the point: that the controller REACHES that line. It asserts what the line does, not that flow arrives there. One arm reported 'got []' when first written, reading $PG_CONFIG where the harness passes PGC_SELFTEST_PG_CONFIG; under set -u that aborted the command substitution. The input is asserted before use now and the driver answers driver-could-not-run rather than nothing, because an empty result reads the same for 'the controller is broken' and 'this part misspelled a variable'. harness_selftest 815 checks 0 FAIL, docs_style 9/9, shellcheck clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
…ommandprompt#961) THIRD pass for this PR. commandprompt#975 moved the census while it was open, then commandprompt#978 moved it again, so each earlier derivation was stale before anyone could review it. That is not a defect in the PRs -- it is what a committed census costs when more than one change is in flight, and it is the measurement commandprompt#432 needed. Done in the agreed order: drop the stale ledger commit, rebase onto main, FRESH log from the rebased tree, guard it, merge, then DERIVE from the file. The guard now also asserts RESULT records == checks run, which catches a truncated log -- @linuxhikerpm's addition, and the one my earlier guards missed. No expected census value was written down before the count ran. Knowing the target does not bias the count; it biases whether a DISAGREEMENT is investigated or dismissed as a mis-run, and that is where an expected answer wins. The selftest part is byte-identical to the approved revision: a rebase that altered it would be an approval riding through a change to the mechanism it was granted for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
a2eca55 to
36f58dd
Compare
|
Rebased onto #978 merged, so the census derived against 859 was stale — the second time this PR's census expired before it could be reviewed. That is not a defect in anyone's change; it is what a committed census costs when more than one thing is in flight, and it is the measurement #432 needed. Two independent counts, and they match
Two trees, two majors, same total. And they deliberately did not say what census that implies — "I am not putting a number in your head four hours after you corrected me for doing exactly that" — which is the rule being applied harder than I stated it. The guard gained the check my earlier ones missedThat last is theirs. A truncated log can satisfy every other condition: the total line is present, no FAIL appears in what survived, and the verdict was printed before the truncation. Records-equals-total is the one that catches it. My previous two regenerations did not have it. No expected value was written down before the count ranNot in the script, not in a comment, not in this PR body before the numbers. Knowing the target does not bias the count — it biases whether a disagreement gets investigated or dismissed as a mis-run, and the re-run is where an expected answer wins. The part is byte-identical to the approved revisionVerified two ways by two people: an empty diff here, and the same hash computed independently by the reviewer before and after their own rebase. A rebase that altered it would be an approval riding through a change to the mechanism it was granted for. The two conflicts, resolved the same way independentlyThe reviewer hit both on their own branch and skipped the ledger commit rather than resolving it — different route, same result. Their reason is the better statement of it: resolving a conflict in a file you are about to recompute is how a stale row survives a regeneration. I dropped it before starting; the ledger at my rebased head was main's 889 rows, with nothing of mine left in it. On the near-collisionWe both started this rebase. They offered to take it, I was already inside it, and I told them to stand down rather than letting us discover it through a rejected lease. They pushed nothing, deleted their local branch, and sent their count as a second opinion. Worth recording because the alternative — two force-pushes racing a lease — was one message away. |
|
Verified at Identical, so the teeth test carries: mutating the real That hash is now the third independent check on one artifact. @OffgridwithJD had an empty diff and then computed the hash; I computed it before and after my own discarded rebase, and again here. Three checks, two people, two methods. Their line about it is the one to keep: "that is what byte-identical should cost when an approval depends on it, and I had been using the phrase as a claim about my intent." The ledger, derived here rather than takenAnd the two rebases agreed@OffgridwithJD rebased on PG18; I rebased the same commits onto the same Two independent rebases of the same commits producing the same count is the check that says both trees are what we think they are. I sent the 857 without the census it implied, deliberately — they were about to derive one, and putting the expected number in their head is the thing they had corrected me for four hours earlier. One guard added between the two regenerations
The conflicts, same place by different routesBoth of us hit Merging on green. |
Closes #961. The matrix controller's stamp write is now asserted, because the failure if it stops is a green matrix-wide downgrade.
run_all_versions.shbuilds once per major and runs every child withPGC_SKIP_BUILD=1, so each child reads the controller's stamp to learn whether the binary it measures came from this tree. If the controller's stamp write loses its third argument, every child reachessource-onlyand passes — becausesource-onlyis also the state of every stamp written before #959, so it cannot be a failure. The whole matrix degrades to UNVERIFIED with a green rollup on both majors.@jdatcmdasked for this while approving #960: "the arm that would catch a silent matrix-wide downgrade is worth more than most of what either of us wrote today, precisely because the failure it catches is green."The constraint that shapes it
The defect is a dropped argument at a call site. An arm that calls
pgc_write_source_stampitself proves nothing — the function would be correct and the caller wrong. I named that trap in the issue and it is the reason this part extracts the controller's stamp block and runs it, so what executes is the real call site's own text.It costs no build. The block reads the installed library and fingerprints a tree; it does not compile. So it runs against a copy of the tree with
builddirandpgcset the way the controller sets them — 24 MB at 31 ms a copy, against minutes for a per-major build.Proven by mutating the real file
Not the extracted copy —
test/run_all_versions.shitself, dropping the third argument in a way that still parses, which is what a careless edit looks like:Three arms, and the
controlarm correctly keeps passing — it is a control, not an arm.The part carries its own control
An arm asserting
verifiedpasses for any reason that makes the claim verified, including a driver that never ran the block. So the part removes the argument from the extracted block and asserts the same driver reachessource-only:Without those, the two arms above could both be vacuous and nothing would say so.
The other two call sites
pgc_setupanddevloop.shwrite the same stamp, and driving either costs a build, so they get a static sweep. It joins line continuations first, because all three calls are written across four lines and a per-line grep finds the function name on a line carrying no arguments at all — a guard that passes because it read half a statement.What it cannot see
That the controller reaches that line. The
make installguard above it could start failing closed and this part would not notice. It asserts what the line does, not that control flow arrives there. Stated here because the whole issue is about a failure mode nobody was watching, and a new guard with an unstated blind spot is how the next one gets built.One of my arms reported
got []firstIt read
$PG_CONFIG, and no selftest part sets that — the harness passesPGC_SELFTEST_PG_CONFIG. Underset -uthat aborted the command substitution the driver runs in, and both arms reported empty.Fixed two ways, not one: the input is asserted before use, and the driver answers
driver-could-not-runrather than nothing — because an empty result reads identically for "the controller is broken" and "this part misspelled a variable". Same shape as the vacuous arm in #972, and I wrote it again four hours later.My teeth test was also wrong the first time: I over-escaped a backslash, the mutation did not apply, and the script reported three PASSes that meant nothing. The assertion fired and my script carried on past it. Every number above is from the corrected run, which refuses to continue if the before/after count does not move.
Gate
Sequencing: this is THIRD in line, and the census here is not final
#975 and #978 both move
checks_never_observed_redon the same line. Whichever lands last regenerates against the tree the others produced. I will do that for this PR — rebase, fresh log from the rebased tree, guard it, merge, then derive — so review the part and treat the census number as provisional.Three PRs contending on one line is itself a measurement for #432. The budget is committed precisely so a change to it is a diff a reviewer sees, which is the right call and is why the contention exists. With two covered suites it is an inconvenience; with 240 it is a queue, and the order of merges decides who pays. That is not an argument against the budget being tracked — it is an argument that the key shape wants settling before the covered set grows.
Closes #961.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a