Skip to main content

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 let a caller combine per-file and per-layer results
325    /// without losing the failure signal.
326    ///
327    /// On a key collision in `failed_files`, the **first** reason wins. A
328    /// file failing twice is still one failure, and the two reasons are not
329    /// meaningfully combinable - the first one is at least specific to the
330    /// file, while a hypothetical last-wins policy would let a later
331    /// analyzer overwrite a more informative first reason with a generic
332    /// one.
333    pub fn merge(&mut self, other: AnalysisResult) {
334        self.findings.extend(other.findings);
335        // `union_failures` rather than the loop written out again: the
336        // first-writer-wins rule is one decision, and two copies of it are two
337        // places for it to change independently.
338        union_failures(&mut self.failed_files, other.failed_files);
339        self.dropped_out_of_range = self
340            .dropped_out_of_range
341            .saturating_add(other.dropped_out_of_range);
342    }
343
344    /// True when any file went unanalyzed.
345    ///
346    /// The caller maps this to process exit 2: "could not analyze" is
347    /// distinct from both "clean" (exit 0) and "found issues" (exit 1),
348    /// because a gate that cannot distinguish them rubber-stamps the day
349    /// the LLM endpoint goes down.
350    pub fn has_failures(&self) -> bool {
351        !self.failed_files.is_empty()
352    }
353}
354
355#[cfg(test)]
356mod tests {
357    use super::*;
358
359    /// `merge` keeps the first reason on a key collision - the documented
360    /// first-writer-wins rule. A last-wins policy would silently overwrite
361    /// the more informative first reason with a generic later one.
362    #[test]
363    fn merge_keeps_first_reason_on_key_collision() {
364        let mut a = AnalysisResult::default();
365        a.failed_files.insert(
366            PathBuf::from("src/lib.rs"),
367            FailureReason::Transport {
368                status: Some(429),
369                message: "rate limited".to_owned(),
370            },
371        );
372
373        let mut b = AnalysisResult::default();
374        b.failed_files.insert(
375            PathBuf::from("src/lib.rs"),
376            FailureReason::Transport {
377                status: Some(500),
378                message: "internal".to_owned(),
379            },
380        );
381
382        a.merge(b);
383
384        assert_eq!(a.failed_files.len(), 1);
385        let reason = a.failed_files.get(&PathBuf::from("src/lib.rs")).unwrap();
386        assert_eq!(
387            reason,
388            &FailureReason::Transport {
389                status: Some(429),
390                message: "rate limited".to_owned(),
391            },
392            "first reason wins on collision"
393        );
394    }
395
396    /// A `Transport` failure with a status surfaces the code in the rendered
397    /// line. The whole point of keeping the status as a number is that it
398    /// reaches the user; this pins that the rendering preserves it.
399    #[test]
400    fn transport_render_includes_the_http_status() {
401        let reason = FailureReason::Transport {
402            status: Some(429),
403            message: "rate limited".to_owned(),
404        };
405        let rendered = reason.one_line();
406        assert!(
407            rendered.contains("429"),
408            "rendered line must contain 429, got {rendered:?}"
409        );
410    }
411
412    #[test]
413    fn display_honours_formatter_width_and_alignment() {
414        let reason = FailureReason::Truncated;
415        assert_eq!(
416            format!("{reason:>30}"),
417            format!("{:>30}", reason.one_line())
418        );
419    }
420}