Adopt pending/incremental/completed delivery format by xperiandri · Pull Request #609 · fsprojects/FSharp.Data.GraphQL

xperiandri · 2026-09-18T14:01:24Z

Summary

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.

Dependency chain: #611 → #612 → #608 → this PR.

Verification

Focused incremental/WebSocket tests passed (85 passed, 1 skipped). git diff --check is clean.

github-actions · 2026-09-19T14:06:13Z

Test Results

9 files 9 suites 13m 12s ⏱️
801 tests 796 ✅ 5 💤 0 ❌
2 403 runs 2 388 ✅ 15 💤 0 ❌

Results for commit 3be1952.

♻️ This comment has been updated with latest results.

Copilot

Copilot review overview

🟡 Changes recommended

Lazy stream announcements break Apollo merging, while early failures and empty streams produce incorrect completion behavior.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 · 2 · 1

Open (4)

Preannounce streams to ensure Apollo merges streamed items · New
Preserve stream type for pre-item failures · New
Handle empty streams with pending and completed entries · New
Correct comment describing mixed-success deferred error batches · New

What changed in this PR

Adopts the GraphQL 17 incremental-delivery wire format for WebSocket responses.

Changes:

Adds incremental result contracts and stateful delivery translation.
Emits completion markers for deferred and streamed fields.
Updates serialization, behavior tests, and documentation.

File	Description
`RELEASE_NOTES.md`	Documents the new wire format.
`docs/type-system.md`	Documents WebSocket incremental delivery.
`src/FSharp.Data.GraphQL.Shared/WebSockets.fs`	Defines incremental payload contracts.
`src/FSharp.Data.GraphQL.Server/IO.fs`	Adds deferred completion events.
`src/FSharp.Data.GraphQL.Server/Execution.fs`	Emits completion markers.
`src/FSharp.Data.GraphQL.Server.AspNetCore/IncrementalDelivery.fs`	Translates engine events into wire payloads.
`src/FSharp.Data.GraphQL.Server.AspNetCore/GraphQLWebsocketMiddleware.fs`	Integrates incremental translation.
`src/FSharp.Data.GraphQL.Server.AspNetCore/GraphQLRequestHandler.fs`	Logs completion events.
`src/FSharp.Data.GraphQL.Server.AspNetCore/FSharp.Data.GraphQL.Server.AspNetCore.fsproj`	Includes the translator source.
`tests/FSharp.Data.GraphQL.Tests/Helpers.fs`	Adds completion-filtering helper.
`tests/FSharp.Data.GraphQL.Tests/DeferredTests.fs`	Adapts deferred tests.
`tests/FSharp.Data.GraphQL.Tests/TaskSeqFieldTests.fs`	Tests stream completion behavior.
`tests/FSharp.Data.GraphQL.Tests/MiddlewareTests.fs`	Adapts middleware assertions.
`tests/FSharp.Data.GraphQL.Tests/Relay/NodeTests.fs`	Adapts Relay assertions.
`tests/FSharp.Data.GraphQL.Tests/AspNetCore/SerializationTests.fs`	Tests new payload serialization.
`tests/FSharp.Data.GraphQL.Tests/AspNetCore/IncrementalDeliveryTests.fs`	Tests delivery translation and ordering.
`tests/FSharp.Data.GraphQL.Tests/AspNetCore/IncrementalPayloadSplittingTests.fs`	Removes obsolete splitting tests.
`tests/FSharp.Data.GraphQL.Tests/FSharp.Data.GraphQL.Tests.fsproj`	Registers replacement tests.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

xperiandri · 2026-09-19T14:44:45Z

@copilot descrive GQLDeferredResponseContent cases with XML comments

Copilot · 2026-09-19T14:46:53Z

@copilot descrive GQLDeferredResponseContent cases with XML comments

Addressed in 1f96f19.

Co-authored-by: xperiandri <2365592+xperiandri@users.noreply.github.com>

xperiandri · 2026-09-19T23:15:44Z

+                        (fun event ->
+                            lock gate (fun () ->
+                                if capturePendingPrefix then
+                                    match event with
+                                    | DeferredPending _ -> pendingPrefix.Add event
+                                    | _ ->
+                                        capturePendingPrefix <- false
+                                        tail.OnNext event
+                                else
+                                    tail.OnNext event)),
+                        (fun ex ->
+                            lock gate (fun () ->
+                                capturePendingPrefix <- false
+                                tail.OnError ex)),
+                        (fun () ->
+                            lock gate (fun () ->
+                                capturePendingPrefix <- false
+                                tail.OnCompleted ()))


@copilot better to declare local functions with meaningful names

Addressed in 4c2490d.

Co-authored-by: xperiandri <2365592+xperiandri@users.noreply.github.com>

@defer

These three files were reformatted wholesale by Fantomas even though their dev versions were not Fantomas-formatted, which buried the real change. Restore formatting to match dev so the diff shows only the @defer label argument and the deferred-test updates. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

xperiandri

@copilot I thought System.Threading.Channels will help making code with less locks and mutable variables. But I don't see much difference. Is it possible?

Co-authored-by: xperiandri <2365592+xperiandri@users.noreply.github.com>

Copilot · 2026-09-20T00:06:23Z

@copilot I thought System.Threading.Channels will help making code with less locks and mutable variables. But I don't see much difference. Is it possible?

Partly. A fuller channels rewrite can remove more locks and mutable state, but it would need a broader redesign where one worker owns startup, shutdown, and subscription lifecycle end-to-end. In this PR I kept the remaining lock/mutable pieces because they still coordinate synchronous observer callbacks and cancellation/disposal edges safely.

- Replaced `Error` with `ServerError` for protocol errors in `GraphQLWebsocketMiddleware.fs` to clarify error types. - Moved the `GQLExecutionResult` active pattern to a new auto-open module in tests; updated all usages to open this module. - Updated pattern matching to use `.Content` explicitly for clarity. - Fixed record literal syntax in `WebSockets.fs` `CreateSubsequent`. - Refactored subscription observer cleanup with `sendAndUnsubscribe`. - Replaced `awaitBlocking` with `.Wait()` for blocking tasks. - Updated tests to use the new active pattern module. - Minor cleanup: removed unused opens, improved logging, clarified matches.

Copilot

Copilot review overview

🟡 Changes recommended

Variable-backed defer labels are lost, and completed paths cannot be announced again during later live updates.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1

Open (1)

Reset closed path state before reusing nested deferred deliveries · New

Resolved since last review (4)

Direct delivery application bypasses serialized send chain
Variable-based defer labels are silently dropped
Remove obsolete duplicate batch path implementation
Correct contradictory DeferredErrors.Data breaking-change documentation

Co-authored-by: xperiandri <2365592+xperiandri@users.noreply.github.com>

Copilot

Copilot review overview

🔵 Needs a closer look

Variable-based defer labels are currently dropped, and existing struct-nullable TaskSeq coverage was removed.

Review effort: Balanced
Findings: None

Resolved since last review (1)

Reset closed path state before reusing nested deferred deliveries

Previously missed (2)

In code that hasn't changed since last review

Restore namespace ordering with System namespaces first

tests/FSharp.Data.GraphQL.Tests/TaskSeqFieldTests.fs:11

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

tests/FSharp.Data.GraphQL.Tests/TaskSeqFieldTests.fs:197

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).

xperiandri · 2026-09-20T01:23:19Z

tests/FSharp.Data.GraphQL.Tests/TaskSeqFieldTests.fs:197

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).

@copilot fix

Co-authored-by: xperiandri <2365592+xperiandri@users.noreply.github.com>

Copilot · 2026-09-20T01:26:01Z

tests/FSharp.Data.GraphQL.Tests/TaskSeqFieldTests.fs:197

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.

xperiandri force-pushed the task-seq-field-websocket branch from 60987fd to 3ec49f7 Compare September 18, 2026 14:05

xperiandri force-pushed the incremental-delivery-v02 branch from 09a9e82 to 745cd7b Compare September 18, 2026 14:07

xperiandri force-pushed the task-seq-field-websocket branch from 3ec49f7 to 765ac78 Compare September 18, 2026 16:47

xperiandri force-pushed the incremental-delivery-v02 branch from 745cd7b to ef14e43 Compare September 18, 2026 17:00

xperiandri mentioned this pull request Sep 18, 2026

Use [<Struct>] ? optional parameters and voption in Define/DefineRec and the introspection chain #604

Merged

xperiandri force-pushed the task-seq-field-websocket branch from 765ac78 to 73d00c3 Compare September 18, 2026 17:48

xperiandri mentioned this pull request Sep 18, 2026

Hardened WebSocket streaming and incremental delivery #608

Merged

xperiandri force-pushed the incremental-delivery-v02 branch from ef14e43 to 5dee67c Compare September 18, 2026 17:56

xperiandri force-pushed the task-seq-field-websocket branch from 73d00c3 to 500d88a Compare September 18, 2026 19:54

xperiandri force-pushed the incremental-delivery-v02 branch from 5dee67c to 6b2159f Compare September 18, 2026 19:54

xperiandri force-pushed the task-seq-field-websocket branch from 500d88a to f0ad702 Compare September 18, 2026 23:26

xperiandri force-pushed the incremental-delivery-v02 branch from 6b2159f to 24fd2f2 Compare September 18, 2026 23:26

xperiandri force-pushed the task-seq-field-websocket branch from f0ad702 to 19b5f35 Compare September 18, 2026 23:53

xperiandri force-pushed the incremental-delivery-v02 branch from 24fd2f2 to 248c693 Compare September 18, 2026 23:54

xperiandri force-pushed the task-seq-field-websocket branch 2 times, most recently from 3e3dbb6 to 4faa338 Compare September 19, 2026 00:00

Base automatically changed from task-seq-field-websocket to dev September 19, 2026 01:45

xperiandri force-pushed the incremental-delivery-v02 branch from 248c693 to fcad4f4 Compare September 19, 2026 13:57

xperiandri requested a balanced review from Copilot September 19, 2026 14:00

Copilot started reviewing on behalf of xperiandri September 19, 2026 14:01 View session

Copilot AI reviewed Sep 19, 2026

View reviewed changes

Copilot started work on behalf of xperiandri September 19, 2026 14:10 View session

Copilot finished work on behalf of xperiandri September 19, 2026 14:43

Copilot started work on behalf of xperiandri September 19, 2026 14:45 View session

Copilot finished work on behalf of xperiandri September 19, 2026 14:47

xperiandri force-pushed the incremental-delivery-v02 branch from 1f96f19 to 6a187e2 Compare September 19, 2026 15:06

xperiandri commented Sep 19, 2026

View reviewed changes

Comment thread src/FSharp.Data.GraphQL.Server/IO.fs

Comment thread tests/FSharp.Data.GraphQL.Tests/Helpers.fs Outdated

Rewrite deferred websocket delivery with channels

e01cdb9

Co-authored-by: xperiandri <2365592+xperiandri@users.noreply.github.com>

xperiandri commented Sep 19, 2026

View reviewed changes

Rewrite deferred websocket delivery around channels

17faa71

Co-authored-by: xperiandri <2365592+xperiandri@users.noreply.github.com>

Copilot finished work on behalf of xperiandri September 19, 2026 23:16

Copilot started work on behalf of xperiandri September 19, 2026 23:17 View session

Use clearer local deferred handler names

094b8c7

Co-authored-by: xperiandri <2365592+xperiandri@users.noreply.github.com>

Copilot finished work on behalf of xperiandri September 19, 2026 23:20

xperiandri and others added 2 commits September 20, 2026 01:23

fixup! Rewrite deferred websocket delivery around channels

ffe88e4

xperiandri force-pushed the incremental-delivery-v02 branch from fee54f7 to ffe88e4 Compare September 19, 2026 23:52

xperiandri commented Sep 20, 2026

View reviewed changes

Copilot started work on behalf of xperiandri September 20, 2026 00:01 View session

Refactor websocket middleware helpers

2dd7d3c

Co-authored-by: xperiandri <2365592+xperiandri@users.noreply.github.com>

Copilot finished work on behalf of xperiandri September 20, 2026 00:06

xperiandri requested a balanced review from Copilot September 20, 2026 00:52

Copilot started reviewing on behalf of xperiandri September 20, 2026 00:53 View session

Copilot AI reviewed Sep 20, 2026

View reviewed changes

Comment thread src/FSharp.Data.GraphQL.Server.AspNetCore/IncrementalDelivery.fs

Copilot started work on behalf of xperiandri September 20, 2026 01:05 View session

Fix incremental delivery path reuse

bd0b743

Co-authored-by: xperiandri <2365592+xperiandri@users.noreply.github.com>

Copilot finished work on behalf of xperiandri September 20, 2026 01:13

xperiandri requested a balanced review from Copilot September 20, 2026 01:17

Copilot started reviewing on behalf of xperiandri September 20, 2026 01:17 View session

Copilot AI reviewed Sep 20, 2026

View reviewed changes

Copilot started work on behalf of xperiandri September 20, 2026 01:23 View session

Restore StructNullable TaskSeq defer coverage

c9cf0e8

Co-authored-by: xperiandri <2365592+xperiandri@users.noreply.github.com>

Copilot finished work on behalf of xperiandri September 20, 2026 01:26

Conversation

xperiandri commented Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Review scope

Verification

Uh oh!

github-actions Bot commented Sep 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xperiandri commented Sep 19, 2026

Uh oh!

Copilot AI commented Sep 19, 2026

Uh oh!

Uh oh!

Uh oh!

xperiandri Sep 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 19, 2026

Choose a reason for hiding this comment

Uh oh!

xperiandri left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented Sep 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

xperiandri commented Sep 20, 2026

Uh oh!

Copilot AI commented Sep 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

xperiandri commented Sep 18, 2026 •

edited

Loading

github-actions Bot commented Sep 19, 2026 •

edited

Loading