From b6413d763db5e7569887028932547be604e98d1c Mon Sep 17 00:00:00 2001
From: Jared Vititoe <jjvititoe1@gmail.com>
Date: Fri, 10 Jul 2026 01:21:01 -0400
Subject: [PATCH] fix(threads): harden threads list after review

Address findings from 3 review agents on the Threads list panel:

- Last-activity accuracy (SDK): sort key and the "last reply <time>"
  label now use thread.replyToEvent.getTs() (server bundle latest_event)
  instead of lastReply(), which returns the ROOT time until each thread's
  replies lazily paginate (or permanently on fetch error). Applied to the
  hook signature too.

- Live-refresh completeness (correctness): the useRoomThreads signature
  now includes thread.length and the root event's replacingEventId, so a
  mid-thread redaction (reply count) and a root-message edit (row snippet)
  refresh the row live instead of going stale.

- a11y: the row's aria-label was the button's whole accessible name,
  hiding the snippet/count/unread from screen readers. It now describes
  the thread ("Open thread by <name>, unread, N replies, last reply ..").

- Unread badge: replaced the bare green dot (Success = the mention color)
  with the app-wide UnreadBadge, using the Highlight count so mentions
  render red and ordinary unread renders secondary, matching room-nav.

- Hover/focus affordance: the clickable row moved its inline styles to a
  css class with token-based :hover / :active backgrounds.

- Participant pile now also includes the last replier from the bundle.

- Stabilized the panel's onClose/onOpenThread with useCallback so its
  Escape listener isn't re-subscribed every Room render. Added
  filter->sort pipeline + all/participating immutability tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 LOTUS_FEATURES.md                             |  2 +-
 src/app/features/room/Room.tsx                | 18 +++++--
 .../room/thread/ThreadsListPanel.css.ts       | 16 +++++-
 .../features/room/thread/ThreadsListPanel.tsx | 51 +++++++++++--------
 src/app/hooks/useRoomThreads.ts               | 19 ++++---
 src/app/utils/threadList.test.ts              | 16 +++++-
 6 files changed, 86 insertions(+), 36 deletions(-)

diff --git a/LOTUS_FEATURES.md b/LOTUS_FEATURES.md
index 0db25048d..6e14d921c 100644
--- a/LOTUS_FEATURES.md
+++ b/LOTUS_FEATURES.md
@@ -869,7 +869,7 @@ A right-side drawer (mirrors the members drawer; fullscreen on mobile) with the
 
 A room-level overview of **all** threads, opened from a **Threads** button (🧵) in the room header (mirrors the gallery/widgets toggles). Each row shows the root sender + message snippet, an unread dot, a meta line ("N replies · last reply 5m ago") and a **participant avatar pile**. A segmented **filter** (All / Unread / Participating — the latter via `thread.hasCurrentUserParticipated`) and **sort** (Recent / Oldest, by last-reply time) sit in the toolbar; both persist in localStorage (`cinny_threads_filter_v1` / `cinny_threads_sort_v1`). Clicking a row opens the existing single-thread `ThreadPanel` (reuses `setActiveThreadId`), and reading it clears the row's unread badge live. The list stays live via `ThreadEvent.New/NewReply/Update/Delete` + `RoomEvent.UnreadNotifications` and is virtualized for busy rooms.
 
-- Files: `features/room/thread/ThreadsListPanel.tsx`, `hooks/useRoomThreads.ts` (populates via `room.fetchRoomThreads()` + `room.getThreads()`), `state/threadsList.ts`, pure filter/sort in `utils/threadList.ts` (`filterThreads`/`sortThreads`, unit-tested). Reuses `useThreadSummary` data, `UnreadBadge`, `StackedAvatar`/`useMemberAvatar`, and the Bookmarks-panel segmented-control pattern.
+- Files: `features/room/thread/ThreadsListPanel.tsx`, `hooks/useRoomThreads.ts` (populates via `room.fetchRoomThreads()` + `room.getThreads()`; last-activity + reply-count + root-edit signature so rows refresh live), `state/threadsList.ts`, pure filter/sort in `utils/threadList.ts` (`filterThreads`/`sortThreads`, unit-tested). Unread mirrors `useThreadSummary`'s logic (`getThreadUnreadNotificationCount`, muted threads zeroed) and renders the app-wide `UnreadBadge` (red for mentions via the Highlight count). Reuses `StackedAvatar`/`useMemberAvatar` for the participant pile and the Bookmarks-panel segmented-control pattern.
 
 ### Summary Chips
 
