Fix review findings on the Paratext 9 import WebView

[alex-rawlings-yyc]

Review fixes for #274, branched off pt9-parsed-webview at ef8e70b. Five defects, each with a regression test that fails against that commit.

Read-only enforcement had three holes

#274 guards BoundaryControl and PhraseStripParts, but three parallel mutation routes stayed live in an import view:

• SegmentListView.tsx:428 — the between-rows merge button dispatches segmentationDispatch.merge, which writes boundaries into the user's editable draft and flips the dirty indicator. Nothing visible happens in the import, since book is computed with resegmentBook(verseBook, undefined) there, so it silently corrupts an unrelated draft.
• SegmentView.tsx:191 — Alt+click on a baseline-text split gap: the same silent draft write, in continuous-scroll mode. The Alt-gated SplitMarker in PhraseStripParts is covered; this is its baseline-mode twin.
• ArcOverlay.tsx:291 — the arc split button. analysisMerger.ts:401 emits phraseAnalysisLinks, so imports do draw arcs; hovering one reveals Link2Off, and a click deletes a 2-token phrase outright. With PhraseBox's pill and TokenLinkIcon both gated, this was the only surviving route that visibly mutates the import.

ArcOverlay takes a readOnly prop rather than reading the hook — it's a presentational leaf whose tests render it bare, so the hook would have forced a provider onto every call site.

The offer mode never reached the modal

InterlinearizerLoader.tsx:1261 collapsed pt9Mode === 'offer' into 'import', leaving Pt9ImportModal's entire mode === 'offer' branch dead outside its own unit test. Both consequences contradict the spec added in user-questions.md ("a single Open (dismissing the report also opens…)"): the offer report rendered Close + Open, and dismissing it routed through handlePt9Close → setModal('none'), so a successful conversion was never opened — the user had to go find it in Select. The test "returns to the plain view when an offer-run report is closed" locked that in; it's replaced by two tests asserting the documented behavior.

A failed import-analysis load rendered a blank pane

Once the book has loaded and neither book error applies, all three of loadingOrErrorPanel's conditionals are falsy, so the user got an empty content area plus a toast. Reachable through openImportedProject's :718 fallback. Adds an importLoadFailed state, an on-screen message, and a localized string.

handlePt9Open wrote state after the user backed out

The report stays dismissable during fetchSummary, so Escape → back to Select → resolve closed the Select modal and switched the active project anyway. Adds a modalRef in-flight guard.

One finding investigated and rejected

A claim that a manual sync remounts the store seeded with the pre-sync analysis (initialAnalysis is a non-reactive lazy-ref seed while the key carries updatedAt). Instrumenting the actual mount sequence with and without the proposed fix gave identical seeds both ways — React's batching means the remount already happens after the fresh analysis lands. No code change; the test asserting the correct behavior stays, since nothing covered it before.

Verified clean

No change needed, recorded so it isn't re-checked: savePt9Import's buildNew() spread is field-for-field equivalent to the literal it replaced; the openRequest effect's exhaustive-deps disable has no stale closure; hasDraft/hasNoInterlinearizerState handle ENOENT correctly; __setMockAnalysisReadOnly's module state is reset in every file that touches it.

Left for review

• isPt9TooLargeError calls error.message.includes(...) after isPlatformError, whose real guard only checks for platformErrorVersion. A platform error without message would throw out of the catch in runPt9Import. Whether any producer omits it is unverified, so this is untouched rather than guessed at.
• The on-open auto-sync path had no test; one now covers the sync wording. Worth confirming that's what users should see.

Also

A comment-rules pass: two mock docs naming the wrong lifecycle hook, one over-long doc, one mechanism-explaining inline.

This change is 

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: effa8394-2524-4214-9a4b-703cafe2d364

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[imnasnainaec]

This may be partially redundant with #292. I'm investigating now.

[alex-rawlings-yyc]

@imnasnainaec Right you are. #292 already covers everything on this branch except one thing.

handlePt9Open here drops the state write if the user dismissed the report while fetchSummary was in flight (modalRef guard). #292 wraps that call in try/catch, which handles a rejection but not a resolve the user backed out of — Escape → back to Select still closes the Select modal and switches the active project. The two guards are complementary; #292 should take this one on top of its catch.

Everything else is duplicated by #292 or worse than it. Closing in favor of #292 plus that guard.

[imnasnainaec]

I did take 3 things from this pr:

• Its handlePt9Open in-flight guard is a live defect in this branch: dismiss the report while the summary is fetching, and the resolve switched projects and closed the picker you'd gone back to. Added the guard and a deferred-promise test.
• Gated the failed-load line on !hasError && !showLoading, so it can't print beneath "Loading…".
• Pt9ImportModal's onOpen doc.