Skip to content

Commit 97f1ea4

Browse files
SK-2832: fix severity taxonomy in PR review (Quality is advisory, not blocking)
The Step 1 legend marked Quality as "fix before merge" (blocking) while the consolidated CI output bucketed Quality as non-blocking, so a Quality finding landed in the blocking table while the "Non-blocking (Quality/Smell)" section reported 0 — contradictory output. - Step 1 legend: Quality is now explicitly "No — advisory" (Critical/Bug/Edge remain blocking). - Output (PR/CI mode): add one authoritative severity split — Blocking = Critical/Bug/Edge/High/Medium; Advisory = Quality/Smell/Low/Info — drive the verdict from blocking only, forbid advisory rows in the blocking table, and rename the collapsed section to "Advisory (Quality / Smell / Low / Info)". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent fba0b3e commit 97f1ea4

1 file changed

Lines changed: 15 additions & 11 deletions

File tree

.claude/commands/code-review.md

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,12 @@ Group findings by file and produce a table:
5454

5555
**Severities:**
5656

57-
| Level | Meaning |
58-
|---|---|
59-
| **Critical** | Data loss, silent failure, security risk — must fix before merge |
60-
| **Bug** | Wrong behaviour, incorrect output — must fix before merge |
61-
| **Edge Case** | Unhandled input that will cause runtime failure — fix before merge |
62-
| **Quality** | Maintainability issue, naming violation, missing pattern — fix before merge |
57+
| Level | Meaning | Blocks merge? |
58+
|---|---|---|
59+
| **Critical** | Data loss, silent failure, security risk | Yes |
60+
| **Bug** | Wrong behaviour, incorrect output | Yes |
61+
| **Edge Case** | Unhandled input that will cause runtime failure | Yes |
62+
| **Quality** | Maintainability issue, naming violation, missing pattern | No — advisory |
6363

6464
---
6565

@@ -92,19 +92,23 @@ After all three steps, close with:
9292

9393
## Output (PR / CI mode)
9494

95-
When `GITHUB_ACTIONS` is set, **do not** print the three steps' standalone tables/summaries/verdicts. Merge every finding from Steps 1–3 into a single de-duplicated report (if the same issue is flagged by more than one step, keep it once with the highest severity). Emit **exactly** the following, and nothing else:
95+
When `GITHUB_ACTIONS` is set, **do not** print the three steps' standalone tables/summaries/verdicts. Merge every finding from Steps 1–3 into a single de-duplicated report (if the same issue is flagged by more than one step, keep it once with the highest severity). Emit **exactly** the following, and nothing else.
9696

97-
1. **One-line verdict**`APPROVE` / `APPROVE WITH FIXES` / `REQUEST CHANGES`, followed by a one-sentence rationale.
97+
**Severity buckets (single source of truth for this mode — every finding is exactly one):**
98+
- **Blocking** (must fix before merge): `Critical`, `Bug`, `Edge Case`, `High`, `Medium`.
99+
- **Advisory** (does not block merge): `Quality`, `Smell`, `Low`, `Info`.
98100

99-
2. **One blocking-findings table** (Critical / Bug / Edge Case / High / Medium only). Omit the table entirely if there are none and say "No blocking findings on the changed lines."
101+
1. **One-line verdict**`REQUEST CHANGES` if there is **≥1 blocking** finding; `APPROVE WITH FIXES` if there are only advisory findings; `APPROVE` if there are none. Add a one-sentence rationale.
102+
103+
2. **Blocking-findings table** — blocking severities only. A `Quality` / `Smell` / `Low` / `Info` finding must **never** appear here. Omit the table and say "No blocking findings on the changed lines." if there are none.
100104
```
101105
| File:Line | Severity | Category | Finding |
102106
|-----------|----------|----------|---------|
103107
```
104108

105-
3. **A collapsed section** for everything non-blocking (Quality / Smell / Low / Info):
109+
3. **Advisory section (collapsed)** — every advisory finding (and only advisory ones). The count `N` must match the row count.
106110
```
107-
<details><summary>Non-blocking (Quality / Smell) — N items</summary>
111+
<details><summary>Advisory (Quality / Smell / Low / Info) — N items</summary>
108112
109113
| File:Line | Severity | Finding |
110114
|-----------|----------|---------|

0 commit comments

Comments
 (0)