Skip to content

Conversation

@kamilkisiela
Copy link
Contributor

Fixed an issue where @skip and @include directives were incorrectly removed from the initial Fetch of the Query Plan.

References #579

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @kamilkisiela, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a critical bug in the query planner where GraphQL's @skip and @include directives were not being correctly maintained during the initial query fetch. The changes introduce a new graph edge type to explicitly handle conditional inline fragments and refine the query planning logic to ensure these directives are properly respected and applied, thereby resolving incorrect query plan generation for conditional fields and fragments.

Highlights

  • Fix for @Skip and @include directives: Resolved an issue where @skip and @include directives were erroneously removed from the initial fetch of the Query Plan, ensuring they are correctly propagated.
  • New 'Selfie' Edge Type: Introduced a new Selfie edge type in the query graph to specifically represent and preserve conditions on inline fragments, preventing their premature removal.
  • Improved Condition Handling: Added utility methods (to_skip_if, to_include_if) to the Condition enum and refined the logic in the fetch planner to correctly apply or strip conditions based on their presence in the query path or ancestral steps.
  • Enhanced Test Coverage: Expanded test cases for @include and @skip directives, covering various scenarios including fragments, fields, interfaces, and unions at the root fetch level, to validate the fix.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request correctly fixes an issue where @skip and @include directives were being dropped from the initial fetch node. The introduction of a self-referencing Selfie edge is a clever approach to handle conditional inline fragments on the same type. The changes are well-tested with new snapshot tests covering various scenarios.

I have a few suggestions to improve the code further, mainly around logging, naming, and function signatures to enhance clarity and maintainability, in line with the repository's style guide.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Removed debug print statement for path finding failure.
@github-actions
Copy link

github-actions bot commented Nov 28, 2025

k6-benchmark results

     ✓ response code was 200
     ✓ no graphql errors
     ✓ valid response structure

     █ setup

     checks.........................: 100.00% ✓ 209913      ✗ 0    
     data_received..................: 6.1 GB  204 MB/s
     data_sent......................: 82 MB   2.7 MB/s
     http_req_blocked...............: avg=3.52µs   min=771ns  med=1.84µs  max=5.03ms   p(90)=2.61µs  p(95)=3.01µs  
     http_req_connecting............: avg=831ns    min=0s     med=0s      max=2.28ms   p(90)=0s      p(95)=0s      
     http_req_duration..............: avg=20.94ms  min=2.27ms med=20ms    max=111.15ms p(90)=28.56ms p(95)=31.58ms 
       { expected_response:true }...: avg=20.94ms  min=2.27ms med=20ms    max=111.15ms p(90)=28.56ms p(95)=31.58ms 
     http_req_failed................: 0.00%   ✓ 0           ✗ 69991
     http_req_receiving.............: avg=134.24µs min=24.6µs med=40.21µs max=34.93ms  p(90)=82.71µs p(95)=374.49µs
     http_req_sending...............: avg=25.02µs  min=5.7µs  med=10.9µs  max=21.62ms  p(90)=15.98µs p(95)=25.24µs 
     http_req_tls_handshaking.......: avg=0s       min=0s     med=0s      max=0s       p(90)=0s      p(95)=0s      
     http_req_waiting...............: avg=20.78ms  min=2.18ms med=19.87ms max=82.75ms  p(90)=28.29ms p(95)=31.29ms 
     http_reqs......................: 69991   2327.832223/s
     iteration_duration.............: avg=21.43ms  min=6.82ms med=20.37ms max=232.81ms p(90)=29.05ms p(95)=32.1ms  
     iterations.....................: 69971   2327.167042/s
     vus............................: 50      min=50        max=50 
     vus_max........................: 50      min=50        max=50 

@github-actions
Copy link

🐋 This PR was built and pushed to the following Docker images:

Image Names: ghcr.io/graphql-hive/router

Platforms: linux/amd64,linux/arm64

Image Tags: ghcr.io/graphql-hive/router:pr-591 ghcr.io/graphql-hive/router:sha-77f82e1

Docker metadata
{
"buildx.build.ref": "builder-54695a15-92e0-47c8-984d-a7842c325407/builder-54695a15-92e0-47c8-984d-a7842c3254070/qpfoxwchelo2kh2gn4evfqbm0",
"containerimage.descriptor": {
  "mediaType": "application/vnd.oci.image.index.v1+json",
  "digest": "sha256:4430a9644d2c36b9eb39fb27b4396d45a3753e819e77dc1d83417d9b52cca12e",
  "size": 1609
},
"containerimage.digest": "sha256:4430a9644d2c36b9eb39fb27b4396d45a3753e819e77dc1d83417d9b52cca12e",
"image.name": "ghcr.io/graphql-hive/router:pr-591,ghcr.io/graphql-hive/router:sha-77f82e1"
}

