ci(reborn): count crate-tier tests in coverage + scoped denominator exemptions - #5658
Conversation
Convert the existing crate-tests job's per-package `cargo test -p X` invocation to `cargo llvm-cov -p X ... --lcov ... test` in place — same test execution, instrumented, coverage as a byproduct, no second run of anything. Each bucket concatenates its packages' lcov into one artifact and uploads it for the coverage-report job to merge. Bump job timeout 60m->90m and the per-package inner `timeout` wrapper 28m->40m for instrumentation overhead headroom (composition-core is the single-package bucket with the least slack). Use a dedicated `reborn-tests-crates-cov` cache key so the instrumented build never shares a cache lineage with the plain `reborn-tests-crates` build. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Wire the new instrumented crate-tests bucket artifacts into the coverage-report job: add crate-tests to needs, download the reborn-crate-cov-bucket-* artifact pattern alongside the existing integration-tier lane pattern, and pass every file to the (already generic) merge script in one call. No change to reborn-coverage-merge-lcov.sh. Tighten timeout-minutes 15->10 to make the <=10-minute coverage- calculation SLA a checked fact: this job stays pure lcov-text processing (download, merge, render, upsert comment) with no test execution anywhere in it, by construction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Extend the exemptions schema reborn-coverage-summary.sh parses to accept a whole-crate `crate = "ironclaw_x"` entry alongside the existing per-file `module = "path"` form (exactly one of the two required per entry). Both forms are normalized to a single `label` field at parse time, and every downstream consumer (the is_exempt match, the sort key, the render row) reads that field uniformly instead of branching on which key is present. This is the structural fix, not a two-line patch: the prior code had two more unconditional `entry["module"]` accesses in the render section (sort key + table cell) that would KeyError on the first crate-only entry, since that entry shape has no `module` key at all. Normalizing at parse time removes every such access site instead of just the ones found today. Extends tests/ci/test-reborn-coverage.sh with cases for the crate= form, a mixed module+crate manifest, and the both-present/neither-present validation errors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ironclaw_embeddings, ironclaw_gateway, ironclaw_oauth, ironclaw_tui are consumed exclusively by the root `ironclaw` v1 package (no crates/*/Cargo.toml depends on any of them, confirmed via a reverse-dependency audit) and are already exercised by a separate CI workflow (test.yml, "Tests (Legacy)"). They are not a testing gap, just out of this report's Reborn-scoped denominator — mechanical effect is ~29.61% -> ~30.6% (172,586 -> ~167,035 line denominator, numerator unchanged since these lines contributed 0 hits). Each entry uses the new whole-crate `crate =` exemption form (see the prior commit) with its reverse-dependency rationale inlined so a reviewer never has to re-derive it. NOTE: `issue` fields use the placeholder https://github.com/nearai/ironclaw/issues/NNNN pending a tracking issue ("v1-only crates excluded from Reborn coverage scope" covering all four) — file it and replace NNNN before merge. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Issue #5657 was already filed and referenced in every entry's `issue` field; the leftover "not yet filed" / "NNNN placeholder" comment contradicted the entries right below it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
github-actions
Bot
added
scope: ci
|
Caution Review failedAn error occurred during the review process. Please try again later. 📝 WalkthroughSummary by CodeRabbit
WalkthroughReborn CI now emits crate-bucket LLVM-cov artifacts, merges them with lane coverage, and accepts whole-crate exemptions alongside per-module exemptions in the coverage summary pipeline. ChangesReborn coverage pipeline
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for whole-crate exemptions in the Reborn coverage summary script, allowing entire crates to be excluded from coverage accounting alongside the existing per-file module exemptions. The parser has been updated to validate that exactly one of module or crate is specified, and entries are normalized using a shared label field. Additionally, comprehensive integration tests have been added to verify these changes, and four legacy v1-only crates have been added to the exemptions configuration. There are no review comments, and I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6cf662b4c3
ℹ️ 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".
| cargo llvm-cov -p "$package" ${feature_flags} --all-targets \ | ||
| --lcov --output-path "coverage/${package}.lcov" \ | ||
| test -- --nocapture |
There was a problem hiding this comment.
In buckets with more than one package, each cargo llvm-cov invocation starts by cleaning its previous coverage build unless --no-clean is passed (the cargo-llvm-cov help documents --no-clean as “Build without cleaning any old build artifacts”). Because this loop invokes llvm-cov once per package, the instrumented target restored/generated for one package is discarded before the next package, so multi-package buckets keep rebuilding instead of sharing the cache and are much more likely to hit the new 40m/90m timeouts. Add --no-clean and clean only profraw data between packages to keep reports isolated without deleting compiled artifacts.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Reviewed via thermo-nuclear-code-quality-review: deferring. --no-clean + selective profraw cleanup is a plausible win but needs empirical validation on an actual multi-package bucket run (build-cache/coverage-isolation correctness can't be verified in this sandbox — no Actions runner). Filing as a follow-up rather than landing speculative caching behavior in a coverage-measurement PR. composition-core (the only bucket that sets CARGO_INCREMENTAL=0) remains single-package today, so no bucket is currently near the 40m/90m timeouts.
![coderabbitai[bot]](https://avatars.githubusercontent.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/reborn-tests.yml:
- Around line 306-331: The CARGO_INCREMENTAL setting in the bucket loop is being
exported into the current shell, so it can affect later packages processed by
the same while/done block. Update the logic around the
ironclaw_reborn_composition case in reborn-tests.yml so the variable is scoped
only to that package’s cargo llvm-cov invocation, rather than using a persistent
export. Keep the fix localized to the package-processing loop and ensure no
subsequent packages inherit the setting.
In `@scripts/ci/reborn-coverage-summary.sh`:
- Around line 77-110: The exemption normalization in reborn-coverage-summary.sh
correctly validates and records labels, but crate-based exemptions can still be
silently unused if they never match any parsed lcov crate segment. Update the
lcov parsing flow and the downstream exemption application logic to track which
entries from exempt_crates and exempt_modules are actually matched, then emit a
warning or fail if any exemption remains unapplied by the end. Use the existing
symbols exemptions, exempt_crates, exempt_modules, label, and crate_re to wire
the check into the current parse-time normalization and reporting path.
🪄 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: 2d7d9f6e-7a1c-4634-925a-84cf36ec5363
📒 Files selected for processing (4)
.github/workflows/reborn-tests.ymlscripts/ci/reborn-coverage-summary.shscripts/ci/test-reborn-coverage.shtests/integration/coverage-exemptions.toml
| # Every entry is normalized to one shared shape right here, at parse time: | ||
| # a single `label` field (the per-file path, or "crate: <name>" for the | ||
| # whole-crate form) that every downstream consumer — the is_exempt match, the | ||
| # sort key, and the render row — reads uniformly instead of each branching on | ||
| # which of `module`/`crate` is present. One shape, one branch point; a future | ||
| # call site can't reintroduce a `entry["module"]` KeyError on a crate-only | ||
| # entry because there is no such site left to write. | ||
| exemptions = manifest.get("exemption", []) | ||
| exempt_modules: set[str] = set() | ||
| exempt_crates: set[str] = set() | ||
| for entry in exemptions: | ||
| module = entry.get("module") | ||
| if not module: | ||
| print(f"malformed exemption entry (missing 'module'): {entry}", file=sys.stderr) | ||
| crate_name = entry.get("crate") | ||
| if module and crate_name: | ||
| print(f"malformed exemption entry (exactly one of 'module'/'crate' required, both present): {entry}", file=sys.stderr) | ||
| sys.exit(1) | ||
| if not module and not crate_name: | ||
| print(f"malformed exemption entry (exactly one of 'module'/'crate' required, neither present): {entry}", file=sys.stderr) | ||
| sys.exit(1) | ||
| label = module if module else f"crate: {crate_name}" | ||
| entry["label"] = label | ||
| if not entry.get("reason"): | ||
| print(f"exemption for '{module}' is missing 'reason'", file=sys.stderr) | ||
| print(f"exemption for '{label}' is missing 'reason'", file=sys.stderr) | ||
| sys.exit(1) | ||
| if not entry.get("issue"): | ||
| print(f"exemption for '{module}' is missing 'issue'", file=sys.stderr) | ||
| sys.exit(1) | ||
| if not module.startswith("crates/"): | ||
| print(f"exemption module path '{module}' must be repo-relative and start with 'crates/'", file=sys.stderr) | ||
| print(f"exemption for '{label}' is missing 'issue'", file=sys.stderr) | ||
| sys.exit(1) | ||
| exempt_modules.add(module) | ||
| if module: | ||
| if not module.startswith("crates/"): | ||
| print(f"exemption module path '{module}' must be repo-relative and start with 'crates/'", file=sys.stderr) | ||
| sys.exit(1) | ||
| exempt_modules.add(module) | ||
| else: | ||
| exempt_crates.add(crate_name) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Exactly-one-of validation and label normalization are solid.
Both-present/neither-present checks, label normalization, and the crates/ prefix check on module are all correctly ordered and fail-fast. One gap: a crate value that never matches any file's crate segment in the lcov (typo, or a name outside the ironclaw_[A-Za-z0-9_]+ pattern used by crate_re) silently becomes a no-op — the entry still renders in the exemptions table implying it took effect, but nothing is actually excluded.
Consider tracking which exempt_crates/exempt_modules entries were actually matched during lcov parsing and warning (or failing) on any that were never applied, to catch typos before they ship.
[recommended_refactor]
🤖 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/reborn-coverage-summary.sh` around lines 77 - 110, The exemption
normalization in reborn-coverage-summary.sh correctly validates and records
labels, but crate-based exemptions can still be silently unused if they never
match any parsed lcov crate segment. Update the lcov parsing flow and the
downstream exemption application logic to track which entries from exempt_crates
and exempt_modules are actually matched, then emit a warning or fail if any
exemption remains unapplied by the end. Use the existing symbols exemptions,
exempt_crates, exempt_modules, label, and crate_re to wire the check into the
current parse-time normalization and reporting path.
There was a problem hiding this comment.
Valid, but not actioning separately — bundled with the sibling thread on deriving the crate scope from package-matrix instead of hand-exempting. Adding typo-detection instrumentation to a mechanism that thread proposes replacing would be premature; revisit together.
There was a problem hiding this comment.
@henrypark133 Understood — makes sense to defer this to the package-matrix thread rather than instrument a mechanism that may get replaced. No further action needed here.
(๑˃̵ᴗ˂̵)و
|
🚅 Deployed to the ironclaw-pr-5658 environment in ironclaw-ci-preview
|
Compress narrative CI comments (coverage instrumentation rationale, cache-key separation, lcov merge mechanics, exemption schema/entries) to dense 1-3 line notes carrying the crux + issue ref, per repo comment-economy convention. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
github-actions
Bot
added
size: M
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/reborn-tests.yml (1)
280-281: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick wincargo-llvm-cov tool version is unpinned.
The install-action commit is pinned, but the tool ref is the bare
cargo-llvm-covname (per taiki-e/install-action's supported shorthand), which resolves to whatever version is current at install time rather than a fixed release. Given the repo's dependency-pinning posture (cargo deny checkbefore adding deps), consider pinning to a specificcargo-llvm-cov@<version>to avoid unannounced tool-version drift silently changing coverage output/format across runs.♻️ Suggested pin
- - name: Install cargo-llvm-cov - uses: taiki-e/install-action@62b0f2dec647a8e604c6a0fda0e38530180dce20 # cargo-llvm-cov + - name: Install cargo-llvm-cov + uses: taiki-e/install-action@62b0f2dec647a8e604c6a0fda0e38530180dce20 # cargo-llvm-cov@<pinned-version> + with: + tool: cargo-llvm-cov@<pinned-version>🤖 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/reborn-tests.yml around lines 280 - 281, The Install cargo-llvm-cov step is using the unpinned shorthand name, so the tool version can drift between runs. Update the taiki-e/install-action usage in the reborn-tests workflow to reference a specific cargo-llvm-cov@<version> release instead of the bare cargo-llvm-cov alias, keeping the existing pinned action commit intact. This should be done in the workflow job that installs cargo-llvm-cov so coverage output remains stable.
🤖 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.
Outside diff comments:
In @.github/workflows/reborn-tests.yml:
- Around line 280-281: The Install cargo-llvm-cov step is using the unpinned
shorthand name, so the tool version can drift between runs. Update the
taiki-e/install-action usage in the reborn-tests workflow to reference a
specific cargo-llvm-cov@<version> release instead of the bare cargo-llvm-cov
alias, keeping the existing pinned action commit intact. This should be done in
the workflow job that installs cargo-llvm-cov so coverage output remains stable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6f69d300-71da-4daa-bedb-0b35d6dddee0
📒 Files selected for processing (3)
.github/workflows/reborn-tests.ymlscripts/ci/reborn-coverage-summary.shtests/integration/coverage-exemptions.toml
Reborn integration-tier coverageLine coverage (Reborn crates): 85.29% — 272760 / 319799 lines Per-crate breakdown (65 crates, lowest-covered first)
This signal is informational: coverage never gates the PR — not the percentage, not the per-crate holes, not the 0-coverage callout. Exemptions (4 entry/entries excluded from the accounting above)
|
henrypark133
left a comment
henrypark133
left a comment
There was a problem hiding this comment.
Code Review (multi-agent)
Intent: Make Reborn CI coverage include crate-tier tests and apply scoped denominator exemptions for v1-only crates.
Stats: 3 findings (from 6 raw, 3 after dedup/suppression) across 3 files. Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability, approach. Reviewers failed: none. Body-only: 1.
Suppressed as duplicate: the cargo llvm-cov --no-clean / repeated clean concern is already covered by the current unresolved thread on .github/workflows/reborn-tests.yml:307.
Maintainability
- Medium Derive the coverage crate set instead of hand-exempting crates (
scripts/ci/reborn-coverage-summary.sh:75-100, confidence 75) — anchor:scripts/ci/reborn-coverage-summary.sh:75
The summary script now treats whole-crate exemptions as a second owner of the Reborn coverage denominator, while the workflow already computes the Reborn package set from the allowlist plusironclaw_reborn_clidependency closure. Those two maintained scopes can drift: if one of these currently-exempt crates becomes reachable from Reborn later, it will remain excluded until someone also editscoverage-exemptions.toml.
Tests
- Medium Sticky comment path lacks whole-crate exemption coverage (
scripts/ci/test-reborn-coverage.sh:363-370, confidence 75) — anchor:.claude/rules/testing.md:27
The new tests prove report-mode rendering drops a whole-crate exemption, but the sticky PR comment path also callsreborn-coverage-summary.sh --zero-cratesto decide whether to prepend the 0-coverage warning. There is no caller-level comment test showing that a zero-covered crate excluded withcrate =is omitted from that warning.
Local Patterns
- Low Merged coverage report keeps integration-tier-only names (
.github/workflows/reborn-tests.yml:623-638, confidence 75) — anchor:.github/workflows/reborn-tests.yml:623(no diff position — body only)
This job now merges integration lane artifacts with crate bucket artifacts, but several user-facing surfaces still sayintegration-tier: the job name, render step, summary heading, sticky comment callout, merged filename/artifact, and exemptions header. Readers can reasonably infer the reported number still excludes crate-tier tests.
| # Normalized to one `label` field here (path, or "crate: <name>") so is_exempt/sort/render never branch on module-vs-crate presence. | ||
| exemptions = manifest.get("exemption", []) | ||
| exempt_modules: set[str] = set() | ||
| exempt_crates: set[str] = set() |
There was a problem hiding this comment.
Medium — Derive the coverage crate set instead of hand-exempting crates.
The summary script now treats whole-crate exemptions as a second owner of the Reborn coverage denominator, while the workflow already computes the Reborn package set from the allowlist plus ironclaw_reborn_cli dependency closure. Those two maintained scopes can drift: if one of these currently-exempt crates becomes reachable from Reborn later, it will remain excluded until someone also edits coverage-exemptions.toml.
Fix: Delete the crate = exemption form and pass the package-matrix package list into the summary/comment path as the allowed crate set. Keep coverage-exemptions.toml for exceptional per-file exclusions only.
There was a problem hiding this comment.
Reviewed via thermo-nuclear-code-quality-review: agree with the drift concern in principle, deferring the implementation. Threading package-matrix through isn't a net complexity win as-is — it swaps a small, well-tested allow-set mechanism (with per-entry reason/issue audit trail) for a deny-by-omission scheme requiring new cross-job plumbing (package-matrix output -> coverage-report job dependency -> new script arg in both summary.sh and comment.sh), and loses the per-entry rationale visibility in the rendered report unless that's redesigned too. Filing a follow-up to work out how exclusion reasons stay visible under a package-matrix-derived scheme, rather than redesigning the exemption schema in this PR.
| reason = "v1-only: consumed exclusively by the root ironclaw package" | ||
| issue = "https://github.com/nearai/ironclaw/issues/1" | ||
| TOML | ||
| capture "${summary_sh}" "${fixtures_dir}/a10_two_crates.lcov" "${fixtures_dir}/a10_crate_exemption.toml" |
There was a problem hiding this comment.
Medium — Sticky comment path lacks whole-crate exemption coverage.
The new tests prove report-mode rendering drops a whole-crate exemption, but the sticky PR comment path also calls reborn-coverage-summary.sh --zero-crates to decide whether to prepend the 0-coverage warning. There is no caller-level comment test showing that a zero-covered crate excluded with crate = is omitted from that warning.
Fix: Add a reborn-coverage-comment.sh fake-gh case covering a zero-covered crate excluded by a whole-crate exemption, asserting the sticky comment does not include the 0-coverage callout.
There was a problem hiding this comment.
Fixed — added B4 (reborn-coverage-summary.sh --zero-crates) and C9 (reborn-coverage-comment.sh sticky body) covering a whole-crate exemption suppressing a 0-coverage crate from the callout, reusing the A10 fixture. 97/97 now. This stands regardless of how the sibling thread on deriving the crate set resolves.
…xempt zero-callout CARGO_INCREMENTAL=0 was exported into the shared loop shell instead of scoped to the composition-core invocation, so it would silently leak onto later packages if that bucket ever stops being single-package. Also closes a gap flagged in review: the sticky PR comment's 0-coverage callout had no test proving a whole-crate exemption suppresses it (only report-mode dropped it). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
✅ IronLoop Review StatusHead: Current reviewers:
Reviewer summaries
Recent activity
Available commands
Run metadataAdmission: webhook accepted the request and IronLoop persisted review state before this projection. |
github-actions
Bot
added
size: L
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ IronLoop Review: reviewer
Verdict: ✅ Approved
Findings: 0 blocking / 0 notes
Next: No reviewer action needed.
Head: cb63d093c84def9221552a947b47461d3e03f068
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
No concrete actionable regressions found in the CI coverage changes. The new whole-crate exemption handling is covered by added shell-test cases, and the workflow wiring for crate bucket LCOV artifacts is consistent with the existing integration coverage merge/report path.
Findings
None.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloop review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloop reviewwhen the fix may affect multiple areas. - Use
@ironloop statusto check queued/running/completed/stale/stalled state while reviewers run.
… coverage rule (#5718) * docs: integration-first coverage rule in CLAUDE.md, testing rules, AGENTS.md Production-wired Reborn behavior must ship with an integration-tier test asserting at a seam; crate-tier only with stated reason; no test-only wiring for unwired paths; no ignored tests. Terse rule in CLAUDE.md, full decision rule in .claude/rules/testing.md, Codex-parity pointer in AGENTS.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(ci): extract shared lcov aggregation into scripts/ci/lib Move reborn-coverage-summary.sh's exemption-parsing + lcov-aggregation logic into an importable module so reborn-coverage-ratchet.sh (next commit) can reuse it instead of reimplementing. Behavior-preserving: full test-reborn-coverage.sh suite (35 sections, 97 assertions) passes unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(ci): add coverage-floor.toml schema + reborn-coverage-ratchet.sh (dry-run only) New committed floor file (deny.toml pattern) with real baseline captured from the first green main coverage-report run at 073cc3d (#5658, before #5656's ~42k-line denominator shift): 85.35% (273073/319942 lines), verified byte-identical against the run's own merged-lcov artifact. enforce=false — dry-run only until a follow-up PR recaptures post-#5656 and flips the switch. New reborn-coverage-ratchet.sh reuses the extracted lcov lib (previous commit) to compare merged coverage against the floor: global aggregate + opt-in per-crate floors, percent and/or covered-lines forms (ANDed, not ORed, so denominator dilution can't mask a numerator regression), denominator-delta note always printed, unconditional dry-run/enforcing mode banner. Schema errors (missing fields, floor/exemption conflicts, duplicate crate entries) exit 1 regardless of enforce. New R1-R13 section in test-reborn-coverage.sh (33 assertions) covers the boundary-inclusive check, AND-not-OR semantics, dry-run masking only the exit code, the divide-by-zero guard for a renamed/removed crate, and the unconditional banner. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci(reborn): wire ratchet step into coverage-report + comment; flip roll-up to blocking New "Render Reborn coverage ratchet" step runs reborn-coverage-ratchet.sh right after the existing summary step, tee'd into the job summary — same job, same 10-min budget, no new test execution. The sticky-comment script grows a required floor-toml arg and renders the ratchet script's own report output as a "### Coverage ratchet" section at the very top of the comment body (after the hidden marker, before the 0%-crate callout) — reborn-coverage-summary.sh's output has no seam to splice into "before the per-crate table" specifically, so top-of-comment is the simpler equivalent (highest signal first either way). reborn-tests roll-up: coverage-report's warn-only block becomes a real exit 1, since a ratchet violation is now a real regression, not just a reporting-pipeline bug. Still green throughout the dry-run soak period (enforce=false in the floor file), and coverage-report is not itself a required status check (verified live against the repo's ruleset), so this is the only edit needed to make the gate merge-blocking once enforce=true lands in a follow-up PR. Also: the per-crate table's own "never gates" caption is corrected now that a (currently dry-run) gate exists, and testing.md gains one honestly-worded sentence naming the ratchet. test-reborn-coverage.sh's C section grows a permissive floor fixture (all existing comment-script cases keep testing comment behavior, not ratchet gating) plus C10 (ratchet section ordering) and C11 (missing floor manifest guard) — 138 of 138 assertions pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(ci): split ratchet R-cases out of test-reborn-coverage.sh Post-implementation thermo-nuclear review flagged test-reborn-coverage.sh crossing 1000 lines (815 -> 1108) within this branch once the ratchet R-section landed — a presumptive blocker per the review's file-size rule. Move the R1-R13 cases into a sibling file, sourced by the main script so it keeps sharing the same helpers/fixtures/counters (pure move, no behavior change): 890 -> 897 lines main script, 138/138 assertions still pass identically. shellcheck -x clean on both files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ci): classify-test-scope glob missed the new ratchet-cases sibling The R-section split (previous commit) added scripts/ci/test-reborn-coverage-ratchet-cases.sh, but classify-test-scope.sh's is_shared_test_path glob only listed exact filenames — a PR touching only the new file wouldn't have been classified has_reborn_tests=true, silently skipping the Reborn CI lanes. Widen the glob and pin it with a regression case (confirmed red before the fix, green after). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci(reborn): recapture coverage floor post-#5656, flip enforce=true Recaptured global floor from the first green main coverage-report run after #5656 (run 28817755166 @ 28da8bd): 273132/320188 = 85.30%. Denominator moved +246 lines vs 073cc3d — slack-v2-host-beta sources were already in ironclaw_reborn_composition's counted set; #5656 added numerator. Ratchet dry-runs green (85.30% >= 84.8% effective floor), so flip enforce=true in this PR — no separate follow-up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ci): harden ratchet schema guards, close reborn-scope gap in classifier Addresses 5 PR #5718 review comments on the coverage ratchet, all fail-closed schema hardening + regression coverage, no behavior change for compliant manifests: - classify-test-scope.sh: scripts/ci/lib/reborn_coverage_lcov.py was unmatched by is_reborn_test_path(), so a PR touching only the shared lcov lib would skip Reborn CI + coverage (gemini). - reborn-coverage-ratchet.sh: [global].enforce now requires a native TOML bool — a quoted "false" no longer coerces to Python True via bool() and silently starts enforcing (coderabbit). - reborn-coverage-ratchet.sh: a [crate] table (dict) instead of [[crate]] (array-of-tables) is now rejected instead of silently replaced with an empty list, which would skip the per-crate gate entirely (codex P2). - test-reborn-coverage-ratchet-cases.sh: R16-R18 add regression cases for the three existing fail-closed schema branches (missing [global], [global] without floor_percent, [[crate]] without name) that had no coverage. - test-reborn-coverage.sh: C12 covers the comment script rendering a malformed-but-present floor manifest's schema error into the sticky comment while still exiting 0 (visibility-only, never gates). Verified: 150/150 in test-reborn-coverage.sh (was 138), classifier suite green, shellcheck -x clean, py_compile clean, and the live enforce=true gate still exits 0 against the real tests/integration/coverage-floor.toml with a compliant lcov. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Summary
The Reborn coverage number was integration-tier-only, structurally blind to crate-tier test suites — audits showed
ironclaw_llmreported 3.62% while carrying 894 crate-tier unit tests, and nine crates reported 0% while all nine have real tests. This PR makes the number tell the truth:crate-testsbucketed matrix job (which already runscargo test -p <pkg>across the Reborn dependency closure on every PR) is converted in place tocargo llvm-cov -p <pkg> … --lcov … test— same packages, same feature resolution, same--all-targets, same execution, coverage as a byproduct. Per-bucket lcov artifacts merge into the existing coverage-report job.coverage-exemptions.tomlgains a whole-cratecrate =form (rationale + tracking-issue required per entry); initial entries exempt the four v1-only crates (embeddings,gateway,oauth,tui— reverse-dependency-audited, covered by the legacy test workflow). Tracked in Coverage scope: v1-only crates exempted from Reborn coverage denominator #5657.Constraints honored by construction
timeout-minutes: 10(was 15).Local verification
cargo llvm-covsmoke onironclaw_prompt_envelope(reported 0% today) with the exact new command shape: 13/13 tests pass, measures 97.99% — direct empirical confirmation of the undercount thesis.actionlint+shellcheck/bash -nclean.Verify on first CI run
composition-core(single heavy package) is the bucket to watch.reborn-tests-crates-cov) — first run builds cold.29.6%→30.6%; with crate-tier counted, roughly 55–75% (range, not a point estimate — ~55 crates' suites were never audited for density). The 80% target becomes a real conversation after the first post-merge baseline.Follow-ups (not this PR)
hookspostgres/libsql parity suite runs in a separate job — its coverage still uncounted (candidate for same treatment)🤖 Generated with Claude Code