fix(coding-agent): keep the scroll anchor stable when content shrinks - #2214
Merged
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
flora131
force-pushed
the
fix/2205-scroll-anchor
branch
from
August 6, 2026 20:17
aeb528e to
adc02ad
Compare
flora131
force-pushed
the
fix/2205-progress-emission
branch
from
August 6, 2026 21:33
8659692 to
dfd67b7
Compare
flora131
force-pushed
the
fix/2205-scroll-anchor
branch
from
August 7, 2026 02:19
adc02ad to
31e1692
Compare
flora131
added a commit
that referenced
this pull request
Aug 7, 2026
…terior greptile on #2214 reproduced a viewport jumping from entry-10..19 to entry-13..22 after three entries above the reader were removed. The finding is correct, and the defect is larger than the report suggests. rowsShiftedAboveAnchor compared one scalar row count per component and broke at the first component whose end passed anchorRow, on the stated assumption that a spanning component's changes happen at or below the anchor. In production that assumption never holds: the chat host renders the ENTIRE transcript as a single component in slot 0 (chat-session-host-rendering.ts:23-25), so every scrolled-up reader's anchor is inside it, the loop broke immediately, and the compensation this PR added returned 0 for all of them. It engaged only when the anchor sat past the transcript, parked on the trailing spacer. A production path deletes from that component's interior: compaction_end -> refreshCompactedTranscript -> replaceMessages, which splices the entry array in place (chat-message-renderer.ts:160-164). Windowed components may now report a row map, `rowSegments(width)`, identifying each run of rows by the entry that produced it. For a spanning component the shift is the distance the anchored segment itself moved, rather than a height delta. Segments are matched by object identity, not by cache key. Keys embed the entry index (chat-session-host-rendering.ts:167-173), so a splice renumbers every survivor; the entry objects move through it unchanged. Known limits, deliberate: - A real compaction_end rebuilds entry objects via chatEntriesFromAgentMessages, so only extraEntries keep identity. A viewer anchored in the rebuilt region falls back to the nearest surviving segment above, then below, and lands close rather than exact. - No cross-component identity; duplicate ids resolve to first occurrence. - One small object per entry per measured frame, unmemoized. - Components without rowSegments keep the previous break semantics. Two tests added, both verified to fail with only the consuming branch reverted to `break;` -- entry-10 -> entry-13 and entry-10 -> entry-16 -- and to pass once restored byte-identically. All six existing anchor tests are unchanged and still pass; the only pre-existing line touched in that file is an import. Also repairs the [Unreleased] changelog structure in this file. My conflict resolutions while rebasing #2213/#2218 concatenated bullets without respecting headings, leaving `### Removed` with no blank line before it and two Fixed bullets underneath it. That malformed section is already on main; all six bullets are preserved and no released section is touched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Separate pre-existing defect, not a #2205 regression: chat-transcript.ts is byte-identical between 0.9.12 and 0.9.13-alpha.1. Found while falsifying the initial hypothesis for the Ctrl+O autoscroll bug, which the layer below this one actually fixes. ScrollableComponentViewport stores its offset as a distance from the bottom and compensated it only when content grew: if (this.scrollFromBottom > 0 && this.lastWidth === width && lineCount > this.lastLineCount) { this.scrollFromBottom += lineCount - this.lastLineCount; } A component that loses rows -- a live subagent widget dropping its current-tool row at the end of a tool call -- left scrollFromBottom unchanged, and clampScroll() then pulled the view toward the bottom against a smaller maxScroll. Compensation is now symmetric in both directions, so a user scrolled up stays anchored whether content grows or shrinks, while a user already at the bottom still sticks to it. Scope note: ScrollableComponentViewport is used by ChatSessionHost, not by the normal interactive chat transcript, so this does not by itself change the reported Ctrl+O behavior. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…terior greptile on #2214 reproduced a viewport jumping from entry-10..19 to entry-13..22 after three entries above the reader were removed. The finding is correct, and the defect is larger than the report suggests. rowsShiftedAboveAnchor compared one scalar row count per component and broke at the first component whose end passed anchorRow, on the stated assumption that a spanning component's changes happen at or below the anchor. In production that assumption never holds: the chat host renders the ENTIRE transcript as a single component in slot 0 (chat-session-host-rendering.ts:23-25), so every scrolled-up reader's anchor is inside it, the loop broke immediately, and the compensation this PR added returned 0 for all of them. It engaged only when the anchor sat past the transcript, parked on the trailing spacer. A production path deletes from that component's interior: compaction_end -> refreshCompactedTranscript -> replaceMessages, which splices the entry array in place (chat-message-renderer.ts:160-164). Windowed components may now report a row map, `rowSegments(width)`, identifying each run of rows by the entry that produced it. For a spanning component the shift is the distance the anchored segment itself moved, rather than a height delta. Segments are matched by object identity, not by cache key. Keys embed the entry index (chat-session-host-rendering.ts:167-173), so a splice renumbers every survivor; the entry objects move through it unchanged. Known limits, deliberate: - A real compaction_end rebuilds entry objects via chatEntriesFromAgentMessages, so only extraEntries keep identity. A viewer anchored in the rebuilt region falls back to the nearest surviving segment above, then below, and lands close rather than exact. - No cross-component identity; duplicate ids resolve to first occurrence. - One small object per entry per measured frame, unmemoized. - Components without rowSegments keep the previous break semantics. Two tests added, both verified to fail with only the consuming branch reverted to `break;` -- entry-10 -> entry-13 and entry-10 -> entry-16 -- and to pass once restored byte-identically. All six existing anchor tests are unchanged and still pass; the only pre-existing line touched in that file is an import. Also repairs the [Unreleased] changelog structure in this file. My conflict resolutions while rebasing #2213/#2218 concatenated bullets without respecting headings, leaving `### Removed` with no blank line before it and two Fixed bullets underneath it. That malformed section is already on main; all six bullets are preserved and no released section is touched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
greptile raised the same drift a third time after the previous push, this time naming ScrollableChatTranscriptComponent. The thread arrived marked resolved; the code disagreed, and the reviewer was right. That class builds its transcript WITHOUT a cache key (:505), so supportsRowWindow is false, rowSegments returned [] for anything not windowed, and isRowWindowComponent gated the viewport away from asking. The previous commit therefore fixed the ChatSessionHost path -- which does pass a cache key -- and left the exported class exactly as broken. Passing it a cache key would have been the obvious fix and is wrong: chat-message-renderer.test.ts:188-198 pins that a transcript without one reflects entries mutated in place, which is precisely what caching by key would miss. The absent key is the feature. So identifying rows is decoupled from windowing. renderAllRows already walks every entry and knows each block's height, so it records them as it goes -- no extra render, and it cannot go stale against an in-place mutation the way a key can. rowSegments returns those on the static path, guarded by the width they were measured at. The viewport reads segments from static components too, after rendering them, since the recording is a by-product of that render. Two tests added against the exported class: entries removed above the viewer keep the anchored entry, and an entry mutated in place is still reflected -- the second guards the regression the cache-key approach would have caused. Reverting only the static-segment plumbing fails the first with entry-4 -> entry-7 while the other nine pass, so it is specific to this gap. npm run check clean; 626 files / 5968 tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
flora131
force-pushed
the
fix/2205-scroll-anchor
branch
from
August 7, 2026 04:20
b36b289 to
9938973
Compare
| this.ensureBlockCache(width); | ||
| const segments: RowWindowSegment[] = []; | ||
| for (const block of this.blockCache) { | ||
| if (block !== undefined) segments.push({ id: block.entry, rows: block.lines.length }); |
There was a problem hiding this comment.
rowSegments identifies each rendered segment by its ChatMessageEntry wrapper object. Production compaction calls replaceMessages, which rebuilds those wrappers even for surviving logical messages. The next row map therefore has no matching segment for a reader anchored on a surviving message, leaving the old absolute row in place after older messages are removed. A reader positioned on entry-10 can consequently be moved ahead to entry-16 instead of remaining on entry-10. Use a stable logical message identity for row segments, or preserve wrappers for surviving messages.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/coding-agent/src/modes/interactive/components/chat-transcript.ts
Line: 158
Comment:
**Compaction replaces the logical anchor identity**
`rowSegments` identifies each rendered segment by its `ChatMessageEntry` wrapper object. Production compaction calls `replaceMessages`, which rebuilds those wrappers even for surviving logical messages. The next row map therefore has no matching segment for a reader anchored on a surviving message, leaving the old absolute row in place after older messages are removed. A reader positioned on `entry-10` can consequently be moved ahead to `entry-16` instead of remaining on `entry-10`. Use a stable logical message identity for row segments, or preserve wrappers for surviving messages.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Separate pre-existing defect, not a #2205 regression: chat-transcript.ts
is byte-identical between 0.9.12 and 0.9.13-alpha.1. Found while
falsifying the initial hypothesis for the Ctrl+O autoscroll bug, which
the layer below this one actually fixes.
ScrollableComponentViewport stores its offset as a distance from the
bottom and compensated it only when content grew:
A component that loses rows -- a live subagent widget dropping its
current-tool row at the end of a tool call -- left scrollFromBottom
unchanged, and clampScroll() then pulled the view toward the bottom
against a smaller maxScroll. Compensation is now symmetric in both
directions, so a user scrolled up stays anchored whether content grows
or shrinks, while a user already at the bottom still sticks to it.
Scope note: ScrollableComponentViewport is used by ChatSessionHost, not
by the normal interactive chat transcript, so this does not by itself
change the reported Ctrl+O behavior.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Stack created with GitHub Stacks CLI • Give Feedback 💬
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Greptile Summary
The transcript viewport adds per-entry scroll anchoring, but compaction can still move a reader forward in the conversation when it rebuilds wrappers for surviving messages. A reader positioned on a surviving message does not remain on that message after older history is removed.
T-Rex validation blocked
The focused runtime check executed successfully, but the required Greptile artifact-upload tool is unavailable in this environment. Its test source and observed output therefore cannot be attached as uploaded evidence.
Confidence Score: 4/5
The change is not ready to merge because transcript compaction can lose a reader’s logical scroll position.
One user-visible correctness failure remains: rebuilding message-entry wrappers prevents the viewport from matching the reader’s surviving message after compaction.
Files Needing Attention: packages/coding-agent/src/modes/interactive/components/chat-transcript.ts; packages/coding-agent/src/modes/interactive/components/chat-message-renderer.ts
What T-Rex did
Comments Outside Diff (2)
General comment
entry-10is moved toentry-13after production-stylereplaceMessages(messages.slice(3)), even thoughentry-10survives. The reference compaction that reseats the same wrapper objects leaves the reader onentry-10.refreshCompactedTranscriptcallsLiveChatEntriesController.replaceMessagesatpackages/coding-agent/src/modes/interactive/components/chat-session-host-events.ts:134.replaceMessagesinvokeschatEntriesFromAgentMessagesatpackages/coding-agent/src/modes/interactive/components/chat-message-renderer.ts:161, creating new wrapper objects.ChatTranscriptComponent.rowSegmentsemits wrapper objects as IDs atpackages/coding-agent/src/modes/interactive/components/chat-transcript.ts:158; thenrowsShiftedInsideComponentfinds no old ID in the next map and returns 0 at lines 505-529.ChatMessageEntryobject identity for surviving agent messages during compaction (for example, reconcile/reuse wrappers keyed by the underlying message or a stable message identifier), or make transcript segment IDs stable acrossreplaceMessagesreconstruction.packages/coding-agent/src/modes/interactive/components/chat-message-renderer.ts, line 161 (link)replaceMessagesreconstructsChatMessageEntrywrappers for surviving messages.ChatTranscriptComponentidentifies row segments by wrapper identity, so the anchor matcher cannot locate the same logical entry after compaction. A reader who was viewingentry-10instead seesentry-16after six earlier messages are compacted away.chatEntriesFromAgentMessages(messages)returns new wrapper objects beforereplaceMessagessplices them into the transcript.rowSegmentsuses each wrapper asid, and anchor reconciliation looks those IDs up by identity.replaceMessages, rather than the transientChatMessageEntrywrapper identity. Retain the focused regression test.Prompt To Fix All With AI
Reviews (6): Last reviewed commit: "fix(coding-agent): let a static transcri..." | Re-trigger Greptile