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 452)
39fn main() {
40 let loose = std::env::args().any(|a| a == "--loose");
41 let style = if loose {
42 Style {
43 terse: false,
44 ..Style::default()
45 }
46 } else {
47 Style::default()
48 };
49 if loose {
50 println!("(concision gate OFF: this is what a model would post unedited)");
51 }
52
53 rule("A clean review");
54 println!(
55 "{}",
56 review_comment(
57 "codex",
58 1,
59 &Review {
60 verdict: Verdict::Approve,
61 next_action: NextAction::Merge,
62 summary: "I reviewed the changes on this branch carefully and I am happy to \
63 report that the retry path is correct, the backoff calculation is \
64 sound, and the new test covers the 429 case that the issue described. \
65 I have no objections to this change landing as it stands."
66 .into(),
67 findings: vec![],
68 },
69 &style
70 )
71 );
72
73 rule("A review with real work in it");
74 println!(
75 "{}",
76 review_comment(
77 "codex",
78 2,
79 &Review {
80 verdict: Verdict::ChangesRequested,
81 next_action: NextAction::HandBack,
82 summary: "There is one genuine defect here that should block, along with a \
83 couple of improvements that I do not think need to gate this \
84 particular pull request, and one pre-existing problem I noticed \
85 while reading the surrounding code."
86 .into(),
87 findings: vec![
88 finding(
89 "blocking",
90 "Retry loop never terminates when max_attempts is unset",
91 "I confirmed this by running the 429 test with max_attempts left at its \
92 default of None: the loop spins forever because the guard on line 91 \
93 compares against Some(0) rather than checking for None first. This is \
94 not a theoretical concern, the test hangs and I had to kill it.",
95 "src/net.rs:88",
96 true,
97 ),
98 finding(
99 "non-blocking",
100 "The request timeout is hard coded to thirty seconds",
101 "It would be better if this were configurable, since a slow upstream \
102 will now fail rather than wait, but the previous code had the same \
103 limitation so this is not a regression introduced by the change.",
104 "src/net.rs:44",
105 true,
106 ),
107 finding(
108 "nit",
109 "Log line says \"retrying\" without saying how many attempts remain",
110 "Purely a readability point for whoever is reading the logs at 3am.",
111 "src/net.rs:102",
112 true,
113 ),
114 finding(
115 "blocking",
116 "Config loader swallows a parse error",
117 "Unrelated to this PR, but load_config discards the error from serde and \
118 returns Default::default(), so a typo in the config file is silently \
119 ignored.",
120 "src/config.rs:210",
121 false,
122 ),
123 ],
124 },
125 &style
126 )
127 );
128
129 rule("Answering that review");
130 println!(
131 "{}",
132 disposition_comment(
133 "claude",
134 &ResponseDoc {
135 summary: "One of the two blocking points was right and I have fixed it. I do not \
136 agree with the other and have explained why below rather than changing \
137 working code to make the review go away."
138 .into(),
139 dispositions: vec![],
140 },
141 &["Retry loop never terminates when max_attempts is unset".to_string()],
142 &[
143 "Config loader swallows a parse error. The caller already validates the file \
144 against the schema before load_config is reached, so the discarded error is \
145 unreachable in practice."
146 .to_string()
147 ],
148 &["https://github.com/you/thing/issues/512".to_string()],
149 &style
150 )
151 .unwrap_or_default()
152 );
153
154 rule("The pull request body");
155 println!(
156 "{}",
157 pr_body(
158 478,
159 &Implementation {
160 summary: "Retry a 429 with exponential backoff instead of failing the request."
161 .into(),
162 problem: "A rate limited response was treated as fatal, so a single throttled \
163 call ended a run that had hours of work left in it. The retry path \
164 existed but only covered connection errors, and nothing in the logs \
165 said which of the two had happened."
166 .into(),
167 changes: vec![
168 "`send` now retries a 429, honouring `Retry-After` when the server sets it \
169 and backing off exponentially when it does not"
170 .into(),
171 "the retry budget is bounded at five attempts, so a permanent 429 still \
172 ends the call rather than spinning"
173 .into(),
174 "a retry logs the status it is retrying, which is what made the original \
175 failure impossible to tell apart from a dropped connection"
176 .into(),
177 ],
178 testing: vec![
179 "`cargo test retries_a_rate_limited_request`, which fakes a 429 with a \
180 `Retry-After` of 2 and asserts the wait"
181 .into(),
182 "point it at a throttled endpoint and watch a run finish rather than stop \
183 on the first 429"
184 .into(),
185 ],
186 notes: Some(
187 "Streaming calls do not go through `send` and are unchanged, which is worth \
188 a follow-up but not this one."
189 .into()
190 ),
191 ..Implementation::default()
192 },
193 Some("cargo test"),
194 &style
195 )
196 );
197
198 rule("What a whole run leaves on the PR (the default, one comment)");
199 let mut ended = IssueRun::new(482, "t");
200 ended.disputes = vec![Dispute {
201 title: "Config loader swallows a parse error".into(),
202 file: "src/config.rs".into(),
203 reasoning: "the caller validates against the schema before load_config is reached".into(),
204 }];
205 ended.filed = vec![
206 "https://github.com/you/thing/issues/485".into(),
207 "https://github.com/you/thing/issues/486".into(),
208 ];
209 println!(
210 "{}",
211 outcome_comment(
212 &ended,
213 &spar::model::Ledger::new(),
214 &Ending::OutOfRounds,
215 &style
216 )
217 .unwrap_or_default()
218 );
219 println!("\n (and a clean run that filed nothing posts no comment at all)");
220
221 rule("What a run leaves when the closing pass did not sign it off");
222 let mut left = IssueRun::new(482, "t");
223 left.noted = vec![finding(
224 "non-blocking",
225 "Timeout is not configurable",
226 "The retry budget is fixed at three.",
227 "src/net.rs",
228 true,
229 )];
230 let unresolved = vec![finding(
231 "blocking",
232 "The retry fix never reaches the 429 path",
233 "The guard added in round 2 sits after the early return on line 88, so a rate limited \
234 response still takes the old path. Reproduced with the 429 test.",
235 "src/net.rs:88",
236 true,
237 )];
238 println!(
239 "{}",
240 outcome_comment(
241 &left,
242 &spar::model::Ledger::new(),
243 &Ending::Unresolved(&unresolved),
244 &style
245 )
246 .unwrap_or_default()
247 );
248
249 rule("A review of somebody else's pull request (spar review)");
250 let judged = |standing, severity, title: &str, detail: &str, file: &str, by: &str| Judged {
251 finding: finding(severity, title, detail, file, true),
252 raised_by: by.to_string(),
253 standing,
254 counterpoint: None,
255 defence: None,
256 };
257 let mut disputed = judged(
258 FindingStanding::Disputed,
259 "blocking",
260 "Config loader swallows a parse error",
261 "load_config discards the error from serde and returns a default.",
262 "src/config.rs:210",
263 "claude",
264 );
265 disputed.counterpoint =
266 Some("the caller validates against the schema before load_config is reached".into());
267 println!(
268 "{}",
269 verdict_comment(
270 &[
271 judged(
272 FindingStanding::Corroborated,
273 "blocking",
274 "Retry loop never terminates when max_attempts is unset",
275 "Both reviewers reproduced this: the guard on line 91 compares against \
276 Some(0) rather than checking for None, so the 429 test hangs.",
277 "src/net.rs:88",
278 "claude and codex",
279 ),
280 judged(
281 FindingStanding::Confirmed,
282 "non-blocking",
283 "The request timeout is hard coded",
284 "Not a regression, the previous code had the same limitation.",
285 "src/net.rs:44",
286 "codex",
287 ),
288 judged(
289 FindingStanding::Unverified,
290 "nit",
291 "Log line does not say how many attempts remain",
292 "Readability for whoever reads the logs at 3am.",
293 "src/net.rs:102",
294 "claude",
295 ),
296 disputed,
297 judged(
298 FindingStanding::Withdrawn,
299 "blocking",
300 "Off by one in the backoff",
301 "Withdrawn after the other reviewer pointed at the test that covers it.",
302 "src/net.rs:70",
303 "codex",
304 ),
305 ],
306 &style
307 )
308 );
309
310 rule("A second look at that pull request, after the author pushed (spar review --recheck)");
311 let recorded = |severity: &str, title: &str, detail: &str, file: &str, standing| {
312 spar::model::RecordedFinding {
313 title: title.into(),
314 file: file.into(),
315 severity: Severity::parse_lenient(severity).expect("severity"),
316 detail: detail.into(),
317 in_scope: true,
318 raised_by: "claude and codex".into(),
319 standing,
320 counterpoint: None,
321 defence: None,
322 }
323 };
324 println!(
325 "{}",
326 recheck_comment(
327 &Delta {
328 from: "0123abcdef".into(),
329 commits: vec![
330 "a1b2c3d check for None before comparing attempts".into(),
331 "e4f5a6b make the request timeout configurable".into(),
332 ],
333 rewritten: false,
334 },
335 &[
336 Rechecked {
337 recorded: recorded(
338 "blocking",
339 "Retry loop never terminates when max_attempts is unset",
340 "The guard on line 91 compares against Some(0) rather than checking for None.",
341 "src/net.rs:88",
342 FindingStanding::Corroborated,
343 ),
344 resolution: Resolution::Fixed(
345 "src/net.rs:91 now matches on None first, and the 429 test that used to \
346 hang passes in 0.3s when run with cargo test retry"
347 .into(),
348 ),
349 },
350 Rechecked {
351 recorded: recorded(
352 "non-blocking",
353 "The request timeout is hard coded",
354 "Not a regression, the previous code had the same limitation.",
355 "src/net.rs:44",
356 FindingStanding::Confirmed,
357 ),
358 resolution: Resolution::Rebutted(
359 "the author's reply is right that the timeout comes from the caller's \
360 Client builder since e4f5a6b, so there is nothing left to make configurable \
361 here"
362 .into(),
363 ),
364 },
365 Rechecked {
366 recorded: recorded(
367 "blocking",
368 "Config loader swallows a parse error",
369 "load_config discards the error from serde and returns a default.",
370 "src/config.rs:210",
371 FindingStanding::Confirmed,
372 ),
373 resolution: Resolution::Open(
374 "codex still sees it: load_config still returns Default on a parse error, \
375 and nothing upstream validates the file first"
376 .into(),
377 ),
378 },
379 ],
380 &[judged(
381 FindingStanding::Corroborated,
382 "non-blocking",
383 "The new None branch logs at error level on every retry",
384 "Each retry now writes an error line, so a flaky upstream fills the log with \
385 errors for a request that succeeds on the third try.",
386 "src/net.rs:93",
387 "claude and codex",
388 )],
389 &style,
390 2
391 )
392 );
393
394 rule("An issue both reviewers declined");
395 println!(
396 "{}",
397 skip_comment(
398 &SkippedItem {
399 issue: 91,
400 title: "Add a dark mode".into(),
401 tracker: false,
402 reasons: [
403 (
404 "claude".to_string(),
405 "This was already implemented in 1.4 and shipped behind the theme \
406 setting, so there is nothing left to do here."
407 .to_string()
408 ),
409 (
410 "codex".to_string(),
411 "Duplicate of #62, which is still open and has the full discussion."
412 .to_string()
413 ),
414 ]
415 .into_iter()
416 .collect(),
417 },
418 &style
419 )
420 );
421 rule("Answering a comment in its own thread");
422 for item in [
423 settled(
424 Ask::Implement,
425 "alice",
426 "src/retry.rs:91",
427 "Added the guard on the retry path and a test that reproduces the original failure.",
428 "",
429 true,
430 ),
431 settled(
432 Ask::Decline,
433 "bob",
434 "src/pool.rs:14",
435 "",
436 "Every caller of this function already holds the pool lock, so the check on line 14 \
437 cannot be reached with a null connection. I ran the suite with an assertion in its \
438 place and nothing tripped it.",
439 false,
440 ),
441 settled(
442 Ask::Defer,
443 "carol",
444 "src/electrum.rs:203",
445 "",
446 "This is real and it predates the branch: the reconnect monitor puts a stopped wallet \
447 back into the header router. It is not caused by anything here, so fixing it in this \
448 pull request would put an unrelated change in front of whoever reviews it.",
449 false,
450 ),
451 ] {
452 println!("{}\n", thread_reply(&item, &style));
453 }
454
455 rule("What a check-in leaves on the pull request");
456 let mut fixed = settled(
457 Ask::Implement,
458 "alice",
459 "the pull request",
460 "Retried the 429 with the backoff the issue asked for.",
461 "",
462 true,
463 );
464 fixed.pending.file = None;
465 let mut parked = settled(
466 Ask::Decline,
467 "dave",
468 "the pull request",
469 "",
470 "The two reviewers read the same code and did not agree about whether the guard is \
471 reachable.",
472 false,
473 );
474 parked.pending.file = None;
475 parked.parked = true;
476 println!(
477 "{}",
478 checkin_comment(&[fixed, parked], &style)
479 .unwrap_or_else(|| "(nothing to say, so no comment is posted)".into())
480 );
481 println!("\n (and a check-in with nothing outstanding posts no comment at all)");
482
483 rule("An idea filed from a brainstorm");
484 let idea = Candidate {
485 id: 1,
486 idea: Idea {
487 title: "Batch settlement with hash timelocks".into(),
488 combines: vec![
489 "hash time locked contracts".into(),
490 "batch auctions".into(),
491 "the OP_CHECKSEQUENCEVERIFY delay".into(),
492 ],
493 how_it_works: "Each trade posts a hash timelock, and one batch auction clears every \
494 one of them at the deadline together, so the batch is the settlement."
495 .into(),
496 why_new: "The nearest thing is a plain escrow, which settles one trade at a time \
497 and cannot net them against each other."
498 .into(),
499 enables: "Settling many trades in one transaction.".into(),
500 risks: "A fee spike at the deadline makes the batch uneconomic.".into(),
501 first_experiment: "Simulate ten trades on regtest and count the bytes.".into(),
502 },
503 proposers: vec!["claude".into()],
504 parent: None,
505 objection: Some((
506 "codex".into(),
507 "Coinjoin coordinators already batch settlement.".into(),
508 )),
509 defence: Some(
510 "A coordinator batches inputs, not conditional trades; nothing there clears a hash \
511 lock."
512 .into(),
513 ),
514 standing: Standing::Defended,
515 rank_sum: 3,
516 };
517 println!("{}", issue_body(&idea, &style));
518 println!("\n (who proposed and who objected stays in the session file, never in the issue)");
519
520 println!();
521}