fix(instructor): instrument the v2 retry call path; tolerate modern patch() API

[Harsh23Kashyap]

fix(instructor): instrument the v2 retry call path; tolerate modern patch() API

Target: Arize-ai/openinference, python/instrumentation/openinference-instrumentation-instructor (upstream fix - no downstream monkey-patch).

Versions

• Before: openinference-instrumentation-instructor 0.1.26 (current PyPI release as of 2026-09-16) AND current main (5da0a96) with instructor 1.15.4 / 1.17.0 - 0 spans on the documented path, TypeError/ValueError on the patch() path. main has not fixed this: _instrument still wraps only instructor.patch + handle_response_model, and _PatchWrapper still reads client/create only from kwargs and assumes a callable return.
• After: this patch against main 5da0a96 (diff sha256 bd3027a0118b4a73eaf7c8296be3056dcfe50c0bc29336c5a764b94a3d9d09fb; the PR commit SHA replaces this identifier once opened). Built from source and verified against instructor 1.15.4 (neatlogs 1.4.23's uv.lock baseline) + neatlogs 1.4.23.

What changes

• _instrument wraps instructor.v2.core.retry.retry_sync_v2 / retry_async_v2 (the real per-call boundary in instructor >= 1.15) BEFORE resolving instructor.patch, so v2.core.patch's by-value import binds the wrapper. Emits one CHAIN span per create call.
• _PatchWrapper accepts a positionally passed client and passes through non-callable patch() results (modern client-returning API) instead of calling them.
• Legacy handle_response_model wrapping kept unchanged for instructor < 1.15.

Both documented API paths exercised

1. instructor.from_openai(OpenAI(), mode=Mode.JSON).chat.completions.create(response_model=User, ...) (current README-primary path)
2. instructor.patch(OpenAI(), mode=Mode.JSON) with the client passed positionally (previously crashed)

Proof - 20 repeat deterministic runs

Runs	Passes	Detail
20/20, alternating two isolated venvs	20	exit 0; 4/4 assertions (both paths parse response_model; patch() no crash; 2 instructor.retry_sync_v2 CHAIN spans, status OK); all 20 normalized outputs byte-identical, sha256 prefix 24a522e168f1d87a...

Same probe pre-patch: 0 spans (documented path) + TypeError (patch path), twice-identical.

Known gap (no parity implied)

• instructor.response_model attribute extraction is best-effort: it does not yet read the retry function's real kwargs shape, so the emitted CHAIN span carries kind + input messages but NOT yet the response-model name or full LLM semantic attributes. This fix restores span emission on the call boundary; it does not claim full telemetry parity with the legacy wrapper's attribute set.

Fixes #3762

[feiiiiii5]

Ran this against instructor 1.15.4 and 1.17.0 in isolated venvs (offline, record_mode="none"). The patch() half is a real fix and I confirmed it: at base instructor.patch(OpenAI(), mode=Mode.JSON) raises ValueError: Either client or create must be provided, and with this branch it returns the patched client and emits instructor.retry_sync_v2. Thanks for that part.

The retry-wrap half only lands under one session state, and I could not get it to change this package's own test outcome. Numbers below are from runs I replayed myself.

1. The wrap depends on whether instructor.v2.core.patch has resolved the name yet

instructor/v2/core/patch.py:28-32 (1.17.0) binds retry_sync_v2/retry_async_v2 by value (from instructor.v2.core.retry import …) and calls them as bare globals (:285). So wrap_function_wrapper("instructor.v2.core.retry", …) (__init__.py:51-62) can only intercept if v2.core.patch has not been imported yet. One from_openai(...).create() per fresh process:

state before instrument()	spans
nothing (instrument first)	1
import instructor only	1
instructor.from_openai resolved first	0
instructor.patch resolved first	0

The same split is visible inside this package's own suite at your head, pytest tests -q on 1.17.0:

• whole file: 3 failed, 4 passed, all three assert 0 == 2
• -k test_instructor_instrumentation alone: assert 1 == 2
• -k test_async_instrumentation alone: assert 1 == 2

and whole-file results are identical at base 5da0a966 and at head 534cd87d, on both 1.15.4 and 1.17.0 (3 failed / 4 passed, assert 0 == 2). So in a shared session the wrap silently never lands, and when it does land the emitted span still does not satisfy what the tests expect.

Wrapping the module attribute is not enough on its own: the name has to be wrapped where it is bound too (instructor.v2.core.patch), or intercepted at the point of use, so the result cannot depend on which import a user — or an earlier test — happened to do first.

2. uninstrument() does not undo the new wrap

_uninstrument (__init__.py:97-105) restores instructor.patch and handle_response_model, and __slots__ (:18-23) has no place to keep the retry originals, so nothing restores them. Measured:

after instrument(A):   retry_sync_v2 wrapped ✔ , v2.core.patch bound to that wrapper ✔ , exporter A: ['instructor.retry_sync_v2']
after uninstrument():  module attr restored? False ; v2.core.patch global restored? False
after instrument(B):   next call → exporter A: 2 spans, exporter B: []

That is the mechanism behind assert 0 == 2 in item 1: the first instrument/uninstrument cycle in a pytest process pins the wrapper (and its tracer) into v2.core.patch forever, so every later test's exporter is unreachable.

3. _RetryV2Wrapper is sync but retry_async_v2 is a coroutine function

inspect.iscoroutinefunction(retry_async_v2) → True. wrapped(*args, **kwargs) returns a coroutine, so the with start_as_current_span(...) block (_wrappers.py:427) closes immediately. Measured on the async path: 1 span instructor.retry_async_v2, kind CHAIN, status OK, duration 0.045 ms while the awaited call took 52.9 ms; output.value = '"<coroutine object retry_async_v2 at 0x…>"'; the span's end timestamp precedes the HTTP window, and during the awaited call get_current_span() is the outer span, so nested instrumentation does not parent to it. When the awaited call fails, the span still reads OK.

This package already has the pattern to copy: _PatchWrapper chooses its branch with is_async(func) (_wrappers.py:181, async span at :239).

4. Nothing in CI can see any of 1-3, and the diff has no tests

gh pr diff 3763 = 2 files, both under src/; no tests/ path. test-requirements.txt pins instructor==1.3.7, and python/tox.ini:27 generates only py310-ci-instructor / py312-ci-instructor — the instructor-latest factor at tox.ini:210 is unreachable (tox -l → 145 envs, 67 of them *-latest, none for instructor), so neither the PR checks nor the cron canary ever installs a v2 instructor. On 1.3.7 the patch is a no-op (base 7 passed = head 7 passed) because instructor.v2 does not exist there. Adding py3{10,12}-ci-{instructor,instructor-latest} plus a test that runs on both would let the project verify this itself instead of relying on prose.

5. Smaller, concrete items

• With CI's pinned ruff 0.9.2 the branch is red: _wrappers.py:433:101: E501 Line too long (101 > 100), and ruff format --diff . → 1 file would be reformatted.
• instructor.response_model is read from create_kwargs = kwargs.get("kwargs"), but in the real call shape response_model is in the outer kwargs (measured inner response_model: None), so that branch never fires; model is present in the inner kwargs and unused. The attribute name also appears nowhere in spec/ or the other instrumentors.
• The v2 span carries only openinference.span.kind, input.mime_type, input.value, output.value; the legacy path also set llm.model_name, llm.provider, llm.system, llm.invocation_parameters, output.mime_type. Your "known gap" says this qualitatively — worth naming the lost keys in the body, since anyone upgrading instructor loses those columns in existing views.

Checked and not a problem, for the record: no double spans on older versions (1.14.5 has no instructor.v2, so the loop skips it); _flatten does map the enum, so the emitted kind is the string "CHAIN"; a sync-path exception propagates with the span marked ERROR plus an exception event; suppress_tracing() yields 0 spans; using_session() context attributes are captured.

Repro for the decisive pair:

python -m pytest tests -q                          # 3 failed, all `assert 0 == 2`
python -m pytest tests -q -k test_instructor_instrumentation   # `assert 1 == 2`

The delta between those two lines is the whole problem: the patch's effect depends on what ran before it in the process.

Disclosure: these measurements were produced with an AI coding agent's help (Claude, via Qoder CLI); I re-ran the commands behind each number myself.