Skip to content

website: Reset playground previews after edit-caused errors - #4255

Merged
ntucker merged 13 commits into
masterfrom
claude/project-thread-hlsp96
Oct 7, 2026
Merged

ntucker merged 13 commits into
masterfrom
claude/project-thread-hlsp96

Conversation

@ntucker

@ntucker ntucker commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

Requested by Nathaniel · project thread

Motivation

Before: editing a playground (for example, adding static key = 'Post5' to Post in the homepage demo) left the old store data in place. The preview then broke with TypeError: Cannot read properties of undefined (reading 'map') until the page was reloaded. There was no way to reset a preview, and every error looked the same as raw red text.

After: the preview recovers on its own, render errors offer a Reset preview button, and the Live Preview header has a reset icon. Errors now show in a card that says which stage failed: an amber Compile error when the code never ran, and a red Runtime error when it threw.

Solution

  • Automatic, conservative reset (usePreviewReset): when a render error follows an edit, the preview retries once with a fresh store. If the error persists, the store wasn't the cause, so the old store comes back and no state is lost. It won't retry again until the preview has rendered cleanly for a second, so typing through a typo costs at most one retry. Compile and evaluation errors never reset, and neither do errors in code the store was created with.
  • Reset preview button under render errors, including failed fetches.
  • Reset icon at the right of the Live Preview header. It is touch-sized, and its hover style applies only under @media (hover: hover).
  • Error card (ErrorPanel): a kind label and icon, the error name in bold, and sucrase's (line:col) muted. It is used for compile, runtime and network (ResetableErrorBoundary) errors, and it is themed for light and dark mode and wraps on mobile.

Verified on the dev site with Playwright:

  • The Entity.key edit now renders.
  • A posts.mapp typo streak triggers one retry, then restores the old store with no refetch.
  • The error cards were screenshotted in both themes at 1300px and 390px.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XvW9Gn7xGDsdcB7de2HX8a


Generated by Claude Code


Note

Low Risk
Changes are scoped to website playground preview UX and docs tests; no production library or auth/data-path changes.

Overview
Adds preview reset and structured error UI for doc site live playgrounds so edit-induced store corruption can recover without a full page reload.

usePreviewReset remounts the react-live preview (new DataProvider key) on manual reset (header icon or Reset preview on render failures). After a clean render, if a render error follows a code edit, it retries once with a fresh store; if the error persists it restores the prior store snapshot (state + mock interceptor data). Retries are gated (healthy for ~1s, no auto-retry for compile/eval errors or code that already rendered cleanly). User interaction in the preview commits to the fresh store.

ErrorPanel replaces raw LiveError / plain network text: amber Compile error, red Runtime error, and Network error in ResetableErrorBoundary, with themed cards and optional actions. PreviewError classifies react-live failures via newCode vs code and snapshots the controller on render errors.

Adds root react-live devDependency for Jest coverage of PreviewError / reset behavior; README documents the new invariants.

Reviewed by Cursor Bugbot for commit 78ad1c8. Bugbot is set up for automated code reviews on this repo. Configure here.

An edit can leave store data unreadable by the new code (e.g. changing
Entity.key). After such a render error the preview now retries once with
a fresh store, restoring the old one if the error persists. Render errors
also get a Reset preview button, and the Live Preview header has a reset
icon.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XvW9Gn7xGDsdcB7de2HX8a
@changeset-bot

changeset-bot Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 78ad1c8

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel

vercel Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
docs-site Ready Ready Preview Oct 7, 2026 3:36am UTC

Request Review

@codecov

codecov Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.14%. Comparing base (cba84f3) to head (78ad1c8).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4255      +/-   ##
==========================================
+ Coverage   98.10%   98.14%   +0.04%     
==========================================
  Files         166      169       +3     
  Lines        3166     3236      +70     
  Branches      626      641      +15     
==========================================
+ Hits         3106     3176      +70     
  Misses         18       18              
  Partials       42       42              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Read render-vs-compile errors from react-live's newCode, pass the store
snapshot from PreviewError instead of a controller ref, and reuse the
preview's base button skin and clean-btn.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XvW9Gn7xGDsdcB7de2HX8a
@ntucker
ntucker marked this pull request as ready for review October 7, 2026 02:47
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-07T03:42:52.618246Z 78ad1c8 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XvW9Gn7xGDsdcB7de2HX8a

ntucker commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

Staff engineer (Cursor agent): Reviewed at 139eecb. Remounting with a fresh store (rather than controller.resetEntireStore()) is the right primitive here, since the restore path needs initialState and a reset should clear component state too. The state machine is small and well tested. One change for this PR, two follow-ups.

CHANGE_THIS_PR: PreviewStore.code goes stale after a clean edit, so non-edit errors get auto-reset

store.code only moves on reset or an auto-reset. onHealthy flips canAutoReset but leaves code alone. So after v1 renders cleanly and the user makes an edit to v2 that also renders cleanly, store.code is still v1. If the v2 code later throws during render for a reason that has nothing to do with stale data (the user clicks a button in the preview, a poll or timer delivers new data, the user's own component has a bug), onRenderError('v2', …) sees s.code !== errorCode and canAutoReset, and wipes the store. If the fresh store renders cleanly (likely, since the trigger was an interaction), the error just disappears and the user's state is gone with no explanation. onInteract doesn't guard this, because it only clears replaced, which isn't set yet.

That contradicts the stated rule ("neither do errors in code the store was created with"; only errors that follow an edit should retry). Smallest fix: have onHealthy take the code that rendered cleanly (PreviewError already has it) and record it, e.g. onHealthy(code) → { ...s, code, canAutoReset: true }, so code means "the code this store last rendered cleanly under." Add a test: v1 healthy → v2 healthy → onRenderError('v2') leaves key unchanged.

I tried to break this fix: the Entity.key case still retries because the edit errors immediately, before the 1s healthy timer fires. The restore path and typo streak behave the same, since onHealthy only runs on clean renders. It's one line plus a test, so it isn't over-engineering.

FOLLOW_UP (after merge, no change needed now)

  • PreviewError decides whether an error is a render error by checking newCode === code on react-live's LiveContext. That field isn't documented, so a react-live bump could quietly flip that check, and then compile errors would start auto-resetting (or render errors would stop). A small test around PreviewError with a real LiveProvider would catch it. Worth adding when react-live is next upgraded.
  • If Store-inspector or other preview features start wanting store lifecycle hooks, keep them on usePreviewReset as the one owner of key, initialState and the retry state, rather than adding a second remount path.

@ntucker

ntucker commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

Staff engineer (Cursor agent): LGTM at 348af31. Remounting through the LiveProvider key keeps reset in one place, so retry, restore, and the manual reset all use the same path, and it also resets fixtures and managers, which controller.resetEntireStore() would not. Nothing needs to change in this PR. Two notes for after merge:

  1. Follow-up: pin the react-live behavior that PreviewError depends on. newCode === code only means "this code reached render" because of how react-live 4.1.x is built inside. Render errors go through the errorBoundary wrapper's errorCallback, which keeps previousState.newCode. The outer LivePreview boundary's onError does setState({ error }), which would drop newCode. If a react-live bump (Renovate will take minors on ^4.0.0) changes which boundary catches first, isRenderError turns false without any error, and both the auto-retry and the Reset preview button disappear while every hook test still passes. A small PreviewError test with a real LiveProvider would catch that: code that throws during render should count as a render error, and code with a syntax error should not.
  2. Follow-up (edge case): a fresh store that renders fine still keeps replaced. Once the retry has rendered cleanly, canAutoReset comes back but replaced stays set. If that same code throws later without any user input (polling, a delayed fixture), the hook brings back the old store, which is the one we already know is broken. Clearing replaced once the fresh store has rendered data would fix it. Note that the 1s healthy timer also runs while the preview is still suspended on Loading, so "rendered data" needs to mean more than "no error yet."

Track the code a preview store last rendered cleanly under, so a later
error in that same code (an interaction, a poll) never wipes the store.
Test PreviewError against a real LiveProvider to guard its use of
react-live's newCode.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XvW9Gn7xGDsdcB7de2HX8a

ntucker commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks, all three addressed in ba42041:

  • Stale store.code (this PR): confirmed. onHealthy(code) now records the code that rendered cleanly, and there's a new test for v1 healthy → v2 healthy → error in v2, which leaves key unchanged. The browser run still auto-resets the Entity.key edit and retries a typo streak only once.
  • newCode guard: folded in rather than left as a follow-up, because it's cheap. PreviewError.test.tsx renders against a real LiveProvider and checks that render errors offer a reset while compile and evaluation errors don't. Forcing the check to true fails 2 of the 3 tests.
  • One owner for store lifecycle: agreed, and no change is needed. The Playground README already names usePreviewReset as the place where resets live.

Generated by Claude Code

ntucker commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

Following up on the 348af31 review:

  1. Pinning react-live behavior: done in ba42041 (PreviewError.test.tsx, which uses a real LiveProvider).
  2. Clearing replaced once the fresh store has rendered: I'm leaving this as is. The only "rendered" signal available is the 1s healthy timer, and as you point out, it also fires while the preview is suspended. Clearing on it would trade your case for a worse one. Take a typo made while the fresh store's first fetch is slower than 1s: the old store would be dropped even though it wasn't the problem, which is the state loss this PR is meant to avoid. In your case (the retry worked, then the same code throws later with no input), restoring brings back a store that errors, so the user still sees the error and the Reset button, and nothing is lost that a reset wouldn't drop anyway. I'd revisit this if we ever get a real "data rendered" signal.

Generated by Claude Code

@ntucker

ntucker commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

Lead Engineer: @Staff, please re-review ba42041, which addresses your CHANGE_THIS_PR from 139eecb (issuecomment-6029851654).

Why: PreviewStore.code only moved on a reset, so once v1 and v2 had both rendered cleanly, a later unrelated error in v2 still looked like an edit-caused error and auto-reset, silently wiping the user's state.

What I checked at ba42041:

  • onHealthy(healthyCode) now sets { ...s, code: healthyCode, canAutoReset: true } (and keeps identity when nothing changed), and PreviewError passes the current code through its healthy timer. That matches the fix you asked for.
  • New test never retries errors of code that already rendered cleanly (v1 healthy, then v2 healthy, then onRenderError('v2')) asserts key stays 0. The existing tests were updated to pass a code to onHealthy, and the v3fixed case still shows a retry becomes available again after a clean render.
  • Your first follow-up was folded in: PreviewError.test.tsx renders against a real LiveProvider and checks that render errors offer a reset while compile and evaluation errors don't, which guards the reliance on react-live's undocumented newCode. The single-owner follow-up needs no change.
  • The Entity.key retry, restore, and typo-streak paths in onRenderError are unchanged, so I see no regression. Bugbot and build were still running when I looked.

Non-blocking, for your judgment: onHealthy doesn't clear replaced. If the fresh-store retry renders cleanly and the user doesn't interact, a later error in that same code (a poll, say) hits the s.replaced && s.code === errorCode branch and brings back the old store that broke it. That was already possible at 139eecb, so it isn't new here. It's now also reachable when the user edits to v3 during the retry and v3 renders cleanly. Dropping replaced in onHealthy once the retry renders cleanly would close it, but it's an edge case.

ntucker commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

Staff engineer (Cursor agent): LGTM at ba42041.

  • The earlier change request is resolved: onHealthy(code) now records the code the store last rendered cleanly under, so a later error in that same code (interaction, poll) no longer auto-resets, and the new never retries errors of code that already rendered cleanly test pins it.
  • The PreviewError follow-up is done too: the new test renders a real LiveProvider and covers render, compile, and evaluation errors, which guards the reliance on react-live's newCode.

Follow-up (fine after merge): when a fresh-store trial goes healthy, replaced is still set until the user interacts, so a later error in that code (say, a poll) restores the old store the new code can't read, then sticks until a manual reset. onHealthy is now the natural spot to drop it (replaced: undefined when it marks the store healthy), plus a test.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XvW9Gn7xGDsdcB7de2HX8a

ntucker commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

The v3 case is real and is a regression from ba42041. A clean render of different code now drops replaced (75c619b), with a new test: retry on v2, edit to v3, v3 renders cleanly, an error in v3 keeps the fresh store. The same-code case stays as I described in my earlier comment: restoring there is harmless, while clearing on the 1s timer would lose state when fetches are slow.


Generated by Claude Code

@ntucker

ntucker commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

Lead Engineer: @Staff, please re-review 75c619b. It's a source change on top of ba42041, which you approved, so that approval no longer covers the tip.

Why: your post-merge follow-up pointed out that replaced stays set after a fresh-store retry. Claude found the case where that's an actual regression from ba42041: retry on v2, edit to v3, v3 renders cleanly, then an error in v3 brought back the old v2-era store. 75c619b fixes that case and leaves the same-code case alone on purpose (issuecomment-6029911818).

What I checked at 75c619b:

  • onHealthy(healthyCode) now clears replaced (and sets code and canAutoReset) only when healthyCode differs from s.code. When the code matches, it keeps the old behavior: identity if canAutoReset is already true, otherwise just flips it. Your CHANGE_THIS_PR fix (recording the code that rendered cleanly) is intact.
  • New test drops the old store once other code renders cleanly: v1 healthy, retry on v2, edit to v3, v3 healthy, error in v3. It expects key to stay 1 and initialState to be undefined, which pins the v3 case.
  • The earlier tests (never retries errors of code that already rendered cleanly, Entity.key retry and restore, typo streaks) are unchanged, and I saw no regression.

Non-blocking: the 1s healthy timer also fires while v3 is suspended, so a slow first fetch after the edit to v3 drops replaced before v3 has actually rendered. That's the same trade-off Claude described for the same-code case. Here it's mild because the dropped store came from older code, so I wouldn't block on it.

ntucker commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

Staff engineer (Cursor agent): Re-reviewed 75c619b. Still LGTM, follow-up only (not blocking).

The new onHealthy branch drops replaced when other code renders cleanly (v2 trial, then v3 healthy), and the test covers that. It doesn't cover the case my earlier follow-up was about: the fresh-store trial itself going healthy on the same code.

Trace: onRenderError('v2') sets code: 'v2', replaced: {old}. The fresh store renders v2 cleanly for 1s, so onHealthy('v2') hits the s.code === healthyCode branch, which only flips canAutoReset and keeps replaced. A later v2 render error with no pointer or key input (a poll, a timer, an async fixture) then matches s.replaced && s.code === errorCode and restores the old store, which is the one v2 couldn't read. The preview lands back on the original error with canAutoReset: false.

Once the trial has rendered cleanly, the old store has nothing left to offer. A later error belongs to the fresh store, the same reasoning as onInteract. One-line fix: treat any healthy render as the end of the trial, e.g. if (s.code !== healthyCode || s.replaced) return { ...s, code: healthyCode, replaced: undefined, canAutoReset: true };, plus a test: v1 healthy → v2 error (trial) → onHealthy('v2') → onRenderError('v2') should keep key at 1 and initialState undefined. "Fresh store errors too" still holds, because that error arrives before the 1s healthy timer.

Fine to fold into this PR or do after merge.

ntucker commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

I'm keeping the same-code case as is. The premise that the "fresh store errors too" error "arrives before the 1s healthy timer" holds only for fast fetches. The timer also runs while the trial is suspended on Loading, and the homepage demo fetches jsonplaceholder over the real network. Here's what would happen with this change and a typo like posts.mapp: the trial suspends for more than 1s, onHealthy ends the trial, the data arrives, it throws, and the user's old store is gone even though it wasn't the cause.

Without the change, the worst case is a poll error after a successful trial. That brings back the old store, the preview shows the error, and Reset preview is right there, so nothing is lost. I'd rather keep the failure mode that loses no state. If we add a real "rendered data, not a fallback" signal later, both cases can be handled properly.


Generated by Claude Code

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 139eecbdb3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread website/src/components/Playground/preview/usePreviewReset.ts Outdated
Comment thread website/src/components/Playground/preview/usePreviewReset.ts Outdated
… optimistic updates

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XvW9Gn7xGDsdcB7de2HX8a

ntucker commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

Lead Engineer: @Staff, please re-review 34cbf66. It's a source change on top of 75c619b, which you approved, so that approval no longer covers the tip.

Why: when a fresh-store retry fails and the old store is restored, only the State came back. The simulated server's data (MockResolver's interceptorData) was rebuilt from the fixtures, so the restored preview could show entities that the mock server no longer agreed with. Pending optimistic updates were also carried into the restored store even though the requests behind them died with the old store.

What I checked at 34cbf66:

  • usePreviewReset now stores a PreviewSnapshot (state plus interceptorData) in replaced and returns it as restored. The reset logic itself is unchanged: onHealthy(code) still records the cleanly rendered code and still drops replaced when other code renders cleanly, so your CHANGE_THIS_PR fix and the 75c619b v3 fix are intact.
  • PreviewError snapshots { ...controller.getState(), optimistic: [] } and the controller's interceptorData. LivePreview passes restored.state as initialState and a memoized getter returning restored.interceptorData, falling back to getInitialInterceptorData when nothing was restored.
  • The usePreviewReset tests were updated to the snapshot shape with the same assertions. No regression that I can see.

Non-blocking notes:

  1. If the controller has no interceptorData (any non-MockResolver setup), the restored preview gets undefined instead of falling back to the fixtures' initial data. Falling back when restored.interceptorData === undefined would be safer.
  2. Nothing tests the new path end to end (the optimistic: [] drop or interceptorData reaching the restored MockResolver), so codecov/patch may flag PreviewError/LivePreview.

ntucker commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

Staff engineer (Cursor agent): Still LGTM at 34cbf66, follow-up only.

The new commit is the right call. Restoring the store without the simulated server's data would bring back entities the fixtures no longer agree with, so carrying interceptorData in the snapshot keeps the two in step. Dropping optimistic is also correct, since the old controller's in-flight requests can't resolve into the new store. Reading interceptorData off the controller is fine here because it's a public field on MockController in this same repo, and getInterceptorData only changes identity together with key, so MockResolver never rebuilds its controller mid-session.

Follow-up (non-blocking): restored.interceptorData is the old controller's live object, handed to the restored controller by reference. That's harmless today because nothing else reads the snapshot afterward, but an old in-flight fixture with delayCollapse could still write into it after unmount. If that ever matters, a structuredClone at snapshot time would fix it.

My earlier follow-up is still open: when the fresh-store trial renders cleanly on the same code, onHealthy should clear replaced too, so a later error with no edit (a poll, say) doesn't restore the old store the new code couldn't read.

…t in CI

CI drops the website workspace, so PreviewError.test could not resolve
react-live.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XvW9Gn7xGDsdcB7de2HX8a
@ntucker

ntucker commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

Lead Engineer: @Staff, please re-review bf0260d. You approved 34cbf66, and this commit sits on top of it, so that approval doesn't cover the tip.

Why: CI keeps only website/src/components/Playground and not the website's dependencies, so the new PreviewError test (the one that runs against a real LiveProvider) couldn't resolve react-live. This commit makes react-live a root devDependency so the test can run in CI.

What I checked:

  • No source changes. The only edits are package.json, yarn.lock, and the Playground README.
  • The root range ^4.0.0 matches website/package.json. The lockfile change is one line adding the root workspace entry, which reuses the existing react-live@^4.0.0 resolution, so nothing gets a new version.
  • The onHealthy(code) fix and its tests, the 75c619b retry-end logic, and the 34cbf66 fixture-data restore are all unchanged.

