From 5fa203019a6825e9224962e267d136296c113793 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 13 May 2026 12:13:31 -0400 Subject: [PATCH] feat(consolidation): trigger from log_work and status terminal transitions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit log_work tool now invokes maybe_consolidate(reason='log_added') after a successful create_log. The gate inside the consolidation service handles threshold + setting checks. update_note service snapshots old_status before mutation and fires maybe_consolidate(reason='task_closed') when the status transitions into 'done' or 'cancelled'. Re-saving an already-terminal status doesn't retrigger — only transitions count. --- src/fabledassistant/services/notes.py | 9 ++ src/fabledassistant/services/tools/tasks.py | 4 + tests/test_notes_consolidation_trigger.py | 118 ++++++++++++++++++++ tests/test_tools_tasks.py | 58 ++++++++++ 4 files changed, 189 insertions(+) create mode 100644 tests/test_notes_consolidation_trigger.py create mode 100644 tests/test_tools_tasks.py diff --git a/src/fabledassistant/services/notes.py b/src/fabledassistant/services/notes.py index 0b64a41..4448b47 100644 --- a/src/fabledassistant/services/notes.py +++ b/src/fabledassistant/services/notes.py @@ -281,6 +281,8 @@ async def update_note(user_id: int, note_id: int, **fields: object) -> Note | No old_body = note.body old_title = note.title old_tags = list(note.tags or []) + # Snapshot status to detect terminal transitions for consolidation trigger. + old_status = note.status for key, value in fields.items(): if not hasattr(note, key): continue @@ -325,6 +327,13 @@ async def update_note(user_id: int, note_id: int, **fields: object) -> Note | No from fabledassistant.services.note_versions import create_version await create_version(user_id, note_id, old_body, old_title, old_tags) + # Trigger consolidation when a task transitions into a terminal status. + # Captured before mutation; the gate inside maybe_consolidate handles the + # auto-consolidate setting. + if note.status in ("done", "cancelled") and old_status != note.status: + from fabledassistant.services.consolidation import maybe_consolidate + await maybe_consolidate(user_id, note.id, reason="task_closed") + if note.project_id is not None: await _maybe_reactivate_project(note.project_id) await _maybe_trigger_project_summary(user_id, note.project_id) diff --git a/src/fabledassistant/services/tools/tasks.py b/src/fabledassistant/services/tools/tasks.py index c0a653b..65cc52b 100644 --- a/src/fabledassistant/services/tools/tasks.py +++ b/src/fabledassistant/services/tools/tasks.py @@ -113,4 +113,8 @@ async def log_work_tool(*, user_id, arguments, **_ctx): log = await _create_log(user_id, note.id, content, duration_minutes) except ValueError as e: return {"success": False, "error": str(e)} + + from fabledassistant.services.consolidation import maybe_consolidate + await maybe_consolidate(user_id, note.id, reason="log_added") + return {"success": True, "log": log.to_dict(), "task": note.title} diff --git a/tests/test_notes_consolidation_trigger.py b/tests/test_notes_consolidation_trigger.py new file mode 100644 index 0000000..cb8f109 --- /dev/null +++ b/tests/test_notes_consolidation_trigger.py @@ -0,0 +1,118 @@ +"""Tests for the status-terminal consolidation trigger in update_note.""" +from unittest.mock import AsyncMock, MagicMock, patch + + +def _mock_session_for_update(mock_note): + mock_session = AsyncMock() + mock_result = MagicMock() + mock_result.scalars.return_value.first.return_value = mock_note + mock_session.execute = AsyncMock(return_value=mock_result) + mock_session.commit = AsyncMock() + mock_session.refresh = AsyncMock() + mock_session.__aenter__ = AsyncMock(return_value=mock_session) + mock_session.__aexit__ = AsyncMock(return_value=False) + return mock_session + + +async def test_update_note_to_done_triggers_consolidation(): + """status: in_progress → done should fire maybe_consolidate(task_closed).""" + mock_note = MagicMock() + mock_note.id = 42 + mock_note.status = "in_progress" # pre-update state + mock_note.body = "" + mock_note.title = "" + mock_note.tags = [] + mock_note.started_at = None + mock_note.completed_at = None + mock_note.recurrence_rule = None + mock_note.project_id = None + + with patch( + "fabledassistant.services.notes.async_session", + return_value=_mock_session_for_update(mock_note), + ), patch( + "fabledassistant.services.consolidation.maybe_consolidate", + new=AsyncMock(), + ) as mock_mc: + from fabledassistant.services.notes import update_note + await update_note(1, 42, status="done") + + mock_mc.assert_awaited_once_with(1, 42, reason="task_closed") + + +async def test_update_note_to_cancelled_triggers_consolidation(): + mock_note = MagicMock() + mock_note.id = 42 + mock_note.status = "in_progress" + mock_note.body = "" + mock_note.title = "" + mock_note.tags = [] + mock_note.started_at = None + mock_note.completed_at = None + mock_note.recurrence_rule = None + mock_note.project_id = None + + with patch( + "fabledassistant.services.notes.async_session", + return_value=_mock_session_for_update(mock_note), + ), patch( + "fabledassistant.services.consolidation.maybe_consolidate", + new=AsyncMock(), + ) as mock_mc: + from fabledassistant.services.notes import update_note + await update_note(1, 42, status="cancelled") + + mock_mc.assert_awaited_once_with(1, 42, reason="task_closed") + + +async def test_update_note_to_in_progress_does_not_trigger_consolidation(): + """Non-terminal status changes don't fire the trigger.""" + mock_note = MagicMock() + mock_note.id = 42 + mock_note.status = "todo" + mock_note.body = "" + mock_note.title = "" + mock_note.tags = [] + mock_note.started_at = None + mock_note.completed_at = None + mock_note.recurrence_rule = None + mock_note.project_id = None + + with patch( + "fabledassistant.services.notes.async_session", + return_value=_mock_session_for_update(mock_note), + ), patch( + "fabledassistant.services.consolidation.maybe_consolidate", + new=AsyncMock(), + ) as mock_mc: + from fabledassistant.services.notes import update_note + await update_note(1, 42, status="in_progress") + + mock_mc.assert_not_awaited() + + +async def test_update_note_already_done_does_not_retrigger(): + """If the status was already 'done' and update_note(status='done') runs + again, no fresh trigger fires — only transitions count.""" + mock_note = MagicMock() + mock_note.id = 42 + mock_note.status = "done" + mock_note.body = "" + mock_note.title = "" + mock_note.tags = [] + mock_note.started_at = None + mock_note.completed_at = None + mock_note.recurrence_rule = None + mock_note.project_id = None + + with patch( + "fabledassistant.services.notes.async_session", + return_value=_mock_session_for_update(mock_note), + ), patch( + "fabledassistant.services.consolidation.maybe_consolidate", + new=AsyncMock(), + ) as mock_mc: + from fabledassistant.services.notes import update_note + await update_note(1, 42, status="done") + + mock_mc.assert_not_awaited() diff --git a/tests/test_tools_tasks.py b/tests/test_tools_tasks.py new file mode 100644 index 0000000..8d7d4a0 --- /dev/null +++ b/tests/test_tools_tasks.py @@ -0,0 +1,58 @@ +"""Tests for the task tools — log_work in particular wires into the +consolidation pipeline after every successful log.""" +from types import SimpleNamespace +from unittest.mock import AsyncMock, patch + + +async def test_log_work_tool_invokes_maybe_consolidate(): + """log_work must call maybe_consolidate(user_id, task_id, reason='log_added') + after a successful task_logs.create_log.""" + from fabledassistant.services.tools import tasks as tasks_tool + + fake_task = SimpleNamespace(id=42, title="X", status="in_progress") + fake_log = SimpleNamespace( + to_dict=lambda: {"id": 1, "content": "did stuff"}, + ) + + # get_note_by_title is imported at the top of tools/tasks.py — patch the + # consumer module's bound symbol, not the source. + with patch( + "fabledassistant.services.tools.tasks.get_note_by_title", + new=AsyncMock(return_value=fake_task), + ), patch( + "fabledassistant.services.task_logs.create_log", + new=AsyncMock(return_value=fake_log), + ), patch( + "fabledassistant.services.consolidation.maybe_consolidate", + new=AsyncMock(), + ) as mock_mc: + result = await tasks_tool.log_work_tool( + user_id=1, arguments={"task": "X", "content": "did stuff"}, + ) + + assert result["success"] is True + mock_mc.assert_awaited_once_with(1, 42, reason="log_added") + + +async def test_log_work_tool_does_not_trigger_on_create_log_failure(): + """If create_log raises ValueError, maybe_consolidate must not be called.""" + from fabledassistant.services.tools import tasks as tasks_tool + + fake_task = SimpleNamespace(id=42, title="X", status="in_progress") + + with patch( + "fabledassistant.services.tools.tasks.get_note_by_title", + new=AsyncMock(return_value=fake_task), + ), patch( + "fabledassistant.services.task_logs.create_log", + new=AsyncMock(side_effect=ValueError("bad input")), + ), patch( + "fabledassistant.services.consolidation.maybe_consolidate", + new=AsyncMock(), + ) as mock_mc: + result = await tasks_tool.log_work_tool( + user_id=1, arguments={"task": "X", "content": "did stuff"}, + ) + + assert result["success"] is False + mock_mc.assert_not_awaited()