1use 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#[derive(Debug, Clone, PartialEq, Eq)]
40pub struct Entry {
41 pub title: String,
43 pub body: String,
46 pub span: Range<usize>,
51}
52
53fn 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
72fn 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
94fn 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
104pub fn parse(text: &str) -> Vec<Entry> {
132 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 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 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
198pub 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 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
235const 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
269struct Rendered {
271 text: String,
272 deferred: usize,
275}
276
277fn 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 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
315pub 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#[derive(Debug, Clone, Copy, PartialEq, Eq)]
348pub enum Mode {
349 ScreenOnly,
351 FileOnly,
353 Work,
355}
356
357#[derive(Debug, Default)]
359pub struct Outcome {
360 pub issues: Vec<i64>,
362 pub held: usize,
364 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
378pub 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 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 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 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 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 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 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 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
540fn 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
563fn 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
590pub 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 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 #[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 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 #[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 #[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 #[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 #[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 #[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 #[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 #[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 #[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 #[test]
778 fn removing_every_entry_leaves_an_empty_file() {
779 let entries = parse(REAL);
780 assert_eq!("", without(REAL, &entries));
781 }
782
783 #[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 #[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 #[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 #[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 #[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 #[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 #[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 #[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 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 #[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 #[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 #[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}