Skip to main content

skip_comment

Function skip_comment 

Source
pub fn skip_comment(item: &SkippedItem, style: &Style) -> String
Expand description

What is posted on an issue both agents declined. What is posted on an issue both reviewers declined.

Just the reasons. GitHub already shows that it was closed as not planned, and which model held which opinion is a fact about the run rather than about the issue. Duplicates are collapsed, since two reviewers reaching the same conclusion often reach it in the same words.

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