fix(proxy/memory): harden _extract_tool_calls against malformed choices and blocks#2483
Conversation
…es and blocks
The OpenAI branch did choices[0].get("message", {}) after only a truthiness
check on choices, so a choices: [null] (which OpenAI-compatible gateways send
on a content-filtered / usage-only response) made choices[0].get raise
AttributeError. This runs from has_memory_tool_calls, which is called outside
the try that wraps the rest of memory handling, so the error reached the
request handler. Guard the first choice and its message for dict-ness, the
same way the sibling CCRResponseHandler._extract_assistant_message already
does. Also skip non-dict blocks in the Anthropic branch so a null content
block from a reconstructed response can't crash the block.get("type") filter.
PR governanceThis PR follows the template and is marked ready for human review. |
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
JerrettDavis
left a comment
There was a problem hiding this comment.
Thanks for hardening the outer choices and Anthropic block shapes. There is one remaining malformed upstream shape in the OpenAI branch that still crashes the same detection path:
response = {"choices": [{"message": {"tool_calls": [None]}}]}
handler._extract_tool_calls(response, "openai") # returns [None]
handler.has_memory_tool_calls(response, "openai") # AttributeError: 'NoneType' object has no attribute 'get'I verified that locally on the latest branch. _extract_tool_calls now guards choices[0] and message, but it still returns every element from message.tool_calls without checking the element type. Since both has_memory_tool_calls and handle_memory_tool_calls immediately call .get on each returned entry, a null or string inside the tool-call list still escapes and can 500 before the memory handling try block.
Please filter OpenAI Chat Completions tool_calls to dict entries before returning them, and add a regression for tool_calls: [None, {valid memory_save call}] that proves the malformed element is skipped while the valid call is still detected.
Other checked state: PR is mergeable and visible CI is green. I did not run the full test file after finding this blocker; the reproduction above is from the checked-out latest source.
Description
MemoryHandler._extract_tool_callsreads tool calls from an upstream response. The OpenAI Chat Completions branch only truthiness-checkedchoicesbefore indexing:An OpenAI-compatible gateway can send
choices: [null]on a content-filtered or usage-only response (the same shape the siblingCCRResponseHandler._extract_assistant_messagewas explicitly hardened for, see its comment: "a present-but-emptychoices: [](or[null]) which OpenAI-compatible gateways can send"). With[null],choicesis truthy, sochoices[0].get("message", {})runs onNoneand raisesAttributeError. A non-dict first element (choices: ["..."]) breaks the same way.This is not caught locally:
_extract_tool_callsruns fromhas_memory_tool_calls, which the OpenAI handler calls in theif (...)condition before thetrythat wraps the rest of memory handling, so theAttributeErrorsurfaces to the request handler rather than being swallowed and logged.The Anthropic branch had a smaller version of the same gap:
[block for block in content if block.get("type") == "tool_use"]guardscontentfor list-ness but not each block, so a null element in a reconstructed response crashesblock.get.Fix
Guard the first choice and its
messagefor dict-ness before calling.get, mirroring_extract_assistant_message. A malformed choice now yields no tool calls and correctly falls through to the Responses-APIoutput[]check. Skip non-dict blocks in the Anthropic branch. Well-formed responses are unchanged.Type of Change
Changes Made
headroom/proxy/memory_handler.py: dict-guard the first choice and itsmessagein the OpenAI branch of_extract_tool_calls; skip non-dict content blocks in the Anthropic branch.tests/test_memory_handler_null_function.py: regressions forchoices: [null]/ non-dict choice / null message, fall-through to a Responsesoutput[]function_call, and a non-dict Anthropic content block.Testing
pytest)ruff check .)mypy headroom)Test Output
Real Behavior Proof
uv sync --extra proxy),uvx ruff@0.15.17/uvx mypy@1.20.2, pytest in the venv._extract_tool_calls/has_memory_tool_callson a bareMemoryHandler(viaobject.__new__) withchoices: [null],choices: ["x"],choices: [{"message": null}], a[null]-choices response that also carries a Responsesoutput[]function_call, and an Anthropic response whose content list starts with a null block; then revertedmemory_handler.pyand re-ran.[]/False, theoutput[]fall-through is still detected asTrue, and the valid Anthropic tool_use block is returned; with the fix reverted thechoices: [null]and non-dict-block cases raiseAttributeError: 'NoneType' object has no attribute 'get'. Ran against the actual module viatests/test_memory_handler_null_function.py.choices: [null]on a memory-enabled request, end to end.Review Readiness
Checklist