mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-10 03:28:53 +00:00
fix(responses-bridge): make envelope codec safe for empty payloads and multi-item outputs
Greptile 4/5 follow-up on the envelope codec:
- ``_decode_item_envelope("msg_")`` previously returned the truthy string
``"resp_"``, which leaked through truthy guards in the item_reference
resolver and propagated an empty response_id to the session handler.
The decoder now returns ``None`` when the payload after the prefix is
empty.
- ``_envelope_encode_output_item_ids`` reused the same encoded id for
every ``message`` item in a response, so parallel ``n>1`` choices
collapsed to identical ids. The encoder now takes an ``item_position``
suffix (``.{n}``) that disambiguates each item; the decoder strips
the suffix before handing the inner payload to the existing
response-id decoder, so the round-trip still returns the same
``response_id``.
- 2 new tests: empty-payload decode returns ``None``, multiple message
items receive distinct ids that all decode to the same response_id.
This commit is contained in:
parent
5fb4ff4a22
commit
d07106fda9
2 changed files with 119 additions and 16 deletions
|
|
@ -244,15 +244,21 @@ class ResponsesAPIRequestUtils:
|
|||
prefix: str,
|
||||
custom_llm_provider: Optional[str],
|
||||
model_id: Optional[str],
|
||||
item_position: Optional[int] = None,
|
||||
) -> str:
|
||||
"""Wrap a raw upstream id (e.g. ``chatcmpl-*``) as ``rs_<env>`` or
|
||||
``msg_<env>`` so the value carries the same response_id payload as
|
||||
``response.id`` but with an item-type prefix that clients recognize.
|
||||
|
||||
The envelope reuses
|
||||
:meth:`_build_responses_api_response_id` and swaps the leading
|
||||
``resp_`` for the requested item prefix, so the result round-trips
|
||||
through :meth:`_decode_item_envelope` plus
|
||||
``item_position`` distinguishes multiple items of the same type that
|
||||
share a response_id (e.g. parallel ``n>1`` choices). The position is
|
||||
appended as ``.{n}`` after the base64 payload and stripped by
|
||||
:meth:`_decode_item_envelope` before the inner payload reaches the
|
||||
existing response-id decoder.
|
||||
|
||||
The envelope reuses :meth:`_build_responses_api_response_id` and swaps
|
||||
the leading ``resp_`` for the requested item prefix, so the result
|
||||
round-trips through :meth:`_decode_item_envelope` plus
|
||||
:meth:`_decode_responses_api_response_id` back to ``raw_response_id``.
|
||||
"""
|
||||
resp_form = ResponsesAPIRequestUtils._build_responses_api_response_id(
|
||||
|
|
@ -260,21 +266,32 @@ class ResponsesAPIRequestUtils:
|
|||
model_id=model_id,
|
||||
response_id=raw_response_id,
|
||||
)
|
||||
return f"{prefix}_" + resp_form[len("resp_") :]
|
||||
suffix = "" if item_position is None else f".{item_position}"
|
||||
return f"{prefix}_" + resp_form[len("resp_") :] + suffix
|
||||
|
||||
@staticmethod
|
||||
def _decode_item_envelope(item_id: str) -> Optional[str]:
|
||||
"""Decode ``rs_<env>`` / ``msg_<env>`` back to the ``resp_<env>`` form
|
||||
that :meth:`_decode_responses_api_response_id` understands.
|
||||
|
||||
Returns ``None`` on missing prefix or empty input. All other validation
|
||||
is delegated to the existing response-id decoder.
|
||||
Returns ``None`` on missing prefix, empty input, or an empty payload
|
||||
after the prefix (``"msg_"`` / ``"rs_"``). Strips the optional
|
||||
``.{position}`` item-position suffix before returning, so the inner
|
||||
base64 payload is the same regardless of which item the envelope
|
||||
belonged to.
|
||||
"""
|
||||
if not item_id:
|
||||
return None
|
||||
for prefix in ("rs_", "msg_"):
|
||||
if item_id.startswith(prefix):
|
||||
return "resp_" + item_id[len(prefix) :]
|
||||
payload = item_id[len(prefix) :]
|
||||
# Strip optional ".{position}" suffix used to disambiguate
|
||||
# multiple items of the same type sharing a response_id.
|
||||
if "." in payload:
|
||||
payload = payload.rsplit(".", 1)[0]
|
||||
if not payload:
|
||||
return None
|
||||
return "resp_" + payload
|
||||
return None
|
||||
|
||||
@staticmethod
|
||||
|
|
@ -299,6 +316,7 @@ class ResponsesAPIRequestUtils:
|
|||
if not output:
|
||||
return response
|
||||
|
||||
message_position = 0
|
||||
for item in output:
|
||||
if isinstance(item, dict):
|
||||
item_type = item.get("type")
|
||||
|
|
@ -313,17 +331,24 @@ class ResponsesAPIRequestUtils:
|
|||
or current_id.startswith("rs_")
|
||||
or current_id.startswith("encitem_")
|
||||
):
|
||||
message_position += 1
|
||||
continue
|
||||
# ``item_position`` keeps each message item's id distinct when a
|
||||
# response carries multiple ``message`` items sharing a single
|
||||
# response_id (parallel ``n>1`` choices). All ids still decode to
|
||||
# the same response_id payload via :meth:`_decode_item_envelope`.
|
||||
new_id = ResponsesAPIRequestUtils._encode_item_envelope(
|
||||
raw_response_id,
|
||||
prefix="msg",
|
||||
custom_llm_provider=custom_llm_provider,
|
||||
model_id=model_id,
|
||||
item_position=message_position,
|
||||
)
|
||||
if isinstance(item, dict):
|
||||
item["id"] = new_id
|
||||
else:
|
||||
item.id = new_id
|
||||
message_position += 1
|
||||
return response
|
||||
|
||||
@staticmethod
|
||||
|
|
|
|||
|
|
@ -66,15 +66,46 @@ def test_decode_item_envelope_returns_none_for_empty_input():
|
|||
assert ResponsesAPIRequestUtils._decode_item_envelope("") is None
|
||||
|
||||
|
||||
def test_decode_item_envelope_empty_payload_is_resp_passthrough():
|
||||
"""A ``msg_`` prefix with no payload decodes to ``resp_``, which the
|
||||
response-id decoder treats as raw passthrough rather than crashing.
|
||||
def test_decode_item_envelope_returns_none_for_empty_payload():
|
||||
"""A degenerate ``msg_`` or ``rs_`` prefix with no payload must decode to
|
||||
``None`` so callers (e.g. the item_reference resolver) can fall through
|
||||
to first-turn behavior instead of propagating an empty response_id to
|
||||
the session handler.
|
||||
"""
|
||||
result = ResponsesAPIRequestUtils._decode_item_envelope("msg_")
|
||||
assert result == "resp_"
|
||||
decoded = ResponsesAPIRequestUtils._decode_responses_api_response_id(result)
|
||||
assert decoded["custom_llm_provider"] is None
|
||||
assert decoded["model_id"] is None
|
||||
assert ResponsesAPIRequestUtils._decode_item_envelope("msg_") is None
|
||||
assert ResponsesAPIRequestUtils._decode_item_envelope("rs_") is None
|
||||
|
||||
|
||||
def test_encode_decode_round_trip_with_item_position():
|
||||
"""Distinct item positions yield distinct encoded ids but decode back to
|
||||
the same response_id payload.
|
||||
"""
|
||||
raw = "chatcmpl-multi"
|
||||
first = ResponsesAPIRequestUtils._encode_item_envelope(
|
||||
raw,
|
||||
prefix="msg",
|
||||
custom_llm_provider="hosted_vllm",
|
||||
model_id="m-1",
|
||||
item_position=0,
|
||||
)
|
||||
second = ResponsesAPIRequestUtils._encode_item_envelope(
|
||||
raw,
|
||||
prefix="msg",
|
||||
custom_llm_provider="hosted_vllm",
|
||||
model_id="m-1",
|
||||
item_position=1,
|
||||
)
|
||||
assert first != second
|
||||
assert first.endswith(".0")
|
||||
assert second.endswith(".1")
|
||||
for envelope in (first, second):
|
||||
decoded_resp_form = ResponsesAPIRequestUtils._decode_item_envelope(envelope)
|
||||
decoded = ResponsesAPIRequestUtils._decode_responses_api_response_id(
|
||||
decoded_resp_form
|
||||
)
|
||||
assert decoded["response_id"] == raw
|
||||
assert decoded["custom_llm_provider"] == "hosted_vllm"
|
||||
assert decoded["model_id"] == "m-1"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
@ -154,6 +185,53 @@ def test_envelope_encode_skips_encitem_prefixed_message_id():
|
|||
assert encoded.output[0]["id"] == "encitem_abc123"
|
||||
|
||||
|
||||
def test_envelope_encode_multiple_message_items_get_distinct_ids():
|
||||
"""When a response carries multiple ``message`` items (parallel ``n>1``
|
||||
choices), each rewritten id must be distinct so downstream clients can
|
||||
address them individually. All ids still decode to the same response_id
|
||||
payload via :meth:`_decode_item_envelope`.
|
||||
"""
|
||||
response = ResponsesAPIResponse(
|
||||
id="chatcmpl-multi",
|
||||
object="response",
|
||||
created_at=0,
|
||||
model="hosted_vllm/test-model",
|
||||
output=[
|
||||
{"type": "message", "id": "chatcmpl-multi"},
|
||||
{"type": "message", "id": "chatcmpl-multi"},
|
||||
{"type": "message", "id": "chatcmpl-multi"},
|
||||
],
|
||||
parallel_tool_calls=False,
|
||||
temperature=0,
|
||||
tool_choice="auto",
|
||||
tools=[],
|
||||
top_p=None,
|
||||
max_output_tokens=None,
|
||||
previous_response_id=None,
|
||||
reasoning=None,
|
||||
status="completed",
|
||||
text={},
|
||||
truncation=None,
|
||||
usage=None,
|
||||
user=None,
|
||||
)
|
||||
encoded = ResponsesAPIRequestUtils._envelope_encode_output_item_ids(
|
||||
response=response,
|
||||
raw_response_id="chatcmpl-multi",
|
||||
custom_llm_provider="hosted_vllm",
|
||||
model_id=None,
|
||||
)
|
||||
ids = [item["id"] for item in encoded.output]
|
||||
assert len(set(ids)) == 3
|
||||
for new_id in ids:
|
||||
assert new_id.startswith("msg_")
|
||||
resp_form = ResponsesAPIRequestUtils._decode_item_envelope(new_id)
|
||||
decoded = ResponsesAPIRequestUtils._decode_responses_api_response_id(
|
||||
resp_form
|
||||
)
|
||||
assert decoded["response_id"] == "chatcmpl-multi"
|
||||
|
||||
|
||||
def test_envelope_encode_leaves_function_call_items_untouched():
|
||||
"""function_call items keep their call_id-based id."""
|
||||
response = ResponsesAPIResponse(
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue