Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion docs/adr/20260813-element-data-style-mappers.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ The graph canvas colours each vertex/edge by its type. The obvious Cytoscape idi

Precompute each element's resolved style values onto its Cytoscape `ele.data()` as `ge_*` fields (`vertexStyleData` / `edgeStyleData` in `core/StateProvider/graphElementStyleData.ts`) at the element-construction seams (`renderedEntities.ts` for the explorer graph, `useSchemaGraphData.ts` for the schema view), and read them back through a single `node` rule and single `edge` rule using Cytoscape `data(…)` mappers (`CANVAS_STYLES` in `useGraphStyles.ts`). The stylesheet is now O(1) in the number of types.

Two structured/optional properties can't be a plain always-present mapper, so they keep a gated selector that applies only when the field exists: `node[__iconUrl]` (background image; the field is omitted when a type has no icon) and `edge[ge_lineDashPattern]` (omitted for solid lines). The dotted→dashed remap, the dash-pattern lookup, the border-opacity derivation, and the `isDark` label-text-colour pick are baked into the producer functions so the stylesheet stays pure `data(…)`.
**Every field is always set, so there are no gated selectors.** `cy.json({ elements })` _merges_ element data — `ele.data(obj)` adds and overwrites keys but never deletes ones missing from the new object. A sometimes-absent field therefore can never be cleared once applied, stranding a stale value on an already-drawn element: an icon that stopped resolving kept rendering the previous image. So `ge_iconUrl` carries `"none"` when a type has no icon, and `ge_lineDashPattern` carries cytoscape's default for solid lines, letting both live on the base `node` / `edge` rule. The dotted→dashed remap, the dash-pattern lookup, the border-opacity derivation, and the `isDark` label-text-colour pick are baked into the producer functions so the stylesheet stays pure `data(…)`.

## Consequences

Expand All @@ -21,3 +21,4 @@ Two structured/optional properties can't be a plain always-present mapper, so th
- **A stylesheet consumer must merge into the base `node`/`edge` rules, never replace them.** `useSchemaGraphStyles` adds a schema label by spreading `{ ...baseStyles.node, label: … }`; overwriting the `node`/`edge` keys wholesale would discard every `ge_*` mapper and render the graph unstyled. Guarded by `useSchemaGraphStyles.test.tsx`.
- Context-count regression guards (`useGraphStyles.contextCount.test.tsx`, `useSchemaGraphStyles.test.tsx`) assert the selector count stays O(1) regardless of type count, so the per-type-selector approach can't creep back in unnoticed.
- A future reader seeing `data(ge_*)` mappers and no per-type selectors should not "restore" per-type selectors — that is the exact regression this avoids.
- **The `ge_*` producers must be fed a type set scoped to what is drawn, not the whole schema.** Moving styling into element data moves the per-type cost from the stylesheet into the render path, so resolving a style and an icon for every schema type became the new bottleneck (10,044 types resolved to draw 3). Each surface scopes itself to what it draws, and pairs the style with the element so the drawn set and the styled set are the same set by construction — the canvas from `canvasVerticesAtom`, the schema view from its vertex type configs. Do not scope either from a separately-read schema snapshot: `useActiveSchema` is deferred while the type configs are not, so the two can disagree mid-sync and leave an element styleless. Edges have no canvas-scoped equivalent on purpose — edge style data needs no icon resolution, so scoping would buy nothing.
6 changes: 6 additions & 0 deletions docs/agents/react.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

- This project uses React 19
- The React Compiler is enabled — it auto-memoizes components and hooks, so manual `useMemo`, `useCallback`, and `React.memo` are unnecessary in most cases and should be avoided unless profiling shows a specific need
- When per-call memoization genuinely is needed, a hook must not build and return a closure that mutates its own captured cache — the `react-compiler/*` lint rules reject it. Put the cache in a plain `create*` factory and have the hook call it, as `core/StateProvider/styleDataResolvers.ts` does. The two-line hook is not a pointless wrapper: collapsing it back inline fails lint. The factory also ends up store-free and directly unit-testable.
- Official React docs: https://react.dev

