Repository navigation
fix(minimald): reuse the compose-time graph names in the finalize package check - #2304
Conversation
…kage check The finalize-time package check re-evaluated the project's nickel graph (~1.1 MB of retained heap per evaluation) to answer a question the compose step had already answered minutes earlier: which packages the session's graph declares. run_compose now returns the package names its own graph resolution produced, configure_loadout captures them on the actor, and the check answers membership from that capture — falling back to the original evaluation only for actors that never saw a compose (restored from disk) or whose workspace had no mfile. Expiry no longer relies on the graph evaluation being slow: tokio polls the check before its timer, so a Duration::ZERO deadline would lose the race against judging the captured names. The check now measures its own budget before any judging and steps aside deterministically. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Warning Review limit reached
This review includes 5 billable files and costs up to $1.25. Or wait 39 minutes for your next included review. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Docstring Coverage |  |
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 5 functions across 3 files. |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Title check | |
The title clearly and concisely describes the main change: reusing compose-time graph names during the finalize package check. |
| Description check | |
The description explains the change, motivation, fallback behavior, expiry handling, and verification results. It does not use the template headings exactly and omits the checklist, but it is substant… |
Full details: Description check
-
Explanation
The description explains the change, motivation, fallback behavior, expiry handling, and verification results. It does not use the template headings exactly and omits the checklist, but it is substantially complete.
✨ Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
A rabbit checks the names in view,
Then checks the clock before it’s through.
If names were saved, they guide the way,
If not, the graph has more to say.
Hop, hop—the check steps on its way!
Comment @coderabbitai help to get the list of available commands.
…graph reuse Word an unknown session package through one env::unknown_package helper for both the launch lookup and the captured-names check. Judge captured names inline from a HashSet instead of a blocking-pool round-trip, log the up-front budget expiry like the timer expiry, and add tests that finalize judges the compose-time graph and that an actor without captured names still evaluates the graph and refuses.
|
/review |
|
Approved: no findings at Nothing to do. Clobber: none: |
There was a problem hiding this comment.
Approved on ARCH PASS at fd1e742.
Routing-Key: inbox-route/I_kwDOSUhdos8AAAABVa6jbw
The finalize-time package check re-evaluated the project's nickel graph — the most expensive step of a compose, ~1.1 MB of retained heap per evaluation — to answer a question the compose step had already answered minutes earlier: which packages the session's graph declares.
run_composenow returns the package names its own graph resolution produced, the actor captures them atconfigure_loadout, and the check answers membership from that capture. Actors restored from disk (or with no mfile) keep the original evaluation. Expiry is now deterministic: the check measures its own budget before judging, because aDuration::ZEROdeadline loses tokio's poll-the-future-before-the-timer race against judging the captured names.Verification
cargo fmt --all --check— clean.cargo clippy --workspace --all-targets --locked -- -D warnings— clean.cargo test -p minimald— 999 passed, 11 failed. The 11 (net/switch, connection, rpc) fail identically on a clean-tree baseline run ofmain(999/11, same test names) — pre-existing sandboxed-runner flake, not a regression. All four package-check tests pass, including the zero-deadline expiry test, now deterministically.crates/minimald/**is covered by thenative-daemon-e2eandtest-kvmlanes.Summary by CodeRabbit