From 82802dd8f7aa603ba8c262c0d86054f0b9b3b550 Mon Sep 17 00:00:00 2001 From: aaltshuler Date: Sat, 5 Sep 2026 02:35:12 +0300 Subject: [PATCH] Fix graph state races, restoration, exports, and data adapters --- .changeset/fix-full-code-review.md | 19 ++ apps/demo/e2e/deep-link.spec.ts | 50 +++++ apps/demo/e2e/fit-view-transition.spec.ts | 55 +++++ apps/demo/src/App.tsx | 85 ++++++-- packages/core/src/instance.ts | 193 +++++++++++++----- packages/core/src/viewState.ts | 3 +- packages/core/test/exports.test.ts | 56 +++++ packages/core/test/folds.test.ts | 37 +++- packages/core/test/recovery.test.ts | 26 +++ packages/core/test/undo-redo.test.ts | 69 +++++++ packages/core/test/view-state.test.ts | 148 ++++++++++++++ packages/core/test/worker-admission.test.ts | 79 +++++++ packages/data/src/arrow.ts | 40 +++- packages/data/src/jsonSafe.ts | 19 +- packages/data/test/arrow.test.ts | 85 +++++++- packages/data/test/review-fixes.test.ts | 9 + packages/engine-cosmos/README.md | 6 + packages/engine-cosmos/src/CosmosEngine.ts | 17 +- .../engine-cosmos/test/cosmos-engine.test.ts | 26 +++ packages/omnigraph/src/codegen.ts | 27 ++- packages/omnigraph/src/loader.ts | 2 + packages/omnigraph/src/normalize.ts | 3 + packages/omnigraph/test/codegen.test.ts | 32 +++ packages/omnigraph/test/loader.test.ts | 27 +++ packages/omnigraph/test/normalize.test.ts | 20 ++ .../react/src/components/Navigator/index.tsx | 33 +-- packages/react/src/components/Table/index.tsx | 21 +- .../react/src/components/Tooltip/index.tsx | 5 + packages/react/test/navigator.test.tsx | 33 +++ packages/react/test/table.test.tsx | 49 +++++ packages/react/test/tooltip.test.tsx | 30 +++ 31 files changed, 1187 insertions(+), 117 deletions(-) create mode 100644 .changeset/fix-full-code-review.md create mode 100644 apps/demo/e2e/fit-view-transition.spec.ts diff --git a/.changeset/fix-full-code-review.md b/.changeset/fix-full-code-review.md new file mode 100644 index 0000000..750457d --- /dev/null +++ b/.changeset/fix-full-code-review.md @@ -0,0 +1,19 @@ +--- +"@modernrelay/orbit-core": patch +"@modernrelay/orbit-data": patch +"@modernrelay/orbit-engine-cosmos": patch +"@modernrelay/orbit-omnigraph": patch +"@modernrelay/orbit-react": patch +--- + +Prevent stale worker results and view restores from replacing newer datasets, +preserve saved layouts before mounting, and restore scale domains and fold +counts during undo. Visible exports now respect edge filters. + +Fix table filtering after empty results and refresh navigator and tooltip +content when the scene changes. Keep fit-view zoom limits effective during +position transitions. + +Normalize nested Arrow values and Omnigraph temporal lists, honor cancellation +through the final ingestion boundary, and avoid collisions between generated +node and edge type names. diff --git a/apps/demo/e2e/deep-link.spec.ts b/apps/demo/e2e/deep-link.spec.ts index 337ecdf..708bc03 100644 --- a/apps/demo/e2e/deep-link.spec.ts +++ b/apps/demo/e2e/deep-link.spec.ts @@ -19,6 +19,7 @@ import { expect, test } from '@playwright/test'; import type { Page } from '@playwright/test'; +import type { GraphViewState } from '@modernrelay/orbit-core'; const READY_DOT = '[data-testid="status-dot"][title="ready"]'; const SELECTED_COUNT = '[data-testid="selected-count"]'; @@ -111,6 +112,55 @@ test('a corrupted ?view= applies NOTHING (the atomic rule, adversarially)', asyn await expect(page.locator(SELECTED_COUNT)).toHaveText('3,000'); }); +test('share → reload restores selected styling and keeps the style controls live', async ({ page }) => { + await page.goto('/'); + await ready(page); + const simulation = page.locator('[data-orbit-toolbar-button="simulation"]'); + if (await simulation.getAttribute('aria-pressed') === 'true') await simulation.click(); + + // Use the semantic surface to select one node without GPU picking. A + // nonempty controlled selection exercises aggregate host reflection. + await page.getByTestId('navigator-toggle').click(); + await page.getByRole('option').first().press('Space'); + await expect(page.locator(SELECTED_COUNT)).toHaveText('1'); + await page.getByTestId('navigator-toggle').click(); + await page.getByTestId('node-color-mode').selectOption('degree'); + await page.getByTestId('node-size-mode').selectOption('scale'); + await page.getByTestId('edge-arrows').check(); + await page.getByTestId('show-links').uncheck(); + await page.getByTestId('theme-light').check(); + await page.getByTestId('share-view').click(); + await page.waitForFunction(() => window.location.search.includes('view=')); + const url = page.url(); + const saved = JSON.parse(new URL(url).searchParams.get('view')!) as GraphViewState; + expect(saved.selection.nodeIds).toHaveLength(1); + expect(saved.styling).toMatchObject({ showLinks: false, edgeArrows: true, theme: 'light' }); + + await page.goto(url); + await ready(page); + await expect(page.locator(SELECTED_COUNT)).toHaveText('1'); + await expect(page.getByTestId('app-root')).toHaveAttribute('data-theme', 'light'); + await expect(page.getByTestId('node-color-mode')).toHaveValue('degree'); + await expect(page.getByTestId('node-size-mode')).toHaveValue('scale'); + await expect(page.getByTestId('edge-arrows')).toBeChecked(); + await expect(page.getByTestId('show-links')).not.toBeChecked(); + await expect(page.getByTestId('legend-panel')).toContainText('degree'); + await expect(page.getByTestId('legend-panel-size')).toBeVisible(); + + // Re-sharing reads actual instance state, so these assertions cover more + // than the controls' checked/value attributes. + await page.getByTestId('share-view').click(); + const restored = JSON.parse(new URL(page.url()).searchParams.get('view')!) as GraphViewState; + expect(restored.selection).toEqual(saved.selection); + expect(restored.styling).toEqual(saved.styling); + + await page.getByTestId('node-color-mode').selectOption('category'); + await page.getByTestId('node-size-mode').selectOption('accessor'); + await page.getByTestId('show-links').check(); + await expect(page.getByTestId('legend-panel').locator('[data-orbit-legend-row]')).toHaveCount(6); + await expect(page.getByTestId('legend-panel-size')).toHaveCount(0); +}); + test('a stale dataRef trips the mismatch banner; Restore anyway opts in', async ({ page }) => { await page.goto('/'); await ready(page); diff --git a/apps/demo/e2e/fit-view-transition.spec.ts b/apps/demo/e2e/fit-view-transition.spec.ts new file mode 100644 index 0000000..cc4ce2a --- /dev/null +++ b/apps/demo/e2e/fit-view-transition.spec.ts @@ -0,0 +1,55 @@ +import { expect, test } from '@playwright/test'; + +// Serve a blank same-origin page so this regression exercises the real +// adapter/Cosmos pair without the demo's own camera and simulation controls. +const engineUrl = `/@fs${new URL('../../../packages/engine-cosmos/src/CosmosEngine.ts', import.meta.url).pathname}`; + +for (const scenario of [ + { name: 'shrinking', before: 1000, after: 10, zoom: 1.5 }, + { name: 'expanding', before: 10, after: 1000, zoom: 0.8 }, +]) { + test(`fitView uses ${scenario.name} transition destinations and respects maxZoom`, async ({ page }) => { + await page.route('**/__engine_fit_test__', (route) => + route.fulfill({ contentType: 'text/html', body: '' }), + ); + await page.goto('/__engine_fit_test__'); + const result = await page.evaluate(async ({ url, before, after }) => { + const { CosmosEngine } = await import(url) as typeof import('@modernrelay/orbit-engine-cosmos'); + const host = document.createElement('div'); + host.style.cssText = 'width:1000px;height:1000px'; + document.body.appendChild(host); + const engine = new CosmosEngine({ + initialConfig: { enableSimulation: false, rescalePositions: false, transitionDuration: 1000 }, + }); + const frames = () => new Promise((resolve) => { + requestAnimationFrame(() => requestAnimationFrame(() => resolve())); + }); + try { + await engine.mount(host, {}); + engine.commit({ + revision: 1, + structure: { pointCount: 2, positions: Float32Array.of(0, 0, before, before), links: new Uint32Array() }, + }); + await frames(); + engine.commit({ + revision: 2, + structure: { pointCount: 2, positions: Float32Array.of(0, 0, after, after), links: new Uint32Array() }, + }); + const atFit = Array.from(engine.getPositions()!); + engine.fitView({ maxZoom: 1.5, durationMs: 0 }); + await frames(); + return { atFit, viewport: engine.getViewport() }; + } finally { + engine.destroy(); + host.remove(); + } + }, { url: engineUrl, before: scenario.before, after: scenario.after }); + + // Confirm the test reached a transition: the GPU still holds the old + // positions, while the fit must center and size for the destination. + expect(result.atFit).toEqual([0, 0, scenario.before, scenario.before]); + expect(result.viewport?.zoom).toBeCloseTo(scenario.zoom, 5); + expect(result.viewport?.x).toBeCloseTo(scenario.after / 2, 5); + expect(result.viewport?.y).toBeCloseTo(scenario.after / 2, 5); + }); +} diff --git a/apps/demo/src/App.tsx b/apps/demo/src/App.tsx index ce50d28..1c47aab 100644 --- a/apps/demo/src/App.tsx +++ b/apps/demo/src/App.tsx @@ -93,6 +93,7 @@ import type { GraphSnapshot, GraphPerfSnapshot, GraphStoreState, + GraphViewState, GroupSpec, InstanceStatus, LabelConfig, @@ -594,6 +595,9 @@ export function App() { // --- style state (Style panel) --- const [themeBase, setThemeBase] = useState('dark'); const [colorMode, setColorMode] = useState('category'); + const [restoredNodeColor, setRestoredNodeColor] = useState | null>( + null, + ); const [showLabelType, setShowLabelType] = useState(true); /** * fold counts mirrored from `store.folds` (subscribed below, once @@ -606,6 +610,9 @@ export function App() { */ const [foldCounts, setFoldCounts] = useState>(EMPTY_FOLD_COUNTS); const [sizeMode, setSizeMode] = useState('accessor'); + const [restoredNodeSize, setRestoredNodeSize] = useState | null>( + null, + ); const [edgeArrows, setEdgeArrows] = useState(false); const [showLinks, setShowLinks] = useState(true); @@ -764,18 +771,20 @@ export function App() { // Stream mode keeps the plain accessor in 'category' mode (its cluster // count differs from the declared declarative domain). const nodeColorProp: Scale | typeof nodeColor = - colorMode === 'degree' + restoredNodeColor ?? + (colorMode === 'degree' ? DEGREE_COLOR_SCALE : mode.kind === 'omnigraph' ? ogColorScale ?? nodeColor : mode.kind === 'declarative' ? CLUSTER_COLOR_SCALE - : nodeColor; + : nodeColor); // M5 keeps a fixed point size in accessor mode (see M5_NODE_SIZE); the // degree Scale still applies when the Style panel selects it. const nodeSizeProp: Scale | typeof nodeSize | typeof M5_NODE_SIZE = - sizeMode === 'scale' ? DEGREE_SIZE_SCALE : mode.kind === 'semantic' ? M5_NODE_SIZE : nodeSize; + restoredNodeSize ?? + (sizeMode === 'scale' ? DEGREE_SIZE_SCALE : mode.kind === 'semantic' ? M5_NODE_SIZE : nodeSize); const filter = useMemo | null>(() => { if (mode.kind === 'omnigraph') { @@ -1192,18 +1201,56 @@ export function App() { [mode.kind, gen], ); - /** aggregate reflection: selection is this app's ONE controlled - * lane, so the intent reduces to reflecting it in one commit. */ - const onViewStateRestore = useCallback((intent: { next: unknown }) => { - const next = intent.next as { selection?: { nodeIds?: readonly string[] } }; - setSelection([...(next.selection?.nodeIds ?? [])]); + /** Keep both the graph props and their visible controls aligned with a + * restored view. Retain serialized scale descriptors verbatim; choosing a + * new style in the panel releases the restored override. */ + const reflectStyling = useCallback((styling: GraphViewState['styling']) => { + if (styling === undefined) return; + if (styling.theme !== undefined) setThemeBase(styling.theme); + if (styling.showLinks !== undefined) setShowLinks(styling.showLinks); + if (styling.edgeArrows !== undefined) setEdgeArrows(styling.edgeArrows); + if (styling.nodeColor !== undefined) { + setRestoredNodeColor(styling.nodeColor); + setColorMode(styling.nodeColor.kind === 'categorical' ? 'category' : 'degree'); + } + if (styling.nodeSize !== undefined) { + setRestoredNodeSize(styling.nodeSize); + setSizeMode('scale'); + } }, []); + /** A controlled restore delegates styling to the host too. Reflect every + * participating prop in this same React commit before acknowledging it. */ + const onViewStateRestore = useCallback((intent: { next: unknown }) => { + const next = intent.next as GraphViewState; + setSelection([...next.selection.nodeIds]); + setM5Pinned([...next.pinnedNodeIds]); + if (m5GroupingRef.current === 'manual') { + setM5Groups(next.groups.filter((group): group is GroupSpec => 'id' in group)); + } + reflectStyling(next.styling); + }, [reflectStyling]); + + const restoreView = useCallback(async (raw: unknown, ignoreMismatch = false) => { + const instance = graphRef.current?.instance; + if (instance === undefined) return undefined; + const result = await instance.setViewState(raw, { ignoreMismatch }); + // With no changed controlled slices, core restores internally and emits + // no aggregate intent. Mirror styling into the controls after admission. + if (result.status === 'applied' && graphRef.current?.instance === instance) { + reflectStyling((raw as GraphViewState).styling); + } + return result; + }, [reflectStyling]); + /** Share the current view: ?view= in the URL bar + clipboard best-effort. */ const shareView = useCallback(() => { const handle = graphRef.current; if (handle === null) return; const state = handle.getViewState(); + // The demo customizes the base theme's background, which the core + // intentionally omits. The host still knows its serializable base. + state.styling = { ...state.styling, theme: themeBase }; const url = new URL(window.location.href); url.searchParams.set('view', JSON.stringify(state)); // Deep-links are for HUMAN-scale state. A pathological state (say, all @@ -1219,7 +1266,7 @@ export function App() { } void navigator.clipboard?.writeText(text).catch(() => {}); window.setTimeout(() => setShareNote(null), 4000); - }, []); + }, [themeBase]); /** exports. Each mirrors its output onto window.__lastExport (the * e2e observability hook) and triggers a real download. The ?svgcap= param @@ -1291,21 +1338,21 @@ export function App() { // the restored camera wins. if (st.status !== 'ready' || st.viewport === null) return; restoredRef.current = true; - void instance.setViewState(raw).then((result) => { - if (result.status === 'mismatch') setMismatchRaw(raw); + void restoreView(raw).then((result) => { + if (result?.status === 'mismatch') setMismatchRaw(raw); }); }; attempt(); const unsubscribe = instance.store.subscribe(attempt); return unsubscribe; - }, [mode.kind]); + }, [mode.kind, restoreView]); const restoreAnyway = useCallback(() => { const raw = mismatchRaw; setMismatchRaw(null); if (raw === null) return; - void graphRef.current?.instance.setViewState(raw, { ignoreMismatch: true }); - }, [mismatchRaw]); + void restoreView(raw, true); + }, [mismatchRaw, restoreView]); const graphKey = mode.kind === 'stream' @@ -1480,9 +1527,15 @@ export function App() { themeBase={themeBase} onThemeBaseChange={setThemeBase} colorMode={colorMode} - onColorModeChange={setColorMode} + onColorModeChange={(next) => { + setRestoredNodeColor(null); + setColorMode(next); + }} sizeMode={sizeMode} - onSizeModeChange={setSizeMode} + onSizeModeChange={(next) => { + setRestoredNodeSize(null); + setSizeMode(next); + }} edgeArrows={edgeArrows} onEdgeArrowsChange={setEdgeArrows} showLinks={showLinks} diff --git a/packages/core/src/instance.ts b/packages/core/src/instance.ts index 013b272..8c8b702 100644 --- a/packages/core/src/instance.ts +++ b/packages/core/src/instance.ts @@ -918,7 +918,8 @@ export interface GraphInstance, E = Record; /** * Bounded object export of the pinned model: 'visible' (default) is the - * mask-visible roster with both-endpoint-visible edges; 'accepted' the + * mask-visible roster and edges whose own mask and both endpoints are + * visible; 'accepted' the * full model. Rejects `export-materialization-too-large` past `limit` * (default 100 000 rows) BEFORE allocating — the stream is the remedy. */ @@ -1562,6 +1563,9 @@ export function createGraphInstance, E = Record, string> | undefined; let linkWidth: Accessor, number> | undefined; let layout: LayoutKind = 'force'; + /** An embedded layout restored before readiness freezes its first replay, + * matching a restore performed against an already-mounted engine. */ + let pauseOnReadyAfterRestore = false; // Default = the 'calm' preset: the engine's own defaults keep visible // motion alive for tens of seconds, which reads as jitter on first load. let simulation: SimulationConfig | undefined = resolveSimulation(undefined); @@ -3901,6 +3905,10 @@ export function createGraphInstance, E = Record, E = Record, E = Record 0 || drain.nodesAlpha.length > 0; const edgesAffected = drain.edges.length > 0 || drain.edgesAlpha.length > 0; if (nodesAffected || edgesAffected) { + visibleGen += 1; revisions = { ...prev.revisions }; revisions.scope += 1; revisions.render += 1; if (eng !== null) { nodeAlphaComposer.reset(); // drain unobserved by composers edgeAlphaComposer.reset(); - const buffers: NonNullable = {}; - if (nodesAffected) buffers.pointColor = composeNodeAlphaBuffer(basePointColorBuffer()); + const visDirty: DirtyChannels = { + nodeColor: scaleUsesVisibleDomain(nodeColor), + nodeSize: scaleUsesVisibleDomain(nodeSize), + linkColor: false, + linkWidth: false, + }; + const buffers: NonNullable = + visDirty.nodeColor || visDirty.nodeSize + ? { ...(projectChannelBuffers(visDirty) ?? {}) } + : {}; + if (visDirty.nodeColor || visDirty.nodeSize) filterDiagsChanged = true; + if (nodesAffected && buffers.pointColor === undefined) { + buffers.pointColor = composeNodeAlphaBuffer(basePointColorBuffer()); + } if (edgesAffected) buffers.linkColor = composeEdgeAlphaBuffer(baseLinkColorBuffer()); - eng.commit({ revision: revisions.render, buffers }); + commitToEngine(eng, { revision: revisions.render, buffers }); revisions.appliedRender = eng.appliedRevision(); } } @@ -6257,6 +6282,10 @@ export function createGraphInstance, E = Record, E = Record, E = Record, E = Record, E = Record, E = Record, E = Record, E = Record maskEdgeVisibleAt(linkIndex), }); - const restartAlpha = layout === 'force' ? 1 : null; + const pauseRestoredLayout = pauseOnReadyAfterRestore; + const restartAlpha = layout === 'force' && !pauseRestoredLayout ? 1 : null; const committed = buildAndCommitFullReplay(s, restartAlpha); + pauseOnReadyAfterRestore = false; const restarted = committed && accepted !== null && restartAlpha !== null; // Cosmos holds positions without start (max delta 0.00px over 2.5s), but the pause // is still issued so an INITIALLY-fixed mount reaches the same engine // state as a force→fixed transition, insulating against an engine whose // simulation free-runs by default. - if (layout === 'fixed') eng.pause(); + if (layout === 'fixed' || pauseRestoredLayout) eng.pause(); publish({ status: 'ready', revisions: { ...store.getState().revisions, appliedRender: eng.appliedRevision() }, diagnostics: composeDiagnostics(), - ...(restarted ? { simulationRunning: true } : {}), + ...(restarted ? { simulationRunning: true } : pauseRestoredLayout ? { simulationRunning: false } : {}), }); // highlight remap: a fresh engine gets the surviving interaction @@ -9859,6 +9907,10 @@ export function createGraphInstance, E = Record, E = Record 0; } - function failPendingRestore(code: 'restore-timeout' | 'restore-diverged'): void { + function failPendingRestore( + code: 'restore-timeout' | 'restore-diverged', + options: { problem?: string; rollbackCursor?: boolean } = {}, + ): void { const pending = pendingRestore; if (pending === null) return; pendingRestore = null; clearTimeout(pending.timer); - pending.rollbackCursor?.(); + // Dataset replacement clears both history stacks. Its cancelled walk + // must never move a cursor in the replacement dataset's history. + if (options.rollbackCursor !== false) pending.rollbackCursor?.(); pending.resolve({ status: 'rejected', code, problems: [ - code === 'restore-timeout' + options.problem ?? (code === 'restore-timeout' ? `the host did not reflect the restore intent within ${RESTORE_ACK_TIMEOUT_MS}ms` - : 'the host reflected different values than the intent asked for', + : 'the host reflected different values than the intent asked for'), ], }); } @@ -10408,38 +10465,46 @@ export function createGraphInstance, E = Record 0) { - reconciler.noteEnginePositions(next); - scene = { ...scene, positions: next }; - const prevState = store.getState(); - const nextRevisions: Revisions = { - ...prevState.revisions, - render: prevState.revisions.render + 1, - }; - commitToEngine(eng, { + const next = new Float32Array(scene.positions); + let touched = 0; + for (const [id, x, y] of state.positions) { + const idx = scene.indexById.get(id); + if (idx === undefined) continue; // stale id — ignored per spec + next[2 * idx] = x; + next[2 * idx + 1] = y; + touched++; + } + if (touched > 0) { + // CPU state owns the restored layout even while detached or + // mounting; the next full replay reads this reconciler cache. + reconciler.noteEnginePositions(next); + scene = { ...scene, positions: next }; + labelPositionCache = null; + const prevState = store.getState(); + const nextRevisions: Revisions = { + ...prevState.revisions, + render: prevState.revisions.render + 1, + }; + if (eng !== null) { + const commit: EngineCommit = { revision: nextRevisions.render, structure: { pointCount: scene.count, positions: next, links: scene.links, }, - }); + }; + const syncIndex = structuralPointImageIndex(); + if (syncIndex !== null) commit.resources = { pointImageIndex: syncIndex }; + commitToEngine(eng, commit); nextRevisions.appliedRender = eng.appliedRevision(); eng.pause(); - const patch: Partial = { revisions: nextRevisions }; - if (prevState.simulationRunning) patch.simulationRunning = false; - publish(patch); + } else { + pauseOnReadyAfterRestore = true; } + const patch: Partial = { revisions: nextRevisions }; + if (prevState.simulationRunning) patch.simulationRunning = false; + publish(patch); } } @@ -10728,18 +10793,47 @@ export function createGraphInstance, E = Record | null; + visibleEdgeIds: ReadonlySet | null; } - function captureExportPin(): ExportPin | null { + function captureExportPin(scope: 'visible' | 'accepted'): ExportPin | null { if (accepted === null) return null; let visibleIds: ReadonlySet | null = null; - if (scene !== null) { + let visibleEdgeIds: ReadonlySet | null = null; + if (scope === 'visible' && scene !== null) { const slots = visibleSlotsOf(scene, softMask); const ids = new Set(); for (const i of slots) ids.add(scene.idByIndex[i]!); visibleIds = ids; + const edgeIds = new Set(); + const physicalLinks = scene.groups?.physicalLinkCount ?? scene.linkCount; + for (let k = 0; k < physicalLinks; k++) { + if (!maskEdgeVisibleAt(k)) continue; + const source = scene.idByIndex[scene.links[2 * k]!]!; + const target = scene.idByIndex[scene.links[2 * k + 1]!]!; + if (ids.has(source) && ids.has(target)) edgeIds.add(scene.edgeIdByIndex[k]!); + } + // Parallel grouping replaces physical edges with aggregate scene rows. + // Export their accepted rows, never synthetic keys. An aggregate's mask + // says ANY member passes, so check each underlying edge's hide predicate + // before pinning it; dimmed rows remain visible and exportable. + if (groupRewrite !== null) { + const model = renderModel()!; + for (let j = 0; j < groupRewrite.metaEdges.length; j++) { + if (!maskEdgeVisibleAt(physicalLinks + j)) continue; + for (const k of groupRewrite.metaEdges[j]!.underlying) { + const edge = model.edges[k]!; + if (!ids.has(edge.source) || !ids.has(edge.target)) continue; + if (activeFilterMode === 'hide' && compiledEdgeSelector !== null && !compiledEdgeSelector.test(edge)) { + continue; + } + edgeIds.add(edge.id); + } + } + } + visibleEdgeIds = edgeIds; } - return { accepted, scene, visibleIds }; + return { accepted, scene, visibleIds, visibleEdgeIds }; } /** Visible-node id set under a pin; null = everything visible (no scene). */ @@ -10754,18 +10848,14 @@ export function createGraphInstance, E = Record[]; edges: readonly AcceptedEdge[] }> { - const pin = captureExportPin(); + const pin = captureExportPin(scope); if (pin === null) return { nodes: [], edges: [] }; const rowCount = pinnedRowCount(pin, scope); const limit = opts?.limit ?? 100_000; @@ -10781,16 +10871,14 @@ export function createGraphInstance, E = Record visible.has(n.id)), - edges: pin.accepted.edges.filter( - (e) => visible.has(e.source) && visible.has(e.target), - ), + edges: pin.accepted.edges.filter((e) => pin.visibleEdgeIds!.has(e.id)), }; } function exportDataStream( scope: 'visible' | 'accepted' = 'visible', ): AsyncGenerator { - const pin = captureExportPin(); // EAGER — see ExportPin + const pin = captureExportPin(scope); // EAGER — see ExportPin return (async function* dataRows(): AsyncGenerator { if (pin === null) return; const visible = scope === 'visible' ? pinnedVisibleIds(pin) : null; @@ -10801,7 +10889,7 @@ export function createGraphInstance, E = Record, E = Record { if (destroyed) return; + pauseOnReadyAfterRestore = false; const eng = engineIfReady(); if (eng === null) return; eng.start(); diff --git a/packages/core/src/viewState.ts b/packages/core/src/viewState.ts index 889930f..6b658b8 100644 --- a/packages/core/src/viewState.ts +++ b/packages/core/src/viewState.ts @@ -500,7 +500,8 @@ export type SetViewStateResult = | 'restore-pending' /** The host never reflected the intent within the window. */ | 'restore-timeout' - /** The host reflected DIFFERENT values than the intent asked for. */ + /** The host reflected different values, or dataset replacement / + * destruction invalidated the staged transaction. */ | 'restore-diverged'; problems: readonly string[]; }; diff --git a/packages/core/test/exports.test.ts b/packages/core/test/exports.test.ts index d5e8fd0..1338e41 100644 --- a/packages/core/test/exports.test.ts +++ b/packages/core/test/exports.test.ts @@ -69,6 +69,62 @@ describe('exportImage("png")', () => { }); describe('exportData / exportDataStream', () => { + it.each([false, true])( + 'pins edge masks, counts exportable rows, and preserves accepted edges (parallel grouping: %s)', + async (parallelEdgeGrouping) => { + const { instance } = makeInstance({ fitViewOnFirstData: false }); + try { + instance.applyHostUpdate({ + data: { + ...snap(1, ['a', 'b', 'c']), + edges: [ + { id: 'hidden', source: 'a', target: 'b', attrs: { weight: 1 } }, + { id: 'shown', source: 'a', target: 'b', attrs: { weight: 2 } }, + { id: 'other', source: 'b', target: 'c', attrs: { weight: 1 } }, + ], + }, + parallelEdgeGrouping, + filter: { edges: (edge) => edge.attrs?.weight === 2 }, + }); + const visible = await instance.exportData('visible', { limit: 4 }); + expect(visible.nodes.map((node) => node.id)).toEqual(['a', 'b', 'c']); + expect(visible.edges.map((edge) => edge.id)).toEqual(['shown']); + await expect(instance.exportData('visible', { limit: 3 })).rejects.toMatchObject({ + detail: { code: 'export-materialization-too-large', rowCount: 4, limit: 3 }, + }); + const accepted = await instance.exportData('accepted'); + expect(accepted.edges.map((edge) => edge.id)).toEqual(['hidden', 'shown', 'other']); + + const visibleStream = instance.exportDataStream('visible'); + const acceptedStream = instance.exportDataStream('accepted'); + instance.applyHostUpdate({ filter: { edges: () => false } }); + expect((await instance.exportData('visible', { limit: 3 })).edges).toEqual([]); + // Change the live mask before the first read, then again mid-stream. + const first = await visibleStream.next(); + expect(JSON.parse(first.value!).value.id).toBe('a'); + instance.applyHostUpdate({ filter: null }); + const streamedEdges: string[] = []; + for await (const line of visibleStream) { + const row = JSON.parse(line) as { kind: string; value: { id: string } }; + if (row.kind === 'edge') streamedEdges.push(row.value.id); + } + expect(streamedEdges).toEqual(['shown']); + const acceptedEdges: string[] = []; + for await (const line of acceptedStream) { + const row = JSON.parse(line) as { kind: string; value: { id: string } }; + if (row.kind === 'edge') acceptedEdges.push(row.value.id); + } + expect(acceptedEdges).toEqual(['hidden', 'shown', 'other']); + + instance.applyHostUpdate({ filter: { mode: 'dim', edges: () => false } }); + expect((await instance.exportData('visible')).edges.map((edge) => edge.id)) + .toEqual(['hidden', 'shown', 'other']); + } finally { + instance.destroy(); + } + }, + ); + it('visible scope honors the mask; accepted scope is the full model', async () => { const { instance } = await ready(); instance.hideNodes(['d']); diff --git a/packages/core/test/folds.test.ts b/packages/core/test/folds.test.ts index eb72f90..878f9e8 100644 --- a/packages/core/test/folds.test.ts +++ b/packages/core/test/folds.test.ts @@ -210,19 +210,54 @@ describe('fold ops on the instance', () => { }); it('undo/redo round-trips a fold through the history', async () => { - const { instance } = await ready(); + const { instance, engine } = await ready(); const before = [...drawn(instance)]; instance.foldNode('hub'); expect(drawn(instance)).toEqual(['hub', 'far']); + expect(instance.store.getState().folds.get('hub')).toBe(3); + + let publications = 0; + const unsubscribe = instance.store.subscribe(() => { publications++; }); + const commits = engine.commits.length; instance.undo(); expect(drawn(instance)).toEqual(before); expect(instance.getFold('hub')).toBeNull(); + expect(instance.store.getState().folds.size).toBe(0); + expect(publications).toBe(1); + expect(engine.commits.length - commits).toBe(1); instance.redo(); expect(drawn(instance)).toEqual(['hub', 'far']); expect(instance.getFold('hub')).toEqual({ memberIds: ['s1', 's2', 'other'] }); + expect(instance.store.getState().folds.get('hub')).toBe(3); + expect(publications).toBe(2); + expect(engine.commits.length - commits).toBe(2); + unsubscribe(); + }); + + it('view-state fold restores publish the current count in the same scene update', async () => { + const { instance, engine } = await ready(); + try { + const expanded = instance.getViewState(); + instance.foldNode('hub'); + const folded = instance.getViewState(); + let publications = 0; + const unsubscribe = instance.store.subscribe(() => { publications++; }); + const commits = engine.commits.length; + await instance.setViewState(expanded); + expect(instance.store.getState().folds.size).toBe(0); + expect(publications).toBe(1); + expect(engine.commits.length - commits).toBe(1); + await instance.setViewState(folded); + expect(instance.store.getState().folds.get('hub')).toBe(3); + expect(publications).toBe(2); + expect(engine.commits.length - commits).toBe(2); + unsubscribe(); + } finally { + instance.destroy(); + } }); it('nests under a collapsed group at the instance level', async () => { diff --git a/packages/core/test/recovery.test.ts b/packages/core/test/recovery.test.ts index 2242c41..b2ca689 100644 --- a/packages/core/test/recovery.test.ts +++ b/packages/core/test/recovery.test.ts @@ -92,6 +92,32 @@ describe('context lost', () => { }); describe('context restore', () => { + it('replays positions restored while lost without reheating, then consumes that pending pause', async () => { + const { instance, engine, engines } = await setupReady(); + try { + engine.injectContextLost(); + const commits = engine.commits.length; + await instance.setViewState({ + ...instance.getViewState(), positions: [['a', 30, 40]], + }); + expect(engine.commits).toHaveLength(commits); // CPU-only while lost + expect(instance.isSimulationRunning()).toBe(false); + engine.injectContextRestored(); + expect(engine.commits).toHaveLength(commits + 1); + expect(Array.from(engine.lastStructure!.positions).slice(0, 2)).toEqual([30, 40]); + expect(engine.lastCommit!.restart).toBeUndefined(); + expect(instance.isSimulationRunning()).toBe(false); + expect(callsOf(engine, 'pause').length).toBeGreaterThan(0); + + instance.detach(); + await instance.attach(container); + expect(engines.at(-1)!.lastCommit!.restart).toEqual({ alpha: 1 }); + expect(instance.isSimulationRunning()).toBe(true); + } finally { + instance.destroy(); + } + }); + it('replays the full scene as one commit and re-pushes viewport, selection, pins, positions', async () => { const { instance, engine } = await setupReady(); diff --git a/packages/core/test/undo-redo.test.ts b/packages/core/test/undo-redo.test.ts index f2c8c7d..eb2fbb7 100644 --- a/packages/core/test/undo-redo.test.ts +++ b/packages/core/test/undo-redo.test.ts @@ -35,6 +35,75 @@ const vDim: DimensionSpec = { get: (n) => n.id.length + (n.id.codePointAt(0)! - 96), // a→1, b→2, c→3 (+1 length) — monotonic }; +describe('history restores scope-dependent scales', () => { + it.each(['scope', 'hidden', 'brush'] as const)( + 'undo/redo and view-state restore refresh %s domains, colors, and sizes atomically', + async (action) => { + for (const mounted of [true, false]) { + const h = makeInstance({ fitViewOnFirstData: false }); + const { instance } = h; + try { + if (mounted) await instance.attach(container); + const scope = action === 'scope' ? 'hard-scope' : 'visible'; + instance.applyHostUpdate({ + data: snap(1, ['a', 'b', 'c'], [['a', 'b'], ['b', 'c']]), + nodeColor: { kind: 'sequential', metric: 'degree', range: ['#000', '#fff'], domain: { scope } }, + nodeSize: { kind: 'sequential', metric: 'degree', range: [2, 10], domain: { scope } }, + crossfilter: [{ key: 'id', kind: 'categorical', get: (node) => node.id }], + }); + // DomainPolicy stays with the app; the wire styling intentionally + // omits it. Restore exploration alone to keep testing these scales. + const { styling: _styling, ...saved } = instance.getViewState(); + const engine = h.engines[0]; + const initialColors = engine?.lastBuffer('pointColor')?.slice(); + expect(instance.getScaleInfo('nodeSize')!.domain).toEqual([1, 2]); + if (action === 'scope') instance.applyHostUpdate({ subgraph: { seedIds: ['a', 'c'] } }); + else if (action === 'hidden') instance.hideNodes(['b']); + else await instance.getCrossfilterSession()!.setBrush('id', { excluded: ['b'] }); + expect(instance.getScaleInfo('nodeColor')!.domain).toEqual([1, 1]); + expect(instance.getScaleInfo('nodeSize')!.domain).toEqual([1, 1]); + + let publications = 0; + const unsubscribe = instance.store.subscribe(() => { publications++; }); + const walk = async (direction: 'undo' | 'redo' | 'restore') => { + publications = 0; + const commits = engine?.commits.length ?? 0; + if (direction === 'restore') await instance.setViewState(saved); + else expect(instance[direction]()).toBe(true); + expect(publications).toBe(1); + if (engine !== undefined) expect(engine.commits.length - commits).toBe(1); + }; + await walk('undo'); + expect(instance.getVisibleNodeIds()).toEqual(['a', 'b', 'c']); + expect(instance.getScaleInfo('nodeColor')!.domain).toEqual([1, 2]); + expect(instance.getScaleInfo('nodeSize')!.domain).toEqual([1, 2]); + if (engine !== undefined) { + expect(engine.lastBuffer('pointColor')).toEqual(initialColors); + expect(Array.from(engine.lastBuffer('pointSize')!)).toEqual([2, 10, 2]); + } + await walk('redo'); + expect(instance.getVisibleNodeIds()).toEqual(['a', 'c']); + expect(instance.getScaleInfo('nodeSize')!.domain).toEqual([1, 1]); + if (engine !== undefined) { + expect(engine.lastBuffer('pointSize')![0]).toBe(6); + expect(engine.lastBuffer('pointColor')![0]).toBeCloseTo(128 / 255); + } + await walk('restore'); + expect(instance.getScaleInfo('nodeSize')!.domain).toEqual([1, 2]); + expect(instance.getScaleInfo('nodeColor')!.domain).toEqual([1, 2]); + if (engine !== undefined) { + expect(engine.lastBuffer('pointColor')).toEqual(initialColors); + expect(Array.from(engine.lastBuffer('pointSize')!)).toEqual([2, 10, 2]); + } + unsubscribe(); + } finally { + instance.destroy(); + } + } + }, + ); +}); + describe('selection undo/redo', () => { it('undoes and redoes selection mutations exactly, re-pushing indices', async () => { const { instance, engine } = await readyRig(); diff --git a/packages/core/test/view-state.test.ts b/packages/core/test/view-state.test.ts index 6d7c294..a6655f4 100644 --- a/packages/core/test/view-state.test.ts +++ b/packages/core/test/view-state.test.ts @@ -501,6 +501,66 @@ describe('setViewState', () => { expect(store.revisions.appliedRender).toBe(commit.revision); }); + it.each(['idle', 'mounting', 'detached'] as const)( + 'preserves embedded positions restored while %s through the next engine replay', + async (phase) => { + for (const kind of ['fixed', 'force'] as const) { + const h = makeInstance({ fitViewOnFirstData: false }); + const { instance } = h; + try { + instance.applyHostUpdate({ + data: { + ...snap(1, ['a', 'b']), + nodes: [{ id: 'a', x: 1, y: 2 }, { id: 'b', x: 5, y: 6 }], + }, + layout: kind, + }); + if (phase === 'detached') { + await instance.attach(container); + instance.detach(); + } + const mounting = phase === 'mounting' ? instance.attach(container) : null; + const before = instance.getRevisions(); + let publications = 0; + const unsubscribe = instance.store.subscribe(() => { publications++; }); + const restored = instance.setViewState({ + ...instance.getViewState(), positions: [['a', 30, 40], ['unknown', 90, 100]], + }); + expect(publications).toBe(1); // CPU restore is one publication before readiness + expect(instance.getRevisions().render).toBe(before.render + 1); + unsubscribe(); + await expect(restored).resolves.toEqual({ status: 'applied' }); + expect(await instance.exportLayout()).toEqual(new Map([ + ['a', [30, 40]], ['b', [5, 6]], + ])); + if (mounting !== null) await mounting; + else await instance.attach(container); + const engine = h.engines.at(-1)!; + expect(Array.from(engine.lastStructure!.positions)).toEqual([30, 40, 5, 6]); + expect(engine.lastCommit!.restart).toBeUndefined(); + expect(instance.isSimulationRunning()).toBe(false); + expect(instance.getRevisions().appliedRender).toBe(instance.getRevisions().render); + } finally { + instance.destroy(); + } + } + }, + ); + + it('a new data acceptance after a pre-ready restore can restart force layout', async () => { + const h = makeInstance({ fitViewOnFirstData: false }); + try { + h.instance.applyHostUpdate({ data: snap(1, ['a']) }); + await h.instance.setViewState({ ...h.instance.getViewState(), positions: [['a', 30, 40]] }); + h.instance.applyHostUpdate({ data: snap(2, ['a', 'b']) }); + await h.instance.attach(container); + expect(h.engines[0]!.lastCommit!.restart).toEqual({ alpha: 1 }); + expect(h.instance.isSimulationRunning()).toBe(true); + } finally { + h.instance.destroy(); + } + }); + it('rejects malformed scale shapes through setViewState without applying any lane', async () => { const h = await ready(); const before = h.instance.getRevisions(); @@ -543,6 +603,94 @@ describe('aggregate restore protocol', () => { return h; } + it.each(['snapshot', 'ingest'] as const)( + 'cancels a staged restore when a %s replaces the dataset', + async (source) => { + const h = makeInstance({ fitViewOnFirstData: false }); + const { instance } = h; + const replace = async (datasetKey: string) => { + const session = instance.beginIngest({ + purpose: 'replace', datasetKey, sourceRevision: 1, + baseModelRevision: instance.getRevisions().model, + }); + await session.append({ + sequence: 0, batchId: datasetKey, + nodes: [{ id: datasetKey, attrs: { label: datasetKey } }], edges: [], + }); + await session.commit(); + }; + try { + await instance.attach(container); + if (source === 'snapshot') instance.applyHostUpdate({ data: snap(1, ['old'], [], 'old') }); + else await replace('old'); + instance.applyHostUpdate({ selection: [] }); + instance.on('viewStateRestore', () => {}); + const pending = instance.setViewState({ + ...instance.getViewState(), + selection: { nodeIds: ['old'], edgeIds: [], groupIds: [] }, + subgraph: { seedIds: ['old'] }, + }); + let publications = 0; + const unsubscribe = instance.store.subscribe(() => { publications++; }); + const commits = h.engines[0]!.commits.length; + if (source === 'snapshot') instance.applyHostUpdate({ data: snap(1, ['new'], [], 'new') }); + else await replace('new'); + expect(publications).toBe(1); + expect(h.engines[0]!.commits.length - commits).toBe(1); + unsubscribe(); + await expect(pending).resolves.toMatchObject({ status: 'rejected', code: 'restore-diverged' }); + instance.applyHostUpdate({ selection: ['old'] }); // late acknowledgement + expect(instance.getVisibleNodeIds()).toEqual(['new']); + expect(instance.store.getState().scope).toBeNull(); + expect(instance.store.getState().history).toEqual({ undoDepth: 0, redoDepth: 0 }); + await expect(instance.setViewState(instance.getViewState())).resolves.toEqual({ status: 'applied' }); + } finally { + instance.destroy(); + } + }, + ); + + it('a cancelled history acknowledgement cannot later move the replacement history cursor', async () => { + vi.useFakeTimers(); + const { instance } = makeInstance(); + try { + instance.applyHostUpdate({ data: snap(1, ['old'], [], 'old') }); + instance.selectNodes(['old']); + instance.applyHostUpdate({ selection: ['old'] }); + instance.on('viewStateRestore', () => {}); + expect(instance.undo()).toBe(true); + expect(vi.getTimerCount()).toBe(1); + instance.applyHostUpdate({ data: snap(1, ['new'], [], 'new') }); + expect(vi.getTimerCount()).toBe(0); + instance.hideNodes(['new']); + const before = instance.store.getState().history; + vi.advanceTimersByTime(6000); + expect(instance.store.getState().history).toBe(before); + expect(instance.undo()).toBe(true); + expect(instance.getVisibleNodeIds()).toEqual(['new']); + } finally { + instance.destroy(); + vi.useRealTimers(); + } + }); + + it('destroy settles a staged restore and releases its acknowledgement timer', async () => { + vi.useFakeTimers(); + const h = await controlledRig(); + try { + h.instance.on('viewStateRestore', () => {}); + const pending = h.instance.setViewState({ + ...h.instance.getViewState(), selection: { nodeIds: ['a'], edgeIds: [], groupIds: [] }, + }); + h.instance.destroy(); + await expect(pending).resolves.toMatchObject({ status: 'rejected', code: 'restore-diverged' }); + expect(vi.getTimerCount()).toBe(0); + } finally { + h.instance.destroy(); + vi.useRealTimers(); + } + }); + it('stages, emits ONE intent, applies nothing until the matching reflection', async () => { const h = await controlledRig(); h.instance.hideNodes(['c']); // an internal slice that must ALSO hold diff --git a/packages/core/test/worker-admission.test.ts b/packages/core/test/worker-admission.test.ts index 1373ffe..4aaaf1a 100644 --- a/packages/core/test/worker-admission.test.ts +++ b/packages/core/test/worker-admission.test.ts @@ -78,6 +78,85 @@ async function rig(over: Partial> = { const flush = () => new Promise((r) => setTimeout(r, 0)); +/** Run the real worker runtime, but let the test decide when replies land. */ +function delayedWorkerDouble() { + const double = createWorkerDouble(); + const replies: WorkerEnvelope[] = []; + const shim = { + onmessage: null as ((ev: { data: unknown }) => void) | null, + postMessage(msg: unknown, transfers?: Transferable[]) { + double.post(msg as WorkerEnvelope, (transfers ?? []) as ArrayBuffer[]); + }, + terminate: () => double.terminate(), + }; + double.onReply((reply) => replies.push(reply)); + return { + worker: shim as unknown as Worker, + release() { + expect(replies.length).toBeGreaterThan(0); + for (const reply of replies.splice(0)) shim.onmessage?.({ data: reply }); + }, + }; +} + +describe('worker supersession by ingestion', () => { + it('a newer replace commit survives a late worker reply without detaching rejected data', async () => { + const delayed = delayedWorkerDouble(); + const { instance, engine } = await rig({ + execution: 'worker', + workerFactory: { create: () => delayed.worker }, + }); + try { + const stale = { ...columnarFixture(), bufferOwnership: 'transfer' as const }; + instance.applyHostUpdate({ data: stale }); + const replacement = instance.beginIngest({ + purpose: 'replace', datasetKey: 'new', sourceRevision: 2, baseModelRevision: 0, + }); + await replacement.append({ + sequence: 0, batchId: 'new', nodes: [{ id: 'new', attrs: { label: 'NEW' } }], edges: [], + }); + await replacement.commit(); + expect(instance.getSceneNodeIds()).toEqual(['new']); + const before = instance.store.getState(); + const commits = engine.commits.length; + delayed.release(); + await flush(); + expect(instance.store.getState()).toBe(before); + expect(engine.commits).toHaveLength(commits); + expect(stale.nodes.ids.codes.byteLength).toBeGreaterThan(0); + } finally { + instance.destroy(); + } + }); + + it('a newer overlay publication survives a late worker snapshot', async () => { + const delayed = delayedWorkerDouble(); + const { instance } = await rig({ + execution: 'worker', workerFactory: { create: () => delayed.worker }, + }); + try { + instance.applyHostUpdate({ data: snap(1, ['base']) }); + instance.applyHostUpdate({ data: columnarFixture() }); + const overlay = instance.beginIngest({ + purpose: 'overlay', datasetKey: 'ds', overlayId: 'latest', + baseModelRevision: instance.getRevisions().model, + }); + await overlay.append({ + sequence: 0, batchId: 'extra', nodes: [{ id: 'extra', attrs: { label: 'EXTRA' } }], edges: [], + }); + await overlay.commit(); + const before = instance.store.getState(); + delayed.release(); + await flush(); + expect(instance.store.getState()).toBe(before); + expect(instance.getSceneNodeIds()).toEqual(['base', 'extra']); + expect(instance.getOverlayIds()).toEqual(['latest']); + } finally { + instance.destroy(); + } + }); +}); + describe('async admission (execution: auto + a live lane)', () => { it('rejects non-string entries in every dictionary before worker encoding', async () => { const create = vi.fn(workerFromDouble); diff --git a/packages/data/src/arrow.ts b/packages/data/src/arrow.ts index c653682..cc7b958 100644 --- a/packages/data/src/arrow.ts +++ b/packages/data/src/arrow.ts @@ -13,14 +13,15 @@ * through `getChild` vectors and feed the shared builder; columns come from * the SCHEMA (exact, not sampled). Value normalization for the object lane: * arrow nulls → absent keys, bigints → number when safely representable else - * decimal string (JSON-safe artifacts), everything else passes through. + * decimal string, and LIST/STRUCT values recursively become plain JSON + * containers. Other values pass through. */ import { normalizeJsonSafeValue } from './jsonSafe'; import { buildPrepared } from './builder'; import { EMPTY_ROW_TABLE, type RowTable } from './rowTable'; -import { collectBytes } from './sources'; +import { collectBytes, throwIfAborted } from './sources'; import type { GraphByteSource, GraphColumnMapping, @@ -46,9 +47,12 @@ export const _internals = { importArrow: (): Promise => import('apache-arrow'), }; -async function loadArrowModule(): Promise<{ +interface ArrowModule { tableFromIPC(bytes: Uint8Array): ArrowTableLike; -}> { + materialize(value: object): unknown; +} + +async function loadArrowModule(): Promise { let mod: unknown; try { mod = await _internals.importArrow(); @@ -66,7 +70,18 @@ async function loadArrowModule(): Promise<{ '(unsupported version?)', ); } - return { tableFromIPC: tableFromIPC as (bytes: Uint8Array) => ArrowTableLike }; + const { Vector, StructRow } = mod as typeof import('apache-arrow'); + return { + tableFromIPC: tableFromIPC as (bytes: Uint8Array) => ArrowTableLike, + materialize(value) { + if (value instanceof Vector) return Array.from(value); + // Iterate entries instead of using toJSON/Object.entries: Arrow's + // property access and toJSON can shadow or lose fields such as + // __proto__ and constructor. fromEntries always creates own data. + if (value instanceof StructRow) return Object.fromEntries(value); + return value; + }, + }; } export async function prepareArrowGraphData< @@ -77,8 +92,10 @@ export async function prepareArrowGraphData< mapping: GraphColumnMapping, options: Omit, ): Promise> { - const arrow = await loadArrowModule(); const signal = options.signal; + throwIfAborted(signal); + const arrow = await loadArrowModule(); + throwIfAborted(signal); const deriveNodes = 'deriveNodes' in input && input.deriveNodes === true; const edgeTable = await arrowRowTable(input.edges, arrow, signal); const nodeTable = deriveNodes @@ -105,12 +122,14 @@ function isArrowTable(source: ArrowGraphSource): source is ArrowTableLike { async function arrowRowTable( source: ArrowGraphSource, - arrow: { tableFromIPC(bytes: Uint8Array): ArrowTableLike }, + arrow: ArrowModule, signal: AbortSignal | undefined, ): Promise { + throwIfAborted(signal); const table = isArrowTable(source) ? source : arrow.tableFromIPC(await collectBytes(source as GraphByteSource, signal)); + throwIfAborted(signal); const columns = table.schema.fields.map((f) => f.name); if (columns.length === 0 && table.numRows === 0) return EMPTY_ROW_TABLE; const vectors = columns.map((name) => table.getChild(name)); @@ -118,6 +137,7 @@ async function arrowRowTable( columns, rows: (async function* () { for (let i = 0; i < table.numRows; i++) { + throwIfAborted(signal); const row: Record = {}; for (let j = 0; j < columns.length; j++) { const value = vectors[j]?.get(i); @@ -125,16 +145,16 @@ async function arrowRowTable( // Arrow schemas may legally contain a `__proto__` column. Plain // assignment would invoke the legacy prototype setter and lose it. Object.defineProperty(row, columns[j]!, { - value: normalizeArrowValue(value), + value: normalizeJsonSafeValue(value, arrow.materialize), enumerable: true, writable: true, configurable: true, }); } + throwIfAborted(signal); yield row; } + throwIfAborted(signal); })(), }; } - -const normalizeArrowValue = normalizeJsonSafeValue; diff --git a/packages/data/src/jsonSafe.ts b/packages/data/src/jsonSafe.ts index 492563d..95b0e8a 100644 --- a/packages/data/src/jsonSafe.ts +++ b/packages/data/src/jsonSafe.ts @@ -6,18 +6,29 @@ * Safe integers become numbers, the rest become strings, containers * recurse; '__proto__' keys land as own properties (never the setter). */ -export function normalizeJsonSafeValue(value: unknown): unknown { +export function normalizeJsonSafeValue( + value: unknown, + materialize?: (value: object) => unknown, +): unknown { if (typeof value === 'bigint') { return value >= BigInt(Number.MIN_SAFE_INTEGER) && value <= BigInt(Number.MAX_SAFE_INTEGER) ? Number(value) : value.toString(); } - if (Array.isArray(value)) return value.map(normalizeJsonSafeValue); - if (value !== null && typeof value === 'object' && value.constructor === Object) { + if (value === null || typeof value !== 'object') return value; + if (materialize !== undefined) { + const plain = materialize(value); + if (plain !== value) return normalizeJsonSafeValue(plain, materialize); + } + if (Array.isArray(value)) { + return value.map((item) => normalizeJsonSafeValue(item, materialize)); + } + const prototype = Object.getPrototypeOf(value); + if (prototype === Object.prototype || prototype === null) { const out: Record = {}; for (const [k, v] of Object.entries(value)) { Object.defineProperty(out, k, { - value: normalizeJsonSafeValue(v), + value: normalizeJsonSafeValue(v, materialize), enumerable: true, writable: true, configurable: true, diff --git a/packages/data/test/arrow.test.ts b/packages/data/test/arrow.test.ts index 63f94e5..c4d40e1 100644 --- a/packages/data/test/arrow.test.ts +++ b/packages/data/test/arrow.test.ts @@ -4,10 +4,10 @@ * the missing-optional-dependency error path via the _internals seam. */ -import { tableFromArrays, tableToIPC, type Table } from 'apache-arrow'; -import { afterEach, describe, expect, it } from 'vitest'; +import { Field, Int64, Struct, Table, tableFromArrays, tableToIPC, vectorFromArray } from 'apache-arrow'; +import { afterEach, describe, expect, it, vi } from 'vitest'; import { _internals, prepareArrowGraphData, type ArrowTableLike } from '../src/arrow'; -import { prepareGraphData } from '../src/index'; +import { loadPrepared, prepareGraphData, serializePrepared } from '../src/index'; import { PARITY_EXPECTED_SNAPSHOT, PARITY_EXPECTED_SUMMARIES, @@ -119,6 +119,85 @@ describe('prepareArrowGraphData', () => { expect(prepared.summaries.nodes['views']).toMatchObject({ min: 10, max: 20 }); }); + it.each(['table', 'ipc'] as const)('materializes real nested Arrow containers from %s inputs', async (inputKind) => { + const wide = 2n ** 60n; + const nodes = tableFromArrays({ + id: ['a'], + values: [[1n, wide]], + object: [{ small: 3n, nested: [{ big: wide }] }], + }); + const edges = tableFromArrays({ source: ['a'], target: ['a'], details: [{ weights: [4n] }] }); + const source = (table: Table) => { + if (inputKind === 'table') return asLike(table); + const bytes = tableToIPC(table); + return bytes.buffer.slice(bytes.byteOffset, bytes.byteOffset + bytes.byteLength) as ArrayBuffer; + }; + const prepared = await prepareArrowGraphData( + { nodes: source(nodes), edges: source(edges) }, + { nodes: { id: 'id' }, edges: { source: 'source', target: 'target' } }, + PARITY_OPTIONS, + ); + expect(prepared.snapshot.nodes[0]!.attrs).toEqual({ + values: [1, wide.toString()], + object: { small: 3, nested: [{ big: wide.toString() }] }, + }); + expect(prepared.snapshot.edges[0]!.attrs).toEqual({ details: { weights: [4] } }); + expect(loadPrepared(serializePrepared(prepared))).toEqual(prepared); + }); + + it('preserves special field names inside a real Arrow struct', async () => { + const payload = Object.fromEntries([['__proto__', 1n], ['constructor', 2n], ['toJSON', 3n]]); + const fields = Object.keys(payload).map((name) => new Field(name, new Int64(), false)); + const nodes = new Table({ + id: vectorFromArray(['a']), + payload: vectorFromArray([payload], new Struct(fields)), + }); + const prepared = await prepareArrowGraphData( + { nodes: asLike(nodes), edges: asLike(tableFromArrays({ source: ['a'], target: ['a'] })) }, + { nodes: { id: 'id' }, edges: { source: 'source', target: 'target' } }, + PARITY_OPTIONS, + ); + const actual = prepared.snapshot.nodes[0]!.attrs!['payload'] as Record; + expect(actual).toEqual(Object.fromEntries([['__proto__', 1], ['constructor', 2], ['toJSON', 3]])); + expect(Object.getPrototypeOf(actual)).toBe(Object.prototype); + expect(Object.prototype.hasOwnProperty.call(actual, '__proto__')).toBe(true); + expect(loadPrepared(serializePrepared(prepared))).toEqual(prepared); + }); + + it('rejects an already-aborted signal for existing Arrow tables', async () => { + const controller = new AbortController(); + controller.abort(); + await expect(prepareArrowGraphData( + { nodes: asLike(parityNodesTable()), edges: asLike(parityEdgesTable()) }, + PARITY_MAPPING, + { ...PARITY_OPTIONS, signal: controller.signal }, + )).rejects.toBe(controller.signal.reason); + }); + + it('stops materializing a real Arrow table when cancellation arrives between rows', async () => { + const controller = new AbortController(); + const nodes = parityNodesTable(); + const ids = nodes.getChild('id')!; + const originalGet = ids.get.bind(ids); + const read = vi.spyOn(ids, 'get').mockImplementation((index) => { + // The abort runs after the row yields, while the builder awaits it. + queueMicrotask(() => controller.abort()); + return originalGet(index); + }); + // Preserve the vector instance the spy observes (Table#getChild creates wrappers). + const observedNodes: ArrowTableLike = { + numRows: nodes.numRows, + schema: nodes.schema, + getChild: (name) => name === 'id' ? ids : asLike(nodes).getChild(name), + }; + await expect(prepareArrowGraphData( + { nodes: observedNodes, edges: asLike(parityEdgesTable()) }, + PARITY_MAPPING, + { ...PARITY_OPTIONS, signal: controller.signal }, + )).rejects.toMatchObject({ name: 'AbortError' }); + expect(read).toHaveBeenCalledTimes(1); + }); + it('preserves an Arrow schema column literally named __proto__ as own data', async () => { const table = ( columns: ReadonlyArray, diff --git a/packages/data/test/review-fixes.test.ts b/packages/data/test/review-fixes.test.ts index d6d1315..22036b2 100644 --- a/packages/data/test/review-fixes.test.ts +++ b/packages/data/test/review-fixes.test.ts @@ -361,6 +361,15 @@ describe('async source cleanup across preparation failures', () => { }); describe('deep BigInt normalization in the Arrow/Parquet shared utility', () => { + it('recurses into null-prototype records and records with an own constructor', () => { + const nested = Object.assign(Object.create(null) as Record, { value: 7n }); + const source = Object.fromEntries([['constructor', 1n], ['__proto__', nested]]); + const normalized = normalizeJsonSafeValue(source) as Record; + expect(normalized).toEqual(Object.fromEntries([['constructor', 1], ['__proto__', { value: 7 }]])); + expect(Object.getPrototypeOf(normalized)).toBe(Object.prototype); + expect(Object.prototype.hasOwnProperty.call(normalized, '__proto__')).toBe(true); + }); + it('reaches list/struct leaves; safe integers become numbers, wide ones strings', () => { const nested = normalizeJsonSafeValue({ scalar: 7n, diff --git a/packages/engine-cosmos/README.md b/packages/engine-cosmos/README.md index 971e97d..26005c1 100644 --- a/packages/engine-cosmos/README.md +++ b/packages/engine-cosmos/README.md @@ -39,6 +39,12 @@ Chromium): ## Interaction surface +The `fitView({ maxZoom })` clamp uses the same position snapshot as native +Cosmos fitting: destination positions during a position transition, live GPU +positions otherwise. This uses the exact-pin 3.4.0 runtime helper +`getFitViewPositions` (private in its TypeScript declaration), with shrinking +and expanding transitions covered by the demo's real-engine regression suite. + Coordinate-space facts verified against the exact-pin 3.4.0 dist (see the `CosmosEngine.ts` module header for the exact evidence): diff --git a/packages/engine-cosmos/src/CosmosEngine.ts b/packages/engine-cosmos/src/CosmosEngine.ts index c8ff193..96d6791 100644 --- a/packages/engine-cosmos/src/CosmosEngine.ts +++ b/packages/engine-cosmos/src/CosmosEngine.ts @@ -113,6 +113,17 @@ interface CosmosStoreLike { draggingPointIndex?: number; } +/** + * Exact-pin 3.4.0 camera seam: private in the d.ts, but the runtime helper + * used by native fitView. It reads transition destinations while positions + * animate and live GPU positions otherwise. Using the same helper keeps + * the clamp decision and native fallback on the same coordinate snapshot. + * The real-engine transition regression pins this dependency. + */ +interface CosmosFitViewReader { + getFitViewPositions(): Float32Array; +} + /** * Extracts click modifiers from a cosmos-forwarded event. Duck-typed rather * than `instanceof MouseEvent`: node-safe and cross-realm-safe. Returns @@ -400,8 +411,8 @@ export class CosmosEngine implements GraphEngine { const w = div.clientWidth; const h = div.clientHeight; if (!(w > 0) || !(h > 0)) return false; - const raw = graph.getPointPositions(); - if (raw === undefined || raw.length === 0) return false; + const raw = (graph as unknown as CosmosFitViewReader).getFitViewPositions(); + if (raw.length === 0) return false; let minX = Infinity; let maxX = -Infinity; let minY = Infinity; @@ -409,7 +420,7 @@ export class CosmosEngine implements GraphEngine { for (let i = 0; i + 1 < raw.length; i += 2) { const x = raw[i]!; const y = raw[i + 1]!; - if (Number.isNaN(x) || Number.isNaN(y)) continue; + if (!Number.isFinite(x) || !Number.isFinite(y)) continue; if (x < minX) minX = x; if (x > maxX) maxX = x; if (y < minY) minY = y; diff --git a/packages/engine-cosmos/test/cosmos-engine.test.ts b/packages/engine-cosmos/test/cosmos-engine.test.ts index 0ba8db9..a2a4bb1 100644 --- a/packages/engine-cosmos/test/cosmos-engine.test.ts +++ b/packages/engine-cosmos/test/cosmos-engine.test.ts @@ -98,6 +98,11 @@ const h = vi.hoisted(() => { getZoomLevel(): number { return this.zoomLevel; } pointPositions: number[] = [1, 2, 3, 4]; getPointPositions(): number[] { return this.pointPositions; } + /** Native fit uses destination positions while a position transition runs. */ + fitViewPositions: number[] | null = null; + getFitViewPositions(): Float32Array { + return Float32Array.from(this.fitViewPositions ?? this.pointPositions); + } screenToSpacePosition(p: [number, number]): [number, number] { return [p[0] * 2 - 5, p[1] * 2 - 5]; } @@ -1103,6 +1108,27 @@ describe('CosmosEngine', () => { expect(graph.calls).toEqual([{ method: 'fitView', args: [300, undefined] }]); }); + it('clamps the destination of a shrinking position transition', async () => { + const { engine, graph } = await mountedSized(1000, 1000); + graph.pointPositions = [0, 0, 1000, 1000]; + graph.fitViewPositions = [0, 0, 10, 10]; + engine.fitView({ durationMs: 0, maxZoom: 1.5 }); + expect(graph.calls).toEqual([ + { + method: 'setZoomTransformByPointPositions', + args: [Float32Array.of(5, 5), 0, 1.5, undefined], + }, + ]); + }); + + it('fits the expanding destination instead of clamping around old positions', async () => { + const { engine, graph } = await mountedSized(1000, 1000); + graph.pointPositions = [0, 0, 10, 10]; + graph.fitViewPositions = [0, 0, 1000, 1000]; + engine.fitView({ durationMs: 0, maxZoom: 1.5 }); + expect(graph.calls).toEqual([{ method: 'fitView', args: [0, undefined] }]); + }); + it('a single point clamps at maxZoom (degenerate bbox fits at any zoom)', async () => { const { engine, graph } = await mountedSized(); graph.pointPositions = [10, 20]; diff --git a/packages/omnigraph/src/codegen.ts b/packages/omnigraph/src/codegen.ts index 261f339..75838b9 100644 --- a/packages/omnigraph/src/codegen.ts +++ b/packages/omnigraph/src/codegen.ts @@ -231,8 +231,8 @@ function headerBanner(fingerprintLine: string, extra: string | undefined): strin /** * Generate a self-contained TypeScript module of typed attrs from a parsed * `.pg` schema: one `Props` interface per node type, one - * `EdgeProps` per edge type (suffixed so a node and an edge sharing a - * name cannot collide), `NodeAttrs`/`EdgeAttrs` discriminated unions on the + * `EdgeProps` per edge type (with an additional numeric suffix when + * either preferred name is already used), `NodeAttrs`/`EdgeAttrs` discriminated unions on the * adapter-injected `'orbit:type'` field, and a `TypeMap` lookup interface with * `NodeTypeName`/`EdgeTypeName` key unions. * @@ -245,13 +245,32 @@ export function generateTypes(schema: PgSchema, opts?: GenerateTypesOptions): st : `Schema fingerprint: ${schemaFingerprint(JSON.stringify(schema))} (parsed model — regenerate from .pg source for the source fingerprint).`; const lines = headerBanner(fingerprintLine, opts?.header); + // Reserve every preferred name before allocating collision suffixes, so + // disambiguating one member cannot steal a later member's public name. + const reserved = new Set([ + 'NodeAttrs', 'EdgeAttrs', 'TypeMap', 'NodeTypeName', 'EdgeTypeName', + ...schema.nodes.map((n) => `${n.name}Props`), + ...schema.edges.map((e) => `${e.name}EdgeProps`), + ]); + const used = new Set(); + function allocateName(preferred: string): string { + let name = preferred; + let suffix = 2; + while (used.has(name)) { + do { + name = `${preferred}_${suffix++}`; + } while (reserved.has(name)); + } + used.add(name); + return name; + } const nodeMembers = schema.nodes.map((n) => ({ typeName: n.name, - interfaceName: `${n.name}Props`, + interfaceName: allocateName(`${n.name}Props`), })); const edgeMembers = schema.edges.map((e) => ({ typeName: e.name, - interfaceName: `${e.name}EdgeProps`, + interfaceName: allocateName(`${e.name}EdgeProps`), })); schema.nodes.forEach((n, i) => { diff --git a/packages/omnigraph/src/loader.ts b/packages/omnigraph/src/loader.ts index 02bd4e6..3a30f4b 100644 --- a/packages/omnigraph/src/loader.ts +++ b/packages/omnigraph/src/loader.ts @@ -398,6 +398,7 @@ export function createOmnigraphSource(options: OmnigraphSourceOptions): Omnigrap onProgress?.(buffered.progress); } bufferedBatches.length = 0; + throwIfAborted(signal); await finalSession.commit(); } @@ -446,6 +447,7 @@ export function createOmnigraphSource(options: OmnigraphSourceOptions): Omnigrap // BEFORE commit. throwIfAborted(signal); const headAfter = await newestHead(signal); + throwIfAborted(signal); const counts = { lines, nodes: nodeCount, edges: edgeCount, bytes }; if (driftPolicy === 'accept-warn') { diff --git a/packages/omnigraph/src/normalize.ts b/packages/omnigraph/src/normalize.ts index 060260a..56bd574 100644 --- a/packages/omnigraph/src/normalize.ts +++ b/packages/omnigraph/src/normalize.ts @@ -135,6 +135,9 @@ function formatEpochDay(days: number): string { function normalizeValue(type: PgType | undefined, value: unknown): unknown { if (type === undefined || value === null || value === undefined) return value; + if (typeof type === 'object' && type.kind === 'list' && Array.isArray(value)) { + return value.map((item) => normalizeValue(type.element, item)); + } if (type === 'Date' && typeof value === 'number' && Number.isInteger(value)) { return formatEpochDay(value); } diff --git a/packages/omnigraph/test/codegen.test.ts b/packages/omnigraph/test/codegen.test.ts index f839272..37f9d92 100644 --- a/packages/omnigraph/test/codegen.test.ts +++ b/packages/omnigraph/test/codegen.test.ts @@ -18,6 +18,7 @@ import { mkdirSync, writeFileSync } from 'node:fs'; import { join } from 'node:path'; import { fileURLToPath } from 'node:url'; +import ts from 'typescript'; import { describe, expect, it } from 'vitest'; import { generateTypes, generateTypesFromPgSource } from '../src/codegen'; @@ -237,6 +238,37 @@ describe('demo.pg fixture — committed golden file', () => { }); describe('generateTypes — mapping edge cases', () => { + it('disambiguates node/edge interface collisions in unions and TypeMap and compiles their distinct props', () => { + const source = ` +node Foo { slug: String @key } +node FooEdge { slug: String @key value: String node_only: Bool } +node FooEdgeEdge { slug: String @key value: Bool } +edge Foo: Foo -> FooEdge { value: F64 edge_only: I32 } +edge FooEdge: FooEdge -> FooEdgeEdge { value: String } +`; + const out = generateTypesFromPgSource(source); + const names = [...out.matchAll(/export interface (\w+) \{/g)].map((match) => match[1]); + expect(new Set(names).size).toBe(names.length); + expect(interfaceBody(out, 'FooEdgeProps')).toContain('value: string;'); + expect(interfaceBody(out, 'FooEdgeProps_2')).toContain('value: number;'); + expect(out).toContain("'orbit:type': 'Foo' } & FooEdgeProps_2"); + expect(out).toContain(' Foo: FooEdgeProps_2;'); + expect(out).toContain(' FooEdge: FooEdgeEdgeProps_2;'); + expect(generateTypesFromPgSource(source)).toBe(out); + const dir = fileURLToPath(new URL('./__generated__/', import.meta.url)); + mkdirSync(dir, { recursive: true }); + const path = join(dir, 'collisions.ts'); + writeFileSync(path, out + ` +const node: NodeAttrs = { 'orbit:type': 'FooEdge', id: 'n', slug: 'n', value: 's', node_only: true }; +const edge: EdgeAttrs = { 'orbit:type': 'Foo', id: 'e', value: 1, edge_only: 2 }; +const nodeProps: TypeMap['nodes']['FooEdge'] = node; +const edgeProps: TypeMap['edges']['Foo'] = edge; +void nodeProps; void edgeProps; +`); + const program = ts.createProgram([path], { strict: true, noEmit: true, skipLibCheck: true }); + expect(ts.getPreEmitDiagnostics(program).map((diagnostic) => ts.flattenDiagnosticMessageText(diagnostic.messageText, '\n'))).toEqual([]); + }); + it('maps Vector(n), [T] lists, Bool, Blob, and unknown types', () => { const out = generateTypesFromPgSource(` node Embedding { diff --git a/packages/omnigraph/test/loader.test.ts b/packages/omnigraph/test/loader.test.ts index 9397e00..e122d52 100644 --- a/packages/omnigraph/test/loader.test.ts +++ b/packages/omnigraph/test/loader.test.ts @@ -678,6 +678,33 @@ describe('source-revision drift policy', () => { }); describe('cancellation and error surface', () => { + it.each(['reject', 'retry-once', 'accept-warn'] as const)( + '%s rejects cancellation from the final progress callback before publishing', + async (driftPolicy) => { + const controller = new AbortController(); + let progressCalls = 0; + const { instance, source } = harness(recordedRoutes(), { + driftPolicy, + batchSize: Number.MAX_SAFE_INTEGER, // exactly one, final append + onProgress: () => { + progressCalls++; + controller.abort(); + }, + }); + try { + await expect(source.load(instance, controller.signal)).rejects.toMatchObject({ name: 'AbortError' }); + expect(progressCalls).toBe(1); + const state = instance.store.getState(); + expect(state.nodeCount).toBe(0); + expect(state.edgeCount).toBe(0); + expect(state.revisions.model).toBe(0); + expect(state.revisions.source).toBeNull(); + } finally { + instance.destroy(); + } + }, + ); + it('abort mid-stream aborts the session — no partial graph', async () => { const controller = new AbortController(); const { instance, source } = harness(recordedRoutes(), { diff --git a/packages/omnigraph/test/normalize.test.ts b/packages/omnigraph/test/normalize.test.ts index 416a17a..a16ccf1 100644 --- a/packages/omnigraph/test/normalize.test.ts +++ b/packages/omnigraph/test/normalize.test.ts @@ -128,6 +128,26 @@ describe('normalizeNode identity and value normalization', () => { expect((node.attrs as Record)['at']).toBe('2023-11-14T22:13:20.123Z'); }); + it.each(['node', 'edge'] as const)('normalizes scalar encodings inside %s list properties', (kind) => { + const properties = 'days: [Date] times: [DateTime] payloads: [Blob] missing: [Date]? tags: [String]'; + const schema = parsePgSchema(`node Event { ${properties} } edge Next: Event -> Event { ${properties} }`); + const data = { + id: 'e', days: [0, -1], times: [0, 1000], missing: null, + payloads: ['base64:aGVsbG8=', 's3://bucket/key.bin'], tags: ['one', 'two'], + }; + const normalized = kind === 'node' + ? normalizeNode({ type: 'Event', data }, schema) + : normalizeEdge({ edge: 'Next', from: 'a', to: 'b', data }, schema); + expect(normalized.attrs).toMatchObject({ + days: ['1970-01-01', '1969-12-31'], + times: ['1970-01-01T00:00:00.000Z', '1970-01-01T00:00:01.000Z'], + payloads: ['data:application/octet-stream;base64,aGVsbG8=', 's3://bucket/key.bin'], + missing: null, + tags: ['one', 'two'], + }); + expect(data.days).toEqual([0, -1]); // normalization leaves source rows intact + }); + it('converts inline base64: blobs to data: URIs, leaves URI refs verbatim', () => { const inline = normalizeNode( { type: 'Event', data: { id: 'e', payload: 'base64:aGVsbG8=' } }, diff --git a/packages/react/src/components/Navigator/index.tsx b/packages/react/src/components/Navigator/index.tsx index f7b34d8..029079c 100644 --- a/packages/react/src/components/Navigator/index.tsx +++ b/packages/react/src/components/Navigator/index.tsx @@ -54,8 +54,8 @@ * and invalidated on model change; self-loops are excluded and parallel * edges deduplicate. The navigator caches the resolved neighborhood and the * per-node neighbor counts it has learned in refs keyed by - * `revisions.model`; a model change drops the (now stale) transient root and - * returns to the entry list. Neighbor counts therefore appear on items whose + * `revisions.model` and `revisions.scope`; a scene change drops the stale + * transient root and returns to the entry list. Neighbor counts therefore appear on items whose * neighborhood has been resolved (the current and previously visited roots); * a public O(1) degree read is a post-v0.4 core surface. * @@ -129,8 +129,9 @@ interface NavSection { interface NavRoot { id: NodeId; neighbors: readonly NodeId[]; - /** Model revision the neighborhood was resolved against. */ + /** Model and scope revisions the neighborhood was resolved against. */ model: number; + scope: number; } interface NavStoreSlice { @@ -138,6 +139,7 @@ interface NavStoreSlice { pins: ReadonlyMap; hidden: ReadonlySet; model: number; + scope: number; nodeCount: number; /** Last completed search; null hides the section. */ search: { query: string; results: readonly SearchResult[] } | null; @@ -160,6 +162,7 @@ function useNavStoreSlice(instance: AnyGraphInstance): NavStoreSlice { prev.pins === s.pins && prev.hidden === s.hiddenNodeIds && prev.model === s.revisions.model && + prev.scope === s.revisions.scope && prev.nodeCount === s.nodeCount && prev.search === s.search ) { @@ -170,6 +173,7 @@ function useNavStoreSlice(instance: AnyGraphInstance): NavStoreSlice { pins: s.pins, hidden: s.hiddenNodeIds, model: s.revisions.model, + scope: s.revisions.scope, nodeCount: s.nodeCount, search: s.search, }; @@ -228,7 +232,9 @@ export function GraphNavigator(props: GraphNavigatorProps): ReactElement { void props.keyboardMode; const baseId = useId(); - const [root, setRoot] = useState(null); + const [storedRoot, setRoot] = useState(null); + const root = + storedRoot?.model === slice.model && storedRoot.scope === slice.scope ? storedRoot : null; const [pages, setPages] = useState>({ search: 0, neighbors: 0, @@ -240,20 +246,21 @@ export function GraphNavigator(props: GraphNavigatorProps): ReactElement { /** Stable per-node DOM-id tokens (node ids may contain arbitrary text). */ const tokensRef = useRef>(new Map()); - /** Learned neighbor counts, cached per model revision (see module JSDoc). */ - const degreesRef = useRef<{ model: number; counts: Map }>({ + /** Neighborhoods depend on both the accepted model and scene rewrites. */ + const degreesRef = useRef<{ model: number; scope: number; counts: Map }>({ model: -1, + scope: -1, counts: new Map(), }); const itemsRef = useRef>(new Map()); /** Set by keyboard handlers so the focus effect moves DOM focus once. */ const pendingFocusRef = useRef(false); - // A model change invalidates the resolved neighborhood: drop the transient - // root (back to the entry list) rather than rendering stale rows. + // Drop the transient root after a scope/fold/group or model change. The + // render gate above already prevents one frame of stale neighborhood rows. useEffect(() => { - if (root !== null && root.model !== slice.model) setRoot(null); - }, [root, slice.model]); + if (storedRoot !== null && root === null) setRoot(null); + }, [storedRoot, root]); // release the keyboard ring — but ONLY when no pointer hover owns the // shared channel (blur while the pointer rests on a canvas node must not @@ -267,8 +274,8 @@ export function GraphNavigator(props: GraphNavigatorProps): ReactElement { useEffect(() => () => releaseEmphasis(), [releaseEmphasis]); // Idempotent render-time cache reset (ref only — safe under StrictMode). - if (degreesRef.current.model !== slice.model) { - degreesRef.current = { model: slice.model, counts: new Map() }; + if (degreesRef.current.model !== slice.model || degreesRef.current.scope !== slice.scope) { + degreesRef.current = { model: slice.model, scope: slice.scope, counts: new Map() }; } const accessibility = instance.getAccessibility() as @@ -487,7 +494,7 @@ export function GraphNavigator(props: GraphNavigatorProps): ReactElement { } const neighbors = instance.focusNode(row.nodeId); degreesRef.current.counts.set(row.nodeId, neighbors.length); - setRoot({ id: row.nodeId, neighbors, model: slice.model }); + setRoot({ id: row.nodeId, neighbors, model: slice.model, scope: slice.scope }); setPages((p) => ({ ...p, neighbors: 0 })); setActiveKey(`root:${row.nodeId}`); focusItemSoon(); diff --git a/packages/react/src/components/Table/index.tsx b/packages/react/src/components/Table/index.tsx index 027f4ef..b3c001c 100644 --- a/packages/react/src/components/Table/index.tsx +++ b/packages/react/src/components/Table/index.tsx @@ -3,8 +3,8 @@ * virtualized crossfiltered tabular view. * * Rows are nodes (default) or edges; columns derive from the union of attr - * keys over a BOUNDED sampled prefix (`columnSample`, default 200) plus the - * identity keys, or are picked explicitly via `columns`. Every cell renders + * keys over a BOUNDED sampled prefix before masking (`columnSample`, default + * 200) plus the identity keys, or are picked explicitly via `columns`. Every cell renders * as a TEXT NODE; `renderCell` replaces cell content. * * ## Virtualization @@ -324,15 +324,16 @@ function GraphTableInner( const columnsProp = props.columns; const columns = useMemo(() => { if (columnsProp !== undefined) return normalizeColumns(columnsProp); - const sample = baseRows.slice(0, columnSample).map((row) => { - return row.edge !== undefined - ? (row.edge.attrs as Record | undefined) - : ((instance.getNode(row.key) as GraphNode | undefined)?.attrs as - | Record - | undefined); - }); + void version; + // The table's own brush can hide every row. Discovering columns from + // those masked rows would discard the attributes needed to match the + // next query, trapping a nonempty filter at zero results. + const sample = + mode === 'edges' + ? (edgesProp ?? []).slice(0, columnSample).map((edge) => edge.attrs) + : instance.getSceneNodeIds().slice(0, columnSample).map((id) => instance.getNode(id)?.attrs); return deriveColumns(mode, sample); - }, [instance, baseRows, mode, columnsProp, columnSample]); + }, [instance, version, mode, edgesProp, columnsProp, columnSample]); const valueOf = useCallback( (row: GraphTableRowRef, key: string): unknown => diff --git a/packages/react/src/components/Tooltip/index.tsx b/packages/react/src/components/Tooltip/index.tsx index 1098e35..ff52917 100644 --- a/packages/react/src/components/Tooltip/index.tsx +++ b/packages/react/src/components/Tooltip/index.tsx @@ -153,6 +153,11 @@ function useHoveredTarget(instance: AnyGraphInstance, edges: boolean): HoverKey if (edges && hover.edgeId !== null) return encodeHover('edge', hover.edgeId); return null; }, [instance, edges]); + // The same hovered ID can receive new attrs in a newer snapshot. Refresh + // content on that publication without changing the hover key, so neither + // an already visible card nor its pending delay restarts. + const getModelRevision = useCallback(() => instance.store.getState().revisions.model, [instance]); + useSyncExternalStore(subscribe, getModelRevision, getModelRevision); return useSyncExternalStore(subscribe, getSnapshot, getSnapshot); } diff --git a/packages/react/test/navigator.test.tsx b/packages/react/test/navigator.test.tsx index fa1a09e..da49e68 100644 --- a/packages/react/test/navigator.test.tsx +++ b/packages/react/test/navigator.test.tsx @@ -132,6 +132,17 @@ afterEach(() => { // --- tests ------------------------------------------------------------------ describe(' bounded rendering & paging', () => { + it('updates the entry roster immediately when isolation changes or clears', async () => { + const { instance, view } = await setup({ snapshot: handSnapshot }); + act(() => { + instance.applyHostUpdate({ subgraph: { seedIds: ['b'], hops: 0 } }); + }); + expect(optionTexts(view)).toEqual(['b']); + expect(pageStatus(view).textContent).toBe('All nodes: page 1 of 1 (1 item)'); + act(() => { instance.resetIsolation(); }); + expect(optionTexts(view)).toEqual(['a', 'b', 'c', 'd']); + }); + it('renders a bounded, paged entry list — never one DOM row per entity', async () => { const { view } = await setup({ snapshot: chainSnapshot(200) }); @@ -228,6 +239,28 @@ describe(' roving tabindex & keyboard model', () => { }); describe(' Enter → focusNode neighborhood re-rooting', () => { + it('invalidates the focused neighborhood and learned degrees on scene rewrites', async () => { + const { instance, view } = await setup({ snapshot: handSnapshot }); + fireEvent.keyDown(listbox(view), { key: 'ArrowDown' }); + fireEvent.keyDown(listbox(view), { key: 'Enter' }); + expect(optionTexts(view)).toEqual(['b · 2 neighbors', 'a', 'c']); + + act(() => { + instance.applyHostUpdate({ subgraph: { seedIds: ['b', 'c'], hops: 0 } }); + }); + expect(optionTexts(view)).toEqual(['b', 'c']); + expect(view.queryByText('Focused node')).toBeNull(); + fireEvent.keyDown(listbox(view), { key: 'Enter' }); + expect(optionTexts(view)).toEqual(['b · 1 neighbor', 'c']); + + act(() => { instance.resetIsolation(); }); + expect(optionTexts(view)).toEqual(['a', 'b', 'c', 'd']); + act(() => { instance.foldNode('b', { memberIds: ['a'] }); }); + expect(optionTexts(view)).toEqual(['b', 'c', 'd']); + act(() => { instance.unfoldNode('b'); }); + expect(optionTexts(view)).toEqual(['a', 'b', 'c', 'd']); + }); + it('calls focusNode and re-roots to the 1-hop neighborhood (engine adjacency route)', async () => { const { view, instance, engine } = await setup({ snapshot: handSnapshot }); const focusSpy = vi.spyOn(instance, 'focusNode'); diff --git a/packages/react/test/table.test.tsx b/packages/react/test/table.test.tsx index 66366b3..46c662a 100644 --- a/packages/react/test/table.test.tsx +++ b/packages/react/test/table.test.tsx @@ -259,6 +259,55 @@ describe(' virtualization', () => { // --- bidirectional sync ----------------------------------------------------------- describe(' bidirectional sync', () => { + it('keeps attribute columns searchable after its brush hides every row', async () => { + const { instance, view } = await setup( + { data: syncSnapshot, crossfilter: [tableDim] }, + , + ); + + fireEvent.change(filterInput(view.container), { target: { value: 'no match' } }); + await flush(); + expect(instance.getVisibleNodeIds()).toEqual([]); + expect(rowIds(view.container)).toEqual([]); + expect(headerKeys(view.container)).toEqual(['id', 'name', 'v']); + + // Correcting a typo must work without first clearing the filter. This + // matches an attribute value, not an ID. + fireEvent.change(filterInput(view.container), { target: { value: 'ant' } }); + await flush(); + expect(instance.getVisibleNodeIds()).toEqual(['a']); + expect(rowIds(view.container)).toEqual(['a']); + + fireEvent.change(filterInput(view.container), { target: { value: 'fox' } }); + await flush(); + expect(instance.getVisibleNodeIds()).toEqual(['f']); + expect(rowIds(view.container)).toEqual(['f']); + }); + + it('keeps sampled attributes discoverable when another dimension hides their rows', async () => { + const { instance, view } = await setup( + { + data: { + datasetKey: 'table-heterogeneous', sourceRevision: 1, + nodes: [{ id: 'a', attrs: { first: 'ant', v: 0 } }, { id: 'b', attrs: { second: 'bat', v: 1 } }], + edges: [], + }, + crossfilter: [vDim, tableDim], + }, + , + ); + const session = instance.getCrossfilterSession()!; + await act(async () => { await session.setBrush('v', { min: 0, max: 0 }); }); + fireEvent.change(filterInput(view.container), { target: { value: 'bat' } }); + await flush(); + expect(rowIds(view.container)).toEqual([]); + expect(headerKeys(view.container)).toContain('second'); + + await act(async () => { await session.setBrush('v', null); }); + expect(instance.getVisibleNodeIds()).toEqual(['b']); + expect(rowIds(view.container)).toEqual(['b']); + }); + it('a crossfilter brush narrows the table rows (graph → table)', async () => { const { instance, view } = await setup( { data: syncSnapshot, crossfilter: [vDim, tableDim] }, diff --git a/packages/react/test/tooltip.test.tsx b/packages/react/test/tooltip.test.tsx index 5f6fc09..fc6e68a 100644 --- a/packages/react/test/tooltip.test.tsx +++ b/packages/react/test/tooltip.test.tsx @@ -218,6 +218,36 @@ describe(' delay & visibility', () => { }); describe(' content', () => { + it('refreshes same-ID attributes without restarting an active hover delay', async () => { + vi.useFakeTimers(); + const { instance, engine, view } = await setup(); + act(() => { engine.injectPointHover(0); }); + act(() => { vi.advanceTimersByTime(100); }); + act(() => { + instance.applyHostUpdate({ + data: { + ...defaultSnapshot, sourceRevision: 2, + nodes: [{ id: 'a', attrs: { label: 'Updated', weight: 4 } }, { id: 'b' }], + }, + }); + }); + act(() => { vi.advanceTimersByTime(49); }); + expect(card(view)).toBeNull(); + act(() => { vi.advanceTimersByTime(1); }); + expect(card(view)!.textContent).toBe('Updatedweight4'); + + act(() => { + instance.applyHostUpdate({ + data: { + ...defaultSnapshot, sourceRevision: 3, + nodes: [{ id: 'a', attrs: { label: 'Newest', weight: 5 } }, { id: 'b' }], + }, + }); + }); + expect(instance.store.getState().hover.nodeId).toBe('a'); + expect(card(view)!.textContent).toBe('Newestweight5'); + }); + it('renders label + attr rows as TEXT NODES only (hostile attrs stay literal)', async () => { vi.useFakeTimers(); const payload = '';