Skip to main content

sentinel_core/
diff.rs

1//! Diff stage: compare two analysis reports and produce a delta.
2//!
3//! Primary use case: PR CI integration. Run `analyze` on the base
4//! branch's traces, run it again on the PR branch's traces, then compare
5//! the two reports to surface regressions (new findings, severity
6//! escalations, per-endpoint I/O op increases) and improvements
7//! (resolved findings, severity de-escalations, per-endpoint I/O op
8//! decreases).
9//!
10//! Finding identity for stable comparison is the tuple
11//! `(finding_type, service, source_endpoint, pattern.template)`. The
12//! template is already normalized at detection time so direct equality
13//! suffices, no re-normalization at diff time.
14
15use std::collections::{BTreeMap, BTreeSet};
16
17use serde::Serialize;
18
19use crate::detect::{Finding, FindingType, Severity};
20use crate::report::{PerEndpointIoOps, Report};
21
22/// Stable identity tuple for matching findings between two runs.
23///
24/// Two findings with the same `IdentityKey` are considered "the same
25/// anti-pattern" across runs. If multiple findings in one run share a
26/// key (e.g. the same N+1 template fired in two traces), the diff
27/// engine collapses them to one entry by keeping the worst-severity
28/// finding for that key.
29type IdentityKey = (FindingType, String, String, String);
30
31fn identity_of(finding: &Finding) -> IdentityKey {
32    (
33        finding.finding_type.clone(),
34        finding.service.clone(),
35        finding.source_endpoint.clone(),
36        finding.pattern.template.clone(),
37    )
38}
39
40/// Delta between two analysis runs.
41///
42/// Stable JSON shape. Field names will not be renamed or removed in a
43/// minor release; new optional fields may be added.
44#[derive(Debug, Clone, Serialize)]
45pub struct DiffReport {
46    /// Findings present in `after` but absent from `before`.
47    pub new_findings: Vec<Finding>,
48    /// Findings present in `before` but absent from `after`.
49    pub resolved_findings: Vec<Finding>,
50    /// Findings present in both runs whose worst severity differs.
51    /// Ordered with regressions (worse-after) first, then improvements.
52    pub severity_changes: Vec<SeverityChange>,
53    /// Per-endpoint I/O op deltas. Ordered with the largest regressions
54    /// first (most positive delta), then improvements last.
55    pub endpoint_metric_deltas: Vec<EndpointDelta>,
56}
57
58/// A finding whose worst severity changed between the two runs.
59#[derive(Debug, Clone, Serialize)]
60pub struct SeverityChange {
61    /// The "after" version of the finding (same identity, latest data).
62    pub finding: Finding,
63    pub before_severity: Severity,
64    pub after_severity: Severity,
65}
66
67impl SeverityChange {
68    /// `true` when the after severity is worse than the before severity.
69    /// Used to sort regressions ahead of improvements in the output and
70    /// reused by the CLI text renderer to color regressions red.
71    ///
72    /// Severity derives `Ord` with declaration order (Critical < Warning
73    /// < Info). "Worse" means numerically lower.
74    #[must_use]
75    pub fn is_regression(&self) -> bool {
76        self.after_severity < self.before_severity
77    }
78}
79
80/// Per-endpoint I/O op count delta between two runs.
81#[derive(Debug, Clone, Serialize)]
82pub struct EndpointDelta {
83    pub service: String,
84    pub endpoint: String,
85    pub before_io_ops: usize,
86    pub after_io_ops: usize,
87    /// `after - before`. Positive = regression, negative = improvement.
88    pub delta: i64,
89}
90
91/// Compare two analysis reports.
92///
93/// Both reports are expected to come from `pipeline::analyze` runs on
94/// their respective trace sets, with the same `Config` (otherwise the
95/// per-endpoint counts and severity assignments may not be comparable).
96///
97/// Pure function: takes references and returns owned data. No I/O.
98#[must_use]
99pub fn diff_runs(before: &Report, after: &Report) -> DiffReport {
100    let before_map = build_identity_map(&before.findings);
101    let after_map = build_identity_map(&after.findings);
102
103    let mut new_findings: Vec<Finding> = Vec::new();
104    let mut resolved_findings: Vec<Finding> = Vec::new();
105    let mut severity_changes: Vec<SeverityChange> = Vec::new();
106
107    for (key, after_finding) in &after_map {
108        match before_map.get(key) {
109            None => new_findings.push(after_finding.clone()),
110            Some(before_finding) if before_finding.severity != after_finding.severity => {
111                severity_changes.push(SeverityChange {
112                    finding: after_finding.clone(),
113                    before_severity: before_finding.severity.clone(),
114                    after_severity: after_finding.severity.clone(),
115                });
116            }
117            Some(_) => {}
118        }
119    }
120    for (key, before_finding) in &before_map {
121        if !after_map.contains_key(key) {
122            resolved_findings.push(before_finding.clone());
123        }
124    }
125
126    // Stable, deterministic ordering for the two finding lists. Reuses
127    // the same ordering rule as `analyze` so a reader's mental model
128    // is identical between the two outputs.
129    crate::detect::sort_findings(&mut new_findings);
130    crate::detect::sort_findings(&mut resolved_findings);
131    // Severity changes: regressions (worse-after) first, then
132    // improvements. Within each group, sort by the same finding-order
133    // rule as the regular output for predictability.
134    severity_changes.sort_by(|a, b| {
135        b.is_regression()
136            .cmp(&a.is_regression())
137            .then_with(|| a.finding.finding_type.cmp(&b.finding.finding_type))
138            .then_with(|| a.finding.service.cmp(&b.finding.service))
139            .then_with(|| a.finding.source_endpoint.cmp(&b.finding.source_endpoint))
140            .then_with(|| a.finding.pattern.template.cmp(&b.finding.pattern.template))
141    });
142
143    let endpoint_metric_deltas =
144        diff_per_endpoint_io_ops(&before.per_endpoint_io_ops, &after.per_endpoint_io_ops);
145
146    DiffReport {
147        new_findings,
148        resolved_findings,
149        severity_changes,
150        endpoint_metric_deltas,
151    }
152}
153
154/// Build an identity-keyed map of findings, collapsing duplicates by
155/// keeping the worst severity AND summing `pattern.occurrences` across
156/// all duplicates. Used as the canonical view of each side before
157/// computing the diff.
158///
159/// Returning owned `Finding` values (not borrowed) lets us aggregate
160/// occurrences without mutating the input slice. The aggregated
161/// occurrences mean the user sees the total amplitude of the pattern
162/// across the trace set, not just one trace's count, so a regression
163/// from "6 occurrences in trace A" to "60 occurrences in trace A AND 60
164/// in trace B" surfaces as a meaningful endpoint-delta plus (when
165/// severity escalates) a `severity_change`.
166///
167/// Tie-break for the kept Finding template: the first finding inserted
168/// at a key wins for `trace_id` / `first_timestamp` / `code_location` /
169/// `suggested_fix`. Since `pipeline::analyze` calls `sort_findings`
170/// before returning, this is deterministic.
171fn build_identity_map(findings: &[Finding]) -> BTreeMap<IdentityKey, Finding> {
172    let mut map: BTreeMap<IdentityKey, Finding> = BTreeMap::new();
173    for finding in findings {
174        let key = identity_of(finding);
175        match map.get_mut(&key) {
176            None => {
177                map.insert(key, finding.clone());
178            }
179            Some(existing) => {
180                // Severity derives Ord with declaration order:
181                // Critical < Warning < Info. Worst = min.
182                if finding.severity < existing.severity {
183                    let summed = existing
184                        .pattern
185                        .occurrences
186                        .saturating_add(finding.pattern.occurrences);
187                    *existing = finding.clone();
188                    existing.pattern.occurrences = summed;
189                } else {
190                    existing.pattern.occurrences = existing
191                        .pattern
192                        .occurrences
193                        .saturating_add(finding.pattern.occurrences);
194                }
195            }
196        }
197    }
198    map
199}
200
201/// Pair up `before` and `after` per-endpoint I/O op counts and emit one
202/// `EndpointDelta` per `(service, endpoint)` whose count differs.
203/// Endpoints absent from one side are treated as `0` on that side.
204/// Sorted regressions-first, then improvements (largest absolute delta
205/// inside each group last for the improvements direction).
206fn diff_per_endpoint_io_ops(
207    before: &[PerEndpointIoOps],
208    after: &[PerEndpointIoOps],
209) -> Vec<EndpointDelta> {
210    let mut before_map: BTreeMap<(&str, &str), usize> = BTreeMap::new();
211    for entry in before {
212        before_map.insert((&entry.service, &entry.endpoint), entry.io_ops);
213    }
214    let mut after_map: BTreeMap<(&str, &str), usize> = BTreeMap::new();
215    for entry in after {
216        after_map.insert((&entry.service, &entry.endpoint), entry.io_ops);
217    }
218
219    let mut keys: BTreeSet<(&str, &str)> = BTreeSet::new();
220    keys.extend(before_map.keys().copied());
221    keys.extend(after_map.keys().copied());
222
223    let mut deltas: Vec<EndpointDelta> = keys
224        .iter()
225        .filter_map(|(service, endpoint)| {
226            let before_io = before_map.get(&(*service, *endpoint)).copied().unwrap_or(0);
227            let after_io = after_map.get(&(*service, *endpoint)).copied().unwrap_or(0);
228            if before_io == after_io {
229                return None;
230            }
231            // Cast through i128 to handle the worst case (`usize::MAX`
232            // on either side) without panicking. Any plausible counts
233            // fit comfortably in i64; if a future workload pushes past
234            // `i64::MAX` we clamp and warn rather than overflow silently.
235            let delta = i128::from(after_io as u64) - i128::from(before_io as u64);
236            let delta_i64 = i64::try_from(delta).unwrap_or_else(|_| {
237                tracing::warn!(
238                    target: "perf_sentinel::diff",
239                    service = %service,
240                    endpoint = %endpoint,
241                    before_io = before_io,
242                    after_io = after_io,
243                    "endpoint I/O op delta overflows i64, clamping for output"
244                );
245                if delta > 0 { i64::MAX } else { i64::MIN }
246            });
247            Some(EndpointDelta {
248                service: (*service).to_string(),
249                endpoint: (*endpoint).to_string(),
250                before_io_ops: before_io,
251                after_io_ops: after_io,
252                delta: delta_i64,
253            })
254        })
255        .collect();
256
257    // Regressions first: positive deltas before negative ones, then by
258    // descending magnitude inside each group. Tie-break on `(service,
259    // endpoint)` so the ordering is stable across runs even if
260    // `sort_by`'s internal stability guarantee changes.
261    deltas.sort_by(|a, b| {
262        b.delta
263            .cmp(&a.delta)
264            .then_with(|| a.service.cmp(&b.service))
265            .then_with(|| a.endpoint.cmp(&b.endpoint))
266    });
267    deltas
268}
269
270#[cfg(test)]
271mod tests {
272    use super::*;
273    use crate::detect::{Confidence, Finding, FindingType, Pattern, Severity};
274    use crate::report::{Analysis, GreenSummary, PerEndpointIoOps, QualityGate, Report};
275
276    fn make_report(findings: Vec<Finding>, per_endpoint: Vec<PerEndpointIoOps>) -> Report {
277        Report {
278            analysis: Analysis {
279                duration_ms: 0,
280                events_processed: 0,
281                traces_analyzed: 0,
282            },
283            findings,
284            green_summary: GreenSummary::disabled(0),
285            quality_gate: QualityGate {
286                passed: true,
287                rules: vec![],
288            },
289            per_endpoint_io_ops: per_endpoint,
290            correlations: vec![],
291            warnings: vec![],
292            warning_details: vec![],
293            acknowledged_findings: vec![],
294            binary_version: String::new(),
295            disclosure_waste: None,
296        }
297    }
298
299    fn finding(
300        ft: FindingType,
301        sev: Severity,
302        service: &str,
303        endpoint: &str,
304        template: &str,
305    ) -> Finding {
306        Finding {
307            finding_type: ft,
308            severity: sev,
309            trace_id: "trace-1".to_string(),
310            service: service.to_string(),
311            source_endpoint: endpoint.to_string(),
312            pattern: Pattern {
313                template: template.to_string(),
314                occurrences: 6,
315                window_ms: 200,
316                distinct_params: 6,
317                ..Default::default()
318            },
319            suggestion: "batch".to_string(),
320            first_timestamp: "2025-07-10T14:32:01.000Z".to_string(),
321            last_timestamp: "2025-07-10T14:32:01.250Z".to_string(),
322            green_impact: None,
323            confidence: Confidence::default(),
324            classification_method: None,
325            code_location: None,
326            instrumentation_scopes: Vec::new(),
327            suggested_fix: None,
328            signature: String::new(),
329        }
330    }
331
332    fn endpoint(service: &str, ep: &str, ops: usize) -> PerEndpointIoOps {
333        PerEndpointIoOps {
334            service: service.to_string(),
335            endpoint: ep.to_string(),
336            io_ops: ops,
337        }
338    }
339
340    #[test]
341    fn identical_runs_produce_empty_diff() {
342        let f = finding(
343            FindingType::NPlusOneSql,
344            Severity::Warning,
345            "svc",
346            "POST /api",
347            "SELECT *",
348        );
349        let before = make_report(vec![f.clone()], vec![endpoint("svc", "POST /api", 6)]);
350        let after = make_report(vec![f], vec![endpoint("svc", "POST /api", 6)]);
351        let diff = diff_runs(&before, &after);
352        assert!(diff.new_findings.is_empty());
353        assert!(diff.resolved_findings.is_empty());
354        assert!(diff.severity_changes.is_empty());
355        assert!(diff.endpoint_metric_deltas.is_empty());
356    }
357
358    #[test]
359    fn finding_present_only_in_after_is_new() {
360        let before = make_report(vec![], vec![]);
361        let after = make_report(
362            vec![finding(
363                FindingType::NPlusOneSql,
364                Severity::Warning,
365                "svc",
366                "POST /api",
367                "SELECT *",
368            )],
369            vec![],
370        );
371        let diff = diff_runs(&before, &after);
372        assert_eq!(diff.new_findings.len(), 1);
373        assert!(diff.resolved_findings.is_empty());
374        assert!(diff.severity_changes.is_empty());
375    }
376
377    #[test]
378    fn finding_present_only_in_before_is_resolved() {
379        let before = make_report(
380            vec![finding(
381                FindingType::NPlusOneSql,
382                Severity::Warning,
383                "svc",
384                "POST /api",
385                "SELECT *",
386            )],
387            vec![],
388        );
389        let after = make_report(vec![], vec![]);
390        let diff = diff_runs(&before, &after);
391        assert!(diff.new_findings.is_empty());
392        assert_eq!(diff.resolved_findings.len(), 1);
393        assert!(diff.severity_changes.is_empty());
394    }
395
396    #[test]
397    fn same_identity_with_different_severity_is_severity_change() {
398        let f_warn = finding(
399            FindingType::NPlusOneSql,
400            Severity::Warning,
401            "svc",
402            "POST /api",
403            "SELECT *",
404        );
405        let mut f_crit = f_warn.clone();
406        f_crit.severity = Severity::Critical;
407        let before = make_report(vec![f_warn], vec![]);
408        let after = make_report(vec![f_crit], vec![]);
409        let diff = diff_runs(&before, &after);
410        assert!(diff.new_findings.is_empty());
411        assert!(diff.resolved_findings.is_empty());
412        assert_eq!(diff.severity_changes.len(), 1);
413        let change = &diff.severity_changes[0];
414        assert_eq!(change.before_severity, Severity::Warning);
415        assert_eq!(change.after_severity, Severity::Critical);
416        assert!(
417            change.is_regression(),
418            "warning -> critical is a regression"
419        );
420    }
421
422    #[test]
423    fn severity_changes_sorted_regressions_first() {
424        // After: one regression (warning -> critical), one improvement (critical -> warning).
425        let before = make_report(
426            vec![
427                finding(
428                    FindingType::NPlusOneSql,
429                    Severity::Warning,
430                    "svc-a",
431                    "POST /a",
432                    "SELECT a",
433                ),
434                finding(
435                    FindingType::NPlusOneSql,
436                    Severity::Critical,
437                    "svc-b",
438                    "POST /b",
439                    "SELECT b",
440                ),
441            ],
442            vec![],
443        );
444        let after = make_report(
445            vec![
446                finding(
447                    FindingType::NPlusOneSql,
448                    Severity::Critical,
449                    "svc-a",
450                    "POST /a",
451                    "SELECT a",
452                ),
453                finding(
454                    FindingType::NPlusOneSql,
455                    Severity::Warning,
456                    "svc-b",
457                    "POST /b",
458                    "SELECT b",
459                ),
460            ],
461            vec![],
462        );
463        let diff = diff_runs(&before, &after);
464        assert_eq!(diff.severity_changes.len(), 2);
465        assert!(
466            diff.severity_changes[0].is_regression(),
467            "regression must come first"
468        );
469        assert!(
470            !diff.severity_changes[1].is_regression(),
471            "improvement must come last"
472        );
473    }
474
475    #[test]
476    fn duplicate_identity_in_one_run_is_collapsed_to_worst_severity() {
477        // Two findings with the same identity tuple in `after`, one at
478        // Warning and one at Critical. They should collapse to a single
479        // "after" finding at Critical, and the diff should not interpret
480        // the count difference as a severity change.
481        let before = make_report(
482            vec![finding(
483                FindingType::NPlusOneSql,
484                Severity::Critical,
485                "svc",
486                "POST /api",
487                "SELECT *",
488            )],
489            vec![],
490        );
491        let f_warn = finding(
492            FindingType::NPlusOneSql,
493            Severity::Warning,
494            "svc",
495            "POST /api",
496            "SELECT *",
497        );
498        let mut f_crit = f_warn.clone();
499        f_crit.severity = Severity::Critical;
500        let after = make_report(vec![f_warn, f_crit], vec![]);
501        let diff = diff_runs(&before, &after);
502        assert!(
503            diff.new_findings.is_empty(),
504            "no new findings when identity is shared"
505        );
506        assert!(
507            diff.resolved_findings.is_empty(),
508            "no resolved when identity is shared"
509        );
510        assert!(
511            diff.severity_changes.is_empty(),
512            "worst-severity dedupe should make this a no-op (Critical == Critical)"
513        );
514    }
515
516    #[test]
517    fn endpoint_io_ops_increase_is_a_positive_delta() {
518        let before = make_report(vec![], vec![endpoint("svc", "POST /api/users", 10)]);
519        let after = make_report(vec![], vec![endpoint("svc", "POST /api/users", 20)]);
520        let diff = diff_runs(&before, &after);
521        assert_eq!(diff.endpoint_metric_deltas.len(), 1);
522        let d = &diff.endpoint_metric_deltas[0];
523        assert_eq!(d.service, "svc");
524        assert_eq!(d.endpoint, "POST /api/users");
525        assert_eq!(d.before_io_ops, 10);
526        assert_eq!(d.after_io_ops, 20);
527        assert_eq!(d.delta, 10);
528    }
529
530    #[test]
531    fn endpoint_absent_from_before_is_a_full_addition() {
532        let before = make_report(vec![], vec![]);
533        let after = make_report(vec![], vec![endpoint("svc", "POST /api", 7)]);
534        let diff = diff_runs(&before, &after);
535        assert_eq!(diff.endpoint_metric_deltas.len(), 1);
536        let d = &diff.endpoint_metric_deltas[0];
537        assert_eq!(d.before_io_ops, 0);
538        assert_eq!(d.after_io_ops, 7);
539        assert_eq!(d.delta, 7);
540    }
541
542    #[test]
543    fn endpoint_absent_from_after_is_a_full_removal() {
544        let before = make_report(vec![], vec![endpoint("svc", "POST /api", 5)]);
545        let after = make_report(vec![], vec![]);
546        let diff = diff_runs(&before, &after);
547        assert_eq!(diff.endpoint_metric_deltas.len(), 1);
548        let d = &diff.endpoint_metric_deltas[0];
549        assert_eq!(d.before_io_ops, 5);
550        assert_eq!(d.after_io_ops, 0);
551        assert_eq!(d.delta, -5);
552    }
553
554    #[test]
555    fn endpoint_deltas_sorted_regressions_first() {
556        let before = make_report(
557            vec![],
558            vec![
559                endpoint("svc", "POST /improve", 10),
560                endpoint("svc", "POST /regress", 5),
561                endpoint("svc", "POST /steady", 7),
562            ],
563        );
564        let after = make_report(
565            vec![],
566            vec![
567                endpoint("svc", "POST /improve", 2),
568                endpoint("svc", "POST /regress", 50),
569                endpoint("svc", "POST /steady", 7),
570            ],
571        );
572        let diff = diff_runs(&before, &after);
573        assert_eq!(diff.endpoint_metric_deltas.len(), 2);
574        assert_eq!(diff.endpoint_metric_deltas[0].endpoint, "POST /regress");
575        assert_eq!(diff.endpoint_metric_deltas[0].delta, 45);
576        assert_eq!(diff.endpoint_metric_deltas[1].endpoint, "POST /improve");
577        assert_eq!(diff.endpoint_metric_deltas[1].delta, -8);
578    }
579
580    #[test]
581    fn equal_severity_in_both_runs_is_not_a_severity_change() {
582        // Guard against a future refactor that compares with `<` or
583        // `<=` instead of `!=` and silently classifies equal-severity
584        // findings as severity changes.
585        let f = finding(
586            FindingType::NPlusOneSql,
587            Severity::Critical,
588            "svc",
589            "POST /api",
590            "SELECT *",
591        );
592        let before = make_report(vec![f.clone()], vec![]);
593        let after = make_report(vec![f], vec![]);
594        let diff = diff_runs(&before, &after);
595        assert!(diff.severity_changes.is_empty());
596    }
597
598    #[test]
599    fn same_identity_different_trace_id_is_treated_as_one_finding() {
600        // Two findings with identical (type, service, endpoint, template)
601        // but different trace_ids are conceptually "the same anti-pattern"
602        // observed twice. The diff collapses them and sums occurrences.
603        let mut f_a = finding(
604            FindingType::NPlusOneSql,
605            Severity::Warning,
606            "svc",
607            "POST /api",
608            "SELECT *",
609        );
610        f_a.trace_id = "trace-a".to_string();
611        f_a.pattern.occurrences = 6;
612        let mut f_b = f_a.clone();
613        f_b.trace_id = "trace-b".to_string();
614        f_b.pattern.occurrences = 12;
615
616        let before = make_report(vec![], vec![]);
617        let after = make_report(vec![f_a, f_b], vec![]);
618        let diff = diff_runs(&before, &after);
619        assert_eq!(diff.new_findings.len(), 1, "two duplicates collapse to one");
620        assert_eq!(
621            diff.new_findings[0].pattern.occurrences, 18,
622            "occurrences from both findings sum on collapse"
623        );
624    }
625
626    #[test]
627    fn duplicate_identity_collapse_sums_occurrences() {
628        // Direct test of `build_identity_map` summing semantic.
629        let mut f_a = finding(
630            FindingType::NPlusOneSql,
631            Severity::Warning,
632            "svc",
633            "POST /api",
634            "SELECT *",
635        );
636        f_a.pattern.occurrences = 6;
637        let mut f_b = f_a.clone();
638        f_b.pattern.occurrences = 60;
639
640        let before = make_report(vec![], vec![]);
641        let after = make_report(vec![f_a, f_b], vec![]);
642        let diff = diff_runs(&before, &after);
643        assert_eq!(diff.new_findings.len(), 1);
644        assert_eq!(diff.new_findings[0].pattern.occurrences, 66);
645    }
646
647    #[test]
648    fn diff_sarif_emits_one_result_per_new_finding() {
649        // Smoke test for the public `findings_to_sarif` re-use path
650        // exercised by `perf-sentinel diff --format sarif`.
651        let f = finding(
652            FindingType::NPlusOneSql,
653            Severity::Warning,
654            "svc",
655            "POST /api",
656            "SELECT *",
657        );
658        let before = make_report(vec![], vec![]);
659        let after = make_report(vec![f], vec![]);
660        let diff = diff_runs(&before, &after);
661        assert_eq!(diff.new_findings.len(), 1);
662        let sarif = crate::report::sarif::findings_to_sarif(&diff.new_findings);
663        assert_eq!(
664            sarif.runs[0].results.len(),
665            diff.new_findings.len(),
666            "SARIF results count must match new_findings count"
667        );
668    }
669}