fix(chat): stop recycled message rows from leaking state and stale heights

shared/chat/conversation/list-area/index.tsx

@@ -11,6 +11,7 @@ import {PerfProfiler} from '@/perf/react-profiler'
 import {ThreadRefsContext} from '../normal/context'
 import {useConversationCenter} from '../center-context'
 import {
+  ShownUsernameCacheContext,
   useConversationThreadID,
   useConversationThreadLoadNewerMessagesDueToScroll,
   useConversationThreadLoadOlderMessagesDueToScroll,
@@ -20,7 +21,8 @@ import {
 } from '../thread-context'
 import {useJumpToRecent} from './jump-to-recent'
 import {useThreadLoadStatusOptionsGetter} from '../thread-load-status-context'
-import {getMessageRowType} from '../messages/row-metadata'
+import {getMessageRowType, getMessageShowUsername} from '../messages/row-metadata'
+import {useCurrentUserState} from '@/stores/current-user'
 import * as InputState from '../input-area/input-state'
 import sortedIndexOf from 'lodash/sortedIndexOf'
 import {copyToClipboard} from '@/util/storeless-actions'
@@ -47,21 +49,40 @@ const keyExtractor = (ordinal: ItemType) => String(ordinal)
 // trim off the search-bar lift so the jump button rests ~40px above the bar
 const jumpAboveBarTrim = 40
 
-// Item type for list recycling pool separation
+// Item type for list recycling pool separation. A message that leads its author group renders an
+// avatar + username header (~40px taller) than a grouped follow-on of the same render type. Without
+// splitting the pool, recycleItems reuses one container across both heights, so a recycled view
+// paints at the wrong height for a frame before re-measure — visible as rows overlapping during
+// scroll. Append ':hdr' so header and grouped rows pool separately.
 const useGetItemType = () => {
   const threadStore = useConversationThreadStore()
+  const you = useCurrentUserState(s => s.username)
+  // Must be the same sticky cache the rows render with (wrapper.tsx): without it, a row that keeps
+  // its sticky header after a scroll-back load would be typed headerless here, mixing tall headered
+  // rows into the headerless pool and poisoning that pool's height average.
+  const shownCache = React.useContext(ShownUsernameCacheContext)
   return React.useCallback(
     (ordinal: T.Chat.Ordinal) => {
       if (!ordinal) {
         return 'null'
       }
-      const {messageMap, messageTypeMap} = threadStore.getState()
+      const {messageMap, messageTypeMap, messageOrdinals} = threadStore.getState()
       const message = messageMap.get(ordinal)
-      return message
-        ? getMessageRowType(message, messageTypeMap.get(ordinal))
-        : (messageTypeMap.get(ordinal) ?? 'text')
+      if (!message) {
+        return messageTypeMap.get(ordinal) ?? 'text'
+      }
+      const base = getMessageRowType(message, messageTypeMap.get(ordinal))
+      const showUsername = getMessageShowUsername({
+        message,
+        messageMap,
+        messageOrdinals: messageOrdinals ?? noOrdinals,
+        ordinal,
+        shownCache,
+        you,
+      })
+      return showUsername ? `${base}:hdr` : base
     },
-    [threadStore]
+    [threadStore, you, shownCache]
   )
 }
 
@@ -521,7 +542,9 @@ const DesktopThreadWrapper = function DesktopThreadWrapper() {
           initialScrollAtEnd={initialScrollIndex === undefined}
           initialScrollIndex={initialScrollIndex}
           maintainScrollAtEnd={
-            centeredOrdinal !== undefined ? false : {on: {dataChange: true, itemLayout: true}}
+            centeredOrdinal !== undefined
+              ? false
+              : {on: {dataChange: true, footerLayout: true, itemLayout: true}}
           }
           // Stays on while centered: the full thread response lands after the cached one and
           // re-measures rows above the target, which slides it out of view unless anchored.

shared/chat/conversation/messages/row-metadata.test.ts

@@ -110,7 +110,9 @@ test('showUsername is derived from the previous ordinal and current message data
   ).toBe('bob')
 })
 
