From 088ac91d534e2f7a287759558c5cc810e1a2e847 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Fri, 20 Mar 2026 18:43:15 -0400 Subject: [PATCH] Unify goal prefix stripping into shared strip_goal_decoration() in fabro-util Three places stripped markdown headings and `Plan:` prefixes from goals with slightly different logic. Extract a shared function so all call sites behave consistently, and fix `fabro run` which wasn't stripping at all. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/crates/fabro-cli/src/commands/run.rs | 4 +- lib/crates/fabro-cli/src/commands/runs.rs | 5 +- lib/crates/fabro-util/src/lib.rs | 1 + lib/crates/fabro-util/src/text.rs | 60 +++++++++++++++++++ .../fabro-workflows/src/pull_request.rs | 20 ++----- 5 files changed, 70 insertions(+), 20 deletions(-) create mode 100644 lib/crates/fabro-util/src/text.rs diff --git a/lib/crates/fabro-cli/src/commands/run.rs b/lib/crates/fabro-cli/src/commands/run.rs index b15ff7681..cd53fb4dd 100644 --- a/lib/crates/fabro-cli/src/commands/run.rs +++ b/lib/crates/fabro-cli/src/commands/run.rs @@ -546,8 +546,8 @@ pub(crate) fn prepare_workflow( let goal = graph.goal(); if !goal.is_empty() { - let first_line = goal.lines().next().unwrap_or(goal); - eprintln!("{} {first_line}\n", styles.bold.apply_to("Goal:")); + let stripped = fabro_util::text::strip_goal_decoration(goal); + eprintln!("{} {stripped}\n", styles.bold.apply_to("Goal:")); } print_diagnostics(&diagnostics, styles); diff --git a/lib/crates/fabro-cli/src/commands/runs.rs b/lib/crates/fabro-cli/src/commands/runs.rs index 22632964e..2d02bdd42 100644 --- a/lib/crates/fabro-cli/src/commands/runs.rs +++ b/lib/crates/fabro-cli/src/commands/runs.rs @@ -227,10 +227,7 @@ fn short_run_id(id: &str) -> &str { } fn truncate_goal(goal: &str, max_len: usize) -> String { - let line = goal.lines().next().unwrap_or(""); - let line = line.trim_start_matches('#').trim(); - let line = line.strip_prefix("Plan:").map(|s| s.trim()).unwrap_or(line); - truncate_str(line, max_len) + truncate_str(fabro_util::text::strip_goal_decoration(goal), max_len) } fn truncate_str(s: &str, max_len: usize) -> String { diff --git a/lib/crates/fabro-util/src/lib.rs b/lib/crates/fabro-util/src/lib.rs index 1dc496e23..be19d465a 100644 --- a/lib/crates/fabro-util/src/lib.rs +++ b/lib/crates/fabro-util/src/lib.rs @@ -4,4 +4,5 @@ pub mod path; pub mod redact; pub mod run_log; pub mod terminal; +pub mod text; pub mod version; diff --git a/lib/crates/fabro-util/src/text.rs b/lib/crates/fabro-util/src/text.rs new file mode 100644 index 000000000..cf5cba883 --- /dev/null +++ b/lib/crates/fabro-util/src/text.rs @@ -0,0 +1,60 @@ +/// Strip markdown heading prefixes and `Plan:` prefix from a goal string. +/// +/// Takes the first line, removes all leading `#` characters, then strips +/// a `Plan:` prefix if present. Returns a trimmed `&str` slice. +pub fn strip_goal_decoration(goal: &str) -> &str { + let line = goal.lines().next().unwrap_or(""); + let line = line.trim_start_matches('#').trim(); + line.strip_prefix("Plan:").map(|s| s.trim()).unwrap_or(line) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn strips_h1() { + assert_eq!(strip_goal_decoration("# Title"), "Title"); + } + + #[test] + fn strips_h2() { + assert_eq!(strip_goal_decoration("## Fix bug"), "Fix bug"); + } + + #[test] + fn strips_h3() { + assert_eq!(strip_goal_decoration("### Deep heading"), "Deep heading"); + } + + #[test] + fn strips_plan_prefix() { + assert_eq!(strip_goal_decoration("Plan: do stuff"), "do stuff"); + } + + #[test] + fn strips_heading_and_plan_prefix() { + assert_eq!(strip_goal_decoration("## Plan: migrate DB"), "migrate DB"); + } + + #[test] + fn plain_text_unchanged() { + assert_eq!( + strip_goal_decoration("Fix the login bug"), + "Fix the login bug" + ); + } + + #[test] + fn takes_first_line() { + assert_eq!( + strip_goal_decoration("## Plan: First\n\nMore details"), + "First" + ); + } + + #[test] + fn empty_string() { + assert_eq!(strip_goal_decoration(""), ""); + } +} diff --git a/lib/crates/fabro-workflows/src/pull_request.rs b/lib/crates/fabro-workflows/src/pull_request.rs index 88e847d0e..dcd9666c6 100644 --- a/lib/crates/fabro-workflows/src/pull_request.rs +++ b/lib/crates/fabro-workflows/src/pull_request.rs @@ -33,20 +33,12 @@ impl PullRequestRecord { /// /// Uses the first line, truncated to 120 characters for readability. fn pr_title_from_goal(goal: &str) -> String { - let first_line = goal.lines().next().unwrap_or(goal); - let first_line = first_line - .strip_prefix("## ") - .or_else(|| first_line.strip_prefix("# ")) - .unwrap_or(first_line); - let first_line = first_line - .strip_prefix("Plan:") - .map(|s| s.trim()) - .unwrap_or(first_line); - if first_line.chars().count() > 120 { - let truncated: String = first_line.chars().take(119).collect(); + let stripped = fabro_util::text::strip_goal_decoration(goal); + if stripped.chars().count() > 120 { + let truncated: String = stripped.chars().take(119).collect(); format!("{truncated}…") } else { - first_line.to_string() + stripped.to_string() } } @@ -866,10 +858,10 @@ mod tests { } #[test] - fn pr_title_does_not_strip_h3_prefix() { + fn pr_title_strips_h3_prefix() { assert_eq!( pr_title_from_goal("### Add Draft PR Mode"), - "### Add Draft PR Mode" + "Add Draft PR Mode" ); }