Skip to content

proposal: portable code review system#6

Draft
nick-inkeep wants to merge 1 commit intomainfrom
proposal/portable-review
Draft

proposal: portable code review system#6
nick-inkeep wants to merge 1 commit intomainfrom
proposal/portable-review

Conversation

@nick-inkeep
Copy link
Contributor

Summary

Spec proposal to make the PR review system portable — enabling local execution without a GitHub PR.

  • Problem: The review system is tightly coupled to GitHub (CI workflow, Pending Review API, blob URLs). Reviews can't run locally or as a composable step in /ship.
  • Solution: Add a local execution path alongside the existing GitHub CI path. Both share the same 15 child reviewers and Phases 1-4 orchestration core. Only Phase 5-6 (output delivery) differs per mode.
  • Integration: Local review becomes /ship Phase 3 (pre-push quality gate), invoked as a claude -p subprocess via pr-review.sh — same pattern as /implement.

Key decisions (11 confirmed, 1 deferred)

ID Decision Status
D1 pr-context generated by bash shell script from git state Confirmed
D2 Markdown output (not JSON) — no non-LLM consumer exists Confirmed
D3 Shared core + two thin orchestrator shells (GitHub & local) Confirmed
D4 Three-dot diff, user-configurable target branch Confirmed
D5 Local review = /ship Phase 3, before draft PR creation Confirmed
D6 /ship controls iteration (max 2), methodically evaluates suggestions Confirmed
D7 Rename pr-review-*review-* Deferred (avoids cross-repo blast radius)
D8 claude -p subprocess invocation (not Task tool — nesting limitation) Confirmed
D9 Phase 1-4 extraction: copy verbatim with 5 surgical edits Confirmed
D10 Scripts in bash (matches repo convention) Confirmed
D11 CLI flag combo -p + --plugin-dir + --agent — smoke test at impl time Confirmed

Open questions (2 remaining — non-blocking)

  1. A4 — Shared core delivery mechanism: Reference file (proven, LOW risk) vs. frontmatter skill (architecturally clean, MEDIUM-HIGH risk). See SPEC.md §9 Component 3.
  2. Q9 — Output contract reference format: Dual-format guidance in contract (~30 lines) vs. move concern entirely to pr-context. See SPEC.md §9 Component 3.

Both can be resolved at implementation time.

Evidence files

  • evidence/coupling-analysis.md — children are platform-agnostic (zero GitHub refs)
  • evidence/ship-review-integration.md — /ship ↔ /review integration pattern
  • evidence/ship-phase-sequence.md — /ship phases, claude -p subprocess pattern
  • evidence/naming-analysis.md — rename blast radius (~375 occurrences, deferred)

What's not in this proposal

  • Phase planning (§13) — to be designed during implementation
  • Actual code changes — this is spec only

🤖 Generated with Claude Code

Add spec and evidence for making the PR review system portable. The system
currently requires a GitHub PR; this proposal adds a local execution path
so reviews can run as a pre-push quality gate or composable /ship step.

Key decisions: shared Phases 1-4 core + two thin orchestrator shells,
bash shell scripts for pr-context generation, claude -p subprocess
invocation (same pattern as /implement), markdown output, 2-iteration
cap in /ship flow. Two open questions remain: delivery mechanism for
shared core (reference file vs skill) and output contract reference
format adaptation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant