[codex] Upgrade Legend List chat scrolling#3545
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
vouch:trusted
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| extraData={listAppearanceData} | ||
| renderItem={renderItem} | ||
| keyExtractor={(entry) => `${entry.type}:${entry.id}`} | ||
| keyExtractor={(entry) => entry.id} |
There was a problem hiding this comment.
🟡 Medium threads/ThreadFeed.tsx:1451
keyExtractor returns entry.id directly, so queued messages and their delivered copies share the same list key. When both entries exist in props.feed during the send handoff, KeyboardAwareLegendList receives duplicate keys, causing it to reuse the wrong row or drop one message. Consider including the entry type in the key (e.g., \${entry.type}:${entry.id}``) to ensure uniqueness.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/threads/ThreadFeed.tsx around line 1451:
`keyExtractor` returns `entry.id` directly, so queued messages and their delivered copies share the same list key. When both entries exist in `props.feed` during the send handoff, `KeyboardAwareLegendList` receives duplicate keys, causing it to reuse the wrong row or drop one message. Consider including the entry type in the key (e.g., `\`\${entry.type}:\${entry.id}\``) to ensure uniqueness.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Duplicate keys during outbox promotion
- Added deduplication in
buildThreadFeedto filter out queued messages whosemessageIdalready exists in confirmedloadedMessages, preventing two list items from sharing the same key during the promotion window.
- Added deduplication in
- ✅ Fixed: Composer inset never measured
- Converted
composerOverlayReffrom a plainuseRefto a state-based callback ref (useState+ setter as ref), making theuseLayoutEffectdepend on the element itself so theResizeObserverre-attaches whenever the overlay mounts after the initial empty-thread render.
- Converted
Or push these changes by commenting:
@cursor push 4dbf9bfa67
Preview (4dbf9bfa67)
diff --git a/apps/mobile/src/lib/threadActivity.ts b/apps/mobile/src/lib/threadActivity.ts
--- a/apps/mobile/src/lib/threadActivity.ts
+++ b/apps/mobile/src/lib/threadActivity.ts
@@ -1265,6 +1265,10 @@
const oldestLoadedMessageCreatedAt =
options?.loadedMessages !== undefined ? (loadedMessages[0]?.createdAt ?? null) : null;
const workLogEntries = deriveWorkLogEntries(thread.activities);
+ const confirmedMessageIds =
+ queuedMessages.length > 0
+ ? new Set(loadedMessages.map((message) => message.id))
+ : (undefined as never);
const entries = Arr.sortWith(
[
...loadedMessages.map<RawThreadFeedEntry>((message) => ({
@@ -1273,13 +1277,15 @@
createdAt: message.createdAt,
message,
})),
- ...queuedMessages.map<RawThreadFeedEntry>((queuedMessage) => ({
- type: "queued-message",
- id: queuedMessage.messageId,
- createdAt: queuedMessage.createdAt,
- queuedMessage,
- sending: queuedMessage.messageId === dispatchingQueuedMessageId,
- })),
+ ...queuedMessages
+ .filter((qm) => !confirmedMessageIds.has(qm.messageId))
+ .map<RawThreadFeedEntry>((queuedMessage) => ({
+ type: "queued-message",
+ id: queuedMessage.messageId,
+ createdAt: queuedMessage.createdAt,
+ queuedMessage,
+ sending: queuedMessage.messageId === dispatchingQueuedMessageId,
+ })),
...workLogEntries
.filter((entry) => {
if (options?.loadedMessages === undefined) {
diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx
--- a/apps/web/src/components/ChatView.tsx
+++ b/apps/web/src/components/ChatView.tsx
@@ -1148,7 +1148,7 @@
LastInvokedScriptByProjectSchema,
);
const legendListRef = useRef<LegendListRef | null>(null);
- const composerOverlayRef = useRef<HTMLDivElement | null>(null);
+ const [composerOverlayEl, setComposerOverlayEl] = useState<HTMLDivElement | null>(null);
const [composerOverlayHeight, setComposerOverlayHeight] = useState(0);
const isAtEndRef = useRef(true);
const attachmentPreviewHandoffByMessageIdRef = useRef<Record<string, string[]>>({});
@@ -1157,11 +1157,10 @@
const terminalUiOpenByThreadRef = useRef<Record<string, boolean>>({});
useLayoutEffect(() => {
- const composerOverlay = composerOverlayRef.current;
- if (!composerOverlay) return;
+ if (!composerOverlayEl) return;
const updateHeight = () => {
- const nextHeight = Math.ceil(composerOverlay.getBoundingClientRect().height);
+ const nextHeight = Math.ceil(composerOverlayEl.getBoundingClientRect().height);
setComposerOverlayHeight((currentHeight) =>
currentHeight === nextHeight ? currentHeight : nextHeight,
);
@@ -1171,9 +1170,9 @@
if (typeof ResizeObserver === "undefined") return;
const observer = new ResizeObserver(updateHeight);
- observer.observe(composerOverlay);
+ observer.observe(composerOverlayEl);
return () => observer.disconnect();
- }, []);
+ }, [composerOverlayEl]);
const terminalUiState = useTerminalUiStateStore((state) =>
selectThreadTerminalUiState(state.terminalUiStateByThreadKey, routeThreadRef),
@@ -4805,7 +4804,7 @@
{/* Input bar */}
<div
- ref={composerOverlayRef}
+ ref={setComposerOverlayEl}
data-chat-composer-overlay="true"
className={cn(
"pointer-events-none absolute inset-x-0 bottom-0 z-20 pl-[calc(env(safe-area-inset-left)+0.75rem)] pr-[calc(env(safe-area-inset-right)+0.75rem)] pt-1.5 sm:pl-[calc(env(safe-area-inset-left)+1.25rem)] sm:pr-[calc(env(safe-area-inset-right)+1.25rem)] sm:pt-2",You can send follow-ups to the cloud agent here.
ApprovabilityVerdict: Needs human review 3 blocking correctness issues found. This PR substantially refactors chat scrolling behavior across mobile and web platforms with new anchoring logic. Multiple unresolved review comments identify medium-severity bugs (duplicate keys, scroll yanking, blocked touches) that warrant human review before merging. You can customize Macroscope's approvability policy. Learn more. |
- Remove queued-message entries from thread feeds - Anchor scroll behavior to confirmed messages and active thread keys - Clean up composer overlay measurement and thread send state
github-actions
Bot
added
size:XL
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Scroll guard blocks retry
- Reset
lastScrolledAnchorMessageIdReftonullin the.catch()handler so that subsequent effect runs can retry the scroll whenscrollMessageToEndfails.
- Reset
- ✅ Fixed: Queue wait hides working pill
- Restored
queuedSendStartedAtfrom the first queued outbox message'screatedAtand passed it toderiveActiveWorkStartedAtinstead ofnull, restoring parity with the web client.
- Restored
Or push these changes by commenting:
@cursor push 670582d839
Preview (670582d839)
diff --git a/apps/mobile/src/features/threads/ThreadDetailScreen.tsx b/apps/mobile/src/features/threads/ThreadDetailScreen.tsx
--- a/apps/mobile/src/features/threads/ThreadDetailScreen.tsx
+++ b/apps/mobile/src/features/threads/ThreadDetailScreen.tsx
@@ -292,6 +292,7 @@
}
lastScrolledAnchorMessageIdRef.current = anchorMessageId;
void scrollMessageToEnd({ animated: true, closeKeyboard: false }).catch(() => {
+ lastScrolledAnchorMessageIdRef.current = null;
freeze.set(false);
});
});
diff --git a/apps/mobile/src/state/use-thread-composer-state.ts b/apps/mobile/src/state/use-thread-composer-state.ts
--- a/apps/mobile/src/state/use-thread-composer-state.ts
+++ b/apps/mobile/src/state/use-thread-composer-state.ts
@@ -115,6 +115,7 @@
};
}, [selectedThreadDetail, selectedThreadShell]);
+ const queuedSendStartedAt = selectedThreadQueuedMessages[0]?.createdAt ?? null;
const activeWorkStartedAt = useMemo(() => {
const selectedThread = selectedThreadDetail ?? selectedThreadShell;
if (!selectedThread) {
@@ -124,9 +125,14 @@
return deriveActiveWorkStartedAt(
selectedThread.latestTurn,
selectedThreadSessionActivity,
- null,
+ queuedSendStartedAt,
);
- }, [selectedThreadDetail, selectedThreadSessionActivity, selectedThreadShell]);
+ }, [
+ queuedSendStartedAt,
+ selectedThreadDetail,
+ selectedThreadSessionActivity,
+ selectedThreadShell,
+ ]);
const activeThreadBusy =
!!selectedThread &&You can send follow-ups to the cloud agent here.
- Clear stale anchor state when scroll-to-end fails - Remove end alignment from the mobile thread feed
There was a problem hiding this comment.
🟡 Medium
listAppearanceData omits unsettledTurnId, but renderFeedEntry uses it to compute showAssistantMeta for terminal assistant messages. When a turn completes and latestTurn changes without new feed items, LegendList caches visible rows; the assistant message stays stuck in its in-progress presentation (missing copy/timestamp controls) until the row recycles. Consider adding unsettledTurnId to extraData so the row re-renders when the turn state changes.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/threads/ThreadFeed.tsx around line 1173:
`listAppearanceData` omits `unsettledTurnId`, but `renderFeedEntry` uses it to compute `showAssistantMeta` for terminal assistant messages. When a turn completes and `latestTurn` changes without new feed items, `LegendList` caches visible rows; the assistant message stays stuck in its in-progress presentation (missing copy/timestamp controls) until the row recycles. Consider adding `unsettledTurnId` to `extraData` so the row re-renders when the turn state changes.
There was a problem hiding this comment.
🟡 Medium
handleSendMessage sets anchorMessageId to the returned queued messageId, and the effect at lines 278–305 keeps scrolling to that anchor until the message appears in selectedThreadFeed. Since props.onSendMessage only enqueues the message and returns immediately, if delivery is delayed (offline, slow connection), the user can scroll elsewhere, then get yanked back to the bottom minutes later when the queued message is finally confirmed. Consider clearing the anchor immediately when the send is only queued, or anchoring to a locally rendered optimistic message instead of persisting until remote confirmation.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/features/threads/ThreadDetailScreen.tsx around line 1:
`handleSendMessage` sets `anchorMessageId` to the returned queued `messageId`, and the effect at lines 278–305 keeps scrolling to that anchor until the message appears in `selectedThreadFeed`. Since `props.onSendMessage` only enqueues the message and returns immediately, if delivery is delayed (offline, slow connection), the user can scroll elsewhere, then get yanked back to the bottom minutes later when the queued message is finally confirmed. Consider clearing the anchor immediately when the send is only queued, or anchoring to a locally rendered optimistic message instead of persisting until remote confirmation.
- add configurable chat list anchor offset for composer chrome - restyle the web composer with shared glass/inset chrome - update mobile thread feed anchoring to match measured content
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issue. You can view the agent here.
Reviewed by Cursor Bugbot for commit 11e34a5. Configure here.
| /> | ||
| </div> | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
Lower chrome blocks timeline touches
Medium Severity
Bottom safe-area padding was moved from the pointer-events-none composer overlay root onto chat-composer-lower-chrome, which still uses default pointer events and a semi-transparent background. That full-width band sits above the message list and absorbs taps and scroll drags that previously reached the timeline.
Reviewed by Cursor Bugbot for commit 11e34a5. Configure here.
There was a problem hiding this comment.
Bugbot Autofix determined this is a false positive.
The lower-chrome div inherits pointer-events: none from its parent overlay element because CSS pointer-events is an inherited property, so it correctly passes through all touches to the timeline beneath.
You can send follow-ups to the cloud agent here.
1 participant
What
@legendapp/listfrom3.0.0-beta.44to stable3.2.0across web and mobile, including the keyboard-controller peer requirementcontentInsetEndAdjustmentKeyboardAwareLegendList,useKeyboardChatComposerInset, anduseKeyboardScrollToEndscrollToEndand empty-datainitialScrollAtEndfixes handle the first messageWhy
Legend List v3's floating-composer and AI-chat APIs replace several local scroll and composer-height workarounds. Using the library primitives keeps the scrollbar full-height, prevents the composer from covering the final message, follows streaming output only from the live edge, and avoids pulling users away from older messages when the mobile keyboard opens.
References:
Validation
vp checkvp run typecheckvp run lint:mobilevp testdiscovery: 521 suites / 4,013 tests passed; 15 unrelated desktop suites could not start because the local Electron binary is not installed correctlyNote
Upgrade chat list scrolling to use Legend List keyboard-aware anchored scrolling
ThreadFeedandMessagesTimelinewithKeyboardAwareLegendListand anchored end scrolling via a newresolveChatListAnchoredEndSpaceutility inpackages/shared.MessageIdand scrolls to end once the message appears, replacing the previous optimistic-message scroll timing.MessagesTimelineascontentInsetEndAdjustmentso content is never obscured.buildThreadFeedno longer accepts queued messages andthreadActivitytypes no longer include a'queued-message'variant.@legendapp/listfrom3.0.0-beta.44to3.2.0on both web and mobile.'queued-message'feed entries changes visible send state; optimistic messages sent but not yet persisted will no longer appear in the feed until the server confirms them.Macroscope summarized 11e34a5.
Note
Medium Risk
Large cross-platform scroll and composer-layout changes affect core chat UX; queued messages no longer appear in the mobile feed until confirmed.
Overview
Upgrades
@legendapp/listto 3.2.0 on web and mobile and replaces bespoke chat scroll/inset logic with Legend List’s floating composer and AI-chat anchoring APIs.Web measures the composer overlay height (
ResizeObserver) and passes it to the timeline ascontentInsetEndAdjustment; on send it sets a timeline anchor to the new user message id and uses sharedresolveChatListAnchoredEndSpace. The composer becomes an absolute overlay with glass/backdrop styling, and composer resize no longer forces stick-to-bottom. Virtualized lists gaingetItemTypeandvoidon async imperative scroll calls.Mobile switches the thread feed to
KeyboardAwareLegendListwithuseKeyboardChatComposerInset/useKeyboardScrollToEnd, parent-owned list ref,freeze, and anchor scroll after send whenonSendMessagereturnsMessageId | null. Custom near-end/bottom-inset helpers and in-feedqueued-messagerows are removed; the empty state stays mounted over the list for first-message scroll behavior.Adds
@t3tools/shared/chatList(tests included) and bumps related mobile peers (react-native-keyboard-controller,react-native-nitro-modules).Reviewed by Cursor Bugbot for commit 11e34a5. Bugbot is set up for automated code reviews on this repo. Configure here.