fix(desktop): rank read-marker eviction by read recency

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<Record<string, number>>;
+  /**
+   * 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<string, number>;
+}): SlotSplitResult | null {
+  const {
+    channelEntries,
+    threadMsgEntries,
+    clientId,
+    initialSlotCount,
+    maxSlots,
+    maxBytes,
+    slotIdGenerator,
+    contextSourceCreatedAt,
+  } = args;
+
+  const encoder = new TextEncoder();
+  const blobFor = (c: Record<string, number>) =>
+    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<Record<string, number>> => {
+    const slotContexts: Array<Record<string, number>> = 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<string, number>,
+  clientId: string,
+  maxBytes: number,
+  contextSourceCreatedAt?: ReadonlyMap<string, number>,
+): TrimResult {
+  const encoder = new TextEncoder();
+  const blobFor = (c: Record<string, number>) =>
+    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 };
+}

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<string, number>,
+): number {
+  return contextSourceCreatedAt?.get(contextId) ?? markerTimestamp;
+}
+
 export function maxReadAt(...markers: Array<number | null>): number | null {
   return markers.reduce<number | null>((latest, marker) => {
     if (marker === null) return latest;

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 = {};