diff --git a/.gitignore b/.gitignore index bcebf0a..aa739be 100644 --- a/.gitignore +++ b/.gitignore @@ -31,4 +31,7 @@ playwright-report/ test-results/ # Generated workflow artifacts -workflow-report.md \ No newline at end of file +workflow-report.md + +# Cursor +.cursor/hooks/state/ diff --git a/AGENTS.md b/AGENTS.md index 6b75d92..a12c6de 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -184,6 +184,44 @@ Previously, single-character scenarios used free-form text with inline translati --- +## Abort / Cancellation Strategy for Audio Requests + +### Why this exists +Audio flows can have races: a user may cancel, close/reopen a modal, or start a new 'turn' while a previous Gemini request is still in-flight. If the stale request resolves after the user moved on, it can incorrectly update UI state (wrong transcript/messages/spinners) or throw JSON parsing errors. + +This section is a developer-facing rule to prevent that entire class of bug. + +### Required strategy (use for every future audio request) +1. Create a new `AbortController` per 'turn/request' and store it in a ref that cancellation/timeout handlers can reach. +2. Pass the per-request `signal` into the Gemini SDK via `config.abortSignal` on *every* relevant SDK call (`ai.models.generateContent(...)` and `chatSession.sendMessage(...)`). + - Do not rely on `Promise.race` / wrapper rejection alone. The Gemini SDK must receive the signal so it can stop internally and reject with `AbortError`. +3. Invalidate/discard stale responses: + - Track a request token (e.g. `requestIdRef.current` captured into `currentRequestId`) and check it before any state updates. + - If a newer request started (token changed) or the relevant UI is no longer open, return early and do not mutate UI state. +4. Preserve JSON enforcement when passing per-request config with `abortSignal`: + - Keep `responseMimeType: 'application/json'` and `responseSchema: ...` set in the same request config. + - This avoids the SDK returning plain text (which breaks downstream JSON parsing/validation). +5. Handle `AbortError` so it doesn’t surface to the UI: + - Treat `AbortError` as intentional cancellation and `return` silently. + - Do not switch to generic ERROR UI or show error flashes for intentional aborts. + +### Example: scenario description recording (abort + stale discard) +In the scenario description 'describe by voice' flow: +- Each transcription attempt creates a fresh `AbortController` (`scenarioDescriptionAbortControllerRef`) and increments a request token (`scenarioDescriptionRequestIdRef`). +- The in-flight call passes `abortController.signal` into `transcribeAndCleanupAudio(...)`. +- After awaiting, results are discarded if `currentRequestId !== scenarioDescriptionRequestIdRef.current` or if the modal is closed (`scenarioSetupOpenRef`). +- In `catch`, `AbortError` is ignored, and only non-abort failures show errors / enable retry. +- In `finally`, the transcription spinner is only cleared when the request token still matches (so stale requests can’t affect UI after close+reopen). + +This is the same overall strategy used for the main mic audio flow: per-turn `AbortController`, request-token guarded state updates, and explicit `AbortError` suppression to avoid UI regressions. + +### Related files +- `App.tsx` (main mic + scenario description cancellation/discard logic) +- `services/geminiService.ts` (`transcribeAndCleanupAudio`, `sendVoiceMessage` per-request `config.abortSignal`, and JSON enforcement config) +- `__tests__/scenarioDescriptionRecordingAbortDiscard.test.tsx` / `__tests__/transcribeAndCleanupAudioAbortSignal.test.ts` (abort + discard + config preservation) + +--- + ## Missing AI Credentials Handling **Location:** `App.tsx` handlers, `components/ScenarioSetup.tsx`, `components/AdPersuasionSetup.tsx` diff --git a/App.tsx b/App.tsx index 9bd245f..a66a7db 100644 --- a/App.tsx +++ b/App.tsx @@ -86,6 +86,9 @@ const App: React.FC = () => { // TEF Questioning conversation timer (5-minute limit) const handleTefQuestioningTimeUp = useCallback(() => { + // Treat this as an intentional abort so processAudioMessage doesn't + // fall through into ERROR UI on AbortError. + processingAbortedRef.current = true; if (abortControllerRef.current) { abortControllerRef.current.abort(); abortControllerRef.current = null; @@ -119,6 +122,10 @@ const App: React.FC = () => { const scenarioRecordingRef = useRef(false); const scenarioSetupOpenRef = useRef(false); + // Abort + stale guarding for scenario description transcription requests. + const scenarioDescriptionAbortControllerRef = useRef(null); + const scenarioDescriptionRequestIdRef = useRef(0); + // Ref to track if processing was aborted by the user const processingAbortedRef = useRef(false); // Request ID counter to detect stale responses from previous requests @@ -596,6 +603,13 @@ const App: React.FC = () => { }; // Scenario mode handlers + const abortScenarioDescriptionTranscription = useCallback(() => { + if (scenarioDescriptionAbortControllerRef.current) { + scenarioDescriptionAbortControllerRef.current.abort(); + scenarioDescriptionAbortControllerRef.current = null; + } + }, []); + const handleOpenScenarioSetup = () => { scenarioSetupOpenRef.current = true; setScenarioMode('setup'); @@ -608,6 +622,10 @@ const App: React.FC = () => { }; const handleCloseScenarioSetup = () => { + // Invalidate any in-flight transcription so late-resolving promises + // cannot overwrite transcript state after close+reopen. + scenarioDescriptionRequestIdRef.current += 1; + abortScenarioDescriptionTranscription(); scenarioSetupOpenRef.current = false; setScenarioMode('none'); setScenarioDescription(''); @@ -662,16 +680,29 @@ const App: React.FC = () => { * Process description audio (from recording or retry) and transcribe it */ const processDescriptionAudio = async (audioData: AudioData): Promise => { + // Increment request ID so we can ignore stale results even if the modal + // is closed and then re-opened while a previous transcription is still in-flight. + const currentRequestId = ++scenarioDescriptionRequestIdRef.current; + + // Abort any previous transcription attempt (e.g., quick retry or close+reopen races). + abortScenarioDescriptionTranscription(); + const abortController = new AbortController(); + scenarioDescriptionAbortControllerRef.current = abortController; + setIsTranscribingDescription(true); try { const { base64, mimeType } = audioData; // Single LLM call to transcribe and clean up the audio - using Gemini - const { rawTranscript: rawText, cleanedTranscript: cleanedText } = await transcribeAndCleanupAudio(base64, mimeType); + const { rawTranscript: rawText, cleanedTranscript: cleanedText } = await transcribeAndCleanupAudio( + base64, + mimeType, + abortController.signal + ); - // Modal was closed while transcription was in-flight; discard results - if (!scenarioSetupOpenRef.current) { + // Discard results if a newer request started or the modal has been closed. + if (currentRequestId !== scenarioDescriptionRequestIdRef.current || !scenarioSetupOpenRef.current) { return; } @@ -690,6 +721,16 @@ const App: React.FC = () => { setLastDescriptionAudio(null); setShowTranscriptOptions(true); } catch (error) { + // If this was an intentional cancellation, discard silently. + const errName = error instanceof DOMException ? error.name : (error as any)?.name; + if (errName === 'AbortError') { + return; + } + + if (currentRequestId !== scenarioDescriptionRequestIdRef.current || !scenarioSetupOpenRef.current) { + return; + } + console.error('Error transcribing description:', error); if (scenarioSetupOpenRef.current) { // Enable retry with the stored audio @@ -697,7 +738,15 @@ const App: React.FC = () => { showErrorFlash('Failed to transcribe audio. Please try again.'); } } finally { - setIsTranscribingDescription(false); + // Only clear the spinner for the latest attempt; stale requests must not + // affect transcript UI state after a close+reopen race. + if (currentRequestId === scenarioDescriptionRequestIdRef.current) { + setIsTranscribingDescription(false); + } + + if (scenarioDescriptionAbortControllerRef.current === abortController) { + scenarioDescriptionAbortControllerRef.current = null; + } } }; @@ -752,6 +801,10 @@ const App: React.FC = () => { const handleCancelRecordingDescription = () => { scenarioRecordingRef.current = false; setIsRecordingDescription(false); + // Invalidate any in-flight transcription and clear spinner immediately. + scenarioDescriptionRequestIdRef.current += 1; + setIsTranscribingDescription(false); + abortScenarioDescriptionTranscription(); cancelRecording(); }; @@ -1086,6 +1139,9 @@ const App: React.FC = () => { }; const handleExitTefQuestioning = () => { + // Treat this as an intentional abort so processAudioMessage doesn't + // fall through into ERROR UI on AbortError. + processingAbortedRef.current = true; if (abortControllerRef.current) { abortControllerRef.current.abort(); abortControllerRef.current = null; @@ -1096,6 +1152,7 @@ const App: React.FC = () => { const handleDismissTefQuestioningSummary = () => { // Abort any in-flight processing or recording + processingAbortedRef.current = true; if (abortControllerRef.current) { abortControllerRef.current.abort(); abortControllerRef.current = null; diff --git a/__tests__/scenarioDescriptionRecordingAbortDiscard.test.tsx b/__tests__/scenarioDescriptionRecordingAbortDiscard.test.tsx new file mode 100644 index 0000000..8e803da --- /dev/null +++ b/__tests__/scenarioDescriptionRecordingAbortDiscard.test.tsx @@ -0,0 +1,320 @@ +/** + * TDD tests for ScenarioSetup "describe by voice" transcription cancellation. + * + * The bug to fix: + * - Closing ScenarioSetup while transcription is in-flight must abort the request + * - Aborted/stale transcription results must NOT update state after a close+reopen + * - Subsequent scenario description recordings must work without being blocked + * + * Approach: + * - Render App and drive the ScenarioSetup modal via UI interactions + * - Mock useAudio so recording/stop are deterministic (no MediaRecorder / mic) + * - Mock @google/genai models.generateContent to return deferred promises that + * only resolve when the test resolves them + * - Ensure that after resolving the first (stale) transcription, the UI still + * reflects the second in-flight transcription (i.e., no stale overwrite) + * + * Tests FAIL before the abort/discard fix is implemented. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, fireEvent, waitFor, act } from '@testing-library/react'; + +type Deferred = { + promise: Promise; + resolve: (value: T) => void; + reject: (reason?: unknown) => void; +}; + +const createDeferred = (): Deferred => { + let resolve!: (value: T) => void; + let reject!: (reason?: unknown) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +}; + +const FAKE_AUDIO_BASE64 = 'ZmFrZWF1ZGlv'; +const FAKE_MIME_TYPE = 'audio/webm'; + +// --------------------------------------------------------------------------- +// Mock @google/genai so Gemini transcription calls are fully controlled +// --------------------------------------------------------------------------- +vi.mock('@google/genai', async (importActual) => { + const actual = await importActual(); + return { + ...actual, + GoogleGenAI: vi.fn(), + }; +}); + +import { GoogleGenAI } from '@google/genai'; + +let transcriptionCalls: Array<{ + deferred: Deferred<{ text: string }>; + abortSignal?: AbortSignal; +}> = []; + +// Some tests need to simulate "late resolve even after abort" to verify +// request-id based discard works on close+reopen races. +let rejectOnAbort = true; + +const mockGenerateContent = vi.fn().mockImplementation((request: any) => { + const abortSignal: AbortSignal | undefined = request?.config?.abortSignal; + const deferred = createDeferred<{ text: string }>(); + + transcriptionCalls.push({ deferred, abortSignal }); + + // If the app wires abortSignal through to Gemini, we reject when aborted. + if (abortSignal) { + abortSignal.addEventListener('abort', () => { + if (rejectOnAbort) { + deferred.reject(new DOMException('Request aborted', 'AbortError')); + } + }); + } + + return deferred.promise; +}); + +const mockAi = { + models: { + generateContent: mockGenerateContent, + }, + chats: { + create: vi.fn().mockReturnValue({ sendMessage: vi.fn() }), + }, +}; + +vi.mocked(GoogleGenAI).mockReturnValue(mockAi as unknown as GoogleGenAI); + +// --------------------------------------------------------------------------- +// Mock Vaul so PracticeModeSheet can be imported without the real library. +// (Some environments don't resolve optional deps during Vitest transforms.) +// --------------------------------------------------------------------------- +vi.mock('vaul', () => { + const passthrough: React.FC<{ children?: React.ReactNode }> = ({ children }) => <>{children}; + + const Root: React.FC<{ open: boolean; onOpenChange?: (open: boolean) => void; children?: React.ReactNode }> = ({ + children, + }) => <>{children}; + + const Overlay: React.FC<{ className?: string; children?: React.ReactNode }> = ({ children }) => ( + <>{children} + ); + + const Content: React.FC<{ children?: React.ReactNode }> = ({ children }) => <>{children}; + + const Title: React.FC<{ asChild?: boolean; children?: React.ReactNode }> = ({ children }) => <>{children}; + const Description: React.FC<{ children?: React.ReactNode; className?: string }> = ({ children }) => <>{children}; + + return { + Drawer: { + Root, + Portal: passthrough, + Overlay, + Content, + Title, + Description, + }, + }; +}); + +// --------------------------------------------------------------------------- +// Mock useAudio so recording flows don't touch real browser APIs +// --------------------------------------------------------------------------- +const mockStartRecording = vi.fn().mockResolvedValue(undefined); +const mockStopRecording = vi.fn().mockResolvedValue({ + base64: FAKE_AUDIO_BASE64, + mimeType: FAKE_MIME_TYPE, +}); +const mockCancelRecording = vi.fn(); +const mockGetAudioContext = vi.fn(); +const mockCheckMicrophonePermission = vi.fn().mockResolvedValue('granted'); +const mockRequestMicrophonePermission = vi.fn().mockResolvedValue(true); + +vi.mock('../hooks/useAudio', () => { + return { + useAudio: () => ({ + isRecording: false, + isPlaying: false, + volume: 0, + startRecording: mockStartRecording, + stopRecording: mockStopRecording, + cancelRecording: mockCancelRecording, + getAudioContext: mockGetAudioContext, + checkMicrophonePermission: mockCheckMicrophonePermission, + requestMicrophonePermission: mockRequestMicrophonePermission, + }), + }; +}); + +// --------------------------------------------------------------------------- +// Mock PracticeModeSheet to avoid Vaul dependency during RTL +// --------------------------------------------------------------------------- +vi.mock('../components/PracticeModeSheet.tsx', () => { + return { + PracticeModeSheet: ({ + open, + onOpenChange, + onSelectMode, + }: { + open: boolean; + onOpenChange: (open: boolean) => void; + onSelectMode: (modeId: 'ad-persuasion' | 'role-play' | 'ad-questioning') => void; + }) => { + if (!open) return null; + return ( +
+ +
+ ); + }, + }; +}); + +beforeAll(() => { + // ScenarioSetup scrolls transcript options into view; JSDOM doesn't provide it. + // @ts-expect-error - prototype patch for tests + if (!Element.prototype.scrollIntoView) Element.prototype.scrollIntoView = vi.fn(); +}); + +describe('ScenarioSetup · describe by voice abort + discard', () => { + beforeEach(() => { + localStorage.setItem('parle_api_key_gemini', 'test-key-scenario-abort'); + localStorage.setItem('parle_api_key_openai', 'test-key-openai'); + transcriptionCalls = []; + rejectOnAbort = true; + mockGenerateContent.mockClear(); + mockStartRecording.mockClear(); + mockStopRecording.mockClear(); + mockCancelRecording.mockClear(); + }); + + afterEach(() => { + localStorage.clear(); + vi.restoreAllMocks(); + }); + + it('aborts in-flight transcription on close and prevents stale results overwriting the next attempt', async () => { + const { default: App } = await import('../App'); + render(); + + // Open PracticeModeSheet -> Role Play -> ScenarioSetup modal + fireEvent.click(screen.getByRole('button', { name: /Start Practice/i })); + fireEvent.click(await screen.findByRole('button', { name: /Role Play/i })); + + await screen.findByText(/Practice Role Play/i); + + // Start first "describe by voice" transcription + fireEvent.click(screen.getByRole('button', { name: /Or describe by voice/i })); + await screen.findByRole('button', { name: /Stop Recording/i }); + fireEvent.click(screen.getByRole('button', { name: /Stop Recording/i })); + + await screen.findByText('Transcribing...'); + expect(transcriptionCalls).toHaveLength(1); + + // Close the modal while transcription is still pending + fireEvent.click(screen.getByLabelText('Close')); + + // With the fix, the first generateContent call must receive an abortSignal and get aborted. + const firstAbortSignal = transcriptionCalls[0]?.abortSignal; + expect(firstAbortSignal).toBeDefined(); + expect(firstAbortSignal?.aborted).toBe(true); + + // Re-open ScenarioSetup (second attempt) before resolving the first transcription + fireEvent.click(screen.getByRole('button', { name: /Start Practice/i })); + fireEvent.click(await screen.findByRole('button', { name: /Role Play/i })); + + await screen.findByText(/Practice Role Play/i); + + fireEvent.click(screen.getByRole('button', { name: /Or describe by voice/i })); + await screen.findByRole('button', { name: /Stop Recording/i }); + fireEvent.click(screen.getByRole('button', { name: /Stop Recording/i })); + + await screen.findByText('Transcribing...'); + await waitFor(() => expect(transcriptionCalls).toHaveLength(2)); + + const [call1, call2] = transcriptionCalls; + + // Resolve the stale first transcription AFTER the second attempt started. + // If stale results are not discarded, the UI will switch away from the + // second transcription spinner and show transcript1 instead. + await act(async () => { + call1.deferred.resolve({ + text: JSON.stringify({ rawTranscript: 'RAW_ONE', cleanedTranscript: 'CLEAN_ONE' }), + }); + }); + + // Second transcription must still be in-flight and must not be overwritten. + expect(screen.getByText('Transcribing...')).toBeInTheDocument(); + expect(screen.queryByText(/Choose your transcript version/i)).not.toBeInTheDocument(); + expect(screen.queryByText('RAW_ONE')).not.toBeInTheDocument(); + expect(screen.queryByText('CLEAN_ONE')).not.toBeInTheDocument(); + + // Resolve the second transcription and ensure only attempt #2 appears. + await act(async () => { + call2.deferred.resolve({ + text: JSON.stringify({ rawTranscript: 'RAW_TWO', cleanedTranscript: 'CLEAN_TWO' }), + }); + }); + + expect(await screen.findByText('RAW_TWO')).toBeInTheDocument(); + expect(screen.getByText('CLEAN_TWO')).toBeInTheDocument(); + expect(screen.queryByText('RAW_ONE')).not.toBeInTheDocument(); + }); + + it('invalidates close+reopen without starting a new transcription (late resolve is discarded)', async () => { + // Simulate a provider that may still resolve after AbortError. + rejectOnAbort = false; + + const { default: App } = await import('../App'); + render(); + + // Open PracticeModeSheet -> Role Play -> ScenarioSetup modal + fireEvent.click(screen.getByRole('button', { name: /Start Practice/i })); + fireEvent.click(await screen.findByRole('button', { name: /Role Play/i })); + await screen.findByText(/Practice Role Play/i); + + // Start first "describe by voice" transcription + fireEvent.click(screen.getByRole('button', { name: /Or describe by voice/i })); + await screen.findByRole('button', { name: /Stop Recording/i }); + fireEvent.click(screen.getByRole('button', { name: /Stop Recording/i })); + + await screen.findByText('Transcribing...'); + await waitFor(() => expect(transcriptionCalls).toHaveLength(1)); + + const [call1] = transcriptionCalls; + + // Close the modal while transcription is still pending + fireEvent.click(screen.getByLabelText('Close')); + + // Re-open ScenarioSetup WITHOUT starting a new transcription + fireEvent.click(screen.getByRole('button', { name: /Start Practice/i })); + fireEvent.click(await screen.findByRole('button', { name: /Role Play/i })); + await screen.findByText(/Practice Role Play/i); + + // Late resolve of the first transcription should not overwrite the reopened modal. + await act(async () => { + call1.deferred.resolve({ + text: JSON.stringify({ rawTranscript: 'RAW_ONE', cleanedTranscript: 'CLEAN_ONE' }), + }); + }); + + expect(screen.queryByText('Transcribing...')).not.toBeInTheDocument(); + expect(screen.queryByText(/Choose your transcript version/i)).not.toBeInTheDocument(); + expect(screen.queryByText('RAW_ONE')).not.toBeInTheDocument(); + expect(screen.queryByText('CLEAN_ONE')).not.toBeInTheDocument(); + }); +}); + diff --git a/__tests__/tefQuestioningReviewFixes.test.ts b/__tests__/tefQuestioningReviewFixes.test.ts index dcdfdf7..144dc2d 100644 --- a/__tests__/tefQuestioningReviewFixes.test.ts +++ b/__tests__/tefQuestioningReviewFixes.test.ts @@ -95,7 +95,7 @@ describe('B2 · showLightbox reset in questioning exit/dismiss handlers (App.tsx const src = await import('../App?raw'); // After fix: handleExitTefQuestioning must contain setShowLightbox(false) expect(src.default).toMatch( - /handleExitTefQuestioning[\s\S]{0,300}setShowLightbox\s*\(\s*false\s*\)/ + /handleExitTefQuestioning[\s\S]{0,800}setShowLightbox\s*\(\s*false\s*\)/ ); }); @@ -103,7 +103,7 @@ describe('B2 · showLightbox reset in questioning exit/dismiss handlers (App.tsx const src = await import('../App?raw'); // After fix: the dismiss handler must also reset the lightbox expect(src.default).toMatch( - /handleDismissTefQuestioningSummary[\s\S]{0,500}setShowLightbox\s*\(\s*false\s*\)/ + /handleDismissTefQuestioningSummary[\s\S]{0,800}setShowLightbox\s*\(\s*false\s*\)/ ); }); }); diff --git a/__tests__/transcribeAndCleanupAudioAbortSignal.test.ts b/__tests__/transcribeAndCleanupAudioAbortSignal.test.ts new file mode 100644 index 0000000..26a98d0 --- /dev/null +++ b/__tests__/transcribeAndCleanupAudioAbortSignal.test.ts @@ -0,0 +1,80 @@ +/** + * TDD tests for transcribeAndCleanupAudio(audioBase64, mimeType, abortSignal?) + * in services/geminiService.ts. + * + * Contract: + * - transcribeAndCleanupAudio accepts an optional AbortSignal + * - the AbortSignal is forwarded into ai.models.generateContent config + * - responseMimeType/responseSchema remain set alongside abortSignal + * + * Tests FAIL before the implementation is updated. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; + +// --------------------------------------------------------------------------- +// Module-level mock for @google/genai +// --------------------------------------------------------------------------- +vi.mock('@google/genai', async (importActual) => { + const actual = await importActual(); + return { + ...actual, + GoogleGenAI: vi.fn(), + }; +}); + +import { GoogleGenAI } from '@google/genai'; + +const FAKE_AUDIO_BASE64 = 'ZmFrZWF1ZGlv'; // "fakeaudio" in base64 +const FAKE_MIME_TYPE = 'audio/webm'; + +describe('transcribeAndCleanupAudio · AbortSignal forwarding', () => { + let mockGenerateContent: ReturnType; + + beforeEach(() => { + localStorage.setItem('parle_api_key_gemini', 'test-key-transcribe-abort'); + + mockGenerateContent = vi.fn().mockResolvedValue({ + text: JSON.stringify({ rawTranscript: 'RAW', cleanedTranscript: 'CLEANED' }), + }); + + const mockAi = { + models: { + get generateContent() { + return mockGenerateContent; + }, + }, + chats: { create: vi.fn() }, + }; + + vi.mocked(GoogleGenAI).mockReturnValue(mockAi as unknown as GoogleGenAI); + }); + + afterEach(() => { + localStorage.clear(); + vi.restoreAllMocks(); + }); + + it('forwards AbortSignal into generateContent config without dropping JSON response config', async () => { + const abortController = new AbortController(); + + const { transcribeAndCleanupAudio } = await import('../services/geminiService'); + + const result = await transcribeAndCleanupAudio(FAKE_AUDIO_BASE64, FAKE_MIME_TYPE, abortController.signal); + expect(result).toEqual({ rawTranscript: 'RAW', cleanedTranscript: 'CLEANED' }); + + expect(mockGenerateContent).toHaveBeenCalledTimes(1); + const requestArg = mockGenerateContent.mock.calls[0][0] as any; + + expect(requestArg.config).toBeDefined(); + expect(requestArg.config.abortSignal).toBe(abortController.signal); + + // Must remain set together with abortSignal + expect(requestArg.config.responseMimeType).toBe('application/json'); + expect(requestArg.config.responseSchema).toBeDefined(); + expect(requestArg.config.responseSchema.required).toEqual( + expect.arrayContaining(['rawTranscript', 'cleanedTranscript']) + ); + }); +}); + diff --git a/e2e/scenario-description-abort.spec.ts b/e2e/scenario-description-abort.spec.ts new file mode 100644 index 0000000..0b0388b --- /dev/null +++ b/e2e/scenario-description-abort.spec.ts @@ -0,0 +1,251 @@ +import { test, expect } from '@playwright/test'; + +test.describe('ScenarioSetup · describe by voice abort/discard', () => { + test.beforeEach(async ({ page }) => { + await page.addInitScript(() => { + // Provide dummy API keys so the app doesn't block with the API key modal. + try { + localStorage.setItem('parle_api_key_gemini', 'test-e2e-gemini'); + localStorage.setItem('parle_api_key_openai', 'test-e2e-openai'); + } catch { + // Ignore localStorage failures (shouldn't happen in real browser contexts) + } + + // ---- Stub microphone/audio recording ---- + // The ScenarioSetup "describe by voice" flow depends on Web Audio + MediaRecorder. + // In CI/headless Playwright we stub these so the UI can progress deterministically. + const fakeStream = { + getTracks: () => [{ stop: () => {} }], + }; + + if (!navigator.mediaDevices) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (navigator as any).mediaDevices = {}; + } + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (navigator.mediaDevices as any).getUserMedia = async () => fakeStream; + + // Minimal Web Audio API shim used by `useAudio`. + class FakeAudioContext { + state: 'running' | 'suspended' = 'running'; + // eslint-disable-next-line @typescript-eslint/no-unused-vars + constructor() {} + resume() { + this.state = 'running'; + return Promise.resolve(); + } + // eslint-disable-next-line @typescript-eslint/no-unused-vars + createMediaStreamSource(_stream: any) { + return { connect: () => {} }; + } + createAnalyser() { + return { + fftSize: 256, + frequencyBinCount: 128, + // eslint-disable-next-line @typescript-eslint/no-unused-vars + getByteFrequencyData: (arr: Uint8Array) => { + for (let i = 0; i < arr.length; i++) arr[i] = 0; + }, + }; + } + } + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (window as any).AudioContext = FakeAudioContext; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (window as any).webkitAudioContext = FakeAudioContext; + + // Minimal MediaRecorder shim used by `useAudio`. + class FakeMediaRecorder { + mimeType = 'audio/webm'; + stream: any; + state: 'inactive' | 'recording' = 'inactive'; + ondataavailable: null | ((event: { data: Blob }) => void) = null; + onstop: null | ((event?: any) => void) = null; + + constructor(stream: any) { + this.stream = stream; + } + + start() { + this.state = 'recording'; + if (this.ondataavailable) { + // Provide a non-empty Blob so blobToBase64 produces deterministic output. + const data = new Uint8Array([1, 2, 3]); + const blob = new Blob([data], { type: this.mimeType }); + this.ondataavailable({ data: blob }); + } + } + + stop() { + this.state = 'inactive'; + // Only call onstop if someone registered a handler (the app registers onstop + // only for `stopRecording`, not for `cancelRecording`). + if (this.onstop) this.onstop({}); + } + } + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (window as any).MediaRecorder = FakeMediaRecorder; + }); + + await page.goto('/'); + }); + + test('closing while transcription is in-flight discards stale transcript', async ({ page }) => { + const screenshot = async (name: string) => { + await page.screenshot({ path: `test-results/screenshots/${name}.png`, fullPage: true }); + }; + + type PendingRoute = { route: any; fulfilled: boolean }; + + const transcriptionResponses: PendingRoute[] = []; + let transcriptionCallIndex = 0; + let resolveCall1!: () => void; + let resolveCall2!: () => void; + + const call1Arrived = new Promise(r => { + resolveCall1 = r; + }); + const call2Arrived = new Promise(r => { + resolveCall2 = r; + }); + + // Intercept Gemini transcription calls and keep the first/second attempts pending + // until the test explicitly resolves them. + await page.route('**/models/gemini-2.0-flash-lite:generateContent*', async route => { + const req = route.request(); + let bodyJson: any = null; + try { + bodyJson = req.postDataJSON(); + } catch { + bodyJson = null; + } + + const bodyStr = bodyJson ? JSON.stringify(bodyJson) : ''; + const looksLikeScenarioTranscription = + bodyStr.includes('produce two versions of the transcript') || + bodyStr.includes('Transcribe this audio exactly as spoken'); + + if (!looksLikeScenarioTranscription) { + // We only expect scenario transcription calls in this test. + await route.fallback(); + return; + } + + transcriptionCallIndex += 1; + const pending: PendingRoute = { route, fulfilled: false }; + transcriptionResponses[transcriptionCallIndex - 1] = pending; + + // This test expects exactly two transcription attempts (attempt #1 -> close, attempt #2 -> resolve). + // If something unexpectedly triggers a third call, fulfill it immediately to avoid hanging the UI. + if (transcriptionCallIndex > 2) { + const payload = { + candidates: [ + { + content: { + role: 'model', + parts: [{ text: JSON.stringify({ rawTranscript: '', cleanedTranscript: '' }) }], + }, + }, + ], + }; + try { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify(payload), + }); + } catch { + // Ignore fulfillment errors + } + return; + } + + if (transcriptionCallIndex === 1) resolveCall1(); + if (transcriptionCallIndex === 2) resolveCall2(); + + // Delay fulfillment. The test will resolve these routes later. + }); + + // Open ScenarioSetup: Practice mode sheet -> Role Play -> ScenarioSetup modal + await page.getByRole('button', { name: /Start Practice/i }).click(); + await page.getByRole('button', { name: /Role Play/i }).click(); + const modalHeading = page.getByRole('heading', { name: 'Practice Role Play' }); + await expect(modalHeading).toBeVisible(); + + // Start first transcription attempt. + await page.getByRole('button', { name: /Or describe by voice/i }).click(); + await expect(page.getByRole('button', { name: /Stop Recording/i })).toBeVisible(); + await page.getByRole('button', { name: /Stop Recording/i }).click(); + await expect(page.getByText('Transcribing...')).toBeVisible(); + await call1Arrived; + await screenshot('scenario-1-transcribing'); + + // Close while in-flight: this must abort/invalidate the first attempt + // so it can't overwrite UI state after a close+reopen race. + await page.getByLabel('Close').click(); + await expect(modalHeading).not.toBeVisible(); + await screenshot('scenario-after-close'); + + // Open again and start second transcription attempt. + await page.getByRole('button', { name: /Start Practice/i }).click(); + await page.getByRole('button', { name: /Role Play/i }).click(); + await expect(modalHeading).toBeVisible(); + + await page.getByRole('button', { name: /Or describe by voice/i }).click(); + await expect(page.getByRole('button', { name: /Stop Recording/i })).toBeVisible(); + await page.getByRole('button', { name: /Stop Recording/i }).click(); + await expect(page.getByText('Transcribing...')).toBeVisible(); + await call2Arrived; + await screenshot('scenario-2-transcribing'); + + const fulfillTranscription = async (pending: PendingRoute | undefined, raw: string, cleaned: string) => { + if (!pending || pending.fulfilled) return; + pending.fulfilled = true; + const payload = { + candidates: [ + { + content: { + role: 'model', + parts: [{ text: JSON.stringify({ rawTranscript: raw, cleanedTranscript: cleaned }) }], + }, + }, + ], + }; + + // Route fulfillment may throw if the request was fully aborted, but the app should still + // remain responsive; we treat that as acceptable for this regression test. + try { + await pending.route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify(payload), + }); + } catch { + // Intentionally ignored. + } + }; + + // Resolve the stale first attempt after the second spinner is showing. + await fulfillTranscription(transcriptionResponses[0], 'RAW_ONE', 'CLEAN_ONE'); + + // UI must remain on the second in-flight attempt and must NOT show transcript options + // from the stale response. + await expect(page.getByText('Transcribing...')).toBeVisible(); + await expect(page.locator('text=/Choose your transcript version/i')).toHaveCount(0); + await expect(page.locator('text=RAW_ONE')).toHaveCount(0); + await expect(page.locator('text=CLEAN_ONE')).toHaveCount(0); + + // Resolve the second attempt and verify only attempt #2 transcripts are shown. + await fulfillTranscription(transcriptionResponses[1], 'RAW_TWO', 'CLEAN_TWO'); + + await expect(page.locator('text=RAW_TWO')).toBeVisible(); + await expect(page.locator('text=CLEAN_TWO')).toBeVisible(); + await expect(page.locator('text=RAW_ONE')).toHaveCount(0); + await expect(page.locator('text=CLEAN_ONE')).toHaveCount(0); + await screenshot('scenario-final-transcripts'); + }); +}); + diff --git a/playwright.config.ts b/playwright.config.ts index 9412a31..0cb8348 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -1,5 +1,9 @@ import { defineConfig, devices } from '@playwright/test'; +// Only use system Chrome when explicitly configured. +// Otherwise prefer Playwright's managed Chromium (downloaded into ./node_modules cache or PLAYWRIGHT_BROWSERS_PATH). +const chromeExecutablePath = process.env.PW_CHROME_EXECUTABLE_PATH; + export default defineConfig({ testDir: './e2e', fullyParallel: true, @@ -13,7 +17,17 @@ export default defineConfig({ screenshot: 'only-on-failure', }, outputDir: 'test-results/', - projects: [{ name: 'chromium', use: { ...devices['Desktop Chrome'] } }], + projects: [ + { + name: 'chromium', + use: { + ...devices['Desktop Chrome'], + launchOptions: chromeExecutablePath + ? { executablePath: chromeExecutablePath } + : undefined, + }, + }, + ], webServer: { command: 'npm run dev', port: 3000, diff --git a/services/geminiService.ts b/services/geminiService.ts index 3ff356f..826ff45 100644 --- a/services/geminiService.ts +++ b/services/geminiService.ts @@ -42,6 +42,7 @@ let ai: GoogleGenAI | null = null; let chatSession: Chat | null = null; // Track how many messages from shared history have been synced to the session let syncedMessageCount = 0; +// (debug instrumentation removed) // Track the active scenario for scenario-aware prompting let activeScenario: Scenario | null = null; // Store pending scenario and history when ai is not yet initialized @@ -539,7 +540,8 @@ export const transcribeAudio = async (audioBase64: string, mimeType: string): Pr */ export const transcribeAndCleanupAudio = async ( audioBase64: string, - mimeType: string + mimeType: string, + signal?: AbortSignal ): Promise<{ rawTranscript: string; cleanedTranscript: string }> => { ensureAiInitialized(); @@ -569,6 +571,7 @@ export const transcribeAndCleanupAudio = async ( }], config: { responseMimeType: 'application/json', + abortSignal: signal, responseSchema: { type: Type.OBJECT, properties: { @@ -632,7 +635,8 @@ export const initializeSession = async () => { */ export const generateCharacterSpeech = async ( text: string, - voiceName: string + voiceName: string, + signal?: AbortSignal ): Promise => { if (!ai) { ensureAiInitialized(); @@ -647,6 +651,7 @@ export const generateCharacterSpeech = async ( model: 'gemini-2.5-flash-preview-tts', contents: [{ parts: [{ text: systemPrompt }] }], config: { + abortSignal: signal, responseModalities: [Modality.AUDIO], speechConfig: { voiceConfig: { @@ -732,6 +737,9 @@ export const sendVoiceMessage = async ( }, ], }], + config: { + abortSignal: signal, + }, })); const userText = transcribeResponse.text || ""; @@ -764,8 +772,34 @@ export const sendVoiceMessage = async ( } messageParts.push({ inlineData: { data: audioBase64, mimeType: mimeType } }); + // NOTE: Passing per-request config does NOT inherit chat-level config. + // When we pass abortSignal here, we must also include responseMimeType/responseSchema + // or the SDK may return plain text (which would break JSON parsing below). + const systemInstructionForThisRequest = activeScenario + ? generateScenarioSystemInstruction(activeScenario) + : SYSTEM_INSTRUCTION; + + const responseSchemaForThisRequest = (() => { + if (activeScenario && activeScenario.characters && activeScenario.characters.length > 1) { + return createGeminiMultiCharacterSchema(activeScenario); + } + if (!activeScenario) { + return FREE_CONVERSATION_RESPONSE_SCHEMA; + } + if (activeScenario.isTefQuestioning) { + return TEF_QUESTIONING_RESPONSE_SCHEMA; + } + return SINGLE_CHARACTER_RESPONSE_SCHEMA; + })(); + const chatResponse = await abortablePromise(chatSession.sendMessage({ message: messageParts, + config: { + abortSignal: signal, + systemInstruction: systemInstructionForThisRequest, + responseMimeType: 'application/json', + responseSchema: responseSchemaForThisRequest, + }, })); const rawModelText = chatResponse.text; // Access text property directly @@ -866,7 +900,7 @@ export const sendVoiceMessage = async ( throw new Error(`Character not found: ${charResp.characterName} (ID: ${charResp.characterId})`); } - const audioUrl = await abortablePromise(generateCharacterSpeech(charResp.french, character.voiceName)); + const audioUrl = await abortablePromise(generateCharacterSpeech(charResp.french, character.voiceName, signal)); return { ...charResp, audioUrl, voiceName: character.voiceName }; }); @@ -958,7 +992,7 @@ export const sendVoiceMessage = async ( let audioUrl = ''; try { - audioUrl = await abortablePromise(generateCharacterSpeech(validated.french, voiceName)); + audioUrl = await abortablePromise(generateCharacterSpeech(validated.french, voiceName, signal)); } catch (ttsError) { // Re-throw aborts - user cancelled the operation if (ttsError instanceof DOMException && ttsError.name === 'AbortError') { @@ -1031,7 +1065,7 @@ export const sendVoiceMessage = async ( let audioUrl = ''; try { - audioUrl = await abortablePromise(generateCharacterSpeech(validated.french, voiceName)); + audioUrl = await abortablePromise(generateCharacterSpeech(validated.french, voiceName, signal)); } catch (ttsError) { // Re-throw aborts - user cancelled the operation if (ttsError instanceof DOMException && ttsError.name === 'AbortError') {