Skip to content

Add support for @defer and @stream #3819

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 46 commits into
base: main
Choose a base branch
from
Open

Add support for @defer and @stream #3819

wants to merge 46 commits into from

Conversation

patrick91
Copy link
Member

@patrick91 patrick91 commented Mar 25, 2025

Another attempt :D

Summary by Sourcery

Enable experimental GraphQL incremental execution by adding support for @defer and @stream directives, streaming multipart responses, and related configuration, printer, and execution integrations.

New Features:

  • Support GraphQL incremental execution with @defer and @stream directives, including multipart streaming responses
  • Introduce Streamable annotation and StrawberryConfig flag to enable experimental incremental execution
  • Extend schema printer to include defer and stream directives when experimental execution is enabled

Enhancements:

  • Integrate graphql-core’s experimental_execute_incrementally into async and sync execution flows and remove FieldGroup dependency for GraphQL 3.3 compatibility
  • Revamp async_base_view and encode_multipart_data to handle streaming multipart payloads with proper boundaries and content-length headers
  • Upgrade embedded GraphiQL assets to version 3.8.3

CI:

  • Add GitHub Actions workflow to run E2E tests under the e2e directory using Playwright

Documentation:

  • Add RELEASE.md noting minor release for @defer support

Tests:

  • Add unit tests for multipart subscriptions, basic defer/stream behaviors, and schema printing
  • Add end-to-end demos and Playwright tests for Apollo and Relay clients using defer and stream

Copy link
Contributor

sourcery-ai bot commented Mar 25, 2025

Reviewer's Guide

Integrates experimental incremental GraphQL execution by switching to graphql-core’s experimental_execute_incrementally when enabled; adds multipart streaming support in the async HTTP view; extends schema definitions and printer to include @defer and @stream directives; introduces Streamable annotations; and adds comprehensive incremental tests and E2E suites with a dedicated CI workflow.

Sequence diagram for incremental execution and streaming HTTP response

sequenceDiagram
    participant Client
    participant AsyncBaseView
    participant Schema
    participant experimental_execute_incrementally
    participant GraphQLIncrementalExecutionResults

    Client->>AsyncBaseView: HTTP GraphQL request
    AsyncBaseView->>Schema: execute(...)
    Schema->>experimental_execute_incrementally: execute if incremental enabled
    experimental_execute_incrementally-->>Schema: GraphQLIncrementalExecutionResults
    Schema-->>AsyncBaseView: GraphQLIncrementalExecutionResults
    AsyncBaseView->>AsyncBaseView: stream() (yields multipart chunks)
    AsyncBaseView-->>Client: multipart/mixed streaming response
Loading

ER diagram for BlogPost, Comment, and Author (E2E test schema)

erDiagram
    BLOGPOST ||--o{ COMMENT : has
    COMMENT }o--|| AUTHOR : written_by
    BLOGPOST {
        ID id
        STRING title
        STRING content
    }
    COMMENT {
        ID id
        STRING content
    }
    AUTHOR {
        ID id
        STRING name
    }
Loading

Class diagram for Streamable and related types

classDiagram
    class StrawberryStreamable
    class Streamable {
        <<Annotated[AsyncGenerator[T, None], StrawberryStreamable()]>>
    }
    class AsyncGenerator
    class T
    Streamable --|> AsyncGenerator : Annotated
    Streamable --* StrawberryStreamable : Annotated
    Streamable ..> T : generic
Loading

Class diagram for incremental execution integration in Schema

classDiagram
    class Schema {
        +execute(...)
        +execute_sync(...)
        +_handle_execution_result(...)
        +_parse_and_validate_async(...)
        -_schema
        -config
    }
    class StrawberryConfig {
        +enable_experimental_incremental_execution: bool
    }
    class GraphQLIncrementalExecutionResults
    class experimental_execute_incrementally
    Schema --> StrawberryConfig : config
    Schema ..> GraphQLIncrementalExecutionResults : uses
    Schema ..> experimental_execute_incrementally : uses
Loading

File-Level Changes

Change Details Files
Implement HTTP streaming of incremental GraphQL results
  • Detect GraphQLIncrementalExecutionResults in async_base_view.run and generate multipart/mixed streams
  • Refactor encode_multipart_data to pre-encode JSON, include Content-Length and charset headers
  • Update process_result in http/init.py to accept and early-return incremental results
