Skip to content

chore: improve planning for required fields - #1606

Merged
Noroth merged 3 commits into
ludwig/router-581-engine-alias-on-required-fields-is-misbehavingfrom
ludwig/router-582-engine-improve-planning-for-required-fields
Jul 21, 2026
Merged

chore: improve planning for required fields#1606
Noroth merged 3 commits into
ludwig/router-581-engine-alias-on-required-fields-is-misbehavingfrom
ludwig/router-582-engine-improve-planning-for-required-fields

Conversation

@Noroth

@Noroth Noroth commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

This PR adds tests and reformats the logic for planning required fields in connect

@coderabbitai summary

Checklist

  • I have discussed my proposed changes in an issue and have received approval to proceed.
  • I have followed the coding standards of the project.
  • Tests or benchmarks have been added or updated.

Open Source AI Manifesto

This project follows the principles of the Open Source AI Manifesto. Please ensure your contribution aligns with its principles.

@Noroth
Noroth requested a review from a team as a code owner July 21, 2026 12:33

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.

Tip: disable this comment in your organization's Code Review settings.

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds end-to-end gRPC @requires tests and refactors federation planning to create required-field RPC calls from per-operation instances, using cached entity key messages and planned selections.

Changes

gRPC @requires execution

Layer / File(s) Summary
Federation required-field planning
v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor_federation.go
Stores required-field selection sets in federation configuration, records operation-specific planned instances, and caches entity key messages.
Required-field RPC construction
v2/pkg/engine/datasource/grpc_datasource/execution_plan.go
Builds required-field RPC calls from planned instances, including entity response paths, key messages, selections, and field arguments.
End-to-end execution coverage
execution/engine/execution_engine_grpc_requires_test.go, v2/pkg/grpctest/schema.go
Adds scalar, list, nested, and entity-count mismatch scenarios, with HTTP/gRPC subgraph setup and expanded planner metadata.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant GraphQLRequest
  participant FederationPlanner
  participant GRPCExecutionPlan
  participant ProductsSubgraph
  GraphQLRequest->>FederationPlanner: plan `@requires` selection
  FederationPlanner->>GRPCExecutionPlan: provide planned required-field instance
  GRPCExecutionPlan->>ProductsSubgraph: send entity required-field RPC
  ProductsSubgraph-->>GRPCExecutionPlan: return required-field result
  GRPCExecutionPlan-->>GraphQLRequest: merge result into response path
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly matches the main change: improving required-field planning.
Description check ✅ Passed The description is directly related to the code changes and test additions in this PR.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ludwig/router-582-engine-improve-planning-for-required-fields

Comment @coderabbitai help to get the list of available commands.

@Noroth Noroth changed the title Ludwig/router 582 engine improve planning for required fields chore: improve planning for required fields Jul 21, 2026
@Noroth
Noroth merged commit 6fa75ac into ludwig/router-581-engine-alias-on-required-fields-is-misbehaving Jul 21, 2026
10 of 11 checks passed
@Noroth
Noroth deleted the ludwig/router-582-engine-improve-planning-for-required-fields branch July 21, 2026 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants