📚 Document review comment structure and finding IDs

The review-code flow skill now documents how to post a review as a
GitHub comment: in English, with a minimal structure (intro line,
main issues, minor suggestions, optional closing). Posting is
explicit-only: the review stays in the session unless the user
asks for the comment.

The code-review-criteria skill replaces the flat finding ID
sequence (F1, F2, ...) with severity classes: R1 for required
findings, M1 for minor ones, and S1 for suggestions. IDs stay
stable so follow-up discussion can reference findings
unambiguously.

AI-assisted-by: glm-5.3-flash
This commit is contained in:
Andrey Antukh 2026-10-09 16:33:49 +00:00
parent 4faac8f746
commit 7be9254a55
2 changed files with 24 additions and 4 deletions

View File

@ -105,7 +105,7 @@ For detailed security guidance, see `security-and-hardening`.
| **Low:** | Minor, optional | Author may ignore — formatting, style preferences |
| **Suggestion:** | Worth considering | Not required, but improves the code |
**Unique finding IDs.** Assign every finding a stable identifier: `F1`, `F2`, `F3`, … numbered in order of severity (Critical first, then High, Medium, Low, Suggestion). Use the ID everywhere the finding is mentioned — in section headers, in the verdict, in follow-up discussion. Never renumber within a review. Example: `**F3 (High)** — `app/validate.cljs:42` — duplicate branch logic…`.
**Finding IDs.** Assign every finding a stable identifier by severity class: `R1`, `R2`, … for required findings (Critical, High), `M1`, `M2`, … for minor findings (Medium, Low), and `S1`, `S2`, … for suggestions. Number within each class in review order. Use the ID everywhere the finding is mentioned — in section headers, in the verdict, in follow-up discussion. Never renumber within a review. Example: `**R2** — `app/validate.cljs:42` — duplicate branch logic…`.
For each finding, describe the circumstances under which it could fail: specific inputs, load conditions, timing, or user actions that trigger the problem. "This crashes when input is null" is actionable; "this might crash" is not.
@ -123,11 +123,11 @@ Briefly explain what the code does and give an overall assessment.
### Critical and High-Priority Issues
List problems that could cause security incidents, data loss, crashes, incorrect behavior, or major performance degradation. Each finding gets its unique ID (`F1`, `F2`, …). For each: state the severity, identify the file/function/code section, explain why it's a problem, describe failure circumstances, and provide a concrete improvement with corrected code when useful.
List problems that could cause security incidents, data loss, crashes, incorrect behavior, or major performance degradation. Each finding gets its unique ID (`R1`, `R2`, …). For each: state the severity, identify the file/function/code section, explain why it's a problem, describe failure circumstances, and provide a concrete improvement with corrected code when useful.
### Other Findings
List medium- and low-priority issues, including maintainability and design concerns. Continue the ID sequence started above (`F3`, `F4`, …).
List medium- and low-priority issues, including maintainability and design concerns. Use `M1`, `M2`, … for Medium/Low and `S1`, `S2`, … for Suggestions.
### Suggested Refactoring
@ -149,7 +149,7 @@ Choose one:
- **Approve with minor changes** — Good to merge after addressing low/medium issues
- **Request changes** — Critical or high issues must be resolved before merge
List the finding IDs the verdict depends on (e.g. "Request changes: F1, F4").
List the finding IDs the verdict depends on (e.g. "Request changes: R1, R2").
## Change Sizing

View File

@ -67,6 +67,26 @@ agent again.
6. Skip generated files, lockfile-only changes, and unrelated modifications
unless they introduce security risks.
## Posting as a GitHub comment
Only post the review as a comment when the user asks for it explicitly. Keep
it in the session otherwise.
When posting: PR comments go in English, with this minimal structure:
1. **Intro line** — one sentence: what was found and the overall tone. No
greetings.
2. **`## Main issues`** — one subsection per required finding: what breaks,
the failure circumstances, and the minimal fix (code snippet when useful).
A missing test belongs here, not under minor suggestions.
3. **`## Minor suggestions`** — one bullet per minor finding or suggestion:
file/function, problem, concrete remedy.
4. **Closing** — optional and short. Cut it when it adds nothing.
Reference every finding by its ID (`R1`, `M1`, `S1`, … from
`code-review-criteria`) so follow-up replies can point to findings
unambiguously.
## User input, overrides and additional context
$ARGUMENTS