From 5b14839ae12c514035842d0edfd4921aee6ffe3a Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Thu, 2 Jul 2026 12:20:17 +0000
Subject: [PATCH 1/2] test(configurator): cover previewResolver.svelte.ts and
 StudioHeader's save-error state
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

SL-023 continuation, the last item in the remediation plan's PR7. Two
real gaps remained after PR4/PR5/PR6 merged and the earlier batch's
persistence.ts/SliderRow/TokenRow coverage:

- previewResolver.svelte.ts had zero test coverage. Added tests/previewResolver.test.js
  covering registerPreviewDoc/getActiveTheme, resolveColor/resolveColorForTheme's
  per-expression and per-theme caching, resolveRgb/resolveBackground's
  no-preview-doc fallbacks, and — closing the loop on PR6's SL-020
  rAF-coalescing — that bumpPreviewVersion() reliably invalidates the
  cache and is safe to call repeatedly (a pure counter, no reactive
  loop). Uses a real <iframe>'s contentDocument rather than a detached
  document, since jsdom's getComputedStyle only resolves colors on
  documents with a defaultView — matching how PreviewPanel.svelte
  registers a real iframe in production.

- StudioHeader's 'error' saveState (added by SL-018 in an earlier
  batch) had no test coverage of its own. Added a save-state describe
  block to tests-components/header.test.js covering all four states
  (idle/saving/saved/error), specifically that a failed save surfaces
  a distinct title/label from a fresh save and stays clickable to
  retry, while an in-progress retry is still correctly disabled.
  Corrected the file's baseProps fixture, which was missing
  hasPendingChanges/saveState/onSave (present since SL-018 but never
  added to this test file).

Verified: npm run test:unit (89/89), npm run test:components (23/23),
npm run test (112/112 combined), npx tsc --noEmit, npx svelte-check
(0 errors/warnings), npm run build, and a real-browser e2e smoke pass
against a built preview server (5/6 — the 1 failure is the same
pre-existing /favicon.ico 404 noted in PR5/PR6, unrelated).
---
 configurator/tests-components/header.test.js |  51 +++++-
 configurator/tests/previewResolver.test.js   | 174 +++++++++++++++++++
 2 files changed, 224 insertions(+), 1 deletion(-)
 create mode 100644 configurator/tests/previewResolver.test.js

diff --git a/configurator/tests-components/header.test.js b/configurator/tests-components/header.test.js
index 15c940ed..e34bf67f 100644
--- a/configurator/tests-components/header.test.js
+++ b/configurator/tests-components/header.test.js
@@ -2,7 +2,7 @@
  * Component smoke test for StudioHeader.
  * Verifies that the header renders the branding text and responds to props.
  */
-import { describe, test, expect } from 'vitest';
+import { describe, test, expect, vi } from 'vitest';
 import { render, screen } from '@testing-library/svelte';
 import StudioHeader from '../src/components/shell/StudioHeader.svelte';
 
