Skip to content

Commit 12cafe5

Browse files
fix: Address 4 architectural blockers in PraisonAISessionDataStore adapter
Fixes violating AGENTS.md §4.2 (fail loudly) and §4.6 (safe defaults): - Blocker 1: Remove silent ImportError fallback, fail loudly on missing aiui/praisonaiagents deps - Blocker 2: Implement proper list_sessions() that calls store.list_sessions(limit=50) when available - Blocker 3: Update tests to assert correct behavior instead of broken behavior (list_sessions delegation) - Blocker 4: Remove no-op test_import_fallback that passed regardless of behavior Also adds proper exception logging with logger.exception() for better debugging. All changes follow AGENTS.md principles: minimal, protocol-driven, backward-compatible. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 523ca60 commit 12cafe5

2 files changed

Lines changed: 51 additions & 33 deletions

File tree

src/praisonai/praisonai/ui/_aiui_datastore.py

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,36 +5,36 @@
55
"""
66
from __future__ import annotations
77

8+
import logging
89
import uuid
910
from typing import Any, Optional
1011

12+
logger = logging.getLogger(__name__)
13+
14+
# Fail loudly on missing optional dependencies per AGENTS.md §4.2
1115
try:
1216
from praisonaiui.datastore import BaseDataStore
13-
except ImportError:
14-
# Fallback for when aiui is not installed
15-
class BaseDataStore:
16-
pass
17+
except ImportError as e:
18+
raise ImportError(
19+
"praisonaiui is required for PraisonAISessionDataStore. "
20+
"Install with: pip install 'praisonai[ui]'"
21+
) from e
1722

1823
try:
1924
from praisonaiagents.session import SessionStoreProtocol
2025
from praisonaiagents.session import get_hierarchical_session_store
21-
except ImportError:
22-
# Fallback when praisonaiagents not available
23-
SessionStoreProtocol = None
24-
get_hierarchical_session_store = None
26+
except ImportError as e:
27+
raise ImportError(
28+
"praisonaiagents is required for PraisonAISessionDataStore. "
29+
"Install with: pip install praisonaiagents"
30+
) from e
2531

2632

2733
class PraisonAISessionDataStore(BaseDataStore):
2834
"""Adapter that bridges PraisonAI SessionStoreProtocol to aiui BaseDataStore."""
2935

3036
def __init__(self, store: Optional[SessionStoreProtocol] = None):
3137
"""Initialize with an optional session store, defaults to hierarchical store."""
32-
if get_hierarchical_session_store is None:
33-
raise ImportError(
34-
"praisonaiagents is required for PraisonAISessionDataStore. "
35-
"Install with: pip install praisonaiagents"
36-
)
37-
3838
self._store = store or get_hierarchical_session_store()
3939

4040
def _new_id(self) -> str:
@@ -43,11 +43,17 @@ def _new_id(self) -> str:
4343

4444
async def list_sessions(self) -> list[dict[str, Any]]:
4545
"""List all available sessions."""
46-
# The SessionStoreProtocol doesn't have a list method,
47-
# so we'll need to work around this limitation
48-
# For now, return empty list - this would need enhancement
49-
# in the core SDK to support session listing
50-
return []
46+
# Check if store supports listing (DefaultSessionStore/HierarchicalSessionStore do)
47+
list_fn = getattr(self._store, "list_sessions", None)
48+
if list_fn is None:
49+
return [] # Protocol implementation doesn't support listing
50+
51+
try:
52+
# DefaultSessionStore/HierarchicalSessionStore return list[dict]
53+
return list_fn(limit=50) or []
54+
except Exception:
55+
logger.exception("Failed to list sessions")
56+
return []
5157

5258
async def get_session(self, session_id: str) -> Optional[dict[str, Any]]:
5359
"""Get a specific session by ID."""
@@ -61,7 +67,7 @@ async def get_session(self, session_id: str) -> Optional[dict[str, Any]]:
6167
"messages": chat_history or [],
6268
}
6369
except Exception:
64-
# Session might exist but be corrupted
70+
logger.exception("Failed to load session %s", session_id)
6571
return None
6672

6773
async def create_session(self, session_id: Optional[str] = None) -> dict[str, Any]:
@@ -78,6 +84,7 @@ async def delete_session(self, session_id: str) -> bool:
7884
try:
7985
return self._store.delete_session(session_id)
8086
except Exception:
87+
logger.exception("Failed to delete session %s", session_id)
8188
return False
8289

8390
async def add_message(self, session_id: str, message: dict[str, Any]):
@@ -97,4 +104,5 @@ async def get_messages(self, session_id: str) -> list[dict[str, Any]]:
97104
try:
98105
return self._store.get_chat_history(session_id) or []
99106
except Exception:
107+
logger.exception("Failed to load messages for session %s", session_id)
100108
return []

src/praisonai/tests/unit/test_aiui_datastore.py

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -201,24 +201,34 @@ async def test_get_messages_error(self):
201201
assert result == []
202202

203203
@pytest.mark.asyncio
204-
async def test_list_sessions_empty(self):
205-
"""Test listing sessions when none exist."""
204+
async def test_list_sessions_delegates_to_store(self):
205+
"""Test listing sessions delegates to store when available."""
206206
mock_store = Mock()
207+
mock_store.list_sessions.return_value = [
208+
{"session_id": "a", "created_at": 1, "message_count": 3},
209+
{"session_id": "b", "created_at": 2, "message_count": 5},
210+
]
211+
adapter = PraisonAISessionDataStore(store=mock_store)
212+
213+
result = await adapter.list_sessions()
214+
215+
assert len(result) == 2
216+
assert result[0]["session_id"] == "a"
217+
assert result[1]["session_id"] == "b"
218+
mock_store.list_sessions.assert_called_once_with(limit=50)
219+
220+
@pytest.mark.asyncio
221+
async def test_list_sessions_store_without_list(self):
222+
"""Test listing sessions when store doesn't support listing."""
223+
# Custom SessionStoreProtocol impls don't require list_sessions
224+
mock_store = Mock(spec=["add_message", "get_chat_history",
225+
"clear_session", "delete_session", "session_exists"])
207226
adapter = PraisonAISessionDataStore(store=mock_store)
208227

209228
result = await adapter.list_sessions()
210229

211-
# Current implementation returns empty list
212-
# This is a limitation that would need core SDK enhancement
213230
assert result == []
214231

215232

216-
def test_import_fallback():
217-
"""Test that module handles missing dependencies gracefully."""
218-
# This test verifies the import fallbacks work correctly
219-
# The actual adapter requires praisonaiagents to be installed
220-
try:
221-
from praisonai.ui._aiui_datastore import BaseDataStore
222-
except ImportError:
223-
# If aiui is not available, BaseDataStore should be a fallback class
224-
pass
233+
# Removed test_import_fallback - was a no-op test that passed regardless of behavior
234+
# After Blocker 1 fix, imports now fail loudly instead of falling back silently

0 commit comments

Comments
 (0)