fix(observability): demote WhatsApp ingest SQLite lock noise

[YellowSnnowmann]

Summary

• Added ExpectedErrorKind::WhatsAppDataSqliteBusy for WhatsApp ingest lock-contention errors.

• Added a narrow classifier that matches only the WhatsApp ingest envelope ([whatsapp_data] ingest failed + upsert wa_message) with SQLite lock markers.

• Wired the new error kind into expected-error reporting so this known transient condition is demoted from Sentry error events.

• Added unit tests for both positive and negative cases to protect classification boundaries.

• Kept behavior unchanged for non-WhatsApp and non-ingest SQLite lock failures to avoid over-demotion.

Problem

• Sentry issue TAURI-RUST-BH reports repeated errors from openhumman.whatsapp_data_ingest with database is locked / Error code 5.

• The WhatsApp write path already has retries and busy timeout, but longer-lived external file locks can still exceed retry windows.

• These events are expected transient local-contention conditions and currently create alert noise without actionable remediation.

Solution

• Added a new expected-error kind specific to WhatsApp ingest SQLite busy/locked failures.

• Implemented strict message matching to avoid demoting unrelated lock errors:

• Requires WhatsApp ingest failure prefix.

• Requires upsert wa_message context.

• Requires SQLite lock indicators (database is locked, database table is locked, database file is locked, or error code 5).

• Routed this kind through observability expected-error handling (warn breadcrumb, no Sentry error event).

• Added tests covering:

• Canonical and wrapped WhatsApp ingest lock messages.

• Negative cases from other SQLite lock domains to prevent false positives.

Submission Checklist

If a section does not apply to this change, mark the item as N/A with a one-line reason. Do not delete items.

• Tests added or updated (happy path + at least one failure / edge case) per Testing Strategy
• Diff coverage ≥ 80% — changed lines (Vitest + cargo-llvm-cov merged via diff-cover) meet the gate enforced by .github/workflows/coverage.yml. Run pnpm test:coverage and pnpm test:rust locally; PRs below 80% on changed lines will not merge.
• Coverage matrix updated — added/removed/renamed feature rows in docs/TEST-COVERAGE-MATRIX.md reflect this change (or N/A: behaviour-only change)

N/A: behaviour-only observability classification change.

• All affected feature IDs from the matrix are listed in the PR description under ## Related

N/A: no feature-row change.

• No new external network dependencies introduced (mock backend used per Testing Strategy)
• Manual smoke checklist updated if this touches release-cut surfaces (docs/RELEASE-MANUAL-SMOKE.md)

N/A: no release-cut UI/runtime surface changes.

• Linked issue closed via Closes #NNN in the ## Related section

N/A: tracked via Sentry issue (no linked GitHub issue in this PR).

Impact

• Runtime/platform: Rust core observability path only; impacts desktop telemetry classification.

• Performance: negligible string-match checks in error classification path.

• Security/privacy: reduces unnecessary Sentry error noise for known local contention cases.

• Compatibility: no API/schema/data-model changes; no migration required; no breaking changes expected.

Related

Closes: TAURI-RUST-BH

Summary by CodeRabbit

• Bug Fixes
  • Improved WhatsApp data synchronization by better managing transient database lock errors to prevent unnecessary error alerts while maintaining proper logging and monitoring.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR extends the observability classification system to recognize and demote WhatsApp SQLite contention errors as expected transient failures. A new ExpectedErrorKind::WhatsAppDataSqliteBusy variant is wired into the central dispatcher via a scoped predicate matcher, logged as warn-level breadcrumbs, and validated with unit tests covering both matching and non-matching cases.

Changes

WhatsApp SQLite Busy Error Classification

Layer / File(s)

Summary

WhatsApp SQLite Busy Error Classifier src/core/observability.rs

Defines ExpectedErrorKind::WhatsAppDataSqliteBusy for transient SQLite lock failures in WhatsApp ingest; implements is_whatsapp_data_sqlite_busy_message predicate that anchors on the whatsapp ingest envelope and upsert wa_message context, then detects lock/busy phrases ("database is locked/table is locked/file is locked" and "error code 5"); wires classification into the expected_error_kind dispatcher; adds warn-level logging in report_expected_message; includes tests validating classification of canonical WhatsApp SQLite-lock shapes (including RPC-wrapped) and non-classification of unrelated "database is locked" messages from other domains.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• tinyhumansai/openhuman#2808: Extends src/core/observability.rs by adding a new ExpectedErrorKind variant with predicate matcher, dispatcher routing, and warn-level breadcrumb handling using the same observability classification pattern.
• tinyhumansai/openhuman#2830: Modifies src/core/observability.rs to extend ExpectedErrorKind and the expected message dispatcher, demoting specific transient failure patterns to warn/breadcrumbs.
• tinyhumansai/openhuman#2098: Implements WhatsApp-specific SQLite busy/locked retry behavior for the same lock-failure error strings that this PR's predicate matcher targets for observability demotion.

Suggested labels

rust-core, sentry-traced-bug

Suggested reviewers

• graycyrus
• oxoxDev
• M3gA-Mind

Poem

🐰 A busy lock won't block our way,
WhatsApp data flows today,
Transient squawks demoted down,
No Sentry noise in sleepy town! 🛌✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(observability): demote WhatsApp ingest SQLite lock noise' clearly and directly summarizes the main change: adding a targeted fix to demote/suppress WhatsApp SQLite lock errors in observability logging.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/core/observability.rs (1)

2531-2542: ⚡ Quick win

Add one case per lock indicator to pin classifier boundaries.

Current positives don’t independently assert database table is locked and a standalone error code 5 path. Adding explicit cases will prevent accidental matcher drift.

🤖 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 `@src/core/observability.rs` around lines 2531 - 2542, Add two more positive
test inputs inside the test function classifies_whatsapp_data_sqlite_busy_errors
so the classifier is pinned to each lock indicator: one raw string that contains
the exact phrase "database table is locked" (with surrounding whatsapp_data
context like the other cases) and another raw string that contains a standalone
"Error code 5" message (again keeping the surrounding log context). Assert both
map to Some(ExpectedErrorKind::WhatsAppDataSqliteBusy) using
expected_error_kind, mirroring the existing assert_eq pattern so each indicator
is independently validated.

🤖 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 `@src/core/observability.rs`:
- Around line 186-196: The observability.rs module is getting too large; split
it by extracting the matcher, reporting, and test sections into focused
submodules to keep files under ~500 lines. Create submodules (e.g.,
observability::matcher, observability::reporting, observability::tests) and move
related items—such as the WhatsAppDataSqliteBusy enum variant and its
matcher/reporting logic and any unit tests—into those files, re-exporting public
symbols from observability.rs (pub use crate::observability::matcher::..., pub
use ...::reporting) so existing callers of WhatsAppDataSqliteBusy and associated
functions continue to work. Ensure module declarations (mod matcher; mod
reporting; #[cfg(test)] mod tests;) are added to observability.rs and update use
paths within the moved code.

---

Nitpick comments:
In `@src/core/observability.rs`:
- Around line 2531-2542: Add two more positive test inputs inside the test
function classifies_whatsapp_data_sqlite_busy_errors so the classifier is pinned
to each lock indicator: one raw string that contains the exact phrase "database
table is locked" (with surrounding whatsapp_data context like the other cases)
and another raw string that contains a standalone "Error code 5" message (again
keeping the surrounding log context). Assert both map to
Some(ExpectedErrorKind::WhatsAppDataSqliteBusy) using expected_error_kind,
mirroring the existing assert_eq pattern so each indicator is independently
validated.

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9fc2728c-de66-4f6e-a037-cf5d0b0fc0fb

📥 Commits

Reviewing files that changed from the base of the PR and between c7f4da7 and 7b8250b.

📒 Files selected for processing (1)

• src/core/observability.rs

[graycyrus]

clean observability fix. the double-anchor approach (ingest envelope + upsert operation context) is the right call — narrow enough to avoid demoting unrelated lock failures in other whatsapp_data operations like list_messages.

a few things i checked:

• classifier ordering in expected_error_kind: the new check sits between MemoryStoreBreakerOpen and DiskFull, which is fine since every check is an independent early return with no ordering dependency
• error code 5 as a lock indicator: risk of false positives is real in isolation, but gated behind both envelope and operation anchors so it's safe here
• tracing::warn! level for the reporting arm is correct — matches the pattern for other expected-error kinds, not silently swallowed
• no .unwrap(), no exposed private fns, no PII in the structured warn log
• positive tests cover the canonical case and the rpc-wrapped envelope; negative cases cover other domains, other operations, and a different whatsapp_data op — solid coverage of classifier boundaries

CI 100% green including coverage gate and Rust quality checks. approving.