Commit Graph

8 Commits

Author SHA1 Message Date
bvandeusen 84640a0dc4 fix(events-tools): refuse ambiguous query, require event_id to disambiguate (#161)
Root cause of the 2026-04-29 dentist-appointment incident: the model
called update_event(query="Appointment") when two events had
"Appointment" in their titles. find_events_by_query returned both,
upcoming-first ordered by start_dt — matches[0] was id=2 (a stale
pre-existing event with garbage end_dt), not id=15 (the one the user
just created via the journal flow). update_event_tool silently took
matches[0] and mutated the wrong event.

Fix: a new resolver helper `_resolve_event_for_action` funnels both
update_event_tool and delete_event_tool through one disambiguation
path. Lookup precedence:
  - `event_id` → exact get_event lookup, no query at all
  - `query` matching exactly one event → proceed
  - `query` matching zero → return success=False, "no event found"
  - `query` matching 2+ events → return success=False with a
    `candidates` array of {id, title, start_dt, location} so the
    model can pick one and call again with `event_id`

The candidates list is capped at 8 to keep the model's context tight.
The error message names the count and the next-step ("pass event_id
or refine the query") so the model can self-correct in one turn.

For delete_event, the disambiguation is even more important — the
silent-matches[0] path would have deleted the wrong event outright
rather than just mutating it. The tool description leans into that:
"Deleting the wrong event is a costly user error; never guess."

Tool surface change: `query` and `event_id` are now both optional;
the tool errors clearly when neither is supplied. The model already
knows id values from prior tool results (returned in `data.id`),
which is the natural feeder for the disambiguation flow.

5 new tests in test_calendar_tool_tz.py cover:
- ambiguous query → success=False with candidate list, no mutation
- event_id supplied → bypasses query lookup entirely
- non-existent event_id → clear "no event found" error
- neither identifier → "query or event_id required" error
- same disambiguation enforced for delete_event_tool

46 calendar/events tests pass; ruff clean.

Closes Fable #161.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-04-29 14:29:13 -04:00
bvandeusen b7e7073425 Revert "fix(events): tolerate corrupt end_dt + reject end<=start at write time"
This reverts commit 94b169f31c.
2026-04-29 14:18:01 -04:00
bvandeusen 94b169f31c fix(events): tolerate corrupt end_dt + reject end<=start at write time
A prod event surfaced today with `start_dt=2026-05-01T12:00Z` and
`end_dt=2026-03-30T12:00Z` — end was 32 days BEFORE start, almost
certainly from an earlier tool-call mishap (Fable #161). The
list_events filter trusted the bogus end_dt and excluded the event
from every read path that hit the upcoming window, even though
start_dt was correctly in range. The event stayed visible in the
calendar grid (different range) but vanished from "Upcoming",
search, briefings, and journal prep events list.

This is the hotfix half of the response. The structural follow-up is
Fable #160 — replace end_dt with a duration column so invalid state
becomes inexpressible.

## A. Filter robustness in list_events

Treat `end_dt <= start_dt` as if no end_dt exists. The filter now
splits into two branches:
- valid duration: end_dt IS NOT NULL AND end_dt > start_dt AND
  end_dt >= date_from
- no/invalid duration: (end_dt IS NULL OR end_dt <= start_dt) AND
  start_dt >= date_from

Same change applied to the recurring-event expansion's `duration`
calculation, which was producing negative timedeltas for corrupted
rows and computing nonsensical occurrence end times.

## B. Write-side validation in create/update

`create_event` and `update_event` now raise ValueError when the
resulting state would have end_dt <= start_dt. Update validates
against the *post-update* state, not just the field being changed —
so pushing start_dt past an existing end_dt also fails loudly. Bad
data shouldn't be persistable from any write path.

Surfaced cleanly:
- Calendar tool wrappers (create_event_tool / update_event_tool)
  catch ValueError and return `{success: false, error: ...}`, which
  the model can read and self-correct.
- Route handlers (POST /api/events, PATCH /api/events/<id>) catch
  and return HTTP 400 with the validator's message instead of
  letting it bubble to a 500.

4 new tests in test_events_service.py:
- create rejects end before start
- create rejects equal start/end (zero duration)
- update validates the post-update state (start pushed past existing end)
- list_events surfaces an event whose end_dt is before its start_dt

34 event-related tests pass; ruff clean.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-04-29 13:48:28 -04:00
bvandeusen 35fab6cbf7 fix(calendar-tool): forbid placeholder events in create_event description
Reproducer (2026-04-29 dentist appointment): user said "this Friday,
I have an appointment" with no other details. The model immediately
called create_event with title="Appointment", description="User
mentioned an appointment this Friday but hasn't provided details
yet.", all_day=true. THEN it asked the user for time/location in
its reply. When the user came back with "8am at my dentist for
permanent crown fitting", the model called update_event — but never
updated the title, leaving the placeholder "Appointment" in the
calendar permanently.

The bug isn't about the tool surface, it's that the model created
an event before it had real content. The system prompt had no rule
against this, so the model hedged: "log a placeholder, ask for
details, then update". That pattern pollutes the calendar with
garbage titles and forces immediate update_event calls.

create_event tool description now includes an explicit anti-pattern:
record a moment, ask for the missing pieces, and only call create_event
once you have actual title + time + location. Stand-in titles like
"Appointment" / "Meeting" / "Event" with "details TBD" descriptions
are explicitly named as the failure mode.

Pure prompt change. 18 tests pass; ruff clean.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-04-29 10:54:39 -04:00
bvandeusen 03d725ea3e fix(calendar-tool): anchor today's weekday in prompts + verify expected_weekday on create/update
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 <noreply@anthropic.com>
2026-04-29 08:43:32 -04:00
bvandeusen 611c940527 fix(calendar-tool): split start/end into date+time to make event creation TZ-durable
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 <noreply@anthropic.com>
2026-04-29 08:16:25 -04:00
bvandeusen e4e1d1da49 fix(tz): interpret calendar and briefing dates in user's local timezone
Two related bugs where the server defaulted naive datetimes to UTC instead
of the configured user timezone, causing all-day events to land on the
previous day and briefings to "disappear" at UTC midnight.

- New services/tz.py helpers: get_user_tz, user_today, user_briefing_date
  (the briefing day flips at 4am local to align with the compilation slot,
  so the 00:00-04:00 local window still shows yesterday's briefing until
  the new one is generated).
- calendar create/list/update tools now parse naive datetimes in the
  user's TZ before converting to UTC for storage, and tool descriptions
  tell the model to pass plain local dates.
- briefing_conversations.get_or_create_today_conversation and the
  reset-today route use user_briefing_date so the in-progress briefing
  doesn't get replaced at 19:00 NY / UTC midnight.
- _run_profile_closeout targets user-local "yesterday" for consistency.

Regression tests added for the TZ helpers and the calendar tool.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-04-13 15:35:27 -04:00
bvandeusen ce2d76447c refactor(tools): decorator-based tool registry replaces monolithic tools.py
Split 2566-line tools.py into a tools/ package with @tool decorator
registration. Each tool's schema, metadata, and implementation live
together. Briefing eligibility is now a briefing=True flag instead of
a separate frozenset allowlist. Conditional inclusion (CalDAV, SearXNG)
uses requires= metadata. Public API (get_tools_for_user, execute_tool)
unchanged.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-04-12 10:01:00 -04:00