fix(openapi): preserve SDK contract semantics - #2001
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe ChangesOpenAPI generation and validation
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
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
✅ Approve — automated reviewThe 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. |
There was a problem hiding this comment.
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
⛔ Files ignored due to path filters (2)
.speakeasy/workflow.lockis excluded by!**/*.lock,!**/*.lockreleases/overlays/shared.overlay.yamlis excluded by!**/*.yaml
📒 Files selected for processing (1)
Justfile
There was a problem hiding this comment.
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
⛔ Files ignored due to path filters (2)
.speakeasy/workflow.lockis excluded by!**/*.lock,!**/*.lockreleases/overlays/shared.overlay.yamlis excluded by!**/*.yaml
📒 Files selected for processing (1)
Justfile
Summary
modelNamespaceinput and its discriminator mappingsRoot 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 duplicateddefault-only policy, and Ledger represented fixed union tags as single-value enums while discriminator inference is disabled.Fix
generate-composition-overlay.shderives namespace rewrites from.speakeasy/workflow.yamland 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 directspeakeasy runremains usable on a clean checkout.The same generator discovers every Ledger
V2QueryParamsbranch with a singletonresourceenum, so adding another branch requires no hand-written overlay action.Validation
nix develop --impure --command just build-openapi4XX,5XX, anddefaultbash -n .github/scripts/generate-composition-overlay.shgit diff --check