Fix closure resolution in loop headers - #4232
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughWalkthroughExpression metadata now uses scope-aware keys and binding-based path resolutions. HIR records body and parameter-default contexts, while TIR, MIR, and LSP usage lookup consume the shared semantic-index APIs. Lambda, lexical-scoping, pattern-binding, and prompt-rendering coverage was added. ChangesScoped semantic-index contracts
Compiler metadata and lowering
Regression validation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant HIRBuilder
participant FileSemanticIndex
participant TIRBuilder
participant MIRLowering
participant LSPUsages
HIRBuilder->>FileSemanticIndex: record scoped expression keys and path resolutions
TIRBuilder->>FileSemanticIndex: query PathResolution by ExprMetadataKey
MIRLowering->>FileSemanticIndex: resolve binding identity and expression metadata
LSPUsages->>FileSemanticIndex: query nested lambda and default-expression paths
FileSemanticIndex-->>LSPUsages: return matching BindingId
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
⏭️ Performance benchmarks were skippedPerf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to To run them on this PR, do any of the following, then push a commit (or re-run CI):
|
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 `@baml_language/crates/baml_lsp2_actions/src/usages.rs`:
- Around line 212-216: Update the usage collector to derive and pass each
candidate expression’s owning ExprMetadataScope instead of always using
ExprMetadataScope::Body(enclosing_func_scope). Ensure nested closure expressions
resolve through their own body scope so closure parameters and locals are
included, and add a usages_at regression covering nested-closure references.
🪄 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 Plus
Run ID: d78f48ab-c034-4dbd-a3ea-c20186487a41
📒 Files selected for processing (7)
baml_language/crates/baml_compiler2_hir/src/builder.rsbaml_language/crates/baml_compiler2_hir/src/lib.rsbaml_language/crates/baml_compiler2_hir/src/semantic_index.rsbaml_language/crates/baml_compiler2_mir/src/lower.rsbaml_language/crates/baml_compiler2_tir/src/builder.rsbaml_language/crates/baml_lsp2_actions/src/usages.rsbaml_language/crates/baml_tests/tests/loop_header_closures.rs
Binary size checks passed✅ 7 passed
Generated by |
c27ef90 to
490db81
Compare
490db81 to
1d6e212
Compare
Issue Reference
B-532
Changes
Testing
PR Checklist
Summary by CodeRabbit
Bug Fixes
Tests
foriterables andwhileconditions.usages_atcoverage for references inside nested lambda closures.${ctx.output_format}interpolation companions.