Repository navigation
Fix newly reachable const lint findings - #409
Conversation
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (4)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Summary
WalkthroughUpdate selected Rust operations for const evaluation. Make ChangesConst evaluation updates
Priority: ⬇️ Low Change: Refactor · Unblocks: 3 PRs Merge Risk: ⚪ Minimal · up to The const updates and aligned test fixtures do not show a merge-blocking behavior change. 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
Full details: Developer DocumentationExplanation Fail this check because the pull request changes the crate-visible Resolution Add a section to Const paths now open Comment |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis prerequisite patch addresses newly reachable const and assertion lint findings by const-qualifying private pump helpers and the test-only close-log accessor, using Rust 1.85-compatible File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![codescene-access[bot]](https://avatars.githubusercontent.com/in/3755875?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
b598ec6 to
2552846
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
2552846 to
c49d9db
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
Keep the pump state-machine helpers const-safe on Rust 1.85 and retain the byte-exact privacy diagnostic after its signature changes.
CodeRabbit's pre-merge walkthrough withheld the Developer Documentation check because the const work changed a crate-visible API without a guide entry. Record why `CloseLog::closes` consumes its log and why the pump helpers are `const`, so the next reader does not mistake either for an ownership or visibility change. The workspace denies `missing_const_for_fn`, so an accessor with no reason to run at runtime must be `const`. `Cell::get` is not const-stable on the pinned Rust 1.85.0 toolchain, while `Cell::into_inner` is, and reaching it requires taking `self` by value. `CloseLog` stays `pub(crate)` inside a `#[cfg(any(test, kani))]` module and every caller reads the log once as its final use, so the model's unit tests and Kani proofs keep their call sites. `const` on `pump_machine::step` and `apply_write` qualifies how a function may be called, not who may call it, so neither helper widens its visibility and the pump contract is unchanged. The compile-fail case that pins that encapsulation boundary still fails if either item is widened, even to `pub(crate)`; only its quoted definition line moved.
c49d9db to
a10602e
Compare
|
@coderabbitai — focused follow-up on the Developer Documentation pre-merge row, Head is now The resolution the row asks for now exists.
Please re-evaluate the Developer Documentation row against head |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== HEAD =='
git rev-parse HEAD
printf '%s\n' '== Diff paths against main =='
git diff --name-status main...HEAD
printf '%s\n' '== Documentation section =='
rg -n -A45 -B5 'Const accessors in the test and Kani ownership model' docs/developers-guide.md
printf '%s\n' '== CloseLog declaration and calls =='
rg -n -A18 -B8 'struct CloseLog|fn closes\s*\(' rust/cuprum-native-io/src/fd_ownership_model.rs
rg -n -A3 -B3 '\.closes\s*\(' rust/cuprum-native-io/src rust/cuprum-native-io/tests || true
printf '%s\n' '== Pump helper declarations =='
rg -n -A14 -B8 'const fn step\s*\(|const fn apply_write\s*\(' rust/cuprum-streams/src/pump_machine.rs
printf '%s\n' '== UI privacy assertion =='
sed -n '1,220p' rust/cuprum-streams/tests/ui/fail/pump_transition_unreachable.rs
printf '%s\n' '== Documentation diff =='
git diff --unified=20 main...HEAD -- docs/developers-guide.mdLength of output: 1367 🏁 Script executed: #!/bin/bash
set -u
echo '== Commit topology =='
git show -s --format='HEAD %H%nparents %P%nsubject %s' HEAD
git cat-file -e 33bff5dbc68eac170848d5ef317c377a3198342c^{commit} 2>&1
echo "base_commit_present=$?"
echo '== Files changed by HEAD commit =='
git show --name-status --format='' HEAD
echo '== Documentation section =='
rg -n -A50 -B5 'Const accessors in the test and Kani ownership model' docs/developers-guide.md || true
echo '== CloseLog declaration =='
rg -n -A20 -B8 'struct CloseLog|const fn closes\s*\(' rust/cuprum-native-io/src/fd_ownership_model.rs || true
echo '== CloseLog callers =='
rg -n -A3 -B3 '\.closes\s*\(' rust/cuprum-native-io || true
echo '== Pump helper declarations =='
rg -n -A16 -B8 'const fn step\s*\(|const fn apply_write\s*\(' rust/cuprum-streams/src/pump_machine.rs || true
echo '== UI compile-fail source =='
sed -n '1,220p' rust/cuprum-streams/tests/ui/fail/pump_transition_unreachable.rs
echo '== UI compile-fail expectation =='
sed -n '1,220p' rust/cuprum-streams/tests/ui/fail/pump_transition_unreachable.stderr
echo '== Docs patch in HEAD =='
git show --format='' --unified=12 HEAD -- docs/developers-guide.mdLength of output: 151301 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Documentation heading =='
rg -n 'Const accessors in the test and Kani ownership model' docs/developers-guide.md
echo '== CloseLog declaration =='
rg -n -A18 -B6 'struct CloseLog' rust/cuprum-native-io/src/fd_ownership_model.rs
echo '== CloseLog accessor and all calls =='
rg -n -A4 -B4 'const fn closes\s*\(|\.closes\(\)' rust/cuprum-native-io/src/fd_ownership_model.rs rust/cuprum-native-io/src/fd_ownership_kani_proofs.rs
echo '== Pump declarations =='
rg -n -A12 -B8 'const fn (step|apply_write)\s*\(' rust/cuprum-streams/src/pump_machine.rs
echo '== UI test references =='
rg -n -A8 -B8 'Transition::Wrote|step\(' rust/cuprum-streams/tests/ui/fail/pump_transition_unreachable.rs
echo '== UI expected private diagnostics =='
rg -n -A4 -B4 'private|step|apply_write' rust/cuprum-streams/tests/ui/fail/pump_transition_unreachable.stderr
echo '== Current documentation diff statistics =='
git diff --stat 33bff5dbc68eac170848d5ef317c377a3198342c..HEAD 2>&1 || true
git diff --name-only 33bff5dbc68eac170848d5ef317c377a3198342c..HEAD 2>&1 || trueLength of output: 13071
The checked-out head is The new section at Line 2552 documents both required points:
No further Developer Documentation action is required for this change. ✏️ Learnings added
You are interacting with an AI system. |
|
@coderabbitai approve |
✅ Action performedComments resolved and changes approved. |
Summary
This two-commit prerequisite layer fixes the const and assertion findings
exposed while preparing explicit dev-fast routing. It retains Rust 1.85
compatibility by consuming the private test-only close log with
Cell::into_inner(), makes the private pump helpers const-safe, and refreshesthe byte-exact privacy diagnostic without changing the privacy contract. A
follow-up documentation commit records why both accesses take that shape, so
the const qualifiers are not mistaken for ownership or visibility changes.
PR #412's floating shared spelling-builder policy is inherited through
PR #387 at
1a83587c914f0c603e42d6bed5d0bddb76c2534f. This layer does not alter thatpolicy or any spelling-builder configuration. PR #410
is the next stack layer.
Replay evidence
The owned commit,
Fix newly reachable const lint findings, was replayed fromthe preserved exclusive boundary
1a83587c914f0c603e42d6bed5d0bddb76c2534f..c49d9dba5d65a2756b42c824dfcc492912e66066onto
33bff5dbc68eac170848d5ef317c377a3198342c, the squash-merge of the parentPR #387 that is the current
origin/maintip. The replay is linear andconflict-free. Its stable patch ID remains
8ea3bda75a8cd92041f44a567fabb25cc2d662c6, and its child diff still hasexactly four paths, byte-identical to the pre-rebase result.
The boundary was established from three independent sources rather than from a
merge-base shortcut: PR #387's squash commit has a tree byte-identical to the
retained parent head
245c175a, the commit1a83587ccarries the same stablepatch ID as its counterpart
862c22b5in that retained history, and a priorrestack receipt for this branch records the same target. The intervening 76
main-side paths are untouched by this layer; all four owned paths are
byte-identical between the old base and the new target.
Review walkthrough
Cell::into_inner().constqualifier and why neither widens visibility.Validation
Each gate below ran sequentially against the exact head being pushed,
a10602e5a04edc2e8fff34f065bd182c73eba03d; all eight exited 0.make check-fmt: 496 files formatted, 65 Markdown files unchanged;make lint: ruff, interrogate 100%, pylint 10.00/10 (PyPy shim and thedf12 plugin), ambrleaks, skylos,
cargo doc -D warnings, clippy-D warnings, whitaker, spelling, yamllint, and actionlint;make typecheck:ty 0.0.74reports "All checks passed!";make test: 1,911 passed and 1 skipped of 1,912 collected, plus 125 of125 Rust tests under cargo-nextest — the skip is the Windows-only
winerrorcase, which is expected on Linux;make markdownlint: 65 files, 0 errors;make nixie: all 65 documents'Mermaid diagrams valid;
make boundary-test: 13 Rust and 45 pytest cases passed; andmake boundary-contractexited 0.No tracked file was modified by any gate.
The repository-pinned Rust 1.85 UI run passed the recorded expected
diagnostics. An ad hoc ambient stable trybuild run adds a definition span to
the unrelated inherited
replace_scoped_writer.stderrfixture. The changedpump_transition_unreachablefixture passed in that run; the ambient mismatchdoes not affect this replay. No
msrv-checktarget exists in this repository.Hosted checks and the current-head managed CodeRabbit review remain pending.
Earlier hosted checks and approval apply only to the prior candidate.
Train coordination and limits
shared-actions#496
remains open. It tracks the external deterministic CodeScene coverage CLI
installation problem; it is not a Cuprum source failure.
References