[web-console] Implement bi-directional search bar for logs tab and profiler metrics by Karakatiza666 · Pull Request #6681 · feldera/feldera

Karakatiza666 · 2026-07-20T12:23:45Z

This adds a re-usable searchbar implementation

Switch what search bar Ctrl+F focuses in profiler based on context

The new search bar can be opened with a dedicated button or Ctrl+F and has buttons for forward and backward navigation, and responds to Enter and Shift+Enter shortcuts

Extend search to include top nodes, global profile stats
#6508

Screencast.From.2026-07-20.14-49-47.webm

mythical-fred

Nicely done. The SearchProgress single-source-of-truth pattern is the right shape — counter, highlight, and nav all derive from results, and editing/clearing/closing all funnel through onclear. Test coverage is thorough: the shared SearchBar component gets its own spec, lookup.test.ts locks down the coordinator contract, metricsSearch.test.ts exercises the tiered matcher, and the MonitoringPanel spec was updated to drive the popup end-to-end (including the Ctrl+F → Esc → Ctrl+F reopen flow).

A few small non-blocking observations:

Two useShortcut handlers race for isFindShortcut in SupportBundleViewerLayout. Both are window-capture keydown listeners registered in order. When the graph is focused, the first calls preventDefault; the second still runs, returns false, and no-ops. When the graph is not focused, the first returns false (native find remains theoretically available for a brief moment before the second listener), and the second opens the search bar with preventDefault. The current behavior is correct because Chrome fires the browser find only after all listeners resolve, so a later preventDefault still cancels it. Still, one combined useShortcut with a branch (if diagram focused → node search, else → tab search) would be less subtle and easier to reason about six months from now.
matchCursor and searchQuery in MetricsView are plain let, not $state. They're only read/written inside runSearch, which is invoked imperatively via the lookup coordinator — so no reactivity is needed and this is fine. A one-liner comment (// imperative — no $state on purpose) would prevent a well-meaning future contributor from "fixing" them.
ProfilerDiagram.isFocused() uses pointerOnGraph as a proxy for focus because the visualizer swallows its own pointer events. Comment already explains it; nothing to change. Just note that a keyboard-only user who tabs into the graph area cannot use Ctrl+F to focus "Search node" — they'd land on the tab search instead. Very niche; a11y-wise the graph isn't really keyboard-navigable today anyway.
PR title typo — "bi-directinal" → "bi-directional". Worth fixing in the squash commit message.
Two nested destructurings got reformatted in MetricsView.svelte and IssuesTab.svelte (prettier wrap width). Purely cosmetic, no behavior change — fine.

Everything I looked at holds together. Approving.

mythical-fred

Re-review at b9b49a23c039f6b55e7af7cdb3fc4e76c66be5a5. Substantive changes since my earlier APPROVE at 0cd79fea (not a rebase-only update):

New shared common-ui/SearchBar.svelte popup (icon toggle + input + "x of X" / "No results" counter + prev/next/close). Presentational only — host owns the query and drives the search.
New common-ui/useShortcut.svelte.ts — capture-phase window keydown scoped to an isActive getter. SupportBundleViewerLayout now uses one handler that routes Ctrl/Cmd-F to the graph's "Search node" input when the diagram is focused and to the tab's SearchBar otherwise, collapsing the two prior useShortcut calls into a single context-switching path.
New metricsSearch.ts (buildSearchTargets + matchTargets) with three-tier title → labels → keys ranking and dedup. Metrics search now covers the overview "Global stats" tile and every "Top nodes" table row (each row carries a data-block-id="top-node-<i>" anchor), on top of the per-node metric blocks.
advanceSearch(state, next, direction: 'next'|'prev') for bi-directional stepping; findOccurrence still wraps modulo count.
LookupCoordinator.execute returns SearchProgress — each tab handler (BundleLogsView, TriageResultsView, MetricsView) now reports {current, total} so the layout drives the counter and enables/disables the nav buttons off a single source of truth.
LogList drops onSearchShortcut (the host owns Ctrl/Cmd-F via useShortcut now) and exposes onMatchCountChange from a $derived countOccurrences.
MonitoringPanel swaps the ad-hoc <input> for <SearchBar>, wires useShortcut(isFindShortcut, ..., () => currentTab === 'Logs'), and derives searchResults from logSearch + logMatchCount.
Explicit // Imperative — no $state on purpose comment on the matchCursor / searchQuery pair in MetricsView (the intentionality note I asked about last round).
Node search input on the graph panel now blurs on Escape.
PR title typo fixed (bi-directional) — the commit subject still says bi-directinal, but GitHub's squash uses the PR title.

Test coverage matches the growth: metricsSearch.test.ts (buildSearchTargets shapes + matchTargets three-tier ranking / dedup), the previously-added SearchBar.svelte.spec.ts, the extended MonitoringPanel.svelte.spec.ts (Ctrl+F popup, edit-invalidates-results, Esc close returns focus), logSearch.svelte.spec.ts (countOccurrences, isFindShortcut, advanceSearch direction cases), and lookup.test.ts for the coordinator contract.

Every non-blocking note from my previous review was folded in. LGTM.

One tiny nit for a future pass (not blocking): TriageResultsView.runSearch computes matches.indexOf(idx) + 1 after already scanning to build matches. For triage-sized inputs this is nothing, but if it ever grows tracking the cursor position during the filter pass would drop the extra O(n).

…filer metrics Switch what search bar Ctrl+F focuses in profiler based on context Extend search to include top nodes, global profile stats Signed-off-by: Karakatiza666 <bulakh.96@gmail.com>

mythical-fred

Re-review after force-push. Tip advanced from b9b49a23 (my prior APPROVE) to d4bec133. The delta is a two-file quality polish on top of my last approval — production code is byte-identical to the prior tip except for one tooltip:

SupportBundleViewerLayout.svelte: the graph-panel "Search node" input's title tooltip is expanded to describe all three lookup modes ("by ID, by table or view name, or by a substring of a persistent ID"). Purely user-facing text; matches what profilerDiagram.search() already handled.
MonitoringPanel.svelte.spec.ts: the spec now factors a shared openSearch() helper and gains a targeted regression case — Ctrl+F opens, Esc closes and returns focus to the container, Ctrl+F reopens — which pins down exactly the flow the window-level useShortcut capture-phase handler was introduced to guarantee.

That is exactly the shape of change a re-review should encourage: better test coverage of the invariant that makes this whole design safe (Ctrl/Cmd-F must work regardless of focus), and a more accurate hover title on the node search. CI on the tip: green (1/1 check-run success).

None of the six previously-noted non-blocking nits have been re-introduced or worsened. The prior nits still standing are the small cosmetic ones I already flagged in the previous review body and are explicitly non-blocking — leave them for follow-up if desired:

SupportBundleViewerLayout now uses a single useShortcut(isFindShortcut, …) that dispatches on profilerDiagram?.isFocused() — resolves the earlier double-handler race, thanks.
MetricsView non-reactive let matchCursor / searchQuery — still worth a one-line "on purpose: read-only inside dispatch" comment.
ProfilerDiagram.isFocused() is still derived via pointerOnGraph, so keyboard-only users who tab to the graph won't have Ctrl/Cmd-F routed to node search. Non-blocking; consider extending the predicate to also cover document.activeElement inside the graph pane.
PR/commit title still says "bi-directinal"; please fix at squash-merge.
Two cosmetic destructuring reformats in unrelated files — cosmetic only.

Single commit, DCO Signed-off-by present, no Co-Authored-By: Claude / Authored-By: Claude / "Generated with Claude Code" trailers. Approving.

mihaibudiu · 2026-08-11T17:56:31Z

+  / editing / closing call `onclear` so they clear together.
+
+  Hosts open the search by wiring Ctrl/Cmd-F to the exported `activate()`; the widget handles no
+  shortcut itself. The popup closes only via `toggle()`, Escape, the close button, or blur when


I don't know what "blur" is, should learn about it.

In HTML, the user can "focus" on an interactive element - e.g. when they click on it, or they select it while pressing "Tab". "Blur" is the opposite of that - the event of losing focus on an element; this is the standard terminology.

mihaibudiu · 2026-08-11T17:57:26Z