@kamilkisiela kamilkisiela merged commit 4eaf9ec into main Nov 28, 2025
20 checks passed
@kamilkisiela kamilkisiela deleted the kamil-plan-cond branch November 28, 2025 09:54
kamilkisiela pushed a commit that referenced this pull request Nov 28, 2025
> [!IMPORTANT]
> Merging this pull request will create these releases

# router 0.0.21 (2025-11-28)
## Features

- Subgraph Timeout Configuration (#541)

### Subgraph Request Timeout Feature

Adds support for configurable subgraph request timeouts via the
`traffic_shaping` configuration. The `request_timeout` option allows you
to specify the maximum time the router will wait for a response from a
subgraph before timing out the request. You can set a static timeout
(e.g., `30s`) globally or per-subgraph, or use dynamic timeouts with VRL
expressions to vary timeout values based on request characteristics.
This helps protect your router from hanging requests and enables
fine-grained control over how long requests to different subgraphs
should be allowed to run.

### Rename `original_url` variable to `default` in subgraph URL override
expressions.

This change aligns the variable naming with other configuration
expressions, such as timeout configuration.

When using expressions to override subgraph URLs, use `.default` to
refer to the original URL defined in the subgraph definition.

Example:

```yaml
url:
  expression: |
    if .request.headers."x-region" == "us-east" {
      "https://products-us-east.example.com/graphql"
    } else {
      .default
    }
```

## Fixes

- support `@include` and `@skip` in initial fetch node (#591)
- Fixed an issue where `@skip` and `@include` directives were
incorrectly removed from the initial Fetch of the Query Plan.
# node-addon 0.0.5 (2025-11-28)
## Fixes

- support `@include` and `@skip` in initial fetch node (#591)
- Fixed an issue where `@skip` and `@include` directives were
incorrectly removed from the initial Fetch of the Query Plan.
# query-planner 2.1.1 (2025-11-28)
## Fixes

- support `@include` and `@skip` in initial fetch node (#591)
- Fixed an issue where `@skip` and `@include` directives were
incorrectly removed from the initial Fetch of the Query Plan.
# config 0.0.13 (2025-11-28)
## Features

### Subgraph Request Timeout Feature

Adds support for configurable subgraph request timeouts via the
`traffic_shaping` configuration. The `request_timeout` option allows you
to specify the maximum time the router will wait for a response from a
subgraph before timing out the request. You can set a static timeout
(e.g., `30s`) globally or per-subgraph, or use dynamic timeouts with VRL
expressions to vary timeout values based on request characteristics.
This helps protect your router from hanging requests and enables
fine-grained control over how long requests to different subgraphs
should be allowed to run.

### Rename `original_url` variable to `default` in subgraph URL override
expressions.

This change aligns the variable naming with other configuration
expressions, such as timeout configuration.

When using expressions to override subgraph URLs, use `.default` to
refer to the original URL defined in the subgraph definition.

Example:

```yaml
url:
  expression: |
    if .request.headers."x-region" == "us-east" {
      "https://products-us-east.example.com/graphql"
    } else {
      .default
    }
```

## Fixes

- support `@include` and `@skip` in initial fetch node (#591)
# executor 6.2.0 (2025-11-28)
## Features

### Subgraph Request Timeout Feature

Adds support for configurable subgraph request timeouts via the
`traffic_shaping` configuration. The `request_timeout` option allows you
to specify the maximum time the router will wait for a response from a
subgraph before timing out the request. You can set a static timeout
(e.g., `30s`) globally or per-subgraph, or use dynamic timeouts with VRL
expressions to vary timeout values based on request characteristics.
This helps protect your router from hanging requests and enables
fine-grained control over how long requests to different subgraphs
should be allowed to run.

### Rename `original_url` variable to `default` in subgraph URL override
expressions.

This change aligns the variable naming with other configuration
expressions, such as timeout configuration.

When using expressions to override subgraph URLs, use `.default` to
refer to the original URL defined in the subgraph definition.

Example:

```yaml
url:
  expression: |
    if .request.headers."x-region" == "us-east" {
      "https://products-us-east.example.com/graphql"
    } else {
      .default
    }
```

## Fixes

- support `@include` and `@skip` in initial fetch node (#591)

Co-authored-by: knope-bot[bot] <152252888+knope-bot[bot]@users.noreply.github.com>
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.

1 participant