-test('row type preserves native recycle distinctions', () => {
+test('row type only uses suffixes that are stable for the message lifetime', () => {
+  // pending flips to confirmed after every send; reactions toggle. Both would leave stale
+  // recycling-pool labels behind, so they must NOT affect the row type.
   const pending = makeTextMessage({
     id: T.Chat.numberToMessageID(401),
     ordinal: T.Chat.numberToOrdinal(401),
@@ -140,10 +142,10 @@ test('row type preserves native recycle distinctions', () => {
     reactions: new Map([[':+1:', makeReaction('bob', 5)]]),
   })
 
-  expect(getMessageRowType(pending)).toBe('text:pending')
-  expect(getMessageRowType(failed)).toBe('text:pending')
+  expect(getMessageRowType(pending)).toBe('text')
+  expect(getMessageRowType(failed)).toBe('text:failed')
   expect(getMessageRowType(reply)).toBe('text:reply')
-  expect(getMessageRowType(reaction)).toBe('text:reactions')
+  expect(getMessageRowType(reaction)).toBe('text')
 })
 
 test('showUsername recomputes from the current neighboring ordinal after inserts and deletes', () => {
@@ -211,7 +213,7 @@ test('showUsername recomputes from the current neighboring ordinal after inserts
   ).toBe('bob')
 })
 
-test('row type combines pending, reply, and reaction suffixes after row edits', () => {
+test('row type combines stable suffixes and is unchanged by send confirmation', () => {
   const reply = makeTextMessage({
     id: T.Chat.numberToMessageID(600),
     ordinal: T.Chat.numberToOrdinal(600),
@@ -223,21 +225,33 @@ test('row type combines pending, reply, and reaction suffixes after row edits',
     replyTo: reply,
     submitState: 'pending',
   })
+  const failedReply = makeTextMessage({
+    errorReason: 'send failed',
+    id: T.Chat.numberToMessageID(603),
+    ordinal: T.Chat.numberToOrdinal(603),
+    replyTo: reply,
+    submitState: 'failed',
+  })
   const failedAttachment = makeAttachmentMessage({
     errorReason: 'upload failed',
     id: T.Chat.numberToMessageID(602),
     ordinal: T.Chat.numberToOrdinal(602),
     submitState: 'failed',
   })
 
-  expect(getMessageRowType(pendingReplyWithReaction)).toBe('text:pending:reply:reactions')
-  expect(getMessageRowType(failedAttachment)).toBe('attachment:pending')
+  expect(getMessageRowType(pendingReplyWithReaction)).toBe('text:reply')
+  expect(getMessageRowType(failedReply)).toBe('text:failed:reply')
+  expect(getMessageRowType(failedAttachment)).toBe('attachment:failed')
 
-  const edited = makeTextMessage({
+  // confirmation (pending → sent) must not change the type: the recycling pool label was recorded
+  // at allocation and is never updated in place
+  const confirmed = makeTextMessage({
     id: T.Chat.numberToMessageID(601),
     ordinal: T.Chat.numberToOrdinal(601),
+    reactions: new Map([[':+1:', makeReaction('bob', 5)]]),
+    replyTo: reply,
     submitState: undefined,
   })
 
-  expect(getMessageRowType(edited)).toBe('text')
+  expect(getMessageRowType(confirmed)).toBe('text:reply')
 })

shared/chat/conversation/messages/row-metadata.tsx

@@ -70,22 +70,20 @@ export const getMessageRowRecycleType = (
   let rowRecycleType = baseType
   let needsSpecificRecycleType = false
 
-  if (
-    (message.type === 'text' || message.type === 'attachment') &&
-    (message.submitState === 'pending' || message.submitState === 'failed')
-  ) {
-    rowRecycleType += ':pending'
+  // Only suffixes that are stable for the message's lifetime: the recycling pool label is recorded
+  // when a container is allocated and never updated on in-place changes, so a suffix that can flip
+  // (pending → confirmed after every send, reactions toggling on and off) leaves stale pool labels
+  // behind and recycled containers paint at the wrong pooled height. 'failed' is sticky until an
+  // explicit retry and 'reply' never changes.
+  if ((message.type === 'text' || message.type === 'attachment') && message.submitState === 'failed') {
+    rowRecycleType += ':failed'
     needsSpecificRecycleType = true
   }
 
   if (message.type === 'text' && message.replyTo) {
     rowRecycleType += ':reply'
     needsSpecificRecycleType = true
   }
-  if (message.reactions?.size) {
-    rowRecycleType += ':reactions'
-    needsSpecificRecycleType = true
-  }
 
   return needsSpecificRecycleType ? rowRecycleType : undefined
 }

shared/chat/conversation/messages/text/coinflip/index.tsx

@@ -7,6 +7,7 @@ import {useOrdinal} from '@/chat/conversation/messages/ids-context'
 import {pluralize} from '@/util/string'
 import {useConversationThreadMessage, useConversationThreadSelector} from '../../../thread-context'
 import {useConversationSendActions} from '../../../send-actions'
+import {useSyncRowLayout} from '../../use-sync-row-layout'
 
 // The flip result arrives via a separate status notification, not with the thread, so on initial
 // load (an already-finished flip) the card first-paints with no result and then grows when the
@@ -47,6 +48,11 @@ function CoinFlipContainer() {
   const showParticipants = phase === T.RPCChat.UICoinFlipPhase.complete
   const numParticipants = participants?.length ?? 0
 
+  // The flip result streams in after first paint and grows the card; flush the row measure so the
+  // list re-pins to the newest message instead of parking above it. Keyed on the status signals
+  // that change the card height (loaded yet, phase, participant count, result present).
+  useSyncRowLayout(`${status === undefined ? 0 : 1}|${phase ?? -1}|${numParticipants}|${resultInfo ? 1 : 0}`)
+
   const revealed =
     participants?.reduce((r, p) => {
       return r + (p.reveal ? 1 : 0)

shared/chat/conversation/messages/text/unfurl/unfurl-list/image/index.tsx

@@ -4,6 +4,7 @@ import {clampImageSize} from '@/constants/chat/helpers'
 import {maxWidth} from '@/chat/conversation/messages/attachment/shared'
 import {Video} from './video'
 import {openURL} from '@/util/misc'
+import {useSyncRowLayout} from '@/chat/conversation/messages/use-sync-row-layout'
 
 export type Props = {
   autoplayVideo: boolean
@@ -28,6 +29,10 @@ const UnfurlImage = (p: Props) => {
   const maxSize = Math.min(maxWidth, 320) - (widthPadding || 0)
   const {height, width} = clampImageSize(p.width, p.height, maxSize, 320)
 
+  // Usually the metadata dimensions are known at first paint, but if they arrive in a later update
+  // the image grows; flush the row measure so the list re-pins instead of parking above newest.
+  useSyncRowLayout(`${width}x${height}`)
+
   return isVideo ? (
     <Video
       autoPlay={autoplayVideo}

shared/chat/conversation/messages/use-sync-row-layout.tsx

@@ -0,0 +1,27 @@
+import * as React from 'react'
+import type * as T from '@/constants/types'
+import {useSyncLayout} from '@legendapp/list/react-native'
+import {useOrdinal} from './ids-context'
+
+// When a row's content settles to a new height after first paint (a flip result streams in,
+// reactions appear, an unfurl loads), force LegendList to re-measure this row synchronously so its
+// bottom-anchoring / re-pin uses the final height on the same frame instead of a frame late (which
+// otherwise leaves the thread parked above the newest message). Pass a signature that changes when
+// the height-affecting content changes. Flushes only when the signature changes for the SAME
+// message: the initial mount and a recycled container switching to a new ordinal both get measured
+// by LegendList's own onLayout, so flushing there is wasted sync layout work mid-scroll. Noops
+// wherever the row is not inside a LegendList container, or on the old architecture.
+export const useSyncRowLayout = (signature: string | number) => {
+  const ordinal = useOrdinal()
+  const syncLayout = useSyncLayout()
+  const lastRef = React.useRef<{ordinal: T.Chat.Ordinal; signature: string | number} | undefined>(
+    undefined
+  )
+  React.useLayoutEffect(() => {
+    const last = lastRef.current
+    lastRef.current = {ordinal, signature}
+    if (last?.ordinal !== ordinal) return
+    if (last.signature === signature) return
+    syncLayout()
+  }, [ordinal, signature, syncLayout])
+}

shared/chat/conversation/messages/wrapper/exploding-height-retainer/index.tsx

@@ -31,7 +31,13 @@ const DesktopExplodingHeightRetainer = (p: Props) => {
     doneKey: retainHeight ? messageKey : undefined,
     retainHeight,
   }))
-  const [height, setHeight] = React.useState(17)
+  // keyed to the message: with list recycling this instance is reused for other messages, and a
+  // stale measured height would be retained as the wrong row height
+  const [heightState, setHeightState] = React.useState(() => ({height: 17, key: messageKey}))
+  if (heightState.key !== messageKey) {
+    setHeightState({height: 17, key: messageKey})
+  }
+  const height = heightState.height
 
   let currentAnimationState = animationState
   if (animationState.retainHeight !== retainHeight) {
@@ -64,7 +70,7 @@ const DesktopExplodingHeightRetainer = (p: Props) => {
   const setBoxRef = React.useCallback((ref: Kb.MeasureRef | null) => {
     const measuredHeight = ref?.getBoundingClientRect().height
     if (measuredHeight) {
-      setHeight(lastHeight => (lastHeight === measuredHeight ? lastHeight : measuredHeight))
+      setHeightState(prev => (prev.height === measuredHeight ? prev : {...prev, height: measuredHeight}))
     }
   }, [])
 
@@ -162,9 +168,18 @@ function FlameFront(props: {height: number; stop: boolean}) {
 // Native implementation
 const NativeExplodingHeightRetainer = (p: Props) => {
   const {retainHeight, explodedBy, messageKey, style, children} = p
-  const [height, setHeight] = React.useState(20)
+  // keyed to the message: with recycleItems this instance is reused for other messages, and a
+  // stale measured height would be applied as the retained height of the wrong row — and since
+  // retainHeight forces the style height, onLayout would only ever report the forced value back,
+  // so it could never self-correct
+  const [heightState, setHeightState] = React.useState(() => ({height: 20, key: messageKey}))
+  if (heightState.key !== messageKey) {
+    setHeightState({height: 20, key: messageKey})
+  }
+  const height = heightState.height
   const onLayout = (evt: Kb.LayoutEvent) => {
-    setHeight(evt.nativeEvent.layout.height)
+    const h = evt.nativeEvent.layout.height
+    setHeightState(prev => (prev.height === h ? prev : {...prev, height: h}))
   }
   const numImages = Math.ceil(height / 80)
 
@@ -181,10 +196,12 @@ const NativeExplodingHeightRetainer = (p: Props) => {
       ])}
     >
       {retainHeight ? null : children}
+      {/* keyed so recycling to a different message remounts the tower — its animation state
+          (slider value, exploded tag) is per-message */}
       <AnimatedAshTower
+        key={messageKey}
         exploded={retainHeight}
         explodedBy={explodedBy}
-        messageKey={messageKey}
         numImages={numImages}
       />
     </Kb.Box2>
@@ -194,7 +211,6 @@ const NativeExplodingHeightRetainer = (p: Props) => {
 type AshTowerProps = {
   exploded: boolean
   explodedBy?: string
-  messageKey: string
   numImages: number
 }
 

shared/chat/conversation/messages/wrapper/sent.native.tsx

@@ -4,6 +4,8 @@ import Animated, {FadeInDown} from 'react-native-reanimated'
 // Slide-up + fade for a message you just sent. The thread list is an inverted
 // FlatList (cells are flipped with scaleY: -1), so FadeInDown renders on screen
 // as sliding up from below. Runs entirely on the UI thread with no re-renders.
+// The entering animation only plays when this Animated.View MOUNTS — callers must
+// key it per message (recycled containers reuse instances).
 export function Sent(p: {children: React.ReactNode}) {
   return (
     <Animated.View entering={FadeInDown.duration(200)} style={styles.container}>

shared/chat/conversation/messages/wrapper/wrapper.tsx

@@ -12,6 +12,7 @@ import ExplodingMeta from './exploding-meta'
 import LongPressable from './long-pressable'
 import {useMessagePopup} from '../message-popup'
 import ReactionsRow from '../reactions-rows'
+import {useSyncRowLayout} from '../use-sync-row-layout'
 import SendIndicator from './send-indicator'
 import * as T from '@/constants/types'
 import capitalize from 'lodash/capitalize'
@@ -560,6 +561,9 @@ function TextAndSiblings(p: TSProps) {
   const {hasReactions, popupAnchor, reactions, sendIndicatorFailed, sendIndicatorID} = p
   const {sendIndicatorSent, type, setShowingPicker, showCoinsIcon, shouldShowPopup} = p
   const {showPopup, showExplodingCountdown, showRevoked, showSendIndicator, showingPicker, submitState} = p
+  // Reactions appearing and an unfurl card loading both grow the row after first paint; flush the
+  // measure so the list re-pins to the newest message instead of parking above it.
+  useSyncRowLayout(`${reactions?.size ?? 0}|${hasUnfurlList ? 1 : 0}`)
   const pressableProps = isMobile
     ? {
         onLongPress: decorate && shouldShowPopup ? showPopup : undefined,
@@ -913,7 +917,6 @@ function RightSide(p: RProps) {
 export function WrapperMessage(p: WrapperMessageProps) {
   const {ordinal, bottomChildren, children, messageData: mdata} = p
   const {showPopup, showingPopup, popup, popupAnchor} = p
-  const [showingPicker, setShowingPicker] = React.useState(false)
 
   const {decorate, type, hasReactions, isEditing, shouldShowPopup} = mdata
   const {canShowReactionsPopup, ecrType, exploded, explodesAt, forceExplodingRetainer, messageKey} = mdata
@@ -924,9 +927,23 @@ export function WrapperMessage(p: WrapperMessageProps) {
   const {setEditing, setReplyTo, toggleMessageReaction} = mdata
   const {author, botAlias, isAdhocBot, showUsername, teamID, teamType, teamname, timestamp} = mdata
 
-  // captured at mount: only the row created by your send animates, and the tree
-  // shape stays stable when the message is later confirmed (youSent flips false)
-  const [animateSent] = React.useState(isMobile && mdata.youSent)
+  // Both pieces of per-row state are keyed to messageKey and reset when it changes: with
+  // recycleItems the component instance is REUSED for a different message, so plain mount-captured
+  // state would leak across rows (picker open on the wrong row, sent-animation wrapper stuck on).
+  // Same-message updates keep the captured value, so the tree stays stable when the message is
+  // later confirmed (youSent flips false).
+  const [rowState, setRowState] = React.useState(() => ({
+    animateSent: isMobile && mdata.youSent,
+    key: messageKey,
+    showingPicker: false,
+  }))
+  if (rowState.key !== messageKey) {
+    setRowState({animateSent: isMobile && mdata.youSent, key: messageKey, showingPicker: false})
+  }
+  const {animateSent, showingPicker} = rowState
+  const setShowingPicker = React.useCallback((s: boolean) => {
+    setRowState(prev => (prev.showingPicker === s ? prev : {...prev, showingPicker: s}))
+  }, [])
 
   const isHighlighted = showCenteredHighlight || isEditing
   const tsprops = {
@@ -992,7 +1009,9 @@ export function WrapperMessage(p: WrapperMessageProps) {
 
   return (
     <MessageContext value={messageContext}>
-      {animateSent ? <Sent>{row}</Sent> : row}
+      {/* keyed so a recycled container going straight from one of your sends to another remounts
+          the Animated.View — the entering animation only plays on mount */}
+      {animateSent ? <Sent key={messageKey}>{row}</Sent> : row}
       {popup}
     </MessageContext>
   )