Migrate to Polonius and evolve internal APIs around borrowing (#465) - #472
Migrate to Polonius and evolve internal APIs around borrowing (#465)#472leynos wants to merge 20 commits into
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 55 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (31)
WalkthroughAdopt pinned nightly Rust with ChangesPolonius migration
Sequence Diagram(s)sequenceDiagram
participant Build
participant Cargo
participant RustNightly
participant Polonius
Build->>Cargo: invoke build, test, lint, or check
Cargo->>RustNightly: use nightly-2026-06-25
RustNightly->>Polonius: enable -Zpolonius=next
Polonius-->>Build: compile with Polonius analysis
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 20✅ Passed checks (20 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Reviewer's GuideMigrates the project to a pinned Polonius-enabled nightly toolchain and refactors selected internal APIs to be borrow-centric, introducing a graph node registry abstraction, tightening which/env and cycle detector lifetimes, simplifying the Kani cfg policy test harness, and wiring CI, Makefile, and documentation around the new borrow-checking contract. File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b3e7732fa
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # `-Zpolonius=next` themselves. See | ||
| # docs/adr-006-adopt-polonius-nightly-toolchain.md. | ||
| [build] | ||
| rustflags = ["-Zpolonius=next"] |
There was a problem hiding this comment.
Preserve Polonius settings for registry installs
Once this source is published, the documented bare cargo install netsuke runs outside the checkout and therefore receives neither this flag nor the pinned nightly: the manifest's package allow-list excludes both .cargo/config.toml and rust-toolchain.toml (Cargo's package --list help describes that command as printing the files included in the package). The packaged source now contains a Polonius-only accessor, so ordinary registry installs compile it under the user's toolchain without -Zpolonius=next and fail with E0499. Keep distributed source NLL-compatible or document and validate an install command that explicitly selects the nightly and passes the flag.
AGENTS.md reference: AGENTS.md:L205-L207
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@coderabbitai Has this now been resolved in the latest commit?
Use codegraph analysis to determine your answer.
If this comment is now resolved, please mark it as such using the API. Otherwise, please provide an AI agent prompt for the remaining work to be done to address this comment.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '== Current commit and package configuration =='
git rev-parse HEAD
printf '\n-- Cargo package fields --\n'
rg -n -A12 -B3 '^\[package\]|\b(include|exclude)\s*=' Cargo.toml || true
printf '\n-- Files Cargo says are packaged --\n'
cargo package --list --allow-dirty 2>&1 | sed -n '1,220p' || true
printf '\n== Module map ==\n'
ast-grep outline src/graph_view/mod.rs --items all --match 'NodePathRegistry|ensure_node_mut|EdgeRegistrar|from_build_graph' --view expanded
printf '\n== Borrow-centric registry symbols and callers ==\n'
rg -n -C5 '\b(NodePathRegistry|ensure_node_mut)\b' src/graph_view/mod.rs src/graph_view/tests.rs
printf '\n== Polonius-tagged sites and installation guidance ==\n'
rg -n -C4 'POLONIUS|cargo install|install netsuke|Zpolonius|nightly-2026-06-25' \
Cargo.toml README.md docs AGENTS.md Makefile .cargo/config.toml rust-toolchain.toml \
-g '!docs/contents.md' || true
printf '\n== Tracked package/build configuration status ==\n'
git ls-files .cargo/config.toml rust-toolchain.toml Cargo.toml README.md docs/adr-006-adopt-polonius-nightly-toolchain.md docs/polonius.mdLength of output: 1254
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '== PR head =='
git rev-parse HEAD
printf '%s\n' '== Package boundary =='
sed -n '1,140p' Cargo.toml
printf '%s\n' '-- configuration files tracked by Git --'
git ls-files --stage .cargo/config.toml rust-toolchain.toml
printf '%s\n' '== Registry accessor definition and direct call graph =='
ast-grep outline src/graph_view/mod.rs --items all --match 'NodePathRegistry|ensure_node_mut|EdgeRegistrar|from_build_graph' --view expanded
rg -n -C5 '\b(NodePathRegistry|ensure_node_mut)\b' src/graph_view/mod.rs src/graph_view/tests.rs
printf '%s\n' '== Packaged-install and Polonius guidance =='
rg -n -i -C4 'cargo\s*(\+[^[:space:]]+\s*)?install|install\s+netsuke|registry|Zpolonius|nightly-2026-06-25|POLONIUS' \
README.md Cargo.toml docs AGENTS.md .cargo/config.toml rust-toolchain.toml || trueLength of output: 74717
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '== Documentation contract-test map =='
ast-grep outline tests/documentation_examples_tests.rs --items all --view expanded
printf '%s\n' '== Registry-install example assertions and fixture discovery =='
rg -n -i -C6 'readme-crates-io-install|guide-crates-io-install|RUSTFLAGS=-Zpolonius=next|cargo \+nightly-2026-06-25 install netsuke|rustup toolchain install nightly-2026-06-25' \
tests README.md docs/users-guide.md
printf '%s\n' '== All documentation-example test call sites =='
rg -n -C4 'tested-example|documentation_examples|README\.md|users-guide\.md' tests \
-g '*.rs' -g '*.md' || trueLength of output: 753
@leynos, do not resolve this thread. The latest commit fixes the README and users’ guide, but docs/quickstart.md still tells users to install via bare cargo install netsuke (Line 11). That registry build receives neither the pinned nightly nor -Zpolonius=next, while the NodePathRegistry::ensure_node_mut code path remains explicitly Polonius-only.
The documentation-contract-test inspection also failed because repository setup was unavailable, so its coverage remains unverified.
Use this AI-agent prompt to finish the remaining work:
Fix the remaining unsupported crates.io installation instruction for Netsuke.
Context:
- `Cargo.toml` packages neither `.cargo/config.toml` nor `rust-toolchain.toml`.
- `src/graph_view/mod.rs::NodePathRegistry::ensure_node_mut` requires `-Zpolonius=next`; it is intentionally rejected by NLL.
- The supported registry installation command is:
rustup toolchain install nightly-2026-06-25
RUSTFLAGS=-Zpolonius=next cargo +nightly-2026-06-25 install netsuke
- `README.md` and `docs/users-guide.md` already use this command.
- `docs/quickstart.md` still says `cargo install netsuke`, which is unsupported and must not remain as a standalone installation instruction.
Required changes:
1. Update `docs/quickstart.md` to direct users to the supported pinned-nightly and Polonius registry-install command, or link unambiguously to the canonical installation instructions in `docs/users-guide.md`.
2. Inspect `tests/documentation_examples_tests.rs` and its helpers/fixtures. Add or update a contract test so every tagged registry-install example requires both:
- `cargo +nightly-2026-06-25`
- `RUSTFLAGS=-Zpolonius=next`
3. Search all user-facing documentation for `cargo install netsuke`. Replace any bare registry-install command or explicitly scope it to a checkout-local `cargo install --path .` invocation where the repository configuration applies.
4. Run the project’s documentation validation and relevant test target under the pinned nightly with Polonius enabled.
5. Report the changed files, the exact validation commands, and their results.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.github/workflows/coverage-main.yml:
- Line 27: Disable persisted checkout credentials by adding persist-credentials:
false under the checkout steps in .github/workflows/coverage-main.yml lines
27-27 and .github/workflows/netsukefile-test.yml lines 22-23.
In `@docs/polonius.md`:
- Around line 23-29: Update the validation statement in the documented build
procedure to distinguish per-change no-flag compilation from the full
behavioural suite: use the no-flag build only to classify whether an individual
change depends on Polonius, and require the complete behavioural suite to run
under -Zpolonius=next. Keep the existing POLONIUS(...) classification and
toolchain outcome guidance consistent with this contract.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3e0943fc-fe41-40d0-8fbd-8d0c42fe9a20
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (28)
.cargo/config.toml.github/workflows/build-and-package.yml.github/workflows/ci.yml.github/workflows/coverage-main.yml.github/workflows/netsukefile-test.ymlAGENTS.mdCHANGELOG.mdCargo.tomlMakefileREADME.mddocs/adr-006-adopt-polonius-nightly-toolchain.mddocs/contents.mddocs/developers-guide.mddocs/netsuke-design.mddocs/polonius.mdrust-toolchain.tomlsrc/graph_view/mod.rssrc/graph_view/tests.rssrc/ir/cycle.rssrc/ir/from_manifest_support.rssrc/stdlib/collections.rssrc/stdlib/which/cache.rssrc/stdlib/which/env.rssrc/stdlib/which/lookup/mod.rssrc/stdlib/which/lookup/tests.rssrc/stdlib/which/resolve_error.rstests/kani_cfg_ui_tests.rstests/ui/cfg_kani_policy_pass.rs
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/rstest-bdd(auto-detected)leynos/ortho-config(auto-detected)leynos/shared-actions(auto-detected)
💤 Files with no reviewable changes (1)
- Cargo.toml
|
@coderabbitai Have the following failed checks now been resolved? If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures. Do not treat warnings as optional or aspirational. Where a change is out of scope for this PR, propose a GitHub issue unless one exists already. (Treat documentation and validation coverage as in scope). ❌ Failed checks (1 warning)
|
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Ensure that this is validated against the current version of the codegraph. If further refinement to address this finding would be deleterious, please supply a clear explanatory one to two paragraph markdown message I can paste into the CodeScene web ui's diagnostic suppression function so this diagnostic can be silenced. tests/documentation_examples_tests.rs Comment on lines +157 to +163 // Registry installs run outside a checkout, so the packaged source sees
// neither rust-toolchain.toml nor .cargo/config.toml; the documented
// command must select the pinned nightly and the Polonius flag itself.
let expected_release = concat!(
"rustup toolchain install nightly-2026-06-25\n",
"RUSTFLAGS=-Zpolonius=next cargo +nightly-2026-06-25 install netsuke\n"
);❌ New issue: Large Method |
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
This comment was marked as resolved.
This comment was marked as resolved.
|
@coderabbitai review The "User-Facing Documentation" pre-merge warning is stale: commit 69efa1f updated |
This comment was marked as resolved.
This comment was marked as resolved.
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- current PR head ---'
git rev-parse HEAD
git status --short
printf '%s\n' '--- user-guide source-install/toolchain references ---'
rg -n -i -C 5 'Rust 1\.89|Rust [0-9]+\.[0-9]+|nightly-2026-06-25|Polonius|cargo \+nightly|RUSTFLAGS|crates\.io|source' docs/users-guide.md
printf '%s\n' '--- README source-install/toolchain references ---'
rg -n -i -C 5 'Rust 1\.89|Rust [0-9]+\.[0-9]+|nightly-2026-06-25|Polonius|cargo \+nightly|RUSTFLAGS|crates\.io|source' README.md
printf '%s\n' '--- focused documentation contract test ---'
ast-grep outline tests/documentation_examples_tests.rs --items all --view expanded
rg -n -A125 -B10 'fn installation_examples_match_source_and_release_contracts\s*\(' \
tests/documentation_examples_tests.rs
printf '%s\n' '--- commit claimed by the user, if present locally ---'
git show --no-patch --format='%H%n%s' 69efa1f 2>&1 || trueLength of output: 1025
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/graph_view/mod.rs (1)
201-227: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared per-output
EdgeViewinsertion into one helper.
register_input_edgesandregister_dependenciesboth loopfor out in all_outputs(self.edge)and push anEdgeView { from: ..., to: out.clone(), class }. Factor the inner insertion into a single method (e.g.fn insert_edges(&mut self, from: &Utf8PathBuf, class_for: impl Fn(&Utf8PathBuf) -> EdgeClass)) taking the class either as a fixed value or a small closure, and call it from both sites.♻️ Sketch of the consolidation
impl EdgeRegistrar<'_> { + fn insert_edges_from(&mut self, from: &Utf8PathBuf, class_for: impl Fn(&Utf8PathBuf) -> EdgeClass) { + for out in all_outputs(self.edge) { + self.edges.insert(EdgeView { + from: from.clone(), + to: out.clone(), + class: class_for(out), + }); + } + } + fn register_input_edges(&mut self, input: &Utf8PathBuf, implicit: &BTreeSet<&Utf8PathBuf>) { - for out in all_outputs(self.edge) { - let class = if implicit.contains(out) { - EdgeClass::ImplicitOutput - } else { - EdgeClass::Explicit - }; - self.edges.insert(EdgeView { - from: input.clone(), - to: out.clone(), - class, - }); - } + self.insert_edges_from(input, |out| { + if implicit.contains(out) { + EdgeClass::ImplicitOutput + } else { + EdgeClass::Explicit + } + }); } fn register_dependencies(&mut self, deps: &[Utf8PathBuf], class: EdgeClass) { for dep in deps { self.registry.ensure_node_mut(dep); - for out in all_outputs(self.edge) { - self.edges.insert(EdgeView { - from: dep.clone(), - to: out.clone(), - class, - }); - } + self.insert_edges_from(dep, |_| class); } } }Also applies to: 236-295
🤖 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 `@src/graph_view/mod.rs` around lines 201 - 227, Extract the duplicated per-output EdgeView insertion from register_input_edges and register_dependencies into a shared insert_edges method on the relevant view type, accepting the source path and either a fixed EdgeClass or class-producing closure. Have both callers delegate to this helper while preserving their existing class selection and all_outputs(self.edge) iteration behavior.src/stdlib/which/env.rs (1)
59-74: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAvoid adding
cwdtwice inCwdMode::Always.resolved_dirs()already prependsself.cwd, so a literal.inPATHadds the same directory again. That can return duplicate matches withall=trueand rescan the same directory unnecessarily; skipCurrentDirwhenmodeisAlways.🤖 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 `@src/stdlib/which/env.rs` around lines 59 - 74, Update resolved_dirs so PathEntry::CurrentDir is ignored when mode is CwdMode::Always, since self.cwd is already prepended in that mode; retain the existing CurrentDir handling for CwdMode::Auto and omission for other modes.
🤖 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 `@docs/developers-guide.md`:
- Around line 83-87: Update the documentation paragraph around the rustup and
.cargo/config.toml explanation to state that the configuration supplies the flag
by default, rather than applying it to every Cargo invocation. Preserve the
existing clarification that inherited RUSTFLAGS overrides .cargo/config.toml and
that Makefile recipes use POLONIUS_FLAGS.
---
Outside diff comments:
In `@src/graph_view/mod.rs`:
- Around line 201-227: Extract the duplicated per-output EdgeView insertion from
register_input_edges and register_dependencies into a shared insert_edges method
on the relevant view type, accepting the source path and either a fixed
EdgeClass or class-producing closure. Have both callers delegate to this helper
while preserving their existing class selection and all_outputs(self.edge)
iteration behavior.
In `@src/stdlib/which/env.rs`:
- Around line 59-74: Update resolved_dirs so PathEntry::CurrentDir is ignored
when mode is CwdMode::Always, since self.cwd is already prepended in that mode;
retain the existing CurrentDir handling for CwdMode::Auto and omission for other
modes.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 451ed985-e12c-40b6-b8f0-22c68261325f
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (30)
.cargo/config.toml.github/workflows/build-and-package.yml.github/workflows/ci.yml.github/workflows/coverage-main.yml.github/workflows/netsukefile-test.ymlAGENTS.mdCHANGELOG.mdCargo.tomlMakefileREADME.mddocs/adr-006-adopt-polonius-nightly-toolchain.mddocs/contents.mddocs/developers-guide.mddocs/netsuke-design.mddocs/polonius.mddocs/users-guide.mdrust-toolchain.tomlsrc/graph_view/mod.rssrc/graph_view/tests.rssrc/ir/cycle.rssrc/ir/from_manifest_support.rssrc/stdlib/collections.rssrc/stdlib/which/cache.rssrc/stdlib/which/env.rssrc/stdlib/which/lookup/mod.rssrc/stdlib/which/lookup/tests.rssrc/stdlib/which/resolve_error.rstests/documentation_examples_tests.rstests/kani_cfg_ui_tests.rstests/ui/cfg_kani_policy_pass.rs
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/rstest-bdd(auto-detected)leynos/ortho-config(auto-detected)leynos/shared-actions(auto-detected)
💤 Files with no reviewable changes (1)
- Cargo.toml
019181f to
27a53ff
Compare
Pin `nightly-2026-06-25` in `rust-toolchain.toml` and enable the Polonius alpha borrow-checking analysis (`-Zpolonius=next`) for every build path: - `.cargo/config.toml` applies the flag to plain Cargo invocations, rust-analyzer, and `cargo kani`. - Makefile recipes that set `RUSTFLAGS` (which overrides the config table) re-state the flag via the new `POLONIUS_FLAGS` variable. - CI collapses the stable/MSRV matrices in `ci.yml` and `netsukefile-test.yml` to the pinned nightly and aligns `coverage-main.yml`. - `Cargo.toml` drops `rust-version = "1.89.0"`, which cannot express a nightly requirement. ADR-006 records the policy decision and its release consequences. The developers' guide, design document, README, documentation index, and a new `CHANGELOG.md` document the toolchain contract. This prepares the borrow-centric API evolution tracked by issue #465. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Introduce `NodePathRegistry` in `src/graph_view/mod.rs` and route the three `entry(path.clone()).or_insert(NodeKind::Source)` sites through `ensure_node_mut`, a get-or-insert accessor that returns `&mut NodeKind`, performs one lookup on the hit path, and clones the path only on insertion. Target registration moves behind `insert_target` so callers pass borrowed paths throughout. The accessor is the canonical Polonius problem-case-3 shape: the `get_mut` loan escapes only via the early return, so the insertion on the miss path is accepted under `-Zpolonius=next` and rejected by NLL with E0499. Both outcomes were compiler-verified on nightly-2026-06-25, and the site carries a `POLONIUS(case-3)` tag with anti-regression guidance. Behaviour is unchanged: the graph_view unit, snapshot, and property suites pass as-is, and a new parameterized test pins the hit and miss semantics of the accessor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`group_by_filter` cloned each resolved key value before passing it to `IndexMap::entry`, but the key is not used again afterwards, so the entry call can consume it directly. Classification for the Polonius audit: this compiles under plain NLL as well as `-Zpolonius=next`, so the clone was habit rather than a borrow-checker workaround; no toolchain caveat applies. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`CycleDetector::detect_targets` collected an owned clone of every target path purely to sort them for deterministic traversal. The target map is borrowed for the whole `'targets` lifetime, so the snapshot can hold `&'targets Utf8Path` references instead, removing one allocation per node on every cycle analysis. Classification for the Polonius audit: this compiles under plain NLL as well as `-Zpolonius=next` because the collection exists for sorting, not to end a borrow; the owned snapshot was habit. Behaviour is unchanged and the cycle unit and property suites pass as-is. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`EnvSnapshot::resolved_dirs` cloned the current working directory and every `PATH` entry into an owned `Vec<Utf8PathBuf>` for each uncached resolution, although the search loop and miss diagnostics only read the directories. Return `Vec<&Utf8Path>` borrowed from the snapshot instead, thread the borrowed slice through `HandleMissContext`, and copy the paths into the owned `ResolveError::NotFound` only at the error boundary, where the diagnostic genuinely outlives the snapshot. Classification for the Polonius audit: compiles under plain NLL as well as `-Zpolonius=next` — the owned returns were habit, not a borrow-checker workaround. The lookup tests compare against borrowed paths now; the assertions themselves are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Record the audit's deliberate refusals in source so future passes (by humans or agents) do not convert them into borrow-returning forms: - `register_action` keeps returning the owned action hash because the hash is persistent IR identity stored on every `BuildEdge` and named in the generated Ninja file: `POLONIUS-REFUSED(id-is-data)`. - The which resolver's cache clones hits out of the LRU because a reference cannot outlive the `MutexGuard`: `POLONIUS-REFUSED(lock-boundary)`. - `GroupedValues::new` keeps the owned-key `entry` form because the first-wins registration is miss-dominant: `POLONIUS-REFUSED(miss-dominant)`. No behaviour changes; comments only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add `docs/polonius.md` recording the complete two-pass audit: the compiler-verified `POLONIUS(case-3)` site in graph_view, the pass-both evolutions (group_by key, cycle-detection snapshot, which resolver directories), the tagged refusals with their permanent constraints, the non-candidates reviewed and cleared, clone counts before and after (158 to 151 in `src/`), and the stabilization checklist. Add a "Borrow checker: Polonius, not NLL" section to `AGENTS.md` so coding agents do not rewrite borrow-centric sites into double lookups, defensive clones, or id indirection out of NLL-era habit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The `cases.pass()` trybuild case rebuilt `netsuke` as a dependency of its scratch project, and trybuild discards ambient `RUSTFLAGS` and workspace `build.rustflags` in favour of its own `--config` flags, so the dependency compiled under plain NLL and rejected the `POLONIUS(case-3)` accessor in graph_view (E0499). The fixture only asserts on `include_str!` copies of `Cargo.toml` and the `Makefile` and has no dependencies, so compile it with the workspace `rustc` through the same harness the sibling check-cfg tests use, run the produced binary, and assert both steps succeed. The policy assertions are unchanged; drop the now-unused `trybuild` dev-dependency. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Document that trybuild replaces workspace rustflags and so cannot build the Polonius-only crate as a fixture dependency, and that Kani and Whitaker inherit the flag through the workspace configuration. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CI failed at `make lint`: `actions-rust-lang/setup-rust-toolchain` (nested inside the shared setup-rust action) exports `RUSTFLAGS="-D warnings"` into the job environment when the variable is unset, and an ambient `RUSTFLAGS` overrides the `.cargo/config.toml` `build.rustflags` table. `make lint-clippy` did not re-state the flags, so Clippy compiled under plain NLL and rejected the tagged `POLONIUS(case-3)` accessor with E0499. Fix both layers: - Pre-set `RUSTFLAGS` (including `-Zpolonius=next`) at job level in `ci.yml`, `netsukefile-test.yml`, and `coverage-main.yml`. The setup action defers to an existing value, and `cargo-llvm-cov` appends its instrumentation flags to it, so coverage inherits the flag too. - Harden the Makefile: `lint-clippy` re-states the flags for Clippy, and the `cargo doc` and binary-build recipes append `$(POLONIUS_FLAGS)` to any ambient `RUSTFLAGS` so a hostile environment can never strip the analysis. Verified locally by running `make lint-clippy` and `make build` with `RUSTFLAGS="-D warnings"` exported, reproducing the CI environment; both now pass. The harness-consequences section of `docs/polonius.md` records the CI behaviour and flags the shared mutation workflow as the remaining environment outside this repository's control. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CodeScene's delta gate flagged `register_inputs_and_edges` as a bumpy road: three sibling loops each nesting conditional logic. Replace the free function with an `EdgeRegistrar` that groups the edge with the mutable projection state, splits input registration from dependency-class registration, and shares the duplicated output chain through a single `all_outputs` iterator (also reused by `register_outputs`). Behaviour is unchanged: the graph_view unit, snapshot, and property suites pass as-is, the CodeScene delta reports no issues, and the `POLONIUS(case-3)` accessor still verifies both ways (E0499 under plain NLL). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The release dry run failed: `rust-build-release` invokes the shared setup-rust composite, whose nested `setup-rust-toolchain` exports `RUSTFLAGS="-D warnings"` into the job environment when the variable is unset, shadowing `.cargo/config.toml` and stripping `-Zpolonius=next` from the release cargo build (E0499 at the tagged `POLONIUS(case-3)` site). Pre-set `RUSTFLAGS: -Zpolonius=next` at job level in `build-and-package.yml`, mirroring the fix already applied to the CI, coverage, and Netsukefile workflows: the setup action defers to an existing value, so every nested cargo invocation inherits the flag. A companion shared-actions change exposing a `rustflags` passthrough input is being submitted separately. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Disable persisted checkout credentials in `coverage-main.yml` and `netsukefile-test.yml`, matching the hardened checkout in `ci.yml`; neither job pushes, so the cached token serves no purpose. - Clarify the validation contract in `docs/polonius.md`: the no-flag compile exists only to classify whether an individual change depends on Polonius, while the complete behavioural suite runs under `-Zpolonius=next`, the tree's only supported configuration. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The crates.io package excludes `rust-toolchain.toml` and
`.cargo/config.toml` (and registry builds run outside a checkout, so
Cargo would not apply them regardless), meaning a bare
`cargo install netsuke` of a Polonius-dependent release would compile
under the user's default toolchain and fail with E0499. Keeping the
distributed source NLL-compatible would forfeit the borrow-centric
design this branch exists to adopt, so document and pin the explicit
install command instead:
rustup toolchain install nightly-2026-06-25
RUSTFLAGS=-Zpolonius=next cargo +nightly-2026-06-25 install netsuke
Update the tested-example blocks in the README and users' guide, the
contract expectation in `documentation_examples_tests.rs`, the ADR-006
release consequence, the polonius harness notes, and the changelog.
The users' guide also loses a stale "Rust 1.89 or later" prerequisite.
Source installs from a checkout are unaffected: the pinned toolchain
and workspace configuration apply there.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Reword the developers' guide so `.cargo/config.toml` is described as supplying `-Zpolonius=next` by default rather than applying it to every Cargo invocation, keeping the existing note that an inherited `RUSTFLAGS` overrides the table and Makefile recipes re-state it via `POLONIUS_FLAGS`. - Extract the duplicated per-output `EdgeView` insertion in `EdgeRegistrar` into a shared `insert_edges` helper taking the source path and a class-producing closure; both callers keep their class selection and `all_outputs` iteration. - Stop `resolved_dirs` from listing the working directory twice under `CwdMode::Always`: the mode already prepends the working directory, so a current-directory `PATH` entry (an empty component) only duplicated it in search order and miss diagnostics. `CwdMode::Auto` behaviour is unchanged, and a regression test pins both modes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`installation_examples_match_source_and_release_contracts` reached 71 lines after gaining the registry-install expectations, tripping CodeScene's large-method threshold. Move the Windows help, PATH, and staging assertions into an `assert_windows_setup_examples` helper; the assertions themselves are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Move the crates.io install-command and release-detail assertions into a file-local `assert_release_installation_contract` helper, called first from `installation_examples_match_source_and_release_contracts`, resolving the CodeScene large-method finding. Expected strings, error messages, and assertions are unchanged; the source-install checks stay in the test and the Windows checks remain in the existing helper. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The quickstart still suggested a bare `cargo install netsuke`, which registry builds cannot satisfy: the crates.io package carries neither `rust-toolchain.toml` nor `.cargo/config.toml`, and the tree needs `-Zpolonius=next`. Point the prerequisite at the checkout-scoped source build and the canonical registry-install command in the users' guide instead. Add a contract test requiring every tagged registry-install example to select `cargo +nightly-2026-06-25` and pass `RUSTFLAGS=-Zpolonius=next` (checkout-scoped `cargo install --path .` examples are exempt), and guarding the quickstart prose against reintroducing a bare registry install. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The weave-assisted rebase left two consecutive blank lines ahead of the "Toolchain and borrow checker" heading in the developers' guide, failing markdownlint MD012. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The weave-assisted rebase again left two consecutive blank lines in the developers' guide, this time ahead of the "Temporary executable test helpers" section that main introduced, failing markdownlint MD012. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
7c483f1 to
7618f5c
Compare
Summary
This branch migrates Netsuke to the Polonius alpha borrow-checking analysis (
-Zpolonius=next) and uses the migration to evolve internal APIs towards a borrow-centric ownership model (Mode E: model evolution).Closes #465.
The dated nightly
nightly-2026-06-25is pinned inrust-toolchain.toml, and the flag flows through every build path:.cargo/config.tomlfor plain Cargo, rust-analyzer, andcargo kani; thePOLONIUS_FLAGSvariable in theMakefilefor recipes that setRUSTFLAGS; and the pinned toolchain inci.yml,netsukefile-test.yml, andcoverage-main.yml. The stable/MSRV CI matrices are removed andrust-versionis dropped fromCargo.toml, since Cargo cannot express a nightly requirement; ADR-006 records the policy decision and its release consequences.The
nll-to-poloniustwo-pass audit was run with the compiler as the oracle; every change was verified with and without the flag:NodePathRegistry::ensure_node_mutinsrc/graph_view/mod.rsreplaces the threeentry(path.clone()).or_insert(...)sites with a&mut NodeKind-returning accessor: one lookup on hits, a path clone only on insertion. It passes with-Zpolonius=nextand is rejected by NLL with E0499 (taggedPOLONIUS(case-3)).group_byconsumes its key instead of cloning (src/stdlib/collections.rs); cycle detection snapshots borrowed keys (src/ir/cycle.rs); thewhichresolver borrows its search directories and copies them into the owned error only at the miss boundary (src/stdlib/which/env.rs).POLONIUS-REFUSED(id-is-data)insrc/ir/from_manifest_support.rs), cache hits stay cloned out of the LRU mutex (lock-boundary), and the miss-dominant string-key registration keeps itsentryform.Clone counts fell from 158 to 151 in
src/(graph_view 17→14); the clone-modify-writeback scan was empty before and after.Review walkthrough
docs/polonius.mdfor the complete audit record: rewritten sites, pass-both evolutions, refusals, non-candidates, clone counts, and the stabilization checklist.src/graph_view/mod.rsfor the Polonius-dependent registry andsrc/graph_view/tests.rsfor the new hit/miss accessor test.rust-toolchain.toml,.cargo/config.toml,Makefile, workflows).tests/kani_cfg_ui_tests.rs: trybuild discards workspace rustflags and rebuilt the crate under NLL, so the policy fixture is now compiled and run directly with the workspacerustc; its assertions are unchanged and thetrybuilddev-dependency is removed.AGENTS.mdand the developers' guide.Validation
make check-fmt,make lint(rustdoc, Clippy, Whitaker),make test,make markdownlint, andmake nixieall pass under the pinned nightly with-Zpolonius=next.cargo checkwithout the flag fails only at the taggedPOLONIUS(case-3)site with E0499, confirming the design genuinely depends on Polonius; each pass-both evolution was verified to compile under plain NLL.coderabbit review --agentran after each milestone (toolchain adoption; API evolution and documentation) with zero findings on both passes.Notes
.cargo/config.toml, socargo kaniborrow-checks with the same analysis.References
Summary by Sourcery
Adopt a Polonius-based, borrow-centric ownership model and pin the project to a nightly Rust toolchain, updating graph view APIs, tooling, and documentation accordingly.
Enhancements:
Build:
CI:
Documentation:
Tests:
Chores: