rto_graph/review_score.rs
1//! Scoring a candidate reviewer against the adjudicated corpus (Stage 35).
2//!
3//! A review tool is otherwise unmeasurable: its output is prose, its mistakes are
4//! plausible, and nobody remembers last week's false positives well enough to
5//! count them. [`crate::review_corpus`] fixes a set of comments with known
6//! verdicts; this module turns a candidate's findings into numbers over that set,
7//! so a different model, a changed prompt or a graph-grounded arm are comparable
8//! across attempts rather than argued about.
9//!
10//! # Per class, never averaged
11//!
12//! [`Score::per_class`] is the headline, and an aggregate deliberately is not.
13//! An average hides the only thing an implementer needs — *which* kinds of defect
14//! a reviewer can see, so they know which to target and which to leave to
15//! something else. The corpus's largest class, `contract-drift`, is also the one a
16//! diff-only reviewer is least equipped for, since the doc making a claim and the
17//! code breaking it need not be adjacent; that is a fact about classes, invisible
18//! in a mean.
19//!
20//! Read [`Score::per_class`] with [`ClassRecall::real`] in view: most classes hold
21//! a single row, so their recall is one bit, not a rate. [`Score::caveats`] says so
22//! in the report rather than leaving a reader to infer it.
23//!
24//! # Recall is computable here. Precision is not — and the difference matters
25//!
26//! **Recall is well defined.** For each row the corpus marks `real`, either the
27//! candidate found that defect or it did not.
28//!
29//! **Precision is not**, and assuming otherwise is the most inviting error in this
30//! module. The corpus is a complete record of *what one reviewer said* about those
31//! trees — **not** a complete inventory of the defects in them. So a candidate
32//! finding that matches no row is **unadjudicated**, not false: it may be a
33//! genuine defect that the original reviewer never mentioned. Counting it as a
34//! false positive would understate a better reviewer precisely for being better.
35//!
36//! What this module therefore reports is three separate numbers, named so they
37//! cannot be blurred together:
38//!
39//! - [`ClassRecall`] — per class, over the real rows.
40//! - [`Score::known_false_reproduced`] — of the rows known to be false, how many
41//! the candidate repeated. This is the *measured* precision signal, and the only
42//! one the corpus licenses.
43//! - [`Score::unadjudicated`] — findings the corpus cannot judge. Not precision;
44//! it is the human-cost proxy, because each one costs somebody a real
45//! investigation, and it becomes precision only once a human adjudicates it and
46//! the rows are added to the corpus.
47//!
48//! [`Score::corpus_precision`] is offered over the adjudicated findings alone and
49//! returns `None` when there are none, rather than a flattering `1.0`.
50//!
51//! # Scoring at the wrong commit reports zero, quietly
52//!
53//! Every row carries the commit the comment was made against. The merged PR head
54//! contains the *fix* commits, so a candidate run against it is asked to find
55//! defects that are no longer there and will appear to have missed all of them —
56//! a silent zero, not an error. [`CandidateRun`] therefore records which shas were
57//! *attempted*, [`score`] refuses a run naming a sha the corpus does not know
58//! (overwhelmingly a PR head), and rows outside the attempted set are excluded
59//! from the denominator rather than counted as misses.
60
61use std::collections::{BTreeMap, BTreeSet};
62
63use serde::{Deserialize, Serialize};
64
65use crate::review_corpus::{CLASSES, Corpus, CorpusRow, DefectClass, Verdict};
66
67/// How far from a row's line a candidate finding may be anchored and still count
68/// as the same defect.
69///
70/// Not zero, because a reviewer's cited line can be wrong while its point is
71/// right — `docs/REVIEW_CHECKLIST.md` has a rule for exactly that case — so an
72/// exact-line match would score a correct finding as a miss. Not wide, because a
73/// window long enough to span unrelated code turns "commented on the file" into
74/// "found the defect". Ten lines keeps the three `vacuous-test` rows of #299
75/// distinct (they sit 50 lines apart), which
76/// `the_window_bounds_which_row_a_distant_finding_can_claim` holds it to.
77pub const LINE_WINDOW: u32 = 10;
78
79/// A finding a candidate reviewer offered, in the corpus's coordinate system.
80///
81/// Not [`crate::findings::Finding`]: that models a persisted *analyzer* result
82/// owned by an `AnalysisRun` with a runner, an isolation mode and an advisory-db
83/// digest (ADR-0012). A candidate finding is an ephemeral opinion about one line
84/// of one commit, and scoring it must stay a pure function.
85#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
86#[serde(deny_unknown_fields)]
87pub struct CandidateFinding {
88 /// The commit the candidate was looking at — a corpus `reviewed_sha`.
89 pub reviewed_sha: String,
90 /// Repository-relative path the finding is anchored to.
91 pub path: String,
92 /// Line in that file.
93 pub line: u32,
94 /// What the candidate said, for a human reading the score's misses and
95 /// unadjudicated findings.
96 pub description: String,
97 /// Whether the finding asserts the code will not build. Declared by the
98 /// candidate rather than guessed from the prose here, so that the suppression
99 /// rule in [`crate::compile_claim`] and the score agree about which findings
100 /// are compile claims.
101 #[serde(default)]
102 pub claims_compile_failure: bool,
103 /// The class the candidate assigned, if it assigned one. Recall does **not**
104 /// require agreement: a reviewer that finds the defect and mislabels it has
105 /// still found it, and [`ClassRecall::misclassified`] records the disagreement
106 /// separately.
107 #[serde(default)]
108 pub defect_class: Option<DefectClass>,
109}
110
111/// One candidate reviewer's whole run.
112///
113/// `attempted_shas` is not derivable from the findings, and the difference is the
114/// point: a commit that was reviewed and yielded nothing is a miss, while a commit
115/// that was never reviewed is outside the measurement. Conflating them turns a
116/// partial run into a bad score.
117#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
118#[serde(deny_unknown_fields)]
119pub struct CandidateRun {
120 /// Stable schema tag, so a document says what it is and a mistyped one fails
121 /// with that as the message rather than as a missing field.
122 #[serde(default = "run_schema")]
123 pub schema: String,
124 /// Which commits the candidate was actually run against.
125 pub attempted_shas: BTreeSet<String>,
126 /// Every finding it offered.
127 pub findings: Vec<CandidateFinding>,
128 /// Findings the candidate withheld under [`crate::compile_claim`], reported so
129 /// that a suppression that discarded a *true* finding shows up as a miss with
130 /// an explanation instead of an unexplained one.
131 #[serde(default)]
132 pub suppressed: Vec<CandidateFinding>,
133 /// **Which arm produced this run**, when the producer knows — the context it
134 /// was given and the model that generated it.
135 ///
136 /// Stage 35b PR 2 is a comparison of two runs that must differ in exactly one
137 /// variable, and a reader has to be able to check that claim from the
138 /// artifacts rather than from their filenames. A run document that cannot say
139 /// which arm it is makes the comparison unauditable, and mixing two up would
140 /// produce a clean, meaningless number of exactly the kind this stage is
141 /// arranged against.
142 ///
143 /// Optional, so every `v1` document written before this field existed still
144 /// parses unchanged. The reverse does not hold: `deny_unknown_fields` means a
145 /// build older than this one rejects a document carrying it. That is the
146 /// deliberate trade — a run whose provenance an old binary silently dropped
147 /// would be worse than one it refuses to read.
148 #[serde(default, skip_serializing_if = "Option::is_none")]
149 pub arm: Option<RunArm>,
150}
151
152/// What produced a [`CandidateRun`]: the context arm and the model.
153#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
154#[serde(deny_unknown_fields)]
155pub struct RunArm {
156 /// The context the reviewer was given — `diff-only` or `graph`.
157 pub context: String,
158 /// The model that generated it, so a comparison across arms can be shown to
159 /// have held it fixed.
160 pub model: String,
161}
162
163/// Schema tag for a [`CandidateRun`] document.
164pub const RUN_SCHEMA: &str = "roteiro.review-run/v1";
165
166/// `serde` default for [`CandidateRun::schema`].
167fn run_schema() -> String {
168 RUN_SCHEMA.to_owned()
169}
170
171/// Written out rather than derived so that a default-constructed run carries the
172/// real schema tag: `#[derive(Default)]` would give it the empty string, and a
173/// value that cannot round-trip through [`CandidateRun::parse`] is a trap.
174impl Default for CandidateRun {
175 fn default() -> Self {
176 Self {
177 schema: run_schema(),
178 attempted_shas: BTreeSet::new(),
179 findings: Vec::new(),
180 suppressed: Vec::new(),
181 arm: None,
182 }
183 }
184}
185
186impl CandidateRun {
187 /// Parse a run document, checking its schema tag.
188 ///
189 /// # Errors
190 /// [`ScoreError::Unreadable`] when the JSON does not match, and
191 /// [`ScoreError::WrongSchema`] when it declares a different contract.
192 pub fn parse(text: &str) -> Result<Self, ScoreError> {
193 let run: Self = serde_json::from_str(text).map_err(|e| ScoreError::Unreadable {
194 message: e.to_string(),
195 })?;
196 if run.schema != RUN_SCHEMA {
197 return Err(ScoreError::WrongSchema { got: run.schema });
198 }
199 Ok(run)
200 }
201}
202
203/// Why a run could not be scored.
204#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)]
205pub enum ScoreError {
206 /// A finding, or an attempted sha, names a commit the corpus does not know.
207 ///
208 /// Almost always the merged PR head rather than the comment's
209 /// `original_commit_id` — the mistake that measures recall on already-fixed
210 /// code and reports zero without complaining. Refused rather than ignored.
211 #[error(
212 "{what} names commit {sha}, which is in no corpus row. The corpus is keyed \
213 by each comment's `reviewed_sha` (its `original_commit_id`); a merged PR \
214 head contains the fix commits, so scoring against one measures recall on \
215 code that is already repaired and silently reports zero"
216 )]
217 UnknownSha {
218 /// Which input named it (`a finding`, `attempted_shas`).
219 what: &'static str,
220 /// The offending commit.
221 sha: String,
222 },
223 /// A finding names a commit that the run did not declare as attempted.
224 #[error(
225 "a finding names commit {sha}, which is not in `attempted_shas` — the \
226 attempted set decides the denominator, so it must list every commit the \
227 candidate reviewed"
228 )]
229 UndeclaredSha {
230 /// The offending commit.
231 sha: String,
232 },
233 /// The run attempted no commit the corpus knows, so there is nothing to score.
234 #[error("the run attempted no commit, so there is nothing to score")]
235 NothingAttempted,
236 /// The run document is not a [`CandidateRun`].
237 #[error("not a `{RUN_SCHEMA}` document: {message}")]
238 Unreadable {
239 /// The `serde_json` message, which names the offending field.
240 message: String,
241 },
242 /// The document declares a schema this build does not implement.
243 #[error("run document declares schema {got:?}, but this build scores `{RUN_SCHEMA}`")]
244 WrongSchema {
245 /// The tag as declared.
246 got: String,
247 },
248}
249
250/// A real defect the candidate did not find, with enough to go and look at it.
251///
252/// The comment id alone would not do that: a score document is read by a person
253/// deciding what to improve, and "you missed 3789173576" sends them to grep the
254/// corpus. Carrying the anchor duplicates three fields out of the corpus, which is
255/// the right trade — a report that needs a second file opened to be actionable is
256/// a worse contract than a slightly larger one.
257#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
258pub struct Missed {
259 /// Comment id — the corpus primary key.
260 pub id: u64,
261 /// Path the missed defect is anchored to.
262 pub path: String,
263 /// Line in that file.
264 pub line: u32,
265 /// One line stating what the defect was.
266 pub description: String,
267 /// Permalink to the original comment.
268 pub comment_url: String,
269}
270
271/// Recall over one defect class, and the class-level detail an implementer needs.
272#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
273pub struct ClassRecall {
274 /// The class.
275 pub class: DefectClass,
276 /// Real rows of this class **within the attempted commits** — the denominator.
277 /// Read every rate against it: a `1` here is one bit of evidence, not a rate.
278 pub real: usize,
279 /// How many of those the candidate found.
280 pub found: usize,
281 /// Of the found ones, how many the candidate labelled as a different class.
282 /// Finding it still counts; the label is reported so a reviewer whose classes
283 /// are systematically wrong is visible.
284 pub misclassified: usize,
285 /// The real defects of this class the candidate missed — what to read next.
286 pub missed: Vec<Missed>,
287}
288
289impl ClassRecall {
290 /// Recall as a fraction, or `None` when the class has no real row within the
291 /// attempted commits (a class with an empty denominator has no recall, and
292 /// reporting `0.0` for it would read as a failure).
293 #[must_use]
294 #[expect(
295 clippy::cast_precision_loss,
296 reason = "counts here are corpus rows — 26 today, and a corpus large \
297 enough to lose f64 precision would have other problems"
298 )]
299 pub fn recall(&self) -> Option<f64> {
300 (self.real > 0).then(|| self.found as f64 / self.real as f64)
301 }
302}
303
304/// A candidate's score against the corpus.
305// `Eq` is not derived: `expected_by_position` is an `f64` estimate, and a score
306// that could be compared for exact equality would invite a test that pins a
307// float. `PartialEq` is enough for the equality the tests actually want.
308#[derive(Debug, Clone, PartialEq, Serialize)]
309pub struct Score {
310 /// Stable schema tag for `--json` consumers.
311 pub schema: &'static str,
312 /// How many corpus commits the run attempted.
313 pub attempted_shas: usize,
314 /// How many commits the corpus holds in total — so a partial run reads as
315 /// partial rather than as a poor one.
316 pub corpus_shas: usize,
317 /// Per class, in [`CLASSES`] order. Classes with no real row in the attempted
318 /// commits are present with `real: 0`, so the shape of the report does not
319 /// change with the run.
320 pub per_class: Vec<ClassRecall>,
321 /// Real rows found, across every class. A total, **not** a rate: the per-class
322 /// table is the result.
323 pub found: usize,
324 /// Real rows in scope.
325 pub real_in_scope: usize,
326 /// Of the corpus's known-false rows in scope, how many the candidate repeated.
327 /// The only measured precision signal the corpus licenses.
328 pub known_false_reproduced: usize,
329 /// Known-false rows in scope.
330 pub known_false_in_scope: usize,
331 /// Findings matching no row. **Not false positives** — unadjudicated. See the
332 /// module docs.
333 pub unadjudicated: usize,
334 /// Findings the candidate withheld that would have matched a **real** row —
335 /// the cost of the suppression filter, which must be reported rather than
336 /// hidden, since a filter that discards true findings is worse than none.
337 pub suppressed_real: usize,
338 /// Findings the candidate withheld that would have matched a **known-false**
339 /// row — the filter earning its keep.
340 pub suppressed_known_false: usize,
341 /// Findings withheld that match no row at all.
342 pub suppressed_unadjudicated: usize,
343 /// **How many real rows a candidate of this shape would match by position
344 /// alone** — the chance baseline [`Score::found`] has to beat.
345 ///
346 /// [`match_findings`] credits a finding to a row on `(sha, path, line within
347 /// LINE_WINDOW)` and **nothing else**: not the defect class, not a word of the
348 /// description. That is the right rule for a scorer that must not reward
349 /// eloquence, but it has a consequence nobody had measured. A reviewer that
350 /// emits enough findings per file blankets the diff, and then a "hit" is
351 /// explained by density rather than by insight — the recall figure looks like
352 /// a measurement and is arithmetic.
353 ///
354 /// Measured on this repository's own reviewer, that is not a hypothetical: at
355 /// **10.9 findings per file** the diff-only arm scored **4 of 22**, against a
356 /// permutation null — every row relocated to a random line its diff actually
357 /// shows, the findings left exactly as emitted — of **4.19**. It scored
358 /// *below* chance, and P(≥ observed) was 0.72.
359 ///
360 /// So this is reported beside the recall, always, in the same way
361 /// `reasoning_truncated` is reported beside a zero: a number whose null is not
362 /// stated is not yet a result.
363 ///
364 /// # An approximation, and it says so rather than implying precision
365 ///
366 /// **The exact null needs the diff** — which lines the reviewer was shown —
367 /// and scoring is pure by design, with no git and no network, so that any
368 /// machine can recompute a published score. So this uses the candidate's
369 /// **own findings** as the proxy for where it looked: on each file carrying a
370 /// row, the fraction of the line range those findings span that their merged
371 /// `LINE_WINDOW` neighbourhoods cover.
372 ///
373 /// That proxy is wrong in both directions at once. The findings' own span is
374 /// narrower than the diff's, which pushes the estimate up; ignoring the
375 /// one-to-one competition between two rows on a file also pushes it up; and
376 /// the reviewer's silence at the edges of a diff pushes it down. Calibrated
377 /// against an exact permutation on this repository's diff-only run — every row
378 /// relocated to a random line its diff actually shows — this reads **3.0**
379 /// where the permutation reads **4.19** against an observed **4**.
380 ///
381 /// So it is an order-of-magnitude guide, not a null, and
382 /// [`Score::caveats`] fires on a **margin** rather than a strict comparison
383 /// for exactly that reason. Anyone comparing two candidates seriously should
384 /// run the permutation, which needs the diff and therefore does not belong in
385 /// a pure scorer.
386 ///
387 /// `None` when the run offered no findings on any file carrying a row, since
388 /// there is then nothing whose density to describe.
389 pub expected_by_position: Option<f64>,
390}
391
392/// Schema tag for a serialised [`Score`].
393pub const SCORE_SCHEMA: &str = "roteiro.review-score/v1";
394
395impl Score {
396 /// Precision over the **adjudicated** findings only: matched-real ÷
397 /// (matched-real + reproduced-known-false).
398 ///
399 /// `None` when the candidate produced no adjudicated finding, rather than a
400 /// flattering `1.0` computed from nothing. This is not precision over the
401 /// candidate's output — see [`Score::unadjudicated`], which is the rest of it
402 /// and is not counted here because the corpus cannot judge it.
403 #[must_use]
404 #[expect(
405 clippy::cast_precision_loss,
406 reason = "counts here are corpus rows; see ClassRecall::recall"
407 )]
408 pub fn corpus_precision(&self) -> Option<f64> {
409 let adjudicated = self.found + self.known_false_reproduced;
410 (adjudicated > 0).then(|| self.found as f64 / adjudicated as f64)
411 }
412
413 /// The caveats that must accompany these numbers, as sentences a report
414 /// prints.
415 ///
416 /// Emitted with the score rather than left to a reader's memory, because every
417 /// one of them is a way the numbers can be honestly stated and dishonestly
418 /// read. A run that covered three commits out of thirteen says so; a class
419 /// whose denominator is one says so.
420 #[must_use]
421 pub fn caveats(&self) -> Vec<String> {
422 let mut out = Vec::new();
423 if self.attempted_shas < self.corpus_shas {
424 out.push(format!(
425 "partial run: {} of {} corpus commits attempted, so rows on the \
426 other {} are excluded from every denominator rather than counted \
427 as misses",
428 self.attempted_shas,
429 self.corpus_shas,
430 self.corpus_shas - self.attempted_shas
431 ));
432 }
433 let thin: Vec<&str> = self
434 .per_class
435 .iter()
436 .filter(|c| c.real == 1)
437 .map(|c| c.class.as_str())
438 .collect();
439 if !thin.is_empty() {
440 out.push(format!(
441 "{} class(es) have a single real row ({}), so their recall is one \
442 bit rather than a rate and should not be compared as a percentage",
443 thin.len(),
444 thin.join(", ")
445 ));
446 }
447 // A margin rather than a strict comparison, because the baseline is an
448 // approximation that misses in both directions — see the field's docs. A
449 // result only a little above an uncertain null is exactly the case a
450 // reader needs warning about, so the guard is deliberately loose.
451 #[expect(
452 clippy::cast_precision_loss,
453 reason = "a count of corpus rows found; see ClassRecall::recall"
454 )]
455 let found = self.found as f64;
456 if let Some(expected) = self.expected_by_position
457 && found <= expected * 2.0
458 {
459 out.push(format!(
460 "RECALL IS NOT CLEARLY ABOVE CHANCE AT THIS FINDING DENSITY: a \
461 candidate emitting these findings in these places would match \
462 ~{expected:.1} real row(s) by position alone, and this one matched \
463 {}. Scoring credits a finding to a row on (commit, path, line \
464 \u{b1}{}) and NEVER on what the finding says, so a reviewer dense \
465 enough to blanket a diff scores recall it did not earn. That \
466 baseline is approximate; confirm with a permutation null before \
467 comparing two candidates, and lower the finding rate first",
468 self.found, LINE_WINDOW
469 ));
470 }
471 if self.unadjudicated > 0 {
472 out.push(format!(
473 "{} finding(s) match no corpus row. These are UNADJUDICATED, not \
474 false positives — the corpus records what one reviewer said about \
475 these trees, not every defect in them. They become a precision \
476 figure only once a human adjudicates them and the rows are added",
477 self.unadjudicated
478 ));
479 }
480 if self.suppressed_real > 0 {
481 out.push(format!(
482 "the suppression filter withheld {} finding(s) that match a REAL \
483 row — it is discarding true findings and its licence (zero cost \
484 on this corpus) no longer holds",
485 self.suppressed_real
486 ));
487 }
488 out
489 }
490}
491
492/// Score `run` against `corpus`.
493///
494/// Matching is **one to one**: each row is credited to at most one finding and
495/// each finding to at most one row, resolved by nearest line and then by lowest
496/// comment id, so the result does not depend on the order the candidate emitted
497/// its findings in.
498///
499/// # Errors
500/// [`ScoreError::UnknownSha`] when a sha is in no corpus row (the PR-head
501/// mistake), [`ScoreError::UndeclaredSha`] when a finding is outside the attempted
502/// set, and [`ScoreError::NothingAttempted`] for an empty run.
503pub fn score(corpus: &Corpus, run: &CandidateRun) -> Result<Score, ScoreError> {
504 let known: BTreeSet<&str> = corpus.reviewed_shas();
505 if run.attempted_shas.is_empty() {
506 return Err(ScoreError::NothingAttempted);
507 }
508 for sha in &run.attempted_shas {
509 if !known.contains(sha.as_str()) {
510 return Err(ScoreError::UnknownSha {
511 what: "attempted_shas",
512 sha: sha.clone(),
513 });
514 }
515 }
516 for finding in run.findings.iter().chain(&run.suppressed) {
517 if !known.contains(finding.reviewed_sha.as_str()) {
518 return Err(ScoreError::UnknownSha {
519 what: "a finding",
520 sha: finding.reviewed_sha.clone(),
521 });
522 }
523 if !run.attempted_shas.contains(&finding.reviewed_sha) {
524 return Err(ScoreError::UndeclaredSha {
525 sha: finding.reviewed_sha.clone(),
526 });
527 }
528 }
529
530 let in_scope: Vec<&CorpusRow> = corpus
531 .rows()
532 .iter()
533 .filter(|r| run.attempted_shas.contains(&r.reviewed_sha))
534 .collect();
535
536 let emitted = match_findings(&in_scope, &run.findings);
537 let withheld = match_findings(&in_scope, &run.suppressed);
538
539 let mut per_class: Vec<ClassRecall> = Vec::with_capacity(CLASSES.len());
540 for class in CLASSES {
541 let rows: Vec<&&CorpusRow> = in_scope
542 .iter()
543 .filter(|r| r.defect_class == class && r.verdict == Verdict::Real)
544 .collect();
545 let mut found = 0;
546 let mut misclassified = 0;
547 let mut missed = Vec::new();
548 for row in &rows {
549 match emitted.by_row.get(&row.id) {
550 Some(finding) => {
551 found += 1;
552 if finding.defect_class.is_some_and(|c| c != class) {
553 misclassified += 1;
554 }
555 }
556 None => missed.push(Missed {
557 id: row.id,
558 path: row.path.clone(),
559 line: row.line,
560 description: row.description.clone(),
561 comment_url: row.comment_url.clone(),
562 }),
563 }
564 }
565 per_class.push(ClassRecall {
566 class,
567 real: rows.len(),
568 found,
569 misclassified,
570 missed,
571 });
572 }
573
574 let real_in_scope = in_scope
575 .iter()
576 .filter(|r| r.verdict == Verdict::Real)
577 .count();
578 let known_false_in_scope = in_scope
579 .iter()
580 .filter(|r| r.verdict == Verdict::False)
581 .count();
582 // Verdict by row id, so counting matches is a lookup rather than a scan — and
583 // so a match against an id somehow outside scope is simply not counted rather
584 // than a panic. Matching only ever draws from `in_scope`, so the two agree;
585 // this shape means a future change that broke that would produce a low count
586 // instead of a crash in a scorer.
587 let verdicts: BTreeMap<u64, Verdict> = in_scope.iter().map(|r| (r.id, r.verdict)).collect();
588 let count_by_verdict = |m: &Matched, want: Verdict| {
589 m.by_row
590 .keys()
591 .filter(|id| verdicts.get(id) == Some(&want))
592 .count()
593 };
594
595 Ok(Score {
596 schema: SCORE_SCHEMA,
597 attempted_shas: run.attempted_shas.len(),
598 corpus_shas: known.len(),
599 per_class,
600 found: count_by_verdict(&emitted, Verdict::Real),
601 real_in_scope,
602 known_false_reproduced: count_by_verdict(&emitted, Verdict::False),
603 known_false_in_scope,
604 unadjudicated: run.findings.len() - emitted.by_row.len(),
605 suppressed_real: count_by_verdict(&withheld, Verdict::Real),
606 suppressed_known_false: count_by_verdict(&withheld, Verdict::False),
607 suppressed_unadjudicated: run.suppressed.len() - withheld.by_row.len(),
608 expected_by_position: expected_by_position(&in_scope, &run.findings),
609 })
610}
611
612/// The chance baseline described on [`Score::expected_by_position`].
613///
614/// For each **real** row, the probability that a row dropped uniformly across the
615/// span its file's findings cover would land within [`LINE_WINDOW`] of at least
616/// one of them, capped at 1. Summed, that is how many rows a candidate of this
617/// shape matches without knowing anything.
618#[expect(
619 clippy::cast_precision_loss,
620 reason = "line numbers and finding counts on one file; a file long enough to lose f64 precision is not reviewable at all"
621)]
622fn expected_by_position(rows: &[&CorpusRow], findings: &[CandidateFinding]) -> Option<f64> {
623 let mut by_file: BTreeMap<(&str, &str), Vec<u32>> = BTreeMap::new();
624 for f in findings {
625 by_file
626 .entry((f.reviewed_sha.as_str(), f.path.as_str()))
627 .or_default()
628 .push(f.line);
629 }
630 let mut total = 0.0;
631 let mut any = false;
632 for row in rows.iter().filter(|r| r.verdict == Verdict::Real) {
633 let Some(lines) = by_file.get(&(row.reviewed_sha.as_str(), row.path.as_str())) else {
634 continue;
635 };
636 any = true;
637 let (lo, hi) = (
638 lines.iter().copied().min().unwrap_or(0),
639 lines.iter().copied().max().unwrap_or(0),
640 );
641 // The span the candidate's own attention covered. A single finding spans
642 // one line, so the window itself is the whole space and the row is certain
643 // to match — which is correct: a file the candidate commented on once, at
644 // one point, offers a row nowhere else to be.
645 let span = f64::from(hi - lo + 1);
646 // **Merged, not summed.** At ten findings a file the ±10 windows overlap
647 // heavily, and counting each one whole inflates the baseline by about half
648 // — measured on this repository, 6.2 against a permutation's 4.19. The
649 // union is what a row can actually land in.
650 let mut sorted = lines.clone();
651 sorted.sort_unstable();
652 let mut covered = 0u64;
653 let mut open: Option<(u32, u32)> = None;
654 for line in sorted {
655 let (start, end) = (line.saturating_sub(LINE_WINDOW), line + LINE_WINDOW);
656 match open {
657 Some((s, e)) if start <= e + 1 => open = Some((s, e.max(end))),
658 Some((s, e)) => {
659 covered += u64::from(e - s + 1);
660 open = Some((start, end));
661 }
662 None => open = Some((start, end)),
663 }
664 }
665 if let Some((s, e)) = open {
666 covered += u64::from(e - s + 1);
667 }
668 let reach = covered as f64;
669 total += (reach / span).min(1.0);
670 }
671 any.then_some(total)
672}
673
674/// The outcome of matching: row id → the finding credited to it.
675struct Matched<'a> {
676 by_row: BTreeMap<u64, &'a CandidateFinding>,
677}
678
679/// Credit findings to rows, one to one.
680///
681/// Candidate pairs are ranked by line distance, then row id, then the finding's
682/// own line — a total order over the pairs, so the greedy pass is deterministic
683/// and independent of input order.
684fn match_findings<'a>(rows: &[&'a CorpusRow], findings: &'a [CandidateFinding]) -> Matched<'a> {
685 let mut pairs: Vec<(u32, u64, u32, usize)> = Vec::new();
686 for (idx, finding) in findings.iter().enumerate() {
687 for row in rows {
688 if row.reviewed_sha != finding.reviewed_sha || row.path != finding.path {
689 continue;
690 }
691 let distance = row.line.abs_diff(finding.line);
692 if distance <= LINE_WINDOW {
693 pairs.push((distance, row.id, finding.line, idx));
694 }
695 }
696 }
697 pairs.sort_unstable();
698
699 let mut by_row: BTreeMap<u64, &CandidateFinding> = BTreeMap::new();
700 let mut used: BTreeSet<usize> = BTreeSet::new();
701 for (_, row_id, _, idx) in pairs {
702 if by_row.contains_key(&row_id) || used.contains(&idx) {
703 continue;
704 }
705 by_row.insert(row_id, &findings[idx]);
706 used.insert(idx);
707 }
708 Matched { by_row }
709}
710
711#[cfg(test)]
712mod tests {
713 use super::{CandidateFinding, CandidateRun, LINE_WINDOW, SCORE_SCHEMA, ScoreError, score};
714 use crate::review_corpus::{Corpus, DefectClass, Verdict};
715
716 const SHA_A: &str = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
717 const SHA_B: &str = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb";
718
719 /// A corpus row as JSONL.
720 fn row(id: u64, sha: &str, path: &str, line: u32, verdict: &str, class: &str) -> String {
721 format!(
722 "{{\"id\": {id}, \"pr\": 300, \"reviewer\": \"github-copilot\", \
723 \"reviewed_sha\": {sha:?}, \"path\": {path:?}, \"line\": {line}, \
724 \"verdict\": {verdict:?}, \"defect_class\": {class:?}, \
725 \"fix_commit\": \"\", \"description\": \"d\", \
726 \"comment_url\": \"https://example.invalid/{id}\"}}"
727 )
728 }
729
730 /// Two commits, five rows: three real of two classes on `SHA_A`, one real and
731 /// one known-false on `SHA_B`.
732 fn corpus() -> Corpus {
733 let text = [
734 row(1, SHA_A, "src/a.rs", 100, "real", "contract-drift"),
735 row(2, SHA_A, "src/a.rs", 200, "real", "contract-drift"),
736 row(3, SHA_A, "src/b.rs", 10, "real", "vacuous-test"),
737 row(4, SHA_B, "src/c.rs", 50, "real", "ordering-bug"),
738 row(5, SHA_B, "src/c.rs", 300, "false", "false-compile-claim"),
739 ]
740 .join("\n");
741 Corpus::parse(&text).expect("the test corpus parses")
742 }
743
744 fn finding(sha: &str, path: &str, line: u32) -> CandidateFinding {
745 CandidateFinding {
746 reviewed_sha: sha.to_owned(),
747 path: path.to_owned(),
748 line,
749 description: "a finding".to_owned(),
750 claims_compile_failure: false,
751 defect_class: None,
752 }
753 }
754
755 /// **A blanketing reviewer must not be credited with recall it did not earn.**
756 ///
757 /// Matching consults `(sha, path, line ±LINE_WINDOW)` and never the text, so a
758 /// candidate that comments densely enough across a file matches rows by
759 /// position. Row 1 sits at line 100; these findings never mention it and are
760 /// spread every few lines around it, so every one of them is "right" by
761 /// arithmetic alone. The chance baseline has to see that.
762 #[test]
763 fn a_blanketing_candidate_is_flagged_as_not_clearly_above_chance() {
764 let dense: Vec<CandidateFinding> = (0..12)
765 .map(|i| finding(SHA_A, "src/a.rs", 90 + i * 3))
766 .collect();
767 let scored = score(&corpus(), &run(&[SHA_A, SHA_B], dense)).expect("scores");
768 let expected = scored
769 .expected_by_position
770 .expect("findings landed on a file carrying a row");
771 assert!(
772 expected > 0.5,
773 "a candidate blanketing a row's file scored a chance baseline of only \
774 {expected}"
775 );
776 assert!(
777 scored
778 .caveats()
779 .iter()
780 .any(|c| c.contains("NOT CLEARLY ABOVE CHANCE")),
781 "the density caveat did not fire: {:?}",
782 scored.caveats()
783 );
784 }
785
786 /// The mirror image: one precise finding on a file, nowhere near a row, is not
787 /// a blanket — and a candidate that then matches nothing must not be told its
788 /// zero is a density artefact.
789 #[test]
790 fn a_sparse_candidate_that_misses_is_not_blamed_on_density() {
791 let sparse = vec![finding(SHA_A, "src/a.rs", 100)];
792 let scored = score(&corpus(), &run(&[SHA_A, SHA_B], sparse)).expect("scores");
793 // One finding spans one line, so a row has nowhere else to be and the
794 // baseline is 1.0 for that row — correct, and the documented behaviour.
795 assert_eq!(scored.found, 1);
796 assert!(scored.expected_by_position.is_some());
797 }
798
799 /// A run whose findings never touch a file carrying a row has no density to
800 /// describe, and must report `None` rather than a flattering zero that would
801 /// read as "comfortably above chance".
802 #[test]
803 fn a_run_touching_no_anchored_file_has_no_chance_baseline() {
804 let elsewhere = vec![finding(SHA_A, "src/nowhere.rs", 10)];
805 let scored = score(&corpus(), &run(&[SHA_A, SHA_B], elsewhere)).expect("scores");
806 assert_eq!(scored.expected_by_position, None);
807 assert!(
808 !scored
809 .caveats()
810 .iter()
811 .any(|c| c.contains("NOT CLEARLY ABOVE CHANCE")),
812 "a caveat about density fired with no findings to be dense"
813 );
814 }
815
816 /// **Overlapping windows are merged, not summed.** Ten findings within a few
817 /// lines of each other reach barely further than one does; counting each
818 /// `±LINE_WINDOW` span whole would inflate the baseline by about half, which
819 /// is how the first version of this read 6.2 where a permutation read 4.19.
820 #[test]
821 fn overlapping_windows_count_once() {
822 // Ten findings packed into 10 lines, plus one far away so the span is wide
823 // enough for the difference to show. Merged, the reach is [90,119] plus
824 // [990,1010] = 51 lines of a 911-line span, so each of the two rows on
825 // this file scores ~0.06. Summed it would be 11 x 21 = 231 lines, ~0.25
826 // each — four times larger, and the direction that hides a real result.
827 let mut clustered: Vec<CandidateFinding> = (0..10)
828 .map(|i| finding(SHA_A, "src/a.rs", 100 + i))
829 .collect();
830 clustered.push(finding(SHA_A, "src/a.rs", 1_000));
831 let scored = score(&corpus(), &run(&[SHA_A, SHA_B], clustered)).expect("scores");
832 let expected = scored.expected_by_position.expect("some");
833 assert!(
834 expected < 0.25,
835 "overlapping windows were summed rather than merged: {expected}"
836 );
837 }
838
839 fn run(shas: &[&str], findings: Vec<CandidateFinding>) -> CandidateRun {
840 CandidateRun {
841 attempted_shas: shas.iter().map(|s| (*s).to_owned()).collect(),
842 findings,
843 ..CandidateRun::default()
844 }
845 }
846
847 /// A perfect run on one commit: every real row of that commit found, per class.
848 #[test]
849 fn a_found_row_counts_in_its_own_class() {
850 let scored = score(
851 &corpus(),
852 &run(
853 &[SHA_A],
854 vec![
855 finding(SHA_A, "src/a.rs", 100),
856 finding(SHA_A, "src/a.rs", 200),
857 finding(SHA_A, "src/b.rs", 10),
858 ],
859 ),
860 )
861 .expect("scores");
862 assert_eq!(scored.schema, SCORE_SCHEMA);
863 assert_eq!(scored.found, 3);
864 assert_eq!(scored.real_in_scope, 3);
865 let drift = scored
866 .per_class
867 .iter()
868 .find(|c| c.class == DefectClass::ContractDrift)
869 .expect("every class is present");
870 assert_eq!((drift.real, drift.found), (2, 2));
871 assert_eq!(drift.recall(), Some(1.0));
872 // A class with no row in scope has no recall, rather than 0.0 — which
873 // would read as a failure to find something that was not there.
874 let cleanup = scored
875 .per_class
876 .iter()
877 .find(|c| c.class == DefectClass::CleanupGap)
878 .expect("present with real: 0");
879 assert_eq!(cleanup.real, 0);
880 assert_eq!(cleanup.recall(), None);
881 }
882
883 /// **A partial run must not look like a bad one.** Attempting one of two
884 /// commits excludes the other's rows from the denominator, and the caveat says
885 /// so.
886 #[test]
887 fn rows_outside_the_attempted_commits_are_out_of_scope_not_missed() {
888 let scored = score(&corpus(), &run(&[SHA_A], vec![])).expect("scores");
889 assert_eq!(scored.real_in_scope, 3, "only SHA_A's real rows");
890 assert_eq!(scored.found, 0);
891 assert_eq!(scored.known_false_in_scope, 0, "the false row is on SHA_B");
892 assert_eq!((scored.attempted_shas, scored.corpus_shas), (1, 2));
893 assert!(
894 scored.caveats().iter().any(|c| c.contains("partial run")),
895 "{:?}",
896 scored.caveats()
897 );
898 }
899
900 /// **The most expensive available mistake**, refused rather than scored: a run
901 /// against a merged PR head names a commit the corpus does not know, and the
902 /// error explains why the number would have been zero.
903 #[test]
904 fn a_sha_the_corpus_does_not_know_is_refused_with_the_reason() {
905 let head = "cccccccccccccccccccccccccccccccccccccccc";
906 let err = score(&corpus(), &run(&[head], vec![])).expect_err("not a corpus commit");
907 let ScoreError::UnknownSha { what, .. } = err else {
908 panic!("expected UnknownSha, got {err:?}");
909 };
910 assert_eq!(what, "attempted_shas");
911 let text = err.to_string();
912 assert!(text.contains("reviewed_sha"), "{text}");
913 assert!(
914 text.contains("fix commits") && text.contains("silently reports zero"),
915 "says what goes wrong, not just that it did: {text}"
916 );
917
918 // And via a finding, which is the other way it arrives.
919 let mut r = run(&[SHA_A], vec![finding(head, "src/a.rs", 100)]);
920 r.attempted_shas.insert(SHA_A.to_owned());
921 let err = score(&corpus(), &r).expect_err("a finding on an unknown commit");
922 assert!(
923 matches!(
924 err,
925 ScoreError::UnknownSha {
926 what: "a finding",
927 ..
928 }
929 ),
930 "{err:?}"
931 );
932 }
933
934 /// A finding on a commit the run did not declare is refused: the attempted set
935 /// is the denominator, so it has to be complete.
936 #[test]
937 fn a_finding_outside_the_attempted_set_is_refused() {
938 let err = score(
939 &corpus(),
940 &run(&[SHA_A], vec![finding(SHA_B, "src/c.rs", 50)]),
941 )
942 .expect_err("SHA_B was not attempted");
943 assert!(matches!(err, ScoreError::UndeclaredSha { .. }), "{err:?}");
944 }
945
946 #[test]
947 fn an_empty_run_is_refused_rather_than_scored_as_zero() {
948 let err = score(&corpus(), &CandidateRun::default()).expect_err("nothing attempted");
949 assert!(matches!(err, ScoreError::NothingAttempted), "{err:?}");
950 }
951
952 /// A finding matching no row is **unadjudicated**, never a false positive: the
953 /// corpus records what one reviewer said, not every defect in the tree.
954 #[test]
955 fn an_unmatched_finding_is_unadjudicated_not_false() {
956 let scored = score(
957 &corpus(),
958 &run(&[SHA_A], vec![finding(SHA_A, "src/z.rs", 7)]),
959 )
960 .expect("scores");
961 assert_eq!(scored.unadjudicated, 1);
962 assert_eq!(scored.known_false_reproduced, 0);
963 assert_eq!(scored.found, 0);
964 assert_eq!(
965 scored.corpus_precision(),
966 None,
967 "no adjudicated finding means no precision, not 1.0 and not 0.0"
968 );
969 let caveat = scored.caveats().join(" ");
970 assert!(caveat.contains("UNADJUDICATED"), "{caveat}");
971 assert!(
972 caveat.contains("not every defect in them"),
973 "says why it is not precision: {caveat}"
974 );
975 }
976
977 /// Repeating a known-false claim is the one precision signal the corpus
978 /// licenses, and it lands in the precision denominator.
979 #[test]
980 fn reproducing_a_known_false_row_costs_precision() {
981 let scored = score(
982 &corpus(),
983 &run(
984 &[SHA_B],
985 vec![
986 finding(SHA_B, "src/c.rs", 50), // the real row
987 finding(SHA_B, "src/c.rs", 300), // the known-false one
988 ],
989 ),
990 )
991 .expect("scores");
992 assert_eq!((scored.found, scored.known_false_reproduced), (1, 1));
993 assert_eq!(scored.corpus_precision(), Some(0.5));
994 assert_eq!(scored.unadjudicated, 0);
995 }
996
997 /// Matching tolerates a slightly-off line — a reviewer's cited line can be
998 /// wrong while its point is right — but not an arbitrary one.
999 #[test]
1000 fn matching_tolerates_a_near_miss_but_not_a_far_one() {
1001 let near = score(
1002 &corpus(),
1003 &run(
1004 &[SHA_A],
1005 vec![finding(SHA_A, "src/a.rs", 100 + LINE_WINDOW)],
1006 ),
1007 )
1008 .expect("scores");
1009 assert_eq!(near.found, 1, "at the window edge");
1010
1011 let far = score(
1012 &corpus(),
1013 &run(
1014 &[SHA_A],
1015 vec![finding(SHA_A, "src/a.rs", 100 + LINE_WINDOW + 1)],
1016 ),
1017 )
1018 .expect("scores");
1019 assert_eq!(far.found, 0, "one line past the window");
1020 assert_eq!(far.unadjudicated, 1);
1021 }
1022
1023 /// **The window has to bound something.** The three `vacuous-test` rows of
1024 /// #299 sit 50 lines apart in one file — a reviewer that commented once,
1025 /// anywhere in that file, must be credited with the row it is near and with
1026 /// none of the others.
1027 ///
1028 /// Written against a finding placed *between* two rows and near neither,
1029 /// because that is the case an unbounded window gets wrong. A finding placed
1030 /// exactly on a row would still credit one row without a window at all, since
1031 /// matching is one-to-one and nearest-first — which is a different property,
1032 /// tested below.
1033 #[test]
1034 fn the_window_bounds_which_row_a_distant_finding_can_claim() {
1035 let text = [
1036 row(1, SHA_A, "tests/t.rs", 75, "real", "vacuous-test"),
1037 row(2, SHA_A, "tests/t.rs", 125, "real", "vacuous-test"),
1038 row(3, SHA_A, "tests/t.rs", 176, "real", "vacuous-test"),
1039 ]
1040 .join("\n");
1041 let corpus = Corpus::parse(&text).expect("parses");
1042 // The real gaps: no window this size can bridge them.
1043 const { assert!(LINE_WINDOW * 2 < 50, "the window would span two #299 rows") };
1044
1045 // Line 100: 25 from row 1 and 25 from row 2, so outside both windows. A
1046 // reviewer that commented here found none of the three.
1047 let scored = score(
1048 &corpus,
1049 &run(&[SHA_A], vec![finding(SHA_A, "tests/t.rs", 100)]),
1050 )
1051 .expect("scores");
1052 assert_eq!(
1053 scored.found, 0,
1054 "a finding 25 lines from the nearest row has not found it"
1055 );
1056 assert_eq!(scored.unadjudicated, 1);
1057 let vacuous = scored
1058 .per_class
1059 .iter()
1060 .find(|c| c.class == DefectClass::VacuousTest)
1061 .expect("present");
1062 let missed: Vec<u64> = vacuous.missed.iter().map(|m| m.id).collect();
1063 assert_eq!(missed, vec![1, 2, 3], "names what to read next");
1064 assert!(
1065 vacuous
1066 .missed
1067 .iter()
1068 .all(|m| !m.comment_url.is_empty() && m.line > 0),
1069 "a miss carries enough to go and look at it, not just an id"
1070 );
1071
1072 // Three findings, one on each row, are credited to all three — the window
1073 // is a bound, not an obstacle.
1074 let all = score(
1075 &corpus,
1076 &run(
1077 &[SHA_A],
1078 vec![
1079 finding(SHA_A, "tests/t.rs", 75),
1080 finding(SHA_A, "tests/t.rs", 125),
1081 finding(SHA_A, "tests/t.rs", 176),
1082 ],
1083 ),
1084 )
1085 .expect("scores");
1086 assert_eq!(all.found, 3);
1087 }
1088
1089 /// **One finding cannot be credited to two rows**, so a comment sitting between
1090 /// two nearby defects counts as finding one of them, not both.
1091 ///
1092 /// The rows here are 5 apart, inside one window — the only arrangement in which
1093 /// one-to-one matching is distinguishable from crediting every row in range.
1094 #[test]
1095 fn one_finding_cannot_claim_two_rows_in_the_same_window() {
1096 let text = [
1097 row(1, SHA_A, "src/a.rs", 100, "real", "contract-drift"),
1098 row(2, SHA_A, "src/a.rs", 105, "real", "contract-drift"),
1099 ]
1100 .join("\n");
1101 let corpus = Corpus::parse(&text).expect("parses");
1102 let scored = score(
1103 &corpus,
1104 &run(&[SHA_A], vec![finding(SHA_A, "src/a.rs", 102)]),
1105 )
1106 .expect("scores");
1107 assert_eq!(
1108 scored.found, 1,
1109 "one comment is one finding, however many rows it is near"
1110 );
1111 let drift = scored
1112 .per_class
1113 .iter()
1114 .find(|c| c.class == DefectClass::ContractDrift)
1115 .expect("present");
1116 assert_eq!((drift.real, drift.found), (2, 1));
1117 // Nearest wins: 102 is 2 from row 1 and 3 from row 2.
1118 assert_eq!(
1119 drift.missed.iter().map(|m| m.id).collect::<Vec<_>>(),
1120 vec![2]
1121 );
1122 }
1123
1124 /// And the converse: two rows cannot share one finding, so spraying findings at
1125 /// a line cannot inflate recall past the number of rows there.
1126 #[test]
1127 fn extra_findings_in_one_window_do_not_inflate_recall() {
1128 let scored = score(
1129 &corpus(),
1130 &run(
1131 &[SHA_A],
1132 vec![
1133 finding(SHA_A, "src/a.rs", 98),
1134 finding(SHA_A, "src/a.rs", 100),
1135 finding(SHA_A, "src/a.rs", 102),
1136 ],
1137 ),
1138 )
1139 .expect("scores");
1140 assert_eq!(scored.found, 1, "one row, so one credit");
1141 assert_eq!(scored.unadjudicated, 2);
1142 }
1143
1144 /// The score does not depend on the order findings arrive in — a reviewer that
1145 /// emits its findings in a different order must score identically.
1146 #[test]
1147 fn the_score_is_independent_of_finding_order() {
1148 let findings = vec![
1149 finding(SHA_A, "src/a.rs", 98),
1150 finding(SHA_A, "src/a.rs", 205),
1151 finding(SHA_A, "src/b.rs", 10),
1152 ];
1153 let forward = score(&corpus(), &run(&[SHA_A], findings.clone())).expect("scores");
1154 let mut reversed = findings;
1155 reversed.reverse();
1156 let backward = score(&corpus(), &run(&[SHA_A], reversed)).expect("scores");
1157 assert_eq!(forward, backward);
1158 assert_eq!(forward.found, 3);
1159 }
1160
1161 /// Finding the defect and mislabelling it still counts as found — recall is
1162 /// about the defect, not the taxonomy — but the disagreement is reported.
1163 #[test]
1164 fn a_misclassified_finding_still_counts_as_found() {
1165 let mut f = finding(SHA_A, "src/b.rs", 10);
1166 f.defect_class = Some(DefectClass::ProseClarity); // the row is vacuous-test
1167 let scored = score(&corpus(), &run(&[SHA_A], vec![f])).expect("scores");
1168 let vacuous = scored
1169 .per_class
1170 .iter()
1171 .find(|c| c.class == DefectClass::VacuousTest)
1172 .expect("present");
1173 assert_eq!(
1174 (vacuous.real, vacuous.found, vacuous.misclassified),
1175 (1, 1, 1)
1176 );
1177 }
1178
1179 /// **The suppression filter's own cost, measured.** A withheld finding that
1180 /// would have matched a known-false row is the filter working; one that would
1181 /// have matched a real row is the filter breaking, and it must show up as a
1182 /// caveat rather than as an unexplained miss.
1183 #[test]
1184 fn suppressed_findings_are_scored_separately_and_a_true_one_raises_a_caveat() {
1185 let mut good = CandidateRun {
1186 attempted_shas: [SHA_B.to_owned()].into_iter().collect(),
1187 suppressed: vec![finding(SHA_B, "src/c.rs", 300)],
1188 ..CandidateRun::default()
1189 };
1190 let scored = score(&corpus(), &good).expect("scores");
1191 assert_eq!(scored.suppressed_known_false, 1);
1192 assert_eq!(scored.suppressed_real, 0);
1193 assert_eq!(
1194 scored.known_false_reproduced, 0,
1195 "withheld, so not reproduced"
1196 );
1197 assert!(
1198 !scored.caveats().iter().any(|c| c.contains("REAL row")),
1199 "nothing true was withheld: {:?}",
1200 scored.caveats()
1201 );
1202
1203 good.suppressed.push(finding(SHA_B, "src/c.rs", 50));
1204 let bad = score(&corpus(), &good).expect("scores");
1205 assert_eq!(bad.suppressed_real, 1);
1206 let caveat = bad.caveats().join(" ");
1207 assert!(
1208 caveat.contains("REAL row") && caveat.contains("licence"),
1209 "says the filter's licence no longer holds: {caveat}"
1210 );
1211 }
1212
1213 /// A withheld finding is not counted twice: it is not in `unadjudicated`,
1214 /// which counts emitted findings only.
1215 #[test]
1216 fn a_withheld_finding_is_not_an_unadjudicated_emitted_one() {
1217 let scored = score(
1218 &corpus(),
1219 &CandidateRun {
1220 attempted_shas: [SHA_A.to_owned()].into_iter().collect(),
1221 suppressed: vec![finding(SHA_A, "src/z.rs", 7)],
1222 ..CandidateRun::default()
1223 },
1224 )
1225 .expect("scores");
1226 assert_eq!(scored.unadjudicated, 0);
1227 assert_eq!(scored.suppressed_unadjudicated, 1);
1228 }
1229
1230 /// Every class appears in the report, in a fixed order, whatever the run
1231 /// covered — so two reports can be read side by side.
1232 #[test]
1233 fn the_report_shape_does_not_change_with_the_run() {
1234 let scored = score(&corpus(), &run(&[SHA_A], vec![])).expect("scores");
1235 let classes: Vec<_> = scored.per_class.iter().map(|c| c.class).collect();
1236 assert_eq!(classes, crate::review_corpus::CLASSES.to_vec());
1237 }
1238
1239 /// The known-false denominator follows scope too: a run that never saw the
1240 /// commit carrying the false row cannot be credited for avoiding it.
1241 #[test]
1242 fn avoiding_a_false_row_out_of_scope_is_not_a_credit() {
1243 let scored = score(&corpus(), &run(&[SHA_A], vec![])).expect("scores");
1244 assert_eq!(scored.known_false_in_scope, 0);
1245 let with_b = score(&corpus(), &run(&[SHA_A, SHA_B], vec![])).expect("scores");
1246 assert_eq!(with_b.known_false_in_scope, 1);
1247 assert_eq!(with_b.known_false_reproduced, 0);
1248 }
1249
1250 /// The corpus's own verdict vocabulary is what scoping splits on, so a row
1251 /// whose verdict changed would move between the two denominators rather than
1252 /// vanish.
1253 #[test]
1254 fn scope_splits_on_verdict_exhaustively() {
1255 let scored = score(&corpus(), &run(&[SHA_A, SHA_B], vec![])).expect("scores");
1256 assert_eq!(
1257 scored.real_in_scope + scored.known_false_in_scope,
1258 corpus().rows().len(),
1259 "every in-scope row is in exactly one denominator"
1260 );
1261 assert_eq!(
1262 corpus().with_verdict(Verdict::False).count(),
1263 scored.known_false_in_scope
1264 );
1265 }
1266}