Skip to content

fix: smooth session history scrolling and loading older messages - #41

Merged
devswha merged 4 commits into
devswha:mainfrom
snowykr:fix/session-history-scrolling
Sep 9, 2026
Merged

devswha merged 4 commits into
devswha:mainfrom
snowykr:fix/session-history-scrolling

Conversation

@snowykr

@snowykr snowykr commented Sep 6, 2026 •

Copy link
Copy Markdown
Contributor

What this changes

  • Preserve the viewport across history prepends using pre-commit snapshots of the visible message, including messages inside expanded tool groups and late height changes.
  • Retain normalized message IDs, stable group identities, and ID-aware Query structural sharing so loading older rows does not remount existing cards or invalidate unchanged row conversions.
  • Load history when reaching the top without an extra leave-and-return gesture or an idle message-count barrier; keep loading feedback outside the document flow.
  • Detect new-message badges from actual tail additions/content growth rather than total message count.
  • Stop automatic retries on failed/no-progress history responses and show an explicit retry action.
  • Fix GJC history pagination to count visible rows before selecting pages, keep tool results attached, and retain only selected payloads. Deep offsets are no longer limited by the old raw-record ring buffer.
  • Remove speculative content-visibility/intrinsic heights that changed the scroll range again after prepends.

Rebased onto current upstream main (aedb95c), preserving its session-visit isolation, replay-generation cursors, and explicit skill-request parsing. This PR contains one focused commit; the earlier bypass-dialog fix is not included.

Why

In a reported conversation, scrolling stopped at 204 loaded rows even though the transcript contained 823 visible normalized rows. The backend returned an empty page with hasMore: true, causing repeated requests and a flashing “Retrieving earlier messages” indicator. Raw tool-result rows consumed the bounded buffer before visible-row pagination was applied.

Separately, prepends could trigger false new-message badges, reset expanded groups, and move the reader's content. Estimated offscreen heights also caused an additional large scroll-range correction after the initial prepend.

Known limitations / review notes

  • The reporter still finds scroll acceleration insufficiently resolved. This is an incremental improvement, not a claim that all momentum/trackpad scrolling issues are fixed. Opening the PR in the currently tested state is intentional.
  • This is not list virtualization. Removing speculative offscreen sizing trades some layout optimization for predictable geometry; very long expanded histories still need performance review.
  • History requests remain streaming scans with a metadata index; this is not a persistent transcript index.
  • Unbounded history requests over 5,000 visible rows now fail explicitly with HTTP 413 rather than returning a silently incomplete history. Oversized selected payloads also return 413, and a transcript changing during the multi-pass read returns 409 for retry. Existing full-tool-output/search/export consumers that request unbounded history inherit this limit; paginated history can still reach deep offsets.
  • No schema migration, dependency addition, or product-identity change. The author has not added a CLA signature in this PR; the CLA checkbox remains unchecked.

Verification

After rebasing and installing upstream's locked dependencies with npm ci --no-audit --no-fund:

  • npm run verify: not fully passing. Audit, licenses, notices, typecheck, core checks and server suites ran successfully; the Node client-test stage reports 415 passing and 12 failing test files. All 12 fail at module loading with react-syntax-highlighter/dist/esm/styles/prism not exporting oneDark. The shared syntax-highlighter file is unchanged by this PR. The same failure class was also present before the rebase. Tests were not disabled or suppressed.
  • dist-native/bun test src/components/chat/hooks/useChatSessionState.dom.bun.test.tsx src/components/chat/hooks/useChatFollowScroll.dom.bun.test.tsx src/components/chat/view/ChatScrollAnchor.dom.bun.test.tsx src/components/chat/view/TurnWorkBlock.dom.bun.test.tsx src/stores/useSessionStore.dom.bun.test.tsx: 75 passed.
  • dist-native/bun test src/components/chat/tests/chatMessagesPaneCount.test.tsx: 5 passed.
  • TSX_TSCONFIG_PATH=tsconfig.json node --import tsx --test src/components/chat/tests/useChatMessages.test.ts src/stores/sessionMessageFetch.test.ts src/components/chat/utils/paneItemIdentity.test.ts: 26 passed.
  • Focused GJC history/provider tests passed, including repeated visible-row pagination, paired tool results, exact totals, exhausted offsets, and reaching records beyond the old 5,000-row payload window; included in the successful server portion of the final gate.
  • npm run lint, npm run check:identity, npm run build, and git diff --check: passed.

Before rebase, actual-conversation browser/API verification (not a synthetic transcript):

  • 42 distinct page offsets reached all 823 unique rows and the original first prompt, without an empty page claiming more history.
  • Chrome wheel-input measurement on that conversation found zero late resize corrections after removing estimated heights, with maximum observed content movement matching the 80px input step. This does not supersede the reporter's remaining subjective acceleration issue.
  • Expanded nested-group tests used the real store/hooks/pane and a delayed response; maximum measured anchor drift was 0.5px over 25 frames.

No recordings attached. Browser measurements describe the pre-rebase local verification; focused tests and gates above were rerun on the rebased source.


  • I have signed the Contributor License Agreement, or I am the project owner.
  • npm run verify passes, or I have said below which gate fails and why.

