feat(studio): storyboard markdown source editor (raw + live preview)

[jrusso1020]

Storyboarding — PR 4/5: markdown source editor (raw + live preview)

An in-context way to view and edit the storyboard's canonical files, without leaving the storyboard view.

What's here

• Board | Source sub-toggle inside the storyboard view.
• StoryboardSourceEditor — raw CodeMirror markdown editor (left) + live rendered preview (right), with file tabs for STORYBOARD.md and SCRIPT.md.
• Loads/saves through the shared file-manager API (retry, path-encoding, error typing); Cmd/Ctrl+S to save; on save the Board re-parses, so markdown stays the single source of truth.
• Preview is rendered with marked and sanitized with DOMPurify before injection (the preview runs in the studio origin, so untrusted file content must not execute).
• Deliberately raw, not WYSIWYG, so the structured frame fields (status:, src:, frontmatter) can't be mangled.
• Preview parse is debounced so typing stays responsive on large files.

New deps

marked (preview), @codemirror/lang-markdown (highlighting), dompurify (sanitization).

---

Stack: #1528 → #1529 → #1530 → #1531 → #1532

[jrusso1020]

• feat(studio): storyboard frame focus + voiceover iteration #1532
• feat(studio): storyboard markdown source editor (raw + live preview) #1531 👈 (View in Graphite)
• feat(studio): storyboard frame contact-sheet grid #1530
• feat(studio): storyboard view-mode toggle and shell #1529
• feat(studio): add storyboard manifest contract, parser, and read API #1528
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[miga-heygen]

Approve with suggestions. Well-structured split into Board/Source sub-views with proper ARIA tabs.

The CodeMirror raw editor + debounced DOMPurify-sanitized live preview is the right architecture. Cmd/Ctrl+S keyboard shortcut with status indicator is good UX. The useEditableFile hook cleanly tracks dirty state.

Items worth addressing:

• No unsaved-changes guard on tab switch or navigation away — dirty edits silently discarded.
• marked.parse() returns string | Promise<string> in marked v14+. Explicitly opt into sync: marked.parse(text, { async: false }).
• useMarkdownPreview memo deps include the full data.script object reference, making the memo decorative — depend on data.script?.path and data.script?.exists instead.
• New deps (marked, @codemirror/lang-markdown, dompurify) are well-chosen.

— Miga

[james-russo-rames-d-jusso]

Reviewed at feb9ceb. The shape is clean — marked + DOMPurify with debounced parse, Cmd/Ctrl+S to save, useEditableFile tracks dirty/saved/loading/saving/error sanely, file-tab list reconciles when the underlying file list changes. Markdown stays the canonical source of truth and the Board re-parses on save via onSaved → reload. Deliberate raw editor (not WYSIWYG) is the right call given #1528's frame-field structure.

A few non-blocking concerns and a couple of nits — none block merge given the ENV-gated iterative-shipping intent.

Concerns

• Unsaved edits lost on Board↔Source toggle. StoryboardSourceEditor is mounted inside the subView === "board" ? … : <StoryboardSourceEditor … /> ternary in StoryboardLoaded.tsx (parent of this PR's component), so flipping back to "Board" unmounts it and discards the in-memory content state. The next remount re-reads from disk. With Cmd/Ctrl+S as the only save path and no toggle guard, a user who types into Source then clicks Board silently loses unsaved edits. Iterative-OK to defer the confirm-discard prompt, but worth either (a) a tiny if (file.dirty && !confirm("Discard unsaved markdown?")) on the toggle, or (b) hoisting the useEditableFile state up so Source remembers across toggles. Cheap to do now; trust-corrosive to walk back later.
• Concurrent save / read-modify-write coupling with #1532. writeProjectFile in useFileManager.ts is last-write-wins with no etag/version. If a focus-mode voiceover edit (#1532) lands while the Source editor has stale saved, the user's Cmd+S in Source then overwrites the focus-mode change without warning. Since #1532's focus view replaces the Source view entirely (mutually exclusive in the UI), the only window is "edit in Source, focus elsewhere via Board, focus-mode mutates STORYBOARD.md, switch back to Source which is now showing stale text." Pre-existing studio concurrency model, so not a #1531 regression — flag for the broader storyboard editing story.
• Preview prose styling skips images. PREVIEW_PROSE styles h1-h3 / p / ul / ol / code / pre / hr / a / strong / table / th / td but not img. DOMPurify allows <img> by default (with event handlers stripped, which is correct), so pasted images render with no constraints — they could blow past the right pane width. Minor; add [&_img]:max-w-full [&_img]:h-auto if you care.

Nits

• StoryboardSourceEditor.tsx:140 — useMarkdownPreview debounces at 200ms. On large files (the PR body mentions this case explicitly) the first paint shows an empty preview for 200ms even though source is immediately available. Minor: initialize debounced only when source changes, but seed the synchronous parse on first render so the initial paint is non-empty. (nit)
• StoryboardSourceEditor.tsx:140 — marked v14 may return Promise<string> if any async extensions are registered. You guard with typeof raw === "string" ? raw : "", which silently produces an empty preview if it ever becomes async. Either pass { async: false } to marked.parse or surface "preview unavailable" instead of blank. (nit)
• useEditableFile .save() doesn't cancel an in-flight save; mashing Cmd+S fires two PUTs. Each writes the content captured at dispatch time, so the second is usually a no-op — but if content changed between them, you can briefly write older content. Guard with if (saving) return or capture an inflight token. (nit)
• StoryboardLoaded.tsx:31 — data.script?.exists toggles the SCRIPT.md tab, but if a user creates SCRIPT.md from outside the studio while Source is open, the tab list doesn't refresh until the next reload. (nit)

Questions

• What's the intended UX for a parse error while typing partially-valid markdown? Today the preview re-renders sanitized HTML from marked on every debounce tick — marked is forgiving, so it almost never throws, but is the intent "show whatever marked produces" or "show last-good + error banner"? Today's behavior is the former.

What I didn't verify

• Bundle-size impact of marked + DOMPurify added to studio. Listed for context — iterative-shipping rubric says don't blocker on this, but if studio first-load matters, lazy-importing both at the moment Source first opens would zero them out for users who never visit the Source tab.

Cross-PR coherence with #1532 covered in that review (markdown is the canonical wire format both sides agree on; surgical writers in #1532 are the right primitive for focus-mode edits and avoid the lossy re-serialize trap).

— Rames D Jusso

[vanceingalls]

Minor blockers — hold for fix. Reviewed at feb9ceb1, re-verified at dbfaf4f6 after the stack force-pushed mid-review (the only delta is a ReDoS-protection rebase of parseStoryboard.ts from #1528 rippling through the stack — net-new #1531 content unchanged, all findings below still apply). Posted in parallel with Miga's approval-with-suggestions and Rames's comment-with-concerns above — both landed before I went to post. I agree with effectively all of their substantive findings; this review's net new contribution is (a) elevating the unsaved-edits concern from "iterative-OK" / "worth addressing" to a blocker under the band-aid bar Miguel set on this surface, and (b) one finding neither of them surfaced: an unrelated fixture mutation that is currently the CI red.

Singular puzzle, several telltale fibres — the central architecture is impeccable, but two threads run loose enough that they snag before the merge.

Where I disagree on severity (escalation of Miga/Rames's first concern)

1. Silent dirty-state loss on Board↔Source toggle and tab switch. (StoryboardSourceEditor.tsx:115-118, StoryboardLoaded.tsx:30-46)

Rames noted this on the Board↔Source toggle; Miga noted it on tab switch and navigation. Both surfaces hit the same root cause: useEditableFile(activePath, onSaved) is keyed on activePath and lives below the toggle in the component tree, so:

• Flipping Board | Source un-mounts StoryboardSourceEditor entirely and discards the in-memory content state — dirty buffer vapourised, no prompt, no warning.
• Switching the file-tab from STORYBOARD.md to SCRIPT.md re-fires the hook's useEffect, refetches the new file, and overwrites content + saved. The header continues to display "Saved" for the new tab as if the previous edit never existed.
• A stray browser-tab close hits the same surface — no beforeunload guard while dirty.

Under Miguel's band-aid bar this maps to pattern #3 (scope gap that undercuts the PR's stated coverage). The PR description promises "on save the Board re-parses, so markdown stays the single source of truth" — that's only true if the user can actually get to save. A silent buffer-loss on a routine interaction is the user-facing-half of "single source of truth" with the lid hanging open, and trust-corrosive to walk back later once it's shipped behind the ENV flag.

Acceptable resolutions (any one):

• Hoist useEditableFile state up out of StoryboardSourceEditor so the buffer survives Board↔Source toggles and tab switches; key the dirty map by path. (Long-term fix.)
• Confirm-dialog on toggle and tab switch when dirty, plus a beforeunload listener while any file is dirty. (Minimum acceptable.)

Either is cheap to do in this PR. Both other reviewers' framings are correct for their respective lanes; the elevation belongs in mine because the band-aid bar is the framing I own on this surface.

One finding neither of the parallel reviews surfaced

2. Accidentally-committed fixture rewrite is the CI red. (packages/studio/fixtures/storyboard-sample/index.html)

The single failing required check (Preflight (lint + format) on preview-regression) names exactly one file:

packages/studio/fixtures/storyboard-sample/index.html (10ms)
Format issues found in above 1 files. Run without `--check` to fix.

The diff against #1530's tip shows the fixture going from the pretty-printed form authored in #1530 to a single-line collapsed form with auto-injected data-hf-id="hf-..." attributes on every <div>. That fingerprint matches the studio's own HTML writeback — almost certainly the artefact of loading the sample fixture inside the new editor during local testing and accidentally staging the result. It has zero functional connection to the markdown editor work; it's also the only thing currently keeping the stack from going green.

Under the band-aid bar this is pattern #6 (small footgun that ages into a band-aid): the fixture was deliberately authored in #1530 as the canonical demo asset for the storyboarding flow, and the rewrite both regresses readability for future devs reading the fixture and leaves the door open for the same accidental-stage to ship from any future studio-editor PR. Worth a git checkout origin/storyboard-03-frame-grid -- packages/studio/fixtures/storyboard-sample/index.html and a force-push to restore.

Where I agree with Miga and Rames (mapped to band-aid bar for the record)

• marked.parse() async return type. Both Miga and Rames flagged this. Agree — marked.parse(debounced, { async: false }) is the right shape, drop the typeof raw === "string" narrowing. Pattern #4-adjacent (defensive code that silently swallows on a hypothetical-but-real future path); non-blocking today but trivially correct to do here.
• useMemo ref-equality on data.script (StoryboardLoaded.tsx:21-25). Miga's catch. Agree — depend on data.script?.path and data.script?.exists, not the parent object reference. Every reload() produces a new data.script object, so the current memo is decorative and re-creates the sourceFiles array on every save, which ripples through React's tab-list keys.
• Concurrent save coalescing / read-modify-write with #1532. Rames's catch. The useFileManager.writeProjectFile last-write-wins surface is pre-existing — agreed it's not a #1531 regression but worth flagging on the broader storyboard-editing thread. For this PR specifically, the cheap fix is the if (saving) return guard on the inner save() so a fast Cmd+S double-tap doesn't fire two PUTs.
• Preview prose styles skip <img>. Rames's nit. Agree — [&_img]:max-w-full [&_img]:h-auto keeps a pasted image from blowing past the right pane.
• Debounce initial paint. Rames's nit. Agree — seeding debounced with source on first render avoids the 200ms blank-preview window on large files.
• External SCRIPT.md creation doesn't refresh the tab list. Rames's nit. Agree, non-blocking.

Notes (non-blocking)

• DOMPurify default config blocks <script> / on* / javascript: URLs — fits the threat model (untrusted-but-same-origin user manifest content). Worth noting that default DOMPurify does not inject rel="noopener noreferrer" on links; if a manifest's <a href="https://…"> ever opens in the studio context, the window.opener reverse-tab-nabbing surface is open. Cheap follow-up.
• SourceEditor re-mounts cleanly on filePath change (the mountEditor callback ref destroys + re-creates the EditorView), and the loading branch un-mounts during the load window so there's no flash of stale doc. Clean.
• Stack base is current main (graphite-base/1528 = 1028f52a is reachable from origin/main); single-commit-per-PR; no stale-base squash hazard.

---

Verdict at dbfaf4f6: finding #1 is request-changes under the band-aid bar (escalation of both other reviewers' first concern to a blocker), finding #2 is request-changes because it's literally the CI red. Everything else from the agree-list is clean-up that can ride the same push.

Re-verify if HEAD moves before stamp.

Review by Via

[miga-heygen]

Re-review after rebase onto the updated #1530.

StoryboardSourceEditor — This is the star of the PR. The split-pane raw markdown editor + live preview is well-architected:

• useEditableFile properly tracks dirty state via content !== saved, with clean cancelled-effect cleanup on unmount/path change. The readProjectFile / writeProjectFile calls through the shared file manager context keep the file I/O consistent with the rest of the studio.
• useMarkdownPreview debounces at 200ms (good for keystroke-heavy editing) and sanitizes via DOMPurify before dangerouslySetInnerHTML — the right call for user-authored markdown.
• Cmd/Ctrl+S shortcut wired up at the container level with onKeyDown + preventDefault. Elegant.
• The file tab selector reconciles against the current file list (activePath fallback) so a removed/renamed file can't strand the tab. Nice defensive touch.

SourceEditor changes — The switch → Record<string, () => Extension> refactor for language extensions is cleaner and naturally extensible. Adding markdown + md detection is correct. The fallow-ignore-next-line complexity on mountEditor is fine — it's a single imperative callback, not cyclomatic complexity that needs splitting.

StoryboardLoaded — Clean Board ↔ Source sub-view toggle. The SubViewToggle component with role="tablist" / role="tab" / aria-selected is proper a11y for this pattern.

StoryboardView — Hoisting StoryboardLoaded out of StoryboardView is the right separation: view handles load states, loaded handles the experience.

Dependencies — marked, dompurify, @codemirror/lang-markdown are well-chosen for the job. No bloat.

CI green. No concerns. LGTM — ready for stamp.

— Miga

[miguel-heygen]

Reviewed — storyboard markdown source editor. Rebased, CI green. LGTM.

[vanceingalls]

Addendum to my 06-17 07:08Z review. Re-verified at HEAD 02d4c2c9, post-rebase onto the now-merged #1529 and the updated #1530 base. Both Miga (21:05Z "LGTM — ready for stamp") and Miguel (21:06Z one-liner approval, "Rebased, CI green. LGTM") have lifted on the rebase. Flagging where I still see the band-aid bar tripped — surfacing for the record so a stamper can decide on the evidence rather than the vote count.

Verdict, second pass: minor blockers — same set as yesterday, none cured by the rebase. With two approvals already on file (Miga's substantive re-review and Miguel's rebase-stamp), I am no longer in a position to call this a hold-for-fix in isolation; my dissent here is on severity, not correctness. Read these as flags for the merge-deck to weigh rather than as a unilateral block.

What yesterday's blockers map to at 02d4c2c9

1. Silent dirty-state loss on Board↔Source toggle and tab switch — STILL OPEN.

• The unmount path. StoryboardLoaded.tsx:38-58 — the subView === "board" ? … : <StoryboardSourceEditor … /> ternary survives the rebase intact. Flipping Board | Source un-mounts StoryboardSourceEditor entirely; useEditableFile's content state goes with it; no beforeunload, no confirm, no warning. Yesterday's quote on the PR description — "on save the Board re-parses, so markdown stays the single source of truth" — still has a silent escape hatch where the user's most recent typing simply disappears.
• The tab-switch path. StoryboardSourceEditor.tsx:28-58 — useEffect is still keyed on path alone. Switching STORYBOARD.md → SCRIPT.md re-fires, readProjectFile(path) resolves, and the bare setContent(text); setSaved(text) overwrites both buffers. No if (dirty) confirm(…) guard at line 41; no preserved-per-path map. The status pill reads "Saved" on the new tab as if the previous edit had never existed.
• The browser-close path. No beforeunload handler anywhere in the editor; closing the tab while dirty is silent.

Under Miguel's band-aid bar this still maps to pattern #3 (silent scope gap that undercuts the PR's stated coverage), and that's why I held the threshold at "minor blocker" yesterday rather than "iterative-OK." Rames and Miga both surfaced one face of it on first pass; the fix has not landed.

2. Documentation/code drift in ViewModeContext — STILL OPEN. ViewModeContext.tsx:53-60 still claims the popstate listener will "reflect back/forward navigation and agent-driven URL changes," while the writer at :31-39 uses window.history.replaceState, and the MDN spec is unambiguous: popstate does not fire on replaceState (or pushState). The "agent-driven URL changes" half of that comment is a promise the code does not keep. Either the writer needs to also dispatch a synthetic popstate (or a custom event consumed by the listener), or the comment needs to drop the "agent-driven" claim and document the actual constraint (agents must call setViewMode, not push to ?view= directly). Both fixes are small; the drift is the band-aid pattern #1 surface (doc says X, code does Y).

3. ReDoS rebase note. The earlier dbfaf4f6 ReDoS-protection rebase from #1528 is still in the tree, now upstreamed via the #1528 merge — so the parser-side guards are now in main. Net new #1531 content has not regressed those guards.

Where I diverge from the two LGTMs

Miga's re-review reads the editor architecture cleanly and credits the right wins (useEditableFile shape, debounced sanitization, file-tab reconciliation, ARIA tablist semantics). I agree on each of those — the architecture is impeccable in the abstract. Miguel's stamp is a single-line "Rebased, CI green. LGTM," consistent with a rebase-pass rather than a fresh deep dive on the dirty-state findings I named yesterday — so I read his approval as endorsing the rebase, not as a deliberate severity ruling against those findings. The disagreement, where it exists, is only on whether silent loss of unsaved markdown — on a UI gesture as routine as Board↔Source toggling — clears the band-aid bar. A user types, toggles to peek at the Board, comes back, finds an empty buffer. That is exactly the trust-corrosive footprint pattern #3 was named for; my severity stays at "minor blocker," but I defer to the merge-deck on the call.

Net new finding since 06-16

None. The rebase delta is functionally a no-op over the editor surfaces (the only dbfaf4f6 → 02d4c2c9 differences in scope are the merged-base picks).

Bottom line

Minor blockers in my reading; two approvals on file say otherwise. Net new lines required to clear my findings: ~15 (a beforeunload + a confirm on toggle + a 4-line doc rewrite of the popstate comment). I'll defer to whoever drives the merge train on whether they want those before stamp or as a follow-up issue.

Review by Via

[jrusso1020]

@vanceingalls addressed your blocker at bb5beb92:

1. Silent dirty-state loss on Board↔Source toggle and tab switch — fixed with the "minimum acceptable" resolution you outlined:

• StoryboardSourceEditor now surfaces dirty state to the parent via onDirtyChange. StoryboardLoaded guards the Board ↔ Source toggle with a window.confirm("Discard unsaved markdown changes?") when leaving Source while dirty.
• File-tab switch (STORYBOARD.md ↔ SCRIPT.md) now routes through a guarded selectFile that confirms before discarding an unsaved buffer.
• Added a beforeunload listener while the active file is dirty.

The remaining items from your agree-list (marked { async: false }, useMemo dep on data.script?.path/.exists, [&_img] prose, save() in-flight guard, debounce seed) plus the ViewModeContext popstate/replaceState doc-drift (#3 in your addendum — comment accuracy on already-merged #1529 code) are batched into a single follow-up PR after the stack lands, per the agreed blockers-now / nits-in-follow-up split. The fixture CI red you flagged was restored from the #1530 base earlier in the stack.

The base branch was changed.

[miguel-heygen]

Re-approved after rebase. LGTM.