Skip to main content

spar/
followups.rs

1//! The local follow-up queue, and working it.
2//!
3//! `.spar/followups.md` is what `followups = "local"` writes instead of filing
4//! on the tracker. Until now nothing read it back, so a review's out of scope
5//! findings accumulated in a file with no way to act on them. `spar followup`
6//! reads it, asks one agent whether each entry is still true of the code as it
7//! is now, files the survivors, and hands them to the same pipeline `spar run`
8//! uses, which means both agents still triage each one before anything is
9//! implemented.
10//!
11//! Most of the care here is in the parser, for one reason. An entry is written
12//! as `## <title>`, and `review::issue_report` writes that entry's own sections
13//! as `## Problem`, `## Reproduction`, `## Impact` and `## Expected behavior`,
14//! at the same heading level. A real file had 25 `##` lines and 5 entries, so a
15//! naive split would file twenty issues, four of them titled "Impact".
16
17use std::collections::BTreeSet;
18use std::ops::Range;
19use std::path::Path;
20use std::sync::LazyLock;
21
22use regex::Regex;
23
24use crate::agent::Agent;
25use crate::config::{Config, Followups};
26use crate::error::Result;
27use crate::model::{Finding, ScreenResponse, ScreenVerdict, Screened};
28use crate::repo::{Repo, FOLLOWUP_MARKER};
29use crate::{log, logwarn, schema, spar_err};
30
31static BLANK_RUN: LazyLock<Regex> =
32    LazyLock::new(|| Regex::new(r"\n{3,}").expect("blank run pattern"));
33
34// ---------------------------------------------------------------------------
35// The file format
36// ---------------------------------------------------------------------------
37
38/// One entry as it sits in the note file.
39#[derive(Debug, Clone, PartialEq, Eq)]
40pub struct Entry {
41    /// The `## ` heading text, marker excluded.
42    pub title: String,
43    /// Everything under the heading up to the next entry, trimmed. Keeps the
44    /// report sections and the `Found while working on #N.` line.
45    pub body: String,
46    /// Byte range in the source covering the whole entry, marker included.
47    ///
48    /// The rewrite removes spans out of the original text rather than
49    /// re-rendering what stays, which is what makes a hand edited file safe.
50    pub span: Range<usize>,
51}
52
53/// The headings `review::issue_report` writes inside an entry.
54///
55/// Taken from `Finding::report_sections` rather than written out again. A fifth
56/// section added there and forgotten here would split every entry that used it
57/// in two, and the second half would be filed as its own issue.
58fn report_headings() -> Vec<&'static str> {
59    Finding {
60        problem: Some("x".into()),
61        reproduction: Some("x".into()),
62        impact: Some("x".into()),
63        expected: Some("x".into()),
64        ..Finding::default()
65    }
66    .report_sections()
67    .into_iter()
68    .map(|(heading, _)| heading)
69    .collect()
70}
71
72/// Whether a `## ` heading names a section of a bug report rather than an
73/// entry's title.
74///
75/// Compares the whole heading, so `## Reproduction steps are missing` is a
76/// title and not a section. The extra names are what a model writing
77/// `new_issue_body` free hand produces, which the schema does not constrain.
78fn is_section_heading(text: &str) -> bool {
79    let got = text.trim().trim_end_matches(':').trim().to_lowercase();
80    report_headings().iter().any(|h| h.to_lowercase() == got)
81        || matches!(
82            got.as_str(),
83            "expected behaviour"
84                | "expected"
85                | "actual result"
86                | "actual results"
87                | "actual behavior"
88                | "actual behaviour"
89                | "steps to reproduce"
90                | "summary"
91        )
92}
93
94/// Every line with the byte offset it starts at.
95fn lines_with_offsets(text: &str) -> impl Iterator<Item = (usize, &str)> {
96    let mut at = 0usize;
97    text.split_inclusive('\n').map(move |line| {
98        let start = at;
99        at += line.len();
100        (start, line.trim_end_matches(['\n', '\r']))
101    })
102}
103
104/// Split a note file into entries.
105///
106/// Two rules, in order:
107///
108/// 1. A `<!-- spar:followup -->` line always starts an entry. spar writes one
109///    above every entry it appends, so a file written from this release on is
110///    read exactly rather than guessed at, and the first `## ` line after the
111///    marker is that entry's title rather than a boundary.
112/// 2. Otherwise a `## ` line starts an entry unless it names one of the
113///    sections a bug report is written in, or there is nothing open for it to
114///    be a section of.
115///
116/// Rule 2 is generous about what counts as a section, on purpose, because the
117/// two ways to be wrong are not the same size. Mistaking a section for a title
118/// splits one follow-up into four and files an issue called "Impact" carrying a
119/// fragment: visible, embarrassing, and on somebody else's tracker. Mistaking a
120/// title for a section merges two follow-ups into one issue that carries both:
121/// fat, and recoverable by reading it. Nothing is lost.
122///
123/// A heading inside a fenced code block is never a boundary. A body is free to
124/// carry a snippet, `style::issue_body` deliberately protects fenced blocks
125/// from the length budget, and a snippet containing a `## ` line would
126/// otherwise split the entry that quotes it.
127///
128/// Rejected as a third signal: "an entry follows a `Found while working on #N.`
129/// line". Provenance is not guaranteed, a hand written entry has none, and a
130/// parser that leans on prose is one an edit breaks.
131pub fn parse(text: &str) -> Vec<Entry> {
132    // Entry start, and where its `## ` title line starts if it has one.
133    let mut opens: Vec<(usize, Option<usize>)> = Vec::new();
134    let mut open = false;
135    let mut awaiting_title = false;
136    let mut fenced = false;
137
138    for (offset, line) in lines_with_offsets(text) {
139        let trimmed = line.trim_start();
140        if trimmed.starts_with("```") || trimmed.starts_with("~~~") {
141            fenced = !fenced;
142            continue;
143        }
144        if fenced {
145            continue;
146        }
147        if trimmed.starts_with(FOLLOWUP_MARKER) {
148            opens.push((offset, None));
149            open = true;
150            awaiting_title = true;
151            continue;
152        }
153        let Some(heading) = trimmed.strip_prefix("## ") else {
154            continue;
155        };
156        if awaiting_title {
157            if let Some(last) = opens.last_mut() {
158                last.1 = Some(offset);
159            }
160            awaiting_title = false;
161            continue;
162        }
163        // Nothing is open, so this cannot be a section of anything: a file
164        // whose first heading is `## Problem` holds one entry called Problem.
165        if open && is_section_heading(heading) {
166            continue;
167        }
168        opens.push((offset, Some(offset)));
169        open = true;
170    }
171
172    let mut out = Vec::with_capacity(opens.len());
173    for (i, (start, title_at)) in opens.iter().enumerate() {
174        let end = opens.get(i + 1).map(|(s, _)| *s).unwrap_or(text.len());
175        let (title, body_from) = match title_at {
176            Some(at) => {
177                let line_end = text[*at..end].find('\n').map(|n| at + n + 1).unwrap_or(end);
178                let heading = text[*at..line_end]
179                    .trim()
180                    .trim_start_matches("## ")
181                    .trim()
182                    .to_string();
183                (heading, line_end)
184            }
185            // A marker with no heading under it. Keep the entry rather than
186            // dropping it: the body is still somebody's finding.
187            None => (String::new(), *start),
188        };
189        out.push(Entry {
190            title,
191            body: text[body_from..end].trim().to_string(),
192            span: *start..end,
193        });
194    }
195    out
196}
197
198/// The text with these entries removed, blank lines closed up at the seams.
199///
200/// Removal is by byte span out of the text as it was parsed, never by
201/// re-rendering what stays. Anything in the file spar did not write, a preamble,
202/// an entry in a shape spar does not recognise, an edit somebody made to a body,
203/// comes back out exactly as it went in.
204///
205/// Spans are sorted and merged first, so a caller may pass them in any order and
206/// may pass the same one twice. That is what makes it correct to rewrite the
207/// file once per entry against the *original* text: splicing a splice would
208/// shift every later offset.
209pub fn without(text: &str, removed: &[Entry]) -> String {
210    let mut spans: Vec<Range<usize>> = removed.iter().map(|e| e.span.clone()).collect();
211    spans.sort_by_key(|s| s.start);
212
213    let mut out = String::with_capacity(text.len());
214    let mut cursor = 0usize;
215    for span in spans {
216        // Already covered by an earlier span. Skipping rather than slicing
217        // backwards, because a panic here would take the queue with it.
218        if span.start < cursor {
219            cursor = cursor.max(span.end);
220            continue;
221        }
222        out.push_str(&text[cursor..span.start]);
223        cursor = span.end;
224    }
225    out.push_str(&text[cursor..]);
226
227    let joined = BLANK_RUN.replace_all(out.trim_end(), "\n\n").to_string();
228    if joined.trim().is_empty() {
229        String::new()
230    } else {
231        format!("{joined}\n")
232    }
233}
234
235// ---------------------------------------------------------------------------
236// Screening
237// ---------------------------------------------------------------------------
238
239const SCREEN_PROMPT: &str = "\
240Below are follow-ups recorded against this repository while other work was going
241on. Each was a real finding when it was written. Time has passed and the code has
242moved: some are already fixed, some describe behaviour that no longer exists, and
243some were never worth the interruption.
244
245Read the code in your working directory before judging each one. Do not modify
246anything. The current checkout is what \"now\" means. Judge against it, not
247against what the entry says the code used to do.
248
249For each entry decide:
250- verdict: still_relevant, already_fixed, not_worth_it, or duplicate.
251  - still_relevant: the defect is still there. It becomes a GitHub issue.
252  - already_fixed: go and look. Name the function or the change that fixed it,
253    so somebody reading this can check you.
254  - not_worth_it: real, still there, and not worth a maintainer's queue.
255  - duplicate: an open issue, or an earlier entry in this list, already covers
256    it. Put that number in duplicate_of.
257- reason: one sentence. For anything but still_relevant this is the only record
258  of why the entry was dropped, so give the reason rather than the verdict
259  restated.
260- title: the entry's title, which becomes the issue title. Copy it across unless
261  it is wrong or says nothing.
262
263Say still_relevant when you are unsure. What survives is triaged by both agents
264afterwards and can still be declined there. What you drop here is dropped.
265
266Entries:
267";
268
269/// The queue as the prompt carries it, and what would not fit.
270struct Rendered {
271    text: String,
272    /// Left in the file for a later run, because the queue did not fit in one
273    /// prompt.
274    deferred: usize,
275}
276
277/// Render the queue under one budget.
278///
279/// The same shape as `triage::render` and for the same reason, which is worth
280/// saying rather than sharing: whole entries wait rather than every entry losing
281/// its tail, because a verdict here *deletes* the entry, so judging one on part
282/// of what it says is worse than not having reached it yet.
283///
284/// Unlike an issue body there is no per entry cut at all. Every entry already
285/// went through `style::issue_body` on the way in, and unlike an issue the entry
286/// is all there is.
287fn render(entries: &[Entry], cfg: &Config) -> Rendered {
288    let mut parts: Vec<String> = Vec::new();
289    let mut total = 0usize;
290    let mut deferred = 0usize;
291
292    for (i, entry) in entries.iter().enumerate() {
293        if deferred > 0 {
294            deferred += 1;
295            continue;
296        }
297        let block = format!("{}. {}\n{}", i + 1, entry.title, entry.body);
298        let len = block.chars().count();
299        // The first entry goes in whatever its size. A queue of one that does
300        // not fit is a command that does nothing, forever.
301        if !parts.is_empty() && total + len > cfg.loop_cfg.max_triage_chars {
302            deferred += 1;
303            continue;
304        }
305        total += len;
306        parts.push(block);
307    }
308
309    Rendered {
310        text: parts.join("\n\n"),
311        deferred,
312    }
313}
314
315/// One agent's verdict on every entry that fits in one prompt.
316///
317/// One call for the whole queue rather than one per entry. A repo aware call per
318/// entry is the dominant cost, and `duplicate` is a judgement across entries as
319/// well as against the tracker: an agent shown one entry at a time cannot say
320/// that this is the same defect as the one above it.
321pub fn screen(
322    agent: &Agent,
323    cfg: &Config,
324    repo: &Repo,
325    entries: &[Entry],
326) -> Result<Vec<ScreenVerdict>> {
327    let rendered = render(entries, cfg);
328    if rendered.deferred > 0 {
329        logwarn!(
330            "the queue did not fit in one screening prompt, so {} entry(s) were left in the file \
331             for a later run",
332            rendered.deferred
333        );
334    }
335    let prompt = format!("{SCREEN_PROMPT}{}", rendered.text);
336    let effort = cfg.effort_for_round(&agent.spec, 1);
337    let answer: ScreenResponse =
338        agent.ask_json(&prompt, &schema::screen(), repo.root(), effort.as_deref())?;
339    Ok(answer.entries)
340}
341
342// ---------------------------------------------------------------------------
343// Working the queue
344// ---------------------------------------------------------------------------
345
346/// Where a run of `spar followup` stops.
347#[derive(Debug, Clone, Copy, PartialEq, Eq)]
348pub enum Mode {
349    /// Print the verdicts and touch nothing.
350    ScreenOnly,
351    /// File the issues and stop, leaving them for a later `spar run`.
352    FileOnly,
353    /// File them and work them.
354    Work,
355}
356
357/// What was filed, and what was left behind.
358#[derive(Debug, Default)]
359pub struct Outcome {
360    /// Issues to work, in file order.
361    pub issues: Vec<i64>,
362    /// Left in the file: not reached, not screened, or could not be filed.
363    pub held: usize,
364    /// Could not be filed. A non-zero exit even when the pipeline went fine.
365    pub failed: usize,
366}
367
368impl Outcome {
369    pub fn exit_code(&self) -> i32 {
370        if self.failed > 0 {
371            1
372        } else {
373            0
374        }
375    }
376}
377
378/// Read the queue, screen it, file what still holds, and take the filed entries
379/// out of the file.
380pub fn run(
381    agents: &[Agent],
382    cfg: &Config,
383    repo: &Repo,
384    path: &Path,
385    limit: usize,
386    mode: Mode,
387) -> Result<Outcome> {
388    let mut outcome = Outcome::default();
389
390    // Before any network call, so an empty queue is a purely local no-op.
391    let Ok(original) = std::fs::read_to_string(path) else {
392        log!("no follow-ups recorded in {}", path.display());
393        if repo.followups != Followups::Local {
394            log!(
395                "followups = \"{}\" is configured, so nothing is written to that file.",
396                repo.followups
397            );
398        }
399        return Ok(outcome);
400    };
401    if original.trim().is_empty() {
402        log!("{} is there and empty", path.display());
403        return Ok(outcome);
404    }
405
406    let entries = parse(&original);
407    if entries.is_empty() {
408        // A parser problem must not be reported as an empty queue.
409        log!(
410            "{} has no `## ` headings, so there is nothing to work. An entry is a `## Title` line \
411             and the text under it.",
412            path.display()
413        );
414        return Ok(outcome);
415    }
416
417    let taken: Vec<Entry> = entries.iter().take(limit).cloned().collect();
418    outcome.held += entries.len() - taken.len();
419    if outcome.held > 0 {
420        log!(
421            "{} follow-up(s) recorded, taking the first {limit}. Raise --limit for the rest.",
422            entries.len()
423        );
424    }
425
426    let agent = crate::agent::find(agents, &cfg.first_implementor)?;
427    // An `already_fixed` verdict is uninterpretable without knowing what was
428    // being judged.
429    log!(
430        "screening {} follow-up(s) with {} against {} at {}",
431        taken.len(),
432        agent.name(),
433        repo.git_try(&["rev-parse", "--abbrev-ref", "HEAD"]).trim(),
434        repo.git_try(&["rev-parse", "--short", "HEAD"]).trim(),
435    );
436
437    // A screen that did not happen is not a screen that found nothing: nothing
438    // is filed and the file is not touched.
439    let verdicts = screen(agent, cfg, repo, &taken)?;
440
441    if mode == Mode::ScreenOnly {
442        print_verdicts(&taken, &verdicts);
443        return Ok(outcome);
444    }
445
446    let mut disposed: Vec<Entry> = Vec::new();
447    for (i, entry) in taken.iter().enumerate() {
448        let number = i as i64 + 1;
449        let Some(verdict) = verdicts.iter().find(|v| v.entry == number) else {
450            // Never read as "drop it". Filing something nobody looked at
451            // defeats the point of the screen; leaving it costs one line.
452            logwarn!(
453                "no verdict for '{}', leaving it in the file",
454                first_line(&entry.title)
455            );
456            outcome.held += 1;
457            continue;
458        };
459
460        // "Duplicate of nothing in particular" is unfalsifiable, and
461        // `find_similar_issue` on the filing path is exactly the check for it.
462        let files = verdict.verdict == Screened::StillRelevant
463            || (verdict.verdict == Screened::Duplicate && verdict.duplicate_of.is_none());
464
465        if files {
466            let title = if verdict.title.trim().is_empty() {
467                entry.title.as_str()
468            } else {
469                verdict.title.as_str()
470            };
471            match crate::review::file_as_issue(repo, title, &entry.body) {
472                Ok(filed) => {
473                    log!("  {}", filed.describe(title));
474                    if let Some(n) = filed.number() {
475                        outcome.issues.push(n);
476                    }
477                    repo.archive_followup(title, &entry.body, &format!("Filed: {}", filed.note()));
478                }
479                Err(e) => {
480                    logwarn!("could not file '{}': {e}", first_line(title));
481                    outcome.held += 1;
482                    outcome.failed += 1;
483                    continue;
484                }
485            }
486        } else {
487            let why = dropped_note(verdict);
488            log!("  dropped '{}': {why}", first_line(&entry.title));
489            repo.archive_followup(&entry.title, &entry.body, &format!("Dropped: {why}"));
490        }
491
492        disposed.push(entry.clone());
493        // Written after the entry is dealt with, never before. The window is
494        // one entry wide and it always falls on the side of filing twice rather
495        // than losing one: an entry filed and not yet removed is found again
496        // next run and matched to the issue that was just created, while an
497        // entry removed before it was filed is gone.
498        crate::repo::write_text_atomic(path, &without(&original, &disposed)).map_err(|e| {
499            spar_err!(
500                "{e}\n{} follow-up(s) were already dealt with. Remove them from {} by hand before \
501                 running this again, or they will be filed twice.",
502                disposed.len(),
503                path.display()
504            )
505        })?;
506    }
507
508    let filed = outcome.issues.len();
509    println!(
510        "\nfollowups: {} screened, {filed} filed{}",
511        taken.len(),
512        summarise(&taken, &verdicts)
513    );
514    if outcome.held > 0 {
515        println!("{} entry(s) left in {}", outcome.held, path.display());
516    }
517    if !disposed.is_empty() {
518        println!(
519            "what was dealt with is in {}",
520            repo.worked_followups_path().display()
521        );
522    }
523    Ok(outcome)
524}
525
526fn first_line(text: &str) -> String {
527    crate::style::clip(text.trim().lines().next().unwrap_or("").trim(), 80)
528}
529
530fn dropped_note(v: &ScreenVerdict) -> String {
531    let reason = v.reason.trim();
532    match (v.verdict, v.duplicate_of) {
533        (Screened::Duplicate, Some(n)) if reason.is_empty() => format!("#{n} already covers it"),
534        (Screened::Duplicate, Some(n)) => format!("#{n} already covers it. {reason}"),
535        (_, _) if reason.is_empty() => v.verdict.to_string(),
536        _ => format!("{}. {reason}", v.verdict),
537    }
538}
539
540/// The tail of the summary line, naming each verdict that dropped something.
541fn summarise(taken: &[Entry], verdicts: &[ScreenVerdict]) -> String {
542    let mut counts: Vec<(Screened, usize)> = Vec::new();
543    for v in verdicts {
544        if v.entry < 1 || v.entry as usize > taken.len() {
545            continue;
546        }
547        match counts.iter_mut().find(|(k, _)| *k == v.verdict) {
548            Some((_, n)) => *n += 1,
549            None => counts.push((v.verdict, 1)),
550        }
551    }
552    counts.retain(|(k, _)| *k != Screened::StillRelevant);
553    if counts.is_empty() {
554        return String::new();
555    }
556    let listed: Vec<String> = counts
557        .iter()
558        .map(|(k, n)| format!("{n} {}", k.as_str().replace('_', " ")))
559        .collect();
560    format!(", {}", listed.join(", "))
561}
562
563/// What `--screen-only` prints. `println!`, not `log!`: it is the whole output
564/// of the command, and an entry dropped with no visible record of why is
565/// exactly the failure the archive exists for.
566fn print_verdicts(taken: &[Entry], verdicts: &[ScreenVerdict]) {
567    println!();
568    for (i, entry) in taken.iter().enumerate() {
569        let number = i as i64 + 1;
570        match verdicts.iter().find(|v| v.entry == number) {
571            Some(v) => println!(
572                "  {:<14} {}\n                 {}",
573                v.verdict.as_str(),
574                first_line(&entry.title),
575                v.reason.trim()
576            ),
577            None => println!("  {:<14} {}", "no verdict", first_line(&entry.title)),
578        }
579    }
580    let filed = verdicts
581        .iter()
582        .filter(|v| v.verdict == Screened::StillRelevant)
583        .count();
584    println!(
585        "\n{filed} of {} would be filed. Nothing was written.",
586        taken.len()
587    );
588}
589
590/// The issue numbers a set of outcomes produced, deduplicated and in order.
591pub fn wave(outcome: &Outcome) -> Vec<i64> {
592    outcome
593        .issues
594        .iter()
595        .copied()
596        .collect::<BTreeSet<_>>()
597        .into_iter()
598        .collect()
599}
600
601#[cfg(test)]
602mod tests {
603    use super::*;
604
605    /// Two entries in the shape a real file has: a title, a lead paragraph,
606    /// the four report sections, and the provenance line.
607    const REAL: &str = "\
608## Backend headers never drive commitment CPFP retries
609
610The production ChainWatcher advances monitors and emits block.
611
612## Problem
613
614Configured chain backends route accepted headers through handleNewBlock.
615
616## Reproduction
617
6181. Configure a node with a watcher backend.
6192. Deliver height 101.
620
621## Impact
622
623Nodes do not retry stuck commitment packages on new blocks.
624
625## Expected behavior
626
627Run the pass exactly once for each accepted backend header.
628
629Found while working on #589.
630
631## Overlapping scans can move a recorded spend height backward
632
633## Problem
634
635checkOutputSpend applies its result with no arbitration against a later scan.
636
637## Impact
638
639A stale verdict can overwrite a newer one.
640
641Found while working on #590.
642";
643
644    /// The whole reason this module exists. A follow-up's own sections are
645    /// written at the same heading level as its title, so a naive split on
646    /// `## ` files six issues here, four of them called things like "Impact".
647    #[test]
648    fn an_entry_and_its_sections_are_not_confused_for_each_other() {
649        let entries = parse(REAL);
650        assert_eq!(
651            2,
652            entries.len(),
653            "{:#?}",
654            entries.iter().map(|e| &e.title).collect::<Vec<_>>()
655        );
656        assert!(entries[0].title.starts_with("Backend headers"));
657        assert!(entries[1].title.starts_with("Overlapping scans"));
658        // The sections stay with the entry they belong to.
659        assert!(
660            entries[0].body.contains("## Reproduction"),
661            "{}",
662            entries[0].body
663        );
664        assert!(entries[0].body.contains("Found while working on #589."));
665    }
666
667    /// The heading heuristic alone cannot see a title that collides with a
668    /// section name. The marker is what fixes that going forward, and spar
669    /// writes one above every entry it appends.
670    #[test]
671    fn a_marker_makes_the_boundary_exact() {
672        let text = format!(
673            "{FOLLOWUP_MARKER}\n## Impact\n\nThe first one.\n\n\
674             {FOLLOWUP_MARKER}\n## Problem\n\nThe second one.\n"
675        );
676        let entries = parse(&text);
677        assert_eq!(2, entries.len());
678        assert_eq!("Impact", entries[0].title);
679        assert_eq!("Problem", entries[1].title);
680    }
681
682    /// Keeping the queue by hand is a documented use, and it must not require
683    /// knowing spar's marker.
684    #[test]
685    fn a_hand_written_file_with_no_markers_still_parses() {
686        let text =
687            "## One thing\n\nprose\n\n## Another thing\n\nmore prose\n\n## A third\n\nyet more\n";
688        let entries = parse(text);
689        assert_eq!(3, entries.len());
690        assert_eq!("Another thing", entries[1].title);
691    }
692
693    /// A body is free to carry a snippet, and `style::issue_body` deliberately
694    /// protects fenced blocks from the length budget, so a snippet containing a
695    /// `## ` line would otherwise split the entry that quotes it.
696    #[test]
697    fn a_heading_inside_a_fenced_block_does_not_start_an_entry() {
698        let text = "## Real title\n\n```md\n## Problem\n## Not a title either\n```\n\nprose\n";
699        let entries = parse(text);
700        assert_eq!(1, entries.len(), "{:?}", entries);
701        assert_eq!("Real title", entries[0].title);
702    }
703
704    /// `is_section_heading` compares the whole heading. A prefix match would
705    /// swallow an entry whose title happens to open with a section word.
706    #[test]
707    fn an_entry_whose_title_opens_with_a_section_word_is_still_a_title() {
708        let text =
709            "## First\n\nprose\n\n## Reproduction steps are missing from the docs\n\nprose\n";
710        assert_eq!(2, parse(text).len());
711    }
712
713    /// A fifth section added to a bug report and forgotten here would split
714    /// every entry that used it, and the second half would be filed on its own.
715    #[test]
716    fn the_section_list_covers_every_heading_a_report_writes() {
717        for heading in report_headings() {
718            assert!(
719                is_section_heading(heading),
720                "`## {heading}` would be read as the start of a new follow-up"
721            );
722        }
723    }
724
725    /// A rewrite that re-renders what stays silently deletes whatever a person
726    /// added: a note at the top, an edit inside a body, a trailing reminder.
727    #[test]
728    fn text_the_parser_does_not_own_survives_a_rewrite() {
729        let text = "A note I keep at the top.\n\n\
730                    ## One\n\nfirst\n\n\
731                    ## Two\n\nsecond\n\n\
732                    ## Three\n\nthird\n";
733        let entries = parse(text);
734        assert_eq!(3, entries.len());
735        let out = without(text, &[entries[1].clone()]);
736        assert!(out.starts_with("A note I keep at the top."), "{out}");
737        assert!(out.contains("## One"), "{out}");
738        assert!(!out.contains("## Two"), "{out}");
739        assert!(out.contains("## Three"), "{out}");
740        assert!(out.contains("third"), "{out}");
741    }
742
743    /// The guard against splicing a splice. Spans index the text as it was
744    /// parsed, so removing one entry would shift every later offset, and the
745    /// run rewrites the file once per entry.
746    #[test]
747    fn removing_entries_one_at_a_time_matches_removing_them_at_once() {
748        let entries = parse(REAL);
749        let all_at_once = without(REAL, &entries);
750
751        let mut done = Vec::new();
752        let mut last = String::new();
753        for entry in &entries {
754            done.push(entry.clone());
755            last = without(REAL, &done);
756        }
757        assert_eq!(all_at_once, last);
758        assert!(last.is_empty(), "{last:?}");
759    }
760
761    /// A caller that recorded the same entry twice, or recorded them out of
762    /// order, must not corrupt the file or panic on a backwards slice.
763    #[test]
764    fn without_tolerates_a_repeated_or_unordered_span() {
765        let entries = parse(REAL);
766        let once = without(REAL, &[entries[0].clone()]);
767        let twice = without(REAL, &[entries[0].clone(), entries[0].clone()]);
768        assert_eq!(once, twice);
769
770        let forwards = without(REAL, &[entries[0].clone(), entries[1].clone()]);
771        let backwards = without(REAL, &[entries[1].clone(), entries[0].clone()]);
772        assert_eq!(forwards, backwards);
773    }
774
775    /// An emptied queue is an empty file, not a pile of blank lines that reads
776    /// as content to the next thing that opens it.
777    #[test]
778    fn removing_every_entry_leaves_an_empty_file() {
779        let entries = parse(REAL);
780        assert_eq!("", without(REAL, &entries));
781    }
782
783    /// The link back to the work that found a defect cannot be re-derived, so
784    /// the body has to carry it through unchanged.
785    #[test]
786    fn an_entry_keeps_the_provenance_it_was_written_with() {
787        let entries = parse(REAL);
788        assert!(entries[1].body.ends_with("Found while working on #590."));
789    }
790
791    /// A file edited on Windows must parse the same as one edited anywhere else.
792    #[test]
793    fn crlf_line_endings_parse_the_same_as_lf() {
794        let lf = "## One\n\nfirst\n\n## Two\n\nsecond\n";
795        let crlf = lf.replace('\n', "\r\n");
796        let a = parse(lf);
797        let b = parse(&crlf);
798        assert_eq!(a.len(), b.len());
799        assert_eq!(a[1].title, b[1].title);
800    }
801
802    /// A file whose first heading is a section name holds one entry called
803    /// that, rather than nothing at all.
804    #[test]
805    fn a_file_that_opens_with_a_section_name_still_holds_an_entry() {
806        let entries = parse("## Problem\n\nsomething is wrong\n");
807        assert_eq!(1, entries.len());
808        assert_eq!("Problem", entries[0].title);
809    }
810
811    fn verdict(entry: i64, v: Screened, dup: Option<i64>) -> ScreenVerdict {
812        ScreenVerdict {
813            entry,
814            verdict: v,
815            title: String::new(),
816            reason: "because".into(),
817            duplicate_of: dup,
818        }
819    }
820
821    /// "Duplicate of nothing in particular" is unfalsifiable, and the search on
822    /// the filing path is exactly the check for it. Dropping the entry on that
823    /// verdict would delete a real defect on no evidence.
824    #[test]
825    fn a_duplicate_verdict_with_nothing_to_point_at_would_still_be_filed() {
826        let with_number = verdict(1, Screened::Duplicate, Some(412));
827        let without_number = verdict(1, Screened::Duplicate, None);
828        let files = |v: &ScreenVerdict| {
829            v.verdict == Screened::StillRelevant
830                || (v.verdict == Screened::Duplicate && v.duplicate_of.is_none())
831        };
832        assert!(!files(&with_number));
833        assert!(files(&without_number));
834    }
835
836    /// A short answer must never read as "drop the rest". The entries the
837    /// screen did not rule on stay in the file.
838    #[test]
839    fn an_entry_with_no_verdict_is_not_disposed_of() {
840        let entries = parse(REAL);
841        let verdicts = [verdict(1, Screened::AlreadyFixed, None)];
842        let unruled: Vec<usize> = (1..=entries.len())
843            .filter(|n| !verdicts.iter().any(|v| v.entry == *n as i64))
844            .collect();
845        assert_eq!(vec![2], unruled);
846    }
847
848    /// An out of range index used as a slice index panics, and a verdict for an
849    /// entry that does not exist must not shift the ones that do.
850    #[test]
851    fn a_verdict_naming_an_entry_that_does_not_exist_is_ignored() {
852        let entries = parse(REAL);
853        let verdicts = [verdict(9, Screened::AlreadyFixed, None)];
854        assert_eq!("", summarise(&entries, &verdicts));
855    }
856
857    /// The summary is the only place a dropped entry is accounted for, and it
858    /// has to name what happened rather than only how many.
859    #[test]
860    fn the_summary_names_each_verdict_that_dropped_something() {
861        let entries = parse(REAL);
862        let verdicts = vec![
863            verdict(1, Screened::AlreadyFixed, None),
864            verdict(2, Screened::StillRelevant, None),
865        ];
866        let out = summarise(&entries, &verdicts);
867        assert!(out.contains("1 already fixed"), "{out}");
868        assert!(!out.contains("still relevant"), "{out}");
869    }
870
871    /// The reason is the only record of why an entry left the queue, so it has
872    /// to survive into the log line and the archive.
873    #[test]
874    fn a_dropped_entry_carries_its_reason_and_the_issue_it_duplicates() {
875        let note = dropped_note(&verdict(1, Screened::Duplicate, Some(412)));
876        assert!(note.contains("#412"), "{note}");
877        assert!(note.contains("because"), "{note}");
878    }
879}
880
881#[cfg(test)]
882mod real_file {
883    use super::*;
884
885    /// The queue one real run left on a real repository, captured verbatim.
886    /// Five follow-ups, twenty-five `## ` lines. This is the file the heuristic
887    /// is measured against rather than guessed at.
888    const CORPUS: &str = include_str!("../tests/fixtures/local_followups.md");
889
890    #[test]
891    fn the_real_queue_parses_as_five_follow_ups_not_twenty_five() {
892        let entries = parse(CORPUS);
893        assert_eq!(
894            5,
895            entries.len(),
896            "{:#?}",
897            entries.iter().map(|e| e.title.as_str()).collect::<Vec<_>>()
898        );
899        for entry in &entries {
900            assert!(
901                !is_section_heading(&entry.title),
902                "a section was filed as a follow-up: {}",
903                entry.title
904            );
905            assert!(!entry.body.trim().is_empty(), "{} has no body", entry.title);
906        }
907    }
908
909    /// Every entry in that file ends with the line that says which issue it
910    /// came out of, and an issue filed from it is the only place that link
911    /// survives.
912    #[test]
913    fn every_entry_in_the_real_queue_keeps_its_provenance() {
914        for entry in parse(CORPUS) {
915            assert!(
916                entry.body.contains("Found while working on #"),
917                "{} lost its provenance",
918                entry.title
919            );
920        }
921    }
922
923    /// Removing them one at a time, always against the original text, has to
924    /// end where removing them all at once does, and has to end empty.
925    #[test]
926    fn the_real_queue_drains_to_nothing_one_entry_at_a_time() {
927        let entries = parse(CORPUS);
928        let mut done = Vec::new();
929        let mut text = CORPUS.to_string();
930        for entry in &entries {
931            done.push(entry.clone());
932            text = without(CORPUS, &done);
933        }
934        assert_eq!("", text);
935        assert_eq!(without(CORPUS, &entries), text);
936    }
937
938    /// Taking the middle one out leaves the other four intact, including the
939    /// one a person would notice first if the seams were wrong.
940    #[test]
941    fn draining_one_entry_leaves_the_rest_byte_for_byte() {
942        let entries = parse(CORPUS);
943        let out = without(CORPUS, &[entries[2].clone()]);
944        let left = parse(&out);
945        assert_eq!(4, left.len());
946        for (before, after) in [(0, 0), (1, 1), (3, 2), (4, 3)] {
947            assert_eq!(entries[before].title, left[after].title);
948            assert_eq!(entries[before].body, left[after].body);
949        }
950    }
951}