fix(observability): demote composio validation noise to expected user-state (#3R #3S #33 #34 #97)

[oxoxDev]

Summary

• Add `ExpectedErrorKind::ProviderUserState` and a body-shape classifier that catches the four canonical composio validation phrasings (trigger-type-not-found, toolkit-not-enabled, missing-required-fields, insufficient-scopes).
• Route the integrations / composio HTTP envelope-error paths through `report_error_or_expected` so the new classifier actually runs.
• Targets `OPENHUMAN-TAURI-3R` / `-3S` / `-33` / `-34` / `-97` (~54 events combined).

Problem

Five Sentry IDs assigned to me share a common shape: a third-party API (composio almost always, gmail OAuth once) returned a validation error that the UI already surfaces as an actionable toast — "trigger type X not found", "toolkit Y is not enabled", "missing required fields: WABA ID", "insufficient authentication scopes". There is no remediation Sentry can drive; the user has to reconfigure the integration. Current emission path:

1. The HTTP layer wraps the body either as `Backend returned 500 ... <embedded 400 body>` (composio nests validation 400s as 500s on some routes) or as a 2xx with `success: false` and the user-state text in the envelope.
2. `is_backend_user_error_message` only matches `Backend returned 4xx`, so the 500-wrapped variants slip through.
3. The 2xx-envelope branch in `integrations/client.rs` + `composio/client.rs` calls raw `report_error` rather than `report_error_or_expected`, so even the messages the classifier could catch never reach it.

Solution

`src/core/observability.rs`:

• Add `ExpectedErrorKind::ProviderUserState` between `BackendUserError` and `LocalAiCapabilityUnavailable`. Doc-comment explains the cluster + lists the canonical wire shapes.
• Add `is_provider_user_state_message(lower)` — body-shape only, no HTTP-status anchor, so it catches the 500-wrapped case. Substring set: `"trigger type " + "not found"`, `"toolkit " + "is not enabled"`, `"missing required fields"`, `"insufficient authentication scopes"`, `"cannot enable trigger " + "not found"`.
• Route `expected_error_kind` through the new matcher BEFORE `is_backend_user_error_message` so a 4xx `Toolkit not enabled` lands in the more specific bucket. Precedence-guard test added.
• New `report_expected_message` arm tags `kind="provider_user_state"` so triage can filter the demoted breadcrumb separately from the generic backend-user-error bucket.

`src/openhuman/integrations/client.rs` + `src/openhuman/composio/client.rs`:

• Swap raw `report_error` to `report_error_or_expected` on the 2xx + `success: false` envelope-error branches (POST + GET in integrations, DELETE in composio). Non-2xx paths already route through `report_error_or_expected` from prior waves.

Tests:

• Six new unit tests in `observability.rs`: trigger-not-found, toolkit-not-enabled, missing-required-fields, insufficient-scopes, negative sanity case, precedence guard (`Backend returned 400 ... Toolkit not enabled` → ProviderUserState, NOT BackendUserError).
• Two pre-existing integration tests pinning `BackendUserError` for `Missing required fields` wires were updated to assert the new tighter bucket. Silencing is preserved (either expected-kind suppresses Sentry).

Submission Checklist

• 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.
• N/A: classifier-only behaviour change, no new feature row — Coverage matrix updated — added/removed/renamed feature rows in `docs/TEST-COVERAGE-MATRIX.md` reflect this change
• All affected feature IDs from the matrix are listed in the PR description under `## Related`
• No new external network dependencies introduced (mock backend used per Testing Strategy)
• N/A: no release-cut surface touched — Manual smoke checklist updated if this touches release-cut surfaces (`docs/RELEASE-MANUAL-SMOKE.md`)
• Linked issue closed via `Closes #NNN` in the `## Related` section

Impact

• Core binary + Tauri shell both pick up the new classifier through `openhuman_core`. No frontend / IPC surface touched. No migration.
• Sentry: expect ~54 events/wk drop from `-3R`, `-3S`, `-33`, `-34`, `-97`. The composio sync surface still surfaces these as breadcrumbs at info level via `tracing::info!` with `kind="provider_user_state"`, so they remain greppable in local logs for support triage.
• Performance: one extra substring check per non-success integrations response (negligible compared to the network round-trip).

Related

• Closes OPENHUMAN-TAURI-3R
• Closes OPENHUMAN-TAURI-3S
• Closes OPENHUMAN-TAURI-33
• Closes OPENHUMAN-TAURI-34
• Closes OPENHUMAN-TAURI-97

Summary by CodeRabbit

• Bug Fixes & Improvements

  • Reclassified certain third‑party/provider validation failures as provider user‑state so they’re logged as informational breadcrumbs (info) rather than actionable errors; this classification now takes precedence over generic backend 4xx/user‑error grouping while genuine backend bugs still surface as errors.
  • Adjusted client request handling to route provider-envelope failures through the observability classifier so provider-shaped issues are demoted to info breadcrumbs.

• Tests

  • Updated integration tests to expect the new provider user‑state classification for specific provider error shapes.
  • Relaxed a retry test to assert a bounded gateway hit range (2–4) and clarified its intent.

• Documentation

  • Expanded test comments explaining stacked retry behavior and classification precedence.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 44e4a49d-85fb-4669-acd9-8159802cb25d

📥 Commits

Reviewing files that changed from the base of the PR and between 1777156 and 826b84a.

📒 Files selected for processing (5)

• src/core/observability.rs
• src/openhuman/composio/auth_retry_tests.rs
• src/openhuman/composio/client.rs
• src/openhuman/integrations/client.rs
• src/openhuman/integrations/client_tests.rs

🚧 Files skipped from review as they are similar to previous changes (4)

• src/openhuman/integrations/client_tests.rs
• src/openhuman/integrations/client.rs
• src/openhuman/composio/client.rs
• src/core/observability.rs

---

📝 Walkthrough

Walkthrough

Adds ExpectedErrorKind::ProviderUserState and a substring classifier; prioritizes it in expected_error_kind; logs provider cases at info with kind="provider_user_state"; routes client envelope failures via report_error_or_expected; updates tests and retry assertions.

Changes

Provider User-State Error Classification

Layer / File(s)	Summary
Provider User-State Classification Contract & Tests src/core/observability.rs	Adds ExpectedErrorKind::ProviderUserState, is_provider_user_state_message, updates expected_error_kind precedence, extends report_expected_message to emit info with kind="provider_user_state", and expands unit tests for composio/OAuth shapes and precedence.
Apply Classification to Composio & Integration Clients src/openhuman/composio/client.rs, src/openhuman/integrations/client.rs, src/openhuman/integrations/client_tests.rs, src/openhuman/composio/auth_retry_tests.rs	Switches envelope-failure reporting from report_error to report_error_or_expected in Composio and Integration clients so provider-shaped 4xx are classified as ProviderUserState; updates integration tests to expect the new bucket and relaxes a retry hit-count assertion into a bounded range with clarifying comments.

Sequence Diagram

sequenceDiagram
  participant Client as Composio/Integration Client
  participant Observability as expected_error_kind
  participant Reporter as report_expected_message
  participant Sentry as Sentry

  Client->>Observability: error message (lowercased)
  Observability->>Observability: is_provider_user_state_message?
  alt provider-shaped
    Observability->>Reporter: ProviderUserState
    Reporter->>Sentry: info log (kind="provider_user_state")
  else not provider-shaped
    Observability->>Reporter: other ExpectedErrorKind
    Reporter->>Sentry: normal reporting (error/warn)
  end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related issues

• Composio tool errors are misclassified as 502s #1797: Addresses the same misclassification by adding ProviderUserState and routing envelope errors through report_error_or_expected.
• Transient upstream failures reach Sentry as errors, drowning real bugs #1608: Related observability and integration changes that also adjust how provider/composio validation failures are demoted.

Possibly related PRs

• tinyhumansai/openhuman#1719: Modifies observability classification paths and ExpectedErrorKind similarly (session-related demotion changes).
• tinyhumansai/openhuman#1763: Similar extensions to ExpectedErrorKind and reporting behavior for specific backend error shapes.

Suggested reviewers

• senamakel

Poem

A rabbit sifts through noisy logs,
Turning shouts to softer clogs.
Provider faults now hum below,
Breadcrumbs where wild alerts would go.
Hop, small fixes — steady flow. 🐇

🚥 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 clearly identifies the main change: demotion of composio validation errors to a new expected user-state category for observability. It is specific, concise, and directly reflects the primary objective of the changeset.
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.}

[graycyrus]

Walkthrough

Clean, well-scoped observability fix. Adds a new ProviderUserState expected-error classifier that catches four canonical composio/gmail-OAuth validation shapes by body text (not HTTP status), so the ~54 Sentry events from OPENHUMAN-TAURI-3R/-3S/-33/-34/-97 demote to info breadcrumbs. The envelope-error paths in composio/client.rs and integrations/client.rs are routed through report_error_or_expected to actually invoke the classifier. 6 new unit tests + 2 updated integration tests cover all shapes, wrapped variants, negative cases, and precedence ordering.

Overall this is solid work — follows every established pattern in the observability module, test coverage is thorough, and the scope is tight.

Change Summary

File	Change type	Description
src/core/observability.rs	Modified	New ProviderUserState enum variant, is_provider_user_state_message() classifier, report_expected_message arm, precedence ordering, 6 new tests + 1 updated test
src/openhuman/composio/client.rs	Modified	report_error → report_error_or_expected on DELETE envelope-error path
src/openhuman/integrations/client.rs	Modified	report_error → report_error_or_expected on POST + GET envelope-error paths
src/openhuman/integrations/client_tests.rs	Modified	2 existing tests updated to assert ProviderUserState instead of BackendUserError

Per-file Analysis

src/core/observability.rs

