Add temperature: Option<f32> to api::MessageRequest - #24
Conversation
Restores the determinism regression introduced in PR #23 when model_router.rs migrated to api::OpenAiCompatClient. The old Ollama-native payload pinned temperature: 0.0; the migrated path lost that because MessageRequest didn't expose a temperature knob. Changes - api::MessageRequest gains `pub temperature: Option<f32>` with `#[serde(default, skip_serializing_if = "Option::is_none")]` — backward compatible on the wire (None ⇒ field omitted). - api::MessageRequest::with_temperature(f32) builder method. - openai_compat::build_chat_completion_request copies `temperature` into the outgoing JSON payload when set; AnthropicClient's request body already goes through `serde_json::to_value` so the field flows through automatically. - src/model_router.rs::llm_classify now pins `temperature: Some(0.0)` again. - All 28 MessageRequest struct literals across src/, crates/, and tests/ now include `temperature: None` (default behaviour matches pre-PR semantics). - New unit tests: - types::temperature_serializes_when_set_and_is_omitted_when_none - openai_compat::temperature_is_included_in_payload_when_set Test plan - `cargo test -p api --lib types::` → 3 passed - `cargo test -p api --lib openai_compat::tests::` → 9 passed (including new temperature test) - `cargo test -p api --test openai_compat_integration` → 5 passed - `cargo test -p rusty-claude-cli --test api_integration` → 3 passed - `cargo check --workspace --tests --exclude rustcode` → clean - `cargo check -p rustcode` blocked locally by ort-sys CDN sandbox; CI verifies. Pre-existing failures NOT addressed: - api::client::tests::resolves_existing_and_grok_aliases (opus alias now resolves to claude-opus-4-7, test still asserts -4-6) - 3 prompt_cache disk-cleanup tests Both verified to fail on main as well — out of scope for this PR. https://claude.ai/code/session_014DMg4gxA8VZv1MucmM7PV9
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (18)
📝 WalkthroughWalkthroughThis PR introduces an optional ChangesTemperature Field Addition and Provider Integration
Callers Explicitly Setting Temperature
Model Classifier Determinism
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
Adds
pub temperature: Option<f32>toapi::MessageRequest, fixing the determinism regression introduced in PR #23 whenmodel_router.rsmigrated toapi::OpenAiCompatClient. The old Ollama-native payload pinnedtemperature: 0.0; the migrated path lost that becauseMessageRequestdidn't expose a temperature knob.model_router.rs::llm_classifynow pinstemperature: Some(0.0)again.Changes
api::MessageRequestgainspub temperature: Option<f32>with#[serde(default, skip_serializing_if = "Option::is_none")]— backward-compatible on the wire (None⇒ field omitted).api::MessageRequest::with_temperature(f32)builder method.openai_compat::build_chat_completion_requestcopiestemperatureinto the outgoing JSON payload when set.AnthropicClient's request body already goes throughserde_json::to_value, so the field flows through automatically without further changes.src/model_router.rs::llm_classifynow pinstemperature: Some(0.0)again, with the module comment updated to reflect that determinism is restored.MessageRequeststruct literals acrosssrc/,crates/, andtests/now includetemperature: None.types::temperature_serializes_when_set_and_is_omitted_when_noneopenai_compat::temperature_is_included_in_payload_when_setTest plan
cargo test -p api --lib types::→ 3 passedcargo test -p api --lib openai_compat::tests::→ 9 passed (including new temperature test)cargo test -p api --test openai_compat_integration→ 5 passedcargo test -p rusty-claude-cli --test api_integration→ 3 passedcargo check --workspace --tests --exclude rustcode→ cleancargo check -p rustcode(blocked locally by theort-sysCDN sandbox restriction; CI verifies)Pre-existing failures NOT addressed in this PR
Both verified to fail on
mainas well — out of scope:api::client::tests::resolves_existing_and_grok_aliases(opus alias now resolves toclaude-opus-4-7, test still asserts-4-6)prompt_cachedisk-cleanup tests that race onCLAUDE_CONFIG_HOMEhttps://claude.ai/code/session_014DMg4gxA8VZv1MucmM7PV9
Generated by Claude Code
Summary by CodeRabbit
New Features
Improvements