Keep the reading position stable while paging long conversations - #454
Merged
Merged
Conversation
… timestamp The chat pages history in from the tail, so a suffix of the event log must fold its turns exactly like the whole log does. Two things broke that: - apply_delta looked up the streamed item across the whole timeline. pi streams every turn's assistant message under the same placeholder id (pi-assistant-0:<index>) until completion names it, so a later turn's stream was appended to the earliest turn's entry. Live, the text landed in the wrong turn; during paging, each page that revealed an earlier turn pulled entries out of loaded turns, which the list treated as a replacement and reset to the tail. Deltas now continue only an entry in the current turn. - Synthetic ids (error-N, compacted-N, ...) counted from the start of the fold, so every page that contained such an entry renumbered all later ones. Timestamped records now name themselves; untimestamped legacy logs keep the counter.
Streamed conversations hold hundreds of delta records per turn, so a 400 record snapshot and 200 record pages always began mid-turn: the chat folded a partial first turn whose entries kept shifting as pages arrived, and a 200k record thread needed a thousand round trips (each re-reading the log) to load, which read as a hang. The snapshot and every page now extend back to the record that opens the turn they fall in, within the existing 8 MiB envelope; the 25 MB / 19 turn repro loads in 16 pages.
Three ways a page could move the reader: - When the snapshot held a single partial turn, the first page changed that turn's identity, so list_sync saw no stable row and either reset the list to the tail or appended the new turn at the wrong end. Rows now carry the identity of their newest entry, which a page cannot change, and a page is a prepend whenever the newest loaded turn continues. - While earlier history is unloaded, a page can merge the partial first turn's streamed fragments so its row shrinks; that was read as a replacement and reset the list. The chat passes the history state as TimelineContinuity, and the first turn is remeasured instead. - A reader who had scrolled into the incoming-history reservation was re-anchored at a negative offset into the former first turn, which leaves the rows above it unpainted until the next scroll and then jumps. The anchor now clamps to that row's top and the next frame walks back over the measured page, keeping the same pixel position.
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.
Fixes three reports on a very long (203k-record, 19-turn, pi/deepseek) conversation: visible jumps while scrolling, loading that appears to hang, and the view snapping back to the bottom during continuous up-scroll.
Root causes
list_syncfell back toListSync::Reset(which clears the scroll anchor and re-engages tail following) on 21 of ~1,000 history pages. Two triggers:Timeline::apply_deltalooked up a streamed item id across the whole timeline, so when a page revealed an earlier turn, every later turn's stream (pi reusespi-assistant-0:<i>for each turn) was folded into that earlier turn; and synthetic ids (error-N,compacted-N, …) were numbered from the start of the fold, so any page renumbered later ones. Either way entries changed identity →Reset.offset_in_itemwhen the reader had scrolled into the history reservation, which gpui leaves unpainted (blank viewport, then a >1-viewport jump on the next wheel notch).Changes
apply_deltacontinues only an entry of the current turn;synthetic_idnames timestamped records by timestamp (error-<ts>,-1/-2on same-ms collisions), untimestamped legacy logs keep the counter. A suffix of the log now folds its turns exactly like the full log.TurnListItem.tail_identitylets Prepend be recognised when the newest loaded row continues;TimelineContinuity { Complete, PartialFirstTurn, NewSession }replaces thereset: boolso a partial first turn is remeasured instead of resetting the list. The Prepend anchor clamps to ≥ 0 and, when the reader was inside the reservation, oneon_next_framewalks back by the same distance once the page is measured.Evidence
TurnIndexCache::syncas the client pages: before 21 Reset / 14 Prepend / 978 Incremental; after 0 Reset.streamed_deltas_continue_the_open_turn_not_an_earlier_turn_with_the_same_id,synthetic_entry_ids_do_not_depend_on_how_much_earlier_history_is_folded; runtimehistory_snapshot_and_pages_start_at_turn_boundaries; uihistory_completing_the_only_partial_turn_prepends_instead_of_resetting,completing_the_only_partial_turn_keeps_the_reading_anchor, extendedincoming_page_replaces_scrollable_reservation_without_moving_content(asserts a non-negative anchor after the walk-back frame).cargo fmt --all --check,cargo clippy --workspace --all-targets --locked -- -D warnings,cargo build --workspace --locked,cargo test --workspace --locked,cargo macheteall green locally.Known gaps / follow-ups
responseId, so a turn can show both the streamed and completed text; that is acrates/agent/src/pi.rsissue outside this fix.tcode_threadexport, so it cannot be imported through the UI; it was seeded directly into a throwaway profile for testing.