From cf19f6a9285ed72a66a13ee167b88dc84027e397 Mon Sep 17 00:00:00 2001 From: xander Date: Sat, 9 May 2026 16:35:45 +0800 Subject: [PATCH] =?UTF-8?q?fix(responses):=20extract=20file=E2=86=92input?= =?UTF-8?q?=5Ffile=20helper,=20drop=20setdefault?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback on PR #27523: 1. Greptile P2: switch from `converted.setdefault("file_url", file_id)` to direct assignment so a URL-shaped `file_id` is never silently dropped when a caller also supplies `file_url`. The URL the caller put in `file_id` is the explicit instruction we honour and now wins on conflict. 2. CI lint (PLR0915 "Too many statements"): extract the file→input_file conversion to a `_convert_content_file_to_input_file` static helper so `_convert_content_to_responses_format` stays under the 50-statement ruff limit. The inline branch now just delegates and logs. Adds a test that exercises the both-set conflict (URL `file_id` plus existing `file_url`) so the precedence is locked in. --- .../transformation.py | 56 +++++++++++-------- ...responses_transformation_transformation.py | 37 ++++++++++++ 2 files changed, 70 insertions(+), 23 deletions(-) diff --git a/litellm/completion_extras/litellm_responses_transformation/transformation.py b/litellm/completion_extras/litellm_responses_transformation/transformation.py index 4895322d140..cb3a0b6b4fe 100644 --- a/litellm/completion_extras/litellm_responses_transformation/transformation.py +++ b/litellm/completion_extras/litellm_responses_transformation/transformation.py @@ -712,6 +712,36 @@ class LiteLLMResponsesTransformationHandler(CompletionTransformationBridge): return image_param + @staticmethod + def _convert_content_file_to_input_file(item: Dict[str, Any]) -> Dict[str, Any]: + """ + Convert a Chat Completion `file` content block to a Responses API + `input_file` block. + + Chat Completion shape: + {"type": "file", + "file": {"file_id" | "file_data" | "filename" | "file_url": ...}} + Responses API `input_file` accepts `file_id`, `file_data`, `file_url`, + and optional `filename`. + + If `file_id` is an http(s) URL, route it to `file_url` (overwriting any + previously-set value) — providers (OpenAI, Azure) reject URLs in + `file_id` and require an uploaded file identifier such as + `file-abc123`. The URL the caller placed in `file_id` wins on + conflict so it is never silently dropped. + """ + file_data = item.get("file") if isinstance(item, dict) else None + converted: Dict[str, Any] = {"type": "input_file"} + if isinstance(file_data, dict): + for key in ("file_id", "file_data", "filename", "file_url"): + if key in file_data: + converted[key] = file_data[key] + file_id = converted.get("file_id") + if isinstance(file_id, str) and file_id.startswith(("https://", "http://")): + converted["file_url"] = file_id + converted.pop("file_id", None) + return converted + def _convert_content_to_responses_format( self, content: Optional[ @@ -783,29 +813,9 @@ class LiteLLMResponsesTransformationHandler(CompletionTransformationBridge): f"Chat provider: image -> {converted}" ) elif item_type == "file": - # Map Chat Completion file to Responses API input_file. - # {"type": "file", "file": {"file_id" | "file_data" | "filename" | "file_url": ...}} - # -> {"type": "input_file", "file_id" | "file_data" | "filename" | "file_url": ...} - # If `file_id` is an http(s) URL, route it to `file_url` instead — providers - # require `file_id` to be an uploaded file identifier (e.g. "file-abc123") and - # reject URLs there. - file_data = item.get("file", {}) - converted = {"type": "input_file"} - if isinstance(file_data, dict): - for key in ( - "file_id", - "file_data", - "filename", - "file_url", - ): - if key in file_data: - converted[key] = file_data[key] - file_id = converted.get("file_id") - if isinstance(file_id, str) and file_id.startswith( - ("https://", "http://") - ): - converted.setdefault("file_url", file_id) - converted.pop("file_id", None) + converted = self._convert_content_file_to_input_file( + item + ) result.append(converted) verbose_logger.debug( f"Chat provider: file -> {converted}" diff --git a/tests/test_litellm/completion_extras/litellm_responses_transformation/test_completion_extras_litellm_responses_transformation_transformation.py b/tests/test_litellm/completion_extras/litellm_responses_transformation/test_completion_extras_litellm_responses_transformation_transformation.py index 20afbe0035f..6958670dcaf 100644 --- a/tests/test_litellm/completion_extras/litellm_responses_transformation/test_completion_extras_litellm_responses_transformation_transformation.py +++ b/tests/test_litellm/completion_extras/litellm_responses_transformation/test_completion_extras_litellm_responses_transformation_transformation.py @@ -2232,6 +2232,43 @@ def test_convert_chat_completion_file_type_with_url_file_id_routes_to_file_url(u assert "file_id" not in file_item +def test_convert_chat_completion_file_type_url_file_id_wins_over_existing_file_url(): + """ + If a caller provides both `file_url` and a URL-shaped `file_id`, the URL + from `file_id` is the explicit instruction we honour — it overwrites any + pre-existing `file_url` rather than being silently dropped. + """ + from litellm.completion_extras.litellm_responses_transformation.transformation import ( + LiteLLMResponsesTransformationHandler, + ) + + handler = LiteLLMResponsesTransformationHandler() + + file_id_url = "https://mirror.example.com/doc.pdf" + other_url = "https://canonical.example.com/doc.pdf" + messages = [ + { + "role": "user", + "content": [ + { + "type": "file", + "file": {"file_id": file_id_url, "file_url": other_url}, + }, + ], + } + ] + + ( + input_items, + _, + ) = handler.convert_chat_completion_messages_to_responses_api(messages) + + file_item = input_items[0]["content"][0] + assert file_item["type"] == "input_file" + assert file_item["file_url"] == file_id_url + assert "file_id" not in file_item + + def test_convert_chat_completion_file_type_passes_through_file_url(): """ When the caller already provides `file_url` directly (Responses-API-shaped