feat: add type mismatch relaxation config for field selection merging#1395
feat: add type mismatch relaxation config for field selection merging#1395
Conversation
Add a new configuration flag `RelaxSubgraphOperationFieldSelectionMergingTypeMismatch` that allows completely differing field types (e.g. IssueState vs PullRequestReviewState, or Int vs String) on fields in non-overlapping concrete object types within inline fragments. This extends the existing nullability relaxation pattern and is gated behind an opt-in flag across all 5 layers: validation rule, validator options, plan configuration, datasource interface, and factory pool. Includes unit tests (8 cases), planning tests, and execution integration tests (3 cases). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Change the relaxation flag wiring in CreatePlannerConfiguration from sequential if/if to if/else if, ensuring the broader type mismatch pool is preferred when both flags are set. Add documentation explaining the two-level relaxation hierarchy at the core validation logic, options struct, and factory method. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Change FieldSelectionMerging signature from variadic to pointer param
- Extract duplicated relaxation logic into canRelaxTypeMismatch helper
- Rename test for naming consistency ("allows" instead of "relaxes")
- Add test for both relaxation flags set simultaneously
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe PR extends the field selection merging validation framework by introducing a type-mismatch relaxation option that parallels the existing nullability relaxation. The feature is integrated across validation rules, planner configuration, execution engine initialization, and data source factory configuration. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Planner as Planner/Config
participant DataSource as GraphQL DataSource
participant Engine as Execution Engine
participant Validator as AST Validator
Client->>Planner: Create plan with config
alt RelaxSubgraphOperationFieldSelectionMergingTypeMismatch enabled
Planner->>DataSource: EnableSubgraphFieldSelectionMergingTypeMismatchRelaxation()
DataSource->>DataSource: Use typeMismatchRelaxedPrintKitPool
DataSource->>DataSource: Apply WithRelaxFieldSelectionMergingTypeMismatch()
end
Planner->>Engine: Initialize with relaxation flags
Engine->>Validator: Append validation options
alt Type mismatch in field selection
Validator->>Validator: canRelaxTypeMismatch() check
alt Relaxation enabled
Validator->>Validator: Allow merge (type mismatch relaxed)
else Relaxation disabled
Validator->>Validator: Reject merge (type mismatch error)
end
end
Validator-->>Engine: Validation result
Engine-->>Client: Execution result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
v2/pkg/astvalidation/operation_validation_test.go (2)
5802-5838: Unused BotKind enum can be removed or used.
BotKindisn’t referenced inentityDefinition; either wire it intoBotfor test coverage or remove it to keep the schema focused.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@v2/pkg/astvalidation/operation_validation_test.go` around lines 5802 - 5838, The BotKind enum is declared but unused in the test schema; either remove the unused enum BotKind or add a field on the Bot type that references it (e.g., a "kind: BotKind" field) so BotKind is exercised; update the union Entity/ Bot type accordingly (look for symbols BotKind, type Bot, and union Entity) and adjust any tests expecting the previous shape.
1118-1215: Add an overlap guard test for scalar type mismatches.You already assert that RelaxTypeMismatchCheck doesn’t apply when an interface overlap is possible (nullability case). Consider adding a scalar-type mismatch case (e.g., Int vs String) on overlapping interface/union types to ensure the relaxation never leaks beyond non‑overlapping concrete objects.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@v2/pkg/astvalidation/operation_validation_test.go` around lines 1118 - 1215, Add a new test that mirrors the nullability overlap guard but for scalar type mismatches: call runWithDefinition with the overlapping interface/union definition (e.g., use boxDefinition or another interface-based definition that makes potentiallySameObject=true), a query that selects the same scalar field (e.g., score) on two overlapping inline fragments returning different scalar types (Int vs String), pass FieldSelectionMerging(&FieldSelectionMergingOptions{RelaxTypeMismatchCheck: true}) and assert Invalid with a validation error like "fields 'score' conflict because they return conflicting types 'Int' and 'String'". Place this alongside the existing tests and follow their pattern using runWithDefinition, FieldSelectionMerging, FieldSelectionMergingOptions, and withValidationErrors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@v2/pkg/astvalidation/operation_validation_test.go`:
- Around line 5802-5838: The BotKind enum is declared but unused in the test
schema; either remove the unused enum BotKind or add a field on the Bot type
that references it (e.g., a "kind: BotKind" field) so BotKind is exercised;
update the union Entity/ Bot type accordingly (look for symbols BotKind, type
Bot, and union Entity) and adjust any tests expecting the previous shape.
- Around line 1118-1215: Add a new test that mirrors the nullability overlap
guard but for scalar type mismatches: call runWithDefinition with the
overlapping interface/union definition (e.g., use boxDefinition or another
interface-based definition that makes potentiallySameObject=true), a query that
selects the same scalar field (e.g., score) on two overlapping inline fragments
returning different scalar types (Int vs String), pass
FieldSelectionMerging(&FieldSelectionMergingOptions{RelaxTypeMismatchCheck:
true}) and assert Invalid with a validation error like "fields 'score' conflict
because they return conflicting types 'Int' and 'String'". Place this alongside
the existing tests and follow their pattern using runWithDefinition,
FieldSelectionMerging, FieldSelectionMergingOptions, and withValidationErrors.
Summary
RelaxSubgraphOperationFieldSelectionMergingTypeMismatchconfig flag that allows completely different field types (e.g.,IntvsString) on fields within non-overlapping concrete object types inside inline fragmentsRelaxSubgraphOperationFieldSelectionMergingNullabilityflag: when set, it allows any type difference (including nullability), not just nullability mismatchesTest plan
resolve.Fieldentries withOnTypeNamesgo test ./v2/...andgo test ./execution/...)🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Tests