diff --git a/scripts/review-loop-run b/scripts/review-loop-run index 52b6fb7..61c3aab 100755 --- a/scripts/review-loop-run +++ b/scripts/review-loop-run @@ -41,15 +41,25 @@ report_dir_for() { # _counts_scope → prints the text of the last "count by severity" # section (heading matching /count.*severity|severity.*count|summary/i, up -# to the next heading or EOF), or the last 40 lines if no such heading exists. +# to the next heading of the same or shallower depth or EOF), or the last +# 40 lines if no such heading exists. +# CR-10 (bug 8c865eb4): Only headings of depth <= the starting heading's +# depth terminate the scope; a deeper sub-heading (e.g. `###` under a `##` +# section) is part of the section and must NOT cut scope short. _counts_scope() { local f="$1" local hdr_line hdr_line=$(grep -inE 'count.*severity|severity.*count|summary' "$f" | tail -1 | cut -d: -f1) || true if [[ -n "$hdr_line" ]]; then awk -v start="$hdr_line" ' - NR==start { found=1 } - found && NR>start && /^#{1,4}[[:space:]]/ { exit } + NR==start { + found=1 + match($0, /^(#+)/, arr) + hdr_level = length(arr[1]) + } + found && NR>start { + if (match($0, /^(#+)[[:space:]]/, arr) && length(arr[1]) <= hdr_level) exit + } found { print } ' "$f" else diff --git a/specs/review-loop-run.spec.md b/specs/review-loop-run.spec.md index c18e338..c947ff1 100644 --- a/specs/review-loop-run.spec.md +++ b/specs/review-loop-run.spec.md @@ -58,7 +58,7 @@ 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. +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 of the same or shallower depth (or EOF). If no such heading exists, the scope is the last 40 lines of the file. CR-10 (bug 8c865eb4): a deeper sub-heading (e.g. `###` under a `##` section) is part of the section and must NOT terminate the scope — a count block written as `## Count by Severity` followed by `### CRITICAL: 0` sub-headings keeps all four severity counts in scope. 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. Exception: a short label parenthetical matching `([A-Z][0-9]+)` (e.g. `- CRITICAL: 1 (C1)`) is stripped before the declaration-only check and does not disqualify the line. Deliberately out of scope: `_`/`__` emphasis stripping, blockquote (`>`) and numbered-list prefixes, and any other parenthetical annotation on declaration lines (finding IDs like `(H-R3-1)` read as prose and are rejected; table cells are the first-class summary field). @@ -66,9 +66,9 @@ review-loop-run --help 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. -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. +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. Why (CR-10): a count block written as an h2 with h3 severity subsections (a common pattern in `## Count by Severity` followed by `### CRITICAL: 0`) was being mis-scope-terminated at the first h3, leaving scope empty, firing the derived fallback, and counting the `### CRITICAL: 0` heading itself as a CRITICAL finding — defeating the merge gate on genuinely clean reviews (bug 8c865eb4). -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. +Fixtures: `tests/fixtures/review-loop-run/{bullet-list,table,inline-bold,heading-numbers,no-count-block,annotated-table-cell,prose-historic-quote,suspect-derived,h3-subsections,h2-terminates-scope}.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 CR-7/CR-8/CR-9 fixtures — plus the CR-10 fixtures (`h3-subsections.md`: count block at `##` with each severity value at `###`; `h2-terminates-scope.md`: count block at `##` with a real `##` after it that must terminate scope and exclude a stray `### HIGH: 99` underneath). Scenarios / test matrix (bug 5352fe72; test handles in `tests/test-review-loop-run.sh`): @@ -81,6 +81,8 @@ Scenarios / test matrix (bug 5352fe72; test handles in `tests/test-review-loop-r | 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) | — | +| CR-10 | `counts h3 subsections parse as zero` | `h3-subsections.md`: `## Count by Severity` (h2) followed by `### CRITICAL: 0`, `### HIGH: 0`, `### MEDIUM: 0`, `### LOW: 0` (all h3) — no table, no body declarations | `CRITICAL=0 HIGH=0 MEDIUM=0 LOW=0` | 0 | +| CR-10 | `counts h2 still terminates scope` | `h2-terminates-scope.md`: `## Count by Severity` (h2) with a trailing table `\| CRITICAL \| 0 \|` ... `\| LOW \| 0 \|`, then a `## Next Section` (h2, same depth) and under it `### HIGH: 99` | `CRITICAL=0 HIGH=1 MEDIUM=2 LOW=0` (the `### HIGH: 99` under the second `##` must be ignored — only the table counts) | 4 | ## Templates (per `--kind`) diff --git a/tests/fixtures/review-loop-run/h2-terminates-scope.md b/tests/fixtures/review-loop-run/h2-terminates-scope.md new file mode 100644 index 0000000..8be5c57 --- /dev/null +++ b/tests/fixtures/review-loop-run/h2-terminates-scope.md @@ -0,0 +1,18 @@ +# Review + +## Count by Severity + +| Severity | Count | +|----------|-------| +| CRITICAL | 0 | +| HIGH | 1 | +| MEDIUM | 2 | +| LOW | 0 | + +## Next Section + +This section is outside the count-by-severity scope and must be ignored. + +### HIGH: 99 + +A trailing severity line that should not be picked up. diff --git a/tests/fixtures/review-loop-run/h3-subsections.md b/tests/fixtures/review-loop-run/h3-subsections.md new file mode 100644 index 0000000..0f03e5d --- /dev/null +++ b/tests/fixtures/review-loop-run/h3-subsections.md @@ -0,0 +1,21 @@ +# Review + +Some review content above. + +## Count by Severity + +### CRITICAL: 0 + +No critical findings in this round. + +### HIGH: 0 + +No high findings either. + +### MEDIUM: 0 + +No medium findings. + +### LOW: 0 + +Nothing to fix. diff --git a/tests/test-review-loop-run.sh b/tests/test-review-loop-run.sh index 12d2654..70db268 100755 --- a/tests/test-review-loop-run.sh +++ b/tests/test-review-loop-run.sh @@ -105,6 +105,15 @@ out=$("$SCRIPT" counts "$FIXTURES/declaration-with-annotation.md"); rc=$? assert_contains "counts declaration line annotation rejected" "CRITICAL=0 HIGH=0 MEDIUM=0 LOW=0 (derived)" "$out" # companion handle (not in spec matrix): exit-code check for the same declaration-line-annotation-rejected case assert_exit "counts declaration line annotation rejected exits 0" 0 "$rc" + +echo "review-loop-run — counts (CR-10, bug 8c865eb4)" +out=$("$SCRIPT" counts "$FIXTURES/h3-subsections.md"); rc=$? +assert_contains "counts h3 subsections parse as zero" "CRITICAL=0 HIGH=0 MEDIUM=0 LOW=0" "$out" +assert_exit "counts h3 subsections exit 0" 0 "$rc" + +out=$("$SCRIPT" counts "$FIXTURES/h2-terminates-scope.md"); rc=$? +assert_contains "counts h2 still terminates scope" "CRITICAL=0 HIGH=1 MEDIUM=2 LOW=0" "$out" +assert_exit "counts h2 termination exit 4" 4 "$rc" echo "review-loop-run — status" printf 'x\nexit=0\n' > "$REVIEW_LOOP_RUN_DIR/t-r1-stdout.txt" out=$("$SCRIPT" status --name t 2>&1); rc=$?