Add the two history-loading error/retry keys to all eight remaining locales.
PR devswha#41 added them only to English and Korean, breaking locale parity in
both Node 22 and Node 24 CI jobs.

Keep the parity check intact. The focused locale and full scripts suites,
lint, identity and build pass. Local full verify still encounters the
previously reproduced Node oneDark import issue, unrelated to this fix.
@snowykr

snowykr commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Fixed the CI locale-parity failure in b896f5c.

Both failed Node 22/24 jobs in run 34031218313 reached the scripts phase and failed scripts/check-locale-parity.test.mjs: session.loading.olderMessagesFailed and session.loading.retry were added only to en/ko. Added actual translations for those two keys in de, fr, it, ja, ru, tr, zh-CN and zh-TW — 8 files, 16 added lines, no runtime or test changes.

Verification:

  • Same locale test reproduced the failure before the fix; now 2/2 pass.
  • Full scripts suite: 173 pass, 10 platform-specific skips, 0 failures.
  • npm run lint, npm run check:identity, npm run build: pass.
  • Full local npm run verify also passed audit/licenses/notices/typecheck/Rust/server suites, but hit the already documented local Node oneDark import failure (12 client test files). That was not the GitHub CI failure: the original CI client suites passed. It is not folded into this translation-only repair.

No tests/checks were skipped or relaxed to fix CI. Fresh GitHub checks are running on the pushed commit.

@snowykr

snowykr commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Confirmed on the pushed fix b896f5c: CI run https://github.com/devswha/gajae-code-app/actions/runs/34034993754 is green.

  • Verify Node 22: success (including full npm run verify and standalone website verification).
  • Verify Node 24: success (same gates).

The missing translation keys were the CI blocker; all checks remained enabled.

Follow-up to the visible-row pagination in this branch.

- Tool-result lookup and transcript export walk the transcript in bounded
  pages (`fetchCompleteHistory`) instead of an unbounded read that is now
  refused with 413 past 5,000 visible rows.
- The provider re-reads once or twice when a live writer appends between
  the index and payload passes; only a transcript that keeps changing on
  every attempt surfaces `HISTORY_CHANGED`. A removed transcript is an
  empty window, not a 500.
- `fetchMore` distinguishes a superseded/no-op page from a failed request;
  the retry banner appears only for the latter or a no-progress page.
- The "Get earlier / Get all messages" controls are reachable while history
  remains, so paginated sessions keep a click path; a refused load-all
  raises the same retry alert instead of vanishing.
- Pointer-down stops following only on the scrollbar track, not the pane
  gutter, so a click beside the transcript no longer produces a stale
  unread badge.
- The scroll anchor re-registers resize observers only when the row set
  changes; four unused locale keys and the orphaned `sliceTailPage` helper
  are removed.
@devswha

devswha commented Sep 9, 2026

Copy link
Copy Markdown
Owner

Reviewed on top of a merge with current main (conflict was only the test import in useChatMessages.test.ts). The scroll anchoring, visible-row pagination and ID-aware structural sharing are solid; I pushed 6847c73 (merge) and f2f4f16 to this branch to close the gaps the review found rather than round-trip:

  • 413 blast radius. fetchToolResult and transcript export called fetchHistory({ limit: null }), so "Get complete output" and Export failed outright on any session past 5,000 visible rows. Both now walk bounded pages via fetchCompleteHistory; the raw /messages route keeps the 413.
  • 409 during live runs. A single append between the index and payload passes is routine while a turn is running; the provider now re-reads (3 attempts) before surfacing HISTORY_CHANGED. A missing transcript returns an empty window instead of a 500.
  • False retry banner. fetchMore returned null for superseded, no-op and failed requests alike, so a reconcile after a turn could raise the error banner. It now returns { failed: true } only for a failed request.
  • Load-all unreachable. The top-of-pane "Get earlier / Get all messages" controls now render while hasMoreMessages; "Get earlier" pages once the loaded window is fully visible, and a refused load-all raises the retry alert instead of vanishing.
  • Gutter click stopped following. pointerdown now stops following only on the scrollbar track (offsetX >= clientWidth).
  • Scroll anchor re-registers resize observers only when the row set changes; dropped four unused locale keys and the orphaned sliceTailPage.

Verified on the merged tree: client node 516/516, client bun 517/517, server node 1286 pass (the one failure is a symlinked-node_modules realpath artifact of my worktree; the same file passes on a real checkout), typecheck/lint/locale-parity/build clean. Merging once CI is green.

Follow-ups not in scope here: per-(path,size,mtime) index caching for the three-pass read, and a measured check of a ~2k-row fully loaded session now that content-visibility is gone.

@devswha
devswha merged commit 62fb413 into devswha:main Sep 9, 2026
8 checks passed
devswha added a commit that referenced this pull request Sep 9, 2026
devswha added a commit that referenced this pull request Sep 10, 2026
The encrypted updater key, public key and recovery instructions are now
backed up to iCloud Drive with hash-verified copies; the restore/sign test
and off-Mac password record stay owner-gated. The handoff also records the
closed #41 follow-ups (index cache, 2k-row measurement) and the CLA
reminder on #43.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants