pub fn thread_reply(item: &Settled, style: &Style) -> StringExpand description
The reply that goes into one review thread.
Composed from fields rather than forwarding model prose, like everything else spar posts. A decline is the argument and nothing before it: no “I disagree because”, because the reasoning is the reply, and the last line is the one that says whose move it is.
Examples found in repository?
examples/preview.rs (line 337)
38fn main() {
39 let loose = std::env::args().any(|a| a == "--loose");
40 let style = if loose {
41 Style {
42 terse: false,
43 ..Style::default()
44 }
45 } else {
46 Style::default()
47 };
48 if loose {
49 println!("(concision gate OFF: this is what a model would post unedited)");
50 }
51
52 rule("A clean review");
53 println!(
54 "{}",
55 review_comment(
56 "codex",
57 1,
58 &Review {
59 verdict: Verdict::Approve,
60 next_action: NextAction::Merge,
61 summary: "I reviewed the changes on this branch carefully and I am happy to \
62 report that the retry path is correct, the backoff calculation is \
63 sound, and the new test covers the 429 case that the issue described. \
64 I have no objections to this change landing as it stands."
65 .into(),
66 findings: vec![],
67 },
68 &style
69 )
70 );
71
72 rule("A review with real work in it");
73 println!(
74 "{}",
75 review_comment(
76 "codex",
77 2,
78 &Review {
79 verdict: Verdict::ChangesRequested,
80 next_action: NextAction::HandBack,
81 summary: "There is one genuine defect here that should block, along with a \
82 couple of improvements that I do not think need to gate this \
83 particular pull request, and one pre-existing problem I noticed \
84 while reading the surrounding code."
85 .into(),
86 findings: vec![
87 finding(
88 "blocking",
89 "Retry loop never terminates when max_attempts is unset",
90 "I confirmed this by running the 429 test with max_attempts left at its \
91 default of None: the loop spins forever because the guard on line 91 \
92 compares against Some(0) rather than checking for None first. This is \
93 not a theoretical concern, the test hangs and I had to kill it.",
94 "src/net.rs:88",
95 true,
96 ),
97 finding(
98 "non-blocking",
99 "The request timeout is hard coded to thirty seconds",
100 "It would be better if this were configurable, since a slow upstream \
101 will now fail rather than wait, but the previous code had the same \
102 limitation so this is not a regression introduced by the change.",
103 "src/net.rs:44",
104 true,
105 ),
106 finding(
107 "nit",
108 "Log line says \"retrying\" without saying how many attempts remain",
109 "Purely a readability point for whoever is reading the logs at 3am.",
110 "src/net.rs:102",
111 true,
112 ),
113 finding(
114 "blocking",
115 "Config loader swallows a parse error",
116 "Unrelated to this PR, but load_config discards the error from serde and \
117 returns Default::default(), so a typo in the config file is silently \
118 ignored.",
119 "src/config.rs:210",
120 false,
121 ),
122 ],
123 },
124 &style
125 )
126 );
127
128 rule("Answering that review");
129 println!(
130 "{}",
131 disposition_comment(
132 "claude",
133 &ResponseDoc {
134 summary: "One of the two blocking points was right and I have fixed it. I do not \
135 agree with the other and have explained why below rather than changing \
136 working code to make the review go away."
137 .into(),
138 dispositions: vec![],
139 },
140 &["Retry loop never terminates when max_attempts is unset".to_string()],
141 &[
142 "Config loader swallows a parse error. The caller already validates the file \
143 against the schema before load_config is reached, so the discarded error is \
144 unreachable in practice."
145 .to_string()
146 ],
147 &["https://github.com/you/thing/issues/512".to_string()],
148 &style
149 )
150 .unwrap_or_default()
151 );
152
153 rule("The pull request body");
154 println!(
155 "{}",
156 pr_body(
157 478,
158 &Implementation {
159 summary: "Retry a 429 with exponential backoff instead of failing the request."
160 .into(),
161 problem: "A rate limited response was treated as fatal, so a single throttled \
162 call ended a run that had hours of work left in it. The retry path \
163 existed but only covered connection errors, and nothing in the logs \
164 said which of the two had happened."
165 .into(),
166 changes: vec![
167 "`send` now retries a 429, honouring `Retry-After` when the server sets it \
168 and backing off exponentially when it does not"
169 .into(),
170 "the retry budget is bounded at five attempts, so a permanent 429 still \
171 ends the call rather than spinning"
172 .into(),
173 "a retry logs the status it is retrying, which is what made the original \
174 failure impossible to tell apart from a dropped connection"
175 .into(),
176 ],
177 testing: vec![
178 "`cargo test retries_a_rate_limited_request`, which fakes a 429 with a \
179 `Retry-After` of 2 and asserts the wait"
180 .into(),
181 "point it at a throttled endpoint and watch a run finish rather than stop \
182 on the first 429"
183 .into(),
184 ],
185 notes: Some(
186 "Streaming calls do not go through `send` and are unchanged, which is worth \
187 a follow-up but not this one."
188 .into()
189 ),
190 ..Implementation::default()
191 },
192 &style
193 )
194 );
195
196 rule("What a whole run leaves on the PR (the default, one comment)");
197 let mut ended = IssueRun::new(482, "t");
198 ended.disputes = vec![Dispute {
199 title: "Config loader swallows a parse error".into(),
200 reasoning: "the caller validates against the schema before load_config is reached".into(),
201 }];
202 ended.filed = vec![
203 "https://github.com/you/thing/issues/485".into(),
204 "https://github.com/you/thing/issues/486".into(),
205 ];
206 println!(
207 "{}",
208 outcome_comment(
209 &ended,
210 &spar::model::Ledger::new(),
211 &Ending::OutOfRounds,
212 &style
213 )
214 .unwrap_or_default()
215 );
216 println!("\n (and a clean run that filed nothing posts no comment at all)");
217
218 rule("A review of somebody else's pull request (spar review)");
219 let judged = |standing, severity, title: &str, detail: &str, file: &str, by: &str| Judged {
220 finding: finding(severity, title, detail, file, true),
221 raised_by: by.to_string(),
222 standing,
223 counterpoint: None,
224 defence: None,
225 };
226 let mut disputed = judged(
227 Standing::Disputed,
228 "blocking",
229 "Config loader swallows a parse error",
230 "load_config discards the error from serde and returns a default.",
231 "src/config.rs:210",
232 "claude",
233 );
234 disputed.counterpoint =
235 Some("the caller validates against the schema before load_config is reached".into());
236 println!(
237 "{}",
238 verdict_comment(
239 &[
240 judged(
241 Standing::Corroborated,
242 "blocking",
243 "Retry loop never terminates when max_attempts is unset",
244 "Both reviewers reproduced this: the guard on line 91 compares against \
245 Some(0) rather than checking for None, so the 429 test hangs.",
246 "src/net.rs:88",
247 "claude and codex",
248 ),
249 judged(
250 Standing::Confirmed,
251 "non-blocking",
252 "The request timeout is hard coded",
253 "Not a regression, the previous code had the same limitation.",
254 "src/net.rs:44",
255 "codex",
256 ),
257 judged(
258 Standing::Unverified,
259 "nit",
260 "Log line does not say how many attempts remain",
261 "Readability for whoever reads the logs at 3am.",
262 "src/net.rs:102",
263 "claude",
264 ),
265 disputed,
266 judged(
267 Standing::Withdrawn,
268 "blocking",
269 "Off by one in the backoff",
270 "Withdrawn after the other reviewer pointed at the test that covers it.",
271 "src/net.rs:70",
272 "codex",
273 ),
274 ],
275 &style
276 )
277 );
278
279 rule("An issue both reviewers declined");
280 println!(
281 "{}",
282 skip_comment(
283 &SkippedItem {
284 issue: 91,
285 title: "Add a dark mode".into(),
286 tracker: false,
287 reasons: [
288 (
289 "claude".to_string(),
290 "This was already implemented in 1.4 and shipped behind the theme \
291 setting, so there is nothing left to do here."
292 .to_string()
293 ),
294 (
295 "codex".to_string(),
296 "Duplicate of #62, which is still open and has the full discussion."
297 .to_string()
298 ),
299 ]
300 .into_iter()
301 .collect(),
302 },
303 &style
304 )
305 );
306 rule("Answering a comment in its own thread");
307 for item in [
308 settled(
309 Ask::Implement,
310 "alice",
311 "src/retry.rs:91",
312 "Added the guard on the retry path and a test that reproduces the original failure.",
313 "",
314 true,
315 ),
316 settled(
317 Ask::Decline,
318 "bob",
319 "src/pool.rs:14",
320 "",
321 "Every caller of this function already holds the pool lock, so the check on line 14 \
322 cannot be reached with a null connection. I ran the suite with an assertion in its \
323 place and nothing tripped it.",
324 false,
325 ),
326 settled(
327 Ask::Defer,
328 "carol",
329 "src/electrum.rs:203",
330 "",
331 "This is real and it predates the branch: the reconnect monitor puts a stopped wallet \
332 back into the header router. It is not caused by anything here, so fixing it in this \
333 pull request would put an unrelated change in front of whoever reviews it.",
334 false,
335 ),
336 ] {
337 println!("{}\n", thread_reply(&item, &style));
338 }
339
340 rule("What a check-in leaves on the pull request");
341 let mut fixed = settled(
342 Ask::Implement,
343 "alice",
344 "the pull request",
345 "Retried the 429 with the backoff the issue asked for.",
346 "",
347 true,
348 );
349 fixed.pending.file = None;
350 let mut parked = settled(
351 Ask::Decline,
352 "dave",
353 "the pull request",
354 "",
355 "The two reviewers read the same code and did not agree about whether the guard is \
356 reachable.",
357 false,
358 );
359 parked.pending.file = None;
360 parked.parked = true;
361 println!(
362 "{}",
363 checkin_comment(&[fixed, parked], &style)
364 .unwrap_or_else(|| "(nothing to say, so no comment is posted)".into())
365 );
366 println!("\n (and a check-in with nothing outstanding posts no comment at all)");
367
368 println!();
369}