Conversation
Greptile SummaryThis PR adds support for OpenAI's Responses API as an alternative backend for newer models ( Key changes:
Confidence Score: 2/5
|
| Filename | Overview |
|---|---|
| lib/console/ai/provider/openai.ex | Adds Responses API support for newer OpenAI models (gpt-5, codex, o3, o4) via new structs, routing logic, and request/response helpers. Has three issues: extract_responses_output can raise FunctionClauseError when output is nil, tool_call/4 doesn't route newer models through the Responses API (only completion/3 does), and unused variables n/args will produce compiler warnings. |
| lib/console/ai/stream/exec.ex | Adds handle_openai_responses/1 for streaming the Responses API SSE events; also adds an empty-string guard to handle_openai. Handler functions changed from defp to def to support direct unit testing, which exposes implementation details unnecessarily. |
| lib/console/ai/stream/result.ex | Adds Tool.update/2 and guards to Tool.args/2; refactors Result.tool/3 to use update for accumulating streaming chunks from both Chat Completions and Responses API formats. Changes are clean and correct. |
| test/console/ai/stream/exec_test.exs | New test file providing good unit coverage of all three handler functions. One test ("accumulates function call arguments across multiple deltas") is misleadingly named — it only verifies that each individual delta chunk returns the right value; actual accumulation is handled by Result.tool/3 elsewhere. |
Comments Outside Diff (1)
-
lib/console/ai/provider/openai.ex, line 204-215 (link)tool_call/4bypasses the Responses API routing for newer modelstool_callhardcodeschat/3(Chat Completions endpoint) for every model, includingo3,o4,gpt-5, andcodex— the same models thatcompletion/3routes through the Responses API. If those models have dropped support for the Chat Completions endpoint,tool_callwill receive an API error at runtime.The routing logic added to
completion/3(use_responses_api?/1) should also be applied insidetool_callso newer models useresponses/3withtool_choice: "required"andrequire_tools: true.
Last reviewed commit: 888448d
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| defp extract_responses_output(output) when is_list(output) do | ||
| {text_parts, tool_calls} = Enum.reduce(output, {[], []}, fn item, {texts, tools} -> | ||
| case item do | ||
| %ResponsesOutputItem{type: "message", content: content} when is_list(content) -> | ||
| text = Enum.map_join(content, "", fn | ||
| %ResponsesContentPart{type: "output_text", text: t} when is_binary(t) -> t | ||
| _ -> "" | ||
| end) | ||
| {[text | texts], tools} | ||
|
|
||
| %ResponsesOutputItem{type: "function_call", id: id, name: name, arguments: args} -> | ||
| tool = %Console.AI.Tool{id: id, name: name, arguments: safe_decode_args(args)} | ||
| {texts, [tool | tools]} | ||
|
|
||
| _ -> | ||
| {texts, tools} | ||
| end | ||
| end) | ||
|
|
||
| content = text_parts |> Enum.reverse() |> Enum.join("") | ||
|
|
||
| case Enum.reverse(tool_calls) do | ||
| [] -> {:ok, content} | ||
| tools -> {:ok, content, tools} | ||
| end | ||
| end |
There was a problem hiding this comment.
FunctionClauseError when output is nil
extract_responses_output/1 has an is_list(output) guard but ResponsesResponse.output defaults to nil (no default in the struct). When Poison.decode! deserializes a response with a missing output field, the struct will have output: nil. The pattern match in completion_via_responses will still bind output (it's not a guard), and then extract_responses_output(nil) will raise a FunctionClauseError because no clause matches a non-list value.
A defensive fallback clause should be added:
defp extract_responses_output(nil), do: {:error, "could not generate an ai completion for this context"}
defp extract_responses_output(output) when is_list(output) do
# existing body
end
lib/console/ai/provider/openai.ex
Outdated
| end | ||
|
|
||
| input = Enum.flat_map(other_msgs, fn | ||
| {:tool, msg, %{call_id: id, name: n, arguments: args}} when is_binary(id) -> |
There was a problem hiding this comment.
n and args are bound from the pattern match but never referenced in the function body. Elixir will emit variable "n" is unused and variable "args" is unused warnings at compile time. Prefix them with _ to silence the warnings:
| {:tool, msg, %{call_id: id, name: n, arguments: args}} when is_binary(id) -> | |
| {:tool, msg, %{call_id: id, name: _n, arguments: _args}} when is_binary(id) -> |
| def handle_openai(%{"choices" => [%{"delta" => %{"content" => c}} | _]}) when is_binary(c) and c != "", do: c | ||
| def handle_openai(%{"choices" => [%{"delta" => %{"tool_calls" => [_ | _] = calls}}]}) do | ||
| {:tools, Enum.map(calls, fn %{"index" => ind, "function" => call} = func -> | ||
| {ind, Map.put(call, "id", func["id"])} | ||
| end)} | ||
| end | ||
| defp handle_openai(_), do: :pass | ||
| def handle_openai(_), do: :pass |
There was a problem hiding this comment.
handle_openai/1 and handle_anthropic/1 were previously defp and are now promoted to def solely so that the unit tests in exec_test.exs can call them directly. The test file itself acknowledges this trade-off in a comment. Exposing internal handler functions as public API makes them part of the module's contract and harder to change later.
Consider using @doc false or testing these indirectly through the higher-level openai/2, anthropic/2, and openai_responses/2 functions instead, or extracting the handlers into a dedicated testable sub-module.
kinjalh
force-pushed
the
openai-responses
branch
from
888448d to
6522616
Compare
|
This actually shouldn't be needed with the ReqLLM pr, which is going a lot faster than I anticipated |
2 participants
Test Plan
Checklist
Plural Flow: console