Skip to content

feat: single drawer navigation#2541

Open
karl-power wants to merge 5 commits into
mainfrom
hdx-3942-single-drawer-navigation
Open

feat: single drawer navigation#2541
karl-power wants to merge 5 commits into
mainfrom
hdx-3942-single-drawer-navigation

Conversation

@karl-power

@karl-power karl-power commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Replaces the old stacked/layered side-panel drawers with a single right-hand drawer where all event navigation happens in-place. Everything now renders inside one shell instead of opening a second drawer on top of another.

  • Single-drawer navigation. DBRowSidePanel is refactored into DBRowSidePanelInner, which owns two URL-persisted stacks: sidePanelNavStack for same-source drilldowns (e.g. Surrounding Context, replaced in-place) and sidePanelSourceStack for cross-source navigation (log → trace via the new View Trace action, or session → event). Each frame stores its source id, row id, label, and originating tab. State lives in the URL (sidePanelTab, sidePanelSourceStack, sidePanelNavStack, sessionPanelEvent), so the trail survives reload and shared links.
  • Unified breadcrumbs (SidePanelBreadcrumbs). New shared component used across log, trace, and session panels: source-kind icons (IconLogs/IconConnection/IconDeviceLaptop), label truncation with tooltips, and a clickable trail to jump back to any ancestor.
  • Session panel alignment. SessionSidePanel embeds DBRowSidePanelInner for the selected event instead of portaling a second DBRowSidePanel, so session → event → trace shares the same structure and breadcrumbs. SessionSubpanel/DBSessionPanel report selections via onEventNavigate.
  • Header redesign. Metadata row (timestamp, service, duration, status, span kind), Copy Trace ID, and View Trace for logs with trace context. View Trace / Trace ID are hidden when there's no trace context. Shared SidePanelHeaderActions for shortcuts/full-width/share/close; DBRowSidePanelHeader slimmed to the body section.
  • Navigation fixes. Session Close (X) closes the whole panel; back/Esc pops one level. Returning from a trace restores the prior tab; opening a different event resets the tab and clears stale stacks; pushes jump to the destination's default tab.
  • Background scroll. Removes the side panel overlay on search and sessions pages so that the background lists are scrollable and the user can click to select an alternative log/trace/session.

Screenshots or video

output.mp4

How to test on Vercel preview

Preview routes: /search, /sessions

Navigate around the side panel drawer on both the search and sessions pages.

References

@changeset-bot

changeset-bot Bot commented Jun 29, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: ce7f213

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@hyperdx/app Minor
@hyperdx/api Minor
@hyperdx/otel-collector Minor

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

@vercel

vercel Bot commented Jun 29, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
hyperdx-oss Ready Ready Preview, Comment Jul 6, 2026 2:35pm
hyperdx-storybook Ready Ready Preview, Comment Jul 6, 2026 2:35pm

Request Review

@github-actions github-actions Bot added the review/tier-4 Critical — deep review + domain expert sign-off label Jun 29, 2026
@github-actions

github-actions Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

🔴 Tier 4 — Critical

Touches auth, data models, config, tasks, OTel pipeline, ClickHouse, or CI/CD.

Why this tier:

  • Large diff: 2104 production lines changed (threshold: 1000)
  • Cross-layer change: touches frontend (packages/app) + shared utils (packages/common-utils)

Review process: Deep review from a domain expert. Synchronous walkthrough may be required.
SLA: Schedule synchronous review within 2 business days.

Stats
  • Production files changed: 25
  • Production lines changed: 2104 (+ 1080 in test files, excluded from tier calculation)
  • Branch: hdx-3942-single-drawer-navigation
  • Author: karl-power

To override this classification, remove the review/tier-4 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

@github-actions

github-actions Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

E2E Test Results

All tests passed • 224 passed • 3 skipped • 1543s

Status Count
✅ Passed 224
❌ Failed 0
⚠️ Flaky 3
⏭️ Skipped 3

Tests ran across 4 shards in parallel.

View full report →

@greptile-apps

greptile-apps Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR replaces layered event drawers with one right-hand navigation drawer. The main changes are:

  • URL-backed source and row navigation stacks.
  • Shared breadcrumbs across log, trace, and session panels.
  • In-place session event navigation inside the same drawer.
  • Updated row header metadata and View Trace actions.
  • Scrollable background lists while the drawer is open.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
packages/app/src/components/DBRowSidePanel.tsx Refactors the row side panel into the single-drawer shell with breadcrumbs, stack navigation, updated trace actions, and close cleanup.
packages/app/src/hooks/useSidePanelStack.ts Adds URL-backed side-panel stack state with root ownership checks for stale or ownerless trails.
packages/app/src/SessionSidePanel.tsx Moves session event navigation into the session drawer and gates persisted event state by session id.
packages/app/src/components/DBTracePanel.tsx Adds trace ownership to persisted span-detail state so old span selections do not render for another trace.
packages/app/src/components/tests/DBRowSidePanel.staleStack.test.tsx Adds coverage for owned, stale, changing, and ownerless side-panel stack state.
packages/app/src/tests/SessionSidePanel.test.tsx Adds coverage for persisted session event ownership.

Reviews (11): Last reviewed commit: "extract side panel URL logic to useSideP..." | Re-trigger Greptile

Comment thread packages/app/src/components/DBRowSidePanel.tsx Outdated
Comment thread packages/app/src/components/DBRowSidePanel.tsx
Comment thread packages/app/src/components/DBRowSidePanel.tsx Outdated
Comment thread packages/app/src/components/DBRowSidePanel.tsx Outdated
@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Deep Review

No P0/P1 issues introduced by this diff. The multi-agent pass surfaced one high-severity security claim (SQL injection via shared link); on verification the injection primitive is pre-existingrowWhere and eventRowWhere were already URL params feeding the same raw-SQL sink at the base SHA, and this PR only adds shape validation. It is reclassified below as a pre-existing risk whose attack surface this PR widens. The confirmed new-code issues are P2/P3.

🟡 P2 -- recommended

  • packages/app/src/hooks/useSidePanelStack.ts:144 -- pushNav writes navStack/stackRoot/tab but never overwrites sourceStack, so drilling into a same-source view while a stale trail sits in the URL re-stamps ownership and resurrects the previous row's cross-source frames.
    • Fix: In pushNav, also persist the effective source stack (setSourceStack(sourceStack)) before setting stackRoot, mirroring pushSource, and add a stale-trail test.
    • correctness
  • packages/app/src/components/DBRowSidePanel.tsx:353 -- useHotkeys(['esc'], handlePanelBack) dropped the base gate ({ enabled: subDrawerOpen === false }), so pressing Esc while the keyboard-shortcuts or any child modal is open both dismisses the modal and pops/closes the panel.
    • Fix: Restore an enabled guard tied to !shortcutsOpen && !isChildModalOpen on the Esc hotkey.
    • julik-frontend-races, correctness
  • packages/app/src/components/DBTracePanel.tsx:233 -- The new eventRowWhere.traceId === traceId read gate plus the eviction effect at line 275 treat pre-PR span links (which lack the new traceId field) as foreign, dropping the span selection and scrubbing the URL param.
    • Fix: Treat a missing traceId as a legacy match (eventRowWhere.traceId == null || eventRowWhere.traceId === traceId) and skip the eviction when traceId is absent.
    • api-contract
  • packages/app/src/components/DBRowSidePanel.tsx:386 -- initialMainContent is captured once and never reset on initialRowId change, so selecting a different background row (now possible with the overlay removed) without a remount leaves the root breadcrumb labeled with the prior row's event body.
    • Fix: Reset initialMainContent when initialRowId changes, or key the inner panel state on initialRowId.
    • correctness, ce-adversarial-reviewer, julik-frontend-races
  • packages/app/src/components/DBRowDataPanel.tsx:184 -- (pre-existing; surface widened here) URL-controlled aliasWith (unconstrained sql/name strings) and rowId flow verbatim into with: and where: sent to ClickHouse; this PR routes the same values through additional new params (sidePanelSourceStack, sidePanelNavStack, sessionPanelEvent) whose zod schemas validate shape only, not SQL content.
    • Fix: Persist an opaque row/frame identifier in the URL and re-derive aliasWith/where from trusted source config on load rather than round-tripping raw SQL; track as a security follow-up since the sink predates this PR.
    • ce-security-reviewer
  • packages/app/src/hooks/useSidePanelStack.ts:194 -- The four sidePanel* param names are cleared by hand in three places (clearTrail, SessionSidePanel.clearInnerNavigation, DBRowSidePanel close), while the hook's clearTrail/isStale exports go unused outside tests, so a future param add/rename can silently miss a site and reintroduce trail leakage.
    • Fix: Export the param keys and a single clear helper (or consume clearTrail) and have every close path call it.
    • ce-maintainability-reviewer, ce-kieran-typescript-reviewer
  • packages/app/src/hooks/useSidePanelStack.ts:180 -- New navigation branches lack tests: the truncateTo nav-level tab-restore path (breadcrumb jump within a source), the View Trace traceSpanRowId SQL-clause builder + push, and the session-replay→trace push are all uncovered, and the e2e change removed the log→trace tab flow.
    • Fix: Add unit tests for truncateTo(sourceStack.length, k) tab restore, the traceSpanRowId clause (traceId-only / spanId-only / both / neither), and the breadcrumb→truncateTo level mapping.
    • ce-testing-reviewer
