From fb131b21ba3c95129d577ef56e0ed53fcf5bf67b Mon Sep 17 00:00:00 2001 From: alpha Date: Thu, 6 Aug 2026 19:02:58 -0500 Subject: [PATCH 1/6] fix(suggestions): actually cancel superseded requests Superseding a suggestion never cancelled the previous request anywhere. stopRunningTasks only flipped a boolean, and it was checked before await reader.read(), so a parked read never observed it. requestStream passed no signal to fetch, and the abandoned body got releaseLock() without cancel(). Undici is explicit that an unconsumed, uncancelled response body leaks the connection and can stall or deadlock later requests. Its bodyTimeout defaults to 300s, so a stuck card sat in Pending for five minutes. Because the client never disconnected, the server had nothing to react to and drained the full completion, so superseded requests kept burning the provider budget the new request needed. The stall timeout is a resettable timer, not AbortSignal.timeout: that is a total wall-clock deadline and would truncate a long but healthy generation. It is also not a race against reader.read(), because a losing read promise stays pending having already consumed a read request. Time-to-first-byte is budgeted separately per service, since an action request uploads up to four screenshots before the provider emits a token. Ctrl+Shift+F11 reported as "doesn't show at all": one stalled action stream held the action lock forever, and the same lock gates F9 and F12, so a single stall disabled all three hotkeys with no recovery short of restarting. The lock was also taken before the try block, so a throw during setup leaked it with no network call involved. Also fixed: - an empty stream left a live suggestion in Pending forever, because state was only promoted to Loading inside if (value) and the terminal check only fired for Loading. No timeout rescued it, since the stream ended rather than stalled. The action path had the mirror bug and rendered a blank Success card - clear() now bumps an epoch. An aborted task's terminal write lands a microtask later and used to re-insert a dead session's suggestion into the freshly cleared map - and real aborts make that the normal path - startAssistant's catch never tore down transcription, so a failure after transcription.start() succeeded left isActive true while runningState went back to Idle: live suggestions kept working while every action hotkey refused forever - an orphaned mic partial gated live suggestions until the candidate next finished speaking, which can span several interviewer questions The transcript gate is now logged. It is the only place a suggestion is suppressed without a trace, and it separates "the request was never made" from "the request was made and stalled". Refs #77 Co-Authored-By: Claude Opus 5 --- src/main/api/client.ts | 21 +++- src/main/api/llm.ts | 10 +- src/main/consts.ts | 22 ++++ src/main/services/action-lock.service.ts | 27 +++- .../services/suggestion-action.service.ts | 89 ++++++++++---- src/main/services/suggestion-live.service.ts | 115 ++++++++++++++---- src/main/services/transcript.service.ts | 25 +++- src/renderer/hooks/use-assistant-service.ts | 9 ++ 8 files changed, 257 insertions(+), 61 deletions(-) diff --git a/src/main/api/client.ts b/src/main/api/client.ts index 5cdc6ae5..c9286bb7 100644 --- a/src/main/api/client.ts +++ b/src/main/api/client.ts @@ -112,9 +112,13 @@ export class ApiClient { return this.request('POST', url, body, timeoutMs); } - async postStream(path: string, body?: unknown): Promise | null> { + async postStream( + path: string, + body?: unknown, + signal?: AbortSignal + ): Promise | null> { const url = this.buildUrl(path); - return this.requestStream('POST', url, body); + return this.requestStream('POST', url, body, signal); } async put(path: string, body?: unknown): Promise> { @@ -184,10 +188,14 @@ export class ApiClient { } } + // No `timeoutMs` here on purpose: AbortSignal.timeout is a total wall-clock deadline, which + // would truncate a long-but-healthy generation mid-stream. Callers pass a signal driven by a + // stall timer that resets on every chunk instead. async requestStream( method: string, url: string, - body?: unknown + body?: unknown, + signal?: AbortSignal ): Promise | null> { try { const sessionToken = configStore.getConfig().sessionToken; @@ -199,6 +207,7 @@ export class ApiClient { method, headers: this.headers, body: body ? JSON.stringify(body) : undefined, + signal, }); if (!response.ok) { const responseContent = await response.text().catch(() => ''); @@ -228,6 +237,12 @@ export class ApiClient { throw error; } + // A supersede or stall abort is deliberate. Let it through untouched so callers can + // read `signal.reason` to tell the two apart, and do not log it as a failure. + if (error instanceof Error && error.name === 'AbortError') { + throw error; + } + console.error('[ApiClient] Streaming request error:', { method, url, error }); throw new ApiRequestError( error instanceof Error ? error.message : 'Network request failed', diff --git a/src/main/api/llm.ts b/src/main/api/llm.ts index bd50eea9..60020c95 100644 --- a/src/main/api/llm.ts +++ b/src/main/api/llm.ts @@ -32,9 +32,10 @@ export class LLMApi extends ApiClient { * Generate Live Suggestions */ async generateLiveSuggestions( - data: GenerateLiveSuggestionRequest + data: GenerateLiveSuggestionRequest, + signal?: AbortSignal ): Promise | null> { - return this.postStream('/api/llm/live-suggestion', data); + return this.postStream('/api/llm/live-suggestion', data, signal); } /** @@ -48,9 +49,10 @@ export class LLMApi extends ApiClient { * Generate Action Suggestion */ async generateActionSuggestionStream( - payload: GenerateActionSuggestionRequest + payload: GenerateActionSuggestionRequest, + signal?: AbortSignal ): Promise | null> { - return this.postStream('api/llm/action-suggestion', payload); + return this.postStream('api/llm/action-suggestion', payload, signal); } /** diff --git a/src/main/consts.ts b/src/main/consts.ts index 8ae2a78c..6cfd7cc5 100644 --- a/src/main/consts.ts +++ b/src/main/consts.ts @@ -18,12 +18,34 @@ export const DEFAULT_HEIGHT = 768; // Transcript constants export const TRANSCRIPT_INTER_TRANSCRIPT_GAP_MS = 5000; +// An in-flight mic partial gates live suggestions, and it is only cleared by a matching final. +// An ASR websocket that drops mid-utterance never sends that final, so the partial is orphaned +// and suppresses suggestions until the candidate next finishes speaking - which can span +// several interviewer questions if they stay quiet. Generous on purpose: the gate exists to +// stop suggestions firing over someone mid-answer, so a short value would regress that. +export const SELF_PARTIAL_STALE_MS = 15_000; + // Suggestion constants export const LIVE_SUGGESTION_GAP_MS = 2000; export const LIVE_SUGGESTION_NO_SUGGESTION = 'NO_SUGGESTION_NEEDED'; export const ACTION_SUGGESTION_MAX_CAPTURES = 4; export const ACTION_TIMEOUT_MS = 30_000; // 30 seconds +// Time to first byte. Separate budgets: an action request uploads up to four screenshots and +// the backend base64-encodes them before the provider emits a token, so it starts far slower +// than a live suggestion. These bound a request that never starts, not total generation time. +export const LIVE_SUGGESTION_TTFB_MS = 20_000; +export const ACTION_SUGGESTION_TTFB_MS = 45_000; + +// Gap between chunks once a stream is flowing. Reset on every chunk, so this never caps a +// long-but-healthy generation. +export const SUGGESTION_STALL_MS = 15_000; + +// Backstop only. The action lock is released explicitly on every path; this exists so that a +// bug in a future caller cannot brick Ctrl+Shift+F9/F11/F12 for the rest of a session. Well +// above the longest legitimate action suggestion. +export const ACTION_LOCK_MAX_HOLD_MS = 180_000; + // Stealth mode opacity levels (cycles on each toggle; default = second highest) export const OPACITY_LEVELS = [0.2, 0.5, 0.73, 0.9] as const; export const OPACITY_DEFAULT = OPACITY_LEVELS[OPACITY_LEVELS.length - 2]; // 0.73 diff --git a/src/main/services/action-lock.service.ts b/src/main/services/action-lock.service.ts index 1f0da99f..a5b189e4 100644 --- a/src/main/services/action-lock.service.ts +++ b/src/main/services/action-lock.service.ts @@ -3,6 +3,7 @@ * Manages blocking of long-running action suggestion actions (screenshot capture, suggestion generation) */ +import { ACTION_LOCK_MAX_HOLD_MS } from '../consts.js'; import { pushNotificationService } from './push-notification.service.js'; export enum ActionType { @@ -12,6 +13,7 @@ export enum ActionType { class ActionLockService { private currentAction: ActionType | null = null; + private holdTimer: NodeJS.Timeout | null = null; /** * Try to acquire lock for an action @@ -23,6 +25,19 @@ class ActionLockService { return false; } this.currentAction = action; + + // A held lock blocks all three action hotkeys with no recovery short of restarting the + // app, so never let one outlive its holder. Callers still release explicitly; this only + // fires if that fails. + this.holdTimer = setTimeout(() => { + console.error( + `[ActionLockService] Lock held by ${this.currentAction} for more than ` + + `${ACTION_LOCK_MAX_HOLD_MS}ms, force-releasing. This indicates a leaked lock.` + ); + this.currentAction = null; + this.holdTimer = null; + }, ACTION_LOCK_MAX_HOLD_MS); + return true; } @@ -30,8 +45,16 @@ class ActionLockService { * Release the lock */ release(action: ActionType): void { - if (this.currentAction === action) { - this.currentAction = null; + // Only the holder may release. Clearing the timer on a mismatched call would strip the + // backstop from a lock that is still held by someone else. + if (this.currentAction !== action) { + return; + } + + this.currentAction = null; + if (this.holdTimer) { + clearTimeout(this.holdTimer); + this.holdTimer = null; } } diff --git a/src/main/services/suggestion-action.service.ts b/src/main/services/suggestion-action.service.ts index 75ed88d7..53ec5e45 100644 --- a/src/main/services/suggestion-action.service.ts +++ b/src/main/services/suggestion-action.service.ts @@ -2,7 +2,13 @@ import { BrowserWindow, desktopCapturer, screen } from 'electron'; import sharp from 'sharp'; import { LLMApi } from '../api/llm.js'; -import { ACTION_SUGGESTION_MAX_CAPTURES, ACTION_TIMEOUT_MS, BACKEND_BASE_URL } from '../consts.js'; +import { + ACTION_SUGGESTION_MAX_CAPTURES, + ACTION_SUGGESTION_TTFB_MS, + ACTION_TIMEOUT_MS, + BACKEND_BASE_URL, + SUGGESTION_STALL_MS, +} from '../consts.js'; import { configStore } from '../store/config.store.js'; import { ActionSuggestion, @@ -23,7 +29,7 @@ export class ActionSuggestionService { private llmApi: LLMApi = new LLMApi(); private uploadedImageNames: string[] = []; private suggestions: Map = new Map(); - private abortMap: Map = new Map(); + private abortMap: Map = new Map(); hasUploadedImages(): boolean { return this.uploadedImageNames.length > 0; @@ -146,14 +152,20 @@ export class ActionSuggestionService { this.stopRunningTasks(); const taskId = UuidUtil.generate(); - this.abortMap.set(taskId, false); - this.generateSuggestion(taskId, appState.transcripts); + this.abortMap.set(taskId, new AbortController()); + + // generateSuggestion owns the release, but it can throw before reaching its own try/finally + // (config reads, state reads, the first setSuggestion). Releasing here on a synchronous + // rejection keeps a leaked lock from disabling all three action hotkeys for the session. + this.generateSuggestion(taskId, appState.transcripts).catch((error) => { + console.error('[ActionSuggestionService] generateSuggestion rejected:', error); + actionLockService.release(ActionType.CaptureSuggestion); + }); } stopRunningTasks(): void { - this.abortMap.forEach((_value, key) => { - this.abortMap.set(key, true); - }); + this.abortMap.forEach((controller) => controller.abort()); + this.abortMap.clear(); } async clear(): Promise { @@ -184,6 +196,11 @@ export class ActionSuggestionService { } private async generateSuggestion(taskId: string, transcripts: Transcript[]): Promise { + const controller = this.abortMap.get(taskId); + if (!controller) { + return; + } + const timestamp = DateTimeUtil.now(); const conf = configStore.getConfig(); const interviewConfig = appStateService.getState().interviewConfig; @@ -210,8 +227,20 @@ export class ActionSuggestionService { this.uploadedImageNames = []; + // See the live-suggestion service: a resettable timer, not a race against reader.read(). + let stallTimer: NodeJS.Timeout | null = null; + const armStallTimer = (ms: number): void => { + if (stallTimer) clearTimeout(stallTimer); + stallTimer = setTimeout(() => { + controller.abort(new DOMException('stalled', 'TimeoutError')); + }, ms); + }; + try { - const stream = await this.llmApi.generateActionSuggestionStream(payload); + // Action requests carry up to four screenshots that the backend base64-encodes before + // the provider emits a token, so they legitimately start much slower than live ones. + armStallTimer(ACTION_SUGGESTION_TTFB_MS); + const stream = await this.llmApi.generateActionSuggestionStream(payload, controller.signal); if (!stream) { throw new Error('Failed to get stream response'); } @@ -228,36 +257,52 @@ export class ActionSuggestionService { if (done) break; - if (this.abortMap.get(taskId)) { - this.abortMap.delete(taskId); - - console.info('[ActionSuggestionService] Action suggestion generation stopped by user'); - suggestion.state = SuggestionState.Stopped; - this.setSuggestion(timestamp, suggestion); - return; - } - if (value) { + armStallTimer(SUGGESTION_STALL_MS); const chunk = decoder.decode(value, { stream: true }); suggestion.answer += chunk; - suggestion.state = SuggestionState.Loading; this.setSuggestion(timestamp, suggestion); } } if (suggestion.state === SuggestionState.Loading) { - suggestion.state = SuggestionState.Success; + if (suggestion.answer.length === 0) { + // This path already promoted to Loading before the loop, so an empty stream lands + // here as a blank Success card rather than a stuck one. Still wrong: report it. + suggestion.state = SuggestionState.Error; + suggestion.error = 'The model returned an empty response.'; + } else { + suggestion.state = SuggestionState.Success; + } this.setSuggestion(timestamp, suggestion); } } finally { + // releaseLock does not cancel the body; an abandoned body leaks the connection. + await reader.cancel().catch(() => {}); reader.releaseLock(); } } catch (error) { - console.error('[ActionSuggestionService] Failed to generate action suggestion:', error); - suggestion.state = SuggestionState.Error; - suggestion.error = getSuggestionErrorMessage(error); + const aborted = error instanceof Error && error.name === 'AbortError'; + const stalled = + controller.signal.reason instanceof Error && + controller.signal.reason.name === 'TimeoutError'; + + if (aborted && !stalled) { + console.info('[ActionSuggestionService] Action suggestion generation stopped'); + suggestion.state = SuggestionState.Stopped; + } else { + if (!aborted) { + console.error('[ActionSuggestionService] Failed to generate action suggestion:', error); + } + suggestion.state = SuggestionState.Error; + suggestion.error = stalled + ? 'The response timed out. Please try again.' + : getSuggestionErrorMessage(error); + } this.setSuggestion(timestamp, suggestion); } finally { + if (stallTimer) clearTimeout(stallTimer); + this.abortMap.delete(taskId); actionLockService.release(ActionType.CaptureSuggestion); } } diff --git a/src/main/services/suggestion-live.service.ts b/src/main/services/suggestion-live.service.ts index 9d3c4036..f62f4aec 100644 --- a/src/main/services/suggestion-live.service.ts +++ b/src/main/services/suggestion-live.service.ts @@ -1,5 +1,9 @@ import { LLMApi } from '../api/llm.js'; -import { LIVE_SUGGESTION_NO_SUGGESTION } from '../consts.js'; +import { + LIVE_SUGGESTION_NO_SUGGESTION, + LIVE_SUGGESTION_TTFB_MS, + SUGGESTION_STALL_MS, +} from '../consts.js'; import { configStore } from '../store/config.store.js'; import { LiveSuggestion, Speaker, SuggestionState, Transcript } from '../types/app-state.js'; import { GenerateLiveSuggestionRequest } from '../types/llm.js'; @@ -11,16 +15,26 @@ import { appStateService } from './app-state.service.js'; class LiveSuggestionService { private llmApi: LLMApi = new LLMApi(); private suggestions: Map = new Map(); - private abortMap: Map = new Map(); + private abortMap: Map = new Map(); + + // Bumped by clear(). An aborted task's rejection lands a microtask after clear() has emptied + // the map, and without this its terminal write would re-insert a dead session's suggestion + // into the freshly cleared state. + private epoch = 0; async clear(): Promise { + this.epoch += 1; this.stopRunningTasks(); this.suggestions.clear(); // Update app state appStateService.updateState({ liveSuggestions: [] }); } - private appendSuggestion(timestamp: number, suggestion: LiveSuggestion): void { + private appendSuggestion(timestamp: number, suggestion: LiveSuggestion, epoch: number): void { + if (epoch !== this.epoch) { + return; + } + if ( suggestion.answer.length > 0 && LIVE_SUGGESTION_NO_SUGGESTION.startsWith(suggestion.answer) @@ -39,6 +53,12 @@ class LiveSuggestionService { return; } + const epoch = this.epoch; + const controller = this.abortMap.get(taskId); + if (!controller) { + return; + } + const timestamp = DateTimeUtil.now(); const suggestion: LiveSuggestion = { timestamp, @@ -49,7 +69,18 @@ class LiveSuggestionService { }; // Append initial suggestion - this.appendSuggestion(timestamp, suggestion); + this.appendSuggestion(timestamp, suggestion, epoch); + + // A plain resettable timer, not a race against reader.read(): a losing read promise stays + // pending and has already consumed a read request, so looping would leave two outstanding + // reads on one reader. Aborting makes the read reject on its own. + let stallTimer: NodeJS.Timeout | null = null; + const armStallTimer = (ms: number): void => { + if (stallTimer) clearTimeout(stallTimer); + stallTimer = setTimeout(() => { + controller.abort(new DOMException('stalled', 'TimeoutError')); + }, ms); + }; try { const conf = configStore.getConfig(); @@ -61,48 +92,77 @@ class LiveSuggestionService { transcripts: transcripts, }; - const response = await this.llmApi.generateLiveSuggestions(requestBody); + armStallTimer(LIVE_SUGGESTION_TTFB_MS); + const response = await this.llmApi.generateLiveSuggestions(requestBody, controller.signal); if (!response) { throw new Error('No response from suggestion API'); } const reader = response.getReader(); const decoder = new TextDecoder('utf-8'); + + // Promote out of Pending as soon as the response exists, not on the first chunk. A + // stream that yields zero chunks used to leave the card Pending forever: the terminal + // check below only fires for Loading, and no timeout rescues it because the stream + // ended rather than stalled. An upstream that emits only a block reaches here + // with nothing to yield, since _strip_think_stream swallows the whole buffer. + suggestion.state = SuggestionState.Loading; + this.appendSuggestion(timestamp, suggestion, epoch); + try { while (true) { - // Check if stopped - if (this.abortMap.get(taskId)) { - this.abortMap.delete(taskId); - suggestion.state = SuggestionState.Stopped; - this.appendSuggestion(timestamp, suggestion); - return; - } - const { done, value } = await reader.read(); if (done) break; if (value) { + armStallTimer(SUGGESTION_STALL_MS); const chunk = decoder.decode(value, { stream: true }); suggestion.answer += chunk; - suggestion.state = SuggestionState.Loading; // Update the suggestion - this.appendSuggestion(timestamp, suggestion); + this.appendSuggestion(timestamp, suggestion, epoch); } } - // Mark as successful if not stopped if (suggestion.state === SuggestionState.Loading) { - suggestion.state = SuggestionState.Success; - this.appendSuggestion(timestamp, suggestion); + if (suggestion.answer.length === 0) { + // Indistinguishable from a provider failure, and a stated error beats a card + // that never resolves. + suggestion.state = SuggestionState.Error; + suggestion.error = 'The model returned an empty response.'; + } else { + suggestion.state = SuggestionState.Success; + } + this.appendSuggestion(timestamp, suggestion, epoch); } } finally { + // releaseLock alone does not cancel the body. Undici documents that an unconsumed, + // uncancelled response body leaks the connection and can stall or deadlock later + // requests, which is the whole reason superseding used to break suggestions. + await reader.cancel().catch(() => {}); reader.releaseLock(); } } catch (error) { - console.error('[LiveSuggestionService] Failed to generate suggestion:', error); - suggestion.state = SuggestionState.Error; - suggestion.error = getSuggestionErrorMessage(error); - this.appendSuggestion(timestamp, suggestion); + const aborted = error instanceof Error && error.name === 'AbortError'; + const stalled = + controller.signal.reason instanceof Error && + controller.signal.reason.name === 'TimeoutError'; + + if (aborted && !stalled) { + // Superseded by a newer question. Expected, not a failure. + suggestion.state = SuggestionState.Stopped; + } else { + if (!aborted) { + console.error('[LiveSuggestionService] Failed to generate suggestion:', error); + } + suggestion.state = SuggestionState.Error; + suggestion.error = stalled + ? 'The response timed out. Please try again.' + : getSuggestionErrorMessage(error); + } + this.appendSuggestion(timestamp, suggestion, epoch); + } finally { + if (stallTimer) clearTimeout(stallTimer); + this.abortMap.delete(taskId); } } @@ -125,14 +185,15 @@ class LiveSuggestionService { // Start the background task const taskId = UuidUtil.generate(); - this.abortMap.set(taskId, false); - this.generateSuggestion(taskId, filteredTranscripts); + this.abortMap.set(taskId, new AbortController()); + void this.generateSuggestion(taskId, filteredTranscripts); } stopRunningTasks(): void { - this.abortMap.forEach((_value, key) => { - this.abortMap.set(key, true); - }); + // Aborting tears down the HTTP request itself, so a parked reader.read() rejects + // immediately instead of waiting for a chunk that may never arrive. + this.abortMap.forEach((controller) => controller.abort()); + this.abortMap.clear(); } async stop(): Promise { diff --git a/src/main/services/transcript.service.ts b/src/main/services/transcript.service.ts index 7191ffc5..ece3a498 100644 --- a/src/main/services/transcript.service.ts +++ b/src/main/services/transcript.service.ts @@ -1,4 +1,8 @@ -import { LIVE_SUGGESTION_GAP_MS, TRANSCRIPT_INTER_TRANSCRIPT_GAP_MS } from '../consts.js'; +import { + LIVE_SUGGESTION_GAP_MS, + SELF_PARTIAL_STALE_MS, + TRANSCRIPT_INTER_TRANSCRIPT_GAP_MS, +} from '../consts.js'; import { Speaker, Transcript } from '../types/app-state.js'; import { appStateService } from './app-state.service.js'; import { liveSuggestionService } from './suggestion-live.service.js'; @@ -77,12 +81,27 @@ class TranscriptService { } const lastSelf = cleaned.filter((t) => t.speaker === Speaker.Self).slice(-1)[0]; - if (transcript.speaker === Speaker.Other && transcript.isFinal && !this.selfPartialTranscript) { + if (transcript.speaker === Speaker.Other && transcript.isFinal) { + // These two conditions are the only ways a suggestion is silently suppressed, and + // neither surfaces anywhere. Logged so a field or local repro can distinguish + // "the request was never made" from "the request was made and stalled". + // A partial that has gone quiet is almost certainly orphaned by a dropped ASR socket. + // Treat it as absent rather than letting it gate suggestions indefinitely. + const blockedByPartial = + !!this.selfPartialTranscript && + now - this.selfPartialTranscript.endTimestamp <= SELF_PARTIAL_STALE_MS; const skipDueToRecentSelf = !!lastSelf && lastSelf.isFinal && Date.now() - lastSelf.endTimestamp <= LIVE_SUGGESTION_GAP_MS; - if (!skipDueToRecentSelf) { + + console.info( + `[TranscriptService] suggestion gate: blockedByPartial=${blockedByPartial}` + + ` skipDueToRecentSelf=${skipDueToRecentSelf}` + + ` lastSelfAgeMs=${lastSelf ? Date.now() - lastSelf.endTimestamp : 'none'}` + ); + + if (!blockedByPartial && !skipDueToRecentSelf) { await liveSuggestionService.startGenerateSuggestion(cleaned); } } diff --git a/src/renderer/hooks/use-assistant-service.ts b/src/renderer/hooks/use-assistant-service.ts index 19dd59ac..265f8224 100644 --- a/src/renderer/hooks/use-assistant-service.ts +++ b/src/renderer/hooks/use-assistant-service.ts @@ -47,6 +47,15 @@ export const useAssistantService = create((set) => ({ // Update running state to Running after successful start electron.appState.update({ runningState: RunningState.Running }); } catch (error) { + // Tear down before resetting state. `electron.transcription.start()` may already have + // succeeded, and leaving it active while runningState goes back to Idle strands the app + // in a state where transcripts keep flowing (so live suggestions still fire) but every + // action-suggestion hotkey refuses forever, because those gate on runningState. + await Promise.allSettled([ + liveTranscriptionService.stop(), + electron.transcription.stop(), + ]); + // Reset state to Idle so the button doesn't stay stuck on "Starting..." electron.appState.update({ runningState: RunningState.Idle }); const errorMessage = error instanceof Error ? error.message : 'Failed to start assistant'; From b4baf251326fb6208e07acf954ed9b4c13550e17 Mon Sep 17 00:00:00 2001 From: alpha Date: Thu, 6 Aug 2026 19:11:29 -0500 Subject: [PATCH 2/6] fix(suggestions): close remaining paths that block generation Follow-up to the cancellation work, covering the blocking paths that were deferred from the first pass. - promote an orphaned partial to a final when its ASR socket drops. A reconnect opens a fresh backend session, so the interrupted utterance never gets its final. The staleness guard was only a 15s backstop; this removes the window. The original endTimestamp is kept deliberately - stamping it now would make it the "recent self" the suggestion gate measures against, suppressing the next suggestion at exactly the moment the user is recovering from a dropped socket - cap screenshot capture at CAPTURE_MAX_EDGE_PX. NativeImage.toPNG() is synchronous and runs on the main process, so capturing at full physical resolution stalled the event loop for hundreds of milliseconds per capture, blocking IPC, transcript ingest and any in-flight suggestion stream. Scaling at capture time rather than in sharp afterwards is what makes that cheap - send only the most recent transcripts with a suggestion request. The backend already slices to its own window before building the prompt, so the rest was upload cost that grew for the whole interview. This does not bound retained history: the summary and .docx export read the full transcript from app state - subscribe to app-state broadcasts before the first fetch. refreshState is an IPC round-trip and main never replays, so anything landing during that await was dropped. The subscription now lives for the app's lifetime; tearing it down at zero subscribers reopened the window on every re-init, and StrictMode's double mount plus ordinary route changes drive the count to zero routinely Broadcast coalescing is deliberately not included. test/app-state.test.mjs pins one-broadcast-per-change, and bounding payload size instead would truncate the transcript panel. Both need a design decision, so they belong in their own change. Refs #77 Co-Authored-By: Claude Opus 5 --- src/main/consts.ts | 19 ++++++++++ src/main/ipc/transcript.ts | 3 ++ src/main/preload.cts | 2 ++ .../services/suggestion-action.service.ts | 13 +++++-- src/main/services/suggestion-live.service.ts | 3 +- src/main/services/transcript.service.ts | 31 ++++++++++++++++ src/renderer/hooks/use-app-state.tsx | 36 +++++++++---------- .../services/live-transcription.service.ts | 7 ++++ src/renderer/types/electron-api.d.ts | 1 + 9 files changed, 93 insertions(+), 22 deletions(-) diff --git a/src/main/consts.ts b/src/main/consts.ts index 6cfd7cc5..6bc78929 100644 --- a/src/main/consts.ts +++ b/src/main/consts.ts @@ -25,12 +25,31 @@ export const TRANSCRIPT_INTER_TRANSCRIPT_GAP_MS = 5000; // stop suggestions firing over someone mid-answer, so a short value would regress that. export const SELF_PARTIAL_STALE_MS = 15_000; +// Most recent transcript entries sent with a suggestion request. The backend already slices to +// its own MAX_TRANSCRIPTS_NUM before building the prompt, so everything beyond this was upload +// cost for no effect - and it grew for the whole interview. +// +// Deliberately larger than the backend's window, so a change there does not silently starve the +// prompt. This does NOT bound retained history: the end-of-interview summary and .docx export +// read the full transcript from app state. +export const TRANSCRIPT_UPLOAD_LIMIT = 60; + // Suggestion constants export const LIVE_SUGGESTION_GAP_MS = 2000; export const LIVE_SUGGESTION_NO_SUGGESTION = 'NO_SUGGESTION_NEEDED'; export const ACTION_SUGGESTION_MAX_CAPTURES = 4; export const ACTION_TIMEOUT_MS = 30_000; // 30 seconds +// Longest edge a screenshot is captured at. Capturing at full physical resolution meant +// NativeImage.toPNG() - which is synchronous, on the main process - ran on a 4K bitmap and +// stalled the event loop for hundreds of milliseconds per capture, blocking IPC, transcript +// ingest and any in-flight suggestion stream. It also drove the request payload, which the +// backend then base64-inflates by a third. +// +// Not lower than this: the model has to read code and stack traces off these screenshots, and +// the failure mode of over-shrinking is silent - a confident answer about a blurry image. +export const CAPTURE_MAX_EDGE_PX = 1920; + // Time to first byte. Separate budgets: an action request uploads up to four screenshots and // the backend base64-encodes them before the provider emits a token, so it starts far slower // than a live suggestion. These bound a request that never starts, not total generation time. diff --git a/src/main/ipc/transcript.ts b/src/main/ipc/transcript.ts index 5fffe124..1b4545f8 100644 --- a/src/main/ipc/transcript.ts +++ b/src/main/ipc/transcript.ts @@ -26,6 +26,9 @@ export function registerTranscriptHandlers(): void { ipcMain.handle('transcription:ingest', async (_event, payload) => { await transcriptService.ingest(payload?.channel, payload?.type, payload?.text); }); + ipcMain.handle('transcription:channel-disconnected', async (_event, channel: string) => { + transcriptService.handleChannelDisconnected(channel); + }); ipcMain.handle('transcription:set-session-token', async (_event, sessionToken: string) => { if (!sessionToken) return; const url = BACKEND_BASE_URL.replace(/^ws/i, 'http'); diff --git a/src/main/preload.cts b/src/main/preload.cts index a191475e..e5275d47 100644 --- a/src/main/preload.cts +++ b/src/main/preload.cts @@ -101,6 +101,8 @@ const electronApi = { ipcRenderer.invoke('transcription:ingest', payload), setSessionToken: (token: string) => ipcRenderer.invoke('transcription:set-session-token', token), + channelDisconnected: (channel: 'ch_0' | 'ch_1') => + ipcRenderer.invoke('transcription:channel-disconnected', channel), // Channel names set by the electron-audio-loopback package — cannot be renamed enableLoopbackAudio: () => ipcRenderer.invoke('enable-loopback-audio'), disableLoopbackAudio: () => ipcRenderer.invoke('disable-loopback-audio'), diff --git a/src/main/services/suggestion-action.service.ts b/src/main/services/suggestion-action.service.ts index 53ec5e45..740b945a 100644 --- a/src/main/services/suggestion-action.service.ts +++ b/src/main/services/suggestion-action.service.ts @@ -7,7 +7,9 @@ import { ACTION_SUGGESTION_TTFB_MS, ACTION_TIMEOUT_MS, BACKEND_BASE_URL, + CAPTURE_MAX_EDGE_PX, SUGGESTION_STALL_MS, + TRANSCRIPT_UPLOAD_LIMIT, } from '../consts.js'; import { configStore } from '../store/config.store.js'; import { @@ -209,7 +211,7 @@ export class ActionSuggestionService { config: conf.llmConf, profile_data: interviewConfig.profileData, context: interviewConfig.context, - transcripts: transcripts, + transcripts: transcripts.slice(-TRANSCRIPT_UPLOAD_LIMIT), image_names: [...this.uploadedImageNames], }; @@ -323,12 +325,19 @@ export class ActionSuggestionService { const physicalWidth = Math.round(targetDisplay.size.width * targetDisplay.scaleFactor); const physicalHeight = Math.round(targetDisplay.size.height * targetDisplay.scaleFactor); + // Scale at capture time rather than after. thumbnail.toPNG() below is synchronous and + // runs on the main process, so shrinking the bitmap first is what keeps it from stalling + // the event loop; doing it in sharp afterwards would be too late. + const scale = Math.min(1, CAPTURE_MAX_EDGE_PX / Math.max(physicalWidth, physicalHeight)); + const captureWidth = Math.round(physicalWidth * scale); + const captureHeight = Math.round(physicalHeight * scale); + // desktopCapturer is Electron's built-in screen-capture API. // A timeout guards against indefinite hangs on restricted or virtual display adapters. const sources = await Promise.race([ desktopCapturer.getSources({ types: ['screen'], - thumbnailSize: { width: physicalWidth, height: physicalHeight }, + thumbnailSize: { width: captureWidth, height: captureHeight }, }), new Promise((_, reject) => setTimeout( diff --git a/src/main/services/suggestion-live.service.ts b/src/main/services/suggestion-live.service.ts index f62f4aec..3b1c9566 100644 --- a/src/main/services/suggestion-live.service.ts +++ b/src/main/services/suggestion-live.service.ts @@ -3,6 +3,7 @@ import { LIVE_SUGGESTION_NO_SUGGESTION, LIVE_SUGGESTION_TTFB_MS, SUGGESTION_STALL_MS, + TRANSCRIPT_UPLOAD_LIMIT, } from '../consts.js'; import { configStore } from '../store/config.store.js'; import { LiveSuggestion, Speaker, SuggestionState, Transcript } from '../types/app-state.js'; @@ -89,7 +90,7 @@ class LiveSuggestionService { config: conf.llmConf, profile_data: interviewConfig.profileData, context: interviewConfig.context, - transcripts: transcripts, + transcripts: transcripts.slice(-TRANSCRIPT_UPLOAD_LIMIT), }; armStallTimer(LIVE_SUGGESTION_TTFB_MS); diff --git a/src/main/services/transcript.service.ts b/src/main/services/transcript.service.ts index ece3a498..6506860a 100644 --- a/src/main/services/transcript.service.ts +++ b/src/main/services/transcript.service.ts @@ -109,6 +109,37 @@ class TranscriptService { appStateService.updateState({ transcripts: cleaned }); } + /** + * Promote an in-flight partial to a final when its ASR socket drops. + * + * A reconnect opens a brand-new backend session, so the final for the interrupted utterance + * is never sent. Without this the partial is orphaned: for `ch_1` it gates every live + * suggestion until the candidate next finishes speaking, and its text is silently overwritten + * by the next utterance rather than kept. + */ + handleChannelDisconnected(channelRaw: string): void { + if (!this.isActive) return; + + const speaker = String(channelRaw).toLowerCase() === 'ch_0' ? Speaker.Other : Speaker.Self; + const partial = + speaker === Speaker.Self ? this.selfPartialTranscript : this.otherPartialTranscript; + if (!partial) return; + + // Keep the original endTimestamp. Stamping it with the current time would make this the + // "recent self" the suggestion gate measures against, suppressing the next suggestion for + // LIVE_SUGGESTION_GAP_MS at exactly the moment the user is recovering from a dropped socket. + partial.isFinal = true; + if (speaker === Speaker.Self) { + this.selfTranscripts.push(partial); + this.selfPartialTranscript = null; + } else { + this.otherTranscripts.push(partial); + this.otherPartialTranscript = null; + } + + console.info(`[TranscriptService] promoted orphaned ${channelRaw} partial after disconnect`); + } + async start(): Promise { this.isActive = true; } diff --git a/src/renderer/hooks/use-app-state.tsx b/src/renderer/hooks/use-app-state.tsx index cdeb2c62..723a406c 100644 --- a/src/renderer/hooks/use-app-state.tsx +++ b/src/renderer/hooks/use-app-state.tsx @@ -53,17 +53,24 @@ class AppStateManager { async init() { if (this.initialized) return; this.initialized = true; - await this.refreshState(); + // Subscribe BEFORE the first fetch. refreshState is an IPC round-trip, and main only + // pushes - it never replays - so any broadcast landing during that await was lost. + // Guarding on unsubscribeIPC rather than `initialized` also makes this idempotent under + // HMR, where the manager survives on globalThis and init can run again. if (window.electronAPI?.onAppStateUpdated) { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - this.unsubscribeIPC = window.electronAPI.onAppStateUpdated((raw: any) => { - this.state = this.normalize(raw); - this.emit(); - }); - } else { + if (!this.unsubscribeIPC) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + this.unsubscribeIPC = window.electronAPI.onAppStateUpdated((raw: any) => { + this.state = this.normalize(raw); + this.emit(); + }); + } + } else if (!this.pollingId) { this.pollingId = window.setInterval(() => void this.refreshState(), 1000); } + + await this.refreshState(); } async refreshState() { @@ -95,19 +102,10 @@ class AppStateManager { // emit current value synchronously fn(this.state); return () => { + // Deliberately keep the IPC subscription for the app's lifetime. Tearing it down at zero + // subscribers reopened the drop window on every re-init, and StrictMode's double mount + // plus ordinary route changes both drive the count to zero routinely. this.subscribers.delete(fn); - if (this.subscribers.size === 0) { - // stop polling/ipc when no subscribers - if (this.pollingId) { - clearInterval(this.pollingId); - this.pollingId = null; - } - if (this.unsubscribeIPC) { - this.unsubscribeIPC(); - this.unsubscribeIPC = null; - } - this.initialized = false; - } }; } } diff --git a/src/renderer/services/live-transcription.service.ts b/src/renderer/services/live-transcription.service.ts index 04371153..153d80db 100644 --- a/src/renderer/services/live-transcription.service.ts +++ b/src/renderer/services/live-transcription.service.ts @@ -219,6 +219,13 @@ class AudioWsStream { ws.onclose = () => { if (this.stopping || !this.active) return; + + // A reconnect starts a fresh backend session, so any in-flight utterance never gets its + // final. Tell main to close it out, or the orphaned partial gates live suggestions. + getElectron() + ?.transcription.channelDisconnected(this.channel) + .catch((error) => console.error('Failed to report channel disconnect:', error)); + this.scheduleReconnect(); }; } diff --git a/src/renderer/types/electron-api.d.ts b/src/renderer/types/electron-api.d.ts index 18cdb4fe..12792009 100644 --- a/src/renderer/types/electron-api.d.ts +++ b/src/renderer/types/electron-api.d.ts @@ -109,6 +109,7 @@ declare global { text: string; }) => Promise; setSessionToken: (token: string) => Promise; + channelDisconnected: (channel: 'ch_0' | 'ch_1') => Promise; enableLoopbackAudio: () => Promise; disableLoopbackAudio: () => Promise; }; From 54d46274d3eca94ef3ccfb5f10f2aace18ff48cd Mon Sep 17 00:00:00 2001 From: alpha Date: Thu, 6 Aug 2026 19:21:53 -0500 Subject: [PATCH 3/6] perf(state): coalesce renderer broadcasts Every broadcast structured-clones the whole renderer state, and they fire on each streamed token and each ASR partial - roughly 20/second across two channels, against a transcript array that grows for the whole interview. That cost scaled with events and with session length, which is the shape of a stall that only shows up late in a long interview. Coalescing on a short timer bounds it per unit time instead. 50ms is short enough that streaming still reads as streaming. This changes a contract that test/app-state.test.mjs pinned, so the test moves with it: it now flushes explicitly rather than counting one send per mutation. The two invariants it actually protects are unchanged and still asserted - the CV never reaches the renderer, and an update that changes nothing does not broadcast. flushRenderer is deliberately a no-op when nothing is scheduled, so flushing cannot manufacture a broadcast that change detection suppressed. Refs #77 Co-Authored-By: Claude Opus 5 --- src/main/services/app-state.service.ts | 28 ++++++++++++++++++++++++++ test/app-state.test.mjs | 19 +++++++++++++---- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/src/main/services/app-state.service.ts b/src/main/services/app-state.service.ts index 6a88fd70..46292a3b 100644 --- a/src/main/services/app-state.service.ts +++ b/src/main/services/app-state.service.ts @@ -29,8 +29,15 @@ const DEFAULT_STATE: AppState = { interviewConfigLoaded: false, }; +// Every broadcast structured-clones the whole renderer state, and they fire on each streamed +// token and each ASR partial - roughly 20/second across two channels, against a transcript array +// that grows for the whole interview. Coalescing bounds that cost per unit time instead of per +// event. Short enough that streaming still reads as streaming. +const BROADCAST_COALESCE_MS = 50; + export class AppStateService { private state: AppState; + private broadcastTimer: ReturnType | null = null; constructor() { this.state = { ...DEFAULT_STATE }; @@ -116,6 +123,27 @@ export class AppStateService { } private notifyRenderer(): void { + if (this.broadcastTimer) return; + + this.broadcastTimer = setTimeout(() => { + this.broadcastTimer = null; + this.flushRenderer(); + }, BROADCAST_COALESCE_MS); + } + + /** + * Send a pending broadcast immediately, if one is scheduled. + * + * Coalescing means a caller that needs the renderer to have the current state right now - a + * test, or a shutdown path - cannot simply wait. Deliberately a no-op when nothing is + * pending, so flushing cannot manufacture a broadcast that the change detection suppressed. + */ + flushRenderer(): void { + if (!this.broadcastTimer) return; + + clearTimeout(this.broadcastTimer); + this.broadcastTimer = null; + try { const win = getWindowReference(); if (win && !win.isDestroyed()) { diff --git a/test/app-state.test.mjs b/test/app-state.test.mjs index 77ee53cb..ee363e94 100644 --- a/test/app-state.test.mjs +++ b/test/app-state.test.mjs @@ -1,8 +1,12 @@ /** - * The whole app state is broadcast to the renderer on every change, and the health-check loops - * fire every 1-5s. The interview config holds a CV and job description (up to 128k chars each), - * so main must send a summary rather than the real thing, and must not broadcast at all when - * nothing actually changed. + * The whole app state is broadcast to the renderer, and the health-check loops fire every 1-5s. + * The interview config holds a CV and job description (up to 128k chars each), so main must send + * a summary rather than the real thing, and must not broadcast at all when nothing changed. + * + * Broadcasts are coalesced on a short timer rather than sent per change: they fire on every + * streamed token and every ASR partial, and each one clones a transcript array that grows for + * the whole interview. So these checks flush explicitly rather than counting one send per + * mutation. The two invariants above are unaffected and are what this file pins. */ import { createChecker, loadMain } from './helpers.mjs'; @@ -37,6 +41,7 @@ export async function run() { check('renderer view drops context', !('context' in view.interviewConfig)); check('renderer view keeps the loaded flag', view.interviewConfigLoaded === true); + appStateService.flushRenderer(); const payload = JSON.stringify(sent.at(-1).payload); check(`broadcast stays small (${payload.length} bytes)`, payload.length < 5_000); check('broadcast carries no CV', !payload.includes('xxxxxxxxxx')); @@ -45,14 +50,18 @@ export async function run() { // updates first so the loop below carries no new information at all. appStateService.updateState({ isBackendLive: true }); appStateService.updateState({ credits: 42, userRole: 'user', providedLLMModel: 'm' }); + appStateService.flushRenderer(); const baseline = sent.length; for (let i = 0; i < 10; i++) { appStateService.updateState({ isBackendLive: true }); appStateService.updateState({ credits: 42, userRole: 'user', providedLLMModel: 'm' }); } + // No-op updates must not even schedule a broadcast, so flushing produces nothing. + appStateService.flushRenderer(); check('identical updates do not broadcast', sent.length === baseline); appStateService.updateState({ isBackendLive: false }); + appStateService.flushRenderer(); check('a real change still broadcasts', sent.length === baseline + 1); check('the change is applied', appStateService.getState().isBackendLive === false); @@ -66,8 +75,10 @@ export async function run() { // the broadcast, so the reset landed in main and the renderer kept showing the old content // until something unrelated happened to broadcast. appStateService.updateState({ transcripts: [{ timestamp: 1, text: 'stale', speaker: 'self' }] }); + appStateService.flushRenderer(); const beforeClear = sent.length; appStateService.setPlaceholderState(); + appStateService.flushRenderer(); check('clearing broadcasts to the renderer', sent.length === beforeClear + 1); check( 'the broadcast carries the cleared transcripts', From 19f6712a7c000781c87be914809322876dc98c3c Mon Sep 17 00:00:00 2001 From: alpha Date: Thu, 6 Aug 2026 20:17:29 -0500 Subject: [PATCH 4/6] perf(panels): skip rendering work for off-screen rows All three panels rendered every row on every state broadcast, so layout and paint cost grew with session length and the UI got sluggish late in a long interview. Transcripts grow fastest - one row per ASR final - but the suggestion panels carry markdown, code blocks and screenshots, so their per-row cost is far higher even at smaller counts. Uses content-visibility rather than a windowing library. Rows wrap to variable heights, which fixed-height windowing mis-measures, and `auto` in contain-intrinsic-size remembers each row's last rendered size, so scroll position and scrollIntoView stay accurate without measurement plumbing or a new dependency. Off-screen rows skip style, layout and paint while staying in the DOM, so scrollback is fully preserved. On the suggestion panels this goes on the row content rather than SuggestionReveal's wrapper, which is an animating grid that size containment would fight. The newest card - the streaming one - is always on screen, so containment never applies while it is being written. Refs #77 Co-Authored-By: Claude Opus 5 --- .../custom/panels/action-suggestions-panel.tsx | 4 +++- .../custom/panels/live-suggestions-panel.tsx | 6 +++++- .../components/custom/panels/transcript-panel.tsx | 10 +++++++++- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/renderer/components/custom/panels/action-suggestions-panel.tsx b/src/renderer/components/custom/panels/action-suggestions-panel.tsx index 0390ff13..a47b3d20 100644 --- a/src/renderer/components/custom/panels/action-suggestions-panel.tsx +++ b/src/renderer/components/custom/panels/action-suggestions-panel.tsx @@ -214,7 +214,9 @@ function ActionSuggestionsPanel({ animate={s.timestamp > lastRevealedAt} className="border-b border-border/40 last:border-0" > -
+ {/* Same as the live panel: skip off-screen work. Worth more per card here, + since these carry screenshots and markdown with code blocks. */} +
{idx === 0 && (s.state === SuggestionState.Pending || s.state === SuggestionState.Loading) ? ( diff --git a/src/renderer/components/custom/panels/live-suggestions-panel.tsx b/src/renderer/components/custom/panels/live-suggestions-panel.tsx index 9a10f3ee..459f0559 100644 --- a/src/renderer/components/custom/panels/live-suggestions-panel.tsx +++ b/src/renderer/components/custom/panels/live-suggestions-panel.tsx @@ -204,7 +204,11 @@ function LiveSuggestionsPanel({ animate={s.timestamp > lastRevealedAt} className="border-b border-border/40 last:border-0" > -
+ {/* Skip style, layout and paint for cards scrolled out of view. Applied here + rather than on SuggestionReveal's wrapper, which is an animating grid that + size containment would fight. The newest card - the streaming one - is + always on screen, so containment never applies to it. */} +
{idx === 0 && (s.state === SuggestionState.Pending || s.state === SuggestionState.Loading) ? ( diff --git a/src/renderer/components/custom/panels/transcript-panel.tsx b/src/renderer/components/custom/panels/transcript-panel.tsx index 1b49d07c..9a7d037e 100644 --- a/src/renderer/components/custom/panels/transcript-panel.tsx +++ b/src/renderer/components/custom/panels/transcript-panel.tsx @@ -81,7 +81,15 @@ function TranscriptPanel({ transcripts, isRunning = false }: TranscriptPanelProp <>
{transcripts.map((item, idx) => ( -
+ // content-visibility skips style, layout and paint for rows scrolled out of + // view, which is the cost that grew with transcript length. Chosen over a + // windowing library because rows wrap to variable heights: `auto` in + // contain-intrinsic-size remembers each row's last rendered size, so scroll + // position and scrollIntoView stay accurate without measurement plumbing. +
{/* The dock is wide and short, so the speaker rides inline with the words rather than spending a line of its own on them. */}

From ccf4f1b6bdff61efc0210b2b3031c34c390c05ef Mon Sep 17 00:00:00 2001 From: alpha Date: Thu, 6 Aug 2026 20:23:32 -0500 Subject: [PATCH 5/6] fix(suggestions): detect aborts from the signal, not the error name An aborted fetch rejects with the abort *reason*, so a stall abort surfaces as TimeoutError rather than AbortError. The name checks therefore missed stalls: requestStream wrapped a deliberate cancellation as a network failure and logged it, and the services reported aborted=false and logged a spurious error for a timeout they had themselves requested. Verified against a server that sends one chunk then goes silent: reader.read() rejects within milliseconds of the abort carrying name=TimeoutError, confirming both the mechanism and the misdetection. Keying on signal.aborted covers both reasons. A five-deep supersede chain against that server now resolves as four stopped plus one timed-out, leaves the abort map empty, and closes every socket - which is the leak that made superseding break suggestion generation. Refs #77 Co-Authored-By: Claude Opus 5 --- src/main/api/client.ts | 6 +++++- src/main/services/suggestion-action.service.ts | 5 ++++- src/main/services/suggestion-live.service.ts | 6 +++++- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/main/api/client.ts b/src/main/api/client.ts index c9286bb7..fa49d804 100644 --- a/src/main/api/client.ts +++ b/src/main/api/client.ts @@ -239,7 +239,11 @@ export class ApiClient { // A supersede or stall abort is deliberate. Let it through untouched so callers can // read `signal.reason` to tell the two apart, and do not log it as a failure. - if (error instanceof Error && error.name === 'AbortError') { + // + // Keyed on the signal, not the error name: an abort rejects with the *reason*, so a + // stall abort surfaces as `TimeoutError` rather than `AbortError` and a name check + // would miss it and wrap it as a network failure. + if (signal?.aborted) { throw error; } diff --git a/src/main/services/suggestion-action.service.ts b/src/main/services/suggestion-action.service.ts index 740b945a..56773985 100644 --- a/src/main/services/suggestion-action.service.ts +++ b/src/main/services/suggestion-action.service.ts @@ -284,8 +284,11 @@ export class ActionSuggestionService { reader.releaseLock(); } } catch (error) { - const aborted = error instanceof Error && error.name === 'AbortError'; + // See the live-suggestion service: an abort rejects with the reason, so the signal is + // the reliable source, not the error name. + const aborted = controller.signal.aborted; const stalled = + aborted && controller.signal.reason instanceof Error && controller.signal.reason.name === 'TimeoutError'; diff --git a/src/main/services/suggestion-live.service.ts b/src/main/services/suggestion-live.service.ts index 3b1c9566..39e31e45 100644 --- a/src/main/services/suggestion-live.service.ts +++ b/src/main/services/suggestion-live.service.ts @@ -143,8 +143,12 @@ class LiveSuggestionService { reader.releaseLock(); } } catch (error) { - const aborted = error instanceof Error && error.name === 'AbortError'; + // Keyed on the signal rather than the error name. An abort rejects with the *reason*, + // so a stall surfaces as TimeoutError and a check for AbortError would miss it and + // report a deliberate cancellation as a network failure. + const aborted = controller.signal.aborted; const stalled = + aborted && controller.signal.reason instanceof Error && controller.signal.reason.name === 'TimeoutError'; From a9fc144c28a02675cef281119608767ff6ff6a71 Mon Sep 17 00:00:00 2001 From: alpha Date: Thu, 6 Aug 2026 20:38:33 -0500 Subject: [PATCH 6/6] refactor(suggestions): pass the controller instead of looking it up Review of the diff turned up a latent lock leak. generateSuggestion looked its controller up in the abort map and returned early if it was missing - before the finally that releases the action lock, and resolving normally so the caller's .catch could not release it either. The result would be a permanently held lock disabling F9, F11 and F12 for the session, which is the exact bug this service is being fixed for. Not reachable today: the map entry is set immediately before the call and nothing awaits in between. But it becomes reachable with any future await or reorder, and the failure is silent and unrecoverable. Passing the controller removes the not-found branch entirely, so there is no path that can skip the release. Same shape applied to the live service, and its redundant empty-transcript guard dropped for the same reason - the caller already returns before registering a task, and a second check would have leaked the abort map entry. Verified the lock is released on every exit path: normal completion, network error, abort mid-stream, stall abort, and a throw before the try block. Also tidied the transcript gate: two comments had merged into one unreadable block, and adjacent conditions were reading the clock from different sources. Refs #77 Co-Authored-By: Claude Opus 5 --- .../services/suggestion-action.service.ts | 20 +++++++++------- src/main/services/suggestion-live.service.ts | 23 +++++++++---------- src/main/services/transcript.service.ts | 12 +++++----- 3 files changed, 29 insertions(+), 26 deletions(-) diff --git a/src/main/services/suggestion-action.service.ts b/src/main/services/suggestion-action.service.ts index 56773985..69f56e59 100644 --- a/src/main/services/suggestion-action.service.ts +++ b/src/main/services/suggestion-action.service.ts @@ -154,12 +154,13 @@ export class ActionSuggestionService { this.stopRunningTasks(); const taskId = UuidUtil.generate(); - this.abortMap.set(taskId, new AbortController()); + const controller = new AbortController(); + this.abortMap.set(taskId, controller); // generateSuggestion owns the release, but it can throw before reaching its own try/finally // (config reads, state reads, the first setSuggestion). Releasing here on a synchronous // rejection keeps a leaked lock from disabling all three action hotkeys for the session. - this.generateSuggestion(taskId, appState.transcripts).catch((error) => { + this.generateSuggestion(taskId, controller, appState.transcripts).catch((error) => { console.error('[ActionSuggestionService] generateSuggestion rejected:', error); actionLockService.release(ActionType.CaptureSuggestion); }); @@ -197,12 +198,15 @@ export class ActionSuggestionService { return ''; } - private async generateSuggestion(taskId: string, transcripts: Transcript[]): Promise { - const controller = this.abortMap.get(taskId); - if (!controller) { - return; - } - + private async generateSuggestion( + taskId: string, + controller: AbortController, + transcripts: Transcript[] + ): Promise { + // The controller is passed in rather than looked up. A lookup needs a not-found branch, + // and that branch would return before reaching the finally that releases the action lock - + // leaking it permanently and disabling all three hotkeys, which is the exact bug this + // service is being fixed for. const timestamp = DateTimeUtil.now(); const conf = configStore.getConfig(); const interviewConfig = appStateService.getState().interviewConfig; diff --git a/src/main/services/suggestion-live.service.ts b/src/main/services/suggestion-live.service.ts index 39e31e45..9b873ee1 100644 --- a/src/main/services/suggestion-live.service.ts +++ b/src/main/services/suggestion-live.service.ts @@ -49,17 +49,15 @@ class LiveSuggestionService { }); } - private async generateSuggestion(taskId: string, transcripts: Transcript[]): Promise { - if (!transcripts || transcripts.length === 0) { - return; - } - + private async generateSuggestion( + taskId: string, + controller: AbortController, + transcripts: Transcript[] + ): Promise { + // No empty-transcript guard here on purpose. startGenerateSuggestion already returns + // before registering a task, and a second check would return ahead of the finally that + // clears the abort map entry, leaking it. const epoch = this.epoch; - const controller = this.abortMap.get(taskId); - if (!controller) { - return; - } - const timestamp = DateTimeUtil.now(); const suggestion: LiveSuggestion = { timestamp, @@ -190,8 +188,9 @@ class LiveSuggestionService { // Start the background task const taskId = UuidUtil.generate(); - this.abortMap.set(taskId, new AbortController()); - void this.generateSuggestion(taskId, filteredTranscripts); + const controller = new AbortController(); + this.abortMap.set(taskId, controller); + void this.generateSuggestion(taskId, controller, filteredTranscripts); } stopRunningTasks(): void { diff --git a/src/main/services/transcript.service.ts b/src/main/services/transcript.service.ts index 6506860a..24192d7d 100644 --- a/src/main/services/transcript.service.ts +++ b/src/main/services/transcript.service.ts @@ -85,20 +85,20 @@ class TranscriptService { // These two conditions are the only ways a suggestion is silently suppressed, and // neither surfaces anywhere. Logged so a field or local repro can distinguish // "the request was never made" from "the request was made and stalled". - // A partial that has gone quiet is almost certainly orphaned by a dropped ASR socket. - // Treat it as absent rather than letting it gate suggestions indefinitely. + + // A partial that has gone quiet is almost certainly orphaned by a dropped ASR socket + // whose final never arrived. Treat it as absent rather than gating indefinitely. const blockedByPartial = !!this.selfPartialTranscript && now - this.selfPartialTranscript.endTimestamp <= SELF_PARTIAL_STALE_MS; + const selfAgeMs = lastSelf ? now - lastSelf.endTimestamp : null; const skipDueToRecentSelf = - !!lastSelf && - lastSelf.isFinal && - Date.now() - lastSelf.endTimestamp <= LIVE_SUGGESTION_GAP_MS; + !!lastSelf && lastSelf.isFinal && selfAgeMs !== null && selfAgeMs <= LIVE_SUGGESTION_GAP_MS; console.info( `[TranscriptService] suggestion gate: blockedByPartial=${blockedByPartial}` + ` skipDueToRecentSelf=${skipDueToRecentSelf}` + - ` lastSelfAgeMs=${lastSelf ? Date.now() - lastSelf.endTimestamp : 'none'}` + ` lastSelfAgeMs=${selfAgeMs ?? 'none'}` ); if (!blockedByPartial && !skipDueToRecentSelf) {