ums: measurement-scope-narrower-than-claim, and plain grep -c undercounting - #3448
Conversation
…unting Two learnings from d-morrison/rme#1138 (Closes #3441): - shared/workflow/verify-the-right-artifact.md: a new section for a measurement of the RIGHT artifact that is still scoped narrower than the claim made from it (a pdflatex harness that bypasses Pandoc's macro expansion, an empty git-show read of a submodule path used as a comparison baseline, a display-math-only scan reported as chapter-wide, and an overfull-box count taken without microtype loaded). - memories/tools.md: extends the existing dedup-listing entry to cover plain single-file `grep -c`, which undercounts whenever a line carries more than one match -- a distinct case from the -l/uniq -c dedup shape already documented there. Two other candidate learnings from the same session were confirmed as duplicates of existing coverage and were not re-recorded (see #3441 for the dupe-check): a --jq transform stripping the shape a hook regex needs (already in algorithmatize-checks.md's "Your own command's shape is part of a transcript-read discharge condition"), and a two-dot `git diff origin/main` reporting main's advance as the branch's own deletions (already in memories/git-diffing.md's "Picking the diff range" section). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
The review's finding: the entry cited `hooks/warn-grep-c-counts-lines.py` on branch `hook/grep-c-counts-lines` "in this repo", and no such branch, commit or filename exists anywhere in the remote. Correct --- and my doing: I wrote that guard and then deliberately withheld it, so the reference the UMS pass recorded went stale the moment the decision was made. The entry now carries what a reader can actually act on: the three measured miscounts, why the `Stop`-hook shape was abandoned (its transcript walk excluded every tool call, because Claude Code stores tool results as `type: "user"` entries, so 19 passing tests had validated a shape the harness never emits), and the redesign in ai-config#3450 --- a `PreToolUse` guard on the publishing commands, where the damage actually lands.
The sharpest finding: "same command, same failure" attributed all three to
the line-vs-occurrence gap, and only the first one is that. The entry now
separates them, because "switch -c to -o" would have fixed exactly one:
- "nine \hat sites": 9 lines, 10 occurrences --- the gap exactly.
- "17 raw e^{": the diff held 18 lines carrying 22 occurrences, so 17 was
neither. It was a patch script's replacement count, reported as though it
described the file.
- "103 \sb uses across 37 files": the 103 is the gap; the file count is not,
since no -c undercounts files. That half came from a `| grep -v
latex-macros` filter the sentence never mentioned.
What actually unifies them is what the sibling fragment in this same PR
describes: each number was really derived, and each answered a narrower
question than the sentence quoting it asked. Stating one cause for all three
was itself an instance of the defect being recorded.
Also:
- The shortfall is the number of *extra* matches on multiply-matching lines,
not "exactly the number of lines with more than one match" --- a line with
three matches contributes two while being one line. The entry's own second
figure (shortfall 4) is satisfied by three lines as easily as four, so the
word "exactly" was falsifiable and false.
- The remedy said "`grep -ro` summed across files", which is the per-file
summation this entry warns against, and omitted `-h`. Now reuses the
parent section's `grep -roh 'PATTERN' . | wc -l`.
- The closing parenthetical restated the three figures already given six
lines above, and did so lossily (dropping the 37→38 file count). It now
carries only what the body lacks: the ref, the date, and the Stop-hook
and #3450 material.
- The microtype arm in verify-the-right-artifact.md is the one figure in that
section with no filed anchor --- rme#1154 carries the corrected overfull
table but records nothing about package configuration. Said so, rather than
leaving a reader to check #1154 and find nothing.
- **`git show` on a submodule path is not a silent success.** Measured: `rc=128` and `fatal: path 'latex-macros/macros.qmd' exists on disk, but not in 'HEAD'` on stderr. Only stdout was empty, and my harness read stdout alone. The entry said the opposite, which would have sent a reader looking for a signal git already gives; it now says the failure was the harness discarding status and stderr, and a new Do bullet says to check the exit status rather than whether bytes came back. - **The new section contradicted the fragment's own routing bullets**, which say its shapes "cover substitutions only" and send scope mismatches to `metacognitive-monitoring`. Those bullets now name this section as the case that sits between the two. The section also no longer claims all four of its instances are non-substitutions: two of them are, and saying so is the honest version --- what the four share is the tell (a scope decision made at setup and never repeated in the reporting sentence), not the mechanism. - **The unifying sentence over-generalized again**, in an entry about over-generalizing. "Each answered a narrower question" holds for instances 1 and 3; instance 2's number describes what a script *did*, not a narrower slice of the file, and the sentence quoting it said "remaining" --- the complement of what was replaced. Stated per instance. Also condensed the `memories/tools.md` block: my addition had taken that file from 1200 lines to exactly the 1250-line cap, leaving the next contributor no headroom. The incident analysis belongs in the workflow fragment, which has room; `tools.md` now keeps the tool behaviour, the three corrected figures, and the idiom. 18 lines of headroom restored.
All three findings were overstated scope inside the section about overstated scope, which is the right place to be strict: - "has its own section at the end of this file" --- it is at line 362 of 1435, with eight sections after it, and the pointer named no title to recover from. Now names the section instead of a location. - The Don't bullet paired "one package configuration" with "the worst case", but the 153pt worst case belongs to the empty-baseline instance, not the microtype one, and that instance is a broken-baseline failure the next bullet already covers. Each subset is now paired with the claim it was actually reported under, and the shared-shape sentence names the baseline case separately. - "every empty read in these four instances" quantified over a set of one: only the `git show` baseline involved an empty read. Now says so.
My previous edit narrowed its rationale to "the four shapes below", but the fragment enumerates more than four -- "another shape" sections at lines 158, 414 and 1174, and a "ninth shape" referenced at 1288. A reader whose claim failed to match the summary shape or the MERGED-status shape would have got no coverage from the stated reason. That narrowing was itself a justification scoped narrower than the rule it supports, in the diff whose subject is exactly that.
|
Addressed the review finding and three further rounds of self-review. The finding. The Three further rounds found real errors in my own additions, each the defect this PR is about:
Also condensed the All six gates pass. Latest adversarial review: Ready for merge, zero findings, on Posted by Claude Code (AI agent) --- not written by a human. |
|
Claude finished review — View run ReviewThis is the round following the previous review's I re-verified every one of the prior round's four findings against primary sources (not just the earlier round's search scope), and all four are resolved: 1. Branch citation (previously: cited a nonexistent 2. The "153pt worst case ... really 47pt" figure (previously flagged as unverifiable). This was a false negative in the prior round's search, which was scoped only to 3. The 4. The I also verified: both cross-referenced sections ( No new findings, and the prior round's VerdictReady for merge Structured Review Data (JSON){
"schema_version": "1.1",
"reviewer": "claude",
"commit_sha": "a6ddc7a675fdd67c4762702cf4aa7963df044a11",
"verdict": "CLEAN",
"findings": [],
"detailed_assessment": "Re-verified all four prior-round findings in memories/tools.md and shared/workflow/verify-the-right-artifact.md against primary sources (d-morrison/rme#1138 commit messages, d-morrison/rme#1154, d-morrison/macros#85, ai-config#3450); all four factual/citation problems from the previous NOT_CLEAN verdict are resolved and the new figures check out exactly against those sources.",
"holistic_assessment": "Scope remains two documentation files with resolving cross-references and passing link checks; the corrected content now meets the corpus's requirement that permanently recorded lessons be checkable and accurate against their cited sources."
}Reviewed commit: 9e33779 |
1 participant
Closes #3441.
Two learnings from d-morrison/rme#1138 (one long session), recorded after a dupe-check confirmed two other candidates from the same session were already covered elsewhere in this corpus (see #3441 for the full dupe-check).
Recorded
shared/workflow/verify-the-right-artifact.md-- new section, "A measurement of the right artifact can still be scoped narrower than the claim made from it". Four instances from one session: apdflatexharness that bypasses Pandoc's macro expansion, an emptygit show <ref>:<submodule-path>baseline read, an overfull-box scan reported chapter-wide when it covered a narrower subset, and an overfull-box count taken withoutmicrotypeloaded. Cross-referencesmetacognitive-monitoring.md's existing, more general "a sound measurement does not license the claim standing next to it" section and states the narrower axis this section adds.memories/tools.md-- extends the existing "A deduplicated listing counts distinct values, not occurrences" entry to cover plaingrep -c, which undercounts whenever a line carries more than one match. References the in-progress guard on branchhook/grep-c-counts-lines(hooks/warn-grep-c-counts-lines.py), not yet opened as a PR -- this entry documents the rule, not the mechanism, and does not duplicate the hook.Confirmed duplicates (not re-recorded)
--jqtransform stripping the shape a hook regex needs -- already documented near-verbatim inalgorithmatize-checks.md's "Your own command's shape is part of a transcript-read discharge condition".git diff origin/mainreporting main's advance as the branch's own deletions -- already documented inmemories/git-diffing.md's "Picking the diff range" section (gha#318 case) and cross-referenced fromskills/daytb/SKILL.md.Review
Two rounds of adversarial review (separate subagent dispatches). Round 1 caught real factual errors: wrong repo owner (
Morrison-Lab/rmeinstead ofd-morrison/rme), and wrong numbers in the grep -c example (verified and corrected against the actual PR #1138 thread and the real hook commit on branchhook/grep-c-counts-lines). Round 2 verdict: clean, zero findings.Push note
git pushwas refused by the pre-push self-review guard, which reported a clean-verdict commit SHA (7041ea992) that belongs to an unrelated PR in a different repo (d-morrison/rme#1138) -- the known cross-worktree ref-resolution defect tracked in #3412. Pushed withALLOW_UNREVIEWED_PUSH=1per that issue and per explicit authorization for this session, after the adversarial review above (not the guard) confirmed clean.Posted by Claude Code (AI agent) --- not written by a human.