Repository navigation
ci: restore the original 90% changed-line coverage floor - #7013
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe changed-coverage policy now enforces a 90% changed-line floor and reports branch coverage without a universal branch percentage floor. Documentation, the committed manifest, shell fixtures, and workflow-contract tests validate the policy. ChangesChanged coverage policy
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
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 |
|
🚅 Deployed to the ironclaw-pr-7013 environment in ironclaw-ci-preview
|
🔎 Review · PR #7013
Submitted review →Reviewed the complete trusted base-to-head comparison. The 95% changed-line and 85% changed-branch floors are consistently applied in the committed policy, documentation, and regression tests. Existing fail-closed behavior remains intact. No actionable findings identified. Automatic · PR opened · attempt 1 of 3 · completed in 1m 19s Run details
|
There was a problem hiding this comment.
🔍 Review complete · PR #7013
✅ No actionable findings
Reviewed the complete trusted base-to-head comparison. The 95% changed-line and 85% changed-branch floors are consistently applied in the committed policy, documentation, and regression tests. Existing fail-closed behavior remains intact. No actionable findings identified.
Validation and technical details
- Verified refs/ironloop/base = 5a1d812 and refs/ironloop/head = 31d67a9.
- Inspected all four changed files and the surrounding coverage-gate implementation and workflow integration.
python3 scripts/ci/test_reborn_changed_coverage.pypassed all 8 tests.bash scripts/ci/test-reborn-changed-coverage.shpassed all 67 self-tests, including exact-floor, below-floor, missing instrumentation, empty denominator, exemption, rename, and crate-discovery cases.- Python compilation, Bash syntax validation, and
git diff --check refs/ironloop/base..refs/ironloop/headpassed. - Base:
main - Head:
codex/relax-changed-coverage-thresholdsat31d67a9 - Run:
82490670-5675-412e-8936-ef9260ad5e98
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
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 `@scripts/ci/test-reborn-changed-coverage.sh`:
- Around line 140-143: Update the fixture loop around threshold_lines to use
Bash arithmetic iteration instead of unquoted $(seq ...), while preserving the
existing generated source and diff output. Ensure threshold_lines expansions and
all path variables remain quoted, and confirm the script uses set -euo pipefail
as required for scripts under scripts/**.
- Around line 135-139: Before the later run_gate fixture cases, restore
${case_root}/${source_path} to its original contents rather than leaving the
3-line threshold fixture in place. Rebuild ${work}/change.diff from that
restored source so subsequent cases use the correct file state and diff instead
of stale fixture data.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: faa3251c-e0a1-49eb-bb2b-4d926e305d4b
📒 Files selected for processing (4)
docs/internal/testing-playbook.mdscripts/ci/test-reborn-changed-coverage.shscripts/ci/test_reborn_changed_coverage.pytests/integration/changed-coverage-exemptions.toml
| for line in $(seq 1 "${threshold_lines}"); do | ||
| printf 'pub fn threshold_line_%s() {}\n' "${line}" >>"${case_root}/${source_path}" | ||
| printf '+pub fn threshold_line_%s() {}\n' "${line}" >>"${work}/change.diff" | ||
| done |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Avoid unquoted word splitting in the fixture loop.
Use a Bash arithmetic loop instead of unquoted $(seq ...). This keeps the iteration set controlled and removes word-splitting behavior from merge-gating tooling.
Proposed fix
-for line in $(seq 1 "${threshold_lines}"); do
+for ((line = 1; line <= threshold_lines; line++)); doAs per path instructions, CI scripts under scripts/** must use set -euo pipefail and quoted expansions.
📝 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.
| for line in $(seq 1 "${threshold_lines}"); do | |
| printf 'pub fn threshold_line_%s() {}\n' "${line}" >>"${case_root}/${source_path}" | |
| printf '+pub fn threshold_line_%s() {}\n' "${line}" >>"${work}/change.diff" | |
| done | |
| for ((line = 1; line <= threshold_lines; line++)); do | |
| printf 'pub fn threshold_line_%s() {}\n' "${line}" >>"${case_root}/${source_path}" | |
| printf '+pub fn threshold_line_%s() {}\n' "${line}" >>"${work}/change.diff" | |
| done |
🤖 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-reborn-changed-coverage.sh` around lines 140 - 143, Update
the fixture loop around threshold_lines to use Bash arithmetic iteration instead
of unquoted $(seq ...), while preserving the existing generated source and diff
output. Ensure threshold_lines expansions and all path variables remain quoted,
and confirm the script uses set -euo pipefail as required for scripts under
scripts/**.
Source: Path instructions
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 85.95% — 324552 / 377586 lines Per-crate breakdown (63 crates, lowest-covered first)
This table itself is informational and never gates the PR on its own — not the percentage, not the per-crate holes, not the 0-coverage callout. A separate coverage ratchet (dry-run until enforce=true; see tests/integration/coverage-floor.toml) can fail the build on specific configured floors. Exemptions (18 entry/entries excluded from the accounting above)
|
…ve-2 main (nearai#7032) Audits docs/reborn/target-architecture/ (plus crates/AGENTS.md and the crate guides Wave 2 touched) against merged main at 3be5f05, after nearai#6996, nearai#6998, nearai#7002 and nearai#7018. Docs-only: 13 .md files, no code, no tests. House style throughout — dated amendments, prior text quoted verbatim wherever a clause is corrected, nothing rewritten silently and no decision record deleted. The two structural findings the wave produced and nobody had written down: same-layer edges are invisible to the layer matrix by construction, so the exception count could never have moved in Wave 2 and each removal needed its own purpose-built shrink-only gate (PROPOSAL §8.1, §8.2, §11.1); and the changed-line coverage policy — 90% lines, branch coverage ungated since nearai#7013 — was recorded in no document at all, alongside a stranded-exemption failure mode the new pre-existing-uncovered exclusion creates (CHECKLIST WS10). Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* ci: relax changed coverage floors to 95/85 * ci: restore original 90% changed-line floor
…ve-2 main (nearai#7032) Audits docs/reborn/target-architecture/ (plus crates/AGENTS.md and the crate guides Wave 2 touched) against merged main at 3be5f05, after nearai#6996, nearai#6998, nearai#7002 and nearai#7018. Docs-only: 13 .md files, no code, no tests. House style throughout — dated amendments, prior text quoted verbatim wherever a clause is corrected, nothing rewritten silently and no decision record deleted. The two structural findings the wave produced and nobody had written down: same-layer edges are invisible to the layer matrix by construction, so the exception count could never have moved in Wave 2 and each removal needed its own purpose-built shrink-only gate (PROPOSAL §8.1, §8.2, §11.1); and the changed-line coverage policy — 90% lines, branch coverage ungated since nearai#7013 — was recorded in no document at all, alongside a stranded-exemption failure mode the new pre-existing-uncovered exclusion creates (CHECKLIST WS10). Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Summary
Change Type
Linked Issue
Related #6973, #6881, and #6889.
Validation
cargo fmt --all -- --checkcargo clippy --all --benches --tests --examples --all-features -- -D warningscargo buildcargo test --features integrationif database-backed or integration behavior changedreview-prorpr-shepherd --fixwas run before requesting reviewTest Strategy
User behavior: contributors return to the original 90% changed-line requirement. Defensive async/backend branches remain measured and review-visible without forcing artificial tests or large exemption manifests solely to satisfy a universal branch threshold.
Risk areas:
Tests added or updated:
What the tests prove: exactly 90% changed-line coverage passes; 85% fails; 0% branch coverage is still reported with exact uncovered branch details; missing branch instrumentation, missing production LCOV files, empty per-file denominators, stale exemptions, renamed files, and unattributable crate paths continue to fail closed.
Commands run:
python3 scripts/ci/test_reborn_changed_coverage.py;bash scripts/ci/test-reborn-changed-coverage.sh;python3 -m py_compile scripts/ci/reborn_changed_coverage.py scripts/ci/test_reborn_changed_coverage.py;bash -n scripts/ci/test-reborn-changed-coverage.sh;shellcheck scripts/ci/test-reborn-changed-coverage.sh;scripts/pre-commit-safety.sh;git diff --check.Security Impact
None. The change affects CI coverage policy only and does not modify permissions, networking, secrets, file access, tool execution, or sandbox policy.
Reborn Trust-Boundary Checklist
N/A: no Reborn runtime, authority, persistence, or trust-bearing contract changes.
Database Impact
None.
Blast Radius
The Reborn changed-code coverage step in pull-request and merge-group CI. Missing coverage remains a hard failure. Measured changed lines return to the original 90% floor; branch coverage remains mandatory in LCOV and review-visible but does not gate on a universal percentage.
Rollback Plan
Revert this PR to restore the later 100% line and branch policy.
Review Follow-Through
This restores the policy shape introduced by #6881 while retaining the fail-closed LCOV discovery and reporting improvements added later. Reviewer judgment is requested on whether any explicitly critical functions should continue to rely on the separate mutation gate instead of a universal branch threshold.
Review track: C (CI policy)