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 the whole text, then balanced spans
20/// with the last closing one first.
21pub fn extract_json(text: &str) -> Result<Value> {
22    if text.trim().is_empty() {
23        return Err(SparError::new("empty response, expected JSON"));
24    }
25    candidates(text)
26        .into_iter()
27        .next()
28        .ok_or_else(|| SparError::new(format!("no JSON found in response:\n{}", head(text, 800))))
29}
30
31/// Every JSON value plausibly present in a model response, most likely first.
32///
33/// Fenced blocks come first because a model that fences its answer means it,
34/// then the whole trimmed text, then the balanced spans inside it.
35pub fn candidates(text: &str) -> Vec<Value> {
36    let mut out = Vec::new();
37    let mut push = |value: Value| {
38        if !out.contains(&value) {
39            out.push(value);
40        }
41    };
42
43    let fenced: Vec<&str> = FENCE
44        .captures_iter(text)
45        .filter_map(|c| c.get(1).map(|m| m.as_str()))
46        .collect();
47    for blob in fenced.iter().rev() {
48        if let Ok(value) = serde_json::from_str::<Value>(blob) {
49            push(value);
50        }
51    }
52
53    // A CLI that returns exactly the object is the common case and needs no
54    // scanning at all.
55    if let Ok(value) = serde_json::from_str::<Value>(text.trim()) {
56        push(value);
57    }
58
59    for opener in *b"{[" {
60        for (start, end) in spans(text, opener) {
61            if let Ok(value) = serde_json::from_str::<Value>(&text[start..end]) {
62                push(value);
63            }
64        }
65    }
66    out
67}
68
69/// Byte ranges of balanced `{}` or `[]` pairs, last closing first.
70///
71/// String aware, which the backwards scan this replaces was not. A review whose
72/// detail quotes code is the ordinary case, and code has braces: an unbalanced
73/// `{` inside a string made the depth reach zero at the wrong byte, the
74/// substring failed to parse, and the outer object was never found at all. The
75/// answer was then reported as "a JSON array", because the `[` pass had matched
76/// the findings list, which is not what happened and sends the retry after the
77/// wrong problem.
78///
79/// Last closing first because `extract_into` tries candidates in order and a
80/// model that emits a draft before its real answer means the later one.
81fn spans(text: &str, opener: u8) -> Vec<(usize, usize)> {
82    let closer = if opener == b'{' { b'}' } else { b']' };
83    let bytes = text.as_bytes();
84    let mut open_at: Vec<usize> = Vec::new();
85    let mut found: Vec<(usize, usize)> = Vec::new();
86    let mut in_string = false;
87    let mut escaped = false;
88    for (i, byte) in bytes.iter().enumerate() {
89        if escaped {
90            escaped = false;
91            continue;
92        }
93        match *byte {
94            b'\\' if in_string => escaped = true,
95            b'"' => in_string = !in_string,
96            _ if in_string => {}
97            b if b == opener => open_at.push(i),
98            b if b == closer => {
99                if let Some(start) = open_at.pop() {
100                    found.push((start, i + 1));
101                }
102            }
103            _ => {}
104        }
105    }
106    found.sort_by(|a, b| b.1.cmp(&a.1).then(a.0.cmp(&b.0)));
107    found
108}
109
110/// Whether a response looks cut off rather than merely malformed.
111///
112/// A model with an output limit stops mid-object on a long answer. The braces
113/// it opened outnumber the ones it closed, and every complete object left is
114/// something nested inside the one it was building.
115pub fn looks_truncated(text: &str) -> bool {
116    let mut opened = 0i64;
117    let mut in_string = false;
118    let mut escaped = false;
119    for c in text.chars() {
120        if escaped {
121            escaped = false;
122            continue;
123        }
124        match c {
125            '\\' if in_string => escaped = true,
126            '"' => in_string = !in_string,
127            '{' if !in_string => opened += 1,
128            '}' if !in_string => opened -= 1,
129            _ => {}
130        }
131    }
132    opened > 0
133}
134
135/// Parse a model response straight into a typed value.
136///
137/// Tries every candidate rather than only the last one found. A review cut off
138/// before its outer object closed used to yield the last *nested* finding,
139/// which parsed as JSON perfectly well and then failed to be a review, and the
140/// error blamed the shape rather than the truncation.
141pub fn extract_into<T: serde::de::DeserializeOwned>(text: &str) -> Result<T> {
142    let found = candidates(text);
143    if found.is_empty() {
144        return Err(SparError::new(if looks_truncated(text) {
145            format!(
146                "the response was cut off before any complete JSON:\n{}",
147                head(text, 400)
148            )
149        } else {
150            format!("no JSON found in response:\n{}", head(text, 800))
151        }));
152    }
153
154    // Which failure to report is not the same question as which candidate to
155    // parse. Any candidate that parses wins, and a stray object never will,
156    // because every schema here requires fields it does not have. But when
157    // nothing parses, this error is handed straight back to the model on the
158    // retry, so it has to be about the answer the model meant.
159    //
160    // Neither end of the list is that. The order here is really last-closing
161    // first, so the last object a response happens to contain leads, and a
162    // model that wrote its answer and then a sentence with an object in it gets
163    // told about the sentence. The biggest candidate is the better guess: an
164    // answer is longer than the fragments around it.
165    let mut failures: Vec<(serde_json::Error, &Value)> = Vec::new();
166    for value in &found {
167        match serde_json::from_value::<T>(value.clone()) {
168            Ok(parsed) => return Ok(parsed),
169            Err(e) => failures.push((e, value)),
170        }
171    }
172    let (error, value) = failures
173        .into_iter()
174        .max_by_key(|(_, value)| value.to_string().len())
175        .expect("non-empty");
176    if looks_truncated(text) {
177        return Err(SparError::new(format!(
178            "the response was cut off before the answer was complete, so only fragments of it \
179             parsed ({error}). Ask for less in one go, or give this agent a CLI flag for native \
180             structured output."
181        )));
182    }
183    Err(SparError::new(format!(
184        "response did not match the expected shape ({error}).{}\nGot: {}",
185        envelope_hint(value),
186        head(&value.to_string(), 600)
187    )))
188}
189
190/// An extra sentence when serde's own message would send the model to the wrong
191/// place.
192///
193/// serde maps a JSON array onto a struct's fields by position, so a bare array
194/// of findings tried as a review fails on field zero: "invalid type: map,
195/// expected a string", where the string is `verdict`. A model told that goes
196/// looking at a field, and the field is not what is wrong. Every schema here
197/// asks for one object, so an array is always the envelope rather than the
198/// contents.
199fn envelope_hint(value: &Value) -> &'static str {
200    if value.is_array() {
201        " The answer was a JSON array, and the schema asks for a single object: \
202         the array belongs in a field of it."
203    } else {
204        ""
205    }
206}
207
208fn head(text: &str, max: usize) -> String {
209    text.chars().take(max).collect()
210}
211
212/// The original public finding key, retained for persisted-state and library
213/// compatibility.
214pub fn finding_key(title: &str, file: &str) -> String {
215    let basis: String = format!("{} {}", title.trim(), file.trim())
216        .to_lowercase()
217        .chars()
218        .filter(|c| c.is_ascii_alphanumeric() || matches!(c, ' ' | '/' | '.' | '_' | '-'))
219        .collect();
220    let basis = basis.split_whitespace().collect::<Vec<_>>().join(" ");
221    short_hash(&basis)
222}
223
224/// An exact identity for a review finding within one answer.
225///
226/// Wording noise, punctuation, and title case are discarded. The full location
227/// is retained because the same complaint at two sites in one file is two
228/// complaints. Cross-round matching uses `stable_finding_key` as a guarded
229/// fallback instead of making this identity lossy.
230pub(crate) fn exact_finding_key(title: &str, file: &str) -> String {
231    identity_key(title, file.trim())
232}
233
234/// A location-tolerant identity used only for unambiguous cross-round matches.
235pub(crate) fn stable_finding_key(title: &str, file: &str) -> String {
236    identity_key(title, &finding_file(file))
237}
238
239/// A title with a leading severity tag removed, such as `[blocking] `.
240///
241/// Reviewers and authors are told to copy a finding's title across exactly so
242/// the two can be matched up, and they mostly do, but one side decorating it
243/// with the severity it already reports in its own field is common enough to
244/// cost a round: the disposition matches nothing, the finding it answered stays
245/// open, and both are reported as unresolved. Only a bracketed tag whose word
246/// is a severity is dropped, so a title that genuinely opens with a bracketed
247/// subject keeps it and two findings that differ only there stay distinct.
248pub(crate) fn untagged_title(title: &str) -> &str {
249    let trimmed = title.trim();
250    let Some(rest) = trimmed.strip_prefix('[') else {
251        return trimmed;
252    };
253    let Some((tag, rest)) = rest.split_once(']') else {
254        return trimmed;
255    };
256    if crate::model::Severity::parse_lenient(tag.trim()).is_none() {
257        return trimmed;
258    }
259    let rest = rest.trim_start();
260    if rest.is_empty() {
261        return trimmed;
262    }
263    rest
264}
265
266fn identity_key(title: &str, file: &str) -> String {
267    let title: String = untagged_title(title)
268        .to_lowercase()
269        .chars()
270        .filter(|c| c.is_ascii_alphanumeric() || c.is_whitespace())
271        .collect();
272    let title = title.split_whitespace().collect::<Vec<_>>().join(" ");
273    let basis = format!("{title}\0{file}");
274
275    short_hash(&basis)
276}
277
278fn short_hash(basis: &str) -> String {
279    let digest = Sha256::digest(basis.as_bytes());
280    let hex: String = digest.iter().map(|b| format!("{b:02x}")).collect();
281    hex[..12].to_string()
282}
283
284/// A finding's repository path without an optional trailing line or column.
285///
286/// Review locations move as fixes land. The path identifies the point across
287/// rounds, while the full location is still kept on the finding for display.
288pub(crate) fn finding_file(file: &str) -> String {
289    let mut path = file.trim();
290    while let Some((head, suffix)) = path.rsplit_once(':') {
291        let is_number = !suffix.is_empty() && suffix.chars().all(|c| c.is_ascii_digit());
292        let is_range = suffix.split_once('-').is_some_and(|(start, end)| {
293            !start.is_empty()
294                && !end.is_empty()
295                && start.chars().all(|c| c.is_ascii_digit())
296                && end.chars().all(|c| c.is_ascii_digit())
297        });
298        if !is_number && !is_range {
299            break;
300        }
301        path = head.trim_end();
302    }
303    path.to_string()
304}
305
306#[cfg(test)]
307mod tests {
308    use super::*;
309
310    #[test]
311    fn bare_object() {
312        assert_eq!(
313            serde_json::json!({"a": 1}),
314            extract_json(r#"{"a": 1}"#).unwrap()
315        );
316    }
317
318    #[test]
319    fn fenced_block() {
320        let out = extract_json("here you go:\n```json\n{\"a\": 1}\n```\n").unwrap();
321        assert_eq!(serde_json::json!({"a": 1}), out);
322    }
323
324    #[test]
325    fn trailing_prose() {
326        let out = extract_json("Thoughts...\n{\"verdict\": \"approve\"}\nDone.").unwrap();
327        assert_eq!(serde_json::json!({"verdict": "approve"}), out);
328    }
329
330    #[test]
331    fn picks_the_last_fenced_block() {
332        let text = "```json\n{\"n\": 1}\n```\nrevised:\n```json\n{\"n\": 2}\n```";
333        assert_eq!(serde_json::json!({"n": 2}), extract_json(text).unwrap());
334    }
335
336    #[test]
337    fn nested_braces() {
338        let payload = r#"{"findings": [{"severity": "nit", "d": {"x": [1, 2]}}]}"#;
339        let out = extract_json(&format!("blah {payload} blah")).unwrap();
340        assert_eq!(1, out["findings"].as_array().unwrap().len());
341    }
342
343    #[test]
344    fn top_level_array() {
345        let out = extract_json("result: [1, 2, 3]").unwrap();
346        assert_eq!(3, out.as_array().unwrap().len());
347    }
348
349    #[test]
350    fn multibyte_prose_around_the_payload_does_not_panic() {
351        let out = extract_json("\u{1f600}\u{1f600} {\"a\": 1} \u{1f600}").unwrap();
352        assert_eq!(serde_json::json!({"a": 1}), out);
353    }
354
355    #[test]
356    fn raises_when_there_is_none() {
357        assert!(extract_json("no json here at all").is_err());
358    }
359
360    #[test]
361    fn raises_on_empty() {
362        assert!(extract_json("   ").is_err());
363    }
364
365    #[test]
366    fn malformed_trailing_object_falls_back_to_an_earlier_one() {
367        let text = "{\"good\": true}\nthen: {\"bad\": ,}";
368        assert_eq!(
369            serde_json::json!({"good": true}),
370            extract_json(text).unwrap()
371        );
372    }
373
374    // -- finding_key -----------------------------------------------------
375
376    #[test]
377    fn key_is_stable_across_wording_noise() {
378        assert_eq!(
379            finding_key("Unbounded loop!", "src/x.rs"),
380            finding_key("unbounded loop", "src/x.rs")
381        );
382    }
383
384    #[test]
385    fn key_differs_by_file() {
386        assert_ne!(finding_key("t", "a.rs"), finding_key("t", "b.rs"));
387    }
388
389    #[test]
390    fn public_key_keeps_its_original_case_insensitive_paths() {
391        assert_eq!(
392            finding_key("t", "src/Main.rs"),
393            finding_key("t", "src/main.rs")
394        );
395        assert_ne!(
396            exact_finding_key("t", "src/Main.rs"),
397            exact_finding_key("t", "src/main.rs")
398        );
399    }
400
401    #[test]
402    fn key_is_stable_across_whitespace() {
403        assert_eq!(finding_key("a  b", "x.rs"), finding_key(" a b ", "x.rs"));
404    }
405
406    #[test]
407    fn a_severity_tag_does_not_change_a_title() {
408        assert_eq!(
409            "Unbounded loop",
410            untagged_title("[blocking] Unbounded loop")
411        );
412        assert_eq!("Unbounded loop", untagged_title("  [ NIT ]Unbounded loop "));
413        assert_eq!(
414            exact_finding_key("Unbounded loop", "x.rs"),
415            exact_finding_key("[non-blocking] Unbounded loop", "x.rs")
416        );
417    }
418
419    #[test]
420    fn a_bracketed_subject_is_part_of_the_title() {
421        assert_eq!("[iOS] Startup crash", untagged_title("[iOS] Startup crash"));
422        assert_eq!("[blocking]", untagged_title("[blocking]"));
423        assert_eq!("[blocking Unbounded", untagged_title("[blocking Unbounded"));
424        assert_ne!(
425            exact_finding_key("[iOS] Startup crash", "x.rs"),
426            exact_finding_key("[Android] Startup crash", "x.rs")
427        );
428    }
429
430    #[test]
431    fn exact_key_keeps_distinct_locations() {
432        assert_ne!(
433            exact_finding_key("t", "src/net.rs:88"),
434            exact_finding_key("t", "src/net.rs:91")
435        );
436        assert_eq!(
437            stable_finding_key("t", "src/net.rs:88"),
438            stable_finding_key("t", "src/net.rs:91")
439        );
440        assert_eq!(
441            stable_finding_key("t", "src/net.rs:88-94"),
442            stable_finding_key("t", "src/net.rs")
443        );
444        assert_eq!(
445            stable_finding_key("t", "src/net.rs:88:12"),
446            stable_finding_key("t", "src/net.rs")
447        );
448    }
449
450    #[test]
451    fn a_numeric_filename_is_not_treated_as_a_line_number() {
452        assert_eq!("fixtures/2024", finding_file("fixtures/2024"));
453    }
454
455    #[test]
456    fn key_is_twelve_hex_characters() {
457        let key = finding_key("anything", "file.rs");
458        assert_eq!(12, key.len());
459        assert!(key.chars().all(|c| c.is_ascii_hexdigit()));
460    }
461}
462
463#[cfg(test)]
464mod brace_tests {
465    use super::*;
466
467    /// A finding that quotes code is the ordinary case, and code has braces.
468    ///
469    /// The backwards scan counted raw bytes, so a lone `{` inside a string made
470    /// the depth reach zero at the wrong byte and the outer object was never
471    /// found. The `[` pass then offered the findings array as the only
472    /// candidate, and the model was told "the answer was a JSON array", which
473    /// is not what happened, so the retry was spent on the wrong problem.
474    #[test]
475    fn a_brace_inside_a_string_does_not_lose_the_answer() {
476        let text = r#"{"verdict":"approve","next_action":"merge","summary":"the loop `for (;;) {` never breaks","findings":[]}"#;
477        let found = candidates(text);
478        assert!(!found.is_empty(), "nothing was found at all");
479        assert_eq!(
480            "approve",
481            found[0]["verdict"].as_str().unwrap_or_default(),
482            "the outer object was lost: {found:?}"
483        );
484    }
485
486    /// The same, in the other direction: an unbalanced closer.
487    #[test]
488    fn an_unbalanced_closer_inside_a_string_is_not_a_closer() {
489        let text = r#"prose first
490{"summary":"the guard ends with } and no else","findings":[{"title":"a {"}]}"#;
491        let found = candidates(text);
492        assert_eq!(
493            "the guard ends with } and no else",
494            found[0]["summary"].as_str().unwrap_or_default(),
495            "{found:?}"
496        );
497    }
498
499    /// A model that answers with exactly the object needs no scanning, and
500    /// that is the common case for a CLI with native structured output.
501    #[test]
502    fn the_whole_answer_is_tried_before_anything_is_scanned() {
503        let found = candidates("  {\"a\":1}\n");
504        assert_eq!(1, found[0]["a"].as_i64().unwrap_or_default());
505    }
506
507    /// An array answer still reaches the parser, braces in its strings and all.
508    #[test]
509    fn an_array_with_braces_in_its_strings_still_parses() {
510        let text = r#"here it is: [{"title":"missing } in the macro"},{"title":"second"}]"#;
511        let found = candidates(text);
512        let array = found
513            .iter()
514            .find(|v| v.is_array())
515            .expect("the array was lost");
516        assert_eq!(2, array.as_array().unwrap().len());
517    }
518}
519
520#[cfg(test)]
521mod truncation_tests {
522    use super::*;
523    use serde::Deserialize;
524
525    #[derive(Debug, Deserialize)]
526    struct Review {
527        verdict: String,
528        findings: Vec<Finding>,
529    }
530    #[derive(Debug, Deserialize)]
531    struct Finding {
532        title: String,
533    }
534
535    /// What actually happened on a real pull request. A long review hit the
536    /// model's output limit and stopped before its outer object closed, so the
537    /// last complete JSON in the response was a nested finding. It parsed, it
538    /// was not a review, and the error blamed the shape.
539    const TRUNCATED: &str = r#"Here is my review.
540{"verdict":"changes_requested","next_action":"hand_back","summary":"Two problems.",
541 "findings":[
542   {"severity":"blocking","title":"First","detail":"one","file":"a.ts","in_scope":true},
543   {"severity":"non-blocking","title":"Second","detail":"numbers unnamed keys by position"#;
544
545    #[test]
546    fn a_truncated_review_is_reported_as_truncated_not_as_the_wrong_shape() {
547        let err = extract_into::<Review>(TRUNCATED).unwrap_err().to_string();
548        assert!(err.contains("cut off"), "{err}");
549        assert!(!err.contains("did not match the expected shape"), "{err}");
550    }
551
552    #[test]
553    fn truncation_is_detected_from_the_unclosed_braces() {
554        assert!(looks_truncated(TRUNCATED));
555        assert!(!looks_truncated(r#"{"a":1}"#));
556        // A brace inside a string is not an open brace.
557        assert!(!looks_truncated(r#"{"a":"a { in a string"}"#));
558        assert!(!looks_truncated(r#"{"a":"an escaped \" quote { here"}"#));
559    }
560
561    /// The fix that matters: the right object is found even when it is not the
562    /// last one in the response.
563    #[test]
564    fn the_review_is_found_even_with_nested_objects_after_it() {
565        let text = r#"Thinking out loud first.
566{"verdict":"approve","next_action":"merge","summary":"Fine.","findings":[{"severity":"nit","title":"Wording","detail":"d","file":"a.ts","in_scope":true}]}
567And here is a stray object afterwards: {"title":"not the review"}"#;
568        let review: Review = extract_into(text).unwrap();
569        assert_eq!("approve", review.verdict);
570        assert_eq!(1, review.findings.len());
571        assert_eq!("Wording", review.findings[0].title);
572    }
573
574    #[test]
575    fn candidates_are_offered_most_likely_first() {
576        let text = "```json\n{\"verdict\":\"approve\",\"findings\":[]}\n```\ntrailing {\"x\":1}";
577        let review: Review = extract_into(text).unwrap();
578        assert_eq!("approve", review.verdict);
579    }
580
581    #[test]
582    fn a_genuinely_wrong_shape_still_says_so() {
583        let err = extract_into::<Review>(r#"{"colour":"blue"}"#)
584            .unwrap_err()
585            .to_string();
586        assert!(err.contains("did not match the expected shape"), "{err}");
587        assert!(!err.contains("cut off"), "{err}");
588    }
589
590    /// What a real round two review produced. The model wrote a review object
591    /// and a findings array, the object failed to parse, and the complaint that
592    /// went back to it described the array: "invalid type: map, expected a
593    /// string", which is field zero of a struct serde had mapped an array onto
594    /// by position. The model was sent to look at a field, and the field was
595    /// not what was wrong.
596    #[test]
597    fn the_complaint_is_about_the_answer_the_model_meant() {
598        let text = r#"Here is my review.
599{"verdict":"changes_requested","next_action":"hand_back","summary":"Two problems.","findings":"should have been a list"}
600Supporting detail: [{"detail":"The working tree bumps 0.5.9 to 0.5.10."}]"#;
601        let err = extract_into::<Review>(text).unwrap_err().to_string();
602        // The review object is what failed, and its own field is named.
603        assert!(err.contains("findings"), "{err}");
604        assert!(
605            err.contains("changes_requested"),
606            "the object is shown:\n{err}"
607        );
608        assert!(
609            !err.contains("The working tree"),
610            "the stray array leaked in:\n{err}"
611        );
612    }
613
614    /// serde maps a JSON array onto a struct by position, so a bare array of
615    /// findings fails on the first field and says "expected a string". Left at
616    /// that, the model reads it as a field problem.
617    #[test]
618    fn a_bare_array_is_named_as_the_envelope_problem() {
619        let err = extract_into::<Review>(r#"[{"title":"First"},{"title":"Second"}]"#)
620            .unwrap_err()
621            .to_string();
622        assert!(err.contains("JSON array"), "{err}");
623        assert!(err.contains("single object"), "{err}");
624    }
625
626    /// And an object that is merely the wrong shape says nothing about arrays.
627    #[test]
628    fn a_wrong_object_is_not_told_it_was_an_array() {
629        let err = extract_into::<Review>(r#"{"colour":"blue"}"#)
630            .unwrap_err()
631            .to_string();
632        assert!(!err.contains("JSON array"), "{err}");
633    }
634
635    #[test]
636    fn nothing_parseable_is_still_reported_plainly() {
637        let err = extract_into::<Review>("no json at all")
638            .unwrap_err()
639            .to_string();
640        assert!(err.contains("no JSON found"), "{err}");
641    }
642}