perf(desktop): isolate live reasoning from transcript rendering - #1375
perf(desktop): isolate live reasoning from transcript rendering#1375tonyfettes wants to merge 8 commits 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: 99bb4feb2f
ℹ️ 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".
| previews: self.previews | ||
| .filter(preview => preview.key != key) | ||
| .push({ key, chunks: @vector.new(), complete: true, }), |
There was a problem hiding this comment.
Each normal local run's raw terminal event reaches Finished and leaves a completed preview in this vector; the subsequent top-level AgentFinished only maps existing entries, and no later StepStarted uses the terminated run ID to remove it. A long-lived desktop session therefore retains one tombstone per completed run, while every reasoning delta linearly scans the growing vector. Retire terminal previews or keep the late-event fence in a bounded structure.
Useful? React with 👍 / 👎.
d0b7558 to
b11a7fa
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f375a16686
ℹ️ 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".
| content=self | ||
| .preview_for(target.key) | ||
| .map(preview => preview.content()) | ||
| .unwrap_or(""), |
There was a problem hiding this comment.
When the target has no open preview, this converts None to "", and LivePreview::sync later interprets content.is_empty() as the instruction to clear the host. Carry String? through the Sync effect and branch on None explicitly instead of encoding absence as payload text.
AGENTS.md reference: AGENTS.md:L27-L30
Useful? React with 👍 / 👎.
| fn RemoteBridgeSub::unload(self : RemoteBridgeSub) -> Unit { | ||
| self.active = false | ||
| @interop.ws_close(self.channel) |
There was a problem hiding this comment.
When sign-out or a roster update removes a device, this marks the subscription inactive before closing it, so the on_down callback returns without emitting ChannelDown; no subsequent AgentConnected or RunFinished can arrive after the subscription is gone. Unlike the completed-run tombstone case, any in-progress reasoning chunks for that retired device therefore remain indefinitely in the global LiveReasoningState vector and are scanned on future deltas. Clear the channel as part of this retirement path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Accepted as is. A removed device leaves at most the previews of runs that were mid-reasoning at removal; that set is bounded by the removal, not by time, and a device that returns clears its channel on agent.connected. Not worth a third clearing path beside ChannelDown and AgentConnected.
8871dfb to
69535f3
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69535f350c
ℹ️ 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".
| let image_root = input | ||
| .map(input => input.root) | ||
| .switch_by(_ => message_on_create(emit(ImageRootChanged)), by=root => { | ||
| root.map(uri => uri.to_string()).unwrap_or("") |
There was a problem hiding this comment.
When a conversation or worktree root has not resolved yet, this converts the genuine None state into the string sentinel "" for branch identity, making absence indistinguishable from a payload value and risking a suppressed ImageRootChanged transition if those representations collide. Keep the key optional or use a dedicated tagged identity instead.
AGENTS.md reference: AGENTS.md:L27-L30
Useful? React with 👍 / 👎.
|
I think we'd better fix it on the rabbita side. |
…erver The bridge matched five engine event shapes to call five preview entry points, each re-checking the session and run id. LiveReasoning::observe takes the event once and answers Consumed, Forward, or ForwardAfterReasoning; the bridge only turns that into a command. A retried provider stream reopens the run's preview like a new step. The state machine drops its tombstone per finished run (RunFinished deletes the run's entries; the semantic Finished seal still fences late fragments until the next step), shares one hide_when helper, and appends a delta through one path. The Sync effect carries the rehydrated preview as an option instead of an empty string standing in for "no preview". The preview appends text with CharacterData.appendData on a single text node instead of copying bounded chunks. The step label comes from Input::next_step_label, shared with the stream renderer. The reducer tests move to a whitebox file with their state-only helpers. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dp6jf4fKxaVzVq2heQR6dX
…nversation column A new transcript component's subscription started during the graph read that created it, while the previous conversation's scroller was still on screen. Its first attach observed that scroller's #stream; Rabbita then patched the same <section> in place (same tag and position in the root's positional children) and only replaced the keyed #stream, so the section identity check never re-attached and content growth went unobserved. The root view now keys the conversation column, so a conversation's section is its own element, and the observers attach through an after-render command once this component's DOM is committed. Image reads that arrive before the conversation root resolves are kept as ImageAwaitingRoot and requested when the owner gains a root. The root is a branch keyed by its value, so the change reaches update as one message instead of through a DOM attribute. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dp6jf4fKxaVzVq2heQR6dX
Reasoning fragments are consumed at the transport boundary by the transcript component's live preview and never reach the application, so the model's ReasoningDelta accumulation, the complete flag on its provisional thought, and the "Thinking…" text branch of the assistant view were unreachable. RunState keeps only sealed_reasoning: the run's full reasoning event, shown until its durable commit arrives. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dp6jf4fKxaVzVq2heQR6dX
It cleared a channel's preview when its subscription unloaded. Both unload causes are already covered: a socket that goes down reports ChannelDown, which clears, and a replacement connection announces AgentConnected, which clears again. A device removed from the roster is never focused, and RunFinished drops its entries per run. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dp6jf4fKxaVzVq2heQR6dX
The preview's state machine, DOM island, and reducer tests lived inside the transcript component package but depend only on the transport event type and the channel id. frontend/transcript/live_reasoning now owns them; the component supplies the focus target (Key plus step label) from its input and mirrors its scroll model through set_pinned. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dp6jf4fKxaVzVq2heQR6dX
The preview built the whole activity card by hand (row, details, summary, step label, body) and rebuilt it whenever the target or the step changed. Rabbita now renders that shell from host_view, keyed like any other transcript child, and the preview owns only the descendants of the innermost text element, which has no virtual-DOM children. CSS hides the card while that element is empty. The step label therefore comes from the component's view again, the focus target is just the run Key, and the DOM island shrinks to acquiring the text element, appending text nodes, and following the tail. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dp6jf4fKxaVzVq2heQR6dX
b8e08c1 to
9f292ed
Compare
1 participant
Each streamed reasoning delta previously entered the application update path and caused comparison of the conversation's accumulated DOM. Render the live preview outside the observed stream, append text in animation-frame batches, and let reasoning-only events return without scheduling an application command.
Connection subscriptions filter retired callbacks before forwarding notifications. Remote subscriptions become inactive immediately when the socket reports down; unloaded local and remote subscriptions also reject their queued callbacks. The preview tracks content by device, conversation and run. Source restart or retirement clears that device's transient preview.
Shape after rebasing onto #1383 / #1385 / #1379
frontend/transcript/live_reasoningpackage. State machine (live_reasoning.mbt), text island (dom.mbt), the card shell (view.mbt), reducer tests. The transcript component supplies the focusKeyfrom its input and mirrors its scroll model throughset_pinned; the bridge routes engine events throughLiveReasoning::observe.host_viewrenders the activity card like any other transcript child; only the innermost text element has no virtual-DOM children, and the preview appends text nodes there. CSS hides the card while it is empty. No hand-built DOM, no step label in the state machine.switch_bybranch, not a DOM attribute. The component keys a branch by the run; itsinitfocuses the preview after render and its subscription'sunloadreleases it. Pinned reaches the preview through the subscription; no module-levelRef.observe(channel, session, run_id, event)answersConsumed/Forward/ForwardAfterReasoning.RunFinisheddeletes the run's entries; a retried provider stream reopens the run's preview like a new step. Text is appended withCharacterData.appendData.RunStatekeeps onlysealed_reasoning : String?.Two bugs found while rebasing (both had E2E coverage only under specific timing)
<section id="transcript">in place across two components (same tag, same position in the root's positional children), so a new component's subscription, started during the graph read, attached to the previous#stream. The root view's column is now keyed (transcript\0<slot>), and the observers attach through an after-render command.ImageAwaitingRootand are retried when the owner gains a root; the root is a branch keyed by its value that delivers one message.Codex review: P1 (empty-string sentinel) fixed; P2 tombstones resolved by
RunFinished; P2 residue after a device is removed accepted, see the thread.Verification
moon check --target js --deny-warn; package interfaces syncedmoon test --target js: 3214 passedjust test-browser: 62 passed🤖 Generated with Claude Code
https://claude.ai/code/session_01Dp6jf4fKxaVzVq2heQR6dX