Repository navigation
Require discriminator on polymorphic queries - #855
Merged
Merged
Conversation
Contributor
|
Tick the box to add this pull request to the merge queue (same as
|
rpiaggio
requested review from
hugo-vrijswijk
and
a lite review from Copilot
September 8, 2026 16:26
There was a problem hiding this comment.
🟡 Changes recommended
The new __typename validation currently considers __typename present even when it’s only reachable via same-type fragment flattening and ignores conditional directives, which can allow generating decoders that require a discriminator the query may not select unconditionally.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the Scala/Scalafix GraphQL generator to require __typename whenever a selection includes fragments on subtypes (interfaces/unions), and switches generated Circe decoders from “try each subtype” to a discriminator-based approach to avoid ambiguous decoding when subtypes share shapes.
Changes:
- Add a generator-time validation requiring
__typenamefor selections that include subtype fragments/spreads. - Generate discriminator-based sum decoders keyed by
__typename(or its alias), including anOtherfallback case when not all concrete subtypes are covered. - Update documentation and scripted/golden test fixtures to include
__typenameand reflect the new decoding strategy.
File summaries
| File | Description |
|---|---|
| sbt-plugin/src/sbt-test/clue-plugin/starwars/app/src/clue/scala/StarWarsQuery.scala | Updates scripted test query to include __typename for polymorphic selection. |
| sbt-plugin/src/sbt-test/clue-plugin/starwars-crossproject-pure/app/src/clue/scala/StarWarsQuery.scala | Same as above for crossproject scripted test. |
| README.md | Documents interface/union decoding model and __typename requirement; minor Markdown formatting updates. |
| gen/rules/src/main/scala/clue/gen/QueryGen.scala | Adds __typename validation + selection normalization and sum/fallback generation changes. |
| gen/rules/src/main/scala/clue/gen/Generator.scala | Adds discriminator-aware sum decoding generation (key/cases/fallback). |
| gen/output/src/main/scala/test/StarWarsSingleSubtype.scala | New golden output exercising single-variant + Other fallback behavior. |
| gen/output/src/main/scala/test/StarWarsQuery3.scala | Golden output updated: adds __typename and discriminator-based decoder. |
| gen/output/src/main/scala/test/StarWarsQuery2.scala | Golden output updated: adds __typename and discriminator-based decoder. |
| gen/output/src/main/scala/test/StarWarsQuery.scala | Golden output updated: adds __typename and discriminator-based decoder. |
| gen/output/src/main/scala/test/StarWarsNestedSubquery.scala | Golden output updated: subquery rooted at interface/union becomes a sealed-trait sum decoded via __typename. |
| gen/output/src/main/scala/test/StarWarsInterfaceSubtype.scala | New golden output covering interface-typed fragment condition mapping to concrete __typename. |
| gen/output/src/main/scala/test/StarWarsHumanSubquery.scala | New golden output for a concrete-type-rooted subquery used in splicing tests. |
| gen/output/src/main/scala/test/StarWarsAliasedTypenameOther.scala | New golden output covering aliased discriminator + Other fallback generation. |
| gen/output/src/main/scala/test/StarWarsAliasedTypename.scala | New golden output covering aliased discriminator and subtype decoding switch. |
| gen/output/src/main/scala/test/LucumaQuery3.scala | Golden output updated: adds __typename and discriminator-based decoder. |
| gen/output/src/main/scala/test/LucumaQuery2.scala | Golden output updated: adds __typename, generates sum with fallback for uncovered concrete types. |
| gen/output/src/main/scala/test/LucumaQuery.scala | Golden output updated: adds __typename and discriminator-based decoder. |
| gen/input/src/main/scala/test/StarWarsSubtypeNoTypename.scala | New input fixture asserting generator rejects missing __typename for subtype fragments. |
| gen/input/src/main/scala/test/StarWarsSubqueryInFragmentInvalid.scala | New input fixture asserting generator rejects subquery splicing inside fragments. |
| gen/input/src/main/scala/test/StarWarsSingleSubtype.scala | New input fixture for single-variant fragment case requiring fallback behavior. |
| gen/input/src/main/scala/test/StarWarsQuery3.scala | Input fixture updated to include __typename. |
| gen/input/src/main/scala/test/StarWarsQuery2.scala | Input fixture updated to include __typename. |
| gen/input/src/main/scala/test/StarWarsQuery.scala | Input fixture updated to include __typename. |
| gen/input/src/main/scala/test/StarWarsNestedSubquery.scala | Input fixture updated to include __typename for interface/union-rooted selection. |
| gen/input/src/main/scala/test/StarWarsInterfaceSubtype.scala | New input fixture for interface type-condition fragment case. |
| gen/input/src/main/scala/test/StarWarsHumanSubquery.scala | New input fixture defining a concrete-typed subquery for splicing tests. |
| gen/input/src/main/scala/test/StarWarsAliasedTypenameOther.scala | New input fixture for aliased discriminator + fallback behavior. |
| gen/input/src/main/scala/test/StarWarsAliasedTypename.scala | New input fixture for aliased discriminator across multiple variants. |
| gen/input/src/main/scala/test/LucumaQuery3.scala | Input fixture updated to include __typename. |
| gen/input/src/main/scala/test/LucumaQuery2.scala | Input fixture updated to include __typename. |
| gen/input/src/main/scala/test/LucumaQuery.scala | Input fixture updated to include __typename. |
| gen/input/src/main/resources/graphql/schemas/StarWars.graphql | Extends schema with Pilot interface used for discriminator mapping coverage. |
| build.sbt | Bumps base version (breaking-change release line). |
Review details
- Files reviewed: 32/33 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+425
to
+428
| val hasTypename: Boolean = flat.exists { | ||
| case (None, Query.UntypedSelect(TypeSelect, _, _, _, _)) => true | ||
| case _ => false | ||
| } |
Collaborator
Author
There was a problem hiding this comment.
Addressing in new commit.
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 PR makes
__typenamerequired whenever we query more than one subtype.eg:
Before this, to determine the type of the result, different decoders were tried until one succeeded. This is dangerous in subtypes that have the same shape.
This is a breaking change, but migration is mechanical.