+  import type { SearchDirection, SearchProgress } from './logSearch'
+
+  interface Props {
+    /** The query text. Bindable so the host reads what was typed. */


It would be nice if Bindable<T> was a type.

For better or worse Svelte framework does not provide such a type. Instead it helps to avoid the confusion through linter warnings and runtime console logs. In my experience I do not get confused by this lack of strong typing for bindable props, so I decided to not add such a proprietary type on top of the framework convention.

Can't you define it as a simple wrapper type?

I can, the question is if I should. As I mentioned, I haven't felt the need for it, and the framework conventions do away without it, so adding it now seems superficial.

mihaibudiu · 2026-08-11T18:12:37Z

+  // Ctrl/Cmd-F here, so the shortcut opens-or-refocuses and never closes the search.
+  export async function activate() {
+    if (!open) {
+      opener = document.activeElement as HTMLElement | null


is this always safe? isn't there a race where it could change?

Yes. activate is called synchronously after the user performs the input that called it, so document.activeElement could not have been reassigned by the time activate was called.

mihaibudiu · 2026-08-11T18:13:00Z

+    if (!open) {
+      opener = document.activeElement as HTMLElement | null
+      open = true
+      await tick()


what is the tick for?

tick() resolves once Svelte has flushed pending state changes to the DOM. The input lives behind {#if open}, so right after open = true it has not been created and inputEl is still undefined; awaiting tick() lets Svelte create it and run the bind:this, after which inputEl.focus() has something to focus.

mihaibudiu · 2026-08-11T18:53:45Z

-    if (!matchId) return
-    const el = containerEl.querySelector<HTMLElement>(`[data-block-id="${matchId}"]`)
+    const n = ids.length
+    matchCursor = ((matchCursor % n) + n) % n


this is wraparound for -1?

Yes, and for any negative value, not just -1. JavaScript's % keeps the sign of the dividend (-1 % 3 is -1, not 2), so the extra + n) % n maps the cursor back into [0, n). Stepping back past the first match lands on the last, stepping forward past the last lands on the first.

The same expression appeared in four places, so I extracted it as wraparound(index, length) in common-ui/logSearch.ts, with the explanation of the function and unit tests.

mihaibudiu · 2026-08-11T18:55:10Z

  const { profileData, dataflowData, programCode, callbacks, class: className }: Props = $props()

  // DOM element references
+  let element: HTMLDivElement | undefined = $state()


I don't know what this is

This is a root HTML element of the diagram, that I now keep track of to monitor whether the diagram is currently in user focus (user clicks around in it)
I track it to decide which search bar to focus on Ctrl+F - to search through metrics, or diagram nodes.

mihaibudiu · 2026-08-11T20:57:14Z

@@ -0,0 +1,205 @@
+<!--


How about you start writing some documentation?
How are users supposed to discover all these keys?
I have opened the issue about UI documentation missing years ago.

I constantly have higher priority issues to work on. So I only implement either visible features through buttons, or some intuitive behaviors like resizing the panels or intuitive shortcuts like Ctrl+F.
Adding some inline documentation through hover tooltips on (?) icons throughout the UI also have higher priority

mihaibudiu · 2026-08-11T20:58:02Z

+  let logMatchCount = $state(0)
+  let searchBar: ReturnType<typeof SearchBar> | undefined = $state()
+  // Single source of truth for the search bar: derived from the committed `logSearch`. No
+  // committed pattern → null (counter hidden, nav disabled, nothing highlighted).


what is a "committed pattern"?

It meant "the query the log list is actually running", as opposed to the text currently in the input box: the two diverge while the user types, because a query takes effect only on Enter (or a nav button), not as-you-type.

"Committed" may not have been the best choice, will rephrase to "submitted"

mihaibudiu · 2026-08-11T20:58:39Z

-  const clearLogSearch = () => {
-    logSearchInput = ''
-    submitLogSearch()
+  // Drop the committed search (highlight + counter). The search bar owns and clears the input


committed again?

Signed-off-by: Karakatiza666 <bulakh.96@gmail.com>

mythical-fred approved these changes Jul 20, 2026

View reviewed changes

Karakatiza666 marked this pull request as draft July 20, 2026 15:22

Karakatiza666 changed the title ~~[web-console] Implement bi-directinal search bar for logs tab and profiler metrics~~ [web-console] Implement bi-directional search bar for logs tab and profiler metrics Jul 20, 2026

Karakatiza666 force-pushed the issue6508 branch 2 times, most recently from f22e8f8 to b9b49a2 Compare July 21, 2026 12:19

Karakatiza666 marked this pull request as ready for review July 21, 2026 12:24

mythical-fred approved these changes Jul 21, 2026

View reviewed changes

Karakatiza666 requested a review from mihaibudiu July 21, 2026 14:21

[web-console] Implement bi-directinal search bar for logs tab and pro…

d4bec13

…filer metrics Switch what search bar Ctrl+F focuses in profiler based on context Extend search to include top nodes, global profile stats Signed-off-by: Karakatiza666 <bulakh.96@gmail.com>

Karakatiza666 force-pushed the issue6508 branch from b9b49a2 to d4bec13 Compare July 30, 2026 08:40

mythical-fred approved these changes Jul 31, 2026

View reviewed changes

mihaibudiu requested changes Aug 11, 2026

View reviewed changes

[web-console] Review changes, add function, adjust comments

9c7a9ae

Signed-off-by: Karakatiza666 <bulakh.96@gmail.com>

Karakatiza666 force-pushed the issue6508 branch from 0313c0c to 9c7a9ae Compare August 12, 2026 10:10

Uh oh!

Conversation

Karakatiza666 commented Jul 20, 2026

Uh oh!

mythical-fred left a comment

Choose a reason for hiding this comment

Uh oh!

mythical-fred left a comment

Choose a reason for hiding this comment

Uh oh!

mythical-fred left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Karakatiza666 Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Karakatiza666 Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Karakatiza666 Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Karakatiza666 Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Karakatiza666 Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Karakatiza666 Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Karakatiza666 Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Karakatiza666 Aug 12, 2026 •

edited

Loading

Karakatiza666 Aug 12, 2026 •

edited

Loading

Karakatiza666 Aug 12, 2026 •

edited

Loading

Karakatiza666 Aug 12, 2026 •

edited

Loading

Karakatiza666 Aug 12, 2026 •

edited

Loading

Karakatiza666 Aug 12, 2026 •

edited

Loading

Karakatiza666 Aug 12, 2026 •

edited

Loading