fix(litellm): fall back to last user message when hindsight_query not provided (#641)
* fix(litellm): fall back to last user message when hindsight_query not provided inject_memories=True no longer requires an explicit hindsight_query. The injection path now falls back to extracting the last user message, matching the documented Quick Start behavior that was broken since #167 (v0.4.18). * test(litellm): add regression tests for inject_memories without hindsight_query
This commit is contained in:
parent
8364b9c5d5
commit
5e8952c54a
3 changed files with 99 additions and 30 deletions
|
|
@ -262,15 +262,28 @@ def _inject_memories(
|
|||
if not messages:
|
||||
return messages
|
||||
|
||||
# hindsight_query is required when inject_memories=True
|
||||
if not custom_query:
|
||||
raise ValueError(
|
||||
"hindsight_query is required when inject_memories=True. "
|
||||
"Pass hindsight_query='your query' to specify what to search for in memory. "
|
||||
"Example: hindsight_query=recipient_name or hindsight_query='What do I know about Alice?'"
|
||||
)
|
||||
|
||||
user_query = custom_query
|
||||
# Use custom_query if provided, otherwise fall back to the last user message
|
||||
if custom_query:
|
||||
user_query = custom_query
|
||||
else:
|
||||
user_query = None
|
||||
for msg in reversed(messages):
|
||||
if msg.get("role") == "user":
|
||||
content = msg.get("content")
|
||||
if isinstance(content, str):
|
||||
user_query = content
|
||||
break
|
||||
elif isinstance(content, list):
|
||||
text_parts = [
|
||||
item.get("text", "")
|
||||
for item in content
|
||||
if isinstance(item, dict) and item.get("type") == "text"
|
||||
]
|
||||
if text_parts:
|
||||
user_query = " ".join(text_parts)
|
||||
break
|
||||
if not user_query:
|
||||
return messages
|
||||
|
||||
# Use bank_id from defaults
|
||||
bank_id = defaults.bank_id
|
||||
|
|
|
|||
|
|
@ -162,9 +162,7 @@ class HindsightCallback(CustomLogger):
|
|||
logger.error(f"HTTP POST failed: {e}")
|
||||
raise HindsightError(f"Hindsight API request failed: {e}") from e
|
||||
|
||||
def _http_get(
|
||||
self, url: str, config: HindsightConfig
|
||||
) -> Optional[dict]:
|
||||
def _http_get(self, url: str, config: HindsightConfig) -> Optional[dict]:
|
||||
"""Make a synchronous HTTP GET request.
|
||||
|
||||
Returns:
|
||||
|
|
@ -736,16 +734,11 @@ class HindsightCallback(CustomLogger):
|
|||
if self._should_skip_model(model, config):
|
||||
return
|
||||
|
||||
# hindsight_query is required when inject_memories=True
|
||||
# Use hindsight_query if provided, otherwise fall back to the last user message
|
||||
custom_query = kwargs.get("hindsight_query")
|
||||
if not custom_query:
|
||||
raise ValueError(
|
||||
"hindsight_query is required when inject_memories=True. "
|
||||
"Pass hindsight_query='your query' to specify what to search for in memory. "
|
||||
"Example: hindsight_query=recipient_name or hindsight_query='What do I know about Alice?'"
|
||||
)
|
||||
|
||||
user_query = custom_query
|
||||
user_query = custom_query or self._extract_user_query(messages)
|
||||
if not user_query:
|
||||
return
|
||||
|
||||
# Use reflect or recall based on settings
|
||||
if settings.use_reflect:
|
||||
|
|
@ -802,16 +795,11 @@ class HindsightCallback(CustomLogger):
|
|||
if self._should_skip_model(model, config):
|
||||
return
|
||||
|
||||
# hindsight_query is required when inject_memories=True
|
||||
# Use hindsight_query if provided, otherwise fall back to the last user message
|
||||
custom_query = kwargs.get("hindsight_query")
|
||||
if not custom_query:
|
||||
raise ValueError(
|
||||
"hindsight_query is required when inject_memories=True. "
|
||||
"Pass hindsight_query='your query' to specify what to search for in memory. "
|
||||
"Example: hindsight_query=recipient_name or hindsight_query='What do I know about Alice?'"
|
||||
)
|
||||
|
||||
user_query = custom_query
|
||||
user_query = custom_query or self._extract_user_query(messages)
|
||||
if not user_query:
|
||||
return
|
||||
|
||||
# Use reflect or recall based on settings
|
||||
if settings.use_reflect:
|
||||
|
|
|
|||
|
|
@ -356,6 +356,74 @@ class TestCallback:
|
|||
assert "Relevant Memories" in result[0]["content"]
|
||||
assert "What's my name?" in result[0]["content"]
|
||||
|
||||
def test_inject_memories_uses_last_user_message_when_no_hindsight_query(self):
|
||||
"""Regression test: inject_memories=True should not require hindsight_query.
|
||||
|
||||
The documented Quick Start example does not pass hindsight_query; the
|
||||
injection path must fall back to the last user message automatically.
|
||||
See: feat(litellm) #167 regression.
|
||||
"""
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
callback = HindsightCallback()
|
||||
|
||||
configure(
|
||||
hindsight_api_url="http://localhost:8888",
|
||||
inject_memories=True,
|
||||
)
|
||||
set_defaults(bank_id="test-agent")
|
||||
|
||||
messages = [{"role": "user", "content": "What did we discuss about AI?"}]
|
||||
kwargs = {} # No hindsight_query provided — this is the regression scenario
|
||||
|
||||
mock_memory = MagicMock()
|
||||
mock_memory.text = "AI is cool"
|
||||
mock_memory.type = "world"
|
||||
mock_memory.weight = 0.9
|
||||
|
||||
with patch.object(callback, "_recall_memories_sync", return_value=[mock_memory]) as mock_recall:
|
||||
callback.log_pre_api_call(
|
||||
model="gpt-4o-mini",
|
||||
messages=messages,
|
||||
kwargs=kwargs,
|
||||
)
|
||||
# Should have called recall with the last user message as query
|
||||
mock_recall.assert_called_once()
|
||||
query_used = mock_recall.call_args[0][0]
|
||||
assert query_used == "What did we discuss about AI?"
|
||||
|
||||
# Memories should have been injected into messages
|
||||
assert any("AI is cool" in str(m.get("content", "")) for m in messages)
|
||||
|
||||
def test_inject_memories_hindsight_query_takes_precedence(self):
|
||||
"""When hindsight_query is provided it should be used over the last user message."""
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
callback = HindsightCallback()
|
||||
|
||||
configure(
|
||||
hindsight_api_url="http://localhost:8888",
|
||||
inject_memories=True,
|
||||
)
|
||||
set_defaults(bank_id="test-agent")
|
||||
|
||||
messages = [{"role": "user", "content": "Hello"}]
|
||||
kwargs = {"hindsight_query": "What do I know about Alice?"}
|
||||
|
||||
mock_memory = MagicMock()
|
||||
mock_memory.text = "Alice likes cats"
|
||||
mock_memory.type = "world"
|
||||
mock_memory.weight = 0.9
|
||||
|
||||
with patch.object(callback, "_recall_memories_sync", return_value=[mock_memory]) as mock_recall:
|
||||
callback.log_pre_api_call(
|
||||
model="gpt-4o-mini",
|
||||
messages=messages,
|
||||
kwargs=kwargs,
|
||||
)
|
||||
query_used = mock_recall.call_args[0][0]
|
||||
assert query_used == "What do I know about Alice?"
|
||||
|
||||
def test_should_skip_model_exact_match(self):
|
||||
"""Test model exclusion with exact match."""
|
||||
callback = HindsightCallback()
|
||||
|
|
|
|||
Loading…
Reference in a new issue