• Variant placement, doc-comment, and Sentry-ID references are thorough
• Classifier follows the is_<condition>_message(lower) convention; body-text matching via .contains() is consistent with siblings
• Precedence ordering (ProviderUserState before BackendUserError) is correct and well-commented
• report_expected_message arm uses tracing::info! with the same structured fields (domain, operation, kind, error) as peer arms — consistent
• Negative test covers bare "not found" and bare "is not enabled" without the required anchors — good false-positive guards
• Precedence guard test explicitly pins the 4xx + toolkit-not-enabled ordering — catches future regressions

src/openhuman/composio/client.rs + integrations/client.rs

• Function swap is mechanical (same parameter signature) — low risk
• Comments reference the relevant Sentry IDs and explain why envelope errors now route through the classifier

src/openhuman/integrations/client_tests.rs

• Updated assertions with clear explanatory messages referencing the wave and rationale

---

Reviewed by @graycyrus

[oxoxDev]

FYI for anyone picking up #1797 ("Composio tool errors are misclassified as 502s") — this PR partially addresses its acceptance criteria on the observability side. The 502-bucketing fix, parameter-validation, retry/backoff, reconnect-flow, and metrics-correction work all remain out of scope here.

#1797 acceptance criterion	Covered by #1795
Gmail insufficient-scope 403 handling	Partial — is_provider_user_state_message matches "Request had insufficient authentication scopes" → demotes from Sentry to info breadcrumb. UI reconnect-flow NOT added.
Gmail required-field validation (GMAIL_SEND_EMAIL / ADD_LABEL_TO_EMAIL)	Partial — "missing required fields" substring catches the server-side bounce. Pre-dispatch validation NOT added.
Notion fetch_type missing	Partial — same "missing required fields" catch when backend rejects.
Composio trigger validation (#3R / #3S — "Trigger type X not found")	Yes — trigger type … not found matcher. Sentry events demoted.
Composio toolkit-not-enabled (#34)	Yes — toolkit … is not enabled matcher.
Composio authorize missing-required-fields (#97)	Yes — same missing required fields catch.
502 misclassification fixed	No — this PR only demotes specific user-state error shapes; doesn't change the upstream 502 bucketing logic.
Calendar timeMin/timeMax RFC 3339 normalization	No
Slack 429 throttling + backoff	No
Pre-dispatch validation for to / recipient_email / empty label lists	No — only post-flight Sentry demotion.
Metrics correction (distinguish wrapper / platform / upstream provider errors)	No

Net: ~4/11 acceptance items get a Sentry-noise reduction here. Whoever picks up #1797 still needs to ship parameter normalization, reconnect flows, throttling, and the 502 bucketing fix — keeping #1797 open.