Preserve provider error bodies on non-200 streaming responses - #3
Open
tarzan wants to merge 5 commits into
Open
Conversation
Both streaming Req collectors (DefactoAI.StreamRunner.run/2 and
DefactoAI.Client.LangChain.run_stream_request/6) parsed every body chunk
as SSE, keeping only `data:` lines. When the gateway answers a non-200
status the body is a plain JSON error document with no `data:` lines, so
the collector silently consumed it and callers received
`{:api_error, 422, ""}` with the provider's message lost.
When resp.status != 200, buffer the raw chunk bytes on a private
`:error_body` key instead of SSE-parsing them, and report that text on
the non-200 branch (falling back to the existing body_to_text/1 when
nothing was buffered). The 200 path is unchanged.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
After a validation failure the repair loop re-runs the chain with the model's tool-call assistant turn in the history. That turn has blank content (nil, "" or []) because its payload lives in tool_calls, and ChatOpenAI.for_api/3 serialises it verbatim. OpenAI accepts null/"" next to tool_calls, but gateways fronting Anthropic (e.g. Heroku Inference) require non-empty text on every message and reject the retry with HTTP 400 `messages[2]: content is required` — which the earlier nil -> "" coercion did not avoid. fill_blank_content/1 now substitutes a short placeholder, "(structured response provided as a tool call)", for assistant messages with blank content that carry tool calls, and leaves every other message untouched. Also make validation failures visible in production: the retry is now logged at :info (was :debug) with a bounded (<= 500 chars) reason — strategy, schema, the changeset errors via describe_error/1 and the payload's top-level keys, never the full payload — and the same reason is attached to the [:defacto_ai, :repair_loop, :retry] telemetry metadata as :reason. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…se on retry Production runs against Heroku Inference (claude-4-5-sonnet, streaming, ToolCall strategy) surfaced two follow-on failures. 1. `validation failed (markdown: is required) payload_keys=[]`: the streamed tool call reached decode_payload with empty arguments. DefactoAI.StreamRunner keyed every chunk on `index`, so a gateway that opens the call with index/id/function.name and then streams argument deltas with a missing or different index left the real call empty and the arguments in a nameless slot. Resolve the target slot per chunk: an explicit index at an opened call wins; a delta without a usable index (nil, or a slot never opened) continues the most recently opened call unless it clearly names another one; a complete (non-delta) `choices[0].message.tool_calls` is accepted as finished calls; and the legacy `delta.function_call` shape is treated as index 0. When a call still ends with empty arguments and no content, log at :info a bounded sample (first 5 chunks, 300 chars each) of the raw tool-call chunks so the real shape shows up in production logs. Raw chunks live in the collector state only until build_message/1. 2. The retry then failed with Anthropic's `messages.2: tool_use ids were found without tool_result blocks immediately after`. OpenAI has the same rule. DefactoAI.RepairLoop now answers a rejected tool-call turn with one is_error tool result per tool call (ToolResult with the call's tool_call_id and name, content = the validation error) before the existing corrective user message; ChatOpenAI.for_api/3 serialises each as a `role: tool` message with that tool_call_id. Without tool calls the single user message is kept. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ttrs
A live probe of Heroku Inference (stream: true, forced tool) shows the
real chunk shape: frames are `event: message` + `data: {...}`; the
opening chunk carries index/id/function.name; every argument delta
repeats `"name": ""`; and `tool_calls[].index` is Anthropic's
content-block index, so a text block before the tool_use puts the
argument deltas on a different index than the opening chunk. That is how
the streamed `respond` call reached decode_payload with no arguments.
DefactoAI.StreamRunner now normalises blank ids/names to "not given",
so `""` can neither open a call nor overwrite a real name; a delta
without id and without a non-empty name merges into the most recently
started call regardless of its index (an explicit index is only honoured
when it points at a started call, which keeps OpenAI's parallel calls
apart); argument deltas that arrive before their call opens are folded
into it once it does; and orphan slots without id or name are dropped
when a started call exists.
DefactoAI.Strategy.ToolCall.decode_payload/1 prefers the `respond` call
that actually carries arguments when several tool calls are present,
then any `respond` call, then the first one.
New per-call option `chat_model: keyword | map` on complete_structured/3,
complete_chat/2 and stream_chat/2: ChatOpenAI attributes merged into the
request (e.g. `max_tokens: 8_000` — Heroku's default is too small for a
multi-page markdown answer, and a truncated tool-call arguments JSON
fails to parse and cascades through every strategy). Strategy-controlled
attributes (tool_choice, json_response, stream) win over the caller's.
stream_chat builds its body by hand and honours :max_tokens and
:temperature. Documented in the README and the Client opts typedoc.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Production (Heroku Inference, images + long output) shows the opening
tool-call chunk arriving and then, after ~58 s of generation, no argument
deltas at all: `streamed tool call finished with empty arguments; 1 raw
tool-call chunk(s) seen`. The repair loop then spent both retries on the
same empty payload and the import failed with validation_failed.
Diagnostics: the StreamRunner empty-arguments log now also reports the
assembled content length and a 300-char sample, how many `data:` frames
were parsed, how many of those were not recognised as delta/message
chunks, and one unrecognised frame — so the next occurrence shows where
the bytes went.
Recovery: Strategy.ToolCall.decode_payload/1 hands unparsed argument text
to the JSON pipeline; when the chosen `respond` call has empty arguments
but the message has text content, returns that content so
JSON.decode_and_cast can extract the JSON; and when both are empty
returns {:error, :empty_tool_call}, which the client already maps to
:decode_failed and falls back to JsonMode/TextRepair instead of retrying.
RepairLoop treats an empty decoded map the same way
({:decode_failed, :empty_payload}).
Tests that exercised repair-loop retries via an empty tool call now use a
wrong-key payload, since an empty one is a fallback by design.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.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
1 participant
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.
Problem
DefactoAI.StreamRunner.run/2andDefactoAI.Client.LangChain.stream_chat/2post streaming chat completions with Req usinginto: collector()anddecode_body: false. The collectors parse every body chunk as SSE and keep onlydata:lines.When the gateway answers a non-200 status, the body is a plain JSON error document with no
data:lines. The collector consumed it anyway, so the error surfaced as{:api_error, 422, ""}and the provider's message was lost.Concrete case: Heroku Inference rejects an unreachable image URL with
Consumers classify these errors by matching on the message text (e.g. distinguishing a bad image URL from a genuine provider fault), which is impossible when the body arrives empty.
Fix
In both collectors, when
resp.status != 200, append the raw chunk bytes to a private:error_bodybuffer instead of SSE-parsing them. The non-200 branch reports that buffered text, falling back to the existingbody_to_text(body)when nothing was buffered. The 200 path is unchanged. The raw JSON text is returned as-is (callers match on substrings).Tests
test/defacto_ai/stream_runner_test.exs(Reqplug:stubs): streamed-content happy path, a 422 JSON error body is preserved verbatim, and an empty 503 body still yields"".stream_chat/2case asserting the streamed{:error, {:api_error, 422, body}}element carries the JSON body.Both new error-body tests fail against the previous collectors (
left: "") and pass with this change. Full suite: 97 tests, 0 failures.🤖 Generated with Claude Code