From b1b4f161f8ae2578a78e72ab96eee664fae9a42b Mon Sep 17 00:00:00 2001 From: Alexander Harding Date: Sat, 25 Jul 2026 12:45:05 -0500 Subject: [PATCH 1/2] fix: end-of-feed and all-type search semantics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/providers/lemmyv0/compat.ts | 29 +++++++- src/providers/lemmyv0/index.ts | 88 +++++++++++++--------- src/providers/piefed/index.ts | 121 +++++++++++++++++++++++-------- test/live-smoke.test.ts | 8 ++ test/testing-seed-matrix.test.ts | 35 ++++++++- 5 files changed, 206 insertions(+), 75 deletions(-) diff --git a/src/providers/lemmyv0/compat.ts b/src/providers/lemmyv0/compat.ts index a441ca9..8337ff2 100644 --- a/src/providers/lemmyv0/compat.ts +++ b/src/providers/lemmyv0/compat.ts @@ -505,8 +505,22 @@ export function toModlogView( } } +/** + * Canonical pagination for the page-number softwares (Lemmy v0, PieFed). + * + * `next_page` has to mean "there is more" — handing one out unconditionally + * makes a consumer's `while (next_page)` loop spin forever. The server's own + * cursor wins when the endpoint returns one; otherwise a full page implies + * there may be another and a short page is the end. + */ export function toPageResponse( params: types.PageParams, + page?: { + /** How many items this page returned */ + items: number; + /** The server's own cursor, for endpoints that return one */ + next_page?: null | string; + }, ): types.PagableResponse { const page_cursor = params.page_cursor; @@ -515,9 +529,18 @@ export function toPageResponse( "lemmyv0 does not support string page_cursor", ); - return { - next_page: (page_cursor ?? 1) + 1, - }; + if (page?.next_page !== undefined) + return { + next_page: page.next_page === null ? undefined : Number(page.next_page), + }; + + const nextPage = (page_cursor ?? 1) + 1; + + // Without a limit there's nothing to compare a short page against, so + // keep assuming there's more + if (!page || params.limit === undefined) return { next_page: nextPage }; + + return { next_page: page.items === params.limit ? nextPage : undefined }; } export function toPerson( diff --git a/src/providers/lemmyv0/index.ts b/src/providers/lemmyv0/index.ts index 20a92bb..fbc8f87 100644 --- a/src/providers/lemmyv0/index.ts +++ b/src/providers/lemmyv0/index.ts @@ -256,9 +256,11 @@ export class UnsafeLemmyV0Client implements BaseClient { options, ); + const data = response.comments.map(compat.toCommentView); + return { - ...compat.toPageResponse(payload), - data: response.comments.map(compat.toCommentView), + ...compat.toPageResponse(payload, { items: data.length }), + data, }; } @@ -308,13 +310,15 @@ export class UnsafeLemmyV0Client implements BaseClient { options, ); + const data = Object.values(response) + .flat() + .map(compat.toModlogView) + .filter((m) => !!m) + .sort((a, b) => Date.parse(getLogDate(b)) - Date.parse(getLogDate(a))); + return { - ...compat.toPageResponse(payload), - data: Object.values(response) - .flat() - .map(compat.toModlogView) - .filter((m) => !!m) - .sort((a, b) => Date.parse(getLogDate(b)) - Date.parse(getLogDate(a))), + ...compat.toPageResponse(payload, { items: data.length }), + data, }; } @@ -376,7 +380,7 @@ export class UnsafeLemmyV0Client implements BaseClient { } return { - ...compat.toPageResponse(payload), + ...compat.toPageResponse(payload, { items: data.length }), data, }; } @@ -539,9 +543,11 @@ export class UnsafeLemmyV0Client implements BaseClient { options, ); + const data = response.comment_reports.map(compat.toCommentReportView); + return { - ...compat.toPageResponse(payload), - data: response.comment_reports.map(compat.toCommentReportView), + ...compat.toPageResponse(payload, { items: data.length }), + data, }; } @@ -564,9 +570,11 @@ export class UnsafeLemmyV0Client implements BaseClient { options, ); + const data = response.communities.map(compat.toCommunityView); + return { - ...compat.toPageResponse(payload), - data: response.communities.map(compat.toCommunityView), + ...compat.toPageResponse(payload, { items: data.length }), + data, }; } @@ -604,7 +612,7 @@ export class UnsafeLemmyV0Client implements BaseClient { })(); return { - ...compat.toPageResponse(payload), + ...compat.toPageResponse(payload, { items: data.length }), data, }; } @@ -625,12 +633,14 @@ export class UnsafeLemmyV0Client implements BaseClient { this.#client.getComments(v0Payload, options), ]); + const data = [ + ...comments.map(compat.toCommentView), + ...posts.map(compat.toPostView), + ].sort(sortPostCommentByPublished); + return { - data: [ - ...comments.map(compat.toCommentView), - ...posts.map(compat.toPostView), - ].sort(sortPostCommentByPublished), - ...compat.toPageResponse(payload), + ...compat.toPageResponse(payload, { items: data.length }), + data, }; } @@ -657,9 +667,11 @@ export class UnsafeLemmyV0Client implements BaseClient { options, ); + const data = response.post_reports.map(compat.toPostReportView); + return { - ...compat.toPageResponse(payload), - data: response.post_reports.map(compat.toPostReportView), + ...compat.toPageResponse(payload, { items: data.length }), + data, }; } @@ -674,15 +686,17 @@ export class UnsafeLemmyV0Client implements BaseClient { this.#client.listPostReports(params, options), ]); + const data = [ + ...comment_reports.map(compat.toCommentReportView), + ...post_reports.map(compat.toPostReportView), + ].sort( + (a, b) => + getPostCommentItemCreatedDate(b) - getPostCommentItemCreatedDate(a), + ); + return { - ...compat.toPageResponse(payload), - data: [ - ...comment_reports.map(compat.toCommentReportView), - ...post_reports.map(compat.toPostReportView), - ].sort( - (a, b) => - getPostCommentItemCreatedDate(b) - getPostCommentItemCreatedDate(a), - ), + ...compat.toPageResponse(payload, { items: data.length }), + data, }; } @@ -856,14 +870,16 @@ export class UnsafeLemmyV0Client implements BaseClient { options, ); + const data = [ + ...response.comments.map(compat.toCommentView), + ...response.posts.map(compat.toPostView), + ...response.communities.map(compat.toCommunityView), + ...response.users.map(compat.toPersonView), + ]; + return { - ...compat.toPageResponse(payload), - data: [ - ...response.comments.map(compat.toCommentView), - ...response.posts.map(compat.toPostView), - ...response.communities.map(compat.toCommunityView), - ...response.users.map(compat.toPersonView), - ], + ...compat.toPageResponse(payload, { items: data.length }), + data, }; } diff --git a/src/providers/piefed/index.ts b/src/providers/piefed/index.ts index a2702a6..a45edd3 100644 --- a/src/providers/piefed/index.ts +++ b/src/providers/piefed/index.ts @@ -76,6 +76,19 @@ const piefedMiddleware: Middleware = { }, }; +/** Canonical search types PieFed can actually serve (its enum has no "All") */ +type SearchableType = Exclude; + +const PIEFED_SEARCH_TYPE = { + comments: "Comments", + communities: "Communities", + posts: "Posts", + users: "Users", +} as const satisfies Record< + SearchableType, + NonNullable["type_"] +>; + export class UnsafePiefedClient implements BaseClient { static mode = "piefed" as const; @@ -394,9 +407,14 @@ export class UnsafePiefedClient implements BaseClient { params: { query }, }); + const data = response.data!.comments.map(compat.toCommentView); + return { - ...compat.toPageResponse(payload), - data: response.data!.comments.map(compat.toCommentView), + ...compat.toPageResponse(payload, { + items: data.length, + next_page: response.data!.next_page, + }), + data, }; } @@ -498,7 +516,7 @@ export class UnsafePiefedClient implements BaseClient { } return { - ...compat.toPageResponse(payload), + ...compat.toPageResponse(payload, { items: data.length }), data, }; } @@ -556,9 +574,14 @@ export class UnsafePiefedClient implements BaseClient { params: { query }, }); + const data = response.data!.posts.map(compat.toPostView); + return { - ...compat.toPageResponse(payload), - data: response.data!.posts.map(compat.toPostView), + ...compat.toPageResponse(payload, { + items: data.length, + next_page: response.data!.next_page, + }), + data, }; } @@ -687,9 +710,14 @@ export class UnsafePiefedClient implements BaseClient { }, }); + const data = response.data!.communities.map(compat.toCommunityView); + return { - ...compat.toPageResponse(payload), - data: response.data!.communities.map(compat.toCommunityView), + ...compat.toPageResponse(payload, { + items: data.length, + next_page: response.data!.next_page, + }), + data, }; } @@ -713,8 +741,7 @@ export class UnsafePiefedClient implements BaseClient { ); return { - ...compat.toPageResponse(payload), - + ...compat.toPageResponse(payload, { items: response.length }), data: response, }; } @@ -764,7 +791,7 @@ export class UnsafePiefedClient implements BaseClient { })(); return { - ...compat.toPageResponse(payload), + ...compat.toPageResponse(payload, { items: data.length }), data, }; } @@ -968,27 +995,47 @@ export class UnsafePiefedClient implements BaseClient { ): ReturnType { const { listing_type, search_term, type_, ...rest } = compat.fromPageParams(payload); - const response = await this.#client.GET("/api/alpha/search", { - ...options, - params: { - query: { - ...rest, - listing_type: compat.fromListingType(listing_type), - q: search_term, - // @ts-expect-error piefed's SearchType is narrower than ours; we pass through whatever the caller sent - type_: compat.fromSearchType(type_), + + // PieFed requires a concrete `type_` — its enum has no "All" — so an + // all-type search fans out and merges, matching what the canonical + // `type_: "all"` (or an unspecified type) means everywhere else. + const searchTypes: SearchableType[] = + !type_ || type_ === "all" + ? ["communities", "posts", "users", "comments"] + : [type_]; + + const searchOne = async (searchType: SearchableType) => { + const response = await this.#client.GET("/api/alpha/search", { + ...options, + params: { + query: { + ...rest, + listing_type: compat.fromListingType(listing_type), + q: search_term, + type_: PIEFED_SEARCH_TYPE[searchType], + }, }, - }, - }); + }); + + // Each response carries every bucket, but only the requested one is + // populated — take just that bucket so a fan-out can't double-count + switch (searchType) { + case "comments": + return response.data!.comments.map(compat.toCommentView); + case "communities": + return response.data!.communities.map(compat.toCommunityView); + case "posts": + return response.data!.posts.map(compat.toPostView); + case "users": + return response.data!.users.map(compat.toPersonView); + } + }; + + const data = (await Promise.all(searchTypes.map(searchOne))).flat(); return { - ...compat.toPageResponse(payload), - data: [ - ...response.data!.communities.map(compat.toCommunityView), - ...response.data!.posts.map(compat.toPostView), - ...response.data!.users.map(compat.toPersonView), - ...response.data!.comments.map(compat.toCommentView), - ], + ...compat.toPageResponse(payload, { items: data.length }), + data, }; } @@ -1034,9 +1081,14 @@ export class UnsafePiefedClient implements BaseClient { params: { query: { sort: "New", ...compat.fromPageParams(payload) } }, }); + const data = response.data!.comments.map(compat.toCommentView); + return { - ...compat.toPageResponse(payload), - data: response.data!.comments.map(compat.toCommentView), + ...compat.toPageResponse(payload, { + items: data.length, + next_page: response.data!.next_page, + }), + data, }; } @@ -1050,9 +1102,14 @@ export class UnsafePiefedClient implements BaseClient { params: { query: { sort: "New", ...compat.fromPageParams(payload) } }, }); + const data = response.data!.posts.map(compat.toPostView); + return { - ...compat.toPageResponse(payload), - data: response.data!.posts.map(compat.toPostView), + ...compat.toPageResponse(payload, { + items: data.length, + next_page: response.data!.next_page, + }), + data, }; } } diff --git a/test/live-smoke.test.ts b/test/live-smoke.test.ts index 985cf8f..6b49b35 100644 --- a/test/live-smoke.test.ts +++ b/test/live-smoke.test.ts @@ -58,6 +58,14 @@ describe.runIf(process.env.LIVE_SMOKE)("live smoke", () => { expect(data.length).toBeGreaterThan(0); }); + it("all-type search passes canonical validation", OPTIONS, async () => { + // PieFed has no all-type search endpoint — the adapter fans out and + // merges, so an unspecified type_ must work everywhere + const { data } = await client.search({ limit: 3, search_term: "news" }); + + expect(data.length).toBeGreaterThan(0); + }); + it("search passes canonical validation", OPTIONS, async () => { const { data } = await client.search({ limit: 3, diff --git a/test/testing-seed-matrix.test.ts b/test/testing-seed-matrix.test.ts index e668824..eeedf90 100644 --- a/test/testing-seed-matrix.test.ts +++ b/test/testing-seed-matrix.test.ts @@ -267,10 +267,8 @@ describe.each([ } as Parameters[0]); expect(third.data.map((view) => view.post.name)).toEqual(["Post 5"]); - // End-of-feed differs by provider: Lemmy stops handing out cursors, - // while the piefed adapter always computes the next page number — so - // consumers there stop on a short page instead. - if (mode === "lemmyv1") expect(third.next_page).toBeUndefined(); + // End of feed means the same thing on every provider: no cursor + expect(third.next_page).toBeUndefined(); }); it("serves a trailing empty page when the last page was full", async () => { @@ -379,6 +377,35 @@ describe.each([ expect(none.data).toHaveLength(0); }); + it("searches every type at once", async () => { + const { client, fake } = setup(); + + fake.seed.community({ name: "cats_only", title: "Cats Only" }); + fake.seed.person({ name: "catlover" }); + fake.seed.comment({ content: "cats are great" }); + + // PieFed's API has no all-type search, so the adapter fans out and + // merges — the canonical result matches Lemmy's single request + const { data } = await client.search({ search_term: "cat" }); + + const kinds = data.map((item) => { + switch (true) { + case "comment" in item: + return "comment"; + case "post" in item: + return "post"; + case "community" in item: + return "community"; + default: + return "person"; + } + }); + + expect(new Set(kinds)).toEqual( + new Set(["comment", "community", "person", "post"]), + ); + }); + it("seed.clear() empties the derived feed", async () => { const { client, fake } = setup(); From ce61a534d19d5fbd434b9139a2fa94445f80c985 Mon Sep 17 00:00:00 2001 From: Alexander Harding Date: Sat, 25 Jul 2026 12:57:52 -0500 Subject: [PATCH 2/2] fix: page inference must allow merged pages to overshoot the limit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/providers/lemmyv0/compat.ts | 27 +++++++++---- src/providers/piefed/index.ts | 22 +++++----- test/toPageResponse.test.ts | 71 +++++++++++++++++++++++++++++++++ 3 files changed, 103 insertions(+), 17 deletions(-) create mode 100644 test/toPageResponse.test.ts diff --git a/src/providers/lemmyv0/compat.ts b/src/providers/lemmyv0/compat.ts index 8337ff2..afb250d 100644 --- a/src/providers/lemmyv0/compat.ts +++ b/src/providers/lemmyv0/compat.ts @@ -510,8 +510,14 @@ export function toModlogView( * * `next_page` has to mean "there is more" — handing one out unconditionally * makes a consumer's `while (next_page)` loop spin forever. The server's own - * cursor wins when the endpoint returns one; otherwise a full page implies - * there may be another and a short page is the end. + * cursor wins when the endpoint returns one; otherwise a page at least as + * long as the limit implies there may be another, and a shorter one is the + * end. + * + * "At least" matters: endpoints that merge several requests (notifications, + * person content, all-type search, modlog) legitimately overshoot, because + * these servers apply the limit per bucket. A merged page shorter than the + * limit means every bucket was short — genuinely exhausted. */ export function toPageResponse( params: types.PageParams, @@ -529,10 +535,17 @@ export function toPageResponse( "lemmyv0 does not support string page_cursor", ); - if (page?.next_page !== undefined) - return { - next_page: page.next_page === null ? undefined : Number(page.next_page), - }; + if (page?.next_page !== undefined) { + const serverCursor = + page.next_page === null ? undefined : Number(page.next_page); + + // A cursor we can't use as a page number is no cursor at all — better + // to report end-of-feed than to send the server garbage + if (serverCursor === undefined || !Number.isFinite(serverCursor)) + return { next_page: undefined }; + + return { next_page: serverCursor }; + } const nextPage = (page_cursor ?? 1) + 1; @@ -540,7 +553,7 @@ export function toPageResponse( // keep assuming there's more if (!page || params.limit === undefined) return { next_page: nextPage }; - return { next_page: page.items === params.limit ? nextPage : undefined }; + return { next_page: page.items >= params.limit ? nextPage : undefined }; } export function toPerson( diff --git a/src/providers/piefed/index.ts b/src/providers/piefed/index.ts index a45edd3..062144c 100644 --- a/src/providers/piefed/index.ts +++ b/src/providers/piefed/index.ts @@ -728,21 +728,23 @@ export class UnsafePiefedClient implements BaseClient { switch (payload.type) { case "all": case undefined: { - const response = await Promise.all([ + const [posts, comments] = await Promise.all([ this.#listPersonPosts(payload, options), - this.#listPersonComments(payload, options), - ]).then(([posts, comments]) => - [...posts.data, ...comments.data].sort( - (a, b) => - getPostCommentItemCreatedDate(b) - - getPostCommentItemCreatedDate(a), - ), + ]); + + const data = [...posts.data, ...comments.data].sort( + (a, b) => + getPostCommentItemCreatedDate(b) - getPostCommentItemCreatedDate(a), ); + // Both halves already resolved the server's own cursor; either one + // having more means this merged feed does too. More reliable than + // inferring from the merged length, since piefed silently clamps + // large limits. return { - ...compat.toPageResponse(payload, { items: response.length }), - data: response, + data, + next_page: posts.next_page ?? comments.next_page, }; } diff --git a/test/toPageResponse.test.ts b/test/toPageResponse.test.ts new file mode 100644 index 0000000..a7b02f0 --- /dev/null +++ b/test/toPageResponse.test.ts @@ -0,0 +1,71 @@ +// The page-number providers (Lemmy v0, PieFed) don't all report whether +// more pages exist, so `toPageResponse` derives it. Getting this wrong is +// invisible in the fakes — neither models lemmyv0 — but breaks real feeds, +// so the rules are pinned here directly. + +import { describe, expect, it } from "vitest"; + +import { InvalidPayloadError } from "../src/errors"; +import { toPageResponse } from "../src/providers/lemmyv0/compat"; + +describe("toPageResponse", () => { + it("prefers the server's own cursor", () => { + expect( + toPageResponse({ limit: 10 }, { items: 10, next_page: "3" }), + ).toEqual({ next_page: 3 }); + }); + + it("treats a null server cursor as end of feed", () => { + expect( + toPageResponse({ limit: 10 }, { items: 10, next_page: null }), + ).toEqual({ next_page: undefined }); + }); + + it("treats an unusable server cursor as end of feed", () => { + // Sending a NaN page number back would be worse than stopping + expect( + toPageResponse({ limit: 10 }, { items: 10, next_page: "abc" }), + ).toEqual({ next_page: undefined }); + }); + + it("infers another page from a full one", () => { + expect(toPageResponse({ limit: 10 }, { items: 10 })).toEqual({ + next_page: 2, + }); + expect( + toPageResponse({ limit: 10, page_cursor: 4 }, { items: 10 }), + ).toEqual({ next_page: 5 }); + }); + + it("infers end of feed from a short page", () => { + expect(toPageResponse({ limit: 10 }, { items: 9 })).toEqual({ + next_page: undefined, + }); + expect(toPageResponse({ limit: 10 }, { items: 0 })).toEqual({ + next_page: undefined, + }); + }); + + it("keeps paging when a merged page overshoots the limit", () => { + // Endpoints that merge several requests (notifications, person content, + // all-type search, modlog) get `limit` items *per bucket*, so page one + // legitimately returns more than the limit — it is not the last page + expect(toPageResponse({ limit: 5 }, { items: 70 })).toEqual({ + next_page: 2, + }); + expect(toPageResponse({ limit: 5 }, { items: 20 })).toEqual({ + next_page: 2, + }); + }); + + it("assumes more when there's no limit to compare against", () => { + expect(toPageResponse({}, { items: 3 })).toEqual({ next_page: 2 }); + expect(toPageResponse({})).toEqual({ next_page: 2 }); + }); + + it("rejects string cursors, which these providers can't page with", () => { + expect(() => toPageResponse({ page_cursor: "abc" }, { items: 1 })).toThrow( + InvalidPayloadError, + ); + }); +});