1use 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, changes that would make the codebase worse, and tracking issues. Set tracker as well for the last of those."
30 },
31 "tracker": {
32 "type": "boolean",
33 "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."
34 },
35 "reason": {
36 "type": "string",
37 "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."
38 },
39 "complexity": {"type": "string", "enum": ["s", "m", "l"]},
40 "depends_on": {
41 "type": "array",
42 "items": {"type": "integer"},
43 "description": "Issue numbers from this same list that should land first. Empty if none."
44 },
45 "risk": {"type": "string", "enum": ["low", "med", "high"]}
46 },
47 "required": ["issue", "worth_doing", "tracker", "reason", "complexity", "depends_on", "risk"]
48 }
49 }
50 },
51 "required": ["issues"]
52 })
53}
54
55pub fn review() -> Value {
56 json!({
57 "type": "object",
58 "additionalProperties": false,
59 "properties": {
60 "verdict": {"type": "string", "enum": ["approve", "changes_requested"]},
61 "next_action": {"type": "string", "enum": ["merge", "fix_myself", "hand_back"]},
62 "summary": {
63 "type": "string",
64 "description": "One sentence, at most 200 characters. No preamble, no restating the diff."
65 },
66 "findings": {
67 "type": "array",
68 "items": {
69 "type": "object",
70 "additionalProperties": false,
71 "properties": {
72 "severity": {
73 "type": "string",
74 "enum": ["blocking", "non-blocking", "nit"],
75 "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."
76 },
77 "title": {
78 "type": "string",
79 "description": "Under 80 characters. State the defect, not the fix."
80 },
81 "detail": {
82 "type": "string",
83 "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."
84 },
85 "file": {
86 "type": "string",
87 "description": "Path, with a line number if you have one. Empty string if the finding is general."
88 },
89 "problem": {
90 "type": ["string", "null"],
91 "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."
92 },
93 "reproduction": {
94 "type": ["string", "null"],
95 "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."
96 },
97 "impact": {
98 "type": ["string", "null"],
99 "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."
100 },
101 "expected": {
102 "type": ["string", "null"],
103 "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."
104 },
105 "in_scope": {
106 "type": "boolean",
107 "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, leave this true and say your piece in the finding."
108 }
109 },
110 "required": [
111 "severity",
112 "title",
113 "detail",
114 "file",
115 "in_scope",
116 "problem",
117 "reproduction",
118 "impact",
119 "expected"
120 ]
121 }
122 }
123 },
124 "required": ["verdict", "next_action", "summary", "findings"]
125 })
126}
127
128pub fn implementation() -> Value {
136 json!({
137 "type": "object",
138 "additionalProperties": false,
139 "properties": {
140 "not_worth_doing": {
141 "type": "boolean",
142 "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 commits when this is true."
143 },
144 "reason": {
145 "type": "string",
146 "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."
147 },
148 "summary": {
149 "type": "string",
150 "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."
151 },
152 "problem": {
153 "type": "string",
154 "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."
155 },
156 "changes": {
157 "type": "array",
158 "items": {"type": "string"},
159 "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."
160 },
161 "testing": {
162 "type": "array",
163 "items": {"type": "string"},
164 "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."
165 },
166 "notes": {
167 "type": ["string", "null"],
168 "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."
169 }
170 },
171 "required": ["not_worth_doing", "reason", "summary", "problem", "changes", "testing", "notes"]
172 })
173}
174
175pub fn response() -> Value {
176 json!({
177 "type": "object",
178 "additionalProperties": false,
179 "properties": {
180 "summary": {
181 "type": "string",
182 "description": "One sentence, at most 200 characters."
183 },
184 "dispositions": {
185 "type": "array",
186 "items": {
187 "type": "object",
188 "additionalProperties": false,
189 "properties": {
190 "title": {
191 "type": "string",
192 "description": "Copy the reviewer's finding title exactly, so the two can be matched up."
193 },
194 "file": {
195 "type": "string",
196 "description": "Copy the reviewer's file for this finding exactly. Empty string if it had none."
197 },
198 "action": {
199 "type": "string",
200 "enum": ["fixed", "refuted", "filed_issue"],
201 "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."
202 },
203 "reasoning": {
204 "type": "string",
205 "description": "One or two sentences. For a refutation this is the whole argument, so make it the reason and not an apology."
206 },
207 "new_issue_title": {
208 "type": ["string", "null"],
209 "description": "Only for filed_issue, null otherwise."
210 },
211 "new_issue_body": {
212 "type": ["string", "null"],
213 "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."
214 }
215 },
216 "required": ["title", "file", "action", "reasoning", "new_issue_title", "new_issue_body"]
217 }
218 }
219 },
220 "required": ["summary", "dispositions"]
221 })
222}
223
224pub fn adjudication() -> Value {
231 json!({
232 "type": "object",
233 "additionalProperties": false,
234 "properties": {
235 "verdicts": {
236 "type": "array",
237 "items": {
238 "type": "object",
239 "additionalProperties": false,
240 "properties": {
241 "title": {
242 "type": "string",
243 "description": "Copy the finding's title exactly, so it can be matched up."
244 },
245 "file": {
246 "type": "string",
247 "description": "Copy the finding's file exactly. Empty string if it had none."
248 },
249 "agrees": {
250 "type": "boolean",
251 "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."
252 },
253 "severity": {
254 "type": "string",
255 "enum": ["blocking", "non-blocking", "nit"],
256 "description": "Your own view of how badly it matters, even where you agree the defect is real."
257 },
258 "reasoning": {
259 "type": "string",
260 "description": "One or two sentences. If you disagree, this is the whole argument, so give the reason rather than an opinion."
261 }
262 },
263 "required": ["title", "file", "agrees", "severity", "reasoning"]
264 }
265 }
266 },
267 "required": ["verdicts"]
268 })
269}
270
271pub fn screen() -> Value {
278 json!({
279 "type": "object",
280 "additionalProperties": false,
281 "properties": {
282 "entries": {
283 "type": "array",
284 "items": {
285 "type": "object",
286 "additionalProperties": false,
287 "properties": {
288 "entry": {
289 "type": "integer",
290 "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."
291 },
292 "verdict": {
293 "type": "string",
294 "enum": ["still_relevant", "already_fixed", "not_worth_it", "duplicate"],
295 "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."
296 },
297 "title": {
298 "type": "string",
299 "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."
300 },
301 "reason": {
302 "type": "string",
303 "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."
304 },
305 "duplicate_of": {
306 "type": ["integer", "null"],
307 "description": "Only when the verdict is duplicate, null otherwise. An open issue number, or the number of an earlier entry in this same list."
308 }
309 },
310 "required": ["entry", "verdict", "title", "reason", "duplicate_of"]
311 }
312 }
313 },
314 "required": ["entries"]
315 })
316}
317
318pub fn checkin() -> Value {
326 json!({
327 "type": "object",
328 "additionalProperties": false,
329 "properties": {
330 "verdicts": {
331 "type": "array",
332 "items": {
333 "type": "object",
334 "additionalProperties": false,
335 "properties": {
336 "ref_id": {
337 "type": "string",
338 "description": "The handle printed beside the comment, copied across exactly, so your answer can be matched back to it."
339 },
340 "ask": {
341 "type": "string",
342 "enum": ["implement", "defer", "decline", "answer", "nothing"],
343 "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 rather than a request. nothing: nothing is being asked for."
344 },
345 "request": {
346 "type": "string",
347 "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."
348 },
349 "reasoning": {
350 "type": "string",
351 "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."
352 },
353 "unambiguous": {
354 "type": "boolean",
355 "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."
356 },
357 "new_issue_title": {
358 "type": ["string", "null"],
359 "description": "Only when ask is defer, null otherwise. States the defect, not the fix."
360 },
361 "new_issue_body": {
362 "type": ["string", "null"],
363 "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."
364 }
365 },
366 "required": ["ref_id", "ask", "request", "reasoning", "unambiguous", "new_issue_title", "new_issue_body"]
367 }
368 }
369 },
370 "required": ["verdicts"]
371 })
372}
373
374pub fn checkin_check() -> Value {
376 json!({
377 "type": "object",
378 "additionalProperties": false,
379 "properties": {
380 "checks": {
381 "type": "array",
382 "items": {
383 "type": "object",
384 "additionalProperties": false,
385 "properties": {
386 "ref_id": {
387 "type": "string",
388 "description": "The handle printed beside the comment, copied across exactly."
389 },
390 "agrees": {
391 "type": "boolean",
392 "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."
393 },
394 "ask": {
395 "type": "string",
396 "enum": ["implement", "defer", "decline", "answer", "nothing"],
397 "description": "What you would do instead. Read only when agrees is false."
398 },
399 "unambiguous": {
400 "type": "boolean",
401 "description": "False if the comment could be read more than one way, whatever the other agent said about it."
402 },
403 "reasoning": {
404 "type": "string",
405 "description": "One or two sentences saying what you checked and what it showed. Required when you disagree, and useful when you agree."
406 }
407 },
408 "required": ["ref_id", "agrees", "ask", "unambiguous", "reasoning"]
409 }
410 }
411 },
412 "required": ["checks"]
413 })
414}
415
416pub fn checkin_fix() -> Value {
418 json!({
419 "type": "object",
420 "additionalProperties": false,
421 "properties": {
422 "done": {
423 "type": "array",
424 "items": {
425 "type": "object",
426 "additionalProperties": false,
427 "properties": {
428 "ref_id": {
429 "type": "string",
430 "description": "The handle printed beside the comment, copied across exactly."
431 },
432 "changed": {
433 "type": "boolean",
434 "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."
435 },
436 "summary": {
437 "type": "string",
438 "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."
439 }
440 },
441 "required": ["ref_id", "changed", "summary"]
442 }
443 }
444 },
445 "required": ["done"]
446 })
447}
448
449pub fn all() -> Vec<(&'static str, Value)> {
450 vec![
451 ("triage", triage()),
452 ("implementation", implementation()),
453 ("review", review()),
454 ("response", response()),
455 ("screen", screen()),
456 ("checkin", checkin()),
457 ("checkin_check", checkin_check()),
458 ("checkin_fix", checkin_fix()),
459 ]
460}
461
462#[cfg(test)]
463mod tests {
464 use super::*;
465
466 fn objects(node: &Value, path: String, out: &mut Vec<(String, Value)>) {
468 if let Some(map) = node.as_object() {
469 if map.get("type").and_then(Value::as_str) == Some("object")
470 && map.contains_key("properties")
471 {
472 out.push((path.clone(), node.clone()));
473 if let Some(props) = map.get("properties").and_then(Value::as_object) {
474 for (key, child) in props {
475 objects(child, format!("{path}.{key}"), out);
476 }
477 }
478 }
479 if let Some(items) = map.get("items") {
480 objects(items, format!("{path}[]"), out);
481 }
482 }
483 }
484
485 fn walk(name: &str, schema: &Value) -> Vec<(String, Value)> {
486 let mut out = Vec::new();
487 objects(schema, name.to_string(), &mut out);
488 out
489 }
490
491 #[test]
497 fn every_property_is_required() {
498 for (name, schema) in all() {
499 for (path, node) in walk(name, &schema) {
500 let props: Vec<&String> = node["properties"].as_object().unwrap().keys().collect();
501 let required: Vec<String> = node["required"]
502 .as_array()
503 .unwrap_or(&vec![])
504 .iter()
505 .filter_map(|v| v.as_str().map(str::to_string))
506 .collect();
507 for prop in &props {
508 assert!(
509 required.contains(prop),
510 "{path}: {prop} is in properties but not in required. \
511 Make optional fields nullable instead."
512 );
513 }
514 assert_eq!(props.len(), required.len(), "{path}: required has extras");
515 }
516 }
517 }
518
519 #[test]
525 fn the_review_schema_asks_for_every_field_a_finding_holds() {
526 use crate::model::Finding;
527
528 let asked: Vec<String> = review()["properties"]["findings"]["items"]["properties"]
529 .as_object()
530 .expect("finding properties")
531 .keys()
532 .cloned()
533 .collect();
534
535 let populated = Finding {
538 problem: Some("p".into()),
539 reproduction: Some("r".into()),
540 impact: Some("i".into()),
541 expected: Some("e".into()),
542 ..Finding::default()
543 };
544 let held: Vec<String> = serde_json::to_value(&populated)
545 .expect("serialisable")
546 .as_object()
547 .expect("object")
548 .keys()
549 .cloned()
550 .collect();
551
552 for field in &held {
553 assert!(
554 asked.contains(field),
555 "a Finding holds `{field}` and the schema never asks for it, so the model will \
556 not fill it and the code reading it will always see nothing"
557 );
558 }
559 }
560
561 #[test]
566 fn the_checkin_schema_asks_for_every_field_a_verdict_holds() {
567 use crate::model::{Ask, CommentVerdict};
568
569 let asked: Vec<String> = checkin()["properties"]["verdicts"]["items"]["properties"]
570 .as_object()
571 .expect("verdict properties")
572 .keys()
573 .cloned()
574 .collect();
575
576 let populated = CommentVerdict {
577 ref_id: "c1".into(),
578 ask: Ask::Decline,
579 request: "r".into(),
580 reasoning: "w".into(),
581 unambiguous: true,
582 new_issue_title: Some("t".into()),
583 new_issue_body: Some("b".into()),
584 };
585 let held: Vec<String> = serde_json::to_value(&populated)
586 .expect("serialisable")
587 .as_object()
588 .expect("object")
589 .keys()
590 .cloned()
591 .collect();
592
593 for field in &held {
594 assert!(
595 asked.contains(field),
596 "a CommentVerdict holds `{field}` and the schema never asks for it, so the model \
597 will not fill it and the code reading it will always see nothing"
598 );
599 }
600 }
601
602 #[test]
605 fn the_checkin_ask_enum_matches_the_parser() {
606 use crate::model::Ask;
607
608 for schema in [
609 checkin()["properties"]["verdicts"]["items"]["properties"]["ask"].clone(),
610 checkin_check()["properties"]["checks"]["items"]["properties"]["ask"].clone(),
611 ] {
612 for value in schema["enum"].as_array().expect("an enum") {
613 let text = value.as_str().expect("a string");
614 assert!(
615 Ask::parse_lenient(text).is_some(),
616 "the schema offers `{text}` and the parser refuses it"
617 );
618 }
619 }
620 }
621
622 #[test]
626 fn the_implementation_schema_asks_for_every_field_it_holds() {
627 use crate::model::Implementation;
628
629 let asked: Vec<String> = implementation()["properties"]
630 .as_object()
631 .expect("properties")
632 .keys()
633 .cloned()
634 .collect();
635
636 let populated = Implementation {
637 notes: Some("n".into()),
638 ..Implementation::default()
639 };
640 let held: Vec<String> = serde_json::to_value(&populated)
641 .expect("serialisable")
642 .as_object()
643 .expect("object")
644 .keys()
645 .cloned()
646 .collect();
647
648 for field in &held {
649 assert!(
650 asked.contains(field),
651 "an Implementation holds `{field}` and the schema never asks for it, so the \
652 pull request body will always be missing that part"
653 );
654 }
655 }
656
657 #[test]
659 fn the_response_schema_asks_for_every_field_a_disposition_holds() {
660 use crate::model::{Action, Disposition};
661
662 let asked: Vec<String> = response()["properties"]["dispositions"]["items"]["properties"]
663 .as_object()
664 .expect("disposition properties")
665 .keys()
666 .cloned()
667 .collect();
668
669 let populated = Disposition {
670 title: "t".into(),
671 file: "f".into(),
672 action: Action::Fixed,
673 reasoning: "r".into(),
674 new_issue_title: Some("t".into()),
675 new_issue_body: Some("b".into()),
676 };
677 let held: Vec<String> = serde_json::to_value(&populated)
678 .expect("serialisable")
679 .as_object()
680 .expect("object")
681 .keys()
682 .cloned()
683 .collect();
684
685 for field in &held {
686 assert!(
687 asked.contains(field),
688 "a Disposition holds `{field}`, unasked for"
689 );
690 }
691 }
692
693 #[test]
694 fn objects_forbid_additional_properties() {
695 for (name, schema) in all() {
696 for (path, node) in walk(name, &schema) {
697 assert_eq!(
698 Some(false),
699 node["additionalProperties"].as_bool(),
700 "{path} allows additional properties"
701 );
702 }
703 }
704 }
705
706 #[test]
707 fn optional_fields_are_spelled_as_nullable() {
708 let item = &response()["properties"]["dispositions"]["items"];
709 for field in ["new_issue_title", "new_issue_body"] {
710 let types = item["properties"][field]["type"].to_string();
711 assert!(types.contains("null"), "{field} must accept null: {types}");
712 }
713 }
714
715 #[test]
719 fn a_disposition_carries_the_file_so_the_ledger_key_can_match() {
720 let props = response()["properties"]["dispositions"]["items"]["properties"].clone();
721 assert!(
722 props.get("file").is_some(),
723 "dispositions must carry a file"
724 );
725 }
726
727 #[test]
728 fn severity_and_verdict_enums_match_the_parser() {
729 use crate::model::{Severity, Verdict};
730 let sev =
731 review()["properties"]["findings"]["items"]["properties"]["severity"]["enum"].clone();
732 for value in sev.as_array().unwrap() {
733 assert!(
734 Severity::parse_lenient(value.as_str().unwrap()).is_some(),
735 "schema offers {value} but the parser rejects it"
736 );
737 }
738 let verdicts = review()["properties"]["verdict"]["enum"].clone();
739 for value in verdicts.as_array().unwrap() {
740 assert!(Verdict::parse_lenient(value.as_str().unwrap()).is_some());
741 }
742 }
743
744 #[test]
745 fn triage_enums_match_the_parser() {
746 use crate::model::{Complexity, Risk};
747 let item = &triage()["properties"]["issues"]["items"]["properties"];
748 for value in item["complexity"]["enum"].as_array().unwrap() {
749 assert!(Complexity::parse_lenient(value.as_str().unwrap()).is_some());
750 }
751 for value in item["risk"]["enum"].as_array().unwrap() {
752 assert!(Risk::parse_lenient(value.as_str().unwrap()).is_some());
753 }
754 }
755
756 #[test]
757 fn response_action_enum_matches_the_parser() {
758 use crate::model::Action;
759 let actions = response()["properties"]["dispositions"]["items"]["properties"]["action"]
760 ["enum"]
761 .clone();
762 for value in actions.as_array().unwrap() {
763 assert!(Action::parse_lenient(value.as_str().unwrap()).is_some());
764 }
765 }
766}