perf(desktop): skip settled transcript rows in Rabbita's diff - #1409
perf(desktop): skip settled transcript rows in Rabbita's diff#1409tonyfettes wants to merge 1 commit into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e756b388b
ℹ️ 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".
| for path, state in self.0 { | ||
| guard state is ImageAwaitingRoot else { continue } | ||
| let (next, command) = images.begin_read(owner, path, emit) |
There was a problem hiding this comment.
When a conversation moves from one resolved checkout root to another, this loop retains only ImageAwaitingRoot entries and discards every ImageReady path. Settling an image previously removed its marker and installed the old root's data URL directly in the DOM; because settled rows are now memoized, that DOM is not recreated and no new ImageRequested message is emitted. A relative image can therefore continue displaying bytes from the previous worktree until its row otherwise changes, so rebasing must also reset and re-request already-settled relative images.
Useful? React with 👍 / 👎.
| guard owner.read_target(path) is Some(target) else { | ||
| entries[path] = ImageAwaitingRoot | ||
| return (ImageCache(entries), @cmd.none) |
There was a problem hiding this comment.
When owner.root is already present but @resource.path_for rejects it—for example because the URI belongs to another channel—read_target also returns None, and this branch misclassifies that protocol/ownership error as ImageAwaitingRoot. Since the root may never change again, the placeholder remains pending indefinitely and the real failure is hidden; represent an unresolved root separately from an invalid target and settle or surface the latter as an error.
AGENTS.md reference: AGENTS.md:L27-L35
Useful? React with 👍 / 👎.
Each stream row already keeps its Html between renders (its assoc_by branch recomputes only when the block changes), but Rabbita's diff still compared every settled row's subtree on every frame, so a streaming delta cost a walk over the whole transcript DOM. Rabbita 0.15.7 exposes memo_by, a thunk that the diff skips while its key is unchanged. Wrap each branch's Html in one, keyed by the number of times that branch has rendered: the count moves exactly when the block changes, so an unchanged key means an unchanged subtree. Stream children are keyed, so one branch's count never meets another's. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dp6jf4fKxaVzVq2heQR6dX
6e756b3 to
23697b6
Compare
|
Split: this PR is now only the |
1 participant
Summary
Replaces the approach of #1375. Rabbita 0.15.7 (#1335) exports
@html.memo/@html.memo_by, a thunk that the VDOM diff skips while its key is unchanged, so the transcript no longer needs a DOM island to keep streaming deltas cheap.Each
#streamrow'sassoc_bybranch wraps its Html inmemo_by, keyed by how many times that branch has rendered. The branch renders only when itsBlockchanges (blocks compare by value), so an unchanged key means an unchanged subtree and the diff returns the old node without walking it. Stream children are keyed, so one branch's counter never meets another's. A streaming delta now costs the page skeleton plus oneIntcomparison per settled row, instead of a comparison of every element under#transcript.Live reasoning stays in the model and renders through the ordinary
updatepath, as onmain.The observer / keyed-column / image-root fix that this PR carried at first is unrelated to streaming cost and moved to its own PR: #1415.
Verification
moon check --target js --deny-warn,moon test --target js(3233) indesktop/.just test-browser: 62 passed, two consecutive full runs on this commit alone overmain. (The first run after a freshnpm cifaileddock_visibility.spec.jsonce; it passed alone and in both full reruns.)Profilerover the dev bundle's remote-debugging port); the number to compare is thediff_nodeshare of a streaming-delta frame before and after.🤖 Generated with Claude Code
https://claude.ai/code/session_01Dp6jf4fKxaVzVq2heQR6dX