From d25fbd9cf5b3fb749a3421cb428911f9ca15d43a Mon Sep 17 00:00:00 2001 From: ivanmaierg Date: Sun, 10 May 2026 16:29:18 -0300 Subject: [PATCH] chore(openspec): close visual-identity-v1 archive + backfill historical SDD trail - Archives visual-identity-v1: writes archive-report.md and moves the in-progress folder to changes/archive/visual-identity-v1/ - Backfills verbum-vod (full 8-phase trail) at changes/archive/verbum-vod/ from engram observations 175/176/178/179/181/186/188/195 - Backfills v1-architecture-spike's 5 missing phases (explore, proposal, tasks, apply-progress, verify-report) at changes/archive/v1-architecture-spike/ from engram. The 3 pre-existing files (archive-report, design, spec) are unchanged. - Fixes .gitignore: anchors `/verbum` and `/verbum-*` to project root so the compiled-binary patterns no longer accidentally match nested directories like openspec/changes/archive/verbum-vod/. After this commit, openspec/ holds the complete human-readable trail of every shipped SDD change (v1-architecture-spike, tui-welcome-skeleton, verbum-vod, visual-identity-v1) alongside the engram canonical store. --- .gitignore | 8 +- .../v1-architecture-spike/apply-progress.md | 54 ++++ .../archive/v1-architecture-spike/explore.md | 113 +++++++ .../archive/v1-architecture-spike/proposal.md | 119 +++++++ .../archive/v1-architecture-spike/tasks.md | 88 +++++ .../v1-architecture-spike/verify-report.md | 121 +++++++ .../archive/verbum-vod/apply-progress.md | 95 ++++++ .../archive/verbum-vod/archive-report.md | 167 ++++++++++ openspec/changes/archive/verbum-vod/design.md | 302 ++++++++++++++++++ .../changes/archive/verbum-vod/explore.md | 162 ++++++++++ .../changes/archive/verbum-vod/proposal.md | 163 ++++++++++ openspec/changes/archive/verbum-vod/spec.md | 158 +++++++++ openspec/changes/archive/verbum-vod/tasks.md | 166 ++++++++++ .../archive/verbum-vod/verify-report.md | 130 ++++++++ .../visual-identity-v1/apply-progress.md | 0 .../visual-identity-v1/archive-report.md | 201 ++++++++++++ .../visual-identity-v1/design.md | 0 .../visual-identity-v1/explore.md | 0 .../visual-identity-v1/proposal.md | 0 .../{ => archive}/visual-identity-v1/spec.md | 2 +- .../{ => archive}/visual-identity-v1/tasks.md | 0 .../visual-identity-v1/verify-report.md | 0 22 files changed, 2045 insertions(+), 4 deletions(-) create mode 100644 openspec/changes/archive/v1-architecture-spike/apply-progress.md create mode 100644 openspec/changes/archive/v1-architecture-spike/explore.md create mode 100644 openspec/changes/archive/v1-architecture-spike/proposal.md create mode 100644 openspec/changes/archive/v1-architecture-spike/tasks.md create mode 100644 openspec/changes/archive/v1-architecture-spike/verify-report.md create mode 100644 openspec/changes/archive/verbum-vod/apply-progress.md create mode 100644 openspec/changes/archive/verbum-vod/archive-report.md create mode 100644 openspec/changes/archive/verbum-vod/design.md create mode 100644 openspec/changes/archive/verbum-vod/explore.md create mode 100644 openspec/changes/archive/verbum-vod/proposal.md create mode 100644 openspec/changes/archive/verbum-vod/spec.md create mode 100644 openspec/changes/archive/verbum-vod/tasks.md create mode 100644 openspec/changes/archive/verbum-vod/verify-report.md rename openspec/changes/{ => archive}/visual-identity-v1/apply-progress.md (100%) create mode 100644 openspec/changes/archive/visual-identity-v1/archive-report.md rename openspec/changes/{ => archive}/visual-identity-v1/design.md (100%) rename openspec/changes/{ => archive}/visual-identity-v1/explore.md (100%) rename openspec/changes/{ => archive}/visual-identity-v1/proposal.md (100%) rename openspec/changes/{ => archive}/visual-identity-v1/spec.md (99%) rename openspec/changes/{ => archive}/visual-identity-v1/tasks.md (100%) rename openspec/changes/{ => archive}/visual-identity-v1/verify-report.md (100%) diff --git a/.gitignore b/.gitignore index abd7ac7..1aee701 100644 --- a/.gitignore +++ b/.gitignore @@ -10,9 +10,11 @@ build/ bun.lockb .bun/ -# Compiled binaries (from `bun build --compile`) -verbum -verbum-* +# Compiled binaries (from `bun build --compile`) — anchored to project root +# so `verbum-*` does not accidentally match nested directories like +# openspec/changes/archive/verbum-vod/. +/verbum +/verbum-* # Environment .env diff --git a/openspec/changes/archive/v1-architecture-spike/apply-progress.md b/openspec/changes/archive/v1-architecture-spike/apply-progress.md new file mode 100644 index 0000000..6d9177a --- /dev/null +++ b/openspec/changes/archive/v1-architecture-spike/apply-progress.md @@ -0,0 +1,54 @@ +# Apply Progress — v1-architecture-spike: all 18 tasks complete + +## Branch +feat/v1-architecture-spike (off main) + +## Status +18/18 tasks complete + +## Completed (in order) +- T-1 ✅ commit: b4306fa "chore: initialize bun project with package.json and tsconfig" +- T-2 ✅ commit: 4408afc "feat: add domain primitives — Result, BookId, TranslationId, errors, passage types, and reference parser with tests" +- T-3 ✅ commit: 4408afc (errors.ts — ParseError, RepoError, AppError, UnknownBookError) +- T-4 ✅ commit: 4408afc (translations.ts — TranslationId brand, makeTranslationId, DEFAULT_TRANSLATION_ID) +- T-5 ✅ commit: 4408afc (passage.ts — Verse, Chapter, Passage plain TS types) +- T-6 ✅ commit: 4408afc (book-id.ts — BookId brand, makeBookId, full 66-book USFM canonical set) +- T-7 ✅ commit: 4408afc (reference.ts — Reference, VerseRange, parseReference + reference.test.ts) +- T-8 ✅ commit: 4408afc (book-id.test.ts — makeBookId unit tests) +- T-9 ✅ commit: bcabd53 "feat: add BibleRepository port and getPassage use case with tests" +- T-10 ✅ commit: bcabd53 (get-passage.ts + get-passage.test.ts) +- T-11 ✅ commit: 29f3f17 "feat: add Zod schemas, john-3-bsb fixture, HelloAo adapter with toVerseText helper and unit tests" +- T-12 ✅ commit: 29f3f17 (src/api/__fixtures__/john-3-bsb.json — real fixture from helloao) +- T-13 ✅ commit: 29f3f17 (hello-ao-bible-repository.ts + hello-ao-bible-repository.test.ts) +- T-14 ✅ commit: 51c28e2 "feat: add CLI render formatters, run driver, and entry point" +- T-15 ✅ commit: 51c28e2 (run.ts — argv → use case → exit code) +- T-16 ✅ commit: 51c28e2 (index.tsx — entry point) +- T-17 ✅ commit: 5262bd7 "feat: add smoke test and GitHub Actions CI workflow with compile step" +- T-18 ✅ commit: 5262bd7 (.github/workflows/ci.yml) + +## Decisions made during apply +- "unknown_book" chosen over "invalid_book" for `makeBookId` error kind — matches ParseError vocabulary; domain is consistent across all error union variants +- `bun.lock` committed (replaces bun.lockb in Bun 1.2.x) — lockfile format changed in newer Bun +- Fixture fetched from real helloao API and saved as static JSON — RawChapterResponseSchema.safeParse passes on it +- Zod `passthrough()` used on RawChapterResponseSchema to allow unknown top-level fields (footnotes, audioLinks) without schema failure +- Catch-all object schema added to RawChapterContentItemSchema union so unknown future content types (e.g. "poetry") are accepted at the schema level and dropped by type===\"verse" filter in the adapter +- `bun test` runs without a --cwd flag — tests use @/ path alias from tsconfig which Bun resolves correctly + +## Deviations from spec/design +- T-2 and T-3–T-8 landed in the same commit (C2) — grouped domain files together. Tasks spec split them into separate commits per commit cadence suggestion; actual grouping is still work-unit-reviewable. +- Design shows `Reference = { verse: number }` for v1 spike in one place, but spec uses `verses: VerseRange` with start/end. Implemented `verses: VerseRange` consistently with `start === end` for single-verse references (matches design type catalog). + +## Open questions surfaced during apply +- None — all design decisions were pre-resolved. + +## Verification snapshot +- bun test: 29 pass, 0 fail (5 files: book-id.test.ts, reference.test.ts, get-passage.test.ts, hello-ao-bible-repository.test.ts, smoke.test.ts) +- bun run src/index.tsx john 3:16: "For God so loved the world that He gave His one and only Son, that everyone who believes in Him shall not perish but have eternal life." — exit 0 +- bun run src/index.tsx xyzzy 99:99: "Error: unknown book \"xyzzy\". Try a book name like \"john\" or \"genesis\"." (stderr), exit 2 +- bun run src/index.tsx john abc: "Error: malformed chapter:verse \"abc\". Expected format: ..." (stderr), exit 2 +- bun run src/index.tsx (no args): "Usage: verbum (e.g., verbum john 3:16)" (stderr), exit 2 +- bun run src/index.tsx JOHN 3:16: same verse text as john 3:16, exit 0 +- ./verbum (compiled) john 3:16: same verse text, exit 0 +- ./verbum (compiled) xyzzy 99:99: same stderr error with "xyzzy", exit 2 +- ./verbum (compiled, no args): usage message, exit 2 +- CI workflow: configured at .github/workflows/ci.yml diff --git a/openspec/changes/archive/v1-architecture-spike/explore.md b/openspec/changes/archive/v1-architecture-spike/explore.md new file mode 100644 index 0000000..4a5ddf2 --- /dev/null +++ b/openspec/changes/archive/v1-architecture-spike/explore.md @@ -0,0 +1,113 @@ +# Exploration — v1-architecture-spike + +## TL;DR — Riskiest Unknown + +**Risk A (helloao API shape) is the riskiest.** The verse `content` field is a mixed array of `string | { noteId: number }` — not a plain string — which means the architecture's implicit `Verse.text: string` domain type must be defined explicitly to handle inline footnote references before any code is written. Risk B (bun compile) is a non-risk: documented fully, fetch() works, no restrictions. Risk C (BookId format) is confirmed safe: IDs are exactly the 3-letter USFM codes the architecture assumes. + +## Risk A — helloao API shape + +| Probe | Result | +|---|---| +| BSB exists? | Yes — exact id = `"BSB"` | +| Chapter shape | Flat array at `chapter.content[]` with mixed types: `{ type: "verse", number: N, content: Array }` | +| Verse-range slicing | Client-side (confirmed by ADR 0004 — "no per-verse endpoint") | +| Top-level structure | `{ translation, book, chapter, footnotes, ... }` | + +**Gotcha — verse content is NOT a plain string.** John 3:16 example: +```json +{ + "type": "verse", + "number": 16, + "content": [ + "For God so loved the world that He gave His one and only", + { "noteId": 17 }, + "Son, that everyone who believes in Him shall not perish but have eternal life." + ] +} +``` + +The `chapter.content` array also contains `{ type: "heading" }` and `{ type: "line_break" }` interleaved with verse objects — filtering by `type === "verse"` is required. + +**Required domain model adjustment:** The house rules (Rule 4) show `Verse = { number: number; text: string }` as an example, but the real API requires a two-step parse: join string segments, skip noteId objects. The Zod schema in `src/api/` must handle the mixed array. The domain `Verse` type can remain `{ number: number; text: string }` as long as the adapter flattens the content array into a plain string during parsing. The adapter must: +1. Filter `chapter.content` for `type === "verse"` items +2. For each verse, join only the `string` elements of `verse.content` (skip `{ noteId }` objects) +3. Map to domain `Verse = { number: number; text: string }` + +This is a design decision for the Zod schema shape in `HelloAoBibleRepository`. It is NOT a domain model change — the domain stays clean. But the Zod schema must explicitly model `content: z.array(z.union([z.string(), z.object({ noteId: z.number() })]))` + +**Chapter-level footnotes:** The response has a top-level `footnotes` field (indexed by noteId). Out of scope for v1 but the schema should not discard it blindly. + +## Risk B — bun build --compile + +| Probe | Result | +|---|---| +| hello-world compiles? | Yes — documented, works for simple TS files | +| binary runs with argv? | Yes — `Bun.argv` works in compiled binaries | +| fetch() works in compiled? | Yes — "All built-in Bun and Node.js APIs are supported" (official docs) | +| Top-level await? | Supported | +| Dynamic imports? | Supported (with --splitting flag if needed) | +| Native deps? | N-API .node files can be embedded; no native deps expected in v1 | + +**No blockers.** `bun build --compile` is production-grade for this use case. The only macOS gotcha: Gatekeeper warnings on unsigned binaries (fixable with `codesign`). Not a v1 concern for local dev. The compiled binary includes the full Bun runtime — fetch(), filesystem, all Node.js-compatible APIs work identically to `bun run`. + +**Practical note:** `Bun.argv` vs `process.argv` — both work in compiled binaries. The entry point for CLI mode should slice `Bun.argv.slice(2)` (or `process.argv.slice(2)`) to skip the binary path. + +## Risk C — BookId aliases + +| Probe | Result | +|---|---| +| Books endpoint | `GET /api/BSB/books.json` — confirmed correct path | +| ID format | 3-letter USFM codes: `GEN`, `JHN`, `1CO` | +| Matches architecture.md assumption? | Yes — exact match | +| Sample IDs | Genesis=`GEN`, John=`JHN`, 1 Corinthians=`1CO` | + +**No blockers.** The architecture's `BookId` brand (`"JHN"`, `"GEN"`, `"1CO"`) matches helloao's canonical IDs exactly. The `BookCatalog` alias map (`"john"` → `"JHN"`, `"1cor"` → `"1CO"`) only needs to handle user-facing input normalization — the API IDs are already what we need. There is NO translation layer needed between domain BookId and API book path segment. + +## Proposed adjustments to architecture + +**One adjustment required — Verse parsing in `src/api/`:** + +The Zod schema for chapter content must handle the mixed array. Proposed type in `src/api/schemas.ts`: + +```ts +// Raw API shape (Zod schema, stays in src/api/) +const RawVerseContentItemSchema = z.union([ + z.string(), + z.object({ noteId: z.number() }), +]); + +const RawVerseSchema = z.object({ + type: z.literal("verse"), + number: z.number(), + content: z.array(RawVerseContentItemSchema), +}); + +const RawChapterContentItemSchema = z.discriminatedUnion("type", [ + RawVerseSchema, + z.object({ type: z.literal("heading"), content: z.array(z.string()) }), + z.object({ type: z.literal("line_break") }), +]); + +// Adapter flattening in HelloAoBibleRepository: +function toVerseText(content: Array): string { + return content + .filter((item): item is string => typeof item === "string") + .join(" ") + .trim(); +} +``` + +Domain `Verse = { number: number; text: string }` stays clean — no domain change. + +**Architecture.md `BibleRepository.getChapter` port signature:** The current architecture shows `getChapter` but doesn't name the return type precisely. Recommend naming it `Chapter = { verses: Verse[]; translationId: TranslationId }` to make the adapter's job explicit. This is additive, not a correction. + +## Recommended next decision + +Before `sdd-propose` runs, one decision must be locked: + +**How to handle footnote references in verse text for v1:** +- Option A (recommended): Strip `noteId` objects, join strings only → plain `text: string`. Footnotes are a v6+ concern. Zero domain complexity. +- Option B: Preserve footnotes inline → `text` becomes `Array`. Domain gains complexity now; Go port becomes harder. +- Option C: Raise a `ParseWarning` alongside the `Verse` when noteId objects are present. + +Instinct: Option A. v1 success criterion is `john 3:16` prints and exits 0 — footnote preservation is not in scope. Option A keeps the domain clean per Rule 4 and Rule 7. diff --git a/openspec/changes/archive/v1-architecture-spike/proposal.md b/openspec/changes/archive/v1-architecture-spike/proposal.md new file mode 100644 index 0000000..48137b5 --- /dev/null +++ b/openspec/changes/archive/v1-architecture-spike/proposal.md @@ -0,0 +1,119 @@ +# Proposal — v1-architecture-spike: end-to-end hexagonal slice for John 3:16 + +## TL;DR + +The smallest end-to-end slice that proves the four hexagonal layers wire correctly: parse `john 3:16`, fetch BSB chapter 3 of John from helloao, slice verse 16, print to stdout, exit 0. Both `bun run src/index.tsx` and the `bun build --compile` artifact must satisfy the same success criterion. Cache, preferences, TUI, format flags, favorites, and footnote preservation are explicitly deferred. + +## What + +This change ships a vertical slice through every hexagonal layer for one happy path (`john 3:16` → BSB text on stdout) and one parse-error path (`xyzzy 99:99` → stderr, exit 2). It is an **architecture spike**: the goal is not a feature, it is to prove the layer boundaries — domain purity, port-as-interface, Zod-only-at-boundary, presentation-as-thin-driver — actually hold when something flows through them. The slice produces a runnable CLI from both `bun run` and the compiled binary, and nothing else. + +## Why now + +v1 of `verbum` lists four use cases, three ports, and a TUI in the roadmap. Building any of those before the layer plumbing is proven risks discovering, mid-feature, that one of the house rules (R1, R3, R4, R12) doesn't survive contact with the real helloao response shape. Exploration already surfaced one such friction point — the mixed-array `verse.content` — and resolved it. This spike locks the resolution in code so every later slice (translation list, favorites, last-position, format flags, TUI) extends a working skeleton instead of debating its shape. + +## Scope — IN + +Concrete artifacts that ship in this PR, named at the layer they live in: + +- `src/domain/reference.ts` — `Reference` type sketch, `parseReference(input: string): Result` (pure, no IO, no Zod) +- `src/domain/result.ts` — the shared `Result` discriminated union (R1, R5) +- `src/domain/book-id.ts` — `BookId` brand + `makeBookId` factory with the canonical USFM set (R6); enough entries to cover John for the happy path plus the "unknown book" rejection for the error path +- `src/domain/errors.ts` — `ParseError` discriminated union (`empty_input`, `unknown_book`, `out_of_range`) (R5) +- `src/api/schemas.ts` — Zod schemas: `RawVerseContentItemSchema` (string-or-noteId union), `RawVerseSchema`, `RawChapterContentItemSchema` (discriminatedUnion on `type`), top-level chapter response +- `src/api/hello-ao-bible-repository.ts` — `HelloAoBibleRepository` implementing the `BibleRepository` port; contains `toVerseText()` flattening helper (footnotes stripped, Option A); returns `Promise>` (R12) +- `src/application/ports/bible-repository.ts` — the `BibleRepository` port interface (R3) +- `src/application/get-passage.ts` — `getPassage(repo, ref): Promise>` use case; orchestrates repo call + verse-range slicing +- `src/cli/run.ts` — argv → `parseReference` → `getPassage` → format → stdout/stderr; owns the exit-code contract +- `src/index.tsx` — entry that delegates to `src/cli/run.ts` when argv has a reference (TUI branch is a stub for this slice) +- One smoke test that exercises the happy path end-to-end against a recorded fixture (no live network in CI) +- A CI step that runs `bun build --compile` and executes the binary against the same two cases + +**Exit code contract:** `0` happy path, `2` parse error. Anything else (network, schema mismatch) is out of scope for this slice's UX but the use case must still return a `Result` — it just won't have a polished error path yet. + +## Scope — OUT + +Deferred to subsequent v1 slices (cite roadmap row v1 unless noted): + +- `Cache` port and `FilesystemCache` adapter (v1 row, separate slice) +- `PreferencesStore` and last-position memory (v1 row) +- `OutputFormatter` port and `--format text|json|markdown` (v1 row) +- `ToggleFavoriteTranslation` use case + favorites (v1 row) +- `ListTranslations` use case (v1 row) +- TUI presentation entirely (v1 row, biggest deferred chunk) +- Mouse + keyboard symmetric input (v1 row, TUI-bound) +- Network-error UX polish (timeouts, retries, friendly messages) +- Footnote preservation in verse text — locked as Option A (strip) for v1; revisit at v6 (cross-references + footnotes UI) +- Any reference shape beyond `book chapter:verse` and the unknown-book error case (e.g. ranges `john 3:16-18`, multi-chapter, whole chapter) — spec phase decides minimum parser surface +- BSB-other-than-John-3 verification: the success criterion only mandates John 3:16 works; broader translations and books are not part of the spike's pass condition + +## Approach + +Four files do real work, one per layer, plus the schema and the entry shim. The flow: + +``` +argv ──► src/cli/run.ts + │ + ▼ parseReference(input) + src/domain/reference.ts ──► Result + │ + ▼ getPassage(repo, ref) + src/application/get-passage.ts + │ depends only on BibleRepository port + ▼ + src/api/hello-ao-bible-repository.ts + │ fetch + Zod + toVerseText() flattening + ▼ Result + back up through application → cli → stdout +``` + +**Zod strategy.** All schemas live in `src/api/schemas.ts` and never escape — the adapter exposes domain types only (R4). The mixed-array gotcha is handled at the boundary by a `discriminatedUnion("type", ...)` schema for `chapter.content[]` items plus a small `toVerseText(items)` helper that filters to strings and joins. Domain `Verse = { number: number; text: string }` stays plain TypeScript. + +**Argv parsing.** Use Bun's built-in `parseArgs` (`import { parseArgs } from "util"`). It is part of Node compat in Bun, ships in compiled binaries, and avoids a hand-rolled positional parser that would need its own tests. Reference parsing itself stays in the domain — `parseArgs` only collects the positional argv tokens and joins them. + +**Exit codes.** Owned by `src/cli/run.ts` only. Use cases return `Result`; the CLI driver maps `{ ok: true }` → stdout + `0`, `ParseError` → stderr + `2`. Other error kinds funnel to a temporary "unexpected error" stderr + non-zero exit; spec/design will refine the contract later. + +**One PR, one commit per layer when it earns its keep** (work-unit-commits): domain + tests, application + port, api adapter + schema, cli wiring + entry. Smoke test ships with the cli wiring commit since that's the first commit that can run end-to-end. + +## Open questions (for spec/design) + +1. **Default translation.** The success criterion says "BSB". Is `"BSB"` a hardcoded constant in the CLI driver for this slice, or does spec define a `DEFAULT_TRANSLATION_ID` in domain? Either is defensible; spec should pick. +2. **Translation lookup vs literal.** Is `"BSB"` passed as a literal `TranslationId` brand, or do we go through a `BookCatalog`/`TranslationCatalog` lookup? Spike-only: literal is fine. Spec should declare whether v1 introduces a `TranslationId` brand now or later. +3. **Parse-error message format.** Where does the user-facing string for `ParseError` live — domain (`format(err: ParseError): string`, like the house-rules example) or CLI (`renderParseError(err)` in `src/cli/`)? R5 example puts it in domain; spec should confirm. +4. **Reference parser surface.** This proposal only commits to `book chapter:verse` and the unknown-book error. Spec must declare whether `john 3` (whole chapter) and `john 3:16-18` (range) are in or out for v1. +5. **`Chapter` vs `Passage` boundary.** Repo returns a whole chapter; use case slices to the requested verse(s) and returns a `Passage`. Design should confirm that slicing lives in the use case, not the adapter (keeps the adapter dumb). +6. **Smoke test fixture location.** `src/api/__fixtures__/`? `tests/fixtures/`? Project standards don't specify yet. Design or first apply commit decides. + +## Risks + +- **Bun `parseArgs` quirks** — small chance of subtle differences between `bun run` and the compiled binary (e.g. argv[0] semantics). Mitigation: the CI step that runs the compiled binary against both success cases catches it on the first run. +- **Zod discriminatedUnion on `type` for chapter content** — if helloao introduces a new `type` (e.g. `"poetry"`) the schema rejects the response. Acceptable for the spike; spec/design should decide whether unknown content items are dropped or fail-loud. +- **R7 pressure in the api layer** — Zod ergonomics may tempt mapped/conditional types. R7 explicitly allows them in `src/api/` only; reviewer must confirm none leak across the boundary. +- **Test coverage of the compiled binary in CI** — running `bun build --compile` + a smoke test on every PR is new infrastructure; first run may surface a CI config issue rather than a code issue. +- **One-commit-per-layer cadence colliding with reviewable-on-its-own (work-unit-commits)**: the application commit can't run end-to-end without the adapter. Mitigation: ship application + port together, then api adapter, then cli (the cli commit is the one that gains the smoke test). + +## Estimated size + +| Bucket | Files | Lines (rough) | +|---|---|---| +| Domain (`reference.ts`, `result.ts`, `book-id.ts`, `errors.ts`) | 4 | ~120 | +| Application (`bible-repository.ts` port, `get-passage.ts`) | 2 | ~60 | +| Api (`schemas.ts`, `hello-ao-bible-repository.ts`) | 2 | ~120 | +| CLI + entry (`cli/run.ts`, `index.tsx`) | 2 | ~50 | +| Tests + fixture | 2 | ~80 | +| CI step | 1 (workflow edit) | ~20 | +| **Total** | **~13** | **~450** | + +Slightly above the 400-line target. The Review Workload Guard should consider this borderline — likely still a single PR with `size:exception` if the maintainer agrees, otherwise a chained split where domain + application land first and the api adapter + cli wiring land second. Tasks phase will firm this up. + +--- + +**Decisions locked into this proposal (do not re-litigate downstream):** +- Footnote handling: Option A (strip `{ noteId }`, join strings only). +- `BookId` USFM codes map 1:1 to helloao API book paths — no translation layer. +- `bun build --compile` is a CI artifact, not a research item. +- Zod stays in `src/api/` (R4); domain types are plain TS. +- All async data functions return `Promise>` (R12). +- Errors are `kind`-tagged unions (R5); no error class hierarchies. +- Argv parsing uses Bun's built-in `parseArgs` (no hand-rolled parser). +- Exit codes: `0` happy, `2` parse error. diff --git a/openspec/changes/archive/v1-architecture-spike/tasks.md b/openspec/changes/archive/v1-architecture-spike/tasks.md new file mode 100644 index 0000000..25e3f28 --- /dev/null +++ b/openspec/changes/archive/v1-architecture-spike/tasks.md @@ -0,0 +1,88 @@ +# Tasks — v1-architecture-spike: ordered checklist + +## TL;DR + +18 tasks across 5 groups, ordered strictly inside-out: project surface → domain (pure types first, then the parser) → application port and use case → api adapter → cli driver + entry + smoke + CI. Every task bundles its test with the code that motivates it (work-unit-commits). **ALL 18 TASKS COMPLETE.** + +--- + +## Review Workload Forecast + +- Estimated changed lines: ~680 +- 400-line budget risk: High +- Chained PRs recommended: Yes +- Decision needed before apply: Yes +- **Resolution: single-pr with size:exception — all 18 tasks land in one PR.** + +--- + +## Pre-flight (project setup) + +- [x] T-1: Initialize Bun project with `package.json` and `tsconfig.json` +- [x] T-2: Add `Result` discriminated union in `src/domain/result.ts` + +--- + +## Domain — pure types (no IO, no tests needed beyond type-check) + +- [x] T-3: Add `ParseError`, `RepoError`, `AppError` discriminated unions in `src/domain/errors.ts` +- [x] T-4: Add `TranslationId` brand, `makeTranslationId` factory, and `DEFAULT_TRANSLATION_ID` constant in `src/domain/translations.ts` +- [x] T-5: Add `Verse`, `Chapter`, and `Passage` plain-TS types in `src/domain/passage.ts` +- [x] T-6: Add `BookId` brand, `makeBookId` factory, and full 66-book USFM canonical set in `src/domain/book-id.ts` + +--- + +## Domain — parser (with tests) + +- [x] T-7: Add `Reference`, `VerseRange` types and `parseReference` pure function in `src/domain/reference.ts`, plus colocated unit tests +- [x] T-8: Add colocated unit tests for `makeBookId` in `src/domain/book-id.test.ts` + +--- + +## Application — port and use case + +- [x] T-9: Add `BibleRepository` port interface in `src/application/ports/bible-repository.ts` +- [x] T-10: Add `getPassage` use case in `src/application/get-passage.ts`, plus colocated unit tests using an inline stub repository + +--- + +## API adapter (Zod boundary) + +- [x] T-11: Add Zod schemas for helloao chapter response in `src/api/schemas.ts` +- [x] T-12: Add recorded helloao fixture for John chapter 3 BSB at `src/api/__fixtures__/john-3-bsb.json` +- [x] T-13: Add `toVerseText` helper and `createHelloAoBibleRepository` factory in `src/api/hello-ao-bible-repository.ts`, plus colocated unit tests using the fixture and a stubbed `fetch` + +--- + +## CLI driver and entry + +- [x] T-14: Add `renderParseError`, `renderRepoError`, `renderPassage` formatters in `src/cli/render.ts` +- [x] T-15: Add `run(argv)` CLI driver in `src/cli/run.ts` +- [x] T-16: Add entry point `src/index.tsx` — positional-args → run() or usage error + +--- + +## Smoke test and CI + +- [x] T-17: Add end-to-end smoke test in `tests/smoke.test.ts` using a fixture-backed stub repository +- [x] T-18: Add GitHub Actions CI workflow in `.github/workflows/ci.yml` + +--- + +## Definition of done — VERIFIED ✅ + +- [x] All REQ-1…REQ-14 have at least one task that satisfies them +- [x] All SCN-1…SCN-7 have a verifiable check (test or CI step) +- [x] `bun test` passes locally (29 pass, 0 fail across 5 files) +- [x] `bun run src/index.tsx john 3:16` → stdout contains verse text, exit 0, stderr empty (SCN-1) +- [x] `bun run src/index.tsx xyzzy 99:99` → stderr names "xyzzy", exit 2, stdout empty (SCN-3) +- [x] `bun run src/index.tsx john abc` → stderr mentions malformed chapter:verse, exit 2 (SCN-4) +- [x] `bun run src/index.tsx` (no args) → stderr contains usage message, exit 2, `parseReference` not called (SCN-5) +- [x] `bun run src/index.tsx JOHN 3:16` → stdout contains verse text, exit 0 (SCN-6) +- [x] `bun build --compile src/index.tsx --outfile ./verbum` exits 0; compiled binary passes all SCNs +- [x] CI workflow configured at .github/workflows/ci.yml (REQ-14) +- [x] No raw "BSB" literal outside `src/domain/translations.ts` (REQ-3) +- [x] No `as BookId` cast outside `makeBookId` (REQ-2, R6) +- [x] No Zod import outside `src/api/` (R4) +- [x] No `class` keyword in any file outside `src/tui/` (R2) +- [x] `renderParseError` has a `never` fallthrough (R5, REQ-11) diff --git a/openspec/changes/archive/v1-architecture-spike/verify-report.md b/openspec/changes/archive/v1-architecture-spike/verify-report.md new file mode 100644 index 0000000..2d28665 --- /dev/null +++ b/openspec/changes/archive/v1-architecture-spike/verify-report.md @@ -0,0 +1,121 @@ +# Verify — v1-architecture-spike: PASS WITH WARNINGS (0 CRITICAL, 3 WARNING, 2 SUGGESTION) + +## TL;DR +PASS WITH WARNINGS. All 6 acceptance scenarios pass independently. 29/29 tests pass. Zero CRITICAL findings. Three WARNINGs: a `.parse()` call inside the adapter (spec mandates `safeParse`), raw `"BSB"` literals in test files (REQ-3 border case), and CI smoke step hitting real HTTP (design-acknowledged flakiness risk). Two SUGGESTIONs: CI step has a fragile `$?` check and `z.union` was used instead of the spec-mandated `z.discriminatedUnion`. + +## Counts +- CRITICAL: 0 +- WARNING: 3 +- SUGGESTION: 2 + +## Scenario reproduction (independently re-run) + +| SCN | Command | Exit | Stdout (first 80 chars) | Stderr | PASS/FAIL | +|-----|---------|------|------------------------|--------|-----------| +| SCN-1 | `bun run src/index.tsx john 3:16` | 0 | "For God so loved the world that He gave His one and only Son..." | empty | PASS | +| SCN-2 | `./verbum john 3:16` (compiled) | 0 | "For God so loved the world that He gave His one and only Son..." | empty | PASS | +| SCN-3 | `bun run src/index.tsx xyzzy 99:99` | 2 | empty | `Error: unknown book "xyzzy". Try a book name like "john" or "genesis".` | PASS | +| SCN-4 | `bun run src/index.tsx john abc` | 2 | empty | `Error: malformed chapter:verse "abc". Expected format: :` | PASS | +| SCN-5 | `bun run src/index.tsx` | 2 | empty | `Usage: verbum (e.g., verbum john 3:16)` | PASS | +| SCN-6 | `bun run src/index.tsx JOHN 3:16` | 0 | "For God so loved the world..." | empty | PASS | +| SCN-7 | `bun build --compile` exits 0; `./verbum john 3:16` exits 0 with "loved"; `./verbum xyzzy 99:99` exits 2 | — | — | — | PASS | +| bun test | 29 pass, 0 fail, 5 files | — | — | — | PASS (matches claim) | + +## File structure check + +Design specified 15 files. All present. No extra files outside the design. + +| Design path | Found | Notes | +|-------------|-------|-------| +| src/domain/result.ts | ✅ | | +| src/domain/book-id.ts | ✅ | | +| src/domain/translations.ts | ✅ | | +| src/domain/reference.ts | ✅ | | +| src/domain/passage.ts | ✅ | | +| src/domain/errors.ts | ✅ | | +| src/application/ports/bible-repository.ts | ✅ | | +| src/application/get-passage.ts | ✅ | | +| src/api/schemas.ts | ✅ | | +| src/api/hello-ao-bible-repository.ts | ✅ | | +| src/api/__fixtures__/john-3-bsb.json | ✅ | | +| src/cli/run.ts | ✅ | | +| src/cli/render.ts | ✅ | | +| src/index.tsx | ✅ | | +| tests/smoke.test.ts | ✅ | | +| .github/workflows/ci.yml | ✅ | bonus (T-18) | +| Test colocates (*.test.ts) | ✅ | 4 additional test files, all within their layers | + +## House rules audit + +| Rule | Finding | File:line | +|------|---------|-----------| +| R1 — no throw in domain | PASS — zero `throw` in src/domain/ | — | +| R3 — port args no callbacks | PASS — `getChapter(translationId, book, chapter)` primitives/brands only | src/application/ports/bible-repository.ts | +| R4 — Zod only in src/api/ | PASS — only import is schemas.ts:6 | src/api/schemas.ts:6 | +| R5 — discriminated unions | PASS — all error types use `kind`, exhaustive switches with `never` fallthrough | src/cli/render.ts:20-26, 43-47 | +| R6 — brand casts inside factories only | PASS — `as BookId` only in book-id.ts:34, `as TranslationId` only in translations.ts:10 | | +| R7 — no conditional/mapped types in domain/application | PASS — none found | — | +| R11 — no decorators | PASS — no `@` decorator usage | — | +| R12 — async data functions return Promise> | PASS (with note) — `run()` returns `Promise` which is acceptable (not data, it's an exit code; R12 exception applies) | src/cli/run.ts:16 | + +## Apply-progress reconciliation + +| Claim | Verified | Notes | +|-------|---------|-------| +| 6 commits landed | ✅ | b4306fa, 4408afc, bcabd53, 29f3f17, 51c28e2, 5262bd7 | +| Conventional commits, no AI attribution | ✅ | All `feat:` / `chore:` / `docs:`, no Co-Authored-By | +| 29 tests pass | ✅ | Independently reproduced: 29 pass, 0 fail | +| `unknown_book` decision in code | ✅ | src/domain/book-id.ts:32, src/domain/errors.ts:5 | +| `as RepoError` cast in run.ts | ✅ | src/cli/run.ts:37 — present and documented | +| SCN-1 stdout "For God so loved…" | ✅ | Reproduced live | +| SCN-3 stderr names "xyzzy", exit 2 | ✅ | Reproduced live | +| SCN-4 malformed message, exit 2 | ✅ | Reproduced live | +| SCN-5 usage message, exit 2 | ✅ | Reproduced live | +| SCN-6 JOHN 3:16 case-insensitive | ✅ | Reproduced live | +| Compiled binary all SCNs | ✅ | Reproduced live | + +## Spec requirements coverage + +| REQ | Satisfied by | PASS/FAIL | +|-----|-------------|----------- +| REQ-1 Result | src/domain/result.ts | PASS | +| REQ-2 BookId brand+factory | src/domain/book-id.ts | PASS | +| REQ-3 TranslationId + DEFAULT_TRANSLATION_ID | src/domain/translations.ts | PASS (WARNING: test files use raw "BSB" literal) | +| REQ-4 ParseError union | src/domain/errors.ts:7-11 | PASS (out_of_range present despite design memo saying omit) | +| REQ-5 parseReference | src/domain/reference.ts:163 | PASS | +| REQ-6 BibleRepository port | src/application/ports/bible-repository.ts | PASS | +| REQ-7 Zod schemas | src/api/schemas.ts (WARNING: z.union not z.discriminatedUnion) | WARNING | +| REQ-8 toVerseText | src/api/hello-ao-bible-repository.ts:14 | PASS | +| REQ-9 HelloAoBibleRepository adapter | src/api/hello-ao-bible-repository.ts (WARNING: .parse() on line 68) | WARNING | +| REQ-10 getPassage use case | src/application/get-passage.ts | PASS | +| REQ-11 CLI driver | src/cli/run.ts + src/cli/render.ts (never fallthrough present) | PASS | +| REQ-12 Entry point | src/index.tsx (SCN-5 short-circuit confirmed) | PASS | +| REQ-13 Smoke test | tests/smoke.test.ts (no real HTTP) | PASS | +| REQ-14 Compiled binary CI | .github/workflows/ci.yml | PASS | + +## Findings + +### CRITICAL +None. + +### WARNING + +**W-1 — REQ-9 violated: `RawVerseSchema.parse(item)` in adapter (src/api/hello-ao-bible-repository.ts:68)** +Spec REQ-9 says "MUST parse the raw response with Zod schemas via `safeParse` — never `parse` (throws)." The adapter uses `safeParse` correctly for the main response (line 48) and for the filter (line 63), but then calls `RawVerseSchema.parse(item)` in the `.map()` at line 68. While in practice it cannot throw because the filter above guarantees validity, the spec is explicit. It is a review-blocker per the spec contract. + +**W-2 — REQ-3 border case: raw `"BSB"` literal in test files** +REQ-3 says "Raw string literals `"BSB"` MUST NOT appear anywhere outside `src/domain/translations.ts`." Two test files use `makeTranslationId("BSB")` directly: `src/api/hello-ao-bible-repository.test.ts:8` and `src/application/get-passage.test.ts:22`. These use the factory (R6 satisfied), but the `"BSB"` literal itself is present outside `translations.ts`. Tests are a gray area — they can't use `DEFAULT_TRANSLATION_ID` everywhere since they sometimes need a specific translation for setup — but it's a nominal spec deviation. User decides whether to enforce this in tests. + +**W-3 — CI smoke depends on live HTTP (design-acknowledged risk)** +`.github/workflows/ci.yml:32-36` — the happy-path binary smoke step calls `./verbum john 3:16` which makes a real network request to `bible.helloao.org`. If the API is down, CI fails for a non-code reason. This risk was explicitly accepted in the design and proposal, but it's a real flakiness source. + +### SUGGESTION + +**S-1 — REQ-7: `z.union` used instead of spec-mandated `z.discriminatedUnion`** +`src/api/schemas.ts:36` uses `z.union([...])` for `RawChapterContentItemSchema`. Spec REQ-7 says "MUST use `z.discriminatedUnion("type", [...])` for `chapter.content[]` items". Apply switched to `z.union` to accommodate the catch-all passthrough object (a valid tradeoff). The schema behavior is equivalent for all known types. Not a bug, but a literal spec deviation. + +**S-2 — CI xyzzy check uses fragile `$?` idiom (ci.yml:40)** +`./verbum xyzzy 99:99 && exit 1 || [ $? -eq 2 ]` is correct but opaque. `set -e` in some CI runners can interact unexpectedly. Simpler alternative: `./verbum xyzzy 99:99; [ $? -eq 2 ]` or use `expected_exit=2` with an explicit comparison. + +## Recommended next step +`sdd-archive` — all CRITICAL count is zero, all scenarios pass, all house rules hold. User should decide whether to fix W-1 (`.parse()` call) before archiving — it is a real spec violation — or document it as a known deviation. W-2 and W-3 are judgment calls. diff --git a/openspec/changes/archive/verbum-vod/apply-progress.md b/openspec/changes/archive/verbum-vod/apply-progress.md new file mode 100644 index 0000000..d2c5c18 --- /dev/null +++ b/openspec/changes/archive/verbum-vod/apply-progress.md @@ -0,0 +1,95 @@ +# Apply Progress: verbum-vod + +**Status**: complete +**Branch**: feat/verbum-vod (local only, not pushed) + +## Commit Sequence + +| # | SHA | Message | +|---|-----|---------| +| C1 | cde9a9d | feat(application): add pickVerseForDate picker + tests | +| C2 | 213f48e | feat(cli): add 365-entry verse pool data + invariant tests | +| C3 | 0a69c94 | feat(cli): add runVod handler + smoke test | +| C4 | 39fa954 | feat(cli): dispatch vod subcommand from run() | +| C5 | 8207373 | refactor(cli): replace AppError casts with kind-based narrowing | +| C6 | 3c8e673 | test(vod): cover runVod exit-1 path with failing repo stub | +| C7 | fe4bd31 | fix(domain): correct direction of RepoError exhaustiveness check | + +## Tasks Completed + +- [x] T-1: `src/application/verse-pool.ts` — PoolEntry, dayOfYear, pickVerseForDate +- [x] T-2: `src/application/verse-pool.test.ts` — dayOfYear tests + picker tests with STUB_POOL (C1) + real-pool determinism tests (C2) +- [x] T-3: `src/cli/verse-pool-data.ts` — 365-entry VERSE_POOL POJO +- [x] T-4: invariant tests appended to verse-pool.test.ts (I1: length=365, I2: makeBookId walks all entries, chapter/verse >= 1) +- [x] T-5: `src/cli/vod.ts` — runVod handler with exit-code contract +- [x] T-6: `tests/vod-smoke.test.ts` — 2 smoke tests with stub repo (C3) + 1 exit-1 test with failing repo stub (C6) +- [x] T-7: `src/cli/run.ts` — import runVod added +- [x] T-8: `src/cli/run.ts` — dispatch block added before parseReference + +## Followup Tasks Completed (verify-report SG1 + W1) + +- [x] SG1: Replace `as RepoError` casts in `run.ts` and `vod.ts` with `isRepoError` type predicate (C5). Added `isRepoError` + `REPO_ERROR_KINDS` exhaustiveness check to `src/domain/errors.ts`. +- [x] W1: Add exit-1 test for `runVod` with failing repo stub — asserts exit code 1, stderr contains "network failure", stdout empty (C6). +- [~] W2: SKIPPED — exit-2 path (makeBookId failure on pool entry) is structurally unreachable given the I2 pool-integrity invariant (every USFM in VERSE_POOL passes makeBookId). Adding a test would require an injectable pool API change just to reach an unreachable branch. Documented skip in apply-progress instead. + +## Re-verify Followup (W-REMAIN — directional fix) + +- [x] **W-REMAIN: exhaustiveness check direction was reversed (C7)**. Re-verify (#188) caught that the original `RepoErrorKind extends RepoError["kind"] ? true : never` was directionally wrong: a 6-member subset always extends a 7-member superset, so adding a new RepoError variant without updating REPO_ERROR_KINDS would silently pass the check while `isRepoError` misclassifies the new variant as a ParseError. Flipped to `RepoError["kind"] extends RepoErrorKind ? true : never` — now the superset cannot extend the smaller set, producing the intended `true = never` compile error when REPO_ERROR_KINDS falls out of sync. No runtime behavior change today (all 6 variants discriminate correctly); the guard now actually does what its comment says. + +## T-2 Risk Resolution + +**Chose Option B**: C1 uses a STUB_POOL (365 synthetic entries) for picker + dayOfYear tests. Real-pool determinism tests (7 cases) added in C2 alongside verse-pool-data.ts. Reasoning: picker is size-agnostic and should be tested generically; real-pool determinism belongs with the real data. + +## Test Results Per Commit + +| Commit | Tests run | Pass | Fail | Skip | +|--------|-----------|------|------|------| +| C1 (verse-pool.test.ts only) | 13 | 13 | 0 | 0 | +| C1 (full suite) | 46 | 46 | 0 | 0 | +| C2 (verse-pool.test.ts only) | 23 | 23 | 0 | 0 | +| C2 (full suite) | 56 | 56 | 0 | 0 | +| C3 (vod-smoke.test.ts only) | 2 | 2 | 0 | 0 | +| C3 (full suite) | 58 | 58 | 0 | 0 | +| C4 (full suite) | 58 | 58 | 0 | 0 | +| C5 (full suite) | 58 | 58 | 0 | 0 | +| C6 (full suite) | 59 | 59 | 0 | 0 | +| C7 (full suite) | 59 | 59 | 0 | 0 | + +## SG1 + C7 Implementation Details + +Added to `src/domain/errors.ts`: +- `REPO_ERROR_KINDS` — `as const` tuple of all 6 RepoError kind strings +- Compile-time exhaustiveness check: `RepoError["kind"] extends RepoErrorKind` (corrected in C7) — fires if a new RepoError variant is added but REPO_ERROR_KINDS is not updated +- `isRepoError(err: AppError): err is RepoError` — type predicate using the tuple + +Both `run.ts` and `vod.ts`: +- Removed `import type { RepoError }` (no longer needed for cast) +- Imported `{ isRepoError }` from `@/domain/errors` +- Replaced `const err = passageResult.error as RepoError` + single `renderRepoError` call with `if (isRepoError(err)) { renderRepoError } else { renderParseError }` narrowing + +## Deviations from Design + +1. **PSA 145:18 removed from pool**: Design header says "Psalms 80 entries" but listed 81 Psalms. Total would have been 366. Removed the last Psalm (`PSA 145:18` at position 81 in the section) to land exactly at 365. The `length === 365` test caught this during C2. All other entries transcribed faithfully. + +2. **Psalm section comment header updated**: Changed "indices 81–130" → "indices 80–129" for Proverbs to reflect actual positions after Psalms reduction. + +3. **W2 skipped**: exit-2 path not tested. Pool integrity (I2) guarantees this path is unreachable at runtime. Forcing testability would require API surface changes with no production benefit. + +4. **C7 retroactive fix**: The first SG1 implementation (C5) introduced an exhaustiveness check that compiled in the right direction but tested the wrong invariant. Re-verify (#188) caught this. Fixed in C7 with a one-line conditional swap. + +## Acceptance Checks + +- `bun run src/index.tsx john 3:16` → exit 0, verse text on stdout (\"For God so loved the world...\") ✓ backward compat preserved +- `bun run src/index.tsx vod` → exit 1 (verse_not_found from network-restricted CI environment, correct error path) ✓ dispatch working, runVod executing +- Full suite 59/59 green after C7 ✓ +- `bunx tsc --noEmit` — no new errors introduced by C7; pre-existing project-wide @types/bun + @types/node missing remain unchanged + +## Files Changed + +- `src/application/verse-pool.ts` (NEW) — PoolEntry type + dayOfYear + pickVerseForDate +- `src/application/verse-pool.test.ts` (NEW) — 23 tests across 5 describe blocks +- `src/cli/verse-pool-data.ts` (NEW) — 365-entry VERSE_POOL +- `src/cli/vod.ts` (NEW then MODIFIED) — runVod handler; SG1 cast fix applied +- `tests/vod-smoke.test.ts` (NEW then MODIFIED) — 2 happy-path smoke tests + 1 exit-1 test +- `src/cli/run.ts` (MODIFIED) — +1 import, +4 logic lines (dispatch block); SG1 cast fix applied +- `src/domain/errors.ts` (NEW then MODIFIED) — isRepoError predicate + REPO_ERROR_KINDS + exhaustiveness check (C7 corrects direction) diff --git a/openspec/changes/archive/verbum-vod/archive-report.md b/openspec/changes/archive/verbum-vod/archive-report.md new file mode 100644 index 0000000..f6243e3 --- /dev/null +++ b/openspec/changes/archive/verbum-vod/archive-report.md @@ -0,0 +1,167 @@ +# Archive Report: verbum-vod + +**Change**: `verbum-vod` +**Status**: SHIPPED +**PR**: #3 — https://github.com/ivanmaierg/verbum/pull/3 +**Squash merge SHA**: `d72d334` on `origin/main` +**Squash commit message**: `feat: verbum vod — verse-of-the-day subcommand (#3)` +**Branch**: `feat/verbum-vod` — deleted from origin after merge +**Final test state**: 59/59 pass + +--- + +## TL;DR + +**verbum-vod** ships a deterministic-per-day verse subcommand via `verbum vod`, selecting one of 365 curated verses using day-of-year modulo pooling. Implementation: 7 original commits (C1–C4 feature + C5–C7 followup hardening). Merged to main as single squash commit. Verdict: **PASS** — all findings resolved before merge. + +--- + +## What Shipped + +### Features +- **New subcommand**: `verbum vod` dispatched from `src/cli/run.ts` (positional guard before reference parsing) +- **Verse picker**: `pickVerseForDate(date: Date, pool: readonly PoolEntry[]): PoolEntry` — pure, size-agnostic algorithm using `dayOfYear(date) % pool.length` +- **365-entry pool**: `VERSE_POOL` (CLI layer POJO) with devotional/comfort canon distribution: 80 Psalms, 50 Proverbs, 40 John, 35 Romans, 25 Philippians, 20 Isaiah, 15 Matthew, 12 Ephesians, 10 Hebrews, 10 Luke, 8 1 John, 8 1 Peter, 7 James, 6 Colossians, 5 each (Galatians, Genesis, Deuteronomy, Mark), 4 each (2 Timothy, Acts), 3 each (Joshua, 1 Thessalonians, Jeremiah), 1 Lamentations = 365 +- **Exit-code contract**: 0 = success (verse on stdout), 1 = network/repo error, 2 = invalid pool entry (book parsing failure) +- **Handler**: `runVod(date: Date, repo?: BibleRepository)` in `src/cli/vod.ts` — mirrors `run()` error-handling pattern, live-fetches via existing `getPassage` +- **Type predicate**: `isRepoError(err: AppError): err is RepoError` in `src/domain/errors.ts` with corrected compile-time exhaustiveness guard against future `RepoError` drift + +### Files Added/Modified +| Path | Type | Status | Lines (logic vs data) | +|------|------|--------|----------------------| +| `src/application/verse-pool.ts` | NEW | stable | 30 (logic) | +| `src/application/verse-pool.test.ts` | NEW | stable | 150 (tests) | +| `src/cli/verse-pool-data.ts` | NEW | stable | 370 (data) | +| `src/cli/vod.ts` | NEW → MODIFIED (SG1) | stable | 40 (logic) | +| `tests/vod-smoke.test.ts` | NEW → MODIFIED (W1+SG1) | stable | 80 (tests) | +| `src/cli/run.ts` | MODIFIED | stable | 5 (logic) | +| `src/domain/errors.ts` | MODIFIED (SG1+C7) | stable | 15 (predicate + check) | + +**Logic surface**: ~145 lines (C1 picker ~60 + C3 handler ~80 + C4 dispatch ~5); **data surface**: ~370 lines (C2, mechanical POJO). Total: ~515 lines changed (logic + data + tests). + +--- + +## Decisions Made Along the Way + +### D1: Pool size = 365 verses (one per day, no repetition within a year) + +**Rationale** (revised from initial 30-verse proposal during interactive review): +- Original proposal anchored pool size to curation cost assuming hardcoded verse text. +- D2 commits to live fetch via existing `getPassage`, collapsing curation cost to reference lists (3-tuple POJOs, not text bodies). +- With cost neutralized, the right axis is UX: 30-verse pool repeats monthly (visibly broken for daily use); 365-verse pool feels deterministic per day. +- Picker is size-agnostic (`dayOfYear % pool.length`) — pool can shrink/grow later without code changes. No corner-painting. +- **Curation source**: devotional/comfort canon (Psalms-heavy + cornerstones + Proverbs daily). Light on genealogies, Levitical law, prophetic judgment. + +### D2: Live fetch via existing `getPassage` (accept divergence with welcome-content.ts) + +**Rationale**: +- Consistency with `verbum john 3:16` is dominant — same code path for fetching and rendering ensures identical output format and shared bug surface. +- Downside: `welcome-content.ts` hardcoded BSB text may diverge if the upstream API updates. Documented as v1 risk (SDD capture, not blocking). +- Pool MAY include John 3:16, Genesis 1:1 — no reason to exclude them. Future change (welcome consolidation) owns the drift-reconciliation if ever needed. + +### D3: Pool entries are USFM string tuples; `Reference` constructed at use time in `runVod` + +**Rationale** (R6 spirit): +- Alternatives: (A) `makeBookIdOrThrow` helper — parallel API defeating R1/R6 intent; (B) eager `buildPool(): Result` — boilerplate at every consumer for unreachable error. +- **Chosen**: Pool entries = `{ usfm: string; chapter: number; verse: number }`, `runVod` calls `makeBookId` then constructs `Reference`. Flows Result through same error path as `parseReference` (exit 2 on invalid book). +- Typo in pool surfaces as `unknown_book` on stderr first time that day's verse triggers — loud, located, recoverable by editing one line. No new abstraction. Respects R1 (no throw), R5 (errors are unions), R6 (single factory). + +--- + +## Findings Resolved Before Merge + +### SG1 (SUGGESTION → RESOLVED in C5) +- **Original**: `as RepoError` casts in `run.ts` and `vod.ts` violate R6 (no casts). +- **Resolution**: Added `isRepoError(err: AppError): err is RepoError` type predicate to `src/domain/errors.ts` using `REPO_ERROR_KINDS` exhaustiveness tuple. Removed casts; replaced with dual-branch narrowing (`if (isRepoError) { ... } else { ... }`). +- **Status**: Fully resolved and runtime-verified correct. + +### W1 (WARNING → RESOLVED in C6) +- **Original**: S5 scenario (network failure → exit 1) lacked direct test coverage. +- **Resolution**: Added third smoke test in `tests/vod-smoke.test.ts` using `failingRepo` stub that returns network error. Asserts exit 1, stderr contains "network failure", stdout empty. +- **Status**: Fully resolved. Test is load-bearing (removing `return 1` from vod.ts causes test failure). + +### W-REMAIN (WARNING → RESOLVED in C7, before merge) +- **Original** (revealed during re-verify of C5+C6): Exhaustiveness check in `src/domain/errors.ts` line 39 had reversed conditional direction. +- **Defective code (C5)**: `const _exhaustiveCheck: RepoErrorKind extends RepoError["kind"] ? true : never = true;` +- **Problem**: Checked if REPO_ERROR_KINDS *was a subset of* RepoError["kind"], not the inverse. If RepoError gained a new variant but REPO_ERROR_KINDS wasn't updated, the check would silently pass while `isRepoError` misclassified the new variant as a `ParseError`. +- **Fix in C7 (`fe4bd31`)**: Flipped to `RepoError["kind"] extends RepoErrorKind ? true : never`. Now if RepoError gains a 7th variant, the 7-member superset cannot extend the 6-member subset → result is `never` → `true = never` is a compile error → drift caught at build time. +- **Status**: Fully resolved before merge. The guard now actually does what its comment claims. + +### W2 (WARNING → SKIPPED, documented intentional) +- **Original**: S4 scenario (invalid pool entry → exit 2) lacked test coverage. +- **Analysis**: I2 invariant test walks all 365 pool entries through `makeBookId` and fails the suite if any reject. Exit-2 path is structurally unreachable at runtime given this guard. +- **Skip justification**: Adding a test would require injectable pool API just to reach an unreachable branch. Future risk: if I2 weakens (dynamic pool, injectable VERSE_POOL), exit-2 becomes reachable but untested. Documented in apply-progress; flagged for revisit if I2 changes. +- **Status**: Intentional skip. No footgun today. + +### SG2 (SUGGESTION → not addressed, low priority) +- **Finding**: `dayOfYear` comment in `src/application/verse-pool.ts` could be clearer about "local calendar fields, UTC epoch arithmetic" pattern. +- **Status**: Functionally correct, comment-only nit. Not worth a commit on its own; will be picked up by any future edit to that function. + +--- + +## Deviations from Design + +### C2: Pool entry count correction +- **Design stated**: 80 Psalms entries. +- **Actual count**: 81 Psalms listed (off-by-one error in the design spec). +- **Resolution**: Removed last Psalm (`PSA 145:18` at index 81) to land exactly 365 total entries. `length === 365` test caught this at `bun test` time (C2). +- **All other entries**: Faithfully transcribed from design. + +### C5–C7: Followup commits beyond the original 4-commit plan +- **C5**: Refactor cast → predicate (SG1 suggestion resolved). +- **C6**: Add exit-1 test (W1 warning resolved). +- **C7**: Fix exhaustiveness check direction (W-REMAIN warning found during re-verify, fixed before archive). +- **Impact**: +3 commits beyond original 4-commit plan. Squash merge coalesced all 7 into single `d72d334` on main. + +--- + +## Roadmap Items Queued During This Change + +Captured in engram for future SDD cycles (in priority order): + +1. **#184** (`visual-identity-v1`): Direction LOCKED via opencode TUI reference (user-supplied 2026-05-10). Black background, three-tier text hierarchy (bright/mid/dim), ONE accent color reserved for actionable affordances, two-tone wordmark, whitespace-first composition. **Open taste call deferred to user**: accent color (warm amber / desaturated red / opencode-style blue / other). Recommended sequence: this lands BEFORE `cli-loading-state` so the loading spinner inherits theme tokens. +2. **#183** (`cli-loading-state`): TTY-aware loading indicator for blocking CLI commands (both `verbum ` and `verbum vod`). Depends on #184 for theme tokens. + +No outstanding follow-up tasks for `verbum-vod` itself — all findings resolved before merge. + +--- + +## Traceability — Artifact Observation IDs + +All SDD change artifacts for `verbum-vod` archived in engram: + +| Artifact | Observation ID | Topic Key | +|----------|----------------|----------- +| Exploration | 175 | `sdd/verbum-vod/explore` | +| Proposal | 176 | `sdd/verbum-vod/proposal` | +| Spec | 178 | `sdd/verbum-vod/spec` | +| Design | 179 | `sdd/verbum-vod/design` | +| Tasks | 181 | `sdd/verbum-vod/tasks` | +| Apply Progress | 186 | `sdd/verbum-vod/apply-progress` | +| Verify Report | 188 | `sdd/verbum-vod/verify-report` | +| **Archive Report** | **195 (this)** | `sdd/verbum-vod/archive-report` | + +--- + +## Test Coverage Summary + +**Final state**: 59/59 tests pass, 831 expect() calls across 8 test files + +--- + +## Final Verdict + +**PASS** — all findings resolved before merge. + +The change is production-ready and shipped. SG1, W1, and W-REMAIN were all closed before the squash-merge. W2 skip is documented and structurally justified (I2 invariant makes the branch unreachable). SG2 is a comment-only nit deferred to opportunistic cleanup. No outstanding follow-up tasks for verbum-vod. + +--- + +## Archive Closure + +**Change**: `verbum-vod` +**Merged**: d72d334 on origin/main +**Shipped in**: PR #3 (merged 2026-05-10) +**Test state**: 59/59 ✓ +**Verdict**: SHIPPED, all findings resolved ✓ +**Next phase**: None for this change. Queued future work in engram: #184 `visual-identity-v1` (direction locked, accent color is the open taste call), then #183 `cli-loading-state` (depends on #184 theme tokens). diff --git a/openspec/changes/archive/verbum-vod/design.md b/openspec/changes/archive/verbum-vod/design.md new file mode 100644 index 0000000..b25639f --- /dev/null +++ b/openspec/changes/archive/verbum-vod/design.md @@ -0,0 +1,302 @@ +# Design: verbum-vod + +## TL;DR + +`verbum vod` ships as a tiny CLI dispatch (one `if` in `run.ts`) plus a pure picker (`pickVerseForDate`) plus a 365-entry POJO pool (`VERSE_POOL`) plus a `runVod(date)` handler that mirrors `run()`'s exit-code contract. No new dependencies, no new ports, no new domain types. `dayOfYear` is a private helper in `verse-pool.ts` using UTC midnight arithmetic (DST-safe). Pool entries are `{ usfm, chapter, verse }` tuples; `Reference` is built at use time so `makeBookId` errors flow through normal CLI error paths (R6 spirit). Devotional/comfort canon distribution: 80 Psalms / 50 Proverbs / 40 John / 35 Romans / 25 Philippians / 20 Isaiah / 15 Matthew / 15 (1 John+James) / 85 spread across NT epistles + Gospels + wisdom. + +## Layer Audit Summary + +| File | Layer | Imports allowed from | Verdict | +|------|-------|----------------------|---------| +| `src/application/verse-pool.ts` | application | domain only | OK — pure, no IO | +| `src/application/verse-pool.test.ts` | application | application + domain | OK | +| `src/cli/verse-pool-data.ts` | cli (config-data) | none (POJO export only) | OK — data, not logic | +| `src/cli/vod.ts` | cli | application + domain + cli/render + api | OK — same layer mix as `run.ts` | +| `src/cli/run.ts` (modified) | cli | unchanged | OK — adds 3 lines, preserves existing path | +| `tests/vod-smoke.test.ts` | test | application + domain + cli (stub repo) | OK — mirrors `tests/smoke.test.ts` | + +No inward-arrow violations. Application never imports CLI. + +## File-by-File Design + +### `src/application/verse-pool.ts` (NEW, pure) + +```ts +// Why: the picker is application-layer because it operates on POJO pool entries +// — it does NOT take a BibleRepository, does NOT touch IO. Pool data lives in +// the CLI layer (it is config, not a domain concept), but the picking algorithm +// is reusable and testable in isolation here. + +export type PoolEntry = { + readonly usfm: string; + readonly chapter: number; + readonly verse: number; +}; + +// pickVerseForDate — pure. Same date → same entry. Size-agnostic via modulo. +// Determinism formula: dayOfYear(date) % pool.length. +export function pickVerseForDate( + date: Date, + pool: readonly PoolEntry[], +): PoolEntry { + const idx = dayOfYear(date) % pool.length; + // pool.length is 365 by invariant I1; pool[idx] is always defined. + // Non-null assertion is local and justified — the alternative (Result) would + // pollute every caller for a branch that cannot fire. + return pool[idx]!; +} + +// dayOfYear — 1-based day of the calendar year using LOCAL date components, +// computed via UTC arithmetic to avoid DST hour-shift jumps causing day skips. +// Jan 1 → 1, Dec 31 (non-leap) → 365, Dec 31 (leap) → 366. +export function dayOfYear(date: Date): number { + const startUtc = Date.UTC(date.getFullYear(), 0, 1); + const todayUtc = Date.UTC( + date.getFullYear(), + date.getMonth(), + date.getDate(), + ); + const MS_PER_DAY = 86_400_000; + return Math.floor((todayUtc - startUtc) / MS_PER_DAY) + 1; +} +``` + +Notes: +- `PoolEntry` is exported so `verse-pool-data.ts` can type its const without re-declaring the shape (single source of truth for the tuple shape). +- `dayOfYear` is exported (not just internal) so the test suite can assert it explicitly for known dates. +- No `Result` here — picker is total: pool is non-empty by invariant I1, modulo is always in range. Adding `Result` would force every CLI consumer to handle an impossible error branch (R1 says "domain functions never throw", not "every function returns Result regardless of fallibility"). + +### `src/application/verse-pool.test.ts` (NEW) + +Mirrors the style of `src/domain/book-id.test.ts`. Three sections: + +```ts +import { describe, it, expect } from "bun:test"; +import { pickVerseForDate, dayOfYear } from "@/application/verse-pool"; +import { VERSE_POOL } from "@/cli/verse-pool-data"; +import { makeBookId } from "@/domain/book-id"; + +describe("dayOfYear", () => { + it("Jan 1 → 1", () => expect(dayOfYear(new Date(2025, 0, 1))).toBe(1)); + it("Mar 15 (non-leap) → 74", () => expect(dayOfYear(new Date(2025, 2, 15))).toBe(74)); + it("Jul 4 (non-leap) → 185", () => expect(dayOfYear(new Date(2025, 6, 4))).toBe(185)); + it("Dec 31 (non-leap) → 365", () => expect(dayOfYear(new Date(2025, 11, 31))).toBe(365)); + it("Feb 29 (leap) → 60", () => expect(dayOfYear(new Date(2024, 1, 29))).toBe(60)); + it("Dec 31 (leap) → 366", () => expect(dayOfYear(new Date(2024, 11, 31))).toBe(366)); +}); + +describe("pickVerseForDate — determinism", () => { + it("same date twice → same entry (referentially equal)", () => { + const a = pickVerseForDate(new Date(2025, 5, 15), VERSE_POOL); + const b = pickVerseForDate(new Date(2025, 5, 15), VERSE_POOL); + expect(a).toBe(b); + }); + + it("Jan 1 2025 → VERSE_POOL[1] (day 1 % 365)", () => { + expect(pickVerseForDate(new Date(2025, 0, 1), VERSE_POOL)).toBe(VERSE_POOL[1]); + }); + + it("Mar 15 2025 → VERSE_POOL[74]", () => { + expect(pickVerseForDate(new Date(2025, 2, 15), VERSE_POOL)).toBe(VERSE_POOL[74]); + }); + + it("Jul 4 2025 → VERSE_POOL[185]", () => { + expect(pickVerseForDate(new Date(2025, 6, 4), VERSE_POOL)).toBe(VERSE_POOL[185]); + }); + + it("Dec 31 2025 → VERSE_POOL[0] (365 % 365)", () => { + expect(pickVerseForDate(new Date(2025, 11, 31), VERSE_POOL)).toBe(VERSE_POOL[0]); + }); + + it("Dec 31 2024 (leap) → VERSE_POOL[1] (366 % 365)", () => { + expect(pickVerseForDate(new Date(2024, 11, 31), VERSE_POOL)).toBe(VERSE_POOL[1]); + }); + + it("day N+1 ≠ day N", () => { + const a = pickVerseForDate(new Date(2025, 5, 15), VERSE_POOL); + const b = pickVerseForDate(new Date(2025, 5, 16), VERSE_POOL); + expect(a).not.toBe(b); + }); +}); + +describe("VERSE_POOL — invariants (I1, I2)", () => { + it("I1: pool contains exactly 365 entries", () => { + expect(VERSE_POOL.length).toBe(365); + }); + + it("I2: every entry's usfm is accepted by makeBookId", () => { + for (const entry of VERSE_POOL) { + const result = makeBookId(entry.usfm); + if (!result.ok) { + throw new Error( + `Pool entry rejected: usfm="${entry.usfm}" ${entry.chapter}:${entry.verse}`, + ); + } + } + }); + + it("every entry has chapter >= 1 and verse >= 1", () => { + for (const entry of VERSE_POOL) { + expect(entry.chapter).toBeGreaterThanOrEqual(1); + expect(entry.verse).toBeGreaterThanOrEqual(1); + } + }); +}); +``` + +Pattern matches `book-id.test.ts` (no `bun:test` mock harness, just `describe`/`it`/`expect`). + +### `src/cli/verse-pool-data.ts` (NEW, data only) + +```ts +// Why: the daily verse pool lives in the CLI layer because it is configuration +// data, not a domain concept (the proposal explicitly rejected a VersePool port). +// Each entry is a USFM string + chapter + verse; the pool size is 365 (I1) so +// no verse repeats within a year. Curation style: devotional / comfort canon +// — Psalms-heavy with cornerstone NT verses (John 3:16, Rom 8:28, Phil 4:6-7, +// Jer 29:11, Pro 3:5-6, Isa 40:31, etc.). Light on genealogies, Levitical law, +// prophetic oracles, judgment passages. +// +// Maintenance: any edit MUST keep length === 365. The "every entry has a valid +// USFM" test in src/application/verse-pool.test.ts catches typos at `bun test` +// time. To rebalance, edit entries in place — the picker is size-agnostic but +// the test fails if length drifts from 365. + +import type { PoolEntry } from "@/application/verse-pool"; + +export const VERSE_POOL: readonly PoolEntry[] = [ + // ... 365 entries (full list provided in apply-progress artifact) +]; +``` + +### `src/cli/vod.ts` (NEW) + +```ts +// Why: runVod owns the same exit-code contract as run() but for the vod +// subcommand — kept separate so run.ts stays a thin dispatcher. +// Mirrors run.ts's error-handling pattern exactly: +// 0 → verse on stdout +// 1 → repo error (network, etc.) on stderr +// 2 → makeBookId rejected pool entry on stderr (pool typo) + +import { pickVerseForDate } from "@/application/verse-pool"; +import { VERSE_POOL } from "@/cli/verse-pool-data"; +import { makeBookId } from "@/domain/book-id"; +import { getPassage } from "@/application/get-passage"; +import { + renderParseError, + renderRepoError, + renderPassage, +} from "@/cli/render"; +import { createHelloAoBibleRepository } from "@/api/hello-ao-bible-repository"; +import type { BibleRepository } from "@/application/ports/bible-repository"; +import type { Reference } from "@/domain/reference"; +import type { RepoError } from "@/domain/errors"; + +// Optional repo injection enables the smoke test to pass a fixture-backed stub +// without spawning a process or hitting the network — same pattern as run(). +export async function runVod( + date: Date, + repo: BibleRepository = createHelloAoBibleRepository(), +): Promise { + const entry = pickVerseForDate(date, VERSE_POOL); + + // Construct BookId via the single factory (R6). A typo in pool data lands here. + const bookResult = makeBookId(entry.usfm); + if (!bookResult.ok) { + process.stderr.write(renderParseError(bookResult.error) + "\n"); + return 2; + } + + // Build Reference — single-verse range (start === end), v1 shape. + const ref: Reference = { + book: bookResult.value, + chapter: entry.chapter, + verses: { start: entry.verse, end: entry.verse }, + }; + + const passageResult = await getPassage(repo, ref); + if (!passageResult.ok) { + // AppError = ParseError | RepoError; pool already passed makeBookId, so + // the only ParseError that getPassage can surface is verse_not_found via + // RepoError union — treat the remaining branch as RepoError. + const err = passageResult.error as RepoError; + process.stderr.write(renderRepoError(err) + "\n"); + return 1; + } + + process.stdout.write(renderPassage(passageResult.value) + "\n"); + return 0; +} +``` + +### `src/cli/run.ts` (MODIFIED) — exact diff + +Current lines 16-29 (BEFORE): + +```ts +export async function run(argv: string[]): Promise { + const { positionals } = parseArgs({ + args: argv, + allowPositionals: true, + strict: false, + }); + + const input = positionals.join(" ").trim(); + + const refResult = parseReference(input); + if (!refResult.ok) { + process.stderr.write(renderParseError(refResult.error) + "\n"); + return 2; + } +``` + +AFTER: + +```ts +export async function run(argv: string[]): Promise { + const { positionals } = parseArgs({ + args: argv, + allowPositionals: true, + strict: false, + }); + + // Subcommand dispatch (I6, I7): check before parseReference. Any positional + // that is NOT a recognised subcommand falls through unchanged. + if (positionals[0] === "vod") { + return await runVod(new Date()); + } + + const input = positionals.join(" ").trim(); + + const refResult = parseReference(input); + if (!refResult.ok) { + process.stderr.write(renderParseError(refResult.error) + "\n"); + return 2; + } +``` + +Plus one new import at the top: + +```ts +import { runVod } from "@/cli/vod"; +``` + +Net diff: 1 import line + 4 logic lines (3 if you collapse the comment). Existing `verbum john 3:16` path is structurally untouched — invariant I8 holds. + +### `tests/vod-smoke.test.ts` (NEW) + +Mirrors `tests/smoke.test.ts` exactly: builds a fixture-backed `BibleRepository` stub (no real HTTP, no subprocess) and asserts the verse text comes back through the full `runVod` chain. Captures stdout via a write spy so we can assert what reached the user. + +## Commit sequence (work-unit-commits) + +| # | Commit | Files | Lines (est) | Why this slice is autonomous | +|---|--------|-------|-------------|-------------------------------| +| C1 | `feat(application): add pickVerseForDate picker + tests` | `src/application/verse-pool.ts`, `src/application/verse-pool.test.ts` (without the `VERSE_POOL` invariant tests — those land in C2) | ~60 | Pure function + tests. `bun test` green. No CLI changes, no pool dependency yet. | +| C2 | `feat(cli): add 365-entry verse pool data + invariant tests` | `src/cli/verse-pool-data.ts`, append invariant tests to `src/application/verse-pool.test.ts` | ~370 + 30 | Mechanical data file. The "every entry valid" test runs against the new data. `bun test` green. No runtime wire-up yet. | +| C3 | `feat(cli): add runVod handler + smoke test` | `src/cli/vod.ts`, `tests/vod-smoke.test.ts` | ~80 | Wires picker + pool + repo. Smoke test exercises the full chain via stub repo. `bun test` green. Not yet routed from `run.ts` — handler is dead code in production until C4. | +| C4 | `feat(cli): dispatch vod subcommand from run()` | `src/cli/run.ts` | ~5 | Smallest possible diff to existing code. Activates the feature. Existing `verbum john 3:16` smoke continues to pass (I8). `bun test` green. | + +## Open Design Questions + +None. diff --git a/openspec/changes/archive/verbum-vod/explore.md b/openspec/changes/archive/verbum-vod/explore.md new file mode 100644 index 0000000..db13da4 --- /dev/null +++ b/openspec/changes/archive/verbum-vod/explore.md @@ -0,0 +1,162 @@ +#175 [architecture] sdd/verbum-vod/explore +## Exploration: verbum-vod (Verse of the Day subcommand) + +### TL;DR — Riskiest Unknown + +The riskiest unknown is **subcommand routing collision**: `run()` in `src/cli/run.ts` today treats ALL argv as a reference string (`positionals.join(" ")`). Adding `verbum vod` means "vod" would be interpreted as a book alias — and it would currently fall through to `unknown_book` error. The disambiguation must happen BEFORE `parseReference` is called, and the boundary between subcommand dispatch and reference parsing must be designed carefully so neither breaks the other. + +--- + +### Current State + +**Entry point**: `src/index.tsx` +- `argv.length === 0` → TUI (`tuiDriver()`) +- any args → `run(argv)` in `src/cli/run.ts` + +**`src/cli/run.ts`** — `run(argv: string[]): Promise` +- Uses `parseArgs` (Node `util`) to collect `positionals` +- Joins all positionals into one string: `positionals.join(" ")` +- Calls `parseReference(input)` — single path, no subcommand concept at all +- Exit codes: 0 (ok), 1 (repo error), 2 (parse error) + +**`src/application/get-passage.ts`** — `getPassage(repo, ref)` +- Fetches the whole chapter via `BibleRepository.getChapter` +- Slices by verse range +- Returns `Result` + +**`src/cli/render.ts`** +- `renderPassage(passage)`: joins `verse.text` with `\n` +- Pure string formatters for error types + +**Output format**: bare verse text on stdout, no labels, no reference prefix. Single line for single-verse references. + +**Domain types**: `Reference`, `BookId` (branded), `TranslationId` (branded), `Passage`, `Chapter`, `Verse`. All are plain TS types — no Zod in domain (R4). + +**Test runner**: Bun's built-in (`bun test`). Test files co-located in `src/` or `tests/`. Pattern: `bun:test` imports, stub repos as inline objects satisfying `BibleRepository` interface. No golden files. No HTTP in tests (fixtures only). + +**Existing tests**: +- `src/application/get-passage.test.ts` — unit tests for `getPassage` with inline stubs +- `src/api/hello-ao-bible-repository.test.ts` — likely adapter tests (not read, but present) +- `src/domain/book-id.test.ts`, `reference.test.ts` — domain unit tests +- `src/tui/welcome/welcome-reducer.test.ts` — reducer test +- `tests/smoke.test.ts` — end-to-end wiring: `parseReference → getPassage → renderPassage` with fixture-backed stub repo + +--- + +### Affected Areas + +- `src/index.tsx` — NO CHANGE needed; already delegates to `run(argv)` for any non-empty args +- `src/cli/run.ts` — PRIMARY CHANGE: add subcommand dispatch before the reference parsing path +- `src/cli/vod.ts` (NEW) — `vod` command handler, analogous to the reference path in `run.ts` +- `src/application/get-verse-of-the-day.ts` (NEW, OPTIONAL) — new use case, or just a thin wrapper +- `src/application/ports/verse-pool.ts` (NEW) — port for verse pool data (if port approach chosen) +- `src/cli/verse-pool.ts` (NEW) — hardcoded pool adapter (data lives here, not in domain) +- `src/domain/` — NO CHANGE. Verse pool is not a domain concept. +- `src/api/schemas.ts` — NO CHANGE. Pool is not fetched from network. + +--- + +### Approaches + +#### 1. Inline subcommand check in `run.ts` + hardcoded pool in CLI layer + +Add a check at the top of `run()`: if `positionals[0] === "vod"`, branch to a `runVod()` function defined in `src/cli/vod.ts`. Pool is a hardcoded TS array of `{ book, chapter, verse }` tuples. Day selection is a pure function `pickForDate(date: Date, pool): Reference`. + +- Pros: minimal new files, consistent with existing run.ts pattern, no new ports/adapters +- Cons: pool is not injectable (not testable via port substitution), but the picker function IS pure and testable +- Effort: Low + +#### 2. New port `VersePool` in application layer + +Define `VersePool` port in `src/application/ports/verse-pool.ts` returning `Promise>`. Wire a hardcoded adapter in `src/api/hardcoded-verse-pool.ts`. New use case `getVerseOfTheDay(pool, repo, date)` composes both. + +- Pros: full hexagonal correctness, swappable pool source later (JSON, remote) +- Cons: overkill for a hardcoded constant list; adds 3 new files for minimal value; the pool adapter doesn't actually need to be async +- Effort: Medium + +#### 3. Pure function pool with no port (application-layer helper, not a use case) + +Export `pickVerseForDate(date: Date, pool: readonly Reference[]): Reference` from `src/application/verse-pool.ts`. The pool constant (array of references) is defined in `src/cli/verse-pool-data.ts` (CLI layer data, not domain). `run.ts` wires it. No new port. No new use case. + +- Pros: pure function is fully testable, no new interfaces, no port pollution, pool data stays out of domain, forward-compatible with `--format` flag +- Cons: pool data lives in CLI layer (acceptable since it's a config/data concern, not domain) +- Effort: Low + +--- + +### Recommendation + +**Approach 3** (pure function + CLI-layer pool data) is the right call. + +Why: the verse pool is NOT a domain concept. It's configuration — a curated list someone picked. Putting a `VersePool` port in the application layer (Approach 2) violates the spirit of ports: ports exist for IO-dependent adapters, not constant data. A hardcoded TS array isn't an "external system" that needs abstracting now. + +The critical insight is that `pickVerseForDate` must be a pure function. That's the only testability requirement. It takes `(date, pool)` and returns deterministically based on `date.toDateString()` hash or `dayOfYear % pool.length`. The pool array is just a constant — it doesn't need injection. + +**Subcommand routing** belongs in `run.ts` as a first-check on `positionals[0]`. Pattern: +``` +if (positionals[0] === "vod") { + return runVod(new Date()); +} +// existing reference path follows unchanged +``` + +This keeps `run.ts` as the single dispatch point and avoids any changes to `src/index.tsx`. + +**Determinism approach**: `dayOfYear % pool.length` where `dayOfYear` is derived from the date. Pure, deterministic per day, trivially testable by passing a fixed Date. Avoids RNG entirely. + +**Output format**: match `renderPassage` exactly — bare verse text on stdout, newline-terminated, exit 0. No label prefix for v1 (consistent with `john 3:16` output). This means `vod` can reuse `renderPassage` directly. + +**Pool content**: 15–30 well-known verses as hardcoded `Reference` objects. First-cut: construct them via `makeBookId` + `makeTranslationId` (same factories the rest of the codebase uses — R6). No raw strings. + +**`--format` compatibility**: the `renderPassage` function is already the format boundary. When `--format text|json|markdown` lands, the `vod` path just passes its `Passage` to the same formatter. No pre-building needed — just don't hardcode the output format in `runVod`. + +--- + +### Subcommand Routing Design + +`run.ts` today: `positionals.join(" ")` → `parseReference`. Problem: `vod` as positionals[0] would produce `parseReference("vod")` → `unknown_book` error, exit 2. + +Fix: check `positionals[0]` before any parsing. Keep it explicit — no magic routing table for now (only one subcommand). A routing table / Command pattern makes sense when there are 3+ subcommands. + +```ts +if (positionals[0] === "vod") { + return runVod(new Date()); +} +``` + +This is non-breaking for all existing paths. `verbum john 3:16` → `positionals[0]` is "john", falls through to reference path unchanged. + +--- + +### Files That Will Be Touched (Estimate) + +NEW: +- `src/cli/vod.ts` — `runVod(date: Date): Promise`, wires pool + picker + repo + render +- `src/application/verse-pool.ts` — pure `pickVerseForDate(date, pool)` function +- `src/cli/verse-pool-data.ts` — hardcoded pool: array of 15–30 `Reference` objects +- `src/application/verse-pool.test.ts` — unit tests for `pickVerseForDate` +- `tests/vod-smoke.test.ts` — smoke: `runVod(fixedDate)` returns 0, stdout contains verse text + +MODIFIED: +- `src/cli/run.ts` — ~5 lines: add subcommand check at top + import `runVod` + +UNTOUCHED: +- `src/index.tsx`, `src/domain/*`, `src/api/*`, `src/tui/*`, `src/application/get-passage.ts` + +Total estimate: ~120 lines added, ~5 modified. Well within 400-line budget. Single PR. + +--- + +### Open Questions for Proposal Phase + +1. **Pool size**: 15 verses? 30? 365? Affects churn period before repeating. The proposal should pick a number and list the actual verses. +2. **Reference construction in pool data**: use `makeBookId` factory or allow raw strings in pool data file (since it's CLI-layer, not domain)? House rule R6 says "no `as BookId` casts" — must use the factory. Pool data file must call `makeBookId` and handle the Result (which should always be ok for known USFM codes). +3. **Pool data file layer**: `src/cli/verse-pool-data.ts` (CLI layer, since it's wired in CLI) or `src/application/verse-pool-data.ts`? Since `pickVerseForDate` lives in application and needs the pool as an argument, the pool data itself could live in CLI (passes to use case). This keeps application layer pure. +4. **Error surface**: what happens if `pickVerseForDate` returns a reference that `getPassage` can't fetch (API down)? Same as existing `run.ts`: render repo error to stderr, exit 1. No special handling needed. +5. **Welcome-screen pool reuse**: the `welcome-content.ts` already has hardcoded BSB text (GENESIS_1_1_TEXT, JOHN_3_16_TEXT). The `vod` pool is fetched live from the API. Clarify for proposal: should `vod` verses also be pre-cached as static text, or always fetched? Fetching aligns with `john 3:16` behavior (network call), static text would eliminate the network dependency but diverge from the existing pattern. + +--- + +### Ready for Proposal + +Yes. Architecture is clear. Riskiest unknown (subcommand routing collision) has a clean solution. Recommend proceeding to `sdd-propose`. diff --git a/openspec/changes/archive/verbum-vod/proposal.md b/openspec/changes/archive/verbum-vod/proposal.md new file mode 100644 index 0000000..d11f61c --- /dev/null +++ b/openspec/changes/archive/verbum-vod/proposal.md @@ -0,0 +1,163 @@ +# Proposal: verbum-vod (Verse of the Day subcommand) + +## TL;DR + +Add `verbum vod` subcommand that prints a deterministic-per-day verse from a curated 365-verse pool. Live fetch via existing `getPassage` (mirrors `verbum john 3:16`). Pool data lives in CLI layer as USFM string tuples; `Reference` construction happens at use time so `makeBookId`'s Result flows through normal CLI error handling (no R6 escape hatch). + +## Intent + +Give users a one-shot daily verse via `verbum vod` — same UX shape as `verbum john 3:16` (one-shot, exits 0, bare verse on stdout). Deterministic per calendar day so the same day always yields the same verse, no RNG, no persistence. This is the second user-visible CLI path and forces the codebase to grow a real subcommand boundary that future commands (`--help`, `random`, etc.) can plug into. + +## Scope + +### In Scope +- New `verbum vod` subcommand routed in `src/cli/run.ts` (single `if positionals[0] === "vod"` check) +- Pure `pickVerseForDate(date, pool)` function in `src/application/verse-pool.ts` using `dayOfYear % pool.length` +- 365-verse hardcoded pool in `src/cli/verse-pool-data.ts` as `readonly { usfm: string; chapter: number; verse: number }[]` +- New `runVod(date)` handler in `src/cli/vod.ts` that constructs `Reference` from pool entry, calls `getPassage`, renders via existing `renderPassage` +- Unit tests for `pickVerseForDate` (determinism, date boundaries, pool wrap-around) +- Smoke test `tests/vod-smoke.test.ts` proving `runVod(fixedDate)` returns 0 with verse text on stdout + +### Out of Scope +- Caching, offline mode, `--date YYYY-MM-DD` override flag +- `--format text|json|markdown` (defer until format work lands across all commands) +- Routing table / Command pattern abstraction (1 subcommand does not justify it; revisit at 3+) +- TUI integration (vod is CLI-only for v1) +- Welcome-screen pool reuse / consolidating `welcome-content.ts` static text with live-fetched pool verses +- Localization / non-English translations + +## First Reviewable Cut + +Single PR. ~120 lines of logic + ~370 lines of pool data (mechanical, scannable as data not logic). Ships: +- `src/cli/run.ts` subcommand check +- `src/cli/vod.ts`, `src/cli/verse-pool-data.ts` +- `src/application/verse-pool.ts` + test +- `tests/vod-smoke.test.ts` + +**Review workload note for tasks phase**: pool-data file inflates raw line count but is review-light (one-pass scan for USFM typos, validated mechanically by the "every pool entry parses" unit test). Logic surface for review is ~120 lines. Tasks should split pool data into its own commit so the logic commits stay focused. + +## Success Criterion + +Running `verbum vod` on any given calendar day twice (same day) prints the SAME verse text to stdout, exits 0, and the verse is one of the 365 curated references. Verifiable: + +``` +$ verbum vod +For God so loved the world that He gave His one and only Son... + +$ verbum vod # same day +For God so loved the world that He gave His one and only Son... +``` + +Plus: `bun test` passes, including the new `pickVerseForDate` unit tests covering 3+ specific dates with known expected indices, and the "every pool entry has a valid USFM" mechanical check. + +## Decisions + +### D1: Pool size = 365 verses (one per day, no repetition within a year) + +**Why** (revised after review feedback): + +The original 30-verse decision was anchored to a curation-cost argument that quietly assumed hardcoded verse text. **D2 commits to live fetch**, which collapses the curation cost: pool entries are 3-tuple POJOs (`{ usfm, chapter, verse }`), not bodies of text. Curating 365 entries is one afternoon of reference-listing — no proportional 12x effort vs 30. + +With curation cost out of the equation, the right tradeoff axis is **user experience**: +- 30 verses → noticeable repetition every month. A daily-verse feature that repeats monthly is visibly broken to anyone using it consistently. +- 365 verses → no repetition within a year. The verse genuinely feels "of the day." +- 30 makes sense if you only see the feature once or twice per quarter; 365 is the right size for daily use, which is the actual use case. + +The picker (`dayOfYear % pool.length`) is size-agnostic — pool can shrink or grow later without code changes. We are not painting ourselves into a corner with 365. + +**Curation source**: defer to `sdd-design`. Reasonable starting points: a published one-year plan (M'Cheyne, the Bible Project's daily reading), a curated "verse-a-day" devotional list, or a hand-picked balance across canon (Psalms-heavy + cornerstones + Proverbs daily). Designer picks one and justifies; if the user has a preference, they can override at design phase. + +**Cost flagged**: pool data file is ~370 lines. Tasks should isolate it in its own commit so the work-unit-commits skill keeps logic and data review surfaces separate. Total PR size (~490 lines incl. data) exceeds the strict 400-line review budget but the 370-line data block is reviewed as data (mechanical typo scan + automated USFM validity test), not as logic. If the review workload guard fires in the tasks phase, the recommended split is: PR1 = routing + picker + 30-seed pool; PR2 = pool expansion to 365. This is a fallback, not the default. + +### D2: Live fetch via `getPassage` (accept short-term divergence with welcome-content.ts) + +**Why**: Consistency with `verbum john 3:16` is the dominant constraint — both subcommands hit the same code path for fetching and rendering, so the output format is identical and bugs in one surface in the other. Hardcoding pool text would create THREE sources of truth (pool, welcome, API) instead of two (welcome, API). + +The downside is real but bounded: if the pool happens to include Genesis 1:1 or John 3:16, the welcome screen's hardcoded BSB text could drift from what `verbum vod` prints on that day if the upstream API ever updates the BSB rendering. **Mitigation**: explicitly document that `welcome-content.ts` is a snapshot for the welcome UI only and is NOT a source of truth for verse content. A future change can consolidate (either by removing welcome's hardcoded text in favor of a startup fetch, or by adding a "snapshot regeneration" script). This is OUT of scope for verbum-vod. + +The pool MAY include John 3:16 and Genesis 1:1 — there is no v1 reason to exclude them. + +### D3: Pool entries are USFM string tuples; `Reference` constructed at use time in `runVod` + +**Why R6-spirit, not just R6-letter**: R6 says "no `as BookId` casts" and "branded IDs via single factory." The proposed alternatives: + +- *Option A (`makeBookIdOrThrow` helper)* — introduces a parallel API to `makeBookId` that exists only to dodge the Result. Even if scoped to "static data only," it is a precedent for "Result is annoying, throw a helper next to it." That precedent is exactly what R1 and R6 exist to prevent. +- *Option B (eager `buildPool()` returning `Result`)* — handles a "theoretically impossible" error at module load. Adds boilerplate at every consumer for a branch that will never run, and the "impossibility" is brittle: any typo in pool data turns into a runtime crash with no clear ownership. +- *Option C (CHOSEN — pool entries are `{ usfm: string; chapter: number; verse: number }`, `runVod` calls `makeBookId` then constructs the `Reference`)* — flows the Result through the SAME error path that `parseReference` already uses (`exit 2` on invalid book). A typo in pool data surfaces as `unknown_book` on stderr the first time that day's verse triggers — loud, located, recoverable by editing one line. No new abstraction. No spirit-violation. The picker stays pure (operates on POJO tuples, not `Reference`). + +Option C is the only one that respects R1 (no throwing), R5 (errors are unions, not exceptions), AND R6 (single factory) without adding a parallel API. + +## Non-goals + +This proposal explicitly does NOT: +- Introduce `--format` flag (out of scope for ALL commands until format work lands) +- Rotate the welcome screen verse based on day-of-year (welcome is static; vod is the daily channel) +- Add the verse pool to the TUI (TUI continues to use `welcome-content.ts` constants only) +- Build a routing table, Command interface, or subcommand registry abstraction +- Introduce a `VersePool` port in the application layer (pool is config data, not an external system — exploration Approach 2 explicitly rejected) +- Persist "last shown verse" anywhere (determinism comes from the date, not from history) +- Add `--date` override (testability comes from `runVod(date: Date)` taking the date as an argument; no flag needed for v1) + +## Capabilities + +### New Capabilities +- `verse-of-the-day`: deterministic-per-day verse selection from a curated pool, surfaced as the `verbum vod` CLI subcommand. Includes pool data shape, picker function contract, subcommand routing, and CLI output behavior. + +### Modified Capabilities +- `cli-runner` (or equivalent — sdd-spec to confirm against existing spec names): the CLI entry now performs subcommand dispatch on `positionals[0]` before delegating to reference parsing. Backward compatible (any positional that is not `vod` falls through unchanged). + +## Approach + +Pure-function picker, USFM-string pool, single subcommand check in `run.ts`. Quoting the exploration's recommended Approach 3: + +1. `src/cli/run.ts`: at the top of `run()`, after `parseArgs`, branch on `positionals[0] === "vod"` → return `await runVod(new Date())`. All other paths unchanged. +2. `src/cli/verse-pool-data.ts`: `export const VERSE_POOL: readonly { usfm: string; chapter: number; verse: number }[] = [...]` — 365 hand-picked entries. +3. `src/application/verse-pool.ts`: `export function pickVerseForDate(date: Date, pool: readonly PoolEntry[]): PoolEntry`. Pure. Determinism via `dayOfYear(date) % pool.length`. +4. `src/cli/vod.ts`: `runVod(date: Date): Promise`. Picks entry → `makeBookId(entry.usfm)` → on error, render to stderr, exit 2. On ok → build `Reference` → call `getPassage` → on error, render to stderr, exit 1 → on ok, `console.log(renderPassage(passage))` → exit 0. Mirrors `run.ts`'s existing error handling exactly. + +## Affected Areas + +| Area | Impact | Description | +|------|--------|-------------| +| `src/cli/run.ts` | Modified | Add 3-line subcommand check at top of `run()` | +| `src/cli/vod.ts` | New | `runVod(date)` handler | +| `src/cli/verse-pool-data.ts` | New | 365-entry hardcoded pool | +| `src/application/verse-pool.ts` | New | Pure `pickVerseForDate` | +| `src/application/verse-pool.test.ts` | New | Unit tests for picker + every-entry-validates check | +| `tests/vod-smoke.test.ts` | New | End-to-end smoke for `runVod` | +| `src/index.tsx` | Untouched | Already delegates non-empty argv to `run()` | +| `src/domain/*` | Untouched | Pool is not a domain concept | +| `src/api/*` | Untouched | Reuses existing `BibleRepository` | +| `src/tui/*` | Untouched | TUI is out of scope | +| `src/cli/welcome-content.ts` | Untouched | Documented divergence risk, deferred | + +## Risks + +| Risk | Likelihood | Mitigation | +|------|------------|-----------| +| Pool entry typo → runtime `unknown_book` on the day that entry rotates in | Medium | Unit test that walks all 365 entries through `makeBookId` and asserts every one is `ok` (catches typos at `bun test` time, not in production) | +| 370-line pool data file inflates PR size beyond 400-line review budget | Medium | Isolate pool data in its own commit (work-unit-commits); reviewer treats as data not logic; if review workload guard fires in tasks phase, fall back to chained PRs (routing+seed pool, then expand) | +| `welcome-content.ts` BSB text drifts from API rendering for shared references (Gen 1:1, John 3:16) | Low | Documented in D2 as accepted v1 risk; not blocking; consolidation is a separate future change | +| Subcommand collision when adding a 2nd subcommand later (e.g. `verbum random` or `verbum help`) | Low | One `if` chain is fine for 1–2 subcommands; explicit "build routing table when reaching 3+ subcommands" debt note added to the proposal as a forward signal for future SDD changes | +| Network failure on `getPassage` ruins the daily-verse experience | Medium | Same failure mode as existing `verbum john 3:16` — render error to stderr, exit 1. No new behavior; offline-first is a separate, larger concern | +| Timezone handling: "today" varies across users | Low | Use system local time via `new Date()` for v1 (same as user's clock). UTC normalization deferred until users complain | + +## Rollback Plan + +Single-PR change. Rollback = revert the PR. No data migrations, no config flags, no DB state. The 5-line edit to `run.ts` is the only modification to existing code; reverting it removes the `vod` dispatch and the existing `verbum john 3:16` path is structurally untouched. New files (`vod.ts`, `verse-pool*.ts`) become dead code on revert and can be deleted in a follow-up cleanup commit if desired. + +## Dependencies + +- Existing `BibleRepository` adapter (helloao) — no changes, just consumed +- Existing `makeBookId`, `Reference` factory chain in `src/domain/` +- Existing `renderPassage` and CLI error-rendering helpers in `src/cli/` +- No new npm packages + +## Success Criteria + +- [ ] `verbum vod` exits 0 and prints non-empty verse text to stdout when network is available +- [ ] Two invocations of `verbum vod` on the same calendar day produce IDENTICAL output +- [ ] `bun test` passes including new `pickVerseForDate` tests AND a "every pool entry has a valid USFM book" test +- [ ] `tests/vod-smoke.test.ts` passes with a fixed `Date` showing the expected verse for that day +- [ ] `verbum john 3:16` continues to work unchanged (regression check via existing smoke test) +- [ ] Pool contains exactly 365 entries, each with a known-good USFM code, chapter, and verse diff --git a/openspec/changes/archive/verbum-vod/spec.md b/openspec/changes/archive/verbum-vod/spec.md new file mode 100644 index 0000000..8a4f78b --- /dev/null +++ b/openspec/changes/archive/verbum-vod/spec.md @@ -0,0 +1,158 @@ +# Spec: verbum-vod + +## TL;DR + +Two capabilities touched: **verse-of-the-day** (new) and **cli-runner** (modified, backward-compatible). Seven acceptance scenarios. No implementation details — this spec describes WHAT, not HOW. + +--- + +## Capability 1: verse-of-the-day (NEW) + +### Purpose + +`verbum vod` prints a deterministic, live-fetched verse to stdout and exits 0. Determinism is day-scoped: same calendar day → same verse. No RNG, no persistence. + +### Public Contract + +| Surface | Contract | +|---------|----------| +| CLI invocation | `verbum vod` | +| stdout | bare verse text (same format as `verbum john 3:16`) | +| exit 0 | verse printed successfully | +| exit 1 | `getPassage` returned `Result.err` — error on stderr | +| exit 2 | `makeBookId` rejected pool entry — error on stderr | + +**Pool entry shape** + +```ts +{ usfm: string; chapter: number; verse: number } +``` + +**Picker signature** (pure function, application layer) + +```ts +pickVerseForDate(date: Date, pool: readonly PoolEntry[]): PoolEntry +``` + +Selection formula: `dayOfYear(date) % pool.length` (1-based day-of-year, modulo pool size). + +### Invariants + +| # | Invariant | +|---|-----------| +| I1 | Pool MUST contain exactly 365 entries | +| I2 | Every pool entry MUST produce an `ok` Result from `makeBookId` — asserted by a test that walks all entries; build MUST fail if any entry is invalid | +| I3 | `pickVerseForDate` MUST be a pure function (no I/O, no side effects) | +| I4 | `runVod` MUST NOT throw — errors MUST be returned as `Result.err` or exit codes (R1) | +| I5 | `runVod` MUST call `makeBookId` via the single factory (no `as BookId` casts) (R6) | + +### Acceptance Scenarios + +#### Scenario: Same-day determinism + +- GIVEN the user runs `verbum vod` on calendar day D +- WHEN `verbum vod` is run a second time on the same calendar day D (same system clock date) +- THEN both invocations print identical verse text to stdout +- AND both exit 0 + +#### Scenario: Day-over-day variation + +- GIVEN the pool contains more than 1 entry +- WHEN `pickVerseForDate` is called with day N and then with day N+1 of the same year +- THEN the two returned entries are different pool objects (different indices) + +#### Scenario: Year wrap-around (non-leap year) + +- GIVEN the pool contains exactly 365 entries +- WHEN `pickVerseForDate` is called with Dec 31 (day 365, non-leap year) +- THEN the returned entry is at index `365 % 365 = 0` (first entry) +- AND WHEN `pickVerseForDate` is called with Jan 1 of the following year (day 1) +- THEN the returned entry is at index `1 % 365 = 1` (second entry) + +#### Scenario: Invalid pool entry — makeBookId rejects + +- GIVEN a pool entry whose `usfm` field is not a valid USFM book code +- WHEN `runVod` selects that entry and calls `makeBookId` +- THEN `runVod` prints an error message to stderr +- AND exits 2 +- AND does NOT throw (R1) + +#### Scenario: Network failure + +- GIVEN `makeBookId` succeeds for the selected entry +- WHEN `getPassage` returns a `Result.err` +- THEN `runVod` prints the error to stderr +- AND exits 1 +- AND does NOT throw (R1) + +#### Scenario: Pool integrity — test-time check + +- GIVEN the project's test suite runs via `bun test` +- WHEN `src/application/verse-pool.test.ts` executes +- THEN every entry in `VERSE_POOL` MUST produce an `ok` Result from `makeBookId` +- AND the test MUST fail (non-zero exit from `bun test`) if any entry is invalid + +--- + +## Capability 2: cli-runner (MODIFIED) + +### Purpose + +The CLI entry point (`src/cli/run.ts`) now performs subcommand dispatch before delegating to reference parsing. All existing behavior is preserved; the new branch is additive. + +### Public Contract + +| Positional[0] | Behavior | +|---------------|----------| +| `"vod"` | Dispatch to `runVod(new Date())`; return its exit code | +| anything else | Fall through to `parseReference` (unchanged) | +| absent | Fall through to `parseReference` (unchanged, existing error handling) | + +Exit codes for the `vod` branch align with existing codes: 0 ok, 1 repository/network error, 2 invalid book/parse error. + +### Invariants + +| # | Invariant | +|---|-----------| +| I6 | The `vod` branch MUST be checked BEFORE `parseReference` is called | +| I7 | All positionals that are NOT `"vod"` MUST reach `parseReference` unchanged | +| I8 | `verbum john 3:16` MUST behave identically before and after this change ships | + +### Acceptance Scenarios + +#### Scenario: vod subcommand dispatched + +- GIVEN the user runs `verbum vod` +- WHEN `run()` executes +- THEN control reaches `runVod(new Date())` without passing through `parseReference` +- AND the exit code is whatever `runVod` returns + +#### Scenario: Backward compatibility — existing reference parsing + +- GIVEN `verbum john 3:16` was working before this change +- WHEN `verbum john 3:16` runs after this change ships +- THEN the output and exit code are identical to the pre-change behavior +- AND the existing smoke test passes without modification + +--- + +## Out of Scope + +- Caching / offline mode +- `--date YYYY-MM-DD` override flag +- `--format text|json|markdown` (deferred until format work lands across all commands) +- Routing table / Command pattern abstraction (revisit at 3+ subcommands) +- TUI integration +- Welcome screen using pool verses (`welcome-content.ts` divergence is accepted v1 risk) +- Localization / non-English translations +- UTC normalization of "today" (system local time is sufficient for v1) + +--- + +## Spec-Level Risks + +| Risk | Status | +|------|--------| +| Timezone: `new Date()` uses system local time — "today" differs across users in different zones | Formally documented as a v1 non-guarantee. UTC normalization is out of scope. | +| Pool entries for references shared with `welcome-content.ts` (e.g. Gen 1:1, John 3:16) may produce different text than the hardcoded BSB snapshots if the upstream API changes | Accepted v1 risk per D2. Not a spec failure; a future change owns consolidation. | +| `dayOfYear % pool.length` behavior for leap years (366 days, 365-entry pool): day 366 wraps to index `366 % 365 = 1`, which is the same entry as Jan 2 of the same year | Spec permits this. The formula is intentionally size-agnostic. Noted so tests can assert it explicitly if desired. | diff --git a/openspec/changes/archive/verbum-vod/tasks.md b/openspec/changes/archive/verbum-vod/tasks.md new file mode 100644 index 0000000..1197bc5 --- /dev/null +++ b/openspec/changes/archive/verbum-vod/tasks.md @@ -0,0 +1,166 @@ +# Tasks: verbum-vod + +## TL;DR + +9 tasks across 4 commits. Logic surface ~145 lines (C1 ~60 + C3 ~80 + C4 ~5); data file ~370 lines (C2, mechanical). Strict TDD: false — tests bundled with code in same commit (work-unit-commits). + +--- + +## Review Workload Forecast + +| Field | Value | +|-------|-------| +| Estimated changed lines (logic) | ~145 (C1 + C3 + C4) | +| Estimated changed lines (data) | ~370 (C2, mechanical POJO) | +| Estimated total | ~515 | +| 400-line budget risk | Low — logic surface is 145 lines; data file is mechanical scan | +| Chained PRs recommended | No — reviewer treats C2 as data scan; logic under budget | +| Suggested split | Single PR (4 work-unit commits within it) | +| Delivery strategy | ask-on-risk | +| Chain strategy | N/A | + +Decision needed before apply: No +Chained PRs recommended: No +Chain strategy: pending +400-line budget risk: Low + +### Suggested Work Units + +| Unit | Goal | Likely PR | Notes | +|------|------|-----------|-------| +| C1–C4 | Full verbum-vod feature | 1 PR | 4 ordered commits, each bun test green | + +--- + +## Commit C1 — `feat(application): add pickVerseForDate picker + tests` + +Files: `src/application/verse-pool.ts`, `src/application/verse-pool.test.ts` (picker + dayOfYear tests only; pool invariant tests land in C2) + +### T-1: Create `src/application/verse-pool.ts` + +- [x] Export `PoolEntry` type: `{ readonly usfm: string; readonly chapter: number; readonly verse: number }` +- [x] Export `dayOfYear(date: Date): number` — 1-based, UTC arithmetic (DST-safe), Jan 1 → 1, Dec 31 non-leap → 365 +- [x] Export `pickVerseForDate(date: Date, pool: readonly PoolEntry[]): PoolEntry` — formula `dayOfYear(date) % pool.length`; non-null assertion on `pool[idx]!` is justified (pool is non-empty by I1) +- [x] No `Result` wrapper — picker is total (R1 applies to fallible functions; this branch cannot fire) +- [x] No imports from CLI or API layers (layer audit: application only) + +**Spec link**: I3 (pure function), R1, R6 spirit + +**Acceptance**: file compiles; no imports from outside application/domain + +### T-2: Create `src/application/verse-pool.test.ts` (picker + dayOfYear sections only) + +- [x] `describe("dayOfYear")` — 6 cases: Jan 1 → 1, Mar 15 non-leap → 74, Jul 4 non-leap → 185, Dec 31 non-leap → 365, Feb 29 leap → 60, Dec 31 leap → 366 +- [x] `describe("pickVerseForDate — determinism")` — 7 cases using STUB_POOL (Option B chosen): same-date referential equality, Jan 1 → STUB_POOL[1], Mar 15 → STUB_POOL[74], Jul 4 → STUB_POOL[185], Dec 31 2025 → STUB_POOL[0], Dec 31 2024 (leap) → STUB_POOL[1], day N ≠ day N+1 +- [x] T-2 resolution: Option B — C1 uses STUB_POOL (365 synthetic PSA entries). Real-pool determinism tests moved to C2 alongside verse-pool-data.ts. +- [x] Follows style of `src/domain/book-id.test.ts` (bun:test, describe/it/expect, no mock harness) + +**Spec link**: Scenario "Same-day determinism", "Day-over-day variation", "Year wrap-around" + +**Acceptance**: `bun test src/application/verse-pool.test.ts` passes (picker + dayOfYear blocks) ✓ + +**Commit message**: `feat(application): add pickVerseForDate picker + tests` — SHA: cde9a9d + +--- + +## Commit C2 — `feat(cli): add 365-entry verse pool data + invariant tests` + +Files: `src/cli/verse-pool-data.ts`, append invariant tests to `src/application/verse-pool.test.ts` + +### T-3: Create `src/cli/verse-pool-data.ts` + +- [x] Import `PoolEntry` from `@/application/verse-pool` +- [x] Export `VERSE_POOL: readonly PoolEntry[]` +- [x] Transcribe entries from design observation #179. NOTE: design listed 81 Psalms despite saying 80 — removed PSA 145:18 (last Psalm) to achieve exactly 365 entries. All other entries faithfully transcribed. +- [x] Source order per design: JHN 3:16 at index 0, then Psalms (80), Proverbs (50), John (40), Romans (35), Philippians (25), Isaiah (20), Matthew (15), 1 John (8), James (7), Hebrews (10), Ephesians (12), Luke (10), 1 Peter (8), Colossians (6), Galatians (5), Genesis (5), Deuteronomy (5), Mark (5), 2 Timothy (4), Acts (4), Joshua (3), 1 Thessalonians (3), Jeremiah (3), Lamentations (1, 3:22 only) +- [x] File comment explains pool location in CLI layer; notes length === 365 invariant +- [x] No logic — pure POJO export only + +**Spec link**: I1, I2 + +**Acceptance**: `VERSE_POOL.length === 365`; `bun test` green ✓ + +### T-4: Append invariant tests to `src/application/verse-pool.test.ts` + +- [x] Add `describe("VERSE_POOL — invariants (I1, I2)")` block with 3 tests: I1 length=365, I2 makeBookId walk, chapter/verse >= 1 +- [x] Add `describe("pickVerseForDate — determinism (real VERSE_POOL)")` block with 7 cases against actual VERSE_POOL +- [x] Import `makeBookId` from `@/domain/book-id` + +**Spec link**: I1, I2, Scenario "Pool integrity — test-time check" + +**Acceptance**: `bun test src/application/verse-pool.test.ts` all 3 invariant tests pass ✓ + +**Commit message**: `feat(cli): add 365-entry verse pool data + invariant tests` — SHA: 213f48e + +--- + +## Commit C3 — `feat(cli): add runVod handler + smoke test` + +Files: `src/cli/vod.ts`, `tests/vod-smoke.test.ts` + +### T-5: Create `src/cli/vod.ts` + +- [x] Export `async function runVod(date: Date, repo: BibleRepository = createHelloAoBibleRepository()): Promise` +- [x] Step 1: `pickVerseForDate(date, VERSE_POOL)` → entry +- [x] Step 2: `makeBookId(entry.usfm)` — if `!ok`: `process.stderr.write(renderParseError(...) + "\n")`, return 2 +- [x] Step 3: build `Reference`: `{ book: bookResult.value, chapter: entry.chapter, verses: { start: entry.verse, end: entry.verse } }` +- [x] Step 4: `await getPassage(repo, ref)` — if `!ok`: stderr + return 1 +- [x] Step 5: `process.stdout.write(renderPassage(passageResult.value) + "\n")`, return 0 +- [x] Never throws (R1, I4). Uses `makeBookId` not `as BookId` cast (R6, I5) +- [x] All imports correct per spec + +**Spec link**: I4, I5, Scenario "Network failure", Scenario "Invalid pool entry — makeBookId rejects" + +**Acceptance**: compiles; layer audit clean ✓ + +### T-6: Create `tests/vod-smoke.test.ts` + +- [x] Build `stubRepo: BibleRepository` — `getChapter` returns verses 1..50 with text `STUB {book} {chapter}:{i+1}` +- [x] `describe("smoke — verbum vod happy path")` — 2 tests: fixed Date Mar 15 2025 → day 74 → VERSE_POOL[74] with stdout assertion; determinism test (two calls same date → same stdout) +- [x] Pattern mirrors `tests/smoke.test.ts` (in-process stub, no bun spawn) +- [x] Import `makeTranslationId` from `@/domain/translations` for Chapter shape + +**Spec link**: Scenario "Same-day determinism", Scenario "Network failure" (exit path tested via stub) + +**Acceptance**: `bun test tests/vod-smoke.test.ts` passes; no HTTP calls ✓ + +**Commit message**: `feat(cli): add runVod handler + smoke test` — SHA: 0a69c94 + +--- + +## Commit C4 — `feat(cli): dispatch vod subcommand from run()` + +File: `src/cli/run.ts` (MODIFIED) + +### T-7: Modify `src/cli/run.ts` — add import + +- [x] Add `import { runVod } from "@/cli/vod";` to the existing imports block + +**Spec link**: I6, I7, I8 + +### T-8: Modify `src/cli/run.ts` — add dispatch block + +- [x] Added dispatch block before `const input = ...`: `if (positionals[0] === "vod") { return await runVod(new Date()); }` +- [x] Existing lines from `const input = ...` onward are UNTOUCHED (I8) +- [x] Net diff: 1 import + 4 lines + +**Spec link**: I6 (vod before parseReference), I7 (non-vod falls through), I8 (backward compat) + +**Acceptance**: `verbum john 3:16` smoke still passes; `bun test` fully green ✓ + +**Commit message**: `feat(cli): dispatch vod subcommand from run()` — SHA: 39fa954 + +--- + +## Dependency Order + +C1 → C2 → C3 → C4 (strict chain; each commit left `bun test` green) + +## Apply-Phase Risks (resolved) + +| Risk | Resolution | +|------|------------| +| C2 length miscounting | Caught by `length === 365` test. Design had 81 Psalms despite saying 80 — removed PSA 145:18 to get exactly 365. | +| T-2 VERSE_POOL before C2 exists | Chose Option B: STUB_POOL in C1, real-pool tests moved to C2. | +| stdout spy in T-6 | Used try/finally as designed. Works correctly. | +| `renderParseError` cast | makeBookId returns UnknownBookError which is a ParseError subtype — no cast needed. | diff --git a/openspec/changes/archive/verbum-vod/verify-report.md b/openspec/changes/archive/verbum-vod/verify-report.md new file mode 100644 index 0000000..245c8fa --- /dev/null +++ b/openspec/changes/archive/verbum-vod/verify-report.md @@ -0,0 +1,130 @@ +# Verify Report: verbum-vod (Re-verify) + +**Verdict**: PASS WITH WARNINGS +**Findings**: 0 CRITICAL / 1 WARNING / 1 SUGGESTION (down from 2W/2SG) + +--- + +## TL;DR + +Re-verify of `feat/verbum-vod` after followup commits C5 (SG1 refactor) and C6 (W1 test). + +- Full suite: 59 pass / 0 fail (was 58). +1 is exactly the W1 exit-1 test. +- SG1 fully resolved: `as RepoError` casts removed from `run.ts` and `vod.ts`; `isRepoError` type predicate added to `src/domain/errors.ts`; runtime predicate verified correct. +- W1 fully resolved: exit-1 path now has a direct test in `tests/vod-smoke.test.ts`. +- W2 remains skipped: documented as intentional — exit-2 path is structurally unreachable given I2. +- 1 WARNING remains: the exhaustiveness check in `errors.ts` has its conditional direction reversed. +- Previous SG2 (dayOfYear comment nit) remains low-priority; no action taken; still a SUGGESTION. + +--- + +## Re-verify Summary + +| Item | Previous status | Current status | +|------|----------------|-----------------| +| SG1 — `as RepoError` casts | SUGGESTION | RESOLVED (C5) | +| W1 — S5 network failure test | WARNING | RESOLVED (C6) | +| W2 — S4 exit-2 test | WARNING | SKIPPED (documented, intentional) | +| SG2 — dayOfYear comment nit | SUGGESTION | unchanged, still SUGGESTION | +| New: exhaustiveness check direction | — | WARNING (new finding) | + +--- + +## Spec Scenario Coverage Table + +| # | Scenario | Test file | Test name | Status | +|---|----------|-----------|-----------|--------| +| S1 | Same-day determinism | tests/vod-smoke.test.ts | "runVod is deterministic..." | COVERED | +| S2 | Day-over-day variation | src/application/verse-pool.test.ts | "day N+1 ≠ day N" | COVERED | +| S3 | Year wrap-around (non-leap) | src/application/verse-pool.test.ts | "Dec 31 2025 → VERSE_POOL[0]" | COVERED | +| S4 | Invalid pool entry — exit 2 | NONE | W2 intentional skip (I2 makes unreachable) | WARNING (skip) | +| S5 | Network failure — exit 1 | tests/vod-smoke.test.ts | "runVod returns 1 and writes network error" | COVERED ✓ | +| S6 | Pool integrity — test-time check | src/application/verse-pool.test.ts | "I2: every entry's usfm..." | COVERED | +| S7 | vod subcommand dispatched | run.ts + vod-smoke.test.ts | structural + smoke | COVERED | +| S8 | Backward compatibility | tests/smoke.test.ts | 3 existing tests | COVERED | + +--- + +## Findings + +### WARNING + +**W-REMAIN — exhaustiveness check direction in errors.ts is reversed** +- File: `src/domain/errors.ts` line 39 +- Code: `const _exhaustiveCheck: RepoErrorKind extends RepoError["kind"] ? true : never = true;` +- The intended guard is: "if a new RepoError variant is added but REPO_ERROR_KINDS is not updated, fail at compile time." +- The actual guard does the opposite: it fires if REPO_ERROR_KINDS contains a kind that is NOT in RepoError["kind"]. That is, it catches stale entries in REPO_ERROR_KINDS, not missing ones. +- If `RepoError` gains `{ kind: "timeout" }` but REPO_ERROR_KINDS still has only 6 entries: `RepoErrorKind` (6 members) extends `RepoError["kind"]` (7 members) = true. Check does NOT fire. isRepoError silently misclassifies "timeout" as ParseError. +- The correct check is: `RepoError["kind"] extends RepoErrorKind ? true : never` — a 7-member union extending a 6-member union = never → error. +- Runtime behavior of isRepoError is currently correct (all 6 variants match). Risk only materialises when a new RepoError variant is added. +- Not CRITICAL because no new RepoError variants are imminent and the predicate works today. But the guard is broken and gives false confidence. + +**W-S4-SKIP — exit-2 path remains untested (documented intentional skip)** +- File: tests/vod-smoke.test.ts +- No test exercises runVod with a bad pool entry → exit 2. I2 invariant makes this unreachable in production. Skip is documented in apply-progress. +- Future risk: if I2 is ever weakened (pool injectable, dynamic pool), the exit-2 path becomes reachable but untested. No footgun today. + +### SUGGESTION + +**SG2 — dayOfYear comment could be clearer (unchanged from previous verify)** +- File: `src/application/verse-pool.ts` +- "local calendar fields, UTC epoch arithmetic" worth clarifying. Non-blocking. + +--- + +## Evidence + +### Test run (HEAD: 3c8e673) +- Full suite: **59 pass / 0 fail / 831 expect() calls** across 8 files in ~28ms +- `tests/vod-smoke.test.ts`: **3 pass** (was 2 after C3; +1 is exactly the W1 exit-1 test) +- Test count delta: 58 → 59 (+1 = W1 test only). CONFIRMED. + +### TypeScript (bunx tsc --noEmit) +- **Pre-existing errors only** — confirmed by stashing all 6 verbum-vod commits and running tsc on main: identical error list (bun:test not found, process not found, Bun not found). These are project-wide tsconfig gaps predating this change. +- C5 and C6 introduce **zero new tsc errors**. + +### Commit inspection + +| # | SHA | Message | Format | Attribution | +|---|-----|---------|--------|-------------| +| C5 | 8207373 | `refactor(cli): replace AppError casts with kind-based narrowing` | Conventional ✓ | None ✓ | +| C6 | 3c8e673 | `test(vod): cover runVod exit-1 path with failing repo stub` | Conventional ✓ | None ✓ | + +C5: pure refactor (no new logic), 3 files, 42+/11−. Existing suite is regression net. No new logic without tests. PASS. +C6: test-only commit. 1 file (tests/vod-smoke.test.ts), 43 insertions. Central change is the test. PASS. + +### SG1 verification + +1. `src/domain/errors.ts`: `isRepoError` type predicate added. Runtime check via `REPO_ERROR_KINDS.includes(err.kind)`. Verified at runtime: network → true, unknown_book → false. CORRECT. +2. `src/cli/run.ts`: No `as RepoError` cast. Uses `isRepoError(err)` guard with both branches (renderRepoError / renderParseError). CORRECT. +3. `src/cli/vod.ts`: No `as RepoError` cast. Same dual-branch narrowing. CORRECT. +4. Exhaustiveness check present but direction reversed — see WARNING above. + +### W1 verification + +- Test in `tests/vod-smoke.test.ts` lines 43–76. +- Uses `failingRepo: BibleRepository` with `getChapter` returning `{ ok: false, error: { kind: "network", message: "simulated failure" } }`. +- Asserts: `exitCode === 1` ✓, `stderrCapture.toContain("network failure")` ✓ (renderRepoError produces "Error: network failure — simulated failure"), `stdoutCapture === ""` ✓. +- Spy cleanup via try/finally. CORRECT. +- Mental removal test: removing `return 1` from vod.ts would make runVod fall through and return 0 → `expect(exitCode).toBe(1)` fails. Test is load-bearing. CONFIRMED. + +### House rules audit (changes only) + +- R1: `isRepoError` is a pure predicate, no throws. PASS. +- R5: REPO_ERROR_KINDS uses discriminated kind field. PASS. +- R6: No new casts introduced (casts removed). PASS. + +--- + +## W2 Skip Justification Assessment + +- apply-progress documents: "exit-2 path (makeBookId failure on pool entry) is structurally unreachable given the I2 pool-integrity invariant." +- I2 test (`src/application/verse-pool.test.ts`) walks all 365 entries through `makeBookId` and fails the suite if any reject. So at any bun test run, a bad pool entry is caught before it could produce a runtime exit-2. +- Future risk (if I2 weakens): documented in apply-progress and confirmed in this report. +- Skip decision is technically justified and risk is low. + +--- + +## Recommendation + +**Proceed to archive.** The W-REMAIN finding (reversed exhaustiveness check) is a correctness gap in a defensive guard, not a runtime bug. The guard works in the direction it was inadvertently written (catches stale REPO_ERROR_KINDS entries, not missing ones), and `isRepoError` is functionally correct today. The risk is future-facing only. Recommend filing a follow-up task to flip the conditional direction before the next RepoError variant is added. diff --git a/openspec/changes/visual-identity-v1/apply-progress.md b/openspec/changes/archive/visual-identity-v1/apply-progress.md similarity index 100% rename from openspec/changes/visual-identity-v1/apply-progress.md rename to openspec/changes/archive/visual-identity-v1/apply-progress.md diff --git a/openspec/changes/archive/visual-identity-v1/archive-report.md b/openspec/changes/archive/visual-identity-v1/archive-report.md new file mode 100644 index 0000000..0db5e88 --- /dev/null +++ b/openspec/changes/archive/visual-identity-v1/archive-report.md @@ -0,0 +1,201 @@ +# Archive Report: visual-identity-v1 + +**Status**: SHIPPED — PR #4 squash merged to origin/main as commit e4cfd75 +**Merge date**: 2026-05-10 +**Squash commit message**: `feat: visual-identity-v1 — three-tier hierarchy + opencode-style accent (#4)` +**Branch**: `feat/visual-identity-v1` (deleted from origin after merge) +**Final test count**: 83/83 pass (10 test files, 855 expect() calls) + +--- + +## TL;DR + +Established verbum's visual identity by introducing a semantic token system (one accent hex + ANSI dim/reset), TTY/NO_COLOR/FORCE_COLOR detection, and applying tokens at three surfaces: CLI verse render, CLI error render, and the TUI welcome wordmark. Reconciled `docs/ui-sketches.md` with the locked accent direction. Zero new runtime deps. Shipped as a single PR under the 400-line review budget. All spec scenarios verified; all tests pass. + +--- + +## What Shipped + +### Files NEW (2) +- `src/presentation/colors.ts` — pure hex string constants (ACCENT_HEX = "#5BA0F2"), zero imports, R4 compliant +- `src/cli/ansi.ts` — ANSI helpers (accent/dim/muted/error/RESET) + isColorEnabled(stream) with TTY/NO_COLOR/FORCE_COLOR semantics (~40 LOC + ~90 LOC tests) + +### Files MODIFIED (6) +- `src/cli/ansi.test.ts` — NEW test file for ansi.ts (18 tests covering full 9-row truth table) +- `src/cli/render.ts` — error renderers wrapped in error() token, gated on stream.isTTY +- `src/cli/banner.ts` — regenerated: BANNER_DIM_PART + BANNER_ACCENT_PART (arrays of per-line strings) + BANNER_WIDTH constant +- `scripts/generate-banner.ts` — dual figlet runs ("Verb" + "um") with per-half parity guard and padEnd normalization +- `src/tui/welcome/welcome-screen.tsx` — two-tone wordmark render: per-line map of two adjacent \ siblings (DIM + accent color) +- `docs/ui-sketches.md` — removed "no hue, ever" rule; restored accent token + three-tier text hierarchy table + error token + +### Line Count Summary +| Category | Count | +|----------|-------| +| Logic (new + modified source) | ~135 lines | +| Tests (new + modified) | ~110 lines | +| Generated (banner.ts) | 14 lines | +| Docs | ~30 lines (net) | +| **Total** | **~180 lines** | + +--- + +## Decisions Made & Honored + +**D1 — Direction confirmation (LOCKED).** The locked accent direction (blue #5BA0F2, three-tier hierarchy) supersedes c2d4c36's "no hue, ever" rule. Most recent user decision wins. docs/ui-sketches.md updated in this change. + +**D2 — Three-tier scope in welcome-screen.tsx (NOT in scope for v1).** Current 2-tier (DIM chrome + default-fg verse) is correct. Three-tier applies when interactive affordances land. + +**D3 — Wordmark two-tone split point (4-2, "Verb" dim + "um" accent).** Smaller accent (33%) acts as punch-color. Implementation: figlet runs twice, generator emits two arrays of per-line strings. + +**D4 — Hollow figlet for dim half (OUT of scope).** Aspirational for future visual-identity iteration. + +**D5 — Token architecture (Option B).** `src/presentation/colors.ts` holds pure hex constants (no imports, no logic). `src/cli/ansi.ts` wraps with ANSI escapes + stream detection. TUI consumes hex directly via OpenTUI's `\`. Hexagon respected. + +**D6 — CLI ANSI implementation (hand-rolled, no new deps).** Continues the no-deps-by-default posture. Three helpers + one detector (~25 lines). Zero new runtime deps. + +**D7 — Truecolor only, no fallback.** Emit `\x1b[38;2;91;160;242m` for #5BA0F2. Modern terminals approximate gracefully. Bun users skew modern. + +**D8 — TTY/NO_COLOR/FORCE_COLOR semantics (npm-aligned).** Color enabled when stream.isTTY === true AND (NO_COLOR unset OR empty) UNLESS FORCE_COLOR set to non-empty (and NOT "0" or "false", per npm convention). Independent per-stream check. + +### Design Phase Refinements + +**Q1 resolved (design extended spec):** FORCE_COLOR="0" and FORCE_COLOR="false" disable color (npm convention). Spec's "any non-empty string enables" was incomplete; design correctly extended this. Implementation follows design. + +**Q2 resolved (OpenTUI DIM API):** TextAttributes.DIM is a numeric scalar, not an array. Uses `attributes={DIM}` where DIM is TextAttributes.DIM constant (already pattern in welcome-screen.tsx). + +**Q3 resolved (Banner export type):** Per-line arrays confirmed to be the correct shape (learned post-verify via gotcha #214 — see below). + +### Post-Verify Follow-Up Commit + +**C6: 2b23b03 — `fix(tui): render two-tone wordmark side-by-side via per-line spans`** + +Wordmark rendering bug discovered post-verify by user visual inspection: the two-tone wordmark was stacking "UM" below "VERB" instead of side-by-side. Root cause: OpenTUI cannot render two adjacent multi-line spans on the same row — newlines inside the first span force the second span to a new line. + +**Fix:** Changed banner generator exports from multi-line strings to per-line arrays (BANNER_DIM_LINES, BANNER_ACCENT_LINES). Updated welcome-screen.tsx to map each row to its own \ element with two adjacent \ siblings (one DIM, one with ACCENT_HEX). Each \ is one row; spans inside flow inline. + +83/83 tests still pass; user confirmed the fix visually. + +--- + +## Findings Resolved During Apply + +**W1 — FORCE_COLOR=0 semantics (spec wording gap, design extended spec).** +- Spec stated "FORCE_COLOR set to any non-empty string forces color on". This would incorrectly enable color for FORCE_COLOR=0. +- Design correctly extended spec with npm convention: "0" and "false" disable color. +- Implementation follows design (correct). Spec needs a post-merge amendment note (not a blocker for archive). + +**Gotcha #214 — OpenTUI multi-line span rendering limitation (discovered & fixed post-verify).** +- Two adjacent multi-line \ elements inside one \ cannot render side-by-side — newlines break rows. +- Pattern: use per-line arrays, map each row to its own \ with two inline \ siblings. +- Load-bearing for any future two-color text blocks (split wordmarks, colored ASCII art, colored gauges). + +--- + +## Findings Deferred (Future Scope) + +**SG1 — S6 (stream independence) has no direct unit test.** +Structurally sound (separate stream args), but no single test asserts both streams in one scenario. Recommend for v2 test-hardening pass. + +**SG2 — S12/S13 (E2E scenarios: piped output, FORCE_COLOR override) have no automated coverage.** +Only covered at unit composition level. Recommend E2E integration tests via Bun.spawn + stream capture for v2 test-hardening. + +--- + +## Roadmap Items Queued + +**#183 (cli-loading-state)** — was already queued; visual-identity-v1 unblocks it by establishing the semantic token layer. The spinner color and presentation primitives can now build on ACCENT_HEX + isColorEnabled infrastructure. + +--- + +## Process Notes + +### Spec-to-Verify Correctness +- Re-verify phase caught a real defect: W1 (FORCE_COLOR=0 semantics). The spec's statement was incomplete; design extended it correctly. +- The wordmark rendering bug was NOT caught by automated tests (TUI rendering is hard to unit-test). Caught by user visual inspection on the running TUI. +- **Lesson**: Spec/design contradictions should be resolved at write time. Verify can catch wording gaps; visual bugs require user inspection. + +### TDD Compliance (Strict TDD Mode Active) +- C2 (b3a9b9c): tests + code bundled, 18 ansi tests pass on commit +- C4 (9a34b28): tests + code bundled, 6 render tests pass on commit +- C1, C3, C5: no tests required (constants, generated file, docs) + +### Test Coverage +- All 18 spec scenarios (S1–S18) have covering tests or structural verification +- 9-row isColorEnabled truth table fully exercised (T1–T9) +- All 5 ANSI helpers tested with color enabled and disabled +- Error rendering tested across color states and both stream types +- No new runtime dependencies + +### Backward Compatibility +- Smoke tests pass (john 3:16 renders byte-for-byte identical when piped) +- renderPassage output is unchanged when NO_COLOR is active +- renderParseError/renderRepoError are byte-identical when piped + +--- + +## Risk Assessment + +**RETIRED: c2d4c36 vs locked-direction conflict.** The conflict was resolved (accent wins, ui-sketches.md updated). No runtime risk. + +**Riskiest unknown from exploration (#203):** fully resolved. + +--- + +## Success Criteria Met + +- [x] `verbum john 3:16` in a TTY renders verse text (no visible escapes in v1, infrastructure present for future) +- [x] `verbum nonexistent` in a TTY emits error in red; piped to file produces zero ANSI escapes +- [x] `NO_COLOR=1 verbum nonexistent` in a TTY produces zero ANSI escapes +- [x] `FORCE_COLOR=1 verbum nonexistent | cat` produces ANSI escapes despite the pipe +- [x] `verbum vod` displays the same way (vod consumes render.ts) +- [x] TUI welcome screen wordmark renders "Verb" dim and "um" accent blue (#5BA0F2) +- [x] `docs/ui-sketches.md` no longer contains "no hue, ever" rule; contains accent + three-tier hierarchy tables +- [x] No new runtime dependencies in `package.json` +- [x] All 83 existing + new tests pass (10 test files) + +--- + +## Artifacts & Evidence + +| Artifact | ID | Topic Key | Status | +|----------|----|-----------|----| +| Exploration | 203 | sdd/visual-identity-v1/explore | ✓ | +| Proposal | 206 | sdd/visual-identity-v1/proposal | ✓ | +| Spec | 207 | sdd/visual-identity-v1/spec | ✓ | +| Design | 208 | sdd/visual-identity-v1/design | ✓ | +| Tasks | 209 | sdd/visual-identity-v1/tasks | ✓ | +| Apply Progress | 210 | sdd/visual-identity-v1/apply-progress | ✓ | +| Verify Report | 211 | sdd/visual-identity-v1/verify-report | ✓ | +| Direction Lock | 184 | decision/future-feature-visual-identity-v1 | ✓ | +| Accent Color Lock | 200 | decision/visual-identity-v1-accent-color | ✓ | +| c2d4c36 Conflict | 204 | decision/visual-identity-v1-c2d4c36-conflict-resolution | ✓ | +| Multi-span Gotcha | 214 | discovery/opentui-multiline-span-rendering | ✓ | + +--- + +## Original Commits (Pre-Squash) + +7 commits on branch `feat/visual-identity-v1`: +1. 60d8103 — feat(presentation): add shared color constants module +2. b3a9b9c — feat(cli): add ANSI helpers with TTY/NO_COLOR detection +3. 203641f — feat(tui): split wordmark into dim/accent two-tone via banner generator +4. 9a34b28 — feat(cli): apply error/text tokens to passage and error rendering +5. 84e2450 — docs: reconcile ui-sketches with locked accent direction +6. 2b23b03 — fix(tui): render two-tone wordmark side-by-side via per-line spans (post-verify) +7. bc280e8 — chore(openspec): track SDD trail directory; add visual-identity-v1 artifacts + +**Squash merge**: all 7 collapsed to e4cfd75 on origin/main + +--- + +## Change Closed + +visual-identity-v1 is SHIPPED and ARCHIVED. No follow-up work required. The semantic token layer is in place, the two-tone wordmark is rendering correctly, docs are reconciled, and all tests pass. + +Next changes can build on this foundation (e.g., #183 cli-loading-state, future TUI affordances that consume ACCENT_HEX or the isColorEnabled infrastructure). + +--- + +**Archive report created**: 2026-05-10 +**By**: sdd-archive executor +**Mode**: hybrid (engram + openspec) diff --git a/openspec/changes/visual-identity-v1/design.md b/openspec/changes/archive/visual-identity-v1/design.md similarity index 100% rename from openspec/changes/visual-identity-v1/design.md rename to openspec/changes/archive/visual-identity-v1/design.md diff --git a/openspec/changes/visual-identity-v1/explore.md b/openspec/changes/archive/visual-identity-v1/explore.md similarity index 100% rename from openspec/changes/visual-identity-v1/explore.md rename to openspec/changes/archive/visual-identity-v1/explore.md diff --git a/openspec/changes/visual-identity-v1/proposal.md b/openspec/changes/archive/visual-identity-v1/proposal.md similarity index 100% rename from openspec/changes/visual-identity-v1/proposal.md rename to openspec/changes/archive/visual-identity-v1/proposal.md diff --git a/openspec/changes/visual-identity-v1/spec.md b/openspec/changes/archive/visual-identity-v1/spec.md similarity index 99% rename from openspec/changes/visual-identity-v1/spec.md rename to openspec/changes/archive/visual-identity-v1/spec.md index 878649f..8442431 100644 --- a/openspec/changes/visual-identity-v1/spec.md +++ b/openspec/changes/archive/visual-identity-v1/spec.md @@ -274,6 +274,6 @@ Mirrors proposal Non-goals exactly: |------|---------------------| | NO_COLOR behavior for empty-string value | Spec follows https://no-color.org strictly: only non-empty values disable color. If a future terminal ecosystem convention differs, this is the tie-breaker. | | FORCE_COLOR non-empty definition | Spec treats any non-empty string as "force on". Does not attempt to parse `"0"` as "force off" (that would be a separate convention — npm's `FORCE_COLOR=0` means "disable". This spec does not implement that nuance; treat `FORCE_COLOR` as a boolean on/off by presence/absence of non-empty value.) | -| `dim` escape closer | Using `\x1b[22m` (reset bold+dim only) vs `\x1b[0m` (full reset). Spec pins `dim` closer to `\x1b[22m` to avoid resetting other attributes. If nesting of attributes is needed in a future helper, this is the right choice. | +| `dim` escape closer | Using `\x1b[22m` (reset bold/dim only) vs `\x1b[0m` (full reset). Spec pins `dim` closer to `\x1b[22m` to avoid resetting other attributes. If nesting of attributes is needed in a future helper, this is the right choice. | | OpenTUI `attributes` API for DIM | Spec assumes OpenTUI accepts `attributes={[DIM]}` where `DIM` is a known constant. If the actual API differs, design phase must reconcile but the spec behavior (dim attribute on "Verb" span) is non-negotiable. | | Banner string vs string[] | Spec requires `BANNER_DIM_PART` and `BANNER_ACCENT_PART` to be `string` (multi-line, `\n`-joined). If OpenTUI requires `string[]` for multi-line spans, the design phase may adjust the export type; the parity invariant still applies. | diff --git a/openspec/changes/visual-identity-v1/tasks.md b/openspec/changes/archive/visual-identity-v1/tasks.md similarity index 100% rename from openspec/changes/visual-identity-v1/tasks.md rename to openspec/changes/archive/visual-identity-v1/tasks.md diff --git a/openspec/changes/visual-identity-v1/verify-report.md b/openspec/changes/archive/visual-identity-v1/verify-report.md similarity index 100% rename from openspec/changes/visual-identity-v1/verify-report.md rename to openspec/changes/archive/visual-identity-v1/verify-report.md