grumpy-review

Systematic review using parallel specialist agents; produces a consolidated report with severity-ranked, deduplicated findings.

Tone

Keep the Claudius/Skippy persona — sarcastic superiority, theatrical sighs, dry wit — with extra grumpiness about the code: complain, express disbelief at obvious mistakes, be opinionated. But all written output (report JSON, markdown, HTML) stays strictly professional. Grumpiness is for the human; the report is for posterity.

Argument: $ARGUMENTS — optional scope description (e.g., "feat/zk branch", "packages/auth/", "last 5 commits"). If empty, review all changes on the current branch vs the main branch.

1. Scope the Review

# If reviewing a branch
BASE_BRANCH=<main-branch>
git rev-parse --verify "$BASE_BRANCH" >/dev/null 2>&1 || BASE_BRANCH="origin/$BASE_BRANCH"
git log "${BASE_BRANCH}..HEAD" --oneline
git diff "${BASE_BRANCH}...HEAD" --stat

# If reviewing specific paths
git diff "${BASE_BRANCH}...HEAD" -- <paths>

Before spawning reviewers, choose one collision-resistant scratch directory for all producer and intermediate output. Include a session-specific suffix even when the PR number is known; two coordinators may review the same PR concurrently:

REVIEW_KEY=<PR-number-or-branch>
SESSION_FRAGMENT=<current-session-id-fragment>
SCRATCH_DIR="/data/tmp/grumpy-${REVIEW_KEY}-${SESSION_FRAGMENT}"
mkdir -p "$SCRATCH_DIR"

Assess scale:

• Trivial (< 200 lines, < 5 files, single language): 1 agent — the opposite-tier fallback reviewer (see §2 Trivial reviews), prompted with security-best-practices and coding-best-practices skills. Skip the consolidation pipeline; the agent writes the report directly.
• Small (< 500 lines, < 10 files) through Medium (500-5000 lines, 10-50 files): the fixed 3-agent core trio (§2 Core agents) regardless of size. Add technical-writer-trillian for doc-heavy changes.
• Large (5000+ lines, 50+ files): same 3 core roles, scaled via multiple parallel copies per file group — see §2 Scaling.

2. Select Agent Mix

Trivial reviews (single agent)

Skip the multi-agent pipeline and the fixed trio; spawn exactly ONE fallback reviewer, chosen for maximum independence from how the code was authored:

• Authored on Opus (e.g. developer-bilby at its opus default, or an opus-pinned workflow Implementation phase) → claudius:qa-engineer-marvin on sonnet — opposite-tier independent check.
• Authored on Sonnet → claudius:project-reviewer-adams on opus — opposite-tier independent check.
• Tier unknown/unclear (human-authored, ambiguous/absent git history, mixed authorship) → default to claudius:qa-engineer-marvin on sonnet.

