fix(chat): mount assistant avatar at conversation bottom#32583
Conversation
…turn cluster The avatar slot was nested inside LatestTurnRow, which meant it only rendered when the partition produced an anchor user message. Two visible failure modes: 1. Recovered/assistant-only conversations (no trailing user message) had no avatar at all — partition.anchorMessage is null, so the whole LatestTurnRow tree (including the avatar slot) was skipped. 2. When scrolled away from the latest turn, the avatar appeared to move with the latest turn rather than living at the conversation bottom, which is what Vargas asked for: the avatar should be present at the bottom regardless of message state. Move the avatar out of LatestTurnRow into Transcript itself, inside a new "latest-edge" wrapper that owns the viewport-min-height pinning. The wrapper renders whenever there is an anchor OR a renderAvatar callback, so: - anchor + avatar: anchor pins to viewport top, avatar pins to viewport bottom (same visual layout as before). - anchor, no avatar: same as before, just no avatar. - no anchor + avatar: avatar still mounts at the bottom of history. - empty conversation + avatar: avatar renders inside the wrapper. DOM identity for the avatar is preserved across turn boundaries the same way (no remount across user-send → response gap), so the ChatAvatar entrance-spring flicker comment stays accurate. Tests: - latest-turn-row.test.tsx now covers the cluster shape only (anchor → response order, no avatar/edge sentinel here). - transcript.test.tsx adds 4 cases for renderAvatar behavior (assistant-only history, anchor + response + avatar order, renderAvatar omitted, empty conversation w/ avatar).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 764e9eeb2b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| * whenever the chat has avatar data so DOM identity is | ||
| * preserved across turn boundaries (no ChatAvatar entrance- | ||
| * spring flicker). */} | ||
| {(partition.anchorMessage || rest.renderAvatar) && ( |
There was a problem hiding this comment.
When renderAvatar is present but there is no latest user anchor, this condition still appends a viewport-height latest-edge region after all historyItems. The scroll coordinator auto-pins to the bottom on conversation switches, so assistant-only histories/onboarding content land just above the viewport and the user sees blank space plus the avatar instead of the latest message. Keep the viewport-height spacer tied to an actual anchor, or render the no-anchor avatar without adding a full bottom region after history.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed real. The style={{ minHeight: viewportMinHeight }} was applied whenever renderAvatar was present — including the no-anchor case (assistant-only history, recovered conversations). With #32239 snapping to bottom on container DOM attach, the viewport landed on the empty flex-1 spacer + avatar, pushing the actual latest message above the visible area.
Fix in #32596: gate both minHeight: viewportMinHeight and the flex-1 spacer on partition.anchorMessage. Avatar now renders inline directly below history when no anchor exists. Regression tests added in transcript.test.tsx covering all three states (no-anchor + avatar, anchor + avatar, anchor + no avatar). 👍
| {partition.anchorMessage && ( | ||
| <LatestTurnRow | ||
| anchorMessage={partition.anchorMessage} | ||
| responseItems={partition.responseItems} | ||
| {...rowProps} | ||
| /> |
There was a problem hiding this comment.
When the transcript starts with no anchor (empty or assistant-only) and then the first user message arrives, this unkeyed LatestTurnRow is inserted before the existing spacer/avatar <div> siblings. React reconciles the following <div>s by index, so the current avatar wrapper is reused as the spacer and ChatAvatar unmounts/remounts, replaying the entrance animation this change is trying to avoid. Add stable keys or keep a stable placeholder before the avatar so its DOM identity survives the no-anchor → anchor transition.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Investigated empirically — the avatar wrapper is not reused as the spacer and ChatAvatar does not remount across the no-anchor → anchor transition. React's reconcileChildrenArray tracks fiber.index (the previous render's position in the children array, including false-skipped slots), so the existing avatar fiber at fiber.index=2 still matches newIdx=2 after slot 0 transitions from false to <LatestTurnRow> — same <div> type, fiber reused, no entrance-spring replay.
Locked in with a DOM-lifecycle regression test in #32596 (transcript.test.tsx): mount-tracking avatar, three rerenders (no-anchor → anchor → no-anchor), asserts mount=1, unmount=0 throughout. Verified the test framework can detect remount by sanity-checking against a conversationId change (which re-keys the scroll container and forces remount): asserts pass as mount=2, unmount=1.
If we ever do reshape the conditionals (e.g. reorder siblings, change the conditional shape), the regression test will catch the resulting remount. 👎 with respect — but it's a great defense-in-depth nudge and the test wouldn't exist without it.
… on #32583) (#32596) Two pieces of Codex feedback on #32583. One was a legitimate bug, the other a misread of React reconciliation that this PR locks in as a regression test. ## Codex #1 — Legitimate bug, fixed The merged code applies `minHeight: viewportMinHeight` to the latest-edge wrapper whenever `renderAvatar` is present, even when `partition.anchorMessage` is null (assistant-only history: recovered conversations whose user message was lost, onboarding before the first submit). `useViewportMinHeight` reports the scroll container's `clientHeight` ≈ a full viewport. `refactor(web): scroll to bottom on transcript container DOM attach (#32239)` snaps the scroll to bottom on conversation switch. With no anchor at the top of the spacer, the bottom-pinned viewport lands on blank space + the avatar — the actual latest assistant message sits one viewport above and is invisible until the user scrolls up. Fix: gate both `minHeight: viewportMinHeight` and the `flex-1` spacer on `partition.anchorMessage`. Without an anchor, the avatar renders inline directly below the last history item. ## Codex #2 — Misread, locked in as a regression test Codex claimed that inserting `<LatestTurnRow>` at slot 0 (was `false`) on the no-anchor → anchor transition would cause React to reconcile the following `<div>` siblings by index, reusing the current avatar wrapper as the spacer and remounting `ChatAvatar` — replaying the entrance-spring animation #32583 is trying to avoid. Empirically verified that this does not happen. React's `reconcileChildrenArray` tracks `fiber.index` (the previous render's position in the children array, INCLUDING `false`-skipped slots). When slot 0 transitions from `false` to `<LatestTurnRow>`, the existing avatar fiber at `fiber.index=2` still matches `newIdx=2` in the next render — same `<div>` type, fiber reused, `ChatAvatar` is not unmounted. Locked in with a DOM-lifecycle regression test that: 1. Renders the transcript with no anchor + a mount-tracking avatar. 2. Asserts mount=1, unmount=0. 3. Rerenders with the first user message inserted (no-anchor → anchor). 4. Asserts mount=1, unmount=0 (no remount). 5. Rerenders back to no-anchor. 6. Asserts mount=1, unmount=0 again. A sanity-check version of the test (forcing a conversationId change, which re-keys the scroll container) was used to verify the mount tracker actually detects remounts — confirmed mount=2/unmount=1 when remount really happens, so the assertions can fail. ## Tests - 13 transcript.test.tsx tests pass. - 3 new Codex #1 cases: no-anchor has no min-height; anchor has min-height; anchor-without-avatar still has min-height (regression guard for the original viewport-pinning behavior). - 1 new Codex #2 case: the mount-tracker test above. - Typecheck clean (`tsc --noEmit --skipLibCheck` in `apps/web`). - ESLint clean on touched files. - 4 pre-existing `transcript-message-body.test.tsx` failures in the directory-wide run are also present on `main` — caused by mock bleed between test files, not by this PR. Co-authored-by: vellum-apollo-bot[bot] <242025090+vellum-apollo-bot[bot]@users.noreply.github.com>
1 participant
What
Move the assistant avatar out of
LatestTurnRowand into a new latest-edge region at the bottom ofTranscript. The avatar now renders independent of the latest-turn partition: it lives at the bottom of the conversation regardless of whether there is a trailing user message.Why
The avatar slot was nested inside
LatestTurnRow, which only renders whenpartitionLatestTurnfinds a trailing user message. That meant:partition.anchorMessagewas null, so the wholeLatestTurnRowtree (including the avatar slot) was skipped. This is the case Vargas hit in theWorkspace tools (recovered)conversation.How
flex-1spacer, and thedata-latest-edgesentinel out ofLatestTurnRowand into a new region inTranscript. That region renders whenever there is an anchor OR arenderAvatarcallback.LatestTurnRowis now just the anchor + response cluster. No moreavatarSlot/viewportMinHeightprops.ChatAvatarentrance-spring flicker comment stays accurate.Tests
latest-turn-row.test.tsxsimplified: covers the cluster shape (anchor → response order) and asserts the avatar / edge sentinel are NOT rendered here.transcript.test.tsxgains 4 cases forrenderAvatar:renderAvataromitted → no avatar slot renderedrenderAvatar→ avatar still rendersAll 12 transcript tests pass locally; typecheck clean; eslint clean.
Fix triggered by the
Workspace tools (Recovered)conversation Vargas surfaced where the avatar was missing on every historical message — the avatar was only mounting under the latest-turn cluster, which the recovery-migrated conversation effectively never produced for older messages.