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        reasoning_truncated: reply.contains("<think>"),
667        ..Parsed::default()
668    };
669    for raw in reply.lines() {
670        let line = raw.trim().trim_start_matches(['-', '*', '>', '#', ' ']);
671        let line = line.trim_start_matches('`').trim_end();
672        // Whole-line decoration only. The leading trim above has already taken any
673        // opening `**`, so this closes the pair — for `**NO FINDINGS**`, and for a
674        // finding line a model has bolded end to end. Bold *inside* the line is
675        // deliberately left alone here and dealt with per-field in `parse_one`;
676        // see the note there for why the difference matters.
677        let line = line.strip_suffix("**").unwrap_or(line).trim();
678        if line.eq_ignore_ascii_case(NO_FINDINGS) {
679            out.declared_clean = true;
680            continue;
681        }
682        if !line
683            .get(..FINDING_PREFIX.len())
684            .is_some_and(|p| p.eq_ignore_ascii_case(FINDING_PREFIX))
685        {
686            continue;
687        }
688        match parse_one(reviewed_sha, path, line) {
689            Some(finding) => out.findings.push(finding),
690            None => out.unparsed.push(line.to_owned()),
691        }
692    }
693    out
694}
695
696/// Parse one `FINDING | …` line, or `None` if it carries no usable line number.
697fn parse_one(reviewed_sha: &str, path: &str, line: &str) -> Option<CandidateFinding> {
698    let mut number: Option<u32> = None;
699    let mut class = None;
700    let mut claims_compile_failure = false;
701    let mut description = String::new();
702
703    for field in line.split('|').skip(1) {
704        let field = field.trim().trim_end_matches('`').trim();
705        // **Bold is stripped for the structural read only, and only where it
706        // resolves to a field this format defines.**
707        //
708        // The line-wide `replace("**", "")` this replaces was not there to handle
709        // *leading* bold — the trim in `parse_findings` already takes that. It was
710        // there for bold *inside* the line, i.e. a model emitting
711        // `class=**contract-drift**` or `**line**=42`, which has still followed the
712        // format and must still parse. But applying it to the whole line also
713        // rewrote the description, so emphasis a model put in prose vanished from
714        // the text a human is shown and a corpus is scored against.
715        //
716        // Splitting the two reads keeps the field tolerance and drops the
717        // rewriting: `structural` exists only to decide what this token *is*, and
718        // if the answer is "not a known field" the original bytes go through
719        // untouched.
720        let structural = field.replace("**", "");
721        let key_value = structural
722            .split_once('=')
723            .map(|(k, v)| (k.trim().to_ascii_lowercase(), v.trim()));
724
725        match key_value.as_ref().map(|(k, v)| (k.as_str(), *v)) {
726            Some(("line", value)) => number = value.parse().ok().filter(|n| *n > 0),
727            Some(("class", value)) => class = DefectClass::from_token(&value.to_ascii_lowercase()),
728            Some(("compile", value)) => {
729                claims_compile_failure = matches!(
730                    value.to_ascii_lowercase().as_str(),
731                    "yes" | "true" | "y" | "1"
732                );
733            }
734            // Everything else is description, kept verbatim. A field with no `=`
735            // is the description proper; later ones are its continuation, because
736            // a description may itself contain a pipe. An *unknown* `key=value` is
737            // kept as prose rather than dropped: it is more likely a description
738            // containing an `=` than an invented field, and losing it would leave
739            // a human a bare line number.
740            _ => {
741                if !description.is_empty() {
742                    description.push_str(" | ");
743                }
744                description.push_str(field);
745            }
746        }
747    }
748
749    Some(CandidateFinding {
750        reviewed_sha: reviewed_sha.to_owned(),
751        path: path.to_owned(),
752        line: number?,
753        description: if description.trim().is_empty() {
754            "(no description given)".to_owned()
755        } else {
756            description.trim().to_owned()
757        },
758        claims_compile_failure,
759        defect_class: class,
760    })
761}
762
763/// Derive the [`ClaimSite`] for a compile claim, from the reviewed file's bytes.
764///
765/// `parent_source` is the module's parent (`lib.rs`/`mod.rs`) when the caller has
766/// it, which is the only place a file's feature gate is written.
767///
768/// # Conservative on every axis, deliberately
769///
770/// [`crate::compile_claim`] states the asymmetry this follows: a claim wrongly
771/// suppressed is a defect shipped silently — the #291 macOS teardown shape — while
772/// a claim wrongly kept costs a human one look at a CI page. So every derivation
773/// here errs toward *establishing a requirement*, which makes a site harder to
774/// refute, never easier:
775///
776/// * **Platform.** Any `cfg(target_os = "macos"/"windows")` anywhere in the file
777///   marks the whole file as needing that platform, so no job in this
778///   ubuntu-only CI can refute a claim about it. Coarse, and coarse in the safe
779///   direction: a file with one macOS-gated function keeps its compile claims.
780/// * **Features.** A `#[cfg(feature = …)]` on the module's declaration in
781///   `parent_source` marks the site as needing [`Features::All`], so only an
782///   `--all-features` job covers it. Without `parent_source` nothing is
783///   established, which is the one axis where "unknown" reads as unconditional —
784///   stated here rather than left for a reader to infer from the field docs.
785/// * **Targets.** A path under `tests/`, `benches/` or `examples/` is test code;
786///   so is a line *after* a `#[cfg(test)]` attribute in the file. Both need a job
787///   that passed `--all-targets`, which `msrv` does not.
788#[must_use]
789pub fn claim_site(
790    reviewed_sha: &str,
791    path: &str,
792    line: u32,
793    source: &str,
794    parent_source: Option<&str>,
795) -> ClaimSite {
796    ClaimSite {
797        platform: required_platform(source),
798        features: parent_source.and_then(|p| module_feature_gate(path, p)),
799        is_test_code: is_test_code(path, line, source),
800        toolchain: None,
801        ..ClaimSite::unknown(reviewed_sha, path)
802    }
803}
804
805/// The platform a file's `cfg` gates require, if it names one.
806fn required_platform(source: &str) -> Option<TargetOs> {
807    // macOS first: it is the platform this repository actually has uncompiled
808    // code for, and the one #291 shipped a defect behind.
809    for (needle, os) in [
810        ("target_os = \"macos\"", TargetOs::MacOs),
811        ("target_os = \"windows\"", TargetOs::Windows),
812    ] {
813        if source.contains(needle) {
814            return Some(os);
815        }
816    }
817    None
818}
819
820/// Whether `line` is test code: a test-only path, or a line after a
821/// `#[cfg(test)]` attribute.
822fn is_test_code(path: &str, line: u32, source: &str) -> bool {
823    if ["tests/", "benches/", "examples/"]
824        .iter()
825        .any(|d| path.starts_with(d) || path.contains(&format!("/{d}")))
826    {
827        return true;
828    }
829    // The line-relative form, so a `#[cfg(test)] mod tests` at the bottom of a
830    // source file does not mark the library code above it as test code — which
831    // would disable the filter on nearly every file in this repository.
832    source
833        .lines()
834        .position(|l| l.trim_start().starts_with("#[cfg(test)]"))
835        .is_some_and(|idx| line as usize > idx + 1)
836}
837
838/// The name a file is declared under by the `mod` item in its parent.
839///
840/// For `a/b/thing.rs` that is the file stem, `thing`. For `a/b/mod.rs` it is the
841/// *directory* name, `b`: a `mod.rs` is declared by `mod b;` in `a`'s source, and
842/// never by `mod mod;`.
843///
844/// # Why this is spelled out rather than left as `file_stem`
845///
846/// Taking the stem for a `mod.rs` searches the parent for `mod mod;`, which
847/// cannot match, so the lookup returns `None` — and a `None` on the features axis
848/// reads as *unconditional*, i.e. covered by any green job. That is the
849/// permissive direction: a feature-gated module would have its compile claims
850/// suppressed by a job that never compiled it. [`claim_site`]'s contract is that
851/// every derivation errs toward establishing a requirement, so this one case has
852/// to be got right rather than left to fall through.
853fn declaring_name(path: &str) -> Option<&str> {
854    let stem = path.rsplit('/').next()?.strip_suffix(".rs")?;
855    if stem == "mod" {
856        // `a/b/mod.rs` is declared as `b`; a bare `mod.rs` with no directory
857        // above it is declared by nothing, so there is no name to look for.
858        path.rsplit('/').nth(1)
859    } else {
860        Some(stem)
861    }
862}
863
864/// The feature gate on this file's `mod` declaration in its parent, if any.
865///
866/// Returns [`Features::All`] rather than naming the feature: the coverage model
867/// asks which *job* compiled the code, and this repository's jobs are
868/// `--all-features`, the default set, or nothing. Which named feature it is does
869/// not change the answer.
870///
871/// # Known gap: `#[path = "…"]`
872///
873/// A module declared `#[path = "elsewhere.rs"] mod name;` is not found by this
874/// lookup, because the declaring name cannot be recovered from the file path at
875/// all — the mapping lives in the attribute, in a parent this function is not
876/// given a way to search for. The result is `None`, which is the permissive
877/// direction, so this is a real (if narrow) hole rather than a tidy limitation.
878/// It is left open deliberately: closing it means scanning candidate parents for
879/// `#[path]` attributes and resolving them relative to the declaring file, which
880/// is a different shape of change from this one. This repository contains no
881/// `#[path]` attributes, so nothing here relies on it today.
882fn module_feature_gate(path: &str, parent_source: &str) -> Option<Features> {
883    let stem = declaring_name(path)?;
884    let lines: Vec<&str> = parent_source.lines().collect();
885    let decl = lines.iter().position(|l| {
886        let t = l.trim_start().trim_start_matches("pub ").trim_start();
887        t.starts_with(&format!("mod {stem};")) || t.starts_with(&format!("mod {stem} "))
888    })?;
889    // Walk back over the declaration's own attributes only, stopping at the first
890    // line that is not one — attributes bind to what follows them, so a `cfg` two
891    // items up governs a different item.
892    for above in lines[..decl].iter().rev() {
893        let t = above.trim();
894        if t.is_empty() || t.starts_with("//") {
895            continue;
896        }
897        if !t.starts_with("#[") {
898            break;
899        }
900        if t.contains("cfg(feature") || t.contains("cfg(all(feature") {
901            return Some(Features::All);
902        }
903    }
904    None
905}
906
907#[cfg(test)]
908mod tests {
909    use super::{
910        FileUnderReview, GraphContext, NO_FINDINGS, Prompt, SINGLE_CALL_BUDGET_TOKENS,
911        annotate_diff, build_prompt, claim_site, class_gloss, estimate_tokens, parse_findings,
912    };
913    use crate::compile_claim::{CheckRun, Conclusion, Features, TargetOs, Targets, suppression};
914    use crate::review_corpus::{CLASSES, DefectClass};
915
916    const SHA: &str = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
917
918    fn file(diff: &str) -> FileUnderReview {
919        FileUnderReview {
920            reviewed_sha: SHA.to_owned(),
921            path: "crates/rto-graph/src/lib.rs".to_owned(),
922            diff: diff.to_owned(),
923        }
924    }
925
926    /// **The line column is the reviewer's whole anchoring story**, so it must be
927    /// exact: a finding is credited only within `LINE_WINDOW` of a corpus row, and
928    /// a model asked to do hunk arithmetic instead would miss by more than that
929    /// and be scored as blind rather than as bad at counting.
930    #[test]
931    fn the_annotated_diff_numbers_the_new_side_exactly() {
932        let diff = "@@ -10,3 +20,4 @@ fn thing()\n unchanged\n-gone\n+added\n+also\n context\n";
933        let out = annotate_diff(diff);
934        let numbered: Vec<(u32, String)> = out
935            .lines()
936            .filter_map(|l| {
937                let (num, body) = l.split_once('|')?;
938                let n: u32 = num
939                    .trim()
940                    .trim_end_matches(['+', '-'])
941                    .trim()
942                    .parse()
943                    .ok()?;
944                Some((n, body.to_owned()))
945            })
946            .collect();
947        assert_eq!(
948            numbered,
949            vec![
950                (20, "unchanged".to_owned()),
951                (21, "added".to_owned()),
952                (22, "also".to_owned()),
953                (23, "context".to_owned()),
954            ],
955            "new-side numbering starts at the hunk's + start and skips removals"
956        );
957        // A removed line is shown but carries no citable number.
958        assert!(out.contains("      - |gone"), "{out}");
959    }
960
961    /// A diff with several hunks restarts numbering at each header rather than
962    /// counting straight through — the failure that would put every finding after
963    /// the first hunk out of window.
964    #[test]
965    fn numbering_restarts_at_each_hunk() {
966        let diff = "@@ -1,2 +1,2 @@\n a\n b\n@@ -50,2 +90,2 @@\n c\n d\n";
967        let out = annotate_diff(diff);
968        assert!(out.contains("     1  |a"), "{out}");
969        assert!(out.contains("    90  |c"), "{out}");
970        assert!(out.contains("    91  |d"), "{out}");
971    }
972
973    /// Every class the corpus can score is described to the model. A class with no
974    /// gloss is a class the reviewer was never told to look for, which would be
975    /// measured as a recall failure rather than as the omission it is.
976    #[test]
977    fn class_gloss_covers_every_class() {
978        for class in CLASSES {
979            let gloss = class_gloss(class);
980            assert!(!gloss.is_empty(), "{class} has no gloss");
981        }
982        let prompt = build_prompt(&file("@@ -1 +1 @@\n+x\n"), &GraphContext::none(), 30_000);
983        for class in CLASSES {
984            assert!(
985                prompt.text.contains(class.as_str()),
986                "{class} is not named in the prompt"
987            );
988        }
989    }
990
991    /// A helper for the budget tests: an item whose body is `chars` bytes long,
992    /// so its cost is predictable in `len / 4` terms.
993    fn item(label: &str, bytes: usize) -> super::ContextItem {
994        super::ContextItem {
995            label: label.to_owned(),
996            provenance: "authored".to_owned(),
997            body: "x".repeat(bytes),
998        }
999    }
1000
1001    /// **The cap is on the block, and it is relative to the diff.** A two-line
1002    /// change must not arrive under four thousand tokens of ADR: the whole risk
1003    /// this arm carries is that context drowns the change, and the guard against
1004    /// it is arithmetic rather than judgement.
1005    #[test]
1006    fn context_is_capped_relative_to_the_diff_it_accompanies() {
1007        // A 100-token diff admits at most 200 tokens of context, so the second
1008        // 150-token item cannot join the first.
1009        let fitted = GraphContext::fit(vec![item("a", 600), item("b", 600)], 100);
1010        assert_eq!(
1011            fitted.items.len(),
1012            1,
1013            "two 150-token items fit a 200-token cap"
1014        );
1015        assert_eq!(fitted.dropped_items, 1);
1016        assert!(
1017            fitted.tokens() <= 200,
1018            "cap breached: {} tokens",
1019            fitted.tokens()
1020        );
1021    }
1022
1023    /// The absolute ceiling binds even when the diff is enormous, so a huge file
1024    /// cannot pull in a proportionally huge context.
1025    #[test]
1026    fn the_absolute_cap_binds_on_a_large_diff() {
1027        // 20k diff tokens would allow 40k by the relative rule alone.
1028        let fitted = GraphContext::fit(
1029            (0..20).map(|i| item(&format!("adr-{i}"), 2_000)).collect(),
1030            20_000,
1031        );
1032        assert!(
1033            fitted.tokens() <= super::CONTEXT_CAP_TOKENS,
1034            "the absolute cap did not bind: {} tokens",
1035            fitted.tokens()
1036        );
1037        assert!(
1038            fitted.dropped_items > 0,
1039            "nothing was dropped, so nothing was capped"
1040        );
1041    }
1042
1043    /// **Whole items only.** A half-quoted ADR reads as a complete statement of a
1044    /// decision and is not one, so a model can be handed a promise whose exception
1045    /// was cut off. This asserts the bodies come through byte-identical.
1046    #[test]
1047    fn fitting_never_truncates_an_item_it_keeps() {
1048        let original = item("adr", 400);
1049        let fitted = GraphContext::fit(vec![original.clone(), item("big", 100_000)], 1_000);
1050        assert_eq!(fitted.items, vec![original], "a kept item was rewritten");
1051        assert_eq!(fitted.dropped_items, 1);
1052    }
1053
1054    /// A large item that does not fit must not also discard the small ones behind
1055    /// it — the cap is a budget, not a stopping point.
1056    #[test]
1057    fn an_oversized_item_does_not_evict_the_smaller_ones_after_it() {
1058        let fitted = GraphContext::fit(vec![item("huge", 100_000), item("small", 40)], 1_000);
1059        assert_eq!(fitted.dropped_items, 1);
1060        assert_eq!(
1061            fitted.items.len(),
1062            1,
1063            "the small item behind an oversized one was lost"
1064        );
1065        assert_eq!(fitted.items[0].label, "small");
1066    }
1067
1068    /// An empty diff admits no context at all, which is the conservative
1069    /// direction: `min(cap, 2 * 0) == 0`.
1070    #[test]
1071    fn an_empty_diff_admits_no_context() {
1072        let fitted = GraphContext::fit(vec![item("adr", 40)], 0);
1073        assert!(fitted.is_empty());
1074        assert_eq!(fitted.dropped_items, 1, "the drop must still be counted");
1075    }
1076
1077    /// [`ContextItem::tokens`] must charge for the heading, not just the body —
1078    /// otherwise a context of many tiny items is billed as nearly free while the
1079    /// prompt pays for every `--- label [provenance]` line.
1080    #[test]
1081    fn an_item_is_charged_for_its_heading_as_well_as_its_body() {
1082        let bare = super::ContextItem {
1083            label: String::new(),
1084            provenance: String::new(),
1085            body: "x".repeat(40),
1086        };
1087        let labelled = super::ContextItem {
1088            label: "ADR-0019 §3 governs `resolve`".to_owned(),
1089            provenance: "authored".to_owned(),
1090            body: "x".repeat(40),
1091        };
1092        assert!(
1093            labelled.tokens() > bare.tokens(),
1094            "the heading was not charged: {} vs {}",
1095            labelled.tokens(),
1096            bare.tokens()
1097        );
1098    }
1099
1100    /// A section runs to the next `## `, and a deeper heading inside it is body.
1101    #[test]
1102    fn a_section_body_stops_at_the_next_sibling_heading() {
1103        let md = "# ADR-0005\n\n## Context\nwhy\n\n## Decision\nthe rule\n\n### Detail\nmore\n\n## Consequences\nafter\n";
1104        assert_eq!(
1105            super::section_body(md, "Decision").as_deref(),
1106            Some("the rule\n\n### Detail\nmore"),
1107            "a `###` subheading must not end the section"
1108        );
1109        assert_eq!(super::section_body(md, "Context").as_deref(), Some("why"));
1110        assert_eq!(super::section_body(md, "Absent"), None);
1111    }
1112
1113    /// **The title is matched verbatim, because a slug rule would be a second
1114    /// copy of one this crate cannot see.** Punctuation that a slug would collapse
1115    /// must still resolve here.
1116    #[test]
1117    fn a_heading_with_punctuation_resolves_without_a_slug_rule() {
1118        let md = "## Options considered + consequences\nbody\n\n## Next\nx\n";
1119        assert_eq!(
1120            super::section_body(md, "Options considered + consequences").as_deref(),
1121            Some("body")
1122        );
1123    }
1124
1125    /// A doc comment already visible in the hunks is not worth re-sending: the
1126    /// context block's whole value is the half the diff does *not* show.
1127    #[test]
1128    fn a_doc_already_in_the_diff_is_not_re_quoted() {
1129        let doc = "Returns the cache entry for `key`, evicting the least recently used entry when the cache is full.";
1130        let shown = format!("   12  |/// {doc}\n   13  |pub fn get(&self) {{}}\n");
1131        assert!(
1132            super::doc_already_shown(doc, &shown),
1133            "the doc is in the diff and was not recognised"
1134        );
1135        assert!(
1136            !super::doc_already_shown(doc, "   12  |pub fn unrelated() {}\n"),
1137            "a doc absent from the diff was treated as shown"
1138        );
1139    }
1140
1141    /// The diff carries a line-number column and `+`/` ` markers the graph's copy
1142    /// of the same doc does not, so the comparison must ignore whitespace — this
1143    /// is the case that a naive `contains` gets wrong.
1144    #[test]
1145    fn the_visibility_test_ignores_the_line_number_column() {
1146        let doc = "The slot lock is held only long enough to hand out an `Arc`, never across initialisation.";
1147        // Wrapped across lines and numbered, exactly as `annotate_diff` renders it.
1148        let shown = "    16 +|/// The slot lock is held only long enough to hand\n    17 +|/// out an `Arc`, never across initialisation.\n";
1149        assert!(
1150            super::doc_already_shown(doc, shown),
1151            "wrapping and numbering defeated the visibility test"
1152        );
1153    }
1154
1155    /// A doc too short to state a contract is treated as already shown, so the
1156    /// context block is not padded with one-word comments that cannot drift.
1157    #[test]
1158    fn a_doc_too_short_to_state_a_contract_is_never_carried() {
1159        assert!(super::doc_already_shown("The key.", "unrelated diff text"));
1160    }
1161
1162    /// **PR 1's arm is diff-only, and the prompt must say nothing else.** An empty
1163    /// [`GraphContext`] renders no context heading at all, so the baseline is not
1164    /// quietly a reviewer told it has context and given none — which is a
1165    /// different prompt, and would make the two arms differ by more than the
1166    /// context.
1167    #[test]
1168    fn an_empty_context_renders_no_context_section() {
1169        let bare = build_prompt(&file("@@ -1 +1 @@\n+x\n"), &GraphContext::none(), 30_000);
1170        assert!(!bare.text.contains("Context from"), "{}", bare.text);
1171        assert!(
1172            !bare.text.contains("[authored]") && !bare.text.contains("[derived]"),
1173            "no provenance labels without context: {}",
1174            bare.text
1175        );
1176
1177        let with = build_prompt(
1178            &file("@@ -1 +1 @@\n+x\n"),
1179            &GraphContext {
1180                items: vec![super::ContextItem {
1181                    label: "ADR-0019 §3".to_owned(),
1182                    provenance: "authored".to_owned(),
1183                    body: "the user layer alone never suffices".to_owned(),
1184                }],
1185                dropped_items: 0,
1186            },
1187            30_000,
1188        );
1189        assert!(with.text.contains("Context from"), "{}", with.text);
1190        assert!(with.text.contains("ADR-0019 §3"), "{}", with.text);
1191        assert!(
1192            with.text.contains("[authored]"),
1193            "provenance travels with the item: {}",
1194            with.text
1195        );
1196    }
1197
1198    /// The prompt fits its budget, and says so when it could not fit the diff.
1199    #[test]
1200    fn a_prompt_respects_its_budget_and_reports_what_it_dropped() {
1201        let big = format!(
1202            "@@ -1,1 +1,{0} @@\n{}",
1203            "+a line of code here\n".repeat(20_000)
1204        );
1205        let f = file(&big);
1206        let Prompt {
1207            text,
1208            tokens,
1209            dropped_tokens,
1210        } = build_prompt(&f, &GraphContext::none(), 8_000);
1211        assert!(tokens <= 8_000, "over budget: {tokens}");
1212        assert!(dropped_tokens > 0, "a 20k-line diff cannot have fit");
1213        assert!(
1214            text.contains("truncated to fit"),
1215            "the model is told it is seeing part of a file"
1216        );
1217
1218        // And the common case: nothing dropped, nothing claimed.
1219        let small = build_prompt(&file("@@ -1 +1 @@\n+x\n"), &GraphContext::none(), 30_000);
1220        assert_eq!(small.dropped_tokens, 0);
1221        assert!(!small.text.contains("truncated"));
1222    }
1223
1224    /// **The headroom the graph arm depends on, asserted rather than assumed.**
1225    ///
1226    /// The largest reviewable source file-diff in the corpus is 14,034 raw tokens
1227    /// (`rto-graph/src/models.rs`), which [`annotate_diff`] takes to **17,202** —
1228    /// the numbering column costs a measured **1.21×** across all 190 file-diffs,
1229    /// because it adds a fixed 9 characters per *line* rather than a fraction of
1230    /// the bytes. Reconstructed here at that size and at this repository's line
1231    /// length, so a prompt change that eats the headroom fails here rather than
1232    /// showing up later as a worse score nobody can attribute.
1233    #[test]
1234    fn the_prompt_scaffolding_leaves_the_measured_headroom_intact() {
1235        // ~44 characters per line, this repository's rough average, so the
1236        // annotation overhead lands where it was measured rather than at the
1237        // 4× a two-character line would produce.
1238        let line = format!("+{}\n", "a".repeat(43));
1239        let worst = line.repeat(14_034 * 4 / 44);
1240        let f = file(&format!("@@ -1,1 +1,1 @@\n{worst}"));
1241        let p = build_prompt(&f, &GraphContext::none(), SINGLE_CALL_BUDGET_TOKENS);
1242        assert_eq!(
1243            p.dropped_tokens, 0,
1244            "the corpus's largest source file must not need truncating"
1245        );
1246        let headroom = SINGLE_CALL_BUDGET_TOKENS - p.tokens;
1247        assert!(
1248            headroom > 10_000,
1249            "only {headroom} tokens left for graph context on the worst source \
1250             file; the arm needs room to be testable at all"
1251        );
1252
1253        // And the median file, which is what the headroom claim is really about:
1254        // 1,758 annotated tokens leaves nearly the whole budget free.
1255        let median = file(&format!("@@ -1,1 +1,1 @@\n{}", line.repeat(1_476 * 4 / 44)));
1256        let p = build_prompt(&median, &GraphContext::none(), SINGLE_CALL_BUDGET_TOKENS);
1257        assert!(
1258            SINGLE_CALL_BUDGET_TOKENS - p.tokens > 25_000,
1259            "the median file should leave ~28k free, left {}",
1260            SINGLE_CALL_BUDGET_TOKENS - p.tokens
1261        );
1262    }
1263
1264    #[test]
1265    fn a_well_formed_finding_parses() {
1266        let reply = "FINDING | line=42 | class=contract-drift | compile=no | the doc says X";
1267        let parsed = parse_findings(SHA, "src/a.rs", reply);
1268        assert_eq!(parsed.findings.len(), 1);
1269        let f = &parsed.findings[0];
1270        assert_eq!(f.line, 42);
1271        assert_eq!(f.defect_class, Some(DefectClass::ContractDrift));
1272        assert!(!f.claims_compile_failure);
1273        assert_eq!(f.description, "the doc says X");
1274        assert!(parsed.unparsed.is_empty());
1275    }
1276
1277    /// A model that wraps the format in markdown has still followed it. Being
1278    /// strict here would measure formatting compliance and call it recall.
1279    #[test]
1280    fn presentation_is_tolerated_but_content_is_not() {
1281        let reply = "\
1282- **FINDING** | line=7 | class=vacuous-test | compile=no | asserts nothing
1283  > FINDING | line=9 | class=ordering-bug | compile=YES | will not build
1284FINDING | class=prose-clarity | compile=no | no line at all
1285FINDING | line=0 | class=prose-clarity | compile=no | line zero is not a line
1286here is some prose the model added";
1287        let parsed = parse_findings(SHA, "src/a.rs", reply);
1288        assert_eq!(parsed.findings.len(), 2, "{:?}", parsed.findings);
1289        assert_eq!(parsed.findings[0].line, 7);
1290        assert!(
1291            parsed.findings[1].claims_compile_failure,
1292            "compile= is case-insensitive"
1293        );
1294        // Both unanchored forms are counted, not silently dropped.
1295        assert_eq!(parsed.unparsed.len(), 2, "{:?}", parsed.unparsed);
1296        // Prose that is not an attempted finding is not counted as a failure.
1297        assert!(!parsed.unparsed.iter().any(|u| u.contains("here is some")));
1298    }
1299
1300    /// **Bold inside a field must still parse; bold inside a description must
1301    /// still be there afterwards.** These pull in opposite directions and the two
1302    /// halves of this test are the reason the strip is per-field rather than
1303    /// per-line.
1304    ///
1305    /// Stripping only the line's ends would be the tidy-looking fix and would
1306    /// regress the first half: `class=**contract-drift**` stops resolving and the
1307    /// finding lands unclassified. Stripping the whole line — what this replaces —
1308    /// buys the first half by silently editing the description a human reads.
1309    #[test]
1310    fn bold_is_stripped_from_fields_and_left_in_descriptions() {
1311        let reply = "\
1312FINDING | **line**=42 | class=**contract-drift** | **compile**=YES | the **remote** path is not gated
1313**NO FINDINGS**";
1314        let parsed = parse_findings(SHA, "src/a.rs", reply);
1315        assert_eq!(parsed.findings.len(), 1, "{:?}", parsed.unparsed);
1316
1317        let f = &parsed.findings[0];
1318        assert_eq!(f.line, 42, "a bolded key still names the line field");
1319        assert_eq!(
1320            f.defect_class,
1321            DefectClass::from_token("contract-drift"),
1322            "a bolded value still resolves to its class"
1323        );
1324        assert!(
1325            f.claims_compile_failure,
1326            "a bolded key still names `compile`"
1327        );
1328        assert_eq!(
1329            f.description, "the **remote** path is not gated",
1330            "the model's emphasis is the model's; the parser does not edit prose \
1331             it is about to report"
1332        );
1333
1334        // Whole-line bold is decoration and is closed at the ends, so the clean
1335        // declaration is still recognised rather than read as unparsed prose.
1336        assert!(parsed.declared_clean);
1337    }
1338
1339    /// **"Found nothing" and "ignored the format" are opposite facts.** A run that
1340    /// cannot tell them apart cannot tell a bad model from a bad prompt.
1341    #[test]
1342    fn a_clean_declaration_is_distinguishable_from_silence() {
1343        let clean = parse_findings(SHA, "src/a.rs", NO_FINDINGS);
1344        assert!(clean.declared_clean);
1345        assert!(clean.findings.is_empty() && clean.unparsed.is_empty());
1346        assert!(!clean.reasoning_truncated);
1347
1348        let waffle = parse_findings(SHA, "src/a.rs", "I reviewed the file and it looks fine.");
1349        assert!(
1350            !waffle.declared_clean,
1351            "prose is not the declaration the format requires"
1352        );
1353    }
1354
1355    /// **A reviewer cut off mid-deliberation must not read as a clean file.**
1356    /// Measured on `qwen3.8-27b`: a reasoning GGUF opens `<think>`, and if the
1357    /// token cap lands before `</think>` the reply carries no finding and no
1358    /// declaration — which counts identically to a careful pass unless something
1359    /// says otherwise. That is the same silent zero the corpus's `reviewed_sha`
1360    /// rule exists to prevent, arriving from a third direction.
1361    #[test]
1362    fn a_reply_cut_off_inside_a_reasoning_block_is_not_a_clean_file() {
1363        let cut = parse_findings(
1364            SHA,
1365            "src/a.rs",
1366            "<think>\nLet me check the doc against the code. Line 12 says the cache is\n\
1367             unbounded, and the insert path",
1368        );
1369        assert!(
1370            cut.reasoning_truncated,
1371            "an unterminated block is truncation"
1372        );
1373        assert!(!cut.declared_clean, "and it is emphatically not clean");
1374        assert!(cut.findings.is_empty());
1375
1376        // A block the model actually closed is a normal answer; the caller strips
1377        // it before parsing, so nothing here should fire.
1378        let finished = parse_findings(SHA, "src/a.rs", NO_FINDINGS);
1379        assert!(!finished.reasoning_truncated);
1380    }
1381
1382    /// An unknown class is dropped to `None` rather than failing the finding:
1383    /// recall is about the defect, and `review_score` already reports
1384    /// misclassification separately.
1385    #[test]
1386    fn an_unknown_class_does_not_cost_the_finding() {
1387        let parsed = parse_findings(
1388            SHA,
1389            "src/a.rs",
1390            "FINDING | line=3 | class=off-by-one | compile=no | oops",
1391        );
1392        assert_eq!(parsed.findings.len(), 1);
1393        assert_eq!(parsed.findings[0].defect_class, None);
1394    }
1395
1396    /// A description containing a pipe survives, because the alternative is a
1397    /// human handed a bare line number.
1398    #[test]
1399    fn a_description_may_contain_the_separator() {
1400        let parsed = parse_findings(
1401            SHA,
1402            "src/a.rs",
1403            "FINDING | line=3 | class=prose-clarity | compile=no | says a | b but means a",
1404        );
1405        assert_eq!(parsed.findings[0].description, "says a | b but means a");
1406    }
1407
1408    /// **The #291 shape, derived rather than assumed.** A file with macOS-gated
1409    /// code yields a site no job in this ubuntu-only CI covers, so a compile claim
1410    /// about it survives a wholly green build.
1411    #[test]
1412    fn a_macos_gated_file_yields_an_unrefutable_site() {
1413        let source = "#[cfg(target_os = \"macos\")]\nfn teardown() {}\n";
1414        let site = claim_site(SHA, "crates/rto-llama/src/backend.rs", 2, source, None);
1415        assert_eq!(site.platform, Some(TargetOs::MacOs));
1416        assert!(!suppression(&site, &ci()).is_refuted());
1417
1418        // The same file without the gate is ordinary library code, and a green
1419        // all-features job does refute it — so the filter is not simply off.
1420        let plain = claim_site(
1421            SHA,
1422            "crates/rto-llama/src/backend.rs",
1423            2,
1424            "fn t() {}\n",
1425            None,
1426        );
1427        assert_eq!(plain.platform, None);
1428        assert!(suppression(&plain, &ci()).is_refuted());
1429    }
1430
1431    /// A `#[cfg(test)] mod tests` at the foot of a source file must not mark the
1432    /// library code above it as test code — that would establish a requirement
1433    /// `msrv` cannot meet on nearly every file here and disable the filter
1434    /// wholesale.
1435    #[test]
1436    fn test_code_is_decided_per_line_not_per_file() {
1437        let source = "fn real() {}\n#[cfg(test)]\nmod tests {\n    fn t() {}\n}\n";
1438        let above = claim_site(SHA, "crates/rto-graph/src/lib.rs", 1, source, None);
1439        assert!(!above.is_test_code);
1440        let below = claim_site(SHA, "crates/rto-graph/src/lib.rs", 4, source, None);
1441        assert!(below.is_test_code);
1442
1443        // An integration-test path is test code at any line.
1444        let integration = claim_site(SHA, "crates/rto-graph/tests/review_corpus.rs", 1, "", None);
1445        assert!(integration.is_test_code);
1446    }
1447
1448    /// **The `boxlite.rs` row of the corpus, derived.** Its module is
1449    /// `#[cfg(feature = "exec-boxlite")]` in the parent, so only an
1450    /// `--all-features` job compiles it — which `compile_claim`'s own licence test
1451    /// asserts by hand and this derives from bytes.
1452    #[test]
1453    fn a_feature_gated_module_needs_an_all_features_job() {
1454        let parent = "pub mod subprocess;\n#[cfg(feature = \"exec-boxlite\")]\npub mod boxlite;\n";
1455        let site = claim_site(
1456            SHA,
1457            "crates/rto-exec/src/boxlite.rs",
1458            10,
1459            "fn run() {}\n",
1460            Some(parent),
1461        );
1462        assert_eq!(site.features, Some(Features::All));
1463
1464        // Its ungated sibling establishes nothing, so it is not accidentally
1465        // narrowed to the all-features jobs.
1466        let sibling = claim_site(
1467            SHA,
1468            "crates/rto-exec/src/subprocess.rs",
1469            10,
1470            "fn run() {}\n",
1471            Some(parent),
1472        );
1473        assert_eq!(sibling.features, None);
1474    }
1475
1476    /// **A `mod.rs` is declared by its directory name, not by `mod mod;`.**
1477    ///
1478    /// Latent in this repository rather than live: it has exactly two `mod.rs`
1479    /// files, both under `tests/`, and no `src/**/mod.rs` at all — so no compile
1480    /// claim here has ever taken this path. It is fixed anyway because the failure
1481    /// is permissive. Taking the stem searches for `mod mod;`, never matches, and
1482    /// yields `features: None`, which reads as *unconditional* — a feature-gated
1483    /// module would have its compile claims suppressed by a job that never
1484    /// compiled it. That is the #291 shape: a build reporting coverage it did not
1485    /// have. The reviewer is also scored against a 190-path corpus and pointed at
1486    /// other repositories, where `src/**/mod.rs` is ordinary.
1487    #[test]
1488    fn a_mod_rs_is_gated_by_the_declaration_of_its_directory() {
1489        let parent = "#[cfg(feature = \"serve\")]\npub mod thing;\n";
1490        let site = claim_site(
1491            SHA,
1492            "crates/x/src/thing/mod.rs",
1493            10,
1494            "fn run() {}\n",
1495            Some(parent),
1496        );
1497        assert_eq!(
1498            site.features,
1499            Some(Features::All),
1500            "`thing/mod.rs` is declared by `mod thing;`, so the gate on it applies"
1501        );
1502
1503        // The stem-based lookup this replaces would find nothing and establish
1504        // nothing, which is the permissive answer rather than a missing one.
1505        let ungated = claim_site(
1506            SHA,
1507            "crates/x/src/other/mod.rs",
1508            10,
1509            "fn run() {}\n",
1510            Some(parent),
1511        );
1512        assert_eq!(
1513            ungated.features, None,
1514            "the gate governs `thing`, not every `mod.rs`"
1515        );
1516
1517        // A `mod.rs` with no directory above it is declared by nothing.
1518        assert_eq!(
1519            claim_site(SHA, "mod.rs", 1, "", Some(parent)).features,
1520            None
1521        );
1522    }
1523
1524    /// An attribute belonging to a different item must not be read as this
1525    /// module's gate — the walk stops at the first non-attribute line.
1526    #[test]
1527    fn a_gate_on_another_item_is_not_borrowed() {
1528        let parent = "#[cfg(feature = \"serve\")]\npub mod served;\n\npub mod plain;\n";
1529        let site = claim_site(SHA, "crates/x/src/plain.rs", 1, "", Some(parent));
1530        assert_eq!(
1531            site.features, None,
1532            "the gate governs `served`, not `plain`"
1533        );
1534    }
1535
1536    /// `len / 4` is the basis every budget number in this stage is quoted on.
1537    #[test]
1538    fn token_estimation_is_the_documented_basis() {
1539        assert_eq!(estimate_tokens(&"a".repeat(400)), 100);
1540        assert_eq!(estimate_tokens(""), 0);
1541    }
1542
1543    /// This repository's compiling jobs, as `compile_claim`'s own tests model
1544    /// them: all ubuntu, only `checks`/`default-features` with `--all-targets`.
1545    fn ci() -> Vec<CheckRun> {
1546        vec![
1547            CheckRun {
1548                job: "msrv".to_owned(),
1549                sha: SHA.to_owned(),
1550                conclusion: Conclusion::Success,
1551                toolchain: "1.94".to_owned(),
1552                platform: TargetOs::Linux,
1553                features: Features::All,
1554                targets: Targets::LibsAndBins,
1555            },
1556            CheckRun {
1557                job: "checks".to_owned(),
1558                sha: SHA.to_owned(),
1559                conclusion: Conclusion::Success,
1560                toolchain: "stable".to_owned(),
1561                platform: TargetOs::Linux,
1562                features: Features::All,
1563                targets: Targets::AllTargets,
1564            },
1565        ]
1566    }
1567}