Merge task 1387 iOS transcript history scroll

Author: Pyiner

docs/design/task-1387-ios-transcript-history-scroll.md

@@ -0,0 +1,197 @@
+# TASK-1387 iOS Transcript History Scroll Stability
+
+## Scope
+
+This is an iOS-only fix for chat transcript history expansion in
+`mobile/garyx-mobile`. It does not change desktop behavior, the gateway event
+stream contract, or server-side render-state derivation.
+
+The invariant stays unchanged:
+
+- SSE `events` remain gapless by `after_seq`.
+- `render_state.rows` may be narrowed by a render floor.
+- `render_state.based_on_seq` remains the committed tail.
+- iOS dumb-renders server `render_state.rows`; it does not regroup turns, pair
+  tools, place final answers, or derive tail thinking.
+
+## Reproduction
+
+The red SwiftPM tests added for this task cover the current unstable paths:
+
+- `GaryxTranscriptSyncPlannerTests.testCapturedOneTurnInitialWindowRequiresLargerMobileDefault`
+  decodes a scrubbed structural capture of the live local
+  `thread_render_frame` shape. With `initial_user_turns=1`, the first frames had
+  one visible `user_turn` row and `window.has_more_above=true`, which means the
+  top boundary is immediately materialized.
+- `GaryxConversationScrollStateTests.testCapturedSmallWindowDoesNotAutoPrefetchAfterLightScroll`
+  shows that a barely scrollable one-turn window can pass the current automatic
+  prefetch gate after a small user scroll.
+- `GaryxConversationScrollStateTests.testTopRowAppearanceStillHonorsDistanceFromLoadedHistoryStart`
+  shows that top-row `onAppear` currently bypasses the distance gate with
+  `ignoreDistance=true`.
+- `GaryxHistoryPaginationPlannerTests` intentionally fails to compile because
+  stable history pagination is still app-local. The missing Core seam is the bug
+  boundary for `hasMoreHistoryBefore` flicker and cache-hit idempotence.
+- `GaryxConversationScrollStateTests.testRenderRowPrependPreservesScrollWhenMessagesDidNotChange`
+  drives two server render snapshots through `GaryxMobileRenderStateMapper`.
+  The transcript cache rows are unchanged, but the render row ids prepend when
+  the floor drops, proving `onChange(messages)` alone cannot observe the
+  visible insertion.
+- Existing window/prefetch tests have been updated to the same intended
+  contract, so the suite no longer has contradictory old expectations for
+  `initial_user_turns=1` or `ignoreDistance=true`.
+
+No raw local thread id, message text, personal path, token, or user data is
+committed. The capture in tests keeps only public-safe frame structure.
+
+## Design
+
+### 1. Make the cold window large enough
+
+Change `GaryxThreadWindowPlanner.initialUserTurns` from `1` to `3`.
+
+Reasoning:
+
+- The existing HTTP open path already uses `threadHistoryUserQueryLimit = 3`.
+- The current one-turn SSE cold window is the only intentionally tiny window.
+- This is the narrowest change because `initial_user_turns` is client-declared
+  in `GaryxThreadWindowPlanner`; the gateway already accepts the parameter.
+- Floor/gapless semantics are unchanged. The client asks for a larger initial
+  row window, then reconnects with the server-provided floor exactly as before.
+- The existing
+  `GaryxTranscriptSyncPlannerTests.testThreadWindowPlannerColdReconnectScrollUpSequence`
+  assertion changes from `1` to `3`; gateway tests that explicitly pass
+  `initial_user_turns=1` as a fixture are not changed.
+
+### 2. Stop treating top-row appearance as permission to fetch
+
+Keep top-row `onAppear` as a hint, but route it through the same scroll-distance
+policy as metrics changes. Remove the `ignoreDistance` parameter from automatic
+prefetch entirely:
+
+- `GaryxLoadEarlierHistoryButton.onAppear`
+- `GaryxMobileTurnRowsView.onNearHistoryBoundary`
+
+Tighten `GaryxConversationScrollState.shouldPrefetchOlderHistory` so automatic
+prefetch has this exact signature and requires every gate below:
+
+```swift
+shouldPrefetchOlderHistory(
+    hasMoreHistoryBefore: Bool,
+    isLoadingOlderHistory: Bool,
+    hasPendingPrefetch: Bool
+) -> Bool
+```
+
+- `hasMoreHistoryBefore`
+- not already loading
+- no pending prefetch
+- a real user move toward older history
+- measured scrollable content larger than a tiny cold window: compute
+  `contentHeight = contentBottomOffset - contentTopOffset` and require
+  `contentHeight - viewportHeight >= max(640, viewportHeight)`
+- proximity to the loaded history start
+  (`contentTopOffset >= -max(640, viewportHeight * 1.5)`)
+
+The button tap remains the explicit manual path and can call
+`loadOlderSelectedThreadHistory()` directly.
+
+Expected policy examples with an 800pt viewport:
+
+- `contentTopOffset=-80`, `contentBottomOffset=980` is rejected even though it
+  is near the loaded start, because overflow is only 260pt.
+- `contentTopOffset=-2000`, `contentBottomOffset=2600` is rejected because it is
+  not near the loaded start.
+- `contentTopOffset=-400`, `contentBottomOffset=2600` is accepted after a real
+  user move, because overflow is 2200pt and the loaded start is within the
+  1200pt prefetch band.
+
+This intentionally updates the old
+`testHistoryPrefetchRequiresMovementAndProximity` case that asserted
+`top=-2000` could still prefetch through `ignoreDistance=true`. That assertion
+encoded the deleted bypass behavior and must become false under the new
+contract. The positive automatic prefetch example is now the `-400/2600/800`
+case above.
+
+### 3. Move history pagination truth into Core
+
+Add a Core planner:
+
+- `GaryxHistoryPaginationState`
+- `GaryxHistoryPaginationPage`
+- `GaryxHistoryPaginationPlanner`
+
+It has two inputs:
+
+- HTTP/cache pages, which are authoritative for the cached older boundary.
+- render-window snapshots, which can seed or lower the render floor but must not
+  clear a cached older boundary just because one transient frame says
+  `has_more_above=false`.
+
+Rules:
+
+- A render window with `has_more_above=true` and `floor_seq > 1` yields
+  `hasMoreBefore=true`, `nextBeforeIndex=floor_seq - 1`.
+- A render window with `has_more_above=false` clears pagination only when the
+  cached page state is present and also says there is no older boundary. Missing
+  cache truth preserves the current boundary instead of treating "unknown" as
+  "no older page".
+- The clear path is tested: when both render window and cached state say no
+  older boundary, `hasMoreBefore=false` and `nextBeforeIndex=nil`, so the
+  "Load earlier" affordance disappears at the true top.
+- A cached older page with the same `nextBeforeIndex` is idempotent, so
+  automatic triggers do not repeatedly request the same page after a cache hit
+  or duplicate server response.
+
+Wire this planner into:
+
+- `applyRenderWindowPagination`
+- `updateSelectedThreadHistoryPagination`
+
+The planner owns only `hasMoreBefore` and `nextBeforeIndex`. It does not own
+`selectedThreadRenderFloorByThread`, `render_floor` requests, event cursors, or
+`based_on_seq`; those remain in the existing stream/history code paths.
+
+### 4. Preserve scroll for render-row prepends, not only message prepends
+
+Older-page flow currently prepends bodies, lowers `render_floor`, then restarts
+SSE. The visible row insertion may happen on the later render snapshot, while
+`onChange(of: model.messages)` already fired. Add a view-level row-id change
+hook backed by a Core method:
+
+- track `selectedThreadTurnRows().map(\.id)`
+- call
+  `GaryxConversationScrollState.renderRowsChanged(previousIds:currentIds:threadUnchanged:hasTailContent:)`
+- the Core method uses the existing prepend detector and feeds
+  `contentChanged(isHistoryPrepend: true)` when prior rows moved down
+
+This does not recompute grouping. It only compares server row ids already
+produced by `GaryxMobileRenderStateMapper`.
+
+## Files
+
+- `Sources/GaryxMobileCore/GaryxTranscriptSyncPlanner.swift`
+- `Sources/GaryxMobileCore/GaryxConversationScrollPolicy.swift`
+- `App/GaryxMobile/GaryxMobileConversationViews.swift`
+- `App/GaryxMobile/GaryxMobileModel+ThreadStream.swift`
+- `App/GaryxMobile/GaryxMobileModel+Threads.swift`
+- focused tests under `Tests/GaryxMobileCoreTests`
+
+No new app-target source file is planned. If implementation needs one, run
+`xcodegen generate` and commit the project change.
+
+## Validation
+
+Required before code review:
+
+- focused red tests turn green
+- `swift test --package-path mobile/garyx-mobile`
+- `xcodebuild -project mobile/garyx-mobile/GaryxMobile.xcodeproj -target GaryxMobile -sdk iphonesimulator -configuration Debug build CODE_SIGNING_ALLOWED=NO`
+
+The code-review task must reproduce before/after behavior and confirm:
+
+- no automatic load on cold open or light scroll
+- stable `hasMoreHistoryBefore`
+- idempotent cache/page handling
+- prepend keeps reading position stable
+- floor/gapless contract remains intact

