Fix non-square icons squashed instead of scaled to fit

CONTEXT.md

@@ -54,7 +54,7 @@ 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, plus its natural width/height so a non-square icon scales instead of stretching — 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. Every icon kind resolves asynchronously (a raster's dimensions are measured by loading it), 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**:

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

@@ -3,13 +3,16 @@ import { DynamicIcon } from "lucide-react/dynamic";
 import SVG from "react-inlinesvg";
 
 import { useVertexStyle, type VertexStyle, type VertexType } from "@/core";
+import { ensureSvgViewBox, SVG_ALLOWED_ATTR } 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 },
+    ALLOWED_ATTR: SVG_ALLOWED_ATTR,
   });
+  return ensureSvgViewBox(sanitized);
 }
 
 interface Props {
@@ -54,7 +57,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/core/icons/aspectFit.test.ts

@@ -0,0 +1,17 @@
+import { describe, expect, it } from "vitest";
+
+import { fitAspectRatio } from "./aspectFit";
+
+describe("fitAspectRatio", () => {
+  it("caps the wider axis at base and shrinks the shorter one proportionally", () => {
+    expect(fitAspectRatio(400, 100, 24)).toEqual([24, 6]);
+  });
+
+  it("caps the taller axis at base and shrinks the shorter one proportionally", () => {
+    expect(fitAspectRatio(100, 400, 24)).toEqual([6, 24]);
+  });
+
+  it("returns the base for both axes when already square", () => {
+    expect(fitAspectRatio(100, 100, 24)).toEqual([24, 24]);
+  });
+});

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

@@ -0,0 +1,16 @@
+/**
+ * Scales a width/height pair to fit a `base`-sized box, preserving aspect
+ * ratio: the longer axis becomes exactly `base`, the shorter one shrinks
+ * proportionally. Returns raw numbers — each caller formats its own unit
+ * (an absolute pixel size, a cytoscape percentage, ...).
+ */
+export function fitAspectRatio(
+  width: number,
+  height: number,
+  base: number,
+): [width: number, height: number] {
+  const aspectRatio = width / height;
+  return aspectRatio >= 1
+    ? [base, base / aspectRatio]
+    : [base * aspectRatio, base];
+}

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

@@ -103,4 +103,40 @@ describe("toIconImageUrl", () => {
 
     expect(red).not.toBe(blue);
   });
+
+  // Issue #2108: forcing every icon's intrinsic size to a fixed 24x24 square
+  // bakes a mismatched-aspect letterbox into the rasterized image, which the
+  // consumer's own aspect-aware background-width/height then stretches a
+  // second time — distorting a non-square icon worse than doing nothing.
+  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>`;
+    const TALL_SVG = `<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 100 400"><rect width="100" height="400"/></svg>`;
+
+    it("scales a wide icon's intrinsic width/height to its real aspect ratio", () => {
+      const result = toIconImageUrl(
+        { kind: "svg", svg: WIDE_SVG, width: 400, height: 100 },
+        "#FF0000",
+      );
+
+      expect(decode(result)).toContain('width="24"');
+      expect(decode(result)).toContain('height="6"');
+    });
+
+    it("scales a tall icon's intrinsic width/height to its real aspect ratio", () => {
+      const result = toIconImageUrl(
+        { kind: "svg", svg: TALL_SVG, width: 100, height: 400 },
+        "#FF0000",
+      );
+
+      expect(decode(result)).toContain('width="6"');
+      expect(decode(result)).toContain('height="24"');
+    });
+
+    it("falls back to a 24x24 square when dimensions are unknown", () => {
+      const result = toIconImageUrl({ kind: "svg", svg: WIDE_SVG }, "#FF0000");
+
+      expect(decode(result)).toContain('width="24"');
+      expect(decode(result)).toContain('height="24"');
+    });
+  });
 });

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

@@ -1,7 +1,9 @@
 import type { ResolvedIcon } from "./iconRegistry";
 
