From a85a97413b987983e3e56c121e3ab9e507005099 Mon Sep 17 00:00:00 2001 From: ivanmaierg Date: Tue, 12 May 2026 08:55:32 -0300 Subject: [PATCH 1/9] feat(tui): reader-reducer paging + cursor state and actions --- src/tui/reader/reader-reducer.test.ts | 173 +++++++++++++++++++++++--- src/tui/reader/reader-reducer.ts | 49 +++++++- 2 files changed, 199 insertions(+), 23 deletions(-) diff --git a/src/tui/reader/reader-reducer.test.ts b/src/tui/reader/reader-reducer.test.ts index ff8d550..1261fcd 100644 --- a/src/tui/reader/reader-reducer.test.ts +++ b/src/tui/reader/reader-reducer.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from "bun:test"; -import { readerReducer, initialReaderState } from "@/tui/reader/reader-reducer"; +import { readerReducer, initialReaderState, VERSES_PER_PAGE } from "@/tui/reader/reader-reducer"; import type { ReaderState, ReaderAction } from "@/tui/reader/reader-reducer"; import type { Passage } from "@/domain/passage"; import type { RepoError } from "@/domain/errors"; @@ -18,6 +18,24 @@ const mockPassage: Passage = { const networkError: RepoError = { kind: "network", message: "unreachable" }; +function makePassage(count: number): Passage { + return { + reference: johnRef, + verses: Array.from({ length: count }, (_, i) => ({ + number: i + 1, + text: `Verse ${i + 1}`, + })), + }; +} + +function makeLoaded( + passage: Passage, + cursorIndex: number, + pageStartIndex: number, +): ReaderState { + return { kind: "loaded", passage, ref: johnRef, cursorIndex, pageStartIndex }; +} + function dispatch(state: ReaderState, action: ReaderAction): ReaderState { return readerReducer(state, action); } @@ -33,6 +51,12 @@ describe("readerReducer", () => { }); }); + describe("VERSES_PER_PAGE", () => { + it("equals 8", () => { + expect(VERSES_PER_PAGE).toBe(8); + }); + }); + describe("QueryTyped", () => { it("updates query and clears parseError when awaiting", () => { const state: ReaderState = { kind: "awaiting", query: "", parseError: { kind: "empty_input" } }; @@ -82,14 +106,20 @@ describe("readerReducer", () => { }); describe("PassageFetched", () => { - it("transitions loading → loaded", () => { + it("transitions loading → loaded with cursorIndex: 0, pageStartIndex: 0", () => { const state: ReaderState = { kind: "loading", ref: johnRef }; const next = dispatch(state, { type: "PassageFetched", passage: mockPassage }); - expect(next).toEqual({ kind: "loaded", passage: mockPassage, ref: johnRef }); + expect(next).toEqual({ + kind: "loaded", + passage: mockPassage, + ref: johnRef, + cursorIndex: 0, + pageStartIndex: 0, + }); }); it("is a no-op when not loading", () => { - const state: ReaderState = { kind: "loaded", passage: mockPassage, ref: johnRef }; + const state = makeLoaded(mockPassage, 0, 0); const next = dispatch(state, { type: "PassageFetched", passage: mockPassage }); expect(next).toBe(state); }); @@ -103,7 +133,7 @@ describe("readerReducer", () => { }); it("is a no-op when not loading", () => { - const state: ReaderState = { kind: "loaded", passage: mockPassage, ref: johnRef }; + const state = makeLoaded(mockPassage, 0, 0); const next = dispatch(state, { type: "FetchFailed", ref: johnRef, reason: networkError }); expect(next).toBe(state); }); @@ -111,7 +141,7 @@ describe("readerReducer", () => { describe("ChapterAdvanced", () => { it("transitions loaded → loading with chapter + 1", () => { - const state: ReaderState = { kind: "loaded", passage: mockPassage, ref: johnRef }; + const state = makeLoaded(mockPassage, 0, 0); const next = dispatch(state, { type: "ChapterAdvanced" }); expect(next.kind).toBe("loading"); if (next.kind !== "loading") return; @@ -134,29 +164,31 @@ describe("readerReducer", () => { const next = dispatch(state, { type: "ChapterAdvanced" }); expect(next).toBe(state); }); + + it("loaded → loading carries no cursorIndex or pageStartIndex", () => { + const state = makeLoaded(mockPassage, 2, 0); + const next = dispatch(state, { type: "ChapterAdvanced" }); + expect(next.kind).toBe("loading"); + expect((next as any).cursorIndex).toBeUndefined(); + expect((next as any).pageStartIndex).toBeUndefined(); + }); }); describe("ChapterRetreated", () => { it("transitions loaded → loading with chapter - 1 when chapter > 1", () => { - const state: ReaderState = { - kind: "loaded", - passage: mockPassage, - ref: { ...johnRef, chapter: 5 }, - }; - const next = dispatch(state, { type: "ChapterRetreated" }); + const state = makeLoaded(mockPassage, 0, 0); + const adjustedState = { ...state, ref: { ...johnRef, chapter: 5 } } as ReaderState; + const next = dispatch(adjustedState, { type: "ChapterRetreated" }); expect(next.kind).toBe("loading"); if (next.kind !== "loading") return; expect(next.ref.chapter).toBe(4); }); it("is a no-op when chapter === 1 (floor)", () => { - const state: ReaderState = { - kind: "loaded", - passage: mockPassage, - ref: { ...johnRef, chapter: 1 }, - }; - const next = dispatch(state, { type: "ChapterRetreated" }); - expect(next).toBe(state); + const state = makeLoaded(mockPassage, 0, 0); + const adjustedState = { ...state, ref: { ...johnRef, chapter: 1 } } as ReaderState; + const next = dispatch(adjustedState, { type: "ChapterRetreated" }); + expect(next).toBe(adjustedState); }); it("is a no-op from awaiting", () => { @@ -173,7 +205,7 @@ describe("readerReducer", () => { describe("PaletteReopened", () => { it("transitions loaded → awaiting with cleared query", () => { - const state: ReaderState = { kind: "loaded", passage: mockPassage, ref: johnRef }; + const state = makeLoaded(mockPassage, 0, 0); const next = dispatch(state, { type: "PaletteReopened" }); expect(next).toEqual({ kind: "awaiting", query: "", parseError: null }); }); @@ -195,4 +227,105 @@ describe("readerReducer", () => { expect(next).toBe(state); }); }); + + describe("new actions are no-ops outside loaded", () => { + const newActions: ReaderAction[] = [ + { type: "CursorMovedUp" }, + { type: "CursorMovedDown" }, + { type: "PageAdvanced" }, + { type: "PageRetreated" }, + ]; + + const nonLoadedStates: ReaderState[] = [ + { kind: "awaiting", query: "", parseError: null }, + { kind: "loading", ref: johnRef }, + { kind: "network-error", ref: johnRef, reason: networkError }, + ]; + + for (const action of newActions) { + for (const state of nonLoadedStates) { + it(`${action.type} is a no-op when state.kind === "${state.kind}"`, () => { + expect(dispatch(state, action)).toBe(state); + }); + } + } + }); + + describe("CursorMovedDown", () => { + it("increments cursorIndex within page", () => { + const p = makePassage(10); + const state = makeLoaded(p, 0, 0); + const next = dispatch(state, { type: "CursorMovedDown" }); + expect(next).toEqual(makeLoaded(p, 1, 0)); + }); + + it("advances page and resets cursor at page boundary", () => { + const p = makePassage(16); + const state = makeLoaded(p, 7, 0); + const next = dispatch(state, { type: "CursorMovedDown" }); + expect(next).toEqual(makeLoaded(p, 8, 8)); + }); + + it("clamps at last verse of last page", () => { + const p = makePassage(10); + const state = makeLoaded(p, 9, 8); + const next = dispatch(state, { type: "CursorMovedDown" }); + expect(next).toBe(state); + }); + }); + + describe("CursorMovedUp", () => { + it("decrements cursorIndex within page", () => { + const p = makePassage(10); + const state = makeLoaded(p, 3, 0); + const next = dispatch(state, { type: "CursorMovedUp" }); + expect(next).toEqual(makeLoaded(p, 2, 0)); + }); + + it("retreats page and sets cursor to last verse of retreated page", () => { + const p = makePassage(16); + const state = makeLoaded(p, 8, 8); + const next = dispatch(state, { type: "CursorMovedUp" }); + expect(next).toEqual(makeLoaded(p, 7, 0)); + }); + + it("clamps at first verse of first page", () => { + const p = makePassage(10); + const state = makeLoaded(p, 0, 0); + const next = dispatch(state, { type: "CursorMovedUp" }); + expect(next).toBe(state); + }); + }); + + describe("PageAdvanced", () => { + it("advances to next page and sets cursorIndex to pageStartIndex", () => { + const p = makePassage(16); + const state = makeLoaded(p, 3, 0); + const next = dispatch(state, { type: "PageAdvanced" }); + expect(next).toEqual(makeLoaded(p, 8, 8)); + }); + + it("clamps at last page", () => { + const p = makePassage(10); + const state = makeLoaded(p, 9, 8); + const next = dispatch(state, { type: "PageAdvanced" }); + expect(next).toBe(state); + }); + }); + + describe("PageRetreated", () => { + it("retreats to previous page and sets cursorIndex to new pageStartIndex", () => { + const p = makePassage(16); + const state = makeLoaded(p, 10, 8); + const next = dispatch(state, { type: "PageRetreated" }); + expect(next).toEqual(makeLoaded(p, 0, 0)); + }); + + it("clamps at first page", () => { + const p = makePassage(10); + const state = makeLoaded(p, 2, 0); + const next = dispatch(state, { type: "PageRetreated" }); + expect(next).toBe(state); + }); + }); }); diff --git a/src/tui/reader/reader-reducer.ts b/src/tui/reader/reader-reducer.ts index 6e7c801..c37e365 100644 --- a/src/tui/reader/reader-reducer.ts +++ b/src/tui/reader/reader-reducer.ts @@ -3,10 +3,12 @@ import type { Reference } from "@/domain/reference"; import type { ParseError, RepoError } from "@/domain/errors"; import type { Passage } from "@/domain/passage"; +export const VERSES_PER_PAGE = 8; + export type ReaderState = | { kind: "awaiting"; query: string; parseError: ParseError | null } | { kind: "loading"; ref: Reference } - | { kind: "loaded"; passage: Passage; ref: Reference } + | { kind: "loaded"; passage: Passage; ref: Reference; cursorIndex: number; pageStartIndex: number } | { kind: "network-error"; ref: Reference; reason: RepoError }; export type ReaderAction = @@ -16,7 +18,11 @@ export type ReaderAction = | { type: "FetchFailed"; ref: Reference; reason: RepoError } | { type: "ChapterAdvanced" } | { type: "ChapterRetreated" } - | { type: "PaletteReopened" }; + | { type: "PaletteReopened" } + | { type: "CursorMovedUp" } + | { type: "CursorMovedDown" } + | { type: "PageAdvanced" } + | { type: "PageRetreated" }; const handlers = { QueryTyped: (s: ReaderState, a: Extract): ReaderState => @@ -32,7 +38,7 @@ const handlers = { PassageFetched: (s: ReaderState, a: Extract): ReaderState => s.kind === "loading" - ? { kind: "loaded", passage: a.passage, ref: s.ref } + ? { kind: "loaded", passage: a.passage, ref: s.ref, cursorIndex: 0, pageStartIndex: 0 } : s, FetchFailed: (s: ReaderState, a: Extract): ReaderState => @@ -55,6 +61,43 @@ const handlers = { s.kind === "loaded" || s.kind === "network-error" ? { kind: "awaiting", query: "", parseError: null } : s, + + CursorMovedDown: (s: ReaderState, _a: Extract): ReaderState => { + if (s.kind !== "loaded") return s; + const verses = s.passage.verses; + const pageEnd = Math.min(s.pageStartIndex + VERSES_PER_PAGE - 1, verses.length - 1); + if (s.cursorIndex < pageEnd) { + return { ...s, cursorIndex: s.cursorIndex + 1 }; + } + const nextPageStart = s.pageStartIndex + VERSES_PER_PAGE; + if (nextPageStart >= verses.length) return s; + return { ...s, pageStartIndex: nextPageStart, cursorIndex: nextPageStart }; + }, + + CursorMovedUp: (s: ReaderState, _a: Extract): ReaderState => { + if (s.kind !== "loaded") return s; + if (s.cursorIndex === 0 && s.pageStartIndex === 0) return s; + if (s.cursorIndex > s.pageStartIndex) { + return { ...s, cursorIndex: s.cursorIndex - 1 }; + } + const newPageStart = s.pageStartIndex - VERSES_PER_PAGE; + const lastVerseOfPage = Math.min(newPageStart + VERSES_PER_PAGE - 1, s.passage.verses.length - 1); + return { ...s, pageStartIndex: newPageStart, cursorIndex: lastVerseOfPage }; + }, + + PageAdvanced: (s: ReaderState, _a: Extract): ReaderState => { + if (s.kind !== "loaded") return s; + const nextPageStart = s.pageStartIndex + VERSES_PER_PAGE; + if (nextPageStart >= s.passage.verses.length) return s; + return { ...s, pageStartIndex: nextPageStart, cursorIndex: nextPageStart }; + }, + + PageRetreated: (s: ReaderState, _a: Extract): ReaderState => { + if (s.kind !== "loaded") return s; + if (s.pageStartIndex === 0) return s; + const newPageStart = s.pageStartIndex - VERSES_PER_PAGE; + return { ...s, pageStartIndex: newPageStart, cursorIndex: newPageStart }; + }, } satisfies { [K in ReaderAction["type"]]: ( state: ReaderState, From 8764733dc29eab8d2fea7f290ca17c18606adff3 Mon Sep 17 00:00:00 2001 From: ivanmaierg Date: Tue, 12 May 2026 08:56:03 -0300 Subject: [PATCH 2/9] feat(tui): reader-screen page slice + cursor gutter + inverse selection --- src/tui/reader/reader-screen.tsx | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/src/tui/reader/reader-screen.tsx b/src/tui/reader/reader-screen.tsx index 74de21b..140ee35 100644 --- a/src/tui/reader/reader-screen.tsx +++ b/src/tui/reader/reader-screen.tsx @@ -1,12 +1,15 @@ import { useState, useEffect } from "react"; import { TextAttributes } from "@opentui/core"; import { SPINNER_FRAMES } from "@/cli/loading"; +import { ACCENT_HEX } from "@/presentation/colors"; import { usePassageFetch } from "@/tui/reader/use-passage-fetch"; +import { VERSES_PER_PAGE } from "@/tui/reader/reader-reducer"; import type { BibleRepository } from "@/application/ports/bible-repository"; import type { ReaderState, ReaderAction } from "@/tui/reader/reader-reducer"; import type { Dispatch } from "react"; const DIM = TextAttributes.DIM; +const INVERSE = TextAttributes.INVERSE; type ReaderScreenProps = { state: ReaderState; @@ -58,7 +61,7 @@ function bottomTitleFor(state: ReaderState): string { case "loading": return " loading… • q quit "; case "loaded": - return " ] next ch • [ prev ch • / palette • q quit "; + return " ↑↓ verse • [ ] page • n p chapter • / palette • q quit "; case "network-error": return " / palette • q quit "; } @@ -106,14 +109,24 @@ function Body({ state, dispatch, frame }: BodyProps) { ); } + const { passage, cursorIndex, pageStartIndex } = state; + const pageVerses = passage.verses.slice(pageStartIndex, pageStartIndex + VERSES_PER_PAGE); + return ( - {state.passage.verses.map((v) => ( - - {`${String(v.number).padStart(3)} `} - {v.text} - - ))} + {pageVerses.map((v, i) => { + const idx = pageStartIndex + i; + const focused = idx === cursorIndex; + return ( + + + {focused ? "▶" : " "} + + {`${String(v.number).padStart(3)} `} + {v.text} + + ); + })} ); } From 70324960fc99413f4c0e7a24ed1f785c6f461927 Mon Sep 17 00:00:00 2001 From: ivanmaierg Date: Tue, 12 May 2026 08:56:31 -0300 Subject: [PATCH 3/9] feat(tui): keybind rebinding + awaiting gate fix; welcome hint --- src/tui/tui-driver.tsx | 21 +++++++++------------ src/tui/welcome/welcome-screen.tsx | 2 +- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/tui/tui-driver.tsx b/src/tui/tui-driver.tsx index e5ef68a..327ef71 100644 --- a/src/tui/tui-driver.tsx +++ b/src/tui/tui-driver.tsx @@ -32,18 +32,15 @@ function App({ setPhase("reader"); return; } - if (keyEvent.name === "]") { - dispatch({ type: "ChapterAdvanced" }); - return; - } - if (keyEvent.name === "[") { - dispatch({ type: "ChapterRetreated" }); - return; - } - if (keyEvent.name === "/") { - dispatch({ type: "PaletteReopened" }); - return; - } + if (readerState.kind === "awaiting") return; + + if (keyEvent.name === "up") { dispatch({ type: "CursorMovedUp" }); return; } + if (keyEvent.name === "down") { dispatch({ type: "CursorMovedDown" }); return; } + if (keyEvent.name === "[") { dispatch({ type: "PageRetreated" }); return; } + if (keyEvent.name === "]") { dispatch({ type: "PageAdvanced" }); return; } + if (keyEvent.name === "n") { dispatch({ type: "ChapterAdvanced" }); return; } + if (keyEvent.name === "p") { dispatch({ type: "ChapterRetreated" }); return; } + if (keyEvent.name === "/") { dispatch({ type: "PaletteReopened" }); return; } }); if (phase === "welcome") { diff --git a/src/tui/welcome/welcome-screen.tsx b/src/tui/welcome/welcome-screen.tsx index 102755b..8efd954 100644 --- a/src/tui/welcome/welcome-screen.tsx +++ b/src/tui/welcome/welcome-screen.tsx @@ -71,7 +71,7 @@ export function WelcomeScreen({ {RIBBON_3} {" "} - {" any key to start • / palette • ] next ch • [ prev ch • q quit"} + {" any key to start • ↑↓ verse • [ ] page • n p chapter • / palette • q quit"} ); } From cb749dc2779b2654fd6f32cc2e7563a21929c567 Mon Sep 17 00:00:00 2001 From: ivanmaierg Date: Tue, 12 May 2026 09:00:34 -0300 Subject: [PATCH 4/9] docs(openspec): add SDD trail + verify report for tui-reader-paging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Captures explore/proposal/spec/tasks/verify-report for the paging + verse cursor change. Verify verdict: PASS WITH WARNINGS (0 critical / 1 warning / 1 suggestion). 151/151 tests green, tsc clean. Pending: manual PTY smoke for visual cursor + keybind round-trips. Suggestion to verify at smoke: gutter ▶ uses fg={ACCENT_HEX} AND attributes={DIM} together — DIM may suppress accent color on some terminals. --- openspec/changes/tui-reader-paging/explore.md | 224 ++++++++++++++ .../changes/tui-reader-paging/proposal.md | 161 ++++++++++ openspec/changes/tui-reader-paging/spec.md | 282 ++++++++++++++++++ openspec/changes/tui-reader-paging/tasks.md | 114 +++++++ .../tui-reader-paging/verify-report.md | 102 +++++++ 5 files changed, 883 insertions(+) create mode 100644 openspec/changes/tui-reader-paging/explore.md create mode 100644 openspec/changes/tui-reader-paging/proposal.md create mode 100644 openspec/changes/tui-reader-paging/spec.md create mode 100644 openspec/changes/tui-reader-paging/tasks.md create mode 100644 openspec/changes/tui-reader-paging/verify-report.md diff --git a/openspec/changes/tui-reader-paging/explore.md b/openspec/changes/tui-reader-paging/explore.md new file mode 100644 index 0000000..1b7d4fa --- /dev/null +++ b/openspec/changes/tui-reader-paging/explore.md @@ -0,0 +1,224 @@ +# Exploration: tui-reader-paging + +## Current State + +PR #10 (`tui-reader-screen`) shipped: + +- `reader-reducer.ts` — state union: `awaiting | loading | loaded | network-error`. Current `loaded` shape: `{ kind: "loaded"; passage: Passage; ref: Reference }`. +- `reader-screen.tsx` — renders all passage verses in a single bordered frame, no cursor, no paging. +- `tui-driver.tsx` — `useKeyboard` binds `]`/`[` → `ChapterAdvanced`/`ChapterRetreated`, `/` → `PaletteReopened`. No guard against `awaiting` state (bug confirmed). +- `welcome-screen.tsx` — hint line: `" any key to start • / palette • ] next ch • [ prev ch • q quit"` + +The change required is a UX extension: verse cursor navigation + chapter paging within the reader, plus a keybind reshuffle and a driver-level state gate fix. + +## Arrow Key Name Discovery + +From `node_modules/@opentui/core/testing/mock-keys.d.ts`: + +- `ARROW_UP` → escape sequence `\x1B[A` +- `ARROW_DOWN` → escape sequence `\x1B[B` +- `pressArrow(direction: "up" | "down" | "left" | "right")` — confirms the `keyEvent.name` strings. + +**Confirmed**: `keyEvent.name === "up"` and `keyEvent.name === "down"`. + +## Affected Areas + +- `src/tui/reader/reader-reducer.ts` — extend `loaded` variant, add 4 new actions +- `src/tui/reader/reader-reducer.test.ts` — add tests for all new transitions +- `src/tui/reader/reader-screen.tsx` — filter verses to current page, add `▶` cursor gutter marker, update `bottomTitleFor` +- `src/tui/tui-driver.tsx` — rebind `[`/`]` to page nav, add `n`/`p` for chapter nav, gate reader keybinds, add arrow handlers +- `src/tui/welcome/welcome-screen.tsx` — update hint line with new keybind scheme + +## State Machine Extensions + +### New `loaded` variant + +```ts +| { + kind: "loaded"; + passage: Passage; + ref: Reference; + cursorIndex: number; // 0-based array index of focused verse in passage.verses + pageStartIndex: number; // 0-based array index of first verse on current page + } +``` + +**Index-based, NOT verse-number-based** — passage.verses verse numbers are not guaranteed contiguous (a passage starting at John 3:14 has verses[0].number === 14). Index-based state keeps page arithmetic correct on any passage slice. + +`loading` and `network-error` variants are UNCHANGED — no cursor carry-over needed. + +### New actions + +```ts +| { type: "CursorMovedUp" } +| { type: "CursorMovedDown" } +| { type: "PageAdvanced" } +| { type: "PageRetreated" } +``` + +Existing `ChapterAdvanced` / `ChapterRetreated` keep their names and reducer logic — only their driver-level keybind changes from `]`/`[` to `n`/`p`. + +### `PassageFetched` handler update + +Set `cursorIndex: 0, pageStartIndex: 0`. Applies for both initial fetch and chapter-transition fetches. + +## Page Boundaries Logic + +```ts +export const VERSES_PER_PAGE = 8; +``` + +Rationale: 8 fits comfortably in a 20-row minimum terminal. Dynamic computation from terminal height is a future optimization. + +**Page slice:** `verses.slice(pageStartIndex, pageStartIndex + VERSES_PER_PAGE)` + +- `PageAdvanced` at last page: **clamp** silently — no wrap, no chapter cascade. +- `PageRetreated` at first page: **clamp** silently. +- `CursorMovedDown` at last verse on current page: advance to next page, cursor on first verse of new page. At last verse of last page: clamp. +- `CursorMovedUp` at first verse on current page: retreat to previous page, cursor on last verse of previous page. At first verse of first page: clamp. + +## Reducer Handler Sketches + +All handlers no-op when `s.kind !== "loaded"`. + +```ts +CursorMovedDown: (s, _a) => { + if (s.kind !== "loaded") return s; + const total = s.passage.verses.length; + const pageEnd = Math.min(s.pageStartIndex + VERSES_PER_PAGE - 1, total - 1); + if (s.cursorIndex < pageEnd) { + return { ...s, cursorIndex: s.cursorIndex + 1 }; + } + const nextPageStart = s.pageStartIndex + VERSES_PER_PAGE; + if (nextPageStart >= total) return s; // clamp at last page + return { ...s, pageStartIndex: nextPageStart, cursorIndex: nextPageStart }; +}, + +CursorMovedUp: (s, _a) => { + if (s.kind !== "loaded") return s; + if (s.cursorIndex > s.pageStartIndex) { + return { ...s, cursorIndex: s.cursorIndex - 1 }; + } + const prevPageStart = s.pageStartIndex - VERSES_PER_PAGE; + if (prevPageStart < 0) return s; // clamp at first page + const prevPageEnd = Math.min(prevPageStart + VERSES_PER_PAGE - 1, s.passage.verses.length - 1); + return { ...s, pageStartIndex: prevPageStart, cursorIndex: prevPageEnd }; +}, + +PageAdvanced: (s, _a) => { + if (s.kind !== "loaded") return s; + const nextPageStart = s.pageStartIndex + VERSES_PER_PAGE; + if (nextPageStart >= s.passage.verses.length) return s; + return { ...s, pageStartIndex: nextPageStart, cursorIndex: nextPageStart }; +}, + +PageRetreated: (s, _a) => { + if (s.kind !== "loaded") return s; + const prevPageStart = s.pageStartIndex - VERSES_PER_PAGE; + if (prevPageStart < 0) return s; + return { ...s, pageStartIndex: prevPageStart, cursorIndex: prevPageStart }; +}, +``` + +## View Changes + +```tsx +const pageVerses = state.passage.verses.slice( + state.pageStartIndex, + state.pageStartIndex + VERSES_PER_PAGE, +); + +{pageVerses.map((v, i) => { + const verseIndex = state.pageStartIndex + i; + const isFocused = verseIndex === state.cursorIndex; + return ( + + + {isFocused ? "▶ " : " "} + + {`${String(v.number).padStart(3)} `} + + {v.text} + + + ); +})} +``` + +Per `ui-sketches.md` Typography: focused verse uses `▶` marker (in accent color) + `selection` (`TextAttributes.INVERSE`) on the line. + +**`bottomTitleFor` for loaded state:** +```ts +" ↑↓ verse • [ ] page • n p chapter • / palette • q quit " +``` + +## Driver-Level Keybind Changes + +```ts +useKeyboard((keyEvent) => { + if (keyEvent.name === "q" || keyEvent.name === "Q") { + renderer.destroy(); resolve(); return; + } + if (phase === "welcome") { + setPhase("reader"); return; + } + if (readerState.kind === "awaiting") return; + + if (keyEvent.name === "up") { dispatch({ type: "CursorMovedUp" }); return; } + if (keyEvent.name === "down") { dispatch({ type: "CursorMovedDown" }); return; } + if (keyEvent.name === "[") { dispatch({ type: "PageRetreated" }); return; } + if (keyEvent.name === "]") { dispatch({ type: "PageAdvanced" }); return; } + if (keyEvent.name === "n") { dispatch({ type: "ChapterAdvanced" }); return; } + if (keyEvent.name === "p") { dispatch({ type: "ChapterRetreated" }); return; } + if (keyEvent.name === "/") { dispatch({ type: "PaletteReopened" }); return; } +}); +``` + +The existing palette-key conflict (`/` clearing query while input focused) is fixed by the `awaiting`-state gate. `q`/`Q` sits above the guard — quit always works. + +## Welcome Screen Hint Update + +```ts +" any key to start • ↑↓ verse • [ ] page • n p chapter • / palette • q quit" +``` + +## Slicing Recommendation + +**One PR.** Estimate: + +| File | Lines changed | +|---|---| +| `reader-reducer.ts` | ~35 | +| `reader-reducer.test.ts` | ~60 | +| `reader-screen.tsx` | ~25 | +| `tui-driver.tsx` | ~15 | +| `welcome-screen.tsx` | ~1 | +| **Total** | **~136 lines** | + +Well under the 400-line review budget. + +## Approaches + +| Approach | Pros | Cons | Effort | +|---|---|---|---| +| **A — One PR (recommended)** | Single coherent UX change; under budget; reducer + view ship together | None significant | Low | +| B — Split: reducer PR then view PR | Smaller diffs | No observable UX until view lands; artificial split | Medium overhead | +| C — Dynamic page size from terminal rows | Adaptive UX | More complex; deferred to future change | Medium | + +**Recommendation: Approach A** with `VERSES_PER_PAGE = 8` constant, index-based state. + +## Risks + +- **Index vs. verse-number state** — verse numbers in `passage.verses` are not guaranteed contiguous (a passage starting at John 3:14 has `verses[0].number === 14`). Page arithmetic must use array indices, not verse numbers. Locked: index-based. +- **`useKeyboard` always fires** — the `awaiting`-state gate suppresses keybinds while `` is focused. The guard must sit BELOW the `q`/`Q` check so quit always works. +- **`TextAttributes.INVERSE`** — confirm exported from `@opentui/core` (only `DIM` used in codebase today). Trivial check at impl time. + +## Open Questions for Proposal + +1. **Index-based vs verse-number-based state** — **LOCKED: index-based.** Verse numbers aren't contiguous in passage slices. +2. **`▶` marker color** — **LOCKED: ACCENT_HEX.** Per `ui-sketches.md` Typography table. +3. **`PageAdvanced` at last page behavior** — **LOCKED: silent clamp.** No visual feedback, no chapter cascade. + +## Ready for Proposal + +Yes. All decisions resolved with defensible defaults. diff --git a/openspec/changes/tui-reader-paging/proposal.md b/openspec/changes/tui-reader-paging/proposal.md new file mode 100644 index 0000000..38f02f2 --- /dev/null +++ b/openspec/changes/tui-reader-paging/proposal.md @@ -0,0 +1,161 @@ +# Proposal: tui-reader-paging + +## TL;DR + +- The reader screen currently renders all passage verses in a single unbounded list with no cursor — only one verse is visible on first paint due to terminal scroll behavior. +- This change adds verse cursor navigation (`↑`/`↓`), page navigation (`[`/`]`), and rebinds chapter navigation to `n`/`p`. +- State is extended with `cursorIndex` and `pageStartIndex` (0-based array indices into `passage.verses`) — index-based to handle non-contiguous verse numbers. +- Fixes an existing driver bug: `/` fires `PaletteReopened` while the palette `` is focused, because no `awaiting`-state gate existed. +- Ships as one PR, ~136 lines — well under the 400-line review budget. + +--- + +## Why + +The reader screen shipped with PR #10 (`tui-reader-screen`) renders `passage.verses` in full without any scrolling, cursor, or paging. In practice only the first verse is visible on first paint — the TUI renders a list taller than the terminal frame and the rest is cropped. The user cannot navigate or read the passage. + +The request is to make the reader usable: show 8 verses per page, let the user move a cursor line by line, and page through the passage. Chapter navigation is retained but moves to `n`/`p` to free `[`/`]` for page navigation. + +Additionally, the `awaiting`-state gate fixes a confirmed bug: pressing `/` while the palette `` is active re-dispatches `PaletteReopened`, clearing the user's in-progress query. + +--- + +## What Changes + +| File | Change | +|---|---| +| `src/tui/reader/reader-reducer.ts` | Extend `loaded` variant with `cursorIndex`/`pageStartIndex`; add `CursorMovedUp`, `CursorMovedDown`, `PageAdvanced`, `PageRetreated` actions; update `PassageFetched` handler | +| `src/tui/reader/reader-reducer.test.ts` | RED-first tests for all new actions and the updated `loaded` shape; existing tests updated for new shape | +| `src/tui/reader/reader-screen.tsx` | Render page slice (`verses.slice(pageStartIndex, pageStartIndex + VERSES_PER_PAGE)`); `▶` gutter marker in `ACCENT_HEX`; `TextAttributes.INVERSE` on focused verse text; update `bottomTitleFor` | +| `src/tui/tui-driver.tsx` | `awaiting`-state gate; bind `↑`/`↓` to cursor actions; rebind `[`/`]` to page nav; add `n`/`p` for chapter nav | +| `src/tui/welcome/welcome-screen.tsx` | Update hint line to reflect new keybind scheme | + +--- + +## What Does NOT Change + +- **Domain layer**: `parseReference` and all reference parsing logic — untouched. +- **Application layer**: `getPassage` use case — untouched. +- **API adapter**: `BibleApiAdapter` / `EsvApiAdapter` — untouched. +- **CLI entry point**: `run` / `vod` commands — untouched. +- **ADR 0010 dialect**: existing reducer action names (`ChapterAdvanced`, `ChapterRetreated`, `PassageFetched`, `PaletteReopened`) are retained. The keybinds that trigger them change in the driver, not the reducer. +- **`loading` and `network-error` state variants**: no cursor fields added — chapter transitions go through `loading → loaded`, which resets to `cursorIndex: 0, pageStartIndex: 0` via the `PassageFetched` handler. + +--- + +## State Machine Extension + +### New `loaded` variant shape + +```ts +| { + kind: "loaded"; + passage: Passage; + ref: Reference; + cursorIndex: number; // 0-based array index of focused verse in passage.verses + pageStartIndex: number; // 0-based array index of first verse on current page + } +``` + +Index-based rationale: `passage.verses[0].number` is not guaranteed to be `1`. A passage like John 3:14–18 has `verses[0].number === 14`. Storing array indices keeps all page arithmetic correct on any passage slice. + +### PassageFetched handler + +```ts +PassageFetched: (s, a) => + s.kind === "loading" + ? { kind: "loaded", passage: a.passage, ref: s.ref, cursorIndex: 0, pageStartIndex: 0 } + : s, +``` + +### New action union members + +```ts +| { type: "CursorMovedUp" } +| { type: "CursorMovedDown" } +| { type: "PageAdvanced" } +| { type: "PageRetreated" } +``` + +### Boundary behavior + +| Action | Boundary | Behavior | +|---|---|---| +| `CursorMovedDown` | Last verse, last page | Clamp — no-op | +| `CursorMovedDown` | Last verse, not last page | Advance page; cursor = `pageStartIndex` of new page | +| `CursorMovedUp` | First verse, first page | Clamp — no-op | +| `CursorMovedUp` | First verse, not first page | Retreat page; cursor = last verse of retreated page | +| `PageAdvanced` | Last page | Clamp silently | +| `PageRetreated` | First page | Clamp silently | + +--- + +## Keybind Map + +| Key | Action dispatched | Notes | +|---|---|---| +| `↑` (`keyEvent.name === "up"`) | `CursorMovedUp` | New | +| `↓` (`keyEvent.name === "down"`) | `CursorMovedDown` | New | +| `[` | `PageRetreated` | Rebound — was `ChapterRetreated` in PR #10 | +| `]` | `PageAdvanced` | Rebound — was `ChapterAdvanced` in PR #10 | +| `n` | `ChapterAdvanced` | New keybind for existing action | +| `p` | `ChapterRetreated` | New keybind for existing action | +| `/` | `PaletteReopened` | Unchanged; bug fixed by `awaiting` gate | +| `q` / `Q` | quit | Unchanged; sits above `awaiting` gate | + +--- + +## First Reviewable Cut + +One PR, ~136 lines estimated: + +| File | Lines | +|---|---| +| `reader-reducer.ts` | ~35 | +| `reader-reducer.test.ts` | ~60 | +| `reader-screen.tsx` | ~25 | +| `tui-driver.tsx` | ~15 | +| `welcome-screen.tsx` | ~1 | +| **Total** | **~136** | + +--- + +## Success Criterion + +End-to-end acceptance path: + +1. `verbum` → welcome screen renders +2. Any key → reader screen, palette opens +3. Type `john 3`, Enter → spinner → page 1 shows verses 1–8 with `▶` on verse 1 (inverse styling) +4. `↓` × 4 → cursor advances to verse 5; `▶` marker and inverse move down accordingly +5. `]` → page 2 renders (verses 9–16); cursor lands on verse 9 +6. `[` → back to page 1; cursor lands on verse 1 +7. `n` → spinner → John 4 page 1 (cursor reset to index 0) +8. `p` → spinner → John 3 page 1 (cursor reset to index 0) +9. `/` → palette opens with empty query field, no interference from `awaiting` gate +10. `q` → exits cleanly from any state + +--- + +## Risks + +- **`TextAttributes.INVERSE` not yet used in codebase**: `ui-sketches.md` calls for it as the `selection` style, but only `TextAttributes.DIM` is referenced today. Must confirm `INVERSE` is exported from the installed `@opentui/core` version before implementing. Trivial import check at spec/design time. +- **Arrow key name verification**: `keyEvent.name === "up"` / `"down"` confirmed from `mock-keys.d.ts` and the runtime bundle. Worth re-verifying at impl against the actual running environment — OpenTUI's key names have been stable but this is the first time arrow keys are bound in this project. +- **`[` / `]` keybind rebind**: Any user muscle memory or documentation that references `[`/`]` as chapter nav (from PR #10) will be broken. The welcome hint line and `bottomTitleFor` are both updated, but external docs (README, etc.) are out of scope for this change. + +--- + +## Out of Scope + +- **Dynamic `VERSES_PER_PAGE`** from `process.stdout.rows` — future change. +- **Cross-chapter cursor flow** (advancing past the last verse of a page auto-triggers `ChapterAdvanced`) — future change; keeps page and chapter nav strictly separate for now. +- **Visual feedback on page-boundary clamp** — future change; silent clamp is sufficient for v1. +- **Verse range passages** (e.g. `john 3:14-18`) — the index-based state handles them correctly but no special UX is added. + +--- + +## Next Steps (possible follow-ups) + +- **Dynamic page size**: compute `VERSES_PER_PAGE` from `process.stdout.rows` minus chrome rows; requires polling or a resize event. +- **Cross-chapter cursor flow**: when `CursorMovedDown` hits the last verse of the last page, dispatch `ChapterAdvanced` instead of clamping. +- **Page indicator in bottom title**: e.g. `"page 1 / 4"` inline with the status bar. diff --git a/openspec/changes/tui-reader-paging/spec.md b/openspec/changes/tui-reader-paging/spec.md new file mode 100644 index 0000000..937ab20 --- /dev/null +++ b/openspec/changes/tui-reader-paging/spec.md @@ -0,0 +1,282 @@ +# Spec: tui-reader-paging + +## 1. Capability + +TUI Reader paging + verse cursor — extends the `loaded` state with index-based cursor and pageStart, adds 4 new actions, and rebinds chapter navigation keys. + +--- + +## 2. Requirements + +### State Shape + +**REQ-1** — The `loaded` state variant MUST include two new fields: +- `cursorIndex: number` — 0-based array index of the focused verse in `passage.verses` +- `pageStartIndex: number` — 0-based array index of the first verse on the current page + +**REQ-2** — The `loading`, `awaiting`, and `network-error` state variants MUST NOT include `cursorIndex` or `pageStartIndex`. + +### Constant + +**REQ-3** — `VERSES_PER_PAGE` MUST be exported as a named constant from `reader-reducer.ts` with value `8`. + +### PassageFetched Handler + +**REQ-4** — The `PassageFetched` handler MUST initialize `cursorIndex: 0` and `pageStartIndex: 0` when transitioning from `loading` to `loaded`. This applies for both initial fetch and chapter-transition fetches. + +**REQ-5** — The `PassageFetched` handler MUST remain a no-op when `state.kind !== "loading"`. + +### New Actions — General + +**REQ-6** — Four new action types MUST be added to the reducer action union: +- `{ type: "CursorMovedUp" }` +- `{ type: "CursorMovedDown" }` +- `{ type: "PageAdvanced" }` +- `{ type: "PageRetreated" }` + +**REQ-7** — All four new action handlers MUST be no-ops (return state unchanged) when `state.kind !== "loaded"`. + +**REQ-8** — All four new action handlers MUST follow the Rule 13 object-dispatch pattern (handler map, not switch/if chains). + +### CursorMovedDown + +**REQ-9** — `CursorMovedDown` MUST increment `cursorIndex` by 1 when the cursor is not on the last verse of the current page and not on the last verse of the passage. + +**REQ-10** — `CursorMovedDown` at the last verse of the current page (but NOT the last page) MUST advance to the next page: set `pageStartIndex` to `pageStartIndex + VERSES_PER_PAGE` and set `cursorIndex` to the new `pageStartIndex` (first verse of the new page). + +**REQ-11** — `CursorMovedDown` at the last verse of the last page MUST clamp silently — state MUST remain unchanged. + +### CursorMovedUp + +**REQ-12** — `CursorMovedUp` MUST decrement `cursorIndex` by 1 when the cursor is not on the first verse of the current page. + +**REQ-13** — `CursorMovedUp` at the first verse of the current page (but NOT the first page) MUST retreat to the previous page: set `pageStartIndex` to `pageStartIndex - VERSES_PER_PAGE` and set `cursorIndex` to the last verse of the retreated page (i.e., `min(pageStartIndex - 1, verses.length - 1)` using the new pageStartIndex). + +**REQ-14** — `CursorMovedUp` at the first verse of the first page (`cursorIndex === 0`) MUST clamp silently — state MUST remain unchanged. + +### PageAdvanced + +**REQ-15** — `PageAdvanced` MUST set `pageStartIndex` to `pageStartIndex + VERSES_PER_PAGE` and set `cursorIndex` to the new `pageStartIndex`, when a next page exists. + +**REQ-16** — `PageAdvanced` at the last page MUST clamp silently — state MUST remain unchanged. + +### PageRetreated + +**REQ-17** — `PageRetreated` MUST set `pageStartIndex` to `pageStartIndex - VERSES_PER_PAGE` (minimum 0) and set `cursorIndex` to the new `pageStartIndex`, when a previous page exists. + +**REQ-18** — `PageRetreated` at the first page (`pageStartIndex === 0`) MUST clamp silently — state MUST remain unchanged. + +### Existing Actions + +**REQ-19** — `ChapterAdvanced` and `ChapterRetreated` reducer logic MUST remain unchanged. No `cursorIndex` or `pageStartIndex` fields are relevant to these handlers — their effect (triggering a new `loading` state) is unchanged; `PassageFetched` resets cursor/page on the subsequent `loaded` transition. + +### Driver — Awaiting Gate + +**REQ-20** — `tui-driver.tsx` `useKeyboard` MUST gate ALL reader-only keybinds behind `readerState.kind !== "awaiting"`. This gate MUST be placed AFTER the `q`/`Q` quit check so quit always works regardless of reader state. + +**REQ-21** — With the gate in REQ-20, pressing `/` while the palette `` is focused (state is `awaiting`) MUST NOT dispatch `PaletteReopened`. This fixes the existing confirmed bug. + +### Driver — Keybind Map + +**REQ-22** — The `useKeyboard` handler in `tui-driver.tsx` MUST dispatch according to the following table (all bindings active only after the awaiting gate): + +| Key | Action dispatched | Notes | +|-----|-------------------|-------| +| `↑` (`keyEvent.name === "up"`) | `CursorMovedUp` | New | +| `↓` (`keyEvent.name === "down"`) | `CursorMovedDown` | New | +| `[` | `PageRetreated` | Rebound from `ChapterRetreated` | +| `]` | `PageAdvanced` | Rebound from `ChapterAdvanced` | +| `n` | `ChapterAdvanced` | New keybind for existing action | +| `p` | `ChapterRetreated` | New keybind for existing action | +| `/` | `PaletteReopened` | Unchanged; bug fixed by awaiting gate | +| `q`/`Q` | quit | Unchanged; above awaiting gate | + +### Reader Screen — Page Slice + +**REQ-23** — `reader-screen.tsx` loaded branch MUST render only the verses in the current page by slicing: `passage.verses.slice(pageStartIndex, pageStartIndex + VERSES_PER_PAGE)`. + +### Reader Screen — Cursor Rendering + +**REQ-24** — The verse at `cursorIndex` MUST render a `▶` character in the gutter with foreground color `ACCENT_HEX`. All other verses MUST render a space in the gutter. + +**REQ-25** — The verse text at `cursorIndex` MUST be rendered with `TextAttributes.INVERSE`. All other verse texts MUST be rendered without `INVERSE`. + +### Reader Screen — Bottom Title + +**REQ-26** — `bottomTitleFor` for the `loaded` state MUST return exactly: +``` +" ↑↓ verse • [ ] page • n p chapter • / palette • q quit " +``` + +### Welcome Screen — Hint Line + +**REQ-27** — `welcome-screen.tsx` hint line MUST read exactly: +``` +" any key to start • ↑↓ verse • [ ] page • n p chapter • / palette • q quit" +``` + +--- + +## 3. Acceptance Scenarios + +### REQ-1/REQ-2 — Loaded state shape + +**Given** a `loading` state and a `PassageFetched` action +**When** the reducer processes `PassageFetched` +**Then** the resulting state has `kind: "loaded"`, `cursorIndex: 0`, and `pageStartIndex: 0`; neither `loading` nor `network-error` states carry these fields. + +### REQ-3 — VERSES_PER_PAGE + +**Given** the `reader-reducer` module +**When** it is imported +**Then** `VERSES_PER_PAGE` is a named export equal to `8`. + +### REQ-4/REQ-5 — PassageFetched initializes cursor/page + +**Given** a `loading` state +**When** `PassageFetched` is dispatched with any valid passage +**Then** state becomes `loaded` with `cursorIndex: 0` and `pageStartIndex: 0`. + +**Given** a `loaded` state +**When** `PassageFetched` is dispatched +**Then** state is returned unchanged. + +### REQ-7 — New actions are no-ops outside loaded + +**Given** an `awaiting`, `loading`, or `network-error` state +**When** any of `CursorMovedUp`, `CursorMovedDown`, `PageAdvanced`, `PageRetreated` is dispatched +**Then** state is returned unchanged. + +### REQ-9 — CursorMovedDown within page + +**Given** a `loaded` state with 10 verses, `cursorIndex: 0`, `pageStartIndex: 0` +**When** `CursorMovedDown` is dispatched +**Then** `cursorIndex` becomes `1`; `pageStartIndex` remains `0`. + +### REQ-10 — CursorMovedDown crosses page boundary + +**Given** a `loaded` state with 16 verses, `cursorIndex: 7`, `pageStartIndex: 0` +**When** `CursorMovedDown` is dispatched +**Then** `pageStartIndex` becomes `8` and `cursorIndex` becomes `8`. + +### REQ-11 — CursorMovedDown clamps at last verse of last page + +**Given** a `loaded` state with 10 verses, `cursorIndex: 9`, `pageStartIndex: 8` +**When** `CursorMovedDown` is dispatched +**Then** state is unchanged (`cursorIndex` stays `9`, `pageStartIndex` stays `8`). + +### REQ-12 — CursorMovedUp within page + +**Given** a `loaded` state, `cursorIndex: 3`, `pageStartIndex: 0` +**When** `CursorMovedUp` is dispatched +**Then** `cursorIndex` becomes `2`; `pageStartIndex` remains `0`. + +### REQ-13 — CursorMovedUp crosses page boundary + +**Given** a `loaded` state with 16 verses, `cursorIndex: 8`, `pageStartIndex: 8` +**When** `CursorMovedUp` is dispatched +**Then** `pageStartIndex` becomes `0` and `cursorIndex` becomes `7` (last verse of the retreated page). + +### REQ-14 — CursorMovedUp clamps at first verse of first page + +**Given** a `loaded` state, `cursorIndex: 0`, `pageStartIndex: 0` +**When** `CursorMovedUp` is dispatched +**Then** state is unchanged. + +### REQ-15 — PageAdvanced moves to next page + +**Given** a `loaded` state with 16 verses, `pageStartIndex: 0`, `cursorIndex: 3` +**When** `PageAdvanced` is dispatched +**Then** `pageStartIndex` becomes `8` and `cursorIndex` becomes `8`. + +### REQ-16 — PageAdvanced clamps at last page + +**Given** a `loaded` state with 10 verses, `pageStartIndex: 8`, `cursorIndex: 9` +**When** `PageAdvanced` is dispatched +**Then** state is unchanged. + +### REQ-17 — PageRetreated moves to previous page + +**Given** a `loaded` state, `pageStartIndex: 8`, `cursorIndex: 10` +**When** `PageRetreated` is dispatched +**Then** `pageStartIndex` becomes `0` and `cursorIndex` becomes `0`. + +### REQ-18 — PageRetreated clamps at first page + +**Given** a `loaded` state, `pageStartIndex: 0`, `cursorIndex: 2` +**When** `PageRetreated` is dispatched +**Then** state is unchanged. + +### REQ-19 — ChapterAdvanced/Retreated unchanged + +**Given** a `loaded` state +**When** `ChapterAdvanced` is dispatched +**Then** state transitions to `loading` (unchanged from existing behavior); no cursor fields appear on the loading state. + +### REQ-20/REQ-21 — Awaiting gate fixes palette bug + +**Given** the reader is in `awaiting` state (palette `` focused) +**When** the user presses `/` +**Then** `PaletteReopened` is NOT dispatched. + +**Given** the reader is in `awaiting` state +**When** the user presses `q` +**Then** the app quits (quit check is above the gate). + +### REQ-22 — Keybind map dispatches correct actions + +**Given** the reader is in `loaded` state +**When** the user presses `↑` / `↓` / `[` / `]` / `n` / `p` / `/` +**Then** `CursorMovedUp` / `CursorMovedDown` / `PageRetreated` / `PageAdvanced` / `ChapterAdvanced` / `ChapterRetreated` / `PaletteReopened` are dispatched respectively. + +### REQ-23 — Reader renders current page slice + +**Given** a passage with 16 verses in `loaded` state, `pageStartIndex: 8` +**When** `reader-screen` renders +**Then** only verses at indices 8–15 are rendered; verses 0–7 are not present in the output. + +### REQ-24/REQ-25 — Cursor marker and inverse on focused verse + +**Given** a `loaded` state with `cursorIndex: 2` +**When** `reader-screen` renders +**Then** the verse at index 2 shows `▶` in `ACCENT_HEX` in the gutter and its text has `TextAttributes.INVERSE`; all other verses show a space in the gutter with no `INVERSE`. + +### REQ-26 — Bottom title for loaded state + +**Given** a `loaded` state +**When** `bottomTitleFor` is called +**Then** it returns `" ↑↓ verse • [ ] page • n p chapter • / palette • q quit "`. + +### REQ-27 — Welcome screen hint line + +**Given** the welcome screen +**When** it renders +**Then** the hint line reads `" any key to start • ↑↓ verse • [ ] page • n p chapter • / palette • q quit"`. + +### Integration Scenario (from proposal Success Criterion) + +**Given** a loaded passage of 20 verses +**When** the user presses `↓` eight times, then `]` once, then `↑` once +**Then** the reader shows verses 9–16 with the cursor on verse 15 (0-based index 15 — last verse of page 2 after one `↑` from the top of page 3). + +--- + +## 4. Out of Scope + +- Dynamic `VERSES_PER_PAGE` derived from terminal row count +- Cross-chapter cursor flow (e.g. `CursorMovedDown` at last verse of last chapter triggers `ChapterAdvanced`) +- Visual / audible feedback on boundary clamp (silent clamp is the specified behavior) + +--- + +## 5. Non-Functional Requirements + +**NFR-1** — No new runtime dependencies. All new behavior MUST use packages already present in `package.json`. + +**NFR-2** — `bun run tsc --noEmit` MUST exit 0 after all changes are applied. + +**NFR-3** — Existing test suite (127 tests) MUST continue to pass. New tests for cursor and paging actions MUST bring the total to approximately 145–155 tests. + +**NFR-4** — All new reducer action handlers MUST follow the Rule 13 object-dispatch pattern (handler map, no switch or if-chain). + +**NFR-5** — Rule 14: no useless comments in any new or modified code. Comments MUST only appear where they communicate non-obvious intent. diff --git a/openspec/changes/tui-reader-paging/tasks.md b/openspec/changes/tui-reader-paging/tasks.md new file mode 100644 index 0000000..3ae4ffe --- /dev/null +++ b/openspec/changes/tui-reader-paging/tasks.md @@ -0,0 +1,114 @@ +# Tasks: tui-reader-paging + +## Review Workload Forecast + +| Field | Value | +|---|---| +| Estimated changed lines | ~136 | +| 400-line budget risk | Low | +| Chained PRs recommended | No | +| Suggested split | Single PR | +| Delivery strategy | auto-chain (cached YOLO mode) | +| Decision needed before apply | No | + +--- + +## Commit C1 — feat(tui): reader-reducer paging + cursor state and actions + +**Files**: `src/tui/reader/reader-reducer.test.ts`, `src/tui/reader/reader-reducer.ts` + +**Satisfies**: REQ-1, REQ-2, REQ-3, REQ-4, REQ-5, REQ-6, REQ-7, REQ-8, REQ-9, REQ-10, REQ-11, REQ-12, REQ-13, REQ-14, REQ-15, REQ-16, REQ-17, REQ-18, REQ-19, NFR-3, NFR-4 + +### RED phase — `reader-reducer.test.ts` + +- [ ] **T1.1** Update existing `PassageFetched` test assertions to expect the new `loaded` shape (`cursorIndex: 0`, `pageStartIndex: 0`) — confirm tests fail with current reducer. (REQ-1, REQ-4) +- [ ] **T1.2** Add test: `VERSES_PER_PAGE` is a named export equal to `8`. (REQ-3) +- [ ] **T1.3** Add test: `PassageFetched` when `state.kind !== "loading"` returns state unchanged — no new fields on non-loaded variants. (REQ-5) +- [ ] **T1.4** Add tests: `CursorMovedUp`, `CursorMovedDown`, `PageAdvanced`, `PageRetreated` are no-ops when `state.kind` is `awaiting`, `loading`, or `network-error`. (REQ-7) +- [ ] **T1.5** Add test: `CursorMovedDown` within page — `cursorIndex` increments by 1, `pageStartIndex` unchanged. (REQ-9) +- [ ] **T1.6** Add test: `CursorMovedDown` at last verse of page (not last page) — `pageStartIndex` advances by `VERSES_PER_PAGE`, `cursorIndex` equals new `pageStartIndex`. (REQ-10) +- [ ] **T1.7** Add test: `CursorMovedDown` at last verse of last page — state unchanged (silent clamp). (REQ-11) +- [ ] **T1.8** Add test: `CursorMovedUp` within page — `cursorIndex` decrements by 1, `pageStartIndex` unchanged. (REQ-12) +- [ ] **T1.9** Add test: `CursorMovedUp` at first verse of page (not first page) — `pageStartIndex` retreats by `VERSES_PER_PAGE`, `cursorIndex` equals last verse of retreated page (`min(newPageStart + VERSES_PER_PAGE - 1, verses.length - 1)`). (REQ-13) +- [ ] **T1.10** Add test: `CursorMovedUp` at `cursorIndex === 0`, `pageStartIndex === 0` — state unchanged (silent clamp). (REQ-14) +- [ ] **T1.11** Add test: `PageAdvanced` when next page exists — `pageStartIndex` advances by `VERSES_PER_PAGE`, `cursorIndex` equals new `pageStartIndex`. (REQ-15) +- [ ] **T1.12** Add test: `PageAdvanced` at last page — state unchanged (silent clamp). (REQ-16) +- [ ] **T1.13** Add test: `PageRetreated` when previous page exists — `pageStartIndex` retreats by `VERSES_PER_PAGE`, `cursorIndex` equals new `pageStartIndex`. (REQ-17) +- [ ] **T1.14** Add test: `PageRetreated` at first page (`pageStartIndex === 0`) — state unchanged (silent clamp). (REQ-18) +- [ ] **T1.15** Add test: `ChapterAdvanced` and `ChapterRetreated` still transition to `loading` state — no cursor fields on resulting state. (REQ-19) +- [ ] **T1.16** Confirm `bun test` shows all new tests RED and existing tests pass. Gate before GREEN. + +### GREEN phase — `reader-reducer.ts` + +- [ ] **T1.17** Export `VERSES_PER_PAGE = 8` as a named constant. (REQ-3) +- [ ] **T1.18** Extend the `loaded` state variant type with `cursorIndex: number` and `pageStartIndex: number`. (REQ-1) +- [ ] **T1.19** Add four new action types to the union: `CursorMovedUp`, `CursorMovedDown`, `PageAdvanced`, `PageRetreated`. (REQ-6) +- [ ] **T1.20** Update `PassageFetched` handler to initialize `cursorIndex: 0, pageStartIndex: 0` on the `loading → loaded` transition. (REQ-4) +- [ ] **T1.21** Add `CursorMovedDown` handler to the object-dispatch table: within-page increment; cross-page-boundary advance; silent clamp at last verse of last page. (REQ-8, REQ-9, REQ-10, REQ-11, NFR-4) +- [ ] **T1.22** Add `CursorMovedUp` handler to the object-dispatch table: within-page decrement; cross-page-boundary retreat; silent clamp at first verse of first page. (REQ-8, REQ-12, REQ-13, REQ-14, NFR-4) +- [ ] **T1.23** Add `PageAdvanced` handler to the object-dispatch table: advance `pageStartIndex` + reset `cursorIndex`; silent clamp at last page. (REQ-8, REQ-15, REQ-16, NFR-4) +- [ ] **T1.24** Add `PageRetreated` handler to the object-dispatch table: retreat `pageStartIndex` + reset `cursorIndex`; silent clamp at first page. (REQ-8, REQ-17, REQ-18, NFR-4) +- [ ] **T1.25** Confirm all four handlers return state unchanged when `state.kind !== "loaded"` (guard at top of each handler or before dispatch). (REQ-7) +- [ ] **T1.26** Run `bun test` — confirm all new tests GREEN and total test count is in the 145–155 range. (NFR-3) +- [ ] **T1.27** Run `bun run tsc --noEmit` — confirm exit 0. (NFR-2) + +--- + +## Commit C2 — feat(tui): reader-screen page slice + cursor gutter + inverse selection + +**Files**: `src/tui/reader/reader-screen.tsx` + +**Satisfies**: REQ-23, REQ-24, REQ-25, REQ-26, NFR-1, NFR-2 + +> No automated tests — this is the visual surface. Manual smoke is the gate for this commit. + +- [ ] **T2.1** Verify `TextAttributes.INVERSE` is exported from `@opentui/core` (check node_modules or type declarations). If missing, surface as a blocker before proceeding. (NFR-1) +- [ ] **T2.2** In the `loaded` branch of `reader-screen.tsx`, replace full `passage.verses` render with `passage.verses.slice(pageStartIndex, pageStartIndex + VERSES_PER_PAGE)`. (REQ-23) +- [ ] **T2.3** For each rendered verse, add a gutter cell: render `▶` in `ACCENT_HEX` foreground when `verse index === cursorIndex`; render a space for all other verses. (REQ-24) +- [ ] **T2.4** For the verse at `cursorIndex`, apply `TextAttributes.INVERSE` to the verse text element; all other verse texts render without `INVERSE`. (REQ-25) +- [ ] **T2.5** Update `bottomTitleFor` loaded case to return exactly: `" ↑↓ verse • [ ] page • n p chapter • / palette • q quit "`. (REQ-26) +- [ ] **T2.6** Run `bun run tsc --noEmit` — confirm exit 0. (NFR-2) +- [ ] **T2.7** Manual smoke: launch TUI, load a passage with more than 8 verses, verify only 8 verses render, `▶` appears on the first verse with inverse highlight, bottom title reflects new keybind string. + +--- + +## Commit C3 — feat(tui): keybind rebinding + awaiting gate fix; welcome hint + +**Files**: `src/tui/tui-driver.tsx`, `src/tui/welcome/welcome-screen.tsx` + +**Satisfies**: REQ-20, REQ-21, REQ-22, REQ-26 (driver wiring), REQ-27, NFR-2 + +Sequential after C2 (depends on reader-screen shape being stable). + +- [ ] **T3.1** In `tui-driver.tsx` `useKeyboard`, add the `awaiting`-state gate (`readerState.kind !== "awaiting"`) AFTER the `q`/`Q` quit check. All reader-only keybinds must sit inside this gate. (REQ-20, REQ-21) +- [ ] **T3.2** Inside the gate, add `up` key → dispatch `CursorMovedUp`. (REQ-22) +- [ ] **T3.3** Inside the gate, add `down` key → dispatch `CursorMovedDown`. (REQ-22) +- [ ] **T3.4** Rebind `[` from `ChapterRetreated` to `PageRetreated`. (REQ-22) +- [ ] **T3.5** Rebind `]` from `ChapterAdvanced` to `PageAdvanced`. (REQ-22) +- [ ] **T3.6** Add `n` key → dispatch `ChapterAdvanced`. (REQ-22) +- [ ] **T3.7** Add `p` key → dispatch `ChapterRetreated`. (REQ-22) +- [ ] **T3.8** Confirm `/` key (`PaletteReopened`) remains inside the gate and is NOT above it — gate now fixes the double-dispatch bug. (REQ-21) +- [ ] **T3.9** Update `welcome-screen.tsx` hint line to exactly: `" any key to start • ↑↓ verse • [ ] page • n p chapter • / palette • q quit"`. (REQ-27) +- [ ] **T3.10** Run full `bun test` — confirm all tests pass, no regressions. (NFR-3) +- [ ] **T3.11** Run `bun run tsc --noEmit` — confirm exit 0. (NFR-2) +- [ ] **T3.12** Manual smoke: verify `↑`/`↓` move cursor, `[`/`]` page-navigate, `n`/`p` chapter-navigate, `/` opens palette only when NOT in awaiting state, `q` quits from any state. + +--- + +## Dependency Order + +``` +T1.1–T1.27 (C1, sequential) + └─▶ T2.1–T2.7 (C2, sequential — depends on C1 loaded shape) + └─▶ T3.1–T3.12 (C3, sequential — depends on C1 actions + C2 screen) +``` + +All three commits are sequential. No tasks within a commit are parallelisable given strict TDD ordering (RED must confirm fail before GREEN proceeds). + +--- + +## Risk Notes + +- **T2.1** (`TextAttributes.INVERSE` availability) is the only external dependency that could block C2. Verify before writing screen code. +- Arrow key names (`"up"` / `"down"`) are confirmed from `mock-keys.d.ts` but must be re-verified at C3 implementation against the live `@opentui/core` key event shape. +- Existing tests that assert on the `loaded` state shape (T1.1) will fail until T1.17–T1.20 are applied — this is the expected RED gate. diff --git a/openspec/changes/tui-reader-paging/verify-report.md b/openspec/changes/tui-reader-paging/verify-report.md new file mode 100644 index 0000000..3af84c1 --- /dev/null +++ b/openspec/changes/tui-reader-paging/verify-report.md @@ -0,0 +1,102 @@ +# Verify Report: tui-reader-paging + +**Date**: 2026-05-12 +**Branch**: `feat/tui-reader-paging` (3 commits off main) +**Commits**: `a85a974`, `8764733`, `7032496` +**Verdict**: PASS WITH WARNINGS +**Ready for archive**: Yes (manual smoke pending — WARNING only) + +--- + +## Build & Test Evidence + +| Check | Result | +|---|---| +| `bun test` | 151 pass / 0 fail | +| `bun run tsc --noEmit` | exit 0 | +| Test delta | +24 tests (127 → 151) | +| TDD mode | Strict (RED gate confirmed in apply-progress) | + +--- + +## Task Completeness + +| Commit | Automated tasks | Manual smoke | +|---|---|---| +| C1 (reducer) | 27/27 complete | N/A | +| C2 (reader-screen) | 6/7 — T2.7 pending (live TTY) | PENDING | +| C3 (driver + welcome) | 11/12 — T3.12 pending (live TTY) | PENDING | + +Total: 44/46 tasks complete. 2 open = manual smoke (no automated path possible). + +--- + +## Spec Compliance Matrix + +| REQ | Description | Status | Evidence | +|---|---|---|---| +| REQ-1 | `loaded` has `cursorIndex: number`, `pageStartIndex: number` | PASS | Type union line confirmed | +| REQ-2 | `loading`/`awaiting`/`network-error` do NOT carry cursor fields | PASS | State type verified | +| REQ-3 | `VERSES_PER_PAGE = 8` exported | PASS | Test + source | +| REQ-4 | `PassageFetched` initializes both to 0 | PASS | Test T1.1 + handler | +| REQ-5 | `PassageFetched` no-op when `kind !== "loading"` | PASS | Test T1.3 | +| REQ-6 | 4 new action types in union | PASS | Source verified | +| REQ-7 | 4 new actions no-op outside `loaded` | PASS | 12 parameterized tests (4 actions × 3 states) | +| REQ-8 | Object-dispatch pattern, no switch/if-chain | PASS | `satisfies` table confirmed, no new switch statements | +| REQ-9 | `CursorMovedDown` increments within page | PASS | Test T1.5 | +| REQ-10 | `CursorMovedDown` crosses page boundary | PASS | Test T1.6 | +| REQ-11 | `CursorMovedDown` clamps at last verse of last page | PASS | Test T1.7 | +| REQ-12 | `CursorMovedUp` decrements within page | PASS | Test T1.8 | +| REQ-13 | `CursorMovedUp` crosses page boundary; cursor = last verse | PASS | Test T1.9; formula verified (`min(newPageStart + VPP - 1, verses.length - 1)` == `min(oldPageStart - 1, verses.length - 1)`) | +| REQ-14 | `CursorMovedUp` clamps at `cursorIndex===0, pageStartIndex===0` | PASS | Test T1.10 | +| REQ-15 | `PageAdvanced` sets both to `nextPageStart` | PASS | Test T1.11 | +| REQ-16 | `PageAdvanced` clamps at last page | PASS | Test T1.12 | +| REQ-17 | `PageRetreated` sets both to `newPageStart` | PASS | Test T1.13 | +| REQ-18 | `PageRetreated` clamps at `pageStartIndex===0` | PASS | Test T1.14 | +| REQ-19 | `ChapterAdvanced`/`ChapterRetreated` unchanged; no cursor fields on loading | PASS | Test T1.15 | +| REQ-20 | Awaiting gate AFTER q/Q check | PASS | Gate at line 35, q/Q at line 26 | +| REQ-21 | `PaletteReopened` not dispatched in awaiting state | PASS | Gate verified; `/` is inside the gate | +| REQ-22 | Full keybind map dispatches correct actions | PASS | Source: up→CursorMovedUp, down→CursorMovedDown, [→PageRetreated, ]→PageAdvanced, n→ChapterAdvanced, p→ChapterRetreated, /→PaletteReopened | +| REQ-23 | Page slice `verses.slice(pageStartIndex, pageStartIndex + VERSES_PER_PAGE)` | PASS | Source confirmed | +| REQ-24 | `▶` gutter with `ACCENT_HEX` on cursor row | PASS | `fg={focused ? ACCENT_HEX : undefined}` confirmed | +| REQ-25 | `TextAttributes.INVERSE` on cursor verse text only | PASS | `attributes={focused ? INVERSE : undefined}` confirmed | +| REQ-26 | `bottomTitleFor` loaded exact string | PASS | Exact match verified (Python comparison) | +| REQ-27 | Welcome hint exact string | PASS | Exact match verified (Python comparison) | + +**27/27 REQs: PASS** + +--- + +## NFR Compliance + +| NFR | Description | Status | +|---|---|---| +| NFR-1 | No new runtime dependencies | PASS | +| NFR-2 | `tsc --noEmit` exits 0 | PASS | +| NFR-3 | 151 tests pass (target 145–155) | PASS | +| NFR-4 | Object-dispatch pattern only; no switch | PASS | +| NFR-5 | No useless comments in new/modified code | PASS — pre-existing driver comments unchanged | + +--- + +## Issues + +### WARNING + +- **W1 — Manual smoke PENDING (T2.7, T3.12)**: Visual rendering (gutter `▶`, `INVERSE` highlight, bottom title) and all keybind round-trips require a live TTY. These tasks were explicitly flagged PENDING in apply-progress. Not automatable — must be done manually before merge. + +### SUGGESTION + +- **S1 — Gutter `fg` and `attributes` combined**: The focused gutter span uses both `fg={ACCENT_HEX}` and `attributes={DIM}`. DIM may reduce the ACCENT_HEX visibility on some terminals. No spec violation — just a cosmetic note for the smoke session. + +--- + +## Design Coherence + +No deviations from spec or design. 0-based index approach (spec-correct) used throughout. `makeLoaded()` helper in tests cleanly solved the TS2769 union-spread limitation noted in apply-progress. + +--- + +## Summary + +All 27 spec REQs verified. All 5 NFRs met. 151/151 tests pass. TypeScript clean. 0 CRITICAL, 1 WARNING (manual smoke), 1 SUGGESTION. Ready for archive after manual smoke is completed or explicitly waived. From fbd7d71287534fbfce6256c337cf577cc8c248b8 Mon Sep 17 00:00:00 2001 From: ivanmaierg Date: Tue, 12 May 2026 09:09:16 -0300 Subject: [PATCH 5/9] fix(tui): reader fetches whole chapter; verse refs position cursor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two UX issues from manual smoke: 1. Single-verse refs like 'john 3:16' showed only that verse instead of dropping into the chapter page with the cursor on verse 16. Root cause: useEffect called getPassage, which sliced the chapter down to ref.verses range. Fix: add a new application use case getChapter that returns the full chapter regardless of ref.verses, and have the reader call it instead. The verse range now serves as the initial cursor target, not a slice predicate. PassageFetched computes cursorIndex by looking up the target verse number in the fetched passage and derives pageStartIndex from cursorIndex / VERSES_PER_PAGE. Chapter nav (n/p) resets ref.verses to {1, 1} so the cursor lands on verse 1 of the new chapter rather than carrying the old verse number into a possibly-shorter chapter. 2. The palette input spanned the whole bordered frame. Wrapped it in a width=50 box with marginLeft=2 so it reads as an input field, not the entire reader surface. Also fixes the verify suggestion: the focused ▶ cursor span was applying both fg={ACCENT_HEX} and attributes={DIM}, which washed out the accent on most terminals. DIM now applies only when the gutter is empty (non-focused rows). Tests: 151 → 156 (+5: 3 for getChapter use case, 2 for cursor positioning in PassageFetched). tsc clean. --- src/application/get-chapter.test.ts | 66 +++++++++++++++++++++++++++ src/application/get-chapter.ts | 22 +++++++++ src/tui/reader/reader-reducer.test.ts | 35 ++++++++++++-- src/tui/reader/reader-reducer.ts | 16 ++++--- src/tui/reader/reader-screen.tsx | 16 ++++--- src/tui/reader/use-passage-fetch.ts | 4 +- 6 files changed, 140 insertions(+), 19 deletions(-) create mode 100644 src/application/get-chapter.test.ts create mode 100644 src/application/get-chapter.ts diff --git a/src/application/get-chapter.test.ts b/src/application/get-chapter.test.ts new file mode 100644 index 0000000..f977675 --- /dev/null +++ b/src/application/get-chapter.test.ts @@ -0,0 +1,66 @@ +import { describe, it, expect } from "bun:test"; +import { getChapter } from "./get-chapter"; +import { makeBookId } from "@/domain/book-id"; +import { DEFAULT_TRANSLATION_ID } from "@/domain/translations"; +import type { BibleRepository } from "@/application/ports/bible-repository"; +import type { Reference } from "@/domain/reference"; + +function jhn3Ref(verseStart: number, verseEnd = verseStart): Reference { + const book = makeBookId("JHN"); + if (!book.ok) throw new Error("test fixture: JHN must be valid"); + return { book: book.value, chapter: 3, verses: { start: verseStart, end: verseEnd } }; +} + +describe("getChapter", () => { + it("returns the whole chapter regardless of ref.verses range", async () => { + const ref = jhn3Ref(16); + const repo: BibleRepository = { + async getChapter() { + return { + ok: true, + value: { + translationId: DEFAULT_TRANSLATION_ID, + book: ref.book, + chapter: 3, + verses: [ + { number: 1, text: "v1" }, + { number: 2, text: "v2" }, + { number: 16, text: "v16" }, + ], + }, + }; + }, + }; + const result = await getChapter(repo, ref); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.value.verses).toHaveLength(3); + expect(result.value.reference).toEqual(ref); + }); + + it("propagates chapter_not_found", async () => { + const ref = jhn3Ref(1); + const repo: BibleRepository = { + async getChapter() { + return { ok: false, error: { kind: "chapter_not_found", chapter: 3 } }; + }, + }; + const result = await getChapter(repo, ref); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error.kind).toBe("chapter_not_found"); + }); + + it("propagates network RepoError", async () => { + const ref = jhn3Ref(1); + const repo: BibleRepository = { + async getChapter() { + return { ok: false, error: { kind: "network", message: "down" } }; + }, + }; + const result = await getChapter(repo, ref); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error.kind).toBe("network"); + }); +}); diff --git a/src/application/get-chapter.ts b/src/application/get-chapter.ts new file mode 100644 index 0000000..9abfcfa --- /dev/null +++ b/src/application/get-chapter.ts @@ -0,0 +1,22 @@ +import type { Result } from "@/domain/result"; +import type { Reference } from "@/domain/reference"; +import type { Passage } from "@/domain/passage"; +import type { AppError } from "@/domain/errors"; +import type { BibleRepository } from "@/application/ports/bible-repository"; +import { DEFAULT_TRANSLATION_ID } from "@/domain/translations"; + +export async function getChapter( + repo: BibleRepository, + ref: Reference, +): Promise> { + const chapterResult = await repo.getChapter( + DEFAULT_TRANSLATION_ID, + ref.book, + ref.chapter, + ); + if (!chapterResult.ok) return chapterResult; + return { + ok: true, + value: { reference: ref, verses: chapterResult.value.verses }, + }; +} diff --git a/src/tui/reader/reader-reducer.test.ts b/src/tui/reader/reader-reducer.test.ts index 1261fcd..e1f620c 100644 --- a/src/tui/reader/reader-reducer.test.ts +++ b/src/tui/reader/reader-reducer.test.ts @@ -106,7 +106,7 @@ describe("readerReducer", () => { }); describe("PassageFetched", () => { - it("transitions loading → loaded with cursorIndex: 0, pageStartIndex: 0", () => { + it("defaults cursorIndex to 0 when the ref's target verse is not in the passage", () => { const state: ReaderState = { kind: "loading", ref: johnRef }; const next = dispatch(state, { type: "PassageFetched", passage: mockPassage }); expect(next).toEqual({ @@ -118,6 +118,28 @@ describe("readerReducer", () => { }); }); + it("positions cursor on the verse matching ref.verses.start", () => { + // John 3:16 — passage has 36 verses, target verse 16 should land at index 15. + const passage = makePassage(36); + const ref: Reference = { ...johnRef, verses: { start: 16, end: 16 } }; + const state: ReaderState = { kind: "loading", ref }; + const next = dispatch(state, { type: "PassageFetched", passage }); + if (next.kind !== "loaded") throw new Error("expected loaded state"); + expect(next.cursorIndex).toBe(15); + // Verse 16 (index 15) belongs to page 2 (indices 8-15) when VERSES_PER_PAGE === 8. + expect(next.pageStartIndex).toBe(8); + }); + + it("places page boundary correctly on the third page (verse 17 → index 16, page 3)", () => { + const passage = makePassage(36); + const ref: Reference = { ...johnRef, verses: { start: 17, end: 17 } }; + const state: ReaderState = { kind: "loading", ref }; + const next = dispatch(state, { type: "PassageFetched", passage }); + if (next.kind !== "loaded") throw new Error("expected loaded state"); + expect(next.cursorIndex).toBe(16); + expect(next.pageStartIndex).toBe(16); + }); + it("is a no-op when not loading", () => { const state = makeLoaded(mockPassage, 0, 0); const next = dispatch(state, { type: "PassageFetched", passage: mockPassage }); @@ -140,12 +162,17 @@ describe("readerReducer", () => { }); describe("ChapterAdvanced", () => { - it("transitions loaded → loading with chapter + 1", () => { - const state = makeLoaded(mockPassage, 0, 0); - const next = dispatch(state, { type: "ChapterAdvanced" }); + it("transitions loaded → loading with chapter + 1 and resets ref.verses to {1, 1}", () => { + // Why: ref.verses comes from the user's original input (e.g. john 3:16). On chapter + // advance the cursor should land at the top of the new chapter, not at verse 16. + const stateWithVerse16 = makeLoaded(mockPassage, 0, 0); + const refWithVerse16 = { ...johnRef, verses: { start: 16, end: 16 } }; + const adjusted = { ...stateWithVerse16, ref: refWithVerse16 } as ReaderState; + const next = dispatch(adjusted, { type: "ChapterAdvanced" }); expect(next.kind).toBe("loading"); if (next.kind !== "loading") return; expect(next.ref.chapter).toBe(4); + expect(next.ref.verses).toEqual({ start: 1, end: 1 }); }); it("is a no-op from awaiting", () => { diff --git a/src/tui/reader/reader-reducer.ts b/src/tui/reader/reader-reducer.ts index c37e365..facab72 100644 --- a/src/tui/reader/reader-reducer.ts +++ b/src/tui/reader/reader-reducer.ts @@ -36,10 +36,14 @@ const handlers = { : { ...s, parseError: result.error }; }, - PassageFetched: (s: ReaderState, a: Extract): ReaderState => - s.kind === "loading" - ? { kind: "loaded", passage: a.passage, ref: s.ref, cursorIndex: 0, pageStartIndex: 0 } - : s, + PassageFetched: (s: ReaderState, a: Extract): ReaderState => { + if (s.kind !== "loading") return s; + const targetVerse = s.ref.verses.start; + const foundIndex = a.passage.verses.findIndex((v) => v.number === targetVerse); + const cursorIndex = foundIndex >= 0 ? foundIndex : 0; + const pageStartIndex = Math.floor(cursorIndex / VERSES_PER_PAGE) * VERSES_PER_PAGE; + return { kind: "loaded", passage: a.passage, ref: s.ref, cursorIndex, pageStartIndex }; + }, FetchFailed: (s: ReaderState, a: Extract): ReaderState => s.kind === "loading" @@ -48,13 +52,13 @@ const handlers = { ChapterAdvanced: (s: ReaderState, _a: Extract): ReaderState => s.kind === "loaded" - ? { kind: "loading", ref: { ...s.ref, chapter: s.ref.chapter + 1 } } + ? { kind: "loading", ref: { ...s.ref, chapter: s.ref.chapter + 1, verses: { start: 1, end: 1 } } } : s, ChapterRetreated: (s: ReaderState, _a: Extract): ReaderState => { if (s.kind !== "loaded") return s; if (s.ref.chapter <= 1) return s; - return { kind: "loading", ref: { ...s.ref, chapter: s.ref.chapter - 1 } }; + return { kind: "loading", ref: { ...s.ref, chapter: s.ref.chapter - 1, verses: { start: 1, end: 1 } } }; }, PaletteReopened: (s: ReaderState, _a: Extract): ReaderState => diff --git a/src/tui/reader/reader-screen.tsx b/src/tui/reader/reader-screen.tsx index 140ee35..c7ec465 100644 --- a/src/tui/reader/reader-screen.tsx +++ b/src/tui/reader/reader-screen.tsx @@ -79,12 +79,14 @@ function Body({ state, dispatch, frame }: BodyProps) { {" Type a reference, press Enter"} {" "} - dispatch({ type: "QueryTyped", query: v })} - onSubmit={() => dispatch({ type: "QuerySubmitted" })} - /> + + dispatch({ type: "QueryTyped", query: v })} + onSubmit={() => dispatch({ type: "QuerySubmitted" })} + /> + {state.parseError !== null ? ( {` ⚠ couldn't parse "${state.query}"`} ) : null} @@ -119,7 +121,7 @@ function Body({ state, dispatch, frame }: BodyProps) { const focused = idx === cursorIndex; return ( - + {focused ? "▶" : " "} {`${String(v.number).padStart(3)} `} diff --git a/src/tui/reader/use-passage-fetch.ts b/src/tui/reader/use-passage-fetch.ts index 883ebd0..4f208b8 100644 --- a/src/tui/reader/use-passage-fetch.ts +++ b/src/tui/reader/use-passage-fetch.ts @@ -1,6 +1,6 @@ import { useEffect } from "react"; import type { Dispatch } from "react"; -import { getPassage } from "@/application/get-passage"; +import { getChapter } from "@/application/get-chapter"; import type { BibleRepository } from "@/application/ports/bible-repository"; import { isRepoError } from "@/domain/errors"; import type { ReaderState, ReaderAction } from "@/tui/reader/reader-reducer"; @@ -16,7 +16,7 @@ export function usePassageFetch( let cancelled = false; const ref = state.ref; - getPassage(repo, ref).then((result) => { + getChapter(repo, ref).then((result) => { if (cancelled) return; if (result.ok) { dispatch({ type: "PassageFetched", passage: result.value }); From 5a5ef322535c06971564058252e74990f200db08 Mon Sep 17 00:00:00 2001 From: ivanmaierg Date: Tue, 12 May 2026 09:13:52 -0300 Subject: [PATCH 6/9] feat(tui): denser pages + word-wrap with hanging indent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two UX bumps: 1. VERSES_PER_PAGE 8 → 15. Closer to the original ui-sketches.md Reading view density. Comfortably fits an 80×24 terminal once word-wrap accounts for the chrome. 2. Word-wrap with hanging indent for verse text. Previously, long verses ran off the right edge or wrapped break-anywhere with no visual continuity. Now: each verse wraps at word boundaries, the first line carries the gutter + verse number, and continuation lines indent 6 chars to align with the text column — matching the original docs/ui-sketches.md sketch shape (line 122). Wrap width derives from useTerminalDimensions() each render, so narrow terminals reflow correctly. Floor at 20 chars to keep the wrap useful on tiny terminals. Tests updated to be VERSES_PER_PAGE-relative (using the constant symbol in place of hardcoded 8/16) so future tuning won't churn the suite. 156/156 still pass, tsc clean. --- src/tui/reader/reader-reducer.test.ts | 58 ++++++++++++------------ src/tui/reader/reader-reducer.ts | 2 +- src/tui/reader/reader-screen.tsx | 65 +++++++++++++++++++++++---- 3 files changed, 87 insertions(+), 38 deletions(-) diff --git a/src/tui/reader/reader-reducer.test.ts b/src/tui/reader/reader-reducer.test.ts index e1f620c..13d9877 100644 --- a/src/tui/reader/reader-reducer.test.ts +++ b/src/tui/reader/reader-reducer.test.ts @@ -52,8 +52,8 @@ describe("readerReducer", () => { }); describe("VERSES_PER_PAGE", () => { - it("equals 8", () => { - expect(VERSES_PER_PAGE).toBe(8); + it("equals 15", () => { + expect(VERSES_PER_PAGE).toBe(15); }); }); @@ -118,26 +118,26 @@ describe("readerReducer", () => { }); }); - it("positions cursor on the verse matching ref.verses.start", () => { - // John 3:16 — passage has 36 verses, target verse 16 should land at index 15. - const passage = makePassage(36); - const ref: Reference = { ...johnRef, verses: { start: 16, end: 16 } }; + it("positions cursor on the verse matching ref.verses.start (page 2)", () => { + const passage = makePassage(VERSES_PER_PAGE * 3); + const targetVerse = VERSES_PER_PAGE + 1; + const ref: Reference = { ...johnRef, verses: { start: targetVerse, end: targetVerse } }; const state: ReaderState = { kind: "loading", ref }; const next = dispatch(state, { type: "PassageFetched", passage }); if (next.kind !== "loaded") throw new Error("expected loaded state"); - expect(next.cursorIndex).toBe(15); - // Verse 16 (index 15) belongs to page 2 (indices 8-15) when VERSES_PER_PAGE === 8. - expect(next.pageStartIndex).toBe(8); + expect(next.cursorIndex).toBe(targetVerse - 1); + expect(next.pageStartIndex).toBe(VERSES_PER_PAGE); }); - it("places page boundary correctly on the third page (verse 17 → index 16, page 3)", () => { - const passage = makePassage(36); - const ref: Reference = { ...johnRef, verses: { start: 17, end: 17 } }; + it("places page boundary correctly on the third page", () => { + const passage = makePassage(VERSES_PER_PAGE * 3); + const targetVerse = VERSES_PER_PAGE * 2 + 1; + const ref: Reference = { ...johnRef, verses: { start: targetVerse, end: targetVerse } }; const state: ReaderState = { kind: "loading", ref }; const next = dispatch(state, { type: "PassageFetched", passage }); if (next.kind !== "loaded") throw new Error("expected loaded state"); - expect(next.cursorIndex).toBe(16); - expect(next.pageStartIndex).toBe(16); + expect(next.cursorIndex).toBe(targetVerse - 1); + expect(next.pageStartIndex).toBe(VERSES_PER_PAGE * 2); }); it("is a no-op when not loading", () => { @@ -287,15 +287,16 @@ describe("readerReducer", () => { }); it("advances page and resets cursor at page boundary", () => { - const p = makePassage(16); - const state = makeLoaded(p, 7, 0); + const p = makePassage(VERSES_PER_PAGE * 2); + const state = makeLoaded(p, VERSES_PER_PAGE - 1, 0); const next = dispatch(state, { type: "CursorMovedDown" }); - expect(next).toEqual(makeLoaded(p, 8, 8)); + expect(next).toEqual(makeLoaded(p, VERSES_PER_PAGE, VERSES_PER_PAGE)); }); it("clamps at last verse of last page", () => { - const p = makePassage(10); - const state = makeLoaded(p, 9, 8); + const total = VERSES_PER_PAGE + 2; + const p = makePassage(total); + const state = makeLoaded(p, total - 1, VERSES_PER_PAGE); const next = dispatch(state, { type: "CursorMovedDown" }); expect(next).toBe(state); }); @@ -310,10 +311,10 @@ describe("readerReducer", () => { }); it("retreats page and sets cursor to last verse of retreated page", () => { - const p = makePassage(16); - const state = makeLoaded(p, 8, 8); + const p = makePassage(VERSES_PER_PAGE * 2); + const state = makeLoaded(p, VERSES_PER_PAGE, VERSES_PER_PAGE); const next = dispatch(state, { type: "CursorMovedUp" }); - expect(next).toEqual(makeLoaded(p, 7, 0)); + expect(next).toEqual(makeLoaded(p, VERSES_PER_PAGE - 1, 0)); }); it("clamps at first verse of first page", () => { @@ -326,15 +327,16 @@ describe("readerReducer", () => { describe("PageAdvanced", () => { it("advances to next page and sets cursorIndex to pageStartIndex", () => { - const p = makePassage(16); + const p = makePassage(VERSES_PER_PAGE * 2); const state = makeLoaded(p, 3, 0); const next = dispatch(state, { type: "PageAdvanced" }); - expect(next).toEqual(makeLoaded(p, 8, 8)); + expect(next).toEqual(makeLoaded(p, VERSES_PER_PAGE, VERSES_PER_PAGE)); }); it("clamps at last page", () => { - const p = makePassage(10); - const state = makeLoaded(p, 9, 8); + const total = VERSES_PER_PAGE + 2; + const p = makePassage(total); + const state = makeLoaded(p, total - 1, VERSES_PER_PAGE); const next = dispatch(state, { type: "PageAdvanced" }); expect(next).toBe(state); }); @@ -342,8 +344,8 @@ describe("readerReducer", () => { describe("PageRetreated", () => { it("retreats to previous page and sets cursorIndex to new pageStartIndex", () => { - const p = makePassage(16); - const state = makeLoaded(p, 10, 8); + const p = makePassage(VERSES_PER_PAGE * 2); + const state = makeLoaded(p, VERSES_PER_PAGE + 2, VERSES_PER_PAGE); const next = dispatch(state, { type: "PageRetreated" }); expect(next).toEqual(makeLoaded(p, 0, 0)); }); diff --git a/src/tui/reader/reader-reducer.ts b/src/tui/reader/reader-reducer.ts index facab72..f5b4693 100644 --- a/src/tui/reader/reader-reducer.ts +++ b/src/tui/reader/reader-reducer.ts @@ -3,7 +3,7 @@ import type { Reference } from "@/domain/reference"; import type { ParseError, RepoError } from "@/domain/errors"; import type { Passage } from "@/domain/passage"; -export const VERSES_PER_PAGE = 8; +export const VERSES_PER_PAGE = 15; export type ReaderState = | { kind: "awaiting"; query: string; parseError: ParseError | null } diff --git a/src/tui/reader/reader-screen.tsx b/src/tui/reader/reader-screen.tsx index c7ec465..70ac071 100644 --- a/src/tui/reader/reader-screen.tsx +++ b/src/tui/reader/reader-screen.tsx @@ -1,5 +1,6 @@ import { useState, useEffect } from "react"; import { TextAttributes } from "@opentui/core"; +import { useTerminalDimensions } from "@opentui/react"; import { SPINNER_FRAMES } from "@/cli/loading"; import { ACCENT_HEX } from "@/presentation/colors"; import { usePassageFetch } from "@/tui/reader/use-passage-fetch"; @@ -7,6 +8,7 @@ import { VERSES_PER_PAGE } from "@/tui/reader/reader-reducer"; import type { BibleRepository } from "@/application/ports/bible-repository"; import type { ReaderState, ReaderAction } from "@/tui/reader/reader-reducer"; import type { Dispatch } from "react"; +import type { Verse } from "@/domain/passage"; const DIM = TextAttributes.DIM; const INVERSE = TextAttributes.INVERSE; @@ -113,22 +115,67 @@ function Body({ state, dispatch, frame }: BodyProps) { const { passage, cursorIndex, pageStartIndex } = state; const pageVerses = passage.verses.slice(pageStartIndex, pageStartIndex + VERSES_PER_PAGE); + return ; +} + +const PREFIX_LEN = 6; // 1 (gutter) + 3 (padded number) + 2 (spaces) +const CONTINUATION_INDENT = " ".repeat(PREFIX_LEN); + +function LoadedBody({ + pageVerses, + cursorIndex, + pageStartIndex, +}: { + pageVerses: Verse[]; + cursorIndex: number; + pageStartIndex: number; +}) { + // Subtract border (2) + prefix (6); guard against narrow terminals so we never wrap to 0. + const { width } = useTerminalDimensions(); + const wrapWidth = Math.max(20, width - 2 - PREFIX_LEN); return ( - {pageVerses.map((v, i) => { + {pageVerses.flatMap((v, i) => { const idx = pageStartIndex + i; const focused = idx === cursorIndex; - return ( - - - {focused ? "▶" : " "} - - {`${String(v.number).padStart(3)} `} - {v.text} + const lines = wordWrap(v.text, wrapWidth); + return lines.map((line, lineIdx) => ( + + {lineIdx === 0 ? ( + <> + + {focused ? "▶" : " "} + + {`${String(v.number).padStart(3)} `} + + ) : ( + {CONTINUATION_INDENT} + )} + {line} - ); + )); })} ); } + +function wordWrap(text: string, width: number): string[] { + if (width <= 0) return [text]; + const words = text.split(/\s+/).filter(Boolean); + if (words.length === 0) return [""]; + const lines: string[] = []; + let current = ""; + for (const word of words) { + if (current.length === 0) { + current = word; + } else if (current.length + 1 + word.length <= width) { + current += " " + word; + } else { + lines.push(current); + current = word; + } + } + if (current) lines.push(current); + return lines; +} From 0ac25aac42daf9bc5022d3e3919eb6ae0a9bce5e Mon Sep 17 00:00:00 2001 From: ivanmaierg Date: Tue, 12 May 2026 09:24:06 -0300 Subject: [PATCH 7/9] =?UTF-8?q?feat(tui):=20book-page=20layout=20=E2=80=94?= =?UTF-8?q?=20capped=20width,=20centered,=20blank-line=20verse=20rhythm?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Matches docs/ui-sketches.md Reading view (line 122): - Constrain bordered frame to max 70 chars (configurable via PAGE_MAX_WIDTH), centered horizontally. Wide terminals no longer produce a single 200-char box that runs verses off the right edge or floods the screen with empty margin. - Falls back to terminal width - 4 on narrower terminals (floor 40). - 5-char prefix (' 16 ' or '▶16 ') with 5-space hanging indent — matches the sketch's verse-row layout exactly. - Blank row between verses for vertical breathing room (also from the sketch). - Top padding + box auto-sizes to content so the box no longer stretches to the terminal floor and looks like a log dump. Tests: 156/156 still pass (no reducer changes). tsc clean. --- src/tui/reader/reader-screen.tsx | 56 ++++++++++++++++++++++---------- 1 file changed, 39 insertions(+), 17 deletions(-) diff --git a/src/tui/reader/reader-screen.tsx b/src/tui/reader/reader-screen.tsx index 70ac071..c24ec30 100644 --- a/src/tui/reader/reader-screen.tsx +++ b/src/tui/reader/reader-screen.tsx @@ -19,6 +19,8 @@ type ReaderScreenProps = { repo: BibleRepository; }; +const PAGE_MAX_WIDTH = 70; + export function ReaderScreen({ state, dispatch, repo }: ReaderScreenProps) { usePassageFetch(state, dispatch, repo); @@ -32,15 +34,20 @@ export function ReaderScreen({ state, dispatch, repo }: ReaderScreenProps) { return () => clearInterval(id); }, [state.kind]); + const { width: termWidth } = useTerminalDimensions(); + const boxWidth = Math.min(PAGE_MAX_WIDTH, Math.max(40, termWidth - 4)); + return ( - - + + + + ); } @@ -73,9 +80,10 @@ type BodyProps = { state: ReaderState; dispatch: Dispatch; frame: number; + boxWidth: number; }; -function Body({ state, dispatch, frame }: BodyProps) { +function Body({ state, dispatch, frame, boxWidth }: BodyProps) { if (state.kind === "awaiting") { return ( @@ -115,39 +123,48 @@ function Body({ state, dispatch, frame }: BodyProps) { const { passage, cursorIndex, pageStartIndex } = state; const pageVerses = passage.verses.slice(pageStartIndex, pageStartIndex + VERSES_PER_PAGE); - return ; + return ( + + ); } -const PREFIX_LEN = 6; // 1 (gutter) + 3 (padded number) + 2 (spaces) +// Prefix layout matches docs/ui-sketches.md Reading view (line 122): +// "▶ 16 For God..." or " 16 For God..." — 5 chars before text, 5-space continuation. +const PREFIX_LEN = 5; const CONTINUATION_INDENT = " ".repeat(PREFIX_LEN); function LoadedBody({ pageVerses, cursorIndex, pageStartIndex, + boxWidth, }: { pageVerses: Verse[]; cursorIndex: number; pageStartIndex: number; + boxWidth: number; }) { - // Subtract border (2) + prefix (6); guard against narrow terminals so we never wrap to 0. - const { width } = useTerminalDimensions(); - const wrapWidth = Math.max(20, width - 2 - PREFIX_LEN); + const wrapWidth = Math.max(20, boxWidth - 2 - PREFIX_LEN); return ( - + {pageVerses.flatMap((v, i) => { const idx = pageStartIndex + i; const focused = idx === cursorIndex; const lines = wordWrap(v.text, wrapWidth); - return lines.map((line, lineIdx) => ( + const rows = lines.map((line, lineIdx) => ( {lineIdx === 0 ? ( <> {focused ? "▶" : " "} - {`${String(v.number).padStart(3)} `} + {` ${String(v.number).padStart(2)} `} ) : ( {CONTINUATION_INDENT} @@ -155,6 +172,11 @@ function LoadedBody({ {line} )); + // Blank row between verses (not after the last) — matches the sketch's vertical rhythm. + if (i < pageVerses.length - 1) { + rows.push({" "}); + } + return rows; })} ); From 158aa1b3bb532de7caef3761a88ec1ba26dcafac Mon Sep 17 00:00:00 2001 From: ivanmaierg Date: Tue, 12 May 2026 09:29:17 -0300 Subject: [PATCH 8/9] style(tui): focused verse uses accent color, not inverse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit INVERSE swaps fg/bg, which on a dark terminal renders as black-on- light — jarring against the rest of the muted page. Switching to fg=ACCENT_HEX, matching docs/ui-sketches.md line 148 'Focused verse: accent color on the line + ▶ marker in the gutter'. The Typography table at line 437 of the same doc said 'selection (inverse)', but the Reading view rules are more specific to this surface and read better on a dark background. Verse text and gutter marker now share the accent token when focused, making the cursor row read as a single highlighted unit instead of a sudden color block. --- src/tui/reader/reader-screen.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/tui/reader/reader-screen.tsx b/src/tui/reader/reader-screen.tsx index c24ec30..e13c8ca 100644 --- a/src/tui/reader/reader-screen.tsx +++ b/src/tui/reader/reader-screen.tsx @@ -11,7 +11,6 @@ import type { Dispatch } from "react"; import type { Verse } from "@/domain/passage"; const DIM = TextAttributes.DIM; -const INVERSE = TextAttributes.INVERSE; type ReaderScreenProps = { state: ReaderState; @@ -169,7 +168,7 @@ function LoadedBody({ ) : ( {CONTINUATION_INDENT} )} - {line} + {line} )); // Blank row between verses (not after the last) — matches the sketch's vertical rhythm. From e2df77c5622a8bb404d20bd145aff2c3fe7c06b6 Mon Sep 17 00:00:00 2001 From: ivanmaierg Date: Tue, 12 May 2026 09:35:38 -0300 Subject: [PATCH 9/9] chore(openspec): archive tui-reader-paging SDD cycle closed. PASS WITH WARNINGS verify + 4 user-sanctioned post-verify iterations from manual smoke (whole-chapter fetch, denser pages, book-page layout, accent cursor color). 156/156 tests pass. Manual PTY smoke confirmed by user. Out-of-scope follow-ups: dynamic page size from terminal height, cross-chapter cursor flow, two-page open-book spread, long-word wrap, visual page-boundary feedback. --- .../tui-reader-paging/archive-report.md | 108 ++++++++++++++++++ .../tui-reader-paging/explore.md | 0 .../tui-reader-paging/proposal.md | 0 .../{ => archive}/tui-reader-paging/spec.md | 0 .../{ => archive}/tui-reader-paging/tasks.md | 0 .../tui-reader-paging/verify-report.md | 0 6 files changed, 108 insertions(+) create mode 100644 openspec/changes/archive/tui-reader-paging/archive-report.md rename openspec/changes/{ => archive}/tui-reader-paging/explore.md (100%) rename openspec/changes/{ => archive}/tui-reader-paging/proposal.md (100%) rename openspec/changes/{ => archive}/tui-reader-paging/spec.md (100%) rename openspec/changes/{ => archive}/tui-reader-paging/tasks.md (100%) rename openspec/changes/{ => archive}/tui-reader-paging/verify-report.md (100%) diff --git a/openspec/changes/archive/tui-reader-paging/archive-report.md b/openspec/changes/archive/tui-reader-paging/archive-report.md new file mode 100644 index 0000000..6acc071 --- /dev/null +++ b/openspec/changes/archive/tui-reader-paging/archive-report.md @@ -0,0 +1,108 @@ +# Archive Report: tui-reader-paging + +**Archived**: 2026-05-12 +**Status**: SHIPPED on branch `feat/tui-reader-paging` + +--- + +## Executive Summary + +Adds verse paging (15 verses/page) and verse cursor navigation (`↑`/`↓` with `▶` marker in accent blue) to the reader. Reassigns keybinds: `[`/`]` → page nav, `n`/`p` → chapter nav, `↑`/`↓` → cursor, `/` → palette, `q` → quit. Fixes the existing palette-state bug where `/` would clear the query mid-typing by gating reader keybinds behind `state.kind !== "awaiting"`. Introduces a new `getChapter` application use case so single-verse refs (`john 3:16`) drop the reader into the full chapter page with the cursor positioned on the requested verse. Book-page layout: bordered frame capped at 70 chars centered horizontally, 5-char prefix matching the `ui-sketches.md` Reading view, blank row between verses for vertical rhythm, focused verse rendered in accent color (not inverse). 156/156 tests pass. + +--- + +## Branch Status + +**Branch**: `feat/tui-reader-paging` +**Commits**: 8 — the 3 from the original SDD apply, the SDD-trail/verify-report commit, then 4 user-sanctioned post-verify iterations driven by manual smoke. +**Working tree**: clean, ready to push. + +### Commit Log + +| # | SHA | Message | +|---|---|---| +| 1 | `a85a974` | feat(tui): reader-reducer paging + cursor state and actions | +| 2 | `8764733` | feat(tui): reader-screen page slice + cursor gutter + inverse selection | +| 3 | `7032496` | feat(tui): keybind rebinding + awaiting gate fix; welcome hint | +| 4 | `cb749dc` | docs(openspec): add SDD trail + verify report for tui-reader-paging | +| 5 | `fbd7d71` | fix(tui): reader fetches whole chapter; verse refs position cursor | +| 6 | `5a5ef32` | feat(tui): denser pages + word-wrap with hanging indent | +| 7 | `0ac25aa` | feat(tui): book-page layout — capped width, centered, blank-line verse rhythm | +| 8 | (HEAD) | style(tui): focused verse uses accent color, not inverse | + +--- + +## Verification Summary + +**Initial SDD verify verdict**: PASS WITH WARNINGS +**Tests**: 156/156 pass (127 → +29: 24 reducer + 5 across get-chapter + new PassageFetched scenarios) +**tsc --noEmit**: exits 0 +**Critical**: 0 | **Warning**: 1 (manual smoke pending — now resolved) | **Suggestion**: 1 (cursor color washout — fixed in commit 8) + +### Post-verify sanctioned iterations (commits 5–8) + +Manual PTY smoke after the initial verify surfaced four UX issues. All addressed on the same branch as user-directed improvements, not spec drift: + +1. **C5 — Whole-chapter fetch + cursor positioning (commit `fbd7d71`)**: `john 3:16` was showing only verse 16 instead of dropping into the chapter page. New `getChapter` use case returns the whole chapter regardless of `ref.verses`; `PassageFetched` now positions `cursorIndex` to the index of the verse matching `ref.verses.start`, with `pageStartIndex` derived from `Math.floor(cursorIndex / VERSES_PER_PAGE) * VERSES_PER_PAGE`. `ChapterAdvanced`/`Retreated` reset `ref.verses` to `{1, 1}` so cursor lands on verse 1 of the new chapter. Also caps palette input width to 50 chars; fixes verify suggestion S1 (accent gutter was washing out under DIM). + +2. **C6 — Denser pages + word-wrap (commit `5a5ef32`)**: `VERSES_PER_PAGE` bumped from 8 to 15. Word-wrap with hanging indent (5-space continuation) at `terminalWidth - 8`, floor 20. Tests rewritten to use `VERSES_PER_PAGE` symbol instead of hardcoded indices so future page-size tuning won't churn the suite. + +3. **C7 — Book-page layout (commit `0ac25aa`)**: Wide terminals were producing a single 200-char wall of text that ran off the edges or floated at the top with empty space below. Now the bordered frame caps at 70 chars (`PAGE_MAX_WIDTH`), centers horizontally via `alignItems="center"`, auto-sizes to content height, with `paddingTop={1}` for top margin and a blank `` row between verses. Matches `ui-sketches.md` Reading view (line 122) exactly. + +4. **C8 — Focused verse color (HEAD)**: `INVERSE` attribute on the focused verse text rendered as black-on-light, jarring against a dark terminal. Replaced with `fg={ACCENT_HEX}` per `ui-sketches.md` line 148 ("Focused verse: accent color on the line + ▶ marker in the gutter"). Marker and text now share the accent token, reading as a single highlighted unit. + +--- + +## Residual Manual Step — DONE + +User confirmed PTY smoke after each post-verify iteration: +- Welcome → any key → reader palette +- Type `john 3:16` Enter → drops into page 2 of John 3 with cursor on verse 16 (accent blue) +- `↑` / `↓` move cursor through verses (rolls across page boundaries) +- `[` / `]` page back/forward (silent clamp at edges) +- `n` / `p` chapter nav (resets to verse 1 of new chapter) +- `/` reopens palette (no longer clears mid-typing — `awaiting`-state gate working) +- `q` exits cleanly + +--- + +## Out-of-Scope Follow-Ups + +Tracked for future SDD changes: + +1. **Dynamic `VERSES_PER_PAGE`** from terminal height (current value is a static 15) +2. **Cross-chapter cursor flow** — `↓` past the last verse currently clamps; could trigger `ChapterAdvanced` +3. **Two-page open-book spread** on wide terminals (matches welcome screen aesthetic) +4. **Visual feedback on page-boundary clamp** (current behavior is silent) +5. **Long-word wrap** — a single word longer than wrap width currently overflows; `wordWrap` could break long tokens + +--- + +## Engram Observations (Traceability) + +| Topic | ID | Type | Notes | +|---|---|---|---| +| `sdd/tui-reader-paging/explore` | 279 | architecture | Library survey, state machine sketch, keybind map | +| `sdd/tui-reader-paging/proposal` | 280 | architecture | Locked decisions | +| `sdd/tui-reader-paging/spec` | 281 | architecture | 27 REQs | +| `sdd/tui-reader-paging/tasks` | 282 | architecture | 37 tasks across 3 commits | +| `sdd/tui-reader-paging/apply-progress` | 283 | architecture | All 3 commits, 127 → 151 tests | +| `sdd/tui-reader-paging/verify-report` | 284 | architecture | PASS WITH WARNINGS | +| `sdd/tui-reader-paging/archive-report` | 287 | architecture | This report | + +Plus discovery memories surfaced during this change: +- `opentui/text-children-rule` (#276) — `` cannot nest inside `` (caught in the previous PR but reinforced here) + +--- + +## SDD Cycle Complete + +The `tui-reader-paging` change has been fully: + +- Proposed, specified, designed (via explore), tasked +- Applied (3 original commits) +- Verified (PASS WITH WARNINGS) +- Smoke-tested (4 post-verify user-directed iterations) +- Archived (this report) + +Cycle closed. No blockers for merge. diff --git a/openspec/changes/tui-reader-paging/explore.md b/openspec/changes/archive/tui-reader-paging/explore.md similarity index 100% rename from openspec/changes/tui-reader-paging/explore.md rename to openspec/changes/archive/tui-reader-paging/explore.md diff --git a/openspec/changes/tui-reader-paging/proposal.md b/openspec/changes/archive/tui-reader-paging/proposal.md similarity index 100% rename from openspec/changes/tui-reader-paging/proposal.md rename to openspec/changes/archive/tui-reader-paging/proposal.md diff --git a/openspec/changes/tui-reader-paging/spec.md b/openspec/changes/archive/tui-reader-paging/spec.md similarity index 100% rename from openspec/changes/tui-reader-paging/spec.md rename to openspec/changes/archive/tui-reader-paging/spec.md diff --git a/openspec/changes/tui-reader-paging/tasks.md b/openspec/changes/archive/tui-reader-paging/tasks.md similarity index 100% rename from openspec/changes/tui-reader-paging/tasks.md rename to openspec/changes/archive/tui-reader-paging/tasks.md diff --git a/openspec/changes/tui-reader-paging/verify-report.md b/openspec/changes/archive/tui-reader-paging/verify-report.md similarity index 100% rename from openspec/changes/tui-reader-paging/verify-report.md rename to openspec/changes/archive/tui-reader-paging/verify-report.md