Skip to content

feat: Add pagination args to getConversations - #49

Merged
amix merged 2 commits into
mainfrom
amix/conversations-pagination-args
Jul 10, 2026
Merged

feat: Add pagination args to getConversations#49
amix merged 2 commits into
mainfrom
amix/conversations-pagination-args

Conversation

@amix

@amix amix commented Jul 10, 2026

Copy link
Copy Markdown
Member

Context

getConversations could only fetch one server-default page (20 rows), which is how quiet conversations went missing from tdc conversation list ("Disappearing group DMs"). Consumers need a cursor to page to exhaustion.

What was changed

  • GetConversationsArgsSchema: optional olderThan (Date), beforeId, limit.
  • Client converts olderThanolder_than_ts explicitly (the transport's generic snake-casing turns a Date into an empty object); beforeId/limit ride the normal snake-casing.
  • olderThan + beforeId together form the strict compound (last_active, id) cursor from Doist/twist-new-backend#690; doc block explains the paging recipe.
  • Wire tests pin the param shapes and that the minimal { workspaceId } call is unchanged.

Out of scope

  • comms-cli adoption (paginate-to-exhaustion) — follow-up PR.
  • Compound-cursor requests against servers without twist-new-backend#690 get a BAD_REQUEST; plain olderThan-only paging keeps legacy inclusive semantics.

Refs

🤖 Generated with Claude Code

olderThan (Date), beforeId, and limit page through /conversations/get.
Paired olderThan+beforeId forms the strict compound (last_active, id)
boundary added in twist-new-backend#690. The Date converts to
older_than_ts in the client because the transport's generic
snake-casing turns a Date into an empty object.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@amix amix added the 👀 Show PR PR must be reviewed before or after merging label Jul 10, 2026
@amix
amix marked this pull request as ready for review July 10, 2026 16:03

@doistbot doistbot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This PR adds optional pagination arguments (olderThan, beforeId, limit) to getConversations, enabling cursor-based paging to exhaustion so quiet conversations no longer go missing.

Few things worth tightening:

  • Guard the empty-page case: When a page has zero conversations (empty workspace or final page), dereferencing last.lastActive will throw. Break the pagination loop on an empty page or only issue the next request when last exists.
  • Encode the compound-cursor contract in the type: The current GetConversationsArgs type permits beforeId without olderThan, but the documented strict cursor requires both together. A union or refinement would prevent consumers from constructing requests with undefined pagination semantics.
  • Prefer explicit field-by-field params construction: The spread pattern (const { olderThan, ...rest } = args) implicitly forwards all remaining schema fields, unlike the explicit picking in getThreads/getComments. If a future z.date() field is added to the schema without a matching conversion, snakeCaseKeys would silently turn it into {} on the wire. Matching the sibling methods' style makes that bug class structurally impossible.

I also included a few optional follow-up notes in the details below.

Optional follow-up note (1)
  • P3 src/clients/conversations-client.ts:78: The spread pattern (const { olderThan, ...rest } = args; const params = { ...rest }) differs from the explicit field-by-field construction in getThreads and getComments. Those methods pick each field individually (e.g. if (args.channelId != null) params.channelId = args.channelId), making the field-to-wire-key mapping explicit and auditable. The spread approach implicitly forwards all remaining schema fields: if a future dev adds another z.date() to GetConversationsArgsSchema without adding a matching conversion here, snakeCaseKeys would turn it into {} and paramsSerializer would serialize [object Object] on the wire. The test guards against this for olderThan specifically, but the explicit pattern used in sibling methods makes that class of bug structurally impossible rather than test-caught.

Share FeedbackReview Logs

Comment thread src/clients/conversations-client.ts Outdated
Comment thread src/types/requests.ts
@amix
amix requested a review from doistbot July 10, 2026 16:07

@doistbot doistbot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This PR adds support for cursor-based pagination in getConversations by introducing optional olderThan, beforeId, and limit arguments with safe Date serialization on the wire.

Few things worth tightening:

  • Empty-page crash guard: The JSDoc paging example throws a TypeError when a page is empty (as page[page.length - 1] is undefined); adding a quick if (page.length === 0) break guard will prevent developers from copying a crashing pattern.
  • Strict cursor validation: The schema currently permits beforeId without olderThan, which can result in undefined pagination behavior. Consider using a union or .refine() to enforce that beforeId requires olderThan at the type level.
  • Explicit parameter mapping: Instead of spreading arguments implicitly into snakeCaseKeys, using the explicit field-by-field assignment pattern from sibling methods (like getThreads) would prevent future date/object fields from accidentally getting serialized incorrectly.

I also included a few optional follow-up notes in the details below.

Optional follow-up note (1)
  • P3 src/clients/conversations-client.ts:78: The spread pattern (const { olderThan, ...rest } = args; const params = { ...rest }) implicitly forwards every remaining schema field through snakeCaseKeys, unlike the explicit field-by-field construction in getThreads (lines 82–88) and getComments (lines 58–62). Those sibling methods pick each field individually with if (args.X != null) params.X = args.X, making the field-to-wire-key mapping explicit and auditable. If a future z.date() field is added to GetConversationsArgsSchema without a matching conversion here, snakeCaseKeys would turn it into {} and paramsSerializer would serialize [object Object] on the wire — a bug the explicit pattern makes structurally impossible.

Share FeedbackReview Logs

Comment thread src/clients/conversations-client.ts Outdated
Comment thread src/types/requests.ts
Explicit field-by-field params (matching getThreads/getComments) so a
future Date field cannot silently hit the generic snake-casing; the doc
example no longer dereferences an empty page; beforeId's standalone
id-order mode is documented.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@amix
amix merged commit 65f1217 into main Jul 10, 2026
4 checks passed
@amix
amix deleted the amix/conversations-pagination-args branch July 10, 2026 16:40
doist-release-bot Bot added a commit that referenced this pull request Jul 10, 2026
## [0.11.0](v0.10.0...v0.11.0) (2026-07-10)

### Features

* Add pagination args to getConversations ([#49](#49)) ([65f1217](65f1217))
@doist-release-bot

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 0.11.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@scottlovegrove scottlovegrove left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

👍🏻

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

Labels

released 👀 Show PR PR must be reviewed before or after merging

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants