Repository navigation
fix: Stale waterfall selection when opening another span - #3285
Conversation
🦋 Changeset detectedLatest commit: 011d4c3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Someone is attempting to deploy a commit to the HyperDX Team on Vercel. A member of the Team first needs to authorize it. |
|
Hi @bahltushar, thanks for the pull request! Before we review code from a first-time contributor we ask that a maintainer vouches for you, and you're not on our list yet. This PR stays open — it just isn't in the review queue until someone vouches. To get vouched, open an issue saying hello and what you're working on: https://github.com/hyperdxio/hyperdx/issues/new?template=introduce-yourself.md A maintainer will usually reply within a day or two, and then this PR gets picked up as normal. More detail in our contributing guide. |
github-actions
Bot
added
the
needs-vouch
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // waterfall. A new result of the same trace must not keep the previous span. | ||
| // The exception is the result that *is* that span — clearing it would drop the | ||
| // selection the click just made. | ||
| export function eventRowWhereForOpenedRow<T extends { id: string }>( |
There was a problem hiding this comment.
🟠 major — "Keep" branch compares two different row-id formats, so it never matches and the selection is always cleared
current.id is the waterfall's id, built by useRowWhere from the waterfall's own columns with SpanAttributes/SpanEvents/__hdx_hidden dropped (DBTraceWaterfallChart.tsx:442-466). openedRowWhere is the search table's id, built from the search select or rowKeyColumns (DBRowTable.tsx:1808-1814). These strings are not equal in real use, so the function always returns null. Here is how that hurts: a user picks span B in the waterfall, then clicks the same highlighted result row again (or uses the expanded row's 'Open in side panel'). B gets cleared, and the opened span isn't re-highlighted either, because appliedHighlightHintRef already holds that hint key (DBTraceWaterfallChart.tsx:930-933). Fix: compare against the currently open rowWhere, i.e. keep eventRowWhere when rowWhere === openedRowWhere. Or match on trace and span identity instead of raw where-strings. Then drop the id-equality branch.
Nothing blocks merge automatically, but a maintainer will expect this fixed if it is a real defect in code this PR changes. If it is about surrounding code, reply and say so instead of patching. Do not widen the PR. How to respond
There was a problem hiding this comment.
The keep check no longer compares the waterfall id to the search-row where string. It keeps eventRowWhere only when the result already open is the one being opened again (rowWhere === openedRowWhere).
PR Review
3 finding(s): 🔴 0 critical · 🟠 0 major · 🔵 3 minor 2 posted as inline comment(s) on the changed lines. 1 listed below. Findings outside the changed lines1 minor
Severity is the reviewer's own estimate and is used for ordering, not filtering. No finding blocks merge automatically; a maintainer decides. |
 [Medium impact] The change affects span selection in trace views across search and session replay. The repository’s component-size requirement remains unmet and must be satisfied before merging.
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Deep Review✅ No critical issues found. The core fix is sound and matches the approach the prior reviewer requested: the selection is cleared where it is made ( 🟡 P2 -- recommended
🔵 P3 nitpicks (4)
Reviewers (8): correctness, adversarial, frontend-races, typescript, testing, maintainability, project-standards, previous-comments. Testing gaps:
Note: The dispatched reviewer sub-agents had not returned verifiable findings at synthesis time; the above is a conservative synthesis from direct diff analysis and the prior-comment thread, re-graded with the ship-blocker rubric (default-down). The previously raised "selection survives a source switch" item was withdrawn by its author as pre-existing and out of scope, and is not counted here. No finding blocks merge automatically. A maintainer will expect P0/P1 findings in code this PR changes to be fixed; P2/P3 are your call -- fix or reply. Never fix findings about surrounding code here; reply instead. Do not widen the PR. How to respond |
|
The equal-predicate source switch already kept the span before this pull request, because eventRowWhere is cleared only when the trace ID changes. Comparing the source here would widen the change. |
pulpdrew
left a comment
pulpdrew
left a comment
There was a problem hiding this comment.
Thanks for tracking this down, and for the clear repro. The diagnosis is right: the panel remounts, sees the old eventRowWhere in the URL, treats it like a deep link, and won't auto-select the newly opened row. I'd like to fix it in a different place, though.
Problems with clearing in the openers
- It only covers two of the openers. This fix lives in DBSqlRowTableWithSidebar.onOpenSidebar and ExpandableRowTable.useSidebarOpener. DBRowSidePanel is also opened from PatternSidePanel, PodDetailsSidePanel and SessionSidePanel, and those still have the bug. Any new opener would also need to know to call the helper.
- The logic is duplicated. The same rule is written out in both openers and has to be kept in sync by hand.
- Keeping the span on close doesn't do anything. After a close, rowWhere is null, so the next open always clears the span. All that's left is a stale eventRowWhere in the URL, which gets copied into shared row links. The PR description also says closing clears it, but the code does the opposite.
Suggested approach: fix it where the selection is made
- Clear the selection when the drawer closes. In DBRowSidePanelErrorBoundary._onClose (DBRowSidePanel.tsx), call setEventRowWhere(null) next to clearTraceWaterfallSearchState(). Every host closes through this path, so you don't need to touch any opener.
- Let a changed hint replace the selection. This covers clicking another result while the drawer stays open. In the auto-select effect in DBTraceWaterfallChartContainer, only the first hint after mount or a trace change should yield to an existing selection. Today the code yields whenever highlightedRowWhere != null. Change that check to:
// Only the first hint after mount or a trace change yields to an existing
// selection (restored from the URL, or picked by the user). A later hint
// change means a different row was opened, so it replaces the selection.
if (appliedHighlightHintRef.current == null && highlightedRowWhere != null) {
appliedHighlightHintRef.current = hintKey;
return;
}
Keep the rest of the effect as it is: the reset on traceId change, the early return when the key already matches, and the row lookup. Reloads and shared links still keep their URL selection. A span the user cleared or picked isn't overridden, because the hint key hasn't changed. Opening the row that's already open also keeps the selection, for the same reason.
I'd suggest pulling this effect out into a small hook (e.g. hooks/useRowHighlightHint.ts) so it can be unit tested with renderHook, without mocking nuqs or the table.
Testing
The current test mocks quite a bit. With this being a scenario that crosses a few components and relies on URL state, Playwright E2E tests would be more appropriate. We should cover both scenarios:
- Opening a span, closing it, opening another span from the same trace
- Opening a span, then selecting another span from the same trace without closing the side panel first.
7e9fab0 to
becea77
Compare
| return; | ||
| } | ||
|
|
||
| if ( |
There was a problem hiding this comment.
🟠 major — The stale span still shows when the Trace tab remounts after another row of the same trace is opened
DBRowSidePanel.tsx:1126 renders DBTracePanel only while the Trace tab is active, so this ref goes back to null whenever the user switches tabs. Here is the failing sequence: open row A, view the Trace tab (A is selected), switch to Overview, click row B of the same trace in the results table while the drawer stays open, then return to Trace. The hook mounts fresh, sees highlightedRowWhere = A, takes the yield branch and keeps A, which is the bug this PR set out to fix. Make the open action decide instead of mount order: clear or replace eventRowWhere when the side panel's row changes (for example in DBSqlRowTableWithSidebar's onOpenSidebar, or in an effect on rowId in DBRowSidePanel). Alternatively, yield only when the existing selection is the hinted row. Add a test that unmounts and remounts the hook with a new hint while a selection exists.
Nothing blocks merge automatically, but a maintainer will expect this fixed if it is a real defect in code this PR changes. If it is about surrounding code, reply and say so instead of patching. Do not widen the PR. How to respond
|
@pulpdrew made the changes as suggested. Following were made:
|
|
|
||
| export type RowHighlightHint = { | ||
| timestamp: string; | ||
| spanId: string; |
There was a problem hiding this comment.
🔵 minor — New RowHighlightHint type is not used where the same shape is already written out inline
DBTraceWaterfallChart.tsx:645 and DBTracePanel.tsx:234 still spell out { timestamp; spanId; body } inline. Replace both with RowHighlightHint so there is one definition (convention: define and import reusable named types, DRY).
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
| clearTraceWaterfallSearchState(); | ||
| // The selected span is in the URL. Leaving it there makes the next open, | ||
| // and any shared row link, show the previous span. | ||
| setEventRowWhere(null); |
There was a problem hiding this comment.
🔵 minor — Span-selection clearing is spread across three call sites and runs twice on normal close
With the boundary, the X button runs Inner's closePanel (clear) and then _onClose (clear again). Outside click, Drawer onClose and the error fallback only reach _onClose, and the session panel only reaches Inner's clear. Every new way out of the panel has to remember to clear, which is how the session breadcrumb path above got missed. Move the clear into one place that owns leaving the panel, e.g. have sidePanelStack.clearTrail() or a shared leavePanel helper clear eventRowWhere, and drop the per-call-site setEventRowWhere(null)/clearSpanSelection() calls.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
| [pushSource], | ||
| ); | ||
|
|
||
| const clearSpanSelection = useOpenedRowSpanSelection(initialRowId); |
There was a problem hiding this comment.
🟠 major — Clicking the session breadcrumb leaves the old span selected, so the session replay bug still happens
The span is only cleared in handlePanelBack (Esc and the Back button) and in closePanel. In packages/app/src/SessionSidePanel.tsx:236-241, the session breadcrumb passes onClick: handleBackToSession straight through to parentBreadcrumbs, and that path never calls clearSpanSelection. Here is how it breaks. Open event A from a replay; the waterfall selects span A. Click the session breadcrumb: selectedEvent becomes null and DBRowSidePanelInner unmounts. Open event B in the same trace. DBRowSidePanelInner mounts again, and openedRowIdRef starts out as B, so useOpenedRowSpanSelection never clears. useRowHighlightHint then treats span A as a restored selection on its first hint and keeps it. Fix: clear eventRowWhere in SessionSidePanel's handleBackToSession (and handleEventNavigate). Another option is to wrap the onClick of each parentBreadcrumbs item in DBRowSidePanelInner with clearSpanSelection. Then add a test for the breadcrumb path.
Nothing blocks merge automatically, but a maintainer will expect this fixed if it is a real defect in code this PR changes. If it is about surrounding code, reply and say so instead of patching. Do not widen the PR. How to respond
11806cc to
11253ef
Compare
| @@ -755,7 +762,23 @@ export const DBRowSidePanelInner = ({ | |||
| const items: BreadcrumbItem[] = []; | |||
|
|
|||
| if (parentBreadcrumbs) { | |||
There was a problem hiding this comment.
🔵 minor — Session span clearing is spread across Inner instead of living in SessionSidePanel.handleBackToSession
The new handlePanelBack branch (clearSpanSelection(); onNavigateToParent()) and the parentBreadcrumbs.map wrapper both exist only to clear the span before SessionSidePanel.handleBackToSession runs. That is the only onNavigateToParent/parent-crumb provider (packages/app/src/SessionSidePanel.tsx:135,238,245), and it already has useClearSpanSelection in scope. Call clearSpanSelection() inside handleBackToSession and drop the crumb-wrapping map and the extra branch in Inner. Back and the crumb then go through one place, and Inner no longer rewrites callbacks it was handed.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
| }, [isFullWidth, setSize]); | ||
|
|
||
| const { clear: clearTraceWaterfallSearchState } = useWaterfallSearchState({}); | ||
| const [, setEventRowWhere] = useQueryState( |
There was a problem hiding this comment.
🔵 minor — _onClose sets up its own eventRowWhere setter instead of using the new useClearSpanSelection
This PR adds useClearSpanSelection (packages/app/src/hooks/useOpenedRowSpanSelection.ts:14), which wraps exactly useQueryState('eventRowWhere', eventRowWhereParser) + set(null). SessionSidePanel uses it, but DBRowSidePanelErrorBoundary sets up its own useQueryState and calls setEventRowWhere(null) directly. Use const clearSpanSelection = useClearSpanSelection() here and drop the nuqs/eventRowWhereParser imports from this file's boundary component. That leaves one definition of how a span selection is cleared.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| [pushSource], | ||
| ); | ||
|
|
||
| const clearSpanSelection = useOpenedRowSpanSelection(initialRowId); |
There was a problem hiding this comment.
DBRowSidePanel.tsx, which is already over 1,400 lines. The repository requires component files to stay under 300 lines and large components to be broken down; the changed DBTracePanel.tsx and DBTraceWaterfallChart.tsx also exceed that limit. This requirement must be satisfied before merging.
Context Used: CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| [pushSource], | ||
| ); | ||
|
|
||
| const clearSpanSelection = useOpenedRowSpanSelection(initialRowId); |
There was a problem hiding this comment.
🟠 major — Span selection is cleared only when the root row changes, so drilldowns inside the panel still show the previous span
useOpenedRowSpanSelection(initialRowId) watches only the root row. A row pushed with pushNav, pushSource ("View Trace"), or reached by Back through popOne changes resolvedRowId but leaves initialRowId the same. For those rows isRowLoading renders "Loading...", which unmounts DBTracePanel. The waterfall then remounts, and useRowHighlightHint treats the old eventRowWhere (same trace) as a restored selection and keeps it. This is the same stale-span bug the PR fixes, reached through in-panel navigation. Example: open log A, click View Trace (span X is selected), go Back, open neighbour log B from the same trace in Context, click View Trace: X is still highlighted. Fix: key the hook on resolvedRowId, plus the active source id, instead of initialRowId.
Nothing blocks merge automatically, but a maintainer will expect this fixed if it is a real defect in code this PR changes. If it is about surrounding code, reply and say so instead of patching. Do not widen the PR. How to respond
| @@ -755,7 +762,23 @@ export const DBRowSidePanelInner = ({ | |||
| const items: BreadcrumbItem[] = []; | |||
|
|
|||
| if (parentBreadcrumbs) { | |||
There was a problem hiding this comment.
🔵 minor — Session-only cleanup is spread into the generic inner panel instead of handleBackToSession
SessionSidePanel is the only caller that passes onNavigateToParent and parentBreadcrumbs, and both point at handleBackToSession (SessionSidePanel.tsx:135). That component already calls clearSpanSelection in handleEventNavigate and handleClose. Call it in handleBackToSession too, then remove the breadcrumb-wrapping map here and the extra clearSpanSelection() in handlePanelBack. That keeps all of the session's clear-on-leave logic in one place.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
| }, [isFullWidth, setSize]); | ||
|
|
||
| const { clear: clearTraceWaterfallSearchState } = useWaterfallSearchState({}); | ||
| const [, setEventRowWhere] = useQueryState( |
There was a problem hiding this comment.
🔵 minor — Close handler re-implements useClearSpanSelection with its own useQueryState
DBRowSidePanelErrorBoundary opens a second useQueryState('eventRowWhere', eventRowWhereParser) just to call setEventRowWhere(null). The new useClearSpanSelection in hooks/useOpenedRowSpanSelection.ts does exactly this, and SessionSidePanel already uses it. Use const clearSpanSelection = useClearSpanSelection() here, and drop the nuqs and eventRowWhereParser imports from this file.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
|
It seems like the new E2E is either flaky or failing, could you try making it more reliable? |
| [pushSource], | ||
| ); | ||
|
|
||
| const clearSpanSelection = useOpenedRowSpanSelection(initialRowId); |
There was a problem hiding this comment.
🟠 major — Navigating to another row inside the panel doesn't clear the span, so the stale-span bug comes back
useOpenedRowSpanSelection(initialRowId) only watches the root row, but the waterfall's hint follows the active row (resolvedRowId, from the nav/source stacks via normalizedRow, ~line 736). To reproduce: open trace span A, so the Trace tab selects A. Switch to Surrounding Context and click a neighbour span B from the same trace (this goes through pushNav, and initialRowId doesn't change). Return to the Trace tab. The waterfall remounts, the first hint gives way to the existing selection A (its traceId still matches), and the header shows B while the highlight and details show A. Back (popOne) has the same problem in reverse. Fix: pass resolvedRowId (or activeRowId) to the hook, and add a nav-stack case to DBRowSidePanel.spanSelection.test.tsx.
Nothing blocks merge automatically, but a maintainer will expect this fixed if it is a real defect in code this PR changes. If it is about surrounding code, reply and say so instead of patching. Do not widen the PR. How to respond
| @@ -755,7 +762,23 @@ export const DBRowSidePanelInner = ({ | |||
| const items: BreadcrumbItem[] = []; | |||
There was a problem hiding this comment.
🔵 minor — The inner panel wraps the parent's callbacks to clear the span on the session's behalf
Both handlePanelBack (via onNavigateToParent) and the parentBreadcrumbs wrapper end up calling SessionSidePanel's handleBackToSession, and that file already has clearSpanSelection. Call clearSpanSelection() inside handleBackToSession in SessionSidePanel.tsx. Then drop the breadcrumb .map wrapper and the extra call in handlePanelBack, so one layer owns this instead of two.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
| }, [isFullWidth, setSize]); | ||
|
|
||
| const { clear: clearTraceWaterfallSearchState } = useWaterfallSearchState({}); | ||
| const [, setEventRowWhere] = useQueryState( |
There was a problem hiding this comment.
🔵 minor — The drawer close handler repeats useClearSpanSelection inline
DBRowSidePanelErrorBoundary sets up its own useQueryState('eventRowWhere', eventRowWhereParser) just to call setEventRowWhere(null). That is exactly what the new useClearSpanSelection in hooks/useOpenedRowSpanSelection.ts does. Use const clearSpanSelection = useClearSpanSelection() and remove the direct nuqs and eventRowWhereParser imports here.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
|
It appears that the E2E are still failing, and there is also a lint failure (make ci-lint should run the lints locally) |
| [pushSource], | ||
| ); | ||
|
|
||
| const clearSpanSelection = useOpenedRowSpanSelection(initialRowId); |
There was a problem hiding this comment.
🟠 major — Span selection is cleared only when the root row changes, not when in-panel navigation changes the row on screen
The hook is keyed on initialRowId, but the panel renders resolvedRowId (leafNav?.rowId ?? activeSourceFrame?.rowId ?? initialRowId, line 304-308). Reproduce: select span X in a row's trace tab, then use pushNav (a surrounding-context neighbour), pushSource (View trace) or Back/breadcrumb (popOne/truncateTo) to reach another row in the same trace. Each of these loads a new row, so the trace panel unmounts and remounts (isRowLoading at line 887). On remount useRowHighlightHint yields to the old eventRowWhere, so span X stays highlighted, which is the bug this PR fixes. Fix: pass resolvedRowId (or a source+row key) to useOpenedRowSpanSelection.
Nothing blocks merge automatically, but a maintainer will expect this fixed if it is a real defect in code this PR changes. If it is about surrounding code, reply and say so instead of patching. Do not widen the PR. How to respond
| @@ -760,7 +767,23 @@ export const DBRowSidePanelInner = ({ | |||
| const items: BreadcrumbItem[] = []; | |||
There was a problem hiding this comment.
🔵 minor — Clearing on return to the session is done in the child by wrapping the parent's callbacks
The only parent that passes onNavigateToParent and parentBreadcrumbs is SessionSidePanel, and it passes handleBackToSession for both. Call clearSpanSelection() once inside handleBackToSession (SessionSidePanel.tsx:135), the way handleClose and handleEventNavigate already do. Then remove the breadcrumb .map wrapper and the extra branch in handlePanelBack, so this flow has one owner instead of two.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
| }, [isFullWidth, setSize]); | ||
|
|
||
| const { clear: clearTraceWaterfallSearchState } = useWaterfallSearchState({}); | ||
| const [, setEventRowWhere] = useQueryState( |
There was a problem hiding this comment.
🔵 minor — Second hand-rolled eventRowWhere setter instead of the new useClearSpanSelection hook
DBRowSidePanelErrorBoundary creates its own useQueryState('eventRowWhere', eventRowWhereParser) only to call setEventRowWhere(null). That is exactly what useClearSpanSelection (hooks/useOpenedRowSpanSelection.ts) does, and SessionSidePanel already uses it. Use const clearSpanSelection = useClearSpanSelection() here and drop the nuqs and parser imports.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
|
@pulpdrew made the changes, lint issue is gone and hopefully now there won't be any e2e test failing. Although I checked in Preview option and can see the issue which was highlighted is fixed. |
| // A parent crumb (the session name) leaves this row without going | ||
| // through Back or Close. Clear the span on that click too. | ||
| items.push( | ||
| ...parentBreadcrumbs.map(item => { |
There was a problem hiding this comment.
🔵 minor — Clearing the span when leaving a session event lives in the wrong component
onNavigateToParent and the parent breadcrumb onClick both point at SessionSidePanel.handleBackToSession (packages/app/src/SessionSidePanel.tsx:135), and that handler is the only one either path calls. Put clearSpanSelection() in handleBackToSession, next to the clears already in handleEventNavigate and handleClose. Then drop the parentBreadcrumbs.map wrapper and the onNavigateToParent-only clear in handlePanelBack. As written, the session's "leave this event" rule is split between the parent and a child that rewrites the parent's callbacks, and a new parent crumb or back path added later will miss it.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
| }, [isFullWidth, setSize]); | ||
|
|
||
| const { clear: clearTraceWaterfallSearchState } = useWaterfallSearchState({}); | ||
| const [, setEventRowWhere] = useQueryState( |
There was a problem hiding this comment.
🔵 minor — The close handler sets up its own eventRowWhere setter instead of using the new useClearSpanSelection hook
This PR adds useClearSpanSelection in packages/app/src/hooks/useOpenedRowSpanSelection.ts for exactly this operation, and SessionSidePanel already uses it. Call const clearSpanSelection = useClearSpanSelection() here and use it in _onClose. That removes the extra useQueryState('eventRowWhere', eventRowWhereParser) and the nuqs/eventRowWhereParser imports from this file, so clearing the span happens in one place.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
|
|
||
| if (parentBreadcrumbs) { | ||
| items.push(...parentBreadcrumbs); | ||
| // A parent crumb (the session name) leaves this row without going |
There was a problem hiding this comment.
🔵 minor — Span clearing for 'leave to session' is added in DBRowSidePanelInner by wrapping callbacks the parent passes in, when the parent already has a single handler for it
Only SessionSidePanel passes onNavigateToParent and parentBreadcrumbs, and both point at handleBackToSession (SessionSidePanel.tsx:135, 238, 245). Call clearSpanSelection() inside handleBackToSession, then drop the parentBreadcrumbs.map wrapper and the extra branch in handlePanelBack. That leaves one clear per exit path, owned by the parent that controls the exit, rather than Inner rewriting another component's callbacks.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
| }, [isFullWidth, setSize]); | ||
|
|
||
| const { clear: clearTraceWaterfallSearchState } = useWaterfallSearchState({}); | ||
| const [, setEventRowWhere] = useQueryState( |
There was a problem hiding this comment.
🔵 minor — Drawer close sets up its own eventRowWhere query state instead of reusing the new useClearSpanSelection hook
Replace the local useQueryState('eventRowWhere', eventRowWhereParser) and setEventRowWhere(null) with const clearSpanSelection = useClearSpanSelection() from @/hooks/useOpenedRowSpanSelection, which this PR adds and already uses in SessionSidePanel and DBRowSidePanelInner. Then every place that clears the span goes through one helper.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
Summary
Opening another span of the same trace left the previous span selected in the waterfall. Closing a result now clears eventRowWhere, and opening a result keeps it only when that result is the span already selected.
The eventRowWhere query parser is shared by the results table, the expandable row opener, and the trace panel, so every path that opens or closes a result updates the selection the same way.
Screenshots or video
2026-10-05_10-50-10.mp4
Environment
HyperDX 2.x (reproduced on the build currently packaged as 2.38.0)
Trace search, Lucene
ClickHouse OpenTelemetry traces source
Steps to reproduce
TraceId:"854cf2e4-1302-4ce2-ba2a-94475bf89d68" AND SpanName:"FareMasterPricerTravelBoardSearch"
Expected
The waterfall highlights b2e2b0df5304a26a, and the Overview shows that span (parent b0be84a5cec5874d, duration 1.735s). eventRowWhere is replaced by the newly opened row, or cleared so the new row becomes the selection.
Actual
The header updates to b2e2b0df5304a26a, but the waterfall highlight and Overview still describe 8cea712d1d4df628.
Test plan
[x] Open a span from a trace, close the result, then open a different span of the same trace. The waterfall should not stay on the first span.
[x] Open the same span that is already selected. The waterfall should stay on that span.
[x] Open a span from a different trace. The previous span selection should not appear in the new waterfall.
[x] yarn ci:unit packages/app/src/components/tests/DBSqlRowTableWithSidebar.test.tsx