Tolerate malformed tool calls in Python SDK chat parsing (fixes #864)#872
Open
justinchuby wants to merge 2 commits into
Open
Tolerate malformed tool calls in Python SDK chat parsing (fixes #864)#872justinchuby wants to merge 2 commits into
justinchuby wants to merge 2 commits into
Conversation
Small models routinely emit an invalid extra tool call (e.g. missing `function.name` with `arguments == "null"`). `ChatClient.complete_chat` and the streaming path validated the raw response with an unguarded `ChatCompletion.model_validate_json` / `ChatCompletionChunk.model_validate`, so a single bad tool call aborted the whole application with an opaque pydantic `ValidationError` — even though the docstrings promise `FoundryLocalException` on failure. This adds two helpers, `_tool_call_is_valid` and `_drop_malformed_tool_calls`, that drop only the malformed tool-call entries (logging a warning for each) so the rest of an otherwise-usable response still parses. Valid `function` and `custom` tool calls are preserved. If validation still fails, the raw payload is now wrapped in a clear, catchable `FoundryLocalException` instead of surfacing an internal pydantic traceback. Applied to both the published SDK (`sdk/python`) and `sdk_v2`, covering the non-streaming and streaming paths, plus unit tests for the sanitizer. Fixes microsoft#864 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
@justinchuby is attempting to deploy a commit to the MSFT-AIP Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds tolerant malformed tool-call handling across both Python SDK generations.
Changes:
- Filters malformed function/custom tool calls.
- Wraps response validation failures in
FoundryLocalException. - Adds legacy SDK unit coverage.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
sdk/python/src/openai/chat_client.py |
Adds sanitization and wrapped parsing. |
sdk/python/test/openai/test_chat_client_tool_calls.py |
Tests sanitization helpers. |
sdk_v2/python/src/foundry_local_sdk/openai/chat_client.py |
Mirrors parsing changes in SDK v2. |
Comment on lines
+36
to
+39
| if tool_call.get("type") == "custom" or "custom" in tool_call: | ||
| return isinstance(tool_call.get("custom"), dict) | ||
| function = tool_call.get("function") | ||
| return isinstance(function, dict) and bool(function.get("name")) |
Comment on lines
+34
to
+37
| if tool_call.get("type") == "custom" or "custom" in tool_call: | ||
| return isinstance(tool_call.get("custom"), dict) | ||
| function = tool_call.get("function") | ||
| return isinstance(function, dict) and bool(function.get("name")) |
Comment on lines
+304
to
+308
| _drop_malformed_tool_calls(raw) | ||
| try: | ||
| chunk_queue.put(ChatCompletionChunk.model_validate(raw)) | ||
| except ValidationError as exc: | ||
| raise FoundryLocalException( |
Comment on lines
+348
to
+352
| _drop_malformed_tool_calls(raw) | ||
| try: | ||
| yield ChatCompletionChunk.model_validate(raw) | ||
| except ValidationError as exc: | ||
| raise FoundryLocalException( |
| f"Foundry Local returned a non-JSON chat completion response: {response_json!r}" | ||
| ) from exc | ||
|
|
||
| _drop_malformed_tool_calls(payload) |
…d, v2 tests - _tool_call_is_valid now dispatches on the explicit "type" discriminator instead of the presence of a "custom" key, so a valid function call carrying an extra "custom" key is no longer dropped; a missing/unknown discriminator is treated as malformed. - Streaming chunk handlers (both SDKs) now wrap JSON decoding, normalization and model validation in one try, so invalid JSON / non-object payloads / malformed shapes surface as a catchable FoundryLocalException instead of JSONDecodeError / AttributeError. - Add sdk_v2 unit tests mirroring the legacy suite plus discriminator cases, and extend the legacy suite with the same cases, so the two implementations cannot silently diverge. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #864.
When a loaded model emits a malformed
tool_call(e.g. a second call with a missingfunction.nameandarguments == "null"— exactly whatsmollm3-3bproduced), the Python SDK crashed with a raw pydanticValidationErrorinstead of theFoundryLocalExceptionits docstrings promise. A single bad tool call aborted the whole application with an internal traceback that gave no hint about the real cause (the model output).The unguarded validation existed in both the non-streaming (
ChatCompletion.model_validate_json) and streaming (ChatCompletionChunk.model_validate) paths.Changes
Adds two small, pure helpers used by both paths:
_tool_call_is_valid(tool_call)— afunctioncall must have a non-emptyfunction.name; acustomcall must have acustomobject._drop_malformed_tool_calls(payload)— removes only the malformed tool-call entries in place (logging a warning per drop), so the rest of an otherwise-usable response still parses. Handles bothmessage(non-streaming) anddelta(streaming) shapes.Validation is now wrapped so that, if parsing still fails, the raw payload is surfaced via a clear, catchable
FoundryLocalExceptioninstead of an internal pydantic traceback (as the docstrings already promise).Behavior for the reported case:
tool_callsis removed and the message still parses.functionandcustomtool calls are never touched.Applied to both
sdk/python(publishedfoundry-local-sdk, where the issue's traceback points) andsdk_v2(2.0.0.dev0), each covering the non-streaming and streaming paths.Tests
Adds
sdk/python/test/openai/test_chat_client_tool_calls.py— 8 unit tests (no native binary / loaded model required) covering_tool_call_is_valid, dropping only malformed entries, the all-malformed case, valid responses left untouched, and the streamingdeltashape. All pass locally.The exact model-free repro from the issue now parses cleanly and returns the single valid
get_weathertool call.