Feature ETP-4657: Fix semantic color token drift from ETP-4554

packages/app-shell-core/src/components/ui/table.jsx

@@ -13,7 +13,8 @@ const Table = React.forwardRef(({ className, ...props }, ref) => (
 Table.displayName = "Table"
 
 const TableHeader = React.forwardRef(({ className, ...props }, ref) => (
-  <thead ref={ref} className={cn("[&_tr]:border-b [&_tr]:border-border-structural", className)} {...props} />
+  // WARN(a11y): border-border-subtle, not border-border-structural — see TableRow below.
+  <thead ref={ref} className={cn("[&_tr]:border-b [&_tr]:border-border-subtle", className)} {...props} />
 ))
 TableHeader.displayName = "TableHeader"
 
@@ -26,18 +27,26 @@ const TableBody = React.forwardRef(({ className, ...props }, ref) => (
 TableBody.displayName = "TableBody"
 
 const TableFooter = React.forwardRef(({ className, ...props }, ref) => (
+  // WARN(a11y): border-border-subtle, not border-border-structural — see TableRow below.
   <tfoot
     ref={ref}
-    className={cn("border-t border-border-structural font-medium [&>tr]:last:border-b-0", className)}
+    className={cn("border-t border-border-subtle font-medium [&>tr]:last:border-b-0", className)}
     {...props} />
 ))
 TableFooter.displayName = "TableFooter"
 
 const TableRow = React.forwardRef(({ className, ...props }, ref) => (
   <tr
     ref={ref}
+    // WARN(a11y): intentionally border-border-subtle, NOT border-border-structural,
+    // for the whole Table (header, body rows, footer). A table hairline isn't a UI
+    // component boundary under WCAG 1.4.11 (cell content + hover/selected bg already
+    // convey the boundary), so it doesn't need the enforced 3:1 contrast — confirmed
+    // against staging (go.staging.etendo.cloud), which never had this distinction.
+    // border-border-structural stays reserved for genuinely structural separators
+    // elsewhere (e.g. BalanceFooterPanel, DocumentTotalsPanel).
     className={cn(
-      "border-b border-border-structural transition-colors hover:bg-muted/50 data-[state=selected]:bg-muted",
+      "border-b border-border-subtle transition-colors hover:bg-muted/50 data-[state=selected]:bg-muted",
       className
     )}
     {...props} />

packages/app-shell-core/src/styles.css

@@ -286,7 +286,7 @@
 
   :root {
     --background: 220 14% 96%;
-    --foreground: 222 47% 11%;
+    --foreground: 222 47% 11%;             /* #0F1729 — matches staging (go.staging.etendo.cloud), the original design spec's color */
     --card: 0 0% 100%;
     --card-foreground: 222 47% 11%;
     --popover: 0 0% 100%;
@@ -296,7 +296,7 @@
     --secondary: 210 40% 96%;
     --secondary-foreground: 222 47% 11%;
     --muted: 210 40% 96%;
-    --muted-foreground: 215 16% 47%;
+    --muted-foreground: 240 12% 48%;       /* #6C6C89 — NOT staging's #65758B: that fails WCAG AA (4.3:1) against --background (only passes 4.7:1 against white --card); this is the a11y-review fix, matches --text-secondary */
     --accent: 210 40% 96%;
     --accent-foreground: 222 47% 11%;
     --destructive: 0 84% 60%;
@@ -306,10 +306,20 @@
     --inverse-muted: 215 16% 72%;
     --inverse-border: 215 20% 30%;
     /* Semantic accessibility contract v1 (ETP-4554). */
-    --border-control: 215 16% 47%;
+    /* WARN(a11y): --border-control (and --input, which aliases it) is back to
+       staging's original light color. It drives BOTH real form-control borders
+       AND the shared outline-button border (shadcn's `border-input` convention),
+       so darkening it for WCAG 1.4.11 compliance visually affected buttons too,
+       which the design didn't want. Known gap: at this value, --border-control
+       only reaches ~1.5:1 contrast against --background/--card, short of the
+       3:1 WCAG 1.4.11 non-text contrast minimum for control boundaries. Revisit
+       by splitting a dedicated (darker) token for real <input>/<select> borders
+       if/when accessibility compliance is prioritized over matching staging.
+       See theme/index.js: removed from the enforced `requirements` contrast list. */
+    --border-control: 214 32% 91%;
     --border-structural: 215 16% 47%;
-    --border-subtle: 214 32% 85%;
-    --text-primary: 240 12% 8%;
+    --border-subtle: 214 32% 91%;          /* #E1E7EF — matches staging exactly (decorative divider, not a11y-gated) */
+    --text-primary: 222 47% 11%;
     --text-secondary: 240 12% 48%;
     --text-disabled: 225 12% 38%;
     --icon-secondary: 215 16% 47%;

packages/app-shell-core/src/theme/__tests__/accessibility.test.js

@@ -3,6 +3,7 @@ import assert from 'node:assert/strict';
 import { readFile } from 'node:fs/promises';
 
 import {
+  ALIAS_TOKEN_PAIRS,
   SEMANTIC_THEME_TOKENS,
   contrastRatio,
   extractThemeTokens,
@@ -11,6 +12,21 @@ import {
 
 const stylesUrl = new URL('../../styles.css', import.meta.url);
 
+const VALID_THEME = {
+  '--background': '0 0% 100%',
+  '--card': '0 0% 100%',
+  '--border-control': '0 0% 40%',
+  '--border-structural': '0 0% 40%',
+  '--border-subtle': '0 0% 90%',
+  '--text-primary': '0 0% 10%',
+  '--text-secondary': '0 0% 40%',
+  '--text-disabled': '0 0% 40%',
+  '--icon-secondary': '0 0% 40%',
+  '--focus-ring': '0 0% 0%',
+  '--foreground': '0 0% 10%',
+  '--muted-foreground': '0 0% 40%',
+};
+
 describe('semantic accessibility theme contract (ETP-4554)', () => {
   it('calculates WCAG contrast for known pairs and accepts uppercase hex', () => {
     assert.equal(contrastRatio('#000000', '#FFFFFF'), 21);
@@ -57,24 +73,71 @@ describe('semantic accessibility theme contract (ETP-4554)', () => {
       '--focus-ring': '0 0% 10%',
     });
 
-    assert.ok(errors.some((error) => error.includes('--border-control') && error.includes('--card')));
+    assert.ok(errors.some((error) => error.includes('--border-structural') && error.includes('--card')));
   });
 
-  it('validates every semantic value and the focus ring against its control boundary', () => {
+  it('does not gate --border-control — it is a documented, accepted a11y gap', () => {
+    // WARN(a11y) in styles.css: --border-control matches staging's light color,
+    // ~1.5:1 against background/card, well short of 3:1. This must NOT fail.
     const errors = validateThemeContract({
       '--background': '0 0% 100%',
       '--card': '0 0% 100%',
-      '--border-control': '0 0% 40%',
+      '--border-control': '0 0% 95%',
+      '--border-structural': '0 0% 40%',
+      '--border-subtle': '0 0% 90%',
+      '--text-primary': '0 0% 10%',
+      '--text-secondary': '0 0% 40%',
+      '--text-disabled': '0 0% 40%',
+      '--icon-secondary': '0 0% 40%',
+      '--focus-ring': '0 0% 10%',
+    });
+
+    assert.deepEqual(errors, []);
+  });
+
+  it('validates every semantic value including the focus ring against page surfaces', () => {
+    const errors = validateThemeContract({
+      '--background': '0 0% 100%',
+      '--card': '0 0% 100%',
+      '--border-control': '0 0% 95%',
       '--border-structural': '0 0% 40%',
       '--border-subtle': 'not-a-colour',
       '--text-primary': '0 0% 10%',
       '--text-secondary': '0 0% 40%',
       '--text-disabled': '0 0% 40%',
       '--icon-secondary': '0 0% 40%',
-      '--focus-ring': '0 0% 40%',
+      '--focus-ring': '0 0% 96%',
     });
 
     assert.ok(errors.some((error) => error.includes('--border-subtle') && error.includes('invalid')));
-    assert.ok(errors.some((error) => error.includes('--focus-ring') && error.includes('--border-control')));
+    assert.ok(errors.some((error) => error.includes('--focus-ring') && error.includes('--card')));
+  });
+
+  it('passes when generic role tokens stay equal to their audited counterparts', () => {
+    assert.deepEqual(validateThemeContract(VALID_THEME), []);
+  });
+
+  it('flags a generic token that drifts from its accessibility-audited counterpart', () => {
+    // Reproduces the ETP-4659 regression: --foreground kept its unaudited shadcn
+    // default while --text-primary was corrected, so the app rendered a
+    // different ink color than the one the contrast contract had verified.
+    const drifted = { ...VALID_THEME, '--foreground': '222 47% 11%' };
+
+    const errors = validateThemeContract(drifted);
+
+    assert.ok(errors.some((error) => error.includes('--foreground') && error.includes('--text-primary')));
+  });
+
+  it('does not require every alias pair to be present to validate the rest', () => {
+    const partial = { ...VALID_THEME };
+    delete partial['--foreground'];
+
+    assert.deepEqual(validateThemeContract(partial), []);
+  });
+
+  it('keeps ALIAS_TOKEN_PAIRS pointing only at real semantic tokens', () => {
+    for (const [, canonical] of ALIAS_TOKEN_PAIRS) {
+      assert.ok(SEMANTIC_THEME_TOKENS.includes(canonical), `${canonical} must be a SEMANTIC_THEME_TOKENS entry`);
+    }
   });
 });

packages/app-shell-core/src/theme/index.js

@@ -15,6 +15,30 @@ export const SEMANTIC_THEME_TOKENS = Object.freeze([
   '--focus-ring',
 ]);
 
+/**
+ * shadcn/Tailwind's generic role tokens (--foreground, --card-foreground, ...)
+ * are what application code actually consumes via Tailwind utilities
+ * (text-foreground, ...) — SEMANTIC_THEME_TOKENS above is rarely used
+ * directly. Each generic token here must resolve to the exact same color as
+ * its audited counterpart in BOTH themes, so a future edit to one side can't
+ * silently drift the app's rendered colors away from the ETP-4554 contrast
+ * contract while this test keeps passing.
+ *
+ * Deliberately excludes --primary, --inverse, --sidebar-foreground and
+ * --sidebar-accent-foreground: those legitimately diverge from --text-primary
+ * in the dark theme (a distinct brand-blue primary, an inverted-background
+ * role, and a separately-tuned near-white sidebar text), so they are not safe
+ * to pin to a single canonical token across both themes.
+ */
+export const ALIAS_TOKEN_PAIRS = Object.freeze([
+  ['--foreground', '--text-primary'],
+  ['--card-foreground', '--text-primary'],
+  ['--popover-foreground', '--text-primary'],
+  ['--secondary-foreground', '--text-primary'],
+  ['--accent-foreground', '--text-primary'],
+  ['--muted-foreground', '--text-secondary'],
+]);
+
 function parseHex(value) {
   const match = value.trim().match(/^#([\da-f]{3}|[\da-f]{6})$/i);
   if (!match) return null;
@@ -92,14 +116,17 @@ export function validateThemeContract(theme) {
     }
   }
 
+  // WARN(a11y): --border-control is deliberately NOT gated here (see styles.css).
+  // It's back to staging's light color, which is ~1.5:1 against --background/--card —
+  // short of the WCAG 1.4.11 3:1 minimum. Known, accepted gap; do not re-add without
+  // also resolving the button-vs-input token conflict documented at its definition.
   const requirements = [
-    ['--border-control', ['--background', '--card'], 3],
     ['--border-structural', ['--background', '--card'], 3],
     ['--text-primary', ['--background', '--card'], 4.5],
     ['--text-secondary', ['--background', '--card'], 4.5],
     ['--text-disabled', ['--background', '--card'], 4.5],
     ['--icon-secondary', ['--background', '--card'], 3],
-    ['--focus-ring', ['--background', '--card', '--border-control'], 3],
+    ['--focus-ring', ['--background', '--card'], 3],
   ];
 
   for (const [token, surfaces, minimum] of requirements) {
@@ -112,5 +139,24 @@ export function validateThemeContract(theme) {
       }
     }
   }
+
+  for (const [alias, canonical] of ALIAS_TOKEN_PAIRS) {
+    if (!theme[alias] || !theme[canonical]) continue;
+    try {
+      const [ar, ag, ab] = parseColor(theme[alias]);
+      const [cr, cg, cb] = parseColor(theme[canonical]);
+      const driftedChannel = [ar - cr, ag - cg, ab - cb].some((delta) => Math.abs(delta) > 1 / 255);
+      if (driftedChannel) {
+        errors.push(
+          `${alias} (${theme[alias]}) has drifted from its accessibility-audited counterpart `
+          + `${canonical} (${theme[canonical]}); keep them equal or move ${alias} into SEMANTIC_THEME_TOKENS `
+          + 'with its own contrast requirement',
+        );
+      }
+    } catch (error) {
+      errors.push(`${alias} or ${canonical} is invalid: ${error.message}`);
+    }
+  }
+
   return errors;
 }