Skip to content

feat(mobile): paginate agent session history#4562

Merged
iscekic merged 19 commits into
mainfrom
feature/mobile-session-message-pagination
Jul 16, 2026
Merged

feat(mobile): paginate agent session history#4562
iscekic merged 19 commits into
mainfrom
feature/mobile-session-message-pagination

Conversation

@iscekic

@iscekic iscekic commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add access-checked cursor pagination for Cloud Agent and remote CLI session messages
  • load the newest 50 messages initially, then merge older pages without disrupting live reconnect behavior
  • render stable prepend loading and explicit retryable, terminal, and omitted-message states in the mobile session transcript

Verification

  • apps/mobile: 816 tests, typecheck, lint, format check, and unused-code check pass
  • apps/web: 334 focused session manager, transport, client, and router tests plus typecheck, lint, and format check pass
  • services/session-ingest: 123 focused tests plus typecheck, lint, format, and diff checks pass
  • fresh independent backend/shared, mobile, Kilobot-repair, and post-main-refresh reviews approved
  • full GitHub CI matrix is green on the latest pushed head

Backend-integrated mobile E2E

A temporary, uncommitted local-only harness seeded normal PostgreSQL session metadata plus the matching real SessionIngestDO SQLite/R2 state. Mobile continued to use the production path: mobile -> tRPC -> authenticated session-ingest /messages -> Durable Object. The harness and all fixture data were removed after verification; git diff --exit-code is clean.

Passed on iOS with a validated cached development build and exact worktree Metro provenance:

  • happy: initial newest 50 opened at the bottom; scrolling issued the cursor-bearing older-page request and prepended messages 001-010; the captured viewport remained on older content rather than jumping back to message 060
  • retryable: missing R2 body produced Couldn't load earlier messages. plus Retry; restoring that exact body and tapping Retry reused the cursor, removed the banner/CTA, and prepended the resolved page
  • non-retryable invalid data: Earlier messages aren't available. rendered with no Retry CTA
  • non-retryable too large: Earlier messages are too large to load. rendered with no Retry CTA
  • omitted: a readable older message loaded and 2 earlier items from this session could not be displayed. rendered with no Retry CTA
  • empty: No messages yet and Send a message below to get started. rendered with no Retry CTA

Cleanup verified: all six sessions were deleted through the normal authenticated DELETE route, referenced R2 bodies were cleared by the DO, the worktree stack was stopped, the simulator was released, and no generated fixture remains.

@iscekic iscekic self-assigned this Jul 15, 2026
Comment thread apps/web/src/lib/cloud-agent-sdk/session-manager.ts Outdated
Comment thread apps/mobile/src/components/agents/use-session-list-auto-scroll.ts Outdated
Comment thread apps/mobile/src/components/agents/use-session-list-auto-scroll.ts Outdated
@kilo-code-bot

kilo-code-bot Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: 3 Issues Found | Recommendation: Address before merge

Executive Summary

The generation-based staleness guard in session-manager.ts's initial-page recording is structured so it can never actually detect a stale callback, risking corrupted pagination state on rapid session re-entry.

Overview

Severity Count
CRITICAL 1
WARNING 2
SUGGESTION 0
Issue Details (click to expand)

CRITICAL

File Line Issue
apps/web/src/lib/cloud-agent-sdk/session-manager.ts 972 recordInitialPage's generation check is a no-op (reads the same live counter on both sides of the comparison), so a stale initial-page callback can overwrite pagination state after a rapid re-switchSession to the same session

WARNING

File Line Issue
apps/mobile/src/components/agents/use-session-list-auto-scroll.ts 92 The 80ms auto-scroll retry now also checks isAutoScrolling, which stays true for 150ms, making the retry's scrollToLatestMessage() call unreachable
apps/mobile/src/components/agents/use-session-list-auto-scroll.ts 189 Adding the isAutoScrolling guard to handleContentSizeChange introduces a 150ms window where content-size changes during streaming no longer schedule an auto-scroll
Files Reviewed (37 files)
  • apps/mobile/src/components/agents/mobile-session-manager.ts / .test.ts
  • apps/mobile/src/components/agents/mobile-session-page-adapter.ts / .test.ts
  • apps/mobile/src/components/agents/session-detail-content.tsx
  • apps/mobile/src/components/agents/session-message-list.tsx
  • apps/mobile/src/components/agents/session-message-list-state.ts / .test.ts
  • apps/mobile/src/components/agents/session-pagination-header.tsx
  • apps/mobile/src/components/agents/session-pagination-header-render-model.ts / .test.ts
  • apps/mobile/src/components/agents/use-session-auto-scroll-state.ts / .test.ts
  • apps/mobile/src/components/agents/use-session-list-auto-scroll.ts - 2 issues
  • apps/web/package.json
  • apps/web/src/lib/cloud-agent-sdk/cli-historical-transport.ts / .test.ts
  • apps/web/src/lib/cloud-agent-sdk/cli-live-transport.ts / .test.ts
  • apps/web/src/lib/cloud-agent-sdk/cloud-agent-transport.ts / .test.ts
  • apps/web/src/lib/cloud-agent-sdk/index.ts
  • apps/web/src/lib/cloud-agent-sdk/session-manager.ts - 1 issue
  • apps/web/src/lib/cloud-agent-sdk/session.ts
  • apps/web/src/lib/cloud-agent-sdk/types.ts
  • apps/web/src/lib/session-ingest-client.ts / .test.ts
  • apps/web/src/routers/cli-sessions-v2-router.ts / .test.ts
  • packages/session-ingest-contracts/src/rpc-contract.ts
  • pnpm-lock.yaml
  • services/session-ingest/src/routes/api.ts / .test.ts
  • services/session-ingest/src/session-ingest-rpc.ts / .test.ts

Fix these issues in Kilo Cloud


Reviewed by claude-sonnet-5 · Input: 54 · Output: 20.2K · Cached: 3M

Review guidance: REVIEW.md from base branch main

@iscekic

iscekic commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

@kilocode-bot please re-review the latest head 9fedc64f6. The three prior findings were fixed, replied to, and resolved with regression tests; the automatic latest-head check failed without producing review output or a new thread.

@kilo-code-bot

kilo-code-bot Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

To use Kilo from GitHub you first need to link your GitHub account to Kilo. Link your Kilo account to continue. After linking, mention me again in this issue or pull request.

@iscekic

iscekic commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

@kilocode-bot New link please.

@kilo-code-bot

kilo-code-bot Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

To use Kilo from GitHub you first need to link your GitHub account to Kilo. Link your Kilo account to continue. After linking, mention me again in this issue or pull request.

iscekic added 2 commits July 16, 2026 15:53
…n-message-pagination

# Conflicts:
#	apps/mobile/src/components/agents/session-detail-content.tsx
#	apps/web/src/lib/cloud-agent-sdk/session-manager.ts
@iscekic

iscekic commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

(bot) @kilocode-bot please review the latest head 86045262c. All prior findings are fixed and resolved, exact-head CI is green, and the six-state mobile E2E completed successfully.

@kilo-code-bot

kilo-code-bot Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Re-review @ 86045262c

Fresh review focused on the three prior findings plus a general pass over the pagination/auto-scroll diff. All three prior findings are resolved correctly. No new blocking issues found.

Prior finding #1 — CRITICAL (generation guard no-op) ✅ Fixed

apps/web/src/lib/cloud-agent-sdk/session-manager.ts:990-993

The guard is now a real snapshot-vs-live comparison. switchSession captures const initialPageGeneration = loadOlderGeneration; after clearAllAtoms() has already bumped loadOlderGeneration (line 565) for the current switch, and recordInitialPage passes that captured constant into applyPage, which compares it against the live counter (expectedGeneration !== loadOlderGeneration, line 824). A second same-session switchSession runs clearAllAtoms()loadOlderGeneration += 1, so a late first-switch callback now fails the check and is dropped. This is no longer tautological.

Regression test session-manager.test.ts:3684 reproduces the exact race: the first fetchSnapshotPage is held on an unresolved promise, a second switchSession to the same id settles first, then the stale first page resolves — and the stale cursor / msg-stale / omittedItemCount: 99 correctly do not overwrite the active msg-current state. The mock harness (line 138-159) genuinely defers the first onInitialPageLoaded until its slow promise resolves, so the test exercises the real ordering rather than a synchronous shortcut.

Prior finding #2 — WARNING (80ms retry dead code) ✅ Fixed

apps/mobile/src/components/agents/use-session-list-auto-scroll.ts:97-104

The 80ms safety-net retry now gates on shouldRetrySessionAutoScroll({ isUserScrolling, shouldAutoScroll }), which deliberately omits the isAutoScrolling check (use-session-auto-scroll-state.ts:60-74). Since isAutoScrolling stays true for the 150ms window, the retry at 80ms is now reachable. It still honours the user-drag and scrolled-away guards. Covered by use-session-auto-scroll-state.test.ts:117.