strawberry/http/async_base_view.py
strawberry/http/__init__.py
Integrate experimental incremental execution into schema execution
  • Add enable_experimental_incremental_execution config flag and choose experimental_execute_incrementally when active
  • Handle GraphQLIncrementalExecutionResults early in _handle_execution_result
  • Update execute and execute_sync to switch execution function based on config
  • Adapt build_resolve_info signature for graphql-core 3.3 compatibility
strawberry/schema/schema.py
Introduce Streamable annotation support
  • Define StrawberryStreamable and Streamable alias in strawberry/streamable.py
  • Enhance annotation resolver to recognize Streamable types
  • Extend ResultType union to include GraphQLIncrementalExecutionResults
  • Expose GraphQLIncrementalExecutionResults and directives via _graphql_core
strawberry/streamable.py
strawberry/annotation.py
strawberry/schema/_graphql_core.py
Extend schema printer to include defer and stream directives
  • Guard access to strawberry-definition extension
  • Append @defer and @stream directive definitions when incremental execution is enabled
strawberry/printer/printer.py
Add end-to-end and incremental test suites with CI workflow
  • Create Playwright-based E2E tests for Apollo and Relay clients in e2e directory
  • Add incremental defer and stream HTTP tests under tests/http/incremental
  • Introduce GitHub Actions workflow 🎭 E2E Tests for running Playwright suite on PRs
  • Include setup docs and configs (README, tsconfigs, CI, noxfile bump)
.github/workflows/e2e-tests.yml
e2e/
tests/http/incremental/

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@botberry
Copy link
Member

botberry commented Mar 25, 2025

Apollo Federation Subgraph Compatibility Results

Federation 1 Support Federation 2 Support
_service🟢
@key (single)🟢
@key (multi)🟢
@key (composite)🟢
repeatable @key🟢
@requires🟢
@provides🟢
federated tracing🔲
@link🟢
@shareable🟢
@tag🟢
@override🟢
@inaccessible🟢
@composeDirective🟢
@interfaceObject🟢

Learn more:

Copy link

codecov bot commented Mar 25, 2025

Codecov Report

Attention: Patch coverage is 60.00000% with 118 lines in your changes missing coverage. Please review.

Project coverage is 94.45%. Comparing base (74c2cef) to head (830ca94).
Report is 8 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3819      +/-   ##
==========================================
- Coverage   94.75%   94.45%   -0.30%     
==========================================
  Files         520      526       +6     
  Lines       33902    34179     +277     
  Branches     1754     1782      +28     
==========================================
+ Hits        32123    32285     +162     
- Misses       1497     1604     +107     
- Partials      282      290       +8     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment on lines +20 to +21
if isinstance(result, GraphQLIncrementalExecutionResults):
return result
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to support this properly

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe in another PR :)

Comment on lines 386 to 389
# for Apollo
# content type is `multipart/mixed;deferSpec=20220824,application/json`
"path": ["blogPost"],
"label": "relayTestsBlogPostQuery$defer$relayTestsCommentsFragment",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should not be hard-coded, in any case having both label and path seems to be ok for both relay and apollo, so we might want to keep them

tbd if we want to keep the .formatted instead of using our own wrappers

apparently there's no good way to define what spec a client is supporting (Apollo sends deferSpec=20220824, but for relay it's up to whoever implements the fetch)

Comment on lines +272 to +276
if self.config.enable_experimental_incremental_execution:
directives = tuple(directives) + tuple(incremental_execution_directives)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to throw errors if user what experimental execution but we are not on graphql core 3.3+

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two raise RuntimeError below, which check that. Maybe it should be here during __init__ instead, and in there just assert?

Comment on lines +550 to +557
# TODO: maybe here use the first result from incremental execution if it exists
if isinstance(result, GraphQLExecutionResult) and result.errors:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure what I meant with that comment :D

but we should probably get rid of the isinstance check?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I remember it correctly, there was one incremental execution type that did not have errors in it (not totally sure, it's been a while 😅)

If the idea is just to limit to results that have errors, maybe hasattr(result, "errors") would be easier?

@@ -0,0 +1,44 @@
import contextlib
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved this from the previous location

"data": {"character": {"id": "1"}},
"hasNext": True,
"pending": [{"path": ["character"], "id": "0"}],
# TODO: check if we need this and how to handle it
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could support these in a future version

Copy link

codspeed-hq bot commented Mar 25, 2025

CodSpeed Performance Report

Merging #3819 will not alter performance

Comparing feature/defer (830ca94) with main (79214e6)

Summary

✅ 26 untouched benchmarks

@botberry
Copy link
Member

botberry commented Mar 31, 2025

Pre-release

👋

Pre-release 0.276.0.dev.1750672223 [afe7eae] has been released on PyPi! 🚀
You can try it by doing:

poetry add strawberry-graphql==0.276.0.dev.1750672223

@strawberry-graphql strawberry-graphql deleted a comment from botberry Mar 31, 2025
@patrick91
Copy link
Member Author

/pre-release

codeflash-ai bot added a commit that referenced this pull request Mar 31, 2025
…efer`)

To optimize the provided code, we can reduce the number of checks by combining related conditionals and accessing properties once instead of multiple times. Furthermore, we can streamline the creation of the `data` dictionary. Here is an optimized version of the code.



In this optimized version.
- We access `result.errors` and `result.extensions` once and store their values in the `errors` and `extensions` variables, respectively.
- We use dictionary unpacking with conditionals to add the "errors" and "extensions" keys to `data` only when they are present.

This should provide a minor performance improvement while maintaining the same functionality.
Copy link

codeflash-ai bot commented Mar 31, 2025

⚡️ Codeflash found optimizations for this PR

📄 100% (1.00x) speedup for process_result in strawberry/http/__init__.py

⏱️ Runtime : 1.78 millisecond 892 microseconds (best of 28 runs)

I created a new dependent PR with the suggested changes. Please review:

If you approve, it will be merged into this PR (branch feature/defer).

@patrick91
Copy link
Member Author

/pre-release

@matclayton
Copy link

Screenshot 2025-04-02 at 17 30 34

Did a quick test on this and fairly sure I've got the following crash. Its possible I've misconfigured something, but this is best understanding on how to enable it

patrick91 pushed a commit that referenced this pull request Apr 15, 2025
…efer`) (#3827)

Co-authored-by: codeflash-ai[bot] <148906541+codeflash-ai[bot]@users.noreply.github.com>
Copy link

codeflash-ai bot commented Apr 15, 2025

This PR is now faster! 🚀 @patrick91 accepted my optimizations from:

Copy link

codeflash-ai bot commented Apr 15, 2025

This PR is now faster! 🚀 codeflash-ai[bot] accepted my code suggestion above.

@patrick91
Copy link
Member Author

@matclayton sorry, I totally missed that message! do you know if you can write a small reproduction? or was that fixed with GraphQL-core 3.3.0a7?

@matclayton
Copy link

Sorry I didn't follow up here, this is fixed in GraphQL core 3.3.0va7, btw we're running GraphQL-core 3.3.0v7 in prod now, and not seen a single issue so far. Need to still confirm the other bits of this PR, I'm hoping the team can take it off me the next week or so.

@patrick91
Copy link
Member Author

/pre-release

@patrick91
Copy link
Member Author

this doesn't render/print the directives (to double check)

@patrick91
Copy link
Member Author

worth checking if those directive should be printed, since we don't print directives like @include and @skip

codeflash-ai bot added a commit that referenced this pull request Jul 1, 2025
…fer`)

Here is an optimized version. Improvements.

- Removed the unnecessary intermediate variables (`errors`, `extensions`) that are only used once.
- Avoided repeated dictionary unpacking by building the dictionary with direct assignments and only adding keys if necessary.
- Used `if-else` statements for error and extensions blocks for slight speed-ups over dictionary unpacking on small dicts.

Rewritten code.



This version allocates less intermediate data and does not do work unless necessary.
Copy link

codeflash-ai bot commented Jul 1, 2025

⚡️ Codeflash found optimizations for this PR

📄 12% (0.12x) speedup for process_result in strawberry/http/__init__.py

⏱️ Runtime : 100 microseconds 89.8 microseconds (best of 153 runs)

I created a new dependent PR with the suggested changes. Please review:

If you approve, it will be merged into this PR (branch feature/defer).

@patrick91 patrick91 marked this pull request as ready for review July 16, 2025 13:58
@botberry
Copy link
Member

botberry commented Jul 16, 2025

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:


This release adds experimental support for GraphQL's @defer and @stream directives, enabling incremental delivery of response data.

Note: this only works when using Strawberry with graphql-core>=3.3.0a9.

Features

  • @defer directive: Allows fields to be resolved asynchronously and delivered incrementally
  • @stream directive: Enables streaming of list fields using the new strawberry.Streamable type
  • strawberry.Streamable[T]: A new generic type for defining streamable fields that work with @stream

Configuration

To enable these experimental features, configure your schema with:

from strawberry.schema.config import StrawberryConfig

schema = strawberry.Schema(
    query=Query, config=StrawberryConfig(enable_experimental_incremental_execution=True)
)

Here's the tweet text:

🆕 Release (next) is out! Thanks to @patrick91 for the PR 👏

This release adds experimental support for GraphQL's @defer and @stream directives, enabling incremental delivery of response data! 🚀

Get it here 👉 https://strawberry.rocks/release/(next)

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @patrick91 - I've reviewed your changes - here's some feedback:

  • The multipart streaming logic in async_base_view has grown quite complex; consider extracting the stream generator and multipart encoding into a reusable helper or class to improve readability and maintainability.
  • In encode_multipart_data, computing Content-Length using len(encoded_data) may be incorrect for non-ASCII content or boundary bytes—consider using the byte length (len(encoded_data.encode('utf-8'))) or otherwise validating the header length.
  • There are several TODO comments around error handling and result processing in the incremental execution paths—please address or formally track these work items so the core execution flow doesn’t rely on unfinished logic.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The multipart streaming logic in async_base_view has grown quite complex; consider extracting the `stream` generator and multipart encoding into a reusable helper or class to improve readability and maintainability.
- In `encode_multipart_data`, computing `Content-Length` using `len(encoded_data)` may be incorrect for non-ASCII content or boundary bytes—consider using the byte length (`len(encoded_data.encode('utf-8'))`) or otherwise validating the header length.
- There are several `TODO` comments around error handling and result processing in the incremental execution paths—please address or formally track these work items so the core execution flow doesn’t rely on unfinished logic.

## Individual Comments

### Comment 1
<location> `strawberry/http/async_base_view.py:437` </location>
<code_context>
         )

     def encode_multipart_data(self, data: Any, separator: str) -> str:
+        encoded_data = self.encode_json(data)
+
</code_context>

<issue_to_address>
The separator is used as a boundary but is hardcoded as '-', which may not be unique.

Consider generating a unique boundary string instead of using '-', to prevent parsing errors if the payload contains the same sequence.

Suggested implementation:

```python
import uuid

                boundary = f"----strawberry-{uuid.uuid4().hex}"
                headers={
                    "Transfer-Encoding": "chunked",
                    "Content-Type": f'multipart/mixed; boundary="{boundary}"',
                },
            )

