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
215/// A title with a leading severity tag removed, such as `[blocking] `.
216///
217/// Reviewers and authors are told to copy a finding's title across exactly so
218/// the two can be matched up, and they mostly do, but one side decorating it
219/// with the severity it already reports in its own field is common enough to
220/// cost a round: the disposition matches nothing, the finding it answered stays
221/// open, and both are reported as unresolved. Only a bracketed tag whose word
222/// is a severity is dropped, so a title that genuinely opens with a bracketed
223/// subject keeps it and two findings that differ only there stay distinct.
224pub(crate) fn untagged_title(title: &str) -> &str {
225    let trimmed = title.trim();
226    let Some(rest) = trimmed.strip_prefix('[') else {
227        return trimmed;
228    };
229    let Some((tag, rest)) = rest.split_once(']') else {
230        return trimmed;
231    };
232    if crate::model::Severity::parse_lenient(tag.trim()).is_none() {
233        return trimmed;
234    }
235    let rest = rest.trim_start();
236    if rest.is_empty() {
237        return trimmed;
238    }
239    rest
240}
241
242fn identity_key(title: &str, file: &str) -> String {
243    let title: String = untagged_title(title)
244        .to_lowercase()
245        .chars()
246        .filter(|c| c.is_ascii_alphanumeric() || c.is_whitespace())
247        .collect();
248    let title = title.split_whitespace().collect::<Vec<_>>().join(" ");
249    let basis = format!("{title}\0{file}");
250
251    short_hash(&basis)
252}
253
254fn short_hash(basis: &str) -> String {
255    let digest = Sha256::digest(basis.as_bytes());
256    let hex: String = digest.iter().map(|b| format!("{b:02x}")).collect();
257    hex[..12].to_string()
258}
259
260/// A finding's repository path without an optional trailing line or column.
261///
262/// Review locations move as fixes land. The path identifies the point across
263/// rounds, while the full location is still kept on the finding for display.
264pub(crate) fn finding_file(file: &str) -> String {
265    let mut path = file.trim();
266    while let Some((head, suffix)) = path.rsplit_once(':') {
267        let is_number = !suffix.is_empty() && suffix.chars().all(|c| c.is_ascii_digit());
268        let is_range = suffix.split_once('-').is_some_and(|(start, end)| {
269            !start.is_empty()
270                && !end.is_empty()
271                && start.chars().all(|c| c.is_ascii_digit())
272                && end.chars().all(|c| c.is_ascii_digit())
273        });
274        if !is_number && !is_range {
275            break;
276        }
277        path = head.trim_end();
278    }
279    path.to_string()
280}
281
282#[cfg(test)]
283mod tests {
284    use super::*;
285
286    #[test]
287    fn bare_object() {
288        assert_eq!(
289            serde_json::json!({"a": 1}),
290            extract_json(r#"{"a": 1}"#).unwrap()
291        );
292    }
293
294    #[test]
295    fn fenced_block() {
296        let out = extract_json("here you go:\n```json\n{\"a\": 1}\n```\n").unwrap();
297        assert_eq!(serde_json::json!({"a": 1}), out);
298    }
299
300    #[test]
301    fn trailing_prose() {
302        let out = extract_json("Thoughts...\n{\"verdict\": \"approve\"}\nDone.").unwrap();
303        assert_eq!(serde_json::json!({"verdict": "approve"}), out);
304    }
305
306    #[test]
307    fn picks_the_last_fenced_block() {
308        let text = "```json\n{\"n\": 1}\n```\nrevised:\n```json\n{\"n\": 2}\n```";
309        assert_eq!(serde_json::json!({"n": 2}), extract_json(text).unwrap());
310    }
311
312    #[test]
313    fn nested_braces() {
314        let payload = r#"{"findings": [{"severity": "nit", "d": {"x": [1, 2]}}]}"#;
315        let out = extract_json(&format!("blah {payload} blah")).unwrap();
316        assert_eq!(1, out["findings"].as_array().unwrap().len());
317    }
318
319    #[test]
320    fn top_level_array() {
321        let out = extract_json("result: [1, 2, 3]").unwrap();
322        assert_eq!(3, out.as_array().unwrap().len());
323    }
324
325    #[test]
326    fn multibyte_prose_around_the_payload_does_not_panic() {
327        let out = extract_json("\u{1f600}\u{1f600} {\"a\": 1} \u{1f600}").unwrap();
328        assert_eq!(serde_json::json!({"a": 1}), out);
329    }
330
331    #[test]
332    fn raises_when_there_is_none() {
333        assert!(extract_json("no json here at all").is_err());
334    }
335
336    #[test]
337    fn raises_on_empty() {
338        assert!(extract_json("   ").is_err());
339    }
340
341    #[test]
342    fn malformed_trailing_object_falls_back_to_an_earlier_one() {
343        let text = "{\"good\": true}\nthen: {\"bad\": ,}";
344        assert_eq!(
345            serde_json::json!({"good": true}),
346            extract_json(text).unwrap()
347        );
348    }
349
350    // -- finding_key -----------------------------------------------------
351
352    #[test]
353    fn key_is_stable_across_wording_noise() {
354        assert_eq!(
355            finding_key("Unbounded loop!", "src/x.rs"),
356            finding_key("unbounded loop", "src/x.rs")
357        );
358    }
359
360    #[test]
361    fn key_differs_by_file() {
362        assert_ne!(finding_key("t", "a.rs"), finding_key("t", "b.rs"));
363    }
364
365    #[test]
366    fn public_key_keeps_its_original_case_insensitive_paths() {
367        assert_eq!(
368            finding_key("t", "src/Main.rs"),
369            finding_key("t", "src/main.rs")
370        );
371        assert_ne!(
372            exact_finding_key("t", "src/Main.rs"),
373            exact_finding_key("t", "src/main.rs")
374        );
375    }
376
377    #[test]
378    fn key_is_stable_across_whitespace() {
379        assert_eq!(finding_key("a  b", "x.rs"), finding_key(" a b ", "x.rs"));
380    }
381
382    #[test]
383    fn a_severity_tag_does_not_change_a_title() {
384        assert_eq!(
385            "Unbounded loop",
386            untagged_title("[blocking] Unbounded loop")
387        );
388        assert_eq!("Unbounded loop", untagged_title("  [ NIT ]Unbounded loop "));
389        assert_eq!(
390            exact_finding_key("Unbounded loop", "x.rs"),
391            exact_finding_key("[non-blocking] Unbounded loop", "x.rs")
392        );
393    }
394
395    #[test]
396    fn a_bracketed_subject_is_part_of_the_title() {
397        assert_eq!("[iOS] Startup crash", untagged_title("[iOS] Startup crash"));
398        assert_eq!("[blocking]", untagged_title("[blocking]"));
399        assert_eq!("[blocking Unbounded", untagged_title("[blocking Unbounded"));
400        assert_ne!(
401            exact_finding_key("[iOS] Startup crash", "x.rs"),
402            exact_finding_key("[Android] Startup crash", "x.rs")
403        );
404    }
405
406    #[test]
407    fn exact_key_keeps_distinct_locations() {
408        assert_ne!(
409            exact_finding_key("t", "src/net.rs:88"),
410            exact_finding_key("t", "src/net.rs:91")
411        );
412        assert_eq!(
413            stable_finding_key("t", "src/net.rs:88"),
414            stable_finding_key("t", "src/net.rs:91")
415        );
416        assert_eq!(
417            stable_finding_key("t", "src/net.rs:88-94"),
418            stable_finding_key("t", "src/net.rs")
419        );
420        assert_eq!(
421            stable_finding_key("t", "src/net.rs:88:12"),
422            stable_finding_key("t", "src/net.rs")
423        );
424    }
425
426    #[test]
427    fn a_numeric_filename_is_not_treated_as_a_line_number() {
428        assert_eq!("fixtures/2024", finding_file("fixtures/2024"));
429    }
430
431    #[test]
432    fn key_is_twelve_hex_characters() {
433        let key = finding_key("anything", "file.rs");
434        assert_eq!(12, key.len());
435        assert!(key.chars().all(|c| c.is_ascii_hexdigit()));
436    }
437}
438
439#[cfg(test)]
440mod truncation_tests {
441    use super::*;
442    use serde::Deserialize;
443
444    #[derive(Debug, Deserialize)]
445    struct Review {
446        verdict: String,
447        findings: Vec<Finding>,
448    }
449    #[derive(Debug, Deserialize)]
450    struct Finding {
451        title: String,
452    }
453
454    /// What actually happened on a real pull request. A long review hit the
455    /// model's output limit and stopped before its outer object closed, so the
456    /// last complete JSON in the response was a nested finding. It parsed, it
457    /// was not a review, and the error blamed the shape.
458    const TRUNCATED: &str = r#"Here is my review.
459{"verdict":"changes_requested","next_action":"hand_back","summary":"Two problems.",
460 "findings":[
461   {"severity":"blocking","title":"First","detail":"one","file":"a.ts","in_scope":true},
462   {"severity":"non-blocking","title":"Second","detail":"numbers unnamed keys by position"#;
463
464    #[test]
465    fn a_truncated_review_is_reported_as_truncated_not_as_the_wrong_shape() {
466        let err = extract_into::<Review>(TRUNCATED).unwrap_err().to_string();
467        assert!(err.contains("cut off"), "{err}");
468        assert!(!err.contains("did not match the expected shape"), "{err}");
469    }
470
471    #[test]
472    fn truncation_is_detected_from_the_unclosed_braces() {
473        assert!(looks_truncated(TRUNCATED));
474        assert!(!looks_truncated(r#"{"a":1}"#));
475        // A brace inside a string is not an open brace.
476        assert!(!looks_truncated(r#"{"a":"a { in a string"}"#));
477        assert!(!looks_truncated(r#"{"a":"an escaped \" quote { here"}"#));
478    }
479
480    /// The fix that matters: the right object is found even when it is not the
481    /// last one in the response.
482    #[test]
483    fn the_review_is_found_even_with_nested_objects_after_it() {
484        let text = r#"Thinking out loud first.
485{"verdict":"approve","next_action":"merge","summary":"Fine.","findings":[{"severity":"nit","title":"Wording","detail":"d","file":"a.ts","in_scope":true}]}
486And here is a stray object afterwards: {"title":"not the review"}"#;
487        let review: Review = extract_into(text).unwrap();
488        assert_eq!("approve", review.verdict);
489        assert_eq!(1, review.findings.len());
490        assert_eq!("Wording", review.findings[0].title);
491    }
492
493    #[test]
494    fn candidates_are_offered_most_likely_first() {
495        let text = "```json\n{\"verdict\":\"approve\",\"findings\":[]}\n```\ntrailing {\"x\":1}";
496        let review: Review = extract_into(text).unwrap();
497        assert_eq!("approve", review.verdict);
498    }
499
500    #[test]
501    fn a_genuinely_wrong_shape_still_says_so() {
502        let err = extract_into::<Review>(r#"{"colour":"blue"}"#)
503            .unwrap_err()
504            .to_string();
505        assert!(err.contains("did not match the expected shape"), "{err}");
506        assert!(!err.contains("cut off"), "{err}");
507    }
508
509    /// What a real round two review produced. The model wrote a review object
510    /// and a findings array, the object failed to parse, and the complaint that
511    /// went back to it described the array: "invalid type: map, expected a
512    /// string", which is field zero of a struct serde had mapped an array onto
513    /// by position. The model was sent to look at a field, and the field was
514    /// not what was wrong.
515    #[test]
516    fn the_complaint_is_about_the_answer_the_model_meant() {
517        let text = r#"Here is my review.
518{"verdict":"changes_requested","next_action":"hand_back","summary":"Two problems.","findings":"should have been a list"}
519Supporting detail: [{"detail":"The working tree bumps 0.5.9 to 0.5.10."}]"#;
520        let err = extract_into::<Review>(text).unwrap_err().to_string();
521        // The review object is what failed, and its own field is named.
522        assert!(err.contains("findings"), "{err}");
523        assert!(
524            err.contains("changes_requested"),
525            "the object is shown:\n{err}"
526        );
527        assert!(
528            !err.contains("The working tree"),
529            "the stray array leaked in:\n{err}"
530        );
531    }
532
533    /// serde maps a JSON array onto a struct by position, so a bare array of
534    /// findings fails on the first field and says "expected a string". Left at
535    /// that, the model reads it as a field problem.
536    #[test]
537    fn a_bare_array_is_named_as_the_envelope_problem() {
538        let err = extract_into::<Review>(r#"[{"title":"First"},{"title":"Second"}]"#)
539            .unwrap_err()
540            .to_string();
541        assert!(err.contains("JSON array"), "{err}");
542        assert!(err.contains("single object"), "{err}");
543    }
544
545    /// And an object that is merely the wrong shape says nothing about arrays.
546    #[test]
547    fn a_wrong_object_is_not_told_it_was_an_array() {
548        let err = extract_into::<Review>(r#"{"colour":"blue"}"#)
549            .unwrap_err()
550            .to_string();
551        assert!(!err.contains("JSON array"), "{err}");
552    }
553
554    #[test]
555    fn nothing_parseable_is_still_reported_plainly() {
556        let err = extract_into::<Review>("no json at all")
557            .unwrap_err()
558            .to_string();
559        assert!(err.contains("no JSON found"), "{err}");
560    }
561}