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 c87ad296..9187f7ad 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 @@ -233,6 +233,8 @@ export function TutorialActivityPanel({ // screen still described this unit's prompt. The key is now pinned to the unit // the run was initiated from, and the prompt is restored on arrival // (PatchLensArea), so the key and the instructions describe the same run. + // Bound to a const so the narrowing survives into the answer callback below. + const unitCheck = unit.check; const { expected: checkExpected, canAnswer: checkHasRun } = resolveCheckKey( unit, store.runTokensByUnit[store.unitIdx], @@ -369,10 +371,10 @@ export function TutorialActivityPanel({ )} {/* Embedded check — auto-scored, log-only */} - {unit.check && ( + {unitCheck && ( store.answerCheck(answer, correct)} + // The key travels with the answer: a run-scored key is this + // unit's own run and is gone by the time anyone grades the + // data (see TutorialEventPayload.expected). + onAnswer={(answer, correct, expected) => + store.answerCheck(answer, correct, { + expected, + checkKind: unitCheck.kind, + }) + } alreadyAnswered={!!store.checkAnsweredByUnit[store.unitIdx]} priorResult={store.checkResultByUnit[store.unitIdx]} /> @@ -689,7 +699,9 @@ function EmbeddedCheck({ * revisited step restate their answer and whether it was right, instead of the * bare "already answered" that a locked check used to show. */ priorResult?: { answer: string; correct: boolean }; - onAnswer: (answer: string, correct: boolean) => void; + /** `expected` is the key the answer was scored against — a choice check's own + * option, or the run-derived key — so the caller can log it. */ + onAnswer: (answer: string, correct: boolean, expected: string | null) => void; }) { const [value, setValue] = useState(""); const [result, setResult] = useState(null); @@ -710,20 +722,27 @@ function EmbeddedCheck({ if (!value.trim() || locked) return; const correct = norm(value) === norm(expected); setResult({ correct, expected: expected ?? "?" }); - onAnswer(value.trim(), correct); + onAnswer(value.trim(), correct, expected); }; const submitChoice = (idx: number) => { if (locked || check.kind !== "choice") return; const correct = idx === check.correctIndex; - setResult({ correct, expected: check.options[check.correctIndex] ?? "?" }); - onAnswer(check.options[idx] ?? String(idx), correct); + const key = check.options[check.correctIndex] ?? null; + setResult({ correct, expected: key ?? "?" }); + onAnswer(check.options[idx] ?? String(idx), correct, key); }; // The correct answer, for restating a wrong prior answer. A choice check // carries its own key; a typed one's key is the run it was scored against, // which a fresh session may no longer have. const correctAnswer = isChoice ? check.options[check.correctIndex] : expected; + // Neutral unless the content opts in. A check scored against the + // participant's own run is often ambiguous — a token they cannot type as it + // renders, a spelling `norm()` does not fold — and being marked wrong on one + // of those discourages a participant who did the step correctly. The score + // still reaches `answerCheck`, so the engagement measure is unaffected. + const showVerdict = check.feedback === "verdict"; return (
@@ -785,31 +804,42 @@ function EmbeddedCheck({ onClick={submitTyped} disabled={locked || !value.trim()} > - Check + {/* "Check" promises a verdict; a neutral check + does not give one. */} + {showVerdict ? "Check" : "Submit"}
)} - {result && ( -

- {result.correct - ? "✓ Correct." - : `Not quite — the answer was “${result.expected}”.`} -

- )} - {!result && priorResult && ( -

- {priorResult.correct - ? `✓ You answered “${priorResult.answer}” — correct.` - : `You answered “${priorResult.answer}” — not quite.`} - {!priorResult.correct && - correctAnswer && - ` The answer was “${correctAnswer}”.`} -

- )} + {result && + (showVerdict ? ( +

+ {result.correct + ? "✓ Correct." + : `Not quite — the answer was “${result.expected}”.`} +

+ ) : ( +

Answer recorded.

+ ))} + {!result && + priorResult && + (showVerdict ? ( +

+ {priorResult.correct + ? `✓ You answered “${priorResult.answer}” — correct.` + : `You answered “${priorResult.answer}” — not quite.`} + {!priorResult.correct && + correctAnswer && + ` The answer was “${correctAnswer}”.`} +

+ ) : ( +

+ You answered “{priorResult.answer}”. +

+ ))} {/* Fallback for a participant whose stored progress predates `checkResultByUnit`: their answer wasn't kept, so all we can honestly say is that they answered. */} diff --git a/workbench/_web/src/db/__tests__/tutorials.test.ts b/workbench/_web/src/db/__tests__/tutorials.test.ts index 92e6569f..89ff3344 100644 --- a/workbench/_web/src/db/__tests__/tutorials.test.ts +++ b/workbench/_web/src/db/__tests__/tutorials.test.ts @@ -147,6 +147,32 @@ describe("tutorial content", () => { units: [{ ...base, check: { question: "?", kind: "layerBand" as never } }], }), ).toThrow(); + // Unsupported check feedback → would fall back to neutral, silently + // dropping the verdict a check was authored to show. + expect(() => + validateTutorialContent({ + version: 1, + units: [ + { + ...base, + check: { + question: "?", + kind: "topToken", + feedback: "scored" as never, + }, + }, + ], + }), + ).toThrow(); + // Both supported values pass. + for (const feedback of ["verdict", "neutral"] as const) { + expect(() => + validateTutorialContent({ + version: 1, + units: [{ ...base, check: { question: "?", kind: "topToken", feedback } }], + }), + ).not.toThrow(); + } }); it("rejects a unit whose rendered fields aren't usable text", () => { diff --git a/workbench/_web/src/lib/queries/tutorialContentDb.ts b/workbench/_web/src/lib/queries/tutorialContentDb.ts index 69501973..057709aa 100644 --- a/workbench/_web/src/lib/queries/tutorialContentDb.ts +++ b/workbench/_web/src/lib/queries/tutorialContentDb.ts @@ -70,6 +70,7 @@ export const validateTutorialContent = (content: TutorialContent): TutorialConte } const validOn = new Set(["run", "patch", "manual"]); const validCheckKinds = new Set(["topToken", "secondToken", "choice"]); + const validCheckFeedback = new Set(["verdict", "neutral"]); for (const u of content.units) { if (!isText(u.id) || !isText(u.title)) { throw new Error("Every unit needs an id and a title"); @@ -195,6 +196,14 @@ export const validateTutorialContent = (content: TutorialContent): TutorialConte if (!isText(u.check.question)) { throw new Error(`Unit "${u.id}" check needs a question`); } + // Absent means neutral (the default the panel renders); a typo'd value + // would silently fall back to it and quietly un-verdict a check that + // was authored to show one. + if (u.check.feedback != null && !validCheckFeedback.has(u.check.feedback)) { + throw new Error( + `Unit "${u.id}" check has an unsupported feedback "${u.check.feedback}"`, + ); + } } // A choice check is scored entirely from its own content, so a missing or // out-of-range key would mark every participant wrong with no run to blame. diff --git a/workbench/_web/src/stores/__tests__/useProlificTutorial.test.ts b/workbench/_web/src/stores/__tests__/useProlificTutorial.test.ts index 3c0df1f7..b068457f 100644 --- a/workbench/_web/src/stores/__tests__/useProlificTutorial.test.ts +++ b/workbench/_web/src/stores/__tests__/useProlificTutorial.test.ts @@ -171,21 +171,21 @@ describe("useProlificTutorial answer keys", () => { }); it("records one check answer per step", () => { - store().answerCheck("Paris", true); + store().answerCheck("Paris", true, { expected: "Paris", checkKind: "topToken" }); expect(store().checkAnsweredByUnit[0]).toBe(true); // A second answer for the same step would double-count the engagement // measure; the store refuses it as well as the input locking. - store().answerCheck("Rome", false); + store().answerCheck("Rome", false, { expected: "Paris", checkKind: "topToken" }); expect(store().checkAnsweredByUnit[0]).toBe(true); }); it("keeps what was answered, not only that it was", () => { // A revisited step restates the answer, so the panel needs more than the // "already answered" boolean. - store().answerCheck("Rome", false); + store().answerCheck("Rome", false, { expected: "Paris", checkKind: "topToken" }); expect(store().checkResultByUnit[0]).toEqual({ answer: "Rome", correct: false }); // Same one-per-step rule as the boolean: the first answer is the record. - store().answerCheck("Paris", true); + store().answerCheck("Paris", true, { expected: "Paris", checkKind: "topToken" }); expect(store().checkResultByUnit[0]).toEqual({ answer: "Rome", correct: false }); }); }); @@ -250,7 +250,7 @@ describe("useProlificTutorial telemetry", () => { store().start(); store().next(); store().prev(); - store().answerCheck("Paris", true); + store().answerCheck("Paris", true, { expected: "Paris", checkKind: "topToken" }); expect(await timeline()).toEqual([ "step_started:u0", "step_started:u4", @@ -259,10 +259,41 @@ describe("useProlificTutorial telemetry", () => { ]); }); + it("logs the answer key alongside the verdict", async () => { + // The participant is not shown whether they were right, so `correct` is the + // whole grading record — and it was computed against a key (their own run) + // that nothing else persists. Without the key on the row, a lenient + // re-grade after the fact is impossible. + store().start(); + store().answerCheck("paris.", false, { expected: "Paris", checkKind: "topToken" }); + await Bun.sleep(20); + const [event] = (await getTutorialEventsForWorkspace(workspaceId)).filter( + (e) => e.eventType === "check_answered", + ); + expect(event?.payload).toMatchObject({ + answer: "paris.", + correct: false, + expected: "Paris", + checkKind: "topToken", + }); + }); + + it("omits the key when the check was scored without one", async () => { + // A literal "null" in the column reads like an answer key of its own. + store().start(); + store().answerCheck("Paris", false, { expected: null, checkKind: "secondToken" }); + await Bun.sleep(20); + const [event] = (await getTutorialEventsForWorkspace(workspaceId)).filter( + (e) => e.eventType === "check_answered", + ); + expect(event?.payload).not.toHaveProperty("expected"); + expect(event?.payload).toMatchObject({ checkKind: "secondToken" }); + }); + it("emits one check_answered even if the check is answered twice", async () => { store().start(); - store().answerCheck("Paris", true); - store().answerCheck("Rome", false); + store().answerCheck("Paris", true, { expected: "Paris", checkKind: "topToken" }); + store().answerCheck("Rome", false, { expected: "Paris", checkKind: "topToken" }); expect(await timeline()).toEqual(["step_started:u0", "check_answered:u0"]); }); }); diff --git a/workbench/_web/src/stores/useProlificTutorial.ts b/workbench/_web/src/stores/useProlificTutorial.ts index 677b06f1..fd283173 100644 --- a/workbench/_web/src/stores/useProlificTutorial.ts +++ b/workbench/_web/src/stores/useProlificTutorial.ts @@ -3,7 +3,7 @@ import { persist } from "zustand/middleware"; import { recordTutorialEvent } from "@/lib/queries/tutorialEventsQueries"; import type { TutorialEventType, TutorialEventPayload } from "@/types/tutorialEvents"; -import type { TutorialUnit } from "@/types/tutorial-content"; +import type { TutorialUnit, UnitCheck } from "@/types/tutorial-content"; import { evalSuccessPredicate } from "@/types/tutorial-content"; /** @@ -152,7 +152,20 @@ interface ProlificTutorialState { markReached: (idx: number) => void; /** Reveal the next hint rung; returns the new highest stage. */ revealHint: () => number; - answerCheck: (answer: string, correct: boolean) => void; + /** + * File the participant's answer to this step's check. + * + * `grading` is required rather than optional: the answer key is what makes + * the logged verdict re-checkable, and it is only in scope at the call site + * (a run-scored key comes from this unit's own run). An optional argument + * would let a future caller drop it and leave a `check_answered` row nobody + * can re-grade. + */ + answerCheck: ( + answer: string, + correct: boolean, + grading: { expected: string | null; checkKind: UnitCheck["kind"] }, + ) => void; submitObservation: (text: string) => void; setPanelPos: (pos: PanelPos) => void; setCollapsed: (collapsed: boolean) => void; @@ -434,7 +447,7 @@ export const useProlificTutorial = create()( return nextStage; }, - answerCheck: (answer, correct) => { + answerCheck: (answer, correct, grading) => { const state = get(); const idx = state.unitIdx; // One check_answered per step, enforced here as well as by the input's @@ -451,6 +464,12 @@ export const useProlificTutorial = create()( emit(state.workspaceId, stepIdForUnit(state, idx), "check_answered", { answer, correct, + // Null key means the check was answerable without one (it + // cannot be, today — `resolveCheckKey` closes the check when + // it has no key), so omit the field rather than logging a + // literal "null" that reads like an answer. + ...(grading.expected != null ? { expected: grading.expected } : {}), + checkKind: grading.checkKind, }); }, diff --git a/workbench/_web/src/types/tutorial-content.ts b/workbench/_web/src/types/tutorial-content.ts index 789ad956..64bc92ff 100644 --- a/workbench/_web/src/types/tutorial-content.ts +++ b/workbench/_web/src/types/tutorial-content.ts @@ -55,12 +55,34 @@ export interface HintRung { spotlights?: SpotlightTarget[]; } +export type CheckFeedback = "verdict" | "neutral"; + +interface BaseCheck { + question: string; + /** + * What the participant is told once they answer. Defaults to `"neutral"`. + * + * - `"neutral"` — the answer is acknowledged and nothing else. It is still + * scored, persisted in `checkResultByUnit`, and emitted on + * `check_answered`; the verdict just never reaches the screen. + * - `"verdict"` — right/wrong, plus the answer key when wrong. + * + * Neutral is the default because most of these are engagement checks scored + * against the participant's own run, and several have no single right + * answer: a token that cannot be typed as it renders, a spelling `norm()` + * does not fold, a runner-up that moved between runs. Being told they are + * wrong on one of those discourages a participant for no reason, and the + * measure we actually want (did they look?) survives without it. Opt a check + * into `"verdict"` only when its key is unambiguous. + */ + feedback?: CheckFeedback; +} + /** * An embedded engagement check. Either auto-scored against the participant's own * run (`topToken` / `secondToken`), or a fixed multiple choice. */ -export interface RunScoredCheck { - question: string; +export interface RunScoredCheck extends BaseCheck { // Which facet of the run result the answer is compared against. kind: "topToken" | "secondToken"; } @@ -71,8 +93,7 @@ export interface RunScoredCheck { * newline, punctuation) answers a different question instead — so a check whose * point is engagement verification rather than typing accuracy uses this. */ -export interface ChoiceCheck { - question: string; +export interface ChoiceCheck extends BaseCheck { kind: "choice"; options: string[]; correctIndex: number; diff --git a/workbench/_web/src/types/tutorialEvents.ts b/workbench/_web/src/types/tutorialEvents.ts index b8cc8d24..fa35bd3d 100644 --- a/workbench/_web/src/types/tutorialEvents.ts +++ b/workbench/_web/src/types/tutorialEvents.ts @@ -4,6 +4,8 @@ // telemetry lives in the app DB only — never PostHog (observation/answer text // must not leave the app DB). +import type { UnitCheck } from "./tutorial-content"; + export const tutorialEventTypes = [ "step_started", "step_completed", @@ -26,6 +28,16 @@ export interface TutorialEventPayload { // check_answered: the participant's answer + whether it was correct. answer?: string; correct?: boolean; + // check_answered: the answer key the answer was scored against, and which + // kind of check produced it. `correct` alone cannot be re-derived after the + // fact — a run-scored key comes from the participant's own run, and the + // comparison is a lenient string match — so the key is logged alongside the + // verdict. That keeps post-hoc re-grading (with a different normaliser, or by + // hand) possible from the event row on its own, which matters now that the + // participant is no longer shown whether they were right. Absent on rows + // written before this field existed. + expected?: string; + checkKind?: UnitCheck["kind"]; // Attempt counter context (e.g. which failed-run attempt triggered a hint). attempt?: number; }