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.* 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 = [ diff --git a/src/fabledassistant/services/journal_pipeline.py b/src/fabledassistant/services/journal_pipeline.py index 0fa658c..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 = { @@ -134,7 +165,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/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/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 90828e2..0218ddc 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,86 @@ 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 + + +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="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. " + "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"}, - "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 +167,36 @@ 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"}, + "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 + # 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}"} + 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: + 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 +286,34 @@ 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. " + "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)"}, "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."}, + "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."}, }, required=["query"], ) @@ -198,16 +332,24 @@ 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}"} + 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: + 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/src/fabledassistant/services/tools/journal.py b/src/fabledassistant/services/tools/journal.py index 38cf416..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, @@ -78,7 +170,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", @@ -171,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", ) ) @@ -199,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_calendar_tool_tz.py b/tests/test_calendar_tool_tz.py index bab715f..71b0e24 100644 --- a/tests/test_calendar_tool_tz.py +++ b/tests/test_calendar_tool_tz.py @@ -132,3 +132,482 @@ 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) + + +# ── 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 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 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 == []