From 2e6ceaa1f8f5ca188fc3d1f5ec6c7e8ec930e388 Mon Sep 17 00:00:00 2001 From: Adam Date: Fri, 2 Oct 2026 11:10:51 +0000 Subject: [PATCH 1/2] Fix section regeneration and workspace autosave ordering Regenerating one Generate section set the full-generation loading flag, which unmounted every result and the toolbar until the call returned. It now tracks the regenerating section separately: only that card shows an overlay, and the other regenerate buttons and the form wait for it, so two regenerations cannot merge over each other. Crossfire and Sparring workspace autosaves fired an unordered RPC on every change. A slower, older save could land last and store shorter progress. A small queue now keeps one save in flight and sends only the newest payload per request ID, so a finished round's last save survives the next round starting. Also reuses useOnlineStatus in four components that rebuilt it, and replaces five copied Select blocks in GenerateForm with a local helper. Adds the gen-regenerate verify recipe. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01CNnE2EyywE54QHAFuAFZr5 --- .../verify-debatelab/features/README.md | 2 +- .../features/case-generation.md | 4 + .../recipes/case-generation.spec.ts | 36 +++++ src/components/OfflineIndicator.tsx | 14 +- .../features/generate/GenerateForm.tsx | 123 +++++++++--------- .../features/generate/GenerateResults.tsx | 13 +- .../features/sparring/SparringRound.tsx | 14 +- .../features/sparring/SparringSetup.tsx | 14 +- src/components/ui/section-card.tsx | 7 +- src/hooks/use-online-status.ts | 4 +- src/lib/workspace/artifact-save-queue.ts | 35 +++++ src/locales/en.json | 1 + src/locales/es.json | 1 + src/locales/hu.json | 1 + src/pages/CrossfirePage.tsx | 7 +- src/pages/GeneratePage.tsx | 10 +- src/pages/SparringPage.tsx | 39 +++--- .../GenerateResultsRegenerate.test.tsx | 41 ++++++ src/test/pages/CrossfirePage.test.tsx | 44 +++++++ src/test/pages/GeneratePage.test.tsx | 36 ++++- .../workspace/artifact-save-queue.test.ts | 85 ++++++++++++ 21 files changed, 399 insertions(+), 132 deletions(-) create mode 100644 src/lib/workspace/artifact-save-queue.ts create mode 100644 src/test/components/GenerateResultsRegenerate.test.tsx create mode 100644 src/test/workspace/artifact-save-queue.test.ts diff --git a/.claude/skills/verify-debatelab/features/README.md b/.claude/skills/verify-debatelab/features/README.md index 71491a9e5..e2bd114fe 100644 --- a/.claude/skills/verify-debatelab/features/README.md +++ b/.claude/skills/verify-debatelab/features/README.md @@ -40,7 +40,7 @@ Each feature file starts with an H1 title and one paragraph describing the user- ## Features - [Demo-code sign-in](./auth-demo-login.md): sign-in, reload, rejected code, sign-out (`e2e/full-stack/auth.spec.ts`), and email one-time-code sign-in through Mailpit (`recipes/auth.spec.ts`). -- [Case generation](./case-generation.md): generate, web research, save and reopen from History on a fresh device, PDF and DOCX export, provider failure and retry, quota block (`recipes/case-generation.spec.ts`). +- [Case generation](./case-generation.md): generate, web research, save and reopen from History on a fresh device, PDF and DOCX export, single-section regeneration, provider failure and retry, quota block (`recipes/case-generation.spec.ts`). - [Library sources](./library-sources.md): add, pause, delete, grounding, and URL import (`recipes/library.spec.ts`). - [AI tools](./ai-tools.md): Attack, Brainstorm, Factoids, and Ask streamed, cited, and with web search (`recipes/ai-tools.spec.ts`). - [Current Debate](./current-debate.md): start, the first-debate offer and the different-motion choice, sources and results scoped to a debate, switching, and the first onboarding step (`recipes/current-debate.spec.ts`); debate context review and pins (`e2e/full-stack/debate-context.spec.ts`). diff --git a/.claude/skills/verify-debatelab/features/case-generation.md b/.claude/skills/verify-debatelab/features/case-generation.md index 124a9740d..6c83cee79 100644 --- a/.claude/skills/verify-debatelab/features/case-generation.md +++ b/.claude/skills/verify-debatelab/features/case-generation.md @@ -9,6 +9,7 @@ A signed-in user enters a motion on Generate and receives a structured debate ca - `gen-error` shows `Generation Failed` on a provider error, and the button becomes enabled again for a retry. - `gen-quota` shows the dialog `Daily Access Paused` when the account has no quota, without calling the provider. - `gen-save` saves a one-off case (the motion dialog answered with `Not now, use it once`) under a title into `cases` with `debate_id: null`. It appears in History under `One-off and unsorted work`, including from a fresh browser context. +- `gen-regenerate` regenerates one section. Only that section shows the overlay `Regenerating
…`; the rest of the case and the result toolbar stay on screen, and every regenerate button and the form's submit button are disabled until it finishes. - `gen-export` downloads a `.pdf` (starting with `%PDF`) and a `.docx` (starting with `PK`). ## How to get to it (user POV) @@ -29,10 +30,13 @@ Preconditions: - **Save (one-off).** Generate with no debate and choose `chooseForMotion(page, "once")`. Click the button named `Save` (use `exact`), fill the dialog field labelled `Case title`, and click the dialog's `Save` button. `cases` for the user is `[{title, debate_id: null}]`. On a fresh browser context, `/app/history` shows `One-off and unsorted work` with a list item holding the title. - **Error.** Call `await fake.fail("generate", 500)`, start the debate, and generate. The text `Generation Failed` (use `exact`) appears. Clicking again then succeeds. - **Quota.** Update `user_settings` to set `daily_limit_override: 0` through `admin`, start the debate, then generate. The dialog shows `Daily Access Paused`. `fake.requests()` has no `generate` entry. +- **Regenerate a section.** After generating, click the button named `Regenerate Arguments` (the section titles are `Case Structure`, `Arguments`, `Predicted Opponent Case`, `Prepared Counters`). Hold the request with `page.route("**/functions/v1/ai", ...)` to check the in-flight state: a `status` with `Regenerating Arguments` is visible and `Regenerate Case Structure` is disabled. The toast `Section regenerated` follows. - **Export.** Wait for `page.waitForEvent("download")` while clicking the button named `PDF` or `DOCX` (use `exact`). Check the suggested filename's extension and the file's magic bytes. ## Gotchas +- While any generation is running, the form's submit button reads `Generating Case...`, not `Generate Debate Case`. Find it by `[data-tour="generate-button"]` then. + - The Generate page shows the heading `Ready to Generate` before the first run. Wait for it after navigating, because lazy route chunks load slowly on a cold Vite. - `Save` appears both on the result toolbar and in the dialog. Scope the second click to `getByRole("dialog")`. The toolbar `Save` exists only for one-off results. - A case filed in a debate is titled with the motion, which the header control also shows (truncated and hidden). Look for it inside the History panel, not with a page-wide `getByText`. diff --git a/.claude/skills/verify-debatelab/recipes/case-generation.spec.ts b/.claude/skills/verify-debatelab/recipes/case-generation.spec.ts index 5b0df3ad9..1920fe22a 100644 --- a/.claude/skills/verify-debatelab/recipes/case-generation.spec.ts +++ b/.claude/skills/verify-debatelab/recipes/case-generation.spec.ts @@ -152,6 +152,42 @@ test("gen-export: a generated case exports to PDF and DOCX", async ({ signedIn: } }); +test("gen-regenerate: regenerating one section keeps the rest of the case and its toolbar on screen", async ({ signedIn: page, evidence }) => { + const { output } = await generate(page); + const thesis = output.caseStructure.thesis; + await evidence.shot("01-generated"); + + // Hold the regenerate request so the in-flight screen can be checked. + let release!: () => void; + const held = new Promise((resolve) => { release = resolve; }); + await page.route("**/functions/v1/ai", async (route) => { + await held; + await route.continue(); + }); + const response = aiResponse(page, "generate"); + await page.getByRole("button", { name: "Regenerate Arguments" }).click(); + await evidence.step("Clicked Regenerate Arguments; request held"); + + const overlay = page.getByRole("status").filter({ hasText: "Regenerating Arguments" }); + await expect(overlay).toBeVisible(); + await expect(page.getByText(thesis).first()).toBeVisible(); + await expect(page.getByRole("button", { name: "PDF", exact: true })).toBeVisible(); + await expect(page.getByRole("button", { name: "Regenerate Case Structure" })).toBeDisabled(); + // The form button reads "Generating..." while busy, so find it by its tour anchor. + await expect(page.locator('[data-tour="generate-button"]')).toBeDisabled(); + await evidence.shot("02-regenerating-one-section"); + await evidence.aria("02-regenerating-one-section"); + + release(); + await response; + await page.unroute("**/functions/v1/ai"); + await expect(page.getByText("Section regenerated").first()).toBeVisible({ timeout: AI_TIMEOUT }); + await expect(overlay).toHaveCount(0); + await expect(page.getByText(thesis).first()).toBeVisible(); + await expect(page.getByRole("button", { name: "Regenerate Case Structure" })).toBeEnabled(); + await evidence.shot("03-regenerated"); +}); + test("gen-error: a provider failure shows an error and the retry succeeds", async ({ signedIn: page, evidence }) => { await fake.fail("generate", 500); await openGenerate(page); diff --git a/src/components/OfflineIndicator.tsx b/src/components/OfflineIndicator.tsx index 54dd56f4e..5384f7949 100644 --- a/src/components/OfflineIndicator.tsx +++ b/src/components/OfflineIndicator.tsx @@ -1,20 +1,10 @@ -import { useEffect, useState } from 'react'; import { WifiOff } from 'lucide-react'; import { useTranslation } from 'react-i18next'; +import { useOnlineStatus } from '@/hooks/use-online-status'; export function OfflineIndicator() { const { t } = useTranslation(); - const [offline, setOffline] = useState(() => typeof navigator !== 'undefined' && !navigator.onLine); - - useEffect(() => { - const update = () => setOffline(!navigator.onLine); - window.addEventListener('online', update); - window.addEventListener('offline', update); - return () => { - window.removeEventListener('online', update); - window.removeEventListener('offline', update); - }; - }, []); + const offline = !useOnlineStatus(); if (!offline) return null; diff --git a/src/components/features/generate/GenerateForm.tsx b/src/components/features/generate/GenerateForm.tsx index b94d544e4..cc9a141ff 100644 --- a/src/components/features/generate/GenerateForm.tsx +++ b/src/components/features/generate/GenerateForm.tsx @@ -32,6 +32,26 @@ interface GenerateFormProps { debatePrefill?: boolean; } +function OptionSelect({ value, onValueChange, options, triggerClassName }: { + value: V; + onValueChange: (value: V) => void; + options: { value: V; label: string }[]; + triggerClassName?: string; +}) { + return ( + + ); +} + export function GenerateForm({ onSubmit, isLoading, debatePrefill = true }: GenerateFormProps) { const { t } = useTranslation(); const { generateForm, setGenerateForm } = useSessionStore(); @@ -154,37 +174,29 @@ export function GenerateForm({ onSubmit, isLoading, debatePrefill = true }: Gene {/* Side */}
- + onValueChange={(value) => setValue('side', value)} + options={[ + { value: 'pro', label: t('generateForm.sidePro') }, + { value: 'con', label: t('generateForm.sideCon') }, + ]} + />
{/* Role */}
- + onValueChange={(value) => setValue('role', value)} + options={[ + { value: 'intro', label: t('generateForm.roleIntro') }, + { value: 'r1', label: t('generateForm.roleR1') }, + { value: 'r2', label: t('generateForm.roleR2') }, + { value: 'conc', label: t('generateForm.roleConc') }, + ]} + />
@@ -220,55 +232,46 @@ export function GenerateForm({ onSubmit, isLoading, debatePrefill = true }: Gene {/* Length */}
- + onValueChange={(value) => setValue('length', value)} + triggerClassName="min-w-0" + options={[ + { value: 'brief', label: t('generateForm.lengthBrief') }, + { value: 'standard', label: t('generateForm.lengthStandard') }, + { value: 'comprehensive', label: t('generateForm.lengthComprehensive') }, + ]} + />
{/* Audience */}
- + onValueChange={(value) => setValue('audience', value)} + triggerClassName="min-w-0" + options={[ + { value: 'beginner', label: t('generateForm.audienceBeginner') }, + { value: 'intermediate', label: t('generateForm.audienceIntermediate') }, + { value: 'advanced', label: t('generateForm.audienceAdvanced') }, + ]} + />
{/* Tone */}
- + onValueChange={(value) => setValue('tone', value)} + triggerClassName="min-w-0" + options={[ + { value: 'formal', label: t('generateForm.toneFormal') }, + { value: 'conversational', label: t('generateForm.toneConversational') }, + { value: 'aggressive', label: t('generateForm.toneAggressive') }, + ]} + />

{t('generateForm.styleNote')}

diff --git a/src/components/features/generate/GenerateResults.tsx b/src/components/features/generate/GenerateResults.tsx index dc0bef4f4..aead67166 100644 --- a/src/components/features/generate/GenerateResults.tsx +++ b/src/components/features/generate/GenerateResults.tsx @@ -20,6 +20,8 @@ interface GenerateResultsProps { result: GenerateOutput; discoveredSources?: DiscoveredSource[]; onRegenerateSection?: (section: string) => void; + /** The section being regenerated; the rest of the results stay on screen. */ + regeneratingSection?: string | null; onDiscuss?: () => void; onSave?: () => void; onUndo?: () => void; @@ -75,10 +77,14 @@ function formatDuration(ms: number | undefined, t: (key: string, options?: Recor return t('generateResults.durationS', { value }); } -export function GenerateResults({ result, discoveredSources, onRegenerateSection, onDiscuss, onSave, onUndo, onImportFlowchart, onPracticeCrossfire }: GenerateResultsProps) { +export function GenerateResults({ result, discoveredSources, onRegenerateSection, regeneratingSection = null, onDiscuss, onSave, onUndo, onImportFlowchart, onPracticeCrossfire }: GenerateResultsProps) { const { t } = useTranslation(); const [isExportingGDocs, setIsExportingGDocs] = useState(false); const [showDriveAuth, setShowDriveAuth] = useState(false); + const regenerateState = (section: string) => ({ + isLoading: regeneratingSection === section, + regenerateDisabled: regeneratingSection !== null, + }); const handleCopyToClipboard = async () => { const lines: string[] = []; @@ -251,6 +257,7 @@ export function GenerateResults({ result, discoveredSources, onRegenerateSection description={t('generateResults.exordioDesc')} icon={} onRegenerate={() => onRegenerateSection?.('exordio')} + {...regenerateState('exordio')} >
@@ -272,6 +279,7 @@ export function GenerateResults({ result, discoveredSources, onRegenerateSection description={t('generateResults.caseStructureDesc')} icon={} onRegenerate={() => onRegenerateSection?.('structure')} + {...regenerateState('structure')} >
@@ -305,6 +313,7 @@ export function GenerateResults({ result, discoveredSources, onRegenerateSection description={`${result.contentions?.length || 0} ${t('generateResults.structuredArguments')}`} icon={} onRegenerate={() => onRegenerateSection?.('contentions')} + {...regenerateState('contentions')} >
{result.contentions?.map((contention, index) => ( @@ -319,6 +328,7 @@ export function GenerateResults({ result, discoveredSources, onRegenerateSection description={t('generateResults.predictedOpponentDesc')} icon={} onRegenerate={() => onRegenerateSection?.('opponent')} + {...regenerateState('opponent')} >
@@ -361,6 +371,7 @@ export function GenerateResults({ result, discoveredSources, onRegenerateSection title={t('generateResults.preparedCounters')} description={t('generateResults.preparedCountersDesc')} onRegenerate={() => onRegenerateSection?.('counters')} + {...regenerateState('counters')} >
{result.counters?.map((counter, i) => ( diff --git a/src/components/features/sparring/SparringRound.tsx b/src/components/features/sparring/SparringRound.tsx index 0fc706974..cb783a20f 100644 --- a/src/components/features/sparring/SparringRound.tsx +++ b/src/components/features/sparring/SparringRound.tsx @@ -1,5 +1,6 @@ import { useEffect, useMemo, useState } from 'react'; import { useTranslation } from 'react-i18next'; +import { useOnlineStatus } from '@/hooks/use-online-status'; import { AlertCircle, Bot, Check, ChevronRight, CirclePause, CirclePlay, Clock3, CloudOff, RotateCcw, Send, UserRound, Wifi } from 'lucide-react'; import { Button } from '@/components/ui/button'; import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/card'; @@ -63,7 +64,7 @@ export function SparringRound({ const pauseTimer = useSparringStore((state) => state.pauseTimer); const resumeTimer = useSparringStore((state) => state.resumeTimer); const resetTurnTimer = useSparringStore((state) => state.resetTurnTimer); - const [isOnline, setIsOnline] = useState(() => typeof navigator === 'undefined' || navigator.onLine); + const isOnline = useOnlineStatus(); const currentTurn = session.turns[session.currentTurnIndex]; const currentWordCount = wordCount(session.draft); const totalWords = useMemo(() => session.speeches.reduce((sum, speech) => sum + wordCount(speech.content), 0), [session.speeches]); @@ -76,17 +77,6 @@ export function SparringRound({ const timeExpired = session.timerRemainingSeconds <= 0 || (session.timerDeadline !== null && session.timerDeadline <= Date.now()); - useEffect(() => { - const handleOnline = () => setIsOnline(true); - const handleOffline = () => setIsOnline(false); - window.addEventListener('online', handleOnline); - window.addEventListener('offline', handleOffline); - return () => { - window.removeEventListener('online', handleOnline); - window.removeEventListener('offline', handleOffline); - }; - }, []); - useEffect(() => { if (!userTurn || !session.timerRunning) return; const intervalId = window.setInterval(() => tickTimer(), 250); diff --git a/src/components/features/sparring/SparringSetup.tsx b/src/components/features/sparring/SparringSetup.tsx index 10c028650..39fcb8dfd 100644 --- a/src/components/features/sparring/SparringSetup.tsx +++ b/src/components/features/sparring/SparringSetup.tsx @@ -1,5 +1,6 @@ import { useEffect, useMemo, useRef, useState } from 'react'; import { useTranslation } from 'react-i18next'; +import { useOnlineStatus } from '@/hooks/use-online-status'; import { ArrowRight, Check, ChevronDown, Cpu, Gauge, Swords } from 'lucide-react'; import { Button } from '@/components/ui/button'; import { DebateContextControl } from '@/components/features/debate/DebateContextControl'; @@ -58,18 +59,7 @@ export function SparringSetup({ onStart, isStarting = false, initialValues, bann const [difficulty, setDifficulty] = useState('medium'); const [voice, setVoice] = useState(false); const voiceAvailability = useVoiceAvailability(); - const [isOnline, setIsOnline] = useState(() => typeof navigator === 'undefined' || navigator.onLine); - - useEffect(() => { - const handleOnline = () => setIsOnline(true); - const handleOffline = () => setIsOnline(false); - window.addEventListener('online', handleOnline); - window.addEventListener('offline', handleOffline); - return () => { - window.removeEventListener('online', handleOnline); - window.removeEventListener('offline', handleOffline); - }; - }, []); + const isOnline = useOnlineStatus(); const selectedFormat = useMemo(() => SPARRING_FORMATS.find((item) => item.id === format) ?? SPARRING_FORMATS[0], [format]); const plannedTurns = useMemo(() => buildSparringTurns(format, userSide), [format, userSide]); diff --git a/src/components/ui/section-card.tsx b/src/components/ui/section-card.tsx index d14b98874..9fe49e7b3 100644 --- a/src/components/ui/section-card.tsx +++ b/src/components/ui/section-card.tsx @@ -13,6 +13,8 @@ interface SectionCardProps { className?: string; onRegenerate?: () => void; isLoading?: boolean; + /** Disables the regenerate button without covering this card. */ + regenerateDisabled?: boolean; icon?: React.ReactNode; } @@ -23,12 +25,13 @@ export function SectionCard({ className, onRegenerate, isLoading, + regenerateDisabled, icon, }: SectionCardProps) { const { t } = useTranslation(); return ( - {isLoading && } + {isLoading && }
@@ -43,7 +46,7 @@ export function SectionCard({ variant="ghost" size="icon" onClick={onRegenerate} - disabled={isLoading} + disabled={isLoading || regenerateDisabled} className="h-8 w-8 shrink-0 hover-icon-spin" aria-label={t('common.regenerate', 'Regenerate {{title}}', { title })} > diff --git a/src/hooks/use-online-status.ts b/src/hooks/use-online-status.ts index 583aee870..d4fa067f7 100644 --- a/src/hooks/use-online-status.ts +++ b/src/hooks/use-online-status.ts @@ -1,9 +1,9 @@ import { useEffect, useState } from "react"; export function useOnlineStatus() { - const [online, setOnline] = useState(() => navigator.onLine); + const [online, setOnline] = useState(() => typeof navigator === "undefined" || navigator.onLine); useEffect(() => { - const update = () => setOnline(navigator.onLine); + const update = (event: Event) => setOnline(event.type === "online"); window.addEventListener("online", update); window.addEventListener("offline", update); return () => { diff --git a/src/lib/workspace/artifact-save-queue.ts b/src/lib/workspace/artifact-save-queue.ts new file mode 100644 index 000000000..d0cd66ced --- /dev/null +++ b/src/lib/workspace/artifact-save-queue.ts @@ -0,0 +1,35 @@ +type ArtifactSave = { requestId: string }; + +/** + * Serializes autosaves of workspace artifacts. The RPC overwrites the row for a + * request ID with whatever arrives, so concurrent saves could land out of order + * and store older progress. This keeps one save in flight; while it runs, only + * the newest payload per request ID waits, so a finished round's last save is + * kept when the next round begins. + */ +export function createArtifactSaveQueue( + save: (input: T) => Promise, + onError: (error: unknown) => void = (error) => console.error("Failed to save workspace artifact:", error), +) { + const pending = new Map(); + let saving = false; + + const drain = async () => { + saving = true; + while (pending.size > 0) { + const [requestId, input] = pending.entries().next().value as [string, T]; + pending.delete(requestId); + try { + await save(input); + } catch (error) { + onError(error); + } + } + saving = false; + }; + + return (input: T) => { + pending.set(input.requestId, input); + if (!saving) void drain(); + }; +} diff --git a/src/locales/en.json b/src/locales/en.json index 65993d18b..2af16c88d 100644 --- a/src/locales/en.json +++ b/src/locales/en.json @@ -227,6 +227,7 @@ "docx": "DOCX", "googleDocs": "Google Docs", "regenerate": "Regenerate {{title}}", + "regenerating": "Regenerating {{title}}…", "openingPage": "Opening {{page}}", "page": "Page", "close": "Close", diff --git a/src/locales/es.json b/src/locales/es.json index 7d69a8c9e..0e8b12f3b 100644 --- a/src/locales/es.json +++ b/src/locales/es.json @@ -227,6 +227,7 @@ "docx": "DOCX", "googleDocs": "Google Docs", "regenerate": "Regenerar {{title}}", + "regenerating": "Regenerando {{title}}…", "openingPage": "Abriendo {{page}}", "page": "Página", "close": "Cerrar", diff --git a/src/locales/hu.json b/src/locales/hu.json index 601ff3244..5e599e5df 100644 --- a/src/locales/hu.json +++ b/src/locales/hu.json @@ -227,6 +227,7 @@ "docx": "DOCX", "googleDocs": "Google Docs", "regenerate": "{{title}} újragenerálása", + "regenerating": "{{title}} újragenerálása folyamatban…", "openingPage": "{{page}} megnyitása", "page": "Oldal", "close": "Bezárás", diff --git a/src/pages/CrossfirePage.tsx b/src/pages/CrossfirePage.tsx index b6e50b1cf..5365847ac 100644 --- a/src/pages/CrossfirePage.tsx +++ b/src/pages/CrossfirePage.tsx @@ -60,6 +60,7 @@ import { useDebateMotionGate } from '@/components/features/debate/DebateMotionGa import { activeDebateId, sameMotion, selectCurrentDebate, useDebateStore } from '@/lib/stores/debate-store'; import { toast } from '@/hooks/use-toast'; import { saveWorkspaceArtifact } from '@/lib/workspace/practice-client'; +import { createArtifactSaveQueue } from '@/lib/workspace/artifact-save-queue'; import { useSpeechCapture } from '@/hooks/use-speech-capture'; import { isAppLanguage } from '@/lib/languages'; import { DeliveryFacts } from '@/components/features/voice/DeliveryFacts'; @@ -318,6 +319,8 @@ export default function CrossfirePage() { const [stateOwnerId, setStateOwnerId] = useState(user?.id); const requestRef = useRef(null); const artifactRequestIdRef = useRef(workspace.context?.requestId ?? crypto.randomUUID()); + // Autosaves fire on every change; the queue keeps an older one from landing last. + const [saveArtifact] = useState(() => createArtifactSaveQueue(saveWorkspaceArtifact)); const signalRequestsRef = useRef(new Set()); const shouldReduceMotion = useReducedMotion(); const sourceDebateId = useDebateStore((debates) => debates.oneOff ? null : debates.currentDebateId); @@ -385,7 +388,7 @@ export default function CrossfirePage() { useEffect(() => { if (!workspace.context) return; - void saveWorkspaceArtifact({ + saveArtifact({ documentEpoch: workspace.context.documentEpoch, documentId: workspace.context.documentId, inputSnapshot: state.phase === 'setup' ? state.config : { config: state.config }, @@ -396,7 +399,7 @@ export default function CrossfirePage() { tool: 'crossfire', workspaceId: workspace.context.workspaceId, }); - }, [state, workspace.context]); + }, [saveArtifact, state, workspace.context]); // A round that finishes on this page is recorded in its debate once. const recordedPhaseRef = useRef(state.phase); diff --git a/src/pages/GeneratePage.tsx b/src/pages/GeneratePage.tsx index fdf6e0040..3c60e8387 100644 --- a/src/pages/GeneratePage.tsx +++ b/src/pages/GeneratePage.tsx @@ -59,6 +59,8 @@ export default function GeneratePage() { }, [shouldReduceMotion, successfulGenerationSequence]); const [saveTitle, setSaveTitle] = useState(''); const [discoveredSources, setDiscoveredSources] = useState([]); + // One section at a time: each regeneration merges into the result it started from. + const [regeneratingSection, setRegeneratingSection] = useState(null); const handleUndo = () => { const prev = (autosavedCases || []).find(item => item.data !== generateResult); @@ -116,6 +118,7 @@ export default function GeneratePage() { }; const handleRegenerateSection = (section: string) => { + if (generateLoading || regeneratingSection) return; if (!generateForm?.motion || !generateResult) { toast({ title: t('generate.regenerateUnavailable', 'Cannot regenerate yet'), description: t('generate.regenerateUnavailableDesc', 'Generate a case first, then retry a section.'), variant: 'destructive' }); return; @@ -136,7 +139,7 @@ export default function GeneratePage() { const debateId = resultDebateId !== undefined ? resultDebateId : sameMotion(current?.motion, data.motion) ? activeDebateId() : null; - setGenerateLoading(true); + setRegeneratingSection(section); void generateCase(data, config, workspace.context ?? undefined, workspace.context ? undefined : debateId) .then(async ({ result: next, discoveredSources: sources }) => { const regenerated = { @@ -158,7 +161,7 @@ export default function GeneratePage() { console.error('Failed to regenerate section:', error); toast({ title: t('generate.failed'), description: getUserFacingAIError(error), variant: 'destructive' }); }) - .finally(() => setGenerateLoading(false)); + .finally(() => setRegeneratingSection(null)); }; const handleSave = () => { @@ -370,7 +373,7 @@ export default function GeneratePage() {
{/* Form */} - + {/* Results */} @@ -385,6 +388,7 @@ export default function GeneratePage() { result={generateResult} discoveredSources={discoveredSources} onRegenerateSection={handleRegenerateSection} + regeneratingSection={regeneratingSection} onDiscuss={handleDiscuss} onSave={recordedCaseId ? undefined : handleSave} onUndo={handleUndo} diff --git a/src/pages/SparringPage.tsx b/src/pages/SparringPage.tsx index 278802642..24b249509 100644 --- a/src/pages/SparringPage.tsx +++ b/src/pages/SparringPage.tsx @@ -1,5 +1,6 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { useTranslation } from 'react-i18next'; +import { useOnlineStatus } from '@/hooks/use-online-status'; import { ArrowLeft, CircleHelp } from 'lucide-react'; import { useLocation, useNavigate } from 'react-router-dom'; import { useSparringStore } from '@/lib/stores/sparring-store'; @@ -16,6 +17,7 @@ import { useDebateMotionGate } from '@/components/features/debate/DebateMotionGa import { toast } from '@/hooks/use-toast'; import { WorkspaceToolScope, useWorkspaceToolScope } from '@/components/features/workspace/WorkspaceToolScope'; import { saveWorkspaceArtifact } from '@/lib/workspace/practice-client'; +import { createArtifactSaveQueue } from '@/lib/workspace/artifact-save-queue'; import { OpponentVoiceBar } from '@/components/features/voice/OpponentVoiceBar'; import { useVoiceAvailability } from '@/lib/voice/availability'; import type { SpeechDelivery } from '@/lib/voice/delivery'; @@ -64,13 +66,15 @@ export default function SparringPage() { const [aiError, setAiError] = useState(null); const [judgingError, setJudgingError] = useState(null); const [submissionError, setSubmissionError] = useState(null); - const [isOnline, setIsOnline] = useState(() => typeof navigator === 'undefined' || navigator.onLine); + const isOnline = useOnlineStatus(); const [retryNonce, setRetryNonce] = useState(0); const aiRequestRef = useRef(null); const judgeRequestRef = useRef(null); const aiInFlightRef = useRef(false); const judgeInFlightRef = useRef(false); const artifactRequestIdRef = useRef(workspace.context?.requestId ?? crypto.randomUUID()); + // Autosaves fire on every change; the queue keeps an older one from landing last. + const [saveArtifact] = useState(() => createArtifactSaveQueue(saveWorkspaceArtifact)); const isOnlineRef = useRef(isOnline); const voiceAvailability = useVoiceAvailability(); // A voice round falls back to typing when the microphone is lost. @@ -99,27 +103,16 @@ export default function SparringPage() { navigate(location.pathname, { replace: true, state: { workspaceContext: workspace.context } }); }, [location.pathname, navigate, restoreWorkspaceSession, workspace.context, workspaceArtifact, workspaceLaunch]); + // Declared before the request effects so they read the new status in the same commit. useEffect(() => { - const handleOnline = () => { - isOnlineRef.current = true; - setIsOnline(true); - // Re-run a request that was gated while offline. Do not disturb a call - // that was already in flight before the connectivity event. - if (!aiInFlightRef.current && !judgeInFlightRef.current) { - setRetryNonce((value) => value + 1); - } - }; - const handleOffline = () => { - isOnlineRef.current = false; - setIsOnline(false); - }; - window.addEventListener('online', handleOnline); - window.addEventListener('offline', handleOffline); - return () => { - window.removeEventListener('online', handleOnline); - window.removeEventListener('offline', handleOffline); - }; - }, []); + const reconnected = isOnline && !isOnlineRef.current; + isOnlineRef.current = isOnline; + // Re-run a request that was gated while offline. Do not disturb a call + // that was already in flight before the connectivity event. + if (reconnected && !aiInFlightRef.current && !judgeInFlightRef.current) { + setRetryNonce((value) => value + 1); + } + }, [isOnline]); useEffect(() => { if (session?.id) window.scrollTo({ top: 0, left: 0, behavior: 'auto' }); @@ -256,7 +249,7 @@ export default function SparringPage() { useEffect(() => { if (!workspace.context || !session) return; - void saveWorkspaceArtifact({ + saveArtifact({ documentEpoch: workspace.context.documentEpoch, documentId: workspace.context.documentId, inputSnapshot: { motion: session.motion, side: session.userSide, format: session.format, difficulty: session.difficulty }, @@ -267,7 +260,7 @@ export default function SparringPage() { tool: 'sparring', workspaceId: workspace.context.workspaceId, }); - }, [session, workspace.context]); + }, [saveArtifact, session, workspace.context]); const handleStart = (values: SparringSetupValues) => debateGate.run(values.motion, (target) => beginRound(values, target.debateId)); diff --git a/src/test/components/GenerateResultsRegenerate.test.tsx b/src/test/components/GenerateResultsRegenerate.test.tsx new file mode 100644 index 000000000..c2a06f499 --- /dev/null +++ b/src/test/components/GenerateResultsRegenerate.test.tsx @@ -0,0 +1,41 @@ +import { describe, it, expect, vi } from 'vitest'; +import { render, screen, within } from '@testing-library/react'; +import { GenerateResults } from '@/components/features/generate/GenerateResults'; +import type { GenerateOutput } from '@/lib/ai/schemas/generate'; + +vi.mock('@/lib/supabase/AuthProvider', () => ({ + useAuth: () => ({ user: { id: 'test' }, session: { access_token: 'test' } }), +})); + +const result: GenerateOutput = { + caseStructure: { thesis: 'Distinct thesis text', framework: 'Framework', keyDefinitions: [], burdenOfProof: 'Burden' }, + contentions: [{ id: 'c1', claim: 'First claim', reasoning: 'Because', evidence: 'Data', needsSource: false, impact: 'Big' }], + predictedOpponentCase: { likelyArguments: ['Their point'], strategicWeaknesses: [], keyClashes: [] }, + counters: [{ targetArgument: 'Their point', counterpoint: 'Our answer', rebuttal: 'Rebuttal' }], + metadata: { side: 'pro', format: 'custom', generatedAt: '2026-10-02T10:00:00.000Z' }, +}; + +describe('GenerateResults section regeneration', () => { + it('covers only the regenerating section and keeps the other sections visible', () => { + render(); + + const status = screen.getByRole('status'); + expect(status).toHaveTextContent('Regenerating'); + const contentionsCard = status.closest('.card-hover-glow') as HTMLElement; + expect(within(contentionsCard).getByText('First claim')).toBeInTheDocument(); + expect(screen.getByText('Distinct thesis text')).toBeInTheDocument(); + expect(screen.getByText('Our answer')).toBeInTheDocument(); + + // Every regenerate button waits for the running section to finish. + const buttons = screen.getAllByRole('button', { name: /^Regenerate / }); + expect(buttons.length).toBeGreaterThan(1); + for (const button of buttons) expect(button).toBeDisabled(); + }); + + it('leaves every section enabled when nothing is regenerating', () => { + render(); + + expect(screen.queryByRole('status')).not.toBeInTheDocument(); + for (const button of screen.getAllByRole('button', { name: /^Regenerate / })) expect(button).toBeEnabled(); + }); +}); diff --git a/src/test/pages/CrossfirePage.test.tsx b/src/test/pages/CrossfirePage.test.tsx index 13b7b3ce4..04d8e8daa 100644 --- a/src/test/pages/CrossfirePage.test.tsx +++ b/src/test/pages/CrossfirePage.test.tsx @@ -6,6 +6,11 @@ const mocks = vi.hoisted(() => ({ userId: 'user-a', crossfire: vi.fn<() => Promise>(() => new Promise(() => undefined)), crossfireSignal: vi.fn<(input: unknown, config: unknown, options: { signal?: AbortSignal }) => Promise>(() => Promise.resolve(undefined)), + saveArtifact: vi.fn<(input: { progress: { config: { resolution: string } } }) => Promise>(), +})); + +vi.mock('@/lib/workspace/practice-client', () => ({ + saveWorkspaceArtifact: mocks.saveArtifact, })); vi.mock('@/lib/supabase/AuthProvider', () => ({ @@ -33,6 +38,7 @@ import CrossfirePage from '@/pages/CrossfirePage'; import { createCrossfireSetupState } from '@/lib/crossfire/reducer'; import { loadCrossfireDraft, saveCrossfireDraft } from '@/lib/crossfire/storage'; import { enableOneOffMode, seedCurrentDebate, TEST_DEBATE_ID } from '@/test/helpers/debate'; +import type { WorkspaceToolContext } from '@/lib/workspace/tool-context'; function renderPage() { return render( @@ -53,6 +59,44 @@ describe('CrossfirePage', () => { localStorage.clear(); }); + it('autosaves workspace progress one save at a time so older text never lands last', async () => { + const workspaceContext: WorkspaceToolContext = { + documentEpoch: 2, + documentId: 'bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb', + documentIds: ['bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb'], + documentText: 'Accepted text', + documentTitle: 'R1', + motion: 'Motion', + requestId: 'dddddddd-dddd-4ddd-8ddd-dddddddddddd', + returnUrl: '/app/workspaces/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa', + side: 'pro', + sourceIds: [], + threadRequest: null, + threadId: null, + workspaceId: 'aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa', + }; + const finishSave: (() => void)[] = []; + mocks.saveArtifact.mockImplementation(() => new Promise((resolve) => { finishSave.push(() => resolve('artifact')); })); + + render( + + + , + ); + await waitFor(() => expect(mocks.saveArtifact).toHaveBeenCalledTimes(1)); + + fireEvent.change(screen.getByTestId('crossfire-resolution'), { target: { value: 'Resolved: a' } }); + fireEvent.change(screen.getByTestId('crossfire-resolution'), { target: { value: 'Resolved: ab' } }); + fireEvent.change(screen.getByTestId('crossfire-resolution'), { target: { value: 'Resolved: abc' } }); + // The first save is still running, so nothing else is sent yet. + expect(mocks.saveArtifact).toHaveBeenCalledTimes(1); + + finishSave[0](); + await waitFor(() => expect(mocks.saveArtifact).toHaveBeenCalledTimes(2)); + expect(mocks.saveArtifact.mock.calls[1][0].progress.config.resolution).toBe('Resolved: abc'); + finishSave[1](); + }); + it('supports arrow-key radio navigation', () => { renderPage(); const questionAi = screen.getByTestId('mode-question-ai'); diff --git a/src/test/pages/GeneratePage.test.tsx b/src/test/pages/GeneratePage.test.tsx index b85df04ab..f2653012d 100644 --- a/src/test/pages/GeneratePage.test.tsx +++ b/src/test/pages/GeneratePage.test.tsx @@ -86,8 +86,8 @@ vi.mock('@/components/features/generate/GenerateForm', () => ({ })); vi.mock('@/components/features/generate/GenerateResults', () => ({ - GenerateResults: ({ result, discoveredSources = [], onDiscuss, onSave, onRegenerateSection, onPracticeCrossfire }: { result: { caseStructure: { thesis: string } }, discoveredSources?: { title: string }[], onDiscuss: () => void, onSave: () => void, onRegenerateSection?: (section: string) => void, onPracticeCrossfire?: () => void }) => ( -
+ GenerateResults: ({ result, discoveredSources = [], onDiscuss, onSave, onRegenerateSection, regeneratingSection = null, onPracticeCrossfire }: { result: { caseStructure: { thesis: string } }, discoveredSources?: { title: string }[], onDiscuss: () => void, onSave: () => void, onRegenerateSection?: (section: string) => void, regeneratingSection?: string | null, onPracticeCrossfire?: () => void }) => ( +

Results for {result.caseStructure.thesis}

{discoveredSources.map((source) => source.title).join(',')}
@@ -272,6 +272,38 @@ describe('GeneratePage Integration', () => { expect(screen.getByTestId('discovered-sources')).not.toHaveTextContent('Old source'); }); + it('keeps the results on screen while one section regenerates', async () => { + mockGenerateResult = { + caseStructure: { thesis: 'Existing Thesis' }, + contentions: [] as any[], + predictedOpponentCase: {}, + counters: [] as any[], + metadata: {}, + }; + let finish: (value: unknown) => void = () => undefined; + (generateCase as Mock).mockReturnValueOnce(new Promise((resolve) => { finish = resolve; })); + + render( + + + + ); + + fireEvent.click(screen.getByTestId('regenerate-section')); + + expect(screen.getByTestId('generate-results')).toHaveAttribute('data-regenerating', 'contentions'); + expect(screen.getByTestId('submit-button')).toBeDisabled(); + expect(mockSetGenerateLoading).not.toHaveBeenCalled(); + + // A second click while the first is in flight would merge over a stale result. + fireEvent.click(screen.getByTestId('regenerate-section')); + expect(generateCase).toHaveBeenCalledTimes(1); + + finish({ result: { ...mockGenerateResult, contentions: [{ claim: 'Updated' }] }, discoveredSources: [] }); + await waitFor(() => expect(screen.getByTestId('generate-results')).toHaveAttribute('data-regenerating', '')); + expect(mockSetGenerateLoading).not.toHaveBeenCalled(); + }); + it('records the case in the Current Debate with its side automatically', async () => { seedCurrentDebate({ motion: 'Test Topic' }); const result = { caseStructure: { thesis: 'Debate thesis' }, contentions: [] as any[], predictedOpponentCase: {}, counters: [] as any[] }; diff --git a/src/test/workspace/artifact-save-queue.test.ts b/src/test/workspace/artifact-save-queue.test.ts new file mode 100644 index 000000000..9eb86c456 --- /dev/null +++ b/src/test/workspace/artifact-save-queue.test.ts @@ -0,0 +1,85 @@ +import { describe, expect, it, vi } from "vitest"; +import { createArtifactSaveQueue } from "@/lib/workspace/artifact-save-queue"; + +type Save = { requestId: string; progress: string }; + +function deferredSaves() { + const calls: { input: Save; resolve: () => void; reject: (error: Error) => void }[] = []; + const save = vi.fn((input: Save) => new Promise((resolve, reject) => { + calls.push({ input, resolve: () => resolve("artifact"), reject }); + })); + return { calls, save }; +} + +const flush = () => new Promise((resolve) => setTimeout(resolve, 0)); + +describe("createArtifactSaveQueue", () => { + it("sends one save at a time and only the newest progress after the one in flight", async () => { + const { calls, save } = deferredSaves(); + const enqueue = createArtifactSaveQueue(save); + + enqueue({ requestId: "round", progress: "a" }); + enqueue({ requestId: "round", progress: "ab" }); + enqueue({ requestId: "round", progress: "abc" }); + expect(save).toHaveBeenCalledTimes(1); + + calls[0].resolve(); + await flush(); + expect(save).toHaveBeenCalledTimes(2); + expect(calls[1].input.progress).toBe("abc"); + + calls[1].resolve(); + await flush(); + expect(save).toHaveBeenCalledTimes(2); + }); + + it("never lets an older save land after a newer one", async () => { + const { calls, save } = deferredSaves(); + const enqueue = createArtifactSaveQueue(save); + + enqueue({ requestId: "round", progress: "short" }); + enqueue({ requestId: "round", progress: "longer text" }); + calls[0].resolve(); + await flush(); + calls[1].resolve(); + await flush(); + + expect(save.mock.calls.map(([input]) => input.progress)).toEqual(["short", "longer text"]); + }); + + it("keeps the final save of a finished round when the next round starts", async () => { + const { calls, save } = deferredSaves(); + const enqueue = createArtifactSaveQueue(save); + + enqueue({ requestId: "round-1", progress: "typing" }); + enqueue({ requestId: "round-1", progress: "complete" }); + enqueue({ requestId: "round-2", progress: "setup" }); + calls[0].resolve(); + await flush(); + calls[1].resolve(); + await flush(); + calls[2]?.resolve(); + await flush(); + + expect(save.mock.calls.map(([input]) => [input.requestId, input.progress])).toEqual([ + ["round-1", "typing"], + ["round-1", "complete"], + ["round-2", "setup"], + ]); + }); + + it("keeps saving after a failed save without an unhandled rejection", async () => { + const { calls, save } = deferredSaves(); + const onError = vi.fn(); + const enqueue = createArtifactSaveQueue(save, onError); + + enqueue({ requestId: "round", progress: "a" }); + enqueue({ requestId: "round", progress: "ab" }); + calls[0].reject(new Error("offline")); + await flush(); + + expect(onError).toHaveBeenCalledWith(expect.any(Error)); + expect(save).toHaveBeenCalledTimes(2); + expect(calls[1].input.progress).toBe("ab"); + }); +}); From d393781f60579ec5bdee02e5e1c6b5fbda5730b5 Mon Sep 17 00:00:00 2001 From: Adam Date: Fri, 2 Oct 2026 11:25:18 +0000 Subject: [PATCH 2/2] Keep section regeneration from reverting newer cases Codex review found two ways a pending regeneration could overwrite a newer case. Leaving Generate and coming back reset the page-local in-flight state, so another generation could start and the older callback would then replace its result. And Undo, now reachable while a section regenerates, was reverted when the section came back. The regenerating section now lives in the session store (not persisted), so it survives a remount. A regeneration is dropped if the case changed since it started, and Undo is disabled while one runs. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01CNnE2EyywE54QHAFuAFZr5 --- .../features/generate/GenerateResults.tsx | 2 +- src/lib/stores/session-store.ts | 6 ++ src/pages/GeneratePage.tsx | 14 +++-- .../GenerateResultsRegenerate.test.tsx | 7 ++- src/test/pages/GeneratePage.test.tsx | 62 +++++++++++++++++-- 5 files changed, 78 insertions(+), 13 deletions(-) diff --git a/src/components/features/generate/GenerateResults.tsx b/src/components/features/generate/GenerateResults.tsx index aead67166..ac9630c21 100644 --- a/src/components/features/generate/GenerateResults.tsx +++ b/src/components/features/generate/GenerateResults.tsx @@ -239,7 +239,7 @@ export function GenerateResults({ result, discoveredSources, onRegenerateSection )} {onUndo && ( - diff --git a/src/lib/stores/session-store.ts b/src/lib/stores/session-store.ts index 436398935..7918535ea 100644 --- a/src/lib/stores/session-store.ts +++ b/src/lib/stores/session-store.ts @@ -123,6 +123,8 @@ export interface FlowchartData { export interface SessionState { generateResult: GenerateOutput | null; generateLoading: boolean; + /** The Generate section being regenerated; kept here so it survives leaving the page. */ + generateRegeneratingSection: string | null; attackResult: AttackOutput | null; attackLoading: boolean; @@ -169,6 +171,7 @@ export interface SessionState { setGenerateResult: (result: GenerateOutput | null) => void; setGenerateLoading: (loading: boolean) => void; + setGenerateRegeneratingSection: (section: string | null) => void; saveCurrentCase: (title: string) => Promise; deleteSavedCase: (id: string) => Promise; loadSavedCase: (id: string) => void; @@ -451,6 +454,7 @@ export const useSessionStore = create()( (set, get) => ({ generateResult: null as GenerateOutput | null, generateLoading: false, + generateRegeneratingSection: null as string | null, attackResult: null as AttackOutput | null, attackLoading: false, brainstormResult: null as BrainstormOutput | null, @@ -808,6 +812,7 @@ export const useSessionStore = create()( } }, setGenerateLoading: (loading) => set({ generateLoading: loading }), + setGenerateRegeneratingSection: (section) => set({ generateRegeneratingSection: section }), saveCurrentCase: async (title) => { const current = get().generateResult; if (!current) return; @@ -1275,6 +1280,7 @@ export const useSessionStore = create()( }, partialize: ({ generateLoading, + generateRegeneratingSection, attackLoading, brainstormLoading, factoidsLoading, diff --git a/src/pages/GeneratePage.tsx b/src/pages/GeneratePage.tsx index 3c60e8387..0ae80763f 100644 --- a/src/pages/GeneratePage.tsx +++ b/src/pages/GeneratePage.tsx @@ -32,7 +32,7 @@ import { useDebateMotionGate } from '@/components/features/debate/DebateMotionGa export default function GeneratePage() { const workspace = useWorkspaceToolScope('generate'); - const { generateResult, generateLoading, generateForm, setGenerateResult, setGenerateLoading, saveCurrentCase, addAskMessage, setAskLoading, askMessages, setPendingFlowchartData, autosavedCases, recordResult, updateRecordedResult } = useSessionStore(); + const { generateResult, generateLoading, generateRegeneratingSection: regeneratingSection, generateForm, setGenerateResult, setGenerateLoading, setGenerateRegeneratingSection, saveCurrentCase, addAskMessage, setAskLoading, askMessages, setPendingFlowchartData, autosavedCases, recordResult, updateRecordedResult } = useSessionStore(); const debateGate = useDebateMotionGate({ disabled: Boolean(workspace.context) }); // The debate row this page's current result was recorded into, if any. const [recordedCaseId, setRecordedCaseId] = useState(null); @@ -59,8 +59,6 @@ export default function GeneratePage() { }, [shouldReduceMotion, successfulGenerationSequence]); const [saveTitle, setSaveTitle] = useState(''); const [discoveredSources, setDiscoveredSources] = useState([]); - // One section at a time: each regeneration merges into the result it started from. - const [regeneratingSection, setRegeneratingSection] = useState(null); const handleUndo = () => { const prev = (autosavedCases || []).find(item => item.data !== generateResult); @@ -139,11 +137,15 @@ export default function GeneratePage() { const debateId = resultDebateId !== undefined ? resultDebateId : sameMotion(current?.motion, data.motion) ? activeDebateId() : null; - setRegeneratingSection(section); + // One section at a time: each regeneration merges into the result it started from. + const base = generateResult; + setGenerateRegeneratingSection(section); void generateCase(data, config, workspace.context ?? undefined, workspace.context ? undefined : debateId) .then(async ({ result: next, discoveredSources: sources }) => { + // Undo or a new generation replaced the case meanwhile; merging now would revert it. + if (useSessionStore.getState().generateResult !== base) return; const regenerated = { - ...generateResult, + ...base, ...(section === 'structure' ? { caseStructure: next.caseStructure } : {}), ...(section === 'contentions' ? { contentions: next.contentions } : {}), ...(section === 'opponent' ? { predictedOpponentCase: next.predictedOpponentCase } : {}), @@ -161,7 +163,7 @@ export default function GeneratePage() { console.error('Failed to regenerate section:', error); toast({ title: t('generate.failed'), description: getUserFacingAIError(error), variant: 'destructive' }); }) - .finally(() => setRegeneratingSection(null)); + .finally(() => setGenerateRegeneratingSection(null)); }; const handleSave = () => { diff --git a/src/test/components/GenerateResultsRegenerate.test.tsx b/src/test/components/GenerateResultsRegenerate.test.tsx index c2a06f499..8bc5b76a0 100644 --- a/src/test/components/GenerateResultsRegenerate.test.tsx +++ b/src/test/components/GenerateResultsRegenerate.test.tsx @@ -17,7 +17,7 @@ const result: GenerateOutput = { describe('GenerateResults section regeneration', () => { it('covers only the regenerating section and keeps the other sections visible', () => { - render(); + render(); const status = screen.getByRole('status'); expect(status).toHaveTextContent('Regenerating'); @@ -30,12 +30,15 @@ describe('GenerateResults section regeneration', () => { const buttons = screen.getAllByRole('button', { name: /^Regenerate / }); expect(buttons.length).toBeGreaterThan(1); for (const button of buttons) expect(button).toBeDisabled(); + // Undo now would be reverted when the section comes back. + expect(screen.getByRole('button', { name: 'Undo' })).toBeDisabled(); }); it('leaves every section enabled when nothing is regenerating', () => { - render(); + render(); expect(screen.queryByRole('status')).not.toBeInTheDocument(); for (const button of screen.getAllByRole('button', { name: /^Regenerate / })) expect(button).toBeEnabled(); + expect(screen.getByRole('button', { name: 'Undo' })).toBeEnabled(); }); }); diff --git a/src/test/pages/GeneratePage.test.tsx b/src/test/pages/GeneratePage.test.tsx index f2653012d..e21870df6 100644 --- a/src/test/pages/GeneratePage.test.tsx +++ b/src/test/pages/GeneratePage.test.tsx @@ -46,14 +46,24 @@ const mockSetAskLoading = vi.fn(); let mockGenerateResult: any = null; let mockGenerateForm: any = { motion: 'Existing motion', side: 'pro', format: 'bp', role: 'intro', length: 'brief', audience: 'beginner', tone: 'formal' }; -vi.mock('@/lib/stores/session-store', () => ({ - useSessionStore: () => ({ +let mockRegeneratingSection: string | null = null; +const mockStoreListeners = new Set<() => void>(); +const mockSetRegeneratingSection = vi.fn((section: string | null) => { + mockRegeneratingSection = section; + mockStoreListeners.forEach((listener) => listener()); +}); + +vi.mock('@/lib/stores/session-store', async () => { + const React = await import('react'); + const state = () => ({ generateResult: mockGenerateResult, generateLoading: false, + generateRegeneratingSection: mockRegeneratingSection, generateForm: mockGenerateForm, autosavedCases: [] as any[], setGenerateResult: mockSetGenerateResult, setGenerateLoading: mockSetGenerateLoading, + setGenerateRegeneratingSection: mockSetRegeneratingSection, saveCurrentCase: mockSaveCurrentCase, addAskMessage: mockAddAskMessage, setAskLoading: mockSetAskLoading, @@ -61,8 +71,18 @@ vi.mock('@/lib/stores/session-store', () => ({ setPendingFlowchartData: vi.fn(), recordResult: mockRecordResult, updateRecordedResult: mockUpdateRecordedResult, - }), -})); + }); + const useSessionStore = () => { + const [, rerender] = React.useReducer((count: number) => count + 1, 0); + React.useEffect(() => { + mockStoreListeners.add(rerender); + return () => { mockStoreListeners.delete(rerender); }; + }, []); + return state(); + }; + useSessionStore.getState = state; + return { useSessionStore }; +}); vi.mock('@/lib/stores/settings-store', () => ({ useSettingsStore: () => ({ @@ -106,6 +126,7 @@ describe('GeneratePage Integration', () => { enableOneOffMode(); vi.clearAllMocks(); mockGenerateResult = null; + mockRegeneratingSection = null; mockGenerateForm = { motion: 'Existing motion', side: 'pro', format: 'bp', role: 'intro', length: 'brief', audience: 'beginner', tone: 'formal' }; }); @@ -304,6 +325,39 @@ describe('GeneratePage Integration', () => { expect(mockSetGenerateLoading).not.toHaveBeenCalled(); }); + it('keeps a pending regeneration visible after leaving Generate and coming back', async () => { + mockGenerateResult = { caseStructure: { thesis: 'Existing Thesis' }, contentions: [] as any[], predictedOpponentCase: {}, counters: [] as any[], metadata: {} }; + (generateCase as Mock).mockReturnValueOnce(new Promise(() => undefined)); + + const first = render(); + fireEvent.click(screen.getByTestId('regenerate-section')); + first.unmount(); + + render(); + expect(screen.getByTestId('generate-results')).toHaveAttribute('data-regenerating', 'contentions'); + expect(screen.getByTestId('submit-button')).toBeDisabled(); + fireEvent.click(screen.getByTestId('regenerate-section')); + expect(generateCase).toHaveBeenCalledTimes(1); + }); + + it('drops a regenerated section when the case changed while it was running', async () => { + const original = { caseStructure: { thesis: 'Existing Thesis' }, contentions: [] as any[], predictedOpponentCase: {}, counters: [] as any[], metadata: {} }; + mockGenerateResult = original; + let finish: (value: unknown) => void = () => undefined; + (generateCase as Mock).mockReturnValueOnce(new Promise((resolve) => { finish = resolve; })); + + render(); + fireEvent.click(screen.getByTestId('regenerate-section')); + + // Undo, or a new generation, replaced the case before the section came back. + mockGenerateResult = { ...original, caseStructure: { thesis: 'Restored Thesis' } }; + finish({ result: { ...original, contentions: [{ claim: 'Updated' }] }, discoveredSources: [] }); + + await waitFor(() => expect(mockSetRegeneratingSection).toHaveBeenLastCalledWith(null)); + expect(mockSetGenerateResult).not.toHaveBeenCalled(); + expect(mockUpdateRecordedResult).not.toHaveBeenCalled(); + }); + it('records the case in the Current Debate with its side automatically', async () => { seedCurrentDebate({ motion: 'Test Topic' }); const result = { caseStructure: { thesis: 'Debate thesis' }, contentions: [] as any[], predictedOpponentCase: {}, counters: [] as any[] };