-/** Intrinsic size; both consumers scale from it. Matches the cytoscape node size. */
-const ICON_SIZE = "24";
+import { fitAspectRatio } from "./aspectFit";
+
+/** Intrinsic size baseline; matches the cytoscape node size. */
+const ICON_SIZE = 24;
 
 /**
  * Pure transform to an image url.
@@ -16,19 +18,44 @@ export function toIconImageUrl(icon: ResolvedIcon, color: string): string {
     case "raster":
       return icon.url;
     case "svg":
-      return encodeSvg(applySizeAndColor(icon.svg, color));
+      return encodeSvg(
+        applySizeAndColor(icon.svg, color, icon.width, icon.height),
+      );
   }
 }
 
-function applySizeAndColor(svgContent: string, color: string): string {
+/**
+ * Sets the SVG's own intrinsic width/height. This must preserve the icon's
+ * real aspect ratio (scaled to fit a 24px box), not force a fixed square:
+ * forcing a square here bakes a mismatched-aspect letterbox into the
+ * rasterized image, which the consumer's own aspect-aware background-width/
+ * height then stretches a second time, distorting worse than doing nothing.
+ */
+function applySizeAndColor(
+  svgContent: string,
+  color: string,
+  naturalWidth?: number,
+  naturalHeight?: number,
+): string {
   const doc = new DOMParser().parseFromString(svgContent, "application/xml");
   const root = doc.documentElement;
-  root.setAttribute("width", ICON_SIZE);
-  root.setAttribute("height", ICON_SIZE);
+  const [width, height] = fitToIconSize(naturalWidth, naturalHeight);
+  root.setAttribute("width", String(width));
+  root.setAttribute("height", String(height));
   applyColor(root, color);
   return new XMLSerializer().serializeToString(root);
 }
 
+function fitToIconSize(
+  naturalWidth?: number,
+  naturalHeight?: number,
+): [width: number, height: number] {
+  if (!naturalWidth || !naturalHeight) {
+    return [ICON_SIZE, ICON_SIZE];
+  }
+  return fitAspectRatio(naturalWidth, naturalHeight, ICON_SIZE);
+}
+
 /**
  * Sets `color` on the root so `currentColor`-authored icons follow the vertex
  * color by inheritance; hardcoded fills are left untouched. Isolated here so

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

@@ -49,7 +49,7 @@ describe("iconRegistry", () => {
     iconRegistry.request([source]);
     await settle();
 
-    expect(iconRegistry.getSnapshot().get(iconSourceId(source)!)).toStrictEqual(
+    expect(iconRegistry.getSnapshot().get(iconSourceId(source)!)).toMatchObject(
       {
         kind: "raster",
         url: "https://example.test/a.png",
@@ -58,18 +58,23 @@ describe("iconRegistry", () => {
     expect(fetch).not.toBeCalled();
   });
 
-  // A url needs no resolution, so making the consumer wait a render for it
-  // would be a pointless async round trip.
-  it("resolves a raster icon synchronously", () => {
+  // Measuring a raster's natural size requires loading it, so — unlike a url,
+  // which needs no resolution — this can no longer settle in the same tick.
+  it("measures a raster icon's natural dimensions", async () => {
     const source = classifyIconSource({
       iconUrl: "https://example.test/a.png",
       iconImageType: "image/png",
     });
 
     iconRegistry.request([source]);
+    await settle();
 
-    expect(iconRegistry.getSnapshot().has(iconSourceId(source)!)).toBe(true);
-    expect(iconRegistry.pendingCount).toBe(0);
+    expect(iconRegistry.getSnapshot().get(iconSourceId(source)!)).toMatchObject(
+      {
+        width: expect.any(Number),
+        height: expect.any(Number),
+      },
+    );
   });
 
   it("fetches and sanitizes a remote svg", async () => {

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

@@ -4,11 +4,13 @@ import { logger } from "@/utils";
 import { getLucideSvgString } from "@/utils/lucideIcons";
 
 import { type IconSource, type IconSourceId, iconSourceId } from "./iconSource";
+import { SVG_ALLOWED_ATTR } from "./svgSanitize";
+import { ensureSvgViewBox } from "./svgViewBox";
 
 /** An icon resolved to a renderable form, with no color applied yet. */
 export type ResolvedIcon =
