Repository navigation
perf: linear cold compiles, one warm cache for every command, flat editor latency - #4105
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughIntroduces shared CLI project sessions, manifest-backed callable-throws reuse, parallel diagnostic collection, bulk database updates, memoized Jinja typing, and target-specific SHA-2 assembly configuration. ChangesCLI cache and project workflow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLICommand
participant ProjectSession
participant CacheContext
participant ProjectDatabase
CLICommand->>ProjectSession: open project with cache mode
ProjectSession->>ProjectDatabase: resolve sources and build database
ProjectSession->>CacheContext: warm reuse plan
CacheContext->>ProjectDatabase: provide seeds and incremental diagnostics
CLICommand->>ProjectDatabase: compile artifacts
CLICommand->>CacheContext: verify and store artifacts
Possibly related PRs
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 |
⏭️ 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):
|
Merging this PR will improve performance by 13.76%
Performance Changes
Tip Curious why this is faster? Comment Comparing 
Footnotes |
Binary size checks passed✅ 7 passed
Generated by |
hellovai
force-pushed
the
hellovai/project-session
branch
from
July 22, 2026 03:17
26ee6bc to
98b988b
Compare
hellovai
force-pushed
the
hellovai/warm-floor-cache
branch
from
July 22, 2026 03:17
dc5828e to
5573885
Compare
hellovai
changed the title
hellovai
marked this pull request as ready for review
July 23, 2026 05:26
hellovai
force-pushed
the
hellovai/project-session
branch
from
July 23, 2026 05:45
98b988b to
9a3dbe5
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
baml_language/crates/baml_cli/src/run_command.rs (1)
737-772: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftConsider consolidating the warm→compile→verify_and_store flow onto
ProjectSession.This incremental-diagnostics +
compile_program_artifacts+verify_and_store(..., || session.honest_db())block is duplicated near-verbatim intest_command.rs(≈144-192),pack_command.rsload_and_compile_project(≈303-338), andcheck_command.rs(≈47-137). Because these copies must stay in lock-step to preserve cache soundness (same reuse-plan handling, same fresh-diagnostics threading, sameverify_and_storeargs), a drift in one is easy to miss. Extracting a singlesessionmethod (e.g.compile_and_store(options, reporter) -> CompiledArtifacts) would keep the cache contract in one place.🤖 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/run_command.rs` around lines 737 - 772, Consolidate the duplicated warm, diagnostic, compile, and cache-verification flow into a single ProjectSession method, such as compile_and_store, and use it from run_command.rs, test_command.rs, pack_command.rs, and check_command.rs. Preserve the existing reuse-plan handling, fresh diagnostics, compile options, error reporting, stdlib_interface_hit, and session.honest_db() verification arguments so all callers share the same cache-soundness contract.baml_language/crates/baml_cli/src/project_load.rs (1)
130-158: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate parallel file-read logic between
resolve_project_sources_lenientandresolve_project_sources.Both functions implement the identical
discover_baml_files(...).into_par_iter().map(...).collect::<Result<Vec<_>>>()block. Consider extracting a shared helper (e.g.read_baml_files_parallel(walk_root) -> Result<Vec<(PathBuf, String)>>) to avoid the two copies drifting (e.g. only the strict variant currently documents the order-preservation guarantee that both rely on).♻️ Proposed extraction
+fn read_baml_files_parallel(walk_root: &Path) -> Result<Vec<(PathBuf, String)>> { + // Read sources across worker threads; `collect` on an indexed parallel + // iterator preserves discovery order, so `FileId` assignment (and with it + // diagnostic ordering) is identical to a serial read. + use rayon::prelude::*; + discover_baml_files(walk_root) + .into_par_iter() + .map(|path| { + let content = std::fs::read_to_string(&path) + .with_context(|| format!("Failed to read {}", path.display()))?; + Ok((path, content)) + }) + .collect::<Result<Vec<_>>>() +} + pub(crate) fn resolve_project_sources_lenient( from: Option<&Path>, ) -> Result<Option<ResolvedProject>> { ... let walk_root = project_source_root(&canonical); - use rayon::prelude::*; - let files = discover_baml_files(&walk_root) - .into_par_iter() - .map(|path| { - let content = std::fs::read_to_string(&path) - .with_context(|| format!("Failed to read {}", path.display()))?; - Ok((path, content)) - }) - .collect::<Result<Vec<_>>>()?; + let files = read_baml_files_parallel(&walk_root)?; Ok(Some(ResolvedProject { root: canonical, manifest, files })) }Also applies to: 303-314
🤖 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/project_load.rs` around lines 130 - 158, Extract the duplicated parallel file-reading block from resolve_project_sources_lenient and resolve_project_sources into a shared helper, such as read_baml_files_parallel, accepting the source root and returning Result<Vec<(PathBuf, String)>>. Have both functions call the helper, preserving rayon’s existing order-preservation behavior and the current read-error context.
🤖 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/project_load.rs`:
- Around line 130-158: Extract the duplicated parallel file-reading block from
resolve_project_sources_lenient and resolve_project_sources into a shared
helper, such as read_baml_files_parallel, accepting the source root and
returning Result<Vec<(PathBuf, String)>>. Have both functions call the helper,
preserving rayon’s existing order-preservation behavior and the current
read-error context.
In `@baml_language/crates/baml_cli/src/run_command.rs`:
- Around line 737-772: Consolidate the duplicated warm, diagnostic, compile, and
cache-verification flow into a single ProjectSession method, such as
compile_and_store, and use it from run_command.rs, test_command.rs,
pack_command.rs, and check_command.rs. Preserve the existing reuse-plan
handling, fresh diagnostics, compile options, error reporting,
stdlib_interface_hit, and session.honest_db() verification arguments so all
callers share the same cache-soundness contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b2933ace-47d4-464d-90c8-a6c85067099a
⛔ Files ignored due to path filters (1)
baml_language/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (23)
baml_language/Cargo.tomlbaml_language/crates/baml_cli/Cargo.tomlbaml_language/crates/baml_cli/src/bytecode_cache.rsbaml_language/crates/baml_cli/src/check_command.rsbaml_language/crates/baml_cli/src/describe_command.rsbaml_language/crates/baml_cli/src/generate.rsbaml_language/crates/baml_cli/src/grep_command.rsbaml_language/crates/baml_cli/src/lib.rsbaml_language/crates/baml_cli/src/pack_command.rsbaml_language/crates/baml_cli/src/project_load.rsbaml_language/crates/baml_cli/src/project_session.rsbaml_language/crates/baml_cli/src/run_command.rsbaml_language/crates/baml_cli/src/test_command.rsbaml_language/crates/baml_lsp2_actions/src/check.rsbaml_language/crates/baml_project/src/check.rsbaml_language/crates/baml_project/src/db.rsbaml_language/crates/baml_project/src/lib.rsbaml_language/crates/bex_cache/Cargo.tomlbaml_language/crates/bex_cache/src/lib.rsbaml_language/crates/bex_project/src/project.rsbaml_language/crates/bex_project/src/seed.rsbaml_language/crates/sys_jinja_types/src/evaluate_type/mod.rsbaml_language/crates/sys_jinja_types/src/evaluate_type/types.rs
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
baml_language/crates/baml_cli/src/run_command.rs (1)
686-793: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftCache-aware compile flow duplicated across commands.
This block (session bootstrap →
warm_prep→ cache-gated incremental vs. full diagnostics →compile_program_artifacts→verify_and_store) is essentially identical topack_command.rs'sload_and_compile_project, differing only in bail-message text and silent vs. verbose logging. Withcheck/testlikely following the same shape (per the PR's stated cohort), this sequence now lives in 3-4 places that must be kept in sync by hand — a future change to cache verification or diagnostics merging risks silently drifting between commands.Consider hoisting this into a single method on
ProjectSession(e.g.compile_with_diagnostics(cache_bail_msg, no_cache_bail_msg, silent: bool)) that each command calls, keeping only per-command formatting/dispatch here.🤖 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/run_command.rs` around lines 686 - 793, Extract the duplicated session bootstrap, warm_prep, diagnostics collection, compilation, and verify_and_store sequence from the current command flow into a shared ProjectSession method such as compile_with_diagnostics. Parameterize only the command-specific bail messages and silent/verbose logging behavior, then update this flow and pack_command.rs’s load_and_compile_project to delegate to it while preserving their existing dispatch and engine creation.
🤖 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/run_command.rs`:
- Around line 686-793: Extract the duplicated session bootstrap, warm_prep,
diagnostics collection, compilation, and verify_and_store sequence from the
current command flow into a shared ProjectSession method such as
compile_with_diagnostics. Parameterize only the command-specific bail messages
and silent/verbose logging behavior, then update this flow and pack_command.rs’s
load_and_compile_project to delegate to it while preserving their existing
dispatch and engine creation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ff06112a-776b-49f8-b0aa-c0be8aca6212
⛔ Files ignored due to path filters (1)
baml_language/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
baml_language/Cargo.tomlbaml_language/crates/baml_cli/Cargo.tomlbaml_language/crates/baml_cli/src/bytecode_cache.rsbaml_language/crates/baml_cli/src/check_command.rsbaml_language/crates/baml_cli/src/describe_command.rsbaml_language/crates/baml_cli/src/generate.rsbaml_language/crates/baml_cli/src/grep_command.rsbaml_language/crates/baml_cli/src/lib.rsbaml_language/crates/baml_cli/src/pack_command.rsbaml_language/crates/baml_cli/src/project_load.rsbaml_language/crates/baml_cli/src/project_session.rsbaml_language/crates/baml_cli/src/run_command.rsbaml_language/crates/baml_cli/src/test_command.rsbaml_language/crates/baml_lsp2_actions/src/check.rsbaml_language/crates/baml_project/src/check.rsbaml_language/crates/baml_project/src/db.rsbaml_language/crates/baml_project/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (15)
- baml_language/Cargo.toml
- baml_language/crates/baml_cli/Cargo.toml
- baml_language/crates/baml_cli/src/lib.rs
- baml_language/crates/baml_project/src/lib.rs
- baml_language/crates/baml_cli/src/grep_command.rs
- baml_language/crates/baml_cli/src/describe_command.rs
- baml_language/crates/baml_cli/src/check_command.rs
- baml_language/crates/baml_cli/src/generate.rs
- baml_language/crates/baml_cli/src/project_load.rs
- baml_language/crates/baml_cli/src/test_command.rs
- baml_language/crates/baml_cli/src/project_session.rs
- baml_language/crates/baml_project/src/check.rs
- baml_language/crates/baml_lsp2_actions/src/check.rs
- baml_language/crates/baml_cli/src/pack_command.rs
- baml_language/crates/baml_cli/src/bytecode_cache.rs
hellovai
force-pushed
the
hellovai/project-session
branch
from
July 23, 2026 06:23
9a3dbe5 to
a8c6544
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
baml_language/crates/bex_project/src/seed.rs (1)
754-754: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winExercise a non-empty callable-throws fragment.
Vec::new()validates only that the updated manifest decodes. Add a fixture with a real non-empty fragment and assert it survives disk serialization and seed application.🤖 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/bex_project/src/seed.rs` at line 754, Add a fixture containing a non-empty callable-throws fragment and use it in the seed test around the callable_throws_fragment field instead of Vec::new(). Assert the fragment is preserved after manifest disk serialization and subsequent seed application, verifying both persistence and application behavior.baml_language/crates/bex_cache/src/lib.rs (1)
635-658: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReuse the computed digest in
store_unit_shared.The new helper avoids a second hash in
load_unit_shared, butstore_unit_sharedstill callsload_rawand thenunit_key(&stored), hashing every existing payload twice. Propagate the digest through the local store-hit check and compare withunit_key_from_digest.🤖 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/bex_cache/src/lib.rs` around lines 635 - 658, Update store_unit_shared to use check_entry_with_digest for the local store-hit validation, retain the returned payload and digest, and compare the requested key via unit_key_from_digest instead of recomputing unit_key(&stored). Preserve the existing store behavior for misses and invalid entries while eliminating the second payload hash.baml_language/scripts/hawk (1)
34-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSingle-source the Rust toolchain version.
The Darwin sysroot lookup duplicates
1.97.1separately frommise.tomland the finalcargo +1.97.1invocation. Centralize this in one shell variable and use it for bothrustcandcargoto prevent macOS-only drift after a toolchain bump.Suggested change
+hawk_toolchain="1.97.1" + ... - sysroot="$(rustc +1.97.1 --print sysroot 2>/dev/null || true)" + sysroot="$(rustc +"$hawk_toolchain" --print sysroot 2>/dev/null || true)" ... -exec cargo +1.97.1 hawk check \ +exec cargo +"$hawk_toolchain" hawk check \🤖 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/scripts/hawk` around lines 34 - 36, Define a single shell variable for the Rust toolchain version near the start of the script, then reuse it in both the Darwin sysroot lookup via rustc and the final cargo invocation. Remove the duplicated hardcoded 1.97.1 references while preserving the existing mise.toml toolchain value.
🤖 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/scripts/hawk`:
- Around line 13-24: Update the cargo-hawk resolution in the scripts/hawk flow
to use mise’s pinned environment via mise exec or mise which, rather than
accepting arbitrary PATH binaries or scanning the installs directory. Ensure all
availability checks and the eventual invocation resolve the exact cargo-hawk
revision configured by mise.toml, while preserving the existing missing-tool
error and exit behavior.
---
Nitpick comments:
In `@baml_language/crates/bex_cache/src/lib.rs`:
- Around line 635-658: Update store_unit_shared to use check_entry_with_digest
for the local store-hit validation, retain the returned payload and digest, and
compare the requested key via unit_key_from_digest instead of recomputing
unit_key(&stored). Preserve the existing store behavior for misses and invalid
entries while eliminating the second payload hash.
In `@baml_language/crates/bex_project/src/seed.rs`:
- Line 754: Add a fixture containing a non-empty callable-throws fragment and
use it in the seed test around the callable_throws_fragment field instead of
Vec::new(). Assert the fragment is preserved after manifest disk serialization
and subsequent seed application, verifying both persistence and application
behavior.
In `@baml_language/scripts/hawk`:
- Around line 34-36: Define a single shell variable for the Rust toolchain
version near the start of the script, then reuse it in both the Darwin sysroot
lookup via rustc and the final cargo invocation. Remove the duplicated hardcoded
1.97.1 references while preserving the existing mise.toml toolchain value.
🪄 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: 49eef261-43a0-4a67-9cd6-6196619d0ad7
⛔ Files ignored due to path filters (1)
baml_language/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (24)
baml_language/Cargo.tomlbaml_language/crates/baml_cli/Cargo.tomlbaml_language/crates/baml_cli/src/bytecode_cache.rsbaml_language/crates/baml_cli/src/check_command.rsbaml_language/crates/baml_cli/src/describe_command.rsbaml_language/crates/baml_cli/src/generate.rsbaml_language/crates/baml_cli/src/grep_command.rsbaml_language/crates/baml_cli/src/lib.rsbaml_language/crates/baml_cli/src/pack_command.rsbaml_language/crates/baml_cli/src/project_load.rsbaml_language/crates/baml_cli/src/project_session.rsbaml_language/crates/baml_cli/src/run_command.rsbaml_language/crates/baml_cli/src/test_command.rsbaml_language/crates/baml_lsp2_actions/src/check.rsbaml_language/crates/baml_project/src/check.rsbaml_language/crates/baml_project/src/db.rsbaml_language/crates/baml_project/src/lib.rsbaml_language/crates/bex_cache/Cargo.tomlbaml_language/crates/bex_cache/src/lib.rsbaml_language/crates/bex_project/src/project.rsbaml_language/crates/bex_project/src/seed.rsbaml_language/crates/sys_jinja_types/src/evaluate_type/mod.rsbaml_language/crates/sys_jinja_types/src/evaluate_type/types.rsbaml_language/scripts/hawk
🚧 Files skipped from review as they are similar to previous changes (20)
- baml_language/Cargo.toml
- baml_language/crates/baml_cli/Cargo.toml
- baml_language/crates/bex_cache/Cargo.toml
- baml_language/crates/sys_jinja_types/src/evaluate_type/mod.rs
- baml_language/crates/baml_project/src/lib.rs
- baml_language/crates/baml_cli/src/lib.rs
- baml_language/crates/baml_cli/src/describe_command.rs
- baml_language/crates/baml_cli/src/grep_command.rs
- baml_language/crates/baml_cli/src/generate.rs
- baml_language/crates/baml_cli/src/run_command.rs
- baml_language/crates/sys_jinja_types/src/evaluate_type/types.rs
- baml_language/crates/baml_lsp2_actions/src/check.rs
- baml_language/crates/baml_cli/src/check_command.rs
- baml_language/crates/baml_cli/src/pack_command.rs
- baml_language/crates/baml_project/src/check.rs
- baml_language/crates/baml_cli/src/test_command.rs
- baml_language/crates/baml_cli/src/project_load.rs
- baml_language/crates/baml_cli/src/project_session.rs
- baml_language/crates/baml_cli/src/bytecode_cache.rs
- baml_language/crates/baml_project/src/db.rs
… the cache-path check Three independent O(N^2) / serialization fixes, found by profiling a 5000-module (295k LOC) corpus where cold check burned 126s of CPU: - Memoize the package-wide Jinja type environment as a salsa query (jinja_base_types, keyed by NamespaceId) instead of rebuilding it inside every check_file call, and skip it entirely for files with no LLM prompts or template_strings. The env walk was O(package items) per file, i.e. O(N^2) per full check - and ran twice per compile because get_bytecode's error gate re-checks the project. PredefinedTypes and friends gain PartialEq so salsa can backdate: edits that leave the env unchanged don't invalidate prompt checks. - Arc-wrap ProjectDatabase's file_map / file_id_to_path so cloning a db handle is O(1) instead of O(files). The parallel check and emit drivers mint a handle per work chunk (~1.55N clones per compile), which deep-cloned two N-entry PathBuf maps each time: quadratic coordinator CPU and the dominant term in peak RSS. - Run the narrowed (cache-path) diagnostics collector through the same rayon fan-out as the honest collector. baml check / run route through the narrowed collector, which checked files serially - the parallel machinery next to it was only reachable with the cache disabled. - compile_outcome (LSP) now proves the project error-free once and calls a new get_bytecode_unchecked instead of letting get_bytecode re-run a full project check inside emit. To keep that sound, the diagnostics candidate now includes package-level diagnostics (cross-file name conflicts / namespace shadows) - which previously never reached the editor at all. Measured on the 295k LOC corpus: cold check 21-46s -> 7.1s wall, 125.7s -> 8.9s user CPU, peak RSS 5.3GB -> 3.6GB; scaling is now ~linear in project size (verified to 8000 modules). Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01V4oee6wrCHzXH6mM2h8eJo
Two gaps kept most workflows off the incremental path: - baml pack never opened a CacheContext: every pack was a fully cold compile, even immediately after a successful run of the same sources. Pack now follows run's exact flow - whole-program hit when nothing changed, per-file unit reuse on a dirty edit, store after a fresh compile. Pack compiles with emit_test_cases:false, so it shares run/check's cache key space; the packaged Program is target-independent (the --target triple only selects host binary bytes) and emit determinism guarantees a reused image is byte-identical to a fresh compile. Standalone --file mode stays cold, same as run. - baml check reused cache entries but never wrote them, so a check-only workflow (editors, CI lint jobs) recompiled everything forever. A clean check now runs the same emit-and-store path as run/test - gated to checks that actually advance the manifest (missing/stale manifest or dirty files), so no-op checks stay emit-free. Seeding is best-effort: an emit failure on a diagnostics-clean project is logged, never surfaced as a check failure. End-user effect (59k LOC project): check 1.0s (cold, seeds) -> check 0.5s -> run 0.10s -> pack 0.13s, all from one warmed cache. Re-pack of a 295k LOC project: 40.5s -> 0.61s. Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01V4oee6wrCHzXH6mM2h8eJo
The warm bytecode-cache path hashes every cached unit payload on each run (~230MB of hashing at a 295k LOC project) and was using sha2's software fallback - the asm feature (SHA-NI / ARMv8 crypto extensions, runtime-detected, target-gated so wasm builds are unaffected) was never enabled. Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01V4oee6wrCHzXH6mM2h8eJo
sha2's asm feature does not build under MSVC (sha2-asm ships GAS .S sources cl.exe cannot assemble), so move the feature from the workspace dependency to target-gated tables in bex_cache: every non-MSVC target keeps the hardware backend, MSVC falls back to the portable implementation. Also fix a rustdoc -D warnings failure: the doc comment on collect_package_level_diagnostics linked the private package_level_diagnostics item. Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01V4oee6wrCHzXH6mM2h8eJo
…roject load; parallel LSP sweep Three fixes for the "12 threads = 1.18x" cold-check scaling wall, diagnosed by comparing against ty/ruff's salsa usage (whose driver pattern ours already matches): - Prime wave: compute every file's PPIR semantic index across workers BEFORE fanning out per-file checks. Whole-package aggregate queries (package_items / namespace_items) fold over every file's index, and every file's check demands them - on a cold database the first worker to claim the aggregate memo computed parse+lowering+indexing for the entire project inline on one thread while all other workers parked on that memo's sync slot. With per-file indexes primed the aggregate fold is cheap and workers stay busy. Also replaces the old first-file serial warm-up, which warmed the builtin package (compiler2_all_files orders builtins first), not the user package. - Bulk project load: add_or_update_files registers all sources with ONE project-file-list write; the per-file path cloned and re-set the whole files Vec and bumped the salsa revision once per file (O(files^2) copies during initial load). Source reads now happen across worker threads, preserving discovery order so FileId assignment and diagnostic ordering are unchanged. - The LSP diagnostics candidate now checks files through the shared order-preserving parallel checker (under the held source guard - the same exclusion the serial loop relied on), so every per-edit path inherits the parallelism and the prime wave. check_files_parallel returns per-file results in input order so the LSP can zip them back to documents; the flat collector delegates. Measured (Apple Silicon, 12 cores): - cold pure check, 295k LOC: 7.4s -> 2.8s wall (1.2 -> 2.4+ cores) - cold check+seed, 295k LOC: 10.5s -> 4.4s; 59k LOC: ~0.9s - LSP edit->diagnostics: 250 files 464 -> 172ms; 500 files 1.7s -> 176ms; 1000 files 5.5s -> 209ms (now ~flat in project size) - LSP load test: burst storms + sustained churn at 250/1000 files, zero crashes, stable RSS, convergence after every storm Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01V4oee6wrCHzXH6mM2h8eJo
…nifest fragments, not unit payloads The warm path spent most of a no-op invocation re-proving its own cache validity: the serve-time throws gate re-derived package_items (re-parsing every file) to convert types for a comparison, and plan_reuse read + hashed + decoded every clean unit payload just to extract the tiny callable-throws fragments it seeds from. At a 295k LOC project that was ~7.2s to conclude "nothing changed" — the same wall time as recompiling from scratch. - No-delta gate skip: when the dirty partition finds no changed, added, or removed files, the gate is provably a tautology — the seeds it would validate are byte-for-byte the values the manifest was stored with (store persists file_throw_facts verbatim; units are content-addressed and written before the manifest), and the solve + runtime conversion are deterministic pure functions of those inputs. Any real delta still takes the full gate. - Manifest-resident fragments: ManifestFile now carries a verbatim copy of each unit's CallableThrowsFragment blob, and the callable_throws seeds project from that copy — so a no-delta invocation opens zero unit payloads. Units are loaded (now in parallel, manifest order preserved) only when a delta means something will be re-emitted or relinked. Both verify oracles (the BAML_CACHE_VERIFY full compare and the 1-in-32 sampled re-derive) now check the manifest copy — exactly the artifact that is served. - The gate path (and every warm collection) primes per-file semantic indexes across workers first, so the package_items derivation the gate and package-level diagnostics demand is a parallel-fed fold instead of a serial parse of the project. - bex_cache: unit_key is derived from the payload's integrity digest, which entry validation already computes — load_unit_shared now reuses that digest instead of hashing the payload a second time. Behavior change, deliberate: a unit file deleted out from under a warm cache is no longer detected on a no-op invocation (nothing reads it); the next compile that needs it falls back to a byte-identical full compile. Slower once, never wrong. Old manifests (without the fragment field) fail borsh decode and are treated as absent — one full recompile re-seeds them. Measured (Apple Silicon): warm no-op check at 295k LOC 7.2s -> 2.0s (zero unit reads, gate skipped); at 59k LOC 0.54s -> 0.30s. Warm leaf-edit check is emit-bound (it seeds the cache for later run/pack hits) and unchanged. Verified: 66 cache tests, BAML_CACHE_VERIFY honest-recompile compare (no-op + after-edit), forced sampled verify across 7 warm invocations, diagnostics byte-identity cold + warm. Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01V4oee6wrCHzXH6mM2h8eJo
…mmand
check, run, test, pack, describe, and grep each hand-rolled their own
subset of the same preamble (resolve sources, build the database, open
the bytecode cache, install warm seeds), and the subsets drifted: pack
shipped with no cache participation at all, and describe/grep got
neither the warm seeds nor the parallel index prime — a describe at
295k LOC cost as much as a cold compile, every invocation.
ProjectSession is the single implementation. A command states how it
uses the cache (ReadWrite / ReadWriteTests / ReadOnly / Off) and gets
the identical ritual; the phases stay explicit so each command keeps
its sequencing:
open / open_lenient resolve + read sources + db + cache open
try_cached_program whole-program hit, before any warm prep
warm_prep stdlib seed + per-file reuse plan (the gate)
warm_prep_seeds_only read-only introspection: seeds installed only
on a no-delta plan, where the serve-time gate
is a proven tautology; with edits pending,
introspection derives honestly instead
prime parallel per-file index prime for commands
that query package aggregates outside the
check collectors
Ported: check, run, test, pack (ReadWrite/ReadWriteTests — behavior
unchanged, byte-identical diagnostics, cache hits intact), and
describe, grep (lenient open + ReadOnly + prime: a broken or missing
baml.toml still never blocks introspection; outside a project the
session is an empty stdlib-only database with the cache off).
Measured: describe on a 295k LOC project 6.3s -> 3.4s (the aggregate
derivation is now parallel-fed); output byte-identical. 43 cache
tests pass; run/pack whole-program hits verified live.
Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01V4oee6wrCHzXH6mM2h8eJo
…or_build generate was the last project-loading command on a bespoke loader. It always passed verbose=false, so the loader's one distinguishing feature (the verbose-gated per-file Loading flood) was never used by it — the session port is a pure consolidation. generate now takes the read-only session like describe/grep: warm seeds where provably faithful plus the parallel index prime before codegen walks the whole project. load_project_for_build had no remaining callers and is removed; the reporting variant stays reachable through resolve_source_location. Smoke: python/pydantic codegen over a 100-module corpus produces the identical 111 files; error rendering for invalid generator configs unchanged (byte-identical against the pre-port binary). Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01V4oee6wrCHzXH6mM2h8eJo
The cargo-hawk pre-commit hook could not succeed on a dev machine without manual setup, which invites SKIP=cargo-hawk — exactly how a dead-public finding slipped past local hooks to CI. Three gaps, each now self-healed inside the script: - PATH: cargo-hawk is provisioned by mise, whose shims are not on the PATH git hook subshells inherit; fall back to the mise install dir, with an actionable "run mise install" error when truly absent. - RUSTC_WRAPPER: hawk's instrumented check runs through its own rustc driver, which the baml-sccache wrapper cannot probe. - macOS: the prebuilt cargo-hawk-driver has no rpath, so it cannot locate the pinned toolchain's librustc_driver without DYLD_FALLBACK_LIBRARY_PATH. Verified: `scripts/hawk` passes from a bare subshell with no environment help (0 findings), i.e. the exact way prek invokes it. Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01V4oee6wrCHzXH6mM2h8eJo
Contributor
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/scripts/hawk`:
- Around line 15-19: Update the mise-handling branch in the hawk script so a
failed or invalid `mise which cargo-hawk` resolution exits that branch/script
before any inherited PATH fallback can be used. Preserve the existing
resolved-binary PATH setup when `hawk_bin` is executable, and allow PATH
fallback only when `mise` is unavailable.
🪄 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: 588a4db9-016c-42fb-820b-f70763b251a4
📒 Files selected for processing (1)
baml_language/scripts/hawk
hellovai
force-pushed
the
hellovai/project-session
branch
from
July 23, 2026 08:23
ddcb967 to
c9989f1
Compare
Contributor
Author
|
@coderabbitai resolve All review items addressed in the latest push: the hawk resolver now goes through |
Contributor
|
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! ✅ Action performedComments resolved and changes approved. |
This was referenced Aug 13, 2026
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Consolidated stack (supersedes #4098, #4100, #4101 — their commits are all contained here, rebased onto current canary). Seven commits, reviewable independently, in dependency order.
What this does
A ground-up performance pass on the compiler driven by head-to-head benchmarking against tsgo (native TypeScript) and
go buildon semantically identical generated corpora (1.5k–295k LOC), followed by root-cause profiling of every gap found.1.
perf(compiler2): kill the two cold-compile quadratics; parallelize the cache-path checkcheck_filecall (O(N²)) — and twice per compile viaget_bytecode's error gate. Now a memoized salsa query (jinja_base_types) + skipped entirely for files with no prompts.ProjectDatabasehandle clones deep-copied two N-entry PathBuf maps; the parallel drivers clone ~1.55N handles per compile (quadratic CPU + peak RSS). Now Arc-wrapped, O(1).baml check/runrouted through a serial diagnostics collector while the parallel one sat unreachable next to it. Now shared.get_bytecode_unchecked), and its diagnostics candidate now includes package-level diagnostics — cross-file name conflicts previously never reached the editor at all.2.
feat(cli):baml packuses the bytecode cache;baml checkseeds itPack was fully cold on every invocation (no cache participation at all); check reused but never wrote, so check-only workflows never warmed. Now one warm cache serves check → run → test → pack.
3–4.
build+fix(ci): hardware SHA-2 (sha2/asm, gated off MSVC), doc-link fixThe warm path hashes ~hundreds of MB of unit payloads per run; it was on the software SHA fallback.
5.
perf(compiler2): prime wave, bulk project load, parallel LSP sweepCold parallel check scaled 1.18× on 12 cores: every file's check demands whole-package aggregates, and the first worker to claim that memo computed parse+index for the entire project inline while all other workers parked on the sync slot (diagnosed against ty/ruff's salsa usage; interventionally verified). Per-file indexes are now primed across workers before fan-out. Project load stops re-writing the file list per file (O(files²)) and reads sources in parallel (FileId order preserved). The LSP candidate uses the shared order-preserving parallel checker.
6.
perf(cache): warm no-op checks stop re-proving the cacheA warm no-op check cost the same as a full recompute: the throws-gate safety compare re-parsed every file, and seeding read+hashed 78MB of unit payloads for metadata. Now: the gate is skipped when the dirty partition is provably empty (a value compared against itself), throws fragments live verbatim in the manifest so a no-op check opens zero unit payloads, and units load in parallel (manifest order preserved) only when a delta means relink will need them. Both verify oracles (
BAML_CACHE_VERIFYfull compare + always-on 1-in-32 sampled re-derive) now check the manifest copy — exactly what is served. Design was adversarially reviewed; the unsound variant (fact-space throws compare) was rejected and every real edit still takes the full runtime-space gate.7.
refactor(cli): one sharedProjectSessionfor every project-loading commandcheck/run/test/pack/describe/grep/generate each hand-rolled a drifting subset of the same preamble — the root cause of "pack forgot the cache" and "describe never primes." One implementation; commands declare
CacheUse::{ReadWrite, ReadWriteTests, ReadOnly, Off}. Introspection installs seeds only on a no-delta plan (provable tautology) and derives honestly otherwise.Measured impact (Apple Silicon, 12 cores, hyperfine)
runafter check (59k / 295k)packafter check (59k / 295k)describe, 295k / 59k LOCCold compile scaling is now ~linear in project size (was quadratic: 5× code cost 33× CPU), verified to 8000 modules.
Verification
BAML_CACHE_VERIFY=1honest-recompile compare (no-op + after-edit) and forcedBAML_CACHE_SAMPLED_VERIFY=1across edit sequences — all pass; 43 bytecode-cache oracle tests updated and green.Deliberate trade-offs
checkat mid-size projects (~15–30k LOC) is ~10% slower than before: it now emits + stores once to warm the cache; every later check/run/pack is served warm.🤖 Generated with Claude Code
https://claude.ai/code/session_01V4oee6wrCHzXH6mM2h8eJo
Summary by CodeRabbit