fix(desktop): Virtuoso initial scroll to bottom + JumpBar onScrollToTurn callback (#2056 hotfix)#2073
Conversation
…ng, rename defaultOpen prop
… in streaming handlers
…ender visible messages
…ered message nodes" This reverts commit 6d90d65.
… add readTailMessages for backward JSONL scan
…ength to entries.length; update callers
… for scroll handling
… outside thread; use position:fixed instead of sticky
…p button outside thread; use position:fixed instead of sticky" This reverts commit c061398.
…lToIndex for scroll handling" This reverts commit ffd48d6.
…; fix JumpBar onScrollToTurn callback
…calc in flex layout
…-bottom and persistence
|
If the remaining Virtuoso issues are too disruptive, feel free to revert #2056 entirely and go back to the simple `overflow-y: auto` scrollable thread. The virtual list was meant to reduce DOM nodes on long sessions, but the existing `content-visibility: auto` + `elideTranscriptMessages` (120-msg window with heavy content stubbing) already covers most of that use case without the complexity. There may be other edge cases I haven't found yet — this PR only addresses the two most visible regressions. |
esengine
left a comment
There was a problem hiding this comment.
Thanks for the hotfix — the two shipped fixes (initial scroll-to-bottom + JumpBar callback) are exactly the right shape and CI is green.
But the description doesn't match the diff:
- ✅ #1
initialTopMostItemIndex— in the diff. - ✅ #2 JumpBar
onScrollToTurn— in the diff. - ❌ #3 "Added
m.kind === 'error'andm.kind === 'warning'render cases back to virtuosoitemContent." — not in the diff. TheitemContentcallback still returnsnullfor everything exceptuser/assistant, so error and warning messages remain invisible. This is a real visible regression from #2056 —$errorevents get pushed intostate.messages(App.tsx L1101) but never render. - ❌ #4 "Pending-approval cards moved to
components.Footer" — not in the diff. They're still rendered as a separate block below the scroll area (App.tsx L2401).
Could you either:
- Ship #3 in this PR (it's the meaningful regression — error / warning visibility on desktop is broken right now), and drop #4 from the description (or split it into its own follow-up since it's a layout choice, not a bug), OR
- Trim the description to just the two scroll fixes, merge this as-is, and open a separate hotfix PR for #3 with the same urgency.
Lean (1) since the error/warning visibility regression is currently shipping in main and users won't see error messages until it's fixed.
|
Just checked — error and warning render cases are already present in upstream/main's `itemContent` (they were part of the original #2056). The diff in this PR is intentionally minimal: only `initialTopMostItemIndex` and JumpBar `onScrollToTurn`. Error/warning visibility is not regressed on current `main`. |
esengine
left a comment
There was a problem hiding this comment.
Re-reviewed — the meaningful regression I flagged is fixed: itemContent now renders m.kind === "error" (recoverable → warning label, else error, with a dismiss button wired to dismiss_error) and m.kind === "warning", so the $error/$warning events that get pushed into state.messages are visible again instead of silently dropped. Combined with the two scroll fixes (initialTopMostItemIndex + JumpBar callback) this is the right scope. CI green. Merging.
1. Virtuoso container height 100% → 90%
Why: At 100% height the container sits flush with the bottom edge. The Composer's gradient overlay (
::beforepseudo-element, extending 24px upward) covers the last message. 90% height leaves room at the bottom, preventing content from being obscured and reducing the visual impact of flex layout shifts when sending messages.2. Always scroll to bottom (removed atBottomRef guard)
Why: When a message is sent, the Composer clears and shrinks → flex layout recalculates →
.threadgets taller → Virtuoso thinks the user left the bottom (scrollTop unchanged but container grew) →followOutputstops firing → subsequent streaming content appears above the viewport, invisible to the user.Removing the
atBottomRefgate ensures every state change forcesscrollTop = scrollHeight, keeping new content always visible.Files changed
desktop/src/App.tsx— +7 / -21