Skip to main content

rto_graph/
reviewer.rs

1//! The reviewer's pure core: what to ask, how to read the answer, and what the
2//! answer is allowed to claim (Stage 35b).
3//!
4//! [`crate::review_score`] built the instrument; this is the thing it measures.
5//! Everything here is a pure function of bytes — prompt assembly, response
6//! parsing, budget arithmetic and the [`crate::compile_claim`] site derivation —
7//! so the whole of the reviewer's *judgement* is testable offline, with no model,
8//! no network and no git. What is left outside is a loop that calls an engine,
9//! and that lives in the binary.
10//!
11//! # Per file, and the budget is not the constraint
12//!
13//! 35a established that whole-diff review is not the shape: reconstructing the 15
14//! review diffs costs ~513k tokens, ~34k mean, and **9 of 15 exceed the ~30k
15//! single-call budget**. What that argument left open is how much room *per-file*
16//! review actually has, and the answer is: a great deal. Measured over all **190
17//! file-diffs** in the corpus:
18//!
19//! | | raw | annotated |
20//! |---|---|---|
21//! | mean | 2,704 | 3,275 |
22//! | median | 1,476 | 1,758 |
23//! | p90 | 5,621 | 6,711 |
24//!
25//! The second column is what a review actually sends, because [`annotate_diff`]
26//! adds a line-number column. That costs a measured **1.21×** over the corpus —
27//! and the shape of the cost is worth knowing: it is 9 characters per *line*, so
28//! it is ~1.2× on ordinary source and would be ~4× on a diff of two-character
29//! lines. It is charged against the budget rather than estimated around.
30//!
31//! Even so, exactly **one of the 190** exceeds the single-call budget annotated,
32//! and it is a generated JSON fixture; the next largest is `Cargo.lock`. **The
33//! largest reviewable *source* file-diff in the entire corpus is 14,034 tokens
34//! raw and 17,202 annotated** — still under two-thirds of the single-call budget.
35//! So the median file leaves ~28k of that budget unused and the worst source file
36//! leaves ~12k.
37//!
38//! That matters for one reason. The stage's central claim is that pre-assembled
39//! graph context lets a per-file reviewer see a doc in *another* file
40//! contradicting the code under review — the `contract-drift` class. Had the
41//! per-file budget been tight, that claim would have been untestable on this
42//! repository whatever the graph contained. It is not tight. [`GraphContext`] is
43//! the slot that headroom is for, and this module reserves it while shipping it
44//! empty: PR 1 measures the diff-only arm, and a filled slot is the comparison.
45//!
46//! # The prompt is derived from the standards, not from the corpus
47//!
48//! [`build_prompt`] states the house's review standards — contract accuracy, the
49//! defect vocabulary, the output shape — from `docs/REVIEW_CHECKLIST.md` and
50//! [`crate::review_corpus::DefectClass`], both of which predate it. It is
51//! deliberately **not** written against the corpus rows.
52//!
53//! This is a property of the experiment rather than a style preference. A prompt
54//! tuned until the known rows are found measures how well it was tuned, and the
55//! resulting recall would not survive the 23rd row. The rows are the test set and
56//! nothing here may read them, which is why this module depends on `DefectClass`
57//! and not on [`crate::review_corpus::BUILTIN`].
58//!
59//! # Nothing here decides what is true
60//!
61//! [`parse_findings`] converts what a model said into
62//! [`crate::review_score::CandidateFinding`]s and no further. It does not check a
63//! finding, rank it, or drop it for looking implausible. The one filter in this
64//! module is [`crate::compile_claim`]'s, and even that is applied by the caller
65//! against evidence the caller supplies — see [`claim_site`], which only computes
66//! *what configuration the code needs*, never whether a check ran.
67
68use std::fmt::Write as _;
69
70use crate::compile_claim::{ClaimSite, Features, TargetOs};
71use crate::review_corpus::{CLASSES, DefectClass};
72use crate::review_score::CandidateFinding;
73
74/// The measured single-call context budget on this repository, in tokens.
75///
76/// 35a's figure, and the one 189 of the corpus's 190 file-diffs fit inside. Used
77/// as the default per-file budget because a per-file reviewer that also fits the
78/// single-call budget needs no second number to explain.
79pub const SINGLE_CALL_BUDGET_TOKENS: usize = 30_000;
80
81/// Estimate a string's token count as `len / 4`.
82///
83/// The same basis every budget figure in this stage is quoted on — 35a's
84/// corpus-wide totals and this module's per-file distribution alike — so the
85/// numbers compare. It is an estimate of the right order, **not** a tokeniser's
86/// count, and is deliberately not swapped for one: a real count would need the
87/// model's vocabulary, which would make a pure function depend on which model is
88/// installed and make two runs on two machines incomparable.
89#[must_use]
90pub fn estimate_tokens(text: &str) -> usize {
91    text.len() / 4
92}
93
94/// One file to review, with the diff that changed it.
95#[derive(Debug, Clone, PartialEq, Eq)]
96pub struct FileUnderReview {
97    /// The commit being reviewed — a corpus `reviewed_sha` on a replay run.
98    pub reviewed_sha: String,
99    /// Repository-relative path.
100    pub path: String,
101    /// The unified diff for this file alone.
102    pub diff: String,
103}
104
105/// One piece of pre-assembled, provenance-tagged context for the file under
106/// review — **the slot that PR 1 ships empty**.
107///
108/// The graph's contribution to a review is not access: an agentic reviewer can
109/// read any file it likes via tool calls, and one has been observed doing so
110/// correctly on this very corpus. It is that the relevant context arrives
111/// *already selected and already labelled with where it came from*, so the model
112/// spends its budget reading rather than searching.
113///
114/// [`provenance`](Self::provenance) is carried into the prompt rather than
115/// flattened away because the three layers mean different things to a reviewer: a
116/// `derived` fact is a deterministic function of the bytes, an `authored` one is
117/// somebody's stated intent, and an `inferred` one is a guess with a confidence.
118/// A reviewer told an ADR *governs* a symbol is being told something different
119/// from a reviewer handed a similar-looking file.
120#[derive(Debug, Clone, PartialEq, Eq)]
121pub struct ContextItem {
122    /// What this is, for the prompt — `ADR-0019 §3`, or `callers of resolve`.
123    pub label: String,
124    /// `derived` | `authored` | `inferred` — the graph's own vocabulary.
125    pub provenance: String,
126    /// The text itself.
127    pub body: String,
128}
129
130/// The hard ceiling on a context block, in estimated tokens.
131///
132/// PR 1 measured ~28k of the single-call budget free on a median file, and that
133/// headroom is what made this arm testable at all. It is **permission, not an
134/// instruction**: a prompt in which the diff is 6% of the tokens is a
135/// needle-in-a-haystack task, and handing a model 28k of loosely-related text is
136/// a plausible way to make both recall *and* the finding rate worse.
137///
138/// So the cap is stated as a constant, before any number was seen, rather than
139/// discovered by watching a score move.
140pub const CONTEXT_CAP_TOKENS: usize = 4_000;
141
142/// How much context one file may carry, relative to its own diff.
143///
144/// The cap is `min(CONTEXT_CAP_TOKENS, RELATIVE * diff_tokens)`, so a two-line
145/// change does not arrive under a thousand lines of ADR. A reviewer should be
146/// reading the change, with context beside it — not the other way round.
147pub const CONTEXT_RELATIVE_TO_DIFF: usize = 2;
148
149/// The graph context handed to one file's review.
150///
151/// [`GraphContext::none`] is the diff-only arm, and [`build_prompt`] renders no
152/// context section for it, so the baseline prompt carries no vestigial heading
153/// promising something that is not there.
154///
155/// # What is in it, and why those and not the rest
156///
157/// The menu `roteiro review` already computes is governing ADRs, callers,
158/// callees, blast radius and authored drift. The arm takes **two** of those and
159/// records the omission of the others as a decision:
160///
161/// * **Governing ADR and blueprint sections** (`authored`). The one thing a
162///   per-file reviewer structurally cannot obtain: a decision written in another
163///   file that the code under review contradicts. This is `contract-drift`'s
164///   defining shape and the authored layer's whole purpose.
165/// * **The file's own doc surface outside the diff** (`derived`). [`build_prompt`]
166///   instructs the model to stay silent unless *both* halves of a conflict are
167///   visible, and a `-U3` diff shows at most three lines either side. A doc
168///   comment at the top of a file and the code that betrays it 700 lines down are
169///   never both in the hunks. The graph holds each symbol's doc comment, so this
170///   makes the promise-half visible without pasting the file.
171/// * **Callers, callees and blast radius are deliberately excluded.** Measured on
172///   this repository a single changed symbol carries dozens of caller keys, and
173///   their bodies would dominate the prompt — the failure mode above, bought
174///   knowingly. Recorded here so the absence reads as a decision rather than an
175///   oversight, and so a later arm that adds them knows it is changing a
176///   pre-registered variable.
177#[derive(Debug, Clone, Default, PartialEq, Eq)]
178pub struct GraphContext {
179    /// The items, in the order they are rendered.
180    pub items: Vec<ContextItem>,
181    /// Items dropped by [`GraphContext::fit`] to stay inside the cap.
182    ///
183    /// Counted rather than silently absorbed, for the same reason
184    /// [`Prompt::dropped_tokens`] is: a run that quietly shed the context it was
185    /// measuring would report the graph arm as having been tested when it was
186    /// partly the diff-only arm wearing its name.
187    pub dropped_items: usize,
188}
189
190impl GraphContext {
191    /// No context — the diff-only arm.
192    #[must_use]
193    pub fn none() -> Self {
194        Self::default()
195    }
196
197    /// Whether any context is present.
198    #[must_use]
199    pub fn is_empty(&self) -> bool {
200        self.items.is_empty()
201    }
202
203    /// Estimated tokens this context will cost, by [`estimate_tokens`] over the
204    /// text [`build_prompt`] actually renders — label, provenance tag and body.
205    #[must_use]
206    pub fn tokens(&self) -> usize {
207        self.items.iter().map(ContextItem::tokens).sum()
208    }
209
210    /// Drop whole items, lowest-priority last-first, until the context fits the
211    /// cap for a diff of `diff_tokens`.
212    ///
213    /// **Whole items, never a truncated one.** A half-quoted ADR is worse than no
214    /// ADR: it reads as a complete statement of a decision and is not one, so a
215    /// model can be handed a promise whose exception was cut off and report the
216    /// code as contradicting it. [`build_prompt`] makes the same choice for the
217    /// same reason and truncates only the diff.
218    ///
219    /// Callers pass items in priority order — most valuable first — because this
220    /// drops from the tail.
221    #[must_use]
222    pub fn fit(items: Vec<ContextItem>, diff_tokens: usize) -> Self {
223        let cap = CONTEXT_CAP_TOKENS.min(CONTEXT_RELATIVE_TO_DIFF.saturating_mul(diff_tokens));
224        let mut kept: Vec<ContextItem> = Vec::new();
225        let mut spent = 0usize;
226        let mut dropped = 0usize;
227        for item in items {
228            let cost = item.tokens();
229            // A later, smaller item is still allowed in after a large one was
230            // refused: the cap is on the block, and skipping an oversized ADR
231            // should not also discard the three short doc comments behind it.
232            if spent + cost > cap {
233                dropped += 1;
234                continue;
235            }
236            spent += cost;
237            kept.push(item);
238        }
239        Self {
240            items: kept,
241            dropped_items: dropped,
242        }
243    }
244}
245
246impl ContextItem {
247    /// What this item costs in the prompt, by [`estimate_tokens`].
248    ///
249    /// Counts the rendered form — the `--- label [provenance]` heading as well as
250    /// the body — because that is what the budget actually pays for.
251    #[must_use]
252    pub fn tokens(&self) -> usize {
253        estimate_tokens(&self.label)
254            + estimate_tokens(&self.provenance)
255            + estimate_tokens(&self.body)
256            + 4
257    }
258}
259
260/// The body of one `## ` section of a markdown document, **by its heading title**.
261///
262/// The graph stores an `adr_section` node per heading but **no body text** — the
263/// node carries a slug, a title and nothing else. So the graph selects *which*
264/// decision governs the code under review, and this renders it. That split is the
265/// point: retrieval is the graph's, quoting is a string operation, and nothing
266/// here decides relevance.
267///
268/// # Matched on the title, not on the slug, and that is deliberate
269///
270/// An `adr_section` key is `adr:0005#decision` — the slug is right there, so
271/// matching on it looks like the obvious read. It would mean reimplementing
272/// `rto_spec`'s slug rule here, because that rule is `pub(crate)` and `rto-graph`
273/// does not depend on `rto-spec` at all. A second copy of a rule this crate cannot
274/// see is a rule free to drift, and the drift would be silent: a heading whose
275/// punctuation the two versions collapsed differently would simply stop resolving,
276/// and the graph arm would quietly run with one fewer ADR than it reported.
277///
278/// The node's `name` is the heading text verbatim, so comparing titles needs no
279/// shared rule and cannot drift. Where two `## ` headings share a title the first
280/// wins; nothing downstream distinguishes them either.
281///
282/// Returns everything after the heading up to the next `## `, or `None` when no
283/// heading matches.
284#[must_use]
285pub fn section_body(markdown: &str, title: &str) -> Option<String> {
286    let mut out: Option<String> = None;
287    for line in markdown.lines() {
288        if let Some(heading) = line.strip_prefix("## ") {
289            if out.is_some() {
290                break;
291            }
292            if heading.trim() == title.trim() {
293                out = Some(String::new());
294            }
295            continue;
296        }
297        // A `# ` title or a deeper `### ` subheading is body, not a boundary: only
298        // `## ` delimits the sections the graph made nodes for.
299        if let Some(body) = out.as_mut() {
300            body.push_str(line);
301            body.push('\n');
302        }
303    }
304    out.map(|b| b.trim().to_owned())
305}
306
307/// How much of a doc comment must already be visible in the diff before quoting
308/// it again is redundant.
309///
310/// Compared on the first `PROBE` significant characters rather than the whole
311/// text: the two copies are never byte-identical, because the diff's is wrapped,
312/// numbered and comment-marked while the graph's is the extracted prose.
313const DOC_PROBE_CHARS: usize = 60;
314
315/// Reduce text to the characters two copies of the same doc comment share.
316///
317/// Whitespace, `annotate_diff`'s line-number column, and Rust's comment markers
318/// all differ between the diff's rendering of a doc and the graph's, and none of
319/// them carry meaning for this comparison. Dropping them is what lets a doc
320/// wrapped across three numbered `///` lines match the single paragraph the
321/// extractor stored.
322fn doc_signature(text: &str, strip_annotation_column: bool) -> String {
323    let mut out = String::with_capacity(text.len());
324    for line in text.lines() {
325        // `annotate_diff` renders `{n:>6} +|body`, `      - |body` and
326        // `{n:>6}  |body`, so the content is whatever follows the first `|`. A
327        // source line containing its own `|` is unaffected: the column's comes
328        // first. Lines before the first hunk carry no column and are taken whole.
329        let body = if strip_annotation_column {
330            line.split_once('|').map_or(line, |(_, rest)| rest)
331        } else {
332            line
333        };
334        let body = body.trim_start();
335        let body = body
336            .strip_prefix("///")
337            .or_else(|| body.strip_prefix("//!"))
338            .or_else(|| body.strip_prefix("//"))
339            .unwrap_or(body);
340        out.extend(body.chars().filter(|c| !c.is_whitespace()));
341    }
342    out
343}
344
345/// Whether `doc` is already visible in `annotated_diff`, so quoting it again
346/// would spend budget on text the model can already read.
347///
348/// **The point of the graph arm is the doc that is *not* in the hunks.**
349/// [`build_prompt`] tells the model to stay silent unless both halves of a
350/// conflict are visible, and a `-U3` diff shows three lines either side — so a
351/// module doc at line 16 and the code that betrays it at line 700 are never both
352/// shown. Re-sending the halves that *are* shown would inflate the context block
353/// with duplicates and buy nothing; worse, on a cap that drops whole items it
354/// would evict the ones that matter.
355///
356/// `annotated_diff` is the diff **as the model sees it** — [`annotate_diff`]'s
357/// output, line-number column and all — because "already visible" is a claim about
358/// the prompt, not about the raw hunks.
359#[must_use]
360pub fn doc_already_shown(doc: &str, annotated_diff: &str) -> bool {
361    let probe: String = doc_signature(doc, false)
362        .chars()
363        .take(DOC_PROBE_CHARS)
364        .collect();
365    // Too short to identify anything, and too short to state a contract that could
366    // drift: treated as shown rather than padding the context with one-word docs.
367    if probe.chars().count() < DOC_PROBE_CHARS {
368        return true;
369    }
370    doc_signature(annotated_diff, true).contains(&probe)
371}
372
373/// An assembled prompt, with what it cost and what it had to leave out.
374#[derive(Debug, Clone, PartialEq, Eq)]
375pub struct Prompt {
376    /// The text to send.
377    pub text: String,
378    /// Its estimated size, by [`estimate_tokens`].
379    pub tokens: usize,
380    /// Diff tokens dropped to fit the budget, or `0`.
381    ///
382    /// Reported rather than silently absorbed: a review of a truncated file is a
383    /// review of part of it, and a run that does not say so reads as coverage it
384    /// did not have.
385    pub dropped_tokens: usize,
386}
387
388/// The output contract, stated once and used twice — [`build_prompt`] asks for
389/// this and [`parse_findings`] reads it.
390///
391/// A line format rather than JSON, because the failure modes are not symmetric.
392/// A model that mangles one line of a line format loses that finding; a model
393/// that mangles one brace of a JSON document loses the whole review, and the
394/// low-tier instruct model this resolver defaults to does the second more often
395/// than the first.
396const FINDING_PREFIX: &str = "FINDING";
397
398/// What a model emits when it has nothing to report — required, so that "no
399/// findings" and "the model ignored the format" are distinguishable in
400/// [`Parsed::unparsed`] rather than both arriving as silence.
401const NO_FINDINGS: &str = "NO FINDINGS";
402
403/// Build the review prompt for one file.
404///
405/// `budget` caps the whole prompt; the diff is truncated to fit and the amount
406/// dropped is reported on [`Prompt::dropped_tokens`]. Context is never truncated —
407/// a half-quoted ADR is worse than none, and on the measured distribution it never
408/// comes to that.
409#[must_use]
410pub fn build_prompt(file: &FileUnderReview, context: &GraphContext, budget: usize) -> Prompt {
411    let mut head = String::new();
412    head.push_str(
413        "You are reviewing ONE FILE of a change to a Rust codebase.\n\n\
414         The defects that matter here are **contract-accuracy** defects: code that \
415         runs correctly but does not mean what it says. They compile, they pass \
416         tests, and CI is green on them by definition — so they are found only by \
417         reading the words against the behaviour. Do not look for crashes or \
418         compile errors; look for places where a promise and its implementation \
419         have come apart.\n\n\
420         Work through the change against each of these, in order:\n\n",
421    );
422    for class in CLASSES {
423        let _ = writeln!(head, "  {} — {}", class.as_str(), class_gloss(class));
424    }
425    head.push_str(
426        "\nFor each one, ask specifically:\n\
427         - Does a doc comment, `///` line, README sentence or ADR in this diff \
428         state something the code beside it does not do? Compare the two texts \
429         word by word — a doc that describes the old behaviour after the code \
430         moved on is the single most common defect in this codebase.\n\
431         - Does an error message name the rule it actually enforces, or a \
432         different one?\n\
433         - Does a test assert the behaviour its name claims, or would it pass with \
434         the feature removed?\n\
435         - Does a check permit the state it exists to forbid (off-by-one, wrong \
436         comparison, missing case)?\n\
437         - Is a key, hash or id built from something lossy, so two different \
438         inputs collide?\n\n\
439         Output format. One finding per line, nothing else on the line:\n\
440         \x20   FINDING | line=<n> | class=<class> | compile=<yes|no> | <one sentence>\n\n\
441         For example:\n\
442         \x20   FINDING | line=214 | class=contract-drift | compile=no | the doc says \
443         the cache is unbounded but `insert` evicts at 256 entries\n\
444         \x20   FINDING | line=87 | class=permissive-constraint | compile=no | uses \
445         `<=` so a zero-length span passes the guard that exists to reject it\n\n\
446         Rules:\n\
447         - Cite the NEW-SIDE line number from the left column. Every line is \
448         numbered for you; never compute one from the hunk header.\n\
449         - **Both halves must be visible below.** Report a conflict only when the \
450         promise AND the behaviour that breaks it are both in the lines shown. If \
451         you can see a doc comment but not the code it describes, or a call but \
452         not the signature it calls, you cannot tell whether they disagree — say \
453         nothing. Do not infer what code you have not been shown does.\n\
454         - Quote the specific words that conflict, so a reader can check you \
455         without opening the file.\n\
456         - Do not restate one point as several findings. Each finding must be a \
457         separate defect a separate commit would fix.\n\
458         - `compile=yes` ONLY if you are claiming the code will not build. \
459         Everything else is `compile=no`.\n",
460    );
461    let _ = writeln!(
462        head,
463        "         - Reply {NO_FINDINGS} only if you have worked through every class \
464         above and found nothing. A file whose change is routine is a normal \
465         outcome, and reporting nothing is better than reporting a guess."
466    );
467
468    let mut context_block = String::new();
469    if !context.is_empty() {
470        context_block.push_str(
471            "\nContext from the repository's graph. This is not part of the \
472             change; it is what the graph knows about the code under review, and \
473             each item says which layer it came from.\n\n",
474        );
475        for item in &context.items {
476            let _ = writeln!(
477                context_block,
478                "--- {} [{}]\n{}",
479                item.label,
480                item.provenance,
481                item.body.trim_end()
482            );
483        }
484    }
485
486    let annotated = annotate_diff(&file.diff);
487    let tail_header = format!("\nFile under review: {}\n\n", file.path);
488    let fixed =
489        estimate_tokens(&head) + estimate_tokens(&context_block) + estimate_tokens(&tail_header);
490    let room = budget.saturating_sub(fixed);
491    let (body, dropped_tokens) = truncate_to_tokens(&annotated, room);
492
493    let text = format!("{head}{context_block}{tail_header}{body}");
494    Prompt {
495        tokens: estimate_tokens(&text),
496        text,
497        dropped_tokens,
498    }
499}
500
501/// A one-line gloss per defect class, for the prompt.
502///
503/// Written from the class's own meaning rather than from any corpus row, and kept
504/// beside [`CLASSES`] so a new class cannot be added without deciding how to
505/// describe it to a reviewer. `class_gloss_covers_every_class` holds that.
506fn class_gloss(class: DefectClass) -> &'static str {
507    match class {
508        DefectClass::CleanupGap => "a guard stops a cleanup path doing its job",
509        DefectClass::ContractDrift => {
510            "a doc comment, README or ADR states something the code does not do"
511        }
512        DefectClass::ErrorTextDrift => "an error message does not state the rule it enforces",
513        DefectClass::FalseCompileClaim => "the code will not compile (see the compile= rule)",
514        DefectClass::LintConvention => "a lint suppression carries no justification",
515        DefectClass::LossyIdentity => {
516            "a key built from a lossy conversion, so distinct inputs collide"
517        }
518        DefectClass::MissingEvent => "an early return skips a documented side effect",
519        DefectClass::OrderingBug => "an aggregate is computed after the mutation it must precede",
520        DefectClass::PerfContract => "the implementation defeats a field's stated design goal",
521        DefectClass::PermissiveConstraint => "a check permits the state it exists to forbid",
522        DefectClass::ProseClarity => "wording that misleads a reader",
523        DefectClass::SilentTruncation => "a read or copy drops a remainder without erroring",
524        DefectClass::UxDiagnostic => "a message tells the user to do the wrong thing",
525        DefectClass::VacuousTest => "a test passes whether or not the behaviour it names works",
526    }
527}
528
529/// Render a unified diff with **new-side line numbers in a left column**.
530///
531/// A reviewer's finding is scored by its line, within
532/// [`crate::review_score::LINE_WINDOW`]. Asking a model to derive a line number
533/// from `@@ -a,b +c,d @@` spends budget on arithmetic it is bad at and turns a
534/// correct finding into a miss — which would be measured as the reviewer failing
535/// to see the defect rather than failing to count. So the arithmetic is done here,
536/// where it is exact.
537///
538/// Removed lines carry no new-side number and are marked `-`, so the model can
539/// still see what was replaced without being able to cite a line that no longer
540/// exists.
541#[must_use]
542pub fn annotate_diff(diff: &str) -> String {
543    let mut out = String::with_capacity(diff.len() + diff.len() / 8);
544    let mut new_line: Option<u32> = None;
545    for raw in diff.lines() {
546        if raw.starts_with("@@") {
547            new_line = parse_hunk_new_start(raw);
548            out.push_str(raw);
549            out.push('\n');
550            continue;
551        }
552        // Everything before the first hunk (`diff --git`, `---`, `+++`, mode
553        // lines) is passed through unnumbered: it is not file content.
554        let Some(n) = new_line else {
555            out.push_str(raw);
556            out.push('\n');
557            continue;
558        };
559        match raw.as_bytes().first() {
560            Some(b'-') => {
561                let _ = writeln!(out, "      - |{}", &raw[1..]);
562            }
563            Some(b'+') => {
564                let _ = writeln!(out, "{n:>6} +|{}", &raw[1..]);
565                new_line = Some(n + 1);
566            }
567            Some(b'\\') => {
568                let _ = writeln!(out, "        |{raw}");
569            }
570            // A context line, including the empty string a bare `\n` produces.
571            _ => {
572                let body = raw.strip_prefix(' ').unwrap_or(raw);
573                let _ = writeln!(out, "{n:>6}  |{body}");
574                new_line = Some(n + 1);
575            }
576        }
577    }
578    out
579}
580
581/// The new-side start line of a `@@ -a,b +c,d @@` header.
582fn parse_hunk_new_start(header: &str) -> Option<u32> {
583    let plus = header.split('+').nth(1)?;
584    let digits: String = plus.chars().take_while(char::is_ascii_digit).collect();
585    digits.parse().ok()
586}
587
588/// Truncate `text` to `budget` tokens at a line boundary, returning the kept text
589/// and the number of tokens dropped.
590///
591/// The head is kept rather than the tail: a diff's first hunks are the ones a
592/// reviewer can still anchor, and a review of the first half of a file is a
593/// partial review, while a review of the second half with no idea what preceded it
594/// is a confused one. The marker is left in the text so the *model* also knows it
595/// is seeing part of a file.
596fn truncate_to_tokens(text: &str, budget: usize) -> (String, usize) {
597    /// Charged against the budget before cutting, so adding it cannot push the
598    /// result back over the budget it was just cut to.
599    const MARKER: &str =
600        "\n[... truncated to fit the context budget: this is PART of the file ...]\n";
601
602    if estimate_tokens(text) <= budget {
603        return (text.to_owned(), 0);
604    }
605    let room = budget.saturating_sub(estimate_tokens(MARKER)) * 4;
606    let mut kept = 0usize;
607    for line in text.split_inclusive('\n') {
608        if kept + line.len() > room {
609            break;
610        }
611        kept += line.len();
612    }
613    let dropped = estimate_tokens(&text[kept..]);
614    (format!("{}{MARKER}", &text[..kept]), dropped)
615}
616
617/// What [`parse_findings`] made of a model's reply.
618#[derive(Debug, Clone, Default, PartialEq, Eq)]
619pub struct Parsed {
620    /// Findings in the corpus's coordinate system, ready to score.
621    pub findings: Vec<CandidateFinding>,
622    /// Lines that looked like an attempted finding but could not be read as one.
623    ///
624    /// Counted rather than discarded. A model that ignores the output format
625    /// scores exactly like a model that found nothing, and those are opposite
626    /// facts about a reviewer: the first needs a different prompt, the second a
627    /// different model. A run reports this so the two cannot be confused.
628    pub unparsed: Vec<String>,
629    /// Whether the reply declared the file clean in the required form.
630    pub declared_clean: bool,
631    /// The generation stopped inside a reasoning block, so the model never
632    /// reached its answer.
633    ///
634    /// **This is the stage's own silent zero, found by walking into it.** A
635    /// reasoning GGUF opens `<think>` and deliberates before answering; hit the
636    /// token cap first and the reply contains no findings and no `NO FINDINGS` —
637    /// indistinguishable, to anything counting findings, from a reviewer that read
638    /// the file and passed it. Measured on `qwen3.8-27b`, whose careful
639    /// doc-versus-code deliberation was **entirely** inside the block and scored
640    /// as silence.
641    ///
642    /// So it is a reported outcome rather than an absence. A run that cannot tell
643    /// "found nothing" from "never answered" is reporting a recall figure it did
644    /// not measure.
645    pub reasoning_truncated: bool,
646}
647
648/// Read a model's reply into findings.
649///
650/// Lenient about presentation and strict about content: a leading bullet, bold
651/// markers or a code fence are stripped, because a model wrapping the format in
652/// markdown has still followed it — but a finding with no readable positive line
653/// number goes to [`Parsed::unparsed`], since an unanchored finding cannot be
654/// scored, shown to a human, or acted on.
655///
656/// Leniency stops at the description. Decoration is stripped from the line's ends
657/// and from the *structure* of each `key=value` field, never from the free-form
658/// prose a human is going to read — a parser that quietly rewrites the text it
659/// reports is editing the evidence.
660#[must_use]
661pub fn parse_findings(reviewed_sha: &str, path: &str, reply: &str) -> Parsed {
662    let mut out = Parsed {
663        // Checked on the text as handed over, which a caller has already run its
664        // `</think>` strip across: an *opening* tag still present means the
665        // closing one never arrived, so generation stopped mid-deliberation.
666        //
667        // **Now a second line of defence rather than the first.** Since #583 the
668        // strip itself refuses a block it never saw closed, and `review_llm`
669        // reports that as `reasoning_truncated` without reaching this function at
670        // all. What survives to here is the case that rule deliberately does not
671        // claim — a block opened part-way through a reply rather than at its
672        // start. Kept because the cost is one `contains` and the failure it
673        // prevents is a truncated review counted as a clean file.
674        reasoning_truncated: reply.contains("<think>"),
675        ..Parsed::default()
676    };
677    for raw in reply.lines() {
678        let line = raw.trim().trim_start_matches(['-', '*', '>', '#', ' ']);
679        let line = line.trim_start_matches('`').trim_end();
680        // Whole-line decoration only. The leading trim above has already taken any
681        // opening `**`, so this closes the pair — for `**NO FINDINGS**`, and for a
682        // finding line a model has bolded end to end. Bold *inside* the line is
683        // deliberately left alone here and dealt with per-field in `parse_one`;
684        // see the note there for why the difference matters.
685        let line = line.strip_suffix("**").unwrap_or(line).trim();
686        if line.eq_ignore_ascii_case(NO_FINDINGS) {
687            out.declared_clean = true;
688            continue;
689        }
690        if !line
691            .get(..FINDING_PREFIX.len())
692            .is_some_and(|p| p.eq_ignore_ascii_case(FINDING_PREFIX))
693        {
694            continue;
695        }
696        match parse_one(reviewed_sha, path, line) {
697            Some(finding) => out.findings.push(finding),
698            None => out.unparsed.push(line.to_owned()),
699        }
700    }
701    out
702}
703
704/// Parse one `FINDING | …` line, or `None` if it carries no usable line number.
705fn parse_one(reviewed_sha: &str, path: &str, line: &str) -> Option<CandidateFinding> {
706    let mut number: Option<u32> = None;
707    let mut class = None;
708    let mut claims_compile_failure = false;
709    let mut description = String::new();
710
711    for field in line.split('|').skip(1) {
712        let field = field.trim().trim_end_matches('`').trim();
713        // **Bold is stripped for the structural read only, and only where it
714        // resolves to a field this format defines.**
715        //
716        // The line-wide `replace("**", "")` this replaces was not there to handle
717        // *leading* bold — the trim in `parse_findings` already takes that. It was
718        // there for bold *inside* the line, i.e. a model emitting
719        // `class=**contract-drift**` or `**line**=42`, which has still followed the
720        // format and must still parse. But applying it to the whole line also
721        // rewrote the description, so emphasis a model put in prose vanished from
722        // the text a human is shown and a corpus is scored against.
723        //
724        // Splitting the two reads keeps the field tolerance and drops the
725        // rewriting: `structural` exists only to decide what this token *is*, and
726        // if the answer is "not a known field" the original bytes go through
727        // untouched.
728        let structural = field.replace("**", "");
729        let key_value = structural
730            .split_once('=')
731            .map(|(k, v)| (k.trim().to_ascii_lowercase(), v.trim()));
732
733        match key_value.as_ref().map(|(k, v)| (k.as_str(), *v)) {
734            Some(("line", value)) => number = value.parse().ok().filter(|n| *n > 0),
735            Some(("class", value)) => class = DefectClass::from_token(&value.to_ascii_lowercase()),
736            Some(("compile", value)) => {
737                claims_compile_failure = matches!(
738                    value.to_ascii_lowercase().as_str(),
739                    "yes" | "true" | "y" | "1"
740                );
741            }
742            // Everything else is description, kept verbatim. A field with no `=`
743            // is the description proper; later ones are its continuation, because
744            // a description may itself contain a pipe. An *unknown* `key=value` is
745            // kept as prose rather than dropped: it is more likely a description
746            // containing an `=` than an invented field, and losing it would leave
747            // a human a bare line number.
748            _ => {
749                if !description.is_empty() {
750                    description.push_str(" | ");
751                }
752                description.push_str(field);
753            }
754        }
755    }
756
757    Some(CandidateFinding {
758        reviewed_sha: reviewed_sha.to_owned(),
759        path: path.to_owned(),
760        line: number?,
761        description: if description.trim().is_empty() {
762            "(no description given)".to_owned()
763        } else {
764            description.trim().to_owned()
765        },
766        claims_compile_failure,
767        defect_class: class,
768    })
769}
770
771/// Derive the [`ClaimSite`] for a compile claim, from the reviewed file's bytes.
772///
773/// `parent_source` is the module's parent (`lib.rs`/`mod.rs`) when the caller has
774/// it, which is the only place a file's feature gate is written.
775///
776/// # Conservative on every axis, deliberately
777///
778/// [`crate::compile_claim`] states the asymmetry this follows: a claim wrongly
779/// suppressed is a defect shipped silently — the #291 macOS teardown shape — while
780/// a claim wrongly kept costs a human one look at a CI page. So every derivation
781/// here errs toward *establishing a requirement*, which makes a site harder to
782/// refute, never easier:
783///
784/// * **Platform.** Any `cfg(target_os = "macos"/"windows")` anywhere in the file
785///   marks the whole file as needing that platform, so no job in this
786///   ubuntu-only CI can refute a claim about it. Coarse, and coarse in the safe
787///   direction: a file with one macOS-gated function keeps its compile claims.
788/// * **Features.** A `#[cfg(feature = …)]` on the module's declaration in
789///   `parent_source` marks the site as needing [`Features::All`], so only an
790///   `--all-features` job covers it. Without `parent_source` nothing is
791///   established, which is the one axis where "unknown" reads as unconditional —
792///   stated here rather than left for a reader to infer from the field docs.
793/// * **Targets.** A path under `tests/`, `benches/` or `examples/` is test code;
794///   so is a line *after* a `#[cfg(test)]` attribute in the file. Both need a job
795///   that passed `--all-targets`, which `msrv` does not.
796#[must_use]
797pub fn claim_site(
798    reviewed_sha: &str,
799    path: &str,
800    line: u32,
801    source: &str,
802    parent_source: Option<&str>,
803) -> ClaimSite {
804    ClaimSite {
805        platform: required_platform(source),
806        features: parent_source.and_then(|p| module_feature_gate(path, p)),
807        is_test_code: is_test_code(path, line, source),
808        toolchain: None,
809        ..ClaimSite::unknown(reviewed_sha, path)
810    }
811}
812
813/// The platform a file's `cfg` gates require, if it names one.
814fn required_platform(source: &str) -> Option<TargetOs> {
815    // macOS first: it is the platform this repository actually has uncompiled
816    // code for, and the one #291 shipped a defect behind.
817    for (needle, os) in [
818        ("target_os = \"macos\"", TargetOs::MacOs),
819        ("target_os = \"windows\"", TargetOs::Windows),
820    ] {
821        if source.contains(needle) {
822            return Some(os);
823        }
824    }
825    None
826}
827
828/// Whether `line` is test code: a test-only path, or a line after a
829/// `#[cfg(test)]` attribute.
830fn is_test_code(path: &str, line: u32, source: &str) -> bool {
831    if ["tests/", "benches/", "examples/"]
832        .iter()
833        .any(|d| path.starts_with(d) || path.contains(&format!("/{d}")))
834    {
835        return true;
836    }
837    // The line-relative form, so a `#[cfg(test)] mod tests` at the bottom of a
838    // source file does not mark the library code above it as test code — which
839    // would disable the filter on nearly every file in this repository.
840    source
841        .lines()
842        .position(|l| l.trim_start().starts_with("#[cfg(test)]"))
843        .is_some_and(|idx| line as usize > idx + 1)
844}
845
846/// The name a file is declared under by the `mod` item in its parent.
847///
848/// For `a/b/thing.rs` that is the file stem, `thing`. For `a/b/mod.rs` it is the
849/// *directory* name, `b`: a `mod.rs` is declared by `mod b;` in `a`'s source, and
850/// never by `mod mod;`.
851///
852/// # Why this is spelled out rather than left as `file_stem`
853///
854/// Taking the stem for a `mod.rs` searches the parent for `mod mod;`, which
855/// cannot match, so the lookup returns `None` — and a `None` on the features axis
856/// reads as *unconditional*, i.e. covered by any green job. That is the
857/// permissive direction: a feature-gated module would have its compile claims
858/// suppressed by a job that never compiled it. [`claim_site`]'s contract is that
859/// every derivation errs toward establishing a requirement, so this one case has
860/// to be got right rather than left to fall through.
861fn declaring_name(path: &str) -> Option<&str> {
862    let stem = path.rsplit('/').next()?.strip_suffix(".rs")?;
863    if stem == "mod" {
864        // `a/b/mod.rs` is declared as `b`; a bare `mod.rs` with no directory
865        // above it is declared by nothing, so there is no name to look for.
866        path.rsplit('/').nth(1)
867    } else {
868        Some(stem)
869    }
870}
871
872/// The feature gate on this file's `mod` declaration in its parent, if any.
873///
874/// Returns [`Features::All`] rather than naming the feature: the coverage model
875/// asks which *job* compiled the code, and this repository's jobs are
876/// `--all-features`, the default set, or nothing. Which named feature it is does
877/// not change the answer.
878///
879/// # Known gap: `#[path = "…"]`
880///
881/// A module declared `#[path = "elsewhere.rs"] mod name;` is not found by this
882/// lookup, because the declaring name cannot be recovered from the file path at
883/// all — the mapping lives in the attribute, in a parent this function is not
884/// given a way to search for. The result is `None`, which is the permissive
885/// direction, so this is a real (if narrow) hole rather than a tidy limitation.
886/// It is left open deliberately: closing it means scanning candidate parents for
887/// `#[path]` attributes and resolving them relative to the declaring file, which
888/// is a different shape of change from this one. This repository contains no
889/// `#[path]` attributes, so nothing here relies on it today.
890fn module_feature_gate(path: &str, parent_source: &str) -> Option<Features> {
891    let stem = declaring_name(path)?;
892    let lines: Vec<&str> = parent_source.lines().collect();
893    let decl = lines.iter().position(|l| {
894        let t = l.trim_start().trim_start_matches("pub ").trim_start();
895        t.starts_with(&format!("mod {stem};")) || t.starts_with(&format!("mod {stem} "))
896    })?;
897    // Walk back over the declaration's own attributes only, stopping at the first
898    // line that is not one — attributes bind to what follows them, so a `cfg` two
899    // items up governs a different item.
900    for above in lines[..decl].iter().rev() {
901        let t = above.trim();
902        if t.is_empty() || t.starts_with("//") {
903            continue;
904        }
905        if !t.starts_with("#[") {
906            break;
907        }
908        if t.contains("cfg(feature") || t.contains("cfg(all(feature") {
909            return Some(Features::All);
910        }
911    }
912    None
913}
914
915#[cfg(test)]
916mod tests {
917    use super::{
918        FileUnderReview, GraphContext, NO_FINDINGS, Prompt, SINGLE_CALL_BUDGET_TOKENS,
919        annotate_diff, build_prompt, claim_site, class_gloss, estimate_tokens, parse_findings,
920    };
921    use crate::compile_claim::{CheckRun, Conclusion, Features, TargetOs, Targets, suppression};
922    use crate::review_corpus::{CLASSES, DefectClass};
923
924    const SHA: &str = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
925
926    fn file(diff: &str) -> FileUnderReview {
927        FileUnderReview {
928            reviewed_sha: SHA.to_owned(),
929            path: "crates/rto-graph/src/lib.rs".to_owned(),
930            diff: diff.to_owned(),
931        }
932    }
933
934    /// **The line column is the reviewer's whole anchoring story**, so it must be
935    /// exact: a finding is credited only within `LINE_WINDOW` of a corpus row, and
936    /// a model asked to do hunk arithmetic instead would miss by more than that
937    /// and be scored as blind rather than as bad at counting.
938    #[test]
939    fn the_annotated_diff_numbers_the_new_side_exactly() {
940        let diff = "@@ -10,3 +20,4 @@ fn thing()\n unchanged\n-gone\n+added\n+also\n context\n";
941        let out = annotate_diff(diff);
942        let numbered: Vec<(u32, String)> = out
943            .lines()
944            .filter_map(|l| {
945                let (num, body) = l.split_once('|')?;
946                let n: u32 = num
947                    .trim()
948                    .trim_end_matches(['+', '-'])
949                    .trim()
950                    .parse()
951                    .ok()?;
952                Some((n, body.to_owned()))
953            })
954            .collect();
955        assert_eq!(
956            numbered,
957            vec![
958                (20, "unchanged".to_owned()),
959                (21, "added".to_owned()),
960                (22, "also".to_owned()),
961                (23, "context".to_owned()),
962            ],
963            "new-side numbering starts at the hunk's + start and skips removals"
964        );
965        // A removed line is shown but carries no citable number.
966        assert!(out.contains("      - |gone"), "{out}");
967    }
968
969    /// A diff with several hunks restarts numbering at each header rather than
970    /// counting straight through — the failure that would put every finding after
971    /// the first hunk out of window.
972    #[test]
973    fn numbering_restarts_at_each_hunk() {
974        let diff = "@@ -1,2 +1,2 @@\n a\n b\n@@ -50,2 +90,2 @@\n c\n d\n";
975        let out = annotate_diff(diff);
976        assert!(out.contains("     1  |a"), "{out}");
977        assert!(out.contains("    90  |c"), "{out}");
978        assert!(out.contains("    91  |d"), "{out}");
979    }
980
981    /// Every class the corpus can score is described to the model. A class with no
982    /// gloss is a class the reviewer was never told to look for, which would be
983    /// measured as a recall failure rather than as the omission it is.
984    #[test]
985    fn class_gloss_covers_every_class() {
986        for class in CLASSES {
987            let gloss = class_gloss(class);
988            assert!(!gloss.is_empty(), "{class} has no gloss");
989        }
990        let prompt = build_prompt(&file("@@ -1 +1 @@\n+x\n"), &GraphContext::none(), 30_000);
991        for class in CLASSES {
992            assert!(
993                prompt.text.contains(class.as_str()),
994                "{class} is not named in the prompt"
995            );
996        }
997    }
998
999    /// A helper for the budget tests: an item whose body is `chars` bytes long,
1000    /// so its cost is predictable in `len / 4` terms.
1001    fn item(label: &str, bytes: usize) -> super::ContextItem {
1002        super::ContextItem {
1003            label: label.to_owned(),
1004            provenance: "authored".to_owned(),
1005            body: "x".repeat(bytes),
1006        }
1007    }
1008
1009    /// **The cap is on the block, and it is relative to the diff.** A two-line
1010    /// change must not arrive under four thousand tokens of ADR: the whole risk
1011    /// this arm carries is that context drowns the change, and the guard against
1012    /// it is arithmetic rather than judgement.
1013    #[test]
1014    fn context_is_capped_relative_to_the_diff_it_accompanies() {
1015        // A 100-token diff admits at most 200 tokens of context, so the second
1016        // 150-token item cannot join the first.
1017        let fitted = GraphContext::fit(vec![item("a", 600), item("b", 600)], 100);
1018        assert_eq!(
1019            fitted.items.len(),
1020            1,
1021            "two 150-token items fit a 200-token cap"
1022        );
1023        assert_eq!(fitted.dropped_items, 1);
1024        assert!(
1025            fitted.tokens() <= 200,
1026            "cap breached: {} tokens",
1027            fitted.tokens()
1028        );
1029    }
1030
1031    /// The absolute ceiling binds even when the diff is enormous, so a huge file
1032    /// cannot pull in a proportionally huge context.
1033    #[test]
1034    fn the_absolute_cap_binds_on_a_large_diff() {
1035        // 20k diff tokens would allow 40k by the relative rule alone.
1036        let fitted = GraphContext::fit(
1037            (0..20).map(|i| item(&format!("adr-{i}"), 2_000)).collect(),
1038            20_000,
1039        );
1040        assert!(
1041            fitted.tokens() <= super::CONTEXT_CAP_TOKENS,
1042            "the absolute cap did not bind: {} tokens",
1043            fitted.tokens()
1044        );
1045        assert!(
1046            fitted.dropped_items > 0,
1047            "nothing was dropped, so nothing was capped"
1048        );
1049    }
1050
1051    /// **Whole items only.** A half-quoted ADR reads as a complete statement of a
1052    /// decision and is not one, so a model can be handed a promise whose exception
1053    /// was cut off. This asserts the bodies come through byte-identical.
1054    #[test]
1055    fn fitting_never_truncates_an_item_it_keeps() {
1056        let original = item("adr", 400);
1057        let fitted = GraphContext::fit(vec![original.clone(), item("big", 100_000)], 1_000);
1058        assert_eq!(fitted.items, vec![original], "a kept item was rewritten");
1059        assert_eq!(fitted.dropped_items, 1);
1060    }
1061
1062    /// A large item that does not fit must not also discard the small ones behind
1063    /// it — the cap is a budget, not a stopping point.
1064    #[test]
1065    fn an_oversized_item_does_not_evict_the_smaller_ones_after_it() {
1066        let fitted = GraphContext::fit(vec![item("huge", 100_000), item("small", 40)], 1_000);
1067        assert_eq!(fitted.dropped_items, 1);
1068        assert_eq!(
1069            fitted.items.len(),
1070            1,
1071            "the small item behind an oversized one was lost"
1072        );
1073        assert_eq!(fitted.items[0].label, "small");
1074    }
1075
1076    /// An empty diff admits no context at all, which is the conservative
1077    /// direction: `min(cap, 2 * 0) == 0`.
1078    #[test]
1079    fn an_empty_diff_admits_no_context() {
1080        let fitted = GraphContext::fit(vec![item("adr", 40)], 0);
1081        assert!(fitted.is_empty());
1082        assert_eq!(fitted.dropped_items, 1, "the drop must still be counted");
1083    }
1084
1085    /// [`ContextItem::tokens`] must charge for the heading, not just the body —
1086    /// otherwise a context of many tiny items is billed as nearly free while the
1087    /// prompt pays for every `--- label [provenance]` line.
1088    #[test]
1089    fn an_item_is_charged_for_its_heading_as_well_as_its_body() {
1090        let bare = super::ContextItem {
1091            label: String::new(),
1092            provenance: String::new(),
1093            body: "x".repeat(40),
1094        };
1095        let labelled = super::ContextItem {
1096            label: "ADR-0019 §3 governs `resolve`".to_owned(),
1097            provenance: "authored".to_owned(),
1098            body: "x".repeat(40),
1099        };
1100        assert!(
1101            labelled.tokens() > bare.tokens(),
1102            "the heading was not charged: {} vs {}",
1103            labelled.tokens(),
1104            bare.tokens()
1105        );
1106    }
1107
1108    /// A section runs to the next `## `, and a deeper heading inside it is body.
1109    #[test]
1110    fn a_section_body_stops_at_the_next_sibling_heading() {
1111        let md = "# ADR-0005\n\n## Context\nwhy\n\n## Decision\nthe rule\n\n### Detail\nmore\n\n## Consequences\nafter\n";
1112        assert_eq!(
1113            super::section_body(md, "Decision").as_deref(),
1114            Some("the rule\n\n### Detail\nmore"),
1115            "a `###` subheading must not end the section"
1116        );
1117        assert_eq!(super::section_body(md, "Context").as_deref(), Some("why"));
1118        assert_eq!(super::section_body(md, "Absent"), None);
1119    }
1120
1121    /// **The title is matched verbatim, because a slug rule would be a second
1122    /// copy of one this crate cannot see.** Punctuation that a slug would collapse
1123    /// must still resolve here.
1124    #[test]
1125    fn a_heading_with_punctuation_resolves_without_a_slug_rule() {
1126        let md = "## Options considered + consequences\nbody\n\n## Next\nx\n";
1127        assert_eq!(
1128            super::section_body(md, "Options considered + consequences").as_deref(),
1129            Some("body")
1130        );
1131    }
1132
1133    /// A doc comment already visible in the hunks is not worth re-sending: the
1134    /// context block's whole value is the half the diff does *not* show.
1135    #[test]
1136    fn a_doc_already_in_the_diff_is_not_re_quoted() {
1137        let doc = "Returns the cache entry for `key`, evicting the least recently used entry when the cache is full.";
1138        let shown = format!("   12  |/// {doc}\n   13  |pub fn get(&self) {{}}\n");
1139        assert!(
1140            super::doc_already_shown(doc, &shown),
1141            "the doc is in the diff and was not recognised"
1142        );
1143        assert!(
1144            !super::doc_already_shown(doc, "   12  |pub fn unrelated() {}\n"),
1145            "a doc absent from the diff was treated as shown"
1146        );
1147    }
1148
1149    /// The diff carries a line-number column and `+`/` ` markers the graph's copy
1150    /// of the same doc does not, so the comparison must ignore whitespace — this
1151    /// is the case that a naive `contains` gets wrong.
1152    #[test]
1153    fn the_visibility_test_ignores_the_line_number_column() {
1154        let doc = "The slot lock is held only long enough to hand out an `Arc`, never across initialisation.";
1155        // Wrapped across lines and numbered, exactly as `annotate_diff` renders it.
1156        let shown = "    16 +|/// The slot lock is held only long enough to hand\n    17 +|/// out an `Arc`, never across initialisation.\n";
1157        assert!(
1158            super::doc_already_shown(doc, shown),
1159            "wrapping and numbering defeated the visibility test"
1160        );
1161    }
1162
1163    /// A doc too short to state a contract is treated as already shown, so the
1164    /// context block is not padded with one-word comments that cannot drift.
1165    #[test]
1166    fn a_doc_too_short_to_state_a_contract_is_never_carried() {
1167        assert!(super::doc_already_shown("The key.", "unrelated diff text"));
1168    }
1169
1170    /// **PR 1's arm is diff-only, and the prompt must say nothing else.** An empty
1171    /// [`GraphContext`] renders no context heading at all, so the baseline is not
1172    /// quietly a reviewer told it has context and given none — which is a
1173    /// different prompt, and would make the two arms differ by more than the
1174    /// context.
1175    #[test]
1176    fn an_empty_context_renders_no_context_section() {
1177        let bare = build_prompt(&file("@@ -1 +1 @@\n+x\n"), &GraphContext::none(), 30_000);
1178        assert!(!bare.text.contains("Context from"), "{}", bare.text);
1179        assert!(
1180            !bare.text.contains("[authored]") && !bare.text.contains("[derived]"),
1181            "no provenance labels without context: {}",
1182            bare.text
1183        );
1184
1185        let with = build_prompt(
1186            &file("@@ -1 +1 @@\n+x\n"),
1187            &GraphContext {
1188                items: vec![super::ContextItem {
1189                    label: "ADR-0019 §3".to_owned(),
1190                    provenance: "authored".to_owned(),
1191                    body: "the user layer alone never suffices".to_owned(),
1192                }],
1193                dropped_items: 0,
1194            },
1195            30_000,
1196        );
1197        assert!(with.text.contains("Context from"), "{}", with.text);
1198        assert!(with.text.contains("ADR-0019 §3"), "{}", with.text);
1199        assert!(
1200            with.text.contains("[authored]"),
1201            "provenance travels with the item: {}",
1202            with.text
1203        );
1204    }
1205
1206    /// The prompt fits its budget, and says so when it could not fit the diff.
1207    #[test]
1208    fn a_prompt_respects_its_budget_and_reports_what_it_dropped() {
1209        let big = format!(
1210            "@@ -1,1 +1,{0} @@\n{}",
1211            "+a line of code here\n".repeat(20_000)
1212        );
1213        let f = file(&big);
1214        let Prompt {
1215            text,
1216            tokens,
1217            dropped_tokens,
1218        } = build_prompt(&f, &GraphContext::none(), 8_000);
1219        assert!(tokens <= 8_000, "over budget: {tokens}");
1220        assert!(dropped_tokens > 0, "a 20k-line diff cannot have fit");
1221        assert!(
1222            text.contains("truncated to fit"),
1223            "the model is told it is seeing part of a file"
1224        );
1225
1226        // And the common case: nothing dropped, nothing claimed.
1227        let small = build_prompt(&file("@@ -1 +1 @@\n+x\n"), &GraphContext::none(), 30_000);
1228        assert_eq!(small.dropped_tokens, 0);
1229        assert!(!small.text.contains("truncated"));
1230    }
1231
1232    /// **The headroom the graph arm depends on, asserted rather than assumed.**
1233    ///
1234    /// The largest reviewable source file-diff in the corpus is 14,034 raw tokens
1235    /// (`rto-graph/src/models.rs`), which [`annotate_diff`] takes to **17,202** —
1236    /// the numbering column costs a measured **1.21×** across all 190 file-diffs,
1237    /// because it adds a fixed 9 characters per *line* rather than a fraction of
1238    /// the bytes. Reconstructed here at that size and at this repository's line
1239    /// length, so a prompt change that eats the headroom fails here rather than
1240    /// showing up later as a worse score nobody can attribute.
1241    #[test]
1242    fn the_prompt_scaffolding_leaves_the_measured_headroom_intact() {
1243        // ~44 characters per line, this repository's rough average, so the
1244        // annotation overhead lands where it was measured rather than at the
1245        // 4× a two-character line would produce.
1246        let line = format!("+{}\n", "a".repeat(43));
1247        let worst = line.repeat(14_034 * 4 / 44);
1248        let f = file(&format!("@@ -1,1 +1,1 @@\n{worst}"));
1249        let p = build_prompt(&f, &GraphContext::none(), SINGLE_CALL_BUDGET_TOKENS);
1250        assert_eq!(
1251            p.dropped_tokens, 0,
1252            "the corpus's largest source file must not need truncating"
1253        );
1254        let headroom = SINGLE_CALL_BUDGET_TOKENS - p.tokens;
1255        assert!(
1256            headroom > 10_000,
1257            "only {headroom} tokens left for graph context on the worst source \
1258             file; the arm needs room to be testable at all"
1259        );
1260
1261        // And the median file, which is what the headroom claim is really about:
1262        // 1,758 annotated tokens leaves nearly the whole budget free.
1263        let median = file(&format!("@@ -1,1 +1,1 @@\n{}", line.repeat(1_476 * 4 / 44)));
1264        let p = build_prompt(&median, &GraphContext::none(), SINGLE_CALL_BUDGET_TOKENS);
1265        assert!(
1266            SINGLE_CALL_BUDGET_TOKENS - p.tokens > 25_000,
1267            "the median file should leave ~28k free, left {}",
1268            SINGLE_CALL_BUDGET_TOKENS - p.tokens
1269        );
1270    }
1271
1272    #[test]
1273    fn a_well_formed_finding_parses() {
1274        let reply = "FINDING | line=42 | class=contract-drift | compile=no | the doc says X";
1275        let parsed = parse_findings(SHA, "src/a.rs", reply);
1276        assert_eq!(parsed.findings.len(), 1);
1277        let f = &parsed.findings[0];
1278        assert_eq!(f.line, 42);
1279        assert_eq!(f.defect_class, Some(DefectClass::ContractDrift));
1280        assert!(!f.claims_compile_failure);
1281        assert_eq!(f.description, "the doc says X");
1282        assert!(parsed.unparsed.is_empty());
1283    }
1284
1285    /// A model that wraps the format in markdown has still followed it. Being
1286    /// strict here would measure formatting compliance and call it recall.
1287    #[test]
1288    fn presentation_is_tolerated_but_content_is_not() {
1289        let reply = "\
1290- **FINDING** | line=7 | class=vacuous-test | compile=no | asserts nothing
1291  > FINDING | line=9 | class=ordering-bug | compile=YES | will not build
1292FINDING | class=prose-clarity | compile=no | no line at all
1293FINDING | line=0 | class=prose-clarity | compile=no | line zero is not a line
1294here is some prose the model added";
1295        let parsed = parse_findings(SHA, "src/a.rs", reply);
1296        assert_eq!(parsed.findings.len(), 2, "{:?}", parsed.findings);
1297        assert_eq!(parsed.findings[0].line, 7);
1298        assert!(
1299            parsed.findings[1].claims_compile_failure,
1300            "compile= is case-insensitive"
1301        );
1302        // Both unanchored forms are counted, not silently dropped.
1303        assert_eq!(parsed.unparsed.len(), 2, "{:?}", parsed.unparsed);
1304        // Prose that is not an attempted finding is not counted as a failure.
1305        assert!(!parsed.unparsed.iter().any(|u| u.contains("here is some")));
1306    }
1307
1308    /// **Bold inside a field must still parse; bold inside a description must
1309    /// still be there afterwards.** These pull in opposite directions and the two
1310    /// halves of this test are the reason the strip is per-field rather than
1311    /// per-line.
1312    ///
1313    /// Stripping only the line's ends would be the tidy-looking fix and would
1314    /// regress the first half: `class=**contract-drift**` stops resolving and the
1315    /// finding lands unclassified. Stripping the whole line — what this replaces —
1316    /// buys the first half by silently editing the description a human reads.
1317    #[test]
1318    fn bold_is_stripped_from_fields_and_left_in_descriptions() {
1319        let reply = "\
1320FINDING | **line**=42 | class=**contract-drift** | **compile**=YES | the **remote** path is not gated
1321**NO FINDINGS**";
1322        let parsed = parse_findings(SHA, "src/a.rs", reply);
1323        assert_eq!(parsed.findings.len(), 1, "{:?}", parsed.unparsed);
1324
1325        let f = &parsed.findings[0];
1326        assert_eq!(f.line, 42, "a bolded key still names the line field");
1327        assert_eq!(
1328            f.defect_class,
1329            DefectClass::from_token("contract-drift"),
1330            "a bolded value still resolves to its class"
1331        );
1332        assert!(
1333            f.claims_compile_failure,
1334            "a bolded key still names `compile`"
1335        );
1336        assert_eq!(
1337            f.description, "the **remote** path is not gated",
1338            "the model's emphasis is the model's; the parser does not edit prose \
1339             it is about to report"
1340        );
1341
1342        // Whole-line bold is decoration and is closed at the ends, so the clean
1343        // declaration is still recognised rather than read as unparsed prose.
1344        assert!(parsed.declared_clean);
1345    }
1346
1347    /// **"Found nothing" and "ignored the format" are opposite facts.** A run that
1348    /// cannot tell them apart cannot tell a bad model from a bad prompt.
1349    #[test]
1350    fn a_clean_declaration_is_distinguishable_from_silence() {
1351        let clean = parse_findings(SHA, "src/a.rs", NO_FINDINGS);
1352        assert!(clean.declared_clean);
1353        assert!(clean.findings.is_empty() && clean.unparsed.is_empty());
1354        assert!(!clean.reasoning_truncated);
1355
1356        let waffle = parse_findings(SHA, "src/a.rs", "I reviewed the file and it looks fine.");
1357        assert!(
1358            !waffle.declared_clean,
1359            "prose is not the declaration the format requires"
1360        );
1361    }
1362
1363    /// **A reviewer cut off mid-deliberation must not read as a clean file.**
1364    /// Measured on `qwen3.8-27b`: a reasoning GGUF opens `<think>`, and if the
1365    /// token cap lands before `</think>` the reply carries no finding and no
1366    /// declaration — which counts identically to a careful pass unless something
1367    /// says otherwise. That is the same silent zero the corpus's `reviewed_sha`
1368    /// rule exists to prevent, arriving from a third direction.
1369    #[test]
1370    fn a_reply_cut_off_inside_a_reasoning_block_is_not_a_clean_file() {
1371        let cut = parse_findings(
1372            SHA,
1373            "src/a.rs",
1374            "<think>\nLet me check the doc against the code. Line 12 says the cache is\n\
1375             unbounded, and the insert path",
1376        );
1377        assert!(
1378            cut.reasoning_truncated,
1379            "an unterminated block is truncation"
1380        );
1381        assert!(!cut.declared_clean, "and it is emphatically not clean");
1382        assert!(cut.findings.is_empty());
1383
1384        // A block the model actually closed is a normal answer; the caller strips
1385        // it before parsing, so nothing here should fire.
1386        let finished = parse_findings(SHA, "src/a.rs", NO_FINDINGS);
1387        assert!(!finished.reasoning_truncated);
1388    }
1389
1390    /// An unknown class is dropped to `None` rather than failing the finding:
1391    /// recall is about the defect, and `review_score` already reports
1392    /// misclassification separately.
1393    #[test]
1394    fn an_unknown_class_does_not_cost_the_finding() {
1395        let parsed = parse_findings(
1396            SHA,
1397            "src/a.rs",
1398            "FINDING | line=3 | class=off-by-one | compile=no | oops",
1399        );
1400        assert_eq!(parsed.findings.len(), 1);
1401        assert_eq!(parsed.findings[0].defect_class, None);
1402    }
1403
1404    /// A description containing a pipe survives, because the alternative is a
1405    /// human handed a bare line number.
1406    #[test]
1407    fn a_description_may_contain_the_separator() {
1408        let parsed = parse_findings(
1409            SHA,
1410            "src/a.rs",
1411            "FINDING | line=3 | class=prose-clarity | compile=no | says a | b but means a",
1412        );
1413        assert_eq!(parsed.findings[0].description, "says a | b but means a");
1414    }
1415
1416    /// **The #291 shape, derived rather than assumed.** A file with macOS-gated
1417    /// code yields a site no job in this ubuntu-only CI covers, so a compile claim
1418    /// about it survives a wholly green build.
1419    #[test]
1420    fn a_macos_gated_file_yields_an_unrefutable_site() {
1421        let source = "#[cfg(target_os = \"macos\")]\nfn teardown() {}\n";
1422        let site = claim_site(SHA, "crates/rto-llama/src/backend.rs", 2, source, None);
1423        assert_eq!(site.platform, Some(TargetOs::MacOs));
1424        assert!(!suppression(&site, &ci()).is_refuted());
1425
1426        // The same file without the gate is ordinary library code, and a green
1427        // all-features job does refute it — so the filter is not simply off.
1428        let plain = claim_site(
1429            SHA,
1430            "crates/rto-llama/src/backend.rs",
1431            2,
1432            "fn t() {}\n",
1433            None,
1434        );
1435        assert_eq!(plain.platform, None);
1436        assert!(suppression(&plain, &ci()).is_refuted());
1437    }
1438
1439    /// A `#[cfg(test)] mod tests` at the foot of a source file must not mark the
1440    /// library code above it as test code — that would establish a requirement
1441    /// `msrv` cannot meet on nearly every file here and disable the filter
1442    /// wholesale.
1443    #[test]
1444    fn test_code_is_decided_per_line_not_per_file() {
1445        let source = "fn real() {}\n#[cfg(test)]\nmod tests {\n    fn t() {}\n}\n";
1446        let above = claim_site(SHA, "crates/rto-graph/src/lib.rs", 1, source, None);
1447        assert!(!above.is_test_code);
1448        let below = claim_site(SHA, "crates/rto-graph/src/lib.rs", 4, source, None);
1449        assert!(below.is_test_code);
1450
1451        // An integration-test path is test code at any line.
1452        let integration = claim_site(SHA, "crates/rto-graph/tests/review_corpus.rs", 1, "", None);
1453        assert!(integration.is_test_code);
1454    }
1455
1456    /// **The `boxlite.rs` row of the corpus, derived.** Its module is
1457    /// `#[cfg(feature = "exec-boxlite")]` in the parent, so only an
1458    /// `--all-features` job compiles it — which `compile_claim`'s own licence test
1459    /// asserts by hand and this derives from bytes.
1460    #[test]
1461    fn a_feature_gated_module_needs_an_all_features_job() {
1462        let parent = "pub mod subprocess;\n#[cfg(feature = \"exec-boxlite\")]\npub mod boxlite;\n";
1463        let site = claim_site(
1464            SHA,
1465            "crates/rto-exec/src/boxlite.rs",
1466            10,
1467            "fn run() {}\n",
1468            Some(parent),
1469        );
1470        assert_eq!(site.features, Some(Features::All));
1471
1472        // Its ungated sibling establishes nothing, so it is not accidentally
1473        // narrowed to the all-features jobs.
1474        let sibling = claim_site(
1475            SHA,
1476            "crates/rto-exec/src/subprocess.rs",
1477            10,
1478            "fn run() {}\n",
1479            Some(parent),
1480        );
1481        assert_eq!(sibling.features, None);
1482    }
1483
1484    /// **A `mod.rs` is declared by its directory name, not by `mod mod;`.**
1485    ///
1486    /// Latent in this repository rather than live: it has exactly two `mod.rs`
1487    /// files, both under `tests/`, and no `src/**/mod.rs` at all — so no compile
1488    /// claim here has ever taken this path. It is fixed anyway because the failure
1489    /// is permissive. Taking the stem searches for `mod mod;`, never matches, and
1490    /// yields `features: None`, which reads as *unconditional* — a feature-gated
1491    /// module would have its compile claims suppressed by a job that never
1492    /// compiled it. That is the #291 shape: a build reporting coverage it did not
1493    /// have. The reviewer is also scored against a 190-path corpus and pointed at
1494    /// other repositories, where `src/**/mod.rs` is ordinary.
1495    #[test]
1496    fn a_mod_rs_is_gated_by_the_declaration_of_its_directory() {
1497        let parent = "#[cfg(feature = \"serve\")]\npub mod thing;\n";
1498        let site = claim_site(
1499            SHA,
1500            "crates/x/src/thing/mod.rs",
1501            10,
1502            "fn run() {}\n",
1503            Some(parent),
1504        );
1505        assert_eq!(
1506            site.features,
1507            Some(Features::All),
1508            "`thing/mod.rs` is declared by `mod thing;`, so the gate on it applies"
1509        );
1510
1511        // The stem-based lookup this replaces would find nothing and establish
1512        // nothing, which is the permissive answer rather than a missing one.
1513        let ungated = claim_site(
1514            SHA,
1515            "crates/x/src/other/mod.rs",
1516            10,
1517            "fn run() {}\n",
1518            Some(parent),
1519        );
1520        assert_eq!(
1521            ungated.features, None,
1522            "the gate governs `thing`, not every `mod.rs`"
1523        );
1524
1525        // A `mod.rs` with no directory above it is declared by nothing.
1526        assert_eq!(
1527            claim_site(SHA, "mod.rs", 1, "", Some(parent)).features,
1528            None
1529        );
1530    }
1531
1532    /// An attribute belonging to a different item must not be read as this
1533    /// module's gate — the walk stops at the first non-attribute line.
1534    #[test]
1535    fn a_gate_on_another_item_is_not_borrowed() {
1536        let parent = "#[cfg(feature = \"serve\")]\npub mod served;\n\npub mod plain;\n";
1537        let site = claim_site(SHA, "crates/x/src/plain.rs", 1, "", Some(parent));
1538        assert_eq!(
1539            site.features, None,
1540            "the gate governs `served`, not `plain`"
1541        );
1542    }
1543
1544    /// `len / 4` is the basis every budget number in this stage is quoted on.
1545    #[test]
1546    fn token_estimation_is_the_documented_basis() {
1547        assert_eq!(estimate_tokens(&"a".repeat(400)), 100);
1548        assert_eq!(estimate_tokens(""), 0);
1549    }
1550
1551    /// This repository's compiling jobs, as `compile_claim`'s own tests model
1552    /// them: all ubuntu, only `checks`/`default-features` with `--all-targets`.
1553    fn ci() -> Vec<CheckRun> {
1554        vec![
1555            CheckRun {
1556                job: "msrv".to_owned(),
1557                sha: SHA.to_owned(),
1558                conclusion: Conclusion::Success,
1559                toolchain: "1.94".to_owned(),
1560                platform: TargetOs::Linux,
1561                features: Features::All,
1562                targets: Targets::LibsAndBins,
1563            },
1564            CheckRun {
1565                job: "checks".to_owned(),
1566                sha: SHA.to_owned(),
1567                conclusion: Conclusion::Success,
1568                toolchain: "stable".to_owned(),
1569                platform: TargetOs::Linux,
1570                features: Features::All,
1571                targets: Targets::AllTargets,
1572            },
1573        ]
1574    }
1575}