diff --git a/hive-forge/src/verbs/labels.rs b/hive-forge/src/verbs/labels.rs index e3566d19..25b9c6c3 100644 --- a/hive-forge/src/verbs/labels.rs +++ b/hive-forge/src/verbs/labels.rs @@ -145,7 +145,7 @@ pub(crate) fn resolve_ids(all: &[Label], names: &[String]) -> Result> { let available: Vec<&str> = all.iter().filter_map(|l| l.name.as_deref()).collect(); bail!( "unresolved label name(s): {} — available labels: {}", - unresolved.join(", "), + crate::verbs::with_suggestions(&unresolved, &available), if available.is_empty() { "(none)".to_owned() } else { diff --git a/hive-forge/src/verbs/milestone.rs b/hive-forge/src/verbs/milestone.rs index ed1966c2..200007e0 100644 --- a/hive-forge/src/verbs/milestone.rs +++ b/hive-forge/src/verbs/milestone.rs @@ -100,7 +100,7 @@ pub(crate) fn ensure_filters_resolve(all: &[Milestone], values: &[String]) -> Re let available: Vec<&str> = all.iter().filter_map(|m| m.title.as_deref()).collect(); bail!( "unresolved milestone(s): {} — available milestones: {}", - unresolved.join(", "), + crate::verbs::with_suggestions(&unresolved, &available), if available.is_empty() { "(none)".to_owned() } else { diff --git a/hive-forge/src/verbs/mod.rs b/hive-forge/src/verbs/mod.rs index e86ed8ab..5a12797b 100644 --- a/hive-forge/src/verbs/mod.rs +++ b/hive-forge/src/verbs/mod.rs @@ -172,6 +172,61 @@ pub(crate) fn pct_encode(s: &str) -> String { out } +/// Render an unresolved-name list with a `did you mean` where one fits. +/// +/// A filter value that doesn't resolve is a **near-miss far more often +/// than an invention** (`area/opps` for `area/ops`), and an error that +/// only lists all 18 available names makes the reader do the diff by eye +/// — on the one occasion they already know they mistyped something. +pub(crate) fn with_suggestions(unresolved: &[&str], available: &[&str]) -> String { + unresolved + .iter() + .map(|u| match nearest(u, available) { + Some(s) => format!("{u} (did you mean \"{s}\"?)"), + None => (*u).to_owned(), + }) + .collect::>() + .join(", ") +} + +/// Closest candidate to `needle`, when one is close enough to be worth +/// suggesting. +/// +/// Thresholded rather than always returning the minimum: **a wrong +/// suggestion is worse than none**, because it invites a second failed +/// attempt at a name that was never there. The bound scales with the +/// needle (a third of its length, capped at 3) so a short name doesn't +/// match half the repo and a long one still tolerates a typo or two. +fn nearest<'a>(needle: &str, candidates: &[&'a str]) -> Option<&'a str> { + let limit = (needle.chars().count() / 3).clamp(1, 3); + candidates + .iter() + .map(|c| (edit_distance(needle, c), *c)) + .filter(|(d, _)| *d <= limit) + // Tie-break on the shorter candidate, then alphabetically, so the + // suggestion is stable rather than dependent on the order the + // forge happened to return its labels in. + .min_by_key(|(d, c)| (*d, c.len(), *c)) + .map(|(_, c)| c) +} + +/// Levenshtein distance, two-row DP. Small enough not to justify a +/// workspace dependency for the one place it is used. +fn edit_distance(a: &str, b: &str) -> usize { + let b: Vec = b.chars().collect(); + let mut prev: Vec = (0..=b.len()).collect(); + let mut cur = vec![0_usize; b.len() + 1]; + for (i, ca) in a.chars().enumerate() { + cur[0] = i + 1; + for (j, cb) in b.iter().enumerate() { + let cost = usize::from(ca != *cb); + cur[j + 1] = (prev[j] + cost).min(prev[j + 1] + 1).min(cur[j] + 1); + } + std::mem::swap(&mut prev, &mut cur); + } + prev[b.len()] +} + /// One reviewer's latest verdict on a PR, plus forgejo's `stale` / /// `dismissed` bits. /// @@ -303,4 +358,48 @@ mod tests { assert_eq!(pct_encode("x&y"), "x%26y"); assert_eq!(pct_encode("a/b"), "a%2Fb"); } + + #[test] + fn nearest_finds_the_one_character_typo() { + let all = ["area/ops", "area/broker", "type/bug"]; + assert_eq!(super::nearest("area/opps", &all), Some("area/ops")); + assert_eq!(super::nearest("type/bugs", &all), Some("type/bug")); + } + + #[test] + fn nearest_suggests_nothing_for_an_invention() { + // The important half: a wrong suggestion invites a second failed + // attempt at a name that was never there, so far-away input must + // fall back to "here is everything". + let all = ["area/ops", "area/broker", "type/bug"]; + assert_eq!(super::nearest("frontend", &all), None); + assert_eq!(super::nearest("", &all), None); + } + + #[test] + fn nearest_is_stable_when_two_candidates_tie() { + // Both are distance 1 from "v3"; the answer must not depend on + // the order the forge returned them in. + let forward = ["v1", "v2"]; + let reversed = ["v2", "v1"]; + assert_eq!( + super::nearest("v3", &forward), + super::nearest("v3", &reversed) + ); + } + + #[test] + fn with_suggestions_annotates_only_the_near_misses() { + let all = ["area/ops", "type/bug"]; + let rendered = super::with_suggestions(&["area/opps", "frontend"], &all); + assert!( + rendered.contains(r#"area/opps (did you mean "area/ops"?)"#), + "{rendered}" + ); + assert!(rendered.contains("frontend"), "{rendered}"); + assert!( + !rendered.contains(r"frontend (did you mean"), + "invented name must not get a suggestion: {rendered}" + ); + } }