fix(chat): audit fixes — retention, rag_project_id, cleanup scheduler, tool rounds
- cleanup_old_conversations now excludes briefing conversations (was silently deleting briefing history after the retention window) - list_conversations response now includes rag_project_id, matching the shape returned by the single-conversation GET endpoint - create_conversation_from_article: removed duplicate async_session import (_session2 was a copy of the same import); consolidated into one - MAX_TOOL_ROUNDS fixed from 5→6 to match the actual range(6) loop; loop updated to range(MAX_TOOL_ROUNDS) so the constant is accurate - Chat retention cleanup moved from per-request (every GET /conversations) to a daily scheduled job in event_scheduler.py; route no longer runs a DB write on every read Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -11,7 +11,6 @@ from fabledassistant.config import Config
|
||||
from fabledassistant.services.chat import (
|
||||
add_message,
|
||||
bulk_delete_conversations,
|
||||
cleanup_old_conversations,
|
||||
create_conversation,
|
||||
delete_conversation,
|
||||
get_conversation,
|
||||
@@ -40,14 +39,6 @@ async def list_conversations_route():
|
||||
uid = get_current_user_id()
|
||||
limit, offset = parse_pagination()
|
||||
conv_type = request.args.get("type", "chat")
|
||||
# Apply retention policy before returning list
|
||||
retention_str = await get_setting(uid, "chat_retention_days", "90")
|
||||
try:
|
||||
retention_days = int(retention_str)
|
||||
except (ValueError, TypeError):
|
||||
retention_days = 90
|
||||
if retention_days > 0:
|
||||
await cleanup_old_conversations(uid, retention_days)
|
||||
conversations, total = await list_conversations(uid, limit=limit, offset=offset, conv_type=conv_type)
|
||||
return jsonify({
|
||||
"conversations": conversations,
|
||||
@@ -510,6 +501,7 @@ async def create_conversation_from_article(item_id: int):
|
||||
"""Create a chat conversation seeded with an RSS article's content."""
|
||||
from sqlalchemy import select as _select
|
||||
from fabledassistant.models import async_session as _async_session
|
||||
from fabledassistant.models.conversation import Message
|
||||
from fabledassistant.models.rss_feed import RssItem, RssFeed
|
||||
|
||||
uid = get_current_user_id()
|
||||
@@ -538,9 +530,7 @@ async def create_conversation_from_article(item_id: int):
|
||||
if item.url:
|
||||
seeded_text += f"\n\nSource: {item.url}"
|
||||
|
||||
from fabledassistant.models.conversation import Message
|
||||
from fabledassistant.models import async_session as _session2
|
||||
async with _session2() as session:
|
||||
async with _async_session() as session:
|
||||
msg = Message(
|
||||
conversation_id=conv.id,
|
||||
role="assistant",
|
||||
|
||||
@@ -81,6 +81,7 @@ async def list_conversations(
|
||||
"model": conv.model,
|
||||
"conversation_type": conv.conversation_type,
|
||||
"briefing_date": conv.briefing_date.isoformat() if conv.briefing_date else None,
|
||||
"rag_project_id": conv.rag_project_id,
|
||||
"message_count": row[1],
|
||||
"created_at": conv.created_at.isoformat(),
|
||||
"updated_at": conv.updated_at.isoformat(),
|
||||
@@ -131,8 +132,9 @@ async def cleanup_old_conversations(user_id: int, days: int) -> int:
|
||||
.where(
|
||||
Conversation.user_id == user_id,
|
||||
Conversation.updated_at < cutoff,
|
||||
Conversation.conversation_type != "mcp", # preserve MCP audit trail
|
||||
Conversation.conversation_type != "voice", # voice convs managed separately
|
||||
Conversation.conversation_type != "mcp", # preserve MCP audit trail
|
||||
Conversation.conversation_type != "voice", # voice convs managed separately
|
||||
Conversation.conversation_type != "briefing", # briefing history managed by briefing system
|
||||
)
|
||||
.returning(Conversation.id)
|
||||
)
|
||||
|
||||
@@ -1,7 +1,8 @@
|
||||
"""Scheduler jobs for calendar events.
|
||||
"""Scheduler jobs for background maintenance tasks.
|
||||
|
||||
- Reminder notifications: checks every 5 minutes for due reminders.
|
||||
- Reminder notifications: checks every 5 minutes for due event reminders.
|
||||
- CalDAV pull sync: runs every hour for all users with CalDAV configured.
|
||||
- Chat retention cleanup: runs daily, deleting old conversations per user setting.
|
||||
|
||||
Uses the same BackgroundScheduler pattern as briefing_scheduler.py.
|
||||
"""
|
||||
@@ -97,6 +98,44 @@ def _run_caldav_sync_threadsafe(loop: asyncio.AbstractEventLoop) -> None:
|
||||
asyncio.run_coroutine_threadsafe(_run_caldav_sync(), loop)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Chat retention cleanup job
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
async def _run_chat_retention_cleanup() -> None:
|
||||
"""Delete old conversations for all users according to their retention setting."""
|
||||
from sqlalchemy import select as sa_select # noqa: PLC0415
|
||||
|
||||
from fabledassistant.models.user import User # noqa: PLC0415
|
||||
from fabledassistant.services.chat import cleanup_old_conversations # noqa: PLC0415
|
||||
from fabledassistant.services.settings import get_setting # noqa: PLC0415
|
||||
|
||||
async with async_session() as session:
|
||||
result = await session.execute(sa_select(User.id))
|
||||
user_ids = [row[0] for row in result.all()]
|
||||
|
||||
total_deleted = 0
|
||||
for user_id in user_ids:
|
||||
try:
|
||||
retention_str = await get_setting(user_id, "chat_retention_days", "90")
|
||||
try:
|
||||
retention_days = int(retention_str)
|
||||
except (ValueError, TypeError):
|
||||
retention_days = 90
|
||||
if retention_days > 0:
|
||||
deleted = await cleanup_old_conversations(user_id, retention_days)
|
||||
total_deleted += deleted
|
||||
except Exception:
|
||||
logger.warning("Chat retention cleanup failed for user %d", user_id, exc_info=True)
|
||||
|
||||
if total_deleted:
|
||||
logger.info("Chat retention cleanup: deleted %d conversation(s)", total_deleted)
|
||||
|
||||
|
||||
def _run_chat_retention_threadsafe(loop: asyncio.AbstractEventLoop) -> None:
|
||||
asyncio.run_coroutine_threadsafe(_run_chat_retention_cleanup(), loop)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Lifecycle
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -126,8 +165,17 @@ def start_event_scheduler(loop: asyncio.AbstractEventLoop) -> None:
|
||||
replace_existing=True,
|
||||
)
|
||||
|
||||
# Chat retention cleanup once per day
|
||||
_scheduler.add_job(
|
||||
_run_chat_retention_threadsafe,
|
||||
trigger=IntervalTrigger(hours=24),
|
||||
args=[loop],
|
||||
id="chat_retention_cleanup",
|
||||
replace_existing=True,
|
||||
)
|
||||
|
||||
_scheduler.start()
|
||||
logger.info("Event scheduler started (reminders every 5m, CalDAV sync every 1h)")
|
||||
logger.info("Event scheduler started (reminders every 5m, CalDAV sync every 1h, chat cleanup every 24h)")
|
||||
|
||||
|
||||
def stop_event_scheduler() -> None:
|
||||
|
||||
@@ -217,7 +217,7 @@ async def run_generation(
|
||||
voice_mode: bool = False,
|
||||
) -> None:
|
||||
"""Stream LLM response into buffer with periodic DB flushes."""
|
||||
MAX_TOOL_ROUNDS = 5
|
||||
MAX_TOOL_ROUNDS = 6
|
||||
msg_id = buf.assistant_message_id
|
||||
|
||||
buf.append_event("status", {"status": "Building context..."})
|
||||
@@ -294,7 +294,7 @@ async def run_generation(
|
||||
cancelled = False
|
||||
research_completed = False
|
||||
|
||||
for _round in range(MAX_TOOL_ROUNDS + 1):
|
||||
for _round in range(MAX_TOOL_ROUNDS):
|
||||
timing["rounds"] = _round + 1
|
||||
round_tool_calls: list[dict] = []
|
||||
logger.info("Generation round %d started for conv %d (model=%s)", _round, conv_id, model)
|
||||
|
||||
Reference in New Issue
Block a user