Skip to content

fix(cache): reject stale object operands without panicking - #4931

Closed
sxlijin wants to merge 1 commit into
canaryfrom
sxlijin/b-1643-stale-cache-panic
Closed

sxlijin wants to merge 1 commit into
canaryfrom
sxlijin/b-1643-stale-cache-panic

Conversation

@sxlijin

@sxlijin sxlijin commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • replace unchecked object-pool indexing during compilation-unit decomposition with typed LoweringError::Internal failures
  • let the existing cache-reuse fallback treat stale or incompatible cached operands as a miss and perform a fresh compile
  • cover both the compiler boundary and the CLI cache path, including a length-changing source edit that remains check-clean

Context

Fixes B-1643.

The reported reproduction used a cache created by an earlier local compiler build, made a length-changing BAML edit, then observed baml test panic in compilation-unit decomposition while baml check stayed green. Clearing .baml caches made the test pass.

PR #4801 already made compiler identity content-addressed and added the general reuse-to-fresh fallback, and PR #4804 moved this path into the current multi-root compiler structure. One unchecked object operand lookup remained in that path, so malformed or incompatible cached units could panic before the fallback received an error.

Reproduction and verification

The new CLI regression test compiles and stores a multi-file project, makes a length-changing source edit, poisons a reused unit with an out-of-range object operand to model an incompatible same-version artifact, verifies diagnostics stay clean, and asserts cache reuse falls back to byte-identical fresh output without panicking. The focused compiler regression first reproduced the raw out-of-bounds panic at the historical lookup and now verifies it returns a typed internal error.

  • cargo test -p baml_compiler2_emit
  • cargo test -p bex_cache
  • cargo test -p baml_cli bytecode_cache::tests
  • cargo clippy -p baml_compiler2_emit -p baml_cli --all-targets -- -D warnings
  • cargo fmt --package baml_compiler2_emit --package baml_cli --check

Summary by CodeRabbit

  • Bug Fixes
    • Compilation now handles invalid references and incompatible cached data without crashing, and produces the same output as a fresh compile.

@vercel

vercel Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
developer-docs Ready Ready Preview Sep 22, 2026 11:22pm UTC

Request Review

@coderabbitai

coderabbitai Bot commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 382c3d97-88bc-48c8-9f5c-b86e460edbfd

📥 Commits

Reviewing files that changed from the base of the PR and between cd4d161 and a371be0.

📒 Files selected for processing (2)
  • baml_language/crates/baml_cli/src/bytecode_cache.rs
  • baml_language/crates/baml_compiler2_emit/src/lib.rs

Included review availability: Your plan provides up to 10 included reviews per hour; 4 remain after this review.


📝 Walkthrough

Walkthrough

The emitter now returns an internal error for out-of-range object operands instead of panicking. A cache regression test checks that a poisoned reused unit falls back to fresh compilation and produces byte-identical output.

Changes

Cache safety

Layer / File(s) Summary
Operand bounds validation
baml_language/crates/baml_compiler2_emit/src/lib.rs
Object operand lookups return an internal error when the target index exceeds the pool length. A test checks that object_symbol rejects an out-of-range index without panicking.
Incompatible cache fallback
baml_language/crates/baml_cli/src/bytecode_cache.rs
A regression test poisons an object operand in a reused unit and checks that compilation falls back to fresh units with byte-identical output.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: 2kai2kai2

Merge Risk: ⚪ Minimal · up to a371b

The stale-cache path now returns a recoverable error and is covered by a fallback-to-fresh-compilation regression test. No remaining merge-blocking risk is identified.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: reject stale object operands without panicking during cache reuse.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

A rabbit checks the bounds with care
No vanished pool can cause a scare
The cache finds fresh units in the end
Byte-identical results it sends
Then hops away through compiler air

Comment @coderabbitai help to get the list of available commands.

@linear

linear Bot commented Sep 17, 2026

Copy link
Copy Markdown

B-1643

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-17T21:13:59.581426Z 13f4495 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

Copy link
Copy Markdown

⏭️ Performance benchmarks were skipped

Perf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to canary/main.

To run them on this PR, do any of the following, then push a commit (or re-run CI):

  • Add RUN_CODSPEED=1 to the PR description, or
  • Include run-perf or /perf in the PR title or any commit message.

@github-actions

github-actions Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

Binary size checks passed

✅ 3 passed

Artifact Platform File Gzip Gated on Baseline Delta Status
✅ baml-cli Linux 🔒 73.0 MB 27.6 MB file 73.1 MB -39.8 KB (-0.1%) OK
✅ packed-program Linux 🔒 29.0 MB 11.2 MB file 28.9 MB +9.8 KB (+0.0%) OK
✅ bridge_wasm WASM 21.7 MB 🔒 5.7 MB gzip 5.8 MB -34.7 KB (-0.6%) OK

🔒 = the size this artifact is GATED on (ceiling + delta). Binaries gate on file size (installed binary); WASM gates on gzip (download size). The other size is shown for information only.


Generated by cargo size-gate · workflow run

This branch was successfully deployed

1 active deployment
Preview – developer-docs — a371be06 Deployed Sep 22, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant