Improve PT9 XML Parser error handling, reflow some tests, improve docs

Author: alex-rawlings-yyc

src/__tests__/components/ContinuousView.test.tsx

@@ -101,7 +101,12 @@ function makeBook(overrides?: Partial<Book>): Book {
   };
 }
 
-/** A two-chapter book: chapter 1 has one segment, chapter 2 has one segment. */
+/**
+ * Builds a two-chapter Book fixture: chapter 1 has one segment ("Alpha"), chapter 2 has one segment
+ * ("Beta"). Used to exercise cross-chapter traversal and verse-jump behaviour.
+ *
+ * @returns A two-chapter Book.
+ */
 function makeTwoChapterBook(): Book {
   return {
     id: 'GEN',
@@ -144,7 +149,12 @@ function makeTwoChapterBook(): Book {
   };
 }
 
-/** A book with exactly one token (minimal edge case). */
+/**
+ * Builds a Book with exactly one word token in one segment. Used to assert that both navigation
+ * arrows are disabled when the strip has nowhere to move.
+ *
+ * @returns A single-token Book.
+ */
 function makeSingleTokenBook(): Book {
   return {
     id: 'GEN',
@@ -218,7 +228,12 @@ function makeMixedBook(): Book {
   };
 }
 
-/** A book where every token is non-word, so phraseEntries is empty. */
+/**
+ * Builds a Book whose only token is punctuation, so phraseEntries is empty. Used to exercise the
+ * code path where ContinuousView renders with no word tokens to navigate between.
+ *
+ * @returns A word-free Book.
+ */
 function makeWordFreeBook(): Book {
   return {
     id: 'GEN',

src/__tests__/components/Interlinearizer.test.tsx

@@ -109,6 +109,13 @@ const GEN_1_MULTI_BOOK: Book = {
   ],
 };
 
+/**
+ * Renders an Interlinearizer component with sensible defaults, allowing individual props to be
+ * overridden per test.
+ *
+ * @param options - Partial props to merge over the defaults.
+ * @returns The render result from @testing-library/react.
+ */
 function renderInterlinearizer({
   book = GEN_1_1_BOOK,
   bookSegments = GEN_1_1_BOOK.segments,
@@ -226,9 +233,9 @@ describe('Interlinearizer', () => {
 
     expect(screen.getByTestId('continuous-view')).toBeInTheDocument();
 
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any, no-type-assertion/no-type-assertion
-    const onVerseChange = capturedContinuousViewProps.onVerseChange as any;
-    expect(onVerseChange).toBeDefined();
+    const { onVerseChange } = capturedContinuousViewProps;
+    if (typeof onVerseChange !== 'function')
+      throw new Error('Expected onVerseChange to be a function');
 
     onVerseChange({ book: 'GEN', chapter: 2, verse: 3 });
 

src/__tests__/hooks/useInterlinearizerBookData.test.ts

@@ -87,14 +87,20 @@ const TEST_BOOK: Book = {
 const GEN_1_1_SRC_REF = { book: 'GEN', chapterNum: 1, verseNum: 1 };
 
 describe('useInterlinearizerBookData', () => {
-  /** Default mock setup for useProjectData */
+  /**
+   * Configures useProjectData to return a resolved USJ object so the hook can proceed to
+   * extractBookFromUsj and tokenizeBook without hitting the error-state branches.
+   */
   const setupDefaultProjectDataMock = () => {
     jest.mocked(useProjectData).mockReturnValue({
       BookUSJ: () => [{ USJ: 'mock-usj' }, jest.fn(), false],
     });
   };
 
-  /** Default mock setup for useProjectSetting */
+  /**
+   * Configures useProjectSetting to return the writing system code 'en' so the hook uses a
+   * valid BCP 47 tag rather than falling back to 'und'.
+   */
   const setupDefaultProjectSettingMock = () => {
     jest.mocked(useProjectSetting).mockReturnValue(['en', jest.fn(), jest.fn(), false]);
   };

src/__tests__/main.test.ts

@@ -50,10 +50,22 @@ const {
   __mockLogger,
 } = papiBackendMock;
 
+/**
+ * Type guard that narrows an unknown value to a callable function.
+ *
+ * @param f - The value to test.
+ * @returns True if f is a function.
+ */
 function isCallable(f: unknown): f is (...args: unknown[]) => unknown {
   return typeof f === 'function';
 }
 
+/**
+ * Finds the handler registered for a command name in the mocked registerCommand calls.
+ *
+ * @param commandName - The command name to look up.
+ * @returns The handler function, or undefined if no matching call was recorded.
+ */
 function findRegisteredHandler(commandName: string): ((...args: unknown[]) => unknown) | undefined {
   const call = jest.mocked(__mockRegisterCommand).mock.calls.find((c) => c[0] === commandName);
   const rawHandler: unknown = call?.[1];
@@ -73,12 +85,24 @@ async function getOpenForWebViewHandler(): Promise<
   };
 }
 
+/**
+ * Retrieves the callback passed to onDidOpenWebView during the most recent activate() call.
+ *
+ * @returns A typed wrapper around the captured callback.
+ * @throws If no callback was registered (i.e. activate was not called first).
+ */
 function getOpenWebViewCallback(): (event: { webView: SavedWebViewDefinition }) => void {
   const cb: unknown = __mockOnDidOpenWebView.mock.calls[0]?.[0];
   if (!isCallable(cb)) throw new Error('onDidOpenWebView callback not found');
   return (event) => cb(event);
 }
 
+/**
+ * Retrieves the callback passed to onDidCloseWebView during the most recent activate() call.
+ *
+ * @returns A typed wrapper around the captured callback.
+ * @throws If no callback was registered (i.e. activate was not called first).
+ */
 function getCloseWebViewCallback(): (event: { webView: SavedWebViewDefinition }) => void {
   const cb: unknown = __mockOnDidCloseWebView.mock.calls[0]?.[0];
   if (!isCallable(cb)) throw new Error('onDidCloseWebView callback not found');

src/__tests__/parsers/papi/bookTokenizer.test.ts

@@ -4,6 +4,12 @@
 import { tokenizeBook } from 'parsers/papi/bookTokenizer';
 import type { RawBook } from 'parsers/papi/usjBookExtractor';
 
+/**
+ * Builds a minimal RawBook fixture for GEN with the given verses.
+ *
+ * @param verses - Array of verse objects (sid + text) to include in the book.
+ * @returns A RawBook with fixed bookCode, writingSystem, and contentHash.
+ */
 function makeRawBook(verses: { sid: string; text: string }[]): RawBook {
   return { bookCode: 'GEN', writingSystem: 'en', contentHash: 'abc123', verses };
 }

src/__tests__/parsers/papi/usjBookExtractor.test.ts

@@ -20,12 +20,14 @@ describe('extractBookFromUsj', () => {
     expect(extractBookFromUsj(usj, 'kmr').writingSystem).toBe('kmr');
   });
 
-  it('produces a stable contentHash independent of writingSystem', () => {
+  it('produces the same contentHash for identical content with different writingSystems', () => {
     const a: UsjDocument = { content: [{ type: 'book', code: 'GEN', content: [] }] };
     const b: UsjDocument = { content: [...a.content] };
-    // Same content, different writingSystem — hash must be identical (it hashes content only).
     expect(extractBookFromUsj(a, 'en').contentHash).toBe(extractBookFromUsj(b, 'es').contentHash);
-    // Different content must yield a different hash.
+  });
+
+  it('produces a different contentHash for different content', () => {
+    const a: UsjDocument = { content: [{ type: 'book', code: 'GEN', content: [] }] };
     const c: UsjDocument = { content: [{ type: 'book', code: 'MAT', content: [] }] };
     expect(extractBookFromUsj(a, WS).contentHash).not.toBe(extractBookFromUsj(c, WS).contentHash);
   });
@@ -263,6 +265,63 @@ describe('extractBookFromUsj', () => {
     expect(verses[0].text).toBe('I have gone astray');
   });
 
+  it('skips text inside a heading para marker that appears mid-verse (before the verse is closed)', () => {
+    // An s1 heading node that arrives while a verse is still open must not contribute its text.
+    const usj: UsjDocument = {
+      content: [
+        { type: 'book', code: 'PSA', content: [] },
+        {
+          type: 'para',
+          marker: 'p',
+          content: [{ type: 'verse', sid: 'PSA 1:1' }, 'Blessed is the man'],
+        },
+        // s1 heading arrives while PSA 1:1 is still the currentVerse
+        { type: 'para', marker: 's1', content: ['Interlude'] },
+        {
+          type: 'para',
+          marker: 'p',
+          content: ['who walks not in the counsel of the wicked.'],
+        },
+      ],
+    };
+    const { verses } = extractBookFromUsj(usj, WS);
+    expect(verses).toHaveLength(1);
+    expect(verses[0].text).toBe('Blessed is the man who walks not in the counsel of the wicked.');
+  });
+
+  it('includes text nested inside multiple levels of inline char nodes', () => {
+    // The traverse fallback recurses into any unknown node that has content, so deeply
+    // nested char nodes must still contribute their text.
+    const usj: UsjDocument = {
+      content: [
+        { type: 'book', code: 'JHN', content: [] },
+        {
+          type: 'para',
+          marker: 'p',
+          content: [
+            { type: 'verse', sid: 'JHN 1:14' },
+            'And the ',
+            {
+              type: 'char',
+              marker: 'em',
+              content: [
+                {
+                  type: 'char',
+                  marker: 'nd',
+                  content: ['Word'],
+                },
+              ],
+            },
+            ' became flesh.',
+          ],
+        },
+      ],
+    };
+    const { verses } = extractBookFromUsj(usj, WS);
+    expect(verses).toHaveLength(1);
+    expect(verses[0].text).toBe('And the Word became flesh.');
+  });
+
   it('produces a stable contentHash when a node has an optional property explicitly set to undefined', () => {
     const withUndefined: UsjDocument = {
       content: [{ type: 'book', code: 'GEN', marker: undefined, content: [] }],

src/__tests__/parsers/pt9/interlinearXmlParser.test.ts

@@ -466,6 +466,47 @@ describe('InterlinearXmlParser', () => {
       });
     });
 
+    it('parses multiple Punctuation entries in a single verse', () => {
+      const xml = `
+        <InterlinearData GlossLanguage="en" BookId="MAT">
+          <Verses>
+            <item>
+              <string>MAT 1:1</string>
+              <VerseData>
+                <Cluster>
+                  <Range Index="0" Length="1" />
+                  <Lexeme Id="x" />
+                </Cluster>
+                <Punctuation>
+                  <Range Index="5" Length="1" />
+                  <BeforeText>,</BeforeText>
+                  <AfterText> </AfterText>
+                </Punctuation>
+                <Punctuation>
+                  <Range Index="12" Length="1" />
+                  <BeforeText>.</BeforeText>
+                  <AfterText></AfterText>
+                </Punctuation>
+              </VerseData>
+            </item>
+          </Verses>
+        </InterlinearData>
+      `;
+      const result = parser.parse(xml);
+
+      expect(result.Verses['MAT 1:1'].Punctuations).toHaveLength(2);
+      expect(result.Verses['MAT 1:1'].Punctuations[0]).toEqual({
+        TextRange: { Index: 5, Length: 1 },
+        BeforeText: ',',
+        AfterText: ' ',
+      });
+      expect(result.Verses['MAT 1:1'].Punctuations[1]).toEqual({
+        TextRange: { Index: 12, Length: 1 },
+        BeforeText: '.',
+        AfterText: '',
+      });
+    });
+
     it('parses VerseData with Punctuation but no Cluster (Cluster ?? [] branch)', () => {
       const xml = `
         <InterlinearData GlossLanguage="en" BookId="MAT">
@@ -718,9 +759,7 @@ describe('InterlinearXmlParser', () => {
       expect(() => parser.parse(xmlNoIndex)).toThrow(
         expect.objectContaining({
           name: 'SyntaxError',
-          message: expect.stringContaining(
-            'Invalid XML: Range missing or invalid Index/Length attributes (must be non-negative integers)',
-          ),
+          message: expect.stringContaining('Invalid XML: Range missing Index or Length attribute'),
         }),
       );
 
@@ -742,9 +781,7 @@ describe('InterlinearXmlParser', () => {
       expect(() => parser.parse(xmlNoLength)).toThrow(
         expect.objectContaining({
           name: 'SyntaxError',
-          message: expect.stringContaining(
-            'Invalid XML: Range missing or invalid Index/Length attributes (must be non-negative integers)',
-          ),
+          message: expect.stringContaining('Invalid XML: Range missing Index or Length attribute'),
         }),
       );
     });
@@ -846,7 +883,7 @@ describe('InterlinearXmlParser', () => {
           expect.objectContaining({
             name: 'SyntaxError',
             message: expect.stringContaining(
-              'Invalid XML: Range missing or invalid Index/Length attributes (must be non-negative integers)',
+              'Invalid XML: Range has invalid Index/Length attributes (must be non-negative integers)',
             ),
           }),
         );

src/parsers/pt9/interlinearXmlParser.ts

@@ -211,8 +211,8 @@ function extractPunctuationsFromVerse(verseDataElement: ParsedVerseData): Punctu
  * @param verseDataElement - Parsed VerseData from fast-xml-parser (may have Cluster array or none).
  * @returns Array of ClusterData: TextRange from Cluster's Range, Lexemes from Lexeme children,
  *   LexemesId (slash-joined), Id (LexemesId/Index-Length or Index-Length when no lexemes).
- * @throws {SyntaxError} If a Cluster is missing its Range element or Range is missing Index or
- *   Length.
+ * @throws {SyntaxError} If a Cluster is missing its Range element, Range is missing Index or
+ *   Length, or Range Index/Length are not non-negative integers.
  */
 function extractClustersFromVerse(verseDataElement: ParsedVerseData): ClusterData[] {
   const clusterElements = verseDataElement.Cluster ?? [];
@@ -223,11 +223,15 @@ function extractClustersFromVerse(verseDataElement: ParsedVerseData): ClusterDat
       throw new SyntaxError('Invalid XML: Cluster missing required Range element');
     }
 
+    if (rangeElement['@_Index'] === undefined || rangeElement['@_Length'] === undefined) {
+      throw new SyntaxError('Invalid XML: Range missing Index or Length attribute');
+    }
+
     const index = parseStrictNumber(rangeElement['@_Index']);
     const length = parseStrictNumber(rangeElement['@_Length']);
     if (index === undefined || length === undefined) {
       throw new SyntaxError(
-        'Invalid XML: Range missing or invalid Index/Length attributes (must be non-negative integers)',
+        'Invalid XML: Range has invalid Index/Length attributes (must be non-negative integers)',
       );
     }
 
@@ -299,8 +303,8 @@ export class InterlinearXmlParser {
    * @throws {SyntaxError} If the `InterlinearData` root element is absent.
    * @throws {SyntaxError} If `GlossLanguage` or `BookId` attributes are missing or empty.
    * @throws {SyntaxError} If the `Verses` element is absent.
-   * @throws {SyntaxError} If a `Cluster` is missing its `Range` element, or `Range` has invalid
-   *   `Index`/`Length` attributes (must be non-negative integers).
+   * @throws {SyntaxError} If a `Cluster` is missing its `Range` element, `Range` is missing
+   *   `Index`/`Length`, or `Index`/`Length` are not non-negative integers.
    * @throws {SyntaxError} If a `Lexeme` element is missing its required `Id` attribute.
    * @throws {SyntaxError} If the same verse reference appears in more than one `item`.
    */