fix(editor): keep graph zoom readable, coalesce undo, and restore last-open - #982
Merged
Merged
Conversation
…t-open A cold graph at Fit (~10%) is unusable, typing a phrase should undo as one step, and a launch with no open= should reopen the last author file instead of an empty buffer. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
… file Cold-launch now opens content/index.md, so the elapsed-time assertion raced past Opening on Linux CI. Hold the mocked open/save long enough to see it. Co-authored-by: Cursor <cursoragent@cursor.com>
|
SummaryCoverage spans editor launch and file persistence, editing and undo behavior, graph navigation, preview rendering, and recovery/error handling across normal and edge-case workflows. Most observed behavior was healthy or inconclusive because the editor was often unavailable, while preview failure handling exposed a user-visible rendering issue. Merge with caution — a PR-attributable medium-severity regression causes a failed preview rebuild to hide the last working preview and leave the interface in an incorrect running state, interrupting inspection without risking saved project data. The remaining observations are primarily environment-blocked rather than confirmed product failures. Tests run by Ito
Tip Reply with @itoqa to send us feedback on this test run. |
![itoqa[bot]](https://avatars.githubusercontent.com/in/2622614?s=60&v=4){kind=small}
There was a problem hiding this comment.
Failed rebuild hides the last valid preview
What failed: The failed rebuild removed the visible preview iframe and left the status at running instead of showing a failed or stale state with the generation-zero frame. The recorded UI message was 'Preview host failed: request failed. Existing output is not current.'
Impact · Steps · Stub / mock · Analysis · Why this is likely a bug
- Severity: Medium
- Impact: When a preview rebuild fails, editors lose the last working preview and cannot inspect it until another rebuild succeeds. This interrupts the preview workflow but does not alter the saved project or its data.
- Steps to Reproduce:
- Open the editor with an existing stale generation-zero preview so the preview frame is visible.
- Click Rebuild preview and make the rebuild request fail.
- Check the Preview pane after the failure.
- Run a successful rebuild and check that generation 1 renders, confirming that the earlier missing frame was not a permanent fixture limitation.
- Stub / mock content: The test used local in-page API mocks for the editor endpoints and a tokened preview URL. The preview mock supplied stale generation 0, a failed rebuild response, and then a successful generation-1 response; no production services, credentials, or application files were used.
- Code Analysis: The direct defect is in editor/ui/src/App.svelte:250-270. rebuildPreview stores previousData and optimistically sets preview.data.phase to running at lines 255-256. If the request succeeds, setPreview replaces that state at lines 260-262. If the request is a watch-daemon refusal, the function restores previousData at lines 263-268, but the general error branch at line 269 only changes preview.status and never restores preview.data or applies the failed/stale response returned by the host. Consequently preview.data remains phase=running. The Preview pane then follows editor/ui/src/components/PreviewPane.svelte:28-31 and 61-70: it hides the new-tab link and iframe unless phase is success or stale, and shows 'No valid Boris preview output is available yet' for running. This matches the observed missing iframe. The host-side implementation in editor/src/preview.zig:62-78 preserves the last dist tree and reports stale when an index exists after a failed build, so the UI is discarding the valid state rather than the build output being unavailable. The smallest fix is to handle every non-watch rebuild error by restoring previousData when it exists, or by converting the returned failed/stale PreviewState into preview.data, while setting the failure message; do not leave the optimistic running state behind. This keeps the generation unchanged on failure and lets the existing PreviewPane rendering logic retain the stale iframe.
- Why this is likely a bug: The behavior contradicts the PR's documented preview contract in content/guides/editor.md:272-277 and editor/README.md:416-421, which says existing output is stale and that the staged output commit preserves the last valid dist tree after a failed rebuild. The test first confirmed a real generation-zero iframe, then observed that the failed rebuild removed it, and a later successful request rendered generation 1. The recovery proves the fixture can render the preview and that the failure is tied to error-state handling. The local API route was intentionally used only to model a failed rebuild response; it did not modify application files or inject UI state beyond the documented failure condition. A targeted UI change to restore the prior preview data or apply the host's stale response is sufficient.
Relevant code
editor/ui/src/App.svelte:250-270
const previousData = preview.data;
if (preview.data) preview.data = { ...preview.data, phase: 'running' };
...
if (result.response.ok) {
setPreview(result.data as PreviewState);
} else if ((result.data as ErrorResponse).error === 'watch_daemon_active') {
if (previousData) preview.data = previousData;
noteWatchRefusal();
} else preview.status = `Preview host failed: ${(result.data as ErrorResponse).error ?? 'request failed'}. Existing output is not current.`;editor/ui/src/components/PreviewPane.svelte:28-31
{#if preview.data && (preview.data.phase === 'success' || preview.data.phase === 'stale')}
<a class="button-link" href={preview.data.preview_url} target="_blank" rel="noreferrer">Open preview in new tab</a>
{/if}editor/ui/src/components/PreviewPane.svelte:61-70
{#if preview.data && (preview.data.phase === 'success' || preview.data.phase === 'stale')}
<div class="preview-frame">
<iframe title="Boris site preview" src={`${preview.data.preview_url}&generation=${preview.data.generation}`} ...></iframe>
</div>
{:else}
<p>No valid Boris preview output is available yet.</p>
{/if}editor/src/preview.zig:69-78
if (self.exit_code == 0) {
...
self.generation += 1;
} else {
self.phase = if (hasIndex(io, self.project_root)) .stale else .failed;
self.used_stderr_fallback = true;
self.setStderrSummary(execution.stderr);
}Evidence Package
Copy prompt for an agent
Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.
**Medium severity — Failed rebuild hides the last valid preview**
**What failed:** The failed rebuild removed the visible preview iframe and left the status at running instead of showing a failed or stale state with the generation-zero frame. The recorded UI message was 'Preview host failed: request failed. Existing output is not current.'
- **Impact:** When a preview rebuild fails, editors lose the last working preview and cannot inspect it until another rebuild succeeds. This interrupts the preview workflow but does not alter the saved project or its data.
- **Steps to reproduce:**
1. Open the editor with an existing stale generation-zero preview so the preview frame is visible.
2. Click Rebuild preview and make the rebuild request fail.
3. Check the Preview pane after the failure.
4. Run a successful rebuild and check that generation 1 renders, confirming that the earlier missing frame was not a permanent fixture limitation.
- **Stub / mock content:** The test used local in-page API mocks for the editor endpoints and a tokened preview URL. The preview mock supplied stale generation 0, a failed rebuild response, and then a successful generation-1 response; no production services, credentials, or application files were used.
- **Code analysis:** The direct defect is in editor/ui/src/App.svelte:250-270. rebuildPreview stores previousData and optimistically sets preview.data.phase to running at lines 255-256. If the request succeeds, setPreview replaces that state at lines 260-262. If the request is a watch-daemon refusal, the function restores previousData at lines 263-268, but the general error branch at line 269 only changes preview.status and never restores preview.data or applies the failed/stale response returned by the host. Consequently preview.data remains phase=running. The Preview pane then follows editor/ui/src/components/PreviewPane.svelte:28-31 and 61-70: it hides the new-tab link and iframe unless phase is success or stale, and shows 'No valid Boris preview output is available yet' for running. This matches the observed missing iframe. The host-side implementation in editor/src/preview.zig:62-78 preserves the last dist tree and reports stale when an index exists after a failed build, so the UI is discarding the valid state rather than the build output being unavailable. The smallest fix is to handle every non-watch rebuild error by restoring previousData when it exists, or by converting the returned failed/stale PreviewState into preview.data, while setting the failure message; do not leave the optimistic running state behind. This keeps the generation unchanged on failure and lets the existing PreviewPane rendering logic retain the stale iframe.
- **Why this is likely a bug:** The behavior contradicts the PR's documented preview contract in content/guides/editor.md:272-277 and editor/README.md:416-421, which says existing output is stale and that the staged output commit preserves the last valid dist tree after a failed rebuild. The test first confirmed a real generation-zero iframe, then observed that the failed rebuild removed it, and a later successful request rendered generation 1. The recovery proves the fixture can render the preview and that the failure is tied to error-state handling. The local API route was intentionally used only to model a failed rebuild response; it did not modify application files or inject UI state beyond the documented failure condition. A targeted UI change to restore the prior preview data or apply the host's stale response is sufficient.
**Relevant code:**
`editor/ui/src/App.svelte:250-270`
~~~svelte
const previousData = preview.data;
if (preview.data) preview.data = { ...preview.data, phase: 'running' };
...
if (result.response.ok) {
setPreview(result.data as PreviewState);
} else if ((result.data as ErrorResponse).error === 'watch_daemon_active') {
if (previousData) preview.data = previousData;
noteWatchRefusal();
} else preview.status = `Preview host failed: ${(result.data as ErrorResponse).error ?? 'request failed'}. Existing output is not current.`;
~~~
`editor/ui/src/components/PreviewPane.svelte:28-31`
~~~svelte
{#if preview.data && (preview.data.phase === 'success' || preview.data.phase === 'stale')}
<a class="button-link" href={preview.data.preview_url} target="_blank" rel="noreferrer">Open preview in new tab</a>
{/if}
~~~
`editor/ui/src/components/PreviewPane.svelte:61-70`
~~~svelte
{#if preview.data && (preview.data.phase === 'success' || preview.data.phase === 'stale')}
<div class="preview-frame">
<iframe title="Boris site preview" src={`${preview.data.preview_url}&generation=${preview.data.generation}`} ...></iframe>
</div>
{:else}
<p>No valid Boris preview output is available yet.</p>
{/if}
~~~
`editor/src/preview.zig:69-78`
~~~zig
if (self.exit_code == 0) {
...
self.generation += 1;
} else {
self.phase = if (hasIndex(io, self.project_root)) .stale else .failed;
self.used_stderr_fallback = true;
self.setStderrSummary(execution.stderr);
}
~~~
Owner
Author
There was a problem hiding this comment.
@itoqa Thank you — we checked this against the code and the observation is right.
rebuildPreview sets phase to running, then on a non-ok host response other than watch_daemon_active it only updates preview.status. PreviewPane hides the iframe unless phase is success or stale, so the last working frame disappears and the phase chip stays running. That matches the message you recorded (Preview host failed: request failed. Existing output is not current.).
Two nits so the card stays honest: this error branch is already on main (this PR did not change rebuildPreview), and a Boris rebuild that returns 200 with stale still keeps the frame via setPreview. Your mock hit the HTTP-failure path, which is the real hole.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Agent Completion Report
t3code/fix-graph-fit-undo-previewt3code-278790e980124451mainCloses #971
Closes #972
Closes #975
content/guides/editor.mddocs/changelog.d/982-editor-graph-fit-undo-preview.mddocs/contracts/editor-host.mdeditor/README.mdeditor/scripts/test-host.sheditor/src/last_open.zigeditor/src/main.zigeditor/src/server.zigeditor/ui/src/App.svelteeditor/ui/src/components/GraphMap.svelteeditor/ui/src/lib/api.tseditor/ui/src/lib/state/buffer.svelte.tseditor/ui/src/lib/types.tseditor/ui/src/styles.csseditor/ui/tests/focus-mode.spec.tseditor/ui/tests/graph-map.spec.tseditor/ui/tests/project-tree.spec.tseditor/ui/tests/safe-editing.spec.tseditor/ui/tests/section-nav.spec.tstest-results/alone; did not touch other worktrees or files outside this editor/docs slice.open=restores disposablelast_openwhen it is still a listed author-owned path, elsecontent/index.md. A presentopen=fragment still wins, including ignored unsafe ones.dist/index.htmlat process start remainsstalewith an iframe (already implemented); the host contract now names that meaning so idle is only “nothing to frame.”last_openis disposable state-root, never project truth.git fetch origin main(HEAD18e8391fmatchedorigin/mainafter fix(editor): dialog names, create errors, graph without a file #981)npm --prefix editor/ui run checknpm --prefix editor/ui run test:e2e(187 passed)zig build --build-file editor/build.zig testzig build --build-file editor/build.zigzig build(product./zig-out/bin/boris)npm --prefix editor/ui run build./editor/scripts/test-host.sh ./zig-out/bin/boris ./editor/zig-out/bin/boris-editor editor/ui/dist./editor/scripts/test-preview.sh ./zig-out/bin/boris ./editor/zig-out/bin/boris-editor editor/ui/distzig build testnpm --prefix editor/ui run check: pass (0 errors, 0 warnings; key-hints OK)npm --prefix editor/ui run test:e2e: pass (187 passed, 30.4s)zig build --build-file editor/build.zig test: pass./editor/scripts/test-host.sh …: pass (editor host safe-editing integration: ok, including last_open persist across restart)./editor/scripts/test-preview.sh …: pass (editor live preview integration: ok)zig build test: pass (product suite, including watch-serve, watch-json, init, version-pin)last_openis not repository output. Productzig build testgates that pin byte-shaped compiler output still passed.editor/ui/dist/(gitignored UI build used by host scripts)test-results/from a local Playwright cwd miss; not committed