From c1cd52573758c593dea6ebd9675b33eab0ad669b Mon Sep 17 00:00:00 2001 From: Jonathan Bell Date: Wed, 12 Aug 2026 16:29:06 +0000 Subject: [PATCH] Guided tutorial: make the spotlit layer visible, and separate forcing from ringing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Steps 7 and 8 both need layer 20 on screen in each heatmap. Step 8 asked for it and appeared not to get it; step 7 never asked at all. Neither was the mechanism failing — the emission side is sound, `setTarget` takes arrays, and against the real Llama-3.1-8B tokenization position 5 is " Tower" / "um" as the content claims. The layer WAS being forced into the grid. It was then clipped out of view. Auto-fit sizes its column budget as if no spotlight existed, and the widget renders the spotlit layer on top of that set — so with the tutorial docked at 1366x768 each grid gets a one-column budget, shows [0, 31], and the forced layer 20 lands past the scroll container's right edge. Measured at 69% visible, and 0% at a 420px panel. That is width-dependent, which is why it read as intermittent and why an 80-layer harness (f3d193a) showed it passing. The widget half is edulogitlens 82a7327, pinned here: auto-fit now budgets in rendered columns, and a grid scrolls its own container to bring a spotlit column into view. Ring visibility goes to 1.00 at every tested width. Three changes on this side: - `forceLayers` on a unit. A spotlight does two jobs — it rings a cell and it keeps auto-fit from downsampling that layer away — and step 7's task is to FIND the landmark row, so it needs the second job without the first. Ringing the cells on arrival would do the step's work for the participant. u4a-compare now forces layer 20 in both grids and still rings nothing until its stage-2 hint; the assertion that it has no unit `spotlights` stays true on purpose. - One derived spotlight payload instead of three effects overwriting each other. The hint-reveal write was imperative and nothing re-applied it, so any remount — a reload, or collapsing and re-expanding the dock, which unmounts the column the panel portals into — dropped the rings while the hint still read as revealed. The revealed stage is persisted, so `resolveUnitSpotlights` derives the whole payload from it and is unit-tested directly. - The arrival effect had no `active` guard, so closing the tutorial on a patch step re-lit its rings on a closed panel. Validation stays looser than the code, so `position` is now optional there too and `forceLayers` rejects an entry carrying one — a dropped position would have the author waiting for a ring that never comes. Verified: 194 tests pass; tsc (34 errors) and lint (35/25) unchanged from main. Ring visibility confirmed in Chromium against the real widget at 32 and 80 layers across 420/528/538/801/809px panels. Known, not addressed: once a patch lands, three grids share the vertical space and the source/target rings fall below their fold — pre-existing, and the result ring is now fully visible where it was 0%. Co-Authored-By: Claude Opus 5 (1M context) --- workbench/_web/bun.lock | 4 +- workbench/_web/package.json | 2 +- .../tutorial/TutorialActivityPanel.tsx | 90 +++++++-------- .../_web/src/db/__tests__/tutorials.test.ts | 45 ++++++++ .../_web/src/lib/queries/tutorialContentDb.ts | 25 ++++- .../tutorials/__tests__/prolificSeed.test.ts | 16 +++ workbench/_web/src/tutorials/prolificSeed.ts | 22 ++++ .../types/__tests__/unitSpotlights.test.ts | 106 ++++++++++++++++++ workbench/_web/src/types/tutorial-content.ts | 87 +++++++++++++- 9 files changed, 343 insertions(+), 54 deletions(-) create mode 100644 workbench/_web/src/types/__tests__/unitSpotlights.test.ts diff --git a/workbench/_web/bun.lock b/workbench/_web/bun.lock index d5089f99..ec97becd 100644 --- a/workbench/_web/bun.lock +++ b/workbench/_web/bun.lock @@ -35,7 +35,7 @@ "d3-delaunay": "^6.0.4", "dotenv": "^17.2.1", "drizzle-orm": "^0.44.4", - "edulogitlens": "github:jon-bell/edulogitlens#fa59004", + "edulogitlens": "github:jon-bell/edulogitlens#82a7327", "framer-motion": "^12.23.22", "html-to-image": "^1.11.13", "lexical": "^0.34.0", @@ -1135,7 +1135,7 @@ "easy-table": ["easy-table@1.1.0", "", { "optionalDependencies": { "wcwidth": ">=1.0.1" } }, "sha512-oq33hWOSSnl2Hoh00tZWaIPi1ievrD9aFG82/IgjlycAnW9hHx5PkJiXpxPsgEE+H7BsbVQXFVFST8TEXS6/pA=="], - "edulogitlens": ["edulogitlens@github:jon-bell/edulogitlens#fa59004", { "dependencies": { "lucide-react": "^0.487.0", "motion": "^12.23.24", "react-dnd": "^16.0.1", "react-dnd-html5-backend": "^16.0.1" }, "peerDependencies": { "react": "^18.0.0 || ^19.0.0", "react-dom": "^18.0.0 || ^19.0.0" } }, "jon-bell-edulogitlens-fa59004", "sha512-Y2h8HImzVd3YtSWE5CkpKuO5Bt3axSSbnRwZrb7LWes6OxNk2NgeSwA/eKxHw7eiP/WpP0cftJEdqf8ByLKLXA=="], + "edulogitlens": ["edulogitlens@github:jon-bell/edulogitlens#82a7327", { "dependencies": { "lucide-react": "^0.487.0", "motion": "^12.23.24", "react-dnd": "^16.0.1", "react-dnd-html5-backend": "^16.0.1" }, "peerDependencies": { "react": "^18.0.0 || ^19.0.0", "react-dom": "^18.0.0 || ^19.0.0" } }, "jon-bell-edulogitlens-82a7327", "sha512-3+2Y/HB5QcOcSOhjYZNtCneuQurbR3z8O1MjzUKb2Jk/i7YmG3iTSOgXjGzo6KopKaJlC0IWESY+uxVyCMCnnA=="], "electron-to-chromium": ["electron-to-chromium@1.5.200", "", {}, "sha512-rFCxROw7aOe4uPTfIAx+rXv9cEcGx+buAF4npnhtTqCJk5KDFRnh3+KYj7rdVh6lsFt5/aPs+Irj9rZ33WMA7w=="], diff --git a/workbench/_web/package.json b/workbench/_web/package.json index b0242c45..28ffc17e 100644 --- a/workbench/_web/package.json +++ b/workbench/_web/package.json @@ -46,7 +46,7 @@ "d3-delaunay": "^6.0.4", "dotenv": "^17.2.1", "drizzle-orm": "^0.44.4", - "edulogitlens": "github:jon-bell/edulogitlens#fa59004", + "edulogitlens": "github:jon-bell/edulogitlens#82a7327", "framer-motion": "^12.23.22", "html-to-image": "^1.11.13", "lexical": "^0.34.0", diff --git a/workbench/_web/src/app/workbench/[workspaceId]/patch-lens/[chartId]/components/tutorial/TutorialActivityPanel.tsx b/workbench/_web/src/app/workbench/[workspaceId]/patch-lens/[chartId]/components/tutorial/TutorialActivityPanel.tsx index ba0424ba..c87ad296 100644 --- a/workbench/_web/src/app/workbench/[workspaceId]/patch-lens/[chartId]/components/tutorial/TutorialActivityPanel.tsx +++ b/workbench/_web/src/app/workbench/[workspaceId]/patch-lens/[chartId]/components/tutorial/TutorialActivityPanel.tsx @@ -1,6 +1,6 @@ "use client"; -import { useEffect, useId, useRef, useState } from "react"; +import { useEffect, useId, useMemo, useRef, useState } from "react"; import { createPortal } from "react-dom"; import { motion, useDragControls } from "motion/react"; import { @@ -22,7 +22,7 @@ import { Input } from "@/components/ui/input"; import { useCapture } from "@/lib/analytics"; import { useProlificTutorial, HINT_AUTO_OFFER_AT } from "@/stores/useProlificTutorial"; import type { GlossaryEntry, HintRung, SpotlightTarget, UnitCheck } from "@/types/tutorial-content"; -import { resolveCheckKey } from "@/types/tutorial-content"; +import { resolveCheckKey, resolveUnitSpotlights } from "@/types/tutorial-content"; import { DEFAULT_GLOSSARY } from "@/tutorials/glossary"; import { CompletionCta } from "./CompletionCta"; import { TutorialGlossary } from "./TutorialGlossary"; @@ -141,56 +141,49 @@ export function TutorialActivityPanel({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [runNonce]); - // Read before the mount/active guard below, so the patch-result effect can + // Read before the mount/active guard below, so the spotlight derivation can // depend on them (a conditional hook isn't an option). const isPatchUnit = unit?.progression.on === "patch"; const patchToken = store.patchTokenByUnit[store.unitIdx] ?? null; + const hintStage = store.hintStageByUnit[store.unitIdx] ?? 0; - // On arriving at a unit, ring the cells that unit asks about — and clear - // whatever the previous unit lit. Declared before the patch-result effect so - // that on arriving back at a patched step, this runs and that one re-lights, - // in the same commit. + // Everything this step spotlights, in one derived value: the layers it forces + // on screen, the cells it rings on arrival, the cells of whatever hint rung has + // been revealed, and — once a patch is filed — the result cell. // - // Unit-level spotlights are not a nicety on the patch step. Its task says - // "drag this ringed cell onto that one", which was a lie while spotlights only - // fired on a revealed hint; and the ring is also what forces the widget to - // render that layer at all, since auto-fit downsamples layers to the column - // width and a narrow display can drop the layer the step is about. - const unitSpotlights = unit?.spotlights; - const spotlitPatch = useRef(null); - useEffect(() => { - spotlitPatch.current = null; - onSpotlight?.(unitSpotlights?.length ? unitSpotlights : null); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [store.unitIdx, store.active, unitSpotlights]); - - // Point at the target's post-patch output the moment a patch lands. On the - // step carrying the whole point of the tool, participants performed the - // intervention successfully and then could not find its result — the panel - // now says what changed (below) and rings the cell it changed in. + // This used to be three effects writing the same channel, each overwriting the + // others. The one that hurt was the hint reveal: it was imperative, so nothing + // re-applied it, and any remount (a reload, or collapsing and re-expanding the + // dock, which unmounts the column the panel portals into) dropped the rings + // while the hint on screen still read as revealed. The revealed stage is + // persisted, so the rings can be derived from it instead of remembered. // - // The result cell is ADDED to the step's own spotlights, never substituted for - // them. `patchToken` is not evidence that a result is on screen: a patch - // restored from an earlier session is re-filed on arrival at this step - // (PatchLensDisplay's "restored patch" effect) even when no result grid is - // rendered. Replacing here meant that token silently deleted the two cells the - // step's task names — leaving the drag it asks for pointed at nothing, and - // (because a spotlight is also what forces a downsampled layer to render) the - // patch layer missing from the grid entirely. Lighting both is safe: an - // unrendered result grid resolves to no cell, so the extra target is inert. + // Two things the effects taught us, kept here: + // - Ringing a cell is also what forces the widget to render its layer — + // auto-fit downsamples layers to the column width, so the layer a step is + // about can be missing from a narrow grid. `forceLayers` does that job + // alone, for a step that must not ring anything (see resolveUnitSpotlights). + // - The patch result is ADDED to the step's own cells, never substituted for + // them (commit f3d193a). `patchToken` is not evidence a result grid is on + // screen: a patch restored from an earlier session is re-filed on arrival + // (PatchLensDisplay's "restored patch" effect) with nothing rendered, and + // substituting there deleted the two cells the step's task names — and with + // them the patch layer itself. Lighting both is safe: an unrendered result + // grid resolves to no cell, so the extra target is inert. + // + // Nothing is spotlit while the tutorial is off screen. The guard lives here + // rather than being inherited from the render, because these hooks sit above + // the `active` early-return (hooks can't be conditional). + const spotlights = useMemo( + () => (store.active ? resolveUnitSpotlights(unit, hintStage, patchToken != null) : null), + [store.active, unit, hintStage, patchToken], + ); useEffect(() => { - // Nothing is spotlit while the tutorial is off screen: these effects sit - // above the `active` guard (hooks can't be conditional), so the invariant - // has to be stated here rather than inherited from the render. - if (!store.active || !isPatchUnit || patchToken == null) return; - if (spotlitPatch.current === patchToken) return; - spotlitPatch.current = patchToken; - onSpotlight?.([ - ...(unitSpotlights ?? []), - { grid: "result", layer: "last", position: "last" }, - ]); + onSpotlight?.(spotlights); + // onSpotlight is a prop the host redeclares every render; re-pushing on the + // payload alone is what keeps this from thrashing the widget. // eslint-disable-next-line react-hooks/exhaustive-deps - }, [store.active, isPatchUnit, patchToken, unitSpotlights]); + }, [spotlights]); // Back to the top of the step on arrival. The steps are long enough to scroll, // and the container keeps its offset across a unit change — so advancing from @@ -228,7 +221,6 @@ export function TutorialActivityPanel({ const total = units.length; const attempts = store.attemptsByUnit[store.unitIdx] ?? 0; - const hintStage = store.hintStageByUnit[store.unitIdx] ?? 0; const completed = store.completedUnits.includes(store.unitIdx); const isLast = store.unitIdx === total - 1; @@ -348,10 +340,10 @@ export function TutorialActivityPanel({ const stage = store.revealHint(); const rung = unit.hints.find((h) => h.stage === stage); if (rung?.insertPrompt) onInsertPrompt(rung.insertPrompt); - // A rung may light several cells — both ends of a - // drag, say. `spotlights` wins over `spotlight`. - const cells = rung?.spotlights?.length ? rung.spotlights : rung?.spotlight; - if (cells) onSpotlight?.(cells); + // The rung's cells are NOT lit from here. `revealHint` persists + // the stage, and the spotlight payload above is derived from it + // — so the rings survive a reload and a dock collapse, which an + // imperative call from this handler did not. }} /> diff --git a/workbench/_web/src/db/__tests__/tutorials.test.ts b/workbench/_web/src/db/__tests__/tutorials.test.ts index dd8eb621..92e6569f 100644 --- a/workbench/_web/src/db/__tests__/tutorials.test.ts +++ b/workbench/_web/src/db/__tests__/tutorials.test.ts @@ -194,6 +194,8 @@ describe("tutorial content", () => { kind: "patch", progression: { on: "patch" }, patchPair: { source: "The Eiffel Tower is in", target: "The Colosseum is in" }, + spotlights: [{ grid: "source", layer: 20, position: 5 }], + forceLayers: [{ grid: "target", layer: 20 }], answerPlaceholder: "e.g. Paris", observationPlaceholder: "What changed?", faqs: [{ q: "What is a patch?", a: "Copying one cell into the other prompt." }], @@ -220,6 +222,49 @@ describe("tutorial content", () => { expect(() => validateTutorialContent(content)).not.toThrow(); }); + // A spotlight the widget can't resolve silently highlights nothing — for a hint, + // exactly the rung a stuck participant reached for. `forceLayers` fails more + // quietly still: the step goes back to being about a column that auto-fit has + // dropped, with nothing on screen to say so. + it("rejects malformed spotlights and forceLayers, and accepts a position-less one", () => { + const base = tinyContent().units[0]; + const withUnit = (overrides: Partial) => + validateTutorialContent({ version: 1, units: [{ ...base, ...overrides }] }); + + expect(() => withUnit({ spotlights: [] })).toThrow(); + expect(() => + withUnit({ spotlights: [{ grid: "middle", layer: 1, position: 1 }] as never }), + ).toThrow(); + expect(() => + withUnit({ spotlights: [{ grid: "source", layer: -1, position: 1 }] }), + ).toThrow(); + // Present-but-unresolvable position still fails; absent is now legal (it + // renders the layer and rings nothing). + expect(() => + withUnit({ spotlights: [{ grid: "source", layer: 1, position: 1.5 }] }), + ).toThrow(); + expect(() => withUnit({ spotlights: [{ grid: "source", layer: 20 }] })).not.toThrow(); + + expect(() => withUnit({ forceLayers: [] })).toThrow(); + expect(() => + withUnit({ forceLayers: [{ grid: "result", layer: "middle" }] as never }), + ).toThrow(); + expect(() => withUnit({ forceLayers: [{ grid: "nope", layer: 20 }] as never })).toThrow(); + // A position here would be dropped on the way to the widget, so an author + // who wrote one is waiting for a ring that never comes. + expect(() => + withUnit({ forceLayers: [{ grid: "source", layer: 20, position: 5 }] as never }), + ).toThrow(); + expect(() => + withUnit({ + forceLayers: [ + { grid: "source", layer: 20 }, + { grid: "target", layer: "last" }, + ], + }), + ).not.toThrow(); + }); + // The welcome slideshow is modal and it is the first thing a participant sees, // so a slide that renders blank blocks the tutorial behind it rather than // degrading into something they can work around. diff --git a/workbench/_web/src/lib/queries/tutorialContentDb.ts b/workbench/_web/src/lib/queries/tutorialContentDb.ts index 91d2d767..69501973 100644 --- a/workbench/_web/src/lib/queries/tutorialContentDb.ts +++ b/workbench/_web/src/lib/queries/tutorialContentDb.ts @@ -117,12 +117,17 @@ export const validateTutorialContent = (content: TutorialContent): TutorialConte // A spotlight the widget can't resolve silently highlights nothing — for a // hint, exactly the rung a stuck participant reached for; for a unit, the // cells its instructions tell them to drag between. + // + // `position` is optional: an entry with a grid and a layer but no position + // forces that layer's column to render and rings nothing (how `forceLayers` + // reaches the widget). A position that IS present still has to resolve, or + // it rings a cell nobody meant. const checkSpotlights = (cells: unknown[], where: string) => { for (const s of cells as { grid?: unknown; layer?: unknown; position?: unknown }[]) { if ( !validGrids.has(s?.grid) || !isCellIndex(s?.layer) || - !isCellIndex(s?.position) + (s?.position !== undefined && !isCellIndex(s?.position)) ) { throw new Error( `Unit "${u.id}" ${where} has a malformed spotlight (needs grid source|target|result and a non-negative integer or "last" layer/position)`, @@ -136,6 +141,24 @@ export const validateTutorialContent = (content: TutorialContent): TutorialConte } checkSpotlights(u.spotlights, "spotlights"); } + // Same shape minus the position, and the same failure when it's wrong: a + // layer the widget can't resolve is silently dropped, and the step that + // asked for it goes back to being about a column that isn't on screen. + if (u.forceLayers !== undefined) { + if (!Array.isArray(u.forceLayers) || u.forceLayers.length === 0) { + throw new Error(`Unit "${u.id}" forceLayers must be a non-empty array`); + } + checkSpotlights(u.forceLayers, "forceLayers"); + // A position here is dropped on the way to the widget, so an author who + // wrote one is expecting a ring they will never get. Say so instead. + for (const f of u.forceLayers as { position?: unknown }[]) { + if (f?.position !== undefined) { + throw new Error( + `Unit "${u.id}" forceLayers entries take no position (they render a layer without ringing a cell) — use spotlights to ring one`, + ); + } + } + } for (const h of u.hints) { if (typeof h?.stage !== "number" || typeof h?.text !== "string") { throw new Error(`Unit "${u.id}" has a malformed hint rung`); diff --git a/workbench/_web/src/tutorials/__tests__/prolificSeed.test.ts b/workbench/_web/src/tutorials/__tests__/prolificSeed.test.ts index 2da50cfb..cc5d3876 100644 --- a/workbench/_web/src/tutorials/__tests__/prolificSeed.test.ts +++ b/workbench/_web/src/tutorials/__tests__/prolificSeed.test.ts @@ -123,6 +123,22 @@ describe("prolific tutorial seed", () => { expect(unit("u4a-compare").spotlights).toBeUndefined(); }); + // The two halves of what a spotlight does, split. The compare step needs layer + // 20 rendered — auto-fit drops it from two heatmaps in one column, and then the + // column the next step drags across only appears once a hint rings it — but it + // must not ring anything, because finding those rows is the step's own task. + it("the compare step shows the patch layer without ringing its cells", () => { + const compare = unit("u4a-compare"); + const forced = compare.forceLayers ?? []; + expect(forced.map((f) => f.grid).sort()).toEqual(["source", "target"]); + // Position-less, or it would ring a cell like any other spotlight. + expect(forced.every((f) => !("position" in f))).toBe(true); + // The same layer the next step drags across, so the participant has already + // looked at the column they are about to patch. + const patched = unit("u4-patching").spotlights ?? []; + expect(forced.map((f) => f.layer)).toEqual(patched.map((c) => c.layer)); + }); + // Every hint that names a cell in prose also rings it. A hint that has to give // coordinates ("the 'um' cell at the end of 'Colosseum'") is a hint about a // missing affordance, and the drag is the one interaction prose can't convey. diff --git a/workbench/_web/src/tutorials/prolificSeed.ts b/workbench/_web/src/tutorials/prolificSeed.ts index 851cde57..8b1d6426 100644 --- a/workbench/_web/src/tutorials/prolificSeed.ts +++ b/workbench/_web/src/tutorials/prolificSeed.ts @@ -45,6 +45,13 @@ import type { TutorialContent } from "@/types/tutorial-content"; * ("the 'um' cell at the end of 'Colosseum'") is a sign the affordance isn't * discoverable. Verified against the Llama-3.1 tokenizer: with BOS at index 0, * position 5 is " Tower" in the source and "um" in the target. + * - **Showing a layer and ringing a cell are separated.** A spotlight does both + * at once — it rings the cell, and it keeps the widget from downsampling that + * layer away — which is right for the patch step (`spotlights`, both ends of + * the drag) and wrong for the compare step before it, whose task is to find + * those same rows. Compare uses `forceLayers` for the column and leaves the + * rings to its stage-2 hint, so layer 20 is on screen from arrival but nothing + * answers the question the step is asking. */ export const PROLIFIC_TUTORIAL_SLUG = "prolific-patch-lens-demo"; @@ -64,6 +71,14 @@ const PATCH_DRAG = [ { grid: "target" as const, layer: 20, position: 5 }, ]; +/** + * The same two columns, with no cell ringed — for the compare step, which needs + * layer 20 on screen but must not point at the rows its task asks the participant + * to find. Derived from PATCH_DRAG so the layer cannot drift between the step that + * shows the column and the step that drags across it. + */ +const PATCH_COLUMNS = PATCH_DRAG.map(({ grid, layer }) => ({ grid, layer })); + export const PROLIFIC_TUTORIAL_SEED: TutorialContent = { version: 1, welcome: { @@ -454,6 +469,13 @@ export const PROLIFIC_TUTORIAL_SEED: TutorialContent = { 'Write your own pair — two sentences worded the same way with different answers, like "The opposite of hot is" and "The opposite of tall is". Run them and find where each answer settles.', prompts: [EIFFEL, COLOSSEUM], patchPair: { source: EIFFEL, target: COLOSSEUM }, + // Layer 20 on screen in both grids, and nothing ringed. The step's task + // is to find the landmark's row, so a ring would do it for them — but + // without the column rendered at all (auto-fit downsamples layers to the + // width, and two heatmaps in one column is the narrow case), the layer + // the next step drags across isn't there to be looked at, and only + // appears once the stage-2 hint rings it. + forceLayers: PATCH_COLUMNS, hints: [ { stage: 1, diff --git a/workbench/_web/src/types/__tests__/unitSpotlights.test.ts b/workbench/_web/src/types/__tests__/unitSpotlights.test.ts new file mode 100644 index 00000000..be0aa9d4 --- /dev/null +++ b/workbench/_web/src/types/__tests__/unitSpotlights.test.ts @@ -0,0 +1,106 @@ +import { describe, it, expect } from "bun:test"; + +import { resolveUnitSpotlights } from "@/types/tutorial-content"; +import type { HintRung, SpotlightTarget, TutorialUnit } from "@/types/tutorial-content"; + +/** + * The panel's spotlight payload. It is a pure function because it used to be three + * effects writing the same channel, each overwriting the others — which is how the + * hint-revealed rings came to vanish on a remount and how a restored patch came to + * erase the cells its step's task names. + */ + +const unit = (overrides: Partial = {}): TutorialUnit => ({ + id: "u", + kind: "lens", + title: "Unit", + task: "task", + concept: "concept", + prompts: [], + hints: [], + observationPrompt: "What did you notice?", + progression: { on: "run" }, + ...overrides, +}); + +const drag: SpotlightTarget[] = [ + { grid: "source", layer: 20, position: 5 }, + { grid: "target", layer: 20, position: 5 }, +]; +const rung = (stage: number, spotlights?: SpotlightTarget[]): HintRung => ({ + stage, + text: `hint ${stage}`, + ...(spotlights ? { spotlights } : {}), +}); + +describe("resolveUnitSpotlights", () => { + it("lights nothing when the unit has nothing to light", () => { + expect(resolveUnitSpotlights(undefined, 0, false)).toBeNull(); + expect(resolveUnitSpotlights(unit(), 3, true)).toBeNull(); + }); + + it("forces a layer's column without ringing a cell", () => { + // No position: the widget renders that layer and rings nothing. This is the + // compare step, whose task is to find those rows — a ring would do the task. + const cells = resolveUnitSpotlights( + unit({ forceLayers: [{ grid: "source", layer: 20 }] }), + 0, + false, + ); + expect(cells).toEqual([{ grid: "source", layer: 20 }]); + expect(cells![0].position).toBeUndefined(); + }); + + it("derives a revealed hint's cells from the persisted stage, not from the reveal", () => { + // The imperative version of this was lost on any remount (a reload, or + // collapsing the tutorial dock) while the hint still read as revealed. + const u = unit({ hints: [rung(1), rung(2, drag)] }); + expect(resolveUnitSpotlights(u, 1, false)).toBeNull(); + expect(resolveUnitSpotlights(u, 2, false)).toEqual(drag); + }); + + it("prefers a rung's `spotlights` over its singular `spotlight`", () => { + const single: SpotlightTarget = { grid: "result", layer: "last", position: "last" }; + const u = unit({ hints: [{ stage: 1, text: "h", spotlight: single, spotlights: drag }] }); + expect(resolveUnitSpotlights(u, 1, false)).toEqual(drag); + const u2 = unit({ hints: [{ stage: 1, text: "h", spotlight: single }] }); + expect(resolveUnitSpotlights(u2, 1, false)).toEqual([single]); + }); + + it("keeps an earlier rung's cells when a later revealed rung has none of its own", () => { + // Revealed rungs stay on screen together, so the rungs below a cell-less one + // are still the instructions being followed. + const u = unit({ hints: [rung(1, drag), rung(2)] }); + expect(resolveUnitSpotlights(u, 2, false)).toEqual(drag); + }); + + it("adds the patch result to the step's own cells rather than replacing them", () => { + // Commit f3d193a: a patch restored from an earlier session is re-filed on + // arrival with no result grid rendered, and substituting there deleted the + // two cells the task names — and with them the patch layer, since a + // spotlight is also what forces a downsampled layer to render. + const u = unit({ kind: "patch", progression: { on: "patch" }, spotlights: drag }); + expect(resolveUnitSpotlights(u, 0, false)).toEqual(drag); + expect(resolveUnitSpotlights(u, 0, true)).toEqual([ + ...drag, + { grid: "result", layer: "last", position: "last" }, + ]); + }); + + it("does not point at a result cell on a step that has no patch", () => { + expect(resolveUnitSpotlights(unit({ spotlights: drag }), 0, true)).toEqual(drag); + }); + + it("unions forced layers, arrival rings and the revealed rung", () => { + const u = unit({ + forceLayers: [{ grid: "source", layer: 20 }], + spotlights: [{ grid: "target", layer: "last", position: "last" }], + hints: [rung(1, drag)], + }); + expect(resolveUnitSpotlights(u, 1, false)).toEqual([ + { grid: "source", layer: 20 }, + { grid: "target", layer: "last", position: "last" }, + ...drag, + ]); + }); +}); diff --git a/workbench/_web/src/types/tutorial-content.ts b/workbench/_web/src/types/tutorial-content.ts index c2ae1fb2..789ad956 100644 --- a/workbench/_web/src/types/tutorial-content.ts +++ b/workbench/_web/src/types/tutorial-content.ts @@ -20,7 +20,24 @@ export type UnitKind = "lens" | "patch" | "explore" | "challenge"; export interface SpotlightTarget { grid: "source" | "target" | "result"; layer: number | "last"; - position: number | "last"; + /** + * Omit to force that layer's column to render without ringing anything. A + * spotlight does two jobs — it rings a cell, and it makes the widget keep the + * layer its auto-fit would otherwise downsample away — and a step whose task + * is to FIND a cell needs the second job without the first. See + * `TutorialUnit.forceLayers`, which is authored as {grid, layer} and reaches + * the widget as one of these. + */ + position?: number | "last"; +} + +/** + * A layer to keep on screen, with no cell ringed. Authoring shape for + * `TutorialUnit.forceLayers`; it becomes a position-less `SpotlightTarget`. + */ +export interface ForceLayerTarget { + grid: "source" | "target" | "result"; + layer: number | "last"; } /** A hint-ladder rung: 1 = nudge, 2 = concrete suggestion, 3 = show-me. */ @@ -148,6 +165,18 @@ export interface TutorialUnit { * does that work for the participant. */ spotlights?: SpotlightTarget[]; + /** + * Layers to force into the grid on arrival WITHOUT ringing a cell. + * + * `spotlights` couples the two things a spotlight does: it rings the cell and + * it keeps the layer rendered (auto-fit downsamples layers to the available + * width, so the layer a step is about can be missing from a narrow grid + * entirely). A step whose task is to find the row holding an answer needs the + * layer on screen but must not have the answer ringed — ringing it performs + * the task for the participant. That step lists the layer here instead, and + * keeps its rings on the hint rung a stuck participant reaches for. + */ + forceLayers?: ForceLayerTarget[]; hints: HintRung[]; check?: UnitCheck; /** @@ -223,6 +252,62 @@ export function resolveCheckKey( return { expected, canAnswer: expected != null }; } +/** + * Everything the widget should spotlight while a participant sits on this unit — + * the whole payload, derived from state that survives a reload, rather than + * whatever the last imperative call happened to leave behind. + * + * Kept pure and out of the panel because three separate effects used to write + * this and each overwrote the others: + * - the hint-reveal handler wrote its rung's cells and nothing re-applied them, + * so a remount (a reload, or collapsing and re-expanding the tutorial dock) + * silently dropped the rings while the hint still read as revealed; + * - the patch-result effect wrote the result cell. It has to ADD it to the + * step's own cells, never substitute: a patch restored from an earlier session + * is re-filed on arrival even with no result grid on screen, and substituting + * there deleted the two cells the step's task names — and with them (a + * spotlight being what forces a downsampled layer to render) the patch layer + * itself. + * + * @param unit the unit on screen, if content has loaded + * @param hintStage the highest hint rung revealed on this unit (persisted) + * @param patchFiled whether a patch result is filed for this unit + * @returns the cells to light, or null when nothing should be lit + */ +export function resolveUnitSpotlights( + unit: TutorialUnit | undefined, + hintStage: number, + patchFiled: boolean, +): SpotlightTarget[] | null { + if (!unit) return null; + // Position-less first: these only keep a layer rendered, so nothing depends on + // where they sit, and a step that also rings cells is unaffected by them. + const cells: SpotlightTarget[] = (unit.forceLayers ?? []).map(({ grid, layer }) => ({ + grid, + layer, + })); + cells.push(...(unit.spotlights ?? [])); + // The revealed rung's cells. Read the highest revealed rung that actually + // carries cells rather than the rung at exactly `hintStage`: rungs stay on + // screen once revealed, so a later rung with no cells of its own must not + // erase the rings an earlier one put up. `spotlights` wins over `spotlight`. + const revealed = unit.hints + .filter((h) => h.stage <= hintStage) + .sort((a, b) => a.stage - b.stage) + .reduce((acc, h) => { + const rung = h.spotlights?.length ? h.spotlights : h.spotlight ? [h.spotlight] : []; + return rung.length ? rung : acc; + }, []); + cells.push(...revealed); + // The post-patch result cell. Participants performed the intervention and then + // could not find what it changed, so the panel rings it. An unrendered result + // grid resolves to no cell, so this is inert when there is nothing to point at. + if (unit.progression.on === "patch" && patchFiled) { + cells.push({ grid: "result", layer: "last", position: "last" }); + } + return cells.length ? cells : null; +} + /** Evaluate a unit's run-based success predicate against the run's top token. */ export function evalSuccessPredicate( predicate: SuccessPredicate | undefined,