Repository navigation
fix(chat): auto-scroll transcript to newest message (#155) - #238
Conversation
Port the auto-scroll policy from PR #182 (commit c60ebaf) in scoped form: src/lib/auto-scroll.ts + its 10 tests, byte-identical to PR head 1181372, wired into app/session/[id].tsx. Root cause: the transcript is an inverted FlatList and nothing observed content growth, so an arriving or streaming reply never followed; the only scroll-to-newest path was the manual button. maintainVisibleContentPosition stays (it stops jumps when older pages load), so the fix scrolls explicitly via the existing scrollToBottom when new content arrives — but only while the view is within 200px of the newest message, so a user scrolled up reading history is never yanked. The scroll-to-bottom button now uses the same predicate, so the two cannot disagree about where 'bottom' is. Signature kept at #182's 2-field form (count:length): the only case the 4-field form would catch is a same-length swap of the newest message, negligible for a streaming transcript. Gates: npm run check:versions clean (0.4.17/44), tsc --noEmit clean, full suite 405 pass / 0 fail (395 baseline + 10 ported auto-scroll tests). Device proof still owed before merge.
Stage 4 review verdict: PASSChecked against head Test-count delta: 395 → 405 pass (+10 cases), 50 → 51 test files (+1). Baseline re-run independently in a temp worktree at Compliance (one criterion at a time)
Bug hunt — no validated findingsChecked the call sites:
CI at review time: typecheck/test/secret-scan green; |
Closes #155.
Root cause
The transcript is an inverted
FlatList, so "scroll to the newest message" means scrolling to offset 0 — and nothing was observing content growth, so nothing ever scrolled at all. The only scroll-to-newest path was the manual scroll-to-bottom button.maintainVisibleContentPosition(kept: it stops jumps when older pages load viaonEndReached) actively holds visible items still when new messages insert at index 0, parking new content just outside the viewport.Approach
Scoped port of the auto-scroll part of PR #182 (commit
c60ebaf), which is unmergeable as a whole (49 files spanning keyboard, SSE, voice-alert and other work):src/lib/auto-scroll.ts+src/lib/auto-scroll.test.ts— byte-identical to PR fix: keyboard/composer (#156), auto-scroll (#155), dead SSE (#186), blocked SSE startup (#189), stale transcripts, uncopyable text, axis-perfect table scrolling, 50-session list cap, transcript flash, dark-mode contrast #182 head1181372f(verified by blob hash). No other fix: keyboard/composer (#156), auto-scroll (#155), dead SSE (#186), blocked SSE startup (#189), stale transcripts, uncopyable text, axis-perfect table scrolling, 50-session list cap, transcript flash, dark-mode contrast #182 files taken.app/session/[id].tsx: import the helpers;handleScrollnow writes ascrollOffsetRefand usesshouldShowScrollButtoninstead of the hardcoded> 200; the content signature is computed frommessageData[0]'s parts; a followuseEffectcalls the existingscrollToBottomwhenshouldAutoScrollsays so.maintainVisibleContentPositionuntouched. Nothing else touched: notSessionInfo.onScrollToTop, not the keyboard code, not any other fix: keyboard/composer (#156), auto-scroll (#155), dead SSE (#186), blocked SSE startup (#189), stale transcripts, uncopyable text, axis-perfect table scrolling, 50-session list cap, transcript flash, dark-mode contrast #182 part.The follow only fires while the view is within 200px of the newest message, so a user scrolled up reading history is never yanked back down. The scroll-to-bottom button uses the same predicate, so the two cannot disagree about where "bottom" is.
Signature kept at #182's 2-field form (
count:length): the only case the 4-field form would catch is a same-length swap of the newest message, negligible for a streaming transcript.Test evidence
npm run check:versions— clean (Version metadata aligned: 0.4.17 (44))npm run typecheck(tsc --noEmit) — cleannpm test— 405 pass / 0 fail (baseline 395 + 10 ported auto-scroll tests; standalonenode --test src/lib/auto-scroll.test.ts10/10)Test-count delta: +1 test file, +10 cases — the ported auto-scroll tests are AC-mapped cases for genuinely new behavior (recorded exception, context pack on #155 criterion 5).
Still owed before merge
Device proof (acceptance criterion 6): emulator/physical verification that a streaming reply follows and that reading history is never yanked. Unit tests only so far.