dev: add coverage reporting to CI and devenv by OGKevin · Pull Request #659 · OGKevin/cadmus

OGKevin · 2026-07-09T17:26:11Z

Use codecov for coverage and test analytics. Add some devenv shortcuts for local code coverage reporting as well.

Change-Id: 01a9a641e7cdd3294f74002c97a90128
Change-Id-Short: zypqptvylsnm

codecov · 2026-07-09T17:53:45Z

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

coderabbitai · 2026-07-09T19:54:27Z

Warning

Review limit reached

@OGKevin, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 33 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 0830b117-085b-4630-afb3-9bd6d06057de

📥 Commits

Reviewing files that changed from the base of the PR and between d9f4237 and aa550cf.

📒 Files selected for processing (10)

.agents/skills/build-cadmus-native/SKILL.md
.codecov.yml
.config/nextest.toml
.github/actions/cargo-cache/action.yml
.github/workflows/cargo.yml
.github/workflows/codecov-config.yml
README.md
devenv.nix
docs/src/contributing/devenv-setup.md
xtask/src/tasks/test.rs

Walkthrough

This PR adds code coverage instrumentation to the test workflow. The xtask test task gains --coverage, --save-coverage, and --save-junit flags, running tests via cargo-llvm-cov and producing Codecov JSON/LCOV reports plus JUnit XML. CI (cargo.yml) installs llvm-tools/cargo-llvm-cov, runs coverage-enabled tests per matrix entry, uploads results, and aggregates coverage in a new job uploaded to Codecov. .codecov.yml and a new validation workflow configure and check Codecov settings. devenv.nix adds local coverage scripts, packages, env vars, and a macOS toolchain symlink fix. Documentation and README are updated accordingly.

Possibly related PRs

OGKevin/cadmus#518: Both PRs modify the same devenv.nix Rust toolchain symlink-handling logic for macOS.
OGKevin/cadmus#571: Both PRs modify the cargo-cache action's cache-key computation used across CI workflows.

Suggested labels: documentation

Poem

Coverage lines now glow and gleam,
llvm-cov joins the CI team 🐇
lcov, JUnit, badges bright,
Codecov checks with gentle might.
devenv scripts to peek and diff—
hop along, no bug shall drift! 🥕

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title follows a conventional commits-style format and clearly summarizes the PR’s coverage-reporting changes.

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 3

🧹 Nitpick comments (3)

.github/workflows/codecov-config.yml (1)

16-16: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Set persist-credentials: false on the checkout.

This job only reads .codecov.yml and posts it to the Codecov validate API — no git operations need the token.

🔒 Proposed fix

       - uses: actions/checkout@v7
+        with:
+          persist-credentials: false
       - name: Validate Codecov YAML

🤖 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/codecov-config.yml at line 16, The checkout step in the
Codecov validation workflow should disable persisted git credentials because
this job only reads the config and sends it to Codecov. Update the
actions/checkout usage in the workflow to set persist-credentials to false so
the token is not stored unnecessarily.

Source: Linters/SAST tools

.github/workflows/cargo.yml (1)

387-389: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Set persist-credentials: false on the coverage-report checkout.

This job only downloads artifacts and uploads to Codecov — it doesn't need GITHUB_TOKEN credentials. The default persist-credentials: true leaves a token in the local git config, which zizmor flags as an artipacked risk.
🔒 Proposed fix
       - uses: actions/checkout@v7
         with:
           fetch-depth: 0
+          persist-credentials: false
🤖 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/cargo.yml around lines 387 - 389, The coverage-report
job’s checkout step currently keeps GitHub credentials in the local git config
by default, which is unnecessary for this artifact-only workflow. Update the
actions/checkout step in the coverage-report job to explicitly set
persist-credentials to false, keeping the existing checkout behavior otherwise
unchanged.
Source: Linters/SAST tools

xtask/src/tasks/test.rs (1)

214-248: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

save_codecov_report and save_lcov_report are near-duplicates differing only in the format flag.

Consider unifying them into a single helper to eliminate the duplication.

♻️ Proposed refactor

-fn save_codecov_report(root: &Path, output: &Path) -> Result<()> {
-    ensure_parent_dir(output)?;
-    let output_str = output.to_string_lossy();
-    run_cargo(
-        &[
-            "llvm-cov".to_owned(),
-            "report".to_owned(),
-            "--codecov".to_owned(),
-            "--output-path".to_owned(),
-            output_str.into_owned(),
-            "--ignore-filename-regex".to_owned(),
-            COVERAGE_IGNORE_RE.to_owned(),
-        ],
-        root,
-        &[],
-    )
-}
-
-fn save_lcov_report(root: &Path, output: &Path) -> Result<()> {
-    ensure_parent_dir(output)?;
-    let output_str = output.to_string_lossy();
-    run_cargo(
-        &[
-            "llvm-cov".to_owned(),
-            "report".to_owned(),
-            "--lcov".to_owned(),
-            "--output-path".to_owned(),
-            output_str.into_owned(),
-            "--ignore-filename-regex".to_owned(),
-            COVERAGE_IGNORE_RE.to_owned(),
-        ],
-        root,
-        &[],
-    )
+fn save_coverage_report(root: &Path, output: &Path, format: &str) -> Result<()> {
+    ensure_parent_dir(output)?;
+    let output_str = output.to_string_lossy();
+    run_cargo(
+        &[
+            "llvm-cov".to_owned(),
+            "report".to_owned(),
+            format.to_owned(),
+            "--output-path".to_owned(),
+            output_str.into_owned(),
+            "--ignore-filename-regex".to_owned(),
+            COVERAGE_IGNORE_RE.to_owned(),
+        ],
+        root,
+        &[],
+    )
 }

Then call save_coverage_report(&root, path, "--codecov") and save_coverage_report(&root, Path::new(LOCAL_LCOV_PATH), "--lcov") from run.

🤖 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 `@xtask/src/tasks/test.rs` around lines 214 - 248, `save_codecov_report` and
`save_lcov_report` duplicate the same `run_cargo` setup and only differ by the
coverage format flag. Refactor them into a shared `save_coverage_report(root,
output, format_flag)` helper that handles `ensure_parent_dir`, path conversion,
and the common `llvm-cov report` arguments, then update `run` to call it with
the appropriate flag for codecov and lcov.

🤖 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/cargo.yml:
- Around line 335-339: The test job’s cargo-llvm-cov restore step lacks a
cache-miss fallback, so coverage runs can fail when the binary is not present.
Update the workflow around the Restore cargo-llvm-cov step to add an id (for
example, cargo-llvm-cov-cache) and use its cache-hit output to conditionally run
an install step when the cache is missed, similar to the existing cargo-nextest
fallback.

In `@docs/src/contributing/devenv-setup.md`:
- Around line 131-162: The devenv setup doc adds new English content, but the
translation template is not updated. Regenerate and commit docs/po/messages.pot
so it includes the new strings introduced in
docs/src/contributing/devenv-setup.md, keeping the gettext catalog in sync with
the updated documentation.

In `@xtask/src/tasks/test.rs`:
- Around line 10-11: The doctest coverage path in run_doctests is dead code
because both branches currently call run_plain_doctests, so the coverage flag
has no effect. Either change run_doctests to actually run doctests under
llvm-cov when coverage is enabled, or simplify the logic by removing the
coverage branch and updating the module docs to say doctests always run with
plain cargo test --doc. If you choose the simplification, also update run_entry
to stop passing the coverage argument and remove the now-unused
run_plain_doctests helper if it is no longer needed.

---

Nitpick comments:
In @.github/workflows/cargo.yml:
- Around line 387-389: The coverage-report job’s checkout step currently keeps
GitHub credentials in the local git config by default, which is unnecessary for
this artifact-only workflow. Update the actions/checkout step in the
coverage-report job to explicitly set persist-credentials to false, keeping the
existing checkout behavior otherwise unchanged.

In @.github/workflows/codecov-config.yml:
- Line 16: The checkout step in the Codecov validation workflow should disable
persisted git credentials because this job only reads the config and sends it to
Codecov. Update the actions/checkout usage in the workflow to set
persist-credentials to false so the token is not stored unnecessarily.

In `@xtask/src/tasks/test.rs`:
- Around line 214-248: `save_codecov_report` and `save_lcov_report` duplicate
the same `run_cargo` setup and only differ by the coverage format flag. Refactor
them into a shared `save_coverage_report(root, output, format_flag)` helper that
handles `ensure_parent_dir`, path conversion, and the common `llvm-cov report`
arguments, then update `run` to call it with the appropriate flag for codecov
and lcov.

🪄 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: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: c9495925-5932-45be-b0a3-ae6ec389fc52

📥 Commits

Reviewing files that changed from the base of the PR and between 1201c62 and d9f4237.

📒 Files selected for processing (10)

.agents/skills/build-cadmus-native/SKILL.md
.codecov.yml
.config/nextest.toml
.github/actions/cargo-cache/action.yml
.github/workflows/cargo.yml
.github/workflows/codecov-config.yml
README.md
devenv.nix
docs/src/contributing/devenv-setup.md
xtask/src/tasks/test.rs

📜 Review details

🧰 Additional context used

📓 Path-based instructions (7)

docs/src/**/*.md

📄 CodeRabbit inference engine (docs/AGENTS.md)

docs/src/**/*.md: Use conversational and friendly tone in user-facing documentation
Use active voice in user-facing documentation (e.g., 'Copy the file' instead of 'The file should be copied')
Avoid jargon in user-facing documentation: replace 'artifact' with 'file' or 'package', 'deploy' with 'install' or 'download', 'configure' with 'set up', 'bundle' with 'package', 'on-device' with 'on your Kobo' or 'wirelessly'
Use standardized admonition syntax for documentation: [!NOTE], [!TIP], [!IMPORTANT], [!WARNING], [!CAUTION]

Regenerate the POT file (docs/po/messages.pot) when any English doc source (docs/src/**/*.md) is modified

Files:

docs/src/contributing/devenv-setup.md

docs/src/contributing/**/*.md

📄 CodeRabbit inference engine (docs/AGENTS.md)

docs/src/contributing/**/*.md: Use clear and direct language in contributor documentation with code examples where helpful
Document setup steps precisely in contributor documentation

Files:

docs/src/contributing/devenv-setup.md

docs/**/*.md

📄 CodeRabbit inference engine (docs/AGENTS.md)

docs/**/*.md: Format Markdown with Prettier
Ensure Markdown passes markdownlint
Use code blocks with language tags in Markdown

docs/**/*.md: Format Markdown with Prettier
Ensure Markdown passes markdownlint validation

Files:

docs/src/contributing/devenv-setup.md

docs/src/contributing/devenv-setup.md

📄 CodeRabbit inference engine (docs/REVIEW.md)

docs/src/contributing/devenv-setup.md: Document new scripts in the 'Available Commands' table of docs/src/contributing/devenv-setup.md when devenv.nix changes
Document platform limitations in the 'Platform Support' section of docs/src/contributing/devenv-setup.md when devenv.nix changes
Document new services and ports in the 'Observability Stack' section of docs/src/contributing/devenv-setup.md when devenv.nix changes
Document breaking changes in the 'Troubleshooting' section of docs/src/contributing/devenv-setup.md when devenv.nix changes

Files:

docs/src/contributing/devenv-setup.md

**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: In Rust library and app code, prefer ? over unwrap() / expect().
In Rust code, use thiserror for custom error types and anyhow for ad-hoc errors.
In Rust code, use iterators instead of index-based loops.
In Rust code, use &str instead of String in function parameters when ownership is not needed.
In Rust code, prefer borrowing over cloning.
In Rust code, inline expressions directly into struct fields instead of creating intermediate bindings solely to pass them into a struct literal.
In Rust code, avoid unsafe unless it is required and documented.
In Rust code, avoid premature collect() and keep iterators lazy where possible.
In Rust code, ensure the code compiles without warnings.
Prefer newtype wrappers over raw primitives for domain concepts such as fingerprints, file kinds, and identifiers.
For newtypes, implement Display, FromStr, and other standard traits so they remain ergonomic.
When matching on enum-backed domain values, match on enum variants instead of comparing strings.
In code comments, comment why rather than what; most code should avoid comments unless naming is insufficient.
Do not use inline comments; if one seems necessary, extract the code into a well-named, documented function instead.
Do not leave dead-code comments, changelog comments, or divider comments in source files.
Use TODO, FIXME, HACK, and NOTE annotations only when they include useful context.
Use structured fields with the tracing crate; never use string formatting for log data.
In tracing logs, use structured fields only, avoid module/name prefixes in message text, and do not mix structured fields with format arguments.
In tracing logs, use the correct field formatter: direct for primitives, % for Display, and ? for Debug types.
Use the tracing log levels consistently: debug for development detail, info for important runtime events, warn for recoverable issues, and error for failures requiring attention.
In SQLite/sqlx...

Files:

xtask/src/tasks/test.rs

**/*.{rs,ron}

📄 CodeRabbit inference engine (AGENTS.md)

When testing strings rendered through fl!, build expected strings with fl! too so Unicode isolation marks do not cause false failures.

Files:

xtask/src/tasks/test.rs

.github/workflows/cargo.yml

📄 CodeRabbit inference engine (REVIEW.md)

.github/workflows/cargo.yml: When adding a new Cargo feature flag, verify that .github/workflows/cargo.yml has new matrix entries in both clippy and test jobs for the feature alone and in combination with every other feature
Ensure feature flag test coverage is not limited to --all-features alone; paths marked with #[cfg(not(feature = "..."))] must be tested individually in CI matrix

.github/workflows/cargo.yml: When adding a new feature flag, add matrix entries to both the clippy and test jobs for the feature on its own and in combination with every other feature.
Workspace-wide feature matrix entries should use cargo_args: "--workspace --all-targets --features <name>".
Crate-specific feature matrix entries should use cargo_args: "-p <crate> --all-targets --features <name>".
All feature-flag combinations in the Cargo CI matrix must run on ubuntu-latest.
Only default and test features produce build artifacts.

Files:

.github/workflows/cargo.yml

🧠 Learnings (1)

📚 Learning: 2026-05-31T07:53:12.579Z

Learnt from: OGKevin
Repo: OGKevin/cadmus PR: 553
File: docs/src/installation/migration.md:87-88
Timestamp: 2026-05-31T07:53:12.579Z
Learning: For markdown files under docs/src/ in the cadmus (OGKevin/cadmus) repo, validate mdBook admonition syntax using the allowed types: NOTE, TIP, WARNING, IMPORTANT, and CAUTION. The correct format is a blockquote prefix like `> [!NOTE]`, `> [!TIP]`, `> [!WARNING]`, `> [!IMPORTANT]`, or `> [!CAUTION]`. Do not flag `[!CAUTION]` or `[!IMPORTANT]` as non-standard in this repo.

Applied to files:

docs/src/contributing/devenv-setup.md

🪛 LanguageTool

.agents/skills/build-cadmus-native/SKILL.md

[grammar] ~83-~83: Ensure spelling is correct
Context: ...test-coverage, cadmus-coverage-show, cadmus-coverage-diff`): bash cadmus-test-coverage cadmus-coverage-show cadmus-coverage-diff Without devenv: ```bash cargo xtask tes...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

🪛 markdownlint-cli2 (0.22.1)

README.md

[warning] 15-15: Images should have alternate text (alt text)

(MD045, no-alt-text)

🪛 SkillSpector (2.3.7)

.agents/skills/build-cadmus-native/SKILL.md

[warning] 47: [MP2] Context Window Stuffing: Skill attempts to fill the context window with filler content, displacing legitimate instructions and safety constraints. This can degrade agent performance or bypass safety boundaries.