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>
This commit is contained in:
@@ -54,19 +54,22 @@ async def create_event():
|
||||
end_dt = _parse_dt(data["end_dt"]) if data.get("end_dt") else None
|
||||
except ValueError:
|
||||
return jsonify({"error": "Invalid datetime format"}), 400
|
||||
event = await events_svc.create_event(
|
||||
user_id=_get_current_user_id(),
|
||||
title=data["title"],
|
||||
start_dt=start_dt,
|
||||
end_dt=end_dt,
|
||||
all_day=data.get("all_day", False),
|
||||
description=data.get("description", ""),
|
||||
location=data.get("location", ""),
|
||||
color=data.get("color", ""),
|
||||
recurrence=data.get("recurrence"),
|
||||
project_id=data.get("project_id"),
|
||||
reminder_minutes=data.get("reminder_minutes"),
|
||||
)
|
||||
try:
|
||||
event = await events_svc.create_event(
|
||||
user_id=_get_current_user_id(),
|
||||
title=data["title"],
|
||||
start_dt=start_dt,
|
||||
end_dt=end_dt,
|
||||
all_day=data.get("all_day", False),
|
||||
description=data.get("description", ""),
|
||||
location=data.get("location", ""),
|
||||
color=data.get("color", ""),
|
||||
recurrence=data.get("recurrence"),
|
||||
project_id=data.get("project_id"),
|
||||
reminder_minutes=data.get("reminder_minutes"),
|
||||
)
|
||||
except ValueError as exc:
|
||||
return jsonify({"error": str(exc)}), 400
|
||||
return jsonify(event.to_dict()), 201
|
||||
|
||||
|
||||
@@ -106,11 +109,14 @@ async def update_event(event_id: int):
|
||||
fields[dt_field] = _parse_dt(data[dt_field])
|
||||
except ValueError:
|
||||
return jsonify({"error": f"Invalid datetime for {dt_field}"}), 400
|
||||
event = await events_svc.update_event(
|
||||
user_id=_get_current_user_id(),
|
||||
event_id=event_id,
|
||||
**fields,
|
||||
)
|
||||
try:
|
||||
event = await events_svc.update_event(
|
||||
user_id=_get_current_user_id(),
|
||||
event_id=event_id,
|
||||
**fields,
|
||||
)
|
||||
except ValueError as exc:
|
||||
return jsonify({"error": str(exc)}), 400
|
||||
if event is None:
|
||||
return jsonify({"error": "Event not found"}), 404
|
||||
return jsonify(event.to_dict())
|
||||
|
||||
@@ -32,7 +32,17 @@ async def create_event(
|
||||
attendees: list[str] | None = None,
|
||||
calendar_name: str | None = None,
|
||||
) -> Event:
|
||||
"""Create an event in the DB, then fire a CalDAV push task."""
|
||||
"""Create an event in the DB, then fire a CalDAV push task.
|
||||
|
||||
Raises ValueError if `end_dt` is set but not strictly after `start_dt`.
|
||||
Catching this at write time prevents the "end before start" data state
|
||||
that breaks downstream filters (Fable #160).
|
||||
"""
|
||||
if end_dt is not None and end_dt <= start_dt:
|
||||
raise ValueError(
|
||||
f"end_dt ({end_dt.isoformat()}) must be after start_dt "
|
||||
f"({start_dt.isoformat()}); pass end_dt=None for point events."
|
||||
)
|
||||
uid = str(uuid.uuid4())
|
||||
async with async_session() as session:
|
||||
event = Event(
|
||||
@@ -87,12 +97,16 @@ async def list_events(
|
||||
async with async_session() as session:
|
||||
# Match strategy:
|
||||
# - Recurring events: fetch all, expand via rrule below.
|
||||
# - Non-recurring with an end_dt: standard overlap — starts before
|
||||
# date_to and ends after date_from.
|
||||
# - Non-recurring with no end_dt: treat as a point event at
|
||||
# start_dt, include only if start_dt falls within the window.
|
||||
# (Previously this branch matched any event with a null end_dt,
|
||||
# returning all past events as "happening today".)
|
||||
# - Non-recurring with a VALID end_dt (end > start): standard
|
||||
# overlap — starts before date_to and ends after date_from.
|
||||
# - Non-recurring with no end_dt OR an invalid end_dt (end <=
|
||||
# start; almost always a tool-call mishap): treat as a point
|
||||
# event at start_dt and include only if start_dt is in the
|
||||
# window. The fallback for invalid end_dt is critical — without
|
||||
# it, an end_dt accidentally set in the past makes the event
|
||||
# vanish from every list_events call regardless of where its
|
||||
# start_dt actually is. Discovered 2026-04-29; see Fable #160
|
||||
# for the structural fix that replaces end_dt with duration.
|
||||
result = await session.execute(
|
||||
select(Event).where(
|
||||
Event.user_id == user_id,
|
||||
@@ -102,9 +116,16 @@ async def list_events(
|
||||
Event.recurrence.is_(None),
|
||||
Event.start_dt <= date_to,
|
||||
or_(
|
||||
Event.end_dt >= date_from,
|
||||
and_(
|
||||
Event.end_dt.is_(None),
|
||||
Event.end_dt.isnot(None),
|
||||
Event.end_dt > Event.start_dt,
|
||||
Event.end_dt >= date_from,
|
||||
),
|
||||
and_(
|
||||
or_(
|
||||
Event.end_dt.is_(None),
|
||||
Event.end_dt <= Event.start_dt,
|
||||
),
|
||||
Event.start_dt >= date_from,
|
||||
),
|
||||
),
|
||||
@@ -120,8 +141,14 @@ async def list_events(
|
||||
items.append(event.to_dict())
|
||||
continue
|
||||
|
||||
# Expand recurring event occurrences within [date_from, date_to]
|
||||
duration = (event.end_dt - event.start_dt) if event.end_dt else None
|
||||
# Expand recurring event occurrences within [date_from, date_to].
|
||||
# Treat end_dt <= start_dt as no-end (invalid data); falling back
|
||||
# to point events instead of computing a negative duration.
|
||||
duration = (
|
||||
(event.end_dt - event.start_dt)
|
||||
if event.end_dt and event.end_dt > event.start_dt
|
||||
else None
|
||||
)
|
||||
try:
|
||||
rule = rrulestr(event.recurrence, dtstart=event.start_dt, ignoretz=False)
|
||||
occurrences = rule.between(date_from, date_to, inc=True)
|
||||
@@ -173,7 +200,13 @@ async def search_events(
|
||||
|
||||
|
||||
async def update_event(user_id: int, event_id: int, **fields) -> Event | None:
|
||||
"""Partial update. Returns updated event or None if not found."""
|
||||
"""Partial update. Returns updated event or None if not found.
|
||||
|
||||
Raises ValueError if the resulting state would have `end_dt <= start_dt`.
|
||||
Validation runs against the *post-update* values so updates that fix
|
||||
one side of the constraint without explicitly clearing the other still
|
||||
fail loudly instead of persisting invalid data.
|
||||
"""
|
||||
async with async_session() as session:
|
||||
result = await session.execute(
|
||||
select(Event).where(Event.id == event_id, Event.user_id == user_id)
|
||||
@@ -186,6 +219,17 @@ async def update_event(user_id: int, event_id: int, **fields) -> Event | None:
|
||||
"location", "color", "recurrence", "project_id", "reminder_minutes"}
|
||||
# Nullable fields that callers can explicitly set to None to clear
|
||||
nullable = {"end_dt", "recurrence", "project_id", "reminder_minutes"}
|
||||
|
||||
# Pre-validate the *resulting* start/end against the post-update
|
||||
# state, not against either field's pre-update value alone.
|
||||
new_start = fields["start_dt"] if "start_dt" in fields and fields["start_dt"] is not None else event.start_dt
|
||||
new_end = fields["end_dt"] if "end_dt" in fields else event.end_dt
|
||||
if new_end is not None and new_end <= new_start:
|
||||
raise ValueError(
|
||||
f"end_dt ({new_end.isoformat()}) must be after start_dt "
|
||||
f"({new_start.isoformat()}); pass end_dt=None for point events."
|
||||
)
|
||||
|
||||
for key, value in fields.items():
|
||||
if key in allowed and (value is not None or key in nullable):
|
||||
setattr(event, key, value)
|
||||
|
||||
@@ -213,22 +213,25 @@ async def create_event_tool(*, user_id, arguments, **_ctx):
|
||||
proj = await resolve_project(user_id, project_name)
|
||||
if proj:
|
||||
project_id = proj.id
|
||||
event = await events_create_event(
|
||||
user_id=user_id,
|
||||
title=arguments.get("title", "Untitled Event"),
|
||||
start_dt=start_dt,
|
||||
end_dt=end_dt,
|
||||
all_day=all_day,
|
||||
description=arguments.get("description") or "",
|
||||
location=arguments.get("location") or "",
|
||||
color=arguments.get("color") or "",
|
||||
recurrence=arguments.get("recurrence"),
|
||||
project_id=project_id,
|
||||
duration=arguments.get("duration"),
|
||||
reminder_minutes=arguments.get("reminder_minutes"),
|
||||
attendees=arguments.get("attendees"),
|
||||
calendar_name=arguments.get("calendar_name"),
|
||||
)
|
||||
try:
|
||||
event = await events_create_event(
|
||||
user_id=user_id,
|
||||
title=arguments.get("title", "Untitled Event"),
|
||||
start_dt=start_dt,
|
||||
end_dt=end_dt,
|
||||
all_day=all_day,
|
||||
description=arguments.get("description") or "",
|
||||
location=arguments.get("location") or "",
|
||||
color=arguments.get("color") or "",
|
||||
recurrence=arguments.get("recurrence"),
|
||||
project_id=project_id,
|
||||
duration=arguments.get("duration"),
|
||||
reminder_minutes=arguments.get("reminder_minutes"),
|
||||
attendees=arguments.get("attendees"),
|
||||
calendar_name=arguments.get("calendar_name"),
|
||||
)
|
||||
except ValueError as exc:
|
||||
return {"success": False, "error": str(exc)}
|
||||
return {"success": True, "type": "event", "data": event.to_dict()}
|
||||
|
||||
|
||||
@@ -360,7 +363,10 @@ async def update_event_tool(*, user_id, arguments, **_ctx):
|
||||
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)
|
||||
try:
|
||||
updated = await events_update_event(user_id=user_id, event_id=event_to_update.id, **fields)
|
||||
except ValueError as exc:
|
||||
return {"success": False, "error": str(exc)}
|
||||
if updated is None:
|
||||
return {"success": False, "error": "Event not found or update failed."}
|
||||
return {"success": True, "type": "event_updated", "data": updated.to_dict()}
|
||||
|
||||
Reference in New Issue
Block a user