From c7b712effa72aeddb21f3e2465b32015ce85c0a4 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Thu, 11 Jun 2026 16:13:40 +0000 Subject: [PATCH] fix(translation): wire v2 tests into CI and clear the code-quality gate The new tests/test_litellm/translation suite was in no CI shard, so it never ran and codecov reported 0% patch coverage; add it to the misc unit-test workflow and the Makefile group so it runs and reports Allowlist the boundary JSON walkers freeze and thaw in recursive_detector, the same treatment every other cycle-free JSON/schema walker gets (bounded by input nesting depth, no cycles possible in JSON) Drop the unused engine/http.py I/O port; it had no caller in a request-only slice, so it was dead code with zero coverage. The injected port returns with the response and stream increment that actually performs I/O Add failures-as-values tests covering the parser's error branches (malformed roles, content parts, images, tools, tool_calls, tool_choice) and four more differential parity cases: system as array, stop as string, max_completion_tokens, and a tool without a description --- .github/workflows/test-unit-misc.yml | 1 + Makefile | 2 +- litellm/translation/CLAUDE.md | 7 +- litellm/translation/engine/__init__.py | 2 +- litellm/translation/engine/http.py | 28 ---- .../code_coverage_tests/recursive_detector.py | 2 + .../test_differential_anthropic_request.py | 39 ++++++ .../translation/test_parse_errors.py | 124 ++++++++++++++++++ 8 files changed, 173 insertions(+), 32 deletions(-) delete mode 100644 litellm/translation/engine/http.py create mode 100644 tests/test_litellm/translation/test_parse_errors.py diff --git a/.github/workflows/test-unit-misc.yml b/.github/workflows/test-unit-misc.yml index a7363ac3b4..f221f04251 100644 --- a/.github/workflows/test-unit-misc.yml +++ b/.github/workflows/test-unit-misc.yml @@ -34,6 +34,7 @@ jobs: tests/test_litellm/interactions tests/test_litellm/passthrough tests/test_litellm/vector_stores + tests/test_litellm/translation tests/test_litellm/test_*.py workers: 2 reruns: 2 diff --git a/Makefile b/Makefile index a00a90da60..56ceb67c1f 100644 --- a/Makefile +++ b/Makefile @@ -155,7 +155,7 @@ test-unit-core-utils: install-test-deps $(UV_RUN) pytest tests/test_litellm/litellm_core_utils --tb=short -vv -n 2 --durations=20 test-unit-other: install-test-deps - $(UV_RUN) pytest tests/test_litellm/caching tests/test_litellm/responses tests/test_litellm/secret_managers tests/test_litellm/vector_stores tests/test_litellm/a2a_protocol tests/test_litellm/anthropic_interface tests/test_litellm/completion_extras tests/test_litellm/containers tests/test_litellm/enterprise tests/test_litellm/experimental_mcp_client tests/test_litellm/google_genai tests/test_litellm/images tests/test_litellm/interactions tests/test_litellm/passthrough tests/test_litellm/router_strategy tests/test_litellm/router_utils tests/test_litellm/types --tb=short -vv -n 4 --durations=20 + $(UV_RUN) pytest tests/test_litellm/caching tests/test_litellm/responses tests/test_litellm/secret_managers tests/test_litellm/vector_stores tests/test_litellm/a2a_protocol tests/test_litellm/anthropic_interface tests/test_litellm/completion_extras tests/test_litellm/containers tests/test_litellm/enterprise tests/test_litellm/experimental_mcp_client tests/test_litellm/google_genai tests/test_litellm/images tests/test_litellm/interactions tests/test_litellm/passthrough tests/test_litellm/router_strategy tests/test_litellm/router_utils tests/test_litellm/translation tests/test_litellm/types --tb=short -vv -n 4 --durations=20 test-unit-root: install-test-deps $(UV_RUN) pytest tests/test_litellm/test_*.py --tb=short -vv -n 4 --durations=20 diff --git a/litellm/translation/CLAUDE.md b/litellm/translation/CLAUDE.md index 207e1f5baf..155c0dea86 100644 --- a/litellm/translation/CLAUDE.md +++ b/litellm/translation/CLAUDE.md @@ -23,10 +23,13 @@ translation/ ├── providers/ # one subpackage per wire format: IR -> body. Pure, no I/O │ └── anthropic/ └── engine/ - ├── pipeline.py # composition + the public translate entry point - └── http.py # the ONLY I/O, an injected port (functional core / imperative shell) + └── pipeline.py # composition + the public translate entry point ``` +The injected I/O port (functional core, imperative shell) lands with the +response and stream increment, when there is actual I/O to inject; this slice is +a pure request transform with no network. + ## Conventions - Composition over inheritance: a provider is a module of pure functions, not a base diff --git a/litellm/translation/engine/__init__.py b/litellm/translation/engine/__init__.py index b3732fe47f..7c87cc874d 100644 --- a/litellm/translation/engine/__init__.py +++ b/litellm/translation/engine/__init__.py @@ -1 +1 @@ -"""Engine: composition, the injected I/O port, and stream accumulation.""" +"""Engine: composition and the public translate entry point.""" diff --git a/litellm/translation/engine/http.py b/litellm/translation/engine/http.py deleted file mode 100644 index cc82cb8361..0000000000 --- a/litellm/translation/engine/http.py +++ /dev/null @@ -1,28 +0,0 @@ -"""The only I/O in the package, defined as an injected port. - -The pure core never imports an HTTP client; the imperative shell passes one in -that satisfies ``HttpPort``. That keeps the translation functions deterministic -and unit-testable without a network, and lets the proxy choose the client. -""" - -from __future__ import annotations - -from dataclasses import dataclass -from typing import Protocol - -from expression.collections import Map - -from ..ir import Body, PlainJson - - -@dataclass(frozen=True) -class HttpResponse: - status: int - headers: Map[str, str] - body: PlainJson - - -class HttpPort(Protocol): - async def post( - self, url: str, headers: Map[str, str], body: Body - ) -> HttpResponse: ... diff --git a/tests/code_coverage_tests/recursive_detector.py b/tests/code_coverage_tests/recursive_detector.py index 254d700ee5..d42ec81abd 100644 --- a/tests/code_coverage_tests/recursive_detector.py +++ b/tests/code_coverage_tests/recursive_detector.py @@ -50,6 +50,8 @@ IGNORE_FUNCTIONS = [ "_resolve", # OCI: $ref resolver bounded by `resolving_stack` cycle guard. "resolve_oci_schema_anyof", # OCI: bounded by JSON-schema tree depth (no cycles possible in well-formed input). "sanitize_oci_schema", # OCI: bounded by JSON-schema tree depth. + "freeze", # translation/boundary: plain JSON -> frozen IR walker, bounded by input nesting depth (no cycles possible in JSON). + "thaw", # translation/boundary: inverse of freeze, bounded by IR nesting depth (no cycles possible). ] diff --git a/tests/test_litellm/translation/test_differential_anthropic_request.py b/tests/test_litellm/translation/test_differential_anthropic_request.py index fde2c3f214..562e80b7bd 100644 --- a/tests/test_litellm/translation/test_differential_anthropic_request.py +++ b/tests/test_litellm/translation/test_differential_anthropic_request.py @@ -170,6 +170,45 @@ CORPUS = { {"role": "tool", "tool_call_id": "call_1", "content": "Sunny, 20C"}, ], }, + "system_as_array": { + "model": MODEL, + "max_tokens": 30, + "messages": [ + { + "role": "system", + "content": [ + {"type": "text", "text": "You are helpful"}, + {"type": "text", "text": "Be concise"}, + ], + }, + {"role": "user", "content": "Hi"}, + ], + }, + "stop_as_string": { + "model": MODEL, + "max_tokens": 40, + "stop": "STOP", + "messages": [{"role": "user", "content": "Hello"}], + }, + "max_completion_tokens": { + "model": MODEL, + "max_completion_tokens": 77, + "messages": [{"role": "user", "content": "Hello"}], + }, + "tool_without_description": { + "model": MODEL, + "max_tokens": 50, + "tools": [ + { + "type": "function", + "function": { + "name": "ping", + "parameters": {"type": "object", "properties": {}}, + }, + } + ], + "messages": [{"role": "user", "content": "ping"}], + }, "image_data_uri": { "model": MODEL, "max_tokens": 100, diff --git a/tests/test_litellm/translation/test_parse_errors.py b/tests/test_litellm/translation/test_parse_errors.py new file mode 100644 index 0000000000..d3d8639460 --- /dev/null +++ b/tests/test_litellm/translation/test_parse_errors.py @@ -0,0 +1,124 @@ +"""Malformed requests surface a descriptive error value, never an exception. + +These pin the failures-as-values contract: each bad shape must come back as an +``Error`` whose ``.summary`` names the problem, so the FastAPI edge has one +string to return and nothing in the package raises. +""" + +import pytest + +from litellm.translation import translate_chat_request + +MODEL = "claude-3-5-sonnet-20241022" +_USER = {"role": "user", "content": "hi"} + + +def _req(**overrides) -> dict: + base = {"model": MODEL, "messages": [_USER]} + base.update(overrides) + return base + + +CASES = [ + ("missing model and messages", {}, ["model", "messages"]), + ("non-object message", _req(messages=[123]), ["each message must be an object"]), + ( + "unsupported role", + _req(messages=[{"role": "function", "content": "x"}]), + ["unsupported message role"], + ), + ( + "content wrong type", + _req(messages=[{"role": "user", "content": 123}]), + ["string or an array"], + ), + ( + "content part not object", + _req(messages=[{"role": "user", "content": [123]}]), + ["each content part must be an object"], + ), + ( + "text part missing text", + _req(messages=[{"role": "user", "content": [{"type": "text"}]}]), + ["missing 'text'"], + ), + ( + "unsupported part type", + _req(messages=[{"role": "user", "content": [{"type": "audio"}]}]), + ["unsupported content part type"], + ), + ( + "image missing url", + _req( + messages=[ + {"role": "user", "content": [{"type": "image_url", "image_url": {}}]} + ] + ), + ["missing a 'url'"], + ), + ( + "image not a data uri", + _req( + messages=[ + { + "role": "user", + "content": [ + {"type": "image_url", "image_url": {"url": "https://x/y.png"}} + ], + } + ] + ), + ["only base64 data"], + ), + ( + "tool message missing id", + _req(messages=[{"role": "tool", "content": "x"}]), + ["tool message requires"], + ), + ("tools not an array", _req(tools=5), ["'tools' must be an array"]), + ( + "tool missing function", + _req(tools=[{"type": "function"}]), + ["must have a 'function' object"], + ), + ( + "tool function missing name", + _req(tools=[{"type": "function", "function": {}}]), + ["requires a 'name'"], + ), + ( + "tool_call missing id", + _req( + messages=[ + { + "role": "assistant", + "content": None, + "tool_calls": [{"function": {"name": "f", "arguments": "{}"}}], + } + ] + ), + ["requires 'id' and 'function.name'"], + ), + ( + "tool_calls not an array", + _req(messages=[{"role": "assistant", "content": None, "tool_calls": 5}]), + ["'tool_calls' must be an array"], + ), + ("tool_choice unsupported", _req(tool_choice=5), ["unsupported tool_choice"]), + ( + "tool_choice object missing name", + _req(tool_choice={"foo": 1}), + ["requires 'function.name'"], + ), +] + + +@pytest.mark.parametrize( + "name, request_body, expected", CASES, ids=[case[0] for case in CASES] +) +def test_malformed_request_is_an_error_value(name, request_body, expected) -> None: + result = translate_chat_request(request_body, "anthropic") + assert result.is_error(), f"{name} should have failed to parse" + summary = result.error.summary + for fragment in expected: + assert fragment in summary, f"{name}: {fragment!r} not in {summary!r}"