fix(router): do not charge children of null-parents in actual costs - #3051
Merged
ysmolski merged 5 commits intoJul 7, 2026
Conversation
This fix includes basic and often used cases, but it does not include some combinations of abstract fields and fragments. For those I have included the test cases (disabled right now). I have simplified how parents are used in calculations. I have added children multiplier to distiungish them from the field cost which is always multiplied even when null was returned on that field. This solution can be sophisticated further if there is a need for it. Potentially, I could take different approach of walking the response from subgraphs and match them with Cost Tree. That would remove many heuristics and exceptions and make things much more simple. But that would happen in a separate PR.
Router image scan passed✅ No security vulnerabilities found in image: |
…6-cost-is-charged-for-fields-even-though-they-are-not-resolved
Contributor
WalkthroughThis PR bumps ChangesCost Control Enablement
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3051 +/- ##
===========================================
+ Coverage 43.43% 61.28% +17.85%
===========================================
Files 1069 261 -808
Lines 139019 30604 -108415
Branches 7229 0 -7229
===========================================
- Hits 60385 18757 -41628
+ Misses 76779 10324 -66455
+ Partials 1855 1523 -332
🚀 New features to boost your workflow:
|
…6-cost-is-charged-for-fields-even-though-they-are-not-resolved
ysmolski
enabled auto-merge (squash)
July 7, 2026 08:22
devsergiy
approved these changes
Jul 7, 2026
ysmolski
deleted the
yury/eng-9806-cost-is-charged-for-fields-even-though-they-are-not-resolved
branch
July 7, 2026 09:53
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
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.
This fix includes basic and often used cases, but it does not include
some combinations of abstract fields and fragments.
For those I have included the test cases (disabled right now).
I have simplified how parents are used in calculations.
I have added children multiplier to distiungish them from the field cost
which is always multiplied even when null was returned on that field.
This solution can be sophisticated further if there is a need for it.
Potentially, I could take different approach of walking the response
from subgraphs and match them with Cost Tree. That would remove
many heuristics and exceptions and make things much more simple.
But that would happen in a separate PR.
Summary by CodeRabbit