From bcd1ef9ebc2007665a4ffd024aa7375ae8f15902 Mon Sep 17 00:00:00 2001 From: dvcdsys Date: Fri, 21 Aug 2026 15:07:40 +0100 Subject: [PATCH] fix(dashboard): search on Enter, not while typing; repoint three stale comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TWO UNRELATED THINGS, BOTH SMALL, BOTH USER-VISIBLE OR READER-VISIBLE. SEARCH FIRED WHILE TYPING /search debounced the input into the URL after 250ms idle, and a change to the URL is what runs a search. That is the usual pattern and it is wrong here: a semantic search embeds the query through the configured provider, so every pause while typing spent a real API call and a full fan-out to answer a half-written question. "retry with exponential backoff" typed at a normal pace fires on "retry", "retry with", "retry with expo" — three searches nobody asked for and one they did. On a metered provider that is money; on a local sidecar it is a queue of pointless work in front of the query the user meant. Typing now changes local state and nothing else. The URL — and therefore the search — moves only on submit. SearchBar already had the onSubmit path; only the debounce had to go. The empty state says "press Enter to search" instead of implying results appear on their own. Verified through the real component in devmock, which boots the app with a mock fetch and no login: typing 31 characters one at a time issues ZERO search requests, and submitting issues exactly one, with no navigation. A note on how that was verified, because the first attempt was worthless: driving Enter through the browser-automation key API produced a page "reload" that looked like a regression. It was not — a keydown listener on the input recorded NOTHING, so those key events never reached the page and that test asserted nothing at all. The real check goes through form.requestSubmit(), which is the exact path a keypress takes. THREE STALE COMMENTS IN workspacesearch.go All three are from #265, all three describe code that commit changed: - projectHits' doc said the two sides "are fused inside the goroutine". They are not — fuseRRF runs in the serial loop after g.Wait(), and the comment above that loop says so in as many words. The struct doc contradicted a comment 650 lines below it. - the handler doc said "each project runs two queries in parallel: dense and sparse". There is one BM25 query for the whole workspace now, which is what #265 was. - BM25Signal's doc explained its normalization but never said it is computed on the RAW, unfused list while FusedChunks beside it is post-RRF. That asymmetry decides whether a panel reorder means what it appears to mean, and the one place a reader would look for it did not mention it. Same class as 7cc70a2 and as two commits in #266: the code moved and the comment above it did not. go test ./... green, go vet clean, gofmt clean on the touched files. Dashboard built with `npm run build` (tsc -b + vite); dashboard build is not on PR CI, so it was validated locally. Co-Authored-By: Claude Opus 5 --- .../src/modules/search/SearchPage.tsx | 27 ++++++++++--------- .../modules/search/components/SearchBar.tsx | 2 +- server/internal/httpapi/workspacesearch.go | 21 ++++++++++----- 3 files changed, 30 insertions(+), 20 deletions(-) diff --git a/server/dashboard/src/modules/search/SearchPage.tsx b/server/dashboard/src/modules/search/SearchPage.tsx index 2c9cd04..f61d8ca 100644 --- a/server/dashboard/src/modules/search/SearchPage.tsx +++ b/server/dashboard/src/modules/search/SearchPage.tsx @@ -44,18 +44,15 @@ export default function SearchPage() { const queryParam = params.get('q') ?? ''; const [draft, setDraft] = useState(queryParam); - // Debounce input → URL after 250ms idle; Enter commits immediately. - useEffect(() => { - const id = setTimeout(() => { - if (draft === queryParam) return; - const next = new URLSearchParams(params); - if (draft.trim()) next.set('q', draft); - else next.delete('q'); - setParams(next, { replace: true }); - }, 250); - return () => clearTimeout(id); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [draft]); + // Typing changes `draft` and nothing else. The query in the URL — which is + // what actually runs a search — moves only on submit. + // + // This used to debounce draft into the URL after 250ms idle. That is the + // usual pattern and it is wrong here: a semantic search embeds the query + // through the configured provider, so every pause while typing spent a real + // API call and a full fan-out to answer a half-written question. "retry with + // exponential backoff" typed at a normal pace fires on "retry", "retry with", + // "retry with expo" — three searches nobody asked for and one they did. // Follow the URL when it changes from outside (a pasted link, back button). useEffect(() => { @@ -172,7 +169,11 @@ function Results({ ); } if (query.trim().length < 2) { - return At least two characters, then results appear here.; + return ( + + At least two characters, then press Enter to search. + + ); } switch (mode) { case 'semantic': diff --git a/server/dashboard/src/modules/search/components/SearchBar.tsx b/server/dashboard/src/modules/search/components/SearchBar.tsx index d6570bf..92977e3 100644 --- a/server/dashboard/src/modules/search/components/SearchBar.tsx +++ b/server/dashboard/src/modules/search/components/SearchBar.tsx @@ -13,7 +13,7 @@ export function SearchBar({ }: { value: string; onChange: (v: string) => void; - /** Fired on Enter — bypasses the debounce and commits immediately. */ + /** Fired on Enter. This is the ONLY thing that runs a search — typing does not. */ onSubmit?: (v: string) => void; placeholder?: string; className?: string; diff --git a/server/internal/httpapi/workspacesearch.go b/server/internal/httpapi/workspacesearch.go index e3ca4d3..757d0ba 100644 --- a/server/internal/httpapi/workspacesearch.go +++ b/server/internal/httpapi/workspacesearch.go @@ -96,8 +96,10 @@ type workspaceSearchStaleFTSRepoPayload struct { } // projectHits is the per-project intermediate state accumulated across -// the parallel fan-out. Dense and BM25 sides arrive separately and are -// fused inside the goroutine before being collected. +// the parallel fan-out. Dense and BM25 sides arrive separately and are fused +// AFTER the fan-out, in the serial loop below g.Wait() — fusion needs both +// sides, so it cannot live in either goroutine. See the comment above that +// loop for why parallelising it buys nothing. type projectHits struct { ProjectPath string // FusedChunks are the per-project chunks ranked by RRF over the @@ -110,6 +112,13 @@ type projectHits struct { // (positive, unbounded — SQLite's bm25() flipped via -bm25 at // the chunksfts boundary). Normalized into candidacy via // per-query min-max before being blended. + // + // Computed on the RAW BM25 list, not on FusedChunks beside it. The two + // fields are scored at different layers: the chunk list a caller sees has + // been through RRF, where the dense side gets an equal vote, while the + // projects panel ranks on this number alone. So anything that moves BM25 + // moves the panel directly and the chunk list only after fusion has had a + // say — worth knowing before reading a panel reorder as a ranking change. BM25Signal float32 // Candidacy is the α-blended, per-query-normalized score the // projects panel ranks by; recomputed after every project's @@ -119,10 +128,10 @@ type projectHits struct { // WorkspaceSearch — GET /api/v1/workspaces/{id}/search. // -// Hybrid BM25+dense fan-out. Each project runs two queries in -// parallel: dense (vector-store cosine) and sparse (SQLite FTS5 BM25 over -// chunks_fts). Per project, the two ranked lists are fused via -// Reciprocal Rank Fusion. Across projects, an α-blended candidacy +// Hybrid BM25+dense fan-out. Dense (vector-store cosine) runs once per +// project, concurrently; sparse is ONE FTS5 BM25 query over chunks_fts for the +// whole workspace, partitioned per project by the caller. Per project, the two +// ranked lists are fused via Reciprocal Rank Fusion. Across projects, an α-blended candidacy // score (with per-query min-max normalization on both signals) plus // a relative threshold (`candidacy ≥ best × 0.4`) keeps the result // set focused on repos that actually share vocabulary or semantics