refactor(vanilla): unify gradient and clip-path ID generation via nextSvgId

Two SVG ID generation strategies existed: gradient-utils used a monotonic
counter (introduced by 73ef048 after random-hex collisions surfaced in
production), while svg-renderer still used `Math.random().toString(36)` for
clip-path IDs. That left the clip-path path with the same ~1/16M collision
risk the gradient fix eliminated.

Unified both under a new `nextSvgId(prefix)` helper in `svg-ids.ts`. The
counter is shared across ID types, so gradient IDs may now skip numbers
when clip-paths consume counter slots. Still monotonic, still unique.

Format changes: clip-path IDs go from `oc-clip-<hex>` to `oc-clip-N`.
Gradient IDs stay `oc-grad-N` but the numbering sequence now interleaves
with clip-path allocations. Internal SVG IDs aren't part of our public API,
but snapshot-based consumer tests matching the specific hex clip-path
format would break. Visual regression tests strip all `[id]` attributes
before screenshot comparison, so rendering output is unaffected.

Extended gradient-ids.test.ts to cover clip-path uniqueness and the core
cross-type uniqueness guarantee across two chart mounts.

Author: rileyhilliard

packages/vanilla/src/__tests__/gradient-ids.test.ts

@@ -1,17 +1,15 @@
 /**
- * Characterization test for multi-chart gradient ID uniqueness.
+ * Multi-chart SVG ID uniqueness: gradients AND clip-paths.
  *
- * Part of refactor/v7-cohesion step 1. Pins the behavior of the global
- * gradient counter in `packages/vanilla/src/gradient-utils.ts` (commit 73ef048).
+ * Originally locked gradient-counter behavior from commit 73ef048 (fix for
+ * random-hex gradient collisions). Step 6 of refactor/v7-cohesion unified
+ * gradient and clip-path ID generation under `nextSvgId` in `svg-ids.ts`,
+ * so this test now guards both halves.
  *
- * Before the fix, gradient IDs used random hex suffixes that could collide when
- * multiple charts with gradient fills rendered into the same document. Because
- * SVG url(#id) resolves against the full document, a collision caused one chart
- * to inherit another chart's gradient. The fix replaced the random suffix with a
- * module-global monotonic counter so IDs are always unique across charts.
- *
- * Step 6 of the v7 refactor plans to unify gradient and clipPath ID generation.
- * This test guards uniqueness across that consolidation.
+ * Why it matters: SVG `url(#id)` resolves against the full document. If two
+ * charts on the same page generate overlapping IDs, one chart silently
+ * inherits the other's gradient fill or clip region. The shared monotonic
+ * counter makes uniqueness unconditional.
  */
 
 import type { ChartSpec, LinearGradient } from '@opendata-ai/openchart-core';
@@ -69,7 +67,16 @@ function collectGradientIds(root: HTMLElement): string[] {
   return ids;
 }
 
-describe('gradient ID uniqueness across charts', () => {
+function collectClipPathIds(root: HTMLElement): string[] {
+  const ids: string[] = [];
+  for (const el of root.querySelectorAll('clipPath')) {
+    const id = el.getAttribute('id');
+    if (id) ids.push(id);
+  }
+  return ids;
+}
+
+describe('SVG ID uniqueness across charts', () => {
   let a: HTMLDivElement;
   let b: HTMLDivElement;
 
@@ -82,7 +89,7 @@ describe('gradient ID uniqueness across charts', () => {
     document.body.innerHTML = '';
   });
 
-  it('two charts mounted in separate containers produce disjoint gradient IDs', () => {
+  it('two charts produce disjoint gradient IDs', () => {
     const chartA = createChart(a, makeBarSpec(barGradient));
     const chartB = createChart(b, makeBarSpec(altGradient));
 
@@ -105,4 +112,48 @@ describe('gradient ID uniqueness across charts', () => {
     chartA.destroy();
     chartB.destroy();
   });
+
+  it('two charts produce disjoint clip-path IDs', () => {
+    // Every chart render creates a <clipPath> for the plot area, so any spec
+    // works here. Reuse the gradient specs for consistency.
+    const chartA = createChart(a, makeBarSpec(barGradient));
+    const chartB = createChart(b, makeBarSpec(altGradient));
+
+    const idsA = collectClipPathIds(a);
+    const idsB = collectClipPathIds(b);
+
+    expect(idsA.length).toBeGreaterThan(0);
+    expect(idsB.length).toBeGreaterThan(0);
+
+    const all = [...idsA, ...idsB];
+    expect(new Set(all).size).toBe(all.length);
+
+    for (const id of all) {
+      expect(id).toMatch(/^oc-clip-\d+$/);
+    }
+
+    chartA.destroy();
+    chartB.destroy();
+  });
+
+  it('gradient and clip-path IDs never collide across two charts', () => {
+    // Core guarantee of the unified nextSvgId counter: even though gradients
+    // and clip-paths use different prefixes, the counter values are shared.
+    // Collecting every ID from both <defs> trees should yield a strict set.
+    const chartA = createChart(a, makeBarSpec(barGradient));
+    const chartB = createChart(b, makeBarSpec(altGradient));
+
+    const all = [
+      ...collectGradientIds(a),
+      ...collectClipPathIds(a),
+      ...collectGradientIds(b),
+      ...collectClipPathIds(b),
+    ];
+
+    expect(all.length).toBeGreaterThan(0);
+    expect(new Set(all).size).toBe(all.length);
+
+    chartA.destroy();
+    chartB.destroy();
+  });
 });

packages/vanilla/src/gradient-utils.ts

@@ -5,6 +5,7 @@
 
 import type { GradientDef, LinearGradient, RadialGradient } from '@opendata-ai/openchart-core';
 import { isGradientDef } from '@opendata-ai/openchart-core';
+import { nextSvgId } from './svg-ids';
 
 const SVG_NS = 'http://www.w3.org/2000/svg';
 
@@ -89,18 +90,15 @@ function appendStop(
   parent.appendChild(stopEl);
 }
 
-/**
- * Global counter for gradient IDs. Ensures uniqueness across all charts
- * on the same page, since SVG url(#id) resolves globally in the document.
- */
-let globalGradientCounter = 0;
-
 /**
  * Scan all marks for GradientDef fill values, create SVG gradient elements
  * in the provided <defs> node, and return a map from gradient key to element ID.
  *
  * Identical gradients (by key) share a single SVG element within one chart.
- * IDs are globally unique across charts to avoid SVG url(#id) collisions.
+ * IDs come from `nextSvgId` so they're globally unique across charts and
+ * share the counter with clip-path IDs (see svg-ids.ts). Gradient ID numbers
+ * may skip values when clip-paths consume counter slots - still monotonic,
+ * still unique.
  */
 export function buildGradientDefs(
   marks: Array<{ fill?: unknown }>,
@@ -113,7 +111,7 @@ export function buildGradientDefs(
     if (fill && isGradientDef(fill)) {
       const key = gradientKey(fill);
       if (!map.has(key)) {
-        const id = `oc-grad-${globalGradientCounter++}`;
+        const id = nextSvgId('oc-grad');
         const el = createGradientElement(fill, id);
         defs.appendChild(el);
         map.set(key, id);

packages/vanilla/src/svg-ids.ts

@@ -0,0 +1,26 @@
+/**
+ * Monotonic SVG ID generator shared by gradient and clipPath defs.
+ *
+ * Why a single counter: SVG `url(#id)` references resolve against the full
+ * document, so any ID collision (across charts on the same page) silently
+ * cross-wires one chart's fill/clip into another. Random-hex suffixes have a
+ * small-but-real collision probability that surfaced in production; a global
+ * monotonic counter makes uniqueness unconditional.
+ *
+ * Gradient and clip-path IDs share one counter so we can't regress one half of
+ * the system by accident. Consumers just pick a prefix.
+ */
+
+let counter = 0;
+
+export function nextSvgId(prefix: string): string {
+  return `${prefix}-${counter++}`;
+}
+
+/**
+ * Reset the counter. Intended for isolated test runs only - production code
+ * should never call this. The underscore prefix marks it as non-public.
+ */
+export function _resetSvgIdCounter(): void {
+  counter = 0;
+}

packages/vanilla/src/svg-renderer.ts

@@ -37,6 +37,7 @@ import {
 } from '@opendata-ai/openchart-core';
 import { clampStaggerDelay } from '@opendata-ai/openchart-engine';
 import { buildGradientDefs, resolveMarkFill } from './gradient-utils';
+import { nextSvgId } from './svg-ids';
 
 const SVG_NS = 'http://www.w3.org/2000/svg';
 
@@ -1244,7 +1245,7 @@ export function renderChartSVG(
   // Clip path to prevent marks (especially area fills) from overflowing
   // into the chrome region (title/subtitle). Extends full width so
   // end-of-line labels aren't clipped, but constrains vertically.
-  const clipId = `oc-clip-${Math.random().toString(36).slice(2, 8)}`;
+  const clipId = nextSvgId('oc-clip');
   const defs = createSVGElement('defs');
   const clipPath = createSVGElement('clipPath');
   clipPath.setAttribute('id', clipId);