feat(analytics): split error_kind buckets for clearer install funnel

src/opik_mcp/analytics/wrappers.py

@@ -14,6 +14,8 @@
 from weakref import WeakSet
 
 import anyio
+import httpx
+from pydantic import ValidationError as PydanticValidationError
 
 from opik_mcp.analytics import (
     EVENT_SESSION_INITIALIZED,
@@ -22,6 +24,7 @@
 )
 from opik_mcp.comet_client import (
     CometAuthError,
+    CometPermissionError,
     CometProtocolError,
     OllieNotEnabledError,
 )
@@ -30,6 +33,7 @@
 from opik_mcp.opik_client import (
     OpikAuthError,
     OpikNotFoundError,
+    OpikPermissionError,
     OpikServerError,
     OpikValidationError,
 )
@@ -38,18 +42,54 @@
 
 T = TypeVar("T")
 
+# Linear walk: first matching row wins, so list subclasses BEFORE their parents
+# (OpikPermissionError before OpikAuthError, CometPermissionError before
+# CometAuthError). The httpx network errors are listed *after* the typed-API
+# errors so an authenticated 401 isn't mis-bucketed as a network failure (an
+# OpikAuthError instance is itself not an httpx exception, but ordering keeps
+# the contract explicit if anyone later adds a hybrid type).
+#
+# PRIVACY: the *classifier* (``_classify`` below) keys off exception class
+# only — never ``exc.args`` / ``exc.message`` — so adding a row here is
+# privacy-neutral. This guarantee covers ONLY the ``error_kind`` field. The
+# exception messages themselves DO carry user data (entity ids, workspace
+# names, ~200 chars of response body via ``_error_detail``); they are safe
+# *because* nothing here serializes them. Anyone adding a future field like
+# ``error_detail`` MUST bucket / hash / drop the source string — never
+# ``str(exc)``.
 _ERROR_KIND_TABLE: tuple[tuple[type[BaseException], str], ...] = (
     (MissingConfigError, "missing_config"),
+    # Comet — subclass first
+    (CometPermissionError, "comet_permission_denied"),
     (CometAuthError, "comet_auth_failed"),
     (OllieNotEnabledError, "ollie_not_enabled"),
     (CometProtocolError, "comet_protocol_error"),
-    (OpikAuthError, "opik_http_4xx"),
-    (OpikNotFoundError, "opik_http_4xx"),
-    (OpikValidationError, "opik_http_4xx"),
+    # Opik HTTP — split by status, subclass first
+    (OpikPermissionError, "opik_permission_denied"),
+    (OpikAuthError, "opik_auth_failed"),
+    (OpikNotFoundError, "opik_not_found"),
+    (OpikValidationError, "opik_validation_failed"),
     (OpikServerError, "opik_http_5xx"),
+    # Ollie streaming
     (PodNotReadyError, "pod_warmup_timeout"),
     (OllieAuthError, "ollie_auth_failed"),
     (OllieStreamError, "ollie_stream_error"),
+    # Pydantic validation from INSIDE the tool body — e.g.
+    # ``RunExperimentConfig.model_validate(experiment_config)`` in
+    # ``server.run_experiment`` or ``op.pydantic_model.model_validate(data)``
+    # in the write dispatcher. NOTE: FastMCP's ``Tool.run`` validates the
+    # tool's outer signature BEFORE calling our wrapped function and converts
+    # the resulting ``ValidationError`` into a ``ToolError``, so the very
+    # outermost arg-coercion failure never hits this branch. The bucket only
+    # fires when a tool itself calls ``model_validate`` on a sub-payload.
+    (PydanticValidationError, "tool_args_invalid"),
+    # Network — httpx.RequestError is the common base for ConnectError,
+    # TimeoutException (read/connect/write/pool), ReadError, etc. Catches the
+    # bulk of what used to land in "unknown" on flaky networks. HTTPStatusError
+    # is intentionally NOT here: when we use it, the typed Opik/Comet wrappers
+    # have already classified the status — a raw HTTPStatusError reaching this
+    # layer is a bug, not a network failure.
+    (httpx.RequestError, "network_error"),
 )
 
 

src/opik_mcp/comet_client.py

@@ -5,7 +5,15 @@
 
 
 class CometAuthError(RuntimeError):
-    """Comet-backend rejected the API key / workspace."""
+    """Comet-backend rejected the API key (401)."""
+
+
+class CometPermissionError(CometAuthError):
+    """Comet-backend returned 403 — caller is authenticated but the workspace
+    rejects the request. Subclass of ``CometAuthError`` to preserve existing
+    ``except CometAuthError`` callers; analytics distinguishes them via
+    table-order in the kind classifier.
+    """
 
 
 class OllieNotEnabledError(RuntimeError):
