Merge pull request #13 from Artsen/fix/semantic-token-collisions

Prevent semantic token collisions and stabilize duplicate contrast rows

Author: Artsen

docs/architecture.md

@@ -114,11 +114,16 @@ The frontend generates every export without an API request:
 - SVG swatch sheet
 
 CSS emits base `--color-*` values and assigned `--role-*` aliases. Tailwind
-emits base keys and assigned `role-*` semantic keys. SVG annotates each row with
-its assigned roles. Comments replace line breaks and the `*/` sequence in the
-palette name. SVG output escapes `&`, `<`, `>`, `"`, and `'`. Each swatch label
-uses black or white according to the higher measured contrast ratio. Download
-uses an object URL and revokes the URL after the browser starts the download.
+emits base keys and assigned `role-*` semantic keys. The Tailwind `role-*`
+namespace is reserved even when roles are unassigned. The shared base-token
+allocator adds deterministic numeric suffixes when a normalized color name
+would collide with a reserved or previously allocated key. CSS keeps base and
+semantic values in separate `--color-*` and `--role-*` namespaces. SVG
+annotates each row with its assigned roles. Comments replace line breaks and
+the `*/` sequence in the palette name. SVG output escapes `&`, `<`, `>`, `"`,
+and `'`. Each swatch label uses black or white according to the higher measured
+contrast ratio. Download uses an object URL and revokes the URL after the
+browser starts the download.
 
 Export does not create or update a saved palette record.
 

docs/user-guide.md

@@ -202,6 +202,9 @@ stated in percentage points.
 
 Named colors produce safe ASCII keys in CSS and Tailwind output and visible
 labels in SVG. Unnamed colors keep the `palette-1`, `palette-2` numeric pattern.
+Tailwind reserves the generated `role-*` namespace for semantic role keys. A
+color name that normalizes to a reserved key receives a deterministic numeric
+suffix, such as `role-primary-action-2`.
 ColorCraft JSON schema version 3 preserves Unicode names, exact order,
 extraction metadata, and unambiguous role ownership. Each color receives a
 document-local key such as `color-1`; the file does not contain internal
@@ -213,8 +216,9 @@ exported file through the browser. Export does not create or update a saved
 palette record.
 
 CSS output adds assigned `--role-*` aliases that reference base `--color-*`
-tokens. Tailwind output adds assigned `role-*` keys, and SVG rows identify their
-assigned roles. Imported version-1 and version-2 files still use HEX role
+tokens. These separate CSS namespaces prevent collisions. Tailwind output adds
+assigned `role-*` keys, and SVG rows identify their assigned roles. Imported
+version-1 and version-2 files still use HEX role
 references. When duplicate HEX values make those older files ambiguous,
 ColorCraft maps the role to the first matching color in palette order.
 

docs/writing-style.md

@@ -206,7 +206,7 @@ contents, clipboard contents, or unrelated palette data in user-facing errors.
 | exported file | A file that the browser creates from the current palette. | Do not imply that ColorCraft stores the file. |
 | ColorCraft JSON | The portable, single-palette JSON format that ColorCraft exports and imports in the browser. | Distinguish its schema version from the IndexedDB saved-palette schema and backend API schemas. |
 | portable color key | A deterministic document-local color reference such as `color-1` in ColorCraft JSON version 3. | Do not call it an internal workspace ID. |
-| semantic role token | An assigned `--role-*` CSS alias or `role-*` Tailwind key generated from a color role. | Keep it separate from the base palette token. |
+| semantic role token | An assigned `--role-*` CSS alias or `role-*` Tailwind key generated from a color role. | The Tailwind `role-*` namespace is reserved. Keep CSS base `--color-*` and semantic `--role-*` tokens separate. |
 | imported palette | A validated ColorCraft JSON palette activated in the current session. | State that it remains unsaved until **Save palette** is selected. |
 | copy | Place generated export text on the system clipboard. | Do not use as a synonym for export or download. |
 | download | Save generated export data as a local file through the browser. | Do not use as a synonym for copy. |

frontend/src/components/AnalysisResults.test.tsx

@@ -1,5 +1,5 @@
 import { render, screen } from '@testing-library/react'
-import { describe, expect, it } from 'vitest'
+import { describe, expect, it, vi } from 'vitest'
 import AnalysisResults from './AnalysisResults'
 import { analysis, blue, red } from '../test/fixtures'
 
@@ -26,4 +26,39 @@ describe('AnalysisResults', () => {
       screen.getByText('#ff0000 + #0000ff • Ratio: 2.15'),
     ).toBeInTheDocument()
   })
