Skip to main content

magi/
followup.rs

1//! Follow-up tasks for the findings a merged run left open.
2//!
3//! A run can hand off after its review budget is spent with findings still
4//! open (and, with `land_approval` off, merge unattended). Those findings
5//! live in the pull request body, which nobody reads after the merge. This
6//! module files them as queue tasks instead, once the merge is confirmed.
7//!
8//! What is filed: every Major-or-above finding of the last review round, and
9//! every finding of a seat whose final vote was reject (a vote carries no
10//! per-finding reasoning, so the seat's Minor/Nit findings come along). Other
11//! Minor/Nit findings are only listed, in the pull-request comment.
12//!
13//! Best-effort by construction, like `bump::after_merge`: [`after_merge`]
14//! returns `()`, records every failure as a run event and never touches
15//! `status`.
16
17use anyhow::Result;
18
19use crate::queue::{FollowUp, Queue, Source, Task};
20use crate::run::{FollowupRecord, RunState};
21use crate::verdict::{Finding, ReviewVote};
22
23/// Node name carried by [`Source::Agent`] on a filed follow-up.
24pub const NODE: &str = "followup";
25
26/// A task of this generation (or deeper) does not file follow-ups of its own.
27pub const MAX_FOLLOWUP_GENERATION: u32 = 2;
28
29/// Two findings in one file this many lines apart are one defect.
30const LINE_WINDOW: u32 = 5;
31
32/// What one pass did.
33#[derive(Debug, Default, PartialEq, Eq)]
34pub struct Outcome {
35    /// Task ids created by this pass.
36    pub filed: Vec<String>,
37    /// Findings selected but not filed (cap reached), as ids.
38    pub capped: Vec<String>,
39    /// Minor/Nit findings of the last round that were only listed.
40    pub unfiled: Vec<String>,
41    /// Findings not filed because another task already covers them, as
42    /// `(finding id, covering task id)`. Recomputed from the queue each pass.
43    pub covered: Vec<(String, String)>,
44}
45
46/// File follow-ups for a run that just merged, and comment on the pull
47/// request. Never fails: problems become run events.
48pub async fn after_merge(state: &mut RunState, pr_url: &str) {
49    if !state.config.graph.file_followups {
50        return;
51    }
52    let queue = Queue::open();
53    let outcome = match file(state, pr_url, &queue) {
54        Ok(o) => o,
55        Err(e) => {
56            state.event(NODE, format!("follow-up filing failed: {e:#}"));
57            return;
58        }
59    };
60    if outcome.filed.is_empty()
61        && (state.followup_commented || (state.followups.is_empty() && outcome.covered.is_empty()))
62    {
63        return;
64    }
65    let body = comment_body(state, &outcome);
66    match crate::bump::gh_pr_comment(&state.repo, pr_url, &body).await {
67        Ok(()) => state.followup_commented = true,
68        Err(e) => state.event(NODE, format!("follow-up comment not posted: {e:#}")),
69    }
70}
71
72/// The deterministic half of [`after_merge`]: select, group, and file into
73/// `queue`. Records the event and the run's own bookkeeping; the caller
74/// saves the state.
75pub fn file(state: &mut RunState, pr_url: &str, queue: &Queue) -> Result<Outcome> {
76    let mut out = Outcome::default();
77    let Some(round) = state.reviews.last().cloned() else {
78        return Ok(out);
79    };
80    let rejecters: Vec<usize> = round
81        .final_votes()
82        .into_iter()
83        .filter(|(_, _, v)| *v == ReviewVote::Reject)
84        .map(|(seat, _, _)| seat)
85        .collect();
86    let mut chosen: Vec<Finding> = Vec::new();
87    for rec in &round.reviews {
88        let rejected = rejecters.contains(&rec.reviewer);
89        for f in &rec.findings {
90            if f.severity.blocks() || rejected {
91                chosen.push(f.clone());
92            } else {
93                out.unfiled.push(f.id.clone());
94            }
95        }
96    }
97    if chosen.is_empty() {
98        return Ok(out);
99    }
100
101    let origin_task = state.origin.as_ref().and_then(|o| o.task.clone());
102    // The depth recorded when the run started wins; the queue is only a
103    // fallback for runs that predate it, and an unknown depth is read as
104    // unbounded-until-proven-otherwise only when there is no origin task.
105    let parent_gen = match state.followup_generation {
106        Some(g) => g,
107        None => match origin_task.as_deref().map(|t| queue.get(t)) {
108            Some(Ok(t)) => {
109                let g = t.followup.map_or(0, |f| f.generation);
110                state.followup_generation = Some(g);
111                g
112            }
113            Some(Err(_)) => MAX_FOLLOWUP_GENERATION,
114            None => 0,
115        },
116    };
117    if parent_gen >= MAX_FOLLOWUP_GENERATION {
118        out.capped = chosen.iter().map(|f| f.id.clone()).collect();
119        state.event(
120            NODE,
121            format!(
122                "generation cap reached ({parent_gen}); not filing follow-ups for open finding(s): {}",
123                out.capped.join(", ")
124            ),
125        );
126        return Ok(out);
127    }
128
129    let mut done: Vec<String> = state
130        .followups
131        .iter()
132        .flat_map(|r| r.findings.iter().cloned())
133        .collect();
134    for t in queue.list() {
135        if let Some(f) = t.followup
136            && f.run == state.id
137        {
138            done.extend(f.findings.iter().cloned());
139            if !state.followups.iter().any(|r| r.task == t.id) {
140                state.followups.push(FollowupRecord {
141                    task: t.id,
142                    findings: f.findings,
143                });
144            }
145        }
146    }
147
148    let tasks = queue.list();
149    let mut failure = None;
150    for group in group_findings(chosen) {
151        let ids: Vec<String> = group.iter().map(|f| f.id.clone()).collect();
152        let own = task_id(&state.id, &ids);
153        let covers: Vec<Option<String>> = ids
154            .iter()
155            .map(|id| covering_task(&tasks, &own, &state.id, pr_url, id))
156            .collect();
157        if covers.iter().all(Option::is_some) {
158            for (id, by) in ids.iter().zip(covers.into_iter().flatten()) {
159                state.event(
160                    NODE,
161                    format!("not filing follow-up for {id}: already covered by task {by}"),
162                );
163                out.covered.push((id.clone(), by));
164            }
165            continue;
166        }
167        if ids.iter().any(|id| done.contains(id)) {
168            continue;
169        }
170        let mut task = build_task(state, pr_url, origin_task.clone(), &group, parent_gen + 1);
171        match queue.create_new(&mut task) {
172            Ok(created) => {
173                if created {
174                    out.filed.push(task.id.clone());
175                }
176                state.followups.push(FollowupRecord {
177                    task: task.id,
178                    findings: ids,
179                });
180            }
181            Err(e) => {
182                failure = Some(e);
183                break;
184            }
185        }
186    }
187    if !out.filed.is_empty() {
188        state.event(
189            NODE,
190            format!(
191                "filed {} follow-up task(s): ids {}",
192                out.filed.len(),
193                out.filed.join(", ")
194            ),
195        );
196    }
197    if let Some(e) = failure {
198        state.event(NODE, format!("follow-up filing stopped: {e:#}"));
199    }
200    Ok(out)
201}
202
203/// The id of a task, other than `own`, that already covers finding `id` of
204/// run `run` merged as `pr_url`, whatever its status. Concrete identifiers
205/// only: a follow-up of the same run listing the finding, or an instruction
206/// naming both the finding id and the pull request (URL or `#<number>`).
207fn covering_task(tasks: &[Task], own: &str, run: &str, pr_url: &str, id: &str) -> Option<String> {
208    tasks
209        .iter()
210        .find(|t| {
211            t.id != own
212                && (t
213                    .followup
214                    .as_ref()
215                    .is_some_and(|f| f.run == run && f.findings.iter().any(|x| x == id))
216                    || (names_word(&t.instruction, id) && names_pr(&t.instruction, pr_url)))
217        })
218        .map(|t| t.id.clone())
219}
220
221/// ASCII only: an id is often followed directly by prose in another script
222/// (`R3-1-1を修正`), which must not read as part of the id.
223fn is_name_char(c: char) -> bool {
224    c.is_ascii_alphanumeric() || matches!(c, '_' | '-')
225}
226
227/// `needle` in `text` with no name character on either side.
228fn names_word(text: &str, needle: &str) -> bool {
229    !needle.is_empty()
230        && text.match_indices(needle).any(|(i, _)| {
231            let before = text[..i].chars().next_back();
232            let after = text[i + needle.len()..].chars().next();
233            before.is_none_or(|c| !is_name_char(c)) && after.is_none_or(|c| !is_name_char(c))
234        })
235}
236
237/// Does `text` name the pull request by its URL or its `#<number>` form,
238/// with no further digit (or name character before `#`) so `#47` is not `#473`?
239fn names_pr(text: &str, pr_url: &str) -> bool {
240    let url = pr_url.trim_end_matches('/');
241    let digit_after = |rest: &str| rest.chars().next().is_some_and(|c| c.is_ascii_digit());
242    if !url.is_empty()
243        && text
244            .match_indices(url)
245            .any(|(i, _)| !digit_after(&text[i + url.len()..]))
246    {
247        return true;
248    }
249    let Some(n) = url
250        .rsplit('/')
251        .next()
252        .filter(|n| !n.is_empty() && n.bytes().all(|b| b.is_ascii_digit()))
253    else {
254        return false;
255    };
256    let tag = format!("#{n}");
257    text.match_indices(&tag).any(|(i, _)| {
258        let before = text[..i].chars().next_back();
259        before.is_none_or(|c| !is_name_char(c)) && !digit_after(&text[i + tag.len()..])
260    })
261}
262
263/// Group findings that are the same defect: same file with lines within
264/// [`LINE_WINDOW`] of each other, or, with no file, the same normalized
265/// title. Deterministic, so a re-run groups identically.
266fn group_findings(mut findings: Vec<Finding>) -> Vec<Vec<Finding>> {
267    findings.sort_by(|a, b| (&a.file, a.line, &a.id).cmp(&(&b.file, b.line, &b.id)));
268    let mut groups: Vec<Vec<Finding>> = Vec::new();
269    for f in findings {
270        let hit = groups
271            .iter()
272            .position(|g| g.iter().any(|o| same_defect(o, &f)));
273        match hit {
274            Some(i) => groups[i].push(f),
275            None => groups.push(vec![f]),
276        }
277    }
278    for g in &mut groups {
279        g.sort_by(|a, b| a.id.cmp(&b.id));
280    }
281    groups
282}
283
284fn same_defect(a: &Finding, b: &Finding) -> bool {
285    match (&a.file, &b.file) {
286        (Some(x), Some(y)) => {
287            x == y
288                && match (a.line, b.line) {
289                    (Some(l), Some(m)) => l.abs_diff(m) <= LINE_WINDOW,
290                    (None, None) => true,
291                    _ => false,
292                }
293        }
294        (None, None) => normalize(&a.title) == normalize(&b.title),
295        _ => false,
296    }
297}
298
299fn normalize(title: &str) -> String {
300    title
301        .to_lowercase()
302        .split(|c: char| !c.is_alphanumeric())
303        .filter(|w| !w.is_empty())
304        .collect::<Vec<_>>()
305        .join(" ")
306}
307
308/// Deterministic id: the run id plus a hash of the group's finding ids, so
309/// two passes name the same task and [`Queue::create_new`] can refuse the
310/// second. Ends in a `-` segment like every id, so `queue::short` works.
311fn task_id(run: &str, ids: &[String]) -> String {
312    let mut h: u64 = 0xcbf2_9ce4_8422_2325;
313    for b in ids.join("+").bytes() {
314        h ^= u64::from(b);
315        h = h.wrapping_mul(0x0100_0000_01b3);
316    }
317    format!("{run}-f{:05x}", h & 0xf_ffff)
318}
319
320fn location(f: &Finding) -> String {
321    match (&f.file, f.line) {
322        (Some(p), Some(l)) => format!("{p}:{l}"),
323        (Some(p), None) => p.clone(),
324        _ => "(no location)".to_owned(),
325    }
326}
327
328fn build_task(
329    state: &RunState,
330    pr_url: &str,
331    origin_task: Option<String>,
332    group: &[Finding],
333    generation: u32,
334) -> Task {
335    let ids: Vec<String> = group.iter().map(|f| f.id.clone()).collect();
336    let first = &group[0];
337    let title = format!(
338        "follow-up: {} ({})",
339        crate::queue::title_from(&first.title, 80),
340        location(first)
341    );
342    let mut s = format!(
343        "This is a follow-up to a change that was already merged: run {run} merged \
344         {pr_url} while the review finding(s) below were still open. Branch from the \
345         current main - do not use the merged branch - and fix the defect described, \
346         keeping the change small. If a finding no longer applies to current main, say so \
347         and change nothing for it.\n\n\
348         Merged run: {run}\nPull request: {pr_url}\n",
349        run = state.id,
350    );
351    if let Some(t) = &origin_task {
352        s.push_str(&format!("Original task: {t}\n"));
353    }
354    s.push_str("\n## Findings\n");
355    for f in group {
356        s.push_str(&format!(
357            "\n### {} [{:?}] {}\n\nLocation: {}\n\n{}\n",
358            f.id,
359            f.severity,
360            f.title,
361            location(f),
362            f.detail.trim()
363        ));
364    }
365    let mut task = Task::new(
366        title,
367        s,
368        state.repo.clone(),
369        Source::Agent {
370            run: state.id.clone(),
371            node: NODE.to_owned(),
372        },
373    );
374    task.id = task_id(&state.id, &ids);
375    task.solo = true;
376    task.followup = Some(FollowUp {
377        run: state.id.clone(),
378        origin_task,
379        pr: pr_url.to_owned(),
380        findings: ids,
381        generation,
382    });
383    task
384}
385
386fn comment_body(state: &RunState, out: &Outcome) -> String {
387    let mut s = format!(
388        "<!-- magi-followup run={} -->\nThis pull request merged with review findings still open.",
389        state.id
390    );
391    if state.followups.is_empty() {
392        s.push_str(" Each is already covered by another task:\n");
393    } else {
394        s.push_str(" They were filed as follow-up tasks:\n\n");
395        for r in &state.followups {
396            s.push_str(&format!("- `{}`: {}\n", r.task, r.findings.join(", ")));
397        }
398        if !out.covered.is_empty() {
399            s.push_str("\nNot filed, already covered:\n");
400        }
401    }
402    if !out.covered.is_empty() {
403        s.push('\n');
404    }
405    for (id, by) in &out.covered {
406        s.push_str(&format!("- {id}: already covered by task `{by}`\n"));
407    }
408    if !out.unfiled.is_empty() {
409        s.push_str(&format!(
410            "\nMinor/nit findings that were only listed, not filed: {}\n",
411            out.unfiled.join(", ")
412        ));
413    }
414    s
415}
416
417#[cfg(test)]
418mod tests {
419    use super::*;
420    use crate::config::Config;
421    use crate::queue::Queue;
422    use crate::run::ReviewRecord;
423    use crate::run::{Origin, ReviewRound, RunState};
424    use crate::verdict::{Finding, ReviewVote, Severity};
425    use std::path::PathBuf;
426
427    fn finding(id: &str, sev: Severity, file: &str, line: u32) -> Finding {
428        Finding {
429            id: id.to_owned(),
430            severity: sev,
431            file: Some(file.to_owned()),
432            line: Some(line),
433            title: format!("title {id}"),
434            detail: format!("detail of {id}"),
435        }
436    }
437
438    fn record(seat: usize, findings: Vec<Finding>, vote: ReviewVote) -> ReviewRecord {
439        ReviewRecord {
440            attempts: 0,
441            reviewer: seat,
442            agent: "alpha".to_owned(),
443            summary: String::new(),
444            findings,
445            vote: Some(vote),
446            failed: None,
447            duration_ms: 0,
448        }
449    }
450
451    fn round(reviews: Vec<ReviewRecord>) -> ReviewRound {
452        ReviewRound {
453            round: 3,
454            head: "deadbee".to_owned(),
455            verified_head: None,
456            verified_at: None,
457            reviews,
458            e2e: Vec::new(),
459            verify_retried: false,
460            e2e_deferred: false,
461            e2e_defer_reason: None,
462            fix: None,
463            blocking: 0,
464            answered: 2,
465            expected: 2,
466            clean: false,
467            progressed: false,
468            vote_split: false,
469            reconsideration: Vec::new(),
470            verdict: None,
471        }
472    }
473
474    fn merged(reviews: Vec<ReviewRecord>) -> RunState {
475        let mut s = RunState::new(
476            PathBuf::from("/repo"),
477            "main".to_owned(),
478            "abc1234".to_owned(),
479            "do it".to_owned(),
480            Config::default(),
481        );
482        s.reviews.push(round(reviews));
483        s
484    }
485
486    fn two_seats() -> Vec<ReviewRecord> {
487        vec![
488            record(
489                1,
490                vec![
491                    finding("R3-1-1", Severity::Major, "src/waiter.rs", 211),
492                    finding("R3-1-2", Severity::Minor, "src/a.rs", 1),
493                ],
494                ReviewVote::Reject,
495            ),
496            record(
497                2,
498                vec![
499                    finding("R3-2-1", Severity::Major, "src/waiter.rs", 213),
500                    finding("R3-2-2", Severity::Nit, "src/b.rs", 9),
501                ],
502                ReviewVote::Approve,
503            ),
504        ]
505    }
506
507    #[test]
508    fn one_task_per_distinct_defect_with_the_findings_in_full() {
509        let dir = tempfile::tempdir().unwrap();
510        let q = Queue::at(dir.path().join("queue"));
511        let mut s = merged(two_seats());
512        let out = file(&mut s, "https://example.invalid/o/r/pull/9", &q).unwrap();
513        // waiter.rs pair is one defect; seat 1 rejected so its Minor is filed
514        // too; seat 2's Nit is only listed.
515        assert_eq!(out.filed.len(), 2, "{out:?}");
516        assert_eq!(out.unfiled, vec!["R3-2-2".to_owned()]);
517        let tasks = q.list();
518        let t = tasks
519            .iter()
520            .find(|t| t.followup.as_ref().unwrap().findings.len() == 2)
521            .unwrap();
522        assert!(t.solo);
523        assert_eq!(t.followup.as_ref().unwrap().generation, 1);
524        assert_eq!(
525            t.source,
526            Source::Agent {
527                run: s.id.clone(),
528                node: NODE.to_owned()
529            }
530        );
531        for needle in [
532            "R3-1-1",
533            "R3-2-1",
534            "src/waiter.rs:211",
535            "detail of R3-2-1",
536            "pull/9",
537            &s.id,
538            "current main",
539        ] {
540            assert!(
541                t.instruction.contains(needle),
542                "{needle}: {}",
543                t.instruction
544            );
545        }
546        assert!(
547            s.events
548                .iter()
549                .any(|e| e.message.contains("filed 2 follow-up"))
550        );
551        assert_eq!(s.followups.len(), 2);
552    }
553
554    #[test]
555    fn filing_twice_does_not_duplicate() {
556        let dir = tempfile::tempdir().unwrap();
557        let q = Queue::at(dir.path().join("queue"));
558        let mut s = merged(two_seats());
559        file(&mut s, "u", &q).unwrap();
560        let again = file(&mut s, "u", &q).unwrap();
561        assert!(again.filed.is_empty());
562        assert_eq!(q.list().len(), 2);
563        // Even with the run's own record lost, the queue says what exists.
564        s.followups.clear();
565        let third = file(&mut s, "u", &q).unwrap();
566        assert!(third.filed.is_empty());
567        assert_eq!(q.list().len(), 2);
568        assert_eq!(s.followups.len(), 2, "records are restored from the queue");
569    }
570
571    #[test]
572    fn a_clean_merge_files_nothing() {
573        let dir = tempfile::tempdir().unwrap();
574        let q = Queue::at(dir.path().join("queue"));
575        let mut s = merged(vec![record(
576            1,
577            vec![finding("R1-1-1", Severity::Minor, "a.rs", 1)],
578            ReviewVote::Approve,
579        )]);
580        let out = file(&mut s, "u", &q).unwrap();
581        assert!(out.filed.is_empty());
582        assert!(q.list().is_empty());
583        assert!(s.followups.is_empty());
584    }
585
586    #[test]
587    fn a_failing_queue_is_an_error_event_not_a_status_change() {
588        let dir = tempfile::tempdir().unwrap();
589        let root = dir.path().join("queue");
590        std::fs::write(&root, "not a directory").unwrap();
591        let q = Queue::at(root);
592        let mut s = merged(two_seats());
593        s.status = crate::run::RunStatus::Merged;
594        let _ = file(&mut s, "u", &q);
595        assert_eq!(s.status, crate::run::RunStatus::Merged);
596        assert!(
597            s.events
598                .iter()
599                .any(|e| e.node == NODE && e.message.contains("stopped"))
600        );
601        assert!(s.followups.is_empty());
602    }
603
604    #[test]
605    fn the_generation_cap_stops_the_chain() {
606        let dir = tempfile::tempdir().unwrap();
607        let q = Queue::at(dir.path().join("queue"));
608        let mut parent = Task::new(
609            "p".to_owned(),
610            "i".to_owned(),
611            PathBuf::from("/repo"),
612            Source::Human,
613        );
614        parent.followup = Some(FollowUp {
615            run: "r0".to_owned(),
616            origin_task: None,
617            pr: "u".to_owned(),
618            findings: vec!["R1-1-1".to_owned()],
619            generation: MAX_FOLLOWUP_GENERATION,
620        });
621        q.put(&mut parent).unwrap();
622        let mut s = merged(two_seats());
623        s.origin = Some(Origin {
624            by: crate::run::StartedBy::Operator,
625            task: Some(parent.id.clone()),
626        });
627        s.origin = Some(Origin {
628            by: crate::run::StartedBy::Operator,
629            task: Some("gone".to_owned()),
630        });
631        s.followup_generation = Some(MAX_FOLLOWUP_GENERATION);
632        let out = file(&mut s, "u", &q).unwrap();
633        assert!(out.filed.is_empty());
634        assert!(!out.capped.is_empty());
635        assert_eq!(q.list().len(), 1);
636        assert!(
637            s.events
638                .iter()
639                .any(|e| e.message.contains("generation cap"))
640        );
641    }
642
643    #[test]
644    fn a_deeper_task_files_one_generation_further() {
645        let dir = tempfile::tempdir().unwrap();
646        let q = Queue::at(dir.path().join("queue"));
647        let mut parent = Task::new(
648            "p".to_owned(),
649            "i".to_owned(),
650            PathBuf::from("/repo"),
651            Source::Human,
652        );
653        q.put(&mut parent).unwrap();
654        let mut s = merged(two_seats());
655        s.origin = Some(Origin {
656            by: crate::run::StartedBy::Operator,
657            task: Some(parent.id.clone()),
658        });
659        file(&mut s, "u", &q).unwrap();
660        let t = q.list().into_iter().find(|t| t.followup.is_some()).unwrap();
661        assert_eq!(t.followup.unwrap().origin_task, Some(parent.id));
662    }
663
664    const PR: &str = "https://github.com/o/r/pull/473";
665
666    fn manual(text: &str) -> Task {
667        Task::new(
668            "manual".to_owned(),
669            text.to_owned(),
670            PathBuf::from("/repo"),
671            Source::Human,
672        )
673    }
674
675    /// File follow-ups with one manual task already in the queue.
676    fn filed_with(text: &str) -> (Outcome, RunState, Queue, String, tempfile::TempDir) {
677        let dir = tempfile::tempdir().unwrap();
678        let q = Queue::at(dir.path().join("queue"));
679        let mut t = manual(text);
680        t.status = crate::queue::TaskStatus::Done;
681        q.put(&mut t).unwrap();
682        let mut s = merged(two_seats());
683        let out = file(&mut s, PR, &q).unwrap();
684        (out, s, q, t.id, dir)
685    }
686
687    #[test]
688    fn a_task_naming_the_pr_url_and_ids_covers_the_group() {
689        let (out, s, _q, id, _d) = filed_with(&format!("Fix R3-1-1 and R3-2-1 from {PR}"));
690        // the waiter.rs group is covered; the other seat-1 Minor is still filed
691        assert_eq!(out.filed.len(), 1, "{out:?}");
692        assert_eq!(
693            out.covered,
694            vec![
695                ("R3-1-1".to_owned(), id.clone()),
696                ("R3-2-1".to_owned(), id.clone())
697            ]
698        );
699        assert!(out.capped.is_empty());
700        assert!(
701            s.events
702                .iter()
703                .any(|e| e.message.contains("already covered"))
704        );
705        let body = comment_body(&s, &out);
706        assert!(body.contains(&format!("R3-1-1: already covered by task `{id}`")));
707    }
708
709    #[test]
710    fn the_hash_form_covers_but_a_prefix_number_does_not() {
711        let (out, ..) = filed_with("R3-1-1 R3-2-1 fixed in PR #473");
712        assert_eq!(out.covered.len(), 2);
713        let (out, ..) = filed_with("R3-1-1 R3-2-1 fixed in #47");
714        assert!(out.covered.is_empty());
715        assert_eq!(out.filed.len(), 2);
716        let (out, ..) = filed_with("R3-1-1 R3-2-1 fixed in #4731");
717        assert!(out.covered.is_empty());
718    }
719
720    #[test]
721    fn an_id_without_the_pr_does_not_cover() {
722        let (out, ..) = filed_with("R3-1-1 and R3-2-1 are bad");
723        assert!(out.covered.is_empty());
724        assert_eq!(out.filed.len(), 2);
725    }
726
727    #[test]
728    fn an_id_prefix_does_not_cover() {
729        let (out, ..) = filed_with(&format!("R3-1-10 R3-2-10 in {PR}"));
730        assert!(out.covered.is_empty());
731    }
732
733    #[test]
734    fn partial_coverage_still_files() {
735        let (out, ..) = filed_with(&format!("R3-1-1 in {PR}"));
736        assert!(out.covered.is_empty());
737        assert_eq!(out.filed.len(), 2);
738    }
739
740    #[test]
741    fn another_followups_findings_cover() {
742        let dir = tempfile::tempdir().unwrap();
743        let q = Queue::at(dir.path().join("queue"));
744        let mut s = merged(two_seats());
745        let mut other = manual("x");
746        other.followup = Some(FollowUp {
747            run: s.id.clone(),
748            origin_task: None,
749            pr: PR.to_owned(),
750            findings: vec!["R3-1-1".to_owned(), "R3-2-1".to_owned()],
751            generation: 1,
752        });
753        q.put(&mut other).unwrap();
754        let out = file(&mut s, PR, &q).unwrap();
755        assert_eq!(out.covered.len(), 2, "{out:?}");
756        assert_eq!(out.filed.len(), 1);
757    }
758
759    #[test]
760    fn a_fully_covered_comment_lists_only_the_covered() {
761        let (out, mut s, ..) = filed_with(&format!("R3-1-1 R3-2-1 {PR}"));
762        s.followups.clear();
763        let body = comment_body(&s, &out);
764        assert!(body.contains("already covered by task"));
765        assert!(!body.contains("filed as follow-up"));
766    }
767
768    #[test]
769    fn an_id_next_to_japanese_prose_still_covers() {
770        let (out, ..) = filed_with("PR #473 の R3-1-1を修正、R3-2-1も対応");
771        assert_eq!(out.covered.len(), 2, "{out:?}");
772    }
773}