refactor(configurator): shape-guard localStorage, migrate lucide-svelte (SL-015/018/019/021/022/025)

[jackgranatowski]

Summary

Fifth themed PR from the SLASHED technical-debt audit (PR #469), covering the configurator correctness/UX/dependency-cleanup group. Targets the long-lived integration branch claude/pr-469-audit-rebase-ggp0e4 (parallel-safe with PR4 per the remediation plan — both touch App.svelte/CheatsheetPanel.svelte but on non-overlapping lines).

• SL-019: persistence.ts's loadInitialOverrides() now reuses savedThemes.ts's existing isStringRecord() shape guard (exported for this purpose) instead of trusting JSON.parse(local) as Record<string, string> unchecked. Malformed or wrong-shaped localStorage content now falls back to {} instead of passing through as-is. Updated the two existing tests/persistence.test.js cases that pinned the old (unguarded) behavior, and added a case for a record with a non-string value.
• SL-022: migrated lucide-svelte → @lucide/svelte across all 9 importing files (import-source only — no icon name changes were needed; verified with tsc/svelte-check/a full vite build after the migration). Removed lucide-svelte from configurator/package.json, ran npm install to update the lockfile, and reran npm run check:version per CLAUDE.md's rule for touching configurator/package.json.
• SL-025: added a comment on previewResolver.svelte.ts's previewVersion explaining why it's deliberately a module-level singleton (one shared preview iframe for the whole app) rather than component-scoped state.
• SL-018: investigated, no change needed — the 'error' saveState, the catch-handler wiring, and the StudioHeader render branch for it were already fully implemented in an earlier batch.
• SL-015: investigated, no change needed — the finding assumes two competing "is this token overridden" idioms (name in overrides vs overrides[name] !== undefined) that need unifying into a helper. Only the in idiom actually exists in this codebase, consistently, across ~120 call sites in ~15 panel files (well beyond the plan's ~15-site/4-file estimate). There's nothing to reconcile, and key in overrides is already a clear, terse, standard idiom — introducing a helper would add indirection across ~120 sites for no correctness or readability gain. Confirmed with the repo owner before skipping.
• SL-021: verified already resolved (no motion dependency or imports remain in the configurator).

Test plan

• npx tsc --noEmit — clean
• npx svelte-check --tsconfig ./tsconfig.json — 0 errors, 0 warnings
• npm run test:unit — 73/73 passed
• npm run test:components — 17/17 passed
• npm run build (full vite build) — succeeds with the new icon package
• npm audit — 0 vulnerabilities
• npm run check:version (root) — all references in sync after touching configurator/package.json
• Manual Playwright e2e smoke pass (tests-e2e/shell.spec.js) against a real built preview server — 5/6 passing; the 1 failure is a pre-existing /favicon.ico 404 console error unrelated to this PR (index.html never referenced a favicon, confirmed independent of this diff)

---

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: 78c88662-edaf-42b4-ade5-f3af4a9d415a

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-pr5-configurator-ux

---

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

Configurator: validate localStorage overrides and migrate to @lucide/svelte

🐞 Bug fix ⚙️ Configuration changes 🧪 Tests 🕐 20-40 Minutes

AI Description

• Validate persisted overrides using a shared shape guard; malformed localStorage now loads as {}.
• Migrate all configurator icon imports from lucide-svelte to @lucide/svelte.
• Update persistence unit tests and document the shared preview iframe version singleton.

Diagram

graph TD
  A["UI panels/components"] --> B["persistence.ts"] --> C["localStorage overrides"]
  B --> D["savedThemes.ts (isStringRecord)"]
  A --> E["@lucide/svelte icons"]
  B --> F["URL hash share"]
  A --> G["previewResolver (previewVersion)"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Inline a local shape guard in persistence.ts

• ➕ Avoids coupling persistence.ts to savedThemes.ts
• ➕ Keeps persistence validation self-contained
• ➖ Duplicates logic already present in savedThemes.ts
• ➖ Higher risk of guards drifting apart over time

2. Adopt a schema validator (e.g., zod) for persisted payloads

• ➕ More expressive validation with clearer error reporting
• ➕ Scales better if persisted format becomes more complex
• ➖ Adds a dependency and bundle cost for a small validation need
• ➖ Overkill for a simple Record check

3. Create an internal icon wrapper module (re-export icons)

• ➕ Future package migrations only touch one file
• ➕ Can standardize icon import conventions
• ➖ Adds indirection and may worsen tree-shaking depending on bundler config
• ➖ Immediate migration already mechanical and low-cost

Recommendation: Current approach is the best tradeoff: exporting and reusing isStringRecord() centralizes the correctness fix with minimal code and no new dependencies, and the direct @lucide/svelte migration keeps changes explicit and easy to verify via typecheck/build. A schema library or icon wrapper module only becomes worthwhile if persisted formats or icon sourcing change frequently.

Files changed (14) +37 / -21

Bug fix (1) +5 / -1

persistence.tsShape-guard localStorage overrides during initial load +5/-1

Shape-guard localStorage overrides during initial load

• Stops trusting JSON.parse(localStorage) as Record<string,string> and validates the parsed value with isStringRecord(). Malformed or wrong-shaped persisted content now falls back to {}.

configurator/src/lib/persistence.ts

Refactor (10) +10 / -10

App.svelteUpdate icon imports to @lucide/svelte +1/-1

Update icon imports to @lucide/svelte

• Changes the SlidersHorizontal/Eye/RotateCcw import source to @lucide/svelte. No runtime logic changes.

configurator/src/App.svelte

DomainPanel.svelteUpdate icon imports to @lucide/svelte +1/-1

Update icon imports to @lucide/svelte

• Migrates SlidersHorizontal/List imports from lucide-svelte to @lucide/svelte. No behavior changes.

configurator/src/components/DomainPanel.svelte

CheatsheetPanel.svelteUpdate icon imports to @lucide/svelte +1/-1

Update icon imports to @lucide/svelte

• Migrates Copy/Check imports from lucide-svelte to @lucide/svelte. Panel behavior remains the same.

configurator/src/components/panels/CheatsheetPanel.svelte

ExportPanel.svelteUpdate icon imports to @lucide/svelte +1/-1

Update icon imports to @lucide/svelte

• Migrates Check/Copy/Download/Link imports to @lucide/svelte. No logic changes in export/share behavior.

configurator/src/components/panels/ExportPanel.svelte

HomePanel.svelteUpdate icon imports to @lucide/svelte +1/-1

Update icon imports to @lucide/svelte

• Switches the large icon list import source to @lucide/svelte. No functional changes.

configurator/src/components/panels/HomePanel.svelte

ThemesPanel.svelteUpdate icon imports to @lucide/svelte +1/-1

Update icon imports to @lucide/svelte

• Migrates Save/Trash2/Check imports to @lucide/svelte. Theme save/delete logic is unchanged.

configurator/src/components/panels/ThemesPanel.svelte

PreviewPanel.svelteUpdate icon imports to @lucide/svelte +1/-1

Update icon imports to @lucide/svelte

• Migrates device/refresh/link icon imports to @lucide/svelte. Preview rendering behavior is unchanged.

configurator/src/components/shell/PreviewPanel.svelte

SidebarNav.svelteUpdate icon imports to @lucide/svelte +1/-1

Update icon imports to @lucide/svelte

• Migrates sidebar navigation icon imports to @lucide/svelte. No navigation logic changes.

configurator/src/components/shell/SidebarNav.svelte

StudioHeader.svelteUpdate icon imports to @lucide/svelte +1/-1

Update icon imports to @lucide/svelte

• Migrates header action/status icon imports to @lucide/svelte. No header behavior changes.

configurator/src/components/shell/StudioHeader.svelte

savedThemes.tsExport isStringRecord() shape guard for reuse +1/-1

Export isStringRecord() shape guard for reuse

• Exports the existing isStringRecord() helper so other modules (persistence) can reuse the same Record<string,string> validation logic. No behavior change within savedThemes itself.

configurator/src/lib/savedThemes.ts

Tests (1) +12 / -8

persistence.test.jsUpdate and extend tests for guarded localStorage behavior +12/-8

Update and extend tests for guarded localStorage behavior

• Updates SL-019 expectations so non-object JSON now yields {} instead of passing through. Adds a new test asserting that non-string values in the stored record also cause a {} fallback.

configurator/tests/persistence.test.js

Documentation (1) +8 / -0

previewResolver.svelte.tsDocument module-level previewVersion singleton rationale +8/-0

Document module-level previewVersion singleton rationale

• Adds an explanatory comment describing why previewVersion is intentionally module-scoped and wrapped in an object for rune reactivity. No runtime behavior changes.

configurator/src/lib/previewResolver.svelte.ts

Other (1) +2 / -2

package.jsonReplace lucide-svelte with @lucide/svelte dependency +2/-2

Replace lucide-svelte with @lucide/svelte dependency

• Swaps the icon library dependency from lucide-svelte to @lucide/svelte. Keeps other dependencies unchanged.

configurator/package.json

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review