Type-check a Session assignment against its binding - #4531
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
⏭️ 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):
|
|
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 selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughSession assignments now use uniquely named typed temporary locals. Rejected assignments match ordinary BAML behavior and preserve bindings. Regression tests cover shadowing, compound assignments, numeric operands, structured values, and same-type updates. ChangesSession assignment semantics
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🟡 Moderate · up to This change updates Rust runtime compilation behavior, and the required library test command has not been explicitly confirmed; merge should wait for that check or explicit owner acceptance. Sequence Diagram(s)sequenceDiagram
participant reflect.Session
participant runtime_compile
participant GlobalBinding
reflect.Session->>runtime_compile: lower visible assignment
runtime_compile->>runtime_compile: validate through typed temporary
runtime_compile->>GlobalBinding: commit valid assignment
runtime_compile-->>reflect.Session: return runtime result or failure
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 |
Binary size checks passed✅ 7 passed
Generated by |
|
Review round applied — blocker fixed, both hardenings in, changelog corrected off a measurement. Pushed as Blocker — the synthesized local could collide with a user bindingReproduced against this branch before fixing it. A user binding mints
Fixed as suggested: the local is now Non-blocking, same roundChangelog — measured rather than assumed. I ran the Both test hardenings. Span remapping filed as MIG_BRIEF Fix 12: session diagnostics carry generated-module GateFull pinned gate on the final tree: 3,864/3,864 passed, 24 skipped, no unreferenced |
In a Session (the notebook), assigning to a variable skipped type checking entirely. This compiled:
In ordinary BAML,
n = "seven"on an int is a compile error. Now sessions behave the same way:How
Instead of adding a second checker, the session rewrite now emits the assignment as an assignment (
let __target = <the binding>; __target = value), so the ordinary checker runs and the error message is the ordinary one by construction — same code, same wording, byte for byte (pinned by tests that compile the same program both ways and compare).Notes
n += 1.5on an int compiles (and panics at runtime — a pre-existing ordinary-code hole), whilen += (1.5)is a compile error. Sessions now match both behaviors exactly, including the hole. Before this PR, a session'sn += 1.5produced 6.5 by accident; now it lands where ordinary code lands.__baml_assign_...) that no user binding can ever collide with — during review we found that a user variable literally namedtarget_1could silently capture the old name and turn an assignment into a no-op.