Skip to content

Commit 62e9ee7

Browse files
npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7wpfleger96
andcommitted
fix(read-state): address Thufir review findings on multi-slot splitting
Four findings from pass 1 review: CRITICAL 1 — publishSplitSlots retry storm: add no-op suppression by computing the union of all slot contexts and comparing to lastPublishedContexts before resetting and publishing. Without this, every debounce cycle in split mode re-published all slots even when nothing changed. CRITICAL 2 — split→single transition stale keys: reset lastPublishedContexts = {} before single-slot publish so stale keys from a previous split don't cause isIdenticalToLastPublished to return false forever. Add deleteExtraSlots() which publishes NIP-09 kind:5 delete events (a-tag) for each extra slot d-tag and clears extraSlotIds from localStorage. Called at the top of publish() when transitioning from split to single mode. IMPORTANT 1 — thread/msg entries silently dropped in split mode: splitContextsIntoSlots now collects thread/msg entries separately and adds them to the primary slot (slot 0) after channel key distribution. trimContextsToBudget is applied to slot 0 to keep it within budget. IMPORTANT 2 — no tests for multi-slot machinery: extract core split logic into exported pure function splitContextsIntoBudgetedSlots (takes channelEntries, threadMsgEntries, clientId, slotIdGenerator, etc.) so it can be tested without class instantiation. Add 5 unit tests: - fitsInOneSlot_returnsSingleSlot - requiresGrowth_allocatesExtraSlot - exceedsMaxSlots_returnsNull - includesThreadMsgInPrimarySlot - threadMsgTrimmedWhenPrimarySlotOverBudget Also addresses Thufir MINOR on distribute loop elegance: replaced the clear-and-redistribute-in-place pattern with a clean distribute(count) closure that allocates fresh slot maps each call. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
1 parent b299c09 commit 62e9ee7

2 files changed

Lines changed: 340 additions & 54 deletions

File tree

desktop/src/features/channels/readState/readStateManager.test.mjs

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
applyRemoteContextTimestamp,
66
evictDominatedEntries,
77
resolveEffectiveTimestamp,
8+
splitContextsIntoBudgetedSlots,
89
trimContextsToBudget,
910
} from "./readStateManager.ts";
1011

@@ -401,3 +402,173 @@ test("evictDominatedEntries_channelKeysNeverTouched", () => {
401402
assert.ok(CHANNEL_ID in contexts, "channel key untouched");
402403
assert.ok("other-key" in contexts, "non-prefixed key untouched");
403404
});
405+
406+
// ── splitContextsIntoBudgetedSlots ────────────────────────────────────────────
407+
408+
// Build a channel key that is ~70 bytes in the JSON blob:
409+
// `"channel-<64-hex>":1` ≈ 70 bytes including quotes, colon, comma.
410+
const makeChannelKey = (n) => `channel-${n.toString().padStart(64, "0")}`;
411+
const makeThreadKey = (n) => `thread:${n.toString().padStart(64, "0")}`;
412+
const makeMsgKey = (n) => `msg:${n.toString().padStart(64, "0")}`;
413+
414+
// Compute the byte size of a single-slot blob with the given contexts.
415+
const blobSize = (clientId, contexts) => {
416+
const encoder = new TextEncoder();
417+
return encoder.encode(JSON.stringify({ v: 1, client_id: clientId, contexts }))
418+
.length;
419+
};
420+
421+
let slotCounter = 0;
422+
const deterministicSlotId = () =>
423+
`slot-${(++slotCounter).toString().padStart(4, "0")}`;
424+
425+
test("splitContextsIntoBudgetedSlots_fitsInOneSlot_returnsSingleSlot", () => {
426+
// 3 channel keys — easily fits in one slot with a generous budget.
427+
const channelEntries = [
428+
[makeChannelKey(1), 100],
429+
[makeChannelKey(2), 200],
430+
[makeChannelKey(3), 300],
431+
];
432+
const result = splitContextsIntoBudgetedSlots({
433+
channelEntries,
434+
threadMsgEntries: [],
435+
clientId: CLIENT_ID,
436+
initialSlotCount: 1,
437+
maxSlots: 8,
438+
maxBytes: 1_000_000,
439+
slotIdGenerator: deterministicSlotId,
440+
});
441+
442+
assert.ok(result !== null, "should succeed");
443+
assert.equal(result.slots.length, 1, "single slot");
444+
assert.equal(result.extraSlotIds.length, 0, "no extra slots allocated");
445+
// All channel keys present in slot 0.
446+
for (const [key] of channelEntries) {
447+
assert.ok(key in result.slots[0], `${key} should be in slot 0`);
448+
}
449+
});
450+
451+
test("splitContextsIntoBudgetedSlots_requiresGrowth_allocatesExtraSlot", () => {
452+
// Build enough channel keys that a single slot overflows a tight budget
453+
// but two slots fit.
454+
const channelEntries = [];
455+
for (let i = 0; i < 20; i++) {
456+
channelEntries.push([makeChannelKey(i), i + 1]);
457+
}
458+
const encoder = new TextEncoder();
459+
// Budget that fits ~10 channel keys but not 20.
460+
const tenKeyContexts = Object.fromEntries(channelEntries.slice(0, 10));
461+
const tenKeySize = encoder.encode(
462+
JSON.stringify({ v: 1, client_id: CLIENT_ID, contexts: tenKeyContexts }),
463+
).length;
464+
const budget = tenKeySize + 50; // fits 10 but not 20
465+
466+
const result = splitContextsIntoBudgetedSlots({
467+
channelEntries,
468+
threadMsgEntries: [],
469+
clientId: CLIENT_ID,
470+
initialSlotCount: 1,
471+
maxSlots: 8,
472+
maxBytes: budget,
473+
slotIdGenerator: deterministicSlotId,
474+
});
475+
476+
assert.ok(result !== null, "should succeed with 2 slots");
477+
assert.equal(result.slots.length, 2, "two slots");
478+
assert.equal(result.extraSlotIds.length, 1, "one extra slot allocated");
479+
// All 20 keys present across both slots.
480+
const allKeys = new Set([
481+
...Object.keys(result.slots[0]),
482+
...Object.keys(result.slots[1]),
483+
]);
484+
for (const [key] of channelEntries) {
485+
assert.ok(allKeys.has(key), `${key} should appear in some slot`);
486+
}
487+
// Each slot fits within budget.
488+
for (const slotContexts of result.slots) {
489+
const size = encoder.encode(
490+
JSON.stringify({ v: 1, client_id: CLIENT_ID, contexts: slotContexts }),
491+
).length;
492+
assert.ok(size <= budget, `slot size ${size} exceeds budget ${budget}`);
493+
}
494+
});
495+
496+
test("splitContextsIntoBudgetedSlots_exceedsMaxSlots_returnsNull", () => {
497+
// Build enough channel keys that even maxSlots=2 can't fit them with a
498+
// very tight budget (1 byte — nothing can fit).
499+
const channelEntries = [[makeChannelKey(1), 1]];
500+
const result = splitContextsIntoBudgetedSlots({
501+
channelEntries,
502+
threadMsgEntries: [],
503+
clientId: CLIENT_ID,
504+
initialSlotCount: 1,
505+
maxSlots: 2,
506+
maxBytes: 1, // impossibly small
507+
slotIdGenerator: deterministicSlotId,
508+
});
509+
510+
assert.equal(result, null, "should return null when max slots exceeded");
511+
});
512+
513+
test("splitContextsIntoBudgetedSlots_includesThreadMsgInPrimarySlot", () => {
514+
// Channel key in slot 0; thread and msg entries should also land in slot 0.
515+
const channelEntries = [[makeChannelKey(1), 100]];
516+
const threadMsgEntries = [
517+
[makeThreadKey(1), 200],
518+
[makeMsgKey(1), 300],
519+
];
520+
521+
const result = splitContextsIntoBudgetedSlots({
522+
channelEntries,
523+
threadMsgEntries,
524+
clientId: CLIENT_ID,
525+
initialSlotCount: 1,
526+
maxSlots: 8,
527+
maxBytes: 1_000_000,
528+
slotIdGenerator: deterministicSlotId,
529+
});
530+
531+
assert.ok(result !== null, "should succeed");
532+
assert.equal(result.slots.length, 1);
533+
// Channel key in slot 0.
534+
assert.ok(makeChannelKey(1) in result.slots[0], "channel key in slot 0");
535+
// Thread and msg entries in slot 0.
536+
assert.ok(makeThreadKey(1) in result.slots[0], "thread key in slot 0");
537+
assert.ok(makeMsgKey(1) in result.slots[0], "msg key in slot 0");
538+
});
539+
540+
test("splitContextsIntoBudgetedSlots_threadMsgTrimmedWhenPrimarySlotOverBudget", () => {
541+
// Channel key fills the primary slot to near-budget. Thread/msg entries
542+
// added to slot 0 would overflow — trimContextsToBudget must evict them.
543+
const channelEntries = [[makeChannelKey(1), 100]];
544+
// Compute the size of a blob with just the channel key.
545+
const channelOnlyContexts = { [makeChannelKey(1)]: 100 };
546+
const channelOnlySize = blobSize(CLIENT_ID, channelOnlyContexts);
547+
// Budget = channel-only size + 5 bytes: fits the channel key but not
548+
// an additional thread/msg entry (~70+ bytes each).
549+
const budget = channelOnlySize + 5;
550+
551+
const threadMsgEntries = [[makeThreadKey(1), 200]];
552+
553+
const result = splitContextsIntoBudgetedSlots({
554+
channelEntries,
555+
threadMsgEntries,
556+
clientId: CLIENT_ID,
557+
initialSlotCount: 1,
558+
maxSlots: 8,
559+
maxBytes: budget,
560+
slotIdGenerator: deterministicSlotId,
561+
});
562+
563+
assert.ok(result !== null, "should succeed");
564+
// Channel key must survive (never evicted by trimContextsToBudget).
565+
assert.ok(makeChannelKey(1) in result.slots[0], "channel key survives");
566+
// Thread entry must be evicted (doesn't fit within budget).
567+
assert.ok(
568+
!(makeThreadKey(1) in result.slots[0]),
569+
"thread key evicted to fit budget",
570+
);
571+
// Slot 0 must fit within budget.
572+
const size = blobSize(CLIENT_ID, result.slots[0]);
573+
assert.ok(size <= budget, `slot 0 size ${size} exceeds budget ${budget}`);
574+
});

0 commit comments

Comments
 (0)