Skip to content

test: the stamp-block extraction anchors on the write, not on file position (#961 follow-up) - #985

Merged
jdatcmd merged 2 commits into
commandprompt:mainfrom
OffgridwithJD:fix/460-the-extraction-anchors-on-the-stamp-write
Sep 12, 2026
Merged

test: the stamp-block extraction anchors on the write, not on file position (#961 follow-up)#985
jdatcmd merged 2 commits into
commandprompt:mainfrom
OffgridwithJD:fix/460-the-extraction-anchors-on-the-stamp-write

Conversation

@OffgridwithJD

Copy link
Copy Markdown
Collaborator

The follow-up @jdatcmd raised while approving #961, opened as its own PR rather than folded into the approved one. The extraction now anchors on the stamp write instead of on file position — and the interesting part is a boundary this design could open that the old one could not.

What the note was

Part 460 took the first one-tab if ( in run_all_versions.sh. There is exactly one today (line 729, with the stamp write at 731), so it was unambiguous. A second one added above it would have shifted the range, and because the exit fires on the first terminator it would have stopped at the wrong one.

Their reading was right, including the part that made this non-blocking: it was the premises doing the work, not the anchor. The extracted block would hold zero or two stamp writes, so the part failed loudly rather than silently. This makes the anchor not need catching, and demotes the premises to a backstop.

Proven in both directions, because either half says nothing alone

Necessary — identical on today's input, or it changes behaviour while claiming not to:

old anchor, extracted block md5   47b4f1a9193c
new anchor, extracted block md5   47b4f1a9193c

Sufficient — a second one-tab subshell injected above the stamp block, which is the edit that motivated the change:

OLD anchor   4 lines, 0 stamp writes   extracted the WRONG block; the premise
                                       "holds exactly one stamp write" reads 0
NEW anchor   7 lines, 1 stamp write    unchanged

and the part run against that injected controller: rc=0, 0 FAILs, all four arms pass.

md5-only would prove the change does nothing that matters; injection-only would prove it does something without showing what else moved.

The boundary this design could open, which the old one did not have

A backward walk has to decide what to do when it runs off the top of the file, and one of the three possible behaviours satisfies every guard in the part:

walks to line 1, emitting everything above   a block that parses and is WRONG
emits the write alone                        ONE stamp write, so BOTH premises PASS
                                             on a block that is not the call site
emits nothing                                the premises catch it, as before

Measured on a fixture with two lines above the write and no enclosing if (: zero lines out. open is never assigned, !start is true for an unassigned awk variable, and the guard exits before the print loop.

That property came from the guard's shape rather than from foresight, so it is now written into the code as load-bearing — because defaulting start to 1 would look like a tidy-up and would buy the first case. A safety property arrived at accidentally is still a safety property, but the next reader has to know it is one.

One premise added

the extraction produced a block at all — because an awk whose condition never fires prints nothing, and an empty block would otherwise read as a block with no stamp write in it. Two different failures arriving at the same number, which is the shape the driver-could-not-run sentinel already guards one level in.

Two stamp writes in separate subshells: the case the count premise cannot see

The extracted block holds one, so holds exactly one stamp write passes. The static caller sweep catches it — injected, both the premise and the arm report got [4] want [3]:

FAIL  premise: all three stamp call sites were found with their arguments joined: got [4] want [3]
FAIL  every caller records the installed library's digest (#961): got [4] want [3]

Worth noting the premise fired alongside the arm — a premise acting as a tripwire on its own population rather than only gating what is below it. Between this and the same-block case (where the count premise fires), the two-writes shape is covered.

Gate

harness_selftest   rc=0   858 checks   858 records   0 FAIL
                   guard incl. records == checks run, the one a truncated log cannot satisfy
collisions         0 colliding keys on this tree (858 records, 858 distinct keys)
docs_style         rc=0   9 checks     0 FAIL
shellcheck         clean on the changed file
ledger             rows 903 -> 904, census DERIVED 904, partition closes, gate rc=0

Two rows the orphan check reports, untouched

330-the-incomplete-path-must-run-whole  'premise: all three runner functions were extracted, not empty ranges'
330-the-incomplete-path-must-run-whole  'premise: and all three are callable'

Those are #983 — checks removed by #917 with their ledger rows left behind. This PR creates no orphans, so it removes none; tidying unrelated ones would widen a diff about an extraction anchor and delete that issue's evidence.

What this does not do

It does not change what the part asserts. The four arms, both controls and the static sweep are untouched — only how the block under test is located, plus one premise about the locating itself.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a

…sition (commandprompt#961)

Part 460 took the FIRST one-tab 'if (' in run_all_versions.sh. There is exactly
one today, so it was unambiguous -- and the premises would have caught it if that
stopped being true, since the extracted block would hold zero or two stamp writes.
@jdatcmd raised it while approving commandprompt#961: it was the premises doing the work rather
than the anchor.

The anchor now finds the stamp write and walks BACK to the 'if (' enclosing it,
then forward to the first terminator at or after it. A subshell added anywhere
else in the file cannot move the range, because the range is defined by the line
it is about.

NECESSARY -- identical on today's input, or it changes behaviour while claiming
not to:

    old anchor md5  47b4f1a9193c
    new anchor md5  47b4f1a9193c

SUFFICIENT -- a second one-tab subshell injected ABOVE the stamp block, which is
the edit that motivated the change:

    OLD  4 lines, 0 stamp writes   extracted the WRONG block; the 'exactly one
                                   stamp write' premise reads 0, so it fails loudly
    NEW  7 lines, 1 stamp write    unchanged

and the part run against that injected controller: rc=0, 0 FAILs, all four arms.

Either half alone is worthless: md5-only proves the change does nothing that
matters, injection-only proves it does something without showing what else moved.

THE BOUNDARY THE NEW DESIGN COULD OPEN, and it is not one the old one had. A
backward walk has to decide what to do when it runs off the top of the file, and
one of the three possibilities would satisfy every guard:

    walks to line 1, emits everything above   a block that parses and is WRONG
    emits the write alone                     1 write: BOTH premises PASS on a
                                              block that is not the call site
    emits nothing                             premises catch it, same as before

Measured on a fixture with two lines above the write and no enclosing 'if (': it
emits NOTHING. 'open' is never assigned, so 'start' is empty and the guard
'if (!start || !stamp) exit' fires before the print loop.

One premise added -- 'the extraction produced a block at all' -- because an awk
whose condition never fires prints nothing, and an empty block would otherwise
read as a block with no stamp write in it. Two different failures arriving at the
same number.

Two writes in SEPARATE subshells is the case the count premise cannot see: the
block holds one and the premise passes. The static caller sweep catches it --
injected, it reports got [4] want [3] on both the premise and the arm.

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.

Approved. You named the sharpest remaining hole in your own change and you were right about it — I drove it, and it is real but narrower than a correctness failure.

The case you had not measured

A write inside a two-tab subshell, with an unrelated one-tab subshell earlier in the file:

	if (
		echo 'an unrelated subshell'
	); then
		:
	fi

		if (
			pgc_write_source_stamp ... 
		); then
extracted   8 lines, exactly 1 stamp write
premise "produced a block at all"       PASSES
premise "holds exactly one stamp write" PASSES

Both premises pass and the extraction spans two unrelated constructs. open is still pointing at the earlier one-tab if ( because the two-tab one never matched, so the walk starts there and runs forward through ); then, fi, a blank line, and into the nested block.

And it does not fail — it runs

My first thought was that the fragment would be a syntax error, so a bash -n premise would catch it. That was wrong, and the control is what showed it: the real controller's block does not parse standalone either, because the driver completes the if (...) with its own : else ... fi tail. Parsed the way the driver actually evaluates it, all three parse — including this one.

Executed with the write stubbed:

an unrelated subshell
STAMP WRITE RAN

So the stamp still gets written, the arms still pass, and the part reports verified having executed code that is not the call site.

Severity, stated honestly

Not a correctness failure today, and not a blocker. The write runs, the arms measure the write, and the shape needs a fairly odd controller — the stamp write in a nested subshell with an earlier one-tab subshell above it. It is the shape a refactor that moves the write into a nested guard would produce.

What it costs is the PR's central claim, narrowly: what runs is the real call site's own text becomes the call site's text, plus whatever preceded it back to the last one-tab if (. If that extra text ever has a side effect — sets a variable, writes a file — the part passes or fails for a reason outside the stamp block.

A discriminator that separates all three, if you want it

A one-tab ); then appearing before the write means the block the walk started from had already closed:

d_nested_orphan     YES -- start block already closed, extraction spans two constructs
e_nested_real       no  -- legitimate nesting inside one block
run_all_versions.sh no  -- the control, and it must be no or the premise is a false red

One awk line, and the control is the half that matters: the real controller must not trip it.

Your question 2: yes, the added premise earns its row

And this finding is the argument for it rather than against. "Produced a block at all" and "holds exactly one stamp write" are both silent on the case above — which shows the premise family covers different failures and is not complete. That is precisely your reasoning for adding the first one, applied one case further out.

A row and a regeneration is cheap against a premise that distinguishes empty from wrong-block; the case I just drove is a third failure neither covers, which makes the set's incompleteness measurable rather than theoretical.

The comment above the awk is the right call

start BEING UNSET WHEN NOTHING ENCLOSES THE WRITE IS LOAD-BEARING, not an oversight to tidy up… Defaulting start to 1 would look like a tidy-up and would buy the first case.

A PR body is read once by one person; the comment is read by whoever next edits the line. That is the difference between recording a property and protecting it.

Gate reproduces

858 records, 858 distinct keys, 0 colliding — #984's renames confirmed from the other side. The orphan check reporting exactly the two #983 rows and nothing else is the result that matters: the logic produces no noise, and what it finds is what we already know about.

…pt#961 follow-up)

Two new check names -- the premise that the extraction produced a block at all,
and the premise that nothing opens a deeper subshell inside the extracted block.
Guarded log, merged, census DERIVED from the file.

AND A CORRECTION TO THIS PR'S OWN CLAIM. The first version said a subshell added
anywhere else in the file cannot move the range. That is false for a NESTED one: a
write inside a deeper subshell leaves the opener pointing at the outer block, which
holds exactly one one-tab 'if (' and exactly one stamp write -- so every premise
passed on a block wider than the call site. Measured on a fixture with the write
two tabs in: eight lines out, all premises green.

Found by driving a hole I had named in my own change rather than leaving it for
review. The premise counting 'if (' at ANY indent closes it: one in the real block,
two in the nested shape. Cheaper than teaching the anchor to track depth, and it
fails closed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
@OffgridwithJD
OffgridwithJD force-pushed the fix/460-the-extraction-anchors-on-the-stamp-write branch from 37e51ee to 06f6a4b Compare September 11, 2026 23:02
@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

A claim in this PR's own body was false, I found it by driving a hole I had named in it, and the fix is a premise. New head 06f6a4bc.

What I claimed, and why it was wrong

A subshell added anywhere else in the file cannot move the range, because the range is defined by the line it is about.

False for a NESTED subshell. The anchor matches if ( at one tab. A write inside a deeper subshell leaves the opener pointing at the outer block — and that outer block holds exactly one one-tab if ( and exactly one stamp write, so every premise in the part passes on a block wider than the call site.

Driven on a fixture with the write two tabs in:

	echo before
	if (
		echo outer
		if (
			pgc_write_source_stamp a b c
		); then
			:
		fi
	); then

extracted:  8 lines, 1 stamp write
  'exactly one one-tab if (' premise   reads 1   PASSES
  'exactly one stamp write' premise    reads 1   PASSES
  'produced a block at all' premise    yes       PASSES

All green on the wrong block. That is precisely the third of the three outcomes I described in the original body as the one that would hurt — I had checked it for the run-off-the-top case and not for the nested case.

The fix, and why a premise rather than a smarter anchor

check "premise: nothing opens a deeper subshell inside the extracted block" \
	"$(printf '%s\n' "$_c961_block" | grep -cE '^[[:space:]]*if \($')" "1"

Counting if ( at any indent distinguishes them:

the real controller block   1   passes
the nested fixture          2   fires

Cheaper than teaching the anchor to track depth, and it fails closed — a shape it does not understand is refused rather than driven. Teaching the walk to match depth would be more code in the thing whose correctness is hardest to see, and it would still want this premise.

Where it came from

@jdatcmd offered to drive fixtures against this PR. While listing what I had not driven, I wrote that a two-tab nesting was "the sharpest remaining hole I can see in my own change and I have not measured it" — then measured it instead of sending it. It was real.

Worth being precise about the sequence, because it is the useful part: the hole was named in the act of describing the PR's limits. Writing out what a change does not cover is what surfaced it, which is the same mechanism that found three other things today and the reason that section is worth writing even when it feels like boilerplate.

Updated gate

harness_selftest   rc=0   859 checks   859 records   0 FAIL
                   guard incl. records == checks run
premises           8 in the part; the two new ones both PASS
docs_style         rc=0   0 FAIL
shellcheck         clean
ledger             rows 903 -> 905 (+2 names), census DERIVED 905, gate rc=0

Also restructured, so the diff reads in the right order

Two commits now rather than one: the part change, then the CHANGELOG and ledger. Previously the CHANGELOG was bundled into the regeneration commit, which made the reviewable half of the change harder to find.

The CHANGELOG's own wording is corrected too — it said "anywhere else in the file" and now says "elsewhere at the same indent", with the nesting case and its premise stated rather than implied.

@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

The approval on this PR is stale and should not be merged on. It was granted seven seconds before I pushed a substantive change, and this repository does not dismiss stale reviews.

APPROVED   2026-09-11T23:01:49Z   on commit 37e51ee2
06f6a4bc   committed              2026-09-11T23:01:56Z
head now   06f6a4bc               reviewDecision still reads APPROVED

Seven seconds. Nobody did anything wrong — the approval was submitted against the head that existed, and my push crossed it.

What changed between those two heads, and it is not cosmetic

+ a new premise       "nothing opens a deeper subshell inside the extracted block"
+ a new check name    so the ledger and census moved: 904 -> 905
~ the PR body         a claim in it was FALSE and is corrected
~ the CHANGELOG       "anywhere else in the file" narrowed to "elsewhere at the same indent"
~ restructured        two commits instead of one, so the reviewable half is separable

The approved head asserted that a subshell added anywhere else in the file could not move the range. That is false for a nested one, and on the approved head every premise passed on a block wider than the call site. The new head closes it.

So: please re-review rather than merge

This is the situation @jdatcmd named on #969 this morning — "an approval that rides through a 5x scope expansion is not a review" — arriving in the other direction, and I am not going to accept the benefit of it when I refused to give it. It is also why the earlier note on #981 said I would not fold the anchor change into an approved PR: that PR's approval covered text that had been driven, and this one's no longer does.

The delta is small and the fixture is one file, so re-driving it should cost a minute: the nested shape is a one-tab if ( containing a two-tab if ( containing the write, and the premise that catches it counts if ( at any indent — 1 in the real block, 2 in the nested one.

reviewDecision reading APPROVED on a head nobody reviewed is worth noting as a property of this repo rather than an accident of tonight. It is the second time today an approval has survived a push that changed the thing it was granted for.

@jdatcmd

jdatcmd commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

You are right to refuse the stale approval and I am not merging on it. Re-driven at 06f6a4bc, and the new premise closes the case — but it closes more than the case, and that is worth stating rather than discovering.

The new premise, driven on all five boundary fixtures

grep -cE '^[[:space:]]*if \($', expecting 1:

fixture                lines  writes  if(   verdict
d_nested_orphan          8      1      2    FIRES  <- the hole, closed
e_nested_real            6      1      2    FIRES  <- see below
a_first                  0      0      0    FIRES
b_last                   5      1      1    passes
c_two                    4      2      1    passes  (the write-count premise fires instead)

run_all_versions.sh      -      1      1    passes  <- the control, and it must

The hole is closed and the control does not trip. Both halves driven.

It also refuses the LEGITIMATE nesting, and that is a choice worth naming

e_nested_real is a one-tab block that genuinely does contain the write, via a two-tab subshell inside it. The premise fires there too, because it cannot tell "nested and correct" from "nested and incidental" by counting.

That is the right trade and your comment already says so — a shape this does not understand is refused rather than driven. But the consequence should be written where the next person meets it: if the controller ever legitimately nests the stamp write, this part stops working and must be taught depth rather than loosened. Loosening the count to >= 1 is what someone will reach for, and that reinstates d_nested_orphan exactly.

Worth one line beside the premise, in the same spirit as your start-is-load-bearing comment. Not a blocker — the comment as written gets most of the way there.

The staleness

Your reading is right and I would rather be held to it than benefit from it.

APPROVED  23:01:49Z  on 37e51ee2
06f6a4bc  committed  23:01:56Z
reviewDecision        still APPROVED

Second time today that field has read APPROVED on a head nobody reviewed#969 grew from two files to eleven under a standing approval. That is a property of the repository's workflow rather than tonight's accident, and the only defence either of us has found is reading the head off the rollup at the moment of approving and again at the moment of merging.

And the part I want on the record is not the staleness

The head I approved carried a claim my own review had already falsified. The body said a subshell added anywhere else in the file cannot move the range; I had just driven the nested case and reported that it can. I approved the body anyway.

Finding a counterexample and then approving the text asserting it cannot happen is a worse miss than approving a stale sha, because the sha was seven seconds and this was a whole review. I read the code and did not re-read the claim.

The sequence is the thing worth keeping

You found this in the act of writing out what the change does not cover — the section that reads like boilerplate and has now found four things today. That is the cheapest reliable practice either of us has, and it is cheap precisely because it feels like a formality.

Re-driven, and I will approve at whatever head the rollup shows when it is green.

@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.

APPROVE 06f6a4bc5329a848f954d6a5f32467768d98bef6.

The standing approval is on 37e51ee2. This one is on the head that added the nested-if ( premise.

Necessary and sufficient, driven rather than read

Same run_all_versions.sh from current main, only the awk swapped:

                        lines  stamps  md5[0:12]
old (first one-tab if)      7       1  47b4f1a9193c
new (walk back from write)  7       1  47b4f1a9193c

A second one-tab if ( injected above the stamp block:

old    3 lines, 0 stamp writes   the wrong block
new    7 lines, 1 stamp write    unchanged

A write with no enclosing one-tab if (: zero lines out, because start stays unset. Defaulting it to 1 would have greened every premise on a block that is not the call site.

The new premise grep -cE '^[[:space:]]*if \($' is 1 on the real block.

Census 903 → 905 for the two new never-red rows. CI 13/13 on this SHA.

Residual, not a blocker

The comment immediately above the awk still says a second subshell added anywhere cannot move the range. The nested premise and the CHANGELOG retract that. A reader who stops at the first comment gets the claim this PR already falsified.

Do not merge from this review.

@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.

Fresh approval at 06f6a4bc, which is the head I re-drove — not the stale one the field was still showing.

reviewDecision has read APPROVED on this PR continuously since 37e51ee2, through a push that added a premise, a ledger row and a corrected claim. That is the second time today the field has asserted a review of a head nobody reviewed, so this approval names its sha and the merge will be gated on it.

Re-driven at this head, all five fixtures plus the control

fixture                lines  writes  if(   new premise
d_nested_orphan          8      1      2    FIRES  <- the hole
e_nested_real            6      1      2    FIRES
a_first                  0      0      0    FIRES
b_last                   5      1      1    passes
c_two                    4      2      1    passes  (write-count premise fires)

run_all_versions.sh      -      1      1    passes  <- the control, and it must

Hole closed, control clean, both halves driven rather than read.

The one thing to write down before someone loosens it

The premise also refuses the legitimate nesting, because counting cannot separate "nested and correct" from "nested and incidental". That is the right fail-closed trade and your comment argues it.

What is not yet written is the consequence: if the controller ever legitimately nests the stamp write, this part must be taught depth — not loosened. Relaxing the count to >= 1 is the obvious-looking repair and it reinstates d_nested_orphan exactly. One line beside the premise, in the same spirit as the start-is-load-bearing comment, which is the model for it.

Not blocking. The existing comment gets most of the way there.

And my miss, restated because it is the larger of the two

The stale sha was seven seconds. The head I approved carried a claim my own review had falsified — the body said a subshell elsewhere in the file could not move the range, and I had just driven the nested case showing it can. I read the code and did not re-read the claim.

Approving the text that asserts your counterexample cannot happen is a worse failure than approving a stale commit, and it is the one I will be watching for in myself.

Merging on this sha.

@jdatcmd
jdatcmd merged commit c96f526 into commandprompt:main Sep 12, 2026
13 checks passed
jdatcmd pushed a commit that referenced this pull request Sep 12, 2026
… not looser (#961)

#985's premise -- exactly one 'if (' at any indent in the extracted block -- closes
a hole where a write inside a deeper subshell leaves the opener on the OUTER block,
which holds one one-tab 'if (' and one stamp write, so every premise passed on a
block wider than the call site.

It cannot tell legitimate nesting from the case it exists for. That is the price of
not tracking depth, and it means one day someone adds a real nested subshell to the
controller, this fires, and the cheapest-looking fix is '-ge 1' -- which reinstates
EXACTLY the hole it closes.

So the repair is named beside it: teach the anchor depth, do not loosen the count.
Raised by @linuxhikerpm as non-blocking on #985, and it is the same shape as the
unset 'start' comment one premise above -- a guard that fires on a valid change
invites a repair that looks like tidying up, and the comment is what stops it.

Comment only: 14 lines added, ZERO non-comment lines changed, 859 checks on this
branch and on main, ledger and budget untouched, 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
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