Skip to content

fix(router): incorrect alias behavior for required fields - #3101

Merged
Noroth merged 4 commits into
mainfrom
ludwig/router-581-engine-alias-on-required-fields-is-misbehaving
Jul 22, 2026
Merged

fix(router): incorrect alias behavior for required fields#3101
Noroth merged 4 commits into
mainfrom
ludwig/router-581-engine-alias-on-required-fields-is-misbehaving

Conversation

@Noroth

@Noroth Noroth commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Related engine PR: wundergraph/graphql-go-tools#1606

Summary by CodeRabbit

Summary

  • Bug Fixes
    • Improved GraphQL @requires behavior when requested fields use aliases, including nested/abstract data and composite @requires scenarios.
    • Ensured alias-based composite @requires queries return null when the employee does not exist.
  • Tests
    • Expanded the GraphQL gRPC subgraph test suite with new alias-based cases to validate @requires resolution, nested alias access, and missing-employee behavior.
  • Chores
    • Updated the GraphQL tooling dependency version.

Checklist

Open Source AI Manifesto

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

@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown

Router image scan passed

✅ No security vulnerabilities found in image:

ghcr.io/wundergraph/cosmo/router:sha-0923afbb6db4ebca435a0cc6e9b74c39d9372c42

@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6f97893d-8f0b-44a3-8827-a71bbfb2856c

📥 Commits

Reviewing files that changed from the base of the PR and between 69cfd51 and ed5cf37.

⛔ Files ignored due to path filters (2)
  • router-tests/go.sum is excluded by !**/*.sum
  • router/go.sum is excluded by !**/*.sum
📒 Files selected for processing (2)
  • router-tests/go.mod
  • router/go.mod
🚧 Files skipped from review as they are similar to previous changes (2)
  • router/go.mod
  • router-tests/go.mod

Walkthrough

The router and router-tests modules update graphql-go-tools/v2 from v2.13.2 to v2.14.1. gRPC subgraph tests add alias coverage for tagged, union, abstract, and composite @requires field resolution.

Changes

@requires alias coverage

Layer / File(s) Summary
Dependency update
router/go.mod, router-tests/go.mod
Updates graphql-go-tools/v2 from v2.13.2 to v2.14.1 in both modules.
Alias regression coverage
router-tests/protocol/grpc_subgraph_test.go
Adds aliased test cases for tagged, union, and abstract-through-concrete @requires fields, plus nullability coverage for aliased composite fields on nonexistent employees.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: fixing alias handling for required fields in the router.
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.

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"


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

@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.83%. Comparing base (40a0efa) to head (ed5cf37).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3101      +/-   ##
==========================================
- Coverage   62.02%   61.83%   -0.20%     
==========================================
  Files         262      262              
  Lines       30810    30810              
==========================================
- Hits        19110    19050      -60     
- Misses      10188    10233      +45     
- Partials     1512     1527      +15     

see 11 files with indirect coverage changes

🚀 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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@router-tests/protocol/grpc_subgraph_test.go`:
- Around line 415-419: Add a test case in the existing employee query coverage
using the aliased field `aliasedReviewReport: reviewReport`, and assert its
expected non-null value to exercise alias resolution. Retain the current
non-existent employee null-propagation case only if it is still required by the
test contract.
🪄 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: f34580ef-0c6b-4740-b80c-93dfc71feace

📥 Commits

Reviewing files that changed from the base of the PR and between 991a647 and 9d3f481.

⛔ Files ignored due to path filters (2)
  • router-tests/go.sum is excluded by !**/*.sum
  • router/go.sum is excluded by !**/*.sum
📒 Files selected for processing (3)
  • router-tests/go.mod
  • router-tests/protocol/grpc_subgraph_test.go
  • router/go.mod

Comment thread router-tests/protocol/grpc_subgraph_test.go
@Noroth
Noroth marked this pull request as ready for review July 22, 2026 08:50
@Noroth
Noroth requested a review from a team as a code owner July 22, 2026 08:50

@SkArchon SkArchon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm

@Noroth
Noroth merged commit cfee827 into main Jul 22, 2026
36 checks passed
@Noroth
Noroth deleted the ludwig/router-581-engine-alias-on-required-fields-is-misbehaving branch July 22, 2026 09:58
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.

3 participants