harness(openai): make local models (Ollama, LM Studio) work reliably#57
Conversation
Runs a scenario matrix (tool calls, streaming, forced tool choice, response formats, thinking-tag leakage) against any OpenAI-compatible endpoint via OPENAI_BASE_URL/OPENAI_MODEL, printing PASS/FAIL per scenario. Used to verify Ollama and LM Studio integrations. Claude-Session: https://claude.ai/code/session_017qCRgEVrmhF1J7p1YnRZPr
…rvers
Add two capability knobs to the OpenAI adapter so instances pointed at
local OpenAI-compatible runtimes (LM Studio, llama.cpp server) can avoid
the two request shapes those servers reject with HTTP 400:
- with_named_tool_choice(false): a ToolChoice::Tool(name) is wired as
tool_choice:"required" with the tools array filtered down to just the
named tool, preserving the "must call this tool" semantics exactly.
- with_json_object_format(false): a ResponseFormat::JsonObject is wired
as a permissive json_schema ({schema:{type:object}, strict:false}),
which still guarantees a JSON object.
Both default to supported=true, so hosted OpenAI behavior is unchanged.
Threaded through a new Degrade flag set so translate_request stays pure
and unit-testable. Verified accepted by LM Studio.
Claude-Session: https://claude.ai/code/session_017qCRgEVrmhF1J7p1YnRZPr
Zero-config compatibility for local OpenAI-compatible servers: even with the capability knobs left at their supported defaults, a chat-completions call that fails with HTTP 400 whose error body implicates a named tool_choice (for a ToolChoice::Tool request) or response_format (for a JsonObject request) is retried exactly once with the offending shape degraded. Mirrors the existing max_output_tokens auto-retry on the Responses path, extended to the chat path and shared by both invoke and stream so the streaming path degrades too. Bounded to a single retry per call, and only for the shape the request actually used. Claude-Session: https://claude.ai/code/session_017qCRgEVrmhF1J7p1YnRZPr
Add a "Local-server compatibility" section to the provider README and a module-doc summary covering the two rejected request shapes, the degraded wire forms, the with_named_tool_choice / with_json_object_format knobs, and the zero-config single-shot auto-degrade retry on HTTP 400. Claude-Session: https://claude.ai/code/session_017qCRgEVrmhF1J7p1YnRZPr
Add a focused reasoning_tags module with a streaming-safe state machine (getPotentialStartIndex-style partial-tag hold-back) and a whole-string extractor. Wire non-streaming (parse_chat_response) and streaming (OpenAiStreamAcc) to move inline chain-of-thought onto the reasoning channel as a leading Thinking block, consistent with the side-channel path. Configurable via OpenAiModel::with_reasoning_tag_extraction, default on for the plain `think` tag. Claude-Session: https://claude.ai/code/session_017qCRgEVrmhF1J7p1YnRZPr
…action Cover the SSE delta path (tag inside one delta, opening/closing tag split across delta boundaries, DeepSeek start-with-reasoning mode, disabled opt-out) and the non-streaming parse_chat_response path, including the side-channel-leads-then-inline combination rule. Claude-Session: https://claude.ai/code/session_017qCRgEVrmhF1J7p1YnRZPr
Describe the default-on <think> extraction, streaming-safe partial-tag buffering, DeepSeek start-with-reasoning mode, side-channel interaction rule, and the with_reasoning_tag_extraction toggle in the module README. Claude-Session: https://claude.ai/code/session_017qCRgEVrmhF1J7p1YnRZPr
…del-compat # Conflicts: # src/harness/providers/openai/README.md # src/harness/providers/openai/transport.rs
Small local servers (Ollama, LM Studio, llama.cpp, vLLM) violate the OpenAI tool-call contract: Ollama omitted `id` until v0.12.11, some compat servers send `function.arguments` as a JSON object instead of a stringified string, and `type` can be absent. A single missing/unexpected field used to fail deserialization of the whole response, so the model appeared "broken". Make the wire structs lenient (additive to the serde shapes so hosted OpenAI is unaffected): - `ToolCallWire.id` / `kind` default instead of being required. - `FunctionCallWire.name` defaults; `arguments` accepts a string, an object (re-serialized to JSON text), or absent via `deserialize_arguments`. - The streaming `FunctionChunkWire.arguments` gets the same leniency via `deserialize_optional_arguments`, preserving the `Option` for fragments. Claude-Session: https://claude.ai/code/session_017qCRgEVrmhF1J7p1YnRZPr
Ollama's /v1 endpoint emitted parallel tool calls all with `index: 0` (ollama/ollama#15457). An explicit index always selected that slot, silently merging two distinct calls into one. In `resolve_slot`, when a delta carries an explicit index *and* a non-empty id that conflicts with the id already recorded at that slot, treat it as a new tool call: reuse an existing slot already opened for that id, otherwise open a fresh one. Id-less continuation fragments keep the current append behavior. Claude-Session: https://claude.ai/code/session_017qCRgEVrmhF1J7p1YnRZPr
… run
Small local models occasionally emit tool-call arguments that are not valid
JSON. The OpenAI provider hard-failed the whole model call (non-retryable),
which mature frameworks avoid: LangChain surfaces `invalid_tool_calls` and the
AI SDK surfaces invalid dynamic tool parts back to the model so it can retry.
Add `ToolCall::invalid: Option<String>` (serde default, additive) plus
`ToolCall::invalid(..)`/`is_invalid()`. When arguments cannot be parsed even
after the conservative chat-template-marker repair, the provider now produces a
`ToolCall` with the raw string preserved in `arguments` and the parse reason in
`invalid`, rather than erroring — on both the non-streaming (`convert.rs`) and
streaming (`sse.rs`, now infallible) paths. The non-streaming path also
synthesizes the same `tool-{index}` fallback id the streaming path uses when
the provider omits/empties `id`.
The agent loop responds to an invalid call by injecting an error tool result
(an `AgentEvent::InvalidToolArgs` recovery, mirroring the schema-invalid and
unknown-tool seams) and continuing, so the model can retry and a malformed blob
can never become a never-resolving tool call that stalls the loop. This is
unconditional (not gated by `InvalidArgsPolicy`, which governs schema
validation of well-formed args): an unparseable payload is a transport defect.
Tests: id-less/empty-id non-streaming call, object-form arguments, index-0
parallel calls kept distinct, empty-name continuation not overwriting the name,
streaming malformed args reconstructing as an invalid call, and the agent-loop
malformed-args-to-error-tool-result recovery. Existing fail-fast tests updated
to assert the new lenient representation.
Claude-Session: https://claude.ai/code/session_017qCRgEVrmhF1J7p1YnRZPr
…rgs recovery Add a "Local-model tolerance (tool calls)" section to the OpenAI provider README (lenient id/type/arguments wire handling, index-0 parallel-call de-duplication, empty-name protection, and the invalid-tool-call representation) and an "Invalid tool-argument recovery" section to the harness tool module doc distinguishing schema-invalid (InvalidArgsPolicy) from unparseable (unconditional recovery). Claude-Session: https://claude.ai/code/session_017qCRgEVrmhF1J7p1YnRZPr
…l-model-compat # Conflicts: # src/harness/providers/openai/README.md
Local reasoning models (e.g. qwen3 via Ollama) intermittently spend the entire token budget on the hidden reasoning channel and return finish_reason="length" with empty visible content, no tool calls, and no structured output. The loop previously surfaced this as a blank success. Detect that condition in the finalization branch (before structured extraction, which would fail on the empty completion) and re-issue the model call: doubling the request's max_tokens when set (clamped at 4x the original) or a plain retry when unset. Bounded by the new RunPolicy::truncated_empty_retries knob (default 1). The retry runs before error_on_empty_response, so that guard only applies once retries exhaust. Each attempt counts as a model call and emits AgentEvent::RetryScheduled. Shared run_loop covers both the invoke and streaming paths. Claude-Session: https://claude.ai/code/session_017qCRgEVrmhF1J7p1YnRZPr
…ix reused-index continuations Review findings on the local-model compat work: - Inline <think> extraction was enabled unconditionally, so hosted OpenAI responses mentioning a literal <think> tag lost content. Until with_reasoning_tag_extraction is called, the built-in default now applies only when base_url differs from the hosted default. - An id-less argument continuation carrying a reused index (Ollama's index-0 parallel-call bug combined with the standard continuation shape) routed to the raw index, corrupting the first call's arguments and orphaning the second. resolve_slot now keeps a wire index -> slot cursor that conflict handling repoints, so continuations follow the call most recently opened at that index. Claude-Session: https://claude.ai/code/session_017qCRgEVrmhF1J7p1YnRZPr
Cover the new recovery with the testkit ScriptedModel (which records each request): a length-truncated empty response followed by a usable one recovers with two model calls, a RetryScheduled event, and a doubled token budget on the retried request; an unset budget stays unset over a plain retry; exhausted retries fall back to the blank final (guard off) or EmptyResponse (guard on); truncated_empty_retries=0 disables the retry; and a length response that still carries text is never treated as empty. Claude-Session: https://claude.ai/code/session_017qCRgEVrmhF1J7p1YnRZPr
Explain RunPolicy::truncated_empty_retries: the length/empty detection, the doubled-budget retry (capped at 4x, overriding the per-turn cap), ordering ahead of error_on_empty_response, the RetryScheduled event, and the 0 opt-out. Covers both the unary and streaming paths. Claude-Session: https://claude.ai/code/session_017qCRgEVrmhF1J7p1YnRZPr
# Conflicts: # examples/local_model_probe.rs # src/harness/providers/openai/mod.rs # src/harness/providers/openai/transport.rs
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 29 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (28)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9af7a670b1
ℹ️ 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".
| let mut truncated_empty_retries_used: u32 = 0; | ||
| let mut boosted_max_tokens: Option<u32> = None; | ||
| let mut truncation_base: Option<u32> = None; |
There was a problem hiding this comment.
These retry variables live for the entire agent loop, not just the logical turn described in the comment. If a truncated-empty attempt is retried and the retry returns tool calls, the run continues after executing the tools with truncated_empty_retries_used already consumed and boosted_max_tokens still set, so later model turns can exceed the configured per-turn cap and cannot use the configured truncated-empty recovery if they hit the same failure. Reset this state once a non-truncated response proceeds to tool execution (or scope it to one turn).
Useful? React with 👍 / 👎.
| messages.pop(); | ||
| truncated_empty_retries_used += 1; |
There was a problem hiding this comment.
When a response cache is attached and the original request has no max_tokens, the first truncated-empty response is cached by invoke_model_with_retry before this recovery branch runs; after messages.pop(), the retry rebuilds the identical request, so it is served from that cached empty response instead of reissuing the stochastic provider call. That makes the documented plain retry for uncapped calls ineffective (and can keep returning a blank final) unless caching is disabled or the retry bypasses/evicts the cached truncated-empty response.
Useful? React with 👍 / 👎.
1 participant
Summary
Users reported that local models don't work properly with openhuman. This PR fixes every root cause we could reproduce live against Ollama (qwen3:4b, llama3.2:3b) and LM Studio (gemma-4-e4b), informed by how LangChain/LangGraph and the Vercel AI SDK handle the same local-server quirks.
Request-side degradation (LM Studio & friends 400 on standard shapes)
tool_choice(object form) degrades totool_choice:"required"+ the wiretoolsarray filtered to the named tool;response_format: json_objectdegrades to a permissivejson_schema. Knobs:with_named_tool_choice(false)/with_json_object_format(false).tool_choice/response_formattriggers one degraded retry (unary + streaming), sofrom_env()/presets just work.Response-side tolerance (local servers violate the tool-call contract)
id(Ollama < 0.12.11), missingtype,argumentsas JSON object instead of string. Missing ids get the sametool-{index}fallback the streaming path uses.index: 0(OpenAI-compatible streaming: tool_calls index is always 0 for multiple tool calls ollama/ollama#15457) are kept distinct via id-conflict detection with a wire-index→slot cursor so id-less continuations follow the right call; empty-namecontinuation fragments never clobber a recorded name (lmstudio-bug-tracker#649).ToolCall { invalid: Some(reason) }and the agent loop feeds the error back to the model as an error tool result (mirrors LangChaininvalid_tool_calls/ AI SDK invalid tool parts).Reasoning models
<think>…</think>extraction onto the Thinking channel (partial-tag hold-back, DeepSeek-R1 no-opening-tag mode via config). Default ON only for non-hosted base URLs; hosted OpenAI is ungated so literal<think>mentions are never stripped.max_tokensbudget is burned on reasoning (finish_reason:"length", empty content):RunPolicy::truncated_empty_retries(default 1) retries with a doubled budget (clamped at 4x) before the existingerror_on_empty_responseguard applies. This addresses the empty-final-answer class of complaints (openhuman#4638).Diagnostics
examples/local_model_probe.rs: a 13-scenario PASS/FAIL matrix (tool loop, streaming deltas, forced/required tool choice, parallel calls, zero-arg tools, response formats, thinking leakage) runnable against any OpenAI-compatible endpoint.Verification
cargo fmt --check,cargo clippy --all-targets -- -D warnings, fullcargo test: all green.openai_tools,openai_chat): 3/3 runs per server produce correct non-empty answers with tool calls; the truncated-empty retry visibly recovers qwen3 runs.Known non-blocking notes:
tests/live_streaming.rshardcodesmax_tokens: 16and fails against qwen3 (pre-existing test design colliding with reasoning burn, bypasses loop recovery); one unreproduced LM Studiostreaming-toolsflake (empty model output) observed once in 3 probe runs.https://claude.ai/code/session_017qCRgEVrmhF1J7p1YnRZPr