Repository navigation
test: speed make coverage-rust via profile align and parallel acceptance - #391
Merged
Merged
Conversation
Align core llvm-cov with the release adapter profile and run independent acceptance lanes concurrently while retaining verified artifact and coverage evidence. Add fail-closed resume stamps and opt-in HTML so repeat local runs preserve floor and ledger integrity without adding full coverage to PR CI. The runner records same-SHA per-phase timings in build/coverage-rust/timing.log; an A/B full-run ledger was not captured in this change because the prior run can exceed the local execution window. Closes #388 Co-authored-by: Cursor <cursoragent@cursor.com>
WalkthroughThe Rust coverage runner now supports resumable stamped phases, optional HTML output, release-mode coverage, parallel Python and Node acceptance tests, and timing records. A strict validator runs during ChangesRust coverage workflow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant coverage_rust.sh
participant cargo_llvm_cov
participant native_adapters
participant Python_acceptance
participant Node_acceptance
coverage_rust.sh->>cargo_llvm_cov: run release-mode core coverage
cargo_llvm_cov-->>coverage_rust.sh: produce coverage outputs
coverage_rust.sh->>native_adapters: build or resume stamped artifacts
native_adapters-->>coverage_rust.sh: return validated artifacts
coverage_rust.sh->>Python_acceptance: start instrumented acceptance phase
coverage_rust.sh->>Node_acceptance: start instrumented acceptance phase
Python_acceptance-->>coverage_rust.sh: return phase status
Node_acceptance-->>coverage_rust.sh: return phase status
Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@scripts/ci/test-coverage-rust.sh`:
- Around line 20-23: Update the ripgrep pattern in the CI coverage check to
match “make pre-push” only when it is a complete command, requiring a command
boundary after `pre-push`; continue rejecting coverage and exact `make pre-push`
references while allowing `make pre-push-fast`.
In `@scripts/coverage-rust.sh`:
- Around line 134-143: Update the native-artifacts stamp handling around
stamp_matches and write_stamp to include the generated Python and Node native
artifact paths. Ensure stamp_matches validates that both artifacts exist and
their hashes match the recorded values before resuming; otherwise rerun both
build commands and record the artifacts with write_stamp.
- Around line 214-232: Update the unmatched resume branches around the Python
and Node acceptance phases to remove that phase’s existing raw profiles and
merged profile before launching its rerun. Ensure the Python branch cleans only
Python artifacts and the Node branch cleans only Node artifacts, while leaving
the matching-stamp resume paths unchanged.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 83910e59-65bb-455b-81e4-ac78467b3d6a
📒 Files selected for processing (3)
Makefilescripts/ci/test-coverage-rust.shscripts/coverage-rust.sh
Comment on lines
+20
to
+23
| if rg -n --glob '*.{yml,yaml}' 'coverage-rust|make coverage|make pre-push' "$ROOT/.github"; then | ||
| echo "Rust coverage must remain outside PR CI" >&2 | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not reject make pre-push-fast.
The make pre-push alternative also matches make pre-push-fast. That target explicitly excludes coverage, so it is valid for PR CI.
Require a command boundary after pre-push.
Proposed fix
-if rg -n --glob '*.{yml,yaml}' 'coverage-rust|make coverage|make pre-push' "$ROOT/.github"; then
+if rg -n --glob '*.{yml,yaml}' \
+ 'coverage-rust|make coverage([[:space:]]|$)|make pre-push([[:space:]]|$)' \
+ "$ROOT/.github"; then📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if rg -n --glob '*.{yml,yaml}' 'coverage-rust|make coverage|make pre-push' "$ROOT/.github"; then | |
| echo "Rust coverage must remain outside PR CI" >&2 | |
| exit 1 | |
| fi | |
| if rg -n --glob '*.{yml,yaml}' \ | |
| 'coverage-rust|make coverage([[:space:]]|$)|make pre-push([[:space:]]|$)' \ | |
| "$ROOT/.github"; then | |
| echo "Rust coverage must remain outside PR CI" >&2 | |
| exit 1 | |
| fi |
🤖 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 `@scripts/ci/test-coverage-rust.sh` around lines 20 - 23, Update the ripgrep
pattern in the CI coverage check to match “make pre-push” only when it is a
complete command, requiring a command boundary after `pre-push`; continue
rejecting coverage and exact `make pre-push` references while allowing `make
pre-push-fast`.
Comment on lines
+134
to
+143
| native_input="$(printf '%s\n' "$CORE_COVERAGE_ARGS" "$COVERAGE_ENV" | shasum -a 256)" | ||
| if stamp_matches native-artifacts "$native_input"; then | ||
| echo "Resuming verified native artifact build" | ||
| else | ||
| phase_started=$SECONDS | ||
| uv run maturin develop --release -m crates/graphforge-bindings-py/Cargo.toml | ||
| pnpm --filter @curatelabs/graphforge exec napi build --platform --release | ||
| write_stamp native-artifacts "$native_input" | ||
| record_timing native-artifacts "$phase_started" | ||
| fi |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Validate native artifacts before resuming the phase.
stamp_matches receives no output paths for native-artifacts. The stamp therefore remains valid when the built artifacts are missing or replaced.
Record the Python and Node artifacts in this stamp. Require both files to exist and match their recorded hashes before the runner skips the builds.
🤖 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 `@scripts/coverage-rust.sh` around lines 134 - 143, Update the native-artifacts
stamp handling around stamp_matches and write_stamp to include the generated
Python and Node native artifact paths. Ensure stamp_matches validates that both
artifacts exist and their hashes match the recorded values before resuming;
otherwise rerun both build commands and record the artifacts with write_stamp.
Comment on lines
+214
to
+232
| if stamp_matches python-acceptance "$python_input" "${PROFILE_DIR}"/python-*.profraw; then | ||
| echo "Resuming verified Python native acceptance" | ||
| else | ||
| ( | ||
| phase_started=$SECONDS | ||
| run_python_acceptance | ||
| write_stamp python-acceptance "$python_input" "${PROFILE_DIR}"/python-*.profraw | ||
| record_timing python-acceptance "$phase_started" | ||
| ) & | ||
| python_pid=$! | ||
| fi | ||
| if stamp_matches node-acceptance "$node_input" "${PROFILE_DIR}"/node-*.profraw; then | ||
| echo "Resuming verified Node native acceptance" | ||
| else | ||
| ( | ||
| phase_started=$SECONDS | ||
| run_node_acceptance | ||
| write_stamp node-acceptance "$node_input" "${PROFILE_DIR}"/node-*.profraw | ||
| record_timing node-acceptance "$phase_started" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Remove invalid phase profiles before an acceptance rerun.
When a resume stamp does not match, the runner keeps existing python-*.profraw or node-*.profraw files. The new processes create different filenames. The ledger can then reject stale profiles that predate the current artifact.
Remove only the affected phase profiles and merged profile before starting that phase.
Proposed fix
else
(
phase_started=$SECONDS
+ rm -f "$PROFILE_DIR"/python-*.profraw "$OUT_DIR/python.profdata"
run_python_acceptance
write_stamp python-acceptance "$python_input" "${PROFILE_DIR}"/python-*.profraw
record_timing python-acceptance "$phase_started"
@@
else
(
phase_started=$SECONDS
+ rm -f "$PROFILE_DIR"/node-*.profraw "$OUT_DIR/node.profdata"
run_node_acceptance
write_stamp node-acceptance "$node_input" "${PROFILE_DIR}"/node-*.profraw
record_timing node-acceptance "$phase_started"🤖 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 `@scripts/coverage-rust.sh` around lines 214 - 232, Update the unmatched resume
branches around the Python and Node acceptance phases to remove that phase’s
existing raw profiles and merged profile before launching its rerun. Ensure the
Python branch cleans only Python artifacts and the Node branch cleans only Node
artifacts, while leaving the matching-stamp resume paths unchanged.
This was referenced Aug 7, 2026
5 of 6 tasks
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
1 participant
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.
Summary
cargo llvm-covwith instrumented release profile.tests/*.pyafter hash checks.COVERAGE_RUST_HTML=1); fail-closed resume stamps (COVERAGE_RUST_RESUME=1).Closes #388.
Test plan
make pre-push-fastbash scripts/ci/test-coverage-rust.shuv run --no-sync python scripts/ci/test-rust-coverage-ledger.pyBDD evidence
--release; ledger floors still enforced by existing ledger tests.*.pyparallelized inscripts/coverage-rust.sh.Made with Cursor
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Note
Speed up Rust coverage via parallel acceptance, resume stamps, and profile alignment
COVERAGE_RUST_RESUME=1) to skip already-completed phases when outputs match.--releaseby default; the HTML report is opt-in viaCOVERAGE_RUST_HTML=1to avoid unnecessary output.build/coverage-rust/timing.log; Python binding tests run per-file in parallel viaxargs -Pwith a configurablePYTHON_BINDING_WORKERScount, and the phase fails fast if no binding tests are found.pre-pushMakefile target; it validates thatcoverage-rust.shcontains required commands and that GitHub workflows do not invoke Rust coverage directly.Macroscope summarized 32654a9.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation