Feat/wire event search worker into dashboard - #443
Merged
Osuochasam merged 3 commits intoAug 30, 2026
Merged
Conversation
The wiring existed once (2255a0d) and a merge (e5fedda) deleted half of it, leaving main in a state where app/dashboard/DashboardClient.tsx did not parse: the useEventSearch destructuring block and the index-building effect were dropped, and a duplicate half-written filteredEvents useMemo was left in their place, while searchHits/buildIndex/addSearchEvents/isSearching were still referenced further down the file. Restores the destructuring and replaces the broken fragment with a correct index lifecycle. Incremental indexing The pre-merge effect diffed arrays by object identity (!liveEvents.includes(e) || liveEvents.indexOf(e) === allEvents.indexOf(e)) and then sent an arbitrary slice of at most 20. That is not a correct incremental path, so it is not what was restored. The effect now tracks indexed event ids in a ref and sends the worker only ids it has not seen. That is stable across re-renders that produce new object identities for the same events, and across an event arriving twice. BUILD_INDEX runs once; every later arrival is an ADD_EVENTS. Double-add fix handleNewEvent also called addSearchEvents([event]) directly, so every streamed event was sent to the worker twice — once by the callback and once by the effect. Indexing now lives only in the effect, which is the path that knows what is already indexed. Highlighting searchValue is passed to EventFeedTable as highlightQuery. isIndexed drives the input placeholder, so the field says "Building search index…" until the index is ready instead of silently returning nothing. 13 integration tests covering what the unit tests do not: that the component builds once and adds incrementally, debounces (no worker message per keystroke), filters the feed by the returned hits, restores the feed when the query clears, surfaces a worker error, and terminates the worker on unmount including across repeated mount/unmount cycles.
Adds an optional highlightQuery prop to EventFeedTable. When set, matching substrings in the translated description and the event type are wrapped in <mark>, so the user can see why a row matched rather than only that it did. The query is escaped before being built into a RegExp. Contract and function names routinely contain regex metacharacters — transfer(, a.b, x[0] — and an unescaped transfer( is an unterminated group that would throw while rendering the feed. No query means the plain string is returned, so ordinary renders do no extra work and emit no wrapper elements. 9 tests: multiple occurrences, case-insensitive matching that preserves original casing, event type as well as description, the metacharacter case, and that surrounding text stays intact.
@testing-library/react needs @testing-library/dom at runtime but does not depend on it directly. Without it lib/workers/eventSearchClient.test.ts — 8 existing tests for the search hook — failed at collection with "Cannot find module @testing-library/dom" and had never actually run. Installing it makes those 8 pre-existing tests execute (and pass), and lets the new dashboard integration tests render components at all.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Wires the in-browser event search worker into the dashboard's live feed: index lifecycle, debounced querying, result highlighting, and worker teardown.
Closes #419
What the issue predicted vs. what was actually there
The issue describes the worker and client as fully-built but unimported. That was true when it was written; it isn't quite the situation on
mainnow, and the difference matters.The wiring existed in commit
2255a0d, and a later merge (e5fedda) deleted half of it.app/dashboard/DashboardClient.tsxonmaindoes not parse:useEventSearchdestructuring block was droppeduseEffectwas replaced by a duplicate, half-writtenfilteredEventsuseMemosearchHits,buildIndex,addSearchEvents,isSearching,isIndexed,searchErrorandisFallbackare still referenced further down the fileThe search input, loading spinner, error display and fallback notice all survived the merge. So this PR is repair plus correction, not a greenfield integration.
Changes
app/dashboard/DashboardClient.tsxRestored the
useEventSearchdestructuring and the index lifecycle.Did not restore the pre-merge incremental logic — it was wrong. It diffed events by object identity:
then sent an arbitrary slice of at most 20. That is not a correct incremental path. The effect now tracks indexed event IDs in a ref and sends the worker only IDs it hasn't seen — stable across re-renders that produce new object identities for the same events, and across the same event arriving twice.
BUILD_INDEXruns once; every later arrival is anADD_EVENTS.Fixed a double-add bug.
handleNewEventcalledaddSearchEvents([event])directly and the effect added it again, so every streamed event was sent to the worker twice. Indexing now lives only in the effect — the path that knows what is already indexed. The integration test caught this (3 calls where 2 were expected).isIndexednow drives the input placeholder, so the field reads "Building search index…" rather than silently returning nothing before the index is ready.components/dashboard/EventFeedTable.tsxHighlighting was entirely absent — the component had no such prop. Added optional
highlightQuery; matching substrings in the translated description and event type are wrapped in<mark>.The query is escaped before being built into a
RegExp. Contract and function names routinely contain regex metacharacters (transfer(,a.b,x[0]), and an unescapedtransfer(is an unterminated group that would throw while rendering the feed.package.jsonAdded
@testing-library/dom— see below.Two things the broken file was masking
tscwas hiding 10 real errors. Onmain,tsc --noEmitreports only 5 errors, all syntax errors inDashboardClient.tsx. TypeScript skips semantic checking when syntactic errors exist, so 10 pre-existing type errors inlib/dag/,components/dag/andsrc/worker/indexer.tswere never surfacing. They are visible now. That is Issue #8's subsystem and out of scope here, but the typecheck will look worse after this PR merges — those errors predate it and are unrelated to search.8 existing tests had never run.
@testing-library/reactrequires@testing-library/domat runtime but doesn't depend on it directly, and it was not installed.lib/workers/eventSearchClient.test.ts— 8 tests for the search hook — failed at collection withCannot find module '@testing-library/dom'. Installing the peer dep makes them execute, and they pass.Existing coverage checked before writing tests
Per the issue's instruction not to duplicate:
lib/workers/eventSearchClient.unit.test.ts— the client class and its lifecycle against a mockWorkerlib/workers/eventSearchClient.test.ts— theuseEventSearchhook's debounce, lifecycle and incremental updates against a mocked clientNeither exercises the component. That is what the merge broke, and that is what the new tests cover.
New tests
app/dashboard/DashboardClient.search.test.tsx(13) —EventSearchClientis mocked so the protocol is observable; the component and hook are real.ADD_EVENTSand no rebuildcomponents/dashboard/EventFeedTable.highlight.test.tsx(9) — multiple occurrences, case-insensitive matching that preserves original casing, event type as well as description, no marks without a query, and the regex-metacharacter case.Debounce tests use
fireEvent, notuser-event: user-event's internal awaits deadlock againstvi.useFakeTimers(), and these tests exist specifically to control the debounce clock.Fallback behaviour
Already implemented in
useEventSearchand left as-is: if theWorkerconstructor throws, the hook setsisFallbackand runs the same search synchronously on the main thread over a locally-held event list. The dashboard renders "Running search on main thread (Web Worker unavailable) — results may be slower." Documenting it here since the issue asked for the behaviour to be defined rather than left undefined.Verification
tsc --noEmitclean on both files touched.Acceptance criteria
BUILD_INDEX, thenADD_EVENTSper arrival, asserted over a 5-event streamOut of scope
Server-side historical search (Issue #10) is untouched; the contract-ID
SearchBarstill hits that API unchanged. Ranking is the worker's —SearchResponse.hits[].scoreis used as returned, not redesigned. The DAG subsystem (Issue #8) is untouched, including its now-visible type errors.