Add Responses translation semantics#703
Conversation
praxis-bot
left a comment
There was a problem hiding this comment.
Review: Add Responses translation semantics
Extends the Chat Completions translator with response/stream conversion and schema-complete resource building. The request-side additions (logprobs, text format, stream options, function-call history, strict schemas) are well structured and tested. The response and stream translators are substantial and mostly correct.
| Severity | Count |
|---|---|
| Medium | 3 |
Non-inline findings
None -- all findings are inline.
| let name = obj.get("name").and_then(Value::as_str).unwrap_or(""); | ||
| let arguments = obj.get("arguments").and_then(Value::as_str).unwrap_or("{}"); | ||
|
|
||
| messages.push(json!({ |
There was a problem hiding this comment.
[Medium] append_function_call creates a separate assistant message per function call item. When the Responses input contains consecutive function_call items (parallel tool calls), the Chat Completions API expects them batched into a single assistant message's tool_calls array. Separate messages will cause provider errors because the intervening tool response messages won't have a matching preceding tool_calls entry.
Merge consecutive function_call items into one assistant message, or accumulate all tool calls and emit a single message at the end of the function-call sequence.
| "url": url | ||
| } | ||
| }) | ||
| if let Some(url) = part.get("image_url").or_else(|| part.get("url")) { |
There was a problem hiding this comment.
[Medium] The base version fell back to part.get("file_id") for input images, which matches the Responses API input_image schema. This PR changes the fallback to part.get("url"), which is not a documented input_image field. If a caller sends {"type": "input_image", "file_id": "file-abc"}, it will now fall through to the else branch and clone the raw part instead of extracting the file reference.
Restore file_id as an additional fallback: .or_else(|| part.get("file_id")).
| state.finish_reason = Some(finish_reason.to_owned()); | ||
| } | ||
|
|
||
| let content = choice |
There was a problem hiding this comment.
[Medium] process_stream_choice only extracts delta.content (text). Tool-call streaming chunks carry deltas in delta.tool_calls[].function.arguments and delta.tool_calls[].function.name, which are silently dropped here. A streaming request that triggers function calls will produce a terminal response.completed event with an empty output array and no function call items.
If tool-call streaming is out of scope for this PR, add a // TODO or tracking issue and document the limitation. Otherwise, accumulate delta.tool_calls fragments and emit function_call output items in finish_stream.
77e19aa to
6568916
Compare
06ef27b to
9c51857
Compare
Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>
6568916 to
4eb496c
Compare
9c51857 to
83442d7
Compare
Part of #213.
Summary
Stack
Validation