Repository navigation
fix(app): make the turn navigator easier to hit and land on the right turn - #57572
Merged
Merged
Conversation
1 task
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.
Summary
Fixes the experimental turn navigator from Kit's review: it was hard to hit, clicking a tick could land on the wrong turn, and previews showed the wrong reply.
Causes
timeline-history.spec.ts("keeps visible content through …","limit": 9007199254740991). The rail's user-message index fired on a 1s timer from mount, so on slow runners it landed in that test's exact log of transcript pages. The index load now waits until the timeline's own history requests have been idle for 1s, reads newest-first with a 1,000-turn cap, and is aborted on unmount. Because that test logs every transcript read, it also turns the rail off.pointerleave, so scrolling kept it open over turns that had moved. Now there is one always-mounted preview card. Scrolling over the rail closes it, and the next pointer move reopens it on the turn under the pointer.Feel
frameLoop.addMeasured.Proof
Before (left) and after (right), in the mocked web app. The cursor sweeps the rail, clicks a middle turn, clicks the bottom tick, then scrolls over the rail:
before-after-clip.mp4
Tests
timeline-history.spec.ts, new keeper: the rail keeps timeline order when message IDs from other clients don't sort by time. An unloaded turn previews exactly its prompt and its own reply, and clicking it lands on that turn after paging. Scrolling closes the single preview. The last tick lands on the latest turn. It fails onbeta(the preview shows "Prompt number 6" for the sixth tick) and passes with the change, along with the existing navigator keeper. The wholetimeline-history.spec.tsand the settings navigator keeper pass on a built app.beforeanchor without a type filter, matching the server.LOC: code +275/−123 (
turn-navigator.tsx, plus a few lines ininteraction.ts,use-session-hash-scroll.tsandmessage-timeline.tsx), tests +52/−3.bun typecheck(app) passes, andlint:changedreports nothing in the changed files. The pre-pushcheckfails on an existinggui-extensions/src/details/popover.tsxlint error frombeta, unrelated to this change.Review follow-ups
messagesBeforefrom fix(app): order messages by position, not by ID string #57581 once that lands.Performance
session-timeline-benchmarkstreaming with 320 history turns, 160 deltas and 30× CPU throttle on the Pro. I ran six rounds, alternating the merge-base build and this head on one machine. Medians:No difference beyond noise. An earlier, non-alternating run had pointed to a regression: the rail read layout on every streamed frame. That is fixed above. The session-entry benchmark times out before reporting, on beta as well as on this branch.
Overlap: this stays in the navigator and its click path. It doesn't touch
virtualizer.tsx(thetimeline-motionrework) or grouped user messages (#57552).