feat: two-pass intent-aware review (v0.6) - #27
Merged
Conversation
Add --intent flag for intent-aware code review. Pass 1 infers the developer's intent (classification, risk, scope) via Summarize(). Pass 2 injects that intent as a prompt layer and reviews against it. Smart defaults: - CI mode: intent review ON by default - Local mode: intent review OFF by default - Override: --intent / --no-intent / YAML intent_review / REVIEW_INTENT env Intent-aware review rules (classification-specific): - feat → flag missing test coverage for new behavior - fix → verify root cause vs symptom fix - refactor → verify no behavioral changes introduced - Always: scope creep detection against inferred scope areas - Breaking changes → verify documentation exists New 'scope' finding category for intent-derived findings. Output: - Terminal: compact one-liner before findings - CI/GitLab: markdown intent table at top of MR comment Graceful fallback: if pass 1 fails, standard review runs. Files: - config.go: IntentReview/NoIntentReview fields, smart CI defaults - prompt.go: BuildIntentContext(), BuildPromptFull() intent layer - reviewer.go: pass 1 in Run(), output wiring - intent.go: formatIntentOneLiner(), formatIntentMarkdown() - 11 new tests across intent_prompt_test.go and intent_test.go
📝 WalkthroughWalkthroughAdds optional intent-aware two-pass reviews with configuration controls, intent-specific prompt context, pre-review summarization, token accounting, fallback behavior, and terminal or GitLab intent output. ChangesIntent-aware review
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Reviewer.Run
participant SummarizeProvider
participant BuildIntentContext
participant ReviewModel
participant GitLab
Reviewer.Run->>SummarizeProvider: Summarize numbered diff
SummarizeProvider-->>Reviewer.Run: SummaryResult and token usage
Reviewer.Run->>BuildIntentContext: Build developer intent context
BuildIntentContext-->>ReviewModel: Intent-aware system prompt
ReviewModel-->>Reviewer.Run: Review result
Reviewer.Run->>GitLab: Post summary with inferred intent markdown
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
internal/model/intent_prompt_test.go (1)
8-131: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer table-driven tests per coding guidelines. Both files use repeated, near-identical
TestXxxfunctions instead of a single table-driven test, which the guideline explicitly calls for.
internal/model/intent_prompt_test.go#L8-L131: consolidateTestBuildIntentContext_{Nil,FeatClassification,FixClassification,RefactorClassification,BreakingChanges,MinimalFields,ScopeCategory}into one table-driven test with per-caseSummaryResultinput andwantContains/wantNotContainsstring slices.internal/reviewer/intent_test.go#L14-L147: consolidateTestIntentReview_{InjectsContext,FallbackOnError,DisabledByDefault}into one table-driven test parameterized bycfg.IntentReview,summaryResult/summaryErr, and expectedsummarizeCalled/prompt-contains outcomes.As per coding guidelines, "
**/*_test.go: Prefer table-driven tests and mock through interfaces (ModelReviewer,VCSClient) rather than concrete types."🤖 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/model/intent_prompt_test.go` around lines 8 - 131, Consolidate the seven BuildIntentContext tests in internal/model/intent_prompt_test.go:8-131 into one table-driven test using per-case SummaryResult inputs and wantContains/wantNotContains assertions. Consolidate the three TestIntentReview cases in internal/reviewer/intent_test.go:14-147 into one table-driven test parameterized by cfg.IntentReview, summaryResult/summaryErr, expected summarizeCalled, and prompt contents; retain interface-based mocks such as ModelReviewer and VCSClient.Source: Coding guidelines
internal/model/prompt.go (1)
264-326: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueLabel this as model-inferred, not developer intent
BuildIntentContext()turns pass-1 model output into a system-prompt section; keep the header explicit (for example,MODEL-INFERRED INTENT) so it doesn’t read like authoritative developer guidance.🤖 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/model/prompt.go` around lines 264 - 326, Update BuildIntentContext to label the generated section as model-inferred intent rather than developer intent. Change the prompt header and any directly associated wording so it clearly indicates the content comes from pass-1 model output, while preserving the existing intent fields and review rules.Source: Coding guidelines
🤖 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 327-329: Update the configuration propagation around
rc.IntentReview so an explicitly provided false value also sets the
corresponding NoIntentReview state, preventing CI auto-default logic from
overriding it. Preserve the existing behavior for omitted values and true, and
align the change with the CI-default logic near the referenced configuration
handling.
- Around line 448-449: Update the intent flag registration in the configuration
flag setup to use local variables rather than binding directly to c.IntentReview
and c.NoIntentReview, preserving loaded YAML/environment values when flags are
absent. In the existing “Apply flags” section, explicitly set IntentReview and
NoIntentReview only when their corresponding local flags are true, following the
pattern used by noColor and summarize.
- Around line 198-206: Update the CI auto-enable logic in config validation to
distinguish an unset IntentReview value from an explicit false value supplied by
applyRepoConfig, preserving YAML intent_review: false in CI. Also prevent
automatic enabling when Summarize is enabled, so existing --ci --summarize
configurations remain valid with the validate() constraint. Adjust
applyRepoConfig and the surrounding IntentReview/NoIntentReview handling to
retain explicit-disable state without changing explicit CLI overrides.
- Line 154: Update the configuration documentation to cover the IntentReview
setting in repoConfig: add intent_review to .code-reviewer.example.yaml and the
README configuration/flags tables, including the --intent and --no-intent
options and their behavior. Keep the documented names aligned with the
IntentReview YAML key and existing CLI conventions.
In `@internal/model/prompt.go`:
- Around line 264-326: Update BuildIntentContext to replace every
WriteString(fmt.Sprintf(...)) pattern with fmt.Fprintf writing directly to sb,
including the Classification, Intent, RiskLevel, ScopeAreas, BreakingChanges,
and scope-creep rule messages; preserve all existing format strings and output.
In `@internal/reviewer/intent.go`:
- Around line 28-52: Update formatIntentMarkdown to pass s.Intent through
escapeTableCell before inserting it into the Intent cell, and escape each
BreakingChanges entry before joining and rendering them. Preserve the existing
table structure and formatting for all other fields.
In `@internal/reviewer/reviewer.go`:
- Around line 181-217: The intent-inference pre-pass in the main review flow
must apply the same token-limit safeguards as the later chunked review calls.
Update the logic around buildNumberedDiff and sp.Summarize to chunk diffs using
diff.TokenLimitForModel(r.cfg.Model), then summarize each chunk without sending
the entire diff in one request; preserve the existing fallback, intentContext,
and usage accumulation behavior.
- Around line 185-208: Update the IntentReview block around the
model.SummarizeProvider assertion to log a warning when intent review is enabled
but r.provider does not implement SummarizeProvider. Keep the existing
summarization and error-handling paths unchanged, and make the warning clearly
indicate that intent review was requested but skipped.
---
Nitpick comments:
In `@internal/model/intent_prompt_test.go`:
- Around line 8-131: Consolidate the seven BuildIntentContext tests in
internal/model/intent_prompt_test.go:8-131 into one table-driven test using
per-case SummaryResult inputs and wantContains/wantNotContains assertions.
Consolidate the three TestIntentReview cases in
internal/reviewer/intent_test.go:14-147 into one table-driven test parameterized
by cfg.IntentReview, summaryResult/summaryErr, expected summarizeCalled, and
prompt contents; retain interface-based mocks such as ModelReviewer and
VCSClient.
In `@internal/model/prompt.go`:
- Around line 264-326: Update BuildIntentContext to label the generated section
as model-inferred intent rather than developer intent. Change the prompt header
and any directly associated wording so it clearly indicates the content comes
from pass-1 model output, while preserving the existing intent fields and review
rules.
🪄 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: f3ab9d3c-9472-4fea-842e-2d78f7f29013
📒 Files selected for processing (7)
internal/config/config.gointernal/model/intent_prompt_test.gointernal/model/prompt.gointernal/model/prompt_test.gointernal/reviewer/intent.gointernal/reviewer/intent_test.gointernal/reviewer/reviewer.go
| APIURL string `yaml:"api_url"` | ||
| Summarize bool `yaml:"summarize"` | ||
| SummaryUpdateDescription bool `yaml:"summary_update_description"` | ||
| IntentReview *bool `yaml:"intent_review"` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -i 'code-reviewer.example.yaml'
fd -i 'README.md' | xargs -I{} rg -n -i 'intent' {}Repository: OpticDiff/code-reviewer
Length of output: 380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant config file and locate all references to the new fields.
fd -i 'config.go' internal
rg -n --hidden -S 'IntentReview|NoIntentReview|intent_review|no_intent_review|repoConfig|loadFlags\(|applyRepoConfig\(' internal .code-reviewer.example.yaml README.md
# Show the relevant slices of the config file and any docs/config examples if present.
for f in \
internal/config/config.go \
.code-reviewer.example.yaml \
README.md
do
if [ -f "$f" ]; then
echo "===== $f ====="
wc -l "$f"
fi
doneRepository: OpticDiff/code-reviewer
Length of output: 2664
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- .code-reviewer.example.yaml ---'
nl -ba .code-reviewer.example.yaml | sed -n '1,120p'
echo '--- README.md intent-related sections ---'
rg -n -C 3 -i 'intent|no-intent|intent_review' README.md .code-reviewer.example.yaml internal/config/config.goRepository: OpticDiff/code-reviewer
Length of output: 237
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- .code-reviewer.example.yaml ---'
sed -n '1,120p' .code-reviewer.example.yaml
echo '--- README.md intent-related matches ---'
rg -n -C 2 -i 'intent_review|no-intent|intent' README.md .code-reviewer.example.yamlRepository: OpticDiff/code-reviewer
Length of output: 4367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the CLI flags / env vars table and any intent-related entries.
rg -n -C 4 --hidden -S 'no-intent|--intent|--no-intent|intent_review|CLI flags|flags table|Environment variables|REVIEW_' README.md
# Show the surrounding README section where flags are documented.
python3 - <<'PY'
from pathlib import Path
text = Path("README.md").read_text()
for needle in ["CLI flags", "Environment variables", "--summarize", "no-intent", "intent_review"]:
idx = text.lower().find(needle.lower())
if idx != -1:
start = max(0, text.rfind("\n", 0, idx-400))
end = min(len(text), text.find("\n\n", idx+400))
print(f"\n===== around {needle} =====")
print(text[start:end])
PYRepository: OpticDiff/code-reviewer
Length of output: 6619
Add the new intent settings to the config docs.
intent_review is wired into repoConfig, but .code-reviewer.example.yaml and the README flags/config tables still omit --intent / --no-intent, so the new setting is hard to discover.
🤖 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` at line 154, Update the configuration
documentation to cover the IntentReview setting in repoConfig: add intent_review
to .code-reviewer.example.yaml and the README configuration/flags tables,
including the --intent and --no-intent options and their behavior. Keep the
documented names aligned with the IntentReview YAML key and existing CLI
conventions.
Source: Coding guidelines
Comment on lines
+181
to
+217
| // Pass 1: Intent inference (if enabled). | ||
| var intentContext string | ||
| var intentSummary *model.SummaryResult | ||
| var totalUsage model.TokenUsage | ||
| if r.cfg.IntentReview { | ||
| if sp, ok := r.provider.(model.SummarizeProvider); ok { | ||
| fullDiff := buildNumberedDiff(diffs) | ||
| sysPrompt := model.BuildSummaryPrompt() | ||
| uPrompt := model.BuildSummaryUserPrompt(mrTitle, mrDesc, fullDiff) | ||
| summaryResult, serr := sp.Summarize(ctx, sysPrompt, uPrompt) | ||
| if serr != nil { | ||
| slog.Warn("intent inference failed, falling back to standard review", "error", serr) | ||
| } else { | ||
| intentSummary = summaryResult | ||
| intentContext = model.BuildIntentContext(summaryResult) | ||
| slog.Info("intent inferred", | ||
| "classification", summaryResult.Classification, | ||
| "intent", summaryResult.Intent, | ||
| "risk", summaryResult.RiskLevel, | ||
| ) | ||
| if summaryResult.Usage != nil { | ||
| totalUsage.InputTokens += summaryResult.Usage.InputTokens | ||
| totalUsage.OutputTokens += summaryResult.Usage.OutputTokens | ||
| totalUsage.TotalTokens += summaryResult.Usage.TotalTokens | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Step 4: Build prompt and call model for each chunk. | ||
| // In CI mode, source REVIEW.md from the trusted base/target ref so that | ||
| // contributor-controlled branches cannot inject review instructions. | ||
| 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) | ||
| systemPrompt := model.BuildPromptFull(r.cfg.CustomPrompt, reviewMD, r.cfg.Focus, r.cfg.ExtraRules, intentContext) |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Intent-inference pre-pass bypasses the token-limit/chunking safeguards used later in this same function.
fullDiff := buildNumberedDiff(diffs) sends the entire (unchunked) diff set to sp.Summarize(...) in a single call. A few lines later, chunker.Chunk(diffs, tokenLimit) splits the same diffs precisely because a single model call may exceed diff.TokenLimitForModel(r.cfg.Model). Large diffs that legitimately require chunking for the main review will still be sent whole to the summarization call, risking API failures or truncated/degraded summaries for the exact diffs where accurate intent inference matters most.
🤖 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 181 - 217, The intent-inference
pre-pass in the main review flow must apply the same token-limit safeguards as
the later chunked review calls. Update the logic around buildNumberedDiff and
sp.Summarize to chunk diffs using diff.TokenLimitForModel(r.cfg.Model), then
summarize each chunk without sending the entire diff in one request; preserve
the existing fallback, intentContext, and usage accumulation behavior.
1. YAML intent_review: false now sets NoIntentReview to prevent CI auto-default from overriding explicit team opt-out. 2. --intent/--no-intent flags use local variables instead of BoolVar to avoid clobbering YAML/env values when flags aren't passed (same pattern as --summarize, --no-color). 3. Markdown table injection: escape pipe chars in model-derived text (Intent, BreakingChanges) via escapeTableCell(). 4. Silent no-op: log warning when intent review is enabled but provider doesn't implement SummarizeProvider. 5. Config precedence tests (5 new): - CI auto-enable default - YAML false overrides CI default - Env false overrides CI default - Local/diff mode default off - --intent + --summarize mutual exclusivity
There was a problem hiding this comment.
🧹 Nitpick comments (2)
internal/config/config_test.go (2)
785-901: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd the CLI override to a table-driven precedence matrix.
These cases repeat setup but do not cover
--no-intent, theLoad()branch that prevents CI auto-enable. Convert them to table-driven cases and add--ci --no-intentassertingIntentReview == false.As per coding guidelines,
**/*_test.go: “Prefer table-driven tests”.🤖 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_test.go` around lines 785 - 901, Convert the related IntentReview tests into a table-driven precedence matrix covering CI default enablement, YAML false, REVIEW_INTENT=false, local default-off behavior, and the mutually exclusive --intent/--summarize error. Add a --ci --no-intent case asserting IntentReview is false, while preserving each case’s required environment, YAML setup, and expected error or value.Source: Coding guidelines
788-790: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSerialize process-global test state.
os.Argsand the working directory are shared across the package, and these cases can race with parallel tests. Guard the mutations with a package-level mutex and restore both values int.Cleanupcallbacks.🤖 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_test.go` around lines 788 - 790, Update the test setup around the os.Args mutation to serialize process-global state changes with a package-level mutex. Lock before modifying os.Args or the working directory, register t.Cleanup callbacks to restore both original values and unlock the mutex, and remove the existing defer-based os.Args restoration.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.
Nitpick comments:
In `@internal/config/config_test.go`:
- Around line 785-901: Convert the related IntentReview tests into a
table-driven precedence matrix covering CI default enablement, YAML false,
REVIEW_INTENT=false, local default-off behavior, and the mutually exclusive
--intent/--summarize error. Add a --ci --no-intent case asserting IntentReview
is false, while preserving each case’s required environment, YAML setup, and
expected error or value.
- Around line 788-790: Update the test setup around the os.Args mutation to
serialize process-global state changes with a package-level mutex. Lock before
modifying os.Args or the working directory, register t.Cleanup callbacks to
restore both original values and unlock the mutex, and remove the existing
defer-based os.Args restoration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: dce80805-ae58-40c3-9ed6-b1c7d3e34057
📒 Files selected for processing (4)
internal/config/config.gointernal/config/config_test.gointernal/reviewer/intent.gointernal/reviewer/reviewer.go
🚧 Files skipped from review as they are similar to previous changes (3)
- internal/reviewer/intent.go
- internal/reviewer/reviewer.go
- internal/config/config.go
This was referenced Jul 20, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Two-pass intent-aware code review. Pass 1 infers the developer's intent from the diff. Pass 2 reviews the code against that intent, flagging scope creep, missing test coverage, and undocumented breaking changes.
Architecture
Smart Defaults
--ci)--no-intent--diff)--intentintent_review: true/falseREVIEW_INTENT=true/falseClassification-Specific Rules
featfixrefactorOutput
Terminal — compact one-liner:
CI/GitLab — markdown intent table at top of MR comment with classification, risk emoji, scope, and breaking changes.
Graceful Fallback
If pass 1 (intent inference) fails, the review runs normally without intent context. No hard failures.
Files Changed (475 lines)
config.goIntentReview/NoIntentReviewfields,--intent/--no-intentflags,REVIEW_INTENTenv, smart CI defaults, mutual exclusivity with--summarizeprompt.goBuildIntentContext(),BuildPromptFull()intent layer (between focus overlays and extra rules),scopecategoryreviewer.goRun(), output wiring for terminal + CIintent.goformatIntentOneLiner(),formatIntentMarkdown()intent_prompt_test.goBuildIntentContextintent_test.goprompt_test.goBuildPromptFullsignatureTesting
go build ./...passesgo test ./...passes (all 11 new + existing tests)Summary by CodeRabbit
--intent/--no-intent, plusREVIEW_INTENTand YAMLintent_review, with CI auto-enable defaults.lineto matchnew_line.