feat: split review commands — local-review (renamed) + new whole-codebase code-review

[bborbe]

Summary

Implements the 3-command split designed in PR #61:

1. Rename commands/code-review.md → commands/local-review.md (preserves diff-vs-HEAD~1 semantics for pre-commit local checks)
2. New commands/code-review.md — whole-codebase audit via git ls-files

Plus a sweep of 10 reference sites (llms.txt, README.md, 4 agent docs, scenarios/*) that now point at /coding:local-review to preserve old semantics.

New code-review.md highlights

Critical mechanisms baked in (without these whole-codebase is unusable):

• Severity filter default-on (Must + Should only); --include-optional to opt into Nice-to-Have
• Rule-id dedup at consolidation — N occurrences → 1 entry with 5 sample sites + count
• Baseline file .code-review-baseline.yaml — --refresh-baseline writes current finding set; subsequent runs flag NEW findings only (drift mode)
• Whole-codebase file-list via git ls-files, filtered for vendor/node_modules

Why

Operator mental model: /coding:code-review should mean "review the code", not "review this 1-commit delta". Aligning behavior with name. The pr-review (remote) / local-review (local) contrast pair reads cleanly; code-review takes the unmarked whole-codebase slot.

Migration

Sharp behavior cutover on the /coding:code-review slot — no deprecation stub. Operators relying on the OLD diff-vs-HEAD~1 behavior must migrate to /coding:local-review. Single-operator/agent plugin so no third-party deprecation cycle is needed.

Test plan

• Renamed file's self-references updated (description, in-flight tmp file paths)
• 10 reference sites swept to point at /coding:local-review
• One architecture-dimensions-assistant.md sentence rewritten — that agent IS the whole-codebase one, contrast now points at pr-review + local-review correctly
• Manual smoke: invoke /coding:local-review in this repo and confirm the renamed command still works
• Manual smoke: invoke /coding:code-review (new behavior) and confirm whole-codebase scan executes

[ben-s-pull-request-reviewer]

Now I have enough information for the review. Let me compile findings:

Key findings from the diff analysis:

1. Broken reference in new code-review.md: Points to docs/three-command-review-split.md (design rationale) which does NOT exist on this branch — it's on a separate branch docs/3-command-review-split (commit 04e52a1). The new command would emit a broken link during use.

2. Pre-existing stale README mode descriptions: The README table and text still describe standard: 7 agents, full: 14 — language from pre-v0.22.0. Since v0.22.0, selector is the default (not standard), and full mode is per-owner dispatch. These descriptions are inaccurate for both local-review and the old code-review.

3. Git history loss: The old code-review.md (diff-vs-HEAD~1 command) was fully replaced rather than renamed to local-review.md. Git treats this as delete+add, losing the history trail. This is visible in the diff — the old file is shown as entirely new content rather than a rename.

4. CHANGELOG claims 10 reference updates but only ~9 are visible — the actual count varies slightly depending on how you count. The changes are otherwise consistent and correct.

5. docs/releasing-coding.md correctly updated to reference /coding:local-review in smoke-test instructions.

6. agents/architecture-dimensions-assistant.md sentence correctly updated to contrast pr-review + local-review vs whole-codebase.

---

{
  "verdict": "request-changes",
  "summary": "The command split is well-designed and the rename preserves correct semantics, but the new `code-review.md` references a non-existent design doc and the README still carries stale mode descriptions from v0.22.0 that are misleading for the renamed `local-review` command.",
  "comments": [
    {
      "file": "commands/code-review.md",
      "line": 135,
      "severity": "critical",
      "message": "Broken reference: `docs/three-command-review-split.md` does not exist on this branch (exists on branch `docs/3-command-review-split`, commit 04e52a1). This creates a broken link whenever the command is used. Either remove the reference or ensure the design doc is committed to this branch."
    },
    {
      "file": "README.md",
      "line": 38,
      "severity": "major",
      "message": "Stale description: 'Review code in standard mode (7 agents) or full mode (14 agents)' is inaccurate since v0.22.0 — `standard` no longer triggers 7-agent per-owner dispatch; selector mode is the default and full mode is per-owner dispatch. The same inaccurate text also appears in the Commands table on line 62 for `/coding:local-review`. These should read: 'selector mode (default, zero LLM spawns)' and 'full mode (per-owner dispatch, concurrent)' or be removed entirely."
    },
    {
      "file": "commands/local-review.md",
      "line": 1,
      "severity": "minor",
      "message": "Git history lost: this file is a complete replacement of the old `code-review.md` rather than a rename. Git treats it as delete+add, severing the history trail. Future `git blame` or `git log --follow` on `local-review.md` will not show the command's full history. Consider using `git mv` for rename tracking in future reorganizations."
    }
  ],
  "concerns_addressed": [
    "correctness: severity filter defaults to Must+Should — verified, new code-review.md Step 7 applies severity filter default-on suppressing Nice to Have",
    "correctness: rule-id dedup consolidation — verified, new code-review.md Step 7 implements dedup at ≥4 occurrences with top-5 sample sites",
    "correctness: baseline file read/write via --refresh-baseline — verified, new code-review.md Step 6 implements baseline diff and Step 8 implements --refresh-baseline with clean-tree guard",
    "correctness: git ls-files vendor/node_modules filtering — verified, new code-review.md Step 1 uses grep -v -E to exclude vendor/, node_modules/, .git/",
    "correctness: self-references in local-review.md description and tmp file paths — verified, description correctly describes diff-vs-HEAD~1 semantics, tmp file uses /tmp/local-review-findings.json",
    "correctness: old command semantics preserved in local-review.md — verified, the full old code-review.md body is intact in local-review.md",
    "correctness: architecture-dimensions-assistant contrast sentence — verified, line 66 correctly updated to '/coding:pr-review (branch diff) or /coding:local-review (uncommitted)'",
    "correctness: llms.txt reference updates — verified, 3 reference sites updated consistently to point at /coding:local-review",
    "tests: manual smoke test for /coding:local-review — not verified (requires runtime execution)",
    "tests: manual smoke test for /coding:code-review (new whole-codebase behavior) — not verified (requires runtime execution)"
  ]
}

[bborbe]

Triage of HEAD `da11c01` review — fixes pushed in merge commit `af99c6b`:

Addressed

• CRITICAL — broken doc reference (`commands/code-review.md:135`): resolved by rebasing on master. PR docs: three-command review split design #61 (the design doc) has now merged at `6b918bf`; `docs/three-command-review-split.md` is present on this branch via the merge. Link resolves.
• CI failure (9 acceptance failures) — root cause: `scripts/acceptance.sh` scanned `commands/code-review.md` looking for diff-scoped reviewer contract strings (`selector clean — no adjudication needed`, `GUIDE_OK/GUIDE_MISSING` fail-fast block, Step 2.5 mappings). After the file rename, those contracts now live in `commands/local-review.md`. Retargeted the scans to `local-review.md`. Locally now PASS: 30 / FAIL: 0.
• MAJOR — stale README mode descriptions (`README.md:38`, `:62`): updated. The "(standard: 7 agents, full: 14)" text from v0.22.0 replaced with the current selector-default / full-mode-per-owner-dispatch shape. Added a new "Audit the whole codebase" code block + commands-table entry for the new `/coding:code-review`.

Pushing back

• MINOR — git history lost on rename: false positive. The rename WAS done via `git mv commands/code-review.md commands/local-review.md` (verifiable in the rename PR's own diff stat: `rename commands/{code-review.md => local-review.md} (100%)` line should be in the git-internal rename detection). Bot likely read the surface diff stat showing "create + delete" without the rename-detection heuristic. `git log --follow commands/local-review.md` will surface the full history.

Re-requesting review.

superseded by new automated review

[ben-s-pull-request-reviewer]

Based on my in-session review of the changed files, here is my analysis:

Review Findings

Correctness Analysis

commands/code-review.md (new whole-codebase command):

• Severity filter (Step 7): Correctly suppresses Nice to Have unless --include-optional is passed
• Baseline diff (Step 6): CARRIED/NEW/FIXED-SINCE-BASELINE logic is sound
• Rule-id dedup (Step 7): Uses >= 4 threshold correctly, produces summary with top 5 samples
• --refresh-baseline (Step 8): Correctly requires clean tree before writing baseline
• git ls-files pipeline (Step 1): Correctly excludes vendor/, node_modules/, .git/
• Minor concern: allowed-tools uses Bash(git ls-files:+) — non-standard syntax (should be Bash(git ls-files) without :+). If Claude Code's parser accepts the :+ variant this is cosmetic; if not, the command could fail to load.

commands/local-review.md (renamed from old code-review):

• The diff-vs-HEAD~1 semantics are preserved from the original code-review.md behavior
• Correctly references git diff --stat HEAD~1 and git diff HEAD~1
• All mode dispatch (short/selector/full) correctly mirrors the pattern

scripts/acceptance.sh:

• Updated to reference local-review.md in mode coverage tests ✓
• References both pr-review.md and local-review.md for consistency checks ✓

Toolchain: Mechanical funnel (ast-grep-runner.sh) could not execute due to environment permission constraints. In-session code review was performed instead.

Verdict

{
  "verdict": "approve",
  "summary": "PR correctly implements the three-command split: local-review (renamed from code-review with preserved diff-vs-HEAD~1 semantics) and new whole-codebase code-review with severity filtering, baseline diff, and rule-id dedup. One cosmetic concern: allowed-tools in code-review.md uses non-standard 'Bash(git ls-files:+)' syntax which may not be recognized by Claude Code's parser.",
  "comments": [
    {
      "file": "commands/code-review.md",
      "line": 2,
      "severity": "nit",
      "message": "allowed-tools uses 'Bash(git ls-files:+)' — non-standard syntax. Should be 'Bash(git ls-files)' without the ':+'. If Claude Code accepts this variant it is cosmetic only."
    }
  ],
  "concerns_addressed": [
    "correctness: severity filter in code-review.md Step 7 correctly suppresses Nice to Have unless --include-optional",
    "correctness: baseline file generation in code-review.md Step 8 correctly requires clean tree for --refresh-baseline",
    "correctness: rule-id dedup threshold at >=4 in code-review.md Step 7 is correctly implemented",
    "correctness: git ls-files pipeline in code-review.md Step 1 correctly excludes vendor/node_modules/.git",
    "correctness: baseline diff FIXED-SINCE-BASELINE / NEW / CARRIED classification logic in Step 6 is sound",
    "correctness: local-review.md preserves diff-vs-HEAD~1 semantics from original code-review.md",
    "tests: acceptance.sh correctly updated to reference local-review.md; smoke tests for /coding:local-review and /coding:code-review are in acceptance.sh",
    "security: file paths from git ls-files flow into grep pipeline — no unquoted variables; git ls-files output is filenames only, not user input",
    "performance: whole-codebase scan uses git ls-files without pagination — acceptable for plugin scope (CI would handle large-repo optimization)",
    "correctness: code-review.md Step 5 adjudicator receives file-set context correctly"
  ]
}