Resolve any occurrence back to its rule, and count what a repeat adds strands
The lists only ever build allRows from a rule's first two occurrences (LIST_OCCURRENCES), but the timeline draws every occurrence the board window admits. Clicking a bar past the second set openId to an id allRows could not resolve, so the detail sheet silently failed to open and per-occurrence completion was unreachable for those bars. occurrenceForId parses the day an occurrence id names and regenerates just that neighbourhood via occurrencesOf, so App.tsx can resolve through the rule when the direct lookup misses. Separately, strandedBy's `if (record.repeat === null) return 0` hid the warning on exactly the transition where it matters most: a reader marks a plain event done, then edits it to add a repeat, and the save re-keys every row from `id` to `id#<date>` with no notice that the mark under the bare id is about to become unreachable. strandedOccurrences checks the record's own id when it has no repeat yet, and its next occurrences once it does. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
14b221a627
commit
b0523a7cb9
+28
-15
@@ -30,11 +30,12 @@ import {
|
||||
} from "./state/lens.ts";
|
||||
import { clockFor, formatRemaining } from "../shared/time.ts";
|
||||
import { dailySummary, isDaily, resolveDaily } from "../shared/daily.ts";
|
||||
import { nextOccurrences } from "../shared/recurrence.ts";
|
||||
import { occurrenceForId, 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,
|
||||
recordFor,
|
||||
type CustomEvents,
|
||||
type CustomGames,
|
||||
@@ -357,7 +358,24 @@ export function App() {
|
||||
);
|
||||
const perGame = useMemo(() => countByGame(scopedTodo), [scopedTodo]);
|
||||
|
||||
const openRow = allRows.find((r) => r.event.id === openId) ?? null;
|
||||
/**
|
||||
* The lists only ever build `allRows` from a rule's first two occurrences
|
||||
* (`LIST_OCCURRENCES`), but the timeline draws every occurrence the board
|
||||
* window admits — so a bar past the second names an id `allRows` cannot
|
||||
* resolve. Resolved through the rule behind it rather than dropped, or every
|
||||
* later bar would be a dead click with no way to reach per-occurrence
|
||||
* completion.
|
||||
*/
|
||||
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);
|
||||
return { event, clock: clockFor(event, prefs.region, now) };
|
||||
})();
|
||||
|
||||
/** One row, wired up. Both lists render the same thing from the same props. */
|
||||
const renderRow = (row: RowEvent) => (
|
||||
@@ -706,19 +724,14 @@ export function App() {
|
||||
onSave: (_id: string, draft: EventDraft) =>
|
||||
custom.editEvent(record.id, draft),
|
||||
onDelete: () => custom.removeEvent(record.id),
|
||||
strandedBy: () => {
|
||||
// What the reader has actually recorded against the occurrences
|
||||
// this rule generates today, and would no longer reach once the
|
||||
// ids move. Twelve is a season of a fortnightly rule — enough to
|
||||
// make the number meaningful without walking a decade of a
|
||||
// daily one.
|
||||
if (record.repeat === null) return 0;
|
||||
return nextOccurrences(record, now, 12).filter(
|
||||
(o) =>
|
||||
prog.progress[o.id] !== undefined ||
|
||||
(daily.logs[o.id]?.days.length ?? 0) > 0,
|
||||
).length;
|
||||
},
|
||||
strandedBy: () =>
|
||||
strandedOccurrences(
|
||||
record,
|
||||
now,
|
||||
(id) =>
|
||||
prog.progress[id] !== undefined ||
|
||||
(daily.logs[id]?.days.length ?? 0) > 0,
|
||||
),
|
||||
};
|
||||
})()}
|
||||
/>
|
||||
|
||||
@@ -277,6 +277,49 @@ export function occurrencesOf(
|
||||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
* The one occurrence a row's id names, or null.
|
||||
*
|
||||
* The lists and the timeline's own click-to-open lookup only ever hold the
|
||||
* first two occurrences of a rule (`LIST_OCCURRENCES`), but the timeline
|
||||
* draws every occurrence the board window admits. A bar past the second is an
|
||||
* id nothing else can resolve, so this is the other half: parse the local day
|
||||
* the id names, bracket it with a window one interval wide on each side, and
|
||||
* let `occurrencesOf` regenerate just that neighbourhood — cheap, and no
|
||||
* caller has to walk a whole series to open one sheet.
|
||||
*
|
||||
* One interval each way is enough because `occurrencesOf` admits an
|
||||
* occurrence whose *window* overlaps the range, not only one that *starts*
|
||||
* inside it — an unstated end runs a full interval past its own start, so the
|
||||
* occurrence naming a given day can start up to one interval before or after
|
||||
* that day and still be the one the id points at.
|
||||
*/
|
||||
export function occurrenceForId(rule: RepeatingEvent, id: string): Occurrence | null {
|
||||
if (rule.repeat === null) return null;
|
||||
const sepAt = id.indexOf(OCCURRENCE_SEP);
|
||||
if (sepAt === -1) return null;
|
||||
|
||||
const suffix = id.slice(sepAt + 1);
|
||||
const m = /^(\d{4})-(\d{2})-(\d{2})$/.exec(suffix);
|
||||
if (m === null) return null;
|
||||
const [, y, mo, d] = m as unknown as [string, string, string, string];
|
||||
const day = new Date(Number(y), Number(mo) - 1, Number(d));
|
||||
// Same silent-rollover hazard `readerInstant` refuses on the way in: "30
|
||||
// February" parses to a real Date, just not the one the id claims to name.
|
||||
if (
|
||||
day.getFullYear() !== Number(y) ||
|
||||
day.getMonth() !== Number(mo) - 1 ||
|
||||
day.getDate() !== Number(d)
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const dayStartMs = day.getTime();
|
||||
const spanMs = addUnits(dayStartMs, rule.repeat.unit, rule.repeat.interval) - dayStartMs;
|
||||
const window = occurrencesOf(rule, dayStartMs - spanMs, dayStartMs + spanMs);
|
||||
return window.find((o) => o.id === id) ?? null;
|
||||
}
|
||||
|
||||
/**
|
||||
* The next `count` occurrences that have not finished, oldest first.
|
||||
*
|
||||
@@ -300,3 +343,28 @@ export function nextOccurrences(
|
||||
);
|
||||
return occurrencesOf(event, nowMs, horizon, count);
|
||||
}
|
||||
|
||||
/**
|
||||
* How many of a rule's ids the reader has actually recorded something
|
||||
* against — what a schedule edit that re-keys ids would strand.
|
||||
*
|
||||
* **A plain event has exactly one id at risk: its own.** That is the
|
||||
* transition that matters most, and the one a `repeat === null` short-circuit
|
||||
* used to hide entirely: a reader who has already marked a plain event done
|
||||
* or ticked days against it, then edits it to add a repeat, moves every row
|
||||
* from `id` to `id#<date>` on save — orphaning marks the form never warned
|
||||
* about. A rule already repeating checks its next `count` occurrences
|
||||
* instead, since those are the ids the same edit would re-key.
|
||||
*/
|
||||
export function strandedOccurrences(
|
||||
record: RepeatingEvent,
|
||||
nowMs: number,
|
||||
hasMark: (id: string) => boolean,
|
||||
count = 12,
|
||||
): number {
|
||||
const ids =
|
||||
record.repeat === null
|
||||
? [record.id]
|
||||
: nextOccurrences(record, nowMs, count).map((o) => o.id);
|
||||
return ids.filter(hasMark).length;
|
||||
}
|
||||
|
||||
+90
-1
@@ -1,5 +1,5 @@
|
||||
import { describe, expect, test } from "bun:test";
|
||||
import { addUnits, comesRoundEarly, Repeat, isOccurrenceId, occurrenceId, ruleIdOf, movesOccurrences, nextOccurrences, occurrencesOf, type RepeatingEvent } from "../src/shared/recurrence.ts";
|
||||
import { addUnits, comesRoundEarly, Repeat, isOccurrenceId, occurrenceId, occurrenceForId, ruleIdOf, movesOccurrences, nextOccurrences, occurrencesOf, strandedOccurrences, type RepeatingEvent } from "../src/shared/recurrence.ts";
|
||||
import { CustomEventId, isCustomEventId } from "../src/shared/custom.ts";
|
||||
|
||||
// Pinned so the DST cases mean something. Copenhagen is UTC+1 in winter and
|
||||
@@ -324,6 +324,95 @@ describe("occurrencesOf", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("occurrenceForId", () => {
|
||||
function rule(over: Partial<RepeatingEvent> = {}): RepeatingEvent {
|
||||
return {
|
||||
id: "myevent:k3f9qa2m01",
|
||||
startsAt: new Date("2026-09-01T09:00:00").toISOString(),
|
||||
startPrecision: "exact",
|
||||
endsAt: new Date("2026-09-08T09:00:00").toISOString(),
|
||||
endPrecision: "exact",
|
||||
repeat: { unit: "weeks", interval: 2, until: null },
|
||||
...over,
|
||||
};
|
||||
}
|
||||
|
||||
test("an id off a real occurrence round-trips to that occurrence", () => {
|
||||
const got = occurrenceForId(rule(), "myevent:k3f9qa2m01#2026-09-15");
|
||||
expect(got?.id).toBe("myevent:k3f9qa2m01#2026-09-15");
|
||||
expect(got?.startsAt).toBe(new Date("2026-09-15T09:00:00").toISOString());
|
||||
});
|
||||
|
||||
test("this is exactly what click-to-open needs beyond the list's first two",
|
||||
() => {
|
||||
// LIST_OCCURRENCES only ever carries the first two; the third is exactly
|
||||
// the case a timeline bar can name but `allRows.find` cannot resolve.
|
||||
const got = occurrenceForId(rule(), "myevent:k3f9qa2m01#2026-09-29");
|
||||
expect(got?.id).toBe("myevent:k3f9qa2m01#2026-09-29");
|
||||
});
|
||||
|
||||
test("a bogus suffix returns null", () => {
|
||||
expect(occurrenceForId(rule(), "myevent:k3f9qa2m01#not-a-date")).toBeNull();
|
||||
expect(occurrenceForId(rule(), "myevent:k3f9qa2m01#2026-02-30")).toBeNull();
|
||||
expect(occurrenceForId(rule(), "myevent:k3f9qa2m01")).toBeNull();
|
||||
});
|
||||
|
||||
test("a suffix that names no occurrence of this rule returns null", () => {
|
||||
// 2026-09-08 falls between two fortnightly openings (1st and 15th) and
|
||||
// matches none of them.
|
||||
expect(occurrenceForId(rule(), "myevent:k3f9qa2m01#2026-09-08")).toBeNull();
|
||||
// A day inside a running occurrence's window but not that occurrence's
|
||||
// own start day — ids are keyed by start day only.
|
||||
expect(occurrenceForId(rule(), "myevent:k3f9qa2m01#2026-09-04")).toBeNull();
|
||||
});
|
||||
|
||||
test("a non-repeating rule has no occurrences to resolve", () => {
|
||||
expect(occurrenceForId(rule({ repeat: null }), "myevent:k3f9qa2m01#2026-09-01")).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("strandedOccurrences", () => {
|
||||
function rule(over: Partial<RepeatingEvent> = {}): RepeatingEvent {
|
||||
return {
|
||||
id: "myevent:k3f9qa2m01",
|
||||
startsAt: new Date("2026-09-01T09:00:00").toISOString(),
|
||||
startPrecision: "exact",
|
||||
endsAt: new Date("2026-09-08T09:00:00").toISOString(),
|
||||
endPrecision: "exact",
|
||||
repeat: { unit: "weeks", interval: 2, until: null },
|
||||
...over,
|
||||
};
|
||||
}
|
||||
|
||||
test("a plain event checks its own bare id, not the empty occurrence list", () => {
|
||||
// This is the transition that matters most: a reader marks a plain event
|
||||
// done, then edits it to add a repeat. `record.repeat === null` used to
|
||||
// short-circuit straight to 0 here, hiding the warning on exactly the
|
||||
// save that re-keys every row out from under the mark.
|
||||
const plain = rule({ repeat: null });
|
||||
expect(strandedOccurrences(plain, Date.now(), (id) => id === "myevent:k3f9qa2m01")).toBe(1);
|
||||
expect(strandedOccurrences(plain, Date.now(), () => false)).toBe(0);
|
||||
});
|
||||
|
||||
test("a repeating rule checks its next occurrences, not its own bare id", () => {
|
||||
const now = new Date("2026-09-03T12:00:00").getTime();
|
||||
const marked = new Set(["myevent:k3f9qa2m01#2026-09-15"]);
|
||||
expect(strandedOccurrences(rule(), now, (id) => marked.has(id))).toBe(1);
|
||||
// The bare id is never at risk once a rule repeats — nothing is stored
|
||||
// under it.
|
||||
expect(strandedOccurrences(rule(), now, (id) => id === "myevent:k3f9qa2m01")).toBe(0);
|
||||
});
|
||||
|
||||
test("counts every marked id among the next `count` occurrences", () => {
|
||||
const now = new Date("2026-09-03T12:00:00").getTime();
|
||||
const marked = new Set([
|
||||
"myevent:k3f9qa2m01#2026-09-15",
|
||||
"myevent:k3f9qa2m01#2026-09-29",
|
||||
]);
|
||||
expect(strandedOccurrences(rule(), now, (id) => marked.has(id), 3)).toBe(2);
|
||||
});
|
||||
});
|
||||
|
||||
describe("nextOccurrences", () => {
|
||||
function rule(over: Partial<RepeatingEvent> = {}): RepeatingEvent {
|
||||
return {
|
||||
|
||||
Reference in New Issue
Block a user