One resolver decides what the calendar shows (#93) #94

Merged
claude-bot merged 2 commits from fix/resolved-day-endpoint into main 2026-08-31 23:48:05 +00:00
Contributor

Fixes #93. Prerequisite for #48 items 1–2 (conflict icon and filter).

The problem

buildDayMap re-implemented priority resolution client-side and had drifted from priority_resolver.get_events_for_date — the one that decides what actually reaches the controller:

backend calendar
Date pins applied first never fetched
User-event shadowing parent excluded ignored
Equal-priority tiebreak source rank array order

Measured on the real dev database: 2026 has 43 days where the top priority is tied, and on each the displayed primary could differ from the pushed one. Worse, since pins were never fetched, "Promote Secondary" appeared to do nothing — the backend honoured it, the calendar kept showing the old primary. That is precisely the symptom #43 fixed, reintroduced one layer up.

The fix

Resolution happens once, on the server.

  • resolve_active(active, pinned_event_id) -> (primary, secondaries, conflict) is pure and shared. get_events_for_date delegates to it; a new resolve_year walks a year through the same function, loading events and pins once instead of querying per day.
  • GET /api/v1/events/resolved?year= returns {date, primary, secondaries, conflict, pinned}.
  • buildDayMap becomes a join of those days with schemes. It no longer resolves anything.

Rather than encode the tiebreak a third time, there is now one implementation and a test asserting resolve_year agrees with get_events_for_date day for day.

What conflict means

Tied at the top — two or more events share the highest (priority, source_rank), so the winner was an arbitrary tiebreak and a human should decide.

A month-long backdrop losing to a higher-priority day event is deliberately not a conflict; spec §2.4 calls that the normal backdrop behaviour. On real data that distinction is 43 actionable days versus 91 — flagging a third of the year would make the upcoming filter useless.

conflict is reported independently of pinned, so a settled day still records that an ambiguity existed; the UI decides whether to keep warning.

Route ordering

/events/resolved is declared before /events/{event_id}. Otherwise FastAPI matches "resolved" as an event id and the endpoint 404s forever. There is a regression test asserting 200, not just a comment.

Verification

Backend, locally:

ruff / ruff format / mypy (61 files)   clean
pytest                                 266 passed  (was 253)

Frontend, in a node:22 container matching CI — I'd flagged earlier that I couldn't check this, and now can:

tsc -b --noEmit    exit 0
eslint             exit 0  (--max-warnings 0)
vitest             40 passed  (was 37)
npm run build      succeeded

13 backend tests and 6 frontend tests added, including the conflict cases that must not flag.

Two things worth knowing

The three buildDayMap tests asserting client-side resolution are gone, replaced by tests of the join. That behaviour is the backend's now and is covered there — keeping them would have re-asserted the bug.

tsc -b passed even with the old call signature still in the tests, because it does not include the test files; vitest caught it. That is a gap in its own right and probably worth a follow-up.

Fixes #93. Prerequisite for #48 items 1–2 (conflict icon and filter). ## The problem `buildDayMap` re-implemented priority resolution client-side and had drifted from `priority_resolver.get_events_for_date` — the one that decides what actually reaches the controller: | | backend | calendar | |---|---|---| | Date pins | applied first | **never fetched** | | User-event shadowing | parent excluded | ignored | | Equal-priority tiebreak | source rank | array order | Measured on the real dev database: 2026 has **43 days where the top priority is tied**, and on each the displayed primary could differ from the pushed one. Worse, since pins were never fetched, **"Promote Secondary" appeared to do nothing** — the backend honoured it, the calendar kept showing the old primary. That is precisely the symptom #43 fixed, reintroduced one layer up. ## The fix Resolution happens **once**, on the server. - `resolve_active(active, pinned_event_id) -> (primary, secondaries, conflict)` is pure and shared. `get_events_for_date` delegates to it; a new `resolve_year` walks a year through the same function, loading events and pins once instead of querying per day. - `GET /api/v1/events/resolved?year=` returns `{date, primary, secondaries, conflict, pinned}`. - `buildDayMap` becomes a **join** of those days with schemes. It no longer resolves anything. Rather than encode the tiebreak a third time, there is now one implementation and a test asserting `resolve_year` agrees with `get_events_for_date` day for day. ## What `conflict` means **Tied at the top** — two or more events share the highest `(priority, source_rank)`, so the winner was an arbitrary tiebreak and a human should decide. A month-long backdrop losing to a higher-priority day event is deliberately **not** a conflict; spec §2.4 calls that the normal backdrop behaviour. On real data that distinction is **43 actionable days versus 91** — flagging a third of the year would make the upcoming filter useless. `conflict` is reported independently of `pinned`, so a settled day still records that an ambiguity existed; the UI decides whether to keep warning. ## Route ordering `/events/resolved` is declared **before** `/events/{event_id}`. Otherwise FastAPI matches `"resolved"` as an event id and the endpoint 404s forever. There is a **regression test asserting 200**, not just a comment. ## Verification Backend, locally: ``` ruff / ruff format / mypy (61 files) clean pytest 266 passed (was 253) ``` Frontend, in a `node:22` container matching CI — I'd flagged earlier that I couldn't check this, and now can: ``` tsc -b --noEmit exit 0 eslint exit 0 (--max-warnings 0) vitest 40 passed (was 37) npm run build succeeded ``` 13 backend tests and 6 frontend tests added, including the conflict cases that must **not** flag. ## Two things worth knowing **The three `buildDayMap` tests asserting client-side resolution are gone**, replaced by tests of the join. That behaviour is the backend's now and is covered there — keeping them would have re-asserted the bug. **`tsc -b` passed even with the old call signature still in the tests**, because it does not include the test files; vitest caught it. That is a gap in its own right and probably worth a follow-up.
The calendar re-implemented priority resolution in buildDayMap and drifted from
the resolver that decides what actually reaches the controller: it never fetched
date pins, ignored user-event shadowing, and broke ties by array order instead
of source rank. Against the real dev database that is 43 days in 2026 where the
displayed primary can differ from the pushed one.

Rather than encode the tiebreak a third time, the resolution is extracted into a
pure resolve_active(active, pinned_event_id) -> (primary, secondaries, conflict)
that both paths call. get_events_for_date now delegates to it, and a new
resolve_year walks a whole year through the same function, loading events and
pins once instead of querying per day.

New GET /api/v1/events/resolved?year= returns
{date, primary, secondaries, conflict, pinned} per day.

conflict means two or more events tie on (priority, source_rank) at the TOP --
the winner was chosen by an arbitrary tiebreak, so a human should decide. A
month-long backdrop losing to a higher-priority day event is deliberately NOT a
conflict: spec 2.4 calls that normal. On real data that is the difference
between 43 actionable days and 91 noisy ones, a third of the year.

conflict is reported independently of the pin, so a settled day still shows that
an ambiguity existed; the UI decides whether to keep warning.

Route ordering matters here: /events/resolved is declared BEFORE
/events/{event_id}, or FastAPI matches "resolved" as an event id and the
endpoint 404s forever. There is a regression test asserting a 200, not just a
comment.

13 new tests: conflict semantics including the backdrop and lower-tie cases that
must NOT flag, resolve_year agreeing with get_events_for_date day for day, pins,
and the endpoint shape. Full gate green: ruff, format, mypy on 61 files, 266
tests passing.

Refs #93, #48.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Frontend: render the backend's resolved days instead of re-resolving (#93)
All checks were successful
CI / Alembic migration check (pull_request) Successful in 37s
CI / Python lint & type-check (pull_request) Successful in 1m7s
CI / Frontend lint, test & build (pull_request) Successful in 1m14s
CI / Python tests (pull_request) Successful in 1m58s
CI / Docker build, health smoke & E2E (pull_request) Successful in 5m33s
9a82428329
buildDayMap no longer resolves priority. It now joins GET /events/resolved with
the schemes that render each day, so the calendar shows exactly what the push
path will act on.

What the client used to get wrong, silently:

  - date pins were never fetched at all, so "Promote Secondary" appeared to do
    nothing even though the backend honoured it (the symptom #43 fixed, one
    layer up)
  - user-event shadowing was ignored, so a shadowed parent could still display
  - equal priorities were broken by array order, not source rank -- 43 days in
    2026 on the real dev database

DayInfo gains conflict and pinned, carried straight through from the server, so
the day-cell icon and conflicts filter (#48 items 1-2) can be built on the same
answer rather than a fourth reimplementation.

The three buildDayMap tests that asserted client-side resolution are replaced:
resolution is the backend's job and is covered by test_priority_resolver.py.
What is tested here is the join -- scheme matching by date, days with no
primary, secondaries and the new flags passing through, and that no day is
invented that the backend did not return.

Verified locally in a node:22 container matching CI, not deferred:

  tsc -b --noEmit   exit 0
  eslint            exit 0 (--max-warnings 0)
  vitest            40 passed, up from 37
  npm run build     succeeded

Worth noting: tsc passed even with the old call signature still in the tests,
because `tsc -b` does not include the test files. vitest caught it. That is a
gap in its own right.

Refs #93, #48.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
claude-bot deleted branch fix/resolved-day-endpoint 2026-08-31 23:48:07 +00:00
Sign in to join this conversation.
No description provided.