Outline extractor execution#2729
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughTwo files in the ChangesOutline Extraction Implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2729 +/- ##
==========================================
+ Coverage 86.74% 86.90% +0.16%
==========================================
Files 121 121
Lines 20761 20985 +224
==========================================
+ Hits 18009 18238 +229
+ Misses 2752 2747 -5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/cli/src/outline.rs`:
- Line 271: The mpsc::channel() call creating the tx and rx pair is unbounded,
allowing workers to enqueue OutlineFile values faster than they can be consumed,
causing potential memory bloat on large trees. Replace the unbounded
mpsc::channel() with mpsc::channel(capacity) using an appropriate bounded
capacity value (e.g., 1000 or similar) to provide backpressure and prevent
unbounded queue growth. This ensures that when the queue reaches capacity,
senders will block until the receiver consumes items.
In `@crates/outline/src/default_rules/typescript.yml`:
- Around line 355-366: The ts-class-constructor rule hardcodes isPublic: true,
which incorrectly marks private and protected constructors as public. Replace
the hardcoded isPublic property with a modifier-based visibility check that
mirrors the pattern used for methods elsewhere in the file, specifically the
approach that examines the modifier context to determine if a constructor is
private, protected, or public. Also add test cases covering private and
protected constructor scenarios to ensure the modifier-based detection works
correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4f0c3b77-e62e-479d-a9f9-93844e99ba32
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
crates/cli/Cargo.tomlcrates/cli/src/lib.rscrates/cli/src/outline.rscrates/outline/src/combined_extractor.rscrates/outline/src/default_rule.rscrates/outline/src/default_rules/go.ymlcrates/outline/src/default_rules/java.ymlcrates/outline/src/default_rules/kotlin.ymlcrates/outline/src/default_rules/python.ymlcrates/outline/src/default_rules/swift.ymlcrates/outline/src/default_rules/typescript.ymlcrates/outline/src/extractor.rscrates/outline/src/lib.rscrates/outline/tests/common/mod.rscrates/outline/tests/jvm_swift_outline_rules.rscrates/outline/tests/python_go_outline_rules.rscrates/outline/tests/typescript_outline_rules.rs
HerringtonDarkholme
force-pushed
the
outline-extractor-execution
branch
from
June 19, 2026 14:41
0011822 to
f0a3408
Compare
| //! | ||
| //! This module only builds those indexes. It does not traverse the AST or | ||
| //! perform scoped member extraction. | ||
| //! Extraction uses explicit pre-order traversals instead of `find_all`. For each |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
gh issue create \
--repo "ast-grep/ast-grep" \
--title "TODO: Explore whether level-order (BFS) traversal is faster than pre-order in CombinedExtractors::extract" \
--body "## Background
In \`crates/outline/src/combined_extractor.rs\`, \`CombinedExtractors::extract\` currently uses an explicit pre-order (DFS) traversal via a stack to match top-level outline items and skip their descendants once matched.
## Task
Explore whether switching to a **level-order (BFS) traversal** would be faster in practice. Potential advantages of BFS:
- Outline items (functions, classes, etc.) tend to cluster near the top levels of the AST, so BFS might find matches earlier and avoid allocating deep stack frames.
- The \"skip descendants once matched\" semantics may integrate naturally with a queue-based approach.
Potential drawbacks:
- BFS requires a queue/deque, which may have higher allocation overhead for deeply nested or wide ASTs.
- Pre-order with early skip may already be near-optimal for typical AST shapes.
## Suggested investigation
- Benchmark both approaches on representative source files (Rust, TypeScript, Go) with the existing outline rules.
- Measure allocation count and throughput.
## References
- PR: https://github.com/ast-grep/ast-grep/pull/2729
- Comment: https://github.com/ast-grep/ast-grep/pull/2729#discussion_r3443167022
Requested by `@HerringtonDarkholme`." \
--assignee "HerringtonDarkholme"Length of output: 1554
@HerringtonDarkholme, I've logged the TODO as GitHub issue #2736: Explore whether level-order (BFS) traversal is faster than pre-order in CombinedExtractors::extract, assigned to you. It captures the current DFS stack-based approach, potential BFS trade-offs, and suggested benchmarking directions.
🧠 Learnings used
Learnt from: HerringtonDarkholme
Repo: ast-grep/ast-grep PR: 2484
File: crates/wasm/src/ts_types.rs:33-47
Timestamp: 2026-02-22T15:51:19.118Z
Learning: In Rust WASM targets (single-threaded async environment), thread_local! creates a per-thread singleton with no cross-thread shared mutable state, so data races in Rust terms do not occur. Since await points can interleave tasks, ensure library initialization is idempotent to remain safe across yields. When reviewing code, verify thread_local usage and idempotent initialization where applicable, and avoid relying on mutable global state across await boundaries.
|
|
||
| fn match_item<'tree, D: Doc>( | ||
| &self, | ||
| node: Node<'tree, D>, |
Member
Author
There was a problem hiding this comment.
too much clone, can you use reference here?
HerringtonDarkholme
force-pushed
the
outline-extractor-execution
branch
from
June 19, 2026 15:09
f0a3408 to
3c16ebc
Compare
| } | ||
| } | ||
|
|
||
| fn push_children<'tree, D: Doc>(stack: &mut Vec<Node<'tree, D>>, node: Node<'tree, D>) { |
Member
Author
There was a problem hiding this comment.
this is definitely performance killer lol
This was referenced Jun 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Bug Fixes
Improvements