From a7de3296b3448397bc71ba6358c8aab4d212d406 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 28 Apr 2026 19:44:22 -0400 Subject: [PATCH 1/7] docs(design-system): note Flutter app port MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a "Flutter app port — shipped 2026-04-28" section between the web Surface phase and Open threads. Records the two FabledApp commits (foundation 0f05f47, surface b9e68e3), explains the ActionColors ThemeExtension shape, points to reference call sites for the ActionColors.primary (calendar event Save) and ActionColors.destructive (confirm-Delete dialogs across notes/tasks/chat/calendar) patterns so downstream screens have a template to follow when reclassifying their own buttons opportunistically. Co-Authored-By: Claude Opus 4.7 --- docs/design-system.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/design-system.md b/docs/design-system.md index ab9f373..78e9df2 100644 --- a/docs/design-system.md +++ b/docs/design-system.md @@ -568,6 +568,17 @@ Items deliberately not addressed in this round; revisit when a real need surface - Standalone voice/tone audit across every UI string — opportunistic-only; full sweep deferred unless drift becomes visible. - A handful of editor utility buttons (`.btn-suggest-tags`, `.btn-link-all`, AI assist generate/proofread/accept/reject set, etc.) — currently ghost-styled and visually compliant; revisited only if they read off in practice. +### Flutter app port — shipped 2026-04-28 + +The companion mobile app (`fabled_app` / FabledApp repo) tracks the same design system. Two commits: + +- **Foundation port** — `0f05f47`. `lib/core/theme.dart` rewritten with the Obsidian/Iron/Pewter dark palette, warm parchment light palette, dusty violet `#5B4A8A` primary. Inter loaded for body, JetBrains Mono available at call sites, Fraunces for headlines ≥18px. New `ActionColors` ThemeExtension exposes Moss/Bronze/Oxblood/Pewter outside the `ColorScheme` (Material's primary/secondary/tertiary slots all carry brand accent, so action tokens need their own home). `GradientButton` recolored to dusty-violet gradient. +- **Surface phase** — `b9e68e3`. `lucide_icons ^0.257.0` installed; 107 `Icons.*` references across 21 files swapped to `LucideIcons.*`. Input border radius 24 → 8 in both themes. ChatMessageBubble Illuminated Transcript fixes — neutral border on user bubbles, `surface`/Iron bg on assistant bubbles, asymmetric corner restoration (only bottom-left clipped, not both left corners), accent-tinted glow shadow added. 5 destructive confirm buttons across notes / tasks / chat / calendar wired to `ActionColors.destructive`. Calendar event Save wired to `ActionColors.primary` as the reference Moss site. 4 hardcoded indigo Color literals → dusty-violet equivalents. + +The Flutter port doesn't decompose into 7 PRs the way web did because Flutter's centralized `theme.dart` means most palette/font work happens in one file. Per-screen Save / Cancel reclassification beyond the calendar event Save is opportunistic — the wiring pattern (`Theme.of(context).extension()!.primary`) is established and applied incrementally as files are touched. + +Pattern reference for downstream screens: see `lib/screens/calendar/event_form_sheet.dart` for `ActionColors.primary` usage on Save buttons; see the dialog spots in `note_edit_screen.dart` / `task_edit_screen.dart` / `note_detail_screen.dart` / `conversations_tab_screen.dart` for `ActionColors.destructive` on confirm-Delete buttons. + ### Open threads *New threads will accumulate here as gaps surface in real use.* From 3b2a0a119f40945738f2025e807e4689c065570c Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 29 Apr 2026 08:08:01 -0400 Subject: [PATCH 2/7] =?UTF-8?q?chore(fable-mcp):=20bump=20version=20to=200?= =?UTF-8?q?.3.0=20=E2=80=94=20journal=20tools=20replace=20briefing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The MCP server's briefing introspection tools were replaced with journal equivalents in c549827, but the package version stayed at 0.2.6 — so pipx upgrades against existing installs were no-ops and production MCP clients still served the obsolete briefing tools (which 404 against the migrated backend). Bumping minor since this is a breaking tool-surface change (briefing tools removed). Reinstall via `pipx install --force` to pick up. Co-Authored-By: Claude Opus 4.7 --- fable-mcp/pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fable-mcp/pyproject.toml b/fable-mcp/pyproject.toml index d3212e8..ff90611 100644 --- a/fable-mcp/pyproject.toml +++ b/fable-mcp/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "fable-mcp" -version = "0.2.6" +version = "0.3.0" description = "MCP server for Fabled Scribe" requires-python = ">=3.12" dependencies = [ From 611c9405275c80477cb00b1e432dd8b5e046416a Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 29 Apr 2026 08:16:25 -0400 Subject: [PATCH 3/7] fix(calendar-tool): split start/end into date+time to make event creation TZ-durable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A user reported "next Friday at 8am" landing on the wrong day. The current `start` parameter accepts a combined ISO datetime string — when the model emits something like `"2026-05-01T00:00:00Z"`, the parser correctly honors the UTC tag and stores `2026-05-01 00:00 UTC`, which displays as `2026-04-30 19:00` for a UTC-5 user. The bug isn't in our parser; it's that we let the model TZ-tag the calendar day at all. The fix moves the foot-gun: `create_event` and `update_event` now prefer split fields (`start_date` + `start_time`, plus end variants). A `YYYY-MM-DD` string carries no TZ metadata for a model to mis-tag, and the backend builds the local datetime explicitly via `datetime.combine(date, time, tzinfo=user_tz).astimezone(UTC)`. Strict regex validation rejects anything with a TZ suffix on either field. The legacy combined `start` / `end` fields are kept as a fallback so saved tool-call payloads in conversation history still replay; new calls are steered toward the split shape via the tool description. 7 new regression tests cover Eastern, Pacific, Tokyo (positive offset), all-day inference, strict-shape rejection on both fields, backcompat with the legacy `start` field, and the same fix for `update_event`. 27 of the event-related tests pass; ruff clean. Co-Authored-By: Claude Opus 4.7 --- .../services/tools/calendar.py | 168 ++++++++--- tests/test_calendar_tool_tz.py | 263 ++++++++++++++++++ 2 files changed, 395 insertions(+), 36 deletions(-) diff --git a/src/fabledassistant/services/tools/calendar.py b/src/fabledassistant/services/tools/calendar.py index 90828e2..12fc43c 100644 --- a/src/fabledassistant/services/tools/calendar.py +++ b/src/fabledassistant/services/tools/calendar.py @@ -2,7 +2,8 @@ from __future__ import annotations -from datetime import datetime, timezone +import re +from datetime import date as _date, datetime, time as _time, timezone from fabledassistant.services.events import ( create_event as events_create_event, @@ -17,10 +18,49 @@ from fabledassistant.services.tools._registry import tool from fabledassistant.services.tz import get_user_tz +_DATE_RE = re.compile(r"^\d{4}-\d{2}-\d{2}$") +_TIME_RE = re.compile(r"^\d{2}:\d{2}(:\d{2})?$") + + +async def _combine_local_in_user_tz( + user_id: int, date_str: str, time_str: str | None +) -> datetime: + """Build a UTC datetime from separate date and time strings. + + The whole point of this helper: a `YYYY-MM-DD` string carries no TZ + metadata that a model could mis-tag, so the calendar day cannot drift + across the local→UTC boundary. The wall-clock `HH:MM` likewise has no + TZ; we attach the user's local zone explicitly via ``datetime.combine`` + and then convert to UTC for storage. + + Strict shape validation rejects anything that isn't a bare date or a + bare time — no `2026-05-01Z`, no `08:00 UTC` slipping through. + """ + if not _DATE_RE.match(date_str): + raise ValueError( + f"start_date / end_date must be YYYY-MM-DD with no timezone; got {date_str!r}" + ) + d = _date.fromisoformat(date_str) + if time_str is None or time_str == "": + t = _time(0, 0) + else: + if not _TIME_RE.match(time_str): + raise ValueError( + f"start_time / end_time must be HH:MM (or HH:MM:SS), no timezone; got {time_str!r}" + ) + t = _time.fromisoformat(time_str) + user_tz = await get_user_tz(user_id) + local = datetime.combine(d, t, tzinfo=user_tz) + return local.astimezone(timezone.utc) + + async def _parse_datetime_in_user_tz( user_id: int, value: str ) -> tuple[datetime, bool]: - """Parse a date/datetime string from the model into a UTC-aware datetime. + """Legacy single-string parser. Kept as a fallback when the model + emits the older `start` / `end` shape; new calls should use + `start_date`+`start_time` (and `end_date`+`end_time`) which sidestep + the TZ-tagging foot-gun this parser is vulnerable to. Naive inputs are interpreted in the **user's local timezone** and then converted to UTC for storage. Never default to UTC for naive inputs — @@ -38,14 +78,54 @@ async def _parse_datetime_in_user_tz( return dt.astimezone(timezone.utc), was_date_only +async def _resolve_event_start( + user_id: int, args: dict +) -> tuple[datetime, bool]: + """Resolve the start datetime from either the new split fields + (`start_date` + optional `start_time`) or the legacy combined `start`. + Returns ``(utc_datetime, was_date_only)``.""" + if "start_date" in args and args["start_date"]: + date_str = args["start_date"] + time_str = args.get("start_time") or None + return await _combine_local_in_user_tz(user_id, date_str, time_str), time_str is None + if "start" in args and args["start"]: + return await _parse_datetime_in_user_tz(user_id, args["start"]) + raise ValueError("Either start_date or start is required") + + +async def _resolve_event_end( + user_id: int, args: dict +) -> datetime | None: + """Resolve the end datetime from either the new split fields or the + legacy combined `end`. Returns ``None`` when no end fields are set.""" + if "end_date" in args and args["end_date"]: + date_str = args["end_date"] + time_str = args.get("end_time") or None + return await _combine_local_in_user_tz(user_id, date_str, time_str) + if "end" in args and args["end"]: + dt, _ = await _parse_datetime_in_user_tz(user_id, args["end"]) + return dt + return None + + @tool( name="create_event", - description="Create a calendar event for the user. Use this when the user asks to schedule, add, or create a meeting, appointment, or event. Pass dates and datetimes in the user's local time — a bare date like '2026-09-30' is interpreted as that day in the user's configured timezone.", + description=( + "Create a calendar event for the user. Use this when the user asks " + "to schedule, add, or create a meeting, appointment, or event. " + "Always pass `start_date` (YYYY-MM-DD) and `start_time` (HH:MM) as " + "separate fields in the user's local time — never combine them and " + "never include a timezone suffix. The server attaches the user's " + "configured timezone. Omit `start_time` (or set `all_day=true`) " + "for all-day events like birthdays or holidays." + ), parameters={ "title": {"type": "string", "description": "A descriptive event title"}, - "start": {"type": "string", "description": "Start date (YYYY-MM-DD) or datetime (YYYY-MM-DDTHH:MM) in the user's local time. Include a timezone offset only if the user explicitly mentions one."}, - "end": {"type": "string", "description": "Optional end date or datetime in the user's local time"}, - "duration": {"type": "integer", "description": "Optional duration in minutes (default 60, ignored if end is set or all_day is true)"}, + "start_date": {"type": "string", "description": "Start calendar date as YYYY-MM-DD in the user's local time. No timezone suffix."}, + "start_time": {"type": "string", "description": "Start wall-clock time as HH:MM (24-hour). Omit for all-day events. No timezone suffix."}, + "end_date": {"type": "string", "description": "Optional end calendar date as YYYY-MM-DD."}, + "end_time": {"type": "string", "description": "Optional end wall-clock time as HH:MM."}, + "duration": {"type": "integer", "description": "Optional duration in minutes (default 60, ignored if end_date/end_time is set or all_day is true)"}, "description": {"type": "string", "description": "Optional event description"}, "location": {"type": "string", "description": "Optional event location"}, "color": {"type": "string", "description": "Optional hex color for the event (e.g. '#6366f1')"}, @@ -55,30 +135,31 @@ async def _parse_datetime_in_user_tz( "attendees": {"type": "array", "items": {"type": "string"}, "description": "Optional list of attendee email addresses"}, "calendar_name": {"type": "string", "description": "Optional calendar name to create the event in. Falls back to default calendar."}, "project": {"type": "string", "description": "Optional project name to associate this event with"}, + # Legacy combined fields kept for backward compatibility with saved + # tool-call payloads in conversation history. New calls should use + # start_date + start_time. Hidden from typical model output via the + # description above; still accepted by the resolver as a fallback. + "start": {"type": "string", "description": "[Deprecated] Combined start datetime — prefer start_date + start_time."}, + "end": {"type": "string", "description": "[Deprecated] Combined end datetime — prefer end_date + end_time."}, }, - required=["title", "start"], + required=["title"], ) async def create_event_tool(*, user_id, arguments, **_ctx): - start_str = arguments["start"] - end_str = arguments.get("end") all_day = arguments.get("all_day", False) try: - # Naive dates/datetimes are interpreted in the user's local - # timezone, not UTC. Storing UTC for naive inputs caused all-day - # events to land on the previous day for negative-offset users. - start_dt, start_was_date_only = await _parse_datetime_in_user_tz( - user_id, start_str - ) - except (ValueError, TypeError): - return {"success": False, "error": f"Invalid start datetime: {start_str!r}"} + start_dt, start_was_date_only = await _resolve_event_start(user_id, arguments) + except ValueError as exc: + return {"success": False, "error": str(exc)} + except TypeError as exc: + return {"success": False, "error": f"Invalid start: {exc}"} if start_was_date_only: all_day = True - end_dt = None - if end_str: - try: - end_dt, _ = await _parse_datetime_in_user_tz(user_id, end_str) - except (ValueError, TypeError): - return {"success": False, "error": f"Invalid end datetime: {end_str!r}"} + try: + end_dt = await _resolve_event_end(user_id, arguments) + except ValueError as exc: + return {"success": False, "error": str(exc)} + except TypeError as exc: + return {"success": False, "error": f"Invalid end: {exc}"} project_id = None project_name = arguments.get("project") if project_name: @@ -168,18 +249,29 @@ async def search_events_tool(*, user_id, arguments, **_ctx): @tool( name="update_event", - description="Update an existing calendar event. Use this when the user asks to change, move, reschedule, or modify an event.", + description=( + "Update an existing calendar event. Use this when the user asks to " + "change, move, reschedule, or modify an event. Pass `start_date` " + "(YYYY-MM-DD) and `start_time` (HH:MM) as separate fields in the " + "user's local time when rescheduling — never combine them, never " + "include a timezone suffix." + ), parameters={ "query": {"type": "string", "description": "Search term to find the event to update (matches against title)"}, "title": {"type": "string", "description": "New title for the event"}, - "start": {"type": "string", "description": "New start datetime in ISO 8601 format"}, - "end": {"type": "string", "description": "New end datetime in ISO 8601 format"}, + "start_date": {"type": "string", "description": "New start calendar date as YYYY-MM-DD in the user's local time. No timezone suffix."}, + "start_time": {"type": "string", "description": "New start wall-clock time as HH:MM. No timezone suffix."}, + "end_date": {"type": "string", "description": "New end calendar date as YYYY-MM-DD."}, + "end_time": {"type": "string", "description": "New end wall-clock time as HH:MM."}, "all_day": {"type": "boolean", "description": "Whether the event is all-day"}, "description": {"type": "string", "description": "New event description"}, "location": {"type": "string", "description": "New event location"}, "color": {"type": "string", "description": "New hex color for the event (e.g. '#6366f1')"}, "recurrence": {"type": "string", "description": "New iCalendar RRULE"}, "reminder_minutes": {"type": "integer", "description": "Reminder N minutes before the event. Pass 0 to remove an existing reminder."}, + # Legacy combined fields kept for backcompat — see create_event. + "start": {"type": "string", "description": "[Deprecated] Combined start datetime — prefer start_date + start_time."}, + "end": {"type": "string", "description": "[Deprecated] Combined end datetime — prefer end_date + end_time."}, }, required=["query"], ) @@ -198,16 +290,20 @@ async def update_event_tool(*, user_id, arguments, **_ctx): if "reminder_minutes" in arguments: rm = arguments["reminder_minutes"] fields["reminder_minutes"] = None if rm == 0 else rm - for dt_field, key in (("start_dt", "start"), ("end_dt", "end")): - val = arguments.get(key) - if val: - try: - # Naive datetimes are user-local, not UTC — see - # ``_parse_datetime_in_user_tz`` docstring. - dt, _ = await _parse_datetime_in_user_tz(user_id, val) - fields[dt_field] = dt - except (ValueError, TypeError): - return {"success": False, "error": f"Invalid datetime for {key}: {val!r}"} + # Resolve start: split fields preferred, legacy `start` as fallback. + if arguments.get("start_date") or arguments.get("start"): + try: + start_dt, _ = await _resolve_event_start(user_id, arguments) + except (ValueError, TypeError) as exc: + return {"success": False, "error": f"Invalid start: {exc}"} + fields["start_dt"] = start_dt + if arguments.get("end_date") or arguments.get("end"): + try: + end_dt = await _resolve_event_end(user_id, arguments) + except (ValueError, TypeError) as exc: + return {"success": False, "error": f"Invalid end: {exc}"} + if end_dt is not None: + fields["end_dt"] = end_dt updated = await events_update_event(user_id=user_id, event_id=event_to_update.id, **fields) if updated is None: return {"success": False, "error": "Event not found or update failed."} diff --git a/tests/test_calendar_tool_tz.py b/tests/test_calendar_tool_tz.py index bab715f..6d711fa 100644 --- a/tests/test_calendar_tool_tz.py +++ b/tests/test_calendar_tool_tz.py @@ -132,3 +132,266 @@ async def test_list_events_bare_date_range_covers_local_day(): assert (df.year, df.month, df.day, df.hour) == (2026, 9, 30, 4) # 2026-09-30 23:59:59 NY (EDT) = 03:59:59 UTC next day assert (dt.year, dt.month, dt.day, dt.hour) == (2026, 10, 1, 3) + + +# ── Split date/time field tests (durable shape) ────────────────────────────── +# +# These exercise the start_date + start_time path that the model is now +# steered toward. The split-field shape is structurally immune to the +# class of bugs where a model emits a TZ-tagged combined datetime that +# the parser correctly honors but lands on the wrong calendar day for +# negative-offset users. + + +@pytest.mark.asyncio +async def test_create_event_split_fields_friday_8am_no_drift_eastern(): + """The reported bug: 'next Friday at 8am' for a NY user must land on + 2026-05-01 08:00 NY, never 04-30 19:00. With split fields the model + can't TZ-tag the date string, so the calendar day is fixed.""" + from fabledassistant.services.tools.calendar import create_event_tool + + captured = {} + + async def fake_create_event(**kwargs): + captured.update(kwargs) + event = AsyncMock() + event.to_dict.return_value = {"id": 1} + return event + + with patch( + "fabledassistant.services.tools.calendar.get_user_tz", + AsyncMock(return_value=__import__("zoneinfo").ZoneInfo("America/New_York")), + ), patch( + "fabledassistant.services.tools.calendar.events_create_event", + side_effect=fake_create_event, + ): + result = await create_event_tool( + user_id=1, + arguments={ + "title": "Meeting", + "start_date": "2026-05-01", + "start_time": "08:00", + }, + ) + + assert result["success"] is True + utc = captured["start_dt"].astimezone(timezone.utc) + # 08:00 NY (EDT, UTC-4) on 2026-05-01 = 12:00 UTC same day + assert (utc.year, utc.month, utc.day, utc.hour, utc.minute) == (2026, 5, 1, 12, 0) + assert captured["all_day"] is False + + +@pytest.mark.asyncio +async def test_create_event_split_fields_no_drift_pacific(): + """Same scenario for a UTC-8 user — the calendar day must be 5/1 + regardless of how big the offset gets.""" + from fabledassistant.services.tools.calendar import create_event_tool + + captured = {} + + async def fake_create_event(**kwargs): + captured.update(kwargs) + event = AsyncMock() + event.to_dict.return_value = {"id": 1} + return event + + with patch( + "fabledassistant.services.tools.calendar.get_user_tz", + AsyncMock(return_value=__import__("zoneinfo").ZoneInfo("America/Los_Angeles")), + ), patch( + "fabledassistant.services.tools.calendar.events_create_event", + side_effect=fake_create_event, + ): + await create_event_tool( + user_id=1, + arguments={ + "title": "Standup", + "start_date": "2026-05-01", + "start_time": "08:00", + }, + ) + + utc = captured["start_dt"].astimezone(timezone.utc) + # 08:00 LA (PDT, UTC-7) = 15:00 UTC same day + assert (utc.year, utc.month, utc.day, utc.hour) == (2026, 5, 1, 15) + + +@pytest.mark.asyncio +async def test_create_event_split_fields_no_drift_positive_offset(): + """A positive-offset user (Tokyo, UTC+9) — 08:00 Tokyo on 2026-05-01 + is 23:00 UTC on 2026-04-30, but the local calendar day must still be + stored as 2026-05-01 from the user's perspective.""" + from fabledassistant.services.tools.calendar import create_event_tool + + captured = {} + + async def fake_create_event(**kwargs): + captured.update(kwargs) + event = AsyncMock() + event.to_dict.return_value = {"id": 1} + return event + + with patch( + "fabledassistant.services.tools.calendar.get_user_tz", + AsyncMock(return_value=__import__("zoneinfo").ZoneInfo("Asia/Tokyo")), + ), patch( + "fabledassistant.services.tools.calendar.events_create_event", + side_effect=fake_create_event, + ): + await create_event_tool( + user_id=1, + arguments={ + "title": "Sync", + "start_date": "2026-05-01", + "start_time": "08:00", + }, + ) + + utc = captured["start_dt"].astimezone(timezone.utc) + # 08:00 Tokyo (UTC+9) = 23:00 UTC previous day; the round-trip back + # to Tokyo TZ recovers 2026-05-01 08:00. + assert (utc.year, utc.month, utc.day, utc.hour) == (2026, 4, 30, 23) + tokyo = captured["start_dt"].astimezone(__import__("zoneinfo").ZoneInfo("Asia/Tokyo")) + assert (tokyo.year, tokyo.month, tokyo.day, tokyo.hour) == (2026, 5, 1, 8) + + +@pytest.mark.asyncio +async def test_create_event_split_date_only_is_all_day(): + """Omitting start_time means the event is all-day; cache row uses + local midnight.""" + from fabledassistant.services.tools.calendar import create_event_tool + + captured = {} + + async def fake_create_event(**kwargs): + captured.update(kwargs) + event = AsyncMock() + event.to_dict.return_value = {"id": 1} + return event + + with patch( + "fabledassistant.services.tools.calendar.get_user_tz", + AsyncMock(return_value=__import__("zoneinfo").ZoneInfo("America/New_York")), + ), patch( + "fabledassistant.services.tools.calendar.events_create_event", + side_effect=fake_create_event, + ): + await create_event_tool( + user_id=1, + arguments={"title": "Birthday", "start_date": "2026-09-30"}, + ) + + assert captured["all_day"] is True + utc = captured["start_dt"].astimezone(timezone.utc) + assert (utc.year, utc.month, utc.day, utc.hour) == (2026, 9, 30, 4) + + +@pytest.mark.asyncio +async def test_create_event_split_rejects_tz_suffix_in_date(): + """The whole point of split fields: a TZ suffix on the date string + must be rejected, not silently honored.""" + from fabledassistant.services.tools.calendar import create_event_tool + + with patch( + "fabledassistant.services.tools.calendar.get_user_tz", + AsyncMock(return_value=__import__("zoneinfo").ZoneInfo("America/New_York")), + ): + result = await create_event_tool( + user_id=1, + arguments={"title": "x", "start_date": "2026-05-01Z", "start_time": "08:00"}, + ) + + assert result["success"] is False + assert "YYYY-MM-DD" in result["error"] + + +@pytest.mark.asyncio +async def test_create_event_split_rejects_tz_suffix_in_time(): + """A TZ suffix on the time string must also be rejected.""" + from fabledassistant.services.tools.calendar import create_event_tool + + with patch( + "fabledassistant.services.tools.calendar.get_user_tz", + AsyncMock(return_value=__import__("zoneinfo").ZoneInfo("America/New_York")), + ): + result = await create_event_tool( + user_id=1, + arguments={"title": "x", "start_date": "2026-05-01", "start_time": "08:00 UTC"}, + ) + + assert result["success"] is False + assert "HH:MM" in result["error"] + + +@pytest.mark.asyncio +async def test_create_event_legacy_combined_start_still_works(): + """Backcompat: saved tool-call payloads using the old `start` field + must still produce the same UTC datetime as before.""" + from fabledassistant.services.tools.calendar import create_event_tool + + captured = {} + + async def fake_create_event(**kwargs): + captured.update(kwargs) + event = AsyncMock() + event.to_dict.return_value = {"id": 1} + return event + + with patch( + "fabledassistant.services.tools.calendar.get_user_tz", + AsyncMock(return_value=__import__("zoneinfo").ZoneInfo("America/New_York")), + ), patch( + "fabledassistant.services.tools.calendar.events_create_event", + side_effect=fake_create_event, + ): + result = await create_event_tool( + user_id=1, + arguments={"title": "Legacy", "start": "2026-05-01T08:00"}, + ) + + assert result["success"] is True + utc = captured["start_dt"].astimezone(timezone.utc) + assert (utc.year, utc.month, utc.day, utc.hour) == (2026, 5, 1, 12) + + +@pytest.mark.asyncio +async def test_update_event_split_fields_reschedule_no_drift(): + """update_event with split fields must drift no calendar day either.""" + from fabledassistant.services.tools.calendar import update_event_tool + + captured = {} + + async def fake_find(*, user_id, query): + ev = AsyncMock() + ev.id = 99 + ev.title = "Coffee" + return [ev] + + async def fake_update(*, user_id, event_id, **fields): + captured.update(fields) + ev = AsyncMock() + ev.to_dict.return_value = {"id": event_id, **fields} + return ev + + with patch( + "fabledassistant.services.tools.calendar.get_user_tz", + AsyncMock(return_value=__import__("zoneinfo").ZoneInfo("America/New_York")), + ), patch( + "fabledassistant.services.tools.calendar.find_events_by_query", + side_effect=fake_find, + ), patch( + "fabledassistant.services.tools.calendar.events_update_event", + side_effect=fake_update, + ): + result = await update_event_tool( + user_id=1, + arguments={ + "query": "Coffee", + "start_date": "2026-05-01", + "start_time": "08:00", + }, + ) + + assert result["success"] is True + utc = captured["start_dt"].astimezone(timezone.utc) + assert (utc.year, utc.month, utc.day, utc.hour) == (2026, 5, 1, 12) From 03d725ea3e7ce98eea6eefbe3d84247deac4f59f Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 29 Apr 2026 08:43:32 -0400 Subject: [PATCH 4/7] fix(calendar-tool): anchor today's weekday in prompts + verify expected_weekday on create/update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A user asked Fable to schedule "this Friday at 8am" on Wednesday 4/29 2026. The model picked 4/30 (Thursday) and confidently labeled it "Friday." The TZ pipeline did everything correctly given the model's date — the bug was upstream: the model was guessing weekdays from ISO dates without an anchor, and the calendar tools had no way to verify. Three layered fixes: 1. **System prompts now name the weekday alongside the ISO date.** Both the journal-conversation prompt and the general chat prompt used to say "Today is 2026-04-29 (America/New_York)." They now say "Today is Wednesday, 2026-04-29 (...)." LLMs are unreliable at deriving weekday names from ISO dates; supplying the name removes the guess. 2. **`expected_weekday` parameter on create_event / update_event.** When the model passes `expected_weekday="friday"`, the backend computes the resolved start_date's weekday in the user's local timezone and rejects mismatches with a self-correcting error ("Date 2026-04-30 falls on Thursday, not Friday. Recompute..."), without creating the event. The check is local-aware: a Friday 23:00 event in Tokyo crosses midnight UTC but the local view stays Friday, and the validator respects that. 3. **Tool descriptions instruct echo-and-confirm.** create_event and update_event descriptions now tell the model: when the user names a weekday, state the resolved date in the reply BEFORE calling the tool, and pass `expected_weekday`. Costs nothing in code, reinforces the validator. 6 new tests — match success, mismatch rejection (with create/update not invoked), omitted-param backcompat, invalid weekday name, local- not-UTC weekday computation, and the update_event variant. All 18 calendar-tool tests + 33 event-related tests pass; ruff clean. Co-Authored-By: Claude Opus 4.7 --- .../services/journal_pipeline.py | 6 +- src/fabledassistant/services/llm.py | 9 +- .../services/tools/calendar.py | 50 +++- tests/test_calendar_tool_tz.py | 216 ++++++++++++++++++ 4 files changed, 276 insertions(+), 5 deletions(-) diff --git a/src/fabledassistant/services/journal_pipeline.py b/src/fabledassistant/services/journal_pipeline.py index 0fa658c..32de232 100644 --- a/src/fabledassistant/services/journal_pipeline.py +++ b/src/fabledassistant/services/journal_pipeline.py @@ -134,7 +134,11 @@ async def build_journal_system_prompt( static_block = f"{JOURNAL_PERSONA}\n\n{JOURNAL_CALIBRATION}" today_iso = day_date.isoformat() - tz_block = f"Today is {today_iso} ({user_timezone})." + # Include the day-of-week explicitly. LLMs are unreliable at deriving + # weekday names from ISO dates, which causes "this Friday" / "next + # Monday" to land on the wrong calendar day. + weekday = day_date.strftime("%A") + tz_block = f"Today is {weekday}, {today_iso} ({user_timezone})." profile_context = await build_profile_context(user_id) profile_section = f"\n\n{profile_context}" if profile_context else "" diff --git a/src/fabledassistant/services/llm.py b/src/fabledassistant/services/llm.py index 21238c6..316936a 100644 --- a/src/fabledassistant/services/llm.py +++ b/src/fabledassistant/services/llm.py @@ -602,7 +602,12 @@ async def build_context( } assistant_name = await get_setting(user_id, "assistant_name", "Fable") - today = date_type.today().isoformat() + _today_obj = date_type.today() + today = _today_obj.isoformat() + # Day-of-week paired with the ISO date so the model doesn't have to + # derive the weekday — that derivation is a documented failure mode + # for "this Friday" / "next Monday"-style requests. + today_weekday = _today_obj.strftime("%A") has_caldav = await is_caldav_configured(user_id) # Build tool usage guidance based on available integrations @@ -678,7 +683,7 @@ async def build_context( entities_context = await get_people_and_places_context(user_id) entities_section = f"\n\n{entities_context}" if entities_context else "" - dynamic_tail = f"\n\nToday's date is {today}.{tz_line}{profile_section}{entities_section}" + dynamic_tail = f"\n\nToday is {today_weekday}, {today}.{tz_line}{profile_section}{entities_section}" # --- System message: stable content only --- # Workspace context and history summary stay here because they carry diff --git a/src/fabledassistant/services/tools/calendar.py b/src/fabledassistant/services/tools/calendar.py index 12fc43c..0218ddc 100644 --- a/src/fabledassistant/services/tools/calendar.py +++ b/src/fabledassistant/services/tools/calendar.py @@ -108,6 +108,34 @@ async def _resolve_event_end( return None +def _validate_weekday(start_dt_utc: datetime, user_tz, expected: str | None) -> str | None: + """Verify the resolved local date falls on the expected day of the week. + + Models routinely miscompute "this Friday" / "next Monday" when the + system prompt only carries an ISO date without a weekday. When the + model passes `expected_weekday`, the backend rejects mismatches with + a self-correcting error message naming the actual weekday. + + Returns an error string on mismatch, or ``None`` when the check + passes (or no expected weekday was supplied). + """ + if not expected: + return None + expected_norm = expected.strip().lower() + valid = {"monday", "tuesday", "wednesday", "thursday", "friday", "saturday", "sunday"} + if expected_norm not in valid: + return f"expected_weekday must be a full English weekday name; got {expected!r}." + local = start_dt_utc.astimezone(user_tz) + actual = local.strftime("%A").lower() + if actual == expected_norm: + return None + return ( + f"Date {local.date().isoformat()} falls on {actual.title()}, " + f"not {expected_norm.title()}. Recompute the date for " + f"{expected_norm.title()} or confirm with the user before retrying." + ) + + @tool( name="create_event", description=( @@ -117,7 +145,11 @@ async def _resolve_event_end( "separate fields in the user's local time — never combine them and " "never include a timezone suffix. The server attaches the user's " "configured timezone. Omit `start_time` (or set `all_day=true`) " - "for all-day events like birthdays or holidays." + "for all-day events like birthdays or holidays. " + "When the user names a weekday ('this Friday', 'next Monday'), " + "state the resolved calendar date in your reply BEFORE calling " + "this tool, and pass `expected_weekday` so the server can verify " + "the date falls on the day you intended." ), parameters={ "title": {"type": "string", "description": "A descriptive event title"}, @@ -135,6 +167,7 @@ async def _resolve_event_end( "attendees": {"type": "array", "items": {"type": "string"}, "description": "Optional list of attendee email addresses"}, "calendar_name": {"type": "string", "description": "Optional calendar name to create the event in. Falls back to default calendar."}, "project": {"type": "string", "description": "Optional project name to associate this event with"}, + "expected_weekday": {"type": "string", "description": "Optional weekday name (e.g. 'friday') the start_date should fall on. Pass this whenever the user names a weekday so the server can verify the date is correct. Rejects with a corrective error if the date falls on a different day."}, # Legacy combined fields kept for backward compatibility with saved # tool-call payloads in conversation history. New calls should use # start_date + start_time. Hidden from typical model output via the @@ -154,6 +187,10 @@ async def create_event_tool(*, user_id, arguments, **_ctx): return {"success": False, "error": f"Invalid start: {exc}"} if start_was_date_only: all_day = True + user_tz = await get_user_tz(user_id) + weekday_err = _validate_weekday(start_dt, user_tz, arguments.get("expected_weekday")) + if weekday_err: + return {"success": False, "error": weekday_err} try: end_dt = await _resolve_event_end(user_id, arguments) except ValueError as exc: @@ -254,7 +291,11 @@ async def search_events_tool(*, user_id, arguments, **_ctx): "change, move, reschedule, or modify an event. Pass `start_date` " "(YYYY-MM-DD) and `start_time` (HH:MM) as separate fields in the " "user's local time when rescheduling — never combine them, never " - "include a timezone suffix." + "include a timezone suffix. " + "When the user names a weekday ('move to Friday'), state the " + "resolved calendar date in your reply BEFORE calling this tool, " + "and pass `expected_weekday` so the server can verify the date " + "falls on the day you intended." ), parameters={ "query": {"type": "string", "description": "Search term to find the event to update (matches against title)"}, @@ -269,6 +310,7 @@ async def search_events_tool(*, user_id, arguments, **_ctx): "color": {"type": "string", "description": "New hex color for the event (e.g. '#6366f1')"}, "recurrence": {"type": "string", "description": "New iCalendar RRULE"}, "reminder_minutes": {"type": "integer", "description": "Reminder N minutes before the event. Pass 0 to remove an existing reminder."}, + "expected_weekday": {"type": "string", "description": "Optional weekday name (e.g. 'friday') the new start_date should fall on. Pass whenever the user names a weekday."}, # Legacy combined fields kept for backcompat — see create_event. "start": {"type": "string", "description": "[Deprecated] Combined start datetime — prefer start_date + start_time."}, "end": {"type": "string", "description": "[Deprecated] Combined end datetime — prefer end_date + end_time."}, @@ -296,6 +338,10 @@ async def update_event_tool(*, user_id, arguments, **_ctx): start_dt, _ = await _resolve_event_start(user_id, arguments) except (ValueError, TypeError) as exc: return {"success": False, "error": f"Invalid start: {exc}"} + user_tz = await get_user_tz(user_id) + weekday_err = _validate_weekday(start_dt, user_tz, arguments.get("expected_weekday")) + if weekday_err: + return {"success": False, "error": weekday_err} fields["start_dt"] = start_dt if arguments.get("end_date") or arguments.get("end"): try: diff --git a/tests/test_calendar_tool_tz.py b/tests/test_calendar_tool_tz.py index 6d711fa..71b0e24 100644 --- a/tests/test_calendar_tool_tz.py +++ b/tests/test_calendar_tool_tz.py @@ -395,3 +395,219 @@ async def test_update_event_split_fields_reschedule_no_drift(): assert result["success"] is True utc = captured["start_dt"].astimezone(timezone.utc) assert (utc.year, utc.month, utc.day, utc.hour) == (2026, 5, 1, 12) + + +# ── expected_weekday verification (catches "this Friday" → Thursday bugs) ──── + + +@pytest.mark.asyncio +async def test_create_event_expected_weekday_match_succeeds(): + """When expected_weekday agrees with the resolved local date's + weekday, the event is created normally.""" + from fabledassistant.services.tools.calendar import create_event_tool + + captured = {} + + async def fake_create_event(**kwargs): + captured.update(kwargs) + event = AsyncMock() + event.to_dict.return_value = {"id": 1} + return event + + with patch( + "fabledassistant.services.tools.calendar.get_user_tz", + AsyncMock(return_value=__import__("zoneinfo").ZoneInfo("America/New_York")), + ), patch( + "fabledassistant.services.tools.calendar.events_create_event", + side_effect=fake_create_event, + ): + result = await create_event_tool( + user_id=1, + arguments={ + "title": "Meeting", + "start_date": "2026-05-01", # is a Friday + "start_time": "08:00", + "expected_weekday": "friday", + }, + ) + + assert result["success"] is True + assert captured["start_dt"] is not None + + +@pytest.mark.asyncio +async def test_create_event_expected_weekday_mismatch_rejects_and_names_actual(): + """The reported failure mode: model picks Thursday and calls it + Friday. With expected_weekday set, the create is rejected and the + error names the actual weekday so the model can self-correct.""" + from fabledassistant.services.tools.calendar import create_event_tool + + create_called = False + + async def fake_create_event(**kwargs): + nonlocal create_called + create_called = True + event = AsyncMock() + event.to_dict.return_value = {"id": 1} + return event + + with patch( + "fabledassistant.services.tools.calendar.get_user_tz", + AsyncMock(return_value=__import__("zoneinfo").ZoneInfo("America/New_York")), + ), patch( + "fabledassistant.services.tools.calendar.events_create_event", + side_effect=fake_create_event, + ): + result = await create_event_tool( + user_id=1, + arguments={ + "title": "Dentist", + "start_date": "2026-04-30", # Thursday + "start_time": "08:00", + "expected_weekday": "friday", + }, + ) + + assert result["success"] is False + assert "Thursday" in result["error"] + assert "Friday" in result["error"] + # Critical: the event must NOT have been created when the check failed. + assert create_called is False + + +@pytest.mark.asyncio +async def test_create_event_expected_weekday_omitted_skips_check(): + """Backcompat: when expected_weekday isn't passed, no validation + runs — the existing create flow is unchanged.""" + from fabledassistant.services.tools.calendar import create_event_tool + + captured = {} + + async def fake_create_event(**kwargs): + captured.update(kwargs) + event = AsyncMock() + event.to_dict.return_value = {"id": 1} + return event + + with patch( + "fabledassistant.services.tools.calendar.get_user_tz", + AsyncMock(return_value=__import__("zoneinfo").ZoneInfo("America/New_York")), + ), patch( + "fabledassistant.services.tools.calendar.events_create_event", + side_effect=fake_create_event, + ): + result = await create_event_tool( + user_id=1, + arguments={ + "title": "x", + "start_date": "2026-04-30", + "start_time": "08:00", + }, + ) + + assert result["success"] is True + + +@pytest.mark.asyncio +async def test_create_event_expected_weekday_invalid_value_rejects(): + """Garbage in expected_weekday produces a clear validation error, + not a silent pass.""" + from fabledassistant.services.tools.calendar import create_event_tool + + with patch( + "fabledassistant.services.tools.calendar.get_user_tz", + AsyncMock(return_value=__import__("zoneinfo").ZoneInfo("America/New_York")), + ): + result = await create_event_tool( + user_id=1, + arguments={ + "title": "x", + "start_date": "2026-05-01", + "start_time": "08:00", + "expected_weekday": "fri", # abbreviation not accepted + }, + ) + + assert result["success"] is False + assert "weekday" in result["error"].lower() + + +@pytest.mark.asyncio +async def test_update_event_expected_weekday_mismatch_rejects(): + """update_event must enforce the same weekday check on reschedules.""" + from fabledassistant.services.tools.calendar import update_event_tool + + update_called = False + + async def fake_find(*, user_id, query): + ev = AsyncMock() + ev.id = 99 + ev.title = "Coffee" + return [ev] + + async def fake_update(*, user_id, event_id, **fields): + nonlocal update_called + update_called = True + ev = AsyncMock() + ev.to_dict.return_value = {"id": event_id} + return ev + + with patch( + "fabledassistant.services.tools.calendar.get_user_tz", + AsyncMock(return_value=__import__("zoneinfo").ZoneInfo("America/New_York")), + ), patch( + "fabledassistant.services.tools.calendar.find_events_by_query", + side_effect=fake_find, + ), patch( + "fabledassistant.services.tools.calendar.events_update_event", + side_effect=fake_update, + ): + result = await update_event_tool( + user_id=1, + arguments={ + "query": "Coffee", + "start_date": "2026-04-30", # Thursday + "start_time": "08:00", + "expected_weekday": "friday", + }, + ) + + assert result["success"] is False + assert "Thursday" in result["error"] + assert update_called is False + + +@pytest.mark.asyncio +async def test_create_event_weekday_check_uses_local_not_utc(): + """The weekday check must use the LOCAL date, not the UTC date. + A late-evening Friday event in Tokyo (UTC+9) crosses midnight UTC, + so a UTC-day check would call it Saturday — but the user's calendar + says Friday. The check must respect the user's local view.""" + from fabledassistant.services.tools.calendar import create_event_tool + + captured = {} + + async def fake_create_event(**kwargs): + captured.update(kwargs) + event = AsyncMock() + event.to_dict.return_value = {"id": 1} + return event + + with patch( + "fabledassistant.services.tools.calendar.get_user_tz", + AsyncMock(return_value=__import__("zoneinfo").ZoneInfo("Asia/Tokyo")), + ), patch( + "fabledassistant.services.tools.calendar.events_create_event", + side_effect=fake_create_event, + ): + result = await create_event_tool( + user_id=1, + arguments={ + "title": "Friday night", + "start_date": "2026-05-01", # Friday in Tokyo + "start_time": "23:00", # 14:00 UTC same day; safe + "expected_weekday": "friday", + }, + ) + + assert result["success"] is True From 6c309f133107df4b4565314d891254ab850927ea Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 29 Apr 2026 09:21:35 -0400 Subject: [PATCH 5/7] =?UTF-8?q?fix(journal):=20tune=20persona=20=E2=80=94?= =?UTF-8?q?=20capture-first,=20anti-overhelp,=20first-person=20moments=20(?= =?UTF-8?q?#157)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three related rough edges in the journal voice surfaced from real journal usage 2026-04-27 → 2026-04-29: 1. **Persona overhelps.** When the user logged "today I'm prepping for an ISP migration at Branch 14 Bedford for work at famous supply", the assistant came back with "ISP migrations can be tricky. Are you handling the network configuration yourself, or is there a team supporting you? Also, are there any specific tasks or checks you need to complete before the switch?" — pushing IT-helpdesk advice the user didn't ask for. The user had to push back. JOURNAL_PERSONA now leads with "CAPTURE first, advise only if asked" and the RESPONSE STYLE block has an explicit anti-pattern banning troubleshooting / checklist / process-advice follow-ups unless the user explicitly invites them. 2. **Moments stored in third-person observer voice.** The dentist appointment beat got written as "The user mentioned having an appointment this Friday but hasn't provided details yet." — reads like an LLM transcript annotation, not a journal jot. The record_moment tool's `content` description previously said "in the user's voice or third-person", which was the literal source of the bug. New phrasing requires first-person/imperative with concrete GOOD/BAD examples, and the JOURNAL_CALIBRATION block reinforces it. 3. **Inconsistent emoji use.** 4/27 was clinical, 4/29 had 😊 and 🛠️ in the appointment confirmation. RESPONSE STYLE now bans emojis outright — the journal is a thinking-companion surface and the emoji warmth reads as out-of-register chat-bot tone. Bonus while in here: - New MOMENT ENTITY LINKING section explicitly forbids attaching a task_titles link unless the user references the task by name (the 4/27 Docker→ADHD auto-link bug; rest of that fix is in #158). - Same section rejects generic place placeholders ("work" / "home" / "office") in favor of letting the user name the real place. 22 tests pass (4 journal + 18 calendar tool); ruff clean. Closes Fable task #157. Co-Authored-By: Claude Opus 4.7 --- .../services/journal_pipeline.py | 39 +++++++++++++++++-- src/fabledassistant/services/tools/journal.py | 11 +++++- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/src/fabledassistant/services/journal_pipeline.py b/src/fabledassistant/services/journal_pipeline.py index 32de232..35e5687 100644 --- a/src/fabledassistant/services/journal_pipeline.py +++ b/src/fabledassistant/services/journal_pipeline.py @@ -18,10 +18,12 @@ from fabledassistant.services.user_profile import build_profile_context JOURNAL_PERSONA = ( "You are the user's assistant. They've opened their journal — a day-anchored " - "conversation surface where they record their day. Behave like the rest of " - "the app's chat: respond conversationally, ask follow-up questions about what " - "they just said, verify details from earlier in the conversation when " - "relevant, and use tools naturally to act on their behalf when it makes sense. " + "conversation surface where they record their day. Your primary job is to " + "CAPTURE what they share, not to advise on it. Treat journal mode as a quiet " + "thinking-companion surface: record the beat, optionally ask one short " + "follow-up that doesn't presume they want help, and let the user lead. " + "Use tools to act on their behalf when they ask, not when you think they " + "might find it useful. " "The day's prep message at the top of the conversation is your context — " "build on it, don't restate it." ) @@ -57,6 +59,26 @@ these beats; if you skip the call, the beat is lost. Multiple distinct beats in one message → multiple record_moment calls, one per beat. +MOMENT PHRASING — write it the way the user would jot it themselves. +First-person or imperative, never third-person observer voice. + - GOOD: "Restaging Docker on the Bedford swarm; one Windows node had + network breakage." + - GOOD: "Appointment this Friday — details TBD." + - GOOD: "Coffee with Sarah; she's hiring." + - BAD: "The user mentioned having an appointment this Friday but + hasn't provided details yet." + - BAD: "User reports Docker swarm restage in progress." +Strip "the user…" / "user mentioned…" / "user is…" framings entirely. + +MOMENT ENTITY LINKING — be conservative. + - Only attach a `task_titles` link when the user *explicitly references + that task* in the message. Do NOT link to a task just because it's + in the prep context or the only task currently open. + - Only attach `place_names` you can ground in something the user + actually said. Generic placeholders like "work" / "home" / "office" + are NOT places — drop them and let the user name the real one if it + matters. + WHEN LINKING ENTITIES: use the *_names parameters (person_names, place_names, task_titles, note_titles). Server resolves them to IDs by lookup. Do NOT pass *_ids unless you have an exact ID returned from @@ -89,6 +111,15 @@ RESPONSE STYLE: - Don't repeat a prior reply verbatim. If the user circles back on a theme, pick a specific concrete detail from the new message to react to. - Match the user's length. Short message → short reply. Don't pad. +- DON'T offer troubleshooting steps, checklists, or generic process advice + for the user's work unless they explicitly ask. When the user is logging + what they're doing, they want to be heard, not coached. A statement like + "I'm prepping for an ISP migration" should be acknowledged and recorded — + not met with "Are you handling the network configuration yourself? Are + there checks you need to do first?" If a follow-up would presume they + want help, drop it. +- No emojis. The journal is a thinking-companion surface; emojis read as + chat-bot warmth that's out of register. """ PHASE_GREETINGS = { diff --git a/src/fabledassistant/services/tools/journal.py b/src/fabledassistant/services/tools/journal.py index 38cf416..eb4fde4 100644 --- a/src/fabledassistant/services/tools/journal.py +++ b/src/fabledassistant/services/tools/journal.py @@ -78,7 +78,16 @@ async def _resolve_entity_ids_by_name( parameters={ "content": { "type": "string", - "description": "1-2 sentence distillation of the moment in the user's voice or third-person.", + "description": ( + "1-2 sentence distillation of the moment in the user's " + "voice — first-person or imperative, like a journal jot. " + "GOOD: 'Restaging Docker on the Bedford swarm; one " + "Windows node had network breakage.' / 'Appointment " + "Friday — details TBD.' " + "BAD: 'The user mentioned having an appointment.' / " + "'User reports Docker swarm restage.' Strip 'the user…' " + "/ 'user mentioned…' framings entirely." + ), }, "occurred_at": { "type": "string", From 4f18023284f53bd50aa2adb0e53651cf3e2397ab Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 29 Apr 2026 09:25:16 -0400 Subject: [PATCH 6/7] fix(journal): server-side guards on record_moment links + place names (#158) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Belt-and-suspenders to the prompt-layer changes in 6c309f1. Even when the model emits bogus task or place links, the server now refuses to persist them. ## Task auto-linking guard Reproducer (2026-04-27): a moment about restaging Docker on the swarm ended up with `task_ids: [2]` (Weston's ADHD Evaluation) — the only task in that day's prep. The model picked it up as filler. `_filter_task_ids_by_keyword_overlap` now runs after id resolution: it fetches each linked task's title, tokenizes both content and title through `_content_keywords` (lowercased, stopwords stripped, <3-char tokens dropped), and drops any link whose title shares no meaningful keyword with the moment content. The drop is logged at INFO so we can observe how often it fires post-deploy. The guard runs against the merged id list, so it covers both the preferred `task_titles` resolution path and the discouraged explicit `task_ids` path. ## Place placeholder guard Reproducer (2026-04-27): `place_names=["work"]` got passed to `record_moment`. "work" / "home" / "office" aren't places — they're role-labels for already-known geocoded locations. `_filter_placeholder_places` drops a small set of generic single-word labels before name resolution. Real user-named places that happen to be one word (e.g. "Akron") pass through. ## Tests 9 new unit tests in `tests/test_record_moment_guards.py` cover: - keyword tokenization & stopword stripping - placeholder place filtering (generic, case-insensitive, real-place pass-through) - keyword-overlap filtering (the exact 4/27 reproducer, the genuine- reference case, mixed/partial relevance, empty input) 13 tests pass; ruff clean. Closes Fable task #158. Co-Authored-By: Claude Opus 4.7 --- src/fabledassistant/services/tools/journal.py | 110 ++++++++++- tests/test_record_moment_guards.py | 181 ++++++++++++++++++ 2 files changed, 290 insertions(+), 1 deletion(-) create mode 100644 tests/test_record_moment_guards.py diff --git a/src/fabledassistant/services/tools/journal.py b/src/fabledassistant/services/tools/journal.py index eb4fde4..42c2133 100644 --- a/src/fabledassistant/services/tools/journal.py +++ b/src/fabledassistant/services/tools/journal.py @@ -3,6 +3,7 @@ from __future__ import annotations import datetime import logging +import re from typing import Any from sqlalchemy import func, select @@ -15,6 +16,97 @@ from fabledassistant.services.tools._registry import tool logger = logging.getLogger(__name__) +# Generic placeholders the model occasionally emits for `place_names` when no +# real place was named. Filtered out server-side as belt-and-suspenders to the +# prompt-layer guidance — these aren't places, just role-labels for locations +# the user already has named (Home / Work weather targets, etc.). +_PLACEHOLDER_PLACE_NAMES = frozenset({ + "work", "home", "office", "the office", "my office", "my work", + "my home", "house", "my house", "the house", +}) + +# Short closed-class words excluded from the keyword-overlap check below. +# Case is normalized to lowercase before comparison. +_STOPWORDS = frozenset({ + "the", "a", "an", "and", "or", "but", "at", "in", "on", "of", "to", + "for", "with", "by", "from", "about", "as", "is", "was", "were", "be", + "been", "being", "have", "has", "had", "do", "does", "did", "will", + "would", "could", "should", "may", "might", "must", "this", "that", + "these", "those", "i", "you", "he", "she", "we", "they", "it", "his", + "her", "their", "my", "your", "our", "its", "me", "him", "us", "them", + "are", "if", "so", "no", "not", "yes", "now", "then", "than", "too", + "very", "just", "also", "any", "all", "some", "one", "two", "out", + "up", "down", "off", "over", "under", "into", "onto", "upon", +}) + + +def _content_keywords(text: str) -> set[str]: + """Tokenize text into the meaningful keyword set used for overlap checks. + + Lowercased, alphanumeric runs only, stopwords removed, tokens shorter + than 3 chars dropped. Numeric tokens are kept (e.g. "Branch 14" yields + {"branch", "14"}) because they often anchor task references. + """ + tokens = re.split(r"[^a-z0-9]+", text.lower()) + return {t for t in tokens if len(t) >= 3 and t not in _STOPWORDS} + + +def _filter_placeholder_places(names: list[str]) -> tuple[list[str], list[str]]: + """Drop generic placeholders from a `place_names` list. + + Returns ``(kept, dropped)``. The dropped list is used purely for log + visibility — the moment is still created, the bogus links are just + not persisted. + """ + kept: list[str] = [] + dropped: list[str] = [] + for n in names: + norm = (n or "").strip() + if norm and norm.lower() in _PLACEHOLDER_PLACE_NAMES: + dropped.append(norm) + else: + kept.append(norm) + return kept, dropped + + +async def _filter_task_ids_by_keyword_overlap( + *, user_id: int, content: str, task_ids: list[int] +) -> list[int]: + """Drop task links whose title shares no meaningful keyword with the + moment content. + + Reproducer this guards against (2026-04-27): the model emitted + `task_titles=["Research Weston's ADHD Evaluation"]` on a moment about + restaging Docker — the title was real (Weston's task is in the prep + context) but the user never referenced it. Without this guard, the + only task surfaced in the prep gets attached to every moment as + filler. After this guard the link is dropped (zero-overlap) and a + log entry is emitted at INFO so we can observe how often this fires. + """ + if not task_ids: + return [] + async with async_session() as session: + stmt = select(Note.id, Note.title).where( + Note.user_id == user_id, + Note.id.in_(task_ids), + ) + rows = (await session.execute(stmt)).all() + title_by_id = {nid: (title or "") for nid, title in rows} + content_kw = _content_keywords(content) + kept: list[int] = [] + for tid in task_ids: + title = title_by_id.get(tid, "") + title_kw = _content_keywords(title) + if title_kw and content_kw & title_kw: + kept.append(tid) + else: + logger.info( + "record_moment: dropped task link id=%s title=%r — no keyword overlap with content %r", + tid, title, content[:80], + ) + return kept + + async def _resolve_entity_ids_by_name( *, user_id: int, @@ -180,10 +272,17 @@ async def record_moment_tool(*, user_id, arguments, conv_id=None, **_ctx): note_type="person", ) ) + raw_place_names = arguments.get("place_names") or [] + kept_place_names, dropped_place_names = _filter_placeholder_places(raw_place_names) + if dropped_place_names: + logger.info( + "record_moment: dropped placeholder place_names %r — not real places", + dropped_place_names, + ) place_ids.extend( await _resolve_entity_ids_by_name( user_id=user_id, - names=arguments.get("place_names") or [], + names=kept_place_names, note_type="place", ) ) @@ -208,6 +307,15 @@ async def record_moment_tool(*, user_id, arguments, conv_id=None, **_ctx): task_ids = list(dict.fromkeys(task_ids)) note_ids = list(dict.fromkeys(note_ids)) + # Drop task links that don't share a keyword with the moment content. + # Belt-and-suspenders to the prompt-layer rule "only link tasks the user + # explicitly references" — if the model attaches a task anyway (because + # it's in the prep context), the keyword check refuses to persist a link + # the moment can't justify. + task_ids = await _filter_task_ids_by_keyword_overlap( + user_id=user_id, content=content, task_ids=task_ids, + ) + moment = await create_moment( user_id=user_id, content=content, diff --git a/tests/test_record_moment_guards.py b/tests/test_record_moment_guards.py new file mode 100644 index 0000000..2772dcd --- /dev/null +++ b/tests/test_record_moment_guards.py @@ -0,0 +1,181 @@ +"""Tests for the record_moment server-side data-hygiene guards. + +These guard against two failure modes observed in real journal usage: + +1. The LLM emits ``task_titles`` referencing a task that's only in the + prep context — not actually mentioned by the user. Without a check, + every moment ends up linked to whatever's open in the user's queue. + +2. The LLM emits generic placeholder ``place_names`` like ``"work"`` / + ``"home"`` instead of real place notes. These role-labels aren't + places. + +Both are exercised through the pure helpers; full-stack handler tests +would require a session-bound DB fixture this suite doesn't have yet. +""" + +from unittest.mock import AsyncMock, patch + +import pytest + + +def test_content_keywords_drops_stopwords_and_short_tokens(): + from fabledassistant.services.tools.journal import _content_keywords + + kw = _content_keywords( + "I went to the store to pick up milk and bread for the kids." + ) + # Stopwords (the, to, for, and, i) gone; short tokens (kids? 4 chars - kept) + # filtered. Real content words kept. + assert "store" in kw + assert "milk" in kw + assert "bread" in kw + assert "kids" in kw + assert "the" not in kw + assert "to" not in kw + assert "for" not in kw + assert "i" not in kw + + +def test_content_keywords_extracts_named_entities(): + """Place names and project nouns survive tokenization. Short numeric + fragments (e.g. '14') get filtered alongside other <3-char tokens — + the surrounding alpha keywords carry the overlap weight in practice.""" + from fabledassistant.services.tools.journal import _content_keywords + + kw = _content_keywords("Migration prep at Branch 14 Bedford.") + assert "branch" in kw + assert "bedford" in kw + assert "migration" in kw + + +def test_filter_placeholder_places_drops_generic_role_labels(): + from fabledassistant.services.tools.journal import _filter_placeholder_places + + kept, dropped = _filter_placeholder_places( + ["work", "Famous Supply", "home", "Branch 14 Bedford", "the office"] + ) + assert "Famous Supply" in kept + assert "Branch 14 Bedford" in kept + assert "work" in dropped + assert "home" in dropped + assert "the office" in dropped + + +def test_filter_placeholder_places_is_case_insensitive(): + from fabledassistant.services.tools.journal import _filter_placeholder_places + + kept, dropped = _filter_placeholder_places(["WORK", "Home", "OFFICE"]) + assert kept == [] + assert set(dropped) == {"WORK", "Home", "OFFICE"} + + +def test_filter_placeholder_places_preserves_real_places_named_similarly(): + """A user-defined place that happens to be ONE word but isn't a generic + role-label (e.g. 'Akron') stays.""" + from fabledassistant.services.tools.journal import _filter_placeholder_places + + kept, dropped = _filter_placeholder_places(["Akron", "Cleveland"]) + assert kept == ["Akron", "Cleveland"] + assert dropped == [] + + +@pytest.mark.asyncio +async def test_keyword_overlap_drops_unrelated_task_link(): + """The reported failure mode: model attached Weston's ADHD task to a + Docker-swarm moment. With the keyword guard, the link gets dropped.""" + from fabledassistant.services.tools.journal import ( + _filter_task_ids_by_keyword_overlap, + ) + + # Mock the DB lookup to return the offending task title for id=2. + fake_session = AsyncMock() + fake_session.execute = AsyncMock() + fake_session.execute.return_value.all = lambda: [ + (2, "Research Weston's ADHD Evaluation"), + ] + fake_session.__aenter__ = AsyncMock(return_value=fake_session) + fake_session.__aexit__ = AsyncMock(return_value=None) + + with patch( + "fabledassistant.services.tools.journal.async_session", + return_value=fake_session, + ): + kept = await _filter_task_ids_by_keyword_overlap( + user_id=1, + content="Working on restaging Docker in the swarm; encountered network breakage on a Windows node.", + task_ids=[2], + ) + + assert kept == [] + + +@pytest.mark.asyncio +async def test_keyword_overlap_keeps_genuinely_referenced_task(): + """When the moment content shares a meaningful keyword with the task + title, the link is preserved.""" + from fabledassistant.services.tools.journal import ( + _filter_task_ids_by_keyword_overlap, + ) + + fake_session = AsyncMock() + fake_session.execute = AsyncMock() + fake_session.execute.return_value.all = lambda: [ + (5, "Restage Docker on Fam-dockerwin04"), + ] + fake_session.__aenter__ = AsyncMock(return_value=fake_session) + fake_session.__aexit__ = AsyncMock(return_value=None) + + with patch( + "fabledassistant.services.tools.journal.async_session", + return_value=fake_session, + ): + kept = await _filter_task_ids_by_keyword_overlap( + user_id=1, + content="Reinstalled Microsoft container support; Docker restage now works.", + task_ids=[5], + ) + + # 'docker' appears in both → keep + assert kept == [5] + + +@pytest.mark.asyncio +async def test_keyword_overlap_handles_partial_relevance(): + """Mixed ids: keep the related one, drop the unrelated one.""" + from fabledassistant.services.tools.journal import ( + _filter_task_ids_by_keyword_overlap, + ) + + fake_session = AsyncMock() + fake_session.execute = AsyncMock() + fake_session.execute.return_value.all = lambda: [ + (2, "Research Weston's ADHD Evaluation"), + (5, "Restage Docker on Fam-dockerwin04"), + ] + fake_session.__aenter__ = AsyncMock(return_value=fake_session) + fake_session.__aexit__ = AsyncMock(return_value=None) + + with patch( + "fabledassistant.services.tools.journal.async_session", + return_value=fake_session, + ): + kept = await _filter_task_ids_by_keyword_overlap( + user_id=1, + content="Working on restaging Docker in the swarm.", + task_ids=[2, 5], + ) + + assert kept == [5] + + +@pytest.mark.asyncio +async def test_keyword_overlap_empty_task_ids_returns_empty(): + from fabledassistant.services.tools.journal import ( + _filter_task_ids_by_keyword_overlap, + ) + + kept = await _filter_task_ids_by_keyword_overlap( + user_id=1, content="anything", task_ids=[], + ) + assert kept == [] From 9f8b451d15570494c1eef435cbded62e779e57f7 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 29 Apr 2026 09:31:12 -0400 Subject: [PATCH 7/7] fix(journal-prep): bucket tasks + drop non-proximate events (#159) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two filtering issues that made the daily prep noisy and trained the user to ignore it. ## Tasks: bucket into due-today / upcoming / overdue The prep was calling `list_notes(due_before=day_date)` and labeling the result as "tasks due today". That filter is strictly less-than, so it returned only OVERDUE tasks (a single 68-day-stale task in this user's case), while the prompt still framed them as fresh today's work. Each day of the prep treated the same overdue task as new — the user learned to ignore the line entirely. `gather_daily_sections` now runs three queries: - `tasks_due_today` — `due_after=day_date AND due_before=day_date+1` - `tasks_upcoming` — next 7 days, exclusive of today - `tasks_overdue` — strictly before today Overdue entries carry a `days_overdue` count. `_render_sections_for_prompt` emits three labeled headers ("TASKS DUE TODAY", "UPCOMING TASKS", "OVERDUE TASKS (still on the list, not currently due)"). The system prompt has a new TASK BUCKETS rule telling the model: don't call overdue items "due today"; surface them with their staleness duration ("still on the list 68 days") and frame as a backlog reminder rather than today's work. Backwards-compat: `sections["tasks"]` still exists, now as the union of all three buckets — strictly more useful than the prior overdue- only behavior any frontend consumer was getting before. ## Events: tz-aware window + proximity filter The user's "Birthday — 2026-09-29 (FREQ=YEARLY)" event was surfacing in every daily prep, 5 months out. Root cause: `gather_daily_sections` built `day_start`/`day_end` as NAIVE datetimes; `list_events` then called `rrulestr(...).between(naive_from, naive_to)` against an aware `dtstart`, which throws TypeError, hits the `except Exception` fallback, and appends the canonical event row — regardless of whether today is anywhere near a recurrence. Fix: 1. Construct the day window as TZ-aware in the user's local timezone and convert to UTC before the query. RRULE expansion now runs correctly. 2. Defense-in-depth `_filter_proximate_events` drops events whose start_dt is more than 7 days from `day_date` (in the user's local TZ — not UTC, so a Friday 23:00 NY event isn't misclassified as Saturday). If list_events ever leaks a far-future row again, the prep doesn't surface it. 10 new tests in `tests/test_journal_prep_filtering.py` cover task bucketing (overdue marker, due-today no-marker, no-due-date), the proximity filter (the 4/29 reproducer, in-window keeps, local-vs-UTC boundary, unparseable dates kept rather than suppressed), and the rendering (overdue staleness shown, due-today doesn't repeat the date, correct section ordering). 53 tests pass across journal_prep + journal_search + record_moment + calendar_tool + events. Ruff clean. Closes Fable task #159. Co-Authored-By: Claude Opus 4.7 --- src/fabledassistant/services/journal_prep.py | 180 +++++++++++++---- tests/test_journal_prep_filtering.py | 193 +++++++++++++++++++ 2 files changed, 340 insertions(+), 33 deletions(-) create mode 100644 tests/test_journal_prep_filtering.py diff --git a/src/fabledassistant/services/journal_prep.py b/src/fabledassistant/services/journal_prep.py index ec7966a..73734ac 100644 --- a/src/fabledassistant/services/journal_prep.py +++ b/src/fabledassistant/services/journal_prep.py @@ -23,6 +23,7 @@ from __future__ import annotations import datetime import logging +from zoneinfo import ZoneInfo from sqlalchemy import select @@ -38,6 +39,65 @@ from fabledassistant.services.weather import get_cached_weather_rows logger = logging.getLogger(__name__) +# How many days out from today an event needs to be before the prep treats +# it as too far-future to surface. Catches recurring-event canonical rows +# whose RRULE expansion missed (an `rrulestr` failure falls back to the +# canonical event in `list_events`, which leaks far-future occurrences +# into today's prep). +_EVENT_PROXIMITY_DAYS = 7 + + +def _task_to_prep_dict(task, today: datetime.date) -> dict: + """Render a Note row as a prep-payload task entry, tagging overdue + staleness when relevant so the prompt can frame it correctly.""" + d = { + "id": task.id, + "title": task.title, + "status": task.status, + "priority": task.priority, + "due_date": task.due_date.isoformat() if task.due_date else None, + } + if task.due_date and task.due_date < today: + d["days_overdue"] = (today - task.due_date).days + return d + + +def _filter_proximate_events( + events: list[dict], *, day_date: datetime.date, user_tz: ZoneInfo +) -> list[dict]: + """Drop events whose start_dt is more than ``_EVENT_PROXIMITY_DAYS`` + away from ``day_date`` in the user's local timezone. + + Belt-and-suspenders against `list_events` returning a canonical + far-future event (e.g. when RRULE expansion fails and the loop falls + back to the original event row, regardless of date). The user + observed "Birthday — 2026-09-29 (FREQ=YEARLY)" surfacing in every + daily prep 5 months out; this filter keeps the prep proximate. + """ + proximate: list[dict] = [] + for e in events: + raw = e.get("start_dt") or "" + try: + start_dt = datetime.datetime.fromisoformat( + raw.replace("Z", "+00:00") if isinstance(raw, str) else "" + ) + local_date = start_dt.astimezone(user_tz).date() + delta = abs((local_date - day_date).days) + except (ValueError, TypeError, AttributeError): + # Unparseable date — keep the event rather than suppress real data. + proximate.append(e) + continue + if delta <= _EVENT_PROXIMITY_DAYS: + proximate.append(e) + else: + logger.info( + "daily_prep: dropping non-proximate event %r start_local=%s " + "(%d days from day %s)", + e.get("title"), local_date.isoformat(), delta, day_date.isoformat(), + ) + return proximate + + async def gather_daily_sections( *, user_id: int, @@ -48,41 +108,71 @@ async def gather_daily_sections( Pure data fetching — no LLM call. Each section degrades to an empty list/dict on failure so the caller always gets a complete shape. + + Tasks are returned in three explicit buckets so the prompt can frame + overdue items correctly (instead of calling them "due today" — the + pre-2026-04-29 behavior, before this rewrite). """ sections: dict = {} + next_day = day_date + datetime.timedelta(days=1) + upcoming_end = day_date + datetime.timedelta(days=8) try: - tasks_today, _ = await list_notes( - user_id=user_id, - is_task=True, - status=["todo", "in_progress"], - due_before=day_date, - limit=20, - sort="due_date", - order="asc", + due_today_rows, _ = await list_notes( + user_id=user_id, is_task=True, status=["todo", "in_progress"], + due_after=day_date, due_before=next_day, + limit=20, sort="due_date", order="asc", + ) + upcoming_rows, _ = await list_notes( + user_id=user_id, is_task=True, status=["todo", "in_progress"], + due_after=next_day, due_before=upcoming_end, + limit=20, sort="due_date", order="asc", + ) + overdue_rows, _ = await list_notes( + user_id=user_id, is_task=True, status=["todo", "in_progress"], + due_before=day_date, + limit=20, sort="due_date", order="asc", + ) + sections["tasks_due_today"] = [_task_to_prep_dict(t, day_date) for t in due_today_rows] + sections["tasks_upcoming"] = [_task_to_prep_dict(t, day_date) for t in upcoming_rows] + sections["tasks_overdue"] = [_task_to_prep_dict(t, day_date) for t in overdue_rows] + # Backwards-compat alias for any consumers still reading sections["tasks"]. + # The combined view is more useful than the prior overdue-only behavior. + sections["tasks"] = ( + sections["tasks_due_today"] + + sections["tasks_upcoming"] + + sections["tasks_overdue"] ) - sections["tasks"] = [ - { - "id": t.id, - "title": t.title, - "status": t.status, - "priority": t.priority, - "due_date": t.due_date.isoformat() if t.due_date else None, - } - for t in tasks_today - ] except Exception: logger.exception("daily_prep tasks section failed for user %d", user_id) + sections["tasks_due_today"] = [] + sections["tasks_upcoming"] = [] + sections["tasks_overdue"] = [] sections["tasks"] = [] try: - day_start = datetime.datetime.combine(day_date, datetime.time.min) - day_end = datetime.datetime.combine(day_date, datetime.time.max) - sections["events"] = await list_events( + try: + user_tz = ZoneInfo(user_timezone) + except Exception: + logger.warning("daily_prep: invalid user_timezone %r — defaulting to UTC", user_timezone) + user_tz = ZoneInfo("UTC") + # Build the local-day window in the user's TZ, then convert to UTC + # for the DB / RRULE expansion. A naive datetime here previously + # caused rrule.between() to throw, falling back to the canonical + # event row regardless of date — the source of stale recurring + # events polluting every daily prep. + day_start_local = datetime.datetime.combine(day_date, datetime.time.min, tzinfo=user_tz) + day_end_local = datetime.datetime.combine(day_date, datetime.time.max, tzinfo=user_tz) + day_start = day_start_local.astimezone(datetime.timezone.utc) + day_end = day_end_local.astimezone(datetime.timezone.utc) + all_events = await list_events( user_id=user_id, date_from=day_start, date_to=day_end, ) + sections["events"] = _filter_proximate_events( + all_events, day_date=day_date, user_tz=user_tz, + ) except Exception: logger.exception("daily_prep events section failed for user %d", user_id) sections["events"] = [] @@ -148,22 +238,40 @@ async def _open_threads(*, user_id: int, day_date: datetime.date) -> list[dict]: ] +def _render_task_line(t: dict, *, include_due: bool, include_overdue: bool) -> str: + line = f" - {t.get('title', '?')}" + if include_overdue and t.get("days_overdue"): + line += f" (due {t['due_date']}, {t['days_overdue']} days ago)" + elif include_due and t.get("due_date"): + line += f" (due {t['due_date']})" + if t.get("priority") and t["priority"] not in (None, "none"): + line += f" [{t['priority']} priority]" + if t.get("status") == "in_progress": + line += " [in progress]" + return line + + def _render_sections_for_prompt(sections: dict) -> str: """Render the gathered sections as a structured plain-text block for the LLM.""" lines: list[str] = [] - tasks = sections.get("tasks") or [] - if tasks: - lines.append("TASKS (todo or in-progress):") - for t in tasks[:12]: - line = f" - {t.get('title', '?')}" - if t.get("due_date"): - line += f" (due {t['due_date']})" - if t.get("priority") and t["priority"] not in (None, "none"): - line += f" [{t['priority']} priority]" - if t.get("status") == "in_progress": - line += " [in progress]" - lines.append(line) + due_today = sections.get("tasks_due_today") or [] + upcoming = sections.get("tasks_upcoming") or [] + overdue = sections.get("tasks_overdue") or [] + if due_today: + lines.append("TASKS DUE TODAY:") + for t in due_today[:8]: + lines.append(_render_task_line(t, include_due=False, include_overdue=False)) + lines.append("") + if upcoming: + lines.append("UPCOMING TASKS (next 7 days):") + for t in upcoming[:8]: + lines.append(_render_task_line(t, include_due=True, include_overdue=False)) + lines.append("") + if overdue: + lines.append("OVERDUE TASKS (still on the list, not currently due):") + for t in overdue[:8]: + lines.append(_render_task_line(t, include_due=False, include_overdue=True)) lines.append("") events = sections.get("events") or [] @@ -242,6 +350,12 @@ _PREP_SYSTEM_PROMPT = ( "keep the prose factual and useful, not sentimental.\n" "- 4 to 7 sentences total. Tight. No padding, no flowery openings, no \"Good morning\" " "greetings unless the actual content warrants two clauses' worth.\n" + "- TASK BUCKETS — three sections may appear: TASKS DUE TODAY, UPCOMING TASKS, " + "OVERDUE TASKS. Lead with TASKS DUE TODAY when present. Do NOT call overdue items " + "\"due today\" — they aren't. When OVERDUE TASKS appears, surface it with the " + "staleness duration (\"still on the list 68 days\") and frame it as something to " + "revisit, not as today's work. If the only data is overdue, lead with it but " + "frame it as a backlog reminder.\n" "- If RECENT JOURNAL MOMENTS or OPEN THREADS are present, mention one or two BRIEFLY " "at the end as context — not as the lead. Skip them if nothing notable.\n" "- Close with one short invitation to journal: \"What's on your mind?\", " diff --git a/tests/test_journal_prep_filtering.py b/tests/test_journal_prep_filtering.py new file mode 100644 index 0000000..58f68fe --- /dev/null +++ b/tests/test_journal_prep_filtering.py @@ -0,0 +1,193 @@ +"""Tests for the journal prep filtering helpers added in #159.""" + +import datetime +from types import SimpleNamespace +from zoneinfo import ZoneInfo + +import pytest + + +# ── _task_to_prep_dict ──────────────────────────────────────────────────────── + + +def test_task_to_prep_dict_marks_overdue_with_days_count(): + from fabledassistant.services.journal_prep import _task_to_prep_dict + + today = datetime.date(2026, 4, 29) + task = SimpleNamespace( + id=2, title="Research Weston's ADHD Evaluation", + status="todo", priority="high", + due_date=datetime.date(2026, 2, 20), + ) + d = _task_to_prep_dict(task, today) + assert d["days_overdue"] == 68 + assert d["due_date"] == "2026-02-20" + + +def test_task_to_prep_dict_due_today_no_overdue_marker(): + from fabledassistant.services.journal_prep import _task_to_prep_dict + + today = datetime.date(2026, 4, 29) + task = SimpleNamespace( + id=10, title="Pick up dry cleaning", + status="todo", priority="none", + due_date=today, + ) + d = _task_to_prep_dict(task, today) + assert "days_overdue" not in d + + +def test_task_to_prep_dict_handles_no_due_date(): + from fabledassistant.services.journal_prep import _task_to_prep_dict + + task = SimpleNamespace( + id=11, title="Long-running side project", + status="in_progress", priority="medium", + due_date=None, + ) + d = _task_to_prep_dict(task, datetime.date(2026, 4, 29)) + assert d["due_date"] is None + assert "days_overdue" not in d + + +# ── _filter_proximate_events ────────────────────────────────────────────────── + + +def test_filter_proximate_drops_far_future_recurring_event(): + """The reported failure: a recurring Birthday event with start_dt + 2026-09-29 surfaced in every daily prep. The proximity filter drops it.""" + from fabledassistant.services.journal_prep import _filter_proximate_events + + events = [ + {"id": 13, "title": "Birthday", "start_dt": "2026-09-29T00:00:00+00:00"}, + ] + kept = _filter_proximate_events( + events, + day_date=datetime.date(2026, 4, 29), + user_tz=ZoneInfo("America/New_York"), + ) + assert kept == [] + + +def test_filter_proximate_keeps_events_within_window(): + """Events within ±7 days of the prep date stay.""" + from fabledassistant.services.journal_prep import _filter_proximate_events + + events = [ + {"id": 1, "title": "Today", "start_dt": "2026-04-29T13:00:00+00:00"}, + {"id": 2, "title": "Tomorrow", "start_dt": "2026-04-30T12:00:00+00:00"}, + {"id": 3, "title": "Next week", "start_dt": "2026-05-05T15:00:00+00:00"}, + {"id": 4, "title": "Two weeks out", "start_dt": "2026-05-15T15:00:00+00:00"}, + ] + kept = _filter_proximate_events( + events, + day_date=datetime.date(2026, 4, 29), + user_tz=ZoneInfo("America/New_York"), + ) + kept_ids = [e["id"] for e in kept] + assert 1 in kept_ids + assert 2 in kept_ids + assert 3 in kept_ids + assert 4 not in kept_ids + + +def test_filter_proximate_uses_local_date_not_utc(): + """An event at 04:30 UTC on 2026-04-30 = 00:30 local on 2026-04-30 in + NY (EDT, UTC-4). Local date is 2026-04-30, delta from 2026-04-29 = 1 + day. Must NOT cross-classify based on UTC date alone.""" + from fabledassistant.services.journal_prep import _filter_proximate_events + + events = [ + {"id": 99, "title": "Late-night dentist", "start_dt": "2026-04-30T04:30:00+00:00"}, + ] + kept = _filter_proximate_events( + events, + day_date=datetime.date(2026, 4, 29), + user_tz=ZoneInfo("America/New_York"), + ) + assert len(kept) == 1 + + +def test_filter_proximate_keeps_unparseable_dates(): + """Bad date strings are kept rather than silently suppressing real + events on a parser bug.""" + from fabledassistant.services.journal_prep import _filter_proximate_events + + events = [ + {"id": 50, "title": "Garbage", "start_dt": "not-a-date"}, + {"id": 51, "title": "Empty", "start_dt": ""}, + {"id": 52, "title": "Missing"}, + ] + kept = _filter_proximate_events( + events, + day_date=datetime.date(2026, 4, 29), + user_tz=ZoneInfo("America/New_York"), + ) + assert len(kept) == 3 + + +# ── _render_sections_for_prompt — overdue framing ──────────────────────────── + + +def test_render_overdue_includes_staleness_duration(): + """The rendered prompt block must surface days_overdue so the LLM + can frame stale tasks correctly.""" + from fabledassistant.services.journal_prep import _render_sections_for_prompt + + sections = { + "tasks_due_today": [], + "tasks_upcoming": [], + "tasks_overdue": [{ + "id": 2, "title": "Research Weston's ADHD Evaluation", + "status": "todo", "priority": "high", + "due_date": "2026-02-20", "days_overdue": 68, + }], + } + rendered = _render_sections_for_prompt(sections) + assert "OVERDUE TASKS" in rendered + assert "68 days ago" in rendered + assert "2026-02-20" in rendered + # Crucially, NOT framed as due today. + assert "DUE TODAY" not in rendered + + +def test_render_due_today_no_due_date_repetition(): + """Tasks in the DUE TODAY bucket don't need the (due 2026-04-29) + parenthetical — the section header already says 'today'.""" + from fabledassistant.services.journal_prep import _render_sections_for_prompt + + sections = { + "tasks_due_today": [{ + "id": 1, "title": "Pick up dry cleaning", + "status": "todo", "priority": "none", + "due_date": "2026-04-29", + }], + "tasks_upcoming": [], + "tasks_overdue": [], + } + rendered = _render_sections_for_prompt(sections) + assert "TASKS DUE TODAY" in rendered + assert "Pick up dry cleaning" in rendered + # The line itself shouldn't repeat the due date. + line = next( + (l for l in rendered.splitlines() if "Pick up dry cleaning" in l), + "", + ) + assert "2026-04-29" not in line + + +def test_render_three_buckets_in_correct_order(): + """When all three buckets have content, the prompt sees them in + DUE TODAY → UPCOMING → OVERDUE order so the LLM leads with today.""" + from fabledassistant.services.journal_prep import _render_sections_for_prompt + + sections = { + "tasks_due_today": [{"id": 1, "title": "Today task", "status": "todo", "priority": "none", "due_date": "2026-04-29"}], + "tasks_upcoming": [{"id": 2, "title": "Upcoming task", "status": "todo", "priority": "none", "due_date": "2026-05-02"}], + "tasks_overdue": [{"id": 3, "title": "Stale task", "status": "todo", "priority": "none", "due_date": "2026-02-20", "days_overdue": 68}], + } + rendered = _render_sections_for_prompt(sections) + today_idx = rendered.index("TASKS DUE TODAY") + upcoming_idx = rendered.index("UPCOMING TASKS") + overdue_idx = rendered.index("OVERDUE TASKS") + assert today_idx < upcoming_idx < overdue_idx