Claude/optimize playlist queries n1nt b by marcodejongh · Pull Request #908 · marcodejongh/boardsesh

marcodejongh · 2026-03-04T10:11:17Z

No description provided.

…hunking Replace unbounded single-request pattern with incremental fetch + chunking to fix server validation failures and eliminate wasteful refetching: - Create generic useIncrementalQuery hook that tracks fetched UUIDs, only fetches new ones, chunks requests >500 into parallel batches, and accumulates results in a stable cache key with optimistic update support - Refactor useClimbActionsData to use useIncrementalQuery for both favorites and playlist membership queries - Add server-side validation for favorites climbUuids (max 500, matching existing playlistsForClimbs limit) - Update tests with incremental fetch and reorder-stability scenarios https://claude.ai/code/session_0146aSM1evtmv8pnVAazL6FP

https://claude.ai/code/session_0146aSM1evtmv8pnVAazL6FP

claude · 2026-03-04T10:13:18Z

Claude Review

✅ Ready to merge - Minor issues noted below, but nothing blocking.

Issues

Memory growth - fetchedUuidsRef in use-incremental-query.ts:61 grows unboundedly during a session with no cleanup mechanism except logout. Users browsing many climbs could accumulate thousands of entries.
Missing test coverage for useIncrementalQuery - The new hook lacks dedicated unit tests. Current tests only exercise it indirectly through useClimbActionsData. Missing coverage for:
- Chunk failure scenarios (when one of multiple parallel fetches fails)
- Edge cases around cache invalidation/reset behavior
Redundant state updates - use-incremental-query.ts:145-149 calls setAccumulated(cached) on all cache update events, including self-triggered ones from the merge effect at line 122. This causes unnecessary re-renders (not a bug, just wasteful).
Removed error logging - use-climb-actions-data.tsx:67-76 removed try-catch blocks with detailed error logging. While React Query handles errors, the specific context logs are now lost which may make debugging harder.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 064c0e941f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-04T10:16:11Z

packages/web/app/hooks/use-incremental-query.ts

+  const newUuids = useMemo(
+    () => (enabled ? uuids.filter((uuid) => !fetchedUuidsRef.current.has(uuid)) : []),
+    [uuids, enabled, invalidationCount],


Reset fetched UUID tracking when query identity changes

Track key changes (accumulatedKey/fetchKeyPrefix) as part of the incremental state reset, because newUuids currently depends only on uuids, enabled, and invalidationCount. When callers change context (e.g., boardName or angle in useClimbActionsData) but pass the same UUID list, those UUIDs stay marked as fetched and no new request is issued, so stale accumulated results from the previous context are reused.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-03-04T10:16:11Z

packages/web/app/hooks/use-climb-actions-data.tsx

-      await queryClient.cancelQueries({ queryKey: favoritesQueryKey });
-      const previousFavorites = queryClient.getQueryData<Set<string>>(favoritesQueryKey);
-      queryClient.setQueryData<Set<string>>(favoritesQueryKey, (old) => {
+      await queryClient.cancelQueries({ queryKey: favAccKey });


Cancel in-flight favorites fetches before optimistic toggle

Canceling only favAccKey in onMutate does not stop active incremental fetch queries (['favorites', ..., 'fetch', ...]), so a stale response started before the toggle can still merge afterward and overwrite the optimistic removal/addition. This race is user-visible when toggling while favorites are loading, because the UI can revert to the pre-toggle state until a later manual refresh.

Useful? React with 👍 / 👎.

…lQuery tests - Add lastCacheWriteRef guard to skip self-triggered cache subscription events, preventing redundant setAccumulated calls from the merge effect's own write - Restore try-catch error logging in fetchChunk callbacks for both favorites and playlist memberships to aid debugging - Add comprehensive useIncrementalQuery unit tests covering chunking, chunk failure, cache invalidation, optimistic updates, auth reset, and incremental fetch behavior https://claude.ai/code/session_0146aSM1evtmv8pnVAazL6FP

claude · 2026-03-04T10:23:10Z

Claude Review

✅ Ready to merge - Well-structured optimization with good test coverage. No significant issues found.

Minor observations (non-blocking)

hasSetChanged/hasMapChanged only compare sizes (use-climb-actions-data.tsx:39-45) - This works correctly for the merge-only use case, but the naming suggests content comparison. Consider renaming to hasSizeChanged or adding a comment clarifying this is intentional for performance.
Backend validation now rejects empty arrays (queries.ts:58) - The new FavoritesQueryClimbUuidsSchema.min(1) validation would reject empty climbUuids arrays. The frontend guards against this (enabled && newUuids.length > 0), so no breaking change, but worth noting for any future direct API consumers.

Tests are comprehensive - covering incremental fetching, chunking, error handling, cache invalidation, and optimistic updates.

P1 fixes: - Reset fetchedUuidsRef when accumulatedKey identity changes (e.g., user switches board or angle). Previously, same UUID list with a different context would reuse stale accumulated results from the old context. - Cancel in-flight fetch queries in onMutate (not just accumulated key) to prevent a completing fetch from overwriting the optimistic update. Expose cancelFetches() from useIncrementalQuery for this purpose. Minor: - Rename hasSetChanged/hasMapChanged to hasSetSizeChanged/hasMapSizeChanged with comments clarifying the size-only comparison is intentional for performance (merge always grows, so size change is a reliable signal). - Add test for context switch reset behavior. https://claude.ai/code/session_0146aSM1evtmv8pnVAazL6FP

claude · 2026-03-04T11:36:36Z

Claude Review

✅ Ready to merge - Well-implemented optimization with comprehensive tests. Minor observations noted below.

Minor Observations (non-blocking)

Shallow merge assumption - use-climb-actions-data.tsx:43: mergeMapFn doesn't deep-merge Sets for overlapping keys (just overwrites). This works due to the incremental pattern guaranteeing no key overlap, but is an implicit assumption.
Mutation race condition coverage - use-climb-actions-data.tsx:232-277: addToPlaylist and removeFromPlaylist don't cancel in-flight fetches before updating cache (unlike toggleFavorite which does). Not a practical issue due to incremental fetch pattern, but inconsistent.
Render-time mutation - use-incremental-query.ts:83-90: Mutating ref during render is documented and intentional, but could cause issues with React concurrent features.

Positive Notes

Good test coverage for edge cases (chunking, reorder stability, cache invalidation, context switching)
Server validation properly enforces 500-item limit matching existing playlistsForClimbs
No documentation updates needed - internal optimization only

- Replace render-time ref mutation with effect-based keyChangeCount for React concurrent mode safety (discarded renders won't leave stale refs) - Cancel in-flight fetches in addToPlaylist/removeFromPlaylist (consistent with toggleFavorite) to prevent stale responses overwriting cache - Document shallow merge assumption on mergeMapFn - Remove duplicate prevAccKeyRef effect (consolidated into key-change effect) - Fix implicit any parameters in cache subscription - Fix test TypeScript: type mockFetchChunk, widen literal types for context switch test rerender https://claude.ai/code/session_0146aSM1evtmv8pnVAazL6FP

claude · 2026-03-04T11:45:43Z

Claude Review

✅ Ready to merge - Well-designed incremental query pattern with solid test coverage. Minor observations below, but nothing blocking.

Minor Notes

use-incremental-query.ts:199 - The key comparison uses reference equality (!==) which works fine for primitives but would fail silently if object values were passed in keys. The current usage is safe, but consider documenting this constraint.
use-climb-actions-data.tsx:229 - The boardName !== 'moonboard' condition for playlist memberships is an implicit business rule that could benefit from a brief comment explaining why MoonBoard is excluded.
Test coverage - Comprehensive tests for both hooks covering incremental fetching, chunking, error handling, optimistic updates, and cache invalidation. Good edge case coverage.

- Document primitive-only constraint on accumulatedKey/fetchKeyPrefix - Note reference equality usage in cache subscription key comparison - Explain why MoonBoard is excluded from playlist memberships query https://claude.ai/code/session_0146aSM1evtmv8pnVAazL6FP

claude · 2026-03-04T12:06:27Z

Claude Review

✅ Ready to merge - Well-designed incremental query pattern with comprehensive tests. No significant issues found.

Minor observations (non-blocking):

use-incremental-query.ts:214 - The lastCacheWriteRef clearing logic is correct but subtle. The self-triggered event skip pattern works, just noting it's a tricky area for future maintainers.
Test query order assumption (use-climb-actions-data.test.tsx) - Tests assume mockRequest calls arrive in a specific order (favorites, playlists, memberships). React Query doesn't guarantee this, but since queries use different documents, the tests work correctly in practice.

Verified:

Chunking logic matches server validation limit (500)
Cache cleanup on unmount and context change
Race condition handling via cancelFetches for optimistic updates
Proper cleanup effect closure behavior for cache key changes

claude added 2 commits March 4, 2026 10:09

Update package-lock.json after dependency install

064c0e9

https://claude.ai/code/session_0146aSM1evtmv8pnVAazL6FP

chatgpt-codex-connector bot reviewed Mar 4, 2026

View reviewed changes

marcodejongh merged commit 4eee7c1 into main Mar 4, 2026
5 of 6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Claude/optimize playlist queries n1nt b#908

Claude/optimize playlist queries n1nt b#908
marcodejongh merged 6 commits intomainfrom
claude/optimize-playlist-queries-n1ntB

marcodejongh commented Mar 4, 2026

Uh oh!

claude bot commented Mar 4, 2026

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

chatgpt-codex-connector bot Mar 4, 2026

Uh oh!

chatgpt-codex-connector bot Mar 4, 2026

Uh oh!

claude bot commented Mar 4, 2026

Uh oh!

claude bot commented Mar 4, 2026

Uh oh!

claude bot commented Mar 4, 2026

Uh oh!

claude bot commented Mar 4, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

marcodejongh commented Mar 4, 2026

Uh oh!

claude bot commented Mar 4, 2026

Claude Review

Issues

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot commented Mar 4, 2026

Claude Review

Minor observations (non-blocking)

Uh oh!

claude bot commented Mar 4, 2026

Claude Review

Minor Observations (non-blocking)

Positive Notes

Uh oh!

claude bot commented Mar 4, 2026

Claude Review

Minor Notes

Uh oh!

claude bot commented Mar 4, 2026

Claude Review

Minor observations (non-blocking):

Verified:

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants