edufeed/edufeed-app
Watch
1
0
Fork
You've already forked edufeed-app
 0
Code Issues 17 Pull requests 6 Projects Releases Packages 1 Wiki Activity Actions

fix(calendar): write updated events to the EventStore so the cache is not left stale (#62) #63

Merged
laoc merged 3 commits from fix/issue-62-stale-cache-after-update into dev 2026-07-30 13:20:53 +00:00
Conversation 0 Commits 3 Files changed 5 +287 -16
laoc commented 2026-07-30 12:22:46 +00:00
Owner
Copy link

Closes #62.

The bug

Editing a calendar event publishes the replacement to the relays but never offers it to the local cache, so every reload of the detail page renders the pre-edit version — indefinitely.

Measured by @TestOER: after a save, the relay held exactly one event at the coordinate carrying the new title, while the page rendered the old one. So the relay is fine; the client serves a stale read.

Mechanism

  • calendar/event/[naddr]/+page.js:5 is ssr = false; load() calls fetchEventById, which is firstValueFrom(addressLoader(...)) (nostrUtils.js:243-245).
  • That loader is built with cacheRequest (loaders/base.js:41-43), and addressPointerLoadingSequence drops a pointer from remaining as soon as any event at that address arrives. A cache hit ends the sequence — relay hints, additional relays and lookup relays are never tried.
  • The cache is fed from eventStore.insert$ (event-cache.svelte.js:70-78), and 31922/31923 are explicitly cacheable.
  • createEvent uses publishEventOptimistic, which does eventStore.add (publish-service.js:244). updateEvent uses publishEvent, which does not — and calendar-actions.svelte.js did not import eventStore at all.

That explains the apparent counter-evidence too: creation passes (nothing cached yet) and the prefill test passes (it asserts the original title, which is exactly what the stale read returns).

Updating calendarStore is not a substitute: that is the list view's state, not the detail page's read path.

Why not just swap in publishEventOptimistic

Its failure path is eventStore.remove(signedEvent) (publish-service.js:333), and the cache pipeline is insert-only — no code path deletes a single event from IndexedDB. For an addressable kind that is worse than a leak: nostr-idb keys by kind:pubkey:d, newest-wins, so a rejected publish leaves a phantom that overwrites the last good version, and by the cache-hit short-circuit above the relay is never asked to correct it.

So: await publishEvent(...), then eventStore.add(signed) only on success — the shape EditProfileModal, pin-list-service and helpers/comments.js already use.

The add is best-effort. eventStore.add validates and throws on a malformed event; by then the publish has already landed, so a cache-write failure must not surface as Failed to update calendar event. It degrades to the old stale-read behaviour, matching the "cache is ADDITIVE" contract in event-cache.svelte.js.

Tests

New src/lib/__tests__/calendar-update-event-eventstore.test.js, 5 cases: the add happens and carries the new title at the same coordinate; the signed event is added rather than the dTag-decorated return value; no add when successCount is 0; a throwing add does not fail the update; and an ordering control that publish precedes add.

Negative control: with the source change reverted, 4 of the 5 fail. The fifth ("does NOT cache when the publish failed") passes vacuously without the fix — stated so nobody reads 5/5 as five independent guards.

Verification

  • Unit suite 5185/5185, 469 files, with .env exported (see #55 — without it the suite is red for unrelated reasons).
  • pnpm check 0 errors (4 pre-existing warnings in test fixtures), pnpm lint clean.
  • The Errors: N on GlobalFAB.test.js is the known #55-family teardown race, not this change: 3 consecutive isolated runs in this worktree give 0 errors, and it appears on untouched worktrees too.
  • Browser not run. The acceptance criterion is @TestOER turning the three calendar-editing tests green.

Not fixed here, filed separately

Same missing-eventStore.add shape found while in the area, all confirmed by grep rather than assumed:

site status
relay-settings-service.js:66 (kind 10002) affected, and the worst of the set — grep -c eventStore returns 0, 10002 is cacheable, and it is the relay list itself: a stale read there mis-routes every subsequent query
calendar-actions.svelte.js:279 (kind 31924) affected but milder — it is a create, so there is no stale prior version, only "missing until a relay round-trip"
createEvent via publishEventOptimistic a create that reaches no relay leaves a phantom in IndexedDB that survives reload — the remove-can't-reach-IDB hole, live today for 31922/31923
pin-list-service.js:102,135 not affected — eventStore.add at 103 and 136
calendarActions.deleteEvent (:234) publishes a kind 5 with no eventStore.add and no cacheDeletion, but has zero callers (grep -rn '\.deleteEvent(' src/ is empty; the UI goes through helpers/eventDeletion.js). Dead code encoding the wrong pattern — left in place because deleting it is outside this fix

Also noted: the all-day toggle flips 31922<->31923 while keeping the d-tag, so the coordinate changes and nothing is replaced. Documented on #62; not the cause of the three failing tests, none of which touch the toggle.

Closes #62. ## The bug Editing a calendar event publishes the replacement to the relays but never offers it to the local cache, so every reload of the detail page renders the pre-edit version — indefinitely. Measured by @TestOER: after a save, the relay held **exactly one event** at the coordinate carrying the **new** title, while the page rendered the **old** one. So the relay is fine; the client serves a stale read. ## Mechanism - `calendar/event/[naddr]/+page.js:5` is `ssr = false`; `load()` calls `fetchEventById`, which is `firstValueFrom(addressLoader(...))` (`nostrUtils.js:243-245`). - That loader is built with `cacheRequest` (`loaders/base.js:41-43`), and `addressPointerLoadingSequence` drops a pointer from `remaining` as soon as *any* event at that address arrives. **A cache hit ends the sequence — relay hints, additional relays and lookup relays are never tried.** - The cache is fed from `eventStore.insert$` (`event-cache.svelte.js:70-78`), and 31922/31923 are explicitly cacheable. - `createEvent` uses `publishEventOptimistic`, which does `eventStore.add` (`publish-service.js:244`). `updateEvent` uses `publishEvent`, which does not — and `calendar-actions.svelte.js` did not import `eventStore` at all. That explains the apparent counter-evidence too: creation passes (nothing cached yet) and the prefill test passes (it asserts the *original* title, which is exactly what the stale read returns). Updating `calendarStore` is **not** a substitute: that is the list view's state, not the detail page's read path. ## Why not just swap in `publishEventOptimistic` Its failure path is `eventStore.remove(signedEvent)` (`publish-service.js:333`), and the cache pipeline is **insert-only** — no code path deletes a single event from IndexedDB. For an addressable kind that is worse than a leak: nostr-idb keys by `kind:pubkey:d`, newest-wins, so a rejected publish leaves a phantom that **overwrites the last good version**, and by the cache-hit short-circuit above the relay is never asked to correct it. So: `await publishEvent(...)`, then `eventStore.add(signed)` **only on success** — the shape `EditProfileModal`, `pin-list-service` and `helpers/comments.js` already use. The add is best-effort. `eventStore.add` validates and throws on a malformed event; by then the publish has already landed, so a cache-write failure must not surface as `Failed to update calendar event`. It degrades to the old stale-read behaviour, matching the "cache is ADDITIVE" contract in `event-cache.svelte.js`. ## Tests New `src/lib/__tests__/calendar-update-event-eventstore.test.js`, 5 cases: the add happens and carries the new title at the same coordinate; the **signed** event is added rather than the `dTag`-decorated return value; no add when `successCount` is 0; a throwing `add` does not fail the update; and an ordering control that publish precedes add. **Negative control:** with the source change reverted, 4 of the 5 fail. The fifth ("does NOT cache when the publish failed") passes vacuously without the fix — stated so nobody reads 5/5 as five independent guards. ## Verification - Unit suite `5185/5185`, 469 files, with `.env` exported (see #55 — without it the suite is red for unrelated reasons). - `pnpm check` **0 errors** (4 pre-existing warnings in test fixtures), `pnpm lint` clean. - The `Errors: N` on `GlobalFAB.test.js` is the known #55-family teardown race, not this change: 3 consecutive isolated runs in this worktree give 0 errors, and it appears on untouched worktrees too. - **Browser not run.** The acceptance criterion is @TestOER turning the three `calendar-editing` tests green. ## Not fixed here, filed separately Same missing-`eventStore.add` shape found while in the area, all confirmed by grep rather than assumed: | site | status | |---|---| | `relay-settings-service.js:66` (kind 10002) | **affected, and the worst of the set** — `grep -c eventStore` returns 0, 10002 is cacheable, and it is the relay list itself: a stale read there mis-routes every subsequent query | | `calendar-actions.svelte.js:279` (kind 31924) | affected but milder — it is a *create*, so there is no stale prior version, only "missing until a relay round-trip" | | `createEvent` via `publishEventOptimistic` | a create that reaches no relay leaves a phantom in IndexedDB that survives reload — the remove-can't-reach-IDB hole, live today for 31922/31923 | | `pin-list-service.js:102,135` | **not affected** — `eventStore.add` at 103 and 136 | | `calendarActions.deleteEvent` (`:234`) | publishes a kind 5 with no `eventStore.add` and no `cacheDeletion`, but has **zero callers** (`grep -rn '\.deleteEvent(' src/` is empty; the UI goes through `helpers/eventDeletion.js`). Dead code encoding the wrong pattern — left in place because deleting it is outside this fix | Also noted: the all-day toggle flips 31922<->31923 while keeping the d-tag, so the coordinate changes and nothing is replaced. Documented on #62; not the cause of the three failing tests, none of which touch the toggle.
Editing a calendar event published the replacement to the relays but never
offered it to the local cache, so every reload of the detail page rendered
the pre-edit version indefinitely.

The detail route is `ssr = false` and loads via applesauce's addressLoader,
whose first step is the IndexedDB cache. A cache hit ends the loading
sequence — relay hints, extra relays and lookup relays are never tried — so
the page shows whatever the cache holds. The cache is fed from
`eventStore.insert$`. `createEvent` goes through `publishEventOptimistic`,
which adds to the EventStore; `updateEvent` goes through `publishEvent`,
which does not, and `calendar-actions.svelte.js` did not import eventStore
at all. Updating `calendarStore` is not a substitute: that is the list
view's state, not the detail page's read path.

Add the signed event to the EventStore after a successful publish. Not
`publishEventOptimistic`: its failure path calls `eventStore.remove`, and
the cache pipeline is insert-only (`persistEventsToCache`, batchTime 1s),
so for an addressable kind a rejected publish would leave a phantom in
IndexedDB that overwrites the last good version and — per the cache-hit
short-circuit above — is never corrected from a relay. Gating an explicit
add on success is the shape `EditProfileModal` and `pin-list-service`
already use.

The add is best-effort: `eventStore.add` validates and throws on a
malformed event, and by then the publish has already landed, so a
cache-write failure must not be reported as a failed update.

Reported and measured by TestOER: the relay held exactly one event at the
coordinate, carrying the new title, while the page rendered the old one.

Refs: #62
The EventStore write alone was necessary but not sufficient. A replacement
that lands in the same wall-clock second as its predecessor is dropped by
three independent layers, with two different tie-breaks:

  - relays (NIP-01): on equal created_at the LOWER id wins — a coin flip,
    so the edit survives on the relay only about half the time;
  - applesauce EventStore: same rule, `incomingBeatsWinner` in
    event-store/event-store.js;
  - nostr-idb: strict `event.created_at > existing` in database/insert.js,
    so on a tie the IDB write is ALWAYS rejected.

Only the relay layer is a coin flip. The cache is deterministically stale,
and because the IDB cache is the first step of applesauce's address-loader
sequence and a cache hit ends that sequence, no relay ever corrects it. So
a same-second edit reads to the user as "the save did nothing" even in the
runs where the relay accepted the replacement.

Stamp created_at as max(now, existing.created_at + 1). The bump only closes
a tie: an edit made a second or more after the event it replaces still gets
wall-clock time.

Reachable outside the tests by a user editing straight after creating, and
by two tabs replacing the same coordinate.

Diagnosed by TestOER from a full relay recording of a failing e2e run: in
the one attempt that passed, the create's d-tag was minted 23ms before a
second boundary; in the five that failed there was 232-474ms of headroom,
so create and update shared a second.

Refs: #62
The last red test in the calendar-editing set was not the stale read. Once
the update actually reached the page, the summary rendered through
MarkdownRenderer (CalendarEventDetailView.svelte:218-224) and the page-wide
getByText resolved to two elements — the wrapper and the paragraph it
produces. Same fix already applied one file over at
calendar-creation.test.js:165.

Worth recording that the assertion failed two different ways: "element(s)
not found" while #62 was live (the pre-edit event had no description, so no
card rendered at all) and "strict mode violation" once the read was fresh.
The change in failure mode is what proved the read had been fixed.

Also refresh two stale comments in the same file: the KNOWN FAILING blocks
now describe what these tests guard, and the claim that the modal triggers
window.location.reload() is corrected — it calls invalidateAll(), which
preserves the JS context, so the explicit page.goto() is the only hard
navigation in that test.

Refs: #62
laoc merged commit b82bfe9c8c into dev 2026-07-30 13:20:53 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
bug
Something is broken

  
discussion
Needs a decision or concept first

  
enhancement
New feature or improvement

No labels
bug
discussion
enhancement
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
edufeed/edufeed-app!63
No description provided.