diff --git a/desktop/src/features/channels/readState/readStateBudget.ts b/desktop/src/features/channels/readState/readStateBudget.ts new file mode 100644 index 00000000000..fdbcfa1e326 --- /dev/null +++ b/desktop/src/features/channels/readState/readStateBudget.ts @@ -0,0 +1,189 @@ +/** + * Byte-budget eviction for NIP-RS read-state blobs. + * + * Pure helpers, split out of `readStateManager` so the eviction policy lives + * next to the recency signal it ranks by. Callers should prefer the manager's + * `currentContexts()` / `splitContextsIntoSlots()`; these are exported for + * direct unit testing. + */ +import { + MSG_PREFIX, + THREAD_PREFIX, + readActionRecency, +} from "@/features/channels/readState/readStateFormat"; + +/** + * Result of a `splitContextsIntoBudgetedSlots` call. + */ +export interface SlotSplitResult { + /** Contexts record for each slot (primary slot first). */ + slots: Array>; + /** + * Extra slot IDs allocated beyond the first. Length is `slots.length - 1`. + * The caller is responsible for persisting these. + */ + extraSlotIds: string[]; +} + +/** + * Partition `channelEntries` across slots so each slot's blob fits within + * `maxBytes`. Thread/msg entries are added to the primary slot (index 0) and + * trimmed to budget. + * + * `initialSlotCount` is the number of slots already available (≥ 1). If the + * initial distribution doesn't fit, new slot IDs are generated via + * `slotIdGenerator` until everything fits or `maxSlots` is reached. + * + * Returns `{ slots, extraSlotIds }` on success, or `null` when even `maxSlots` + * slots can't accommodate all channel keys. + * + * Exported for unit testing; callers should prefer `splitContextsIntoSlots()`. + */ +export function splitContextsIntoBudgetedSlots(args: { + channelEntries: [string, number][]; + threadMsgEntries: [string, number][]; + clientId: string; + initialSlotCount: number; + maxSlots: number; + maxBytes: number; + slotIdGenerator: () => string; + contextSourceCreatedAt?: ReadonlyMap; +}): SlotSplitResult | null { + const { + channelEntries, + threadMsgEntries, + clientId, + initialSlotCount, + maxSlots, + maxBytes, + slotIdGenerator, + contextSourceCreatedAt, + } = args; + + const encoder = new TextEncoder(); + const blobFor = (c: Record) => + JSON.stringify({ v: 1, client_id: clientId, contexts: c }); + + let slotCount = initialSlotCount; + const extraSlotIds: string[] = []; + + // Distribute channel keys and check fit. Grow slot count until all fit. + const distribute = (count: number): Array> => { + const slotContexts: Array> = Array.from( + { length: count }, + () => ({}), + ); + for (let i = 0; i < channelEntries.length; i++) { + const [key, ts] = channelEntries[i]; + slotContexts[i % count][key] = ts; + } + return slotContexts; + }; + + let slotContexts = distribute(slotCount); + while ( + slotContexts.some((c) => encoder.encode(blobFor(c)).length > maxBytes) && + slotCount < maxSlots + ) { + extraSlotIds.push(slotIdGenerator()); + slotCount++; + slotContexts = distribute(slotCount); + } + + if (slotContexts.some((c) => encoder.encode(blobFor(c)).length > maxBytes)) { + return null; + } + + // Add thread/msg entries to the primary slot and trim to budget. + for (const [key, ts] of threadMsgEntries) { + slotContexts[0][key] = ts; + } + trimContextsToBudget( + slotContexts[0], + clientId, + maxBytes, + contextSourceCreatedAt, + ); + + return { slots: slotContexts, extraSlotIds }; +} + +/** + * Result of a `trimContextsToBudget` call. + */ +export interface TrimResult { + /** Number of entries removed from `contexts`. */ + evicted: number; + /** True when the serialized blob fits within `maxBytes` after trimming. */ + fitsAfterTrim: boolean; +} + +/** + * Trim a contexts map to fit within `maxBytes` when serialized as the JSON + * blob `{v:1, client_id, contexts}`. Evicts least-recently-read `msg:` entries + * first, then least-recently-read `thread:` entries. Channel keys are never + * evicted. Mutates `contexts` in place. + * + * Recency comes from `contextSourceCreatedAt` (see `readActionRecency`) and + * falls back to the marker value. Ranking by the marker value alone evicts a + * marker the user just created on an older message ahead of markers they last + * touched days ago, so the read never survives the publish. + * + * Returns `{ evicted, fitsAfterTrim }`. `fitsAfterTrim` is false when the + * remaining blob (channel keys only) still exceeds `maxBytes` — the caller + * must not publish in that case. + * + * Exported for unit testing; callers should prefer `currentContexts()`. + */ +export function trimContextsToBudget( + contexts: Record, + clientId: string, + maxBytes: number, + contextSourceCreatedAt?: ReadonlyMap, +): TrimResult { + const encoder = new TextEncoder(); + const blobFor = (c: Record) => + JSON.stringify({ v: 1, client_id: clientId, contexts: c }); + + let currentBytes = encoder.encode(blobFor(contexts)).length; + if (currentBytes <= maxBytes) { + return { evicted: 0, fitsAfterTrim: true }; + } + + const msgEntries: [string, number][] = []; + const threadEntries: [string, number][] = []; + for (const [key, ts] of Object.entries(contexts)) { + if (key.startsWith(MSG_PREFIX)) { + msgEntries.push([key, ts]); + } else if (key.startsWith(THREAD_PREFIX)) { + threadEntries.push([key, ts]); + } + } + // Least-recently-read first within each tier. + const byReadRecency = (a: [string, number], b: [string, number]) => + readActionRecency(a[0], a[1], contextSourceCreatedAt) - + readActionRecency(b[0], b[1], contextSourceCreatedAt); + msgEntries.sort(byReadRecency); + threadEntries.sort(byReadRecency); + + // O(n) pass: subtract each entry's byte contribution from currentBytes and + // collect entries to evict. The per-entry estimate is `,"key":timestamp` + // (key.length + 3 bytes for `"`, `"`, `:` plus 1 comma) + timestamp digits. + // This is an approximation — the final encode below is the authoritative check. + const toEvict: string[] = []; + for (const [key, ts] of [...msgEntries, ...threadEntries]) { + if (currentBytes <= maxBytes) break; + // Contribution: `,"key":timestamp` — comma + quoted key + colon + value + currentBytes -= key.length + 3 + String(ts).length + 1; + toEvict.push(key); + } + + for (const key of toEvict) { + delete contexts[key]; + } + + // Final authoritative check — handles JSON comma-accounting edge cases + // (e.g. last-entry comma disappears) that the per-entry estimate ignores. + const fitsAfterTrim = encoder.encode(blobFor(contexts)).length <= maxBytes; + return { evicted: toEvict.length, fitsAfterTrim }; +} diff --git a/desktop/src/features/channels/readState/readStateFormat.ts b/desktop/src/features/channels/readState/readStateFormat.ts index 4cd61e9f43e..c5dc6f4c691 100644 --- a/desktop/src/features/channels/readState/readStateFormat.ts +++ b/desktop/src/features/channels/readState/readStateFormat.ts @@ -37,6 +37,24 @@ export const THREAD_PREFIX = "thread:"; const EVENT_ID_PATTERN = /^[0-9a-f]{64}$/; +/** + * Recency of the READ ACTION behind a marker — when this read fact entered the + * client — falling back to the marker value when unknown (contexts seeded + * before this signal was recorded). + * + * Eviction must rank by this, never by the marker timestamp. The marker value + * is the age of the *message* that was read, so ranking by it discards a marker + * the user just created on an older message while keeping long-stale markers + * that happen to point at recent messages. + */ +export function readActionRecency( + contextId: string, + markerTimestamp: number, + contextSourceCreatedAt?: ReadonlyMap, +): number { + return contextSourceCreatedAt?.get(contextId) ?? markerTimestamp; +} + export function maxReadAt(...markers: Array): number | null { return markers.reduce((latest, marker) => { if (marker === null) return latest; diff --git a/desktop/src/features/channels/readState/readStateManager.test.mjs b/desktop/src/features/channels/readState/readStateManager.test.mjs index 8831a952bf6..121020e6c77 100644 --- a/desktop/src/features/channels/readState/readStateManager.test.mjs +++ b/desktop/src/features/channels/readState/readStateManager.test.mjs @@ -5,9 +5,11 @@ import { ReadStateManager, applyRemoteContextTimestamp, resolveEffectiveTimestamp, +} from "./readStateManager.ts"; +import { splitContextsIntoBudgetedSlots, trimContextsToBudget, -} from "./readStateManager.ts"; +} from "./readStateBudget.ts"; // ── ReadStateManager integration helpers ───────────────────────────────────── // Provide browser globals required by ReadStateManager (localStorage, @@ -268,6 +270,66 @@ test("publish flushes pending local state first", async () => { } }); +test("publishing does not refresh read recency", async () => { + globalThis.window.localStorage = makeLocalStorage(); + const { restore } = withFakeTimers(); + const pubkey = "7".repeat(64); + const manager = new ReadStateManager(pubkey, makeFakeRelay()); + const msgKey = `msg:${"c".repeat(64)}`; + const markerValue = 1_000_000; // timestamp of the message that was read + const readHappenedAt = 1_500_000; // when that read entered the client + + // @tauri-apps/api/core reads `window.__TAURI_INTERNALS__.invoke`. + const originalInternals = globalThis.window.__TAURI_INTERNALS__; + const invoked = []; + globalThis.window.__TAURI_INTERNALS__ = { + invoke: (cmd, args) => { + invoked.push(cmd); + if (cmd === "nip44_encrypt_to_self") return Promise.resolve("ciphertext"); + if (cmd === "sign_event") { + return Promise.resolve( + JSON.stringify({ + id: "e".repeat(64), + pubkey, + created_at: args.createdAt, + kind: args.kind, + tags: args.tags, + content: args.content, + sig: "f".repeat(128), + }), + ); + } + return Promise.resolve(null); + }, + }; + + try { + manager.markContextRead(msgKey, markerValue); + manager.contextSourceCreatedAt.set(msgKey, readHappenedAt); + // Fresh launch on an account whose blob is trimmed: the relay copy that + // seeds lastPublishedContexts does not carry this key, so it reads as + // changed on the next publish. + manager.lastPublishedContexts = {}; + manager.fetchOwnBlobBeforePublish = async () => {}; + + await manager.publish(); + + assert.ok( + invoked.includes("sign_event"), + "precondition: the blob must actually publish", + ); + assert.equal( + manager.contextSourceCreatedAt.get(msgKey), + readHappenedAt, + "a publish is not a read: recency must stay at the time of the read", + ); + } finally { + globalThis.window.__TAURI_INTERNALS__ = originalInternals; + manager.destroy(); + restore(); + } +}); + test("destroyed manager cannot persist after an in-flight fetch resolves", async () => { const storage = makeLocalStorage(); globalThis.window.localStorage = storage; @@ -414,7 +476,28 @@ test("applyRemoteContextTimestamp ignores older remote read markers from newer s assert.equal(result, "unchanged"); assert.equal(effectiveState.get("channel-1"), 200); - assert.equal(contextSourceCreatedAt.get("channel-1"), 11); + // Recency tracks the last read ADVANCE, not the last blob that mentioned the + // context — a routine republish carrying nothing new must not refresh it. + assert.equal(contextSourceCreatedAt.get("channel-1"), 10); +}); + +test("applyRemoteContextTimestamp keeps recency stable across repeated republishes", () => { + const effectiveState = new Map([["channel-1", 200]]); + const contextSourceCreatedAt = new Map([["channel-1", 10]]); + + for (const eventCreatedAt of [50, 60, 70]) { + applyRemoteContextTimestamp({ + effectiveState, + contextSourceCreatedAt, + contextId: "channel-1", + timestamp: 200, + eventCreatedAt, + }); + } + + // Without this, every context in every republished blob would look freshly + // read and recency-ranked eviction would degenerate to publish order. + assert.equal(contextSourceCreatedAt.get("channel-1"), 10); }); test("applyRemoteContextTimestamp advances to newer remote read markers", () => { @@ -506,6 +589,44 @@ test("trimContextsToBudget_overBudget_evictsMsgEntriesOldestFirst", () => { ); }); +test("trimContextsToBudget_evictsLeastRecentlyRead_notOldestMessage", () => { + // msg A points at the OLDEST message but was read just now; msg B and C point + // at newer messages read long ago. Ranking by marker value would evict A — + // the read the user just performed. + const justReadOldMessage = `msg:${MSG_ID}`; + const contexts = { + [justReadOldMessage]: 1, + [`msg:${"c".repeat(64)}`]: 3, + [`msg:${"d".repeat(64)}`]: 2, + }; + const readRecency = new Map([ + [justReadOldMessage, 9_000], + [`msg:${"c".repeat(64)}`, 10], + [`msg:${"d".repeat(64)}`, 20], + ]); + const encoder = new TextEncoder(); + const budget = + encoder.encode(JSON.stringify({ v: 1, client_id: CLIENT_ID, contexts })) + .length - 10; + + const { fitsAfterTrim } = trimContextsToBudget( + contexts, + CLIENT_ID, + budget, + readRecency, + ); + + assert.equal(fitsAfterTrim, true); + assert.ok( + justReadOldMessage in contexts, + "the marker read most recently must survive", + ); + assert.ok( + !(`msg:${"c".repeat(64)}` in contexts), + "the least recently read marker should be evicted", + ); +}); + test("trimContextsToBudget_channelKeysNeverEvicted", () => { // Fill with msg entries plus one channel key; budget forces eviction. const contexts = {}; diff --git a/desktop/src/features/channels/readState/readStateManager.ts b/desktop/src/features/channels/readState/readStateManager.ts index 3ba382dc61e..e7ebbea172d 100644 --- a/desktop/src/features/channels/readState/readStateManager.ts +++ b/desktop/src/features/channels/readState/readStateManager.ts @@ -13,6 +13,10 @@ import { localExtraSlotIdsKey, type ReadStateBlob, } from "@/features/channels/readState/readStateFormat"; +import { + splitContextsIntoBudgetedSlots, + trimContextsToBudget, +} from "@/features/channels/readState/readStateBudget"; import { parseReadStateEvent } from "@/features/channels/readState/readStateSnapshot"; import { readStoredReadState, @@ -136,171 +140,16 @@ export function applyRemoteContextTimestamp(args: { if (result === "advanced") { effectiveState.set(contextId, next); - } - if (eventCreatedAt > sourceCreatedAt) { - contextSourceCreatedAt.set(contextId, eventCreatedAt); - } - return result; -} - -/** - * Result of a `splitContextsIntoBudgetedSlots` call. - */ -export interface SlotSplitResult { - /** Contexts record for each slot (primary slot first). */ - slots: Array>; - /** - * Extra slot IDs allocated beyond the first. Length is `slots.length - 1`. - * The caller is responsible for persisting these. - */ - extraSlotIds: string[]; -} - -/** - * Partition `channelEntries` across slots so each slot's blob fits within - * `maxBytes`. Thread/msg entries are added to the primary slot (index 0) and - * trimmed to budget. - * - * `initialSlotCount` is the number of slots already available (≥ 1). If the - * initial distribution doesn't fit, new slot IDs are generated via - * `slotIdGenerator` until everything fits or `maxSlots` is reached. - * - * Returns `{ slots, extraSlotIds }` on success, or `null` when even `maxSlots` - * slots can't accommodate all channel keys. - * - * Exported for unit testing; callers should prefer `splitContextsIntoSlots()`. - */ -export function splitContextsIntoBudgetedSlots(args: { - channelEntries: [string, number][]; - threadMsgEntries: [string, number][]; - clientId: string; - initialSlotCount: number; - maxSlots: number; - maxBytes: number; - slotIdGenerator: () => string; -}): SlotSplitResult | null { - const { - channelEntries, - threadMsgEntries, - clientId, - initialSlotCount, - maxSlots, - maxBytes, - slotIdGenerator, - } = args; - - const encoder = new TextEncoder(); - const blobFor = (c: Record) => - JSON.stringify({ v: 1, client_id: clientId, contexts: c }); - - let slotCount = initialSlotCount; - const extraSlotIds: string[] = []; - - // Distribute channel keys and check fit. Grow slot count until all fit. - const distribute = (count: number): Array> => { - const slotContexts: Array> = Array.from( - { length: count }, - () => ({}), - ); - for (let i = 0; i < channelEntries.length; i++) { - const [key, ts] = channelEntries[i]; - slotContexts[i % count][key] = ts; - } - return slotContexts; - }; - - let slotContexts = distribute(slotCount); - while ( - slotContexts.some((c) => encoder.encode(blobFor(c)).length > maxBytes) && - slotCount < maxSlots - ) { - extraSlotIds.push(slotIdGenerator()); - slotCount++; - slotContexts = distribute(slotCount); - } - - if (slotContexts.some((c) => encoder.encode(blobFor(c)).length > maxBytes)) { - return null; - } - - // Add thread/msg entries to the primary slot and trim to budget. - for (const [key, ts] of threadMsgEntries) { - slotContexts[0][key] = ts; - } - trimContextsToBudget(slotContexts[0], clientId, maxBytes); - - return { slots: slotContexts, extraSlotIds }; -} - -/** - * Result of a `trimContextsToBudget` call. - */ -export interface TrimResult { - /** Number of entries removed from `contexts`. */ - evicted: number; - /** True when the serialized blob fits within `maxBytes` after trimming. */ - fitsAfterTrim: boolean; -} - -/** - * Trim a contexts map to fit within `maxBytes` when serialized as the JSON - * blob `{v:1, client_id, contexts}`. Evicts oldest `msg:` entries first - * (lowest timestamp), then oldest `thread:` entries. Channel keys are never - * evicted. Mutates `contexts` in place. - * - * Returns `{ evicted, fitsAfterTrim }`. `fitsAfterTrim` is false when the - * remaining blob (channel keys only) still exceeds `maxBytes` — the caller - * must not publish in that case. - * - * Exported for unit testing; callers should prefer `currentContexts()`. - */ -export function trimContextsToBudget( - contexts: Record, - clientId: string, - maxBytes: number, -): TrimResult { - const encoder = new TextEncoder(); - const blobFor = (c: Record) => - JSON.stringify({ v: 1, client_id: clientId, contexts: c }); - - let currentBytes = encoder.encode(blobFor(contexts)).length; - if (currentBytes <= maxBytes) { - return { evicted: 0, fitsAfterTrim: true }; - } - - const msgEntries: [string, number][] = []; - const threadEntries: [string, number][] = []; - for (const [key, ts] of Object.entries(contexts)) { - if (key.startsWith(MSG_PREFIX)) { - msgEntries.push([key, ts]); - } else if (key.startsWith(THREAD_PREFIX)) { - threadEntries.push([key, ts]); + // Only an advance is a NEW read fact. Re-learning an unchanged context from + // a routine republish must not refresh its recency, or every context in + // every blob would look freshly read and `contextSourceCreatedAt` would + // collapse to "time of last publish" — useless as an eviction key. The + // publish path does not write recency at all, for the same reason. + if (eventCreatedAt > sourceCreatedAt) { + contextSourceCreatedAt.set(contextId, eventCreatedAt); } } - // Oldest-first within each tier. - msgEntries.sort((a, b) => a[1] - b[1]); - threadEntries.sort((a, b) => a[1] - b[1]); - - // O(n) pass: subtract each entry's byte contribution from currentBytes and - // collect entries to evict. The per-entry estimate is `,"key":timestamp` - // (key.length + 3 bytes for `"`, `"`, `:` plus 1 comma) + timestamp digits. - // This is an approximation — the final encode below is the authoritative check. - const toEvict: string[] = []; - for (const [key, ts] of [...msgEntries, ...threadEntries]) { - if (currentBytes <= maxBytes) break; - // Contribution: `,"key":timestamp` — comma + quoted key + colon + value - currentBytes -= key.length + 3 + String(ts).length + 1; - toEvict.push(key); - } - - for (const key of toEvict) { - delete contexts[key]; - } - - // Final authoritative check — handles JSON comma-accounting edge cases - // (e.g. last-entry comma disappears) that the per-entry estimate ignores. - const fitsAfterTrim = encoder.encode(blobFor(contexts)).length <= maxBytes; - return { evicted: toEvict.length, fitsAfterTrim }; + return result; } export class ReadStateManager { @@ -722,11 +571,15 @@ export class ReadStateManager { `[ReadStateManager] publish accepted slotId=${slotId} createdAt=${createdAt}`, ); - for (const key of Object.keys(contexts)) { - if (this.lastPublishedContexts[key] !== contexts[key]) { - this.contextSourceCreatedAt.set(key, createdAt); - } - } + // Publishing is not a read action, so it must not refresh + // `contextSourceCreatedAt`. Stamping "keys that changed since the last + // publish" is not the narrow filter it looks like: lastPublishedContexts + // is seeded from the *trimmed* relay blob, so every key + // trimContextsToBudget dropped reads as changed on the first publish + // after each launch, and the recency signal collapses to "time of last + // publish" for the whole prunable tier. Recency is written only where a + // read happens — markContextRead, and the advance branch of + // applyRemoteContextTimestamp. // Merge this slot's contexts into lastPublishedContexts (union). for (const [key, ts] of Object.entries(contexts)) { this.lastPublishedContexts[key] = ts; @@ -855,6 +708,7 @@ export class ReadStateManager { contexts, this.clientId, READ_STATE_MAX_PLAINTEXT_BYTES, + this.contextSourceCreatedAt, ); if (evicted > 0) { console.warn( @@ -907,6 +761,7 @@ export class ReadStateManager { maxSlots: READ_STATE_MAX_SLOTS, maxBytes: READ_STATE_MAX_PLAINTEXT_BYTES, slotIdGenerator: () => generateHex(16), + contextSourceCreatedAt: this.contextSourceCreatedAt, }); if (result === null) { diff --git a/desktop/src/features/channels/readState/readStateStorage.test.mjs b/desktop/src/features/channels/readState/readStateStorage.test.mjs index ad7eb031ebc..43fc435b610 100644 --- a/desktop/src/features/channels/readState/readStateStorage.test.mjs +++ b/desktop/src/features/channels/readState/readStateStorage.test.mjs @@ -76,6 +76,74 @@ test("pruneStaleContexts caps within-horizon prunable entries, newest kept", () assert.equal(pruned.has(`msg:${String(total - 1).padStart(64, "0")}`), false); }); +test("pruneStaleContexts keeps a marker just read on an old message", () => { + // The user marks a 30-day-old message read today. The marker carries the + // MESSAGE's timestamp, so a value-ranked horizon drops it on the same write. + const oldMessage = `msg:${"e".repeat(64)}`; + const contexts = new Map([ + [oldMessage, NOW - READ_STATE_HORIZON_SECONDS * 4], + ]); + const readJustNow = new Map([[oldMessage, NOW]]); + + assert.equal( + pruneStaleContexts(contexts, NOW).has(oldMessage), + false, + "value-ranked horizon drops it (documents the old behavior)", + ); + assert.equal( + pruneStaleContexts(contexts, NOW, readJustNow).has(oldMessage), + true, + "read-recency horizon keeps it", + ); +}); + +test("pruneStaleContexts cap evicts least recently read, not oldest message", () => { + const contexts = new Map(); + const recency = new Map(); + const total = LOCAL_MAX_PRUNABLE_CONTEXTS + 1; + // Newest messages first (i=0 newest), all read a long time ago... + for (let i = 0; i < total - 1; i++) { + const key = `msg:${String(i).padStart(64, "0")}`; + contexts.set(key, NOW - i); + recency.set(key, NOW - READ_STATE_HORIZON_SECONDS / 2); + } + // ...and one old message read just now, which must survive the cap. + const justRead = `msg:${"f".repeat(64)}`; + contexts.set(justRead, NOW - READ_STATE_HORIZON_SECONDS + 60); + recency.set(justRead, NOW); + + const valueRanked = pruneStaleContexts(contexts, NOW); + assert.equal(valueRanked.size, LOCAL_MAX_PRUNABLE_CONTEXTS); + assert.equal( + valueRanked.has(justRead), + false, + "value-ranked cap evicts it (documents the old behavior)", + ); + + const recencyRanked = pruneStaleContexts(contexts, NOW, recency); + assert.equal(recencyRanked.size, LOCAL_MAX_PRUNABLE_CONTEXTS); + assert.equal(recencyRanked.has(justRead), true, "read-recency cap keeps it"); +}); + +test("writeStoredReadState keeps a marker just read on an old message", () => { + installLocalStorage(); + const pubkey = "c".repeat(64); + const nowSeconds = Math.floor(Date.now() / 1_000); + const oldMessage = `msg:${"a".repeat(64)}`; + + writeStoredReadState( + pubkey, + new Map([[oldMessage, nowSeconds - READ_STATE_HORIZON_SECONDS * 4]]), + new Set([oldMessage]), + new Map([[oldMessage, nowSeconds]]), + ); + + const state = JSON.parse( + window.localStorage.getItem(localReadStateKey(pubkey)), + ); + assert.deepEqual(Object.keys(state), [oldMessage]); +}); + test("writeStoredReadState prunes all three keys consistently", () => { installLocalStorage(); const pubkey = "f".repeat(64); diff --git a/desktop/src/features/channels/readState/readStateStorage.ts b/desktop/src/features/channels/readState/readStateStorage.ts index f5ac8996134..490d17793b3 100644 --- a/desktop/src/features/channels/readState/readStateStorage.ts +++ b/desktop/src/features/channels/readState/readStateStorage.ts @@ -8,6 +8,7 @@ import { MSG_PREFIX, READ_STATE_HORIZON_SECONDS, THREAD_PREFIX, + readActionRecency, } from "@/features/channels/readState/readStateFormat"; import { setLocalStorageItemWithRecovery } from "@/shared/lib/localStorageQuota"; @@ -110,33 +111,51 @@ function isPrunableContextKey(contextId: string): boolean { } /** - * Drops msg:/thread: markers older than the relay's 7-day horizon, then caps - * the survivors at LOCAL_MAX_PRUNABLE_CONTEXTS (oldest first). Channel keys - * are never pruned — they are small, bounded by membership, and losing one - * would resurrect the channel's unread badge. Mirrors the eviction order the - * publish path already applies in trimContextsToBudget. + * Drops msg:/thread: markers whose READ ACTION is older than the 7-day horizon, + * then caps the survivors at LOCAL_MAX_PRUNABLE_CONTEXTS (least recently read + * evicted first). Channel keys are never pruned — they are small, bounded by + * membership, and losing one would resurrect the channel's unread badge. + * Mirrors the eviction order the publish path applies in trimContextsToBudget. + * + * Both the horizon and the cap rank by `contextSourceCreatedAt` (see + * `readActionRecency`), NOT by the marker value. Ranking by the marker value + * means marking an older message read is undone by the very write that records + * it: the new marker carries that message's old timestamp, so it sorts below + * the cap — or below the horizon — and is dropped before it ever reaches disk. */ export function pruneStaleContexts( contexts: ReadonlyMap, nowUnixSeconds: number, + contextSourceCreatedAt?: ReadonlyMap, ): Map { const cutoff = nowUnixSeconds - READ_STATE_HORIZON_SECONDS; const kept = new Map(); - const prunable: [string, number][] = []; + const prunable: Array<{ + contextId: string; + timestamp: number; + recency: number; + }> = []; for (const [contextId, timestamp] of contexts) { if (!isPrunableContextKey(contextId)) { kept.set(contextId, timestamp); - } else if (timestamp >= cutoff) { - prunable.push([contextId, timestamp]); + continue; + } + const recency = readActionRecency( + contextId, + timestamp, + contextSourceCreatedAt, + ); + if (recency >= cutoff) { + prunable.push({ contextId, timestamp, recency }); } } if (prunable.length > LOCAL_MAX_PRUNABLE_CONTEXTS) { - prunable.sort((a, b) => b[1] - a[1]); + prunable.sort((a, b) => b.recency - a.recency); prunable.length = LOCAL_MAX_PRUNABLE_CONTEXTS; } - for (const [contextId, timestamp] of prunable) { + for (const { contextId, timestamp } of prunable) { kept.set(contextId, timestamp); } return kept; @@ -148,7 +167,11 @@ export function writeStoredReadState( publishableContextIds: ReadonlySet, contextSourceCreatedAt: ReadonlyMap, ): void { - const pruned = pruneStaleContexts(contexts, Math.floor(Date.now() / 1_000)); + const pruned = pruneStaleContexts( + contexts, + Math.floor(Date.now() / 1_000), + contextSourceCreatedAt, + ); const state: Record = {}; for (const [contextId, timestamp] of pruned) {