fix(litellm): make temperature=0 override opt-in via explicit REFLEXIO_LLM_SEED - #62
Conversation
…O_LLM_SEED The seed-injection block defaulted REFLEXIO_LLM_SEED to "42" and then forced temperature=0.0 on every non-restricted model, silently overriding any caller-configured temperature. That made `LiteLLMConfig(temperature=0.3)` and all other custom temperature values inert in production. Split the two halves: - `seed=42` is still injected by default (cheap, harmless on providers that ignore it, helpful on providers that honor it). - The companion `temperature=0.0` override now fires only when the operator explicitly opts in via `REFLEXIO_LLM_SEED=<int>`. Caller-configured temperature flows through unchanged otherwise. Test changes: - Renamed `test_non_restricted_model_defaults_to_deterministic_temperature` to `test_non_restricted_model_default_injects_seed_only` and updated its assertion to expect the configured temperature (0.3) rather than 0.0. - Added `test_explicit_seed_env_forces_temperature_zero` to cover the opt-in path. - Patched `_cheap_should_run_reject` in `test_should_run_before_extraction_combines_all_extractor_criteria` so the new pre-filter short-circuit doesn't pre-empt the mocked LLM call.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe LiteLLM client now always injects an integer ChangesSeed and Temperature Determinism Updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@reflexio/server/llm/litellm_client.py`:
- Around line 773-780: The code currently skips setting params["seed"] when
REFLEXIO_LLM_SEED is present but not an int; update the try/except in the block
that reads seed_explicit/seed_raw so that on ValueError you set params["seed"]
to the default (int("42") or the same default used when REFLEXIO_LLM_SEED is
absent) and log a warning that the value was invalid and the default is being
used; reference the variables seed_explicit, seed_raw, params, and self.logger
so you locate and modify the seed parsing/except branch to always inject a seed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 402224de-712c-406a-9ff4-0a7afef386c4
📒 Files selected for processing (3)
reflexio/server/llm/litellm_client.pytests/server/llm/test_litellm_client_unit.pytests/server/services/test_profile_generation_service.py
Addresses CodeRabbit review: the previous ValueError branch logged a warning but skipped setting params["seed"], contradicting the "seed is always injected" contract for the case REFLEXIO_LLM_SEED="abc" (or any other non-int). Inject the default seed=42 instead so the contract holds for all inputs. The temperature=0 override still fires (operator explicitly opted into determinism, even if they typoed the value). Added test_invalid_seed_env_falls_back_to_default to cover the fallback.
Summary
The seed-injection block in
litellm_client.pywas silently overriding any caller-configuredtemperaturewith0.0on every non-restricted model. The fallbackseed_env = os.environ.get("REFLEXIO_LLM_SEED") or "42"meant the override always fired —LiteLLMConfig(temperature=0.3)and friends never reached the provider.Behavior change
seed=42is still injected by default. Cheap, harmless on providers that ignore it, helpful on providers that honor it.temperature=0.0override now fires only when the operator explicitly opts in viaREFLEXIO_LLM_SEED=<int>. Caller-configured temperature flows through unchanged otherwise.To restore the prior "always deterministic" behavior in a deployment, set
REFLEXIO_LLM_SEED=42in the environment.Tests
test_non_restricted_model_defaults_to_deterministic_temperature→test_non_restricted_model_default_injects_seed_only; asserts seed is injected but configured temperature flows through.test_explicit_seed_env_forces_temperature_zeroto cover the opt-in path withmonkeypatch.setenv("REFLEXIO_LLM_SEED", "7")._cheap_should_run_rejectintest_should_run_before_extraction_combines_all_extractor_criteriaso the new cheap pre-filter short-circuit doesn't pre-empt the mocked LLM call.Test plan
uv run pytest tests/server/llm/test_litellm_client_unit.py::TestTemperatureRestriction— all 13 pass.uv run pytest tests/server/services/test_profile_generation_service.py::test_should_run_before_extraction_combines_all_extractor_criteria— passes.mainare unchanged by this PR (orthogonal — same count, same names, with or without the diff applied).Summary by CodeRabbit
Bug Fixes
Tests