Skip to content

Commit 39db73d

Browse files
ldionneclaude
andcommitted
[UI] Refactor Graph page: introduce GraphDataCache, fix rendering bugs
Extract all graph page caching into a GraphDataCache class that manages test data (LRU), baseline data, test names, and scaffolds behind a clean async/sync API. This fixes three bugs: - Baselines now appear immediately when added (not only after filter change) - Changing aggregation now re-plots traces immediately - Text filter changes use cached test names instead of re-querying the API Also: remove batched chart rendering (caused flicker on legend toggle), remove dead setsEqual, deduplicate test-name discovery logic, parallelize baseline fetches, fix doPlot/filter race condition. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
1 parent 2fd12db commit 39db73d

6 files changed

Lines changed: 941 additions & 408 deletions

File tree

docs/design/v5-ui.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,8 +176,8 @@ The primary performance-over-time visualization. Replaces v4's graph page. This
176176
- Sample aggregation: how to combine multiple samples within a run (median/mean/min/max)
177177
- **Lazy loading with progressive rendering**: Data is fetched newest-first (`sort=-order&limit=10000`) and rendered incrementally. The chart first appears with the full x-axis scaffold (if available) and then progressively fills in data as pages arrive via cursor-based pagination (`fetchOneCursorPage`). This avoids blocking the UI on large datasets.
178178
- **X-axis scaffolding**: To prevent the x-axis from resizing/shifting as lazy-loaded pages arrive, the graph page pre-fetches the complete list of order values for each selected machine via paginated calls to the `GET machines/{name}/runs` endpoint (using `fetchOneCursorPage` with `sort=order`). When multiple machines are selected, the scaffold is the **union** of all machines' order values, so the x-axis spans the full range across all machines. Traces naturally have gaps where their machine has no data at a given order. Each machine's scaffold is fetched and cached independently; the union is recomputed when machines are added or removed. If a scaffold fetch fails for one machine, that machine's orders are simply not included in the union — the chart still works.
179-
- **Per-metric client-side caching**: Fetched data is cached per metric key (`machine::metric` combination). Each machine's data is fetched and cached independently. Changing the test filter, aggregation mode, or baselines re-renders instantly from the cache without any additional API calls. Switching back to a previously-viewed metric is also instant. Adding a second machine starts its own fetch pipeline while the first machine's data is already displayed. The cache is preserved across page unmount/remount, so navigating away and pressing browser back renders instantly from cache. In-flight fetches are aborted on unmount, but resume from where they left off on remount. Each machine's cache is cleared when that machine is removed from the chip list.
180-
- **Interactive controls**: All settings — metric, test filter, aggregation mode, baselines — are interactive from cache. Changing any of them re-renders without an API refetch. All settings sync to the URL for shareability. The legend table updates synchronously (instant feedback while typing), while the chart update is **skipped entirely when the active trace set has not changed** (e.g., typing additional filter characters that match the same tests). When the active set does change, user-initiated changes use **batched rendering**: traces are rendered in batches of 10 via `requestAnimationFrame`, so the chart achieves eventual consistency without freezing the browser — this matters when a filter matches thousands of tests and the 20-cap is disabled. During progressive data loading (new pages arriving from the API), the chart is updated in a single deferred `requestAnimationFrame` call (no batching) to avoid the batch sequence being repeatedly canceled by rapid page arrivals.
179+
- **Client-side caching**: Test names, data points, scaffolds, and baseline data are cached locally. Test names are fetched once per machine/metric combination (all names, no server-side filter) and filtered client-side. Changing the test filter, aggregation mode, or baselines re-renders instantly from cache without any additional API calls. Adding a second machine starts its own fetch pipeline while the first machine's data is already displayed. The cache is preserved across page unmount/remount, so navigating away and pressing browser back renders instantly. All caches are cleared on suite change.
180+
- **Interactive controls**: All settings — metric, test filter, aggregation mode, baselines — are interactive from cache. Changing any of them re-renders without an API refetch. All settings sync to the URL for shareability. The legend table updates synchronously (instant feedback while typing). Chart updates are deferred via `requestAnimationFrame` to keep the UI responsive; a generation counter ensures only the latest update paints.
181181
- **Incremental chart updates**: The chart component exposes a `ChartHandle` API (via `createTimeSeriesChart`) that supports incremental updates through `Plotly.react()` — the chart is updated in-place as new pages of data arrive, rather than being destroyed and re-created.
182182
- **Zoom preservation during progressive loading**: If the user zooms into the chart while data is still loading, the zoom is preserved across incremental updates. The x-axis range is always preserved (it was established by the scaffold or by user zoom). The y-axis range is preserved only when the user has explicitly zoomed; otherwise, it auto-ranges to accommodate new data as it arrives. Double-clicking the chart resets the zoom to the full range as usual.
183183
- **Legend table**: Below the chart, a table lists traces sorted alphabetically by name (not in a scrollable container — the table is part of the page flow, like the Compare page's table). A message line above the table rows always shows a matching count (e.g., "42 of 150 tests matching"); when the 20-cap is active, the cap warning replaces it. Each row represents one trace (`{test name} - {machine name}`), with a colored marker symbol character (●, ▲, ■, etc. in the trace's color) identifying both the test (by color) and the machine (by shape). The test filter matches on test name only — matching test names show all their machine variants; non-matching names are hidden entirely. Tests that are inactive (manually hidden or beyond the 20-cap) are grayed out in place. Clicking a row toggles the test's visibility on the chart. Double-clicking a row is a shortcut for hiding all other visible tests (equivalent to single-clicking every other test one by one) — it populates `manuallyHidden` with all visible tests except the double-clicked one. Double-clicking the same test again when it is the only visible test restores all tests. Subsequent single-clicks work naturally against the `manuallyHidden` set. Plotly's built-in legend is disabled; the table replaces it. Bidirectional hover highlighting: hovering a table row highlights the corresponding chart trace by emphasizing the entire trace line (thicker line, full opacity) while dimming all other traces via `Plotly.restyle()`; hovering a chart trace highlights the table row.

