fix(ollama): keep structured tools prompt-guided

[Audacity88]

Summary

• Base branch: master
• What changed and why:
  • Ollama advertises supports_native_tools() == false, but the structured chat() path still converted ToolSpec values into a native /api/chat.tools payload whenever tools were present.
  • That made tool-using prompts depend on model-side native Ollama tool support, which can fail for models like the ones reported in [Bug]: Ollama Provider call failed when tools are needed #5962 instead of using ZeroClaw's prompt-guided tool protocol.
  • This PR keeps structured Ollama tool requests on the prompt-guided path: it injects the existing tool protocol instructions into the chat messages and sends /api/chat without a native tools array.
  • The response parsing shared by chat() and chat_with_tools() now preserves returned text, native tool calls if Ollama still returns them, reasoning fallback behavior, and Ollama token usage counts.
  • A regression test captures the outgoing request body and verifies the request omits tools, includes the prompt-guided file_read schema, and preserves response usage.
• Scope boundary: This does not change retry policy, session recovery, Ollama transport/auth routing, live model capability detection, or the explicit chat_with_tools() native JSON path. It only aligns the structured provider chat() path with Ollama's advertised non-native-tools contract.
• Blast radius: Ollama provider structured chat requests that include tool specs. Other providers and non-tool Ollama chat requests are not intended to change.
• Linked issue(s): Closes [Bug]: Ollama Provider call failed when tools are needed #5962. Supersedes fix(ollama): avoid native tool payload on fallback path #6029.
• Labels: bug, risk: medium, size: M, provider, provider:ollama.

Validation Evidence (required)

• Commands run and public-safe result digest:

cargo fmt --all -- --check
Result: passed.

git diff --check origin/master...HEAD
Result: passed.

cargo test -p zeroclaw-providers chat_with_tool_specs_omits_native_tools_payload -- --nocapture
Result: passed. 1 passed; 0 failed; 896 filtered.

cargo clippy -p zeroclaw-providers --all-targets -- -D warnings
Result: passed.

Temporary combined validation stack: current master + #7091 + this PR
git diff --check origin/master...HEAD
Result: passed.

cargo fmt --all -- --check
Result: passed.

scripts/zc-cargo-broad clippy --workspace --exclude zeroclaw-desktop --all-targets --features ci-all -- -D warnings
Result: passed.

scripts/zc-cargo-broad --lane nextest nextest run --locked --workspace --exclude zeroclaw-desktop
Result: passed. 7444 tests passed; 10 skipped.

• Beyond CI — what did you manually verify? The regression test verifies the request body sent to /api/chat: no top-level native tools key is present, the injected system instructions include the prompt-guided tool protocol, the file_read tool name, and the path schema field, and the returned Ollama usage counters are preserved in ChatResponse.usage. I also merged this PR locally with fix(cron): expose DingTalk in cron delivery schemas #7091 over current master and ran the repo-shaped broad clippy and nextest gates on that combined stack.
• If any command was intentionally skipped, why: I did not run live Ollama against the reporter's Windows models. The focused provider regression verifies the corrected request shape directly, and the combined local broad validation plus green GitHub Quality Gate cover the broader Rust test/lint surface.

Security & Privacy Impact (required)

• New permissions, capabilities, or file system access scope? No
• New external network calls? No
• Secrets / tokens / credentials handling changed? No
• PII, real identities, or personal data in diff, tests, fixtures, or docs? No
• If any Yes, describe the risk and mitigation: N/A

The new test uses a local loopback HTTP server only. The production change removes a native tool payload from the structured Ollama request path; it does not add a new network destination, credential path, filesystem operation, or autonomous capability.

Compatibility (required)

• Backward compatible? Yes
• Config / env / CLI surface changed? No
• If No or Yes to either: No user migration is required.

The user-visible behavior change is that structured Ollama tool requests now follow the existing prompt-guided tool contract instead of sending native Ollama tool arrays from a provider that advertises native tools as unsupported.

Rollback (required for risk: medium and risk: high)

• Fast rollback command/path: git revert 6e0c2f9e9
• Feature flags or config toggles: None
• Observable failure symptoms: Ollama tool-using prompts may again send native tool payloads from the structured chat path and fail with provider 500 responses such as {"error":"EOF"} on models that do not support Ollama native tool arrays.

