fix(chat): stop stranding ordinals outside the loaded thread window

.claude/hooks/pre-commit-check.sh

@@ -12,6 +12,13 @@ if ! (cd "$REPO_ROOT/shared" && yarn lint 2>&1); then
   exit 2
 fi
 
+# Bailouts are invisible to eslint: no react-compiler rule is wired into
+# eslint.config.mjs, so this is the only check that catches them.
+if ! (cd "$REPO_ROOT/shared" && yarn lint:bailouts 2>&1); then
+  echo "React-compiler bailout check failed — commit blocked." >&2
+  exit 2
+fi
+
 if ! (cd "$REPO_ROOT/shared" && yarn tsc 2>&1); then
   echo "TypeScript check failed — commit blocked." >&2
   exit 2

CLAUDE.md

@@ -26,3 +26,5 @@ Repo root is `client/`. TS source lives in `shared/`. Always use absolute paths
 
 ## Validation
 After TS changes (from `shared/`): `yarn lint:all` (= `yarn lint` && `yarn lint:bailouts` && `yarn tsc`). Plain `yarn lint` is eslint only and does NOT catch react-compiler bailouts — no compiler rule is wired into `eslint.config.mjs`, so bailouts only surface via `lint:bailouts`. `lint:bailouts` also flags components the compiler cannot name (an `isMobile ? arrow : arrow` ternary is never compiled at all, so nothing in it is memoized — name both branches instead), and memo scopes keyed on the whole props object (a `props.x` read inside a callback, or a destructure below one, makes the compiler key on `props` itself, so the cache never hits — read every prop through one destructure at the top, above every callback). Repo baseline is 0 bailouts and 0 whole-props deps; keep it there. When debugging visually, skip until fix is confirmed. Never delete the ESLint cache.
+
+Before reporting any TS change complete: run `yarn lint:all`, then run `/code-review high` against your own diff and fix what it finds. Report done only after both are clean — do not hand unvalidated work to the user for review. If a finding is wrong, say why instead of applying it.

shared/chat/conversation/attachment-actions.tsx

@@ -455,47 +455,8 @@ export const useConversationAttachmentActions = () => {
     ignorePromise(f())
   }
 
-  const loadNextAttachment = async (from: T.Chat.Ordinal, backInTime: boolean) => {
-    const fromMsg = threadStore.getState().messageMap.get(from)
-    if (!fromMsg) {
-      return Promise.reject(new Error('Incorrect from'))
-    }
-    const {deviceName, username} = useCurrentUserState.getState()
-    const getLastOrdinal = () => threadStore.getState().messageOrdinals?.at(-1) ?? T.Chat.numberToOrdinal(0)
-    const result = await T.RPCChat.localGetNextAttachmentMessageLocalRpcPromise({
-      assetTypes: [T.RPCChat.AssetMetadataType.image, T.RPCChat.AssetMetadataType.video],
-      backInTime,
-      convID: T.Chat.keyToConversationID(conversationIDKey),
-      identifyBehavior: T.RPCGen.TLFIdentifyBehavior.chatGui,
-      messageID: fromMsg.id,
-    })
-
-    if (result.message) {
-      const goodMessage = Message.uiMessageToMessage(
-        conversationIDKey,
-        result.message,
-        username,
-        getLastOrdinal,
-        deviceName
-      )
-      if (goodMessage?.type === 'attachment') {
-        actions.addMessages([goodMessage])
-        let ordinal = goodMessage.ordinal
-        if (goodMessage.outboxID && !threadStore.getState().messageMap.get(ordinal)) {
-          const pendingOrdinal = threadStore.getState().pendingOutboxToOrdinal.get(goodMessage.outboxID)
-          if (pendingOrdinal) {
-            ordinal = pendingOrdinal
-          }
-        }
-        return ordinal
-      }
-    }
-    return Promise.reject(new Error('No more results'))
-  }
-
   return {
     attachmentDownload,
-    loadNextAttachment,
     messageAttachmentNativeSave,
     messageAttachmentNativeShare,
     showAttachmentPreview: (ordinal: T.Chat.Ordinal, message?: T.Chat.MessageAttachment) => {

shared/chat/conversation/normal/container.test.tsx

@@ -290,6 +290,45 @@ test('initial load uses the read message ID from mount even if meta changes befo
   expectOrangeLine(T.Chat.numberToOrdinal(15))
 })
 
+test('the read position is latched when localization lands, not when the load finishes', async () => {
+  // The DB-nuke order: the conversation is unlocalized and the thread is still loading, so the
+  // loaded gate skips the fetch when localization lands. The load then finishing flips `loaded`
+  // and issues mark-read in the same breath, so by the next render the meta can already carry the
+  // advanced read position. Asking the service about that one puts the unreadline at the newest
+  // message and the thread shows no divider at all; the position from the moment localization
+  // landed is the only one that means anything here.
+  const unreadlineRpc = getUnreadlineRpc().mockResolvedValue({
+    offline: false,
+    unreadlineID: T.Chat.numberToMessageID(6),
+  })
+  mockLoaded = false
+  mockMeta = makeMeta(convID, -1)
+
+  render(<NormalWrapper />)
+  await flushOrangeLine()
+  expect(unreadlineRpc).not.toHaveBeenCalled()
+
+  // Localization lands while the thread load is still in flight.
+  mockMeta = makeMeta(convID, 5, 9)
+  act(() => {
+    useShellState.setState({mobileAppState: 'background'})
+  })
+  await flushOrangeLine()
+  expect(unreadlineRpc).not.toHaveBeenCalled()
+
+  // The load finishes, and the mark-read it issues has already moved the read position.
+  mockMeta = makeMeta(convID, 9, 9)
+  act(() => {
+    mockLoaded = true
+    useShellState.setState({mobileAppState: 'active'})
+  })
+  await flushOrangeLine()
+
+  expect(unreadlineRpc).toHaveBeenCalledTimes(1)
+  expectUnreadlineRpcReadMsgID(unreadlineRpc, 5)
+  expectOrangeLine(T.Chat.numberToOrdinal(6))
+})
+
 test('a thread reload does not refetch the orange line against the stale mount read position', async () => {
   const unreadlineRpc = getUnreadlineRpc().mockResolvedValue({
     offline: false,
@@ -322,7 +361,12 @@ test('a thread reload does not refetch the orange line against the stale mount r
   expectOrangeLine(noOrangeLine)
 })
 
-test('negative read message IDs are clamped before fetching the orange line', async () => {
+test('an unknown read position draws no orange line rather than one above everything', async () => {
+  // A negative read position means the conversation's meta has not landed yet
+  // (emptyConversationMeta reads -1), which a DB nuke makes the norm. The old code clamped it to 0,
+  // and the service answers 0 with "everything is unread", pinning the line above the oldest
+  // message - and since the state is set once, that answer used to stick for the life of the mount.
+  // 0 itself is a real read position and is still asked about; see the zero-value test below.
   const unreadlineRpc = getUnreadlineRpc().mockResolvedValue({
     offline: false,
     unreadlineID: T.Chat.numberToMessageID(8),
@@ -332,9 +376,58 @@ test('negative read message IDs are clamped before fetching the orange line', as
   render(<NormalWrapper />)
   await flushOrangeLine()
 
-  expect(unreadlineRpc).toHaveBeenCalledTimes(1)
-  expectUnreadlineRpcReadMsgID(unreadlineRpc, 0)
-  expectOrangeLine(T.Chat.numberToOrdinal(8))
+  expect(unreadlineRpc).not.toHaveBeenCalled()
+  expectOrangeLine(noOrangeLine)
+})
+
+test('an unknown read position is not asked about', async () => {
+  // -1 is emptyConversationMeta's "not localized yet", the norm right after a DB nuke. The old code
+  // clamped it to 0, so the service answered "everything is unread" and pinned the line above the
+  // oldest message - and since the state is set once, that answer stuck.
+  const unreadlineRpc = getUnreadlineRpc().mockResolvedValue({
+    offline: false,
+    unreadlineID: T.Chat.numberToMessageID(8),
+  })
+  mockMeta = makeMeta(convID, -1)
+
+  render(<NormalWrapper />)
+  await flushOrangeLine()
+
+  expect(unreadlineRpc).not.toHaveBeenCalled()
+  expectOrangeLine(noOrangeLine)
+})
+
+test('an inactive conversation with an unknown read position is not asked about either', async () => {
+  // The inactive refresh passes the live readMsgID rather than the mount-time one, so it reaches
+  // loadOrangeLine with -1 directly and needs its own guard.
+  const unreadlineRpc = getUnreadlineRpc().mockResolvedValue({
+    offline: false,
+    unreadlineID: T.Chat.numberToMessageID(8),
+  })
+  mockMeta = makeMeta(convID, -1)
+  useShellState.setState({active: false})
+
+  render(<NormalWrapper />)
+  await flushOrangeLine()
+
+  expect(unreadlineRpc).not.toHaveBeenCalled()
+})
+
+test('a zero read position is a real answer and is still asked about', async () => {
+  // ReaderInfo reports 0 for a conversation you have genuinely never read - every first open of a
+  // new channel or DM. "Everything is unread" is the correct answer there, so suppressing the
+  // request would silently drop the orange line for exactly those conversations. Only a negative
+  // read position means "not known yet".
+  const unreadlineRpc = getUnreadlineRpc().mockResolvedValue({
+    offline: false,
+    unreadlineID: T.Chat.numberToMessageID(8),
+  })
+  mockMeta = makeMeta(convID, 0)
+
+  render(<NormalWrapper />)
+  await flushOrangeLine()
+
+  expect(unreadlineRpc).toHaveBeenCalledWith(expect.objectContaining({readMsgID: 0}))
 })
 
 test('zero unreadline responses render as no orange line', async () => {

shared/chat/conversation/normal/container.tsx

@@ -58,18 +58,43 @@ const useOrangeLine = (
   const {maxVisibleMsgID, readMsgID} = useThreadMeta(
     C.useShallow(m => ({maxVisibleMsgID: m.maxVisibleMsgID, readMsgID: m.readMsgID}))
   )
-  // Keep the read position from when this conversation mounted. Mark-as-read updates
-  // readMsgID shortly after navigation, but the open thread should retain its orange line.
-  const [initialReadMsgID] = React.useState(() => readMsgID)
+  // Keep the read position from when this conversation mounted. Mark-as-read updates readMsgID
+  // shortly after navigation, but the open thread should retain its orange line.
+  //
+  // An unlocalized conversation reads -1 ("not known yet"), which a DB nuke makes the norm, so
+  // freezing on mount would pin that and the thread would never get an orange line for the life of
+  // the mount. Latch the first real value instead, on the commit it arrives in, rather than reading
+  // the live one where it is used. The two differ exactly when it matters: after a nuke the thread
+  // is still loading when localization lands, so the load below is skipped, and the load then
+  // finishing flips `loaded` and issues mark-read in the same breath. A live read on the next
+  // commit can already see the advanced position, and the thread then shows no unread divider at
+  // all - the failure this latch exists to prevent.
+  const latchedReadMsgIDRef = React.useRef(readMsgID)
+  React.useEffect(() => {
+    if (latchedReadMsgIDRef.current < 0 && readMsgID >= 0) {
+      latchedReadMsgIDRef.current = readMsgID
+    }
+  }, [readMsgID])
 
   const loadOrangeLine = React.useEffectEvent(
     (conversationIDKey: T.Chat.ConversationIDKey, readMsgID: T.Chat.MessageID) => {
+      // Negative means we do not know the read position yet: an unlocalized conversation reads -1
+      // from emptyConversationMeta, which a DB nuke makes the norm, and the old code turned that
+      // into 0 - so the service answered "everything is unread" and put the line above the oldest
+      // message. Since the state is set once and only refreshed while the conversation is
+      // inactive, that answer stuck.
+      //
+      // Zero is different and must still be asked: ReaderInfo reports 0 for a conversation you
+      // have genuinely never read, where "everything is unread" is the right answer.
+      if (readMsgID < 0) {
+        return
+      }
       const f = async () => {
         const convID = T.Chat.keyToConversationID(conversationIDKey)
         const unreadlineRes = await T.RPCChat.localGetUnreadlineRpcPromise({
           convID,
           identifyBehavior: T.RPCGen.TLFIdentifyBehavior.chatGui,
-          readMsgID: readMsgID < 0 ? 0 : readMsgID,
+          readMsgID,
         })
         const nextOrangeLine = T.Chat.numberToOrdinal(
           unreadlineRes.unreadlineID ? unreadlineRes.unreadlineID : 0
@@ -100,11 +125,16 @@ const useOrangeLine = (
   // messages we sent ourselves.
   const initialOrangeLineLoadedRef = React.useRef(false)
   React.useEffect(() => {
-    if (loaded && !initialOrangeLineLoadedRef.current) {
+    // Only claim the latch once there is a read position to ask about, so an unlocalized
+    // conversation gets its orange line when localization lands rather than never. readMsgID is a
+    // dep so that landing wakes this effect; the value asked about is the latched one, and the
+    // effect that sets it is declared above so it has already run for this commit.
+    const readMsgIDAtLocalization = latchedReadMsgIDRef.current
+    if (loaded && !initialOrangeLineLoadedRef.current && readMsgIDAtLocalization >= 0) {
       initialOrangeLineLoadedRef.current = true
-      loadOrangeLine(id, initialReadMsgID)
+      loadOrangeLine(id, readMsgIDAtLocalization)
     }
-  }, [id, loaded, initialReadMsgID])
+  }, [id, loaded, readMsgID])
 
   // just use the rpc for orange line if we're not active
   // if we are active we want to keep whatever state we had so it is maintained