fix(config): honor default_temperature config while running "zeroclaw agent" without temperature parameter#3067
Merged
theonlyhennygod merged 13 commits intozeroclaw-labs:masterfrom Mar 11, 2026
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Note
|
| Cohort / File(s) | Summary |
|---|---|
CLI / Runtime src/main.rs |
Agent variant temperature: f64 → temperature: Option<f64>; removed local range check and now calls config::schema::validate_temperature; compute final_temperature = temperature.unwrap_or(config.default_temperature) and pass to agent::run; updated CLI help and tests for Some/None. |
Configuration Schema src/config/schema.rs |
Added pub const TEMPERATURE_RANGE: RangeInclusive<f64> = 0.0..=2.0, DEFAULT_TEMPERATURE helper, deserialize_temperature (serde), and pub fn validate_temperature(f64) -> Result<f64,String>; Config.default_temperature annotated with #[serde(default = "default_temperature", deserialize_with = "deserialize_temperature")]; env var ZEROCLAW_TEMPERATURE now validated with warnings on invalid/ out-of-range values and ignored when invalid. |
Tests tests/config_schema.rs |
Refactored config_empty_toml test to assert default (0.7); added config_out_of_range_temperature_fails and config_negative_temperature_fails to validate deserialization rejects invalid values; added tests for CLI Agent temperature parsing and fallback to config/default. |
Manifest Cargo.toml, manifest_file |
Updated manifest metadata/files (lines changed noted). |
Sequence Diagram(s)
sequenceDiagram
participant CLI as CLI (user)
participant Runtime as Runtime (main)
participant Config as Config Loader
participant Agent as Agent Runner
CLI->>Runtime: invoke `agent` (temperature: Some or None)
Runtime->>Config: load configuration (includes default_temperature)
Config-->>Runtime: return validated Config
Runtime->>Runtime: parsed_temp = CLI.temperature
alt parsed_temp is Some
Runtime->>Agent: run(final_temperature = parsed_temp)
else parsed_temp is None
Runtime->>Runtime: final_temperature = config.default_temperature
Runtime->>Agent: run(final_temperature = config.default_temperature)
end
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title accurately and clearly describes the main fix: honoring the default_temperature config when running the agent command without explicit temperature parameter. |
| Linked Issues check | ✅ Passed | The code changes directly address issue #3033 by implementing temperature fallback logic that uses config.default_temperature when --temperature is not provided, and adding centralized validation that enforces the valid range (0.0..=2.0). |
| Out of Scope Changes check | ✅ Passed | All changes are scoped to the stated objective: making Agent command temperature optional, implementing fallback logic to honor config.default_temperature, and centralizing temperature validation in config schema with tests. |
| Docstring Coverage | ✅ Passed | Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%. |
| Description check | ✅ Passed | The pull request description is comprehensive and follows most of the required template with detailed validation evidence, edge case testing, and screenshots demonstrating the fix. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
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 @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/main.rs (2)
190-192: Document the config fallback inzeroclaw agent --help.Now that omitting
--temperaturemeans “useconfig.default_temperature”, the flag help should say that explicitly so the CLI contract is visible to users.✏️ Suggested wording
- /// Temperature (0.0 - 2.0) + /// Temperature override (0.0 - 2.0). Defaults to config.default_temperature when omitted. #[arg(short, long, value_parser = parse_temperature)] temperature: Option<f64>,Based on learnings, Treat config keys and CLI commands as public API contract; document defaults, compatibility impact, and migration/rollback path for changes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.rs` around lines 190 - 192, Update the CLI help for the temperature flag so it documents the new config fallback: modify the #[arg(...)] on the temperature field (the temperature: Option<f64> argument parsed with parse_temperature) to include a help string that states the valid range (0.0–2.0) and explicitly that omitting --temperature will use config.default_temperature; keep the existing short/long and value_parser settings and ensure the messaging mentions the config key name "config.default_temperature" so users see the CLI ↔ config contract.
779-795: Make the effective-temperature rule directly testable.The fix is correct here, but the new tests only prove Clap returns
Some/None. If this line ever regresses to a hardcoded value again, those tests still pass. A tiny helper for resolving the effective temperature would keep the fallback localized and let you unit-test the actual behavior.♻️ Minimal extraction
+fn resolve_agent_temperature(cli_temperature: Option<f64>, config: &Config) -> f64 { + cli_temperature.unwrap_or(config.default_temperature) +} + ... - // Implement temperature fallback logic: - // 1. Use --temperature if provided - // 2. Use config.default_temperature if --temperature not provided - // 3. Use hardcoded 0.7 if config.default_temperature not set (though config always has default) - let final_temperature = temperature.unwrap_or(config.default_temperature); + let final_temperature = resolve_agent_temperature(temperature, &config);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.rs` around lines 779 - 795, Extract the temperature fallback logic into a small pure helper function (e.g., resolve_effective_temperature or effective_temperature) that takes Option<f64> temperature and &Config (or f64 default_temperature) and returns the resolved f64; replace the inline unwrap_or call that sets final_temperature with a call to that helper (referencing temperature, config.default_temperature, and final_temperature) so the rule is localized and then write unit tests for the helper covering Some, None with config default, and None with a fallback default to ensure behavior is directly testable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/main.rs`:
- Around line 190-192: Update the CLI help for the temperature flag so it
documents the new config fallback: modify the #[arg(...)] on the temperature
field (the temperature: Option<f64> argument parsed with parse_temperature) to
include a help string that states the valid range (0.0–2.0) and explicitly that
omitting --temperature will use config.default_temperature; keep the existing
short/long and value_parser settings and ensure the messaging mentions the
config key name "config.default_temperature" so users see the CLI ↔ config
contract.
- Around line 779-795: Extract the temperature fallback logic into a small pure
helper function (e.g., resolve_effective_temperature or effective_temperature)
that takes Option<f64> temperature and &Config (or f64 default_temperature) and
returns the resolved f64; replace the inline unwrap_or call that sets
final_temperature with a call to that helper (referencing temperature,
config.default_temperature, and final_temperature) so the rule is localized and
then write unit tests for the helper covering Some, None with config default,
and None with a fallback default to ensure behavior is directly testable.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/config/schema.rs (1)
272-274: Reuse this helper inConfig::default()as well.Line 3603 still hardcodes
0.7, so the default now lives in two places. Calling this helper there would keep the config contract from drifting later.Based on learnings, treat config keys as public contract: document defaults, compatibility impact, and migration/rollback path.♻️ Small follow-up
- default_temperature: 0.7, + default_temperature: default_temperature(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config/schema.rs` around lines 272 - 274, The default temperature helper default_temperature() is defined but not used in Config::default(), which still hardcodes 0.7; update Config::default() to call default_temperature() (instead of the literal 0.7) so the default value is defined in one place (refer to the Config::default() implementation and the default_temperature() function), ensuring the config default remains consistent and documented as the public contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main.rs`:
- Around line 779-790: The config's default_temperature can be out-of-range
because TOML deserialization and apply_env_overrides() don't validate it, so add
a shared validator that enforces 0.0..=2.0: implement a custom serde
deserializer (e.g., deserialize_temperature) and annotate
Config::default_temperature with it so TOML loads fail on invalid values; reuse
the same validation in parse_temperature for CLI parity and call it from
apply_env_overrides() to return an error (instead of silently ignoring) when
ZEROCLAW_TEMPERATURE is out of range; finally, ensure any post-deserialize paths
(config loader before agent::run) check/propagate this validation error so
final_temperature used in main.rs is guaranteed valid.
---
Nitpick comments:
In `@src/config/schema.rs`:
- Around line 272-274: The default temperature helper default_temperature() is
defined but not used in Config::default(), which still hardcodes 0.7; update
Config::default() to call default_temperature() (instead of the literal 0.7) so
the default value is defined in one place (refer to the Config::default()
implementation and the default_temperature() function), ensuring the config
default remains consistent and documented as the public contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9fa71b11-b86f-4a64-ac64-53da7dd5a960
📒 Files selected for processing (3)
src/config/schema.rssrc/main.rstests/config_schema.rs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/config/schema.rs`:
- Around line 275-305: Config currently only enforces temperature bounds for
default_temperature via deserialize_temperature, leaving
DelegateAgentConfig.temperature unchecked; update Config::validate() to iterate
all agents and, for each DelegateAgentConfig.temperature (the Option<f64>), call
validate_temperature(value) and bail/return an error if out of range (use
anyhow::bail! or propagate a clear Err with a message like "agent <name>
temperature X out of range"), so invalid config like `[agents.foo] temperature =
6` fails at load time; you can alternatively add a custom deserializer for
Option<f64> that reuses deserialize_temperature, but the simplest fix is adding
the explicit per-agent checks in Config::validate() referencing
DelegateAgentConfig.temperature and validate_temperature.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9fe5954b-8076-4fcf-bc3e-0c4b61e2ca05
📒 Files selected for processing (1)
src/config/schema.rs
Contributor
Author
|
@JordanTheJet my PR is ready for review. Requesting a review please. :) |
kunalk16
changed the title
…x-honor-default-temperature-agent-command
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main.rs (1)
779-790: Extract the resolver and test that shared code path.The new fallback tests replay
user_temperature.unwrap_or(config.default_temperature)instead of exercising the production branch at Line 779, so they would still pass if this dispatch regressed. A tiny helper also makes it easy to cover the missing CLI-over-config precedence and invalid-input cases.♻️ Suggested refactor
+fn resolve_agent_temperature(cli_temperature: Option<f64>, config: &Config) -> f64 { + cli_temperature.unwrap_or(config.default_temperature) +} + ... - let final_temperature = temperature.unwrap_or(config.default_temperature); + let final_temperature = resolve_agent_temperature(temperature, &config);- let user_temperature: Option<f64> = None; - let final_temperature = user_temperature.unwrap_or(config.default_temperature); + let final_temperature = resolve_agent_temperature(None, &config); assert!((final_temperature - 1.5).abs() < f64::EPSILON); } #[test] fn agent_fallback_uses_hardcoded_when_config_uses_default() { let config = Config::default(); - let user_temperature: Option<f64> = None; - let final_temperature = user_temperature.unwrap_or(config.default_temperature); + let final_temperature = resolve_agent_temperature(None, &config); assert!((final_temperature - 0.7).abs() < f64::EPSILON); } + + #[test] + fn agent_fallback_prefers_cli_temperature_over_config_default() { + let mut config = Config::default(); + config.default_temperature = 1.5; + + let final_temperature = resolve_agent_temperature(Some(0.5), &config); + + assert!((final_temperature - 0.5).abs() < f64::EPSILON); + } + + #[test] + fn agent_command_rejects_out_of_range_temperature() { + let err = Cli::try_parse_from(["zeroclaw", "agent", "--temperature", "6"]) + .expect_err("out-of-range temperature should fail"); + + assert!(err.to_string().contains("out of range")); + }Also applies to: 2008-2031
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.rs` around lines 779 - 790, Extract the temperature resolution into a small helper (e.g., resolve_temperature) that encapsulates the precedence logic currently expressed by temperature.unwrap_or(config.default_temperature) and any validation for invalid inputs; replace the inline computation of final_temperature in main.rs with a call to this helper before invoking agent::run so the same code path is exercised in production, and add unit tests that call resolve_temperature directly to cover CLI-over-config precedence, the config fallback (config.default_temperature), and invalid-input cases; reference the symbols temperature, final_temperature, config.default_temperature, agent::run, and the new resolve_temperature helper when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/main.rs`:
- Around line 779-790: Extract the temperature resolution into a small helper
(e.g., resolve_temperature) that encapsulates the precedence logic currently
expressed by temperature.unwrap_or(config.default_temperature) and any
validation for invalid inputs; replace the inline computation of
final_temperature in main.rs with a call to this helper before invoking
agent::run so the same code path is exercised in production, and add unit tests
that call resolve_temperature directly to cover CLI-over-config precedence, the
config fallback (config.default_temperature), and invalid-input cases; reference
the symbols temperature, final_temperature, config.default_temperature,
agent::run, and the new resolve_temperature helper when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4110bc0f-c923-4c18-8b30-ca6f521a57a6
📒 Files selected for processing (3)
src/config/schema.rssrc/main.rstests/config_schema.rs
…/github.com/kunalk16/zeroclaw into fix-honor-default-temperature-agent-command
Contributor
Author
|
@JordanTheJet Gentle reminder, my PR is ready for review. Requesting a review please. |
…x-honor-default-temperature-agent-command
3 tasks
…x-honor-default-temperature-agent-command
…x-honor-default-temperature-agent-command
…x-honor-default-temperature-agent-command
lantrinh1999
pushed a commit
to lantrinh1999/zeroclaw-1
that referenced
this pull request
Mar 18, 2026
…-temperature-agent-command fix(config): honor default_temperature config while running "zeroclaw agent" without temperature parameter
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Describe this PR in 2-5 bullets:
masterfor all contributions):master'default_temperatureconfig from config.toml is not honored when we execute thezeroclaw agentcommand without the --temperaturezeroclaw agent, we have to pass --temperature parameter otherwise it defaults to the hardcoded 0.7 and the LLM chat completion call failszeroclaw agentcommandzeroclaw agentcommandLabel Snapshot (required)
risk: low|medium|high):lowsize: XS|S|M|L|XL, auto-managed/read-only):XScore|agent|channel|config|cron|daemon|doctor|gateway|health|heartbeat|integration|memory|observability|onboard|provider|runtime|security|service|skillforge|skills|tool|tunnel|docs|dependencies|ci|tests|scripts|dev, comma-separated):config<module>: <component>, for examplechannel: telegram,provider: kimi,tool: shell):config:default_temperaturetrusted contributor|experienced contributor|principal contributor|distinguished contributor, auto-managed/read-only; author merged PRs >=5/10/20/50):auto-managed/read-onlyChange Metadata
bug|feature|refactor|docs|security|chore):bugruntime|provider|channel|memory|security|ci|docs|multi):runtimeLinked Issue
Supersede Attribution (required when
Supersedes #is used)#<pr> by @<author>, one per line):Co-authored-bytrailers added for materially incorporated contributors? (Yes/No)No, explain why (for example: inspiration-only, no direct code/design carry-over):\n): (Pass/Fail)Validation Evidence (required)
Commands and result summary:
cargo fmt --all -- --check cargo clippy --all-targets -- -D warnings cargo testcargo fmt --all -- --check -> Fixed all issues
cargo clippy --all-targets -- -D warnings -> Fixed all issues
cargo test -> All unit tests are passing
screenshotSecurity Impact (required)
Yes/No)NoYes/No)NoYes/No)NoYes/No)NoYes, describe risk and mitigation:Privacy and Data Hygiene (required)
pass|needs-follow-up):passCompatibility / Migration
Yes/No)Not applicableYes/No)Not applicableYes/No)Not applicablei18n Follow-Through (required when docs or user-facing wording changes)
Yes/No)Not applicableYes, locale navigation parity updated inREADME*,docs/README*, anddocs/SUMMARY.mdfor supported locales (en,zh-CN,ja,ru,fr,vi)? (Yes/No)Yes, localized runtime-contract docs updated where equivalents exist (minimum forfr/vi:commands-reference,config-reference,troubleshooting)? (Yes/No/N.A.)Yes, Vietnamese canonical docs underdocs/i18n/vi/**synced and compatibility shims underdocs/*.vi.mdvalidated? (Yes/No/N.A.)No/N.A., link follow-up issue/PR and explain scope decision:Human Verification (required)
What was personally validated beyond CI: Manual testing done
Validation screenshots:
Previously before the fix:
After the fix:
default_temperature set to 0.7value from temperature parameter is honoreddefault_temperature set to 1default_temperature config missing defaults to 0.7default_temperature out of rangeSide Effects / Blast Radius (required)
zeroclaw agentcommandAgent Collaboration Notes (recommended)
Github CopilotAGENTS.md+CONTRIBUTING.md):Rollback Plan (required)
PR Revert & ReleaseRisks and Mitigations
List real risks in this PR (or write
None).NoneSummary by CodeRabbit
New Features
Behavior Change
Tests