From e688210c3f2e15c6bf35be0c277542ef8c7df61b Mon Sep 17 00:00:00 2001 From: Daniel JB Clark Date: Sat, 22 Aug 2026 12:00:24 -0400 Subject: [PATCH] fix(clinepass): correct the model namespace, harden truncation detection Addresses findings from three independent code reviews (Claude Opus 5, GPT-5.6-sol xhigh via codex, Grok 4.6 xhigh via cursor) ahead of proposing this branch upstream. Correctness: * The restored model qualifier was `clinepass/`, a namespace that does not exist. The catalog namespace is `cline-pass/` (hyphenated); `clinepass/` is LiteLLM's own routing prefix, which is stripped before the request is built. This failed silently because the API validates only the *shape* of a model id -- `totallybogus/deepseek-v4-flash` also returns HTTP 200. It is not inert, though: verified against the live API, `cline-pass/deepseek-v4-flash` resolves to `deepseek/deepseek-v4-flash` while any unrecognised namespace falls back to the date-pinned `deepseek/deepseek-v4-flash-0731`. * `_correct_truncated_finish_reason` compared an aggregate `usage.completion_tokens` against a per-choice `max_tokens`. With `n > 1` that relabels naturally-finished choices as truncated: two 60-token choices under a cap of 100 aggregate to 120 and both became `length`. Restricted to single-choice responses, where the inference is sound. * Zero, negative and `bool` caps are now rejected. `bool` subclasses `int`, so `max_tokens=True` was read as a cap of 1 and would relabel everything. Float usage/caps are now accepted rather than silently skipped. * `_unwrap_response_envelope` reached for the private `_request` attribute because `httpx.Response.request` raises `RuntimeError` instead of returning `None`. Ask for the public attribute defensively instead. * `get_models()` is overridden to return an empty catalog. ClinePass has no `/models` endpoint (404), and the inherited OpenAI implementation asked for it at the wrong path. * Dropped the `async_transform_request` override. `BaseLLMHTTPHandler` builds the body with the synchronous `transform_request` on both the sync and async paths, so it was dead code -- the same shape of bug this provider shipped once already. Pinned with a test. Packaging / CI: * Added the `clinepass` entry to `provider_endpoints_support.json` and its backup. Without it `check_provider_folders_documented.py` fails, which is a required Code Quality job -- verified failing before, passing after. * `transformation.py` did not satisfy `ruff format` under the repo's pinned ruff 0.15.3, which the changed-file CI gate runs. Reformatted. * This commit replaces the previous branch tip, which had accidentally swept in ~435 regenerated Next.js artifacts under `litellm/proxy/_experimental/`. The branch is now +883/-0 across 12 files. Honesty note on the truncation fix: re-probing the live API (streaming and non-streaming, caps of 2000 and 4000, both namespaces) could NOT reproduce the `stop`-instead-of-`length` misreport that motivated it. The correction is kept as a conservative safety net and documented as such rather than as a workaround for a currently-observable defect. Streaming is deliberately not covered; see the docstring. Tests: 41 pass (was 32). openai_like + cometapi regressions: 126 passed, 8 skipped. (cherry picked from commit c2dda4fd8b7a4e1874642b25d73656292cb52468) --- litellm/llms/clinepass/chat/transformation.py | 128 +++++++++--- .../provider_endpoints_support_backup.json | 18 ++ provider_endpoints_support.json | 18 ++ .../chat/test_clinepass_transformation.py | 193 +++++++++++++++++- 4 files changed, 331 insertions(+), 26 deletions(-) diff --git a/litellm/llms/clinepass/chat/transformation.py b/litellm/llms/clinepass/chat/transformation.py index 7b077467ebd..e0d61697ba6 100644 --- a/litellm/llms/clinepass/chat/transformation.py +++ b/litellm/llms/clinepass/chat/transformation.py @@ -9,13 +9,13 @@ ClinePass is OpenAI-compatible apart from two quirks, both handled here: inherited SSE handling needs no change. 2. A bare model id is rejected with HTTP 400 ``invalid model format. Expected format: modelType/model``, but LiteLLM strips its own ``clinepass/`` routing - prefix before the request is built, so it has to be restored. + prefix before the request is built, so a qualifier has to be restored. Documentation: https://docs.cline.bot/ """ import json -from typing import Any, List, Tuple, Union +from typing import Any, List, Optional, Tuple, Union import httpx @@ -32,14 +32,90 @@ CLINEPASS_API_BASE = "https://api.cline.bot/api/v1" # ClinePass nests the completion under this key on non-streaming responses. CLINEPASS_RESPONSE_ENVELOPE_KEY = "data" -# The qualifier ClinePass requires on outbound model ids. -CLINEPASS_MODEL_PREFIX = "clinepass/" +# The qualifier ClinePass expects on outbound model ids. +# +# Note the hyphen: the catalog namespace is ``cline-pass/``, not ``clinepass/`` +# (the latter is LiteLLM's own routing prefix, which is stripped before the +# request is built). The API only validates the *shape* of a model id -- any +# ``/`` is accepted with HTTP 200 -- so an incorrect namespace +# fails silently rather than loudly. It is not inert, though: for at least one +# model an unrecognised namespace resolves to a different, date-pinned snapshot +# (``cline-pass/deepseek-v4-flash`` -> ``deepseek/deepseek-v4-flash``, while +# ``clinepass/deepseek-v4-flash`` -> ``deepseek/deepseek-v4-flash-0731``). +CLINEPASS_MODEL_PREFIX = "cline-pass/" # Headers that describe the original byte stream and would be wrong once the # body is rewritten by _unwrap_response_envelope(). _BODY_SPECIFIC_HEADERS = ("content-length", "content-encoding") +def _as_positive_number(value: Any) -> Optional[float]: + """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: + max_tokens = _as_positive_number(request_data.get("max_completion_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. @@ -66,11 +142,19 @@ def _unwrap_response_envelope(raw_response: httpx.Response) -> httpx.Response: headers = {k: v for k, v in raw_response.headers.items() if k.lower() not in _BODY_SPECIFIC_HEADERS} + # httpx.Response.request raises RuntimeError rather than returning None when + # no request is attached, so ask for it defensively instead of reaching for + # the private attribute behind it. + try: + original_request = raw_response.request + except RuntimeError: + original_request = None + return httpx.Response( status_code=raw_response.status_code, headers=headers, content=json.dumps(inner).encode("utf-8"), - request=getattr(raw_response, "_request", None), + request=original_request, ) @@ -119,6 +203,17 @@ class ClinePassConfig(OpenAIGPTConfig): return f"{api_base}/chat/completions" + def get_models(self, api_key: Optional[str] = None, api_base: Optional[str] = None) -> List[str]: + """ClinePass exposes no model catalog. + + ``GET https://api.cline.bot/api/v1/models`` returns HTTP 404, and the + inherited OpenAI implementation would additionally ask for it at the + wrong path -- it rewrites the base URL down to scheme+host and appends + ``/v1/models``. Return an empty catalog rather than making a request + that is known to fail. + """ + return [] + def map_openai_params( self, non_default_params: dict, @@ -145,6 +240,9 @@ class ClinePassConfig(OpenAIGPTConfig): litellm_params: dict, headers: dict, ) -> dict: + # BaseLLMHTTPHandler builds the body with this synchronous method on + # both the sync and the async path, so there is deliberately no + # async_transform_request() override -- it would never be called. data = super().transform_request( model=model, messages=messages, @@ -154,23 +252,6 @@ class ClinePassConfig(OpenAIGPTConfig): ) return _apply_model_prefix(data) - async def async_transform_request( - self, - model: str, - messages: List[AllMessageValues], - optional_params: dict, - litellm_params: dict, - headers: dict, - ) -> dict: - data = await super().async_transform_request( - model=model, - messages=messages, - optional_params=optional_params, - litellm_params=litellm_params, - headers=headers, - ) - return _apply_model_prefix(data) - def transform_response( self, model: str, @@ -185,7 +266,7 @@ class ClinePassConfig(OpenAIGPTConfig): api_key: str | None = None, json_mode: bool | None = None, ) -> ModelResponse: - return super().transform_response( + response = super().transform_response( model=model, raw_response=_unwrap_response_envelope(raw_response), model_response=model_response, @@ -198,6 +279,7 @@ 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: Union[dict, httpx.Headers] diff --git a/litellm/provider_endpoints_support_backup.json b/litellm/provider_endpoints_support_backup.json index c9635587eeb..8f52571799a 100644 --- a/litellm/provider_endpoints_support_backup.json +++ b/litellm/provider_endpoints_support_backup.json @@ -492,6 +492,24 @@ "interactions": true } }, + "clinepass": { + "display_name": "ClinePass (`clinepass`)", + "url": "https://docs.litellm.ai/docs/providers/clinepass", + "endpoints": { + "chat_completions": true, + "messages": true, + "responses": true, + "embeddings": false, + "image_generations": false, + "audio_transcriptions": false, + "audio_speech": false, + "moderations": false, + "batches": false, + "rerank": false, + "a2a": true, + "interactions": true + } + }, "cloudflare": { "display_name": "Cloudflare AI Workers (`cloudflare`)", "url": "https://docs.litellm.ai/docs/providers/cloudflare_workers", diff --git a/provider_endpoints_support.json b/provider_endpoints_support.json index 7ffaacdb3aa..1082d2046eb 100644 --- a/provider_endpoints_support.json +++ b/provider_endpoints_support.json @@ -546,6 +546,24 @@ "interactions": true } }, + "clinepass": { + "display_name": "ClinePass (`clinepass`)", + "url": "https://docs.litellm.ai/docs/providers/clinepass", + "endpoints": { + "chat_completions": true, + "messages": true, + "responses": true, + "embeddings": false, + "image_generations": false, + "audio_transcriptions": false, + "audio_speech": false, + "moderations": false, + "batches": false, + "rerank": false, + "a2a": true, + "interactions": true + } + }, "cloudflare": { "display_name": "Cloudflare AI Workers (`cloudflare`)", "url": "https://docs.litellm.ai/docs/providers/cloudflare_workers", diff --git a/tests/test_litellm/llms/clinepass/chat/test_clinepass_transformation.py b/tests/test_litellm/llms/clinepass/chat/test_clinepass_transformation.py index e29968dcda0..588aa7ec42f 100644 --- a/tests/test_litellm/llms/clinepass/chat/test_clinepass_transformation.py +++ b/tests/test_litellm/llms/clinepass/chat/test_clinepass_transformation.py @@ -17,6 +17,7 @@ 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 @@ -123,7 +124,14 @@ def test_get_complete_url(api_base, expected): def test_model_prefix_restored_on_bare_id(): - assert _apply_model_prefix({"model": "deepseek-v4-flash"})["model"] == "clinepass/deepseek-v4-flash" + """The restored qualifier is the catalog namespace ``cline-pass/`` (hyphenated), + NOT LiteLLM's own ``clinepass/`` routing prefix. + + The API validates only the *shape* of a model id, so a wrong namespace still + returns HTTP 200 -- but it does not always resolve to the same underlying + model, which makes a wrong value silent rather than harmless. + """ + assert _apply_model_prefix({"model": "deepseek-v4-flash"})["model"] == "cline-pass/deepseek-v4-flash" def test_model_prefix_left_alone_when_qualifier_present(): @@ -208,7 +216,7 @@ def test_completion_unwraps_envelope_and_prefixes_model(): ) assert captured["url"] == "https://api.cline.bot/api/v1/chat/completions" - assert captured["body"]["model"] == "clinepass/deepseek-v4-flash" + assert captured["body"]["model"] == "cline-pass/deepseek-v4-flash" assert response.choices[0].message.content == "pong" assert response.choices[0].message.reasoning_content == "the user asked for pong" @@ -230,7 +238,7 @@ async def test_acompletion_unwraps_envelope_and_prefixes_model(): ) assert captured["url"] == "https://api.cline.bot/api/v1/chat/completions" - assert captured["body"]["model"] == "clinepass/deepseek-v4-flash" + assert captured["body"]["model"] == "cline-pass/deepseek-v4-flash" assert response.choices[0].message.content == "pong" @@ -293,3 +301,182 @@ def test_upstream_401_maps_to_authentication_error(): ) assert excinfo.value.status_code == 401 + + +# -------------------------------------------------------------------------- +# Truncation reporting +# +# 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. +# -------------------------------------------------------------------------- + + +def _truncated_envelope(completion_tokens: int, finish_reason: str = "stop") -> dict: + payload = json.loads(json.dumps(ENVELOPED_COMPLETION)) + payload["data"]["choices"][0]["finish_reason"] = finish_reason + payload["data"]["usage"]["completion_tokens"] = completion_tokens + return payload + + +def _complete(payload: dict, **kwargs): + def fake_post(self, url, *args, **post_kwargs): + return _response(payload) + + with patch.object(HTTPHandler, "post", fake_post): + return litellm.completion( + model="clinepass/deepseek-v4-flash", + messages=[{"role": "user", "content": "ping"}], + **kwargs, + ) + + +def test_completion_at_the_cap_is_reported_as_length_not_stop(): + response = _complete(_truncated_envelope(4000), max_tokens=4000) + assert response.choices[0].finish_reason == "length" + + +def test_completion_over_the_cap_is_reported_as_length(): + response = _complete(_truncated_envelope(4001), max_tokens=4000) + assert response.choices[0].finish_reason == "length" + + +def test_completion_below_the_cap_keeps_stop(): + response = _complete(_truncated_envelope(3999), max_tokens=4000) + assert response.choices[0].finish_reason == "stop" + + +def test_upstream_length_is_left_alone(): + response = _complete(_truncated_envelope(4000, finish_reason="length"), max_tokens=4000) + assert response.choices[0].finish_reason == "length" + + +def test_no_max_tokens_means_no_rewrite(): + response = _complete(_truncated_envelope(4000)) + assert response.choices[0].finish_reason == "stop" + + +def test_max_completion_tokens_also_detects_truncation(): + response = _complete(_truncated_envelope(4000), max_completion_tokens=4000) + assert response.choices[0].finish_reason == "length" + + +@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 +# -------------------------------------------------------------------------- + + +def test_get_models_returns_empty_without_calling_the_api(): + """ClinePass has no /models endpoint (404), and the inherited OpenAI + implementation would ask for it at the wrong path. It must not make the + request at all.""" + + def explode(*args, **kwargs): # pragma: no cover - must never run + raise AssertionError("get_models() must not perform an HTTP request") + + with patch.object(litellm.module_level_client, "get", explode): + assert ClinePassConfig().get_models(api_key=API_KEY) == [] + + +# -------------------------------------------------------------------------- +# httpx internals +# -------------------------------------------------------------------------- + + +def test_unwrap_envelope_survives_a_response_with_no_request_attached(): + """`httpx.Response.request` RAISES RuntimeError rather than returning None + when no request is attached, so the unwrap must ask for it defensively.""" + raw = httpx.Response(200, json=ENVELOPED_COMPLETION) + unwrapped = _unwrap_response_envelope(raw) + assert unwrapped.json() == ENVELOPED_COMPLETION["data"] + + +def test_clinepass_config_has_no_async_transform_request_override(): + """BaseLLMHTTPHandler builds the body with the sync transform_request on both + paths, so an async override would be dead code -- the shape of bug this + provider already shipped once.""" + assert "async_transform_request" not in ClinePassConfig.__dict__