From 5fb3b7e401cc7fa5b95549ee5b2ecfbcb755ba91 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Mon, 14 Sep 2026 17:03:48 +0000 Subject: [PATCH] Fix fuzzy match scoring within Unicode lowercase expansions (#45475) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why Matches starting inside a lowercase expansion such as `İ` → `i̇` could receive an incorrect prefix bonus or gap penalty, causing strings that lowercase identically to rank differently. ## What changed Track the first matched position in the lowercased text directly when calculating scores. Preserve original character indices for highlighting. ## Testing Add a skill popup regression test and snapshot covering ranking and highlighting for matches beginning at the combining dot in expanded and already-lowercase names. GitOrigin-RevId: 78a8b79f1defca9f76fde5df945b0b1ff4af25da --- codex-rs/tui/src/bottom_pane/skill_popup.rs | 27 ++++++++++++++ ...ests__skill_popup_lowercase_expansion.snap | 36 +++++++++++++++++++ codex-rs/utils/fuzzy-match/src/lib.rs | 15 +++----- 3 files changed, 68 insertions(+), 10 deletions(-) create mode 100644 codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__skill_popup__tests__skill_popup_lowercase_expansion.snap diff --git a/codex-rs/tui/src/bottom_pane/skill_popup.rs b/codex-rs/tui/src/bottom_pane/skill_popup.rs index 4c7c2980c5..d70c95423d 100644 --- a/codex-rs/tui/src/bottom_pane/skill_popup.rs +++ b/codex-rs/tui/src/bottom_pane/skill_popup.rs @@ -331,6 +331,33 @@ mod tests { insta::assert_snapshot!("skill_popup_scrolled", render_popup(&popup, /*width*/ 72)); } + #[test] + fn lowercase_expansion_preserves_match_ranking_and_highlighting() { + let mut popup = SkillPopup::new(vec![ + named_mention_item("İx", &[]), + named_mention_item("i\u{0307}x", &[]), + named_mention_item("aİx", &[]), + named_mention_item("ai\u{0307}x", &[]), + ]); + popup.set_query("\u{0307}x"); + + // Each pair lowercases identically. The dot starts a contiguous match, + // but not a prefix, even when it came from the expansion of 'İ'. + assert_eq!( + popup.filtered(), + vec![ + (3, Some(vec![2, 3]), 0), + (2, Some(vec![1, 2]), 0), + (1, Some(vec![1, 2]), 0), + (0, Some(vec![0, 1]), 0), + ] + ); + insta::assert_snapshot!( + "skill_popup_lowercase_expansion", + render_popup(&popup, /*width*/ 48) + ); + } + #[test] fn display_name_match_sorting_beats_worse_secondary_search_term_matches() { let mut popup = SkillPopup::new(vec![ diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__skill_popup__tests__skill_popup_lowercase_expansion.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__skill_popup__tests__skill_popup_lowercase_expansion.snap new file mode 100644 index 0000000000..c88b3aa5c2 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__skill_popup__tests__skill_popup_lowercase_expansion.snap @@ -0,0 +1,36 @@ +--- +source: tui/src/bottom_pane/skill_popup.rs +expression: "render_popup(&popup, 48)" +--- +Buffer { + area: Rect { x: 0, y: 0, width: 48, height: 6 }, + content: [ + " ai̇x [Skill] ", + " aİx [Skill] ", + " i̇x [Skill] ", + " İx [Skill] ", + " ", + " Press enter to insert or esc to close ", + ], + styles: [ + x: 0, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 2, y: 0, fg: Cyan, bg: Reset, underline: Reset, modifier: BOLD, + x: 14, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 3, y: 1, fg: Reset, bg: Reset, underline: Reset, modifier: BOLD, + x: 5, y: 1, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 7, y: 1, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 14, y: 1, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 2, y: 2, fg: Reset, bg: Reset, underline: Reset, modifier: BOLD, + x: 4, y: 2, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 7, y: 2, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 14, y: 2, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 2, y: 3, fg: Reset, bg: Reset, underline: Reset, modifier: BOLD, + x: 4, y: 3, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 7, y: 3, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 14, y: 3, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 8, y: 5, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 13, y: 5, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 27, y: 5, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 30, y: 5, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + ] +} diff --git a/codex-rs/utils/fuzzy-match/src/lib.rs b/codex-rs/utils/fuzzy-match/src/lib.rs index d644b31677..106f60d832 100644 --- a/codex-rs/utils/fuzzy-match/src/lib.rs +++ b/codex-rs/utils/fuzzy-match/src/lib.rs @@ -8,7 +8,7 @@ /// lowercased haystack back to the original character index in `haystack`. /// This ensures the returned indices can be safely used with /// `str::chars().enumerate()` consumers for highlighting, even when -/// lowercasing expands certain characters (e.g., ß → ss, İ → i̇). +/// lowercasing expands certain characters (e.g., İ → i̇). pub fn fuzzy_match(haystack: &str, needle: &str) -> Option<(Vec, i32)> { if needle.is_empty() { return Some((Vec::new(), i32::MAX)); @@ -26,6 +26,7 @@ pub fn fuzzy_match(haystack: &str, needle: &str) -> Option<(Vec, i32)> { let lowered_needle: Vec = needle.to_lowercase().chars().collect(); let mut result_orig_indices: Vec = Vec::with_capacity(lowered_needle.len()); + let mut first_lower_pos: Option = None; let mut last_lower_pos: Option = None; let mut cur = 0usize; for &nc in lowered_needle.iter() { @@ -40,18 +41,12 @@ pub fn fuzzy_match(haystack: &str, needle: &str) -> Option<(Vec, i32)> { } let pos = found_at?; result_orig_indices.push(lowered_to_orig_char_idx[pos]); + first_lower_pos.get_or_insert(pos); last_lower_pos = Some(pos); } - let first_lower_pos = if result_orig_indices.is_empty() { - 0usize - } else { - let target_orig = result_orig_indices[0]; - lowered_to_orig_char_idx - .iter() - .position(|&oi| oi == target_orig) - .unwrap_or(0) - }; + // Score using the actual lowered positions, even within a character expansion. + let first_lower_pos = first_lower_pos?; // last defaults to first for single-hit; score = extra span between first/last hit // minus needle len (≥0). // Strongly reward prefix matches by subtracting 100 when the first hit is at index 0.