mirror of
https://wget.la/https://github.com/leookun/cursor-byok
synced 2026-10-03 18:23:51 +08:00
fix(provider): keep OpenAI Chat tool calls that finish with reason "stop"
`map_finish` matched `"stop" | "content_filter"` before the `has_tools`
fallback, so the fallback only ever applied to finish reasons the adapter did
not recognise. When an OpenAI-compatible server streams tool calls and then
reports `finish_reason: "stop"` the adapter yielded `Done(Stop)` even though
`ToolCallStart` / `ToolCallEnd` had already been emitted.
`run/model_cycle.rs` rejects that combination:
let has_tool_calls = !calls.is_empty();
if matches!(finish_reason, FinishReason::ToolUse) != has_tool_calls {
return Err(failure(RunFailure::Protocol(
"finish reason and tool calls disagree".into()), ...));
}
so the whole turn fails with a protocol error and the tool never runs. Servers
that report `"stop"` alongside `tool_calls` are common in BYOK setups
(llama.cpp, Ollama's OpenAI shim, several proxies), which makes those models
unusable for anything agentic.
Move the `has_tools` arm ahead of `"stop" | "content_filter"` so an observed
tool call outranks the label the provider attached to the stop. This matches
the two sibling adapters (`anthropic.rs` checks `_ if saw_tool` before every
reason except `Length`, `openai_responses.rs` derives the reason from
`saw_tool` alone) and this adapter's own `[DONE]` fallback, which already
infers `ToolUse` from `!tools.is_empty()`. `"length"` still wins so a
truncated response is still reported as truncated, and behaviour with no tool
calls is byte-for-byte unchanged.
Before (with the arm restored):
cargo test -p cursor-server --lib provider::openai_chat
-> observed_tool_calls_outrank_a_stop_finish_reason FAILED
assertion `left == right` failed: left: Stop, right: ToolUse
After:
cargo test -p cursor-server --lib provider::openai_chat -> 3 passed
This commit is contained in:
@@ -368,7 +368,9 @@ fn map_finish(value: &str, has_tools: bool) -> FinishReason {
|
||||
match value {
|
||||
"tool_calls" | "function_call" => FinishReason::ToolUse,
|
||||
"length" => FinishReason::Length,
|
||||
"stop" | "content_filter" => FinishReason::Stop,
|
||||
// Observed tool calls outrank whatever the provider labelled the stop:
|
||||
// OpenAI-compatible servers routinely report "stop" while streaming tool
|
||||
// calls, and the run rejects a finish reason that contradicts them.
|
||||
_ if has_tools => FinishReason::ToolUse,
|
||||
_ => FinishReason::Stop,
|
||||
}
|
||||
@@ -434,3 +436,30 @@ pub(crate) fn openai_usage(value: &Value) -> Usage {
|
||||
.and_then(Value::as_u64),
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn observed_tool_calls_outrank_a_stop_finish_reason() {
|
||||
assert_eq!(map_finish("stop", true), FinishReason::ToolUse);
|
||||
assert_eq!(map_finish("content_filter", true), FinishReason::ToolUse);
|
||||
assert_eq!(map_finish("", true), FinishReason::ToolUse);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn finish_reason_mapping_without_tool_calls_is_unchanged() {
|
||||
assert_eq!(map_finish("stop", false), FinishReason::Stop);
|
||||
assert_eq!(map_finish("content_filter", false), FinishReason::Stop);
|
||||
assert_eq!(map_finish("", false), FinishReason::Stop);
|
||||
assert_eq!(map_finish("tool_calls", false), FinishReason::ToolUse);
|
||||
assert_eq!(map_finish("function_call", false), FinishReason::ToolUse);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_truncated_response_stays_truncated_even_with_tool_calls() {
|
||||
assert_eq!(map_finish("length", true), FinishReason::Length);
|
||||
assert_eq!(map_finish("length", false), FinishReason::Length);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user