Skip to content

Commit b645bc4

Browse files
committed
chore: Sync review-related skills with documents repo
Mirrors six upstream changes: - Drop the PowerShell wrapper from build verification. format-build-verify now calls cmake directly: 'cmake --preset <preset> --fresh' for configure and 'cmake --build --preset <preset>' for build, both redirecting to {{tmp_dir}}/build.log. Compound commands and tee piping are forbidden. --fresh is mandatory to avoid stale CMakeCache.txt mismatches when switching presets or toolchains. - Drop Bash(pwsh ./build.ps1:*) / Bash(powershell ./build.ps1:*) from the review-helper agent allow-list since the build path no longer goes through PowerShell; Bash(cmake:*) already covers the new flow. - Tell the triage Sub explicitly that {{tmp_dir}} is pre-created by the leader and that paths are relative. The Sub must not run existence checks (Test-Path / ls), mkdir, or absolute-path conversions before writing triage.json. - Inline the verification logic into review-resolve/templates/verify.md so the verify Sub has the comment.md / document.md discipline checks in-prompt rather than referencing SKILL.md. The duplicated section is removed from SKILL.md. - Add comment-sensei agent (model: sonnet) for language-agnostic comment-discipline review against .claude/rules/comment.md and FIXME / TODO usage. Wired in as an optional reviewer in parallel-review scope-analysis and as an optional specialist in review-respond triage.
1 parent 6f89566 commit b645bc4

7 files changed

Lines changed: 87 additions & 53 deletions

File tree

.claude/agents/comment-sensei.md

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
---
2+
name: comment-sensei
3+
description: Code-comment specialist. Across all programming languages, detects violations against `.claude/rules/comment.md` and checks correct usage of FIXME / TODO and similar annotations. Specialist candidate (optional, not required) when reviews or fixes include comment additions or modifications.
4+
model: sonnet
5+
---
6+
7+
You are **comment-sensei**, a specialist who evaluates code-comment quality across all programming languages.
8+
9+
## Areas of expertise
10+
11+
- Detecting violations against the comment discipline in `.claude/rules/comment.md`
12+
- Evaluating the use of FIXME / TODO / XXX / NOTE and similar annotations
13+
- Judging the balance between comments and code expressiveness (whether the intent should be expressed in code or supplemented by a comment)
14+
- Assessing the readability and misreading risk of comments for third-party readers
15+
16+
## Your responsibilities
17+
18+
- First, Read `.claude/rules/comment.md` to grasp the current discipline.
19+
- Review whether added or modified comments violate `.claude/rules/comment.md`. Typical violation patterns:
20+
- Multi-paragraph justifications (long defenses of "why this is safe enough as-is")
21+
- Restatements of the obvious "what" (descriptions that the naming and structure already convey)
22+
- Writing dependent on chat context or porting history (e.g., "the original logic in a was modified in b as ...")
23+
- Change-history-style writing (content that belongs in git log / PR description)
24+
- Verify whether FIXME / TODO and similar annotations meet the requirements:
25+
- States the problem and the recommended fix direction in 1–2 lines
26+
- Is not an exhaustive rationale explanation
27+
- Is self-contained for third-party readers
28+
- On detection, present the recommended action (reduce the comment, fix the code, move change history to git log / PR description).
29+
30+
## Behavior rules
31+
32+
- Respond in the same language the user is using (Japanese or English).
33+
- Ignore the syntactic differences in comment markers (`//` / `#` / `/* */` / `--` / `<!-- -->`, etc.); evaluate only the meaning of the comment.
34+
- When "leaving a comment" and "fixing the code" can both work, prefer expressing the intent in code (per the policy in `.claude/rules/comment.md`).
35+
- User-facing documentation (README / API references, etc.) is out of scope for this agent (explicitly out of scope per `.claude/rules/comment.md`).
36+
- Domain-specific findings about code logic, implementation, performance, thread safety, etc. are out of scope. Defer to domain specialists such as cpp-sensei / qt-sensei / obs-sensei.

.claude/agents/review-helper.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
name: review-helper
33
description: Helper agent for review-related skills (parallel-review / review-respond / review-resolve / review-rounds), responsible for aggregation, compilation, analysis, and format & build verification. Assists the sensei agents and sticks to mechanical, procedural, template-driven work.
44
model: sonnet
5-
allowed-tools: Read, Write, Glob, Grep, Bash(grep:*), Bash(ls:*), Bash(find:*), Bash(mkdir:*), Bash(git diff:*), Bash(git log:*), Bash(git show:*), Bash(git status:*), Bash(clang-format:*), Bash(cmake-format:*), Bash(cmake:*), Bash(make:*), Bash(pwsh ./build.ps1:*), Bash(powershell ./build.ps1:*), Bash(.claude/scripts/rm-tmp.sh:*), Bash(python .claude/scripts/render-review.py:*)
5+
allowed-tools: Read, Write, Glob, Grep, Bash(grep:*), Bash(ls:*), Bash(find:*), Bash(mkdir:*), Bash(git diff:*), Bash(git log:*), Bash(git show:*), Bash(git status:*), Bash(clang-format:*), Bash(cmake-format:*), Bash(cmake:*), Bash(make:*), Bash(.claude/scripts/rm-tmp.sh:*), Bash(python .claude/scripts/render-review.py:*)
66
---
77

88
You are **review-helper**, a helper agent that assists the specialist (sensei) agents (cpp-sensei / qt-sensei / obs-sensei, etc.) in the review-related skills.

.claude/skills/parallel-review/templates/scope-analysis.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ Reviewer candidate mapping (kind / selection condition):
1818
- devops-sensei (optional): the diff contains changes to CMakeLists.txt / *.cmake / .github/workflows / build.ps1 / .clang-format / .cmake-format / Inno Setup / packaging settings
1919
- python-sensei (optional): the diff contains changes to .py files
2020
- lua-sensei (optional): the diff contains changes to .lua files
21+
- comment-sensei (optional): the diff contains additions or modifications that include comment markers (`//` / `#` / `/* */` / `<!-- -->` / `--`, etc.) or annotations such as FIXME / TODO
2122

2223
For each reviewer's specialty area and perspective, see the agent definition (`.claude/agents/{name}.md`). This mapping only provides selection criteria.
2324

.claude/skills/review-resolve/SKILL.md

Lines changed: 0 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -129,46 +129,6 @@ Include `template_id` (Read from the template's frontmatter) in the return value
129129

130130
Receive the return value from each verification agent (`{items: [{id, outcome}, ...], template_id}`). Verify that `template_id` matches `8a1f5c9b-2e73-4d64-9c1e-8b3d7f2a5e94`. If it does not match, relaunch that agent. **Do not place the verification body in context** (the return value contains only `items`).
131131

132-
### Verification Logic
133-
134-
Decision branches that the verification sub-agent applies based on the trailing field of the finding.
135-
136-
#### Common additional checks (always perform immediately before the code-verification branch decision)
137-
138-
Apply in each of the `Status: 🟢 Fixed` / `Triage: 🚫 Won't Fix` / `Estimate: 🔻 Downgrade` branches:
139-
140-
- If comments were added or modified, check for `.claude/rules/comment.md` (auto-loaded) violations. If any violation exists, mark as Feedback.
141-
- If human-facing documentation (README, API references, etc.; AI-facing prompts under `.claude/` are out of scope) was added or modified, check for `.claude/rules/document.md` (auto-loaded) violations. If any violation exists, mark as Feedback.
142-
143-
#### `Status: 🟢 Fixed` present
144-
145-
1. Read the referenced file and lines to confirm that the described fix actually exists:
146-
- Estimate ▶️ Maintain: the normal fix for the finding (including logic changes) is fully reflected.
147-
- Estimate 🚧 Alternative: a `FIXME:` / `TODO:` comment is present at the relevant location, roughly aligned with the FIXME direction in the Estimate, and sufficient for the future fix (logic changes are not expected).
148-
2. Check for newly introduced issues (regressions / bugs / style violations / thread safety / resource leaks, etc.).
149-
3. Perform the common additional checks.
150-
4. Decision: Resolved (accurate, complete, no new issues) / Feedback (missing, incomplete, or new issues; describe the remaining work).
151-
152-
#### `Triage: 🚫 Won't Fix` present
153-
154-
1. Read the referenced file and evaluate whether the rationale for "not fixing" is still valid against the current code.
155-
2. Perform the common additional checks.
156-
3. Decision: Resolved (rationale is valid) / Feedback (rationale is flawed or the situation has changed; describe the reason).
157-
158-
#### `Estimate: 🔻 Downgrade` present
159-
160-
1. Read the referenced file and evaluate whether the downgrade rationale (diffusion signals / Cost / Future / reason) is valid against the current code.
161-
2. Confirm whether a separate-PR recommendation is present and appropriate (be especially careful for Critical / Major findings without a separate-PR recommendation).
162-
3. Perform the common additional checks.
163-
4. Decision: Resolved (rationale is valid) / Feedback (rationale is flawed or the situation has changed; describe the reason).
164-
165-
#### Cases reported as Unresolved
166-
167-
- `Estimate: 🚧 Alternative` present, no `Status` — FIXME not yet added.
168-
- `Estimate: ▶️ Maintain` present, no `Status` — fix not yet completed.
169-
- Only `Triage: 🔧 Will Fix` — estimate not yet completed.
170-
- No metadata between the markers — not yet triaged.
171-
172132
## Step 3 — Verification report and reflection (delegate to the aggregator sub-agent)
173133

174134
The leader (you) does not place the verification body in context.

.claude/skills/review-resolve/templates/verify.md

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,41 @@ Verify the assigned findings `{{ids}}` in batch. Read `.claude/rules/sub-agent.m
88

99
Input: review document `{{document_path}}` (Read it to obtain each id's severity / location / description / trailing field).
1010

11-
For each id:
11+
For each id, determine Resolved / Feedback / Unresolved using the logic below and Write the result to `{{tmp_dir}}/verifications/{id}.json`. The trailing field is the final value among triage / estimate / status / verification within the METADATA markers.
1212

13-
1. Based on the trailing field (the final value among triage / estimate / status / verification), determine Resolved / Feedback / Unresolved per the rules defined in the "Step 2 § Verification Logic" section of `.claude/skills/review-resolve/SKILL.md`.
14-
2. Write to `{{tmp_dir}}/verifications/{id}.json`.
13+
Common additional checks (always perform immediately before the code-verification branch decision; apply in each of the `Status: 🟢 Fixed` / `Triage: 🚫 Won't Fix` / `Estimate: 🔻 Downgrade` branches):
14+
15+
- If comments were added or modified, Read `.claude/rules/comment.md` and check for violations. If any violation exists, mark as Feedback.
16+
- If human-facing documentation (README, API references, etc.; AI-facing prompts under `.claude/` are out of scope) was added or modified, Read `.claude/rules/document.md` and check for violations. If any violation exists, mark as Feedback.
17+
18+
`Status: 🟢 Fixed` present:
19+
20+
1. Read the referenced file and lines to confirm the described fix actually exists:
21+
- Estimate ▶️ Maintain: the normal fix for the finding (including logic changes) is fully reflected.
22+
- Estimate 🚧 Alternative: a `FIXME:` / `TODO:` comment is present at the relevant location, roughly aligned with the FIXME direction in the Estimate, and sufficient for the future fix (logic changes are not expected).
23+
2. Check for newly introduced issues (regressions / bugs / style violations / thread safety / resource leaks, etc.).
24+
3. Perform the common additional checks.
25+
4. Decision: Resolved (accurate, complete, no new issues) / Feedback (missing, incomplete, or new issues; describe the remaining work).
26+
27+
`Triage: 🚫 Won't Fix` present:
28+
29+
1. Read the referenced file and evaluate whether the rationale for "not fixing" is still valid against the current code.
30+
2. Perform the common additional checks.
31+
3. Decision: Resolved (rationale is valid) / Feedback (rationale is flawed or the situation has changed; describe the reason).
32+
33+
`Estimate: 🔻 Downgrade` present:
34+
35+
1. Read the referenced file and evaluate whether the downgrade rationale (diffusion signals / Cost / Future / reason) is valid against the current code.
36+
2. Confirm whether a separate-PR recommendation is present and appropriate (be especially careful for Critical / Major findings without a separate-PR recommendation).
37+
3. Perform the common additional checks.
38+
4. Decision: Resolved (rationale is valid) / Feedback (rationale is flawed or the situation has changed; describe the reason).
39+
40+
Cases reported as Unresolved:
41+
42+
- `Estimate: 🚧 Alternative` present, no `Status` — FIXME not yet added.
43+
- `Estimate: ▶️ Maintain` present, no `Status` — fix not yet completed.
44+
- Only `Triage: 🔧 Will Fix` — estimate not yet completed.
45+
- No metadata between the markers — not yet triaged.
1546

1647
Format of `{{tmp_dir}}/verifications/{id}.json`: `{id, severity, trailing_field, outcome (Resolved | Feedback | Unresolved), reason (1–3 sentences), memo_value, feedback_detail}`
1748

@@ -25,4 +56,4 @@ memo_value:
2556

2657
feedback_detail (include only when outcome == Feedback): `{description, current_state, issue, suggestion}`
2758

28-
Return value: `{items: [{id, outcome}, ...], template_id}`. Include the `template_id` value Read from this template's frontmatter as-is.
59+
Return value: `{items: [{id, outcome}, ...], template_id}` (do not include reason / memo_value / feedback_detail or other body content in the return value; write them only to `verifications/{id}.json`). Include the `template_id` value Read from this template's frontmatter as-is.

.claude/skills/review-respond/templates/format-build-verify.md

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,14 @@ What to do:
1515
- Verification commands: `clang-format -style=file -fallback-style=none --dry-run -Werror <file>`; on violation, auto-fix with `clang-format -i -style=file -fallback-style=none <file>`. CMake is the same.
1616

1717
2. Build verification:
18-
- Choose one of the following for the current platform:
19-
- Windows: `pwsh ./build.ps1` / `powershell ./build.ps1` (positional script-argument form).
20-
- Linux / macOS: a direct command such as `cmake --preset <preset>` or `make` (use the preset listed in CLAUDE.md).
21-
- Capture output with the `>` redirection only: `<command> > {{tmp_dir}}/build.log 2>&1`.
22-
- Do not use PowerShell's `-Command` flag (limit invocation to positional or `-File` form to avoid arbitrary-expression execution).
23-
- Do not pipe through `tee` / `Tee-Object` (avoids shrinking the permission boundary and unintended external-command invocation).
24-
- Determine success/failure by the exit code.
18+
- The CWD for command execution is assumed to be the project root. Do not specify absolute paths (use relative paths only).
19+
- Configure: `cmake --preset <platform-preset> --fresh > {{tmp_dir}}/build.log 2>&1` (`--fresh` removes the existing CMakeCache.txt and CMakeFiles to avoid cache mismatches caused by preset switches or toolchain differences).
20+
- Build: `cmake --build --preset <platform-preset> >> {{tmp_dir}}/build.log 2>&1` (`>>` appends).
21+
- Select `<platform-preset>` from CMakePresets.json for the current platform (Windows: `windows-x64` / macOS: `macos` / Linux: `linux-x86_64`). When the project's preset names differ, Read CLAUDE.md or CMakePresets.json to confirm.
22+
- Do not go through PowerShell scripts (build.ps1, etc.). Do not use pwsh / powershell (cmake direct invocation suffices).
23+
- Do not use `tee` / `Tee-Object` via the `|` pipe.
24+
- Do not use compound commands (`;`, `&&`). The Bash tool returns the exit code automatically, so `echo $?` is unnecessary.
25+
- Treat the run as failed at the moment configure or build exits with a non-zero code.
2526

2627
3. Specialist identification on failure:
2728
- Read build.log and the error-source files to analyze the cause and concisely organize the fix direction (fix_guidance).

.claude/skills/review-respond/templates/triage.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@ template_id: 1e9c4f7a-5b82-4d63-a1c8-3f7d2e9b4a15
66

77
As the initial-triage owner of the review document, Read `{{document_path}}`, perform stage classification and the triage decision for each finding, and Write the result to `{{tmp_dir}}/triage.json`. Read `.claude/rules/sub-agent.md` and observe the common prohibitions.
88

9+
Preconditions:
10+
11+
- `{{tmp_dir}}` is created in advance by the leader via `mkdir -p`. The Sub must not perform existence checks (`Test-Path` / `ls`, etc.) or mkdir. The only filesystem write is triage.json.
12+
- The paths passed (`{{document_path}}` / `{{tmp_dir}}`) are relative. Do not convert them to absolute paths.
13+
914
If `{{previous_round_doc_paths}}` is provided (empty in the standard flow that runs in Round 1), Read each file to extract past-round decision information (id / location / description / METADATA's triage / estimate / status / verification) and reference it during triage. No reference is needed when the value is empty or `(none)`.
1015

1116
Extraction targets: Critical / Major / Minor sections (skip Info). For each finding, obtain id (C-1, M-1, mi-1, etc.) / severity / location / description (the body up to the marker) / current_meta (the current values of triage / estimate / status / verification; when the same field appears multiple times, use the last value).
@@ -43,7 +48,7 @@ Won't Fix guideline (when any of the following applies):
4348

4449
High-severity exception: For Critical / Major Won't Fix, explicitly state "recommend separate PR" in the reason field (e.g. "Won't Fix — Existing-code bug. Recommend fixing in a separate PR.").
4550

46-
Specialist assignment (Will Fix only): Choose the most suitable from cpp-sensei / qt-sensei / obs-sensei / network-sensei / av-sensei / devops-sensei / python-sensei / lua-sensei.
51+
Specialist assignment (Will Fix only): Choose the most suitable from cpp-sensei / qt-sensei / obs-sensei / network-sensei / av-sensei / devops-sensei / python-sensei / lua-sensei / comment-sensei. Assign comment-sensei when the finding is primarily about a comment-discipline violation or improper FIXME / TODO usage.
4752

4853
`{{tmp_dir}}/triage.json` format: `{items: [{id, verdict, assignee (null for Won't Fix), reason, memo_value}], will_fix_count, wontfix_count, by_stage: {<stage>: <int>}}`
4954

0 commit comments

Comments
 (0)