Raise Codecov target to reflect actual coverage baseline

[nanotaboada]

Problem

After #78 (exclusions) and #56 (error type refactor), the remaining coverage gaps in the included paths (src/services/, src/routes/, src/repositories/) are six infrastructure error branches that are only reachable if the database itself fails mid-request — not missing business logic tests:

src/services/player_service.rs

• Lines 88–89: _ => CreateError::Database(e) — non-unique-violation insert failure
• Line 114: return Err(UpdateError::NotFound) — TOCTOU race between existence check and update

src/routes/players.rs

• Line 113: Err(CreateError::Database(_)) => Err(Status::InternalServerError)
• Line 147: Err(UpdateError::Database(_)) => Err(Status::InternalServerError)
• Line 169: Err(_) => Err(Status::InternalServerError) (delete)

The codecov.yml target is still at 80%, which no longer reflects the real baseline (~95–96% on included paths after current exclusions).

Proposed Solution

1. Add #[cfg(not(tarpaulin_include))] exclusions on the six infrastructure error branches above
2. Run cargo tarpaulin locally and upload via codecovcli to confirm the new baseline
3. Raise codecov.yml target to match the confirmed baseline

Acceptance Criteria

• Infrastructure error branches excluded from tarpaulin instrumentation
• codecov.yml target updated to reflect the confirmed baseline
• Codecov reports 100% (or as close as meaningfully achievable) on the included paths
• No legitimate business logic is excluded from the report
• No regressions in existing tests

References

• Tune codecov.yml exclusions to improve coverage signal on business logic layers #78 — initial codecov.yml exclusions (parent)
• Refactor player service to wrap read/delete errors in domain-specific error types #56 — error type refactor (coverage note: Refactor player service to wrap read/delete errors in domain-specific error types #56 (comment))

[coderabbitai]

🔗 Related PRs

#49 - test(routes): add HTTP-level route and concurrency tests (#21) [closed]
#52 - chore(data): normalize player dataset to 2022 FIFA World Cup Argentina squad [merged]
#73 - test(players): normalize unknown scenario tests (#69) [merged]
#76 - ci(workflow): add coverage reporting with Codecov (#42) [merged]
#81 - chore(coverage): remove pre-seeded DB and tune Codecov targets [closed]

---

📝 Issue Planner

_{Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.}

✅ Successfully generated implementation plan.

💡 Use @coderabbitai <feedback> to replan

---

🧪 Issue enrichment is currently in open beta.

You can configure auto-planning by selecting labels in the issue_enrichment configuration.

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

💬 Have feedback or questions? Drop into our discord!

[coderabbitai]

Coding Plan

Summary

• Infrastructure error branches (database failures, TOCTOU races) cannot be directly annotated with #[cfg(not(tarpaulin_include))] because they are match arms, not items
• Extract each branch's logic into small private helper functions annotated with #[cfg(not(tarpaulin_include))] that tarpaulin will skip
• This achieves surgical exclusion without hiding any business logic or requiring nightly Rust features
• After exclusions are in place, update codecov.yml target from 80% to the new baseline (~98-100%)

Design Choices

Design Choice 1: How to apply exclusions to match arms that don't support direct attribute annotation?

Options Considered:

1. Extract infrastructure error logic into small helper functions annotated with #[cfg(not(tarpaulin_include))]
2. Use #[tarpaulin::skip] (requires nightly Rust and #![register_tool(tarpaulin)])
3. Accept partial coverage and only update the target to ~95-96%

Chosen Option: 1

Rationale: Helper functions are the idiomatic stable-Rust approach for excluding specific code paths from tarpaulin coverage without requiring nightly features or leaving branches uncovered.

💡 User Tips

Regenerate the plan with different choices with @coderabbitai <feedback>.

Implementation Steps

Phase 1: Add Tarpaulin-Excluded Helpers to Service Layer

Extract the three infrastructure error paths in player_service.rs into small annotated helper functions, keeping the existing behavior intact while enabling per-branch coverage exclusion.

Task 1: Create Database Error Mapping Helper in player_service.rs

Extract the "catch-all database error" mapping logic from the create function into an annotated helper that tarpaulin will skip.

• Add a private helper function (e.g., map_other_db_error_to_create) annotated with #[cfg(not(tarpaulin_include))] in src/services/player_service.rs
• The helper accepts diesel::result::Error and returns CreateError::Database(e)
• Refactor the .map_err(|e| match e { ... }) closure on the insert call (around line 95–101) to call this helper for the catch-all arm
• Ensure the UniqueViolation → DuplicateSquadNumber mapping remains inline (this IS business logic that should be covered)

