chore(demo): fix code generation for cost directives#2667
Merged
Conversation
Include new directives into the schemas to make gqlgen happy.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughUpdated demo and router-tests Go module pins and indirect deps; added explicit GraphQL directive definitions ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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. 📝 Coding Plan
Comment |
Router-nonroot image scan passed✅ No security vulnerabilities found in image: |
![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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@demo/go.mod`:
- Line 152: The dependency entry for github.com/wundergraph/graphql-go-tools/v2
is updated to a release-candidate v2.0.0-rc.265; decide whether to pin this
exact RC or revert to a stable release: update the go.mod entry to either keep
the exact version v2.0.0-rc.265 (to lock behavior and plan a future migration
when v2.0.0 GA is released) or change it to the latest stable GA when
API-stability is required, and document the chosen strategy in the repo’s
dependency policy so maintainers know to revisit the RC once v2.0.0 final is
published.
🪄 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: 7d184b58-d803-4a07-bb7a-a4ef59500947
⛔ Files ignored due to path filters (3)
demo/go.sumis excluded by!**/*.sumdemo/pkg/subgraphs/employees/subgraph/generated/generated.gois excluded by!**/generated/**demo/pkg/subgraphs/products/subgraph/generated/generated.gois excluded by!**/generated/**
📒 Files selected for processing (3)
demo/go.moddemo/pkg/subgraphs/employees/subgraph/schema.graphqlsdemo/pkg/subgraphs/products/subgraph/schema.graphqls
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2667 +/- ##
==========================================
+ Coverage 62.81% 63.01% +0.20%
==========================================
Files 245 245
Lines 26260 26260
==========================================
+ Hits 16496 16549 +53
+ Misses 8407 8366 -41
+ Partials 1357 1345 -12 🚀 New features to boost your workflow:
|
ysmolski
requested review from
Noroth,
SkArchon,
endigma and
jensneuse
as code owners
Noroth
reviewed
Mar 19, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
router-tests/testenv/testdata/configWithEdfs.json (1)
3248-3248: Consider sourcing the duplicated employees SDL from one place.This is effectively a second full copy of the Line 203 SDL. If this fixture stays hand-maintained, generating both values from the same source would reduce drift the next time the demo schema changes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@router-tests/testenv/testdata/configWithEdfs.json` at line 3248, The file contains a duplicated GraphQL SDL string assigned to the serviceSdl field (a second full copy of the employees SDL); extract the common SDL into a single shared source (e.g., a constant or fixture used by both places) and replace this literal in serviceSdl with a reference to that shared symbol (ensure the shared symbol name is descriptive like EMPLOYEES_SDL or sharedServiceSdl and update any tests/fixtures that currently embed the duplicate so both use the same value).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/demo-ci.yaml:
- Around line 8-10: The concurrency group currently uses github.head_ref which
groups runs by branch name and can cancel unrelated PRs; update the concurrency
group expression to include the PR number (e.g. use
github.event.pull_request.number) so runs are isolated per PR. Locate the
concurrency block (symbol: concurrency, keys: group and cancel-in-progress) and
replace or augment github.head_ref in the group expression with
github.event.pull_request.number (or fallback to github.run_id for non-PR
events) to ensure only the same PR run cancels in-progress jobs.
---
Nitpick comments:
In `@router-tests/testenv/testdata/configWithEdfs.json`:
- Line 3248: The file contains a duplicated GraphQL SDL string assigned to the
serviceSdl field (a second full copy of the employees SDL); extract the common
SDL into a single shared source (e.g., a constant or fixture used by both
places) and replace this literal in serviceSdl with a reference to that shared
symbol (ensure the shared symbol name is descriptive like EMPLOYEES_SDL or
sharedServiceSdl and update any tests/fixtures that currently embed the
duplicate so both use the same value).
🪄 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: f22b7f4a-28b2-4396-bc81-d50c0cd61bd1
📒 Files selected for processing (3)
.github/workflows/demo-ci.yamlrouter-tests/testenv/testdata/config.jsonrouter-tests/testenv/testdata/configWithEdfs.json
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/router-ci.yaml:
- Around line 165-167: The "Generate test configs from demo" workflow step uses
"run: update-config-no-edg.sh" which fails without an explicit path; update the
step so the run command invokes the script with a relative path (e.g., change to
"./update-config-no-edg.sh") while keeping the existing working-directory
(router-tests) so the script is executed from the correct folder.
- Around line 69-71: The workflow invokes the script by bare name
"update-config-no-edg.sh", which fails because the shell won't search the
current directory; update both occurrences of the invocation (the steps that run
update-config-no-edg.sh, e.g., the step titled "Generate test configs from demo"
and the other symmetric invocation later) to call it with a ./ prefix
("./update-config-no-edg.sh") so the script is executed from the working
directory.
🪄 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: 22717464-f9ce-4983-b4d6-e7890dad8be7
📒 Files selected for processing (1)
.github/workflows/router-ci.yaml
Noroth
approved these changes
Mar 19, 2026
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
2 participants
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.
Include new directives into the schemas to make gqlgen happy.
Fixes ENG-9243
Summary by CodeRabbit
Chores
New Features
Configuration
CI
Tests