Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions desktop/src/app/AppShell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,7 @@ export function AppShell() {
feedItemState.unreadSet,
threadActivityFeedItems,
getThreadReadAt,
getMessageReadAt,
);

const dueReminderBadge = useDueReminderBadgeCount(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,28 @@
import assert from "node:assert/strict";
import test from "node:test";

import { isMsgContextKey, msgContextKey } from "./readStateFormat.ts";
import {
isMsgContextKey,
isThreadContextKey,
msgContextKey,
} from "./readStateFormat.ts";

const EVENT_ID = "a".repeat(64);

test("msgContextKey_prefixesId_returnsMsgKey", () => {
assert.equal(msgContextKey("abc123"), "msg:abc123");
assert.equal(msgContextKey(EVENT_ID), `msg:${EVENT_ID}`);
});

test("isMsgContextKey_wellFormedKey_returnsTrue", () => {
assert.equal(isMsgContextKey("msg:abc123"), true);
assert.equal(isMsgContextKey(`msg:${EVENT_ID}`), true);
});

test("isThreadContextKey_wellFormedKey_returnsTrue", () => {
assert.equal(isThreadContextKey(`thread:${EVENT_ID}`), true);
});

test("isMsgContextKey_threadKey_returnsFalse", () => {
assert.equal(isMsgContextKey(`thread:${"a".repeat(64)}`), false);
assert.equal(isMsgContextKey(`thread:${EVENT_ID}`), false);
});

test("isMsgContextKey_channelKey_returnsFalse", () => {
Expand All @@ -23,11 +33,19 @@ test("isMsgContextKey_emptyId_returnsFalse", () => {
assert.equal(isMsgContextKey("msg:"), false);
});

test("isMsgContextKey_shortId_returnsFalse", () => {
assert.equal(isMsgContextKey("msg:abc123"), false);
});

test("isMsgContextKey_msgPrefixWrappingThreadKey_returnsFalse", () => {
// A thread key accidentally re-prefixed must not pass as a message key.
assert.equal(isMsgContextKey(`msg:thread:${"a".repeat(64)}`), false);
assert.equal(isMsgContextKey(`msg:thread:${EVENT_ID}`), false);
});

test("isThreadContextKey_shortId_returnsFalse", () => {
assert.equal(isThreadContextKey("thread:abc123"), false);
});

test("msgContextKey_output_roundTripsThroughValidator", () => {
assert.equal(isMsgContextKey(msgContextKey("event-id")), true);
assert.equal(isMsgContextKey(msgContextKey(EVENT_ID)), true);
});
13 changes: 8 additions & 5 deletions desktop/src/features/channels/readState/readStateFormat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,20 @@ export const MAX_CONTEXTS = 10_000;
export const MSG_PREFIX = "msg:";
export const THREAD_PREFIX = "thread:";

const EVENT_ID_PATTERN = /^[0-9a-f]{64}$/;

export function msgContextKey(messageId: string): string {
return `${MSG_PREFIX}${messageId}`;
}

// A well-formed per-message context key: the msg: prefix with a non-empty id
// that does NOT itself start with thread: (guards against a thread key being
// mistaken for, or double-prefixed into, a message key).
export function isThreadContextKey(value: string): value is `thread:${string}` {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor — isThreadContextKey and isMsgContextKey don't have any production callers right now (only readStateFormat.test.mjs exercises them); the write path doesn't validate and eviction matches on raw startsWith. So the strict 64-hex tightening is safe (no persisted key gets re-classified at runtime), but these are effectively spec-conformance assertions for the tests rather than live guards. Might be worth a one-line comment saying so, so nobody assumes they're gating the read path.

if (!value.startsWith(THREAD_PREFIX)) return false;
return EVENT_ID_PATTERN.test(value.slice(THREAD_PREFIX.length));
}

export function isMsgContextKey(value: string): value is `msg:${string}` {
if (!value.startsWith(MSG_PREFIX)) return false;
const id = value.slice(MSG_PREFIX.length);
return id.length > 0 && !id.startsWith(THREAD_PREFIX);
return EVENT_ID_PATTERN.test(value.slice(MSG_PREFIX.length));
}

export function localReadStateKey(pubkey: string): string {
Expand Down
20 changes: 11 additions & 9 deletions desktop/src/features/channels/readState/readStateManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import {
READ_STATE_D_TAG_PREFIX,
READ_STATE_FETCH_LIMIT,
READ_STATE_HORIZON_SECONDS,
MSG_PREFIX,
THREAD_PREFIX,
isValidBlob,
isValidReadStateDTag,
sanitizeContexts,
Expand Down Expand Up @@ -593,23 +595,23 @@ export class ReadStateManager {
contexts[ctx] = ts;
}

// Evict oldest thread:* entries when approaching the MAX_CONTEXTS cap
// (10,000). Channel keys are never evicted — only thread read-state keys
// are eligible. This prevents silent blob rejection by isValidBlob().
// Evict oldest per-thread/per-message entries when approaching the
// MAX_CONTEXTS cap (10,000). Channel keys are never evicted. This prevents
// silent blob rejection by isValidBlob().
const EVICTION_THRESHOLD = 8_000;
const contextCount = Object.keys(contexts).length;
if (contextCount > EVICTION_THRESHOLD) {
const threadEntries: [string, number][] = [];
const detailEntries: [string, number][] = [];
for (const [key, ts] of Object.entries(contexts)) {
if (key.startsWith("thread:")) {
threadEntries.push([key, ts]);
if (key.startsWith(THREAD_PREFIX) || key.startsWith(MSG_PREFIX)) {
detailEntries.push([key, ts]);
}
}
// Sort oldest-first (lowest timestamp = oldest)
threadEntries.sort((a, b) => a[1] - b[1]);
detailEntries.sort((a, b) => a[1] - b[1]);
const toEvict = contextCount - EVICTION_THRESHOLD;
for (let i = 0; i < Math.min(toEvict, threadEntries.length); i++) {
delete contexts[threadEntries[i][0]];
for (let i = 0; i < Math.min(toEvict, detailEntries.length); i++) {
delete contexts[detailEntries[i][0]];
}
}

Expand Down
16 changes: 10 additions & 6 deletions desktop/src/features/channels/unreadChannelCounts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,13 +122,17 @@ export function observedUnreadEventReadAt(
event: ObservedUnreadEvent,
channelReadAt: number | null,
getThreadOwnMarker: (rootId: string) => number | null,
getMessageOwnMarker: (messageId: string) => number | null = () => null,
): number | null {
if (event.rootId === null) return channelReadAt;
const markers = [channelReadAt, getMessageOwnMarker(event.id)];

const threadReadAt = getThreadOwnMarker(event.rootId);
if (threadReadAt === null) return channelReadAt;
if (channelReadAt === null || threadReadAt > channelReadAt) {
return threadReadAt;
if (event.rootId !== null) {
markers.push(getThreadOwnMarker(event.rootId));
}
return channelReadAt;

return markers.reduce<number | null>((latest, marker) => {
if (marker === null) return latest;
if (latest === null || marker > latest) return marker;
return latest;
}, null);
}
57 changes: 56 additions & 1 deletion desktop/src/features/channels/unreadReadMarker.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,34 @@ test("observedUnreadEventReadAt_openedThreadReplyUsesThreadMarker", () => {
assert.equal(event.createdAt > readAt, false);
});

test("observedUnreadEventReadAt_openedMessageReplyUsesMessageMarker", () => {
const event = observed("reply", 500, "root-1");

const readAt = observedUnreadEventReadAt(
event,
300,
() => null,
(messageId) => (messageId === "reply" ? 500 : null),
);

assert.equal(readAt, 500);
assert.equal(event.createdAt > readAt, false);
});

test("observedUnreadEventReadAt_usesNewestAvailableMarker", () => {
const event = observed("reply", 500, "root-1");

assert.equal(
observedUnreadEventReadAt(
event,
300,
() => 450,
() => 400,
),
450,
);
});

test("observedUnreadEventReadAt_topLevelUsesChannelMarker", () => {
assert.equal(
observedUnreadEventReadAt(observed("top", 500), 300, () => 900),
Expand Down Expand Up @@ -156,12 +184,13 @@ function observed(
};
}

function readAtFor(channelMarker, threadMarkers) {
function readAtFor(channelMarker, threadMarkers, messageMarkers = new Map()) {
return (event) =>
observedUnreadEventReadAt(
event,
channelMarker,
(rootId) => threadMarkers.get(rootId) ?? null,
(messageId) => messageMarkers.get(messageId) ?? null,
);
}

Expand Down Expand Up @@ -211,6 +240,32 @@ test("sidebarPipeline_openThreadClearsOnlyUnreadThreadContribution", () => {
);
});

test("sidebarPipeline_openedReplyMessageMarkerClearsDelayedReplay", () => {
const channelId = "chan";
const rootId = "root-1";
const reply = observed("reply", 500, rootId);
const observedByChannel = new Map();
recordObservedUnreadEvent(observedByChannel, channelId, reply, 20);

const beforeOpenReadAt = readAtFor(300, new Map(), new Map());
assert.equal(
countUnreadObservedEvents(
observedByChannel.get(channelId),
beforeOpenReadAt,
),
1,
);

const afterOpenReadAt = readAtFor(300, new Map(), new Map([["reply", 500]]));
assert.equal(
countUnreadObservedEvents(
observedByChannel.get(channelId),
afterOpenReadAt,
),
0,
);
});

test("latestObservedEvent_latestThreadReadDoesNotImplyChannelClear", () => {
const events = new Map([
["older", observed("older", 400, "root-older")],
Expand Down
7 changes: 5 additions & 2 deletions desktop/src/features/channels/useUnreadChannels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -826,8 +826,11 @@ export function useUnreadChannels(
);
const channelReadAt = getEffectiveTimestamp(channel.id);
const readAtForObservedEvent = (event: ObservedUnreadEvent) =>
observedUnreadEventReadAt(event, channelReadAt, (rootId) =>
getOwnTimestamp(`thread:${rootId}`),
observedUnreadEventReadAt(
event,
channelReadAt,
(rootId) => getOwnTimestamp(`thread:${rootId}`),
(messageId) => getOwnTimestamp(`msg:${messageId}`),
);

const unreadCount = countUnreadObservedEvents(
Expand Down
2 changes: 2 additions & 0 deletions desktop/src/features/home/ui/HomeView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ export function HomeView({
const {
getChannelReadAt,
getThreadReadAt,
getMessageReadAt,
feedItemState,
markChannelRead,
markThreadRead,
Expand Down Expand Up @@ -202,6 +203,7 @@ export function HomeView({
items: inboxItems,
getChannelReadAt,
getThreadReadAt,
getMessageReadAt,
readStateVersion,
localDoneSet: doneSet,
localUnreadSet: unreadSet,
Expand Down
44 changes: 44 additions & 0 deletions desktop/src/features/home/useHomeInboxReadState.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
getGroupedChannelReadTimestamp,
getGroupedInboxItemIds,
hasGroupedUnreadOverride,
resolveInboxItemReadAt,
} from "./useHomeInboxReadState.ts";

const CHANNEL_ID = "9a1657ac-f7aa-5db0-b632-d8bbeb6dfb50";
Expand Down Expand Up @@ -123,3 +124,46 @@ test("grouped unread override matches any item represented by the row", () => {
false,
);
});

test("thread inbox row read state includes per-message marker", () => {
const replyItem = feedItem({
id: "reply-event",
createdAt: 200,
tags: [
["h", CHANNEL_ID],
["e", "root-event", "", "root"],
["e", "parent-event", "", "reply"],
],
});

assert.equal(
resolveInboxItemReadAt(inboxItem([replyItem]), {
getChannelReadAt: () => 100,
getThreadReadAt: () => null,
getMessageReadAt: (messageId) =>
messageId === "reply-event" ? 200 : null,
}),
200,
);
});

test("thread inbox row read state uses newest thread or message marker", () => {
const replyItem = feedItem({
id: "reply-event",
createdAt: 200,
tags: [
["h", CHANNEL_ID],
["e", "root-event", "", "root"],
["e", "parent-event", "", "reply"],
],
});

assert.equal(
resolveInboxItemReadAt(inboxItem([replyItem]), {
getChannelReadAt: () => 100,
getThreadReadAt: () => 250,
getMessageReadAt: () => 200,
}),
250,
);
});
Loading
Loading