You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Updates incremental delivery to the pending / incremental / completed / hasNext wire format.
Review scope
Please review only the incremental-delivery wire format and behavior introduced on top of #608. Response-data contracts, generic streaming/middleware behavior, and WebSocket lifecycle are reviewed separately in #611, #612, and #608 and should not be reviewed again here.
The project’s open-order convention puts System namespaces first; moving Xunit above them makes this file inconsistent. Restore the System/FSharp/external ordering.
Preserve StructNullable list defer coverage in a separate test
This repurposes the only TaskSeq defer test that covered StructNullable (ListOf ...), leaving that supported wrapper combination untested. Keep the struct-nullable test and add the completion-order assertion as a separate fact (or retain the original field definition and initial-data assertion here).
This repurposes the only TaskSeq defer test that covered StructNullable (ListOf ...), leaving that supported wrapper combination untested. Keep the struct-nullable test and add the completion-order assertion as a separate fact (or retain the original field definition and initial-data assertion here).
This repurposes the only TaskSeq defer test that covered StructNullable (ListOf ...), leaving that supported wrapper combina...
Fixed in c9cf0e8. The original defer test now again covers StructNullable (ListOf ...), and the completion-order assertion remains in its own separate fact.
Engine, translator and serialization tests for what the branch already
delivers (end-to-end `pending`/`incremental`/`completed`/`hasNext`
sequences with protocol invariants, defer inside stream, error bubbling
inside a deferred payload, null parents, root-level and mutation defers),
plus skipped tests for every spec feature not implemented yet (`if`,
`initialCount`, stream `label`, fragment-level defer, `subPath`, validation
rules) and three real-socket tests of the `graphql-transport-ws` middleware
hosted through `WebApplicationFactory`.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Each connection now runs a reader loop (sole reader of the socket), a
control loop (sole owner of the subscription registry) and a sender loop
(sole writer of the socket, including its close), plus one worker per
subscription whose observer only queues events. Every producer writes into
a `System.Threading.Channels` channel, so no lock, no `SemaphoreSlim` gate,
no `.Wait()` and no start-up barrier remain; the connection-init timeout is
a `Task.WhenAny` instead of a cancelled receive, and shutdown closes the
socket gracefully.
The former closure nest of the middleware is split into WebSocketErrors,
WebSocketMessaging, WebSocketTransport, SubscriptionPayloads,
SubscriptionWorker and WebSocketConnection; `GraphQLSubscriptionsManagement`
and the public `SubscriptionsDict`, `SubscriptionUnsubscriber` and
`OnUnsubscribeAction` aliases are removed, as is the now unused
`Observable.withCompletionMarker`.
Wire contract: `SubscriptionExecutionResult.Data` and `IncrementalResult.Data`
are `Skippable<obj voption>`, `IncrementalResult` gains `SubPath`, a deferred
field is announced at the object containing it and delivered as an object
map of that field, and a field whose announcement never reached the client
is no longer completed.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`@stream` only on list fields; no `@defer` or `@stream` in a subscription
operation or on a mutation root field unless disabled with `if: false`;
labels must be string literals, unique in the document.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…tal delivery
The announcements of nested deferred and streamed fields are carried as
data by a private observable wrapper (`AnnouncedEvents`), so the containing
payload replays them with a plain concatenation instead of capturing them
under a lock at subscription time; `ResolverResult` keeps its signature.
`ofAsyncEnumerableResolved` delivers results through a single-reader
channel: the emitter is the only caller of the observer and releases each
concurrency slot after emitting, which keeps the throttle semantics of the
`SemaphoreSlim`, the only synchronization primitive left.
`@defer` and `@stream` declare `if: Boolean = true` and `label: String`,
`@stream` also `initialCount: Int = 0`; `@defer` is allowed on fields,
fragment spreads and inline fragments, `@stream` on fields only. `if: false`
executes the field inline (a literal is decided while planning, a variable
at execution), `initialCount` delivers the first items with the initial
payload and streams the rest through the same enumerator, and a stream's
label is carried by its announcement.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- `DeferredPending` carries `InitialCount`, so the graphql-transport-ws
translator expects the streamed items of a `@stream(initialCount: n)`
field from index n instead of buffering them forever.
- `@defer` and `@stream` labels are unique per operation, over the
fragments the operation spreads, instead of across the whole document.
- `errors` is omitted from a result payload when there are none, as the
GraphQL response format requires.
- `GraphQLTransportWS.SubProtocol` names the sub-protocol once.
- Directive names in XML comments are wrapped in `<c>`, the comments
referring to the specification link to it, and the constructors of the
new connection types document their parameters.
- Review suggestions applied: `vtryFind`/`vchoose`, open-statement groups,
the stale test header, release notes grouped by action.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A doc comment that uses any tag must start with `<summary>`, otherwise
the compiler escapes the tags into the text. The project instructions
now also say where the parameters of a primary constructor are
documented.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Spec links added as <see href> to the directive definitions, the validation rules and the translator in 00eaebb; the XML comments that used a tag without <summary> were wrapped in 72180cf.
The parameters of a primary constructor are documented on the
constructor itself: the `<param>` lines go between the type name and
the parameter list, not into the type's comment. The project
instructions now show the form.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The nested stream test accepts its two items delivered either one by one
or as the one batch the engine produces when both resolve into the same
buffered event, and the delivery wait allows for a slow test item taking
several times its sleep on a busy CI runner.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
The broad public-contract, execution, incremental translation, and concurrent WebSocket lifecycle changes require final human validation despite substantial focused coverage.
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
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.
xperiandri
left a comment
Summary
Updates incremental delivery to the
pending/incremental/completed/hasNextwire format.Review scope
Please review only the incremental-delivery wire format and behavior introduced on top of #608. Response-data contracts, generic streaming/middleware behavior, and WebSocket lifecycle are reviewed separately in #611, #612, and #608 and should not be reviewed again here.
Dependency chain: #611 → #612 → #608 → this PR.
Verification
Focused incremental/WebSocket tests passed (85 passed, 1 skipped).
git diff --checkis clean.