@@ -47,10 +55,12 @@ async def discover_pod(self, workspace: str) -> PodDiscovery:
         else:
             resp = await self._client.get(url, headers=headers)
 
-        if resp.status_code in (401, 403):
-            raise CometAuthError(
-                f"Comet rejected the request ({resp.status_code}). "
-                "Check OPIK_API_KEY and COMET_WORKSPACE."
+        if resp.status_code == 401:
+            raise CometAuthError("Comet rejected the request (401). Check OPIK_API_KEY.")
+        if resp.status_code == 403:
+            raise CometPermissionError(
+                "Comet rejected the request (403). The API key is valid "
+                "but lacks access to this workspace. Check COMET_WORKSPACE."
             )
         if resp.status_code == 400:
             preview = resp.text[:300].replace("\n", " ")

src/opik_mcp/opik_client.py

@@ -26,7 +26,15 @@
 
 
 class OpikAuthError(RuntimeError):
-    """Opik rejected the API key or workspace (401/403)."""
+    """Opik rejected the API key (401)."""
+
+
+class OpikPermissionError(OpikAuthError):
+    """Opik returned 403 — caller is authenticated but not allowed for the
+    target workspace / resource. Subclass of ``OpikAuthError`` so existing
+    handlers that catch the auth case continue to catch this too; analytics
+    classification distinguishes them via table-order (specific class first).
+    """
 
 
 class OpikNotFoundError(RuntimeError):
@@ -622,9 +630,14 @@ def _raise_for_status(resp: httpx.Response, entity_hint: str) -> None:
         return
     detail = _error_detail(resp)
     suffix = f" — {detail}" if detail else ""
-    if status in (401, 403):
+    if status == 401:
         raise OpikAuthError(
-            f"Opik rejected the request ({status}). Check OPIK_API_KEY and COMET_WORKSPACE.{suffix}"
+            f"Opik rejected the request (401). Check OPIK_API_KEY and COMET_WORKSPACE.{suffix}"
+        )
+    if status == 403:
+        raise OpikPermissionError(
+            f"Opik rejected the request (403). The API key is valid but lacks "
+            f"permission for {entity_hint}. Check COMET_WORKSPACE access.{suffix}"
         )
     if status == 404:
         raise OpikNotFoundError(f"{entity_hint} not found (404).{suffix}")

src/opik_mcp/run_experiment.py

@@ -17,6 +17,7 @@
     OpikAuthError,
     OpikClient,
     OpikNotFoundError,
+    OpikPermissionError,
     OpikServerError,
     OpikValidationError,
 )
@@ -43,10 +44,14 @@ def _raise_for_execute_status(resp: httpx.Response) -> None:
     if 200 <= resp.status_code < 300:
         return
     body_excerpt = (resp.text or "")[:500]
-    if resp.status_code in (401, 403):
+    if resp.status_code == 401:
         raise OpikAuthError(
-            f"Opik rejected the experiment execute request ({resp.status_code}). "
-            "Check OPIK_API_KEY and COMET_WORKSPACE."
+            "Opik rejected the experiment execute request (401). Check OPIK_API_KEY."
+        )
+    if resp.status_code == 403:
+        raise OpikPermissionError(
+            "Opik rejected the experiment execute request (403). The API key is "
+            "valid but lacks permission for this workspace. Check COMET_WORKSPACE."
         )
     if resp.status_code == 404:
         raise OpikNotFoundError(f"Test suite not found (404) — {body_excerpt}")

tests/test_analytics_wrappers.py

@@ -1,20 +1,42 @@
 from typing import Any
 
+import httpx
 import pytest
+from pydantic import BaseModel
+from pydantic import ValidationError as PydanticValidationError
 
 from opik_mcp.analytics import EVENT_TOOL_CALLED
 from opik_mcp.analytics.wrappers import instrument_tool
-from opik_mcp.comet_client import CometAuthError, CometProtocolError, OllieNotEnabledError
+from opik_mcp.comet_client import (
+    CometAuthError,
+    CometPermissionError,
+    CometProtocolError,
+    OllieNotEnabledError,
+)
 from opik_mcp.config import MissingConfigError
 from opik_mcp.ollie_client import OllieAuthError, OllieStreamError, PodNotReadyError
 from opik_mcp.opik_client import (
     OpikAuthError,
     OpikNotFoundError,
+    OpikPermissionError,
     OpikServerError,
     OpikValidationError,
 )
 
 
+def _build_pydantic_error() -> PydanticValidationError:
+    """Construct a real ``pydantic.ValidationError`` (can't be instantiated directly)."""
+
+    class _M(BaseModel):
+        x: int
+
+    try:
+        _M.model_validate({"x": "not-an-int"})
+    except PydanticValidationError as e:
+        return e
+    raise AssertionError("model_validate did not raise — pydantic upgraded?")
+
+
 @pytest.fixture
 def anyio_backend() -> str:
     return "asyncio"
