Skip to content

perf(genre-tree): fetch full genre playlist pages in parallel - #172

Merged
Andreas-Garcia merged 1 commit into
developfrom
feature/parallel-full-playlist-pages
Oct 2, 2026
Merged

Andreas-Garcia merged 1 commit into
developfrom
feature/parallel-full-playlist-pages

Conversation

@Andreas-Garcia

Copy link
Copy Markdown
Member

Summary

useListFullGenrePlaylists asked for pageSize=1000, but grow-api clamps page size to 100 (PAGINATION_PAGE_SIZE_MAX). The canonical tree therefore came back as 18 pages, fetched one after another, so the genre tree waited on 18 sequential round-trips before rendering.

Changes

  • FULL_LIST_PAGE_SIZE is now 100, the server's real maximum, so the request no longer suggests that one call returns everything.
  • fetchAllPages fetches page 1 first, reads totalPages, then fetches pages 2..totalPages in parallel with Promise.all. Results are merged in page order. The return shape is the same as before.
  • Requests have no concurrency limit (marked with a ponytail: comment). Browsers already cap connections per host, so this is fine at the current page count.

Test plan

  • pnpm lint && pnpm build && pnpm test pass locally.
  • New test: a response with a single page makes exactly one fetch.
  • New test: 3 pages and 250 results are merged in order, with page args 1, 2 and 3 at pageSize: 100.

🤖 Generated with Claude Code

grow-api clamps pageSize to 100, so the canonical list (~1800 playlists)
took 18 sequential round-trips. Request the real max and fetch pages
2..totalPages concurrently once page 1 reports the count.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 2, 2026 00:50
@Andreas-Garcia
Andreas-Garcia merged commit fbd8703 into develop Oct 2, 2026
3 checks passed

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The change is small, focused, correctly handles edge cases, preserves the return shape, includes matching tests and a changelog entry, and follows repository conventions.

Review effort: Balanced
Findings: None

What changed in this PR

This PR optimizes useListFullGenrePlaylists in packages/app-kit/src/genre-tree/useGenrePlaylist.ts. The previous implementation requested pageSize=1000 (which grow-api silently clamps to its PAGINATION_PAGE_SIZE_MAX of 100) and then walked pages sequentially via next, meaning the canonical tree waited on ~18 serial round-trips before rendering. The change aligns the requested page size with the server's true maximum and parallelizes the remaining page fetches, reducing wall-clock latency to roughly two sequential waves regardless of page count.

Changes:

  • Lowered FULL_LIST_PAGE_SIZE from 1000 to 100 to match grow-api's real cap, with an explanatory comment.
  • Rewrote fetchAllPages to fetch page 1, read totalPages, then fetch pages 2..totalPages concurrently with Promise.all, merging results in page order while preserving the prior return shape.
  • Added a changelog entry and updated/extended tests to cover the single-page and multi-page ordered-merge paths.
File Description
packages/​app-kit/​src/​genre-tree/​useGenrePlaylist.ts Sets page size to 100 and parallelizes page 2..N fetches based on totalPages.
packages/​app-kit/​src/​genre-tree/​useGenrePlaylist.test.ts Updates pageSize expectations to 100; adds single-page and ordered multi-page merge tests.
CHANGELOG.md Adds an [Unreleased] > Changed entry describing the parallel full-list fetch.

Notes from review (non-blocking): the Math.max(first.totalPages - 1, 0) guard correctly handles the empty/single-page case, Promise.all preserves page order so the merge is deterministic, and the returned shape (page: 1, pageSize: results.length, totalPages: 1, with next/previous/overallTotal from the last page) matches the prior behavior. The // ponytail: marker is an established in-repo convention (also used in TrackListContext.tsx:233), not a stray comment. totalPages is a required field in PaginatedResponseSchema, so trusting it is consistent with the parsed contract; this does trade the old overallTotal/next cross-check for reliance on totalPages, but there is no evidence that field is unreliable.


💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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