ci: restore eval reproducibility self-check - #418
Conversation
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
the
size:S
📝 WalkthroughSummaryThe PR restores the eval reproducibility self-check in the Rust CI job. It updates the Cargo lockfile checksum and adds integration coverage for the CI invocation. The changes are scoped and should merge. Must FixNone identified. Should FixNone identified. ConsiderThe integration test helps prevent future removal of the CI self-check. Approve / Request ChangesApprove. WalkthroughThe Rust CI workflow now runs the PowerShell reproducibility manifest self-check. The manifest checksum is updated, and Rust tests verify the workflow command and shell configuration. ChangesReproducibility self-check
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![codeant-ai[bot]](https://avatars.github.com/in/646884?s=60&v=4){kind=small}
| assert!( | ||
| ci_workflow.contains("shell: pwsh\n run: ./scripts/eval-repro-check.ps1 -SelfCheck"), | ||
| "ci.yml must invoke eval-repro-check.ps1 -SelfCheck with PowerShell" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn ci_workflow_runs_eval_repro_self_check_with_powershell() { | ||
| let ci_workflow = std::fs::read_to_string(repo_root().join(".github/workflows/ci.yml")) | ||
| .expect("expected ci.yml to be readable"); | ||
| assert!( | ||
| ci_workflow.contains("shell: pwsh\n run: ./scripts/eval-repro-check.ps1 -SelfCheck"), | ||
| "ci.yml must invoke eval-repro-check.ps1 -SelfCheck with PowerShell" | ||
| ); |
There was a problem hiding this comment.
Suggestion: This assertion only proves that a text fragment exists somewhere in ci.yml; it does not verify that the command belongs to the Rust job or that its failure is enforced. In the current workflow the Rust job is continue-on-error, so the test can pass while the SelfCheck is non-blocking. Validate the step's job placement and failure semantics, or avoid naming this as an enforcement check. [possible bug]
Severity Level: Major ⚠️
- ❌ Eval reproducibility regressions can merge undetected.
- ⚠️ Rust CI failures are explicitly advisory.
- ⚠️ The test verifies wiring, not enforcement semantics.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** tests/eval_repro.rs
**Line:** 39:52
**Comment:**
*Possible Bug: This assertion only proves that a text fragment exists somewhere in `ci.yml`; it does not verify that the command belongs to the Rust job or that its failure is enforced. In the current workflow the Rust job is `continue-on-error`, so the test can pass while the SelfCheck is non-blocking. Validate the step's job placement and failure semantics, or avoid naming this as an enforcement check.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
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/ci.yml:
- Around line 90-92: Remove the job-level continue-on-error setting affecting
the Rust job so the eval reproducibility manifest SelfCheck step running
./scripts/eval-repro-check.ps1 -SelfCheck blocks the workflow when it fails.
In `@tests/eval_repro.rs`:
- Around line 36-52: Remove the duplicate ci.yml assertion from
eval_repro_manifest_self_check_validates_lockfile_and_fixtures, keeping the
identical PowerShell workflow assertion only in
ci_workflow_runs_eval_repro_self_check_with_powershell. Preserve the manifest
self-check test’s lockfile and fixture validations.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c81d0a54-9c7c-45da-b71c-65f6de72eaea
📒 Files selected for processing (3)
.github/workflows/ci.ymldocs/ops/eval-manifest.jsontests/eval_repro.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (35)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: sl-daemon · repository builder image offline build / sl-daemon · repository builder image offline build
- GitHub Check: hermetic · reusable workflow provenance (soft)
- GitHub Check: miri permutation · race_model
- GitHub Check: fuzz blocking · sustained 30s
- GitHub Check: browser e2e · axe · responsive · visual
- GitHub Check: update check hard · sl-daemon tests
- GitHub Check: update check hard · root SelfCheck wrapper
- GitHub Check: jemalloc default-on · windows default build
- GitHub Check: load macro gate · macro routes smoke
- GitHub Check: jemalloc default-on · unix default build
- GitHub Check: cargo deny check
- GitHub Check: Detect Languages
- GitHub Check: visual contract · WCAG AA
- GitHub Check: tsan permutation · race_model
- GitHub Check: loom permutation · hermetic wrappers
- GitHub Check: loom permutation · daemon pipeline
- GitHub Check: loom permutation · core models
- GitHub Check: loom permutation · daemon shutdown
- GitHub Check: race smoke + channel/cancel model · macos-latest
- GitHub Check: loom permutation · daemon broadcast
- GitHub Check: soft loom · loom_model core
- GitHub Check: race smoke + channel/cancel model · windows-latest
- GitHub Check: sl-viewer macOS app · artifact
- GitHub Check: soft loom · daemon broadcast
- GitHub Check: jemalloc hard · feature build
- GitHub Check: sl-viewer help · unit tests
- GitHub Check: session-ledger build · macos-latest
- GitHub Check: session-ledger build · ubuntu-latest
- GitHub Check: sl-daemon build · ubuntu-latest
- GitHub Check: session-ledger build · windows-latest
- GitHub Check: sl-daemon build · windows-latest
- GitHub Check: shuttle permutation · cargo test shuttle_permutation
- GitHub Check: Summary
- GitHub Check: Summary
⚠️ CI failures not shown inline (6)
GitHub Actions: Trunk Check / Lint & Format: ci: restore eval reproducibility self-check
Conclusion: failure
##[group]GITHUB_TOKEN Permissions
Contents: read
Metadata: read
Packages: read
##[endgroup]
Secret source: Actions
Prepare workflow directory
Prepare all required actions
Getting action download info
##[error]Unable to resolve action `trunk-io/trunk-action@d90b9166660d5e5afae248a58172a3a0e99d56d5`, unable to find version `d90b9166660d5e5afae248a58172a3a0e99d56d5`
GitHub Actions: Trunk Check / 0_Lint & Format.txt: ci: restore eval reproducibility self-check
Conclusion: failure
##[group]GITHUB_TOKEN Permissions
Contents: read
Metadata: read
Packages: read
##[endgroup]
Secret source: Actions
Prepare workflow directory
Prepare all required actions
Getting action download info
##[error]Unable to resolve action `trunk-io/trunk-action@d90b9166660d5e5afae248a58172a3a0e99d56d5`, unable to find version `d90b9166660d5e5afae248a58172a3a0e99d56d5`
GitHub Actions: qgate / browser e2e · axe · responsive · visual: ci: run eval reproducibility self-check
Conclusion: failure
##[group]Run bail() {
�[36;1mbail() {�[0m
�[36;1m printf '::error::install-action: %s\n' "$*"�[0m
GitHub Actions: qgate / prepare: ci: run eval reproducibility self-check
Conclusion: failure
##[group]Run mkdir -p coverage
�[36;1mmkdir -p coverage�[0m
�[36;1mcargo llvm-cov --package session-ledger --features sqlite --all-targets --lcov --output-path coverage/lcov.info�[0m
�[36;1mecho "lcov size: $(wc -l < coverage/lcov.info) lines"�[0m
shell: /usr/bin/bash -e {0}
env:
CARGO_HOME: /home/runner/.cargo
CARGO_INCREMENTAL: 0
CARGO_TERM_COLOR: always
CACHE_ON_FAILURE: false
##[endgroup]
info: cargo-llvm-cov currently setting cfg(coverage); you can opt-out it by passing --no-cfg-coverage
info: running `rustup component add llvm-tools-preview --toolchain 1.96.0-x86_64-unknown-linux-gnu` to install the `llvm-tools-preview` component for the selected toolchain
info: downloading component llvm-tools
�[1m�[92m Downloading�[0m crates ...
�[1m�[92m Downloaded�[0m adler2 v2.0.1
�[1m�[92m Downloaded�[0m anes v0.1.6
�[1m�[92m Downloaded�[0m addr2line v0.25.1
�[1m�[92m Downloaded�[0m time-core v0.1.9
�[1m�[92m Downloaded�[0m jobserver v0.1.35
�[1m�[92m Downloaded�[0m time-macros v0.2.32
�[1m�[92m Downloaded�[0m ciborium-ll v0.2.2
�[1m�[92m Downloaded�[0m crypto-common v0.2.2
�[1m�[92m Downloaded�[0m ciborium-io v0.2.2
�[1m�[92m Downloaded�[0m zstd v0.13.3
�[1m�[92m Downloaded�[0m cast v0.3.0
�[1m�[92m Downloaded�[0m errno v0.3.14
�[1m�[92m Downloaded�[0m cpufeatures v0.3.0
�[1m�[92m Downloaded�[0m anstyle v1.0.14
�[1m�[92m Downloaded�[0m cfg-if v1.0.4
�[1m�[92m Downloaded�[0m alloca v0.4.0
�[1m�[92m Downloaded�[0m half v2.7.1
�[1m�[92m Downloaded�[0m clap_lex v1.1.0
�[1m�[92m Downloaded�[0m wait-timeout v0.2.1
�[1m�[92m Downloaded�[0m thousands v0.2.0
�[1m�[92m Downloaded�[0m tinytemplate v1.2.1
�[1m�[92m Downloaded�[0m scopeguard v1.2.0
�[1m�[92m Downloaded�[0m same-file v1.0.6
�[1m�[92m Downloaded�[0m lock_api v0.4.14
�[1m�[92m Downloaded�[0m walkdir v2.5.0
�[1m�[92m Downloaded�[0m zstd-safe v7.2.4
�[1m�[92m Downloaded�[0m itoa v1.0.18
�[1m�[92m Downloaded�[0m zmij v1.0.23
�[1m�[92m Downl...
GitHub Actions: qgate / 2_prepare.txt: ci: run eval reproducibility self-check
Conclusion: failure
##[group]Run mkdir -p coverage
�[36;1mmkdir -p coverage�[0m
�[36;1mcargo llvm-cov --package session-ledger --features sqlite --all-targets --lcov --output-path coverage/lcov.info�[0m
�[36;1mecho "lcov size: $(wc -l < coverage/lcov.info) lines"�[0m
shell: /usr/bin/bash -e {0}
env:
CARGO_HOME: /home/runner/.cargo
CARGO_INCREMENTAL: 0
CARGO_TERM_COLOR: always
CACHE_ON_FAILURE: false
##[endgroup]
info: cargo-llvm-cov currently setting cfg(coverage); you can opt-out it by passing --no-cfg-coverage
info: running `rustup component add llvm-tools-preview --toolchain 1.96.0-x86_64-unknown-linux-gnu` to install the `llvm-tools-preview` component for the selected toolchain
info: downloading component llvm-tools
�[1m�[92m Downloading�[0m crates ...
�[1m�[92m Downloaded�[0m adler2 v2.0.1
�[1m�[92m Downloaded�[0m anes v0.1.6
�[1m�[92m Downloaded�[0m addr2line v0.25.1
�[1m�[92m Downloaded�[0m time-core v0.1.9
�[1m�[92m Downloaded�[0m jobserver v0.1.35
�[1m�[92m Downloaded�[0m time-macros v0.2.32
�[1m�[92m Downloaded�[0m ciborium-ll v0.2.2
�[1m�[92m Downloaded�[0m crypto-common v0.2.2
�[1m�[92m Downloaded�[0m ciborium-io v0.2.2
�[1m�[92m Downloaded�[0m zstd v0.13.3
�[1m�[92m Downloaded�[0m cast v0.3.0
�[1m�[92m Downloaded�[0m errno v0.3.14
�[1m�[92m Downloaded�[0m cpufeatures v0.3.0
�[1m�[92m Downloaded�[0m anstyle v1.0.14
�[1m�[92m Downloaded�[0m cfg-if v1.0.4
�[1m�[92m Downloaded�[0m alloca v0.4.0
�[1m�[92m Downloaded�[0m half v2.7.1
�[1m�[92m Downloaded�[0m clap_lex v1.1.0
�[1m�[92m Downloaded�[0m wait-timeout v0.2.1
�[1m�[92m Downloaded�[0m thousands v0.2.0
�[1m�[92m Downloaded�[0m tinytemplate v1.2.1
�[1m�[92m Downloaded�[0m scopeguard v1.2.0
�[1m�[92m Downloaded�[0m same-file v1.0.6
�[1m�[92m Downloaded�[0m lock_api v0.4.14
�[1m�[92m Downloaded�[0m walkdir v2.5.0
�[1m�[92m Downloaded�[0m zstd-safe v7.2.4
�[1m�[92m Downloaded�[0m itoa v1.0.18
�[1m�[92m Downloaded�[0m zmij v1.0.23
�[1m�[92m Downl...
GitHub Actions: qgate / 1_browser e2e · axe · responsive · visual.txt: ci: run eval reproducibility self-check
Conclusion: failure
##[group]Run bail() {
�[36;1mbail() {�[0m
�[36;1m printf '::error::install-action: %s\n' "$*"�[0m
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{rs,toml}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,toml}: Use the Rust toolchain pinned inrust-toolchain.toml; the workspace MSRV is Rust 1.85.
Validate Rust workspace changes with the prescribed locked build, all-features test suite, Clippy, and rustfmt checks where applicable.
Files:
tests/eval_repro.rs
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
Fix Clippy warnings; do not add
#[allow]unless it includes a tracking-issue comment.
Files:
tests/eval_repro.rs
🪛 GitHub Actions: rootless no-net / 0_rootless_no-net · SelfCheck.txt
.github/workflows/ci.yml
[error] 1-1: Rootless/no-net SelfCheck failed: ci.yml is missing the required reference to 'rootless-nonet.yml'. Command './scripts/rootless-nonet-check.ps1 -SelfCheck' failed with exit code 1.
🪛 GitHub Actions: rootless no-net / rootless_no-net · SelfCheck
.github/workflows/ci.yml
[error] 1-1: rootless-nonet-check.ps1 SelfCheck failed because ci.yml is missing the required reference to 'rootless-nonet.yml'. Command './scripts/rootless-nonet-check.ps1 -SelfCheck' exited with code 1.
🔇 Additional comments (1)
docs/ops/eval-manifest.json (1)
47-47: LGTM!
| - name: eval reproducibility manifest SelfCheck | ||
| shell: pwsh | ||
| run: ./scripts/eval-repro-check.ps1 -SelfCheck |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow excerpt =="
if [ -f .github/workflows/ci.yml ]; then
nl -ba .github/workflows/ci.yml | sed -n '1,130p'
else
echo "missing .github/workflows/ci.yml"
fi
echo
echo "== references to eval-repro-check and continue-on-error =="
rg -n "eval-repro-check\.ps1|continue-on-error|SelfCheck|reproducibility|Cargo\.lock|checksum" .github/workflows/ci.yml scripts 2>/dev/null || trueRepository: KooshaPari/SessionLedger
Length of output: 225
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow excerpt =="
if [ -f .github/workflows/ci.yml ]; then
awk '{printf "%6d %s\n", NR, $0}' .github/workflows/ci.yml | sed -n '1,130p'
else
echo "missing .github/workflows/ci.yml"
fi
echo
echo "== references to eval-repro-check and continue-on-error =="
if command -v rg >/dev/null 2>&1; then
rg -n "eval-repro-check\.ps1|continue-on-error|SelfCheck|reproducibility|Cargo\.lock|checksum" .github/workflows/ci.yml scripts 2>/dev/null || true
else
command -v grep >/dev/null 2>&1 || true
if [ -f .github/workflows/ci.yml ]; then
grep -nE "eval-repro-check\.ps1|continue-on-error|SelfCheck|reproducibility|Cargo\.lock|checksum" .github/workflows/ci.yml || true
fi
fiRepository: KooshaPari/SessionLedger
Length of output: 50380
🌐 Web query:
GitHub Actions continue-on-error job level failure does not fail workflow success documentation
💡 Result:
In GitHub Actions, setting continue-on-error: true at the job level prevents a failed job from causing the overall workflow run to fail [1][2]. When this property is enabled, the workflow will continue to execute subsequent jobs as if the job with the failure had succeeded, effectively treating the failed job as a success for the purpose of workflow completion status [3][4][2]. It is important to note the following technical nuances regarding this behavior: 1. Job Status Representation: While continue-on-error: true prevents the workflow from failing, the individual job itself may still be visually represented as having failed (e.g., displaying a red X or error status in the GitHub UI) in certain contexts, despite the workflow reporting success [3][5][6]. This is a known point of contention as it can create ambiguity in PR status checks [7][5]. 2. Step vs. Job Level: - At the step level, continue-on-error: true allows subsequent steps within the same job to execute even if the marked step fails [3][2]. The step's outcome (before the property is applied) is failure, but its final conclusion is success [4][8]. - At the job level, the setting specifically prevents the job's failure from bubbling up to the workflow level, ensuring the entire workflow run does not get marked as a failure [1][2]. 3. Conditional Execution: Because the job is treated as successful by the runner when continue-on-error: true is applied, downstream jobs that depend on it (e.g., using needs) will proceed regardless of the actual failure of the steps within that job [3][9]. If you require downstream jobs to respond to the actual failure, you may need to explicitly check the status of the job or step using context expressions rather than relying on standard dependency checks [3][5][9].
Citations:
- 1: https://qaskills.sh/blog/ci-fail-fast-vs-continue-on-error-jobs
- 2: https://notes.kodekloud.com/docs/GitHub-Actions/Continuous-Integration-with-GitHub-Actions/Using-continue-on-error-expression/page
- 3: https://www.kenmuse.com/blog/how-to-handle-step-and-job-errors-in-github-actions/
- 4: https://robk.uk/posts/training/github/2025-github-actions/07-controlling-workflow/
- 5: Wrong behaviour when combining 'continue-on-error' and 'failure()' in subsequent steps actions/toolkit#1034
- 6: Wrong behaviour when combining 'continue-on-error' and 'failure()' in subsequent steps actions/toolkit#1034
- 7: Please support something like "allow-failure" for a given job actions/runner#2347
- 8: https://stackoverflow.com/questions/62045967/is-there-a-way-to-continue-on-error-while-still-getting-correct-feedback
- 9:
continue-on-errorshould not returnsuccessinneeds_job_resultactions/toolkit#1739
Make the reproducibility self-check a blocking step.
continue-on-error: true at Line 85 makes failures in this Rust job non-blocking for the workflow. If ./scripts/eval-repro-check.ps1 -SelfCheck fails, the CI run can still complete successfully, so a stale Cargo.lock checksum can land without a failed status check. Remove the job-level continue-on-error, or move the self-check to a separate required blocking job.
🤖 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 @.github/workflows/ci.yml around lines 90 - 92, Remove the job-level
continue-on-error setting affecting the Rust job so the eval reproducibility
manifest SelfCheck step running ./scripts/eval-repro-check.ps1 -SelfCheck blocks
the workflow when it fails.
|
|
||
| let ci_workflow = std::fs::read_to_string(repo_root().join(".github/workflows/ci.yml")) | ||
| .expect("expected ci.yml to be readable"); | ||
| assert!( | ||
| ci_workflow.contains("shell: pwsh\n run: ./scripts/eval-repro-check.ps1 -SelfCheck"), | ||
| "ci.yml must invoke eval-repro-check.ps1 -SelfCheck with PowerShell" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn ci_workflow_runs_eval_repro_self_check_with_powershell() { | ||
| let ci_workflow = std::fs::read_to_string(repo_root().join(".github/workflows/ci.yml")) | ||
| .expect("expected ci.yml to be readable"); | ||
| assert!( | ||
| ci_workflow.contains("shell: pwsh\n run: ./scripts/eval-repro-check.ps1 -SelfCheck"), | ||
| "ci.yml must invoke eval-repro-check.ps1 -SelfCheck with PowerShell" | ||
| ); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Remove the duplicate workflow assertion.
eval_repro_manifest_self_check_validates_lockfile_and_fixtures and ci_workflow_runs_eval_repro_self_check_with_powershell assert the same string. Keep the assertion in the dedicated workflow test and remove it from the manifest self-check test. This prevents duplicate failures and keeps each test focused.
🤖 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 `@tests/eval_repro.rs` around lines 36 - 52, Remove the duplicate ci.yml
assertion from eval_repro_manifest_self_check_validates_lockfile_and_fixtures,
keeping the identical PowerShell workflow assertion only in
ci_workflow_runs_eval_repro_self_check_with_powershell. Preserve the manifest
self-check test’s lockfile and fixture validations.
User description
Summary
Evidence
pwsh -NoProfile -File ./scripts/eval-repro-check.ps1 -SelfCheckpassescargo test --test eval_repro eval_repro_manifest_self_check_validates_lockfile_and_fixturespasses (1/1).github/workflows/ci.yml,docs/ops/eval-manifest.json, andtests/eval_repro.rsActionlint reports only pre-existing shellcheck/undefined-expression warnings elsewhere in
ci.yml; no new workflow syntax errors.CodeAnt-AI Description
Restore CI validation for reproducible evaluation manifests
What Changed
Impact
✅ Earlier detection of evaluation fixture drift✅ CI catches lockfile checksum mismatches✅ Reproducible evaluation checks run automatically💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.