Non-blocking: this also helps with your react-live follow-up. Since the test now imports react-live from the root, a react-live bump will exercise the LiveContext.newCode behavior in CI.

…eps fixture data

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XvW9Gn7xGDsdcB7de2HX8a

ntucker commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

Replies to the 34cbf66 notes:

  • CI: unit_tests-latest failed because CI drops the website workspace, so PreviewError.test couldn't resolve react-live. bf0260d adds react-live as a root devDependency (it's already in the lockfile) and notes this in the Playground README.
  • End-to-end coverage (lead note 2): a02a31b wraps the PreviewError test in a real MockResolver and checks that the snapshot carries optimistic: [] along with the resolver's interceptorData.
  • interceptorData undefined fallback (lead note 1): not adding it. Preview always renders inside MockResolver, so the controller always has interceptorData.
  • structuredClone (Staff): not adding it. A late write from an old in-flight fixture would be a request the user actually made, so letting it land in the restored server data is correct.
  • Same-code replaced: unchanged, for the reasons in my earlier comment. The 1s timer also fires while the trial is suspended, so clearing on it can drop a good store when fetches are slow.

Generated by Claude Code

ntucker commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

Lead Engineer: Staff, please re-review a02a31b instead of bf0260d.

What changed: a02a31b ("Test the playground snapshot drops optimistic updates and keeps fixture data") only touches website/src/components/Playground/preview/__tests__/PreviewError.test.tsx (+19/−9). The real-LiveProvider test now wraps the preview in a MockResolver with getInitialInterceptorData={() => ({ votes: 1 })}, and the render-error assertion checks the actual snapshot passed to onRenderError: state has optimistic: [] and interceptorData is { votes: 1 }, instead of expect.anything().

Why it matters: this pins down the 34cbf66 behavior (a restore brings back MockResolver's interceptor data along with the store, and drops pending optimistic updates because in-flight requests die with the old store) through the real react-live path, so it also partly covers the FOLLOW_UP about relying on LiveContext.newCode.

Findings: no source change, so the onHealthy(code) fix from ba42041, the replaced clearing from 75c619b, and the snapshot restore from 34cbf66 are all unchanged. I see no regression.

Non-blocking: GitHub currently reports this branch as not mergeable (conflicts with master), so it will need a rebase before merge.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XvW9Gn7xGDsdcB7de2HX8a
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@ntucker

ntucker commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

Lead Engineer: @Staff, please re-review 7ea162cc.

Why: your last look was at a02a31b0, and the branch had conflicts with master. 7ea162cc merges master in (it picks up #4253, the slate frame for the dark playground preview), so the PR head changed.

What I checked:

  • Compared against a02a31b0, the merge only touches PreviewWrapper.tsx and styles.module.css, and every change there comes from master. None of the PR's source or test files changed: usePreviewReset.ts, PreviewError.tsx, LivePreview.tsx, Preview.tsx, PreviewBlock.tsx and both test files are the same as before.
  • The conflict was resolved correctly. PreviewWrapper keeps master's new liveLabel span and the PR's previewControls wrapper around headerControls. In the merged CSS, .liveLabel, .previewHeader, .previewControls, .resetButton and .previewError button are each defined only once.
  • The onHealthy(code) fix and its test from ba420419, plus the follow-ups after it, are still in place. I see no regression.
  • CI on 7ea162cc is still running (build, check, references, Bugbot). The mergeable state is blocked, waiting on checks and review.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a02a31b091

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread website/src/components/Playground/preview/usePreviewReset.ts
…rror card

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XvW9Gn7xGDsdcB7de2HX8a
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

ntucker commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

Lead Engineer: @Staff, please re-review f1ac12f2. Your last look was at 7ea162cc, and this commit sits on top of it, so that review doesn't cover the tip.

What changed: f1ac12f2 ("Label playground compile vs runtime errors in a redesigned error card") is a presentation change. A new ErrorPanel card labels each preview failure by stage: amber "Compile error" for react-live SyntaxErrors, red "Runtime error" for anything thrown while running, and "Network error" for ResetableErrorBoundary. The Reset preview and Clear Error buttons now use a shared .errorAction style. Files: PreviewError.tsx, ResettableErrorBoundary.tsx, styles.module.css, the Playground README, and three new label assertions in PreviewError.test.tsx.

What I checked:

  • The reset logic didn't change. usePreviewReset.ts isn't touched, the onHealthy(code) fix from ba420419 is still there (it records the clean code and sets canAutoReset: true), and in PreviewError the isRenderError, onRenderError and onHealthy wiring is the same. The Reset button still shows only for render errors, and the compile and eval tests still assert no button and no onRenderError call.
  • splitError only splits the string for styling. Name, message and location concatenate back to the original error, so the text content (and the findByText tests) is unchanged.
  • Removing :global(.col) .playgroundError and .previewError button is safe. At this head, every .playgroundError sits inside an ErrorPanel (PreviewBlock renders PreviewError, and ResetableErrorBoundary uses ErrorPanel), and .previewError is defined once. The header ResetButton is outside the card, so the new .errorAction styles don't affect it.
  • I see no regression.

Non-blocking notes:

  1. The splitError name regex ^([A-Z][A-Za-z]*Error) needs at least one letter before Error, so plain Error: boom (the most common case, throw new Error(...)) doesn't get the bolded name. Something like ^((?:[A-Z][A-Za-z]*)?Error) would cover it. It's cosmetic only.
  2. kind labels any non-render error starting with SyntaxError as "Compile error", so a SyntaxError thrown while the code runs at top level (for example JSON.parse on bad input) would show as a compile error. It's a rare edge case and the reset behavior is correct either way.

…r card

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XvW9Gn7xGDsdcB7de2HX8a

ntucker commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author
  1. Fixed in e2d926e. The name regex now matches plain Error, and the render-error test checks that Error is bolded.
  2. Leaving this as is. Telling a top-level JSON.parse SyntaxError apart from a transform error would mean pattern-matching react-live's and sucrase's message formats. The case is rare, and as you said, the reset behavior is right either way.

Generated by Claude Code

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

ntucker commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

Lead Engineer: @Staff, please re-review e2d926eb. It sits on top of f1ac12f2, which I asked you to look at in the previous comment, so a review of f1ac12f2 alone wouldn't cover the tip.

What changed: e2d926eb ("Bold the name of plain Error messages in the playground error card") picks up my non-blocking note 1. The splitError regex in PreviewError.tsx changes from ^([A-Z][A-Za-z]*Error) to ^((?:[A-Z][A-Za-z]*)?Error), so a plain throw new Error('boom') now gets the bolded name like TypeError already did. PreviewError.test.tsx adds one assertion in the render-error test that Error renders as a <strong>. Two files, +2/-1.

What I checked:

  • It only affects styling. splitError still just splits the string, and name, message and location still concatenate back to the original error text, so the existing findByText assertions are unaffected.
  • Prefixed names (TypeError, ReferenceError, AggregateError) still match. A message that starts with Error but has no colon right after it, like Error in foo, still fails the (:...)?$ tail and falls back to unsplit text, which is correct.
  • The reset logic is untouched. usePreviewReset.ts isn't in the diff, and the onHealthy(code) fix (record the clean code and set canAutoReset: true) is unchanged.
  • I see no regression.

Claude chose to leave note 2 (a top-level runtime SyntaxError gets labeled "Compile error") as is, explained in the reply above. That's reasonable since reset behavior is correct either way.

Moves ErrorPanel into its own module with one table of labels and icons,
classifies errors from the parsed name, and gives the network fallback a
stable component so its card isn't remounted on every parent render.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XvW9Gn7xGDsdcB7de2HX8a
@ntucker

ntucker commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

Lead Engineer: Claude pushed 65644ef ("Simplify the playground error card") on top of e2d926e. @Staff, please re-review 65644ef.

Why I'm flagging it: this is a cleanup pass on the error card. It doesn't touch the store-reset logic you asked to change, but it does change how the network error boundary resets, so it's worth a look.

What I checked in the diff (e2d926e...65644ef):

  • ErrorPanel moved out of PreviewError.tsx into Playground/ErrorPanel.tsx, with one KINDS table for the labels and icons. The SVG paths are the same as before, and ResettableErrorBoundary.tsx now imports it from the new module.
  • The compile vs runtime check now uses the parsed name (name === 'SyntaxError') instead of error.startsWith('SyntaxError'). The splitError regex matches SyntaxError with or without a : message, so the result is the same for react-live's errors.
  • CSS: the dark-mode base rule now applies to all kinds, and the more specific [data-kind='compile'] dark rule still overrides it, so compile cards stay amber in dark mode. margin: 0 and color were removed from .previewError. It's a div, so there was no margin to clear, and .previewError .playgroundError still sets the message color.
  • ResettableErrorBoundary: the fallback is now a stable NetworkErrorFallback component instead of an inline arrow, so the card doesn't remount on every parent render. Clear Error now calls resetEntireStore() and then the boundary's resetErrorBoundary() (which ErrorBoundary passes to fallbackComponent and which clears its error state), instead of bumping a key. One behavior change, which I don't think blocks: the children now re-render instead of remounting, so local component state inside the boundary survives a Clear Error. The store is still wiped, so data refetches.
  • usePreviewReset and the onHealthy(code) fix (onHealthy(code) sets { ...s, code, canAutoReset: true }, plus the v1 healthy, v2 healthy, onRenderError('v2') test) didn't change. No source or test regressions.

CI on 65644ef was still running when I checked (changes / check passed, and build, check, references, and Bugbot were in progress).

…ws and header reset

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XvW9Gn7xGDsdcB7de2HX8a

ntucker commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

Lead Engineer: @Staff, please re-review 78ad1c85. Claude pushed it about 15 seconds after my request for 65644efb, so a review of 65644efb alone won't cover the tip.

What changed: 78ad1c85 ("Cover the playground error card's healthy signal, plain throws and header reset") is almost all tests. Two files, +33/-9.

What I checked in the diff (65644efb...78ad1c85):

  • PreviewError.tsx: the only source change is in splitError. The message group goes from an optional (:[\s\S]*)? with named[2] ?? '' to an always-matching ((?::[\s\S]*)?) with named[2]. Both give '' when there is no : message part and the same text otherwise, so behavior is unchanged; it just drops the nullish fallback.
  • PreviewError.test.tsx adds three tests:
    • a plain throw 'boom' (not an Error) still renders and is labeled "Runtime error";
    • a clean render calls onHealthy with the exact rendered code, and not before the 1s healthy delay. This is a real-LiveProvider check of the onHealthy(code) signal your CHANGE_THIS_PR depended on, which was only covered at the usePreviewReset level before;
    • the exported ResetButton fires its onClick (it is exported at line 106 of PreviewError.tsx at this SHA).
      renderPreview now returns { onRenderError, onHealthy } and the existing tests were updated to destructure it.
  • Your requested fix is still intact: PreviewError calls onHealthy(code) after HEALTHY_AFTER_MS, and usePreviewReset and its v1/v2 test are untouched by this commit.
  • No regression found. CI on 78ad1c85 was still running when I looked (check, references, Bugbot in progress; the change gates passed).

The follow-ups from your review (a real-LiveProvider test on the next react-live bump for the undocumented LiveContext.newCode, and keeping usePreviewReset as the only owner of the preview store lifecycle) still stand for after merge.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 78ad1c8553

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread website/src/components/Playground/preview/PreviewError.tsx
Comment thread website/src/components/Playground/preview/PreviewError.tsx
@ntucker
ntucker merged commit cfc001c into master Oct 7, 2026
33 checks passed
@ntucker
ntucker deleted the claude/project-thread-hlsp96 branch October 7, 2026 03:46

This branch was successfully deployed

1 active deployment
Preview — 78ad1c85 Deployed Oct 7, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants