Skip to content

Conversation

gabritto
Copy link
Member

We were doing an infinite BFS in the project collection builder while looking for a tsconfig for a given file. The loop happened because the BFS node type included a pointer to a logger, which changed every time we computed the neighbors of a node, and so no node was ever considered as already visited, even if we had already visited its corresponding tsconfig.

@Copilot Copilot AI review requested due to automatic review settings August 29, 2025 17:59
Copy link
Contributor

@Copilot Copilot AI left a comment

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 fixes an infinite loop bug in the breadth-first search (BFS) used during TypeScript config file lookup. The issue occurred because the searchNode type included a logger pointer that was different for each node computation, preventing the visited node tracking from working correctly.

Key changes:

  • Separated the node identity from the logger by introducing a searchNodeKey type containing only the essential fields (configFileName and loadKind)
  • Updated the BFS algorithm to use a generic approach with separate key and node types
  • Added a regression test to verify that project lookup terminates properly

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
internal/core/bfs.go Modified BFS algorithm to support separate key and node types, preventing identity issues with varying logger pointers
internal/core/bfs_test.go Updated tests to use the new BFS signature with explicit key types and added Key() method to test node type
internal/project/projectcollectionbuilder.go Introduced searchNodeKey type and updated BFS calls to use the new signature for proper node deduplication
internal/project/projectcollection.go Updated BFS call signature to match new generic parameters
internal/project/project.go Added Key() method to Project type for BFS compatibility
internal/project/projectcollectionbuilder_test.go Added regression test with circular project references to verify termination

Copy link
Member

@jakebailey jakebailey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine to me?

@gabritto gabritto added this pull request to the merge queue Sep 2, 2025
Merged via the queue into main with commit d28e000 Sep 2, 2025
22 checks passed
@gabritto gabritto deleted the gabritto/bfsloop branch September 2, 2025 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants