feat: --explain mode - #28
Merged
Merged
Conversation
Add --explain flag that asks the model to explain what a diff does in plain Markdown instead of reviewing it. Useful for onboarding, code handoffs, and understanding unfamiliar changes. Architecture: - model/explain.go: Free-form explain prompt (no JSON parsing) - model/explain_provider.go: ExplainProvider interface wrapping generateRaw - reviewer/explain.go: RunExplain pipeline (diff → prompt → render) - config.go: --explain flag, mutually exclusive with --summarize/--intent - main.go: dispatch to RunExplain Output: - Terminal: 🔍 header + markdown explanation - CI/GitLab: posted as MR comment with ## header 7 new tests across explain_test.go and explain_provider tests.
📝 WalkthroughWalkthroughAdds an ChangesExplain mode
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant Reviewer
participant DiffSource
participant Model
participant GitLab
CLI->>Reviewer: RunExplain(ctx)
Reviewer->>DiffSource: fetch and filter diffs
Reviewer->>Model: Explain(systemPrompt, userPrompt)
Model-->>Reviewer: explanation and token usage
Reviewer->>GitLab: post Markdown note when running in CI
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 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/model/explain.go`:
- Around line 32-36: Update the prompt-building logic in
internal/model/explain.go to treat mrTitle, mrDescription, and all diff content
as untrusted data. Extend the guardrail beyond code constructs so none of these
inputs can provide instructions or steer the generated GitLab comment;
explicitly state that they are informational content only. Apply the same
protection to the related logic covering the additional referenced section.
In `@internal/reviewer/explain.go`:
- Line 69: Sanitize the untrusted explanation in the rendering path before the
fmt.Print call, using the existing formatExplainTerminal flow as the integration
point. Remove terminal control sequences while preserving ordinary newlines and
tabs, then print the sanitized result.
🪄 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: f9955dac-2c5b-4e57-b726-ac76f486837f
📒 Files selected for processing (7)
cmd/code-reviewer/main.gointernal/config/config.gointernal/model/explain.gointernal/model/explain_provider.gointernal/model/explain_test.gointernal/reviewer/explain.gointernal/reviewer/explain_test.go
1. Prompt injection: Expanded adversarial content warning to explicitly cover MR title, MR description, and diff as untrusted data — not just code constructs. Prevents malicious MR metadata from steering output. 2. Terminal injection: Added stripControlChars() that removes OSC/CSI escape sequences from model output before terminal rendering, while preserving newlines and tabs. Prevents injected sequences from manipulating terminal (clear screen, change title, etc). 5 new tests: - Adversarial warning coverage (title, desc, diff, 'untrusted data') - stripControlChars preserves normal text - stripControlChars removes ESC/CSI sequences - stripControlChars removes OSC sequences - formatExplainTerminal sanitizes embedded control chars
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/model/explain_test.go`:
- Around line 20-31: Strengthen the adversarial warning assertions in the test
around result validation so they verify that “MR title,” “MR description,” and
“diff” are collectively identified as “untrusted data.” Replace the independent
strings.Contains checks with an exact complete-clause assertion or a regex that
binds each input to the untrusted-data label.
In `@internal/reviewer/explain_test.go`:
- Around line 144-163: Strengthen TestStripControlChars_RemovesOSC and
TestFormatExplainTerminal_SanitizesControlChars by asserting that text following
the OSC and CSI sequences remains in the sanitized output. Verify “normal” and
“rest” are preserved for stripControlChars, and “but this clears screen” is
preserved for formatExplainTerminal, while retaining the existing
control-character checks.
🪄 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: 9b2234ec-fd0b-4ea8-9ecb-b6fdb02bce1b
📒 Files selected for processing (4)
internal/model/explain.gointernal/model/explain_test.gointernal/reviewer/explain.gointernal/reviewer/explain_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- internal/model/explain.go
- internal/reviewer/explain.go
Comment on lines
+20
to
+31
| // Must explicitly call out ALL untrusted inputs. | ||
| if !strings.Contains(result, "MR title") { | ||
| t.Error("adversarial warning must mention MR title as untrusted") | ||
| } | ||
| if !strings.Contains(result, "MR description") { | ||
| t.Error("adversarial warning must mention MR description as untrusted") | ||
| } | ||
| if !strings.Contains(result, "diff") { | ||
| t.Error("adversarial warning must mention diff as untrusted") | ||
| } | ||
| if !strings.Contains(result, "untrusted data") { | ||
| t.Error("adversarial warning must label inputs as untrusted data") |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Assert the adversarial warning semantically.
Independent strings.Contains checks can pass even if “MR title”, “MR description”, and “diff” are no longer labeled as untrusted together. Assert the complete warning clause, or use a regex that binds each input to “untrusted data”.
As per coding guidelines, adversarial-content sections in prompts are security-critical and require meaningful validation.
🤖 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/explain_test.go` around lines 20 - 31, Strengthen the
adversarial warning assertions in the test around result validation so they
verify that “MR title,” “MR description,” and “diff” are collectively identified
as “untrusted data.” Replace the independent strings.Contains checks with an
exact complete-clause assertion or a regex that binds each input to the
untrusted-data label.
Source: Coding guidelines
Comment on lines
+144
to
+163
| func TestStripControlChars_RemovesOSC(t *testing.T) { | ||
| // OSC (Operating System Command) can change terminal title. | ||
| input := "normal\x1b]0;malicious title\x07rest" | ||
| result := stripControlChars(input) | ||
| if strings.Contains(result, "\x1b") || strings.Contains(result, "\x07") { | ||
| t.Errorf("expected control chars removed, got %q", result) | ||
| } | ||
| } | ||
|
|
||
| func TestFormatExplainTerminal_SanitizesControlChars(t *testing.T) { | ||
| // Model output with embedded CSI sequence. | ||
| explanation := "This is safe\x1b[2J\x1b[H but this clears screen" | ||
| result := formatExplainTerminal(explanation, false) | ||
| if strings.Contains(result, "\x1b") { | ||
| t.Error("expected control sequences stripped from terminal output") | ||
| } | ||
| if !strings.Contains(result, "This is safe") { | ||
| t.Error("expected safe content preserved") | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Verify text after control sequences is preserved.
The OSC test only checks that control bytes disappear, and the terminal test only checks the prefix. A sanitizer that truncates output at the OSC/CSI sequence would pass both tests. Also assert that "normal"/"rest" and "but this clears screen" remain.
🤖 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/explain_test.go` around lines 144 - 163, Strengthen
TestStripControlChars_RemovesOSC and
TestFormatExplainTerminal_SanitizesControlChars by asserting that text following
the OSC and CSI sequences remains in the sanitized output. Verify “normal” and
“rest” are preserved for stripControlChars, and “but this clears screen” is
preserved for formatExplainTerminal, while retaining the existing
control-character checks.
This was referenced Jul 20, 2026
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
New
--explainflag that asks the model to explain what a diff does in plain Markdown — instead of reviewing or summarizing it.Use cases: onboarding, code handoffs, understanding unfamiliar changes, PR walkthroughs.
Usage
Architecture
model/explain.gomodel/explain_provider.goExplainProviderinterface wrappinggenerateRaw()reviewer/explain.goRunExplain()pipeline: diff → prompt → renderconfig.go--explainflag, mutually exclusive with--summarize/--intentmain.goRunExplainOutput
Terminal:
CI/GitLab: Posted as MR comment with
## 🔍 Diff Explanationheader.Validation
--summarizeand--intentExplainProvidergo build ./...passesgo test ./...passesSummary by CodeRabbit
--explainmode that generates a plain-language explanation of code changes.--explainwith summarization and with intent review options.