diff --git a/src/app/features/room/Room.tsx b/src/app/features/room/Room.tsx
index e36a6c137..e543b60dc 100644
--- a/src/app/features/room/Room.tsx
+++ b/src/app/features/room/Room.tsx
@@ -70,6 +70,17 @@ export function Room() {
     ),
   );
 
+  // Stable handlers for the threads-list panel so its document-level Escape
+  // listener isn't torn down and re-added on every Room re-render.
+  const closeThreadsList = useCallback(() => setThreadsListOpen(false), [setThreadsListOpen]);
+  const openThreadFromList = useCallback(
+    (threadId: string) => {
+      setActiveThreadId(threadId);
+      setThreadsListOpen(false);
+    },
+    [setActiveThreadId, setThreadsListOpen],
+  );
+
   const callView = callEmbed?.roomId === room.roomId || room.isCallRoom() || callMembers.length > 0;
 
   // The content panels (thread / media gallery / widgets) are mutually exclusive
@@ -190,11 +201,8 @@ export function Room() {
             <ThreadsListPanel
               key={room.roomId}
               room={room}
-              onClose={() => setThreadsListOpen(false)}
-              onOpenThread={(threadId) => {
-                setActiveThreadId(threadId);
-                setThreadsListOpen(false);
-              }}
+              onClose={closeThreadsList}
+              onOpenThread={openThreadFromList}
             />
           </>
         )}
diff --git a/src/app/features/room/thread/ThreadsListPanel.css.ts b/src/app/features/room/thread/ThreadsListPanel.css.ts
index 9dc7d5be3..75360528a 100644
--- a/src/app/features/room/thread/ThreadsListPanel.css.ts
+++ b/src/app/features/room/thread/ThreadsListPanel.css.ts
@@ -1,5 +1,5 @@
 import { style } from '@vanilla-extract/css';
-import { config, toRem } from 'folds';
+import { color, config, toRem } from 'folds';
 
 export const ThreadsListPanel = style({
   width: toRem(340),
@@ -28,3 +28,17 @@ export const ThreadsListToolbar = style({
 export const ThreadsListContent = style({
   padding: config.space.S200,
 });
+
+export const ThreadRow = style({
+  width: '100%',
+  textAlign: 'left',
+  cursor: 'pointer',
+  border: 'none',
+  borderRadius: config.radii.R300,
+  padding: config.space.S300,
+  background: color.SurfaceVariant.Container,
+  selectors: {
+    '&:hover': { background: color.SurfaceVariant.ContainerHover },
+    '&:active': { background: color.SurfaceVariant.ContainerActive },
+  },
+});
diff --git a/src/app/features/room/thread/ThreadsListPanel.tsx b/src/app/features/room/thread/ThreadsListPanel.tsx
index 87dace71c..579695e9e 100644
--- a/src/app/features/room/thread/ThreadsListPanel.tsx
+++ b/src/app/features/room/thread/ThreadsListPanel.tsx
@@ -4,7 +4,6 @@ import { atomWithStorage, createJSONStorage } from 'jotai/utils';
 import { NotificationCountType, Room, Thread } from 'matrix-js-sdk';
 import {
   Avatar,
-  Badge,
   Box,
   Button,
   Header,
@@ -13,7 +12,6 @@ import {
   Icons,
   Scroll,
   Text,
-  color,
   config,
 } from 'folds';
 import classNames from 'classnames';
@@ -23,6 +21,7 @@ import { ContainerColor } from '../../../styles/ContainerColor.css';
 import { VirtualTile } from '../../../components/virtualizer';
 import { UserAvatar } from '../../../components/user-avatar';
 import { StackedAvatar } from '../../../components/stacked-avatar';
+import { UnreadBadge, UnreadBadgeCenter } from '../../../components/unread-badge';
 import { useMemberAvatar } from '../../../hooks/useMemberAvatar';
 import { trimReplyFromBody } from '../../../utils/room';
 import { scaleSystemEmoji } from '../../../plugins/react-custom-html-parser';
@@ -122,10 +121,11 @@ type ThreadRowProps = {
   room: Room;
   thread: Thread;
   unread: number;
+  highlight: number;
   participants: string[];
   onOpen: (threadId: string) => void;
 };
-function ThreadRow({ room, thread, unread, participants, onOpen }: ThreadRowProps) {
+function ThreadRow({ room, thread, unread, highlight, participants, onOpen }: ThreadRowProps) {
   const rootEvent = thread.rootEvent;
   const rootSender = rootEvent?.getSender() ?? '';
   const { name: rootName, avatarUrl } = useMemberAvatar(room, rootSender);
@@ -135,25 +135,21 @@ function ThreadRow({ room, thread, unread, participants, onOpen }: ThreadRowProp
       : '';
   const snippet = bodyRaw ? scaleSystemEmoji(trimReplyFromBody(bodyRaw)) : '(no preview)';
   const count = thread.length;
-  const lastTs = thread.lastReply()?.getTs() ?? rootEvent?.getTs();
+  const lastTs = thread.replyToEvent?.getTs() ?? rootEvent?.getTs();
   const extra = participants.length - MAX_PARTICIPANTS;
+  const replyLabel = `${count} ${count === 1 ? 'reply' : 'replies'}`;
+  const ariaLabel = `Open thread by ${rootName}${unread > 0 ? ', unread' : ''}, ${replyLabel}${
+    typeof lastTs === 'number' ? `, last reply ${formatTimeAgo(lastTs)}` : ''
+  }`;
 
   return (
     <Box
       as="button"
       direction="Column"
       gap="100"
+      className={css.ThreadRow}
       onClick={() => onOpen(thread.id)}
-      aria-label={`Open thread by ${rootName}`}
-      style={{
-        width: '100%',
-        textAlign: 'left',
-        cursor: 'pointer',
-        padding: config.space.S300,
-        borderRadius: config.radii.R300,
-        background: color.SurfaceVariant.Container,
-        border: 'none',
-      }}
+      aria-label={ariaLabel}
     >
       <Box alignItems="Center" gap="200">
         <Avatar size="200" radii="300">
@@ -168,7 +164,9 @@ function ThreadRow({ room, thread, unread, participants, onOpen }: ThreadRowProp
           {rootName}
         </Text>
         {unread > 0 && (
-          <Badge variant="Success" fill="Solid" radii="Pill" size="200" style={{ flexShrink: 0 }} />
+          <UnreadBadgeCenter>
+            <UnreadBadge highlight={highlight > 0} count={unread} />
+          </UnreadBadgeCenter>
         )}
       </Box>
 
@@ -233,19 +231,27 @@ export function ThreadsListPanel({ room, onClose, onOpenThread }: ThreadsListPan
     return () => document.removeEventListener('keydown', handleKeyDown);
   }, [onClose]);
 
-  const { visible, unreadById, participantsById, threadById } = useMemo(() => {
+  const { visible, unreadById, highlightById, participantsById, threadById } = useMemo(() => {
     const mutedSet = getMutedThreads(threadNotifications, room.roomId);
     const unreadMap = new Map<string, number>();
+    const highlightMap = new Map<string, number>();
     const partsMap = new Map<string, string[]>();
     const byId = new Map<string, Thread>();
     const snaps: ThreadSnapshot[] = threads.map((t) => {
       byId.set(t.id, t);
-      const rawUnread =
-        room.getThreadUnreadNotificationCount(t.id, NotificationCountType.Total) ?? 0;
-      const unread = mutedSet.has(t.id) ? 0 : rawUnread;
+      const muted = mutedSet.has(t.id);
+      const unread = muted
+        ? 0
+        : (room.getThreadUnreadNotificationCount(t.id, NotificationCountType.Total) ?? 0);
+      const highlight = muted
+        ? 0
+        : (room.getThreadUnreadNotificationCount(t.id, NotificationCountType.Highlight) ?? 0);
       unreadMap.set(t.id, unread);
+      highlightMap.set(t.id, highlight);
 
-      // Participant senders: root first, then timeline order, deduped.
+      // Participant senders: root first, then timeline order, plus the last
+      // replier (from the server bundle) so it's present even if not paginated
+      // into the loaded timeline yet. Deduped, order preserved.
       const seen = new Set<string>();
       const parts: string[] = [];
       const push = (s?: string | null) => {
@@ -256,11 +262,12 @@ export function ThreadsListPanel({ room, onClose, onOpenThread }: ThreadsListPan
       };
       push(t.rootEvent?.getSender());
       t.timeline.forEach((e) => push(e.getSender()));
+      push(t.replyToEvent?.getSender());
       partsMap.set(t.id, parts);
 
       return {
         id: t.id,
-        latestTs: t.lastReply()?.getTs() ?? t.rootEvent?.getTs() ?? 0,
+        latestTs: t.replyToEvent?.getTs() ?? t.rootEvent?.getTs() ?? 0,
         unread,
         participated: t.hasCurrentUserParticipated,
       };
@@ -269,6 +276,7 @@ export function ThreadsListPanel({ room, onClose, onOpenThread }: ThreadsListPan
     return {
       visible: visibleSnaps,
       unreadById: unreadMap,
+      highlightById: highlightMap,
       participantsById: partsMap,
       threadById: byId,
     };
@@ -368,6 +376,7 @@ export function ThreadsListPanel({ room, onClose, onOpenThread }: ThreadsListPan
                         room={room}
                         thread={thread}
                         unread={unreadById.get(snap.id) ?? 0}
+                        highlight={highlightById.get(snap.id) ?? 0}
                         participants={participantsById.get(snap.id) ?? []}
                         onOpen={onOpenThread}
                       />
diff --git a/src/app/hooks/useRoomThreads.ts b/src/app/hooks/useRoomThreads.ts
index 307d1856b..dcfd06c81 100644
--- a/src/app/hooks/useRoomThreads.ts
+++ b/src/app/hooks/useRoomThreads.ts
@@ -1,16 +1,23 @@
 import { useEffect, useRef, useState } from 'react';
 import { NotificationCountType, Room, RoomEvent, Thread, ThreadEvent } from 'matrix-js-sdk';
 
-// Cheap signature over the fields the Threads list actually depends on (id,
-// last-activity ts, unread count, participation). We only push a new snapshot
-// when this changes, so noisy repeated ThreadEvent.Update / UnreadNotifications
-// re-emits that don't alter the list don't cause a re-render.
+// Cheap signature over the fields the Threads list actually renders on (id,
+// last-activity ts, reply count, unread, participation, and a root-edit token).
+// We only push a new snapshot when this changes, so noisy repeated
+// ThreadEvent.Update / UnreadNotifications re-emits that don't alter the list
+// don't cause a re-render.
+// `replyToEvent` reads the server bundle's latest_event, so last-activity is
+// accurate immediately (before/without lazily paginating each thread's replies);
+// `length` covers reply-count changes (e.g. a mid-thread redaction) and
+// `replacingEventId` covers a root-message edit changing the row snippet.
 const signatureOf = (room: Room, threads: Thread[]): string =>
   threads
     .map((t) => {
-      const ts = t.lastReply()?.getTs() ?? t.rootEvent?.getTs() ?? 0;
+      const ts = t.replyToEvent?.getTs() ?? t.rootEvent?.getTs() ?? 0;
       const unread = room.getThreadUnreadNotificationCount(t.id, NotificationCountType.Total) ?? 0;
-      return `${t.id}:${ts}:${unread}:${t.hasCurrentUserParticipated ? 1 : 0}`;
+      const rootEdit = t.rootEvent?.replacingEventId() ?? '';
+      const participated = t.hasCurrentUserParticipated ? 1 : 0;
+      return `${t.id}:${ts}:${t.length}:${unread}:${participated}:${rootEdit}`;
     })
     .sort()
     .join('|');
diff --git a/src/app/utils/threadList.test.ts b/src/app/utils/threadList.test.ts
index 74ce1d0df..56e4d88a7 100644
--- a/src/app/utils/threadList.test.ts
+++ b/src/app/utils/threadList.test.ts
@@ -67,10 +67,13 @@ test('sortThreads breaks ties deterministically by id', () => {
   );
 });
 
-test('filterThreads / sortThreads do not mutate input', () => {
-  const input = [t('a', 100), t('b', 300)];
+test('filterThreads / sortThreads do not mutate input (all/participating branches)', () => {
+  const input = [t('a', 100, 1, true), t('b', 300, 0, false)];
   const before = input.map((x) => x.id);
+  filterThreads(input, 'all');
+  filterThreads(input, 'participating');
   filterThreads(input, 'unread');
+  sortThreads(input, 'recent');
   sortThreads(input, 'oldest');
   assert.deepEqual(
     input.map((x) => x.id),
@@ -78,6 +81,15 @@ test('filterThreads / sortThreads do not mutate input', () => {
   );
 });
 
+test('filter → sort pipeline composes (unread + recent)', () => {
+  const input = [t('a', 100, 2), t('b', 300, 0), t('c', 200, 1)];
+  const out = sortThreads(filterThreads(input, 'unread'), 'recent');
+  assert.deepEqual(
+    out.map((x) => x.id),
+    ['c', 'a'],
+  );
+});
+
 test('isThreadFilter / isThreadSort accept valid and reject junk', () => {
   assert.equal(isThreadFilter('all'), true);
   assert.equal(isThreadFilter('unread'), true);