Skip to content

Fix UseConnection ignoring global paging options in cost analysis - #9975

Merged
michaelstaib merged 4 commits into
mainfrom
mst/cost-issue-connection
Jun 27, 2026
Merged

michaelstaib merged 4 commits into
mainfrom
mst/cost-issue-connection

Conversation

@michaelstaib

Copy link
Copy Markdown
Member

No description provided.

Copilot AI review requested due to automatic review settings June 23, 2026 21:36

Copilot AI 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.

Pull request overview

This PR addresses a cost analysis discrepancy where connection fields configured via [UseConnection] could ignore globally configured paging options (notably MaxPageSize) during schema-cost metadata generation, leading to skewed @listSize(assumedSize: ...) values.

Changes:

  • Update [UseConnection] attribute configuration to set merged paging options (including global defaults) onto field features during type creation.
  • Add regression tests validating that @listSize.assumedSize honors global MaxPageSize for [UseConnection] when no per-field override is provided.
  • Add coverage for explicit per-field overrides via [UseConnection(MaxPageSize = ...)].

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/HotChocolate/CostAnalysis/test/CostAnalysis.Tests/ConnectionOverridePagingCostTests.cs Adds regression tests ensuring @listSize.assumedSize reflects global and per-field paging options for [UseConnection].
src/HotChocolate/Core/src/Types.CursorPagination/Extensions/UseConnectionAttribute.cs Ensures paging options stored on field features are merged with global paging options early enough for cost analysis defaults.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +35 to +53
// [UsePaging] and [UseConnection] are two ways to declare the same connection field, so they
// must derive the same @listSize.assumedSize from the global MaxPageSize when no per-field
// override is set. The source-gen connection shape (connection flag + [UseConnection]) must
// match too.
[Fact]
public async Task ListSize_AssumedSize_Should_Be_Equal_For_UsePaging_And_UseConnection_Without_Override()
{
// arrange & act
var usePaging = await GetBooksAssumedSizeAsync(b => b.AddQueryType<UsePagingQuery>());
var useConnection = await GetBooksAssumedSizeAsync(b => b.AddQueryType<NoOverrideQueryType>());
var sourceGenConnection = await GetBooksAssumedSizeAsync(b => b.AddQueryType<NoOverrideQueryType>());

// assert
Assert.Equal(100, usePaging);
Assert.Equal(100, useConnection);
Assert.Equal(100, sourceGenConnection);
Assert.Equal(usePaging, useConnection);
Assert.Equal(usePaging, sourceGenConnection);
}
@github-code-quality

Copy link
Copy Markdown

Code Coverage Overview

Languages: C#

C# / code-coverage/dotnet

The overall coverage in the mst/cost-issue-conne... branch remains at 49%, unchanged from the main branch.

Show a code coverage summary of the most impacted files.
File main 303ba5f mst/cost-issue-conne... d3002cb +/-
/home/runner/wo...geDispatcher.cs 77% 70% -7%
/home/runner/wo...gerMessage.g.cs 19% 15% -4%
/home/runner/wo...ueryExecutor.cs 67% 65% -2%
/home/runner/wo...tor/Mediator.cs 91% 89% -2%
/home/runner/wo...DefaultTopic.cs 89% 90% +1%
/home/runner/wo...nManagerBase.cs 82% 83% +1%
/home/runner/wo...cutorManager.cs 94% 95% +1%
/home/runner/wo...gerMessage.g.cs 6% 10% +4%
/home/runner/wo...TypeRegistry.cs 95% 100% +5%
/home/runner/wo...eiveEndpoint.cs 69% 80% +11%

Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

github-actions Bot commented Jun 26, 2026 •

Copy link
Copy Markdown
Contributor

Patch coverage

100.0% of changed lines covered (1/1)

File Covered Changed Patch %
…/Types.CursorPagination/Extensions/UseConnectionAttribute.cs 1 1 100.0% 🟢

Project coverage: 52.4% (215284/411029 lines)

@michaelstaib
michaelstaib merged commit 2ec6985 into main Jun 27, 2026
147 checks passed
@michaelstaib
michaelstaib deleted the mst/cost-issue-connection branch June 27, 2026 15:38
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.

2 participants