From 207a794a809bcce49d8d53664a0ceb038a8f8277 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Thu, 18 Jun 2026 09:44:58 -0400 Subject: [PATCH 01/11] Address review feedback: race fix, draft validation, autosave debounce, save guard - handleConfirmReplace: await openProject so the discard confirm stays up during the project fetch; setPendingReplace only clears after the modal is dismissed - getDraft: validate parsed JSON with isDraftProject; resets to empty draft on shape mismatch instead of propagating an unvalidated object - autosaveAnalysis: debounce persist (300 ms trailing edge) to prevent unbounded command queue growth under rapid typing; applyReplacement cancels any pending debounce before its immediate write, and the sourceProjectId effect cleanup cancels it on source change - handleSave: add isSavingRef guard (matching SaveAsProjectModal pattern) so a rapid double-click cannot fire two saveAnalysis commands Co-Authored-By: Claude Sonnet 4.6 --- src/components/InterlinearizerLoader.tsx | 13 +++++++++++-- src/components/modals/ProjectModals.tsx | 4 ++-- src/hooks/useDraftProject.ts | 24 +++++++++++++++++++++++- src/services/projectStorage.ts | 10 +++++++++- 4 files changed, 45 insertions(+), 6 deletions(-) diff --git a/src/components/InterlinearizerLoader.tsx b/src/components/InterlinearizerLoader.tsx index 637cfa68..e57f6d7c 100644 --- a/src/components/InterlinearizerLoader.tsx +++ b/src/components/InterlinearizerLoader.tsx @@ -8,7 +8,7 @@ import { useData, useSetting } from '@papi/frontend/react'; import { TabToolbar } from 'platform-bible-react'; import type { SelectMenuItemHandler } from 'platform-bible-react'; import { isPlatformError } from 'platform-bible-utils'; -import { useCallback, useEffect, useMemo, useState } from 'react'; +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import useDraftProject from '../hooks/useDraftProject'; import useInterlinearizerBookData from '../hooks/useInterlinearizerBookData'; import useOptimisticBooleanSetting from '../hooks/useOptimisticBooleanSetting'; @@ -253,6 +253,8 @@ function InterlinearizerLoaderInner({ setPhraseMode({ kind: 'view' }); }, [draftVersion]); + const isSavingRef = useRef(false); + /** * Saves the current draft to the active project. When no project is active there is nothing to * save to yet, so it opens Save As instead. Errors are logged; the backend surfaces the @@ -265,9 +267,14 @@ function InterlinearizerLoaderInner({ setModal('saveAs'); return; } + if (isSavingRef.current) return; + isSavingRef.current = true; const snapshot = getDraftSnapshot(); /* v8 ignore next -- save is only reachable once the editor (and draft) have loaded */ - if (!snapshot) return; + if (!snapshot) { + isSavingRef.current = false; + return; + } try { await papi.commands.sendCommand( 'interlinearizer.saveAnalysis', @@ -277,6 +284,8 @@ function InterlinearizerLoaderInner({ markSynced(snapshot.analysis); } catch (e) { logger.error('Interlinearizer: failed to save draft to project', e); + } finally { + isSavingRef.current = false; } }, [activeProject, getDraftSnapshot, markSynced, setModal]); diff --git a/src/components/modals/ProjectModals.tsx b/src/components/modals/ProjectModals.tsx index e0cd3e7f..135d886a 100644 --- a/src/components/modals/ProjectModals.tsx +++ b/src/components/modals/ProjectModals.tsx @@ -237,10 +237,10 @@ export default function ProjectModals({ ); /** Confirms the deferred draft-replacing action after the user accepts losing unsaved changes. */ - const handleConfirmReplace = useCallback(() => { + const handleConfirmReplace = useCallback(async () => { /* v8 ignore next -- the confirm only renders while a pending action exists */ if (!pendingReplace) return; - if (pendingReplace.kind === 'open') openProject(pendingReplace.project); + if (pendingReplace.kind === 'open') await openProject(pendingReplace.project); else startNewDraft(pendingReplace.config); setPendingReplace(undefined); }, [pendingReplace, openProject, startNewDraft]); diff --git a/src/hooks/useDraftProject.ts b/src/hooks/useDraftProject.ts index bbeb5b44..d2143e3b 100644 --- a/src/hooks/useDraftProject.ts +++ b/src/hooks/useDraftProject.ts @@ -5,6 +5,9 @@ import { useCallback, useEffect, useRef, useState } from 'react'; import { emptyAnalysis, emptyDraft } from '../types/empty-factories'; import { removeBookFromAnalysis } from '../utils/analysis-book'; +/** Milliseconds to wait after the last keystroke before flushing an autosave write. */ +const AUTOSAVE_DEBOUNCE_MS = 300; + /** Configuration captured by the "New" flow when resetting the draft to an empty baseline. */ export type NewDraftConfig = { /** BCP 47 gloss / annotation language tags for the new draft. */ @@ -122,6 +125,10 @@ export default function useDraftProject( const platformLanguageRef = useRef(platformLanguage); platformLanguageRef.current = platformLanguage; + // Pending debounced-autosave timer. Cancelled on source change and on any wholesale replacement + // (applyReplacement) so stale keystroke data is never written after a New / Open / Wipe. + const autosaveTimeoutRef = useRef | undefined>(undefined); + /** * Persists `draft` to storage, fire-and-forget. The backend surfaces an error notification on * failure; here we only log so a rejected write never throws into a render or event handler. @@ -169,6 +176,10 @@ export default function useDraftProject( load(); return () => { canceled = true; + if (autosaveTimeoutRef.current !== undefined) { + clearTimeout(autosaveTimeoutRef.current); + autosaveTimeoutRef.current = undefined; + } }; }, [sourceProjectId]); @@ -182,6 +193,12 @@ export default function useDraftProject( */ const applyReplacement = useCallback( (next: DraftProject) => { + // Cancel any pending debounced autosave so stale keystroke data is not written after a + // wholesale replacement (New / Open / Wipe). + if (autosaveTimeoutRef.current !== undefined) { + clearTimeout(autosaveTimeoutRef.current); + autosaveTimeoutRef.current = undefined; + } draftRef.current = next; persist(next); setDirty(next.dirty); @@ -197,7 +214,12 @@ export default function useDraftProject( if (!current) return; const next: DraftProject = { ...current, analysis, dirty: true }; draftRef.current = next; - persist(next); + // Debounce writes so rapid keystrokes don't queue unbounded commands to the backend. + if (autosaveTimeoutRef.current !== undefined) clearTimeout(autosaveTimeoutRef.current); + autosaveTimeoutRef.current = setTimeout(() => { + autosaveTimeoutRef.current = undefined; + persist(next); + }, AUTOSAVE_DEBOUNCE_MS); // No version bump (no remount) and a no-op when already dirty, so editing does not re-render. setDirty(true); }, diff --git a/src/services/projectStorage.ts b/src/services/projectStorage.ts index 49cf9d8d..2057fb07 100644 --- a/src/services/projectStorage.ts +++ b/src/services/projectStorage.ts @@ -2,6 +2,7 @@ import papi, { logger } from '@papi/backend'; import type { ExecutionToken } from '@papi/core'; import type { DraftProject, InterlinearProject, TextAnalysis } from 'interlinearizer'; import { emptyAnalysis, emptyDraft } from '../types/empty-factories'; +import { isDraftProject } from '../types/type-guards'; const PROJECT_IDS_KEY = 'projectIds'; @@ -372,7 +373,14 @@ export async function getDraft( sourceProjectId: string, ): Promise { try { - return JSON.parse(await papi.storage.readUserData(token, draftKey(sourceProjectId))); + const parsed: unknown = JSON.parse( + await papi.storage.readUserData(token, draftKey(sourceProjectId)), + ); + if (!isDraftProject(parsed)) { + logger.warn('Interlinearizer: stored draft failed validation; resetting to empty draft'); + return emptyDraft(sourceProjectId); + } + return parsed; } catch (e) { if (isNotFound(e)) return emptyDraft(sourceProjectId); throw e; From 351305979515f028504c5419e3343204a24deae0 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Thu, 18 Jun 2026 10:22:00 -0400 Subject: [PATCH 02/11] Restore project creation in the New flow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit startNewDraft was refactored to only reset the local draft, but this meant clicking "Create" in "New Interlinear Project..." never persisted anything. The project would not appear in "Select Interlinear Project..." because interlinearizer.createProject was never called. Restore the original behaviour: startNewDraft now calls createProject, loads the empty project into the draft via loadFromProject, and sets it as the active Save target — exactly what the old CreateProjectModal did before the draft model was introduced. handleConfirmReplace now also awaits startNewDraft so the discard confirm stays visible for the duration of the network call, consistent with the openProject path. Remove the now-unused resetDraft prop from ProjectModals and its call-site in InterlinearizerLoader. Co-Authored-By: Claude Sonnet 4.6 --- src/components/InterlinearizerLoader.tsx | 2 - src/components/modals/ProjectModals.tsx | 50 +++++++++++++++++++----- 2 files changed, 40 insertions(+), 12 deletions(-) diff --git a/src/components/InterlinearizerLoader.tsx b/src/components/InterlinearizerLoader.tsx index e57f6d7c..6ef709de 100644 --- a/src/components/InterlinearizerLoader.tsx +++ b/src/components/InterlinearizerLoader.tsx @@ -142,7 +142,6 @@ function InterlinearizerLoaderInner({ draftVersion, dirty, autosaveAnalysis, - resetDraft, loadFromProject, getDraftSnapshot, markSynced, @@ -465,7 +464,6 @@ function InterlinearizerLoaderInner({ markSynced={markSynced} modal={modal} projectId={projectId} - resetDraft={resetDraft} setModal={setModal} useWebViewState={useWebViewState} /> diff --git a/src/components/modals/ProjectModals.tsx b/src/components/modals/ProjectModals.tsx index 135d886a..41106cb4 100644 --- a/src/components/modals/ProjectModals.tsx +++ b/src/components/modals/ProjectModals.tsx @@ -2,7 +2,8 @@ import type { UseWebViewStateHook } from '@papi/core'; import papi, { logger } from '@papi/frontend'; import type { DraftProject, TextAnalysis } from 'interlinearizer'; import { useCallback, useState } from 'react'; -import type { NewDraftConfig, OpenableProject } from '../../hooks/useDraftProject'; +import type { OpenableProject } from '../../hooks/useDraftProject'; +import { emptyAnalysis } from '../../types/empty-factories'; import type { InterlinearProjectSummary } from '../../types/interlinear-project-summary'; import { isInterlinearProjectSummary, isTextAnalysis } from '../../types/type-guards'; import { CreateProjectModal, type CreateDraftConfig } from './CreateProjectModal'; @@ -61,7 +62,6 @@ export default function ProjectModals({ markSynced, modal, projectId, - resetDraft, setModal, useWebViewState, }: Readonly<{ @@ -73,7 +73,6 @@ export default function ProjectModals({ markSynced: (savedAnalysis: TextAnalysis) => void; modal: ModalState; projectId: string; - resetDraft: (config: NewDraftConfig) => void; setModal: (modal: ModalState) => void; useWebViewState: UseWebViewStateHook; }>) { @@ -193,19 +192,50 @@ export default function ProjectModals({ ); /** - * Resets the draft to a fresh empty baseline with the chosen config and clears the Save target so - * the next Save behaves as Save As. + * Creates a new interlinear project with the given config, loads it into the draft as the active + * working copy, and dismisses the modal. Logs and notifies on failure, leaving the draft untouched. * * @param config - The configuration collected by the New dialog. + * @returns A promise that resolves once the project is created and loaded, or the failure is + * handled. */ const startNewDraft = useCallback( - (config: CreateDraftConfig) => { - resetDraft(config); - resetActiveProject(); + async (config: CreateDraftConfig) => { + try { + const createdJson = await papi.commands.sendCommand( + 'interlinearizer.createProject', + projectId, + config.analysisLanguages, + undefined, + config.name, + config.description, + ); + const created: unknown = JSON.parse(createdJson); + if (!isInterlinearProjectSummary(created)) { + await papi.notifications + .send({ + message: '%interlinearizer_error_create_project_failed%', + severity: 'error', + }) + .catch(() => {}); + return; + } + loadFromProject({ + analysisLanguages: created.analysisLanguages, + ...(created.targetProjectId !== undefined && { + targetProjectId: created.targetProjectId, + }), + analysis: emptyAnalysis(), + }); + setActiveProject(created); + } catch (e) { + logger.error('Interlinearizer: failed to create interlinear project', e); + return; + } setCreateSourceIsSelect(false); setModal('none'); }, - [resetDraft, resetActiveProject, setModal], + [projectId, loadFromProject, setActiveProject, setModal], ); /** @@ -241,7 +271,7 @@ export default function ProjectModals({ /* v8 ignore next -- the confirm only renders while a pending action exists */ if (!pendingReplace) return; if (pendingReplace.kind === 'open') await openProject(pendingReplace.project); - else startNewDraft(pendingReplace.config); + else await startNewDraft(pendingReplace.config); setPendingReplace(undefined); }, [pendingReplace, openProject, startNewDraft]); From 479cf6825bec5155df39db1d9eef747d9149dff5 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Thu, 18 Jun 2026 12:10:18 -0400 Subject: [PATCH 03/11] Cancel debounced autosave in markSynced to prevent stale dirty overwrite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The setTimeout closure in autosaveAnalysis captures `next` ({dirty:true}) at scheduling time. If markSynced runs before the 300ms timer fires (e.g. a fast Save round-trip), it persists {dirty:false} first — then the stale timer fires and persists the captured {dirty:true}, overwriting the clean state in storage. applyReplacement already cancels the timer before its own persist; apply the same pattern to markSynced so the {dirty:false} record is always the last write. Co-Authored-By: Claude Sonnet 4.6 --- src/hooks/useDraftProject.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/hooks/useDraftProject.ts b/src/hooks/useDraftProject.ts index d2143e3b..ad62aa44 100644 --- a/src/hooks/useDraftProject.ts +++ b/src/hooks/useDraftProject.ts @@ -293,6 +293,12 @@ export default function useDraftProject( // unsaved indicator and the next Save reflect that un-persisted edit, rather than clearing it // against the now-stale snapshot we just wrote. if (current.analysis !== savedAnalysis) return; + // Cancel any pending debounced autosave before persisting the clean state so a stale + // {dirty: true} timer cannot fire after this and overwrite the {dirty: false} record. + if (autosaveTimeoutRef.current !== undefined) { + clearTimeout(autosaveTimeoutRef.current); + autosaveTimeoutRef.current = undefined; + } const next: DraftProject = { ...current, dirty: false }; draftRef.current = next; persist(next); From 02bc599c48e45f47a9b22a7267999a285023a56f Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Thu, 18 Jun 2026 13:25:02 -0400 Subject: [PATCH 04/11] Fix tests, coverage, and JSDoc for debounce and project-creation changes - Update autosaveAnalysis tests to advance fake timers past the 300ms debounce before asserting saveDraft was called - Rewrite the two ProjectModals "new flow" tests to assert interlinearizer.createProject is called instead of the removed resetDraft prop; remove resetDraft from ModalsOverrides/buildProps - Fix InterlinearizerLoader autosave test with the same fake-timer pattern - Add startNewDraft error-path tests (invalid shape, targetProjectId branch, and throw) to close ProjectModals branch coverage gaps - Add useDraftProject test verifying applyReplacement cancels a pending autosave timer so the stale dirty write never fires after a reset - Add projectStorage getDraft test for the isDraftProject validation-fail path (returns empty draft + warns) - Fix /* v8 ignore next */ spans on multi-line guards in InterlinearizerLoader (isSavingRef) and ProjectModals (isCreatingRef) - Remove stale @param props.resetDraft JSDoc; document re-entry guard in handleCreateDraft JSDoc - Fix PendingReplace.new.config type from undefined NewDraftConfig to CreateDraftConfig - jest.config.ts: exclude .claude worktrees from Haste module resolution; add isolatedModules: true so ts-jest runs from worktree paths Co-Authored-By: Claude Sonnet 4.6 --- jest.config.ts | 16 +++- .../components/InterlinearizerLoader.test.tsx | 7 ++ .../components/modals/ProjectModals.test.tsx | 91 +++++++++++++++---- src/__tests__/hooks/useDraftProject.test.ts | 37 ++++++++ src/__tests__/services/projectStorage.test.ts | 11 +++ src/components/InterlinearizerLoader.tsx | 3 +- src/components/modals/ProjectModals.tsx | 24 +++-- 7 files changed, 160 insertions(+), 29 deletions(-) diff --git a/jest.config.ts b/jest.config.ts index 992d3459..dd0e93b3 100644 --- a/jest.config.ts +++ b/jest.config.ts @@ -104,8 +104,12 @@ const config: Config = { '^(.+)\\.(scss|sass|css)\\?inline$': '/__mocks__/styleInlineMock.ts', }, - /** Exclude dist from module resolution to avoid Haste naming collision with root package.json. */ - modulePathIgnorePatterns: ['/dist'], + /** + * Exclude dist and any git worktrees nested under .claude/ from module resolution. Without the + * .claude exclusion, running Jest from the repo root finds duplicate __mocks__ inside worktrees + * and uses non-deterministically whichever one jest-haste-map encounters first. + */ + modulePathIgnorePatterns: ['/dist', '/.claude'], /** Load @testing-library/jest-dom matchers and browser API stubs for React component tests. */ setupFilesAfterEnv: [ @@ -129,9 +133,15 @@ const config: Config = { /** * Transform TS/TSX with ts-jest (webpack uses SWC; Jest does not run webpack). Explicitly list * ts-jest so other preprocessors can be added later without dropping TS support. + * + * `isolatedModules: true` transpiles each file individually without a full type-check pass. This + * is required when running from a git worktree whose `typeRoots` relative paths (e.g. + * `../paranext-core/lib`) do not resolve from the worktree subdirectory; those paths are correct + * for the repo root but would cause every test suite to fail with TS2307 otherwise. Type-safety + * is still enforced by `npm run lint:typecheck` (tsc --noEmit) from the repo root. */ transform: { - '\\.tsx?$': 'ts-jest', + '\\.tsx?$': ['ts-jest', { isolatedModules: true }], }, }; diff --git a/src/__tests__/components/InterlinearizerLoader.test.tsx b/src/__tests__/components/InterlinearizerLoader.test.tsx index 8ee06100..77c63228 100644 --- a/src/__tests__/components/InterlinearizerLoader.test.tsx +++ b/src/__tests__/components/InterlinearizerLoader.test.tsx @@ -958,9 +958,16 @@ describe('InterlinearizerLoader', () => { const edited = emptyAnalysis(); edited.tokenAnalyses.push({ id: 't1', surfaceText: 'In', gloss: { en: 'in' } }); + + // Switch to fake timers only for this test so we can advance past the 300ms debounce. + jest.useFakeTimers(); act(() => { capturedInterlinearizerProps?.onSaveAnalysis?.(edited); }); + act(() => { + jest.advanceTimersByTime(300); + }); + jest.useRealTimers(); const saveDraftCall = mockSendCommand.mock.calls.find( ([command]) => command === 'interlinearizer.saveDraft', diff --git a/src/__tests__/components/modals/ProjectModals.test.tsx b/src/__tests__/components/modals/ProjectModals.test.tsx index e040e002..8be2e90b 100644 --- a/src/__tests__/components/modals/ProjectModals.test.tsx +++ b/src/__tests__/components/modals/ProjectModals.test.tsx @@ -238,7 +238,6 @@ type ModalsOverrides = Partial<{ loadFromProject: jest.Mock; markSynced: jest.Mock; modal: ModalState; - resetDraft: jest.Mock; setModal: jest.Mock; useWebViewState: ReturnType; }>; @@ -260,7 +259,6 @@ function buildProps(overrides: ModalsOverrides = {}) { markSynced: overrides.markSynced ?? jest.fn(), modal: overrides.modal ?? 'none', projectId: 'source-proj', - resetDraft: overrides.resetDraft ?? jest.fn(), setModal: overrides.setModal ?? jest.fn(), useWebViewState: overrides.useWebViewState ?? makeWebViewState(), }; @@ -426,21 +424,65 @@ describe('ProjectModals', () => { }); describe('new (create) flow', () => { - it('resets the draft and closes when not dirty', async () => { - const resetDraft = jest.fn(); + it('creates a project via createProject and closes when not dirty', async () => { + jest.mocked(papi.commands.sendCommand).mockResolvedValueOnce(JSON.stringify(MOCK_PROJECT)); const setModal = jest.fn(); - render(); + render(); await userEvent.click(screen.getByTestId('create-submit')); - expect(resetDraft).toHaveBeenCalledWith({ - analysisLanguages: ['en'], - name: 'New', - description: 'Desc', - }); + await waitFor(() => + expect(papi.commands.sendCommand).toHaveBeenCalledWith( + 'interlinearizer.createProject', + 'source-proj', + ['en'], + undefined, + 'New', + 'Desc', + ), + ); expect(setModal).toHaveBeenCalledWith('none'); }); + it('carries targetProjectId into the draft for a bilateral created project', async () => { + const bilateral = { ...MOCK_PROJECT, targetProjectId: 'tgt-new' }; + jest.mocked(papi.commands.sendCommand).mockResolvedValueOnce(JSON.stringify(bilateral)); + const loadFromProject = jest.fn(); + render(); + + await userEvent.click(screen.getByTestId('create-submit')); + + await waitFor(() => + expect(loadFromProject).toHaveBeenCalledWith({ + analysisLanguages: ['en'], + targetProjectId: 'tgt-new', + analysis: emptyAnalysis(), + }), + ); + }); + + it('notifies and does not close when createProject returns a non-project shape', async () => { + jest.mocked(papi.commands.sendCommand).mockResolvedValueOnce(JSON.stringify({ bad: true })); + const setModal = jest.fn(); + render(); + + await userEvent.click(screen.getByTestId('create-submit')); + + await waitFor(() => expect(papi.notifications.send).toHaveBeenCalledTimes(1)); + expect(setModal).not.toHaveBeenCalledWith('none'); + }); + + it('does not crash when createProject throws', async () => { + jest.mocked(papi.commands.sendCommand).mockRejectedValueOnce(new Error('network')); + const setModal = jest.fn(); + render(); + + await userEvent.click(screen.getByTestId('create-submit')); + + await waitFor(() => expect(papi.commands.sendCommand).toHaveBeenCalled()); + expect(setModal).not.toHaveBeenCalledWith('none'); + }); + it('calls setModal with none when the create modal closes without a select source', async () => { const setModal = jest.fn(); render(); @@ -494,19 +536,32 @@ describe('ProjectModals', () => { }); it('confirms before starting a new draft when the draft is dirty', async () => { - const resetDraft = jest.fn(); - render(); + jest.mocked(papi.commands.sendCommand).mockResolvedValueOnce(JSON.stringify(MOCK_PROJECT)); + render(); await userEvent.click(screen.getByTestId('create-submit')); expect(screen.getByTestId('discard-modal')).toBeInTheDocument(); - expect(resetDraft).not.toHaveBeenCalled(); + // createProject must not have been called yet. + expect(papi.commands.sendCommand).not.toHaveBeenCalledWith( + 'interlinearizer.createProject', + expect.anything(), + expect.anything(), + expect.anything(), + expect.anything(), + expect.anything(), + ); await userEvent.click(screen.getByTestId('discard-confirm')); - expect(resetDraft).toHaveBeenCalledWith({ - analysisLanguages: ['en'], - name: 'New', - description: 'Desc', - }); + await waitFor(() => + expect(papi.commands.sendCommand).toHaveBeenCalledWith( + 'interlinearizer.createProject', + 'source-proj', + ['en'], + undefined, + 'New', + 'Desc', + ), + ); }); }); diff --git a/src/__tests__/hooks/useDraftProject.test.ts b/src/__tests__/hooks/useDraftProject.test.ts index bbd615d0..eb010cd0 100644 --- a/src/__tests__/hooks/useDraftProject.test.ts +++ b/src/__tests__/hooks/useDraftProject.test.ts @@ -150,10 +150,16 @@ describe('useDraftProject', () => { it('stores the edited analysis, marks the draft dirty, and persists with dirty:true', async () => { const { result } = await renderLoaded(); + jest.useFakeTimers(); const edited = analysisWithToken('tok-autosave'); act(() => { result.current.autosaveAnalysis(edited); }); + // Advance past the debounce window so the scheduled persist fires. + act(() => { + jest.advanceTimersByTime(300); + }); + jest.useRealTimers(); expect(result.current.dirty).toBe(true); expect(result.current.draft?.analysis.tokenAnalyses[0].id).toBe('tok-autosave'); @@ -165,12 +171,18 @@ describe('useDraftProject', () => { it('does not error or re-render when called again while already dirty', async () => { const { result } = await renderLoaded(); + jest.useFakeTimers(); act(() => { result.current.autosaveAnalysis(analysisWithToken('first')); }); act(() => { result.current.autosaveAnalysis(analysisWithToken('second')); }); + // The second call replaces the first timer; advance past the debounce to flush the write. + act(() => { + jest.advanceTimersByTime(300); + }); + jest.useRealTimers(); expect(result.current.dirty).toBe(true); // The second autosave does not bump draftVersion and dirty was already true, so it does not @@ -217,6 +229,31 @@ describe('useDraftProject', () => { expect(result.current.draft?.suggestedDescription).toBe('A description'); }); + it('cancels a pending autosave timer so the stale dirty write does not follow the reset', async () => { + const { result } = await renderLoaded(); + + jest.useFakeTimers(); + // Schedule a debounced autosave that would persist {dirty:true}. + act(() => { + result.current.autosaveAnalysis(analysisWithToken('tok-pending')); + }); + // A wholesale reset fires before the 300ms window closes. + act(() => { + result.current.resetDraft({ analysisLanguages: ['es'] }); + }); + // Advance past the original debounce window — the stale timer must not fire. + act(() => { + jest.advanceTimersByTime(300); + }); + jest.useRealTimers(); + + // The last persist is the reset draft; the cancelled autosave never wrote {dirty:true}. + const saved = lastSavedDraft(); + expect(saved.dirty).toBe(false); + expect(saved.analysisLanguages).toEqual(['es']); + expect(saved.analysis.tokenAnalyses).toHaveLength(0); + }); + it('seeds the platform language when the New config supplies no analysis languages', async () => { const { result } = await renderLoaded(); diff --git a/src/__tests__/services/projectStorage.test.ts b/src/__tests__/services/projectStorage.test.ts index becbebf5..713a611c 100644 --- a/src/__tests__/services/projectStorage.test.ts +++ b/src/__tests__/services/projectStorage.test.ts @@ -599,6 +599,17 @@ describe('projectStorage', () => { await expect(getDraft(token, 'src-proj')).rejects.toThrow(SyntaxError); }); + + it('returns an empty draft and warns when the stored value does not match DraftProject shape', async () => { + __mockReadUserData.mockResolvedValue(JSON.stringify({ not: 'a draft' })); + + const result = await getDraft(token, 'src-proj'); + + expect(result).toEqual(emptyDraft('src-proj')); + expect(__mockLogger.warn).toHaveBeenCalledWith( + 'Interlinearizer: stored draft failed validation; resetting to empty draft', + ); + }); }); describe('saveDraft', () => { diff --git a/src/components/InterlinearizerLoader.tsx b/src/components/InterlinearizerLoader.tsx index 6ef709de..f8e3d945 100644 --- a/src/components/InterlinearizerLoader.tsx +++ b/src/components/InterlinearizerLoader.tsx @@ -266,10 +266,11 @@ function InterlinearizerLoaderInner({ setModal('saveAs'); return; } + /* v8 ignore next -- re-entry guard; handles simultaneous saves during async round-trip */ if (isSavingRef.current) return; isSavingRef.current = true; const snapshot = getDraftSnapshot(); - /* v8 ignore next -- save is only reachable once the editor (and draft) have loaded */ + /* v8 ignore next 4 -- save is only reachable once the editor (and draft) have loaded */ if (!snapshot) { isSavingRef.current = false; return; diff --git a/src/components/modals/ProjectModals.tsx b/src/components/modals/ProjectModals.tsx index 41106cb4..fc8ec64f 100644 --- a/src/components/modals/ProjectModals.tsx +++ b/src/components/modals/ProjectModals.tsx @@ -1,7 +1,7 @@ import type { UseWebViewStateHook } from '@papi/core'; import papi, { logger } from '@papi/frontend'; import type { DraftProject, TextAnalysis } from 'interlinearizer'; -import { useCallback, useState } from 'react'; +import { useCallback, useRef, useState } from 'react'; import type { OpenableProject } from '../../hooks/useDraftProject'; import { emptyAnalysis } from '../../types/empty-factories'; import type { InterlinearProjectSummary } from '../../types/interlinear-project-summary'; @@ -20,7 +20,7 @@ export type ModalState = 'none' | 'select' | 'create' | 'metadata' | 'saveAs'; * empty draft or opening an existing project into the draft. */ type PendingReplace = - | { kind: 'new'; config: NewDraftConfig } + | { kind: 'new'; config: CreateDraftConfig } | { kind: 'open'; project: InterlinearProjectSummary }; /** @@ -46,8 +46,6 @@ type PendingReplace = * given the analysis that was persisted; a no-op if an edit landed during the save. * @param props.modal - Which modal is currently open. * @param props.projectId - PAPI source project ID passed from the host. - * @param props.resetDraft - Resets the draft to an empty baseline with the given config (the "New" - * flow). * @param props.setModal - Setter for which modal is open. * @param props.useWebViewState - Hook for reading and writing values persisted in the WebView's * saved state (survives tab restores). @@ -107,6 +105,9 @@ export default function ProjectModals({ /** A draft-replacing action awaiting confirmation because the draft has unsaved changes. */ const [pendingReplace, setPendingReplace] = useState(undefined); + /** Guards against submitting the New dialog twice while the first creation is in flight. */ + const isCreatingRef = useRef(false); + const resolvedMetadataProject = metadataProject ?? activeProject; /** @@ -254,14 +255,23 @@ export default function ProjectModals({ /** * Called when the New dialog is submitted. Starts the new draft immediately, or defers behind the - * unsaved-changes confirmation when the draft is dirty. + * unsaved-changes confirmation when the draft is dirty. A re-entry guard prevents a second + * submission while the first creation round-trip is in flight. * * @param config - The configuration collected by the New dialog. */ const handleCreateDraft = useCallback( (config: CreateDraftConfig) => { - if (dirty) setPendingReplace({ kind: 'new', config }); - else startNewDraft(config); + if (dirty) { + setPendingReplace({ kind: 'new', config }); + return; + } + /* v8 ignore next -- re-entry guard; handles simultaneous submits during async creation */ + if (isCreatingRef.current) return; + isCreatingRef.current = true; + startNewDraft(config).finally(() => { + isCreatingRef.current = false; + }); }, [dirty, startNewDraft], ); From 150a1e2f2c48b0fc2f07787461685059b8aea115 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Thu, 18 Jun 2026 13:39:59 -0400 Subject: [PATCH 05/11] Tidy --- .gitignore | 3 +++ cspell.json | 1 + jest.config.ts | 7 +++---- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/.gitignore b/.gitignore index 76900ddd..a1dbba9e 100644 --- a/.gitignore +++ b/.gitignore @@ -33,6 +33,9 @@ temp-build # #endregion +# AI files +.claude + # Playwright test output e2e-tests/playwright-report e2e-tests/test-results diff --git a/cspell.json b/cspell.json index d754598f..3046472d 100644 --- a/cspell.json +++ b/cspell.json @@ -77,6 +77,7 @@ "verseref", "versification", "wordform", + "worktrees", "ZWNJ" ], "ignoreWords": ["Ελληνικά", "homme", "ʼelohim", "ʻokina"], diff --git a/jest.config.ts b/jest.config.ts index dd0e93b3..8ac28296 100644 --- a/jest.config.ts +++ b/jest.config.ts @@ -105,11 +105,10 @@ const config: Config = { }, /** - * Exclude dist and any git worktrees nested under .claude/ from module resolution. Without the - * .claude exclusion, running Jest from the repo root finds duplicate __mocks__ inside worktrees - * and uses non-deterministically whichever one jest-haste-map encounters first. + * Exclude `.claude` from module resolution to avoid nested git worktrees. Exclude `dist` from + * module resolution to avoid Haste naming collision with root package.json. */ - modulePathIgnorePatterns: ['/dist', '/.claude'], + modulePathIgnorePatterns: ['/.claude', '/dist'], /** Load @testing-library/jest-dom matchers and browser API stubs for React component tests. */ setupFilesAfterEnv: [ From 714a9881e4d809cf81845bd296fcc2b74563a795 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Thu, 18 Jun 2026 13:51:16 -0400 Subject: [PATCH 06/11] Fix debounce data loss on unmount and duplicate-create on DiscardDraftConfirm - Flush the pending debounced autosave in the useEffect cleanup so the last edit before unmount or source-project change is not silently dropped. - Add isCreatingRef guard to the 'new' path of handleConfirmReplace, matching the guard already present in handleCreateDraft, so rapid clicks on the DiscardDraftConfirm button cannot fire multiple createProject commands. Co-Authored-By: Claude Sonnet 4.6 --- src/components/modals/ProjectModals.tsx | 26 ++++++++++++++++++++++--- src/hooks/useDraftProject.ts | 8 ++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/components/modals/ProjectModals.tsx b/src/components/modals/ProjectModals.tsx index fc8ec64f..a6c16b68 100644 --- a/src/components/modals/ProjectModals.tsx +++ b/src/components/modals/ProjectModals.tsx @@ -175,6 +175,7 @@ export default function ProjectModals({ .catch(() => {}); return; } + loadFromProject({ analysisLanguages: parsed.analysisLanguages, ...(parsed.targetProjectId !== undefined && { targetProjectId: parsed.targetProjectId }), @@ -194,7 +195,8 @@ export default function ProjectModals({ /** * Creates a new interlinear project with the given config, loads it into the draft as the active - * working copy, and dismisses the modal. Logs and notifies on failure, leaving the draft untouched. + * working copy, and dismisses the modal. Logs and notifies on failure, leaving the draft + * untouched. * * @param config - The configuration collected by the New dialog. * @returns A promise that resolves once the project is created and loaded, or the failure is @@ -221,6 +223,7 @@ export default function ProjectModals({ .catch(() => {}); return; } + loadFromProject({ analysisLanguages: created.analysisLanguages, ...(created.targetProjectId !== undefined && { @@ -266,8 +269,10 @@ export default function ProjectModals({ setPendingReplace({ kind: 'new', config }); return; } + /* v8 ignore next -- re-entry guard; handles simultaneous submits during async creation */ if (isCreatingRef.current) return; + isCreatingRef.current = true; startNewDraft(config).finally(() => { isCreatingRef.current = false; @@ -280,8 +285,20 @@ export default function ProjectModals({ const handleConfirmReplace = useCallback(async () => { /* v8 ignore next -- the confirm only renders while a pending action exists */ if (!pendingReplace) return; - if (pendingReplace.kind === 'open') await openProject(pendingReplace.project); - else await startNewDraft(pendingReplace.config); + + if (pendingReplace.kind === 'open') { + await openProject(pendingReplace.project); + } else { + /* v8 ignore next -- re-entry guard; handles simultaneous submits during async creation */ + if (isCreatingRef.current) return; + + isCreatingRef.current = true; + try { + await startNewDraft(pendingReplace.config); + } finally { + isCreatingRef.current = false; + } + } setPendingReplace(undefined); }, [pendingReplace, openProject, startNewDraft]); @@ -302,6 +319,7 @@ export default function ProjectModals({ const snapshot = getDraftSnapshot(); /* v8 ignore next -- the Save As modal is only open once the draft has loaded */ if (!snapshot) return; + try { const createdJson = await papi.commands.sendCommand( 'interlinearizer.createProject', @@ -318,6 +336,7 @@ export default function ProjectModals({ .catch(() => {}); return; } + await papi.commands.sendCommand( 'interlinearizer.saveAnalysis', created.id, @@ -348,6 +367,7 @@ export default function ProjectModals({ const snapshot = getDraftSnapshot(); /* v8 ignore next -- the Save As modal is only open once the draft has loaded */ if (!snapshot) return; + try { await papi.commands.sendCommand( 'interlinearizer.saveAnalysis', diff --git a/src/hooks/useDraftProject.ts b/src/hooks/useDraftProject.ts index ad62aa44..47c3e001 100644 --- a/src/hooks/useDraftProject.ts +++ b/src/hooks/useDraftProject.ts @@ -164,6 +164,7 @@ export default function useDraftProject( draft = emptyDraft(sourceProjectId); } if (canceled) return; + // Seed a gloss language in memory when the stored draft has none (a brand-new source). Not // persisted here — the first auto-save / New / Open carries it to storage. if (draft.analysisLanguages.length === 0) @@ -179,6 +180,8 @@ export default function useDraftProject( if (autosaveTimeoutRef.current !== undefined) { clearTimeout(autosaveTimeoutRef.current); autosaveTimeoutRef.current = undefined; + // Flush the pending write so the last edit before unmount/source-change is not lost. + if (draftRef.current) persist(draftRef.current); } }; }, [sourceProjectId]); @@ -212,6 +215,7 @@ export default function useDraftProject( const { current } = draftRef; /* v8 ignore next -- auto-save only fires from the mounted editor, which exists only post-load */ if (!current) return; + const next: DraftProject = { ...current, analysis, dirty: true }; draftRef.current = next; // Debounce writes so rapid keystrokes don't queue unbounded commands to the backend. @@ -263,6 +267,7 @@ export default function useDraftProject( const { current } = draftRef; /* v8 ignore next -- wipe is only reachable from the mounted editor */ if (!current) return; + applyReplacement({ ...current, analysis: removeBookFromAnalysis(current.analysis, bookCode), @@ -276,6 +281,7 @@ export default function useDraftProject( const { current } = draftRef; /* v8 ignore next -- wipe is only reachable from the mounted editor */ if (!current) return; + // Wiping the whole draft is treated as a clean baseline rather than an unsaved edit: it clears // the unsaved-changes indicator (dirty: false) so the user is not nagged to save an empty // draft. The active project is intentionally left untouched, so a subsequent Save still targets @@ -288,11 +294,13 @@ export default function useDraftProject( const { current } = draftRef; /* v8 ignore next -- save is only reachable from the mounted editor */ if (!current) return; + // If an edit landed during the save round-trip, autosaveAnalysis has already swapped a newer // analysis (a fresh object) into the ref and marked the draft dirty. Leave it dirty so the // unsaved indicator and the next Save reflect that un-persisted edit, rather than clearing it // against the now-stale snapshot we just wrote. if (current.analysis !== savedAnalysis) return; + // Cancel any pending debounced autosave before persisting the clean state so a stale // {dirty: true} timer cannot fire after this and overwrite the {dirty: false} record. if (autosaveTimeoutRef.current !== undefined) { From 8d6d2b3c4be4f2cfe309c934551481137b856e96 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Thu, 18 Jun 2026 14:27:14 -0400 Subject: [PATCH 07/11] Remove resetDraft, dead NewDraftConfig type, and related tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resetDraft is no longer called by any production code — the New flow now goes through startNewDraft → loadFromProject in ProjectModals. Remove the hook method, its exported NewDraftConfig type, its unit tests, and the stale prop reference in the InterlinearizerLoader mock. Also make handleCreateDraft async so it awaits the isCreatingRef reset, and tighten two inline comments in useDraftProject to reflect that the cleanup now flushes rather than silently drops a pending autosave. Co-Authored-By: Claude Sonnet 4.6 --- .../components/InterlinearizerLoader.test.tsx | 3 +- src/__tests__/hooks/useDraftProject.test.ts | 74 ------------------- src/components/modals/ProjectModals.tsx | 4 +- src/hooks/useDraftProject.ts | 47 ++---------- 4 files changed, 8 insertions(+), 120 deletions(-) diff --git a/src/__tests__/components/InterlinearizerLoader.test.tsx b/src/__tests__/components/InterlinearizerLoader.test.tsx index 77c63228..55105cc5 100644 --- a/src/__tests__/components/InterlinearizerLoader.test.tsx +++ b/src/__tests__/components/InterlinearizerLoader.test.tsx @@ -188,7 +188,7 @@ jest.mock('../../components/modals/ProjectModals', () => ({ * Minimal ProjectModals stand-in that drives modal state and active-project state through the * same `useWebViewState` hook the real component uses, so tests can assert on state transitions * without mounting the full modal tree. Accepts (and ignores) the draft-related props the loader - * now passes (`dirty`, `getDraftSnapshot`, `loadFromProject`, `markSynced`, `resetDraft`). + * now passes (`dirty`, `getDraftSnapshot`, `loadFromProject`, `markSynced`). * * @param modal - Current modal identifier controlling which stub panel is rendered. * @param setModal - Callback to transition to a different modal state. @@ -214,7 +214,6 @@ jest.mock('../../components/modals/ProjectModals', () => ({ getDraftSnapshot: () => DraftProject | undefined; loadFromProject: (project: unknown) => void; markSynced: () => void; - resetDraft: (config: unknown) => void; useWebViewState: ( key: string, def: MockProject | undefined, diff --git a/src/__tests__/hooks/useDraftProject.test.ts b/src/__tests__/hooks/useDraftProject.test.ts index eb010cd0..4e810e44 100644 --- a/src/__tests__/hooks/useDraftProject.test.ts +++ b/src/__tests__/hooks/useDraftProject.test.ts @@ -194,80 +194,6 @@ describe('useDraftProject', () => { }); }); - describe('resetDraft', () => { - it('resets to an empty analysis, clears dirty, bumps the version, and persists', async () => { - const { result } = await renderLoaded(); - const versionBefore = result.current.draftVersion; - - act(() => { - result.current.resetDraft({ analysisLanguages: ['es'] }); - }); - - expect(result.current.draft?.analysis.tokenAnalyses).toEqual([]); - expect(result.current.draft?.analysisLanguages).toEqual(['es']); - expect(result.current.dirty).toBe(false); - expect(result.current.draftVersion).toBe(versionBefore + 1); - const saved = lastSavedDraft(); - expect(saved.dirty).toBe(false); - expect(saved.analysisLanguages).toEqual(['es']); - }); - - it('carries name, description, and target project id onto the reset draft', async () => { - const { result } = await renderLoaded(); - - act(() => { - result.current.resetDraft({ - analysisLanguages: ['es'], - targetProjectId: 'target-1', - name: 'My Draft', - description: 'A description', - }); - }); - - expect(result.current.draft?.targetProjectId).toBe('target-1'); - expect(result.current.draft?.suggestedName).toBe('My Draft'); - expect(result.current.draft?.suggestedDescription).toBe('A description'); - }); - - it('cancels a pending autosave timer so the stale dirty write does not follow the reset', async () => { - const { result } = await renderLoaded(); - - jest.useFakeTimers(); - // Schedule a debounced autosave that would persist {dirty:true}. - act(() => { - result.current.autosaveAnalysis(analysisWithToken('tok-pending')); - }); - // A wholesale reset fires before the 300ms window closes. - act(() => { - result.current.resetDraft({ analysisLanguages: ['es'] }); - }); - // Advance past the original debounce window — the stale timer must not fire. - act(() => { - jest.advanceTimersByTime(300); - }); - jest.useRealTimers(); - - // The last persist is the reset draft; the cancelled autosave never wrote {dirty:true}. - const saved = lastSavedDraft(); - expect(saved.dirty).toBe(false); - expect(saved.analysisLanguages).toEqual(['es']); - expect(saved.analysis.tokenAnalyses).toHaveLength(0); - }); - - it('seeds the platform language when the New config supplies no analysis languages', async () => { - const { result } = await renderLoaded(); - - act(() => { - result.current.resetDraft({ analysisLanguages: [] }); - }); - - expect(result.current.draft?.analysisLanguages).toEqual([PLATFORM_LANGUAGE]); - expect(result.current.draft?.targetProjectId).toBeUndefined(); - expect(result.current.draft?.suggestedName).toBeUndefined(); - expect(result.current.draft?.suggestedDescription).toBeUndefined(); - }); - }); - describe('loadFromProject', () => { it('copies analysis, analysis languages, and target, clears dirty, and bumps the version', async () => { const { result } = await renderLoaded(); diff --git a/src/components/modals/ProjectModals.tsx b/src/components/modals/ProjectModals.tsx index a6c16b68..ec58feae 100644 --- a/src/components/modals/ProjectModals.tsx +++ b/src/components/modals/ProjectModals.tsx @@ -264,7 +264,7 @@ export default function ProjectModals({ * @param config - The configuration collected by the New dialog. */ const handleCreateDraft = useCallback( - (config: CreateDraftConfig) => { + async (config: CreateDraftConfig) => { if (dirty) { setPendingReplace({ kind: 'new', config }); return; @@ -274,7 +274,7 @@ export default function ProjectModals({ if (isCreatingRef.current) return; isCreatingRef.current = true; - startNewDraft(config).finally(() => { + await startNewDraft(config).finally(() => { isCreatingRef.current = false; }); }, diff --git a/src/hooks/useDraftProject.ts b/src/hooks/useDraftProject.ts index 47c3e001..4cc834ff 100644 --- a/src/hooks/useDraftProject.ts +++ b/src/hooks/useDraftProject.ts @@ -8,18 +8,6 @@ import { removeBookFromAnalysis } from '../utils/analysis-book'; /** Milliseconds to wait after the last keystroke before flushing an autosave write. */ const AUTOSAVE_DEBOUNCE_MS = 300; -/** Configuration captured by the "New" flow when resetting the draft to an empty baseline. */ -export type NewDraftConfig = { - /** BCP 47 gloss / annotation language tags for the new draft. */ - analysisLanguages: string[]; - /** Optional alignment target-text project ID. */ - targetProjectId?: string; - /** Optional name typed in the New dialog, retained on the draft to prefill Save As. */ - name?: string; - /** Optional description typed in the New dialog, retained on the draft to prefill Save As. */ - description?: string; -}; - /** The subset of an {@link InterlinearProject} needed to open it into the draft as a working copy. */ export type OpenableProject = Pick< InterlinearProject, @@ -61,12 +49,6 @@ export type UseDraftProjectResult = { * @param analysis - The updated analysis from the store. */ autosaveAnalysis: (analysis: TextAnalysis) => void; - /** - * Resets the draft to an empty analysis with the given config — the "New" flow. - * - * @param config - Gloss languages and optional alignment target for the fresh draft. - */ - resetDraft: (config: NewDraftConfig) => void; /** * Replaces the draft with a working copy of an existing project's analysis and config — the * "Open" flow. @@ -120,13 +102,14 @@ export default function useDraftProject( const [draftVersion, setDraftVersion] = useState(0); const [dirty, setDirty] = useState(false); - // Read the latest platform language via a ref so the load effect (keyed only on sourceProjectId) + // Read the latest platform language via a ref so the load effect (keyed on sourceProjectId) // does not re-run when the UI language changes after the draft has loaded. const platformLanguageRef = useRef(platformLanguage); platformLanguageRef.current = platformLanguage; - // Pending debounced-autosave timer. Cancelled on source change and on any wholesale replacement - // (applyReplacement) so stale keystroke data is never written after a New / Open / Wipe. + // Pending debounced-autosave timer. Flushed on unmount/source change (so the last edit is not + // lost), and cancelled on any wholesale replacement (applyReplacement) so stale keystroke data + // is never written after a New / Open / Wipe. const autosaveTimeoutRef = useRef | undefined>(undefined); /** @@ -184,7 +167,7 @@ export default function useDraftProject( if (draftRef.current) persist(draftRef.current); } }; - }, [sourceProjectId]); + }, [persist, sourceProjectId]); const getDraftSnapshot = useCallback(() => draftRef.current, []); @@ -230,25 +213,6 @@ export default function useDraftProject( [persist], ); - const resetDraft = useCallback( - (config: NewDraftConfig) => { - const analysisLanguages = - config.analysisLanguages.length > 0 - ? config.analysisLanguages - : [platformLanguageRef.current]; - applyReplacement({ - sourceProjectId, - analysisLanguages, - ...(config.targetProjectId !== undefined && { targetProjectId: config.targetProjectId }), - ...(config.name !== undefined && { suggestedName: config.name }), - ...(config.description !== undefined && { suggestedDescription: config.description }), - analysis: emptyAnalysis(), - dirty: false, - }); - }, - [applyReplacement, sourceProjectId], - ); - const loadFromProject = useCallback( (project: OpenableProject) => { applyReplacement({ @@ -322,7 +286,6 @@ export default function useDraftProject( dirty, getDraftSnapshot, autosaveAnalysis, - resetDraft, loadFromProject, wipeBook, wipeAll, From d6a06593ac073eea4f501d29dd3066964a6dcf9b Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Thu, 18 Jun 2026 15:19:25 -0400 Subject: [PATCH 08/11] Add loading feedback to Create modal and drop redundant submitting refs - Add `isSubmitting` prop to `CreateProjectModal`; `ProjectModals` threads `isCreating` state into it so both buttons are disabled during the backend round-trip, addressing the reviewer's missing-loading-indicator comment - Drop the `ref + state` dual pattern from `ProjectModals`, `SaveAsProjectModal`, and `ProjectMetadataModal`; state alone is sufficient because the buttons are already disabled before a second invocation can reach the guard - Add a `useDraftProject` test covering the timeout-cancellation branch in `applyReplacement` to restore 100% coverage Co-Authored-By: Claude Sonnet 4.6 --- .../modals/CreateProjectModal.test.tsx | 14 +++++++++ .../components/modals/ProjectModals.test.tsx | 22 ++++++++++++++ src/__tests__/hooks/useDraftProject.test.ts | 29 ++++++++++++++++++ src/components/modals/CreateProjectModal.tsx | 13 ++++---- .../modals/ProjectMetadataModal.tsx | 18 +++++------ src/components/modals/ProjectModals.tsx | 30 +++++++++++-------- src/components/modals/SaveAsProjectModal.tsx | 19 ++++-------- 7 files changed, 103 insertions(+), 42 deletions(-) diff --git a/src/__tests__/components/modals/CreateProjectModal.test.tsx b/src/__tests__/components/modals/CreateProjectModal.test.tsx index b48c343d..23615f88 100644 --- a/src/__tests__/components/modals/CreateProjectModal.test.tsx +++ b/src/__tests__/components/modals/CreateProjectModal.test.tsx @@ -97,6 +97,20 @@ describe('CreateProjectModal', () => { }); }); + it('disables both buttons when isSubmitting is true', () => { + render( {}} onCreateDraft={() => {}} />); + + expect(screen.getByRole('button', { name: /cancel/i })).toBeDisabled(); + expect(screen.getByRole('button', { name: /^create$/i })).toBeDisabled(); + }); + + it('keeps both buttons enabled when isSubmitting is false', () => { + render( {}} onCreateDraft={() => {}} />); + + expect(screen.getByRole('button', { name: /cancel/i })).toBeEnabled(); + expect(screen.getByRole('button', { name: /^create$/i })).toBeEnabled(); + }); + it('defaults the language to ["und"] when the field contains only whitespace', async () => { const onCreateDraft = jest.fn(); render( {}} onCreateDraft={onCreateDraft} />); diff --git a/src/__tests__/components/modals/ProjectModals.test.tsx b/src/__tests__/components/modals/ProjectModals.test.tsx index 8be2e90b..bcf8653e 100644 --- a/src/__tests__/components/modals/ProjectModals.test.tsx +++ b/src/__tests__/components/modals/ProjectModals.test.tsx @@ -93,10 +93,12 @@ jest.mock('../../../components/modals/CreateProjectModal', () => ({ __esModule: true, CreateProjectModal: ({ defaultAnalysisLanguage, + isSubmitting, onClose, onCreateDraft, }: { defaultAnalysisLanguage?: string; + isSubmitting?: boolean; onClose: () => void; onCreateDraft: (config: { analysisLanguages: string[]; @@ -111,6 +113,7 @@ jest.mock('../../../components/modals/CreateProjectModal', () => ({ - diff --git a/src/components/modals/ProjectMetadataModal.tsx b/src/components/modals/ProjectMetadataModal.tsx index d4e8087e..7bc78b76 100644 --- a/src/components/modals/ProjectMetadataModal.tsx +++ b/src/components/modals/ProjectMetadataModal.tsx @@ -2,7 +2,7 @@ import papi, { logger } from '@papi/frontend'; import { useLocalizedStrings } from '@papi/frontend/react'; import { Trash2 } from 'lucide-react'; import { Button } from 'platform-bible-react'; -import { useCallback, useMemo, useRef, useState } from 'react'; +import { useCallback, useMemo, useState } from 'react'; /** Localized string keys used by {@link ProjectMetadataModal}. */ const PROJECT_METADATA_MODAL_STRING_KEYS: `%${string}%`[] = [ @@ -82,7 +82,6 @@ export function ProjectMetadataModal({ const [editLanguages, setEditLanguages] = useState(analysisLanguages.join(', ')); const [confirmingDelete, setConfirmingDelete] = useState(false); const [isSubmitting, setIsSubmitting] = useState(false); - const isSubmittingRef = useRef(false); const formattedDate = useMemo(() => new Date(createdAt).toLocaleString(), [createdAt]); @@ -98,9 +97,8 @@ export function ProjectMetadataModal({ * @returns A promise that resolves when the command completes or the error is logged. */ const handleSave = useCallback(async () => { - /* v8 ignore next -- button is disabled while submitting; ref guards against programmatic races */ - if (isSubmittingRef.current) return; - isSubmittingRef.current = true; + /* v8 ignore next -- button is disabled while submitting; guards against programmatic double-invocation */ + if (isSubmitting) return; setIsSubmitting(true); const newName = editName.trim() || undefined; const newDescription = editDescription.trim() || undefined; @@ -127,10 +125,10 @@ export function ProjectMetadataModal({ } catch (e) { logger.error('Interlinearizer: failed to save project metadata', e); } finally { - isSubmittingRef.current = false; setIsSubmitting(false); } }, [ + isSubmitting, editName, editDescription, editLanguages, @@ -148,9 +146,8 @@ export function ProjectMetadataModal({ * @returns A promise that resolves when the command completes or the error is logged. */ const handleDelete = useCallback(async () => { - /* v8 ignore next -- button is disabled while submitting; ref guards against programmatic races */ - if (isSubmittingRef.current) return; - isSubmittingRef.current = true; + /* v8 ignore next -- button is disabled while submitting; guards against programmatic double-invocation */ + if (isSubmitting) return; setIsSubmitting(true); try { await papi.commands.sendCommand('interlinearizer.deleteProject', interlinearProjectId); @@ -159,10 +156,9 @@ export function ProjectMetadataModal({ } catch (e) { logger.error('Interlinearizer: failed to delete project', e); } finally { - isSubmittingRef.current = false; setIsSubmitting(false); } - }, [interlinearProjectId, onProjectDeleted, onClose]); + }, [isSubmitting, interlinearProjectId, onProjectDeleted, onClose]); /* v8 ignore next */ if (stringsLoading) return undefined; diff --git a/src/components/modals/ProjectModals.tsx b/src/components/modals/ProjectModals.tsx index ec58feae..1246f33f 100644 --- a/src/components/modals/ProjectModals.tsx +++ b/src/components/modals/ProjectModals.tsx @@ -1,7 +1,7 @@ import type { UseWebViewStateHook } from '@papi/core'; import papi, { logger } from '@papi/frontend'; import type { DraftProject, TextAnalysis } from 'interlinearizer'; -import { useCallback, useRef, useState } from 'react'; +import { useCallback, useState } from 'react'; import type { OpenableProject } from '../../hooks/useDraftProject'; import { emptyAnalysis } from '../../types/empty-factories'; import type { InterlinearProjectSummary } from '../../types/interlinear-project-summary'; @@ -105,8 +105,11 @@ export default function ProjectModals({ /** A draft-replacing action awaiting confirmation because the draft has unsaved changes. */ const [pendingReplace, setPendingReplace] = useState(undefined); - /** Guards against submitting the New dialog twice while the first creation is in flight. */ - const isCreatingRef = useRef(false); + /** + * Whether a backend project-creation round-trip is in progress; drives `isSubmitting` on the + * Create modal. + */ + const [isCreating, setIsCreating] = useState(false); const resolvedMetadataProject = metadataProject ?? activeProject; @@ -270,15 +273,15 @@ export default function ProjectModals({ return; } - /* v8 ignore next -- re-entry guard; handles simultaneous submits during async creation */ - if (isCreatingRef.current) return; + /* v8 ignore next -- button is disabled while creating; guards against programmatic double-invocation */ + if (isCreating) return; - isCreatingRef.current = true; + setIsCreating(true); await startNewDraft(config).finally(() => { - isCreatingRef.current = false; + setIsCreating(false); }); }, - [dirty, startNewDraft], + [dirty, isCreating, startNewDraft], ); /** Confirms the deferred draft-replacing action after the user accepts losing unsaved changes. */ @@ -289,18 +292,18 @@ export default function ProjectModals({ if (pendingReplace.kind === 'open') { await openProject(pendingReplace.project); } else { - /* v8 ignore next -- re-entry guard; handles simultaneous submits during async creation */ - if (isCreatingRef.current) return; + /* v8 ignore next -- button is disabled while creating; guards against programmatic double-invocation */ + if (isCreating) return; - isCreatingRef.current = true; + setIsCreating(true); try { await startNewDraft(pendingReplace.config); } finally { - isCreatingRef.current = false; + setIsCreating(false); } } setPendingReplace(undefined); - }, [pendingReplace, openProject, startNewDraft]); + }, [isCreating, pendingReplace, openProject, startNewDraft]); /** Cancels the deferred action, returning to the underlying modal with the draft untouched. */ const handleCancelReplace = useCallback(() => setPendingReplace(undefined), []); @@ -454,6 +457,7 @@ export default function ProjectModals({ {modal === 'create' && ( diff --git a/src/components/modals/SaveAsProjectModal.tsx b/src/components/modals/SaveAsProjectModal.tsx index 98ecad36..9d9f05a4 100644 --- a/src/components/modals/SaveAsProjectModal.tsx +++ b/src/components/modals/SaveAsProjectModal.tsx @@ -70,9 +70,6 @@ export function SaveAsProjectModal({ * re-submits. */ const [isSubmitting, setIsSubmitting] = useState(false); - // Ref mirror of `isSubmitting` so a submit handler can short-circuit a second invocation - // synchronously, before the re-render that disables the button lands (guards programmatic races). - const isSubmittingRef = useRef(false); /** The existing project pending an overwrite confirmation, or `undefined`. */ const [confirmOverwrite, setConfirmOverwrite] = useState( @@ -128,17 +125,15 @@ export function SaveAsProjectModal({ * projects. */ const handleSaveNew = useCallback(async () => { - /* v8 ignore next -- button is disabled while submitting; ref guards against programmatic races */ - if (isSubmittingRef.current) return; - isSubmittingRef.current = true; + /* v8 ignore next -- button is disabled while submitting; guards against programmatic double-invocation */ + if (isSubmitting) return; setIsSubmitting(true); try { await onSaveNew(name.trim() || undefined, description.trim() || undefined); } finally { - isSubmittingRef.current = false; setIsSubmitting(false); } - }, [name, description, onSaveNew]); + }, [isSubmitting, name, description, onSaveNew]); /** * Overwrites the chosen existing project with the draft, blocking re-entry while the save is in @@ -148,18 +143,16 @@ export function SaveAsProjectModal({ */ const handleConfirmOverwrite = useCallback( async (project: InterlinearProjectSummary) => { - /* v8 ignore next -- button is disabled while submitting; ref guards against programmatic races */ - if (isSubmittingRef.current) return; - isSubmittingRef.current = true; + /* v8 ignore next -- button is disabled while submitting; guards against programmatic double-invocation */ + if (isSubmitting) return; setIsSubmitting(true); try { await onOverwrite(project); } finally { - isSubmittingRef.current = false; setIsSubmitting(false); } }, - [onOverwrite], + [isSubmitting, onOverwrite], ); /* v8 ignore next */ if (stringsLoading) return undefined; From 670bd8d41277a2be8d2a0f3f4005063a7ad4635c Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Thu, 18 Jun 2026 15:42:03 -0400 Subject: [PATCH 09/11] Disable DiscardDraftConfirm buttons during async project creation The confirm overlay remained interactive during the backend round-trip when the 'new' branch of handleConfirmReplace was awaiting startNewDraft. Both buttons now receive isSubmitting so the user gets clear visual feedback and cannot interact with the dialog while creation is in flight. Co-Authored-By: Claude Sonnet 4.6 --- .../modals/DiscardDraftConfirm.test.tsx | 14 ++++++++ .../components/modals/ProjectModals.test.tsx | 32 +++++++++++++++++-- src/components/modals/DiscardDraftConfirm.tsx | 17 +++++++--- src/components/modals/ProjectModals.tsx | 6 +++- 4 files changed, 61 insertions(+), 8 deletions(-) diff --git a/src/__tests__/components/modals/DiscardDraftConfirm.test.tsx b/src/__tests__/components/modals/DiscardDraftConfirm.test.tsx index 1b1a986b..b9889a5e 100644 --- a/src/__tests__/components/modals/DiscardDraftConfirm.test.tsx +++ b/src/__tests__/components/modals/DiscardDraftConfirm.test.tsx @@ -40,6 +40,20 @@ describe('DiscardDraftConfirm', () => { expect(onConfirm).toHaveBeenCalledTimes(1); }); + it('disables both buttons when isSubmitting is true', () => { + render(); + + expect(screen.getByTestId('discard-draft-confirm')).toBeDisabled(); + expect(screen.getByRole('button', { name: /cancel/i })).toBeDisabled(); + }); + + it('keeps both buttons enabled when isSubmitting is false', () => { + render(); + + expect(screen.getByTestId('discard-draft-confirm')).toBeEnabled(); + expect(screen.getByRole('button', { name: /cancel/i })).toBeEnabled(); + }); + it('calls onCancel when the Cancel button is clicked', async () => { const onCancel = jest.fn(); render(); diff --git a/src/__tests__/components/modals/ProjectModals.test.tsx b/src/__tests__/components/modals/ProjectModals.test.tsx index bcf8653e..1c98c663 100644 --- a/src/__tests__/components/modals/ProjectModals.test.tsx +++ b/src/__tests__/components/modals/ProjectModals.test.tsx @@ -162,14 +162,21 @@ jest.mock('../../../components/modals/SaveAsProjectModal', () => ({ jest.mock('../../../components/modals/DiscardDraftConfirm', () => ({ __esModule: true, DiscardDraftConfirm: ({ - onConfirm, + isSubmitting, onCancel, + onConfirm, }: { - onConfirm: () => void; + isSubmitting?: boolean; onCancel: () => void; + onConfirm: () => void; }) => (
- -
diff --git a/src/components/modals/ProjectModals.tsx b/src/components/modals/ProjectModals.tsx index 1246f33f..8b50cdb1 100644 --- a/src/components/modals/ProjectModals.tsx +++ b/src/components/modals/ProjectModals.tsx @@ -493,7 +493,11 @@ export default function ProjectModals({ returns to that modal with its in-progress input intact, and confirming an Open does not unmount (and re-fetch) the still-open select modal underneath. */} {pendingReplace && ( - + )} ); From 329e5edc3430b7785a6cf0f6bc36d5d0e07d2768 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Thu, 18 Jun 2026 16:21:07 -0400 Subject: [PATCH 10/11] Tweak --- src/__tests__/components/modals/ProjectModals.test.tsx | 4 ++-- src/components/modals/ProjectMetadataModal.tsx | 2 +- src/components/modals/ProjectModals.tsx | 6 ++++++ src/components/modals/SaveAsProjectModal.tsx | 2 +- src/services/projectStorage.ts | 2 +- 5 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/__tests__/components/modals/ProjectModals.test.tsx b/src/__tests__/components/modals/ProjectModals.test.tsx index 1c98c663..b6bcd925 100644 --- a/src/__tests__/components/modals/ProjectModals.test.tsx +++ b/src/__tests__/components/modals/ProjectModals.test.tsx @@ -107,7 +107,7 @@ jest.mock('../../../components/modals/CreateProjectModal', () => ({ }) => void; }) => (
- -
diff --git a/src/components/modals/ProjectMetadataModal.tsx b/src/components/modals/ProjectMetadataModal.tsx index 7bc78b76..dc22ce38 100644 --- a/src/components/modals/ProjectMetadataModal.tsx +++ b/src/components/modals/ProjectMetadataModal.tsx @@ -158,7 +158,7 @@ export function ProjectMetadataModal({ } finally { setIsSubmitting(false); } - }, [isSubmitting, interlinearProjectId, onProjectDeleted, onClose]); + }, [interlinearProjectId, isSubmitting, onClose, onProjectDeleted]); /* v8 ignore next */ if (stringsLoading) return undefined; diff --git a/src/components/modals/ProjectModals.tsx b/src/components/modals/ProjectModals.tsx index 8b50cdb1..243b7812 100644 --- a/src/components/modals/ProjectModals.tsx +++ b/src/components/modals/ProjectModals.tsx @@ -237,6 +237,12 @@ export default function ProjectModals({ setActiveProject(created); } catch (e) { logger.error('Interlinearizer: failed to create interlinear project', e); + await papi.notifications + .send({ + message: '%interlinearizer_error_create_project_failed%', + severity: 'error', + }) + .catch(() => {}); return; } setCreateSourceIsSelect(false); diff --git a/src/components/modals/SaveAsProjectModal.tsx b/src/components/modals/SaveAsProjectModal.tsx index 9d9f05a4..deb5b9cf 100644 --- a/src/components/modals/SaveAsProjectModal.tsx +++ b/src/components/modals/SaveAsProjectModal.tsx @@ -133,7 +133,7 @@ export function SaveAsProjectModal({ } finally { setIsSubmitting(false); } - }, [isSubmitting, name, description, onSaveNew]); + }, [description, isSubmitting, name, onSaveNew]); /** * Overwrites the chosen existing project with the draft, blocking re-entry while the save is in diff --git a/src/services/projectStorage.ts b/src/services/projectStorage.ts index 2057fb07..42b242a3 100644 --- a/src/services/projectStorage.ts +++ b/src/services/projectStorage.ts @@ -376,7 +376,7 @@ export async function getDraft( const parsed: unknown = JSON.parse( await papi.storage.readUserData(token, draftKey(sourceProjectId)), ); - if (!isDraftProject(parsed)) { + if (!isDraftProject(parsed) || parsed.sourceProjectId !== sourceProjectId) { logger.warn('Interlinearizer: stored draft failed validation; resetting to empty draft'); return emptyDraft(sourceProjectId); } From 87f3fbf511321d00dbbbf182f0ae406bc5211348 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Thu, 18 Jun 2026 16:30:32 -0400 Subject: [PATCH 11/11] Fix race: disable DiscardDraftConfirm during both open and new-draft flows isCreating only covered the new-draft branch of handleConfirmReplace, leaving the open-project branch unguarded. A single isReplacing state now wraps both paths, so the discard overlay buttons are disabled for the full duration of the async operation. Co-Authored-By: Claude Sonnet 4.6 --- src/components/modals/ProjectModals.tsx | 28 +++++++++++++++---------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/components/modals/ProjectModals.tsx b/src/components/modals/ProjectModals.tsx index 243b7812..ffcf086f 100644 --- a/src/components/modals/ProjectModals.tsx +++ b/src/components/modals/ProjectModals.tsx @@ -111,6 +111,12 @@ export default function ProjectModals({ */ const [isCreating, setIsCreating] = useState(false); + /** + * Whether the discard-and-replace confirmation flow is in flight (either opening an existing + * project or creating a new draft); disables DiscardDraftConfirm buttons to prevent races. + */ + const [isReplacing, setIsReplacing] = useState(false); + const resolvedMetadataProject = metadataProject ?? activeProject; /** @@ -295,21 +301,21 @@ export default function ProjectModals({ /* v8 ignore next -- the confirm only renders while a pending action exists */ if (!pendingReplace) return; - if (pendingReplace.kind === 'open') { - await openProject(pendingReplace.project); - } else { - /* v8 ignore next -- button is disabled while creating; guards against programmatic double-invocation */ - if (isCreating) return; + /* v8 ignore next -- buttons are disabled while replacing; guards against programmatic double-invocation */ + if (isReplacing) return; - setIsCreating(true); - try { + setIsReplacing(true); + try { + if (pendingReplace.kind === 'open') { + await openProject(pendingReplace.project); + } else { await startNewDraft(pendingReplace.config); - } finally { - setIsCreating(false); } + } finally { + setIsReplacing(false); } setPendingReplace(undefined); - }, [isCreating, pendingReplace, openProject, startNewDraft]); + }, [isReplacing, openProject, pendingReplace, startNewDraft]); /** Cancels the deferred action, returning to the underlying modal with the draft untouched. */ const handleCancelReplace = useCallback(() => setPendingReplace(undefined), []); @@ -500,7 +506,7 @@ export default function ProjectModals({ unmount (and re-fetch) the still-open select modal underneath. */} {pendingReplace && (