fix(desktop): fold separators in model search so filters and highlights agree
Model ids use hyphens/underscores, display names use spaces, versions use
dots. The pickers' filter haystacks contained both the raw id and the
display name, but the highlight only ever saw the display label — so
'qwen3.8-flash' revealed the row (via the id segment) while lighting up
nothing, and model-picker.tsx had the inverse polarity (spaces matched,
hyphens didn't highlight). This violated HighlightMatches' own documented
contract: the query must mirror the filter's semantics or the emphasis
lies.
One length-preserving searchFold ([-_.] -> space, 1 char in / 1 char out)
now runs on both sides of every model-search filter AND inside
HighlightMatches' range finder. Length preservation keeps mark ranges
valid against the original text, so <mark> rendering is untouched. The
fold is a per-character substitution applied to both sides, so any query
that matched before still matches — only coverage grows.
- lib/text.ts: searchFold + foldIncludes, the one matcher for all pickers
- highlight-matches.tsx: ranges computed on folded text, marks slice original
- model-catalog-menu.tsx: family, MoA, and download haystacks use foldIncludes
- model-visibility-dialog.tsx, model-picker.tsx: same (fixes inverse polarity)
- dropdown-menu.tsx: DropdownMenuSearch sets spellCheck={false} — squiggles
under model ids are noise; composer/settings inputs already disable it
Tests: fold primitives (equivalence, 1:1 length, superset), highlighter
fold behavior + index fidelity, end-to-end menu behavior (hyphen query
marks the spaced label; space query finds the hyphenated id without
over-matching), and an updated hidden-model-search test whose id-style
query now legitimately highlights.
This commit is contained in:
@@ -0,0 +1,108 @@
|
||||
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
|
||||
import { cleanup, fireEvent, render, screen } from '@testing-library/react'
|
||||
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { DropdownMenu, DropdownMenuContent } from '@/components/ui/dropdown-menu'
|
||||
import { $localModelsEnabled } from '@/store/local-models-flag'
|
||||
import { $localRuntimeJobs } from '@/store/local-runtime-jobs'
|
||||
import { $visibleModels } from '@/store/model-visibility'
|
||||
|
||||
import { ModelCatalogMenu, type ModelMenuController } from './model-catalog-menu'
|
||||
|
||||
// Radix calls these on open; jsdom doesn't implement them.
|
||||
beforeAll(() => {
|
||||
Element.prototype.scrollIntoView = vi.fn()
|
||||
Element.prototype.hasPointerCapture = vi.fn(() => false)
|
||||
Element.prototype.releasePointerCapture = vi.fn()
|
||||
})
|
||||
|
||||
const getGlobalModelOptions = vi.fn()
|
||||
|
||||
vi.mock('@/hermes', () => ({
|
||||
getGlobalModelOptions: (...args: unknown[]) => getGlobalModelOptions(...args),
|
||||
getLocalModelsJobs: vi.fn(async () => ({ jobs: [] })),
|
||||
getLocalModelsStatus: vi.fn().mockResolvedValue({ loading: {} }),
|
||||
setApiRequestProfile: vi.fn()
|
||||
}))
|
||||
|
||||
beforeEach(() => {
|
||||
$visibleModels.set(null)
|
||||
$localRuntimeJobs.set([])
|
||||
$localModelsEnabled.set(false)
|
||||
getGlobalModelOptions.mockResolvedValue({
|
||||
providers: [{ models: ['qwen3.8-flash', 'gpt-5.1'], name: 'OpenRouter', slug: 'openrouter' }]
|
||||
})
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
cleanup()
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
function renderMenu() {
|
||||
const controller: ModelMenuController = {
|
||||
applyPreset: vi.fn(),
|
||||
current: { effort: '', fast: false, model: '', provider: '' },
|
||||
presetFor: () => ({}),
|
||||
select: vi.fn(async () => true),
|
||||
setOptions: vi.fn()
|
||||
}
|
||||
|
||||
const client = new QueryClient({ defaultOptions: { queries: { retry: false } } })
|
||||
|
||||
render(
|
||||
<QueryClientProvider client={client}>
|
||||
<DropdownMenu open>
|
||||
<DropdownMenuContent>
|
||||
<ModelCatalogMenu controller={controller} />
|
||||
</DropdownMenuContent>
|
||||
</DropdownMenu>
|
||||
</QueryClientProvider>
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
|
||||
|
||||
// The row label span (class "truncate") carries the full label text plus the
|
||||
// meta suffix ("High"/"Min"), so match by prefix. Works for both the plain
|
||||
// and the <mark>-split rendering.
|
||||
function rowTruncateSpan(prefix: string) {
|
||||
return screen.queryByText((_, element) =>
|
||||
Boolean(element?.classList.contains('truncate') && (element?.textContent ?? '').startsWith(prefix))
|
||||
)
|
||||
}
|
||||
|
||||
// The search filter and the highlight must agree: whatever a query reveals,
|
||||
// the row's label marks the query (separator-folded equivalence). Pre-fix,
|
||||
// hyphen queries revealed rows with zero marks, and space queries matched
|
||||
// only via the display segment - both polarity failures covered here.
|
||||
describe('model catalog search: fold between filter and highlight', () => {
|
||||
it('HYPHEN query both filters and highlights the SPACED label (the reported defect - fails pre-fix)', async () => {
|
||||
renderMenu()
|
||||
await screen.findByText(/Qwen3\.8 Flash/i)
|
||||
|
||||
fireEvent.change(screen.getByRole('textbox', { name: 'Search models' }), { target: { value: 'qwen3.8-flash' } })
|
||||
|
||||
await vi.waitFor(() => {
|
||||
// Whole label matches contiguously -> renders as a single <mark>.
|
||||
expect(screen.getByText('Qwen3.8 Flash', { selector: 'mark' })).toBeDefined()
|
||||
})
|
||||
})
|
||||
|
||||
it('SPACE query finds the HYPHENATED id (superset guarantee) and highlights', async () => {
|
||||
renderMenu()
|
||||
await screen.findByText(/Qwen3\.8 Flash/i)
|
||||
|
||||
fireEvent.change(screen.getByRole('textbox', { name: 'Search models' }), { target: { value: 'qwen3 8' } })
|
||||
|
||||
await vi.waitFor(() => {
|
||||
// Partial label match -> <mark>Qwen3.8</mark> + ' Flash'.
|
||||
expect(screen.getByText('Qwen3.8', { selector: 'mark' })).toBeDefined()
|
||||
// The row label (before the meta suffix) is intact.
|
||||
expect(rowTruncateSpan('Qwen3.8 Flash')).toBeDefined()
|
||||
// The fold must not over-match: a qwen query still hides GPT rows.
|
||||
expect(rowTruncateSpan('GPT-5.1')).toBeNull()
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -108,7 +108,16 @@ describe('the catalog owns model curation', () => {
|
||||
fireEvent.change(input, { target: { value: 'gemini-3.1' } })
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(screen.queryByText(/Gemini 3\.1 Pro/i)).not.toBeNull()
|
||||
// The fold makes this id-style query highlight the spaced label: the
|
||||
// row renders as <mark>Gemini 3.1</mark> + ' Pro'.
|
||||
expect(screen.getByText('Gemini 3.1', { selector: 'mark' })).toBeDefined()
|
||||
// Display name is "Gemini 3.1 pro" (no title-case for gemini ids); the
|
||||
// row label span carries it (plus the effort meta suffix).
|
||||
expect(
|
||||
screen.getByText((_, element) =>
|
||||
Boolean(element?.classList.contains('truncate') && (element?.textContent ?? '').startsWith('Gemini 3.1 pro'))
|
||||
)
|
||||
).toBeDefined()
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -24,7 +24,7 @@ import { useI18n } from '@/i18n'
|
||||
import { modelOptionsQueryKey, requestModelOptions } from '@/lib/model-options'
|
||||
import { displayModelName, modelDisplayParts } from '@/lib/model-status-label'
|
||||
import { DEFAULT_REASONING_EFFORT, reasoningEffortLabel } from '@/lib/reasoning-effort'
|
||||
import { normalize } from '@/lib/text'
|
||||
import { foldIncludes, normalize } from '@/lib/text'
|
||||
import { useStoreSelector } from '@/lib/use-session-slice'
|
||||
import { cn } from '@/lib/utils'
|
||||
import { $localModelsEnabled } from '@/store/local-models-flag'
|
||||
@@ -265,7 +265,7 @@ export function ModelCatalogMenu({
|
||||
// In-flight downloads render inside the Local provider group when it
|
||||
// exists, else as their own trailing 'Local' group (first download —
|
||||
// nothing staged yet, so the catalog has no local provider row).
|
||||
const shownDownloads = q ? downloads.filter(job => (job.target || '').toLowerCase().includes(q)) : downloads
|
||||
const shownDownloads = q ? downloads.filter(job => foldIncludes(job.target || '', q)) : downloads
|
||||
const hasLocalGroup = pickerProviders.some(provider => provider.slug === LOCAL_PROVIDER_SLUG)
|
||||
|
||||
// Resolve visibility HERE, against the catalog we actually fetched: an empty
|
||||
@@ -285,7 +285,7 @@ export function ModelCatalogMenu({
|
||||
// sitting under zero model matches would otherwise become the "first match"
|
||||
// Enter commits.
|
||||
const shownMoaPresets = useMemo(
|
||||
() => (q ? moaPresets.filter(preset => `moa ${preset}`.toLowerCase().includes(q)) : moaPresets),
|
||||
() => (q ? moaPresets.filter(preset => foldIncludes(`moa ${preset}`, q)) : moaPresets),
|
||||
[moaPresets, q]
|
||||
)
|
||||
|
||||
@@ -729,9 +729,10 @@ function groupModels(
|
||||
}
|
||||
|
||||
const matches = (family: ModelFamily) =>
|
||||
`${family.id} ${family.fastId ?? ''} ${provider.name} ${provider.slug} ${displayModelName(family.id)}`
|
||||
.toLowerCase()
|
||||
.includes(q)
|
||||
foldIncludes(
|
||||
`${family.id} ${family.fastId ?? ''} ${provider.name} ${provider.slug} ${displayModelName(family.id)}`,
|
||||
q
|
||||
)
|
||||
|
||||
let shown: Set<string>
|
||||
|
||||
|
||||
@@ -6,7 +6,7 @@ import { useI18n } from '@/i18n'
|
||||
import { modelOptionsQueryKey, requestModelOptions } from '@/lib/model-options'
|
||||
import { modelSearchText } from '@/lib/model-search-text'
|
||||
import { currentPickerSelection } from '@/lib/model-status-label'
|
||||
import { normalize } from '@/lib/text'
|
||||
import { foldIncludes, normalize } from '@/lib/text'
|
||||
import { useStoreSelector } from '@/lib/use-session-slice'
|
||||
import { $localModelsEnabled } from '@/store/local-models-flag'
|
||||
import { $localRuntimeJobs, runningModelDownloads, watchLocalRuntimeJobs } from '@/store/local-runtime-jobs'
|
||||
@@ -266,9 +266,9 @@ function ModelResults({
|
||||
|
||||
const matches = (provider: ModelOptionProvider, model: string) =>
|
||||
!q ||
|
||||
modelSearchText(model).toLowerCase().includes(q) ||
|
||||
provider.name.toLowerCase().includes(q) ||
|
||||
provider.slug.toLowerCase().includes(q)
|
||||
foldIncludes(modelSearchText(model), q) ||
|
||||
foldIncludes(provider.name, q) ||
|
||||
foldIncludes(provider.slug, q)
|
||||
|
||||
// Only configured providers (those with curated models) are selectable
|
||||
// here. Switching to a NOT-yet-configured provider goes through the
|
||||
|
||||
@@ -14,7 +14,7 @@ import { useI18n } from '@/i18n'
|
||||
import { Search } from '@/lib/icons'
|
||||
import { modelOptionsQueryKey, requestModelOptions } from '@/lib/model-options'
|
||||
import { displayModelName, modelDisplayParts } from '@/lib/model-status-label'
|
||||
import { normalize } from '@/lib/text'
|
||||
import { foldIncludes, normalize } from '@/lib/text'
|
||||
import {
|
||||
$visibleModels,
|
||||
collapseModelFamilies,
|
||||
@@ -76,7 +76,7 @@ export function ModelVisibilityDialog({
|
||||
const q = normalize(search)
|
||||
|
||||
const matches = (provider: ModelOptionProvider, model: string) =>
|
||||
!q || `${model} ${provider.name} ${provider.slug} ${displayModelName(model)}`.toLowerCase().includes(q)
|
||||
!q || foldIncludes(`${model} ${provider.name} ${provider.slug} ${displayModelName(model)}`, q)
|
||||
|
||||
return (
|
||||
<Dialog onOpenChange={onOpenChange} open={open}>
|
||||
|
||||
@@ -61,6 +61,10 @@ function DropdownMenuSearch({
|
||||
|
||||
onKeyDown?.(event)
|
||||
}}
|
||||
// Search fields here filter ids, slugs, and model names — dictionary
|
||||
// squiggles under them are noise (matching the composer/settings
|
||||
// inputs, which already disable spellcheck).
|
||||
spellCheck={false}
|
||||
type="text"
|
||||
{...props}
|
||||
/>
|
||||
|
||||
@@ -5,41 +5,43 @@ import { HighlightMatches } from './highlight-matches'
|
||||
|
||||
const marksOf = (container: HTMLElement) => Array.from(container.querySelectorAll('mark')).map(m => m.textContent)
|
||||
|
||||
describe('HighlightMatches', () => {
|
||||
it('wraps every case-insensitive occurrence in a <mark> without altering the text', () => {
|
||||
const { container } = render(<HighlightMatches query="ro" text="Grok 4.5 Retro" />)
|
||||
describe('HighlightMatches with separator folding', () => {
|
||||
it('HYPHEN query marks the SPACED label (the reported defect — fails pre-fix)', () => {
|
||||
const { container } = render(<HighlightMatches query="qwen3.8-flash" text="Qwen3.8 Flash" />)
|
||||
|
||||
expect(container.textContent).toBe('Grok 4.5 Retro')
|
||||
expect(marksOf(container)).toEqual(['ro', 'ro'])
|
||||
expect(marksOf(container)).toEqual(['Qwen3.8 Flash'])
|
||||
})
|
||||
|
||||
it('renders plain text when the query is empty or does not occur in this label', () => {
|
||||
// Non-occurrence matters: rows can match the filter on id/slug while the
|
||||
// display label contains no occurrence — must not crash or mis-mark.
|
||||
for (const query of ['', ' ', 'zzz']) {
|
||||
const { container } = render(<HighlightMatches query={query} text="Fable 5" />)
|
||||
it('SPACE query marks the HYPHENATED text (model-picker polarity)', () => {
|
||||
const { container } = render(<HighlightMatches query="qwen3 8 flash" text="qwen3.8-flash" />)
|
||||
|
||||
expect(container.textContent).toBe('Fable 5')
|
||||
expect(container.querySelector('mark')).toBeNull()
|
||||
}
|
||||
expect(marksOf(container)).toEqual(['qwen3.8-flash'])
|
||||
})
|
||||
|
||||
it('normalizes the query the same way the pickers filter (trim + lowercase)', () => {
|
||||
const { container } = render(<HighlightMatches query=" GROK " text="grok-4.5" />)
|
||||
it('folding must not shift mark indices: slices return the ORIGINAL characters', () => {
|
||||
const { container } = render(<HighlightMatches query="gpt 5" text="GPT-5 Turbo" />)
|
||||
|
||||
expect(marksOf(container)).toEqual(['grok'])
|
||||
// mark covers "GPT-5" exactly — original characters, not folded ones.
|
||||
expect(marksOf(container)).toEqual(['GPT-5'])
|
||||
expect(container.textContent).toBe('GPT-5 Turbo')
|
||||
})
|
||||
|
||||
it('marks every term of a multi-term query (palette AND semantics)', () => {
|
||||
const { container } = render(<HighlightMatches query={['open', 'set']} text="Open Settings" />)
|
||||
it('literal spaces in the label are marked as part of the range', () => {
|
||||
const { container } = render(<HighlightMatches query="3 8 f" text="Qwen3.8 Flash" />)
|
||||
|
||||
expect(container.textContent).toBe('Open Settings')
|
||||
expect(marksOf(container)).toEqual(['Open', 'Set'])
|
||||
expect(marksOf(container)).toEqual(['3.8 F'])
|
||||
})
|
||||
|
||||
it('merges overlapping and adjacent term ranges into one mark', () => {
|
||||
const { container } = render(<HighlightMatches query={['gro', 'rok']} text="Grok" />)
|
||||
it('prefix queries still mark (existing contract intact)', () => {
|
||||
const { container } = render(<HighlightMatches query="qwen3.8" text="Qwen3.8 Flash" />)
|
||||
|
||||
expect(marksOf(container)).toEqual(['Grok'])
|
||||
expect(marksOf(container)).toEqual(['Qwen3.8'])
|
||||
})
|
||||
|
||||
it('non-matching queries render plain text', () => {
|
||||
const { container } = render(<HighlightMatches query="zzz-9" text="Qwen3.8 Flash" />)
|
||||
|
||||
expect(container.querySelector('mark')).toBeNull()
|
||||
expect(container.textContent).toBe('Qwen3.8 Flash')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import type { ReactNode } from 'react'
|
||||
|
||||
import { normalize } from '@/lib/text'
|
||||
import { normalize, searchFold } from '@/lib/text'
|
||||
import { cn } from '@/lib/utils'
|
||||
|
||||
/**
|
||||
@@ -13,9 +13,14 @@ import { cn } from '@/lib/utils'
|
||||
* The query must mirror the surface's OWN filter semantics, or the emphasis
|
||||
* lies about why a row matched:
|
||||
* - a string for literal-substring filters (the model pickers) — spaces and
|
||||
* all, exactly what `.includes()` saw;
|
||||
* all, exactly what the filter saw, after the shared `searchFold` runs on
|
||||
* BOTH sides (separators -_. and case stop mattering, mirroring
|
||||
* `foldIncludes` — a hyphen query must light up the spaced label);
|
||||
* - a string[] for per-term AND matchers (the command palette) — every term
|
||||
* is marked wherever it occurs, overlapping/adjacent ranges merged.
|
||||
*
|
||||
* The fold is length-preserving, so ranges found in the folded text index
|
||||
* the original unchanged — marks slice the original characters.
|
||||
*/
|
||||
export function HighlightMatches({
|
||||
className,
|
||||
@@ -32,7 +37,7 @@ export function HighlightMatches({
|
||||
return <>{text}</>
|
||||
}
|
||||
|
||||
const ranges = matchRanges(text.toLowerCase(), terms)
|
||||
const ranges = matchRanges(searchFold(text), terms.map(searchFold))
|
||||
|
||||
if (ranges.length === 0) {
|
||||
// No occurrence (the row matched on its id/slug/keywords, not this label).
|
||||
|
||||
@@ -0,0 +1,45 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { foldIncludes, searchFold } from './text'
|
||||
|
||||
describe('searchFold', () => {
|
||||
it('folds separator characters to spaces, both directions equivalent', () => {
|
||||
expect(searchFold('qwen3.8-flash')).toBe('qwen3 8 flash')
|
||||
expect(searchFold('Qwen3.8 Flash')).toBe('qwen3 8 flash')
|
||||
expect(searchFold('GPT-5')).toBe('gpt 5')
|
||||
expect(searchFold('gpt 5')).toBe('gpt 5')
|
||||
expect(searchFold('Q4_K_XL')).toBe('q4 k xl')
|
||||
})
|
||||
|
||||
it('is length-preserving (1 char in, 1 char out) so highlight indices survive', () => {
|
||||
for (const text of ['qwen3.8-flash', 'GPT-5', 'a--b__c..d', 'Grok 4.5 Retro', '']) {
|
||||
expect(searchFold(text).length).toBe(text.length)
|
||||
}
|
||||
})
|
||||
|
||||
it('already-folded text is unchanged (idempotent)', () => {
|
||||
expect(searchFold('qwen3 8 flash')).toBe('qwen3 8 flash')
|
||||
})
|
||||
})
|
||||
|
||||
describe('foldIncludes', () => {
|
||||
it('matches across the hyphen/space/dot/underscore separator difference', () => {
|
||||
expect(foldIncludes('Qwen3.8 Flash', 'qwen3.8-flash')).toBe(true)
|
||||
expect(foldIncludes('qwen3.8-flash', 'qwen3 8 flash')).toBe(true)
|
||||
expect(foldIncludes('GPT-5', 'gpt 5')).toBe(true)
|
||||
expect(foldIncludes('gpt-5', 'GPT.5')).toBe(true)
|
||||
expect(foldIncludes('Q4_K_XL', 'q4 k xl')).toBe(true)
|
||||
})
|
||||
|
||||
it('is a superset of the plain-lowercase includes it replaces', () => {
|
||||
// Any query that matched before must still match.
|
||||
expect(foldIncludes('qwen3.8-flash', 'qwen3.8')).toBe(true)
|
||||
expect(foldIncludes('Qwen3.8 Flash', 'flash')).toBe(true)
|
||||
expect(foldIncludes('nous-portal', 'nous')).toBe(true)
|
||||
})
|
||||
|
||||
it('still rejects genuinely different text', () => {
|
||||
expect(foldIncludes('Gemini Flash', 'qwen')).toBe(false)
|
||||
expect(foldIncludes('gpt-5', 'gpt-6')).toBe(false)
|
||||
})
|
||||
})
|
||||
@@ -27,3 +27,19 @@ export const firstStringField = (record: Record<string, unknown>, keys: readonly
|
||||
|
||||
return ''
|
||||
}
|
||||
|
||||
/** Search-equivalence fold: separators (`-`, `_`, `.`) and case stop matter —
|
||||
* `qwen3.8-flash`, `Qwen3.8 Flash` and `qwen3 8_flash` all fold to the same
|
||||
* string. One char in, one char out (length preserved), so highlight ranges
|
||||
* computed on folded text index the ORIGINAL text unchanged. Model ids use
|
||||
* hyphens, display names use spaces, quants use underscores — a picker search
|
||||
* must not care. Filter and highlight MUST call this on BOTH sides; a fold on
|
||||
* only one is what made hyphen queries match without ever highlighting. */
|
||||
export function searchFold(v: unknown): string {
|
||||
return asText(v).toLowerCase().replace(/[-_.]/g, ' ')
|
||||
}
|
||||
|
||||
/** `searchFold` + substring: the one matcher every searchable picker uses. */
|
||||
export function foldIncludes(text: unknown, query: unknown): boolean {
|
||||
return searchFold(text).includes(searchFold(query))
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user