mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-11 03:38:38 +00:00
fix(clinepass): report the upstream finish reason instead of inferring truncation
`_correct_truncated_finish_reason` rewrote a single-choice response's `finish_reason` from `stop` to `length` whenever completion usage reached the requested cap. Two independent reviewers flagged it, and it is unsound: * usage reaching the cap is a token count, not a reason for stopping. A natural completion, or a stop-sequence match, can land exactly on the cap, and the heuristic then mislabels a successful response as truncated -- which can provoke spurious continuation requests in callers; * it read no explicit provider truncation signal, so it was pure inference; * it was inconsistent. Real streaming never reaches `transform_response` -- `BaseLLMHTTPHandler` delegates to `get_model_response_iterator()` -- so a streamed response kept the provider's `stop` while the identical non-streamed response was rewritten to `length`. The original incident is recorded in a comment on `transform_response` so it is not lost: ClinePass was once observed returning `stop` on a completion cut off at 4000 tokens, and follow-up probes on 2026-08-22 did not reproduce it. Tests: the three end-to-end rewrite tests become regression tests asserting an explicit `stop` survives at and above the cap, including through the `max_completion_tokens` -> `max_tokens` mapping path. The streaming test now appends a terminal finish-reason chunk and asserts it survives, which closes the streaming/non-streaming asymmetry that motivated the change. Ten tests that only exercised the removed helper's internals (non-positive cap filtering, missing usage, multi-choice guard, float coercion) are deleted with it: 46 -> 36 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Daniel JB Clark
parent
69ecca0c7b
commit
edf31323fd
2 changed files with 33 additions and 162 deletions
|
|
@ -49,71 +49,6 @@ CLINEPASS_MODEL_PREFIX: Final = "cline-pass/"
|
|||
_BODY_SPECIFIC_HEADERS: Final = ("content-length", "content-encoding")
|
||||
|
||||
|
||||
def _as_positive_number(value: Any) -> float | None:
|
||||
"""Return ``value`` as a positive number, or ``None`` if it is not one.
|
||||
|
||||
``bool`` is rejected explicitly: it is a subclass of ``int``, and ``True``
|
||||
would otherwise read as a cap of 1.
|
||||
"""
|
||||
if isinstance(value, bool) or not isinstance(value, (int, float)):
|
||||
return None
|
||||
if value <= 0:
|
||||
return None
|
||||
return float(value)
|
||||
|
||||
|
||||
def _correct_truncated_finish_reason(response: ModelResponse, request_data: dict) -> ModelResponse:
|
||||
"""Report a truncated ClinePass completion as ``length``, not ``stop``.
|
||||
|
||||
This is a defensive correction for an upstream bug in which ClinePass
|
||||
returned ``finish_reason: "stop"`` on a completion that had actually been
|
||||
cut off by ``max_tokens``: a request capped at 4000 came back with
|
||||
``completion_tokens == 4000`` and still claimed a natural stop. Callers that
|
||||
trust ``finish_reason`` -- the documented way to detect truncation -- cannot
|
||||
then distinguish a complete answer from a guillotined one.
|
||||
|
||||
Re-probing the live API later (2026-08-22, both streaming and non-streaming,
|
||||
caps of 2000 and 4000, across both the ``cline-pass/`` and the fallback
|
||||
namespace) did *not* reproduce the misreport: every capped response
|
||||
correctly returned ``length``. The upstream bug appears to have been fixed,
|
||||
or to be intermittent. This correction is therefore kept as a cheap safety
|
||||
net rather than as a workaround for a currently-observable defect, and it is
|
||||
deliberately conservative:
|
||||
|
||||
- only an upstream ``stop`` is ever rewritten; ``length`` is already right,
|
||||
- only when usage shows the cap was actually reached,
|
||||
- and only for single-choice responses. ``usage.completion_tokens`` is an
|
||||
aggregate across all choices while ``max_tokens`` is a per-choice limit,
|
||||
so with ``n > 1`` the aggregate cannot identify *which* choice was
|
||||
truncated -- two naturally-finished 60-token choices under a cap of 100
|
||||
would otherwise both be relabelled ``length``.
|
||||
|
||||
Streaming is deliberately not covered: chunks are assembled by the inherited
|
||||
SSE iterator, the terminal ``finish_reason`` arrives before the usage chunk
|
||||
that would justify rewriting it, and callers that omit
|
||||
``stream_options.include_usage`` never receive usage at all. Since the
|
||||
misreport no longer reproduces, buffering the stream to correct it is not
|
||||
worth the latency and complexity.
|
||||
"""
|
||||
if len(response.choices) != 1:
|
||||
return response
|
||||
|
||||
max_tokens = _as_positive_number(request_data.get("max_tokens"))
|
||||
if max_tokens is None:
|
||||
return response
|
||||
|
||||
usage = getattr(response, "usage", None)
|
||||
completion_tokens = _as_positive_number(getattr(usage, "completion_tokens", None))
|
||||
if completion_tokens is None or completion_tokens < max_tokens:
|
||||
return response
|
||||
|
||||
for choice in response.choices:
|
||||
if getattr(choice, "finish_reason", None) == "stop":
|
||||
choice.finish_reason = "length"
|
||||
|
||||
return response
|
||||
|
||||
|
||||
def _unwrap_response_envelope(raw_response: httpx.Response) -> httpx.Response:
|
||||
"""Strip ClinePass's ``data`` wrapper off a JSON completion body.
|
||||
|
||||
|
|
@ -263,7 +198,12 @@ class ClinePassConfig(OpenAIGPTConfig):
|
|||
api_key: str | None = None,
|
||||
json_mode: bool | None = None,
|
||||
) -> ModelResponse:
|
||||
response = super().transform_response(
|
||||
# ClinePass was once observed returning finish_reason "stop" on a completion
|
||||
# cut off by max_tokens. Follow-up probes on 2026-08-22 did not reproduce it.
|
||||
# The provider therefore reports the upstream finish reason unmodified: inferring
|
||||
# truncation from usage equalling the cap produces false positives on natural
|
||||
# completions that happen to land exactly on the cap.
|
||||
return super().transform_response(
|
||||
model=model,
|
||||
raw_response=_unwrap_response_envelope(raw_response),
|
||||
model_response=model_response,
|
||||
|
|
@ -276,11 +216,8 @@ class ClinePassConfig(OpenAIGPTConfig):
|
|||
api_key=api_key,
|
||||
json_mode=json_mode,
|
||||
)
|
||||
return _correct_truncated_finish_reason(response, request_data)
|
||||
|
||||
def get_error_class(
|
||||
self, error_message: str, status_code: int, headers: dict | httpx.Headers
|
||||
) -> BaseLLMException:
|
||||
def get_error_class(self, error_message: str, status_code: int, headers: dict | httpx.Headers) -> BaseLLMException:
|
||||
return ClinePassException(
|
||||
message=error_message,
|
||||
status_code=status_code,
|
||||
|
|
|
|||
|
|
@ -17,7 +17,6 @@ import litellm
|
|||
from litellm.llms.clinepass.chat.transformation import (
|
||||
ClinePassConfig,
|
||||
_apply_model_prefix,
|
||||
_correct_truncated_finish_reason,
|
||||
_unwrap_response_envelope,
|
||||
)
|
||||
from litellm.llms.custom_httpx.http_handler import AsyncHTTPHandler, HTTPHandler
|
||||
|
|
@ -275,6 +274,15 @@ def test_completion_streaming_is_not_unwrapped():
|
|||
}
|
||||
for piece in ["one ", "two ", "three"]
|
||||
]
|
||||
chunks.append(
|
||||
{
|
||||
"id": "chatcmpl-test",
|
||||
"object": "chat.completion.chunk",
|
||||
"created": 1,
|
||||
"model": "clinepass/deepseek-v4-flash",
|
||||
"choices": [{"index": 0, "delta": {}, "finish_reason": "stop"}],
|
||||
}
|
||||
)
|
||||
body = "".join(f"data: {json.dumps(c)}\n\n" for c in chunks) + "data: [DONE]\n\n"
|
||||
|
||||
def fake_post(self, url, *args, **kwargs):
|
||||
|
|
@ -292,9 +300,17 @@ def test_completion_streaming_is_not_unwrapped():
|
|||
max_tokens=4000,
|
||||
stream=True,
|
||||
)
|
||||
text = "".join(c.choices[0].delta.content or "" for c in stream if c.choices)
|
||||
text = ""
|
||||
finish_reason = None
|
||||
for c in stream:
|
||||
if c.choices:
|
||||
if c.choices[0].delta.content:
|
||||
text += c.choices[0].delta.content
|
||||
if c.choices[0].finish_reason:
|
||||
finish_reason = c.choices[0].finish_reason
|
||||
|
||||
assert text == "one two three"
|
||||
assert finish_reason == "stop"
|
||||
|
||||
|
||||
def test_upstream_401_maps_to_authentication_error():
|
||||
|
|
@ -329,9 +345,8 @@ def test_upstream_401_maps_to_authentication_error():
|
|||
#
|
||||
# ClinePass was once observed returning finish_reason "stop" on a completion cut
|
||||
# off by max_tokens. Re-probing the live API on 2026-08-22 could not reproduce
|
||||
# it (see _correct_truncated_finish_reason's docstring), so the correction is a
|
||||
# conservative safety net: single-choice only, upstream "stop" only, and only
|
||||
# when usage shows the cap was actually reached.
|
||||
# it, so the provider now reports the upstream finish reason unmodified to avoid
|
||||
# false positives on natural completions that land exactly on the cap.
|
||||
# --------------------------------------------------------------------------
|
||||
|
||||
|
||||
|
|
@ -354,14 +369,14 @@ def _complete(payload: dict, **kwargs):
|
|||
)
|
||||
|
||||
|
||||
def test_completion_at_the_cap_is_reported_as_length_not_stop():
|
||||
def test_completion_at_the_cap_preserves_upstream_stop():
|
||||
response = _complete(_truncated_envelope(4000), max_tokens=4000)
|
||||
assert response.choices[0].finish_reason == "length"
|
||||
assert response.choices[0].finish_reason == "stop"
|
||||
|
||||
|
||||
def test_completion_over_the_cap_is_reported_as_length():
|
||||
def test_completion_over_the_cap_preserves_upstream_stop():
|
||||
response = _complete(_truncated_envelope(4001), max_tokens=4000)
|
||||
assert response.choices[0].finish_reason == "length"
|
||||
assert response.choices[0].finish_reason == "stop"
|
||||
|
||||
|
||||
def test_completion_below_the_cap_keeps_stop():
|
||||
|
|
@ -379,93 +394,12 @@ def test_no_max_tokens_means_no_rewrite():
|
|||
assert response.choices[0].finish_reason == "stop"
|
||||
|
||||
|
||||
def test_max_completion_tokens_param_detects_truncation_after_mapping():
|
||||
def test_max_completion_tokens_param_preserves_upstream_stop():
|
||||
"""``max_completion_tokens`` is mapped to ``max_tokens`` before ``request_data`` is built."""
|
||||
response = _complete(_truncated_envelope(4000), max_completion_tokens=4000)
|
||||
assert response.choices[0].finish_reason == "length"
|
||||
assert response.choices[0].finish_reason == "stop"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("bad", [None, "4000", 0, -1, True, False])
|
||||
def test_unusable_cap_is_ignored(bad):
|
||||
"""Non-numeric, zero, negative and bool caps carry no truncation signal.
|
||||
|
||||
``bool`` matters because it subclasses ``int``: ``True`` would otherwise be
|
||||
read as a cap of 1 and relabel every response as truncated.
|
||||
"""
|
||||
|
||||
class _Choice:
|
||||
finish_reason = "stop"
|
||||
|
||||
class _Usage:
|
||||
completion_tokens = 9999
|
||||
|
||||
class _Response:
|
||||
choices = [_Choice()]
|
||||
usage = _Usage()
|
||||
|
||||
result = _correct_truncated_finish_reason(_Response(), {"max_tokens": bad})
|
||||
assert result.choices[0].finish_reason == "stop"
|
||||
|
||||
|
||||
def test_missing_usage_is_ignored():
|
||||
class _Choice:
|
||||
finish_reason = "stop"
|
||||
|
||||
class _Response:
|
||||
choices = [_Choice()]
|
||||
usage = None
|
||||
|
||||
result = _correct_truncated_finish_reason(_Response(), {"max_tokens": 4000})
|
||||
assert result.choices[0].finish_reason == "stop"
|
||||
|
||||
|
||||
def _stub_response(finish_reasons, completion_tokens):
|
||||
"""Minimal ModelResponse-shaped stub for the truncation helper."""
|
||||
|
||||
class _Choice:
|
||||
def __init__(self, reason):
|
||||
self.finish_reason = reason
|
||||
|
||||
class _Usage:
|
||||
pass
|
||||
|
||||
usage = _Usage()
|
||||
usage.completion_tokens = completion_tokens
|
||||
|
||||
class _Response:
|
||||
pass
|
||||
|
||||
response = _Response()
|
||||
response.choices = [_Choice(r) for r in finish_reasons]
|
||||
response.usage = usage
|
||||
return response
|
||||
|
||||
|
||||
def test_multi_choice_response_is_never_rewritten():
|
||||
"""`usage.completion_tokens` is an aggregate across choices while `max_tokens`
|
||||
is per choice, so the aggregate cannot say WHICH choice was truncated.
|
||||
|
||||
Two naturally-finished 60-token choices under a cap of 100 aggregate to 120,
|
||||
which would otherwise relabel both as `length`.
|
||||
"""
|
||||
response = _stub_response(["stop", "stop"], completion_tokens=120)
|
||||
result = _correct_truncated_finish_reason(response, {"max_tokens": 100})
|
||||
assert [c.finish_reason for c in result.choices] == ["stop", "stop"]
|
||||
|
||||
|
||||
def test_single_choice_at_the_cap_is_still_rewritten():
|
||||
"""The n>1 guard must not disable the correction for the normal n=1 case."""
|
||||
response = _stub_response(["stop"], completion_tokens=100)
|
||||
result = _correct_truncated_finish_reason(response, {"max_tokens": 100})
|
||||
assert result.choices[0].finish_reason == "length"
|
||||
|
||||
|
||||
def test_float_usage_and_cap_are_honoured():
|
||||
"""A gateway that reports usage as JSON floats must still be understood."""
|
||||
response = _stub_response(["stop"], completion_tokens=4000.0)
|
||||
result = _correct_truncated_finish_reason(response, {"max_tokens": 4000.0})
|
||||
assert result.choices[0].finish_reason == "length"
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# Model catalog
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue