magi/

verdict.rs

//! Structured answers extracted from free-form agent output.
//!
//! Agents are asked to end with a fenced `json` block. They mostly do, and
//! sometimes they narrate afterwards, emit two blocks, or wrap the object in
//! prose. [`extract_json`] therefore scans for *every* balanced top-level
//! object in the text and returns the last one that deserializes into the type
//! the caller wants, rather than trusting a single regex to find the right one.
use std::collections::BTreeMap;

use anyhow::{Result, bail};
use serde::{Deserialize, Serialize};

/// A judge's independent ranking of the candidates.
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct Ranking {
    /// Labels, best first. Must be a permutation of the presented labels.
    pub ranking: Vec<String>,
    /// Per-label justification.
    #[serde(default)]
    pub reasons: BTreeMap<String, String>,
    /// Self-reported confidence, 1-5.
    #[serde(default)]
    pub confidence: Option<u8>,
    /// Per-label strengths worth porting onto the winner, written for the
    /// candidates the judge did not rank first. Optional: a judge that
    /// names none simply leaves the graft phase nothing to do.
    #[serde(default, deserialize_with = "lenient_strengths")]
    pub strengths: BTreeMap<String, Vec<String>>,
}

/// Read `strengths` without ever failing: this field is optional, so a value of
/// the wrong shape (`null`, a string, a map of plain strings) must cost the
/// judge nothing but the strengths. Entries that are not a string array keep
/// their string members only; anything else is dropped.
fn lenient_strengths<'de, D>(d: D) -> Result<BTreeMap<String, Vec<String>>, D::Error>
where
    D: serde::Deserializer<'de>,
{
    let value = serde_json::Value::deserialize(d)?;
    let mut out = BTreeMap::new();
    if let serde_json::Value::Object(map) = value {
        for (label, v) in map {
            match v {
                serde_json::Value::Array(items) => {
                    let list: Vec<String> = items
                        .into_iter()
                        .filter_map(|i| i.as_str().map(str::to_owned))
                        .collect();
                    if !list.is_empty() {
                        out.insert(label, list);
                    }
                }
                serde_json::Value::String(s) if !s.trim().is_empty() => {
                    out.insert(label, vec![s]);
                }
                _ => {}
            }
        }
    }
    Ok(out)
}

impl Ranking {
    /// The judge's first choice.
    pub fn top(&self) -> Option<&str> {
        self.ranking.first().map(String::as_str)
    }

    /// Reject a ranking that is not a permutation of `labels`, so a malformed
    /// verdict is retried instead of silently skewing the tally.
    pub fn validate(&self, labels: &[char]) -> Result<()> {
        let mut got: Vec<char> = self
            .ranking
            .iter()
            .filter_map(|s| s.trim().chars().next())
            .map(|c| c.to_ascii_uppercase())
            .collect();
        got.sort_unstable();
        got.dedup();
        let mut want: Vec<char> = labels.to_vec();
        want.sort_unstable();
        if got != want {
            bail!(
                "ranking {:?} is not a permutation of the candidate labels {:?}",
                self.ranking,
                labels
            );
        }
        Ok(())
    }

    /// Normalise labels to single uppercase characters.
    pub fn normalized(&self) -> Vec<char> {
        self.ranking
            .iter()
            .filter_map(|s| s.trim().chars().next())
            .map(|c| c.to_ascii_uppercase())
            .collect()
    }
}

/// A judge's final vote, collected privately.
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct FinalVote {
    /// The winning label, in this judge's view.
    pub vote: String,
    /// Why.
    #[serde(default)]
    pub reason: String,
}

impl FinalVote {
    /// The voted label as a single uppercase char.
    pub fn label(&self) -> Option<char> {
        self.vote
            .trim()
            .chars()
            .next()
            .map(|c| c.to_ascii_uppercase())
    }
}

/// How bad a review finding is.
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)]
#[serde(rename_all = "lowercase")]
pub enum Severity {
    /// Cosmetic.
    Nit,
    /// Should fix, does not block.
    Minor,
    /// Should fix before merge.
    Major,
    /// Must fix before merge.
    Blocker,
}

impl Severity {
    /// Does this finding hold the merge?
    pub fn blocks(self) -> bool {
        matches!(self, Self::Major | Self::Blocker)
    }
}

/// One review finding.
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct Finding {
    /// Assigned by magi after parsing, e.g. `R1-1-2`. Never trusted from the
    /// agent, because the fixer's adoption report is keyed by it.
    #[serde(default)]
    pub id: String,
    /// How bad.
    pub severity: Severity,
    /// File it concerns.
    #[serde(default)]
    pub file: Option<String>,
    /// Line it concerns.
    #[serde(default)]
    pub line: Option<u32>,
    /// One-line summary.
    pub title: String,
    /// The argument.
    #[serde(default)]
    pub detail: String,
}

/// A reviewer seat's verdict on the patch, cast alongside its findings.
///
/// Three values, not a boolean, because "fine to proceed" and "fine, but
/// look at these" are different signals for the operator watching the run —
/// collapsing them would hide exactly the middle case a lens-based reviewer
/// is most likely to land on. Ord follows declaration order (least to most
/// cautious) so a round's [`ReviewVote::worst`] is a plain `max`, the same
/// trick [`Severity`] uses for `blocks`.
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)]
#[serde(rename_all = "snake_case")]
pub enum ReviewVote {
    /// No reservations.
    Approve,
    /// Fine to proceed, but the findings below are worth fixing.
    ApproveWithFindings,
    /// Do not proceed as-is.
    Reject,
}

impl ReviewVote {
    /// Operator-facing label, used in events and the report — never
    /// `Debug`, so a rename of a variant does not silently reword history.
    pub fn label(self) -> &'static str {
        match self {
            Self::Approve => "approve",
            Self::ApproveWithFindings => "approve with findings",
            Self::Reject => "reject",
        }
    }

    /// The most cautious of a set of votes, or `None` if none were cast.
    ///
    /// A round's recorded verdict must never read softer than its most
    /// cautious seat — the same reason a single blocking finding, not an
    /// average of severities, decides whether a round is clean.
    pub fn worst(votes: impl IntoIterator<Item = Self>) -> Option<Self> {
        votes.into_iter().max()
    }
}

/// A reviewer's report.
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct Review {
    /// Findings, worst first is conventional but not required.
    #[serde(default)]
    pub findings: Vec<Finding>,
    /// Reviewer's overall verdict prose.
    #[serde(default)]
    pub summary: String,
    /// This seat's vote. Required, not defaulted: a reviewer that skips it
    /// is retried the same as one that skipped `severity` on a finding,
    /// rather than silently counted as an `approve`.
    pub vote: ReviewVote,
}

/// A reviewer seat's revote after a split round, cast once it has read every
/// seat's findings and votes (still numbered, never named — see
/// [`crate::prompt::review_reconsider`]).
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct ReviewRevote {
    /// The seat's revote.
    pub vote: ReviewVote,
    /// Why, one or two sentences.
    #[serde(default)]
    pub reason: String,
}

/// The fixer's response to a round of findings.
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct FixReport {
    /// Finding ids that were acted on.
    #[serde(default)]
    pub addressed: Vec<String>,
    /// Finding ids that were deliberately not acted on, with the reason.
    #[serde(default)]
    pub rejected: Vec<Rejection>,
    /// What changed.
    #[serde(default)]
    pub notes: String,
}

/// A finding the fixer declined, and why.
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct Rejection {
    /// Finding id.
    pub id: String,
    /// Argument for not acting.
    #[serde(default)]
    pub why: String,
}

/// A judge's position during deliberation.
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct Position {
    /// Where the judge currently stands.
    #[serde(default)]
    pub tentative: Option<String>,
}

/// One advisor's independent design proposal, gathered by
/// `graph::Runner::advise` before a run's implementer seats begin.
///
/// No patch and no plan of tool calls - a design proposal is cheap exactly
/// because it is a few paragraphs an agent can write without touching the
/// repository, which is what makes running several of them, on every run,
/// affordable in a way several full implementations were not.
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct Proposal {
    /// What to do and how, in the advisor's own words.
    pub approach: String,
    /// The one tradeoff this design turns on.
    pub key_tradeoff: String,
    /// What could go wrong.
    #[serde(default)]
    pub risks: Vec<String>,
    /// Files or modules the design touches.
    #[serde(default)]
    pub touches: Vec<String>,
    /// Why this earns its complexity over the obvious first draft - the
    /// question that keeps a proposal from being a restatement of the task.
    pub why_not_naive: String,
}

impl Proposal {
    /// Reject a proposal with an empty field that matters, so a malformed or
    /// lazy answer is retried instead of silently thinning the panel down to
    /// prose nobody can act on.
    pub fn validate(&self) -> Result<()> {
        for (field, value) in [
            ("approach", &self.approach),
            ("key_tradeoff", &self.key_tradeoff),
            ("why_not_naive", &self.why_not_naive),
        ] {
            if value.trim().is_empty() {
                bail!("`{field}` is empty");
            }
        }
        Ok(())
    }
}

/// Extract the last balanced JSON object in `text` that parses as `T`.
///
/// Handles fenced blocks, trailing prose, and multiple objects. Strings and
/// escapes are tracked so a `}` inside a string literal does not close an
/// object early.
pub fn extract_json<T: serde::de::DeserializeOwned>(text: &str) -> Result<T> {
    let bytes = text.as_bytes();
    let mut spans: Vec<(usize, usize)> = Vec::new();
    let mut i = 0usize;
    while i < bytes.len() {
        if bytes[i] != b'{' {
            i += 1;
            continue;
        }
        let mut depth = 0usize;
        let mut in_str = false;
        let mut escaped = false;
        let mut j = i;
        while j < bytes.len() {
            let c = bytes[j];
            if in_str {
                if escaped {
                    escaped = false;
                } else if c == b'\\' {
                    escaped = true;
                } else if c == b'"' {
                    in_str = false;
                }
            } else {
                match c {
                    b'"' => in_str = true,
                    b'{' => depth += 1,
                    b'}' => {
                        depth -= 1;
                        if depth == 0 {
                            spans.push((i, j + 1));
                            break;
                        }
                    }
                    _ => {}
                }
            }
            j += 1;
        }
        // Skip past this object's opening brace either way; a truncated object
        // must not make the scan quadratic on long transcripts.
        i = if depth == 0 && j < bytes.len() {
            j + 1
        } else {
            i + 1
        };
    }

    let mut last_err = None;
    for (start, end) in spans.iter().rev() {
        match serde_json::from_str::<T>(&text[*start..*end]) {
            Ok(v) => return Ok(v),
            Err(e) => last_err = Some(e),
        }
    }
    match last_err {
        Some(e) => bail!("no JSON object in the reply matched the expected shape: {e}"),
        None => bail!("the reply contained no JSON object"),
    }
}

/// Pull the text after a `## <heading>` marker, to the end or the next heading.
///
/// Used for the prose sections agents are asked to emit alongside their JSON.
pub fn section(text: &str, heading: &str) -> Option<String> {
    let want = heading.to_ascii_lowercase();
    let mut out: Option<String> = None;
    for line in text.lines() {
        let trimmed = line.trim();
        if let Some(rest) = trimmed.strip_prefix("##") {
            let name = rest.trim_start_matches('#').trim().to_ascii_lowercase();
            if name == want {
                out = Some(String::new());
                continue;
            }
            if out.is_some() {
                break;
            }
            continue;
        }
        if let Some(buf) = out.as_mut() {
            buf.push_str(line);
            buf.push('\n');
        }
    }
    out.map(|s| s.trim().to_owned()).filter(|s| !s.is_empty())
}

/// The marker `prompt::implement`'s reply format documents for an
/// implementer that investigated and concluded, with evidence, that no code
/// change belongs in this worktree — see that prompt's "Reply format"
/// section for the exact wording asked for.
pub const NO_CHANGE_NEEDED_MARKER: &str = "NO CHANGE NEEDED:";

