fix(studio): shell UX — data-loss guards, error surfacing, dialog contracts, toasts

[vanceingalls]

Summary

Studio shell fixes from the UX review — including the single worst finding: a failed composition fetch rendered an empty editor whose autosave could overwrite the real file with empty content.

Changes

Data-loss guards (critical):

• Composition fetch failure is no longer swallowed (.catch(() => {})): persistent error toast, editor shows a loading placeholder, and autosave cannot fire while content is unloaded.
• Source-editor autosave failures surface a persistent error toast ("edits are NOT persisted", deduped) — was telemetry-only while the user kept typing.
• File create/delete/rename/duplicate failures now toast — were console-only.

Toasts (new system):

• Stacked (max 3) instead of single-slot overwrite; error-tone toasts persist until dismissed (was: errors auto-vanished in 4s); role="alert"/role="status"; real enter/exit animations with motion-reduce guards.

Modals get the dialog contract (via useDialogBehavior):

• AskAgentModal: focus trap, Escape, dirty-draft guard — a stray backdrop click no longer discards typed instructions.
• LintModal: parameterized title/prompt so runtime console errors stop masquerading as "HyperFrame Lint Results"; the copied agent prompt now includes the real project dir (was window.location.href labeled as "Project path"); Escape + labeled close.
• One overlay at a time (console-errors modal waits behind the lint modal — they used to stack unreachably).

Header / toolbar:

• Capture: spinner + re-entrant click guard (was: silent 35s operation, parallel captures on double-click). Export: disabled while rendering with explain-why tooltip (was: double-fire enqueued two jobs). Native title tooltips → ui Tooltip. ViewModeToggle completes the APG tabs pattern.
• TimelineToolbar: aria-label/aria-pressed on tool buttons; Split/Add-beat render disabled instead of unmounting (stable slots — no more shifting targets mid-task); disabled tooltips explain why ("Move the playhead inside the clip to split").
• "Dismiss" on the save-queue banner relabeled "Retry saving" — it always reset the circuit breaker; the label lied.

Panels / misc:

• All three shell splitters: role="separator" + arrow-key resize + onPointerCancel. Right-panel tab/content mismatch fixed. MediaPreview gets designed error states. ErrorBoundary gains "Reload Studio". Splash resolving state labeled. FeedbackBar animates in without the 32px layout jolt. Full-screen backdrops fade in (150ms ease-out).

Verification

• TimelineToolbar tests 2/2; oxlint/oxfmt clean; tsc clean at stack top

Stack

PR 3/7 of the studio UX-review fixes (needs the ui-primitives and render-cancel PRs below it).

🤖 Generated with Claude Code

[vanceingalls]

• fix(studio): captions UX — mode exit, undo, autosave surfacing, honest gating #1968
• fix(studio): player UX — keyboard timeline, honest waveform, keyframe menu actions #1967
• fix(studio): sidebar/panels UX — asset delete confirm, rename, search trap, undo #1966
• fix(studio): editor panel UX — commit safety, keyboard a11y, wired BlockParamsPanel #1965
• fix(studio): shell UX — data-loss guards, error surfacing, dialog contracts, toasts #1964 👈 (View in Graphite)
• feat(studio,studio-server,cli): render cancel end-to-end + renders/nle/storyboard UX #1963
• feat(studio): harden ui primitives — focus rings, keyboard tooltips, dialog hook #1962
• main

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

[miga-heygen]

TITLE: fix(studio): shell UX — data-loss guards, error surfacing, dialog contracts, toasts

VERDICT: Approve (with two non-blocking items)

SUMMARY

This is a very solid PR. The biggest win is eliminating the data-loss vector where a failed composition fetch rendered an empty editor whose autosave could silently overwrite real content — that alone justifies the whole changeset. The remaining work is thorough and well-considered: the toast system graduates from fragile single-slot to a proper stack, dialogs get real focus traps and dirty-draft guards, toolbar buttons get re-entrance protection and explain-why disabled states, and splitters become keyboard-accessible. None of the findings below are blockers.

PONYTAIL

Structural integrity: The useCompositionContentLoader extraction is clean — it correctly keeps content: null until the fetch resolves, and the corresponding guard in StudioLeftSidebar prevents mounting the editor on null content. The toast system's lifecycle (enter → auto-dismiss / persist → leaving animation → remove) is well-orchestrated with proper timer cleanup on unmount. The dialog contract via useDialogBehavior centralizes Escape/backdrop handling correctly, and the AskAgentModal's canClose: () => !value.trim() is the right predicate. The one-modal-at-a-time guard (lintModal === null && consoleErrors !== null) is a simple, correct solution to the stacking problem. ViewModeToggle's APG tabs implementation (roving tabIndex + arrow-key nav) is textbook. The SaveQueuePausedBanner label fix ("Dismiss" → "Retry saving") is a small but meaningful honesty improvement — the action always reset the circuit breaker, so the old label lied.

FINDINGS

[non-blocking] useToast: module-level counter won't reset across HMR / test isolation

let nextToastId = 1 at module scope means IDs monotonically increase across the app lifetime, which is fine in production. But in tests or HMR cycles, the counter persists across re-mounts (module state survives HMR in Vite). This is unlikely to cause actual bugs since IDs only need to be unique within a single toasts array, but it's worth noting: if tests ever assert on specific toast IDs, they'll be flaky. A trivial fix would be to derive IDs from a ref inside the hook, but this is genuinely non-blocking since nothing currently depends on ID values.

packages/studio/src/hooks/useToast.ts

---

[non-blocking, style] handleFileSelect still swallows non-OK responses before the JSON parse

In useFileManager.ts, the updated handleFileSelect now surfaces errors via toast (good!), but it still calls r.json() without checking r.ok first. If the server returns a 404 or 500 with a non-JSON body, the .json() call throws a parse error, and the toast shows a generic "SyntaxError" instead of "404 Not Found." The new useCompositionContentLoader does this correctly (if (!r.ok) throw new Error(...) before .json()), so this is a consistency gap — not a regression, since the old code swallowed errors entirely.

packages/studio/src/hooks/useFileManager.ts, around the handleFileSelect callback.

---

[observation] TimelineToolbar: inline IIFE store reads

The Split and Add-beat sections use (() => { const { ... } = usePlayerStore.getState(); ... })() inline in JSX. This is a pre-existing pattern (not introduced by this PR), and it works because the toolbar itself re-renders on store changes. Flagging it only as context for future refactors — it's not a problem this PR needs to solve.

---

[positive] AskAgentModal dirty-draft guard

The canClose: () => !value.trim() check in useDialogBehavior is exactly right — a backdrop click or stray Escape while the user has typed instructions was the single most rage-inducing UX bug in the agent modal. The X button still bypasses the guard (calls onClose directly), which is correct: an explicit close action should always work.

---

[positive] Capture re-entrance guard

The capturingRef + capturing state pair in useFrameCapture correctly prevents parallel captures on double-click. Using a ref for the guard (checked synchronously) and state for the UI (spinner + disabled styling) is the right separation. The finally block ensures the guard clears even on error.

---

[positive] Export button disabled during render

disabled={isRendering} on the Export button with the explain-why tooltip ("A render is already in progress") is a good guard against the old double-fire bug. The extra if (isRendering) return; inside onClick is belt-and-suspenders — disabled buttons don't fire onClick in practice, but it's harmless insurance.

DIFF_STATS

+693 / -276 across 23 files. Key new file: useCompositionContentLoader.ts (40 lines). Heaviest churn: StudioHeader.tsx (+184/-106), useToast.ts (+66/-13), TimelineToolbar.tsx (+63/-29).

— Review by Miga

[miguel-heygen]

Stack review for PR 3/7.

Audited: packages/studio/src/hooks/useCompositionContentLoader.ts, StudioLeftSidebar.tsx, AskAgentModal.tsx, StudioRightPanel.tsx, useToast.ts, and the related file-manager/error-surfacing changes at head d89e8e4.

The key data-loss fix is correct: source editing no longer mounts with content: "" while a composition fetch is unresolved, so a fetch failure cannot arm autosave over the real file with empty content. This PR also completes the render-queue wiring introduced in #1963 by passing cancelRender, loadError, retry, and action-error props into RenderQueue.

The dialog behavior adoption is appropriate for Ask Agent: Escape/backdrop can be vetoed for dirty drafts while explicit close remains available. Toast persistence for errors and cleanup of timers are also shaped correctly. Miga’s handleFileSelect note is valid polish: non-OK non-JSON responses can still surface a generic parse error, but that is an improvement path rather than a blocker.

Verdict: APPROVE
Reasoning: This PR closes the source-editor empty-autosave failure mode and finishes the render-cancel UI wiring without introducing a blocking regression in the audited paths.

— Magi

[vanceingalls]

Addressed at head 1823e477b: handleFileSelect now checks r.ok before .json() and surfaces the status code ("Failed to load x (404)") instead of a JSON parse error — consistent with useCompositionContentLoader.

🤖 Generated with Claude Code

The base branch was changed.

[james-russo-rames-d-jusso]

Reviewed at db3f931 (merged 2026-07-06 23:56 UTC — post-merge second-pass, findings apply as follow-up cleanup).

Layering on Miga's Approve-with-two-nits and Magi's APPROVE. Both are right on the load-bearing work: the useCompositionContentLoader extraction closes the autosave-over-empty data-loss vector, the toast stack + persistent-error semantics are correctly shaped, and the dialog contract + focus-trap adoption is clean. Two second-pass findings worth a follow-up ticket.

---

🟠 openSourceForSelection still uses the pre-PR anti-pattern the rest of this file abandoned

packages/studio/src/hooks/useFileManager.ts:181-193 — click-to-source does .then((r) => r.json()) with no !r.ok check, then .catch(() => {}) swallows every failure silently. Compare against:

• useCompositionContentLoader.ts:26-27 (this PR): if (!r.ok) throw new Error(...) → toast on catch. ✅
• useFileManager.ts:149-161 (updated in this PR): same new pattern → toast on catch. ✅
• useFileManager.ts:181-193 (NOT touched in this PR): still swallows silently. ❌

Miga flagged this pattern for handleFileSelect at the review-time SHA and it landed fixed there; openSourceForSelection was missed. Same failure surface (click-to-source on a broken file → user sees nothing happen; no toast, no console, no telemetry). Trivial follow-up: mirror the new .then + throw + catch → showToast shape.

---

🟠 New user-visible failure paths are toast-only — no observability for ops

The PR's thesis is "silent failures become visible" — but the visibility is user-only. useToast.ts (showToast) fires no telemetry:

• Composition load failure → toast, no metric.
• Autosave-persist failure → persistent toast, no metric.
• Capture failure → toast, no metric.
• File create/delete/rename/duplicate failures → toast, no metric.

Studio already has packages/studio/src/telemetry/events.ts (used by useRazorSplit, useGsapAwareEditing, useElementLifecycleOps, useDomEditCommits). Adding a trackStudioErrorToast({ context, message }) inside showToast (or at each surfacing call site with a stable context tag) would let ops dashboard the actual rate at which each of these new failure surfaces fires. Without it, "we made failures visible" holds for users but not for the team; the first time an oncall wants to know how often autosave is failing across the whole user base, they'll have to add the instrumentation retroactively.

Per my feedback_observability_pr_failure_path_coverage rubric: observability PRs (or observability-adjacent PRs like this one) need failure-path telemetry, not just success-path signals.

---

🟡 Nits already covered by Miga

• Module-scoped let nextToastId = 1 at useToast.ts:15 — HMR/test-isolation edge case, correctly non-blocking.
• Inline IIFE store reads in TimelineToolbar — pre-existing pattern, fine.

---

🟢 Confirmed good

• useCompositionContentLoader.ts has exactly one consumer (App.tsx:394) and keeps content: null until fetch resolves — I greped every editingFile.content reference (26 hits) and every callee correctly guards on != null (StudioLeftSidebar.tsx:125, useFileManager.ts:174, SlideshowPanel.tsx:174). No sibling caller depends on the old content: {} semantics. Data-loss vector really is closed.
• useToast.ts capping at MAX_TOASTS with error-tone persistence + role="alert"/status is correct — errors won't auto-vanish, and the a11y announce is layered right.
• capturingRef + capturing state (ref for guard, state for UI) in the capture flow is the correct separation Miga called out.

---

Verdict: 🟢 the merge is defensible; two 🟠 follow-ups (openSourceForSelection sibling, telemetry on error surfacings) worth a ticket. Not gating anything; leaving as a comment for the follow-up trail.

— Review by Rames D Jusso