Prior finding #3 — WARNING (content-size 150ms blind spot) ✅ Fixed

apps/mobile/src/components/agents/use-session-list-auto-scroll.ts:189-213

handleContentSizeChange now bypasses scheduleScrollToLatestMessage and uses shouldFollowSessionContentSize({ isUserScrolling, shouldAutoScroll, didContentHeightChange }), calling scrollToLatestMessage() directly. This removes the isAutoScrolling gate for streaming content growth while adding a didContentHeightChange guard so redundant measurements don't re-scroll. scheduleScrollToLatestMessage (used by the initial itemCount / handleListLayout triggers) keeps its isAutoScrolling guard, which is correct for those one-shot triggers. Covered by use-session-auto-scroll-state.test.ts:152.

General review of the diff

Solid overall. Verified:

  • Cursor pagination staleness: loadOlderMessages captures expectedGeneration before every await and re-checks after each resume (lines 870, 880, 885); applyPage also enforces outcome.info.id === activeSessionId as defense-in-depth. The mobile adapter sets info.id = result.kiloSessionId, which matches activeSessionId, so the session-id guard functions for both initial and older pages.
  • Retryable vs terminal error handling: loadOlderMessages preserves the cursor and leaves hasOlderMessages true on retryable_failure, while invalid_data/too_large/null-outcome set olderMessagesTerminal and stop further backend hits. shouldTriggerOlderMessagesLoad mirrors this (blocks non-retryable, allows retryable re-fire). Header priority in selectSessionMessageListHeaderState (error → loading → omitted → hidden) is consistent.
  • Reconnect path (cloud-agent-transport.ts:221-252) deliberately skips onInitialPageLoaded so a reconnect never resets the user's older-pages cursor — good.
  • Worker HTTP route (services/session-ingest/src/routes/api.ts) applies the shared default page size, bounds limit/before, performs its own owner+org-membership access check mirroring findOwnedAccessibleSession, and maps schema-parse failures to { kind: 'invalid_data' } and null history to a readable "not found". Consistent with the tRPC getSessionMessagesPage access check.

Non-blocking observations (SUGGESTION)

  1. apps/mobile/src/components/agents/session-transcript.ts:30-34 — orphan preparation attempts (whose triggerMessageId is not among the loaded messages) are appended at the end (newest position). With bounded pagination, a preparation attempt whose trigger message lives in an as-yet-unloaded older page would render at the bottom, out of order. Low risk in practice because preparationAttempts are cleared on switchSession and populated by live events for the current run (whose trigger messages are in the newest page), but worth a comment or a guard if preparation history ever gets paginated too.

  2. apps/mobile/src/components/agents/session-detail-content.tsx:640onLoadOlderMessages={() => { void manager.loadOlderMessages(); }} is a fresh closure each render, which recreates handleStartReached in SessionMessageList every render. Purely a minor perf nit (wrap in useCallback); no correctness impact.

Verification performed

  • Ran the 5 new pure-logic mobile suites (use-session-auto-scroll-state, session-message-list-state, session-transcript, session-pagination-header-render-model, mobile-session-page-adapter): 49/49 pass.
  • The web Jest suite (session-manager.test.ts) could not be executed in this sandbox because the harness requires a Postgres test DB (failure is in cleanupDbForTest global setup, not in the code under test). Logic was verified by reading; the PR reports CI green.

Recommendation

Approve. All three prior findings are correctly fixed with targeted regression tests, and the surrounding pagination / auto-scroll / error-state logic is consistent and defensive. The two suggestions above are optional follow-ups and need not block merge.

@kilo-code-bot

kilo-code-bot Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

The re-review has already been posted to the PR — approved, all three prior findings (critical generation-guard no-op, and two auto-scroll warnings) confirmed fixed at head 86045262c, with only two non-blocking suggestions remaining. No further action needed from me here.

…n-message-pagination

# Conflicts:
#	apps/mobile/src/components/agents/session-detail-content.tsx
@iscekic
iscekic enabled auto-merge (squash) July 16, 2026 16:44
@iscekic
iscekic merged commit 8322bf6 into main Jul 16, 2026
65 checks passed
@iscekic
iscekic deleted the feature/mobile-session-message-pagination branch July 16, 2026 16:50
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