```

```python
            async def stream() -> AsyncGenerator[str, None]:
                yield f"--{boundary}"

```
</issue_to_address>

### Comment 2
<location> `strawberry/http/async_base_view.py:424` </location>
<code_context>
+                sub_response,
+                headers={
+                    "Transfer-Encoding": "chunked",
+                    "Content-Type": 'multipart/mixed; boundary="-"',
+                },
+            )
</code_context>

<issue_to_address>
The Content-Type boundary value should match the actual boundary used in the body.

Make sure the boundary in the header exactly matches the one used in the multipart body. It's best to use a unique string for the boundary.

Suggested implementation:

```python
                    boundary = "graphql-subscription-12345"
                    yield self.encode_multipart_data(response, boundary)

                yield f"--{boundary}--\r\n"

            return await self.create_streaming_response(
                request,
                stream,
                sub_response,
                headers={
                    "Transfer-Encoding": "chunked",

```

```python
                    "Content-Type": f'multipart/mixed; boundary={boundary}',
                },
            )

```
</issue_to_address>

### Comment 3
<location> `strawberry/http/__init__.py:19` </location>
<code_context>

-def process_result(result: ExecutionResult) -> GraphQLHTTPResponse:
-    data: GraphQLHTTPResponse = {"data": result.data}
+def process_result(result: ResultType) -> GraphQLHTTPResponse:
+    if isinstance(result, GraphQLIncrementalExecutionResults):
+        return result
</code_context>

<issue_to_address>
Returning 'result' directly when it's a GraphQLIncrementalExecutionResults may break expected return type.

Returning a GraphQLIncrementalExecutionResults instead of a GraphQLHTTPResponse may lead to type inconsistencies for consumers expecting a dictionary-like response.
</issue_to_address>

### Comment 4
<location> `strawberry/annotation.py:339` </location>
<code_context>
         return _annotation_is_maybe(annotation)

+    @classmethod
+    def _is_streamable(cls, annotation: Any, args: list[Any]) -> bool:
+        return any(isinstance(arg, StrawberryStreamable) for arg in args)
+
     @classmethod
</code_context>

<issue_to_address>
Type checking with isinstance on annotation arguments may not be robust.

Consider checking types directly or using typing.get_origin/get_args, as annotation arguments may not always be instances.
</issue_to_address>

<suggested_fix>
<<<<<<< SEARCH
    def _is_streamable(cls, annotation: Any, args: list[Any]) -> bool:
        return any(isinstance(arg, StrawberryStreamable) for arg in args)
=======
    def _is_streamable(cls, annotation: Any, args: list[Any]) -> bool:
        from typing import get_origin, get_args

        def is_streamable_type(arg):
            # Check if the argument is the StrawberryStreamable type or a generic of it
            origin = get_origin(arg) or arg
            return origin is StrawberryStreamable

        return any(is_streamable_type(arg) for arg in args)
>>>>>>> REPLACE

</suggested_fix>

### Comment 5
<location> `strawberry/schema/schema.py:534` </location>
<code_context>
-def process_result(result: ExecutionResult) -> GraphQLHTTPResponse:
-    data: GraphQLHTTPResponse = {"data": result.data}
+def process_result(result: ResultType) -> GraphQLHTTPResponse:
+    if isinstance(result, GraphQLIncrementalExecutionResults):
+        return result

-    if result.errors:
</code_context>

<issue_to_address>
Returning the raw incremental execution result may break expected interfaces.

Returning GraphQLIncrementalExecutionResults instead of ExecutionResult may cause runtime errors if consumers expect the standard interface. Please ensure the returned type matches the expected interface.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@@ -363,12 +435,16 @@ async def run(
)

def encode_multipart_data(self, data: Any, separator: str) -> str:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (bug_risk): The separator is used as a boundary but is hardcoded as '-', which may not be unique.

Consider generating a unique boundary string instead of using '-', to prevent parsing errors if the payload contains the same sequence.

Suggested implementation:

import uuid

                boundary = f"----strawberry-{uuid.uuid4().hex}"
                headers={
                    "Transfer-Encoding": "chunked",
                    "Content-Type": f'multipart/mixed; boundary="{boundary}"',
                },
            )
            async def stream() -> AsyncGenerator[str, None]:
                yield f"--{boundary}"

sub_response,
headers={
"Transfer-Encoding": "chunked",
"Content-Type": 'multipart/mixed; boundary="-"',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (bug_risk): The Content-Type boundary value should match the actual boundary used in the body.

Make sure the boundary in the header exactly matches the one used in the multipart body. It's best to use a unique string for the boundary.

Suggested implementation:

                    boundary = "graphql-subscription-12345"
                    yield self.encode_multipart_data(response, boundary)

                yield f"--{boundary}--\r\n"

            return await self.create_streaming_response(
                request,
                stream,
                sub_response,
                headers={
                    "Transfer-Encoding": "chunked",
                    "Content-Type": f'multipart/mixed; boundary={boundary}',
                },
            )

Comment on lines +339 to +340
def _is_streamable(cls, annotation: Any, args: list[Any]) -> bool:
return any(isinstance(arg, StrawberryStreamable) for arg in args)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Type checking with isinstance on annotation arguments may not be robust.

Consider checking types directly or using typing.get_origin/get_args, as annotation arguments may not always be instances.

Suggested change
def _is_streamable(cls, annotation: Any, args: list[Any]) -> bool:
return any(isinstance(arg, StrawberryStreamable) for arg in args)
def _is_streamable(cls, annotation: Any, args: list[Any]) -> bool:
from typing import get_origin, get_args
def is_streamable_type(arg):
# Check if the argument is the StrawberryStreamable type or a generic of it
origin = get_origin(arg) or arg
return origin is StrawberryStreamable
return any(is_streamable_type(arg) for arg in args)

Comment on lines +534 to +535
if isinstance(result, GraphQLIncrementalExecutionResults):
return result
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): Returning the raw incremental execution result may break expected interfaces.

Returning GraphQLIncrementalExecutionResults instead of ExecutionResult may cause runtime errors if consumers expect the standard interface. Please ensure the returned type matches the expected interface.

import pytest
from inline_snapshot import snapshot

from tests.conftest import skip_if_gql_32
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (code-quality): Don't import test modules. (dont-import-test-modules)

ExplanationDon't import test modules.

Tests should be self-contained and don't depend on each other.

If a helper function is used by multiple tests,
define it in a helper module,
instead of importing one test from the other.

from inline_snapshot import snapshot

from tests.conftest import skip_if_gql_32
from tests.http.clients.base import HttpClient
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (code-quality): Don't import test modules. (dont-import-test-modules)

ExplanationDon't import test modules.

Tests should be self-contained and don't depend on each other.

If a helper function is used by multiple tests,
define it in a helper module,
instead of importing one test from the other.


import strawberry
from strawberry.schema.config import StrawberryConfig
from tests.conftest import skip_if_gql_32
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (code-quality): Don't import test modules. (dont-import-test-modules)

ExplanationDon't import test modules.

Tests should be self-contained and don't depend on each other.

If a helper function is used by multiple tests,
define it in a helper module,
instead of importing one test from the other.

"\n",
"\r\n",
"Content-Type: application/json; charset=utf-8\r\n",
"Content-Length: " + str(len(encoded_data)) + "\r\n",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (code-quality): We've found these issues:

Suggested change
"Content-Length: " + str(len(encoded_data)) + "\r\n",
f"Content-Length: {len(encoded_data)}" + "\r\n",

@@ -561,9 +561,9 @@ def print_schema_definition(
def print_directive(
directive: GraphQLDirective, *, schema: BaseSchema
) -> Optional[str]:
strawberry_directive = directive.extensions["strawberry-definition"]
strawberry_directive = directive.extensions.get("strawberry-definition")

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (code-quality): We've found these issues:

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Summary

This PR implements experimental support for GraphQL incremental execution, specifically adding the @defer and @stream directives to Strawberry. These features allow for more efficient data loading patterns by enabling parts of a GraphQL response to be deferred or streamed, improving perceived performance for complex queries.

Key changes include:

  1. Adding a new enable_experimental_incremental_execution flag to StrawberryConfig
  2. Implementing support for @defer and @stream directives in the schema printer
  3. Adding a new Streamable type for handling async generators
  4. Creating comprehensive end-to-end tests using both Apollo and Relay clients
  5. Setting up a new CI workflow for e2e testing with Playwright

The implementation is thoughtfully structured as an opt-in experimental feature, requiring explicit configuration to enable. The e2e test suite is particularly thorough, testing both immediate and deferred data loading scenarios across multiple GraphQL clients.

Confidence score: 3/5

  1. The PR is safe to merge but requires careful monitoring in production due to its experimental nature
  2. While the implementation is solid and well-tested, the experimental status of the features and reliance on graphql-core>=3.3.0a9 introduces some risk
  3. Key files needing attention:
    • strawberry/schema/schema.py: Complex changes to core execution logic
    • strawberry/http/init.py: Critical changes to response handling
    • tests/http/incremental/: New test infrastructure needs verification

49 files reviewed, 12 comments
Edit PR Review Bot Settings | Greptile

Comment on lines 147 to 148
if self._is_streamable(evaled_type, args):
return self.create_list(list[evaled_type])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: streamable types are converted to lists implicitly - ensure all consuming code expects list types for streamable fields

.alexrc Outdated
Comment on lines 16 to 17
"failed"
"crash"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax: Missing comma after 'failed', which makes this invalid JSON

Suggested change
"failed"
"crash"
"failed",
"crash"

import { clsx, type ClassValue } from "clsx"
import { twMerge } from "tailwind-merge"

export function cn(...inputs: ClassValue[]) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: consider adding a brief JSDoc comment explaining the purpose of the cn function and its parameters

return Hero(id=strawberry.ID("1"), name="Thiago Bellini")

@strawberry.field
async def streambable_field(self) -> strawberry.Streamable[str]:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax: Typo in field name: 'streambable_field' should be 'streamable_field'

Suggested change
async def streambable_field(self) -> strawberry.Streamable[str]:
async def streamable_field(self) -> strawberry.Streamable[str]:

return (
<div className="flex flex-col gap-4">
<h1 className="text-2xl font-bold">Apollo Tests</h1>
<div className=" gap-4">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax: Empty space in className string serves no purpose

Suggested change
<div className=" gap-4">
<div className="gap-4">

Comment on lines +31 to +32
for comment in fetch_comments():
yield comment
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: The example code references undefined function fetch_comments()

Suggested change
for comment in fetch_comments():
yield comment
# Example: fetch comments from database or API
for comment in [
Comment(id="1", content="Great article!"),
Comment(id="2", content="Thanks for sharing")
]:
yield comment

Comment on lines +624 to +630
if schema.config.enable_experimental_incremental_execution:
directives.append(
"directive @defer(if: Boolean, label: String) on FRAGMENT_SPREAD | INLINE_FRAGMENT"
)
directives.append(
"directive @stream(if: Boolean, label: String, initialCount: Int = 0) on FIELD"
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider placing directive definitions in a constant or configuration.


```python
import strawberry
from dataclasses import dataclass
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: dataclass import is unused in the example code


