Repository navigation
Rank visible title matches above hidden metadata in workspace switcher - #5148
Conversation
…word Query "ios" should rank a workspace titled "iOS Mobile Terminal" above one that only matched a hidden branch/description token "ios". The nucleo scorer ranks the hidden exact-keyword match first today, so this test is red until the scoring fix lands. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… scorer A short query that exactly matched a hidden search-field line (branch, directory, or description token) scored 30_030 and beat a visible title prefix, which only reached nucleo(~88) + 2_000. In the workspace switcher this put a row with no highlighted title at the top for queries like "ios". Add a whole-query title-literal tier (exact 34_000, prefix 32_000, floored at 31_000 so even a long prefix clears keyword-exact's 30_030) layered into weighted_query_score, so a workspace whose visible title starts with the query wins. Per-token and multi-token sums are untouched; this only adds a max() term when the whole query is a title prefix. Matches the Swift CommandPaletteSearchEngine reference ordering (title prefix > exact keyword). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds exact-and-prefix whole-query title literal scoring (precompiled Atoms), integrates that tier into weighted query scoring (selecting the higher available token-based or title-literal score), and adds three regression tests asserting visible-title-prefix matches outrank metadata-only matches (including diacritic normalization). ChangesTitle Literal Scoring for Command Palette Ranking
sequenceDiagram
participant Client
participant WeightedScorer
participant TokenScorer
participant TitleLiteralScorer
Client->>WeightedScorer: search(query, candidate)
WeightedScorer->>TokenScorer: compute token_sum_score
WeightedScorer->>TitleLiteralScorer: compute title_literal_score
TokenScorer-->>WeightedScorer: token score (Some/None)
TitleLiteralScorer-->>WeightedScorer: title score (Some/None)
WeightedScorer->>WeightedScorer: select higher available score
WeightedScorer-->>Client: return ranked score
🎯 3 (Moderate) | ⏱️ ~20 minutes
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (17 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d337bb97ca
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| fn chars_equal_ignoring_case(lhs: char, rhs: char) -> bool { | ||
| lhs == rhs || lhs.to_ascii_lowercase() == rhs.to_ascii_lowercase() |
There was a problem hiding this comment.
Handle Unicode case in title prefixes
When the visible title prefix differs from the query only by non-ASCII case, this comparator returns false because to_ascii_lowercase() leaves characters like É unchanged. In that scenario (for example title Éclair with query é, while another row has hidden metadata line é) the new literal-title tier is not applied, so the hidden short exact match still gets the 30,010 keyword score and can remain above the visible title prefix this change is meant to prefer. Use Unicode-aware case folding here, or normalize title/query the same way the Swift search path does, so localized workspace titles get the same ranking fix.
Useful? React with 👍 / 👎.
Greptile SummaryFixes a scoring bug in the Rust nucleo FFI scorer where hidden metadata exact-keyword matches (scoring up to 30,030 per token) outranked visible title prefix matches, causing e.g. "Fork Session Not Found" (matched only on a hidden "ios" branch token) to appear above "iOS Mobile Terminal" for query "ios". The fix layers a whole-query title-literal tier into
Confidence Score: 4/5Safe to merge for the reported bug fix, but the two-tier title scoring design is not implemented as described — the lower prefix tier is dead code. The reported ranking inversion (hidden metadata beating a visible title prefix) is correctly fixed: for the 'ios' / 'iOS Mobile Terminal' case, the title now scores 34,030 vs the hidden keyword's 30,030. However, AtomKind::Exact is a substring matcher in nucleo (fzf's 'foo), not a full-string equality matcher. This means the prefix branch in title_literal_score is permanently unreachable code, and any title that merely contains the query anywhere receives the highest tier (34,030) rather than only titles that start with or equal the query. The TITLE_PREFIX_TOKEN_SCORE constant serves no purpose. The fix is a one-word change (Exact → Equal), and the existing tests would still pass but would then also exercise the previously dead prefix tier. Native/CommandPaletteNucleoFFI/src/lib.rs — specifically the TitleLiteralQuery::new constructor and title_literal_score function. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Search query arrives] --> B[Normalize query]
B --> C[Build search_tokens per word]
B --> D[Build TitleLiteralQuery\nexact + prefix Atom]
C --> E[token_sum_score\nper-token fuzzy + keyword path]
D --> F[title_literal_score\ncheck whole query vs title]
E --> G{token result}
F --> H{literal result}
G -->|Some score| I[max of both]
H -->|Some score| I
G -->|None| J{literal result?}
H -->|None| K[token result only]
J -->|Some literal| L[literal score only]
J -->|None| M[candidate excluded]
I --> N[ScoredCandidate appended to BinaryHeap]
L --> N
K --> N
subgraph title_literal_score
F --> F1{Atom::Exact fires?\nquery is substring of title}
F1 -->|Yes| F2[TITLE_EXACT_TOKEN_SCORE\n34030 x token_count]
F1 -->|No| F3{Atom::Prefix fires?\ntitle starts with query}
F3 -->|Yes| F4[TITLE_PREFIX_TOKEN_SCORE\n32030 x token_count\nCurrently unreachable]
F3 -->|No| F5[None]
end
Reviews (5): Last reviewed commit: "Match title-literal tier with matcher no..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmuxTests/CommandPaletteNucleoFFITests.swift`:
- Line 111: Update the inline comment that references the placeholder PR URL
"pull/0000" to the real PR number by replacing "pull/0000" with "pull/5148" in
the comment inside CommandPaletteNucleoFFITests.swift (the line that currently
reads "// no highlighted title yet sat at the top.
https://github.com/manaflow-ai/cmux/pull/0000").
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 26fe426f-b150-45e6-970d-b81c8114be1e
📒 Files selected for processing (2)
Native/CommandPaletteNucleoFFI/src/lib.rscmuxTests/CommandPaletteNucleoFFITests.swift
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Native/CommandPaletteNucleoFFI/src/lib.rs">
<violation number="1" location="Native/CommandPaletteNucleoFFI/src/lib.rs:547">
P2: Unicode titles lose the new literal-prefix boost because case matching is ASCII-only (`to_ascii_lowercase`).</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
The earlier per-extra-character penalty made "shortest title wins" among prefix matches, which mis-ranks when one title is a character-prefix of another: query "workspace 1901" preferred "Workspace 19010 ..." over "Workspace 1901 ...". Verified against the real CommandPaletteNucleoFFITests fixtures (open-folder top-2, initialism, cafe, deep-workspace) replayed through the actual scorer. Give every title prefix the same 32_000 and let scored_candidate_order's (rank, index) tiebreak decide, preserving the switcher's natural order. Exact title (34_000) still beats prefix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b0f4fd5a07
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // jackpot in `exact_search_text_line_score`. `title_literal_score` sits above every keyword | ||
| // tier so the visible title wins. This mirrors the Swift `CommandPaletteSearchEngine` | ||
| // reference ordering (title prefix > exact keyword). | ||
| match (token_score, title_literal_score(candidate, query)) { |
There was a problem hiding this comment.
Preserve title-prefix priority for short multi-token queries
For multi-word queries made of short tokens, this max still lets hidden metadata win because token_sum_score adds the 30,030 exact-line jackpot once per token before comparing with the flat 32,000 title-prefix tier. In the workspace switcher, description metadata is indexed both as the original text and as component tokens, so a row whose hidden description contributes lines like ios and dev scores about 60,060 for query ios dev and can still rank above a visible title prefix such as iOS Dev Tools, leaving the same “top row has no highlighted title” failure for two short words.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Codex review caught that the fix only handled single-token queries. The per-token keyword path is summed in weighted_query_score, so a multi-token query like "ios app" lets a hidden row score an exact line per token (~30_030 each, ~60_060 summed), which beat a flat title-literal score and reproduced the same inversion for "iOS App". Derive the title tiers from KEYWORD_EXACT_CEILING and multiply by the query token count so a literal title match dominates token_count * ceiling for any query length. Add a multi-token regression test (testNucleoFFIPrefersTitleOverSummedHiddenKeywordsForMultiTokenQuery). Verified single-token, multi-token, open-folder top-2, initialism, café, and the numeric prefix-collision case through the real scorer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d01461338
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| fn chars_equal_ignoring_case(lhs: char, rhs: char) -> bool { | ||
| lhs == rhs || lhs.to_ascii_lowercase() == rhs.to_ascii_lowercase() |
There was a problem hiding this comment.
Use Unicode-aware title prefix matching
When the visible title prefix differs from the query only by non-ASCII case, this helper returns false because to_ascii_lowercase() leaves characters such as É unchanged. In that scenario (for example title Éclair with query é, while another row has a hidden metadata line é) the new title-literal tier is skipped, so the hidden exact keyword can still outrank the visible title prefix this change is meant to prefer; normalize title/query the same way as the Swift search path or use Unicode-aware case folding here.
Useful? React with 👍 / 👎.
… review) Codex review caught that the hand-rolled ASCII prefix check missed normalized non-ASCII title prefixes: a localized title like "Éclair" was not recognized as a prefix of "e", so a row with a hidden exact "e"/"é" metadata token kept the 30_0xx jackpot and still outranked the visible title. Replace the char-by-char comparison with nucleo `Atom`s (Exact + Prefix) built once per query and scored against each title, so case and Smart diacritic normalization are identical to the fuzzy matcher (e.g. "e" prefix-matches "Éclair", "пр" matches "Привет"). Add a diacritic regression test. Verified single/multi-token, diacritic/Cyrillic/café, open-folder top-2, initialism, and the numeric prefix-collision cases through the real scorer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| fn title_literal_score( | ||
| state: &mut SearchState, | ||
| query: &TitleLiteralQuery, | ||
| candidate: &Candidate, | ||
| ) -> Option<f64> { | ||
| let token_count = query.token_count as f64; | ||
| if state.atom_score(&query.exact, &candidate.title).is_some() { | ||
| return Some(TITLE_EXACT_TOKEN_SCORE * token_count); | ||
| } | ||
| if state.atom_score(&query.prefix, &candidate.title).is_some() { | ||
| return Some(TITLE_PREFIX_TOKEN_SCORE * token_count); | ||
| } | ||
| None | ||
| } |
There was a problem hiding this comment.
TITLE_PREFIX_TOKEN_SCORE tier is unreachable dead code
In nucleo, AtomKind::Exact is a verbatim substring match (equivalent to fzf's 'foo operator) — it returns Some whenever the query appears as a contiguous substring anywhere in the haystack. Because every prefix is also a substring, any title that would fire the prefix Atom will already have fired the exact Atom on the line above, making the second if branch permanently unreachable.
The PR description says "a title equal to the query scores 34,000 and a title prefix scores a flat 32,000," but the current implementation gives 34,030 to any title that contains the query as a substring (including mid-title positions like "My iOS Project" for query "ios"), and the 32,030 tier is never awarded. To match the stated two-tier design, the exact atom should use AtomKind::Equal (which nucleo defines as full-string equality, equivalent to fzf's ^foo$). Using AtomKind::Prefix for the lower tier would then work as intended.
| exact: Atom::new( | ||
| query, | ||
| CaseMatching::Ignore, | ||
| Normalization::Smart, | ||
| AtomKind::Exact, | ||
| true, | ||
| ), |
There was a problem hiding this comment.
The
exact atom should use AtomKind::Equal (full-string equality, fzf's ^foo$) rather than AtomKind::Exact (verbatim substring, fzf's 'foo). With AtomKind::Exact, any title that merely contains the query as a contiguous substring (e.g. "My iOS Project" for query "ios") fires the top tier and scores 34,030. AtomKind::Equal restricts the top tier to titles that equal the query (case-insensitive), while the prefix atom still handles the "starts-with" case at 32,030.
| exact: Atom::new( | |
| query, | |
| CaseMatching::Ignore, | |
| Normalization::Smart, | |
| AtomKind::Exact, | |
| true, | |
| ), | |
| exact: Atom::new( | |
| query, | |
| CaseMatching::Ignore, | |
| Normalization::Smart, | |
| AtomKind::Equal, | |
| true, | |
| ), |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Summary
Native/CommandPaletteNucleoFFI/src/lib.rs). For a short query, an exact match on any hidden search-field line scored 30,030, while a visible title prefix only reached nucleo(~88) + 2,000, so the hidden match always won.weighted_query_score: a title equal to the query scores 34,000 and a title prefix scores a flat 32,000, both above keyword-exact's 30,030. A workspace whose visible title starts with the query now wins. Per-token and multi-token sums are untouched; this only adds amax()term when the whole query is a title prefix. This matches the SwiftCommandPaletteSearchEnginereference ordering (title prefix > exact keyword), so the two engines now agree.(rank, index)order, preserving the switcher's natural order.Testing
cargo build --release(offline) clean, no warnings.testNucleoFFIPrefersVisibleTitlePrefixOverHiddenMetadataKeywordincmuxTests/CommandPaletteNucleoFFITests.swift, run by the CI tests job (which builds the dylib). Two-commit red/green: the test lands first, the scoring fix second.CommandPaletteNucleoFFITestsfixtures (open-folder top-2, initialism, café accent, deep-workspace numeric prefix) through the real scorer to confirm no ordering regressions; the deep-workspace replay is what caught and killed the length-penalty mis-ranking.Notes
Issues
Summary by CodeRabbit
New Features
Tests
Note
Medium Risk
Changes command-palette/workspace switcher result ordering in production scoring; well-tested but affects core UX for all palette searches.
Overview
Fixes workspace/command-palette ranking where hidden metadata (branch/description tokens) could beat a visible title that actually matches what the user typed—e.g. query
iosputting a row with only a hiddenioskeyword above iOS Mobile Terminal.The Rust nucleo FFI scorer adds a whole-query title literal tier: case-insensitive exact or prefix match on the candidate title (via
Atom+ Smart diacritic folding), combined with existing token scores asmax(token, literal). Literal tiers sit above the short-query keyword-exact jackpot (~30_030) and scale by query token count so multi-token queries likeios appstill lose to summed hidden keyword lines.Prefix matches use a flat score (no length penalty) so numeric/workspace titles don’t mis-order; ties still use existing rank/index ordering. Swift FFI tests cover single-token, multi-token, and diacritic regressions.
Reviewed by Cursor Bugbot for commit d4718dc. Bugbot is set up for automated code reviews on this repo. Configure here.