Skip to content

feat(runner): DEV-2159 preview pane chrome + boot / refresh / error states (T5) - #97

Merged
demtario merged 5 commits into
feat/DEV-2027-redesignfrom
feat/DEV-2159-preview-status
Jul 31, 2026
Merged

feat(runner): DEV-2159 preview pane chrome + boot / refresh / error states (T5)#97
demtario merged 5 commits into
feat/DEV-2027-redesignfrom
feat/DEV-2159-preview-status

Conversation

@demtario

@demtario demtario commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

T5 of the DEV-2027 redesign. DEV-2159 · plan: runner/docs/dev-2027-redesign-plan.md · ADR-0023.

Frames: 48:6560 (ready), 72:26445 (refresh in flight), 72:14610 (loading), 48:6701 / 72:15699 (the bar itself).

Why

T2 deleted PreviewPane's full-width accent strip on the grounds that ● ready belonged in T5's bottom bar — so until now a running preview reported nothing at all, and s.statusBar() sat in styles.ts styling nothing. T2 also added DemoRuntime.reload() and the row-2 refresh button, but pressing it produced no feedback whatsoever. This closes both.

The bar is a sibling of PreviewPane, not a child

The load-bearing decision. Every overlay in that section is position: absolute; inset: 0, so a bar rendered inside it is painted over by the boot, error and refresh overlays — which is exactly why the pre-T2 top strip had to buy its way out with inset: "28px 0 0 0" on every overlay. As the preview column's last child it also lands in the editor's band with no arithmetic: measured flush at top: 734, height: 28, with T6's 1px splitter track between them. 48:6701 and 48:6740 are both y=800 h=28 inside their own column frames, which is that band.

Both bars share s.paneStatusBar — sharing the object is what stops them drifting apart. It sits on editorBg (#ffffff / #19191c, exact). previewBg is wrong here: #070604 in dark, a step darker, where the frames paint this band lighter than the preview surround. Extends open item 7.

data-preview-status and aria-label="Preview" stay on the <section> and the iframe stays a descendant — e2e/starter-matrix.spec.ts:144 polls the first, e2e/docs-examples.spec.ts:305 selects on the second. Both re-verified in the DOM after the restructure. Please don't tidy them away.

reload() now returns Promise<void>

types.ts, container.ts, sandpack.ts. It is what the refresh spinner waits on, and it never rejects — failure already has onError.

The two tiers settle on genuinely different things, and the difference is not cosmetic:

  • Container resolves on the reloaded page's load. src navigation is what a refresh is, and load fires on the frame element regardless of origin. 10s timeout and dispose() as backstops.
  • Sandpack resolves on its own transpile + dispatchnot on a done, because the bundler sends nothing back for an updateSandbox(setup, true).

Correction. An earlier revision of this description claimed Sandpack resolved on the bundler's next done, "measured at ~3.1s off the real done". Both halves were wrong. The number came from sampling across two tool round-trips, so it timed my own latency; and the mechanism doesn't exist. Instrumenting the page shows no messages arrive at all in the 11s after a refresh click, while a done on mount arrives normally and drives emitReady() — so the listener is fine and this specific call goes unanswered. The promise therefore never resolved early and always rode its timeout out: the pane sat blanked under a spinner for a full 10s on every Tier-1 refresh. Now 48/50/62ms across three consecutive runs, no timeouts.

Found by following up a Bugbot report about settler ordering. Worth noting that the two "obvious" fixes both measured worse: arming the listener after dispatch loses the race against its own done (10.07s), and registering early behind a dispatched flag is ordering-correct but still timed out (10.02s) — which is what finally pointed at there being no message at all.

The boot log stays, gated on the engine

The design shows only a spinner and "Loading data …" and omits the live install/dev-server log. That log is the only signal when a container is slow or stuck, so it is a gap in the design, not a removal.

My first cut inferred the tier from bootLog being non-empty. That read correctly until a wedged container pool made POST /api/session hang for 100s+ — and the log only starts arriving after that request returns, so the window where the user most needs to be told the wait is expected is the window with no log to infer it from. containerBoot is now an explicit prop. Tier 1 gets the designed spinner alone; Tier 2 adds the wait explanation, the newest log line (Preparing container… until the first arrives), and the tail behind a Details disclosure.

Details is a button plus a chevron rather than <details>/<summary>: hiding the native marker needs list-style: none and ::-webkit-details-marker { display: none }, both pseudo-element rules inline styles cannot express, and editor-shell may not reach the app's global block — the same constraint that moved hot-spin's keyframes into THEME_CSS.

syncing stays the non-blocking corner badge, restyled. It fires on Tier-2 keystroke bursts; blanking the grid per keystroke is worse than a badge in the corner. refreshing suppresses it, since the pane is already blank behind the refresh spinner.

Error state has no frame, so T9's rule applies: a tokenized card with a danger heading and the message in a mono block.

Also here

Spinner extracted from PreviewPane (four callers now) with its keyframes hoisted into THEME_CSS; Splash rebuilt to 72:14610; s.statusBar() deleted. frameworkName is its own prop rather than a repurposed frameworkLabel — for a docs example entry.displayName is the long `"Columns ▸ … · Standard example · React (TS)"` breadcrumb, so the short label resolves through the starter catalog by framework key (.find, not getEntry, which throws).

Verification

pnpm typecheck clean across all four workspace projects.

Browser (runner:verify), both light and dark:

state result
ready, Tier 1 ● ready · React (Vite, TS) · Handsontable 18.0.0; bars flush
booting, Tier 2 pre-log spinner + "Loading data …" + wait explanation + Preparing container…
booting, Tier 2 with log live line updates; Details expands the tail
refresh in flight pane blanks to previewBg, bar still ● ready, clears in 48–62ms (3 runs)
syncing accent badge 12px inside the pane; grid iframe never removed
error tokenized card, bar ● error

Full suite: 25 passed / 80 skipped with E2E_LIVE=1, against this branch's build on a dedicated port. Re-run after rebasing onto T7 (#96), whose docs-examples.spec.ts rewrite this branch now sits on top of. The 80 skips are starter-matrix.spec.ts, gated on E2E_STARTER_MATRIX=1. Open item 17 notes a default run skips every live: spec, so E2E_LIVE=1 is the only setting that proves anything about preview mount/teardown.

Correction. An earlier revision of this description reported 21/22 and called the one failure pre-existing. That was wrong, and the way it was wrong is worth recording: playwright.config.ts sets reuseExistingServer: !CI on port 4173, and its webServer command passes --port … --strictPort through pnpm --filter … preview --, where pnpm forwards the -- literally and vite ignores both flags. So the run silently attached to a vite preview from another worktree (DEV-2161-cascader) and tested that build instead of this one. My "it fails at base too" check ran the same way, hit the same foreign server, and failed identically — which is exactly why it looked like a pre-existing failure rather than a harness fault. Re-run with E2E_BASE_URL pointed at a dedicated port serving this branch's build: no failures. Nothing was wrong with the cascader.

Not run: starter-matrix.spec.ts (gated on E2E_STARTER_MATRIX=1, iterates every framework × major). Its data-preview-status contract was asserted directly in the DOM instead.

Open items logged, not resolved

Renumbered 26–29 in the rebase onto T7, which had taken 20–25:

26 — Tier 1 has no refresh-completion signal, and may not recompile at all (includes what a follow-up should not re-derive) · 27 — the version renders twice in play mode (no frame shows the collision; 48:6560 is a saved demo with no version menu) · 28 — 72:14610 draws chrome above a splash that renders before the shell exists · 29 — README.md lists runner/apps/viewer/, which does not exist.

🤖 Generated with Claude Code


Note

Medium Risk
Touches preview lifecycle and runtime reload semantics (Sandpack vs container); behavior changes are UX-visible but isolated from auth and persistence.

Overview
T5 (DEV-2159) adds the designed preview bottom bar and rebuilds boot, refresh, and error UI. Readiness moves from the removed top accent strip to PreviewStatusBar ( status, short framework label, Handsontable {version}), placed as a sibling of PreviewPane so full-pane overlays do not cover it.

PreviewPane now uses shared Spinner, tier-aware boot (containerBoot + optional install log behind Details), a previewBg refresh overlay, restyled syncing pill, and a tokenized error card. Boot log parsing strips full ANSI/\r progress lines. s.paneStatusBar is shared with the editor status bar; s.statusBar() is deleted.

DemoRuntime.reload() returns Promise<void> (never rejects). Authoring drives a refreshing spinner with a sequence guard; remounts cancel in-flight refresh. Container reload settles on iframe load (10s timeout / dispose). Sandpack settles on transpile + updateSandbox dispatch because refresh does not emit done.

Authoring Splash matches the loading frame; frameworkName is resolved from the starter catalog for docs examples. Plan docs log open items 26–29.

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

Comment thread runner/packages/editor-shell/src/PreviewPane.tsx Outdated
demtario added a commit that referenced this pull request Jul 31, 2026
Addresses bugbot on PR #97. The pattern was `/\[[0-9;]*m/`, which omits the
ESC byte: it left an orphan `\x1b` wherever it removed a colour code, and it
matched a literal `[31m` in ordinary prose. `container.ts:228` has always used
the correct `\x1b\[…m` form.

The line is unchanged from before this branch, so the PR did not introduce it —
but it is newly load-bearing. `emitProgress` streams the log raw (container's
own strip is on the failure path only), and T5 promotes the newest line from a
collapsed block to an always-visible single row, where the artefacts show.

Widened to full CSI rather than colour alone: pnpm and vite redraw progress with
erase-line and cursor-move codes (`\x1b[2K`, `\x1b[1G`), which a boot log is
full of. Carriage returns get the same treatment — a line rewritten with `\r`
should read as whatever it ended up as, not as every frame concatenated.

Before / after on a realistic pnpm+vite fragment:
  old: "\x1bProgress\x1b: resolved 12\r\x1b[2K\x1b[1GProgress: resolved 340"
  new: "Progress: resolved 340"

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
demtario and others added 3 commits July 31, 2026 10:07
…tates

T5. The preview column gets the bottom status bar the design puts there
(`48:6701`): `● ready` on the left, the project label and Handsontable
version on the right, on the same 28px band as the editor's status bar.
T2 removed the old full-width accent strip on the grounds that `● ready`
belonged here, so until now a running preview reported nothing at all.

The bar renders as a sibling of `PreviewPane`, not inside it. Every
overlay in that section is `inset: 0`, so a bar inside would be painted
over by the boot, error and refresh overlays — which is why the pre-T2
strip had to buy its way out with `inset: "28px 0 0 0"` on each one.
As a sibling it also lands in the same band as `EditorStatusBar` with no
arithmetic, and `s.paneStatusBar` is shared by both so they cannot drift.
`data-preview-status` and `aria-label` stay on the section: the starter
matrix polls the first and the docs suite selects on the second.

Both bars sit on `editorBg` (#ffffff / #19191c, measured). `previewBg` is
a step darker in dark mode — the frames paint this band lighter than the
preview surround, not level with it. See open item 7.

`reload()` now returns a promise that settles when the refresh has
landed, which is what lets the pane blank behind a spinner and recover
(`72:26445`). Container resolves on the reloaded page's `load`; Sandpack
on the bundler's next `done`, settled at the top of that branch so a
compile error settles it too rather than waiting out the timeout. Both
also settle on a 10s timeout and on `dispose()`, so no spinner can
outlive what it was waiting on. It never rejects: failure already has
`onError`.

The Tier-2 boot log stays — it is the only signal when a container is
slow or stuck, and the design omitting it is a gap, not a removal. The
overlay leads with the design's spinner and "Loading data …", then adds
the wait explanation, the newest log line, and the tail behind a
`Details` disclosure. Gated on the engine rather than on the log being
non-empty: the session POST is what stalls when the container pool is
full, so the window where the explanation matters most is exactly the
window with no log to infer it from.

`syncing` stays a non-blocking corner badge. It fires on Tier-2
keystroke bursts, and blanking the grid on every keystroke would be
worse than a badge in the corner.

Error state has no frame, so T9's rule applies: a tokenized card with a
`danger` heading and the message in a mono block.

Also here: `Spinner` extracted from `PreviewPane` (four callers now) with
its keyframes hoisted into `THEME_CSS`, since `editor-shell` cannot reach
the app's global stylesheet; `Splash` rebuilt to `72:14610`; and the
orphaned `s.statusBar()` deleted.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Addresses bugbot on PR #97. The pattern was `/\[[0-9;]*m/`, which omits the
ESC byte: it left an orphan `\x1b` wherever it removed a colour code, and it
matched a literal `[31m` in ordinary prose. `container.ts:228` has always used
the correct `\x1b\[…m` form.

The line is unchanged from before this branch, so the PR did not introduce it —
but it is newly load-bearing. `emitProgress` streams the log raw (container's
own strip is on the failure path only), and T5 promotes the newest line from a
collapsed block to an always-visible single row, where the artefacts show.

Widened to full CSI rather than colour alone: pnpm and vite redraw progress with
erase-line and cursor-move codes (`\x1b[2K`, `\x1b[1G`), which a boot log is
full of. Carriage returns get the same treatment — a line rewritten with `\r`
should read as whatever it ended up as, not as every frame concatenated.

Before / after on a realistic pnpm+vite fragment:
  old: "\x1bProgress\x1b: resolved 12\r\x1b[2K\x1b[1GProgress: resolved 340"
  new: "Progress: resolved 340"

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@demtario
demtario force-pushed the feat/DEV-2159-preview-status branch from 50f1e0b to 4b5664b Compare July 31, 2026 08:12
Comment thread runner/packages/runtime/src/sandpack.ts Outdated
Addresses bugbot on PR #97. `ContainerRuntime.reload()`'s settler deletes
itself from the set; Sandpack's did not, so one that fired from the timeout
stayed registered.

No behaviour change today: `settleReload()` empties the set wholesale and
calling a settler twice resolves an already-resolved promise. What it fixes is
the set growing for as long as reloads keep timing out with no `done` behind
them, and the fact that leaving stale entries in there quietly depended on
`settleReload()` never becoming selective about which waiters it settles.

`settle` also becomes an arrow so it can reach the set — a `function`
declaration's `this` is not the instance — with the timer handle hoisted above
it rather than relying on declaration hoisting.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@cursor cursor 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.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit a6b130d. Configure here.

Comment thread runner/packages/runtime/src/sandpack.ts Outdated
…t on `done`

Addresses bugbot on PR #97, and in chasing it found the bigger problem the
report was pointing at.

Bugbot's case was real: `reload()` armed its settler before `pushUpdate`, which
awaits a transpile before dispatching, so a `done` from a compile already in
flight — a keystroke a moment earlier — could settle a refresh whose files had
not been sent.

Arming after dispatch instead does not work, and measuring it is what exposed
the real issue. The `done` for our own update can land in the same turn the
dispatch promise resolves, so arming late loses the race: the refresh then sat
out the full 10s timeout (measured 10.07s to clear). Gating a
registered-early waiter on a `dispatched` flag fixed the ordering and *still*
timed out at 10.02s — because no `done` ever arrives at all.

Verified in a browser: after a refresh click the parent window sees no messages
whatsoever for 11s, while a `done` on mount arrives normally and drives
emitReady(). So the listener is fine and this specific call goes unanswered,
which means the promise never resolved early and the shell blanked the pane and
held a spinner over it for a full 10 seconds on *every* Tier-1 refresh. My
earlier "cleared at ~3.1s off the real done" was a bad measurement, taken across
two tool round-trips.

So Sandpack now settles on the work we actually do and can time — transpile plus
handing the update to the bundler. Measured 48/50/62ms on a warm React starter,
three consecutive runs, no timeouts. The waiter set, the `settleReload()` hook
and the timeout all go with it; Container keeps its real `load` signal and its
backstops.

Open item 26 rewritten to record the measurement, and to name what is still
unknown: whether `updateSandbox(setup, true)` recompiles at all on an unchanged
file set. If it no-ops then the Tier-1 refresh button does nothing and the
silence is a symptom rather than the bug — which would make T2's
"isInitializationCompile re-runs the sandbox" claim wrong. That predates T5 and
wants its own ticket.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@demtario
demtario merged commit 5b3e8bf into feat/DEV-2027-redesign Jul 31, 2026
2 checks passed
@demtario
demtario deleted the feat/DEV-2159-preview-status branch July 31, 2026 08:36
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.

1 participant