async def test_basic_stream(http_client: HttpClient):
response = await http_client.query(
method="get",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: test_basic_stream is only testing GET method, unlike test_basic_defer which tests both GET and POST. Consider parameterizing this test as well for consistency

method="get",
query="""
query Stream {
streambableField @stream
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax: Typo in field name: 'streambableField' should be 'streamableField'

Suggested change
streambableField @stream
streamableField @stream

@patrick91 patrick91 changed the title Defer Add support for @defer and @stream Jul 16, 2025
Copy link
Member

@bellini666 bellini666 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some suggestions, leaving this already approved 😊

cd e2e
poetry run strawberry server app:schema --port 8000 &
echo $! > server.pid
sleep 5 # Wait for server to start
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe:

    max_attempts=30
    attempt=0
    echo "Waiting for server to start..."
    while [ $attempt -lt $max_attempts ]; do
      if curl -s http://localhost:8000/graphql -o /dev/null; then
        echo "Server is up and running!"
        break
      fi
      attempt=$((attempt+1))
      echo "Attempt $attempt/$max_attempts: Server not ready yet, waiting..."
      sleep 1
    done
    
    if [ $attempt -eq $max_attempts ]; then
      echo "Server failed to start after $max_attempts seconds"
      exit 1
    fi

So we can remove the following check?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you're better than claude :D

The response will be delivered incrementally:

```json
# Initial payload
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this needs to be jsonc or json5 to be able to have comments and render properly? Then the comments might need to be written with // instead of #

if isinstance(result, GraphQLIncrementalExecutionResults):

async def stream() -> AsyncGenerator[str, None]:
yield "---"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the suggestion from sourcery of using a unique boundary instead of -. Wdyt?

Comment on lines +272 to +276
if self.config.enable_experimental_incremental_execution:
directives = tuple(directives) + tuple(incremental_execution_directives)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two raise RuntimeError below, which check that. Maybe it should be here during __init__ instead, and in there just assert?

Comment on lines +550 to +557
# TODO: maybe here use the first result from incremental execution if it exists
if isinstance(result, GraphQLExecutionResult) and result.errors:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I remember it correctly, there was one incremental execution type that did not have errors in it (not totally sure, it's been a while 😅)

If the idea is just to limit to results that have errors, maybe hasattr(result, "errors") would be easier?

@@ -130,6 +138,16 @@ def set_header(self, info: strawberry.Info, name: str) -> str:

return name

@strawberry.field
def character(self) -> Hero:
return Hero(id=strawberry.ID("1"), name="Thiago Bellini")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦸‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants