drep/analysis/result.rs
1//! What one analysis pass produced.
2//!
3//! A single pass over a file can produce findings AND fail to fully analyze
4//! the file (a truncated response gives a partial list, an unknown severity
5//! in one record fails the file, a transport error produces zero findings but
6//! still surfaces as a failure). Reporting the findings while forgetting the
7//! failure is the exact bug this type exists to prevent — the gate would
8//! green-light a commit whenever the LLM endpoint was unreachable, which is
9//! worse than having no gate at all.
10//!
11//! `failed_files` is a [`BTreeMap`] rather than a `Vec` because two passes
12//! over the same file set must UNION, never sum. Summing counts one
13//! unreachable endpoint twice, drifting the failure count up without any
14//! matching file to investigate. The map's value carries the reason so the
15//! caller can render something a user can act on, not just a path.
16//!
17//! `dropped_out_of_range` counts rather than silently drops out-of-range
18//! findings so that a model which consistently reports wrong lines is
19//! observable to the caller — not invisible.
20
21use std::collections::BTreeMap;
22use std::fmt;
23use std::path::PathBuf;
24
25use crate::analysis::findings::Finding;
26use crate::llm::error::BackendErrorKind;
27
28/// Why one file went unanalyzed.
29///
30/// A bare set of paths cannot tell a dead endpoint from a rate limit from a
31/// truncated response, and the caller needs that to print something a user can
32/// act on. `LlmError` already carries the detail; it used to be discarded at
33/// the analyzer boundary.
34#[derive(Debug, Clone, PartialEq, Eq)]
35pub enum FailureReason {
36 /// The endpoint was unreachable, or returned a retryable status too many
37 /// times. `status` is the HTTP code when there was one.
38 Transport {
39 status: Option<u16>,
40 message: String,
41 },
42 /// A non-HTTP backend failed with a structured routing class.
43 Backend {
44 kind: BackendErrorKind,
45 message: String,
46 },
47 /// A response arrived and no JSON could be extracted from it.
48 Unparseable(String),
49 /// Cache-only review found no response for this exact prompt and provider.
50 CacheMiss,
51 /// The model stopped before producing JSON, and the server said why.
52 ///
53 /// Distinct from [`Self::Unparseable`] because the cause is known and
54 /// deterministic - an output-token cap or a content filter - so the answer
55 /// is not "ask again" but "this request cannot be served as sent". `finish`
56 /// is the server's own word for it, kept as a machine tag beside the human
57 /// message exactly as [`Self::Transport`] keeps its status.
58 ModelStopped { finish: String, message: String },
59 /// The response parsed only after closing unbalanced delimiters, so it is
60 /// a prefix of what the model meant to say.
61 Truncated,
62 /// A record in the response could not be understood - unknown severity,
63 /// missing field, unusable line number.
64 MalformedFinding(String),
65 /// A deterministic tool that should have run could not.
66 ToolUnavailable { tool: String, detail: String },
67 /// The file on disk exceeded the read guard, so drep never read it.
68 ///
69 /// Distinct from [`Self::PayloadTooLarge`] because the two measure
70 /// different things: this is the file's own size, checked before any I/O,
71 /// and that one is the size of the text the model would have been sent.
72 /// They were one variant sharing one limit, which meant `bytes` held the
73 /// file size on one code path and the rendered-payload size on another -
74 /// so "file is too large (330102 bytes)" could name a file that `ls`
75 /// reports as 261900 bytes.
76 FileTooLarge { bytes: u64, limit: u64 },
77 /// The rendered LLM payload exceeded the ceiling. See [`Self::FileTooLarge`].
78 PayloadTooLarge { bytes: u64, limit: u64 },
79 /// The file could not be read from disk.
80 Unreadable(String),
81 /// The user named a file that the running command has no analyzer for.
82 ///
83 /// Only ever produced for an **explicitly named** path. A walk that turns
84 /// up nothing analyzable is legitimately empty - `drep check .` in a
85 /// documentation repository has correctly found no code. A path the user
86 /// typed is different: reporting "No issues found." for a file drep
87 /// declined to look at is the single failure this codebase is built to
88 /// prevent, and it is the same distinction `resolve_paths` already draws
89 /// for an argument that does not exist at all.
90 ///
91 /// `hint` names the command that *does* handle the type, when there is
92 /// one. Markdown has `drep lint-docs`, so the error is a redirection
93 /// rather than a dead end.
94 Unsupported {
95 /// The extension as written, with its dot. `None` when the file has
96 /// none, which reads differently in the message.
97 extension: Option<String>,
98 /// What to run instead, phrased as an imperative.
99 hint: Option<String>,
100 },
101 /// A failover chain produced no answer, with what each provider
102 /// contributed.
103 ///
104 /// Only produced for a chain of **two or more** providers. A one-provider
105 /// config - what `drep init` writes, and what almost every run uses -
106 /// collapses to that provider's own reason, so it reports exactly what it
107 /// did before failover existed, JSON `kind` included. The trigger is the
108 /// chain's length, not the number of providers that failed: a two-provider
109 /// chain stopped dead at the head by a 401 has one failure and is exactly
110 /// the case where "which provider, and why did my fallback not run" is the
111 /// user's live question.
112 ///
113 /// Keeping only the last reason would hide a dead local endpoint behind
114 /// the cloud fallback's 401; keeping only the first would hide the broken
115 /// fallback. A user fixing the run needs both.
116 ///
117 /// The list can be shorter than the chain - a 401 at the head stops it, and
118 /// the providers below were never consulted.
119 ChainFailed(Vec<ProviderFailure>),
120}
121
122/// One provider's contribution to a file that no provider could analyze.
123///
124/// `reason` is always a non-chain LLM-layer variant. That is a property of the
125/// only thing that builds these: the conversion runs over one provider's
126/// `LlmError`, so a nested `ChainFailed` is not merely absent but unreachable.
127#[derive(Debug, Clone, PartialEq, Eq)]
128pub struct ProviderFailure {
129 /// Zero-based position in the chain. Rendered one-based, matching how
130 /// `doctor` numbers the same list.
131 pub provider: usize,
132 /// The model that provider asks for.
133 pub model: String,
134 /// Why it did not produce an answer.
135 pub reason: FailureReason,
136 /// True when the provider was already demoted and was not contacted for
137 /// this file. Worth reporting: a user needs to know the local endpoint has
138 /// been dead since the third file, not just that the fallback then failed.
139 pub skipped: bool,
140}
141
142impl FailureReason {
143 /// Build an [`Self::Unsupported`] for `path`.
144 ///
145 /// The extension convention (leading dot, `None` when there is none) is
146 /// stated here, beside the variant whose `one_line` renders it, rather than
147 /// at each command that raises one. It was written out twice, which is one
148 /// copy per command pointing at the other.
149 pub fn unsupported(path: &std::path::Path, hint: Option<String>) -> Self {
150 FailureReason::Unsupported {
151 extension: path
152 .extension()
153 .map(|ext| format!(".{}", ext.to_string_lossy())),
154 hint,
155 }
156 }
157
158 /// A single line suitable for a terminal, derived from the variant.
159 ///
160 /// The HTTP status is rendered next to the message so a 429 is visible
161 /// without the user having to match the message against a status code
162 /// list. This is the load-bearing reason the `Transport` variant carries
163 /// the status as a number rather than only inside the string.
164 pub fn one_line(&self) -> String {
165 match self {
166 FailureReason::Transport {
167 status: Some(code),
168 message,
169 } => {
170 format!("LLM transport failed (HTTP {code}): {message}")
171 }
172 FailureReason::Transport {
173 status: None,
174 message,
175 } => {
176 format!("LLM transport failed: {message}")
177 }
178 FailureReason::Unparseable(message) => {
179 format!("LLM response was unparseable: {message}")
180 }
181 FailureReason::CacheMiss => {
182 "LLM review is not cached; run a normal check to warm it".to_owned()
183 }
184 FailureReason::Backend { kind, message } => {
185 format!("LLM backend {kind}: {message}")
186 }
187 // Deliberately says nothing about *which* command is running: both
188 // `check` and `lint-docs` produce this, pointing at each other.
189 FailureReason::Unsupported { extension, hint } => {
190 let what = match extension {
191 Some(ext) => format!("`{ext}` files"),
192 None => "files with no extension".to_owned(),
193 };
194 match hint {
195 Some(hint) => format!("no analyzer for {what}: {hint}"),
196 None => format!("no analyzer for {what}"),
197 }
198 }
199 // The message is already a sentence a user can act on; prefixing it
200 // with a category would bury the actionable half.
201 FailureReason::ModelStopped { message, .. } => message.clone(),
202 FailureReason::Truncated => "response was truncated".to_owned(),
203 FailureReason::MalformedFinding(detail) => format!("malformed finding: {detail}"),
204 FailureReason::ToolUnavailable { tool, detail } => {
205 format!("{tool} could not run: {detail}")
206 }
207 FailureReason::FileTooLarge { bytes, limit } => {
208 format!("file is too large to read ({bytes} bytes; limit is {limit})")
209 }
210 FailureReason::PayloadTooLarge { bytes, limit } => {
211 format!("the code sent for review is too large ({bytes} bytes; limit is {limit})")
212 }
213 FailureReason::Unreadable(detail) => format!("file could not be read: {detail}"),
214 FailureReason::ChainFailed(failures) => {
215 let each: Vec<String> = failures.iter().map(ProviderFailure::one_line).collect();
216 // Phrased by what happened, not by a count. "All N providers
217 // failed" is wrong for the case that matters most - a chain
218 // stopped at the head by a 401 has one entry and more
219 // providers behind it that were deliberately not asked.
220 if each.is_empty() {
221 "no LLM provider analyzed this file".to_owned()
222 } else {
223 format!("no LLM provider analyzed this file: {}", each.join("; "))
224 }
225 }
226 }
227 }
228
229 /// The HTTP status, when the failure had one.
230 ///
231 /// Only `Transport` ever carries one. Exposed as a number rather than
232 /// left inside the message because a caller has to distinguish a 429 from
233 /// a 401 - the message is prose and prose gets reworded.
234 pub fn status(&self) -> Option<u16> {
235 match self {
236 FailureReason::Transport { status, .. } => *status,
237 // Deliberately not "the first attempt's status". A chain failure
238 // has one status *per provider*, and flattening them to one number
239 // would tell a consumer a 401 was the whole story when a 500 came
240 // first. The JSON renderer exposes the per-provider list instead.
241 _ => None,
242 }
243 }
244}
245
246impl ProviderFailure {
247 /// One line naming the provider, its model, and what it said.
248 pub fn one_line(&self) -> String {
249 let skipped = if self.skipped {
250 " (already down earlier in this run)"
251 } else {
252 ""
253 };
254 format!(
255 "[{}] {}: {}{}",
256 self.provider + 1,
257 self.model,
258 self.reason.one_line(),
259 skipped
260 )
261 }
262}
263
264impl fmt::Display for FailureReason {
265 fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
266 f.pad(&self.one_line())
267 }
268}
269
270/// What one analysis pass produced.
271///
272/// `findings` and `failed_files` are independent axes: a file can contribute
273/// findings AND be unanalyzed (a truncated response gives a partial list).
274/// Reporting the findings while forgetting the failure is the exact bug this
275/// type exists to prevent.
276#[derive(Debug, Default, Clone, PartialEq, Eq)]
277pub struct AnalysisResult {
278 /// Findings the analyzer could attribute to a real line of code.
279 pub findings: Vec<Finding>,
280 /// Files that could not be fully analyzed, with the reason. A `BTreeMap`
281 /// because two passes over the same file set must UNION, never sum —
282 /// summing counts one unreachable endpoint twice.
283 pub failed_files: BTreeMap<PathBuf, FailureReason>,
284 /// Findings discarded because their line was not in the payload's
285 /// `valid_lines`. Counted rather than silently dropped, so the drop is
286 /// observable.
287 pub dropped_out_of_range: usize,
288}
289
290/// Fold `src` into `dst`, keeping the reason already present on a collision.
291///
292/// The one statement of the failure-union rule. It was written out longhand as
293/// `entry().or_insert()` at four sites - `merge` here plus three in the CLI -
294/// each with its own comment re-explaining it. The two analysis layers cover
295/// the same files, so the sets union rather than sum: one unreachable endpoint
296/// is one failure, not two. First-wins because the reasons cannot be
297/// meaningfully combined and the earlier layer saw the file first.
298pub fn union_failures(
299 dst: &mut BTreeMap<PathBuf, FailureReason>,
300 src: BTreeMap<PathBuf, FailureReason>,
301) {
302 for (path, reason) in src {
303 dst.entry(path).or_insert(reason);
304 }
305}
306
307impl AnalysisResult {
308 /// One file, one failure, no findings.
309 ///
310 /// The shape was hand-assembled at four call sites - `default()`, insert,
311 /// return - each of which independently had to know that `findings` and
312 /// `dropped_out_of_range` stay at their defaults. Forgetting the insert at
313 /// any one of them reports an unanalyzed file as clean, which is the single
314 /// failure this whole type exists to prevent, so it gets a constructor.
315 pub fn failed(path: PathBuf, reason: FailureReason) -> Self {
316 let mut result = Self::default();
317 result.failed_files.insert(path, reason);
318 result
319 }
320
321 /// Fold `other` into `self`: findings concatenate, `failed_files`
322 /// unions, `dropped_out_of_range` sums.
323 ///
324 /// The merge semantics are what let a caller run several analyzers
325 /// (code quality, docstrings, ...) and combine their results without
326 /// losing the failure signal. Calling `merge` in a loop over per-file
327 /// results inside `analyze_files` is the planned usage.
328 ///
329 /// On a key collision in `failed_files`, the **first** reason wins. A
330 /// file failing twice is still one failure, and the two reasons are not
331 /// meaningfully combinable - the first one is at least specific to the
332 /// file, while a hypothetical last-wins policy would let a later
333 /// analyzer overwrite a more informative first reason with a generic
334 /// one.
335 pub fn merge(&mut self, other: AnalysisResult) {
336 self.findings.extend(other.findings);
337 // `union_failures` rather than the loop written out again: the
338 // first-writer-wins rule is one decision, and two copies of it are two
339 // places for it to change independently.
340 union_failures(&mut self.failed_files, other.failed_files);
341 self.dropped_out_of_range = self
342 .dropped_out_of_range
343 .saturating_add(other.dropped_out_of_range);
344 }
345
346 /// True when any file went unanalyzed.
347 ///
348 /// The caller maps this to process exit 2: "could not analyze" is
349 /// distinct from both "clean" (exit 0) and "found issues" (exit 1),
350 /// because a gate that cannot distinguish them rubber-stamps the day
351 /// the LLM endpoint goes down.
352 pub fn has_failures(&self) -> bool {
353 !self.failed_files.is_empty()
354 }
355}
356
357#[cfg(test)]
358mod tests {
359 use super::*;
360
361 /// `merge` keeps the first reason on a key collision - the documented
362 /// first-writer-wins rule. A last-wins policy would silently overwrite
363 /// the more informative first reason with a generic later one.
364 #[test]
365 fn merge_keeps_first_reason_on_key_collision() {
366 let mut a = AnalysisResult::default();
367 a.failed_files.insert(
368 PathBuf::from("src/lib.rs"),
369 FailureReason::Transport {
370 status: Some(429),
371 message: "rate limited".to_owned(),
372 },
373 );
374
375 let mut b = AnalysisResult::default();
376 b.failed_files.insert(
377 PathBuf::from("src/lib.rs"),
378 FailureReason::Transport {
379 status: Some(500),
380 message: "internal".to_owned(),
381 },
382 );
383
384 a.merge(b);
385
386 assert_eq!(a.failed_files.len(), 1);
387 let reason = a.failed_files.get(&PathBuf::from("src/lib.rs")).unwrap();
388 assert_eq!(
389 reason,
390 &FailureReason::Transport {
391 status: Some(429),
392 message: "rate limited".to_owned(),
393 },
394 "first reason wins on collision"
395 );
396 }
397
398 /// A `Transport` failure with a status surfaces the code in the rendered
399 /// line. The whole point of keeping the status as a number is that it
400 /// reaches the user; this pins that the rendering preserves it.
401 #[test]
402 fn transport_render_includes_the_http_status() {
403 let reason = FailureReason::Transport {
404 status: Some(429),
405 message: "rate limited".to_owned(),
406 };
407 let rendered = reason.one_line();
408 assert!(
409 rendered.contains("429"),
410 "rendered line must contain 429, got {rendered:?}"
411 );
412 }
413
414 #[test]
415 fn display_honours_formatter_width_and_alignment() {
416 let reason = FailureReason::Truncated;
417 assert_eq!(
418 format!("{reason:>30}"),
419 format!("{:>30}", reason.one_line())
420 );
421 }
422}