Skip to content

Commit 0c63abe

Browse files
haasonsaasclaude
andcommitted
Enrich wide event pipeline with full AI reviewer telemetry
Wire token usage from LLM adapters through ReviewResult to ReviewEvent and the frontend. Add per-file metrics (latency, tokens, comment count), hotspot risk scoring, convention suppression counts, and per-pass comment breakdowns. Extend ReviewEventBuilder and EventPanel UI to surface all new telemetry data. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent c339384 commit 0c63abe

9 files changed

Lines changed: 526 additions & 26 deletions

File tree

src/commands/eval.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -526,7 +526,8 @@ async fn run_eval_fixture(
526526
})
527527
.unwrap_or_else(|| PathBuf::from("."));
528528

529-
let comments = review_diff_content_raw(&diff_content, config.clone(), &repo_path).await?;
529+
let review_result = review_diff_content_raw(&diff_content, config.clone(), &repo_path).await?;
530+
let comments = review_result.comments;
530531
let total_comments = comments.len();
531532
let mut failures = Vec::new();
532533
let mut required_matches = 0usize;

src/commands/pr.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,9 @@ pub async fn pr_command(
9999
return Ok(());
100100
}
101101

102-
let comments =
102+
let review_result =
103103
review::review_diff_content_raw(&diff_content, config.clone(), &repo_root).await?;
104+
let comments = review_result.comments;
104105

