diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 18a246fb..6122e91b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -30,11 +30,18 @@ jobs: node-version: '20' cache: 'npm' cache-dependency-path: explorer/package-lock.json - - name: Build Explorer frontend + - name: Install Explorer frontend dependencies + working-directory: explorer + run: npm ci + - name: Test Explorer frontend working-directory: explorer run: | - npm ci - npm run build + npm run test:graph-store + npm run test:graph-workspace + npm run test:plugin-registry + - name: Build Explorer frontend + working-directory: explorer + run: npm run build - run: pip install build - run: python -m build - name: Verify Explorer frontend is packaged diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index c32d6955..9aedaf40 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -32,7 +32,7 @@ jobs: # meaningful state carried over from a failed attempt. - name: Initialize CodeQL (attempt 1) id: codeql-init-1 - uses: github/codeql-action/init@d1ba80a13dd99fba24a470575428917156a28b43 # v4 + uses: github/codeql-action/init@5595ccaf912efad79be6eef63a5619ff05969be3 # v4 continue-on-error: true with: languages: python @@ -42,7 +42,7 @@ jobs: - name: Initialize CodeQL (attempt 2) id: codeql-init-2 if: steps.codeql-init-1.outcome == 'failure' - uses: github/codeql-action/init@d1ba80a13dd99fba24a470575428917156a28b43 # v4 + uses: github/codeql-action/init@5595ccaf912efad79be6eef63a5619ff05969be3 # v4 continue-on-error: true with: languages: python @@ -52,17 +52,17 @@ jobs: - name: Initialize CodeQL (attempt 3) id: codeql-init-3 if: steps.codeql-init-2.outcome == 'failure' - uses: github/codeql-action/init@d1ba80a13dd99fba24a470575428917156a28b43 # v4 + uses: github/codeql-action/init@5595ccaf912efad79be6eef63a5619ff05969be3 # v4 with: languages: python queries: security-and-quality config-file: .github/codeql/codeql-config.yml - name: Autobuild - uses: github/codeql-action/autobuild@d1ba80a13dd99fba24a470575428917156a28b43 # v4 + uses: github/codeql-action/autobuild@5595ccaf912efad79be6eef63a5619ff05969be3 # v4 - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@d1ba80a13dd99fba24a470575428917156a28b43 # v4 + uses: github/codeql-action/analyze@5595ccaf912efad79be6eef63a5619ff05969be3 # v4 with: category: "/language:python" upload: false @@ -72,7 +72,7 @@ jobs: # Uploads results only when Default Setup is not active. # If Default Setup is still enabled, this step skips gracefully # instead of failing the workflow with HTTP 409. - uses: github/codeql-action/upload-sarif@d1ba80a13dd99fba24a470575428917156a28b43 # v4 + uses: github/codeql-action/upload-sarif@5595ccaf912efad79be6eef63a5619ff05969be3 # v4 with: sarif_file: ${{ steps.codeql.outputs.sarif-output }} category: "/language:python" diff --git a/.github/workflows/defender-for-devops.yml b/.github/workflows/defender-for-devops.yml index aa71bd69..c561dc17 100644 --- a/.github/workflows/defender-for-devops.yml +++ b/.github/workflows/defender-for-devops.yml @@ -57,7 +57,7 @@ jobs: # avoiding the guardian.cmd/checkov exit-code bug in the MSDO wrapper. tools: eslint,templateanalyzer,terrascan - name: Upload results to Security tab - uses: github/codeql-action/upload-sarif@d1ba80a13dd99fba24a470575428917156a28b43 # v4 + uses: github/codeql-action/upload-sarif@5595ccaf912efad79be6eef63a5619ff05969be3 # v4 with: sarif_file: ${{ steps.msdo.outputs.sarifFile }} @@ -82,7 +82,7 @@ jobs: } - name: Upload Checkov results to Security tab - uses: github/codeql-action/upload-sarif@d1ba80a13dd99fba24a470575428917156a28b43 # v4 + uses: github/codeql-action/upload-sarif@5595ccaf912efad79be6eef63a5619ff05969be3 # v4 if: always() with: sarif_file: reports/checkov.sarif diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c1c1db8..330f933e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,6 +60,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Explorer Temporal panel never rendered after clicking the toolbar button** (#830, #836) by @Sameer6305 + - The panel stayed permanently stuck on "Loading temporal…" in `npm run dev`, with repeating "Maximum update depth exceeded" errors in the browser console. Two independent render loops were responsible: + - **Diagnostics state churn**: `handleDiagnosticsChange` unconditionally called `setGraphDiagnosticsState` on every invocation. `buildEffectAvailability` (inside `GraphCanvas`'s diagnostics `useEffect`) always returns a new object, so each call scheduled a re-render that immediately retriggered the effect. Fixed by comparing the incoming snapshot field-by-field against the last accepted value via `lastDiagnosticsRef` before calling `setState` + - **scrubberTime churn**: React 18 concurrent mode re-ran `TimelinePanel`'s `useEffect` with a structurally-new `Date` object for the same timestamp when speculative renders discarded `useMemo` caches, causing repeated `setScrubberTime` calls that propagated into `temporalState` churn and retriggered the diagnostics effect. Fixed by deduplicating by millisecond value via `onTimeChange`/`lastScrubberMsRef` + - **Bonus**: `temporal-overlay`'s `shouldLoad` predicate was changed to gate strictly on `panelState["temporal-panel"]`, removing the `|| temporalState?.currentTime` branch that caused eager loading on every scrubber update and continuously cancelled in-flight `load()` completions + - **Bonus**: `temporalState` removed from the plugin-loading `useEffect` dependency array; predicates extracted into `pluginRegistryPredicates.ts` and wired through `GraphWorkspace.tsx` so regression tests exercise the production code rather than a local copy + - The `scrubberTime`-churn fix was also applied to the equivalent (but currently unused/unmounted) `GraphWorkspaceShell.tsx`, which shares the same `TimelinePanel` integration pattern but does not have the diagnostics-churn code path + - **Follow-up review fix**: the diagnostics dedup's `structureLayer` comparison now also covers `disabledReason`, `curveCount`, `bridgeCurveCount`, and `backboneCurveCount` (previously only `cacheKey`/`lastDrawAt`/`enabled` were compared, so a pure `disabledReason` transition could leave the dev-only diagnostics panel stale) + - **Follow-up review fix**: `test:graph-store`, `test:graph-workspace`, and the new `test:plugin-registry` regression test are now run in CI (`.github/workflows/ci.yml`) — previously none of the Explorer frontend's `node --test` suites executed anywhere in CI, only `npm run build`, so this fix's own regression coverage (and all prior frontend test coverage) provided no protection against silent regressions - **`HybridSearch.search()` crashed with `AttributeError` for any `VectorStore` backend other than `inmemory`** (#833, #837) by @KaifAhmad1 - `HybridSearch.search()` read `self.vector_store.vectors` directly, an internal dict `VectorStore` only populates for `backend="inmemory"`; every other backend (faiss, weaviate, qdrant, milvus, pinecone, pgvector, sqlite) raised `AttributeError`, making `HybridSearch` unusable against any real store. It now delegates to `VectorStore.search_vectors()` (the backend-agnostic public API) for non-inmemory backends, applies `metadata_filter` as a post-filter over the returned candidates, and normalizes results to a consistent `{id, score, distance, metadata}` shape - **Fixed along the way**: `vector_ids` could stay `None` when callers passed explicit `vectors`/`metadata` without `vector_ids`, crashing downstream list indexing — now defaulted to generated positional IDs diff --git a/explorer/package.json b/explorer/package.json index 63a88d39..72e36f73 100644 --- a/explorer/package.json +++ b/explorer/package.json @@ -9,7 +9,8 @@ "lint": "eslint .", "preview": "vite preview", "test:graph-store": "node --test tests/graphStore.multi-edge.test.mjs", - "test:graph-workspace": "node --import tsx --test tests/graphSceneState.display.test.ts" + "test:graph-workspace": "node --import tsx --test tests/graphSceneState.display.test.ts", + "test:plugin-registry": "node --import tsx --test tests/pluginRegistry.temporal.test.mjs" }, "dependencies": { "@monaco-editor/react": "^4.7.0", diff --git a/explorer/src/workspaces/GraphWorkspace/GraphWorkspace.tsx b/explorer/src/workspaces/GraphWorkspace/GraphWorkspace.tsx index db35bce9..df2cd6ec 100644 --- a/explorer/src/workspaces/GraphWorkspace/GraphWorkspace.tsx +++ b/explorer/src/workspaces/GraphWorkspace/GraphWorkspace.tsx @@ -38,6 +38,7 @@ import { type GraphPluginPanelDescriptor, type GraphPluginToolbarItem, } from "./plugins"; +import { explorationEffectsShouldLoad, neighborhoodPanelShouldLoad, temporalOverlayShouldLoad } from "./pluginRegistryPredicates"; import type { LinkPrediction, PathResponse } from "./GraphInspectorPanel"; import type { GraphSceneHandle, GraphSceneRuntime } from "./scene"; import type { @@ -126,7 +127,7 @@ type LazyPluginRegistryEntry = { load: () => Promise; shouldLoad: (context: { panelState: Record; - temporalState: GraphTemporalState | null; + temporalState?: GraphTemporalState | null; }) => boolean; }; @@ -1119,6 +1120,18 @@ export function GraphWorkspace({ externalFocusNodeId, externalFocusToken }: Grap const [activeNodeCount, setActiveNodeCount] = useState(null); const [temporalBounds, setTemporalBounds] = useState(null); const [scrubberTime, setScrubberTime] = useState(null); + // Deduplicates setScrubberTime calls by millisecond value so that React 18 + // concurrent-mode re-renders with a new Date object for the same timestamp + // do not churn temporalState and retrigger the diagnostics effect (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 [pluginPanelState, setPluginPanelState] = useState>({ "effects-panel": false, @@ -1129,6 +1142,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>({}); @@ -2056,7 +2072,7 @@ export function GraphWorkspace({ externalFocusNodeId, externalFocusToken }: Grap title: "Open exploration effects controls", order: 18, load: loadExplorationEffectsPlugin, - shouldLoad: ({ panelState }) => Boolean(panelState["effects-panel"]), + shouldLoad: explorationEffectsShouldLoad, }, { id: "neighborhood-panel", @@ -2065,7 +2081,7 @@ export function GraphWorkspace({ externalFocusNodeId, externalFocusToken }: Grap title: "Toggle neighborhood panel", order: 30, load: loadNeighborhoodPanelPlugin, - shouldLoad: ({ panelState }) => Boolean(panelState["neighborhood-panel"]), + shouldLoad: neighborhoodPanelShouldLoad, }, { id: "temporal-overlay", @@ -2074,7 +2090,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: temporalOverlayShouldLoad, }, ], [], @@ -2092,7 +2108,7 @@ export function GraphWorkspace({ externalFocusNodeId, externalFocusToken }: Grap return; } - if (!entry.shouldLoad({ panelState: pluginPanelState, temporalState })) { + if (!entry.shouldLoad({ panelState: pluginPanelState })) { return; } @@ -2111,7 +2127,7 @@ export function GraphWorkspace({ externalFocusNodeId, externalFocusToken }: Grap return () => { cancelled = true; }; - }, [loadedPlugins, pluginPanelState, pluginRegistry, temporalState]); + }, [loadedPlugins, pluginPanelState, pluginRegistry]); const setEffectToggle = useCallback((effect: GraphEffectToggle, enabled: boolean | ((current: boolean) => boolean)) => { setEffectsState((current) => { @@ -2274,6 +2290,55 @@ export function GraphWorkspace({ externalFocusNodeId, externalFocusToken }: Grap if (!GRAPH_THEME.effects.diagnostics.enabledInDev) { return; } + + // Compare against the last accepted snapshot synchronously before calling + // setState. buildEffectAvailability always returns a new object, so an + // unconditional setGraphDiagnosticsState on every call created a + // render → diagnostics effect → setState → render cycle that exceeded + // React's max update depth in dev mode (issue #830). + 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 || + prev.structureLayer?.disabledReason !== diagnostics.structureLayer?.disabledReason || + prev.structureLayer?.curveCount !== diagnostics.structureLayer?.curveCount || + prev.structureLayer?.bridgeCurveCount !== diagnostics.structureLayer?.bridgeCurveCount || + prev.structureLayer?.backboneCurveCount !== diagnostics.structureLayer?.backboneCurveCount; + + // distanceVisual is compared by reference: GraphCanvas passes the same + // object when distances haven't changed. + const distanceVisualChanged = prev.distanceVisual !== diagnostics.distanceVisual; + + if (!availabilityChanged && !edgeClassesChanged && !structureLayerChanged && !distanceVisualChanged) { + return; + } + } + + lastDiagnosticsRef.current = diagnostics; setGraphDiagnosticsState(diagnostics); }, []); @@ -2922,7 +2987,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/pluginRegistryPredicates.ts b/explorer/src/workspaces/GraphWorkspace/pluginRegistryPredicates.ts new file mode 100644 index 00000000..e5a5331a --- /dev/null +++ b/explorer/src/workspaces/GraphWorkspace/pluginRegistryPredicates.ts @@ -0,0 +1,24 @@ +/** + * shouldLoad predicates for the GraphWorkspace lazy plugin registry. + * + * Extracted into a pure module so the predicates can be unit-tested without + * importing the full GraphWorkspace React component. Each predicate gates + * whether a plugin's module is lazily imported; none reference temporalState + * so temporal scrubber updates never retrigger plugin loading (issue #830). + */ + +export type PluginShouldLoadContext = { + panelState: Record; +}; + +export function explorationEffectsShouldLoad({ panelState }: PluginShouldLoadContext): boolean { + return Boolean(panelState["effects-panel"]); +} + +export function neighborhoodPanelShouldLoad({ panelState }: PluginShouldLoadContext): boolean { + return Boolean(panelState["neighborhood-panel"]); +} + +export function temporalOverlayShouldLoad({ panelState }: PluginShouldLoadContext): boolean { + return Boolean(panelState["temporal-panel"]); +} diff --git a/explorer/src/workspaces/GraphWorkspace/plugins/temporalDiffState.ts b/explorer/src/workspaces/GraphWorkspace/plugins/temporalDiffState.ts deleted file mode 100644 index 9e7010ac..00000000 --- a/explorer/src/workspaces/GraphWorkspace/plugins/temporalDiffState.ts +++ /dev/null @@ -1,23 +0,0 @@ -// Fetch wrapper for GET /api/temporal/diff (see semantica/explorer/routes/temporal.py). -// added_nodes are node IDs active at to_time but not at from_time; removed_nodes are the -// inverse. Both are plain node-ID lists (no edge-level diffing), matching the snapshot -// route's active_node_ids shape used elsewhere in this workspace. -export interface TemporalDiffResult { - from_time: string; - to_time: string; - added_nodes: string[]; - removed_nodes: string[]; -} - -export async function fetchTemporalDiff( - fromTime: string, - toTime: string, - signal?: AbortSignal, -): Promise { - const params = new URLSearchParams({ from_time: fromTime, to_time: toTime }); - const response = await fetch(`/api/temporal/diff?${params}`, { signal }); - if (!response.ok) { - throw new Error(`Temporal diff request failed with status ${response.status}`); - } - return response.json(); -} 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, -}; diff --git a/explorer/tests/pluginRegistry.temporal.test.mjs b/explorer/tests/pluginRegistry.temporal.test.mjs new file mode 100644 index 00000000..8d5e156d --- /dev/null +++ b/explorer/tests/pluginRegistry.temporal.test.mjs @@ -0,0 +1,90 @@ +/** + * Regression tests for issue #830: plugin registry shouldLoad predicates. + * + * Imports the production predicates from pluginRegistryPredicates.ts so that + * a regression in GraphWorkspace.tsx is detected here. The key invariant: no + * predicate may read temporalState — doing so caused a render loop because + * temporalState.currentTime is non-null from startup, which triggered eager + * plugin loads on every scrubber update and continuously cancelled in-flight + * load() calls before they could register the plugin. + */ +import test from "node:test"; +import assert from "node:assert/strict"; +import { createRequire } from "node:module"; + +const require = createRequire(import.meta.url); + +const { + explorationEffectsShouldLoad, + neighborhoodPanelShouldLoad, + temporalOverlayShouldLoad, +} = require("../src/workspaces/GraphWorkspace/pluginRegistryPredicates.ts"); + +// ── temporal-overlay ───────────────────────────────────────────────────────── + +test("temporal-overlay shouldLoad: false when panel is closed and no scrubber time", () => { + assert.equal( + temporalOverlayShouldLoad({ panelState: { "temporal-panel": false } }), + false, + ); +}); + +test("temporal-overlay shouldLoad: false when panel is closed even if scrubber time is set", () => { + // Before the fix, a non-null currentTime caused an eager load on every scrubber update. + assert.equal( + temporalOverlayShouldLoad({ + panelState: { "temporal-panel": false }, + temporalState: { currentTime: new Date() }, + }), + false, + ); +}); + +test("temporal-overlay shouldLoad: true only when the panel is explicitly opened", () => { + assert.equal( + temporalOverlayShouldLoad({ panelState: { "temporal-panel": true } }), + true, + ); +}); + +test("temporal-overlay shouldLoad: true when panel opened even without a scrubber time", () => { + assert.equal( + temporalOverlayShouldLoad({ + panelState: { "temporal-panel": true }, + temporalState: { currentTime: null }, + }), + true, + ); +}); + +// ── other entries — confirm they also gate only on panelState ───────────────── + +test("exploration-effects shouldLoad: gates only on effects-panel state", () => { + assert.equal(explorationEffectsShouldLoad({ panelState: { "effects-panel": false } }), false); + assert.equal(explorationEffectsShouldLoad({ panelState: { "effects-panel": true } }), true); +}); + +test("neighborhood-panel shouldLoad: gates only on neighborhood-panel state", () => { + assert.equal(neighborhoodPanelShouldLoad({ panelState: { "neighborhood-panel": false } }), false); + assert.equal(neighborhoodPanelShouldLoad({ panelState: { "neighborhood-panel": true } }), true); +}); + +test("all three shouldLoad conditions are consistent: none reference temporalState", () => { + // A regressed predicate reading temporalState?.currentTime would return true + // for a closed panel when currentTime is set — detecting the loop bug. + const nonNullTemporalState = { currentTime: new Date(), activeNodeCount: 6 }; + + assert.equal( + temporalOverlayShouldLoad({ panelState: { "temporal-panel": false }, temporalState: nonNullTemporalState }), + false, + "temporal-overlay must not load when panel is closed, regardless of scrubber time", + ); + assert.equal( + explorationEffectsShouldLoad({ panelState: { "effects-panel": false }, temporalState: nonNullTemporalState }), + false, + ); + assert.equal( + neighborhoodPanelShouldLoad({ panelState: { "neighborhood-panel": false }, temporalState: nonNullTemporalState }), + false, + ); +});