Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 24 additions & 11 deletions code-review/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ No external services (Jira, GitHub CLI) are required. The workflow operates enti

### Project Discovery

On first run, the workflow reads the project's AGENTS.md, CLAUDE.md, CONTRIBUTING.md, linting configs, and CI workflows to build a reviewer profile. This profile determines what the reviewer focuses on and what conventions it enforces. No manual initialization needed.
On first run, the workflow reads the project's AGENTS.md, CLAUDE.md, CONTRIBUTING.md, linting configs, and CI workflow filenames to build a reviewer profile. The profile is cached at `.artifacts/code-review/` (not under `{branch}/`) so it survives review cleanup. `.meta.json` includes `skill_version`; a mismatch is a cache miss. The reviewer sees an index and hunk slices, not a dump. If more than 20 files need hunk Reads, remaining files get `--stat` coverage only; those paths are listed in the change summary. No manual initialization needed.

### The Decision Table

Expand Down Expand Up @@ -89,18 +89,27 @@ When the reviewer approves and the user confirms, all artifacts in `.artifacts/c

## Artifacts

All artifacts are stored in `.artifacts/code-review/{branch}/`.
Review artifacts for a given branch live in
`.artifacts/code-review/{branch}/`. The reviewer-profile cache
(`_reviewer-profile.md` and `.meta.json`) lives at
`.artifacts/code-review/` (workflow root, not under `{branch}/`) so it
survives cleanup. Do not place or remove the cache under the branch
directory.

```text
.artifacts/code-review/feature-xyz/
00-reviewer-profile.md (project conventions and review focus)
01-change-summary.md (what changed, files affected)
review-metadata.json (iteration count, state, timestamps)
decisions-001.json (user decisions per round)
code-review-001.md (initial review)
review-response-001.md (changes made, rejections documented)
code-review-002.md (re-review)
...
.artifacts/code-review/
_reviewer-profile.md (cached profile; reused while sources match)
.meta.json
feature-xyz/
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
00-reviewer-profile.md (project conventions and review focus)
01-change-summary.md (what changed, files affected, stat-only coverage)
diff-index-001.json (name-status + hashes; compared on re-review)
review-metadata.json (iteration count, state, timestamps)
decisions-001.json (user decisions per round)
code-review-001.md (initial review)
review-response-001.md (changes made, rejections documented)
code-review-002.md (re-review)
...
```

## Optional Focus Guidance
Expand Down Expand Up @@ -148,6 +157,10 @@ code-review/
start.md # /start command
continue.md # /continue command
clean.md # /clean command
templates/
00-reviewer-profile.md # Reviewer profile skeleton
01-change-summary.md # Change summary skeleton
code-review.md # Review round skeleton
```

## Getting Started
Expand Down
2 changes: 1 addition & 1 deletion code-review/SKILL.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
name: code-review
version: 0.3.0
version: 0.4.0
description: >-
AI-driven code review workflow that reviews uncommitted changes using a
discoverable reviewer profile, presents findings for human decision, and
Expand Down
6 changes: 5 additions & 1 deletion code-review/commands/continue.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@ description: "Implement accepted changes, re-review, present findings or finaliz
---
# /continue

Read `../skills/controller.md` and follow it.
Read `../skills/controller.md` only if it is not already in this session.
Resolve the continue skill via `../../_shared/recipes/phase-override-resolution.md`
(WORKFLOW=`code-review`, PHASE_FILE=`continue.md`). Read the resolved
`continue.md` and execute it. Do not glob this workflow. Do not call
`GetDynamicTools`.

Dispatch the **continue** phase. Context:

Expand Down
6 changes: 5 additions & 1 deletion code-review/commands/start.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@ description: "Discover project context, review uncommitted changes, present find
---
# /start

Read `../skills/controller.md` and follow it.
Read `../skills/controller.md` only if it is not already in this session.
Resolve the start skill via `../../_shared/recipes/phase-override-resolution.md`
(WORKFLOW=`code-review`, PHASE_FILE=`start.md`). Read the resolved
`start.md` and execute it. Do not glob this workflow. Do not call
`GetDynamicTools`.

Dispatch the **start** phase. Context:

Expand Down
12 changes: 8 additions & 4 deletions code-review/skills/clean.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,10 @@ List all files in the artifact directory:
ls -la .artifacts/code-review/{branch}/
```

Present the list to the user:
Present the list to the user. Use the files `ls` actually returned — the
table below is a shape example, not an exhaustive list. Include every
`diff-index-{NNN}.json`, `code-review-{NNN}.md`, `review-response-{NNN}.md`,
and `decisions-{NNN}.json` that exists.

```markdown
## Review artifacts to remove
Expand All @@ -56,9 +59,10 @@ Branch: {branch}
|------|-------------|
| 00-reviewer-profile.md | Project reviewer profile |
| 01-change-summary.md | Change summary |
| code-review-001.md | Review round 1 |
| review-response-001.md | Response round 1 |
| decisions-001.json | Decisions round 1 |
| diff-index-{NNN}.json | Diff index (every round) |
| code-review-{NNN}.md | Review (every round) |
| review-response-{NNN}.md | Response (every round) |
| decisions-{NNN}.json | Decisions (every round) |
| review-metadata.json | Review state |

Confirm removal? (This cannot be undone.)
Expand Down
151 changes: 123 additions & 28 deletions code-review/skills/continue.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,15 @@ confirms, clean up all artifacts.
`code-review-002.md`, round 3 produces `code-review-003.md`, etc.
- **Clean up on approval.** When the reviewer approves and the user
confirms (or in unattended mode, when the reviewer approves), delete
all artifacts in the review directory.
all artifacts in the review directory. Keep
`.artifacts/code-review/_reviewer-profile.md` and
`.artifacts/code-review/.meta.json`.
- **Index the diff; do not dump it.** Capture `--stat` and `--name-status`.
Never paste full `git diff HEAD` into this conversation or a subagent
prompt. Read hunk neighborhoods (`offset`/`limit` ~80 lines). Cap **≤20**
hunk Reads. Skip generated/vendor/lock paths.
- **Do not glob this workflow. Do not call `GetDynamicTools`.**
If this file is already in session, do not re-Read it.

## Process

Expand Down Expand Up @@ -224,34 +232,104 @@ Clean it up before handing it to the reviewer.

### Step 7: Obtain Re-Review

Obtain a fresh review of the current state, following the same approach
as Step 6 of `/start`:

**If the AI runtime supports subagents:** Check `review-metadata.json`
for a `reviewer_agent_id`. If one exists and the runtime supports agent
resumption, resume the same reviewer agent — this gives it memory of
previous reviews and concerns. If resumption is not available or there
is no stored agent ID, spawn a new subagent. Load it with the reviewer
profile, the updated diff (`git diff HEAD`), all previous review and
response files (so the reviewer has full history), the project's
`AGENTS.md`/`CLAUDE.md`, and the workflow's guidelines (`../guidelines.md`).
Obtain a fresh review of the **full current** uncommitted change, not
only the files touched in this `/continue` round. Last-round delta is
not the review scope. Spawn a **new, independent** reviewer (do not
resume a prior reviewer — that would reload dumped history). Do **not**
pass `AGENTS.md`, `../guidelines.md`, the raw patch, or historical
`code-review-*.md` / `review-response-*.md` files other than the latest
round.

Capture a **diff index** (do not dump `git diff HEAD`):

```bash
git diff HEAD --stat
git diff HEAD --name-status
git ls-files --others --exclude-standard
```
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

If any of these commands fail (non-zero exit), stop and report the
error. Do not review with an incomplete index.

`git diff HEAD --stat` and `--name-status` are empty when the workspace
has only untracked files. That is not a failure. Combine tracked
name-status with untracked paths. If the combined index is empty, tell
the user there is nothing to review and stop.

Write `{branch}/diff-index-{NNN}.json` **once** (same schema as start's
`diff-index-001.json`, with this round's number). Compare each path's
`hashes` entry to the previous round's `diff-index-*.json`. A file
changed this round only if its hash differs — being listed again in
`git diff HEAD --name-status` is not enough.

Build a short **already decided** list from **every**
`decisions-*.json` in the branch artifact directory, not only the latest
round. Compact entries: finding number, title, file, location,
accept/reject. Key by file + location + title — two findings at the same
location are distinct if their titles differ. A finding rejected in
round 1 stays settled in later rounds. Unattended mode must not accept a
previously rejected finding unless that file's hash changed this round
and the new review re-raised it with a different title.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

**Hunk Reads (reviewer and sequential fallback):** For each relevant
**tracked** path in the **full current** index, Read ~80 lines around
changed lines (`offset`/`limit`). Cap **≤20** hunk Reads. Skip
generated, vendor, and lockfile paths. The slice is for context, not
isolation: does a new function duplicate existing functionality? Is the
error handling consistent with the rest of the file? Does the change
interact correctly with surrounding code?

**Untracked files:** They have no `git diff HEAD` hunks. For each
relevant untracked path, Read the first ~80 lines (`limit` 80). If that
chunk is not enough to understand the file's purpose, one additional
~80-line continuation. Each of these Reads counts toward the ≤20 cap.
Do not Read whole large untracked files.

If the cap is hit, list files that got only `--stat` coverage (tracked)
or no content Read (untracked) in the review summary.

**If the AI runtime supports subagents:** Spawn a subagent. Load **only**:

- `00-reviewer-profile.md`
- `01-change-summary.md`
- the name-status / stat index (not the patch)
- `../../_shared/review-protocol.md`
- the **latest** `code-review-{NNN}.md`
- the **latest** `review-response-{NNN}.md`
- the **already decided** list

Store any new agent ID in the metadata.

**If subagents are not available:** Perform the review sequentially. Focus
on the current state of the diff, not just the delta from last round.
Re-evaluate previously flagged areas to confirm they were addressed.
**If subagents are not available:** Read `../../_shared/review-protocol.md`
(not `../guidelines.md`) and review sequentially. Same hunk-Read rules.
Evaluate the code as if you did not write it.

The reviewer should evaluate all categories defined in
The reviewer should evaluate all categories in
`../../_shared/review-protocol.md` and additionally:

- Verify that accepted findings were addressed correctly
- Check whether the changes introduced new issues
- Re-evaluate any previously rejected findings only if the code context
changed in a way that affects them
- Re-check **rejected** findings only if that finding's file **hash
changed this round**. Do not re-argue settled rejects on files that
are still in `git diff HEAD` but were not modified this round
- Issue a verdict: APPROVED or CHANGES_REQUESTED

Increment the iteration counter now. Write the new review to
`.artifacts/code-review/{branch}/code-review-{NNN}.md` (using the
incremented number) in the same format as the initial review.
Parent Reads `../templates/code-review.md` **once**. Instruct the
subagent to write
`.artifacts/code-review/{branch}/code-review-{NNN}.md` (incremented
number) using that skeleton. The subagent writes that file. The parent
writes it **only** when reviewing sequentially (no subagent). Exactly
one writer. Do not overwrite the subagent file.

#### 7a: Validate finding references

For each finding, confirm that the cited file and location actually exist
in the change. AI reviewers hallucinate file paths and line numbers. If a
finding references a file that was not changed (not on the name-status /
untracked list and not a path the reviewer Read), or a location that does
not exist, discard it — do not present it to the user. Note discarded
findings and why in your internal assessment (not in the user-facing
table).

### Step 8: Update Metadata

Expand Down Expand Up @@ -348,7 +426,9 @@ the "APPROVED with no findings" path above) before proceeding to Step 10.

In **unattended mode**, still present the approval and suggestions table
— the user can interrupt at any time, and the table gives them the
information to decide whether to do so. Then: if any remaining
information to decide whether to do so. Do not skip the table because
the mode is unattended; unattended means the user delegated decisions,
not visibility. Then: if any remaining
suggestions have an "Agree" assessment, persist the decisions to
`decisions-{NNN}.json` and loop back to Step 3 to implement them. If all
remaining suggestions are assessed as "Disagree", compile
Expand All @@ -371,9 +451,18 @@ In **attended mode**, present the decision table and wait:

| # | Severity | Category | Finding | Implementor Assessment | Recommendation |
|---|----------|----------|---------|----------------------|----------------|
...

Review the table and let me know your decisions, then run /continue.
| 1 | HIGH | Correctness | {short description} | Agree -- {rationale} | Accept |
| 2 | MEDIUM | Conventions | {short description} | Disagree -- {rationale} | Reject |
| 3 | LOW | Naming | {short description} | Agree -- adds clarity | Accept |
| 4 | LOW | Naming | {short description} | Disagree -- current name is idiomatic for this codebase | Reject |

Review the table and let me know your decisions. You can:
- Accept all recommendations as-is
- Override any specific recommendation (e.g., "reject #1, accept #2")
- Add guidance for how a finding should be addressed
- Correct or supplement any question answers

Then run /continue to implement the accepted changes.
```

Once the user states decisions, persist them to
Expand All @@ -382,12 +471,17 @@ Once the user states decisions, persist them to

In **unattended mode**, still present the decision table — the user can
interrupt at any time, and the table gives them the information to decide
whether to do so. Then:
whether to do so. Do not skip the table because the mode is unattended;
unattended means the user delegated decisions, not visibility. Then:

1. If any CRITICAL finding has a "Disagree" assessment, stop and
escalate to the user (same guardrail as `/start`).
escalate to the user — present the finding and explain the
disagreement. Wait for the user to decide (same guardrail as
`/start`).
2. Otherwise, treat the implementor's recommendations as decisions,
persist them to `decisions-{NNN}.json`, and loop back to Step 3 to
except do not accept a finding that matches a prior reject (same
file + location + title) unless that file's hash changed this round.
Persist them to `decisions-{NNN}.json`, and loop back to Step 3 to
implement the next round.

### Step 10: Clean Up Artifacts
Expand All @@ -411,6 +505,7 @@ Tell the user the review is complete and artifacts have been cleaned up.
## Output

- `.artifacts/code-review/{branch}/review-response-{NNN}.md`
- `.artifacts/code-review/{branch}/diff-index-{NNN}.json`
- `.artifacts/code-review/{branch}/code-review-{NNN}.md`
- `.artifacts/code-review/{branch}/decisions-{NNN}.json`
- Updated `review-metadata.json`
Expand Down
15 changes: 12 additions & 3 deletions code-review/skills/controller.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,15 @@ Other options:
## Starting the Workflow

Before dispatching any phase, check if the project has its own `AGENTS.md`
or `CLAUDE.md`. If so, read it -- it may contain project-specific conventions,
testing standards, or other guidance that affects how the review operates.
or `CLAUDE.md`. If they are **already in this session** (workspace rules
or a prior read), do not re-read them. Otherwise read them — they may
contain project-specific conventions, testing standards, or other
guidance that affects how the review operates.

For **start**, do not load `../guidelines.md` into the reviewer. Those rules
apply to the parent; the reviewer reads `../../_shared/review-protocol.md`.
Do not glob the code-review workflow directory. Do not call
`GetDynamicTools`.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

When the user runs `/start`, execute the start phase. If the user runs
`/start` and artifacts already exist for the current branch, warn that a
Expand Down Expand Up @@ -143,7 +150,9 @@ There are two distinct uses of subagents in this workflow:

1. **Reviewer subagents** (start.md Step 6, continue.md Step 7): The
reviewer runs as a subagent to ensure independence from the
implementor. This is the primary subagent pattern and is described
implementor. Load profile, summary, diff index, and
`../../_shared/review-protocol.md` — not `../guidelines.md`, `AGENTS.md`, or
a raw patch. This is the primary subagent pattern and is described
in the skill files themselves.

2. **Phase-level subagents** (this section): If the AI's output quality
Expand Down
Loading
Loading