Skip to main content

spar/
jsonx.rs

1//! Getting structured data back out of a model, and hashing it stably.
2
3use serde_json::Value;
4use sha2::{Digest, Sha256};
5use std::sync::LazyLock;
6
7use regex::Regex;
8
9use crate::error::{Result, SparError};
10
11static FENCE: LazyLock<Regex> = LazyLock::new(|| {
12    Regex::new(r"(?s)```(?:json)?\s*(\{.*?\}|\[.*?\])\s*```").expect("fence pattern")
13});
14
15/// Pull the last JSON value out of a model response.
16///
17/// Models wrap JSON in prose or fences despite explicit instructions not to,
18/// and some emit a draft before the real answer, so the *last* well formed
19/// value wins: fenced blocks first, then brace matching backwards from the end.
20pub fn extract_json(text: &str) -> Result<Value> {
21    if text.trim().is_empty() {
22        return Err(SparError::new("empty response, expected JSON"));
23    }
24    candidates(text)
25        .into_iter()
26        .next()
27        .ok_or_else(|| SparError::new(format!("no JSON found in response:\n{}", head(text, 800))))
28}
29
30/// Every JSON value plausibly present in a model response, most likely first.
31///
32/// Fenced blocks come first because a model that fences its answer means it,
33/// then whole objects matched backwards from the end.
34pub fn candidates(text: &str) -> Vec<Value> {
35    let mut out = Vec::new();
36    let mut push = |value: Value| {
37        if !out.contains(&value) {
38            out.push(value);
39        }
40    };
41
42    let fenced: Vec<&str> = FENCE
43        .captures_iter(text)
44        .filter_map(|c| c.get(1).map(|m| m.as_str()))
45        .collect();
46    for blob in fenced.iter().rev() {
47        if let Ok(value) = serde_json::from_str::<Value>(blob) {
48            push(value);
49        }
50    }
51
52    let bytes = text.as_bytes();
53    for (opener, closer) in [(b'{', b'}'), (b'[', b']')] {
54        let mut end = rfind_byte(bytes, closer, bytes.len());
55        while let Some(e) = end {
56            let mut depth = 0i32;
57            let mut start = None;
58            for i in (0..=e).rev() {
59                if bytes[i] == closer {
60                    depth += 1;
61                } else if bytes[i] == opener {
62                    depth -= 1;
63                    if depth == 0 {
64                        start = Some(i);
65                        break;
66                    }
67                }
68            }
69            if let Some(s) = start {
70                if let Ok(value) = serde_json::from_str::<Value>(&text[s..=e]) {
71                    push(value);
72                }
73            }
74            end = rfind_byte(bytes, closer, e);
75        }
76    }
77    out
78}
79
80/// Whether a response looks cut off rather than merely malformed.
81///
82/// A model with an output limit stops mid-object on a long answer. The braces
83/// it opened outnumber the ones it closed, and every complete object left is
84/// something nested inside the one it was building.
85pub fn looks_truncated(text: &str) -> bool {
86    let mut opened = 0i64;
87    let mut in_string = false;
88    let mut escaped = false;
89    for c in text.chars() {
90        if escaped {
91            escaped = false;
92            continue;
93        }
94        match c {
95            '\\' if in_string => escaped = true,
96            '"' => in_string = !in_string,
97            '{' if !in_string => opened += 1,
98            '}' if !in_string => opened -= 1,
99            _ => {}
100        }
101    }
102    opened > 0
103}
104
105/// Parse a model response straight into a typed value.
106///
107/// Tries every candidate rather than only the last one found. A review cut off
108/// before its outer object closed used to yield the last *nested* finding,
109/// which parsed as JSON perfectly well and then failed to be a review, and the
110/// error blamed the shape rather than the truncation.
111pub fn extract_into<T: serde::de::DeserializeOwned>(text: &str) -> Result<T> {
112    let found = candidates(text);
113    if found.is_empty() {
114        return Err(SparError::new(if looks_truncated(text) {
115            format!(
116                "the response was cut off before any complete JSON:\n{}",
117                head(text, 400)
118            )
119        } else {
120            format!("no JSON found in response:\n{}", head(text, 800))
121        }));
122    }
123
124    // Which failure to report is not the same question as which candidate to
125    // parse. Any candidate that parses wins, and a stray object never will,
126    // because every schema here requires fields it does not have. But when
127    // nothing parses, this error is handed straight back to the model on the
128    // retry, so it has to be about the answer the model meant.
129    //
130    // Neither end of the list is that. The order here is really last-closing
131    // first, so the last object a response happens to contain leads, and a
132    // model that wrote its answer and then a sentence with an object in it gets
133    // told about the sentence. The biggest candidate is the better guess: an
134    // answer is longer than the fragments around it.
135    let mut failures: Vec<(serde_json::Error, &Value)> = Vec::new();
136    for value in &found {
137        match serde_json::from_value::<T>(value.clone()) {
138            Ok(parsed) => return Ok(parsed),
139            Err(e) => failures.push((e, value)),
140        }
141    }
142    let (error, value) = failures
143        .into_iter()
144        .max_by_key(|(_, value)| value.to_string().len())
145        .expect("non-empty");
146    if looks_truncated(text) {
147        return Err(SparError::new(format!(
148            "the response was cut off before the answer was complete, so only fragments of it \
149             parsed ({error}). Ask for less in one go, or give this agent a CLI flag for native \
150             structured output."
151        )));
152    }
153    Err(SparError::new(format!(
154        "response did not match the expected shape ({error}).{}\nGot: {}",
155        envelope_hint(value),
156        head(&value.to_string(), 600)
157    )))
158}
159
160/// An extra sentence when serde's own message would send the model to the wrong
161/// place.
162///
163/// serde maps a JSON array onto a struct's fields by position, so a bare array
164/// of findings tried as a review fails on field zero: "invalid type: map,
165/// expected a string", where the string is `verdict`. A model told that goes
166/// looking at a field, and the field is not what is wrong. Every schema here
167/// asks for one object, so an array is always the envelope rather than the
168/// contents.
169fn envelope_hint(value: &Value) -> &'static str {
170    if value.is_array() {
171        " The answer was a JSON array, and the schema asks for a single object: \
172         the array belongs in a field of it."
173    } else {
174        ""
175    }
176}
177
178fn rfind_byte(haystack: &[u8], needle: u8, before: usize) -> Option<usize> {
179    haystack[..before.min(haystack.len())]
180        .iter()
181        .rposition(|b| *b == needle)
182}
183
184fn head(text: &str, max: usize) -> String {
185    text.chars().take(max).collect()
186}
187
188/// The original public finding key, retained for persisted-state and library
189/// compatibility.
190pub fn finding_key(title: &str, file: &str) -> String {
191    let basis: String = format!("{} {}", title.trim(), file.trim())
192        .to_lowercase()
193        .chars()
194        .filter(|c| c.is_ascii_alphanumeric() || matches!(c, ' ' | '/' | '.' | '_' | '-'))
195        .collect();
196    let basis = basis.split_whitespace().collect::<Vec<_>>().join(" ");
197    short_hash(&basis)
198}
199
200/// An exact identity for a review finding within one answer.
201///
202/// Wording noise, punctuation, and title case are discarded. The full location
203/// is retained because the same complaint at two sites in one file is two
204/// complaints. Cross-round matching uses `stable_finding_key` as a guarded
205/// fallback instead of making this identity lossy.
206pub(crate) fn exact_finding_key(title: &str, file: &str) -> String {
207    identity_key(title, file.trim())
208}
209
210/// A location-tolerant identity used only for unambiguous cross-round matches.
211pub(crate) fn stable_finding_key(title: &str, file: &str) -> String {
212    identity_key(title, &finding_file(file))
213}
214
215fn identity_key(title: &str, file: &str) -> String {
216    let title: String = title
217        .trim()
218        .to_lowercase()
219        .chars()
220        .filter(|c| c.is_ascii_alphanumeric() || c.is_whitespace())
221        .collect();
222    let title = title.split_whitespace().collect::<Vec<_>>().join(" ");
223    let basis = format!("{title}\0{file}");
224
225    short_hash(&basis)
226}
227
228fn short_hash(basis: &str) -> String {
229    let digest = Sha256::digest(basis.as_bytes());
230    let hex: String = digest.iter().map(|b| format!("{b:02x}")).collect();
231    hex[..12].to_string()
232}
233
234/// A finding's repository path without an optional trailing line or column.
235///
236/// Review locations move as fixes land. The path identifies the point across
237/// rounds, while the full location is still kept on the finding for display.
238pub(crate) fn finding_file(file: &str) -> String {
239    let mut path = file.trim();
240    while let Some((head, suffix)) = path.rsplit_once(':') {
241        let is_number = !suffix.is_empty() && suffix.chars().all(|c| c.is_ascii_digit());
242        let is_range = suffix.split_once('-').is_some_and(|(start, end)| {
243            !start.is_empty()
244                && !end.is_empty()
245                && start.chars().all(|c| c.is_ascii_digit())
246                && end.chars().all(|c| c.is_ascii_digit())
247        });
248        if !is_number && !is_range {
249            break;
250        }
251        path = head.trim_end();
252    }
253    path.to_string()
254}
255
256#[cfg(test)]
257mod tests {
258    use super::*;
259
260    #[test]
261    fn bare_object() {
262        assert_eq!(
263            serde_json::json!({"a": 1}),
264            extract_json(r#"{"a": 1}"#).unwrap()
265        );
266    }
267
268    #[test]
269    fn fenced_block() {
270        let out = extract_json("here you go:\n```json\n{\"a\": 1}\n```\n").unwrap();
271        assert_eq!(serde_json::json!({"a": 1}), out);
272    }
273
274    #[test]
275    fn trailing_prose() {
276        let out = extract_json("Thoughts...\n{\"verdict\": \"approve\"}\nDone.").unwrap();
277        assert_eq!(serde_json::json!({"verdict": "approve"}), out);
278    }
279
280    #[test]
281    fn picks_the_last_fenced_block() {
282        let text = "```json\n{\"n\": 1}\n```\nrevised:\n```json\n{\"n\": 2}\n```";
283        assert_eq!(serde_json::json!({"n": 2}), extract_json(text).unwrap());
284    }
285
286    #[test]
287    fn nested_braces() {
288        let payload = r#"{"findings": [{"severity": "nit", "d": {"x": [1, 2]}}]}"#;
289        let out = extract_json(&format!("blah {payload} blah")).unwrap();
290        assert_eq!(1, out["findings"].as_array().unwrap().len());
291    }
292
293    #[test]
294    fn top_level_array() {
295        let out = extract_json("result: [1, 2, 3]").unwrap();
296        assert_eq!(3, out.as_array().unwrap().len());
297    }
298
299    #[test]
300    fn multibyte_prose_around_the_payload_does_not_panic() {
301        let out = extract_json("\u{1f600}\u{1f600} {\"a\": 1} \u{1f600}").unwrap();
302        assert_eq!(serde_json::json!({"a": 1}), out);
303    }
304
305    #[test]
306    fn raises_when_there_is_none() {
307        assert!(extract_json("no json here at all").is_err());
308    }
309
310    #[test]
311    fn raises_on_empty() {
312        assert!(extract_json("   ").is_err());
313    }
314
315    #[test]
316    fn malformed_trailing_object_falls_back_to_an_earlier_one() {
317        let text = "{\"good\": true}\nthen: {\"bad\": ,}";
318        assert_eq!(
319            serde_json::json!({"good": true}),
320            extract_json(text).unwrap()
321        );
322    }
323
324    // -- finding_key -----------------------------------------------------
325
326    #[test]
327    fn key_is_stable_across_wording_noise() {
328        assert_eq!(
329            finding_key("Unbounded loop!", "src/x.rs"),
330            finding_key("unbounded loop", "src/x.rs")
331        );
332    }
333
334    #[test]
335    fn key_differs_by_file() {
336        assert_ne!(finding_key("t", "a.rs"), finding_key("t", "b.rs"));
337    }
338
339    #[test]
340    fn public_key_keeps_its_original_case_insensitive_paths() {
341        assert_eq!(
342            finding_key("t", "src/Main.rs"),
343            finding_key("t", "src/main.rs")
344        );
345        assert_ne!(
346            exact_finding_key("t", "src/Main.rs"),
347            exact_finding_key("t", "src/main.rs")
348        );
349    }
350
351    #[test]
352    fn key_is_stable_across_whitespace() {
353        assert_eq!(finding_key("a  b", "x.rs"), finding_key(" a b ", "x.rs"));
354    }
355
356    #[test]
357    fn exact_key_keeps_distinct_locations() {
358        assert_ne!(
359            exact_finding_key("t", "src/net.rs:88"),
360            exact_finding_key("t", "src/net.rs:91")
361        );
362        assert_eq!(
363            stable_finding_key("t", "src/net.rs:88"),
364            stable_finding_key("t", "src/net.rs:91")
365        );
366        assert_eq!(
367            stable_finding_key("t", "src/net.rs:88-94"),
368            stable_finding_key("t", "src/net.rs")
369        );
370        assert_eq!(
371            stable_finding_key("t", "src/net.rs:88:12"),
372            stable_finding_key("t", "src/net.rs")
373        );
374    }
375
376    #[test]
377    fn a_numeric_filename_is_not_treated_as_a_line_number() {
378        assert_eq!("fixtures/2024", finding_file("fixtures/2024"));
379    }
380
381    #[test]
382    fn key_is_twelve_hex_characters() {
383        let key = finding_key("anything", "file.rs");
384        assert_eq!(12, key.len());
385        assert!(key.chars().all(|c| c.is_ascii_hexdigit()));
386    }
387}
388
389#[cfg(test)]
390mod truncation_tests {
391    use super::*;
392    use serde::Deserialize;
393
394    #[derive(Debug, Deserialize)]
395    struct Review {
396        verdict: String,
397        findings: Vec<Finding>,
398    }
399    #[derive(Debug, Deserialize)]
400    struct Finding {
401        title: String,
402    }
403
404    /// What actually happened on a real pull request. A long review hit the
405    /// model's output limit and stopped before its outer object closed, so the
406    /// last complete JSON in the response was a nested finding. It parsed, it
407    /// was not a review, and the error blamed the shape.
408    const TRUNCATED: &str = r#"Here is my review.
409{"verdict":"changes_requested","next_action":"hand_back","summary":"Two problems.",
410 "findings":[
411   {"severity":"blocking","title":"First","detail":"one","file":"a.ts","in_scope":true},
412   {"severity":"non-blocking","title":"Second","detail":"numbers unnamed keys by position"#;
413
414    #[test]
415    fn a_truncated_review_is_reported_as_truncated_not_as_the_wrong_shape() {
416        let err = extract_into::<Review>(TRUNCATED).unwrap_err().to_string();
417        assert!(err.contains("cut off"), "{err}");
418        assert!(!err.contains("did not match the expected shape"), "{err}");
419    }
420
421    #[test]
422    fn truncation_is_detected_from_the_unclosed_braces() {
423        assert!(looks_truncated(TRUNCATED));
424        assert!(!looks_truncated(r#"{"a":1}"#));
425        // A brace inside a string is not an open brace.
426        assert!(!looks_truncated(r#"{"a":"a { in a string"}"#));
427        assert!(!looks_truncated(r#"{"a":"an escaped \" quote { here"}"#));
428    }
429
430    /// The fix that matters: the right object is found even when it is not the
431    /// last one in the response.
432    #[test]
433    fn the_review_is_found_even_with_nested_objects_after_it() {
434        let text = r#"Thinking out loud first.
435{"verdict":"approve","next_action":"merge","summary":"Fine.","findings":[{"severity":"nit","title":"Wording","detail":"d","file":"a.ts","in_scope":true}]}
436And here is a stray object afterwards: {"title":"not the review"}"#;
437        let review: Review = extract_into(text).unwrap();
438        assert_eq!("approve", review.verdict);
439        assert_eq!(1, review.findings.len());
440        assert_eq!("Wording", review.findings[0].title);
441    }
442
443    #[test]
444    fn candidates_are_offered_most_likely_first() {
445        let text = "```json\n{\"verdict\":\"approve\",\"findings\":[]}\n```\ntrailing {\"x\":1}";
446        let review: Review = extract_into(text).unwrap();
447        assert_eq!("approve", review.verdict);
448    }
449
450    #[test]
451    fn a_genuinely_wrong_shape_still_says_so() {
452        let err = extract_into::<Review>(r#"{"colour":"blue"}"#)
453            .unwrap_err()
454            .to_string();
455        assert!(err.contains("did not match the expected shape"), "{err}");
456        assert!(!err.contains("cut off"), "{err}");
457    }
458
459    /// What a real round two review produced. The model wrote a review object
460    /// and a findings array, the object failed to parse, and the complaint that
461    /// went back to it described the array: "invalid type: map, expected a
462    /// string", which is field zero of a struct serde had mapped an array onto
463    /// by position. The model was sent to look at a field, and the field was
464    /// not what was wrong.
465    #[test]
466    fn the_complaint_is_about_the_answer_the_model_meant() {
467        let text = r#"Here is my review.
468{"verdict":"changes_requested","next_action":"hand_back","summary":"Two problems.","findings":"should have been a list"}
469Supporting detail: [{"detail":"The working tree bumps 0.5.9 to 0.5.10."}]"#;
470        let err = extract_into::<Review>(text).unwrap_err().to_string();
471        // The review object is what failed, and its own field is named.
472        assert!(err.contains("findings"), "{err}");
473        assert!(
474            err.contains("changes_requested"),
475            "the object is shown:\n{err}"
476        );
477        assert!(
478            !err.contains("The working tree"),
479            "the stray array leaked in:\n{err}"
480        );
481    }
482
483    /// serde maps a JSON array onto a struct by position, so a bare array of
484    /// findings fails on the first field and says "expected a string". Left at
485    /// that, the model reads it as a field problem.
486    #[test]
487    fn a_bare_array_is_named_as_the_envelope_problem() {
488        let err = extract_into::<Review>(r#"[{"title":"First"},{"title":"Second"}]"#)
489            .unwrap_err()
490            .to_string();
491        assert!(err.contains("JSON array"), "{err}");
492        assert!(err.contains("single object"), "{err}");
493    }
494
495    /// And an object that is merely the wrong shape says nothing about arrays.
496    #[test]
497    fn a_wrong_object_is_not_told_it_was_an_array() {
498        let err = extract_into::<Review>(r#"{"colour":"blue"}"#)
499            .unwrap_err()
500            .to_string();
501        assert!(!err.contains("JSON array"), "{err}");
502    }
503
504    #[test]
505    fn nothing_parseable_is_still_reported_plainly() {
506        let err = extract_into::<Review>("no json at all")
507            .unwrap_err()
508            .to_string();
509        assert!(err.contains("no JSON found"), "{err}");
510    }
511}