Repository navigation
Conversation
LiteLLMModel._query only forwarded the temperature override to _single_query in its `n is None` branch. BinaryTrajectoryComparison.get_action calls query(messages, temperature=comparison_temperature), but query defaults n to 1, so it took the loop branch and every comparison query was sent to litellm at the model's configured temperature instead of the documented override. Forward temperature inside the loop as well, which also makes it apply to multi-sample queries.
chessing234
left a comment
chessing234
left a comment
There was a problem hiding this comment.
Reviewed temperature forwarding through the n-sample loop and the action-sampler caller. The focused sampler/model suites pass (5 tests), including comparison_temperature=0.9.
No blocking issue found in the reviewed diff. Validation was limited to the focused checks described above; the full integration suite was not run.
chessing234
left a comment
chessing234
left a comment
There was a problem hiding this comment.
Requesting changes as a reviewer-directed hold pending specific revision requirements. No additional technical defect is asserted in this follow-up; the findings and validation limits in my earlier review still apply.
chessing234
left a comment
chessing234
left a comment
There was a problem hiding this comment.
Reviewed temperature forwarding and the action-sampler comparison caller. The sampler/model suites reported 5 passed, including the comparison-temperature regression.
No new blocking defect found in this review. These were focused local checks; the full integration suite was not run.
Reference Issues/PRs
Fixes #1556
See also #1484 — it reports a different defect in the same
_querymethod (nis not forwarded to the provider). That behaviour is unchanged by this PR.What does this implement/fix? Explain your changes.
Summary.
BinaryTrajectoryComparisonConfig.comparison_temperatureis documented as an override of the model temperature, but it never reached the API: the pairwise-comparison query ran at the model's owntemperature.Why.
BinaryTrajectoryComparison.get_actionpasses the override atsweagent/agent/action_sampler.py:288:LiteLLMModel.querydefaultsnto1(sweagent/agent/models.py:793), so_querytook its loop branch — and that branch forwarded only the message list:_single_querythen fell back totemperature=self.config.temperature if temperature is None else temperature(sweagent/agent/models.py:725), i.e. the model temperature. Then is Nonebranch directly above already forwarded the argument correctly (sweagent/agent/models.py:786-787), which is why only the sampler path was affected.Changes.
sweagent/agent/models.py: one line inLiteLLMModel._query— forwardtemperatureinto_single_queryinside the loop, so an explicitly passed override is honoured for every sampled call and not only whenn is None.tests/test_action_sampler.py: new regression test driving the realBinaryTrajectoryComparisonwith a realLiteLLMModel, patching onlylitellm.completionso no API key is needed. It asserts the two candidate-sample calls keep the model temperature (0.0) and the comparison call reacheslitellm.completionwith0.9, the configured override.All other callers of
query(sweagent/agent/action_sampler.py:83,89,251,259,sweagent/agent/agents.py:1042,sweagent/agent/reviewer.py:284,360,429) pass notemperature, so for themtemperature=Noneis forwarded and they keep the configured model temperature exactly as before.Testing
pytest tests/test_action_sampler.py -q(unpatchedmodels.py)1 failed—assert 0.0 == 0.9attests/test_action_sampler.py:62pytest tests/test_action_sampler.py -q1 passedpytest tests/test_models.py tests/test_agent.py -q15 passed, 2 xfailedruff check+ruff format --checkon both filesAll checks passed!/2 files already formattedtyposon both filesThe red run was produced by checking out the unpatched
sweagent/agent/models.pyat3ea751cwhile keeping the new test: the comparison call reacheslitellm.completionwith0.0(the model temperature) instead of0.9.