-  | { kind: "raster"; url: string }
-  | { kind: "svg"; svg: string };
+  | { kind: "raster"; url: string; width?: number; height?: number }
+  | { kind: "svg"; svg: string; width?: number; height?: number };
 
 /**
  * Bounded so a permanently broken icon stops re-fetching, but not one-shot: a
@@ -48,30 +50,17 @@ class IconRegistry {
 
   /** Idempotent: starts only what is neither resolved, running, nor exhausted. */
   request(sources: Iterable<IconSource>): void {
-    let next: Map<IconSourceId, ResolvedIcon> | undefined;
-
     for (const source of sources) {
       const id = iconSourceId(source);
       if (id === null || this.#resolved.has(id) || this.#inFlight.has(id)) {
         continue;
       }
-      if (source.kind === "raster") {
-        // A url needs no work, so resolve it now rather than a render later.
-        next ??= new Map(this.#resolved);
-        next.set(id, { kind: "raster", url: source.url });
-        continue;
-      }
       if ((this.#failures.get(id) ?? 0) >= MAX_ATTEMPTS) {
         continue;
       }
       this.#inFlight.add(id);
       void this.#resolve(id, source, this.#epoch);
     }
-
-    if (next) {
-      this.#resolved = next;
-      this.#notify();
-    }
   }
 
   /**
@@ -139,29 +128,97 @@ async function resolveIconSource(
   switch (source.kind) {
     case "none":
       return null;
-    case "raster":
-      return { kind: "raster", url: source.url };
+    case "raster": {
+      const dimensions = await measureImageDimensions(source.url);
+      return { kind: "raster", url: source.url, ...dimensions };
+    }
     case "lucide": {
-      const svg = await getLucideSvgString(source.name);
-      if (svg === null) {
+      const raw = await getLucideSvgString(source.name);
+      if (raw === null) {
         logger.warn("Unknown lucide icon", source.name);
         return null;
       }
-      return { kind: "svg", svg };
+      const svg = ensureSvgViewBox(raw);
+      const dimensions = extractSvgDimensions(svg);
+      return { kind: "svg", svg, ...dimensions };
     }
     case "svg": {
       // Untrusted: a user-supplied SVG, sanitized before it is used anywhere.
       const response = await fetch(source.url);
-      const svg = DOMPurify.sanitize(await response.text(), {
+      const sanitized = DOMPurify.sanitize(await response.text(), {
         USE_PROFILES: { svg: true, svgFilters: true },
+        ALLOWED_ATTR: SVG_ALLOWED_ATTR,
       });
       // A 404 body sanitizes to something that is not SVG. Reject it here so
       // consumers can treat `ResolvedIcon` as renderable.
-      return isParseableSvg(svg) ? { kind: "svg", svg } : null;
+      if (!isParseableSvg(sanitized)) {
+        return null;
+      }
+      const svg = ensureSvgViewBox(sanitized);
+      const dimensions = extractSvgDimensions(svg);
+      return { kind: "svg", svg, ...dimensions };
     }
   }
 }
 
+async function measureImageDimensions(
+  url: string,
+): Promise<{ width?: number; height?: number }> {
+  try {
+    // Never rejects — `onerror` resolves to a fallback instead. This only
+    // guards a synchronous throw from constructing `Image` or setting `src`.
+    return await new Promise(resolve => {
+      const img = new Image();
+      img.onload = () => {
+        resolve({ width: img.naturalWidth, height: img.naturalHeight });
+      };
+      img.onerror = () => {
+        resolve({});
+      };
+      img.src = url;
+    });
+  } catch (_e) {
+    return {};
+  }
+}
+
+function extractSvgDimensions(svg: string): {
+  width?: number;
+  height?: number;
+} {
+  try {
+    const doc = new DOMParser().parseFromString(svg, "application/xml");
+    const root = doc.documentElement;
+
+    if (root.localName !== "svg") {
+      return {};
+    }
+
+    const width = parseFloat(root.getAttribute("width") ?? "");
+    const height = parseFloat(root.getAttribute("height") ?? "");
+
+    if (!isNaN(width) && !isNaN(height)) {
+      return { width, height };
+    }
+
+    const viewBox = root.getAttribute("viewBox");
+    if (viewBox) {
+      const parts = viewBox.split(/\s+/);
+      if (parts.length >= 4) {
+        const vbWidth = parseFloat(parts[2]);
+        const vbHeight = parseFloat(parts[3]);
+        if (!isNaN(vbWidth) && !isNaN(vbHeight)) {
+          return { width: vbWidth, height: vbHeight };
+        }
+      }
+    }
+
+    return {};
+  } catch (_e) {
+    return {};
+  }
+}
+
 function isParseableSvg(svg: string): boolean {
   const doc = new DOMParser().parseFromString(svg, "application/xml");
   return (