mobile/garyx-mobile/App/GaryxMobile/GaryxMobileConversationViews.swift

@@ -155,6 +155,7 @@ struct GaryxConversationView: View {
     @State private var scrollStateBox = GaryxConversationScrollStateBox()
     @State private var showsScrollToBottomButton = false
     @State private var scrollPreservationThreadId: String?
+    @State private var rowScrollPreservationThreadId: String?
     @State private var pendingHistoryPrefetchThreadId: String?
     @State private var bottomChromeHeight: CGFloat = 0
     @State private var tailScrollRequestGeneration = 0
@@ -236,6 +237,7 @@ struct GaryxConversationView: View {
                 }
                 .onChange(of: model.selectedThread?.id) { _, _ in
                     scrollPreservationThreadId = model.selectedThread?.id
+                    rowScrollPreservationThreadId = model.selectedThread?.id
                     pendingHistoryPrefetchThreadId = nil
                     updateScrollState(proxy: proxy) { $0.threadOpened() }
                     resetTailThinkingPresentation(proxy: proxy)
@@ -259,6 +261,18 @@ struct GaryxConversationView: View {
                         )
                     }
                 }
+                .onChange(of: model.selectedThreadTurnRows().map(\.id)) { oldValue, newValue in
+                    let threadUnchanged = model.selectedThread?.id == rowScrollPreservationThreadId
+                    rowScrollPreservationThreadId = model.selectedThread?.id
+                    updateScrollState(proxy: proxy) {
+                        $0.renderRowsChanged(
+                            previousIds: oldValue,
+                            currentIds: newValue,
+                            threadUnchanged: threadUnchanged,
+                            hasTailContent: !newValue.isEmpty || showsDebouncedTailThinking
+                        )
+                    }
+                }
                 .onChange(of: model.showsTailThinkingIndicator) { _, _ in
                     syncTailThinkingPresentation(proxy: proxy)
                 }
