refactor(streaming): resolve-then-split strategy for @defer#9
Conversation
…lit strategy Replace complex sub-blueprint re-resolution with a simpler approach: 1. Run standard resolution (resolves everything including @defer fields) 2. Collect @defer metadata (label, field_names, parent_path) via AST walk 3. Store in streaming context for the transport layer to split The transport layer (absinthe_plug) handles splitting the final result into initial/incremental SSE payloads after the Result phase runs. Fixes: CaseClauseError in Projector.do_collect (nil selections), KeyError in build_sub_blueprint (:path not in Execution struct), and broken path tracking across sibling nodes in prewalk.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3f68ce1. Configure here.
| # Create a minimal blueprint with just the node to resolve | ||
| %{blueprint | execution: execution, operations: [wrap_in_operation(node, blueprint)]} | ||
| [ | ||
| %{label: config[:label], field_names: field_names, parent_path: parent_path} |
There was a problem hiding this comment.
Defer info uses parent_path but consumers expect path
High Severity
The defer info maps use the key parent_path but downstream consumers in Absinthe.Incremental.Response and Absinthe.Incremental.Dataloader access .path on these entries. Since deferred_fragments is set to defer_info for backwards compatibility, calling Response.build_initial/1 (which is invoked by both transport.ex and delivery.ex) will raise a KeyError when build_pending_list or filter_deferred_fragments tries to access fragment.path.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 3f68ce1. Configure here.
| defp extract_field_names(%{selections: sels}) when is_list(sels) do | ||
| Enum.flat_map(sels, &extract_field_names/1) | ||
| end | ||
| defp extract_field_names(_), do: [] |
There was a problem hiding this comment.
extract_field_names silently drops Fragment.Spread fields
Medium Severity
extract_field_names doesn't handle Fragment.Spread nodes. Since Spread structs have no selections key, they fall through to the catch-all clause returning []. When a @defer fragment contains a spread (e.g., ... @defer { ...MyFragment }), the fields contributed by that named fragment are silently omitted from field_names, causing the transport layer to potentially miss fields when splitting the response.
Reviewed by Cursor Bugbot for commit 3f68ce1. Configure here.
Formats 16 files flagged by CI's `mix format --check-formatted` step on the Elixir 1.18 / OTP 28 matrix entry. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Dialyzer (14 → 0): - dataloader.ex: replace non-existent Resolution.resolver/0 with inline function type - error_handler.ex: drop unused default nil on format_exception/2 - response.ex: broaden @type definitions and @specs for incremental response shapes to match what functions actually return - transport.ex: use any() for conn_or_socket (Plug/Phoenix are optional deps) - coordinate.ex: drop unreachable resolve_parsed clauses and unused get_enum_value/2, get_input_field/2 - incremental_directives.ex: store :defer/:stream config on node.flags via update_in (matches auto_defer_stream pattern), rather than calling Blueprint.put_flag/3 which demands a module as third arg Compile warnings: - remove unused aliases (error_handler, complexity_test) - underscore-prefix unused variables (dataloader, response, complexity, auto_defer_stream) - group update_stats/2 clauses together (resource_manager) Verified: mix format, mix compile (no warnings from our code), mix dialyzer (0 errors, exit 0), mix test (1696 tests, 0 failures) all pass on Elixir 1.18 / OTP 28. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two clauses regressed line-length during the dialyzer fix commit. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2 participants
Summary
Internal branch copy of juscyllan's fix (originally PR #8) pushed to this repo so we can track it on a gigsmart-owned branch and open a matching upstream PR to absinthe-graphql/absinthe.
StreamingResolutionto resolve everything in one pass (standard resolution), then store@defermetadata in the streaming contextReview feedback addressed
Fragment.Spreadnodes (not justFragment.Inline) when walking for@defer, including traversal of named fragmentsdeferred_fragments,streamed_fields,deferred_tasks,stream_tasks) on__streaming__context so downstreamAbsinthe.Incremental.*consumers don't raiseKeyErrorRelated
Depends on
Note
Medium Risk
Changes the incremental-delivery execution strategy by removing deferred re-resolution and always running full
Resolutionwhen@deferis present, which can affect response shape/timing and load. Risk is moderated by being scoped to incremental delivery and mainly refactoring into metadata collection plus context flags.Overview
Simplifies
StreamingResolutionto only support@deferby running standardResolutiononce (resolving all fields) and deferring payload splitting to the transport layer.Replaces the prior streaming implementation (skipping nodes, building sub-blueprints, and creating deferred/stream tasks) with a lightweight walk that collects
@defermetadata (includingFragment.Spreadand nested inline fragments) and stores it underexecution.context[:__streaming__], while settingexecution.incremental_delivery = trueand keeping backwards-compatibleAbsinthe.Incremental.*keys populated.Reviewed by Cursor Bugbot for commit 3f68ce1. Bugbot is set up for automated code reviews on this repo. Configure here.