From 86ba2ccc1dd7103a4ac71597f0bb496e6f47e7d1 Mon Sep 17 00:00:00 2001 From: Lucas Winther Date: Fri, 28 Aug 2026 05:25:37 +0200 Subject: [PATCH] Make a stored id always name a row MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found the new settings row opening nothing for a rule whose `until` precedes its `startsAt`. That parses — nothing ties the two together — and yields no occurrence at all, so nextOccurrences and the anchor fallback both come back empty, nearestOccurrence returns null, and openRow cannot resolve a bare rule id. It was the undeletable record this index exists to rescue, now with a button that lies about it. So resolution moves out of App into displayEventFor, with the rule itself as the floor: a stored id always names something the reader can edit and delete, whatever the rule does or does not generate. Exported because nothing here can click and no test renders App, which is exactly how a dead button shipped — the chain is now covered without a DOM. Two of the caption tests could not fail, proven by mutation rather than read: deleting the whole "starts" branch and removing the null-end guard both left the suite green, because `not.toContain("ended")` was never watching the branch at risk. They assert what the caption says now. And a repeating series that has stopped said only how often it repeats — in the one place whose job is explaining why an event is on no other surface. A healthy cadence explains nothing; it says when it stopped. Co-Authored-By: Claude Opus 5 (1M context) --- docs/PRD.md | 6 ++-- src/client/App.tsx | 21 +++++------ src/client/components/YourOwn.tsx | 18 ++++++++-- src/shared/custom.ts | 46 +++++++++++++++++++++++- test/custom-ui.test.tsx | 36 ++++++++++++++----- test/custom.test.ts | 58 +++++++++++++++++++++++++++++++ 6 files changed, 157 insertions(+), 28 deletions(-) diff --git a/docs/PRD.md b/docs/PRD.md index 71017e8..60faa1b 100644 --- a/docs/PRD.md +++ b/docs/PRD.md @@ -317,8 +317,10 @@ Four constraints, each protecting something that already exists: - **Their events never touch the ingest pipeline.** `sanitize.ts` and `merge.ts` exist for pages we do not control; a reader's own typing is neither untrusted markup nor a second opinion to reconcile. Nothing they enter is fetched, parsed, merged, scored or quarantined. -- **They are always reachable.** Settings lists every event the reader has made, whatever state it - is in, and opens the same detail sheet a row does. Every other surface drops an event once it has +- **They are reachable whenever the app is.** Settings lists every event the reader has made, + whatever state it is in, and opens the same detail sheet a row does. "Whenever the app is" is + literal: settings renders on the ready path, so a feed that fails to load takes the whole panel + with it — a pre-existing limit worth naming rather than implying otherwise. Every other surface drops an event once it has ended, so without this list a one-off of their own became unreachable the day it finished — impossible to edit, and impossible to delete out of a store nothing else can see. Events filed under a game we track are listed too; the form allows that, so the index has to. diff --git a/src/client/App.tsx b/src/client/App.tsx index 2ff9fea..354476a 100644 --- a/src/client/App.tsx +++ b/src/client/App.tsx @@ -30,12 +30,12 @@ import { } from "./state/lens.ts"; import { clockFor, formatRemaining } from "../shared/time.ts"; import { dailySummary, isDaily, resolveDaily } from "../shared/daily.ts"; -import { nearestOccurrence, occurrenceForId, strandedOccurrences } from "../shared/recurrence.ts"; +import { strandedOccurrences } from "../shared/recurrence.ts"; import { orderGames } from "./state/gameOrder.ts"; import { GameMetaProvider, type MetaResolver } from "./state/gameMeta.tsx"; import { metaOnTheme, useTheme } from "./state/theme.ts"; import { - asOccurrenceEvent, + displayEventFor, recordFor, type CustomEvents, type CustomGames, @@ -369,11 +369,8 @@ export function App() { const openRow = (() => { const hit = allRows.find((r) => r.event.id === openId) ?? null; if (hit !== null || openId === null) return hit; - const record = recordFor(custom.events, openId); - if (record === undefined) return null; - const occurrence = occurrenceForId(record, openId); - if (occurrence === null) return null; - const event = asOccurrenceEvent(record, occurrence); + const event = displayEventFor(custom.events, openId, now); + if (event === null) return null; return { event, clock: clockFor(event, prefs.region, now) }; })(); @@ -654,12 +651,10 @@ export function App() { // The index lists rules; the sheet opens rows. A repeating rule's // own id is never a row — the lists hold its occurrences — so it is // resolved to whichever occurrence is nearest before opening. - onOpen: (id) => { - const record = custom.events[id]; - const occurrence = - record === undefined ? null : nearestOccurrence(record, now); - setOpenId(occurrence === null ? id : occurrence.id); - }, + // The stored id is enough: `displayEventFor` resolves a rule to + // whichever occurrence the sheet can show, and to the rule itself + // when it has none. + onOpen: setOpenId, }} onExport={() => exportProgress(prog.progress, daily.logs, ignored.marks, prefs, { diff --git a/src/client/components/YourOwn.tsx b/src/client/components/YourOwn.tsx index d6016d6..25470de 100644 --- a/src/client/components/YourOwn.tsx +++ b/src/client/components/YourOwn.tsx @@ -29,7 +29,16 @@ import { cadenceLabel, EventForm, GameForm } from "./CustomForms.tsx"; */ export function eventCaption(event: CustomEvent, nowMs: number): string { const cadence = cadenceLabel(event.repeat); - if (cadence !== null) return cadence; + if (cadence !== null) { + // A series that has stopped is exactly what this list exists to explain, + // and a healthy-looking cadence explains nothing: the reader would see + // "on a weekly cycle" and no reason it is missing from the board. + const until = event.repeat?.until ?? null; + if (until !== null && Date.parse(until) < nowMs) { + return `stopped ${formatAbsolute(Date.parse(until), false)}`; + } + return cadence; + } if (event.endsAt !== null && Date.parse(event.endsAt) < nowMs) { return `ended ${formatAbsolute(Date.parse(event.endsAt), false)}`; @@ -78,7 +87,10 @@ export function YourOwn({ // An event may be filed under a game we track — a source can miss one — and // those have no row above to nest under. Listing them separately is what // keeps this index complete: an ended event under Genshin is on no other - // surface either, and would be just as stuck. + // surface either, and would be just as stuck. It also catches an event whose + // own lane has since gone, which `removeGame` refuses to cause but an import + // can still deliver — hence "another game" rather than "a game we track", + // which would be false for exactly that row. const underTracked = Object.values(events).filter( (e) => games[e.game] === undefined, ); @@ -186,7 +198,7 @@ export function YourOwn({ {/* Its own heading rather than trailing the list above, which read as though these belonged to whichever game happened to be last. */} {underTracked.length > 0 && ( -

Filed under a game we track

+

Filed under another game

)} {underTracked.length > 0 && (