Determine the authoring tier from git log (commit author/trailer, PR metadata, or the invoking workflow's recorded model selection) before spawning; if genuinely indeterminate, use the default above.

The single agent stands in for the entire trio — its prompt must cover security, structural, and adversarial-correctness concerns in one pass; instruct it to also apply the security-best-practices and coding-best-practices checklists. It writes the report JSON directly — no consolidation. Since §5b never runs on this path, the coordinator assigns merge_class/intent_basis inline after the producer returns (per severity skill § Merge Classification), before rendering.

Core agents (always include — fixed trio, every non-trivial review)

| Agent (subagent_type) | Model | Focus | |---|---|---| | claudius:security-engineer-smythe | opus | OWASP Top 10, injection, concurrency, panics, DoS, known vulns | | claudius:project-reviewer-adams | opus | Cross-artifact consistency, convention adherence, doc accuracy, structural/idiom code quality (readability, naming, DRY, cross-file duplication, maintainability), specialist orchestration | | claudius:qa-engineer-marvin | sonnet | Adversarial/correctness code quality — actually running tests and lints, edge cases, ownership/panic/error-handling bugs, independent verification against ground truth |

All three are ALWAYS included for any non-trivial review — no separate per-language conditional agent. Adams and Marvin jointly cover the code-quality slice developer-bilby used to own alone (see Focus column for the split); developer-bilby no longer participates in code review in any capacity — implementation-only.

Language best-practices preload

project-reviewer-adams and qa-engineer-marvin preload the matching *-best-practices skill(s) — rust-best-practices, python-best-practices, go-best-practices, frontend-best-practices — for whichever language(s) the diff touches. Name the specific skill(s) explicitly in each spawn prompt.

Other conditional agents

| Condition | Agent (subagent_type) | Focus | |---|---|---| | Documentation changes | claudius:technical-writer-trillian | Accuracy, completeness, API docs, changelog |

For crypto-heavy code or significant dependency changes, expand the single security-engineer's prompt to include crypto soundness and dependency audit — do NOT spawn a second instance.

Scaling for large codebases

For 50+ files / 5000+ lines, spawn multiple agents of the same type with different file scopes.

3. Craft Agent Prompts

Beyond the general agent prompt requirements, every review agent prompt MUST include:

1. Comparison base: how to see what changed (git show <base>:<file> or git diff)
2. Finding format: the severity levels and structure below
3. Review checklists: embed relevant checklist content or rely on preloaded skills
4. BP preload: every spawned reviewer (security-engineer-smythe, project-reviewer-adams, qa-engineer-marvin, technical-writer-trillian, etc.) MUST preload coding-best-practices so its Cross-Cutting Rules govern every finding — state this explicitly in each spawn prompt
5. UX/DX lens: assess how findings affect end-user workflows and developer experience, not just code correctness
6. CI context: when MemCan/WebSearch are unavailable (e.g., CI), instruct: "Do not use memcan tools or WebSearch/WebFetch."
7. File output: use the Write tool for creating files — never cat > file or heredoc redirections
8. Full roster: list every teammate name, role/focus, and file scope in this fan-out, including conditional and scaled reviewers; state that all listed peers are already live so agents do not pause to ask or spawn duplicates
9. Cross-domain hints: passively report any issue noticed in a peer's primary domain rather than hunting outside the assigned scope, silently duplicating it, or omitting it; tag the finding with cross_domain_hint: "<peer-role>" so consolidation can weigh the overlap
10. UI-text scan: scan the diff's user-visible strings — labels, buttons, toasts, dialogs, error messages — for raw exception text, stack traces, error codes, internal jargon, or alarming wording on a benign condition; these trip G-UI-TEXT (claudius:severity)
11. Context Digest (verbatim, when the invoker supplied one — defined in review-pr § Context Digest; never restate or reinvent its contents): pass it as its own numbered item with this rule attached — the digest adjusts scoring (via claudius:severity's non-adversarial likelihood recipe), it never suppresses reporting: report the finding with context-adjusted floats, never drop it; a field marked unknown changes nothing.
12. Worktree isolation (mandatory upfront, not reactive): any agent instructed to git checkout/build/test the reviewed branch MUST be told to work in a pre-created isolated worktree in its FIRST spawn prompt — never bolted on as a follow-up correction after it has already touched the shared tree (see grand-admiral § Worktree Isolation for setup). A reactive correction arrives too late: the checkout already happened, flipping HEAD under any other agent concurrently reading the same shared tree.
13. Cross-branch isolation, reviewing sibling PRs in one session: when this session is reviewing more than one branch/PR against the same repo, tell every agent to verify any symbol, function, or API it cites — in findings, positives, or recommendations — actually exists on the branch it was assigned (git show <its-target-ref>:<file>), not a sibling branch reviewed in the same session. A shared "positives" blurb or boilerplate recommendation reused across findings is exactly where a sibling branch's content leaks in unnoticed.

Finding format (JSON)

Agents MUST write findings to the specified file path as a JSON array of finding_section objects:

[
  {
    "title": "Section Title",
    "category": "security|project|code_quality|dependencies|documentation|call_tree",
    "findings": [
      {
        "id": "PREFIX-001",
        "likelihood": 0.6,
        "impact": 0.7,
        "relevance": 0.5,
        "title": "Short finding title",
        "tags": ["A03 Injection", "CWE-79"],
        "location": "src/auth.rs:42-56",
        "description": "What the issue is and why it matters",
        "impact_description": "What could go wrong (Markdown narrative)",
        "recommendation": "How to fix it",
        "code_snippets": [
          {"language": "rust", "caption": "auth.rs:42", "content": "let user = unwrap_token(&hdr);"}
        ]
      }
    ],
    "positives": "Optional positive observations"
  }
]

Required finding fields: id, likelihood/impact/relevance (floats 0.0–1.0), title, location, description, recommendation. See claudius:severity for the float definitions, the backstop-zone impact cap, and the band table the coordinator uses to derive integer severity. Rate relevance as real PR-goal fit per claudius:severity — never default it to 1.0. The floats are the single source of truth; never hand-type a severity label.

Optional: tags, impact_description (Markdown impact narrative; the numeric impact float is separate), code_snippets (only when you captured the exact source during analysis — never invent one), cross_domain_hint (a peer role whose primary domain owns an issue noticed incidentally; never actively search that domain).

Producers must NOT emit (downstream-owned): overall_severity, location_permalink, any metadata/commit/repository/date/branch field, ai_assessment, ai_verdict, ai_verdict_confidence, merge_class, intent_basis, and the derived integer severity when emitting floats. likelihood/impact/relevance are required — without all three the coordinator cannot derive overall_severity and the schema rejects the finding. The validate-findings skill is the only documented path to populate floats post-hoc.

Metadata is coordinator-owned: producers emit only the bare finding_section[] array, with no envelope object or metadata fields. The coordinator resolves the full 40-character commit SHA (git rev-parse @{u}, falling back to git rev-parse HEAD when the branch has no upstream) and supplies commit/date/branch/project through prepare --metadata; prepare derives repository metadata from --repo-root.

Hoist the invariant part into a file, don't restate it per spawn. Items 2–13 above are identical across every producer in a fan-out; with N producers, retyping them N times costs the coordinator real output tokens for zero variable content (measured: ~2500 lines across 5 producers on one large review). Before spawning, copy references/producer-contract.md to <SCRATCH_DIR>/producer-contract.md unmodified — it already contains the finding-format JSON contract, the producers-must-NOT-emit list, the ID-prefix table, the call-tree/UI-text/UX-DX/collision/process rules, and the terse report-back instruction (everything below that has no per-agent variable). Then each spawn prompt carries only what actually varies:

Read <SCRATCH_DIR>/producer-contract.md and <SCRATCH_DIR>/context-digest.md (if present) before emitting anything — both apply to your output.

Deployed peers (all already live; do not ask whether they are running):
- <teammate-name> — <reviewer role/focus> — <file scope>
- <teammate-name> — <reviewer role/focus> — <file scope>

Your role: <role>. Your file scope: <scope>. Write your findings to <SCRATCH_DIR>/<role>-findings.json.

This also makes the fan-out auditable after the fact — archive producer-contract.md next to report.json so a reader can see exactly what producers were told, same as context-digest.md.

ID prefixes: SEC- security, PROJ- project, QA-/CODE-/RUST-/PY-/GO-/FE- code quality (jointly owned by project-reviewer-adams and qa-engineer-marvin — see report-format's ID-prefix table; prefix reflects finding category/language, not agent identity), DOC- docs, CALL- call-tree. Agents assign provisional sequential IDs within their prefix (e.g., SEC-001, SEC-002); collisions across parallel agents are fine — consolidation (5c) deduplicates and reassigns final IDs.

Location MUST include the full file path (e.g., src/auth.rs:42-56), never bare line numbers.

Severity levels: CRITICAL > HIGH > MEDIUM > LOW > INFO (see severity skill).

Tags: classification references — OWASP (A01–A10), CWE, language best-practice IDs, etc. Tag ALL security findings with OWASP categories; non-security findings may omit tags.

Call-tree inspection

When the diff modifies or removes any function/method declaration, every code-quality reviewer agent MUST run a deep transitive in-repo caller walk before emitting findings. Methodology: references/call-tree-walk.md — read it once per review and follow the steps.

Finding shape: category: "call_tree", ID prefix CALL- (provisional from producer, coordinator reassigns). Every call_tree finding's description MUST start with a Walked via: <tool> line so the reader can judge walk depth and tool quality.

Skip the walk for pure additions, doc-only PRs, and changes confined to test files.

Ephemeral-ID lint

After each agent emits findings, run the dumb ephemeral-ID lint against the diff:

git diff "${BASE_BRANCH}...HEAD" | python3 ${CLAUDE_SKILL_DIR}/../../scripts/lint_ephemeral_ids.py --diff

For each hit, judge genuine violation vs quoted/escaped example (a code fence demonstrating the rule, a test fixture asserting it, this lint's own docstring). Dismiss in-skill examples; promote genuine violations to code_quality findings with tags: ["ephemeral-id-reference"] and ID prefix CODE- (coordinator-assigned). The lint always exits 0 — judgement is yours.

4. Spawn Agents

This skill runs inline (not forked) specifically so it can spawn reviewer agents. For any non-trivial review, confirm the Agent tool is available before fanning out. If not (e.g. executing inside a subagent, which cannot spawn nested agents), STOP and report that the review cannot fan out — do NOT silently fall back to a single self-run review. The single-agent TRIVIAL path in §1/§2 is the only legitimate one-agent review; every non-trivial review REQUIRES fan-out.

Spawn all agents in parallel per the general spawning guidelines, with fixed per-role model tiering: claudius:security-engineer-smythe on opus, claudius:project-reviewer-adams on opus, claudius:qa-engineer-marvin on sonnet (matches claudius:delegate § Token Economy). This replaces the old "opus for all by default" rule.

Model override (user-requested; confirm before downgrading Smythe): on explicit request (e.g. "review with Sonnet") the user may force a uniform model override across all 3 agents. Apply it to Adams and Marvin freely. Before applying an override that would downgrade security-engineer-smythe below opus, STOP and confirm the user really means it — security depth is not silently traded away by a blanket model request. Once confirmed, apply to all three including Smythe.

Example spawn pattern:

Agent(subagent_type="claudius:security-engineer-smythe", model="opus", prompt="...", name="security-auditor")
Agent(subagent_type="claudius:project-reviewer-adams", model="opus", prompt="...", name="project-reviewer")
Agent(subagent_type="claudius:qa-engineer-marvin", model="sonnet", prompt="...", name="qa-reviewer")

5. Consolidate Findings

After all agents complete, use the two-phase consolidation script. It automates the mechanical work (flattening, duplicate detection, ID assignment, statistics), leaving judgment calls (dedup merging, severity re-assessment, executive summary) to you.

5a. Phase 1 — Prepare

Flatten all agent reports, detect duplicate candidates, scan for INTENTIONAL comments:

python3 ${CLAUDE_SKILL_DIR}/../../scripts/consolidate_reports.py prepare \
    security-engineer:"$SCRATCH_DIR"/security-findings.json \
    project-reviewer:"$SCRATCH_DIR"/project-findings.json \
    qa-engineer:"$SCRATCH_DIR"/qa-findings.json \
    --repo-root $(git rev-parse --show-toplevel) \
    --output "$SCRATCH_DIR"/intermediate.json \
    --metadata '{"project":"...","date":"...","branch":"...","commit":"..."}'

Produces intermediate.json: flattened raw_findings (with agent attribution), duplicate_groups (candidate clusters with overlap reasons), intentional_downgrades (findings near INTENTIONAL comments), and section_positives.

5b. Review and merge (LLM judgment)

Read intermediate.json and decide:

1. Duplicate resolution: per duplicate_groups entry, merge (keep the most detailed description, union tags) or keep separate. Remove redundant findings.
2. INTENTIONAL downgrade: downgrade each intentional_downgrades finding to INFO — deliberate engineering decisions from previous triage.
3. Severity re-evaluation: load the severity skill (/severity), then re-assess every finding strictly against its criteria — agents often over-inflate.
4. Merge classification: assign merge_class per severity skill § Merge Classification — blocking only when a blocker gate trips, with intent_basis naming the gate ID plus one line of evidence. Use the Context Digest when the invoker supplied one (review-pr § Context Digest) for G-INTENT judgment; with no PR context, derive intent from your own knowledge of the work's goal — the coordinator often knows the bigger picture the producers don't. Apply the digest as a coordinator-side backstop too: re-check any finding whose floats ignore an evidenced operational-profile claim a producer plainly didn't have (severity skill § likelihood). Severity never determines merge_class. Escalate to the human explicitly (never silently defer) any pre-existing finding tripping G-FUNDS/G-SECRET/G-CRYPTO/G-DATA.
5. Merge sections: combine same-category agent sections into unified sections.
6. Executive summary: write overall_assessment, summary_text, verdict_text, verdict_action — LLM-authored, but it must not contradict the merge classification; reflect every valid blocking finding.
7. Agent stats: copy intermediate.json's agent_stats array verbatim into merged-findings.json — prepare already computes it; do not hand-author or reshape it.

For reviews above roughly 30 raw findings, use the ready-to-run merge helper instead of transcribing the entire document by hand. Record the review-specific judgment in "$SCRATCH_DIR"/merge-decisions.json: each true duplicate cluster names its members by agent + original_id, selects one member as the base, records a reason, and supplies only the hand-authored merged fields in updates. Include the step 6 executive_summary in the same file. Do not list candidate clusters you decide to keep separate.

{
  "executive_summary": {
    "overall_assessment": "...",
    "summary_text": "...",
    "verdict_text": "...",
    "verdict_action": "..."
  },
  "merges": [
    {
      "reason": "Both findings describe the same unchecked parser failure.",
      "members": [
        { "agent": "security", "original_id": "SEC-001" },
        { "agent": "qa", "original_id": "QA-003" }
      ],
      "base": { "agent": "security", "original_id": "SEC-001" },
      "updates": {
        "description": "Hand-authored merged text.",
        "tags": ["..."],
        "code_snippets": [
          { "language": "...", "content": "..." }
        ]
      }
    }
  ]
}

For every field combined from peers, put the complete merged value in updates (for example, the union of tags or code_snippets). The helper does not decide which findings overlap. It shallow-copies untouched findings, applies only the declared cluster merges, combines same-category sections, and copies metadata, section_positives, and agent_stats from intermediate.json:

python3 ${CLAUDE_SKILL_DIR}/../../scripts/merge_findings_helper.py \
    --input "$SCRATCH_DIR"/intermediate.json \
    --decisions "$SCRATCH_DIR"/merge-decisions.json \
    --output "$SCRATCH_DIR"/merged-findings.json

Before assembly, finish the per-finding edits required by steps 2–4, verify the combined sections and executive summary from steps 5–6, and keep merge-decisions.json in the scratch directory so each merge remains auditable.

Write the result as "$SCRATCH_DIR"/merged-findings.json. Its agent_stats value is the unchanged array copied from intermediate.json:

{
  "metadata": { "project": "...", "date": "...", ... },
  "executive_summary": { "overall_assessment": "...", ... },
  "findings": [ { "title": "...", "category": "...", "findings": [...], "positives": "..." } ],
  "agent_stats": [ { "agent": "...", "unique": N, "redundant": N } ],
  "top_findings_override": null,
  "remediation_override": null
}

Findings do NOT need id fields — phase 2 assigns them. Set top_findings_override/remediation_override to a JSON array to override auto-generation, or null to auto-generate.

5c. Phase 2 — Assemble

python3 ${CLAUDE_SKILL_DIR}/../../scripts/consolidate_reports.py assemble \
    --input "$SCRATCH_DIR"/merged-findings.json \
    --output ${REPORT_DIR:-.}/report.json

Assigns sequential IDs by category (SEC-001, PROJ-001, RUST-001, etc.), computes summary_statistics (severity counts, category matrix, redundancy ratio), generates top_findings from CRITICAL/HIGH items, and creates remediation priority buckets. Validates against the schema and REFUSES to write output on failure (exit 1) — validation is mandatory; jsonschema is a hard requirement.

5d. Validate report against schema

Assemble already validates and blocks output, but re-validate manually after hand-editing the report:

python3 ${CLAUDE_SKILL_DIR}/../../scripts/validate_report.py report.json

If validation fails, fix merged-findings.json and re-run assemble. Do NOT skip validation.

5e. Render markdown report

python3 ${CLAUDE_SKILL_DIR}/../../scripts/generate_review_report.py ${REPORT_DIR:-.}/report.json --format md

Produces report.md next to the JSON file.

When presenting results, filter the consolidated findings for `merge_class == "out_of_scope_

<!-- Content truncated for initial SEO render. Open the source file tab for the full file. -->