@@ -333,14 +347,14 @@ struct GaryxConversationView: View {
                             }
                         }
                         .onAppear {
-                            prefetchOlderHistoryIfNeeded(ignoreDistance: true)
+                            prefetchOlderHistoryIfNeeded()
                         }
                     }
                     GaryxMobileTurnRowsView(
                         rows: turnRows,
                         prefetchBoundaryRowCount: garyxHistoryPrefetchBoundaryRows
                     ) {
-                        prefetchOlderHistoryIfNeeded(ignoreDistance: true)
+                        prefetchOlderHistoryIfNeeded()
                     }
                     if showsDebouncedTailThinking {
                         GaryxThinkingLabel()
@@ -617,13 +631,12 @@ struct GaryxConversationView: View {
         ].joined(separator: "|")
     }
 
-    private func prefetchOlderHistoryIfNeeded(ignoreDistance: Bool = false) {
+    private func prefetchOlderHistoryIfNeeded() {
         guard let threadId = model.selectedThread?.id,
               scrollStateBox.state.shouldPrefetchOlderHistory(
                 hasMoreHistoryBefore: model.selectedThreadHasMoreHistoryBefore,
                 isLoadingOlderHistory: model.isLoadingOlderThreadHistory,
-                hasPendingPrefetch: pendingHistoryPrefetchThreadId == threadId,
-                ignoreDistance: ignoreDistance
+                hasPendingPrefetch: pendingHistoryPrefetchThreadId == threadId
               ) else {
             return
         }

mobile/garyx-mobile/App/GaryxMobile/GaryxMobileModel+ThreadStream.swift

@@ -236,15 +236,24 @@ extension GaryxMobileModel {
     private func applyThreadRenderSnapshot(_ snapshot: GaryxRenderSnapshot, threadId: String) {
         guard selectedThread?.id == threadId else { return }
         setRenderSnapshot(snapshot, for: threadId)
-        applyRenderWindowPagination(snapshot.window, threadId: threadId)
+        let pagination = applyRenderWindowPagination(snapshot.window, threadId: threadId)
         let base = transcriptSnapshot(for: threadId)
+        let windowHasMoreBefore: Bool
+        let windowNextBeforeIndex: Int?
+        if let pagination {
+            windowHasMoreBefore = pagination.hasMoreBefore
+            windowNextBeforeIndex = pagination.nextBeforeIndex
+        } else {
+            windowHasMoreBefore = base?.hasMoreBefore ?? false
+            windowNextBeforeIndex = base?.nextBeforeIndex
+        }
         let window = GaryxCachedTranscript(
             threadId: threadId,
             savedAt: Date(),
             messages: base?.messages ?? [],
             renderSnapshot: snapshot,
-            hasMoreBefore: base?.hasMoreBefore ?? false,
-            nextBeforeIndex: base?.nextBeforeIndex
+            hasMoreBefore: windowHasMoreBefore,
+            nextBeforeIndex: windowNextBeforeIndex
         )
         cachedTranscriptSnapshots[threadId] = window
         if !isThreadBusy(threadId) {
@@ -254,20 +263,24 @@ extension GaryxMobileModel {
         scheduleSelectedThreadStreamFlush(for: threadId)
     }
 
-    private func applyRenderWindowPagination(_ renderWindow: GaryxRenderWindow?, threadId: String) {
-        guard selectedThread?.id == threadId else { return }
+    @discardableResult
+    private func applyRenderWindowPagination(
+        _ renderWindow: GaryxRenderWindow?,
+        threadId: String
+    ) -> GaryxHistoryPaginationState? {
+        guard selectedThread?.id == threadId else { return nil }
         guard let renderWindow else {
             selectedThreadRenderFloorByThread[threadId] = nil
-            return
+            return nil
         }
         selectedThreadRenderFloorByThread[threadId] = renderWindow.floorSeq
-        if renderWindow.hasMoreAbove, renderWindow.floorSeq > 1 {
-            selectedThreadHasMoreHistoryBefore = true
-            selectedThreadNextHistoryBeforeIndex = renderWindow.floorSeq - 1
-        } else {
-            selectedThreadHasMoreHistoryBefore = false
-            selectedThreadNextHistoryBeforeIndex = nil
-        }
+        let next = GaryxHistoryPaginationPlanner.applyingRenderWindow(
+            renderWindow,
+            current: selectedHistoryPaginationState(),
+            cached: cachedHistoryPaginationState(for: threadId)
+        )
+        applySelectedThreadHistoryPagination(next)
+        return next
     }
 
     /// Leading-throttle (mirrors scheduleAssistantDeltaFlush): the first row schedules

mobile/garyx-mobile/App/GaryxMobile/GaryxMobileModel+Threads.swift

@@ -1516,21 +1516,44 @@ extension GaryxMobileModel {
         preservingLoadedOlderPages: Bool = false
     ) {
         guard selectedThread?.id == threadId else { return }
-        if preservingLoadedOlderPages,
-           let oldestLoadedIndex = oldestLoadedHistoryIndex(for: threadId),
-           let latestPageStartIndex = preserveRemoteBeforeIndex(from: transcript),
-           oldestLoadedIndex < latestPageStartIndex {
-            if oldestLoadedIndex > 0 {
-                selectedThreadHasMoreHistoryBefore = true
-                selectedThreadNextHistoryBeforeIndex = oldestLoadedIndex
-            } else {
-                selectedThreadHasMoreHistoryBefore = false
-                selectedThreadNextHistoryBeforeIndex = nil
-            }
-            return
+        let page = GaryxHistoryPaginationPage(
+            hasMoreBefore: transcript.pageInfo?.hasMoreBefore ?? false,
+            nextBeforeIndex: transcript.pageInfo?.nextBeforeIndex,
+            oldestLoadedIndex: oldestLoadedHistoryIndex(for: threadId),
+            latestPageStartIndex: preserveRemoteBeforeIndex(from: transcript)
+        )
+        let next = GaryxHistoryPaginationPlanner.applyingTranscriptPage(
+            page,
+            current: selectedHistoryPaginationState(),
+            preservingLoadedOlderPages: preservingLoadedOlderPages
+        )
+        applySelectedThreadHistoryPagination(next)
+    }
+
+    func selectedHistoryPaginationState() -> GaryxHistoryPaginationState {
+        GaryxHistoryPaginationState(
+            hasMoreBefore: selectedThreadHasMoreHistoryBefore,
+            nextBeforeIndex: selectedThreadNextHistoryBeforeIndex
+        )
+    }
+
+    func cachedHistoryPaginationState(for threadId: String) -> GaryxHistoryPaginationState? {
+        guard let snapshot = transcriptSnapshot(for: threadId) else {
+            return nil
+        }
+        return GaryxHistoryPaginationState(
+            hasMoreBefore: snapshot.hasMoreBefore,
+            nextBeforeIndex: snapshot.nextBeforeIndex
+        )
+    }
+
+    func applySelectedThreadHistoryPagination(_ state: GaryxHistoryPaginationState) {
+        if selectedThreadHasMoreHistoryBefore != state.hasMoreBefore {
+            selectedThreadHasMoreHistoryBefore = state.hasMoreBefore
+        }
+        if selectedThreadNextHistoryBeforeIndex != state.nextBeforeIndex {
+            selectedThreadNextHistoryBeforeIndex = state.nextBeforeIndex
         }
-        selectedThreadHasMoreHistoryBefore = transcript.pageInfo?.hasMoreBefore ?? false
-        selectedThreadNextHistoryBeforeIndex = transcript.pageInfo?.nextBeforeIndex
     }
 
     func oldestLoadedHistoryIndex(for threadId: String) -> Int? {