fix(observability): scope before_send filter to llm_provider domain

Per CodeRabbit review on tinyhumansai#1529 (Major, observability.rs:81-89): the
before_send filter matched on `failure=non_2xx` + `status` alone, so an
unrelated subsystem emitting those tags would have its event silently
dropped. Add a `domain == "llm_provider"` precondition so only
provider-originated events qualify for the transient filter.

All current emit sites at `providers/ops.rs:218-227` already pass
`"llm_provider"` as the domain argument to `report_error`, which the
helper writes as a tag — so behavior is preserved for the target
events; the guard only narrows what *future* call sites can match.

Tests updated to include the domain tag in the existing fixtures, plus
two new regression cases: events with no domain tag, and events from
a different domain (`scheduler`) reusing the same `failure`/`status`
shape. Both must surface to Sentry rather than being dropped.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: oxoxDev

src/core/observability.rs

@@ -90,12 +90,17 @@ pub fn report_error<E: Display + ?Sized>(
 /// site that emits a `tracing::error!` with the same shape but bypasses
 /// the classifier.
 ///
-/// Match criteria:
+/// Match criteria (all required):
+/// - tag `domain == "llm_provider"` — pins the filter to provider-originated
+///   events so an unrelated subsystem emitting `failure=non_2xx`/`status=503`
+///   for its own reasons doesn't get silently dropped
 /// - tag `failure == "non_2xx"` (the marker set by `ops::api_error`)
-/// - tag `status` matches a known transient status (see
-///   [`TRANSIENT_PROVIDER_HTTP_STATUSES`])
+/// - tag `status` parses to one of [`TRANSIENT_PROVIDER_HTTP_STATUSES`]
 pub fn is_transient_provider_http_failure(event: &sentry::protocol::Event<'_>) -> bool {
     let tags = &event.tags;
+    if tags.get("domain").map(String::as_str) != Some("llm_provider") {
+        return false;
+    }
     if tags.get("failure").map(String::as_str) != Some("non_2xx") {
         return false;
     }
@@ -157,7 +162,11 @@ mod tests {
     #[test]
     fn transient_filter_drops_429_408_502_503_504() {
         for status in ["429", "408", "502", "503", "504"] {
-            let event = event_with_tags(&[("failure", "non_2xx"), ("status", status)]);
+            let event = event_with_tags(&[
+                ("domain", "llm_provider"),
+                ("failure", "non_2xx"),
+                ("status", status),
+            ]);
             assert!(
                 is_transient_provider_http_failure(&event),
                 "status {status} must be classified as transient and filtered"
@@ -168,7 +177,11 @@ mod tests {
     #[test]
     fn transient_filter_keeps_permanent_failures() {
         for status in ["400", "401", "403", "404", "500"] {
-            let event = event_with_tags(&[("failure", "non_2xx"), ("status", status)]);
+            let event = event_with_tags(&[
+                ("domain", "llm_provider"),
+                ("failure", "non_2xx"),
+                ("status", status),
+            ]);
             assert!(
                 !is_transient_provider_http_failure(&event),
                 "status {status} must NOT be filtered — it's actionable"
@@ -178,7 +191,11 @@ mod tests {
 
     #[test]
     fn transient_filter_keeps_aggregate_all_exhausted() {
-        let event = event_with_tags(&[("failure", "all_exhausted"), ("status", "503")]);
+        let event = event_with_tags(&[
+            ("domain", "llm_provider"),
+            ("failure", "all_exhausted"),
+            ("status", "503"),
+        ]);
         assert!(
             !is_transient_provider_http_failure(&event),
             "aggregate all_exhausted events must surface (they are the cascade signal)"
@@ -187,10 +204,39 @@ mod tests {
 
     #[test]
     fn transient_filter_keeps_events_with_no_status_tag() {
-        let event = event_with_tags(&[("failure", "non_2xx")]);
+        let event = event_with_tags(&[("domain", "llm_provider"), ("failure", "non_2xx")]);
         assert!(
             !is_transient_provider_http_failure(&event),
             "missing status tag must not be silently dropped"
         );
     }
+
+    // Regression guard for CodeRabbit review on #1529: the filter must scope
+    // to provider events only. Other subsystems emit `failure=non_2xx` (e.g.
+    // `providers/compatible.rs` uses the same marker for OAI-compatible
+    // error paths, but every site goes through `report_error(..,
+    // "llm_provider", ..)` so the domain tag is consistent), but the broader
+    // point is: any future caller that re-uses the same tag set for a
+    // different domain must NOT be silently dropped by this filter.
+    #[test]
+    fn transient_filter_keeps_events_with_no_domain_tag() {
+        let event = event_with_tags(&[("failure", "non_2xx"), ("status", "503")]);
+        assert!(
+            !is_transient_provider_http_failure(&event),
+            "missing domain tag means the event isn't provider-originated — must surface"
+        );
+    }
+
+    #[test]
+    fn transient_filter_keeps_events_from_other_domains() {
+        let event = event_with_tags(&[
+            ("domain", "scheduler"),
+            ("failure", "non_2xx"),
+            ("status", "503"),
+        ]);
+        assert!(
+            !is_transient_provider_http_failure(&event),
+            "non-provider domain must surface even if failure/status tags collide"
+        );
+    }
 }

tests/observability_smoke.rs

@@ -64,7 +64,13 @@ fn drops_per_attempt_429_503_504_408_502() {
     // before_send, none should leak through to the transport.
     let events = ["429", "503", "504", "408", "502"]
         .into_iter()
-        .map(|status| event_with_tags(&[("failure", "non_2xx"), ("status", status)]))
+        .map(|status| {
+            event_with_tags(&[
+                ("domain", "llm_provider"),
+                ("failure", "non_2xx"),
+                ("status", status),
+            ])
+        })
         .collect();
     assert_eq!(
         count_captured(events),
@@ -79,7 +85,13 @@ fn keeps_permanent_failures() {
     // they must reach Sentry exactly as before.
     let events = ["400", "401", "403", "404", "500"]
         .into_iter()
-        .map(|status| event_with_tags(&[("failure", "non_2xx"), ("status", status)]))
+        .map(|status| {
+            event_with_tags(&[
+                ("domain", "llm_provider"),
+                ("failure", "non_2xx"),
+                ("status", status),
+            ])
+        })
         .collect();
     assert_eq!(
         count_captured(events),
@@ -95,6 +107,7 @@ fn keeps_aggregate_all_exhausted_event() {
     // tried. That's the cascade signal we want — only the per-attempt
     // noise gets dropped.
     let event = event_with_tags(&[
+        ("domain", "llm_provider"),
         ("failure", "all_exhausted"),
         ("model", "claude-haiku-4-5-20251001"),
         ("attempts", "12"),
@@ -111,7 +124,7 @@ fn keeps_event_missing_status_tag() {
     // Belt-and-suspenders: an event with `failure=non_2xx` but no `status`
     // tag (e.g. a future call site forgets to attach one) must NOT be
     // silently dropped — we'd rather see it and fix the tag emission.
-    let event = event_with_tags(&[("failure", "non_2xx")]);
+    let event = event_with_tags(&[("domain", "llm_provider"), ("failure", "non_2xx")]);
     assert_eq!(
         count_captured(vec![event]),
         1,