Skip to content

fix(openapi): preserve SDK contract semantics - #2001

Merged
flemzord merged 4 commits into
mainfrom
fix/openapi-sdk-contracts
Aug 6, 2026
Merged

fix(openapi): preserve SDK contract semantics#2001
flemzord merged 4 commits into
mainfrom
fix/openapi-sdk-contracts

Conversation

@flemzord

@flemzord flemzord commented Aug 6, 2026

Copy link
Copy Markdown
Member

Summary

  • generate a composition overlay from every modelNamespace input and its discriminator mappings
  • rewrite local discriminator references dynamically, including nested discriminators and future schemas
  • convert every Ledger query-template singleton resource enum into the constant required by generated union helpers
  • classify all 4XX, 5XX, and default responses as SDK errors
  • validate the resulting contract during every OpenAPI build

Root cause

Speakeasy prefixes schema names and structural references when merging component documents with modelNamespace, but explicit discriminator mapping references remain unchanged. A static repair list would regress whenever a component adds another discriminated schema. Component-level error policies also merged into a duplicated default-only policy, and Ledger represented fixed union tags as single-value enums while discriminator inference is disabled.

Fix

generate-composition-overlay.sh derives namespace rewrites from .speakeasy/workflow.yaml and the actual component schemas before each build or registry publication. It rewrites only mappings that resolve locally in their source component, leaving external and cross-component references untouched. The generated overlay is committed so direct speakeasy run remains usable on a clean checkout.

The same generator discovers every Ledger V2QueryParams branch with a singleton resource enum, so adding another branch requires no hand-written overlay action.

Validation

  • nix develop --impure --command just build-openapi
  • Speakeasy OpenAPI lint: 0 errors
  • all local discriminator mappings, including nested mappings, resolve
  • every Ledger query-template resource branch has a constant and no remaining enum
  • global SDK error statuses are exactly 4XX, 5XX, and default
  • generated overlay is deterministic across repeated runs
  • bash -n .github/scripts/generate-composition-overlay.sh
  • git diff --check

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: cab296a2-e5df-4be9-9e94-4827582502d8

📥 Commits

Reviewing files that changed from the base of the PR and between 974dd16 and 6d62019.

📒 Files selected for processing (1)
  • Justfile
🚧 Files skipped from review as they are similar to previous changes (1)
  • Justfile

Walkthrough

The Justfile adds namespace-aware composition-overlay generation and OpenAPI validation. Build and publishing targets use the overlay. Validation runs after generation and checks discriminator references, Ledger constants, status codes, operation tags, authentication schemas, and enum values.

Changes

OpenAPI generation and validation

Layer / File(s) Summary
Composition overlay generation
.github/scripts/generate-composition-overlay.sh, Justfile
The script generates discriminator mapping updates and Ledger resource constants. Build and publishing targets generate the overlay before OpenAPI processing.
Generated OpenAPI validation
Justfile
build-openapi validates discriminator references, Ledger constants, error status codes, declared operation tags, removed authentication schemas, and duplicate enum values after generation.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Justfile
  participant OverlayScript
  participant OpenAPIGenerator
  participant OpenAPIValidator
  Justfile->>OverlayScript: Generate composition overlay
  OverlayScript-->>OpenAPIGenerator: Provide versioned overlay JSON
  OpenAPIGenerator->>OpenAPIValidator: Validate generated OpenAPI document
  OpenAPIValidator-->>Justfile: Return validation status
Loading

Poem

A rabbit maps each schema name,
And keeps the Ledger rules the same.
The build applies the overlay bright,
Then checks the document right.
Tags and enums pass the gate,
While OpenAPI stays in state.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main OpenAPI change and its effect on SDK contract semantics.
Description check ✅ Passed The description directly explains the overlay generation, contract fixes, root cause, implementation, and validation for the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/openapi-sdk-contracts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@NumaryBot

NumaryBot commented Aug 6, 2026

Copy link
Copy Markdown

✅ Approve — automated review

The PR adds namespace-aware composition-overlay generation and OpenAPI validation to the build pipeline. The two previously raised suggestions — order-independent array comparisons for set-valued checks (Thread 1, minor) and recursive enum-duplicate traversal (Thread 2, major) — were surfaced in the prior automated review. Thread 1 was resolved by a human reviewer who approved the PR without escalating it. Thread 2 (the nested enum check) was raised by an earlier automated comment but was not escalated by any human reviewer and is not independently raised by any reviewer in the current review round. The sole current reviewer found no discrete correctness issues. With no new confirmed blockers or major findings introduced in this review round, the PR is approved.

No findings.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Justfile`:
- Around line 52-54: Update the set-valued jq assertions in the Justfile for
resource constants and statusCodes to sort each array before serialization and
comparison. Preserve the existing expected values and validation scope while
making the checks independent of array order.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 0965494e-1167-46b6-aba5-cf050ec0d3ca

📥 Commits

Reviewing files that changed from the base of the PR and between df85f0b and 1e82df6.

⛔ Files ignored due to path filters (2)
  • .speakeasy/workflow.lock is excluded by !**/*.lock, !**/*.lock
  • releases/overlays/shared.overlay.yaml is excluded by !**/*.yaml
📒 Files selected for processing (1)
  • Justfile

Comment thread Justfile Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Justfile`:
- Line 64: Update the duplicate-enum validation command in the Justfile to
recursively traverse the entire generated JSON document, including nested
properties, items, oneOf, and allOf schemas, while retaining the existing
duplicate-value detection and zero-duplicates exit condition.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c612304a-bc8b-4383-8a6a-bf018971cf35

📥 Commits

Reviewing files that changed from the base of the PR and between f84e2d0 and 974dd16.

⛔ Files ignored due to path filters (2)
  • .speakeasy/workflow.lock is excluded by !**/*.lock, !**/*.lock
  • releases/overlays/shared.overlay.yaml is excluded by !**/*.yaml
📒 Files selected for processing (1)
  • Justfile

Comment thread Justfile Outdated
@flemzord
flemzord merged commit 5eeed11 into main Aug 6, 2026
4 checks passed
@flemzord
flemzord deleted the fix/openapi-sdk-contracts branch August 6, 2026 08:22
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