fix(anthropic): drop redundant bt_safe_deep_copy passes in tracing#583
Merged
Abhijeet Prasad (AbhiPrasad) merged 1 commit intoJul 16, 2026
Merged
Conversation
Braintrust already deep-copies via bt_safe_deep_copy in SpanImpl.log_internal,
so the eager copies in the Anthropic integration were duplicating work on every
span open/log. Remove them from _normalize_anthropic_input,
_get_input_from_kwargs, and _redact_server_tool_output; the remaining walks
(_normalize_anthropic_data, _process_input_attachments, and the inlined
redaction) already build fresh dicts/lists and never mutate user data.
This mirrors what google_genai/tracing.py does and matches the sdk-integrations
skill guidance ("Do not over-serialize. Braintrust serializes at send/log
time."). All existing cassette-backed tests pass on both matrix versions
(latest=0.116.0 and 0.48.0).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Abhijeet Prasad (AbhiPrasad)
approved these changes
Jul 16, 2026
Abhijeet Prasad (AbhiPrasad)
enabled auto-merge (squash)
July 16, 2026 21:28
Abhijeet Prasad (AbhiPrasad)
deleted the
fix/anthropic-drop-redundant-deep-copies
branch
July 16, 2026 21:35
2 tasks
Abhijeet Prasad (AbhiPrasad)
pushed a commit
that referenced
this pull request
Jul 17, 2026
…586) ## Summary - `_iter_tool_calls` ran `_try_to_dict(output)` (Pydantic `model_dump("python")`) on every chat/stream response just to probe for `tool_calls`, even when none were present — an extra recursive walk on top of the one `SpanImpl.log_internal` already runs via `bt_safe_deep_copy`. - Replaced the eager conversion with `_get_field(output, "tool_calls")`, which already handles both dict and Pydantic-object shapes. Downstream tool-call helpers (`_tool_call_name`, `_tool_call_input`, `_tool_call_metadata`) all read via `_get_field`, so behavior is preserved. - Matches the `sdk-integrations` skill guidance ("Do not over-serialize. Braintrust serializes at send/log time.") and mirrors #583 (anthropic). ## Test plan - [x] `nox -s "test_cohere(latest)"` (cohere==7.0.5) — 21 passed - [x] `nox -s "test_cohere(5.0.0)"` — 10 passed, 11 legitimately skipped for v2/audio surfaces not present on 5.0.0 - No cassettes re-recorded, no new tests — used the existing VCR-backed coverage in `test_cohere.py`, which already asserts tool-call span parent/child structure for both v1 (`test_wrap_cohere_chat_v1_tool_call_spans`) and v2 (`test_wrap_cohere_chat_v2_tool_call_spans`). 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- sfk:created-approved-by --> Created by abhijeet <!-- sfk:slack-thread --> [Slack thread](https://starfolkai.slack.com/archives/C0AQDETAVT3/p1784304169602969?thread_ts=1784304169.602969&cid=C0AQDETAVT3) Co-authored-by: Starfolk <noreply@starfolk.ai> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
7 tasks
Abhijeet Prasad (AbhiPrasad)
pushed a commit
that referenced
this pull request
Jul 17, 2026
…er serialization (#587) ## Summary Align the CrewAI integration with `.agents/skills/sdk-integrations/SKILL.md`. Audit turned up five issues: - **Missing `metadata.provider` on `crewai.llm` spans** — spec requires every `llm` span to carry both `metadata.model` and `metadata.provider`. Added `_provider_from_source()`, which reads the `provider` string off the emitting CrewAI `LLM` object (`"openai"` / `"anthropic"` / `"bedrock"` / ...). - **Tools in `input` instead of `metadata.tools`** — spec says tool definitions live in `metadata.tools`, not `input`. Moved both the LLM-call `tools` and the agent `tools` onto `metadata`. - **Token double-counting on `crewai.llm`** — the previous rule only skipped tokens when the LiteLLM integration was patched, but CrewAI 1.x routes `gpt-*` to a native `openai` client, `claude-*` to a native `anthropic` client, etc. When those integrations are patched (as `auto_instrument()` does by default), they emit a leaf span with tokens and `crewai.llm` double-counted at every ancestor in the trace-tree rollup. The SKILL is explicit here: "Do not add 'if OpenAI is patched, skip metrics' checks — define clear ownership instead." CrewAI is always an orchestration layer that delegates to a provider SDK, so the clear rule is: **`crewai.llm` never owns tokens**. Same pattern `pydantic_ai` already uses for its wrapper spans. Dropped `_litellm_owns_leaf_span` + the token-name/prefix maps + the `extra_metrics` plumbing on `_end_span` accordingly. - **Excess serialization** — dropped eager `_try_to_dict` / `_normalize_output` / `_normalize_tools` calls across kickoff, task, agent, LLM, and tool output paths. `bt_json._to_bt_safe` at log time already handles Pydantic v2 (`model_dump`), Pydantic v1 (`dict`), and dataclasses; the eager pre-pass was wasted work (same pattern as #585 for claude_agent_sdk and #583 for anthropic). All other metadata extractors (`_causal_metadata`, `_crew_metadata`, `_task_metadata`, `_LLM_CONFIG_FIELDS`) were already allowlist-based — no changes needed there. ## Test plan - [x] Extended `test_kickoff_llm_event_tree_parents_and_shape` to emit with a real `LLM` source and assert `metadata.provider == "openai"`. - [x] Added `test_llm_tools_route_to_metadata_not_input` (positive-in-metadata + negative-not-in-input). - [x] Replaced the two conditional token tests with a single `test_llm_never_emits_token_metrics` that asserts no token key (`tokens`, `prompt_tokens`, `completion_tokens`, `prompt_cached_tokens`, `completion_reasoning_tokens`) leaks onto `crewai.llm` regardless of which provider integrations are patched. - [x] **No new cassettes.** The test file docstring documents why VCR is impractical for CrewAI (pytest-vcr + httpcore + CrewAI's native `OpenAICompletion` `model_post_init` interaction bug). Direct-event tests remain the source of truth here; the LiteLLM `mock_response` smoke test still exercises the full `crew.kickoff()` path. - [x] `cd py && BRAINTRUST_TEST_PACKAGE_VERSION=latest pytest src/braintrust/integrations/crewai/test_crewai.py -v` — 17 passed - [x] `cd py && nox -s pylint` — success - [x] `pre-commit run` on the changed files — passed (ruff format, ruff check, codespell, EOF/trailing-whitespace) 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- sfk:created-approved-by --> Created by abhijeet <!-- sfk:slack-thread --> [Slack thread](https://starfolkai.slack.com/archives/C0AQDETAVT3/p1784304223300019?thread_ts=1784304223.300019&cid=C0AQDETAVT3) --------- Co-authored-by: Starfolk <noreply@starfolk.ai> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Merged
5 tasks
Abhijeet Prasad (AbhiPrasad)
pushed a commit
that referenced
this pull request
Jul 17, 2026
…op eager serialization (#590) ## Summary Align the Google GenAI integration with `.agents/skills/sdk-integrations/SKILL.md`. Audit turned up four issues: - **Missing `metadata.provider` on every span** — spec requires every `llm` span to carry both `metadata.model` and `metadata.provider`. Added `metadata.provider = \"google\"` on `generate_content`, `generate_content_stream`, `embed_content`, `generate_images`, and every `interactions.*` span. - **Tools in `input.config` instead of `metadata.tools`** — spec says tool definitions live in `metadata.tools`. Route the serialized Google-native tool declarations onto `metadata.tools` for the models path, and route the interactions `tools` kwarg onto `metadata.tools` for the interactions path. New `_config_for_input()` strips `tools` from the config that flows into `input` via a shallow field iteration (no full model_dump). - **Excess serialization** — dropped `bt_safe_deep_copy(config)` in `_serialize_input` and `_prepare_generate_images_traced_call` (span logging already runs `bt_safe_deep_copy` at send time, so the eager copy was duplicate work — same pattern as #583 for anthropic and #585 for claude_agent_sdk). Also dropped the eager `grounding_metadata.model_dump(exclude_none=True)` in `_aggregate_generate_content_chunks`, and the eager `_materialize_interaction_value(...)` on interaction tools. - **Denylist → allowlist** in `_tool_span_input` / `_tool_span_output`. Replaced `key not in {\"id\", \"name\", \"type\", \"signature\", \"server_name\", ...}` with explicit `_TOOL_CALL_INPUT_FIELDS = (\"arguments\", \"code\", \"language\", \"url\", \"query\")` and `_TOOL_RESULT_OUTPUT_FIELDS = (\"result\", \"output\", \"outcome\", \"content\")`. Any unrecognized field on a call/result payload is dropped rather than passed through. Also trimmed narrative docstrings/comments that just described what the code does; kept the section dividers and the two \"why\" comments about interaction tool span lifetime. ## Test plan - [x] Extended existing VCR-backed tests to assert `metadata.provider == \"google\"`, `metadata.tools` presence for both `models.generate_content` and `interactions.create` tool cases, and negative assertions that tools do not leak into `input.config` / `input`. - [x] `cd py && CI=1 BRAINTRUST_TEST_PACKAGE_VERSION=latest pytest src/braintrust/integrations/google_genai/test_google_genai.py` — 45 passed - [x] `cd py && CI=1 BRAINTRUST_TEST_PACKAGE_VERSION=1.75.0 pytest src/braintrust/integrations/google_genai/test_google_genai.py` — 45 passed - [x] `cd py && CI=1 BRAINTRUST_TEST_PACKAGE_VERSION=1.30.0 pytest src/braintrust/integrations/google_genai/test_google_genai.py` — 38 passed, 7 pre-existing skips (interactions API unavailable on that version) - [x] **No cassettes re-recorded, no new mocks/fakes** — used the existing VCR-backed suite. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- sfk:created-approved-by --> Created by abhijeet <!-- sfk:slack-thread --> [Slack thread](https://starfolkai.slack.com/archives/C0AQDETAVT3/p1784308772517939?thread_ts=1784308772.517939&cid=C0AQDETAVT3) Co-authored-by: Starfolk <noreply@starfolk.ai> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
5 tasks
Abhijeet Prasad (AbhiPrasad)
pushed a commit
that referenced
this pull request
Jul 21, 2026
…ta pass-through (#604) ## Summary Aligns the Mistral integration with `.agents/skills/sdk-integrations/SKILL.md`, following the same pattern as #594 (litellm) and #583 (anthropic). Audit turned up two issues: - **Excess serialization.** `_normalize_mistral_multimodal_value` (which recursively calls `value.model_dump(mode="python", by_alias=True)`) was invoked 25 times across `tracing.py`. Two sites did real waste: - Every non-streaming response finalizer (`_finalize_completion_response`, `_finalize_conversation_response`, `_finalize_ocr_response`, `_finalize_embeddings_response`) full-dumped the response just to read a handful of fields (`id`, `model`, `choices`/`outputs`/`pages`, `usage`). - `_build_request_metadata` walk-and-dumped every metadata value (`temperature`, `top_p`, `tools`, `response_format`, `stop`, …) — none of which carry attachments. Both are redundant: `bt_safe_deep_copy` in `logger.log_internal` already `model_dump`s pydantic values at log time. Replaced with attribute reads through the existing `_get_value` helper (already dict-or-pydantic aware and handles Mistral's `Unset` sentinel — no new helper needed). - **Aggregators.** `_aggregate_completion_events` / `_aggregate_conversation_events` / `_aggregate_transcription_events` / `_aggregate_speech_events` pre-dumped `usage` / `segments` into the intermediate dict. `_parse_usage_metrics` already accepts pydantic, and `bt_safe_deep_copy` handles the output. Dropped. - **Tool-span walkers.** `_completion_tool_calls`, `_conversation_tool_outputs`, `_log_completion_tool_spans`, `_log_conversation_tool_spans` switched to `_get_value` so they walk raw pydantic responses instead of assuming pre-dumped dict shape. Multimodal walker retained where attachment materialization matters (`_start_span` input path, `_append_delta_content` / `_merge_tool_calls` streaming assembly, per-field conversation-output accumulators). Metadata is still allowlist-per-surface — no changes to what fields we capture. Comments were not excessive; nothing to trim. Net: −36 LOC in `tracing.py`, one dead helper file removed (`_response_data_to_metadata`, `_conversation_response_data_to_metadata`, `_conversation_outputs_data`, `_ocr_output_data` collapsed into the pydantic-aware equivalents). ## Test plan - [x] **No new mocks/fakes.** The two existing narrow fakes (`_PlainResponse` for the plain-Python usage path, and lambda fail-injectors for error-propagation tests) fall under the SKILL's "narrow error injection / provider-independent helpers" carve-out. All provider-behavior coverage stays on the 30+ `@pytest.mark.vcr` cassette-backed tests. - [x] **No cassette re-recording.** HTTP behavior is unchanged; only in-process span shaping was touched. - [x] `cd py && nox -s "test_mistral(latest)"` — 45/45 passed. - [x] `cd py && nox -s "test_mistral(1.12.4)"` — 43/43 passed (2 pre-existing skips for speech API which doesn't exist on 1.12.4). - [x] `ruff check` + `ruff format --check` on touched files — clean. Two existing unit tests were updated because their assertions hinged on the old eager-serialization behavior: - `test_wrappers_ignore_usage_when_response_normalization_fails` → `test_wrappers_read_usage_from_plain_python_responses`. The old test asserted metrics were *dropped* for plain-Python responses — that only happened because `.model_dump()` failed on non-pydantic objects and we bailed out. The new code correctly reads `usage` via `getattr`; assertion is now positive (metrics *are* present). - `test_aggregate_completion_events_merges_tool_calls_and_content`: `aggregated["usage"]` is now the raw pydantic `UsageInfo` (dumps at log time, not in the aggregator); switched to `getattr(..., "total_tokens")`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- sfk:created-approved-by --> Created by abhijeet <!-- sfk:slack-thread --> [Slack thread](https://starfolkai.slack.com/archives/C0AQDETAVT3/p1784591709340249?thread_ts=1784591709.340249&cid=C0AQDETAVT3) Co-authored-by: Starfolk <noreply@starfolk.ai> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
8 tasks
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
2 participants
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
SpanImpl.log_internalalready runsbt_safe_deep_copyon every logged event (seelogger.py:4699), so the eager copies in_normalize_anthropic_input,_get_input_from_kwargs, and_redact_server_tool_outputwere duplicating work on every span open/log.bt_safe_deep_copycalls and dropped the now-unused import. The remaining walks (_normalize_anthropic_data,_process_input_attachments, inlined redaction) already build fresh dicts/lists and never mutate user data, so behavior is preserved.google_genai/tracing.pydoes today and matches thesdk-integrationsskill guidance: "Do not over-serialize. Braintrust serializes at send/log time."Test plan
pytest src/braintrust/integrations/anthropic/test_anthropic.pyon anthropic==0.116.0 (matrixlatest) — 48 passedpytest src/braintrust/integrations/anthropic/test_anthropic.pyon anthropic==0.48.0 — 39 passed, 9 legitimately skipped for features unavailable on that version🤖 Generated with Claude Code
Created by abhijeet
Slack thread