/// Pull the evidence out of an implementer's `## SUMMARY` section when it
/// opens with [`NO_CHANGE_NEEDED_MARKER`].
///
/// Returns `None` for an ordinary bullet-list summary, and also for a
/// marker with nothing after it — an unsupported claim must read exactly
/// like a candidate that never made one, not like a verified one with an
/// empty reason. This only recognises the *declaration*; whether it is
/// trusted is [`crate::graph`]'s adoption guard's call, made from evidence
/// this function cannot see (the CLI's own exit status, whether the tree is
/// really empty, whether a command went unconfirmed).
pub fn verified_noop(summary: &str) -> Option<String> {
    let evidence = summary.trim_start().strip_prefix(NO_CHANGE_NEEDED_MARKER)?;
    let evidence = evidence.trim();
    (!evidence.is_empty()).then(|| evidence.to_owned())
}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn malformed_strengths_never_cost_the_ranking() {
        for bad in [
            "null",
            r#"{"B":"one point"}"#,
            r#""x""#,
            r#"[1]"#,
            r#"{"B":[1,"ok"]}"#,
        ] {
            let text = format!(r#"{{"ranking":["A","B"],"strengths":{bad}}}"#);
            let r: Ranking = extract_json(&text).unwrap();
            assert_eq!(r.ranking, ["A", "B"]);
        }
    }

    #[test]
    fn fenced_block_is_found() {
        let text = "Here is my verdict.\n\n```json\n{\"ranking\":[\"B\",\"A\"]}\n```\n";
        let r: Ranking = extract_json(text).unwrap();
        assert_eq!(r.top(), Some("B"));
    }

    #[test]
    fn last_matching_object_wins_over_an_earlier_example() {
        let text = concat!(
            "The format is {\"ranking\":[\"X\"]} for illustration.\n",
            "```json\n{\"ranking\":[\"C\",\"A\",\"B\"],\"confidence\":4}\n```\n",
            "Happy to elaborate.\n"
        );
        let r: Ranking = extract_json(text).unwrap();
        assert_eq!(r.normalized(), ['C', 'A', 'B']);
        assert_eq!(r.confidence, Some(4));
    }

    #[test]
    fn braces_inside_strings_do_not_close_the_object() {
        let text = r#"{"ranking":["A"],"reasons":{"A":"uses format!(\"{}\", x) safely}"}}"#;
        let r: Ranking = extract_json(text).unwrap();
        assert_eq!(r.top(), Some("A"));
        assert!(r.reasons["A"].contains("format!"));
    }

    #[test]
    fn objects_of_the_wrong_shape_are_skipped() {
        let text = concat!(
            "```json\n{\"ranking\":[\"A\",\"B\"]}\n```\n",
            "and some telemetry: {\"tokens\":123}\n"
        );
        let r: Ranking = extract_json(text).unwrap();
        assert_eq!(r.normalized(), ['A', 'B']);
    }

    #[test]
    fn no_json_is_an_error_not_a_default() {
        let err = extract_json::<Ranking>("I decline to produce JSON.").unwrap_err();
        assert!(err.to_string().contains("no JSON object"));
    }

    #[test]
    fn truncated_object_does_not_hang() {
        let err = extract_json::<Ranking>("{\"ranking\": [\"A\"").unwrap_err();
        assert!(err.to_string().contains("no JSON object"));
    }

    #[test]
    fn ranking_validation_rejects_a_non_permutation() {
        let r = Ranking {
            ranking: vec!["A".to_owned(), "A".to_owned()],
            reasons: BTreeMap::new(),
            confidence: None,
            strengths: BTreeMap::new(),
        };
        assert!(r.validate(&['A', 'B', 'C']).is_err());

        let r = Ranking {
            ranking: vec!["c".to_owned(), "B".to_owned(), "A".to_owned()],
            reasons: BTreeMap::new(),
            confidence: None,
            strengths: BTreeMap::new(),
        };
        r.validate(&['A', 'B', 'C']).expect("case is normalised");
        assert_eq!(r.normalized(), ['C', 'B', 'A']);
    }

    #[test]
    fn final_vote_label_is_normalised() {
        let v: FinalVote = extract_json(r#"{"vote":" b ","reason":"tests"}"#).unwrap();
        assert_eq!(v.label(), Some('B'));
    }

    #[test]
    fn severity_blocking_is_major_and_up() {
        assert!(Severity::Blocker.blocks());
        assert!(Severity::Major.blocks());
        assert!(!Severity::Minor.blocks());
        assert!(!Severity::Nit.blocks());
        assert!(Severity::Blocker > Severity::Nit);
    }

    #[test]
    fn review_parses_with_optional_fields_missing() {
        let r: Review = extract_json(
            r#"{"vote":"reject","findings":[{"severity":"blocker","title":"panics on empty input"}]}"#,
        )
        .unwrap();
        assert_eq!(r.findings.len(), 1);
        assert!(r.findings[0].file.is_none());
        assert_eq!(r.findings[0].id, "");
        assert_eq!(r.vote, ReviewVote::Reject);
    }

    #[test]
    fn review_without_a_vote_is_rejected_rather_than_defaulted() {
        let err = extract_json::<Review>(r#"{"findings":[]}"#).unwrap_err();
        assert!(err.to_string().contains("no JSON object"), "{err}");
    }

    #[test]
    fn review_vote_worst_is_the_most_cautious() {
        assert_eq!(
            ReviewVote::worst([ReviewVote::Approve, ReviewVote::Reject, ReviewVote::Approve]),
            Some(ReviewVote::Reject)
        );
        assert_eq!(
            ReviewVote::worst([ReviewVote::Approve, ReviewVote::ApproveWithFindings]),
            Some(ReviewVote::ApproveWithFindings)
        );
        assert_eq!(ReviewVote::worst(Vec::<ReviewVote>::new()), None);
    }

    #[test]
    fn review_revote_parses_the_reconsideration_shape() {
        let r: ReviewRevote =
            extract_json(r#"{"vote":"approve","reason":"the other findings do not hold"}"#)
                .unwrap();
        assert_eq!(r.vote, ReviewVote::Approve);
        assert_eq!(r.reason, "the other findings do not hold");
    }

    #[test]
    fn fix_report_parses_rejections() {
        let f: FixReport = extract_json(
            r#"{"addressed":["R1-1-1"],"rejected":[{"id":"R1-2-1","why":"not reachable"}]}"#,
        )
        .unwrap();
        assert_eq!(f.addressed, ["R1-1-1"]);
        assert_eq!(f.rejected[0].id, "R1-2-1");
    }

    #[test]
    fn proposal_parses_with_optional_fields_missing() {
        let p: Proposal = extract_json(
            r#"{"approach":"do X","key_tradeoff":"simpler now, slower later","why_not_naive":"the naive version corrupts state under a retry"}"#,
        )
        .unwrap();
        assert!(p.risks.is_empty());
        assert!(p.touches.is_empty());
        p.validate()
            .expect("a proposal with only required fields is valid");
    }

    #[test]
    fn proposal_validation_rejects_an_empty_required_field() {
        let p = Proposal {
            approach: String::new(),
            key_tradeoff: "t".to_owned(),
            risks: Vec::new(),
            touches: Vec::new(),
            why_not_naive: "w".to_owned(),
        };
        let err = p.validate().unwrap_err();
        assert!(err.to_string().contains("approach"));
    }

    #[test]
    fn sections_are_sliced_by_heading() {
        let text = "## SUMMARY\nchanged the retry loop.\nadded a test.\n\n## NOTES\nignore me\n";
        assert_eq!(
            section(text, "summary").unwrap(),
            "changed the retry loop.\nadded a test."
        );
        assert_eq!(section(text, "notes").unwrap(), "ignore me");
        assert!(section(text, "missing").is_none());
    }

    #[test]
    fn no_change_needed_marker_yields_its_evidence() {
        let summary = "NO CHANGE NEEDED: already fixed by b32cfc4, which is on main \
                        (git merge-base --is-ancestor confirms it).";
        assert_eq!(
            verified_noop(summary).unwrap(),
            "already fixed by b32cfc4, which is on main (git merge-base --is-ancestor \
             confirms it)."
        );
    }

    #[test]
    fn no_change_needed_marker_with_no_evidence_is_not_verified() {
        assert!(verified_noop("NO CHANGE NEEDED:").is_none());
        assert!(verified_noop("NO CHANGE NEEDED:   \n  ").is_none());
    }

    #[test]
    fn an_ordinary_summary_is_not_verified() {
        assert!(verified_noop("- changed the retry loop.\n- added a test.").is_none());
        // The marker only counts leading the section, not mentioned in passing.
        assert!(verified_noop("I considered NO CHANGE NEEDED: but wrote a fix instead.").is_none());
    }
}