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 368)
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("An issue both reviewers declined");
311 println!(
312 "{}",
313 skip_comment(
314 &SkippedItem {
315 issue: 91,
316 title: "Add a dark mode".into(),
317 tracker: false,
318 reasons: [
319 (
320 "claude".to_string(),
321 "This was already implemented in 1.4 and shipped behind the theme \
322 setting, so there is nothing left to do here."
323 .to_string()
324 ),
325 (
326 "codex".to_string(),
327 "Duplicate of #62, which is still open and has the full discussion."
328 .to_string()
329 ),
330 ]
331 .into_iter()
332 .collect(),
333 },
334 &style
335 )
336 );
337 rule("Answering a comment in its own thread");
338 for item in [
339 settled(
340 Ask::Implement,
341 "alice",
342 "src/retry.rs:91",
343 "Added the guard on the retry path and a test that reproduces the original failure.",
344 "",
345 true,
346 ),
347 settled(
348 Ask::Decline,
349 "bob",
350 "src/pool.rs:14",
351 "",
352 "Every caller of this function already holds the pool lock, so the check on line 14 \
353 cannot be reached with a null connection. I ran the suite with an assertion in its \
354 place and nothing tripped it.",
355 false,
356 ),
357 settled(
358 Ask::Defer,
359 "carol",
360 "src/electrum.rs:203",
361 "",
362 "This is real and it predates the branch: the reconnect monitor puts a stopped wallet \
363 back into the header router. It is not caused by anything here, so fixing it in this \
364 pull request would put an unrelated change in front of whoever reviews it.",
365 false,
366 ),
367 ] {
368 println!("{}\n", thread_reply(&item, &style));
369 }
370
371 rule("What a check-in leaves on the pull request");
372 let mut fixed = settled(
373 Ask::Implement,
374 "alice",
375 "the pull request",
376 "Retried the 429 with the backoff the issue asked for.",
377 "",
378 true,
379 );
380 fixed.pending.file = None;
381 let mut parked = settled(
382 Ask::Decline,
383 "dave",
384 "the pull request",
385 "",
386 "The two reviewers read the same code and did not agree about whether the guard is \
387 reachable.",
388 false,
389 );
390 parked.pending.file = None;
391 parked.parked = true;
392 println!(
393 "{}",
394 checkin_comment(&[fixed, parked], &style)
395 .unwrap_or_else(|| "(nothing to say, so no comment is posted)".into())
396 );
397 println!("\n (and a check-in with nothing outstanding posts no comment at all)");
398
399 rule("An idea filed from a brainstorm");
400 let idea = Candidate {
401 id: 1,
402 idea: Idea {
403 title: "Batch settlement with hash timelocks".into(),
404 combines: vec![
405 "hash time locked contracts".into(),
406 "batch auctions".into(),
407 "the OP_CHECKSEQUENCEVERIFY delay".into(),
408 ],
409 how_it_works: "Each trade posts a hash timelock, and one batch auction clears every \
410 one of them at the deadline together, so the batch is the settlement."
411 .into(),
412 why_new: "The nearest thing is a plain escrow, which settles one trade at a time \
413 and cannot net them against each other."
414 .into(),
415 enables: "Settling many trades in one transaction.".into(),
416 risks: "A fee spike at the deadline makes the batch uneconomic.".into(),
417 first_experiment: "Simulate ten trades on regtest and count the bytes.".into(),
418 },
419 proposers: vec!["claude".into()],
420 parent: None,
421 objection: Some((
422 "codex".into(),
423 "Coinjoin coordinators already batch settlement.".into(),
424 )),
425 defence: Some(
426 "A coordinator batches inputs, not conditional trades; nothing there clears a hash \
427 lock."
428 .into(),
429 ),
430 standing: Standing::Defended,
431 rank_sum: 3,
432 };
433 println!("{}", issue_body(&idea, &style));
434 println!("\n (who proposed and who objected stays in the session file, never in the issue)");
435
436 println!();
437}