@@ -54,17 +76,32 @@ async def fn() -> str:
 @pytest.mark.parametrize(
     "exc, expected_kind",
     [
+        # Auth/permission — subclass must match its specific bucket, NOT parent.
+        # OpikPermissionError extends OpikAuthError but must surface as
+        # "opik_permission_denied" so 403 vs 401 stay distinguishable in BI.
+        (OpikAuthError("x"), "opik_auth_failed"),
+        (OpikPermissionError("x"), "opik_permission_denied"),
+        (OpikNotFoundError("x"), "opik_not_found"),
+        (OpikValidationError("x"), "opik_validation_failed"),
+        (OpikServerError("x"), "opik_http_5xx"),
+        # Comet — same subclass-first contract as Opik.
         (CometAuthError("x"), "comet_auth_failed"),
-        (OllieNotEnabledError("x"), "ollie_not_enabled"),
+        (CometPermissionError("x"), "comet_permission_denied"),
         (CometProtocolError("x"), "comet_protocol_error"),
-        (OpikAuthError("x"), "opik_http_4xx"),
-        (OpikNotFoundError("x"), "opik_http_4xx"),
-        (OpikValidationError("x"), "opik_http_4xx"),
-        (OpikServerError("x"), "opik_http_5xx"),
+        # Ollie streaming.
+        (OllieNotEnabledError("x"), "ollie_not_enabled"),
         (PodNotReadyError("x"), "pod_warmup_timeout"),
         (OllieAuthError("x"), "ollie_auth_failed"),
         (OllieStreamError("x"), "ollie_stream_error"),
+        # Config / network / tool-args.
         (MissingConfigError("x"), "missing_config"),
+        # httpx network errors — common base RequestError covers the family.
+        (httpx.ConnectError("connect refused"), "network_error"),
+        (httpx.ReadTimeout("read timed out"), "network_error"),
+        (httpx.ReadError("read error"), "network_error"),
+        # pydantic validation on tool args.
+        (_build_pydantic_error(), "tool_args_invalid"),
+        # Genuine catch-all.
         (ValueError("x"), "unknown"),
     ],
 )

tests/test_opik_client.py

@@ -7,6 +7,7 @@
     OpikAuthError,
     OpikClient,
     OpikNotFoundError,
+    OpikPermissionError,
     OpikServerError,
     OpikValidationError,
 )
@@ -176,7 +177,7 @@ async def test_thread_comment_uses_thread_id_in_path() -> None:
     ("status", "expected_exc"),
     [
         (401, OpikAuthError),
-        (403, OpikAuthError),
+        (403, OpikPermissionError),
         (404, OpikNotFoundError),
         (400, OpikValidationError),
         (422, OpikValidationError),

tests/test_opik_client_read.py

@@ -14,6 +14,7 @@
     OpikAuthError,
     OpikClient,
     OpikNotFoundError,
+    OpikPermissionError,
     OpikServerError,
     OpikValidationError,
 )
@@ -273,7 +274,7 @@ async def test_list_name_filter_omitted_when_none(method: str, path: str) -> Non
     ("status", "expected_exc"),
     [
         (401, OpikAuthError),
-        (403, OpikAuthError),
+        (403, OpikPermissionError),
         (404, OpikNotFoundError),
         (400, OpikValidationError),
         (422, OpikValidationError),

tests/test_run_experiment.py

@@ -8,6 +8,7 @@
     OpikAuthError,
     OpikClient,
     OpikNotFoundError,
+    OpikPermissionError,
     OpikServerError,
     OpikValidationError,
 )
@@ -115,6 +116,22 @@ async def test_run_experiment_impl_maps_401_to_auth() -> None:
             )
 
 
+@pytest.mark.anyio
+async def test_run_experiment_impl_maps_403_to_permission() -> None:
+    """403 must surface as OpikPermissionError, not generic OpikAuthError —
+    so the analytics layer can bucket it as opik_permission_denied (workspace
+    mismatch) rather than opik_auth_failed (bad key)."""
+    with respx.mock(base_url=OPIK_BASE) as mock:
+        mock.post("/v1/private/experiments/execute").mock(return_value=httpx.Response(403))
+        with pytest.raises(OpikPermissionError):
+            await run_experiment_impl(
+                config=_config(),
+                client=_client(),
+                comet_base_url="https://www.comet.com",
+                workspace="ws",
+            )
+
+
 @pytest.mark.anyio
 async def test_run_experiment_impl_maps_503_to_server() -> None:
     with respx.mock(base_url=OPIK_BASE) as mock: