feat: auto-summary mode (--summarize) - #24
Conversation
Phase 1 of intent-driven development checking. New features: - --summarize flag generates structured MR descriptions from diffs - SummaryResult type: classification, intent, risk level, scope areas, breaking changes, confidence score - SummarizeProvider interface on Vertex AI and HTTP providers - Rich terminal output with ANSI colors - GitLab markdown comment posting in CI mode - JSON output for scripting Refactors: - Extract generateRaw() from Review() in both providers - Extract cleanJSONText() from parseReviewJSON() for shared JSON parsing - Remove ResponseSchema constraint from Vertex provider (rely on prompt) Docs: - README: add auto-summary feature, --summarize flag, usage examples - ROADMAP: mark v0.5 done, add v0.6 (intent-aware review) and v1.0 (compliance & audit) milestones
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a ChangesAuto-summary feature
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant Config
participant Reviewer
participant DiffSource
participant SummarizeProvider
participant GitLab
CLI->>Config: load --summarize settings
Config->>Reviewer: provide summary configuration
Reviewer->>DiffSource: fetch and filter diffs
Reviewer->>SummarizeProvider: generate structured summary
SummarizeProvider-->>Reviewer: return summary and token usage
Reviewer->>GitLab: post Markdown summary in CI mode
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 |
There was a problem hiding this comment.
Actionable comments posted: 4
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)
528-533: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCI summaries still require a GitLab token. CI summary mode always posts an MR note, but validation permits an empty token, producing an unauthenticated GitLab request.
internal/config/config.go#L528-L533: requireGITLAB_TOKENwhenever CI summary output will post to GitLab.internal/reviewer/summary.go#L80-L86: keep tokenless operation limited to local or dry-run output paths.🤖 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 528 - 533, The CI summary path must require GITLAB_TOKEN before posting GitLab notes. Update config validation around the GitLabToken/Summarize check in internal/config/config.go:528-533 to reject an empty token whenever CI summary output targets GitLab, while preserving tokenless local or dry-run behavior. In internal/reviewer/summary.go:80-86, keep tokenless operation restricted to local or dry-run output paths and do not attempt authenticated GitLab posting without a token.
🤖 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 `@cmd/code-reviewer/main.go`:
- Around line 118-121: Update the validation around cfg.Summarize and the
configured models to reject the --summarize --models combination before
dispatching to rev.RunSummary, or extend internal/model.MultiProvider with the
required model.SummarizeProvider implementation. Ensure invalid combinations
fail during validation rather than at runtime, while preserving normal summary
and review execution paths.
In `@internal/model/http_provider.go`:
- Around line 169-170: Update the HTTP client used by the retry.Do model call in
the surrounding request flow to configure CheckRedirect, rejecting redirects or
permitting only redirects to the expected endpoint origin. Ensure model payloads
are never followed to an unexpected host while preserving the existing request
and retry behavior.
In `@internal/model/provider.go`:
- Around line 199-205: Validate decoded model contracts after JSON
unmarshalling: in internal/model/provider.go lines 199-205, update the
ReviewResult parsing around cleanJSONText and json.Unmarshal to require all
mandatory review fields and distinguish an absent findings field from a present
but empty findings array; in internal/model/summarize.go lines 51-58, validate
mandatory summary fields, permitted classification and risk values, and
confidence bounds before returning the result. Return descriptive validation
errors instead of accepting structurally valid but incomplete responses.
In `@internal/reviewer/summary.go`:
- Around line 43-46: Update the summary flow around buildNumberedDiff,
BuildSummaryPrompt, and BuildSummaryUserPrompt to enforce the configured
MaxTokens and the model’s context limit before sending a request. Bound the diff
input or chunk and synthesize partial summaries so large merge requests produce
a summary instead of exceeding provider limits.
---
Outside diff comments:
In `@internal/config/config.go`:
- Around line 528-533: The CI summary path must require GITLAB_TOKEN before
posting GitLab notes. Update config validation around the GitLabToken/Summarize
check in internal/config/config.go:528-533 to reject an empty token whenever CI
summary output targets GitLab, while preserving tokenless local or dry-run
behavior. In internal/reviewer/summary.go:80-86, keep tokenless operation
restricted to local or dry-run output paths and do not attempt authenticated
GitLab posting without a token.
🪄 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: 8911a48f-c58c-46b9-affb-ddbc91736927
📒 Files selected for processing (10)
README.mdROADMAP.mdcmd/code-reviewer/main.gointernal/config/config.gointernal/model/http_provider.gointernal/model/provider.gointernal/model/summarize.gointernal/model/summary.gointernal/model/summary_test.gointernal/reviewer/summary.go
| if cfg.Summarize { | ||
| exitCode, err = rev.RunSummary(ctx) | ||
| } else { | ||
| exitCode, err = rev.Run(ctx) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/sh
fd -t f -e go . internal/model -x rg -n -C2 \
'type MultiProvider|func \(.*MultiProvider.*\) Summarize|SummarizeProvider' {}Repository: OpticDiff/code-reviewer
Length of output: 675
🏁 Script executed:
#!/bin/sh
set -eu
printf '\n== cmd/code-reviewer/main.go (relevant range) ==\n'
sed -n '90,150p' cmd/code-reviewer/main.go
printf '\n== internal/model outline ==\n'
ast-grep outline internal/model --view expanded || true
printf '\n== search for Summary support in internal/model ==\n'
rg -n -C 2 'SummarizeProvider|func \(.*\) Summarize\(|type MultiProvider|RunSummary|SummaryResult' internal/model cmd/code-reviewerRepository: OpticDiff/code-reviewer
Length of output: 14960
🏁 Script executed:
#!/bin/sh
set -eu
printf '\n== reviewer package summary entry points ==\n'
rg -n -C 3 'func .*RunSummary|SummarizeProvider|SummaryResult|Summarize\(' reviewer cmd internal | sed -n '1,220p'
printf '\n== outline reviewer files ==\n'
ast-grep outline reviewer --view expanded || trueRepository: OpticDiff/code-reviewer
Length of output: 5911
🏁 Script executed:
#!/bin/sh
set -eu
printf '\n== search for summarize/model validation ==\n'
rg -n -C 3 'Summarize|summarize|models|NewMultiProvider|MultiProvider|provider does not support summarize mode|Mode\(' cmd internal | sed -n '1,260p'
printf '\n== internal/model/multi.go relevant methods ==\n'
sed -n '1,140p' internal/model/multi.goRepository: OpticDiff/code-reviewer
Length of output: 19440
Reject --summarize --models
RunSummary requires model.SummarizeProvider, but internal/model.MultiProvider only implements Review/Close, so this flag combination fails at runtime. Block it in validation or add summarize support to MultiProvider.
🤖 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 `@cmd/code-reviewer/main.go` around lines 118 - 121, Update the validation
around cfg.Summarize and the configured models to reject the --summarize
--models combination before dispatching to rev.RunSummary, or extend
internal/model.MultiProvider with the required model.SummarizeProvider
implementation. Ensure invalid combinations fail during validation rather than
at runtime, while preserving normal summary and review execution paths.
| cleaned := cleanJSONText(text) | ||
|
|
||
| // Try direct JSON parse first (for well-behaved models). | ||
| var result ReviewResult | ||
| if err := json.Unmarshal([]byte(text), &result); err == nil { | ||
| return &result, nil | ||
| if err := json.Unmarshal([]byte(cleaned), &result); err != nil { | ||
| return nil, fmt.Errorf("could not parse model response as JSON: %w", err) | ||
| } | ||
| return &result, nil |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Validate the decoded result contracts, not only JSON syntax. After removing Vertex’s review response schema, {} parses successfully into an empty ReviewResult; summary parsing has the same problem. This can silently report no findings or publish an empty summary.
internal/model/provider.go#L199-L205: validate required review fields after unmarshalling; distinguish a missingfindingsfield from an empty findings array.internal/model/summarize.go#L51-L58: validate required summary fields, allowed classification/risk values, and confidence bounds.
📍 Affects 2 files
internal/model/provider.go#L199-L205(this comment)internal/model/summarize.go#L51-L58
🤖 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/provider.go` around lines 199 - 205, Validate decoded model
contracts after JSON unmarshalling: in internal/model/provider.go lines 199-205,
update the ReviewResult parsing around cleanJSONText and json.Unmarshal to
require all mandatory review fields and distinguish an absent findings field
from a present but empty findings array; in internal/model/summarize.go lines
51-58, validate mandatory summary fields, permitted classification and risk
values, and confidence bounds before returning the result. Return descriptive
validation errors instead of accepting structurally valid but incomplete
responses.
| // Step 3: Build numbered diff and prompts. | ||
| numberedDiff := buildNumberedDiff(diffs) | ||
| systemPrompt := model.BuildSummaryPrompt() | ||
| userPrompt := model.BuildSummaryUserPrompt(mrTitle, mrDesc, numberedDiff) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Bound summary input before building one prompt.
Summary mode sends the entire filtered diff in one request and ignores MaxTokens and model context limits. Large MRs will fail at the provider instead of producing a summary. Apply a budget/context check here, or chunk and synthesize partial summaries.
🤖 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/summary.go` around lines 43 - 46, Update the summary flow
around buildNumberedDiff, BuildSummaryPrompt, and BuildSummaryUserPrompt to
enforce the configured MaxTokens and the model’s context limit before sending a
request. Bound the diff input or chunk and synthesize partial summaries so large
merge requests produce a summary instead of exceeding provider limits.
- Remove unused reviewResultSchema (golangci-lint unused) - Fix CI+summarize validation: require GITLAB_TOKEN unless --dry-run - Block --summarize --models (MultiProvider lacks SummarizeProvider) - Add HTTP redirect protection (CheckRedirect) to prevent SSRF - Add validateSummary for required fields (title, classification, risk)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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`:
- Line 528: Rewrite the condition in the validation logic around c.GitLabToken,
c.Summarize, and c.DryRun using the equivalent De Morgan form that satisfies
Staticcheck QF1001. Preserve the existing behavior: the block should remain
skipped only when summarization and dry-run are both enabled while GitLabToken
is empty.
🪄 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: cbe482d7-38c2-4501-8954-22a8b1af4c41
📒 Files selected for processing (4)
internal/config/config.gointernal/model/http_provider.gointernal/model/provider.gointernal/model/summarize.go
💤 Files with no reviewable changes (1)
- internal/model/provider.go
🚧 Files skipped from review as they are similar to previous changes (2)
- internal/model/summarize.go
- internal/model/http_provider.go
Phase 1: Intent-Driven Development Checking
--summarizegenerates structured MR descriptions from diffs instead of code reviews.What it does
Usage
Implementation
SummaryResulttype + dedicated summary promptSummarizeProviderinterface on both Vertex AI and HTTP providersReview()→ sharedgenerateRaw()+cleanJSONText()(no behavior change)--summarizeflag +summarize:yaml configTests
go build ./...✅go test ./...✅ (all 8 packages)Next
Summary by CodeRabbit
--summarize(optionally updating the merge request description).