feat(suggestions): optional professional mode for at-a-glance hints

[alpha5611331]

Closes #96
Backend half: PowerInterviewAI/backend#48

What

An opt-in mode that switches live and triggered suggestions from full sentences to hints - a bold one-line core answer plus 3-5 keyword bullets:

**Cut p99 from 1.8s to 210ms on the orders API**
- Bottleneck: N+1 query, 40 calls/request
- Fix: batch loader + composite index
- Cache: Redis, 60s TTL on hot reads
- Result: 8x throughput, no infra spend
- Watch for: cache invalidation on write path

Off by default. The migration IIFE backfills professionalMode: false, so upgrading installs and fresh installs both keep prose. test/config-store.test.mjs pins that a runtime seeded without the key reads back off, and that an explicit true survives an unrelated write.

Safe to merge before the backend deploys: mode defaults to normal server-side, so an older deployment ignores the field.

Three decisions worth reviewing

The toggle is not disabled while running. Unlike the LLM and audio buttons, this is a mid-interview control - the whole point is flipping it when a question turns out to need a different format. It only affects the next suggestion; in-flight streams are untouched.

Ctrl+Shift+F7, not Ctrl+Shift+P. globalShortcut claims accelerators system-wide, so binding P would take the command palette away from every editor on the machine for as long as this app runs. F7 also sits with the existing F8-F12 block, and a function key keeps it reachable in stealth mode where the control panel is hidden - the same reasoning the F8 comment already gives.

Each LiveSuggestion carries the mode it was generated under, and the panel picks its renderer from that rather than from the current setting. Reading the live setting would reformat every card on screen the moment the user toggles, parsing already-delivered prose as Markdown. Professional answers render through SafeMarkdown, the component the action panel already uses, so no new styling was needed.

Incidental fix

generateSuggestion is called fire-and-forget, and its abort-map cleanup lives in its own finally. The config read now sits above the try that guards it, so a throw there would leak an entry and leave a dead controller. Added the same .catch the action service already carries for exactly this reason.

Verification

• pnpm lint clean
• pnpm exec tsc clean on both tsconfig.app.json and tsconfig.electron.json
• pnpm build succeeds
• pnpm test:main - all checks pass, including three new config-store assertions

Not yet exercised end-to-end against a running backend with professional mode enabled, since PowerInterviewAI/backend#48 is not deployed. Worth a manual pass once it is: toggle mid-interview and confirm the existing card keeps its rendering while the next one switches.

🤖 Generated with Claude Code

[alpha5611331]

Thanks @gitar-bot - right about the symptom, and I went and checked the proposed fix before applying it. It does not hold either, for a different reason.

updateConfig writes { ...DEFAULT_RUNTIME_CONFIG, ...stored, ...updates }, so it re-spreads the defaults on every write. The seeded runtime in that test is missing the autoScroll* keys, so the migration IIFE fires regardless and professionalMode: false lands on disk whether or not its own migration branch exists. getStoredRuntime()?.professionalMode === false therefore passes unconditionally too.

I verified this by breaking each mechanism in turn rather than reasoning about it:

change	getConfig assertion	why
delete the migration branch	still passes	DEFAULT_RUNTIME_CONFIG.professionalMode is false
flip the default to true	still passes	the migration branch pins false, not the default

So the two mechanisms are genuinely redundant, and while both hold no single assertion can isolate one. I could have written a second seeded scenario with every other migration-guarded key present to isolate the branch, but that pins implementation rather than behaviour - the key materialising on disk has no functional consequence, since getConfig() returns false either way.

The redundancy is worth keeping, though, and the second row above is why: if professional mode ever becomes the default for new installs, the migration branch is what keeps existing users on prose. That is a real behavioural guarantee, just not one this fixture can currently distinguish.

Left the code as-is and rewrote the comment in 99ebab9 to state what the assertion actually guards - the invariant itself - instead of claiming to pin the backfill. The misleading comment was the real defect here.

[alpha5611331]

Self-review: side effects checked

Went back over the diff for things neither CI nor Gitar would catch. Four checked and clear, two residual risks worth knowing about.

Clear

Import cycle. src/main/types/app-state.ts now imports SuggestionMode from ./llm.js, and llm.ts already imported Transcript from ./app-state.js - a cycle on paper. Checked the compiled output rather than assuming: both electron-dist/types/app-state.js and types/llm.js emit zero imports, because SuggestionMode is used only in type position in app-state.ts and TS elides it. No runtime cycle. (tsconfig.electron.json does not set verbatimModuleSyntax; the renderer configs do, and types/suggestion.ts uses import type accordingly.)

Request construction sites. Only two exist (suggestion-live.service.ts:93, suggestion-action.service.ts:214), both updated. LLMApi serialises the body whole, so no allowlist to update.

Abort-map cleanup. The new .catch can only fire when generateSuggestion throws before its try, in which case the finally never ran. When it throws inside, the internal catch handles it and the promise resolves - so no double-delete path. A stale taskId delete after stopRunningTasks() has cleared the map is a no-op on a Map.

Export. buildExportMarkdown interpolates s.answer raw into a Markdown document under #### ***Suggestion***. Professional answers are already Markdown, so bold and bullets nest correctly. Genuinely a no-op, confirmed by reading it rather than assuming.

Residual risks

1. NO_SUGGESTION_NEEDED depends on prompt adherence. The client filter is LIVE_SUGGESTION_NO_SUGGESTION.startsWith(answer) - it drops the card while the stream is still a prefix of the sentinel. If the model wraps it as **NO_SUGGESTION_NEEDED** in professional mode, the first chunk ** is not a prefix, the filter misses, and the user sees a literal sentinel card on every filler utterance.

The backend prompt guards this explicitly ("emit it alone, bare, with no bold, no bullet, no punctuation"), and PowerInterviewAI/backend#48 pins that the instruction survives in both modes. But that is prompt adherence, not a guarantee. I deliberately did not add client-side stripping: it would change the filter for normal mode too, on a hypothesis. This is the first thing to watch in manual testing - if it shows up, normalising before the prefix check is the fix.

2. Markdown re-parses on every stream chunk. appendSuggestion broadcasts on each chunk, so SafeMarkdown re-parses the accumulating answer each time. The action panel already behaves this way, but it is triggered manually, whereas live suggestions fire on every interviewer utterance. Professional answers target under 60 words, so each parse is small - acceptable, but it is a new per-chunk cost that did not exist on this path. Unclosed ** will also render as literal asterisks mid-stream and snap to bold when the closing pair arrives; self-correcting, and identical to today's action panel behaviour.

Both are inherent to the "prompt change, no post-processing" approach and are best judged against a real interview once backend#48 deploys.

[gitar-bot]

Confirmed — checked config.store.ts and your analysis holds. updateConfig (line 112) does { ...DEFAULT_RUNTIME_CONFIG, ...stored, ...updates } on every call, so the migration IIFE's own write backfills professionalMode on disk regardless of whether its raw?.professionalMode === undefined branch (line 238) actually fires. The default spread and the migration branch are both sufficient on their own, so getStoredRuntime() isn't any more discriminating than getConfig() here — my suggested fix would have been just as unconditional.

Your empirical check (deleting the migration branch, then flipping the default) is the right way to confirm two mechanisms are redundant rather than reasoning about the spread order in the abstract, and the forward-compatibility argument for keeping both is solid — the migration branch is what protects existing installs if professionalMode defaults change later. Rewriting the comment to describe the invariant instead of implying the assertion pins the backfill path is the correct fix; thanks for tracking it down.

[alpha5611331]

Update: residual risk 1 is resolved

Above I flagged the NO_SUGGESTION_NEEDED prefix match as the first thing to watch, since **NO_SUGGESTION_NEEDED** would slip past LIVE_SUGGESTION_NO_SUGGESTION.startsWith(answer) and put a literal sentinel card on screen for every filler utterance.

Tested it against the real free-tier model rather than leaving it to first deploy. A filler-only transcript ("hmm") in professional mode returns exactly 'NO_SUGGESTION_NEEDED' - no bold, no bullet, no trailing punctuation. I ran this file's own condition against the raw output and it drops the card correctly.

So no client-side normalisation is needed, which is the outcome I was hoping for - the alternative would have changed the filter's behaviour for normal mode too.

Detail and the other probes are on PowerInterviewAI/backend#48. Summary of what the model actually produced in professional mode:

**Reduced orders API p99 latency from 1.8s to 210ms**
- Problem: N+1 queries, missing index
- Fix: Rewrite query, add composite index
- Stack: FastAPI, MongoDB, Redis cache
- Result: p99 210ms, 88% latency drop

Four bullets, 35 words. That renders through SafeMarkdown as a bold line plus a list-disc list, which is exactly what this panel change assumes.

Residual risk 2 (per-chunk Markdown re-parse) still stands and is unchanged - at 35 words the parse is trivial, but it is a real new per-chunk cost on the live path. Worth a look at CPU during a long session.

Still not covered by any of this: the rendered result in the actual app. The control panel is behind login, so I have not put eyes on the new button or the Markdown card. That is the remaining manual check.

[anton-karlovskiy]

LGTM

[kevinkamto]

LGTM

[chmm195]

LGTM

[gitar-bot]

Code Review ✅ Approved 1 resolved / 1 findings

Adds optional professional mode for concise, bulleted live suggestions rendered as Markdown. Addressed the tautological migration test finding.

✅ 1 resolved

✅ Quality: Migration test is tautological, doesn't verify backfill

📄 test/config-store.test.mjs:70 📄 src/main/store/config.store.ts:98-102
getConfig() returns { ...DEFAULT_RUNTIME_CONFIG, ...stored }, and DEFAULT_RUNTIME_CONFIG.professionalMode is false, so cfg.professionalMode === false passes regardless of whether the migration IIFE actually backfilled the key to disk. The assertion therefore does not test what its comment claims (that a seeded-without-the-key runtime reads back off after migration). To actually pin the migration, assert on the persisted layer, e.g. store.configStore.getStoredRuntime()?.professionalMode === false, so a broken/absent backfill would fail the test.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[mchl7852]

Good job 👍