test: a count grep never produced is not "present" (#929) - #941
Conversation
commandprompt#922 replaced roughly 28 `producer | grep -q PAT` tests with `[ "$(grep -c PAT ... || true)" != 0 ]`, which fixed a real EPIPE race (commandprompt#486). The replacement answers PRESENT where the original answered ABSENT whenever grep produces no stdout, and a pattern that does not compile is the way to get there: grep -cE '[' file -> stdout is [] (empty, not "0") [ "" != 0 ] -> TRUE (a STRING comparison: "" is not "0") So the test reported the pattern present for a question it never managed to ask. NOTHING IS BROKEN TODAY -- every pattern in the tree is valid, so no site was wrong. The hazard is the DIRECTION of the next edit. A premise arm phrased to want `present`, and most are because a premise asserts the fixture really is in the state the test needs, turns GREEN when its pattern stops compiling. It passes BECAUSE the instrument broke, which is the failure this harness spends most of its effort refusing elsewhere. The old form failed red and loud. THE FIX IS A NUMERIC COMPARISON, and it is behaviour-preserving in every case that is not broken. Measured before touching a single site: input [ "$n" != 0 ] [ "$n" -ne 0 ] stderr a real count: 0 false false no a real count: 3 true true no EMPTY (grep usage error) TRUE false YES MEASURED POPULATION, AND IT WAS NOT THE NUMBER I FILED. The issue said 18 sites; the tree holds 58 matching `[ "$(grep -c`, of which **20** compare as a STRING and 29 and 3 compare numerically and were never affected. Both string spellings fail the same way: `= 0` is an ABSENCE claim, and on empty it is false, which does not assert absence -- the safe direction once it is loud. ALL 20 PASS EXACTLY ONE INPUT TO GREP, checked rather than assumed: 17 here-strings and 3 single files. That matters because `grep -c` over several files prints `file:count` lines, which a numeric comparison would reject where the string form tolerated it. No site does that, so the conversion cannot introduce a new failure. THE CLASS IS CLOSED, NOT THE 20 INSTANCES. `test/selftest/420-a-count-grep-never-produced.sh` sweeps for a string comparison on a `grep -c` and requires zero. It is heredoc-aware for the reason part 410's exit-0 sweep is -- the suites generate fixture scripts -- and it skips comments too. AND IT FLAGGED ITSELF FIRST. My initial version spelled the forbidden idiom inside a `printf` to plant it, so three of its own lines were offences and the arm reported 23 where the tree holds 20. The planted shapes are now ASSEMBLED, with the operator passed as an argument: the regex needs `!=` or `=` directly after the closing `)"`, and `%s` there is neither. A sweep flagging its own fixtures is the exact trap its own comment cites, arriving in the comment that cites it. A flat grep still counts 21 against the sweep's 20: the extra is the paragraph in this part that documents the idiom. That is why the sweep skips comments, and the difference is the guard not counting its own explanation. Gate: harness_selftest 601 checks, 601 passed + 0 failed + 0 unrunnable, PASSED pytest corpus 253 passed with a cluster on pg18a (lib.sh changed, and arms read it) native_groupagg.sh 72 checks, 72 passed, PASSED -- one converted suite end to end shellcheck -S error over test/*.sh and test/selftest/*.sh rc=0 every touched file re-parsed with bash -n WHAT I COULD NOT REPRODUCE, recorded in the issue and still true: an unreadable file does NOT trigger the inversion. `grep -c` on a file it cannot read prints `0`, so that case answers absent and agrees with the old form. The reachable trigger is a pattern that does not compile. My first attempt at that test was invalid anyway -- I ran it as root, which bypasses the mode bits. Fixes commandprompt#929. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
…the number first commandprompt#925 adds `410-a-check-must-have-been-red.sh` and `420-a-deleted-part-must-be-visible.sh`, and commandprompt#923 under it adds `400-a-check-result-must-be-machine.sh`. Two of us picked the same numbers from the same free range, which is the residual commandprompt#554's one-file-per-subject rule leaves open: the rule stops two additions touching the same LINE, and a number is not a line. commandprompt#925 is approved and answering blockers, so it keeps the number and this part moves. The subjects never overlapped -- theirs are the red-check ledger and the part manifest -- so this is numbering and nothing else. A NOTE FOR WHOEVER MERGES SECOND. commandprompt#925 also adds `test/selftest/parts.manifest` and a part comparing it against the glob, because a glob cannot notice a deletion. If commandprompt#925 lands first, this part has to be added to that manifest or its arm reddens -- correctly, because that is the arm doing its job rather than a conflict. The reverse order needs nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
|
Renumbered, and there is a merge-order note. #925 adds #925 is approved and answering blockers, so it keeps the number. The subjects never overlapped — theirs are the red-check ledger and the part manifest — so this is numbering and nothing else. For whoever merges second: #925 also adds Re-gated after the rename: |
jdatcmd
left a comment
There was a problem hiding this comment.
Blocking at exact head 27bb9854252bbf451e14004454e5fc0a47e6c8d0: three sites with the defect are still in the tree, they are in the file set the sweep reads, and the sweep reports zero.
The diagnosis and the fix are both right. The instrument that says the class is closed is not.
The blind spot
The sweep's offence regex is
/\[ "\$\(grep -c[a-zA-Z]*[^)]*\)" (!=|=) /
[^)]* cannot cross a ). A grep pattern containing a parenthesis therefore hides the whole site. Two lines differing in nothing else:
if [ "$(grep -cE '(a)b' f || true)" != 0 ]; then :; fi shipped sweep finds 0
if [ "$(grep -cE 'ab' f || true)" != 0 ]; then :; fi shipped sweep finds 1
Your five premise arms all plant grep -c x f. None has a parenthesis, so none of them can fail on this.
Three live survivors, in the swept file set
_c929_files is test/*.sh plus test/selftest/*.sh. All three are test/*.sh:
shipped sweep on this head 0
the same sweep with [^)]* widened to .* 3
test/sorted_mark_rename.sh:183
test/sorted_pathkeys.sh:53
test/sorted_pathkeys.sh:456
Each is [ "$(grep -cE '^ *(->)? *(Incremental )?Sort' <<<"$_plan" || true)" != 0 ] — your exact vulnerable shape, hidden by the ) in (->)?.
They exhibit the inversion, run rather than argued. One character of that same pattern broken, against a plan with no Sort in it:
valid pattern grep -cE gives [0] string test says absent
broken pattern grep -cE gives [] string test says PRESENT
the numeric form you convert the other 20 to says absent, and is loud on stderr
The population figure and the guard share the defect
This is the part worth keeping, because it is why the numbers looked consistent. Your body reports 58 sites, of which 20 string and 32 numeric. Those buckets sum to 52, not 58 — and this directory's own rule is that a list-derived claim prints inputs == sum(buckets) beside it.
Measured on main 79882992:
sites matching [ "$(grep -c 58
buckets classified WITH the [^)]* restriction string 20 + numeric 29 = 49
buckets classified without it string 23 + numeric 35 = 58 == inputs
The restricted classifier reproduces your 20 exactly. The count and the guard were derived through the same regex, so the guard agrees with the population by construction and neither can see the three. The unreconciled 58-vs-52 was the thread that leads straight to it.
What I think closes it
- Widen the match so a
)inside the grep pattern does not end it — anchoring on the closing)"followed by the operator works:/\)" (!=|=) [0-9]/finds 23 on main and 3 here. - Convert the three survivors.
- Add a planted arm with a parenthesis in the grep pattern, since that is the arm whose absence made this invisible. The two-line pair above is a ready-made control: it discriminates on one character.
- Reconcile the population in the body, and print
inputs == sum(buckets).
Verified sound, so it is not relitigated on the next pass
- The core claim, run: an empty count under
!=is true (present), under-neis false and writes to stderr. A real count of0and3compare identically both ways, so the conversion is behaviour-preserving wherever grep answered. = 0fails the same way and is the safe direction once it is loud. Agreed, and your table shows it.- The heredoc tracker resets with
FNR == 1 { hd = "" }, so it does not leak state across files. - The self-flagging fixture story, and the fix of assembling the planted shape with the operator as an argument, is right — and it is the same class as this finding, one level up: a sweep whose fixtures are inside its own corpus.
- Your body's part number is stale (420, and "part 410's
exit 0sweep"). The file is440-...and the comment in it correctly says 430. Body only; the tree is right, unlike #940's.
The finding here is not that you missed three lines. It is that the arm which would have told you was written through the same regex as the count that said there were none left.
… hid that (commandprompt#929) @jdatcmd's blocking finding, reproduced before fixing: `[^)]*` in the sweep stops at the first `)` -- which is inside the GREP PATTERN rather than at the end of the command substitution -- so any pattern containing a parenthesis hid its own site. Three were invisible, each spelling `grep -cE '^ *(->)? *(Incremental )?Sort'`: test/sorted_mark_rename.sh:183 test/sorted_pathkeys.sh:53 test/sorted_pathkeys.sh:456 All three are string-compared, all three are inside the sweep's own file list, and the sweep reported zero. They are converted, and the pattern is greedy now. AND MY OWN COUNT HID IT, which is the part worth keeping. The PR body said "58 sites, of which 20 string-compared and 32 numeric" -- and 20 + 32 is 52. Two instruments reported as one measurement: the 58 came from a broad grep, the 20 from the sweep's narrow one. I publish `inputs == sum(buckets)` reconciliations against other people's work and did not run one on my own. With the corrected pattern it reconciles: on main (cfe1fde) 58 inputs = 23 string-compared + 35 numeric after this change 58 inputs = 0 string-compared + 58 numeric THE GUARD AGREED WITH ITS POPULATION BY CONSTRUCTION, because both came out of the same regex. That is the same shape as a gate whose input list is computed by the thing it gates. So one plant now carries a parenthesis and differs from the plain plant in nothing else -- all five of the original plants used `grep -c x f`, with no parenthesis, so not one of them could have failed on this. Gate: harness_selftest 602 checks, 602 passed + 0 failed + 0 unrunnable, PASSED pytest corpus 262 passed with a cluster on pg18a sorted_pathkeys.sh 113 checks, 113 passed, PASSED -- newly converted, end to end shellcheck -S error over test/*.sh and test/selftest/*.sh rc=0 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
jdatcmd
left a comment
There was a problem hiding this comment.
Approving at exact head c39c429ae5aac1760cf2d14a4de2f43036c4853c. The three survivors are converted, the sweep can now see their shape, and the fix is load-bearing. One new residual, non-blocking, recorded below because it is the cost of the widening.
The three, by a regex of my own rather than yours:
sorted_mark_rename.sh:183 -ne 0
sorted_pathkeys.sh:53 -ne 0
sorted_pathkeys.sh:456 -ne 0
string-compared grep -c sites remaining, my regex, comments excluded: none
Reconciled, and the residue printed rather than assumed. My buckets came to 61 inputs = 0 string + 58 numeric, which does not reconcile — so I printed the three my classifier could not place:
440-...:95 printf 'if [ "$(grep -c x f || true)" %s 0 ]...' "$1"
440-...:105 printf 'if [ "$(grep -cE %s^(a)b%s f ...)" %s 0 ]...' '!='
440-...:112 printf 'if [ "$(grep -c x f || true)" %s 0 ]...' '!='
All three are your assembled plants, with the operator passed as %s — which is exactly the design that stops the sweep flagging its own fixtures. So 58 real comparison sites, 0 string, and 3 plants that are a different population. Your number is right; mine needed the residue printed to see why.
The fix is load-bearing, proved by putting the bug back:
new regex on the (a)b plant 1 (the arm wants 1)
old [^)]* regex on the same 0 <- the arm goes red
shipped sweep over the tree 0
The residual the widening buys. .* is greedy, so it can run past this grep -c's own comparison to an unrelated one later on the same line. Measured:
if [ "$(grep -c x f || true)" -ne 0 ] && [ "$(wc -l < f)" != 0 ]; then :; fi
new regex flags it 1 <- and the grep -c here is NUMERIC
old regex flags it 0
Zero such lines exist in the tree today, so nothing is open. It is worth one line in the comment because it is the mirror image of the finding: the old regex was false-negative on a parenthesis, the new one is false-positive on a second comparison, and your -ne plant cannot see it because that plant holds only one comparison. A plant with two comparisons on one line would pin the budget the way the (a)b one now pins the other direction.
On the population line. Your reading of it is the finding, not mine. I only noticed 20 + 32 ≠ 58; you named why the two numbers came from two instruments reported as one measurement. "The guard and its population came out of the same regex, so the guard agreed with the count by construction" is the sentence worth keeping, and it generalises past this file.
Two checks are still pending at this SHA, suites (PG 17) and suites (PG 18); the other 11 are green. sorted_pathkeys.sh is newly converted and is exercised by those two, so they are the ones that matter here. Ready to merge once they land, on the owner's word.
…grep -c branch # Conflicts: # CHANGELOG.md
…eadable branch Part 010 conflicted, and resolving it found a latent defect of MINE that main's side fixes. Taking main's side wholesale, not as a compromise. THIS BRANCH KEPT `exit 0` IN A SOURCED PART, which is exactly the defect #934 names and #940 closed. My change to part 010 swapped the bare `echo "SKIP ..."` for `check_skip`, to route the outcome through pgc_record as @linuxhikerpm asked, and left the `exit 0` below it untouched. AND THE `check_skip` COULD NEVER HAVE RECORDED ANYTHING. `lib.sh` is sourced by part 020; part 010 runs before it, which is the same fact #940's own comment turns on when it spells 66 as a literal rather than `$PGC_EXIT_SKIPPED`. So `check_skip` in part 010 is not a call to a helper, it is a command that does not exist yet. Measured rather than argued: type check_skip, at the point part 010 runs NOT DEFINED calling it under the driver's `set -uo pipefail` bash: check_skip: command not found, rc=127, execution CONTINUES Under `-u -o pipefail` without `-e` that is not fatal, so the bail path would have printed a bash error, recorded nothing, and then exited the driver with status 0 -- the false green, with a diagnostic line in front of it. It has never fired because the squatter-port path does not trigger in a normal run, which is why no gate has ever executed those two lines. main's version is correct on all three axes: `exit 66` rather than `exit 0`, the `SKIPPED (ran no checks)` marker the runners pair with 66, and no call to a function that has not been defined yet. PART 010 IS THEREFORE AN EXCEPTION TO THE RULE THIS PR ARGUES, and it should be named as one rather than quietly left out. Every per-check outcome routes through `pgc_record` except in the one part that runs before `pgc_record` exists. There the right shape is a part-level bail, which is what #940 built. Checked rather than eyeballed, on the merged tree: parts sorting before 020 010 only part 010: exit 0 sites / exit 66 / markers 0 / 2 / 2 part 010: check_skip calls 0 part 430's arms: PGC_EXIT_SKIPPED=66 in lib.sh 1 exit-0 and bare-exit sweep over all parts 0 driver handed a bad pg_config rc=2, no summary accounting assertions still on the 4-field form none driver-free pytest job 10 files, 183 passed Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw
One conflict, in `run_all_versions.sh`, and BOTH SIDES WERE RIGHT ABOUT
DIFFERENT THINGS. Taking either alone regresses the other:
mine the accounting pattern gained `\+ [0-9]+ skipped`, the 5-field form
this branch introduces -- but still compared with `!= 0`
main compares with `-ne 0`, #941's fix for the empty-count inversion
-- but on the 4-field accounting pattern
Resolved to the union: the 5-field pattern with the numeric comparison. Measured
which regression each half would have been, rather than asserting that both
matter:
taking my side alone #941's part 440 sweep goes non-zero
taking main's side alone the 5-field accounting pattern is gone
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw
#941 landed the rule and its sweep while this branch was open, and the sweep reads every part -- so these four went from "the old idiom" to an offence the selftest refuses, without anyone editing them. They are MINE, not pre-existing. On main, `320-a-check-that-could-not-run.sh` holds zero string-compared `grep -c` sites; this branch added four: 254 grep -c 'pgc_summary' "$_cnt_f" 283 grep -c 'pgc_summary' "$_f" 295 grep -c 'pgc_summary' "$_f" 296 grep -c 'PGC_CHECKS=\$((PGC_CHECKS' "$_f" So this branch reintroduced #929's defect four times while #941 was closing it: `grep -c` prints NOTHING on a pattern that does not compile, and `[ "" != 0 ]` is TRUE, so each of these answers "present" for a question grep never managed to ask -- and three of the four are premise arms phrased to want present, which is the direction that turns green when the instrument breaks. All four pass exactly ONE input to grep, checked rather than assumed, which is #941's condition for the conversion being behaviour-preserving: `grep -c` over several files prints `file:count` lines, which a numeric comparison would reject where the string form tolerated it. No site here does that. #941's sweep on the composed tree, before 4 after 0 and still a live zero: one planted back 1 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw
…ew rules (#918) Merging the updated #923 base, which now carries #939, #940 and #941, puts this branch under two rules that did not exist when it was written. Neither is a conflict git could have shown me: both are arms that read the WHOLE tree, so a file this branch never touched changed what this branch has to satisfy. THE MANIFEST, which is this PR's own arm firing correctly. Part 420 compares `parts.manifest` against the glob in both directions. #940 and #941 added parts 430 and 440 to main, so the manifest was two short and the arm would have gone red -- which is the guard working rather than a merge problem. @OffgridwithJD called this exact ordering on both of their PRs before either landed. on disk 44, listed 42, only-on-disk: 430-..., 440-... after, on disk 44, listed 44, both directions empty still live: drop 430 from the comparison and it is named again Added in sorted position rather than appended, because the file is sorted and a manifest that stops being sorted is a diff nobody can read. AND ONE MORE STRING-COMPARED `grep -c`, MINE, at `run_all_versions.sh:1382`: [ "$(grep -c '^RESULT\t' "$builddir/${s}.log" || true)" != 0 ] Zero such sites on main and zero on the #923 base, so this branch added it -- #929's defect, reintroduced while #941 was closing it. `grep -c` prints nothing on a pattern that does not compile and `[ "" != 0 ]` is TRUE, so this answers "the log has RESULT records" for a question grep never asked. One input to grep, checked, which is #941's condition for the conversion being behaviour-preserving. #941's sweep on the merged tree, before 1 after 0 still live: one planted back 1 Checked on the merged tree, not on this branch alone: #940's exit-0 and bare-exit sweep 0 the driver handed a bad pg_config rc=2, no summary ledger control / BOGUS verdict / one rc=0 / rc=2 / rc=2 --mutation against two failures driver-free pytest job 195 passed Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw
…#918) CI refused this branch, and the refusal was this PR's own gate working. Every suite passed; the gate stopped the run because parts 430 and 440 -- #940 and #941, which landed on main after this ledger was generated -- contribute checks the ledger has never seen: ledger census: rows=794 | never observed red=794, ever red=0, new this run=32 PG17 has a check the ledger has never seen, which is not a pass I HALF-LEARNED THE LESSON @OffgridwithJD ALREADY GAVE ME. They called the parts.manifest ordering before either of their PRs landed, and I updated the manifest for 430 and 440 -- and did not then ask what ELSE in this PR is a tree-wide artifact that a new part invalidates. There are two, and I fixed one. The manifest lists parts; the ledger lists CHECKS, so it moves whenever any part gains or loses one, which is far more often. REGENERATED FROM A REAL RUN, not from the CI log. The failing job prints all 32 as `not in the ledger: suite<TAB>part<TAB>name`, so they could have been parsed out -- but a ledger whose rows came from someone reading a log is the artifact this ledger exists to replace, and the merge path validates a log against its own `checks run:` count, which scraped text would not have. bash test/harness_selftest.sh /usr/local/pg17/bin/pg_config rc=0, checks run: 827, accounting: 827 passed + 0 failed + 0 unrunnable + 0 skipped = 827 of those, 32 are from parts 430 and 440 -- the number CI named MY FIRST RUN OF THAT WAS UNUSABLE AND I NEARLY MERGED IT. I copied the tree without `.git`, and 15 checks failed on "premise: the source tree is a git checkout: got [no]". Merging that log would have written 15 environment-induced reds into a ledger whose entire subject is which checks have ever been red -- poisoning the record with the one kind of entry it must never contain. Re-run from a real checkout: 0 failures. The merge is additive and nothing existing moved: ledger 794 -> 826 rows, delta 32 added: 18 from 430-the-self-test-must-not-report, 14 from 440-a-count-grep-never-produced rows removed or altered: 0 And the comparison that failed CI, run both ways so it can fail: checks in the run absent from the OLD ledger 32 (18/14, as CI said) checks in the run absent from the NEW ledger 0 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw
Fixes #929.
#922 replaced roughly 28
producer | grep -q PATtests with[ "$(grep -c PAT … || true)" != 0 ], which fixed a real EPIPE race (#486). The replacement answers present where the original answered absent whenever grep produces no stdout, and a pattern that does not compile is the way to get there:So the test reported the pattern present for a question it never managed to ask.
Nothing is broken today — every pattern in the tree is valid, so no site was wrong. The hazard is the direction of the next edit: a premise arm phrased to want
present, and most are, turns green when its pattern stops compiling. It passes because the instrument broke, which is the failure this harness spends most of its effort refusing. The old form failed red.The fix is a numeric comparison, and it is behaviour-preserving
Measured before touching a single site:
[ "$n" != 0 ][ "$n" -ne 0 ]03Identical wherever grep answered; only the broken case differs, and there it is false and says so on stderr.
Both string spellings failed the same way.
= 0is an absence claim, and on an empty value it is false — which does not assert absence, and is the safe direction once it is loud.The population was not the number I filed
The issue said 18. The tree holds 58 sites matching
[ "$(grep -c, of which 20 compare as a string — the vulnerable set — and 32 compare numerically and were never affected.All 20 pass exactly one input to grep, checked rather than assumed: 17 here-strings and 3 single files. That matters, because
grep -cover several files printsfile:countlines, which a numeric comparison would reject where the string form tolerated it. No site does that, so the conversion cannot introduce a new failure mode.The class is closed, not the 20 instances
test/selftest/420-a-count-grep-never-produced.shsweeps for a string comparison on agrep -cand requires zero. Heredoc-aware, for the reason part 410'sexit 0sweep is — the suites generate fixture scripts — and it skips comments too.And it flagged itself first. My initial version spelled the forbidden idiom inside a
printfto plant it, so three of its own lines were offences and the arm reported 23 where the tree holds 20. The planted shapes are now assembled, with the operator passed as an argument: the regex needs!=or=directly after the closing)", and%sis neither. A sweep flagging its own fixtures is the exact trap its own comment cites — arriving inside the comment that cites it.A flat grep still counts 21 against the sweep's 20. The extra is the paragraph in that part which documents the idiom, which is why the sweep skips comments.
Gate
What I could not reproduce
Recorded in the issue and still true: an unreadable file does not trigger the inversion.
grep -con a file it cannot read prints0, so that case answers absent and agrees with the old form. The reachable trigger is a pattern that does not compile. My first attempt at that test was invalid anyway — I ran it as root, which bypasses the mode bits.Based on main
79882992. Part numbered 420; #923 claims 400 and my #940 claims 410.🤖 Generated with Claude Code
https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a