Skip to content

perf(feed): batch engagement for anon viewers too (R2 from #341)#357

Merged
spe1020 merged 2 commits intozapcooking:mainfrom
spe1020:perf/feed-batch-engagement
Apr 26, 2026
Merged

perf(feed): batch engagement for anon viewers too (R2 from #341)#357
spe1020 merged 2 commits intozapcooking:mainfrom
spe1020:perf/feed-batch-engagement

Conversation

@spe1020
Copy link
Copy Markdown
Contributor

@spe1020 spe1020 commented Apr 26, 2026

Summary

Closes the last outstanding Stage-2 recommendation from the perf review in #341. R1 (feed remount → prop-driven mode) shipped as #344 and R4 (ReactionPills key) shipped as #342; this PR finishes the trio with R2.

Problem

`FoodstrFeedOptimized.svelte` already imports and calls `batchFetchEngagement` at two sites — a rendered-near-viewport preload and a visibility-update reactive. Both gate on `$userPublickey`, so:

  • Signed-in viewers: batch path runs. Per-card `fetchEngagement` calls hit the 5-min freshness gate inside the batch helper and bail.
  • Anonymous viewers: batch path is skipped entirely. Per-card children fan out individually.

For each rendered note, the per-card fan-out is up to 5×:

Component mount-time call
`NoteTotalLikes` `fetchEngagement(eventId)`
`NoteTotalComments` `fetchEngagement(eventId)`
`NoteRepost` `fetchEngagement(eventId)`
`NoteTotalZaps` `fetchEngagement(eventId)`
`ReactionPills` `fetchEngagement(eventId)`

Multiply by a 30-event feed window on every cold paint and every newly-rendered card. The cache singleton dedups the actual network query, but the active-subscription counter still climbs.

Why the gate isn't load-bearing

  • `batchFetchEngagement` → `batchFetchFromServerAPI` doesn't require auth.
  • The NDK subscription fallback only uses `userPublickey` to flag `userReacted` / `userReposted` — for anonymous users those flags correctly stay false either way.
  • The per-card children themselves don't gate on sign-in. Only the batch did.

Fix

Drop the `$userPublickey` check from both reactive blocks. Same code path runs for signed-in and anonymous viewers; signed-in behavior is unchanged because the gate was always passing for them.

```diff

  • $: if (… && $ndk && $userPublickey && events.length > 0 && !loading && renderedNotes.size > 0) {
  • $: if (… && $ndk && events.length > 0 && !loading && renderedNotes.size > 0) {
  • $: if (… && $ndk && $userPublickey && visibleNotes.size > 0 && events.length > 0) {
  • $: if (… && $ndk && visibleNotes.size > 0 && events.length > 0) {
    ```

Comments inline document the reasoning so the next reader doesn't re-add the gate.

Risk

Test plan

  • Open Foodstr feed signed out → counts populate with no observed regression.
  • Open Foodstr feed signed in → counts populate as before.
  • DevTools → Network: confirm only one server-counts batch request fires per visible window (signed in and signed out).
  • Scroll the feed signed out → new visible notes resolve their counts in batched calls, not 5× per-card.
  • React / repost / zap interactions still work for signed-in users (engagement updates flow through the same store).
  • `pnpm check` 0 errors, `pnpm build` ✅ (verified locally).

🤖 Generated with Claude Code

Closes the last open recommendation from the perf review docs in
PR zapcooking#341 (R1 / R4 already shipped via zapcooking#344 and zapcooking#342). The batch
helper itself is wired in at two sites — the rendered-events
preload and the visibility-update reactive — but both gate on
`$userPublickey`, which means signed-in users get batched fetches
while anonymous viewers fall back to the full per-card fan-out:

  NoteTotalLikes      → fetchEngagement(eventId)
  NoteTotalComments   → fetchEngagement(eventId)
  NoteRepost          → fetchEngagement(eventId)
  NoteTotalZaps       → fetchEngagement(eventId)
  ReactionPills       → fetchEngagement(eventId)

That's up to 5× the active subscription churn per card, multiplied
across a 30-event feed window, on every cold paint and every newly-
rendered card.

The gate isn't doing anything useful: `batchFetchEngagement`'s
server-counts API doesn't require auth, and the NDK fallback
subscription only uses `userPublickey` to flag userReacted /
userReposted — which already correctly stay false for anonymous
users either way. The per-card children themselves don't gate on
sign-in; only the batch did.

Drop the gate on both reactives so anonymous viewers get the same
batched preload + visibility refresh signed-in users already
benefit from. The 5-minute freshness gate inside
batchFetchEngagement preempts the subsequent per-card fetches
just like it does for the signed-in path.

No behavior change for signed-in viewers (gate was always passing
for them). Anonymous viewers see fewer concurrent NDK subs during
feed scroll. Counts and groups continue to populate identically;
no impact to the count-correctness work in zapcooking#321/zapcooking#323.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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

Enables the existing engagement batching path in FoodstrFeedOptimized.svelte for anonymous viewers (not just logged-in users), aligning anonymous feed behavior with the signed-in performance path and reducing per-card engagement fetch fan-out.

Changes:

  • Remove $userPublickey gating from the engagement preload and visibility-driven batch fetch reactive blocks.
  • Add inline comments documenting why batching should run for anonymous viewers.
  • Bump package.json version from 4.2.310 to 4.2.311.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/components/FoodstrFeedOptimized.svelte Runs batched engagement fetching for both anonymous and signed-in viewers; adds rationale comments.
package.json Patch version bump for the release containing the feed perf change.

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

Comment thread src/components/FoodstrFeedOptimized.svelte Outdated
Comment on lines 4850 to 4852
let lastBatchedIds = new Set<string>();
let engagementPreloadTimeout: ReturnType<typeof setTimeout> | null = null;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@spe1020 spe1020 merged commit 31733b7 into zapcooking:main Apr 26, 2026
1 check passed
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