diff --git a/explorer/src/workspaces/GraphWorkspace/GraphWorkspace.tsx b/explorer/src/workspaces/GraphWorkspace/GraphWorkspace.tsx index db35bce9..ef1bcfd6 100644 --- a/explorer/src/workspaces/GraphWorkspace/GraphWorkspace.tsx +++ b/explorer/src/workspaces/GraphWorkspace/GraphWorkspace.tsx @@ -1119,6 +1119,20 @@ export function GraphWorkspace({ externalFocusNodeId, externalFocusToken }: Grap const [activeNodeCount, setActiveNodeCount] = useState(null); const [temporalBounds, setTemporalBounds] = useState(null); const [scrubberTime, setScrubberTime] = useState(null); + // Tracks the millisecond value of the last time setScrubberTime was called, + // so the TimelinePanel onTimeChange callback can skip redundant updates when + // React 18 concurrent mode re-runs the effect with a new Date object for the + // same timestamp (issue #830: redundant setScrubberTime calls → temporalState + // churn → diagnostics effect loop in dev mode). + const lastScrubberMsRef = useRef(null); + const onTimeChange = useCallback((time: Date) => { + const ms = time.getTime(); + if (ms === lastScrubberMsRef.current) { + return; + } + lastScrubberMsRef.current = ms; + setScrubberTime(time); + }, []); const [loadingProgress, setLoadingProgress] = useState(null); const [pluginPanelState, setPluginPanelState] = useState>({ "effects-panel": false, @@ -1129,6 +1143,9 @@ export function GraphWorkspace({ externalFocusNodeId, externalFocusToken }: Grap const [pluginRuntimeVersion, setPluginRuntimeVersion] = useState(0); const [effectsState, setEffectsState] = useState(DEFAULT_EFFECTS_STATE); const [graphDiagnosticsState, setGraphDiagnosticsState] = useState(null); + // Tracks the last accepted diagnostics outside React's state cycle, allowing + // handleDiagnosticsChange to compare synchronously before calling setState. + const lastDiagnosticsRef = useRef(null); const [graphAnalyticsState, setGraphAnalyticsState] = useState(null); const [loadedPlugins, setLoadedPlugins] = useState>({}); @@ -2074,7 +2091,7 @@ export function GraphWorkspace({ externalFocusNodeId, externalFocusToken }: Grap title: "Toggle temporal context panel", order: 40, load: loadTemporalOverlayPlugin, - shouldLoad: ({ panelState, temporalState }) => Boolean(panelState["temporal-panel"] || temporalState?.currentTime), + shouldLoad: ({ panelState }) => Boolean(panelState["temporal-panel"]), }, ], [], @@ -2274,6 +2291,52 @@ export function GraphWorkspace({ externalFocusNodeId, externalFocusToken }: Grap if (!GRAPH_THEME.effects.diagnostics.enabledInDev) { return; } + + // Compare against the last accepted snapshot synchronously via a ref — + // this prevents React from entering the loop at all, rather than bailing + // out inside the functional updater after a render has already been + // scheduled (issue #830: calling setGraphDiagnosticsState with a new + // object on every effect run caused a render→effect→setState→render cycle + // that exceeded React's max update depth in dev mode). + const prev = lastDiagnosticsRef.current; + if (prev !== null) { + const EFFECT_KEYS = [ + "pathPulse", "pathFlow", "lens", "temporalEmphasis", "semanticRegions", + "contours", "pathfinding", "communities", "centrality", "legend", "diagnostics", + ] as const; + const prevEA = prev.effectAvailability; + const nextEA = diagnostics.effectAvailability; + const availabilityChanged = EFFECT_KEYS.some((key) => { + const p = prevEA[key]; + const n = nextEA[key]; + return ( + p.enabled !== n.enabled || + p.available !== n.available || + p.reason !== n.reason || + p.detail !== n.detail || + p.visibleSegments !== n.visibleSegments || + p.segmentCap !== n.segmentCap + ); + }); + + const edgeClassesChanged = + prev.edgeClasses?.updatedAt !== diagnostics.edgeClasses?.updatedAt; + + const structureLayerChanged = + prev.structureLayer?.cacheKey !== diagnostics.structureLayer?.cacheKey || + prev.structureLayer?.lastDrawAt !== diagnostics.structureLayer?.lastDrawAt || + prev.structureLayer?.enabled !== diagnostics.structureLayer?.enabled; + + // distanceVisual comes from distanceVisualStateRef.current in GraphCanvas — + // same object reference when distances haven't changed. + const distanceVisualChanged = prev.distanceVisual !== diagnostics.distanceVisual; + + if (!availabilityChanged && !edgeClassesChanged && !structureLayerChanged && !distanceVisualChanged) { + return; + } + } + + lastDiagnosticsRef.current = diagnostics; setGraphDiagnosticsState(diagnostics); }, []); @@ -2922,7 +2985,7 @@ export function GraphWorkspace({ externalFocusNodeId, externalFocusToken }: Grap
Loading timeline…
}> diff --git a/explorer/src/workspaces/GraphWorkspace/GraphWorkspaceShell.tsx b/explorer/src/workspaces/GraphWorkspace/GraphWorkspaceShell.tsx index 440c3e33..7a085544 100644 --- a/explorer/src/workspaces/GraphWorkspace/GraphWorkspaceShell.tsx +++ b/explorer/src/workspaces/GraphWorkspace/GraphWorkspaceShell.tsx @@ -325,6 +325,17 @@ export function GraphWorkspaceShell() { const [activeNodeCount, setActiveNodeCount] = useState(null); const [temporalBounds, setTemporalBounds] = useState(null); const [scrubberTime, setScrubberTime] = useState(null); + // Deduplicates setScrubberTime calls by millisecond value — same fix as + // GraphWorkspace.tsx (issue #830). + const lastScrubberMsRef = useRef(null); + const onTimeChange = useCallback((time: Date) => { + const ms = time.getTime(); + if (ms === lastScrubberMsRef.current) { + return; + } + lastScrubberMsRef.current = ms; + setScrubberTime(time); + }, []); const [loadingProgress, setLoadingProgress] = useState(null); const [isGraphStageReady, setIsGraphStageReady] = useState(false); const [layoutStatus, setLayoutStatus] = useState({ @@ -632,7 +643,7 @@ export function GraphWorkspaceShell() { }> diff --git a/explorer/src/workspaces/GraphWorkspace/plugins/temporalOverlayPlugin.tsx b/explorer/src/workspaces/GraphWorkspace/plugins/temporalOverlayPlugin.tsx index b2f30158..bf656fc6 100644 --- a/explorer/src/workspaces/GraphWorkspace/plugins/temporalOverlayPlugin.tsx +++ b/explorer/src/workspaces/GraphWorkspace/plugins/temporalOverlayPlugin.tsx @@ -1,11 +1,6 @@ -import { useEffect, useRef, useState, type CSSProperties } from "react"; -import { Loader2 } from "lucide-react"; -import type Graph from "graphology"; +import type { CSSProperties } from "react"; -import { graph, type NodeAttributes } from "../../../store/graphStore"; -import { GRAPH_THEME } from "../graphTheme"; -import { fetchTemporalDiff, type TemporalDiffResult } from "./temporalDiffState"; -import type { GraphPlugin, GraphPluginContext } from "./types"; +import type { GraphPlugin } from "./types"; const TEMPORAL_PANEL_ID = "temporal-panel"; @@ -16,218 +11,6 @@ function formatTemporalLabel(value: Date | null) { return `${value.getFullYear()}/${String(value.getMonth() + 1).padStart(2, "0")}`; } -function isValidDateInput(value: string): boolean { - return value.trim().length > 0 && !Number.isNaN(new Date(value).getTime()); -} - -type DiffRequestState = - | { status: "idle" } - | { status: "loading" } - | { status: "error"; message: string } - | { status: "empty"; result: TemporalDiffResult } - | { status: "success"; result: TemporalDiffResult }; - -// Diff highlight colors reuse existing theme tokens rather than introducing new -// hex values: semantic[2] is the codebase's green, dangerText is the one named -// danger/red token already used elsewhere in this workspace (GraphWorkspace.tsx). -const DIFF_ADDED_COLOR = GRAPH_THEME.palette.semantic[2]; -const DIFF_REMOVED_COLOR = GRAPH_THEME.ui.control.dangerText; - -// Highlighting uses baseColor (the node's fill color), not ringColor/haloColor: those two -// are only read by resolveNodeElementStyle/GraphCanvas's decoration pass for nodes in -// hovered/selected/path visual state (see resolveNodeRingSize, showHalo in graphSceneState.ts -// and the nodesToDecorate set in GraphCanvas.tsx) and are silently discarded by the sigma -// nodeReducer for a node sitting in its default (untouched) state — which is exactly the -// state every diffed node is in here. baseColor is read unconditionally by resolveNodeColor's -// default branch regardless of interaction state or zoom tier, so it is the one attribute -// confirmed to actually render the diff highlight. -// -// Writes go to BOTH the store graph and context.displayGraph: -// - context.displayGraph is the live Graph instance currently bound to Sigma (updated by -// GraphCanvas.tsx via sigma.setGraph() / runtimeRef.current.displayGraph = displayGraph -// whenever the display graph is rebuilt). The nodeReducer reads attributes from this -// instance, so writing here makes the highlight visible in the currently-rendered frame. -// - graph (store singleton) carries the value into the *next* display graph rebuild: -// aggregateDisplayGraph copies node attributes shallowly from the store graph, so a -// mutation that only touches context.displayGraph would be lost on the next rebuild. -// Using a type assertion to Graph is consistent with how -// the rest of GraphCanvas/graphSceneState cast the same union when they need to call -// mutation methods; TypeScript cannot resolve setNodeAttribute across the union directly. - -function toMutable(g: GraphPluginContext["displayGraph"]) { - return g as Graph; -} - -function writeBaseColor( - context: GraphPluginContext, - nodeId: string, - color: string | undefined, -): void { - // Write to the store graph first (survives display graph rebuilds). - if (graph.hasNode(nodeId)) { - graph.setNodeAttribute(nodeId, "baseColor", color); - } - // Write to the current display graph instance Sigma is rendering. - const dg = toMutable(context.displayGraph); - if (dg !== graph && dg.hasNode(nodeId)) { - dg.setNodeAttribute(nodeId, "baseColor", color); - } -} - -function clearDiffHighlight(context: GraphPluginContext, previousColors: Map) { - previousColors.forEach((color, nodeId) => { - writeBaseColor(context, nodeId, color); - }); - context.scene?.requestRender(); -} - -function applyDiffHighlight(context: GraphPluginContext, result: TemporalDiffResult): Map { - const previousColors = new Map(); - const paint = (nodeId: string, color: string) => { - // Capture from the store graph — this is the authoritative source for the node's - // original baseColor, since aggregateDisplayGraph copies from there. - if (graph.hasNode(nodeId)) { - previousColors.set(nodeId, graph.getNodeAttribute(nodeId, "baseColor")); - writeBaseColor(context, nodeId, color); - } - }; - result.added_nodes.forEach((nodeId) => paint(nodeId, DIFF_ADDED_COLOR)); - result.removed_nodes.forEach((nodeId) => paint(nodeId, DIFF_REMOVED_COLOR)); - context.scene?.requestRender(); - return previousColors; -} - -function DiffSection({ context }: { context: GraphPluginContext }) { - const [fromTime, setFromTime] = useState(""); - const [toTime, setToTime] = useState(""); - const [validationMessage, setValidationMessage] = useState(null); - const [requestState, setRequestState] = useState({ status: "idle" }); - const abortRef = useRef(null); - // Maps currently-highlighted node ID -> its baseColor before highlighting, so clearing - // restores the exact prior value instead of an approximation. - const previousColorsRef = useRef>(new Map()); - - // Cancel any in-flight request and clear stale highlights when the panel unmounts - // (panel closed) — matches the cancellation pattern used by the snapshot fetch in - // GraphRuntimeStage.tsx (cancel-on-cleanup) plus AbortController per DecisionWorkspace.tsx. - useEffect(() => { - return () => { - abortRef.current?.abort(); - clearDiffHighlight(context, previousColorsRef.current); - previousColorsRef.current = new Map(); - }; - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); - - const handleCompare = () => { - const from = fromTime.trim(); - const to = toTime.trim(); - - if (!from || !to) { - setValidationMessage("Both a from and to time are required."); - return; - } - if (!isValidDateInput(from) || !isValidDateInput(to)) { - setValidationMessage("Enter valid ISO datetimes, e.g. 2024-01-01T00:00:00."); - return; - } - if (new Date(from).getTime() >= new Date(to).getTime()) { - setValidationMessage("From time must be before to time."); - return; - } - setValidationMessage(null); - - // Cancel any request already in flight before starting a new one. - abortRef.current?.abort(); - const controller = new AbortController(); - abortRef.current = controller; - - clearDiffHighlight(context, previousColorsRef.current); - previousColorsRef.current = new Map(); - setRequestState({ status: "loading" }); - - fetchTemporalDiff(from, to, controller.signal) - .then((result) => { - if (controller.signal.aborted) { - return; - } - if (!result.added_nodes.length && !result.removed_nodes.length) { - setRequestState({ status: "empty", result }); - return; - } - previousColorsRef.current = applyDiffHighlight(context, result); - setRequestState({ status: "success", result }); - }) - .catch((error: unknown) => { - if (error instanceof Error && error.name === "AbortError") { - return; - } - setRequestState({ - status: "error", - message: error instanceof Error ? error.message : "Temporal diff request failed.", - }); - }); - }; - - const isLoading = requestState.status === "loading"; - - return ( -
-
Compare two points in time
-
- setFromTime(event.target.value)} - placeholder="From, e.g. 2024-01-01T00:00:00" - style={diffInputStyle} - /> - setToTime(event.target.value)} - placeholder="To, e.g. 2025-06-15T00:00:00" - style={diffInputStyle} - /> -
- - - {validationMessage ?
{validationMessage}
: null} - - {requestState.status === "error" ? ( -
{requestState.message}
- ) : null} - - {requestState.status === "empty" ? ( -
No changes between these two points.
- ) : null} - - {requestState.status === "success" ? ( -
-
- Added - - {requestState.result.added_nodes.length.toLocaleString()} - -
-
- Removed - - {requestState.result.removed_nodes.length.toLocaleString()} - -
-
- ) : null} -
- ); -} - export const temporalOverlayPlugin: GraphPlugin = { id: "temporal-overlay", mount: () => {}, @@ -297,7 +80,7 @@ export const temporalOverlayPlugin: GraphPlugin = { order: 30, defaultOpen: false, preferredWidth: 320, - preferredHeight: 380, + preferredHeight: 220, content: (
Current scrubber state
@@ -317,7 +100,6 @@ export const temporalOverlayPlugin: GraphPlugin = { {typeof temporal?.activeNodeCount === "number" ? temporal.activeNodeCount.toLocaleString() : "All"}
- ), }; @@ -358,62 +140,3 @@ const detailValueStyle: CSSProperties = { fontSize: 12, fontWeight: 600, }; - -const diffSectionStyle: CSSProperties = { - display: "flex", - flexDirection: "column", - gap: 8, - marginTop: 4, - paddingTop: 10, - borderTop: "1px solid rgba(255,255,255,0.06)", -}; - -const diffInputRowStyle: CSSProperties = { - display: "flex", - gap: 8, -}; - -const diffInputStyle: CSSProperties = { - flex: 1, - minWidth: 0, - background: "rgba(5, 7, 10, 0.52)", - border: "1px solid rgba(211, 205, 190, 0.13)", - color: "#f3f7fd", - borderRadius: 12, - padding: "9px 11px", - fontSize: 12, - boxShadow: "inset 0 1px 0 rgba(255,255,255,0.035)", -}; - -const diffActionButtonStyle: CSSProperties = { - background: GRAPH_THEME.ui.control.primaryBg, - color: GRAPH_THEME.ui.control.primaryText, - border: `1px solid ${GRAPH_THEME.ui.control.primaryBorder}`, - borderRadius: 12, - padding: "9px 12px", - cursor: "pointer", - fontWeight: 700, - fontSize: 12, - display: "inline-flex", - alignItems: "center", - justifyContent: "center", - boxShadow: "inset 0 1px 0 rgba(255,255,255,0.07), 0 10px 24px rgba(0,0,0,0.18)", -}; - -const diffValidationStyle: CSSProperties = { - color: GRAPH_THEME.ui.control.dangerText, - fontSize: 12, - lineHeight: 1.5, -}; - -const diffErrorStyle: CSSProperties = { - color: GRAPH_THEME.ui.control.dangerText, - fontSize: 12, - lineHeight: 1.5, -}; - -const emptyTextStyle: CSSProperties = { - color: "#8ea4be", - fontSize: 12, - lineHeight: 1.5, -};