diff --git a/ui/desktop/src/components/conversation/SearchBar.test.tsx b/ui/desktop/src/components/conversation/SearchBar.test.tsx index 1f73ef0d4..1c625c1b5 100644 --- a/ui/desktop/src/components/conversation/SearchBar.test.tsx +++ b/ui/desktop/src/components/conversation/SearchBar.test.tsx @@ -72,4 +72,81 @@ describe('SearchBar', () => { expect(onClose).not.toHaveBeenCalled(); }); + + /** + * The minimum length used to be the literal `2`, the same on every surface + * that mounts this bar. Two of them highlight a transcript, where a + * one-character term is hundreds of forced layouts; the rest only filter a + * short list of rows, where it is the whole point of the matcher's + * short-term rule. The floor is a prop now, and this is both halves of it. + */ + describe('minimum search length', () => { + const type = (value: string) => + fireEvent.change(screen.getByPlaceholderText('Search chat...'), { target: { value } }); + + it('searches a one-character term on a surface whose floor is one', () => { + vi.useFakeTimers(); + const onSearch = vi.fn(); + render(); + + type('r'); + act(() => vi.advanceTimersByTime(200)); + + expect(onSearch).toHaveBeenCalledWith('r', false); + expect(screen.queryByTestId('conversation-search-bar-minimum')).not.toBeInTheDocument(); + }); + + it('says it has not searched yet when the term is below the floor', () => { + vi.useFakeTimers(); + const onSearch = vi.fn(); + render(); + + type('r'); + act(() => vi.advanceTimersByTime(200)); + + // The term it reports is empty — a list consumer draws its whole catalog + // — so the bar has to SAY that, or the control looks like it filtered. + expect(onSearch).toHaveBeenCalledWith('', false); + expect(onSearch).not.toHaveBeenCalledWith('r', false); + expect(screen.getByTestId('conversation-search-bar-minimum')).toHaveTextContent( + 'Type at least 2 characters to search' + ); + }); + + it('says nothing when the box is empty, which is browsing and not a short query', () => { + render(); + + type('r'); + type(''); + + expect(screen.queryByTestId('conversation-search-bar-minimum')).not.toBeInTheDocument(); + }); + + it('asks the reveal animation for room for the second row', () => { + // The bar opens under a `max-height` transition with `overflow: hidden`, + // so the ceiling clips rather than scrolls: without this class the + // sentence above is cut through its middle. jsdom cannot see that — the + // stylesheet's half is asserted in `styles/searchBarNote.test.ts`. + render(); + + type('r'); + + expect(screen.getByTestId('conversation-search-bar').parentElement).toHaveClass( + 'search-bar-has-note' + ); + }); + + it('does not let the case toggle search a term typing was refused', () => { + vi.useFakeTimers(); + const onSearch = vi.fn(); + render(); + + type('r'); + fireEvent.click(screen.getByTitle('Case sensitive')); + act(() => vi.advanceTimersByTime(200)); + + expect(onSearch).not.toHaveBeenCalledWith('r', true); + expect(onSearch).not.toHaveBeenCalledWith('r', false); + }); + }); }); diff --git a/ui/desktop/src/components/conversation/SearchBar.tsx b/ui/desktop/src/components/conversation/SearchBar.tsx index 1d8ba8a76..78481b0fe 100644 --- a/ui/desktop/src/components/conversation/SearchBar.tsx +++ b/ui/desktop/src/components/conversation/SearchBar.tsx @@ -24,8 +24,33 @@ interface SearchBarProps { initialSearchTerm?: string; /** Placeholder text for the search input */ placeholder?: string; + /** + * How many characters this surface needs before it will search. Below it the + * bar reports an EMPTY term and says so in as many words — see + * {@link DEFAULT_MIN_SEARCH_LENGTH}. + */ + minSearchLength?: number; } +/** + * The floor the HIGHLIGHTING surfaces keep — the chat and a saved transcript. + * + * ⚠ **It is a measured cost, not a taste.** `SearchView` answers a term by + * walking its container's text nodes and creating a positioned overlay element + * per match, and every match costs a `range.getClientRects()`, which is a forced + * layout. Measured 2026-09-12 by replaying that loop in the running app over a + * SHORT chat (2,170 characters of transcript): `e` found 217 matches and took + * 432 ms to build the overlay and `a` 115 matches / 223 ms, against 40 ms for + * the two-character `er` and 31 ms for `the`. The work is linear in matches at + * ~2 ms each and a real transcript is two orders of magnitude longer, so a + * one-character find there is seconds of blocked layout. + * + * A surface that only FILTERS A LIST pays none of that — its rows are filtered + * before the highlighter ever sees them — so it passes `minSearchLength={1}` + * and a one-character query reaches its matcher. + */ +export const DEFAULT_MIN_SEARCH_LENGTH = 2; + /** * SearchBar provides a search input with case-sensitive toggle and result navigation. */ @@ -37,6 +62,7 @@ export const SearchBar: React.FC = ({ inputRef: externalInputRef, initialSearchTerm = '', placeholder = 'Search chat...', + minSearchLength = DEFAULT_MIN_SEARCH_LENGTH, }: SearchBarProps) => { const [searchTerm, setSearchTerm] = useState(initialSearchTerm); const [caseSensitive, setCaseSensitive] = useState(false); @@ -73,11 +99,11 @@ export const SearchBar: React.FC = ({ useEffect(() => { if (initialSearchTerm) { setSearchTerm(initialSearchTerm); - if (initialSearchTerm.length >= 2) { + if (initialSearchTerm.length >= minSearchLength) { debouncedSearchRef.current?.(initialSearchTerm, caseSensitive); } } - }, [initialSearchTerm, caseSensitive, debouncedSearchRef]); + }, [initialSearchTerm, caseSensitive, debouncedSearchRef, minSearchLength]); const [localSearchResults, setLocalSearchResults] = useState(undefined); @@ -102,11 +128,13 @@ export const SearchBar: React.FC = ({ // Update display term immediately for UI feedback setSearchTerm(value); - // Only trigger search if we have 2 or more characters - if (value.length >= 2) { + // Only trigger a search once the surface's minimum is reached. Below it the + // term reported is EMPTY, which every list consumer reads as "no filter" — + // so the bar owes the user the sentence below rather than a list that looks + // filtered and is not. + if (value.length >= minSearchLength) { debouncedSearchRef.current?.(value, caseSensitive); } else { - // Clear results if less than 2 characters onSearch('', caseSensitive); } }; @@ -133,8 +161,10 @@ export const SearchBar: React.FC = ({ const toggleCaseSensitive = () => { const newCaseSensitive = !caseSensitive; setCaseSensitive(newCaseSensitive); - // Immediately trigger a new search with updated case sensitivity - if (searchTerm) { + // Immediately trigger a new search with updated case sensitivity. Guarded on + // the SAME floor as typing: on `if (searchTerm)` a below-minimum term that + // `handleSearch` had refused was searched anyway the moment `Aa` was clicked. + if (searchTerm.length >= minSearchLength) { debouncedSearchRef.current?.(searchTerm, newCaseSensitive); } inputRef.current?.focus(); @@ -181,85 +211,107 @@ export const SearchBar: React.FC = ({ const hasResults = searchResults && searchResults.count > 0; + // The typed term is short of this surface's floor, so nothing was searched. + // An empty box is not short of it: that is browsing, and the full list is the + // honest answer there. + const belowMinimum = searchTerm.length > 0 && searchTerm.length < minSearchLength; + return (
-
- -
- -
+
+
+ +
+ +
-
-
-
- {(() => { - return localSearchResults?.count && localSearchResults.count > 0 && searchTerm - ? `${localSearchResults.currentIndex}/${localSearchResults.count}` - : null; - })()} +
+
+
+ {(() => { + return localSearchResults?.count && localSearchResults.count > 0 && searchTerm + ? `${localSearchResults.currentIndex}/${localSearchResults.count}` + : null; + })()} +
-
- -
- -
+
+ +
+ + +
+
+
- -
+ {`Type at least ${minSearchLength} characters to search — everything is still showing.`} +
+ )}
); diff --git a/ui/desktop/src/components/conversation/SearchView.test.tsx b/ui/desktop/src/components/conversation/SearchView.test.tsx index 08985abeb..55d1cbb80 100644 --- a/ui/desktop/src/components/conversation/SearchView.test.tsx +++ b/ui/desktop/src/components/conversation/SearchView.test.tsx @@ -10,10 +10,17 @@ const highlighterMocks = vi.hoisted(() => ({ })); vi.mock('./SearchBar', () => ({ - default: ({ onSearch }: { onSearch: (term: string, caseSensitive: boolean) => void }) => ( + default: ({ + onSearch, + minSearchLength, + }: { + onSearch: (term: string, caseSensitive: boolean) => void; + minSearchLength?: number; + }) => (
+ {String(minSearchLength)}
), })); @@ -43,6 +50,21 @@ describe('SearchView', () => { vi.useRealTimers(); }); + // The floor is the BAR's, and every surface's choice of it arrives through + // here. Unforwarded, a list view asking for a one-character search silently + // got the two-character default and its query never ran. + it("hands the surface's minimum search length to the bar", () => { + render( + +

alpha beta

+ + ); + + fireEvent.keyDown(window, { key: 'f', metaKey: true }); + + expect(screen.getByTestId('min-search-length')).toHaveTextContent('1'); + }); + it('cancels a pending highlight when a newer term arrives', () => { render( diff --git a/ui/desktop/src/components/conversation/SearchView.tsx b/ui/desktop/src/components/conversation/SearchView.tsx index bcdb140f3..e1f1465b8 100644 --- a/ui/desktop/src/components/conversation/SearchView.tsx +++ b/ui/desktop/src/components/conversation/SearchView.tsx @@ -20,6 +20,13 @@ interface SearchViewProps { } | null; /** Placeholder text for the search input */ placeholder?: string; + /** + * How many characters this surface needs before it searches. Defaults to + * {@link DEFAULT_MIN_SEARCH_LENGTH}, which is what the highlighting surfaces + * (the chat, a saved transcript) can afford; a surface that only filters a + * list of rows passes 1 so a one-character query reaches its matcher. + */ + minSearchLength?: number; } interface SearchContainerElement extends HTMLDivElement { @@ -38,6 +45,7 @@ export const SearchView: React.FC> = ({ onNavigate, searchResults, placeholder, + minSearchLength, }) => { const [isSearchVisible, setIsSearchVisible] = useState(false); const [initialSearchTerm, setInitialSearchTerm] = useState(''); @@ -388,6 +396,7 @@ export const SearchView: React.FC> = ({ inputRef={searchInputRef} initialSearchTerm={initialSearchTerm} placeholder={placeholder} + minSearchLength={minSearchLength} /> )} {children} diff --git a/ui/desktop/src/components/skills/SkillsView.test.tsx b/ui/desktop/src/components/skills/SkillsView.test.tsx index a1fdebeff..634ce3336 100644 --- a/ui/desktop/src/components/skills/SkillsView.test.tsx +++ b/ui/desktop/src/components/skills/SkillsView.test.tsx @@ -2,6 +2,7 @@ import { fireEvent, render, screen, waitFor, within } from '@testing-library/rea import { beforeEach, describe, expect, it, vi } from 'vitest'; import SkillsView from './SkillsView'; import type { CatalogBundle, CatalogSkill, CatalogView } from '../../api'; +import { DEFAULT_MIN_SEARCH_LENGTH } from '../conversation/SearchBar'; const mocks = vi.hoisted(() => ({ skillCatalogHandler: vi.fn(), @@ -44,16 +45,33 @@ vi.mock('../Layout/ReadableContent', () => ({ // The real SearchView owns a cmd-F overlay and a scroll-area contract; all the // view reads from it is the term it reports, so the mock is that one wire — // without it the search-filtered branches below are unreachable from a test. +// +// ⚠ **The mock carries the MINIMUM-LENGTH floor, because the real bar does.** +// It used to hand every keystroke straight through, so the `R` tests below were +// green while measuring nothing: in the app a one-character query never reached +// this view at all — `SearchBar` reported an empty term and the whole catalog +// rendered under its provenance headings. The floor is imported rather than +// retyped so the mock cannot drift from the component it stands in for, and the +// bar's own half of the contract is asserted directly in `SearchBar.test.tsx`. +const searchMocks = vi.hoisted(() => ({ defaultFloor: 2 })); + vi.mock('../conversation/SearchView', () => ({ SearchView: ({ children, onSearch, + minSearchLength = searchMocks.defaultFloor, }: { children: React.ReactNode; onSearch: (term: string, caseSensitive: boolean) => void; + minSearchLength?: number; }) => (
- onSearch(event.target.value, false)} /> + + onSearch(event.target.value.length >= minSearchLength ? event.target.value : '', false) + } + /> {children}
), @@ -452,6 +470,14 @@ describe('SkillsView search', () => { const search = (term: string) => fireEvent.change(screen.getByLabelText('Search skills'), { target: { value: term } }); + it("stands in for the bar with the bar's own default floor", () => { + // The mock cannot import the constant — its factory is hoisted above the + // imports — so the two are pinned here instead. Without this the default + // could move and the one-letter tests below would go green again while + // measuring a floor the app does not have. + expect(DEFAULT_MIN_SEARCH_LENGTH).toBe(searchMocks.defaultFloor); + }); + it('finds the skills a multi-word phrase names, best match first', async () => { serve({ skills: [skill('ggplot'), skill('pdf'), skill('r-scripting')] }); render(); @@ -489,6 +515,26 @@ describe('SkillsView search', () => { expect(screen.queryByText('markdown-render')).not.toBeInTheDocument(); }); + /** + * The defect this view had until the floor became per-surface: `SearchBar` + * refused anything under two characters and reported an EMPTY term, which + * this view reads as "browsing". Measured in the running app before the fix, + * on a two-skill catalog: `z` showed both rows under `FROM THIS PROJECT (2)` + * while `zz` correctly showed "No matching skills" — a control that looks + * like it filtered and did not. + */ + it('answers a one-letter query that matches nothing, instead of showing everything', async () => { + serve({ skills: [skill('markdown-render'), skill('r-scripting')] }); + render(); + await screen.findByText('r-scripting'); + + search('z'); + + expect(await screen.findByText('No matching skills')).toBeInTheDocument(); + expect(screen.queryByText('r-scripting')).not.toBeInTheDocument(); + expect(screen.queryByText(/Biorouter Skills/)).not.toBeInTheDocument(); + }); + it('keeps the provenance groups when nothing is typed', async () => { serve({ skills: [ diff --git a/ui/desktop/src/components/skills/SkillsView.tsx b/ui/desktop/src/components/skills/SkillsView.tsx index 4af02f622..16feedb40 100644 --- a/ui/desktop/src/components/skills/SkillsView.tsx +++ b/ui/desktop/src/components/skills/SkillsView.tsx @@ -210,6 +210,17 @@ export default function SkillsView() { setSearchTerm(term)} placeholder="Search skills..." + /* ⚠ **One character is a real query here.** The shared matcher's + short-term rule (`baam/search.ts`) exists for exactly this: a term + under three characters matches whole WORDS, so `R` finds the R + skills and not every row holding the letter. The bar's default + floor of two characters put that query out of reach — `R` reported + an EMPTY term and this view rendered its whole catalog under the + provenance headings, which reads as a filter that matched + everything. Filtering a handful of rows costs none of what the + floor is there to prevent: the highlighter that pays it runs over + the rows this filter already dropped. */ + minSearchLength={1} > {catalog.error && ( diff --git a/ui/desktop/src/styles/search.css b/ui/desktop/src/styles/search.css index 35ea6c6f1..ff8a707ff 100644 --- a/ui/desktop/src/styles/search.css +++ b/ui/desktop/src/styles/search.css @@ -31,6 +31,18 @@ transition: max-height 150ms ease-out; } +/* ⚠ **The ceiling has to clear what the bar actually renders.** The reveal is + a `max-height` transition, and `overflow: hidden` is what makes it one — so a + bar that is TALLER than the ceiling is silently cut off rather than scrolled. + When the bar is saying it has not searched yet it is two rows, and at the + one-row 72px the sentence was clipped through its middle (measured in the app + on Settings → Extensions, 2026-09-12). Raised only while that row is there, + so the ordinary open/close keeps the travel it was tuned for. */ +.search-bar-enter.search-bar-has-note, +.search-bar-exit.search-bar-has-note { + --search-bar-height: 132px; +} + .search-bar-enter { max-height: var(--search-bar-height); } diff --git a/ui/desktop/src/styles/searchBarNote.test.ts b/ui/desktop/src/styles/searchBarNote.test.ts new file mode 100644 index 000000000..eca71d047 --- /dev/null +++ b/ui/desktop/src/styles/searchBarNote.test.ts @@ -0,0 +1,49 @@ +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { describe, expect, it } from 'vitest'; + +/** + * The search bar's open/close reveal is a `max-height` transition under + * `overflow: hidden`, so the ceiling is also a CLIP: a bar taller than + * `--search-bar-height` is cut off, not scrolled. + * + * When the bar is telling the user that the term is too short to search, it is + * two rows rather than one, and the one-row ceiling cut that sentence through + * its middle — measured in the running app on Settings → Extensions before this + * rule existed. `SearchBar` adds `search-bar-has-note` for exactly that state + * (asserted in `SearchBar.test.tsx`, which can read the class but not the + * layout) and this is the stylesheet's half: the class has to buy real room. + * + * ⚠ **Asserted at the source, and it has to be.** jsdom has no layout engine and + * never loads this file, so a component test that reads `maxHeight` sees the + * empty string whether the rule exists or not. + */ +const CSS = readFileSync(join(__dirname, 'search.css'), 'utf8'); + +/** The `--search-bar-height` a selector sets, in px. */ +function ceiling(selector: RegExp): number { + const rule = CSS.match(new RegExp(`${selector.source}[^}]*\\}`)); + expect(rule, `no rule matching ${selector}`).not.toBeNull(); + const value = rule![0].match(/--search-bar-height:\s*(\d+)px/); + expect(value, `no --search-bar-height in ${rule![0]}`).not.toBeNull(); + return Number(value![1]); +} + +describe('the search bar reveal ceiling', () => { + it('gives the two-row bar more room than the one-row bar', () => { + const oneRow = ceiling(/\.search-bar-enter,\s*\n?\s*\.search-bar-exit\s*\{/); + const withNote = ceiling( + /\.search-bar-enter\.search-bar-has-note,\s*\n?\s*\.search-bar-exit\.search-bar-has-note\s*\{/ + ); + + // The note row measured 33px in the app (py-2 + a line + its hairline), and + // it wraps to two lines in a narrow window. + expect(withNote).toBeGreaterThanOrEqual(oneRow + 33); + }); + + it('still collapses to nothing on the way out', () => { + // The exit animates to 0 whatever the ceiling is; a `has-note` bar that + // raised the floor instead of the ceiling would never close. + expect(CSS).toMatch(/\.search-bar-exit\s*\{\s*max-height:\s*0;/); + }); +});