From 1301d0c0000f2e5ae9ecd7dd599b8e8eb3607181 Mon Sep 17 00:00:00 2001 From: Eason WaveKat Date: Mon, 10 Aug 2026 21:17:27 +1200 Subject: [PATCH] feat(book): enumerate a vocabulary on any grid MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `required_assets` is computed from the grid compiled into each device, so a renderer that knows only its own can serve only devices that agree — which makes narrowing wait on the whole fleet updating, an event that does not occur. Adds an explicit grid to both twins (`bookVocabularyRefs(node, { granularityMins })`, `vocabulary_refs_on`) so a renderer can cover the union of every grid still installed and stop being pinned to its oldest device's opinion. Defaults are untouched: every existing caller, including the arming check itself, gets exactly what it got before. See the platform's docs/36. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018stjkZJ4kxDViq1ENnVDmH --- crates/wavekat-flow/src/book.rs | 115 ++++++++++++++++++++++--- packages/flow-schema/src/book.ts | 59 ++++++++++--- packages/flow-schema/test/book.test.ts | 66 ++++++++++++++ 3 files changed, 216 insertions(+), 24 deletions(-) diff --git a/crates/wavekat-flow/src/book.rs b/crates/wavekat-flow/src/book.rs index 6df95e8..58d5a2e 100644 --- a/crates/wavekat-flow/src/book.rs +++ b/crates/wavekat-flow/src/book.rs @@ -85,12 +85,17 @@ pub const MAX_BOOK_OFFERS: u64 = 5; /// the server offers a time the vocabulary has no clip for, the caller /// hears silence where the time should be. /// -/// **This constant is why the change is staged.** [`required_assets`] -/// is computed from the value compiled into *this* build, not from -/// anything a version carries — so a daemon still on the quarter hour, -/// handed a version published after this moved, asks for `bktime_0915`, -/// does not find it, and refuses to arm the flow at all. Platforms -/// narrow what they offer first; this follows once the fleet has it. +/// **Narrowing this is not a free change.** [`required_assets`] is +/// computed from the value compiled into *this* build, not from anything +/// a version carries — so a daemon still on the quarter hour, handed a +/// version published against a narrower grid, asks for `bktime_0915`, +/// does not find it, and refuses to arm the flow at all. +/// +/// That does not make the change wait on the fleet, which never fully +/// updates. A renderer covers the devices it must serve by rendering the +/// union of every grid still installed; [`vocabulary_refs_on`] is how it +/// enumerates the members it no longer compiles in. See the platform's +/// docs/36. /// /// [`required_assets`]: crate::model_ext::required_assets pub const BOOK_GRANULARITY_MINS: u64 = 30; @@ -207,21 +212,33 @@ fn minutes_of(hhmm: &str) -> Option { /// instants and so may drop a candidate this keeps on a daylight-saving /// boundary. That direction is safe: the vocabulary may be a superset (a /// clip nobody plays), never a subset (a time nobody can say). -fn starts_in_range(range: &TimeRange, duration_mins: u64) -> Vec { +/// The same walk on a grid this build does not necessarily use. +/// +/// A device decides whether it can run a flow by computing the required +/// set from the grid compiled into *it*, so a renderer that knows only +/// its own can serve only devices that agree. Rendering the union of +/// every grid still installed is what removes that coupling, and this is +/// how the other members are enumerated. See the platform's docs/36. +fn starts_in_range_on(range: &TimeRange, duration_mins: u64, granularity_mins: u64) -> Vec { + // Stepping by zero would never terminate — on a device, that is a + // hang rather than a wrong answer. + if granularity_mins == 0 { + return Vec::new(); + } let (Some(open), Some(close)) = (minutes_of(&range.open), minutes_of(&range.close)) else { return Vec::new(); }; if close <= open { return Vec::new(); } - let first = open.div_ceil(BOOK_GRANULARITY_MINS) * BOOK_GRANULARITY_MINS; + let first = open.div_ceil(granularity_mins) * granularity_mins; let mut starts = Vec::new(); let mut mins = first; while mins < close { if mins + duration_mins <= close { starts.push(mins); } - mins += BOOK_GRANULARITY_MINS; + mins += granularity_mins; } starts } @@ -236,6 +253,19 @@ fn starts_in_range(range: &TimeRange, duration_mins: u64) -> Vec { /// (when it is played). Empty for every other kind of node, so a caller /// can map it over a whole flow. pub fn vocabulary_refs(node: &Node) -> Vec { + vocabulary_refs_on(node, BOOK_GRANULARITY_MINS) +} + +/// The same set on a grid this build does not necessarily use. +/// +/// Twin of `bookVocabularyRefs(node, { granularityMins })`. Only a +/// *renderer* has a reason to call this: it has to satisfy devices whose +/// compiled-in grid differs from its own, and covering the union of the +/// grids still installed is what stops a narrowing change from waiting on +/// a fleet that never fully updates. A device answering "can I run this +/// flow?" uses [`vocabulary_refs`] and its own constant, which is the +/// question it is actually being asked. See the platform's docs/36. +pub fn vocabulary_refs_on(node: &Node, granularity_mins: u64) -> Vec { let Node::Book { schedule, exceptions, @@ -283,7 +313,7 @@ pub fn vocabulary_refs(node: &Node) -> Vec { } for range in ranges { - for start in starts_in_range(range, *duration) { + for start in starts_in_range_on(range, *duration, granularity_mins) { refs.insert(time_ref(start)); } } @@ -336,9 +366,66 @@ mod tests { // First grid point at or after 09:10 is 09:30 — never 09:10, and // no longer 09:15; the last start that still finishes by 10:30 // with a 30-minute appointment is 10:00. - assert_eq!(starts_in_range(&range, 30), vec![570, 600]); + assert_eq!( + starts_in_range_on(&range, 30, BOOK_GRANULARITY_MINS), + vec![570, 600] + ); // An appointment longer than the window produces nothing at all. - assert!(starts_in_range(&range, 120).is_empty()); + assert!(starts_in_range_on(&range, 120, BOOK_GRANULARITY_MINS).is_empty()); + } + + // The twin of `bookVocabularyRefs(node, { granularityMins })`. + // + // A device answers "can I run this flow?" from the grid compiled into + // it, so a renderer that only knows its own can serve only devices + // that agree — which makes narrowing wait on the whole fleet + // updating, an event that does not occur. This is how a renderer + // enumerates the grids it no longer compiles in, so it can cover the + // union. See the platform's docs/36. + #[test] + fn an_explicit_grid_is_walked_instead_of_this_builds() { + let range = TimeRange { + open: "09:00".into(), + close: "11:00".into(), + }; + // Quarter hours, from a build whose own constant says thirty. + assert_eq!( + starts_in_range_on(&range, 30, 15), + vec![540, 555, 570, 585, 600, 615, 630] + ); + // This build's own grid, for contrast — the halves of the same + // window, and what every existing caller keeps getting. + assert_eq!( + starts_in_range_on(&range, 30, BOOK_GRANULARITY_MINS), + vec![540, 570, 600, 630] + ); + } + + #[test] + fn a_finer_grid_is_a_superset_of_a_coarser_one() { + // The property the union rests on: widening never drops a ref, so + // a device on the finer grid finds everything it computes inside + // what a renderer covering both froze. + let range = TimeRange { + open: "09:00".into(), + close: "17:00".into(), + }; + let fine = starts_in_range_on(&range, 30, 15); + for start in starts_in_range_on(&range, 30, 30) { + assert!(fine.contains(&start), "{start} missing from the finer grid"); + } + } + + #[test] + fn a_zero_grid_yields_nothing_rather_than_spinning() { + // Unreachable through `vocabulary_refs`, which passes a constant. + // Asserted anyway because the loop steps by this value, and the + // failure would be a hung device rather than a wrong answer. + let range = TimeRange { + open: "09:00".into(), + close: "17:00".into(), + }; + assert!(starts_in_range_on(&range, 30, 0).is_empty()); } #[test] @@ -347,12 +434,12 @@ mod tests { open: "17:00".into(), close: "09:00".into(), }; - assert!(starts_in_range(&backwards, 30).is_empty()); + assert!(starts_in_range_on(&backwards, 30, BOOK_GRANULARITY_MINS).is_empty()); let nonsense = TimeRange { open: "nine".into(), close: "five".into(), }; - assert!(starts_in_range(&nonsense, 30).is_empty()); + assert!(starts_in_range_on(&nonsense, 30, BOOK_GRANULARITY_MINS).is_empty()); } #[test] diff --git a/packages/flow-schema/src/book.ts b/packages/flow-schema/src/book.ts index cd8d20b..ef48bc3 100644 --- a/packages/flow-schema/src/book.ts +++ b/packages/flow-schema/src/book.ts @@ -94,11 +94,15 @@ export const MAX_BOOK_OFFERS = 5; * **Narrowing this is not a free change**, and the direction matters. * A daemon computes `requiredAssets` from *its own* copy of this * constant, not from anything the version carries — so a device still on - * the quarter hour, handed a version published after this change, asks - * for `bktime_0915`, does not find it, and refuses to arm the flow at - * all. The safe order is: platforms narrow what they *offer* first - * (leaving the frozen set a superset), fleets update, and only then does - * this move. See the platform's docs/35 §2. + * the quarter hour, handed a version published against a narrower one, + * asks for `bktime_0915`, does not find it, and refuses to arm the flow + * at all. + * + * That does **not** make a change here wait on the fleet, which never + * fully updates. A renderer covers the devices it has to serve by + * rendering the union of every grid still installed — + * {@link BookVocabularyOptions.granularityMins} is how it enumerates the + * members it no longer compiles in. See the platform's docs/36. */ export const BOOK_GRANULARITY_MINS = 30; @@ -217,19 +221,44 @@ function minutesOf(hhmm: string): number | null { * allowed to be a superset (a clip nobody plays), never a subset (a time * nobody can say). */ -function startsInRange(range: TimeRange, durationMins: number): number[] { +function startsInRange( + range: TimeRange, + durationMins: number, + granularityMins: number, +): number[] { const open = minutesOf(range.open); const close = minutesOf(range.close); if (open === null || close === null || close <= open) return []; - const first = Math.ceil(open / BOOK_GRANULARITY_MINS) * BOOK_GRANULARITY_MINS; + const first = Math.ceil(open / granularityMins) * granularityMins; const starts: number[] = []; - for (let mins = first; mins < close; mins += BOOK_GRANULARITY_MINS) { + for (let mins = first; mins < close; mins += granularityMins) { if (mins + durationMins <= close) starts.push(mins); } return starts; } +/** Options for {@link bookVocabularyRefs}. */ +export interface BookVocabularyOptions { + /** + * Walk this grid instead of {@link BOOK_GRANULARITY_MINS}. + * + * Exists for one caller: a renderer that has to satisfy devices which + * do not share its own constant. A device decides whether it can run a + * flow by computing this set from the grid compiled into *it*, so a + * renderer that only knows its own can only ever serve devices that + * agree — which makes narrowing the grid wait on the whole fleet + * updating, an event that does not occur. Rendering the union of every + * grid still installed removes the wait; this is how you enumerate the + * other members. See the platform's docs/36. + * + * Not for deciding what to *offer* a caller. That is the build's own + * constant, and disagreeing with it means offering a time no clip + * exists for. + */ + granularityMins?: number; +} + /** * Every asset ref a `book` node needs in order to speak: the nine day * phrases, one clip per bookable time of day, one "press N" per offer it @@ -240,7 +269,15 @@ function startsInRange(range: TimeRange, durationMins: number): number[] { * the same at publish (when it is rendered) and on a call months later * (when it is played). */ -export function bookVocabularyRefs(node: BookNode): string[] { +export function bookVocabularyRefs(node: BookNode, options: BookVocabularyOptions = {}): string[] { + const granularityMins = options.granularityMins ?? BOOK_GRANULARITY_MINS; + // A zero would not terminate and a fraction would name minutes no ref + // format can spell. Both are the caller's bug; falling back to the + // default would hide it behind a set that looks plausible. + if (!Number.isInteger(granularityMins) || granularityMins < 1) { + throw new Error(`granularityMins must be a positive whole number of minutes, got ${granularityMins}`); + } + const refs = new Set(); for (const day of BOOK_DAY_KEYS) refs.add(bookDayRef(day)); @@ -274,7 +311,9 @@ export function bookVocabularyRefs(node: BookNode): string[] { } for (const range of ranges) { - for (const start of startsInRange(range, duration)) refs.add(bookTimeRef(start)); + for (const start of startsInRange(range, duration, granularityMins)) { + refs.add(bookTimeRef(start)); + } } return [...refs].sort(); diff --git a/packages/flow-schema/test/book.test.ts b/packages/flow-schema/test/book.test.ts index 2f2bcbe..97f8079 100644 --- a/packages/flow-schema/test/book.test.ts +++ b/packages/flow-schema/test/book.test.ts @@ -10,6 +10,7 @@ import { describe, expect, it } from 'vitest'; import { + BOOK_GRANULARITY_MINS, BOOK_TAKEN_REF, bookVocabularyRefs, parseBookVocabularyRef, @@ -148,6 +149,71 @@ describe('bookVocabularyRefs', () => { }); }); +// Asking for a grid other than this build's own. +// +// A device decides whether it can run a flow by computing this set from +// the constant compiled into *it*, so a renderer that only knows its own +// grid can only ever satisfy devices that agree with it. The parameter +// exists so the platform can render the union of every grid still +// installed and stop being pinned to its oldest device's opinion — see +// the platform's docs/36. The default is what every existing caller, +// including that arming check, keeps getting. +describe('bookVocabularyRefs — an explicit grid', () => { + const timesOn = (node: BookNode, granularityMins: number): string[] => + bookVocabularyRefs(node, { granularityMins }).filter( + (ref) => parseBookVocabularyRef(ref)?.kind === 'time', + ); + + it("walks the grid it was given, not this build's", () => { + expect(timesOn(bookNode(), 15)).toEqual([ + 'bktime_0900', + 'bktime_0915', + 'bktime_0930', + 'bktime_0945', + 'bktime_1000', + 'bktime_1015', + 'bktime_1030', + ]); + }); + + it("reproduces the default exactly when handed this build's own grid", () => { + const node = bookNode({ schedule: { tue: [{ open: '09:10', close: '16:40' }] } }); + expect(bookVocabularyRefs(node, { granularityMins: BOOK_GRANULARITY_MINS })).toEqual( + bookVocabularyRefs(node), + ); + }); + + it('leaves the non-time refs alone', () => { + // Day phrases, keypad clips and the taken line have nothing to do + // with the grid; a caller unioning two grids must not find them + // duplicated or dropped. + const coarse = bookVocabularyRefs(bookNode(), { granularityMins: 60 }); + for (const ref of ['bkday_mon', 'bkpress_1', BOOK_TAKEN_REF]) { + expect(coarse).toContain(ref); + } + }); + + it('makes a finer grid a superset of a coarser one', () => { + // The property the union rests on: widening never loses a ref, so a + // device on the finer grid finds everything it computes inside what + // a renderer covering both froze. + const node = bookNode({ schedule: { mon: [{ open: '09:00', close: '17:00' }] } }); + const fine = new Set(bookVocabularyRefs(node, { granularityMins: 15 })); + for (const ref of bookVocabularyRefs(node, { granularityMins: 30 })) { + expect(fine).toContain(ref); + } + }); + + it('refuses a grid that is not a positive whole number of minutes', () => { + // A zero would not terminate and a fraction would produce refs no + // renderer can name; both are a caller's bug, and silently falling + // back to the default would hide it. + for (const bad of [0, -30, 7.5, Number.NaN]) { + expect(() => bookVocabularyRefs(bookNode(), { granularityMins: bad })).toThrow(); + } + }); +}); + describe('parseBookVocabularyRef', () => { it('reads a ref back into what it says', () => { expect(parseBookVocabularyRef('bktime_0930')).toEqual({ kind: 'time', hour: 9, minute: 30 });