Repository navigation
refactor(mir): remove dead visualization nodes from MIR - #4633
Conversation
The MIR still carried `StatementKind::VizEnter`/`VizExit`, `VizNode`, `VizNodeType`, `MirFunctionBody::viz_nodes` and `MirBuilder::add_viz_node`, left over from the original playground animation scheme that inserted viz opcodes around control flow. Nothing has produced these since the `//# header` lowering was removed in #3247, and the emitter has treated them as no-ops since the VM opcode was dropped in #3616. The live playground builds its graph from the AST (`baml_compiler2_visualization`) and animates nodes from the run event stream, so the MIR surface is unused. Delete the variants, types, field and builder helper, and the pass-through match arms and test struct literals that referenced them. No snapshot references `viz_enter`/`viz_exit`, so output is unchanged. Claude-Session: https://claude.ai/code/session_01Qai5VpnzsTj8SxtmDXB9Tz
|
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 (9)
💤 Files with no reviewable changes (6)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe MIR visualization node data and ChangesMIR visualization removal
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change removes unused visualization data and no-op handling from MIR without changing runtime output or supported behavior. No actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
Summary
The MIR still carried the remnants of the original playground animation scheme, where viz opcodes were inserted around every control-flow statement:
StatementKind::VizEnter/StatementKind::VizExitVizNode,VizNodeTypeMirFunctionBody::viz_nodesMirBuilder::add_viz_node(zero callers)None of it is used anymore:
//# headerlowering inlower.rs) were removed in paulo/rust functions 2 #3247.VizNodeMetawere dropped in refactor(tracing): trim the event-stream surface for a fresh start #3616; since thenemit.rshas treated both statements as explicit no-ops.bex_vm,bex_vm_types,bex_engine,bex_events,bex_projectandbridge_wasmhave zero viz references.typescript2/pkg-playground) builds its graph from the AST viabaml_compiler2_visualizationand derives node run-state from theRunPatch/RunSnapshotevent stream, not from bytecode.This PR deletes the types, variants, field and builder helper, plus the ~20 pass-through match arms and test struct literals that referenced them. Nothing under
engine/ortypescript/is touched — those trees are legacy and carry their own complete viz system.No
.snap,.mdor TS file referencedviz_enter/viz_exit/VizNode, so output is unchanged. Net −115/+7.Test plan
cargo fmtwith repo configcargo clippy --workspace --all-targets -- -D warnings(clean)cargo test --lib— green forbaml_tests(954),baml_compiler2_*,baml_db,baml_ide,bex_vm,bex_engine,bex_project; only failures locally are environment-only (baml_bridgeneeds a prebuiltlibbridge_cffi.dylib,sdk_test_*need external toolchains, onebex_prof_storeStreamInUseflake that passes in isolation)https://claude.ai/code/session_01Qai5VpnzsTj8SxtmDXB9Tz
Summary by CodeRabbit