Skip to content

fix(converter): keep only last message from full-history agent output (HYBIM-988) - #227

Open
etserend wants to merge 2 commits into
mainfrom
HYBIM-988-fix-agent-output-full-history
Open

fix(converter): keep only last message from full-history agent output (HYBIM-988)#227
etserend wants to merge 2 commits into
mainfrom
HYBIM-988-fix-agent-output-full-history

Conversation

@etserend

@etserend etserend commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • LangGraph passes the full accumulated message history as the agent output
  • After dedup removes the input prefix, multiple messages remain (intermediate steps, tool responses, etc.)
  • The UI renders the first message's text content — which is often empty — showing
  • Fix: when output comes from full history, keep only the last message (the agent's final response)
    [ {"role": "assistant", "parts": [{"type": "tool_call", ...}]}, # content = "" {"role": "tool", "parts": [...]}, {"role": "assistant", "parts": [{"type": "text", "content": "Common dosage..."}]}, ]

Test plan

  • Existing test_attribute_mapping.py tests pass (41 tests)
  • Validated on lab0 erden-framework-testing / healthcare-assistant stream — invoke_agent Agent output column now shows the response text instead of

Fixes https://splunk.atlassian.net/browse/HYBIM-988

@fercor-cisco fercor-cisco left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 This review was generated by the Astra agent (claude-opus-5). It may contain mistakes.

Verdict: request_changes — The reduction is gated on full_history alone rather than on the output actually being accumulated history, so it silently drops real messages from non-root LangGraph node spans (e.g. a ToolNode returning one ToolMessage per parallel tool call).

General Comments

  • 🟠 major (design): The fix is applied at the wrong layer, and a narrower gate exists.

_set_orchestration_content is shared by set_workflow_attributes and set_agent_attributes, but the bug in HYBIM-988 is specific to the root invoke_agent Agent span, where LangGraph hands back the whole accumulated state. Gating on full_history alone catches every span whose output happens to be a top-level {"messages": [...]} container — which via on_chain_end (handlers/langchain/handler.py:119-125) is every LangGraph node, since base_handler.py:145-157 maps non-root chains to WorkflowSpans and serialization.py:197/245/257 guarantees each serialized message carries a role.

Note there is already precedent for solving this at the handler layer: handlers/langchain/middleware.py:199-202 and :213-216 (after_agent / aafter_agent) already do exactly this keep-last reduction on the root agent node before it reaches the converter. Two reasonable directions:

  1. Gate on the dedup having fired (smallest change, keeps the converter as the single place). Per the ticket, dedup does fire in the reported scenario, so this still fixes the bug while leaving node-level spans alone. See the line comment.
  2. Do it in the callback handler, mirroring the middleware, so only the root agent node is reduced and the converter stays a faithful mapper.

Option 1 is the smaller diff; option 2 is more consistent with the existing middleware behavior and avoids the converter making root-vs-node judgements it has no information to make. Either is preferable to the current unconditional gate.

  • 🟡 minor (documentation): No CHANGELOG.md entry under [Unreleased]. This changes what the SDK puts on the wire in gen_ai.output.messages / splunk_ao.output.messages, which is user-visible behavior, and AGENTS.md (Change Workflow step 3) requires a changelog update in that case. The closest precedent, #215 (fix(converter): emit OTel multimodal message parts), did add one.

Follow-ups

Suggested follow-up work that could be tracked as Jira tickets:

  • src/splunk_ao/converter/attribute_mapping.py:220-232: full_history is only True for a top-level "messages" key (container is source, line 228). Accumulated history arriving via a nested "update" key — i.e. a LangGraph Command, which serialization.py:280-286 does serialize as {"update": {...}} — or as a bare JSON list is classified full_history=False and so is never deduped or reduced. Worth confirming whether that asymmetry is intentional; if a root agent output can ever arrive in Command form, HYBIM-988 would still reproduce there.
  • src/splunk_ao/handlers/langchain/middleware.py:196-218: after_agent / aafter_agent already reduce the root agent state to its last message at the handler layer. Once this PR adds an equivalent reduction in the converter, two layers implement the same policy independently. Consider consolidating on one so the behavior cannot drift between the middleware and callback-handler paths.

Comment on lines 448 to +453
if full_history and input_messages is not None and output_messages[: len(input_messages)] == input_messages:
output_messages = output_messages[len(input_messages) :]
# Full history includes all messages in the run, not just the final response.
# Keep only the last message — it is always the agent's final output.
if full_history and len(output_messages) > 1:
output_messages = [output_messages[-1]]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🟠 major (bug): Silent message loss on non-root LangGraph node spans.

full_history is True for any output that parses to a mapping with a top-level "messages" list of role-bearing dicts (_message_container_is_message_sequence, lines 220-228). It does not mean "this output is accumulated history" — only "this output is a message container". Since on_chain_end records every chain's raw return value (handlers/langchain/handler.py:119-125) and base_handler.py:145-157 emits non-root chains as WorkflowSpans, node-level state updates land here too.

Concrete failure: LangGraph's prebuilt ToolNode returns one ToolMessage per tool call, so a turn with two parallel tool calls produces node output {"messages": [tool_msg_1, tool_msg_2]}. Serialized messages always carry role (utils/serialization.py:245), so full_history is True. The dedup on line 448 cannot fire (2 messages vs. the N-message input state), so line 452 fires and gen_ai.output.messages for that invoke_workflow tools span becomes [tool_msg_2]tool_msg_1 is dropped from telemetry entirely. This repo ships examples that hit exactly this shape: examples/agent/langgraph-traceloop/agent.py:180 (ToolNode) and the create_react_agent FSI examples, both used with SplunkAOCallback.

The ticket's own wording is "After dedup removes the input prefix, multiple messages remain" — so gate on the dedup actually having matched. That preserves the fix for the reported case and leaves genuine multi-message node updates intact.

One caveat to confirm: if a graph ever prepends or trims messages so the input is no longer a prefix, the dedup won't match and the root span won't be reduced. If that is a real shape for your app, prefer reducing at the handler layer for the root agent node instead (mirroring middleware.py:199-202).

Suggested change
if full_history and input_messages is not None and output_messages[: len(input_messages)] == input_messages:
output_messages = output_messages[len(input_messages) :]
# Full history includes all messages in the run, not just the final response.
# Keep only the last message — it is always the agent's final output.
if full_history and len(output_messages) > 1:
output_messages = [output_messages[-1]]
history_stripped = False
if full_history and input_messages is not None and output_messages[: len(input_messages)] == input_messages:
output_messages = output_messages[len(input_messages) :]
history_stripped = True
# The stripped remainder is the run's intermediate steps plus the final response. Child
# LLM/tool spans already carry the intermediates, so surface only the final message here.
if history_stripped and len(output_messages) > 1:
output_messages = [output_messages[-1]]

🤖 Generated by the Astra agent

Comment on lines +450 to +451
# Full history includes all messages in the run, not just the final response.
# Keep only the last message — it is always the agent's final output.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🟡 minor (bug): "it is always the agent's final output" is not true, and the comment will mislead whoever touches this next.

The last message is only the final answer when the run terminated normally on an assistant text message. It is not when the run ends on a tool-call AIMessage — e.g. interrupt_before=["tools"], a hit recursion limit, or an aborted run. In that case content is "" and _mapped_message (line 190) produces parts == [tool_call_part] with no text part, so the Output column still renders and the preceding assistant text that was present is now discarded as well. The bug this PR sets out to fix therefore survives for those runs.

At minimum, soften the comment to describe the heuristic rather than assert an invariant. If you want the interrupted-run case actually covered, prefer the last message that yields a non-empty text part, falling back to the last message — and add a test for it.

Suggested change
# Full history includes all messages in the run, not just the final response.
# Keep only the last message — it is always the agent's final output.
# Full history includes every message in the run, not just the final response. Child spans
# already carry the intermediate steps, so surface only the last message here. Note this is a
# heuristic: a run that ends on a tool-call message has no final text to show.

🤖 Generated by the Astra agent

Comment on lines +489 to +502
def test_orchestration_non_full_history_output_not_reduced() -> None:
# Plain string output (full_history=False) — the last-message reduction must NOT fire.
span = AgentSpan(
name="Agent",
agent_type=AgentType.default,
input="What is Lisinopril?",
output="Lisinopril is a blood pressure medication.",
)

attrs = build_span_attributes(span)

output_messages = json.loads(attrs["gen_ai.output.messages"])
assert len(output_messages) == 1
assert output_messages[0]["parts"][0]["content"] == "Lisinopril is a blood pressure medication."

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🟠 major (testing): This test is vacuous — it passes identically with or without the change, so it does not guard the full_history gate it claims to.

Tracing a plain string through _message_container: _parse_json_string fails and returns the string, _mapping_view returns None, and the Sequence branch at attribute_mapping.py:234 explicitly excludes str. So it returns (value, False, False) and _message_sequence yields exactly one message. Both full_history and len(output_messages) > 1 are independently false, so the new branch could never fire for this input. The len == 1 assertion is satisfied by the input shape, not by the guard.

To actually exercise the guard, use an input where the reduction would fire if the gate were dropped — a bare JSON list of messages, which _message_container:234-236 classifies as full_history=False while still producing multiple messages:

Suggested change
def test_orchestration_non_full_history_output_not_reduced() -> None:
# Plain string output (full_history=False) — the last-message reduction must NOT fire.
span = AgentSpan(
name="Agent",
agent_type=AgentType.default,
input="What is Lisinopril?",
output="Lisinopril is a blood pressure medication.",
)
attrs = build_span_attributes(span)
output_messages = json.loads(attrs["gen_ai.output.messages"])
assert len(output_messages) == 1
assert output_messages[0]["parts"][0]["content"] == "Lisinopril is a blood pressure medication."
def test_orchestration_bare_message_list_output_not_reduced() -> None:
# Given: output is a bare message list (full_history=False), not a {"messages": [...]} container.
# When: attributes are built.
# Then: every message survives — the last-message reduction must not fire.
messages = [
{"role": "assistant", "content": "First"},
{"role": "assistant", "content": "Second"},
]
span = AgentSpan(
name="Agent",
agent_type=AgentType.default,
input="What is Lisinopril?",
output=json.dumps(messages),
)
attrs = build_span_attributes(span)
output_messages = json.loads(attrs["gen_ai.output.messages"])
assert len(output_messages) == 2
assert output_messages[0]["parts"][0]["content"] == "First"
assert output_messages[1]["parts"][0]["content"] == "Second"

🤖 Generated by the Astra agent

Comment on lines +421 to +486
def test_orchestration_full_history_with_tool_call_keeps_last_message() -> None:
# LangGraph accumulated state: user → tool-call AI (empty content) → tool response → final AI
# The first post-dedup message has empty content; the UI would show "—" without the fix.
user = {"role": "user", "content": "What is the dosage of Lisinopril?"}
ai_toolcall = {"role": "assistant", "content": "", "tool_calls": [{"id": "tc1", "function": {"name": "rag_search", "arguments": '{"query":"Lisinopril dosage"}'}}]}
tool_resp = {"role": "tool", "content": "Lisinopril: 10mg daily", "tool_call_id": "tc1"}
ai_final = {"role": "assistant", "content": "Common dosage is 10mg once daily."}

span = AgentSpan(
name="Agent",
agent_type=AgentType.default,
input=json.dumps({"messages": [user]}),
output=json.dumps({"messages": [user, ai_toolcall, tool_resp, ai_final]}),
)

attrs = build_span_attributes(span)

output_messages = json.loads(attrs["gen_ai.output.messages"])
assert len(output_messages) == 1
assert output_messages[0]["role"] == "assistant"
assert output_messages[0]["parts"][0]["content"] == "Common dosage is 10mg once daily."


def test_orchestration_full_history_multi_turn_keeps_last_message() -> None:
# Multi-turn: output contains the full conversation history after multiple exchanges.
# Only the last message should be kept regardless of role.
user1 = {"role": "user", "content": "Hello"}
ai1 = {"role": "assistant", "content": "Hi, how can I help?"}
user2 = {"role": "user", "content": "What is Lisinopril?"}
ai2 = {"role": "assistant", "content": "Lisinopril is a blood pressure medication."}

span = AgentSpan(
name="Agent",
agent_type=AgentType.default,
input=json.dumps({"messages": [user1]}),
output=json.dumps({"messages": [user1, ai1, user2, ai2]}),
)

attrs = build_span_attributes(span)

output_messages = json.loads(attrs["gen_ai.output.messages"])
assert len(output_messages) == 1
assert output_messages[0]["parts"][0]["content"] == "Lisinopril is a blood pressure medication."


def test_orchestration_full_history_multiple_tool_rounds_keeps_last_message() -> None:
# Two tool call rounds before the final answer — last message is still the only output.
user = {"role": "user", "content": "Compare Lisinopril and Amlodipine"}
tc1_ai = {"role": "assistant", "content": "", "tool_calls": [{"id": "tc1", "function": {"name": "search", "arguments": '{"query":"Lisinopril"}'}}]}
tc1_resp = {"role": "tool", "content": "Lisinopril: ACE inhibitor", "tool_call_id": "tc1"}
tc2_ai = {"role": "assistant", "content": "", "tool_calls": [{"id": "tc2", "function": {"name": "search", "arguments": '{"query":"Amlodipine"}'}}]}
tc2_resp = {"role": "tool", "content": "Amlodipine: calcium channel blocker", "tool_call_id": "tc2"}
ai_final = {"role": "assistant", "content": "Lisinopril is an ACE inhibitor; Amlodipine is a calcium channel blocker."}

span = AgentSpan(
name="Agent",
agent_type=AgentType.default,
input=json.dumps({"messages": [user]}),
output=json.dumps({"messages": [user, tc1_ai, tc1_resp, tc2_ai, tc2_resp, ai_final]}),
)

attrs = build_span_attributes(span)

output_messages = json.loads(attrs["gen_ai.output.messages"])
assert len(output_messages) == 1
assert "Amlodipine" in output_messages[0]["parts"][0]["content"]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🟠 major (testing): Missing coverage for the cases where this change is most likely to be wrong. All three new positive tests are AgentSpan with a clean user → tool-call → tool → final-AI history, which is the one shape known to work. Please add:

  1. WorkflowSpan with a multi-message node update that is not accumulated history — e.g. output=json.dumps({"messages": [tool_msg_1, tool_msg_2]}) with an unrelated input state. This is the parallel-tool-call ToolNode shape and currently loses tool_msg_1; it is the regression flagged on attribute_mapping.py:448.
  2. Last message is a tool-call assistant with content="" (interrupted / recursion-limited run) — asserts what the exported output looks like when the heuristic does not hold.
  3. Last message has role: "tool" — the multi-turn test comments "regardless of role" but never actually exercises a non-assistant final message.

Also note test_orchestration_full_history_multiple_tool_rounds_keeps_last_message (line 467) is nearly a duplicate of the first test — it adds a second tool round but no new branch. Consider folding the two into a pytest.mark.parametrize and spending the saved test on case 1 above, which is the one that actually catches a defect.

Separately: test_orchestration_output_omits_repeated_input_history (line 403) is now non-discriminating for the prefix-dedup on lines 448-449 — the reduction alone produces the same single assistant message, so deleting the dedup would not fail it. Gating the reduction on the dedup (see the line 448 comment) restores that test's value.

🤖 Generated by the Astra agent

# LangGraph accumulated state: user → tool-call AI (empty content) → tool response → final AI
# The first post-dedup message has empty content; the UI would show "—" without the fix.
user = {"role": "user", "content": "What is the dosage of Lisinopril?"}
ai_toolcall = {"role": "assistant", "content": "", "tool_calls": [{"id": "tc1", "function": {"name": "rag_search", "arguments": '{"query":"Lisinopril dosage"}'}}]}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔵 nit (other): ruff format --check reports this file would be reformatted — lines 425, 469, 471, and 473 exceed the 120-char limit. .pre-commit-config.yaml runs the ruff-format hook, so this will fail pre-commit. Running ruff format tests/test_attribute_mapping.py splits these four dict literals across lines.

🤖 Generated by the Astra agent

Sign up for free to 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.

2 participants