From 2c68fedd96475e26d8ded5407447fb7f7339099c Mon Sep 17 00:00:00 2001 From: alpha Date: Tue, 15 Sep 2026 10:14:30 -0400 Subject: [PATCH] feat(mock): ask about live hints in setup, and default them on The setting lived only on the mock session bar, so finding it meant already knowing the comparison existed - mid-question, which is the worst moment to go looking. Off by default on top of that, most people never met it. Turning it off was right about the risk (answers beside the question stop you composing your own) and wrong about the remedy: that argues for asking, not for answering silently. MockHintsField renders the setting on Configuration and as its own wizard step, from one component like every other setting on those screens, and the session bar still turns it off for the run where composing unaided is the point. The pre-rename key is read forward again. It was scrubbed unread only because the old `true` default would have undone the reversal; with the default back, a stored value is that same default or a real choice. Closes #127 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Kt5ucR5XPzzzxqPYzSpXaL --- CLAUDE.md | 18 +++++++--- src/main/store/config.store.ts | 33 +++++++++-------- src/main/types/mock-interview.ts | 8 ++--- .../custom/settings/mock-hints-field.tsx | 32 +++++++++++++++++ .../hooks/use-mock-live-suggestions.ts | 26 +++++++++----- src/renderer/pages/configuration/index.tsx | 10 +++--- src/renderer/pages/onboarding/index.tsx | 36 +++++++++++++------ src/renderer/types/config.ts | 3 +- test/config-store.test.mjs | 25 ++++++++++++- 9 files changed, 143 insertions(+), 48 deletions(-) create mode 100644 src/renderer/components/custom/settings/mock-hints-field.tsx diff --git a/CLAUDE.md b/CLAUDE.md index f8ce211d..83801db2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -501,11 +501,21 @@ unfocusable, with no interview on screen to explain why. ### The mock interview turn -**Live suggestions are off by default** (`mockLiveHintsEnabled`, and the rename is the reversal: -see the scrub note in the config store for why the old key's value is deliberately not carried -across). A mock interview is for answering the question yourself, and a column of model-written +**Live suggestions are on by default** (`mockLiveHintsEnabled`), and the setting is asked outright +rather than only discoverable. It was turned off for a real reason - a column of model-written answers beside the question while you are trying to think of your own is the one thing most likely -to stop that working. One click on the session bar for the run where comparing is the point. +to stop you composing your own - but that reason argues for asking the question, not for answering +it silently. With the only control on the session bar, a candidate had to already know the +comparison existed to find it, mid-question, which is the worst moment to go looking; so most +people never met it at all. `MockHintsField` now renders the setting on the configuration page and +as its own step in the first-run wizard, and the session bar still turns it off for the run where +composing unaided is the point. + +The pre-rename key `mockLiveSuggestionsEnabled` is read forward again, the way `professionalMode` +is: while the default was reversed it deliberately was not, because every stored `true` was the old +migration's write rather than a choice and reading those forward would have undone the reversal on +every machine it was for. The default now matches the old one, so a stored value is either that +same default or a real choice, and there is nothing left to protect against. **There is no Repeat and no Skip.** Both were escape hatches from a question rather than ways of answering one, and both cost the interview something the candidate could not see: a skip is diff --git a/src/main/store/config.store.ts b/src/main/store/config.store.ts index a933f368..7a8df018 100644 --- a/src/main/store/config.store.ts +++ b/src/main/store/config.store.ts @@ -34,7 +34,7 @@ export interface RuntimeConfig { hintOnlyMode: boolean; // mock interview: also generate what the live assistant would have suggested for each - // question. Off by default - see the note on the default value below. + // question. On by default - see the note on the default value below. mockLiveHintsEnabled: boolean; } @@ -60,13 +60,15 @@ const DEFAULT_RUNTIME_CONFIG: RuntimeConfig = { // migration below. hintOnlyMode: true, - // Opt-in, and a reversal: this shipped on by default on the grounds that seeing what the live - // assistant would have said is one of the two reasons to run a mock interview. In practice it - // is the other one that people run it for - answering the question yourself - and a panel of - // model-written answers sitting beside the question while you try to think of your own is the - // single thing most likely to stop that working. It is one click away on the session bar for - // the run where comparing is the point. - mockLiveHintsEnabled: false, + // On by default. It was turned off on the grounds that a panel of model-written answers beside + // the question stops the candidate composing their own, which is true and is not the whole of + // it: off by default with the only control on the session bar, most people never found out the + // comparison existed, and a hint they have to discover mid-question is worse than one they + // chose in advance. It is asked outright in the first-run wizard now and settable from + // Configuration, so the default is what someone who has not thought about it gets rather than + // the only answer they are ever offered - and the session bar still turns it off for the run + // where composing unaided is the point. + mockLiveHintsEnabled: true, }; // interviewConf (full name, profile, context) used to be cached under `runtime`, but it's now @@ -270,7 +272,12 @@ export const configStore = new ConfigStore(); migration.hintOnlyMode = typeof legacy === 'boolean' ? legacy : true; } if (raw?.mockLiveHintsEnabled === undefined) { - migration.mockLiveHintsEnabled = false; + // `mockLiveSuggestionsEnabled` is the pre-rename name, and it is read forward for the same + // reason `professionalMode` is: the setting means what it meant and now defaults the way it + // defaulted, so a stored value is either the same default or a choice worth keeping. + const legacy = (raw as (StoredRuntime & Record) | undefined) + ?.mockLiveSuggestionsEnabled; + migration.mockLiveHintsEnabled = typeof legacy === 'boolean' ? legacy : true; } // perform migration only if there are values to set if (Object.keys(migration).length > 0) { @@ -312,12 +319,8 @@ scrubRetiredKey('lastSessionMode'); // to carry the user's choice across. Scrubbed after that, so the two can never disagree. scrubRetiredKey('professionalMode'); -// `mockLiveSuggestionsEnabled` is `mockLiveHintsEnabled` under its old name and its old default. -// Deliberately *not* carried across the way `professionalMode` was: that rename kept the user's -// value because the meaning of the setting had not changed, whereas this one exists to reverse a -// default that was wrong. Every install that ever launched holds a `true` the migration wrote for -// it rather than a choice anyone made, so reading those forward would leave the old default in -// place on every machine the reversal is for. The setting is one click away on the session bar. +// `mockLiveSuggestionsEnabled` is `mockLiveHintsEnabled` under its old name, whose migration +// above reads it one last time. Scrubbed after that, so the two can never disagree. scrubRetiredKey('mockLiveSuggestionsEnabled'); // `headphoneNoticeAcknowledged` was replaced by the mock-interview-aware `HeadphoneNoticeDialog` diff --git a/src/main/types/mock-interview.ts b/src/main/types/mock-interview.ts index b7df52b9..04baafd5 100644 --- a/src/main/types/mock-interview.ts +++ b/src/main/types/mock-interview.ts @@ -214,10 +214,10 @@ export interface MockInterviewSessionState { /** The partial/final transcript of the answer currently being given. */ currentAnswerText: string; /** - * What the live assistant would have suggested for each question, off unless the candidate - * turns it on - see `mockLiveHintsEnabled` in `RuntimeConfig`. Empty for the ordinary run. One - * entry per question asked (follow-ups included), oldest first, in the same shape the live - * panel already renders. + * What the live assistant would have suggested for each question, unless the candidate has + * turned it off - see `mockLiveHintsEnabled` in `RuntimeConfig`. Empty for a session run + * without them. One entry per question asked (follow-ups included), oldest first, in the same + * shape the live panel already renders. */ liveHints: LiveSuggestion[]; report: MockReport | null; diff --git a/src/renderer/components/custom/settings/mock-hints-field.tsx b/src/renderer/components/custom/settings/mock-hints-field.tsx new file mode 100644 index 00000000..764c8911 --- /dev/null +++ b/src/renderer/components/custom/settings/mock-hints-field.tsx @@ -0,0 +1,32 @@ +import { Checkbox } from '@/components/ui/checkbox'; +import { useMockLiveSuggestions } from '@/hooks/use-mock-live-suggestions'; + +/** + * Whether a practice interview also shows what the live assistant would have answered, on the + * configuration page and in the first-run wizard. + * + * The same row shape `TranscriptPanelField` uses, and here for the same reason the setting has a + * default at all: it used to live only on the mock session bar, where a candidate had to already + * know the comparison existed to find it - mid-question, which is the worst moment to go looking. + */ +export function MockHintsField() { + const { enabled, setEnabled } = useMockLiveSuggestions(); + + return ( + + ); +} diff --git a/src/renderer/hooks/use-mock-live-suggestions.ts b/src/renderer/hooks/use-mock-live-suggestions.ts index 6139a57a..836c308a 100644 --- a/src/renderer/hooks/use-mock-live-suggestions.ts +++ b/src/renderer/hooks/use-mock-live-suggestions.ts @@ -7,22 +7,30 @@ import { useConfigStore } from './use-config-store'; * Whether a mock session also generates what the live assistant would have suggested, plus a * toggle that persists the change. * - * Absent means **off**, the same direction `hintOnlyMode` is read in and the opposite of what - * this setting used to do. A mock interview is for answering the question yourself, and a panel - * of model-written answers beside the question while you are trying to think of your own is the - * one thing most likely to stop that working - so it is opt-in, for the run where comparing your - * answer against the assistant's is actually the point. + * Absent means **on**, the same direction `hintOnlyMode` is read in, and the main-process store + * backfills the same default - so the two sides agree about a config written before the setting + * existed. Turning it off is what a candidate does for the run where composing the answer + * unaided is the point, and it is asked in the first-run wizard rather than only discoverable + * on the session bar. + * + * Shared by the session bar, the configuration page and the onboarding wizard. `toggle` reads + * the store imperatively so it stays referentially stable. */ export function useMockLiveSuggestions() { const { config } = useConfigStore(); - const toggle = useCallback(() => { - const { config: current, updateConfig } = useConfigStore.getState(); - updateConfig({ mockLiveHintsEnabled: current?.mockLiveHintsEnabled !== true }).catch((e) => { + const persist = useCallback((enabled: boolean) => { + const { updateConfig } = useConfigStore.getState(); + updateConfig({ mockLiveHintsEnabled: enabled }).catch((e) => { console.error('Failed to save mock live suggestions setting', e); toast.error('Failed to save live suggestions setting'); }); }, []); - return { enabled: config?.mockLiveHintsEnabled === true, toggle }; + const toggle = useCallback(() => { + const { config: current } = useConfigStore.getState(); + persist(current?.mockLiveHintsEnabled === false); + }, [persist]); + + return { enabled: config?.mockLiveHintsEnabled !== false, setEnabled: persist, toggle }; } diff --git a/src/renderer/pages/configuration/index.tsx b/src/renderer/pages/configuration/index.tsx index 109334ec..10e173c9 100644 --- a/src/renderer/pages/configuration/index.tsx +++ b/src/renderer/pages/configuration/index.tsx @@ -6,22 +6,23 @@ import { HotkeyCheatsheetDialog } from '@/components/custom/hotkey-cheatsheet'; import PageHeader from '@/components/custom/page-header'; import { LanguageField } from '@/components/custom/settings/language-field'; import { MicrophoneField } from '@/components/custom/settings/microphone-field'; +import { MockHintsField } from '@/components/custom/settings/mock-hints-field'; import { SuggestionModeField } from '@/components/custom/settings/suggestion-mode-field'; import { TranscriptPanelField } from '@/components/custom/settings/transcript-panel-field'; import { ZoomField } from '@/components/custom/settings/zoom-field'; import { Button } from '@/components/ui/button'; /** - * How the interview runs: the microphone, the language, how suggestions read, and whether the - * transcript is docked. + * How the interview runs: the microphone, the language, how suggestions read, whether practice + * interviews show them at all, and whether the transcript is docked. * * Every control here writes straight through to the config store as it is changed - there is no * Save button, because there is nothing to batch and nothing that could be half-applied. That is * the other reason this is not a tab of the account page, which does have a Save and does need * one. * - * These are the same four settings the first-run wizard walks a new user through, rendered from - * the same components, so what the wizard set is what this page shows. + * These are the same settings the first-run wizard walks a new user through, rendered from the + * same components, so what the wizard set is what this page shows. */ export default function ConfigurationPage() { const navigate = useNavigate(); @@ -35,6 +36,7 @@ export default function ConfigurationPage() { + diff --git a/src/renderer/pages/onboarding/index.tsx b/src/renderer/pages/onboarding/index.tsx index 39e21514..360059ac 100644 --- a/src/renderer/pages/onboarding/index.tsx +++ b/src/renderer/pages/onboarding/index.tsx @@ -6,6 +6,7 @@ import { toast } from 'sonner'; import { LoadingPage } from '@/components/custom/loading'; import { LanguageField } from '@/components/custom/settings/language-field'; import { MicrophoneField } from '@/components/custom/settings/microphone-field'; +import { MockHintsField } from '@/components/custom/settings/mock-hints-field'; import { ContextField, FullNameField, @@ -21,7 +22,15 @@ import { useOnboardingDismissed } from '@/hooks/use-onboarding-dismissed'; import { APP_NAME } from '@/lib/consts'; import { getElectron } from '@/lib/utils'; -type StepId = 'profile' | 'context' | 'language' | 'microphone' | 'mode' | 'zoom' | 'transcript'; +type StepId = + | 'profile' + | 'context' + | 'language' + | 'microphone' + | 'mode' + | 'mock-hints' + | 'zoom' + | 'transcript'; interface Step { id: StepId; @@ -33,7 +42,7 @@ interface Step { /** * One thing per step, in the order a first interview needs them: who you are, what you are - * interviewing for, then the five things that decide how the session looks and behaves. + * interviewing for, then the six things that decide how a session looks and behaves. * * Profile first because it is the only step that can block a start - the start sequence refuses * to run without a name and a CV - and the only one that is worth typing rather than picking. @@ -72,6 +81,13 @@ const STEPS: Step[] = [ title: 'How should suggestions read?', description: 'Change your mind at any time, including mid-interview.', }, + { + id: 'mock-hints', + label: 'Practice', + title: 'Hints while you practise?', + description: + 'Practice interviews are the ones you run against yourself. This decides whether they hand you the answer as well.', + }, { id: 'zoom', label: 'Size', @@ -101,8 +117,8 @@ const STEPS: Step[] = [ * that collect them, so a user who closes the app halfway through still keeps what they typed. * * Nothing here is a trap. Skip is on every step, every setting has a working default, and - * Configuration can re-run the whole thing later - which is what lets this screen ask six - * questions without any of them being a decision the user has to get right now. + * Configuration can re-run the whole thing later - which is what lets this screen ask as much as + * it does without any of it being a decision the user has to get right now. */ export default function OnboardingPage() { const navigate = useNavigate(); @@ -235,8 +251,9 @@ export default function OnboardingPage() { if (isLast) { setFinishing(true); try { - // Finish stays put on a failed write, unlike Skip: the user has just answered six - // questions, and leaving on a write that did not land means being asked all six again. + // Finish stays put on a failed write, unlike Skip: the user has just worked through + // every step, and leaving on a write that did not land means being asked again from the + // top. if (!(await complete())) { toast.error('Could not save your setup. Check your connection and try again.'); return; @@ -252,7 +269,7 @@ export default function OnboardingPage() { }; // Signed out, this screen has no account to read or write and every step would fail. Sent - // where `/` sends them, rather than rendering six steps that cannot save. + // where `/` sends them, rather than rendering a wizard whose every step fails to save. if (appState?.isLoggedIn === false) return ; if (appState?.isLoggedIn !== true) return ; @@ -329,6 +346,7 @@ export default function OnboardingPage() { {step.id === 'language' && } {step.id === 'microphone' && } {step.id === 'mode' && } + {step.id === 'mock-hints' && } {step.id === 'zoom' && } {step.id === 'transcript' && } @@ -347,9 +365,7 @@ export default function OnboardingPage() { className="text-muted-foreground" onClick={() => void handleSkip()} disabled={finishing} - title={ - isFirstRun ? 'You can run setup again later from Configuration' : undefined - } + title={isFirstRun ? 'You can run setup again later from Configuration' : undefined} > {isFirstRun ? 'Skip for now' : 'Close'} diff --git a/src/renderer/types/config.ts b/src/renderer/types/config.ts index e9fcc11b..3cf2e8fc 100644 --- a/src/renderer/types/config.ts +++ b/src/renderer/types/config.ts @@ -30,6 +30,7 @@ export interface Config { // sentences. The default; full-sentence mode is the opt-out. hintOnlyMode: boolean; - // Mock interview: also show what the live assistant would have suggested. Off by default. + // Mock interview: also show what the live assistant would have suggested. The default; the + // session bar turns it off for the run where composing unaided is the point. mockLiveHintsEnabled: boolean; } diff --git a/test/config-store.test.mjs b/test/config-store.test.mjs index 83ab5067..787f1d16 100644 --- a/test/config-store.test.mjs +++ b/test/config-store.test.mjs @@ -40,6 +40,10 @@ export async function run(userDataDir) { // Both mechanisms that touch it are exercised below: the migration reads it once to // carry the choice across, then scrubRetiredKey removes it. professionalMode: false, + // The pre-rename name for `mockLiveHintsEnabled`, set to a real choice rather than the + // default of the day. Carried across for the same reason `professionalMode` is, and + // scrubbed afterwards for the same reason too. + mockLiveSuggestionsEnabled: false, }, }) ); @@ -93,7 +97,6 @@ export async function run(userDataDir) { !('professionalMode' in (store.configStore.getStoredRuntime() ?? {})) ); - store.configStore.updateConfig({ hintOnlyMode: true }); check('hintOnlyMode is persisted', store.configStore.getStoredRuntime()?.hintOnlyMode === true); @@ -103,6 +106,26 @@ export async function run(userDataDir) { store.configStore.getConfig().hintOnlyMode === true ); + // Mock live hints default to on, so an upgrading install that had turned them off must come + // back off rather than being handed the new default - which is exactly what would happen if the + // migration stopped reading the pre-rename key. + check('mock live hints carry the upgrading choice across', cfg.mockLiveHintsEnabled === false); + check( + 'and the pre-rename mock hints key is scrubbed', + !('mockLiveSuggestionsEnabled' in (store.configStore.getStoredRuntime() ?? {})) + ); + + // The other half of that: absent on disk has to read as on. Every consumer reads through + // getConfig, so this is the backfill the main-process service actually sees - a default flipped + // in one of the two places and not the other is invisible until a session runs. + const withoutHints = { ...(store.configStore.getStoredRuntime() ?? {}) }; + delete withoutHints.mockLiveHintsEnabled; + store.configStore.setStoredRuntime(withoutHints); + check( + 'an absent mockLiveHintsEnabled reads as on', + store.configStore.getConfig().mockLiveHintsEnabled === true + ); + // `lastSessionMode` backed the control bar's split Start button, which no longer exists. // Seeded above, so this pins the scrub rather than merely that RuntimeConfig stopped declaring // it - every read and write in the store spreads the raw stored object through, and nothing