## General
Expand All @@ -16,6 +17,11 @@
- Server state goes in TanStack Query
- **Exception: vertex icons.** They resolve through the `core/icons/` registry, read via `useSyncExternalStore`, because a per-hook subscription scaled with vertex-type count and locked up the schema view at 10k types. Don't move icon resolution back into TanStack Query — see `docs/adr/20260813-icon-registry-not-react-query.md`

## Client state (Jotai)

- When a derivation is consumed by more than one hook or pipeline, define it as a derived atom so the store computes it once, rather than a hook each caller re-runs. `visibleVertexIdsAtom` is shared by `useRenderedVertices` and `useRenderedEdges` for exactly this reason; as a hook it ran the filter loop once per call site
- `atomFamily` never evicts unless you call `remove`/`setShouldRemove`, and nothing in this codebase does. Key a family on a stable branded ID, never on a freshly allocated object or array — the latter interns a new entry per recomputation that can never be reached again

## Feature Modules

- Feature modules in `src/modules/` contain all related components, hooks, and utilities
Expand Down
71 changes: 57 additions & 14 deletions packages/graph-explorer/src/components/ColorPopover.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import type { ComponentPropsWithRef, CSSProperties } from "react";

import { PencilIcon } from "lucide-react";
import {
type ComponentPropsWithRef,
type CSSProperties,
useEffect,
useState,
} from "react";
import { HexColorInput, HexColorPicker } from "react-colorful";

