test(harness): live Ollama + LM Studio coverage, and the tool-call defects it found - #93
Conversation
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 12 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 (14)
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: 3cc45b6f1b
ℹ️ 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".
| if let Some(call) = parse_bare_tool_call(&text) { | ||
| calls.push(call); | ||
| response.message.tool_calls.extend(calls); |
There was a problem hiding this comment.
When tools are offered and the model returns a legitimate JSON answer such as {"name":"Alice","age":30}, this branch treats the name field alone as proof of a tool call, removes the answer text, and sends an unknown Alice call through the agent loop. This is especially likely when combining tools with a JSON response format; bare-call recovery needs to confirm that the name matches an offered tool (and preferably that an argument-envelope key is present) before consuming the response.
Useful? React with 👍 / 👎.
| // The object was the whole visible text, so nothing survives as | ||
| // prose — but a reasoning model's `Thinking` block must, hence | ||
| // `replace_text_blocks` with empty text rather than clearing the | ||
| // content outright. | ||
| response.message.content = replace_text_blocks(response.message.content, String::new()); |
There was a problem hiding this comment.
For a streaming response containing a bare tool-call object, clean_stream_item forwards every JSON MessageDelta because its scrubber only recognizes tagged calls, and this terminal-only branch then removes the text from Completed. Consequently a consumer rendering incremental deltas displays the raw tool-call JSON even though the final response says it was consumed as a tool call. The streaming path must buffer or otherwise retract bare-object candidates so its visible deltas remain consistent with the recovered completion.
Useful? React with 👍 / 👎.
1 participant
Summary
Adds live test coverage for local model runtimes — Ollama and LM Studio — and
fixes the defects that writing it uncovered.
live_provider_matrixalready proves a configured endpoint answers chat,streaming, and a one-shot tool call. That is necessary but not sufficient for a
local runtime: the ways local servers break are specific to them and mostly
invisible to a single-call probe. Two new files drive the whole loop instead:
tests/live_local_models.rs(11 tests) — model discovery, chat, streamingdeltas, a one-shot tool call, a full tool round trip through
AgentHarness(model requests the tool, harness runs it, model answers usingthe result), and structured JSON output.
tests/live_local_embeddings.rs(9 tests) — dimensional honesty,positional integrity of batches, semantic separation, and a real
index-and-retrieve round trip.
e2e_embeddings.rscovers the same plumbingagainst
MockEmbeddingModel, which hashes text to a stable vector, so itsranking assertion is tautological by construction; these use real vectors and
queries that share no content words with the target document.
Every assertion runs against every reachable runtime, so one file covers both
and any future OpenAI-compatible local server. Both are opt-in via
LOCAL_MODEL_TESTS=1and skip a runtime that is not listening — but failif the variable is set and nothing is reachable, since every assertion lives
inside a loop over the reachable runtimes and an empty list would otherwise be a
green run that tested nothing.
Four defects found by running it
Tool calls emitted as text were silently dropped. Roughly one response in
twelve,
llama3.2:3bundertool_choice: "required"puts the call incontentrather than the wire'stool_callsarray, with no<tool_call>markup and often malformed JSON
(
{"name":"get_weather","parameters':{'city':"Paris"}}). The loop saw anassistant message with no tool calls, treated it as the final answer, and
returned JSON-looking prose to the user while the tool never ran.
apply_prompt_tool_callsnow recovers it, requiring the entire messagecontent to parse as one object naming a tool so prose that merely quotes JSON
is never swallowed. Mismatched and single-quoted keys are repaired;
single-quoted values deliberately are not, because an apostrophe in a value
is ordinary English.
Arguments buried in an envelope failed validation. Captured shapes:
{"type":"object","properties":{"city":"Paris"}}(schema echo),{"arguments":{…}},{"param":{…}}.normalize_tool_argumentsnow unwrapsone envelope level — but only when the outer object is already
schema-invalid, the tool does not itself declare an argument of that name,
and the unwrapped value validates. Failing any of those, the original
arguments survive so the model still sees a precise error.
LM Studio had no preset. It was reachable only by bare base URL, which
resolves to
ProviderKind::Compatibleand therefore misses the local-runtimepath entirely: no
Authorizationsuppression, no/v1base-URLnormalisation, and none of the request-shape degradations llama.cpp-backed
servers need. Adds
ProviderKind::LmStudioand routes both local kindsthrough one
local_runtime_default_rootseam. Also: the provider matrixskipped local runtimes unless given a placeholder API key, so a correctly
configured local server was excluded by a variable that could never be filled
in meaningfully.
OllamaEmbeddingModel::try_new(url, model, 0)does not mean "discover" —it means "use the default" (1024), so
nomic-embed-textat 768 fails everycall on dimension validation. Documented alongside the equivalent trap on the
OpenAI-compatible adapter, which validates against
text-embedding-3-small's 1536 by default.Three findings left for maintainer judgement, not changed
RunPolicy::invalid_argsdefaults toFail— the first schema-invalidtool call kills the whole run, and it also disables the argument recovery in
(2), which only runs under the recovering policy. Defensible for a frontier
model; for a 3B it makes the loop unusable. Local hosts must opt into
NormalizeThenReturnToolError. Pinned in a test so a future change to thedefault is deliberate rather than silent.
empty vector per blank and never dials; the OpenAI-compatible adapter rejects
the batch. Both are position-safe — neither drops a blank and shifts every
later vector onto the wrong id — but they are not interchangeable. Both
behaviours are pinned rather than one being picked.
ToolChoice::Requiredis best-effort on local models. Measured 11/12 onllama3.2:3b. The tool tests re-roll up to four times and print when they do,so the model's non-determinism is visible rather than hidden, and a runtime
that genuinely cannot emit a tool call still fails all four.
API Or Behavior Changes
ProviderKind::LmStudio. Additive; it is the onlypreset with no default model, because the served id is whatever GGUF the
operator loaded.
every_built_in_preset_name_resolvesdocuments thatexemption explicitly.
ProviderKind::inferacceptslmstudio:/lm_studio:/lm-studio:.arguments one envelope level down, is recovered where it previously was not.
Both paths are gated so a well-formed call can never be rewritten — the
envelope unwrap additionally requires that the unwrapped value validates.
relaxed_json::recover_relaxed_objectwidened frompub(super)topub(crate)so the prompt parser can reuse the repair. Still crate-private.Tests
Run from the crate root:
cargo fmt --checkcargo clippy --all-targets -- -D warningscargo clippy --all-targets --all-features -- -D warningscargo build --all-targetscargo build --all-targets --all-featurescargo test— 92 test binaries, 0 failurescargo test --all-features— 92 test binaries, 0 failuresLive runs against real servers on an RTX 5060 (8 GB):
llama3.2:3b,nomic-embed-text) — 6/6 consecutive clean runs ofthe model suite after the fixes above.
qwen/qwen3-4b,nomic-embed-text-v1.5) — green, but see thecaveat below.
Caveat on the LM Studio runs
LM Studio's headless daemon (
llmster0.0.20-1) intermittently fails anotherwise-valid request with
HTTP 400 {"error":"Engine protocol predict request failed: fetch failed"}— its own internal engine RPC, not a request the adapterbuilt wrongly. It hits a different test each time and survives a full daemon
restart, so it is an LM Studio bug rather than something this branch introduces;
the same tests pass on adjacent runs against the same server. Roughly half the
suite runs on this box were clean. The tests deliberately do not retry
around it: swallowing a provider-level error would hide a real outage, which is
the opposite of what this suite is for.
Also worth knowing for anyone reproducing: an 8 GB card cannot hold both
runtimes at once. When it is oversubscribed Ollama silently drops to partial CPU
offload (
25%/75% CPU/GPU) and calls go from ~1 s to over 5 min, which looksexactly like a hang. Run the two runtimes separately, or expect that.
Documentation
docs/modules/harness/local-models.md— presets, every failure mode above with the captured wire shapes, the
InvalidArgsPolicyguidance, the reasoning-model token-budget trap, the twoembedding adapters and their blank-input divergence, and how to run the suites.
docs/modules/harness/README.md.providers.env.example— LM Studio now documented as a preset, and the stale"set any non-blank value" advice for keyless local runtimes corrected.