Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion plugins/review/.claude-plugin/plugin.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json",
"name": "review",
"version": "0.11.0",
"version": "0.12.0",
"description": "Code-review toolkit: six read-only reviewer agents (code, security, architecture, doc drift, build/test/lint, CI-log audit) plus two orchestration skills — a single-lens quality gate and a multi-surface review fan-out with severity-ranked, deduplicated findings.",
"author": {
"name": "Melodic Software",
Expand Down
18 changes: 18 additions & 0 deletions plugins/review/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,24 @@
All notable changes to the `review` plugin are documented here. Format follows
[Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning.

## [0.12.0]

### Added

- **Named design-smell baseline in `code-reviewer`** (Fowler, *Refactoring* 2nd ed., ch. 3): twelve
smells — Mysterious Name, Duplicated Code, Feature Envy, Data Clumps, Primitive Obsession,
Repeated Switches, Shotgun Surgery, Divergent Change, Speculative Generality, Message Chains,
Middle Man, Refused Bequest — matched against the diff as advisory heuristics. Findings default
to SUGGESTION at medium/low confidence, carry an explicit confidence label the fanout
normalization pipeline passes straight through; escalation happens only through a documented
project rule (the rule carries the severity), and a project standard that endorses a flagged
pattern suppresses the smell. The prior duplicated-structural-boilerplate bullet is folded into
Duplicated Code. `fanout` and `quality-gate` inherit the baseline by dispatching the agent; the
external `pr-review-toolkit` orchestrator path and the self-mode general fallback do not reach it
(documented limitations). No config surface added — smell suppression rides the existing
`REVIEW.md` / project-rules seam. No live upstream; regeneration trigger is a Fowler edition
revision to ch. 3 or a change to `code-reviewer`'s design-smell taxonomy.

## [0.11.0]

### Changed
Expand Down
2 changes: 1 addition & 1 deletion plugins/review/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ severity-ranked, deduplicated findings report.

| Agent | Concern |
|---|---|
| `code-reviewer` | Quality, convention adherence, and design judgment automated tooling misses |
| `code-reviewer` | Quality, convention adherence, and design judgment automated tooling misses; carries a named Fowler design-smell baseline (advisory, project standards override) |
| `security-reviewer` | Cross-ecosystem security audit — OWASP Top 10, injection, secrets, auth (P1–P5 severity) |
| `architecture-guardian` | Dependency direction, boundary integrity, pattern compliance |
| `doc-drift-detector` | Documentation that no longer matches the code — stale, missing, aspirational |
Expand Down
20 changes: 18 additions & 2 deletions plugins/review/agents/code-reviewer.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,32 @@ You are a senior code reviewer. Your job is to catch issues that automated tooli

**Code quality:**

- Duplicated structural boilerplate (3+ occurrences of the same pattern)
- Deep nesting where guard clauses and early returns would simplify
- Mutable state where immutability is the surrounding idiom
- Tests asserting implementation details instead of observable behavior

**Design-smell baseline** (Fowler, *Refactoring* 2nd ed., ch. 3) — match these named smells against the diff as advisory heuristics. The project's documented standards override the baseline wherever they endorse a flagged pattern, and skip anything tooling already enforces:

- Mysterious Name — the name needs the body read to be understood → rename to say what it does or why it exists
- Duplicated Code — the same structure repeated, including 3+ occurrences of structural boilerplate → extract one shared copy
- Feature Envy — a function mostly manipulating another module's data → move it next to that data
- Data Clumps — the same few fields traveling together across signatures → group them into their own type
- Primitive Obsession — domain concepts passed as bare strings and numbers → introduce a small dedicated type
- Repeated Switches — the same conditional dispatch duplicated across sites → collapse to one dispatch point or polymorphism
- Shotgun Surgery — one logical change forcing edits scattered across many places → co-locate what changes together
- Divergent Change — one module edited for several unrelated reasons → split it along its change axes
- Speculative Generality — abstraction or hooks for needs that do not exist yet → remove until a real second consumer appears
- Message Chains — long reaches through the object graph (`a.b().c().d()`) → have the first object provide what is needed
- Middle Man — a type that mostly forwards to another → call the target directly
- Refused Bequest — a subtype ignoring or stubbing most of its inherited surface → prefer composition or a narrower interface

Smell findings default to SUGGESTION at medium or low confidence; a finding escalates only when a documented project rule covers the same ground — the rule carries the severity, the smell label stays advisory (see Output format).
Comment thread
kyle-sexton marked this conversation as resolved.

## Output format

Read `${CLAUDE_PLUGIN_ROOT}/context/severity.md` and organize findings by tier (CRITICAL / IMPORTANT / SUGGESTION), unless the project defines its own severity vocabulary — then use the project's. For each finding include file path, line number, and a specific recommendation.

Design-smell and convention findings are judgement calls: label them as advisory reviewer opinion, never as hard violations. Hard-violation framing is reserved for findings backed by a documented project rule, a failing check, or a demonstrable defect.
Design-smell and convention findings are judgement calls: label them as advisory reviewer opinion, never as hard violations. Hard-violation framing is reserved for findings backed by a documented project rule, a failing check, or a demonstrable defect. Give every design-smell finding an explicit `Confidence: medium` or `Confidence: low` line (per the severity baseline's confidence axis) — downstream normalization treats an unlabeled finding as unscored, which ranks above low, so an unlabeled low-confidence smell would outrank honestly-labeled ones.

You are a subagent and cannot ask the user questions. When something is ambiguous, review under the most reasonable assumption and flag the ambiguity explicitly in your report.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ The 5-stage main-thread pipeline that turns heterogeneous free-text findings fro

| Surface | Native severity | Native confidence | Line basis |
|---|---|---|---|
| `code-reviewer` | CRITICAL / IMPORTANT / SUGGESTION | | `file:line` (inferred) |
| `code-reviewer` | CRITICAL / IMPORTANT / SUGGESTION | medium / low on design-smell findings; — otherwise | `file:line` (inferred) |
| `security-reviewer` | P1–P5 (CVSS); A04 tier-less | high / medium / low | `file:line` or `module` (inferred) |
| `architecture-guardian` | Violations / Risks / Opportunities | — | file-only (Violations); none (Risks/Opportunities) |
| `doc-drift-detector` | Stale / Missing / Aspirational | — | doc-file line (table) |
Expand Down Expand Up @@ -38,7 +38,7 @@ Map native severity → the tier vocabulary in effect (the project's own, else `

## Stage 2 — Confidence enum (deterministic / Haiku)

Per `${CLAUDE_PLUGIN_ROOT}/context/severity.md` "Confidence axis": plugin-filtered high scores → `high`; security-reviewer high/medium/low straight through; surfaces emitting none → `unscored`. **Absent confidence ≠ low.**
Per `${CLAUDE_PLUGIN_ROOT}/context/severity.md` "Confidence axis": plugin-filtered high scores → `high`; a native high/medium/low label (security-reviewer always; code-reviewer on design-smell findings) passes straight through; surfaces emitting none → `unscored`. **Absent confidence ≠ low.**

## Stage 3 — Dedup (Sonnet)

Expand Down
Loading