use std::collections::BTreeSet;
use std::ops::Range;
use std::path::Path;
use std::sync::LazyLock;
use regex::Regex;
use crate::agent::Agent;
use crate::config::{Config, Followups};
use crate::error::Result;
use crate::model::{Finding, ScreenResponse, ScreenVerdict, Screened};
use crate::repo::{Repo, FOLLOWUP_MARKER};
use crate::{log, logwarn, schema, spar_err};
static BLANK_RUN: LazyLock<Regex> =
LazyLock::new(|| Regex::new(r"\n{3,}").expect("blank run pattern"));
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Entry {
pub title: String,
pub body: String,
pub span: Range<usize>,
}
fn report_headings() -> Vec<&'static str> {
Finding {
problem: Some("x".into()),
reproduction: Some("x".into()),
impact: Some("x".into()),
expected: Some("x".into()),
..Finding::default()
}
.report_sections()
.into_iter()
.map(|(heading, _)| heading)
.collect()
}
fn is_section_heading(text: &str) -> bool {
let got = text.trim().trim_end_matches(':').trim().to_lowercase();
report_headings().iter().any(|h| h.to_lowercase() == got)
|| matches!(
got.as_str(),
"expected behaviour"
| "expected"
| "actual result"
| "actual results"
| "actual behavior"
| "actual behaviour"
| "steps to reproduce"
| "summary"
)
}
fn lines_with_offsets(text: &str) -> impl Iterator<Item = (usize, &str)> {
let mut at = 0usize;
text.split_inclusive('\n').map(move |line| {
let start = at;
at += line.len();
(start, line.trim_end_matches(['\n', '\r']))
})
}
pub fn parse(text: &str) -> Vec<Entry> {
let mut opens: Vec<(usize, Option<usize>)> = Vec::new();
let mut open = false;
let mut awaiting_title = false;
let mut fenced = false;
for (offset, line) in lines_with_offsets(text) {
let trimmed = line.trim_start();
if trimmed.starts_with("```") || trimmed.starts_with("~~~") {
fenced = !fenced;
continue;
}
if fenced {
continue;
}
if trimmed.starts_with(FOLLOWUP_MARKER) {
opens.push((offset, None));
open = true;
awaiting_title = true;
continue;
}
let Some(heading) = trimmed.strip_prefix("## ") else {
continue;
};
if awaiting_title {
if let Some(last) = opens.last_mut() {
last.1 = Some(offset);
}
awaiting_title = false;
continue;
}
if open && is_section_heading(heading) {
continue;
}
opens.push((offset, Some(offset)));
open = true;
}
let mut out = Vec::with_capacity(opens.len());
for (i, (start, title_at)) in opens.iter().enumerate() {
let end = opens.get(i + 1).map(|(s, _)| *s).unwrap_or(text.len());
let (title, body_from) = match title_at {
Some(at) => {
let line_end = text[*at..end].find('\n').map(|n| at + n + 1).unwrap_or(end);
let heading = text[*at..line_end]
.trim()
.trim_start_matches("## ")
.trim()
.to_string();
(heading, line_end)
}
None => (String::new(), *start),
};
out.push(Entry {
title,
body: text[body_from..end].trim().to_string(),
span: *start..end,
});
}
out
}
pub fn without(text: &str, removed: &[Entry]) -> String {
let mut spans: Vec<Range<usize>> = removed.iter().map(|e| e.span.clone()).collect();
spans.sort_by_key(|s| s.start);
let mut out = String::with_capacity(text.len());
let mut cursor = 0usize;
for span in spans {
if span.start < cursor {
cursor = cursor.max(span.end);
continue;
}
out.push_str(&text[cursor..span.start]);
cursor = span.end;
}
out.push_str(&text[cursor..]);
let joined = BLANK_RUN.replace_all(out.trim_end(), "\n\n").to_string();
if joined.trim().is_empty() {
String::new()
} else {
format!("{joined}\n")
}
}
const SCREEN_PROMPT: &str = "\
Below are follow-ups recorded against this repository while other work was going
on. Each was a real finding when it was written. Time has passed and the code has
moved: some are already fixed, some describe behaviour that no longer exists, and
some were never worth the interruption.
Read the code in your working directory before judging each one. Do not modify
anything. The current checkout is what \"now\" means. Judge against it, not
against what the entry says the code used to do.
For each entry decide:
- verdict: still_relevant, already_fixed, not_worth_it, or duplicate.
- still_relevant: the defect is still there. It becomes a GitHub issue.
- already_fixed: go and look. Name the function or the change that fixed it,
so somebody reading this can check you.
- not_worth_it: real, still there, and not worth a maintainer's queue.
- duplicate: an open issue, or an earlier entry in this list, already covers
it. Put that number in duplicate_of.
- reason: one sentence. For anything but still_relevant this is the only record
of why the entry was dropped, so give the reason rather than the verdict
restated.
- title: the entry's title, which becomes the issue title. Copy it across unless
it is wrong or says nothing.
Say still_relevant when you are unsure. What survives is triaged by both agents
afterwards and can still be declined there. What you drop here is dropped.
Entries:
";
struct Rendered {
text: String,
deferred: usize,
}
fn render(entries: &[Entry], cfg: &Config) -> Rendered {
let mut parts: Vec<String> = Vec::new();
let mut total = 0usize;
let mut deferred = 0usize;
for (i, entry) in entries.iter().enumerate() {
if deferred > 0 {
deferred += 1;
continue;
}
let block = format!("{}. {}\n{}", i + 1, entry.title, entry.body);
let len = block.chars().count();
if !parts.is_empty() && total + len > cfg.loop_cfg.max_triage_chars {
deferred += 1;
continue;
}
total += len;
parts.push(block);
}
Rendered {
text: parts.join("\n\n"),
deferred,
}
}
pub fn screen(
agent: &Agent,
cfg: &Config,
repo: &Repo,
entries: &[Entry],
) -> Result<Vec<ScreenVerdict>> {
let rendered = render(entries, cfg);
if rendered.deferred > 0 {
logwarn!(
"the queue did not fit in one screening prompt, so {} entry(s) were left in the file \
for a later run",
rendered.deferred
);
}
let prompt = format!("{SCREEN_PROMPT}{}", rendered.text);
let effort = cfg.effort_for_round(&agent.spec, 1);
let answer: ScreenResponse =
agent.ask_json(&prompt, &schema::screen(), repo.root(), effort.as_deref())?;
Ok(answer.entries)
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum Mode {
ScreenOnly,
FileOnly,
Work,
}
#[derive(Debug, Default)]
pub struct Outcome {
pub issues: Vec<i64>,
pub held: usize,
pub failed: usize,
}
impl Outcome {
pub fn exit_code(&self) -> i32 {
if self.failed > 0 {
1
} else {
0
}
}
}
pub fn run(
agents: &[Agent],
cfg: &Config,
repo: &Repo,
path: &Path,
limit: usize,
mode: Mode,
) -> Result<Outcome> {
let mut outcome = Outcome::default();
let Ok(original) = std::fs::read_to_string(path) else {
log!("no follow-ups recorded in {}", path.display());
if repo.followups != Followups::Local {
log!(
"followups = \"{}\" is configured, so nothing is written to that file.",
repo.followups
);
}
return Ok(outcome);
};
if original.trim().is_empty() {
log!("{} is there and empty", path.display());
return Ok(outcome);
}
let entries = parse(&original);
if entries.is_empty() {
log!(
"{} has no `## ` headings, so there is nothing to work. An entry is a `## Title` line \
and the text under it.",
path.display()
);
return Ok(outcome);
}
let taken: Vec<Entry> = entries.iter().take(limit).cloned().collect();
outcome.held += entries.len() - taken.len();
if outcome.held > 0 {
log!(
"{} follow-up(s) recorded, taking the first {limit}. Raise --limit for the rest.",
entries.len()
);
}
let agent = crate::agent::find(agents, &cfg.first_implementor)?;
log!(
"screening {} follow-up(s) with {} against {} at {}",
taken.len(),
agent.name(),
repo.git_try(&["rev-parse", "--abbrev-ref", "HEAD"]).trim(),
repo.git_try(&["rev-parse", "--short", "HEAD"]).trim(),
);
let verdicts = screen(agent, cfg, repo, &taken)?;
if mode == Mode::ScreenOnly {
print_verdicts(&taken, &verdicts);
return Ok(outcome);
}
let mut disposed: Vec<Entry> = Vec::new();
for (i, entry) in taken.iter().enumerate() {
let number = i as i64 + 1;
let Some(verdict) = verdicts.iter().find(|v| v.entry == number) else {
logwarn!(
"no verdict for '{}', leaving it in the file",
first_line(&entry.title)
);
outcome.held += 1;
continue;
};
let files = verdict.verdict == Screened::StillRelevant
|| (verdict.verdict == Screened::Duplicate && verdict.duplicate_of.is_none());
if files {
let title = if verdict.title.trim().is_empty() {
entry.title.as_str()
} else {
verdict.title.as_str()
};
match crate::review::file_as_issue(repo, title, &entry.body) {
Ok(filed) => {
log!(" {}", filed.describe(title));
if let Some(n) = filed.number() {
outcome.issues.push(n);
}
repo.archive_followup(title, &entry.body, &format!("Filed: {}", filed.note()));
}
Err(e) => {
logwarn!("could not file '{}': {e}", first_line(title));
outcome.held += 1;
outcome.failed += 1;
continue;
}
}
} else {
let why = dropped_note(verdict);
log!(" dropped '{}': {why}", first_line(&entry.title));
repo.archive_followup(&entry.title, &entry.body, &format!("Dropped: {why}"));
}
disposed.push(entry.clone());
crate::repo::write_text_atomic(path, &without(&original, &disposed)).map_err(|e| {
spar_err!(
"{e}\n{} follow-up(s) were already dealt with. Remove them from {} by hand before \
running this again, or they will be filed twice.",
disposed.len(),
path.display()
)
})?;
}
let filed = outcome.issues.len();
println!(
"\nfollowups: {} screened, {filed} filed{}",
taken.len(),
summarise(&taken, &verdicts)
);
if outcome.held > 0 {
println!("{} entry(s) left in {}", outcome.held, path.display());
}
if !disposed.is_empty() {
println!(
"what was dealt with is in {}",
repo.worked_followups_path().display()
);
}
Ok(outcome)
}
fn first_line(text: &str) -> String {
crate::style::clip(text.trim().lines().next().unwrap_or("").trim(), 80)
}
fn dropped_note(v: &ScreenVerdict) -> String {
let reason = v.reason.trim();
match (v.verdict, v.duplicate_of) {
(Screened::Duplicate, Some(n)) if reason.is_empty() => format!("#{n} already covers it"),
(Screened::Duplicate, Some(n)) => format!("#{n} already covers it. {reason}"),
(_, _) if reason.is_empty() => v.verdict.to_string(),
_ => format!("{}. {reason}", v.verdict),
}
}
fn summarise(taken: &[Entry], verdicts: &[ScreenVerdict]) -> String {
let mut counts: Vec<(Screened, usize)> = Vec::new();
for v in verdicts {
if v.entry < 1 || v.entry as usize > taken.len() {
continue;
}
match counts.iter_mut().find(|(k, _)| *k == v.verdict) {
Some((_, n)) => *n += 1,
None => counts.push((v.verdict, 1)),
}
}
counts.retain(|(k, _)| *k != Screened::StillRelevant);
if counts.is_empty() {
return String::new();
}
let listed: Vec<String> = counts
.iter()
.map(|(k, n)| format!("{n} {}", k.as_str().replace('_', " ")))
.collect();
format!(", {}", listed.join(", "))
}
fn print_verdicts(taken: &[Entry], verdicts: &[ScreenVerdict]) {
println!();
for (i, entry) in taken.iter().enumerate() {
let number = i as i64 + 1;
match verdicts.iter().find(|v| v.entry == number) {
Some(v) => println!(
" {:<14} {}\n {}",
v.verdict.as_str(),
first_line(&entry.title),
v.reason.trim()
),
None => println!(" {:<14} {}", "no verdict", first_line(&entry.title)),
}
}
let filed = verdicts
.iter()
.filter(|v| v.verdict == Screened::StillRelevant)
.count();
println!(
"\n{filed} of {} would be filed. Nothing was written.",
taken.len()
);
}
pub fn wave(outcome: &Outcome) -> Vec<i64> {
outcome
.issues
.iter()
.copied()
.collect::<BTreeSet<_>>()
.into_iter()
.collect()
}
#[cfg(test)]
mod tests {
use super::*;
const REAL: &str = "\
## Backend headers never drive commitment CPFP retries
The production ChainWatcher advances monitors and emits block.
## Problem
Configured chain backends route accepted headers through handleNewBlock.
## Reproduction
1. Configure a node with a watcher backend.
2. Deliver height 101.
## Impact
Nodes do not retry stuck commitment packages on new blocks.
## Expected behavior
Run the pass exactly once for each accepted backend header.
Found while working on #589.
## Overlapping scans can move a recorded spend height backward
## Problem
checkOutputSpend applies its result with no arbitration against a later scan.
## Impact
A stale verdict can overwrite a newer one.
Found while working on #590.
";
#[test]
fn an_entry_and_its_sections_are_not_confused_for_each_other() {
let entries = parse(REAL);
assert_eq!(
2,
entries.len(),
"{:#?}",
entries.iter().map(|e| &e.title).collect::<Vec<_>>()
);
assert!(entries[0].title.starts_with("Backend headers"));
assert!(entries[1].title.starts_with("Overlapping scans"));
assert!(
entries[0].body.contains("## Reproduction"),
"{}",
entries[0].body
);
assert!(entries[0].body.contains("Found while working on #589."));
}
#[test]
fn a_marker_makes_the_boundary_exact() {
let text = format!(
"{FOLLOWUP_MARKER}\n## Impact\n\nThe first one.\n\n\
{FOLLOWUP_MARKER}\n## Problem\n\nThe second one.\n"
);
let entries = parse(&text);
assert_eq!(2, entries.len());
assert_eq!("Impact", entries[0].title);
assert_eq!("Problem", entries[1].title);
}
#[test]
fn a_hand_written_file_with_no_markers_still_parses() {
let text =
"## One thing\n\nprose\n\n## Another thing\n\nmore prose\n\n## A third\n\nyet more\n";
let entries = parse(text);
assert_eq!(3, entries.len());
assert_eq!("Another thing", entries[1].title);
}
#[test]
fn a_heading_inside_a_fenced_block_does_not_start_an_entry() {
let text = "## Real title\n\n```md\n## Problem\n## Not a title either\n```\n\nprose\n";
let entries = parse(text);
assert_eq!(1, entries.len(), "{:?}", entries);
assert_eq!("Real title", entries[0].title);
}
#[test]
fn an_entry_whose_title_opens_with_a_section_word_is_still_a_title() {
let text =
"## First\n\nprose\n\n## Reproduction steps are missing from the docs\n\nprose\n";
assert_eq!(2, parse(text).len());
}
#[test]
fn the_section_list_covers_every_heading_a_report_writes() {
for heading in report_headings() {
assert!(
is_section_heading(heading),
"`## {heading}` would be read as the start of a new follow-up"
);
}
}
#[test]
fn text_the_parser_does_not_own_survives_a_rewrite() {
let text = "A note I keep at the top.\n\n\
## One\n\nfirst\n\n\
## Two\n\nsecond\n\n\
## Three\n\nthird\n";
let entries = parse(text);
assert_eq!(3, entries.len());
let out = without(text, &[entries[1].clone()]);
assert!(out.starts_with("A note I keep at the top."), "{out}");
assert!(out.contains("## One"), "{out}");
assert!(!out.contains("## Two"), "{out}");
assert!(out.contains("## Three"), "{out}");
assert!(out.contains("third"), "{out}");
}
#[test]
fn removing_entries_one_at_a_time_matches_removing_them_at_once() {
let entries = parse(REAL);
let all_at_once = without(REAL, &entries);
let mut done = Vec::new();
let mut last = String::new();
for entry in &entries {
done.push(entry.clone());
last = without(REAL, &done);
}
assert_eq!(all_at_once, last);
assert!(last.is_empty(), "{last:?}");
}
#[test]
fn without_tolerates_a_repeated_or_unordered_span() {
let entries = parse(REAL);
let once = without(REAL, &[entries[0].clone()]);
let twice = without(REAL, &[entries[0].clone(), entries[0].clone()]);
assert_eq!(once, twice);
let forwards = without(REAL, &[entries[0].clone(), entries[1].clone()]);
let backwards = without(REAL, &[entries[1].clone(), entries[0].clone()]);
assert_eq!(forwards, backwards);
}
#[test]
fn removing_every_entry_leaves_an_empty_file() {
let entries = parse(REAL);
assert_eq!("", without(REAL, &entries));
}
#[test]
fn an_entry_keeps_the_provenance_it_was_written_with() {
let entries = parse(REAL);
assert!(entries[1].body.ends_with("Found while working on #590."));
}
#[test]
fn crlf_line_endings_parse_the_same_as_lf() {
let lf = "## One\n\nfirst\n\n## Two\n\nsecond\n";
let crlf = lf.replace('\n', "\r\n");
let a = parse(lf);
let b = parse(&crlf);
assert_eq!(a.len(), b.len());
assert_eq!(a[1].title, b[1].title);
}
#[test]
fn a_file_that_opens_with_a_section_name_still_holds_an_entry() {
let entries = parse("## Problem\n\nsomething is wrong\n");
assert_eq!(1, entries.len());
assert_eq!("Problem", entries[0].title);
}
fn verdict(entry: i64, v: Screened, dup: Option<i64>) -> ScreenVerdict {
ScreenVerdict {
entry,
verdict: v,
title: String::new(),
reason: "because".into(),
duplicate_of: dup,
}
}
#[test]
fn a_duplicate_verdict_with_nothing_to_point_at_would_still_be_filed() {
let with_number = verdict(1, Screened::Duplicate, Some(412));
let without_number = verdict(1, Screened::Duplicate, None);
let files = |v: &ScreenVerdict| {
v.verdict == Screened::StillRelevant
|| (v.verdict == Screened::Duplicate && v.duplicate_of.is_none())
};
assert!(!files(&with_number));
assert!(files(&without_number));
}
#[test]
fn an_entry_with_no_verdict_is_not_disposed_of() {
let entries = parse(REAL);
let verdicts = [verdict(1, Screened::AlreadyFixed, None)];
let unruled: Vec<usize> = (1..=entries.len())
.filter(|n| !verdicts.iter().any(|v| v.entry == *n as i64))
.collect();
assert_eq!(vec![2], unruled);
}
#[test]
fn a_verdict_naming_an_entry_that_does_not_exist_is_ignored() {
let entries = parse(REAL);
let verdicts = [verdict(9, Screened::AlreadyFixed, None)];
assert_eq!("", summarise(&entries, &verdicts));
}
#[test]
fn the_summary_names_each_verdict_that_dropped_something() {
let entries = parse(REAL);
let verdicts = vec![
verdict(1, Screened::AlreadyFixed, None),
verdict(2, Screened::StillRelevant, None),
];
let out = summarise(&entries, &verdicts);
assert!(out.contains("1 already fixed"), "{out}");
assert!(!out.contains("still relevant"), "{out}");
}
#[test]
fn a_dropped_entry_carries_its_reason_and_the_issue_it_duplicates() {
let note = dropped_note(&verdict(1, Screened::Duplicate, Some(412)));
assert!(note.contains("#412"), "{note}");
assert!(note.contains("because"), "{note}");
}
}
#[cfg(test)]
mod real_file {
use super::*;
const CORPUS: &str = include_str!("../tests/fixtures/local_followups.md");
#[test]
fn the_real_queue_parses_as_five_follow_ups_not_twenty_five() {
let entries = parse(CORPUS);
assert_eq!(
5,
entries.len(),
"{:#?}",
entries.iter().map(|e| e.title.as_str()).collect::<Vec<_>>()
);
for entry in &entries {
assert!(
!is_section_heading(&entry.title),
"a section was filed as a follow-up: {}",
entry.title
);
assert!(!entry.body.trim().is_empty(), "{} has no body", entry.title);
}
}
#[test]
fn every_entry_in_the_real_queue_keeps_its_provenance() {
for entry in parse(CORPUS) {
assert!(
entry.body.contains("Found while working on #"),
"{} lost its provenance",
entry.title
);
}
}
#[test]
fn the_real_queue_drains_to_nothing_one_entry_at_a_time() {
let entries = parse(CORPUS);
let mut done = Vec::new();
let mut text = CORPUS.to_string();
for entry in &entries {
done.push(entry.clone());
text = without(CORPUS, &done);
}
assert_eq!("", text);
assert_eq!(without(CORPUS, &entries), text);
}
#[test]
fn draining_one_entry_leaves_the_rest_byte_for_byte() {
let entries = parse(CORPUS);
let out = without(CORPUS, &[entries[2].clone()]);
let left = parse(&out);
assert_eq!(4, left.len());
for (before, after) in [(0, 0), (1, 1), (3, 2), (4, 3)] {
assert_eq!(entries[before].title, left[after].title);
assert_eq!(entries[before].body, left[after].body);
}
}
}