Fix thinking-block collapse and Safari layout shifts#2426
Conversation
Reproduced three shift mechanisms around reasoning blocks with harness probes before fixing: - When thinking outgrew the send-anchor's fill slack, its ~300px collapse at answer-start re-inflated the spacer and re-anchored the sent message — a measured 267px scroll of the view mid-read. The spacer is now monotonic within a turn (fresh turns still inflate, the composer-clearance floor still wins), so the collapse rides the controller's clamp path instead, which keeps everything below the block viewport-stable. - Safari has no native scroll anchoring (overflow-anchor unsupported), so any above-viewport shrink — a collapsing thinking block, a late image — shoved a detached reader's text by the full delta (350px in the probe; 0px on Chrome, whose native anchoring compensates). Where anchoring is unavailable, the chat glue now tracks the message at the viewport top while detached (binary search per scroll event) and restores its position after content resizes through a new attribution-safe controller adjustment. - The collapse itself was a single-frame teleport, and for the common lone-thinking case it couldn't even be animated: the answer-start branch flip destroyed the expanded component and mounted a collapsed one. ChatMessage now renders one renderUnits template for both the streaming and settled phases, keeping the lone thinking block's instance alive across the flip, and its expand/collapse is a height slide (220ms, disabled under prefers-reduced-motion). Side effect: earlier multi-block tool runs stay summarized while a later step streams instead of re-expanding flat. Two regression tests pin the fixed behaviors (in-turn collapse keeps content below stable; simulated-Safari detached reader stays put through an above-viewport collapse). WebKit itself cannot launch in this container, so Safari is simulated by forcing overflow-anchor off — the only engine difference relevant to these bugs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0148VmxBDazfhczvfE122XVW
![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: cf5bde58f2
ℹ️ 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".
| const el = children[found]; | ||
| this.readAnchor = { el, offset: el.getBoundingClientRect().top - containerTop }; |
There was a problem hiding this comment.
When Safari manual anchoring is enabled and the viewport top is inside a long assistant message, content.children are only the top-level messagesEl children, so this stores the outer message wrapper. Collapsing a thinking block or loading an image earlier inside that same message leaves the wrapper's top unchanged while the visible paragraph moves, making delta stay zero and the Safari jump remain; anchor a descendant near the viewport top rather than the whole message wrapper.
Useful? React with 👍 / 👎.
The Safari manual-anchoring fallback tracked only top-level message wrappers. With the viewport top inside a long assistant message, a thinking block collapsing (or an image loading) above the reading position within that same message leaves the wrapper's top unchanged — delta reads zero and the jump survives. Descend from the straddling wrapper to the deepest element at the viewport top (linear scans below the top level, where children are few and can sit out of flow), which is what native scroll anchoring does. Regression test covers the in-message collapse. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0148VmxBDazfhczvfE122XVW
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5057c0c767
ℹ️ 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".
| private compensateReadAnchor() { | ||
| if (!this.manualAnchoring || this.state.pinned || !this.container) return; | ||
| const anchor = this.readAnchor; | ||
| if (!anchor?.el.isConnected) return; |
There was a problem hiding this comment.
In Safari/manual anchoring, if the anchor is one of the descendants selected above and a markdown worker result or streaming {@html} update in MarkdownRenderer.svelte replaces that node while the user is detached, isConnected becomes false. This early return leaves the stale anchor in place, so the resize that caused the replacement and any subsequent above-viewport resize are not compensated until the next user scroll, reintroducing the jump for rendered/streaming markdown; re-resolve the viewport anchor when the stored element is gone.
Useful? React with 👍 / 👎.
Markdown worker swaps and streaming re-renders replace exactly the deep paragraph nodes the manual anchor tracks. A dead node made compensation early-return, silencing it until the next user scroll — reintroducing the Safari jump for rendered/streaming markdown. The anchor is now a deepest-first ancestor chain: compensation falls back to the nearest surviving ancestor (still covering every shift above it) and re-resolves a fresh deep anchor after every pass, so a replacement can never leave a stale chain behind. Regression test replaces the anchored node in the same pass as an above-viewport shrink and checks the following pass too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0148VmxBDazfhczvfE122XVW
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9513dc4f3a
ℹ️ 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".
| class="thinking-viewport mt-2 flex max-h-56 flex-col justify-end overflow-hidden md:max-h-80" | ||
| class:has-overflow={contentHeight > viewportHeight} | ||
| > | ||
| <div transition:slide={{ duration: collapseDuration, easing: cubicOut }}> |
There was a problem hiding this comment.
When a long streaming thought finishes, loading flips to false while isOpen is still true until the effect closes it, so this slide wrapper is measured after the inner branch has switched from the capped .thinking-viewport to the uncapped settled prose. For reasoning longer than the 56/80 max-height viewport, the block can expand to the full reasoning height before the outro starts, producing a larger layout jump than the one this change is meant to smooth; keep the outgoing content in the capped streaming shape until the collapse is complete.
Useful? React with 👍 / 👎.
When loading flipped false, the template swapped the reasoning content from the capped streaming viewport to the uncapped settled prose in the same flush, BEFORE the auto-collapse effect flipped isOpen — so the slide-out of a long thought started from the full reasoning height (potentially thousands of px), an expand-then-collapse bounce worse than the jump the animation replaced. The open/collapse tracker now runs pre-render ($effect.pre), so the outro starts from the still-capped DOM, and the inner branch keeps the streaming shape whenever the block is closing, so no mid-outro re-render can grow the collapsing box. The regression test asserts the settled prose never appears during the collapse and the box never exceeds its streaming height (shape-based assertions: Tailwind's cap utilities don't load in the browser test environment, which is also why the pre-existing mask test fails there). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0148VmxBDazfhczvfE122XVW
2 participants
Follow-up to #2423, fixing the reported shifting around thinking blocks (Safari especially). Each mechanism was reproduced with harness probes before fixing:
What was shifting
overflow-anchorunsupported), so any above-viewport shrink — a collapsing thinking block, a late image — shoved a detached reader's text by the full height delta. Chrome's native anchoring compensates silently, which is why this read as Safari-specific.Fixes
CSS.supports("overflow-anchor", "auto"). While detached, the chat glue tracks the message element at the viewport top (binary search per scroll event) and restores its position after content resizes through a new attribution-safeadjustByon the controller. Chrome keeps native anchoring; Safari gets the equivalent.renderUnitstemplate across the streaming and settled phases, so the lone thinking block's component instance survives the answer-start flip, and its expand/collapse is a 220ms height slide (instant underprefers-reduced-motion). Side effect: earlier multi-block tool runs stay summarized while a later step streams instead of re-expanding flat.Testing
Two new browser-mode regression tests pin the fixed behaviors (in-turn collapse keeps content below stable; simulated-Safari detached reader stays put through an above-viewport collapse), alongside the existing 50-test scroll suite — all green with the full repo suite, svelte-check, and lint. WebKit itself cannot launch in the CI container (missing system libraries), so Safari is simulated by forcing
overflow-anchoroff in Chromium — the only engine difference relevant to this bug class. A manual pass on real Safari/iOS after deploy is recommended.🤖 Generated with Claude Code
https://claude.ai/code/session_0148VmxBDazfhczvfE122XVW
Generated by Claude Code