docs/v5-ui-implementation-plan.md

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1577,18 +1577,15 @@ The graph page is the most data-intensive page. It uses **lazy loading with per-
15771577
- Colors are assigned by alphabetical index of all **test names** (not trace names) using the D3 category10 palette (`PLOTLY_COLORS`), ensuring the same test on different machines shares the same color.
15781578
- Plotly's built-in legend is disabled (`showlegend: false` always). Traces receive explicit `line.color` and `marker.color` from the color map, and `marker.symbol` from the machine symbol assignment.
15791579
- Bidirectional hover: the legend table dispatches `GRAPH_TABLE_HOVER` events; the chart dispatches `GRAPH_CHART_HOVER` events. The graph page wires these via `onCustomEvent()` (which now returns a cleanup function) to call `chartHandle.hoverTrace()` and `legendHandle.highlightRow()` respectively. `hoverTrace()` uses `Plotly.restyle()` to emphasize the entire trace line (line width 3px, opacity 1.0) and dim all other main traces (opacity 0.2) and pinned-order traces. Passing `null` restores all traces to their normal appearance (line width 1.5px, opacity 1.0). The restyle calls are chained after `plotReady` to avoid race conditions with newPlot/react.
1580-
- `manuallyHidden` (Set of trace names — `'{test} - {machine}'`), `autoCapped` (boolean), `prevActiveTraceNames` (`Set<string>` — the active set from the last chart render, used to skip no-op chart updates), and `legendHandle` are module-scope state. They are preserved across unmount/remount (like the cache). `computeActiveTests()` takes four inputs (allTraceNames, testFilter, manuallyHidden, autoCapped) and returns the active set. The test filter matches against the **test name portion** of each trace name. Double-click isolation is implemented purely through `manuallyHidden` — no separate `isolatedTest` state.
1581-
1582-
3. **Per-metric client-side cache**:
1583-
- Cache structure: `Map<string, CachedMetricData>` where the key is `${machine}::${metric}` and the value holds the accumulated data points, the next cursor (if fetching is still in progress), and whether fetching is complete. Each machine's data is cached independently.
1584-
- Cache is populated incrementally as pages arrive for each machine.
1585-
- **Instant interactions from cache**: When the user changes the test filter, aggregation mode, or pinned orders, the page re-processes all machines' cached data — no API call, no loading spinner. This is the primary UX benefit of the caching architecture. The `renderFromCache()` function accepts a `batch` parameter and is split into two phases:
1586-
- **Synchronous phase** (legend table + progress): For each machine, extract test names from its cache. Compute trace names (`{test} - {machine}`), compute active set, build legend entries, and update the legend table. This is cheap DOM work and provides instant feedback (e.g., showing which tests match while the user types a filter).
1587-
- **Deferred chart update phase**: For each machine, build traces with the machine's `markerSymbol`. Merge all machines' traces into a single list, then feed to the chart via `requestAnimationFrame`. **Before scheduling any chart work, compare the new active trace name set to `prevActiveTraceNames` (a module-scope `Set<string>`). If the sets are identical and no new data has arrived (`batch = true` indicates a user-initiated change, not a data update), skip the entire deferred phase — the chart already shows the correct traces.** When the chart does need updating, the behavior depends on the `batch` parameter:
1588-
- **`batch = true`** (user-initiated changes: filter, toggle, aggregation, pinned orders): Traces are fed in **batches of `CHART_BATCH_SIZE` (10)** per animation frame. This batching exists to prevent the browser from freezing when a filter matches thousands of tests and the 20-cap is disabled — the chart achieves eventual consistency while the UI stays responsive.
1589-
- **`batch = false`** (progressive data loading: new pages arriving from the API): All traces are rendered in a **single deferred `requestAnimationFrame` call**.
1590-
- In both modes, a module-scope `chartRenderGen` generation counter ensures stale render sequences are abandoned. A `pendingChartRAF` ID is also tracked so the pending frame can be canceled on `unmount()`. Pinned orders are included in every update so pinned-order lines appear from the first frame.
1591-
- **Cache persists across navigation**: The per-machine data cache and scaffolds are module-scope variables that survive `unmount()`/`mount()` cycles. When the user navigates away and presses browser back, `doPlot()` finds the cached data and renders instantly. In-flight fetches are aborted on unmount (their `finally` blocks reset `entry.loading = false`), so `startLazyLoad()` resumes from the saved `nextCursor` on remount. A machine's cache is cleared when that machine is removed from the chip list. Module-scope UI state (`manuallyHidden`, `autoCapped`, `prevActiveTraceNames`, `chartRenderGen`, `cachedSuggestions`) is reset on unmount to prevent stale state on remount — the machines list is restored from URL params.
1580+
- `manuallyHidden` (Set of trace names — `'{test} · {machine}'`), `autoCapped` (boolean), and `legendHandle` are module-scope state. They are preserved across unmount/remount (like the cache). A module-level `GraphDataCache` instance serves as the sole data access layer — all test names, data points, scaffolds, and baseline data are accessed through it. `computeActiveTests()` takes four inputs (allTraceNames, testFilter, manuallyHidden, autoCapped) and returns the active set. The test filter matches against the **test name portion** of each trace name. Double-click isolation is implemented purely through `manuallyHidden` — no separate `isolatedTest` state.
1581+
1582+
3. **`GraphDataCache` -- centralized data access layer**:
1583+
- A `GraphDataCache` class (in `pages/graph-data-cache.ts`) manages all data access for the graph page. It provides both async methods (fetch on demand) and sync methods (read cache only, never trigger fetches).
1584+
- **Main data** keyed by `(suite, machine, metric, test)` with the invariant that entries always contain ALL orders (full time series). LRU-bounded at 500 entries. **Baseline data** keyed by `(suite, machine, order, metric)` with `fetchedTests` tracking (separate, not LRU-bounded). **Test names** and **scaffolds** are simple Maps. All cleared on `cache.clear()`.
1585+
- **Instant interactions from cache**: When the user changes the test filter, aggregation mode, or baselines, the page re-processes cached data via `readCachedTestData()` and `readCachedBaselineData()`. The `renderFromDiscoveredTests()` function is split into two phases:
1586+
- **Synchronous phase** (legend table + progress): reads cached data, builds traces, computes active set, and updates the legend table.
1587+
- **Deferred chart update phase**: feeds all traces to the chart via a single deferred `requestAnimationFrame` call. The chart is always updated (no no-op optimization). A module-scope `chartRenderGen` generation counter ensures stale render sequences are abandoned. A `pendingChartRAF` ID is also tracked so the pending frame can be canceled on `unmount()`. Baselines are included in every update so baseline lines appear from the first frame.
1588+
- **Cache persists across navigation**: The `GraphDataCache` instance is a module-scope variable that survives `unmount()`/`mount()` cycles. When the user navigates away and presses browser back, `doPlot()` finds cached data and renders instantly. `cache.clear()` is called on suite change. Module-scope UI state (`manuallyHidden`, `currentVisibleTraceNames`, `chartRenderGen`) is reset on unmount -- the machines list is restored from URL params.
15921589

