Skip to content

Commit 3ff9eb7

Browse files
committed
fix: address sparring review findings
1 parent 8979f7b commit 3ff9eb7

17 files changed

Lines changed: 362 additions & 117 deletions

‎e2e/sparring.spec.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,13 +71,13 @@ test('completes an LD sparring round through mocked opponent turns and judging',
7171
['Second affirmative rebuttal', 'The universal policy is more reliable because every marginal trip is covered.'],
7272
] as const;
7373

74-
for (const [turnLabel, content] of userSpeeches) {
74+
for (const [index, [turnLabel, content]] of userSpeeches.entries()) {
7575
const draft = page.getByRole('textbox', { name: `Draft for ${turnLabel}` });
7676
await expect(draft).toBeVisible();
7777
await draft.fill(content);
7878
await page.getByRole('button', { name: 'Submit speech' }).click();
7979
if (turnLabel !== 'Second affirmative rebuttal') {
80-
await expect(page.getByText(`Opponent response ${responseNumber + 1}:`)).toBeVisible();
80+
await expect(page.getByText(`Opponent response ${index + 1}:`)).toBeVisible();
8181
}
8282
}
8383

‎plans/sparring-and-streaming.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ interface SparringScorecard {
9595
Streaming remains tracked independently in `plans/streaming-ai.md`; it is not part of the turn-based sparring contract.
9696

9797
## Original Estimated Effort
98+
9899
- Setup + Round UI: 4 hours
99100
- AI integration (prompts, edge function): 3 hours
100101
- Scorecard + judging: 2 hours

‎src/components/features/sparring/SparringRound.tsx‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ interface SparringRoundProps {
1818
aiError?: string | null;
1919
judging?: boolean;
2020
judgingError?: string | null;
21+
submissionError?: string | null;
2122
onSubmit: (content: string) => void;
2223
onRetryAi?: () => void;
2324
onRetryJudging?: () => void;
@@ -41,6 +42,7 @@ export function SparringRound({
4142
aiError,
4243
judging = false,
4344
judgingError,
45+
submissionError,
4446
onSubmit,
4547
onRetryAi,
4648
onRetryJudging,
@@ -61,6 +63,8 @@ export function SparringRound({
6163
: t('sparring.round.yourSpeechDescription', 'Make the case in the time you have. The opponent will respond to this text.');
6264
const timerProgress = currentTurn?.timeLimitSeconds ? Math.max(0, Math.min(100, (session.timerRemainingSeconds / currentTurn.timeLimitSeconds) * 100)) : 0;
6365
const isUrgent = session.timerRemainingSeconds <= 30 && session.timerRemainingSeconds > 0;
66+
const timeExpired = session.timerRemainingSeconds <= 0
67+
|| (session.timerDeadline !== null && session.timerDeadline <= Date.now());
6468

6569
useEffect(() => {
6670
const handleOnline = () => setIsOnline(true);
@@ -82,7 +86,7 @@ export function SparringRound({
8286
const handleKeyDown = (event: React.KeyboardEvent<HTMLTextAreaElement>) => {
8387
if ((event.metaKey || event.ctrlKey) && event.key === 'Enter') {
8488
event.preventDefault();
85-
if (userTurn && session.draft.trim() && !judging && isOnline) onSubmit(session.draft);
89+
if (userTurn && session.draft.trim() && !judging && isOnline && !timeExpired) onSubmit(session.draft);
8690
}
8791
};
8892

@@ -185,10 +189,12 @@ export function SparringRound({
185189
/>
186190
<div className="flex flex-col gap-3 sm:flex-row sm:items-center sm:justify-between">
187191
<p className="text-xs text-muted-foreground">{isOnline ? <><kbd className="rounded border border-border bg-muted px-1.5 py-0.5 font-mono">⌘</kbd> or <kbd className="rounded border border-border bg-muted px-1.5 py-0.5 font-mono">Ctrl</kbd> + <kbd className="rounded border border-border bg-muted px-1.5 py-0.5 font-mono">Enter</kbd> {t('sparring.round.toSubmit', 'to submit')}</> : t('sparring.round.offlineCannotSubmit', 'Reconnect to submit this speech.')}</p>
188-
<Button type="button" onClick={() => onSubmit(session.draft)} disabled={!session.draft.trim() || !isOnline} className="sm:min-w-36">
192+
<Button type="button" onClick={() => onSubmit(session.draft)} disabled={!session.draft.trim() || !isOnline || timeExpired} className="sm:min-w-36">
189193
{t('sparring.round.submitSpeech', 'Submit speech')} <Send className="h-4 w-4" aria-hidden="true" />
190194
</Button>
191195
</div>
196+
{timeExpired && <p className="text-sm text-destructive" role="alert">{t('sparring.round.timeExpired', 'This speaking time has ended. Reset the turn to continue.')}</p>}
197+
{!timeExpired && submissionError && <p className="text-sm text-destructive" role="alert">{submissionError}</p>}
192198
</CardContent>
193199
</Card>
194200
) : (

‎src/lib/stores/sparring-store.ts‎

Lines changed: 64 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,58 @@ const PersistedSparringStateSchema = z.object({
105105
});
106106

107107
type PersistedSparringState = Pick<SparringStoreState, 'currentSession' | 'savedSessions'>;
108+
const MAX_SAVED_SESSIONS = 50;
109+
const PERSIST_DELAY_MS = 250;
110+
let pendingStorageWrite: { name: string; value: string } | null = null;
111+
let storageWriteTimer: ReturnType<typeof setTimeout> | null = null;
112+
113+
function flushPendingStorageWrite() {
114+
if (!pendingStorageWrite || typeof sessionStorage === 'undefined') return;
115+
sessionStorage.setItem(pendingStorageWrite.name, pendingStorageWrite.value);
116+
pendingStorageWrite = null;
117+
if (storageWriteTimer) clearTimeout(storageWriteTimer);
118+
storageWriteTimer = null;
119+
}
120+
121+
function withoutVolatileDraftFields(value: string): string | null {
122+
try {
123+
const parsed = JSON.parse(value) as { state?: { currentSession?: { draft?: string; updatedAt?: number } | null } };
124+
if (parsed.state?.currentSession) {
125+
parsed.state.currentSession.draft = '';
126+
parsed.state.currentSession.updatedAt = 0;
127+
}
128+
return JSON.stringify(parsed);
129+
} catch {
130+
return null;
131+
}
132+
}
133+
134+
const debouncedSessionStorage = {
135+
getItem: (name: string) => sessionStorage.getItem(name),
136+
setItem: (name: string, value: string) => {
137+
const persistedValue = sessionStorage.getItem(name);
138+
const draftOnlyChange = persistedValue !== null
139+
&& withoutVolatileDraftFields(persistedValue) === withoutVolatileDraftFields(value);
140+
if (!draftOnlyChange) {
141+
if (storageWriteTimer) clearTimeout(storageWriteTimer);
142+
storageWriteTimer = null;
143+
pendingStorageWrite = null;
144+
sessionStorage.setItem(name, value);
145+
return;
146+
}
147+
pendingStorageWrite = { name, value };
148+
if (storageWriteTimer) clearTimeout(storageWriteTimer);
149+
storageWriteTimer = setTimeout(flushPendingStorageWrite, PERSIST_DELAY_MS);
150+
},
151+
removeItem: (name: string) => {
152+
if (pendingStorageWrite?.name === name) pendingStorageWrite = null;
153+
if (storageWriteTimer) clearTimeout(storageWriteTimer);
154+
storageWriteTimer = null;
155+
sessionStorage.removeItem(name);
156+
},
157+
};
158+
159+
if (typeof window !== 'undefined') window.addEventListener('pagehide', flushPendingStorageWrite);
108160

109161
function migratePersistedSparringState(value: unknown): PersistedSparringState {
110162
const parsed = PersistedSparringStateSchema.safeParse(value);
@@ -173,7 +225,7 @@ function mergeSessions(local: SparringSession[], remote: SparringSession[]): Spa
173225
const current = byId.get(session.id);
174226
if (!current || current.updatedAt < session.updatedAt) byId.set(session.id, session);
175227
}
176-
return [...byId.values()].sort((a, b) => b.updatedAt - a.updatedAt);
228+
return [...byId.values()].sort((a, b) => b.updatedAt - a.updatedAt).slice(0, MAX_SAVED_SESSIONS);
177229
}
178230

179231
function parseSavedRows(value: unknown): SparringSession[] {
@@ -282,7 +334,10 @@ export const useSparringStore = create<SparringStoreState>()(
282334
const session = get().currentSession;
283335
const turn = session ? currentTurn(session) : undefined;
284336
const cleanContent = content.trim();
285-
if (!session || session.phase !== 'round' || turn?.speaker !== 'user' || !cleanContent || cleanContent.length > 6_000) return null;
337+
const deadlinePassed = session?.timerDeadline !== null
338+
&& session?.timerDeadline !== undefined
339+
&& session.timerDeadline <= now;
340+
if (!session || session.phase !== 'round' || turn?.speaker !== 'user' || !cleanContent || cleanContent.length > 6_000 || session.timerRemainingSeconds <= 0 || deadlinePassed) return null;
286341
const remaining = session.timerRunning && session.timerDeadline !== null
287342
? Math.max(0, Math.ceil((session.timerDeadline - now) / 1000))
288343
: session.timerRemainingSeconds;
@@ -378,12 +433,16 @@ export const useSparringStore = create<SparringStoreState>()(
378433
}),
379434
{
380435
name: 'debate-sparring-storage',
381-
storage: createJSONStorage(() => sessionStorage),
382-
version: 2,
436+
storage: createJSONStorage(() => debouncedSessionStorage),
437+
version: 3,
383438
migrate: (persistedState) => migratePersistedSparringState(persistedState),
439+
merge: (persistedState, currentState) => ({
440+
...currentState,
441+
...migratePersistedSparringState(persistedState),
442+
}),
384443
partialize: (state) => ({
385444
currentSession: state.currentSession,
386-
savedSessions: state.savedSessions,
445+
savedSessions: state.savedSessions.slice(0, MAX_SAVED_SESSIONS),
387446
}),
388447
},
389448
),

‎src/locales/en.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1292,6 +1292,8 @@
12921292
"draftFor": "Draft for {{label}}",
12931293
"toSubmit": "to submit",
12941294
"submitSpeech": "Submit speech",
1295+
"timeExpired": "This speaking time has ended. Reset the turn to continue.",
1296+
"speechRejected": "This speech could not be submitted. Check the timer and length, then try again.",
12951297
"offlineCannotSubmit": "Reconnect to submit this speech.",
12961298
"opponentFailed": "The opponent could not answer",
12971299
"retryOpponent": "Retry opponent turn",

‎src/locales/es.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1292,6 +1292,8 @@
12921292
"draftFor": "Borrador para {{label}}",
12931293
"toSubmit": "para enviar",
12941294
"submitSpeech": "Enviar intervención",
1295+
"timeExpired": "El tiempo de esta intervención ha terminado. Reinicia el turno para continuar.",
1296+
"speechRejected": "No se pudo enviar esta intervención. Comprueba el cronómetro y la longitud e inténtalo de nuevo.",
12951297
"offlineCannotSubmit": "Vuelve a conectarte para enviar esta intervención.",
12961298
"opponentFailed": "El oponente no pudo responder",
12971299
"retryOpponent": "Reintentar turno del oponente",

‎src/locales/hu.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1292,6 +1292,8 @@
12921292
"draftFor": "Piszkozat: {{label}}",
12931293
"toSubmit": "beküldéshez",
12941294
"submitSpeech": "Beszéd beküldése",
1295+
"timeExpired": "A felszólalási idő lejárt. A folytatáshoz állítsd vissza a kört.",
1296+
"speechRejected": "A beszédet nem sikerült beküldeni. Ellenőrizd az időt és a hosszt, majd próbáld újra.",
12951297
"offlineCannotSubmit": "Csatlakozz újra a beszéd beküldéséhez.",
12961298
"opponentFailed": "Az ellenfél nem tudott válaszolni",
12971299
"retryOpponent": "Az ellenfél körének újrapróbálása",

‎src/pages/SparringPage.tsx‎

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useCallback, useEffect, useRef, useState } from 'react';
1+
import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
22
import { useTranslation } from 'react-i18next';
33
import { ArrowLeft, CircleHelp } from 'lucide-react';
44
import { useNavigate } from 'react-router-dom';
@@ -38,6 +38,7 @@ export default function SparringPage() {
3838
const [aiLoading, setAiLoading] = useState(false);
3939
const [aiError, setAiError] = useState<string | null>(null);
4040
const [judgingError, setJudgingError] = useState<string | null>(null);
41+
const [submissionError, setSubmissionError] = useState<string | null>(null);
4142
const [isOnline, setIsOnline] = useState(() => typeof navigator === 'undefined' || navigator.onLine);
4243
const [retryNonce, setRetryNonce] = useState(0);
4344
const aiRequestRef = useRef<string | null>(null);
@@ -68,7 +69,7 @@ export default function SparringPage() {
6869
};
6970
}, []);
7071

71-
const config = getFeatureConfig('sparring');
72+
const config = useMemo(() => getFeatureConfig('sparring'), [getFeatureConfig]);
7273

7374
const retryAi = useCallback(() => {
7475
aiRequestRef.current = null;
@@ -102,7 +103,6 @@ export default function SparringPage() {
102103
aiInFlightRef.current = true;
103104
setAiLoading(true);
104105
setAiError(null);
105-
let active = true;
106106
const { speaker: _speaker, ...turnInput } = currentTurn;
107107
const input: SparringRespondInput = {
108108
action: 'respond',
@@ -136,17 +136,15 @@ export default function SparringPage() {
136136
appendAiSpeech(speech);
137137
})
138138
.catch((error: unknown) => {
139-
if (!active || aiRequestRef.current !== requestKey) return;
139+
if (aiRequestRef.current !== requestKey) return;
140140
setAiError(readableError(error, t('sparring.round.aiRequestFailed', 'The AI request failed. Check your connection and try again.')));
141141
})
142142
.finally(() => {
143-
aiInFlightRef.current = false;
144-
if (active && aiRequestRef.current === requestKey) setAiLoading(false);
143+
if (aiRequestRef.current === requestKey) {
144+
aiInFlightRef.current = false;
145+
setAiLoading(false);
146+
}
145147
});
146-
147-
return () => {
148-
active = false;
149-
};
150148
}, [appendAiSpeech, config, outputLanguage, retryNonce, session, t]);
151149

152150
useEffect(() => {
@@ -160,7 +158,6 @@ export default function SparringPage() {
160158
judgeRequestRef.current = requestKey;
161159
judgeInFlightRef.current = true;
162160
setJudgingError(null);
163-
let active = true;
164161
const input: SparringScoreInput = {
165162
action: 'score',
166163
sessionId: session.id,
@@ -178,15 +175,12 @@ export default function SparringPage() {
178175
completeSession(result.scorecard);
179176
})
180177
.catch((error: unknown) => {
181-
if (!active || judgeRequestRef.current !== requestKey) return;
178+
if (judgeRequestRef.current !== requestKey) return;
182179
setJudgingError(readableError(error, t('sparring.round.judgeRequestFailed', 'The judge request failed. Check your connection and try again.')));
183180
})
184181
.finally(() => {
185-
judgeInFlightRef.current = false;
182+
if (judgeRequestRef.current === requestKey) judgeInFlightRef.current = false;
186183
});
187-
return () => {
188-
active = false;
189-
};
190184
}, [completeSession, config, outputLanguage, retryNonce, session, t]);
191185

192186
const handleStart = (values: SparringSetupValues) => {
@@ -196,6 +190,7 @@ export default function SparringPage() {
196190
setRetryNonce(0);
197191
aiRequestRef.current = null;
198192
judgeRequestRef.current = null;
193+
setSubmissionError(null);
199194
};
200195

201196
const handleRestart = () => {
@@ -205,6 +200,12 @@ export default function SparringPage() {
205200
setRetryNonce(0);
206201
aiRequestRef.current = null;
207202
judgeRequestRef.current = null;
203+
setSubmissionError(null);
204+
};
205+
206+
const handleSubmit = (content: string) => {
207+
const result = submitUserSpeech(content);
208+
setSubmissionError(result ? null : t('sparring.round.speechRejected', 'This speech could not be submitted. Check the timer and length, then try again.'));
208209
};
209210

210211
const handleExportDocx = () => {
@@ -243,7 +244,8 @@ export default function SparringPage() {
243244
aiError={aiError}
244245
judging={session.phase === 'judging'}
245246
judgingError={judgingError}
246-
onSubmit={(content) => submitUserSpeech(content)}
247+
submissionError={submissionError}
248+
onSubmit={handleSubmit}
247249
onRetryAi={retryAi}
248250
onRetryJudging={retryJudging}
249251
/>

‎src/test/pages/SparringPage.test.tsx‎

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { act, fireEvent, render, screen, waitFor } from '@testing-library/react';
2-
import { beforeEach, describe, expect, it, vi } from 'vitest';
2+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
33
import { MemoryRouter } from 'react-router-dom';
44
import type { ReactNode } from 'react';
55
import type { ProviderConfig } from '@/lib/ai/providers/types';
@@ -109,6 +109,10 @@ describe('SparringPage', () => {
109109
judgeMock.mockResolvedValue({ action: 'score', scorecard: SCORECARD });
110110
});
111111

112+
afterEach(() => {
113+
vi.unstubAllGlobals();
114+
});
115+
112116
it('renders setup, validates the motion, and starts a chosen round', async () => {
113117
renderPage();
114118

@@ -179,6 +183,28 @@ describe('SparringPage', () => {
179183
expect(screen.getByText('Universal access removes the price barrier.')).toBeInTheDocument();
180184
});
181185

186+
it('blocks a speech after the turn deadline', () => {
187+
const session = useSparringStore.getState().startSession({
188+
motion: 'This House would make public transport free.',
189+
userSide: 'pro',
190+
format: 'standard',
191+
difficulty: 'medium',
192+
});
193+
useSparringStore.setState({
194+
currentSession: {
195+
...session,
196+
draft: 'A speech submitted too late.',
197+
timerRemainingSeconds: 0,
198+
timerRunning: false,
199+
timerDeadline: null,
200+
},
201+
});
202+
203+
renderPage();
204+
expect(screen.getByRole('button', { name: 'Submit speech' })).toBeDisabled();
205+
expect(screen.getByRole('alert')).toHaveTextContent('This speaking time has ended');
206+
});
207+
182208
it('offers a retry after an opponent failure without duplicating the user speech', async () => {
183209
generateMock
184210
.mockRejectedValueOnce(new Error('Temporary provider failure'))
@@ -232,12 +258,12 @@ describe('SparringPage', () => {
232258
});
233259

234260
it('renders safely at a narrow viewport with reduced motion enabled', () => {
235-
window.matchMedia = vi.fn().mockImplementation((query: string) => ({
261+
vi.stubGlobal('matchMedia', vi.fn().mockImplementation((query: string) => ({
236262
matches: query === '(prefers-reduced-motion: reduce)',
237263
media: query,
238264
addEventListener: (): void => undefined,
239265
removeEventListener: (): void => undefined,
240-
}));
266+
})));
241267

242268
renderPage();
243269
expect(screen.getByRole('heading', { name: 'Spar with an opponent who listens.' })).toBeInTheDocument();

‎src/test/sparring-ai.test.ts‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,20 @@ describe('Sparring AI contract', () => {
4545
});
4646

4747
it('caps session context at twelve speeches and requires all score categories', () => {
48-
const speech = {
49-
id: 'speech-1', turnId: 'pf-1', speaker: 'user', side: 'pro', role: 'Pro constructive',
50-
content: 'A complete argument.', createdAt: 1, timeLimitSeconds: 240, timeUsedSeconds: 20,
51-
};
52-
expect(SparringInputSchema.safeParse({ ...session, action: 'score', speeches: Array(13).fill(speech) }).success).toBe(false);
48+
const speeches = Array.from({ length: 13 }, (_, index) => ({
49+
id: `speech-${index + 1}`,
50+
turnId: `turn-${index + 1}`,
51+
speaker: index % 2 === 0 ? 'user' as const : 'ai' as const,
52+
side: index % 2 === 0 ? 'pro' as const : 'con' as const,
53+
role: `Speaker ${index + 1}`,
54+
content: `Complete argument ${index + 1}.`,
55+
createdAt: index + 1,
56+
timeLimitSeconds: 240,
57+
timeUsedSeconds: 20,
58+
...(index % 2 === 1 ? { integrityToken: `${'x'.repeat(40)}${index + 1}` } : {}),
59+
}));
60+
expect(SparringInputSchema.safeParse({ ...session, action: 'score', speeches: speeches.slice(0, 12) }).success).toBe(true);
61+
expect(SparringInputSchema.safeParse({ ...session, action: 'score', speeches }).success).toBe(false);
5362
const category = (name: 'Logic' | 'Evidence' | 'Delivery' | 'Strategy') => ({ name, userScore: 20, aiScore: 18, feedback: 'Clear comparison.' });
5463
const scorecard = {
5564
userScore: 80, aiScore: 72,

0 commit comments

Comments
 (0)