Fix fuzzy search false positives in model picker#29
Conversation
… matching - Add parseSearchQuery to detect "provider: term" prefix syntax for hard provider filtering (e.g. "openai: gpt-4" shows only OpenAI models) - Replace isSubsequenceMatch with scoreMatch (exact > word-boundary > normalized substring; numeric terms require contiguous digit group match) - Update filterBySearch to accept providerFilter and sort by score - Update modelGroups useMemo to use parseSearchQuery and short-circuit non-matching provider groups Fixes false positives like "5.4" matching "4.6" or "420". Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Pull request overview
This PR refines the model picker search in ManageModelsBox.tsx to avoid fuzzy-search false positives by adding provider-prefix parsing (provider: query), replacing subsequence matching with tiered scoring, and sorting results by match score.
Changes:
- Added
parseSearchQueryto supportprovider:prefixes (with normalization likeOpen AI:) and return{ providerFilter, modelTerms }. - Replaced subsequence matching with
scoreMatch(substring / word-prefix / normalized substring, plus stricter numeric matching). - Updated model group filtering to apply optional provider hard-filtering and score-based sorting.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const matched = KNOWN_PROVIDERS.find((p) => | ||
| p.startsWith(potentialProvider) || potentialProvider.startsWith(p), | ||
| ); | ||
| if (matched) { | ||
| const remainder = query.slice(colonIndex + 1).toLowerCase(); | ||
| const modelTerms = remainder.split(" ").filter(Boolean); | ||
| return { providerFilter: matched, modelTerms }; |
There was a problem hiding this comment.
parseSearchQuery will treat an empty provider prefix as a match because "" satisfies p.startsWith(potentialProvider). For inputs like ":" or " : gpt", potentialProvider becomes "" and KNOWN_PROVIDERS.find(...) returns the first provider (currently anthropic), unintentionally hard-filtering results. Consider guarding with if (potentialProvider.length > 0) before attempting provider matching (and/or requiring an exact match).
| const matched = KNOWN_PROVIDERS.find((p) => | |
| p.startsWith(potentialProvider) || potentialProvider.startsWith(p), | |
| ); | |
| if (matched) { | |
| const remainder = query.slice(colonIndex + 1).toLowerCase(); | |
| const modelTerms = remainder.split(" ").filter(Boolean); | |
| return { providerFilter: matched, modelTerms }; | |
| if (potentialProvider.length > 0) { | |
| const matched = KNOWN_PROVIDERS.find((p) => | |
| p.startsWith(potentialProvider) || | |
| potentialProvider.startsWith(p), | |
| ); | |
| if (matched) { | |
| const remainder = query.slice(colonIndex + 1).toLowerCase(); | |
| const modelTerms = remainder.split(" ").filter(Boolean); | |
| return { providerFilter: matched, modelTerms }; | |
| } |
| query.slice(0, colonIndex), | ||
| ); | ||
| const matched = KNOWN_PROVIDERS.find((p) => | ||
| p.startsWith(potentialProvider) || potentialProvider.startsWith(p), | ||
| ); | ||
| if (matched) { | ||
| const remainder = query.slice(colonIndex + 1).toLowerCase(); | ||
| const modelTerms = remainder.split(" ").filter(Boolean); | ||
| return { providerFilter: matched, modelTerms }; |
There was a problem hiding this comment.
Provider prefix matching is currently ambiguous for partial inputs because it uses find with startsWith in both directions. For example, open: would match openai (first match) even though openrouter is also a valid match. To avoid surprising filtering, consider requiring an exact normalized provider name, or only accepting prefixes when they produce a unique match (or preferring the longest match).
| const KNOWN_PROVIDERS = [ | ||
| "anthropic", | ||
| "openai", | ||
| "google", | ||
| "perplexity", | ||
| "grok", | ||
| "ollama", | ||
| "lmstudio", | ||
| "openrouter", | ||
| ] as const; |
There was a problem hiding this comment.
KNOWN_PROVIDERS duplicates provider-name knowledge that already exists in @core/chorus/Models (ProviderName includes e.g. "meta"). Keeping this list in the UI risks drifting from the source of truth as providers are added/renamed, which would silently break provider: parsing. Consider typing this as ProviderName[] and/or exporting a shared provider list from the core model layer so the parser stays in sync.
| return models | ||
| .filter((m) => { | ||
| // Hard-filter by provider when specified | ||
| if ( | ||
| providerFilter !== null && | ||
| getProviderName(m.modelId) !== providerFilter | ||
| ) { | ||
| return false; | ||
| } | ||
|
|
||
| if (modelTerms.length === 0) return true; | ||
|
|
||
| const providerLabel = getProviderLabel(m.modelId); | ||
| const haystack = `${m.displayName} ${providerLabel} ${m.modelId}`.toLowerCase(); | ||
|
|
||
| return modelTerms.every( | ||
| (term) => scoreMatch(term, haystack) > 0, | ||
| ); | ||
| }) | ||
| .sort((a, b) => { | ||
| if (modelTerms.length === 0) return 0; | ||
| const haystackA = | ||
| `${a.displayName} ${getProviderLabel(a.modelId)} ${a.modelId}`.toLowerCase(); | ||
| const haystackB = | ||
| `${b.displayName} ${getProviderLabel(b.modelId)} ${b.modelId}`.toLowerCase(); | ||
| const scoreA = modelTerms.reduce( | ||
| (acc, term) => acc + scoreMatch(term, haystackA), | ||
| 0, | ||
| ); | ||
| const scoreB = modelTerms.reduce( | ||
| (acc, term) => acc + scoreMatch(term, haystackB), | ||
| 0, | ||
| ); | ||
| return scoreB - scoreA; | ||
| }); |
There was a problem hiding this comment.
filterBySearch sorting recomputes scoreMatch (including normalization) repeatedly inside the Array.sort comparator, which can be noticeably expensive as the model list grows. Consider precomputing a single score per model (e.g., map to {model, score}) during the filter pass and then sorting by that cached score.
| return models | |
| .filter((m) => { | |
| // Hard-filter by provider when specified | |
| if ( | |
| providerFilter !== null && | |
| getProviderName(m.modelId) !== providerFilter | |
| ) { | |
| return false; | |
| } | |
| if (modelTerms.length === 0) return true; | |
| const providerLabel = getProviderLabel(m.modelId); | |
| const haystack = `${m.displayName} ${providerLabel} ${m.modelId}`.toLowerCase(); | |
| return modelTerms.every( | |
| (term) => scoreMatch(term, haystack) > 0, | |
| ); | |
| }) | |
| .sort((a, b) => { | |
| if (modelTerms.length === 0) return 0; | |
| const haystackA = | |
| `${a.displayName} ${getProviderLabel(a.modelId)} ${a.modelId}`.toLowerCase(); | |
| const haystackB = | |
| `${b.displayName} ${getProviderLabel(b.modelId)} ${b.modelId}`.toLowerCase(); | |
| const scoreA = modelTerms.reduce( | |
| (acc, term) => acc + scoreMatch(term, haystackA), | |
| 0, | |
| ); | |
| const scoreB = modelTerms.reduce( | |
| (acc, term) => acc + scoreMatch(term, haystackB), | |
| 0, | |
| ); | |
| return scoreB - scoreA; | |
| }); | |
| const scoredModels = models.reduce<{ model: ModelConfig; score: number }[]>( | |
| (acc, model) => { | |
| // Hard-filter by provider when specified | |
| if ( | |
| providerFilter !== null && | |
| getProviderName(model.modelId) !== providerFilter | |
| ) { | |
| return acc; | |
| } | |
| if (modelTerms.length === 0) { | |
| acc.push({ model, score: 0 }); | |
| return acc; | |
| } | |
| const providerLabel = getProviderLabel(model.modelId); | |
| const haystack = | |
| `${model.displayName} ${providerLabel} ${model.modelId}`.toLowerCase(); | |
| const termScores = modelTerms.map((term) => scoreMatch(term, haystack)); | |
| if (termScores.some((score) => score <= 0)) { | |
| return acc; | |
| } | |
| acc.push({ | |
| model, | |
| score: termScores.reduce((total, score) => total + score, 0), | |
| }); | |
| return acc; | |
| }, | |
| [], | |
| ); | |
| if (modelTerms.length > 0) { | |
| scoredModels.sort((a, b) => b.score - a.score); | |
| } | |
| return scoredModels.map(({ model }) => model); |
…ute scores
- Use ProviderName[] for KNOWN_PROVIDERS to stay in sync with core model layer
(adds "meta"; eliminates drift risk)
- Guard against empty provider prefix (": gpt" no longer accidentally matches
first provider in the list)
- Require exact or unambiguous prefix match for provider (avoids "open:" being
ambiguous between openai/openrouter)
- Precompute scoreMatch per model in filterBySearch to avoid redundant work
inside the sort comparator
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const visibleProviderModels = useMemo(() => { | ||
| return filterModelsBySearch( | ||
| providerModels, | ||
| subProviderSearch, | ||
| subProviders, | ||
| subProviderFilters, | ||
| ); |
There was a problem hiding this comment.
subProviderSearch is now being passed into filterModelsBySearch, which supports subProvider: prefix syntax. However filteredSubProviders is still derived by doing a raw includes() match on the full search string, so entering something like openai: (or openai: gpt-4) will make the chip list empty (except “All”) because no sub-provider contains a colon. Consider parsing the search value (e.g., split on : and only use the provider part for chip filtering), or keep separate state for “chip search” vs “model search” so the chip list remains usable when prefix syntax is used.
| // Derived from ProviderName to stay in sync with the core model layer | ||
| const KNOWN_PROVIDERS: ProviderName[] = [ | ||
| "anthropic", | ||
| "openai", | ||
| "google", | ||
| "perplexity", | ||
| "grok", | ||
| "ollama", | ||
| "lmstudio", | ||
| "openrouter", | ||
| "meta", | ||
| ]; |
There was a problem hiding this comment.
The comment says KNOWN_PROVIDERS is “Derived from ProviderName to stay in sync”, but the array is manually maintained and TypeScript won’t ensure it remains exhaustive when ProviderName changes. Either derive it from a single shared source-of-truth constant, or adjust the comment to reflect that this is a manually curated list (and ideally add a compile-time exhaustiveness check).
| import { filterModelsBySearch, getSubProvider } from "./visibleModelsSearch"; | ||
|
|
There was a problem hiding this comment.
PR description focuses on fixing fuzzy search in ManageModelsBox.tsx, but this PR also changes VisibleModelsTab.tsx search/filtering behavior and introduces visibleModelsSearch.ts + tests. Please update the PR description (or split into separate PRs) so reviewers understand the additional UI/search behavior change and why it’s coupled to the model picker search fix.
Fixes false positives in the model picker search (
ManageModelsBox.tsx).Changes
parseSearchQuery: Detectsprovider: termprefix syntax (e.g.openai: gpt-4). Normalizes variations likeopen ai:orOpenAI:. Returns{ providerFilter, modelTerms }.scoreMatch: ReplacesisSubsequenceMatchwith a tiered scoring function:5.4won't match4.6)filterBySearch: Updated signature to acceptproviderFilter; hard-filters by provider when set; results sorted by score.modelGroupsuseMemo: UsesparseSearchQuery; short-circuits non-matching provider groups to[]when a provider prefix is specified.Test plan
openai: gpt-4→ shows only OpenAI gpt-4 variants, no other providersanthropic: claude→ shows only Anthropic Claude modelsgpt 4(no colon) → searches all providers, shows gpt-4 variantsOpen AI: gpt5.4→ shows NO results (no such model), no false positives5.4→ does NOT match4.6,4.20, etc.claude→ shows all Claude models across providers (backward compatible)google:→ shows all Google models