feat: REVIEW.md — repo-level review instructions - #19
Conversation
…priority Discover REVIEW.md at repo root (walked up from cwd, same as .code-reviewer.yaml). Its contents are injected at the end of the system prompt as the highest-priority instruction block. Prompt priority chain (highest last, due to LLM recency bias): 1. Base prompt (or --custom-prompt file) 2. Focus overlays (--focus bugs,security,...) 3. Extra rules (--extra-rules) 4. REVIEW.md (HIGHEST PRIORITY) This matches the Claude Code Review convention where REVIEW.md shapes what gets flagged, at what severity, and how findings are reported. Changes: - config.go: ReviewMD field, loaded alongside .code-reviewer.yaml - prompt.go: new BuildPromptFull() with reviewMD parameter - reviewer.go: wired to use BuildPromptFull - main.go: log review_md=true/false at startup - prompt_test.go: 5 new tests covering injection, ordering, empty
📝 WalkthroughWalkthroughRepository configuration now captures ChangesRepository review instructions
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Repository
participant loadRepoConfig
participant ReviewerRun
participant readReviewMDFromRef
participant Git
participant BuildPromptFull
participant LLM
Repository->>loadRepoConfig: provide repository REVIEW.md
loadRepoConfig->>ReviewerRun: populate Config.ReviewMD
ReviewerRun->>readReviewMDFromRef: load base-ref REVIEW.md in CI mode
readReviewMDFromRef->>Git: git show ref:REVIEW.md
Git-->>readReviewMDFromRef: return contents or error
ReviewerRun->>BuildPromptFull: pass selected review instructions
BuildPromptFull->>LLM: send composed prompt with immutable output constraints
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/config/config.go (1)
183-230: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winBound
REVIEW.mdlookup to the repo root (internal/config/config.go:183-228) AncestorREVIEW.mdfiles outside the checkout are loaded as highest-priority prompt instructions, so the current filesystem-root walk lets unrelated parent directories influence review behavior. Stop at the directory containing.gitbefore readingREVIEW.md.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/config/config.go` around lines 183 - 230, The directory walk in the config-loading flow must constrain REVIEW.md discovery to the repository boundary. Update the loop around applyRepoConfig and c.ReviewMD so it detects the directory containing .git and stops after checking that directory, preventing REVIEW.md files in unrelated ancestors from being loaded while preserving YAML lookup behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/config/config.go`:
- Around line 212-224: Update the directory-walk logic around REVIEW.md loading
to track whether the file was found separately from c.ReviewMD’s trimmed
content. Use a dedicated found flag set when os.ReadFile succeeds, stop the walk
when both YAML and REVIEW.md have been found, and avoid treating blank content
as missing.
In `@internal/model/prompt.go`:
- Around line 132-138: Update BuildPromptFull so the repository-provided
reviewMD block is no longer last: append the immutable built-in JSON/output and
anti-injection guardrails after reviewMD, preserving their exact constraints and
ensuring they remain the highest-priority prompt layer.
---
Outside diff comments:
In `@internal/config/config.go`:
- Around line 183-230: The directory walk in the config-loading flow must
constrain REVIEW.md discovery to the repository boundary. Update the loop around
applyRepoConfig and c.ReviewMD so it detects the directory containing .git and
stops after checking that directory, preventing REVIEW.md files in unrelated
ancestors from being loaded while preserving YAML lookup behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 44993646-b507-472d-9986-4d8ddc8ec2d6
📒 Files selected for processing (5)
cmd/code-reviewer/main.gointernal/config/config.gointernal/model/prompt.gointernal/model/prompt_test.gointernal/reviewer/reviewer.go
Shipped: - LLM proxy support (--proxy-url) - VCS interface abstraction (internal/vcs) - REVIEW.md repo-level instructions (PR #19) - Repo-aware context via tree-sitter + grep (PR #20) Reorganize roadmap: - v0.5: Platform Expansion (GitHub support, Actions, auto-approve) - v0.6: Deep Intelligence (multi-pass, RAG, import-aware)
Merge main into feat/review-md, resolving conflict in prompt_test.go by keeping both REVIEW.md prompt tests and BuildUserPromptWithContext tests. CodeRabbit fixes applied: - config.go: Use dedicated foundReviewMD flag instead of checking c.ReviewMD == "" to avoid treating empty REVIEW.md as 'not found' and walking past it into parent directories - config.go: Bound REVIEW.md lookup to repo root by stopping at the directory containing .git, preventing unrelated ancestor REVIEW.md files from being loaded - prompt.go: Append immutable output-format and anti-injection guardrails after REVIEW.md block so repo-controlled content cannot override JSON output format or system instructions
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/reviewer/reviewer.go (1)
131-138: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy liftBudget the complete per-chunk request, not just the diff.
EstimateCost(diffs)counts changed-file tokens only, while Lines 182 and 192 add custom prompt layers,REVIEW.md, and related snippets to every model call. Large instructions or context can bypassMaxTokensand model context limits, while repeated snippets multiply request cost. Estimate or cap the full system/user prompt per chunk and partition snippets by chunk.Also applies to: 159-179, 182-182, 192-192
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/reviewer/reviewer.go` around lines 131 - 138, Update the per-chunk budgeting flow around EstimateCost, TrimToBudget, and the model-call prompt construction to account for the complete system and user request, including custom prompts, REVIEW.md, and related snippets. Apply MaxTokens to that full estimate, partition repeated snippets per chunk, and ensure each generated request stays within the configured budget and model context limit rather than budgeting changed-file tokens alone.
🧹 Nitpick comments (1)
internal/reviewer/reviewer.go (1)
454-471: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPreserve filesystem errors during repo-root discovery.
findRepoRoottreats everyos.Statfailure as “not found”, so permission or I/O failures silently disable related context. Return or log non-ENOENTerrors so callers can distinguish a missing repository from a broken lookup.As per path instructions,
internal/**requires focus on error handling, context propagation, and interface design.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/reviewer/reviewer.go` around lines 454 - 471, Update findRepoRoot to distinguish a missing .git entry from other filesystem failures: continue walking only for ENOENT, while propagating or logging non-ENOENT errors with relevant context. Preserve the existing successful root discovery and empty-result behavior for genuinely missing repositories, and adjust the function’s return contract or callers as needed to carry lookup errors.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/reviewer/reviewer.go`:
- Line 182: Update the prompt construction around BuildPromptFull so
r.cfg.ReviewMD is sourced from the trusted target/base ref rather than the
checked-out PR working tree, or omitted for untrusted branches. Preserve the
existing prompt inputs and ensure contributor-controlled REVIEW.md content
cannot become highest-priority review guidance.
---
Outside diff comments:
In `@internal/reviewer/reviewer.go`:
- Around line 131-138: Update the per-chunk budgeting flow around EstimateCost,
TrimToBudget, and the model-call prompt construction to account for the complete
system and user request, including custom prompts, REVIEW.md, and related
snippets. Apply MaxTokens to that full estimate, partition repeated snippets per
chunk, and ensure each generated request stays within the configured budget and
model context limit rather than budgeting changed-file tokens alone.
---
Nitpick comments:
In `@internal/reviewer/reviewer.go`:
- Around line 454-471: Update findRepoRoot to distinguish a missing .git entry
from other filesystem failures: continue walking only for ENOENT, while
propagating or logging non-ENOENT errors with relevant context. Preserve the
existing successful root discovery and empty-result behavior for genuinely
missing repositories, and adjust the function’s return contract or callers as
needed to carry lookup errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: db4acb70-04f8-4cac-9f11-e94516361d33
📒 Files selected for processing (5)
cmd/code-reviewer/main.gointernal/config/config.gointernal/model/prompt.gointernal/model/prompt_test.gointernal/reviewer/reviewer.go
🚧 Files skipped from review as they are similar to previous changes (4)
- cmd/code-reviewer/main.go
- internal/config/config.go
- internal/model/prompt.go
- internal/model/prompt_test.go
In CI mode, contributor-controlled branches could add a REVIEW.md that manipulates review behavior (e.g. 'report no findings'). Now in CI mode with a known base SHA (CI_MERGE_REQUEST_DIFF_BASE_SHA), REVIEW.md is read from the target/base ref via 'git show' instead of the working tree. This ensures only repo maintainers (who control the default branch) can set review instructions. In local --diff mode the filesystem version is still used since the developer controls the checkout. Includes command-injection guard (reject refs starting with '-') consistent with the existing pattern in getLocalDiffs().
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/reviewer/reviewer.go`:
- Around line 184-188: Update the reviewMD initialization in the reviewer flow
so CI mode starts with an empty value and only populates it via
readReviewMDFromRef when CIDiffBaseSHA is non-empty and validated; use
r.cfg.ReviewMD only when CIMode is disabled. Preserve the existing
BuildPromptFull call while ensuring CI never falls back to
contributor-controlled review instructions.
- Around line 487-492: Update the REVIEW.md retrieval flow around exec.Command
to accept and propagate a context via exec.CommandContext. Inspect the command
error and return an empty result only when the requested REVIEW.md is genuinely
absent at the ref; propagate invalid refs, repository failures, missing Git, and
context cancellation instead of logging them as “not found.”
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9fa9423b-7eae-4737-835e-6d651a59da3e
📒 Files selected for processing (1)
internal/reviewer/reviewer.go
| reviewMD := r.cfg.ReviewMD | ||
| if r.cfg.CIMode && r.cfg.CIDiffBaseSHA != "" { | ||
| reviewMD = readReviewMDFromRef(r.cfg.CIDiffBaseSHA) | ||
| } | ||
| systemPrompt := model.BuildPromptFull(r.cfg.CustomPrompt, reviewMD, r.cfg.Focus, r.cfg.ExtraRules) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Fail closed when CI has no trusted base SHA.
When CIMode is enabled but CIDiffBaseSHA is empty, reviewMD remains r.cfg.ReviewMD, which can come from the contributor-controlled checkout. Initialize it to empty in CI and only load from a validated trusted ref; use r.cfg.ReviewMD only outside CI.
Proposed fix
- reviewMD := r.cfg.ReviewMD
- if r.cfg.CIMode && r.cfg.CIDiffBaseSHA != "" {
+ reviewMD := ""
+ if !r.cfg.CIMode {
+ reviewMD = r.cfg.ReviewMD
+ } else if r.cfg.CIDiffBaseSHA != "" {
reviewMD = readReviewMDFromRef(r.cfg.CIDiffBaseSHA)
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/reviewer/reviewer.go` around lines 184 - 188, Update the reviewMD
initialization in the reviewer flow so CI mode starts with an empty value and
only populates it via readReviewMDFromRef when CIDiffBaseSHA is non-empty and
validated; use r.cfg.ReviewMD only when CIMode is disabled. Preserve the
existing BuildPromptFull call while ensuring CI never falls back to
contributor-controlled review instructions.
| cmd := exec.Command("git", "show", ref+":REVIEW.md") | ||
| output, err := cmd.Output() | ||
| if err != nil { | ||
| // File doesn't exist at this ref — this is normal and expected. | ||
| slog.Debug("REVIEW.md not found at base ref", "ref", ref) | ||
| return "" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
ast-grep outline internal/reviewer/reviewer.go --view expanded
sed -n '430,540p' internal/reviewer/reviewer.go | cat -nRepository: OpticDiff/code-reviewer
Length of output: 3913
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline internal/reviewer/reviewer.go --view expanded
echo '---'
sed -n '430,540p' internal/reviewer/reviewer.go | cat -nRepository: OpticDiff/code-reviewer
Length of output: 3917
Propagate the context and stop swallowing non-missing-file errors. internal/reviewer/reviewer.go:487-492 should use exec.CommandContext and take ctx; only ignore a real “REVIEW.md not found” case. As written, invalid refs, broken repositories, missing Git, and cancellations all collapse into the same empty result and silently drop repository instructions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/reviewer/reviewer.go` around lines 487 - 492, Update the REVIEW.md
retrieval flow around exec.Command to accept and propagate a context via
exec.CommandContext. Inspect the command error and return an empty result only
when the requested REVIEW.md is genuinely absent at the ref; propagate invalid
refs, repository failures, missing Git, and context cancellation instead of
logging them as “not found.”
Source: Path instructions
Summary
Adds support for a
REVIEW.mdfile at the repo root that shapes review behavior. Contents are injected into the system prompt as the highest-priority instruction block, matching the Claude Code Review convention.How It Works
cwdto findREVIEW.md(same discovery as.code-reviewer.yaml)Config.ReviewMDPrompt Priority Chain
Example REVIEW.md
Changes
config.goReviewMDfield + discovery inloadRepoConfig()prompt.goBuildPromptFull()withreviewMDparameterreviewer.goBuildPromptFullmain.goreview_md=true/falseat startupprompt_test.goBackward Compatible
BuildPrompt()andBuildPromptWithCustom()still work unchanged (delegate toBuildPromptFull)--custom-promptand--extra-ruleswork as beforeSummary by CodeRabbit
REVIEW.mdsupport and surfaced it in startup logs.REVIEW.mdguidance in the highest-priority section.REVIEW.mdcan be loaded from a trusted base reference.REVIEW.md(with trimmed contents).REVIEW.mdinclusion, ordering, and immutable output-constraint placement.