mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-22 00:31:44 +00:00
fix(responses): extract file→input_file helper, drop setdefault
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.
This commit is contained in:
parent
40cd8427b5
commit
cf19f6a928
2 changed files with 70 additions and 23 deletions
|
|
@ -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}"
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue