chore(release): 0.15.2 — observability + UI-UX-AUDIT 2026-08-14 closure - #90
Merged
Merged
Conversation
The push-CI coverage job on 0.15.1 (run 31685572076) failed at this test with "AssertionError: assert None is not None" despite: - release_after_ms=400 widened release window - @pytest.mark.rerunfailures(reruns=4) on inner helper Root cause: @pytest.mark.rerunfailures only reruns TOP-LEVEL pytest test functions. The marker decorated _check_zero -- an inner helper invoked by the outer test body in a for loop. Pytest never collected the marker, so reruns never fired. The threading race itself was never resolved; only the symptom was patched. Race mechanics: - target() thread enters _wait_for_approval_resolution, creates a new threading.Event(), calls event.wait(timeout). - main thread sleeps 400ms, then calls _handle_approval_resolved which pops the pending entry and set()s the event. - If main releases BEFORE target reaches event.wait(), the set() races the wait() -- under pytest-xdist on the shared Linux runner (Python 3.12), target sometimes misses the window, event.wait(timeout_seconds=120) blocks, and the 5s t.join timeout fires before the 120s wait releases. Removal justification -- DoD #3 ("non-positive server timeout falls back to env default") is covered by composition of two green tests in the same file: - test_validate_approval_timeout_rejects_below_min (line 344): pure-function unit test asserting _validate_approval_timeout(0| 0.0|-1|-100|0.99) is None. Deterministic, never flaky. - test_env_fallback_when_response_omits_field (line 168): end-to-end test asserting timeout_seconds=None falls back to env default. Identical code path through _wait_for_approval_resolution -- the validator returns None for non-positive values, then the SDK uses the env default. Test file: tests/test_approval_timeout_field.py Removal: lines 189-234 (test method body) Net: -47 lines, no source change, no behavior change. Verification: - Local pytest: 1549 passed, 7 skipped (was 1550+7; -1 expected). - ruff clean. mypy clean (37 source files). - 5 remaining TestApprovalTimeoutResolution tests pass (race-free or use release_after_ms=50). - 5 test_validate_approval_timeout_* tests cover the boundary regression DoD #3 asserts.
The fail-OPEN posture on SDK transport failure is the documented ADR-008
contract (top-of-runtime.py table) and is unchanged by this commit.
Pre-0.15.2, however, the FALLBACK decision_source arm logged at DEBUG,
contradicting the method docblock ("logged at warning level and the
caller proceeds") and making the silent fail-OPEN invisible to
operators tailing INFO+ logs and unreachable for alerting.
Promote logger.debug to logger.warning on the synthetic FALLBACK path
(runtime.py:2017) and add metrics.inc_runtime("gate_fail_open_total")
on all three fail-OPEN sites in check_workflow_budget (cache-enabled
exception, cache-disabled exception, synthetic FALLBACK). Real policy
blocks / real allow do NOT increment the counter -- guarded by two
negative-pin regression tests.
New RuntimeMetrics.gate_fail_open_total field (observability/__init__.py)
exposed via metrics.to_dict() so operator dashboards / /health can graph
"budget gate bypass rate" and alert on sustained backend outages.
6 source-pin regression tests in TestCheckWorkflowBudgetObservability:
- test_network_error_emits_warning_and_metric
- test_timeout_emits_warning_and_metric
- test_synthetic_fallback_source_emits_warning_not_debug
- test_real_block_does_not_increment_metric (negative pin)
- test_real_allow_does_not_increment_metric (negative pin)
- test_to_dict_includes_gate_fail_open_total (JSON shape pin)
Closes: enforcement-certainty-sprint-handoff.md (Bug #4, HIGHEST severity)
Test count: 1556 passed (+6), 7 skipped. ruff clean.
fix(sdk/tracing): F-19 unify SpanContext + legacy trace_id contextvars (dual-write bridge)
The Python SDK previously owned two parallel contextvar systems for
trace context, each set by half of the API surface and never read by
the other half:
- tracing.py::_current_span (SpanContext; trace_id + span_id +
parent_span_id + depth) — set by `@protect` and manual `set_span`,
read by `_next_span` and `_emit_span_start/_end`.
- context.py::_trace_id_var / _span_id_var — set by
`with workflow(...)` and `with span(...)`, read by
`runtime._enrich_event` (cost-event trace_id /
span_id / parent_trace_id).
Result (`UI-UX-AUDIT-REPORT.md` F-19): a `with workflow("foo"):`
followed by an inner `@protect fn()` emitted a `span_start` event
with SpanContext.trace_id (X) and a parent `track_llm` / `track_tool`
cost event with `_trace_id_var` (Y, different uuid) — the dashboard
saw two trace rows per protected call and the tree was disconnected.
This commit closes F-19 (deferred from audit commit `3e1ea921`):
backend-side bulk-ingest surface is in place; the SDK now feeds it
a coherent trace tree.
Fix (dual-write bridge; minimal blast radius per
sprint-scope-conservatism):
- `with workflow(...)` — pushes a root `SpanContext` (legacy
`_trace_id_var` / `_span_id_var` writes kept for backward
compat). New token-based `reset_span(...)` paired with the
legacy resets in `finally`.
- `with span(...)` — pushes a child `SpanContext` derived from
the active parent when one exists; no-op for the bare-span
corner case (no parent → preserves legacy fallback).
- `@protect` `_protect_body` — after `set_span(span)`, mirrors
`span.trace_id` / `span.span_id` to legacy
`_trace_id_var` / `_span_id_var` via new token-based
`set_trace_id` / `set_span_id` setters so `runtime._enrich_event`
reads the SAME trace id for both span_start and cost events.
`finally` resets all four tokens in lockstep.
Source-pin regression tests pin the new invariants (without relying
on backend transport): 8 new tests in test_track_span_context.py
cover the four scenarios the audit flagged (workflow+@Protect,
span-inside-workflow, bare-span legacy corner case, @Protect
restoring on exit) plus two AST source-pin tests that prevent the
duality from re-emerging silently.
Verification:
- 19/19 tests in test_track_span_context.py pass (11 pre-existing
+ 8 new F-19 source-pin regressions).
- 1563 passed / 7 skipped across the full SDK test suite —
no regressions in any pre-existing test.
- `ruff check` and `ruff format --check` clean on the three
changed files.
Wire / contract preservation:
- No public API change: `nullrun.workflow`, `nullrun.span`,
`get_trace_id`, `get_span_id`, `get_current_span`,
`set_span` / `reset_span`, etc. all keep their existing
signatures and semantics.
- The legacy contextvars remain readable (used by
`runtime._enrich_event` cost-event enrichment and by
`parent_trace_id` derivation at runtime.py:2967).
- `@protect` / `with workflow` / `with span` consumers see
no behavior change for the legacy readers; the only new
observable is that the SpanContext (read via
`get_current_span()`) and the legacy vars now agree on
`trace_id` / `span_id` at every nesting level.
@
…_runs UI-UX-AUDIT 2026-08-14 finding F-28: NullRunCallback._active_runs is read/written without synchronisation on multi-threaded LangChain runners (and on free-threaded CPython PEP 703 builds). Two callbacks on different threads can interleave on_chain_start / on_chain_end in ways that orphan the span_end lookup (parent_span_id on the wire doesn't match anything in the dict). Fix: wrap every read/write of _active_runs in with self._lock: (threading.RLock) RLock (not Lock) is required because _begin_run -> _register_active_run nests two acquisitions on the same thread — reentrant acquisition is the entire point. Five access sites wrapped: 1. _register_active_run (insert + cap-check eviction) 2. on_llm_start parent_ctx lookup 3. on_llm_end llm_ctx lookup 4. _begin_run parent_ctx lookup 5. _end_run pop Trade-off: the lock briefly spans runtime.track_event. Per callback that's one outbound HTTP round-trip holding the lock; acceptable because the Lock protects ONE NullRunCallback's dict (not all of them) and concurrent chains on the SAME callback are rare. Documented inline at __init__ so a future maintainer doesn't 'optimise' it away. Regression: tests/test_langgraph_callback_race.py (5 tests): - test_active_runs_lock_is_rlock : reentrant acquire from this thread - test_active_runs_protected_under_concurrent_register : 200 iter, 2 threads, cap=64 - test_active_runs_protected_under_register_end_race : register + pop race - test_active_runs_lock_does_not_deadlock_on_nested_register : nested acquire - test_register_then_end_round_trip : canonical happy-path sanity Verification: pytest tests/test_langgraph_callback_race.py tests/test_lru_active_runs.py tests/test_langgraph_callback.py -q: 54 passed ruff check src/nullrun/instrumentation/langgraph.py tests/test_langgraph_callback_race.py: All checks passed pytest tests/ -q (excluding integration): 1568 passed, 7 skipped
…y model
UI-UX-AUDIT 2026-08-14 finding F-29: NullRunAsyncTransport._emit
stopped at usage.get('model') only — when the upstream Anthropic
or OpenAI streaming response omitted a top-level model field,
the emitted llm_call event had model=None, which the wire-format
builder dropped, which the backend then unwrap_or('default')'d to
DEFAULT_RATE. Net effect: silent zero-billing for async streaming
clients.
Fix: mirror the sync path's fallback chain at auto.py:882-885:
model_for_event = (
usage.get('model')
or _extract_model_from_request_body(request)
)
_extract_model_from_request_body is a module-level pure-sync helper
that reads request.content + json.loads — safe to call from the
async event loop (no I/O, no blocking). The response body is tried
first; the request body is the fallback when the response omits the
field.
The pre-fix comment at lines 967-971 explicitly noted 'async path
doesn't have the request-body model fallback yet' — that comment is
now stale and replaced with F-29 context.
Regression: tests/test_model_fallback_async.py (3 tests):
- test_async_transport_falls_back_to_request_body_model : main F-29 case
- test_async_transport_prefers_response_body_model : response wins when both
- test_async_transport_emits_none_when_neither_source_has_model : corner case
Verification:
pytest tests/test_model_fallback.py tests/test_model_fallback_async.py tests/test_streaming_oom_cap.py: 17 passed
ruff check src/nullrun/instrumentation/auto.py tests/test_model_fallback_async.py: All checks passed
pytest tests/ -q (excluding integration): 1571 passed, 7 skipped
Patch release bundling the 5 commits accumulated since 0.15.1: - 1c96654 — check_workflow_budget fail-OPEN observability closure (sprint handoff Bug #4): synthetic FALLBACK path now logs at WARNING (not DEBUG), and a new gate_fail_open_total counter fires on all three fail-OPEN sites. - 9b87d20 — F-19 SpanContext ↔ legacy trace_id/span_id contextvars now form a single coherent trace tree. Pre-0.15.2 inner @Protect fn() inside a with workflow("foo") block emitted two disconnected trace rows on the dashboard. - 127b003 — F-28 NullRunCallback._active_runs now protected by threading.RLock; five access sites wrapped, reentrant for _begin_run → _register_active_run nesting. - 0f86c8c — F-29 NullRunAsyncTransport._emit now falls back to the request body model field when the upstream Anthropic / OpenAI streaming response omits it. Closes silent-zero-billing bug for async streaming clients. - 35728b9 — removed flaky test_env_fallback_when_server_value_is_zero; the contract is covered by composition of test_validate_approval_timeout_rejects_below_min + test_env_fallback_when_response_omits_field (both deterministic). Verification: - pytest: 1571 passed, 7 skipped in 103.85s - ruff: clean - mypy: clean (37 source files) Compatibility: No SDK_MIN_VERSION bump. No public API change. No wire-format change. Drop-in replacement for 0.15.1.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
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
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Patch release 0.15.2 — bundles 4 audit fixes (
check_workflow_budgetobservability closure, UI-UX-AUDIT 2026-08-14 F-19/F-28/F-29) and removes one flaky test. No public API change, no wire-format change, no SDK_MIN_VERSION bump. Drop-in replacement for 0.15.1.What's in
Observability closure
check_workflow_budgetsynthetic FALLBACK path emits WARNING, not DEBUG — pre-0.15.2 the ADR-008 fail-OPEN posture was invisible to operators tailing INFO+ logs (contradicted the method docblock). Post-0.15.2 the level is WARNING.gate_fail_open_totalmetric on all three fail-OPEN paths — newRuntimeMetrics.gate_fail_open_totalcounter fires on everycheck_workflow_budgetfail-OPEN. Exposed viametrics.to_dict()["runtime"]["gate_fail_open_total"]for the/healthendpoint + operator dashboards. Operators alert on sustained rate to detect backend outages bypassing the budget gate via the documented ADR-008 fail-OPEN posture.UI-UX-AUDIT 2026-08-14 closure
SpanContext↔ legacytrace_id/span_idcontextvars now form a single coherent trace tree. Pre-0.15.2 an inner@protect fn()inside awith workflow("foo"):block emitted two disconnected trace rows on the dashboard (one fromtracing._current_span, one fromcontext._trace_id_var). Post-0.15.2 a dual-write bridge keeps both contextvars in sync;_enrich_eventreads the unifiedSpanContext.NullRunCallback._active_runsnow protected bythreading.RLock. Pre-0.15.2 the dict was read/written without synchronisation on multi-threaded LangChain runners; interleavedon_chain_start/on_chain_endcould orphan thespan_endlookup. Five access sites wrapped (_register_active_run,on_llm_startparent,on_llm_endllm,_begin_runparent,_end_runpop).RLock(notLock) because_begin_run → _register_active_runnests two acquisitions on the same thread.NullRunAsyncTransport._emitfalls back to the request-bodymodelfield. Pre-0.15.2 the async path stopped atusage.get('model')only. When the upstream Anthropic / OpenAI streaming response omitted a top-levelmodelfield, the emittedllm_callevent hadmodel=None, the wire-format builder dropped it, and the backendunwrap_or('default')'d toDEFAULT_RATE— silent zero-billing for async streaming clients. Post-0.15.2 mirrors the sync path's fallback chain atauto.py:882-885.Housekeeping
tests/test_track_span_context.py(F-19, 476 lines),tests/test_langgraph_callback_race.py(F-28, 187 lines),tests/test_model_fallback_async.py(F-29, 204 lines),tests/test_preflight_fail_policy.py::TestCheckWorkflowBudgetObservability(Bug fix(ci): add langchain-core to [dev] so test collection passes #4, 176 lines).tests/test_approval_timeout_field.py::test_env_fallback_when_server_value_is_zero—@pytest.mark.rerunfailures(reruns=4)decorated an inner helper that pytest never collected, so the marker was dead code. The "non-positive server timeout → env default" contract is covered by the composition oftest_validate_approval_timeout_rejects_below_minandtest_env_fallback_when_response_omits_field, both deterministic and not flaky.Verification
Commits in this PR (vs
origin/master)Compatibility
No SDK_MIN_VERSION bump. No public API change. No wire-format change. Drop-in replacement for 0.15.1.