Skip to content

fix(experiments): maintain propagated context in async experiments - #1587

Merged
hassiebp merged 1 commit into
mainfrom
fix-experiments-propagate-attributes
Mar 30, 2026
Merged

fix(experiments): maintain propagated context in async experiments#1587
hassiebp merged 1 commit into
mainfrom
fix-experiments-propagate-attributes

Conversation

@hassiebp

@hassiebphassiebp commented Mar 30, 2026

Copy link
Copy Markdown
Collaborator

Disclaimer: Experimental PR review

Greptile Summary

This PR fixes context propagation for async experiments by capturing the caller's contextvars context at the point where _RunAsyncThread is created and executing the coroutine within that copied context. Previously, when run_experiment was called from an async context (e.g. a FastAPI handler or Jupyter notebook), the worker thread started a brand-new context, silently dropping any ContextVar values — such as user_id set via propagate_attributes — before the experiment spans were created.

Key changes:

  • langfuse/_client/utils.py: _RunAsyncThread.__init__ now calls contextvars.copy_context() to snapshot the caller's context, and run() uses self.context.run(asyncio.run, self.coro) to execute the coroutine inside that snapshot.
  • tests/test_utils.py: Adds a focused unit test (test_run_async_context_preserves_contextvars) that sets a ContextVar in an async caller, invokes run_async_safely, and asserts the value is readable inside the threaded coroutine.
  • tests/test_propagate_attributes.py: Adds an end-to-end integration test confirming that user_id set via propagate_attributes is present on the experiment-item-run span when run_experiment is called from async code.

Confidence Score: 5/5

Safe to merge — the fix is minimal, correct, and well-tested; the single finding is a P2 style issue (inline import) that does not affect runtime behavior.

The core change in utils.py is a well-understood Python idiom (contextvars.copy_context() + context.run()) with no risk of regression. Both a unit test and an integration test cover the new behavior. The only finding is a cosmetic import placement issue in the new test, which does not affect correctness or merge safety.

tests/test_propagate_attributes.py — minor: import asyncio should be moved to the module top per project convention.

Important Files Changed

FilenameOverview
langfuse/_client/utils.pyCaptures caller's contextvars via contextvars.copy_context() at thread creation and executes the coroutine inside that copied context, correctly propagating ContextVar values (e.g. user_id) to async experiments running in a worker thread.
tests/test_utils.pyAdds test_run_async_context_preserves_contextvars to directly verify that a ContextVar set before calling run_async_safely is readable inside the threaded coroutine; import moved correctly to the module top.
tests/test_propagate_attributes.pyAdds an end-to-end async experiment test verifying user_id propagation; contains an import asyncio placed inside the test function body, violating the module-level import convention.

Sequence Diagram

sequenceDiagram
participant AC as Async Caller
participant RAS as run_async_safely()
participant RAT as _RunAsyncThread
participant CTX as contextvars.Context (copied)
participant CORO as Coroutine (experiment task)
AC->>AC: propagate_attributes(user_id="x") sets ContextVar
AC->>RAS: run_async_safely(coro)
RAS->>RAS: asyncio.get_running_loop() → running loop detected
RAS->>RAT: __init__(coro) — copy_context() captures current ContextVars
RAS->>RAT: start() + join()
RAT->>CTX: context.run(asyncio.run, coro)
CTX->>CORO: executes with copied context — user_id visible
CORO-->>RAT: result
RAT-->>RAS: thread.result
RAS-->>AC: return result
Loading

Reviews (1): Last reviewed commit: "fix(experiments): maintain propagated co..." | Re-trigger Greptile

Context used:

  • Rule used - Move imports to the top of the module instead of p... (source)

Learnt From
langfuse/langfuse-python#1387

@github-actions

Copy link
Copy Markdown

@claude review

@claudeclaudeBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — the fix is correct and well-tested.

Extended reasoning...

Overview

The PR modifies _RunAsyncThread in langfuse/_client/utils.py to capture the caller's contextvars context via contextvars.copy_context() at construction time and then execute the coroutine within that context using self.context.run(asyncio.run, self.coro). This is a 3-line production change plus tests.

Security risks

None. This is a pure context propagation fix with no auth, crypto, or permission implications.

Level of scrutiny

Low. The change is small, follows the documented Python pattern for context propagation across threads, and is well-covered by the new unit test in test_utils.py and integration test in test_propagate_attributes.py.

Other factors

Two bugs were flagged by the bug hunter: a nit about time.sleep vs await asyncio.sleep in the new async test (the test still passes correctly; no competing coroutines), and a pre-existing dead else-branch in run_async_safely that is unrelated to this PR. Neither warrants blocking the fix.

Comment threadtests/test_propagate_attributes.py
Comment threadlangfuse/_client/utils.py
@hassiebp
hassiebp merged commit df702da into mainMar 30, 2026
14 checks passed
@hassiebp
hassiebp deleted the fix-experiments-propagate-attributes branch March 30, 2026 12:04
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@hassiebp