@@ -10,11 +10,14 @@ const baseProps = {
   overridesCount: 0,
   canUndo: false,
   canRedo: false,
+  hasPendingChanges: false,
+  saveState: 'idle',
   onUndo: () => {},
   onRedo: () => {},
   onResetAll: () => {},
   onImport: () => {},
   onExport: () => {},
+  onSave: () => {},
 };
 
 describe('StudioHeader', () => {
@@ -44,4 +47,50 @@ describe('StudioHeader', () => {
     render(StudioHeader, { props: { ...baseProps, overridesCount: 0 } });
     expect(screen.queryByText('customised · Export →')).toBeNull();
   });
+
+  // SL-018: a failed save must be visibly distinguishable from "never
+  // attempted" (idle) or "in progress" (saving), and must stay retryable.
+  describe('save state', () => {
+    test('idle: save button is disabled and reads "Save" when there are no pending changes', () => {
+      render(StudioHeader, { props: { ...baseProps, hasPendingChanges: false, saveState: 'idle' } });
+      const btn = screen.getByTitle('No unsaved changes');
+      expect(btn).toBeDisabled();
+      expect(btn).toHaveTextContent('Save');
+    });
+
+    test('idle: save button is enabled when there are pending changes', () => {
+      render(StudioHeader, { props: { ...baseProps, hasPendingChanges: true, saveState: 'idle' } });
+      expect(screen.getByTitle('Save changes (Ctrl+S)')).not.toBeDisabled();
+    });
+
+    test('saving: button is disabled and reads "Saving…" regardless of pending changes', () => {
+      render(StudioHeader, { props: { ...baseProps, hasPendingChanges: true, saveState: 'saving' } });
+      const btn = screen.getByTitle('Save changes (Ctrl+S)');
+      expect(btn).toBeDisabled();
+      expect(btn).toHaveTextContent('Saving…');
+    });
+
+    test('saved: button reads "Saved" with a distinct title from a fresh, unattempted save', () => {
+      render(StudioHeader, { props: { ...baseProps, hasPendingChanges: false, saveState: 'saved' } });
+      const btn = screen.getByTitle('No unsaved changes');
+      expect(btn).toHaveTextContent('Saved');
+    });
+
+    test('error: button surfaces a distinct "Save failed" title and label, and stays clickable to retry', async () => {
+      const onSave = vi.fn();
+      render(StudioHeader, { props: { ...baseProps, hasPendingChanges: true, saveState: 'error', onSave } });
+      const btn = screen.getByTitle('Save failed — click to retry');
+      expect(btn).toHaveTextContent('Save failed');
+      expect(btn).not.toBeDisabled();
+      await btn.click();
+      expect(onSave).toHaveBeenCalledOnce();
+    });
+
+    test('error: a subsequent save is still blocked while actually saving', () => {
+      // Mirrors App.svelte's handleSave guard (saveState === 'saving' disables
+      // retry) even though the button was left enabled after a prior error.
+      render(StudioHeader, { props: { ...baseProps, hasPendingChanges: true, saveState: 'saving' } });
+      expect(screen.getByTitle('Save changes (Ctrl+S)')).toBeDisabled();
+    });
+  });
 });
