Skip to content

chore(sdks): drop x-speakeasy-pagination overlay from all SDK sources#919

Merged
leggetter merged 1 commit into
fix/spec-sdk-fixesfrom
chore/drop-pagination-overlay
May 28, 2026
Merged

chore(sdks): drop x-speakeasy-pagination overlay from all SDK sources#919
leggetter merged 1 commit into
fix/spec-sdk-fixesfrom
chore/drop-pagination-overlay

Conversation

@alexluong
Copy link
Copy Markdown
Collaborator

What

Removes pagination-config-overlay.yaml from all three SDK sources (TS, Go, Python) in .speakeasy/workflow.yaml, and deletes the overlay file. SDK regeneration is not included in this commit — to be done as a follow-up so the workflow change reviews cleanly.

Why

The shared pagination overlay added x-speakeasy-pagination to list endpoints. Speakeasy turned that annotation into:

In TS the wrapper is a clear DX regression. In Go/Python the helper saves one line at the cost of hiding which request params get carried across the cursor call. Net: not worth it — symmetric, explicit cursor paging is cleaner across all three.

Originally the annotation lived in the Python-only overlay (auto-pagination for Python users). f5b4f45 extracted it into a shared overlay applied to all three sources on the assumption Go/TS would also benefit; turns out the ergonomic tradeoff doesn't pan out.

Caller-side impact (post-regen)

TypeScript

Before:

const response = await sdk.events.list({ tenantId });
for await (const page of response) {
  for (const ev of page.result.models) { ... }
}
// or: response.result.models + response.next()

After:

const result = await sdk.events.list({ tenantId });
for (const ev of result.models) { ... }
if (result.pagination?.next) {
  const more = await sdk.events.list({ tenantId, next: result.pagination.next });
}

Return type: Promise<PageIterator<...>>Promise<EventPaginatedResult>.

Go

Before:

res, err := s.Events.List(ctx, operations.ListEventsRequest{})
if res.EventPaginatedResult != nil {
    for {
        // handle items
        res, err = res.Next()
        if res == nil { break }
    }
}

After:

res, err := s.Events.List(ctx, operations.ListEventsRequest{})
if res.EventPaginatedResult != nil {
    for _, ev := range res.EventPaginatedResult.Models { ... }
    if res.EventPaginatedResult.Pagination.Next != nil {
        req := operations.ListEventsRequest{Next: res.EventPaginatedResult.Pagination.Next}
        res, err = s.Events.List(ctx, req)
    }
}

Lost: the Next func() (*ListEventsResponse, error) field. Response envelope unchanged.

Python

Before:

res = outpost.events.list(request={})
while res is not None:
    # res.models
    res = res.next()

After:

res = outpost.events.list(request={})
# res.models, res.pagination.next
while res.pagination and res.pagination.next:
    res = outpost.events.list(request={"next": res.pagination.next})

Return type: ListEventsResponse (with .next()) → EventPaginatedResult.

Follow-ups

The shared pagination-config-overlay added x-speakeasy-pagination to list
endpoints, which Speakeasy then converted into PageIterator wrappers (TS),
res.Next() helpers (Go), and res.next() chaining (Python).

In TS the wrapper changed the response shape to response.result.models,
which is a noisy regression for the common single-page case. In Go and
Python the helper saves one line at the cost of hiding which request
params get carried across the cursor call.

Drop the overlay from all three sources for symmetric, explicit DX:
callers paginate manually using res.pagination.next on the flat response.

SDK regeneration is intentionally left out of this commit so the workflow
change can be reviewed in isolation.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@leggetter leggetter changed the base branch from main to feat/spec-sdk-fixes May 28, 2026 16:35
@leggetter leggetter merged commit 030a980 into fix/spec-sdk-fixes May 28, 2026
1 check passed
@leggetter leggetter deleted the chore/drop-pagination-overlay branch May 28, 2026 17:08
leggetter added a commit that referenced this pull request May 28, 2026
…verlay drop

Trigger: #919 (merged into umbrella) drops the x-speakeasy-pagination
overlay, so the TS SDK no longer wraps list responses in PageIterator.
events.list()/tenants.list() return EventPaginatedResult/TenantPaginatedResult
directly — accessor is `response.models`, not `response.result.models`.

Tests in #920 had been adapted to the wrapper shape; this realigns them
back to the flat shape post-regen. Local run after fresh TS SDK regen:
153 passing / 0 failing / 15 pending.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
alexluong added a commit that referenced this pull request May 29, 2026
…event.data oneOf (#924)

* fix(openapi): add type discriminator to DestinationUpdate union (#920)

Trigger: the DestinationUpdate union lost its discriminator in 8be278c
(May 2025), when `type` wasn't on PATCH. Server has since added `type`
acceptance and RFC 7396 merge-patch (bd7701b), making the missing
discriminator an active bug — Hookdeck's permissive `config: {}` variant
greedily matched any partial-config payload, the SDK sent it through
with no field remap (camelCase reached the server), and merge-patch
silently no-op'd. All non-Webhook partial config updates affected.

Spec: `type` becomes a required, enum-locked discriminator on every
DestinationUpdate* variant; 17 new `*ConfigUpdate`/`*CredentialsUpdate`
companions model the now-explicit partial PATCH shape; Hookdeck's
`config: {}` removed.

Impact:
  * Breaking for typed SDK consumers: `update()` must include `type`
    after regen. Server is more lenient (still accepts PATCH without
    `type`), so raw HTTP callers keep working but are spec-noncompliant.
  * SDK regen left to the bot — separate PR. CI compile-check on this
    PR will fail until that lands; tests were validated locally
    against a 1.4.1 regen.

spec-sdk-tests (knock-on): `type` added to 46 update sites; paginator
shape fixed (supersedes fix/spec-sdk-tests-paginator-shape); SDK retries
enabled; cleanup parallelized; cleanup-error logging hardened.

Test status (managed Outpost, local regen'd SDK): 133→152 passing,
14→0 failing. spec-sdk-tests have no CI today; follow-up PR will add one.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(spec): expose event.data on Attempt when include=event.data (#917)

* test(spec-sdk): fix paginator response shape (response.result.models)

Speakeasy CLI 1.741.7 → 1.753.0 (commit 3f311e0, 2026-03-13) changed
the generated TS SDK return type for list operations from the flat
paginated body to a PageIterator wrapper: ListEventsResponse now wraps
the body in `result`, so callers access `response.result.models` instead
of `response.models`. The shape change was a side-effect of the
Speakeasy version bump and wasn't surfaced in the SDK regen PR changelog
(which only flagged the unrelated `request` query-param restyle).

Tests in spec-sdk-tests/tests/{events,tenants}.test.ts still used the
pre-bump accessor and failed to compile against the current SDK.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(spec-sdk): assert event.data is exposed when include=event.data

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(spec): order EventFull before EventSummary in Attempt.event oneOf

Speakeasy's TS generator emits zod unions in declaration order. The
non-strict EventSummary matched first and silently stripped event.data
from the parsed attempt — losing the payload added by include=event.data.
Declaring EventFull first lets it match when data is present, with
EventSummary as the fallback.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore(sdks): drop x-speakeasy-pagination overlay from all SDK sources (#919)

The shared pagination-config-overlay added x-speakeasy-pagination to list
endpoints, which Speakeasy then converted into PageIterator wrappers (TS),
res.Next() helpers (Go), and res.next() chaining (Python).

In TS the wrapper changed the response shape to response.result.models,
which is a noisy regression for the common single-page case. In Go and
Python the helper saves one line at the cost of hiding which request
params get carried across the cursor call.

Drop the overlay from all three sources for symmetric, explicit DX:
callers paginate manually using res.pagination.next on the flat response.

SDK regeneration is intentionally left out of this commit so the workflow
change can be reviewed in isolation.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(spec-sdk): revert paginator wrapper accessors after pagination overlay drop

Trigger: #919 (merged into umbrella) drops the x-speakeasy-pagination
overlay, so the TS SDK no longer wraps list responses in PageIterator.
events.list()/tenants.list() return EventPaginatedResult/TenantPaginatedResult
directly — accessor is `response.models`, not `response.result.models`.

Tests in #920 had been adapted to the wrapper shape; this realigns them
back to the flat shape post-regen. Local run after fresh TS SDK regen:
153 passing / 0 failing / 15 pending.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Alex Luong <alex.luong@hookdeck.com>
leggetter added a commit that referenced this pull request May 29, 2026
… a local Outpost (#925)

* fix(openapi): add type discriminator to DestinationUpdate union (#920)

Trigger: the DestinationUpdate union lost its discriminator in 8be278c
(May 2025), when `type` wasn't on PATCH. Server has since added `type`
acceptance and RFC 7396 merge-patch (bd7701b), making the missing
discriminator an active bug — Hookdeck's permissive `config: {}` variant
greedily matched any partial-config payload, the SDK sent it through
with no field remap (camelCase reached the server), and merge-patch
silently no-op'd. All non-Webhook partial config updates affected.

Spec: `type` becomes a required, enum-locked discriminator on every
DestinationUpdate* variant; 17 new `*ConfigUpdate`/`*CredentialsUpdate`
companions model the now-explicit partial PATCH shape; Hookdeck's
`config: {}` removed.

Impact:
  * Breaking for typed SDK consumers: `update()` must include `type`
    after regen. Server is more lenient (still accepts PATCH without
    `type`), so raw HTTP callers keep working but are spec-noncompliant.
  * SDK regen left to the bot — separate PR. CI compile-check on this
    PR will fail until that lands; tests were validated locally
    against a 1.4.1 regen.

spec-sdk-tests (knock-on): `type` added to 46 update sites; paginator
shape fixed (supersedes fix/spec-sdk-tests-paginator-shape); SDK retries
enabled; cleanup parallelized; cleanup-error logging hardened.

Test status (managed Outpost, local regen'd SDK): 133→152 passing,
14→0 failing. spec-sdk-tests have no CI today; follow-up PR will add one.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(spec): expose event.data on Attempt when include=event.data (#917)

* test(spec-sdk): fix paginator response shape (response.result.models)

Speakeasy CLI 1.741.7 → 1.753.0 (commit 3f311e0, 2026-03-13) changed
the generated TS SDK return type for list operations from the flat
paginated body to a PageIterator wrapper: ListEventsResponse now wraps
the body in `result`, so callers access `response.result.models` instead
of `response.models`. The shape change was a side-effect of the
Speakeasy version bump and wasn't surfaced in the SDK regen PR changelog
(which only flagged the unrelated `request` query-param restyle).

Tests in spec-sdk-tests/tests/{events,tenants}.test.ts still used the
pre-bump accessor and failed to compile against the current SDK.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(spec-sdk): assert event.data is exposed when include=event.data

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(spec): order EventFull before EventSummary in Attempt.event oneOf

Speakeasy's TS generator emits zod unions in declaration order. The
non-strict EventSummary matched first and silently stripped event.data
from the parsed attempt — losing the payload added by include=event.data.
Declaring EventFull first lets it match when data is present, with
EventSummary as the fallback.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore(sdks): drop x-speakeasy-pagination overlay from all SDK sources (#919)

The shared pagination-config-overlay added x-speakeasy-pagination to list
endpoints, which Speakeasy then converted into PageIterator wrappers (TS),
res.Next() helpers (Go), and res.next() chaining (Python).

In TS the wrapper changed the response shape to response.result.models,
which is a noisy regression for the common single-page case. In Go and
Python the helper saves one line at the cost of hiding which request
params get carried across the cursor call.

Drop the overlay from all three sources for symmetric, explicit DX:
callers paginate manually using res.pagination.next on the flat response.

SDK regeneration is intentionally left out of this commit so the workflow
change can be reviewed in isolation.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(spec-sdk): revert paginator wrapper accessors after pagination overlay drop

Trigger: #919 (merged into umbrella) drops the x-speakeasy-pagination
overlay, so the TS SDK no longer wraps list responses in PageIterator.
events.list()/tenants.list() return EventPaginatedResult/TenantPaginatedResult
directly — accessor is `response.models`, not `response.result.models`.

Tests in #920 had been adapted to the wrapper shape; this realigns them
back to the flat shape post-regen. Local run after fresh TS SDK regen:
153 passing / 0 failing / 15 pending.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci(spec-sdk-tests): add workflow that runs the contract suite against a local Outpost

Closes #921.

Trigger: PRs touching the spec, TS SDK, Speakeasy config, handlers,
destination providers, models, or server entry. Plus workflow_dispatch
and a weekly cron as drift safety net.

Job: spin up Postgres/Redis/RabbitMQ as service containers, build the
outpost binary from source, run migrations, start api + delivery in
background, wait for /healthz, build the TS SDK from this PR's spec
state, then run `npm test` in spec-sdk-tests pointed at localhost.

Notes:
* Uses RabbitMQ for the message queue (default in .outpost.yaml.dev).
* spec-sdk-tests/.env is written inline so we never need to commit a
  CI-specific .env to the repo.
* Server logs are dumped on failure for debuggability.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci(spec-sdk-tests): fix YAML — use block scalar for migrations run step

The quoted-string form 'run: "$OUTPOST_BIN" migrate apply --yes' confuses
the YAML parser (treats the leading quote as opening a scalar). Block-scalar
form matches the other run: steps in the file.

* ci(spec-sdk-tests): use outpost-server in singular mode, not the CLI wrapper

The 'outpost' binary is just a CLI wrapper that delegates 'serve' to
a separate 'outpost-server' binary. Running it without a subcommand
prints help and exits — which is why /healthz never came up.

Switch to building both binaries: 'outpost' for migrations, then
'outpost-server' for the actual server. Drop SERVICE=api / SERVICE=delivery
in favour of singular mode (empty SERVICE → api + log + delivery in one
process), which is enough for the contract tests and halves the moving
parts.

* ci(spec-sdk-tests): use npm install for spec-sdk-tests deps

spec-sdk-tests/.gitignore excludes package-lock.json (intentional —
the test suite isn't published, and treating SDK contract tests as
production-locked dependencies adds noise to PRs without value).
npm ci needs a lock file; switch to npm install.

* ci(spec-sdk-tests): regen TS SDK from the PR's spec before running tests

The whole point of this workflow is to validate that spec ↔ SDK ↔
server agree. If we test against the checked-in SDK src/ (which can
lag the spec arbitrarily, since regen is bot-driven), we're testing
the wrong thing — drift hides instead of surfacing.

Add a Speakeasy regen step before the existing build. Now any PR that
changes docs/apis/openapi.yaml gets its tests run against an SDK
regen'd from that PR's spec, exercising the actual contract this CI
exists to validate.

Uses the SPEAKEASY_API_KEY secret already configured for the publish
workflows.

* ci(spec-sdk-tests): use redis-stack-server for RediSearch

tenants.list requires RediSearch; vanilla redis:7-alpine returns
501 'list tenant feature is not enabled'. Swap to the official
redis/redis-stack-server image which bundles RediSearch. Same port
(6379), same redis-cli ping for health check.

* ci(spec-sdk-tests): expand header with explicit scope and rationale

Makes it clear at the top of the file what this workflow validates
(PR's server vs PR's spec), what it does not (managed drift, SDK
version compat, post-deploy smoke), and why we picked this shape
over alternatives. Spares future maintainers from re-litigating
the design decision.

* ci(spec-sdk-tests): drop the weekly cron

The cron was copied from the docs-eval pattern out of habit. In this
workflow it adds essentially no signal — between PRs, main's state
hasn't changed, so a scheduled run re-tests what we last tested. The
"deploy drift" question a cron would help with is out of scope for
this workflow anyway (it's about PR's spec vs PR's server, not
managed vs main).

* ci(spec-sdk-tests): add cmd/outpost-server/** to trigger paths

Workflow builds and runs ./cmd/outpost-server but the path filter only
watched cmd/outpost/** (the CLI wrapper). Changes to the actual server
entrypoint could land without triggering the contract suite.

Spotted by Copilot review on #925.

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Alex Luong <alex.luong@hookdeck.com>
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