Task 2: Create Post-Insert Guard Helper in player_service.rs

Extract the defensive .ok_or(...) guard that handles the "impossible" missing row after successful insert.

• Add a private helper function (e.g., not_found_after_insert_error) annotated with #[cfg(not(tarpaulin_include))]
• The helper returns CreateError::Database(diesel::result::Error::NotFound)
• Replace the .ok_or(CreateError::Database(diesel::result::Error::NotFound)) call (line 106) with .ok_or_else(not_found_after_insert_error)

Task 3: Create TOCTOU Race Handler in player_service.rs

Extract the if updated == 0 TOCTOU race condition guard from the update function.

• Add a private helper function (e.g., handle_toctou_race) annotated with #[cfg(not(tarpaulin_include))]
• The helper returns Result<(), UpdateError> with Err(UpdateError::NotFound)
• Replace the if updated == 0 { return Err(UpdateError::NotFound); } block (lines 124–126) with a call to this helper followed by ?

🤖 Prompt for AI agents

This phase extracts three unreachable infrastructure error paths in
`src/services/player_service.rs` into private helper functions annotated with
`#[cfg(not(tarpaulin_include))]` so tarpaulin skips them during coverage
measurement.

**Task 1.1 — `map_other_db_error_to_create` helper:**
- In `src/services/player_service.rs`, add a private function
`map_other_db_error_to_create` annotated with `#[cfg(not(tarpaulin_include))]`
- It accepts a `diesel::result::Error` and returns `CreateError::Database(e)`
- In the `create` function, locate the `.map_err(|e| match e { ... })` closure
on the `insert` call (~lines 95–101); replace the catch-all arm's body with a
call to `map_other_db_error_to_create`
- Leave the `UniqueViolation` → `DuplicateSquadNumber` arm unchanged (it is
business logic that must remain covered)

**Task 1.2 — `not_found_after_insert_error` helper:**
- Add a private function `not_found_after_insert_error` annotated with
`#[cfg(not(tarpaulin_include))]`
- It takes no arguments and returns
`CreateError::Database(diesel::result::Error::NotFound)`
- Replace the `.ok_or(CreateError::Database(diesel::result::Error::NotFound))`
call at line 106 with `.ok_or_else(not_found_after_insert_error)`

**Task 1.3 — `handle_toctou_race` helper:**
- Add a private function `handle_toctou_race` annotated with
`#[cfg(not(tarpaulin_include))]`
- It returns `Result<(), UpdateError>` and always returns
`Err(UpdateError::NotFound)`
- In the `update` function, replace the `if updated == 0 { return
Err(UpdateError::NotFound); }` block (lines 124–126) with a call to
`handle_toctou_race()` followed by `?`

Phase 2: Add Tarpaulin-Excluded Helpers to Route Handlers

Extract the Database(_) => InternalServerError mappings from the three route handlers into annotated helpers.

Task 1: Create InternalServerError Mapping Helpers in routes/players.rs

Add annotated helper functions for converting database errors to HTTP 500 responses.

• Add a private helper (e.g., create_db_error_to_500) annotated with #[cfg(not(tarpaulin_include))] that accepts CreateError::Database(_) and returns Err(Status::InternalServerError)
• Add a similar helper for UpdateError::Database(_) (e.g., update_db_error_to_500)
• Add a similar helper for the generic Err(_) in delete (e.g., delete_error_to_500)
• Consider whether a single generic helper can serve all three cases to reduce duplication

Task 2: Refactor create_player Handler

Update the create_player match expression to use the annotated helper for the database error arm.

• Modify the match at lines 117–121 so the Err(CreateError::Database(_)) arm calls the helper function
• Keep the Ok and DuplicateSquadNumber arms unchanged (these are business logic)

Task 3: Refactor update_player Handler

Update the update_player match expression to use the annotated helper for the database error arm.

• Modify the match at lines 153–157 so the Err(UpdateError::Database(_)) arm calls the helper function
• Keep the Ok and NotFound arms unchanged

Task 4: Refactor delete_player Handler

Update the delete_player match expression to use the annotated helper for the generic error arm.

• Modify the match at lines 175–179 so the Err(_) arm calls the helper function
• Keep the Ok(true) and Ok(false) arms unchanged

🤖 Prompt for AI agents

This phase extracts three `Database(_) => InternalServerError` route-handler
arms in `src/routes/players.rs` into private helper functions annotated with
`#[cfg(not(tarpaulin_include))]`.

