fix(sdk-generator): distinguish nullable values - #60
Merged
calvin-archastro merged 1 commit intoAug 3, 2026
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Review on ArchCode
What changed
OpenAPI
nullablevalues now have a distinct SDK-generator AST representation instead of reusing the representation for an omitted object key. This keeps requiredness and nullability independent across the TypeScript, Python, Swift, and Go backends.For TypeScript, required nullable properties now emit
T | nullwith Zod.nullable(). Optional nullable properties retain both states asT | null | undefinedwith.nullable().optional(). The cross-language emitters and contract-value generators preserve their native equivalents, including recursive references, auth token discovery, required channel payloads, query serialization, and Go pointer depth.Generated TypeScript and Python contract harnesses now start Prism in static mode. Dynamic generation is not needed for these shape contracts and triggers a nondeterministic
json-schema-fakercrash on the valid nestedoneOfstructure in ActivityFeed responses.Scope
Tooling/backend-only. This changes generated SDK source and generated contract-test harnesses; it does not modify the platform API or product UI.
Risk
Medium.
TypeRefis shared by four language backends, so the blast radius extends beyond the TypeScript failure that exposed the defect. Focused tests cover every affected backend and an independent audit checked wrapper traversal, required nullable payloads, query encoding, recursive Swift models, Python auth discovery, and Go contract pointer depth.User impact
SDK consumers receive correct required-nullable types instead of properties that are incorrectly optional. Generated contract tests become deterministic for complex valid response schemas.
Testing
npm run buildinpackages/sdk-generator— passes on generator0.8.0.npm testinpackages/sdk-generator— 339 tests pass.packages/sdk-generator/__tests__/backends/typescript.test.ts,keeps nullability separate from whether an object key is required. It parses a minimal OpenAPI document through the real frontend and emits the TypeScript interface and Zod validator, asserting required-nullable and optional-nullable output together.uses deterministic Prism responses for generated shape contractsin the TypeScript and Python backend test files asserts the generated process configuration does not enable dynamic faker responses.json-schema-faker@0.5.8ActivityFeed crash. Isolated dynamic requests failed roughly 60–66% of the time; static Prism responses succeeded consistently.The repository does not contain an honest end-to-end test that crosses the published npm package and downstream
archastro-jsworkflow boundary. The manual downstream regeneration above covers that boundary for this change; publishing the next generator version and regeneratingarchastro-jsis the follow-up that makes it permanent.Follow-ups and known issues
@archastro/sdk-generatorversion after merge.archastro-jsfrom the refreshed OpenAPI spec with that version, replacing the currently failing generated artifact in PR 44.CustomObjectJoinResponse; itsrequired + nullableschema is valid.