From e1832ee5d5108b7a1ed4e8e7b96d2c1e021b525f Mon Sep 17 00:00:00 2001 From: Nimrod Lahav Date: Thu, 20 Aug 2026 15:52:19 +0300 Subject: [PATCH 1/2] [OPIK_8005] [FE] perf: cut per-cell store subscriptions 5x, skip no-op dataset hydration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two contained changes for the Playground large-dataset slowdown. Both are behaviour-preserving; neither touches layout. This is partial relief, not a fix for OPIK_8005 — the unvirtualized output grid remains the structural problem. PlaygroundOutputCell subscribed to the store five times per cell (value, isLoading, stale, traceId, selectedRuleIds). All five of those hooks delegate to the same useOutputByPromptDatasetItemId selector, and the field access happens outside the selector, so zustand was comparing the same output object five times over. Every streaming token therefore ran that selector (dataset items x prompts x 5) times — roughly 4,600 evaluations per token on a 458-row dataset with two prompt variants. Read the output object once and pick the fields off it instead. The defaults applied here are the ones those hooks applied, so re-render behaviour is unchanged; there are simply 5x fewer subscriptions. The five hooks stay exported, as PlaygroundPromptOutput still uses three of them. useIncrementalDatasetHydration looped every row and rebuilt the whole array per iteration, but hydrateDatasetItemData only fetches when the item carries truncated media. On a text/JSON dataset every iteration was a no-op, so a 1000-row dataset did ~1M array copies to arrive back at the data it started with. Select the media-bearing indexes up front and loop only over those. Sized against a real production workspace: datasets there run to 1078 items and carry zero truncated media, so the hydration loop was pure waste. Refs: OPIK_8005, CUST_6878, AI_546 Co-Authored-By: Claude Opus 5 (1M context) --- .../PlaygroundOutputCell.tsx | 41 +++++++------------ .../useIncrementalDatasetHydration.ts | 29 +++++++++++-- 2 files changed, 41 insertions(+), 29 deletions(-) diff --git a/apps/opik-frontend/src/v2/pages/PlaygroundPage/PlaygroundOutputs/PlaygroundOutputTable/PlaygroundOutputCell.tsx b/apps/opik-frontend/src/v2/pages/PlaygroundPage/PlaygroundOutputs/PlaygroundOutputTable/PlaygroundOutputCell.tsx index 6edf74bb1d0..0563290bb68 100644 --- a/apps/opik-frontend/src/v2/pages/PlaygroundPage/PlaygroundOutputs/PlaygroundOutputTable/PlaygroundOutputCell.tsx +++ b/apps/opik-frontend/src/v2/pages/PlaygroundPage/PlaygroundOutputs/PlaygroundOutputTable/PlaygroundOutputCell.tsx @@ -4,11 +4,7 @@ import { ListTree } from "lucide-react"; import CellWrapper from "@/shared/DataTableCells/CellWrapper"; import { - useOutputLoadingByPromptDatasetItemId, - useOutputStaleStatusByPromptDatasetItemId, - useOutputValueByPromptDatasetItemId, - useSelectedRuleIdsByPromptDatasetItemId, - useTraceIdByPromptDatasetItemId, + useOutputByPromptDatasetItemId, useDatasetType, useExperimentIdByPromptId, } from "@/store/PlaygroundStore"; @@ -46,30 +42,23 @@ const PlaygroundOutputCell: React.FunctionComponent< const workspaceName = useAppStore((state) => state.activeWorkspaceName); - const value = useOutputValueByPromptDatasetItemId( - promptId, - originalRow.dataItemId, - ); - - const isLoading = useOutputLoadingByPromptDatasetItemId( - promptId, - originalRow.dataItemId, - ); - - const stale = useOutputStaleStatusByPromptDatasetItemId( - promptId, - originalRow.dataItemId, - ); - - const traceId = useTraceIdByPromptDatasetItemId( - promptId, - originalRow.dataItemId, - ); - - const selectedRuleIds = useSelectedRuleIdsByPromptDatasetItemId( + // One store subscription per cell, not five. Each of the fields below used to + // come from its own hook, and every one of those hooks runs the *same* + // selector, so a single streaming token re-ran it (dataset items x prompts x 5) + // times. Reading the output object once and picking the fields off it is + // equivalent — the defaults below are the ones those hooks applied — while + // cutting the per-token selector work by 5x. The selector returns the stored + // object reference, which only changes when this cell's own output changes, so + // unrelated updates still don't re-render this cell. + const output = useOutputByPromptDatasetItemId( promptId, originalRow.dataItemId, ); + const value = output?.value ?? null; + const isLoading = output?.isLoading ?? false; + const stale = output?.stale ?? false; + const traceId = output?.traceId ?? null; + const selectedRuleIds = output?.selectedRuleIds; const datasetType = useDatasetType(); const experimentId = useExperimentIdByPromptId(promptId); diff --git a/apps/opik-frontend/src/v2/pages/PlaygroundPage/useIncrementalDatasetHydration.ts b/apps/opik-frontend/src/v2/pages/PlaygroundPage/useIncrementalDatasetHydration.ts index 4a4161c7b72..c259e74c341 100644 --- a/apps/opik-frontend/src/v2/pages/PlaygroundPage/useIncrementalDatasetHydration.ts +++ b/apps/opik-frontend/src/v2/pages/PlaygroundPage/useIncrementalDatasetHydration.ts @@ -1,6 +1,7 @@ import { useEffect, useRef, useState } from "react"; import { DatasetItem } from "@/types/datasets"; import { useHydrateDatasetItemData } from "@/v2/pages/PlaygroundPage/useHydrateDatasetItemData"; +import { containsTruncatedMedia } from "@/lib/media"; export function useIncrementalDatasetHydration(datasetItems: DatasetItem[]): { hydratedItems: DatasetItem[]; @@ -21,19 +22,41 @@ export function useIncrementalDatasetHydration(datasetItems: DatasetItem[]): { } setHydratedItems(datasetItems); + + // Only items carrying truncated media need a round-trip — that is the sole + // condition under which hydrateDatasetItemData fetches anything. Selecting + // them up front means the loop below runs once per *media* item rather than + // once per row, and each pass rebuilt the whole array, so a 1000-row text + // dataset was doing ~1M array copies to arrive back at the data it started + // with. + const indexesToHydrate = datasetItems.reduce( + (acc, item, index) => { + if (containsTruncatedMedia(item.data)) { + acc.push(index); + } + return acc; + }, + [], + ); + + if (indexesToHydrate.length === 0) { + setIsHydrating(false); + return; + } + setIsHydrating(true); const hydrateItems = async () => { - for (let i = 0; i < datasetItems.length; i++) { + for (const index of indexesToHydrate) { if (cancelledRef.current) return; - const hydratedData = await hydrateDatasetItemData(datasetItems[i]); + const hydratedData = await hydrateDatasetItemData(datasetItems[index]); if (cancelledRef.current) return; setHydratedItems((prev) => prev.map((item, idx) => - idx === i ? { ...item, data: hydratedData } : item, + idx === index ? { ...item, data: hydratedData } : item, ), ); } From fb17f8e60aa1f70aa42bd6ca3404397476bc81e6 Mon Sep 17 00:00:00 2001 From: Nimrod Lahav Date: Thu, 20 Aug 2026 16:16:43 +0300 Subject: [PATCH 2/2] [OPIK-8005] [FE] fix: scope hydration cancellation to the effect run (Baz review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Baz caught a real bug in the previous commit, and it is worth being precise that the new early-return path made a latent flaw dangerous rather than merely inelegant. cancelledRef is a single ref shared across every effect run, and the effect body reset it to false on entry. React runs the previous run's cleanup before the next body, so the sequence was: run 1 starts hydrating dataset A and installs cleanup -> dataset changes -> cleanup sets cancelled = true -> run 2 enters and resets it to false, un-cancelling run 1's still-pending loop -> run 1's continuation passes its post-await check and calls setHydratedItems on the new array. Before this PR the only early return sat after setHydratedItems([]), so that stale continuation's prev.map() ran over an empty array and produced nothing — harmless. The no-media early return added in the previous commit sits after setHydratedItems(datasetItems) with a populated array, so the same continuation writes dataset A's data into dataset B's array at a positional index. That is silent cross-dataset corruption, reachable by switching dataset or page size while a media-bearing dataset is still hydrating. Replace the shared ref with a `cancelled` flag scoped to each effect run. Each run observes only its own flag, so early-returning without installing cleanup cannot resurrect an older run's loop. This also fixes the pre-existing empty- dataset path and the setState-after-unmount case, and removes the useRef import. Verified: tsc, eslint --max-warnings=0, prettier, and the full vitest suite (150 files, 2229 tests) all pass. Refs: OPIK-8005, CUST_6878, AI_546 Co-Authored-By: Claude Opus 5 (1M context) --- .../useIncrementalDatasetHydration.ts | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/apps/opik-frontend/src/v2/pages/PlaygroundPage/useIncrementalDatasetHydration.ts b/apps/opik-frontend/src/v2/pages/PlaygroundPage/useIncrementalDatasetHydration.ts index c259e74c341..8e6943d4054 100644 --- a/apps/opik-frontend/src/v2/pages/PlaygroundPage/useIncrementalDatasetHydration.ts +++ b/apps/opik-frontend/src/v2/pages/PlaygroundPage/useIncrementalDatasetHydration.ts @@ -1,4 +1,4 @@ -import { useEffect, useRef, useState } from "react"; +import { useEffect, useState } from "react"; import { DatasetItem } from "@/types/datasets"; import { useHydrateDatasetItemData } from "@/v2/pages/PlaygroundPage/useHydrateDatasetItemData"; import { containsTruncatedMedia } from "@/lib/media"; @@ -10,10 +10,16 @@ export function useIncrementalDatasetHydration(datasetItems: DatasetItem[]): { const hydrateDatasetItemData = useHydrateDatasetItemData(); const [hydratedItems, setHydratedItems] = useState([]); const [isHydrating, setIsHydrating] = useState(false); - const cancelledRef = useRef(false); useEffect(() => { - cancelledRef.current = false; + // Scoped to this effect run rather than a shared ref. React runs the previous + // run's cleanup before this body, so an in-flight hydration from an earlier + // dataset observes its own `cancelled === true` and stops — even when this run + // returns early below and installs no cleanup of its own. A shared ref instead + // got reset here on every run, un-cancelling the previous run's loop; its + // continuation would then write the old dataset's data into the new array at a + // positional index. Also covers unmount. + let cancelled = false; if (datasetItems.length === 0) { setHydratedItems([]); @@ -48,11 +54,11 @@ export function useIncrementalDatasetHydration(datasetItems: DatasetItem[]): { const hydrateItems = async () => { for (const index of indexesToHydrate) { - if (cancelledRef.current) return; + if (cancelled) return; const hydratedData = await hydrateDatasetItemData(datasetItems[index]); - if (cancelledRef.current) return; + if (cancelled) return; setHydratedItems((prev) => prev.map((item, idx) => @@ -67,7 +73,7 @@ export function useIncrementalDatasetHydration(datasetItems: DatasetItem[]): { hydrateItems(); return () => { - cancelledRef.current = true; + cancelled = true; }; }, [datasetItems, hydrateDatasetItemData]);