diff --git a/.agents/skills/sdk-integrations/SKILL.md b/.agents/skills/sdk-integrations/SKILL.md index e74aac300..92eb36aae 100644 --- a/.agents/skills/sdk-integrations/SKILL.md +++ b/.agents/skills/sdk-integrations/SKILL.md @@ -1,17 +1,21 @@ --- name: sdk-integrations -description: Create or update Braintrust Python SDK integrations built on the integrations API under `py/src/braintrust/integrations/`. Use when adding a new integration package, extending an existing provider integration, changing patchers, tracing, manual `wrap_*()` helpers, integration exports, `auto_instrument()` wiring, `py/noxfile.py` sessions, integration tests, or cassettes. Do not use when migrating an existing legacy wrapper from `py/src/braintrust/wrappers/` into the integrations API; use `sdk-wrapper-migrations` for that. +description: Create or update Braintrust Python SDK integrations built on the integrations API under `py/src/braintrust/integrations/`. Use when adding a new integration package, extending an existing provider integration, changing patchers, tracing, span shape, manual `wrap_*()` helpers, integration exports, `auto_instrument()` wiring, `py/noxfile.py` sessions, integration tests, or cassettes. --- # SDK Integrations -Work under `py/src/braintrust/integrations/`. Use `sdk-wrapper-migrations` instead if the task is moving a real implementation from `py/src/braintrust/wrappers//` into the integrations API. +Work under `py/src/braintrust/integrations/`. For cassette recording and VCR debugging, also use `sdk-vcr-workflows`. ## Spec -Authoritative source for span shape, allowed fields, and instrumentation rules: https://github.com/braintrustdata/braintrust-spec/blob/main/docs/instrumentation-guide.md +The instrumentation spec is the authority for span shape, allowed fields, metrics, and instrumentation rules. Start at https://github.com/braintrustdata/braintrust-spec/blob/main/skills/instrumentation-spec/SKILL.md. It points to `references/instrumentation-guide.md` (the full guide) and `references/features/*.md` (decision spans, MCP servers, prompt cache, attachments, token and cost metrics, and others). Fetch with: -Read it first for anything involving span type/name, `input`/`output`/`metadata`/`metrics`/`error`/`context` shape, tool calls, prompt provenance, streaming, reasoning models, prompt caching, multimodal, completion-vs-agentic, or new attribute namespacing. Do not invent new top-level span fields, metric keys, or span types; update the spec first. +```bash +gh api repos/braintrustdata/braintrust-spec/contents/skills/instrumentation-spec/references/instrumentation-guide.md --jq .content | base64 --decode +``` + +There is no `docs/instrumentation-guide.md` in that repo; it moved in July 2026. Read the spec sections and feature files that cover the span shape you are changing before you write code. Do not invent new top-level span fields, metric keys, or span types; change the spec first. ## Read First @@ -20,16 +24,18 @@ Always: `integrations/base.py`, `integrations/versioning.py`, `integrations/__in Existing integration: `integrations//{__init__,integration,patchers,tracing,test_*}.py`. Situational: -- `auto.py` + `integrations/auto_test_scripts/` — anything touching `auto_instrument()` or import timing +- `auto.py` + `integrations/auto_test_scripts/` — anything touching `auto_instrument()` or import timing. Import-order and subprocess regressions usually only surface here. - `conftest.py`, `integrations/conftest.py` — VCR / per-version cassette resolution - `integrations/test_utils.py` — shared attachment / multimodal helpers -- `integrations/{adk,anthropic,google_genai}/tracing.py` — attachment + multimodal reference implementations -Import-order and subprocess regressions usually only surface via `auto_test_scripts/`. Do not skip it. +References in this skill (open when the task needs them): +- [references/span-design.md](references/span-design.md) — span type/name, completion vs agentic, payload format, `metadata` fields, token ownership, metric keys, attachments. Open before defining or changing span shape. +- [references/span-origin.md](references/span-origin.md) — the `_INSTRUMENTATION = "-auto"` pattern. Open when creating spans in a new integration or a new code path. +- [references/streaming-lifecycle.md](references/streaming-lifecycle.md) — exit paths and GC finalization for streamed responses. Open when wrapping any stream. ## Version And CI Routing -Never guess versions or session names. The routing chain: +Never guess versions or session names: - `pyproject.toml` `[tool.braintrust.matrix]` — supported provider versions, what `latest` resolves to - `pyproject.toml` `[tool.braintrust.cassette-dirs]` — versioned cassette directory ownership @@ -39,9 +45,7 @@ Never guess versions or session names. The routing chain: When changing version-gated behavior: identify every matrix version, check `min_version`/`max_version`/`superseded_by`/feature-detect branches, test the narrowest affected version first, re-record cassettes only for versions whose observable provider behavior intentionally changed. -## Pick A Reference - -Start from the nearest existing integration: +## Pick A Reference Integration - **ADK** — direct method patching, `target_module`, `CompositeFunctionWrapperPatcher`, manual `wrap_*()`, context propagation, inline data → `Attachment` - **Agno** — multi-target patching, related patchers, version-conditional fallbacks with `superseded_by` @@ -52,278 +56,74 @@ Start from the nearest existing integration: 1. Read the spec section(s) covering the affected span shape. 2. Read the provider package + shared primitives. -3. Decide completion-style vs agentic-style (see [Span Design](#span-design)). +3. Define span shape (`type`, `name`, `input`, `output`, `metadata`, `metrics`, `error`) before writing patchers. See [references/span-design.md](references/span-design.md). 4. Pick the public API surface to patch — prefer stable public entry points over internal helpers. -5. Define span shape (`type`, `name`, `input`, `output`, `metadata`, `metrics`, `error`) before writing patchers. -6. Implement patchers, then tracing helpers. -7. For behavior changes: add/update a failing test first, then fix (red → green). Cassette-backed if the provider payload triggers the bug. -8. Run the narrowest nox session first. Expand only if shared code changed. - -Do not start by wiring patchers and defer span-shape decisions. +5. Implement patchers, then tracing helpers. +6. For behavior changes: add/update a failing test first, then fix (red → green). Cassette-backed if the provider payload triggers the bug. +7. Run the narrowest nox session first. Expand only if shared code changed. +8. Consolidate tests (see [Testing Rules](#testing-rules)) before handoff. ## Route The Task -### New integration - -1. Create `integrations//` with `__init__.py`, `integration.py`, `patchers.py`, `tracing.py`, `test_.py`, and `cassettes//` (one dir per matrix version + `latest/`). +**New integration:** +1. Create `integrations//` with `__init__.py` (exports, `setup_()`, `wrap_*()`), `integration.py` (thin `BaseIntegration` subclass), `patchers.py`, `tracing.py`, `test_.py`, and `cassettes//` (one dir per matrix version + `latest/`). 2. Export from `integrations/__init__.py`. -3. Add the provider session in `noxfile.py`. -4. Update `auto.py` only if it should participate in `auto_instrument()`; add a subprocess test in `auto_test_scripts/` when it does. -5. Add the `_INSTRUMENTATION = "-auto"` shadow (see [Span origin](#span-origin)) and assert it in the test suite. - -### Existing integration update - -Change only affected patchers, tracing, exports, tests, cassettes. Preserve the public `setup_*()` / `wrap_*()` surface. Don't touch `auto.py` / `integrations/__init__.py` / `noxfile.py` unless required. Check whether the change also needs a subprocess auto-instrument test update. +3. Add the provider session in `noxfile.py` and the `[tool.braintrust.cassette-dirs]` entry in `pyproject.toml`. +4. Update `auto.py` only if it should participate in `auto_instrument()`; prefer `_instrument_integration(...)` over a custom helper, and add a subprocess test in `auto_test_scripts/`. +5. Add the span-origin shadow ([references/span-origin.md](references/span-origin.md)) and assert it in tests. -### `auto_instrument()` only +**Existing integration update:** change only affected patchers, tracing, exports, tests, cassettes. Preserve the public `setup_*()` / `wrap_*()` surface. Don't touch `auto.py` / `integrations/__init__.py` / `noxfile.py` unless required. -Update `auto.py`. Prefer `_instrument_integration(...)` over a custom `_instrument_*` helper. Add/update the subprocess auto-instrument test. +**Entry-point parity:** `setup_()`, `wrap_*()`, and `auto_instrument()` must emit equivalent spans. If a behavior change lands in one, check the other two. Validate `auto_instrument()` with a subprocess test, not just in-process. -### Entry-point parity - -The three entry points must emit equivalent spans: -- `setup_()` — package-level patching -- `wrap_*()` helpers — manual wrapping -- `auto_instrument()` — import-order-sensitive discovery - -If a behavior change lands in one, check the other two. Validate `auto_instrument()` with a subprocess test, not just in-process. - -## Package Layout - -Ownership: -- `__init__.py` — public exports, `setup_()`, `wrap_*()` helpers -- `integration.py` — `BaseIntegration` subclass + patcher registration (keep thin) -- `patchers.py` — patchers and manual wrapping helpers -- `tracing.py` — request/response normalization, metadata extraction, stream handling, error logging -- `test_*.py` — provider behavior tests -- `cassettes/` — VCR recordings - -If logic is shared across integrations, put it in `integrations/utils.py`, not copies. Check `utils.py` and neighboring integrations before writing a local helper. +Shared logic goes in `integrations/utils.py`, not copies. Check it and neighboring integrations before writing a local helper. ## Integration Rules -Declarative setup on the `BaseIntegration` subclass: `name`, `import_names`, `patchers`. Use `min_version`/`max_version` only when feature detection isn't enough. Prefer `detect_module_version(...)` / `version_satisfies(...)` / `make_specifier(...)`. - -Let `BaseIntegration.resolve_patchers()` reject duplicate patcher ids; don't hide duplicates. +Declarative setup on the `BaseIntegration` subclass: `name`, `import_names`, `patchers`. Use `min_version`/`max_version` only when feature detection isn't enough. Prefer `detect_module_version(...)` / `version_satisfies(...)` / `make_specifier(...)`. Let `BaseIntegration.resolve_patchers()` reject duplicate patcher ids. -**Non-invasiveness:** - Provider errors MUST propagate. Never swallow or rewrap. -- Preserve return types, iterator/async-iterator semantics, generator/subclass behavior. -- Sync and async traced schemas MUST stay aligned when both exist. +- Preserve return types, iterator/async-iterator semantics, generator/subclass behavior. Keep sync and async traced schemas aligned. - Setup / teardown / patching MUST be idempotent (rely on the base patcher marker). -- Contain instrumentation failures. Extraction/normalization/logging errors must be logged or ignored, never raised into the user's call path. -- Treat provider inputs/results/events/headers as untrusted. Avoid arbitrary attribute access; don't mutate third-party objects. -- Only instrument AI-generation-relevant ops (LLM calls, embeddings, tool exec, media gen, agent runs). No unrelated CRUD. - -## Span Design - -### Type + name - -- `llm` — one provider API call -- `task` — parent for an agent run, pipeline step, or named operation -- `tool` — model-initiated tool/function execution - -Use spec-recommended names (`Chat Completion`, `anthropic.messages.create`, `generate_content`). For new providers, pick a stable provider-specific name. - -### Completion-style vs agentic-style - -- **Completion** (single request/response, user runs any tool calls): one `llm` span, no children. Tool calls appear in `output`. -- **Agentic** (SDK/framework runs the tool loop): one parent `task` wrapping child `llm` (per model call) + `tool` (per tool exec) spans in execution order. - -Completion-style *frameworks* (LiteLLM, OpenRouter, etc.) still emit `llm` spans with `metadata.provider` set to the underlying provider, NOT the framework name. Agentic frameworks (Vercel AI SDK w/ tools, LangChain agents, OpenAI Agents SDK, Claude Agent SDK, ADK, Agno) emit the full parent+children tree. - -### Payload format - -Canonical shape is OpenAI Chat Completions format. Providers with dedicated UI normalizers (OpenAI, Anthropic, Google) MAY preserve provider-native payloads if `metadata.provider` is set correctly; all other providers MUST convert to OpenAI shape. If a provider passes system prompt separately, insert it as a `role: "system"` entry. - -### Fields - -- `input` — meaningful user request (messages / prompt / provider-native input) -- `output` — meaningful provider result (normalized, not opaque SDK instances) -- `metadata` — see below -- `metrics` — spec-listed keys only (see [Metrics](#metrics)) -- `error` — pass the `Exception` instance directly to span logging; do not pre-format - -**Every `llm` span MUST include `metadata.model` and `metadata.provider`** (`provider` = whose pricing applies, even when going through a gateway or framework). - -**Tool definitions go in `metadata.tools`** (OpenAI-shaped regardless of underlying provider), NOT in `input` messages. Preserve provider-native built-in tool types (Anthropic `computer_use`, OpenAI `web_search`) as opaque JSON-serializable entries. Never log executable tool handlers. - -**Prompt provenance goes in `metadata.prompt`** (`id`, `project_id`, `version`, `variables`, plus `prompt_session_id` for playground calls). Not in `input`, request payloads, or `metadata.tools`. Strip carrier fields like `span_info` before logging. - -**`metadata` allowlist-per-provider:** capture the spec-defined keys plus provider-specific detail fields (request/response ids, safety data, model-family flags) chosen deliberately. Do not dump entire raw request/response objects. Redact secrets. If a provider-specific field ends up broadly useful, promote it to the spec. - -### Token metrics - -The integration that directly owns the model/provider API response owns token accounting. Orchestration/framework integrations should not log token metrics when underlying provider integrations create leaf spans with usage. Agentic parent `task` spans MAY aggregate across children when the framework doesn't delegate to a separately instrumented provider client. Do not add "if OpenAI is patched, skip metrics" checks — define clear ownership instead. - -**Framework `llm`-like spans that delegate to a separately instrumented client:** if the framework's per-model-call span sits above a call the framework itself dispatches through another patched client (e.g. DSPy's LM callback → LiteLLM → OpenAI), do NOT type the framework's span `llm`. Two nested `llm` spans for one API call is confusing in the UI and — because the framework typically cannot supply tokens from its own callback contract — would produce a token-less `llm` span that violates the "tokens required for LLM spans" rule. Leave the framework span untyped (or `task`), keep `metadata.model` / `metadata.provider` on it for attribution, and let the underlying provider integration own the `llm` leaf with usage. The same logic applies to any framework that always delegates the transport layer to another instrumentable client. - -### Shaping guidance - -- Flatten positional args into named fields; normalize SDK objects into dicts/lists/scalars; drop noisy transport fields. -- Aggregate streaming chunks into one final `output` + stream-specific `metrics`. One `llm` span per API call, not per chunk. -- Do not over-serialize. Braintrust serializes at send/log time. Integration tracing only needs readable Python dicts/lists/scalars and materialized attachments. -- Keep wrapper bodies thin: prepare traced input, open span, call provider, normalize result, log. -- Do not wrap `span.log(...)` / `span.set_attributes(...)` in broad try/except. Braintrust span methods are boundary-safe. -- **Rerank exception to "no silent caps":** rerank result lists can be huge and are noisy in the span UI; cap the `output` list at a fixed max (e.g. 100 entries) via a named constant. The cap is compact-span hygiene, not coverage bounding — the model already ranked everything. - -### Span origin - -Every span an integration creates MUST carry `context.span_origin.instrumentation.name = "-auto"` (e.g. `openai-auto`, `anthropic-auto`, `adk-auto`, `google-genai-auto`). Match the JS SDK exactly for cross-SDK filtering. - -`start_span(...)` and all provider-level `start_span` methods accept `internal: SpanInternalOptions | None`, a TypedDict reserved for SDK internals — external callers should not use it. Integrations pass `internal={"instrumentation": "-auto"}`. When unset, spans fall back to the channel default (`braintrust-python-logger` / `braintrust-python-otel`). - -The name does NOT propagate through the parent/child edge — each `start_span` call decides independently. User-owned spans nested inside a wrapped call (e.g. a `@traced` scorer) must NOT inherit the integration's name. - -Standard pattern at the top of `tracing.py` (or `callbacks.py` / `plugin.py`): - -```python -from braintrust.logger import start_span as _bt_start_span - -_INSTRUMENTATION = "-auto" - - -def start_span(*args, **kwargs): - internal = dict(kwargs.get("internal") or {}) - internal.setdefault("instrumentation", _INSTRUMENTATION) - kwargs["internal"] = internal - return _bt_start_span(*args, **kwargs) -``` - -Module-level `start_span(...)` calls flow through automatically. Calls that go through a `Logger` / `Experiment` / parent-`Span` instance (`logger.start_span(...)`, `parent.start_span(...)`) need `internal={"instrumentation": _INSTRUMENTATION}` explicitly. Tests assert the exported provenance: `span["context"]["span_origin"]["instrumentation"]["name"] == "-auto"`. - -## Metrics - -Only emit spec-listed keys: -- `tokens`, `prompt_tokens`, `completion_tokens` — required for LLM spans; values MUST be non-negative -- `time_to_first_token` — required for streaming; SDK measures from request start to first chunk -- `completion_reasoning_tokens` — required when provider reports reasoning tokens (o-series etc.) -- `prompt_cached_tokens`, `prompt_cache_creation_tokens`, `prompt_cache_creation_5m_tokens`, `prompt_cache_creation_1h_tokens` — provider prompt caching -- `prompt_audio_tokens`, `completion_audio_tokens`, `completion_image_tokens` — audio/image models when provider reports -- `start`, `end` — standard span timing - -Map provider modality breakdowns only to standardized metrics: input audio → `prompt_audio_tokens`, output audio → `completion_audio_tokens`, and output image → `completion_image_tokens`. These are subsets of prompt/completion usage, not additional tokens. Preserve unstandardized modality breakdowns as deliberately named provider metadata rather than inventing metric keys. - -For streaming, still produce one span per API call with accumulated `input`/`output`. Capture usage from stream metadata (e.g. OpenAI `stream_options.include_usage`) when surfaced. - -For reasoning models, capture the full output structure (reasoning summaries + message blocks) and include prior reasoning in `input` for multi-turn calls. - -## Multimodal And Attachments - -Inline binary media (images, PDFs, audio, video) MUST be replaced by a Braintrust attachment at the leaf position. Do not add a separate top-level `attachments` list. - -- Prefer `_materialize_attachment(...)` in `integrations/utils.py`. Don't reimplement base64/file decoding. -- Convert raw `bytes` / base64 / data URLs / file inputs / generated media → `braintrust.logger.Attachment`. Preserve remote URLs as strings — do not fetch to create an attachment. -- Payload shape after materialization: - - images → `{"image_url": {"url": attachment}}` - - non-image media/documents/files → `{"file": {"file_data": attachment, "filename": resolved.filename}}` -- Do not force non-image payloads through `image_url`. -- If materialization fails, keep the original value; never drop, null it out, or raise. -- Preserve non-attachment values while walking nested payloads unless intentionally normalizing. -- Keep MIME type, size, safety data, filenames, provider ids next to the attachment. -- Providers with UI normalizers (OpenAI, Anthropic, Google) — MAY preserve the provider-native structure and only replace the raw media leaf. -- Generated media → log the attachment in `output`, not `metadata`. For streaming, aggregate into one final `output` attachment. +- Instrumentation failures (extraction, normalization) must never raise into the user's call path. But do not wrap `span.log(...)` / `span.set_attributes(...)` in try/except — they are boundary-safe. +- Treat provider inputs/results/events/headers as untrusted. Don't mutate third-party objects. +- Only instrument AI-generation-relevant ops (LLM calls, embeddings, tool exec, media gen, agent runs). +- Do not over-serialize. `span.log` already runs `bt_safe_deep_copy` (pydantic, dataclasses, attachments). Log readable dicts/lists/scalars and materialized attachments; no JSON dumps/loads or recursive conversion in tracing code. +- Every `llm` span has `metadata.model` and `metadata.provider` (whose pricing applies, not the framework name). ## Patcher Rules -One patcher per coherent target. Prefer: - -- `FunctionWrapperPatcher` — one import path / one constructor / one method surface -- `CompositeFunctionWrapperPatcher` — one logical surface across multiple related targets -- `CallbackPatcher` — setup side effects after applicability succeeds - -Use `target_module` for patch targets outside the module named by `import_names` (deep or optional submodules). Use `superseded_by` for version-conditional fallbacks — not custom target-selection logic. Use lower `priority` only when ordering matters (e.g. context propagation before tracing). - -Manual wrapping helpers stay thin: - -```python -def wrap_agent(Agent: Any) -> Any: - return AgentPatcher.wrap_target(Agent) -``` - -Every patcher needs: stable `name`, clean existence checks, version gating only when necessary, idempotence via the base patcher marker. Prefer patching stable public API surfaces; internal helpers need version-specific maintenance. +One patcher per coherent target: `FunctionWrapperPatcher` (one import path / method surface), `CompositeFunctionWrapperPatcher` (one logical surface across related targets), `CallbackPatcher` (setup side effects). Use `target_module` for deep or optional submodules, `superseded_by` for version-conditional fallbacks, lower `priority` only when ordering matters. Manual `wrap_*()` helpers stay thin: `return AgentPatcher.wrap_target(Agent)`. ## Testing Rules -Tests live in the provider package. Default bug-fix workflow: red → green — add/update a failing test first, then fix. - -**Prefer VCR-backed real provider coverage with `@pytest.mark.vcr`.** The burden of proof is on mocks: use them only for narrow error injection, purely local version-routing logic, patcher existence checks, or provider-independent helpers where the response shape is not part of the contract. Do not swap a cassette-backed regression for a mock just because the fix lives in `tracing.py` or a serializer — if a real payload triggers the bug, the primary regression stays cassette-backed. - -Assert on emitted spans (not just provider return values): -- span `type` and `name` -- `input` shape (spec-required messages/prompt/config fields) -- `output` shape (normalized, not opaque SDK instances) -- `metadata.model`, `metadata.provider`, `metadata.tools`/`tool_choice`/`prompt` when present -- required `metrics` keys (tokens for LLM; `time_to_first_token` for streaming) -- parent/child structure for agentic APIs (parent `task`, child `llm`+`tool` in execution order) -- `context.span_origin.instrumentation.name == "-auto"` -- attachments: images under `image_url.url`, non-images under `file.file_data`, `Attachment` objects (not raw bytes/base64) -- error propagation, error logging -- setup/teardown/wrapping idempotence, patcher resolution when relevant - -For streaming, assert both the provider iterator/async-iterator still works AND the final span has aggregated `output` + stream-specific `metrics`. - -### Streaming lifecycle review - -Review every exit path, not just full consumption. Check the real provider's -iterator, context-manager, cancellation, and garbage-collection behavior before -choosing a wrapper. A proxy can preserve transport cleanup while still losing -the span's final output and end time. - -- Cover exhaustion, provider errors, explicit close/cancel, and context-manager - exit where the provider supports it. Preserve exception and return semantics. -- Cover `break` followed by dropping the last stream reference, and dropping a - stream before consuming any chunks. `break` alone does not close a retained - iterator; do not promise immediate finalization while callers still hold it. -- Use existing recordings for partial-consumption tests. Drop the proxy, force - collection, and assert partial (or empty) output, `metrics.end`, correct - parentage, and no duplicate finalization after explicit close or exhaustion. -- A GC fallback must not retain the stream through its callback or closure. - For async streams, do not run or schedule event-loop work from a finalizer; - finalize trace state and preserve the provider's own cleanup behavior. -- Check that neither iteration nor cleanup leaves the stream span current in - the caller's context. GC is a best-effort fallback, not a substitute for - deterministic cleanup when the caller explicitly closes the stream. - -Cassettes live in `integrations//cassettes//` (e.g. `cassettes/latest/`, `cassettes/0.48.0/`). Nox sets `BRAINTRUST_TEST_PACKAGE_VERSION` so cassettes land correctly. Do not add per-test `vcr_cassette_dir` / `cassette_library_dir` fixtures — `integrations/conftest.py` handles it. Re-record only when behavior intentionally changed. Sanitize binary media in both request and response bodies so checked-in cassettes do not retain large base64 payloads. - -Confirm the exact session name from `noxfile.py` — don't assume it matches the folder. +Tests live in the provider package. Red → green for bug fixes. + +- **VCR, not mocks or fakes.** Use `@pytest.mark.vcr` with real provider recordings. Mocks are only for narrow error injection, purely local version-routing logic, or patcher existence checks. A fix in `tracing.py` still gets a cassette-backed regression when a real payload triggers the bug. Hand-built stand-ins for provider objects count as fakes. +- **Assert on emitted spans**, not just provider return values: `type`, `name`, `input`/`output` shape, `metadata.model`/`provider`/`tools`, required `metrics`, parent/child order for agentic APIs, `context.span_origin.instrumentation.name`, attachments, error logging. +- **Streaming:** assert the provider iterator still works and the final span has aggregated `output` + `time_to_first_token`. Follow [references/streaming-lifecycle.md](references/streaming-lifecycle.md) for exit-path coverage. +- **Cassettes** live in `integrations//cassettes//`; `integrations/conftest.py` resolves the version dir — never add per-test `vcr_cassette_dir` fixtures. Re-record only when behavior intentionally changed. Sanitize binary media in request and response bodies. +- **Consolidate before handoff (required, do not wait to be asked).** For every test file you touched, merge tests that differ only in inputs into one `@pytest.mark.parametrize` test, and delete tests whose assertions another test already makes (for example a sync test that a parametrized sync/async test covers). Keep the red → green regression test. Then run `make check-unused-cassettes INTEGRATIONS=""` and delete cassettes no test reads. In the handoff, list the tests merged or removed and the test count before and after. ## Commands ```bash -cd py && nox -s "test_(latest)" +cd py && nox -s "test_(latest)" # confirm the session name in noxfile.py cd py && nox -s "test_(latest)" -- -k "test_name" cd py && nox -s "test_(latest)" -- --vcr-record=all -k "test_name" -cd py && make test-core -cd py && make lint +cd py && make test-core # shared integration code changed +cd py && make lint # shared files or repo wiring changed ``` -## Validation Checklist - -- Narrowest provider session first. -- Subprocess auto-instrument test if patchers / setup / import timing / `auto.py` changed. -- `make test-core` if shared integration code changed. -- `make lint` before handoff if shared files or repo wiring changed. +Also run the subprocess auto-instrument test if patchers, setup, import timing, or `auto.py` changed. ## Common Mistakes -- Treating a wrapper migration as fresh integration work (use `sdk-wrapper-migrations`). - Changing shared primitives when provider-local code should own the behavior. -- Combining unrelated patch targets into one patcher. -- Forgetting repo wiring for new providers (`integrations/__init__.py`, `noxfile.py`, sometimes `auto.py`). +- Combining unrelated patch targets into one patcher, or patching internal helpers when a public surface works. +- Forgetting repo wiring for new providers (`integrations/__init__.py`, `noxfile.py`, `cassette-dirs`, sometimes `auto.py`). - Forgetting subprocess auto-instrument tests, or async/streaming coverage. -- Re-recording cassettes when behavior didn't intentionally change. -- Custom `_instrument_*` helper where `_instrument_integration()` fits. -- Missing `target_module` for deep/optional patch targets. -- Inventing new top-level span fields / metric keys / span types (metadata is looser — see allowlist-per-provider). - Tool definitions in `input` instead of `metadata.tools`; prompt provenance outside `metadata.prompt`. -- Missing `internal={"instrumentation": "-auto"}` on integration spans, or leaking it onto user spans nested inside. -- Framework name in `metadata.provider` for a completion-style framework wrapper. -- Per-chunk spans for streamed calls (should be one accumulated span per API call). -- Instrumentation errors escaping into the user's call path, or swallowing real provider errors. -- Double-counting tokens across orchestration + provider spans, or provider-specific ownership checks instead of clear rules. -- Over-serializing (JSON dumps/loads, recursive conversion) in tracing code. -- `try`/`except` around Braintrust span-logging methods — they're boundary-safe. -- Forcing non-image attachments through `image_url` shims; dropping unrecognized file inputs; re-serializing non-attachment values. -- Patching internal helpers when a stable public API surface would work. +- Framework name in `metadata.provider`; per-chunk spans for streamed calls; double-counting tokens across framework + provider spans. +- Missing `internal={"instrumentation": "-auto"}` on integration spans, or leaking it onto nested user spans. +- Handing off with redundant or overlapping tests. diff --git a/.agents/skills/sdk-integrations/references/span-design.md b/.agents/skills/sdk-integrations/references/span-design.md new file mode 100644 index 000000000..2499ed3ea --- /dev/null +++ b/.agents/skills/sdk-integrations/references/span-design.md @@ -0,0 +1,88 @@ +# Span Design, Metrics, And Attachments + +Repo-specific notes on top of the instrumentation spec (`braintrustdata/braintrust-spec` `skills/instrumentation-spec/references/instrumentation-guide.md`). The spec wins on any conflict. + +## Span Design + +### Type + name + +- `llm` — one provider API call +- `task` — parent for an agent run, pipeline step, or named operation +- `tool` — model-initiated tool/function execution + +Use spec-recommended names (`Chat Completion`, `anthropic.messages.create`, `generate_content`). For new providers, pick a stable provider-specific name. + +### Completion-style vs agentic-style + +- **Completion** (single request/response, user runs any tool calls): one `llm` span, no children. Tool calls appear in `output`. +- **Agentic** (SDK/framework runs the tool loop): one parent `task` wrapping child `llm` (per model call) + `tool` (per tool exec) spans in execution order. + +Completion-style *frameworks* (LiteLLM, OpenRouter, etc.) still emit `llm` spans with `metadata.provider` set to the underlying provider, NOT the framework name. Agentic frameworks (Vercel AI SDK w/ tools, LangChain agents, OpenAI Agents SDK, Claude Agent SDK, ADK, Agno) emit the full parent+children tree. + +### Payload format + +Canonical shape is OpenAI Chat Completions format. Providers with dedicated UI normalizers (OpenAI, Anthropic, Google) MAY preserve provider-native payloads if `metadata.provider` is set correctly; all other providers MUST convert to OpenAI shape. If a provider passes system prompt separately, insert it as a `role: "system"` entry. + +### Fields + +- `input` — meaningful user request (messages / prompt / provider-native input) +- `output` — meaningful provider result (normalized, not opaque SDK instances) +- `metadata` — see below +- `metrics` — spec-listed keys only (see [Metrics](#metrics)) +- `error` — pass the `Exception` instance directly to span logging; do not pre-format + +**Every `llm` span MUST include `metadata.model` and `metadata.provider`** (`provider` = whose pricing applies, even when going through a gateway or framework). + +**Tool definitions go in `metadata.tools`** (OpenAI-shaped regardless of underlying provider), NOT in `input` messages. Preserve provider-native built-in tool types (Anthropic `computer_use`, OpenAI `web_search`) as opaque JSON-serializable entries. Never log executable tool handlers. + +**Prompt provenance goes in `metadata.prompt`** (`id`, `project_id`, `version`, `variables`, plus `prompt_session_id` for playground calls). Not in `input`, request payloads, or `metadata.tools`. Strip carrier fields like `span_info` before logging. + +**`metadata` allowlist-per-provider:** capture the spec-defined keys plus provider-specific detail fields (request/response ids, safety data, model-family flags) chosen deliberately. Do not dump entire raw request/response objects. Redact secrets. If a provider-specific field ends up broadly useful, promote it to the spec. + +### Token metrics + +The integration that directly owns the model/provider API response owns token accounting. Orchestration/framework integrations should not log token metrics when underlying provider integrations create leaf spans with usage. Agentic parent `task` spans MAY aggregate across children when the framework doesn't delegate to a separately instrumented provider client. Do not add "if OpenAI is patched, skip metrics" checks — define clear ownership instead. + +**Framework `llm`-like spans that delegate to a separately instrumented client:** if the framework's per-model-call span sits above a call the framework itself dispatches through another patched client (e.g. DSPy's LM callback → LiteLLM → OpenAI), do NOT type the framework's span `llm`. Two nested `llm` spans for one API call is confusing in the UI and — because the framework typically cannot supply tokens from its own callback contract — would produce a token-less `llm` span that violates the "tokens required for LLM spans" rule. Leave the framework span untyped (or `task`), keep `metadata.model` / `metadata.provider` on it for attribution, and let the underlying provider integration own the `llm` leaf with usage. The same logic applies to any framework that always delegates the transport layer to another instrumentable client. + +### Shaping guidance + +- Flatten positional args into named fields; normalize SDK objects into dicts/lists/scalars; drop noisy transport fields. +- Aggregate streaming chunks into one final `output` + stream-specific `metrics`. One `llm` span per API call, not per chunk. +- Do not over-serialize. Braintrust serializes at send/log time. Integration tracing only needs readable Python dicts/lists/scalars and materialized attachments. +- Keep wrapper bodies thin: prepare traced input, open span, call provider, normalize result, log. +- Do not wrap `span.log(...)` / `span.set_attributes(...)` in broad try/except. Braintrust span methods are boundary-safe. +- **Rerank exception to "no silent caps":** rerank result lists can be huge and are noisy in the span UI; cap the `output` list at a fixed max (e.g. 100 entries) via a named constant. The cap is compact-span hygiene, not coverage bounding — the model already ranked everything. + + +## Metrics + +Only emit spec-listed keys: +- `tokens`, `prompt_tokens`, `completion_tokens` — required for LLM spans; values MUST be non-negative +- `time_to_first_token` — required for streaming; SDK measures from request start to first chunk +- `completion_reasoning_tokens` — required when provider reports reasoning tokens (o-series etc.) +- `prompt_cached_tokens`, `prompt_cache_creation_tokens`, `prompt_cache_creation_5m_tokens`, `prompt_cache_creation_1h_tokens` — provider prompt caching +- `prompt_audio_tokens`, `completion_audio_tokens`, `completion_image_tokens` — audio/image models when provider reports +- `start`, `end` — standard span timing + +Map provider modality breakdowns only to standardized metrics: input audio → `prompt_audio_tokens`, output audio → `completion_audio_tokens`, and output image → `completion_image_tokens`. These are subsets of prompt/completion usage, not additional tokens. Preserve unstandardized modality breakdowns as deliberately named provider metadata rather than inventing metric keys. + +For streaming, still produce one span per API call with accumulated `input`/`output`. Capture usage from stream metadata (e.g. OpenAI `stream_options.include_usage`) when surfaced. + +For reasoning models, capture the full output structure (reasoning summaries + message blocks) and include prior reasoning in `input` for multi-turn calls. + +## Multimodal And Attachments + +Inline binary media (images, PDFs, audio, video) MUST be replaced by a Braintrust attachment at the leaf position. Do not add a separate top-level `attachments` list. + +- Prefer `_materialize_attachment(...)` in `integrations/utils.py`. Don't reimplement base64/file decoding. +- Convert raw `bytes` / base64 / data URLs / file inputs / generated media → `braintrust.logger.Attachment`. Preserve remote URLs as strings — do not fetch to create an attachment. +- Payload shape after materialization: + - images → `{"image_url": {"url": attachment}}` + - non-image media/documents/files → `{"file": {"file_data": attachment, "filename": resolved.filename}}` +- Do not force non-image payloads through `image_url`. +- If materialization fails, keep the original value; never drop, null it out, or raise. +- Preserve non-attachment values while walking nested payloads unless intentionally normalizing. +- Keep MIME type, size, safety data, filenames, provider ids next to the attachment. +- Providers with UI normalizers (OpenAI, Anthropic, Google) — MAY preserve the provider-native structure and only replace the raw media leaf. +- Generated media → log the attachment in `output`, not `metadata`. For streaming, aggregate into one final `output` attachment. diff --git a/.agents/skills/sdk-integrations/references/span-origin.md b/.agents/skills/sdk-integrations/references/span-origin.md new file mode 100644 index 000000000..1ddee449b --- /dev/null +++ b/.agents/skills/sdk-integrations/references/span-origin.md @@ -0,0 +1,24 @@ +# Span Origin + +Every span an integration creates MUST carry `context.span_origin.instrumentation.name = "-auto"` (e.g. `openai-auto`, `anthropic-auto`, `adk-auto`, `google-genai-auto`). Match the JS SDK exactly for cross-SDK filtering. + +`start_span(...)` and all provider-level `start_span` methods accept `internal: SpanInternalOptions | None`, a TypedDict reserved for SDK internals — external callers should not use it. Integrations pass `internal={"instrumentation": "-auto"}`. When unset, spans fall back to the channel default (`braintrust-python-logger` / `braintrust-python-otel`). + +The name does NOT propagate through the parent/child edge — each `start_span` call decides independently. User-owned spans nested inside a wrapped call (e.g. a `@traced` scorer) must NOT inherit the integration's name. + +Standard pattern at the top of `tracing.py` (or `callbacks.py` / `plugin.py`): + +```python +from braintrust.logger import start_span as _bt_start_span + +_INSTRUMENTATION = "-auto" + + +def start_span(*args, **kwargs): + internal = dict(kwargs.get("internal") or {}) + internal.setdefault("instrumentation", _INSTRUMENTATION) + kwargs["internal"] = internal + return _bt_start_span(*args, **kwargs) +``` + +Module-level `start_span(...)` calls flow through automatically. Calls that go through a `Logger` / `Experiment` / parent-`Span` instance (`logger.start_span(...)`, `parent.start_span(...)`) need `internal={"instrumentation": _INSTRUMENTATION}` explicitly. Tests assert the exported provenance: `span["context"]["span_origin"]["instrumentation"]["name"] == "-auto"`. diff --git a/.agents/skills/sdk-integrations/references/streaming-lifecycle.md b/.agents/skills/sdk-integrations/references/streaming-lifecycle.md new file mode 100644 index 000000000..b9b31e38f --- /dev/null +++ b/.agents/skills/sdk-integrations/references/streaming-lifecycle.md @@ -0,0 +1,23 @@ +# Streaming Lifecycle Review + +Open this when an integration wraps a streamed response (sync or async iterator, context manager, or provider stream object). + +Review every exit path, not just full consumption. Check the real provider's +iterator, context-manager, cancellation, and garbage-collection behavior before +choosing a wrapper. A proxy can preserve transport cleanup while still losing +the span's final output and end time. + +- Cover exhaustion, provider errors, explicit close/cancel, and context-manager + exit where the provider supports it. Preserve exception and return semantics. +- Cover `break` followed by dropping the last stream reference, and dropping a + stream before consuming any chunks. `break` alone does not close a retained + iterator; do not promise immediate finalization while callers still hold it. +- Use existing recordings for partial-consumption tests. Drop the proxy, force + collection, and assert partial (or empty) output, `metrics.end`, correct + parentage, and no duplicate finalization after explicit close or exhaustion. +- A GC fallback must not retain the stream through its callback or closure. + For async streams, do not run or schedule event-loop work from a finalizer; + finalize trace state and preserve the provider's own cleanup behavior. +- Check that neither iteration nor cleanup leaves the stream span current in + the caller's context. GC is a best-effort fallback, not a substitute for + deterministic cleanup when the caller explicitly closes the stream. diff --git a/.agents/skills/sdk-vcr-workflows/SKILL.md b/.agents/skills/sdk-vcr-workflows/SKILL.md index 4e48d5af5..0d1d22d32 100644 --- a/.agents/skills/sdk-vcr-workflows/SKILL.md +++ b/.agents/skills/sdk-vcr-workflows/SKILL.md @@ -5,399 +5,80 @@ description: Work with Braintrust Python SDK VCR and cassette-backed tests. Use # SDK VCR Workflows -Use this skill for VCR and cassette work in the Braintrust Python SDK repo. - -This repo prefers real recorded integration coverage over mocks for provider behavior. Do not treat cassette work as an afterthought. The normal bug-fix path here is: reproduce with a failing cassette-backed test, implement the fix, and only re-record when the behavior change is intentional. +This repo prefers real recorded provider coverage over mocks. The normal bug-fix path: reproduce with a failing cassette-backed test, implement the fix, and re-record only when the behavior change is intentional. `AGENTS.md` rules (work from `py/`, `mise`, red → green, no guessed session names) apply throughout. ## Read First -Always read: - -- `AGENTS.md` -- `py/pyproject.toml` (`[tool.braintrust.matrix]` for provider version pins) -- `py/noxfile.py` -- `py/src/braintrust/conftest.py` -- `py/src/braintrust/integrations/conftest.py` (shared version-aware cassette directory resolution) -- the target provider test file under `py/src/braintrust/integrations//` or `py/src/braintrust/wrappers/` - -Read when relevant: - -- the provider integration package under `py/src/braintrust/integrations//` -- the provider cassette directory under `py/src/braintrust/integrations//cassettes//` -- `py/src/braintrust/cassettes/` -- `py/src/braintrust/wrappers/cassettes/` -- `py/src/braintrust/devserver/cassettes/` -- `py/src/braintrust/wrappers/claude_agent_sdk/` for subprocess transport recordings instead of HTTP VCR -- `py/src/braintrust/integrations/auto_test_scripts/` when auto-instrument subprocess coverage and cassettes interact - -Do not guess cassette locations or session names. Check the current tree and `py/noxfile.py` first. - -## Repo Rules To Follow - -These rules must stay aligned with `AGENTS.md`: - -- Work from `py/` for SDK tasks. -- Use `mise` as the source of truth for tools and environment. -- Do not guess nox session names or provider/version coverage. Provider version pins (including what "latest" resolves to) are in `py/pyproject.toml` `[tool.braintrust.matrix]`. -- Default bug-fix workflow is red -> green. -- Prefer VCR-backed provider tests over mocks or fakes whenever practical. -- Treat mock/fake tests for provider behavior as an exception that requires justification, not as a neutral alternative. -- Only re-record HTTP or subprocess cassettes when the behavior change is intentional. If unsure, ask the user. -- Do not assume optional provider packages are installed outside the active nox session. - -## How VCR Works In This Repo - -Behavior from `py/src/braintrust/conftest.py` is the source of truth. - -Current defaults: - -- local default: `record_mode="once"` -- CI default: `record_mode="none"` -- fixtures inject dummy API keys and reset global state - -### Per-version cassette directories +- `py/pyproject.toml` `[tool.braintrust.matrix]` (provider version pins, what `latest` resolves to) and `py/noxfile.py` (session names) +- `py/src/braintrust/conftest.py` (record modes, dummy keys, media sanitizers) and `py/src/braintrust/integrations/conftest.py` (version-aware cassette dirs) +- the target test file and its `cassettes//` directory -Integration cassettes are stored in version-specific subdirectories: +Open these references when the task needs them: +- [references/recording-failures.md](references/recording-failures.md) — a live recording failed (auth, quota, retired model, partial run), or a cassette-backed test fails and you need the debugging order. +- [references/discovery-engine.md](references/discovery-engine.md) — recording `google_discoveryengine` (REST VCR + gRPC helper, gcloud ADC setup). -- `py/src/braintrust/integrations//cassettes//` (e.g. `cassettes/latest/`, `cassettes/1.71.0/`) +## How VCR Works Here -The mechanism: - -- Nox sessions pass the version under test via the `BRAINTRUST_TEST_PACKAGE_VERSION` env var. -- The shared `py/src/braintrust/integrations/conftest.py` provides a `vcr_cassette_dir` fixture that resolves `/cassettes//`. -- When the env var is absent (e.g. running a test file directly outside nox), cassettes fall back to the base `cassettes/` directory. -- Individual test files do not define their own `vcr_cassette_dir` or `cassette_library_dir` fixtures; the shared conftest handles it. -- Claude Agent SDK `_test_transport.py` also reads `BRAINTRUST_TEST_PACKAGE_VERSION` for version-specific cassette resolution. - -Implications: - -- A test that passes locally by silently recording new traffic may still fail in CI if the cassette is missing or stale. -- CI will not save you by recording fresh traffic. If the cassette is wrong, CI should fail. -- Reproducing a CI VCR failure locally usually means running the exact nox session named in `py/noxfile.py`, not raw pytest in whatever environment happens to exist. -- When re-recording, always use a nox session so cassettes land in the correct version subdirectory. +- Record mode: `once` locally, `none` in CI. A local pass that silently recorded new traffic can still fail in CI. +- Cassettes are per version: `integrations//cassettes//` (e.g. `latest/`, `1.71.0/`). Nox sets `BRAINTRUST_TEST_PACKAGE_VERSION`; `integrations/conftest.py` resolves the directory. Without the env var (raw pytest), cassettes fall back to base `cassettes/` — so always record through nox. +- Never add per-test `vcr_cassette_dir` / `cassette_library_dir` fixtures. +- Other locations: `py/src/braintrust/cassettes/`, `py/src/braintrust/wrappers/cassettes/`, `py/src/braintrust/devserver/cassettes/`. Keep cassettes next to their tests and move them with the tests. ## Standard Workflow -1. Identify the exact provider test or cassette-backed failure. -2. Read `py/noxfile.py` and reproduce with the exact provider session and version. -3. If fixing a bug, add or update a focused failing cassette-backed test first. -4. Decide whether the intended behavior should reuse an existing cassette or requires a new or updated one. -5. Re-record only the narrowest affected test when behavior intentionally changes. -6. Re-run the exact provider session after recording. -7. Expand only if shared integration code changed. - -Do not re-record a broad provider suite when one focused test is enough. - -Do not default to mocks/fakes just to avoid recording work. Extra setup effort is not, by itself, a good reason to abandon cassette-backed coverage. - -## Choosing The Right Coverage Style - -### Prefer cassette-backed tests when: - -- validating real provider request or response shape -- testing tracing/span contents derived from actual provider payloads -- checking streaming aggregation against real wire behavior -- confirming version-specific provider behavior that mocks could misrepresent -- replacing older fake- or mock-heavy provider tests - -### Use mocks/fakes only when: - -- injecting a narrow error path that is hard to provoke via recordings -- testing purely local patcher resolution or applicability logic -- validating behavior that does not need real provider traffic -- the code under test is entirely internal and a cassette would add little value - -If the task is about provider behavior and you can reasonably record it, prefer VCR. - -Do not use mocks/fakes as the primary regression test for provider behavior merely because: -- the bug appears in local tracing or post-processing code -- a mock is faster to write -- recording requires adding a new cassette -- an existing mock test already exists nearby - -In those situations, the right move is usually to add or update a focused cassette-backed test and keep any mock/unit test only as supplemental coverage. +1. Identify the exact failing test, then reproduce with the exact nox session and version from `py/noxfile.py`. +2. For a bug, add or update a focused failing cassette-backed test first. +3. Decide whether the test can reuse an existing cassette or needs a new one. +4. Record only the narrowest affected test, then inspect the cassette diff. +5. Re-run the provider session in replay (`--vcr-record=none`). Expand only if shared code changed. -## Cassette Locations +## Choosing Coverage -Common locations in this repo: +Use a cassette whenever the provider's request or response shape is part of the behavior: tracing/span contents derived from real payloads, streaming aggregation, version-specific provider behavior, or replacing older mock-heavy tests. -- `py/src/braintrust/cassettes/` -- `py/src/braintrust/wrappers/cassettes/` -- `py/src/braintrust/devserver/cassettes/` -- `py/src/braintrust/integrations//cassettes//` for per-version integration cassettes -- `py/src/braintrust/integrations/claude_agent_sdk/cassettes//` for Claude Agent SDK subprocess transport recordings - -Each version tested in a nox session gets its own cassette subdirectory (e.g. `cassettes/latest/`, `cassettes/0.48.0/`). The `latest` subdirectory holds cassettes for the unpinned latest version. - -Keep cassettes next to the tests they support. When migrating or moving tests, move the cassettes with them. +Mocks or fakes only for: a narrow error path recordings cannot provoke, purely local patcher resolution or version routing, or internal code where traffic adds nothing. Hand-built stand-ins for provider objects (stub clients, fake tools, response dicts) count as fakes. "The bug is in local post-processing", "a mock is faster", or "a mock already exists nearby" are not reasons. If you choose a mock for provider behavior, state why a cassette is impractical. ## Commands -Run from `py/` unless the task is clearly repo-level. - -Common provider VCR commands: - -```bash -cd py -nox -s "test_openai(latest)" -nox -s "test_openai(latest)" -- -k "test_openai_chat_metrics" -nox -s "test_openai(latest)" -- --disable-vcr -nox -s "test_openai(latest)" -- --vcr-record=all -k "test_openai_chat_metrics" -``` - -Generic reproduction shape: - ```bash cd py nox -s "test_()" nox -s "test_()" -- -k "test_name" -nox -s "test_()" -- --vcr-record=all -k "test_name" -``` +nox -s "test_()" -- --vcr-record=all -k "test_name" # narrow re-record +nox -s "test_()" -- --vcr-record=none # replay-only check +nox -s "test_()" -- --disable-vcr -Claude Agent SDK transport-recording commands: - -```bash -cd py -nox -s "test_claude_agent_sdk(latest)" -BRAINTRUST_CLAUDE_AGENT_SDK_RECORD_MODE=all nox -s "test_claude_agent_sdk(latest)" -BRAINTRUST_CLAUDE_AGENT_SDK_RECORD_MODE=all nox -s "test_claude_agent_sdk(latest)" -- -k "test_calculator_with_multiple_operations" +# Claude Agent SDK: subprocess transport recordings, not HTTP VCR +BRAINTRUST_CLAUDE_AGENT_SDK_RECORD_MODE=all nox -s "test_claude_agent_sdk(latest)" -- -k "test_name" ``` -## Re-recording Rules - -Re-record only when the behavior change is intentional. - -Good reasons to re-record: - -- the traced request or response shape intentionally changed -- the provider behavior you are validating intentionally changed -- the old cassette covered the wrong scenario and the test now targets the correct one -- a new focused cassette-backed regression test is being added - -Bad reasons to re-record: - -- masking an unexplained failure -- making a flaky test pass without understanding the difference -- updating many cassettes when only one focused scenario changed -- papering over version mismatches or incorrect assumptions about old provider releases - -If a cassette diff is large, verify that the behavioral change is truly intended before keeping it. - -## When Recording Fails - -Live recording fails for reasons unrelated to the SDK change. Handle each case so a failed recording does not destroy a good cassette or leave a bad one behind. - -- **Missing or invalid API key (401/403, auth errors):** stop and ask the user for a working key. Never hand-write or edit a cassette to stand in for a recording. -- **Quota or rate limits (429, 402 "no remaining credits"):** a cassette that captured an error response is worse than the old one. Delete only the cassettes that captured the error, wait or ask for another key, then refill them with the default `once` mode. -- **Retired models or upstream drift:** if the live API no longer returns what the test asserts (a retired model returns 404, a moderation block, a search result that no longer appears, a different stop reason), keep the old cassette with `git checkout` and tell the user. Do not loosen the assertion just to get a recording. -- **Hand-authored cassettes:** some cassettes are hand-edited to force a scenario such as a validation retry. Never re-record them; restore them with `git checkout`. - -### Restore from git vs. re-record - -- Restore from git when the cassette is hand-authored, upstream behavior drifted, or the provider is unusable locally (no key, no quota). The old cassette still matches the request the SDK sends. -- Re-record when the SDK's request shape intentionally changed, or when the new pin changes the wire format and playback fails. -- If some tests in a directory may fail to record, record without deleting the directory first, so failed tests keep their old cassettes. If a test fails partway through, `git checkout` its cassette, because a partial refresh can leave truncated files. -- After any refresh, run `git status` on the cassette directory and look for ` D` entries (cassettes deleted but never recreated). +Claude Agent SDK talks to the bundled `claude` subprocess over stdin/stdout; `integrations/claude_agent_sdk/_test_transport.py` records and resolves per-version cassettes. Do not force HTTP VCR patterns onto it. -### Traps that silently lose cassettes - -- **Skipped sessions record nothing.** If a session skips on your Python version (see `_skip_if_incompatible` and the constraints in `py/noxfile.py`), deleting its cassettes first destroys them. Run nox from a compatible interpreter instead. -- **Multi-file sessions stop at the first failing file.** After a partial failure, re-run in `once` mode to fill the files that later test files never recorded. -- **Shared cassette names under `--vcr-record=all`.** When several parametrizations share a cassette name, later cases read an earlier case's response. Delete only those cassettes and re-run in `once` mode, so the first case of each name records and the rest replay. -- **Hangs.** Tests that drive long-lived live sessions can hang while recording. Exclude them with `-k "not "` and keep their old cassettes. - -## Narrow Recording Strategy - -When you need new traffic, record the smallest thing possible: - -1. use the exact session from `py/noxfile.py` -2. target a single test with `-k` when practical -3. record only the affected scenario -4. inspect the cassette diff before moving on - -Preferred shape: - -```bash -cd py -nox -s "test_google_genai(latest)" -- --vcr-record=all -k "test_interactions_create_and_get" -``` - -Avoid broad commands like re-recording an entire provider suite unless the change genuinely affects the whole suite. - -## Debugging VCR Failures - -When a cassette-backed test fails, check these in order: - -1. **Wrong nox session or provider version** - - Did you reproduce under the exact session from `py/noxfile.py`? - - Are you accidentally testing `latest` when CI pins an older version? - -2. **Cassette missing or stale** - - Does the expected cassette file exist in the right directory? - - Did the test move without moving the cassette? - - Does the cassette still match the intended request shape? - -3. **Behavior changed but cassette did not** - - Did the code intentionally change the payload, headers, streaming sequence, or traced output? - - If yes, re-record narrowly. +## Re-recording Rules -4. **Behavior did not change, but the test became too strict** - - Prefer fixing brittle assertions before churning cassettes. - - Assert the meaningful span structure, not incidental provider noise. +Re-record only when behavior intentionally changed: the traced request/response shape changed, the provider behavior under test changed, the old cassette covered the wrong scenario, or you are adding a new focused regression. If unsure, ask the user. -5. **Local-only pass, CI fail** - - Remember CI uses `record_mode="none"`. - - A local silent record can hide a missing cassette or wrong request shape. +Never re-record to mask an unexplained failure, fix a flake you don't understand, paper over a version mismatch, or as a broad suite refresh when one scenario changed. A large cassette diff needs an explanation. -6. **Optional dependency or auth confusion** - - Do not rely on globally installed provider packages. - - Use the active nox session. - - Read `py/src/braintrust/conftest.py` before assuming credential behavior. +When a live recording fails: on auth errors stop and ask for a key; never hand-write a cassette; delete cassettes that captured 429/402 error responses; restore hand-authored or drifted cassettes with `git checkout`. Then check `git status` for ` D` entries. Details and traps: [references/recording-failures.md](references/recording-failures.md). ## Cassette Hygiene -Keep cassettes reviewable and intentional. - -Prefer: - -- one focused cassette per scenario or regression when practical -- reusing existing cassette patterns in the same provider package -- keeping fixture names stable and descriptive -- moving cassettes with tests when code moves from wrappers to integrations - -Be careful about: - -- duplicate old cassettes left behind after moves or renames -- storing unnecessary raw binary content when sanitization is possible -- overfitting tests to incidental details in recordings -- broad cassette churn from unfocused recording commands - -When the provider returns binary HTTP responses or generated media, sanitize the recordings as needed so the repo does not keep unnecessary raw file bytes. - -For image tests that do not depend on the actual pixels or original byte size, replace large generated image payloads with a tiny valid image while preserving the provider response structure and usage metadata. Scope the recording hook to the relevant tests, retain sensitive-header scrubbing, and update content-length headers when changing bodies. See `sanitize_generated_images` in `py/src/braintrust/conftest.py` and the OpenAI and LiteLLM image tests for an example; Google GenAI has its own media sanitizer. Apply the same transformation to existing recordings where appropriate, and verify replay across the affected provider versions. Keep original media where its contents, format, dimensions, or size are part of the behavior under test. - -## Claude Agent SDK Exception - -Claude Agent SDK coverage is cassette-backed, but not through HTTP VCR. - -Important differences: +- One focused cassette per scenario; reuse existing patterns in the provider package. +- After deleting or renaming tests, run `make check-unused-cassettes INTEGRATIONS=""` and remove orphans. After dropping matrix versions, run `make check-stale-cassettes`. +- Sanitize binary bodies. For image tests that don't depend on pixels or byte size, replace generated images with a tiny valid image, keeping response structure, usage metadata, header scrubbing, and correct content-length (see `sanitize_generated_images` in `py/src/braintrust/conftest.py`). Keep original media when its contents, format, or size is under test. +- Prefer fixing brittle assertions over cassette churn. Assert meaningful span structure, not incidental provider noise. -- it talks to the bundled `claude` subprocess over stdin/stdout -- it uses transport-level cassette helpers instead of HTTP request recording -- cassettes are stored per-version under `integrations/claude_agent_sdk/cassettes//` -- `_test_transport.py` reads `BRAINTRUST_TEST_PACKAGE_VERSION` (set by nox) to resolve the version subdirectory -- use `BRAINTRUST_CLAUDE_AGENT_SDK_RECORD_MODE=all` when re-recording +## Related Skills -Do not try to force ordinary HTTP VCR patterns onto Claude Agent SDK subprocess tests. - -## Discovery Engine Recording - -Discovery Engine tests use HTTP VCR for sync REST calls and the test-only -`integrations/google_discoveryengine/_test_grpc.py` helper for async gRPC calls. Both -recording formats live under `py/src/braintrust/integrations/google_discoveryengine/cassettes//`. - -### Prerequisites - -- Install `gcloud` and use a Google Cloud project with billing and the - `discoveryengine.googleapis.com` API enabled. -- Authenticate with Application Default Credentials (ADC). The account needs - permission to invoke Discovery Engine in the target project. A `GEMINI_API_KEY` - alone does not authenticate these tests. -- For answer and conversation tests, create a search app with generative responses - enabled and attach a populated datastore. Wait for indexing to finish before - recording. Ranking and grounding-check tests do not require indexed documents. - -```sh -gcloud config set project YOUR_PROJECT_ID -gcloud auth application-default login -gcloud auth application-default set-quota-project YOUR_PROJECT_ID -``` - -The recording fixture obtains an access token using -`gcloud auth application-default print-access-token`, outside the recorded call. -Playback uses anonymous credentials and needs no Google account. - -Live recording requires explicit resource IDs through environment variables. -There are no private-project defaults in the test code. Tests use the `global` -location. Ranking and grounding checks require the project; answer and -conversation coverage also requires the app and datastore. - -A datastore can be populated from Google's public Alphabet earnings-report -sample PDFs at `gs://cloud-samples-data/gen-app-builder/search/alphabet-investor-pdfs`. -Wait for indexing to finish before recording. - -```sh -export BRAINTRUST_GOOGLE_DISCOVERYENGINE_PROJECT="your-project" -export BRAINTRUST_GOOGLE_DISCOVERYENGINE_APP="your-app-id" -export BRAINTRUST_GOOGLE_DISCOVERYENGINE_DATASTORE="your-datastore-id" -``` - -### Record and replay - -From `py/`, select a focused scenario. `--vcr-record=all` enables recording for -both REST and gRPC; the gRPC helper otherwise requires an existing cassette. - -```sh -# REST ranking, including manual/setup entry-point coverage. -mise exec -- nox -s 'test_google_discoveryengine(latest)' -- --vcr-record=all -k 'test_rank and not test_rank_output_limit' - -# Async gRPC ranking. -mise exec -- nox -s 'test_google_discoveryengine(latest)' -- --vcr-record=all -k 'test_async_grpc and rank' - -# Verify all recordings without network access to Google. -mise exec -- nox -R -s 'test_google_discoveryengine(latest)' -- --vcr-record=none -``` - -Playback derives resource paths from the checked-in REST cassettes and ignores -resource environment overrides. No access to the recorded project's resources is -needed. The auto-instrument subprocess test also reads its ranking resource from -the cassette. Keep the project's recordings consistent when recording against a -different project. - -The gRPC helper temporarily replaces one callable on one client transport, -records real protobuf requests/responses and errors as JSON, then restores the -callable. Playback reconstructs provider response objects and checks requests; -it does not exercise networking, retries, or real cancellation behavior. -HTTP authorization headers are filtered by shared VCR configuration; gRPC -recordings omit credentials and transport metadata. Inspect new recordings before -checking them in. - -`generate_grounded_content` and `stream_generate_grounded_content` are deferred: -the v1 endpoints returned method-not-found errors during initial recording. -Do not fabricate success recordings for those methods. - -## Relationship To Other Skills - -- Use `sdk-integrations` when the main task is integration implementation, patchers, tracing, or provider package structure. -- Use this skill when the main difficulty is cassette-backed test design, VCR behavior, re-recording decisions, or cassette hygiene. -- Use `sdk-ci-triage` when the entry point is a GitHub Actions failure, even if the fix eventually involves VCR. -- Use `sdk-wrapper-migrations` when moving a legacy wrapper into `py/src/braintrust/integrations/` while preserving tests and cassettes. +- `sdk-integrations` — integration implementation, patchers, tracing, span shape. +- `sdk-ci-triage` — the entry point is a GitHub Actions failure, even if the fix involves VCR. +- `sdk-dependency-updates` — re-recording `latest` cassettes after a provider version bump. ## Validation Checklist -- Read `py/noxfile.py` before choosing commands. -- Reproduce under the exact provider session and version. -- Use red -> green when fixing a bug. -- Prefer a focused cassette-backed test over mocks/fakes. -- If you choose a mock/fake test for provider behavior, be able to state exactly why a cassette-backed test is impractical. -- Re-record only when behavior intentionally changed. -- Record the narrowest affected test first. -- Inspect the cassette diff before finishing. -- Re-run the relevant provider session after recording. -- If patching or `auto_instrument()` changed, also check the relevant subprocess auto-instrument coverage. - -## Common Mistakes - -Avoid these failures: - -- running raw pytest in an ad hoc environment instead of the exact nox session -- re-recording against `latest` when CI covers an older pinned version -- letting local `record_mode="once"` hide a missing or stale cassette -- replacing meaningful assertions with cassette churn -- using mocks for provider behavior that should be validated from real recordings -- treating local tracing/span-shaping bugs as mock-first when the trigger is a real provider payload -- forgetting that Claude Agent SDK uses subprocess transport recordings, not HTTP VCR -- leaving duplicate stale cassettes behind after moving tests or renaming scenarios -- broad re-records that create unnecessary review noise -- running raw pytest outside nox when re-recording, causing cassettes to land in the base `cassettes/` directory instead of the correct version subdirectory -- adding per-test `vcr_cassette_dir` or `cassette_library_dir` fixtures — the shared `integrations/conftest.py` handles version routing +- Exact provider session and version reproduced; red → green for bugs. +- Cassette-backed test over mocks/fakes, or a stated reason why not. +- Narrowest recording; cassette diff inspected; no ` D` entries left behind. +- Provider session re-run in replay mode after recording. +- Subprocess auto-instrument coverage checked if patching or `auto_instrument()` changed. diff --git a/.agents/skills/sdk-vcr-workflows/references/discovery-engine.md b/.agents/skills/sdk-vcr-workflows/references/discovery-engine.md new file mode 100644 index 000000000..9b2086800 --- /dev/null +++ b/.agents/skills/sdk-vcr-workflows/references/discovery-engine.md @@ -0,0 +1,75 @@ +# Discovery Engine Recording + +Discovery Engine tests use HTTP VCR for sync REST calls and the test-only +`integrations/google_discoveryengine/_test_grpc.py` helper for async gRPC calls. Both +recording formats live under `py/src/braintrust/integrations/google_discoveryengine/cassettes//`. + +### Prerequisites + +- Install `gcloud` and use a Google Cloud project with billing and the + `discoveryengine.googleapis.com` API enabled. +- Authenticate with Application Default Credentials (ADC). The account needs + permission to invoke Discovery Engine in the target project. A `GEMINI_API_KEY` + alone does not authenticate these tests. +- For answer and conversation tests, create a search app with generative responses + enabled and attach a populated datastore. Wait for indexing to finish before + recording. Ranking and grounding-check tests do not require indexed documents. + +```sh +gcloud config set project YOUR_PROJECT_ID +gcloud auth application-default login +gcloud auth application-default set-quota-project YOUR_PROJECT_ID +``` + +The recording fixture obtains an access token using +`gcloud auth application-default print-access-token`, outside the recorded call. +Playback uses anonymous credentials and needs no Google account. + +Live recording requires explicit resource IDs through environment variables. +There are no private-project defaults in the test code. Tests use the `global` +location. Ranking and grounding checks require the project; answer and +conversation coverage also requires the app and datastore. + +A datastore can be populated from Google's public Alphabet earnings-report +sample PDFs at `gs://cloud-samples-data/gen-app-builder/search/alphabet-investor-pdfs`. +Wait for indexing to finish before recording. + +```sh +export BRAINTRUST_GOOGLE_DISCOVERYENGINE_PROJECT="your-project" +export BRAINTRUST_GOOGLE_DISCOVERYENGINE_APP="your-app-id" +export BRAINTRUST_GOOGLE_DISCOVERYENGINE_DATASTORE="your-datastore-id" +``` + +### Record and replay + +From `py/`, select a focused scenario. `--vcr-record=all` enables recording for +both REST and gRPC; the gRPC helper otherwise requires an existing cassette. + +```sh +# REST ranking, including manual/setup entry-point coverage. +mise exec -- nox -s 'test_google_discoveryengine(latest)' -- --vcr-record=all -k 'test_rank and not test_rank_output_limit' + +# Async gRPC ranking. +mise exec -- nox -s 'test_google_discoveryengine(latest)' -- --vcr-record=all -k 'test_async_grpc and rank' + +# Verify all recordings without network access to Google. +mise exec -- nox -R -s 'test_google_discoveryengine(latest)' -- --vcr-record=none +``` + +Playback derives resource paths from the checked-in REST cassettes and ignores +resource environment overrides. No access to the recorded project's resources is +needed. The auto-instrument subprocess test also reads its ranking resource from +the cassette. Keep the project's recordings consistent when recording against a +different project. + +The gRPC helper temporarily replaces one callable on one client transport, +records real protobuf requests/responses and errors as JSON, then restores the +callable. Playback reconstructs provider response objects and checks requests; +it does not exercise networking, retries, or real cancellation behavior. +HTTP authorization headers are filtered by shared VCR configuration; gRPC +recordings omit credentials and transport metadata. Inspect new recordings before +checking them in. + +`generate_grounded_content` and `stream_generate_grounded_content` are deferred: +the v1 endpoints returned method-not-found errors during initial recording. +Do not fabricate success recordings for those methods. diff --git a/.agents/skills/sdk-vcr-workflows/references/recording-failures.md b/.agents/skills/sdk-vcr-workflows/references/recording-failures.md new file mode 100644 index 000000000..123bee4d2 --- /dev/null +++ b/.agents/skills/sdk-vcr-workflows/references/recording-failures.md @@ -0,0 +1,52 @@ +# When Recording Fails + +Live recording fails for reasons unrelated to the SDK change. Handle each case so a failed recording does not destroy a good cassette or leave a bad one behind. + +- **Missing or invalid API key (401/403, auth errors):** stop and ask the user for a working key. Never hand-write or edit a cassette to stand in for a recording. +- **Quota or rate limits (429, 402 "no remaining credits"):** a cassette that captured an error response is worse than the old one. Delete only the cassettes that captured the error, wait or ask for another key, then refill them with the default `once` mode. +- **Retired models or upstream drift:** if the live API no longer returns what the test asserts (a retired model returns 404, a moderation block, a search result that no longer appears, a different stop reason), keep the old cassette with `git checkout` and tell the user. Do not loosen the assertion just to get a recording. +- **Hand-authored cassettes:** some cassettes are hand-edited to force a scenario such as a validation retry. Never re-record them; restore them with `git checkout`. + +### Restore from git vs. re-record + +- Restore from git when the cassette is hand-authored, upstream behavior drifted, or the provider is unusable locally (no key, no quota). The old cassette still matches the request the SDK sends. +- Re-record when the SDK's request shape intentionally changed, or when the new pin changes the wire format and playback fails. +- If some tests in a directory may fail to record, record without deleting the directory first, so failed tests keep their old cassettes. If a test fails partway through, `git checkout` its cassette, because a partial refresh can leave truncated files. +- After any refresh, run `git status` on the cassette directory and look for ` D` entries (cassettes deleted but never recreated). + +### Traps that silently lose cassettes + +- **Skipped sessions record nothing.** If a session skips on your Python version (see `_skip_if_incompatible` and the constraints in `py/noxfile.py`), deleting its cassettes first destroys them. Run nox from a compatible interpreter instead. +- **Multi-file sessions stop at the first failing file.** After a partial failure, re-run in `once` mode to fill the files that later test files never recorded. +- **Shared cassette names under `--vcr-record=all`.** When several parametrizations share a cassette name, later cases read an earlier case's response. Delete only those cassettes and re-run in `once` mode, so the first case of each name records and the rest replay. +- **Hangs.** Tests that drive long-lived live sessions can hang while recording. Exclude them with `-k "not "` and keep their old cassettes. + +## Debugging VCR Failures + +When a cassette-backed test fails, check these in order: + +1. **Wrong nox session or provider version** + - Did you reproduce under the exact session from `py/noxfile.py`? + - Are you accidentally testing `latest` when CI pins an older version? + +2. **Cassette missing or stale** + - Does the expected cassette file exist in the right directory? + - Did the test move without moving the cassette? + - Does the cassette still match the intended request shape? + +3. **Behavior changed but cassette did not** + - Did the code intentionally change the payload, headers, streaming sequence, or traced output? + - If yes, re-record narrowly. + +4. **Behavior did not change, but the test became too strict** + - Prefer fixing brittle assertions before churning cassettes. + - Assert the meaningful span structure, not incidental provider noise. + +5. **Local-only pass, CI fail** + - Remember CI uses `record_mode="none"`. + - A local silent record can hide a missing cassette or wrong request shape. + +6. **Optional dependency or auth confusion** + - Do not rely on globally installed provider packages. + - Use the active nox session. + - Read `py/src/braintrust/conftest.py` before assuming credential behavior.