fix: Ensure that tool calls with no arguments get handled correctly

[jaideepr97]

What does this PR do?

When a model decides to use an MCP tool call that requires no arguments, it sets the arguments field to None. This causes the user to see a 400 bad requst error due to validation errors down the stack because this field gets removed when being parsed by an openai compatible inference provider like vLLM
This PR ensures that, as soon as the tool call args are accumulated while streaming, we check to ensure no tool call function arguments are set to None - if they are we replace them with "{}"

Closes #3456

Test Plan

Added new unit test to verify that any tool calls with function arguments set to None get handled correctly

[cdoern]

looks good, but do we perhaps want an integration test for this rather than unit?

[jaideepr97]

looks good, but do we perhaps want an integration test for this rather than unit?

added an integration test, lmk if it looks good
also updated the unit test function to better reflect the case we are testing here

[bbrowning]

Note that an inference server passing None as the arguments here is technically a bug in that inference server - see the Chat Completions API spec at https://github.com/openai/openai-openapi/blob/498c71ddf6f1c45b983f972ccabca795da211a3e/openapi.yaml#L17760-L17776 . So, we should open a bug in whichever inference servers are doing this to get that fixed.

With that said, I'm all for making this work as opposed to expecting every inference server to strictly adhere to the spec. So, I'll kick off the workflows here and let CI chew on this.

[cdoern]

@jaideepr97 I think you'll need to re-record the integration tests locally

[jaideepr97]

Note that an inference server passing None as the arguments here is technically a bug in that inference server - see the Chat Completions API spec at https://github.com/openai/openai-openapi/blob/498c71ddf6f1c45b983f972ccabca795da211a3e/openapi.yaml#L17760-L17776 . So, we should open a bug in whichever inference servers are doing this to get that fixed.

With that said, I'm all for making this work as opposed to expecting every inference server to strictly adhere to the spec. So, I'll kick off the workflows here and let CI chew on this.

+1 makes sense @bbrowning
Its interesting that vLLM (in this case) enforces conformance with the openai spec for input messages but apparently doesn't do it for messages generated by it

[iamemilio]

It looks like the completions spec expects the client to handle validating the JSON for tool calling arguments https://github.com/openai/openai-python/blob/a1493f92a7cd4399d57046aadc943aeadda5b8e7/src/openai/types/chat/chat_completion_message_function_tool_call_param.py#L10-L17

That said, it actually looks like vLLM does sometimes JSON dump the model output, but not consistently. Is it a bug or a feature? 🐞 🚀 ❓

[jaideepr97]

@cdoern @bbrowning on further thought, this change is only affecting how llama stack responds when it encounters a None from an external inference provider by ensuring it doesn't return something that violates the openai spec. As such, I don't think there is a need for an integration test because inference providers can already handle the corrected format of the input
there is only a need to ensure llama stack is internally behaving as expected, which is covered by the unit tests
thoughts?

[cdoern]

@cdoern @bbrowning on further thought, this change is only affecting how llama stack responds when it encounters a None from an external inference provider by ensuring it doesn't return something that violates the openai spec. As such, I don't think there is a need for an integration test because inference providers can already handle the corrected format of the input there is only a need to ensure llama stack is internally behaving as expected, which is covered by the unit tests thoughts?

seems fair enough to me. I also don't know if what you are hitting in the tests is necessarily your fault 😅 so I don't think it makes sense to have you fix it.

[bbrowning]

The 500 error in the integration tests, from the server logs:

ERROR    2025-09-30 12:17:30,901 __main__:263 core::server: Error executing endpoint route='/v1/responses' method='post': No recorded response found  
         for request hash: 5c8d7ada4919f36864e427d2e4909cf1de103af6c9f60419ad067fa7b275d233                                                           
         Request: POST http://0.0.0.0:11434/v1/v1/chat/completions {'model': 'llama3.2:3b-instruct-fp16', 'messages': [{'role': 'user', 'content':    
         "what's the current time? You MUST call the `get_current_time` function to find out."}], 'response_format': {'type': 'text'}, 'stream': True,
         'tools': [{'type': 'function', 'function': {'type': 'function', 'name': 'get_current_time', 'description': 'Get the current time',           
         'parameters': {}, 'strict': None}}]}                                                                                                         
         Model: llama3.2:3b-instruct-fp16                                                                                                             
         To record this response, run with LLAMA_STACK_TEST_INFERENCE_MODE=record                                                                     
INFO     2025-09-30 12:17:30,927 uvicorn.access:476 uncategorized: ::1:57458 - "POST /v1/responses HTTP/1.1" 500                                      

The new test will need a recorded response before it can pass.

[jaideepr97]

@bbrowning Looks like some recordings got added and updated after I ran ./scripts/github/schedule-record-workflow.sh --subdirs "agents" --setup ollama locally
could you retrigger the CI again? or I suppose this means CI has passed?

[cdoern]

lots of re-recordings, but I think that is ok?

[bbrowning]

The actual changes look good to me, and the added tests match the changes. I'm assuming all the recording stuff is fine, since CI is happy with it.

Thanks!