Fix non-square icons squashed instead of scaled to fit

[mjuarros]

Description

Non-square custom vertex icons (raster images and SVG) were stretched into a square instead of scaled down to fit, distorting them. This happened on both the graph canvas and DOM surfaces (search results, legend).

Root cause: cytoscape cannot both preserve an image's aspect ratio and inset it. background-fit: contain keeps the ratio but fills the whole node box, and the nodes are ellipses, so a square-ish icon's corners spill outside the shape. Setting background-width/background-height percentages insets the icon but overrides the image's natural dimensions on both axes, so drawInscribedImage squashes it — the image is already distorted before background-fit runs. The DOM rendered images with object-fit: fill (the default), which behaves identically.

How it's fixed

Padded SVG wrapper on the canvas. The inset is baked into a square SVG that wraps the icon, and the nested <image preserveAspectRatio="xMidYMid meet"> does the fitting. Cytoscape is then handed a square to contain with auto axes, so it never forces a dimension. This needs no measurement and works the same for raster and SVG.

The wrapper is applied on the canvas path only. VertexSymbolIcon already insets to 60% in its own SVG coordinates, so wrapping inside toIconImageUrl would apply the inset twice and render at 36%. The wrapper is skipped for a raster url that isn't a data: uri: nesting an external reference inside a data: SVG puts it in the SVG-as-image sandbox, which fetches nothing external, so it would render blank rather than distorted. Every icon url reachable through the app today is lucide: or data:image/*;base64,, so this is defensive rather than a case anyone hits.

viewBox synthesis. A viewBox is synthesized from width/height when an SVG has none. This is load-bearing, not a nicety: without a viewBox the nested image has no intrinsic ratio for preserveAspectRatio to fit, so it fills the padded box and comes out square — the original bug. A plain <svg width height> export, which is what many icon exporters produce, hits exactly this. A percentage-dimension SVG with no viewBox (width="100%") has no recoverable aspect ratio at all and is left square — a documented, tested limitation rather than a guess.

DOM surface. VertexIcon gains object-contain so its <img> respects the ratio.

Shared geometry and sanitization. ICON_BOX, ICON_RATIO, and the inset math are centralized in iconGeometry.ts so the canvas wrapper and the style preview (VertexSymbol) can't silently drift apart. The DOMPurify-sanitize-then-viewBox-synthesize sequence, previously duplicated in VertexIcon.tsx and the icon registry, is now one shared sanitizeSvg.

Robustness. A stored icon url that isn't well-formed UTF-16 is skipped (with a one-time warning) instead of throwing during style computation — that throw previously took down the whole app via the route-level error boundary. The (icon, color) render cache key uses a NUL separator instead of |, since a | in a color value could otherwise collide two different cache entries.

How to read

1. src/modules/GraphViewer/useBackgroundImageMap.ts — the wrapper, the non-data: fallback, and why cytoscape can't do this alone
2. src/components/Graph/styles/defaultNodeStyle.ts — contain with auto axes, replacing the fixed 60% percentages
3. src/core/icons/svgViewBox.ts — viewBox synthesis and the shared sanitizeSvg
4. src/core/icons/iconGeometry.ts — the geometry constants and inset math shared between the canvas and the preview
5. src/core/icons/iconImageUrl.ts — bakes the color only; sizing is the consumer's job
6. src/components/VertexIcon.tsx — object-contain on the DOM path

Validation

All unit tests pass, including regressions for: the no-viewBox case (the one input the wrapper alone doesn't handle), a non-data: raster url, an unescaped < in the wrapped url, a malformed icon url, and the cache-key collision.

Verified by rendering this branch's actual output on cytoscape 3.34.3 with the real node geometry, measuring the drawn icon as a pixel diff against an icon-free baseline (24px node at 8× zoom, so 192px is the full node):

icon	drawn box	ratio	width vs node
4:1, with viewBox	116×30	3.87	60.4%
4:1, no viewBox	116×30	3.87	60.4%
1:4, no viewBox	30×116	0.26	15.6%
1:1, no viewBox	116×116	1.00	60.4%

Ratios match the sources (±antialiasing) and the 60% inset is preserved, including for the square control. Chrome headless only.

Also manually verified in the running app on the graph canvas, uploading a 4:1 SVG that declares width/height and no viewBox — the case the wrapper alone gets wrong — which renders at its natural aspect ratio. @kmcginnes independently verified against a live air_routes container, measuring geometry off the rendered canvas: 60% inset exact to within half a device pixel across 4:1/1:4/1:1 SVG, 4:1 PNG, and a viewBox-less 4:1 SVG.

Before Fix

After Fix

• Closes Non-square icons are squashed instead of scaled to fit #2108

[kmcginnes]

Your root-cause analysis is correct. There's a shorter route to the same pixels, though.

Wrap every icon, raster included, in a padded square SVG and let preserveAspectRatio do the fitting:

background-fit: contain
background-width: auto
background-height: auto

<svg viewBox="0 0 100 100">
  <image href="<icon>" x="20" y="20" width="60" height="60"
         preserveAspectRatio="xMidYMid meet"/>
</svg>

The prototype I sent produces pixel-identical output to your branch across 4:1, 1:4, 1:1, 10:1 and a raster, in Chrome, Firefox and Safari 27. No measurement anywhere.

Note that contain with auto alone is not enough: it fits the 24x24 bounding box, and our nodes are ellipses, so a square-ish icon's corners spill outside the circle. The padded wrapper is what keeps the 60% inset.

This also lands the canvas on the same mechanism VertexSymbol already uses on the DOM side.

What it removes

measureImageDimensions and new Image(), async raster resolution, width?/height? on ResolvedIcon, fitAspectRatio, computeAspectRatioAwareDimensions, extractSvgDimensions, ensureSvgViewBox, and the Image double in setupTests.ts. Around 40 lines instead of 448, none async. Most of my review notes disappear with that code rather than needing fixes.

Still needed

Delete svgSanitize.ts and both ALLOWED_ATTR arguments. The option never takes effect: DOMPurify reads it at purify.cjs.js:742, then line 806 runs ALLOWED_ATTR = create(null) inside if (USE_PROFILES) and rebuilds from the profile sets. Only ADD_ATTR composes. Output is byte-identical with and without it across an 8-SVG diff, and the svg profile at line 316 already allows width, height, viewbox and preserveaspectratio, so the doc comment's premise is wrong too. Worse than dead: the list omits d, points, transform and stroke-width, so removing the apparently-redundant USE_PROFILES later would blank every path-based icon.

Restore the \u0000 cache key separator in useBackgroundImageMap.ts. | occurs in both the id and the color, so two entries can collide and swap icons between types.

Comment the background-fit line with why contain alone fails, plus a line in CONTEXT.md.

Fix the PR description's SVG bullet. It says SVGs "already carry a viewBox, so they letterbox themselves naturally," but the branch rewrites every SVG's intrinsic size. Your commit message has it right.

Rebase too. The safeSessionStorage.test.ts failure is a stale base, fixed by 5a50963.

Pre-existing

The review surfaced several older issues in files you touched. #2182 is filed; I'm tracking the rest. None block this.

[mjuarros]

Thanks — the prototype made this much easier to reason about, and four of the five action items were straightforward. All are applied and the branch is rebased onto main (now at 66603850).

Applied

Deleted svgSanitize.ts and both ALLOWED_ATTR arguments. You're right, and it's worse than dead. I verified against the installed DOMPurify 3.4.15: line 804 resolves ALLOWED_ATTR from config, then line 870 — inside if (USE_PROFILES) — runs ALLOWED_ATTR = create(null) and rebuilds purely from the profile sets, so the option never applied. The svg profile already allows width, height, viewbox, preserveaspectratio, so the doc comment's premise was false too. And it also allows d, points, transform, stroke-width, which my list omitted — exactly the trap you described. Rather than just delete it I added a test that pins those eight attributes surviving sanitization, so nobody reintroduces an allowlist on the same false premise. Since the allowlist was introduced by this branch, I folded the removal into the original commit; it never appears in the history now.

Restored the \u0000 cache-key separator. Confirmed reachable, and I have it on record as a red test first: with |, the second vertex type resolved to style="color:x|#FF0000" — the other type's color, i.e. the icon swap. One nuance for the record: IconSourceId embeds the user-supplied url verbatim, but it's color that has to contain the | for keys to collide, and the color picker can't produce that. So it's malformed-config hardening rather than something users hit today. \u0000 is still strictly correct and free.

Added the background-fit rationale comment and a CONTEXT.md line. Both now record the ellipse/inset reason, plus the constraint in the next section.

Rebased. safeSessionStorage.test.ts passes. One conflict: main has since dropped expect.extend(matchers) (now covered by the @testing-library/jest-dom/vitest import), so I took main's side and kept only the Image double.

pnpm check:lint, pnpm check:types and pnpm test (2719 tests) are green.

On variant E — it breaks remote raster icons

This is the one place I'd push back, and I think the harness hides it.

I ran the wrapper through Chrome the way cytoscape consumes it (Image → canvas) and measured the pixels. The test image is a uniform opaque orange 400×100 PNG:

case	rendered box	ratio	is it our image?
E: wrapper + external http raster	54×60	0.9	no
E: wrapper + inline data-url raster	60×16	3.75 ✔	yes
today: bare external raster url	100×100	1.0 (the bug)	yes

In the external case the rendered colors are 88,174,57 / 163,163,163 / 197,214,243 with zero orange pixels — that's Chrome's broken-image placeholder, not the icon. Same for href and xlink:href.

The cause is the sandbox your own VertexSymbolIcon comment already names: an SVG consumed as an image is a separate, script-disabled image document, and it won't fetch external resources either. The wrapper is exactly that once it's a data: uri. The last row shows the URL itself is perfectly loadable — it's the nesting that kills it.

The prototype can't surface this because its raster is canvas.toDataURL(), i.e. inline, which is the one raster case that works. In production toIconImageUrl returns icon.url verbatim for rasters, and the fixtures are https://example.test/a.png.

So for a remote raster, E trades "distorted but recognizable" for "a broken-image glyph on every node of that type."

On "the same mechanism VertexSymbol already uses" — structurally true, and I checked: ICON_RATIO = 0.6, viewBox 0 0 96 96, <image x=19.2 y=19.2 width=57.6 height=57.6 preserveAspectRatio="xMidYMid meet">. But VertexSymbol is live DOM, where an external href fetches normally; that's why it renders remote rasters today. Same markup, opposite behaviour, one level further in.

That's why I kept the measurement. Working the drawInscribedImage arithmetic, the alternatives for a remote raster are: contain + auto/auto preserves ratio but fills the full 24px and loses the inset (variant B), and mixing one percentage with one auto is nonsense (60% width + auto height on 4:1 gives 3.46×24). Aspect and inset requires knowing the ratio — so the measurement is load-bearing rather than incidental, unless we accept raster icons rendering visibly larger than SVG ones and diverging from VertexSymbol's 0.6.

Happy to be wrong about this if it reproduces differently for you. The probe is self-contained (spins its own server, generates its own PNG, no deps) and prints that table — I can drop it in a gist or add it to the prototype if useful. Worth re-running in Firefox/Safari; I only had Chrome headless, though the restriction is standard SVG-as-image sandboxing rather than a Chrome quirk.

Two smaller notes on the framing: the 448 lines include ~196 lines of tests plus the 32-line file that's now gone (production delta is ~220), and the raster row in "pixel-identical across 4:1, 1:4, 1:1, 10:1 and a raster" is the inline-data-url case.

PR description

You're right that the SVG bullet was wrong — the branch does rewrite every SVG's intrinsic size. Corrected bullet:

SVG handling: An SVG's intrinsic width/height are rewritten to the aspect-fitted size rather than forced to a square. Forcing a 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. A viewBox is synthesized from width/height when absent, because without one there is no coordinate system to scale from and resizing the root just clips the content.

I have applied that to the description, and corrected the "How to read" entry for svgViewBox.ts — it synthesizes a viewBox rather than extracting and validating one.

[mjuarros]

Correcting my previous comment: my objection to variant E was wrong, and I'd rather retract it explicitly than leave it standing.

Where I was wrong

I claimed the padded-SVG wrapper would break remote raster icons. The browser behaviour I measured is real — inside a data: uri wrapper an external <image href> is not fetched, and Chrome paints a broken-image glyph — but I never checked whether a remote raster icon is reachable in the first place. It isn't. ICON_VALUE_PATTERN in core/styling/stylingParser.ts gates every icon value to lucide:<name> or data:image/*;base64,, enforced at both write paths (isAllowedIconValue at the upload seam, safeIconValue in the import parser, which strips an injected iconUrl specifically so it cannot bypass the gate). Every icon is inline. The https://example.test/a.png fixtures are test-only, and I over-read them as representative.

So the failure mode I described cannot occur, and my conclusion that the measurement is load-bearing does not follow from it.

What I verified instead

I spiked variant E against the app's real cytoscape 3.34.3 with production node geometry from defaultNodeStyle.ts, icons supplied as base64 data uris exactly as the upload seam stores them, measuring the drawn icon as a pixel diff against an icon-free baseline. Two runs, identical output.

case	current (measured %)	variant E	E + ensureSvgViewBox
png 4:1	116×30 r3.9	116×30 r3.9	116×30 r3.9
png 1:4	28×116 r0.24	30×116 r0.26	30×116 r0.26
png 1:1	116×116 r1	116×116 r1	116×116 r1
svg 4:1	116×30 r3.9	116×30 r3.9	116×30 r3.9
svg 4:1 currentColor	116×30 r3.9	116×30 r3.9	116×30 r3.9
lucide-like 1:1	96×96 r1	96×96 r1	96×96 r1
svg 4:1, no viewBox	116×30 r3.9	116×116 r1.0	116×30 r3.9
svg 4:1, viewBox only	116×30 r3.9	116×30 r3.9	116×30 r3.9

You're right on the main point: E is pixel-equivalent everywhere it matters (the ±2px is antialiasing at 8× zoom), and it keeps the 60% inset. I also checked the thing I thought might bite — currentColor survives the extra nesting level, so baking the colour into the inner SVG still works when that SVG is one level deeper as an <image href>.

One entry to keep on the removal list

ensureSvgViewBox is load-bearing. An SVG with width/height and no viewBox renders square under E — 116×116, ratio 1.0 — which is #2108 reintroduced. preserveAspectRatio has no intrinsic ratio to fit, so the image fills the padded box. Synthesizing the viewBox first restores the correct 116×30.

That case is reachable: it is what a plain <svg width height> export produces, and users upload arbitrary SVGs.

Plan

Implementing E now, keeping ensureSvgViewBox, with a regression test for the no-viewBox case so it can't quietly come back. That removes measureImageDimensions and new Image(), the async raster path, width?/height? on ResolvedIcon, fitAspectRatio, computeAspectRatioAwareDimensions, extractSvgDimensions, and the Image double in setupTests.ts; useBackgroundImageMap goes back to Map<VertexType, string> and the sizing moves to background-fit: contain with auto axes in defaultNodeStyle.

One implementation note: the wrapper has to be applied on the canvas path only, not inside toIconImageUrl. VertexSymbolIcon already insets to 60% in its own SVG coordinates, so wrapping there too would apply the inset twice and render at 36%.

Also worth noting the double percent-encoding is a non-issue: it inflates the style string 1.2× for rasters, 1.6× for SVG, 2.2× for lucide, with a largest absolute value of ~2.5KB. Unique icons stay in the dozens, so it doesn't approach the Schema View concern.

Caveats on my numbers: Chrome headless only, the lucide row is a stand-in rather than real lucide output, and the bbox carries ~2px of antialiasing noise.

[mjuarros]

Pushed as 4ca8fdd9 — variant E is in, with ensureSvgViewBox kept for the reason above.

What went away: measureImageDimensions and new Image(), the async raster path (a raster url resolves synchronously again), width?/height? on ResolvedIcon, fitAspectRatio and aspectFit.ts, computeAspectRatioAwareDimensions, extractSvgDimensions, and the Image double in setupTests.ts. useBackgroundImageMap is back to Map<VertexType, string>, and useGraphStyles.ts and setupTests.ts are now identical to main again. Net diff is 14 files, +328/-47, of which ~240 lines are tests.

What stayed: ensureSvgViewBox, and the parse/serialize in toIconImageUrl that bakes the colour.

I verified the branch's real output rather than a port of it — emitting the actual urls the hook produces, then rendering them on cytoscape 3.34.3 with the updated defaultNodeStyle:

icon	drawn box	ratio	width vs node
4:1, with viewBox	116×30	3.87	60.4%
4:1, no viewBox	116×30	3.87	60.4%
1:4, no viewBox	30×116	0.26	15.6%
1:1, no viewBox	116×116	1.00	60.4%

The description is updated to describe the wrapper. The two commits from the earlier round (the NUL cache key, and never introducing the ALLOWED_ATTR list) are unchanged; I squashed the rationale commit into this one since it documented the approach that just got replaced.

[kmcginnes]

This is the approach I wanted, and it works. I verified it in the real app rather than a harness: dev server off your branch against a live air_routes TinkerPop container, geometry measured off the cytoscape canvas.

Icon	Extent (device px)	Circle	Aspect
4:1 SVG	144x36	28x28	1.000
1:4 SVG	36x142	28x28	1.000
1:1 SVG	144x142	114x114	1.000
4:1 PNG	144x36	28x28	1.000
viewBox-less 4:1 SVG	144x36	28x28	1.000

The inset is exact, 0.600 of the node box on the major axis in every case, centred to within half a device pixel. The 1:1 icon clears the ellipse outline by 18.9px, which is the part the first attempt got wrong. I reproduced the bug on the merge base with the same script for comparison: the 4:1 PNG came out 144x142 with the circle crushed to 28x114, aspect 0.246.

One thing worth knowing that neither of us had established: a viewBox-carrying SVG was already rendering correctly before this branch. #2108 only ever bit raster icons and viewBox-less SVGs. My earlier comment overstated the SVG half.

The nesting question is settled too. In Chrome, a wrapper with a nested data: URL paints fine and issues zero external requests, so the SVG-as-image sandbox is not in the way.

One blocker

insetIconImage introduces the first encodeURIComponent on the raster path, and encodeURIComponent throws URIError on a string that isn't well-formed UTF-16. A stored iconUrl containing a lone surrogate therefore throws during render of useGraphStyles, which lands in the ErrorBoundary around the whole <Outlet />. The app is replaced by AppErrorPage, whose only action reloads and re-throws, and the styling panel that would clear the bad value is inside the blanked route. At the merge base the raster arm returned the url by reference, so nothing on that path could throw.

Fix: test isWellFormed() on the url and skip the icon when it fails, logging a warning. The vertex then renders without a background image instead of taking the app down.

There are also two validation gaps further upstream that let a bad value get stored in the first place. Those predate this branch and I'm tracking them separately, so don't worry about them here.

Before merge

ensureSvgViewBox silently does nothing for several real inputs, including an SVG using xlink:href without declaring xmlns:xlink, and one with no xmlns at all. The intermediate application/xml parse fails and the function returns its input, so no viewBox is synthesized and the icon still renders square. That is the class of icon this branch exists to fix. Parsing as image/svg+xml should cover it.

Share the inset geometry instead of redeclaring it. ICON_RATIO = 0.6 now exists in both useBackgroundImageMap.ts and VertexSymbol.tsx, and CONTEXT.md states it as one rule, so changing either silently desyncs the canvas from the style preview with no test to catch it. encodeSvg is duplicated the same way. Also BOX = 100 is commented "arbitrary", but it sets the resolution the wrapper rasterizes at, and VertexSymbol already has the reasoned value for that quantity, VIEWBOX = 96, which is exactly 4x a node.

Two tests don't test what they claim. In iconRegistry.test.ts, toContain("d") passes no matter what, because the fixture's own preserveAspectRatio contains a d, and toContain("width") is satisfied by stroke-width alone. Assert values rather than bare names. And in useGraphStyles.test.tsx the raster assertion only checks the data: prefix, so it passes even if the wrapper lost the url.

There's no VertexIcon.test.tsx anywhere, so the inline-DOM surface is the only one of the three whose fix ships unverified.

CONTEXT.md still says the canvas passes a raster "as its plain url" two sentences before the new clause about wrapping, so it now describes rasters both ways. The icon-registry ADR has the same staleness in its raster row, and its note about image contexts blocking external references reads as forbidding what the canvas now does. Worth a qualifier distinguishing data: URLs.

Small

• backgroundWidth/backgroundHeight: "auto" restate cytoscape's own defaults, so the comment crediting them with the fix is off. contain plus dropping the percentages is what changed the rendering.
• That same comment should say why contain alone fails, naming the ellipse. That was the specific thing I asked for.
• ensureSvgViewBox guards on isNaN and > 0, so width="1e400" writes viewBox="0 0 Infinity Infinity" and the icon vanishes. width="100%" reads as 100 user units and crops the artwork; I measured a 200-wide rect cut off at x=100. Reject anything that isn't a plain number, and use Number.isFinite.
• The try/catch in ensureSvgViewBox is unreachable. DOMParser with application/xml returns a parsererror document rather than throwing.
• Copy the "wrapper belongs on the canvas path only, or it insets twice" rule from CONTEXT.md into the toIconImageUrl doc comment, which is where someone would make that mistake.
• No tall-icon fixture in the new tests. One line.

Nothing else: pnpm checks and all 2714 tests pass, the escaping held under every payload I threw at it, and the sanitizer config is unchanged from the merge base.

[mjuarros]

Thank you for the geometry verification against a live container — that's a stronger check than anything I ran. All fixed, pushed as four commits on top of the rebase (now at b6137046).

Fixed

The blocker. Confirmed: encodeURIComponent throws URIError on a lone surrogate, this runs during style computation, and DefaultLayout.tsx wraps the whole <Outlet /> with no in-app recovery. insetIconImage now checks iconUrl.isWellFormed() first and skips the icon with a logger.warn instead of calling encodeURIComponent on it. Test asserts renderMap doesn't throw on a lone-surrogate url and the vertex type is absent from the result.

ensureSvgViewBox's two numeric bugs. Switched to Number.isFinite and reject a trailing %; 1e400 and 100% are now left untouched instead of synthesizing Infinity/a cropping viewBox. 400px still works. Removed the try/catch — traced through it and it's genuinely dead: DOMParser with application/xml never throws, it returns a document whose root tag doesn't match what was requested, so root.localName !== "svg" already rejects it before reaching the numeric guard.

Shared geometry. ICON_RATIO, the box size, and encodeSvg now live in one module (iconGeometry.ts) that both useBackgroundImageMap and VertexSymbol import, with a test pinning the values. Took the box size to 96 — your reasoned value — over my arbitrary 100.

The two weak tests. iconRegistry.test.ts's geometry check now asserts full attribute values with distinct numbers per attribute rather than bare names (verified it now fails when stroke-width/d are actually wrong). useGraphStyles.test.tsx's raster check now decodes the wrapper and confirms the real url is nested inside.

VertexIcon.test.tsx now exists — object-contain on the raster path, lucide color inheritance, unknown-lucide-renders-nothing.

Docs. CONTEXT.md's raster sentence is scoped so it doesn't contradict itself. The ADR's table and external-references line are updated, with a new paragraph on why the wrapper doesn't cost the sandbox. defaultNodeStyle's comment credits contain correctly and names the ellipse.

Tall-icon fixture added alongside the wide one.

Two things I want to correct, since I'd rather flag it than let it stand

image/svg+xml doesn't fix the xlink case — I ran it. Both application/xml and image/svg+xml are XML-strict in DOMParser, so xlink:href without a declared xmlns:xlink fails identically either way; only lenient text/html parsing recovers it. Separately, that input never reaches ensureSvgViewBox at all on the untrusted-SVG path: DOMPurify's sanitized output doesn't round-trip through isParseableSvg's application/xml check either, so the icon is rejected upstream, in code that predates this branch. The symptom for that input today is a missing icon, not a squashed one.

The "no xmlns at all" case, though, I couldn't reproduce — <svg width="400" height="100"> with no xmlns parses fine as application/xml and gets its viewBox synthesized correctly in my testing.

Given both of those, I've left ensureSvgViewBox on application/xml for now rather than switching parsers, since the fix you proposed doesn't close the gap and I don't have a case in hand where it currently fails incorrectly. If you have a concrete input that reaches ensureSvgViewBox and still gets skipped, I'd like to see it — that'd change my answer.

The px-suffix concern: rejecting "anything that isn't a plain number" would have taken width="400px" down with it, which is legitimate SVG that was working before. I kept parseFloat for that reason and only added the % and finiteness checks.

pnpm checks and all 2726 tests pass.

[kmcginnes]

The core fix holds up. Canvas and DOM both keep the aspect ratio, square icons keep the 60% inset, and VertexSymbol isn't double inset. A few things to address:

Should fix

• Raster icons from an http(s) url go blank on the canvas. useBackgroundImageMap.ts nests the raster url in a data: SVG as <image href>, and an SVG loaded as an image won't fetch external resources. Uploaded icons are already data: urls so they work, but an iconUrl pointing at a remote or relative path breaks. The tests only check the url string, so they wouldn't catch it. I'd skip the wrapper for non-data: rasters, or confirm none can reach this path.
• escapeXmlAttribute doesn't escape <. XML doesn't allow a bare < in attribute values, so a raster url containing one produces a malformed wrapper. SVG icons are fine because they're percent-encoded first.
• CONTEXT.md still describes the measuring approach from the first commit. The Icon Registry entry says it holds the raster's "natural width/height" and that "every icon kind resolves asynchronously". Neither is true anymore. The stale docs commit missed this one.

Smaller

• A malformed url returns null without caching, so logger.warn fires again on every recompute.
• An SVG with percentage width/height and no viewBox still gets no viewBox, so it still fills the box.
• encodeSvg in iconGeometry.ts isn't geometry. I'd move it back to iconImageUrl.ts.
• The ICON_BOX * ICON_RATIO inset math is duplicated in VertexSymbol.tsx and useBackgroundImageMap.ts. A shared helper in iconGeometry would make it a real single source of truth.
• The DOMPurify config plus ensureSvgViewBox sequence is duplicated in VertexIcon.tsx and iconRegistry.ts.
• Several comments restate what the code does and could be trimmed to just the rationale: defaultNodeStyle.ts, the insetIconImage JSDoc, parseFiniteLength, iconGeometry.ts header.