clone/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-04-24 06:35:50 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(tui): sort skill mentions by display name first (#16710)

Browse Source 
## Summary

The skill list opened by '$' shows `interface.display_name` preferably
if available but the sorting order of the search results use the
`skill.name` for sorting the results regardless.

This can be clearly seen in this example below: I expected with "pr" as
the search term to have "PR Babysitter" be the first item, but instead
it's way down the list.

The reason is because "PR Babysitter" skill name is "babysit-pr" and
therefore it doesn't rank as high as "pr-review-triage".

This PR fixes this behavior.

| Before | After |
| --- | --- |
| <img width="659" height="376" alt="image"
src="https://github.com/user-attachments/assets/51a71491-62ec-4163-a6f3-943ddf55856d"
/> | <img width="618" height="429" alt="image"
src="https://github.com/user-attachments/assets/f5ec4f4a-c539-4a5d-bdc5-c3e3e630f530"
/> |


## Testing

- `just fmt`
- `cargo test -p codex-tui
bottom_pane::skill_popup::tests::display_name_match_sorting_beats_worse_secondary_search_term_matches
--lib -- --exact`
- `cargo test -p codex-tui`
This commit is contained in:
fcoury-oai
2026-04-03 18:09:30 -03:00
committed by GitHub
parent 1d4b5f130c
commit 3d8cdac797
1 changed files with 133 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

167
codex-rs/tui/src/bottom_pane/skill_popup.rs
View File

@@ -137,31 +137,18 @@ impl SkillPopup {
continue;
}
let mut best_match: Option<(Option<Vec<usize>>, i32)> = None;
if let Some((indices, score)) = fuzzy_match(&mention.display_name, filter) {
best_match = Some((Some(indices), score));
}
for term in &mention.search_terms {
if term == &mention.display_name {
continue;
}
if let Some((_indices, score)) = fuzzy_match(term, filter) {
match best_match.as_mut() {
Some((best_indices, best_score)) => {
if score > *best_score {
*best_score = score;
*best_indices = None;
}
}
None => {
best_match = Some((None, score));
}
}
}
}
let best_match =
if let Some((indices, score)) = fuzzy_match(&mention.display_name, filter) {
Some((Some(indices), score))
} else {
mention
.search_terms
.iter()
.filter(|term| *term != &mention.display_name)
.filter_map(|term| fuzzy_match(term, filter).map(|(_indices, score)| score))
.min()
.map(|score| (None, score))
};
if let Some((indices, score)) = best_match {
out.push((idx, indices, score));
@@ -169,15 +156,25 @@ impl SkillPopup {
}
out.sort_by(|a, b| {
self.mentions[a.0]
.sort_rank
.cmp(&self.mentions[b.0].sort_rank)
.then_with(|| a.2.cmp(&b.2))
.then_with(|| {
let an = self.mentions[a.0].display_name.as_str();
let bn = self.mentions[b.0].display_name.as_str();
an.cmp(bn)
})
if filter.is_empty() {
self.mentions[a.0]
.sort_rank
.cmp(&self.mentions[b.0].sort_rank)
} else {
a.1.is_none()
.cmp(&b.1.is_none())
.then_with(|| a.2.cmp(&b.2))
.then_with(|| {
self.mentions[a.0]
.sort_rank
.cmp(&self.mentions[b.0].sort_rank)
})
}
.then_with(|| {
let an = self.mentions[a.0].display_name.as_str();
let bn = self.mentions[b.0].display_name.as_str();
an.cmp(bn)
})
});
out
@@ -249,6 +246,34 @@ mod tests {
}
}
fn ranked_mention_item(
display_name: &str,
search_terms: &[&str],
category_tag: &str,
sort_rank: u8,
) -> MentionItem {
MentionItem {
display_name: display_name.to_string(),
description: None,
insert_text: format!("${display_name}"),
search_terms: search_terms
.iter()
.map(|term| (*term).to_string())
.collect(),
path: None,
category_tag: Some(category_tag.to_string()),
sort_rank,
}
}
fn named_mention_item(display_name: &str, search_terms: &[&str]) -> MentionItem {
ranked_mention_item(display_name, search_terms, "[Skill]", /*sort_rank*/ 1)
}
fn plugin_mention_item(display_name: &str, search_terms: &[&str]) -> MentionItem {
ranked_mention_item(display_name, search_terms, "[Plugin]", /*sort_rank*/ 0)
}
#[test]
fn filtered_mentions_preserve_results_beyond_popup_height() {
let popup = SkillPopup::new((0..(MAX_POPUP_ROWS + 2)).map(mention_item).collect());
@@ -288,4 +313,78 @@ mod tests {
insta::assert_snapshot!("skill_popup_scrolled", render_popup(&popup, /*width*/ 72));
}
#[test]
fn display_name_match_sorting_beats_worse_secondary_search_term_matches() {
let mut popup = SkillPopup::new(vec![
named_mention_item("pr-review-triage", &["pr-review-triage"]),
named_mention_item("prd", &["prd"]),
named_mention_item("PR Babysitter", &["babysit-pr", "PR Babysitter"]),
named_mention_item("Plugin Creator", &["plugin-creator", "Plugin Creator"]),
named_mention_item(
"Logging Best Practices",
&["logging-best-practices", "Logging Best Practices"],
),
]);
popup.set_query("pr");
let filtered_names: Vec<String> = popup
.filtered_items()
.into_iter()
.map(|idx| popup.mentions[idx].display_name.clone())
.collect();
assert_eq!(
filtered_names,
vec![
"PR Babysitter".to_string(),
"pr-review-triage".to_string(),
"prd".to_string(),
"Plugin Creator".to_string(),
"Logging Best Practices".to_string(),
]
);
}
#[test]
fn query_match_score_sorts_before_plugin_rank_bias() {
let mut popup = SkillPopup::new(vec![
plugin_mention_item("GitHub", &["github", "pull requests", "pr"]),
named_mention_item("pr-review-triage", &["pr-review-triage"]),
named_mention_item("prd", &["prd"]),
named_mention_item("Plugin Creator", &["plugin-creator", "Plugin Creator"]),
named_mention_item(
"Logging Best Practices",
&["logging-best-practices", "Logging Best Practices"],
),
named_mention_item("PR Babysitter", &["babysit-pr", "PR Babysitter"]),
]);
popup.set_query("pr");
let filtered_items: Vec<(String, Option<String>)> = popup
.filtered_items()
.into_iter()
.map(|idx| {
(
popup.mentions[idx].display_name.clone(),
popup.mentions[idx].category_tag.clone(),
)
})
.collect();
assert_eq!(
filtered_items,
vec![
("PR Babysitter".to_string(), Some("[Skill]".to_string())),
("pr-review-triage".to_string(), Some("[Skill]".to_string())),
("prd".to_string(), Some("[Skill]".to_string())),
("Plugin Creator".to_string(), Some("[Skill]".to_string())),
(
"Logging Best Practices".to_string(),
Some("[Skill]".to_string())
),
("GitHub".to_string(), Some("[Plugin]".to_string())),
]
);
}
}