feat(consolidation): trigger from log_work and status terminal transitions
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.
This commit is contained in:
@@ -281,6 +281,8 @@ async def update_note(user_id: int, note_id: int, **fields: object) -> Note | No
|
|||||||
old_body = note.body
|
old_body = note.body
|
||||||
old_title = note.title
|
old_title = note.title
|
||||||
old_tags = list(note.tags or [])
|
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():
|
for key, value in fields.items():
|
||||||
if not hasattr(note, key):
|
if not hasattr(note, key):
|
||||||
continue
|
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
|
from fabledassistant.services.note_versions import create_version
|
||||||
await create_version(user_id, note_id, old_body, old_title, old_tags)
|
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:
|
if note.project_id is not None:
|
||||||
await _maybe_reactivate_project(note.project_id)
|
await _maybe_reactivate_project(note.project_id)
|
||||||
await _maybe_trigger_project_summary(user_id, note.project_id)
|
await _maybe_trigger_project_summary(user_id, note.project_id)
|
||||||
|
|||||||
@@ -113,4 +113,8 @@ async def log_work_tool(*, user_id, arguments, **_ctx):
|
|||||||
log = await _create_log(user_id, note.id, content, duration_minutes)
|
log = await _create_log(user_id, note.id, content, duration_minutes)
|
||||||
except ValueError as e:
|
except ValueError as e:
|
||||||
return {"success": False, "error": str(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}
|
return {"success": True, "log": log.to_dict(), "task": note.title}
|
||||||
|
|||||||
@@ -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()
|
||||||
@@ -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()
|
||||||
Reference in New Issue
Block a user