Skip to content

fix: convert Langfuse AGENT observations regardless of parent - #314

Open
poshinchen wants to merge 2 commits into
strands-agents:mainfrom
poshinchen:fix/langfuse-agent-with-parent
Open

fix: convert Langfuse AGENT observations regardless of parent#314
poshinchen wants to merge 2 commits into
strands-agents:mainfrom
poshinchen:fix/langfuse-agent-with-parent

Conversation

@poshinchen

Copy link
Copy Markdown
Contributor

Description

LangfuseProvider never produced an AgentInvocationSpan for Strands agent traces instrumented via OTEL → Langfuse.

The routing in _convert_observation gated both CHAIN and AGENT types on obs.parent_observation_id is None:

if obs_type in ("CHAIN", "AGENT") and obs.parent_observation_id is None:
    return self._convert_agent_invocation(obs, session_id)

The null-parent check is correct for LangChain CHAIN observations (LangChain emits a CHAIN per sub-chain, and only the root chain is the agent invocation). But Strands SDK traces always nest the AGENT observation under a framework SPAN:

SPAN: agent_my-agent_UUID            (type=SPAN, parent=root)
  AGENT: invoke_agent Strands Agents (type=AGENT, parent=agent_span)  ← dropped
    SPAN: execute_event_loop_cycle
      GENERATION: chat
      TOOL: my-tool

Because the AGENT observation has a parent, it was silently skipped. The resulting session contained only InferenceSpan and ToolExecutionSpan objects, so every evaluator that calls _get_last_turn() (which requires at least one AgentInvocationSpan) failed with ValueError. The reporter observed 0/70 production Strands traces producing an AgentInvocationSpan.

Changes

  • Route AGENT observations to _convert_agent_invocation regardless of parent — Strands nests them under a SPAN, so requiring a null parent drops every agent invocation.
  • Keep the null-parent check on CHAIN, where only the root chain is the agent invocation.
  • Update the routing docstring to document the distinction.

The SPAN fallback (name starts with invoke_agent), GENERATION, and TOOL paths are unchanged.

Related Issues

Fixes #311

Documentation PR

N/A

Type of Change

Bug fix

Testing

Added TestLangfuseAgentType covering:

Existing test_child_chain_is_skipped still passes, confirming LangChain sub-chain filtering is unaffected. Full tests/strands_evals/providers/test_langfuse_provider.py suite passes; ruff clean.

  • I ran hatch run prepare

Checklist

  • I have read the CONTRIBUTING document
  • I have reviewed and understand every line of code in this PR, including any generated by AI tools, and I can explain why it works
  • My change is focused and reasonably small; I have split unrelated work into separate PRs
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@poshinchen
poshinchen requested a review from a team as a code owner July 15, 2026 19:22
@poshinchen
poshinchen requested a review from pgrayy July 15, 2026 19:22
@poshinchen
poshinchen temporarily deployed to manual-approval July 15, 2026 19:23 — with GitHub Actions Inactive
@github-actions github-actions Bot added bug Something isn't working area-tracing Trace/session ingestion: providers, session mappers, extractors, telemetry/OTEL labels Jul 15, 2026
@poshinchen
poshinchen force-pushed the fix/langfuse-agent-with-parent branch from ec72436 to 5586628 Compare July 16, 2026 15:15
return content, error
# Strands OTEL format: {"message": "<JSON string of [{\"text\": ...}]>", "id": "..."}
if "message" in obs_output:
return self._parse_tool_result_message(obs_output["message"]), None

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issue: The Strands OTEL message branch always returns error=None, unlike the result/status branch above which surfaces failures. If a Strands OTEL tool result represents a failure, the error is silently dropped, so failure-detection / root-cause evaluators would treat a failed tool call as successful.

Suggestion: Confirm whether the OTEL TOOL output ever carries error/status information (e.g. a status field or an error marker inside message). If it can, surface it as the error return value; if the format genuinely never encodes tool errors, add a brief comment noting this limitation so it's a documented decision rather than an accidental gap.

@github-actions

Copy link
Copy Markdown