import {
Expand All @@ -10,6 +14,7 @@ import {
PopoverContent,
PopoverTrigger,
} from "@/components";
import { useDebounceValue, usePrevious } from "@/hooks";
import { cn } from "@/utils";

export function ColorPopover({
Expand All @@ -30,23 +35,61 @@ export function ColorPopover({
</Button>
</PopoverTrigger>
<PopoverContent side="bottom" align="end" className="flex flex-col gap-4">
<HexColorInput
alpha
color={color}
onChange={onColorChange}
className={cn(inputStyles())}
autoFocus
/>
<HexColorPicker
onChange={onColorChange}
color={color}
className="block size-[200px] w-auto"
/>
<ColorPicker color={color} onColorChange={onColorChange} />
</PopoverContent>
</Popover>
);
}

/**
* Tracks the pointer locally and commits on a delay, following the same pattern
* as the display-name field in `modules/Styles/VertexStyleRow.tsx`.
*
* `react-colorful` fires `onChange` on every pointermove, and each committed
* style rebuilds every element's data and re-serializes the whole canvas — one
* dropped frame per commit, measured up to ~200ms on a 76 node graph. Local
* state keeps the swatch following the pointer at full rate regardless.
*
* Separate from `ColorPopover` so it mounts with the popover content, which
* Radix unmounts on close: the draft is therefore seeded from `color` on every
* open and cannot drift across openings.
*/
function ColorPicker({
color,
onColorChange,
}: {
color: string;
onColorChange: (color: string) => void;
}) {
const [draft, setDraft] = useState(color);
const debouncedDraft = useDebounceValue(draft, 150);
const previousDraft = usePrevious(debouncedDraft);

useEffect(() => {
if (previousDraft === null || previousDraft === debouncedDraft) {
return;
}
onColorChange(debouncedDraft);
}, [debouncedDraft, previousDraft, onColorChange]);

return (
<>
<HexColorInput
alpha
color={draft}
onChange={setDraft}
className={cn(inputStyles())}
autoFocus
/>
<HexColorPicker
onChange={setDraft}
color={draft}
className="block size-[200px] w-auto"
/>
</>
);
}

function ColorSwatch({
color,
className,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,13 +125,6 @@ export const getStyles = ({
addDefault("edge.connections-filter-out", toCyEdgeStyle(outOfFocusEdgeStyle));
addDefault("edge.out-of-focus", toCyEdgeStyle(outOfFocusEdgeStyle));

rootStyles.push({
selector: "node[__iconUrl]",
style: {
"background-image": "data(__iconUrl)",
},
});

return rootStyles;
};

Expand Down
114 changes: 114 additions & 0 deletions packages/graph-explorer/src/core/StateProvider/displayVertex.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,17 @@ import {
createVertexId,
createVertexType,
type DisplayAttribute,
displayVerticesInCanvasSelector,
getAppStore,
getRawId,
nodesAtom,
nodesSelectedIdsAtom,
schemaAtom,
type SchemaStorageModel,
toNodeMap,
useDisplayVertexFromVertex,
useDisplayVerticesFromVertices,
useSelectedDisplayVertices,
type Vertex,
} from "@/core";
import { formatDate, LABELS } from "@/utils";
Expand Down Expand Up @@ -238,3 +245,110 @@ describe("useDisplayVertexFromVertex", () => {
};
}
});

describe("displayVerticesInCanvasSelector", () => {
it("should map every node in the canvas, keyed by id, in insertion order", () => {
const store = getAppStore();
const vertices = [
createRandomVertex(),
createRandomVertex(),
createRandomVertex(),
];
store.set(nodesAtom, toNodeMap(vertices));

const result = store.get(displayVerticesInCanvasSelector);

expect(result.keys().toArray()).toStrictEqual(vertices.map(v => v.id));
for (const vertex of vertices) {
expect(result.get(vertex.id)?.original).toBe(vertex);
}
});

it("should be empty when the canvas is empty", () => {
const store = getAppStore();
expect(store.get(displayVerticesInCanvasSelector).size).toBe(0);
});

it("should reflect nodes added and removed across mutations", () => {
const store = getAppStore();
const first = createRandomVertex();
const second = createRandomVertex();

store.set(nodesAtom, toNodeMap([first]));
expect(
store.get(displayVerticesInCanvasSelector).keys().toArray(),
).toStrictEqual([first.id]);

store.set(nodesAtom, toNodeMap([first, second]));
expect(
store.get(displayVerticesInCanvasSelector).keys().toArray(),
).toStrictEqual([first.id, second.id]);

store.set(nodesAtom, toNodeMap([second]));
expect(
store.get(displayVerticesInCanvasSelector).keys().toArray(),
).toStrictEqual([second.id]);
});

/**
* Pins the per-id caching that replaced the array-keyed atom family: an
* unchanged node must not be re-derived when other nodes are added.
*/
it("should keep the same DisplayVertex instance for an unchanged node", () => {
const store = getAppStore();
const unchanged = createRandomVertex();

store.set(nodesAtom, toNodeMap([unchanged]));
const before = store.get(displayVerticesInCanvasSelector).get(unchanged.id);

store.set(nodesAtom, toNodeMap([unchanged, createRandomVertex()]));
const after = store.get(displayVerticesInCanvasSelector).get(unchanged.id);

expect(after).toBe(before);
});
});

describe("useSelectedDisplayVertices", () => {
it("should map only the selected nodes", () => {
const selected = createRandomVertex();
const notSelected = createRandomVertex();

const { result } = renderHookWithJotai(
useSelectedDisplayVertices,
store => {
store.set(nodesAtom, toNodeMap([selected, notSelected]));
store.set(nodesSelectedIdsAtom, new Set([selected.id]));
},
);

expect(result.current.map(v => v.id)).toStrictEqual([selected.id]);
});

it("should ignore selected ids that are no longer in the canvas", () => {
const missing = createRandomVertex();

const { result } = renderHookWithJotai(
useSelectedDisplayVertices,
store => {
store.set(nodesSelectedIdsAtom, new Set([missing.id]));
},
);

expect(result.current).toStrictEqual([]);
});
});

describe("useDisplayVerticesFromVertices", () => {
it("should map arbitrary vertices that are not in the canvas", () => {
const vertices = [createRandomVertex(), createRandomVertex()];

const { result } = renderHookWithJotai(() =>
useDisplayVerticesFromVertices(vertices),
);

expect(result.current.keys().toArray()).toStrictEqual(
vertices.map(v => v.id),
);
expect(result.current.get(vertices[0].id)?.original).toBe(vertices[0]);
});
});
Loading