Fix non-square icons squashed instead of scaled to fit

CONTEXT.md

@@ -54,11 +54,11 @@ The classification of a Vertex Type's configured icon into what it takes to rend
 _Avoid_: Icon type (ambiguous with `iconImageType`, the stored MIME string)
 
 **Icon Registry**:
-The single store of resolved icons, keyed by Icon Source Id and shared by every Icon Surface. Holds a color-free artifact — a sanitized SVG string or a raster url — so applying a Vertex Type's color stays a pure transform at the point of use. A plain external store outside React/Jotai, bridged by `useSyncExternalStore`; explicitly **not** TanStack Query, because a per-hook subscription scaled with Vertex Type count and locked up the Schema View at 10k. Resolves a raster url synchronously, allows a failed icon three attempts in total, and never stores a failure as a result. See `docs/adr/20260813-icon-registry-not-react-query.md`.
+The single store of resolved icons, keyed by Icon Source Id and shared by every Icon Surface. Holds a color-free artifact — a sanitized SVG string or a raster url — so applying a Vertex Type's color stays a pure transform at the point of use. Every stored SVG is guaranteed to carry a `viewBox`, synthesized from `width`/`height` when the source omits one, because the surfaces fit icons by `preserveAspectRatio` and an SVG with no `viewBox` has no ratio to fit (issue #2108). A plain external store outside React/Jotai, bridged by `useSyncExternalStore`; explicitly **not** TanStack Query, because a per-hook subscription scaled with Vertex Type count and locked up the Schema View at 10k. Resolves a raster url synchronously, allows a failed icon three attempts in total, and never stores a failure as a result. See `docs/adr/20260813-icon-registry-not-react-query.md`.
 _Avoid_: Icon cache (it is the source of truth for resolution, not a layer in front of one)
 
 **Icon Surface**:
-One of the three places an icon is drawn, which differ in how color is applied and how much they trust the markup. The **canvas** (`useBackgroundImageMap` → cytoscape `background-image`) and the **sandboxed DOM** (`VertexSymbolIcon` → `<image href>`) both render the icon as a separate image document, so CSS cannot reach it: an SVG is passed as a `data:` uri with the color baked into the markup, a raster as its plain url. **Inline DOM** (`VertexIcon`, and the lucide branch of `VertexSymbolIcon`) renders live elements that inherit `color` through `currentColor`, making recolor free. Only trusted lucide geometry is inlined by `VertexSymbolIcon`; `VertexIcon` also inlines sanitized user SVG, which predates that rule and is the known outlier.
+One of the three places an icon is drawn, which differ in how color is applied and how much they trust the markup. The **canvas** (`useBackgroundImageMap` → cytoscape `background-image`) and the **sandboxed DOM** (`VertexSymbolIcon` → `<image href>`) both render the icon as a separate image document, so CSS cannot reach it: an SVG is passed as a `data:` uri with the color baked into the markup, a raster as its plain url — `VertexSymbolIcon` places either directly. Both surfaces inset the icon to 60% of the node and fit it with `preserveAspectRatio`, but only `VertexSymbolIcon` can do so directly, in its own SVG coordinates; cytoscape cannot both preserve a ratio and inset, so the canvas instead wraps whatever `toIconImageUrl` returned — the plain raster url included — in its own padded square SVG and lets the nested `<image>` fit itself (issue #2108). That wrapper is applied on the canvas path alone — adding it inside `toIconImageUrl` would inset twice on the DOM side. **Inline DOM** (`VertexIcon`, and the lucide branch of `VertexSymbolIcon`) renders live elements that inherit `color` through `currentColor`, making recolor free. Only trusted lucide geometry is inlined by `VertexSymbolIcon`; `VertexIcon` also inlines sanitized user SVG, which predates that rule and is the known outlier.
 _Avoid_: Icon renderer (three similarly-named components — `VertexIcon`, `VertexSymbol`, `VertexSymbolIcon` — differ by surface, so name the surface)
 
 **Neighbors**:

docs/adr/20260813-icon-registry-not-react-query.md

@@ -31,18 +31,20 @@ Separately, the DOM surface paid for a workaround it did not need. `VertexSymbol
 
 **2. Icons render per kind, on the surface's own terms.**
 
-| kind     | canvas         | DOM                                        |
-| -------- | -------------- | ------------------------------------------ |
-| Lucide   | baked data uri | `<DynamicIcon>`, live DOM, color inherited |
-| user SVG | baked data uri | `<image href="data:…">`, color baked       |
-| raster   | url            | `<image href>`                             |
+| kind     | canvas                                 | DOM                                        |
+| -------- | -------------------------------------- | ------------------------------------------ |
+| Lucide   | baked data uri, wrapped for aspect fit | `<DynamicIcon>`, live DOM, color inherited |
+| user SVG | baked data uri, wrapped for aspect fit | `<image href="data:…">`, color baked       |
+| raster   | plain url, wrapped for aspect fit      | `<image href>`                             |
 
 Lucide markup is trusted bundled geometry with no ids, defs, or script, so inlining it costs nothing and recoloring becomes synchronous.
 
-Untrusted SVG is deliberately **not** inlined on these surfaces. `<image href="data:…">` renders it as a script-disabled image document (W3C SVG Integration §3.4/§3.6 — an image context disables both script execution and external references). Inlining would trade that browser-enforced boundary for DOMPurify alone, and add id collisions with the `useId()`-generated `clipPath` ids and unsanitized `<style>` blocks. DOMPurify stays; the sandbox stays with it. The cost is that hardcoded fills in custom SVG still do not follow the vertex color (#2105, pre-existing).
+Untrusted SVG is deliberately **not** inlined on these surfaces. `<image href="data:…">` renders it as a script-disabled image document (W3C SVG Integration §3.4/§3.6 — an image context disables both script execution and external references not embedded in the `data:` uri itself). Inlining would trade that browser-enforced boundary for DOMPurify alone, and add id collisions with the `useId()`-generated `clipPath` ids and unsanitized `<style>` blocks. DOMPurify stays; the sandbox stays with it. The cost is that hardcoded fills in custom SVG still do not follow the vertex color (#2105, pre-existing).
 
 This is not codebase-wide: `components/VertexIcon.tsx` inlines sanitized user SVG into the live DOM via `react-inlinesvg`, with no sandbox. It predates this decision and is the outlier, not the pattern to copy.
 
+**Canvas sizing (issue #2108, PR #2142).** Cytoscape cannot both preserve an icon's aspect ratio and inset it to 60% of the node: `background-fit: contain` keeps the ratio but fills the whole node, and the node is an ellipse, so a square-ish icon's corners spill past the shape. The canvas wraps every kind's icon url in its own padded square SVG and lets a nested `<image preserveAspectRatio>` do the fitting — the same mechanism `VertexSymbolIcon` already uses directly. That wrapper is itself a `data:` uri, so it stays within the image-document sandbox above: nesting one `data:`-uri image inside another issues no external request either.
+
 **3. `clip-path` goes on an ancestor `<g>`, never on the nested `<svg>`.**
 
 Chrome renders **nothing** when `clip-path` sits directly on a nested `<svg>`, and nothing about layout or color reveals the fault — the path still reports a correct bounding box and inherited stroke. Since only real pixels expose it, `VertexSymbol.test.tsx` guards the structure instead. The failure mode is a silently invisible icon.

packages/graph-explorer/src/components/Graph/styles/defaultNodeStyle.ts

@@ -4,9 +4,15 @@ const defaultNodeStyle: RenderedNodeStyle = {
   background: "#128EE5",
   backgroundOpacity: 0.4,
   borderColor: "#128EE5",
-  backgroundFit: "none",
-  backgroundWidth: "60%",
-  backgroundHeight: "60%",
+  // The icon image is a square wrapper that already insets the artwork
+  // (issue #2108), so the node only has to fit that square without
+  // distorting it: `contain` is what does that — `auto`/`auto` are
+  // cytoscape's own defaults, restated for intent. `contain` alone, without
+  // the wrapper, is not enough: it fits the whole square node box, and the
+  // node is an ellipse, so a square-ish icon's corners spill past the shape.
+  backgroundFit: "contain",
+  backgroundWidth: "auto",
+  backgroundHeight: "auto",
   borderWidth: 1,
   borderStyle: "solid",
   borderOpacity: 0,

packages/graph-explorer/src/components/VertexIcon.test.tsx

@@ -0,0 +1,63 @@
+// @vitest-environment jsdom
+
+import { render, waitFor } from "@testing-library/react";
+import { describe, expect, it } from "vitest";
+
+import {
+  appDefaultVertexStyle,
+  createVertexType,
+  type VertexStyle,
+} from "@/core";
+
+import VertexIcon from "./VertexIcon";
+
+function renderIcon(overrides: Partial<VertexStyle>) {
+  const vertexStyle: VertexStyle = {
+    ...appDefaultVertexStyle,
+    type: createVertexType("Person"),
+    ...overrides,
+  };
+  return render(<VertexIcon vertexStyle={vertexStyle} />).container;
+}
+
+describe("VertexIcon", () => {
+  // The one thing this branch changed here (issue #2108): without
+  // object-contain, object-fit's default is `fill`, which stretches a
+  // non-square raster to the fixed size-6 box instead of scaling it.
+  it("renders a raster icon with object-contain so it scales instead of stretching", () => {
+    const container = renderIcon({
+      iconUrl: "https://example.test/wide.png",
+      iconImageType: "image/png",
+    });
+
+    const img = container.querySelector("img");
+    expect(img).toBeTruthy();
+    expect(img!.className).toContain("object-contain");
+    expect(img!.getAttribute("src")).toBe("https://example.test/wide.png");
+  });
+
+  it("renders a lucide icon inline so it inherits the vertex color", async () => {
+    const container = renderIcon({
+      iconUrl: "lucide:plane",
+      iconImageType: "image/svg+xml",
+      color: "#FF0000",
+    });
+
+    await waitFor(() => expect(container.querySelector("svg")).toBeTruthy());
+    const icon = container.querySelector("svg");
+    expect(icon).toBeTruthy();
+    expect((icon as SVGElement).style.color).toBe("rgb(255, 0, 0)");
+    // Live DOM, not an <img>/<image> — this is the inline-DOM surface.
+    expect(container.querySelector("img")).toBeNull();
+  });
+
+  it("renders nothing for an unknown lucide reference", () => {
+    const container = renderIcon({
+      iconUrl: "lucide:not-a-real-icon-name-xyz",
+      iconImageType: "image/svg+xml",
+    });
+
+    expect(container.querySelector("svg")).toBeNull();
+    expect(container.querySelector("img")).toBeNull();
+  });
+});

packages/graph-explorer/src/components/VertexIcon.tsx

@@ -3,13 +3,15 @@ import { DynamicIcon } from "lucide-react/dynamic";
 import SVG from "react-inlinesvg";
 
 import { useVertexStyle, type VertexStyle, type VertexType } from "@/core";
+import { ensureSvgViewBox } from "@/core/icons";
 import { cn } from "@/utils";
 import { getLucideName, isValidLucideIconName } from "@/utils/lucideIcons";
 
 function sanitizeSvg(svg: string): string {
-  return DOMPurify.sanitize(svg, {
+  const sanitized = DOMPurify.sanitize(svg, {
     USE_PROFILES: { svg: true, svgFilters: true },
   });
+  return ensureSvgViewBox(sanitized);
 }
 
 interface Props {
@@ -54,7 +56,7 @@ function VertexIcon({ vertexStyle, className, alt }: Props) {
     <img
       src={vertexStyle.iconUrl}
       alt={altText}
-      className={cn("size-6 shrink-0", className)}
+      className={cn("size-6 shrink-0 object-contain", className)}
       style={{ color: vertexStyle.color }}
     />
   );

packages/graph-explorer/src/components/VertexSymbol/VertexSymbol.tsx

@@ -1,21 +1,20 @@
 import { useId } from "react";
 
 import { useVertexStyle, type VertexStyle, type VertexType } from "@/core";
+import { ICON_BOX, ICON_RATIO } from "@/core/icons/iconGeometry";
 import { cn } from "@/utils";
 
 import { resolveShapeGeometry } from "./nodeShapes";
 import { VertexSymbolIcon } from "./VertexSymbolIcon";
 
-const VIEWBOX = 96;
-const ICON_RATIO = 0.6;
 const CANVAS_NODE_SIZE = 24;
 /**
  * How much larger the preview draws things than the graph canvas: the ratio of
- * the SVG viewBox (96) to a canvas node's size in cytoscape units (24). Applied
- * to any canvas-unit length (border width, label font/padding) to render it at
- * preview size.
+ * the SVG viewBox ({@link ICON_BOX}) to a canvas node's size in cytoscape units
+ * (24). Applied to any canvas-unit length (border width, label font/padding)
+ * to render it at preview size.
  */
-export const PREVIEW_SCALE = VIEWBOX / CANVAS_NODE_SIZE;
+export const PREVIEW_SCALE = ICON_BOX / CANVAS_NODE_SIZE;
 
 interface Props {
   vertexStyle: VertexStyle;
@@ -26,11 +25,11 @@ export function VertexSymbol({ vertexStyle, className }: Props) {
   // SVG url(#...) references reject the colons in React's raw useId format.
   const clipId = `vs-${useId().replace(/:/g, "")}`;
   const strokeWidth = vertexStyle.borderWidth * PREVIEW_SCALE;
-  const insetSize = Math.max(1, VIEWBOX - strokeWidth * 2);
+  const insetSize = Math.max(1, ICON_BOX - strokeWidth * 2);
   const geometry = resolveShapeGeometry(vertexStyle.shape, insetSize);
 
-  const iconSize = VIEWBOX * ICON_RATIO;
-  const iconOffset = (VIEWBOX - iconSize) / 2;
+  const iconSize = ICON_BOX * ICON_RATIO;
+  const iconOffset = (ICON_BOX - iconSize) / 2;
 
   // The shape is rendered twice: once filled/stroked, once as the icon's
   // clipPath. clipPath children must be shape elements directly — a wrapping
@@ -41,7 +40,7 @@ export function VertexSymbol({ vertexStyle, className }: Props) {
 
   return (
     <svg
-      viewBox={`0 0 ${VIEWBOX} ${VIEWBOX}`}
+      viewBox={`0 0 ${ICON_BOX} ${ICON_BOX}`}
       className={cn("size-9 shrink-0", className)}
       // An inline SVG is required for the clipPath and nested icon.
       // eslint-disable-next-line jsx-a11y/prefer-tag-over-role

packages/graph-explorer/src/core/icons/iconGeometry.test.ts

@@ -0,0 +1,23 @@
+import { describe, expect, it } from "vitest";
+
+import { encodeSvg, ICON_BOX, ICON_RATIO } from "./iconGeometry";
+
+describe("iconGeometry", () => {
+  // ICON_BOX and ICON_RATIO are the single source of truth for how much of the
+  // icon's square box the artwork occupies. The canvas (useBackgroundImageMap)
+  // and the style preview (VertexSymbol) both import them rather than
+  // declaring their own copy — this pins the values themselves, so a future
+  // edit to one file cannot silently drift from the other without also
+  // changing this test.
+  it("insets the icon to 60% of a box that is 4x a canvas node (24 units)", () => {
+    expect(ICON_RATIO).toBe(0.6);
+    expect(ICON_BOX).toBe(96);
+    expect(ICON_BOX / 24).toBe(4);
+  });
+
+  it("percent-encodes svg markup as a data uri", () => {
+    expect(encodeSvg("<svg>&</svg>")).toBe(
+      "data:image/svg+xml;utf8," + encodeURIComponent("<svg>&</svg>"),
+    );
+  });
+});

packages/graph-explorer/src/core/icons/iconGeometry.ts

@@ -0,0 +1,19 @@
+/**
+ * Shared inset geometry for every icon surface. The canvas
+ * (`useBackgroundImageMap`) and the style preview (`VertexSymbol`) each fit an
+ * icon into a square box by `preserveAspectRatio`, and both must agree on how
+ * much of that box the icon occupies — changing one without the other would
+ * silently desync what the preview shows from what the canvas renders.
+ *
+ * {@link ICON_BOX} is not arbitrary: it is exactly 4x a canvas node's size in
+ * cytoscape units (24), the same ratio `VertexSymbol`'s own viewBox already
+ * uses for everything else it scales (border width, label font/padding).
+ */
+export const ICON_BOX = 96;
+
+/** Fraction of {@link ICON_BOX} the icon occupies, leaving room for the shape's curve. */
+export const ICON_RATIO = 0.6;
+
+export function encodeSvg(svgContent: string): string {
+  return "data:image/svg+xml;utf8," + encodeURIComponent(svgContent);
+}

packages/graph-explorer/src/core/icons/iconImageUrl.test.ts

@@ -29,11 +29,14 @@ describe("toIconImageUrl", () => {
     expect(decode(result)).toContain("<path");
   });
 
-  it("sizes the svg to the cytoscape node size", () => {
+  // Sizing belongs to the consumer, which fits the icon by `preserveAspectRatio`
+  // against its own `viewBox`. Overriding the intrinsic size here would fight
+  // that, and forcing a square would reintroduce issue #2108.
+  it("leaves the svg's own size and viewBox alone", () => {
     const result = toIconImageUrl({ kind: "svg", svg: SVG }, "#FF0000");
 
-    expect(decode(result)).toContain('width="24"');
-    expect(decode(result)).toContain('height="24"');
+    expect(decode(result)).toContain('viewBox="0 0 24 24"');
+    expect(decode(result)).not.toContain('width="24"');
   });
 
   // The color reaches a currentColor-authored icon through CSS inheritance, so
@@ -103,4 +106,22 @@ describe("toIconImageUrl", () => {
 
     expect(red).not.toBe(blue);
   });
+
+  // Issue #2108: sizing is the consumer's job. Both consumers place the icon
+  // with `preserveAspectRatio`, which fits the icon against its own `viewBox`,
+  // so overriding its intrinsic size here would only fight that — and forcing
+  // a square would bake in the very distortion the issue is about.
+  describe("non-square icons (issue #2108)", () => {
+    const WIDE_SVG = `<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 400 100"><rect width="400" height="100"/></svg>`;
+
+    it("leaves the icon's own geometry untouched", () => {
+      const result = decode(
+        toIconImageUrl({ kind: "svg", svg: WIDE_SVG }, "#FF0000"),
+      );
+
+      expect(result).toContain('viewBox="0 0 400 100"');
+      expect(result).not.toContain('width="24"');
+      expect(result).not.toContain('height="24"');
+    });
+  });
 });

packages/graph-explorer/src/core/icons/iconImageUrl.ts

@@ -1,7 +1,6 @@
 import type { ResolvedIcon } from "./iconRegistry";
 
-/** Intrinsic size; both consumers scale from it. Matches the cytoscape node size. */
-const ICON_SIZE = "24";
+import { encodeSvg } from "./iconGeometry";
 
 /**
  * Pure transform to an image url.
@@ -10,38 +9,38 @@ const ICON_SIZE = "24";
  * separate image document — the cytoscape `background-image`, and the `<image>`
  * element used for untrusted SVG — which CSS cannot reach. Icons rendered as
  * live DOM inherit `color` instead and never call this.
+ *
+ * No size is applied. Both consumers place the icon with
+ * `preserveAspectRatio`, which needs the icon's own `viewBox` to fit against;
+ * overriding its intrinsic size here would only fight that.
+ *
+ * Do not wrap the result in another inset SVG here: `VertexSymbolIcon` already
+ * insets to 60% in its own SVG coordinates, so this stays a single fit for
+ * every caller. Only the canvas path (`useBackgroundImageMap`) needs its own
+ * wrapper, because cytoscape — unlike an inline SVG — cannot fit an image by
+ * `preserveAspectRatio` itself.
  */
 export function toIconImageUrl(icon: ResolvedIcon, color: string): string {
   switch (icon.kind) {
     case "raster":
       return icon.url;
     case "svg":
-      return encodeSvg(applySizeAndColor(icon.svg, color));
+      return encodeSvg(applyColor(icon.svg, color));
   }
 }
 
-function applySizeAndColor(svgContent: string, color: string): string {
-  const doc = new DOMParser().parseFromString(svgContent, "application/xml");
-  const root = doc.documentElement;
-  root.setAttribute("width", ICON_SIZE);
-  root.setAttribute("height", ICON_SIZE);
-  applyColor(root, color);
-  return new XMLSerializer().serializeToString(root);
-}
-
 /**
  * Sets `color` on the root so `currentColor`-authored icons follow the vertex
  * color by inheritance; hardcoded fills are left untouched. Isolated here so
  * switching to "tint everything" stays a one-function change (issue #2105).
  */
-function applyColor(root: Element, color: string): void {
+function applyColor(svgContent: string, color: string): string {
+  const doc = new DOMParser().parseFromString(svgContent, "application/xml");
+  const root = doc.documentElement;
   const existing = root.getAttribute("style");
   root.setAttribute(
     "style",
     existing ? `${existing};color:${color}` : `color:${color}`,
   );
-}
-
-function encodeSvg(svgContent: string): string {
-  return "data:image/svg+xml;utf8," + encodeURIComponent(svgContent);
+  return new XMLSerializer().serializeToString(root);
 }