fix: calculate costs for abstract fields correctly by ysmolski · Pull Request #2925 · wundergraph/cosmo

ysmolski · 2026-06-05T11:33:46Z

This PR fixes how costs for abstract types are calculated. Especially
for fragments used on abstract types.

For example, for actual costs, instead of summing each type of the
union, engine sums only costs for types actually seen in the returned
results.

For estimated costs, instead of summing all the implementing nodes of
abstract types, we just pick the implementing type with the maximum cost
and add that to the costs of fields selected on the abstract type
itself.

Also this includes fixes for jsonschema:

root variables object is always a non-nullable "object"
align nullability with JSON Schema 2020-12
emit $ref/$defs for recursive input types

Summary by CodeRabbit

Chores
- Bumped GraphQL tooling dependency to a newer patch release.
Bug Fixes / Improvements
- Improved accuracy of GraphQL response cost headers; actual cost computation adjusted to use more detailed resolver stats.
Tests
- Updated cost-related tests to match revised cost calculations and header values.
- Adjusted protocol/integration tests to reflect updated GraphQL input-schema serialization format.

This PR fixes how costs for abstract types are calculated. Especially for fragments used on abstract types. For example, for actual costs, instead of summing each type of the union, engine sums only costs for types actually seen in the returned results. For estimated costs, instead of summing all the implementing nodes of abstract types, we just pick the implementing type with the maximum cost and add that to the costs of fields selected on the abstract type itself. I had to recalculate and verify numbers in tests. Unfortunately, this fix is not final. There is unfinished work with how the cost of the field itself is selected. Right now we pick the maximum weight across implementing types and dataSources independently from children costs selection. This concerns the estimation the most. The better approach would be to be tie this process with the selection of the type with maximum cost.

…1-inline-fragments-on-abstract-list-types-charge-all-items

coderabbitai · 2026-06-05T11:34:05Z

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: d7e71710-f2af-48dd-99b3-d685ebb47a8a

📥 Commits

Reviewing files that changed from the base of the PR and between 9176855 and fbf6e91.

📒 Files selected for processing (2)

router-tests/protocol/mcp_test.go
router-tests/security/costs_test.go

Walkthrough

Bump graphql-go-tools to v2.4.4; switch router cost-header wiring and fallback to use map[string]resolve.TypeNameStats; update tests and MCP expected schema JSON accordingly.

Changes

Cost calculation TypeNameStats migration

Layer / File(s)	Summary
Dependency version bump `router-tests/go.mod`, `router/go.mod`	`github.com/wundergraph/graphql-go-tools/v2` updated to v2.4.4 in both modules.
Header callback type and wiring `router/core/header_rule_engine.go`	`headerPropagationWriter.costHeaderSetter` signature changed to accept `map[string]resolve.TypeNameStats`; `Write` now passes `h.resolveCtx.TypeNameStats`. OpenTelemetry and `zap` imports consolidated.
Synchronous cost calculation implementation `router/core/graphql_handler.go`	Response header callback computes `costActual` from `TypeNameStats`; post-resolution fallback uses `resolveCtx.TypeNameStats` instead of list-size map.
Tests and MCP schema expectation updates `router-tests/security/costs_test.go`, `router-tests/protocol/mcp_test.go`	Multi-subgraph cost test expected `X-WG-Cost-Actual` assertions updated; MCP test expected input-schema JSON updated to new representation (nullable expressed via `type`/`enum`).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

wundergraph/cosmo#2882: Changes to router cost-header computation and related tests.
wundergraph/cosmo#2887: Related changes to GraphQL operation cost computation and test expectations.
wundergraph/cosmo#2781: Prior modifications to cost accounting that affect actual-cost header results.

🚥 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 'fix: calculate costs for abstract fields correctly' clearly and concisely summarizes the main change: correcting cost calculations for abstract fields (union/interface types), which is the primary focus across the modified files.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

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 and usage tips.}

codecov · 2026-06-05T11:38:45Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.05%. Comparing base (9fa886b) to head (fbf6e91).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #2925       +/-   ##
===========================================
+ Coverage   42.18%   66.05%   +23.87%     
===========================================
  Files         846      258      -588     
  Lines      122252    27309    -94943     
  Branches     9731        0     -9731     
===========================================
- Hits        51573    18040    -33533     
+ Misses      70339     7818    -62521     
- Partials      340     1451     +1111

Files with missing lines	Coverage Δ
router/core/graphql_handler.go	`65.02% <100.00%> (ø)`
router/core/header_rule_engine.go	`89.98% <100.00%> (ø)`

... and 1102 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.

github-actions · 2026-06-05T15:42:06Z

Router image scan passed

✅ No security vulnerabilities found in image:

