fix: harden lazy-load reset and dedup against window reshape (PR review)

olafura · claude · olafura · commit 75d25e266834 · 2026-06-22T22:46:54.000+02:00
Address the follow-up review on the reconnect/revert fix (web + mobile):

- Stale in-flight load after reset: the reset cleared older pages but an in-flight
  load only checked the thread key (unchanged on a same-thread reshape), so its
  late result repopulated cleared state. Add a generation counter that bumps on
  every reset; the load captures it and drops its result if it changed.
- Reset trigger now also fires when the live window SHRINKS (checkpoint revert
  that removes rows without changing the oldest row), not only when the oldest id
  changes (reconnect) — a pure append (same oldest, larger count) still keeps the
  loaded pages. Fixes the revert cases on web and mobile.
- Empty page no longer churns: when dedup leaves nothing new, return without a new
  array reference and stop, so an unadvanced cursor can't loop identical requests.
- Dedup against the full merged set (older + live) on both platforms.

Also add a server regression test proving the "unsequenced cursor hides sequenced
history" concern is a non-issue: NULL-sequence rows always sort oldest, so when
the oldest loaded row is unsequenced every sequenced row is already in the window
— the `sequence IS NULL` cursor reaches all older rows without stranding any.

Co-Authored-By: Claude Opus 4.8 &lt;noreply@anthropic.com&gt;
diff --git a/apps/mobile/src/state/use-thread-composer-state.ts b/apps/mobile/src/state/use-thread-composer-state.ts
@@ -113,21 +113,46 @@ export function useThreadComposerState() {
   const activityRequestKey = selectedThreadShell
     ? `${selectedThreadShell.environmentId}\u0000${selectedThreadShell.id}`
     : null;
-  const activityRequestKeyRef = useRef(activityRequestKey);
-  activityRequestKeyRef.current = activityRequestKey;
-
   const liveActivities = selectedThreadDetail?.activities ?? EMPTY_ACTIVITIES;
-  // The live window's oldest activity is stable while new activities only
-  // append; it changes when the window is re-snapshotted (reconnect) or rows are
-  // removed (checkpoint revert), either of which can make prepended older pages
-  // stale or gappy — so reset the lazy-load state when it (or the thread) changes.
   const liveOldestActivityId = liveActivities[0]?.id ?? null;
+  const liveActivityCount = liveActivities.length;
+  // Bumps on every lazy-load reset so a late in-flight load can't repopulate the
+  // freshly-cleared state (the thread key alone doesn't change on a same-thread
+  // window reshape).
+  const olderActivitiesGenRef = useRef(0);
+  // The request key of an in-flight older-history load — coalesces the duplicate
+  // dispatches the list fires before the loading state updates.
+  const inFlightOlderKeyRef = useRef<string | null>(null);
+  // Reset the lazy-loaded older pages when the live window is *reshaped* rather
+  // than purely appended-to: a different thread or a re-snapshot (reconnect)
+  // changes its oldest row, and a checkpoint revert removes rows so the count
+  // shrinks. A pure append (same thread, same oldest, larger count) keeps them.
+  const olderWindowRef = useRef({
+    key: activityRequestKey,
+    oldest: liveOldestActivityId,
+    count: liveActivityCount,
+  });
   useEffect(() => {
+    const prev = olderWindowRef.current;
+    olderWindowRef.current = {
+      key: activityRequestKey,
+      oldest: liveOldestActivityId,
+      count: liveActivityCount,
+    };
+    const reshaped =
+      activityRequestKey !== prev.key ||
+      liveOldestActivityId !== prev.oldest ||
+      liveActivityCount < prev.count;
+    if (!reshaped) {
+      return;
+    }
+    olderActivitiesGenRef.current += 1;
+    inFlightOlderKeyRef.current = null;
     setOlderActivities([]);
     setOlderLoaded(false);
     setOlderHasMore(false);
     setLoadingOlderActivities(false);
-  }, [activityRequestKey, liveOldestActivityId]);
+  }, [activityRequestKey, liveOldestActivityId, liveActivityCount]);
   const mergedActivities = useMemo(
     () =>
       olderActivities.length > 0 ? [...olderActivities, ...liveActivities] : liveActivities,
@@ -138,10 +163,6 @@ export function useThreadComposerState() {
     ? olderHasMore
     : (selectedThreadDetail?.hasMoreActivities ?? false);
 
-  // Synchronous in-flight guard keyed by thread: the list fires onLoadOlder
-  // repeatedly while pinned at the top, but loading *state* only updates next
-  // render, so without this a fast scroll dispatches duplicate same-cursor calls.
-  const inFlightOlderKeyRef = useRef<string | null>(null);
   const onLoadOlderActivities = useCallback(() => {
     if (!selectedThreadShell || !hasMoreOlderActivities) {
       return;
@@ -158,39 +179,42 @@ export function useThreadComposerState() {
         ? { beforeSequence: oldestActivity.sequence }
         : { beforeCreatedAt: oldestActivity.createdAt, beforeActivityId: oldestActivity.id };
     const requestKey = activityRequestKey;
+    const gen = olderActivitiesGenRef.current;
     inFlightOlderKeyRef.current = requestKey;
     setLoadingOlderActivities(true);
     void loadThreadActivities({
       environmentId: selectedThreadShell.environmentId,
       input: { threadId: selectedThreadShell.id, ...cursorInput },
     })
       .then((result) => {
-        if (activityRequestKeyRef.current !== requestKey) {
+        // Window/thread reset while in flight — drop the page so it can't
+        // repopulate state cleared by the reset.
+        if (olderActivitiesGenRef.current !== gen) {
           return;
         }
         if (result._tag !== "Success") {
           return;
         }
         const page = result.value;
-        setOlderActivities((prev) => {
-          // Dedup against both already-loaded older pages and the live window,
-          // since mobile merges everything into one array (duplicate ids would
-          // produce duplicate React keys in the feed).
-          const seen = new Set(prev.map((activity) => activity.id));
-          for (const activity of liveActivities) {
-            seen.add(activity.id);
-          }
-          const fresh = page.activities.filter((activity) => !seen.has(activity.id));
-          return [...fresh, ...prev];
-        });
+        // Dedup against everything already loaded (older pages + live window).
+        // Loads are serialized by the in-flight ref, so mergedActivities is a
+        // current snapshot here.
+        const seen = new Set(mergedActivities.map((activity) => activity.id));
+        const fresh = page.activities.filter((activity) => !seen.has(activity.id));
+        if (fresh.length === 0) {
+          // Nothing new — stop rather than re-dispatch the same (unadvanced)
+          // cursor in a loop, and don't churn the array reference.
+          setOlderLoaded(true);
+          setOlderHasMore(false);
+          return;
+        }
+        setOlderActivities((prev) => [...fresh, ...prev]);
         setOlderLoaded(true);
         setOlderHasMore(page.hasMore);
       })
       .finally(() => {
-        if (inFlightOlderKeyRef.current === requestKey) {
+        if (olderActivitiesGenRef.current === gen) {
           inFlightOlderKeyRef.current = null;
-        }
-        if (activityRequestKeyRef.current === requestKey) {
           setLoadingOlderActivities(false);
         }
       });
@@ -199,7 +223,6 @@ export function useThreadComposerState() {
     hasMoreOlderActivities,
     mergedActivities,
     activityRequestKey,
-    liveActivities,
     loadThreadActivities,
   ]);
 
diff --git a/apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.test.ts b/apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.test.ts
@@ -1309,6 +1309,103 @@ projectionSnapshotLayer("ProjectionSnapshotQuery", (it) => {
       }),
   );
 
+  it.effect(
+    "unsequenced cursor reaches all older rows without stranding sequenced ones",
+    () =>
+      // Regression for the "unsequenced cursor hides sequenced history" concern:
+      // sequenced rows always sort newer than NULL-sequence (legacy) rows, so when
+      // the oldest loaded row is unsequenced every sequenced row is already in the
+      // window — the `sequence IS NULL` cursor can't strand sequenced rows.
+      Effect.gen(function* () {
+        const snapshotQuery = yield* ProjectionSnapshotQuery;
+        const sql = yield* SqlClient.SqlClient;
+        yield* sql`DELETE FROM projection_projects`;
+        yield* sql`DELETE FROM projection_threads`;
+        yield* sql`DELETE FROM projection_thread_activities`;
+        yield* sql`DELETE FROM projection_state`;
+        yield* sql`
+          INSERT INTO projection_projects (
+            project_id, title, workspace_root, default_model_selection_json,
+            scripts_json, created_at, updated_at, deleted_at
+          ) VALUES (
+            'project-1', 'Project 1', '/tmp/project-1',
+            '{"provider":"codex","model":"gpt-5-codex"}', '[]',
+            '2026-04-01T00:00:00.000Z', '2026-04-01T00:00:01.000Z', NULL
+          )
+        `;
+        yield* sql`
+          INSERT INTO projection_threads (
+            thread_id, project_id, title, model_selection_json, runtime_mode,
+            interaction_mode, branch, worktree_path, latest_turn_id,
+            latest_user_message_at, pending_approval_count, pending_user_input_count,
+            has_actionable_proposed_plan, created_at, updated_at, archived_at, deleted_at
+          ) VALUES (
+            'thread-1', 'project-1', 'Thread 1',
+            '{"provider":"codex","model":"gpt-5-codex"}', 'full-access', 'default',
+            NULL, NULL, NULL, NULL, 0, 0, 0,
+            '2026-04-01T00:00:02.000Z', '2026-04-01T00:00:03.000Z', NULL, NULL
+          )
+        `;
+        // 600 legacy unsequenced rows (older) + 3 sequenced rows (newer). The
+        // window keeps the 3 sequenced + the most-recent 497 unsequenced, so the
+        // oldest loaded row is unsequenced and 103 older unsequenced remain.
+        yield* Effect.forEach(
+          Array.from({ length: 600 }, (_u, index) => index + 1),
+          (n) =>
+            sql`
+              INSERT INTO projection_thread_activities (
+                activity_id, thread_id, turn_id, tone, kind, summary, payload_json,
+                sequence, created_at
+              ) VALUES (
+                ${`unseq-${String(n).padStart(4, "0")}`}, 'thread-1', NULL,
+                'info', 'runtime.note', ${`unseq-${n}`}, '{}', NULL,
+                ${`2026-04-01T00:00:01.${String(n).padStart(3, "0")}Z`}
+              )
+            `,
+          { discard: true },
+        );
+        yield* Effect.forEach(
+          [1, 2, 3],
+          (seq) =>
+            sql`
+              INSERT INTO projection_thread_activities (
+                activity_id, thread_id, turn_id, tone, kind, summary, payload_json,
+                sequence, created_at
+              ) VALUES (
+                ${`seq-${seq}`}, 'thread-1', NULL, 'info', 'runtime.note',
+                ${`seq-${seq}`}, '{}', ${seq}, ${`2026-04-01T09:00:0${seq}.000Z`}
+              )
+            `,
+          { discard: true },
+        );
+
+        const detail = yield* snapshotQuery.getThreadDetailById(ThreadId.make("thread-1"));
+        assert.equal(detail._tag, "Some");
+        if (detail._tag !== "Some") return;
+        const windowed = detail.value.activities;
+        assert.equal(windowed.length, 500);
+        // Sequenced rows are the newest (end of the ascending window); the oldest
+        // loaded row is unsequenced — exactly the case the concern is about.
+        assert.equal(windowed.at(-1)?.summary, "seq-3");
+        assert.equal(windowed[0]?.sequence, undefined);
+
+        // The client pages with the unsequenced cursor of the oldest loaded row.
+        const oldest = windowed[0];
+        assert.ok(oldest);
+        const olderPage = yield* snapshotQuery.getThreadActivitiesPage({
+          threadId: ThreadId.make("thread-1"),
+          beforeCreatedAt: oldest.createdAt,
+          beforeActivityId: oldest.id,
+          limit: 500,
+        });
+        // The 103 older unsequenced rows come back, none are sequenced, and no
+        // sequenced row was stranded (all 3 are already in the window).
+        assert.equal(olderPage.activities.length, 103);
+        assert.equal(olderPage.hasMore, false);
+        assert.ok(olderPage.activities.every((a) => a.sequence === undefined));
+      }),
+  );
+
   it.effect("uses projection_threads.latest_turn_id for bulk command and shell snapshots", () =>
     Effect.gen(function* () {
       const snapshotQuery = yield* ProjectionSnapshotQuery;
diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx
@@ -1702,20 +1702,46 @@ function ChatViewContent(props: ChatViewProps) {
   const activeThreadActivityRequestKey = activeThread
     ? `${activeThread.environmentId}\u0000${activeThread.id}`
     : null;
-  const activeThreadActivityRequestKeyRef = useRef(activeThreadActivityRequestKey);
-  activeThreadActivityRequestKeyRef.current = activeThreadActivityRequestKey;
   const liveThreadActivities = activeThread?.activities ?? EMPTY_ACTIVITIES;
-  // The live window's oldest activity is stable while new activities only
-  // append; it changes when the window is re-snapshotted (reconnect) or rows are
-  // removed (checkpoint revert), either of which can make prepended older pages
-  // stale or gappy — so reset the lazy-load state when it (or the thread) changes.
   const liveOldestActivityId = liveThreadActivities[0]?.id ?? null;
+  const liveActivityCount = liveThreadActivities.length;
+  // Bumps on every lazy-load reset so a late in-flight load can't repopulate the
+  // freshly-cleared state (the thread key alone doesn't change on a same-thread
+  // window reshape).
+  const olderActivitiesGenRef = useRef(0);
+  // The request key of an in-flight older-history load — coalesces the duplicate
+  // dispatches a fast scroll-to-top fires before the loading state updates.
+  const inFlightOlderKeyRef = useRef<string | null>(null);
+  // Reset the lazy-loaded older pages when the live window is *reshaped* rather
+  // than purely appended-to: a different thread or a re-snapshot (reconnect)
+  // changes its oldest row, and a checkpoint revert removes rows so the count
+  // shrinks. A pure append (same thread, same oldest, larger count) keeps them.
+  const olderWindowRef = useRef({
+    key: activeThreadActivityRequestKey,
+    oldest: liveOldestActivityId,
+    count: liveActivityCount,
+  });
   useEffect(() => {
+    const prev = olderWindowRef.current;
+    olderWindowRef.current = {
+      key: activeThreadActivityRequestKey,
+      oldest: liveOldestActivityId,
+      count: liveActivityCount,
+    };
+    const reshaped =
+      activeThreadActivityRequestKey !== prev.key ||
+      liveOldestActivityId !== prev.oldest ||
+      liveActivityCount < prev.count;
+    if (!reshaped) {
+      return;
+    }
+    olderActivitiesGenRef.current += 1;
+    inFlightOlderKeyRef.current = null;
     setOlderActivities([]);
     setOlderLoaded(false);
     setOlderHasMore(false);
     setLoadingOlderActivities(false);
-  }, [activeThreadActivityRequestKey, liveOldestActivityId]);
+  }, [activeThreadActivityRequestKey, liveOldestActivityId, liveActivityCount]);
 
   const threadActivities = useMemo(
     () =>
@@ -1729,11 +1755,6 @@ function ChatViewContent(props: ChatViewProps) {
   const hasMoreOlderActivities = olderLoaded
     ? olderHasMore
     : (activeThread?.hasMoreActivities ?? false);
-  // Tracks the request key of an in-flight older-history load. The scroll
-  // handler fires onLoadOlder on every frame while at the top, but the loading
-  // *state* only updates on the next render — without a synchronous guard a fast
-  // scroll-to-top dispatches several duplicate requests for the same cursor.
-  const inFlightOlderKeyRef = useRef<string | null>(null);
   const loadOlderActivities = useCallback(() => {
     if (!activeThread || !hasMoreOlderActivities) {
       return;
@@ -1750,39 +1771,42 @@ function ChatViewContent(props: ChatViewProps) {
         ? { beforeSequence: oldestActivity.sequence }
         : { beforeCreatedAt: oldestActivity.createdAt, beforeActivityId: oldestActivity.id };
     const requestKey = activeThreadActivityRequestKey;
+    const gen = olderActivitiesGenRef.current;
     inFlightOlderKeyRef.current = requestKey;
     setLoadingOlderActivities(true);
     void loadThreadActivities({
       environmentId: activeThread.environmentId,
       input: { threadId: activeThread.id, ...cursorInput },
     })
       .then((result) => {
-        if (activeThreadActivityRequestKeyRef.current !== requestKey) {
+        // The window/thread was reset while this was in flight — drop the page
+        // so it can't repopulate state cleared by the reset.
+        if (olderActivitiesGenRef.current !== gen) {
           return;
         }
         if (result._tag !== "Success") {
           return;
         }
         const page = result.value;
-        setOlderActivities((prev) => {
-          // Dedup against both already-loaded older pages and the live window so
-          // an overlap at the window boundary can't leave duplicate ids (which
-          // would break timeline keys and work-log derivation).
-          const seen = new Set(prev.map((activity) => activity.id));
-          for (const activity of liveThreadActivities) {
-            seen.add(activity.id);
-          }
-          const fresh = page.activities.filter((activity) => !seen.has(activity.id));
-          return [...fresh, ...prev];
-        });
+        // Dedup against everything already loaded (older pages + live window) so a
+        // boundary overlap can't leave duplicate ids. Loads are serialized by the
+        // in-flight ref, so `threadActivities` is a current snapshot here.
+        const seen = new Set(threadActivities.map((activity) => activity.id));
+        const fresh = page.activities.filter((activity) => !seen.has(activity.id));
+        if (fresh.length === 0) {
+          // The page delivered nothing new; stop rather than re-dispatch the same
+          // (unadvanced) cursor in a loop, and don't churn the array reference.
+          setOlderLoaded(true);
+          setOlderHasMore(false);
+          return;
+        }
+        setOlderActivities((prev) => [...fresh, ...prev]);
         setOlderLoaded(true);
         setOlderHasMore(page.hasMore);
       })
       .finally(() => {
-        if (inFlightOlderKeyRef.current === requestKey) {
+        if (olderActivitiesGenRef.current === gen) {
           inFlightOlderKeyRef.current = null;
-        }
-        if (activeThreadActivityRequestKeyRef.current === requestKey) {
           setLoadingOlderActivities(false);
         }
       });
@@ -1791,7 +1815,6 @@ function ChatViewContent(props: ChatViewProps) {
     activeThreadActivityRequestKey,
     hasMoreOlderActivities,
     threadActivities,
-    liveThreadActivities,
     loadThreadActivities,
   ]);
 
