Skip to main content

magi/
verdict.rs

1//! Structured answers extracted from free-form agent output.
2//!
3//! Agents are asked to end with a fenced `json` block. They mostly do, and
4//! sometimes they narrate afterwards, emit two blocks, or wrap the object in
5//! prose. [`extract_json`] therefore scans for *every* balanced top-level
6//! object in the text and returns the last one that deserializes into the type
7//! the caller wants, rather than trusting a single regex to find the right one.
8use std::collections::BTreeMap;
9
10use anyhow::{Result, bail};
11use serde::{Deserialize, Serialize};
12
13/// A judge's independent ranking of the candidates.
14#[derive(Debug, Clone, Serialize, Deserialize)]
15pub struct Ranking {
16    /// Labels, best first. Must be a permutation of the presented labels.
17    pub ranking: Vec<String>,
18    /// Per-label justification.
19    #[serde(default)]
20    pub reasons: BTreeMap<String, String>,
21    /// Self-reported confidence, 1-5.
22    #[serde(default)]
23    pub confidence: Option<u8>,
24}
25
26impl Ranking {
27    /// The judge's first choice.
28    pub fn top(&self) -> Option<&str> {
29        self.ranking.first().map(String::as_str)
30    }
31
32    /// Reject a ranking that is not a permutation of `labels`, so a malformed
33    /// verdict is retried instead of silently skewing the tally.
34    pub fn validate(&self, labels: &[char]) -> Result<()> {
35        let mut got: Vec<char> = self
36            .ranking
37            .iter()
38            .filter_map(|s| s.trim().chars().next())
39            .map(|c| c.to_ascii_uppercase())
40            .collect();
41        got.sort_unstable();
42        got.dedup();
43        let mut want: Vec<char> = labels.to_vec();
44        want.sort_unstable();
45        if got != want {
46            bail!(
47                "ranking {:?} is not a permutation of the candidate labels {:?}",
48                self.ranking,
49                labels
50            );
51        }
52        Ok(())
53    }
54
55    /// Normalise labels to single uppercase characters.
56    pub fn normalized(&self) -> Vec<char> {
57        self.ranking
58            .iter()
59            .filter_map(|s| s.trim().chars().next())
60            .map(|c| c.to_ascii_uppercase())
61            .collect()
62    }
63}
64
65/// A judge's final vote, collected privately.
66#[derive(Debug, Clone, Serialize, Deserialize)]
67pub struct FinalVote {
68    /// The winning label, in this judge's view.
69    pub vote: String,
70    /// Why.
71    #[serde(default)]
72    pub reason: String,
73}
74
75impl FinalVote {
76    /// The voted label as a single uppercase char.
77    pub fn label(&self) -> Option<char> {
78        self.vote
79            .trim()
80            .chars()
81            .next()
82            .map(|c| c.to_ascii_uppercase())
83    }
84}
85
86/// How bad a review finding is.
87#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)]
88#[serde(rename_all = "lowercase")]
89pub enum Severity {
90    /// Cosmetic.
91    Nit,
92    /// Should fix, does not block.
93    Minor,
94    /// Should fix before merge.
95    Major,
96    /// Must fix before merge.
97    Blocker,
98}
99
100impl Severity {
101    /// Does this finding hold the merge?
102    pub fn blocks(self) -> bool {
103        matches!(self, Self::Major | Self::Blocker)
104    }
105}
106
107/// One review finding.
108#[derive(Debug, Clone, Serialize, Deserialize)]
109pub struct Finding {
110    /// Assigned by magi after parsing, e.g. `R1-1-2`. Never trusted from the
111    /// agent, because the fixer's adoption report is keyed by it.
112    #[serde(default)]
113    pub id: String,
114    /// How bad.
115    pub severity: Severity,
116    /// File it concerns.
117    #[serde(default)]
118    pub file: Option<String>,
119    /// Line it concerns.
120    #[serde(default)]
121    pub line: Option<u32>,
122    /// One-line summary.
123    pub title: String,
124    /// The argument.
125    #[serde(default)]
126    pub detail: String,
127}
128
129/// A reviewer seat's verdict on the patch, cast alongside its findings.
130///
131/// Three values, not a boolean, because "fine to proceed" and "fine, but
132/// look at these" are different signals for the operator watching the run —
133/// collapsing them would hide exactly the middle case a lens-based reviewer
134/// is most likely to land on. Ord follows declaration order (least to most
135/// cautious) so a round's [`ReviewVote::worst`] is a plain `max`, the same
136/// trick [`Severity`] uses for `blocks`.
137#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)]
138#[serde(rename_all = "snake_case")]
139pub enum ReviewVote {
140    /// No reservations.
141    Approve,
142    /// Fine to proceed, but the findings below are worth fixing.
143    ApproveWithFindings,
144    /// Do not proceed as-is.
145    Reject,
146}
147
148impl ReviewVote {
149    /// Operator-facing label, used in events and the report — never
150    /// `Debug`, so a rename of a variant does not silently reword history.
151    pub fn label(self) -> &'static str {
152        match self {
153            Self::Approve => "approve",
154            Self::ApproveWithFindings => "approve with findings",
155            Self::Reject => "reject",
156        }
157    }
158
159    /// The most cautious of a set of votes, or `None` if none were cast.
160    ///
161    /// A round's recorded verdict must never read softer than its most
162    /// cautious seat — the same reason a single blocking finding, not an
163    /// average of severities, decides whether a round is clean.
164    pub fn worst(votes: impl IntoIterator<Item = Self>) -> Option<Self> {
165        votes.into_iter().max()
166    }
167}
168
169/// A reviewer's report.
170#[derive(Debug, Clone, Serialize, Deserialize)]
171pub struct Review {
172    /// Findings, worst first is conventional but not required.
173    #[serde(default)]
174    pub findings: Vec<Finding>,
175    /// Reviewer's overall verdict prose.
176    #[serde(default)]
177    pub summary: String,
178    /// This seat's vote. Required, not defaulted: a reviewer that skips it
179    /// is retried the same as one that skipped `severity` on a finding,
180    /// rather than silently counted as an `approve`.
181    pub vote: ReviewVote,
182}
183
184/// A reviewer seat's revote after a split round, cast once it has read every
185/// seat's findings and votes (still numbered, never named — see
186/// [`crate::prompt::review_reconsider`]).
187#[derive(Debug, Clone, Serialize, Deserialize)]
188pub struct ReviewRevote {
189    /// The seat's revote.
190    pub vote: ReviewVote,
191    /// Why, one or two sentences.
192    #[serde(default)]
193    pub reason: String,
194}
195
196/// The fixer's response to a round of findings.
197#[derive(Debug, Clone, Serialize, Deserialize)]
198pub struct FixReport {
199    /// Finding ids that were acted on.
200    #[serde(default)]
201    pub addressed: Vec<String>,
202    /// Finding ids that were deliberately not acted on, with the reason.
203    #[serde(default)]
204    pub rejected: Vec<Rejection>,
205    /// What changed.
206    #[serde(default)]
207    pub notes: String,
208}
209
210/// A finding the fixer declined, and why.
211#[derive(Debug, Clone, Serialize, Deserialize)]
212pub struct Rejection {
213    /// Finding id.
214    pub id: String,
215    /// Argument for not acting.
216    #[serde(default)]
217    pub why: String,
218}
219
220/// A judge's position during deliberation.
221#[derive(Debug, Clone, Serialize, Deserialize)]
222pub struct Position {
223    /// Where the judge currently stands.
224    #[serde(default)]
225    pub tentative: Option<String>,
226}
227
228/// One advisor's independent design proposal, gathered by
229/// `graph::Runner::advise` before a run's implementer seats begin.
230///
231/// No patch and no plan of tool calls - a design proposal is cheap exactly
232/// because it is a few paragraphs an agent can write without touching the
233/// repository, which is what makes running several of them, on every run,
234/// affordable in a way several full implementations were not.
235#[derive(Debug, Clone, Serialize, Deserialize)]
236pub struct Proposal {
237    /// What to do and how, in the advisor's own words.
238    pub approach: String,
239    /// The one tradeoff this design turns on.
240    pub key_tradeoff: String,
241    /// What could go wrong.
242    #[serde(default)]
243    pub risks: Vec<String>,
244    /// Files or modules the design touches.
245    #[serde(default)]
246    pub touches: Vec<String>,
247    /// Why this earns its complexity over the obvious first draft - the
248    /// question that keeps a proposal from being a restatement of the task.
249    pub why_not_naive: String,
250}
251
252impl Proposal {
253    /// Reject a proposal with an empty field that matters, so a malformed or
254    /// lazy answer is retried instead of silently thinning the panel down to
255    /// prose nobody can act on.
256    pub fn validate(&self) -> Result<()> {
257        for (field, value) in [
258            ("approach", &self.approach),
259            ("key_tradeoff", &self.key_tradeoff),
260            ("why_not_naive", &self.why_not_naive),
261        ] {
262            if value.trim().is_empty() {
263                bail!("`{field}` is empty");
264            }
265        }
266        Ok(())
267    }
268}
269
270/// Extract the last balanced JSON object in `text` that parses as `T`.
271///
272/// Handles fenced blocks, trailing prose, and multiple objects. Strings and
273/// escapes are tracked so a `}` inside a string literal does not close an
274/// object early.
275pub fn extract_json<T: serde::de::DeserializeOwned>(text: &str) -> Result<T> {
276    let bytes = text.as_bytes();
277    let mut spans: Vec<(usize, usize)> = Vec::new();
278    let mut i = 0usize;
279    while i < bytes.len() {
280        if bytes[i] != b'{' {
281            i += 1;
282            continue;
283        }
284        let mut depth = 0usize;
285        let mut in_str = false;
286        let mut escaped = false;
287        let mut j = i;
288        while j < bytes.len() {
289            let c = bytes[j];
290            if in_str {
291                if escaped {
292                    escaped = false;
293                } else if c == b'\\' {
294                    escaped = true;
295                } else if c == b'"' {
296                    in_str = false;
297                }
298            } else {
299                match c {
300                    b'"' => in_str = true,
301                    b'{' => depth += 1,
302                    b'}' => {
303                        depth -= 1;
304                        if depth == 0 {
305                            spans.push((i, j + 1));
306                            break;
307                        }
308                    }
309                    _ => {}
310                }
311            }
312            j += 1;
313        }
314        // Skip past this object's opening brace either way; a truncated object
315        // must not make the scan quadratic on long transcripts.
316        i = if depth == 0 && j < bytes.len() {
317            j + 1
318        } else {
319            i + 1
320        };
321    }
322
323    let mut last_err = None;
324    for (start, end) in spans.iter().rev() {
325        match serde_json::from_str::<T>(&text[*start..*end]) {
326            Ok(v) => return Ok(v),
327            Err(e) => last_err = Some(e),
328        }
329    }
330    match last_err {
331        Some(e) => bail!("no JSON object in the reply matched the expected shape: {e}"),
332        None => bail!("the reply contained no JSON object"),
333    }
334}
335
336/// Pull the text after a `## <heading>` marker, to the end or the next heading.
337///
338/// Used for the prose sections agents are asked to emit alongside their JSON.
339pub fn section(text: &str, heading: &str) -> Option<String> {
340    let want = heading.to_ascii_lowercase();
341    let mut out: Option<String> = None;
342    for line in text.lines() {
343        let trimmed = line.trim();
344        if let Some(rest) = trimmed.strip_prefix("##") {
345            let name = rest.trim_start_matches('#').trim().to_ascii_lowercase();
346            if name == want {
347                out = Some(String::new());
348                continue;
349            }
350            if out.is_some() {
351                break;
352            }
353            continue;
354        }
355        if let Some(buf) = out.as_mut() {
356            buf.push_str(line);
357            buf.push('\n');
358        }
359    }
360    out.map(|s| s.trim().to_owned()).filter(|s| !s.is_empty())
361}
362
363/// The marker `prompt::implement`'s reply format documents for an
364/// implementer that investigated and concluded, with evidence, that no code
365/// change belongs in this worktree — see that prompt's "Reply format"
366/// section for the exact wording asked for.
367pub const NO_CHANGE_NEEDED_MARKER: &str = "NO CHANGE NEEDED:";
368
369/// Pull the evidence out of an implementer's `## SUMMARY` section when it
370/// opens with [`NO_CHANGE_NEEDED_MARKER`].
371///
372/// Returns `None` for an ordinary bullet-list summary, and also for a
373/// marker with nothing after it — an unsupported claim must read exactly
374/// like a candidate that never made one, not like a verified one with an
375/// empty reason. This only recognises the *declaration*; whether it is
376/// trusted is [`crate::graph`]'s adoption guard's call, made from evidence
377/// this function cannot see (the CLI's own exit status, whether the tree is
378/// really empty, whether a command went unconfirmed).
379pub fn verified_noop(summary: &str) -> Option<String> {
380    let evidence = summary.trim_start().strip_prefix(NO_CHANGE_NEEDED_MARKER)?;
381    let evidence = evidence.trim();
382    (!evidence.is_empty()).then(|| evidence.to_owned())
383}
384
385#[cfg(test)]
386mod tests {
387    use super::*;
388
389    #[test]
390    fn fenced_block_is_found() {
391        let text = "Here is my verdict.\n\n```json\n{\"ranking\":[\"B\",\"A\"]}\n```\n";
392        let r: Ranking = extract_json(text).unwrap();
393        assert_eq!(r.top(), Some("B"));
394    }
395
396    #[test]
397    fn last_matching_object_wins_over_an_earlier_example() {
398        let text = concat!(
399            "The format is {\"ranking\":[\"X\"]} for illustration.\n",
400            "```json\n{\"ranking\":[\"C\",\"A\",\"B\"],\"confidence\":4}\n```\n",
401            "Happy to elaborate.\n"
402        );
403        let r: Ranking = extract_json(text).unwrap();
404        assert_eq!(r.normalized(), ['C', 'A', 'B']);
405        assert_eq!(r.confidence, Some(4));
406    }
407
408    #[test]
409    fn braces_inside_strings_do_not_close_the_object() {
410        let text = r#"{"ranking":["A"],"reasons":{"A":"uses format!(\"{}\", x) safely}"}}"#;
411        let r: Ranking = extract_json(text).unwrap();
412        assert_eq!(r.top(), Some("A"));
413        assert!(r.reasons["A"].contains("format!"));
414    }
415
416    #[test]
417    fn objects_of_the_wrong_shape_are_skipped() {
418        let text = concat!(
419            "```json\n{\"ranking\":[\"A\",\"B\"]}\n```\n",
420            "and some telemetry: {\"tokens\":123}\n"
421        );
422        let r: Ranking = extract_json(text).unwrap();
423        assert_eq!(r.normalized(), ['A', 'B']);
424    }
425
426    #[test]
427    fn no_json_is_an_error_not_a_default() {
428        let err = extract_json::<Ranking>("I decline to produce JSON.").unwrap_err();
429        assert!(err.to_string().contains("no JSON object"));
430    }
431
432    #[test]
433    fn truncated_object_does_not_hang() {
434        let err = extract_json::<Ranking>("{\"ranking\": [\"A\"").unwrap_err();
435        assert!(err.to_string().contains("no JSON object"));
436    }
437
438    #[test]
439    fn ranking_validation_rejects_a_non_permutation() {
440        let r = Ranking {
441            ranking: vec!["A".to_owned(), "A".to_owned()],
442            reasons: BTreeMap::new(),
443            confidence: None,
444        };
445        assert!(r.validate(&['A', 'B', 'C']).is_err());
446
447        let r = Ranking {
448            ranking: vec!["c".to_owned(), "B".to_owned(), "A".to_owned()],
449            reasons: BTreeMap::new(),
450            confidence: None,
451        };
452        r.validate(&['A', 'B', 'C']).expect("case is normalised");
453        assert_eq!(r.normalized(), ['C', 'B', 'A']);
454    }
455
456    #[test]
457    fn final_vote_label_is_normalised() {
458        let v: FinalVote = extract_json(r#"{"vote":" b ","reason":"tests"}"#).unwrap();
459        assert_eq!(v.label(), Some('B'));
460    }
461
462    #[test]
463    fn severity_blocking_is_major_and_up() {
464        assert!(Severity::Blocker.blocks());
465        assert!(Severity::Major.blocks());
466        assert!(!Severity::Minor.blocks());
467        assert!(!Severity::Nit.blocks());
468        assert!(Severity::Blocker > Severity::Nit);
469    }
470
471    #[test]
472    fn review_parses_with_optional_fields_missing() {
473        let r: Review = extract_json(
474            r#"{"vote":"reject","findings":[{"severity":"blocker","title":"panics on empty input"}]}"#,
475        )
476        .unwrap();
477        assert_eq!(r.findings.len(), 1);
478        assert!(r.findings[0].file.is_none());
479        assert_eq!(r.findings[0].id, "");
480        assert_eq!(r.vote, ReviewVote::Reject);
481    }
482
483    #[test]
484    fn review_without_a_vote_is_rejected_rather_than_defaulted() {
485        let err = extract_json::<Review>(r#"{"findings":[]}"#).unwrap_err();
486        assert!(err.to_string().contains("no JSON object"), "{err}");
487    }
488
489    #[test]
490    fn review_vote_worst_is_the_most_cautious() {
491        assert_eq!(
492            ReviewVote::worst([ReviewVote::Approve, ReviewVote::Reject, ReviewVote::Approve]),
493            Some(ReviewVote::Reject)
494        );
495        assert_eq!(
496            ReviewVote::worst([ReviewVote::Approve, ReviewVote::ApproveWithFindings]),
497            Some(ReviewVote::ApproveWithFindings)
498        );
499        assert_eq!(ReviewVote::worst(Vec::<ReviewVote>::new()), None);
500    }
501
502    #[test]
503    fn review_revote_parses_the_reconsideration_shape() {
504        let r: ReviewRevote =
505            extract_json(r#"{"vote":"approve","reason":"the other findings do not hold"}"#)
506                .unwrap();
507        assert_eq!(r.vote, ReviewVote::Approve);
508        assert_eq!(r.reason, "the other findings do not hold");
509    }
510
511    #[test]
512    fn fix_report_parses_rejections() {
513        let f: FixReport = extract_json(
514            r#"{"addressed":["R1-1-1"],"rejected":[{"id":"R1-2-1","why":"not reachable"}]}"#,
515        )
516        .unwrap();
517        assert_eq!(f.addressed, ["R1-1-1"]);
518        assert_eq!(f.rejected[0].id, "R1-2-1");
519    }
520
521    #[test]
522    fn proposal_parses_with_optional_fields_missing() {
523        let p: Proposal = extract_json(
524            r#"{"approach":"do X","key_tradeoff":"simpler now, slower later","why_not_naive":"the naive version corrupts state under a retry"}"#,
525        )
526        .unwrap();
527        assert!(p.risks.is_empty());
528        assert!(p.touches.is_empty());
529        p.validate()
530            .expect("a proposal with only required fields is valid");
531    }
532
533    #[test]
534    fn proposal_validation_rejects_an_empty_required_field() {
535        let p = Proposal {
536            approach: String::new(),
537            key_tradeoff: "t".to_owned(),
538            risks: Vec::new(),
539            touches: Vec::new(),
540            why_not_naive: "w".to_owned(),
541        };
542        let err = p.validate().unwrap_err();
543        assert!(err.to_string().contains("approach"));
544    }
545
546    #[test]
547    fn sections_are_sliced_by_heading() {
548        let text = "## SUMMARY\nchanged the retry loop.\nadded a test.\n\n## NOTES\nignore me\n";
549        assert_eq!(
550            section(text, "summary").unwrap(),
551            "changed the retry loop.\nadded a test."
552        );
553        assert_eq!(section(text, "notes").unwrap(), "ignore me");
554        assert!(section(text, "missing").is_none());
555    }
556
557    #[test]
558    fn no_change_needed_marker_yields_its_evidence() {
559        let summary = "NO CHANGE NEEDED: already fixed by b32cfc4, which is on main \
560                        (git merge-base --is-ancestor confirms it).";
561        assert_eq!(
562            verified_noop(summary).unwrap(),
563            "already fixed by b32cfc4, which is on main (git merge-base --is-ancestor \
564             confirms it)."
565        );
566    }
567
568    #[test]
569    fn no_change_needed_marker_with_no_evidence_is_not_verified() {
570        assert!(verified_noop("NO CHANGE NEEDED:").is_none());
571        assert!(verified_noop("NO CHANGE NEEDED:   \n  ").is_none());
572    }
573
574    #[test]
575    fn an_ordinary_summary_is_not_verified() {
576        assert!(verified_noop("- changed the retry loop.\n- added a test.").is_none());
577        // The marker only counts leading the section, not mentioned in passing.
578        assert!(verified_noop("I considered NO CHANGE NEEDED: but wrote a fix instead.").is_none());
579    }
580}