diff --git a/desktop/src/app/useChannelActivityProjection.test.mjs b/desktop/src/app/useChannelActivityProjection.test.mjs index 6b54f3224..3d36ef09b 100644 --- a/desktop/src/app/useChannelActivityProjection.test.mjs +++ b/desktop/src/app/useChannelActivityProjection.test.mjs @@ -28,3 +28,18 @@ test("channel activity read state honors a channel marker without a message mark 300, ); }); + +test("channel activity read state ignores the passive timeline marker", () => { + const markers = new Map([ + ["general", 100], + ["channel-timeline:general", 500], + ]); + + assert.equal( + resolveChannelActivityFeedItemReadAt( + { id: "reply-general", channelId: "general" }, + (contextId) => markers.get(contextId) ?? null, + ), + 100, + ); +}); diff --git a/desktop/src/app/useChannelActivityProjection.ts b/desktop/src/app/useChannelActivityProjection.ts index b1961318c..4a4714e6d 100644 --- a/desktop/src/app/useChannelActivityProjection.ts +++ b/desktop/src/app/useChannelActivityProjection.ts @@ -54,12 +54,12 @@ export function useChannelActivityProjection({ const threadReadAt = getOwnReadAt(`thread:${rootId}`); if (!channelId) return threadReadAt; - const channelReadAt = getChannelReadAt(channelId); + const channelReadAt = getOwnReadAt(channelId); if (threadReadAt === null) return channelReadAt; if (channelReadAt === null) return threadReadAt; return Math.max(threadReadAt, channelReadAt); }, - [getChannelReadAt, getOwnReadAt], + [getOwnReadAt], ); const markThreadRead = React.useCallback( (rootId: string, timestamp: number) => diff --git a/desktop/src/features/channels/observedUnreadStorage.ts b/desktop/src/features/channels/observedUnreadStorage.ts index 57264fb40..f88dcbb12 100644 --- a/desktop/src/features/channels/observedUnreadStorage.ts +++ b/desktop/src/features/channels/observedUnreadStorage.ts @@ -366,16 +366,18 @@ export function pruneObservedUnreadByMarkers( eventsByChannel: Map>, latestByChannel: Map, getChannelReadAt: (channelId: string) => number | null, + getChannelTimelineReadAt: (channelId: string) => number | null, getOwnTimestamp: (contextId: string) => number | null, ): boolean { let changed = false; for (const [channelId, eventsById] of eventsByChannel) { const channelReadAt = getChannelReadAt(channelId); + const channelTimelineReadAt = getChannelTimelineReadAt(channelId); const toDelete: string[] = []; for (const event of eventsById.values()) { const readAt = observedUnreadEventReadAt( event, - channelReadAt, + event.rootId === null ? channelTimelineReadAt : channelReadAt, (rootId) => getOwnTimestamp(`thread:${rootId}`), (messageId) => getOwnTimestamp(`msg:${messageId}`), ); diff --git a/desktop/src/features/channels/observedUnreadTestHarness.mjs b/desktop/src/features/channels/observedUnreadTestHarness.mjs index da3cfa3e9..dcaa4f207 100644 --- a/desktop/src/features/channels/observedUnreadTestHarness.mjs +++ b/desktop/src/features/channels/observedUnreadTestHarness.mjs @@ -208,7 +208,7 @@ export function makeObservedEvent({ return { id, createdAt, - rootId: rootId ?? `root-${id}`, + rootId: rootId === undefined ? `root-${id}` : rootId, highPriority, countsTowardBadge, countsTowardAppBadge, @@ -254,6 +254,7 @@ export async function mountHook(props, refs) { isReady, readStateVersion, getTs, + getTimelineTs, getOwn, onPruned, }) { @@ -263,6 +264,7 @@ export async function mountHook(props, refs) { isReady, readStateVersion, getTs, + getTimelineTs ?? getTs, getOwn, refs.eventsRef, refs.latestRef, diff --git a/desktop/src/features/channels/readState/readStateFormat.test.mjs b/desktop/src/features/channels/readState/readStateFormat.test.mjs index 417e40b02..aada48eb1 100644 --- a/desktop/src/features/channels/readState/readStateFormat.test.mjs +++ b/desktop/src/features/channels/readState/readStateFormat.test.mjs @@ -2,6 +2,7 @@ import assert from "node:assert/strict"; import test from "node:test"; import { + channelTimelineContextKey, isMsgContextKey, isThreadContextKey, maxReadAt, @@ -22,6 +23,13 @@ test("msgContextKey_prefixesId_returnsMsgKey", () => { assert.equal(msgContextKey(EVENT_ID), `msg:${EVENT_ID}`); }); +test("channelTimelineContextKey_separatesPassiveTimelineReads", () => { + assert.equal( + channelTimelineContextKey("channel-1"), + "channel-timeline:channel-1", + ); +}); + test("isMsgContextKey_wellFormedKey_returnsTrue", () => { assert.equal(isMsgContextKey(`msg:${EVENT_ID}`), true); }); diff --git a/desktop/src/features/channels/readState/readStateFormat.ts b/desktop/src/features/channels/readState/readStateFormat.ts index 4cd61e9f4..50913bc5c 100644 --- a/desktop/src/features/channels/readState/readStateFormat.ts +++ b/desktop/src/features/channels/readState/readStateFormat.ts @@ -34,6 +34,7 @@ export const READ_STATE_MAX_SLOTS = 8; // two key families apart. export const MSG_PREFIX = "msg:"; export const THREAD_PREFIX = "thread:"; +export const CHANNEL_TIMELINE_PREFIX = "channel-timeline:"; const EVENT_ID_PATTERN = /^[0-9a-f]{64}$/; @@ -49,6 +50,18 @@ export function msgContextKey(messageId: string): string { return `${MSG_PREFIX}${messageId}`; } +/** + * Read frontier for a channel's top-level timeline only. + * + * Passive channel viewing advances this context instead of the bare channel + * context. Thread and message contexts inherit the bare channel frontier, so + * this separation lets the visible timeline become read without acknowledging + * unopened thread activity that happens to be older than a later root message. + */ +export function channelTimelineContextKey(channelId: string): string { + return `${CHANNEL_TIMELINE_PREFIX}${channelId}`; +} + // Spec-conformance helpers for well-known interoperable context keys. Runtime // folding/eviction remains prefix-based so opaque client-local keys still work. export function isThreadContextKey(value: string): value is `thread:${string}` { diff --git a/desktop/src/features/channels/ui/useChannelUnreadState.ts b/desktop/src/features/channels/ui/useChannelUnreadState.ts index 884aa02d8..cd8b51bde 100644 --- a/desktop/src/features/channels/ui/useChannelUnreadState.ts +++ b/desktop/src/features/channels/ui/useChannelUnreadState.ts @@ -22,6 +22,7 @@ import { computeChannelUnreadMarker, computeThreadUnreadMarker, } from "@/features/messages/lib/unreadMarker"; +import { isThreadReply } from "@/features/messages/lib/threading"; import type { TimelineMessage } from "@/features/messages/types"; import { isConversationalUnreadKind } from "@/shared/constants/kinds"; @@ -392,7 +393,10 @@ export function useChannelUnreadState({ if (!message) return false; const { firstUnreadReplyId } = computeThreadUnreadMarker( [message], - getMessageReadAt, + (id) => + activeChannelId && !isThreadReply(message.tags ?? []) + ? getChannelReadAt(activeChannelId) + : getMessageReadAt(id), currentPubkey, isMsgForcedUnread, ); @@ -400,6 +404,8 @@ export function useChannelUnreadState({ }, [ messageById, + activeChannelId, + getChannelReadAt, getMessageReadAt, currentPubkey, isMsgForcedUnread, diff --git a/desktop/src/features/channels/unreadReadMarker.test.mjs b/desktop/src/features/channels/unreadReadMarker.test.mjs index 660d4f5c6..73ae51c6a 100644 --- a/desktop/src/features/channels/unreadReadMarker.test.mjs +++ b/desktop/src/features/channels/unreadReadMarker.test.mjs @@ -14,9 +14,11 @@ import { import { addThreadActivityItems, channelCatchUpEventKinds, +} from "./useUnreadChannels.ts"; +import { resolveChannelReadMarker, resolveObservedUnreadRootId, -} from "./useUnreadChannels.ts"; +} from "./unreadReadState.ts"; import { isChannelUnreadTriggerKind, trackSeenEvent, diff --git a/desktop/src/features/channels/unreadReadState.ts b/desktop/src/features/channels/unreadReadState.ts new file mode 100644 index 000000000..2d749bfaf --- /dev/null +++ b/desktop/src/features/channels/unreadReadState.ts @@ -0,0 +1,37 @@ +import { + getThreadReference, + isBroadcastReply, +} from "@/features/messages/lib/threading"; + +function parseTimestamp(value: string | null | undefined) { + if (!value) return null; + + const timestamp = Date.parse(value); + return Number.isNaN(timestamp) ? null : timestamp; +} + +function toUnixSeconds(isoOrMs: string | null | undefined): number | null { + const ms = parseTimestamp(isoOrMs); + return ms === null ? null : Math.floor(ms / 1_000); +} + +// Fold the caller's timeline position with the newest event observed live so +// explicit reads also cover events that arrived ahead of channel metadata. +export function resolveChannelReadMarker( + callerReadAt: string | null | undefined, + observedLatest: number | undefined, +): { markAt: number | null; clearObserved: boolean } { + const callerUnix = toUnixSeconds(callerReadAt); + const markAt = Math.max(callerUnix ?? 0, observedLatest ?? 0) || null; + return { + markAt, + clearObserved: + markAt !== null && + observedLatest !== undefined && + observedLatest <= markAt, + }; +} + +export function resolveObservedUnreadRootId(tags: string[][]): string | null { + return isBroadcastReply(tags) ? null : getThreadReference(tags).rootId; +} diff --git a/desktop/src/features/channels/useObservedUnreadPersistence.test.mjs b/desktop/src/features/channels/useObservedUnreadPersistence.test.mjs index a9d422a4c..05441cda1 100644 --- a/desktop/src/features/channels/useObservedUnreadPersistence.test.mjs +++ b/desktop/src/features/channels/useObservedUnreadPersistence.test.mjs @@ -42,6 +42,7 @@ const DEFAULT_PROPS = { isReady: true, readStateVersion: 0, getTs: () => null, + getTimelineTs: () => null, getOwn: () => null, }; @@ -232,6 +233,16 @@ test("marker prune: thread and channel markers prune covered events; sibling cha createdAt: NOW_S + 10, rootId: "root-sv", }); + const evtTimeline = makeObservedEvent({ + id: "evt-timeline", + createdAt: NOW_S - 10, + rootId: null, + }); + const evtThreadBeforeTimeline = makeObservedEvent({ + id: "evt-thread-before-timeline", + createdAt: NOW_S - 10, + rootId: "root-before-timeline", + }); const stored = new Map(); const ch1 = new Map(); @@ -245,6 +256,10 @@ test("marker prune: thread and channel markers prune covered events; sibling cha const chB = new Map(); chB.set("evt-survivor", evtSurvivor); stored.set("channel-b", chB); + const chTimeline = new Map(); + chTimeline.set("evt-timeline", evtTimeline); + chTimeline.set("evt-thread-before-timeline", evtThreadBeforeTimeline); + stored.set("channel-timeline", chTimeline); writeObservedUnreadToStorage(PUBKEY, RELAY, stored); let pruneCount = 0; @@ -259,6 +274,8 @@ test("marker prune: thread and channel markers prune covered events; sibling cha isReady: true, readStateVersion: 1, getTs: (channelId) => (channelId === "channel-a" ? NOW_S - 5 : null), + getTimelineTs: (channelId) => + channelId === "channel-timeline" ? NOW_S - 5 : null, getOwn: (ctx) => (ctx === "thread:root-a" ? NOW_S - 5 : null), onPruned: () => { pruneCount += 1; @@ -281,6 +298,16 @@ test("marker prune: thread and channel markers prune covered events; sibling cha ); // Sibling: channel-b unaffected. assert.ok(eventsRef.current.has("channel-b"), "channel-b must survive"); + assert.ok( + !eventsRef.current.get("channel-timeline")?.has("evt-timeline"), + "the top-level event must be pruned by the timeline marker", + ); + assert.ok( + eventsRef.current + .get("channel-timeline") + ?.has("evt-thread-before-timeline"), + "the timeline marker must not prune older thread activity", + ); assert.equal(pruneCount, 1, "onPruned must fire exactly once"); await harness.unmount(); diff --git a/desktop/src/features/channels/useObservedUnreadPersistence.ts b/desktop/src/features/channels/useObservedUnreadPersistence.ts index c1a94570b..c3af51692 100644 --- a/desktop/src/features/channels/useObservedUnreadPersistence.ts +++ b/desktop/src/features/channels/useObservedUnreadPersistence.ts @@ -59,7 +59,8 @@ export function useObservedUnreadPersistence( normalizedRelayUrl: string, isReadStateReady: boolean, readStateVersion: number, - getEffectiveTimestamp: (channelId: string) => number | null, + getChannelReadAt: (channelId: string) => number | null, + getChannelTimelineReadAt: (channelId: string) => number | null, getOwnTimestamp: (contextId: string) => number | null, observedUnreadEventsByChannelRef: React.MutableRefObject< Map> @@ -133,7 +134,8 @@ export function useObservedUnreadPersistence( const changed = pruneObservedUnreadByMarkers( observedUnreadEventsByChannelRef.current, latestByChannelRef.current, - getEffectiveTimestamp, + getChannelReadAt, + getChannelTimelineReadAt, getOwnTimestamp, ); if (changed) { diff --git a/desktop/src/features/channels/useUnreadChannels.ts b/desktop/src/features/channels/useUnreadChannels.ts index 0464a00fe..5aebf1fcd 100644 --- a/desktop/src/features/channels/useUnreadChannels.ts +++ b/desktop/src/features/channels/useUnreadChannels.ts @@ -15,7 +15,15 @@ import { recordObservedUnreadEvent, type ObservedUnreadEvent, } from "@/features/channels/unreadChannelCounts"; +import { + channelTimelineContextKey, + maxReadAt, +} from "@/features/channels/readState/readStateFormat"; import { useReadState } from "@/features/channels/readState/useReadState"; +import { + resolveChannelReadMarker, + resolveObservedUnreadRootId, +} from "@/features/channels/unreadReadState"; import { makeRootIdStore } from "@/features/channels/unreadRootIdStore"; import { forcedUnreadStore, @@ -84,48 +92,6 @@ const authoredStore = makeRootIdStore("buzz-thread-authored.v1"); const mentionedStore = makeRootIdStore("buzz-thread-mentioned.v1"); const mutedStore = makeRootIdStore("buzz-thread-muted.v1"); -function parseTimestamp(value: string | null | undefined) { - if (!value) { - return null; - } - - const timestamp = Date.parse(value); - return Number.isNaN(timestamp) ? null : timestamp; -} - -function toUnixSeconds(isoOrMs: string | null | undefined): number | null { - const ms = parseTimestamp(isoOrMs); - return ms === null ? null : Math.floor(ms / 1_000); -} - -// Resolve where the read marker should land when a channel is marked read. -// Folds the caller's timeline position together with the newest event this -// client has observed live (`observedLatest`), so an explicit "mark read" still -// covers messages that arrived faster than channel metadata — this fold is -// load-bearing for the Esc shortcut, sidebar mark-read, and empty-channel open, -// all of which pass a null/stale caller value. `clearObserved` reports whether -// the resulting marker covers the observed timestamp, signalling the caller to -// drop its observed refs so the unread memo sees `latest === undefined` until a -// genuinely newer event arrives. -export function resolveChannelReadMarker( - callerReadAt: string | null | undefined, - observedLatest: number | undefined, -): { markAt: number | null; clearObserved: boolean } { - const callerUnix = toUnixSeconds(callerReadAt); - const markAt = Math.max(callerUnix ?? 0, observedLatest ?? 0) || null; - return { - markAt, - clearObserved: - markAt !== null && - observedLatest !== undefined && - observedLatest <= markAt, - }; -} - -export function resolveObservedUnreadRootId(tags: string[][]): string | null { - return isBroadcastReply(tags) ? null : getThreadReference(tags).rootId; -} - export function useUnreadChannels( channels: Channel[], activeChannel: Channel | null, @@ -147,7 +113,7 @@ export function useUnreadChannels( : ""; const { - getEffectiveTimestamp, + getEffectiveTimestamp: getContextReadAt, isReady: isReadStateReady, markContextRead, drainSyncedAdvances, @@ -155,6 +121,14 @@ export function useUnreadChannels( readStateVersion, getOwnTimestamp, } = useReadState(pubkey, relayClient); + const getChannelTimelineReadAt = React.useCallback( + (channelId: string) => + maxReadAt( + getContextReadAt(channelId), + getOwnTimestamp(channelTimelineContextKey(channelId)), + ), + [getContextReadAt, getOwnTimestamp], + ); // Per-channel latest observed external trigger timestamp (unix seconds) and // per-event metadata. Derived relay evidence, not source-of-truth; the unread @@ -247,7 +221,8 @@ export function useUnreadChannels( normalizedRelayUrl, isReadStateReady, readStateVersion, - getEffectiveTimestamp, + getContextReadAt, + getChannelTimelineReadAt, getOwnTimestamp, observedUnreadEventsByChannelRef, latestByChannelRef, @@ -319,7 +294,10 @@ export function useUnreadChannels( observedLatest, ); if (markAt === null) return; - markContextRead(channelId, markAt); + markContextRead( + topLevelOnly ? channelTimelineContextKey(channelId) : channelId, + markAt, + ); // Delegate destructive observed-ref removal to the fenced owner operation — // the parent must not delete from latestByChannelRef or // observedUnreadEventsByChannelRef directly on the clear-observed path, @@ -603,12 +581,13 @@ export function useUnreadChannels( void Promise.all( toFetch.map(async (channelId): Promise => { try { - const readAt = getEffectiveTimestamp(channelId); + const channelReadAt = getContextReadAt(channelId); + const channelTimelineReadAt = getChannelTimelineReadAt(channelId); const channel = channels.find((c) => c.id === channelId); // NIP-01 `since` is inclusive of `created_at >= since`. The +1 // makes the relay-side filter strict-newer; the client-side // `> readAt` check below is the belt to the suspenders. - const sinceParam = readAt === null ? 0 : readAt + 1; + const sinceParam = channelReadAt === null ? 0 : channelReadAt + 1; const events = await relayClient.fetchEvents({ kinds: [...channelCatchUpEventKinds(channel?.channelType)], @@ -658,7 +637,15 @@ export function useUnreadChannels( ) { continue; } - if (readAt !== null && event.created_at <= readAt) continue; + const evtRef = getThreadReference(event.tags); + const isThreadedReply = + evtRef.parentId !== null && !isBroadcastReply(event.tags); + const eventReadAt = isThreadedReply + ? channelReadAt + : channelTimelineReadAt; + if (eventReadAt !== null && event.created_at <= eventReadAt) { + continue; + } const eventChannelId = event.tags.find((t) => t[0] === "h")?.[1] ?? null; if ( @@ -673,9 +660,6 @@ export function useUnreadChannels( ) { continue; } - const evtRef = getThreadReference(event.tags); - const isThreadedReply = - evtRef.parentId !== null && !isBroadcastReply(event.tags); if (event.created_at > maxExternal) { maxExternal = event.created_at; } @@ -743,7 +727,7 @@ export function useUnreadChannels( didAdvance = true; } if (maxExternal > 0) { - const readAtNow = getEffectiveTimestamp(channelId) ?? 0; + const readAtNow = getContextReadAt(channelId) ?? 0; if (maxExternal > readAtNow) { const current = latestByChannelRef.current.get(channelId) ?? 0; if (maxExternal > current) { @@ -785,7 +769,8 @@ export function useUnreadChannels( }; }, [ channelIdsKey, - getEffectiveTimestamp, + getChannelTimelineReadAt, + getContextReadAt, isReadStateReady, normalizedPubkey, normalizedRelayUrl, @@ -829,11 +814,12 @@ export function useUnreadChannels( const observedEvents = observedUnreadEventsByChannelRef.current.get( channel.id, ); - const channelReadAt = getEffectiveTimestamp(channel.id); + const channelReadAt = getContextReadAt(channel.id); + const channelTimelineReadAt = getChannelTimelineReadAt(channel.id); const readAtForObservedEvent = (event: ObservedUnreadEvent) => observedUnreadEventReadAt( event, - channelReadAt, + event.rootId === null ? channelTimelineReadAt : channelReadAt, (rootId) => getOwnTimestamp(`thread:${rootId}`), (messageId) => getOwnTimestamp(`msg:${messageId}`), ); @@ -892,7 +878,8 @@ export function useUnreadChannels( }, [ activeChannelId, channels, - getEffectiveTimestamp, + getChannelTimelineReadAt, + getContextReadAt, getOwnTimestamp, isReadStateReady, latestVersion, @@ -918,7 +905,7 @@ export function useUnreadChannels( delete forcedUnreadRef.current[channelId]; const unixSeconds = latestByChannelRef.current.get(channelId) ?? - getEffectiveTimestamp(channelId) ?? + getChannelTimelineReadAt(channelId) ?? null; if (unixSeconds !== null) { markContextRead(channelId, unixSeconds); @@ -933,7 +920,7 @@ export function useUnreadChannels( // (Fenced record writes in handleChannelMessage and catch-up remain in the parent.) observedPersistence.clearAll(); bumpLatestVersion(); - }, [getEffectiveTimestamp, markContextRead, observedPersistence, pubkey]); + }, [getChannelTimelineReadAt, markContextRead, observedPersistence, pubkey]); // Identity-stable snapshots of the membership sets for the notify gate. // Re-derived only when membershipVersion bumps (a set actually changed), so @@ -969,7 +956,7 @@ export function useUnreadChannels( // off the same NIP-RS read marker without instantiating a second // ReadStateManager. readStateVersion is the invalidation signal callers // should include in memo deps. - getEffectiveTimestamp, + getEffectiveTimestamp: getChannelTimelineReadAt, getOwnTimestamp, readStateVersion, setContextParentResolver, diff --git a/desktop/tests/e2e/channel-activity-popover.spec.ts b/desktop/tests/e2e/channel-activity-popover.spec.ts index a62d064d7..69adf83f9 100644 --- a/desktop/tests/e2e/channel-activity-popover.spec.ts +++ b/desktop/tests/e2e/channel-activity-popover.spec.ts @@ -409,6 +409,25 @@ test.describe("channel activity hover preview", () => { await expect(groupedRow).toContainText("2 unread"); }); + test("keeps older thread activity after opening a channel with newer timeline activity", async ({ + page, + }) => { + await seedChannelActivity(page, { includeAgent: false }); + await emitMockMessage(page, "Newer top-level timeline message", { + pubkey: SELF_PUBKEY, + createdAt: Math.floor(Date.now() / 1_000) + 180, + }); + + await expect(page.getByTestId("channel-unread-dot-general")).toBeVisible(); + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + await expect(page.getByTestId("channel-unread-dot-general")).toBeVisible(); + + await page.getByTestId("channel-general").click({ button: "right" }); + await page.getByRole("menuitem", { name: "Mark as read" }).click(); + await expect(page.getByTestId("channel-unread-dot-general")).toHaveCount(0); + }); + test("marks projected thread activity read from the active channel menu", async ({ page, }) => { @@ -657,9 +676,11 @@ test.describe("channel activity hover preview", () => { .filter({ hasText: "Keep this separate timeline message unread." }); await manualUnreadRow.hover(); await page.getByTestId(`more-actions-${manualUnreadMessage.id}`).click(); - await page - .getByTestId(`mark-read-toggle-${manualUnreadMessage.id}`) - .click(); + const markUnreadItem = page.getByTestId( + `mark-read-toggle-${manualUnreadMessage.id}`, + ); + await expect(markUnreadItem).toHaveText("Mark unread"); + await markUnreadItem.click(); await page.getByRole("button", { name: "Inbox", exact: true }).click(); const groupedRootId = "grouped-inbox-root-preserve-manual";