feat: pluggable tool-call adapter + OpenAI tool support (closes #95, #96, #97) - #98
Conversation
, #97) Introduces IToolCallAdapter (Core) and ToolCallAdapterRegistry keyed on GGUF architecture. Ships adapters for Qwen2/3 (<tool_call> wrapper), Qwen3-Coder (bare <function=...>), Llama-3 (<|python_tag|>...<|eom_id|>), and DeepSeek-R1 (<|tool_calls_begin|>...<|tool_calls_end|>). ChatTemplateRenderer resolves the adapter once at engine-load time. AnthropicEndpoints: replaces the hard-coded <tool_call> matcher in HandleStreaming + non-streaming ParseToolCalls with the adapter's open/close marker API. tool_result history-replay now goes through adapter.RenderToolResult so families like Llama-3 (role: ipython) replay correctly. OpenAiEndpoints: adds Tools / ToolChoice on ChatCompletionRequest, tool_calls on OaiAssistantMessage + ChunkDelta, OaiToolCall/OaiToolCallDelta records, history role=tool messages, finish_reason=tool_calls. Streaming buffering activates only when tools are in the request so the no-tools per-chunk cadence is preserved. Tests: 21 adapter unit tests (Qwen, Qwen3-Coder, Llama, DeepSeek + registry), 7 endpoint tests covering Qwen3-Coder /v1/messages non-stream + stream (closes #95), OpenAI /v1/chat/completions non-stream + stream + tool history + no-tools regression guard. Full suite green: 505/505. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces a translation layer for LLM tool calls by implementing IToolCallAdapter and concrete adapters for Qwen, Llama, and DeepSeek model families, integrating them into the Anthropic and OpenAI endpoints. The review feedback identifies critical robustness issues, including potential ArgumentOutOfRangeException and NullReferenceException crashes when handling malformed model outputs or request payloads, as well as a memory leak from an undisposed JsonDocument in ParseJsonCallBlock.
| int nameEnd = block.IndexOf('>', funcStart); | ||
| if (nameEnd < 0) return null; | ||
| string funcName = block[(funcStart + funcTag.Length)..nameEnd].Trim(); | ||
| if (funcName.Length == 0) return null; |
There was a problem hiding this comment.
If the model output is malformed and does not contain a valid function name (e.g., <function=>), nameEnd can be less than or equal to funcStart + funcTag.Length. This causes an ArgumentOutOfRangeException when slicing the string. Adding a guard prevents potential server crashes on untrusted model outputs.
int nameEnd = block.IndexOf('>', funcStart);
if (nameEnd < 0 || nameEnd <= funcStart + funcTag.Length) return null;
string funcName = block[(funcStart + funcTag.Length)..nameEnd].Trim();| int paramNameEnd = funcBody.IndexOf('>', paramStart); | ||
| if (paramNameEnd < 0) break; | ||
| string paramName = funcBody[(paramStart + paramTag.Length)..paramNameEnd].Trim(); | ||
|
|
There was a problem hiding this comment.
Similarly to the function name parsing, if a parameter tag is malformed (e.g., <parameter=>), paramNameEnd can be less than or equal to paramStart + paramTag.Length. This leads to an ArgumentOutOfRangeException during slicing. A guard is needed here to safely handle malformed parameters.
int paramNameEnd = funcBody.IndexOf('>', paramStart);
if (paramNameEnd < 0 || paramNameEnd <= paramStart + paramTag.Length) break;
string paramName = funcBody[(paramStart + paramTag.Length)..paramNameEnd].Trim();| ["function"] = new Dictionary<string, object?>(StringComparer.Ordinal) | ||
| { | ||
| ["name"] = tc.Function.Name, | ||
| // OpenAI stringifies arguments; pass through as-is. | ||
| ["arguments"] = tc.Function.Arguments, | ||
| }, |
There was a problem hiding this comment.
If a client sends a history message with a tool_calls array where the function property is missing or null, accessing tc.Function.Name or tc.Function.Arguments directly will throw a NullReferenceException. Using null-conditional operators prevents server crashes on malformed request payloads.
["function"] = new Dictionary<string, object?>(StringComparer.Ordinal)
{
["name"] = tc.Function?.Name,
// OpenAI stringifies arguments; pass through as-is.
["arguments"] = tc.Function?.Arguments,
},| ["function"] = new Dictionary<string, object?>(StringComparer.Ordinal) | ||
| { | ||
| ["name"] = t.Function.Name, | ||
| ["description"] = t.Function.Description, | ||
| ["parameters"] = t.Function.Parameters, | ||
| }, |
There was a problem hiding this comment.
If an incoming tool definition in the request payload has a null or missing function object (e.g., {"type": "function"}), accessing t.Function.Name will result in a NullReferenceException. Null-conditional operators should be used to handle such malformed inputs gracefully.
["function"] = new Dictionary<string, object?>(StringComparer.Ordinal)
{
["name"] = t.Function?.Name,
["description"] = t.Function?.Description,
["parameters"] = t.Function?.Parameters,
},| object? parsed = argsElem.ValueKind == JsonValueKind.String | ||
| ? JsonElementToObject(JsonDocument.Parse(argsElem.GetString() ?? "{}").RootElement) | ||
| : JsonElementToObject(argsElem); |
There was a problem hiding this comment.
JsonDocument.Parse allocates unmanaged resources and implements IDisposable. Failing to dispose of the parsed document results in memory leaks, which can degrade performance over time in high-throughput environments. Wrapping the parsing in a using block resolves this leak.
object? parsed = null;
if (argsElem.ValueKind == JsonValueKind.String)
{
using var innerDoc = JsonDocument.Parse(argsElem.GetString() ?? "{}");
parsed = JsonElementToObject(innerDoc.RootElement);
}
else
{
parsed = JsonElementToObject(argsElem);
}…ol-call surfacing
- ToolCallAdapterRegistry: ConcurrentDictionary instead of Dictionary (was racy
between request-time Get and startup-time Register).
- ParseJsonCallBlock: dispose the inner JsonDocument when arguments is a JSON
string (leak under high QPS).
- ParseXmlFunctionBlock: skip <parameter=> tags with empty names instead of
inserting an empty-string dict key.
- OaiToolCall.Function and OaiTool.Function are now nullable to match runtime
reality of source-gen JSON; rich-message builder uses ?. so malformed history
/ tool defs no longer NRE the request.
- Anthropic + OpenAI streaming: when the stream ends mid-tool-call, flush the
buffered bytes as a text content delta and signal truncation via
finish_reason=length (OpenAI) / stop_reason=max_tokens (Anthropic) instead
of silently dropping the partial call.
- Request-deserialization catches narrowed to JsonException +
BadHttpRequestException; OperationCanceledException now propagates. Error
body now includes the parse error message instead of being empty.
- Streaming finally-block catches narrowed to OperationCanceledException +
IOException; non-aborted errors propagate instead of being silently
swallowed.
- AnthropicEndpoints non-streaming tool input fallback narrowed to
JsonException, and reuses the existing EmptyJsonObject instead of
parsing "{}" per call.
Tests:
- New OpenAi_WithTools_Streaming_SplitOpenMarkerAcrossChunks regression guard
for the maxOpenLen buffering — delivers <tool_ / call> across two engine
chunks.
- Anthropic streaming test now parses the message_delta event and asserts
stop_reason==tool_use explicitly rather than substring-matching.
Verified end-to-end against a running Qwen3-8B server: malformed JSON now
returns a 400 with the parse error in the body, null function on tool defs
no longer NREs, OpenAI tool-history echo path produces the expected reply,
Anthropic SSE shape unchanged for the non-tool case.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Summary
Completes the tool-calling feature across both HTTP wire formats by introducing a per-architecture translation layer between the model's free-text tool-call output and the structured
tool_use/tool_callsshapes the endpoints emit.IToolCallAdapter+ToolCallAdapterRegistryinSharpInference.Corekeyed on the GGUFgeneral.architecturefield. Adapter ships for Qwen2/3 (<tool_call>wrapper, JSON or XML inside), Qwen3-Coder (bare<function=name>...</function>, closes Tool-call parser misses Qwen3-Coder's bare <function=...> format (no <tool_call> wrapper) #95), Llama-3 (<|python_tag|>{json}<|eom_id|>), and DeepSeek-R1 multi-call envelope./v1/messagesstreaming + non-streaming now drive through the adapter. Streaming open/close-marker scan replaces the hard-coded<tool_call>matcher; the buffer guard becomesadapter.MaxOpenTagLength - 1.tool_resulthistory-replay goes throughadapter.RenderToolResultso families that need a non-toolrole (e.g. Llama-3ipython) replay correctly./v1/chat/completionsgains full tool support (OpenAI /v1/chat/completions: tool-call wire-format support (parity with /v1/messages) #97):tools/tool_choicerequest fields,tool_callsarray on the assistant message,tool_callsstreaming deltas,finish_reason: "tool_calls", history-siderole: "tool"echoes viatool_call_id. The buffering state machine activates only when tools are present so the existing no-tools per-chunk streaming cadence is preserved.Test plan
SharpInference.Tests.Core(registry + Qwen, Qwen3-Coder, Llama, DeepSeek wire formats).SharpInference.Tests.Server: Qwen3-Coder/v1/messagesnon-stream + stream (closes Tool-call parser misses Qwen3-Coder's bare <function=...> format (no <tool_call> wrapper) #95), OpenAI tool roundtrip non-stream + stream, OpenAI tool history echo, OpenAI no-tools per-chunk cadence regression guard.Closes #95, #96, #97.
🤖 Generated with Claude Code