Skip to content

Read live PT book via USJ and open interlinearizer from project menu#24

Merged
imnasnainaec merged 18 commits intomainfrom
read-current-pt-book
Apr 23, 2026
Merged

Read live PT book via USJ and open interlinearizer from project menu#24
imnasnainaec merged 18 commits intomainfrom
read-current-pt-book

Conversation

@alex-rawlings-yyc
Copy link
Copy Markdown
Contributor

@alex-rawlings-yyc alex-rawlings-yyc commented Apr 21, 2026

Summary

  • Replace the hard-coded bundled XML path with live papi data: the web view now fetches the active book via useProjectData('platformScripture.USJ_Book') and re-renders as the user navigates in Paratext
  • Register an interlinearizer.open command that shows a project picker when no project is provided, then opens the web view linked to the chosen project; each project gets its own tab, and reopening an already-open project brings that tab to the front
  • Wire up a main menu item under Platform → Interlinearizer → Open Interlinearizer that invokes the command, with localized strings
  • Expand interlinearizer.d.ts with the full interlinear data model (InterlinearAlignment, InterlinearText, Book, Segment, Token, TextAnalysis, TokenAnalysis, Phrase, AlignmentLink, etc.) and declare the new command in papi-shared-types

This change is Reviewable

Summary by CodeRabbit

  • New Features
    • Interlinearizer menu and command to open per-project or target a specific view; tabs reuse per project.
  • UI
    • Web view is project-driven: prompts to pick a project, shows Loading…, error messages, or raw content when available.
  • Localization
    • Added English strings for Interlinearizer labels and dialogs.
  • Tests
    • Expanded mocks and tests for command/open-webview lifecycles.
  • Breaking Changes
    • Internal interlinear data type exports removed; command-based entry points introduced.
  • Chores
    • Updated editor and test-runner settings and mock reset behavior.

@alex-rawlings-yyc alex-rawlings-yyc self-assigned this Apr 21, 2026
@alex-rawlings-yyc alex-rawlings-yyc linked an issue Apr 21, 2026 that may be closed by this pull request
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 21, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • ✅ Review completed - (🔄 Check again to review again)
📝 Walkthrough

Walkthrough

Adds project-aware Interlinearizer flows: new commands to open/reuse per-project webviews, project picker integration, webview component switched to loading BookUSJ via platform hooks, expanded mocks/types/tests, and localization/menu entries for the feature.

Changes

Cohort / File(s) Summary
Editor & Test Config
\.vscode/settings.json, jest.config.ts
Scoped VS Code JS/TS validation/format settings; Jest config changed from clearMocks to resetMocks.
Localization & Menus
contributions/localizedStrings.json, contributions/menus.json
Added English localized strings and a top-menu group/item to invoke Interlinearizer for the Scripture Editor webview.
Platform Mocks
__mocks__/papi-backend.ts, __mocks__/platform-bible-utils.ts
Expanded mocked papi API (registerCommand, dialogs.selectProject, webViews.getOpenWebViewDefinition, onDidOpenWebView, onDidCloseWebView) and added PlatformError shape plus isPlatformError type guard.
Core Runtime & Commands
src/main.ts
Added InterlinearizerOpenOptions (projectId?), per-project openWebViewsByProject cache, new commands interlinearizer.open and interlinearizer.openForWebView, open/close subscriptions, and getWebView propagation of projectId.
WebView Component
src/interlinearizer.web-view.tsx
Reworked component to accept WebViewProps (projectId, scroll hook), derive scrRef, fetch BookUSJ via useProjectData, and render prompt/loading/error/book JSON (removed XML parser UI).
Parser & Types
src/parsers/interlinearXmlParser.ts, src/types/interlinearizer.d.ts
Parser now exports local interlinear data interfaces; removed public interlinear data types from interlinearizer module and added command handler signatures to shared types.
Tests
src/__tests__/main.test.ts, src/__tests__/interlinearizer.web-view.test.tsx
Updated tests to cover new command registration and behaviors (picker, reuse, cancel/open semantics) and rewrote webview tests to rely on project-data/hook-driven flows.
Misc / Mocks
__mocks__/*, README.md
Minor mock export additions, newline/format tweaks, and README capitalization fix.

Sequence Diagram

sequenceDiagram
    participant User
    participant Cmd as interlinearizer.open
    participant Picker as papi.dialogs.selectProject
    participant Cache as openWebViewsByProject
    participant PapiWebView as papi.webViews.openWebView
    participant WebViewDef as papi.webViews.getOpenWebViewDefinition
    participant Component as InterlinearizerWebView

    User->>Cmd: Execute (projectId?)
    alt projectId provided
        Cmd->>Cache: check for existing webViewId
        alt cached
            Cache->>User: return existing webViewId
        else not cached
            Cmd->>PapiWebView: openWebView(projectId, openOptions)
            PapiWebView->>Component: init with projectId
            Component->>Component: useProjectData.BookUSJ(scrRef)
            PapiWebView->>Cache: store webViewId for projectId
            PapiWebView->>User: return new webViewId
        end
    else projectId omitted
        Cmd->>Picker: selectProject()
        Picker->>User: show picker
        User->>Picker: select or cancel
        alt selected
            Picker->>Cmd: return projectId
            Cmd->>PapiWebView: open/reuse as above
        else canceled
            Picker->>Cmd: return undefined
            Cmd->>User: return undefined
        end
    end

    %% openForWebView flow
    User->>Cmd: Execute interlinearizer.openForWebView(webViewId?)
    Cmd->>WebViewDef: getOpenWebViewDefinition(webViewId)
    alt definition has projectId
        Cmd->>Cmd: call open(projectId)
    else missing projectId
        Cmd->>Picker: selectProject()
        Picker->>Cmd: return projectId or undefined
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

🟨Medium

Suggested reviewers

  • jasonleenaylor

Poem

🐰 A hop, a pick, a webview bright,

I map each project through the night,
From picker prompt to book-loaded screen,
The XML left — BookUSJ is seen,
Commands now open tabs just right.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 61.54% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main changes: reading live PT book data via USJ and opening the interlinearizer from a project menu, which aligns with the PR's core objectives.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch read-current-pt-book

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

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]

This comment was marked as resolved.

@alex-rawlings-yyc alex-rawlings-yyc force-pushed the read-current-pt-book branch 3 times, most recently from 28fda1e to 7423c1f Compare April 21, 2026 21:54
@alex-rawlings-yyc alex-rawlings-yyc linked an issue Apr 21, 2026 that may be closed by this pull request
@alex-rawlings-yyc

This comment was marked as outdated.

@coderabbitai

This comment was marked as outdated.

@imnasnainaec
Copy link
Copy Markdown
Contributor

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 22, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor

@imnasnainaec imnasnainaec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@imnasnainaec reviewed 3 files and all commit messages, and made 3 comments.
Reviewable status: 3 of 13 files reviewed, 3 unresolved discussions (waiting on alex-rawlings-yyc).


src/main.ts line 133 at r1 (raw file):
⛏️ Devin suggests

Adding openWebViewsByProject.clear() in deactivate() would be a defensive improvement.


__mocks__/papi-backend.ts line 9 at r1 (raw file):

const mockRegisterCommand = jest.fn().mockResolvedValue({ dispose: jest.fn() });
const mockOpenWebView = jest.fn().mockResolvedValue('mock-webview-id');
const mockSelectProject = jest.fn().mockResolvedValue(undefined);

❓ Aren't the mock resolved values here superfluously if jest.config.ts has resetMocks: true,?


src/interlinearizer.web-view.tsx line 44 at r1 (raw file):

          {!bookError && (
            <pre className="tw-overflow-auto tw-rounded-md tw-border tw-border-border tw-bg-muted tw-p-4 tw-text-sm tw-font-mono tw-leading-relaxed">
              {bookUsj ? JSON.stringify(bookUsj, undefined, 2) : 'Loading…'}

When I try to open the Interlinearizer with SDBG, it gets stuck on "Loading..." even though the console has

project-lookup.service-model.ts:247
Uncaught (in promise) Error: No project found with ID SDBG and project interface platformScripture.USJ_Book
    at Object.getMetadataForProject (project-lookup.service-model.ts:247:11)
    at async get (project-data-provider.service.ts:173:20)
    at async index.js:9229:19

...so the error handling isn't complete.

@alex-rawlings-yyc
Copy link
Copy Markdown
Contributor Author

__mocks__/papi-backend.ts line 9 at r1 (raw file):

Previously, imnasnainaec (D. Ror.) wrote…

❓ Aren't the mock resolved values here superfluously if jest.config.ts has resetMocks: true,?

Good catch. Removing the mock resolved values.

Copy link
Copy Markdown
Contributor Author

@alex-rawlings-yyc alex-rawlings-yyc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alex-rawlings-yyc made 2 comments.
Reviewable status: 3 of 13 files reviewed, 3 unresolved discussions (waiting on alex-rawlings-yyc and imnasnainaec).


src/interlinearizer.web-view.tsx line 44 at r1 (raw file):

Previously, imnasnainaec (D. Ror.) wrote…

When I try to open the Interlinearizer with SDBG, it gets stuck on "Loading..." even though the console has

project-lookup.service-model.ts:247
Uncaught (in promise) Error: No project found with ID SDBG and project interface platformScripture.USJ_Book
    at Object.getMetadataForProject (project-lookup.service-model.ts:247:11)
    at async get (project-data-provider.service.ts:173:20)
    at async index.js:9229:19

...so the error handling isn't complete.

Good catch


src/main.ts line 133 at r1 (raw file):

Previously, imnasnainaec (D. Ror.) wrote…

⛏️ Devin suggests

Adding openWebViewsByProject.clear() in deactivate() would be a defensive improvement.

Adding

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
src/main.ts (1)

122-130: Optional: hoist openInterlinearizerForWebView to module scope.

It does not close over anything defined inside activate (it only uses the module-level papi and openInterlinearizer), so placing it at module scope — next to openInterlinearizer — keeps the two handlers symmetric and avoids re-creating the closure on each activation. Purely stylistic.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main.ts` around lines 122 - 130, The helper openInterlinearizerForWebView
is currently defined inside activate but doesn't close over activate-specific
state (it only uses module-level symbols openInterlinearizer and papi); move its
declaration to module scope next to openInterlinearizer so it isn't re-created
on each activation, then update activate to reference the now-module-level
openInterlinearizerForWebView; ensure the moved function remains async and its
signature (openInterlinearizerForWebView(webViewId?: string): Promise<string |
undefined>) is preserved and that any callers inside activate still work with
the hoisted name.
src/interlinearizer.web-view.tsx (1)

11-14: useProjectData is invoked unconditionally even when projectId is undefined.

The hook gracefully handles an undefined project ID and returns a loading state rather than throwing—however, the unconditional call still triggers unnecessary work. When projectId is undefined, the code path at line 22 constructs an error message "Project undefined does not support the USJ book interface", even though this error is never displayed to the user because it remains guarded behind the outer !projectId check (lines 31–35).

While harmless in practice, this represents wasted computation and muddies the code flow. Consider isolating the data-fetching path behind a subcomponent that only mounts when projectId is defined:

♻️ Proposed structure
-globalThis.webViewComponent = function InterlinearizerWebView({
-  projectId,
-  useWebViewScrollGroupScrRef,
-}: WebViewProps) {
-  const [scrRef] = useWebViewScrollGroupScrRef();
-
-  const [bookResult, , isLoading] = useProjectData('platformScripture.USJ_Book', projectId).BookUSJ(
-    scrRef,
-    undefined,
-  );
-  ...
-};
+function ProjectBookView({ projectId, scrRef }: { projectId: string; scrRef: SerializedVerseRef }) {
+  const [bookResult, , isLoading] = useProjectData('platformScripture.USJ_Book', projectId).BookUSJ(
+    scrRef,
+    undefined,
+  );
+  // ...existing derivation + render of body...
+}
+
+globalThis.webViewComponent = function InterlinearizerWebView({
+  projectId,
+  useWebViewScrollGroupScrRef,
+}: WebViewProps) {
+  const [scrRef] = useWebViewScrollGroupScrRef();
+  return (
+    <div className="tw-flex tw-flex-col tw-gap-4 tw-p-6">
+      <h1 className="tw-text-2xl tw-font-semibold tw-tracking-tight">Interlinearizer</h1>
+      {projectId ? (
+        <ProjectBookView projectId={projectId} scrRef={scrRef} />
+      ) : (
+        <p className="tw-text-sm tw-text-muted-foreground">
+          Open this webview from a Paratext project to load its source book.
+        </p>
+      )}
+    </div>
+  );
+};
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/interlinearizer.web-view.tsx` around lines 11 - 14, The hook
useProjectData(...) and its BookUSJ(...) call are being invoked even when
projectId is undefined; refactor by moving that hook and the BookUSJ(scrRef,
...) usage into a small child component (e.g., ProjectBookFetcher) that is only
rendered when projectId is truthy, or conditionally render the JSX that uses
bookResult/isLoading after checking projectId; update references to bookResult,
isLoading, BookUSJ, scrRef and ensure the parent component no longer calls
useProjectData when projectId is undefined to avoid the unnecessary work and the
dead error message construction.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/interlinearizer.web-view.tsx`:
- Around line 11-14: The hook useProjectData(...) and its BookUSJ(...) call are
being invoked even when projectId is undefined; refactor by moving that hook and
the BookUSJ(scrRef, ...) usage into a small child component (e.g.,
ProjectBookFetcher) that is only rendered when projectId is truthy, or
conditionally render the JSX that uses bookResult/isLoading after checking
projectId; update references to bookResult, isLoading, BookUSJ, scrRef and
ensure the parent component no longer calls useProjectData when projectId is
undefined to avoid the unnecessary work and the dead error message construction.

In `@src/main.ts`:
- Around line 122-130: The helper openInterlinearizerForWebView is currently
defined inside activate but doesn't close over activate-specific state (it only
uses module-level symbols openInterlinearizer and papi); move its declaration to
module scope next to openInterlinearizer so it isn't re-created on each
activation, then update activate to reference the now-module-level
openInterlinearizerForWebView; ensure the moved function remains async and its
signature (openInterlinearizerForWebView(webViewId?: string): Promise<string |
undefined>) is preserved and that any callers inside activate still work with
the hoisted name.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 98490d80-7862-4dd1-a10c-606e7dda5d19

📥 Commits

Reviewing files that changed from the base of the PR and between 7423c1f and 6d46558.

📒 Files selected for processing (8)
  • __mocks__/papi-backend.ts
  • contributions/localizedStrings.json
  • contributions/menus.json
  • src/__tests__/interlinearizer.web-view.test.tsx
  • src/__tests__/main.test.ts
  • src/interlinearizer.web-view.tsx
  • src/main.ts
  • src/types/interlinearizer.d.ts
✅ Files skipped from review due to trivial changes (1)
  • contributions/localizedStrings.json
🚧 Files skipped from review as they are similar to previous changes (1)
  • mocks/papi-backend.ts

Copy link
Copy Markdown
Contributor

@imnasnainaec imnasnainaec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than adding a "Platform" menu item that opens a project picker. We should just add Interlinearizer to the project menu (so that the user doesn't have to pick a project when it's already open). Here's how we do that in the Lexicon extension: https://github.com/sillsdev/languageforge-lexbox/blob/develop/platform.bible-extension/contributions/menus.json

Also, the rabbit has a couple new ⛏️s.

@imnasnainaec made 1 comment and resolved 2 discussions.
Reviewable status: 3 of 13 files reviewed, 1 unresolved discussion (waiting on alex-rawlings-yyc).

Copy link
Copy Markdown
Contributor Author

@alex-rawlings-yyc alex-rawlings-yyc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good stuff, thanks

@alex-rawlings-yyc made 1 comment.
Reviewable status: 3 of 13 files reviewed, 1 unresolved discussion (waiting on alex-rawlings-yyc and imnasnainaec).

@alex-rawlings-yyc
Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 22, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/main.ts (1)

34-50: ⚠️ Potential issue | 🔴 Critical

Fix method signature mismatch and guard against missing openWebViewOptions.

The getWebView method has two issues:

  1. Signature mismatch: Tests call getWebView(savedWebView, options, nonce) with 3 arguments, but the implementation only accepts 2 parameters. The method needs to accept an optional nonce parameter as the third argument.

  2. Defensive handling of options: Even with the correct signature, openWebViewOptions is currently required but should be optional. When the platform rehydrates a persisted SavedWebViewDefinition on startup, it may not provide options, causing an unconditional dereference of openWebViewOptions.projectId to fail. Use optional chaining to handle this:

🛡️ Suggested fix
   async getWebView(
     savedWebView: SavedWebViewDefinition,
-    openWebViewOptions: InterlinearizerOpenOptions,
+    openWebViewOptions: InterlinearizerOpenOptions | undefined,
+    nonce: string,
   ): Promise<WebViewDefinition | undefined> {
     if (savedWebView.webViewType !== mainWebViewType) {
       throw new Error(
         `${mainWebViewType} provider received request to provide a ${savedWebView.webViewType} WebView`,
       );
     }
     return {
       ...savedWebView,
-      projectId: openWebViewOptions.projectId ?? savedWebView.projectId,
+      projectId: openWebViewOptions?.projectId ?? savedWebView.projectId,
       title: 'Interlinearizer',
       content: interlinearizerReact,
       styles: interlinearizerStyles,
     };
   },
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main.ts` around lines 34 - 50, Change the getWebView method signature to
accept an optional third parameter (nonce) so it matches callers (e.g.,
getWebView(savedWebView, options, nonce)); make openWebViewOptions optional in
the signature and use optional chaining when accessing projectId (e.g.,
openWebViewOptions?.projectId) to guard against undefined options coming from
persisted SavedWebViewDefinition rehydration; keep all other returned fields
(title, content, styles) intact and ensure any usage of nonce inside getWebView
or returned object is handled appropriately.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/interlinearizer.web-view.tsx`:
- Around line 20-26: The current logic in the component sets bookError to
"Project ... does not support the USJ book interface" when (!isLoading &&
bookResult === undefined), which can wrongly conflate absence of data with lack
of capability; change the error message (where bookResult, isLoading, bookError,
projectId, and scrRef.book are used) to a softer, specific message like "No USJ
book available for ${scrRef.book} in project ${projectId}" or alternatively add
a capability check at mount to distinguish unsupported projects from empty data,
and ensure bookUsj is only set in the else branch when bookResult is a valid
value.

In `@src/main.ts`:
- Around line 53-92: openWebViewsByProject is only populated on
openInterlinearizer and cleared in deactivate, so it accumulates stale IDs on
tab close and misses webviews that persist across extension restarts; fix by:
subscribe to webview-close events (use the platform/PAPI webViews close or
onDidClose hook) and remove entries from openWebViewsByProject when a tab closes
(reference openWebViewsByProject and the webView ID handling in
openInterlinearizer), and rebuild/populate openWebViewsByProject during
activation (in activate or module init) by querying currently open webviews
(e.g., a papi.webViews.getOpenWebViews(mainWebViewType) or iterating
papi.webViews.getOpenWebViewDefinition() results) so openInterlinearizer
receives a live existingId instead of stale ones.

---

Outside diff comments:
In `@src/main.ts`:
- Around line 34-50: Change the getWebView method signature to accept an
optional third parameter (nonce) so it matches callers (e.g.,
getWebView(savedWebView, options, nonce)); make openWebViewOptions optional in
the signature and use optional chaining when accessing projectId (e.g.,
openWebViewOptions?.projectId) to guard against undefined options coming from
persisted SavedWebViewDefinition rehydration; keep all other returned fields
(title, content, styles) intact and ensure any usage of nonce inside getWebView
or returned object is handled appropriately.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f9711e33-4a78-4116-8ca1-dea9dd4d4ed0

📥 Commits

Reviewing files that changed from the base of the PR and between 6d46558 and a271627.

📒 Files selected for processing (4)
  • contributions/localizedStrings.json
  • contributions/menus.json
  • src/interlinearizer.web-view.tsx
  • src/main.ts
✅ Files skipped from review due to trivial changes (1)
  • contributions/localizedStrings.json

Comment thread src/interlinearizer.web-view.tsx
Comment thread src/main.ts
@imnasnainaec
Copy link
Copy Markdown
Contributor

@alex-rawlings-yyc I just pushed a commit rather than add nit ⛏️ comments. I didn't address CodeRabbit's newest "⚠️ Potential issue | 🔴 Critical".

Copy link
Copy Markdown
Contributor

@imnasnainaec imnasnainaec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@imnasnainaec partially reviewed 6 files, made 1 comment, and resolved 1 discussion.
Reviewable status: 6 of 14 files reviewed, 1 unresolved discussion (waiting on alex-rawlings-yyc).


src/main.ts line 113 at r6 (raw file):

  const openCommandRegistration = await papi.commands.registerCommand(
    'interlinearizer.open',
    openInterlinearizer,

❓ This command is no longer used in the menu. Do we have a reason to keep it (for potential future use)?

@alex-rawlings-yyc
Copy link
Copy Markdown
Contributor Author

Great, I addressed it in a previous commit

@alex-rawlings-yyc
Copy link
Copy Markdown
Contributor Author

src/main.ts line 113 at r6 (raw file):

Previously, imnasnainaec (D. Ror.) wrote…

❓ This command is no longer used in the menu. Do we have a reason to keep it (for potential future use)?

Possibly. Do we want to enable users to open the interlinearizer from the main menu and select a project after, or do we only want them to open it from within a project?

alex-rawlings-yyc and others added 4 commits April 23, 2026 11:45
…r message

- Add webViewNonce param and make openWebViewOptions optional in getWebView to match IWebViewProvider interface and guard against undefined options during SavedWebViewDefinition rehydration
- Subscribe to onDidOpenWebView/onDidCloseWebView in activate to keep openWebViewsByProject accurate across tab closes and extension restarts, replacing the incorrect claim that the platform handles stale IDs
- Soften the absent-USJ-book error message to avoid implying the project lacks capability when data may simply be unavailable
@imnasnainaec
Copy link
Copy Markdown
Contributor

src/main.ts line 113 at r6 (raw file):

Previously, imnasnainaec (D. Ror.) wrote…
Possibly. Do we want to enable users to open the interlinearizer from the main menu and select a project after, or do we only want them to open it from within a project?

@alex-rawlings-yyc I just asked Jason during The Combine's standup. We can't think of a reason to open Interlinearizer without a project already open/specified.

@alex-rawlings-yyc
Copy link
Copy Markdown
Contributor Author

src/main.ts line 113 at r6 (raw file):

Previously, imnasnainaec (D. Ror.) wrote…

@alex-rawlings-yyc I just asked Jason during The Combine's standup. We can't think of a reason to open Interlinearizer without a project already open/specified.

Great, I'll get rid of the command

@imnasnainaec imnasnainaec mentioned this pull request Apr 23, 2026
Copy link
Copy Markdown
Contributor

@imnasnainaec imnasnainaec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@imnasnainaec reviewed 7 files and all commit messages, and resolved 1 discussion.
Reviewable status: 11 of 14 files reviewed, all discussions resolved (waiting on alex-rawlings-yyc).

Copy link
Copy Markdown
Contributor Author

@alex-rawlings-yyc alex-rawlings-yyc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alex-rawlings-yyc reviewed 14 files and all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on alex-rawlings-yyc).

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/interlinearizer.web-view.tsx (1)

43-47: Minor: "Loading…" can briefly show after data arrives if bookUsj is falsy.

The render uses bookUsj ? JSON.stringify(...) : 'Loading…', which will display "Loading…" for any falsy value (e.g. null, empty string). In practice BookUSJ returns an object so this is unlikely to misfire, but a more precise condition would be:

♻️ Suggested tweak
-      {!bookError && (
-        <pre className="tw-overflow-auto tw-rounded-md tw-border tw-border-border tw-bg-muted tw-p-4 tw-text-sm tw-font-mono tw-leading-relaxed">
-          {bookUsj ? JSON.stringify(bookUsj, undefined, 2) : 'Loading…'}
-        </pre>
-      )}
+      {!bookError && (
+        <pre className="tw-overflow-auto tw-rounded-md tw-border tw-border-border tw-bg-muted tw-p-4 tw-text-sm tw-font-mono tw-leading-relaxed">
+          {isLoading ? 'Loading…' : JSON.stringify(bookUsj, undefined, 2)}
+        </pre>
+      )}

This requires lifting isLoading into the render scope (currently in scope already inside ProjectBookFetcher).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/interlinearizer.web-view.tsx` around lines 43 - 47, The current render
uses a falsy check on bookUsj which can show "Loading…" briefly after data
arrives; update the conditional to use the explicit isLoading flag (from
ProjectBookFetcher) instead of bookUsj truthiness and render JSON when bookUsj
is a real value (e.g., check bookUsj !== null/undefined) while keeping the
existing bookError guard; specifically, inside the component that renders the
<pre> (referencing bookUsj, isLoading, ProjectBookFetcher, and bookError),
replace the ternary that shows 'Loading…' based on bookUsj with one that shows
'Loading…' when isLoading is true and otherwise stringifies bookUsj only when
bookUsj is non-null/undefined.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/main.ts`:
- Around line 75-82: The mapping update after awaiting papi.webViews.openWebView
can re-insert a stale closed WebView id; remove the explicit
openWebViewsByProject.set(resolvedProjectId, webViewId) in openInterlinearizer
and rely on the lifecycle handlers (onDidOpenWebView / onDidCloseWebView) to
authoritatively maintain openWebViewsByProject, or if you prefer a safer
approach only set the map after the await when the map has no entry and
papi.webViews confirms the webViewId is still open.

---

Nitpick comments:
In `@src/interlinearizer.web-view.tsx`:
- Around line 43-47: The current render uses a falsy check on bookUsj which can
show "Loading…" briefly after data arrives; update the conditional to use the
explicit isLoading flag (from ProjectBookFetcher) instead of bookUsj truthiness
and render JSON when bookUsj is a real value (e.g., check bookUsj !==
null/undefined) while keeping the existing bookError guard; specifically, inside
the component that renders the <pre> (referencing bookUsj, isLoading,
ProjectBookFetcher, and bookError), replace the ternary that shows 'Loading…'
based on bookUsj with one that shows 'Loading…' when isLoading is true and
otherwise stringifies bookUsj only when bookUsj is non-null/undefined.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b0eb59e7-09c6-43d0-89c7-3f64681d1ce2

📥 Commits

Reviewing files that changed from the base of the PR and between a271627 and 9948161.

📒 Files selected for processing (13)
  • .vscode/settings.json
  • README.md
  • __mocks__/papi-backend.ts
  • __mocks__/platform-bible-utils.ts
  • contributions/localizedStrings.json
  • contributions/menus.json
  • jest.config.ts
  • src/__tests__/interlinearizer.web-view.test.tsx
  • src/__tests__/main.test.ts
  • src/interlinearizer.web-view.tsx
  • src/main.ts
  • src/parsers/interlinearXmlParser.ts
  • src/types/interlinearizer.d.ts
✅ Files skipped from review due to trivial changes (4)
  • README.md
  • contributions/localizedStrings.json
  • mocks/platform-bible-utils.ts
  • src/parsers/interlinearXmlParser.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • .vscode/settings.json
  • contributions/menus.json
  • src/tests/interlinearizer.web-view.test.tsx

Comment thread src/main.ts
Copy link
Copy Markdown
Contributor

@imnasnainaec imnasnainaec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@imnasnainaec reviewed 4 files and all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on alex-rawlings-yyc).

Copy link
Copy Markdown
Contributor Author

@alex-rawlings-yyc alex-rawlings-yyc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alex-rawlings-yyc resolved 1 discussion.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on alex-rawlings-yyc).

Copy link
Copy Markdown
Contributor Author

@alex-rawlings-yyc alex-rawlings-yyc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alex-rawlings-yyc reviewed 2 files and all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on alex-rawlings-yyc).

Copy link
Copy Markdown
Contributor Author

@alex-rawlings-yyc alex-rawlings-yyc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alex-rawlings-yyc reviewed 1 file and all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on imnasnainaec).

Copy link
Copy Markdown
Contributor

@imnasnainaec imnasnainaec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@imnasnainaec partially reviewed 2 files.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on alex-rawlings-yyc).

@imnasnainaec imnasnainaec merged commit 34252a7 into main Apr 23, 2026
8 checks passed
@imnasnainaec imnasnainaec deleted the read-current-pt-book branch April 23, 2026 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Read current PT book PT menu-item for opening “our” extension

2 participants