From 01ae924292a6b3f9a4c7341b8c82bb8d2da8d42e Mon Sep 17 00:00:00 2001 From: Paul O'Reilly Date: Wed, 16 Sep 2026 14:35:03 +1200 Subject: [PATCH] =?UTF-8?q?spec(review-loop-run):=20CR-7/8/9=20=E2=80=94?= =?UTF-8?q?=20annotated=20table=20cells,=20declaration-only=20prose=20line?= =?UTF-8?q?s,=20suspect=20derived=20zero=20(bug=205352fe72)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude-Session: https://claude.ai/code/session_01G3SkMRBRWXyiGuMGs8WVas --- specs/review-loop-run.spec.md | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/specs/review-loop-run.spec.md b/specs/review-loop-run.spec.md index 084735c..6015c9e 100644 --- a/specs/review-loop-run.spec.md +++ b/specs/review-loop-run.spec.md @@ -59,11 +59,28 @@ review-loop-run --help `counts` parses the review's own count-by-severity block first, never bare numbers scattered through the file (a finding heading like "## Finding 3 — HTTP 404 on ..." or "### MEDIUM: ..." must not be misread as a count): 1. **Locate the scope.** Find the *last* line matching `/count.*severity|severity.*count|summary/i` and take from that heading up to the next `#`-heading (or EOF). If no such heading exists, the scope is the last 40 lines of the file. -2. **Extract per severity, within scope only.** For each of CRITICAL/HIGH/MEDIUM/LOW, take the *last* match in scope of any of: `SEV: n`, `SEV = n`, `- SEV: n`, `**SEV**: n` (markdown emphasis markers around/inside the label are stripped before matching, so `**SEV:**` also matches), `| SEV | n |`, `SEV (n)`, or `n SEV` (word-boundary). A severity with no match in scope is 0 for this pass. -3. **Fallback (derived).** Only if scope step 2 finds *no* severity at all (no count block present anywhere), fall back to counting lines that START with the severity word — `^\s*(#+\s*)?(\*\*)?(CRITICAL|HIGH|MEDIUM|LOW)\b` followed by `:`, `—`, or `-` — one count per finding line, never bare numbers elsewhere. Append a literal ` (derived)` marker to the output in this case. -4. Missing severity → 0. Output stays `CRITICAL=n HIGH=n MEDIUM=n LOW=n` (plus the optional trailing `(derived)` marker); exit 4 iff CRITICAL+HIGH>0, else 0. +2. **Extract per severity, within scope only.** For each of CRITICAL/HIGH/MEDIUM/LOW, take the *last* match in scope of any of the accepted forms below. A severity with no match in scope is 0 for this pass. Markdown emphasis markers around/inside the label are stripped before matching, so `**SEV:**` also matches. The accepted forms are (bug 5352fe72 — *Revokes prior text:* the earlier form list required a bare-integer table cell and allowed `SEV: n` to match mid-prose; both are superseded here): + - **CR-7 Table row:** `| SEV | n |` or `| SEV | n () |` — the value cell may carry trailing text after the integer (e.g. `| HIGH | 1 (H-R3-1: ...) |`); the *leading* run of digits in the value cell is the count (thousands separators, decimals, and exponents are not interpreted — `1,000` reads as 1; counts are small integers in practice). A cell whose text does not begin with an integer does not match. + - **CR-8 Declaration line:** `SEV: n` / `SEV = n` / `- SEV: n` / `**SEV**: n` / `SEV (n)` / `n SEV` — accepted only when the line is a *count declaration line*: after stripping `*` emphasis markers and any leading list-bullet (`-`, `*`, `+`) or heading (`#`) prefix, the entire line consists of one or more such severity-count tokens separated only by spaces, commas, or semicolons, with nothing else. Prose lines that merely mention a count (e.g. `MEDIUM=404 seen on 2026-09-13` inside a quoted verdict) MUST NOT match — the trailing prose disqualifies the line. Deliberately out of scope: `_`/`__` emphasis stripping, blockquote (`>`) and numbered-list prefixes, and parenthetical annotations on declaration lines (table cells are the first-class summary field; declaration-line annotations read as prose and are rejected). +3. **Fallback (derived).** Only if scope step 2 finds no severity value for any of CRITICAL/HIGH/MEDIUM/LOW (found_any == 0), fall back to counting lines that START with the severity word — `^\s*(#+\s*)?(\*\*)?(CRITICAL|HIGH|MEDIUM|LOW)\b` followed by `:`, `—`, or `-` — one count per matching line, never bare numbers elsewhere. Append a literal ` (derived)` marker to the output in this case. +4. **CR-9 Suspect derived zero (bug 5352fe72).** Applies only when step 3 actually ran (found_any == 0; an explicit count block yielding C=0 H=0 is trusted and never suspect). If the derived fallback yields CRITICAL=0 and HIGH=0 but the file contains a heading line matching `^#{1,6}\s*(\*\*)?(CRITICAL|HIGH)\b` (case-insensitive) anywhere in the file — not restricted to the count-block scope — (a severity section header the derived regex cannot count, e.g. bare `## CRITICAL`), the result is untrustworthy: append ` (suspect)` instead of ` (derived)` to the output and exit 4 (fail closed) regardless of the printed counts. Fenced code blocks are NOT excluded from this scan: a false suspect on a file quoting `## CRITICAL` in a fence is fail-closed (a human reads the file), which is the preferred failure direction. Suspect detection is restricted to CRITICAL/HIGH because only they gate the merge. Suspect and genuine C+H>0 share exit 4 deliberately — the consumer action (another round / human read) is identical; distinguish via the `(suspect)` marker. +5. Missing severity → 0. An empty result file prints `CRITICAL=0 HIGH=0 MEDIUM=0 LOW=0` and exits 0. Output stays `CRITICAL=n HIGH=n MEDIUM=n LOW=n` (plus the optional trailing `(derived)` or `(suspect)` marker); exit 4 iff CRITICAL+HIGH>0 or the result is suspect, else 0. -Fixtures: `tests/fixtures/review-loop-run/{bullet-list,table,inline-bold,heading-numbers,no-count-block}.md` cover, respectively: a bullet-list count block, a markdown-table count block, an inline `**HIGH**: 2` style block, a file with "HTTP 404" and a bare number in headings alongside a correct trailing count-by-severity table (regression case for the 2026-09-13 MEDIUM=404/HIGH=11 mis-parse), and a file with no count block at all (derived fallback from finding headings). +Why (CR-7/CR-8): review models routinely annotate summary-table cells with finding IDs, and verify verdicts quote historic counts in prose; both produced silent 0/0 mis-reports on 2026-09-16 that defeated the /review-loop merge gate. Why (CR-9): a derived 0/0 on a file that visibly has CRITICAL/HIGH section headers means the parser failed, not that the review is clean — the gate must fail closed. + +Fixtures: `tests/fixtures/review-loop-run/{bullet-list,table,inline-bold,heading-numbers,no-count-block,annotated-table-cell,prose-historic-quote,suspect-derived}.md` cover, respectively: a bullet-list count block, a markdown-table count block, an inline `**HIGH**: 2` style block, a file with "HTTP 404" and a bare number in headings alongside a correct trailing count-by-severity table (regression case for the 2026-09-13 MEDIUM=404/HIGH=11 mis-parse), and a file with no count block at all (derived fallback from finding headings) — plus the three new fixtures below. + +Scenarios / test matrix (bug 5352fe72; test handles in `tests/test-review-loop-run.sh`): + +| Req | Test handle | Fixture (required shape) | Expected stdout | Exit | +|---|---|---|---|---| +| CR-7 | `counts annotated table cell` | `annotated-table-cell.md`: `## Count by severity` then a table with rows `\| CRITICAL \| 0 \|`, `\| HIGH \| 1 (H-R3-1: gate bypass) \|`, `\| MEDIUM \| 2 (M-1, M-2) \|`, `\| LOW \| 0 \|` | `CRITICAL=0 HIGH=1 MEDIUM=2 LOW=0` | — | +| CR-7 | `counts annotated cell exits 4` | same fixture | — | 4 | +| CR-8 | `counts prose historic quote ignored` | `prose-historic-quote.md`: `## Count by severity` then `\| HIGH \| 2 \|` table rows (all other sevs 0) followed by prose line `Note: the earlier bug reported MEDIUM=404 seen on 2026-09-13.` inside the same scope | `CRITICAL=0 HIGH=2 MEDIUM=0 LOW=0` (MEDIUM must not be 404) | 4 | +| CR-9 | `counts suspect derived zero marks suspect` | `suspect-derived.md`: no count block, a bare `## CRITICAL` header with prose under it, no line matching the derived finding-line regex | contains ` (suspect)`, not ` (derived)` | — | +| CR-9 | `counts suspect derived zero exits 4` | same fixture | — | 4 | +| CR-9 | `counts suspect derived zero case insensitive` | `suspect-derived-lower.md`: same shape as `suspect-derived.md` but header is `## critical` (lowercase) | contains ` (suspect)` | 4 | +| CR-8 | `counts declaration line annotation rejected` | `declaration-with-annotation.md`: `## Count by severity` then no table, only the bullet line `- HIGH: 1 (H-R3-1)` and no other severity lines | HIGH must not be 1 (the parenthetical disqualifies the declaration line; the file then has no count values, so derived/suspect rules apply to whatever remains) | — | ## Templates (per `--kind`)