feat(cli): simplify and clarify baml describe#3989
Conversation
Rework describe for coding-agent navigation based on benchmark traces: - add --grep semantic project search (term1 || term2 syntax), replacing the standalone grep command (module renamed to project_search) - support multi-symbol batching in one invocation - add --agent compact output mode - add --limit/--budget output bounding
|
@ashley-ha is attempting to deploy a commit to the Boundary Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthrough
ChangesDescribe and project search consolidation
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant ProjectSearch
participant DescriptionBuilder
participant ViewRenderer
CLI->>ProjectSearch: Search terms and filters
ProjectSearch-->>CLI: Semantic descriptions or text matches
CLI->>DescriptionBuilder: Build selected description options
DescriptionBuilder-->>CLI: Symbol descriptions
CLI->>ViewRenderer: Render selected view within max-lines
ViewRenderer-->>CLI: Text or JSON output
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
baml_language/crates/baml_lsp2_actions/src/testing.rs (1)
313-342: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDoc comments on these helpers still read
Rungrep()``, but they now delegate toproject_search::search_project/`list_symbols`. Cosmetic only.🤖 Prompt for 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. In `@baml_language/crates/baml_lsp2_actions/src/testing.rs` around lines 313 - 342, Update the doc comments for search_project and grep_case_insensitive to describe project_search::search_project rather than grep(), and update the list_symbols and list_symbols_compiler2_visible comments to accurately describe project_search::list_symbols. Keep the helper behavior unchanged.
🤖 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.
Nitpick comments:
In `@baml_language/crates/baml_lsp2_actions/src/testing.rs`:
- Around line 313-342: Update the doc comments for search_project and
grep_case_insensitive to describe project_search::search_project rather than
grep(), and update the list_symbols and list_symbols_compiler2_visible comments
to accurately describe project_search::list_symbols. Keep the helper behavior
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: fb4e5c61-bc04-44d8-8f1f-f1efe576300d
⛔ Files ignored due to path filters (6)
baml_language/crates/baml_lsp2_actions/src/snapshots/baml_lsp2_actions__grep_tests__grep_case_insensitive_text_search.snapis excluded by!**/*.snapbaml_language/crates/baml_lsp2_actions/src/snapshots/baml_lsp2_actions__project_search_tests__case_insensitive_text_search.snapis excluded by!**/*.snapbaml_language/crates/baml_lsp2_actions/src/snapshots/baml_lsp2_actions__project_search_tests__enum_symbol_search.snapis excluded by!**/*.snapbaml_language/crates/baml_lsp2_actions/src/snapshots/baml_lsp2_actions__project_search_tests__list_symbols_snapshot.snapis excluded by!**/*.snapbaml_language/crates/baml_lsp2_actions/src/snapshots/baml_lsp2_actions__project_search_tests__semantic_result_snapshot.snapis excluded by!**/*.snapbaml_language/crates/baml_lsp2_actions/src/snapshots/baml_lsp2_actions__project_search_tests__text_search_with_matches.snapis excluded by!**/*.snap
📒 Files selected for processing (11)
baml_language/crates/baml_cli/src/commands.rsbaml_language/crates/baml_cli/src/describe_command.rsbaml_language/crates/baml_cli/src/describe_command_tests.rsbaml_language/crates/baml_cli/src/grep_command.rsbaml_language/crates/baml_cli/src/lib.rsbaml_language/crates/baml_cli/tests/exit_code_e2e.rsbaml_language/crates/baml_lsp2_actions/src/describe.rsbaml_language/crates/baml_lsp2_actions/src/lib.rsbaml_language/crates/baml_lsp2_actions/src/project_search.rsbaml_language/crates/baml_lsp2_actions/src/project_search_tests.rsbaml_language/crates/baml_lsp2_actions/src/testing.rs
💤 Files with no reviewable changes (3)
- baml_language/crates/baml_cli/src/lib.rs
- baml_language/crates/baml_cli/src/grep_command.rs
- baml_language/crates/baml_cli/src/commands.rs
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@docs/benchmarks/baml-describe-agent-navigation/README.md`:
- Around line 35-44: Add a clear condition-name mapping note in the benchmark
README connecting the table labels “natural grep/read,” “bounded grep control,”
and “bounded `describe` hybrid” to the transcript names “natural,”
“guided-grep,” and “guided-hybrid-slim.” Apply the mapping consistently across
all three benchmark tables and transcript link descriptions so readers can
cross-reference results without inference.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 24af38f6-d2e0-4275-ad46-d3f3c6e06888
📒 Files selected for processing (5)
baml_language/crates/baml_lsp2_actions/src/testing.rsdocs/benchmarks/baml-describe-agent-navigation/README.mddocs/benchmarks/baml-describe-agent-navigation/transcripts/claude-trophy-flow.mddocs/benchmarks/baml-describe-agent-navigation/transcripts/codex-edit-file-transfer.mddocs/benchmarks/baml-describe-agent-navigation/transcripts/codex-trophy-flow.md
🚧 Files skipped from review as they are similar to previous changes (1)
- baml_language/crates/baml_lsp2_actions/src/testing.rs
There was a problem hiding this comment.
🧹 Nitpick comments (1)
baml_language/crates/baml_cli/src/describe_command.rs (1)
683-697: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
checked_div+unwrap_or(0)is defensive but unreachable.
descriptions.is_empty()is guarded at line 675 (returnsExitCode::Other), sodescriptions.len() >= 1here.checked_divwill never returnNone, makingunwrap_or(0)dead code. Considersaturating_div(descriptions.len())for clarity, or add a comment noting the guard.🤖 Prompt for 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. In `@baml_language/crates/baml_cli/src/describe_command.rs` around lines 683 - 697, In the result formatting logic around batch_description_to_json, remove the unreachable checked_div(...).unwrap_or(0) fallback and use saturating_div with descriptions.len() after the existing empty-descriptions guard. Preserve the current max-line calculation and per-result mapping behavior.
🤖 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.
Nitpick comments:
In `@baml_language/crates/baml_cli/src/describe_command.rs`:
- Around line 683-697: In the result formatting logic around
batch_description_to_json, remove the unreachable checked_div(...).unwrap_or(0)
fallback and use saturating_div with descriptions.len() after the existing
empty-descriptions guard. Preserve the current max-line calculation and
per-result mapping behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 47107214-acb9-4922-9128-155174947c9d
⛔ Files ignored due to path filters (3)
baml_language/crates/baml_cli/src/snapshots/baml_cli__describe_command_tests__render_describe_alias_string.snapis excluded by!**/*.snapbaml_language/crates/baml_cli/src/snapshots/baml_cli__describe_command_tests__render_describe_builtin_item_by_definition.snapis excluded by!**/*.snapbaml_language/crates/baml_cli/src/snapshots/baml_cli__describe_command_tests__render_describe_builtin_string.snapis excluded by!**/*.snap
📒 Files selected for processing (5)
baml_language/TEST_INSTRUCTIONS.mdbaml_language/crates/baml_cli/src/describe_command.rsbaml_language/crates/baml_cli/src/describe_command_tests.rsbaml_language/crates/baml_cli/tests/exit_code_e2e.rsdocs/benchmarks/baml-describe-agent-navigation/README.md
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/benchmarks/baml-describe-agent-navigation/README.md
- baml_language/crates/baml_cli/src/describe_command_tests.rs
ashley-ha
changed the title
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
🧹 Nitpick comments (3)
baml_language/crates/baml_cli/src/describe_command.rs (1)
569-591: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
source_candidate_rangesreconstructs every symbol's full body just to readitem_range.
describe_listing_entry(..., DescribeOptions::source())is called once per top-level entry solely to compute(start_line, end_line)fromdescription.item_range.item_rangeis already computed unconditionally indescribe_top_levelbefore any options-gated field, butDescribeOptions::source()also setsfull_body: true, which forces aclean_body_sourcereconstruction (CST comment-stripping over the whole item) for every entry — work that's discarded immediately. This runs on every--searchinvocation across all matching top-level entries, which is now the primary discovery path.⚡ Add a minimal options preset for range-only lookups
+ pub const fn identity() -> Self { + Self { + shape: false, + docstring: false, + dependencies: false, + implementation_dependencies: false, + resolved_type: false, + references: false, + methods: false, + full_body: false, + } + }let description = describe_listing_entry( db, files, entry, - baml_lsp2_actions::DescribeOptions::source(), + baml_lsp2_actions::DescribeOptions::identity(), )?;🤖 Prompt for 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. In `@baml_language/crates/baml_cli/src/describe_command.rs` around lines 569 - 591, Update source_candidate_ranges to use a minimal DescribeOptions preset that requests the item range without enabling full_body or other source rendering fields. Add or reuse a range-only option near DescribeOptions and pass it to describe_listing_entry, preserving the existing definition_line_range calculation and returned entry ranges.baml_language/crates/baml_lsp2_actions/src/describe.rs (2)
1193-1261: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
describe_class_methodalways computes dependencies/implementation_dependencies, unlikedescribe_top_level.
describe_top_levelgates the new dependency work behindDescribeOptions(lines 419-430), butdescribe_class_method— anddescribe_item_member, which calls it and is invoked directly from the CLI'sResolvedTarget::Memberarm — take noDescribeOptionsparameter at all. Every method/member describe (even--view source) unconditionally runscollect_ty_depsplus the fullfind_implementation_dependenciesscan, regardless of what the caller actually needs.♻️ Thread DescribeOptions through the member path
-fn describe_class_method( - db: &dyn Db, - files: &[SourceFile], - class_loc: baml_compiler2_hir::loc::ClassLoc<'_>, - member_name: &str, -) -> Option<SymbolDescription> { +fn describe_class_method( + db: &dyn Db, + files: &[SourceFile], + class_loc: baml_compiler2_hir::loc::ClassLoc<'_>, + member_name: &str, + options: DescribeOptions, +) -> Option<SymbolDescription> { ... - let mut dependencies = Vec::new(); - let mut seen = std::collections::HashSet::from([...]); - if let Some(ef) = ef { - ... - } - ... - let implementation_dependencies = - find_implementation_dependencies(db, file, m.span, &sym, &dependencies); + let mut dependencies = Vec::new(); + if options.dependencies { + let mut seen = std::collections::HashSet::from([...]); + if let Some(ef) = ef { + ... + } + } + let implementation_dependencies = if options.implementation_dependencies { + find_implementation_dependencies(db, file, m.span, &sym, &dependencies) + } else { + Vec::new() + };
describe_item_memberwould need the sameoptions: DescribeOptionsparameter, and its one call site indescribe_command.rs'sResolvedTarget::Memberarm should passself.describe_options().🤖 Prompt for 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. In `@baml_language/crates/baml_lsp2_actions/src/describe.rs` around lines 1193 - 1261, Thread DescribeOptions through describe_item_member and describe_class_method, updating the ResolvedTarget::Member call in describe_command.rs to pass self.describe_options(). Gate collect_ty_deps and find_implementation_dependencies behind the same options used by describe_top_level, so source-only member descriptions skip dependency computation while dependency-enabled requests retain the existing behavior.
1621-1666: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
find_implementation_dependencieswalks the entire file's token stream on every call.
tree.descendants_with_tokens()iterates every token in the whole file; theitem_rangebounds are only applied as a filter inside the loop, so this is O(file size) per describe invocation regardless of how small the described symbol is. This function is called once perdescribe_top_level(for any symbol withimplementation_dependenciesenabled — thedependenciesview and JSON/default output) and once perdescribe_class_methodcall (currently unconditional, see the sibling comment), so for thedependenciesview or JSON output on a large file this repeats a full-file scan per symbol described.rowan's
SyntaxNodesupports finding the node/token covering a given range in O(log N) (e.g.covering_element), so the walk can be scoped to the item's own subtree instead of the whole tree — similar to howextract_docstring(line ~1507) already locates the item node viatoken_at_offset+parent_ancestors().♻️ Scope the token walk to the item's subtree
- let tree = baml_compiler_parser::syntax_tree(db, file); + let tree = baml_compiler_parser::syntax_tree(db, file); + let root: rowan::NodeOrToken<_, _> = tree.covering_element(item_range); let mut seen = contract_dependencies...; ... - for node_or_token in tree.descendants_with_tokens() { + let tokens: Vec<_> = match root { + rowan::NodeOrToken::Node(node) => node.descendants_with_tokens().collect(), + rowan::NodeOrToken::Token(token) => vec![rowan::NodeOrToken::Token(token)], + }; + for node_or_token in tokens { let rowan::NodeOrToken::Token(token) = node_or_token else { continue }; if token.kind() != SyntaxKind::WORD && !token.kind().is_keyword() { continue; } - let token_range = token.text_range(); - if token_range.start() < item_range.start() || token_range.end() > item_range.end() { - continue; - } + let token_range = token.text_range(); ... }Please confirm the exact scoping API available in the rowan version pinned by this workspace (e.g.
covering_elementvs. a differently-named helper) before applying.🤖 Prompt for 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. In `@baml_language/crates/baml_lsp2_actions/src/describe.rs` around lines 1621 - 1666, Update find_implementation_dependencies to locate the syntax node covering item_range using the rowan API available in this workspace, first confirming whether covering_element or its version-specific equivalent is supported. Walk descendants_with_tokens only from that item subtree, preserving the existing token filtering, definition lookup, and deduplication behavior.
🤖 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.
Nitpick comments:
In `@baml_language/crates/baml_cli/src/describe_command.rs`:
- Around line 569-591: Update source_candidate_ranges to use a minimal
DescribeOptions preset that requests the item range without enabling full_body
or other source rendering fields. Add or reuse a range-only option near
DescribeOptions and pass it to describe_listing_entry, preserving the existing
definition_line_range calculation and returned entry ranges.
In `@baml_language/crates/baml_lsp2_actions/src/describe.rs`:
- Around line 1193-1261: Thread DescribeOptions through describe_item_member and
describe_class_method, updating the ResolvedTarget::Member call in
describe_command.rs to pass self.describe_options(). Gate collect_ty_deps and
find_implementation_dependencies behind the same options used by
describe_top_level, so source-only member descriptions skip dependency
computation while dependency-enabled requests retain the existing behavior.
- Around line 1621-1666: Update find_implementation_dependencies to locate the
syntax node covering item_range using the rowan API available in this workspace,
first confirming whether covering_element or its version-specific equivalent is
supported. Walk descendants_with_tokens only from that item subtree, preserving
the existing token filtering, definition lookup, and deduplication behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: cede6325-714a-40ab-8b17-174a99cdcb2b
📒 Files selected for processing (9)
baml_language/TEST_INSTRUCTIONS.mdbaml_language/crates/baml_cli/src/commands.rsbaml_language/crates/baml_cli/src/describe_command.rsbaml_language/crates/baml_cli/src/describe_command_tests.rsbaml_language/crates/baml_cli/tests/exit_code_e2e.rsbaml_language/crates/baml_lsp2_actions/src/describe.rsbaml_language/crates/baml_lsp2_actions/src/describe_tests.rsbaml_language/crates/baml_lsp2_actions/src/project_search.rsdocs/benchmarks/baml-describe-agent-navigation/README.md
🚧 Files skipped from review as they are similar to previous changes (4)
- baml_language/TEST_INSTRUCTIONS.md
- docs/benchmarks/baml-describe-agent-navigation/README.md
- baml_language/crates/baml_cli/src/describe_command_tests.rs
- baml_language/crates/baml_lsp2_actions/src/project_search.rs
1 participant
Problem
baml describehad accumulated overlapping discovery, rendering, and agent-specific flags. The CLI made users choose a consumer-orientedcompactoutput mode even though color, serialization, search confidence, relationship direction, and line budgeting are separate concerns.That made the command harder to predict:
impactand inward dependencies were easy to conflatecompactchanged workflow behavior as well as presentationFix
The command now centers on three workflows:
Command surface
--search--kindand--filenarrow discovery--view overview|source|usage|impact|dependenciesselects relationship direction--max-linesis the single soft output budget--output text|jsoncontrols serialization only--color auto|always|nevercontrols styling independentlyRendering and navigation
Breaking changes
This branch is unreleased, so the PR makes a clean break:
--grep,--symbols,--limit,--ignore-case,--agent,--json,--budget, and--depth--output compact; valid output values are nowtextandjson--colorand--featurestop-level-onlyEvidence
The guided
describedependency benchmark coveredparse_trophy,agent.tool_edit_file, androot.cc.Parser.parse_stmt:describeA broader multiview run tested eight ordinary user intents:
describeThe benchmark work also exposed and fixed two correctness issues:
references (0)sectionEvidence packages:
Real-project verification
Exercised against
tools/agent-tries-baml:Verified that terminal text is colored, JSON remains valid and ANSI-free even with
--color always, multi-symbol text stays bounded, and removedcompactinvocations fail with a clear valid-values message.Testing
cargo fmt --manifest-path baml_language/Cargo.toml --check -p baml_clicargo test --manifest-path baml_language/Cargo.toml -p baml_clitools/agent-tries-bamlgit diff --checkThe three Vercel authorization checks shown on this PR are repository authorization failures rather than failures in this change.