Skip to main content

spar/
model.rs

1//! The shapes that cross the boundary between spar and a model, and the shapes
2//! spar keeps for itself.
3//!
4//! Everything a model produces is parsed leniently: an LLM that answers
5//! "medium" where the schema said "med" is not a reason to abandon a run that
6//! has already spent real money. Anything genuinely unrecognisable is still an
7//! error, because silently downgrading a blocking finding is worse than
8//! stopping.
9
10use std::collections::BTreeMap;
11use std::fmt;
12
13use serde::de::{self, Deserializer, Visitor};
14use serde::{Deserialize, Serialize, Serializer};
15
16fn norm_token(text: &str) -> String {
17    text.trim()
18        .to_lowercase()
19        .chars()
20        .filter(|c| c.is_ascii_alphanumeric())
21        .collect()
22}
23
24macro_rules! string_enum {
25    (
26        $(#[$meta:meta])*
27        pub enum $name:ident {
28            $( $(#[$vmeta:meta])* $variant:ident = $canonical:literal $( | $alias:literal )* ),+ $(,)?
29        }
30    ) => {
31        $(#[$meta])*
32        #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
33        pub enum $name { $( $(#[$vmeta])* $variant ),+ }
34
35        impl $name {
36            pub fn as_str(self) -> &'static str {
37                match self { $( $name::$variant => $canonical ),+ }
38            }
39
40            /// Accept the canonical spelling, any listed alias, and any
41            /// difference in case, spacing, hyphens, or underscores.
42            pub fn parse_lenient(text: &str) -> Option<Self> {
43                let got = norm_token(text);
44                $(
45                    if got == norm_token($canonical) $( || got == norm_token($alias) )* {
46                        return Some($name::$variant);
47                    }
48                )+
49                None
50            }
51
52            pub fn valid_values() -> String {
53                [$( $canonical ),+].join(", ")
54            }
55        }
56
57        impl fmt::Display for $name {
58            fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
59                f.write_str(self.as_str())
60            }
61        }
62
63        impl Serialize for $name {
64            fn serialize<S: Serializer>(&self, s: S) -> Result<S::Ok, S::Error> {
65                s.serialize_str(self.as_str())
66            }
67        }
68
69        impl<'de> Deserialize<'de> for $name {
70            fn deserialize<D: Deserializer<'de>>(d: D) -> Result<Self, D::Error> {
71                let raw = String::deserialize(d)?;
72                $name::parse_lenient(&raw).ok_or_else(|| {
73                    de::Error::custom(format!(
74                        "{} is not one of: {}",
75                        raw,
76                        $name::valid_values()
77                    ))
78                })
79            }
80        }
81    };
82}
83
84string_enum! {
85    /// How much work an issue is. Drives ordering: cheap unblocking work first.
86    pub enum Complexity {
87        S = "s" | "small" | "sm" | "xs" | "trivial",
88        M = "m" | "medium" | "med" | "moderate",
89        L = "l" | "large" | "lg" | "xl" | "big" | "huge",
90    }
91}
92
93string_enum! {
94    /// How likely a change here is to break something.
95    pub enum Risk {
96        Low = "low" | "l" | "minimal" | "none",
97        Med = "med" | "medium" | "m" | "moderate",
98        High = "high" | "h" | "severe" | "critical",
99    }
100}
101
102string_enum! {
103    /// Only `Blocking` gates a merge. This is the whole defence against the
104    /// nitpick spiral: a competent reviewer can always find something, so
105    /// "no objections remaining" is not a stopping condition but "no blocking
106    /// objections" is.
107    pub enum Severity {
108        Blocking = "blocking" | "block" | "major" | "critical",
109        NonBlocking = "non-blocking" | "nonblocking" | "non_blocking" | "minor" | "suggestion",
110        Nit = "nit" | "nitpick" | "style" | "trivial",
111    }
112}
113
114string_enum! {
115    pub enum Verdict {
116        Approve = "approve" | "approved" | "lgtm",
117        ChangesRequested = "changes_requested" | "changes-requested" | "request_changes" | "reject",
118    }
119}
120
121string_enum! {
122    /// What a formal pull request review says. `Verdict` is what a model
123    /// answers; this is what spar submits, and spar never asks a model for it.
124    /// It is derived from the findings: approve when nothing blocks, request
125    /// changes when something both reviewers count does.
126    pub enum ReviewVerdict {
127        Approve = "approve" | "approved",
128        RequestChanges = "request_changes" | "changes_requested" | "request-changes",
129    }
130}
131
132impl ReviewVerdict {
133    /// The `event` the reviews API submits it as.
134    pub fn api_event(self) -> &'static str {
135        match self {
136            ReviewVerdict::Approve => "APPROVE",
137            ReviewVerdict::RequestChanges => "REQUEST_CHANGES",
138        }
139    }
140
141    /// The words that stand in for the badge when GitHub will not take the
142    /// verdict itself, on the viewer's own pull request.
143    pub fn words(self) -> &'static str {
144        match self {
145            ReviewVerdict::Approve => "approve",
146            ReviewVerdict::RequestChanges => "changes requested",
147        }
148    }
149}
150
151string_enum! {
152    pub enum NextAction {
153        Merge = "merge" | "approve" | "ship",
154        FixMyself = "fix_myself" | "fix-myself" | "fix" | "self_fix",
155        HandBack = "hand_back" | "hand-back" | "handback" | "return",
156    }
157}
158
159string_enum! {
160    /// A reviewer's point gets exactly one of these. Refutation is a first
161    /// class outcome, not friction: an agent that accepts every comment to get
162    /// approved produces worse code, not better.
163    pub enum Action {
164        Fixed = "fixed" | "fix" | "accepted" | "done",
165        Refuted = "refuted" | "refute" | "rejected" | "disagree" | "wontfix",
166        FiledIssue = "filed_issue" | "filed-issue" | "filed" | "deferred" | "out_of_scope",
167    }
168}
169
170string_enum! {
171    /// What spar should do about one comment somebody left.
172    ///
173    /// Every value maps to exactly one action, which is the point: a verdict
174    /// that needs a second field to decide what it means is one that gets
175    /// decided differently in two places.
176    pub enum Ask {
177        /// A change is asked for, it is right, and it belongs on this branch.
178        /// Make it, push it, say so, and resolve the thread.
179        Implement = "implement" | "do" | "accept" | "fix",
180        /// Right, but really its own piece of work. File it, say where it went.
181        Defer = "defer" | "file_issue" | "filed_issue" | "out_of_scope" | "followup",
182        /// Should not be made. Reply with the reason, leave the thread open.
183        Decline = "decline" | "refute" | "reject" | "disagree" | "wontfix",
184        /// A question rather than a request. Answer it in words.
185        Answer = "answer" | "question" | "reply" | "clarify",
186        /// Nothing is being asked. Praise, or a thread they settled themselves.
187        Nothing = "nothing" | "none" | "no_request" | "noop" | "skip",
188    }
189}
190
191string_enum! {
192    /// What one screening pass decided about one recorded follow-up.
193    ///
194    /// Only `StillRelevant` files anything. The other three all take the entry
195    /// out of the queue, which is why the prompt asks for a reason and the log
196    /// prints it: they are the verdicts nobody sees the working for.
197    pub enum Screened {
198        StillRelevant = "still_relevant" | "still-relevant" | "relevant" | "keep" | "file",
199        AlreadyFixed = "already_fixed" | "already-fixed" | "fixed" | "done" | "resolved",
200        NotWorthIt = "not_worth_it" | "not-worth-it" | "not_worth_doing" | "skip" | "drop" | "wontfix",
201        Duplicate = "duplicate" | "dupe" | "dup",
202    }
203}
204
205string_enum! {
206    /// Terminal state of one issue or one resumed PR.
207    pub enum Status {
208        Pending = "pending",
209        Abandoned = "abandoned",
210        Approved = "approved",
211        Merged = "merged",
212        Escalated = "escalated",
213        Error = "error",
214        /// Review only: findings were produced and posted, nothing was changed.
215        Reviewed = "reviewed",
216        /// Review only: both reviewers found nothing that blocks a merge.
217        Clean = "clean",
218        /// Check-in: comments were read and answered.
219        Answered = "answered",
220        /// Split: the item was broken into parts, which were filed or opened.
221        Split = "split",
222        /// Split: read, and left as one piece.
223        Whole = "whole",
224        /// Review only: the head moved while it was read, or GitHub had not
225        /// caught up with a push yet, so nothing was posted. It runs again on
226        /// the new head.
227        Moved = "moved",
228    }
229}
230
231impl Complexity {
232    pub fn rank(self) -> u8 {
233        match self {
234            Complexity::S => 0,
235            Complexity::M => 1,
236            Complexity::L => 2,
237        }
238    }
239}
240
241impl Severity {
242    /// How badly it matters, independent of the order the variants happen to
243    /// be declared in. Relying on derived `Ord` here would silently invert the
244    /// moment somebody reorders the enum.
245    pub fn rank(self) -> u8 {
246        match self {
247            Severity::Nit => 0,
248            Severity::NonBlocking => 1,
249            Severity::Blocking => 2,
250        }
251    }
252
253    /// The graver of two judgements.
254    ///
255    /// Two reviewers disagreeing about severity is resolved upward on purpose:
256    /// advice that under-reports a real defect is worse than advice that
257    /// over-reports a small one.
258    pub fn graver(self, other: Self) -> Self {
259        if self.rank() >= other.rank() {
260            self
261        } else {
262            other
263        }
264    }
265}
266
267impl Risk {
268    pub fn rank(self) -> u8 {
269        match self {
270            Risk::Low => 0,
271            Risk::Med => 1,
272            Risk::High => 2,
273        }
274    }
275}
276
277// ---------------------------------------------------------------------------
278// Lenient scalar helpers
279// ---------------------------------------------------------------------------
280
281/// An integer that may arrive as a number, a float, or a quoted string, with or
282/// without a leading `#`.
283pub fn de_i64<'de, D: Deserializer<'de>>(d: D) -> Result<i64, D::Error> {
284    struct V;
285    impl<'de> Visitor<'de> for V {
286        type Value = i64;
287        fn expecting(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
288            f.write_str("an issue number")
289        }
290        fn visit_i64<E: de::Error>(self, v: i64) -> Result<i64, E> {
291            Ok(v)
292        }
293        fn visit_u64<E: de::Error>(self, v: u64) -> Result<i64, E> {
294            Ok(v as i64)
295        }
296        fn visit_f64<E: de::Error>(self, v: f64) -> Result<i64, E> {
297            Ok(v as i64)
298        }
299        fn visit_str<E: de::Error>(self, v: &str) -> Result<i64, E> {
300            v.trim()
301                .trim_start_matches('#')
302                .parse()
303                .map_err(|_| E::custom(format!("{v} is not a number")))
304        }
305    }
306    d.deserialize_any(V)
307}
308
309fn de_i64_vec<'de, D: Deserializer<'de>>(d: D) -> Result<Vec<i64>, D::Error> {
310    #[derive(Deserialize)]
311    struct One(#[serde(deserialize_with = "de_i64")] i64);
312    let raw = Option::<Vec<One>>::deserialize(d)?;
313    Ok(raw
314        .unwrap_or_default()
315        .into_iter()
316        .map(|One(n)| n)
317        .collect())
318}
319
320/// A boolean that may arrive as `true`, `"true"`, `"yes"`, or `1`.
321pub fn de_bool<'de, D: Deserializer<'de>>(d: D) -> Result<bool, D::Error> {
322    struct V;
323    impl<'de> Visitor<'de> for V {
324        type Value = bool;
325        fn expecting(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
326            f.write_str("a boolean")
327        }
328        fn visit_bool<E: de::Error>(self, v: bool) -> Result<bool, E> {
329            Ok(v)
330        }
331        fn visit_i64<E: de::Error>(self, v: i64) -> Result<bool, E> {
332            Ok(v != 0)
333        }
334        fn visit_u64<E: de::Error>(self, v: u64) -> Result<bool, E> {
335            Ok(v != 0)
336        }
337        fn visit_str<E: de::Error>(self, v: &str) -> Result<bool, E> {
338            match norm_token(v).as_str() {
339                "true" | "yes" | "y" | "1" => Ok(true),
340                "false" | "no" | "n" | "0" => Ok(false),
341                other => Err(E::custom(format!("{other} is not a boolean"))),
342            }
343        }
344    }
345    d.deserialize_any(V)
346}
347
348/// An optional number that may arrive as 412, "412", "#412", null, or "none".
349///
350/// Anything unparseable yields None rather than failing. A duplicate pointer is
351/// decoration on a verdict that stands without it, and taking a whole batch down
352/// over one stray string costs a second full repo pass to learn nothing.
353pub fn de_opt_i64<'de, D: Deserializer<'de>>(d: D) -> Result<Option<i64>, D::Error> {
354    Ok(match Option::<serde_json::Value>::deserialize(d)? {
355        Some(serde_json::Value::Number(n)) => n.as_i64(),
356        Some(serde_json::Value::String(s)) => s.trim().trim_start_matches('#').parse().ok(),
357        _ => None,
358    })
359}
360
361/// A list of strings that may arrive as null or be missing entirely.
362fn de_string_vec<'de, D: Deserializer<'de>>(d: D) -> Result<Vec<String>, D::Error> {
363    Ok(Option::<Vec<String>>::deserialize(d)?.unwrap_or_default())
364}
365
366fn de_bool_default_true<'de, D: Deserializer<'de>>(d: D) -> Result<bool, D::Error> {
367    #[derive(Deserialize)]
368    struct Wrap(#[serde(deserialize_with = "de_bool")] bool);
369    Ok(Option::<Wrap>::deserialize(d)?
370        .map(|Wrap(b)| b)
371        .unwrap_or(true))
372}
373
374fn de_string<'de, D: Deserializer<'de>>(d: D) -> Result<String, D::Error> {
375    Ok(Option::<String>::deserialize(d)?.unwrap_or_default())
376}
377
378// ---------------------------------------------------------------------------
379// What the models return
380// ---------------------------------------------------------------------------
381
382#[derive(Debug, Clone, Serialize, Deserialize)]
383pub struct TriageVerdict {
384    #[serde(deserialize_with = "de_i64")]
385    pub issue: i64,
386    #[serde(deserialize_with = "de_bool")]
387    pub worth_doing: bool,
388    /// The issue holds context for work filed elsewhere. Never opened, and
389    /// never closed either.
390    #[serde(default, deserialize_with = "de_bool")]
391    pub tracker: bool,
392    #[serde(default, deserialize_with = "de_string")]
393    pub reason: String,
394    pub complexity: Complexity,
395    #[serde(default, deserialize_with = "de_i64_vec")]
396    pub depends_on: Vec<i64>,
397    pub risk: Risk,
398}
399
400#[derive(Debug, Clone, Serialize, Deserialize)]
401pub struct TriageResponse {
402    #[serde(default)]
403    pub issues: Vec<TriageVerdict>,
404}
405
406/// One agent's ruling on one entry in the local follow-up queue.
407#[derive(Debug, Clone, Serialize, Deserialize)]
408pub struct ScreenVerdict {
409    /// The entry's position in the list it was given, from 1.
410    ///
411    /// Matched back by index rather than by title, because the entries are
412    /// spar's own data and so can carry a handle a model cannot paraphrase.
413    #[serde(deserialize_with = "de_i64")]
414    pub entry: i64,
415    pub verdict: Screened,
416    #[serde(default, deserialize_with = "de_string")]
417    pub title: String,
418    #[serde(default, deserialize_with = "de_string")]
419    pub reason: String,
420    /// The open issue a duplicate points at.
421    ///
422    /// Kept apart from `duplicate_of_entry`, because one number cannot say
423    /// which it is and the two are acted on differently: an issue already
424    /// exists, while a sibling entry may itself be dropped or fail to file.
425    #[serde(default, deserialize_with = "de_opt_i64")]
426    pub duplicate_of_issue: Option<i64>,
427    /// The number of another entry in the same list a duplicate points at.
428    #[serde(default, deserialize_with = "de_opt_i64")]
429    pub duplicate_of_entry: Option<i64>,
430}
431
432impl ScreenVerdict {
433    /// Whether this verdict names anything at all as the thing that covers it.
434    pub fn names_a_duplicate(&self) -> bool {
435        self.duplicate_of_issue.is_some() || self.duplicate_of_entry.is_some()
436    }
437}
438
439#[derive(Debug, Clone, Serialize, Deserialize)]
440pub struct ScreenResponse {
441    #[serde(default)]
442    pub entries: Vec<ScreenVerdict>,
443}
444
445/// One agent's ruling on whether one open item is worth splitting at all.
446///
447/// The cheap screen the bare `spar split` runs over a whole queue, so that two
448/// agent calls per item are only spent on the ones where the answer might be
449/// yes.
450#[derive(Debug, Clone, Serialize, Deserialize)]
451pub struct SplitScreen {
452    /// The issue or pull request number, which is spar's own data and so cannot
453    /// be paraphrased back wrong.
454    #[serde(deserialize_with = "de_i64")]
455    pub item: i64,
456    #[serde(deserialize_with = "de_bool")]
457    pub split: bool,
458    #[serde(default, deserialize_with = "de_string")]
459    pub reason: String,
460}
461
462#[derive(Debug, Clone, Serialize, Deserialize)]
463pub struct SplitScreenDoc {
464    #[serde(default)]
465    pub items: Vec<SplitScreen>,
466}
467
468/// One piece of a proposed decomposition.
469#[derive(Debug, Clone, Default, Serialize, Deserialize)]
470pub struct SplitPart {
471    #[serde(default, deserialize_with = "de_string")]
472    pub title: String,
473    /// The issue body, or the rationale for a pull request slice.
474    #[serde(default, deserialize_with = "de_string")]
475    pub body: String,
476    /// The paths this slice carries. Empty for an issue split, where there is
477    /// no diff to partition.
478    #[serde(default, deserialize_with = "de_string_vec")]
479    pub files: Vec<String>,
480}
481
482/// One agent's decomposition of an issue or a pull request.
483///
484/// Deliberately one proposal rather than two. Two agents asked to decompose the
485/// same thing return two decompositions that cannot be reconciled mechanically,
486/// and reconciling them is a third judgement nobody asked for.
487#[derive(Debug, Clone, Default, Serialize, Deserialize)]
488pub struct SplitProposal {
489    #[serde(default, deserialize_with = "de_bool")]
490    pub should_split: bool,
491    #[serde(default, deserialize_with = "de_string")]
492    pub reason: String,
493    /// Whether the parts are sequential, so each is based on its predecessor
494    /// rather than on the base branch. A property of the change, not of the
495    /// repository, which is why it rides on the proposal.
496    #[serde(default, deserialize_with = "de_bool")]
497    pub stacked: bool,
498    #[serde(default)]
499    pub parts: Vec<SplitPart>,
500}
501
502/// The second agent's ruling on the first one's decomposition.
503#[derive(Debug, Clone, Default, Serialize, Deserialize)]
504pub struct SplitCheck {
505    #[serde(default, deserialize_with = "de_bool")]
506    pub accept: bool,
507    #[serde(default, deserialize_with = "de_bool")]
508    pub stacked: bool,
509    /// Part numbers, from 1, that should not be split out.
510    #[serde(default, deserialize_with = "de_i64_vec")]
511    pub strike: Vec<i64>,
512    #[serde(default, deserialize_with = "de_string")]
513    pub reasoning: String,
514}
515
516/// One agent's judgement of one comment somebody left on a pull request.
517#[derive(Debug, Clone, Serialize, Deserialize)]
518pub struct CommentVerdict {
519    /// The handle spar printed beside the comment, copied back so the answer
520    /// can be matched to it. Not a title: a finding's title is the only handle
521    /// there is because findings are model authored, but a comment is spar's
522    /// own data, so it gets one that cannot be paraphrased or collided.
523    #[serde(default, deserialize_with = "de_string")]
524    pub ref_id: String,
525    pub ask: Ask,
526    /// What is being asked for, in one sentence, in the agent's own words.
527    /// How spar checks the comment was understood before acting on it.
528    #[serde(default, deserialize_with = "de_string")]
529    pub request: String,
530    /// The whole argument. For a decline this is posted in the thread, so it is
531    /// written for the person who raised the point.
532    #[serde(default, deserialize_with = "de_string")]
533    pub reasoning: String,
534    /// False when the comment could be read more than one way. spar answers an
535    /// ambiguous comment in words and never guesses at a commit.
536    #[serde(deserialize_with = "de_bool")]
537    pub unambiguous: bool,
538    #[serde(default)]
539    pub new_issue_title: Option<String>,
540    #[serde(default)]
541    pub new_issue_body: Option<String>,
542}
543
544#[derive(Debug, Clone, Serialize, Deserialize)]
545pub struct CheckinDoc {
546    #[serde(default)]
547    pub verdicts: Vec<CommentVerdict>,
548}
549
550/// The second agent's ruling on the first one's call.
551#[derive(Debug, Clone, Serialize, Deserialize)]
552pub struct CommentCheck {
553    #[serde(default, deserialize_with = "de_string")]
554    pub ref_id: String,
555    #[serde(deserialize_with = "de_bool")]
556    pub agrees: bool,
557    /// What this agent would do. It has to say implement alongside
558    /// `agrees` before anything is implemented: the two coming apart is the
559    /// model contradicting itself, and that resolves toward saying rather than
560    /// doing.
561    pub ask: Ask,
562    #[serde(deserialize_with = "de_bool")]
563    pub unambiguous: bool,
564    #[serde(default, deserialize_with = "de_string")]
565    pub reasoning: String,
566}
567
568#[derive(Debug, Clone, Serialize, Deserialize)]
569pub struct CheckDoc {
570    #[serde(default)]
571    pub checks: Vec<CommentCheck>,
572}
573
574/// What the fix pass did about one comment.
575#[derive(Debug, Clone, Serialize, Deserialize)]
576pub struct FixOutcome {
577    #[serde(default, deserialize_with = "de_string")]
578    pub ref_id: String,
579    /// False when the change turned out to be wrong once the code was open.
580    /// Declining here is a better answer than making a change you now believe
581    /// is a mistake.
582    #[serde(deserialize_with = "de_bool")]
583    pub changed: bool,
584    /// One sentence naming what changed, or why it was left alone. Posted in
585    /// the thread, so it is written for the person who asked.
586    #[serde(default, deserialize_with = "de_string")]
587    pub summary: String,
588    /// The paths this comment's change touched, checked against the diff so a
589    /// reply never claims a fix that is not in it.
590    #[serde(default)]
591    pub files: Vec<String>,
592}
593
594#[derive(Debug, Clone, Serialize, Deserialize)]
595pub struct FixReport {
596    #[serde(default)]
597    pub done: Vec<FixOutcome>,
598}
599
600/// What spar has already answered on one pull request or issue.
601///
602/// Keyed by thread or comment, valued by the newest message in it that spar did
603/// not write. A thread that has moved since spar answered carries a different
604/// value and is read again, which is what makes "they replied to my reply"
605/// work. GitHub's own resolved flag covers the threads spar fixed; this covers
606/// the ones it argued with, which stay open on purpose and would otherwise be
607/// re-argued once per run, forever.
608///
609/// Written only after a reply has posted. A run that could not post is a run
610/// that has not answered, and recording it as answered would lose the comment.
611#[derive(Debug, Clone, Default, Serialize, Deserialize)]
612pub struct Answered {
613    #[serde(default)]
614    pub version: u32,
615    #[serde(default)]
616    pub seen: BTreeMap<String, String>,
617}
618
619#[derive(Debug, Clone, Serialize, Deserialize)]
620pub struct Finding {
621    pub severity: Severity,
622    #[serde(default, deserialize_with = "de_string")]
623    pub title: String,
624    #[serde(default, deserialize_with = "de_string")]
625    pub detail: String,
626    #[serde(default, deserialize_with = "de_string")]
627    pub file: String,
628    /// A real problem that this PR did not cause. Those become follow-ups
629    /// rather than review comments, so they cannot gate an unrelated merge.
630    #[serde(default = "yes", deserialize_with = "de_bool_default_true")]
631    pub in_scope: bool,
632
633    // -- the parts of a bug report ---------------------------------------
634    //
635    // Filled when a finding is going to become an issue somebody picks up
636    // cold. `detail` is the one line the pull request thread shows; these are
637    // what a person needs when the thread is not in front of them. All
638    // optional: a finding that stays in the thread has no use for them.
639    /// What is wrong, with the specifics.
640    #[serde(default)]
641    pub problem: Option<String>,
642    /// Steps to reproduce it, and what actually happens.
643    #[serde(default)]
644    pub reproduction: Option<String>,
645    /// What it costs somebody.
646    #[serde(default)]
647    pub impact: Option<String>,
648    /// What it should do instead.
649    #[serde(default)]
650    pub expected: Option<String>,
651}
652
653impl Default for Finding {
654    /// A blank finding, for building one field at a time.
655    ///
656    /// Severity is spelled out here rather than derived, because a severity
657    /// arriving by default is exactly the mistake this codebase refuses
658    /// elsewhere: it is the field that decides whether a merge is gated, and
659    /// the least severe value is the only safe thing to assume.
660    fn default() -> Self {
661        Self {
662            severity: Severity::Nit,
663            title: String::new(),
664            detail: String::new(),
665            file: String::new(),
666            in_scope: true,
667            problem: None,
668            reproduction: None,
669            impact: None,
670            expected: None,
671        }
672    }
673}
674
675impl Finding {
676    /// The parts of a bug report this finding carries, in the order they are
677    /// written, skipping the ones it does not.
678    pub fn report_sections(&self) -> Vec<(&'static str, &str)> {
679        [
680            ("Problem", self.problem.as_deref()),
681            ("Reproduction", self.reproduction.as_deref()),
682            ("Impact", self.impact.as_deref()),
683            ("Expected behavior", self.expected.as_deref()),
684        ]
685        .into_iter()
686        .filter_map(|(heading, text)| {
687            text.map(str::trim)
688                .filter(|t| !t.is_empty())
689                .map(|t| (heading, t))
690        })
691        .collect()
692    }
693}
694
695fn yes() -> bool {
696    true
697}
698
699impl Finding {
700    pub fn blocks(&self) -> bool {
701        self.severity == Severity::Blocking && self.in_scope
702    }
703
704    pub fn where_at(&self) -> &str {
705        if self.file.trim().is_empty() {
706            "general"
707        } else {
708            self.file.trim()
709        }
710    }
711}
712
713#[derive(Debug, Clone, Serialize, Deserialize)]
714pub struct Review {
715    pub verdict: Verdict,
716    pub next_action: NextAction,
717    #[serde(default, deserialize_with = "de_string")]
718    pub summary: String,
719    #[serde(default)]
720    pub findings: Vec<Finding>,
721}
722
723#[derive(Debug, Clone, Serialize, Deserialize)]
724pub struct Disposition {
725    #[serde(default, deserialize_with = "de_string")]
726    pub title: String,
727    /// Carried so a refutation lands on the same ledger key the reviewer's
728    /// finding will hash to next round. Without it the re-litigation guard
729    /// silently never fires for any finding that names a file.
730    #[serde(default, deserialize_with = "de_string")]
731    pub file: String,
732    pub action: Action,
733    #[serde(default, deserialize_with = "de_string")]
734    pub reasoning: String,
735    #[serde(default)]
736    pub new_issue_title: Option<String>,
737    #[serde(default)]
738    pub new_issue_body: Option<String>,
739}
740
741/// What a reviewer did about one of its own findings when it chose to fix them.
742///
743/// The other three editing calls return a structured report. This one returned
744/// free text, and three things were inferred from it: that every finding was
745/// fixed, what the commit subject should be, and that the author had nothing to
746/// answer. A reviewer that fixed two of three points and said so in prose had
747/// all three recorded as fixed.
748#[derive(Debug, Clone, Serialize, Deserialize)]
749pub struct OwnFix {
750    #[serde(default, deserialize_with = "de_string")]
751    pub title: String,
752    /// Carried for the same reason a disposition carries it: the ledger key has
753    /// to be the one the finding hashes to.
754    #[serde(default, deserialize_with = "de_string")]
755    pub file: String,
756    #[serde(deserialize_with = "de_bool")]
757    pub fixed: bool,
758    #[serde(default, deserialize_with = "de_string")]
759    pub reason: String,
760}
761
762#[derive(Debug, Clone, Serialize, Deserialize)]
763pub struct OwnFixDoc {
764    #[serde(default, deserialize_with = "de_string")]
765    pub summary: String,
766    #[serde(default)]
767    pub fixes: Vec<OwnFix>,
768}
769
770/// One reviewer's judgement of a finding the *other* reviewer raised.
771///
772/// This is the whole point of review only mode. A finding both models raise
773/// independently is worth a maintainer's attention; a finding one raised and
774/// the other examined and rejected is usually not, and saying so is more useful
775/// than forwarding both.
776#[derive(Debug, Clone, Serialize, Deserialize)]
777pub struct Adjudication {
778    #[serde(default, deserialize_with = "de_string")]
779    pub title: String,
780    #[serde(default, deserialize_with = "de_string")]
781    pub file: String,
782    /// Whether the defect is real, judged by reading the code rather than by
783    /// deferring to the other reviewer.
784    #[serde(deserialize_with = "de_bool")]
785    pub agrees: bool,
786    /// This reviewer's own view of how badly it matters.
787    pub severity: Severity,
788    #[serde(default, deserialize_with = "de_string")]
789    pub reasoning: String,
790}
791
792#[derive(Debug, Clone, Serialize, Deserialize)]
793pub struct AdjudicationDoc {
794    #[serde(default)]
795    pub verdicts: Vec<Adjudication>,
796}
797
798string_enum! {
799    /// What a second look found about one earlier finding.
800    pub enum Ruling {
801        /// The code now answers the point.
802        Fixed = "fixed" | "resolved" | "addressed" | "done",
803        /// The code did not change for it, and a response showed the point
804        /// was wrong or does not apply.
805        Rebutted = "rebutted" | "refuted" | "answered" | "withdrawn" | "not_a_bug",
806        /// Neither. The point stands, or the change made for it does not
807        /// settle it.
808        Open = "open" | "still_open" | "unresolved" | "stands" | "unfixed",
809    }
810}
811
812/// One reviewer's ruling on one earlier finding.
813#[derive(Debug, Clone, Serialize, Deserialize)]
814pub struct RecheckRuling {
815    /// The 1-based number the prompt gave the earlier finding. A number rather
816    /// than a title, for the reason the follow-up screen uses one: a model
817    /// rewrites titles, and a number it copies.
818    #[serde(deserialize_with = "de_i64")]
819    pub index: i64,
820    pub outcome: Ruling,
821    #[serde(default, deserialize_with = "de_string")]
822    pub reasoning: String,
823}
824
825/// What one reviewer answers on a second look: a ruling per earlier finding,
826/// and ordinary findings for anything new.
827#[derive(Debug, Clone, Default, Serialize, Deserialize)]
828pub struct RecheckDoc {
829    #[serde(default, deserialize_with = "de_string")]
830    pub summary: String,
831    #[serde(default)]
832    pub rulings: Vec<RecheckRuling>,
833    #[serde(default)]
834    pub findings: Vec<Finding>,
835}
836
837// ---------------------------------------------------------------------------
838// Brainstorming
839// ---------------------------------------------------------------------------
840
841/// One idea from a brainstorm: existing pieces, paired in a way that has not
842/// been done.
843///
844/// `combines` is the load bearing field. The premise of `spar brainstorm` is
845/// that the parts of an answer already exist and have not been put together, so
846/// an idea that cannot name its parts is a wish rather than a proposal, and
847/// `why_new` has to say what the nearest existing thing is rather than assert
848/// novelty.
849#[derive(Debug, Clone, Default, Serialize, Deserialize)]
850pub struct Idea {
851    #[serde(default, deserialize_with = "de_string")]
852    pub title: String,
853    /// The existing techniques, primitives, results, or patterns this pairs.
854    #[serde(default, deserialize_with = "de_string_vec")]
855    pub combines: Vec<String>,
856    #[serde(default, deserialize_with = "de_string")]
857    pub how_it_works: String,
858    /// The nearest thing that already exists, and how this differs from it.
859    #[serde(default, deserialize_with = "de_string")]
860    pub why_new: String,
861    #[serde(default, deserialize_with = "de_string")]
862    pub enables: String,
863    #[serde(default, deserialize_with = "de_string")]
864    pub risks: String,
865    /// The cheapest thing that would show whether it works.
866    #[serde(default, deserialize_with = "de_string")]
867    pub first_experiment: String,
868}
869
870#[derive(Debug, Clone, Default, Serialize, Deserialize)]
871pub struct IdeasDoc {
872    #[serde(default)]
873    pub ideas: Vec<Idea>,
874}
875
876string_enum! {
877    /// What one agent does with an idea the other proposed.
878    pub enum CrossKind {
879        Build = "build" | "extend" | "improve" | "combine",
880        Challenge = "challenge" | "reject" | "exists" | "object",
881        Keep = "keep" | "agree" | "accept" | "fine",
882    }
883}
884
885/// One agent's ruling on an idea the *other* agent proposed.
886#[derive(Debug, Clone, Serialize, Deserialize)]
887pub struct CrossVerdict {
888    /// The number the idea was given in the list, so the ruling can be matched
889    /// back to it. Titles are not used for this: a model rewrites them.
890    #[serde(default, deserialize_with = "de_i64")]
891    pub idea: i64,
892    pub verdict: CrossKind,
893    #[serde(default, deserialize_with = "de_string")]
894    pub reason: String,
895    /// The revised idea, when the verdict is build.
896    #[serde(default)]
897    pub build: Option<Idea>,
898}
899
900#[derive(Debug, Clone, Default, Serialize, Deserialize)]
901pub struct CrossDoc {
902    #[serde(default)]
903    pub verdicts: Vec<CrossVerdict>,
904}
905
906/// What the agent that proposed a challenged idea says to the objection.
907#[derive(Debug, Clone, Serialize, Deserialize)]
908pub struct Defence {
909    #[serde(default, deserialize_with = "de_i64")]
910    pub idea: i64,
911    /// True to stand by it with specifics, false to withdraw it.
912    #[serde(deserialize_with = "de_bool")]
913    pub stands: bool,
914    #[serde(default, deserialize_with = "de_string")]
915    pub reply: String,
916}
917
918/// The last round's answer: defences for the agent's own challenged ideas, and
919/// its ranking of everything still standing.
920#[derive(Debug, Clone, Default, Serialize, Deserialize)]
921pub struct ConvergeDoc {
922    #[serde(default)]
923    pub defences: Vec<Defence>,
924    /// Idea numbers, best first.
925    #[serde(default, deserialize_with = "de_i64_vec")]
926    pub ranking: Vec<i64>,
927}
928
929/// A finding after both reviewers have had their say.
930#[derive(Debug, Clone)]
931pub struct Judged {
932    pub finding: Finding,
933    /// Who first raised it.
934    pub raised_by: String,
935    /// How it ended up.
936    pub standing: Standing,
937    /// The other reviewer's reasoning, when they had something to say.
938    pub counterpoint: Option<String>,
939    /// What the reviewer who raised it said when the objection came back.
940    /// Kept apart from the objection: running both together behind a single
941    /// "the other says" turns the most valuable content in the comment into
942    /// one unreadable sentence.
943    pub defence: Option<String>,
944    /// The other reviewer's own severity, when the cross-check had it agree
945    /// the defect is real. Not kept in the review record: the counterpoint
946    /// says it in words, and nothing after the cross-check reads it.
947    pub rated: Option<Severity>,
948}
949
950string_enum! {
951    /// How a finding stands after both reviewers have had their say. A word
952    /// rather than a bare enum because the review record keeps it on disk.
953    pub enum Standing {
954        /// Both reviewers raised it independently. The strongest signal there is.
955        Corroborated = "corroborated" | "both",
956        /// One raised it, the other read the code and agreed.
957        Confirmed = "confirmed",
958        /// One raised it, the other read the code and rejected it, and it survived
959        /// a rebuttal. Or the other agreed it is real but not on whether it
960        /// blocks. A person decides.
961        Disputed = "disputed",
962        /// Raised, rejected, and withdrawn by the reviewer who raised it.
963        Withdrawn = "withdrawn",
964        /// Raised with nobody left to check it, because the round budget ran out.
965        Unverified = "unverified" | "one_reviewer",
966    }
967}
968
969/// What the implementor did, and what the pull request body is built from.
970///
971/// Structured for the reason every other exchange here is structured: a model
972/// asked for a description writes a paragraph about having written one, while a
973/// model asked for a problem, a change list, and a way to check them answers
974/// each of those. The body's substance comes from the fields being asked for
975/// separately, and its brevity from spar composing them rather than the model
976/// narrating.
977#[derive(Debug, Clone, Default, Serialize, Deserialize)]
978pub struct Implementation {
979    /// The issue should not be implemented. No changes, and `reason` says why.
980    #[serde(default, deserialize_with = "de_bool")]
981    pub not_worth_doing: bool,
982    /// Only when declining. Posted on the issue, so it is written for whoever
983    /// opened it rather than for the harness.
984    #[serde(default, deserialize_with = "de_string")]
985    pub reason: String,
986    /// One sentence saying what changed. Leads the body.
987    #[serde(default, deserialize_with = "de_string")]
988    pub summary: String,
989    /// What was actually wrong, as understood after reading the code. Not a
990    /// restatement of the issue: the reviewer can follow the link.
991    #[serde(default, deserialize_with = "de_string")]
992    pub problem: String,
993    /// One line per change that alters behaviour.
994    #[serde(default)]
995    pub changes: Vec<String>,
996    /// How a reviewer confirms the change works.
997    #[serde(default)]
998    pub testing: Vec<String>,
999    /// Anything the reviewer would otherwise have to ask about: a deliberate
1000    /// omission, a decision worth defending, a risk. Usually nothing.
1001    #[serde(default)]
1002    pub notes: Option<String>,
1003}
1004
1005#[derive(Debug, Clone, Serialize, Deserialize)]
1006pub struct ResponseDoc {
1007    #[serde(default, deserialize_with = "de_string")]
1008    pub summary: String,
1009    #[serde(default)]
1010    pub dispositions: Vec<Disposition>,
1011}
1012
1013// ---------------------------------------------------------------------------
1014// What spar keeps
1015// ---------------------------------------------------------------------------
1016
1017#[derive(Debug, Clone, Serialize, Deserialize)]
1018pub struct PlanItem {
1019    pub issue: i64,
1020    pub title: String,
1021    pub complexity: Complexity,
1022    pub risk: Risk,
1023    pub depends_on: Vec<i64>,
1024    pub reason: String,
1025}
1026
1027#[derive(Debug, Clone, Serialize, Deserialize)]
1028pub struct SkippedItem {
1029    pub issue: i64,
1030    pub title: String,
1031    /// Keyed by agent name, so the plan file says who said what.
1032    pub reasons: BTreeMap<String, String>,
1033    /// An umbrella or epic, which spar comments on and leaves open.
1034    ///
1035    /// Set when *either* agent said so, unlike everything else here, which
1036    /// needs both. Closing already requires agreement, on the principle that
1037    /// one agent's opinion is not enough to close somebody's report; one agent
1038    /// saying the issue is not finished is the same principle from the other
1039    /// side.
1040    #[serde(default)]
1041    pub tracker: bool,
1042}
1043
1044#[derive(Debug, Clone, Serialize, Deserialize)]
1045pub struct ContestedItem {
1046    pub issue: i64,
1047    pub title: String,
1048    /// Agent name to "do" or "skip".
1049    pub positions: BTreeMap<String, String>,
1050    pub reasons: BTreeMap<String, String>,
1051    #[serde(default, skip_serializing_if = "Option::is_none")]
1052    pub note: Option<String>,
1053}
1054
1055/// An issue triage disagreed about, remembered between runs.
1056///
1057/// A disagreement is the one triage outcome that needs a person, and it was
1058/// also the one that reached them least reliably: a log line suppressed by
1059/// `--quiet`, an entry in `plan.json`, and two calls spent on it again on every
1060/// later run.
1061#[derive(Debug, Clone, Default, Serialize, Deserialize)]
1062pub struct Contested {
1063    #[serde(default)]
1064    pub title: String,
1065    /// Agent name to "do" or "skip".
1066    #[serde(default)]
1067    pub positions: BTreeMap<String, String>,
1068    #[serde(default)]
1069    pub reasons: BTreeMap<String, String>,
1070}
1071
1072#[derive(Debug, Clone, Default, Serialize, Deserialize)]
1073pub struct Plan {
1074    #[serde(default)]
1075    pub order: Vec<PlanItem>,
1076    #[serde(default)]
1077    pub skipped: Vec<SkippedItem>,
1078    #[serde(default)]
1079    pub contested: Vec<ContestedItem>,
1080}
1081
1082string_enum! {
1083    /// How a point stands, one round to the next.
1084    ///
1085    /// Three of these are endings: the code will not change for the point here,
1086    /// so raising it again only spends a round. They do not mean the same thing
1087    /// to a person, which is why `Dropped` is not folded into `Filed`: only
1088    /// `Filed` promises that somewhere holds the point.
1089    ///
1090    /// `Fixed` is not an ending. The code changed, the change is the author's
1091    /// claim about the point, and nothing has checked it. It is here because the
1092    /// ledger is the only thing a later round reads, and recording nothing for a
1093    /// fix left it holding only the points the reviewer lost. Six fix rounds
1094    /// across two pull requests produced no entry at all, and the guard that
1095    /// ends an argument had nothing to match.
1096    pub enum Settled {
1097        Refuted = "refuted" | "refute" | "rejected",
1098        Filed = "filed" | "filed_issue" | "filed-issue" | "out_of_scope",
1099        Dropped = "dropped" | "not_filed" | "not-filed" | "unfiled",
1100        Fixed = "fixed" | "fix" | "changed",
1101    }
1102}
1103
1104impl Default for Settled {
1105    /// State written before the ledger held anything but refutations.
1106    fn default() -> Self {
1107        Settled::Refuted
1108    }
1109}
1110
1111/// What one attempt to record a follow-up actually did.
1112///
1113/// The distinction the ledger needs. A point written down somewhere is settled
1114/// and tracked; a point deliberately not written down is settled and untracked;
1115/// a point that failed to be written down is not settled at all, and saying it
1116/// was loses it. A single `Option<String>` collapsed all three into "no URL".
1117#[derive(Debug, Clone, PartialEq, Eq)]
1118pub enum Followup {
1119    /// A tracker issue or a local note now carries it. The string is what a
1120    /// reader is shown: an issue URL, or a note line.
1121    Recorded(String),
1122    /// Something already covers it and nothing was written: a closed issue, or
1123    /// a note the local queue still holds. There is a reference to point at,
1124    /// but no new work to hand anyone.
1125    Covered(String),
1126    /// Deliberately not recorded, with the reason. Follow-ups are off, or the
1127    /// run has spent its cap.
1128    Dropped(&'static str),
1129    /// Nothing was written and nothing covers it. The point is still open, so
1130    /// the next round is free to try again.
1131    Failed,
1132}
1133
1134impl Followup {
1135    /// The reference to add to the run's filed list, when there is a live one.
1136    ///
1137    /// `Covered` is deliberately excluded: a closed issue reported as filed
1138    /// goes back into a wave to be implemented again.
1139    pub fn url(&self) -> Option<&str> {
1140        match self {
1141            Followup::Recorded(url) => Some(url),
1142            _ => None,
1143        }
1144    }
1145}
1146
1147#[derive(Debug, Clone, Serialize, Deserialize)]
1148pub struct LedgerEntry {
1149    pub title: String,
1150    pub file: String,
1151    pub reasoning: String,
1152    pub round: u32,
1153    #[serde(default)]
1154    pub reraised: u32,
1155    #[serde(default)]
1156    pub outcome: Settled,
1157}
1158
1159/// Settled points, keyed by the review loop's finding identity. Ordered so the settled block in a
1160/// prompt is stable between rounds, which keeps prompt caches warm and diffs
1161/// readable.
1162pub type Ledger = BTreeMap<String, LedgerEntry>;
1163
1164#[derive(Debug, Clone, Serialize, Deserialize)]
1165pub struct Dispute {
1166    pub title: String,
1167    #[serde(default, deserialize_with = "de_string")]
1168    pub file: String,
1169    pub reasoning: String,
1170}
1171
1172/// The outcome of working one issue, or resuming one PR.
1173#[derive(Debug, Clone, Serialize, Deserialize)]
1174pub struct IssueRun {
1175    pub issue: i64,
1176    pub title: String,
1177    pub status: Status,
1178    #[serde(default, skip_serializing_if = "Option::is_none")]
1179    pub pr: Option<String>,
1180    #[serde(default)]
1181    pub rounds: u32,
1182    #[serde(default)]
1183    pub disputes: Vec<Dispute>,
1184    #[serde(default)]
1185    pub filed: Vec<String>,
1186    /// Real points a reviewer judged smaller than another round.
1187    ///
1188    /// Nothing else carries them. The round comment is off under the default
1189    /// `pr_comments = "outcome"` and a non-blocking finding is not filed under
1190    /// the default `file_non_blocking = false`, so without this the severity
1191    /// ladder is a way to make a finding disappear rather than a way to stop it
1192    /// costing a round. Silence on a pull request should mean nothing was
1193    /// found, not that nothing was gated.
1194    #[serde(default)]
1195    pub noted: Vec<Finding>,
1196    #[serde(default)]
1197    pub notes: Vec<String>,
1198    /// Runtime circuit breaker after an external follow-up write could not be
1199    /// verified. A later invocation rechecks GitHub before any new write.
1200    #[serde(skip)]
1201    pub followup_writes_uncertain: bool,
1202}
1203
1204impl IssueRun {
1205    pub fn new(issue: i64, title: impl Into<String>) -> Self {
1206        Self {
1207            issue,
1208            title: title.into(),
1209            status: Status::Pending,
1210            pr: None,
1211            rounds: 0,
1212            disputes: Vec::new(),
1213            filed: Vec::new(),
1214            noted: Vec::new(),
1215            notes: Vec::new(),
1216            followup_writes_uncertain: false,
1217        }
1218    }
1219
1220    /// Whether this outcome counts as the run having done its job.
1221    ///
1222    /// A review that produced findings did its job: the findings are the
1223    /// product, and a PR needing work is not a failure of the reviewer.
1224    pub fn succeeded(&self) -> bool {
1225        matches!(
1226            self.status,
1227            Status::Merged
1228                | Status::Approved
1229                | Status::Abandoned
1230                | Status::Reviewed
1231                | Status::Clean
1232                | Status::Answered
1233                // A split that correctly decided nothing needed splitting did
1234                // its job. Without Whole here, `spar split` over a tidy queue
1235                // exits non-zero, which in a script reads as a failure.
1236                | Status::Split
1237                | Status::Whole
1238        )
1239    }
1240}
1241
1242/// Everything needed to pick a review back up, including on another machine.
1243#[derive(Debug, Clone, Serialize, Deserialize)]
1244pub struct PersistedState {
1245    pub version: u32,
1246    /// Monotonic checkpoint order for writes within this repository.
1247    #[serde(default)]
1248    pub checkpoint: u64,
1249    pub round: u32,
1250    pub next_actor: String,
1251    pub status: Status,
1252    /// Published pull request head to which this checkpoint applies.
1253    #[serde(default, skip_serializing_if = "String::is_empty")]
1254    pub pr_head: String,
1255    #[serde(default)]
1256    pub ledger: Ledger,
1257    #[serde(default)]
1258    pub filed: Vec<String>,
1259    #[serde(default, skip_serializing_if = "Vec::is_empty")]
1260    pub open_findings: Vec<Finding>,
1261    #[serde(default, skip_serializing_if = "Vec::is_empty")]
1262    pub disputes: Vec<Dispute>,
1263    #[serde(default, skip_serializing_if = "Vec::is_empty")]
1264    pub noted: Vec<Finding>,
1265}
1266
1267pub const STATE_VERSION: u32 = 2;
1268
1269/// One finding as the review record keeps it: enough to list it again, rule on
1270/// it, and post it, and nothing a model is not asked for. The parts of a bug
1271/// report a `Finding` may carry are for an issue filed from it, which a record
1272/// never does.
1273#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
1274pub struct RecordedFinding {
1275    pub title: String,
1276    #[serde(default)]
1277    pub file: String,
1278    pub severity: Severity,
1279    #[serde(default)]
1280    pub detail: String,
1281    #[serde(default = "yes")]
1282    pub in_scope: bool,
1283    #[serde(default)]
1284    pub raised_by: String,
1285    pub standing: Standing,
1286    #[serde(default, skip_serializing_if = "Option::is_none")]
1287    pub counterpoint: Option<String>,
1288    #[serde(default, skip_serializing_if = "Option::is_none")]
1289    pub defence: Option<String>,
1290}
1291
1292impl RecordedFinding {
1293    pub fn blocks(&self) -> bool {
1294        self.severity == Severity::Blocking && self.in_scope
1295    }
1296
1297    pub fn where_at(&self) -> &str {
1298        if self.file.trim().is_empty() {
1299            "general"
1300        } else {
1301            self.file.trim()
1302        }
1303    }
1304
1305    /// Back to the in-memory shape, for the renderer and the record that
1306    /// follows a second look.
1307    pub fn to_judged(&self) -> Judged {
1308        Judged {
1309            finding: Finding {
1310                severity: self.severity,
1311                title: self.title.clone(),
1312                detail: self.detail.clone(),
1313                file: self.file.clone(),
1314                in_scope: self.in_scope,
1315                ..Finding::default()
1316            },
1317            raised_by: self.raised_by.clone(),
1318            standing: self.standing,
1319            counterpoint: self.counterpoint.clone(),
1320            defence: self.defence.clone(),
1321            rated: None,
1322        }
1323    }
1324}
1325
1326impl From<&Judged> for RecordedFinding {
1327    fn from(j: &Judged) -> Self {
1328        Self {
1329            title: j.finding.title.clone(),
1330            file: j.finding.file.clone(),
1331            severity: j.finding.severity,
1332            detail: j.finding.detail.clone(),
1333            in_scope: j.finding.in_scope,
1334            raised_by: j.raised_by.clone(),
1335            standing: j.standing,
1336            counterpoint: j.counterpoint.clone(),
1337            defence: j.defence.clone(),
1338        }
1339    }
1340}
1341
1342/// What `spar review` leaves behind, and what a second look reads and rewrites.
1343///
1344/// `spar review` used to leave nothing machine readable: not the commit it
1345/// read, not the points it raised. There was therefore nothing to check a later
1346/// push against, and the only second look possible was a first look again.
1347#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
1348pub struct ReviewRecord {
1349    pub version: u32,
1350    /// The commit the review worktree had checked out when the reviewers read
1351    /// it. From `git rev-parse HEAD` there, never from a second `gh` call.
1352    pub head: String,
1353    /// When the review began, in the one shape GitHub returns,
1354    /// `2026-10-20T18:04:05Z`, so it compares with a comment's `created_at`
1355    /// as a string. The start rather than the end, so a reply made while a
1356    /// long review ran still counts as new on the next look.
1357    pub reviewed_at: String,
1358    #[serde(default)]
1359    pub findings: Vec<RecordedFinding>,
1360    /// Second looks taken so far, which `[listen] max_rechecks` caps.
1361    #[serde(default)]
1362    pub rechecks: u32,
1363    /// The formal verdict posted, when one was. None for a plain comment.
1364    #[serde(default, skip_serializing_if = "Option::is_none")]
1365    pub verdict: Option<ReviewVerdict>,
1366    /// Whether the review reached the pull request. A dry run's record is kept
1367    /// so `spar post` can complete it, but nobody has seen its findings, so it
1368    /// does not count as a review to take a second look after.
1369    #[serde(default)]
1370    pub posted: bool,
1371}
1372
1373pub const REVIEW_RECORD_VERSION: u32 = 1;
1374
1375impl ReviewRecord {
1376    pub fn short_head(&self) -> &str {
1377        let end = self.head.len().min(7);
1378        &self.head[..end]
1379    }
1380
1381    /// The points a second look rules on: everything but what was withdrawn.
1382    pub fn open_points(&self) -> impl Iterator<Item = &RecordedFinding> {
1383        self.findings
1384            .iter()
1385            .filter(|f| f.standing != Standing::Withdrawn)
1386    }
1387}
1388
1389// ---------------------------------------------------------------------------
1390// What gh returns
1391// ---------------------------------------------------------------------------
1392
1393#[derive(Debug, Clone, Deserialize)]
1394pub struct Label {
1395    #[serde(default)]
1396    pub name: String,
1397}
1398
1399#[derive(Debug, Clone, Deserialize, Default)]
1400pub struct Author {
1401    #[serde(default)]
1402    pub login: String,
1403}
1404
1405/// A login, never empty. A deleted account is null on GitHub and becomes
1406/// `ghost`, which no trust setting but `anyone` will act on.
1407pub fn login_of(author: Option<&Author>) -> &str {
1408    match author.map(|a| a.login.trim()) {
1409        Some(login) if !login.is_empty() => login,
1410        _ => "ghost",
1411    }
1412}
1413
1414/// One top level comment on an issue, as `gh issue view --json comments`
1415/// returns it, oldest first.
1416#[derive(Debug, Clone, Deserialize, Default)]
1417#[serde(rename_all = "camelCase")]
1418pub struct IssueComment {
1419    /// Null when the account was deleted.
1420    #[serde(default)]
1421    pub author: Option<Author>,
1422    #[serde(default)]
1423    pub author_association: String,
1424    /// Whether the authenticated GitHub user wrote this comment.
1425    #[serde(default)]
1426    pub viewer_did_author: bool,
1427    #[serde(default)]
1428    pub body: String,
1429    #[serde(default)]
1430    pub created_at: String,
1431    #[serde(default)]
1432    pub is_minimized: bool,
1433}
1434
1435impl IssueComment {
1436    pub fn login(&self) -> &str {
1437        login_of(self.author.as_ref())
1438    }
1439}
1440
1441#[derive(Debug, Clone, Deserialize)]
1442pub struct Issue {
1443    pub number: i64,
1444    #[serde(default)]
1445    pub title: String,
1446    #[serde(default)]
1447    pub body: Option<String>,
1448    #[serde(default)]
1449    pub state: String,
1450    /// Why it was closed: `completed` or `not_planned`, empty when open or
1451    /// when the field was not asked for.
1452    ///
1453    /// The tracker's tick means the work landed, and a closed issue on its own
1454    /// does not say that. With `close_skipped = true`, the default, an item
1455    /// both agents declined is closed as not planned, and ticking it writes
1456    /// "this was done" into somebody's checklist.
1457    #[serde(default, rename = "stateReason")]
1458    pub state_reason: Option<String>,
1459    #[serde(default)]
1460    pub url: String,
1461    #[serde(default)]
1462    pub labels: Vec<Label>,
1463    /// Who filed it. None when the field was not asked for.
1464    #[serde(default)]
1465    pub author: Option<Author>,
1466    /// What was said on it since it was filed. Empty when the field was not
1467    /// asked for, which a listing of the whole queue never does.
1468    #[serde(default)]
1469    pub comments: Vec<IssueComment>,
1470}
1471
1472impl Issue {
1473    pub fn body_text(&self) -> &str {
1474        self.body.as_deref().unwrap_or("")
1475    }
1476
1477    /// The body as a prompt carries it, and whether anything was left off.
1478    ///
1479    /// Shortened only past `max`, which is sized so that nothing a person
1480    /// wrote ever reaches it. When it does fire the cut is announced in the
1481    /// text itself: an agent handed a fragment with no marker has no way to
1482    /// tell it from an issue that simply ended there, so it judges the part it
1483    /// saw and reports the confidence of having seen all of it.
1484    ///
1485    /// The cut lands on a line boundary, and an unbalanced code fence is closed
1486    /// rather than left hanging. Broken markdown reads as a defect in the issue
1487    /// and costs the model attention to rule out.
1488    pub fn body_for_prompt(&self, max: usize) -> (String, bool) {
1489        let body = self.body_text().trim();
1490        if body.chars().count() <= max {
1491            return (body.to_string(), false);
1492        }
1493        let clipped: String = body.chars().take(max).collect();
1494        let mut kept = match clipped.rfind('\n') {
1495            Some(at) => clipped[..at].to_string(),
1496            None => clipped,
1497        };
1498        if kept.matches("```").count() % 2 == 1 {
1499            kept.push_str("\n```");
1500        }
1501        kept.push_str("\n\n[Shortened to fit. The rest of this issue was not included.]");
1502        (kept, true)
1503    }
1504
1505    pub fn is_closed(&self) -> bool {
1506        self.state.eq_ignore_ascii_case("closed")
1507    }
1508
1509    /// Closed because the work was done, rather than because somebody decided
1510    /// not to do it.
1511    ///
1512    /// An unknown reason counts as completed: GitHub leaves `stateReason`
1513    /// empty on issues closed before it existed, and reading those as declined
1514    /// would leave every old checklist unticked.
1515    pub fn closed_as_completed(&self) -> bool {
1516        self.is_closed() && !self.closed_as_not_planned()
1517    }
1518
1519    pub fn closed_as_not_planned(&self) -> bool {
1520        self.is_closed()
1521            && self
1522                .state_reason
1523                .as_deref()
1524                .is_some_and(|reason| reason.eq_ignore_ascii_case("not_planned"))
1525    }
1526}
1527
1528#[derive(Debug, Clone, Deserialize)]
1529pub struct PrRef {
1530    pub number: i64,
1531    #[serde(default)]
1532    pub url: String,
1533    #[serde(default)]
1534    pub title: String,
1535}
1536
1537#[derive(Debug, Clone, Deserialize)]
1538pub struct IssueRef {
1539    pub number: i64,
1540}
1541
1542/// An open pull request's size, as one listing call reports it.
1543///
1544/// Enough for the screen to say whether a change is worth splitting without
1545/// fetching its head, which would be one network round trip per pull request
1546/// for a question whose answer is usually no.
1547#[derive(Debug, Clone, Default, Deserialize)]
1548#[serde(rename_all = "camelCase")]
1549pub struct PrRow {
1550    pub number: i64,
1551    #[serde(default)]
1552    pub title: String,
1553    #[serde(default)]
1554    pub changed_files: i64,
1555    #[serde(default)]
1556    pub additions: i64,
1557    #[serde(default)]
1558    pub deletions: i64,
1559}
1560
1561impl PrRow {
1562    /// The size, as the screen prompt carries it.
1563    pub fn size(&self) -> String {
1564        format!(
1565            "{} file(s), +{} -{}",
1566            self.changed_files, self.additions, self.deletions
1567        )
1568    }
1569}
1570
1571#[derive(Debug, Clone, Deserialize)]
1572#[serde(rename_all = "camelCase")]
1573pub struct PrView {
1574    pub number: i64,
1575    #[serde(default)]
1576    pub url: String,
1577    #[serde(default)]
1578    pub title: String,
1579    /// The description, which is the author's own account of the change. Read
1580    /// by every pass that judges whether the change does what it claims.
1581    #[serde(default)]
1582    pub body: String,
1583    #[serde(default)]
1584    pub head_ref_name: String,
1585    #[serde(default)]
1586    pub base_ref_name: String,
1587    #[serde(default)]
1588    pub state: String,
1589    #[serde(default)]
1590    pub closing_issues_references: Vec<IssueRef>,
1591    /// True when the PR's head branch lives on a fork rather than this
1592    /// repository.
1593    #[serde(default)]
1594    pub is_cross_repository: bool,
1595    /// Who owns the repository the head branch is in: for a pull request
1596    /// from a fork, the fork's owner. That is what tells the authenticated
1597    /// user's own fork, which spar can push to, from anybody else's.
1598    #[serde(default)]
1599    pub head_repository_owner: Option<Author>,
1600    /// The repository the head branch is in.
1601    #[serde(default)]
1602    pub head_repository: Option<HeadRepository>,
1603}
1604
1605#[derive(Debug, Clone, Default, Deserialize)]
1606pub struct HeadRepository {
1607    #[serde(default)]
1608    pub name: String,
1609}
1610
1611/// The fields a sweep reads from `gh pr list` and `gh pr view`: enough to
1612/// notice a head that moved, a review request, an assignment, and a close.
1613pub const PR_SWEEP_FIELDS: &str =
1614    "number,title,headRefOid,updatedAt,author,isDraft,isCrossRepository,reviewRequests,assignees,state";
1615
1616/// One row of `gh pr list --json PR_SWEEP_FIELDS`, or of `gh pr view` with
1617/// the same fields.
1618#[derive(Debug, Clone, Default, Deserialize)]
1619#[serde(rename_all = "camelCase")]
1620pub struct PrSweepRow {
1621    pub number: i64,
1622    #[serde(default)]
1623    pub title: String,
1624    #[serde(default)]
1625    pub head_ref_oid: String,
1626    #[serde(default)]
1627    pub updated_at: String,
1628    #[serde(default)]
1629    pub author: Option<Author>,
1630    #[serde(default)]
1631    pub is_draft: bool,
1632    #[serde(default)]
1633    pub is_cross_repository: bool,
1634    #[serde(default)]
1635    pub review_requests: Vec<Login>,
1636    #[serde(default)]
1637    pub assignees: Vec<Login>,
1638    /// OPEN, CLOSED, or MERGED.
1639    #[serde(default)]
1640    pub state: String,
1641}
1642
1643/// A user in a `reviewRequests` or `assignees` array. A team entry has `name`
1644/// and `slug` instead, which reads as an empty login and never matches the
1645/// viewer.
1646#[derive(Debug, Clone, Default, Deserialize)]
1647pub struct Login {
1648    #[serde(default)]
1649    pub login: String,
1650    /// A team's `org/slug`, as gh gives it. Empty for a user.
1651    #[serde(default)]
1652    pub slug: String,
1653}
1654
1655/// One review request from a pull request's timeline: when it was made, and
1656/// who it asked, a user by `login` or a team by `slug` as in `Login`.
1657#[derive(Debug, Clone, Default)]
1658pub struct ReviewRequested {
1659    pub at: String,
1660    pub who: Login,
1661}
1662
1663/// Issues and pull requests share one number sequence per repository, so a
1664/// number names exactly one of them and spar can work out which.
1665#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
1666#[serde(rename_all = "lowercase")]
1667pub enum ItemKind {
1668    Issue,
1669    Pr,
1670}
1671
1672impl std::fmt::Display for ItemKind {
1673    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
1674        f.write_str(match self {
1675            ItemKind::Issue => "issue",
1676            ItemKind::Pr => "pull request",
1677        })
1678    }
1679}
1680
1681impl PrView {
1682    pub fn is_open(&self) -> bool {
1683        self.state.eq_ignore_ascii_case("open")
1684    }
1685}
1686
1687/// Where a pull request's head stands right now, asked two ways.
1688///
1689/// GitHub moves a pull request to a pushed commit some time after the push,
1690/// usually seconds and sometimes minutes, and until then `headRefOid` and
1691/// `refs/pull/N/head` both name the commit before it. The branch itself moves
1692/// with the push. A look that trusted the pull request alone read the commit
1693/// before a push, and by the time it posted its verdict was two pushes old.
1694#[derive(Debug, Clone, PartialEq, Eq)]
1695pub struct LiveHead {
1696    /// The head GitHub's pull request names.
1697    pub pull: String,
1698    /// Where the branch is, for a branch in this repository. None for a fork,
1699    /// whose branch `origin` does not have.
1700    pub branch: Option<String>,
1701}
1702
1703impl LiveHead {
1704    /// The head, once the pull request and its branch agree on it.
1705    pub fn settled(&self) -> Option<&str> {
1706        match &self.branch {
1707            Some(branch) if !branch.eq_ignore_ascii_case(&self.pull) => None,
1708            _ => Some(&self.pull),
1709        }
1710    }
1711
1712    /// The newer head when either differs from the one a look read. The
1713    /// branch is asked first, since it moves before the pull request does.
1714    pub fn moved_from(&self, reviewed: &str) -> Option<&str> {
1715        self.branch
1716            .as_deref()
1717            .filter(|branch| !branch.eq_ignore_ascii_case(reviewed))
1718            .or_else(|| (!self.pull.eq_ignore_ascii_case(reviewed)).then_some(self.pull.as_str()))
1719    }
1720}
1721
1722#[cfg(test)]
1723mod tests {
1724    use super::*;
1725
1726    #[test]
1727    fn severity_accepts_the_canonical_spelling() {
1728        assert_eq!(
1729            Some(Severity::NonBlocking),
1730            Severity::parse_lenient("non-blocking")
1731        );
1732    }
1733
1734    #[test]
1735    fn severity_accepts_near_misses() {
1736        for text in ["NonBlocking", "non_blocking", " NON-BLOCKING ", "minor"] {
1737            assert_eq!(
1738                Some(Severity::NonBlocking),
1739                Severity::parse_lenient(text),
1740                "{text}"
1741            );
1742        }
1743    }
1744
1745    #[test]
1746    fn severity_rejects_nonsense() {
1747        assert_eq!(None, Severity::parse_lenient("catastrophic-ish"));
1748    }
1749
1750    #[test]
1751    fn complexity_ordering_is_cheapest_first() {
1752        assert!(Complexity::S.rank() < Complexity::M.rank());
1753        assert!(Complexity::M.rank() < Complexity::L.rank());
1754    }
1755
1756    #[test]
1757    fn finding_defaults_to_in_scope() {
1758        let f: Finding = serde_json::from_value(serde_json::json!({
1759            "severity": "blocking", "title": "t", "detail": "d", "file": "a.rs"
1760        }))
1761        .unwrap();
1762        assert!(f.in_scope);
1763        assert!(f.blocks());
1764    }
1765
1766    #[test]
1767    fn out_of_scope_blocking_does_not_block() {
1768        let f: Finding = serde_json::from_value(serde_json::json!({
1769            "severity": "blocking", "title": "t", "detail": "d",
1770            "file": "a.rs", "in_scope": false
1771        }))
1772        .unwrap();
1773        assert!(!f.blocks());
1774    }
1775
1776    #[test]
1777    fn triage_tolerates_a_quoted_issue_number() {
1778        let v: TriageVerdict = serde_json::from_value(serde_json::json!({
1779            "issue": "#42", "worth_doing": "yes", "reason": "r",
1780            "complexity": "medium", "depends_on": ["39"], "risk": "low"
1781        }))
1782        .unwrap();
1783        assert_eq!(42, v.issue);
1784        assert!(v.worth_doing);
1785        assert_eq!(Complexity::M, v.complexity);
1786        assert_eq!(vec![39], v.depends_on);
1787    }
1788
1789    #[test]
1790    fn triage_tolerates_a_missing_depends_on() {
1791        let v: TriageVerdict = serde_json::from_value(serde_json::json!({
1792            "issue": 1, "worth_doing": false, "reason": "r",
1793            "complexity": "s", "risk": "high"
1794        }))
1795        .unwrap();
1796        assert!(v.depends_on.is_empty());
1797    }
1798
1799    #[test]
1800    fn review_tolerates_a_missing_findings_array() {
1801        let r: Review = serde_json::from_value(serde_json::json!({
1802            "verdict": "approve", "next_action": "merge", "summary": "fine"
1803        }))
1804        .unwrap();
1805        assert!(r.findings.is_empty());
1806    }
1807
1808    #[test]
1809    fn a_null_reason_is_an_empty_string_not_a_failure() {
1810        let v: TriageVerdict = serde_json::from_value(serde_json::json!({
1811            "issue": 1, "worth_doing": true, "reason": null,
1812            "complexity": "s", "depends_on": [], "risk": "low"
1813        }))
1814        .unwrap();
1815        assert_eq!("", v.reason);
1816    }
1817
1818    #[test]
1819    fn unknown_severity_is_an_error_not_a_silent_downgrade() {
1820        let out: Result<Finding, _> = serde_json::from_value(serde_json::json!({
1821            "severity": "showstopper-maybe", "title": "t", "detail": "d", "file": "a.rs"
1822        }));
1823        assert!(out.is_err());
1824    }
1825
1826    /// A `spar split` that correctly decided nothing needed splitting did its
1827    /// job. Without both of these in `succeeded`, it exits non-zero over a tidy
1828    /// queue, which in a script reads as a failure.
1829    #[test]
1830    fn deciding_not_to_split_is_not_a_failure() {
1831        for status in [Status::Split, Status::Whole] {
1832            let mut run = IssueRun::new(1, "t");
1833            run.status = status;
1834            assert!(run.succeeded(), "{status}");
1835        }
1836    }
1837
1838    #[test]
1839    fn status_round_trips_through_json() {
1840        let run = IssueRun::new(4, "t");
1841        let text = serde_json::to_string(&run).unwrap();
1842        let back: IssueRun = serde_json::from_str(&text).unwrap();
1843        assert_eq!(Status::Pending, back.status);
1844    }
1845
1846    /// Nothing was posted, so a script that runs `spar review` has something
1847    /// to try again.
1848    #[test]
1849    fn a_look_whose_head_moved_did_not_do_its_job() {
1850        let mut run = IssueRun::new(4, "t");
1851        run.status = Status::Moved;
1852        assert!(!run.succeeded());
1853        let text = serde_json::to_string(&run).unwrap();
1854        assert!(text.contains("\"moved\""), "{text}");
1855        let back: IssueRun = serde_json::from_str(&text).unwrap();
1856        assert_eq!(Status::Moved, back.status);
1857    }
1858
1859    fn live(pull: &str, branch: Option<&str>) -> LiveHead {
1860        LiveHead {
1861            pull: pull.into(),
1862            branch: branch.map(String::from),
1863        }
1864    }
1865
1866    /// A branch can be at the pushed commit for minutes while its pull request
1867    /// still names the one before.
1868    #[test]
1869    fn a_head_is_settled_only_once_the_pull_request_has_caught_up_with_its_branch() {
1870        assert_eq!(None, live("fb3a7ab", Some("2d5e2b5")).settled());
1871        assert_eq!(Some("2d5e2b5"), live("2d5e2b5", Some("2d5e2b5")).settled());
1872        assert_eq!(Some("2d5e2b5"), live("2d5e2b5", Some("2D5E2B5")).settled());
1873        // A fork's branch is not on origin, so the pull request is all there is.
1874        assert_eq!(Some("fb3a7ab"), live("fb3a7ab", None).settled());
1875    }
1876
1877    #[test]
1878    fn a_head_moved_from_the_reviewed_one_names_the_newer_commit() {
1879        assert_eq!(None, live("fb3a7ab", Some("fb3a7ab")).moved_from("fb3a7ab"));
1880        assert_eq!(None, live("FB3A7AB", None).moved_from("fb3a7ab"));
1881        // The branch moves first, so it is the newer of the two.
1882        assert_eq!(
1883            Some("c493635"),
1884            live("2d5e2b5", Some("c493635")).moved_from("fb3a7ab")
1885        );
1886        assert_eq!(
1887            Some("c493635"),
1888            live("fb3a7ab", Some("c493635")).moved_from("fb3a7ab")
1889        );
1890        assert_eq!(Some("2d5e2b5"), live("2d5e2b5", None).moved_from("fb3a7ab"));
1891        assert_eq!(
1892            Some("2d5e2b5"),
1893            live("2d5e2b5", Some("fb3a7ab")).moved_from("fb3a7ab")
1894        );
1895    }
1896}
1897
1898#[cfg(test)]
1899mod body_for_prompt_tests {
1900    use super::*;
1901
1902    fn issue(body: &str) -> Issue {
1903        let mut i: Issue = serde_json::from_value(serde_json::json!({
1904            "number": 1, "title": "t", "state": "open", "url": "u"
1905        }))
1906        .expect("an issue");
1907        i.body = Some(body.to_string());
1908        i
1909    }
1910
1911    /// The case that is every real issue: nothing is touched and nothing is
1912    /// claimed to be.
1913    #[test]
1914    fn an_issue_that_fits_is_handed_over_whole() {
1915        let (body, cut) = issue("The guard is inverted.").body_for_prompt(60_000);
1916        assert_eq!("The guard is inverted.", body);
1917        assert!(!cut);
1918    }
1919
1920    /// A fragment with no marker is indistinguishable from an issue that ended
1921    /// there, so the agent judges what it saw with the confidence of having
1922    /// seen everything. That is what the silent caps did.
1923    #[test]
1924    fn a_shortened_body_says_so_in_the_text() {
1925        let long = "line of text\n".repeat(500);
1926        let (body, cut) = issue(&long).body_for_prompt(200);
1927        assert!(cut);
1928        assert!(body.contains("Shortened to fit"), "{body}");
1929        assert!(body.len() < long.len());
1930    }
1931
1932    /// Broken markdown reads as a defect in the issue, and costs the model
1933    /// attention to rule out.
1934    #[test]
1935    fn a_cut_never_leaves_a_code_fence_open() {
1936        let body = format!("intro\n\n```rust\n{}\n```\n", "let x = 1;\n".repeat(200));
1937        let (out, cut) = issue(&body).body_for_prompt(120);
1938        assert!(cut);
1939        assert_eq!(0, out.matches("```").count() % 2, "{out}");
1940    }
1941
1942    /// Cutting mid-word turns the last thing the agent reads into nonsense.
1943    #[test]
1944    fn a_cut_lands_on_a_line_boundary() {
1945        let body = "aaaa bbbb cccc\n".repeat(100);
1946        let (out, _) = issue(&body).body_for_prompt(100);
1947        let kept = out.split("\n\n[Shortened").next().expect("the kept part");
1948        assert!(kept.ends_with("cccc"), "{kept:?}");
1949    }
1950}
1951
1952#[cfg(test)]
1953mod review_record_tests {
1954    use super::*;
1955
1956    fn judged() -> Judged {
1957        Judged {
1958            finding: Finding {
1959                severity: Severity::Blocking,
1960                title: "Retry loop never terminates".into(),
1961                detail: "the guard compares against Some(0)".into(),
1962                file: "src/net.rs:88".into(),
1963                in_scope: true,
1964                problem: Some("dropped on the way to the record".into()),
1965                ..Finding::default()
1966            },
1967            raised_by: "claude and codex".into(),
1968            standing: Standing::Corroborated,
1969            counterpoint: None,
1970            defence: Some("stands".into()),
1971            rated: None,
1972        }
1973    }
1974
1975    #[test]
1976    fn a_review_record_round_trips_through_json_and_keeps_its_standing_as_a_word() {
1977        let record = ReviewRecord {
1978            version: REVIEW_RECORD_VERSION,
1979            head: "0123abcdef".into(),
1980            reviewed_at: "2026-10-20T18:04:05Z".into(),
1981            findings: vec![RecordedFinding::from(&judged())],
1982            rechecks: 2,
1983            verdict: Some(ReviewVerdict::RequestChanges),
1984            posted: true,
1985        };
1986        let text = serde_json::to_string_pretty(&record).unwrap();
1987        assert!(text.contains("\"standing\": \"corroborated\""), "{text}");
1988        assert!(text.contains("\"verdict\": \"request_changes\""), "{text}");
1989        assert!(text.contains("\"severity\": \"blocking\""), "{text}");
1990        assert!(!text.contains("problem"), "{text}");
1991        let back: ReviewRecord = serde_json::from_str(&text).unwrap();
1992        assert_eq!(record, back);
1993        assert_eq!("0123abc", back.short_head());
1994    }
1995
1996    #[test]
1997    fn a_record_missing_the_newer_fields_still_reads() {
1998        let text = r#"{"version":1,"head":"abc","reviewed_at":"2026-10-20T18:04:05Z",
1999            "findings":[{"title":"T","severity":"nit","standing":"one_reviewer"}]}"#;
2000        let back: ReviewRecord = serde_json::from_str(text).unwrap();
2001        assert_eq!(0, back.rechecks);
2002        assert!(!back.posted);
2003        assert_eq!(None, back.verdict);
2004        assert!(back.findings[0].in_scope);
2005        assert_eq!(Standing::Unverified, back.findings[0].standing);
2006        assert_eq!("general", back.findings[0].where_at());
2007    }
2008
2009    #[test]
2010    fn a_recorded_finding_turns_back_into_a_judged_finding() {
2011        let original = judged();
2012        let back = RecordedFinding::from(&original).to_judged();
2013        assert_eq!(original.finding.title, back.finding.title);
2014        assert_eq!(original.finding.file, back.finding.file);
2015        assert_eq!(original.finding.severity, back.finding.severity);
2016        assert_eq!(original.raised_by, back.raised_by);
2017        assert_eq!(original.standing, back.standing);
2018        assert_eq!(original.defence, back.defence);
2019        assert_eq!(None, back.finding.problem);
2020        assert!(back.finding.blocks());
2021    }
2022
2023    #[test]
2024    fn a_ruling_tolerates_a_quoted_index_and_a_synonym_outcome() {
2025        let doc: RecheckDoc = serde_json::from_str(
2026            r#"{"summary":"holds","rulings":[{"index":"2","outcome":"Resolved","reasoning":"line 4"}],"findings":[]}"#,
2027        )
2028        .unwrap();
2029        assert_eq!(2, doc.rulings[0].index);
2030        assert_eq!(Ruling::Fixed, doc.rulings[0].outcome);
2031        let doc: RecheckDoc =
2032            serde_json::from_str(r#"{"summary":"","rulings":[],"findings":[]}"#).unwrap();
2033        assert!(doc.rulings.is_empty());
2034        assert_eq!("APPROVE", ReviewVerdict::Approve.api_event());
2035        assert_eq!("changes requested", ReviewVerdict::RequestChanges.words());
2036    }
2037}