feat(analytics): bucket error_kind on tool_called + ask_ollie_completed#124
Merged
Conversation
Replace the per-class fine-grained error_kind values (opik_auth_failed, ollie_stream_error, network_error, …) with a 9-bucket coarse taxonomy shared by both opik_mcp_tool_called and opik_mcp_ask_ollie_completed: auth, validation, not_found, permission, timeout, network, upstream_5xx, cancelled, unknown. The granular signal is preserved on a new exception_type property (class name), and HTTP-bearing exceptions also emit http_status. The SSE error frame's optional code field is captured on ask_ollie as upstream_error_code (length-capped at 64 chars). The new opik_mcp/analytics/errors module is class-only (never reads exc.args / str(exc)) so the privacy contract is preserved end-to-end; new canary tests in test_analytics_privacy.py drive the failure paths through the real tool surfaces (server.read, run_ask_ollie) to make sure no exception message ever surfaces in an analytics event. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…cals - Add a privacy test that emits a >64-char upstream_code and asserts the truncated tail never reaches the recorded analytics payload. The previous canary was 36 chars long, so the [:64] slice was a no-op — the test would have passed even if the cap regressed. - Tighten ``error_upstream_code`` to ``str | None`` in ask_ollie.py (was ``Any``) so a future refactor that accidentally stores a non-string is caught by mypy at the assignment site rather than silently emitted via ``isinstance(..., str)`` falsiness later on. - Add 408 to the bucket_exception(HTTPStatusError) parametrize. bucket_http_status(408) was tested directly, but the chained HTTPStatusError → bucket_http_status path was only covered for 504. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
awkoy
added a commit
that referenced
this pull request
May 25, 2026
PR #124 replaced the in-wrappers dispatch table with `bucket_exception` in `analytics/errors.py`. Two cardinality-contract comments still pointed at the old symbol; update them so future readers find the live implementation. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
error_kindvalues onopik_mcp_tool_calledandopik_mcp_ask_ollie_completedwith a coarse 9-bucket taxonomy (auth,validation,not_found,permission,timeout,network,upstream_5xx,cancelled,unknown) that dashboards can group on without per-class enumeration.exception_type(class name) on both events so the granular signal is still available for drill-downs, plushttp_statusfor typed Opik/Comet errors andhttpx.HTTPStatusError.ask_ollie_completedalso getsupstream_error_code(length-capped at 64 chars) sourced from the SSEerrorframe's optionalcodefield.opik_mcp/analytics/errorsmodule that is class-only (never readsexc.args/str(exc)/ response body) so the privacy contract is preserved end-to-end.Why
Today
tool_called.error_kindandask_ollie_completed.error_kindeach define their own per-class enum (16+ values combined). Dashboards have to maintain a hand-rolled allowlist per event and any new typed exception breaks the chart silently. The coarse taxonomy folds the noise into 9 fixed buckets whileexception_typekeeps the granular signal as a free-text class name — net more information than before, no harder-to-evolve enum on the BI side.Test plan
uv run pytest tests/test_analytics_*.py -q— green (107 tests)uv run pytest tests/ -q— green (584 passed, 2 skipped)uv run ruff check src/ tests/— cleanuv run ruff format --check src/ tests/— cleanuv run mypy src/— cleantool_called(viaserver.read+OpikAuthErrorwith a PII message) andask_ollie_completed(via a fake pod streaming an SSEerrorframe with a PII message + structuredcode).🤖 Generated with Claude Code