Streaming support for smithy4s-ndjson - #1
Merged
Merged
Conversation
Operations of an `org.polyvariant.ndjson#ndjsonRestJson` service may stream their request body, their response body, or both. Smithy restricts `@streaming` to `blob`/`union` and the protocol requires such a member to carry `@httpPayload`, so there are exactly two framings, applied identically in both directions: a blob is the body verbatim, a union is one JSON value per line. A streamed member is surfaced as an `AsyncIterable` on the generated type itself, so signatures stay ordinary — `watch(input: WatchInput): Promise<WatchOutput>`, with `WatchOutput.events` an `AsyncIterable<WatchEvent>`. The member is left out of the zod object (validating it would mean consuming the stream) and ndjson elements are checked one at a time as they are pulled. Framing is the transport's job: `StreamTransport.requestStream` receives explicit `requestStreamEncoding` / `responseStreamEncoding` rather than inferring from a content type. Services with streaming operations take both halves of the transport; a model with no streaming emits byte-for-byte what it did before. Mocks mirror the client, so a story implements a streaming operation as an async generator. Only the protocol module is needed — the codegen keys off `@streaming` members, so nothing scala-specific from smithy4s-ndjson is involved. Alongside it, a nix flake that type-checks the emitted TypeScript with the real `tsc`. The Scala tests assert on substrings, which cannot catch a type error; running tsc over a committed sample (typecheck/, covering every construct, plus a consumer-side usage file) caught several, including two pre-existing bugs unrelated to streaming: - a `@httpQuery` timestamp was cast to `string | number | boolean`, which a `Date` does not satisfy — it is now serialised per target shape; - a structure whose only member streams intersected with `z.infer` of an empty `z.object`, i.e. `Record<string, never>`, making every property `never`. CI installs nix, runs the check, and fails if the committed sample has drifted from the model. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MGq82T8PTfFTS6s2tH5Xs9
`scalafmtAll` doesn't cover .sbt files — `scalafmtSbtCheck` does, and it was
failing on the new task definitions.
`tsCodegenSampleCheck` is defined on the root build, but the CI step ran it
after `project rootJVM`, where it isn't in scope ("Not a valid command").
Select `project /` first, as the surrounding scalafmtSbtCheck step already
does.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGq82T8PTfFTS6s2tH5Xs9
The assertion predated the empty-object fix: in that model `events` was WatchOutput's only member, so the type is now the stream alone rather than an intersection with `z.infer` of an empty `z.object`. Give Watch a bound `session` header as well, so the scripted test covers the intersection case (stream + other members) rather than only stream-alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MGq82T8PTfFTS6s2tH5Xs9
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.
Adds codegen support for
org.polyvariant.ndjson#ndjsonRestJson, so operations can stream their request body, their response body, or both.The model
Smithy restricts
@streamingtoblob/union, and the protocol requires such a member to carry@httpPayload. That gives exactly two framings, applied identically in both directions:@streaming blobapplication/octet-streamAsyncIterable<Uint8Array>@streaming unionapplication/x-ndjsonAsyncIterable<TheUnion>All four in/out combinations fall out of that.
What it looks like
A streamed member is surfaced as an
AsyncIterableon the generated type itself, so signatures stay ordinary — noOmit<…>at call sites:The member is left out of the zod object (validating it would mean consuming the stream), and ndjson elements are schema-checked one at a time as they are pulled — a bad element throws
StreamDecodeErrorat that element instead of truncating the stream.Framing is the transport's job:
StreamTransport.requestStreamreceives explicitrequestStreamEncoding/responseStreamEncodingrather than inferring from a content type. Mocks mirror the client, so a story implements a streaming operation as an async generator.Only the protocol module is a dependency — the codegen keys off
@streamingmembers, so nothing Scala-specific from smithy4s-ndjson is involved.Backward compatibility
Verified by diffing a non-streaming model's output before and after: byte-for-byte identical. Getting there caught a member-ordering regression that the substring tests missed.
Typechecking the generated TypeScript
The Scala tests assert on substrings, which cannot catch a type error. This adds a nix flake that runs the real
tscover a committed sample (typecheck/— a model covering every construct, plus a consumer-sideusage.tsexercising clients, streams and mocks as a caller would), understrict+erasableSyntaxOnly.It immediately earned its keep, catching two pre-existing bugs unrelated to streaming:
@httpQuerytimestamp was cast tostring | number | boolean, which aDatedoes not satisfy — now serialised per target shape;z.inferof an emptyz.object(Record<string, never>), making every propertynever.CI installs nix, runs
nix flake check, and fails if the committed sample has drifted from the model.Testing
sbtPlugin/scripted, extended with a streaming servicenix flake check— verified it fails on an introduced type error, not just that it passes🤖 Generated with Claude Code
https://claude.ai/code/session_01MGq82T8PTfFTS6s2tH5Xs9