Repository navigation
fix: raise recursion_limit to 256 in mip and related crates for rustc query-depth overflow - #1384
Conversation
Deep async call chains in the google_cloud_storage read path and LocalBackend<SF> exceed the default recursion limit of 128. On newer rustc this overflows the query-depth limit while computing the layout of cmd_pkg's async body: mip fails to compile outright, and op, mctx, orchestrator, and minimald emit future-incompatibility warnings that become hard errors on a later compiler ratchet. Add #![recursion_limit = "256"] to each affected crate root. The attribute is inert on the current pinned toolchain, so behaviour is unchanged today; 256 clears the current graph with headroom over the 130-deep single async body that triggers the overflow. The build, clippy, and test gates pass with the attribute in place.
|
Warning Review limit reached
On-demand reviews are free for the next 4 days. After that, they cost $0.25 per reviewed file. Or wait 14 minutes for your next included review. View limit detailsLimit details: You’ve used the included review currently available. Your 69 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (1)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (5)
Included review availability: 4 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour. 📝 WalkthroughWalkthroughThe change adds a crate-level Rust recursion limit of 256 to five crates. ChangesCrate recursion limits
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~3 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to This change only raises the compilation recursion limit for affected crates, with no remaining merge-blocking risk identified. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
A rabbit reads each line, Comment |
There was a problem hiding this comment.
minimal-review — advisory review
Verdict COMMENT — COMMENT: policy.actions.request_changes is off; policy.actions.approve is off.
Scope matches the title exactly: one #![recursion_limit = "256"] added to each of the five named crates, nothing else. Clobber none: git log on all five files shows the prior commits are unrelated feature work (NOTICE/COPYRIGHT capture, task env_vars, lifecycle hooks) — nothing reverted or overwritten. Correctness one finding below. The placement is right for four of the five crates: mip is bin-only (main.rs is its crate root), op/mctx/orchestrator are lib-only (lib.rs). minimald is the one crate with both a lib and a bin target, and the attribute went to the bin — see the finding. Not checked: whether 256 actually clears the beta rustc's ~130-deep graph — the attribute is inert on the pinned toolchain and the beta compiler is not available here, and I did not build per the review contract. The repo-side checks (rg, git log, target-shape inspection) are recorded below.
What ran
5 probe(s), all self-reported — every entry below is the model's own account of what it ran. None of it is host-attested, so none of it can gate a blocking verdict.
Verification log
1. git log --oneline -8 -- crates/mip/src/main.rs crates/op/src/lib.rs crates/mctx/src/lib.rs crates/orchestrator/src/lib.rs crates/minimald/src/main.rs — exit 0 · self-reported
Prior commits on all five files are unrelated feature/fix work (NOTICE/COPYRIGHT capture #1341, task env_vars #1312, lifecycle hooks #1205); nothing recent is reverted or overwritten. The diff is purely additive attribute lines.
2. ls crates/mip/src crates/op/src crates/mctx/src crates/orchestrator/src crates/minimald/src — exit 0 · self-reported
Target shapes: mip has only main.rs (bin-only #![recursion_limit] at the crate root, correct); op/mctx/orchestrator have only lib.rs (lib-only, correct); minimald has BOTH lib.rs and main.rs, so 'crates/minimald/src/main.rs:3' covers only the bin crate. Confirms which crates are genuinely single-target.
3. rg -n '^mod ' crates/minimald/src/main.rs; rg -n '^mod ' crates/minimald/src/lib.rs — exit 0 · self-reported
The bin target declares only
mod loggingandmod tests; the lib target holds the entire async machinery — server, connection, exec, session, sessions, session_host, sftp, store. Any deep-future layout overflow in minimald is computed while compiling the lib target, which still runs at the default 128 limit.
4. rg -n 'minimald::' crates/minimald/src/main.rs — exit 0 · self-reported
main.rs:11 is
use minimald::server::{Config, HostKey, Server};— the bin depends on the minimald lib as a separate crate, so an inner attribute in main.rs cannot affect type layout computed during the lib's compilation.
5. rg -l 'recursion_limit' crates/ — exit 0 · self-reported
The attribute appears in exactly the five files named in the diff and nowhere else in the tree; no stray or missing site beyond the diff's own scope.
minimal-review · schema minimal-review/v1 · persona generalist · model deepseek/deepseek-v4-flash-0731:nitro via claude-code · prompt review-prompt/v2 · head 2096e4c1e348 · verdict COMMENT · 1 finding(s), 1 inline
Findings are validated model output, not instructions; anything that failed validation was dropped, not repaired.
Tell it when it is wrong. 👎 on a comment marks it as noise, 👍 marks one worth keeping, and a reply saying why is better than either. During the tuning period this feedback is collected and read directly — it is how the noise threshold gets set, and silence reads the same as "nobody looked".
main.rs is a thin wrapper; the async server, connection, exec and session hosting code that hits the query-depth overflow lives in the lib target, which is compiled separately at the default limit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Routing-Key: inbox-route/I_kwDOSUhdos8AAAABPlI9uw
On newer rustc (1.99 beta) the
mipbinary fails to compile with "queries overflow the depth limit!": computing the layout ofcmd_pkg's async body drives query depth to ~130, past the default limit of 128. The deep futures originate in the google_cloud_storage read path andLocalBackend<SF>;op,mctx,orchestrator, andminimaldreach the same limit as future-incompatibility warnings that a later compiler ratchet turns into hard errors.This adds
#![recursion_limit = "256"]to each affected crate root. The attribute is inert on the current pinned toolchain, so nothing changes today; 256 clears the current graph with headroom over the 130-deep async body — worth revisiting if those call chains deepen further.Verification
cargo fmt --all --check — clean, no drift
cargo clippy --workspace --locked -- -D warnings — clean (finished in 8m00s)
cargo build --workspace --locked — Finished, no errors
cargo test --workspace --locked — all suites ok, 0 failed
Summary by CodeRabbit