Issue: The PR title, description, and "Testing" section only describe the AGENT routing fix (#311). However roughly half the diff adds Strands OTEL TOOL parsing — _parse_tool_arguments_from_list, _parse_tool_result_message, the message/list branches in _parse_tool_result, and three new tests whose docstrings reference issue #312. This is functionally related work, so bundling is defensible, but a reviewer reading the description wouldn't know it's here.

Suggestion: Update the description to document the tool-parsing changes and link #312 (and reflect them in the Testing section), or split them into a separate PR to keep each change focused per the contributor checklist.

@github-actions

Copy link
Copy Markdown

Assessment: Comment (non-blocking)

The core fix is correct and well-reasoned: gating AGENT conversion on a null parent dropped every Strands invocation (nested under a framework SPAN), while CHAIN correctly keeps the null-parent check for LangChain sub-chains. The docstring clearly documents the distinction, tests pass (46/46), and ruff is clean.

Review themes

Nice, targeted root-cause fix with a clear explanation of the LangChain-vs-Strands nesting difference.

Strands SDK traces nest the AGENT observation under a framework SPAN, so
the null-parent gate on the combined CHAIN/AGENT branch dropped every
Strands agent invocation. No AgentInvocationSpan was produced, and every
evaluator calling _get_last_turn() failed with ValueError.

Route AGENT to _convert_agent_invocation unconditionally; keep the
null-parent check on CHAIN, where only the root chain is the agent
invocation (LangChain emits a CHAIN per sub-chain).

Fixes strands-agents#311
@opieter-aws

Copy link
Copy Markdown

@strandly-the-agent can you review this PR?

@strandly-the-agent strandly-the-agent 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.

Verdict: the narrow fix is correct — verified with a repro, existing suite (49/49), and a read of the Strands tracer source. One real 🟡 should-fix: the same nesting problem the docstring calls out for CHAIN also applies to AGENT, but only CHAIN gets a root-only guard. Everything below is checked against src/strands_evals/providers/langfuse_provider.py at pr314 (5c5bc3e) vs merge-base c5461d0.

1. Routing fix itself — correct

_convert_observation (langfuse_provider.py:229-232): AGENT now converts unconditionally, CHAIN keeps the null-parent gate. LangChain behavior is unaffected — test_child_chain_is_skipped and TestLangChainEndToEnd still pass, and I confirmed obs.type on the installed SDK (langfuse 3.15.0) is a required str field on Observation (not Optional), so the "obs.type is None" concern from the brief doesn't apply — ruled out, not a live risk.

2. 🟡 Should-fix: no root-only guard for AGENT ⇒ multi-agent traces get >1 AgentInvocationSpan, silently corrupting output/eval inputs

Confirmed via Strands SDK source (strands/agent/agent.py:1470tracer.start_agent_span, strands/telemetry/tracer.py:687-740): every Agent() invocation emits its own invoke_agent <name> span, with no distinction between a top-level call and a sub-agent invoked from inside a tool/Swarm/Graph node. So any Strands multi-agent trace (agent-as-tool, Swarm, Graph) produces multiple obs.type=="AGENT" observations in one Langfuse trace — this is the mainstream shape for Strands multi-agent apps, not a contrived edge case.

Repro (/tmp/repro/repro_multiagent.py, orchestrator + sub-agent-as-tool, both type=="AGENT"):

_extract_output → 'Flights: AA123, UA456'          (should be the orchestrator's answer)
_get_last_turn().agent_response → 'Flights: AA123, UA456'  (same — used by ~12 evaluators, evaluator.py:136)
turn[0].session_history and turn[1].session_history both contain the SAME full tool-call list

Root cause chain:

  • langfuse_provider.py:769-777 (_extract_output) and trace_extractor.py:43-85/evaluator.py:128-136 (_get_last_turn) both pick the trace's last AgentInvocationSpan in list order — the sub-agent's, not the orchestrator's.
  • trace_extractor.py:47 (_find_tool_execution_spans) collects all ToolExecutionSpans in the trace once, then attaches that full list to every turn (trace_extractor.py:60-68) — duplicated tool history per turn.
  • _fetch_all_pages (langfuse_provider.py:130-139) never sorts observations, and Langfuse's observations.get_many has no order_by param at all — so "last in list" isn't even chronological, it's whatever order the API happens to return.

Not a regression from this PR — I checked out the merge-base and reran the same shape through the pre-existing SPAN+name="invoke_agent" fallback (langfuse_provider.py:236-241, unchanged by this diff, no parent check either): it already produced 2 AgentInvocationSpans and the same wrong _extract_output before this PR. What this PR changes is that the new mainstream ingestion path (Langfuse v4-style type=="AGENT") now also hits this latent gap — previously it hit a different bug (0 spans → loud ValueError), now it silently succeeds with plausible-but-wrong data for multi-agent traces, while correctly fixing the single-agent case (the reported issue #311).

Sibling mappers already carry the fix for this exact class of problem: mappers/langchain_otel_session_mapper.py:143-152 and mappers/openinference_session_mapper.py:256-261 explicitly keep only the root AgentInvocationSpan when a trace yields more than one, with a comment calling out multi-agent nesting. langfuse_provider.py has no equivalent for AGENT (or CHAIN, or the SPAN fallback, for that matter).

Minimal in-scope mitigation, if you want it in this PR (otherwise worth a tracking issue so it's not lost): after converting a trace's observations, when >1 AgentInvocationSpan came from AGENT/SPAN-fallback routing, keep only the one whose source observation has no ancestor (via obs.parent_observation_id, which Langfuse does give you, unlike the ADOT-based mappers that fall back to a start-time heuristic) that is itself one of the other AgentInvocationSpan observations. That's ordering-independent, which matters since observations aren't sorted here.

3. Other correctness notes
  • A separate SPAN-named-invoke_agent observation coexisting with an AGENT observation in the same trace doesn't add a new failure mode beyond #2 above — both routes feed the same unguarded _convert_agent_invocation call, so it just adds another entry to the same "multiple AgentInvocationSpans" bucket.
  • Exception handling is unaffected by this diff — _convert_observations (langfuse_provider.py:186-193) already catches and logs per-observation conversion failures.
4. Docstring — accurate but has a gap

The new Note (langfuse_provider.py:215-219) correctly explains why AGENT and CHAIN differ, and in the same breath says "CHAIN emits one per sub-chain, only the root is the agent invocation" — but Strands' own AGENT observations have the identical multiplicity property (per #2), and the docstring doesn't mention it. Given the fix is deliberately silent on that symmetry, worth a one-line caveat so it doesn't read as "AGENT is always 1-per-trace." Also ⚪ nit: the class-level bullet list a few lines up (langfuse_provider.py:199-203) still doesn't mention AGENT at all, even though the Routing table right below it does.

Nothing here should block landing the single-agent fix (#311) — it's correct and the tests for it pass. I'd just make sure the multi-agent gap doesn't get lost; happy to file a tracking issue if that's preferred over holding this PR.

@opieter-aws

Copy link
Copy Markdown

@poshinchen do you want to pull in the multi agent fix here too?

@strandly-the-agent

Copy link
Copy Markdown

Follow-up to my review above. @opieter-aws — re your question to poshinchen: yes, I'd pull it in. Adversarial testing turned that 🟡 into a repro-backed 🔴, and it is not Strands-only — nested LangChain/LangGraph AGENT nodes hit it too, which also undercuts the docstring's stated rationale. A ~20-line parent-linkage mitigation passes 356/356 existing tests; an xfail-pinned version is the cheaper option if you'd rather ship #311 as-is.

Still recommend landing the #311 fix — it turns a loud ValueError into working evals for the single-agent majority. The question is only whether the multi-agent gap ships with it or as a follow-up.

✅ Evidence ledger — what I ran myself

Reviewed pr314 @ 5c5bc3e5, merge-base c5461d0a.

Check Result
tests/.../test_langfuse_provider.py + extractors/ on PR head 90 passed
Two-AGENT trace through real provider + TraceExtractor + _get_last_turn 2 agent spans, output = sub-agent's answer
Same trace, observation order reversed output changes → order-dependent
Langfuse ObservationTypeMapper.ts (upstream main) invoke_agentAGENT, per-span, no nesting awareness
strands 1.50.2 start_agent_span 1 call site (agent/agent.py:1470), unconditional operation_name="invoke_agent" (telemetry/tracer.py:711)
Installed langfuse/langchain/CallbackHandler.py classifier 5/8 chain components → agent
Mitigation prototype: providers + extractors + mappers suites 356 passed
Mitigation prototype: 6 multi-agent scenarios 6/6

Not verified: no live model calls or real Langfuse instance — the Strands/LangChain span shapes are source-verified, then replayed as fixtures through the real provider code.

1. 🔴 The wrong answer, reproduced end to end (raised from 🟡)

Orchestrator + sub-agent, both type=="AGENT", through the real provider and extractor:

AgentInvocationSpans: 2 ['Booked!', 'found 3 options']
TaskOutput.output:        'found 3 options'   <-- sub-agent, not the orchestrator
TRACE_LEVEL turns:        2
last turn agent_response: 'found 3 options'   <-- what ~12 evaluators score

Why this is the mainstream shape and not a contrived one: start_agent_span has exactly one call site (strands/agent/agent.py:1470) and always sets operation_name="invoke_agent" with no root/nested distinction, and Langfuse maps that attribute to AGENT per-span with no awareness of parents. So every sub-agent in an agent-as-tool / Swarm / Graph app becomes another AgentInvocationSpan.

2. Genuinely new: this isn't Strands-only, and the docstring rationale doesn't hold

The new docstring says "LangChain emits a CHAIN per sub-chain and only the root chain is the agent invocation". That isn't what the installed Langfuse integration does: langfuse/langchain/CallbackHandler.py:186-207 classifies a chain callback as agent on a bare substring match against the serialized class path or the run name, regardless of nesting — and on_chain_start:328,346 attaches it under parent_run_id. Ran its own classifier (langchain 1.3.14): AgentExecutor (root) → agent, nested RunnableAgentagent, a LangGraph create_react_agent node named 'agent'agent, a subgraph named 'research_agent'agent.

Replaying a LangGraph supervisor + 2 sub-agents in that shape: _extract_output'4' (the math sub-agent) instead of 'Paris has ~2.1M residents; 2+2=4.', and 4 turns handed to a judge. This is exactly what mappers/langchain_otel_session_mapper.py:143-152 deliberately collapses to the root. TestLangChainChainType/TestLangChainEndToEnd stay green only because neither covers a name-matching nested chain.

3. Why a list-order fix (like the sibling mappers') isn't safe on this path

_fetch_all_pages (langfuse_provider.py:130-140) never sorts, and ObservationsClient.get_many exposes no order_by/sort param at all — its params are page, limit, name, user_id, type, trace_id, level, parent_observation_id, environment, from_start_time, to_start_time, version, filter, request_options. So "last in the list" is whatever row order the server returns. Same fixture as §1, observations reversed:

TaskOutput.output:                       'found 3 options'
TaskOutput.output (reversed obs order):  'Booked!'

The scored output flips. Worth noting langchain_otel_session_mapper.py:146-151 picks the root as agent_spans[-1] ("outermost finishes last") — an end-time assumption that doesn't hold here. Keying on parent linkage avoids the issue entirely.

4. Mitigation prototype (~20 lines, not applied to this branch)

Mirrors the sibling mappers but keys on parent linkage over the raw observations — that detail matters: Strands' intermediate execute_event_loop_cycle observations are skipped during conversion, so walking converted spans breaks the chain and the agent-as-tool case still fails. span_id is obs.id (langfuse_provider.py:258), so the ids line up.

def _drop_nested_agent_invocations(self, spans, observations):
    agent_ids = {s.span_info.span_id for s in spans if isinstance(s, AgentInvocationSpan)}
    if len(agent_ids) < 2:
        return spans
    parent_of = {obs.id: obs.parent_observation_id for obs in observations}

    def nested_under_agent(obs_id):
        seen, cur = {obs_id}, parent_of.get(obs_id)
        while cur is not None and cur not in seen:
            if cur in agent_ids:
                return True
            seen.add(cur); cur = parent_of.get(cur)
        return False

    return [s for s in spans
            if not isinstance(s, AgentInvocationSpan) or not nested_under_agent(s.span_info.span_id)]

Called as return self._drop_nested_agent_invocations(spans, observations) at the end of _convert_observations. Verified 6/6:

[PASS] #311 single agent under wrapper SPAN                 n_agent=1 output='4'
[PASS] agent-as-tool (start_time ASC / DESC / end_time ASC) n_agent=1 output=orchestrator's
[PASS] LangGraph supervisor + 2 nested sub-agents           n_agent=1 output=root's
[PASS] Strands Graph: 2 sibling agents (both kept)          n_agent=2

The last row is the case to be careful about: siblings under a non-agent invoke_graph parent are genuinely separate turns, so they're kept. providers + extractors + mappers suites with it applied: 356 passed, including both new TestLangfuseAgentType tests.

If you'd rather keep this PR narrow, the cheap alternative is landing the three multi-agent cases as xfail so the gap is pinned rather than undocumented. Happy to open either as a separate PR.

5. Suggested test for the gap (passes against this branch today)

Follows the file's _obs/_get_spans conventions:

def test_two_agent_observations_both_convert(self, provider, mock_client):
    """Nested AGENT (orchestrator -> sub-agent) both become AgentInvocationSpan.

    Pins the multi-agent fanout: TraceExtractor emits one turn per span and
    _get_last_turn takes the last, so evaluators score the sub-agent's answer.
    If AGENT is later restricted to the outermost observation, this moves with it.
    """
    spans = self._get_spans(
        provider, mock_client,
        [
            _obs("o-orch", "t1", "AGENT", name="invoke_agent Orchestrator",
                 obs_input="Book a flight", obs_output="Booked!", parent_observation_id=None),
            _obs("o-sub", "t1", "AGENT", name="invoke_agent FlightFinder",
                 obs_input="find flights", obs_output="found 3 options", parent_observation_id="o-orch"),
        ],
    )
    agents = [s for s in spans if isinstance(s, AgentInvocationSpan)]
    assert len(agents) == 2
    assert {a.agent_response for a in agents} == {"Booked!", "found 3 options"}
Housekeeping
  • The 2026-07-16 scope comment on this thread is stale. It was accurate at 99fd32a8, but that tool-parsing work landed separately as fix: fix tool parsing from list #313 and is now in main; git diff c5461d0a..5c5bc3e5 today is only the routing fix + tests. Scope is clean — flagging so nobody reading top-to-bottom is misled.
  • get_evaluation_data's output shape changes for Strands traces (0 → ≥1 AgentInvocationSpans). Anyone using the subclass workaround from [BUG] LangfuseProvider skips AGENT observations with parent (Strands agent traces never produce AgentInvocationSpan) #311 will see different data — maybe worth a line in the release notes.
  • Labels bug + area-tracing look right; the unchecked hatch run prepare / docs boxes read as honest rather than sloppy (ruff + mypy are clean on the file from my run).
Open questions for `poshinchen` (non-blocking)
  1. Which Langfuse ingestion path produced the reporter's 70 traces? Their diagram shows an outer agent_my-agent_UUID wrapper SPAN that a plain-SDK capture doesn't produce (the root invoke_agent has parent=None there) — looks like Agent Studio / AgentCore. Were the nested invoke_agent spans in those traces also AGENT-typed, or did that pipeline only ever emit one agent observation?
  2. Was the LangChain AGENT nesting behaviour (§2) considered when writing the "only the root chain is the agent invocation" rationale? If the intent is "Strands nests, LangChain doesn't", the installed handler doesn't quite support that split.

All of the above is one agent's pass with the repros attached — worth a human sanity-check before you act on it, particularly the call on whether the multi-agent fix belongs in this PR or the next one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-tracing Trace/session ingestion: providers, session mappers, extractors, telemetry/OTEL bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] LangfuseProvider skips AGENT observations with parent (Strands agent traces never produce AgentInvocationSpan)

4 participants