diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5aa7557..fe5b8f2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,3 +40,79 @@ jobs: - run: pnpm lint - run: pnpm test + + # The end-to-end journey suite, in its own job so it runs alongside verify + # rather than behind it. `pnpm test` stays the fast one and never waits on a + # browser. See e2e/README.md for what the journeys cover and why the server + # is faked at its edges rather than mocked at its HTTP boundary. + e2e-web: + runs-on: macos-14 + env: + # The web project never launches Electron, so the binary is dead weight + # here. The e2e-electron job below deliberately omits this. + ELECTRON_SKIP_BINARY_DOWNLOAD: 1 + steps: + - uses: actions/checkout@v4 + + - uses: pnpm/action-setup@v4 + + - uses: actions/setup-node@v4 + with: + node-version: 24 + cache: pnpm + + - run: pnpm install --frozen-lockfile + + - name: Cache Playwright browsers + uses: actions/cache@v4 + with: + path: ~/Library/Caches/ms-playwright + key: ${{ runner.os }}-playwright-${{ hashFiles('pnpm-lock.yaml') }} + restore-keys: ${{ runner.os }}-playwright- + + - run: pnpm exec playwright install chromium + + # No separate build step. e2e/global-setup.ts runs `vite build` itself, + # so there is exactly one code path and no way for a job to run the + # journeys against a bundle it did not build. + - run: pnpm e2e --project=web + + - uses: actions/upload-artifact@v4 + if: failure() + with: + name: playwright-report-web + path: playwright-report/ + retention-days: 7 + + # Isolated from e2e-web because it launches a real GUI application and needs + # the Electron binary, which is the one thing every other job skips + # downloading. One `--project` flag keeps it out of the web suite. + e2e-electron: + runs-on: macos-14 + steps: + - uses: actions/checkout@v4 + + - uses: pnpm/action-setup@v4 + + - uses: actions/setup-node@v4 + with: + node-version: 24 + cache: pnpm + + - name: Cache Electron binary + uses: actions/cache@v4 + with: + path: ~/Library/Caches/electron + key: ${{ runner.os }}-electron-${{ hashFiles('pnpm-lock.yaml') }} + restore-keys: ${{ runner.os }}-electron- + + - run: pnpm install --frozen-lockfile + + - run: pnpm e2e --project=electron + + - uses: actions/upload-artifact@v4 + if: failure() + with: + name: playwright-report-electron + path: playwright-report/ + retention-days: 7 diff --git a/.gitignore b/.gitignore index cf8be6e..0d7d5fe 100644 --- a/.gitignore +++ b/.gitignore @@ -7,4 +7,6 @@ books/ *.log .worktrees/ .claude/worktrees/ -.DS_Store \ No newline at end of file +.DS_Store +playwright-report/ +test-results/ \ No newline at end of file diff --git a/docs/plans/refactor/phase-6.md b/docs/plans/refactor/phase-6.md new file mode 100644 index 0000000..b37dba7 --- /dev/null +++ b/docs/plans/refactor/phase-6.md @@ -0,0 +1,135 @@ +# Phase 6 — Committed E2E Journey Suite on Fake Adapters + +> **PARALLEL-EXECUTION ADDENDUM (architect, owner-approved):** Phase 7 develops CONCURRENTLY with this phase, in the main checkout, while Phase 6 runs in its own worktree off post-P2 master. Phase 6 MERGES FIRST. Consequences: land S1 (walking skeleton) and push it as early as possible and message the architect when it is on the branch, Phase 7 wants it for local verification. Journey (h)'s per-error-class variants may be added by Phase 7's lead as its final integration task after this phase merges, so build journey (h) with a parameterizable failure-injection point. Do not touch server/ or shared/ source (Phase 7 owns them this window); if a journey needs a server change, report it, do not make it. + +## Objective + +Prove the whole app works end to end, deterministically, with zero live-provider traffic and zero API keys, by driving the real browser UI against the real Fastify server whose edges are fakes. Pure test addition. No production file changes except config lines and two devDependencies. The suite is the regression net Phase 7's durability work will lean on, and the thing a cold reader opens to see that the hexagon is not decoration. + +## 0. Reconciled during execution + +Facts found in the code that contradict what this plan was written against. The plan below is corrected in place, and this list records what moved and why, so the difference between plan and reality is never silent. + +1. **The Phase 2 precondition is clean on master.** `rg -l "model-client" server --glob '!server/adapters/**' --glob '!**/*.test.ts'` returns nothing at `e8a5dd1`. Phase 2 finished the job. No journey is blocked, so (a), (b), and (f) are all in scope. +2. **`server/services/key-store.ts` no longer exists.** The plan's harness section warns that it builds its vault at module load, which forces `TUTOR_DATA_DIR` to be set before the first server import. Phase 2 replaced it with `createFileKeyVault({ dataDir })`, constructed inside `createPorts` from a `getDataDir()` call that reads the environment every time. The env var is still set before the dynamic import, because the guarantee costs nothing and `server/test/setup-env.ts` documents the same reasoning, but the constraint is now belt and braces rather than load bearing. +3. **The config surface is four lines, not three.** `pnpm lint` globs `client/ server/ electron/ shared/` explicitly, so an ESLint boundary block for `e2e/` would never run without adding `e2e/` to that script. The four touches are the vitest `exclude`, the tsconfig `include`, the ESLint boundary block, and the lint script glob. The phase gate's real invariant, that `git diff --stat master -- client/ server/ shared/ electron/` shows nothing, is unaffected, and it is the invariant this phase holds to. +4. **The scripted adapter must satisfy `FakeTextGeneration`, not bare `TextGeneration`.** `describeTextGenerationContract` types its `makeSubject` as `() => FakeTextGeneration`, because two of the behaviours it pins are only observable through the fake's scripting and recording surface. So the scripted adapter keeps the FIFO `scriptStreamText` / `scriptGenerateObject` / `scriptToolConversation` methods and the `requests` record, and adds intent rules underneath them. A queued script wins, rules answer when the queue is empty, and an unmatched call throws. That ordering is what lets one object satisfy the contract suite and the journeys at once. +5. **The renamed data directory guard.** Per-test isolation is a fresh `buildServer()` plus a fresh books directory, and the fixture removes `${dataDir}/books` on teardown rather than the whole worker directory, because the worker directory also holds the key vault file the real adapter would write to if anything ever asked it to. +6. **The suite found a production bug, and two assertions are quarantined on it.** `server/routes/generation.ts`'s `pipeHubToSse` attaches `request.raw.on('close', ...)` and ends the reply from it. On Node 16 and later that event fires when the request stream finishes rather than when the client disconnects, so for a POST whose body Fastify already read it fires immediately, and `POST /api/books/:id/generate-next` answers 200 with an empty body. Generation still runs and the chapter still lands on disk, but the reader hangs in its generating state forever. Confirmed against the production `startServer` path over a real socket with no browser, no fakes, and no static plugin. It affects the three routes that share that helper, `generate-next`, `regenerate`, and `generation-stream`, and nothing else, because `openSseStream` attaches no close listener. Filed as issue 50 and NOT fixed here, because Phase 7 owns `server/` this window and this phase changes no production file. Journey (b) keeps its adaptive-loop proof green, since the server does run the generation and the scripted model still records the chapter-2 prompt, and quarantines only the on-screen render. Journey (h) proves the same claim through the creation wizard's chapter-1 path, which is unaffected, and quarantines only its reader-surface variant. Both quarantines are `test.fixme` citing the issue, so they flip green when the fix lands. +7. **`fastify.inject` structurally cannot catch that class of bug.** The identical request through inject returns the complete event stream, because light-my-request does not model the socket lifecycle. Every existing inject test passes over it. That is the clearest argument this phase can make for its own existence, so it is recorded here rather than only in the issue. + +## Precondition on Phase 2 (hard gate, verified before starting) + +The harness premise is that overriding `textGeneration` makes generation deterministic. Verified empty at `e8a5dd1`: + +``` +rg -l "model-client" server --glob '!server/adapters/**' --glob '!**/*.test.ts' +``` + +## Runner decision: `@playwright/test`, standalone, `e2e/` top level + +Recommended over extending Phase 3's hand-rolled CDP harness, and over vitest browser mode. + +- **Maturity.** Playwright is the boring industry default. Phase 3's CDP harness was correct for what it did, a one-off pixel measurement where a screenshot and a byte compare were the whole job. A committed journey suite needs auto-waiting locators, per-test isolation, traces, and reporting, and hand-rolling those is precisely the clever-over-simple trap the owner's principles reject. +- **Deterministic waits come free.** Web-first assertions such as `await expect(locator).toContainText(...)` retry to a deadline. That is the answer to SSE timing flake, and it is why no journey contains a sleep. +- **One tool covers Electron.** `_electron.launch()` gives journey (g) without a second harness. That single fact decides it, because the alternative is CDP-attaching to Electron by hand. +- **Vitest browser mode is rejected** as the newest, least battle-tested option, and mixing it into `pnpm test` would put a 90-second suite behind the 2.5-second unit run. `pnpm test` stays fast and E2E is `pnpm e2e`. + +Cost is honest. One devDependency, one Chromium download in CI which is cached, and a second test runner in the repo. Two Playwright projects, `web` for chromium and all journeys, and `electron` for one smoke. + +## Harness design + +Server boots in-process inside a Playwright fixture, using the real `buildServer` from `server/index.ts`, so the fake objects stay addressable from the test and can be scripted and asserted mid-journey. + +```ts +// e2e/support/app.ts (worker-scoped dataDir, test-scoped server) +process.env.TUTOR_DATA_DIR ??= mkdtempSync(join(tmpdir(), 'tutor-e2e-')) +const { buildServer } = await import('@server/index.js') // AFTER the env var + +const model = createScriptedTextGeneration() +const app = await buildServer({ + textGeneration: model, + keyVault: createFakeKeyVault({ anthropic: 'e2e-key' }), + speechSynthesis: createFakeSpeechSynthesis(), + audioAssembly: createFakeAudioAssembly(), + imageGeneration: createFakeImageGeneration(), +}) +await app.register(fastifyStatic, { root: distDir }) // test-only, serves dist/ +await app.listen({ port: 0, host: '127.0.0.1' }) +``` + +Why this shape: + +- **`buildServer(overrides)` is Phase 2's real signature** at `server/index.ts:61`, the same one `server/test/route-harness.ts` uses. `startServer` cannot be used because Fastify forbids registering the static plugin after `listen()`. The harness calls `recoverFromCrash()` itself if a journey needs boot parity. +- **Single origin.** In web mode `client/api/http.ts` resolves `_base = ''` and issues relative `/api/...` paths, because `window.electronAPI` is absent. Serving `dist/` from the same instance means no proxy, no CORS, and no `electronAPI` stub. `@fastify/static` is a first-party plugin and a new devDependency, and it replaces the hand-rolled static server Phase 3 used. No SPA fallback is needed because the client routes on view state rather than on URLs. +- **Data isolation.** `TUTOR_DATA_DIR` is worker-scoped and set once, before the first dynamic import of any server module. Per-test isolation comes from a fresh `buildServer()` plus a fresh books directory per test. +- **Zero live traffic is structural, not a promise.** No provider key exists on the real vault, the fake vault is in-memory, and any un-scripted model call throws by construction. + +**Scripted model adapter** (`e2e/support/scripted-text-generation.ts`) implements `FakeTextGeneration` and answers by intent rather than by call order. Rules match on request shape, meaning `schemaName` or a substring of the prompt, and return fixture content while recording every request. Phase 2's FIFO `createFakeTextGeneration` stays untouched for unit and contract tests. A journey makes N calls in an order the UI decides, and coupling fixtures to that order is the single largest flake source. Fidelity is enforced two ways, by `req.schema.parse(value)` before returning so a drifted fixture fails loudly exactly as a malformed model response would, and by running the adapter against Phase 2's own `text-generation.contract.ts` suite in vitest. + +**Fixtures** (`e2e/fixtures/`) are TS modules rather than JSON, so `tsc` catches schema drift. Plus `sample-book.epub`, a small binary generated once by a committed script using the same `epub-gen-memory` the export adapter uses, and `redux-state.json` for the Electron experiment. + +**Journey helpers** (`e2e/support/journeys/*.ts`) are thin page objects exposing intent, such as `wizard.createBook({topic})` and `reader.finishChapter()`. Locators are role- and text-based and never CSS. Phase 3 gave every icon-only button an accessible name, which makes this possible and makes the suite double as an accessibility net. No `data-testid` is added to production components. If one journey genuinely cannot address an element, that is a finding to report rather than a silent client edit. + +## Journeys and acceptance checks + +| # | Journey | Acceptance | +|---|---|---| +| a | Create to TOC streams to edit/approve to chapter 1 streams to read | Wizard accepts topic, `toc` events render chapter titles from the fixture, editing a title persists to `toc.yml`, approve triggers `/start`, chapter 1 prose appears in the reader, `books/{id}/meta.yml` shows `generatedUpTo: 1` | +| b | Read to quiz to submit to feedback to next chapter | Quiz renders the fixture questions, a wrong answer records in the feedback record, submitting feedback triggers generation, chapter 2 text renders, and the scripted adapter's recorded chapter-2 prompt contains the feedback text, which proves the adaptive loop rather than just the plumbing | +| c | EPUB import preview to confirm | `setInputFiles` with `sample-book.epub`, preview dialog shows the fixture title and chapter count, confirm returns to the library with the book present | +| d | EPUB export | Export from the library, background task reaches done, `waitForEvent('download')` yields a non-empty `.epub`, and the file exists under the temp data dir | +| e | Library CRUD and search | Rename persists across reload, tag add shows in the filter surface, search by title filters the grid, delete removes the book from disk | +| f | Audiobook install gate | With an empty data dir, status shows not-installed and the gate appears, and the install button is never clicked because it downloads roughly 90MB from evermeet.cx. A second test seeds the binary and model directory in the temp dir, then generates chapter audio through the fake synthesis and asserts the player appears | +| g | Electron rehydration smoke (`@electron`) | Two runs, same seeded library, differing only in `redux-state.json`. Seeded position opens the reader at the seeded chapter, removed falls back to the server-derived position. Fixture-seeded and generation-free by construction, since fakes cannot be injected into the packaged main process | +| h | Failure journey | Scripted adapter raises with a distinctive message on the chapter stream, and the UI surfaces that message rather than a generic fallback. The injection point is parameterized so Phase 7 can add per-error-class variants | + +## Implementer tasks + +- **S1 — Walking skeleton.** The `e2e/` scaffold, `playwright.config.ts`, the `app.ts` fixture, the scripted adapter, `pnpm e2e`, plus one journey that creates a book and asserts the fixture TOC titles render. Proves static serving, api-base resolution, SSE, the scripted model, and the temp data dir. Also the vitest `exclude: ['e2e/**']` because the default include collects `*.spec.ts` and would otherwise break `pnpm test`, the tsconfig `include`, the ESLint boundary allowing `e2e` to reach `@server` and `@shared` while forbidding `e2e` to reach `@client`, and the lint script glob. +- **S2 — Contract-test the scripted adapter** against `server/ports/text-generation.contract.ts` in vitest. Lands with S1 or immediately after, and this is the fidelity guarantee. +- **S3** — journey (a) completed through approve to chapter 1 to read. +- **S4** — journey (b), including the prompt-contains-feedback assertion. +- **S5** — fixture builder script and committed `sample-book.epub`, plus journey (c). +- **S6** — journey (d). +- **S7** — journey (e). +- **S8** — journey (f), both halves. +- **S9** — journey (h). +- **S10** — Electron project and journey (g), CI jobs, and `e2e/README.md` covering how to run, how to add a journey, why fakes, and why no testids. + +One commit per task, `test(e2e): ...`. S3 to S9 are independent once S1 lands and can fan out across worktrees. + +## CI integration + +Append to the existing `.github/workflows/ci.yml` rather than creating a file, per consolidation delta 5. + +- **`e2e-web`** runs on macos-14 with node 24, keeps `ELECTRON_SKIP_BINARY_DOWNLOAD: 1`, caches `~/Library/Caches/ms-playwright`, runs `pnpm exec playwright install chromium`, `pnpm build`, and `pnpm e2e --project=web`, and uploads `playwright-report/` on failure. +- **`e2e-electron`** is the same but without `ELECTRON_SKIP_BINARY_DOWNLOAD`, because the binary is required. It caches `~/Library/Caches/electron` and runs `pnpm e2e --project=electron`. + +Both run in parallel with `verify`. Budget is roughly 30s for the build, 60 to 90s for the web suite, and 30s for electron, all inside the 3 to 4 minute target. Flake policy is `retries: 0`, `trace: 'retain-on-failure'`, and `workers: 2`. With in-process fakes and no network there is no legitimate source of nondeterminism, so a retry would hide a real bug. A journey that flakes twice gets quarantined with `test.fixme` plus an issue, never a blanket retry. + +## Risks + +1. **Fake-adapter fidelity drift** is the highest. Fixtures could satisfy the tests while diverging from what a real model returns. Mitigated by `schema.parse` on every scripted object, by running Phase 2's contract suite against the scripted adapter in S2, and by fixtures being TS modules typed against the real schemas. Residual risk stays on prompt content, which no fake can cover, and the manual pass in the Phase 2 and Phase 7 gates remains the backstop. +2. **Playwright resolving the server's TS.** Server modules import with `.js` specifiers and `@server` and `@shared` aliases. Playwright's loader handles tsconfig `paths` and extension substitution, but that is unverified here and is S1's first checkpoint. The fallback if it fails is to boot the server in a `tsx` child process and drive the scripted adapter through a test-only control route, which has more moving parts, so it is only used if forced. +3. **Electron in CI.** A GUI app on a hosted macOS runner, plus a large binary download. Isolated in its own job and tagged `@electron` so it can be excluded with one flag without touching the web suite. +4. **SSE timing.** Handled by auto-retrying assertions and zero-delay scripted chunks. The adapter takes an optional `chunkDelayMs` for the one or two journeys where streaming render order matters. +5. **Audiobook install gate touching the network.** Structural, because the install button is never clicked and the journey asserts the gate rather than the install. +6. **Suite runtime creep** as journeys accrete. The gate below pins a number. + +## Phase gate + +- `pnpm e2e` green twice consecutively, locally and in CI, with `retries: 0`. +- Web suite under 120s wall clock, and both e2e jobs under 4 minutes each. +- `pnpm test`, `pnpm typecheck`, and `pnpm lint` unchanged and green, with E2E excluded from vitest and included in tsc and ESLint. +- **Mutation evidence.** With one deliberate break, the suite fails and names the journey, recorded in the PR body. A test suite that has never failed is not yet evidence. +- Zero production source files changed. `git diff --stat master -- client/ server/ shared/ electron/` shows nothing. +- Zero outbound provider traffic. No key exists in either vault, and the scripted adapter has no SDK import. + +### Critical Files for Implementation + +- `server/index.ts` holds `buildServer(overrides)`, the exact boot mechanism the harness calls +- `server/composition-root.ts` holds the `Ports` interface and the override semantics +- `server/ports/text-generation.ts` and its `.fake.ts` and `.contract.ts` siblings define the shape the scripted adapter implements and is contract-tested against +- `client/api/http.ts` explains why single-origin static serving works in web mode +- `.github/workflows/ci.yml` is the file the two E2E jobs append to diff --git a/e2e/README.md b/e2e/README.md new file mode 100644 index 0000000..ddc9a7a --- /dev/null +++ b/e2e/README.md @@ -0,0 +1,106 @@ +# End-to-end journeys + +Up: [../README.md](../README.md) + +This suite drives the real browser UI against the real Fastify server, with the server's outermost adapters swapped for fakes. It is the answer to a fair question a cold reader asks about a hexagonal codebase, which is whether the ports are load bearing or decorative. Every journey here boots the production `buildServer` and hands it fakes through the same `overrides` argument Electron uses to hand it a diagram renderer, and it works. + +## Running it + +```bash +pnpm e2e # everything, both projects +pnpm e2e --project=web # the browser journeys +pnpm e2e --project=electron # the packaged-app smoke, needs the Electron binary +pnpm e2e create-book # one file +pnpm e2e --ui # Playwright's time-travel debugger +``` + +The run builds `dist/` first, every time, in `global-setup.ts`. That is one code path rather than two, so a journey can never test yesterday's client. Set `E2E_SKIP_BUILD=1` while iterating on a journey to skip it. + +Set `E2E_VERBOSE=1` to see the Fastify server's log lines. They are suppressed by default because each test boots its own instance and the JSON request log drowns the reporter. + +`pnpm test` is a different thing and stays fast. It is the vitest unit and contract suite and it never opens a browser. The one file under `e2e/` that vitest DOES own is `support/scripted-text-generation.test.ts`, because that test needs no browser. + +## What a journey gets + +```ts +import { expect, test } from '../support/app.js' + +test('...', async ({ page, model, app }) => { /* ... */ }) +``` + +| Fixture | What it is | +|---|---| +| `page` | A browser page already pointed at this test's own server instance, so `page.goto('/')` just works | +| `app` | `{ origin, dataDir, bookDir(id), fastify }`. `app.fastify.inject(...)` gives an API-level assertion without a second HTTP client | +| `model` | The scripted `TextGeneration` this test's server is wired to. Add rules before the action that triggers a call, read `model.requests` after | + +Plus the helpers: + +- `support/seed.ts` puts books on disk through the real filesystem repository. +- `support/journeys/*.ts` are page objects that expose intent, such as `wizard.approveToc()` and `reader.finishChapter()`. +- `fixtures/*.ts` are the content the scripted model returns, as TypeScript so `tsc` catches drift. + +## Adding a journey + +1. Add a `*.spec.ts` under `e2e/journeys/`, importing `test` and `expect` from `../support/app.js`, never from `@playwright/test`. The re-export is what carries the fixtures. +2. Get to your subject the cheap way. If the journey is about exporting a book, `seedBook(app.dataDir, ...)` rather than driving the whole creation wizard, so a wizard regression fails the wizard journey and not yours. +3. Address the UI the way a screen reader does. `getByRole` first, `getByText` second, and never a CSS class. +4. Assert with `await expect(locator)...`, which retries to a deadline. Never sleep. +5. If your journey needs a model call nobody has scripted yet, add a rule to `support/default-script.ts` matching a phrase lifted verbatim from the server prompt that produces it, and put the content in `fixtures/`. + +## Why fakes rather than mocks + +The server is faked at its ports, not mocked at its HTTP boundary. Everything between the browser and the AI provider is the real thing, meaning real routes, real services, real domain rules, real YAML on a real disk. Only the four edges that would cost money, need a network, or need a 90MB binary are swapped, and they are swapped through `buildServer(overrides)`, the same seam production already has. + +Mocking `fetch` in the browser instead would have tested the client against a fiction of the server. This tests it against the server. + +Zero live-provider traffic is a property of the design rather than a promise. The real key vault is never constructed, the fake one lives only in memory, and the scripted model has no SDK import and throws on any call nobody wrote a fixture for. There is no path from a journey to a provider even if someone wanted one. + +## Why the model answers by intent + +`support/scripted-text-generation.ts` matches on the shape of a request, not on the order calls arrive in. Phase 2's `createFakeTextGeneration` answers strictly first-in first-out, which is right for a unit test that knows exactly how many calls the code under test makes. A journey does not know that. Approving a table of contents fires a skill classification, a chapter stream, and a quiz, in an order the UI decides and may reorder tomorrow, and a fixture list coupled to that order is the largest available source of flake. + +Rules are consulted newest first, so a test shadows a default with one line, which is also how failure injection works: + +```ts +model.onStreamText({ + name: 'chapter 2 is rate limited', + match: isChapterStreamFor(2), + respond: { throws: new Error('rate limit exceeded') }, +}) +``` + +`throws` takes any value and rethrows it unchanged, so a typed error class survives all the way to the caller. + +The fidelity guarantee is `support/scripted-text-generation.test.ts`, which runs the port's own contract suite against this adapter with no exemptions, plus `req.schema.parse(value)` on every generated object so a drifted fixture fails as loudly as a malformed model response would. + +## Why no test ids + +There is not one `data-testid` in the client, and adding one would be the easy way out of every hard locator in this suite. The rule holds because addressing the UI by role and by name means the suite doubles as an accessibility net. Writing these journeys found seven real gaps, all filed as issue 51 rather than papered over with a test id. + +1. A book card is a `div` with an `onClick`, so it has no role, no accessible name, and no focus. A keyboard user cannot open a book at all. This is the serious one. +2. Two controls share the name "Next section", the chapter rail's and the tap zone's, both wired to the same callback. +3. The book context menu is a plain `div` of buttons with no `menu` or `menuitem` semantics. +4. The feedback form's textareas are not label-associated, so neither has an accessible name. +5. The Rename dialog's Title and Subtitle labels are not wired to their inputs. +6. The Delete dialog's confirmation input has no label. +7. The library toolbar's icon-only search and view toggles carry a `title` attribute but no `aria-label`. + +The one sanctioned exception is a hidden ``, which has no role and no accessible name by construction. It is commented where it appears. + +## Flake policy + +`retries: 0`, and that is the most load-bearing line in `playwright.config.ts`. The server is in process, the adapters are fakes, and nothing touches a network, so there is no legitimate source of nondeterminism left. A retry could only convert a real bug into a green run. A journey that flakes twice gets quarantined with `test.fixme` and an issue, never a blanket retry. + +Two assertions are quarantined today, and neither is flake. Both are blocked on issue 50, a real bug this suite found in chapter generation's event stream, and each `test.fixme` names it. They flip green when it is fixed. + +One real source of nondeterminism does exist inside the product and journeys have to respect it. `server/services/generate-quiz.ts` shuffles a quiz's options with `Math.random()` before saving, so a quiz option must be located by its text and never by its position. + +## Projects + +| Project | Selects | Runs where | +|---|---|---| +| `web` | everything not tagged `@electron` | the `e2e-web` CI job, Chromium, headless | +| `electron` | tests tagged `@electron` | the `e2e-electron` CI job, which is the only one that downloads the Electron binary | + +The Electron project exists for one thing that the web project cannot reach at all. In the browser redux-persist writes to localStorage, and in the packaged app it writes through IPC to a file the main process owns, so the rehydration path is only observable with a real main process. Fakes cannot be injected into a packaged main process, so that journey seeds its library rather than generating one. diff --git a/e2e/fixtures/chapter-stream.ts b/e2e/fixtures/chapter-stream.ts new file mode 100644 index 0000000..36cc041 --- /dev/null +++ b/e2e/fixtures/chapter-stream.ts @@ -0,0 +1,32 @@ +import { TOC_CHAPTERS } from './toc-stream.js' + +/** + * The chapter prose the scripted model streams back. + * + * Every chapter carries a distinctive marker sentence, so a journey can + * assert it is looking at chapter 2 and not a stale chapter 1 without + * matching on a heading the UI might also render in a sidebar. + */ + +/** The sentence that appears only in chapter `num`. Journeys locate on this. */ +export function chapterMarker(num: number): string { + return `This is the seeded prose for chapter ${num}.` +} + +/** The full markdown for chapter `num`, matching the shape the reader renders. */ +export function chapterMarkdown(num: number): string { + const chapter = TOC_CHAPTERS[num - 1] + const title = chapter ? chapter.title : `Chapter ${num}` + return [ + `# ${title}\n\n`, + `${chapterMarker(num)}\n\n`, + '## Why it matters\n\n', + 'A body that spins loses energy to the tide it raises on its partner, and it keeps losing energy until the spin and the orbit agree.\n\n', + '> The brake never releases, it only runs out of spin to remove.\n', + ].join('') +} + +/** The same markdown split into stream chunks, so the journey exercises the real SSE reassembly. */ +export function chapterStreamChunks(num: number): string[] { + return chapterMarkdown(num).split(/(?<=\n\n)/) +} diff --git a/e2e/fixtures/quiz.ts b/e2e/fixtures/quiz.ts new file mode 100644 index 0000000..add373b --- /dev/null +++ b/e2e/fixtures/quiz.ts @@ -0,0 +1,64 @@ +/** + * The quiz the scripted model returns after each chapter. + * + * Note for journeys: `server/services/generate-quiz.ts` shuffles the options + * with `Math.random()` before saving, so the stored `correctIndex` is not the + * one written here and the on-screen option order is not this order. A + * journey must therefore locate an option by its TEXT, never by its position, + * and must decide right from wrong by comparing against these strings. + */ + +export interface QuizFixtureQuestion { + question: string + options: string[] + /** Index into `options` as written here, before the service shuffles them. */ + correctIndex: number +} + +export const QUIZ_QUESTIONS: QuizFixtureQuestion[] = [ + { + question: 'What removes a body\'s rotational energy during tidal locking?', + options: [ + 'The tidal bulge dragging against the body\'s own rotation', + 'The solar wind stripping momentum from the upper atmosphere', + 'Magnetic coupling between the two bodies\' iron cores', + 'Radioactive decay slowly redistributing mass in the mantle', + ], + correctIndex: 0, + }, + { + question: 'What state does a tidally locked body settle into?', + options: [ + 'Its rotation period matches its orbital period around the partner', + 'Its rotation stops entirely relative to the background stars', + 'Its orbit becomes perfectly circular and stops precessing', + 'Its axis of rotation aligns exactly with the orbital plane normal', + ], + correctIndex: 0, + }, + { + question: 'What mainly sets how long locking takes?', + options: [ + 'The separation between the bodies, which the tidal force depends on steeply', + 'The absolute temperature of the smaller body at the time of formation', + 'The total number of other satellites sharing the same orbital resonance', + 'The chemical composition of the atmosphere retained by the larger body', + ], + correctIndex: 0, + }, +] + +/** The value the scripted adapter returns for a quiz generateObject call. */ +export const QUIZ_FIXTURE = { questions: QUIZ_QUESTIONS } + +/** The correct option text for a question, located by its text rather than its shuffled position. */ +export function correctOptionFor(question: QuizFixtureQuestion): string { + return question.options[question.correctIndex] +} + +/** Any option that is not the correct one, for journeys that need to answer wrongly on purpose. */ +export function wrongOptionFor(question: QuizFixtureQuestion): string { + const wrong = question.options.find((_option, index) => index !== question.correctIndex) + if (!wrong) throw new Error('quiz fixture: every option is the correct one, so there is no wrong answer to pick') + return wrong +} diff --git a/e2e/fixtures/redux-state.ts b/e2e/fixtures/redux-state.ts new file mode 100644 index 0000000..b8bb535 --- /dev/null +++ b/e2e/fixtures/redux-state.ts @@ -0,0 +1,47 @@ +/** + * Builds the `redux-state.json` file the Electron main process reads on + * behalf of redux-persist. + * + * The nesting is redux-persist's, not this project's, and it is worth + * spelling out because it is easy to get wrong from the outside. The file is + * an object keyed by storage key. Under `persist:tutor` sits a JSON STRING + * whose parsed value is an object with one entry per persisted slice, and + * each of THOSE values is itself a JSON string. Two levels of stringification, + * because redux-persist serialises each slice independently so that one + * corrupt slice cannot take the rest down with it. + * + * `client/store/persist.ts` sets `key: 'tutor'` and swaps its storage for + * Electron IPC when `window.electronAPI.storageGet` exists, and + * `electron/main.ts` answers that IPC out of this file. + * + * Only the reading position is written here, deliberately. redux-persist's + * default reconciler merges one level deep, meaning a persisted slice + * REPLACES that slice's initial state wholesale rather than being merged into + * it. Writing a partial `settings` slice therefore deletes the provider + * configuration the client reads on its first render, and the renderer dies + * with "Cannot read properties of undefined". Found the hard way. Persist a + * slice here only if the fixture supplies every field that slice's initial + * state declares. + */ + +export interface ReduxStateFixture { + /** Where the reader resumes. Omit it to make the reader fall back to the server-derived position. */ + position?: { bookId: string; chapter: number; section: number } +} + +/** The parsed contents of `redux-state.json` for the given fixture. */ +export function buildReduxState(fixture: ReduxStateFixture): Record { + const slices: Record = { + _persist: JSON.stringify({ version: -1, rehydrated: true }), + } + + if (fixture.position) { + const { bookId, chapter, section } = fixture.position + slices.readingProgress = JSON.stringify({ + positions: { [bookId]: { chapter, section, lastReadAt: new Date().toISOString() } }, + furthest: { [bookId]: chapter }, + }) + } + + return { 'persist:tutor': JSON.stringify(slices) } +} diff --git a/e2e/fixtures/sample-book.epub b/e2e/fixtures/sample-book.epub new file mode 100644 index 0000000..239b959 Binary files /dev/null and b/e2e/fixtures/sample-book.epub differ diff --git a/e2e/fixtures/sample-book.ts b/e2e/fixtures/sample-book.ts new file mode 100644 index 0000000..9b21507 --- /dev/null +++ b/e2e/fixtures/sample-book.ts @@ -0,0 +1,35 @@ +/** + * The EPUB fixture the import journey (`e2e/journeys/epub-import.spec.ts`) drives + * through the real epub2 parser (`server/adapters/epub2-import.ts`), unlike every + * other journey, whose fixtures are consumed by the scripted model instead. + * + * These constants are the single source of truth for what + * `scripts/build-e2e-epub-fixture.ts` bakes into the committed binary at + * `e2e/fixtures/sample-book.epub`, and for what the journey then asserts + * against, so the fixture's real content and the journey's expectations + * cannot drift the way two hand-copied string literals could. + */ + +export const SAMPLE_BOOK_TITLE = 'Bioluminescence in the Midnight Zone' + +export interface SampleBookChapter { + title: string + /** Wrapped in a single

when the fixture script builds the EPUB. */ + body: string +} + +/** The chapters the fixture EPUB contains, in spine order. The journey asserts against this title list and its length. */ +export const SAMPLE_BOOK_CHAPTERS: SampleBookChapter[] = [ + { + title: 'Light Without Heat', + body: 'Bioluminescent light comes from a chemical reaction rather than from heat, so a deep-sea body can glow without ever warming the water around it.', + }, + { + title: 'Signaling in the Dark', + body: 'Below the reach of sunlight, a flash or a glow carries the signal that color and shape would otherwise carry, from a mating display to a warning.', + }, + { + title: 'The Predators That Glow Back', + body: 'Some hunters borrow the very glow their prey uses to hide, turning a warning signal into a lure that draws the next meal closer.', + }, +] diff --git a/e2e/fixtures/skills.ts b/e2e/fixtures/skills.ts new file mode 100644 index 0000000..16f7035 --- /dev/null +++ b/e2e/fixtures/skills.ts @@ -0,0 +1,25 @@ +import { TOC_CHAPTERS } from './toc-stream.js' + +/** + * The skill classification `server/services/start-book.ts` asks for right + * before it streams chapter 1. Its schema is declared inline in that service, + * so this fixture is validated against it at run time by the scripted + * adapter's `schema.parse`, which is what catches drift if that schema + * changes. + * + * Classification failure is non-fatal in the service, so a drifted fixture + * would not fail the journey, it would silently skip the skills. The + * `skills_classified` assertion in journey (a) is what keeps that honest. + */ +export const SKILL_CLASSIFICATION = { + skills: [ + { name: 'Orbital Mechanics', weight: 5 }, + { name: 'Classical Dynamics', weight: 3 }, + ], + chapters: TOC_CHAPTERS.map((_chapter, index) => ({ + chapterIndex: index, + skills: [ + { skill: 'Orbital Mechanics', subskill: `Concept ${index + 1}`, weight: 2 }, + ], + })), +} diff --git a/e2e/fixtures/toc-stream.ts b/e2e/fixtures/toc-stream.ts new file mode 100644 index 0000000..cc8a6f5 --- /dev/null +++ b/e2e/fixtures/toc-stream.ts @@ -0,0 +1,66 @@ +/** + * The table of contents the scripted model streams back for every journey + * that creates a book. + * + * It is a TS module rather than JSON so `tsc` sees it, and it is split into + * chunks rather than one string so the journey exercises the real SSE path, + * many `toc` events accumulating in the client, rather than a single write + * that would hide a reassembly bug. + * + * The markdown shape is the one `server/services/toc-parser.ts` accepts, a + * `#` title, an italic subtitle, then numbered `**bold** — description` + * lines. Changing the shape here without checking that parser is the fastest + * way to make every creation journey fail at once. + */ + +export const TOC_BOOK_TITLE = 'Tidal Locking' +export const TOC_BOOK_SUBTITLE = 'How Worlds Stop Spinning' + +export interface TocFixtureChapter { + title: string + description: string +} + +/** The chapters the fixture TOC parses into, in order. Journeys assert against these. */ +export const TOC_CHAPTERS: TocFixtureChapter[] = [ + { title: 'Angular Momentum', description: 'Why a spinning body keeps spinning until something takes the spin away.' }, + { title: 'Raising the Bulge', description: 'The tidal force that deforms a body and drags its own rotation backwards.' }, + { title: 'The Slow Brake', description: 'Working out how long a world takes to fall into a locked rotation.' }, +] + +/** + * The fixture streamed as the model would produce it, one chunk per line, so + * the client accumulates the same partial markdown a real run would. + */ +export const TOC_STREAM_CHUNKS: string[] = [ + `# ${TOC_BOOK_TITLE}\n`, + `*${TOC_BOOK_SUBTITLE}*\n`, + '\n', + ...TOC_CHAPTERS.map((chapter, index) => `${index + 1}. **${chapter.title}** — ${chapter.description}\n`), +] + +/** The whole fixture as one string, for assertions that want the parsed result rather than the stream. */ +export const TOC_MARKDOWN = TOC_STREAM_CHUNKS.join('') + +/** + * A revised table of contents, with visibly different chapter titles than + * `TOC_CHAPTERS` but the same descriptions, title, and subtitle, standing in + * for a reader who asked to simplify the chapter titles without touching + * their content. `default-script.ts` wires the `revise-toc` stream to this + * fixture rather than to `TOC_STREAM_CHUNKS`, so a revision journey can + * assert that revising actually changed something instead of the stream + * just replaying the original TOC back. + */ +export const TOC_REVISED_CHAPTERS: TocFixtureChapter[] = [ + { title: 'Why It Keeps Turning', description: 'Why a spinning body keeps spinning until something takes the spin away.' }, + { title: 'The Bulge Explained', description: 'The tidal force that deforms a body and drags its own rotation backwards.' }, + { title: 'Braking To Zero', description: 'Working out how long a world takes to fall into a locked rotation.' }, +] + +/** `TOC_REVISED_CHAPTERS`, streamed the same way `TOC_STREAM_CHUNKS` streams `TOC_CHAPTERS`. */ +export const TOC_REVISED_STREAM_CHUNKS: string[] = [ + `# ${TOC_BOOK_TITLE}\n`, + `*${TOC_BOOK_SUBTITLE}*\n`, + '\n', + ...TOC_REVISED_CHAPTERS.map((chapter, index) => `${index + 1}. **${chapter.title}** — ${chapter.description}\n`), +] diff --git a/e2e/global-setup.ts b/e2e/global-setup.ts new file mode 100644 index 0000000..73ea266 --- /dev/null +++ b/e2e/global-setup.ts @@ -0,0 +1,22 @@ +import { execFileSync } from 'node:child_process' +import { fileURLToPath } from 'node:url' + +/** + * Builds the client bundle the app fixture serves out of `dist/`. + * + * This runs once per `playwright test` invocation rather than being a + * separate CI step, so there is exactly one code path and no way to run the + * journeys against a stale bundle. `vite build` also emits `dist-electron/`, + * which the Electron project needs. + * + * Set `E2E_SKIP_BUILD=1` when iterating on a journey and the client has not + * changed, which turns a 15 second startup into an instant one. + */ +export default function globalSetup(): void { + if (process.env.E2E_SKIP_BUILD) { + console.log('[e2e] E2E_SKIP_BUILD set, reusing the existing dist/ build') + return + } + const repoRoot = fileURLToPath(new URL('..', import.meta.url)) + execFileSync('pnpm', ['exec', 'vite', 'build'], { cwd: repoRoot, stdio: 'inherit' }) +} diff --git a/e2e/journeys/adaptive-loop.spec.ts b/e2e/journeys/adaptive-loop.spec.ts new file mode 100644 index 0000000..515a613 --- /dev/null +++ b/e2e/journeys/adaptive-loop.spec.ts @@ -0,0 +1,125 @@ +import { chapterMarker } from '../fixtures/chapter-stream.js' +import { QUIZ_QUESTIONS, correctOptionFor, wrongOptionFor } from '../fixtures/quiz.js' +import { TOC_BOOK_TITLE } from '../fixtures/toc-stream.js' +import { expect, test } from '../support/app.js' +import { library } from '../support/journeys/library.js' +import { reader } from '../support/journeys/reader.js' +import { quiz, feedback } from '../support/journeys/quiz.js' +import { bookRepository, seedBook } from '../support/seed.js' + +/** + * Journey (b): the adaptive loop, split across two tests on purpose. + * + * Everything downstream of a chapter's quiz and feedback works. The quiz + * renders the fixture questions, a wrong answer is recorded, submitting + * feedback triggers chapter 2's generation, and the scripted model records a + * chapter-2 prompt that contains both feedback markers and the wrongly + * answered question's text. TEST 1 below proves exactly that, and it is the + * single most important assertion in this suite, because it is what tells + * the adaptive loop apart from a button that merely looks like it worked. + * + * What does not work today is the browser ever finding out. Generation is + * server-side truth, the model is genuinely called and the chapter is + * genuinely saved to disk, but the SSE reply meant to carry that content + * back to the reader is closed with zero bytes almost immediately. This is a + * real production bug in server/routes/generation.ts's pipeHubToSse, which + * listens for the incoming request's own close event rather than the + * response's, filed as github.com/rsml/tutor/issues/50 and out of scope for + * this phase. TEST 1 therefore never asserts on-screen chapter 2 content and + * never waits on it, it waits on the model's own recorded request instead. + * TEST 2 is the on-screen assertion, quarantined with test.fixme until issue + * 50 lands, so a reader does not mistake TEST 1's silence on rendering for + * an oversight. + * + * See support/journeys/quiz.ts for why every quiz option below is located by + * exact text rather than position. generate-quiz.ts shuffles the options + * with Math.random() before saving, so the on-screen order is neither the + * fixture's authored order nor stable across runs. + */ +test('quiz answers and chapter feedback shape the chapter-2 prompt the model actually receives', async ({ page, app, model }) => { + const book = await seedBook(app.dataDir, { generatedUpTo: 1 }) + + await page.goto('/') + await library(page).waitForReady() + await library(page).openBook(TOC_BOOK_TITLE) + + await reader(page).waitForChapter() + await reader(page).finishChapter() + + // The quiz, not chapter 2, is what "Next Chapter" opens at the end of a + // chapter. Every fixture question must be visible before any is answered. + await quiz(page).waitForQuiz() + for (const question of QUIZ_QUESTIONS) { + await expect(quiz(page).questionText(question)).toBeVisible() + } + + const [wronglyAnswered, ...restAnsweredCorrectly] = QUIZ_QUESTIONS + await quiz(page).answer(wrongOptionFor(wronglyAnswered)) + for (const question of restAnsweredCorrectly) { + await quiz(page).answer(correctOptionFor(question)) + } + await quiz(page).reveal() + await quiz(page).confirm() + + const LIKED_MARKER = 'Glimmercrash pinwheel marker, the tidal bulge analogy landed instantly' + const DISLIKED_MARKER = 'Wobbletrail marker, the angular momentum section moved too fast' + + await feedback(page).waitForForm(1) + await feedback(page).fill(LIKED_MARKER, DISLIKED_MARKER) + await feedback(page).submit() + + // Generation is server-side truth even though the browser never learns it + // finished (issue 50), so the wait is on the model's own recorded request + // rather than on anything the page renders. + await expect.poll(() => model.requests.streamText.some(r => (r.prompt ?? '').includes('This is Chapter 2 of'))).toBe(true) + + // The wrong answer is recorded on disk against chapter 1's feedback, not + // just reflected back in the UI. + const savedFeedback = await bookRepository(app.dataDir).getFeedback(book.id, 1) + expect(savedFeedback.feedback.liked).toBe(LIKED_MARKER) + expect(savedFeedback.feedback.disliked).toBe(DISLIKED_MARKER) + expect(savedFeedback.quiz.score).toBe(2) + expect(savedFeedback.quiz.questions[0].correct).toBe(false) + expect(savedFeedback.quiz.questions[1].correct).toBe(true) + expect(savedFeedback.quiz.questions[2].correct).toBe(true) + + // THE KEY ASSERTION. generate-next-chapter.ts builds chapter 2's prompt + // from getAllFeedback(), wrapping liked/disliked text in / + // tags and, when a quiz answer was wrong, adding a + // "Struggled with: " line. Finding this exact request and + // checking its prompt is what proves the feedback loop feeds the next + // chapter, rather than merely that a button was clicked. + expect(model.requests.streamText).toHaveLength(1) + const chapterTwo = model.requests.streamText.find(r => (r.prompt ?? '').includes('This is Chapter 2 of')) + expect(chapterTwo?.prompt).toContain(LIKED_MARKER) + expect(chapterTwo?.prompt).toContain(DISLIKED_MARKER) + expect(chapterTwo?.prompt).toContain(wronglyAnswered.question) +}) + +test.fixme('renders chapter 2 in the reader once generation finishes', async ({ page, app }) => { + // Quarantined by github.com/rsml/tutor/issues/50. pipeHubToSse closes the + // SSE reply with zero bytes before the browser ever hears "done", so + // chapter 2 never reaches the screen today. Flip back to test() once that + // lands, nothing else here should need to change. + await seedBook(app.dataDir, { generatedUpTo: 1 }) + + await page.goto('/') + await library(page).waitForReady() + await library(page).openBook(TOC_BOOK_TITLE) + + await reader(page).waitForChapter() + await reader(page).finishChapter() + + await quiz(page).waitForQuiz() + for (const question of QUIZ_QUESTIONS) { + await quiz(page).answer(correctOptionFor(question)) + } + await quiz(page).reveal() + await quiz(page).confirm() + + await feedback(page).waitForForm(1) + await feedback(page).fill('liked', 'disliked') + await feedback(page).submit() + + await expect(reader(page).prose(chapterMarker(2))).toBeVisible() +}) diff --git a/e2e/journeys/audiobook.spec.ts b/e2e/journeys/audiobook.spec.ts new file mode 100644 index 0000000..c61a566 --- /dev/null +++ b/e2e/journeys/audiobook.spec.ts @@ -0,0 +1,114 @@ +import { TOC_BOOK_TITLE, TOC_CHAPTERS } from '../fixtures/toc-stream.js' +import { expect, test } from '../support/app.js' +import { library } from '../support/journeys/library.js' +import { reader } from '../support/journeys/reader.js' +import { audiobook } from '../support/journeys/audiobook.js' +import { seedBook } from '../support/seed.js' + +/** + * This is journey (f). It covers the audiobook narration engine's install + * gate, and the happy path once that engine is already installed. + * + * Both halves seed a fully generated book, because "Generate audiobook" (the + * library's book context menu, see support/journeys/audiobook.ts) renders + * disabled until generatedUpTo === totalChapters. + * + * Neither half ever performs a real install. The first half never installs + * anything at all, and asserts the gate that appears in its absence. The + * second half flips the fake speechSynthesis port's installed state through + * the server's own route, not through the UI's Download button, for reasons + * explained on that test. + */ + +test('blocks audiobook generation behind a missing-components install gate', async ({ page, app }) => { + await seedBook(app.dataDir, { generatedUpTo: TOC_CHAPTERS.length }) + + await page.goto('/') + await library(page).waitForReady() + await audiobook(page).openBookMenu(TOC_BOOK_TITLE) + await audiobook(page).generateAudiobookMenuItem().click() + + // Nothing in this process has installed the model or ffmpeg (the fixture + // wipes books/ per test and never seeds either), so the fake + // speechSynthesis port reports both missing and the gate opens instead of + // starting generation. + const gate = audiobook(page).downloadGateDialog() + await expect(gate).toBeVisible() + await expect(gate.getByText('Kokoro')).toBeVisible() + await expect(gate.getByText('FFmpeg')).toBeVisible() + + // The button itself reports a size, which is the proof components are + // actually reported missing rather than the dialog just opening with + // placeholder copy. + await expect(audiobook(page).downloadGateButton()).toBeVisible() + + // NEVER CLICK THIS BUTTON, and never add a `.click()` on downloadGateButton + // anywhere in this file. In the real app this button downloads the Kokoro + // model and an ffmpeg binary, on the order of 195 MB combined, from + // huggingface and evermeet.cx over the public internet (see + // server/services/audiobook-installer.ts, KOKORO_MODEL_SIZE_BYTES and + // FFMPEG_SIZE_BYTES). This suite's fixture fakes out speechSynthesis + // (e2e/support/app.ts), so this test process would not actually reach the + // network, but the rule holds regardless of what happens to back the + // button today. This test's job is to prove the gate appears, never to + // press the control that starts a real installation. +}) + +test('generates chapter audio through the fake engine once installed and shows the listen control', async ({ page, app }) => { + // Flips the fake speechSynthesis port's installed state directly through + // the server, the same operation server/services/generate-audiobook.test.ts + // performs by calling speechSynthesis.install() on the fake object + // directly (see installEngine() in that file). fastify.inject() runs the + // real route handler in-process with no socket involved, the same + // technique create-book.spec.ts already uses to read /api/books directly. + // + // Filesystem seeding does not work for this. server/composition-root.ts's + // createPorts() spreads its `overrides` object after building every + // default adapter, so the override e2e/support/app.ts passes, + // createFakeSpeechSynthesis(), replaces ports.speechSynthesis outright. + // Every audiobook route calls only ports.speechSynthesis, never + // server/services/audiobook-installer.ts directly (routes/audiobook.ts's + // /api/audiobook/status handler, for one, calls + // ports.speechSynthesis.isInstalled() and .missingComponents()). The + // fake's isInstalled() is two in-memory booleans that only flip inside its + // own install() (server/ports/speech-synthesis.fake.ts), and that file's + // own doc comment says it "never touches the real filesystem". So writing + // bin/ffmpeg or a models/kokoro/**.onnx file under app.dataDir, the paths + // server/services/audiobook-installer.ts checks, would be inert here. No + // code path in this process ever reads them. + // + // This is not the gate's Download button from the test above. It is an + // in-process request the test driver makes before any page exists, so + // there is no download control on screen for it to stand in for. The + // fake's install() only flips those two booleans in memory. It does not + // write to the filesystem, spawn a child process, or make a network call. + await app.fastify.inject({ method: 'POST', url: '/api/audiobook/install' }) + await expect.poll(async () => { + const status = await app.fastify.inject({ method: 'GET', url: '/api/audiobook/status' }) + return (status.json() as { installed: boolean }).installed + }).toBe(true) + + await seedBook(app.dataDir, { generatedUpTo: TOC_CHAPTERS.length }) + + await page.goto('/') + await library(page).waitForReady() + await audiobook(page).openBookMenu(TOC_BOOK_TITLE) + await audiobook(page).generateAudiobookMenuItem().click() + + // Engine already installed, so the library goes straight to the voice + // picker instead of the download gate. + await audiobook(page).startGenerationButton().click() + + await library(page).openBook(TOC_BOOK_TITLE) + await reader(page).waitForChapter() + + // The fake speechSynthesis and fake audioAssembly (both wired in + // e2e/support/app.ts) narrate and stitch in memory with no real timers, so + // generation has typically already finished by the time the reader mounts. + // This is still a web-first assertion rather than an assumption. It + // retries regardless, covering the rare case where it is still catching + // up. + await expect(audiobook(page).listenButton()).toBeVisible() + await audiobook(page).listenButton().click() + await expect(audiobook(page).closePlayerButton()).toBeVisible() +}) diff --git a/e2e/journeys/create-book.spec.ts b/e2e/journeys/create-book.spec.ts new file mode 100644 index 0000000..608959c --- /dev/null +++ b/e2e/journeys/create-book.spec.ts @@ -0,0 +1,119 @@ +import type { Page } from '@playwright/test' +import { chapterMarker } from '../fixtures/chapter-stream.js' +import { TOC_BOOK_TITLE, TOC_CHAPTERS, TOC_REVISED_CHAPTERS } from '../fixtures/toc-stream.js' +import { expect, test } from '../support/app.js' +import { library } from '../support/journeys/library.js' +import { wizard } from '../support/journeys/wizard.js' +import { bookRepository, readBook } from '../support/seed.js' + +/** + * The revise-toc panel's own input and send control, addressed locally + * rather than added to `wizard()` because `e2e/support/journeys/wizard.ts` + * is owned by another concurrent phase-6 agent. Promote this into that file + * if a later journey needs it too. + */ +function reviseTocPanel(page: Page) { + return { + /** Types a revision instruction and submits it. */ + async submit(feedback: string): Promise { + await page.getByPlaceholder('Change chapter 1 to be simpler...').fill(feedback) + await page.getByRole('button', { name: 'Send revision' }).click() + }, + } +} + +/** + * Journey (a), first half: the walking skeleton. + * + * This one test is doing more than it looks like. Passing it proves the + * whole harness at once, that the built client is served from the same + * origin as the API, that `client/api/http.ts` resolves its base to empty + * and issues relative requests, that the server-sent-event stream reaches + * the browser and reassembles, that the scripted model answers the + * create-book call site by intent, and that everything wrote into a + * throwaway data directory instead of the reader's real library. + * + * S3 extends this file through approval, chapter 1, and the reader. + */ +test('creates a book and streams the table of contents into the wizard', async ({ page, model, app }) => { + await page.goto('/') + await library(page).waitForReady() + await library(page).openWizard() + + await wizard(page).submit({ topic: 'Tidal locking', details: 'Aim it at a physics graduate.' }) + + // Web-first assertions retry to a deadline, which is how this suite waits + // for a stream to finish without ever sleeping. + for (const chapter of TOC_CHAPTERS) { + await expect(wizard(page).tocEntry(chapter.title)).toBeVisible() + } + await wizard(page).waitForTocApproval() + + // The prompt the server actually sent, read off the same object the server + // called. This is what an in-process fake buys over a child process. + expect(model.requests.streamText).toHaveLength(1) + expect(model.requests.streamText[0].prompt).toContain('Tidal locking') + expect(model.requests.streamText[0].prompt).toContain('Aim it at a physics graduate.') + + // And the book landed on disk, in the throwaway directory, with the title + // parsed out of the fixture rather than the topic the reader typed. + const books = await app.fastify.inject({ method: 'GET', url: '/api/books' }) + expect(books.statusCode).toBe(200) + const library_ = books.json() as Array<{ id: string; title: string; status: string }> + expect(library_).toHaveLength(1) + expect(library_[0].title).toBe(TOC_BOOK_TITLE) + expect(library_[0].status).toBe('toc_review') +}) + +/** + * Journey (a), second half: revise the table of contents, approve it, and + * land on chapter 1. + * + * Revising first is what makes the approval meaningful to test here — it + * proves the book that gets started is the one on disk after the edit, not a + * stale in-memory copy of the original streamed TOC. `default-script.ts` + * points the revise-toc stream at `TOC_REVISED_CHAPTERS`, a fixture with + * visibly different titles than `TOC_CHAPTERS`, so asserting those titles + * render and persist actually exercises the revision rather than coincidence. + */ +test('revises the table of contents, persists it, and generates chapter 1 on approval', async ({ page, model, app }) => { + await page.goto('/') + await library(page).waitForReady() + await library(page).openWizard() + await wizard(page).submit({ topic: 'Tidal locking', details: 'Aim it at a physics graduate.' }) + await wizard(page).waitForTocApproval() + + await wizard(page).openTocEditor() + await reviseTocPanel(page).submit('Make every chapter title a single short, punchy phrase.') + + // The revised titles rendering is the stream having reassembled the new + // fixture, the same way TOC_CHAPTERS rendering proves it in the first test. + for (const chapter of TOC_REVISED_CHAPTERS) { + await expect(wizard(page).tocEntry(chapter.title)).toBeVisible() + } + + const booksRes = await app.fastify.inject({ method: 'GET', url: '/api/books' }) + const id = (booksRes.json() as Array<{ id: string }>)[0].id + + // Persistence, checked on disk rather than on screen: the revision wrote + // through to toc.yml, not just into the streaming buffer the page renders. + const toc = await bookRepository(app.dataDir).getToc(id) + expect(toc.chapters.map(chapter => chapter.title)).toEqual(TOC_REVISED_CHAPTERS.map(chapter => chapter.title)) + + await wizard(page).approveToc() + + await expect(page.getByText(chapterMarker(1)).first()).toBeVisible() + + const book = await readBook(app.dataDir, id) + expect(book.generatedUpTo).toBe(1) + expect(book.status).toBe('reading') + + // And the model path /start actually drives: a skill classification call + // and a chapter-1 stream, not just the revise-toc stream from earlier. + expect(model.requests.generateObject.some( + req => (req.prompt ?? '').includes('classifying the learning content of a book'), + )).toBe(true) + expect(model.requests.streamText.some( + req => (req.prompt ?? '').includes('This is Chapter 1 of'), + )).toBe(true) +}) diff --git a/e2e/journeys/electron-rehydration.spec.ts b/e2e/journeys/electron-rehydration.spec.ts new file mode 100644 index 0000000..f0d661d --- /dev/null +++ b/e2e/journeys/electron-rehydration.spec.ts @@ -0,0 +1,126 @@ +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { fileURLToPath } from 'node:url' +import { _electron as electron, expect, test, type ElectronApplication, type Page } from '@playwright/test' +import { chapterMarker } from '../fixtures/chapter-stream.js' +import { buildReduxState, type ReduxStateFixture } from '../fixtures/redux-state.js' +import { TOC_BOOK_TITLE } from '../fixtures/toc-stream.js' +import { bookRepository, seedBook } from '../support/seed.js' + +/** + * Journey (g): the packaged app rehydrates where the reader left off. + * + * This is the only journey that runs the real Electron main process, and it + * is deliberately the smallest thing worth running there. Fakes cannot be + * injected into a packaged main process, so anything that would call a model + * is out of reach by construction, which is why this journey seeds its + * library rather than generating one. + * + * What IS only observable here is the rehydration path. In the browser + * redux-persist writes to localStorage, and in Electron it writes through IPC + * to a file the main process owns, so the file is a surface the web project + * cannot reach at all. Two runs over the same seeded library, differing only + * in `redux-state.json`. With a saved position the reader opens where the + * reader was, and without one it falls back to a position derived from the + * server's own progress record. Both are correct, and getting them backwards + * is exactly the bug a smoke test should catch. + * + * Tagged `@electron` so the `electron` Playwright project selects it and the + * `web` project excludes it with one flag, which is what keeps the Electron + * binary out of every other CI job. + */ + +const REPO_ROOT = fileURLToPath(new URL('../..', import.meta.url)) + +/** How many chapters the seeded book records as read. The fallback position is derived from this. */ +const CHAPTERS_READ = 3 + +/** Electron has a real window to open and a real server to start, so it gets longer than a browser page load. */ +const BOOT_TIMEOUT_MS = 40_000 + +interface LaunchedApp { + app: ElectronApplication + page: Page + dataDir: string + bookId: string +} + +/** + * Seeds a data directory with one readable book and a `redux-state.json` + * built from `fixture`, then launches the packaged app against it. + */ +async function launchWithState(fixture: (bookId: string) => ReduxStateFixture): Promise { + const dataDir = mkdtempSync(join(tmpdir(), 'tutor-e2e-electron-')) + const book = await seedBook(dataDir, { + status: 'reading', + totalChapters: CHAPTERS_READ, + generatedUpTo: CHAPTERS_READ, + chaptersOnDisk: CHAPTERS_READ, + }) + + // The server-derived fallback reads the progress record rather than + // generatedUpTo, so the record has to exist for the second run to have + // anything to fall back to. + const books = bookRepository(dataDir) + for (let num = 1; num <= CHAPTERS_READ; num++) { + await books.saveChapterProgress(book.id, num, { scroll: 1, completed: true, completedAt: new Date().toISOString() }) + } + + writeFileSync(join(dataDir, 'redux-state.json'), JSON.stringify(buildReduxState(fixture(book.id)))) + + const app = await electron.launch({ + args: [REPO_ROOT], + // A temp data directory, so the packaged app never opens the reader's + // real library, and no provider key exists in it, so nothing this app + // does can reach a live provider. + env: { ...process.env, TUTOR_DATA_DIR: dataDir }, + }) + + return { app, page: await app.firstWindow(), dataDir, bookId: book.id } +} + +/** The reader's own "Chapter N" label, scoped past the chapter tab strip that repeats it. */ +function chapterLabel(page: Page, num: number) { + return page.getByRole('article').getByText(`Chapter ${num}`, { exact: true }) +} + +async function openTheSeededBook(page: Page): Promise { + await expect(page.getByRole('banner').getByRole('button', { name: 'New Book' })).toBeVisible({ timeout: BOOT_TIMEOUT_MS }) + await page.getByText(TOC_BOOK_TITLE, { exact: true }).first().click() +} + +test.describe('@electron rehydration', () => { + test('opens the reader at the position redux-state.json remembers', async () => { + const { app, page, dataDir, bookId } = await launchWithState(id => ({ + position: { bookId: id, chapter: 0, section: 0 }, + })) + + try { + await openTheSeededBook(page) + // Chapter 0 was remembered, so the reader must land there and must NOT + // take the server-derived fallback, which would be chapter 3. + await expect(chapterLabel(page, 1)).toBeVisible({ timeout: BOOT_TIMEOUT_MS }) + await expect(page.getByText(chapterMarker(1)).first()).toBeVisible() + expect(bookId).toBeTruthy() + } finally { + await app.close() + rmSync(dataDir, { recursive: true, force: true }) + } + }) + + test('falls back to the server-derived position when no saved position exists', async () => { + const { app, page, dataDir } = await launchWithState(() => ({})) + + try { + await openTheSeededBook(page) + // Three chapters are recorded as read and nothing was remembered, so + // `ReaderPage.tsx` derives the position from `chaptersRead - 1`. + await expect(chapterLabel(page, CHAPTERS_READ)).toBeVisible({ timeout: BOOT_TIMEOUT_MS }) + await expect(page.getByText(chapterMarker(CHAPTERS_READ)).first()).toBeVisible() + } finally { + await app.close() + rmSync(dataDir, { recursive: true, force: true }) + } + }) +}) diff --git a/e2e/journeys/epub-export.spec.ts b/e2e/journeys/epub-export.spec.ts new file mode 100644 index 0000000..576f4b6 --- /dev/null +++ b/e2e/journeys/epub-export.spec.ts @@ -0,0 +1,82 @@ +import { readFile } from 'node:fs/promises' +import { join } from 'node:path' +import { expect, test } from '../support/app.js' +import { library } from '../support/journeys/library.js' +import { seedBook } from '../support/seed.js' + +/** + * Journey (d): exporting a finished book to EPUB from the library. + * + * The book is seeded complete, three chapters written against a + * three-chapter table of contents, so the context menu's Export EPUB entry + * is enabled immediately. Getting a book to that state through the UI is + * the generation journeys' subject, not this one's. + * + * The export itself is deliberately not faked. Every other journey stubs its + * AI port with the scripted model, but an EPUB export makes no AI call at + * all, only markdown-to-HTML conversion and the real epub-gen-memory library + * assembling a zip. Faking that library would only prove this test calls a + * fake correctly. Running it for real is what proves the double-default + * handling in server/adapters/epub-gen-export.ts, which exists specifically + * for the Electron production build, still works end to end. + * + * The flow this journey rides: the click starts a background task (see + * createExportEpub in server/services/export-epub.ts), the task's + * `task_done` SSE event tells useBackgroundTaskEffects.ts to fetch the EPUB + * and click a hidden anchor carrying a `download` attribute, and that + * anchor click is what Playwright surfaces as a `download` event on the + * page. + */ +test('exports a finished book to EPUB from the library context menu', async ({ page, app }) => { + const book = await seedBook(app.dataDir, { generatedUpTo: 3, totalChapters: 3 }) + + await page.goto('/') + await library(page).waitForReady() + + // The card is a plain div with an onContextMenu handler and no role or + // accessible name (see card()'s doc comment in library.ts), so + // right-clicking its title is how a reader opens this menu too. + await library(page).card(book.title).click({ button: 'right' }) + + // The context menu is a plain div of buttons, not an ARIA menu, confirmed + // against test-results/*/error-context.md on a real run: "Export EPUB"'s + // accessible role is "button", never "menuitem". + const exportItem = page.getByRole('button', { name: 'Export EPUB' }) + await expect(exportItem).toBeVisible() + + // The download only fires once the background task finishes, well after + // this click resolves, but Promise.all still guards against a + // pathological fast path firing it before the listener attaches. + const [download] = await Promise.all([ + page.waitForEvent('download'), + exportItem.click(), + ]) + + expect(download.suggestedFilename()).toMatch(/\.epub$/) + + const savedPath = join(app.dataDir, 'downloaded-book.epub') + await download.saveAs(savedPath) + const savedBytes = await readFile(savedPath) + expect(savedBytes.length).toBeGreaterThan(0) + // A real EPUB is a zip archive, so its first two bytes are the local file + // header signature "PK". A stubbed or truncated export would still pass a + // bare non-empty check, so this is the one that actually proves a real + // archive came back. + expect(savedBytes.subarray(0, 2).toString('latin1')).toBe('PK') + + // createExportEpub writes book.epub to disk (artifactStore.writeEpub) + // before it ever calls backgroundTasks.succeed(), and succeed() is what + // the client waits on before it fetches the file for download, so this + // on-disk copy is guaranteed to already exist by the time the download + // above resolved. + const onDiskBytes = await readFile(join(app.bookDir(book.id), 'book.epub')) + expect(onDiskBytes.length).toBeGreaterThan(0) + + // And the background task the export ran as reached done, the same + // status the client's own auto-download waited on. + const tasksResponse = await app.fastify.inject({ method: 'GET', url: '/api/tasks' }) + expect(tasksResponse.statusCode).toBe(200) + const task = (tasksResponse.json() as Array<{ bookId: string; type: string; status: string }>) + .find(t => t.bookId === book.id && t.type === 'generate-epub') + expect(task?.status).toBe('done') +}) diff --git a/e2e/journeys/epub-import.spec.ts b/e2e/journeys/epub-import.spec.ts new file mode 100644 index 0000000..0e05cd4 --- /dev/null +++ b/e2e/journeys/epub-import.spec.ts @@ -0,0 +1,58 @@ +import { fileURLToPath } from 'node:url' +import { SAMPLE_BOOK_CHAPTERS, SAMPLE_BOOK_TITLE } from '../fixtures/sample-book.js' +import { expect, test } from '../support/app.js' +import { library } from '../support/journeys/library.js' +import { bookRepository } from '../support/seed.js' + +/** + * Journey (c): importing an EPUB previews it, then lands it in the library. + * + * This journey never touches the scripted model. Import has no AI step at + * all (see server/services/import-book.ts), so there is nothing to script; + * what it exercises instead is the real epub2 parser + * (server/adapters/epub2-import.ts) against a real file, since + * e2e/support/app.ts deliberately never overrides `epubImport`. + * sample-book.epub, built by scripts/build-e2e-epub-fixture.ts from the + * constants in ../fixtures/sample-book.ts, is that real file. + */ +const FIXTURE_PATH = fileURLToPath(new URL('../fixtures/sample-book.epub', import.meta.url)) + +test('imports an EPUB, previews it, and lands it in the library', async ({ page, app }) => { + await page.goto('/') + await library(page).waitForReady() + + // The file-chooser event needs no CSS selector. The visible control is an + // accessibly-named button whose click handler calls .click() on a hidden + // , which is what actually opens the native chooser + // Playwright intercepts here. + const [chooser] = await Promise.all([ + page.waitForEvent('filechooser'), + page.getByRole('button', { name: 'Import', exact: true }).click(), + ]) + await chooser.setFiles(FIXTURE_PATH) + + // The preview dialog, scoped by its own accessible name (wired up by its + // DialogTitle) so its "Import" confirm button can never collide with the + // toolbar's "Import" trigger, which stays mounted behind it. + const dialog = page.getByRole('dialog', { name: 'Import EPUB' }) + await expect(dialog.getByRole('heading', { name: SAMPLE_BOOK_TITLE, exact: true })).toBeVisible() + await expect(dialog.getByText(`${SAMPLE_BOOK_CHAPTERS.length} chapters`, { exact: true })).toBeVisible() + + await dialog.getByRole('button', { name: 'Import', exact: true }).click() + + // The dialog only closes once confirming has round-tripped to the server and + // back (LibraryPage's handleImportConfirm closes it after confirmEpubImport + // resolves, and leaves it open on failure), so waiting for it to disappear is + // what ties the rest of this test to the import actually finishing rather than + // to the click having merely been dispatched. It also matters for the very + // next locator: while the dialog is still open, its own preview heading is a + // second, exact-text match for library(page).card(), which would otherwise + // report success on a dialog that never actually confirmed anything. + await expect(dialog).toBeHidden() + + // Back in the library, the imported book is on screen and, independently, on disk. + await expect(library(page).card(SAMPLE_BOOK_TITLE)).toBeVisible() + + const books = await bookRepository(app.dataDir).listBooks() + expect(books.map(b => b.title)).toContain(SAMPLE_BOOK_TITLE) +}) diff --git a/e2e/journeys/generation-failure.spec.ts b/e2e/journeys/generation-failure.spec.ts new file mode 100644 index 0000000..e552549 --- /dev/null +++ b/e2e/journeys/generation-failure.spec.ts @@ -0,0 +1,69 @@ +import { + runGenerationFailureCase, + runWizardChapterOneFailureCase, + type GenerationFailureCase, +} from '../support/journeys/generation-failure.js' +import { test } from '../support/app.js' + +/** + * Journey (h) locks in that a scripted provider failure reaches the reader + * with its own message intact, not flattened into either surface's generic + * fallback. + * + * See e2e/support/journeys/generation-failure.ts for what each runner + * drives and why the wizard case is not folded into the same table. Phase 7 + * extends this suite with per-error-class cases by adding an object literal + * to CASES below. The navigation and assertions in the support module do + * not change. + * + * The reader-path cases below are `test.fixme`, not `test`, because issue + * #50 means `POST /api/books/:id/generate-next` never delivers an SSE event + * to a real browser at all, success or failure, so no assertion against + * GenerationPanel can pass yet. The wizard case is unaffected and carries + * journey (h)'s claim today. Flip `test.fixme` back to `test` once #50 + * lands. + */ + +/** + * A stand-in for whatever typed error class Phase 7's AI error taxonomy + * introduces. The point of this case is only that a subclass survives + * `throws` all the way to the screen, not this particular shape. + */ +class ScriptedProviderError extends Error { + constructor(message: string) { + super(message) + this.name = 'ScriptedProviderError' + } +} + +const CASES: GenerationFailureCase[] = [ + { + name: 'a plain error with a distinctive message', + thrown: new Error('the provider said something very specific'), + expected: 'the provider said something very specific', + }, + { + name: 'a typed error subclass', + thrown: new ScriptedProviderError('rate limited after 3 retries, provider returned HTTP 529'), + expected: /rate limited after 3 retries, provider returned HTTP 529/, + }, +] + +for (const failureCase of CASES) { + // Quarantined behind issue #50 (pipeHubToSse ends /generate-next's SSE + // reply before any event is delivered). Not run until that lands. + test.fixme(`chapter generation failure reaches the reader intact: ${failureCase.name}`, async ({ page, app, model }) => { + await runGenerationFailureCase({ page, app, model }, failureCase) + }) +} + +// Every case runs through the wizard, the typed subclass included. That case +// is the one Phase 7 cares about most, because its error taxonomy is exactly +// a set of typed subclasses, and leaving it to run only on the quarantined +// reader path would have meant this phase never actually demonstrated that a +// subclass reaches the screen with its message intact. +for (const failureCase of CASES) { + test(`chapter 1 generation failure reaches the creation wizard intact: ${failureCase.name}`, async ({ page, app, model }) => { + await runWizardChapterOneFailureCase({ page, app, model }, failureCase) + }) +} diff --git a/e2e/journeys/library-crud.spec.ts b/e2e/journeys/library-crud.spec.ts new file mode 100644 index 0000000..45d8537 --- /dev/null +++ b/e2e/journeys/library-crud.spec.ts @@ -0,0 +1,113 @@ +import { expect, test } from '../support/app.js' +import { library } from '../support/journeys/library.js' +import { bookRepository, readBook, seedBook } from '../support/seed.js' + +/** + * Journey (e): library CRUD and search. + * + * Every mutation here asserts twice, once on what the reader sees and once + * on the YAML the fixture wrote. The two can drift apart in either + * direction: a card can show a change that a failed request never actually + * persisted, or the server can write a change the grid fails to re-render + * because `fetchBooks()` didn't run. Checking only the screen would miss the + * first, checking only disk would miss the second, so both are asserted for + * every operation below. + * + * Split into one `test` per operation rather than one long scenario, so a + * failure names the operation that broke rather than an assertion three + * steps into an unrelated one. + */ + +test('rename persists across reload', async ({ page, app }) => { + const book = await seedBook(app.dataDir, { title: 'Rename Me' }) + + await page.goto('/') + await library(page).waitForReady() + + await library(page).card('Rename Me').click({ button: 'right' }) + await page.getByRole('button', { name: 'Rename' }).click() + + const dialog = page.getByRole('dialog', { name: 'Rename Book' }) + // The Title field's