From 2b22a891248b8481ccd5d3479e991be837059fc5 Mon Sep 17 00:00:00 2001 From: Starfolk Date: Fri, 17 Jul 2026 20:00:09 +0000 Subject: [PATCH] fix(instructor): add model/provider metadata, drop __future__ annotations import MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit against `.agents/skills/sdk-integrations/SKILL.md` found two deviations: - Missing `metadata.model` / `metadata.provider` on the parent `task` span. Per SKILL "framework `llm`-like spans that delegate to a separately instrumented client" — Instructor calls into `wrap_openai`/`wrap_anthropic` which own the `llm` leaf — the framework span should stay `task` but carry `metadata.model` and `metadata.provider` for attribution. Read `model` from call kwargs (`instance.default_model` fallback) and `provider` from Instructor's `Provider` enum (`instance.provider.value`, which is already the short "openai"/"anthropic"/... string we want). - `from __future__ import annotations` in `tracing.py` — violates CLAUDE.md rule 8. Only integration doing this. Removed; `requires-python = ">=3.10"` so `str | None` and `dict[str, Any]` work at runtime unchanged. No new tests, no cassette re-recording — attribution reads from the client instance and call kwargs, not the HTTP response. Extended the two `TestInstructorOpenAISpans` assertions to cover the new keys. Co-Authored-By: Claude Opus 4.7 --- .../instructor/test_instructor.py | 4 ++ .../integrations/instructor/tracing.py | 55 ++++++++++++++++--- 2 files changed, 52 insertions(+), 7 deletions(-) diff --git a/py/src/braintrust/integrations/instructor/test_instructor.py b/py/src/braintrust/integrations/instructor/test_instructor.py index 4f024c44..0cb3eefe 100644 --- a/py/src/braintrust/integrations/instructor/test_instructor.py +++ b/py/src/braintrust/integrations/instructor/test_instructor.py @@ -124,6 +124,8 @@ def test_instructor_openai_single_success(self, setup_logger, memory_logger): assert parent["context"]["span_origin"]["instrumentation"]["name"] == "instructor-auto" assert parent["span_attributes"]["name"] == "instructor.create" meta = parent.get("metadata", {}) + assert meta.get("model") == "gpt-4o-mini" + assert meta.get("provider") == "openai" assert meta.get("response_model") == "Person" assert meta.get("mode") == "TOOLS" assert meta.get("max_retries") == 3 @@ -172,6 +174,8 @@ def test_instructor_openai_retries_then_succeeds(self, setup_logger, memory_logg parent = task_spans[0] meta = parent.get("metadata", {}) + assert meta.get("model") == "gpt-4o-mini" + assert meta.get("provider") == "openai" assert meta.get("response_model") == "Person" assert meta.get("mode") == "TOOLS" assert meta.get("max_retries") == 3 diff --git a/py/src/braintrust/integrations/instructor/tracing.py b/py/src/braintrust/integrations/instructor/tracing.py index 48b52b3b..5f110bf8 100644 --- a/py/src/braintrust/integrations/instructor/tracing.py +++ b/py/src/braintrust/integrations/instructor/tracing.py @@ -16,19 +16,21 @@ - ``output``: the extracted Pydantic model (or list of models for iterable/partial helpers). This is Instructor's product and is not present on any provider child span. -- ``metadata``: ``response_model`` (class name), ``mode``, ``max_retries``, - ``retry_count``, ``validation_errors``. +- ``metadata``: ``model``, ``provider``, ``response_model`` (class name), + ``mode``, ``max_retries``, ``retry_count``, ``validation_errors``. The + ``model`` / ``provider`` pair carries attribution for the framework span + per the SKILL "framework delegates transport" rule; the provider child + span still owns token accounting. - ``metrics``: empty. Token usage stays on the provider child span. """ -from __future__ import annotations - import inspect import logging from collections.abc import AsyncIterator, Iterator from typing import Any from braintrust.logger import start_span as _bt_start_span +from braintrust.span_types import SpanTypeAttribute _INSTRUMENTATION = "instructor-auto" @@ -41,9 +43,6 @@ def start_span(*args, **kwargs): return _bt_start_span(*args, **kwargs) -from braintrust.span_types import SpanTypeAttribute - - log = logging.getLogger(__name__) @@ -81,6 +80,37 @@ def _mode_name(instance: Any) -> str | None: return getattr(mode, "name", None) or str(mode) +def _provider_name(instance: Any) -> str | None: + """Read the underlying provider ("openai", "anthropic", ...) from the client. + + Instructor exposes a ``Provider`` enum on every ``Instructor`` / + ``AsyncInstructor`` instance whose ``value`` is exactly the short provider + string we want for ``metadata.provider``. Fall back to the enum ``name`` + lowercased for older builds that only expose the name. + """ + provider = getattr(instance, "provider", None) + if provider is None: + return None + value = getattr(provider, "value", None) + if isinstance(value, str) and value: + return value + name = getattr(provider, "name", None) + if isinstance(name, str) and name: + return name.lower() + return None + + +def _model_name(kwargs: Any, instance: Any) -> str | None: + """Pull the underlying model name from call kwargs (or the client default).""" + model = kwargs.get("model") if isinstance(kwargs, dict) else None + if isinstance(model, str) and model: + return model + default = getattr(instance, "default_model", None) + if isinstance(default, str) and default: + return default + return None + + def _max_retries_value(max_retries: Any) -> Any: """Normalize Instructor's ``max_retries`` argument for metadata logging. @@ -124,11 +154,14 @@ def _build_metadata( *, response_model: Any, instance: Any, + kwargs: Any, max_retries: Any, retry_count: int, validation_errors: list[str], ) -> dict[str, Any]: return { + "model": _model_name(kwargs, instance), + "provider": _provider_name(instance), "response_model": _response_model_name(response_model), "mode": _mode_name(instance), "max_retries": _max_retries_value(max_retries), @@ -211,6 +244,7 @@ def _wrapper(wrapped: Any, instance: Any, args: Any, kwargs: Any) -> Any: metadata=_build_metadata( response_model=response_model, instance=instance, + kwargs=kwargs, max_retries=max_retries, retry_count=tracker.retry_count, validation_errors=tracker.validation_errors, @@ -222,6 +256,7 @@ def _wrapper(wrapped: Any, instance: Any, args: Any, kwargs: Any) -> Any: metadata=_build_metadata( response_model=response_model, instance=instance, + kwargs=kwargs, max_retries=max_retries, retry_count=tracker.retry_count, validation_errors=tracker.validation_errors, @@ -252,6 +287,7 @@ async def _wrapper(wrapped: Any, instance: Any, args: Any, kwargs: Any) -> Any: metadata=_build_metadata( response_model=response_model, instance=instance, + kwargs=kwargs, max_retries=max_retries, retry_count=tracker.retry_count, validation_errors=tracker.validation_errors, @@ -263,6 +299,7 @@ async def _wrapper(wrapped: Any, instance: Any, args: Any, kwargs: Any) -> Any: metadata=_build_metadata( response_model=response_model, instance=instance, + kwargs=kwargs, max_retries=max_retries, retry_count=tracker.retry_count, validation_errors=tracker.validation_errors, @@ -299,6 +336,7 @@ def _iterate() -> Iterator[Any]: metadata=_build_metadata( response_model=response_model, instance=instance, + kwargs=kwargs, max_retries=max_retries, retry_count=tracker.retry_count, validation_errors=tracker.validation_errors, @@ -310,6 +348,7 @@ def _iterate() -> Iterator[Any]: metadata=_build_metadata( response_model=response_model, instance=instance, + kwargs=kwargs, max_retries=max_retries, retry_count=tracker.retry_count, validation_errors=tracker.validation_errors, @@ -349,6 +388,7 @@ async def _iterate() -> AsyncIterator[Any]: metadata=_build_metadata( response_model=response_model, instance=instance, + kwargs=kwargs, max_retries=max_retries, retry_count=tracker.retry_count, validation_errors=tracker.validation_errors, @@ -360,6 +400,7 @@ async def _iterate() -> AsyncIterator[Any]: metadata=_build_metadata( response_model=response_model, instance=instance, + kwargs=kwargs, max_retries=max_retries, retry_count=tracker.retry_count, validation_errors=tracker.validation_errors,