From 85d98be6b2f60699c67ed5bd32b992039af30d2e Mon Sep 17 00:00:00 2001 From: Gary Basin Date: Thu, 16 Jul 2026 23:11:25 -0400 Subject: [PATCH] fix(web): a deleted message never counts as unread The server stopped counting deleted messages toward a channel's unread counter, but the timeline's "N new" pill still counted a deleted message that kept its tombstone to host replies. The pill could say "1 new" while the sidebar said nothing, and the new thing was the words "Message deleted". Deletion takes the content away, so there is nothing left to have missed. Any replies still under the tombstone announce themselves through their own broadcast events. Renderability and unread are now separate questions, because they have different answers: a deleted message with replies still paints a row, so it is still the newest thing a reader can scroll to, but it is never something to catch up on. Conflating the two is what stranded a channel unread forever. --- surface/shared/src/timeline.ts | 9 +++++++++ surface/web/src/components/Timeline.test.tsx | 17 ++++++++++++++++- surface/web/src/components/Timeline.tsx | 14 +++++++++----- 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/surface/shared/src/timeline.ts b/surface/shared/src/timeline.ts index b563e23e..19d7ac0f 100644 --- a/surface/shared/src/timeline.ts +++ b/surface/shared/src/timeline.ts @@ -240,6 +240,15 @@ export function isRenderableMessage(m: ChatMessage): boolean { return !(m.deleted === true && m.replyCount === 0); } +/** + * Does this message contribute to unread state? Unlike row rendering, deletion + * always removes the unread content; a tombstone may remain to host replies, + * which contribute to unread state through their own broadcast events. + */ +export function countsAsUnread(m: ChatMessage): boolean { + return m.deleted !== true; +} + export interface ChannelTimeline { /** Root messages: confirmed sorted by id asc, then pending/failed in send order. */ main: ChatMessage[]; diff --git a/surface/web/src/components/Timeline.test.tsx b/surface/web/src/components/Timeline.test.tsx index 14ddf1d8..2f57132e 100644 --- a/surface/web/src/components/Timeline.test.tsx +++ b/surface/web/src/components/Timeline.test.tsx @@ -445,7 +445,7 @@ describe('Timeline anchored agent answers', () => { // unread, scroll landing, and mark-read has to agree it isn't there — otherwise // deleting the newest message in a channel strands the read cursor forever. describe('Timeline deleted tail message', () => { - const deletedTail = () => message({ id: 3, text: '', deleted: true, replyCount: 0 }); + const deletedTail = (replyCount = 0) => message({ id: 3, text: '', deleted: true, replyCount }); function mockRowVisibility(visibleEid: string) { return vi.spyOn(HTMLElement.prototype, 'getBoundingClientRect').mockImplementation(function (this: HTMLElement) { @@ -488,6 +488,21 @@ describe('Timeline deleted tail message', () => { expect(screen.queryByTestId('jump-to-unread')).toBeNull(); }); + it('does not count a deleted tombstone with replies as unread', () => { + const view = renderTimeline({ + messages: [message({ id: 1, text: 'Message 1' }), message({ id: 2, text: 'Message 2' }), deletedTail(1)], + unreadDividerAfterId: 2, + }); + + expect(view.container.querySelector('[data-eid="3"]')).not.toBeNull(); + const log = screen.getByRole('log', { name: 'Messages' }); + setScrollMetrics(log, { scrollHeight: 1000, clientHeight: 200 }); + log.scrollTop = 0; + fireEvent.scroll(log); + + expect(screen.queryByTestId('jump-to-unread')).toBeNull(); + }); + it('lands at the bottom when the only unread message is a deleted one', () => { const rect = mockRowVisibility('2'); Object.defineProperty(HTMLDivElement.prototype, 'scrollHeight', { configurable: true, value: 1000 }); diff --git a/surface/web/src/components/Timeline.tsx b/surface/web/src/components/Timeline.tsx index d32f24a6..dad7d373 100644 --- a/surface/web/src/components/Timeline.tsx +++ b/surface/web/src/components/Timeline.tsx @@ -1,7 +1,7 @@ import { memo, useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react'; import type { ChatMessage, UserRef } from '@atrium/surface-client'; import type { Session } from '../sessions/types'; -import { buildTimelineItems, isAgentVoiceBroadcast, isRenderableMessage } from '@atrium/surface-client'; +import { buildTimelineItems, countsAsUnread, isAgentVoiceBroadcast, isRenderableMessage } from '@atrium/surface-client'; import { ChevronDownIcon } from './icons'; import { MessageRow } from './MessageRow'; import type { MentionContext } from './useMentionTypeahead'; @@ -149,6 +149,7 @@ function TimelineImpl({ const anchoredUnread = messages.find( (message) => (message.id ?? 0) > unreadDividerAfterId && + countsAsUnread(message) && isRenderableMessage(message) && (message.sessionEventType === 'replied' || message.sessionEventType === 'question_requested' || @@ -157,15 +158,18 @@ function TimelineImpl({ loadedRootIds.has(message.threadRootEventId), ); if (anchoredUnread?.threadRootEventId != null) return anchoredUnread.threadRootEventId; - return visibleMessages.find((message) => (message.id ?? 0) > unreadDividerAfterId)?.id ?? null; + return ( + visibleMessages.find((message) => countsAsUnread(message) && (message.id ?? 0) > unreadDividerAfterId)?.id ?? null + ); }, [loadedRootIds, messages, unreadDividerAfterId, visibleMessages]); // Counted over `messages`, not `visibleMessages`: an anchored answer is // presented inside its root's cluster, so it is genuinely something new to - // see. A message that paints nothing is not, and counting one strands the - // pill at "1 new" pointing at a row that does not exist. + // see. `countsAsUnread` subsumes the renderability question here — every + // message that paints no row is a deleted one, and a deleted message never + // counts, so the pill can't point at a row that isn't there. const unreadCount = useMemo(() => { if (unreadDividerAfterId == null || unreadDividerAfterId <= 0) return 0; - return messages.filter((m) => isRenderableMessage(m) && (m.id ?? 0) > unreadDividerAfterId).length; + return messages.filter((message) => countsAsUnread(message) && (message.id ?? 0) > unreadDividerAfterId).length; }, [messages, unreadDividerAfterId]); const isAtBottom = useCallback((el: HTMLElement) => {