fix(ollama): restore compiling master build

[singlerider]

Summary

• Base branch: master
• What changed and why: master does not build. Two defects, both in crates/zeroclaw-providers/src/ollama.rs, both latent because PR fix(ollama): keep structured tools prompt-guided #7095's CI ran against a two-day-stale base and never validated against current master:
  1. error[E0308]: mismatched types — fix(ollama): keep structured tools prompt-guided #7095 added let temperature = temperature.unwrap_or(self.default_temperature()); in OllamaModelProvider::chat(), shadowing the Option<f64> parameter into a bare f64, then passed it to send_request(.., temperature: Option<f64>, ..). This breaks cargo build outright — users hit it via the curl | bash source install. Fix: drop the shadowing so Option<f64> flows through unchanged, matching the file's three other send_request callers. Temperature is still honoured downstream in build_chat_request_with_think via self.tuning.temperature_override.or(temperature); None correctly defers to Ollama's native model default. No behavioural loss.
  2. use of a disallowed method tokio::spawn — a bare tokio::spawn in the prompt-guided-tools test (also added by fix(ollama): keep structured tools prompt-guided #7095) trips the disallowed_methods clippy lint under -D warnings, failing the CI lint gate. Fix: use zeroclaw_spawn::spawn!, matching the established test-server pattern in factory.rs and openrouter.rs, so the spawned task inherits the caller's attribution span.
• Scope boundary: Two minimal edits in ollama.rs. Does not touch the prompt-guided-tools logic fix(ollama): keep structured tools prompt-guided #7095 introduced; only the compile error and lint violation it shipped alongside.
• Blast radius: Ollama provider only. Restores compilation of zeroclaw-providers, unblocking the entire workspace build, and clears the lint gate.
• Linked issue(s): Related fix(ollama): keep structured tools prompt-guided #7095 (introduced both regressions).
• Labels: bug, risk: medium, size: XS

Validation Evidence (required)

cargo fmt -p zeroclaw-providers -- --check                    # exit 0, clean
cargo clippy -p zeroclaw-providers --all-targets -- -D warnings # Finished, clean (was: disallowed tokio::spawn)
cargo check -p zeroclaw-providers                             # Finished (was: E0308)
cargo test -p zeroclaw-providers ollama                      # 68 passed; 0 failed

• Commands run and tail output:
  • cargo clippy --all-targets -- -D warnings: Finished dev profile — failed before with error: use of a disallowed method tokio::spawn at ollama.rs:1270.
  • cargo check: Finished dev profile — failed before with error[E0308]: mismatched types ... expected Option<f64>, found f64.
  • cargo test ollama: test result: ok. 68 passed; 0 failed; 0 ignored
  • cargo fmt --check: clean
• Beyond CI — what did you manually verify? Reproduced the original E0308 on a clean worktree at master (a1b641e01) before fixing, confirming this is the exact failure users reported on the source install. Confirmed the lint failure by running the CI lint command verbatim.
• If any command was intentionally skipped, why: None.

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

Compatibility (required)

• Backward compatible? Yes
• Config / env / CLI surface changed? No
• Upgrade steps: none.

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

• Fast rollback command/path: git revert <sha>
• Feature flags or config toggles: None
• Observable failure symptoms: cargo build fails with error[E0308]: mismatched types, or cargo clippy --all-targets -- -D warnings fails with use of a disallowed method tokio::spawn, in crates/zeroclaw-providers/src/ollama.rs.

[JordanTheJet]

I reproduced the failure on origin/master and walked the fix against the local source. This is a clean, correct compile fix — the kind of one-line revert that unblocks everyone's build. Approving.

I confirmed the regression is real: on master, OllamaModelProvider::chat() shadows the temperature: Option<f64> parameter with let temperature = temperature.unwrap_or(self.default_temperature());, and since default_temperature() -> f64, the binding collapses to a bare f64. That value then flows into chat_with_tools / chat_with_history, both of which take Option<f64> — so the call site genuinely fails with error[E0308]: mismatched types. I traced the introducing change to f992d42 (#7095), which matches your PR description. Removing the shadow lets the Option<f64> flow through untouched, exactly like the file's three other send_request callers.

🟢 What looks good — minimal patch, correct boundary

The fix is scoped to the single offending line and leaves #7095's prompt-guided-tools logic alone. That's the right altitude for a regression fix — you didn't reach for the larger refactor of changing send_request's signature to f64, which would have been the wrong direction since Option<f64> is the established contract across the trait and every caller. The validation evidence is thorough and honest, including the upfront disclosure that the pre-existing tokio::spawn disallowed-method lint at ollama.rs:1270 is unrelated test-module debt already on master, not something this diff touches.

🟢 What looks good — behaviour is genuinely lossless

I verified the downstream path: build_chat_request_with_think sets temperature: self.tuning.temperature_override.or(temperature), so when the per-call temperature is None and no operator override is set, options.temperature resolves to None and Ollama applies its native per-model default. That's the pre-#7095 pass-through semantics restored — no behavioural loss relative to the last shipping release.

🔵 Suggestion — the commit-message rationale is slightly off (no code change needed)

One small thing for the record, since this PR will be read later as the canonical explanation of why the removal is safe. The commit message says "default_temperature() is already applied downstream in build_chat_request_with_think, so the shadowing was redundant." That isn't literally what happens — build_chat_request_with_think applies temperature_override.or(temperature) and never calls default_temperature(). The accurate framing is that removing the shadow restores the pre-#7095 pass-through, where a None temperature defers to Ollama's native model default (rather than being coerced to TEMPERATURE_DEFAULT = 0.8 before send). The conclusion — safe and lossless — is correct; only the stated reason is imprecise. Worth noting because the two paths produce different values in the override = None, per-call = None case, and the restored behaviour (defer to Ollama) is the intended one.

For what it's worth, I also confirmed default_temperature() does not become dead code after this removal: it's a ModelProvider trait-impl method, so it stays reachable through the trait and clippy doesn't flag it — consistent with your clean cargo clippy -p zeroclaw-providers run.

Approving. Thanks for the fast turnaround on a build-breaker.

[tidux]

Reproduced the regression and the fix end-to-end against upstream/master at a1b641e01 plus this PR at 42dbcf0. cargo check -p zeroclaw-providers on master fails with exactly the error[E0308]: mismatched types ... expected Option<f64>, found f64 the PR body cites, pointing at the temperature argument inside chat()'s send_request call. Applying the one-line removal restores the build cleanly. So this is the right shape of fix for a build-breaking regression sitting on master right now.

The shadowing introduced by #7095 was both incorrect and redundant — build_chat_request_with_think already applies self.tuning.temperature_override.or(temperature) at the request-building seam (ollama.rs:445), and Options::temperature carries #[serde(skip_serializing_if = "Option::is_none")] (ollama.rs:142-143), so passing None through really does omit the field from the request body and defer to the Ollama model's own default. The PR body's claim of "no behavioural loss" holds under inspection.

🟢 What looks good — restoring callsite-uniformity

After this diff, all four ModelProvider chat surfaces on OllamaModelProvider (chat_with_system at ollama.rs:928, chat_with_history at ollama.rs:969, chat_with_tools at ollama.rs:1021, and chat at ollama.rs:1055) call send_request with the same temperature: Option<f64> flowing straight through. That uniformity is what makes the shadowing a real outlier — and it's also why the fix is one line rather than a structural change.

🟢 What looks good — scope discipline

The diff is exactly the one line that needs to move. No drive-by reformatting in chat(), no touching the prompt-guided-tools logic that #7095 actually intended to add, no opportunistic test edits. That keeps the bisect story and the rollback (git revert <sha>) trivially clean, which is the right posture for a hot-path build regression.

🟢 What looks good — template completeness for a risk: medium fix

The PR body's Validation Evidence reproduces under my hands byte-for-byte: cargo fmt -p zeroclaw-providers -- --check clean, cargo check -p zeroclaw-providers finishes, cargo clippy -p zeroclaw-providers --lib -- -D warnings clean, and cargo test -p zeroclaw-providers --lib ollama is 68 passed; 0 failed. The Rollback section gives the right command for a single-commit fix: PR. The Compatibility section is honest about backward compatibility — None semantics through to the wire are preserved.

🔵 Suggestion — the pre-existing tokio::spawn lint in the test module

The PR body correctly carves the use of a disallowed method tokio::spawn at ollama.rs:1270 out of scope, and that's the right call here — it sits inside mod tests and is unrelated to the temperature regression. Confirmed it's verbatim on upstream/master (tokio::spawn(async move { axum::serve(listener, app).await ... }) for a per-test mock HTTP server). Worth filing a tiny follow-up to either swap it for zeroclaw_spawn::spawn! or add a #[allow(clippy::disallowed_methods)] with a "test-local axum mock server" justification so future cargo clippy --all-targets runs aren't noisy here. No action needed on this PR.

🔵 Suggestion — small process curiosity for the maintainer log

Worth knowing for the v0.8.0 release retro: #7095 merged green at f992d4238 despite introducing this E0308, even though send_request: Option<f64> was already the signature on the merge-base. The plausible explanation is that the PR's last green CI ran against an earlier merge-base than the one that actually landed (auto-merge / merge-train ordering), and the squash never re-validated against current master. Calling this out only so that, if it keeps happening, the v0.8.0 tracker (#7112) gets a "require fresh CI before merge button" entry under stable-tier blockers. Nothing actionable on this PR.

Verdict

Approved on 42dbcf0. This is the kind of fix that should land as soon as a second reviewer signs off so the source-install path stops handing users an error[E0308]. The "Related #7095" link in the body is correct framing — this is a regression fix on #7095's commit train, not a revert.

[theonlyhennygod]

Review of PR #7231

Reviewed commit: 42dbcf0 (current head)
Reviewer: @theonlyhennygod
Review type: First review

I read the full diff, pulled the master copy of crates/zeroclaw-providers/src/ollama.rs to cross-check the surrounding logic (my local working tree carries a different in-flight fix for the same breakage, so I checked against master directly rather than my checkout), traced the temperature flow downstream, and read the live CI rollup including the Lint job log. This is the fix for the exact E0308 I flagged on #7223 an hour ago, so I had strong motivation to confirm it's the right one.

---

🟢 What looks good — correct, minimal, and the faithful restoration

Removing let temperature = temperature.unwrap_or(self.default_temperature()); is the right fix, and it's the right shape of fix. I verified the whole chain:

• The Option<f64> parameter now flows straight into send_request(.., temperature: Option<f64>, ..), which clears the E0308. Check (all features), Check (no default features), Check (32-bit), and Build x86_64-unknown-linux-gnu are all green on the head — the workspace compiles again.
• Temperature is still honoured downstream: ollama.rs:445 builds the request with temperature: self.tuning.temperature_override.or(temperature), so an explicit per-call value still wins, and None correctly falls through to Ollama's native model default. The two regression tests at ollama.rs:1492 (temperature_override_replaces_per_call_temperature) and ollama.rs:1517 (temperature_override_unset_passes_per_call_temperature) pin exactly this behavior.
• It makes the chat() call site consistent with the other three send_request callers in the file, which already thread the Option through unshadowed.

One thing worth stating explicitly for the record: this restores the pre-#7095 semantics (let None defer to Ollama). That's the behaviorally correct choice. An alternative fix that wraps the value as Some(temperature.unwrap_or(self.default_temperature())) would also compile, but it would silently change behavior — forcing the provider default temperature on every call where the caller passed None, instead of deferring to the model's own default. If any competing E0308 patch is in flight, this is the one to take. And to preempt a dead-code worry: dropping the only caller of default_temperature() does not orphan it, because it's a ModelProvider trait-method impl, not an inherent method — the compiler confirms this (the lib compiles clean with zero dead-code warnings).

---

🟢 What looks good — honest validation evidence and disclosure

The validation section is exactly what I want to see on an urgent break-fix: the reproduction of the original E0308 on a clean master worktree, the targeted cargo check/clippy/test runs, and — importantly — the upfront disclosure of the pre-existing tokio::spawn lint at ollama.rs:1270 as a known, unrelated issue. That disclosure is what let me immediately attribute the Lint failure correctly instead of pinning it on this diff. Separating "what I fixed" from "what was already broken" is precisely the discipline FND-005 §receiving/giving-feedback is built on.

---

🟡 Conditional — this fix unmasks a pre-existing Lint failure, so it doesn't make CI green on its own

The Lint job is red on the head, and I want to be precise about why, because it changes the merge plan. The failure is not in your diff — it's the disallowed-method ban from clippy.toml firing on the test-only tokio::spawn at ollama.rs:1271:

error: use of a disallowed method `tokio::spawn`
1270 |         let server = tokio::spawn(async move {
     = note: `-D clippy::disallowed-methods` implied by `-D warnings`
could not compile `zeroclaw-providers` (lib test) due to 1 previous error

On master this lint was masked: the lib didn't compile (E0308), so clippy never reached the test module. Your fix makes the lib compile, which lets clippy get far enough to hit the pre-existing violation. So the practical effect is: cargo build is unblocked (great, that's the headline), but the Lint required gate stays red until that tokio::spawn is also resolved.

That leaves two paths, and I'd like the team to pick one on the record rather than letting the PR sit half-green:

1. Fold the one-line test fix in here. Swap the tokio::spawn for ::zeroclaw_spawn::spawn!(...) (or a scoped #[allow(clippy::disallowed_methods)] on that test fn with a // pre-existing, tracked in #NNNN note). This keeps the PR's stated goal — "unblock the build" — fully delivered and lets it merge through the normal gate. Given the change is test-only and trivial, I think this is the cleaner option for an urgent unblock.
2. Keep the scope surgical and admin-merge. If the team prefers to hold this to the literal one-line E0308 fix, then this needs a tracked follow-up issue with an assignee for the tokio::spawn lint (a deferral without an owner is a wish, not a plan — FND-005 §conditional), plus an explicit decision to admin-merge once Build/Check are fully green, accepting Lint red on a pre-existing test-only violation.

Either is defensible; both improve on the current master, which doesn't compile at all. I just don't want "build unblocked" to be read as "CI green" when the gate will still be red.

---

Verdict

Approve. The change is correct, minimal, behaviorally faithful, and well-validated — I'm happy with the diff exactly as written and I don't want to slow an urgent break-fix. My approval is on the code. The 🟡 above is a merge-readiness condition, not a defect: please pick path 1 or 2 for the pre-existing tokio::spawn lint so the Lint gate's state is intentional before this lands.

Next steps:

• Decide the tokio::spawn path (fold-in vs. tracked-follow-up + admin-merge).
• Once Build aarch64-apple-darwin / Build x86_64-pc-windows-msvc / Test finish green, this is ready to go.

---

Foundations referenced:

• FND-005 (Contribution Culture) — feedback taxonomy, conditional deferrals require an assignee, separating the work from the person
• AGENTS.md — risk tiers, validation discipline, the tokio::spawn / zeroclaw_spawn::spawn! workspace ban

[WareWolf-MoonWall]

Reviewed head commit: 42dbcf0 — post-merge review completing the formal assignment.

Three thorough reviews are already on record. I've read all three, traced the full diff, and cross-checked the downstream temperature path in the local source. I won't re-raise settled points; I'll add what's still worth noting.

---

✅ Resolved — @theonlyhennygod's 🟡 Lint gate condition

@theonlyhennygod's review correctly identified that a pre-existing tokio::spawn disallowed-method lint at ollama.rs:1270 would remain red after a single-fix patch. That condition is resolved in the head commit. The diff at 42dbcf0 contains two changes, not one:

1. Remove let temperature = temperature.unwrap_or(self.default_temperature()); — clears the E0308.
2. Replace tokio::spawn(async move { … }) with zeroclaw_spawn::spawn!(async move { … }) — clears the lint gate.

The PR view confirms ✓ Checks passing. @theonlyhennygod's conditional no longer applies at the merged head; both paths they described (fold-in vs. admin-merge) converged on the same outcome. Noting this explicitly so the record is clean.

---

🟢 What looks good — downstream temperature semantics verified

I traced the full temperature chain against the local source to confirm the prior reviewers' behaviour claim holds:

• build_chat_request_with_think at L444 sets options.temperature to self.tuning.temperature_override.or(temperature). With temperature_override = None and a per-call None, this resolves to None.
• Options::temperature carries #[serde(skip_serializing_if = "Option::is_none")] — confirmed at L142-143 — so a None value is omitted from the JSON body entirely.
• Ollama therefore applies its native per-model default, which is the correct pre-#7095 pass-through semantics.

The two regression tests temperature_override_replaces_per_call_temperature (L1490) and temperature_override_unset_passes_per_call_temperature (L1515) pin these cases. The fix is genuinely lossless.

---

🟢 What looks good — the right fix, not the convenient fix

@theonlyhennygod named this explicitly, and it bears repeating: the alternative that compiles — wrapping as Some(temperature.unwrap_or(self.default_temperature())) — would also have cleared E0308, but it would silently force TEMPERATURE_DEFAULT = 0.8 on every call where the caller passes None, changing the wire semantics relative to every other provider. Removing the shadow instead restores the Option<f64> pass-through and leaves Ollama's own model default as the tie-breaker. That is the precise choice that makes this fix a restoration rather than a silent behaviour change.

---

🔵 Suggestion — process note for the v0.8.0 tracker

@tidux flagged a process curiosity worth carrying forward: #7095 introduced E0308 despite shipping green CI, likely because the PR's last CI run was against an earlier merge-base than the one that actually landed. If this pattern recurs, the v0.8.0 milestone tracker (#7112) is the right place for a "require fresh CI on the merge-base commit before the merge button" note. Nothing actionable on this PR; surfacing it so the signal doesn't get lost in the review archive.

---

Approving. The fix is correct, complete, and already shipping. Thanks for the fast turnaround on a build-breaker that was blocking the source-install path.