chore: fix comments and formatting in debug

[ysmolski]

Something I fixed as I read the code.

Summary by CodeRabbit

• Chores
  • Enhanced internal query planning for improved data source navigation in federated operations.
  • Updated debug logging output formatting for better readability.
  • Minor documentation wording adjustments and code formatting improvements.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Adds a new collectNodes private method to DataSourceFilter for collecting node information and building jump graphs for navigation between data sources via federation @key directives. Includes minor formatting, documentation, and debug logging updates across multiple query planning visitor files.

Changes

Cohort / File(s)	Summary
Jump Graph Construction v2/pkg/engine/plan/datasource_filter_visitor.go	New collectNodes method initializes a nodesCollector, collects node data, and constructs DataSourceJumpsGraph instances for each (Path, TypeName) combination to enable data source navigation via @key directives; called during data source filtering in findBestDataSourceSet.
Query Planning Formatting & Comments v2/pkg/engine/plan/node_selection_builder.go, v2/pkg/engine/plan/planner.go, v2/pkg/engine/plan/visitor.go	Function call reformatting, debug logging format adjustments (e.g., "EnterField:" vs "EnterField : "), and inline comment wording refinements across planner/visitor classes.
Visitor Documentation & Logging v2/pkg/engine/plan/node_selection_visitor.go	Updated documentation text to reference plural "data sources" and "abstract selections"; enhanced EnterField debug output to include handleRequires flag.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

• The new collectNodes method is localized to one file and follows a clear aggregation pattern; review focus should verify correct graph construction and integration with existing filtering logic.
• Majority of changes are formatting, comments, and debug output adjustments with minimal functional impact.

Possibly related PRs

• fix: planner fixes for parent entity jumps and unique nodes selections #1230: Modifies node selection and duplicate-handling logic in datasource_filter_visitor.go alongside main PR's jump-graph setup.
• fix: fix parent node jump lookup #1252: Fixes parent node jump lookup logic in the same file where collectNodes is added.
• fix: improve query planning time #1326: Overlaps in data-source collection/visitor logic and FilterDataSources signature modifications with collectNodes additions.

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the changeset: the PR introduces a new private method (collectNodes) for data source filtering and contains multiple comment/formatting fixes across several files, aligning with the 'chore: fix comments and formatting in debug' title.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch yury/comment_formatting

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

v2/pkg/engine/plan/datasource_filter_visitor.go (1)

148-200: Consider simplifying the KeyIndex usage.

The method correctly extracts and organizes node collection logic. However, line 198 creates a new KeyIndex struct with the same fields as the loop variable keyIndex, which is redundant.

Apply this diff to simplify:

 	for keyIndex, keysPerDS := range keysForPathForTypename {
-		f.jumpsForPathForTypename[KeyIndex{Path: keyIndex.Path, TypeName: keyIndex.TypeName}] = NewDataSourceJumpsGraph(usedDsHashes, keysPerDS, keyIndex.TypeName)
+		f.jumpsForPathForTypename[keyIndex] = NewDataSourceJumpsGraph(usedDsHashes, keysPerDS, keyIndex.TypeName)
 	}

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 16905bc and 27b53eb.

📒 Files selected for processing (5)

• v2/pkg/engine/plan/datasource_filter_visitor.go (1 hunks)
• v2/pkg/engine/plan/node_selection_builder.go (2 hunks)
• v2/pkg/engine/plan/node_selection_visitor.go (2 hunks)
• v2/pkg/engine/plan/planner.go (1 hunks)
• v2/pkg/engine/plan/visitor.go (2 hunks)

🧰 Additional context used

🧠 Learnings (3)

📚 Learning: 2025-08-08T09:43:07.433Z

Learnt from: Noroth
Repo: wundergraph/graphql-go-tools PR: 1262
File: v2/pkg/engine/datasource/grpc_datasource/json_builder.go:0-0
Timestamp: 2025-08-08T09:43:07.433Z
Learning: In v2/pkg/engine/datasource/grpc_datasource/json_builder.go, mergeEntities intentionally uses the loop index when calling indexMap.getResultIndex because the index map is type-aware, making per-type counters unnecessary under the current assumptions. Avoid suggesting per-type ordinal counters for this path in future reviews.

Applied to files:

• v2/pkg/engine/plan/datasource_filter_visitor.go

📚 Learning: 2025-11-19T10:53:06.342Z

Learnt from: Noroth
Repo: wundergraph/graphql-go-tools PR: 1341
File: v2/pkg/engine/datasource/grpc_datasource/execution_plan.go:1039-1097
Timestamp: 2025-11-19T10:53:06.342Z
Learning: In v2/pkg/engine/datasource/grpc_datasource field resolver response handling, the `resolveRequiredFields` function intentionally uses two distinct approaches: for simple GraphQL object types it populates `message.Fields`, while for composite types (interface/union) it exclusively uses `message.FieldSelectionSet` with fragment-based selections. This differs from `buildFieldMessage` (regular queries) because field resolver responses returning composite types must align with protobuf oneOf structure, where all selections—including common interface fields—are handled through fragment selections built by `buildCompositeField`. The two approaches cannot be mixed in field resolver responses.

Applied to files:

• v2/pkg/engine/plan/node_selection_visitor.go

📚 Learning: 2025-12-02T08:25:26.682Z

Learnt from: Noroth
Repo: wundergraph/graphql-go-tools PR: 1341
File: v2/pkg/engine/datasource/grpc_datasource/execution_plan.go:1053-1118
Timestamp: 2025-12-02T08:25:26.682Z
Learning: In v2/pkg/engine/datasource/grpc_datasource field resolver response handling for composite types (interface/union), buildFieldResolverTypeMessage correctly combines both message.Fields and message.FieldSelectionSet: message.Fields contains interface-level fields that can be selected directly on the interface (such as __typename or fields defined in the interface itself), while message.FieldSelectionSet contains type-specific fields from inline fragments. This mixing is intentional and correct for GraphQL interfaces, as interface-level fields exist outside inline fragment selections and must be handled separately from type-specific fragment selections.

Applied to files:

• v2/pkg/engine/plan/node_selection_visitor.go

⏰ 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). (4)

• GitHub Check: Build and test (go 1.25 / ubuntu-latest)
• GitHub Check: Build and test (go 1.25 / windows-latest)
• GitHub Check: Build and test (go 1.25 / windows-latest)
• GitHub Check: Build and test (go 1.25 / ubuntu-latest)

🔇 Additional comments (7)

v2/pkg/engine/plan/node_selection_builder.go (2)

113-114: LGTM! Formatting improvement.

The function call reformatting improves readability by placing each argument on its own line.

---

130-133: LGTM! Comment improvements.

The updated comments provide clearer context about the purpose of the additional runs and the handling of rewritten fields.

v2/pkg/engine/plan/datasource_filter_visitor.go (1)

145-147: LGTM! Clear documentation.

The comment block effectively describes the purpose of the collectNodes method, explaining both the data collection and jump graph construction aspects.

v2/pkg/engine/plan/node_selection_visitor.go (2)

13-14: LGTM! Documentation improvement.

The comment updates improve consistency by using plural forms for "data sources" and "abstract selections."

---

223-223: LGTM! Enhanced debugging visibility.

Adding the requires flag to the debug output improves traceability when tracking field handling with @requires directives.

v2/pkg/engine/plan/planner.go (1)

147-147: LGTM! Debug formatting improvement.

The formatted debug header with underline improves visual separation in log output, making it easier to identify the planning visitor stage.

v2/pkg/engine/plan/visitor.go (1)

74-108: LGTM! Debug output formatting improved.

The formatting changes consistently remove spaces before colons in debug messages (e.g., "EnterField:" instead of "EnterField :"), improving debug output consistency across the planning visitor. These are cosmetic improvements with no functional impact.