SIGN IN SIGN UP

fix: let the caller own the flow span so a run's spans nest under it (#14511)

* fix: let the caller own the flow span so a run's spans nest under it

async_start is an async generator, so a span it opens cannot be made current:
the context token would be attached and detached across the generator's
suspension points and leak into whatever task resumed it. A span that is not
current cannot parent anything, so every span produced during the run sat beside
the flow span instead of under it. Under HTTP that means siblings of the server
span; with no ambient span, a separate trace entirely.

The callers are coroutines and thread entry points, which have no such problem,
so the span moves to them and async_start takes open_flow_span=False. Converted:
the agentic assistant (both coordinator consumers, the served path), the lfx CLI,
lfx run, preload, and Graph.start.

The flag defaults to True, so a caller that has not been converted still gets a
span rather than silently none. Those runs keep the flat shape they have today.
The mistake the flag makes possible is deferring and then not opening one, which
is silent, so async_start warns when it is told the caller owns the span and no
span is current. It warns rather than raising, because telemetry must never take
a run down.

The nesting test in test_mcp_tool_span.py was an xfail and is now a real
assertion. Its counterpart is kept, asserting the detached span cannot parent,
so the reason the span had to move does not get lost.

Verified: a span opened mid-run parents to the caller's flow span, an
unconverted caller still emits exactly one flow span, and deferring with no
caller span emits none. 752 passed across graph, mcp and execution; the two
catalog-policy failures there fail identically with these changes stashed.

* fix: pass the event manager to async_start by keyword

async_start takes (inputs, max_iterations, config, event_manager). The sync
start forwarded the event manager positionally, so it landed in config and
__apply_config subscripted it.

That happens on the worker thread start spawns, so nothing reaches the
caller: the exception is logged there and the generator finishes empty. A run
that produces no results and no error is the worst shape for this, so the
regression test asserts on the results rather than on not raising.

Measured before and after on a ChatInput -> ChatOutput graph:
  start(event_manager=...)  0 results -> 3 results

Pre-existing rather than introduced here; release-1.12.0 has the same call
without the keyword. Fixed on this branch because it is the line this PR
already changes. config is applied by start itself before the thread, so it
is deliberately not forwarded again.

Found by CodeRabbit on this PR.

* fix: carry the caller context into the worker thread

Graph.start hands the run to a worker thread, and a thread starts with an
empty context. So the flow span opened in there found no current span and
began a brand-new trace: measured, the caller's span and flow.execute came
back with different trace ids and no parent and no link between them. A
caller's span and the run it started were two unrelated traces in the
operator's APM, which is the correlation this branch exists to provide.

Copy the context and run the thread body inside it. The same copy carries the
protocol and client contextvars, which are set at the entry point for exactly
this reason.

The caller's span is still recording while the generator is consumed, so the
flow span parents to it normally. A caller that abandons the generator early
leaves a dead parent, which flow_execution_span already handles by linking
instead of parenting.

The regression test asserts on the trace id rather than the parent alone: a
parent could be satisfied by a span that happened to nest under something
else in the worker. Red before this with two different traces.

Found by CodeRabbit on this PR.

* test: give the stand-in graph the flow span the caller now opens

This branch moved the flow span out of async_start and into the callers, so
_run_graph_with_events opens graph.flow_execution_span() itself. The
coordinator test drives it with a dataclass stand-in that has no such method,
and the AttributeError is swallowed by the broad except around that block, so
the coordinator was never reached.

The failure read as 'the coordinator was not used' (assert 0 == 1) rather
than 'the stand-in is missing a method', which is the more expensive kind of
red. Verified it fails at this branch's head before my commits and passes on
release-1.12.0, so it came in with the span move rather than with the keyword
and context fixes.

src/backend/tests/unit/agentic: 1621 passed. The one remaining failure there,
test_execute_named_assistant_flow_is_graceful_not_500, reproduces on
release-1.12.0 untouched.

* fix: let a failed sync run reach both the caller and the span

The worker caught ValueError inside the flow span scope and let everything
else escape the thread entirely. Two different wrong outcomes from one block.

Measured before, on a cyclic graph hitting max_iterations, which is where
async_start raises a raw ValueError:

  raised_to_caller=ValueError   flow spans exported=0

The handler sat inside the span scope, so the consumer raised the moment the
exception was queued, the generator was abandoned, thread.join never ran and
the worker never finished ending the span. The run failed and the operator's
APM has nothing at all for it.

And on a component failure, which arrives wrapped as ComponentBuildError and
so missed the ValueError handler completely:

  raised_to_caller=None         span status=error

It propagated out of the worker, died with the thread, and the caller read the
end-of-stream sentinel and saw a clean finish. A run that failed returned no
error to a synchronous caller.

Nothing is caught inside the scope now. The failure leaves the span, the span
records it, and a worker-boundary handler enqueues every Exception rather
than just ValueError. The consumer already re-raises whatever it finds, so the
caller gets what a synchronous call should have given it. The None sentinel is
still written in the finally, after the exception, so the caller reads the
error first.

Regression covers both kinds and both halves, and each fails without the fix
for its own reason: the ValueError case on 0 spans, the other on the caller
receiving nothing.

Found by erichare in review.
G
Gabriel Luiz Freitas Almeida committed
11169d7ece0bbd14d500249d33130aa5481bbb40
Parent: 68796a6
Committed by GitHub <noreply@github.com> on 8/13/2026, 11:43:24 PM