From 2fcc990a3d689c4642d2f1bd07f0cc5e8329b7f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C4=ABlav=C4=81pi=20Cheesley?= Date: Tue, 22 Sep 2026 19:19:26 +0100 Subject: [PATCH] Wire up repair drills, and let the drill surface be heard repeating itself MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three loose ends, all in the same file, so one change. Repair drills existed as a module with nothing calling them. The result card already worked out which keys slipped; it now offers a drill built from exactly those and builds it on the lesson they slipped in. The offer is made only when something is wired up to honour it, the same rule advancing follows, because a button naming a thing it will not do is worse than one offering another go. A repair carries the lesson's id so its keys feed the same statistics, and no next lesson, because a repair is not a rung and nobody should climb the ladder by fumbling. generateRepairText throws on a key the lesson cannot type; that throw is surfaced, never turned back into an ordinary drill. The drill surface now announces through announceInto. A live region speaks only when its contents change, so mistyping the same key twice in a row was announced once — and the drill surface is where that happens. Two tidies the sprint work could not reach from inside its own file list: sprint.ts is exported from the drill barrel like the rest of its layer, and the sprint text helpers move into tests/e2e/helpers.ts instead of being duplicated in two spec files. Fills in the repair-drill journey, which needed the wiring to exist. That leaves every one of the eight shipped regressions asserted and one journey outstanding: whether a screen reader announces the right things, which no test can answer. Verified by hand as well as by test: a lesson fumbled on "t" offered a repair, and the repair read "fast nta at tan data idti sit ditst fast fast" — every character inside the lesson's key set, "t" eleven times, never doubled. A timed sprint abandoned mid-drill left the following lesson able to run all sixty characters to three stars, which is the failure this project started with. Co-Authored-By: Claude Opus 5 --- src/drill/index.ts | 1 + src/ui/drill-view.ts | 35 ++++++++++++++++-- src/ui/load-form.ts | 54 +++++++++++++++++++++++++++ tests/e2e/helpers.ts | 18 +++++++++ tests/e2e/pending.spec.ts | 77 ++++++++++++++++++++++++++++----------- tests/e2e/sprint.spec.ts | 17 +-------- 6 files changed, 163 insertions(+), 39 deletions(-) diff --git a/src/drill/index.ts b/src/drill/index.ts index 6f3dd99..cac2f4b 100644 --- a/src/drill/index.ts +++ b/src/drill/index.ts @@ -9,4 +9,5 @@ export * from './limits.js'; export * from './types.js'; export * from './engine.js'; export * from './text.js'; +export * from './sprint.js'; export * from './repair.js'; diff --git a/src/ui/drill-view.ts b/src/ui/drill-view.ts index c29973e..bcc83bb 100644 --- a/src/ui/drill-view.ts +++ b/src/ui/drill-view.ts @@ -56,7 +56,7 @@ import { } from '../drill/sprint.js'; import type { Keymap } from '../keymap/types.js'; import { keyStatId } from '../stats/storage.js'; -import { required } from './dom.js'; +import { announceInto, required } from './dom.js'; /** * A sample drill, and the seam where generated text will arrive. @@ -147,6 +147,12 @@ export interface DrillViewOptions { * not pass this, and the button offers another go at this lesson instead. */ readonly onAdvance?: () => void; + /** + * Called when the learner takes up an offer to drill the keys that slipped, + * with exactly the keys the scoring picked out. Like `onAdvance`, the offer is + * only made when there is something wired up to deliver it. + */ + readonly onRepair?: (weakKeys: readonly string[]) => void; /** * Called whenever the next key changes, and with null when there is none. The * board highlight is wired up through this, so that reinforcement is the @@ -343,8 +349,11 @@ export function createDrillView(options: DrillViewOptions): DrillView { const lessonName = options.lessonName ?? null; const nextLessonName = options.nextLessonName ?? null; const onAdvance = options.onAdvance ?? null; + const onRepair = options.onRepair ?? null; /** Whether the result on screen earned a move to the next lesson. */ let advanceEarned = false; + /** The keys the result on screen offers to repair, empty when it offers none. */ + let repairOffered: readonly string[] = []; const limitMs = options.limitMs ?? NO_LIMIT; const session = options.session ?? createDrillSession(options.now === undefined ? {} : { now: options.now }); @@ -430,10 +439,14 @@ export function createDrillView(options: DrillViewOptions): DrillView { el.resultWhy.textContent = ''; el.continue.textContent = ''; advanceEarned = false; + repairOffered = []; } function announce(message: string): void { - el.progress.textContent = message; + // Through the shared helper, because a live region only speaks when its + // contents change: mistyping the same key twice in a row produces the same + // sentence twice, and the second one was silent. + announceInto(el.progress, message); } function setCaptureState(message: string): void { @@ -668,7 +681,17 @@ export function createDrillView(options: DrillViewOptions): DrillView { // it. A button that names a rung and then restarts this one is worse than a // plain "go again". advanceEarned = score.nextStep.advances && onAdvance !== null; - el.continue.textContent = advanceEarned ? score.nextStep.label : 'Drill this lesson again'; + // A repair is offered only when scoring picked keys out AND something is + // wired up to build one. Same rule as advancing: never name a thing on a + // button that the button will not do. + repairOffered = + !advanceEarned && onRepair !== null && score.nextStep.repairKeys.length > 0 + ? score.nextStep.repairKeys + : []; + el.continue.textContent = + advanceEarned || repairOffered.length > 0 + ? score.nextStep.label + : `Drill this ${mode === 'lesson' ? 'lesson' : mode} again`; el.result.hidden = false; // Unhidden first, then written, so the assertive region announces the result @@ -860,6 +883,12 @@ export function createDrillView(options: DrillViewOptions): DrillView { onAdvance(); return; } + if (repairOffered.length > 0 && onRepair !== null) { + const keys = repairOffered; + repairOffered = []; + onRepair(keys); + return; + } startDrill(); } diff --git a/src/ui/load-form.ts b/src/ui/load-form.ts index 9676421..52829a5 100644 --- a/src/ui/load-form.ts +++ b/src/ui/load-form.ts @@ -30,6 +30,7 @@ import { import { describeFailure, required } from './dom.js'; import { createDrillView, SAMPLE_DRILL_TEXT, type DrillView } from './drill-view.js'; import { DrillTextError, generateDrillText } from '../drill/text.js'; +import { generateRepairText, RepairDrillError } from '../drill/repair.js'; import { highestUnlockedLesson } from '../drill/scoring.js'; import { SprintError, sprintLesson, sprintWordCount } from '../drill/sprint.js'; import { summariseKeymap } from './layout-summary.js'; @@ -226,6 +227,30 @@ export function wireUp(root: ParentNode = document, options: WireUpOptions = {}) // Named only when there is somewhere to advance to, so the result card // never offers a rung that does not exist. nextLessonName: nextLesson?.name ?? null, + // Taking up the offer to repair rebuilds the drill from exactly the keys + // that slipped. The keys come from scoring, which took them from this + // lesson's own statistics, so they are typeable here by construction -- + // but generateRepairText throws if they are not, and that throw is + // surfaced rather than caught and turned back into an ordinary drill. A + // repair drill that quietly practises the wrong keys is the whole thing + // this feature exists to avoid. + ...(lesson === null + ? {} + : { + onRepair: (weakKeys: readonly string[]): void => { + let text: string; + try { + text = generateRepairText(lesson, weakKeys, { seed: seed() }); + } catch (cause) { + if (cause instanceof RepairDrillError) { + showError(`Could not build a repair drill for ${lesson.name}: ${cause.message}`); + return; + } + throw cause; + } + showRepair(keymap, lesson, text); + }, + }), // And the offer is real: taking it rebuilds the drill on the next lesson // and remembers where the learner has got to. ...(nextLesson === null @@ -323,6 +348,35 @@ export function wireUp(root: ParentNode = document, options: WireUpOptions = {}) * `showError` hides every section, which is right for a layout that would not * parse and wrong here: the lesson on screen is still perfectly good. */ + /** + * Replaces the drill surface with a repair drill built from the keys that + * slipped, on the lesson they slipped in. + * + * It carries the lesson's id, so the keys it practises go back into the same + * per-key statistics the weak-key report reads. It carries no next lesson: a + * repair is not a rung and must never look like one, or a learner could climb + * the ladder by fumbling. + */ + function showRepair(keymap: Keymap, lesson: Lesson, text: string): void { + clearError(); + drillView?.destroy(); + drillView = createDrillView({ + board: GLOVE80, + keymap, + text, + session, + mode: 'repair', + lessonId: lesson.id, + lessonName: lesson.name, + nextLessonName: null, + root, + onNextKey: followNextKey, + onScored: saveAndRefresh, + }); + drillSection.hidden = false; + drillStart.focus(); + } + function showSprintError(message: string): void { error.textContent = message; error.hidden = false; diff --git a/tests/e2e/helpers.ts b/tests/e2e/helpers.ts index 374f885..ad86365 100644 --- a/tests/e2e/helpers.ts +++ b/tests/e2e/helpers.ts @@ -4,6 +4,7 @@ import AxeBuilder from '@axe-core/playwright'; import { expect, type Page } from '@playwright/test'; import { GLOVE80 } from '../../src/board/index.js'; import { generateDrillText } from '../../src/drill/text.js'; +import { sprintLesson, sprintWordCount } from '../../src/drill/sprint.js'; import { parseMoErgoLayoutText } from '../../src/keymap/moergo.js'; import { generateLadder, type Lesson } from '../../src/ladder/index.js'; import { describeKey, indexKeys } from '../../src/board/types.js'; @@ -141,3 +142,20 @@ export function expectedKeyHand(character: string): string { } return key.hand; } + +/** + * The text a sprint will drill, worked out the same way the app works it out. + * + * With no saved progress only the first rung is unlocked, so that is what a + * sprint runs across. + */ +export function expectedSprintText(limitMs: number): string { + return generateDrillText(sprintLesson(referenceLadder(), 0), { + seed: DRILL_SEED, + words: sprintWordCount(limitMs), + }); +} + +export function sprintPrefix(count: number, limitMs: number): string { + return [...expectedSprintText(limitMs)].slice(0, count).join(''); +} diff --git a/tests/e2e/pending.spec.ts b/tests/e2e/pending.spec.ts index b140ea9..5213e5f 100644 --- a/tests/e2e/pending.spec.ts +++ b/tests/e2e/pending.spec.ts @@ -3,36 +3,20 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { expect, test } from '@playwright/test'; import { - DRILL_SEED, REFERENCE_LAYOUT_PATH, drillPrefix, + drillCharAt, + wrongKeyFor, expectedDrillText, expectedKeyDescription, gotoApp, loadReferenceLayout, referenceLadder, + expectedSprintText, + sprintPrefix, } from './helpers.js'; -import { sprintLesson, sprintWordCount } from '../../src/drill/sprint.js'; -import { generateDrillText } from '../../src/drill/text.js'; import type { Page } from '@playwright/test'; -/** - * The text a sprint will drill, worked out the way the app works it out: the - * pinned seed, and the whole key set unlocked so far. With no saved progress - * that is the first rung, so this is the ladder's first lesson's keys rather - * than its text. - */ -function expectedSprintText(limitMs: number): string { - return generateDrillText(sprintLesson(referenceLadder(), 0), { - seed: DRILL_SEED, - words: sprintWordCount(limitMs), - }); -} - -function sprintPrefix(count: number, limitMs: number): string { - return [...expectedSprintText(limitMs)].slice(0, count).join(''); -} - /** * Tab forward until the element with this id has focus, returning every stop on * the way so a failure says where focus actually went. Out here rather than in a @@ -131,7 +115,58 @@ test.describe('the trainer journeys', () => { await expect(page.locator('#drill-text')).not.toHaveText(expectedDrillText(0)); }); - test.fixme('builds a repair drill from exactly the keys that were missed', () => {}); + test('builds a repair drill from exactly the keys that were missed', async ({ page }) => { + await gotoApp(page); + await loadReferenceLayout(page); + await page.getByRole('button', { name: 'Start drill' }).click(); + + // Fumble the third key repeatedly, then type the rest cleanly. Enough + // mistakes to stay under two stars, so the result offers a repair rather + // than the next rung -- a learner must not climb the ladder by fumbling. + const fumbled = drillCharAt(2); + await page.keyboard.type(drillPrefix(2), { delay: 15 }); + for (let attempt = 0; attempt < 5; attempt += 1) { + await page.keyboard.press(wrongKeyFor(fumbled)); + await page.keyboard.press('Backspace'); + } + await page.keyboard.type(expectedDrillText().slice(2), { delay: 15 }); + await expect(page.locator('#drill-result')).toBeVisible(); + + // The offer names the key that slipped. The card presents it upper case, so + // the comparison ignores case rather than assuming which one it picked. + const offer = page.locator('#drill-continue'); + await expect(offer).toContainText(new RegExp(`repair`, 'i')); + await expect(offer).toContainText( + new RegExp(fumbled.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'), 'i'), + ); + + await offer.click(); + + // The repair drill is built from exactly that key, plus anchors it can + // type. Every character in it must be one the lesson has unlocked. + const drillText = page.locator('#drill-text'); + await expect(drillText).toContainText(fumbled); + await expect(drillText).not.toHaveText(expectedDrillText()); + + // Read back for the set check below, which needs the characters themselves + // rather than a match against them. + const repair = await drillText.innerText(); + + const lesson = referenceLadder()[0]; + expect(lesson).toBeDefined(); + const allowed = new Set([...lesson!.keys, ' ']); + for (const character of repair) { + expect(allowed.has(character), `repair drill used ${JSON.stringify(character)}`).toBe(true); + } + + // And it is typeable: the surface accepts the first key of it. + await page + .getByRole('button', { name: /Start|Resume/ }) + .first() + .click(); + await page.keyboard.press([...repair][0] ?? 'a'); + await expect(page.locator('#drill-text .drill-char[data-mark="correct"]')).toHaveCount(1); + }); test('runs a sprint to its limit, and offers an untimed option', async ({ page }) => { // The page's own clock is faked, so a thirty second sprint costs no wall diff --git a/tests/e2e/sprint.spec.ts b/tests/e2e/sprint.spec.ts index faa311f..53052fc 100644 --- a/tests/e2e/sprint.spec.ts +++ b/tests/e2e/sprint.spec.ts @@ -1,14 +1,13 @@ import { expect, test, type Page } from '@playwright/test'; import { NO_LIMIT, SPRINT_DURATIONS_MS } from '../../src/drill/limits.js'; -import { sprintLesson, sprintWordCount } from '../../src/drill/sprint.js'; -import { generateDrillText } from '../../src/drill/text.js'; import { - DRILL_SEED, expectedDrillText, expectNoAxeViolations, gotoApp, loadReferenceLayout, referenceLadder, + expectedSprintText, + sprintPrefix, } from './helpers.js'; /** @@ -23,18 +22,6 @@ import { * the generator fails these rather than being silently agreed with. */ -/** With no saved progress only the first rung is unlocked, so that is the sprint. */ -function expectedSprintText(limitMs: number): string { - return generateDrillText(sprintLesson(referenceLadder(), 0), { - seed: DRILL_SEED, - words: sprintWordCount(limitMs), - }); -} - -function sprintPrefix(count: number, limitMs: number): string { - return [...expectedSprintText(limitMs)].slice(0, count).join(''); -} - /** Choose a duration and start a sprint with it. */ async function startSprint(page: Page, limitMs: number): Promise { await page.locator('#sprint-duration').selectOption(String(limitMs));