Skip to main content

spar/
schema.rs

1//! JSON schemas for the three structured exchanges.
2//!
3//! These are what make convergence machine checkable instead of regex matching
4//! prose for "LGTM". Three properties are load bearing for strict structured
5//! output and are asserted in the tests: every property appears in `required`,
6//! every object sets `additionalProperties: false`, and an optional field is
7//! spelled as one that may be null rather than one that may be absent.
8//!
9//! The `description` on each field is also the cheapest place to ask for
10//! brevity, since it travels with the request rather than sitting a thousand
11//! tokens back in the prompt.
12
13use serde_json::{json, Value};
14
15pub fn triage() -> Value {
16    json!({
17        "type": "object",
18        "additionalProperties": false,
19        "properties": {
20            "issues": {
21                "type": "array",
22                "items": {
23                    "type": "object",
24                    "additionalProperties": false,
25                    "properties": {
26                        "issue": {"type": "integer", "description": "The issue number."},
27                        "worth_doing": {
28                            "type": "boolean",
29                            "description": "False for duplicates, stale requests, things already fixed, vague reports with nothing reproducible, or changes that would make the codebase worse."
30                        },
31                        "reason": {
32                            "type": "string",
33                            "description": "One sentence. This is posted verbatim on the issue when both agents decline it, so write it for the person who opened it."
34                        },
35                        "complexity": {"type": "string", "enum": ["s", "m", "l"]},
36                        "depends_on": {
37                            "type": "array",
38                            "items": {"type": "integer"},
39                            "description": "Issue numbers from this same list that should land first. Empty if none."
40                        },
41                        "risk": {"type": "string", "enum": ["low", "med", "high"]}
42                    },
43                    "required": ["issue", "worth_doing", "reason", "complexity", "depends_on", "risk"]
44                }
45            }
46        },
47        "required": ["issues"]
48    })
49}
50
51pub fn review() -> Value {
52    json!({
53        "type": "object",
54        "additionalProperties": false,
55        "properties": {
56            "verdict": {"type": "string", "enum": ["approve", "changes_requested"]},
57            "next_action": {"type": "string", "enum": ["merge", "fix_myself", "hand_back"]},
58            "summary": {
59                "type": "string",
60                "description": "One sentence, at most 200 characters. No preamble, no restating the diff."
61            },
62            "findings": {
63                "type": "array",
64                "items": {
65                    "type": "object",
66                    "additionalProperties": false,
67                    "properties": {
68                        "severity": {
69                            "type": "string",
70                            "enum": ["blocking", "non-blocking", "nit"],
71                            "description": "blocking: the PR should not merge as is, real defects only. non-blocking: a genuine improvement that need not gate this PR. nit: style or taste."
72                        },
73                        "title": {
74                            "type": "string",
75                            "description": "Under 80 characters. State the defect, not the fix."
76                        },
77                        "detail": {
78                            "type": "string",
79                            "description": "At most two sentences. For a blocking finding, say what you did to confirm it. No restating the title."
80                        },
81                        "file": {
82                            "type": "string",
83                            "description": "Path, with a line number if you have one. Empty string if the finding is general."
84                        },
85                        "in_scope": {
86                            "type": "boolean",
87                            "description": "False for a real problem that exists but is not caused by this PR. Those become follow up issues rather than review comments."
88                        }
89                    },
90                    "required": ["severity", "title", "detail", "file", "in_scope"]
91                }
92            }
93        },
94        "required": ["verdict", "next_action", "summary", "findings"]
95    })
96}
97
98pub fn response() -> Value {
99    json!({
100        "type": "object",
101        "additionalProperties": false,
102        "properties": {
103            "summary": {
104                "type": "string",
105                "description": "One sentence, at most 200 characters."
106            },
107            "dispositions": {
108                "type": "array",
109                "items": {
110                    "type": "object",
111                    "additionalProperties": false,
112                    "properties": {
113                        "title": {
114                            "type": "string",
115                            "description": "Copy the reviewer's finding title exactly, so the two can be matched up."
116                        },
117                        "file": {
118                            "type": "string",
119                            "description": "Copy the reviewer's file for this finding exactly. Empty string if it had none."
120                        },
121                        "action": {
122                            "type": "string",
123                            "enum": ["fixed", "refuted", "filed_issue"],
124                            "description": "fixed: valid and in scope, you fixed it. refuted: the point is wrong or not worth acting on. filed_issue: valid but unrelated to this PR."
125                        },
126                        "reasoning": {
127                            "type": "string",
128                            "description": "One or two sentences. For a refutation this is the whole argument, so make it the reason and not an apology."
129                        },
130                        "new_issue_title": {
131                            "type": ["string", "null"],
132                            "description": "Only for filed_issue, null otherwise."
133                        },
134                        "new_issue_body": {
135                            "type": ["string", "null"],
136                            "description": "Only for filed_issue, null otherwise. Under 600 characters."
137                        }
138                    },
139                    "required": ["title", "file", "action", "reasoning", "new_issue_title", "new_issue_body"]
140                }
141            }
142        },
143        "required": ["summary", "dispositions"]
144    })
145}
146
147/// One reviewer judging the other reviewer's findings.
148///
149/// Used only in review only mode, where nobody is going to fix anything and the
150/// product is the finding list itself. Asking each model to read the code and
151/// rule on the other's claims is what separates a defect worth a maintainer's
152/// attention from one model's pattern match.
153pub fn adjudication() -> Value {
154    json!({
155        "type": "object",
156        "additionalProperties": false,
157        "properties": {
158            "verdicts": {
159                "type": "array",
160                "items": {
161                    "type": "object",
162                    "additionalProperties": false,
163                    "properties": {
164                        "title": {
165                            "type": "string",
166                            "description": "Copy the finding's title exactly, so it can be matched up."
167                        },
168                        "file": {
169                            "type": "string",
170                            "description": "Copy the finding's file exactly. Empty string if it had none."
171                        },
172                        "agrees": {
173                            "type": "boolean",
174                            "description": "True only if you read the code and the defect is real. Do not defer to the other reviewer, and do not agree to be agreeable: a finding you cannot confirm is one a maintainer should not have to spend time on."
175                        },
176                        "severity": {
177                            "type": "string",
178                            "enum": ["blocking", "non-blocking", "nit"],
179                            "description": "Your own view of how badly it matters, even where you agree the defect is real."
180                        },
181                        "reasoning": {
182                            "type": "string",
183                            "description": "One or two sentences. If you disagree, this is the whole argument, so give the reason rather than an opinion."
184                        }
185                    },
186                    "required": ["title", "file", "agrees", "severity", "reasoning"]
187                }
188            }
189        },
190        "required": ["verdicts"]
191    })
192}
193
194pub fn all() -> Vec<(&'static str, Value)> {
195    vec![
196        ("triage", triage()),
197        ("review", review()),
198        ("response", response()),
199    ]
200}
201
202#[cfg(test)]
203mod tests {
204    use super::*;
205
206    /// Yield every object schema, however deeply nested.
207    fn objects(node: &Value, path: String, out: &mut Vec<(String, Value)>) {
208        if let Some(map) = node.as_object() {
209            if map.get("type").and_then(Value::as_str) == Some("object")
210                && map.contains_key("properties")
211            {
212                out.push((path.clone(), node.clone()));
213                if let Some(props) = map.get("properties").and_then(Value::as_object) {
214                    for (key, child) in props {
215                        objects(child, format!("{path}.{key}"), out);
216                    }
217                }
218            }
219            if let Some(items) = map.get("items") {
220                objects(items, format!("{path}[]"), out);
221            }
222        }
223    }
224
225    fn walk(name: &str, schema: &Value) -> Vec<(String, Value)> {
226        let mut out = Vec::new();
227        objects(schema, name.to_string(), &mut out);
228        out
229    }
230
231    /// Strict structured output rejects any property that is not also in
232    /// `required`. The Python original violated this in the response schema
233    /// from the start and nothing caught it, because the response schema is
234    /// only reached when a review is handed back with blocking findings, and
235    /// almost every run approved in round one.
236    #[test]
237    fn every_property_is_required() {
238        for (name, schema) in all() {
239            for (path, node) in walk(name, &schema) {
240                let props: Vec<&String> = node["properties"].as_object().unwrap().keys().collect();
241                let required: Vec<String> = node["required"]
242                    .as_array()
243                    .unwrap_or(&vec![])
244                    .iter()
245                    .filter_map(|v| v.as_str().map(str::to_string))
246                    .collect();
247                for prop in &props {
248                    assert!(
249                        required.contains(prop),
250                        "{path}: {prop} is in properties but not in required. \
251                         Make optional fields nullable instead."
252                    );
253                }
254                assert_eq!(props.len(), required.len(), "{path}: required has extras");
255            }
256        }
257    }
258
259    #[test]
260    fn objects_forbid_additional_properties() {
261        for (name, schema) in all() {
262            for (path, node) in walk(name, &schema) {
263                assert_eq!(
264                    Some(false),
265                    node["additionalProperties"].as_bool(),
266                    "{path} allows additional properties"
267                );
268            }
269        }
270    }
271
272    #[test]
273    fn optional_fields_are_spelled_as_nullable() {
274        let item = &response()["properties"]["dispositions"]["items"];
275        for field in ["new_issue_title", "new_issue_body"] {
276            let types = item["properties"][field]["type"].to_string();
277            assert!(types.contains("null"), "{field} must accept null: {types}");
278        }
279    }
280
281    /// The re-litigation guard hashes a refutation by title *and* file. If the
282    /// disposition cannot carry the file, the key it records can never match
283    /// the key the next round's finding hashes to, and the guard is dead code.
284    #[test]
285    fn a_disposition_carries_the_file_so_the_ledger_key_can_match() {
286        let props = response()["properties"]["dispositions"]["items"]["properties"].clone();
287        assert!(
288            props.get("file").is_some(),
289            "dispositions must carry a file"
290        );
291    }
292
293    #[test]
294    fn severity_and_verdict_enums_match_the_parser() {
295        use crate::model::{Severity, Verdict};
296        let sev =
297            review()["properties"]["findings"]["items"]["properties"]["severity"]["enum"].clone();
298        for value in sev.as_array().unwrap() {
299            assert!(
300                Severity::parse_lenient(value.as_str().unwrap()).is_some(),
301                "schema offers {value} but the parser rejects it"
302            );
303        }
304        let verdicts = review()["properties"]["verdict"]["enum"].clone();
305        for value in verdicts.as_array().unwrap() {
306            assert!(Verdict::parse_lenient(value.as_str().unwrap()).is_some());
307        }
308    }
309
310    #[test]
311    fn triage_enums_match_the_parser() {
312        use crate::model::{Complexity, Risk};
313        let item = &triage()["properties"]["issues"]["items"]["properties"];
314        for value in item["complexity"]["enum"].as_array().unwrap() {
315            assert!(Complexity::parse_lenient(value.as_str().unwrap()).is_some());
316        }
317        for value in item["risk"]["enum"].as_array().unwrap() {
318            assert!(Risk::parse_lenient(value.as_str().unwrap()).is_some());
319        }
320    }
321
322    #[test]
323    fn response_action_enum_matches_the_parser() {
324        use crate::model::Action;
325        let actions = response()["properties"]["dispositions"]["items"]["properties"]["action"]
326            ["enum"]
327            .clone();
328        for value in actions.as_array().unwrap() {
329            assert!(Action::parse_lenient(value.as_str().unwrap()).is_some());
330        }
331    }
332}