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