Skip to content

feat(smithy): provides fixes based on C# spike#55

Open
Tr00d wants to merge 2 commits into
feat/smithy-modelsfrom
feat/smithy-models-findings-csharp
Open

feat(smithy): provides fixes based on C# spike#55
Tr00d wants to merge 2 commits into
feat/smithy-modelsfrom
feat/smithy-models-findings-csharp

Conversation

@Tr00d

@Tr00d Tr00d commented Jul 3, 2026

Copy link
Copy Markdown

Model fixes from the C# codegen spike (SDK-1107)

The C# spike (supabase-community/supabase-csharp#273) generated clients from these models with two toolchains (NSwag 14.7.1, Kiota 1.32.4) and ran both against a live local platform (supabase start). The runs surfaced defects in the models that static review had not caught. Both generators translated the models faithfully — every issue below is a model/conversion gap, not a generator bug — so each fix propagates to every SDK on regeneration. This PR fixes what is fixable, documents what is not, and was validated by rerunning the spike against the fixed artifacts (results at the bottom).

1. List responses are wrapped in an envelope the API does not send

Issue. The model wraps list outputs in a structure (ListBucketsOutput { items: BucketList }), producing response schemas like {"items": [...]}. The live API returns bare arrays:

GET /bucket   → [ {...}, {...} ]     (actual)
              → { "items": [...] }   (modelled)

Impact. Generated clients fail in opposite ways: Kiota deserializes the mismatch silently and returns an empty list — no error, wrong data on a green call; NSwag throws on deserialization. Affects ListBuckets, ListObjects, DeleteObjects, CreateSignedUrls.

Why. Smithy operation outputs must be structures, and restJson1 rejects @httpPayload on list members ("AWS Protocols only support binding the following shape types to the payload: string, blob, structure, union, and document") — a bare-array response is inexpressible in the model.

Fix. patch-openapi.py unwraps single-items envelopes into top-level array schemas (the same mechanism already used for multipart). The model documents the limitation on ListBucketsOutput.

2. Optional scalar fields are not nullable

Issue. Non-@required members convert to plain non-nullable schemas. Generators for languages with non-nullable value types (C#, Kotlin) map them to types whose default is a real value (0/false) and serialize it.

Impact (observed live). CreateBucket sent file_size_limit: 0 → the bucket rejects every upload with 413. ListObjects sent limit: 0 → 400.

Fix. patch-openapi.py adds nullable: true to optional number/integer/boolean properties, so generators emit optional value types (long?, int?). Scoped to scalars deliberately: blanket-nullable would claim null is a valid value for fields where the server rejects it (e.g. sortBy must be an object or absent).

3. Streaming bodies convert to $ref + format: byte

Issue. @streaming blob / blob payloads emit $ref'd schemas with format: byte (base64). Generators then buffer and base64-encode instead of streaming (NSwag emits byte[]; swift-openapi-generator needs binary to emit HTTPBody).

Fix. All application/octet-stream request/response bodies are now inline {"type": "string", "format": "binary"} — Functions (all five methods, request + response), TUS UploadChunk, and the Database row payloads. Generalises the previous UploadChunk-only patch.

4. Greedy path label leaks + into OpenAPI

Issue. Smithy's {wildcardPath+} greedy label carries the RFC-6570 + into path templates and parameter names — invalid identifiers in generated code (24 C# compile errors; SDK teams had to pre-patch the spec locally before generating).

Fix. patch-openapi.py strips the + from path templates and parameter names. Consumers can now feed the committed artifacts to generators unmodified.

5. Build is not reproducible from a fresh clone

Issue. smithy-build.json resolved dependencies only from file://~/.m2/repository, so smithy build fails on any machine without a pre-warmed local Maven cache.

Fix. Added Maven Central as a fallback repository.

Also in this PR

  • Component schemas orphaned by fixes 1 and 3 (*ResponseContent envelopes, *Payload byte schemas) are removed from the generated output.
  • All three OpenAPI artifacts regenerated (smithy build + patch-openapi.py).

Known gaps, not addressed here

  • Auth is not modelled — needs models or a formal decision to scope Auth out as hand-written.
  • Write operations model 200 only — the API can return 204 with an empty body; clients currently handle this in hand-written code.
  • Multipart non-file parts (cacheControl, metadata) are already optional on this branch — no change needed.

Validation — spike rerun against the fixed artifacts

Both generators were rerun on the regenerated artifacts (no local pre-patching needed anymore), both spike projects compile with 0 errors, and the live harnesses were rerun against supabase start:

Check Result
Kiota live harness 4/4 passListBuckets returns real counts (previously silently 0), ListObjects counts the uploaded object
NSwag live harness ListBuckets deserializes (previously threw); CreateBucket works without file_size_limit (server accepts null-as-unset — no more 413 bucket); streaming upload works
NSwag remaining failure ListObjects 400: NSwag serializes unset members as explicit nulls and the server rejects limit: null. Generator-side serialization behaviour (WhenWritingNull + nullable value types in client config), not a model defect
Streaming signatures NSwag Functions now generates Stream body + streamed FileResponse via ResponseHeadersRead (previously buffered base64 byte[]); Kiota unchanged (Task<Stream>). Live Functions run still pending (needs a deployed edge function)

🤖 Generated with Claude Code

@Tr00d
Tr00d requested a review from a team as a code owner July 3, 2026 12:29
@Tr00d
Tr00d marked this pull request as draft July 3, 2026 12:30
@grdsdev
grdsdev marked this pull request as ready for review July 21, 2026 08:26
@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (1)
  • develop

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ac1bd286-3147-465b-9b7a-ec08a9981d42

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

@grdsdev

grdsdev commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

@copilot resolve the merge conflicts in this pull request

…ithy-models-findings-csharp

# Conflicts:
#	smithy/openapi/FunctionsService.openapi.json

Copilot AI commented Jul 21, 2026

Copy link
Copy Markdown

@copilot resolve the merge conflicts in this pull request

Merged origin/feat/smithy-models into this branch (commit eb59d8b). The only conflict was in smithy/openapi/FunctionsService.openapi.json — the base branch had regenerated it with functionName+ in paths, while our branch already had that + stripped by patch-openapi.py and also had the inline binary schema fixes applied. Resolved by keeping our HEAD version of the artifact (which is the correct post-patched state) while accepting the functions.smithy model update (greedy label) from the base branch.

Copilot AI requested a review from grdsdev July 21, 2026 08:29
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.

3 participants