jiantw83
51c2705667
What：skills/code-review/SKILL.md 的審查範圍表，第 5 組由原本一句 「smells.md group 5」改成明確的 5.1 到 5.8，並點名 5.6、5.7、5.8 三個新項目； Notes 加一行分工，Swagger 文件稽核歸 api-doc。README.md 新增 api-doc 的技能 小節，另補一張工具表格列出 swagger-detect.sh。 Why：sub agent 讀的是 SKILL.md 的審查範圍表，範圍表沒寫清楚，新增的 5.6 到 5.8 就不會被查。使用者讀的是 README.md，新技能與新工具沒列出來就找不到。 兩支技能都碰註解與文件，界線不寫明就會對同一個缺失重複回報。 How：範圍表的第 5 組直接列出三個新項目與各自的判準重點，完整清單仍指向 references/smells.md。Notes 寫明 api-doc 負責狀態碼的回覆類型、Swagger 參數 說明與範例，第 5 組只負責原始碼註解契約。README.md 的技能小節寫明偵測先行、 只裝套件沒掛接就停手；工具表格列出 swagger-detect.sh 的用途、輸出欄位與結束碼。 Who：jsc-review 的 code-review 技能與存取庫說明文件。
46 lines
4.4 KiB
Markdown
46 lines
4.4 KiB
Markdown
---
|
|
name: code-review
|
|
description: Review changed code against the Refactoring smell catalog in six groups (bloaters, obscurity, couplers, dispensables, comment contract, shallow modules). Run when a file change is complete or an implementation is complete, e.g. from jsc-sdlc implement. Each group runs as a sub agent over the git diff; findings are reported with file:line, severity, and refactoring, and the caller decides whether to fix. Not a replacement for the CLI's built-in security or bug review.
|
|
---
|
|
|
|
# code-review
|
|
|
|
Review changed code against `references/smells.md` (from the book *Refactoring*). The reference is written in Traditional Chinese; read it as-is.
|
|
|
|
## When to run
|
|
|
|
1. **When a file change is complete**: one file or one related group of files is done.
|
|
2. **When an implementation is complete**: all todos of a work package are done. This is the call site in `jsc-sdlc:implement` — the end of a work package, after its last todo and before it is committed and turned into a PR.
|
|
|
|
## Division of labor
|
|
|
|
- This skill covers the *Refactoring* smells, the comment contract (group 5), and shallow modules (group 6).
|
|
- Security, logic bugs, and test coverage belong to the CLI's built-in review (e.g. claude's `/security-review`); do not duplicate them.
|
|
- Swagger document auditing belongs to `jsc-review:api-doc`: response type declarations per HTTP status code, Swagger parameter descriptions, and Swagger examples down the nested data models. Group 5 here covers the source comment contract only; the two never report the same gap twice.
|
|
|
|
## Steps
|
|
|
|
1. Get the review scope: `git diff` (uncommitted changes) or `git diff {base}...HEAD` (against the base branch when an implementation is complete); list the changed files. An empty diff → report the literal 「無發現」 and stop here; spawn no sub agent. Completion condition: the changed-file list is non-empty and reported, or the run already ended with 「無發現」.
|
|
2. Review in six groups, and every group **MUST run as a sub agent**; the six groups may run in parallel:
|
|
|
|
| Group | Scope |
|
|
| --- | --- |
|
|
| 1 Bloaters | smells.md group 1 |
|
|
| 2 Obscurity | smells.md group 2, including 2.5 document reference leak — comments carrying issue ids, wiki page ids, work package ids, commit hashes, @ mentions, or external document links; the full banned and allowed lists live in `references/comment-scope.md` |
|
|
| 3 Couplers | smells.md group 3 |
|
|
| 4 Dispensables & Others | smells.md group 4 |
|
|
| 5 Comment contract | smells.md group 5, 5.1 to 5.8 — including 5.6 navigation links to the functions a method calls, 5.7 XML comment tag layout (opening and closing tag each on its own line), and 5.8 code-element markup by language convention (`<paramref>`, `<see cref>`, `<c>` in XML; backticks in JSDoc and docstrings) |
|
|
| 6 Shallow Module | smells.md group 6 |
|
|
|
|
Instructions for each sub agent: read only, change nothing; check every changed line and its enclosing function or class against the group's definitions and detection signals in smells.md; report each finding as `file:line`, smell name, severity (高、中、低 per the smells.md scale), one sentence of evidence, and the suggested refactoring. Findings are reported in Traditional Chinese.
|
|
|
|
Completion condition: all six groups have returned — a group with nothing to report still returns 「無發現」 for its group.
|
|
3. Merge the six groups' findings: deduplicate (when one location hits several groups, merge and list every smell), then sort by severity. Start this step only once all six groups have returned; a group still running means the merge waits. Completion condition: every finding appears exactly once in the merged list, ordered 高 → 中 → 低.
|
|
4. Report the finding list. **This skill never modifies code**; the caller decides what to fix (inside the implementation flow, 高 and 中 are normally mandatory, 低 is judgment). Completion condition: the report is handed to the caller and the fix decision is left to them.
|
|
|
|
## Notes
|
|
|
|
- `jsc-hooks`' `comment-scope.sh` already matches the pattern-detectable items after every file write. Group 2 here covers what patterns cannot decide — project code names and customer names — plus the overall judgment; the two never report the same finding twice.
|
|
- If group 5 examples are fetched from a database, they must be de-identified; never include personal data.
|
|
- When there are no findings, report the literal 「無發現」 explicitly; never leave the report empty.
|