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