Conversation
WalkthroughRefactors resolvability into walker-based traversal, introduces NodeResolutionData and many new typed modules, replaces legacy resolvability utils with new utils/types, renames graph node/state shapes and exports, adjusts validate() to return a ValidationResult, and updates tests and a few utility imports/constant names. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🧰 Additional context used🧬 Code graph analysis (2)composition/src/resolvability-graph/walker/entity-walker/entity-walker.ts (4)
composition/src/resolvability-graph/graph-nodes.ts (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
Comment |
Aenimus
requested review from
JivusAyrus,
StarpTech and
jensneuse
as code owners
Router image scan passed✅ No security vulnerabilities found in image: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (6)
composition/src/resolvability-graph/types/types.ts (1)
7-15: Consider branded types for stronger compile-time safety.The string-based type aliases provide semantic clarity, and
NodeName's template literal enforces structure. For even stronger type safety, consider branded types forFieldName,SelectionPath,SubgraphName, andTypeNameto prevent accidental mixing of semantically different strings at compile time.Example using branded types:
export type FieldName = string & { readonly __brand: 'FieldName' }; export type SubgraphName = string & { readonly __brand: 'SubgraphName' }; export type TypeName = string & { readonly __brand: 'TypeName' }; export type SelectionPath = string & { readonly __brand: 'SelectionPath' };This prevents errors like passing a
FieldNamewhere aTypeNameis expected, while maintaining runtime compatibility.composition/src/resolvability-graph/graph.ts (1)
380-388: Cache skip may hide errors when the same entity appears under multiple unshared root fieldsSkipping a visit when resDataByNodeName already holds data for entityNodeName can mask failures if another root field reaches the same entity via a different path with different field coverage. Confirm intended semantics.
Consider scoping the skip by root field context or validating at least once per root field return type. Add tests where two distinct unshared root fields return the same entity and only one is resolvable.
composition/src/resolvability-graph/types/params.ts (1)
1-27: Types read clean and align with the new walker flowParam surfaces are coherent and minimal. Consider brief doc comments for relativeOriginPaths and the distinction between encounteredEntityNodeNames vs visitedEntities in future pass.
composition/src/resolvability-graph/graph-nodes.ts (1)
91-92: Rename parameter for consistency.The parameter
fieldDataByFieldNameon line 91 should be renamed tofieldDataByNameto match the field naming convention used throughout the file.Apply this diff:
- removeInaccessibleEdges(fieldDataByFieldName: Map<string, GraphFieldData>) { + removeInaccessibleEdges(fieldDataByName: Map<string, GraphFieldData>) { for (const [fieldName, edges] of this.headToSharedTailEdges) { - if (fieldDataByFieldName.has(fieldName)) { + if (fieldDataByName.has(fieldName)) { continue; }composition/src/resolvability-graph/walker/entity-walker/entity-walker.ts (1)
123-129: Remove unused variable.The
removeChildPathsvariable is declared and assigned (line 129) but never used. This appears to be dead code or an incomplete implementation.Apply this diff to remove the unused variable:
- let removeChildPaths: boolean | undefined = undefined; for (const [fieldName, edge] of node.headToTailEdges) { const { visited, areDescendantsResolved, isRevisitedNode } = this.visitEntityDescendantEdge({ edge, selectionPath, }); - removeChildPaths &&= isRevisitedNode; this.propagateVisitedField({composition/src/resolvability-graph/utils/utils.ts (1)
145-145: Optional chaining unnecessary.The parameter
entityAncestorsis not optional in the function signature (GenerateSharedResolvabilityErrorReasonsParams), so the optional chaining operator?.is unnecessary here.Apply this diff to remove the unnecessary optional chaining:
- if (typeName !== entityAncestors?.typeName) { + if (typeName !== entityAncestors.typeName) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
composition/src/errors/errors.ts(1 hunks)composition/src/index.ts(1 hunks)composition/src/resolvability-graph/constants/string-constants.ts(1 hunks)composition/src/resolvability-graph/graph-nodes.ts(5 hunks)composition/src/resolvability-graph/graph.ts(6 hunks)composition/src/resolvability-graph/node-resolution-data/node-resolution-data.ts(1 hunks)composition/src/resolvability-graph/node-resolution-data/types/params.ts(1 hunks)composition/src/resolvability-graph/types/params.ts(1 hunks)composition/src/resolvability-graph/types/types.ts(1 hunks)composition/src/resolvability-graph/utils.ts(0 hunks)composition/src/resolvability-graph/utils/types/params.ts(1 hunks)composition/src/resolvability-graph/utils/types/types.ts(1 hunks)composition/src/resolvability-graph/utils/utils.ts(1 hunks)composition/src/resolvability-graph/walker/entity-walker/entity-walker.ts(1 hunks)composition/src/resolvability-graph/walker/entity-walker/types/params.ts(1 hunks)composition/src/resolvability-graph/walker/root-field-walkers/root-field-walker.ts(1 hunks)composition/src/resolvability-graph/walker/root-field-walkers/types/params.ts(1 hunks)composition/src/utils/utils.ts(1 hunks)composition/src/v1/federation/federation-factory.ts(1 hunks)composition/src/v1/normalization/normalization-factory.ts(1 hunks)composition/tests/v1/resolvability.test.ts(27 hunks)
💤 Files with no reviewable changes (1)
- composition/src/resolvability-graph/utils.ts
🧰 Additional context used
🧬 Code graph analysis (13)
composition/src/resolvability-graph/utils/types/types.ts (1)
composition/src/resolvability-graph/types/types.ts (2)
SubgraphName(13-13)TypeName(15-15)
composition/src/resolvability-graph/node-resolution-data/types/params.ts (2)
composition/src/resolvability-graph/types/types.ts (2)
FieldName(7-7)TypeName(15-15)composition/src/utils/types.ts (1)
GraphFieldData(25-30)
composition/src/resolvability-graph/walker/root-field-walkers/root-field-walker.ts (4)
composition/src/resolvability-graph/types/types.ts (3)
NodeName(9-9)SelectionPath(11-11)VisitNodeResult(1-5)composition/src/resolvability-graph/node-resolution-data/node-resolution-data.ts (2)
NodeResolutionData(6-81)areDescendantsResolved(62-64)composition/src/resolvability-graph/walker/root-field-walkers/types/params.ts (7)
RootFieldWalkerParams(5-8)VisitEdgeParams(10-13)VisitNodeParams(15-18)GetNodeResolutionDataParams(42-45)PropagateVisitedFieldParams(20-27)PropagateVisitedSharedFieldParams(29-35)VisitRootFieldEdgesParams(37-40)composition/src/utils/utils.ts (2)
add(153-159)getValueOrDefault(143-151)
composition/src/resolvability-graph/types/params.ts (3)
composition/src/resolvability-graph/types/types.ts (4)
NodeName(9-9)SelectionPath(11-11)SubgraphName(13-13)RootFieldData(17-21)composition/src/resolvability-graph/node-resolution-data/node-resolution-data.ts (1)
NodeResolutionData(6-81)composition/src/resolvability-graph/walker/root-field-walkers/root-field-walker.ts (1)
RootFieldWalker(14-276)
composition/src/resolvability-graph/walker/root-field-walkers/types/params.ts (4)
composition/src/resolvability-graph/types/types.ts (4)
NodeName(9-9)SelectionPath(11-11)FieldName(7-7)TypeName(15-15)composition/src/resolvability-graph/node-resolution-data/node-resolution-data.ts (1)
NodeResolutionData(6-81)composition/src/resolvability-graph/graph-nodes.ts (2)
Edge(5-19)GraphNode(26-77)composition/src/resolvability-graph/walker/entity-walker/types/params.ts (2)
PropagateVisitedFieldParams(25-32)GetNodeResolutionDataParams(34-37)
composition/src/resolvability-graph/walker/entity-walker/types/params.ts (4)
composition/src/resolvability-graph/types/types.ts (4)
NodeName(9-9)SelectionPath(11-11)SubgraphName(13-13)FieldName(7-7)composition/src/resolvability-graph/node-resolution-data/node-resolution-data.ts (1)
NodeResolutionData(6-81)composition/src/resolvability-graph/graph-nodes.ts (2)
Edge(5-19)GraphNode(26-77)composition/src/resolvability-graph/walker/root-field-walkers/types/params.ts (2)
PropagateVisitedFieldParams(20-27)GetNodeResolutionDataParams(42-45)
composition/src/resolvability-graph/graph-nodes.ts (3)
composition/src/utils/types.ts (1)
GraphFieldData(25-30)composition/src/resolvability-graph/types/types.ts (2)
SubgraphName(13-13)TypeName(15-15)composition/src/utils/utils.ts (1)
getEntriesNotInHashSet(33-41)
composition/src/resolvability-graph/graph.ts (6)
composition/src/resolvability-graph/types/types.ts (5)
TypeName(15-15)NodeName(9-9)SelectionPath(11-11)ValidationResult(32-32)SubgraphName(13-13)composition/src/resolvability-graph/graph-nodes.ts (4)
EntityDataNode(103-116)GraphNode(26-77)RootNode(79-101)GraphNodeOptions(21-24)composition/src/resolvability-graph/node-resolution-data/node-resolution-data.ts (2)
NodeResolutionData(6-81)areDescendantsResolved(62-64)composition/src/resolvability-graph/walker/entity-walker/entity-walker.ts (1)
EntityWalker(14-235)composition/src/resolvability-graph/utils/utils.ts (5)
getMultipliedRelativeOriginPaths(311-323)newRootFieldData(24-37)generateRootResolvabilityErrors(191-226)generateEntityResolvabilityErrors(228-268)generateSharedEntityResolvabilityErrors(270-309)composition/src/resolvability-graph/walker/root-field-walkers/root-field-walker.ts (1)
RootFieldWalker(14-276)
composition/tests/v1/resolvability.test.ts (8)
composition/src/resolvability-graph/utils/utils.ts (6)
newRootFieldData(24-37)generateResolvabilityErrorReasons(53-102)UnresolvableFieldData(17-22)renderSelectionSet(169-179)generateSelectionSetSegments(153-167)generateSharedResolvabilityErrorReasons(104-151)composition/src/utils/string-constants.ts (2)
QUERY(115-115)OBJECT(104-104)composition/tests/utils/utils.ts (2)
federateSubgraphsFailure(49-57)federateSubgraphsSuccess(59-72)composition/src/errors/errors.ts (1)
unresolvablePathError(1429-1438)composition/src/resolvability-graph/utils/types/types.ts (1)
EntityAncestorCollection(9-13)composition/src/utils/types.ts (1)
GraphFieldData(25-30)composition/src/subgraph/types.ts (1)
Subgraph(11-15)composition/src/ast/utils.ts (1)
parse(272-274)
composition/src/resolvability-graph/node-resolution-data/node-resolution-data.ts (5)
composition/src/resolvability-graph/types/types.ts (2)
FieldName(7-7)SubgraphName(13-13)composition/src/utils/types.ts (1)
GraphFieldData(25-30)composition/src/resolvability-graph/node-resolution-data/types/params.ts (1)
NodeResolutionDataParams(4-10)composition/src/errors/errors.ts (1)
unexpectedEdgeFatalError(333-339)composition/src/resolvability-graph/utils.ts (1)
NodeResolutionData(6-26)
composition/src/resolvability-graph/utils/utils.ts (7)
composition/src/resolvability-graph/types/types.ts (5)
TypeName(15-15)FieldName(7-7)SubgraphName(13-13)RootFieldData(17-21)SelectionPath(11-11)composition/src/resolvability-graph/constants/string-constants.ts (2)
QUOTATION_JOIN(8-8)LITERAL_SPACE(6-6)composition/src/utils/types.ts (1)
GraphFieldData(25-30)composition/src/resolvability-graph/utils/types/params.ts (6)
GenerateResolvabilityErrorReasonsParams(49-53)GenerateSharedResolvabilityErrorReasonsParams(55-59)RootResolvabilityErrorsParams(22-26)ResolvabilityErrorsParams(28-34)SharedResolvabilityErrorsParams(36-42)GetMultipliedRelativeOriginPathsParams(44-47)composition/src/resolvability-graph/utils/types/types.ts (1)
SelectionSetSegments(15-19)composition/src/utils/utils.ts (1)
getOrThrowError(25-31)composition/src/errors/errors.ts (1)
unresolvablePathError(1429-1438)
composition/src/resolvability-graph/walker/entity-walker/entity-walker.ts (4)
composition/src/resolvability-graph/types/types.ts (4)
NodeName(9-9)SelectionPath(11-11)SubgraphName(13-13)VisitNodeResult(1-5)composition/src/resolvability-graph/node-resolution-data/node-resolution-data.ts (2)
NodeResolutionData(6-81)areDescendantsResolved(62-64)composition/src/resolvability-graph/walker/entity-walker/types/params.ts (7)
EntityWalkerParams(5-13)GetNodeResolutionDataParams(34-37)VisitEntityDescendantEdgeParams(15-18)VisitEntityDescendantNodeParams(20-23)PropagateVisitedFieldParams(25-32)AddUnresolvablePathsParams(39-42)RemoveUnresolvablePathsParams(44-47)composition/src/utils/utils.ts (2)
getValueOrDefault(143-151)add(153-159)
composition/src/resolvability-graph/utils/types/params.ts (4)
composition/src/resolvability-graph/types/types.ts (4)
NodeName(9-9)SelectionPath(11-11)RootFieldData(17-21)SubgraphName(13-13)composition/src/resolvability-graph/node-resolution-data/node-resolution-data.ts (1)
NodeResolutionData(6-81)composition/src/resolvability-graph/utils/types/types.ts (2)
EntityAncestorData(3-7)EntityAncestorCollection(9-13)composition/src/resolvability-graph/utils/utils.ts (1)
UnresolvableFieldData(17-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: build_push_image (nonroot)
- GitHub Check: image_scan (nonroot)
- GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
- GitHub Check: integration_test (./telemetry)
- GitHub Check: build_push_image
- GitHub Check: integration_test (./events)
- GitHub Check: image_scan
- GitHub Check: build_test
- GitHub Check: build_test
- GitHub Check: build_push_image
- GitHub Check: build_push_image
- GitHub Check: build_test
- GitHub Check: build_test
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🔇 Additional comments (36)
composition/src/utils/utils.ts (1)
251-252: LGTM! Parameter rename improves semantic clarity.The rename from
hashSettocollectionbetter reflects that the parameter accepts bothSetandMaptypes, making the function signature more accurate and self-documenting.composition/src/resolvability-graph/types/types.ts (3)
1-5: LGTM! Well-structured visit result type.The
VisitNodeResulttype clearly models the traversal state with explicit tracking of visit status, descendant resolution, and optional revisit detection.
17-21: LGTM! RootFieldData type is well-structured.The template literal for
coordsenforces theTypeName.FieldNamecoordinate format, and usingSet<SubgraphName>appropriately models unique subgraph membership.
23-32: LGTM! Clean discriminated union for validation results.The
ValidationResultdiscriminated union follows TypeScript best practices, enabling type narrowing based on thesuccessproperty. The structure clearly separates success and failure cases.composition/src/resolvability-graph/utils/types/params.ts (2)
1-6: LGTM! Import structure is clean and well-organized.The imports are properly organized with clear separation between external type dependencies and local utilities.
7-59: LGTM! Comprehensive and well-structured parameter types.The nine parameter types exhibit excellent consistency and clear naming conventions:
- Parallel structure: Entity/Root params have corresponding EntityShared/Shared variants for multi-entity scenarios (singular
EntityAncestorDatavs. collectionEntityAncestorCollection, singleNodeNamevs.Set<NodeName>)- Optional context:
pathFromRootis optional in broader params, allowing flexible usage- Type safety: Leverages imported type aliases (
NodeName,SelectionPath, etc.) for semantic correctness- Appropriate collections: Uses
Setfor uniqueness,Mapfor lookups,Iterablefor flexibilitycomposition/src/errors/errors.ts (1)
16-16: LGTM! Clean import path update.The import path refactor aligns with the module restructuring described in the PR summary. The change from
'../resolvability-graph/utils'to'../resolvability-graph/utils/utils'is consistent with moving utilities into a nested utils module.composition/src/v1/federation/federation-factory.ts (1)
2800-2802: LGTM! Improved validation result handling.The refactor from array-based error checking to a structured
ValidationResultobject withsuccessanderrorsproperties is a clean improvement. This makes the validation API more explicit and maintainable.composition/src/resolvability-graph/graph.ts (1)
20-31: Overall: walker index and per-node/path state look solidThe new maps (resDataByNodeName, resDataByRelativePathByEntity, visitedEntitiesByOriginEntity) and getNextWalkerIndex() establish clear traversal scoping. No issues spotted.
composition/src/resolvability-graph/graph-nodes.ts (5)
3-3: LGTM!The new imports support the template literal type for
nodeNameand align with the broader type system refactor.
27-27: LGTM!The field rename from
fieldDataByFieldNametofieldDataByNameimproves consistency and conciseness.
52-52: LGTM!The usage of the renamed field
fieldDataByNameis correct. The logic properly identifies inaccessible field names by comparing edges against available field data.
30-30: LGTM!The template literal type
${SubgraphName}.${TypeName}fornodeNameprovides better documentation of the expected format and aligns with the constructor assignment on line 43.
80-81: LGTM!The field renames improve consistency and clarity:
fieldDataByNamealigns withGraphNodeheadToSharedTailEdgesis clearer than "Shareable"composition/src/resolvability-graph/walker/entity-walker/types/params.ts (3)
1-13: LGTM!The imports and
EntityWalkerParamstype are well-structured and provide clear parameterization for theEntityWalkerconstructor.
15-37: LGTM!The method parameter types are well-defined. Note that
PropagateVisitedFieldParamsusesnodeName: NodeNameinstead ofnode: GraphNode(as in the root-field-walker variant), which aligns with the entity walker's usage pattern where only the node name is needed for propagation.
39-47: LGTM!The unresolvable path parameter types are clear and appropriately structured, with the optional
removeDescendantPathsflag providing necessary flexibility.composition/src/resolvability-graph/walker/entity-walker/entity-walker.ts (7)
14-42: LGTM!The class structure and constructor are well-organized with clear property documentation. All parameters are properly initialized.
44-64: Verify the design intent for multiple relative origin paths.The method returns the
NodeResolutionDatafrom the first relative origin path (line 61:returnData ??= data). While this is internally consistent, it assumes all origin path copies are equivalent or that returning any one is acceptable. Please verify this design choice aligns with the intended resolvability semantics.
66-110: LGTM with solid loop prevention logic.The edge visiting logic correctly handles:
- Inaccessible and leaf nodes
- Loop detection via
visitedIndices- Entity sibling prevention via
encounteredEntityNodeNames- Dispatching to abstract/concrete node handlers
112-148: LGTM!The concrete node visiting logic correctly:
- Handles leaf nodes
- Uses resolution data to avoid redundant work
- Iterates and propagates field resolution
- Manages unresolvable paths based on resolution status
150-162: LGTM!The abstract node visiting logic correctly requires all descendant edges (type implementations) to be resolved for the abstract node to be considered resolved.
164-199: LGTM on the overall propagation structure.The field propagation logic correctly updates resolution data across both node-specific and origin-specific data structures, with proper handling of relative origin paths.
211-234: LGTM!The unresolvable path removal logic correctly handles both single-path and origin-prefixed scenarios, with proper cascade deletion of descendant paths when requested.
composition/src/resolvability-graph/utils/utils.ts (12)
1-15: LGTM!The imports are well-organized and all appear to be utilized in the code.
17-22: LGTM!The type definition is clear and appropriate for representing unresolvable field data.
24-37: LGTM!The factory function correctly constructs RootFieldData with proper plural handling and message formatting.
39-51: LGTM!The formatting function correctly handles both leaf and non-leaf fields with appropriate indentation.
53-102: LGTM!The error reason generation logic is comprehensive and handles both entity ancestor and root-level scenarios correctly.
153-167: LGTM!The regex pattern and string-building logic correctly generate nested selection set segments while avoiding fragment spread syntax.
169-179: LGTM!The function correctly assembles the selection set with proper indentation.
181-189: LGTM!The path concatenation logic correctly handles all combinations of fieldPath and pathFromRoot.
191-226: LGTM!The error generation logic correctly processes unresolvable paths and constructs detailed error messages with reasons.
270-309: LGTM!The shared entity error generation logic is correct and avoids mutating input parameters.
311-323: LGTM!The path multiplication logic correctly handles both the presence and absence of relative origin paths.
256-257: The scripts will run and provide their output shortly.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
composition/src/resolvability-graph/graph-nodes.ts (2)
69-76: Update parameter type for consistency.The
accessibleEntityNodeNamesparameter is typed asSet<string>but receives aSet<NodeName>from the caller at line 64. While this works at runtime sinceNodeNameis a string alias, update the type for consistency:- getAccessibleEntityNodeNames(node: GraphNode, accessibleEntityNodeNames: Set<string>) { + getAccessibleEntityNodeNames(node: GraphNode, accessibleEntityNodeNames: Set<NodeName>) {
91-100: Rename parameter for consistency with property rename.The parameter
fieldDataByFieldNameshould be renamed tofieldDataByNameto align with the property rename at line 80.- removeInaccessibleEdges(fieldDataByFieldName: Map<FieldName, GraphFieldData>) { + removeInaccessibleEdges(fieldDataByName: Map<FieldName, GraphFieldData>) { for (const [fieldName, edges] of this.headToSharedTailEdges) { - if (fieldDataByFieldName.has(fieldName)) { + if (fieldDataByName.has(fieldName)) { continue; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
composition/src/resolvability-graph/graph-nodes.ts(6 hunks)composition/src/resolvability-graph/graph.ts(6 hunks)composition/src/resolvability-graph/node-resolution-data/node-resolution-data.ts(1 hunks)composition/src/resolvability-graph/node-resolution-data/types/params.ts(1 hunks)composition/src/resolvability-graph/utils/utils.ts(1 hunks)composition/src/resolvability-graph/walker/entity-walker/entity-walker.ts(1 hunks)composition/src/resolvability-graph/walker/entity-walker/types/params.ts(1 hunks)composition/src/resolvability-graph/walker/root-field-walkers/root-field-walker.ts(1 hunks)composition/src/v1/federation/federation-factory.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- composition/src/v1/federation/federation-factory.ts
- composition/src/resolvability-graph/node-resolution-data/types/params.ts
- composition/src/resolvability-graph/walker/entity-walker/types/params.ts
- composition/src/resolvability-graph/node-resolution-data/node-resolution-data.ts
- composition/src/resolvability-graph/walker/root-field-walkers/root-field-walker.ts
- composition/src/resolvability-graph/graph.ts
🧰 Additional context used
🧬 Code graph analysis (3)
composition/src/resolvability-graph/utils/utils.ts (7)
composition/src/resolvability-graph/types/types.ts (5)
TypeName(15-15)FieldName(7-7)SubgraphName(13-13)RootFieldData(17-21)SelectionPath(11-11)composition/src/resolvability-graph/constants/string-constants.ts (2)
QUOTATION_JOIN(8-8)LITERAL_SPACE(6-6)composition/src/utils/types.ts (1)
GraphFieldData(25-30)composition/src/resolvability-graph/utils/types/params.ts (6)
GenerateResolvabilityErrorReasonsParams(49-53)GenerateSharedResolvabilityErrorReasonsParams(55-59)RootResolvabilityErrorsParams(22-26)ResolvabilityErrorsParams(28-34)SharedResolvabilityErrorsParams(36-42)GetMultipliedRelativeOriginPathsParams(44-47)composition/src/resolvability-graph/utils/types/types.ts (1)
SelectionSetSegments(15-19)composition/src/utils/utils.ts (1)
getOrThrowError(25-31)composition/src/errors/errors.ts (1)
unresolvablePathError(1429-1438)
composition/src/resolvability-graph/walker/entity-walker/entity-walker.ts (4)
composition/src/resolvability-graph/types/types.ts (4)
NodeName(9-9)SelectionPath(11-11)SubgraphName(13-13)VisitNodeResult(1-5)composition/src/resolvability-graph/node-resolution-data/node-resolution-data.ts (2)
NodeResolutionData(6-73)areDescendantsResolved(54-56)composition/src/resolvability-graph/walker/entity-walker/types/params.ts (7)
EntityWalkerParams(5-13)GetNodeResolutionDataParams(34-37)VisitEntityDescendantEdgeParams(15-18)VisitEntityDescendantNodeParams(20-23)PropagateVisitedFieldParams(25-32)AddUnresolvablePathsParams(39-42)RemoveUnresolvablePathsParams(44-47)composition/src/utils/utils.ts (2)
getValueOrDefault(143-151)add(153-159)
composition/src/resolvability-graph/graph-nodes.ts (3)
composition/src/resolvability-graph/types/types.ts (4)
FieldName(7-7)NodeName(9-9)SubgraphName(13-13)TypeName(15-15)composition/src/utils/types.ts (1)
GraphFieldData(25-30)composition/src/utils/utils.ts (2)
getEntriesNotInHashSet(33-41)getValueOrDefault(143-151)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: build_test
- GitHub Check: build_push_image
- GitHub Check: build_push_image (nonroot)
- GitHub Check: build_push_image
- GitHub Check: integration_test (./events)
- GitHub Check: image_scan (nonroot)
- GitHub Check: integration_test (./telemetry)
- GitHub Check: image_scan
- GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
- GitHub Check: build_test
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: build_test
- GitHub Check: build_test
- GitHub Check: build_push_image
🔇 Additional comments (9)
composition/src/resolvability-graph/graph-nodes.ts (7)
3-3: LGTM: Type imports strengthen type safety.The centralized type imports from
./types/typesprovide semantic clarity and type-level documentation without runtime overhead.
27-38: LGTM: Property refinements improve clarity.The rename of
fieldDataByFieldNametofieldDataByNameand the addition of branded string types (NodeName,SubgraphName,TypeName) improve semantic clarity without changing runtime behavior.
40-46: LGTM: Constructor signature aligns with type refinements.The constructor correctly accepts the new branded types and derives
nodeNameusing a template literal that matches theNodeNametype definition.
48-60: LGTM: Property rename applied consistently.The method correctly uses the renamed
fieldDataByNameproperty. Logic remains unchanged and correct.
62-67: LGTM: Return type refinement enhances type safety.The return type
Set<NodeName>provides better semantic clarity while maintaining compatibility sinceNodeNameis a string alias.
80-89: LGTM: RootNode properties and constructor updated correctly.The property renames (
fieldDataByName,headToSharedTailEdges) and type refinements are consistent with the broader refactor.
104-118: LGTM: EntityDataNode type refinements are correct.The branded types (
SubgraphName) are consistently applied to the bidirectional mapping between field sets and target subgraph names. Logic remains correct.composition/src/resolvability-graph/utils/utils.ts (2)
24-37: LGTM!The function correctly constructs
RootFieldDatawith appropriate pluralization logic and message formatting.
256-257: Verify safety of mutatingentityAncestorData.subgraphName
Line 257 mutates the inputentityAncestorData.subgraphName, introducing a side effect that may corrupt subsequent error reports if the same object is reused. Manually verify that callers ofgenerateEntityResolvabilityErrorsnever read or reuse this object after the mutation. Otherwise, either cloneentityAncestorDatabefore mutating, or refactor the API to passsubgraphNamedirectly togenerateResolvabilityErrorReasons.
2 participants
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Tests
Checklist