perf(configurator): rAF-coalesce preview iframe writes during rapid drags (SL-020)

[jackgranatowski]

Summary

Sixth themed PR from the SLASHED technical-debt audit (PR #469): the configurator preview debounce. Targets the long-lived integration branch claude/pr-469-audit-rebase-ggp0e4, after PR5 per the remediation plan's ordering.

• SL-020: PreviewPanel.svelte's two $effects that write to the preview iframe(s) (style/attribute mutation, font injection) and call bumpPreviewVersion() re-run on every overrides tick — e.g. every input event while dragging a slider, often several times per animation frame. Deferred the DOM-writing work inside each effect to a requestAnimationFrame callback that closes over the latest captured values, with the effect's cleanup (return () => cancelAnimationFrame(rafId)) cancelling any not-yet-fired frame from a superseded run before scheduling the next one. This collapses a same-frame burst of reruns into a single apply using the last state, instead of one apply per keystroke.
• Reactive dependency tracking is unaffected: overrides/previewTheme/loadCount/etc. are still read synchronously at the top of each effect (unchanged from before), so Svelte's re-run triggering is untouched — only the actual DOM-writing work is deferred.
• previewResolver.svelte.ts's bumpPreviewVersion() itself is untouched, preserving its existing documented invariant ("pure write, no reactive read" — avoids an effect_update_depth infinite loop). The coalescing lives entirely at the call site in PreviewPanel.svelte, per the plan.
• Out of scope (per the plan): persistence.ts's separate injectLivePreview call path (used for the App-level live <style> tag, not this iframe preview) has no debounce either — that's a different path, addressed independently in SLASHED-Plugins' PR-C3 for the frontend-overlay case.

Test plan

• npx tsc --noEmit — clean
• npx svelte-check --tsconfig ./tsconfig.json — 0 errors, 0 warnings
• npm run test:unit — 72/72 passed
• npm run test:components — 17/17 passed
• npm run build — succeeds
• Manual verification (per the plan's requirement): built and served the app (vite preview), then drove a real headless Chromium session to rapid-fire 40 synthetic input events on a slider — faster than one per animation frame — simulating an aggressive drag:
  • Zero console/page errors, specifically none matching effect_update_depth/"Maximum update depth"/infinite-loop patterns
  • The preview iframe's actually-applied <style id="slashed-overrides"> content exactly matched the last dispatched value (--sf-code-font-size: 1.1em, the final value in the simulated drag) — confirming no dropped trailing frame after the burst settles
  • Existing tests-e2e/shell.spec.js smoke suite: 5/6 passing against the built preview server (the 1 failure is the same pre-existing /favicon.ico 404 noted in PR5, unrelated to this change)

---

Generated by Claude Code

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 855e168f-c732-4b53-a5ec-e78299582692

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/audit-pr6-preview-debounce

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[qodo-code-review]

PR Summary by Qodo

perf(configurator): rAF-coalesce preview iframe DOM writes during rapid drags

✨ Enhancement 🕐 20-40 Minutes

AI Description

• Coalesce preview-iframe DOM mutations to one apply per animation frame during rapid override
 changes.
• Preserve Svelte reactive dependency tracking by only deferring DOM writes, not reads.
• Cancel superseded scheduled frames via effect cleanup to prevent stale iframe updates.

Diagram

graph TD
S(["Overrides & theme"]) --> P["PreviewPanel $effects"] --> RAF["rAF coalescer"] --> I["Preview iframe DOM"] --> R["previewResolver"] --> U["Consumers (swatches)"]
R --> Cache[("resolveCache")]
U --> R

subgraph Legend
direction LR
_state(["Reactive state"]) ~~~ _work["Effect / work"] ~~~ _db[("Cache")]
end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Debounce/throttle overrides at the input layer

• ➕ Reduces recomputation across the whole app, not just preview iframe writes
• ➕ May lower overall CPU usage during drags beyond the preview subsystem
• ➖ Changes UX semantics (lag/step behavior) and can affect non-preview consumers
• ➖ Broader surface area and harder to reason about correctness across features

2. Extract a shared helper (e.g., rafCoalescedEffect)

• ➕ Removes duplicated rAF+cleanup pattern across effects
• ➕ Centralizes cancellation semantics and makes future usage consistent
• ➖ Introduces abstraction around Svelte effect lifecycle that can be misused
• ➖ Adds indirection for a localized, already-small change

3. Microtask batching (queueMicrotask/Promise) instead of rAF

• ➕ Lower latency than waiting for the next frame in some cases
• ➕ Simpler scheduling model if you only need same-tick coalescing
• ➖ Still can execute multiple times per animation frame under heavy input
• ➖ More likely to compete with layout/paint work than rAF for DOM writes

Recommendation: Keep the current localized requestAnimationFrame coalescing in PreviewPanel. It minimizes behavioral risk, aligns writes with the browser paint loop, and preserves the existing previewResolver invariant (no reactive reads) while materially reducing redundant iframe DOM work during slider drags.

Files changed (1) +64 / -43

Enhancement (1) +64 / -43

PreviewPanel.svelteCoalesce preview iframe updates via rAF inside both DOM-writing $effects +64/-43

Coalesce preview iframe updates via rAF inside both DOM-writing $effects

• Wraps the single-iframe and split-mode preview update effects in requestAnimationFrame callbacks and cancels pending frames on effect cleanup. Reactive reads (overrides/theme/load counts) remain synchronous to preserve dependency tracking, while style text updates, font injection, attribute mutation, registerPreviewDoc, and bumpPreviewVersion are deferred and coalesced per frame.

configurator/src/components/shell/PreviewPanel.svelte

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 6 rules

1. Redundant previewVersion bumps ✓ Resolved 🐞 Bug ➹ Performance

Description

PreviewPanel calls bumpPreviewVersion() immediately after registerPreviewDoc(doc), but
registerPreviewDoc() already calls bumpPreviewVersion() internally, causing duplicate previewVersion
increments and resolveCache.clear() per apply. This triggers unnecessary downstream
recomputation/cache invalidation during rapid override updates (and can happen multiple times per
frame across the two effects).

Code

configurator/src/components/shell/PreviewPanel.svelte[R523-526]

+      // This single-mode iframe is the canonical resolver source.
+      registerPreviewDoc(doc);
+      bumpPreviewVersion();
+    });

Relevance

⭐⭐⭐ High

Team accepts removing redundant work (e.g., dead assignment removal in previewResolver). Duplicate
bump/cache-clear likely trimmed.

PR-#429

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The PreviewPanel effect explicitly calls bumpPreviewVersion() after registerPreviewDoc(), while
registerPreviewDoc() itself calls bumpPreviewVersion() unconditionally (either in the
activeDoc===doc early-return or after replacing the doc). That makes the explicit bump redundant and
causes extra version increments/cache clears.

configurator/src/components/shell/PreviewPanel.svelte[496-529]
configurator/src/lib/previewResolver.svelte.ts[48-76]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`registerPreviewDoc(doc)` already calls `bumpPreviewVersion()` (including `resolveCache.clear()`), but `PreviewPanel.svelte` calls `bumpPreviewVersion()` again immediately afterwards. This double-bumps `previewVersion.value`, causing dependent `$derived`/`$effect` computations (e.g. swatch resolving) to rerun more than necessary.

### Issue Context
- `registerPreviewDoc()` bumps on both doc change and doc re-registration.
- The extra explicit `bumpPreviewVersion()` in `PreviewPanel.svelte` is therefore redundant in single-iframe mode and likely redundant in split mode when `registerPreviewDoc()` is hit.

### Fix Focus Areas
- configurator/src/components/shell/PreviewPanel.svelte[523-526]
- configurator/src/lib/previewResolver.svelte.ts[48-76]

### Suggested fix
- In the single-iframe effect: remove the explicit `bumpPreviewVersion()` after `registerPreviewDoc(doc)`.
- In split mode: ensure you only bump once per rAF apply. Options:
 - If `registerPreviewDoc(doc)` is called (light pane present), rely on its internal bump and avoid a second unconditional bump; OR
 - Refactor `registerPreviewDoc` to support a `skipBump`/`bump` option so callers can control when the single bump happens after both panes update.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Split effect runs always ✓ Resolved 🐞 Bug ➹ Performance

Description

The split-mode $effect schedules a requestAnimationFrame, generates CSS, and calls
bumpPreviewVersion() even when splitMode is false (i.e., split iframes are not rendered). This adds
avoidable per-frame work and extra reactive churn during rapid drags in the normal single-iframe
mode.

Code

configurator/src/components/shell/PreviewPanel.svelte[R531-568]

  $effect(() => {
    const _ov = overrides;
    const _lightCount = splitLightLoadCount;
    const _darkCount = splitDarkLoadCount;
    const _lock = lumlockerPreview.value;
-    const css = generateCSS(withDerivedOverrides(_ov), { mode: "root", banner: false });

-    const applyLock = (doc: Document) => {
-      if (_lock) doc.documentElement.setAttribute("data-lumlocker", "");
-      else doc.documentElement.removeAttribute("data-lumlocker");
-    };
-
-    if (splitLightEl && _lightCount > 0) {
-      const doc = splitLightEl.contentDocument;
-      if (doc) {
-        const styleEl = doc.getElementById("slashed-overrides");
-        if (styleEl) styleEl.textContent = css;
-        injectFontsIntoDoc(doc, _ov);
-        applyLock(doc);
-        // In split mode the light pane is the canonical resolver source.
-        registerPreviewDoc(doc);
+    const rafId = requestAnimationFrame(() => {
+      const css = generateCSS(withDerivedOverrides(_ov), { mode: "root", banner: false });
+
+      const applyLock = (doc: Document) => {
+        if (_lock) doc.documentElement.setAttribute("data-lumlocker", "");
+        else doc.documentElement.removeAttribute("data-lumlocker");
+      };
+
+      if (splitLightEl && _lightCount > 0) {
+        const doc = splitLightEl.contentDocument;
+        if (doc) {
+          const styleEl = doc.getElementById("slashed-overrides");
+          if (styleEl) styleEl.textContent = css;
+          injectFontsIntoDoc(doc, _ov);
+          applyLock(doc);
+          // In split mode the light pane is the canonical resolver source.
+          registerPreviewDoc(doc);
+        }
      }
-    }
-    if (splitDarkEl && _darkCount > 0) {
-      const doc = splitDarkEl.contentDocument;
-      if (doc) {
-        const styleEl = doc.getElementById("slashed-overrides");
-        if (styleEl) styleEl.textContent = css;
-        injectFontsIntoDoc(doc, _ov);
-        applyLock(doc);
+      if (splitDarkEl && _darkCount > 0) {
+        const doc = splitDarkEl.contentDocument;
+        if (doc) {
+          const styleEl = doc.getElementById("slashed-overrides");
+          if (styleEl) styleEl.textContent = css;
+          injectFontsIntoDoc(doc, _ov);
+          applyLock(doc);
+        }
      }
-    }
-    bumpPreviewVersion();
+      bumpPreviewVersion();
+    });
+
+    return () => cancelAnimationFrame(rafId);

Relevance

⭐⭐⭐ High

They’ve accepted guarding expensive $effects when UI/feature inactive (avoid recompute when
collapsed). Similar early-return expected.

PR-#433
PR-#440

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The split-mode effect unconditionally schedules rAF and bumps previewVersion, while the component
only renders split iframes when splitMode is true; therefore in single mode this effect still
performs work without any DOM to apply it to.

configurator/src/components/shell/PreviewPanel.svelte[531-569]
configurator/src/components/shell/PreviewPanel.svelte[681-734]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The split-preview `$effect` runs on every overrides tick regardless of whether split mode is active, and it always schedules an rAF callback that computes CSS and bumps `previewVersion`. When `splitMode` is false, there are no split iframes to update, so this becomes pure overhead.

### Issue Context
The template only renders the split iframes under `{#if splitMode}`; in single mode `splitLightEl`/`splitDarkEl` will be null, but the effect still computes `css` and bumps the version.

### Fix Focus Areas
- configurator/src/components/shell/PreviewPanel.svelte[531-569]
- configurator/src/components/shell/PreviewPanel.svelte[681-734]

### Suggested fix
- Read `splitMode` at the top of the split effect and return early when it is `false` (so the effect becomes a no-op in single mode).
- Alternatively (or additionally), only compute `css` and call `bumpPreviewVersion()` when at least one split iframe document is actually present/loaded.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[jackgranatowski]

Both good catches — fixed in 0515efa:

1. Redundant previewVersion bumps: registerPreviewDoc() already bumps internally on every code path, so the explicit bumpPreviewVersion() right after it in the single-iframe effect was always a double-bump/double-cache-clear. Removed it.
2. Split effect runs always: added an early return on !splitMode in the split-preview effect, since its iframes only exist in the DOM under {#if splitMode} — it was doing per-frame no-op work in the normal single-iframe mode. Also made its trailing bump conditional (only fires when the dark pane updated without the light pane also updating, since registerPreviewDoc's internal bump already covers that case) so consumers still get notified exactly once per actual change.

Re-verified with the same rapid-drag simulation as the original commit, now covering both single- and split-preview modes: zero console/page errors, both modes' applied CSS matched the last dragged value exactly.

---

Generated by Claude Code