-
Notifications
You must be signed in to change notification settings - Fork 1
fix(editor): keep graph zoom readable, coalesce undo, and restore last-open #982
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
bf037cc
fix(editor): keep graph zoom readable, coalesce undo, and restore las…
drawmeanelephant 8012445
docs(changelog): name the fragment for PR 982
drawmeanelephant 5c1ca60
fix(editor): keep the open/save wait-copy test off the default launch…
drawmeanelephant File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| <!-- | ||
| Filename: 982-editor-graph-fit-undo-preview.md | ||
| Keep exactly one category heading. Replace this example link with a relevant | ||
| repository-root-relative link; contract-visible work links its updated contract. | ||
| --> | ||
|
|
||
| ### Fixed | ||
|
|
||
| - Editor graph map opens at a readable zoom (50% floor) centered on the active | ||
| page while Fit still shows the whole graph; typing coalesces into one undo | ||
| step per phrase; a cold launch restores the last author-owned file from the | ||
| disposable state root (or `content/index.md`); and an existing `dist/` tree | ||
| is framed as stale instead of an empty idle preview. Links: [the editor-host | ||
| contract](/docs/contracts/editor-host.md), [the editor | ||
| guide](/content/guides/editor.md). |
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| //! Last-opened author path, stored under the disposable editor state root. | ||
| //! Never project truth: a missing, corrupt, or unsafe file is ignored. | ||
|
|
||
| const std = @import("std"); | ||
| const Io = std.Io; | ||
| const file_api = @import("file_api.zig"); | ||
|
|
||
| const file_name = "last-open.json"; | ||
|
|
||
| const Document = struct { | ||
| path: []const u8, | ||
| }; | ||
|
|
||
| pub fn save(allocator: std.mem.Allocator, io: Io, state_root: []const u8, path: []const u8) !void { | ||
| try file_api.validatePath(path); | ||
| try Io.Dir.cwd().createDirPath(io, state_root); | ||
| var dir = try Io.Dir.cwd().openDir(io, state_root, .{ .follow_symlinks = false }); | ||
| defer dir.close(io); | ||
|
|
||
| const bytes = try std.json.Stringify.valueAlloc(allocator, .{ .path = path }, .{}); | ||
| defer allocator.free(bytes); | ||
| var atomic = try dir.createFileAtomic(io, file_name, .{ .replace = true }); | ||
| defer atomic.deinit(io); | ||
| var write_buffer: [1024]u8 = undefined; | ||
| var writer = atomic.file.writer(io, &write_buffer); | ||
| try writer.interface.writeAll(bytes); | ||
| try writer.flush(); | ||
| try atomic.file.sync(io); | ||
| try atomic.replace(io); | ||
| } | ||
|
|
||
| pub fn load(allocator: std.mem.Allocator, io: Io, state_root: []const u8) std.mem.Allocator.Error!?[]u8 { | ||
| var dir = Io.Dir.cwd().openDir(io, state_root, .{ .follow_symlinks = false }) catch return null; | ||
| defer dir.close(io); | ||
| const bytes = dir.readFileAlloc(io, file_name, allocator, .limited(8192)) catch return null; | ||
| defer allocator.free(bytes); | ||
| var parsed = std.json.parseFromSlice(Document, allocator, bytes, .{}) catch return null; | ||
| defer parsed.deinit(); | ||
| file_api.validatePath(parsed.value.path) catch return null; | ||
| return try allocator.dupe(u8, parsed.value.path); | ||
| } | ||
|
|
||
| pub fn clear(io: Io, state_root: []const u8) !void { | ||
| var dir = Io.Dir.cwd().openDir(io, state_root, .{ .follow_symlinks = false }) catch |err| switch (err) { | ||
| error.FileNotFound => return, | ||
| else => |other| return other, | ||
| }; | ||
| defer dir.close(io); | ||
| dir.deleteFile(io, file_name) catch |err| switch (err) { | ||
| error.FileNotFound => {}, | ||
| else => |other| return other, | ||
| }; | ||
| } | ||
|
|
||
| test "last-open path survives a new store instance" { | ||
| const allocator = std.testing.allocator; | ||
| const io = std.testing.io; | ||
| var temp = std.testing.tmpDir(.{}); | ||
| defer temp.cleanup(); | ||
| const root = try temp.dir.realPathFileAlloc(io, ".", allocator); | ||
| defer allocator.free(root); | ||
|
|
||
| try std.testing.expect(try load(allocator, io, root) == null); | ||
| try save(allocator, io, root, "content/guides/start.md"); | ||
| const loaded = try load(allocator, io, root); | ||
| defer if (loaded) |path| allocator.free(path); | ||
| try std.testing.expectEqualStrings("content/guides/start.md", loaded.?); | ||
| try clear(io, root); | ||
| try std.testing.expect(try load(allocator, io, root) == null); | ||
| } | ||
|
|
||
| test "unsafe last-open files are ignored" { | ||
| const allocator = std.testing.allocator; | ||
| const io = std.testing.io; | ||
| var temp = std.testing.tmpDir(.{}); | ||
| defer temp.cleanup(); | ||
| const root = try temp.dir.realPathFileAlloc(io, ".", allocator); | ||
| defer allocator.free(root); | ||
|
|
||
| try std.testing.expectError(error.PathNotAuthorOwned, save(allocator, io, root, "dist/index.html")); | ||
| try std.testing.expectError(error.InvalidPath, save(allocator, io, root, "../secret")); | ||
|
|
||
| try Io.Dir.cwd().createDirPath(io, root); | ||
| var dir = try Io.Dir.cwd().openDir(io, root, .{ .follow_symlinks = false }); | ||
| defer dir.close(io); | ||
| try dir.writeFile(io, .{ .sub_path = file_name, .data = "{\"path\":\"dist/index.html\"}" }); | ||
| try std.testing.expect(try load(allocator, io, root) == null); | ||
| try dir.writeFile(io, .{ .sub_path = file_name, .data = "{not-json" }); | ||
| try std.testing.expect(try load(allocator, io, root) == null); | ||
| } |
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
Relevant code
editor/ui/src/App.svelte:250-270const 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-78Evidence Package
Copy prompt for an agent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@itoqa Thank you — we checked this against the code and the observation is right.
rebuildPreviewsetsphasetorunning, then on a non-ok host response other thanwatch_daemon_activeit only updatespreview.status.PreviewPanehides the iframe unless phase issuccessorstale, so the last working frame disappears and the phase chip staysrunning. 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 changerebuildPreview), and a Boris rebuild that returns 200 withstalestill keeps the frame viasetPreview. Your mock hit the HTTP-failure path, which is the real hole.