diff --git a/configurator/tests/previewResolver.test.js b/configurator/tests/previewResolver.test.js
new file mode 100644
index 00000000..a8a9eef7
--- /dev/null
+++ b/configurator/tests/previewResolver.test.js
@@ -0,0 +1,174 @@
+/**
+ * Tests for previewResolver.svelte.ts — resolves real, browser-computed
+ * colors from the live preview iframe, with a per-version cache cleared on
+ * every bumpPreviewVersion() call (see PreviewPanel.svelte's SL-020
+ * rAF-coalesced effects, which are the production callers of that clear).
+ *
+ * Uses a real <iframe> rather than document.implementation.createHTMLDocument():
+ * jsdom's getComputedStyle only resolves colors on documents that have a
+ * defaultView, which a detached document lacks but an iframe's contentDocument
+ * has — matching how PreviewPanel.svelte registers a real iframe in production.
+ *
+ * jsdom doesn't implement CSS custom-property substitution or color functions
+ * (var()/oklch()/color-mix()), so fixtures use plain, jsdom-resolvable values
+ * (keyword/hex colors) — these tests exercise the resolver's caching/
+ * invalidation/theme-scoping logic, not real CSS cascade computation (the
+ * browser's job, already trusted).
+ */
+import { describe, test, expect, vi } from 'vitest';
+import {
+  previewVersion,
+  registerPreviewDoc,
+  getActiveTheme,
+  bumpPreviewVersion,
+  resolveColor,
+  resolveColorForTheme,
+  resolveRgb,
+  resolveBackground,
+} from '../src/lib/previewResolver.svelte';
+
+function makePreviewDoc(theme = 'light') {
+  const iframe = document.createElement('iframe');
+  document.body.appendChild(iframe);
+  const doc = iframe.contentDocument;
+  doc.documentElement.setAttribute('data-theme', theme);
+  return doc;
+}
+
+describe('registerPreviewDoc / getActiveTheme', () => {
+  test('bumps previewVersion when registering a new document', () => {
+    const before = previewVersion.value;
+    registerPreviewDoc(makePreviewDoc());
+    expect(previewVersion.value).toBeGreaterThan(before);
+  });
+
+  test('bumps previewVersion again when re-registering the same document', () => {
+    const doc = makePreviewDoc();
+    registerPreviewDoc(doc);
+    const before = previewVersion.value;
+    registerPreviewDoc(doc);
+    expect(previewVersion.value).toBeGreaterThan(before);
+  });
+
+  test('getActiveTheme reflects the registered document\'s data-theme attribute', () => {
+    registerPreviewDoc(makePreviewDoc('dark'));
+    expect(getActiveTheme()).toBe('dark');
+    registerPreviewDoc(makePreviewDoc('light'));
+    expect(getActiveTheme()).toBe('light');
+  });
+
+  test('getActiveTheme defaults to light when data-theme is absent', () => {
+    const doc = makePreviewDoc();
+    doc.documentElement.removeAttribute('data-theme');
+    registerPreviewDoc(doc);
+    expect(getActiveTheme()).toBe('light');
+  });
+});
+
+describe('resolveColor', () => {
+  test('returns "" when no preview document is registered', () => {
+    registerPreviewDoc(null);
+    expect(resolveColor('red')).toBe('');
+  });
+
+  test('resolves a CSS color as painted by the preview document', () => {
+    registerPreviewDoc(makePreviewDoc());
+    expect(resolveColor('red')).toBe('rgb(255, 0, 0)');
+  });
+
+  test('caches resolved values — a repeated call does not recompute getComputedStyle', () => {
+    const doc = makePreviewDoc();
+    registerPreviewDoc(doc);
+    const spy = vi.spyOn(doc.defaultView, 'getComputedStyle');
+    const first = resolveColor('#00ff00');
+    const callsAfterFirst = spy.mock.calls.length;
+    const second = resolveColor('#00ff00');
+    expect(second).toBe(first);
+    expect(spy.mock.calls.length).toBe(callsAfterFirst);
+    spy.mockRestore();
+  });
+
+  // Closes the loop on SL-020: PreviewPanel.svelte's rAF-coalesced effects
+  // rely on bumpPreviewVersion() reliably invalidating the cache every time
+  // it runs, however rarely (once per frame) it now gets called.
+  test('bumpPreviewVersion() invalidates the cache — a cached expression is recomputed after bumping', () => {
+    const doc = makePreviewDoc();
+    registerPreviewDoc(doc);
+    const spy = vi.spyOn(doc.defaultView, 'getComputedStyle');
+    resolveColor('blue');
+    const callsBeforeBump = spy.mock.calls.length;
+    bumpPreviewVersion();
+    resolveColor('blue');
+    expect(spy.mock.calls.length).toBeGreaterThan(callsBeforeBump);
+    spy.mockRestore();
+  });
+
+  test('bumpPreviewVersion can be called repeatedly without throwing (pure counter, no reactive loop)', () => {
+    registerPreviewDoc(makePreviewDoc());
+    const before = previewVersion.value;
+    expect(() => {
+      for (let i = 0; i < 50; i++) bumpPreviewVersion();
+    }).not.toThrow();
+    expect(previewVersion.value).toBe(before + 50);
+  });
+});
+
+describe('resolveColorForTheme', () => {
+  test('resolves via an independent probe per theme, scoped under a [data-theme] wrapper', () => {
+    registerPreviewDoc(makePreviewDoc());
+    expect(resolveColorForTheme('red', 'light')).toBe('rgb(255, 0, 0)');
+    expect(resolveColorForTheme('blue', 'dark')).toBe('rgb(0, 0, 255)');
+  });
+
+  test('returns "" when no preview document is registered', () => {
+    registerPreviewDoc(null);
+    expect(resolveColorForTheme('red', 'light')).toBe('');
+  });
+
+  test('caches per theme+expression key independently of resolveColor\'s cache', () => {
+    const doc = makePreviewDoc();
+    registerPreviewDoc(doc);
+    const spy = vi.spyOn(doc.defaultView, 'getComputedStyle');
+    resolveColorForTheme('green', 'dark');
+    const callsAfterFirst = spy.mock.calls.length;
+    resolveColorForTheme('green', 'dark');
+    expect(spy.mock.calls.length).toBe(callsAfterFirst);
+    // A different theme with the same expression is a different cache key —
+    // expect at least one more getComputedStyle call, not a cache hit.
+    resolveColorForTheme('green', 'light');
+    expect(spy.mock.calls.length).toBeGreaterThan(callsAfterFirst);
+    spy.mockRestore();
+  });
+});
+
+describe('resolveRgb', () => {
+  test('returns null when no preview document is registered', () => {
+    registerPreviewDoc(null);
+    expect(resolveRgb('red')).toBeNull();
+  });
+
+  // This sandbox has no `canvas` npm package installed, so jsdom's
+  // HTMLCanvasElement.getContext('2d') is unimplemented and logs (but does
+  // not throw) a "not implemented" error — resolveRgb must degrade to null
+  // rather than propagating that. Silencing the expected console.error here
+  // to keep test output clean; the assertion is what matters.
+  test('degrades to null (not a throw) when no 2D canvas context is available', () => {
+    registerPreviewDoc(makePreviewDoc());
+    const spy = vi.spyOn(console, 'error').mockImplementation(() => {});
+    expect(() => resolveRgb('red')).not.toThrow();
+    expect(resolveRgb('red')).toBeNull();
+    spy.mockRestore();
+  });
+});
+
+describe('resolveBackground', () => {
+  test('returns "" when no preview document is registered', () => {
+    registerPreviewDoc(null);
+    expect(resolveBackground('linear-gradient(red, blue)')).toBe('');
+  });
+
+  test('returns the resolved background-image as painted by the preview', () => {
+    registerPreviewDoc(makePreviewDoc());
+    expect(resolveBackground('linear-gradient(red, blue)')).toContain('gradient');
+  });
+});

From 156e4214abd0d6e781d56f0b1956113656c710ee Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Thu, 2 Jul 2026 12:27:46 +0000
Subject: [PATCH 2/2] fix(configurator): clean up leaked test iframes and
 decouple canvas test from environment
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Qodo review on PR7 flagged two real issues in tests/previewResolver.test.js:

1. makePreviewDoc() appended a new <iframe> to document.body on every
   call with no teardown — registerPreviewDoc() only cleans up the
   resolver's internal probe elements, not the test-created iframes
   themselves, so they accumulated across the file's 16+ calls. Added
   an afterEach that removes every iframe makePreviewDoc() created and
   unregisters the preview doc.

2. The canvas-degrades-to-null test asserted resolveRgb('red') returns
   null based on this sandbox's jsdom lacking the optional `canvas` npm
   package, rather than the documented contract — if canvas support
   ever became available the test would start failing for an unrelated
   reason. Replaced with an explicit getContext('2d') stub, plus a new
   companion test asserting the positive path (a real 2D context
   available) returns an [r,g,b] triple.

   Fixing this surfaced a real subtlety Qodo's own suggested fix would
   have missed: each iframe is a separate jsdom realm with its own
   HTMLCanvasElement constructor, so a canvas created via
   activeDoc.createElement('canvas') is an instance of
   doc.defaultView.HTMLCanvasElement, not the top-level test file's
   global HTMLCanvasElement — spying on the wrong one is a silent no-op
   (verified: the positive-path test failed until the spy target was
   corrected to the iframe's own realm).

Verified: npm run test:unit (90/90, up from 89 with the new positive-
path test), npx tsc --noEmit, npx svelte-check (0 errors/warnings), and
a standalone check confirming zero leftover <iframe> elements in
document.body after the full previewResolver suite runs.
---
 configurator/tests/previewResolver.test.js | 47 ++++++++++++++++++----
 1 file changed, 39 insertions(+), 8 deletions(-)

diff --git a/configurator/tests/previewResolver.test.js b/configurator/tests/previewResolver.test.js
index a8a9eef7..49c068af 100644
--- a/configurator/tests/previewResolver.test.js
+++ b/configurator/tests/previewResolver.test.js
@@ -15,7 +15,7 @@
  * invalidation/theme-scoping logic, not real CSS cascade computation (the
  * browser's job, already trusted).
  */
-import { describe, test, expect, vi } from 'vitest';
+import { describe, test, expect, vi, afterEach } from 'vitest';
 import {
   previewVersion,
   registerPreviewDoc,
@@ -27,14 +27,26 @@ import {
   resolveBackground,
 } from '../src/lib/previewResolver.svelte';
 
+// Tracks every <iframe> created by makePreviewDoc() so afterEach can remove
+// them — registerPreviewDoc() only tears down the resolver's own internal
+// probe elements, not the fixtures this file appends to document.body.
+let createdIframes = [];
+
 function makePreviewDoc(theme = 'light') {
   const iframe = document.createElement('iframe');
   document.body.appendChild(iframe);
+  createdIframes.push(iframe);
   const doc = iframe.contentDocument;
   doc.documentElement.setAttribute('data-theme', theme);
   return doc;
 }
 
+afterEach(() => {
+  registerPreviewDoc(null);
+  createdIframes.forEach((iframe) => iframe.remove());
+  createdIframes = [];
+});
+
 describe('registerPreviewDoc / getActiveTheme', () => {
   test('bumps previewVersion when registering a new document', () => {
     const before = previewVersion.value;
@@ -147,18 +159,37 @@ describe('resolveRgb', () => {
     expect(resolveRgb('red')).toBeNull();
   });
 
-  // This sandbox has no `canvas` npm package installed, so jsdom's
-  // HTMLCanvasElement.getContext('2d') is unimplemented and logs (but does
-  // not throw) a "not implemented" error — resolveRgb must degrade to null
-  // rather than propagating that. Silencing the expected console.error here
-  // to keep test output clean; the assertion is what matters.
+  // Explicitly stubbing getContext('2d') to return null (rather than relying
+  // on this sandbox's jsdom having no `canvas` npm package installed) so this
+  // test asserts resolveRgb's documented contract, not an environment detail
+  // that would silently start failing if a canvas backend became available.
+  //
+  // Spies on the preview *iframe's own* HTMLCanvasElement, not the top-level
+  // test file's global one — each iframe is a separate jsdom realm with its
+  // own constructors, so a canvas created via activeDoc.createElement('canvas')
+  // is an instance of doc.defaultView.HTMLCanvasElement, not window.HTMLCanvasElement.
   test('degrades to null (not a throw) when no 2D canvas context is available', () => {
-    registerPreviewDoc(makePreviewDoc());
-    const spy = vi.spyOn(console, 'error').mockImplementation(() => {});
+    const doc = makePreviewDoc();
+    registerPreviewDoc(doc);
+    const spy = vi.spyOn(doc.defaultView.HTMLCanvasElement.prototype, 'getContext').mockReturnValue(null);
     expect(() => resolveRgb('red')).not.toThrow();
     expect(resolveRgb('red')).toBeNull();
     spy.mockRestore();
   });
+
+  test('converts a resolved color to an [r,g,b] triple when a 2D context is available', () => {
+    const doc = makePreviewDoc();
+    registerPreviewDoc(doc);
+    const fakeCtx = {
+      clearRect: vi.fn(),
+      fillRect: vi.fn(),
+      fillStyle: '',
+      getImageData: vi.fn(() => ({ data: new Uint8ClampedArray([255, 0, 0, 255]) })),
+    };
+    const spy = vi.spyOn(doc.defaultView.HTMLCanvasElement.prototype, 'getContext').mockReturnValue(fakeCtx);
+    expect(resolveRgb('red')).toEqual([255, 0, 0]);
+    spy.mockRestore();
+  });
 });
 
 describe('resolveBackground', () => {