mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-01 02:02:20 +00:00
fix(compact_20260112): surface error states + strip tool_result blocks in last user question
applied_edits_for_response() now includes compact_20260112 edits that carry an error field (summary_model_not_configured, summary_call_failed, summary_extraction_failed) so clients and operators can see why compaction was requested but not applied. _select_last_user_question() now strips tool_result blocks from mixed [tool_result, text] turns rather than passing them through as-is. After compaction the paired tool_use assistant turn no longer exists, so forwarding tool_result blocks translates to orphaned role=tool messages on non-Anthropic providers and produces a 400.
This commit is contained in:
parent
764c4ba5c8
commit
57a15fce2f
3 changed files with 111 additions and 35 deletions
|
|
@ -277,42 +277,38 @@ def _count_effective_tokens(
|
|||
return total
|
||||
|
||||
|
||||
def _is_tool_result_only_user_turn(msg: Dict[str, Any]) -> bool:
|
||||
"""Return True if ``msg`` is a ``role=user`` turn that carries only
|
||||
``tool_result`` blocks (i.e. an Anthropic tool-use response).
|
||||
|
||||
Such turns are not real "user question" turns and must not be used as
|
||||
the sole downstream message after a full compaction: the adapter
|
||||
translates them to OpenAI ``tool``-role messages, which require a
|
||||
preceding assistant ``tool_calls`` turn that no longer exists once the
|
||||
history has been summarized away.
|
||||
"""
|
||||
if msg.get("role") != "user":
|
||||
return False
|
||||
content = msg.get("content")
|
||||
if not isinstance(content, list) or not content:
|
||||
return False
|
||||
for block in content:
|
||||
if not isinstance(block, dict):
|
||||
return False
|
||||
if block.get("type") != "tool_result":
|
||||
return False
|
||||
return True
|
||||
|
||||
|
||||
def _select_last_user_question(
|
||||
messages: List[Dict[str, Any]],
|
||||
) -> List[Dict[str, Any]]:
|
||||
"""Pick the most recent ``user`` turn that is a real question.
|
||||
|
||||
Returns a one-element message list, or a synthetic continuation prompt
|
||||
if no eligible turn exists (e.g. the conversation only ever contained
|
||||
``tool_result`` turns, or contained no user turns at all). The
|
||||
downstream call always needs a non-empty user message.
|
||||
Returns a one-element message list with any ``tool_result`` blocks
|
||||
stripped: after compaction the paired ``tool_use`` assistant turn no
|
||||
longer exists in the downstream context, so forwarding ``tool_result``
|
||||
blocks would translate to orphaned ``role=tool`` messages on
|
||||
non-Anthropic providers (OpenAI, Gemini, …) and cause a 400 error.
|
||||
|
||||
Falls back to a synthetic continuation prompt if no eligible turn
|
||||
exists (e.g. the conversation only ever contained ``tool_result``
|
||||
turns, or contained no user turns at all). The downstream call always
|
||||
needs a non-empty user message.
|
||||
"""
|
||||
for msg in reversed(messages):
|
||||
if msg.get("role") == "user" and not _is_tool_result_only_user_turn(msg):
|
||||
return [msg]
|
||||
if msg.get("role") != "user":
|
||||
continue
|
||||
content = msg.get("content")
|
||||
if isinstance(content, list):
|
||||
filtered = [
|
||||
blk
|
||||
for blk in content
|
||||
if not (isinstance(blk, dict) and blk.get("type") == "tool_result")
|
||||
]
|
||||
if not filtered:
|
||||
# Purely tool_result — skip and look for an earlier turn.
|
||||
continue
|
||||
if len(filtered) < len(content):
|
||||
return [{**msg, "content": filtered}]
|
||||
return [msg]
|
||||
return [
|
||||
{
|
||||
"role": "user",
|
||||
|
|
|
|||
|
|
@ -28,14 +28,19 @@ class PolyfillResult:
|
|||
def applied_edits_for_response(self) -> Optional[List[AppliedEdit]]:
|
||||
"""``applied_edits`` to attach on the client-visible response.
|
||||
|
||||
``compact_20260112`` is included only when a new compaction block was
|
||||
synthesized (slice-only / under-threshold paths omit it). Other edit
|
||||
types are included when the editor returned an ``AppliedEdit``.
|
||||
``compact_20260112`` is included when a new compaction block was
|
||||
synthesized (success) OR when the edit carries an ``error`` field
|
||||
(``summary_model_not_configured``, ``summary_call_failed``,
|
||||
``summary_extraction_failed``) — operators and clients need to see
|
||||
why compaction was requested but not applied. Slice-only /
|
||||
under-threshold paths that produced no edit at all are omitted.
|
||||
Other edit types are included when the editor returned an
|
||||
``AppliedEdit``.
|
||||
"""
|
||||
visible: List[AppliedEdit] = []
|
||||
for edit in self.applied_edits:
|
||||
if edit.get("type") == COMPACT_EDIT_TYPE:
|
||||
if self.compaction_block is not None:
|
||||
if self.compaction_block is not None or edit.get("error"):
|
||||
visible.append(edit)
|
||||
else:
|
||||
visible.append(edit)
|
||||
|
|
|
|||
|
|
@ -24,6 +24,7 @@ from litellm.llms.anthropic.experimental_pass_through.context_management import
|
|||
from litellm.llms.anthropic.experimental_pass_through.context_management.editors.compact import (
|
||||
_augment_system_with_summary,
|
||||
_extract_summary_text,
|
||||
_select_last_user_question,
|
||||
_slice_around_compaction_block,
|
||||
_strip_compaction_blocks,
|
||||
apply_client_compaction_block_history,
|
||||
|
|
@ -88,8 +89,8 @@ def _make_mock_response(
|
|||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_applied_edits_for_response_omits_compact_without_compaction_block():
|
||||
"""Slice-only / under-threshold: no client-visible context_management edit."""
|
||||
def test_applied_edits_for_response_omits_compact_without_block_or_error():
|
||||
"""No compaction block and no error: omit the compact_20260112 edit."""
|
||||
result = PolyfillResult(
|
||||
messages=[],
|
||||
system="summary on system",
|
||||
|
|
@ -99,6 +100,24 @@ def test_applied_edits_for_response_omits_compact_without_compaction_block():
|
|||
assert result.applied_edits_for_response() is None
|
||||
|
||||
|
||||
def test_applied_edits_for_response_includes_compact_when_error_present():
|
||||
"""Error states must surface to the client so operators can debug."""
|
||||
for error in (
|
||||
"summary_model_not_configured",
|
||||
"summary_call_failed",
|
||||
"summary_extraction_failed",
|
||||
):
|
||||
result = PolyfillResult(
|
||||
messages=[],
|
||||
system=None,
|
||||
applied_edits=[{"type": "compact_20260112", "error": error}],
|
||||
compaction_block=None,
|
||||
)
|
||||
visible = result.applied_edits_for_response()
|
||||
assert visible is not None, error
|
||||
assert visible[0]["error"] == error
|
||||
|
||||
|
||||
def test_applied_edits_for_response_includes_compact_when_block_present():
|
||||
result = PolyfillResult(
|
||||
messages=[],
|
||||
|
|
@ -154,6 +173,62 @@ def test_strip_compaction_blocks_removes_block():
|
|||
assert content[0]["type"] == "text"
|
||||
|
||||
|
||||
def test_select_last_user_question_strips_tool_result_from_mixed_turn():
|
||||
"""Mixed [tool_result, text] turn: keep text, drop tool_result blocks."""
|
||||
messages = [
|
||||
{"role": "user", "content": "earlier"},
|
||||
{
|
||||
"role": "user",
|
||||
"content": [
|
||||
{"type": "tool_result", "tool_use_id": "a", "content": "res"},
|
||||
{"type": "text", "text": "follow-up question"},
|
||||
],
|
||||
},
|
||||
]
|
||||
selected = _select_last_user_question(messages)
|
||||
assert len(selected) == 1
|
||||
assert selected[0]["role"] == "user"
|
||||
content = selected[0]["content"]
|
||||
assert isinstance(content, list)
|
||||
assert all(b.get("type") != "tool_result" for b in content)
|
||||
assert any(
|
||||
b.get("type") == "text" and b.get("text") == "follow-up question"
|
||||
for b in content
|
||||
)
|
||||
|
||||
|
||||
def test_select_last_user_question_skips_pure_tool_result_turn():
|
||||
"""Pure tool_result turn: skip and walk back to a real user turn."""
|
||||
messages = [
|
||||
{"role": "user", "content": "real question"},
|
||||
{
|
||||
"role": "assistant",
|
||||
"content": [{"type": "tool_use", "id": "a", "name": "x", "input": {}}],
|
||||
},
|
||||
{
|
||||
"role": "user",
|
||||
"content": [{"type": "tool_result", "tool_use_id": "a", "content": "res"}],
|
||||
},
|
||||
]
|
||||
selected = _select_last_user_question(messages)
|
||||
assert len(selected) == 1
|
||||
assert selected[0]["content"] == "real question"
|
||||
|
||||
|
||||
def test_select_last_user_question_falls_back_when_no_eligible_turn():
|
||||
"""Only tool_result-only user turns: emit a synthetic continuation prompt."""
|
||||
messages = [
|
||||
{
|
||||
"role": "user",
|
||||
"content": [{"type": "tool_result", "tool_use_id": "a", "content": "res"}],
|
||||
},
|
||||
]
|
||||
selected = _select_last_user_question(messages)
|
||||
assert len(selected) == 1
|
||||
assert selected[0]["role"] == "user"
|
||||
assert isinstance(selected[0]["content"], str)
|
||||
|
||||
|
||||
def test_strip_compaction_blocks_drops_compaction_only_turn():
|
||||
messages = [
|
||||
{"role": "user", "content": "hi"},
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue