fix temperature error.#27
Conversation
|
@askserge review this |
There was a problem hiding this comment.
🤗 Serge says:
This PR correctly adds graceful fallback when an LLM endpoint rejects the temperature parameter (e.g., newer Claude models that deprecate it). The implementation introduces a new _TemperatureUnsupported exception, a provider-neutral heuristic _is_temperature_rejection to detect such400s, and refactors the retry loop in complete() to strip the offending field and retry once. The tests cover both the happy-path fallback and the important negative case where a value-range error does not trigger the fallback. No issues found.
serge v0.1.0 · model: moonshotai/Kimi-K2.6 · 3 LLM turns · 2 tool calls · 37.8s · 22080 in / 7375 out tokens
|
@askserge the fix is sound, but can we make the fix more generic. e.g. capture and fallback for any unsupported argument |
There was a problem hiding this comment.
🤗 Serge says:
The fix correctly detects the temperature deprecation 400 and retries without the parameter, which resolves the reported failure. The new tests cover both the fallback path and the case where a value-range error must not be silently swallowed.
Suggestion (non-blocking): As the human reviewer noted, the current implementation is hardcoded to temperature. Consider generalizing the exception and the checker so any unsupported payload argument can be caught and removed without adding new exception types and except blocks for each field. A single _ParameterUnsupported(param) exception and a parameterized _is_parameter_rejection(param, body_preview) check would make the while True loop in complete() naturally extensible for future deprecations (e.g. max_tokens, top_p, etc.).
serge v0.1.0 · model: moonshotai/Kimi-K2.6 · 5 LLM turns · 4 tool calls · 87.9s · 42433 in / 24891 out tokens
| response body preview for logging.""" | ||
|
|
||
|
|
||
| class _TemperatureUnsupported(Exception): |
There was a problem hiding this comment.
The fix is sound for the immediate issue, but the exception is hardcoded to temperature. To make this generic for any unsupported argument as requested, consider a single _ParameterUnsupported(param: str) exception (and a matching _is_parameter_rejection(param, body_preview) helper) so the while True loop in complete() can catch the generic exception and pop whatever parameter it carries, avoiding new exception types and handler blocks for each new deprecated field.
tarekziade
left a comment
There was a problem hiding this comment.
Let's try a more generic approach, as explained by Serge
Fix: