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
15/// A single actionable issue, drafted against the repository.
16pub fn code_issue() -> Value {
17    json!({
18        "type": "object",
19        "additionalProperties": false,
20        "properties": {
21            "title": {"type": "string", "description": "A concise, specific issue title."},
22            "body": {"type": "string", "description": "A detailed Markdown issue with context, requested behavior, scope, acceptance criteria, and a test plan. Distinguish verified facts from assumptions."}
23        },
24        "required": ["title", "body"]
25    })
26}
27
28pub fn triage() -> Value {
29    json!({
30        "type": "object",
31        "additionalProperties": false,
32        "properties": {
33            "issues": {
34                "type": "array",
35                "items": {
36                    "type": "object",
37                    "additionalProperties": false,
38                    "properties": {
39                        "issue": {"type": "integer", "description": "The issue number."},
40                        "worth_doing": {
41                            "type": "boolean",
42                            "description": "False for duplicates, stale requests, things already fixed, vague reports with nothing reproducible, changes that would make the codebase worse, and tracking issues. Set tracker as well for the last of those."
43                        },
44                        "tracker": {
45                            "type": "boolean",
46                            "description": "True when the issue exists to hold context for work that is filed elsewhere: an umbrella, an epic, a meta issue whose parts are their own issues. Judge it by what the issue is, not by whether you agree with it. Nothing is opened for a tracker, but it is not finished either: its parts are still open, and the shared context and rejected alternatives it records are why somebody wrote it. False for an ordinary issue, whatever you decided about it."
47                        },
48                        "reason": {
49                            "type": "string",
50                            "description": "One sentence. This is posted verbatim on the issue when both agents decline it, and the issue may well stay open afterwards, so write it for the person who opened it rather than as a verdict."
51                        },
52                        "complexity": {
53                            "type": "string",
54                            "enum": ["s", "m", "l"],
55                            "description": "How much work the change is. s: one file, or a handful of lines across two, covered by an existing test or one obvious one. m: several files in one area, or a new function others call, or one that needs its own tests written. l: a new module, a change to an interface other code depends on, a format that has to migrate, or anything you cannot verify by running something that already exists. This decides the order the run works in, cheapest first, and the larger of the two reviewers' answers is used."
56                        },
57                        "depends_on": {
58                            "type": "array",
59                            "items": {"type": "integer"},
60                            "description": "Issue numbers from this same list that should land first. Empty if none."
61                        },
62                        "risk": {
63                            "type": "string",
64                            "enum": ["low", "med", "high"],
65                            "description": "The blast radius if the change is wrong, and how easily it is undone. low: contained in one place and a revert puts everything back. med: other code calls into it, or it changes behaviour somebody may rely on, but the failure is visible. high: it touches data that persists, a security or permission boundary, or a path whose failure is silent. The larger of the two reviewers' answers is used."
66                        }
67                    },
68                    "required": ["issue", "worth_doing", "tracker", "reason", "complexity", "depends_on", "risk"]
69                }
70            }
71        },
72        "required": ["issues"]
73    })
74}
75
76pub fn review() -> Value {
77    json!({
78        "type": "object",
79        "additionalProperties": false,
80        "properties": {
81            "verdict": {
82                "type": "string",
83                "enum": ["approve", "changes_requested"],
84                "description": "Your overall call. The findings and next_action are what act, so this has to agree with them: approve means you raised nothing blocking. A disagreement between the two is reported and the findings decide."
85            },
86            "next_action": {"type": "string", "enum": ["merge", "fix_myself", "hand_back"]},
87            "summary": {
88                "type": "string",
89                "description": "One sentence, at most 200 characters. No preamble, no restating the diff."
90            },
91            "findings": {
92                "type": "array",
93                "items": {
94                    "type": "object",
95                    "additionalProperties": false,
96                    "properties": {
97                        "severity": {
98                            "type": "string",
99                            "enum": ["blocking", "non-blocking", "nit"],
100                            "description": "blocking: this should not merge as is, real defects only. non-blocking: real, and smaller than holding the merge for. A minor defect belongs here as much as an improvement does. nit: style or taste."
101                        },
102                        "title": {
103                            "type": "string",
104                            "description": "Under 80 characters. State the defect, not the fix."
105                        },
106                        "detail": {
107                            "type": "string",
108                            "description": "Say what goes wrong, how to reproduce it, and where in the code. For a blocking finding, say what you did to confirm it. Do not restate the title. Lead with one sentence that stands on its own: a shortened form of this appears in the pull request thread, while the full text becomes the body if this is filed as its own issue. A fenced code block is welcome and is never truncated."
109                        },
110                        "file": {
111                            "type": "string",
112                            "description": "Path from the repository root, then :line or :first-last for the line in the branch's version of the file when you have one, as src/net.rs:88. Empty string if the finding is general."
113                        },
114                        "problem": {
115                            "type": ["string", "null"],
116                            "description": "Only when in_scope is false, null otherwise. What is wrong, with the specifics: the function, the call it does not make, the condition it does not check. Name things in backticks. This becomes the Problem section of an issue somebody picks up cold, so write what they need rather than what fits on a line."
117                        },
118                        "reproduction": {
119                            "type": ["string", "null"],
120                            "description": "Only when in_scope is false, null otherwise. Numbered steps to reproduce it, then a short 'Actual result:' list of what happens. If part of what happens is correct and only part is the defect, say which, so nobody chases the wrong thing."
121                        },
122                        "impact": {
123                            "type": ["string", "null"],
124                            "description": "Only when in_scope is false, null otherwise. What it costs somebody: what an operator or a user can do, or loses, because of this. One short paragraph."
125                        },
126                        "expected": {
127                            "type": ["string", "null"],
128                            "description": "Only when in_scope is false, null otherwise. What it should do instead, as a list of requirements specific enough to implement and to test. Say if the behaviour predates this branch."
129                        },
130                        "in_scope": {
131                            "type": "boolean",
132                            "description": "False only for a real defect that exists, that this PR did not cause, and that is worth somebody stopping to fix. It becomes a tracked item a maintainer has to read and triage, so the bar is a defect, not an observation. A thorough reviewer can always find something adjacent; that is not a reason to file it. If you are not sure it is worth a maintainer's time, say your piece in the finding and label it non-blocking."
133                        }
134                    },
135                    "required": [
136                        "severity",
137                        "title",
138                        "detail",
139                        "file",
140                        "in_scope",
141                        "problem",
142                        "reproduction",
143                        "impact",
144                        "expected"
145                    ]
146                }
147            }
148        },
149        "required": ["verdict", "next_action", "summary", "findings"]
150    })
151}
152
153/// What the implementor reports back, and the pull request body it becomes.
154///
155/// The body used to be one scraped `SUMMARY:` line under a `Closes #N`, which
156/// told a reviewer opening the diff cold nothing: not what was wrong, not what
157/// the change does about it, not how to check it. Asking for those separately
158/// is what puts them there, and composing the body from the fields rather than
159/// from the model's prose is what keeps it short enough to read.
160pub fn implementation() -> Value {
161    json!({
162        "type": "object",
163        "additionalProperties": false,
164        "properties": {
165            "not_worth_doing": {
166                "type": "boolean",
167                "description": "True if, having read the code, this should not be implemented: a duplicate, already fixed, too vague to act on, or a change that would make the codebase worse. Make no changes when this is true."
168            },
169            "reason": {
170                "type": "string",
171                "description": "Only when not_worth_doing is true, empty string otherwise. One or two sentences, posted verbatim on the issue, so write it for the person who opened it."
172            },
173            "summary": {
174                "type": "string",
175                "description": "One plain sentence saying what changed, at most 200 characters. It leads the pull request body, and it carries one fact: what this does now that it did not before. Not the signatures, not the null handling, not the edge cases, all of which belong in changes. If it needs a comma to join two ideas, or reads like a changelog line, it is carrying too much."
176            },
177            "problem": {
178                "type": "string",
179                "description": "Two to four short sentences on what was actually wrong and what it cost, as you understand it now that you have read the code. One fact each: a sentence naming three functions and their signatures is one the reviewer has to decipher, and splitting it costs a few words and saves them that. Not a restatement of the issue, which the reviewer can open for themselves: what you found. Empty string for a feature request with no defect behind it, where a sentence on why it is worth having belongs here instead."
180            },
181            "changes": {
182                "type": "array",
183                "items": {"type": "string"},
184                "description": "One short line per change that alters behaviour, in the order a reader should meet them. Say what the code now does, and name the function or file in backticks. This is where a signature or a null case belongs, one per line, rather than crowded into the summary. Not a list of touched files: the diff already has that. Empty when the summary covers it, which for a small change it does."
185            },
186            "testing": {
187                "type": "array",
188                "items": {"type": "string"},
189                "description": "How a reviewer confirms this works, as lines they can act on: the exact command in backticks, or the steps and what to look for. Say what you actually ran, not what could be run. Name the test that covers the fix. Empty only when there is genuinely nothing to run."
190            },
191            "notes": {
192                "type": ["string", "null"],
193                "description": "Null unless there is something the reviewer would otherwise have to ask about: a deliberate omission, a decision worth defending, a risk you are taking knowingly. Not a summary of the above, and not an apology."
194            }
195        },
196        "required": ["not_worth_doing", "reason", "summary", "problem", "changes", "testing", "notes"]
197    })
198}
199
200pub fn response() -> Value {
201    json!({
202        "type": "object",
203        "additionalProperties": false,
204        "properties": {
205            "summary": {
206                "type": "string",
207                "description": "One sentence, at most 200 characters."
208            },
209            "dispositions": {
210                "type": "array",
211                "items": {
212                    "type": "object",
213                    "additionalProperties": false,
214                    "properties": {
215                        "title": {
216                            "type": "string",
217                            "description": "Copy the reviewer's finding title exactly, so the two can be matched up."
218                        },
219                        "file": {
220                            "type": "string",
221                            "description": "Copy the reviewer's file for this finding exactly. Empty string if it had none."
222                        },
223                        "action": {
224                            "type": "string",
225                            "enum": ["fixed", "refuted", "filed_issue"],
226                            "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."
227                        },
228                        "reasoning": {
229                            "type": "string",
230                            "description": "One or two sentences. For a refutation this is the whole argument, so make it the reason and not an apology."
231                        },
232                        "new_issue_title": {
233                            "type": ["string", "null"],
234                            "description": "Only for filed_issue, null otherwise."
235                        },
236                        "new_issue_body": {
237                            "type": ["string", "null"],
238                            "description": "Only for filed_issue, null otherwise. This becomes an issue body somebody picks up cold, so use these markdown sections, skipping any that do not apply: `## Problem` with the specifics, `## Reproduction` with numbered steps and an Actual result list, `## Impact` with what it costs somebody, and `## Expected behavior` as requirements specific enough to implement and to test. Substance rather than length: no preamble, no restating the title. A fenced code block is welcome and is never truncated."
239                        }
240                    },
241                    "required": ["title", "file", "action", "reasoning", "new_issue_title", "new_issue_body"]
242                }
243            }
244        },
245        "required": ["summary", "dispositions"]
246    })
247}
248
249/// One reviewer judging the other reviewer's findings.
250///
251/// Used only in review only mode, where nobody is going to fix anything and the
252/// product is the finding list itself. Asking each model to read the code and
253/// rule on the other's claims is what separates a defect worth a maintainer's
254/// attention from one model's pattern match.
255pub fn adjudication() -> Value {
256    json!({
257        "type": "object",
258        "additionalProperties": false,
259        "properties": {
260            "verdicts": {
261                "type": "array",
262                "items": {
263                    "type": "object",
264                    "additionalProperties": false,
265                    "properties": {
266                        "title": {
267                            "type": "string",
268                            "description": "Copy the finding's title exactly, so it can be matched up."
269                        },
270                        "file": {
271                            "type": "string",
272                            "description": "Copy the finding's file exactly. Empty string if it had none."
273                        },
274                        "agrees": {
275                            "type": "boolean",
276                            "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."
277                        },
278                        "severity": {
279                            "type": "string",
280                            "enum": ["blocking", "non-blocking", "nit"],
281                            "description": "Your own view of how badly it matters, even where you agree the defect is real."
282                        },
283                        "reasoning": {
284                            "type": "string",
285                            "description": "One or two sentences. If you disagree, this is the whole argument, so give the reason rather than an opinion."
286                        }
287                    },
288                    "required": ["title", "file", "agrees", "severity", "reasoning"]
289                }
290            }
291        },
292        "required": ["verdicts"]
293    })
294}
295
296/// Both agents, after a reviewed pull request moved: a ruling per earlier
297/// finding, matched back by the number the prompt gave it, and ordinary
298/// findings for anything new in the delta. The finding item is `review()`'s,
299/// so there is one definition of a finding.
300pub fn recheck() -> Value {
301    let mut findings = review()["properties"]["findings"].clone();
302    findings["description"] = json!(
303        "Only what is new since the last review. An earlier point belongs in its ruling, not here."
304    );
305    json!({
306        "type": "object",
307        "additionalProperties": false,
308        "properties": {
309            "summary": {
310                "type": "string",
311                "description": "One sentence, at most 200 characters, on what changed since the last review and whether it holds up."
312            },
313            "rulings": {
314                "type": "array",
315                "items": {
316                    "type": "object",
317                    "additionalProperties": false,
318                    "properties": {
319                        "index": {
320                            "type": "integer",
321                            "description": "The number the earlier finding was given in the list. Copy it exactly, so the ruling can be matched back to the point it is about."
322                        },
323                        "outcome": {
324                            "type": "string",
325                            "enum": ["fixed", "rebutted", "open"],
326                            "description": "fixed: the code now answers the point. rebutted: the code did not change for it, and a quoted response shows the point was wrong or does not apply here. open: neither. Say open when you are unsure: an open point costs one more look, a wrongly closed one ships."
327                        },
328                        "reasoning": {
329                            "type": "string",
330                            "description": "One or two sentences. For fixed, the file and line that answers it and what you did to confirm that. For rebutted, which response and why it holds against the code. For open, what is still missing. This is posted for the author, so write it to them."
331                        }
332                    },
333                    "required": ["index", "outcome", "reasoning"]
334                }
335            },
336            "findings": findings
337        },
338        "required": ["summary", "rulings", "findings"]
339    })
340}
341
342/// One agent ruling on recorded follow-ups, before any becomes an issue.
343///
344/// Deliberately not the triage schema. Triage asks whether an issue is worth a
345/// pull request; this asks whether a note written weeks ago is still true of the
346/// code, which is a different question with a different expensive mistake:
347/// dropping something real, rather than scheduling something small.
348pub fn screen() -> Value {
349    json!({
350        "type": "object",
351        "additionalProperties": false,
352        "properties": {
353            "entries": {
354                "type": "array",
355                "items": {
356                    "type": "object",
357                    "additionalProperties": false,
358                    "properties": {
359                        "entry": {
360                            "type": "integer",
361                            "description": "The number this entry was given in the list above. Copy it exactly, so the verdict can be matched back to the entry it is about."
362                        },
363                        "verdict": {
364                            "type": "string",
365                            "enum": ["still_relevant", "already_fixed", "not_worth_it", "duplicate"],
366                            "description": "still_relevant files it as an issue. The other three take it out of the queue and file nothing, so say still_relevant when you are unsure: what survives is triaged by both agents afterwards and can be declined there, while what is dropped here is dropped."
367                        },
368                        "title": {
369                            "type": "string",
370                            "description": "The entry's title, which becomes the issue title. Copy it across unchanged unless it is wrong or says nothing, in which case write one that states the defect."
371                        },
372                        "reason": {
373                            "type": "string",
374                            "description": "One sentence. For already_fixed, name the function or the change that fixed it, so somebody can check you. For anything but still_relevant this is the only record of why the entry was dropped, so give the reason rather than the verdict again."
375                        },
376                        "duplicate_of_issue": {
377                            "type": ["integer", "null"],
378                            "description": "Only when the verdict is duplicate and an open issue already covers it. The issue number, null otherwise."
379                        },
380                        "duplicate_of_entry": {
381                            "type": ["integer", "null"],
382                            "description": "Only when the verdict is duplicate and another entry in this same list covers it. That entry's number from the list above, null otherwise. Kept apart from duplicate_of_issue because an entry is not an issue: it may itself be dropped, and then this one is kept rather than lost."
383                        }
384                    },
385                    "required": ["entry", "verdict", "title", "reason", "duplicate_of_issue", "duplicate_of_entry"]
386                }
387            }
388        },
389        "required": ["entries"]
390    })
391}
392
393/// One agent ruling on a whole queue at once: is this worth splitting.
394///
395/// One call rather than N, for the reason `screen` is one call: a repository
396/// aware pass is the dominant cost, and "is this too big to work in one piece"
397/// is partly a judgement across the queue rather than about each item alone.
398pub fn split_screen() -> Value {
399    json!({
400        "type": "object",
401        "additionalProperties": false,
402        "properties": {
403            "items": {
404                "type": "array",
405                "items": {
406                    "type": "object",
407                    "additionalProperties": false,
408                    "properties": {
409                        "item": {
410                            "type": "integer",
411                            "description": "The issue or pull request number from the list above, copied exactly, so the verdict can be matched back to what it is about."
412                        },
413                        "split": {
414                            "type": "boolean",
415                            "description": "True only for something that is plainly several separate pieces of work and would be reviewed better as several. Say false when you are unsure: no is the common answer, and a split proposed on a whim is a proposal somebody now has to read. Size alone is not the test, since a forty file rename is one piece of work and a three file mess can be three."
416                        },
417                        "reason": {
418                            "type": "string",
419                            "description": "One sentence. For true, name the pieces you can see, so somebody can check you before two more agent calls are spent on it."
420                        }
421                    },
422                    "required": ["item", "split", "reason"]
423                }
424            }
425        },
426        "required": ["items"]
427    })
428}
429
430/// One agent decomposing one issue or one pull request.
431///
432/// The descriptions carry the two rules the command rests on: a part that
433/// cannot stand on its own is not a part, and the whole value of splitting is
434/// that each piece can be reviewed and merged by itself.
435pub fn split_proposal() -> Value {
436    json!({
437        "type": "object",
438        "additionalProperties": false,
439        "properties": {
440            "should_split": {
441                "type": "boolean",
442                "description": "False when this is one piece of work, however large. A rename across forty files is one change; three unrelated fixes in one file are three. False is a fine answer and costs one person one read of something that stays as it was."
443            },
444            "reason": {
445                "type": "string",
446                "description": "One or two sentences on why it should or should not be split. For false this is the whole argument, so give the reason rather than the verdict again."
447            },
448            "stacked": {
449                "type": "boolean",
450                "description": "True only when the parts are genuinely sequential, so each needs its predecessor to make sense. Stacked parts are created in order, each branched off the one before. False means each part stands on the base branch and can be reviewed and merged in any order, which is what makes the review loop cheaper, so prefer it wherever the change allows."
451            },
452            "parts": {
453                "type": "array",
454                "items": {
455                    "type": "object",
456                    "additionalProperties": false,
457                    "properties": {
458                        "title": {
459                            "type": "string",
460                            "description": "Under 80 characters. States the piece of work, not the fact that it was split out."
461                        },
462                        "body": {
463                            "type": "string",
464                            "description": "For an issue, the body of the issue this part becomes, written for somebody picking it up cold with no sight of the parent: what is wrong, how to see it, what it should do instead. For a pull request slice, what this slice is and why it stands on its own. Either way, no preamble and no restating the title."
465                        },
466                        "files": {
467                            "type": ["array", "null"],
468                            "items": {"type": "string"},
469                            "description": "Only when splitting a pull request, null when splitting an issue. The paths from the change that this slice carries, copied exactly from the list you were given. Every path belongs to at most one part, and a part whose files cannot build and pass on their own is not a part: fold it into another or leave it out."
470                        }
471                    },
472                    "required": ["title", "body", "files"]
473                }
474            }
475        },
476        "required": ["should_split", "reason", "stacked", "parts"]
477    })
478}
479
480/// The second agent ruling on the first one's decomposition.
481///
482/// Not a second proposal. Two decompositions of one thing cannot be reconciled
483/// mechanically, so this agent rules on the one in front of it: accept, reject,
484/// or accept with named parts struck.
485pub fn split_check() -> Value {
486    json!({
487        "type": "object",
488        "additionalProperties": false,
489        "properties": {
490            "accept": {
491                "type": "boolean",
492                "description": "True only if you read the code and this decomposition is right. Do not defer to the other agent and do not agree to be agreeable. Getting a rejection wrong costs one person one read of something that stays as it was; getting an acceptance wrong costs them issues to close, a checklist to strip out of somebody's body, and branches and pull requests to delete."
493            },
494            "stacked": {
495                "type": "boolean",
496                "description": "Your own view of whether the parts are sequential, whatever the other agent said. True means each part is branched off the one before it."
497            },
498            "strike": {
499                "type": "array",
500                "items": {"type": "integer"},
501                "description": "The numbers of any parts, from 1, that should not be split out: a part that cannot stand on its own, one that is really the same work as another, one that is too small to be worth its own review. Empty to accept the parts as proposed. Striking so many that fewer than two remain means nothing is split, which is the right answer when that is what you think."
502            },
503            "reasoning": {
504                "type": "string",
505                "description": "One or two sentences saying what you checked and what it showed. Required when you reject or strike anything, and useful when you accept."
506            }
507        },
508        "required": ["accept", "stacked", "strike", "reasoning"]
509    })
510}
511
512/// One agent judging the comments other people left on a pull request.
513///
514/// The descriptions carry the asymmetry the whole command rests on, because
515/// they travel with the request rather than sitting a thousand tokens back in
516/// the prompt: getting a decline wrong costs a person one read of a thread that
517/// stays open for them, and getting an implement wrong costs them a commit they
518/// did not ask for on a branch they own.
519pub fn checkin() -> Value {
520    json!({
521        "type": "object",
522        "additionalProperties": false,
523        "properties": {
524            "verdicts": {
525                "type": "array",
526                "items": {
527                    "type": "object",
528                    "additionalProperties": false,
529                    "properties": {
530                        "ref_id": {
531                            "type": "string",
532                            "description": "The handle printed beside the comment, copied across exactly, so your answer can be matched back to it."
533                        },
534                        "ask": {
535                            "type": "string",
536                            "enum": ["implement", "defer", "decline", "answer", "nothing"],
537                            "description": "implement: the change is right and belongs on this branch. defer: the change is right and is really its own piece of work, so supply new_issue_title and new_issue_body. decline: the change should not be made. answer: a question somebody asked, which gets a reply. nothing: nothing is being asked for, such as praise, an approval, a review that found nothing to change, or a bot reporting a build; nothing is said in reply."
538                        },
539                        "request": {
540                            "type": "string",
541                            "description": "What is being asked for, in one sentence, in your own words. This is how the harness checks the comment was understood before acting on it, so restate the request rather than the comment."
542                        },
543                        "reasoning": {
544                            "type": "string",
545                            "description": "One or two sentences. For decline this is the whole argument and it is posted in the thread, so give the reason and write it for the person who raised the point, not for this harness."
546                        },
547                        "unambiguous": {
548                            "type": "boolean",
549                            "description": "False if the comment could be read more than one way, or if you are guessing at what it wants. False costs a reply asking what was meant; a wrong true costs somebody a commit on their branch that they did not ask for."
550                        },
551                        "new_issue_title": {
552                            "type": ["string", "null"],
553                            "description": "Only when ask is defer, null otherwise. States the defect, not the fix."
554                        },
555                        "new_issue_body": {
556                            "type": ["string", "null"],
557                            "description": "Only when ask is defer, null otherwise. Written for somebody picking it up cold months later, not for whoever is reading this thread today."
558                        }
559                    },
560                    "required": ["ref_id", "ask", "request", "reasoning", "unambiguous", "new_issue_title", "new_issue_body"]
561                }
562            }
563        },
564        "required": ["verdicts"]
565    })
566}
567
568/// The second agent ruling on the first one's calls.
569pub fn checkin_check() -> Value {
570    json!({
571        "type": "object",
572        "additionalProperties": false,
573        "properties": {
574            "checks": {
575                "type": "array",
576                "items": {
577                    "type": "object",
578                    "additionalProperties": false,
579                    "properties": {
580                        "ref_id": {
581                            "type": "string",
582                            "description": "The handle printed beside the comment, copied across exactly."
583                        },
584                        "agrees": {
585                            "type": "boolean",
586                            "description": "True only if you went to the code and confirmed the call. Do not defer to the other agent and do not agree to be agreeable: a decision you cannot confirm is one that is about to put a commit on somebody's branch in their name."
587                        },
588                        "ask": {
589                            "type": "string",
590                            "enum": ["implement", "defer", "decline", "answer", "nothing"],
591                            "description": "What you would do instead. Read only when agrees is false."
592                        },
593                        "unambiguous": {
594                            "type": "boolean",
595                            "description": "False if the comment could be read more than one way, whatever the other agent said about it."
596                        },
597                        "reasoning": {
598                            "type": "string",
599                            "description": "One or two sentences saying what you checked and what it showed. Required when you disagree, and useful when you agree."
600                        }
601                    },
602                    "required": ["ref_id", "agrees", "ask", "unambiguous", "reasoning"]
603                }
604            }
605        },
606        "required": ["checks"]
607    })
608}
609
610/// What the fix pass reports back about each change it was asked to make.
611pub fn checkin_fix() -> Value {
612    json!({
613        "type": "object",
614        "additionalProperties": false,
615        "properties": {
616            "done": {
617                "type": "array",
618                "items": {
619                    "type": "object",
620                    "additionalProperties": false,
621                    "properties": {
622                        "ref_id": {
623                            "type": "string",
624                            "description": "The handle printed beside the comment, copied across exactly."
625                        },
626                        "changed": {
627                            "type": "boolean",
628                            "description": "False if the change turned out to be wrong once you were in the code. You are not obliged to make a change you now believe is a mistake, and saying so is a better answer than making it."
629                        },
630                        "summary": {
631                            "type": "string",
632                            "description": "One sentence naming what changed, or why it was left alone. This is posted in the thread the comment sits in, so write it for the person who asked rather than for this harness."
633                        },
634                        "files": {
635                            "type": "array",
636                            "items": {"type": "string"},
637                            "description": "Repository relative paths this comment's change touched, empty when nothing was changed. Checked against the diff: a comment whose files are not in it is answered in words rather than reported as fixed."
638                        }
639                    },
640                    "required": ["ref_id", "changed", "summary", "files"]
641                }
642            }
643        },
644        "required": ["done"]
645    })
646}
647
648/// A reviewer's report on fixing its own findings.
649///
650/// Same shape as a response, minus the parts that only make sense from the
651/// author's side: a reviewer fixing its own point cannot refute it, and filing
652/// it as somebody else's issue is a decision for the round rather than for this
653/// call.
654pub fn own_fix() -> Value {
655    json!({
656        "type": "object",
657        "additionalProperties": false,
658        "properties": {
659            "summary": {
660                "type": "string",
661                "description": "One sentence naming what changed, at most 200 characters. This becomes the commit subject, so write it as one."
662            },
663            "fixes": {
664                "type": "array",
665                "items": {
666                    "type": "object",
667                    "additionalProperties": false,
668                    "properties": {
669                        "title": {
670                            "type": "string",
671                            "description": "Copy your own finding title exactly, so the two can be matched up."
672                        },
673                        "file": {
674                            "type": "string",
675                            "description": "Copy the file from that finding exactly. Empty string if it had none."
676                        },
677                        "fixed": {
678                            "type": "boolean",
679                            "description": "True only if this point is answered by a change in the working tree. False if you left it, whatever the reason."
680                        },
681                        "reason": {
682                            "type": "string",
683                            "description": "For a fix, what you changed and how it answers the point. For one you left, why, in a sentence the author will read: it stays open and goes to them."
684                        }
685                    },
686                    "required": ["title", "file", "fixed", "reason"]
687                }
688            }
689        },
690        "required": ["summary", "fixes"]
691    })
692}
693
694/// One idea's shape, shared by the diverge answer and a cross round's build.
695///
696/// `combines` is required and described at length because it is the whole
697/// method: an idea that cannot name the existing pieces it pairs is a wish.
698fn idea() -> Value {
699    json!({
700        "type": "object",
701        "additionalProperties": false,
702        "properties": {
703            "title": {
704                "type": "string",
705                "description": "One line naming the idea by what it does, not by its ingredients. It becomes a heading, and may become an issue title."
706            },
707            "combines": {
708                "type": "array",
709                "items": {"type": "string"},
710                "description": "The existing techniques, primitives, results, tools, or patterns this idea pairs, one per entry, each named specifically enough to look up. At least two. This is the whole method: the pieces exist already and the pairing is what is new."
711            },
712            "how_it_works": {
713                "type": "string",
714                "description": "The mechanism, concretely enough that somebody could start on it. A paragraph, not a pitch."
715            },
716            "why_new": {
717                "type": "string",
718                "description": "The nearest thing that already exists, named, and what this does that it does not. If you cannot name a nearest thing, look harder before claiming there is none."
719            },
720            "enables": {
721                "type": "string",
722                "description": "What becomes possible that was not, in one or two sentences."
723            },
724            "risks": {
725                "type": "string",
726                "description": "What would make it fail, honestly. The reader decides whether to try it on this."
727            },
728            "first_experiment": {
729                "type": "string",
730                "description": "The cheapest thing that would show whether it works: a prototype, a calculation, a query, a conversation with somebody who would know."
731            }
732        },
733        "required": ["title", "combines", "how_it_works", "why_new", "enables", "risks", "first_experiment"]
734    })
735}
736
737pub fn brainstorm_ideas() -> Value {
738    json!({
739        "type": "object",
740        "additionalProperties": false,
741        "properties": {
742            "ideas": {
743                "type": "array",
744                "items": idea(),
745                "description": "The ideas, best first. Each pairs existing pieces in a way that has not been done."
746            }
747        },
748        "required": ["ideas"]
749    })
750}
751
752/// A cross round: one agent ruling on ideas the other proposed.
753///
754/// Matched back by number rather than title, for the reason the screen schema
755/// carries an entry number: a model rewrites titles.
756pub fn brainstorm_cross() -> Value {
757    let mut build = idea();
758    build["type"] = json!(["object", "null"]);
759    build["description"] = json!("The revised idea in full when the verdict is build, naming everything it combines including the new piece. Null for any other verdict.");
760    json!({
761        "type": "object",
762        "additionalProperties": false,
763        "properties": {
764            "verdicts": {
765                "type": "array",
766                "items": {
767                    "type": "object",
768                    "additionalProperties": false,
769                    "properties": {
770                        "idea": {
771                            "type": "integer",
772                            "description": "The number the idea was given in the list above. Copy it exactly, so the ruling can be matched back to the idea it is about."
773                        },
774                        "verdict": {
775                            "type": "string",
776                            "enum": ["build", "challenge", "keep"],
777                            "description": "build: you can see a further piece that completes it or makes it stronger, and you have written the revised idea in build. challenge: it already exists and you can name where, or it cannot work and you can say why. keep: real and new as far as you can tell, nothing to add."
778                        },
779                        "reason": {
780                            "type": "string",
781                            "description": "For challenge, the thing that already exists or the reason it cannot work, specifically: the proposer answers this with specifics. For build, what the new piece adds. For keep, one sentence or empty."
782                        },
783                        "build": build
784                    },
785                    "required": ["idea", "verdict", "reason", "build"]
786                }
787            }
788        },
789        "required": ["verdicts"]
790    })
791}
792
793/// The last round: defend or withdraw what was challenged, and rank the rest.
794pub fn brainstorm_converge() -> Value {
795    json!({
796        "type": "object",
797        "additionalProperties": false,
798        "properties": {
799            "defences": {
800                "type": "array",
801                "items": {
802                    "type": "object",
803                    "additionalProperties": false,
804                    "properties": {
805                        "idea": {
806                            "type": "integer",
807                            "description": "The number of an idea you proposed that was challenged. One entry per challenged idea of yours, and none for anything else."
808                        },
809                        "stands": {
810                            "type": "boolean",
811                            "description": "True only if you can give the specific evidence that settles the objection in reply. False withdraws the idea, which is the right answer when the objection is correct."
812                        },
813                        "reply": {
814                            "type": "string",
815                            "description": "The evidence when it stands: the prior art that does not actually do this, the mechanism the objection missed, the number that makes it work. One sentence when it does not."
816                        }
817                    },
818                    "required": ["idea", "stands", "reply"]
819                }
820            },
821            "ranking": {
822                "type": "array",
823                "items": {"type": "integer"},
824                "description": "Every idea number that is not withdrawn, best first, including your own and the ones you challenged. Novelty and feasibility together: an idea nobody has done that could be tried this month beats a moonshot, and a moonshot beats a small improvement on something that exists."
825            }
826        },
827        "required": ["defences", "ranking"]
828    })
829}
830
831pub fn all() -> Vec<(&'static str, Value)> {
832    vec![
833        ("code_issue", code_issue()),
834        ("triage", triage()),
835        ("implementation", implementation()),
836        ("review", review()),
837        ("recheck", recheck()),
838        ("response", response()),
839        ("own_fix", own_fix()),
840        ("screen", screen()),
841        ("checkin", checkin()),
842        ("checkin_check", checkin_check()),
843        ("checkin_fix", checkin_fix()),
844        ("split_screen", split_screen()),
845        ("split_proposal", split_proposal()),
846        ("split_check", split_check()),
847        ("brainstorm_ideas", brainstorm_ideas()),
848        ("brainstorm_cross", brainstorm_cross()),
849        ("brainstorm_converge", brainstorm_converge()),
850    ]
851}
852
853#[cfg(test)]
854mod tests {
855    use super::*;
856
857    /// Yield every object schema, however deeply nested.
858    fn objects(node: &Value, path: String, out: &mut Vec<(String, Value)>) {
859        if let Some(map) = node.as_object() {
860            if map.get("type").and_then(Value::as_str) == Some("object")
861                && map.contains_key("properties")
862            {
863                out.push((path.clone(), node.clone()));
864                if let Some(props) = map.get("properties").and_then(Value::as_object) {
865                    for (key, child) in props {
866                        objects(child, format!("{path}.{key}"), out);
867                    }
868                }
869            }
870            if let Some(items) = map.get("items") {
871                objects(items, format!("{path}[]"), out);
872            }
873        }
874    }
875
876    fn walk(name: &str, schema: &Value) -> Vec<(String, Value)> {
877        let mut out = Vec::new();
878        objects(schema, name.to_string(), &mut out);
879        out
880    }
881
882    /// Strict structured output rejects any property that is not also in
883    /// `required`. The Python original violated this in the response schema
884    /// from the start and nothing caught it, because the response schema is
885    /// only reached when a review is handed back with blocking findings, and
886    /// almost every run approved in round one.
887    #[test]
888    fn every_property_is_required() {
889        for (name, schema) in all() {
890            for (path, node) in walk(name, &schema) {
891                let props: Vec<&String> = node["properties"].as_object().unwrap().keys().collect();
892                let required: Vec<String> = node["required"]
893                    .as_array()
894                    .unwrap_or(&vec![])
895                    .iter()
896                    .filter_map(|v| v.as_str().map(str::to_string))
897                    .collect();
898                for prop in &props {
899                    assert!(
900                        required.contains(prop),
901                        "{path}: {prop} is in properties but not in required. \
902                         Make optional fields nullable instead."
903                    );
904                }
905                assert_eq!(props.len(), required.len(), "{path}: required has extras");
906            }
907        }
908    }
909
910    /// The guard that was missing. A schema field can be added to the struct
911    /// and forgotten in the schema, and every test still passes: the tests
912    /// build the struct in Rust, so they never notice the model was never
913    /// asked. That shipped once, as four bug-report fields the agents were
914    /// never told about, which quietly did nothing.
915    #[test]
916    fn the_review_schema_asks_for_every_field_a_finding_holds() {
917        use crate::model::Finding;
918
919        let asked: Vec<String> = review()["properties"]["findings"]["items"]["properties"]
920            .as_object()
921            .expect("finding properties")
922            .keys()
923            .cloned()
924            .collect();
925
926        // Round-tripping a fully populated Finding names every field serde
927        // knows about, without repeating the list here to drift out of date.
928        let populated = Finding {
929            problem: Some("p".into()),
930            reproduction: Some("r".into()),
931            impact: Some("i".into()),
932            expected: Some("e".into()),
933            ..Finding::default()
934        };
935        let held: Vec<String> = serde_json::to_value(&populated)
936            .expect("serialisable")
937            .as_object()
938            .expect("object")
939            .keys()
940            .cloned()
941            .collect();
942
943        for field in &held {
944            assert!(
945                asked.contains(field),
946                "a Finding holds `{field}` and the schema never asks for it, so the model will \
947                 not fill it and the code reading it will always see nothing"
948            );
949        }
950    }
951
952    /// The same guard for the second look: its findings are `review()`'s
953    /// items, and its rulings have to ask for every field a ruling holds.
954    #[test]
955    fn the_recheck_schema_asks_for_every_field_a_finding_and_a_ruling_hold() {
956        use crate::model::{RecheckRuling, Ruling};
957
958        assert_eq!(
959            review()["properties"]["findings"]["items"],
960            recheck()["properties"]["findings"]["items"],
961            "a second look's findings are first look findings"
962        );
963
964        let asked: Vec<String> = recheck()["properties"]["rulings"]["items"]["properties"]
965            .as_object()
966            .expect("ruling properties")
967            .keys()
968            .cloned()
969            .collect();
970        let held: Vec<String> = serde_json::to_value(RecheckRuling {
971            index: 1,
972            outcome: Ruling::Open,
973            reasoning: "r".into(),
974        })
975        .expect("serialisable")
976        .as_object()
977        .expect("object")
978        .keys()
979        .cloned()
980        .collect();
981        for field in &held {
982            assert!(
983                asked.contains(field),
984                "a ruling holds `{field}`, unasked for"
985            );
986        }
987    }
988
989    /// Every outcome the schema offers is one the parser reads back.
990    #[test]
991    fn the_recheck_outcome_enum_matches_the_parser() {
992        use crate::model::Ruling;
993
994        let offered = recheck()["properties"]["rulings"]["items"]["properties"]["outcome"]["enum"]
995            .as_array()
996            .expect("an enum")
997            .clone();
998        assert_eq!(3, offered.len());
999        for value in offered {
1000            let word = value.as_str().expect("a word");
1001            assert!(
1002                Ruling::parse_lenient(word).is_some(),
1003                "{word} is offered and cannot be read back"
1004            );
1005        }
1006    }
1007
1008    /// The same guard for the check-in exchange. A field added to the struct
1009    /// and forgotten in the schema is one the model is never asked for, so the
1010    /// code reading it always sees nothing: an `unambiguous` that is never
1011    /// filled would default false and answer every comment in words.
1012    #[test]
1013    fn the_checkin_schema_asks_for_every_field_a_verdict_holds() {
1014        use crate::model::{Ask, CommentVerdict};
1015
1016        let asked: Vec<String> = checkin()["properties"]["verdicts"]["items"]["properties"]
1017            .as_object()
1018            .expect("verdict properties")
1019            .keys()
1020            .cloned()
1021            .collect();
1022
1023        let populated = CommentVerdict {
1024            ref_id: "c1".into(),
1025            ask: Ask::Decline,
1026            request: "r".into(),
1027            reasoning: "w".into(),
1028            unambiguous: true,
1029            new_issue_title: Some("t".into()),
1030            new_issue_body: Some("b".into()),
1031        };
1032        let held: Vec<String> = serde_json::to_value(&populated)
1033            .expect("serialisable")
1034            .as_object()
1035            .expect("object")
1036            .keys()
1037            .cloned()
1038            .collect();
1039
1040        for field in &held {
1041            assert!(
1042                asked.contains(field),
1043                "a CommentVerdict holds `{field}` and the schema never asks for it, so the model \
1044                 will not fill it and the code reading it will always see nothing"
1045            );
1046        }
1047    }
1048
1049    /// The same guard for the split exchange. A part field added to the struct
1050    /// and forgotten here is one the model is never asked for, so a slice would
1051    /// arrive with no files and carry nothing.
1052    #[test]
1053    fn the_split_schemas_ask_for_every_field_they_hold() {
1054        use crate::model::{SplitCheck, SplitPart, SplitProposal};
1055
1056        let cases: Vec<(&str, Value, serde_json::Value)> = vec![
1057            (
1058                "SplitProposal",
1059                split_proposal()["properties"].clone(),
1060                serde_json::to_value(SplitProposal::default()).expect("serialisable"),
1061            ),
1062            (
1063                "SplitPart",
1064                split_proposal()["properties"]["parts"]["items"]["properties"].clone(),
1065                serde_json::to_value(SplitPart::default()).expect("serialisable"),
1066            ),
1067            (
1068                "SplitCheck",
1069                split_check()["properties"].clone(),
1070                serde_json::to_value(SplitCheck::default()).expect("serialisable"),
1071            ),
1072        ];
1073        for (what, asked, held) in cases {
1074            let asked: Vec<String> = asked
1075                .as_object()
1076                .expect("properties")
1077                .keys()
1078                .cloned()
1079                .collect();
1080            for field in held.as_object().expect("object").keys() {
1081                assert!(
1082                    asked.contains(field),
1083                    "a {what} holds `{field}` and the schema never asks for it, so the code \
1084                     reading it will always see nothing"
1085                );
1086            }
1087        }
1088    }
1089
1090    /// A slice with no files carries nothing, so the one field that decides
1091    /// what a part is has to be spelled as nullable rather than left out.
1092    #[test]
1093    fn a_split_part_may_carry_no_files_without_omitting_the_field() {
1094        let types = split_proposal()["properties"]["parts"]["items"]["properties"]["files"]["type"]
1095            .to_string();
1096        assert!(types.contains("null"), "files must accept null: {types}");
1097    }
1098
1099    /// Every value the schema offers has to be one the parser accepts, or an
1100    /// answer that matched the schema exactly is thrown away.
1101    #[test]
1102    fn the_checkin_ask_enum_matches_the_parser() {
1103        use crate::model::Ask;
1104
1105        for schema in [
1106            checkin()["properties"]["verdicts"]["items"]["properties"]["ask"].clone(),
1107            checkin_check()["properties"]["checks"]["items"]["properties"]["ask"].clone(),
1108        ] {
1109            for value in schema["enum"].as_array().expect("an enum") {
1110                let text = value.as_str().expect("a string");
1111                assert!(
1112                    Ask::parse_lenient(text).is_some(),
1113                    "the schema offers `{text}` and the parser refuses it"
1114                );
1115            }
1116        }
1117    }
1118
1119    /// The same guard for the implementation exchange, where a forgotten field
1120    /// means a pull request body with an empty section in it and nobody the
1121    /// wiser.
1122    #[test]
1123    fn the_implementation_schema_asks_for_every_field_it_holds() {
1124        use crate::model::Implementation;
1125
1126        let asked: Vec<String> = implementation()["properties"]
1127            .as_object()
1128            .expect("properties")
1129            .keys()
1130            .cloned()
1131            .collect();
1132
1133        let populated = Implementation {
1134            notes: Some("n".into()),
1135            ..Implementation::default()
1136        };
1137        let held: Vec<String> = serde_json::to_value(&populated)
1138            .expect("serialisable")
1139            .as_object()
1140            .expect("object")
1141            .keys()
1142            .cloned()
1143            .collect();
1144
1145        for field in &held {
1146            assert!(
1147                asked.contains(field),
1148                "an Implementation holds `{field}` and the schema never asks for it, so the \
1149                 pull request body will always be missing that part"
1150            );
1151        }
1152    }
1153
1154    /// Same guard for the other direction of the same exchange.
1155    #[test]
1156    fn the_response_schema_asks_for_every_field_a_disposition_holds() {
1157        use crate::model::{Action, Disposition};
1158
1159        let asked: Vec<String> = response()["properties"]["dispositions"]["items"]["properties"]
1160            .as_object()
1161            .expect("disposition properties")
1162            .keys()
1163            .cloned()
1164            .collect();
1165
1166        let populated = Disposition {
1167            title: "t".into(),
1168            file: "f".into(),
1169            action: Action::Fixed,
1170            reasoning: "r".into(),
1171            new_issue_title: Some("t".into()),
1172            new_issue_body: Some("b".into()),
1173        };
1174        let held: Vec<String> = serde_json::to_value(&populated)
1175            .expect("serialisable")
1176            .as_object()
1177            .expect("object")
1178            .keys()
1179            .cloned()
1180            .collect();
1181
1182        for field in &held {
1183            assert!(
1184                asked.contains(field),
1185                "a Disposition holds `{field}`, unasked for"
1186            );
1187        }
1188    }
1189
1190    #[test]
1191    fn objects_forbid_additional_properties() {
1192        for (name, schema) in all() {
1193            for (path, node) in walk(name, &schema) {
1194                assert_eq!(
1195                    Some(false),
1196                    node["additionalProperties"].as_bool(),
1197                    "{path} allows additional properties"
1198                );
1199            }
1200        }
1201    }
1202
1203    #[test]
1204    fn optional_fields_are_spelled_as_nullable() {
1205        let item = &response()["properties"]["dispositions"]["items"];
1206        for field in ["new_issue_title", "new_issue_body"] {
1207            let types = item["properties"][field]["type"].to_string();
1208            assert!(types.contains("null"), "{field} must accept null: {types}");
1209        }
1210    }
1211
1212    /// The re-litigation guard hashes a refutation by title *and* file. If the
1213    /// disposition cannot carry the file, the key it records can never match
1214    /// the key the next round's finding hashes to, and the guard is dead code.
1215    #[test]
1216    fn a_disposition_carries_the_file_so_the_ledger_key_can_match() {
1217        let props = response()["properties"]["dispositions"]["items"]["properties"].clone();
1218        assert!(
1219            props.get("file").is_some(),
1220            "dispositions must carry a file"
1221        );
1222    }
1223
1224    #[test]
1225    fn severity_and_verdict_enums_match_the_parser() {
1226        use crate::model::{Severity, Verdict};
1227        let sev =
1228            review()["properties"]["findings"]["items"]["properties"]["severity"]["enum"].clone();
1229        for value in sev.as_array().unwrap() {
1230            assert!(
1231                Severity::parse_lenient(value.as_str().unwrap()).is_some(),
1232                "schema offers {value} but the parser rejects it"
1233            );
1234        }
1235        let verdicts = review()["properties"]["verdict"]["enum"].clone();
1236        for value in verdicts.as_array().unwrap() {
1237            assert!(Verdict::parse_lenient(value.as_str().unwrap()).is_some());
1238        }
1239    }
1240
1241    #[test]
1242    fn triage_enums_match_the_parser() {
1243        use crate::model::{Complexity, Risk};
1244        let item = &triage()["properties"]["issues"]["items"]["properties"];
1245        for value in item["complexity"]["enum"].as_array().unwrap() {
1246            assert!(Complexity::parse_lenient(value.as_str().unwrap()).is_some());
1247        }
1248        for value in item["risk"]["enum"].as_array().unwrap() {
1249            assert!(Risk::parse_lenient(value.as_str().unwrap()).is_some());
1250        }
1251    }
1252
1253    #[test]
1254    fn response_action_enum_matches_the_parser() {
1255        use crate::model::Action;
1256        let actions = response()["properties"]["dispositions"]["items"]["properties"]["action"]
1257            ["enum"]
1258            .clone();
1259        for value in actions.as_array().unwrap() {
1260            assert!(Action::parse_lenient(value.as_str().unwrap()).is_some());
1261        }
1262    }
1263}