fix: end-of-feed and all-type search semantics - #72
Merged
Conversation
Two ways the canonical API misreported reality on the page-number
providers (Lemmy v0, PieFed):
next_page always had a value, so it could never mean 'there is more' —
a consumer looping while (next_page) would spin forever, and paging had
to be stopped by noticing a short page instead. It's now derived: the
server's own cursor where the endpoint returns one (PieFed's list
endpoints do), otherwise inferred from a full page. Absent means end of
feed on every provider now.
search({ type_: 'all' }) — and a search with no type_ at all — 400'd
against real PieFed, whose search requires a concrete type_ and has no
'All' in its enum (verified live). The adapter now fans out across the
four concrete types and merges, taking only each response's requested
bucket so nothing double-counts. The canonical-to-PieFed search type map
is now explicit rather than borrowed from lemmyv0, so an unsupported
value can't slip through the query spread's widened typing again.
Covered by a matrix test (uniform end-of-feed + all-type search across
both fakes) and a live smoke scenario that exercises the previously
failing call against real instances.
Review catch, confirmed live: these servers apply the limit per bucket, so every endpoint that merges requests (modlog's 14 buckets, notifications, person content, all-type search) returns MORE than the limit on page one. Comparing with === called that the last page, ending those feeds after a single page — Voyager's modlog, modqueue, inbox, profile, saved and voted feeds, plus large comment threads. >= is the correct rule, not a patch: if any bucket came back full the merged page is at least the limit, and a merged page shorter than the limit means every bucket was exhausted. Also: an unusable server cursor (NaN) now reports end-of-feed instead of being sent back to the server, and piefed's merged person content ORs the two sub-cursors it already had rather than inferring from the merged length (piefed silently clamps large limits). test/toPageResponse.test.ts pins all of these rules directly — the fakes don't model lemmyv0, so none of it was reachable from the matrix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two places the canonical API misreported reality on the page-number providers (Lemmy v0, PieFed) — both found while building the fakes' pagination/search, both verified against live servers.
next_pagecould never mean "there is more"toPageResponsecomputed(page_cursor ?? 1) + 1unconditionally, so a consumer loopingwhile (next_page)would never terminate on v0/PieFed; paging could only be stopped by noticing a short page. It's now derived:post/list,comment/list,community/list,user/replies,user/mentionsall do, and we were throwing it away;limitto compare against, the old assumption is kept rather than guessing.Absent now means end-of-feed on every provider, which is what the canonical type always implied. The matrix test's provider-specific carve-out for this is gone.
All-type search 400'd against real PieFed
PieFed's search requires
type_and its enum has noAll, sosearch({ type_: "all" })— and a search with notype_— failed in production:The adapter now fans out across the four concrete types and merges, taking only each response's requested bucket so a fan-out can't double-count. The canonical→PieFed search-type map is explicit instead of borrowed from lemmyv0's (whose
Allwas slipping past the query spread's widened typing behind a@ts-expect-error).Verification
447 unit tests; matrix coverage for both fixes on both fakes; live smoke gains an all-type search scenario — the exact call that used to 400 — now passing against lemmy.world (v0), ds9.lemmy.ml (v1) and piefed.social. Live suites 13/13 smoke + 9/9 fidelity.