diff --git a/apps/streamdeck/README.md b/apps/streamdeck/README.md index f9bac824f..0e1db308c 100644 --- a/apps/streamdeck/README.md +++ b/apps/streamdeck/README.md @@ -96,12 +96,12 @@ the selection, the dials review it.** │ STREAM DECK + │ │ │ │ ┌────────┐ ┌────────┐ ┌────────┐ ┌────────┐ │ -│ │ Builder│ │ Builder│ │ Builder│ │ Builder│ Row 1: │ -│ │ slot 1 │ │ slot 2 │ │ slot 3 │ │ slot 4 │ selectors │ +│ │OpenArch│ │ Builder│ │ Builder│ │ Builder│ Row 1: │ +│ │ (main) │ │ (1st) │ │ (2nd) │ │ (3rd) │ selectors │ │ └────────┘ └────────┘ └────────┘ └────────┘ │ │ ┌────────┐ ┌────────┐ ┌────────┐ ┌────────┐ │ -│ │Approve │ │ Dev │ │Send Fb │ │ Bldr │ Row 2: │ -│ │ Gate │ │ Server │ │ (N) │ │ Term │ palette │ +│ │ │ │Approve │ │OpenArch│ │ Bldr │ Row 2: │ +│ │ (free) │ │ Gate │ │ (bldr) │ │ Term │ palette │ │ └────────┘ └────────┘ └────────┘ └────────┘ │ │ ┌──────────────────────────────────────────────┐ │ │ │ touch strip: each dial's title + live detail │ │ @@ -112,15 +112,27 @@ the selection, the dials review it.** └──────────────────────────────────────────────────────────┘ ``` -- **Row 1 — fleet selectors.** Four **Builder Action** keys, one per slot (1–4). - They are a **4-wide window** onto the fleet: with more than four builders the - **Select dial** (Zoom Navigator rotate) scrolls the window to builders 5–8, 9–N, - and the slot holding the current selection is accented. Press selects the builder +- **Row 1 — selectors, anchored by the architect.** Slot 1 is **Open Architect + Terminal** pinned to **Main** mode: a fixed anchor at the left of the board that + opens the workspace's `main` architect. It can sit in Row 1 without breaking the + "Row 1 selects" invariant precisely because Main mode is **selection-independent** — + it always targets `main`, so it neither reads nor moves the shared selection. Slots + 2–4 are **Builder Action** selectors — here **three** of them. + The selectors are a **window** onto the fleet whose width is **exactly the number of + Builder Action keys you place** (three here), not a fixed count: the keys self-order + by physical position (left to right, top row first), so there are no slot numbers to + set, and a Mini or an XL sizes itself the same way. With more builders than keys the + **Select dial** (Zoom Navigator rotate) scrolls the window a page at a time, and the + slot holding the current selection is accented. Because the window follows the placed + keys, a builder is never selectable while shown on no key. Press selects the builder (Row 2 + the dials follow) and opens its phase artifact. - **Row 2 — action palette**, fixed in place, always acting on the **selected** - builder: **Approve Gate · Dev Server · Send Feedback (N) · Open Builder - Terminal**. Its sibling **Open Architect Terminal** opens the owning architect - instead — place it where a slot frees up (e.g. Send Feedback in forward mode). + builder: slot 1 is **free** (drop any key here — e.g. Dev Server or Send Feedback), + then **Approve Gate · Open Architect Terminal (builder mode) · Open Builder + Terminal**. The Row-2 architect key is pinned to **builder** mode, so it opens the + **selected builder's owning architect** — the per-builder complement to Row 1's + `main`-mode anchor. Between the two keys, both `main` and whoever spawned the current + builder are one press away. Nothing is fixed — drag whatever you want onto each slot in the Stream Deck app. The 5th encoder, **Spawn from Backlog**, can swap onto a dial in place of any of @@ -130,13 +142,16 @@ the four above (e.g. replace PR Nav when you are triaging the backlog). ### Keys -- **Builder Action** (Row 1) — a live tile for a builder **slot**, but as a 4-wide - **window** onto the fleet, not a fixed index: slot N shows the Nth builder on the - current page, and the **Select dial** scrolls the page so a fleet larger than four - is fully reachable. It shows the builder's issue + phase, accents the slot holding - the selection, and on press selects the builder (Row 2 + the dials follow) and - opens its phase artifact. The default press verb is **Automatic** (the current - phase's spec / plan / diff); pick a fixed verb in the PI to always run that. +- **Builder Action** (Row 1) — a live tile for a builder **slot**, as a **window** + onto the fleet whose width is the number of these keys you placed (not a fixed + index): the key's slot is its position among them (reading order, row then column), + so the Nth key shows the Nth builder on the current page, and the **Select dial** + scrolls the page so a fleet larger than the window is fully reachable. Because the + window matches the placed keys, a builder is never selectable while shown on no key. + It shows the builder's issue + phase, accents the slot holding the selection, and on + press selects the builder (Row 2 + the dials follow) and opens its phase artifact. + The default press verb is **Automatic** (the current phase's spec / plan / diff); + pick a fixed verb in the PI to always run that. - **Approve Gate** (Row 2) — the **single** approve affordance. Acts on the **selected** builder: the face shows its pending gate (e.g. `Plan · Approve`), and press surfaces that gate's **approval modal in the focused VSCode window** for you @@ -164,12 +179,11 @@ the four above (e.g. replace PR Nav when you are triaging the backlog). in Main mode when `main` isn't live, VSCode opens the first architect while the face still reads `Main` (the mode reflects your configured intent); and a live architect registration behind a dead terminal opens a session nobody reads (the - deck can't detect it). Placement caveat: the Main-mode key is selection- - independent, so it can live on a **Row 1** key without affecting selection — but - Row 1's window is a fixed page of four (independent of how many Builder Action - keys you place), so freeing a Row 1 key leaves three builder slots and hides - every fourth builder past a three-builder fleet. Recommended for small working - sets; self-sizing the Row 1 window is tracked separately (#1465). + deck can't detect it). Placement: the Main-mode key is selection-independent, so + it can live on a **Row 1** key without affecting selection — and because Row 1's + window sizes itself to the Builder Action keys you actually place (#1465), giving a + Row 1 key to this one simply leaves a correctly-sized three-wide builder window, + with no hidden builders. That is the recommended layout above (Row 1 slot 1). - **Codev Action** — fires a workspace verb. Choose it in the Property Inspector (Open Architect/Builder Terminal, View Diff, Send Message, Spawn Builder, Refresh Overview). Defaults to Refresh Overview. (The Open Architect Terminal diff --git a/apps/streamdeck/com.cluesmith.codev.sdPlugin/manifest.json b/apps/streamdeck/com.cluesmith.codev.sdPlugin/manifest.json index 5c0ebf330..5f79862ca 100644 --- a/apps/streamdeck/com.cluesmith.codev.sdPlugin/manifest.json +++ b/apps/streamdeck/com.cluesmith.codev.sdPlugin/manifest.json @@ -130,7 +130,7 @@ { "Name": "Builder Action", "UUID": "com.cluesmith.codev.builder-action", - "Tooltip": "Live tile for the Nth builder (set the slot in the property inspector). Shows its issue + phase; press selects the builder and opens the artifact for its current phase (spec / plan / diff), or a fixed verb you choose.", + "Tooltip": "Live tile for a builder — its slot is where you place the key (the selectors self-order left to right, top row first). Shows its issue + phase; press selects the builder and opens the artifact for its current phase (spec / plan / diff), or a fixed verb you choose.", "Icon": "icons/list/builder-action", "Controllers": [ "Keypad" diff --git a/apps/streamdeck/com.cluesmith.codev.sdPlugin/ui/builder-action.html b/apps/streamdeck/com.cluesmith.codev.sdPlugin/ui/builder-action.html index 29bf9b615..899477bf7 100644 --- a/apps/streamdeck/com.cluesmith.codev.sdPlugin/ui/builder-action.html +++ b/apps/streamdeck/com.cluesmith.codev.sdPlugin/ui/builder-action.html @@ -3,23 +3,11 @@ - - - - - - - - - - - - - @@ -32,10 +20,13 @@ - Pins this key to the Nth builder (slot 1 = first builder) and shows - its issue + phase. Press selects that builder. On Automatic, the press opens - the artifact for its current phase (spec / plan / diff), re-openable every - press; pick a fixed verb to always run that instead. + Each Builder Action key is one slot in a window onto the fleet, sized to + how many of these keys you place: the leftmost (top row first) shows the first + builder, the next shows the second, and so on, and the Select dial scrolls the + window when there are more builders than keys. Just place as many as you like — + no slot numbers to set. Press selects that builder; on Automatic it opens the + artifact for its current phase (spec / plan / diff), re-openable every press. + Pick a fixed verb to always run that instead. diff --git a/apps/streamdeck/src/__tests__/actions.test.ts b/apps/streamdeck/src/__tests__/actions.test.ts index 88ac5a0f0..c540c1e19 100644 --- a/apps/streamdeck/src/__tests__/actions.test.ts +++ b/apps/streamdeck/src/__tests__/actions.test.ts @@ -71,6 +71,28 @@ const dial = (ticks: number) => ({ payload: { ticks, settings: {} }, }); +/** A placed Builder Action key at board (row, column) — a defined `coordinates` marks it + * as an on-board key (not a multi-action instance), which #1465's windowing sorts by. */ +const bkey = (id: string, column: number, row = 0) => ({ + id, isKey: () => true, isDial: () => false, coordinates: { column, row }, + showAlert: vi.fn(), showOk: vi.fn(), setImage: vi.fn(), setTitle: vi.fn(), +}); +/** A Builder Action key inside a multi-action: the SDK reports `coordinates: undefined`, so + * #1465 excludes it from the window (no slot). */ +const multiKey = (id: string) => ({ + id, isKey: () => true, isDial: () => false, coordinates: undefined, + showAlert: vi.fn(), showOk: vi.fn(), setImage: vi.fn(), setTitle: vi.fn(), +}); +/** Place `n` builder keys left-to-right on row 0 (reading order) and return them. */ +function placeKeys(ba: BuilderAction, n: number) { + const keys = Array.from({ length: n }, (_, i) => bkey(`k${i}`, i)); + keys.forEach((k) => ba.onWillAppear({ action: k, payload: { settings: {} } } as never)); + return keys; +} +/** Press a placed key with optional PI settings (verb). */ +const pressKey = (ba: BuilderAction, action: unknown, settings: Record = {}) => + ba.onKeyDown({ action, payload: { settings } } as never); + describe('verb keypads', () => { let ctx: ReturnType; beforeEach(() => { ctx = makeStore(); }); @@ -100,54 +122,55 @@ describe('verb keypads', () => { }); it('BuilderAction defaults to Automatic — opens slot 1 builder’s current-phase artifact', async () => { - // pir-1 is blocked at plan-approval → Automatic resolves to open-plan. - await new BuilderAction(ctx.store).onKeyDown(keyEvent() as never); + // The first placed key is slot 0 → pir-1, blocked at plan-approval → Automatic = open-plan. + const ba = new BuilderAction(ctx.store); + const [k0] = placeKeys(ba, 1); + await pressKey(ba, k0); expect(ctx.sent[0]).toEqual({ verb: 'open-plan', args: ['pir-1'], ws: '/work/alpha' }); }); it('BuilderAction Automatic falls back to open-terminal for an unknown-state builder', async () => { ctx.store.overview = { builders: [{ id: 'pir-x', roleId: null, issueId: null, issueTitle: null, blocked: null, blockedGate: null, protocolPhase: '', progress: 0, worktreePath: '/w' }], pendingPRs: [], backlog: [], recentlyClosed: [] } as never; - await new BuilderAction(ctx.store).onKeyDown(keyEvent() as never); + const ba = new BuilderAction(ctx.store); + const [k0] = placeKeys(ba, 1); + await pressKey(ba, k0); expect(ctx.sent[0]).toEqual({ verb: 'open-terminal', args: ['pir-x'], ws: '/work/alpha' }); }); it('BuilderAction with an explicit verb fires it verbatim, ignoring phase', async () => { - await new BuilderAction(ctx.store).onKeyDown(keyEvent({ slot: '2', verb: 'open-terminal' }) as never); + // Two placed keys → the second (slot 1) is pir-2. + const ba = new BuilderAction(ctx.store); + const [, k1] = placeKeys(ba, 2); + await pressKey(ba, k1, { verb: 'open-terminal' }); expect(ctx.sent[0]).toEqual({ verb: 'open-terminal', args: ['pir-2'], ws: '/work/alpha' }); }); it('BuilderAction Automatic opens the FIRST file diff (dial-ready), not the aggregate, for a diff-phase builder (#1414)', async () => { - // pir-2 is in `implement` → the phase artifact is the diff; Automatic remaps it to + // pir-2 (slot 1) is in `implement` → the phase artifact is the diff; Automatic remaps it to // `open-diff-first` so the SD+ dials step from file 1, never `view-diff` (aggregate). - await new BuilderAction(ctx.store).onKeyDown(keyEvent({ slot: '2' }) as never); + const ba = new BuilderAction(ctx.store); + const [, k1] = placeKeys(ba, 2); + await pressKey(ba, k1); expect(ctx.sent[0]).toEqual({ verb: 'open-diff-first', args: ['pir-2'], ws: '/work/alpha' }); }); it('BuilderAction explicit "View Diff" still fires view-diff (aggregate) verbatim (#1414)', async () => { // The PI View Diff option is unchanged: only Automatic remaps to open-diff-first. - await new BuilderAction(ctx.store).onKeyDown(keyEvent({ slot: '2', verb: 'view-diff' }) as never); + const ba = new BuilderAction(ctx.store); + const [, k1] = placeKeys(ba, 2); + await pressKey(ba, k1, { verb: 'view-diff' }); expect(ctx.sent[0]).toEqual({ verb: 'view-diff', args: ['pir-2'], ws: '/work/alpha' }); }); it('BuilderAction press selects the slot builder (cursor follows)', async () => { - await new BuilderAction(ctx.store).onKeyDown(keyEvent({ slot: '2' }) as never); + const ba = new BuilderAction(ctx.store); + const [, k1] = placeKeys(ba, 2); + await pressKey(ba, k1); expect(ctx.store.selectedBuilder()?.id).toBe('pir-2'); }); }); describe('slot keys', () => { - it('a slot past the end of the builder list alerts and sends nothing', async () => { - const ctx = makeStore(); // only 2 builders - const ev = keyEvent({ slot: '8' }); - await new BuilderAction(ctx.store).onKeyDown(ev as never); - expect(ctx.sent).toHaveLength(0); - expect(ev.action.showAlert).toHaveBeenCalled(); - }); - - // One SingletonAction instance serves every key of its type — these guard the - // per-instance fix (without it, all keys collided on shared state). - const slotKey = (id: string) => ({ id, isKey: () => true, setImage: vi.fn(), setTitle: vi.fn() }); - // renderTo hands setImage a base64 data URI (Stream Deck drops raw SVG strings); decode to // assert on the underlying face. const decodeFace = (action: { setImage: { mock: { calls: unknown[][] } } }): string => { @@ -156,14 +179,31 @@ describe('slot keys', () => { return Buffer.from(arg.slice('data:image/svg+xml;base64,'.length), 'base64').toString('utf8'); }; + it('a slot past the end of the fleet alerts and sends nothing', async () => { + const ctx = makeStore(); // only 2 builders + const ba = new BuilderAction(ctx.store); + const [, , k2] = placeKeys(ba, 3); // 3 keys → slot 2 has no builder + await pressKey(ba, k2); + expect(ctx.sent).toHaveLength(0); + expect(k2.showAlert).toHaveBeenCalled(); + }); + + it('a multi-action key (no coordinates) has no slot — alerts and sends nothing', async () => { + const ctx = makeStore(); // 2 builders + const ba = new BuilderAction(ctx.store); + placeKeys(ba, 2); // two placed keys → window is 2 wide + const m = multiKey('M'); + ba.onWillAppear({ action: m, payload: { settings: {} } } as never); + await pressKey(ba, m); + expect(ctx.sent).toHaveLength(0); + expect(m.showAlert).toHaveBeenCalled(); + }); + it('renders each slot key against its own slot (different slots → different builders)', () => { const ctx = makeStore(); // pir-1 (#101), pir-2 (#102) const ba = new BuilderAction(ctx.store); - const a = slotKey('A'); - const b = slotKey('B'); - ba.onWillAppear({ action: a, payload: { settings: { slot: '1' } } } as never); - ba.onWillAppear({ action: b, payload: { settings: { slot: '2' } } } as never); - // The face is now a composite SVG handed to setImage (not a title). + const [a, b] = placeKeys(ba, 2); // a at column 0, b at column 1 + // The face is a composite SVG handed to setImage (not a title). expect(decodeFace(a)).toContain('#101'); expect(decodeFace(b)).toContain('#102'); }); @@ -171,10 +211,7 @@ describe('slot keys', () => { it('renders the builder’s state as a colour-coded, mapped face (mirrors the sidebar)', () => { const ctx = makeStore(); // pir-1 blocked plan-approval, pir-2 phase "implement" const ba = new BuilderAction(ctx.store); - const a = slotKey('A'); - const b = slotKey('B'); - ba.onWillAppear({ action: a, payload: { settings: { slot: '1' } } } as never); - ba.onWillAppear({ action: b, payload: { settings: { slot: '2' } } } as never); + const [a, b] = placeKeys(ba, 2); // Blocked at plan-approval → mapped label "Plan" in warning yellow (not the wire "plan review"). const aSvg = decodeFace(a); expect(aSvg).toContain('>Plan<'); @@ -185,21 +222,17 @@ describe('slot keys', () => { expect(bSvg).toContain('#73c991'); }); - it('renders the empty-slot face when no builder occupies the slot', () => { - const ctx = makeStore(); // only 2 builders → slot 5 is empty + it('renders the empty-slot face (labelled by position) when no builder occupies the slot', () => { + const ctx = makeStore(); // only 2 builders → the 3rd key's slot is empty const ba = new BuilderAction(ctx.store); - const a = slotKey('A'); - ba.onWillAppear({ action: a, payload: { settings: { slot: '5' } } } as never); - expect(decodeFace(a)).toContain('Slot 5'); + const [, , k2] = placeKeys(ba, 3); + expect(decodeFace(k2)).toContain('Slot 3'); // position 2 → 1-based label "Slot 3" }); it('re-renders every slot key on a store change (fixes stale-on-workspace-switch)', () => { const ctx = makeStore(); const ba = new BuilderAction(ctx.store); - const a = slotKey('A'); - const b = slotKey('B'); - ba.onWillAppear({ action: a, payload: { settings: { slot: '1' } } } as never); - ba.onWillAppear({ action: b, payload: { settings: { slot: '2' } } } as never); + const [a, b] = placeKeys(ba, 2); a.setImage.mockClear(); b.setImage.mockClear(); ctx.store.setLevel('builders'); // any store change → onChange → render all keys @@ -291,9 +324,10 @@ describe('OpenTerminalAction (Row 2 — per-builder, #1410)', () => { }); }); -describe('Row 1 windowing (#1410)', () => { - /** A store with `n` builders (ids builder-0..builder-(n-1)), selection at `cursor`. */ - function windowedStore(n: number, cursor: number) { +describe('Row 1 windowing (#1410, dynamic size #1465)', () => { + /** A store with `n` builders (ids b0..b(n-1)), selection at `cursor`, window `size` wide + * (the count of placed builder keys the `BuilderAction` singleton would report). */ + function windowedStore(n: number, cursor: number, size: number) { const ctx = makeStore(); ctx.store.overview = { builders: Array.from({ length: n }, (_, i) => ({ @@ -303,35 +337,176 @@ describe('Row 1 windowing (#1410)', () => { pendingPRs: [], backlog: [], recentlyClosed: [], } as never; ctx.store.cursor = { ...ctx.store.cursor, builder: cursor, level: 'builders' }; + ctx.store.setBuilderWindowSize(size); return ctx.store; } - it('slot i shows builder i on page 0 (first four)', () => { - const store = windowedStore(10, 0); + it('slot i shows builder i on page 0', () => { + const store = windowedStore(10, 0, 4); expect([0, 1, 2, 3].map((i) => store.windowedBuilder(i)?.id)).toEqual(['b0', 'b1', 'b2', 'b3']); }); - it('scrolls a page when the selection moves past the fourth builder', () => { - const store = windowedStore(10, 4); // selection on b4 → page 1 + it('scrolls a page (= the window width) when the selection moves past the window', () => { + const store = windowedStore(10, 4, 4); // selection on b4 → page 1 expect([0, 1, 2, 3].map((i) => store.windowedBuilder(i)?.id)).toEqual(['b4', 'b5', 'b6', 'b7']); }); it('trailing slots are empty on a partial last page', () => { - const store = windowedStore(10, 9); // selection on b9 → page 2 (b8, b9, -, -) + const store = windowedStore(10, 9, 4); // selection on b9 → page 2 (b8, b9, -, -) expect([0, 1, 2, 3].map((i) => store.windowedBuilder(i)?.id)).toEqual(['b8', 'b9', undefined, undefined]); }); + it('pages by the placed-key count, not a constant 4 — a 3-wide window puts b3 on slot 0', () => { + // The bug: with the old fixed 4, cursor on b3 gives windowStart 0, so b3 lands at slot 3 — + // a key that does not exist when only 3 are placed, hiding b3. Sized to 3, b3 is on slot 0. + const store = windowedStore(10, 3, 3); + expect([0, 1, 2].map((i) => store.windowedBuilder(i)?.id)).toEqual(['b3', 'b4', 'b5']); + }); + + it('INVARIANT: the selected builder is always on a rendered slot, for every cursor', () => { + // The core correctness guarantee. For each window size and each possible selection, the + // selected builder must resolve to some slot in [0, size) — never selectable-but-hidden. + for (const size of [3, 4]) { + for (const n of [3, 4, 5, 8, 11]) { + for (let cursor = 0; cursor < n; cursor++) { + const store = windowedStore(n, cursor, size); + const selectedId = store.selectedBuilder()?.id; + const shown = Array.from({ length: size }, (_, i) => store.windowedBuilder(i)?.id); + expect(shown).toContain(selectedId); + } + } + } + }); + it('BuilderAction renders the windowed builder and accents the selected slot', () => { - const store = windowedStore(10, 5); // page 1: slots show b4..b7; b5 is selected (slot 1) - const render = (slot: string) => { - const key = { id: `k${slot}`, isKey: () => true, setImage: vi.fn(), setTitle: vi.fn() }; - new BuilderAction(store).onWillAppear({ action: key, payload: { settings: { slot } } } as never); - return Buffer.from(String(key.setImage.mock.calls.at(-1)?.[0]).split(',')[1], 'base64').toString('utf8'); - }; - expect(render('1')).toContain('#104'); // slot 1 → b4 (issueId 104) - expect(render('2')).toContain('#105'); // slot 2 → b5 (selected) - expect(render('2')).toContain('stroke-width="3"'); // selected accent ring - expect(render('1')).not.toContain('stroke-width="3"'); // unselected slot has no ring + vi.useFakeTimers(); + try { + // 4 placed keys, selection on b5 (10 builders) → page 1 shows b4..b7; b5 is selected. + const store = windowedStore(10, 5, 4); + const ba = new BuilderAction(store); + const keys = Array.from({ length: 4 }, (_, i) => bkey(`k${i}`, i)); + keys.forEach((k) => ba.onWillAppear({ action: k, payload: { settings: {} } } as never)); + vi.advanceTimersByTime(60); // let the window settle so every slot reflects the final page + const face = (k: (typeof keys)[number]) => + Buffer.from(String(k.setImage.mock.calls.at(-1)?.[0]).split(',')[1], 'base64').toString('utf8'); + expect(face(keys[0])).toContain('#104'); // slot 0 → b4 (issueId 104) + expect(face(keys[1])).toContain('#105'); // slot 1 → b5 (selected) + expect(face(keys[1])).toContain('stroke-width="3"'); // selected accent ring + expect(face(keys[0])).not.toContain('stroke-width="3"'); // unselected slot has no ring + } finally { + vi.useRealTimers(); + } + }); +}); + +describe('Row 1 window recompute (#1465)', () => { + /** A store with `n` builders, selection at `cursor`. */ + function fleet(n: number, cursor: number) { + const ctx = makeStore(); + ctx.store.overview = { + builders: Array.from({ length: n }, (_, i) => ({ + id: `b${i}`, roleId: `builder-b${i}`, issueId: String(100 + i), issueTitle: `Task ${i}`, + blocked: null, blockedGate: null, protocolPhase: 'implement', progress: 0, worktreePath: `/w/b${i}`, + })), + pendingPRs: [], backlog: [], recentlyClosed: [], + } as never; + ctx.store.cursor = { ...ctx.store.cursor, builder: cursor, level: 'builders' }; + return ctx.store; + } + const face = (k: { setImage: { mock: { calls: unknown[][] } } }) => + Buffer.from(String(k.setImage.mock.calls.at(-1)?.[0]).split(',')[1], 'base64').toString('utf8'); + + it('window paging follows the placed-key count as keys appear and disappear', () => { + // Observe the window WIDTH through paging: windowStart = floor(cursor / size) * size. + const store = fleet(8, 3); // selection on b3 + const ba = new BuilderAction(store); + const keys = Array.from({ length: 4 }, (_, i) => bkey(`k${i}`, i)); + keys.forEach((k) => ba.onWillAppear({ action: k, payload: { settings: {} } } as never)); + // 4 placed → windowStart floor(3/4)*4 = 0 → slot 0 = b0. + expect(store.windowedBuilder(0)?.id).toBe('b0'); + ba.onWillDisappear({ action: keys[3], payload: { settings: {} } } as never); + // 3 placed → windowStart floor(3/3)*3 = 3 → slot 0 = b3. + expect(store.windowedBuilder(0)?.id).toBe('b3'); + }); + + it('multi-action instances are excluded from the window count', () => { + // Selection on b3; three placed keys + a multi-action instance. If the multi-action key + // were counted the window would be 4 wide (windowStart 0 → slot 0 = b0); excluded, it is + // 3 wide (windowStart 3 → slot 0 = b3). + const store = fleet(8, 3); + const ba = new BuilderAction(store); + const keys = Array.from({ length: 3 }, (_, i) => bkey(`k${i}`, i)); + keys.forEach((k) => ba.onWillAppear({ action: k, payload: { settings: {} } } as never)); + ba.onWillAppear({ action: multiKey('M'), payload: { settings: {} } } as never); + expect(store.windowedBuilder(0)?.id).toBe('b3'); // still 3 wide + }); + + it('slots keys in (row, column) reading order regardless of arrival order or row', () => { + vi.useFakeTimers(); + try { + // Keys arrive out of reading order and span two rows: a lower row and a higher column + // must each sort later. Reading order is (r0,c0)=A, (r0,c1)=B, (r1,c0)=C, (r1,c1)=D → + // slots 0..3 → builders b0..b3. Press each and confirm it targets the reading-order builder. + const store = fleet(8, 0); + const ba = new BuilderAction(store); + const A = bkey('A', 0, 0); // (row 0, col 0) — first + const B = bkey('B', 1, 0); // (row 0, col 1) — second + const C = bkey('C', 0, 1); // (row 1, col 0) — third: its row wins over its lower column + const D = bkey('D', 1, 1); // (row 1, col 1) — last + // Deliberately out of reading order: + [D, C, B, A].forEach((k) => ba.onWillAppear({ action: k, payload: { settings: {} } } as never)); + vi.advanceTimersByTime(60); + const pressed: string[] = []; + const client = store.client as unknown as { sendCommand: ReturnType }; + client.sendCommand.mockImplementation((_v: string, args: string[]) => { + pressed.push(args[0]); + return Promise.resolve({ ok: true, status: 200, data: { ok: true } }); + }); + return Promise.all( + [A, B, C, D].map((k) => ba.onKeyDown({ action: k, payload: { settings: { verb: 'noop' } } } as never)), + ).then(() => { + expect(pressed).toEqual(['b0', 'b1', 'b2', 'b3']); // A→b0, B→b1, C→b2, D→b3 + }); + } finally { + vi.useRealTimers(); + } + }); + + it('debounced settle re-renders keys whose page shifted as later keys arrived', () => { + vi.useFakeTimers(); + try { + // Selection on b4 (5 builders). Keys arrive one at a time; as the window grows the page + // start shifts, so an earlier key's builder changes and must be corrected on settle. + const store = fleet(5, 4); + const ba = new BuilderAction(store); + const keys = Array.from({ length: 3 }, (_, i) => bkey(`k${i}`, i)); + keys.forEach((k) => ba.onWillAppear({ action: k, payload: { settings: {} } } as never)); + // Before settle, k0 still shows its stale immediate render (windowStart was 4 when it + // appeared alone → b4). After the 3-wide window settles, windowStart is 3 → k0 shows b3. + vi.advanceTimersByTime(60); + expect(face(keys[0])).toContain('#103'); // b3 + expect(face(keys[1])).toContain('#104'); // b4 (the selected builder — on a key) + // And the selected builder is visible on a slot, never hidden. + const shown = keys.map((k) => face(k)); + expect(shown.some((s) => s.includes('#104'))).toBe(true); + } finally { + vi.useRealTimers(); + } + }); + + it('cursor paging stays coherent when the window size changes under a selection', () => { + // Selection on b3 (4 builders). With 3 keys the window is 3 wide → b3 on slot 0 (page 1). + const store = fleet(4, 3); + const ba = new BuilderAction(store); + const keys = Array.from({ length: 3 }, (_, i) => bkey(`k${i}`, i)); + keys.forEach((k) => ba.onWillAppear({ action: k, payload: { settings: {} } } as never)); + expect(store.windowedBuilder(0)?.id).toBe('b3'); // selected builder is shown + // A 4th key appears → window 4 wide → page 0 → b3 on slot 3. Still shown, no gap, no crash. + const k3 = bkey('k3', 3); + ba.onWillAppear({ action: k3, payload: { settings: {} } } as never); + const slots = [0, 1, 2, 3].map((i) => store.windowedBuilder(i)?.id); + expect(slots).toEqual(['b0', 'b1', 'b2', 'b3']); + expect(slots).toContain('b3'); // the selection remains on a rendered slot }); }); diff --git a/apps/streamdeck/src/actions.ts b/apps/streamdeck/src/actions.ts index 3cdd813d8..4dbd0bde7 100644 --- a/apps/streamdeck/src/actions.ts +++ b/apps/streamdeck/src/actions.ts @@ -1,5 +1,6 @@ import { SingletonAction, + type Coordinates, type KeyAction, type DialAction, type KeyDownEvent, @@ -85,35 +86,35 @@ export class DevServerAction extends VerbKey { // ── Slot keys (pinned builder board) ──────────────────────────────────────── -/** PI settings shared by the slot-based keys: a 1-based builder slot + a verb. */ -type SlotSettings = { slot?: string; verb?: string }; +/** PI settings for the slot-based keys: an optional per-key verb. The former manual + * `slot` field is retired (#1465) — a key's slot is derived from where it physically + * sits, not hand-numbered, so the window can never disagree with the placed keys. */ +type SlotSettings = { verb?: string }; -/** - * Resolve the builder a slot points at. Slot N is a POSITION in Row 1's 4-wide - * window onto the fleet (#1410), not an absolute index: the window is the page - * containing the selection, so the Select dial scrolls builders 5-8, 9-N into the - * same four keys. A slot past the end of the fleet resolves to `undefined` (an - * empty slot on the last page). - */ -function slotBuilder(store: CodevStore, settings: SlotSettings): OverviewBuilder | undefined { - const slot = Number.parseInt(settings.slot ?? '1', 10); - const slotIndex = (Number.isFinite(slot) && slot > 0 ? slot : 1) - 1; - return store.windowedBuilder(slotIndex); -} +/** How long to let the page-load `willAppear`/`willDisappear` burst settle before + * the full re-render (#1465). Keys arrive over several events at load, and a later + * key can shift an earlier key's page; one coalesced pass avoids thrashing the + * window size and flickering the faces. */ +const WINDOW_SETTLE_MS = 50; /** - * A keypad pinned to a builder slot (the Nth builder) that fires a verb for that - * builder. The Property Inspector picks the slot and the verb; the press is - * resilient to builder ids changing because it indexes by position, not id. - * Subclasses override the default verb, the render, and (optionally) the verb - * resolution (BuilderAction resolves its `automatic` default to a phase artifact). + * A keypad pinned to a builder SLOT — the Nth builder in Row-1's window onto the + * fleet — that fires a verb for that builder. A key's slot is its RANK among the + * placed builder keys sorted by physical position (row, then column), derived from + * `KeyAction.coordinates`, NOT a hand-numbered setting (#1465): so the window is + * exactly as wide as the keys you placed, and a builder is never selectable while + * shown on no key. The press indexes by position, so it survives builder ids + * changing. Subclasses override the default verb, the render, and (optionally) the + * verb resolution (BuilderAction resolves its `automatic` default to a phase artifact). */ abstract class SlotKey extends SingletonAction { protected abstract readonly defaultVerb: string; // One SingletonAction instance serves EVERY key of this type, so per-key state - // (its settings + render handle) is tracked per instance, keyed by the action - // context id — never in shared fields, which would collide across keys. - private readonly keys = new Map(); + // (its action handle, settings, and board coordinates) is tracked per instance, + // keyed by the action context id — never in shared fields, which would collide. + // `coordinates` is `undefined` for a multi-action instance (excluded from the window). + private readonly keys = new Map(); + private settleTimer?: ReturnType; constructor(protected readonly store: CodevStore) { super(); @@ -123,21 +124,28 @@ abstract class SlotKey extends SingletonAction { override onWillAppear(ev: WillAppearEvent): void { if (!ev.action.isKey()) return; const settings = ev.payload.settings ?? {}; - this.keys.set(ev.action.id, { action: ev.action, settings }); - this.renderTo(ev.action, settings); + this.keys.set(ev.action.id, { action: ev.action, settings, coordinates: ev.action.coordinates }); + // Size the window from the placed keys right away so a press resolves against the + // current layout, and render THIS key now; the debounced pass re-renders the rest + // once the page-load burst settles (a later key can shift an earlier key's page). + this.store.setBuilderWindowSize(this.placedKeys().length); + this.renderTo(ev.action); + this.scheduleSettle(); } override onWillDisappear(ev: WillDisappearEvent): void { this.keys.delete(ev.action.id); + this.store.setBuilderWindowSize(this.placedKeys().length); + this.scheduleSettle(); } override onDidReceiveSettings(ev: DidReceiveSettingsEvent): void { const entry = this.keys.get(ev.action.id); if (!entry) return; entry.settings = ev.payload.settings ?? {}; - this.renderTo(entry.action, entry.settings); + // A verb change doesn't move any key, so just re-render this one. + this.renderTo(entry.action); } override async onKeyDown(ev: KeyDownEvent): Promise { - const settings = ev.payload.settings ?? {}; - const b = slotBuilder(this.store, settings); + const b = this.builderFor(ev.action); if (!b) { await ev.action.showAlert(); return; @@ -145,18 +153,56 @@ abstract class SlotKey extends SingletonAction { // Pressing a builder key focuses it: the shared cursor follows, so the diff // dials and other selection-scoped keys now act on the builder you pressed. this.store.syncToBuilder(b.id); + const settings = ev.payload.settings ?? {}; const verb = this.resolveVerb(settings, b); const res = await this.store.client.sendCommand(verb, [b.id], this.store.selectedWorkspacePath()); await ack(ev.action, res.ok); } + + /** Placed builder keys — those with defined coordinates (multi-action instances + * report `undefined` and are excluded) — in reading order: row, then column. */ + private placedKeys(): KeyAction[] { + return [...this.keys.values()] + .filter( + (e): e is { action: KeyAction; settings: SlotSettings; coordinates: Coordinates } => + e.coordinates !== undefined, + ) + .sort((a, b) => a.coordinates.row - b.coordinates.row || a.coordinates.column - b.coordinates.column) + .map((e) => e.action); + } + /** The 0-based slot a key occupies (its rank among the placed keys), or `undefined` + * when it is a multi-action instance with no coordinates. */ + protected slotIndexOf(action: KeyAction): number | undefined { + const slotIndex = this.placedKeys().findIndex((a) => a.id === action.id); + return slotIndex < 0 ? undefined : slotIndex; + } + /** The builder a key shows: its slot indexes the fleet window. `undefined` for a + * multi-action key (no slot) or a slot past the end of the fleet (trailing empty). */ + protected builderFor(action: KeyAction): OverviewBuilder | undefined { + const slotIndex = this.slotIndexOf(action); + if (slotIndex === undefined) return undefined; + return this.store.windowedBuilder(slotIndex); + } + + /** Debounce a full re-render across the willAppear/willDisappear settle (#1465); + * reassert the window size from the settled key set before rendering. */ + private scheduleSettle(): void { + if (this.settleTimer) clearTimeout(this.settleTimer); + this.settleTimer = setTimeout(() => { + this.settleTimer = undefined; + this.store.setBuilderWindowSize(this.placedKeys().length); + this.renderAll(); + }, WINDOW_SETTLE_MS); + } + /** The verb a press fires. Base: the per-key setting, else the default. */ protected resolveVerb(settings: SlotSettings, _b: OverviewBuilder): string { return settings.verb ?? this.defaultVerb; } private renderAll(): void { - for (const { action, settings } of this.keys.values()) this.renderTo(action, settings); + for (const { action } of this.keys.values()) this.renderTo(action); } - protected abstract renderTo(action: KeyAction, settings: SlotSettings): void; + protected abstract renderTo(action: KeyAction): void; } /** @@ -179,19 +225,22 @@ export class BuilderAction extends SlotKey { const auto = phaseArtifactVerb(b) ?? 'open-terminal'; return auto === 'view-diff' ? 'open-diff-first' : auto; } - protected renderTo(action: KeyAction, settings: SlotSettings): void { - const b = slotBuilder(this.store, settings); + protected renderTo(action: KeyAction): void { + const b = this.builderFor(action); // Compose the WHOLE face as one SVG (icon zone + reserved text band) instead of stacking a // title over the manifest bolt PNG — see face.ts for the layout and the sidebar-mirrored // colour/icon vocabulary. setTitle('') suppresses the SDK title layer so nothing overlays it. let svg: string; if (b) { - // Accent the slot holding the shared selection so the live builder among - // the four is unmistakable (#1410). + // Accent the slot holding the shared selection so the live builder among the + // placed keys is unmistakable (#1410). const selected = b.id === this.store.selectedBuilder()?.id; svg = builderFaceSvg(faceForBuilder(b, selected)); } else { - svg = builderFaceSvg({ kind: 'empty', slot: settings.slot ?? '1' }); + // A placed but off-fleet slot shows its 1-based position; a multi-action key + // (no slot) shows a dash. + const slotIndex = this.slotIndexOf(action); + svg = builderFaceSvg({ kind: 'empty', slot: slotIndex === undefined ? '—' : String(slotIndex + 1) }); } void action.setImage(svgToDataUri(svg)); void action.setTitle(''); diff --git a/apps/streamdeck/src/store.ts b/apps/streamdeck/src/store.ts index 0c2c8bd92..15e7b37bb 100644 --- a/apps/streamdeck/src/store.ts +++ b/apps/streamdeck/src/store.ts @@ -23,10 +23,6 @@ export interface CodevStoreOptions { openUrl?: (url: string) => void | Promise; } -/** Row-1 selector width on the SD+ (a 2×4 keypad): the fleet window is this many - * builders wide, and the Select dial scrolls it a page at a time (#1410). */ -export const ROW1_WINDOW_SIZE = 4; - export class CodevStore { readonly client: ControllerClient; readonly openUrl: (url: string) => void | Promise; @@ -41,6 +37,14 @@ export class CodevStore { /** Monotonic token so an out-of-order overview fetch can't overwrite a newer one. */ private overviewReq = 0; + /** Row-1 window width = the number of `BuilderAction` selector keys currently + * placed on the deck (#1465, replacing the fixed 4 of #1410). The `BuilderAction` + * singleton counts its visible instances and reports it via `setBuilderWindowSize`; + * the window then pages the fleet by that count, so a builder is never selectable + * while shown on no key. Defaults to 1 until the first key reports — with no keys + * placed nothing renders, so the value is only a division guard. */ + private builderWindowSize = 1; + private readonly listeners = new Set<() => void>(); private stopSse?: () => void; @@ -143,21 +147,31 @@ export class CodevStore { return this.builders()[this.cursor.builder]; } + /** Report how many `BuilderAction` selector keys are currently placed (#1465). The + * window pages the fleet by this count, so the Row-1 window is exactly as wide as + * the keys on the board and a builder can't be selected while shown on no key. + * Never below 1 (a division guard for the no-keys-placed case). */ + setBuilderWindowSize(count: number): void { + this.builderWindowSize = Math.max(1, count); + } + /** - * The builder shown in Row-1 selector slot `slotIndex` (0-based, 0..3). Row 1 - * is a 4-wide WINDOW onto the fleet, not a fixed view of the first four (#1410): - * the window is the page containing the selection, so rotating the Select dial - * past the 4th builder scrolls Row 1 to builders 5-8, then 9-N. A slot past the - * end of the fleet returns `undefined` (a trailing empty slot on the last page). + * The builder shown in Row-1 selector slot `slotIndex` (0-based). Row 1 is a + * WINDOW onto the fleet whose width is the number of placed builder keys (#1465, + * replacing the fixed 4 of #1410): the window is the page containing the selection, + * so rotating the Select dial past the last visible builder scrolls Row 1 to the + * next page. A slot past the end of the fleet returns `undefined` (a trailing empty + * slot on the last page). */ windowedBuilder(slotIndex: number): OverviewBuilder | undefined { return this.builders()[this.builderWindowStart() + slotIndex]; } - /** First builder index of the Row-1 window: the page (of `ROW1_WINDOW_SIZE`) - * that contains the current selection. */ + /** First builder index of the Row-1 window: the page (of `builderWindowSize`, the + * placed-key count) that contains the current selection. */ private builderWindowStart(): number { - return Math.floor(this.cursor.builder / ROW1_WINDOW_SIZE) * ROW1_WINDOW_SIZE; + const size = Math.max(1, this.builderWindowSize); + return Math.floor(this.cursor.builder / size) * size; } /** The workspace's review-feedback delivery mode (#1410); `'forward'` until an diff --git a/codev/plans/1465-stream-deck-size-the-row-1-bui.md b/codev/plans/1465-stream-deck-size-the-row-1-bui.md new file mode 100644 index 000000000..35c6024dc --- /dev/null +++ b/codev/plans/1465-stream-deck-size-the-row-1-bui.md @@ -0,0 +1,194 @@ +# PIR Plan: Size the Row-1 builder window from the placed keys + +## Understanding + +**The correctness bug (this is what earns the lane).** Row-1 selection pages the fleet +by a hardcoded constant, not by how many builder keys the user actually placed: + +- `ROW1_WINDOW_SIZE = 4` (`apps/streamdeck/src/store.ts:28`). +- `builderWindowStart()` = `floor(cursor.builder / 4) * 4` (`store.ts:159-161`). +- `windowedBuilder(slotIndex)` = `builders()[windowStart + slotIndex]` (`store.ts:153-155`). +- Each `BuilderAction` key resolves the builder it shows from its Property-Inspector + `slot` field (1-based → `slotIndex`), via `slotBuilder()` (`actions.ts:98-102`). + +The page step is fixed at four and is **independent of the number of placed `BuilderAction` +keys**. Place three builder keys (the exact configuration #1463 creates when its Open +Architect key takes a Row-1 slot) and every builder at index ≡ 3 (mod 4) lands at +`slotIndex 3` — a slot with no physical key. It renders on **nothing**, yet the Select dial +(`ZoomNav` rotate → `rotateCursor`, clamped only to `builders().length`) still walks the +cursor onto it. So that invisible builder becomes `selectedBuilder()`: it drives Row 2's +entire palette (Approve, Send Fb, Open Terminal, Open Architect) and both review dials, and +shows **no accent ring anywhere on the board**. The reviewer is acting on a builder the deck +never displays. The board is only safe while the fleet is no larger than the placed-key count. + +The constant is also wrong for most hardware (Mini 3×2, Standard 5×3, XL 8×4, SD+ keypad +4×2) — but that multi-device sizing is the bonus; the ambiguous-selection bug is the reason. + +**Root cause, stated precisely:** the window size (page step) is a compile-time constant, +while the *slot* a key represents is a hand-numbered PI field. Neither is tied to the ground +truth — the set of `BuilderAction` keys physically on the board — so the two can disagree and +a builder can be selectable while shown on no key. + +**Feasibility — verified against `@elgato/streamdeck@2.1.0` source in `node_modules`:** + +- There is **no profile-structure API**. `plugin/profiles.d.ts` states plugins "may only + switch to profiles distributed with the plugin … and cannot access user-defined profiles", + and `switchToProfile` is the only profile call. The layout cannot be read directly; it must + be **derived from the lifecycle**. (Do not go looking for a profile reader — there isn't one.) +- Every visible key fires `willAppear` and exposes `KeyAction.coordinates: { column, row } | + undefined` — `undefined` when the action is part of a multi-action + (`dist/plugin/actions/key.d.ts:19`, `dist/api/events/action.d.ts:181,188`). +- `Device.size = { columns, rows }` and `Device.type` are available via `action.device` and + `streamDeck.devices` (`dist/plugin/devices/device.d.ts:39,44`) — available, but **not needed** + for this fix (see Risks). + +## Proposed Change + +Make the ground truth — the placed `BuilderAction` keys — drive both the window size and each +key's position. Two coupled moves: + +**1. Window size = count of currently-visible `BuilderAction` keys (replaces the constant).** +`BuilderAction` is a single `SingletonAction` instance serving every builder key; it already +tracks its keys in a per-context `Map` (`actions.ts:116`, `keys.set/delete` on +willAppear/willDisappear). Extend that map to capture each key's `coordinates`, and have the +action report the live count to the store, which pages by it: + +- `store.ts`: replace the `ROW1_WINDOW_SIZE` constant with an instance field + `builderWindowSize` (default `1`) plus `setBuilderWindowSize(n)`. `builderWindowStart()` + becomes `floor(cursor.builder / max(1, builderWindowSize)) * max(1, builderWindowSize)`. + `windowedBuilder(slotIndex)` keeps its formula. The `max(1, …)` guards division when no + builder keys are visible. + +*What "count" means — placed keys, not device capacity.* The window size is the number of +`BuilderAction` keys **you actually placed and that are currently on screen**, never the +device's key count. `BuilderAction` is one `SingletonAction` serving every builder key; each +placed key fires `willAppear` on it (and `willDisappear` when removed or paged away), so its +key map holds exactly the live builder keys. Any other action in a neighbouring slot is a +*different* `SingletonAction` — e.g. #1463's Open Architect key fires on `OpenArchitectAction`, +not `BuilderAction` — so it never enters the builder count; the manifest UUID does that routing +for free. Concretely on an SD+ (a 4-wide top row) with **3 Builder Action keys + 1 Open +Architect key**: the map has 3 entries, the window size is 3, the Select dial pages the fleet +by 3, and there is no phantom 4th slot — which is exactly the bug (today's constant `4` invents +a slot-3 with no key, so every 4th builder is selectable but shown nowhere). `Device.size` / +`Device.type` are deliberately *not* consulted: device size describes the whole keypad, not +which keys are builder selectors. The count is also **per visible page** — a profile/page switch +fires `willDisappear`/`willAppear`, so it always reflects what is currently displayed. + +**2. Slot order derived by sorting visible keys on `(row, column)`; retire the manual `slot` +field.** In `BuilderAction`, recompute on `willAppear`/`willDisappear`: + +- Filter to keys with **defined** coordinates (skip multi-action instances). +- Sort by `(row, column)` — reading order, left-to-right then top-to-bottom (so builder keys + that span rows on a larger deck still fill in a sensible order). +- Assign each key its rank as its `slotIndex`; the count of these keys is the window size. +- **Debounce** the recompute (~50 ms, injectable for tests): at page load keys arrive over + several `willAppear` events, and an eager recompute would thrash the size and flicker the + render. One coalesced recompute after the burst settles, then `renderAll()`. + +`renderTo(key)` and `onKeyDown(ev)` resolve their builder from the key's derived `slotIndex` +(looked up by the action's context id in the freshly-computed position map), not from +`settings.slot`. `slotBuilder()` is replaced by this position lookup. The empty-slot face uses +`position + 1` for its label instead of the retired `slot` string. + +**Decision — the manual `slot` PI field is RETIRED (argued below; flagged for your confirm).** + +The `slot` selector on Builder Action existed only because identical singleton instances had no +cheap way to learn their rank among siblings. `KeyAction.coordinates` now supplies that directly, +so the field is redundant. I recommend **removing it** rather than keeping it as an override: + +- *Derived order matches intent in essentially every real layout.* Users place "slot 1" on the + leftmost key; sorting by `(row, column)` reproduces exactly that, minus the manual step. +- *It kills a whole misconfiguration class.* The manual field permits duplicate slots (two keys + claiming "slot 2") and gaps (slots 1, 2, 4 → a builder silently skipped at position 3) — the + same "a builder is hidden while selectable" failure this issue exists to remove. +- *Keeping it as an override reintroduces that surface.* An override needs precedence rules + ("explicit slot beats positional") and collision handling (tiebreak two keys at the same slot, + compact-or-skip a gap) — complexity bought only for an exotic "physically reorder my keys by + number" case that the Stream Deck app already serves by dragging keys. + +Retirement is **user-visible and irreversible** for anyone who deliberately set non-physical +slot numbers, so I am not defaulting silently: **this is the one decision I want you to confirm +at plan-approval.** Backward-compat is graceful — no migration needed. The PI drops the Slot +selector; any `slot` value already persisted on a key is simply ignored (it sits unused in +settings), and the key re-orders by its physical position on next `willAppear`. If you prefer +keeping `slot` as an override, say so and I will add the precedence + collision rules instead. + +## Files to Change + +- `apps/streamdeck/src/store.ts:26-28,153-161` — remove the `ROW1_WINDOW_SIZE` constant; add + `builderWindowSize` field + `setBuilderWindowSize()`; make `builderWindowStart()` page by the + dynamic size with a `max(1, …)` guard; refresh the `windowedBuilder` / `builderWindowStart` + doc comments (no longer "4-wide", no longer "0..3"). +- `apps/streamdeck/src/actions.ts:88-199` — in the `SlotKey`/`BuilderAction` layer: capture + `coordinates` in the key map; add a debounced recompute that sorts visible (defined-coord) + keys by `(row, column)`, sets the store window size, and re-renders; resolve each key's builder + from its derived position (replacing `slotBuilder()` and the `settings.slot` reads); label the + empty face from `position + 1`. Drop `slot` from `SlotSettings`. +- `apps/streamdeck/com.cluesmith.codev.sdPlugin/ui/builder-action.html` — remove the `Slot` + `` item and the "Pins this key to the Nth builder (slot 1 = first builder)" + sentence; keep the `verb` ("On press") selector unchanged. +- `apps/streamdeck/README.md:99-136` — update the Row-1 description: the Builder Action keys are + a window **sized to the number of placed builder keys** (not a fixed 4-wide / slot 1–4); the + Select dial scrolls it a page (= that many keys) at a time; keys self-order by physical position. +- `apps/streamdeck/src/__tests__/actions.test.ts` (+ possibly a small `store` unit) — rewrite the + "Row 1 windowing" block to drive via coordinates + visible-key count; see Test Plan for the new + cases. + +Out of scope, explicitly: Row 2 keys, the review/zoom dials, and #1381's larger-profile question. + +## Risks & Alternatives Considered + +- **Regression risk — window size changes underneath a live selection.** A key appearing or + disappearing re-pages the window while `cursor.builder` stays fixed. This is the main hazard, + so it gets a dedicated test: after a size change, the selected builder must still land on a + rendered key. `rotateCursor` clamps only to `builders().length` and nothing clamps the cursor + to the window size, so no crash — but the invariant ("selected builder is always on a key") + must hold, and that is exactly what the bug broke. Tested directly (Test Plan). +- **Async settle / thrash.** Keys arrive over several `willAppear` events at page load; an eager + recompute flickers size and render. Mitigation: debounce (injectable interval) + one + `renderAll()` after the burst. +- **Multi-action instances (undefined coordinates).** A Builder Action inside a multi-action has + `coordinates === undefined`; it is excluded from the count and from positioning, and resolves + to no builder (inert / empty face) when pressed. Tested. +- **Alternative — read the profile / use `Device.size` to size the window.** Rejected: no profile + API exists, and device size describes the whole board, not which keys are Builder Action keys. + The count of visible builder keys is both sufficient and *more precise* — it counts exactly the + placed selectors, on any device, with no device-type table to maintain. +- **Alternative — keep `slot` as an override on top of positional order.** Rejected (see Proposed + Change): reintroduces the duplicate/gap misconfiguration surface for a niche capability the + Stream Deck app already covers by dragging keys. Deferred to your call at the gate. + +## Test Plan + +**Unit (vitest, `apps/streamdeck` — runs headless in the worktree):** + +- *Dynamic window size.* With N visible builder keys reported, `windowedBuilder` pages by N: + N = 3 over a fleet of 4+ shows builders 0–2 then 3–…; the fourth builder is reachable and never + falls off a page. +- *Core invariant (the bug).* For a fleet larger than the key count, at **every** `cursor.builder` + in `[0, builders)` there exists a `slotIndex` in `[0, size)` with + `windowedBuilder(slotIndex).id === selectedBuilder().id` — i.e. the selected builder is always + on a rendered key. Asserted for size = 3 and size = 4. +- *Positional ordering.* Keys given `(row, column)` out of placement order still resolve to + builders in reading order; two keys at different coordinates get distinct, contiguous positions. +- *Cursor paging under a size change.* Select a builder, then add/remove a key so the window size + changes; assert the selected builder still lands on a key (re-pages, no gap, no crash). +- *Multi-action skip.* A key with `coordinates === undefined` is excluded from the count and + positioning and does not shift the others. +- *Settle/debounce.* A burst of `willAppear` events yields one authoritative recompute (fake + timers); the final window size equals the placed-key count. + +**Manual — hardware (dev-approval session, Amr driving the deck):** + +- *3-key layout (the bug repro).* Place **3** Builder Action keys (one Row-1 slot taken by the + Open Architect key, per #1463). Spawn 4+ builders. Rotate the Select dial onto the 4th builder + and confirm: it now renders on a key **and** shows the accent ring; Row 2 + the review dials act + on the builder that is actually displayed. (Before this change, the 4th builder was selected but + shown nowhere.) +- *4-key layout (no regression).* Place **4** Builder Action keys; confirm paging and accent match + today's behaviour for fleets of 4, 5, and 8. +- *Non-SD+ device, if available.* On a Standard/Mini (or XL), place a device-appropriate number of + Builder Action keys and confirm the window sizes to the placed count and the selection is always + visible. +- *Reorder sanity.* Drag a Builder Action key to a different position and confirm the slot order + follows physical placement (validates the retired manual field). diff --git a/codev/projects/1465-stream-deck-size-the-row-1-bui/1465-review-iter1-rebuttals.md b/codev/projects/1465-stream-deck-size-the-row-1-bui/1465-review-iter1-rebuttals.md new file mode 100644 index 000000000..923680eca --- /dev/null +++ b/codev/projects/1465-stream-deck-size-the-row-1-bui/1465-review-iter1-rebuttals.md @@ -0,0 +1,66 @@ +# Rebuttal — 3-way consultation, iteration 1 (PIR #1465) + +Verdicts: **Gemini APPROVE**, **Codex REQUEST_CHANGES**, **Claude REQUEST_CHANGES**. + +All three REQUEST_CHANGES findings were real (two overlapped across Codex and Claude), +all were documentation/coverage issues around an otherwise-approved core fix, and all +are **accepted and fixed** on-branch (commit `39da9eee5`). No point is disputed. + +PIR runs the consultation once (`max_iterations: 1`), so these fixes are not +independently re-reviewed — they are surfaced to the human at the `pr` gate for +verification, and recorded in the review file's "3-Way Consultation — Verdicts & +Dispositions" section. + +## 1. Stale README caveat describing the old fixed-4 window as current — FIXED +Raised by Codex (`README.md:184-187`) and Claude (`README.md:182-187`). + +The Open Architect Terminal bullet still carried a "Placement caveat" stating Row 1 +"is a fixed page of four … hides every fourth builder past a three-builder fleet" and +deferred self-sizing to "#1465" as future work. That is a verbatim description of the +bug this PR fixes, and it contradicted the new recommended layout earlier in the same +file (which places a Main-mode Open Architect key in Row 1 slot 1). This is exactly the +fixed-4 text #1463 shipped for *this* issue to remove; I missed it in the first pass. + +**Fix**: rewrote the caveat — the Main-mode key is selection-independent, and because +the Row-1 window now sizes itself to the placed Builder Action keys (#1465), giving a +Row 1 key to it leaves a correctly-sized three-wide builder window with no hidden +builders; pointed at the recommended layout (Row 1 slot 1). No "#1465 is future work". + +## 2. Manifest tooltip still instructs setting the retired slot field — FIXED +Raised by Claude (`manifest.json:133`). + +The Builder Action `Tooltip` read "Live tile for the Nth builder (**set the slot in the +property inspector**)". The PI Slot control was removed in this PR, so the tooltip — which +is user-visible in the Stream Deck app's action list, and was not in the PR's original +file list — pointed at a field that no longer exists. A reviewer at the hardware session +would go looking for it. + +**Fix**: tooltip now says the slot is where you place the key (selectors self-order left +to right, top row first). + +## 3. Missing positional-order test — FIXED +Raised by Codex and Claude. + +The plan listed a case for keys given `(row, column)` out of placement order; every +fixture used `row 0` with ascending columns, so the comparator's row term and its +out-of-arrival-order behaviour were never exercised — and that comparator is the whole +basis of the "no slot numbers" claim. + +**Fix**: added a test that appears four keys across two rows in reverse reading order +(D, C, B, A) and asserts each press targets its reading-order builder (A→b0, B→b1, +C→b2, D→b3), so both the row term and the arrival-order independence are covered. + +## Non-blocking notes — accepted, no change +- `WINDOW_SETTLE_MS` is a module constant rather than injectable (the plan said + injectable). Fake timers cover the debounce fully, so no change — noted by Claude as + needing none. +- `onWillDisappear` doesn't guard `isKey()`; `Map.delete` on an absent id is harmless. + +Claude independently confirmed the core fix correct: the `max(1,·)` divide-by-zero guard, +multi-action (undefined-coordinate) exclusion, and that `setBuilderWindowSize` does not +`emit()` so the eager size update in `onWillAppear` can't re-enter `renderAll` — the +debounce genuinely debounces. + +## Verification after fixes +`npm run check-types` (tsc) ✓, `npm test` ✓ (213 tests), `npm run build` ✓, +`npm run validate` ✓ (manifest edited). diff --git a/codev/projects/1465-stream-deck-size-the-row-1-bui/status.yaml b/codev/projects/1465-stream-deck-size-the-row-1-bui/status.yaml new file mode 100644 index 000000000..87484d2f6 --- /dev/null +++ b/codev/projects/1465-stream-deck-size-the-row-1-bui/status.yaml @@ -0,0 +1,30 @@ +id: '1465' +title: stream-deck-size-the-row-1-bui +protocol: pir +phase: verified +plan_phases: [] +current_plan_phase: null +gates: + plan-approval: + status: approved + requested_at: '2026-08-15T09:54:31.993Z' + approved_at: '2026-08-15T09:59:20.586Z' + dev-approval: + status: approved + requested_at: '2026-08-15T10:13:55.805Z' + approved_at: '2026-08-15T10:45:46.628Z' + pr: + status: approved + requested_at: '2026-08-15T10:53:13.341Z' + approved_at: '2026-08-15T10:55:22.533Z' +iteration: 1 +build_complete: true +history: [] +started_at: '2026-08-15T09:49:32.581Z' +updated_at: '2026-08-15T10:55:45.137Z' +pr_history: + - phase: review + pr_number: 1468 + branch: builder/pir-1465 + created_at: '2026-08-15T10:47:50.023Z' +pr_ready_for_human: false diff --git a/codev/resources/lessons-learned.md b/codev/resources/lessons-learned.md index c157f101d..a6d353a77 100644 --- a/codev/resources/lessons-learned.md +++ b/codev/resources/lessons-learned.md @@ -365,6 +365,19 @@ Generalizable wisdom extracted from review documents, ordered by impact. Updated glyph line out of `face.ts` — so a new glyph must be a single line with **no trailing comment**, and after running it, restore any pre-existing icon PNGs it re-touched (byte churn) so the diff stays scoped. +- [From #1465] A UI "window onto a list" must size itself from the elements actually **placed**, not + a hardcoded page constant. The Stream Deck Row-1 selector paged by a fixed `ROW1_WINDOW_SIZE = 4`; + place fewer keys than that and a selectable item at index ≡ (size) mod 4 rendered on **no key** while + the selection dial still walked onto it — selectable but invisible, driving the rest of the board with + no accent anywhere. The fix derives the window width from the visible action instances. Two SDK facts + make it work: there is **no profile-structure API** (`@elgato/streamdeck` `profiles.d.ts` says a plugin + "cannot access user-defined profiles"), so the layout can only be **derived from the lifecycle** — + each visible key fires `willAppear` carrying `KeyAction.coordinates {column,row}` (`undefined` for a + multi-action instance, which must be excluded). Sort the placed keys by `(row, column)` for slot order, + count them for the window width, and **debounce the recompute** — keys arrive over several `willAppear` + events at page load, so an eager recompute thrashes the size and flickers the faces. General rule: when + a fixed count and a hand-numbered index can disagree with the true placed set, tie both to the placed + set so a selection can never point at nothing. - [From #1428] Stream Deck's `setImage` accepts an SVG per the SDK d.ts, but a *raw* `` string is silently dropped on-device (Stream Deck 6.9) — the key reverts to its manifest PNG with no error. Two undocumented requirements: encode as a base64 `data:image/svg+xml` data URI, diff --git a/codev/reviews/1465-stream-deck-size-the-row-1-bui.md b/codev/reviews/1465-stream-deck-size-the-row-1-bui.md new file mode 100644 index 000000000..5dd1c8835 --- /dev/null +++ b/codev/reviews/1465-stream-deck-size-the-row-1-bui.md @@ -0,0 +1,140 @@ +# PIR Review: Size the Row-1 builder window from the placed keys + +Fixes #1465 + +## Summary + +The Stream Deck Row-1 fleet selector paged by a hardcoded `ROW1_WINDOW_SIZE = 4`, +independent of how many `BuilderAction` keys the user actually placed. With fewer +placed keys than four, a builder at index ≡ 3 (mod 4) rendered on no key while the +Select dial still walked the cursor onto it — so it became the selected builder, +drove Row 2 and both review dials, and showed no accent ring anywhere. This change +sizes the window to the number of visible `BuilderAction` keys and derives each key's +slot from its physical board position (`KeyAction.coordinates`, sorted by row then +column), retiring the manual `slot` Property-Inspector field. The window now follows +the placed keys, so a builder can never be selected while shown on no key. + +## Files Changed + +- `apps/streamdeck/src/store.ts` — removed the `ROW1_WINDOW_SIZE` constant; the window + is sized by a reported placed-key count (`setBuilderWindowSize`, `max(1,·)` guard). +- `apps/streamdeck/src/actions.ts` — `SlotKey` captures each key's `coordinates`, sorts + placed keys by `(row, column)` for slot order, reports the count, skips multi-action + (undefined-coordinate) instances, and debounces a full re-render across the page-load + settle. `slotBuilder()` and `settings.slot` retired. +- `apps/streamdeck/com.cluesmith.codev.sdPlugin/ui/builder-action.html` — dropped the + Slot selector (slots are now positional); kept the verb selector; rewrote help text. +- `apps/streamdeck/README.md` — recommended SD+ layout now leads with an Open Architect + (main-mode) anchor in Row 1 slot 1 + three Builder selectors; Row 2 revised to + free · Approve Gate · Open Architect (builder mode) · Open Builder Terminal; windowing + prose stated in placed-key terms. +- `apps/streamdeck/src/__tests__/actions.test.ts` — windowing tests rewritten to drive + via coordinates + placed-key count, plus the selected-always-shown invariant, the + debounced settle, and cursor-paging coherence under a window-size change. + +## Commits + +``` +8c016a973 [PIR #1465] Docs: revise recommended Row 2 (free · Gate · Arch(bldr) · Bldr Term) +3bbf15782 [PIR #1465] Docs: recommended SD+ layout leads with the architect anchor +2feb84922 [PIR #1465] Tests: dynamic window sizing, the selected-always-shown invariant, settle + paging +99491b5f5 [PIR #1465] Retire the manual Slot PI field; document placed-key windowing +a73523b71 [PIR #1465] Size the Row-1 window from placed builder keys +``` +(plus builder-thread and porch bookkeeping commits.) + +## Test Results + +In the worktree (`apps/streamdeck`): `npm run build` ✓, `npm run check-types` (tsc) ✓, +`npm test` ✓ (212 tests, incl. the rewritten dynamic-window suite), `npm run validate` +✓. Porch's `build` + `tests` gate checks also passed. The running plugin was verified +on hardware at the `dev-approval` gate (3-key and 4-key Row-1 layouts). + +## Architecture Updates + +No arch changes. The fix is internal to the Stream Deck plugin's Row-1 windowing — +it changes no module boundary, wire contract, state store, or the four-tier resolver, +so nothing qualifies for `arch-critical.md` (hot) or `arch.md` (cold). + +## Lessons Learned Updates + +Routed one **COLD** lesson to `codev/resources/lessons-learned.md` (UI/UX), tagged +`[From #1465]`: a UI "window onto a list" must size itself from the elements actually +placed, not a hardcoded page constant, or a selection can point at an element rendered +on no key; and — Stream-Deck-specific — with no profile-structure API the layout must be +derived from the lifecycle (`willAppear` `KeyAction.coordinates`, excluding undefined-coord +multi-action instances), sorted by `(row, column)`, counted for the width, and debounced +across the page-load settle. This is a plugin-narrow recipe, so COLD, not the hot tier. + +## 3-Way Consultation — Verdicts & Dispositions + +Gemini **APPROVE**; Codex **REQUEST_CHANGES**; Claude **REQUEST_CHANGES**. PIR runs the +consultation once (`max_iterations: 1`), so these were not independently re-reviewed — +the fixes below and the human `pr`-gate review are the backstop. All three findings were +real (two overlapped across Codex and Claude), all were doc/coverage issues around the +otherwise-approved core fix, and all are addressed in commit(s) on this branch: + +- **Stale README caveat describing the old fixed-4 window as current** (Codex + Claude, + `README.md` Open Architect Terminal bullet). The "Placement caveat" still said Row 1 "is + a fixed page of four … hides every fourth builder" and deferred self-sizing to #1465 — + a verbatim description of the bug this PR fixes, contradicting the new recommended layout + earlier in the same file. This was the fixed-4 text #1463 deliberately shipped for this + issue to remove, and I missed it in the first pass. **Fixed**: rewritten to say the window + self-sizes to the placed keys, so giving Row 1 slot 1 to the Main-mode architect key leaves + a correctly-sized three-wide builder window with no hidden builders. +- **Manifest tooltip still instructs setting the retired slot field** (Claude, + `manifest.json` Builder Action `Tooltip`: "set the slot in the property inspector"). The PI + control is gone; user-visible in the Stream Deck app's action list during the hardware test. + Not in the original file list, so it was missed. **Fixed**: tooltip now says the slot is the + key's physical position (selectors self-order left to right, top row first). +- **Missing positional-order test** (Codex + Claude). The plan listed a case for keys given + `(row, column)` out of placement order; every fixture used row 0 with ascending columns, so + the comparator's row term and out-of-arrival-order behavior were never exercised. **Fixed**: + added a test that appears four keys across two rows in reverse reading order and asserts each + resolves to its reading-order builder (A→b0 … D→b3). + +Non-blocking notes accepted as-is (no change): `WINDOW_SETTLE_MS` is a module constant rather +than injectable (fake timers cover it); `onWillDisappear` doesn't guard `isKey()` (a `Map.delete` +on an absent id is harmless). Claude independently confirmed the core fix correct — `max(1,·)` +divide-by-zero guard, multi-action exclusion, and that `setBuilderWindowSize` does not `emit()` +so the eager size update can't re-enter `renderAll` (the debounce genuinely debounces). + +## Things to Look At During PR Review + +- **The correctness invariant.** The load-bearing test is "the selected builder is always + on a rendered slot, for every cursor × window size 3 and 4" (`actions.test.ts`). That is + the property the bug violated; it directly encodes the fix's guarantee. +- **Immediate render vs debounced settle.** `willAppear` sizes the window and renders the + arriving key synchronously (so a press always resolves against the current layout), while + a debounced `renderAll` (`WINDOW_SETTLE_MS = 50`) coalesces the page-load burst. A rare + reload with a non-first builder already selected shows a ~50 ms transient face before the + settle corrects it — a deliberate trade to avoid flicker, called out here so it isn't read + as a bug. The debounce is on the full re-render, never on press resolution. +- **Retiring the manual `slot` field is user-visible.** Any `slot` value previously persisted + on a key is now ignored (it sits unused in settings) and the key re-orders by physical + position. This was raised and confirmed by the reviewer at plan-approval; no migration is + needed, but it is the one behavior change a returning user could notice. +- **Docs shipped with the fix on purpose.** The recommended 3-key layout would hide every + fourth builder under the old constant, so the README layout guidance and the code fix must + land together (owner-directed, folded into this lane rather than a separate PR). + +## How to Test Locally + +For a reviewer pulling the branch (hardware): + +```bash +pnpm --filter @cluesmith/codev-sdk build +pnpm --filter @cluesmith/codev-streamdeck build +cd apps/streamdeck +npx streamdeck unlink com.cluesmith.codev # it may be linked to another worktree +npx streamdeck link "$(pwd)/com.cluesmith.codev.sdPlugin" +npx streamdeck restart com.cluesmith.codev +npx streamdeck list # confirm it points at this worktree +``` + +Repro the fixed bug: place **3** Builder Action keys + an Open Architect key in the 4th +Row-1 slot, spawn 4+ builders, and rotate the Select dial onto the 4th builder — it now +renders on a key with the accent ring (before, it was selected but shown nowhere). Then +confirm a 4-key layout still pages/accents as before. + +Unit only: `cd apps/streamdeck && npm test`. diff --git a/codev/state/pir-1465_thread.md b/codev/state/pir-1465_thread.md new file mode 100644 index 000000000..c453de6f8 --- /dev/null +++ b/codev/state/pir-1465_thread.md @@ -0,0 +1,85 @@ +# pir-1465 — Stream Deck: size the Row-1 builder window from placed keys + +## Context +PIR lane for issue #1465. Correctness bug: Row-1 windowing pages by a hardcoded +`ROW1_WINDOW_SIZE = 4`, independent of how many `BuilderAction` keys the user placed. +With 3 keys against a page of 4, every builder at index ≡ 3 (mod 4) renders on NO key +while the Select dial still walks onto it — so it becomes the selected builder, drives +Row 2 + both review dials, and shows no accent ring anywhere. Triggered by #1463's +Open Architect key taking a Row-1 slot (3 builder keys left). + +## Phase: PLAN (2026-08-15) +Investigated the code. Verified SDK feasibility against `@elgato/streamdeck@2.1.0`: +- `KeyAction.coordinates: {column,row} | undefined` (undefined for multi-action) — confirmed + in `dist/plugin/actions/key.d.ts` + `dist/api/events/action.d.ts`. +- `Device.size {columns,rows}` + `Device.type`, `action.device`, `streamDeck.devices` — confirmed. +- No profile-structure API (profiles.d.ts: plugins "cannot access user-defined profiles"), + so layout must be DERIVED from lifecycle. Confirmed. + +Design decisions taken into the plan: +- Window size = count of currently-visible BuilderAction keys (replaces the constant). +- Slot order derived by sorting visible keys on (row, column); skip undefined-coord (multi-action). +- Debounce recompute across the page-load willAppear/willDisappear settle. +- **Retire the manual `slot` PI field** (argued, not defaulted — flagged for architect confirm as + the one irreversible/user-visible call). Physical order matches intent in ~all real layouts and + kills the duplicate-slot / gap misconfiguration class; keeping it as an override reintroduces + exactly that surface. +- Core invariant to test: the selected builder is ALWAYS on a rendered key (the bug violated this), + and cursor paging stays coherent when window size changes underneath a selection. +- Row 2 + dials + #1381 profile question OUT of scope. Device size/type available but not needed — + the visible-key count already encodes the physical layout. + +Plan written to codev/plans/1465-stream-deck-size-the-row-1-bui.md. Routing to architect before +plan-approval gate per instruction. dev-approval is a hardware session (3-key + 4-key + non-SD+). + +## Phase: IMPLEMENT (2026-08-15) — done, at dev-approval gate +Plan approved (architect confirmed retiring the manual slot field is fine). Implemented: +- store.ts: removed `ROW1_WINDOW_SIZE` constant; added private `builderWindowSize` (default 1) + + `setBuilderWindowSize()`; `builderWindowStart()` pages by it with a `max(1,·)` guard. +- actions.ts: `SlotKey` now captures `KeyAction.coordinates` per key; derives each key's slot by + sorting placed keys on (row, column); reports the count to the store; skips undefined-coord + (multi-action) instances. Window size set synchronously on willAppear/willDisappear (so a press + resolves against the current layout); a debounced (`WINDOW_SETTLE_MS=50`) full renderAll coalesces + the page-load burst. `slotBuilder()` + `settings.slot` retired; empty face labels by position. +- builder-action.html: dropped the Slot selector; kept the verb selector; rewrote help text. +- README.md: Row-1 diagram now shows 3 builders + Open Architect (the #1463 layout); prose says the + window sizes to the placed keys, keys self-order by position. +- Tests (actions.test.ts): rewrote the windowing blocks to drive via coordinates + placed-key count. + Added: the core INVARIANT (selected builder always on a rendered slot, for every cursor × size 3/4), + paging-follows-placed-count, multi-action exclusion, debounced-settle re-render, and + cursor-paging-coherent-under-size-change. + +Design note (for review): immediate render on willAppear is KEPT (correct in the common cursor≈0 +load; a rare later-selection load shows a ~50ms transient before the settle corrects it). The +debounce is on the full renderAll (the thrash the architect flagged), not on press resolution. + +Verified in worktree: `npm run build` ✓, `npm run check-types` (tsc) ✓, `npm test` ✓ (212 tests), +`npm run validate` ✓. dev-approval is the hardware session next. + +## Scope addition at dev-approval (owner-directed, 2026-08-15) +Owner wants the recommended SD+ layout to lead with the architect anchor. Updated README +"Recommended layout" section: Row 1 slot 1 = Open Architect Terminal in MAIN mode (fixed, +selection-independent anchor — that's WHY it can sit in Row 1 without breaking "Row 1 selects"), +slots 2-4 = three Builder Action selectors. Redrew the ASCII diagram, rewrote the Row-1 bullet +in placed-keys terms (never "4-wide"), dropped the stale "place Open Architect where a slot frees +up" note. Folded into this lane (no separate PR) because a 3-key Row 1 is only correct once the +window follows the placed keys — the doc and the fix must ship together. readme-design test ✓. +Committed 3bbf15782. Still at dev-approval gate. + +## Row 2 layout revision (owner-directed, 2026-08-15) +Owner revised recommended Row 2: slot 1 free, slot 2 Approve Gate, slot 3 Open Architect +Terminal in BUILDER mode (selected builder's owning architect — per-builder complement to +Row 1's main-mode anchor), slot 4 Open Builder Terminal. Updated ASCII diagram + Row-2 bullet. +Docs-only, readme-design test ✓. Committed 8c016a973. Still at dev-approval gate. + +## Review phase + 3-way consultation (2026-08-15) +PR #1468 opened. Consultation: gemini APPROVE, codex + claude REQUEST_CHANGES. All findings real +(doc/coverage around an approved core fix), all fixed on-branch: +- README Open Architect Terminal bullet still had the fixed-4 "hides every fourth builder" caveat + deferring to #1465 — the exact #1463-shipped text this issue removes; I missed it first pass. Rewrote. +- manifest.json Builder Action Tooltip still said "set the slot in the property inspector" (retired + field, user-visible in the SD app). Fixed to positional. +- Added positional-order test: 4 keys across 2 rows in reverse reading order → reading-order slots. +Non-blocking notes (WINDOW_SETTLE_MS module const, onWillDisappear no isKey guard) accepted, no change. +213 tests ✓, tsc ✓, build ✓, validate ✓. PIR is single-pass — documented dispositions in review file, +escalating to human at pr gate. Reviewers must verify the fixes there (no AI re-review).