Repository navigation
fix(handlers): SAO-18193 Responses API and multimodal messages - #272
Conversation
…AI Agents (SAO-18193) The Responses API path (the Agents SDK default) recorded the whole input item list as one JSON user message, the output as a JSON blob, and workflow/agent spans as "Workflow Step" with raw output_text parts. Normalize Responses input items into chat messages (tool calls, tool results, developer -> system, role-less items skipped) and output items into one assistant message, so the workflow input/output fallback works on this path too. Convert text, image, and file parts on both paths into ingest content blocks instead of dumping the message as JSON, and build LoggedLlmSpan on the ingestion-hook path so the blocks survive. Streamed Chat Completions runs record a Responses-shaped response object as the generation output; normalize it the same way instead of dumping it. Co-Authored-By: Claude Code <noreply@anthropic.com>
…ses path (SAO-18193) Streamed Chat Completions whose output is a reasoning-only or empty Responses object fell through to None instead of the same serialize_to_str(None) fallback the Responses branch uses; on the ingestion-hook path that None reached LoggedLlmSpan(output=...), which requires a LoggedMessage and rejected it, failing the whole trace. The _log_node_tree llm branch had the same gap for an explicit None stored under node.span_params["output"]. A function_call_output/custom_tool_call_output whose output is a list of parts (e.g. text + an image, as the Agents SDK produces for multimodal tool results) was JSON-dumped as text instead of converted into content blocks, same as other multimodal content on this path. Co-Authored-By: Claude Code <noreply@anthropic.com>
etserend
left a comment
There was a problem hiding this comment.
🤖 This review was generated by the Astra agent (claude-opus-5-5). It may contain mistakes.
Verdict: approve — The Responses, multimodal, and streamed paths work as described. One gap remains for input_file parts that carry a URL or raw base64, and some tests are missing.
General Comments
- 🟡 minor (testing): The new tests do not cover these paths:
- A Responses API LLM span on the ingestion-hook path.
LoggedLlmSpannow receives tool messages andtool_callsdicts there, and only Chat Completions input is tested on that path. - An
input_filepart on either path, withfile_dataor withfile_url. - End-to-end export of a
toolmessage whose content is a list of content blocks. Only the util-level normalizer is tested for this case.
Add one focused test for each path. Theinput_filegap in the line comment shows that this path needs a test.
Follow-ups
Suggested follow-up work that could be tracked as Jira tickets:
src/splunk_ao/utils/openai_agents.py:94-100: An assistant text message and the function calls after it are one model turn, but they become two assistant messages in the input._normalize_responses_outputmerges them into one message for the same turn. Consider merging function calls into a preceding assistant text message too, so input and output agree.src/splunk_ao/utils/openai_agents.py:423-433: A reasoning-only response, or an unset response, records the literal string "null" as the LLM output. The workflow can then inherit it as its output. An empty assistant message is probably a better fallback than "null".
opikalova
left a comment
There was a problem hiding this comment.
🤖 This review was generated by the Astra agent (claude-opus-5-5). It may contain mistakes.
Verdict: approve — The Responses, multimodal, and streamed paths are correct. The remaining issues are minor: None handling has gaps, and the open input_file thread is still valid.
General Comments
- 🟡 minor (documentation): Astra found no Jira ticket in the branch name, the title or the description.
Follow-ups
Suggested follow-up work that could be tracked as Jira tickets:
src/splunk_ao/utils/openai_agents.py:407-409: On the Responses path, the system prompt goes inresponse.instructions. It is not in the input items. The LLM span stores it only inmetadata["instructions"], but the Chat Completions path records it as asysteminput message. Record it as a leadingsystemmessage or asgen_ai.system_instructions, so that the two paths agree.
…d tool-output blocks (SAO-18193) Scope the output-None guard added for streamed/reasoning-only responses to the llm/chat branch only in _log_node_tree - the previous version defaulted every node type's output through `or ""`, which also converted other legitimately falsy-but-valid outputs (e.g. a retriever's output=[] for "no documents found") into an empty string instead of passing them through. Guard LLM span input=None the same way output=None was guarded: with RunConfig(trace_include_sensitive_data=False), the Agents SDK leaves both input and output as None, and the output-only fix still let a bare None input reach LoggedLlmSpan, crashing the whole trace on the ingestion-hook path. An empty list is the valid "no data" representation. Replace the serialize_to_str(None) fallback (-> the literal string "null") with "" on the streamed Chat Completions and Responses output branches - "null" is truthy, so it defeated every downstream workflow/trace fallback and would show up as the visible answer text for a reasoning-only response. Fix input_file handling: a file_url part now becomes a document data block instead of a JSON-dumped text block; raw base64 file_data (no "data:" URI prefix) no longer has its payload dumped into the text fallback. Add tests for a Responses-path LLM span on the ingestion-hook path, both input_file forms, and an end-to-end export of a tool message with image content blocks (previously only unit-tested at the normalizer level). Co-Authored-By: Claude Code <noreply@anthropic.com>
etserend
left a comment
There was a problem hiding this comment.
🤖 This review was generated by the Astra agent (claude-opus-5-5). It may contain mistakes.
Verdict: approve — The Responses, multimodal and streamed paths work correctly. One minor gap remains: raw base64 file parts on the Chat Completions path still leak.
Follow-ups
Suggested follow-up work that could be tracked as Jira tickets:
src/splunk_ao/utils/openai_agents.py:61-65:_content_blockreturns None for a content part that is not astrordict, and_message_contentsilently drops it._as_dictconverts only top-level items. Apply_as_dictto each part too, so that a pydantic part from a custom model keeps its content.
| return {"type": "text", "text": serialize_to_str({k: v for k, v in part.items() if k != "file_data"})} | ||
| # Responses API carries the file fields on the part itself, not under "file". | ||
| part = {"type": "file", "file": part} | ||
| return _convert_langchain_content_block(part) |
There was a problem hiding this comment.
🟡 minor (bug): The raw-base64 guard applies to Responses input_file parts only. The Chat Completions path has the same gap.
The Agents SDK Chat Completions converter copies input_file.file_data unchanged into a {"type": "file", "file": {...}} part. That part reaches _convert_langchain_content_block directly. When file_data has no data: prefix, the converter returns json.dumps(block). That text contains the full base64 payload.
Failure scenario:
- A user sends
{"type": "input_file", "filename": "a.pdf", "file_data": "<raw b64>"}throughOpenAIChatCompletionsModel. - The LLM span input holds a text block with the whole file payload.
The ticket requires "no JSON dump" on both paths. Apply the same guard to kind == "file". Add a TestGenerationMultimodalInput case for a raw-base64 file part.
| return _convert_langchain_content_block(part) | |
| elif kind == "file": | |
| inner = part.get("file") | |
| file_data = inner.get("file_data") if isinstance(inner, dict) else None | |
| if isinstance(file_data, str) and file_data and not file_data.startswith("data:"): | |
| # Raw base64 without a data URI: do not dump the payload as text. | |
| meta = {k: v for k, v in inner.items() if k != "file_data"} | |
| return {"type": "text", "text": serialize_to_str({"type": "file", "file": meta})} | |
| return _convert_langchain_content_block(part) |
🤖 Generated by the Astra agent
opikalova
left a comment
There was a problem hiding this comment.
🤖 This review was generated by the Astra agent (claude-opus-5-5). It may contain mistakes.
Verdict: approve — The latest commit fixes the None, "null", and input_file findings correctly. Two minor issues remain: one test is missing, and the open Chat Completions raw-base64 thread is still valid.
General Comments
- 🟡 minor (documentation): Astra found no Jira ticket in the branch name, the title or the description.
Follow-ups
Suggested follow-up work that could be tracked as Jira tickets:
src/splunk_ao/utils/serialization.py:49-59:_convert_langchain_content_blockreturnsjson.dumps(block)for afilepart whosefile_datais not a data URI. That text holds the full raw base64 payload. The open thread onopenai_agents.py:58proposes a guard only in_content_block. Put the guard in the shared converter instead. Then the LangChain handler path and both OpenAI Agents paths drop the payload in the same way.src/splunk_ao/converter/attribute_mapping.py:194-198: For atoolmessage, the exporter puts the content intotool_call_response.responsewithout changes. A tool result that contains content blocks thus exports the SDK-internal shape{"type": "data", "base64": ...}. User messages get the OTelblob/uripart shape instead.test_responses_api_tool_output_with_image_exports_content_blocksasserts this internaldatatype. Consider mapping content-block lists through_content_part, so that tool results use the same wire shape as messages.
| # GenerationSpanData.input/output stay None when tracing.include_data() is false | ||
| # (e.g. RunConfig(trace_include_sensitive_data=False)); LoggedLlmSpan requires a message | ||
| # list, not None, so an empty list is the valid "no data" representation. | ||
| data["input"] = _normalize_chat_messages(data["input"]) if data["input"] is not None else [] |
There was a problem hiding this comment.
🟡 minor (testing): The commit message says that this line fixes a whole-trace crash on the ingestion-hook path when trace_include_sensitive_data=False. No test covers that case.
test_ingestion_hook_llm_span_with_no_output_does_not_crash_trace sets input=[...]. Thus it would still pass if this line and line 442 went back to passing None through.
Add one hook-path test for each span type, with no data set:
generation_span(model="gpt-4", parent=root_span)with no input and no output.response_span(parent=root_span)with no input and no response.
For both, assert that the trace commits, that llm_span.input == [], and that llm_span.output.content == "". Also assert that the workflow output is not "null". This locks in the change from serialize_to_str(None) to "".
🤖 Generated by the Astra agent
Summary
Follow-up to #259 (merged in 0.5.0). Fixes
SplunkAOTracingProcessor(OpenAI Agents SDK) on the paths #259 left unchanged:usermessage holding the whole item list as JSON, and output was one JSON blob. Input items now become chat messages in the order sent: tool calls as assistanttool_calls, tool results astoolmessages,developerassystem, and role-less items such as reasoning are skipped. Output items become one assistant message with text and tool calls."Workflow Step"as input and a raw{"type": "output_text", ...}part as output. They now show the latest user message and the answer text.LoggedLlmSpan, so the blocks survive there too.Runner.run_streamed). The SDK records a Responses-shaped response object as the generation output; it is now normalized like Responses output instead of being dumped as JSON.Not changed (by design)
file_id-only image or file has no data or URL to attach, so it stays a small text part.Test plan
poetry run pytest tests/test_openai_agents.py tests/test_openai_agents_utils.py -n 0— 72 passed, 1 skipped, 2 failed (both pre-existing network-dependent VCR tests, identical failure onmain, unrelated to this change)ruff checkandruff format --checkon changed files — cleanpoetry run invoke type-check— clean, no issues🤖 Generated with Claude Code