Supersede Attribution (required only when Supersedes # is used)

• Superseded PRs + authors (#<pr> by @<author>, one per line): fix(ollama): avoid native tool payload on fallback path #6029 by @TeoConnexioh
• Scope materially carried forward: fix(ollama): avoid native tool payload on fallback path #6029 identified the same Ollama provider contract gap: structured tool requests were reaching Ollama as native tool payloads even though the provider advertises prompt-guided tools. This PR carries that direction forward from current master, keeps the fix in the structured chat() path, adds request-body regression coverage, preserves response usage, and keeps the explicit native JSON helper path scoped separately.
• Co-authored-by trailers added in commit messages for incorporated contributors? No
• If No, why (inspiration-only, no direct code/design carry-over): No direct code was copied from fix(ollama): avoid native tool payload on fallback path #6029. The public attribution is the Supersedes #6029 relationship and this section.

[WareWolf-MoonWall]

Review on head of PR #7095. CI is green (13/13 checks passing). No prior reviews exist. I read the PR body (comprehensive validation section, supersede attribution), the diff (single file, ~200 lines, structured refactor + new test), and cross-checked against the linked issue #5962 and superseded PR #6029.

🟢 What looks good — Fixes the advertised contract violation

Ollama advertises supports_native_tools() == false, but the structured chat() path was still serializing ToolSpec values into a native /api/chat tools array whenever tools were present. This made tool-using prompts depend on model-side native tool support even when the provider explicitly says it doesn't support that mode. The fix is correct: structured tool requests now stay on the prompt-guided path by injecting tool instructions into the system message and omitting the native tools payload. This closes a real bug (#5962) where Windows users with models lacking native tool support saw {"error":"EOF"} failures.

🟢 What looks good — Regression test verifies the fix at the wire level

The new chat_with_tool_specs_omits_native_tools_payload test spins up a local Axum server, captures the outgoing /api/chat request body, and verifies three things: (1) no top-level tools key is present, (2) the system message includes the prompt-guided tool protocol instructions with the file_read tool schema, and (3) the response preserves Ollama's token usage counters. This is exactly the right level of test — it verifies the serialized wire format, not just the Rust-level structure.

🟢 What looks good — Clean refactor with shared response parsing

The patch extracts the duplicated response-to-ChatResponse logic into response_to_chat_response() and the message-prep logic into with_prompt_guided_tool_instructions(). Both the structured chat() path and the explicit chat_with_tools() path now share the same response parsing, which means token usage, native tool calls (if Ollama still returns them), reasoning fallback, and text normalization are consistent across both entry points. No logic changes to chat_with_tools() — the refactor is scoped to the structured path.

🟢 What looks good — Supersede attribution is exemplary

The PR body includes a full "Supersede Attribution" section that explains what #6029 identified, what this PR carries forward, and why no Co-authored-by trailer was added (no direct code from #6029). This is the model for how to attribute a superseding PR.

🟡 Warning — Combined validation stack is good practice but not required

The PR body notes "Temporary combined validation stack: current master + #7091 + this PR" with full broad clippy and nextest results. This is excellent due diligence — it verifies the patch doesn't conflict with other in-flight work — but it's not a formal requirement. The green CI on this PR's own head is sufficient for approval. Calling this out as a positive example of going beyond minimum validation.

🔵 Suggestion — Consider live validation with reporter's model before v0.7.6 tag

The validation evidence states "I did not run live Ollama against the reporter's Windows models. The focused provider regression verifies the corrected request shape directly." The regression test is sufficient for approving this PR, but before the v0.7.6 release tag, consider asking the #5962 reporter (or another Windows user with the affected models) to verify the fix end-to-end. Not blocking, but it would close the loop on the original bug report. If that's not practical, the wire-level test is good enough.

---

Verdict

APPROVED. This is a well-executed provider bug fix with the right scope (structured chat path only), the right test (wire-level request capture), and exemplary supersede attribution. The fix directly addresses the #5962 EOF errors without changing the explicit native-tool path or other providers. CI is green. Milestone v0.7.6 is correct for a targeted provider bug fix.

No #6848 compatibility concerns — this doesn't touch provider selection, model-first patterns, or agent configuration.

[bheatwole]

This commit somehow passed the commit gates, but breaks the build. A previous commit #6848 changes the signature of send_request to use an Option<f64> but this commit passes a f64

[NiuBlibing]

This commit somehow passed the commit gates, but breaks the build. A previous commit #6848 changes the signature of send_request to use an Option<f64> but this commit passes a f64

The ci gate needs harded.

[Audacity88]

@bheatwole thanks again for catching this. I merged #7095, so the merge break is on me. The immediate issue is fixed by #7231, but the process issue is that #7095 had green checks from an older base and then landed after #6848 changed the same Ollama request path/signature shape.

A few comments have suggested reinstating the strict "must be up to date with base before merge" rule. That would have prevented this, but we have tried that before and it created a lot of update/recheck churn during active review periods. GitHub's merge queue looks like the better compromise: instead of requiring every PR branch to stay manually current, the queue tests the exact merge group against current master before landing it.

GitHub docs: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/merging-a-pull-request-with-a-merge-queue

Until we have that, the maintainer-side rule should be: if a PR is behind/stale and current master touched the same file, crate, API surface, build/release path, or config/schema area, we either refresh official CI on the current merge result or run a focused local merge-result smoke before merging.