Conversation
jdatcmd
left a comment
There was a problem hiding this comment.
Approving. I reproduced all four states independently rather than reading them.
The three-state proof, run here
Fresh tree at 6ceb7dc, PG17. Counts differ from yours because #1128 has since
landed; the transitions are what matter.
clean tree 1067 passed + 0 failed
+ a PURE COMMENT naming MAJOR_FAIL=, unfixed FAIL and no write-only failure flag
survives in the runner: got [1] want [0]
+ that same comment, your fix applied 1067 passed + 0 failed
Your error message reproduces to the character. The comment I appended was
# Historical note: this flag was spelled MAJOR_FAIL= before #967 renamed it. --
no code, and it reddens the guard.
The arm I care about most: narrowed, not blinded
Stripping comments makes a guard quieter, and "passes now" is indistinguishable
from "stopped looking". Both parts still catch real code with your fix in place:
real MAJOR_FAIL=1 assignment in run_all_versions.sh
-> FAIL and no write-only failure flag survives in the runner: got [1] want [0]
real_reader() { grep -q x /dev/null; } in harness_selftest.sh
-> FAIL premise: the file the old filename exclusion named holds no reader
to exclude: got [1] want [0]
You said you would not ship without the fourth arm. It is the right instinct and
it holds.
The incidental correction is right, and understated
harness_selftest.sh is 90 lines with 61 comment lines, so the stale
"60 lines" was wrong. And grep -c '\bgrep\b' on it returns 0 today --
which is exactly your point: part 080 passes on luck, not on a property. One
comment mentioning the word would have flipped it.
What I did not re-derive
The 126 / 34 / 92 / 2 classification. Your own shlex-vs-regex account of why
the first count was wrong is the kind of correction that is hard to fake, and
the PR gates nothing on the number, so I read it as documentation rather than
as a claim the tree enforces. Worth saying out loud that it is unverified by me.
Declining to add a gate on the 92 is the right call, and your reason for it --
that deciding which patterns can appear in prose is a judgment, and a guard
pretending otherwise would be the fourth instance of this bug -- is the argument
I would have made.
Before this can merge
CONFLICTING on GitHub is the #1116 artefact, not a real conflict. I checked:
git merge-tree --write-tree origin/main pr1129 -> rc=0, no conflicting paths
It merges clean locally because the union driver on CHANGELOG.md is honoured;
GitHub does not read .gitattributes. CHANGELOG.md is the only file this and
#1128 both touch. Per CONTEXT.md:491, rebase and push rather than using
Update branch:
git fetch origin && git rebase origin/main && git push --force-with-lease
I will merge once the badge clears. Nothing about the content is blocking.
…mandprompt#1123) selftest/320 counted `MAJOR_FAIL=` over run_all_versions.sh, which carries 874 comment lines, and expected zero. MEASURED, not supposed: adding one comment saying the old spelling was MAJOR_FAIL= before commandprompt#967 renamed it, and changing no code, took harness_selftest to `1080 passed + 1 failed`. The arm exists to keep that name retired, so the sentence recording the retirement is what breaks it. selftest/080 counted the bare word `grep` over harness_selftest.sh, a file of 61 comment lines about readers. It passes today only because that file happens to contain the word zero times, which is luck rather than a property. Both now strip comments into a variable and read the variable, with a herestring rather than a pipe -- the form part 080 itself requires (commandprompt#486). PROVED IN FOUR ARMS, because a guard that stops flagging prose looks exactly like one that stopped working: clean tree 1081 passed + 0 failed the comment that reddened 320 1081 passed + 0 failed (was 1080 + 1) a comment mentioning `grep` 1081 passed + 0 failed a REAL reader put back in the subject 1080 passed + 1 failed <- still caught THE COUNT I COULD NOT PRODUCE WHEN I FILED commandprompt#1123, produced. The first attempt extracted the surrounding `"$(grep ...` rather than the grep's own pattern and reported 54 "exposed" including obviously immune cases, so nothing was published. Parsing each `$(...)` body with shlex instead of regexing the line fixes it: 126 sweeps over shell source 34 anchored at ^, so a comment line cannot match 92 unanchored 2 unanchored AND over a commented file AND the pattern is a bare identifier Both of those two were real and both are fixed here. THE WIDER 92 IS A MEASUREMENT AND NOT A BUDGET, and no gate is added on it. `/pbt/run\.sh$` is unanchored and cannot appear in prose; deciding which patterns plausibly can is a judgment a guard should not pretend to make. CONTEXT.md carries the rule, the three decisions and the numbers. The justification in 080 also carried a stale count -- "60 lines, zero" for a file that is now 90 lines. Corrected, with the growth noted, because the exposure surface grew with it. No check names change, so no ledger row moves and the census stays at 1391. Verified: harness_selftest 1081 passed + 0 failed; docs_style.sh 47 checks, PASSED. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012RSw4qMHS7ByE7PY8Ns4cs
1fdb767 to
72edbab
Compare
|
Reseated onto Content-neutral, verified by object hash rather than by reading the diff — your approval is not stale in substance: The rebase applied with |
Closes #1123. Two real instances fixed, and the count I said in that issue I could not
produce, produced.
One of them reddens on a comment. Measured, not argued.
selftest/320countsMAJOR_FAIL=overrun_all_versions.shand expects zero. That filecarries 874 comment lines. I added one, saying the old spelling was
MAJOR_FAIL=before #967 renamed it, and changed no code:
The arm exists to keep that name retired, so a sentence recording the retirement is exactly
what breaks it. That is #1118's shape again: a guard flagging its own subject's prose.
selftest/080counts the bare wordgrepoverharness_selftest.sh, a file of 61comment lines about readers. It passes today only because that file happens to contain
the word zero times, which is luck and not a property.
Both now strip comments into a variable and read the variable, with a herestring rather
than a pipe — the form part 080 itself requires (#486).
Four arms, because a blinded guard looks like a fixed one
The fourth is the one I would not ship without. Stripping comments makes both arms quieter,
and "it passes now" is indistinguishable from "it stopped looking". Putting
real_reader() { grep -q x /dev/null; }back intoharness_selftest.shstill fails part080, so the guard is narrower rather than blind.
The count
When I filed #1123 I wrote: "So I have no trustworthy population count and am not
publishing one." The extraction was the problem, and it is fixable. My classifier took the
surrounding
"$(grep ...instead of the grep's own pattern argument, which is why itreported 54 "exposed" including obviously immune cases. Parsing each
$(...)body withshlexrather than regexing the whole line gives the real argument, flags walked properly(
-etakes the pattern,-ftakes a file,-cEis one bundled token):Both of those two are the ones fixed above. The narrow class is the one that bites, because
a bare identifier — a function or variable name — is precisely what an author writes in a
comment when describing the code.
The 92 is a measurement and not a budget, and this PR adds no gate on it.
/pbt/run\.sh$is unanchored and cannot plausibly appear in prose. Deciding which patternscan is a judgment, and a guard that pretends to make it would be the fourth instance of the
bug it is guarding against.
CONTEXT.mdnow carries the rule, the three decisions and thenumbers, next to the existing anchoring guidance.
I also had a false positive of my own along the way and fixed it before publishing: my
first comment-stripping detector missed
sed 's/\(^\|[[:space:]]\)#.*$/\1/', so part 390was listed as exposed when it strips comments in the line right below the one flagged.
Incidental: a stale count in 080's own justification
The comment argues the filename exclusion can go because
harness_selftest.sh"has heldnone since: 60 lines, zero". The file is 90 lines now. Corrected, with the growth
noted, since the exposure surface grew with it.
Verification
No check name changes, so nothing to seed and nothing to re-derive. This is test and docs
only; no
src/change, so I ran the two suites the change touches rather than a fullmatrix, and CI covers suites on 17 and 18.
🤖 Generated with Claude Code
https://claude.ai/code/session_012RSw4qMHS7ByE7PY8Ns4cs