mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-06-15 17:25:26 -04:00
35b4dd2824
* docs: add implementation plan for fixing chat context drifting (#135) * fix: make Session.history immutable + fix {}.history crash - Session.history now exposes a COPY of the internal _history list - add_message() replaces history with a fresh copy each time - get_context_messages() derives from _history directly - replace_messages() updates both _history and history - truncate_messages() updates both _history and history - _persist_message() line 207: fixed {}.history fallback crash - Added 11 tests for session isolation and edge cases Addresses #135 root cause #1: shared mutable references * fix: task scheduler uses SessionManager methods instead of overwriting sessions - Added ensure_task_session() to SessionManager (checks cache first) - Task scheduler now uses ensure_task_session() instead of direct dict assignment - Task scheduler now uses SessionManager.add_message() for message persistence - Removed direct sess_obj.history.append() that was silently losing data Addresses #135 root causes #2 and #3 * fix: add age guard to cleanup_empty_sessions — don't delete sessions <1h old Prevents the cleanup task from deleting sessions that were just created and haven't received any messages yet (message_count == 0). Addresses #135 root cause #5 * test: comprehensive session isolation tests (10/10 passing) * refactor: consolidate _session_manager into singleton pattern - Added set_session_manager_instance / get_session_manager_instance to core/models - kept backward-compat aliases (set_session_manager, get_session_manager) - session_manager.py re-exports the singleton functions - ai_interaction.set_session_manager now syncs with the core singleton - context_compactor uses get_session_manager_instance() instead of getattr hack - app.py initializes the singleton once Addresses #135 root cause #4: fragile global wiring * test: add concurrent session isolation integration tests Verifies: - Concurrent add_message to different sessions doesn't cross-contaminate - Rapid parallel writes maintain isolation - Read-write concurrent access is safe All 3 async tests pass, proving the immutable history fix works under concurrency * fix: pre-import core.models in conftest to prevent test pollution test_agent_loop.py stubs sys.modules['core.models'] = MagicMock() at module level during collection. Any test collected after it imports Session as a MagicMock. Pre-importing core.models in conftest.py before test_agent_loop.py's module-level code runs prevents this. * fix: make .history authoritative mutable list, address PR review Per review feedback: keep .history as the authoritative mutable list so existing code doing .history.pop(), .history = [...], etc. still works. Fix the cross-contamination bug by ensuring __post_init__() gives each Session its OWN unique history list (never shared). Changes: - core/models.py: .history IS the authoritative list. _history aliases it. Each Session gets its own list in __post_init__. - core/session_manager.py: add_message() delegates to Session.add_message() instead of appending directly — no double-append, single source of truth. - tests/test_session_manager.py: updated test to reflect that .history references see new messages (same list, not a snapshot). - docs/plans/2026-06-01-fix-chat-context-drifting.md: removed (not for shipping — useful design context but too much process/doc to ship). All 272 tests pass (3 pre-existing failures unrelated). * Fix session manager message persistence * Fix session history alias regressions * Fix session history aliasing and task delivery
132 lines
4.1 KiB
Python
132 lines
4.1 KiB
Python
# core/models.py
|
|
"""
|
|
Pure data models — no database logic, no side effects.
|
|
|
|
These are simple datacontainers. All persistence is handled by SessionManager.
|
|
"""
|
|
|
|
from dataclasses import dataclass
|
|
from typing import Dict, List, Any, Optional, TYPE_CHECKING
|
|
|
|
if TYPE_CHECKING:
|
|
from .session_manager import SessionManager
|
|
|
|
# Module-level session manager singleton (single source of truth)
|
|
_SESSION_MANAGER_INSTANCE: Optional["SessionManager"] = None
|
|
|
|
|
|
def set_session_manager_instance(manager: "SessionManager"):
|
|
"""Set the global SessionManager singleton."""
|
|
global _SESSION_MANAGER_INSTANCE
|
|
_SESSION_MANAGER_INSTANCE = manager
|
|
|
|
|
|
def get_session_manager_instance() -> Optional["SessionManager"]:
|
|
"""Get the global SessionManager singleton."""
|
|
return _SESSION_MANAGER_INSTANCE
|
|
|
|
|
|
# Keep legacy name for backward compatibility
|
|
set_session_manager = set_session_manager_instance
|
|
get_session_manager = get_session_manager_instance
|
|
|
|
|
|
@dataclass
|
|
class ChatMessage:
|
|
"""A single chat message."""
|
|
role: str
|
|
content: str
|
|
metadata: Optional[Dict[str, Any]] = None
|
|
|
|
def to_dict(self) -> Dict[str, Any]:
|
|
"""Convert to dict for API responses."""
|
|
result = {"role": self.role, "content": self.content}
|
|
if self.metadata:
|
|
result["metadata"] = self.metadata
|
|
return result
|
|
|
|
def get(self, key: str, default=None):
|
|
"""Dict-like access for compatibility."""
|
|
return getattr(self, key, default)
|
|
|
|
|
|
@dataclass
|
|
class Session:
|
|
"""A chat session — pure data container.
|
|
|
|
``.history`` is the authoritative mutable message list. Callers may
|
|
read, append, pop, or reassign it directly — these changes take
|
|
effect immediately. ``_history`` remains a compatibility alias that
|
|
always resolves to the authoritative ``history`` list.
|
|
|
|
Each session gets its own unique history list at construction time
|
|
(the dataclass default is never shared between instances).
|
|
"""
|
|
|
|
id: str
|
|
name: str
|
|
endpoint_url: str
|
|
model: str
|
|
rag: bool = False
|
|
archived: bool = False
|
|
headers: Optional[Dict[str, str]] = None
|
|
history: List[ChatMessage] = None
|
|
owner: Optional[str] = None
|
|
is_important: bool = False
|
|
message_count: int = 0
|
|
|
|
def __post_init__(self):
|
|
if self.headers is None:
|
|
self.headers = {}
|
|
# Ensure each session gets its OWN list (not the shared dataclass default)
|
|
if self.history is None:
|
|
self.history = []
|
|
|
|
@property
|
|
def _history(self) -> List[ChatMessage]:
|
|
"""Compatibility alias for callers that still reference ``_history``."""
|
|
return self.history
|
|
|
|
@_history.setter
|
|
def _history(self, messages: List[ChatMessage]):
|
|
self.history = messages
|
|
|
|
def add_message(self, message: ChatMessage):
|
|
"""
|
|
Add a message to this session.
|
|
|
|
Appends to the authoritative history list and increments
|
|
message_count. Delegates to SessionManager for persistence
|
|
if available.
|
|
"""
|
|
self.history.append(message)
|
|
self.message_count = len(self.history)
|
|
|
|
# Delegate to session manager for persistence
|
|
if _SESSION_MANAGER_INSTANCE:
|
|
_SESSION_MANAGER_INSTANCE._persist_message(self.id, message)
|
|
|
|
def get_context_messages(self) -> List[Dict[str, Any]]:
|
|
"""Get messages in format for LLM API.
|
|
|
|
Slash-command / setup replies are persisted to history so they render
|
|
in the transcript, but they are UI chatter (e.g. ``/setup ...`` and its
|
|
status lines) the user never meant as conversation. They carry
|
|
``metadata.source == "slash"``; exclude them here so they never reach
|
|
the model. Display/history-load paths use the raw ``history`` and are
|
|
unaffected.
|
|
"""
|
|
return [
|
|
msg.to_dict()
|
|
for msg in self.history
|
|
if (msg.metadata or {}).get("source") != "slash"
|
|
]
|
|
|
|
def get(self, key: str, default=None):
|
|
"""Dict-like access for compatibility."""
|
|
return getattr(self, key, default)
|
|
|
|
def __getitem__(self, key: str):
|
|
"""Allow session['field'] syntax."""
|
|
return getattr(self, key)
|