fix(converter): keep only last message from full-history agent output (HYBIM-988) - #227
fix(converter): keep only last message from full-history agent output (HYBIM-988)#227etserend wants to merge 2 commits into
Conversation
fercor-cisco
left a comment
There was a problem hiding this comment.
🤖 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:
- 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.
- 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.mdentry under[Unreleased]. This changes what the SDK puts on the wire ingen_ai.output.messages/splunk_ao.output.messages, which is user-visible behavior, andAGENTS.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_historyis only True for a top-level"messages"key (container is source, line 228). Accumulated history arriving via a nested"update"key — i.e. a LangGraphCommand, whichserialization.py:280-286does serialize as{"update": {...}}— or as a bare JSON list is classifiedfull_history=Falseand so is never deduped or reduced. Worth confirming whether that asymmetry is intentional; if a root agent output can ever arrive inCommandform, HYBIM-988 would still reproduce there.src/splunk_ao/handlers/langchain/middleware.py:196-218:after_agent/aafter_agentalready 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.
| 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]] |
There was a problem hiding this comment.
🟠 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).
| 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
| # 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. |
There was a problem hiding this comment.
🟡 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.
| # 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
| 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." |
There was a problem hiding this comment.
🟠 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:
| 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
| 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"] |
There was a problem hiding this comment.
🟠 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:
WorkflowSpanwith 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-callToolNodeshape and currently losestool_msg_1; it is the regression flagged onattribute_mapping.py:448.- 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. - 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"}'}}]} |
There was a problem hiding this comment.
🔵 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
Summary
—[ {"role": "assistant", "parts": [{"type": "tool_call", ...}]}, # content = "" {"role": "tool", "parts": [...]}, {"role": "assistant", "parts": [{"type": "text", "content": "Common dosage..."}]}, ]Test plan
test_attribute_mapping.pytests pass (41 tests)erden-framework-testing / healthcare-assistantstream —invoke_agent Agentoutput column now shows the response text instead of—Fixes https://splunk.atlassian.net/browse/HYBIM-988