Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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,37 @@
import assert from "node:assert/strict";
import test from "node:test";

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

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

test("maxReadAt_usesNewestNonNullMarker", () => {
assert.equal(maxReadAt(null, 10, 5, null, 30), 30);
});

test("maxReadAt_allNull_returnsNull", () => {
assert.equal(maxReadAt(null, null), null);
});

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 +42,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);
});
23 changes: 18 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,30 @@ 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 maxReadAt(...markers: Array<number | null>): number | null {
return markers.reduce<number | null>((latest, marker) => {
if (marker === null) return latest;
if (latest === null || marker > latest) return marker;
return latest;
}, null);
}

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).
// 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}` {

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
14 changes: 8 additions & 6 deletions desktop/src/features/channels/unreadChannelCounts.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { maxReadAt } from "@/features/channels/readState/readStateFormat";

export type ObservedUnreadEvent = {
id: string;
createdAt: number;
Expand Down Expand Up @@ -122,13 +124,13 @@ 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 maxReadAt(...markers);
}
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
65 changes: 65 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,67 @@ test("grouped unread override matches any item represented by the row", () => {
false,
);
});

test("thread inbox row without a marker ignores local done fallback", () => {
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: () => null,
}),
null,
);
});

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