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/archive/tui-reader-paging/explore.md b/openspec/changes/archive/tui-reader-paging/explore.md new file mode 100644 index 0000000..1b7d4fa --- /dev/null +++ b/openspec/changes/archive/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/archive/tui-reader-paging/proposal.md b/openspec/changes/archive/tui-reader-paging/proposal.md new file mode 100644 index 0000000..38f02f2 --- /dev/null +++ b/openspec/changes/archive/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/archive/tui-reader-paging/spec.md b/openspec/changes/archive/tui-reader-paging/spec.md new file mode 100644 index 0000000..937ab20 --- /dev/null +++ b/openspec/changes/archive/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/archive/tui-reader-paging/tasks.md b/openspec/changes/archive/tui-reader-paging/tasks.md new file mode 100644 index 0000000..3ae4ffe --- /dev/null +++ b/openspec/changes/archive/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/archive/tui-reader-paging/verify-report.md b/openspec/changes/archive/tui-reader-paging/verify-report.md new file mode 100644 index 0000000..3af84c1 --- /dev/null +++ b/openspec/changes/archive/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. 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 ff8d550..13d9877 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 15", () => { + expect(VERSES_PER_PAGE).toBe(15); + }); + }); + describe("QueryTyped", () => { it("updates query and clears parseError when awaiting", () => { const state: ReaderState = { kind: "awaiting", query: "", parseError: { kind: "empty_input" } }; @@ -82,14 +106,42 @@ describe("readerReducer", () => { }); describe("PassageFetched", () => { - it("transitions loading → loaded", () => { + 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({ kind: "loaded", passage: mockPassage, ref: johnRef }); + expect(next).toEqual({ + kind: "loaded", + passage: mockPassage, + ref: johnRef, + cursorIndex: 0, + pageStartIndex: 0, + }); + }); + + 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(targetVerse - 1); + expect(next.pageStartIndex).toBe(VERSES_PER_PAGE); + }); + + 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(targetVerse - 1); + expect(next.pageStartIndex).toBe(VERSES_PER_PAGE * 2); }); 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,19 +155,24 @@ 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); }); }); describe("ChapterAdvanced", () => { - it("transitions loaded → loading with chapter + 1", () => { - const state: ReaderState = { kind: "loaded", passage: mockPassage, ref: johnRef }; - 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", () => { @@ -134,29 +191,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 +232,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 +254,107 @@ 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(VERSES_PER_PAGE * 2); + const state = makeLoaded(p, VERSES_PER_PAGE - 1, 0); + const next = dispatch(state, { type: "CursorMovedDown" }); + expect(next).toEqual(makeLoaded(p, VERSES_PER_PAGE, VERSES_PER_PAGE)); + }); + + it("clamps at last verse of last page", () => { + 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); + }); + }); + + 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(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, VERSES_PER_PAGE - 1, 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(VERSES_PER_PAGE * 2); + const state = makeLoaded(p, 3, 0); + const next = dispatch(state, { type: "PageAdvanced" }); + expect(next).toEqual(makeLoaded(p, VERSES_PER_PAGE, VERSES_PER_PAGE)); + }); + + it("clamps at last page", () => { + 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); + }); + }); + + describe("PageRetreated", () => { + it("retreats to previous page and sets cursorIndex to new pageStartIndex", () => { + 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)); + }); + + 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..f5b4693 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 = 15; + 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 => @@ -30,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 } - : 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" @@ -42,19 +52,56 @@ 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 => 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, diff --git a/src/tui/reader/reader-screen.tsx b/src/tui/reader/reader-screen.tsx index 74de21b..e13c8ca 100644 --- a/src/tui/reader/reader-screen.tsx +++ b/src/tui/reader/reader-screen.tsx @@ -1,10 +1,14 @@ 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"; +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; @@ -14,6 +18,8 @@ type ReaderScreenProps = { repo: BibleRepository; }; +const PAGE_MAX_WIDTH = 70; + export function ReaderScreen({ state, dispatch, repo }: ReaderScreenProps) { usePassageFetch(state, dispatch, repo); @@ -27,15 +33,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 ( - - + + + + ); } @@ -58,7 +69,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 "; } @@ -68,20 +79,23 @@ 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 ( {" 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} @@ -106,14 +120,83 @@ function Body({ state, dispatch, frame }: BodyProps) { ); } + const { passage, cursorIndex, pageStartIndex } = state; + const pageVerses = passage.verses.slice(pageStartIndex, pageStartIndex + VERSES_PER_PAGE); + return ( + + ); +} + +// 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; +}) { + const wrapWidth = Math.max(20, boxWidth - 2 - PREFIX_LEN); + return ( - - {state.passage.verses.map((v) => ( - - {`${String(v.number).padStart(3)} `} - {v.text} - - ))} + + {pageVerses.flatMap((v, i) => { + const idx = pageStartIndex + i; + const focused = idx === cursorIndex; + const lines = wordWrap(v.text, wrapWidth); + const rows = lines.map((line, lineIdx) => ( + + {lineIdx === 0 ? ( + <> + + {focused ? "▶" : " "} + + {` ${String(v.number).padStart(2)} `} + + ) : ( + {CONTINUATION_INDENT} + )} + {line} + + )); + // Blank row between verses (not after the last) — matches the sketch's vertical rhythm. + if (i < pageVerses.length - 1) { + rows.push({" "}); + } + return rows; + })} ); } + +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; +} 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 }); 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"} ); }