fix(sdk): parent_trace_id contextvar ALWAYS overrides caller-set (0.13.8 hotfix #2)#65
Closed
maltsev-dev wants to merge 5 commits into
Closed
fix(sdk): parent_trace_id contextvar ALWAYS overrides caller-set (0.13.8 hotfix #2)#65maltsev-dev wants to merge 5 commits into
maltsev-dev wants to merge 5 commits into
Conversation
The test_runtime fixture in test_protect_branches.py built a real NullRunRuntime(api_key, _test_mode=True) without going through the mock_api conftest. The runtime's Transport.start() calls _replay_from_wal() which reads /tmp/nullrun.wal (the default NULLRUN_WAL_PATH fallback). If a previous test run in a different Python version (3.10 or 3.12) had persisted a non-empty WAL, the 3.11 worker would replay those events to a real HTTP endpoint, get HTTP 401, and the fixture would fail at setup with NullRunAuthError: nullrun.breaker.exceptions.NullRunAuthError: Invalid API key This bit CI on 2026-07-11 (run 29156199607): tests 3.10 + 3.12 passed, test 3.11 failed with that error in the fixture setup of test_enforce_sensitive_tool_dict_with_fallback_fail_open. The 3.10/3.12 runs cleared the global /tmp/nullrun.wal by reading it first, so 3.11 picked up the next writer. Order- dependent; flaky on the matrix. Pin NULLRUN_WAL_PATH to a tmp_path-scoped file so each test session reads its own fresh empty WAL. Resolves the flake without touching SDK source (no production code change). Verified locally: - pytest tests/test_protect_branches.py -> 43/43 pass - with pre-seeded stale /tmp/nullrun.wal, the previously failing test now passes. No public API change. No SDK_MIN_VERSION bump. Backends on 1.0.0 keep working unchanged. Recommended: 0.13.6 (no version bump needed for a test-only fix).
Follows up on commit 41a16f7 which pinned NULLRUN_WAL_PATH for the test_runtime fixture only. Other tests in test_protect_branches.py / test_runtime_branches.py / test_toolbox_langgraph.py build NullRunRuntime inline (no fixture) and were still picking up a stale WAL from a previous test run, causing HTTP 401 `NullRunAuthError` in 3.12 (CI run 29158094827, job `test (3.12)`). This commit: 1. Adds a shared `make_test_runtime` factory fixture to conftest.py that pins NULLRUN_WAL_PATH to tmp_path, stubs _do_flush / _do_flush_locked / _client, and resets the singleton around the factory. 2. Replaces 4 inline `NullRunRuntime(api_key=..., _test_mode=True)` calls in test_protect_branches.py with `make_test_runtime()`, including: - test_protect_async_kill_re_raises_WorkflowKilledInterrupt - test_get_protected_runtime_falls_back_to_get_runtime 3. Patches the local _make_test_runtime / _make_runtime_with_mocked_auth helpers in test_runtime_branches.py to set NULLRUN_WAL_PATH per-call (via tempfile.mkdtemp) before constructing the runtime. 4. Extends the autouse _test_runtime fixture in test_toolbox_langgraph.py to take tmp_path and pin NULLRUN_WAL_PATH, matching conftest::make_test_runtime. Verified locally on 3.11: - pytest tests/ -n auto: 1219 passed, 1 failed, 7 skipped (1 failure: test_actions.py::TestPauseAction ::test_is_paused_respects_cooldown — pre-existing flake on master, NOT introduced by this commit; verified by git stash + repro on bare master) - ruff check src/: all checks passed - mypy src/: success, no issues in 34 source files Public API unchanged. No SDK_MIN_VERSION bump. Backends on 1.0.0 keep working unchanged. Recommended: 0.13.6 (no version bump needed).
Pre-fix (commit efff530 / release/0.13.6): - langgraph.py::on_llm_end set event["parent_trace_id"] on the llm_call cost event when an LLM call sat inside a chain / agent (parent span from on_chain_start). - BUT: runtime._enrich_event never stamped parent_trace_id from the active span contextvar, so non-langgraph integrations (crewai, autogen, llama_index, plain httpx transport) emitted the field as None. - AND: _build_v3_track_payload (runtime.py:2982) did not map parent_trace_id onto the v3 /track wire payload, so even when the langgraph callback set it, the field dropped at the SDK wire boundary. Result on production (VPS Postgres after deploy 2026-07-11): SELECT count(*), count(parent_trace_id) FROM cost_events WHERE created_at > '2026-07-11 17:54:00'; -- 28 | 0 Zero rows carried the parent trace — the backend unified SELECT third JOIN arm (cs.join_kind = parent_trace_id) never matched, and the workflow detail Recent executions panel showed empty Model / Tokens / Cost on every orchestration row that owned an LLM call. Fix: 1. runtime._enrich_event: stamp parent_trace_id from get_trace_id() contextvar when the caller did NOT set it explicitly. The langgraph callback explicit value wins (no second-guessing), preserving the existing contract. 2. runtime._build_v3_track_payload: map parent_trace_id from wire_event onto the v3 /track body, mirroring the existing trace_id / span_id handling. 3. nullrun.context: add set_trace_id / reset_trace_id / clear_trace_id helpers. Tests that pin the trace contextvar (mimicking @Protect blocks) need a way to set + restore. Matches the existing pattern of set_/get_/clear_server_minted_execution_id. Tests (7 new in test_drift_fixes_2026_07_04.py, all passing): - test_build_v3_track_payload_includes_parent_trace_id mapper surfaces the field on the wire. - test_build_v3_track_payload_omits_parent_trace_id_when_absent backward-compat: legacy single-shot path stays clean. - test_enrich_event_stamps_parent_trace_id_from_contextvar non-langgraph integrations get the field. - test_enrich_event_preserves_caller_set_parent_trace_id langgraph callback explicit value is never overwritten. - test_enrich_event_leaves_parent_trace_id_blank_when_no_contextvar legacy callers do not get a stale value bleed. - test_enrich_event_omits_empty_string_parent_trace_id falsy boundary value treated as None. - test_enrich_event_parent_trace_id_matches_existing_trace_id_field SpanContext invariant (child inherits parent trace_id) protects the backend JOIN. Verification: - pytest tests/test_drift_fixes_2026_07_04.py — 22/22 passed. - pytest tests/ -n auto -q — 1142 passed, 1 pre-existing flake (test_is_paused_respects_cooldown, NOT introduced by this commit). - ruff check src/ — All checks passed. - mypy src/ — Success: no issues found in 34 source files. No public API change. No SDK_MIN_VERSION bump. Backends on 1.0.0 keep working unchanged. Recommended: 0.13.6 (no version bump needed for this wire-fix; the 0.13.6 release ships it).
Bump version 0.13.6 -> 0.13.7 and prepend changelog entry covering the parent_trace_id wire-end-to-end fix (commit e011ba3 on this branch). This release ships the runtime changes that were missing in 0.13.6: - runtime._enrich_event now stamps parent_trace_id from the active span contextvar (so non-langgraph integrations participate in the multi-agent span attachment flow). - runtime._build_v3_track_payload now maps parent_trace_id onto the v3 /track body (so the field reaches the wire even when the langgraph callback set it on the event). After 0.13.6 the SDK was emitting parent_trace_id = null on every cost event, so cost_events.parent_trace_id was 0 / 28 on a production install with active traffic. 0.13.7 fixes the wire side; the backend side (migration 217 + unified SELECT third JOIN arm) was already shipped in the 0.13.6 / master pair. No public API change. No SDK_MIN_VERSION bump. Backends on 1.0.0 keep working unchanged. Backend must have cost_events.parent_trace_id column (migration 217) — already deployed on prod. Recommended: 0.13.6 -> 0.13.7 (patch).
…3.8 hotfix #2) Hotfix #2 for the parent_trace_id wire-add end-to-end work from PR #64 / 0.13.7. PR #64 wired the field on the wire, but a diagnostic script (sdk_diag.py) running on the deployed 0.13.7 revealed cost_events on the backend were missing parent_trace_id: trace_id=cccccccc-... parent_trace_id=NULL model=gpt-4.1-mini tokens=10 The drift was in _enrich_event's parent_trace_id fallback (commit efff530): it used an "if not in enriched" guard, so when langgraph.py::on_llm_end's _active_runs[run_id] lookup missed (run_id drift between the auto-injected chat_model callback and an explicit user-supplied one, or no matching on_llm_start because the user wrapped the LLM call in a non-langgraph stack), the field was absent, the trace_id fallback at line 2422 overwrote the event with the chain contextvar, but parent_trace_id stayed NULL because the old condition was skipped. Override semantics: the chain contextvar is the single source of truth for "what chain does this event belong to". Both langgraph.py::on_llm_end's caller-set value and a non-langgraph caller's absence resolve to the same contextvar; preferring the contextvar when present is idempotent for the happy path AND closes the drift in the unhappy path. Also: - Bump version 0.13.7 -> 0.13.8. - Prepend v3.23 / 0.13.8 changelog entry. - Rewrite the existing test_enrich_event_preserves_caller_set_parent_trace_id to match the new override semantics. - Add 2 new regression tests in TestEnrichEventParentTraceOverride: test_enrich_event_sets_parent_trace_id_when_chain_contextvar_set (drift scenario) and test_enrich_event_parent_trace_id_matches_trace_id_in_chain_mode (SpanContext invariant). Regression coverage: 24 tests in test_drift_fixes_2026_07_04.py (22 prior + 2 new). Full critical suite 157/157 passed. ruff clean, mypy 34/34 source files clean. End-to-end after this hotfix: the next real LLM call inside a chain contextvar should arrive at the backend with both cost_events.trace_id and cost_events.parent_trace_id set to the chain contextvar value, and the unified SELECT third JOIN arm should populate the dashboard's Recent executions panel with Model / Tokens / Cost on the orchestration row.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hotfix #2 for PR #64. Closes the drift bug where cost_events.parent_trace_id stayed NULL on the backend even though cost_events.trace_id carried the chain contextvar value. Root cause was an "if not in enriched" guard in _enrich_event that skipped the parent_trace_id fallback when langgraph.py::on_llm_end's _active_runs[run_id] lookup missed. Fix: always override from the chain contextvar when one is in scope. The chain contextvar is the single source of truth for "what chain does this event belong to" — both langgraph callback's caller-set value and a non-langgraph caller's absence resolve to the same contextvar, so preferring the contextvar when present is idempotent for the happy path AND closes the drift in the unhappy path. Regression coverage: 2 new tests in TestEnrichEventParentTraceOverride (drift scenario + SpanContext invariant). 24/24 tests in test_drift_fixes_2026_07_04.py, 157/157 critical suite, ruff clean, mypy 34/34 source files clean.