From aa8ea819bba3101d577e441c82a96a55a1df3923 Mon Sep 17 00:00:00 2001 From: Matthew Valancy Date: Wed, 28 Oct 2026 20:50:36 -0700 Subject: [PATCH 1/3] docs: retain actual overview zoom budget failure --- ...29-hidden-items-must-not-delay-overview.md | 45 +++++++++++++++++++ .agents/issues/INDEX.md | 1 + 2 files changed, 46 insertions(+) create mode 100644 .agents/inbox/2026-10-29-hidden-items-must-not-delay-overview.md diff --git a/.agents/inbox/2026-10-29-hidden-items-must-not-delay-overview.md b/.agents/inbox/2026-10-29-hidden-items-must-not-delay-overview.md new file mode 100644 index 00000000..a64ab99f --- /dev/null +++ b/.agents/inbox/2026-10-29-hidden-items-must-not-delay-overview.md @@ -0,0 +1,45 @@ +# Hidden item rendering must not delay the visible overview + +## Evidence — 2026-10-29 UTC + +Actual post-merge `dev` revision `17acc25dc98978c64f87a5b0d6fb36b1833cdfa7` +failed the unchanged five-second browser zoom guard in +[CI run 40812241527](https://github.com/GraphDone/GraphDone-Core/actions/runs/40812241527). +The [retained smoke artifact](https://github.com/GraphDone/GraphDone-Core/actions/runs/40812241527/artifacts/12870958655) +contains `test-artifacts/ci-large-symbols/report.json` and its `large-symbols` +assets: `9.json` (detail zoom), `12.json` (overview zoom), `15.zip` (trace). +Recover with `gh run download 40812241527 --repo GraphDone/GraphDone-Core --name smoke-results-17acc25dc98978c64f87a5b0d6fb36b1833cdfa7`. + +The real 2,000-item / 5,000-edge case loaded in 27,088 ms. Its detail zoom took +3,745.5 ms and overview zoom took 5,119.1 ms against `< 5,000 ms`. The case failed +at 125,186 ms, below its 240-second total timeout. The new bus-inspection journey +had completed in 45,516 ms. Only four of thirteen strict reports ran; the later +zoom, final snapshot, no-write and error guards were not reached. Comprehensive +Tests run 40812241568 passed, which does not replace this failed browser guard. + +## Cause and working hypothesis + +`InteractiveGraphVisualization` clears item culling below scale 0.5, then the +zoom-end callback rebuilds item titles/metadata and forces original-edge routing +and label work. Its source groups are CSS-hidden while semantic overview owns +the display. That control flow is verified; the exact share of the 5,119 ms +attributable to hidden work is not yet measured. Previous pre-merge success is +not evidence that the actual integration passed. + +## Corrective rule + +Let the visible presentation own expensive rendering. Skip hidden item graphics +and coalesce terminal camera work, while preserving source simulation, canonical +selection, layout and camera ownership. Refresh actual cards and relationships +when returning to Items, including manual Items selection with no camera event. +Keep the five-second zoom and 45-second load guards and retain the original +failure. Validate correctness with small serial controls locally; measure the +real 2k journey and all final guards on hosted exact-revision CI. + +## Limits and tracking + +This is issue [#235](https://github.com/GraphDone/GraphDone-Core/issues/235), linked +to #231/#193. It does not claim to fix placement/routing spacing (#220/#233/#234), +change source layout ownership, or establish a timing improvement before new +measurements. Initial failed-run fingerprint: +`34c6cda2542943a3e65e610b3c92dd9f94e0f837e7f418f42f3dcb8a617337b5`. diff --git a/.agents/issues/INDEX.md b/.agents/issues/INDEX.md index acfd21ec..c36392ac 100644 --- a/.agents/issues/INDEX.md +++ b/.agents/issues/INDEX.md @@ -2,6 +2,7 @@ | Symptom | Canonical lesson and regression | | --- | --- | +| Overview zoom exceeds five seconds while original item groups are hidden | [Hidden items must not delay overview](../inbox/2026-10-29-hidden-items-must-not-delay-overview.md); issue #235, retained actual-dev 2k failure | | Overview wires cannot be hovered, do not light endpoints, or lose focus when their route disappears | [Interaction geometry and original-edge inspection](../inbox/2026-10-28-overview-wires-need-interaction-geometry.md); `OverviewBusLayer`, semantic LOD and 2k bus inspection journeys | | Your agent mixes setup, existing access and notes; narrow permission text clips, or unavailable access looks empty | [Separate task regions and honest availability](../inbox/2026-10-28-agent-setup-needs-separate-task-regions.md); `agent-connection` browser sequence and `AgentConnection.test.tsx` | | Inspector code remains11–13px, nested previews have microscopic labels, long titles hide content, or inspection clicks enqueue unchanged positions | [Readable preview levels and read-only gestures](../inbox/2026-10-28-inspector-previews-need-readable-levels.md); inspector-readability browser journey and subgraph preview conservation/component cases | From 968e95697514e005739a8d741239cf20dcf6e2d8 Mon Sep 17 00:00:00 2001 From: Matthew Valancy Date: Wed, 28 Oct 2026 20:59:18 -0700 Subject: [PATCH 2/3] fix: defer hidden item graphics and refresh Items on return --- ...29-hidden-items-must-not-delay-overview.md | 36 +++++ .../InteractiveGraphVisualization.tsx | 125 +++++++++------- .../src/components/SemanticGraphOverview.tsx | 4 +- .../src/hooks/useItemPresentation.test.tsx | 138 ++++++++++++++++++ packages/web/src/hooks/useItemPresentation.ts | 56 +++++++ tests/e2e/graph/large-graph-symbols.spec.ts | 15 ++ tests/e2e/graph/semantic-lod.spec.ts | 22 +++ 7 files changed, 342 insertions(+), 54 deletions(-) create mode 100644 packages/web/src/hooks/useItemPresentation.test.tsx create mode 100644 packages/web/src/hooks/useItemPresentation.ts diff --git a/.agents/inbox/2026-10-29-hidden-items-must-not-delay-overview.md b/.agents/inbox/2026-10-29-hidden-items-must-not-delay-overview.md index a64ab99f..e97e9b0d 100644 --- a/.agents/inbox/2026-10-29-hidden-items-must-not-delay-overview.md +++ b/.agents/inbox/2026-10-29-hidden-items-must-not-delay-overview.md @@ -43,3 +43,39 @@ to #231/#193. It does not claim to fix placement/routing spacing (#220/#233/#234 change source layout ownership, or establish a timing improvement before new measurements. Initial failed-run fingerprint: `34c6cda2542943a3e65e610b3c92dd9f94e0f837e7f418f42f3dcb8a617337b5`. + +## Correction and local controls — 2026-10-29 UTC + +`useItemPresentation` coalesces terminal zoom work into one animation frame after +React commits the active presentation. Overview activation/deactivation is +reported in layout effects. Hidden source culling, item opacity and original-edge +routing/label work are skipped. Source simulation and camera ownership are +unchanged; the geometry scale still follows the terminal camera. Returning to +Items clears the item presentation cache and refreshes current cards, opacity, +culling and routes before paint, including manual Items without a camera event. +The scheduler cancels detached work and checks the current simulation. + +Local commands, run serially with Node 22 and one Vitest worker: + +- `npm run test --workspace=@graphdone/web -- --maxWorkers=1 src/hooks/useItemPresentation.test.tsx src/components/SemanticGraphOverview.test.tsx src/components/OverviewBusLayer.test.tsx src/lib/overviewBusHit.test.ts`: **16 passed / 4.86 s**. +- `npm run typecheck --workspace=@graphdone/web`: passed. The first local check + caught a D3 selection union on chained `attr`; individual attribute writes + corrected it before publication. +- Scoped ESLint on both presentation components and the new hook/test: **0 errors**, + 302 existing-style warnings in the large graph component. +- Playwright discovery finds the existing three 301-item desktop/phone and 2k + cases. No local browser or large fixture was launched. + +Two retained local negative controls establish the regressions: replacing the +hidden branch with unconditional item rendering fails two tests (hidden work +appears); suppressing the Items-return refresh also fails two tests (the displayed +card remains at scale 1.2/revision 0 instead of scale 0.2/revision 1). Corrected +source passes both. The hook test includes the actual `SemanticGraphOverview` +mode selector and automatic exit, and asserts current SVG text/path plus visible +host ownership. This small control is not a full-application timing measurement. + +Existing browser journeys now capture actual item-presentation pass/scale +attributes, require zero item passes during the 2k overview zoom, and check real +cards and paths after manual Items selection. These are additional assertions, +not substituted readiness or timing gates. Hosted exact-source acceptance and +independent review remain required before claiming the performance issue solved. diff --git a/packages/web/src/components/InteractiveGraphVisualization.tsx b/packages/web/src/components/InteractiveGraphVisualization.tsx index 5d16d807..866163e5 100644 --- a/packages/web/src/components/InteractiveGraphVisualization.tsx +++ b/packages/web/src/components/InteractiveGraphVisualization.tsx @@ -52,6 +52,7 @@ import { NodeQuickEdit, type QuickEditCommit } from './NodeQuickEdit'; import { WorkItem, WorkItemEdge } from '../types/graph'; import { RelationshipType, RELATIONSHIP_OPTIONS, getRelationshipConfig } from '../constants/workItemConstants'; import { useAdaptiveQuality } from '../hooks/useAdaptiveQuality'; +import { useItemPresentation } from '../hooks/useItemPresentation'; import { nodeLifeClasses, nodeGlowFilter, isActiveStatus, isCompletedStatus, edgeFlowClass } from '../lib/nodeAnimations'; import { mergeSimulationNodes, mergeSimulationEdges } from '../lib/graphDataMerge'; import type { ObstacleBox } from '../lib/edgeLabelLayout'; @@ -1381,12 +1382,15 @@ export function InteractiveGraphVisualization({ onResetLayout, onNodeSelected, i // A graph owns one simulation. Detached simulations otherwise keep mutating // shared node objects after a rebuild and can fight the visible simulation. + const [semanticOverviewActive, setSemanticOverviewActive] = useState(false); + const itemPresentation = useItemPresentation(semanticOverviewActive); const stopActiveSimulation = useCallback(() => { + itemPresentation.clear(); simulationRef.current?.on('tick', null).on('end.persist', null).alphaTarget(0).stop(); simulationRef.current = null; simulationLayoutScopeRef.current = null; delete (window as any).__graphPerf; - }, []); + }, [itemPresentation]); // Initialize empty visualization canvas const initializeEmptyVisualization = useCallback(() => { @@ -3453,7 +3457,7 @@ export function InteractiveGraphVisualization({ onResetLayout, onNodeSelected, i element.select('.long-press-feedback').remove(); }); - const nodePresentationScales = new WeakMap(); + let nodePresentationScales = new WeakMap(); const updateNodePresentationFor = (element: SVGGElement, d: WorkItem) => { const presentedScale = itemPresentationScaleRef.current; if (nodePresentationScales.get(d) === presentedScale) return; @@ -3515,6 +3519,7 @@ export function InteractiveGraphVisualization({ onResetLayout, onNodeSelected, i }; let displayedPriority: typeof priorityProjection | null = null; const updateEdgePositions = (forceAvoid = false) => { + if (itemPresentation.isOverview()) return; if (dotModeRef.current && !forceAvoid) return; const priority = relationshipPriorityRef.current; if (displayedPriority !== priority) { @@ -3705,6 +3710,7 @@ export function InteractiveGraphVisualization({ onResetLayout, onNodeSelected, i applyEdgeDisplays(); }; const applyViewportCull = () => { + if (itemPresentation.isOverview()) return; const svgEl = svg.node(); if (!svgEl) return; const t = d3.zoomTransform(svgEl); @@ -3794,6 +3800,44 @@ export function InteractiveGraphVisualization({ onResetLayout, onNodeSelected, i // Update zoom with LOD updates let labelsShownAtScale = liveScale >= 0.6; + const updateZoomPresentation = (scale: number) => { + if (itemPresentation.isOverview()) return; + // Smooth opacity transitions based on zoom level + const getSmoothedOpacity = (threshold: number, fadeRange: number = 0.2) => { + if (scale >= threshold + fadeRange) return 1; + if (scale <= threshold - fadeRange) return 0; + return (scale - (threshold - fadeRange)) / (fadeRange * 2); + }; + + // Update text opacities with smooth transitions + const activeNodes = nodeElements.filter((d: any) => !cullActive || !d._culled); + activeNodes.selectAll('.node-type-text' as any) + .style('opacity', getSmoothedOpacity(LOD_THRESHOLDS.FAR)); + // Title appears across the working band (not only when zoomed all the way + // in) and is counter-scaled to a legibility floor so it stays readable. The + // group transform is updated live here (the only path that runs on zoom). + activeNodes.selectAll('.node-title-text' as any) + .style('opacity', scale >= 0.3 ? 1 : 0); + activeNodes.selectAll('.node-description-text' as any) + .style('opacity', getSmoothedOpacity(LOD_THRESHOLDS.CLOSE)); + // Edge labels + arrows follow the edge LOD module (zoom-aware, crowd-aware). + if (labelsShownAtScale !== (scale >= 0.6)) { + labelsShownAtScale = scale >= 0.6; + edgeLabelGroups.style('opacity', labelsShownAtScale ? 1 : 0).style('pointer-events', labelsShownAtScale ? 'all' : 'none'); + } + arrowElements.filter((d: any) => edgeDisplay(d) !== 'none') + .style('opacity', (d: any) => + directionStrategy(scale) === 'hidden' ? 0 : arrowVisibility(scale, d?._pTotal || 1).opacity); + linkElements.filter((d: any) => edgeDisplay(d) !== 'none') + .style('opacity', Math.max(0.1, getSmoothedOpacity(LOD_THRESHOLDS.VERY_FAR, 0.1))) + .attr('stroke-width', function(d: any) { + // Don't override stroke width if this edge has an active dialog + if (editingEdge && editingEdge.edge && editingEdge.edge.id === d.id) { + return 10; // Keep thick for active dialog + } + return ITEM_WIRE_WIDTH; + }); + }; zoom.on('zoom', (event) => { // D3 sets sourceEvent only for direct wheel/pointer gestures. Programmatic // fit/restore transforms must remain distinguishable so an automatic fit @@ -3843,51 +3887,11 @@ export function InteractiveGraphVisualization({ onResetLayout, onNodeSelected, i (window as any).updateMiniMapViewport(viewportUpdate); } - // Update LOD based on zoom level - const scale = event.transform.k; - - // Smooth opacity transitions based on zoom level - const getSmoothedOpacity = (threshold: number, fadeRange: number = 0.2) => { - if (scale >= threshold + fadeRange) return 1; - if (scale <= threshold - fadeRange) return 0; - return (scale - (threshold - fadeRange)) / (fadeRange * 2); - }; - - // Update text opacities with smooth transitions - const activeNodes = nodeElements.filter((d: any) => !cullActive || !d._culled); - activeNodes.selectAll('.node-type-text' as any) - .style('opacity', getSmoothedOpacity(LOD_THRESHOLDS.FAR)); - // Title appears across the working band (not only when zoomed all the way - // in) and is counter-scaled to a legibility floor so it stays readable. The - // group transform is updated live here (the only path that runs on zoom). - activeNodes.selectAll('.node-title-text' as any) - .style('opacity', scale >= 0.3 ? 1 : 0); - activeNodes.selectAll('.node-description-text' as any) - .style('opacity', getSmoothedOpacity(LOD_THRESHOLDS.CLOSE)); - // Edge labels + arrows follow the edge LOD module (zoom-aware, crowd-aware). - if (labelsShownAtScale !== (scale >= 0.6)) { - labelsShownAtScale = scale >= 0.6; - edgeLabelGroups.style('opacity', labelsShownAtScale ? 1 : 0).style('pointer-events', labelsShownAtScale ? 'all' : 'none'); - } - arrowElements.filter((d: any) => edgeDisplay(d) !== 'none') - .style('opacity', (d: any) => - directionStrategy(scale) === 'hidden' ? 0 : arrowVisibility(scale, d?._pTotal || 1).opacity); - linkElements.filter((d: any) => edgeDisplay(d) !== 'none') - .style('opacity', Math.max(0.1, getSmoothedOpacity(LOD_THRESHOLDS.VERY_FAR, 0.1))) - .attr('stroke-width', function(d: any) { - // Don't override stroke width if this edge has an active dialog - if (editingEdge && editingEdge.edge && editingEdge.edge.id === d.id) { - return 10; // Keep thick for active dialog - } - return ITEM_WIRE_WIDTH; - }); + updateZoomPresentation(event.transform.k); }); - zoom.on('end.labels', event => { - if (simulationRef.current !== simulation) return; - itemPresentationScaleRef.current = event.transform.k; - updateNodePresentation(); - updateEdgePositions(true); + zoom.on('end.labels', () => { + if (simulationRef.current === simulation) itemPresentation.schedule(); }); // Configure simulation for stability. Start hot only if there are @@ -3931,10 +3935,28 @@ export function InteractiveGraphVisualization({ onResetLayout, onNodeSelected, i // Fully-pinned graphs don't tick, so without this their labels would stay at // the default midpoint and could overlap → clean starting positions need it. runLabelAvoidanceRef.current = () => updateEdgePositions(true); - itemPresentationScaleRef.current = svg.node() ? d3.zoomTransform(svg.node()!).k : 1; - if (cullEnabled) applyViewportCull(); - updateNodePresentation(); - updateEdgePositions(true); + let itemPresentationPasses = 0; + itemPresentation.bind({ + isCurrent: () => simulationRef.current === simulation, + renderItems: resuming => { + if (resuming) nodePresentationScales = new WeakMap(); + const scale = d3.zoomTransform(svg.node()!).k; + itemPresentationScaleRef.current = scale; + if (cullEnabled) applyViewportCull(); + updateNodePresentation(); + updateZoomPresentation(scale); + updateEdgePositions(true); + g.attr('data-presentation-kind', 'items'); + g.attr('data-item-presentation-scale', scale); + g.attr('data-item-presentation-passes', ++itemPresentationPasses); + }, + presentOverview: () => { + itemPresentationScaleRef.current = d3.zoomTransform(svg.node()!).k; + g.attr('data-presentation-kind', 'overview'); + g.attr('data-presentation-scale', d3.zoomTransform(svg.node()!).k); + }, + }); + itemPresentation.refresh(); setTimeout(() => { if (simulationRef.current === simulation) updateEdgePositions(true); }, 500); }, [nodes, validatedEdges, handleNodeClick, initializeEmptyVisualization]); // Include handleNodeClick to get fresh connection state @@ -3949,10 +3971,9 @@ export function InteractiveGraphVisualization({ onResetLayout, onNodeSelected, i const simulationRef = useRef | null>(null); useLayoutEffect(() => () => stopActiveSimulation(), [layoutScope, stopActiveSimulation]); const zoomBehaviorRef = useRef | null>(null); - const [semanticOverviewActive, setSemanticOverviewActive] = useState(false); useLayoutEffect(() => { - if (!semanticOverviewActive) runLabelAvoidanceRef.current?.(); - }, [priorityProjection, semanticOverviewActive, itemRelationshipScope]); + itemPresentation.refresh(); + }, [priorityProjection, itemRelationshipScope, itemPresentation]); const handleOverviewActive = useCallback((active: boolean) => { setSemanticOverviewActive(active); if (active) setSelectedNode(null); diff --git a/packages/web/src/components/SemanticGraphOverview.tsx b/packages/web/src/components/SemanticGraphOverview.tsx index 82c7ba08..d9273cd7 100644 --- a/packages/web/src/components/SemanticGraphOverview.tsx +++ b/packages/web/src/components/SemanticGraphOverview.tsx @@ -231,11 +231,11 @@ export function SemanticGraphOverview({ : null, [traceProjection] ); - useEffect(() => { + useLayoutEffect(() => { setAutomatic(previous => overviewAtScale(previous, camera.scale)); setAutomaticDepth(previous => semanticDepthAtScale(previous, camera.scale)); }, [camera.scale]); - useEffect(() => { + useLayoutEffect(() => { onActiveChange(active); return () => onActiveChange(false); }, [active, onActiveChange]); diff --git a/packages/web/src/hooks/useItemPresentation.test.tsx b/packages/web/src/hooks/useItemPresentation.test.tsx new file mode 100644 index 00000000..c2c42852 --- /dev/null +++ b/packages/web/src/hooks/useItemPresentation.test.tsx @@ -0,0 +1,138 @@ +import { useLayoutEffect, useRef, useState } from 'react'; +import { act, cleanup, fireEvent, render, renderHook, screen } from '@testing-library/react'; +import { afterEach, beforeEach, expect, it, vi } from 'vitest'; +import { SemanticGraphOverview } from '../components/SemanticGraphOverview'; +import { useItemPresentation, type ItemPresentationRenderer } from './useItemPresentation'; + +const frames = new Map void>(); +let nextFrame = 0; +const flushFrames = () => act(() => { + const pending = [...frames.values()]; + frames.clear(); + pending.forEach(callback => callback()); +}); +beforeEach(() => { + frames.clear(); + vi.stubGlobal('requestAnimationFrame', (callback: () => void) => { + frames.set(++nextFrame, callback); + return nextFrame; + }); + vi.stubGlobal('cancelAnimationFrame', (id: number) => frames.delete(id)); + vi.stubGlobal('ResizeObserver', class { + constructor(private callback: (entries: unknown[]) => void) {} + observe() { this.callback([{ contentRect: { width: 1440, height: 900 } }]); } + disconnect() {} + }); +}); +afterEach(() => { cleanup(); vi.unstubAllGlobals(); }); +const renderer = (): ItemPresentationRenderer => ({ + isCurrent: () => true, + renderItems: vi.fn(), + presentOverview: vi.fn(), +}); + +it('coalesces terminal zoom work after overview ownership commits', () => { + const { result, rerender } = renderHook(({ active }) => useItemPresentation(active), { initialProps: { active: false } }); + const draw = renderer(); + result.current.bind(draw); + result.current.schedule(); + result.current.schedule(); + expect(frames.size).toBe(1); + rerender({ active: true }); + flushFrames(); + expect(draw.renderItems).not.toHaveBeenCalled(); + expect(draw.presentOverview).toHaveBeenCalledTimes(1); + result.current.schedule(); + flushFrames(); + expect(draw.renderItems).not.toHaveBeenCalled(); + expect(draw.presentOverview).toHaveBeenCalledTimes(2); + rerender({ active: false }); + expect(draw.renderItems).toHaveBeenCalledTimes(1); + expect(draw.renderItems).toHaveBeenLastCalledWith(true); + result.current.refresh(); + expect(draw.renderItems).toHaveBeenLastCalledWith(false); +}); + +it('cancels detached work and refuses a renderer whose simulation is no longer current', () => { + const { result, unmount } = renderHook(() => useItemPresentation(false)); + const old = renderer(), next = renderer(); + result.current.bind(old); + result.current.schedule(); + result.current.bind(next); + flushFrames(); + expect(old.renderItems).not.toHaveBeenCalled(); + expect(next.renderItems).not.toHaveBeenCalled(); + next.isCurrent = () => false; + result.current.schedule(); + flushFrames(); + expect(next.renderItems).not.toHaveBeenCalled(); + next.isCurrent = () => true; + result.current.schedule(); + unmount(); + expect(frames.size).toBe(0); + flushFrames(); + expect(next.renderItems).not.toHaveBeenCalled(); +}); + +const items = [{ id: 'a', title: 'Alpha', type: 'TASK', status: 'ACTIVE' }, { id: 'b', title: 'Beta', type: 'TASK', status: 'ACTIVE' }]; +const edges = [{ id: 'ab', source: 'a', target: 'b', type: 'BLOCKS' }]; +const positions = () => [{ id: 'a', x: 0, y: 0 }, { id: 'b', x: 100, y: 0 }]; +const noop = () => {}; +let zoom: (scale: number) => void; +let reviseEdges: () => void; +const drawn: { scale: number; revision: number; overview: string | undefined }[] = []; +function Fixture() { + const [scale, setScale] = useState(1.2); + const [active, setActive] = useState(false); + const [revision, setRevision] = useState(0); + const current = useRef({ scale, revision }); + current.current = { scale, revision }; + const host = useRef(null); + const presentation = useItemPresentation(active); + zoom = next => { setScale(next); presentation.schedule(); }; + reviseEdges = () => setRevision(value => value + 1); + useLayoutEffect(() => { + presentation.bind({ + isCurrent: () => !!host.current, + renderItems: () => { + const value = current.current; + drawn.push({ ...value, overview: host.current?.dataset.overview }); + host.current?.querySelector('text')?.replaceChildren(`Card ${value.scale}`); + host.current?.querySelector('path')?.setAttribute('d', `M0,0 L${value.scale * 100},${value.revision}`); + }, + presentOverview: noop, + }); + presentation.refresh(); + }, [presentation]); + useLayoutEffect(() => presentation.refresh(), [revision, presentation]); + return
+ + +
; +} + +it('refreshes cards and current relationships before paint on automatic exit and manual Items without another zoom', () => { + drawn.length = 0; + const { container } = render(); + act(() => zoom(0.4)); + flushFrames(); + const beforeHidden = drawn.length; + act(() => { zoom(0.2); reviseEdges(); }); + flushFrames(); + expect(drawn).toHaveLength(beforeHidden); + fireEvent.change(screen.getByRole('combobox', { name: 'Graph detail' }), { target: { value: 'items' } }); + expect(drawn.at(-1)).toEqual({ scale: 0.2, revision: 1, overview: undefined }); + expect(container.querySelector('svg > text')?.textContent).toBe('Card 0.2'); + expect(container.querySelector('svg > path')?.getAttribute('d')).toBe('M0,0 L20,1'); + fireEvent.change(screen.getByRole('combobox', { name: 'Graph detail' }), { target: { value: 'auto' } }); + act(() => { zoom(0.1); reviseEdges(); }); + flushFrames(); + const hiddenAgain = drawn.length; + act(() => zoom(1.2)); + expect(drawn.length).toBe(hiddenAgain + 1); + expect(drawn.at(-1)).toEqual({ scale: 1.2, revision: 2, overview: undefined }); + flushFrames(); + expect(drawn.length).toBe(hiddenAgain + 1); + expect(container.querySelector('svg > path')?.getAttribute('d')).toBe('M0,0 L120,2'); +}); diff --git a/packages/web/src/hooks/useItemPresentation.ts b/packages/web/src/hooks/useItemPresentation.ts new file mode 100644 index 00000000..bf7864e2 --- /dev/null +++ b/packages/web/src/hooks/useItemPresentation.ts @@ -0,0 +1,56 @@ +import { useCallback, useLayoutEffect, useMemo, useRef } from 'react'; + +export interface ItemPresentationRenderer { + isCurrent: () => boolean; + renderItems: (resuming: boolean) => void; + presentOverview: () => void; +} + +export function useItemPresentation(overviewActive: boolean) { + const renderer = useRef(null); + const frame = useRef(null); + const overview = useRef(overviewActive); + const hidden = useRef(false); + const cancel = useCallback(() => { + if (frame.current !== null) cancelAnimationFrame(frame.current); + frame.current = null; + }, []); + const refresh = useCallback(() => { + cancel(); + const current = renderer.current; + if (!current?.isCurrent()) return; + if (overview.current) { + hidden.current = true; + current.presentOverview(); + } else { + current.renderItems(hidden.current); + hidden.current = false; + } + }, [cancel]); + const clear = useCallback(() => { + cancel(); + renderer.current = null; + hidden.current = false; + }, [cancel]); + const bind = useCallback((next: ItemPresentationRenderer) => { + cancel(); + renderer.current = next; + hidden.current = false; + }, [cancel]); + const schedule = useCallback(() => { + cancel(); + frame.current = requestAnimationFrame(() => { + frame.current = null; + refresh(); + }); + }, [cancel, refresh]); + useLayoutEffect(() => { + overview.current = overviewActive; + refresh(); + }, [overviewActive, refresh]); + useLayoutEffect(() => clear, [clear]); + return useMemo(() => ({ + bind, clear, refresh, schedule, + isOverview: () => overview.current, + }), [bind, clear, refresh, schedule]); +} diff --git a/tests/e2e/graph/large-graph-symbols.spec.ts b/tests/e2e/graph/large-graph-symbols.spec.ts index 8a0d7330..1dd739d7 100644 --- a/tests/e2e/graph/large-graph-symbols.spec.ts +++ b/tests/e2e/graph/large-graph-symbols.spec.ts @@ -178,7 +178,13 @@ test('2000 loaded items and 5000 relationships survive overview and detail zoom paintedCards?: number; labels?: number; }[] = []; + const presentation = () => page.locator('.main-graph-group').evaluate(element => ({ + kind: element.getAttribute('data-presentation-kind'), + itemScale: Number(element.getAttribute('data-item-presentation-scale')), + passes: Number(element.getAttribute('data-item-presentation-passes')), + })); for (const scale of [1.2, 0.1, 1.2]) { + const priorPresentation = await presentation(); const start = Date.now(); const elapsed = await page.evaluate(measureGraphZoom, { x: target.positionX, @@ -186,6 +192,7 @@ test('2000 loaded items and 5000 relationships survive overview and detail zoom scale, }); const wallMs = Date.now() - start; + const settledPresentation = await presentation(); const timingPath = info.outputPath(`zoom-${timings.length}-timing.json`); await writeFile( timingPath, @@ -196,6 +203,8 @@ test('2000 loaded items and 5000 relationships survive overview and detail zoom scale, elapsed, wallMs, + priorPresentation, + settledPresentation, }) ); await info.attach(`zoom-${timings.length}-timing.json`, { @@ -207,6 +216,9 @@ test('2000 loaded items and 5000 relationships survive overview and detail zoom 'Zoom must finish within five seconds on the CI runner' ).toBeLessThan(5000); if (scale > 1) { + expect(settledPresentation.kind).toBe('items'); + expect(settledPresentation.itemScale).toBe(scale); + expect(settledPresentation.passes).toBeGreaterThan(priorPresentation.passes); const geometry = await inspectItemSymbols( page, info, @@ -239,6 +251,9 @@ test('2000 loaded items and 5000 relationships survive overview and detail zoom labels: geometry.labels.length, }); } else { + expect(settledPresentation.kind).toBe('overview'); + expect(settledPresentation.itemScale).toBe(priorPresentation.itemScale); + expect(settledPresentation.passes).toBe(priorPresentation.passes); await inspectSchematicOverview( overview, info, diff --git a/tests/e2e/graph/semantic-lod.spec.ts b/tests/e2e/graph/semantic-lod.spec.ts index 5e0279b9..fd12b60b 100644 --- a/tests/e2e/graph/semantic-lod.spec.ts +++ b/tests/e2e/graph/semantic-lod.spec.ts @@ -1,3 +1,4 @@ +import { writeFile } from 'node:fs/promises'; import { test as base, expect, type CDPSession } from '@playwright/test'; import { login, TEST_USERS } from '../../lib/auth'; import { gql } from '../../lib/graphRequest'; @@ -245,7 +246,28 @@ for (const width of [1440, 390]) return target.__lodOriginalViewportUpdate?.(...args); }; }, seeded.nodeIds[8]); + const itemPresentationBefore = await svg.locator('.main-graph-group').evaluate(element => ({ + passes: Number(element.getAttribute('data-item-presentation-passes')), + scale: Number(element.getAttribute('data-item-presentation-scale')), + })); await overview.getByRole('combobox', { name: 'Graph detail' }).selectOption('items'); + await expect(svg.locator('.main-graph-group')).toHaveAttribute('data-presentation-kind', 'items'); + await expect(svg.locator('.main-graph-group')).toHaveAttribute('data-item-presentation-scale', String(fixedCamera.k)); + expect(await svg.locator('.main-graph-group').getAttribute('data-item-presentation-passes')).toBe(String(itemPresentationBefore.passes + 1)); + const returnedItems = await svg.locator('.main-graph-group').evaluate(element => ({ + cards: Array.from(element.querySelectorAll('.node')).filter(node => getComputedStyle(node).display !== 'none').map(node => ({ + detail: node.getAttribute('data-detail'), + title: node.querySelector('.node-title-text')?.textContent, + })), + edges: Array.from(element.querySelectorAll('.edges-group .edge')).filter(edge => getComputedStyle(edge).display !== 'none').map(edge => edge.getAttribute('d')), + })); + expect(returnedItems.cards.length).toBeGreaterThan(0); + expect(returnedItems.cards.every(card => card.detail === 'compact' && !!card.title)).toBe(true); + expect(returnedItems.edges.length).toBeGreaterThan(0); + expect(returnedItems.edges.every(path => !!path && !/NaN|undefined/.test(path))).toBe(true); + const presentationPath = info.outputPath('manual-items-presentation.json'); + await writeFile(presentationPath, JSON.stringify({ itemPresentationBefore, fixedCamera, returnedItems })); + await info.attach('manual-items-presentation.json', { path: presentationPath, contentType: 'application/json' }); await expect(overview).toHaveAttribute('data-active', 'false'); await expect.poll(() => svg.locator('.main-graph-group .node').evaluateAll((elements, id) => { const node = elements.find(element => (element as Element & { __data__?: { id: string } }).__data__?.id === id)!; From cc7bcb9e1fe4aff660004fcbe0472f14d6d744d2 Mon Sep 17 00:00:00 2001 From: Matthew Valancy Date: Wed, 28 Oct 2026 21:06:15 -0700 Subject: [PATCH 3/3] fix: establish item routes before restoring arrow paint --- ...-29-hidden-items-must-not-delay-overview.md | 15 +++++++++++++++ .../InteractiveGraphVisualization.tsx | 2 +- tests/e2e/graph/semantic-lod.spec.ts | 18 ++++++++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/.agents/inbox/2026-10-29-hidden-items-must-not-delay-overview.md b/.agents/inbox/2026-10-29-hidden-items-must-not-delay-overview.md index e97e9b0d..48a82301 100644 --- a/.agents/inbox/2026-10-29-hidden-items-must-not-delay-overview.md +++ b/.agents/inbox/2026-10-29-hidden-items-must-not-delay-overview.md @@ -79,3 +79,18 @@ attributes, require zero item passes during the 2k overview zoom, and check real cards and paths after manual Items selection. These are additional assertions, not substituted readiness or timing gates. Hosted exact-source acceptance and independent review remain required before claiming the performance issue solved. + +## Review correction: route before restoring arrow paint + +Author self-audit and the independent reviewer identified a return-order problem +in the initial implementation (`968e956`). Arrows created during hidden overview +start at opacity zero. Applying opacity before publishing routes filters those +arrows out; newly represented priority edges have the same problem. A small +D3/JSDOM probe extracted the actual `updateZoomPresentation` callback and used +actual `edgeLOD` functions: old order retained opacity **0**, while route-first +produced opacity **1** at scale 1.2. This is a demonstrated rendering defect, not +a timing inference. The return now establishes routes before restoring opacity. +The existing desktop/phone journey additionally changes priority while hidden, +returns through the actual Items selector, and records/asserts current arrow +transforms and full opacity. Its JSON and screenshots are retained. Fresh final +source CI is required; the earlier `968e956` run is not final acceptance. diff --git a/packages/web/src/components/InteractiveGraphVisualization.tsx b/packages/web/src/components/InteractiveGraphVisualization.tsx index 866163e5..c36d419d 100644 --- a/packages/web/src/components/InteractiveGraphVisualization.tsx +++ b/packages/web/src/components/InteractiveGraphVisualization.tsx @@ -3944,8 +3944,8 @@ export function InteractiveGraphVisualization({ onResetLayout, onNodeSelected, i itemPresentationScaleRef.current = scale; if (cullEnabled) applyViewportCull(); updateNodePresentation(); - updateZoomPresentation(scale); updateEdgePositions(true); + updateZoomPresentation(scale); g.attr('data-presentation-kind', 'items'); g.attr('data-item-presentation-scale', scale); g.attr('data-item-presentation-passes', ++itemPresentationPasses); diff --git a/tests/e2e/graph/semantic-lod.spec.ts b/tests/e2e/graph/semantic-lod.spec.ts index fd12b60b..51db99ad 100644 --- a/tests/e2e/graph/semantic-lod.spec.ts +++ b/tests/e2e/graph/semantic-lod.spec.ts @@ -506,6 +506,24 @@ for (const width of [1440, 390]) 1 ); expect(await overview.locator('defs filter').count()).toBe(0); + const priorityControl = page.getByRole('combobox', { name: 'Relationship priority', exact: true }); + const savedPriority = await priorityControl.inputValue(); + await priorityControl.selectOption('all'); + await overview.getByRole('combobox', { name: 'Graph detail' }).selectOption('items'); + await expect(svg.locator('.main-graph-group')).toHaveAttribute('data-presentation-kind', 'items'); + const itemArrows = await svg.locator('.arrows-group .arrow').evaluateAll(elements => elements + .filter(element => getComputedStyle(element).display !== 'none') + .map(element => ({ id: element.getAttribute('data-edge-id'), opacity: Number(getComputedStyle(element).opacity), transform: element.getAttribute('transform') }))); + expect(itemArrows.length).toBeGreaterThan(0); + expect(itemArrows.every(arrow => arrow.opacity >= 0.99 && !!arrow.transform && !/NaN|undefined/.test(arrow.transform))).toBe(true); + const arrowsPath = info.outputPath('manual-items-priority-arrows.json'); + await writeFile(arrowsPath, JSON.stringify({ savedPriority, itemArrows })); + await info.attach('manual-items-priority-arrows.json', { path: arrowsPath, contentType: 'application/json' }); + const arrowsScreenshot = info.outputPath('manual-items-priority-arrows.png'); + await page.screenshot({ path: arrowsScreenshot }); + await info.attach('manual-items-priority-arrows.png', { path: arrowsScreenshot, contentType: 'image/png' }); + await overview.getByRole('combobox', { name: 'Graph detail' }).selectOption('overview'); + await priorityControl.selectOption(savedPriority); const inspectFirst = async () => { await overview .getByRole('button', { name: '8 groups · 301 items' })