🔵 P3 nitpicks (5)
  • packages/app/src/components/DBRowSidePanel.tsx:208 -- persistStacksInUrl?: boolean is declared on DBRowSidePanelInnerProps but never destructured, used, or passed, implying a configurability the hook doesn't offer.
    • Fix: Remove the unused prop.
  • packages/app/src/hooks/useSidePanelStack.ts:28 -- sidePanelTab is not passed through the owner gate, so a still-valid-but-stale tab persists across a fresh row selection instead of resetting to the destination's default (only invalid tabs are neutralized by reconcileTab).
    • Fix: Reset or owner-gate the persisted tab on row change so a new row lands on its default tab.
  • packages/app/src/components/DBRowSidePanel.types.ts:7 -- Tab.Debug is defined but referenced nowhere (carried over from the old inline enum).
    • Fix: Remove the dead enum member.
  • packages/app/src/components/DBRowSidePanel.tsx:871 -- onClick={(v: any) => setTab(v)} discards TabBar's inferred Tab typing, feeding unchecked any into setTab.
    • Fix: Annotate the callback param as Tab.
  • packages/app/src/components/DBRowSidePanel.tsx:1 -- The file reaches ~1147 lines, well over the repo's documented 300-line component ceiling (already over at base; this PR adds ~500 net).
    • Fix: Extract further sub-panels/handlers (trace/session/context routing, header metadata) into siblings.

Reviewers (11): correctness, ce-adversarial-reviewer, ce-kieran-typescript-reviewer, julik-frontend-races, testing, maintainability, api-contract, security, project-standards, agent-native, learnings-researcher.

Testing gaps: pushNav on a stale (owner-gated) trail is untested; no test switches the selected row/session while the drawer stays open (overlay-removed interaction) to catch stale breadcrumb labels or stack/tab reset; parseAsJsonEncoded round-trip with a populated aliasWith (WithClause with ChSql) and adversarial-yet-schema-valid payloads are uncovered.

Comment thread packages/app/src/components/DBRowSidePanel.tsx Outdated
Comment thread packages/app/src/components/DBRowSidePanel.tsx Outdated
Comment thread packages/app/src/SessionSidePanel.tsx Outdated
Comment thread packages/app/src/components/DBRowSidePanel.tsx Outdated
Comment thread packages/app/src/components/DBRowSidePanel.tsx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review/tier-4 Critical — deep review + domain expert sign-off

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant