From c667c53a9ed811bd96370328a277d580f4d69d75 Mon Sep 17 00:00:00 2001 From: Kris McGinnes Date: Fri, 10 Jul 2026 11:38:32 -0500 Subject: [PATCH 1/5] Remove broken round-polygon shapes from the picker and coerce stored values (#1922) Cytoscape's round-polygon renderer degenerates at 24px for six shapes (round-triangle, round-pentagon, round-hexagon, round-heptagon, round-octagon, round-tag), rendering them as blobs and causing edges to disappear. This removes them from the shape picker so they can't be newly selected, and coerces any previously-stored values to round-rectangle at both the storage-read boundary (ReadTransform) and the styling-import boundary (Zod transform). The shapes remain in SHAPE_STYLES so older files still parse without rejection. --- .../core/StateProvider/graphStyles.test.ts | 31 ++++++++ .../src/core/StateProvider/graphStyles.ts | 25 +++++++ .../src/core/StateProvider/storageAtoms.ts | 5 +- .../vertexStylesTransform.test.ts | 75 +++++++++++++++++++ .../StateProvider/vertexStylesTransform.ts | 29 +++++++ .../src/core/styling/stylingParser.test.ts | 24 +++++- .../src/core/styling/stylingParser.ts | 3 +- .../src/modules/NodesStyling/nodeShape.ts | 6 -- 8 files changed, 188 insertions(+), 10 deletions(-) create mode 100644 packages/graph-explorer/src/core/StateProvider/vertexStylesTransform.test.ts create mode 100644 packages/graph-explorer/src/core/StateProvider/vertexStylesTransform.ts diff --git a/packages/graph-explorer/src/core/StateProvider/graphStyles.test.ts b/packages/graph-explorer/src/core/StateProvider/graphStyles.test.ts index 0957a2df8..63c9687d9 100644 --- a/packages/graph-explorer/src/core/StateProvider/graphStyles.test.ts +++ b/packages/graph-explorer/src/core/StateProvider/graphStyles.test.ts @@ -8,8 +8,10 @@ import { DbState, renderHookWithState } from "@/utils/testing"; import { appDefaultEdgeStyle, appDefaultVertexStyle, + coerceBrokenShape, edgeStyleAtom, type EdgeStyleStorage, + type ShapeStyle, useEdgeStyling, useVertexStyling, vertexStyleAtom, @@ -460,3 +462,32 @@ describe("edgeStyleAtom", () => { ); }); }); + +describe("coerceBrokenShape", () => { + const BROKEN: ShapeStyle[] = [ + "round-triangle", + "round-pentagon", + "round-hexagon", + "round-heptagon", + "round-octagon", + "round-tag", + ]; + + it.each(BROKEN)("coerces %s to round-rectangle", shape => { + expect(coerceBrokenShape(shape)).toBe("round-rectangle"); + }); + + const SAFE: ShapeStyle[] = [ + "ellipse", + "rectangle", + "round-rectangle", + "round-diamond", + "star", + "diamond", + "triangle", + ]; + + it.each(SAFE)("passes %s through unchanged", shape => { + expect(coerceBrokenShape(shape)).toBe(shape); + }); +}); diff --git a/packages/graph-explorer/src/core/StateProvider/graphStyles.ts b/packages/graph-explorer/src/core/StateProvider/graphStyles.ts index 3ce752246..49f5db851 100644 --- a/packages/graph-explorer/src/core/StateProvider/graphStyles.ts +++ b/packages/graph-explorer/src/core/StateProvider/graphStyles.ts @@ -45,6 +45,31 @@ export const SHAPE_STYLES = [ ] as const; export type ShapeStyle = (typeof SHAPE_STYLES)[number]; +/** + * Shapes that cytoscape's round-polygon renderer draws incorrectly at small + * node size (24px): their corner computation degenerates, rendering as a blob + * and producing invalid edge endpoints that cause edges to disappear. These are + * kept in {@link SHAPE_STYLES} so older files still parse, but are coerced to + * `round-rectangle` at every read boundary. + */ +const BROKEN_ROUND_POLYGON_SHAPES: ReadonlySet = new Set([ + "round-triangle", + "round-pentagon", + "round-hexagon", + "round-heptagon", + "round-octagon", + "round-tag", +]); + +const BROKEN_SHAPE_REPLACEMENT: ShapeStyle = "round-rectangle"; + +/** Coerces a broken round-polygon shape to `round-rectangle`, passing all others through. */ +export function coerceBrokenShape(shape: ShapeStyle): ShapeStyle { + return BROKEN_ROUND_POLYGON_SHAPES.has(shape) + ? BROKEN_SHAPE_REPLACEMENT + : shape; +} + export const LINE_STYLES = ["solid", "dashed", "dotted"] as const; export type LineStyle = (typeof LINE_STYLES)[number]; diff --git a/packages/graph-explorer/src/core/StateProvider/storageAtoms.ts b/packages/graph-explorer/src/core/StateProvider/storageAtoms.ts index 97243c7b1..1162f0e0d 100644 --- a/packages/graph-explorer/src/core/StateProvider/storageAtoms.ts +++ b/packages/graph-explorer/src/core/StateProvider/storageAtoms.ts @@ -19,6 +19,7 @@ import { defaultSchemaViewLayout, transformSchemaViewLayout, } from "./schemaViewLayoutDefaults"; +import { transformVertexStyles } from "./vertexStylesTransform"; // Run migrations before the atoms preload so they read the migrated data. // Each migration owns its own failure reporting (surfacing through the @@ -95,7 +96,7 @@ const [ atomWithLocalForage( "user-vertex-styles", new Map(), - { reconcile: reconcileMapByKey }, + { reconcile: reconcileMapByKey, transform: transformVertexStyles }, ), /** User-defined edge style overrides, keyed by type. */ atomWithLocalForage( @@ -111,7 +112,7 @@ const [ atomWithLocalForage( "shared-vertex-styles", new Map(), - { reconcile: reconcileMapByKey }, + { reconcile: reconcileMapByKey, transform: transformVertexStyles }, ), /** Shared edge styles, loaded from a styling file. */ atomWithLocalForage( diff --git a/packages/graph-explorer/src/core/StateProvider/vertexStylesTransform.test.ts b/packages/graph-explorer/src/core/StateProvider/vertexStylesTransform.test.ts new file mode 100644 index 000000000..14c5fc539 --- /dev/null +++ b/packages/graph-explorer/src/core/StateProvider/vertexStylesTransform.test.ts @@ -0,0 +1,75 @@ +import { createVertexType, type VertexType } from "@/core/entities"; + +import type { ShapeStyle, VertexStyleStorage } from "./graphStyles"; + +import { transformVertexStyles } from "./vertexStylesTransform"; + +function vertexMap( + entries: Array<[string, Omit]>, +): Map { + return new Map( + entries.map(([name, fields]) => { + const type = createVertexType(name); + return [type, { ...fields, type }]; + }), + ); +} + +describe("transformVertexStyles", () => { + it("returns the same reference when no shapes need coercion", () => { + const styles = vertexMap([ + ["A", { shape: "ellipse" }], + ["B", { color: "#fff" }], + ]); + + expect(transformVertexStyles(styles)).toBe(styles); + }); + + it("coerces each broken round-polygon shape to round-rectangle", () => { + const broken: ShapeStyle[] = [ + "round-triangle", + "round-pentagon", + "round-hexagon", + "round-heptagon", + "round-octagon", + "round-tag", + ]; + + for (const shape of broken) { + const styles = vertexMap([["X", { shape }]]); + const result = transformVertexStyles(styles); + expect(result.get(createVertexType("X"))!.shape).toBe("round-rectangle"); + } + }); + + it("does not coerce round-rectangle or round-diamond", () => { + const styles = vertexMap([ + ["A", { shape: "round-rectangle" }], + ["B", { shape: "round-diamond" }], + ]); + + expect(transformVertexStyles(styles)).toBe(styles); + }); + + it("preserves other fields on a coerced entry", () => { + const styles = vertexMap([ + ["A", { shape: "round-tag", color: "#ff0000", borderWidth: 2 }], + ]); + + const result = transformVertexStyles(styles); + const entry = result.get(createVertexType("A"))!; + expect(entry.shape).toBe("round-rectangle"); + expect(entry.color).toBe("#ff0000"); + expect(entry.borderWidth).toBe(2); + }); + + it("handles an empty map", () => { + const styles = vertexMap([]); + expect(transformVertexStyles(styles)).toBe(styles); + }); + + it("passes through entries without a shape field", () => { + const styles = vertexMap([["A", { color: "#abc" }]]); + expect(transformVertexStyles(styles)).toBe(styles); + }); +}); diff --git a/packages/graph-explorer/src/core/StateProvider/vertexStylesTransform.ts b/packages/graph-explorer/src/core/StateProvider/vertexStylesTransform.ts new file mode 100644 index 000000000..b388223d5 --- /dev/null +++ b/packages/graph-explorer/src/core/StateProvider/vertexStylesTransform.ts @@ -0,0 +1,29 @@ +import type { VertexType } from "../entities"; + +import { coerceBrokenShape, type VertexStyleStorage } from "./graphStyles"; + +/** + * ReadTransform for vertex style maps: coerces broken round-polygon shapes to + * `round-rectangle` at load time. Entries without a `shape` field are passed + * through unchanged. Returns the same reference when no coercion was needed. + */ +export function transformVertexStyles( + styles: Map, +): Map { + let changed = false; + const result = new Map(); + + for (const [type, entry] of styles) { + if (entry.shape !== undefined) { + const coerced = coerceBrokenShape(entry.shape); + if (coerced !== entry.shape) { + result.set(type, { ...entry, shape: coerced }); + changed = true; + continue; + } + } + result.set(type, entry); + } + + return changed ? result : styles; +} diff --git a/packages/graph-explorer/src/core/styling/stylingParser.test.ts b/packages/graph-explorer/src/core/styling/stylingParser.test.ts index 3084307f6..efe3a7ea2 100644 --- a/packages/graph-explorer/src/core/styling/stylingParser.test.ts +++ b/packages/graph-explorer/src/core/styling/stylingParser.test.ts @@ -64,12 +64,34 @@ describe("style enum validation", () => { "none", ]; + const BROKEN_SHAPES = new Set([ + "round-triangle", + "round-pentagon", + "round-hexagon", + "round-heptagon", + "round-octagon", + "round-tag", + ]); + test.each(SHAPES)("accepts shape %s", shape => { const result = parseStylingPayload({ vertices: { A: { shape } }, edges: {}, }); - expect(result.vertexStyles.get(createVertexType("A"))!.shape).toBe(shape); + const expected = BROKEN_SHAPES.has(shape) ? "round-rectangle" : shape; + expect(result.vertexStyles.get(createVertexType("A"))!.shape).toBe( + expected, + ); + }); + + test("coerces broken round-polygon shapes to round-rectangle on import", () => { + const result = parseStylingPayload({ + vertices: { A: { shape: "round-tag" } }, + edges: {}, + }); + expect(result.vertexStyles.get(createVertexType("A"))!.shape).toBe( + "round-rectangle", + ); }); test("rejects an unknown shape, listing the valid options", () => { diff --git a/packages/graph-explorer/src/core/styling/stylingParser.ts b/packages/graph-explorer/src/core/styling/stylingParser.ts index b38e2348d..7c3844e18 100644 --- a/packages/graph-explorer/src/core/styling/stylingParser.ts +++ b/packages/graph-explorer/src/core/styling/stylingParser.ts @@ -8,6 +8,7 @@ import { createEdgeType, createVertexType } from "@/core/entities"; import { FileEnvelopeError } from "@/core/fileEnvelope"; import { ARROW_STYLES, + coerceBrokenShape, type EdgeStyleStorage, LINE_STYLES, SHAPE_STYLES, @@ -133,7 +134,7 @@ const vertexEntrySchema = z displayLabel: z.string().optional(), displayNameAttribute: z.string().optional(), longDisplayNameAttribute: z.string().optional(), - shape: z.enum(SHAPE_STYLES).optional(), + shape: z.enum(SHAPE_STYLES).transform(coerceBrokenShape).optional(), backgroundOpacity: z.number().optional(), borderWidth: z.number().optional(), borderColor: z.string().optional(), diff --git a/packages/graph-explorer/src/modules/NodesStyling/nodeShape.ts b/packages/graph-explorer/src/modules/NodesStyling/nodeShape.ts index 36931a92a..633fbd4fb 100644 --- a/packages/graph-explorer/src/modules/NodesStyling/nodeShape.ts +++ b/packages/graph-explorer/src/modules/NodesStyling/nodeShape.ts @@ -11,13 +11,7 @@ export const NODE_SHAPE = [ { label: "Rectangle", value: "rectangle" }, { label: "Rhomboid", value: "rhomboid" }, { label: "Round Diamond", value: "round-diamond" }, - { label: "Round Heptagon", value: "round-heptagon" }, - { label: "Round Hexagon", value: "round-hexagon" }, - { label: "Round Octagon", value: "round-octagon" }, - { label: "Round Pentagon", value: "round-pentagon" }, { label: "Round Rectangle", value: "roundrectangle" }, - { label: "Round Tag", value: "round-tag" }, - { label: "Round Triangle", value: "round-triangle" }, { label: "Star", value: "star" }, { label: "Tag", value: "tag" }, { label: "Triangle", value: "triangle" }, From 1281a5c028b197cd688aa3a35d7373fed69b7849 Mon Sep 17 00:00:00 2001 From: Kris McGinnes Date: Fri, 10 Jul 2026 13:24:05 -0500 Subject: [PATCH 2/5] Fix circular-dep TDZ crash and picker alias mismatch Move coerceBrokenShape() and the broken-shape set into vertexStylesTransform.ts so the import from storageAtoms is type-only into graphStyles.ts, breaking the runtime cycle that caused a ReferenceError during initialization. Change the coercion target from "round-rectangle" to "roundrectangle" (the value NODE_SHAPE uses for the picker) so the style dialog displays "Round Rectangle" instead of a blank field for coerced entries. --- .../core/StateProvider/graphStyles.test.ts | 7 ++--- .../src/core/StateProvider/graphStyles.ts | 25 ------------------ .../vertexStylesTransform.test.ts | 11 ++++---- .../StateProvider/vertexStylesTransform.ts | 26 ++++++++++++++++++- .../src/core/styling/stylingParser.test.ts | 6 ++--- .../src/core/styling/stylingParser.ts | 2 +- 6 files changed, 39 insertions(+), 38 deletions(-) diff --git a/packages/graph-explorer/src/core/StateProvider/graphStyles.test.ts b/packages/graph-explorer/src/core/StateProvider/graphStyles.test.ts index 63c9687d9..0f883b567 100644 --- a/packages/graph-explorer/src/core/StateProvider/graphStyles.test.ts +++ b/packages/graph-explorer/src/core/StateProvider/graphStyles.test.ts @@ -8,7 +8,6 @@ import { DbState, renderHookWithState } from "@/utils/testing"; import { appDefaultEdgeStyle, appDefaultVertexStyle, - coerceBrokenShape, edgeStyleAtom, type EdgeStyleStorage, type ShapeStyle, @@ -17,6 +16,7 @@ import { vertexStyleAtom, type VertexStyleStorage, } from "./graphStyles"; +import { coerceBrokenShape } from "./vertexStylesTransform"; function createExpectedVertex(existing: VertexStyleStorage) { return { @@ -473,13 +473,14 @@ describe("coerceBrokenShape", () => { "round-tag", ]; - it.each(BROKEN)("coerces %s to round-rectangle", shape => { - expect(coerceBrokenShape(shape)).toBe("round-rectangle"); + it.each(BROKEN)("coerces %s to roundrectangle", shape => { + expect(coerceBrokenShape(shape)).toBe("roundrectangle"); }); const SAFE: ShapeStyle[] = [ "ellipse", "rectangle", + "roundrectangle", "round-rectangle", "round-diamond", "star", diff --git a/packages/graph-explorer/src/core/StateProvider/graphStyles.ts b/packages/graph-explorer/src/core/StateProvider/graphStyles.ts index 49f5db851..3ce752246 100644 --- a/packages/graph-explorer/src/core/StateProvider/graphStyles.ts +++ b/packages/graph-explorer/src/core/StateProvider/graphStyles.ts @@ -45,31 +45,6 @@ export const SHAPE_STYLES = [ ] as const; export type ShapeStyle = (typeof SHAPE_STYLES)[number]; -/** - * Shapes that cytoscape's round-polygon renderer draws incorrectly at small - * node size (24px): their corner computation degenerates, rendering as a blob - * and producing invalid edge endpoints that cause edges to disappear. These are - * kept in {@link SHAPE_STYLES} so older files still parse, but are coerced to - * `round-rectangle` at every read boundary. - */ -const BROKEN_ROUND_POLYGON_SHAPES: ReadonlySet = new Set([ - "round-triangle", - "round-pentagon", - "round-hexagon", - "round-heptagon", - "round-octagon", - "round-tag", -]); - -const BROKEN_SHAPE_REPLACEMENT: ShapeStyle = "round-rectangle"; - -/** Coerces a broken round-polygon shape to `round-rectangle`, passing all others through. */ -export function coerceBrokenShape(shape: ShapeStyle): ShapeStyle { - return BROKEN_ROUND_POLYGON_SHAPES.has(shape) - ? BROKEN_SHAPE_REPLACEMENT - : shape; -} - export const LINE_STYLES = ["solid", "dashed", "dotted"] as const; export type LineStyle = (typeof LINE_STYLES)[number]; diff --git a/packages/graph-explorer/src/core/StateProvider/vertexStylesTransform.test.ts b/packages/graph-explorer/src/core/StateProvider/vertexStylesTransform.test.ts index 14c5fc539..199adde35 100644 --- a/packages/graph-explorer/src/core/StateProvider/vertexStylesTransform.test.ts +++ b/packages/graph-explorer/src/core/StateProvider/vertexStylesTransform.test.ts @@ -38,14 +38,15 @@ describe("transformVertexStyles", () => { for (const shape of broken) { const styles = vertexMap([["X", { shape }]]); const result = transformVertexStyles(styles); - expect(result.get(createVertexType("X"))!.shape).toBe("round-rectangle"); + expect(result.get(createVertexType("X"))!.shape).toBe("roundrectangle"); } }); - it("does not coerce round-rectangle or round-diamond", () => { + it("does not coerce roundrectangle, round-rectangle, or round-diamond", () => { const styles = vertexMap([ - ["A", { shape: "round-rectangle" }], - ["B", { shape: "round-diamond" }], + ["A", { shape: "roundrectangle" }], + ["B", { shape: "round-rectangle" }], + ["C", { shape: "round-diamond" }], ]); expect(transformVertexStyles(styles)).toBe(styles); @@ -58,7 +59,7 @@ describe("transformVertexStyles", () => { const result = transformVertexStyles(styles); const entry = result.get(createVertexType("A"))!; - expect(entry.shape).toBe("round-rectangle"); + expect(entry.shape).toBe("roundrectangle"); expect(entry.color).toBe("#ff0000"); expect(entry.borderWidth).toBe(2); }); diff --git a/packages/graph-explorer/src/core/StateProvider/vertexStylesTransform.ts b/packages/graph-explorer/src/core/StateProvider/vertexStylesTransform.ts index b388223d5..391f52ef1 100644 --- a/packages/graph-explorer/src/core/StateProvider/vertexStylesTransform.ts +++ b/packages/graph-explorer/src/core/StateProvider/vertexStylesTransform.ts @@ -1,6 +1,30 @@ import type { VertexType } from "../entities"; +import type { ShapeStyle, VertexStyleStorage } from "./graphStyles"; -import { coerceBrokenShape, type VertexStyleStorage } from "./graphStyles"; +/** + * Shapes that cytoscape's round-polygon renderer draws incorrectly at small + * node size (24px): their corner computation degenerates, rendering as a blob + * and producing invalid edge endpoints that cause edges to disappear. These are + * kept in {@link SHAPE_STYLES} so older files still parse, but are coerced to + * `round-rectangle` at every read boundary. + */ +const BROKEN_ROUND_POLYGON_SHAPES: ReadonlySet = new Set([ + "round-triangle", + "round-pentagon", + "round-hexagon", + "round-heptagon", + "round-octagon", + "round-tag", +]); + +const BROKEN_SHAPE_REPLACEMENT: ShapeStyle = "roundrectangle"; + +/** Coerces a broken round-polygon shape to `round-rectangle`, passing all others through. */ +export function coerceBrokenShape(shape: ShapeStyle): ShapeStyle { + return BROKEN_ROUND_POLYGON_SHAPES.has(shape) + ? BROKEN_SHAPE_REPLACEMENT + : shape; +} /** * ReadTransform for vertex style maps: coerces broken round-polygon shapes to diff --git a/packages/graph-explorer/src/core/styling/stylingParser.test.ts b/packages/graph-explorer/src/core/styling/stylingParser.test.ts index efe3a7ea2..bd2adf1db 100644 --- a/packages/graph-explorer/src/core/styling/stylingParser.test.ts +++ b/packages/graph-explorer/src/core/styling/stylingParser.test.ts @@ -78,19 +78,19 @@ describe("style enum validation", () => { vertices: { A: { shape } }, edges: {}, }); - const expected = BROKEN_SHAPES.has(shape) ? "round-rectangle" : shape; + const expected = BROKEN_SHAPES.has(shape) ? "roundrectangle" : shape; expect(result.vertexStyles.get(createVertexType("A"))!.shape).toBe( expected, ); }); - test("coerces broken round-polygon shapes to round-rectangle on import", () => { + test("coerces broken round-polygon shapes to roundrectangle on import", () => { const result = parseStylingPayload({ vertices: { A: { shape: "round-tag" } }, edges: {}, }); expect(result.vertexStyles.get(createVertexType("A"))!.shape).toBe( - "round-rectangle", + "roundrectangle", ); }); diff --git a/packages/graph-explorer/src/core/styling/stylingParser.ts b/packages/graph-explorer/src/core/styling/stylingParser.ts index 7c3844e18..ba20d8305 100644 --- a/packages/graph-explorer/src/core/styling/stylingParser.ts +++ b/packages/graph-explorer/src/core/styling/stylingParser.ts @@ -8,12 +8,12 @@ import { createEdgeType, createVertexType } from "@/core/entities"; import { FileEnvelopeError } from "@/core/fileEnvelope"; import { ARROW_STYLES, - coerceBrokenShape, type EdgeStyleStorage, LINE_STYLES, SHAPE_STYLES, type VertexStyleStorage, } from "@/core/StateProvider/graphStyles"; +import { coerceBrokenShape } from "@/core/StateProvider/vertexStylesTransform"; import { typedEntries } from "@/utils"; // --- Format identity --- From 9fad9e037f74c4aa17651aa522965ad31dbaa27f Mon Sep 17 00:00:00 2001 From: Kris McGinnes Date: Fri, 10 Jul 2026 13:51:04 -0500 Subject: [PATCH 3/5] Map each broken shape to its non-round sibling and drop the import-time coercion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback applied: - Each broken round-polygon shape now maps to its sharp-cornered counterpart (round-hexagon → hexagon, etc.) instead of collapsing all six to a single flat replacement. This preserves the user's visual-differentiation intent. - Remove the Zod .transform() from stylingParser.ts — import stores the value as-is and the ReadTransform coerces on next load, keeping the import path non-destructive and the original recoverable. - Add debug logging to transformVertexStyles so coercions are observable. - Convert the transform test loop to it.each for better failure diagnostics. - Add ADR documenting the coerce-vs-reject decision and reversal condition. - Update the ReadTransform ADR with transformVertexStyles as a second consumer. --- ...ead-time-transform-for-persisted-values.md | 2 + ...710-coerce-retired-round-polygon-shapes.md | 41 +++++++++++ .../core/StateProvider/graphStyles.test.ts | 32 --------- .../vertexStylesTransform.test.ts | 70 +++++++++++++------ .../StateProvider/vertexStylesTransform.ts | 38 +++++----- .../src/core/styling/stylingParser.test.ts | 24 +------ .../src/core/styling/stylingParser.ts | 3 +- 7 files changed, 116 insertions(+), 94 deletions(-) create mode 100644 docs/adr/20260710-coerce-retired-round-polygon-shapes.md diff --git a/docs/adr/20260709-read-time-transform-for-persisted-values.md b/docs/adr/20260709-read-time-transform-for-persisted-values.md index a61638c6e..e87f90e55 100644 --- a/docs/adr/20260709-read-time-transform-for-persisted-values.md +++ b/docs/adr/20260709-read-time-transform-for-persisted-values.md @@ -12,6 +12,8 @@ Persisted state in IndexedDB (via `atomWithLocalForage`) reloads in its stored s Reshape the value **on read**, via a `transform` option on `atomWithLocalForage`: `transform: (loaded: T) => T` runs on the preloaded value before it seeds the atom. The transform lives beside its type (`transformGraphViewLayout` / `transformSchemaViewLayout`, sharing `transformLegacySidebarItem`) and is wired onto the atom in `storageAtoms.ts`. +- **Updated 2026-07-10:** `transformVertexStyles` (in `vertexStylesTransform.ts`) is a second consumer, applied to both `user-vertex-styles` and `shared-vertex-styles`. It coerces retired round-polygon shapes to their non-round counterpart (see ADR `coerce-retired-round-polygon-shapes`). Values arriving through file import are stored verbatim — the same ReadTransform coerces them on the next load, so both entry points (persisted storage and imported files) converge on the same coercion without the import path needing its own transform. + Two decisions here are not obvious from the code: 1. **No write-back.** The corrected value lives in memory; the stale value stays in storage until an unrelated write rewrites the key. This is fine because the transform is pure and total, so re-running it every load is free — convergence buys nothing. It is also why the transform must never throw or do I/O: it seeds atom init with no failure channel. diff --git a/docs/adr/20260710-coerce-retired-round-polygon-shapes.md b/docs/adr/20260710-coerce-retired-round-polygon-shapes.md new file mode 100644 index 000000000..2ee0d77f2 --- /dev/null +++ b/docs/adr/20260710-coerce-retired-round-polygon-shapes.md @@ -0,0 +1,41 @@ +# ADR — Coerce retired round-polygon shapes at read boundaries + +- **Status:** Accepted +- **Date:** 2026-07-10 +- **Related:** ADR `read-time-transform-for-persisted-values` (the mechanism); issue #1922 (the defect); PR #1886 (exposed the shapes in the picker). Cytoscape 3.34.0 is the latest published version. + +## Context + +Cytoscape's round-polygon canvas renderer degenerates at 24px (our node size) for six shapes: `round-triangle`, `round-pentagon`, `round-hexagon`, `round-heptagon`, `round-octagon`, `round-tag`. The corner computation collapses, rendering each as a formless blob and producing invalid edge-endpoint coordinates that cause connected edges to disappear. `round-rectangle` and `round-diamond` are unaffected (they use a different code path). No upstream fix exists; the one related issue (cytoscape/cytoscape.js#3282) describes a crash, not this visual collapse. + +The shapes became selectable in the UI via #1886, so users may have stored them in IndexedDB (user-vertex-styles, shared-vertex-styles) or in exported styling files. We must handle that existing data. + +## Decision + +Keep the six values in `SHAPE_STYLES` and `ShapeStyle` so older styling files still pass Zod validation on import. Remove them from the picker (`NODE_SHAPE`) so they can't be newly selected. At the storage-read boundary, coerce each to its non-round counterpart via a `ReadTransform` on both vertex-styles atoms (`transformVertexStyles`). The mapping preserves the user's visual-differentiation intent: + +| Broken shape | Coerced to | +| ---------------- | ---------- | +| `round-triangle` | `triangle` | +| `round-pentagon` | `pentagon` | +| `round-hexagon` | `hexagon` | +| `round-heptagon` | `heptagon` | +| `round-octagon` | `octagon` | +| `round-tag` | `tag` | + +Import stores the value verbatim — the ReadTransform coerces it on the next read from storage, so the import path stays pure and the original persisted value is never overwritten. + +## Considered Options + +- **Coerce at read boundaries, no write-back (chosen).** Non-destructive: stored originals survive, so a future cytoscape fix can restore them by removing the transform. Each broken shape maps to its sharp-cornered sibling, preserving shape semantics. +- **Reject on import.** Would fail the whole file under the atomic parser contract (`20260624-styling-file-format.md`), punishing users for data the app itself produced. Not acceptable. +- **Remove from `SHAPE_STYLES` / `ShapeStyle`.** Same as rejection: the Zod enum would fail old files that contain the values. +- **Coerce at import time (Zod `.transform()`).** Bakes the coerced value into storage permanently and loses the original on re-export. Makes the transform irreversible — a cytoscape fix can no longer restore the user's original choice. +- **Patch cytoscape (fork or monkey-patch).** High maintenance burden for a rendering detail that may be fixed upstream. Deferred unless upstream remains broken long-term. +- **Coerce to a single flat replacement (`round-rectangle`).** Collapses six visually distinct shapes into one, losing differentiation intent. Per-shape mapping costs one extra `Map` entry per shape and preserves semantics. + +## Consequences + +- Users who stored a broken shape see its non-round counterpart after upgrading — a strict improvement over the previous blob-with-no-edges. +- Styling-file round-trips are lossless in storage (never written back) but lossy in memory (the read value differs from the stored value). This is the read-time-transform ADR's purity contract applied to a new case. +- If cytoscape fixes the rendering, reversal is: restore the six entries in `NODE_SHAPE`, delete `BROKEN_SHAPE_REPLACEMENTS` and the `transformVertexStyles` ReadTransform. Stored originals reappear intact. diff --git a/packages/graph-explorer/src/core/StateProvider/graphStyles.test.ts b/packages/graph-explorer/src/core/StateProvider/graphStyles.test.ts index 0f883b567..0957a2df8 100644 --- a/packages/graph-explorer/src/core/StateProvider/graphStyles.test.ts +++ b/packages/graph-explorer/src/core/StateProvider/graphStyles.test.ts @@ -10,13 +10,11 @@ import { appDefaultVertexStyle, edgeStyleAtom, type EdgeStyleStorage, - type ShapeStyle, useEdgeStyling, useVertexStyling, vertexStyleAtom, type VertexStyleStorage, } from "./graphStyles"; -import { coerceBrokenShape } from "./vertexStylesTransform"; function createExpectedVertex(existing: VertexStyleStorage) { return { @@ -462,33 +460,3 @@ describe("edgeStyleAtom", () => { ); }); }); - -describe("coerceBrokenShape", () => { - const BROKEN: ShapeStyle[] = [ - "round-triangle", - "round-pentagon", - "round-hexagon", - "round-heptagon", - "round-octagon", - "round-tag", - ]; - - it.each(BROKEN)("coerces %s to roundrectangle", shape => { - expect(coerceBrokenShape(shape)).toBe("roundrectangle"); - }); - - const SAFE: ShapeStyle[] = [ - "ellipse", - "rectangle", - "roundrectangle", - "round-rectangle", - "round-diamond", - "star", - "diamond", - "triangle", - ]; - - it.each(SAFE)("passes %s through unchanged", shape => { - expect(coerceBrokenShape(shape)).toBe(shape); - }); -}); diff --git a/packages/graph-explorer/src/core/StateProvider/vertexStylesTransform.test.ts b/packages/graph-explorer/src/core/StateProvider/vertexStylesTransform.test.ts index 199adde35..9b91a8b72 100644 --- a/packages/graph-explorer/src/core/StateProvider/vertexStylesTransform.test.ts +++ b/packages/graph-explorer/src/core/StateProvider/vertexStylesTransform.test.ts @@ -2,7 +2,10 @@ import { createVertexType, type VertexType } from "@/core/entities"; import type { ShapeStyle, VertexStyleStorage } from "./graphStyles"; -import { transformVertexStyles } from "./vertexStylesTransform"; +import { + coerceBrokenShape, + transformVertexStyles, +} from "./vertexStylesTransform"; function vertexMap( entries: Array<[string, Omit]>, @@ -15,6 +18,33 @@ function vertexMap( ); } +describe("coerceBrokenShape", () => { + it.each([ + ["round-triangle", "triangle"], + ["round-pentagon", "pentagon"], + ["round-hexagon", "hexagon"], + ["round-heptagon", "heptagon"], + ["round-octagon", "octagon"], + ["round-tag", "tag"], + ] as [ShapeStyle, ShapeStyle][])("coerces %s to %s", (broken, expected) => { + expect(coerceBrokenShape(broken)).toBe(expected); + }); + + it.each([ + "ellipse", + "rectangle", + "roundrectangle", + "round-rectangle", + "round-diamond", + "star", + "diamond", + "triangle", + "tag", + ] as ShapeStyle[])("passes %s through unchanged", shape => { + expect(coerceBrokenShape(shape)).toBe(shape); + }); +}); + describe("transformVertexStyles", () => { it("returns the same reference when no shapes need coercion", () => { const styles = vertexMap([ @@ -25,28 +55,26 @@ describe("transformVertexStyles", () => { expect(transformVertexStyles(styles)).toBe(styles); }); - it("coerces each broken round-polygon shape to round-rectangle", () => { - const broken: ShapeStyle[] = [ - "round-triangle", - "round-pentagon", - "round-hexagon", - "round-heptagon", - "round-octagon", - "round-tag", - ]; - - for (const shape of broken) { - const styles = vertexMap([["X", { shape }]]); + it.each([ + ["round-triangle", "triangle"], + ["round-pentagon", "pentagon"], + ["round-hexagon", "hexagon"], + ["round-heptagon", "heptagon"], + ["round-octagon", "octagon"], + ["round-tag", "tag"], + ] as [ShapeStyle, ShapeStyle][])( + "coerces %s to %s in a stored map", + (broken, expected) => { + const styles = vertexMap([["X", { shape: broken }]]); const result = transformVertexStyles(styles); - expect(result.get(createVertexType("X"))!.shape).toBe("roundrectangle"); - } - }); + expect(result.get(createVertexType("X"))!.shape).toBe(expected); + }, + ); - it("does not coerce roundrectangle, round-rectangle, or round-diamond", () => { + it("does not coerce round-rectangle or round-diamond", () => { const styles = vertexMap([ - ["A", { shape: "roundrectangle" }], - ["B", { shape: "round-rectangle" }], - ["C", { shape: "round-diamond" }], + ["A", { shape: "round-rectangle" }], + ["B", { shape: "round-diamond" }], ]); expect(transformVertexStyles(styles)).toBe(styles); @@ -59,7 +87,7 @@ describe("transformVertexStyles", () => { const result = transformVertexStyles(styles); const entry = result.get(createVertexType("A"))!; - expect(entry.shape).toBe("roundrectangle"); + expect(entry.shape).toBe("tag"); expect(entry.color).toBe("#ff0000"); expect(entry.borderWidth).toBe(2); }); diff --git a/packages/graph-explorer/src/core/StateProvider/vertexStylesTransform.ts b/packages/graph-explorer/src/core/StateProvider/vertexStylesTransform.ts index 391f52ef1..6a4c1f089 100644 --- a/packages/graph-explorer/src/core/StateProvider/vertexStylesTransform.ts +++ b/packages/graph-explorer/src/core/StateProvider/vertexStylesTransform.ts @@ -1,3 +1,5 @@ +import { logger } from "@/utils"; + import type { VertexType } from "../entities"; import type { ShapeStyle, VertexStyleStorage } from "./graphStyles"; @@ -6,30 +8,31 @@ import type { ShapeStyle, VertexStyleStorage } from "./graphStyles"; * node size (24px): their corner computation degenerates, rendering as a blob * and producing invalid edge endpoints that cause edges to disappear. These are * kept in {@link SHAPE_STYLES} so older files still parse, but are coerced to - * `round-rectangle` at every read boundary. + * their sharp-cornered counterpart at every read boundary. + * + * Each broken shape maps to its non-round sibling to preserve the user's + * visual-differentiation intent (a round-hexagon becomes a hexagon, not a + * generic rectangle). */ -const BROKEN_ROUND_POLYGON_SHAPES: ReadonlySet = new Set([ - "round-triangle", - "round-pentagon", - "round-hexagon", - "round-heptagon", - "round-octagon", - "round-tag", +const BROKEN_SHAPE_REPLACEMENTS = new Map([ + ["round-triangle", "triangle"], + ["round-pentagon", "pentagon"], + ["round-hexagon", "hexagon"], + ["round-heptagon", "heptagon"], + ["round-octagon", "octagon"], + ["round-tag", "tag"], ]); -const BROKEN_SHAPE_REPLACEMENT: ShapeStyle = "roundrectangle"; - -/** Coerces a broken round-polygon shape to `round-rectangle`, passing all others through. */ +/** Coerces a broken round-polygon shape to its non-round counterpart, passing all others through. */ export function coerceBrokenShape(shape: ShapeStyle): ShapeStyle { - return BROKEN_ROUND_POLYGON_SHAPES.has(shape) - ? BROKEN_SHAPE_REPLACEMENT - : shape; + return BROKEN_SHAPE_REPLACEMENTS.get(shape) ?? shape; } /** * ReadTransform for vertex style maps: coerces broken round-polygon shapes to - * `round-rectangle` at load time. Entries without a `shape` field are passed - * through unchanged. Returns the same reference when no coercion was needed. + * their non-round counterpart at load time. Entries without a `shape` field are + * passed through unchanged. Returns the same reference when no coercion was + * needed. */ export function transformVertexStyles( styles: Map, @@ -41,6 +44,9 @@ export function transformVertexStyles( if (entry.shape !== undefined) { const coerced = coerceBrokenShape(entry.shape); if (coerced !== entry.shape) { + logger.debug( + `[vertex-styles] Coercing broken shape "${entry.shape}" to "${coerced}" for type "${type}"`, + ); result.set(type, { ...entry, shape: coerced }); changed = true; continue; diff --git a/packages/graph-explorer/src/core/styling/stylingParser.test.ts b/packages/graph-explorer/src/core/styling/stylingParser.test.ts index bd2adf1db..3084307f6 100644 --- a/packages/graph-explorer/src/core/styling/stylingParser.test.ts +++ b/packages/graph-explorer/src/core/styling/stylingParser.test.ts @@ -64,34 +64,12 @@ describe("style enum validation", () => { "none", ]; - const BROKEN_SHAPES = new Set([ - "round-triangle", - "round-pentagon", - "round-hexagon", - "round-heptagon", - "round-octagon", - "round-tag", - ]); - test.each(SHAPES)("accepts shape %s", shape => { const result = parseStylingPayload({ vertices: { A: { shape } }, edges: {}, }); - const expected = BROKEN_SHAPES.has(shape) ? "roundrectangle" : shape; - expect(result.vertexStyles.get(createVertexType("A"))!.shape).toBe( - expected, - ); - }); - - test("coerces broken round-polygon shapes to roundrectangle on import", () => { - const result = parseStylingPayload({ - vertices: { A: { shape: "round-tag" } }, - edges: {}, - }); - expect(result.vertexStyles.get(createVertexType("A"))!.shape).toBe( - "roundrectangle", - ); + expect(result.vertexStyles.get(createVertexType("A"))!.shape).toBe(shape); }); test("rejects an unknown shape, listing the valid options", () => { diff --git a/packages/graph-explorer/src/core/styling/stylingParser.ts b/packages/graph-explorer/src/core/styling/stylingParser.ts index ba20d8305..b38e2348d 100644 --- a/packages/graph-explorer/src/core/styling/stylingParser.ts +++ b/packages/graph-explorer/src/core/styling/stylingParser.ts @@ -13,7 +13,6 @@ import { SHAPE_STYLES, type VertexStyleStorage, } from "@/core/StateProvider/graphStyles"; -import { coerceBrokenShape } from "@/core/StateProvider/vertexStylesTransform"; import { typedEntries } from "@/utils"; // --- Format identity --- @@ -134,7 +133,7 @@ const vertexEntrySchema = z displayLabel: z.string().optional(), displayNameAttribute: z.string().optional(), longDisplayNameAttribute: z.string().optional(), - shape: z.enum(SHAPE_STYLES).transform(coerceBrokenShape).optional(), + shape: z.enum(SHAPE_STYLES).optional(), backgroundOpacity: z.number().optional(), borderWidth: z.number().optional(), borderColor: z.string().optional(), From fe212eb03cb94728c6937ff6b96bd53f9c5bca50 Mon Sep 17 00:00:00 2001 From: Kris McGinnes Date: Fri, 10 Jul 2026 17:43:57 -0500 Subject: [PATCH 4/5] Add backward-compat integration tests for retired shape coercion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Proves the end-to-end pipeline: legacy data seeded in localForage with a broken round-polygon shape → atomWithLocalForage with the ReadTransform → atom value has the shape coerced to its non-round counterpart. Also verifies no write-back occurs (stored original is preserved). --- .../vertexStylesTransform.test.ts | 95 +++++++++++++++++++ 1 file changed, 95 insertions(+) diff --git a/packages/graph-explorer/src/core/StateProvider/vertexStylesTransform.test.ts b/packages/graph-explorer/src/core/StateProvider/vertexStylesTransform.test.ts index 9b91a8b72..35fafb6c0 100644 --- a/packages/graph-explorer/src/core/StateProvider/vertexStylesTransform.test.ts +++ b/packages/graph-explorer/src/core/StateProvider/vertexStylesTransform.test.ts @@ -1,7 +1,11 @@ +import { createStore } from "jotai"; +import localforage from "localforage"; + import { createVertexType, type VertexType } from "@/core/entities"; import type { ShapeStyle, VertexStyleStorage } from "./graphStyles"; +import { atomWithLocalForage, reconcileMapByKey } from "./atomWithLocalForage"; import { coerceBrokenShape, transformVertexStyles, @@ -18,6 +22,97 @@ function vertexMap( ); } +/** + * BACKWARD COMPATIBILITY — PERSISTED DATA + * + * Vertex styles are persisted to IndexedDB via localForage. Prior to this + * change, the six round-polygon shapes (round-triangle, round-pentagon, + * round-hexagon, round-heptagon, round-octagon, round-tag) could be stored + * via the shape picker (exposed in #1886). These shapes render incorrectly in + * cytoscape at 24px and are now coerced to their non-round counterpart at + * load time via a ReadTransform. + * + * DO NOT delete or weaken these tests without confirming that the shapes are + * no longer in the wild or that cytoscape has fixed the rendering defect. + */ +describe("backward compatibility: retired round-polygon shapes in storage", () => { + beforeEach(async () => { + await localforage.clear(); + }); + + it("coerces a stored broken shape through the full atomWithLocalForage pipeline", async () => { + const store = createStore(); + const key = "test-vertex-styles-compat"; + + const legacyData = new Map([ + [ + createVertexType("Airport"), + { + type: createVertexType("Airport"), + shape: "round-hexagon" as ShapeStyle, + color: "#ff0000", + }, + ], + [ + createVertexType("City"), + { + type: createVertexType("City"), + shape: "ellipse" as ShapeStyle, + color: "#00ff00", + }, + ], + ]); + + await localforage.setItem(key, legacyData); + + const atom = await atomWithLocalForage( + key, + new Map(), + { reconcile: reconcileMapByKey, transform: transformVertexStyles }, + ); + + const value = store.get(atom); + + expect(value.get(createVertexType("Airport"))).toStrictEqual({ + type: createVertexType("Airport"), + shape: "hexagon", + color: "#ff0000", + }); + expect(value.get(createVertexType("City"))).toStrictEqual({ + type: createVertexType("City"), + shape: "ellipse", + color: "#00ff00", + }); + }); + + it("does not write back the coerced value to storage", async () => { + const key = "test-vertex-styles-no-writeback"; + + const legacyData = new Map([ + [ + createVertexType("Airport"), + { + type: createVertexType("Airport"), + shape: "round-tag" as ShapeStyle, + }, + ], + ]); + + await localforage.setItem(key, legacyData); + + await atomWithLocalForage(key, new Map(), { + reconcile: reconcileMapByKey, + transform: transformVertexStyles, + }); + + const storedAfterLoad = + await localforage.getItem>(key); + expect(storedAfterLoad!.get(createVertexType("Airport"))!.shape).toBe( + "round-tag", + ); + }); +}); + describe("coerceBrokenShape", () => { it.each([ ["round-triangle", "triangle"], From dee6af26f5aa25fc526c647dce7103beb5caf6b1 Mon Sep 17 00:00:00 2001 From: Kris McGinnes Date: Fri, 10 Jul 2026 17:55:10 -0500 Subject: [PATCH 5/5] Defer Map allocation in transformVertexStyles until first coercion The common case (no broken shapes stored) now avoids allocating a Map that is immediately discarded, matching the pattern used by the sibling transformGraphViewLayout. --- .../src/core/StateProvider/vertexStylesTransform.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/graph-explorer/src/core/StateProvider/vertexStylesTransform.ts b/packages/graph-explorer/src/core/StateProvider/vertexStylesTransform.ts index 6a4c1f089..c0b10c990 100644 --- a/packages/graph-explorer/src/core/StateProvider/vertexStylesTransform.ts +++ b/packages/graph-explorer/src/core/StateProvider/vertexStylesTransform.ts @@ -37,23 +37,22 @@ export function coerceBrokenShape(shape: ShapeStyle): ShapeStyle { export function transformVertexStyles( styles: Map, ): Map { - let changed = false; - const result = new Map(); + let result: Map | null = null; for (const [type, entry] of styles) { if (entry.shape !== undefined) { const coerced = coerceBrokenShape(entry.shape); if (coerced !== entry.shape) { + if (!result) { + result = new Map(styles); + } logger.debug( `[vertex-styles] Coercing broken shape "${entry.shape}" to "${coerced}" for type "${type}"`, ); result.set(type, { ...entry, shape: coerced }); - changed = true; - continue; } } - result.set(type, entry); } - return changed ? result : styles; + return result ?? styles; }