Repository navigation
[Fusion] Fix silently skipped lookups in batched operation plans - #10229
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes Fusion planning/execution behavior where nested entity lookups could be skipped (silently) in batched operation plans when sibling union-member selections differ, by making batching/grouping and dependency wiring deterministic and correctly optional for merged multi-target operations. This aligns with the reported issue (#10217) and adds regression coverage for both Fusion-internal lookups and Apollo Federation _entities flows.
Changes:
- Assign fresh, deterministic execution node IDs for batch wrapper nodes and ensure root-node detection accounts for redirected step IDs (including de-duplication).
- Treat dependencies as optional for batch nodes that contain either multiple operations or a single merged multi-target operation (planner + JSON plan parser).
- Add/extend regression tests for the union-sibling differing-subselection scenario and update snapshots broadly to reflect the new deterministic ID assignment and node ordering.
Reviewed changes
Copilot reviewed 110 out of 110 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/HotChocolate/Fusion/src/Fusion.Execution/Planning/OperationPlanner.BuildExecutionTree.cs | Generates unique batch wrapper node IDs, stabilizes grouping order, updates root-node selection for redirected IDs, and adjusts dependency wiring rules for batch nodes. |
| src/HotChocolate/Fusion/src/Fusion.Execution/Planning/OperationPlanner.Defer.cs | Ensures deferred plan execution-node IDs start after plan step IDs to avoid collisions when building deferred execution nodes. |
| src/HotChocolate/Fusion/src/Fusion.Execution/Planning/OperationPlanner.cs | Passes planSteps.NextId() into execution-plan building so wrapper nodes can allocate unique IDs beyond step IDs. |
| src/HotChocolate/Fusion/src/Fusion.Execution/Execution/Nodes/Serialization/JsonOperationPlanParser.cs | Deduplicates redirected dependencies and applies optional-dependency rules for merged multi-target batch nodes when parsing JSON plans. |
| src/HotChocolate/Fusion/src/Fusion.Execution/Execution/Nodes/OperationBatchExecutionNode.cs | Updates skip semantics so merged multi-target operations only skip when all their dependencies were skipped. |
| src/HotChocolate/Fusion/test/Fusion.Execution.Tests/Planning/AbstractLookupFanoutPlanningTests.cs | Adds regression tests and ID-collision guard for union-sibling nested lookups with differing sibling selections (Fusion + Apollo variants). |
| src/HotChocolate/Fusion/test/Fusion.Execution.Tests/Execution/Serialization/JsonOperationPlanSerializationTests.cs | Adds coverage ensuring JSON formatting/parsing preserves optional dependency wiring and unique ID invariants for batch nodes. |
| src/HotChocolate/Fusion/test//snapshots/ | Snapshot updates reflecting new deterministic wrapper IDs, dependency redirection, and node ordering across many planning/execution tests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Patch coverage94.8% of changed lines covered (73/77)
Uncovered changed lines (JSON){
"sha": "3172fd33c56ee7af346bbfde735df617073d8f86",
"files": [
{ "path": "src/HotChocolate/Fusion/src/Fusion.Execution/Execution/Nodes/OperationBatchExecutionNode.cs", "ranges": [[448, 449], [458, 458], [460, 460]] }
]
}Project coverage: 54.4% (242613/445891 lines) |
Fixes #10217