Draft
Conversation
Contributor
WalkthroughAdded comprehensive GraphQL Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
router-tests/testdata/queries_defer/employeeDetails_defer_01_single_defer_reconstructed.json (1)
1-1724: Consider reducing cross-fixture duplication for reconstructed outputs.This full reconstructed payload is effectively duplicated across multiple defer scenario files in this PR. Consider loading a shared canonical expected JSON (or generating these fixtures from one source) to reduce drift and fixture-maintenance overhead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@router-tests/testdata/queries_defer/employeeDetails_defer_01_single_defer_reconstructed.json` around lines 1 - 1724, The reconstructed "employees" payload is duplicated across multiple defer test fixtures; create a single shared canonical fixture (e.g., canonical_employeeDetails_reconstructed.json) containing the full reconstructed payload and update each defer test that currently uses the large payload (including the fixture named employeeDetails_defer_01_single_defer_reconstructed.json and other similar files) to load/require that shared JSON instead of inlining the data; ensure test assertions still reference the "data.employees" root structure so behavior remains unchanged.router-tests/defer_test.go (1)
78-83: Gate the engine debug prints behind an opt-in switch.Lines 79-82 enable very verbose planner output for every fixture variation. That will bloat CI logs and slow this suite down even when the test is healthy.
Suggested fix
ModifyEngineExecutionConfiguration: func(cfg *config.EngineExecutionConfiguration) { - cfg.Debug.PrintIntermediateQueryPlans = true - cfg.Debug.PrintPlanningPaths = true - // cfg.Debug.PrintNodeSuggestions = true - cfg.Debug.PrintOperationTransformations = true + if os.Getenv("WG_DEBUG_DEFER_TESTS") == "1" { + cfg.Debug.PrintIntermediateQueryPlans = true + cfg.Debug.PrintPlanningPaths = true + cfg.Debug.PrintOperationTransformations = true + } },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@router-tests/defer_test.go` around lines 78 - 83, The test unconditionally enables verbose planner debug flags inside ModifyEngineExecutionConfiguration (setting cfg.Debug.PrintIntermediateQueryPlans, cfg.Debug.PrintPlanningPaths, cfg.Debug.PrintOperationTransformations), which bloats CI; change this to gate those debug flags behind an opt-in flag (e.g., an environment variable or a test flag) so they remain off by default: read the opt-in (ENV_TEST_VERBOSE_PLANNER or t.Run flag) at test startup, and only set cfg.Debug.PrintIntermediateQueryPlans, cfg.Debug.PrintPlanningPaths, and cfg.Debug.PrintOperationTransformations to true when that opt-in is enabled, leaving them false otherwise.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@router-tests/defer_test.go`:
- Around line 195-227: The loop currently does an early continue when patchData
is nil which skips collecting incremental "errors"; instead, change the loop so
that the error-collection logic always runs whether or not patchData exists:
only skip the merge step when patchData == nil, but always execute the block
that reads item.Get("errors") and merges into result using existing :=
result.Get("errors"), appendArrayValues(existing, patchErrors) and
result.Set(nil, "errors", merged). Keep references to mergeAtPath,
appendArrayValues, patchData, patchErrors, pathKeys and result when modifying
the control flow so that path-building and mergeAtPath are conditional but error
aggregation always happens.
In `@router-tests/go.mod`:
- Line 29: Update the module dependency on go.opentelemetry.io/otel/sdk: in the
go.mod require block change the version for go.opentelemetry.io/otel/sdk to
v1.40.0 or later, and remove or update any replace directive that forces an
older version (the existing replace of go.opentelemetry.io/otel/sdk v1.28.0) so
the require pin is respected; ensure there are no conflicting replace entries
and run go mod tidy to lock the updated version.
In `@router-tests/testdata/queries_defer/README.md`:
- Around line 8-10: The fenced code block containing
"{source}_{number}_{description}.graphql" is missing a language tag (MD040);
update the triple-backtick fence to include a language tag (use "text") so it
becomes ```text before the content and ``` after; modify the README.md fenced
block around the "{source}_{number}_{description}.graphql" snippet to add the
"text" tag.
- Around line 48-50: Update the regeneration command in README.md so the -run
regex matches the actual test name: change the `go test ... -run
TestDeferTestdataQueries` invocation to use `-run TestDeferTestDataQueries` (the
test defined in router-tests/defer_test.go as TestDeferTestDataQueries),
ensuring the documented command will run the new test.
---
Nitpick comments:
In `@router-tests/defer_test.go`:
- Around line 78-83: The test unconditionally enables verbose planner debug
flags inside ModifyEngineExecutionConfiguration (setting
cfg.Debug.PrintIntermediateQueryPlans, cfg.Debug.PrintPlanningPaths,
cfg.Debug.PrintOperationTransformations), which bloats CI; change this to gate
those debug flags behind an opt-in flag (e.g., an environment variable or a test
flag) so they remain off by default: read the opt-in (ENV_TEST_VERBOSE_PLANNER
or t.Run flag) at test startup, and only set
cfg.Debug.PrintIntermediateQueryPlans, cfg.Debug.PrintPlanningPaths, and
cfg.Debug.PrintOperationTransformations to true when that opt-in is enabled,
leaving them false otherwise.
In
`@router-tests/testdata/queries_defer/employeeDetails_defer_01_single_defer_reconstructed.json`:
- Around line 1-1724: The reconstructed "employees" payload is duplicated across
multiple defer test fixtures; create a single shared canonical fixture (e.g.,
canonical_employeeDetails_reconstructed.json) containing the full reconstructed
payload and update each defer test that currently uses the large payload
(including the fixture named
employeeDetails_defer_01_single_defer_reconstructed.json and other similar
files) to load/require that shared JSON instead of inlining the data; ensure
test assertions still reference the "data.employees" root structure so behavior
remains unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7d43ed19-2030-4a3c-82cf-af3d2aa0160d
⛔ Files ignored due to path filters (1)
router-tests/go.sumis excluded by!**/*.sum
📒 Files selected for processing (299)
router-tests/defer_test.gorouter-tests/go.modrouter-tests/testdata/queries_defer/README.mdrouter-tests/testdata/queries_defer/employeeDetails_defer_01_single_defer.graphqlrouter-tests/testdata/queries_defer/employeeDetails_defer_01_single_defer.txtrouter-tests/testdata/queries_defer/employeeDetails_defer_01_single_defer_reconstructed.jsonrouter-tests/testdata/queries_defer/employeeDetails_defer_02_single_defer_between_regular.graphqlrouter-tests/testdata/queries_defer/employeeDetails_defer_02_single_defer_between_regular.txtrouter-tests/testdata/queries_defer/employeeDetails_defer_02_single_defer_between_regular_reconstructed.jsonrouter-tests/testdata/queries_defer/employeeDetails_defer_03_multiple_fields_deferred.graphqlrouter-tests/testdata/queries_defer/employeeDetails_defer_03_multiple_fields_deferred.txtrouter-tests/testdata/queries_defer/employeeDetails_defer_03_multiple_fields_deferred_reconstructed.jsonrouter-tests/testdata/queries_defer/employeeDetails_defer_04_all_fields_deferred.graphqlrouter-tests/testdata/queries_defer/employeeDetails_defer_04_all_fields_deferred.txtrouter-tests/testdata/queries_defer/employeeDetails_defer_04_all_fields_deferred_reconstructed.jsonrouter-tests/testdata/queries_defer/employeeDetails_defer_05_nested_defer.graphqlrouter-tests/testdata/queries_defer/employeeDetails_defer_05_nested_defer.txtrouter-tests/testdata/queries_defer/employeeDetails_defer_05_nested_defer_reconstructed.jsonrouter-tests/testdata/queries_defer/employeeDetails_defer_06_nested_defer_variation.graphqlrouter-tests/testdata/queries_defer/employeeDetails_defer_06_nested_defer_variation.txtrouter-tests/testdata/queries_defer/employeeDetails_defer_06_nested_defer_variation_reconstructed.jsonrouter-tests/testdata/queries_defer/employeeDetails_defer_07_parallel_defers.graphqlrouter-tests/testdata/queries_defer/employeeDetails_defer_07_parallel_defers.txtrouter-tests/testdata/queries_defer/employeeDetails_defer_07_parallel_defers_reconstructed.jsonrouter-tests/testdata/queries_defer/employeeDetails_defer_08_defer_nested_object.graphqlrouter-tests/testdata/queries_defer/employeeDetails_defer_08_defer_nested_object.txtrouter-tests/testdata/queries_defer/employeeDetails_defer_08_defer_nested_object_reconstructed.jsonrouter-tests/testdata/queries_defer/employeeDetails_defer_09_duplicated_field_across_defer.graphqlrouter-tests/testdata/queries_defer/employeeDetails_defer_09_duplicated_field_across_defer.txtrouter-tests/testdata/queries_defer/employeeDetails_defer_09_duplicated_field_across_defer_reconstructed.jsonrouter-tests/testdata/queries_defer/employeeDetails_defer_10_extensive_parallel.graphqlrouter-tests/testdata/queries_defer/employeeDetails_defer_10_extensive_parallel.txtrouter-tests/testdata/queries_defer/employeeDetails_defer_10_extensive_parallel_reconstructed.jsonrouter-tests/testdata/queries_defer/employeeDetails_original.graphqlrouter-tests/testdata/queries_defer/employeeDetails_original.jsonrouter-tests/testdata/queries_defer/employee_defer_01_single_defer.graphqlrouter-tests/testdata/queries_defer/employee_defer_01_single_defer.txtrouter-tests/testdata/queries_defer/employee_defer_01_single_defer_reconstructed.jsonrouter-tests/testdata/queries_defer/employee_defer_02_single_defer_between_regular.graphqlrouter-tests/testdata/queries_defer/employee_defer_02_single_defer_between_regular.txtrouter-tests/testdata/queries_defer/employee_defer_02_single_defer_between_regular_reconstructed.jsonrouter-tests/testdata/queries_defer/employee_defer_03_multiple_fields_deferred.graphqlrouter-tests/testdata/queries_defer/employee_defer_03_multiple_fields_deferred.txtrouter-tests/testdata/queries_defer/employee_defer_03_multiple_fields_deferred_reconstructed.jsonrouter-tests/testdata/queries_defer/employee_defer_04_all_fields_deferred.graphqlrouter-tests/testdata/queries_defer/employee_defer_04_all_fields_deferred.txtrouter-tests/testdata/queries_defer/employee_defer_04_all_fields_deferred_reconstructed.jsonrouter-tests/testdata/queries_defer/employee_defer_05_nested_defer.graphqlrouter-tests/testdata/queries_defer/employee_defer_05_nested_defer.txtrouter-tests/testdata/queries_defer/employee_defer_05_nested_defer_reconstructed.jsonrouter-tests/testdata/queries_defer/employee_defer_06_nested_defer_variation.graphqlrouter-tests/testdata/queries_defer/employee_defer_06_nested_defer_variation.txtrouter-tests/testdata/queries_defer/employee_defer_06_nested_defer_variation_reconstructed.jsonrouter-tests/testdata/queries_defer/employee_defer_07_parallel_defers.graphqlrouter-tests/testdata/queries_defer/employee_defer_07_parallel_defers.txtrouter-tests/testdata/queries_defer/employee_defer_07_parallel_defers_reconstructed.jsonrouter-tests/testdata/queries_defer/employee_defer_08_defer_nested_object.graphqlrouter-tests/testdata/queries_defer/employee_defer_08_defer_nested_object.txtrouter-tests/testdata/queries_defer/employee_defer_08_defer_nested_object_reconstructed.jsonrouter-tests/testdata/queries_defer/employee_defer_09_duplicated_field_across_defer.graphqlrouter-tests/testdata/queries_defer/employee_defer_09_duplicated_field_across_defer.txtrouter-tests/testdata/queries_defer/employee_defer_09_duplicated_field_across_defer_reconstructed.jsonrouter-tests/testdata/queries_defer/employee_defer_10_extensive_parallel.graphqlrouter-tests/testdata/queries_defer/employee_defer_10_extensive_parallel.txtrouter-tests/testdata/queries_defer/employee_defer_10_extensive_parallel_reconstructed.jsonrouter-tests/testdata/queries_defer/employee_original.graphqlrouter-tests/testdata/queries_defer/employee_original.jsonrouter-tests/testdata/queries_defer/employees_defer_01_single_defer.graphqlrouter-tests/testdata/queries_defer/employees_defer_01_single_defer.txtrouter-tests/testdata/queries_defer/employees_defer_01_single_defer_reconstructed.jsonrouter-tests/testdata/queries_defer/employees_defer_02_single_defer_between_regular.graphqlrouter-tests/testdata/queries_defer/employees_defer_02_single_defer_between_regular.txtrouter-tests/testdata/queries_defer/employees_defer_02_single_defer_between_regular_reconstructed.jsonrouter-tests/testdata/queries_defer/employees_defer_03_multiple_fields_deferred.graphqlrouter-tests/testdata/queries_defer/employees_defer_03_multiple_fields_deferred.txtrouter-tests/testdata/queries_defer/employees_defer_03_multiple_fields_deferred_reconstructed.jsonrouter-tests/testdata/queries_defer/employees_defer_04_all_fields_deferred.graphqlrouter-tests/testdata/queries_defer/employees_defer_04_all_fields_deferred.txtrouter-tests/testdata/queries_defer/employees_defer_04_all_fields_deferred_reconstructed.jsonrouter-tests/testdata/queries_defer/employees_defer_05_nested_defer.graphqlrouter-tests/testdata/queries_defer/employees_defer_05_nested_defer.txtrouter-tests/testdata/queries_defer/employees_defer_05_nested_defer_reconstructed.jsonrouter-tests/testdata/queries_defer/employees_defer_06_nested_defer_variation.graphqlrouter-tests/testdata/queries_defer/employees_defer_06_nested_defer_variation.txtrouter-tests/testdata/queries_defer/employees_defer_06_nested_defer_variation_reconstructed.jsonrouter-tests/testdata/queries_defer/employees_defer_07_parallel_defers.graphqlrouter-tests/testdata/queries_defer/employees_defer_07_parallel_defers.txtrouter-tests/testdata/queries_defer/employees_defer_07_parallel_defers_reconstructed.jsonrouter-tests/testdata/queries_defer/employees_defer_08_defer_nested_object.graphqlrouter-tests/testdata/queries_defer/employees_defer_08_defer_nested_object.txtrouter-tests/testdata/queries_defer/employees_defer_08_defer_nested_object_reconstructed.jsonrouter-tests/testdata/queries_defer/employees_defer_09_duplicated_field_across_defer.graphqlrouter-tests/testdata/queries_defer/employees_defer_09_duplicated_field_across_defer.txtrouter-tests/testdata/queries_defer/employees_defer_09_duplicated_field_across_defer_reconstructed.jsonrouter-tests/testdata/queries_defer/employees_defer_10_extensive_parallel.graphqlrouter-tests/testdata/queries_defer/employees_defer_10_extensive_parallel.txtrouter-tests/testdata/queries_defer/employees_defer_10_extensive_parallel_reconstructed.jsonrouter-tests/testdata/queries_defer/employees_defer_11_fragment_around_and_inside.graphqlrouter-tests/testdata/queries_defer/employees_defer_11_fragment_around_and_inside.txtrouter-tests/testdata/queries_defer/employees_defer_11_fragment_around_and_inside_reconstructed.jsonrouter-tests/testdata/queries_defer/employees_defer_12_fragment_body_defer.graphqlrouter-tests/testdata/queries_defer/employees_defer_12_fragment_body_defer.txtrouter-tests/testdata/queries_defer/employees_defer_12_fragment_body_defer_reconstructed.jsonrouter-tests/testdata/queries_defer/employees_defer_13_fragment_spread_defer.graphqlrouter-tests/testdata/queries_defer/employees_defer_13_fragment_spread_defer.txtrouter-tests/testdata/queries_defer/employees_defer_13_fragment_spread_defer_reconstructed.jsonrouter-tests/testdata/queries_defer/employees_original.graphqlrouter-tests/testdata/queries_defer/employees_original.jsonrouter-tests/testdata/queries_defer/findEmployeesNoCriteria_defer_01_single_defer.graphqlrouter-tests/testdata/queries_defer/findEmployeesNoCriteria_defer_01_single_defer.txtrouter-tests/testdata/queries_defer/findEmployeesNoCriteria_defer_01_single_defer_reconstructed.jsonrouter-tests/testdata/queries_defer/findEmployeesNoCriteria_defer_02_single_defer_between_regular.graphqlrouter-tests/testdata/queries_defer/findEmployeesNoCriteria_defer_02_single_defer_between_regular.txtrouter-tests/testdata/queries_defer/findEmployeesNoCriteria_defer_02_single_defer_between_regular_reconstructed.jsonrouter-tests/testdata/queries_defer/findEmployeesNoCriteria_defer_03_multiple_fields_deferred.graphqlrouter-tests/testdata/queries_defer/findEmployeesNoCriteria_defer_03_multiple_fields_deferred.txtrouter-tests/testdata/queries_defer/findEmployeesNoCriteria_defer_03_multiple_fields_deferred_reconstructed.jsonrouter-tests/testdata/queries_defer/findEmployeesNoCriteria_defer_04_all_fields_deferred.graphqlrouter-tests/testdata/queries_defer/findEmployeesNoCriteria_defer_04_all_fields_deferred.txtrouter-tests/testdata/queries_defer/findEmployeesNoCriteria_defer_04_all_fields_deferred_reconstructed.jsonrouter-tests/testdata/queries_defer/findEmployeesNoCriteria_defer_05_nested_defer.graphqlrouter-tests/testdata/queries_defer/findEmployeesNoCriteria_defer_05_nested_defer.txtrouter-tests/testdata/queries_defer/findEmployeesNoCriteria_defer_05_nested_defer_reconstructed.jsonrouter-tests/testdata/queries_defer/findEmployeesNoCriteria_defer_06_nested_defer_variation.graphqlrouter-tests/testdata/queries_defer/findEmployeesNoCriteria_defer_06_nested_defer_variation.txtrouter-tests/testdata/queries_defer/findEmployeesNoCriteria_defer_06_nested_defer_variation_reconstructed.jsonrouter-tests/testdata/queries_defer/findEmployeesNoCriteria_defer_07_parallel_defers.graphqlrouter-tests/testdata/queries_defer/findEmployeesNoCriteria_defer_07_parallel_defers.txtrouter-tests/testdata/queries_defer/findEmployeesNoCriteria_defer_07_parallel_defers_reconstructed.jsonrouter-tests/testdata/queries_defer/findEmployeesNoCriteria_defer_08_defer_nested_object.graphqlrouter-tests/testdata/queries_defer/findEmployeesNoCriteria_defer_08_defer_nested_object.txtrouter-tests/testdata/queries_defer/findEmployeesNoCriteria_defer_08_defer_nested_object_reconstructed.jsonrouter-tests/testdata/queries_defer/findEmployeesNoCriteria_defer_09_duplicated_field_across_defer.graphqlrouter-tests/testdata/queries_defer/findEmployeesNoCriteria_defer_09_duplicated_field_across_defer.txtrouter-tests/testdata/queries_defer/findEmployeesNoCriteria_defer_09_duplicated_field_across_defer_reconstructed.jsonrouter-tests/testdata/queries_defer/findEmployeesNoCriteria_defer_10_extensive_parallel.graphqlrouter-tests/testdata/queries_defer/findEmployeesNoCriteria_defer_10_extensive_parallel.txtrouter-tests/testdata/queries_defer/findEmployeesNoCriteria_defer_10_extensive_parallel_reconstructed.jsonrouter-tests/testdata/queries_defer/findEmployeesNoCriteria_original.graphqlrouter-tests/testdata/queries_defer/findEmployeesNoCriteria_original.jsonrouter-tests/testdata/queries_defer/findEmployees_defer_01_single_defer.graphqlrouter-tests/testdata/queries_defer/findEmployees_defer_01_single_defer.txtrouter-tests/testdata/queries_defer/findEmployees_defer_01_single_defer_reconstructed.jsonrouter-tests/testdata/queries_defer/findEmployees_defer_02_single_defer_between_regular.graphqlrouter-tests/testdata/queries_defer/findEmployees_defer_02_single_defer_between_regular.txtrouter-tests/testdata/queries_defer/findEmployees_defer_02_single_defer_between_regular_reconstructed.jsonrouter-tests/testdata/queries_defer/findEmployees_defer_03_multiple_fields_deferred.graphqlrouter-tests/testdata/queries_defer/findEmployees_defer_03_multiple_fields_deferred.txtrouter-tests/testdata/queries_defer/findEmployees_defer_03_multiple_fields_deferred_reconstructed.jsonrouter-tests/testdata/queries_defer/findEmployees_defer_04_all_fields_deferred.graphqlrouter-tests/testdata/queries_defer/findEmployees_defer_04_all_fields_deferred.txtrouter-tests/testdata/queries_defer/findEmployees_defer_04_all_fields_deferred_reconstructed.jsonrouter-tests/testdata/queries_defer/findEmployees_defer_05_nested_defer.graphqlrouter-tests/testdata/queries_defer/findEmployees_defer_05_nested_defer.txtrouter-tests/testdata/queries_defer/findEmployees_defer_05_nested_defer_reconstructed.jsonrouter-tests/testdata/queries_defer/findEmployees_defer_06_nested_defer_variation.graphqlrouter-tests/testdata/queries_defer/findEmployees_defer_06_nested_defer_variation.txtrouter-tests/testdata/queries_defer/findEmployees_defer_06_nested_defer_variation_reconstructed.jsonrouter-tests/testdata/queries_defer/findEmployees_defer_07_parallel_defers.graphqlrouter-tests/testdata/queries_defer/findEmployees_defer_07_parallel_defers.txtrouter-tests/testdata/queries_defer/findEmployees_defer_07_parallel_defers_reconstructed.jsonrouter-tests/testdata/queries_defer/findEmployees_defer_08_defer_nested_object.graphqlrouter-tests/testdata/queries_defer/findEmployees_defer_08_defer_nested_object.txtrouter-tests/testdata/queries_defer/findEmployees_defer_08_defer_nested_object_reconstructed.jsonrouter-tests/testdata/queries_defer/findEmployees_defer_09_duplicated_field_across_defer.graphqlrouter-tests/testdata/queries_defer/findEmployees_defer_09_duplicated_field_across_defer.txtrouter-tests/testdata/queries_defer/findEmployees_defer_09_duplicated_field_across_defer_reconstructed.jsonrouter-tests/testdata/queries_defer/findEmployees_defer_10_extensive_parallel.graphqlrouter-tests/testdata/queries_defer/findEmployees_defer_10_extensive_parallel.txtrouter-tests/testdata/queries_defer/findEmployees_defer_10_extensive_parallel_reconstructed.jsonrouter-tests/testdata/queries_defer/findEmployees_original.graphqlrouter-tests/testdata/queries_defer/findEmployees_original.jsonrouter-tests/testdata/queries_defer/full_defer_01_single_defer.graphqlrouter-tests/testdata/queries_defer/full_defer_01_single_defer.txtrouter-tests/testdata/queries_defer/full_defer_01_single_defer_reconstructed.jsonrouter-tests/testdata/queries_defer/full_defer_02_single_defer_between_regular.graphqlrouter-tests/testdata/queries_defer/full_defer_02_single_defer_between_regular.txtrouter-tests/testdata/queries_defer/full_defer_02_single_defer_between_regular_reconstructed.jsonrouter-tests/testdata/queries_defer/full_defer_03_multiple_fields_deferred.graphqlrouter-tests/testdata/queries_defer/full_defer_03_multiple_fields_deferred.txtrouter-tests/testdata/queries_defer/full_defer_03_multiple_fields_deferred_reconstructed.jsonrouter-tests/testdata/queries_defer/full_defer_04_all_fields_deferred.graphqlrouter-tests/testdata/queries_defer/full_defer_04_all_fields_deferred.txtrouter-tests/testdata/queries_defer/full_defer_04_all_fields_deferred_reconstructed.jsonrouter-tests/testdata/queries_defer/full_defer_05_nested_defer.graphqlrouter-tests/testdata/queries_defer/full_defer_05_nested_defer.txtrouter-tests/testdata/queries_defer/full_defer_05_nested_defer_reconstructed.jsonrouter-tests/testdata/queries_defer/full_defer_06_nested_defer_variation.graphqlrouter-tests/testdata/queries_defer/full_defer_06_nested_defer_variation.txtrouter-tests/testdata/queries_defer/full_defer_06_nested_defer_variation_reconstructed.jsonrouter-tests/testdata/queries_defer/full_defer_07_parallel_defers.graphqlrouter-tests/testdata/queries_defer/full_defer_07_parallel_defers.txtrouter-tests/testdata/queries_defer/full_defer_07_parallel_defers_reconstructed.jsonrouter-tests/testdata/queries_defer/full_defer_08_defer_nested_object.graphqlrouter-tests/testdata/queries_defer/full_defer_08_defer_nested_object.txtrouter-tests/testdata/queries_defer/full_defer_08_defer_nested_object_reconstructed.jsonrouter-tests/testdata/queries_defer/full_defer_09_duplicated_field_across_defer.graphqlrouter-tests/testdata/queries_defer/full_defer_09_duplicated_field_across_defer.txtrouter-tests/testdata/queries_defer/full_defer_09_duplicated_field_across_defer_reconstructed.jsonrouter-tests/testdata/queries_defer/full_defer_10_extensive_parallel.graphqlrouter-tests/testdata/queries_defer/full_defer_10_extensive_parallel.txtrouter-tests/testdata/queries_defer/full_defer_10_extensive_parallel_reconstructed.jsonrouter-tests/testdata/queries_defer/full_defer_11_fragment_around_and_inside.graphqlrouter-tests/testdata/queries_defer/full_defer_11_fragment_around_and_inside.txtrouter-tests/testdata/queries_defer/full_defer_11_fragment_around_and_inside_reconstructed.jsonrouter-tests/testdata/queries_defer/full_defer_12_fragment_body_defer.graphqlrouter-tests/testdata/queries_defer/full_defer_12_fragment_body_defer.txtrouter-tests/testdata/queries_defer/full_defer_12_fragment_body_defer_reconstructed.jsonrouter-tests/testdata/queries_defer/full_defer_13_fragment_spread_defer.graphqlrouter-tests/testdata/queries_defer/full_defer_13_fragment_spread_defer.txtrouter-tests/testdata/queries_defer/full_defer_13_fragment_spread_defer_reconstructed.jsonrouter-tests/testdata/queries_defer/full_original.graphqlrouter-tests/testdata/queries_defer/full_original.jsonrouter-tests/testdata/queries_defer/products_defer_01_single_defer.graphqlrouter-tests/testdata/queries_defer/products_defer_01_single_defer.txtrouter-tests/testdata/queries_defer/products_defer_01_single_defer_reconstructed.jsonrouter-tests/testdata/queries_defer/products_defer_02_single_defer_between_regular.graphqlrouter-tests/testdata/queries_defer/products_defer_02_single_defer_between_regular.txtrouter-tests/testdata/queries_defer/products_defer_02_single_defer_between_regular_reconstructed.jsonrouter-tests/testdata/queries_defer/products_defer_03_multiple_fields_deferred.graphqlrouter-tests/testdata/queries_defer/products_defer_03_multiple_fields_deferred.txtrouter-tests/testdata/queries_defer/products_defer_03_multiple_fields_deferred_reconstructed.jsonrouter-tests/testdata/queries_defer/products_defer_04_all_fields_deferred.graphqlrouter-tests/testdata/queries_defer/products_defer_04_all_fields_deferred.txtrouter-tests/testdata/queries_defer/products_defer_04_all_fields_deferred_reconstructed.jsonrouter-tests/testdata/queries_defer/products_defer_05_nested_defer.graphqlrouter-tests/testdata/queries_defer/products_defer_05_nested_defer.txtrouter-tests/testdata/queries_defer/products_defer_05_nested_defer_reconstructed.jsonrouter-tests/testdata/queries_defer/products_defer_06_nested_defer_variation.graphqlrouter-tests/testdata/queries_defer/products_defer_06_nested_defer_variation.txtrouter-tests/testdata/queries_defer/products_defer_06_nested_defer_variation_reconstructed.jsonrouter-tests/testdata/queries_defer/products_defer_07_parallel_defers.graphqlrouter-tests/testdata/queries_defer/products_defer_07_parallel_defers.txtrouter-tests/testdata/queries_defer/products_defer_07_parallel_defers_reconstructed.jsonrouter-tests/testdata/queries_defer/products_defer_08_defer_nested_object.graphqlrouter-tests/testdata/queries_defer/products_defer_08_defer_nested_object.txtrouter-tests/testdata/queries_defer/products_defer_08_defer_nested_object_reconstructed.jsonrouter-tests/testdata/queries_defer/products_defer_09_duplicated_field_across_defer.graphqlrouter-tests/testdata/queries_defer/products_defer_09_duplicated_field_across_defer.txtrouter-tests/testdata/queries_defer/products_defer_09_duplicated_field_across_defer_reconstructed.jsonrouter-tests/testdata/queries_defer/products_defer_10_extensive_parallel.graphqlrouter-tests/testdata/queries_defer/products_defer_10_extensive_parallel.txtrouter-tests/testdata/queries_defer/products_defer_10_extensive_parallel_reconstructed.jsonrouter-tests/testdata/queries_defer/products_defer_11_fragment_around_and_inside.graphqlrouter-tests/testdata/queries_defer/products_defer_11_fragment_around_and_inside.txtrouter-tests/testdata/queries_defer/products_defer_11_fragment_around_and_inside_reconstructed.jsonrouter-tests/testdata/queries_defer/products_defer_12_fragment_body_defer.graphqlrouter-tests/testdata/queries_defer/products_defer_12_fragment_body_defer.graphql.todorouter-tests/testdata/queries_defer/products_defer_12_fragment_body_defer.txtrouter-tests/testdata/queries_defer/products_defer_12_fragment_body_defer_reconstructed.jsonrouter-tests/testdata/queries_defer/products_defer_13_fragment_spread_defer.graphqlrouter-tests/testdata/queries_defer/products_defer_13_fragment_spread_defer.txtrouter-tests/testdata/queries_defer/products_defer_13_fragment_spread_defer_reconstructed.jsonrouter-tests/testdata/queries_defer/products_original.graphqlrouter-tests/testdata/queries_defer/products_original.jsonrouter-tests/testdata/queries_defer/requires_different_depth_defer_01_single_defer.graphqlrouter-tests/testdata/queries_defer/requires_different_depth_defer_01_single_defer.txtrouter-tests/testdata/queries_defer/requires_different_depth_defer_01_single_defer_reconstructed.jsonrouter-tests/testdata/queries_defer/requires_different_depth_defer_02_single_defer_between_regular.graphqlrouter-tests/testdata/queries_defer/requires_different_depth_defer_02_single_defer_between_regular.txtrouter-tests/testdata/queries_defer/requires_different_depth_defer_02_single_defer_between_regular_reconstructed.jsonrouter-tests/testdata/queries_defer/requires_different_depth_defer_03_multiple_fields_deferred.graphqlrouter-tests/testdata/queries_defer/requires_different_depth_defer_03_multiple_fields_deferred.txtrouter-tests/testdata/queries_defer/requires_different_depth_defer_03_multiple_fields_deferred_reconstructed.jsonrouter-tests/testdata/queries_defer/requires_different_depth_defer_04_all_fields_deferred.graphqlrouter-tests/testdata/queries_defer/requires_different_depth_defer_04_all_fields_deferred.txtrouter-tests/testdata/queries_defer/requires_different_depth_defer_04_all_fields_deferred_reconstructed.jsonrouter-tests/testdata/queries_defer/requires_different_depth_defer_05_nested_defer.graphqlrouter-tests/testdata/queries_defer/requires_different_depth_defer_05_nested_defer.txtrouter-tests/testdata/queries_defer/requires_different_depth_defer_05_nested_defer_reconstructed.jsonrouter-tests/testdata/queries_defer/requires_different_depth_defer_06_nested_defer_variation.graphqlrouter-tests/testdata/queries_defer/requires_different_depth_defer_06_nested_defer_variation.txtrouter-tests/testdata/queries_defer/requires_different_depth_defer_06_nested_defer_variation_reconstructed.jsonrouter-tests/testdata/queries_defer/requires_different_depth_defer_07_parallel_defers.graphqlrouter-tests/testdata/queries_defer/requires_different_depth_defer_07_parallel_defers.txtrouter-tests/testdata/queries_defer/requires_different_depth_defer_07_parallel_defers_reconstructed.jsonrouter-tests/testdata/queries_defer/requires_different_depth_defer_08_defer_nested_object.graphqlrouter-tests/testdata/queries_defer/requires_different_depth_defer_08_defer_nested_object.txtrouter-tests/testdata/queries_defer/requires_different_depth_defer_08_defer_nested_object_reconstructed.jsonrouter-tests/testdata/queries_defer/requires_different_depth_defer_09_duplicated_field_across_defer.graphqlrouter-tests/testdata/queries_defer/requires_different_depth_defer_09_duplicated_field_across_defer.txtrouter-tests/testdata/queries_defer/requires_different_depth_defer_09_duplicated_field_across_defer_reconstructed.jsonrouter-tests/testdata/queries_defer/requires_different_depth_defer_10_extensive_parallel.graphqlrouter-tests/testdata/queries_defer/requires_different_depth_defer_10_extensive_parallel.txtrouter-tests/testdata/queries_defer/requires_different_depth_defer_10_extensive_parallel_reconstructed.jsonrouter-tests/testdata/queries_defer/requires_different_depth_defer_11_fragment_around_and_inside.graphqlrouter-tests/testdata/queries_defer/requires_different_depth_defer_11_fragment_around_and_inside.txtrouter-tests/testdata/queries_defer/requires_different_depth_defer_11_fragment_around_and_inside_reconstructed.jsonrouter-tests/testdata/queries_defer/requires_different_depth_defer_12_fragment_body_defer.graphqlrouter-tests/testdata/queries_defer/requires_different_depth_defer_12_fragment_body_defer.txtrouter-tests/testdata/queries_defer/requires_different_depth_defer_12_fragment_body_defer_reconstructed.jsonrouter-tests/testdata/queries_defer/requires_different_depth_defer_13_fragment_spread_defer.graphqlrouter-tests/testdata/queries_defer/requires_different_depth_defer_13_fragment_spread_defer.txtrouter-tests/testdata/queries_defer/requires_different_depth_defer_13_fragment_spread_defer_reconstructed.jsonrouter-tests/testdata/queries_defer/requires_different_depth_original.graphqlrouter-tests/testdata/queries_defer/requires_different_depth_original.jsonrouter-tests/testdata/queries_defer/requires_mood_defer_01_single_defer.graphqlrouter-tests/testdata/queries_defer/requires_mood_defer_01_single_defer.txtrouter-tests/testdata/queries_defer/requires_mood_defer_01_single_defer_reconstructed.json
Comment on lines
+195
to
+227
| for _, item := range partVal.GetArray("incremental") { | ||
| patchData := item.Get("data") | ||
| if patchData == nil { | ||
| continue | ||
| } | ||
|
|
||
| // Build path: prepend "data", then each segment from the path array. | ||
| pathKeys := []string{"data"} | ||
| for _, seg := range item.GetArray("path") { | ||
| switch seg.Type() { | ||
| case astjson.TypeNumber: | ||
| pathKeys = append(pathKeys, string(seg.MarshalTo(nil))) | ||
| default: | ||
| s, _ := seg.StringBytes() | ||
| pathKeys = append(pathKeys, string(s)) | ||
| } | ||
| } | ||
|
|
||
| if err := mergeAtPath(result, patchData, pathKeys); err != nil { | ||
| return nil, fmt.Errorf("merge at path %v: %w", pathKeys, err) | ||
| } | ||
|
|
||
| // Collect errors from incremental items into root errors. | ||
| patchErrors := item.Get("errors") | ||
| if patchErrors != nil && patchErrors.Type() == astjson.TypeArray { | ||
| existing := result.Get("errors") | ||
| if existing == nil || existing.Type() == astjson.TypeNull { | ||
| result.Set(nil, "errors", patchErrors) | ||
| } else { | ||
| merged := appendArrayValues(existing, patchErrors) | ||
| result.Set(nil, "errors", merged) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Don’t drop incremental errors when data is absent.
The early continue on Line 198 means an incremental entry that only carries errors never reaches the merge on Lines 217-227, so deferred error cases get reconstructed without those errors.
Suggested fix
for _, item := range partVal.GetArray("incremental") {
+ patchErrors := item.Get("errors")
+ if patchErrors != nil && patchErrors.Type() == astjson.TypeArray {
+ existing := result.Get("errors")
+ if existing == nil || existing.Type() == astjson.TypeNull {
+ result.Set(nil, "errors", patchErrors)
+ } else {
+ merged := appendArrayValues(existing, patchErrors)
+ result.Set(nil, "errors", merged)
+ }
+ }
+
patchData := item.Get("data")
if patchData == nil {
continue
}
@@
- // Collect errors from incremental items into root errors.
- patchErrors := item.Get("errors")
- if patchErrors != nil && patchErrors.Type() == astjson.TypeArray {
- existing := result.Get("errors")
- if existing == nil || existing.Type() == astjson.TypeNull {
- result.Set(nil, "errors", patchErrors)
- } else {
- merged := appendArrayValues(existing, patchErrors)
- result.Set(nil, "errors", merged)
- }
- }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| for _, item := range partVal.GetArray("incremental") { | |
| patchData := item.Get("data") | |
| if patchData == nil { | |
| continue | |
| } | |
| // Build path: prepend "data", then each segment from the path array. | |
| pathKeys := []string{"data"} | |
| for _, seg := range item.GetArray("path") { | |
| switch seg.Type() { | |
| case astjson.TypeNumber: | |
| pathKeys = append(pathKeys, string(seg.MarshalTo(nil))) | |
| default: | |
| s, _ := seg.StringBytes() | |
| pathKeys = append(pathKeys, string(s)) | |
| } | |
| } | |
| if err := mergeAtPath(result, patchData, pathKeys); err != nil { | |
| return nil, fmt.Errorf("merge at path %v: %w", pathKeys, err) | |
| } | |
| // Collect errors from incremental items into root errors. | |
| patchErrors := item.Get("errors") | |
| if patchErrors != nil && patchErrors.Type() == astjson.TypeArray { | |
| existing := result.Get("errors") | |
| if existing == nil || existing.Type() == astjson.TypeNull { | |
| result.Set(nil, "errors", patchErrors) | |
| } else { | |
| merged := appendArrayValues(existing, patchErrors) | |
| result.Set(nil, "errors", merged) | |
| } | |
| } | |
| for _, item := range partVal.GetArray("incremental") { | |
| patchErrors := item.Get("errors") | |
| if patchErrors != nil && patchErrors.Type() == astjson.TypeArray { | |
| existing := result.Get("errors") | |
| if existing == nil || existing.Type() == astjson.TypeNull { | |
| result.Set(nil, "errors", patchErrors) | |
| } else { | |
| merged := appendArrayValues(existing, patchErrors) | |
| result.Set(nil, "errors", merged) | |
| } | |
| } | |
| patchData := item.Get("data") | |
| if patchData == nil { | |
| continue | |
| } | |
| // Build path: prepend "data", then each segment from the path array. | |
| pathKeys := []string{"data"} | |
| for _, seg := range item.GetArray("path") { | |
| switch seg.Type() { | |
| case astjson.TypeNumber: | |
| pathKeys = append(pathKeys, string(seg.MarshalTo(nil))) | |
| default: | |
| s, _ := seg.StringBytes() | |
| pathKeys = append(pathKeys, string(s)) | |
| } | |
| } | |
| if err := mergeAtPath(result, patchData, pathKeys); err != nil { | |
| return nil, fmt.Errorf("merge at path %v: %w", pathKeys, err) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@router-tests/defer_test.go` around lines 195 - 227, The loop currently does
an early continue when patchData is nil which skips collecting incremental
"errors"; instead, change the loop so that the error-collection logic always
runs whether or not patchData exists: only skip the merge step when patchData ==
nil, but always execute the block that reads item.Get("errors") and merges into
result using existing := result.Get("errors"), appendArrayValues(existing,
patchErrors) and result.Set(nil, "errors", merged). Keep references to
mergeAtPath, appendArrayValues, patchData, patchErrors, pathKeys and result when
modifying the control flow so that path-building and mergeAtPath are conditional
but error aggregation always happens.
| github.com/wundergraph/cosmo/router v0.0.0-20260213130455-6e3277e7b850 | ||
| github.com/wundergraph/cosmo/router-plugin v0.0.0-20250808194725-de123ba1c65e | ||
| github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.250 | ||
| github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.250.0.20260327010529-31ffc570e6ea |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Direct OTel requirements =="
rg -n '^\s*go\.opentelemetry\.io/otel(|/sdk|/sdk/metric|/trace)\s+v' router-tests/go.mod
echo
echo "== OTel replace overrides =="
rg -n '^\s*go\.opentelemetry\.io/otel(|/sdk|/sdk/metric|/trace)\s*=>\s*go\.opentelemetry\.io/otel(|/sdk|/sdk/metric|/trace)\s+v' router-tests/go.mod
echo
echo "Expected: go.opentelemetry.io/otel/sdk resolves to >= v1.40.0 with no downgrade via replace."Repository: wundergraph/cosmo
Length of output: 690
Pin go.opentelemetry.io/otel/sdk to v1.40.0 or later in router-tests/go.mod.
The current configuration contains two OTel SDK versions both below the security baseline:
requiredirective:v1.36.0(line 31)replaceoverride:v1.28.0(line 198)
Both versions are vulnerable to GHSA-9h8m-3fm2-qjrq (CVE-2026-24051). Pin go.opentelemetry.io/otel/sdk to v1.40.0 or later in the require block and remove or update the replace directive to maintain the pinned version.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@router-tests/go.mod` at line 29, Update the module dependency on
go.opentelemetry.io/otel/sdk: in the go.mod require block change the version for
go.opentelemetry.io/otel/sdk to v1.40.0 or later, and remove or update any
replace directive that forces an older version (the existing replace of
go.opentelemetry.io/otel/sdk v1.28.0) so the require pin is respected; ensure
there are no conflicting replace entries and run go mod tidy to lock the updated
version.
Comment on lines
+8
to
+10
| ``` | ||
| {source}_{number}_{description}.graphql | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add a language tag to this fenced block.
Line 8 triggers MD040; text is enough here.
Suggested fix
-```
+```text
{source}_{number}_{description}.graphql</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.0)</summary>
[warning] 8-8: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @router-tests/testdata/queries_defer/README.md around lines 8 - 10, The
fenced code block containing "{source}{number}{description}.graphql" is
missing a language tag (MD040); update the triple-backtick fence to include a
language tag (use "text") so it becomes text before the content and
after; modify the README.md fenced block around the
"{source}{number}{description}.graphql" snippet to add the "text" tag.
</details>
<!-- fingerprinting:phantom:medusa:grasshopper -->
<!-- This is an auto-generated comment by CodeRabbit -->
Comment on lines
+48
to
+50
| ```bash | ||
| cd router-tests | ||
| go test -v -run TestDeferTestdataQueries -update ./... |
Contributor
There was a problem hiding this comment.
Fix the test name in the regeneration command.
Line 50 uses TestDeferTestdataQueries, but the test added in router-tests/defer_test.go is TestDeferTestDataQueries. As written, the documented -run regex will miss the new test.
Suggested fix
cd router-tests
-go test -v -run TestDeferTestdataQueries -update ./...
+go test -v -run TestDeferTestDataQueries -update ./...📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ```bash | |
| cd router-tests | |
| go test -v -run TestDeferTestdataQueries -update ./... |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@router-tests/testdata/queries_defer/README.md` around lines 48 - 50, Update
the regeneration command in README.md so the -run regex matches the actual test
name: change the `go test ... -run TestDeferTestdataQueries` invocation to use
`-run TestDeferTestDataQueries` (the test defined in router-tests/defer_test.go
as TestDeferTestDataQueries), ensuring the documented command will run the new
test.
use astjson to reconstruct full response
devsergiy
force-pushed
the
feat/eng-7770-add-defer-support
branch
from
c66159b to
9be5939
Compare
Router image scan passed✅ No security vulnerabilities found in image: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2697 +/- ##
===========================================
- Coverage 63.45% 43.25% -20.21%
===========================================
Files 251 236 -15
Lines 26767 26311 -456
===========================================
- Hits 16986 11380 -5606
- Misses 8414 13636 +5222
+ Partials 1367 1295 -72 🚀 New features to boost your workflow:
|
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
1 participant
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.
closes ENG-7979
Summary by CodeRabbit
Checklist
Open Source AI Manifesto
This project follows the principles of the Open Source AI Manifesto. Please ensure your contribution aligns with its principles.