105106
if post_comments {
106107
info!("Posting {} comments to PR", comments.len());

src/review/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ pub use pipeline::{
1515
review_diff_content, review_diff_content_raw, review_diff_content_raw_with_progress,
1616
review_diff_content_with_repo, ProgressCallback, ProgressUpdate,
1717
};
18+
#[allow(unused_imports)]
19+
pub(crate) use pipeline::{FileMetric, ReviewResult};
1820
pub use rule_helpers::{
1921
apply_rule_overrides, build_pr_summary_comment_body, inject_rule_context, load_review_rules,
2022
normalize_rule_id, summarize_rule_hits,

src/review/pipeline.rs

Lines changed: 141 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use anyhow::Result;
22
use futures::StreamExt;
33
use once_cell::sync::Lazy;
44
use regex::Regex;
5+
use serde::{Deserialize, Serialize};
56
use std::collections::HashMap;
67
use std::path::Path;
78
use std::path::PathBuf;
@@ -19,6 +20,34 @@ use crate::output::OutputFormat;
1920
use crate::parsing::parse_llm_response;
2021
use crate::plugins;
2122

23+
/// Rich result from the review pipeline, carrying comments plus telemetry metadata.
24+
#[derive(Debug, Clone, Default)]
25+
pub struct ReviewResult {
26+
pub comments: Vec<core::Comment>,
27+
/// Aggregate LLM token usage across all files and passes.
28+
pub total_prompt_tokens: usize,
29+
pub total_completion_tokens: usize,
30+
pub total_tokens: usize,
31+
/// Per-file metrics (latency, tokens, comment count).
32+
pub file_metrics: Vec<FileMetric>,
33+
/// Number of comments suppressed by learned convention patterns.
34+
pub convention_suppressed_count: usize,
35+
/// Comment counts grouped by specialized pass tag (e.g., "security-pass": 3).
36+
pub comments_by_pass: HashMap<String, usize>,
37+
/// Hotspot detection results from multi-pass analysis.
38+
pub hotspots: Vec<core::multi_pass::HotspotResult>,
39+
}
40+
41+
#[derive(Debug, Clone, Serialize, Deserialize)]
42+
pub struct FileMetric {
43+
pub file_path: PathBuf,
44+
pub latency_ms: u64,
45+
pub prompt_tokens: usize,
46+
pub completion_tokens: usize,
47+
pub total_tokens: usize,
48+
pub comment_count: usize,
49+
}
50+
2251
/// Progress update emitted per file during review.
2352
pub struct ProgressUpdate {
2453
pub current_file: String,
@@ -54,15 +83,15 @@ pub async fn review_diff_content_with_repo(
5483
repo_path: &Path,
5584
) -> Result<()> {
5685
let rule_priority = config.rule_priority.clone();
57-
let comments = review_diff_content_raw(diff_content, config, repo_path).await?;
58-
crate::output::output_comments(&comments, None, format, &rule_priority).await
86+
let result = review_diff_content_raw(diff_content, config, repo_path).await?;
87+
crate::output::output_comments(&result.comments, None, format, &rule_priority).await
5988
}
6089

6190
pub async fn review_diff_content_raw(
6291
diff_content: &str,
6392
config: config::Config,
6493
repo_path: &Path,
65-
) -> Result<Vec<core::Comment>> {
94+
) -> Result<ReviewResult> {
6695
review_diff_content_raw_with_progress(diff_content, config, repo_path, None).await
6796
}
6897

@@ -73,7 +102,7 @@ pub async fn review_diff_content_raw_with_progress(
73102
config: config::Config,
74103
repo_path: &Path,
75104
on_progress: Option<ProgressCallback>,
76-
) -> Result<Vec<core::Comment>> {
105+
) -> Result<ReviewResult> {
77106
// For local models, chunk oversized diffs instead of truncating
78107
if should_optimize_for_local(&config) {
79108
let context_budget = config.context_window.unwrap_or(8192);
@@ -85,7 +114,7 @@ pub async fn review_diff_content_raw_with_progress(
85114
"Diff split into {} chunks for local model context window",
86115
chunks.len()
87116
);
88-
let mut all_comments = Vec::new();
117+
let mut merged = ReviewResult::default();
89118
for (i, chunk) in chunks.iter().enumerate() {
90119
eprintln!("Processing chunk {}/{}...", i + 1, chunks.len());
91120
match review_diff_content_raw_inner(
@@ -96,13 +125,25 @@ pub async fn review_diff_content_raw_with_progress(
96125
)
97126
.await
98127
{
99-
Ok(comments) => all_comments.extend(comments),
128+
Ok(chunk_result) => {
129+
merged.comments.extend(chunk_result.comments);
130+
merged.total_prompt_tokens += chunk_result.total_prompt_tokens;
131+
merged.total_completion_tokens += chunk_result.total_completion_tokens;
132+
merged.total_tokens += chunk_result.total_tokens;
133+
merged.file_metrics.extend(chunk_result.file_metrics);
134+
merged.convention_suppressed_count +=
135+
chunk_result.convention_suppressed_count;
136+
for (pass, count) in chunk_result.comments_by_pass {
137+
*merged.comments_by_pass.entry(pass).or_insert(0) += count;
138+
}
139+
merged.hotspots.extend(chunk_result.hotspots);
140+
}
100141
Err(e) => {
101142
eprintln!("Warning: chunk {} failed: {}", i + 1, e);
102143
}
103144
}
104145
}
105-
return Ok(all_comments);
146+
return Ok(merged);
106147
}
107148
}
108149

@@ -114,7 +155,7 @@ async fn review_diff_content_raw_inner(
114155
config: config::Config,
115156
repo_path: &Path,
116157
on_progress: Option<ProgressCallback>,
117-
) -> Result<Vec<core::Comment>> {
158+
) -> Result<ReviewResult> {
118159
let diffs = core::DiffParser::parse_unified_diff(diff_content)?;
119160
info!("Parsed {} file diffs", diffs.len());
120161

@@ -468,12 +509,12 @@ async fn review_diff_content_raw_inner(
468509
}
469510
let request_start = Instant::now();
470511
let response = adapter.complete(job.request).await;
512+
let latency_ms = request_start.elapsed().as_millis() as u64;
471513
if is_local {
472-
let elapsed = request_start.elapsed();
473514
eprintln!(
474515
"{}: response received ({:.1}s)",
475516
job.file_path.display(),
476-
elapsed.as_secs_f64()
517+
latency_ms as f64 / 1000.0
477518
);
478519
}
479520
(
@@ -483,6 +524,7 @@ async fn review_diff_content_raw_inner(
483524
job.file_path,
484525
job.pass_kind,
485526
response,
527+
latency_ms,
486528
)
487529
}
488530
})
@@ -492,9 +534,17 @@ async fn review_diff_content_raw_inner(
492534

493535
// Phase 3: Process results in file order
494536
let mut indexed_results = results;
495-
indexed_results.sort_by_key(|(idx, _, _, _, _, _)| *idx);
537+
indexed_results.sort_by_key(|(idx, _, _, _, _, _, _)| *idx);
496538

497-
for (diff_index, active_rules, path_config, file_path, pass_kind, response) in indexed_results {
539+
let mut total_prompt_tokens: usize = 0;
540+
let mut total_completion_tokens: usize = 0;
541+
let mut total_tokens: usize = 0;
542+
let mut file_metrics: Vec<FileMetric> = Vec::new();
543+
let mut comments_by_pass: HashMap<String, usize> = HashMap::new();
544+
545+
for (diff_index, active_rules, path_config, file_path, pass_kind, response, latency_ms) in
546+
indexed_results
547+
{
498548
let diff = &diffs[diff_index];
499549

500550
match response {
@@ -503,6 +553,21 @@ async fn review_diff_content_raw_inner(
503553
continue;
504554
}
505555
Ok(response) => {
556+
// Extract usage metrics from the response
557+
let (resp_prompt_tokens, resp_completion_tokens, resp_total_tokens) =
558+
if let Some(ref usage) = response.usage {
559+
(
560+
usage.prompt_tokens,
561+
usage.completion_tokens,
562+
usage.total_tokens,
563+
)
564+
} else {
565+
(0, 0, 0)
566+
};
567+
total_prompt_tokens += resp_prompt_tokens;
568+
total_completion_tokens += resp_completion_tokens;
569+
total_tokens += resp_total_tokens;
570+
506571
// Validate LLM response before parsing
507572
if let Err(validation_err) = validate_llm_response(&response.content) {
508573
eprintln!(
@@ -515,6 +580,15 @@ async fn review_diff_content_raw_inner(
515580
"Hint: Try a larger model or reduce diff size for better results with local models."
516581
);
517582
}
583+
// Still record file metric even for validation failures
584+
file_metrics.push(FileMetric {
585+
file_path: file_path.clone(),
586+
latency_ms,
587+
prompt_tokens: resp_prompt_tokens,
588+
completion_tokens: resp_completion_tokens,
589+
total_tokens: resp_total_tokens,
590+
comment_count: 0,
591+
});
518592
continue;
519593
}
520594

@@ -549,7 +623,47 @@ async fn review_diff_content_raw_inner(
549623
let comments = apply_rule_overrides(comments, &active_rules);
550624

551625
let comments = filter_comments_for_diff(diff, comments);
626+
let comment_count = comments.len();
627+
628+
// Track comments_by_pass
629+
let pass_tag = pass_kind
630+
.map(|k| k.tag().to_string())
631+
.unwrap_or_else(|| "default".to_string());
632+
*comments_by_pass.entry(pass_tag).or_insert(0) += comment_count;
633+
634+
// Build per-file metric; merge with existing entry for same file if multi-pass
635+
if let Some(existing) =
636+
file_metrics.iter_mut().find(|m| m.file_path == file_path)
637+
{
638+
existing.prompt_tokens += resp_prompt_tokens;
639+
existing.completion_tokens += resp_completion_tokens;
640+
existing.total_tokens += resp_total_tokens;
641+
existing.comment_count += comment_count;
642+
if latency_ms > existing.latency_ms {
643+
existing.latency_ms = latency_ms;
644+
}
645+
} else {
646+
file_metrics.push(FileMetric {
647+
file_path: file_path.clone(),
648+
latency_ms,
649+
prompt_tokens: resp_prompt_tokens,
650+
completion_tokens: resp_completion_tokens,
651+
total_tokens: resp_total_tokens,
652+
comment_count,
653+
});
654+
}
655+
552656
all_comments.extend(comments);
657+
} else {
658+
// Parse failed; still record file metric
659+
file_metrics.push(FileMetric {
660+
file_path: file_path.clone(),
661+
latency_ms,
662+
prompt_tokens: resp_prompt_tokens,
663+
completion_tokens: resp_completion_tokens,
664+
total_tokens: resp_total_tokens,
665+
comment_count: 0,
666+
});
553667
}
554668
}
555669
}
@@ -591,15 +705,24 @@ async fn review_diff_content_raw_inner(
591705
let processed_comments = core::apply_enhanced_filters(&mut enhanced_ctx, processed_comments);
592706

593707
// Apply convention-based suppression: filter out comments matching suppression patterns
594-
let processed_comments =
708+
let (processed_comments, convention_suppressed_count) =
595709
apply_convention_suppression(processed_comments, &enhanced_ctx.convention_store);
596710

597711
// Save updated convention store back to disk
598712
if let Some(ref store_path) = convention_store_path {
599713
save_convention_store(&enhanced_ctx.convention_store, store_path);
600714
}
601715

602-
Ok(processed_comments)
716+
Ok(ReviewResult {
717+
comments: processed_comments,
718+
total_prompt_tokens,
719+
total_completion_tokens,
720+
total_tokens,
721+
file_metrics,
722+
convention_suppressed_count,
723+
comments_by_pass,
724+
hotspots: enhanced_ctx.hotspots.clone(),
725+
})
603726
}
604727

605728
pub fn extract_symbols_from_diff(diff: &core::UnifiedDiff) -> Vec<String> {
@@ -1166,13 +1289,14 @@ fn find_test_files(file_path: &Path, repo_path: &Path) -> Vec<PathBuf> {
11661289

11671290
/// Apply convention-based suppression: filter out comments whose content
11681291
/// matches learned suppression patterns with high confidence.
1292+
/// Returns the filtered comments and the number of comments that were suppressed.
11691293
fn apply_convention_suppression(
11701294
comments: Vec<core::Comment>,
11711295
convention_store: &ConventionStore,
1172-
) -> Vec<core::Comment> {
1296+
) -> (Vec<core::Comment>, usize) {
11731297
let suppression_patterns = convention_store.suppression_patterns();
11741298
if suppression_patterns.is_empty() {
1175-
return comments;
1299+
return (comments, 0);
11761300
}
11771301

11781302
let before_count = comments.len();
@@ -1195,7 +1319,7 @@ fn apply_convention_suppression(
11951319
);
11961320
}
11971321

1198-
filtered
1322+
(filtered, suppressed)
11991323
}
12001324

12011325
#[cfg(test)]

0 commit comments

Comments
 (0)