ghcr.io/wundergraph/cosmo/router:sha-3a1ff72b6bdf6df0a2c924510f56feb798e667bd

…ct" (#1528) Follow-up to #1518. That PR was correct but incomplete and exposed an old defect. ## Problem `GetSchema()` only forced the **root** variables object non-nullable when the operation had a required variable: ```go if len(v.schema.Required) > 0 { v.schema.Nullable = false } ``` Operations with **all-optional variables** kept a nullable root. Before #1518 that serialized to `{"type":"object","nullable":true}` and was harmless (validators ignore the unknown `"nullable"` keyword). After #1518 it serializes to `{"type":["object","null"]}`, which strict consumers reject - the MCP go-sdk's `AddTool` requires the input schema `type` to be exactly `"object"` and panics: ``` panic: AddTool "list_employees": input schema must have type "object" (got [object null]) ``` This breaks the cosmo router engine bump 2.4.2 -> 2.4.3 (wundergraph/cosmo#2925): `pkg/mcpserver`, `protocol` and `security` suites all panic. ## Fix The root variables object is always a concrete object - the container is present or omitted, never the JSON literal `null` - so set `Nullable = false` unconditionally in `GetSchema()`. This is consistent with nested input-object variables, which are already forced non-nullable. Only individual optional fields remain nullable. Golden tests updated (root `["object","null"]` -> `"object"`); the `root_schema_nullable_based_on_required_arguments` subtest renamed to `root schema is always a non-nullable object`. ## Tests `go test ./pkg/engine/jsonschema/` passes; gofmt and go vet clean. Verified against the cosmo router via a local `replace`: `pkg/mcpserver` no longer panics. ## Follow-up Needs a patch release (2.4.4) and a corresponding engine bump in cosmo#2925. Fixes ENG-9682

claude

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

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

…ist-types-charge-all-items

…2926) Updates two stale goldens in router-tests/protocol/mcp_test.go to unblock the ./protocol job on #2925. The engine bump (graphql-go-tools #1518/#1528) changed nullable rendering from {"type":"x","nullable":true} to the union {"type":["x","null"]} (nullable enums gain a trailing null). These goldens hard-code the generated schema, so they went stale. Engine fix is correct; goldens just needed updating.

ysmolski added 3 commits May 29, 2026 17:56

bump the engine to 2.4.3

1cfa480

Merge branch 'main' of github.com:wundergraph/cosmo into yury/eng-958…

ee850e5

…1-inline-fragments-on-abstract-list-types-charge-all-items

ysmolski added the query-planner-skip label Jun 5, 2026

github-actions Bot added the router label Jun 5, 2026

adopt changes in the engine

1fd7987

asoorm mentioned this pull request Jun 5, 2026

fix(jsonschema): root variables object is always a non-nullable "object" wundergraph/graphql-go-tools#1528

Merged

bump to 2.4.4

9176855

ysmolski marked this pull request as ready for review June 8, 2026 10:08

ysmolski requested a review from a team as a code owner June 8, 2026 10:08

claude Bot reviewed Jun 8, 2026

View reviewed changes

Merge branch 'main' into yury/eng-9581-inline-fragments-on-abstract-l…

a58f2c8

…ist-types-charge-all-items

devsergiy approved these changes Jun 8, 2026

View reviewed changes

asoorm mentioned this pull request Jun 8, 2026

test(router): update MCP input-schema goldens to union nullable form #2926

Merged

asoorm and others added 2 commits June 8, 2026 15:08

fix the comment

fbf6e91

ysmolski merged commit 474722f into main Jun 8, 2026
53 of 54 checks passed

ysmolski deleted the yury/eng-9581-inline-fragments-on-abstract-list-types-charge-all-items branch June 8, 2026 12:41

This was referenced Jun 11, 2026

chore: add non-cached benchmark to find the price of costs control #2929

Merged

feat: enable to ignore cost weights on implementing types #2976

Merged

Navigation Menu

Uh oh!

fix: calculate costs for abstract fields correctly - #2925

fix: calculate costs for abstract fields correctly#2925
ysmolski merged 8 commits into
mainfrom
yury/eng-9581-inline-fragments-on-abstract-list-types-charge-all-items

ysmolski commented Jun 5, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 5, 2026 •

edited

Loading

❌ Failed checks (1 warning)

Uh oh!

codecov Bot commented Jun 5, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jun 5, 2026 •

edited

Loading

Uh oh!

claude Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

ysmolski commented Jun 5, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

❌ Failed checks (1 warning)

Uh oh!

codecov Bot commented Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions Bot commented Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Router image scan passed

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Claude Code Review

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ysmolski commented Jun 5, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 5, 2026 •

edited

Loading

codecov Bot commented Jun 5, 2026 •

edited

Loading

github-actions Bot commented Jun 5, 2026 •

edited

Loading