fix(pr-review): drop zero-count summary lines in severity pre-filter (#269)

[cbeaulieu-gt]

No description provided.

[claude-action-runner]

Claude finished @cbeaulieu-gt's task in 3m 34s —— View job

---

PR Review Complete

I've reviewed the incremental changes in this PR that refine the severity pre-filter regex pattern.

Summary

This PR successfully addresses issue #269 by replacing the overly broad pattern :\s*[0-9]+\s*$ with a more specific pattern \((BLOCKING|MAJOR|MEDIUM|LOW)\):\s*[0-9]+\s*$. The new pattern correctly distinguishes between:

• Persona-emitted summary-tally lines (e.g., - 🔴 Critical (BLOCKING): 0) — should be filtered
• Legitimate per-finding tally entries (e.g., - 🟢 Medium: 1) — should NOT be filtered

The key differentiator is the presence of the parenthesized severity-class marker, which only appears in summary lines.

Findings

🟢 Medium

Test coverage gap for MEDIUM and LOW severity classes — pr-review/tests/marker-cases/zero-count-tally-lines.md:11-15

The test case includes examples of (BLOCKING) and (MAJOR) summary lines (lines 11-13), but doesn't include explicit test cases for (MEDIUM) or (LOW) summary lines, even though the regex pattern handles all four classes. While this isn't a blocker (the pattern structure is symmetric and will handle them identically), it would make the test more comprehensive to include:

- 🟢 Medium (MEDIUM): 0
- Nit (LOW): 0

This would validate that all four severity classes are correctly filtered when they appear in the parenthesized summary format.

Nit

Minor edge case with prose content — pr-review/action.yml:403

The pattern \((BLOCKING|MAJOR|MEDIUM|LOW)\):\s*[0-9]+\s*$ could theoretically match prose that ends with text like "section (MAJOR): 42". However, this is an extremely unlikely edge case in practice given the controlled format of persona-generated reviews, and the specificity gain from requiring the parenthesized class marker far outweighs this theoretical risk.

Strengths

✅ Excellent documentation — The inline comments at all three callsites clearly explain the pattern's purpose, the structural signature it targets, and the count-agnostic design decision

✅ Consistency across callsites — All three grep invocations (lines 403, 492, 600) use identical patterns, maintaining the synchronization requirement noted in the comments

✅ Count-agnostic design — Correctly filters summary lines regardless of whether the count is zero or non-zero, as both are tallies rather than individual findings

✅ Test case documentation — The test file includes clear explanatory text distinguishing the two types of count lines

✅ Issue traceability — Good references to #269 and the reproducer glitchwerks/claude-configs#564

---

Verdict: APPROVE