fix(feed): keep sorted pagination advancing - #548
Conversation
Deploying divine-web with
|
| Latest commit: |
1b14ed0
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://197eac50.divine-web.pages.dev |
| Branch Preview URL: | https://fix-sorted-feed-raw-paginati.divine-web.pages.dev |
🚀 Preview DeploymentLast updated:
|
RAW_FEED_BACKFILL_ATTEMPTS bounds the zero-parse retry loop, but it also silently bounds how deep a feed can reach: after ~cap*pageSize consecutive unparseable events, pagination stops even if valid videos exist deeper. The PR body documents the relay-limit stop but not this one. Note the tradeoff so the cap is not later raised or removed without weighing the loop risk.
The raw-event timestamp cursor is correct only because nostr.query returns non-search results sorted created_at desc (NPool) and video feeds bypass the profile/contact cache. If that ordering guarantee changes, the cursor would skip events and silently drop videos from the feed. Record the dependency at the point that depends on it.
mbradley
left a comment
There was a problem hiding this comment.
Reviewed the pagination rework (base head e21bef5). The raw-event cursor is the right fix for the parsed/raw unit mismatch: advancing sorted offsets by events consumed rather than parsed videos skips dropped events without overlap, and keying InfiniteScroll to unfiltered fetchedCount stops mute/verified-only/dedupe filtering from latching the scroller. Verified the 34 affected tests pass, mutation-checked the two core guards (they go red when the fix is reverted), and typecheck is clean.
Pushed two doc-only commits recording invariants the code leans on but did not state: the backfill cap doubles as a feed reachability ceiling, and the chronological cursor depends on NPool returning non-search results newest-first. Neither changes behavior.
One noted trade, no action needed: client-side Classic/sort:top ordering is now per-page-window rather than global on the websocket fallback. It is disclosed in the PR body and it is the right call here, since preserving global order would reintroduce the raw/parsed mismatch this PR removes.
Approving.
mbradley
left a comment
There was a problem hiding this comment.
Re-approving on 1b14ed0. My earlier approval was pinned to 85c534b, before the latch-gap merge you pushed. Checked the delta: v2-cursor now stops on a zero-parse page, the search feed's scroll is keyed to raw fetched count, and the #550 featured-tab conflict resolution kept both sides (both InfiniteScroll instances key to fetchedCount). New v2 guard's test goes red when the guard is reverted, and test (20.x) is green at head. Good to merge.
Summary
VideoFeedinfinite scroll keyed to unfiltered fetched count so dedupe, mute filtering, or verified-only filtering cannot leavereact-infinite-scroll-componentlatched.VideoFeedscroll re-arming, and page-window Classic sorting.Motivation
VideoFeedcould still preserve the user-visible stall after downstream filtering because itsdataLengthwas the rendered filtered count rather than the fetched count.Behavior Notes
Related Issue
Testing
npm run testVisuals