fix(config): wire prm.temperature from config.yaml to PRMScorer - #58
Open
grimmjoww wants to merge 2 commits into
Open
fix(config): wire prm.temperature from config.yaml to PRMScorer#58grimmjoww wants to merge 2 commits into
grimmjoww wants to merge 2 commits into
Conversation
The prm_temperature field existed on SkillClawConfig but was never populated from the prm section of config.yaml, so PRM scoring always sent the hard default 0.6 upstream. Models with sampling constraints (e.g. kimi k3, which only accepts temperature=1) rejected every PRM vote with HTTP 400, silently disabling PRM scoring. Map prm.temperature (already settable via 'skillclaw config prm.temperature <value>') through to SkillClawConfig, keeping 0.6 as the default when unset.
Review feedback on the initial wiring: float(prm.get('temperature',
0.6) or 0.6) silently rewrote an explicit temperature: 0 to 0.6 —
the same 'user setting ignored' defect class this PR exists to fix.
Use a None-check guard instead, and add temperature to the defaults
dict so the key is self-documenting in seeded configs.
Adds tests/test_prm_temperature_config.py pinning: configured values
pass through (1, 0.2), explicit 0 is honored, absent key defaults to
0.6. 4/4 passing.
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
1 participant
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.
Problem
SkillClawConfig.prm_temperatureexists (default 0.6) butConfigStore.to_skillclaw_config()never populates it from theprmsection ofconfig.yaml— every other prm key (provider,url,model,api_key,enabled) is mapped, temperature is silently dropped. PRM scoring therefore always sendstemperature=0.6upstream.Models with sampling constraints reject this. kimi k3 only accepts
temperature=1, so every PRM vote fails:All votes fail → PRM scoring is silently disabled for any temperature-restricted upstream.
Fix
Two lines: read
prm.temperature(default 0.6, unchanged) into_skillclaw_config()and pass it through toSkillClawConfig.The key is already user-settable via the existing generic CLI —
skillclaw config prm.temperature 1round-trips throughConfigStore.get/set— so no CLI changes are needed; the setting is now actually honored.Not hard-coded: each deployment sets the value its model requires (
prm.temperature: 1for k3, anything else for other models); 0.6 remains the default.Verification
skillclaw config prm.temperature 1→to_skillclaw_config().prm_temperature == 1.0(verified against a live config withprm.model: k3)invalid temperature400s in the PRM scorer path