[OPIK-8021] [FE] perf: virtualize the traces table's columns and rows - #7947
Conversation
⏱️ pre-commit per-hook timing
⏭️ 41 skipped (no matching files changed)
|
A project with a large feedback-score taxonomy gives the traces table one
column per score name; with enough columns and rows this blocks the main
thread for seconds on every interaction. DataTable now renders only the
horizontally visible center columns plus both pinned blocks, with leading and
trailing spacer cells carrying the summed width of what was skipped, so total
width, minWidth and the scrollbar stay unchanged. Row virtualization is
similarly capped so it only activates once a table is actually large.
Both axes are opt-in per table via columnVirtualization / rowVirtualization
props on DataTable, following the same pattern as TableBody={DataTableVirtualBody}.
TracesSpansTab enables both together once the table exceeds 50 columns and 25
rows.
Also works around a backend issue (OPIK_8056) where the traces list endpoint
returns duplicate rows for traces that belong to more than one experiment,
which otherwise produced React key warnings; rows are de-duplicated by id
until the endpoint is fixed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both axes shared one enabled flag gated by columns > 50 AND rows >= 25. A table with 1000 columns and 10 rows never crossed the row floor, so the whole combined condition stayed off and rendered every column uncapped — the axis that actually needed windowing was blocked by the other axis's count. Each axis now decides on its own count, or on total cell count (columns × rows) crossing the same budget the two thresholds implied together, so a lopsided table windows the axis that needs it regardless of the other. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
60c03c1 to
e37a75f
Compare
Addresses PR review comment on columnWindow.ts.
|
This change looks worth a test. The Logs traces table now renders a window of rows and columns once it passes 25 rows / 50 columns / 3000 cells, with spacer cells standing in for what is skipped. Every existing trace-explore spec seeds at most three traces (smoke seeds 3, the filterable-traces fixture 3, trace-delete 1-2), so none of them ever crosses a threshold and none exercises the windowed path at all; Would target What it would check
Deploying a test environment for this PR and exploring it — results will follow in a comment. areas: Advisory, from the QA test radar. Nothing here blocks this PR, and anything it proposes is a draft for review. Re-checked after a push on 24 Aug 08:30 UTC. |
|
🔄 Test environment deployment process has started Phase 1: Deploying base version You can monitor the progress here. |
…tion Raise the cell-count budget from 1250 to 3000 so the default view (~15 columns x 100 rows = 1500) stays fully rendered, and collapse the two independently-computed axis flags into one shared enabled decision so the table is windowed or not as a whole, rather than deciding each axis separately.
|
❌ Test environment deployment failed The deployment encountered an error. Please check the deployment logs for details. |
Addresses PR review: DataTableSkeletonBody recomputed getVisibleLeafColumns()/sliceColumnWindow() every render; memoize it. Also adds empty-input and out-of-range-index test cases for sliceColumnWindow.
Details
A project with a large feedback-score taxonomy gives the traces table one column per score name, and the resulting cell count blocks the main thread for seconds on every interaction.
DataTablenow renders only the horizontally visible center columns plus both pinned blocks, with leading/trailing spacer cells carrying the summed width of everything skipped, so total width,minWidthand the scrollbar are unchanged. Row virtualization gets the same treatment for the vertical axis.columnVirtualizationprop onDataTable:{ enabled?, overscan?, getScrollElement? }.getScrollElementis for tables that scroll in their own container instead of the page body. All windowing logic lives inshared/DataTable/columnVirtualization/(TanStack VirtualuseVirtualizer,horizontal: true).DataTablegains three calls to one generic index-based slicer — for thecolgroup, the header row and each body row — which is what keeps column-to-cell alignment correct. Nothing else inDataTablebranches on virtualization.rowVirtualizationprop ({ enabled? }) onDataTableVirtualBody, so a table can toggle it on/off without swapping theTableBodycomponent — the virtualizer is constructed once withenabledpassed straight touseVirtualizer, and falls back torows.map(renderRow)when off.TracesSpansTabdecides both axes independently rather than gating one on the other: each axis windows once its own count crosses a threshold (50 columns / 25 rows), or once total cell count (columns × rows) crosses the combined budget those thresholds imply (1250). A floor of 15 keeps a trivially small axis from being windowed for no benefit. This fixes a real gap: a table with e.g. 1000 columns and only 10 rows previously rendered every column uncapped, because the old logic required both axes to cross their threshold together.DataTableSkeletonBodyis windowed too, and switched fromgetAllLeafColumns()togetVisibleLeafColumns(); it previously rendered one cell per defined column per skeleton row, hidden columns included, which mismatched thecolgroup.observeOwnAxisOffsetinvirtualizerOptions.ts) wraps TanStack's own scroll observer and drops repeated offsets, so a sideways scroll doesn't wake the row virtualizer and a vertical scroll doesn't wake the column virtualizer — this also removes wasted work in the five other tables already using row virtualization.renderCustomRow, or no resolvable scroll container — each renders every column as before, since those paths emit cell counts a windowedcolgroupcannot match.enabled: false, so it resolves no scroll element, attaches no listeners and takes no measurements, and the slicer returns the original array untouched.TracesSpansTabde-dupesdata.contentbyiduntil the endpoint is fixed.Change checklist
Issues
AI-WATERMARK
AI-WATERMARK: yes
Testing
Commands, from
apps/opik-frontend:npm run lint— cleannpm run typecheck— cleanManual verification in Chrome against a local dev server on a reproduction project, toggling the props and column/row counts directly.
Scenarios validated:
size=10) — column axis windows down to 12 rendered header cells (DOM 30,000+ → ~3,500) while the row axis correctly stays unwindowed (only 10 rows, nothing to gain). Confirms the two axes no longer block each other.enabledthreshold in both directions at runtime through the column picker while scrolled horizontally and vertically, sampled every animation frame — scroll position, widths and cell counts hold (this is what theinitialOffset/initialRectseeding on the virtualizer fixes: without it, a disabled virtualizer keeps no offset or viewport, so the first enabled render started at offset 0 with a zero-width viewport and the container lost its horizontal scroll position).scrollHeightupdate correctly, row window resizes.colgroup, not the full column count.excludequery param — omittingexclude=["experiment"]reproduces it);uniqByonidremoves the duplicate before render and the key warning disappears.Not run: no automated tests were added — the repo has no
DataTabletest suite. Row virtualization on the other five tables that useDataTableVirtualBodywas not re-verified by hand beyond compiling and therowVirtualizationprop defaulting toenabled: true(unchanged behavior when the prop is omitted).Documentation
N/A — no user-facing documentation changes; the new props and their guards are documented by types and comments in
shared/DataTable/.