fix(llm-drivers): Copilot streaming empty tool calls + Claude assistant strip#3084
Conversation
…g response The Copilot API proxy can sometimes deliver streaming tool call chunks without a function name, resulting in empty-name tool calls stored in conversation history. When replayed to the API, these cause "tool call must have a tool call ID and function name" errors. Skip malformed tool calls (empty ID or name) during streaming response finalization and log a warning. Ported from openfang abeaaf5.
…via Copilot The Copilot proxy for Claude enforces Anthropic's rule that conversations must end with a user message. For Claude models, strip any trailing assistant message without tool_calls (including non-empty ones). For other models, only strip truly empty assistant messages. This fixes the 'assistant message prefill not supported' error seen in Telegram and other channel adapters when using Claude via Copilot, without causing infinite agent loops for other models. Tool-call-bearing assistant messages are always kept (protocol-essential). Includes unit tests covering both Claude and non-Claude code paths. Ported from openfang 64b1ec5.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0bfa2eebea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// are part of the protocol and must stay). Checks the model name to only | ||
| /// apply for Claude models which enforce this restriction. | ||
| fn strip_trailing_empty_assistant(messages: &mut Vec<OaiMessage>, model: &str) { | ||
| let is_claude = model.contains("claude"); |
There was a problem hiding this comment.
Match Claude model names case-insensitively
strip_trailing_empty_assistant decides Claude behavior with model.contains("claude"), which is case-sensitive. Elsewhere we normalize by lowercasing only for matching but preserve the original model casing, so values like anthropic/Claude-3-5-sonnet can reach this path and skip the Claude-specific strip logic. In those cases the original Copilot prefill error can still occur depending on how the model name was cased in config, making behavior inconsistent for the same model.
Useful? React with 👍 / 👎.
…are filtered When the Copilot proxy drops function-name chunks, every entry in tool_accum can be skipped by the empty-id/name guard. The upstream finish_reason is still 'tool_calls', which previously left stop_reason=ToolUse with an empty tool_calls vector — staging a tool-use turn the agent loop cannot execute. Downgrade to EndTurn in that case so the turn ends cleanly.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Summary
Two fixes to the OpenAI driver (used by github-copilot proxy):
idorname, which then poisons the next API request. We log a warn and drop those entries.The strip is wired through the single
build_requestentry-point in LibreFang (one call site instead of upstream's two).Ported from openfang
abeaaf5and64b1ec5.Test plan
drivers/openai.rscovering claude/non-claude × empty/non-empty/with-tool_callsprovider = "github-copilot"+ a Claude model, send a message that triggers a tool call, verify the conversation continues without the upstream "assistant prefill not supported" error