Skip to content

Commit 47774b2

Browse files
ryzizubclaudeomartinma
authored
feat(review): consolidate output into one numbered report with best-practices checks (#210)
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Oscar <martinm.oscar@gmail.com>
1 parent 7691c77 commit 47774b2

18 files changed

Lines changed: 380 additions & 142 deletions

CLAUDE.md

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,17 +43,24 @@ Supporting skills:
4343
Quality-review agents:
4444

4545
- `vgv-review-agent`
46-
- `code-simplicity-review-agent`
47-
- `test-quality-review-agent`
4846
- `architecture-review-agent`
47+
- `test-quality-review-agent`
48+
- `code-simplicity-review-agent`
49+
- `pr-readiness-review-agent`
50+
51+
Each agent writes a detailed report to a `raw/` subdirectory and returns a structured
52+
findings list. The calling skill deduplicates and orders those findings, assigns stable
53+
`FINDING-NN` ids (plus a stable `<category>/<rule>` id per finding for acting on a whole
54+
class), and renders one consolidated report plus a matching chat summary (see
55+
`skills/shared/references/review-consolidation.md`).
4956

5057
## Output Directories
5158

5259
- `docs/brainstorm/` — Brainstorm documents from `/brainstorm`
5360
- `docs/plan/` — Implementation plans from `/plan`
54-
- `docs/reviews/`Review reports from `/build` (ephemeral, cleaned up by build)
55-
- `docs/hotfix-review/`Review reports from `/hotfix` (ephemeral, cleaned up by hotfix)
56-
- `docs/code-review/`Review reports from `/review` (standalone, user-managed)
61+
- `docs/reviews/`Consolidated `review.md` + per-agent `raw/` from `/build` (ephemeral, cleaned up by build)
62+
- `docs/hotfix-review/`Consolidated `review.md` + per-agent `raw/` from `/hotfix` (ephemeral, cleaned up by hotfix)
63+
- `docs/code-review/`One `<slug>/` directory per run (`review.md` + per-agent `raw/`) from `/review` (standalone, user-managed)
5764
- `docs/debriefs/` — Debrief documents from `/debrief`
5865

5966
## Hooks

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ Execute the plan — write code, write tests, run quality review, and open a PR:
6767

6868
### 4. `/review`
6969

70-
Automated review against best practices, test coverage, accessibility, and performance. Catches issues before they reach PR.
70+
Runs specialized agents in parallel — VGV standards, architecture, test quality, and simplicity. Findings land in one consolidated report with stable `FINDING-NN` ids, and the chat summary mirrors it so you can act on any finding by number. Catches issues before they reach PR.
7171

7272
As simple as this:
7373

agents/codebase-review/code-simplicity-review-agent.md

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,7 @@ Remember: Perfect is the enemy of good. The simplest code that works is often th
9696

9797
## Output Instructions
9898

99-
If a file path is specified in your task prompt, write your full review to that file path and return ONLY a brief summary to the caller covering:
100-
- Verdict (ready to merge / needs work / needs rethink)
101-
- Count of critical and important issues
102-
- One-line description of each critical issue
103-
104-
If no file path is specified, return the full review in your response as usual.
99+
Follow the review agent instructions provided in your task prompt: write the full report to
100+
the given raw report path, then return only the structured findings list — not the full
101+
report text, and with no finding ids (the caller assigns those). If no report path is
102+
provided, return the full review in your response.

agents/codebase-review/vgv-review-agent.md

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -278,9 +278,7 @@ Remember these principles throughout every review:
278278

279279
## Output Instructions
280280

281-
If a file path is specified in your task prompt, write your full review to that file path and return ONLY a brief summary to the caller covering:
282-
- Verdict (ready to merge / needs work / needs rethink)
283-
- Count of critical and important issues
284-
- One-line description of each critical issue
285-
286-
If no file path is specified, return the full review in your response as usual.
281+
Follow the review agent instructions provided in your task prompt: write the full report to
282+
the given raw report path, then return only the structured findings list — not the full
283+
report text, and with no finding ids (the caller assigns those). If no report path is
284+
provided, return the full review in your response.

agents/quality-review/architecture-review-agent.md

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -141,9 +141,7 @@ For each new or modified package, verify:
141141

142142
## Output Instructions
143143

144-
If a file path is specified in your task prompt, write your full review to that file path and return ONLY a brief summary to the caller covering:
145-
- Verdict (ready to merge / needs work / needs rethink)
146-
- Count of critical and important issues
147-
- One-line description of each critical issue
148-
149-
If no file path is specified, return the full review in your response as usual.
144+
Follow the review agent instructions provided in your task prompt: write the full report to
145+
the given raw report path, then return only the structured findings list — not the full
146+
report text, and with no finding ids (the caller assigns those). If no report path is
147+
provided, return the full review in your response.

agents/quality-review/pr-readiness-review-agent.md

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -116,9 +116,7 @@ Items that can be resolved automatically:
116116

117117
## Output Instructions
118118

119-
If a file path is specified in your task prompt, write your full review to that file path and return ONLY a brief summary to the caller covering:
120-
- Verdict (ready to merge / needs work / needs rethink)
121-
- Count of critical and important issues
122-
- One-line description of each critical issue
123-
124-
If no file path is specified, return the full review in your response.
119+
Follow the review agent instructions provided in your task prompt: write the full report to
120+
the given raw report path, then return only the structured findings list — not the full
121+
report text, and with no finding ids (the caller assigns those). If no report path is
122+
provided, return the full review in your response.

agents/quality-review/test-quality-review-agent.md

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -140,9 +140,7 @@ Flag these immediately:
140140

141141
## Output Instructions
142142

143-
If a file path is specified in your task prompt, write your full review to that file path and return ONLY a brief summary to the caller covering:
144-
- Verdict (ready to merge / needs work / needs rethink)
145-
- Count of critical and important issues
146-
- One-line description of each critical issue
147-
148-
If no file path is specified, return the full review in your response as usual.
143+
Follow the review agent instructions provided in your task prompt: write the full report to
144+
the given raw report path, then return only the structured findings list — not the full
145+
report text, and with no finding ids (the caller assigns those). If no report path is
146+
provided, return the full review in your response.

config/cspell.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
"glab",
66
"codegen",
77
"conventionalcommits",
8+
"diffstat",
89
"lockfiles",
910
"frontmatter",
1011
"geolocation",

skills/build/SKILL.md

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ Build Progress:
2222
- [ ] Phase 0: Load plan and confirm scope
2323
- [ ] Phase 1: Read context files
2424
- [ ] Phase 2: Implement, test, and run the surgical-diff gate
25-
- [ ] Phase 3: Run review agents (5 in parallel)
25+
- [ ] Phase 3: Run review agents (5 in parallel), consolidate into one report
2626
- [ ] Phase 4: Final validation, cleanup, and ship
2727
```
2828

@@ -110,21 +110,25 @@ After all implementation tasks are complete, run 5 review agents **in parallel**
110110

111111
### Agent instructions
112112

113-
Each agent prompt must include the [review agent instructions](references/review-agent-instructions.md) with `REPORT_DIR` set to `docs/reviews/`.
113+
Run `pwd` and let `<PWD>` be the result — subagents may change directories, making relative paths unreliable.
114114

115-
The 5 agents and their report filenames:
115+
Each agent prompt must include the [review agent instructions](references/review-agent-instructions.md) with `<RAW_DIR>` set to `<PWD>/docs/reviews/raw` and `<name>` set to the agent's report name below (a bare stem — the agent writes `<RAW_DIR>/<name>.md`). Substitute `<PWD>` with the absolute path.
116116

117-
| Agent | Report file |
117+
The 5 agents and their report names (`<name>`):
118+
119+
| Agent | Report name |
118120
| ----- | ----------- |
119-
| **@vgv-review-agent** | `docs/reviews/vgv-review.md` |
120-
| **@code-simplicity-review-agent** | `docs/reviews/code-simplicity-review.md` |
121-
| **@test-quality-review-agent** | `docs/reviews/test-quality-review.md` |
122-
| **@architecture-review-agent** | `docs/reviews/architecture-review.md` |
123-
| **@pr-readiness-review-agent** | `docs/reviews/pr-readiness-review.md` |
121+
| **@vgv-review-agent** | `vgv-review` |
122+
| **@architecture-review-agent** | `architecture-review` |
123+
| **@test-quality-review-agent** | `test-quality-review` |
124+
| **@code-simplicity-review-agent** | `code-simplicity-review` |
125+
| **@pr-readiness-review-agent** | `pr-readiness-review` |
126+
127+
If an agent fails, note it, continue with the rest, and record the failure in the report header.
124128

125129
### After all reviews complete
126130

127-
Follow the [review consolidation procedure](references/review-consolidation.md): categorize findings, auto-fix minor issues, fix critical issues, present important issues to the user, and record suggestions.
131+
Follow the [review consolidation procedure](references/review-consolidation.md): deduplicate the agents' structured findings, order them deterministically, assign stable `FINDING-NN` ids, and write **one** consolidated file to `<PWD>/docs/reviews/review.md` using the [report template](references/review-report-template.md). Print the aligned chat summary (same ids, order, and titles as the file). Then act: auto-fix minor issues, fix Critical findings by id, present Important findings to the user, and note any still-deferred findings in the PR description.
128132

129133
## Phase 4 — Ship
130134

@@ -152,7 +156,9 @@ Stage all implementation and fix changes. Use this commit format:
152156
Implements <plan title or summary>.
153157
```
154158

155-
Where `<type>` matches the plan's type (`feat`, `fix`, `refactor`, etc.).
159+
Where `<type>` matches the plan's type (`feat`, `fix`, `refactor`, etc.). Review findings are
160+
fixed in place during Phase 3 and the report is deleted at Cleanup, so the commit does not
161+
cite `FINDING-NN` ids (there would be no report left to map them to).
156162

157163
### Ship
158164

@@ -170,7 +176,7 @@ Use **AskUserQuestion** to present options:
170176
- If tests fail mid-build, fix the failing test before moving to the next task. Do not accumulate broken tests across tasks.
171177
- Generated files (mocks, codegen output) must be regenerated after code changes — stale generated files cause confusing test failures.
172178
- If the plan specifies file paths that conflict with existing files, confirm with the user before overwriting. The codebase may have changed since the plan was written.
173-
- Review agent reports are written to `docs/reviews/` and deleted after Phase 4. If the build is interrupted, stale reports may remain — delete them manually before the next run.
179+
- The consolidated report (`docs/reviews/review.md`) and per-agent raw reports (`docs/reviews/raw/`) are deleted after Phase 4. If the build is interrupted, stale reports may remain — delete `docs/reviews/` manually before the next run.
174180

175181
## Important
176182

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
../../shared/references/review-report-template.md

0 commit comments

Comments
 (0)