+
+  it('renders duplicate pairs and issues without duplicate React keys', () => {
+    const pair = { ...analysis.accessibility.pairs[0] }
+    const issue = { ...analysis.accessibility.issues[0] }
+    const duplicateAnalysis = {
+      ...analysis,
+      accessibility: {
+        ...analysis.accessibility,
+        pairs: [{ ...pair }, { ...pair }],
+        issues: [{ ...issue }, { ...issue }],
+      },
+    }
+    const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {})
+    try {
+      const { container } = render(
+        <AnalysisResults
+          analysis={duplicateAnalysis}
+          colors={[red, { ...red, id: 'duplicate-red' }, blue]}
+        />,
+      )
+      expect(
+        screen.getAllByText(
+          'Low contrast detected between #ff0000 and #0000ff.',
+        ),
+      ).toHaveLength(2)
+      expect(
+        container.querySelectorAll('.analysis-section .contrast-row'),
+      ).toHaveLength(2)
+      expect(consoleError.mock.calls.flat().join(' ')).not.toMatch(
+        /same key|unique "key"/i,
+      )
+    } finally {
+      consoleError.mockRestore()
+    }
+  })
 })

frontend/src/components/AnalysisResults.tsx

@@ -154,9 +154,9 @@ export default function AnalysisResults({ analysis }: AnalysisResultsProps) {
           <section className="analysis-section">
             <h3>Issues found</h3>
             <div className="issue-list">
-              {accessibility.issues.map((issue) => (
+              {accessibility.issues.map((issue, index) => (
                 <Notice
-                  key={`${issue.color1}-${issue.color2}`}
+                  key={`${issue.color1}-${issue.color2}-${index}`}
                   variant="warning"
                 >
                   <p>{issue.message}</p>
@@ -175,9 +175,9 @@ export default function AnalysisResults({ analysis }: AnalysisResultsProps) {
         <section className="analysis-section">
           <h3>Color pair contrast</h3>
           <div className="contrast-list">
-            {accessibility.pairs.map((pair) => (
+            {accessibility.pairs.map((pair, index) => (
               <div
-                key={`${pair.color1}-${pair.color2}`}
+                key={`${pair.color1}-${pair.color2}-${index}`}
                 className="contrast-row"
               >
                 <div className="contrast-row-header">

frontend/src/components/ReviewWorkspace.test.tsx

@@ -248,6 +248,48 @@ describe('ReviewWorkspace outcomes', () => {
     expect(within(result).getByText(/1.00 to 1/)).toBeInTheDocument()
   })
 
+  it('renders duplicate all-pairs rows without duplicate React keys', () => {
+    const duplicatePair = {
+      color1: '#FF0000',
+      color2: '#0000FF',
+      ratio: 2.15,
+      aaNormal: false,
+      aaLarge: false,
+      aaaNormal: false,
+      aaaLarge: false,
+    }
+    const duplicateAnalysis = measuredAnalysis()
+    duplicateAnalysis.accessibility.pairs = [
+      { ...duplicatePair },
+      { ...duplicatePair },
+    ]
+    const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {})
+    try {
+      const { container } = render(
+        <ReviewWorkspace
+          colors={[red, blue]}
+          analysis={duplicateAnalysis}
+          analysisStale={false}
+          analyzing={false}
+          selectedTab="contrast"
+          roles={{}}
+          onSelectTab={vi.fn()}
+          onAnalyze={vi.fn()}
+          onAssignRole={vi.fn()}
+          onAddColor={vi.fn()}
+        />,
+      )
+      expect(
+        container.querySelectorAll('.all-pairs .contrast-row'),
+      ).toHaveLength(2)
+      expect(consoleError.mock.calls.flat().join(' ')).not.toMatch(
+        /same key|unique "key"/i,
+      )
+    } finally {
+      consoleError.mockRestore()
+    }
+  })
+
   it('clearly identifies stale analysis without rendering prior results', () => {
     render(
       <ReviewWorkspace

frontend/src/components/ReviewWorkspace.tsx

@@ -532,10 +532,10 @@ function Contrast({
             components or focus indicators.
           </p>
           <div className="contrast-list">
-            {analysis.accessibility.pairs.map((pair) => (
+            {analysis.accessibility.pairs.map((pair, index) => (
               <div
                 className="contrast-row"
-                key={`${pair.color1}-${pair.color2}`}
+                key={`${pair.color1}-${pair.color2}-${index}`}
               >
                 <div className="contrast-row-header">
                   <span>

frontend/src/exporters.test.ts

@@ -6,6 +6,7 @@ import {
   generateJson,
   generateSvg,
   generateTailwind,
+  reservedSemanticTokens,
   sanitizeFilename,
 } from './exporters'
 
@@ -105,6 +106,107 @@ describe('browser palette exporters', () => {
     )
   })
 
+  it('reserves every semantic role token even when roles are unassigned', () => {
+    const colors = reservedSemanticTokens.map((token, index) => ({
+      ...(index % 2 === 0 ? red : blue),
+      id: `reserved-${index}`,
+      name: token
+        .split('-')
+        .map((part) => `${part[0].toUpperCase()}${part.slice(1)}`)
+        .join(' '),
+    }))
+    expect(exportTokens(colors)).toEqual(
+      reservedSemanticTokens.map((token) => `${token}-2`),
+    )
+    const withoutRoles = generateTailwind({
+      ...palette,
+      colors,
+      roles: {},
+    })
+    const withRoles = generateTailwind({
+      ...palette,
+      colors,
+      roles: { primaryAction: colors[0].id },
+    })
+    const keys = (output: string) =>
+      [...output.matchAll(/^\s+'([^']+)':/gm)].map((match) => match[1])
+    expect(keys(withRoles).slice(0, colors.length)).toEqual(
+      keys(withoutRoles).slice(0, colors.length),
+    )
+  })
+
+  it('allocates deterministic suffixes across reserved and existing suffixes', () => {
+    const colors = [
+      { ...red, id: 'one', name: 'Role primary action' },
+      { ...blue, id: 'two', name: 'Role primary action 2' },
+      { ...red, id: 'three', name: 'Role primary action' },
+      { ...blue, id: 'four', name: 'Ordinary' },
+      { ...red, id: 'five', name: 'Ordinary' },
+      { ...blue, id: 'six', name: 'Ordinary 2' },
+    ]
+    expect(exportTokens(colors)).toEqual([
+      'role-primary-action-2',
+      'role-primary-action-2-2',
+      'role-primary-action-3',
+      'ordinary',
+      'ordinary-2',
+      'ordinary-2-2',
+    ])
+  })
+
+  it.each([
+    'Role primary action',
+    'ROLE PRIMARY ACTION',
+    'role---primary___action!!!',
+  ])('normalizes the reserved-name variant %s safely', (name) => {
+    expect(exportTokens([{ ...red, name }])).toEqual(['role-primary-action-2'])
+  })
+
+  it('keeps unnamed fallbacks and ordinary names unchanged', () => {
+    expect(exportTokens([red, blue])).toEqual(['palette-1', 'palette-2'])
+    expect(
+      exportTokens([
+        { ...red, name: 'Primary action' },
+        { ...blue, name: 'Surface neutral' },
+      ]),
+    ).toEqual(['primary-action', 'surface-neutral'])
+  })
+
+  it('uses collision-safe base tokens consistently in CSS and Tailwind', () => {
+    const collisionPalette = {
+      ...palette,
+      colors: [
+        { ...red, name: 'Role primary action' },
+        { ...blue, name: 'Role primary action' },
+      ],
+      roles: {
+        primaryAction: red.id,
+        pageBackground: blue.id,
+      },
+    }
+    const css = generateCss(collisionPalette)
+    expect(css).toContain('--color-role-primary-action-2: #ff0000;')
+    expect(css).toContain('--color-role-primary-action-3: #0000ff;')
+    expect(css).toContain(
+      '--role-primary-action: var(--color-role-primary-action-2);',
+    )
+    expect(css).toContain(
+      '--role-page-background: var(--color-role-primary-action-3);',
+    )
+
+    const tailwind = generateTailwind(collisionPalette)
+    const keys = [...tailwind.matchAll(/^\s+'([^']+)':/gm)].map(
+      (match) => match[1],
+    )
+    expect(new Set(keys).size).toBe(keys.length)
+    expect(keys).toEqual([
+      'role-primary-action-2',
+      'role-primary-action-3',
+      'role-page-background',
+      'role-primary-action',
+    ])
+  })
+
   it('keeps duplicate-color semantic ownership distinct', () => {
     const duplicate = { ...red, id: 'second-red', name: 'Second red' }
     const duplicatePalette = {

frontend/src/exporters.ts

@@ -2,6 +2,7 @@ import type { PaletteColor } from './workspace'
 import { serializePortablePalette } from './portablePalette'
 import {
   contrastRatio,
+  paletteRoles,
   roleAssignmentEntries,
   roleLabels,
   roleTokenNames,
@@ -163,13 +164,31 @@ export function exportToken(value: string, fallback: string): string {
   )
 }
 
+export const reservedSemanticTokens = paletteRoles.map(
+  (role) => `role-${roleTokenNames[role]}`,
+)
+
+function allocateToken(
+  requested: string,
+  allocated: Set<string>,
+  reserved: ReadonlySet<string>,
+): string {
+  let token = requested
+  let suffix = 2
+  while (allocated.has(token) || reserved.has(token)) {
+    token = `${requested}-${suffix}`
+    suffix += 1
+  }
+  allocated.add(token)
+  return token
+}
+
 export function exportTokens(colors: PaletteColor[]): string[] {
-  const counts = new Map<string, number>()
+  const allocated = new Set<string>()
+  const reserved = new Set(reservedSemanticTokens)
   return colors.map((color, index) => {
-    const base = exportToken(color.name ?? '', `palette-${index + 1}`)
-    const count = (counts.get(base) ?? 0) + 1
-    counts.set(base, count)
-    return count === 1 ? base : `${base}-${count}`
+    const requested = exportToken(color.name ?? '', `palette-${index + 1}`)
+    return allocateToken(requested, allocated, reserved)
   })
 }