fix(module_graph): pass correct type count to type limit check by chikingsley · Pull Request #8618 · biomejs/biome

chikingsley · 2025-12-29T16:41:45Z

Summary

The type limit check in flatten_all() was incorrectly passing the loop index i to reached_too_many_types() instead of the actual type count self.types.len().

This meant the limit check would only trigger after 200,000 loop iterations, rather than when the type store actually exceeded 200,000 types. During flattening, types can grow exponentially without the safety check ever triggering, causing 100% CPU usage.

This bug was introduced in #8511 when these files were created.

Fixes #7020

Also addresses the downstream issue reported at haydenbleasel/ultracite#464

AI Disclosure

This PR was written primarily by Claude Code.

Test Plan

Existing tests pass (cargo test -p biome_module_graph - 41/41 passing)
just f (format) passes
just l (lint) passes

Docs

N/A - internal bug fix, no user-facing documentation changes required.

The type limit check in `flatten_all()` was incorrectly passing the loop index `i` to `reached_too_many_types()` instead of the actual type count `self.types.len()`. This meant the limit check would only trigger after 200,000 loop iterations, rather than when the type store actually exceeded 200,000 types. During flattening, types can grow exponentially without the safety check ever triggering, causing 100% CPU usage. Fixes biomejs#7020

changeset-bot · 2025-12-29T16:41:49Z

🦋 Changeset detected

Latest commit: 91714de

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 13 packages

Name	Type
@biomejs/biome	Patch
@biomejs/cli-win32-x64	Patch
@biomejs/cli-win32-arm64	Patch
@biomejs/cli-darwin-x64	Patch
@biomejs/cli-darwin-arm64	Patch
@biomejs/cli-linux-x64	Patch
@biomejs/cli-linux-arm64	Patch
@biomejs/cli-linux-x64-musl	Patch
@biomejs/cli-linux-arm64-musl	Patch
@biomejs/wasm-web	Patch
@biomejs/wasm-bundler	Patch
@biomejs/wasm-nodejs	Patch
@biomejs/backend-jsonrpc	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

coderabbitai · 2025-12-29T16:43:36Z

Walkthrough

This patch changes the type-limit check performed during type flattening from using the current loop index to using the actual total number of types (self.types.len()). The change is applied in the module graph collector and resolver where an early diagnostic return is triggered. A test module was added to validate behaviour below, at, and above the MAX_NUM_TYPES threshold and to ensure the limit is detected when types grow during flattening.

Suggested labels

A-Type-Inference

Suggested reviewers

ematipico
dyc3

Pre-merge checks and finishing touches

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: fixing the type limit check to use the correct type count instead of loop index.
Description check	✅ Passed	The description explains the bug, its impact (100% CPU usage), the fix, what issue it addresses, and provides test coverage details.
Linked Issues check	✅ Passed	The PR fixes issue #7020 by correcting the type limit check to use actual type count, preventing unbounded growth and CPU exhaustion during flattening.
Out of Scope Changes check	✅ Passed	All changes directly address the type limit check bug: fixes in collector.rs, module_resolver.rs, test additions in utils.rs, and a changelog entry—no out-of-scope modifications.

✨ Finishing touches

📝 Generate docstrings

📜 Recent 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 b772957 and 91714de.

📒 Files selected for processing (1)

crates/biome_module_graph/src/js_module_info/utils.rs

🧰 Additional context used

📓 Path-based instructions (1)

**/*.rs

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*.rs: Use inline rustdoc documentation for rules, assists, and their options
Use the dbg!() macro for debugging output in Rust tests and code
Use doc tests (doctest) format with code blocks in rustdoc comments; ensure assertions pass in tests

Files:

crates/biome_module_graph/src/js_module_info/utils.rs

🧠 Learnings (10)

📚 Learning: 2025-11-24T18:05:42.356Z

Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Use `TypeReference` instead of `Arc` for types that reference other types to avoid stale cache issues when modules are replaced

Applied to files:

crates/biome_module_graph/src/js_module_info/utils.rs

📚 Learning: 2025-11-24T18:05:20.371Z

Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
Learning: Applies to crates/biome_formatter/**/biome_*_formatter/tests/spec_tests.rs : Use the `tests_macros::gen_tests!` macro in `spec_tests.rs` to generate test functions for each specification file matching the pattern `tests/specs/<language>/**/*.<ext>`

Applied to files:

crates/biome_module_graph/src/js_module_info/utils.rs

📚 Learning: 2025-11-24T18:05:42.356Z

Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Distinguish between `TypeData::Unknown` and `TypeData::UnknownKeyword` to measure inference effectiveness versus explicit user-provided unknown types

Applied to files:

crates/biome_module_graph/src/js_module_info/utils.rs

📚 Learning: 2025-11-24T18:05:42.356Z

Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/flattening.rs : Implement type flattening to simplify `TypeofExpression` variants once all component types are resolved

Applied to files:

crates/biome_module_graph/src/js_module_info/utils.rs

📚 Learning: 2025-11-24T18:06:12.048Z

Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:12.048Z
Learning: Applies to crates/biome_service/src/workspace/watcher.tests.rs : Implement watcher tests for workspace methods in watcher.tests.rs and end-to-end tests in LSP tests

Applied to files:

crates/biome_module_graph/src/js_module_info/utils.rs

📚 Learning: 2025-11-24T18:05:42.356Z

Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/js_module_info/collector.rs : Implement module-level (thin) inference to resolve `TypeReference::Qualifier` variants by looking up declarations in module scopes and handling import statements

Applied to files:

crates/biome_module_graph/src/js_module_info/utils.rs

📚 Learning: 2025-08-20T16:24:59.781Z

Learnt from: arendjr
Repo: biomejs/biome PR: 7266
File: crates/biome_js_type_info/src/type.rs:94-102
Timestamp: 2025-08-20T16:24:59.781Z
Learning: In crates/biome_js_type_info/src/type.rs, the flattened_union_variants() method returns TypeReference instances that already have the correct module IDs applied to them. These references should be used directly with resolver.resolve_reference() without applying additional module ID transformations, as variant references may originate from nested unions in different modules.

Applied to files:

crates/biome_module_graph/src/js_module_info/utils.rs

📚 Learning: 2025-11-24T18:05:42.356Z

Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Store type data in linear vectors instead of using recursive data structures with `Arc` for improved data locality and performance

Applied to files:

crates/biome_module_graph/src/js_module_info/utils.rs

📚 Learning: 2025-11-24T18:05:42.356Z

Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/scoped_resolver.rs : Implement full inference to resolve `TypeReference::Import` variants across the entire module graph

Applied to files:

crates/biome_module_graph/src/js_module_info/utils.rs

📚 Learning: 2025-11-24T18:05:42.356Z

Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Use `TypeData::Unknown` to indicate when type inference falls short or is not implemented

Applied to files:

crates/biome_module_graph/src/js_module_info/utils.rs

🔇 Additional comments (4)

crates/biome_module_graph/src/js_module_info/utils.rs (4)

19-38: Comprehensive boundary testing.

The three tests thoroughly validate the threshold behaviour at key boundaries. Logic is correct.

48-73: Excellent bug demonstration.

This test clearly illustrates the original defect: the loop index i (0..99) never reaches MAX_NUM_TYPES, even when the actual type count would have exploded. The unused types_len variable effectively documents what the real code would experience. Well-commented and pedagogical.

75-104: Fix validation is sound.

The test correctly simulates type growth mid-flattening and verifies that checking against types.len() triggers the limit immediately. The logic is clear and the assertions are precise.

106-146: Comprehensive infinite loop prevention test.

This test validates the CPU exhaustion fix with a realistic growth scenario (10,000 types per iteration). The mathematics check out: the limit triggers at iteration ~20 when types reach ~200,100, well before the iteration cap. The intentionally permissive loop condition (while i < types_len) effectively demonstrates what would happen without the fix—an unbounded loop as types grow faster than the counter.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

ematipico

I might not understand the fix, however you should add a new test that proves we fixed something

Add tests to verify correct behavior at boundary conditions: - Below limit returns Ok - At limit returns Err - Above limit returns Err 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

ematipico

Those tests aren't testing the change @chikingsley

…rect Add three new tests that simulate the flatten_all loop behavior to prove why passing `self.types.len()` instead of loop index `i` fixes the CPU exhaustion bug: 1. test_flatten_loop_with_loop_index_misses_limit - Shows the OLD buggy behavior: using loop index `i` never triggers the limit even when types grow to millions during flattening. 2. test_flatten_loop_with_types_len_catches_limit - Shows the NEW fixed behavior: using `types.len()` catches the limit when types exceed the threshold. 3. test_fix_prevents_infinite_loop_scenario - Demonstrates that the fix prevents CPU exhaustion by exiting early when types grow unbounded. These tests document the bug and prove the semantic correctness of the fix without requiring impractical test fixtures with 200,000+ types. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

chikingsley · 2025-12-30T04:19:51Z

Hey @ematipico thanks for the feedback on this.

I dug into this a bit deeper today - I thought the issue had to do with the type flattening but when pushing my local setup, it was still running out of memory after this change. for some reason, the linter is crawling everything including node_modules and crushing my ram in the process (is my guess).

So it's something more complicated. I'm going to close this. Thanks for looking, sorry for any wasted time.

Cheers

chikingsley closed this Dec 29, 2025

github-actions bot added the A-Project Area: project label Dec 29, 2025

chikingsley reopened this Dec 29, 2025

ematipico requested changes Dec 29, 2025

View reviewed changes

chikingsley closed this Dec 30, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(module_graph): pass correct type count to type limit check#8618

fix(module_graph): pass correct type count to type limit check#8618
chikingsley wants to merge 3 commits intobiomejs:mainfrom
chikingsley:claude/fix-cpu-usage-BfGie

chikingsley commented Dec 29, 2025

Uh oh!

changeset-bot bot commented Dec 29, 2025 •

edited

Loading

Uh oh!

coderabbitai bot commented Dec 29, 2025 •

edited

Loading

Uh oh!

ematipico left a comment

Uh oh!

ematipico left a comment

Uh oh!

chikingsley commented Dec 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

chikingsley commented Dec 29, 2025

Summary

AI Disclosure

Test Plan

Docs

Uh oh!

changeset-bot bot commented Dec 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

coderabbitai bot commented Dec 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Suggested labels

Suggested reviewers

Pre-merge checks and finishing touches

Uh oh!

ematipico left a comment

Choose a reason for hiding this comment

Uh oh!

ematipico left a comment

Choose a reason for hiding this comment

Uh oh!

chikingsley commented Dec 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

changeset-bot bot commented Dec 29, 2025 •

edited

Loading

coderabbitai bot commented Dec 29, 2025 •

edited

Loading