Fix review findings on the Paratext 9 import WebView

contributions/localizedStrings.json

@@ -201,7 +201,8 @@
       "%interlinearizer_copyModal_create%": "Create copy",
       "%interlinearizer_copyModal_cancel%": "Cancel",
       "%interlinearizer_warning_pt9Sync_failed%": "The Paratext 9 data couldn't be refreshed; showing the last imported data.",
-      "%interlinearizer_modal_metadata_lastSynced_label%": "Last synced"
+      "%interlinearizer_modal_metadata_lastSynced_label%": "Last synced",
+      "%interlinearizer_error_load_import_analysis%": "Could not load the imported interlinear data. Please try again."
     }
   }
 }

src/__tests__/components/ArcOverlay.test.tsx

@@ -121,6 +121,17 @@ describe('ArcOverlay', () => {
     expect(document.querySelector('path')).toBeInTheDocument();
   });
 
+  it('hides the split button but keeps the arc when the analysis is read-only', () => {
+    const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b']);
+    renderOverlay({
+      arcPaths: [makeArcPath('p1', 'tok-a')],
+      phraseLinkById: new Map([['p1', phraseLink]]),
+      readOnly: true,
+    });
+    expect(screen.queryByTestId('split-arc-btn')).not.toBeInTheDocument();
+    expect(document.querySelector('path')).toBeInTheDocument();
+  });
+
   it('with simplifyPhrases on, keeps the split button for the focused phrase', () => {
     const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b']);
     renderOverlay({

src/__tests__/components/Interlinearizer.test.tsx

@@ -101,9 +101,12 @@ const mockPhraseLinkById = new Map<string, PhraseAnalysisLink>();
 /** Read once per `Interlinearizer` render, so this doubles as a render counter. */
 let phraseLinkByIdMapReads = 0;
 
+/** What the mocked `useAnalysisReadOnly` returns; module state, so each test resets it. */
+const mockReadOnly = { current: false };
+
 jest.mock('../../components/AnalysisStore', () => ({
   __esModule: true,
-  useAnalysisReadOnly: () => false,
+  useAnalysisReadOnly: () => mockReadOnly.current,
   /**
    * Pass-through provider stub that renders children directly, keeping AnalysisStore.tsx out of
    * scope.
@@ -426,6 +429,7 @@ beforeEach(() => {
   capturedSegmentation = undefined;
   // The merge control's label comes from a localized string.
   mockKeyAsValueLocalizedStrings();
+  mockReadOnly.current = false;
 });
 
 describe('Interlinearizer', () => {
@@ -1641,6 +1645,13 @@ describe('between-rows merge control', () => {
     expect(button).toHaveAttribute('title', 'Merge');
   });
 
+  it('renders no merge control while the analysis is read-only', () => {
+    mockReadOnly.current = true;
+    renderInterlinearizer({ book: GEN_1_MULTI_BOOK });
+    expect(screen.queryByTestId('segment-merge-btn')).not.toBeInTheDocument();
+    expect(screen.queryByTestId('segment-merge-indicator')).not.toBeInTheDocument();
+  });
+
   it('renders no merge control while a phrase mode is active', () => {
     // A merge mid-mode could re-segment the phrase the mode UI is operating on, so the between-rows
     // control is omitted entirely (not merely disabled) throughout a phrase edit.

src/__tests__/components/InterlinearizerLoader.test.tsx

@@ -183,6 +183,9 @@ type CapturedStoreProps = {
 /* eslint-enable react/no-unused-prop-types */
 let capturedStoreProps: CapturedStoreProps | undefined;
 
+/** The `initialAnalysis` each provider mount was seeded with, oldest first. */
+const seededAnalyses: (TextAnalysis | undefined)[] = [];
+
 // Spy wrapper around the real provider rather than a replacement for it: the lifetime tests compare
 // store identity across a book change, so the store has to be genuine, while the props the loader
 // passes still need to be observable.
@@ -193,8 +196,10 @@ jest.mock('../../components/AnalysisStore', () => {
   return {
     ...actual,
     /** Records the props, then renders the real provider unchanged. */
-    AnalysisStoreProvider(props: CapturedStoreProps) {
+    AnalysisStoreProvider({ initialAnalysis, ...rest }: CapturedStoreProps) {
+      const props = { initialAnalysis, ...rest };
       capturedStoreProps = props;
+      seededAnalyses.push(initialAnalysis);
       return createElement(actual.AnalysisStoreProvider, props);
     },
   };
@@ -538,6 +543,7 @@ describe('InterlinearizerLoader', () => {
   beforeEach(() => {
     capturedInterlinearizerProps = undefined;
     capturedStoreProps = undefined;
+    seededAnalyses.length = 0;
     interlinearizerMountCount = 0;
     mockBookData();
     mockOptimisticSetting();
@@ -1180,7 +1186,7 @@ describe('InterlinearizerLoader', () => {
       );
     });
 
-    it('returns to the plain view when an offer-run report is closed', async () => {
+    it('offers only Open on an offer-run report', async () => {
       mockOfferProbe();
       mockImportCommands({
         importResult: { outcome: 'imported', projectId: 'import-1', report: IMPORT_REPORT },
@@ -1191,13 +1197,35 @@ describe('InterlinearizerLoader', () => {
       await userEvent.click(
         screen.getByRole('button', { name: '%interlinearizer_pt9ConvertPrompt_yes%' }),
       );
+
       await screen.findByTestId('pt9-import-report');
+      expect(
+        screen.queryByRole('button', { name: '%interlinearizer_pt9ImportModal_close%' }),
+      ).not.toBeInTheDocument();
+    });
 
+    it('opens the conversion when an offer-run report is dismissed', async () => {
+      mockOfferProbe();
+      mockImportCommands({
+        importResult: { outcome: 'imported', projectId: 'import-1', report: IMPORT_REPORT },
+      });
+      await act(async () => {
+        renderLoader({ useWebViewState: makeWebViewState({ offerPt9Import: true }) });
+      });
       await userEvent.click(
-        screen.getByRole('button', { name: '%interlinearizer_pt9ImportModal_close%' }),
+        screen.getByRole('button', { name: '%interlinearizer_pt9ConvertPrompt_yes%' }),
+      );
+      await screen.findByTestId('pt9-import-report');
+
+      await userEvent.click(
+        screen.getByRole('button', { name: '%interlinearizer_pt9ImportModal_open%' }),
       );
 
       expect(screen.getByTestId('project-modals')).toHaveAttribute('data-modal', 'none');
+      expect(screen.getByTestId('project-modals')).toHaveAttribute(
+        'data-active-project-name',
+        'Paratext 9 Interlinear',
+      );
     });
 
     it('persists the empty draft and runs no import on No', async () => {
@@ -1324,6 +1352,54 @@ describe('InterlinearizerLoader', () => {
       );
     });
 
+    it('drops an Open whose summary arrives after the report was dismissed', async () => {
+      let releaseSummary = () => {};
+      const summaryHeld = new Promise<void>((resolve) => {
+        releaseSummary = resolve;
+      });
+      let importDone = false;
+      mockSendCommand.mockImplementation(async (...args) => {
+        if (args[0] === 'interlinearizer.getProject') {
+          // Hold only the summary the report's Open asks for, not the import view's own load.
+          if (importDone) await summaryHeld;
+          return JSON.stringify({ ...FRESH_IMPORT_SUMMARY, analysis: emptyAnalysis() });
+        }
+        if (args[0] === 'interlinearizer.importPt9Project') {
+          importDone = true;
+          return JSON.stringify({
+            outcome: 'imported',
+            projectId: 'import-1',
+            report: IMPORT_REPORT,
+          });
+        }
+        return JSON.stringify(emptyDraft(testProjectId));
+      });
+      await act(async () => {
+        renderLoader();
+      });
+      await userEvent.click(screen.getByTestId('tab-toolbar-project-menu'));
+      await userEvent.click(screen.getByTestId('select-modal-import-pt9'));
+      await screen.findByTestId('pt9-import-report');
+
+      await userEvent.click(
+        screen.getByRole('button', { name: '%interlinearizer_pt9ImportModal_open%' }),
+      );
+      // Back out to the select modal while the summary is still in flight, then let it land.
+      await userEvent.click(
+        screen.getByRole('button', { name: '%interlinearizer_pt9ImportModal_close%' }),
+      );
+      await act(async () => {
+        releaseSummary();
+        await summaryHeld;
+      });
+
+      expect(screen.getByTestId('project-modals')).toHaveAttribute('data-modal', 'select');
+      expect(screen.getByTestId('project-modals')).not.toHaveAttribute(
+        'data-active-project-name',
+        'Paratext 9 Interlinear',
+      );
+    });
+
     it('returns to the select modal when an import report is closed', async () => {
       mockImportCommands({
         importResult: { outcome: 'imported', projectId: 'import-1', report: IMPORT_REPORT },
@@ -1447,6 +1523,38 @@ describe('InterlinearizerLoader', () => {
       );
     });
 
+    it('titles the open-path sync as a sync while it runs', async () => {
+      let releaseImport = () => {};
+      const importHeld = new Promise<void>((resolve) => {
+        releaseImport = resolve;
+      });
+      mockSendCommand.mockImplementation(async (...args) => {
+        if (args[0] === 'interlinearizer.getProject')
+          return JSON.stringify({ ...FRESH_IMPORT_SUMMARY, analysis: emptyAnalysis() });
+        if (args[0] === 'interlinearizer.importPt9Project') {
+          await importHeld;
+          return JSON.stringify({ outcome: 'imported', projectId: 'import-1' });
+        }
+        return JSON.stringify(emptyDraft(testProjectId));
+      });
+      mockPdpGet.mockResolvedValue({
+        getPt9InterlinearManifest: async () => ({ 'Lexicon.xml': 'bbbb2222' }),
+      });
+      await act(async () => {
+        renderLoader();
+      });
+      await userEvent.click(screen.getByTestId('tab-toolbar-project-menu'));
+      await userEvent.click(screen.getByTestId('select-modal-open-import'));
+
+      expect(await screen.findByTestId('pt9-import-running')).toHaveTextContent(
+        '%interlinearizer_pt9ImportModal_syncing%',
+      );
+      await act(async () => {
+        releaseImport();
+        await importHeld;
+      });
+    });
+
     it('opens the stored import with a warning when the open-path sync fails', async () => {
       mockImportCommands();
       mockPdpGet.mockRejectedValue(new Error('provider unavailable'));
@@ -1712,6 +1820,51 @@ describe('InterlinearizerLoader', () => {
         severity: 'error',
       });
     });
+
+    it('seeds the store with the synced analysis, not the one the sync replaced', async () => {
+      const syncedAnalysis = emptyAnalysis();
+      syncedAnalysis.tokenAnalyses.push({
+        ...FIXTURE_STAMPS,
+        id: 't-synced',
+        surfaceText: 'In',
+        gloss: { en: 'synced' },
+      });
+      let synced = false;
+      mockSendCommand.mockImplementation(async (...args) => {
+        if (args[0] === 'interlinearizer.getProject')
+          return JSON.stringify({
+            ...FRESH_IMPORT_SUMMARY,
+            analysis: synced ? syncedAnalysis : emptyAnalysis(),
+          });
+        if (args[0] === 'interlinearizer.importPt9Project') {
+          synced = true;
+          return JSON.stringify({
+            outcome: 'imported',
+            projectId: 'import-1',
+            report: IMPORT_REPORT,
+          });
+        }
+        return JSON.stringify(emptyDraft(testProjectId));
+      });
+      await renderImportView();
+
+      await userEvent.click(screen.getByTestId('pt9-sync-button'));
+      await screen.findByTestId('pt9-import-report');
+
+      expect(seededAnalyses.at(-1)).toEqual(syncedAnalysis);
+    });
+
+    it('says so in the view when the imported analysis fails to load', async () => {
+      jest.mocked(papi.notifications.send).mockRejectedValue(new Error('ui offline'));
+      mockSendCommand.mockImplementation(async (...args) =>
+        args[0] === 'interlinearizer.getProject' ? '' : JSON.stringify(emptyDraft(testProjectId)),
+      );
+      await act(async () =>
+        renderLoader({ useWebViewState: makeWebViewState({ activeProject: STUB_IMPORT_PROJECT }) }),
+      );
+
+      expect(await screen.findByTestId('import-load-error')).toBeInTheDocument();
+    });
   });
 
   describe('modal interactions', () => {
@@ -3059,6 +3212,7 @@ describe('analysis store lifetime', () => {
     probeWriteGloss = undefined;
     capturedInterlinearizerProps = undefined;
     capturedStoreProps = undefined;
+    seededAnalyses.length = 0;
     interlinearizerMountCount = 0;
     mockBookData();
     mockOptimisticSetting();

src/__tests__/components/SegmentView.test.tsx

@@ -47,9 +47,12 @@ const mockUsePhraseDispatch = jest.fn<jest.MockedObject<PhraseDispatch>, []>().m
 /** Stable mock fn capturing `useSegmentFreeTranslationDispatch` calls so tests can assert on them. */
 const mockSegmentFreeTranslationDispatch = jest.fn<void, [string, string, string]>();
 
+/** What the mocked `useAnalysisReadOnly` returns; module state, so each test resets it. */
+const mockReadOnly = { current: false };
+
 jest.mock('../../components/AnalysisStore', () => ({
   __esModule: true,
-  useAnalysisReadOnly: () => false,
+  useAnalysisReadOnly: () => mockReadOnly.current,
   AnalysisStoreProvider({ children }: Readonly<{ children: ReactNode; analysisLanguage: string }>) {
     return children;
   },
@@ -236,6 +239,7 @@ describe('SegmentView', () => {
     });
     mockCandidateTokenRefs.current = new Set();
     mockSplitFreeTokenRefs.current = new Set();
+    mockReadOnly.current = false;
   });
 
   it('renders word token chips in token-chip mode (default)', () => {
@@ -595,6 +599,12 @@ describe('SegmentView', () => {
       expect(screen.queryByTestId('baseline-split-gap')).not.toBeInTheDocument();
     });
 
+    it('shows no split gap while the analysis is read-only, even with Alt held', () => {
+      mockReadOnly.current = true;
+      renderBaseline();
+      expect(screen.queryByTestId('baseline-split-gap')).not.toBeInTheDocument();
+    });
+
     it('splits at the resolved anchor on an Alt+click of the gap', () => {
       const { dispatch } = renderBaseline();
       fireEvent.click(screen.getByTestId('baseline-split-gap'), { altKey: true });

src/components/ArcOverlay.tsx

@@ -106,6 +106,11 @@ type ArcOverlayProps = Readonly<{
    * keeps interactive controls only on the focused phrase. Arcs themselves are unaffected.
    */
   simplifyPhrases?: boolean;
+  /**
+   * When `true`, no split button renders: splitting an arc mutates the analysis, which a read-only
+   * one does not allow. The arcs themselves still draw.
+   */
+  readOnly?: boolean;
 }>;
 
 /**
@@ -128,6 +133,7 @@ export function ArcOverlay({
   onSplitHoverChange,
   onHoverPhrase,
   simplifyPhrases = false,
+  readOnly = false,
 }: ArcOverlayProps) {
   const [splitHoveredArc, setSplitHoveredArc] = useState<ArcSplitTarget | undefined>();
 
@@ -288,7 +294,10 @@ export function ArcOverlay({
           {hoveredArcPaths.map(renderArcPath)}
         </svg>
       )}
+      {/* The arcs above still draw read-only; only their split control goes away, since splitting
+          would mutate the analysis. */}
       {phraseMode.kind === 'view' &&
+        !readOnly &&
         sortedArcPaths
           // When simplifyPhrases is on, only the focused phrase keeps its split button; every
           // other phrase's button is hidden while its arc stays drawn.

src/components/ContinuousView.tsx

@@ -11,7 +11,7 @@ import type { ViewOptions } from '../types/view-options';
 import { resolvedOrEmpty } from '../utils/localized-strings';
 import { buildRenderUnits, groupTokens, resolveFocusContext } from '../utils/token-layout';
 import { buildVerseStartLabelsByTokenRef, slotVerseLabel } from '../utils/verse-superscripts';
-import { usePhraseLinkByIdMap, usePhraseLinkMap } from './AnalysisStore';
+import { useAnalysisReadOnly, usePhraseLinkByIdMap, usePhraseLinkMap } from './AnalysisStore';
 import { PhraseStripProvider } from './PhraseStripContext';
 import { PhraseStrip, LINK_SLOT_TRANSITION_MS, type StripItem } from './PhraseStripParts';
 import {
@@ -167,6 +167,7 @@ export default function ContinuousView({
 
   const committedPhraseLinkByRef = usePhraseLinkMap();
   const committedPhraseLinkById = usePhraseLinkByIdMap();
+  const readOnly = useAnalysisReadOnly();
 
   const editPhraseTokens = useEditPhraseTokens(phraseMode);
 
@@ -1198,6 +1199,7 @@ export default function ContinuousView({
             onSplitHoverChange={handleSplitHoverChange}
             onHoverPhrase={setHoveredPhraseId}
             simplifyPhrases={simplifyPhrases}
+            readOnly={readOnly}
           />
           <PhraseStripProvider value={stripContext}>
             <div