15931590
4. **Pinned orders — asynchronous fetch with aggregation**:
15941591
- Pinned orders are fetched **after the first chart render**, so they do not block initial display.
@@ -1646,8 +1643,9 @@ The graph page is the most data-intensive page. It uses **lazy loading with per-
16461643
- Color assignment: verify colors are assigned by alphabetical index of test names (not trace names); verify same test on different machines gets the same color
16471644
- Test cap warning: verify warning shows when > N traces match
16481645
- Aggregation: verify data points are correctly aggregated before charting
1649-
- Cache hit: verify that changing test filter re-renders from cache without API call
1650-
- Skip-no-op: verify that `setsEqual` returns true for identical sets and false for different sets; verify that the chart update is skipped when the active trace set has not changed (batch=true path only)
1646+
- Cache hit: verify that changing test filter re-renders from cache without API call (via `GraphDataCache` test names cache)
1647+
- `GraphDataCache` unit tests: LRU eviction, error handling (no cache on error), abort signals, `ensureTestData` batch fetch, `getBaselineData` re-fetch on expanded test list, `filterTestNames` pure function, `scaffoldUnion` across machines
1648+
- Bug fix integration: verify aggregation change triggers chart update; verify baseline add/remove triggers chart update; verify `setsEqual` returns true for identical sets and false for different sets
16511649
- Cache miss: verify that a new machine triggers a fetch for that machine only
16521650
- Progressive rendering: verify `chartHandle.update()` is called after each page, with traces from all machines
16531651
- AbortController: verify that removing a machine aborts its in-flight fetch without affecting other machines

0 commit comments

Comments
 (0)