**Task 2.1 — Add helper functions:**
- In `src/routes/players.rs`, add a private function `create_db_error_to_500`
annotated with `#[cfg(not(tarpaulin_include))]` that handles
`CreateError::Database(_)` and returns `Err(Status::InternalServerError)`
- Add a private function `update_db_error_to_500` annotated with
`#[cfg(not(tarpaulin_include))]` that handles `UpdateError::Database(_)` and
returns `Err(Status::InternalServerError)`
- Add a private function `delete_error_to_500` annotated with
`#[cfg(not(tarpaulin_include))]` that handles a generic error and returns
`Err(Status::InternalServerError)`
- Evaluate whether a single generic helper can serve all three cases to reduce
duplication

**Task 2.2 — Refactor `create_player`:**
- In the match expression at lines 117–121, replace the body of the
`Err(CreateError::Database(_))` arm with a call to `create_db_error_to_500`
- Leave the `Ok` and `DuplicateSquadNumber` arms unchanged

**Task 2.3 — Refactor `update_player`:**
- In the match expression at lines 153–157, replace the body of the
`Err(UpdateError::Database(_))` arm with a call to `update_db_error_to_500`
- Leave the `Ok` and `NotFound` arms unchanged

**Task 2.4 — Refactor `delete_player`:**
- In the match expression at lines 175–179, replace the body of the `Err(_)` arm
with a call to `delete_error_to_500`
- Leave the `Ok(true)` and `Ok(false)` arms unchanged

Phase 3: Validate Coverage and Update Target

Verify the exclusions work correctly and update codecov.yml to reflect the new achievable baseline.

Task 1: Local Coverage Validation

Run cargo-tarpaulin locally to confirm the exclusions are recognized and calculate the new baseline.

• Run cargo tarpaulin --out html to generate a human-readable report
• Verify that the six infrastructure error branches no longer appear as uncovered lines
• Note the new coverage percentage for src/services/, src/routes/, src/repositories/
• Confirm no legitimate business logic was inadvertently excluded

Task 2: Update codecov.yml Target

Raise the coverage target to match the new baseline.

• Update the target value under coverage.status.project.default in codecov.yml (currently 80%)
• Update the target value under coverage.status.patch.default similarly
• Set both targets to the confirmed baseline (expected to be ~98-100%)
• Add a brief comment in codecov.yml noting the target reflects exclusion of infrastructure error branches

Task 3: Verify CI Integration

Ensure the changes work correctly in the CI pipeline.

• Confirm existing tests still pass (cargo test)
• Verify tarpaulin runs without errors in CI
• After merge, confirm Codecov reports the expected coverage on the PR/main branch

🤖 Prompt for AI agents

This phase validates the tarpaulin exclusions and updates `codecov.yml` to
reflect the new coverage baseline.

**Task 3.1 — Local validation:**
- Run `cargo tarpaulin --out html` to generate a human-readable coverage report
- Confirm the six previously-uncovered infrastructure error branches are no
longer reported as uncovered lines
- Record the new coverage percentage across `src/services/`, `src/routes/`, and
`src/repositories/`
- Confirm no business logic was inadvertently excluded

**Task 3.2 — Update `codecov.yml`:**
- In `codecov.yml`, update the `target` field under
`coverage.status.project.default` from `80%` to the confirmed new baseline
(~98-100%)
- Update the `target` field under `coverage.status.patch.default` to the same
value
- Add a brief inline comment explaining the target reflects exclusion of
infrastructure error branches

**Task 3.3 — CI verification:**
- Run `cargo test` to confirm all existing tests still pass
- Confirm tarpaulin runs without errors in CI
- After merge, verify Codecov reports the expected coverage on the PR/main
branch

Research

This is a Rust/Rocket REST API with a layered architecture: routes handle HTTP concerns, services contain business logic, and repositories manage database access via Diesel. Coverage is measured by cargo-tarpaulin (v0.35.2) and reported to Codecov. The current codecov.yml targets 80% on src/services/, src/routes/, and src/repositories/. No tarpaulin exclusion attributes (#[cfg(not(tarpaulin_include))]) currently exist anywhere in the codebase. The six infrastructure error branches identified are match arms and chained expressions that cannot be directly annotated in Rust—requiring extraction into annotated helper functions.

---

🚀 Next Steps

🤖 All AI agent prompts combined

Task: 1

This phase extracts three unreachable infrastructure error paths in
`src/services/player_service.rs` into private helper functions annotated with
`#[cfg(not(tarpaulin_include))]` so tarpaulin skips them during coverage
measurement.

**Task 1.1 — `map_other_db_error_to_create` helper:**
- In `src/services/player_service.rs`, add a private function
`map_other_db_error_to_create` annotated with `#[cfg(not(tarpaulin_include))]`
- It accepts a `diesel::result::Error` and returns `CreateError::Database(e)`
- In the `create` function, locate the `.map_err(|e| match e { ... })` closure
on the `insert` call (~lines 95–101); replace the catch-all arm's body with a
call to `map_other_db_error_to_create`
- Leave the `UniqueViolation` → `DuplicateSquadNumber` arm unchanged (it is
business logic that must remain covered)

**Task 1.2 — `not_found_after_insert_error` helper:**
- Add a private function `not_found_after_insert_error` annotated with
`#[cfg(not(tarpaulin_include))]`
- It takes no arguments and returns
`CreateError::Database(diesel::result::Error::NotFound)`
- Replace the `.ok_or(CreateError::Database(diesel::result::Error::NotFound))`
call at line 106 with `.ok_or_else(not_found_after_insert_error)`

**Task 1.3 — `handle_toctou_race` helper:**
- Add a private function `handle_toctou_race` annotated with
`#[cfg(not(tarpaulin_include))]`
- It returns `Result<(), UpdateError>` and always returns
`Err(UpdateError::NotFound)`
- In the `update` function, replace the `if updated == 0 { return
Err(UpdateError::NotFound); }` block (lines 124–126) with a call to
`handle_toctou_race()` followed by `?`
===============================================================================

Task: 2

This phase extracts three `Database(_) => InternalServerError` route-handler
arms in `src/routes/players.rs` into private helper functions annotated with
`#[cfg(not(tarpaulin_include))]`.

**Task 2.1 — Add helper functions:**
- In `src/routes/players.rs`, add a private function `create_db_error_to_500`
annotated with `#[cfg(not(tarpaulin_include))]` that handles
`CreateError::Database(_)` and returns `Err(Status::InternalServerError)`
- Add a private function `update_db_error_to_500` annotated with
`#[cfg(not(tarpaulin_include))]` that handles `UpdateError::Database(_)` and
returns `Err(Status::InternalServerError)`
- Add a private function `delete_error_to_500` annotated with
`#[cfg(not(tarpaulin_include))]` that handles a generic error and returns
`Err(Status::InternalServerError)`
- Evaluate whether a single generic helper can serve all three cases to reduce
duplication

**Task 2.2 — Refactor `create_player`:**
- In the match expression at lines 117–121, replace the body of the
`Err(CreateError::Database(_))` arm with a call to `create_db_error_to_500`
- Leave the `Ok` and `DuplicateSquadNumber` arms unchanged

**Task 2.3 — Refactor `update_player`:**
- In the match expression at lines 153–157, replace the body of the
`Err(UpdateError::Database(_))` arm with a call to `update_db_error_to_500`
- Leave the `Ok` and `NotFound` arms unchanged

**Task 2.4 — Refactor `delete_player`:**
- In the match expression at lines 175–179, replace the body of the `Err(_)` arm
with a call to `delete_error_to_500`
- Leave the `Ok(true)` and `Ok(false)` arms unchanged
===============================================================================

Task: 3

This phase validates the tarpaulin exclusions and updates `codecov.yml` to
reflect the new coverage baseline.

**Task 3.1 — Local validation:**
- Run `cargo tarpaulin --out html` to generate a human-readable coverage report
- Confirm the six previously-uncovered infrastructure error branches are no
longer reported as uncovered lines
- Record the new coverage percentage across `src/services/`, `src/routes/`, and
`src/repositories/`
- Confirm no business logic was inadvertently excluded

**Task 3.2 — Update `codecov.yml`:**
- In `codecov.yml`, update the `target` field under
`coverage.status.project.default` from `80%` to the confirmed new baseline
(~98-100%)
- Update the `target` field under `coverage.status.patch.default` to the same
value
- Add a brief inline comment explaining the target reflects exclusion of
infrastructure error branches

**Task 3.3 — CI verification:**
- Run `cargo test` to confirm all existing tests still pass
- Confirm tarpaulin runs without errors in CI
- After merge, verify Codecov reports the expected coverage on the PR/main
branch

💡 Iterate on the plan with: @coderabbitai <feedback>

Example Feedback
- `@coderabbitai` You can skip phase 3. Add a simple unit test case for phase 2.
- `@coderabbitai` For design choice 1 go ahead with option 3 and replan.

---

💬 Have feedback or questions? Drop into our discord!