Skip to content

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

Merged
fcoury-oai merged 5 commits intomainfrom
fcoury/fix-skill-picker-search
Apr 3, 2026
Merged

fix(tui): sort skill mentions by display name first#16710
fcoury-oai merged 5 commits intomainfrom
fcoury/fix-skill-picker-search

Conversation

@fcoury-oai
Copy link
Copy Markdown
Contributor

@fcoury-oai fcoury-oai commented Apr 3, 2026

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
image image

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

Preserve the `interface.display_name` match score when filtering
skill mentions in the composer popup, and only fall back to
secondary `search_terms` when the display name does not match.

Add a regression test covering `PR Babysitter`, whose canonical
`babysit-pr` name previously pushed it below weaker visible-label
matches.
@fcoury-oai fcoury-oai marked this pull request as ready for review April 3, 2026 19:51
@fcoury-oai fcoury-oai requested a review from etraut-openai April 3, 2026 19:51
Sort mention popup results by fuzzy-match score before `sort_rank`
when the user has typed a non-empty query, so weaker plugin matches
no longer stay above stronger skill matches.

Keep the existing plugin-first grouping for the empty-query state and
add regression coverage for `$pr` returning `PR Babysitter` before a
weaker plugin match.
Annotate the positional `sort_rank` literals in the mention popup
test helpers so Bazel argument-comment lint accepts the new regression
coverage.

This keeps the plugin-vs-skill ordering test in place while fixing the
PR-only CI break introduced by the helper refactor.
Adjust the mention popup regression fixture so the plugin row also
matches `$pr` via search terms, which mirrors the real composer
scenario from the screenshot.

This keeps the assertion focused on the intended behavior: the plugin
still appears, but it no longer outranks the stronger skill match.
Rank alias-only mention matches behind display-name matches for
non-empty popup queries, so plugin search terms do not outrank the
visible skill labels users are actually typing for.

This fixes the `$pr` case where a plugin could stay above `PR
Babysitter` even though the plugin only matched through fallback search
terms.
Copy link
Copy Markdown
Collaborator

@etraut-openai etraut-openai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now!

@fcoury-oai fcoury-oai merged commit 3d8cdac into main Apr 3, 2026
22 checks passed
@fcoury-oai fcoury-oai deleted the fcoury/fix-skill-picker-search branch April 3, 2026 21:09
@github-actions github-actions bot locked and limited conversation to collaborators Apr 3, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants