fix(desktop): attach transcript observers after render and key the conversation column - #1415
fix(desktop): attach transcript observers after render and key the conversation column#1415tonyfettes wants to merge 1 commit into
Conversation
…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. The Codex first-turn E2E test filled the composer as soon as the draft-open request was sent, before the Codex page had rendered; with positional children the OpenSeek textarea used to be patched into the Codex one, which hid the race. It now waits for the Codex composer. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dp6jf4fKxaVzVq2heQR6dX
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: be64bdade0
ℹ️ 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 input.root is None, unwrap_or("") converts that absence into a sentinel string instead of retaining a distinct optional/tagged branch. This makes the switch identity depend on an implicit promise that no serialized root can equal the sentinel and violates the repository invariant for representing missing values; construct an explicitly tagged key for None and Some(uri) instead.
AGENTS.md reference: AGENTS.md:L27-L30
Useful? React with 👍 / 👎.
1 participant
Summary
Split out of #1409 (and originally found while rebasing #1375). Two latent transcript bugs, independent of any rendering optimization:
#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 (Map[String, Html]children), 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.begin_readused to drop that read, and nothing retried it unless some later content mutation happened to rescan. Such reads are now kept asImageAwaitingRoot; the root is aswitch_bybranch keyed by its value, so its arrival reachesupdateas oneImageRootChangedmessage andrebaseissues the waiting reads.One E2E test needed a change: "Codex creates a thread, sends its first turn, and stops it" filled the composer as soon as the draft-open request was sent, before the Codex page had rendered. With positional children the OpenSeek textarea was patched into the Codex one, which hid the race; with the keyed column the fill landed in an element that was then replaced. The test now waits for the Codex composer.
On the two Codex P1s raised against #1409
Both target the image-root change and describe behaviour that already exists on
main:mainnever rebased the cache at all, and the settled<img>has already lost itsdata-transcript-imagemarker, so it is not re-requested there either. The transition this change handles isNone → Some(a worktree bound after the first send);checkout_rootoffers no path from one resolved root to another.path_forrejects is classified as awaiting.ImageOwnertakeschannelandrootfrom the sameInput, so a root owned by another channel cannot be produced by the app;mainsilently dropped that read as well. Not changed here.Verification
moon check --target js --deny-warn,moon test --target js(3217),moon test --target native(3248) indesktop/.just test-browser: 58 passed, two consecutive full runs (on the perf(desktop): skip settled transcript rows in Rabbita's diff #1409 tree that carried this commit together with the memo change).🤖 Generated with Claude Code
https://claude.ai/code/session_01Dp6jf4fKxaVzVq2heQR6dX