From da57efad731390f1dd751a285b51cc2741eaabc7 Mon Sep 17 00:00:00 2001 From: Sam Rolfe Date: Thu, 15 Oct 2026 19:09:06 +1100 Subject: [PATCH] Phase 6: fix the live Home Assistant write (Local Calendar, no event id) The first live run against the new calendar.school entity failed loudly twice, and both causes are fixed here. - The Local Calendar rejects `return_response` on calendar.create_event. The client retries the call without it and ignores the changed-states array that Home Assistant returns instead. - Home Assistant then reports no event id. The item keeps the description marker ([compass-nhs:]) as its reference, so it leaves the candidate set and is never written twice. The live retry proved it: both events already in the calendar were adopted by their markers, with no duplicate. - calendar.get_events reports `start` / `end` (a date-time or a bare date), not `start_date_time`; the reader now accepts all three shapes. A model must no longer classify events. The calendar write depends on the school-event rule, and the LLM labelled every event "normal"; events now keep the deterministic rule under both classifiers. Also fix the Phase 5 screenshot script: the calendar banner shares the `.scan` class, so select each banner by its labelled section. Docs record the accepted decision: Sam chose a Home Assistant Local Calendar, so the target is calendar.school. Co-Authored-By: Paperclip --- README.md | 8 +-- docs/phase-6-calendar-notes.md | 69 ++++++++++++++++---------- internal/calendar/ha.go | 57 +++++++++++++++++---- internal/calendar/ha_test.go | 91 ++++++++++++++++++++++++++++------ internal/calendar/sync.go | 18 +++++-- internal/calendar/sync_test.go | 48 ++++++++++++++++-- internal/parse/importance.go | 10 ++++ internal/parse/parse_test.go | 29 +++++++++++ scripts/screenshot-phase5.mjs | 17 +++++-- 9 files changed, 279 insertions(+), 68 deletions(-) diff --git a/README.md b/README.md index 0c752a0..5d5962c 100644 --- a/README.md +++ b/README.md @@ -15,10 +15,10 @@ re-run never duplicates an event) and it fails loudly (a failed write is shown on the dashboard and recorded in `calendar_pushes`). See [`docs/phase-6-calendar-notes.md`](docs/phase-6-calendar-notes.md). -**Open decision:** the brief says *not Google*, and Home Assistant has no -non-Google writable calendar yet. The target entity is configurable -(`SCHOOL_HA_CALENDAR_ENTITY`) and the question is on -[AUD-110](/AUD/issues/AUD-110). +The target is a **Home Assistant Local Calendar**, `calendar.school` (added +2026-10-15 after Sam chose it over the Google-backed entities that Home +Assistant had). The entity stays configurable with +`SCHOOL_HA_CALENDAR_ENTITY`. ```bash scripts/calendar-push.sh entities # list the writable calendar entities diff --git a/docs/phase-6-calendar-notes.md b/docs/phase-6-calendar-notes.md index d4bfd64..8164fe3 100644 --- a/docs/phase-6-calendar-notes.md +++ b/docs/phase-6-calendar-notes.md @@ -1,8 +1,14 @@ # Phase 6 — the calendar write (Sam's Home Assistant calendar) -Status: **code complete and tested; the calendar target needs Sam's decision** -(AUD-110). The service now writes important school events through the Home -Assistant REST API. Nothing is written until a calendar entity is chosen. +Status: **done** (AUD-110). Sam chose a **Home Assistant Local Calendar**, and +the service writes important school events to it through the Home Assistant REST +API. + +| | | +|---|---| +| Target entity | `calendar.school` (Local Calendar "School", added 2026-10-15) | +| Configuration | `environment.d/school-service.env` (gitignored): `SCHOOL_HA_URL`, `SCHOOL_HA_TOKEN`, `SCHOOL_HA_CALENDAR_ENTITY`, `SCHOOL_HA_TIMEOUT` | +| First live events | *Senior Sub-School Community Service for Uniform Infringement* and *End of Year Senior Music Concert — HALL* (2026-10-15) | ## What it does @@ -82,32 +88,37 @@ scripts/calendar-push.sh push --dry-run --json The item screen has an **Add to calendar** button for an important future event, and shows `in the calendar` with the Home Assistant event id once it is written. -## The open decision: which calendar entity? +## Decision (accepted 2026-10-15) -The Home Assistant instance (`192.168.20.30`, HA 2026.9.4) has three calendar -entities, and only two are writable: +The Home Assistant instance (`192.168.20.30`, HA 2026.9.4) had three calendar +entities, and all the writable ones were Google-backed +(`calendar.samuelrolfe_googlemail_com`, `calendar.birthdays`); the brief says +**Sam's Home Assistant calendar (not Google)**, and no non-Google calendar +existed. Sam chose to **add a Local Calendar**, so: -| Entity | Writable | Backing | -|---|---|---| -| `calendar.samuelrolfe_googlemail_com` | yes | Google, synced into Home Assistant | -| `calendar.birthdays` | yes | Google | -| `calendar.holidays_in_australia` | no | Google | +- The Local Calendar integration was added through the Home Assistant config + flow, creating the entity **`calendar.school`** (title "School"). It is + Home-Assistant-local, never touches Google, and the change is purely additive. +- `SCHOOL_HA_CALENDAR_ENTITY=calendar.school`. -The brief says **Sam's Home Assistant calendar (not Google)**. Today Home -Assistant has **no non-Google writable calendar**: the `local_calendar` -integration is not installed, and no CalDAV/remote calendar is configured. +The calendar is a family calendar, so an event carries the child's name only +when it belongs to one child. -So the target must be chosen before the event is written: +## Home Assistant quirks found on the first live run -1. **Add a Local Calendar to Home Assistant** (recommended). One Home Assistant - config-flow entry creates `calendar.school`. It is Home-Assistant-local, never - touches Google, and is entirely additive. The service then uses - `SCHOOL_HA_CALENDAR_ENTITY=calendar.school`. -2. **Use `calendar.samuelrolfe_googlemail_com`.** No Home Assistant change, but - every school event also appears in Sam's personal Google Calendar. +Both are handled, and both were caught by the fail-loudly design rather than +silently writing nothing: -The question is posted on AUD-110. The code needs only the configuration line -changed once the answer is in. +- **The Local Calendar rejects `return_response`.** `calendar.create_event` is + called with the service response to learn the event id; the Local Calendar + answers `Service does not support responses`. The client retries the call + without it. Home Assistant then replies with the changed states, a JSON array, + which the client ignores. +- **The Local Calendar reports no event id.** With no id to store, the item keeps + the description marker (`[compass-nhs:]`) as its reference, so the + item leaves the candidate set and is never written twice. The live retry + proved it: after a failed first attempt the two events already in the calendar + were adopted by their markers, and nothing was duplicated. ## Where the code lives @@ -141,8 +152,12 @@ make test # go test ./... ``` The tests cover the Home Assistant client against a fake Home Assistant (create, -read-back, error mapping), the sync run (write once, adopt by marker, dry run, -loud failure, refuse-to-write-blind), the store (candidates, the push log) and -the web surface (the action, the CSRF guard, the failure banner). +read-back, error mapping, the no-response fallback, the `start`/`end` shapes), +the sync run (write once, adopt by marker, dry run, loud failure, +refuse-to-write-blind, no-event-id idempotency), the store (candidates, the push +log), the parser (a model cannot label an event) and the web surface (the +action, the CSRF guard, the failure banner). -Not tested here: the live Home Assistant write. That needs the entity decision. +Live proof on `.13` (2026-10-15): two Compass school events were written to +`calendar.school`, read back through the Home Assistant API, and the dashboard +and item screen show the calendar state. diff --git a/internal/calendar/ha.go b/internal/calendar/ha.go index a96abc7..b55604d 100644 --- a/internal/calendar/ha.go +++ b/internal/calendar/ha.go @@ -166,6 +166,11 @@ type Event struct { // Assistant event id when the service reports one. It asks for the service // response, so the id can be stored on the item and a re-run cannot duplicate // the event. +// +// Not every calendar answers with a response: the Local Calendar integration +// rejects `return_response` outright. That is not a write failure, so the call +// is retried without it and the empty id is reported to the caller, which then +// falls back to the description marker for idempotency. func (c *Client) CreateEvent(ctx context.Context, ev Event) (string, error) { if strings.TrimSpace(ev.Summary) == "" { return "", errors.New("calendar: event summary is empty") @@ -186,13 +191,29 @@ func (c *Client) CreateEvent(ctx context.Context, ev Event) (string, error) { body["location"] = ev.Location } - resp, err := c.callService(ctx, "calendar", "create_event", body) + resp, err := c.callService(ctx, "calendar", "create_event", body, true) if err != nil { + if noResponseSupport(err) { + if _, rerr := c.callService(ctx, "calendar", "create_event", body, false); rerr != nil { + return "", rerr + } + return "", nil + } return "", err } return serviceUID(resp, c.entity), nil } +// noResponseSupport reports whether Home Assistant refused the request only +// because this calendar does not answer with a service response. +func noResponseSupport(err error) bool { + var haErr *Error + if !errors.As(err, &haErr) { + return false + } + return strings.Contains(strings.ToLower(haErr.Message), "does not support responses") +} + // DeleteEvent removes an event by its Home Assistant uid. It exists so a test // event can be cleaned up, and so a wrong write can be undone. func (c *Client) DeleteEvent(ctx context.Context, uid string) error { @@ -202,7 +223,7 @@ func (c *Client) DeleteEvent(ctx context.Context, uid string) error { _, err := c.callService(ctx, "calendar", "delete_event", map[string]any{ "entity_id": c.entity, "uid": uid, - }) + }, false) return err } @@ -214,7 +235,7 @@ func (c *Client) Events(ctx context.Context, start, end time.Time) ([]Event, err "entity_id": c.entity, "start_date_time": start.Format(time.RFC3339), "end_date_time": end.Format(time.RFC3339), - }) + }, true) if err != nil { return nil, err } @@ -223,12 +244,20 @@ func (c *Client) Events(ctx context.Context, start, end time.Time) ([]Event, err // ---------- transport ---------- -// callService posts to /api/services// with return_response, -// so a service that reports data (the created event id) answers with it. -func (c *Client) callService(ctx context.Context, domain, service string, body map[string]any) (map[string]any, error) { - path := fmt.Sprintf("/api/services/%s/%s?return_response", domain, service) +// callService posts to /api/services//. When withResponse is +// true it adds ?return_response, so a service that reports data (the created +// event id) answers with it. Without a response Home Assistant replies with the +// changed states, a JSON array, which the caller does not need. +func (c *Client) callService(ctx context.Context, domain, service string, body map[string]any, withResponse bool) (map[string]any, error) { + path := fmt.Sprintf("/api/services/%s/%s", domain, service) + if !withResponse { + if err := c.do(ctx, http.MethodPost, path, body, nil); err != nil { + return nil, err + } + return nil, nil + } var out map[string]any - if err := c.do(ctx, http.MethodPost, path, body, &out); err != nil { + if err := c.do(ctx, http.MethodPost, path+"?return_response", body, &out); err != nil { return nil, err } return out, nil @@ -372,13 +401,19 @@ func servicePayloads(resp map[string]any, entity string) []map[string]any { return out } +// parseHATime reads an event time from a calendar.get_events result. Home +// Assistant reports `start` / `end` as either a date-time with an offset or a +// bare date for an all-day event; the *_date_time / *_date shapes are also +// accepted for older replies. func parseHATime(m map[string]any, prefix string) (*time.Time, string) { - if raw := str(m[prefix+"_date_time"]); raw != "" { + for _, key := range []string{prefix + "_date_time", prefix + "_date", prefix} { + raw := str(m[key]) + if raw == "" { + continue + } if t, err := time.Parse(time.RFC3339, raw); err == nil { return &t, raw } - } - if raw := str(m[prefix+"_date"]); raw != "" { if t, err := time.ParseInLocation("2006-01-02", raw, time.Local); err == nil { return &t, raw } diff --git a/internal/calendar/ha_test.go b/internal/calendar/ha_test.go index 8d58ce3..c6e4e75 100644 --- a/internal/calendar/ha_test.go +++ b/internal/calendar/ha_test.go @@ -18,15 +18,16 @@ import ( type fakeHA struct { *httptest.Server - mu sync.Mutex - entity string - events []map[string]any - createCalls int - getCalls int - failCreate bool - returnUID bool - lastCreate map[string]any - lastGet map[string]any + mu sync.Mutex + entity string + events []map[string]any + createCalls int + getCalls int + failCreate bool + returnUID bool + rejectReturnResponse bool + lastCreate map[string]any + lastGet map[string]any } func newFakeHA(t *testing.T, entity string) *fakeHA { @@ -57,6 +58,14 @@ func newFakeHA(t *testing.T, entity string) *fakeHA { if !f.authOK(w, r) { return } + if f.rejectReturnResponse && r.URL.Query().Has("return_response") { + w.WriteHeader(http.StatusBadRequest) + writeJSON(w, map[string]any{"error": map[string]any{ + "code": "unknown_error", + "message": "Service does not support responses. Remove return_response from request.", + }}) + return + } var body map[string]any if err := json.NewDecoder(r.Body).Decode(&body); err != nil { http.Error(w, "bad json", http.StatusBadRequest) @@ -76,12 +85,17 @@ func newFakeHA(t *testing.T, entity string) *fakeHA { uid = "uid-" + str(body["summary"]) } f.events = append(f.events, map[string]any{ - "uid": uid, - "summary": body["summary"], - "description": body["description"], - "start_date_time": body["start_date_time"], - "end_date_time": body["end_date_time"], + "uid": uid, + "summary": body["summary"], + "description": body["description"], + "start": body["start_date_time"], + "end": body["end_date_time"], }) + if !r.URL.Query().Has("return_response") { + // Home Assistant replies with the changed states, a JSON array. + writeJSON(w, []map[string]any{}) + return + } writeJSON(w, map[string]any{ "changed_entities": map[string]any{}, "service_response": map[string]any{f.entity: map[string]any{"uid": uid}}, @@ -309,3 +323,52 @@ func TestFindMarker(t *testing.T) { } } } + +// TestCreateEventFallsBackWhenTheCalendarHasNoResponses proves the Local +// Calendar integration is usable: it rejects `return_response`, which is not a +// write failure. +func TestCreateEventFallsBackWhenTheCalendarHasNoResponses(t *testing.T) { + ha := newFakeHA(t, "calendar.school") + ha.rejectReturnResponse = true + c := testClient(t, ha.URL, "calendar.school") + + start := time.Date(2026, 10, 15, 18, 0, 0, 0, time.UTC) + uid, err := c.CreateEvent(context.Background(), Event{ + Summary: "Concert", Start: &start, End: ptrTime(start.Add(time.Hour)), + Description: "source: " + Marker(29), + }) + if err != nil { + t.Fatalf("CreateEvent: %v", err) + } + if uid != "" { + t.Errorf("want no uid, got %q", uid) + } + if ha.created() != 1 { + t.Fatalf("event was not written (%d create calls)", ha.created()) + } +} + +// TestParseHATime covers the shapes calendar.get_events reports. +func TestParseHATime(t *testing.T) { + timed := map[string]any{"start": "2026-10-15T15:05:00+11:00"} + got, raw := parseHATime(timed, "start") + if got == nil || raw != "2026-10-15T15:05:00+11:00" { + t.Fatalf("timed = %v %q", got, raw) + } + if got.Hour() != 15 || got.Minute() != 5 { + t.Errorf("timed = %v", got) + } + + allDay := map[string]any{"start": "2026-10-15"} + got, raw = parseHATime(allDay, "start") + if got == nil || raw != "2026-10-15" { + t.Fatalf("all-day = %v %q", got, raw) + } + if y, m, d := got.Date(); y != 2026 || m != time.October || d != 15 { + t.Errorf("all-day = %v", got) + } + + if got, raw := parseHATime(map[string]any{}, "start"); got != nil || raw != "" { + t.Errorf("missing = %v %q", got, raw) + } +} diff --git a/internal/calendar/sync.go b/internal/calendar/sync.go index 96612c3..5c6429f 100644 --- a/internal/calendar/sync.go +++ b/internal/calendar/sync.go @@ -192,17 +192,25 @@ func Sync(ctx context.Context, st *store.Store, c *Client, opts Options, log *sl continue } if uid == "" { - // Home Assistant did not report an event id. The marker in the - // description still makes the next run idempotent, but say so. - log.Warn("home assistant returned no event id; relying on the description marker", + // Home Assistant did not report an event id (the Local Calendar does + // not answer with one). Use the description marker as the item's + // reference, so the item leaves the candidate set and is never + // written twice. + log.Warn("home assistant returned no event id; using the description marker as the item reference", "item", first.ID, "child", children, "title", first.Title, "entity", c.Entity()) } for _, it := range g { - if err := st.MarkItemCalendarSynced(ctx, it.ID, uid, now); err != nil { + ref := uid + if ref == "" { + ref = Marker(it.ID) + } + if err := st.MarkItemCalendarSynced(ctx, it.ID, ref, now); err != nil { log.Error("mark calendar item", "item", it.ID, "err", err) } + if _, err := st.RecordCalendarPush(ctx, it.ID, c.Entity(), ref, true, ""); err != nil { + log.Error("record calendar push", "item", it.ID, "err", err) + } } - recordPush(ctx, st, c.Entity(), g, uid, true, "", log) res.Created++ created.UID = uid res.Events = append(res.Events, created) diff --git a/internal/calendar/sync_test.go b/internal/calendar/sync_test.go index 45a5ff6..fc82fae 100644 --- a/internal/calendar/sync_test.go +++ b/internal/calendar/sync_test.go @@ -106,10 +106,10 @@ func TestSyncAdoptsAnEventByItsMarker(t *testing.T) { // The event exists in Home Assistant, but the item has no id (a crashed // run, or a Home Assistant that reports no uid). The marker adopts it. ha.events = []map[string]any{{ - "uid": "uid-existing", - "summary": "Harry — Year 11 Assembly - HALL - CMA", - "description": "source: " + Marker(item.ID), - "start_date_time": due.Format(time.RFC3339), + "uid": "uid-existing", + "summary": "Harry — Year 11 Assembly - HALL - CMA", + "description": "source: " + Marker(item.ID), + "start": due.Format(time.RFC3339), }} c := testClient(t, ha.URL, "calendar.school") @@ -290,3 +290,43 @@ func TestSyncWritesASchoolWideEventOnce(t *testing.T) { } } } + +// TestSyncWithoutEventIDsStillNeverDuplicates proves the fallback path: when +// Home Assistant reports no event id, the item keeps the description marker as +// its reference, so a re-run writes nothing. +func TestSyncWithoutEventIDsStillNeverDuplicates(t *testing.T) { + ctx := context.Background() + st, childID := testStore(t) + due := time.Now().Add(4 * time.Hour).UTC().Truncate(time.Minute) + item := seedEvent(t, st, childID, "Year 11 Assembly - HALL", due, "high", "assembly") + + ha := newFakeHA(t, "calendar.school") + ha.returnUID = false + c := testClient(t, ha.URL, "calendar.school") + + first, err := Sync(ctx, st, c, Options{Now: time.Now()}, silentLogger()) + if err != nil { + t.Fatalf("first Sync: %v", err) + } + if first.Created != 1 || ha.created() != 1 { + t.Fatalf("first result = %+v (create calls %d)", first, ha.created()) + } + ci, err := st.CalendarItemByID(ctx, item.ID) + if err != nil || ci == nil { + t.Fatalf("item: %v %v", ci, err) + } + if ci.HAEventUID != Marker(item.ID) { + t.Errorf("reference = %q, want the description marker %q", ci.HAEventUID, Marker(item.ID)) + } + + second, err := Sync(ctx, st, c, Options{Now: time.Now()}, silentLogger()) + if err != nil { + t.Fatalf("second Sync: %v", err) + } + if second.Candidates != 0 || second.Created != 0 { + t.Fatalf("second result = %+v", second) + } + if ha.created() != 1 { + t.Fatalf("re-run duplicated the event (%d create calls)", ha.created()) + } +} diff --git a/internal/parse/importance.go b/internal/parse/importance.go index af67afb..d80ff1e 100644 --- a/internal/parse/importance.go +++ b/internal/parse/importance.go @@ -80,10 +80,17 @@ func eventIsSchoolEvent(title string) bool { // fails, loudly, on a model error or an unparseable answer; it never silently // leaves a wrong label. Items are updated in place. now lets the model judge // whether a due date is overdue. +// +// Calendar events keep the deterministic rule. A model must not decide whether +// a class period is a "school event": the calendar write depends on that call +// (FR-8), and a wrong label there either spams the family calendar with every +// lesson or silently drops the notice they needed. func ClassifyModel(ctx context.Context, c Completer, items []Item, model string, now time.Time) error { if c == nil { return fmt.Errorf("classify: no model client configured") } + ClassifyDeterministic(items, now) + const system = "You classify school items for a parent's morning brief. " + "Reply with JSON only. No prose, no code fences." @@ -107,6 +114,9 @@ func ClassifyModel(ctx context.Context, c Completer, items []Item, model string, if i >= len(batch) { return fmt.Errorf("classify: items %d-%d: model returned %d labels for %d items", start, end-1, len(labels), len(batch)) } + if batch[i].Type == TypeEvent { + continue // events keep the deterministic rule + } switch label { case importanceHigh, importanceNormal, importanceLow: batch[i].Importance = label diff --git a/internal/parse/parse_test.go b/internal/parse/parse_test.go index 55817dc..a2dec56 100644 --- a/internal/parse/parse_test.go +++ b/internal/parse/parse_test.go @@ -302,6 +302,35 @@ func TestClassifyDeterministic(t *testing.T) { } } +// TestClassifyModelLeavesEventsDeterministic proves a model cannot label a +// school event: the calendar write depends on that call (FR-8). +func TestClassifyModelLeavesEventsDeterministic(t *testing.T) { + at := time.Date(2026, 10, 15, 18, 0, 0, 0, time.UTC) + items := []Item{ + {Type: TypeEvent, Title: "End of Year Senior Music Concert - HALL - KSH", DueAt: &at}, + {Type: TypeEvent, Title: "5 - 11HLEGB5 - D8 - LHT", DueAt: &at}, + {Type: TypeTask, Title: "Essay", DueAt: &at}, + } + reply, _ := json.Marshal([]map[string]any{ + {"i": 0, "importance": "low"}, + {"i": 1, "importance": "high"}, + {"i": 2, "importance": "low"}, + }) + c := &stubCompleter{reply: string(reply)} + if err := ClassifyModel(context.Background(), c, items, "test-model", at); err != nil { + t.Fatalf("ClassifyModel: %v", err) + } + if items[0].Importance != importanceHigh { + t.Errorf("school event = %q, want %q", items[0].Importance, importanceHigh) + } + if items[1].Importance != importanceNormal { + t.Errorf("class period = %q, want %q", items[1].Importance, importanceNormal) + } + if items[2].Importance != importanceLow { + t.Errorf("task = %q, want the model's %q", items[2].Importance, importanceLow) + } +} + func ptr(t time.Time) *time.Time { return &t } // TestClassifyDeterministicEvents proves the Phase 6 importance rule: a school diff --git a/scripts/screenshot-phase5.mjs b/scripts/screenshot-phase5.mjs index c72fe71..a15b2c9 100644 --- a/scripts/screenshot-phase5.mjs +++ b/scripts/screenshot-phase5.mjs @@ -97,14 +97,25 @@ try { } } - // 4. The scan banner on its own, cropped to the top of the dashboard. + // 4. The scan banner on its own, cropped to the top of the dashboard. The + // class is shared with the calendar banner, so select by the labelled + // section, not by `.scan` alone. await page.goto(`${BASE}/`, { waitUntil: "networkidle" }); - await page.waitForSelector(".scan", { timeout: 20000 }); + const scanBanner = page.locator('.scan[aria-labelledby="scan-title"]'); + await scanBanner.waitFor({ timeout: 20000 }); const bannerFile = path.join(OUT, "scan-banner.png"); - await page.locator(".scan").screenshot({ path: bannerFile }); + await scanBanner.screenshot({ path: bannerFile }); shots.push(bannerFile); console.log(`shot ${bannerFile}`); + // 5. The calendar banner (Phase 6, FR-8 / FR-12). + const calendarBanner = page.locator('.scan[aria-labelledby="calendar-title"]'); + await calendarBanner.waitFor({ timeout: 20000 }); + const calendarFile = path.join(OUT, "calendar-banner.png"); + await calendarBanner.screenshot({ path: calendarFile }); + shots.push(calendarFile); + console.log(`shot ${calendarFile}`); + await sleep(100); console.log(`done: ${shots.length} screenshot(s) in ${OUT}`); } finally {