Add agent-ready analysis outputs - #418
Merged
Merged
Conversation
📝 WalkthroughWalkthroughPR adds a Context data model and builder that aggregates language/symbol/risk metadata, introduces SARIF output format support across all commands, and implements ChangesContext, SARIF output, and changed-since filtering
Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Omen AnalysisDiff Risk
Risk Factors
Recommendations
Repository Health
Score Components
Tips for AI agentsUse these commands to investigate and improve low-scoring areas. Run full analysis: omen -f json score # health score with component breakdown
omen -f json diff # PR risk analysis
omen hotspot # high-churn + high-complexity filesCoupling (score: 52.8235294117647) -- needs attention omen graph && omen smellsBreak cyclic dependencies by introducing interfaces or extracting shared types. Reduce fan-out from hub modules by splitting responsibilities. Duplication (score: 42.16365910309094) -- needs attention omen clonesLook for Type-1 (exact) and Type-2 (renamed) clones. Extract shared logic into reusable functions or modules. Prioritize clones in high-churn files. General workflow for improving scores:
|
panbanda
force-pushed
the
feat/agent-analysis-enhancements
branch
from
May 29, 2026 21:53
f3070a3 to
c214cb1
Compare
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Owner
Author
|
@coderabbitai review |
1 similar comment
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
src/main.rs (1)
721-735:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winText output may double-print context.
Lines 721-732 print text headers, then line 733 calls
format.format(&context, ...). If Text formatter also outputs the context fields, output will be duplicated.Proposed fix
Format::Text => { println!("Repository Context"); println!("=================="); println!("Max Tokens: {}", args.max_tokens); println!("Depth: {}", args.depth); if let Some(ref target) = args.target { println!("Target: {}", target.display()); } if let Some(ref symbol) = args.symbol { println!("Symbol: {}", symbol); } println!(); - format.format(&context, &mut stdout())?; }🤖 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 `@src/main.rs` around lines 721 - 735, The Text branch currently prints repository headers (the println! calls using args.max_tokens, args.depth, args.target, args.symbol) and then calls format.format(&context, &mut stdout()), causing duplicate context output when the Text formatter also prints those fields; fix by removing the manual println! header block and rely on format.format(&context, &mut stdout()) to render the full text output (or alternatively move those header prints into the Text formatter implementation), updating the Format::Text arm to only call format.format to avoid double-printing.
🤖 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 `@src/context.rs`:
- Around line 104-147: summarize_risks currently appends SATD entries first and
then complexity entries, and only calls risks.truncate(limit) at the end so
complexity risks get dropped when SATD fills the quota; also complexity sorting
uses only metrics.cognitive and can miss severe cyclomatic outliers. Change
summarize_risks to build a single combined collection (e.g., Vec<(score,
RiskSummary)> or Vec<RankedRisk>), push SATD-derived RiskSummary items (from
satd::Analyzer::default().analyze) and complexity-derived RiskSummary items
(from complexity::Analyzer::default().analyze) into it, compute a numeric
ranking score for each entry that considers severity plus both metrics.cognitive
and metrics.cyclomatic (or use a tuple key like (severity_rank, cognitive,
cyclomatic)), sort the combined collection by that score in descending order,
then take the top limit entries and return their RiskSummary values (instead of
truncating after merging).
In `@src/output/mod.rs`:
- Around line 106-150: The SARIF predicate is too permissive: update
sarif_finding_from_object (used by collect_sarif_findings) to only treat an
object as a finding when it contains an explicit finding indicator (e.g., a
non-default message field like "message"/"text"/"reason"/"name"/"marker", a
"ruleId"/"result" field, or an explicit numeric line like
"line"/"start_line"/"line_start") in addition to a file/path; do not rely solely
on defaulting message or line to create a SarifFinding; require the presence of
at least one explicit message-like key or an explicit line number before
returning Some(SarifFinding) and otherwise return None.
---
Outside diff comments:
In `@src/main.rs`:
- Around line 721-735: The Text branch currently prints repository headers (the
println! calls using args.max_tokens, args.depth, args.target, args.symbol) and
then calls format.format(&context, &mut stdout()), causing duplicate context
output when the Text formatter also prints those fields; fix by removing the
manual println! header block and rely on format.format(&context, &mut stdout())
to render the full text output (or alternatively move those header prints into
the Text formatter implementation), updating the Format::Text arm to only call
format.format to avoid double-printing.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cfae81e9-7223-4d5d-9548-7673a414e89e
📒 Files selected for processing (9)
benches/analyzers.rssrc/cli/mod.rssrc/context.rssrc/core/file_set.rssrc/lib.rssrc/main.rssrc/mcp/mod.rssrc/output/mod.rstests/integration_tests.rs
Comment on lines
+104
to
+147
| fn summarize_risks(ctx: &AnalysisContext<'_>, limit: usize) -> Vec<RiskSummary> { | ||
| let mut risks = Vec::new(); | ||
|
|
||
| if let Ok(satd) = satd::Analyzer::default().analyze(ctx) { | ||
| risks.extend(satd.items.into_iter().map(|item| RiskSummary { | ||
| kind: "satd".to_string(), | ||
| file: item.file, | ||
| line: item.line, | ||
| message: item.text, | ||
| severity: format!("{:?}", item.severity).to_lowercase(), | ||
| })); | ||
| } | ||
|
|
||
| if let Ok(complexity) = complexity::Analyzer::default().analyze(ctx) { | ||
| let mut functions: Vec<_> = complexity | ||
| .files | ||
| .into_iter() | ||
| .flat_map(|file| file.functions) | ||
| .collect(); | ||
| functions.sort_by_key(|func| std::cmp::Reverse(func.metrics.cognitive)); | ||
| risks.extend(functions.into_iter().take(limit).filter_map(|func| { | ||
| if func.metrics.cognitive < 15 && func.metrics.cyclomatic < 10 { | ||
| return None; | ||
| } | ||
| Some(RiskSummary { | ||
| kind: "complexity".to_string(), | ||
| file: func.file, | ||
| line: func.start_line, | ||
| message: format!( | ||
| "{} has cyclomatic {} and cognitive {} complexity", | ||
| func.name, func.metrics.cyclomatic, func.metrics.cognitive | ||
| ), | ||
| severity: if func.metrics.cognitive >= 30 || func.metrics.cyclomatic >= 20 { | ||
| "high" | ||
| } else { | ||
| "medium" | ||
| } | ||
| .to_string(), | ||
| }) | ||
| })); | ||
| } | ||
|
|
||
| risks.truncate(limit); | ||
| risks |
There was a problem hiding this comment.
Rank risks before truncating.
SATD items are appended first and truncate(limit) runs only once at the end, so complexity risks disappear whenever SATD already fills the quota. This also sorts complexity only by cognitive, which can drop the worst cyclomatic outliers. Build one combined ranking, then take limit.
🤖 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 `@src/context.rs` around lines 104 - 147, summarize_risks currently appends
SATD entries first and then complexity entries, and only calls
risks.truncate(limit) at the end so complexity risks get dropped when SATD fills
the quota; also complexity sorting uses only metrics.cognitive and can miss
severe cyclomatic outliers. Change summarize_risks to build a single combined
collection (e.g., Vec<(score, RiskSummary)> or Vec<RankedRisk>), push
SATD-derived RiskSummary items (from satd::Analyzer::default().analyze) and
complexity-derived RiskSummary items (from
complexity::Analyzer::default().analyze) into it, compute a numeric ranking
score for each entry that considers severity plus both metrics.cognitive and
metrics.cyclomatic (or use a tuple key like (severity_rank, cognitive,
cyclomatic)), sort the combined collection by that score in descending order,
then take the top limit entries and return their RiskSummary values (instead of
truncating after merging).
Comment on lines
+106
to
+150
| fn collect_sarif_findings(value: &Value, findings: &mut Vec<SarifFinding>) { | ||
| match value { | ||
| Value::Object(map) => { | ||
| if let Some(finding) = sarif_finding_from_object(map) { | ||
| findings.push(finding); | ||
| } | ||
| for child in map.values() { | ||
| collect_sarif_findings(child, findings); | ||
| } | ||
| } | ||
| Value::Array(items) => { | ||
| for item in items { | ||
| collect_sarif_findings(item, findings); | ||
| } | ||
| } | ||
| _ => {} | ||
| } | ||
| } | ||
|
|
||
| fn sarif_finding_from_object(map: &serde_json::Map<String, Value>) -> Option<SarifFinding> { | ||
| let file = ["file", "path", "file_path"] | ||
| .iter() | ||
| .find_map(|key| map.get(*key).and_then(Value::as_str))?; | ||
| let line = ["line", "start_line", "line_start"] | ||
| .iter() | ||
| .find_map(|key| map.get(*key).and_then(Value::as_u64)) | ||
| .unwrap_or(1) | ||
| .max(1); | ||
| let message = ["text", "reason", "message", "name", "marker"] | ||
| .iter() | ||
| .find_map(|key| map.get(*key).and_then(Value::as_str)) | ||
| .unwrap_or("Omen finding") | ||
| .to_string(); | ||
| let level = map | ||
| .get("severity") | ||
| .and_then(Value::as_str) | ||
| .map(sarif_level) | ||
| .unwrap_or("warning"); | ||
|
|
||
| Some(SarifFinding { | ||
| file: file.to_string(), | ||
| line, | ||
| level, | ||
| message, | ||
| }) |
There was a problem hiding this comment.
Tighten the SARIF finding predicate.
Any object with file/path becomes a SARIF result here, even summary/container nodes. Because message and line are defaulted, those false positives turn into bogus findings at line 1. Require an explicit finding field as well, not just a path.
🤖 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 `@src/output/mod.rs` around lines 106 - 150, The SARIF predicate is too
permissive: update sarif_finding_from_object (used by collect_sarif_findings) to
only treat an object as a finding when it contains an explicit finding indicator
(e.g., a non-default message field like
"message"/"text"/"reason"/"name"/"marker", a "ruleId"/"result" field, or an
explicit numeric line like "line"/"start_line"/"line_start") in addition to a
file/path; do not rely solely on defaulting message or line to create a
SarifFinding; require the presence of at least one explicit message-like key or
an explicit line number before returning Some(SarifFinding) and otherwise return
None.
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.
Summary
Verification
Notes
Summary by CodeRabbit
New Features
--changed-sinceargument to analyze only files modified since a specified git reference.Improvements