Skip to content

fix(repost): dedupe echo of just-published kind-6 against optimistic count#325

Merged
spe1020 merged 2 commits intomainfrom
fix/repost-count-echo-dedup
Apr 21, 2026
Merged

fix(repost): dedupe echo of just-published kind-6 against optimistic count#325
spe1020 merged 2 commits intomainfrom
fix/repost-count-echo-dedup

Conversation

@spe1020
Copy link
Copy Markdown
Contributor

@spe1020 spe1020 commented Apr 21, 2026

The bug

Clicking the repost button bumps the displayed repost count by 2 instead of 1. Net effect mirrors the reaction/comment inflation we fixed in #321 and #323, but was untouched by either of them because the repost path never had optimistic-echo dedup to begin with.

Root cause

NoteRepost.svelte does an optimistic count + 1 on click, then publishes the kind-6. The relay echoes the kind-6 back through the shared engagement subscription, and processRepost() in engagementCache.ts unconditionally runs count++ a second time.

  • Reactions already have optimisticReactions content-matching + markEventAsProcessed id-matching.
  • Zaps already have optimisticZaps amount-matching.
  • Comments don't optimistically add (fix(feed-comment-count): dedupe across concurrent engagementCache subscriptions #323 made the shared dedup Set persist so the subscription handler's processed.has(event.id) short-circuit works correctly).
  • Reposts had neither — hence the inflation.

Fix

Mirror the reaction pattern. NIP-18 is one-repost-per-user-per-target, so the optimistic key is (targetEventId, userPubkey) — no emoji-style distinction needed.

src/lib/engagementCache.ts

  • Add optimisticReposts: Map<string, { timestamp: number }> keyed by \${targetEventId}:\${userPubkey}.
  • Export trackOptimisticRepost(targetEventId, userPubkey) and clearOptimisticRepost(targetEventId, userPubkey).
  • processRepost(data, event, userPublickey, targetEventId?) — new targetEventId param. When event.pubkey === userPublickey, check-and-delete the matching optimistic entry and early-return before the count++.
  • Both call sites (`fetchEngagement` single sub at line ~479, batchFetchEngagement at line ~1001) pass the eventId through.
  • cleanupEngagement(eventId) and clearAllEngagementCaches() clean up the new Map.

src/components/NoteRepost.svelte

  • trackOptimisticRepost(event.id, \$userPublickey) before the optimistic store update and before any await.
  • markEventAsProcessed(event.id, repostEvent.id) between sign() (which populates .id) and publish() — closes the race where the echo arrives before publish() resolves.
  • clearOptimisticRepost(event.id, \$userPublickey) on publish failure alongside the count rollback.
  • Removes a pre-existing unused get import (trivial cleanup while in the file).

Verification

  • `pnpm exec eslint src/lib/engagementCache.ts src/components/NoteRepost.svelte` — clean.
  • `pnpm check` — 4 errors (baseline preserved; all in pre-existing files `kitchens.ts`, `nourishDiscovery.ts`, `FoodstrFeedOptimized.svelte`; none in the two files touched by this PR).
  • Seth on CF Pages preview: repost a fresh kind-1 post → count shows exactly 1 (was 2). Reload → still 1. Cross-check against Primal / Nostrudel rendering of the same event.
  • Click repost on ten different posts → each shows +1, not +2.

Out of scope

  • Primary dedup layer via markEventAsProcessed alone — could work without the optimisticReposts Map if we trusted the pre-publish mark to always win the race. The two-layer approach matches reactions and is more defensive; a single-layer alternative would be simpler but brittle to echoes landing before publish() resolves.

🤖 Generated with Claude Code

…count

Reposts double-counted by 1 after the user clicked the repost button.
NoteRepost.svelte incremented the count optimistically on click, then
the relay echoed the kind-6 back through the shared engagement
subscription and processRepost() unconditionally ran `count++` a
second time. Reactions and zaps already had content-level optimistic
matching (optimisticReactions / optimisticZaps) to skip the echo;
reposts had neither that nor a markEventAsProcessed call.

Fix: mirror the reaction pattern.

- engagementCache.ts: add an optimisticReposts Map keyed by
  `${targetEventId}:${userPubkey}` (NIP-18 is one-per-user-per-target
  so no emoji-style distinction is needed). Export
  trackOptimisticRepost / clearOptimisticRepost. processRepost() now
  takes an optional targetEventId and, when event.pubkey ===
  userPublickey, checks+deletes the matching optimistic entry before
  incrementing. Both call sites (fetchEngagement single sub at line
  ~479, batchFetchEngagement at line ~1001) pass the eventId through.
  cleanupEngagement and clearAllEngagementCaches clean up the new Map.
- NoteRepost.svelte: call trackOptimisticRepost before the optimistic
  store update, markEventAsProcessed between sign() and publish() to
  close the race where the echo arrives before publish() resolves,
  and clearOptimisticRepost on publish failure alongside the count
  rollback. Also removes a pre-existing unused `get` import.

Baseline preserved: eslint clean on the two edited files; pnpm check
still 4 errors in the three pre-existing files (kitchens.ts,
nourishDiscovery.ts, FoodstrFeedOptimized.svelte untouched).
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Apr 21, 2026

Deploying frontend with  Cloudflare Pages  Cloudflare Pages

Latest commit: e04d358
Status:⚡️  Build in progress...

View logs

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Apr 21, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
❌ Deployment failed
View logs
frontend e04d358 Apr 21 2026, 10:47 PM

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes repost count being incremented twice by deduping the relay echo of a just-published kind-6 against the optimistic UI increment.

Changes:

  • Add optimistic repost tracking to engagementCache and skip counting when the echoed repost matches an optimistic entry.
  • Update NoteRepost.svelte to register optimistic reposts pre-await and pre-mark processed ids to reduce echo races.
  • Bump package version.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/lib/engagementCache.ts Introduces optimisticReposts and integrates it into repost processing + cache cleanup.
src/components/NoteRepost.svelte Registers optimistic reposts before async work and pre-marks processed ids to avoid double counts.
package.json Version bump to reflect the fix release.
Comments suppressed due to low confidence (1)

src/components/NoteRepost.svelte:101

  • markEventAsProcessed(event.id, repostEvent.id) is set before publish(), but the catch block rolls back the optimistic UI state without removing that processed-id entry. If publish() throws after partially succeeding (e.g., some relays accepted the event), the subscription echo for this repost id will be dropped by processed.has(event.id) and the UI will stay rolled back even though the repost exists. Consider either (a) only marking as processed after a confirmed successful publish, relying on optimisticReposts for pre-publish dedup, or (b) adding an engagementCache API to unmark/remove the processed id on rollback (and using it here).
      // Secondary protection — mark by event id so the subscription's
      // `processed.has(event.id)` short-circuit fires even if the optimistic
      // key check misses. Done between sign() (which populates .id) and
      // publish() to close the race where the echo arrives before publish()
      // resolves.
      if (repostEvent.id) {
        markEventAsProcessed(event.id, repostEvent.id);
      }

      await repostEvent.publish();
      console.log('Successfully reposted');
    } catch (error) {
      console.error('Error reposting:', error);
      // Revert optimistic update + clear the optimistic-repost tracking so
      // a retry can re-register cleanly.
      clearOptimisticRepost(event.id, $userPublickey);
      store.update((s) => ({
        ...s,
        reposts: { count: s.reposts.count - 1, userReposted: false }
      }));

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

// key check misses. Done between sign() (which populates .id) and
// publish() to close the race where the echo arrives before publish()
// resolves.
if (repostEvent.id) {
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

markEventAsProcessed is a no-op if processedEventIds hasn’t been initialized for this event.id yet (it only adds when the Set exists). Since fetchEngagement(...) is started in onMount and not awaited, a very fast click could call this before the Set is created, weakening the intended dedup. Consider ensuring the Set exists (e.g., have markEventAsProcessed init-if-absent) or otherwise guarantee engagement initialization before marking.

Suggested change
if (repostEvent.id) {
if (repostEvent.id) {
// Ensure engagement state has been initialized for this note before
// marking, otherwise markEventAsProcessed(...) can be a no-op if a
// very fast click beats the onMount-triggered fetch.
await fetchEngagement(event.id);

Copilot uses AI. Check for mistakes.
… timing

Two Copilot review concerns on PR #325, both tied to markEventAsProcessed
timing:

1. (inline) markEventAsProcessed was a no-op if processedEventIds hadn't
   been initialized yet — a fast click between NoteRepost onMount and the
   non-awaited fetchEngagement would silently drop the mark and weaken
   the secondary dedup.
2. (suppressed low-confidence) Pre-publish mark + catch-block rollback
   leaves a dangling processed-id if publish throws after partial
   success. Echoes from relays that did accept the kind-6 would be
   dropped by processed.has(), so the UI (rolled back to 0) would stay
   out of sync with relay reality.

Fix:

- engagementCache.ts: markEventAsProcessed now init-if-absent. A later
  fetchEngagement sees the Set exists → takes re-entry branch → counts
  preserved → subscription starts normally. This also benefits
  ReactionTrigger, which has the same latent race.
- NoteRepost.svelte: move markEventAsProcessed to AFTER publish()
  resolves, matching the ReactionTrigger pattern. The pre-await
  trackOptimisticRepost is the primary defense against the
  pre-publish-resolve echo race (content-match by targetEventId +
  userPubkey); the post-publish mark is only the secondary safety net
  for later re-deliveries. On partial-publish-fail no mark is left, so
  the echo from accepting relays reaches processRepost with
  optimisticReposts already cleared and the count increments to reflect
  relay reality.

Baseline: eslint clean; pnpm check still 4 pre-existing errors in
untouched files.
@spe1020 spe1020 merged commit f202bcd into main Apr 21, 2026
1 of 3 checks passed
@spe1020 spe1020 deleted the fix/repost-count-echo-dedup branch April 21, 2026 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants