Skip to content

Conversation

@tstanmay13
Copy link
Contributor

@tstanmay13 tstanmay13 commented Dec 10, 2025

Description

Refs: Slack thread from @tstanmay13

Fixes the TypeScript SDK's wire test generator to dynamically calculate the expected hasNextPage() value based on pagination type and example data, instead of hardcoding true.

Link to Devin run: https://app.devin.ai/sessions/6353b685386f4bfd821c4daa4196ef35
Requested by: [email protected] (@tstanmay13)

Changes Made

  • Added helper functions to extract property values from JSON examples using the IR's property path definitions:
    • getPropertyValueFromJson() for response properties
    • getRequestPropertyValueFromJson() for request properties
  • Added calculateExpectedHasNextPage() function that mirrors the SDK's actual pagination logic:
    • Cursor pagination: returns true when next property is non-null and non-empty string
    • Offset pagination: returns true when results.length >= step, or falls back to results.length > 0 when step value can't be extracted (e.g., GET requests with query params)
  • Updated pagination test block to use the calculated value instead of hardcoded true
  • Added version 3.39.1 to versions.yml with changelog entry
  • Added "Last page" example to listWithCursorPagination endpoint in pagination test definition to verify hasNextPage: false case
  • Regenerated seed test fixtures for pagination

Testing

  • Lint checks pass (pnpm run check)
  • TypeScript compilation passes
  • Local seed tests pass for pagination fixtures
  • Generated test correctly expects hasNextPage().toBe(false) for the new "Last page" example

Human Review Checklist

  • Verify the pagination logic in calculateExpectedHasNextPage() matches the SDK's actual implementation
  • Confirm the fallback to resultsLength > 0 when step value can't be extracted is acceptable (this happens for GET requests where limit/step is a query param, not in the request body)
  • Check that property path traversal correctly uses originalName for Name types and wireValue for NameAndWireValue types

CI Notes

Two CI failures are unrelated to this PR:

  • java-model: TypeScript errors in RustFilenameRegistry.ts (Rust generator)
  • test: SourceFetcher.test.ts errors in browser-compatible-base

@devin-ai-integration
Copy link
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@devin-ai-integration devin-ai-integration bot changed the title fix(ts-sdk): dynamically calculate expected hasNextPage in pagination tests fix(typescript): dynamically calculate expected hasNextPage in pagination tests Dec 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants