Skip to content

fix: paginate GET /chat (limit + before cursor)#110

Open
bmersereau wants to merge 3 commits into
willchen96:mainfrom
bmersereau:fix/105-chat-list-pagination
Open

fix: paginate GET /chat (limit + before cursor)#110
bmersereau wants to merge 3 commits into
willchen96:mainfrom
bmersereau:fix/105-chat-list-pagination

Conversation

@bmersereau
Copy link
Copy Markdown

@bmersereau bmersereau commented May 13, 2026

Summary

  • Adds limit (default 50, max 200) and before (ISO timestamp cursor) query parameters to GET /chat
  • Prevents unbounded chat list queries for active users with large chat histories
  • Backward-compatible: existing callers with no query params get the first 50 chats (most recent)

Closes #105
Closes #114

Changes

  • backend/src/routes/chat.ts — limit + before-cursor pagination on the chat list query
  • backend/src/lib/__tests__/chatPagination.test.ts — static analysis tests verifying pagination is present
  • backend/tsconfig.json — exclude __tests__ dirs from the main tsc build (tests use vitest directly)
  • backend/package.json"test": "vitest run" script added

Test plan

  • Unit tests added and passing (npx vitest run)
  • Backend type check passes (npx tsc --noEmit)

Adds `limit` (default 50, max 200) and `before` (ISO timestamp cursor)
query parameters to prevent unbounded chat list queries for active users.
Copy link
Copy Markdown
Author

@bmersereau bmersereau left a comment

Choose a reason for hiding this comment

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

PR Review

Summary

Adds cursor-based pagination (limit + before ISO timestamp) to GET /chat. Implementation is correct and backward-compatible — callers with no params get 50 chats. The limit cap at 200 is reasonable. Two static-analysis tests verify the limit and cursor are present.

Findings

  • [severity:praise] before cursor validated with Number.isNaN(new Date(...).getTime()) — correctly rejects non-ISO strings while accepting valid timestamps
  • [severity:nit] The before cursor is applied after the .limit() call, meaning Supabase will apply lt("created_at", before) as a WHERE clause and limit as LIMIT — correct behavior, but a comment clarifying cursor-vs-offset semantics would help future readers
  • [severity:minor] Static tests use readFileSync on the source file; they verify the pattern is present but don't test the actual endpoint behavior (e.g., correct rows returned). Acceptable for now, but a true integration test would give higher confidence

Specific checks

  • "test": "vitest run" in package.json ✓
  • Tests pass: 2/2 ✓
  • tsconfig.json excludes __tests__ from tsc build ✓

Verdict

Approve with nits — ship it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant