Skip to content

Commit e0e39e4

Browse files
Copilotekzhu
andauthored
Fix Redis caching always returning False due to unhandled string values (#7022)
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: ekzhu <320302+ekzhu@users.noreply.github.com> Co-authored-by: Eric Zhu <ekzhu@users.noreply.github.com>
1 parent 29846cb commit e0e39e4

File tree

2 files changed

+148
-0
lines changed

2 files changed

+148
-0
lines changed

python/packages/autogen-ext/src/autogen_ext/models/cache/_chat_completion_cache.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,31 @@ def _check_cache(
223223
except ValidationError:
224224
# If reconstruction fails, treat as cache miss
225225
return None, cache_key
226+
elif isinstance(cached_result, str):
227+
# Handle case where cache store returns a string (e.g., Redis with decode errors)
228+
try:
229+
# Try to parse the string as JSON and reconstruct CreateResult
230+
parsed_data = json.loads(cached_result)
231+
if isinstance(parsed_data, dict):
232+
cached_result = CreateResult.model_validate(parsed_data)
233+
elif isinstance(parsed_data, list):
234+
# Handle streaming results stored as JSON string
235+
reconstructed_list_2: list[CreateResult | str] = []
236+
for item in parsed_data: # type: ignore[reportUnknownVariableType]
237+
if isinstance(item, dict):
238+
reconstructed_list_2.append(CreateResult.model_validate(item))
239+
elif isinstance(item, str):
240+
reconstructed_list_2.append(item)
241+
else:
242+
# If item is neither dict nor str, treat as cache miss
243+
return None, cache_key
244+
cached_result = reconstructed_list_2
245+
else:
246+
# If parsed data is not dict or list, treat as cache miss
247+
return None, cache_key
248+
except (json.JSONDecodeError, ValidationError):
249+
# If JSON parsing or validation fails, treat as cache miss
250+
return None, cache_key
226251
# If it's already the right type (CreateResult or list), return as-is
227252
return cached_result, cache_key
228253

python/packages/autogen-ext/tests/models/test_chat_completion_cache.py

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import copy
2+
import json
23
from typing import Any, Dict, List, Optional, Tuple, Union, cast
34

45
import pytest
@@ -482,6 +483,128 @@ def test_check_cache_already_correct_type() -> None:
482483
assert cache_key is not None
483484

484485

486+
def test_check_cache_string_json_deserialization_success() -> None:
487+
"""Test _check_cache when Redis cache returns a string containing valid JSON.
488+
This tests the fix for the Redis string caching issue where Redis returns
489+
string data instead of dict/CreateResult, causing cache misses.
490+
"""
491+
_, prompts, system_prompt, replay_client, _ = get_test_data()
492+
493+
# Create a JSON string representing a valid CreateResult
494+
create_result_json = json.dumps(
495+
{
496+
"content": "response from string json",
497+
"usage": {"prompt_tokens": 12, "completion_tokens": 6},
498+
"cached": False,
499+
"finish_reason": "stop",
500+
"logprobs": None,
501+
"thought": None,
502+
}
503+
)
504+
505+
# Mock cache store that returns the JSON string (simulating Redis behavior)
506+
mock_store = MockCacheStore(return_value=cast(Any, create_result_json))
507+
cached_client = ChatCompletionCache(replay_client, mock_store)
508+
509+
# Test _check_cache method directly
510+
messages = [system_prompt, UserMessage(content=prompts[0], source="user")]
511+
cached_result, cache_key = cached_client._check_cache(messages, [], None, {}) # type: ignore
512+
513+
# Should successfully reconstruct the CreateResult from JSON string
514+
assert cached_result is not None
515+
assert isinstance(cached_result, CreateResult)
516+
assert cached_result.content == "response from string json"
517+
assert cached_result.usage.prompt_tokens == 12
518+
assert cached_result.usage.completion_tokens == 6
519+
assert cache_key is not None
520+
521+
522+
def test_check_cache_string_json_list_deserialization_success() -> None:
523+
"""Test _check_cache when Redis cache returns a string containing valid JSON list.
524+
This tests the fix for streaming results stored as JSON strings in Redis.
525+
"""
526+
_, prompts, system_prompt, replay_client, _ = get_test_data()
527+
528+
# Create a JSON string representing a streaming result list
529+
streaming_list_json = json.dumps(
530+
[
531+
"streaming chunk 1",
532+
{
533+
"content": "streaming response from json",
534+
"usage": {"prompt_tokens": 8, "completion_tokens": 4},
535+
"cached": False,
536+
"finish_reason": "stop",
537+
"logprobs": None,
538+
"thought": None,
539+
},
540+
"streaming chunk 2",
541+
]
542+
)
543+
544+
# Mock cache store that returns the JSON string (simulating Redis streaming)
545+
mock_store = MockCacheStore(return_value=cast(Any, streaming_list_json))
546+
cached_client = ChatCompletionCache(replay_client, mock_store)
547+
548+
# Test _check_cache method directly
549+
messages = [system_prompt, UserMessage(content=prompts[0], source="user")]
550+
cached_result, cache_key = cached_client._check_cache(messages, [], None, {}) # type: ignore
551+
552+
# Should successfully reconstruct the list from JSON string
553+
assert cached_result is not None
554+
assert isinstance(cached_result, list)
555+
assert len(cached_result) == 3
556+
assert cached_result[0] == "streaming chunk 1"
557+
assert isinstance(cached_result[1], CreateResult)
558+
assert cached_result[1].content == "streaming response from json"
559+
assert cached_result[2] == "streaming chunk 2"
560+
assert cache_key is not None
561+
562+
563+
def test_check_cache_string_invalid_json_failure() -> None:
564+
"""Test _check_cache gracefully handles invalid JSON strings.
565+
This ensures the system degrades gracefully when Redis returns corrupted
566+
string data that cannot be parsed as JSON.
567+
"""
568+
_, prompts, system_prompt, replay_client, _ = get_test_data()
569+
570+
# Create an invalid JSON string
571+
invalid_json_string = '{"content": "test", invalid json}'
572+
573+
# Mock cache store that returns the invalid JSON string
574+
mock_store = MockCacheStore(return_value=cast(Any, invalid_json_string))
575+
cached_client = ChatCompletionCache(replay_client, mock_store)
576+
577+
# Test _check_cache method directly
578+
messages = [system_prompt, UserMessage(content=prompts[0], source="user")]
579+
cached_result, cache_key = cached_client._check_cache(messages, [], None, {}) # type: ignore
580+
581+
# Should return None (cache miss) when JSON parsing fails
582+
assert cached_result is None
583+
assert cache_key is not None
584+
585+
586+
def test_check_cache_string_invalid_data_failure() -> None:
587+
"""Test _check_cache gracefully handles JSON strings with invalid data structure.
588+
This ensures the system handles JSON that parses but doesn't represent valid CreateResult data.
589+
"""
590+
_, prompts, system_prompt, replay_client, _ = get_test_data()
591+
592+
# Create a JSON string that parses but has invalid structure
593+
invalid_data_json = json.dumps({"invalid_structure": "not a CreateResult"})
594+
595+
# Mock cache store that returns the invalid data JSON string
596+
mock_store = MockCacheStore(return_value=cast(Any, invalid_data_json))
597+
cached_client = ChatCompletionCache(replay_client, mock_store)
598+
599+
# Test _check_cache method directly
600+
messages = [system_prompt, UserMessage(content=prompts[0], source="user")]
601+
cached_result, cache_key = cached_client._check_cache(messages, [], None, {}) # type: ignore
602+
603+
# Should return None (cache miss) when validation fails
604+
assert cached_result is None
605+
assert cache_key is not None
606+
607+
485608
@pytest.mark.asyncio
486609
async def test_redis_streaming_cache_integration() -> None:
487610
"""Integration test for Redis streaming cache scenario.

0 commit comments

Comments
 (0)