Compute hotspots per-source coverage gap in SDK (#370)

The Rust hotspots presenter walked the ledger a second time after the
SDK verb already walked it, just to recover the per-source breakdown
that drives the coverage-notice clause. For multi-month ledgers that
doubled the I/O cost of `burn hotspots`.

Widen `HotspotsFidelityBlock` with a non-serialized `excluded_by_source`
field that the SDK populates during the same eligible/excluded split.
The CLI becomes a pure renderer over already-computed data, dropping
its `describe_excluded_turns` re-walk.

Closes #342

https://claude.ai/code/session_01M2cRDSDfUwi4MPSxRq3WaA

Co-authored-by: Claude <noreply@anthropic.com>

Author: willwashburn

crates/relayburn-cli/src/commands/hotspots.rs

@@ -24,14 +24,12 @@
 //!    discriminator and emits the inner `HotspotsAttributionResult`
 //!    shape directly (TS contract).
 
-use std::collections::BTreeMap;
-
 use clap::Args;
 use relayburn_sdk::{
-    hotspots as sdk_hotspots, ingest_all, normalize_since, AttributionMethod, BashAggregation,
-    BashVerbAggregation, FileAggregation, HotspotsAttributionResult, HotspotsGroupBy,
-    HotspotsOptions, HotspotsResult, HotspotsSessionTotal, Ledger, LedgerOpenOptions, Query,
-    SubagentAggregation,
+    hotspots as sdk_hotspots, ingest_all, AttributionMethod, BashAggregation,
+    BashVerbAggregation, FileAggregation, HotspotsAttributionResult, HotspotsExcludedBreakdown,
+    HotspotsExcludedSourceRow, HotspotsGroupBy, HotspotsOptions, HotspotsResult,
+    HotspotsSessionTotal, Ledger, LedgerOpenOptions, SubagentAggregation,
 };
 use serde_json::{json, Map, Value};
 
@@ -148,23 +146,6 @@ fn run_inner(globals: &GlobalArgs, args: HotspotsArgs) -> anyhow::Result<i32> {
         .build()?;
     let raw_opts = relayburn_sdk::RawIngestOptions::default();
     rt.block_on(ingest_all(handle.raw_mut(), &raw_opts))?;
-
-    // Pre-compute the per-source coverage-gap breakdown so the human
-    // renderer can name *which* sources contributed excluded turns and
-    // *what* they were missing. Reaching past the SDK verb here is a
-    // small layering compromise: the verb internally walks the same
-    // slice but only surfaces an aggregate `fidelity` block. Doing it
-    // here keeps the source-attributed clause in `coverage_notice` honest
-    // when the slice mixes sources (e.g. excluded codex + opencode turns
-    // both showing up in the message).
-    let mut q = Query::default();
-    if let Some(p) = args.project.clone() {
-        q.project = Some(p);
-    }
-    if let Some(since) = normalize_since(args.since.as_deref())? {
-        q.since = Some(since);
-    }
-    let breakdown = describe_excluded_turns(&handle, &q)?;
     drop(handle);
 
     let result = sdk_hotspots(HotspotsOptions {
@@ -181,72 +162,10 @@ fn run_inner(globals: &GlobalArgs, args: HotspotsArgs) -> anyhow::Result<i32> {
         return Ok(0);
     }
     let limit = if args.all { usize::MAX } else { DEFAULT_TOP_N };
-    emit_human(&result, limit, &breakdown);
+    emit_human(&result, limit);
     Ok(0)
 }
 
-/// Per-source coverage-gap breakdown, mirroring the TS
-/// `describeExcluded` helper. Only counts turns that *fail* the
-/// hotspots-attribution gate (`hasToolCalls && hasToolResultEvents`);
-/// turns without a `fidelity` field are best-effort full and never
-/// excluded. The SDK's [`relayburn_sdk::HotspotsResult`] reports
-/// aggregate analyzed/excluded counts but not the source mix; we walk
-/// the ledger slice ourselves to recover that detail.
-#[derive(Debug, Clone, Default)]
-struct CoverageGapBreakdown {
-    /// Sorted by source label so the rendered clause is deterministic.
-    sources: BTreeMap<String, SourceClause>,
-    /// Total excluded count across all sources. Equal to
-    /// `attribution_result.fidelity.excluded` for non-empty slices.
-    excluded: u64,
-    /// Total analyzed (eligible) count.
-    analyzed: u64,
-}
-
-#[derive(Debug, Clone, Default)]
-struct SourceClause {
-    /// Distinct missing-coverage labels (`tool-call records`, etc.).
-    missing: BTreeMap<String, ()>,
-    /// Distinct granularities observed on excluded turns from this
-    /// source. Used by the inline rendered form `<missing>, <gran>
-    /// granularity (<source>)`.
-    granularities: BTreeMap<String, ()>,
-    count: u64,
-}
-
-fn describe_excluded_turns(
-    handle: &relayburn_sdk::LedgerHandle,
-    q: &Query,
-) -> anyhow::Result<CoverageGapBreakdown> {
-    let mut out = CoverageGapBreakdown::default();
-    for enriched in handle.raw().query_turns(q)? {
-        let t = &enriched.turn;
-        let Some(f) = t.fidelity.as_ref() else {
-            // No fidelity → treat as best-effort full; never excluded.
-            out.analyzed += 1;
-            continue;
-        };
-        let passes = f.coverage.has_tool_calls && f.coverage.has_tool_result_events;
-        if passes {
-            out.analyzed += 1;
-            continue;
-        }
-        out.excluded += 1;
-        let entry = out.sources.entry(t.source.wire_str().to_string()).or_default();
-        entry.count += 1;
-        if !f.coverage.has_tool_calls {
-            entry.missing.insert("tool-call records".to_string(), ());
-        }
-        if !f.coverage.has_tool_result_events {
-            entry.missing.insert("tool-result events".to_string(), ());
-        }
-        entry
-            .granularities
-            .insert(f.granularity.wire_str().to_string(), ());
-    }
-    Ok(out)
-}
-
 fn emit_json(result: &HotspotsResult) {
     let mut value = hotspots_result_to_json(result);
     coerce_whole_f64_to_int(&mut value);
@@ -492,9 +411,9 @@ fn subagent_to_json(s: &SubagentAggregation) -> Value {
 
 // ---------- human rendering ----------
 
-fn emit_human(result: &HotspotsResult, limit: usize, breakdown: &CoverageGapBreakdown) {
+fn emit_human(result: &HotspotsResult, limit: usize) {
     match result {
-        HotspotsResult::Attribution(a) => emit_human_attribution(a, limit, breakdown),
+        HotspotsResult::Attribution(a) => emit_human_attribution(a, limit),
         // The single-axis group_by surfaces aren't yet tied to a golden
         // snapshot (the snapshot covers the default attribution view),
         // so we render their tables on a best-effort basis with the same
@@ -608,17 +527,13 @@ where
     print!("{}", lines.join("\n"));
 }
 
-fn emit_human_attribution(
-    a: &HotspotsAttributionResult,
-    limit: usize,
-    breakdown: &CoverageGapBreakdown,
-) {
+fn emit_human_attribution(a: &HotspotsAttributionResult, limit: usize) {
     let degraded = a.attribution_degraded;
     let approx_suffix = if degraded { " (approximate)" } else { "" };
     let mut out: Vec<String> = Vec::new();
     out.push(String::new());
     out.push(format!("turns analyzed: {}", format_uint(a.turns_analyzed)));
-    if let Some(notice) = coverage_notice(a, breakdown) {
+    if let Some(notice) = coverage_notice(a) {
         out.push(notice);
     }
     out.push(format!(
@@ -773,10 +688,7 @@ fn emit_human_attribution(
     print!("{}", out.join("\n"));
 }
 
-fn coverage_notice(
-    a: &HotspotsAttributionResult,
-    breakdown: &CoverageGapBreakdown,
-) -> Option<String> {
+fn coverage_notice(a: &HotspotsAttributionResult) -> Option<String> {
     let analyzed = a.fidelity.analyzed;
     let excluded = a.fidelity.excluded;
     if excluded == 0 {
@@ -786,16 +698,14 @@ fn coverage_notice(
     // The TS shape is one inline clause per source kind, joined with " and ".
     // Each clause names the missing field(s) + the granularity bucket the
     // excluded turns carried, with the source name in parens. Sources are
-    // walked in BTreeMap order for stable rendering.
-    let clauses: Vec<String> = breakdown
-        .sources
-        .iter()
-        .map(|(source, row)| render_inline_source_clause(source, row))
-        .collect();
+    // walked in BTreeMap order for stable rendering. The breakdown is
+    // computed by the SDK in the same pass that produced the rest of the
+    // attribution result — no second ledger walk here.
+    let clauses: Vec<String> = render_source_clauses(&a.fidelity.excluded_by_source);
     let suffix = if clauses.is_empty() {
-        // Fall back to the SDK's aggregate counts if for some reason the
-        // re-walk surfaced no breakdown (e.g. record without fidelity but
-        // the SDK still excluded it). Don't fabricate a source label.
+        // Fall back to the SDK's aggregate counts if the breakdown is empty
+        // (e.g. a turn without a fidelity record that the SDK still excluded).
+        // Don't fabricate a source label.
         String::new()
     } else {
         format!(" for {}", clauses.join(" and "))
@@ -809,14 +719,22 @@ fn coverage_notice(
     ))
 }
 
-fn render_inline_source_clause(source: &str, row: &SourceClause) -> String {
+fn render_source_clauses(breakdown: &HotspotsExcludedBreakdown) -> Vec<String> {
+    breakdown
+        .sources
+        .iter()
+        .map(|(source, row)| render_inline_source_clause(source, row))
+        .collect()
+}
+
+fn render_inline_source_clause(source: &str, row: &HotspotsExcludedSourceRow) -> String {
     let mut inner: Vec<String> = Vec::new();
     if !row.missing.is_empty() {
-        let missing: Vec<&str> = row.missing.keys().map(String::as_str).collect();
+        let missing: Vec<&str> = row.missing.iter().map(String::as_str).collect();
         inner.push(format!("missing {}", missing.join(", ")));
     }
     if !row.granularities.is_empty() {
-        let grans: Vec<&str> = row.granularities.keys().map(String::as_str).collect();
+        let grans: Vec<&str> = row.granularities.iter().map(String::as_str).collect();
         inner.push(format!("{} granularity", grans.join("+")));
     }
     if inner.is_empty() {

crates/relayburn-sdk/src/query_verbs.rs

@@ -8,7 +8,7 @@
 //! Option<PathBuf>` so callers don't have to mutate process env to point
 //! at a non-default ledger.
 
-use std::collections::{BTreeMap, HashMap, HashSet};
+use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
 use std::fs;
 use std::path::{Path, PathBuf};
 
@@ -844,6 +844,33 @@ pub struct HotspotsFidelityBlock {
     /// this JSON block directly.
     pub summary: serde_json::Value,
     pub refused: bool,
+    /// Per-source coverage-gap breakdown. Computed in the same pass as the
+    /// eligible/excluded split so CLI/MCP renderers don't need to re-walk the
+    /// ledger to recover *which* sources contributed excluded turns. Not
+    /// serialized — the JSON contract owns the aggregate counts above; this
+    /// is an in-process renderer aid.
+    #[serde(skip)]
+    pub excluded_by_source: HotspotsExcludedBreakdown,
+}
+
+/// Per-source breakdown of turns that failed the hotspots coverage gate.
+/// Sources are keyed by their wire string (e.g. `claude`, `codex`,
+/// `opencode`) so the renderer can produce stable ordering without a second
+/// ledger walk. See `HotspotsFidelityBlock::excluded_by_source`.
+#[derive(Debug, Clone, Default)]
+pub struct HotspotsExcludedBreakdown {
+    pub sources: BTreeMap<String, HotspotsExcludedSourceRow>,
+}
+
+#[derive(Debug, Clone, Default)]
+pub struct HotspotsExcludedSourceRow {
+    pub count: u64,
+    /// Distinct missing-coverage labels (e.g. `tool-call records`,
+    /// `tool-result events`).
+    pub missing: BTreeSet<String>,
+    /// Distinct granularity buckets observed on excluded turns from this
+    /// source.
+    pub granularities: BTreeSet<String>,
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize)]
@@ -971,10 +998,12 @@ fn run_hotspots_attribution(
 ) -> Result<HotspotsResult> {
     let mut eligible: Vec<TurnRecord> = Vec::new();
     let mut excluded: Vec<TurnRecord> = Vec::new();
+    let mut excluded_by_source = HotspotsExcludedBreakdown::default();
     for t in turns {
         if turn_passes_hotspots_coverage(t) {
             eligible.push(t.clone());
         } else {
+            record_excluded_source(&mut excluded_by_source, t);
             excluded.push(t.clone());
         }
     }
@@ -993,6 +1022,7 @@ fn run_hotspots_attribution(
             refusal,
             turns.len() as u64,
             summary_value,
+            excluded_by_source,
         ));
     }
 
@@ -1087,18 +1117,44 @@ fn run_hotspots_attribution(
                 excluded: excluded.len() as u64,
                 summary: summary_value,
                 refused: false,
+                excluded_by_source,
             },
             refused: None,
             refusal_reason: None,
         },
     )))
 }
 
+/// Folds the coverage gap on `t` into the per-source breakdown. Mirrors
+/// the CLI-side `describeExcluded` from `packages/cli/src/commands/hotspots.ts`
+/// so callers can render the inline source clause without a second ledger
+/// walk. Turns without `fidelity` are treated as best-effort full upstream
+/// (`turn_passes_hotspots_coverage`) and never reach this function.
+fn record_excluded_source(out: &mut HotspotsExcludedBreakdown, t: &TurnRecord) {
+    let entry = out
+        .sources
+        .entry(t.source.wire_str().to_string())
+        .or_default();
+    entry.count += 1;
+    if let Some(f) = t.fidelity.as_ref() {
+        if !f.coverage.has_tool_calls {
+            entry.missing.insert("tool-call records".to_string());
+        }
+        if !f.coverage.has_tool_result_events {
+            entry.missing.insert("tool-result events".to_string());
+        }
+        entry
+            .granularities
+            .insert(f.granularity.wire_str().to_string());
+    }
+}
+
 fn refused_for_group(
     group: HotspotsGroupBy,
     refusal: String,
     excluded_total: u64,
     summary_value: serde_json::Value,
+    excluded_by_source: HotspotsExcludedBreakdown,
 ) -> HotspotsResult {
     match group {
         HotspotsGroupBy::Bash => HotspotsResult::Bash {
@@ -1138,6 +1194,7 @@ fn refused_for_group(
                     excluded: excluded_total,
                     summary: summary_value,
                     refused: true,
+                    excluded_by_source,
                 },
                 refused: Some(true),
                 refusal_reason: Some(refusal),