diff --git a/src/__tests__/components/InterlinearNavContext.test.tsx b/src/__tests__/components/InterlinearNavContext.test.tsx index 5a03d007..7f6d3218 100644 --- a/src/__tests__/components/InterlinearNavContext.test.tsx +++ b/src/__tests__/components/InterlinearNavContext.test.tsx @@ -74,12 +74,13 @@ describe('InterlinearNavContext', () => { expect(result.current.liveScrRef).toEqual(ref); }); - it('normalizes a chapter-level (verse 0) reference to verse 1 in liveScrRef', () => { + it('passes a chapter-level (verse 0) reference through to liveScrRef unchanged', () => { + // Verse 0 is a real verse (a Psalm superscription), so it is no longer mapped to verse 1 here. + // The loader resolves it to verse 1 only when the loaded book has no verse-0 segment. const ref: SerializedVerseRef = { book: 'GEN', chapterNum: 3, verseNum: 0 }; const { result } = renderNav(makeScrollGroupHook(ref)); - expect(result.current.liveScrRef).toEqual({ book: 'GEN', chapterNum: 3, verseNum: 1 }); - // The raw reference still reports verse 0 so the editable nav controls reflect the selection. + expect(result.current.liveScrRef).toEqual(ref); expect(result.current.rawScrRef).toEqual(ref); }); @@ -100,9 +101,29 @@ describe('InterlinearNavContext', () => { expect(result.current.liveScrRef).toEqual({ book: 'GEN', chapterNum: 3, verseNum: 7 }); }); - it('normalizes a verse-0 reference that names a different chapter (a real chapter jump)', () => { + it('passes a same-chapter verse-0 echo through when it matches a fresh internal-nav marker', () => { + // Selecting a verse-0 superscription navigates the host to verse 0 of the chapter already shown, + // so the host's echo is shaped exactly like the spurious post-nav chapter echo. A fresh + // internal-nav marker for that verse-0 key marks it as our own deliberate move, so the stickiness + // exception lets it through to the superscription rather than holding the prior verse. + const { result, setRef, rerender } = renderNavMutable({ + book: 'GEN', + chapterNum: 3, + verseNum: 7, + }); + + act(() => result.current.navigate({ book: 'GEN', chapterNum: 3, verseNum: 0 }, 'internal')); + + act(() => setRef({ book: 'GEN', chapterNum: 3, verseNum: 0 })); + rerender(); + + expect(result.current.liveScrRef).toEqual({ book: 'GEN', chapterNum: 3, verseNum: 0 }); + }); + + it('passes a verse-0 reference for a different chapter through as a real chapter jump', () => { // A verse-0 reference for a chapter other than the one shown is a genuine chapter navigation, not - // an echo, so it still normalizes to that chapter's first verse. + // an echo, so it is honored as verse 0 (the loader maps it to verse 1 if that chapter has no + // verse-0 segment). const { result, setRef, rerender } = renderNavMutable({ book: 'GEN', chapterNum: 3, @@ -112,7 +133,7 @@ describe('InterlinearNavContext', () => { act(() => setRef({ book: 'GEN', chapterNum: 4, verseNum: 0 })); rerender(); - expect(result.current.liveScrRef).toEqual({ book: 'GEN', chapterNum: 4, verseNum: 1 }); + expect(result.current.liveScrRef).toEqual({ book: 'GEN', chapterNum: 4, verseNum: 0 }); }); describe('duplicate host deliveries', () => { @@ -136,10 +157,10 @@ describe('InterlinearNavContext', () => { expect(result.current.liveScrRef).toBe(liveBefore); }); - it('keeps liveScrRef identity when a verse-0 chapter jump is followed by its verse-1 form', () => { - // A chapter jump can arrive as a verse-0 reference (normalized to verse 1) followed by the - // explicit verse-1 reference. The raw references differ, but both normalize to the same - // verse, so the committed liveScrRef object must be reused for the second delivery. + it('treats a verse-0 chapter jump and its verse-1 form as two distinct deliveries', () => { + // A chapter jump can arrive as a verse-0 reference followed by an explicit verse-1 reference. + // Verse 0 and verse 1 are now distinct verses (verse 0 is the superscription), so the second + // delivery is a genuine move to verse 1 rather than a deduped duplicate. const { result, setRef, rerender } = renderNavMutable({ book: 'GEN', chapterNum: 3, @@ -149,12 +170,12 @@ describe('InterlinearNavContext', () => { act(() => setRef({ book: 'GEN', chapterNum: 4, verseNum: 0 })); rerender(); const liveAfterJump = result.current.liveScrRef; - expect(liveAfterJump).toEqual({ book: 'GEN', chapterNum: 4, verseNum: 1 }); + expect(liveAfterJump).toEqual({ book: 'GEN', chapterNum: 4, verseNum: 0 }); act(() => setRef({ book: 'GEN', chapterNum: 4, verseNum: 1 })); rerender(); - expect(result.current.liveScrRef).toBe(liveAfterJump); + expect(result.current.liveScrRef).toEqual({ book: 'GEN', chapterNum: 4, verseNum: 1 }); }); it('reuses the previous reference when a duplicate differs only in the verse segment string', () => { @@ -271,16 +292,20 @@ describe('InterlinearNavContext', () => { } }); - it('matches a verse-0 internal mark against the host-normalized verse-1 reference', () => { - // An internal navigation stamped at chapter granularity (verse 0) must still be consumable - // when the host echoes it back normalized to the chapter's first verse (verse 1) — the keys - // are computed through the same normalization so they cannot diverge on the verse-0 boundary. + it('keys a verse-0 internal mark to verse 0, distinct from verse 1', () => { + // Verse 0 (a superscription) is its own verse, so a verse-0 internal navigation is consumable + // only by a verse-0 reference — not by verse 1 of the same chapter. const { result } = renderNav( makeScrollGroupHook({ book: 'GEN', chapterNum: 1, verseNum: 1 }), ); act(() => result.current.navigate({ book: 'GEN', chapterNum: 3, verseNum: 0 }, 'internal')); expect(result.current.consumeInternalNav({ book: 'GEN', chapterNum: 3, verseNum: 1 })).toBe( + false, + ); + + act(() => result.current.navigate({ book: 'GEN', chapterNum: 3, verseNum: 0 }, 'internal')); + expect(result.current.consumeInternalNav({ book: 'GEN', chapterNum: 3, verseNum: 0 })).toBe( true, ); }); diff --git a/src/__tests__/components/Interlinearizer.test.tsx b/src/__tests__/components/Interlinearizer.test.tsx index c5405832..c558403f 100644 --- a/src/__tests__/components/Interlinearizer.test.tsx +++ b/src/__tests__/components/Interlinearizer.test.tsx @@ -313,6 +313,47 @@ const GEN_TWO_CHAPTER_BOOK: Book = { ), }; +/** GEN book whose chapter 1 opens with a verse-0 superscription segment before verse 1. */ +const GEN_SUPERSCRIPTION_BOOK: Book = { + id: 'GEN', + bookRef: 'GEN', + textVersion: 'v1', + segments: [ + { + id: 'GEN 1:0', + startRef: { book: 'GEN', chapter: 1, verse: 0 }, + endRef: { book: 'GEN', chapter: 1, verse: 0 }, + baselineText: 'A song.', + tokens: [ + { + ref: 'GEN 1:0:0', + surfaceText: 'A', + writingSystem: 'en', + type: 'word', + charStart: 0, + charEnd: 1, + }, + ], + }, + { + id: 'GEN 1:1', + startRef: { book: 'GEN', chapter: 1, verse: 1 }, + endRef: { book: 'GEN', chapter: 1, verse: 1 }, + baselineText: 'In the beginning.', + tokens: [ + { + ref: 'GEN 1:1:0', + surfaceText: 'In', + writingSystem: 'en', + type: 'word', + charStart: 0, + charEnd: 2, + }, + ], + }, + ], +}; + /** * Wraps an `` element in an {@link InterlinearNavProvider} so the component's * `useInterlinearNav` call resolves. `Interlinearizer` now writes the reference through the @@ -492,6 +533,70 @@ describe('Interlinearizer', () => { expect(mockNavigate).toHaveBeenCalledWith({ book: 'GEN', chapterNum: 1, verseNum: 2 }); }); + it('writes a verse-0 reference to the host when a verse-0 token is selected', () => { + // Selecting a superscription token navigates the host to verse 0 like any other verse; the + // internal-nav marker keeps the host's chapter echo from bouncing the view (the stickiness + // exception in InterlinearNavContext). Default scrRef is GEN 1:1, so this is a real verse change. + const mockNavigate = jest.fn(); + renderInterlinearizer({ book: GEN_SUPERSCRIPTION_BOOK, navigate: mockNavigate }); + + act(() => { + capturedSegmentViewPropsList[0].onSelect?.( + { book: 'GEN', chapter: 1, verse: 0 }, + 'GEN 1:0:0', + ); + }); + + expect(mockNavigate).toHaveBeenCalledWith({ book: 'GEN', chapterNum: 1, verseNum: 0 }); + }); + + it('writes a verse-0 reference to the host when a verse-0 token is focused from the strip', () => { + const mockNavigate = jest.fn(); + renderInterlinearizer({ + book: GEN_SUPERSCRIPTION_BOOK, + continuousScroll: true, + navigate: mockNavigate, + }); + + if (!capturedContinuousViewProps) + throw new Error('Expected ContinuousView to have been rendered'); + const { onFocusedTokenRefChange } = capturedContinuousViewProps; + + act(() => { + onFocusedTokenRefChange('GEN 1:0:0'); + }); + + expect(mockNavigate).toHaveBeenCalledWith({ book: 'GEN', chapterNum: 1, verseNum: 0 }); + expect(capturedContinuousViewProps.focusedTokenRef).toBe('GEN 1:0:0'); + }); + + it('moves the active-segment highlight to a verse-0 segment when its token is focused', () => { + renderInterlinearizer({ + book: GEN_SUPERSCRIPTION_BOOK, + scrRef: { book: 'GEN', chapterNum: 1, verseNum: 1 }, + }); + + // Active verse (1) is highlighted; the verse-0 superscription is not, yet. + const before = Object.fromEntries( + capturedSegmentViewPropsList.map((p) => [p.segment.id, p.isActive]), + ); + expect(before['GEN 1:0']).toBeFalsy(); + expect(before['GEN 1:1']).toBe(true); + + const { onSelect } = capturedSegmentViewPropsList[0]; + capturedSegmentViewPropsList = []; + act(() => { + onSelect({ book: 'GEN', chapter: 1, verse: 0 }, 'GEN 1:0:0'); + }); + + // Focusing the superscription's token moves the active highlight onto its segment. + const after = Object.fromEntries( + capturedSegmentViewPropsList.map((p) => [p.segment.id, p.isActive]), + ); + expect(after['GEN 1:0']).toBe(true); + expect(after['GEN 1:1']).toBeFalsy(); + }); + it('passes the clicked token through to ContinuousView as focusedTokenRef', () => { jest.useFakeTimers(); try { diff --git a/src/__tests__/components/InterlinearizerLoader.test.tsx b/src/__tests__/components/InterlinearizerLoader.test.tsx index b50c53ef..2eb6425a 100644 --- a/src/__tests__/components/InterlinearizerLoader.test.tsx +++ b/src/__tests__/components/InterlinearizerLoader.test.tsx @@ -469,7 +469,9 @@ describe('InterlinearizerLoader', () => { expect(screen.getByTestId('interlinearizer')).toBeInTheDocument(); }); - it('normalizes a chapter-level (verse 0) reference to verse 1 before passing it to Interlinearizer', async () => { + it('resolves a verse-0 reference to verse 1 when the book has no verse-0 segment', async () => { + // GEN_1_1_BOOK has only a GEN 1:1 segment, so a whole-chapter (verse 0) selection falls back to + // the chapter's first numbered verse rather than leaving nothing highlighted. await act(async () => { renderLoader({ useWebViewScrollGroupScrRef: makeScrollGroupHook({ @@ -487,6 +489,59 @@ describe('InterlinearizerLoader', () => { }); }); + it('keeps a verse-0 reference when the book has a verse-0 (superscription) segment', async () => { + const bookWithSuperscription: Book = { + id: 'PSA', + bookRef: 'PSA', + textVersion: 'v1', + segments: [ + { + id: 'PSA 3:0', + startRef: { book: 'PSA', chapter: 3, verse: 0 }, + endRef: { book: 'PSA', chapter: 3, verse: 0 }, + baselineText: 'A Psalm by David.', + tokens: [], + }, + ], + }; + mockBookData({ book: bookWithSuperscription }); + + await act(async () => { + renderLoader({ + useWebViewScrollGroupScrRef: makeScrollGroupHook({ + book: 'PSA', + chapterNum: 3, + verseNum: 0, + }), + }); + }); + + expect(capturedInterlinearizerProps?.scrRef).toEqual({ + book: 'PSA', + chapterNum: 3, + verseNum: 0, + }); + }); + + it('leaves a verse-0 reference untouched while the book is still loading', async () => { + // With no book loaded yet, the verse-0 resolution has nothing to consult, so the loader shows + // the loading placeholder and does not render the interlinearizer. + mockBookData({ book: undefined, isLoading: true }); + + await act(async () => { + renderLoader({ + useWebViewScrollGroupScrRef: makeScrollGroupHook({ + book: 'GEN', + chapterNum: 3, + verseNum: 0, + }), + }); + }); + + expect(screen.getByText('Loading…')).toBeInTheDocument(); + expect(screen.queryByTestId('interlinearizer')).not.toBeInTheDocument(); + }); + it('passes a verse-level reference through to Interlinearizer unchanged', async () => { await act(async () => { renderLoader({ diff --git a/src/__tests__/components/SegmentView.test.tsx b/src/__tests__/components/SegmentView.test.tsx index 597cd0d0..29a771e0 100644 --- a/src/__tests__/components/SegmentView.test.tsx +++ b/src/__tests__/components/SegmentView.test.tsx @@ -320,7 +320,7 @@ describe('SegmentView', () => { expect(container.firstChild).toHaveAttribute('aria-current', 'true'); }); - it('calls onSelect when clicked in baseline-text mode', async () => { + it('calls onSelect with the first word token when clicked in baseline-text mode', async () => { const handleSelect = jest.fn(); render( , @@ -329,8 +329,10 @@ describe('SegmentView', () => { await userEvent.click(screen.getByTestId('segment-container')); + // Passes the first word token so the segment gains focus (and the active highlight) on click, + // letting the parent both highlight the segment and navigate to its verse. expect(handleSelect).toHaveBeenCalledTimes(1); - expect(handleSelect).toHaveBeenCalledWith({ book: 'GEN', chapter: 1, verse: 1 }); + expect(handleSelect).toHaveBeenCalledWith({ book: 'GEN', chapter: 1, verse: 1 }, 'tok-0'); }); it('renders a free-translation input below the plain text in baseline-text mode', () => { diff --git a/src/__tests__/parsers/papi/bookTokenizer.test.ts b/src/__tests__/parsers/papi/bookTokenizer.test.ts index 3a4203d3..aa118606 100644 --- a/src/__tests__/parsers/papi/bookTokenizer.test.ts +++ b/src/__tests__/parsers/papi/bookTokenizer.test.ts @@ -50,6 +50,27 @@ describe('tokenizeBook', () => { expect(segments[0].endRef).toEqual({ book: 'GEN', chapter: 1, verse: 1 }); }); + it('builds a verse-0 segment from a verse-0 SID (Psalm superscription)', () => { + const raw: RawBook = { + bookCode: 'PSA', + writingSystem: 'en', + contentHash: 'abc123', + verses: [{ sid: 'PSA 3:0', text: 'A Psalm by David.' }], + }; + const { segments } = tokenizeBook(raw); + expect(segments).toHaveLength(1); + expect(segments[0].id).toBe('PSA 3:0'); + expect(segments[0].startRef).toEqual({ book: 'PSA', chapter: 3, verse: 0 }); + expect(segments[0].endRef).toEqual({ book: 'PSA', chapter: 3, verse: 0 }); + expect(segments[0].tokens.map((t) => t.surfaceText)).toEqual([ + 'A', + 'Psalm', + 'by', + 'David', + '.', + ]); + }); + it('upholds the charStart/charEnd invariant for every token', () => { const text = 'In the beginning, God created.'; const { segments } = tokenizeBook(makeRawBook([{ sid: 'GEN 1:1', text }])); diff --git a/src/__tests__/parsers/papi/usjBookExtractor.test.ts b/src/__tests__/parsers/papi/usjBookExtractor.test.ts index e3b031e6..c0c1e8ae 100644 --- a/src/__tests__/parsers/papi/usjBookExtractor.test.ts +++ b/src/__tests__/parsers/papi/usjBookExtractor.test.ts @@ -372,4 +372,126 @@ describe('extractBookFromUsj', () => { }; expect(() => extractBookFromUsj(usj, WS)).toThrow('duplicate verse SID "GEN 1:1"'); }); + + it('captures a d descriptive title before verse 1 as verse 0', () => { + const usj: UsjDocument = { + content: [ + { type: 'book', code: 'PSA', content: [] }, + { type: 'chapter', number: '3', sid: 'PSA 3' }, + { + type: 'para', + marker: 'd', + content: ['A Psalm by David, when he fled from Absalom his son.'], + }, + { + type: 'para', + marker: 'q1', + content: [ + { type: 'verse', sid: 'PSA 3:1' }, + 'Yahweh, how my adversaries have increased!', + ], + }, + ], + }; + const { verses } = extractBookFromUsj(usj, WS); + expect(verses).toHaveLength(2); + expect(verses[0]).toEqual({ + sid: 'PSA 3:0', + text: 'A Psalm by David, when he fled from Absalom his son.', + }); + expect(verses[1]).toEqual({ + sid: 'PSA 3:1', + text: 'Yahweh, how my adversaries have increased!', + }); + }); + + it('does not emit a verse 0 when only a section heading precedes verse 1', () => { + const usj: UsjDocument = { + content: [ + { type: 'book', code: 'GEN', content: [] }, + { type: 'chapter', number: '1', sid: 'GEN 1' }, + { type: 'para', marker: 's1', content: ['The Creation'] }, + { + type: 'para', + marker: 'p', + content: [{ type: 'verse', sid: 'GEN 1:1' }, 'In the beginning.'], + }, + ], + }; + const { verses } = extractBookFromUsj(usj, WS); + expect(verses).toHaveLength(1); + expect(verses[0]).toEqual({ sid: 'GEN 1:1', text: 'In the beginning.' }); + }); + + it('captures a verse 0 that ends the document with no following numbered verse', () => { + const usj: UsjDocument = { + content: [ + { type: 'book', code: 'PSA', content: [] }, + { type: 'chapter', number: '3', sid: 'PSA 3' }, + { type: 'para', marker: 'd', content: ['A Psalm by David.'] }, + ], + }; + const { verses } = extractBookFromUsj(usj, WS); + expect(verses).toEqual([{ sid: 'PSA 3:0', text: 'A Psalm by David.' }]); + }); + + it('does not open a verse 0 scope for a chapter node without a number', () => { + const usj: UsjDocument = { + content: [ + { type: 'book', code: 'GEN', content: [] }, + // A chapter node missing its number cannot name a verse-0 SID, so pre-verse content is + // dropped rather than captured. + { type: 'chapter', sid: 'GEN 1' }, + { type: 'para', marker: 'd', content: ['Stray title.'] }, + { + type: 'para', + marker: 'p', + content: [{ type: 'verse', sid: 'GEN 1:1' }, 'In the beginning.'], + }, + ], + }; + const { verses } = extractBookFromUsj(usj, WS); + expect(verses).toEqual([{ sid: 'GEN 1:1', text: 'In the beginning.' }]); + }); + + it('captures an explicit verse-0 marker as verse 0', () => { + const usj: UsjDocument = { + content: [ + { type: 'book', code: 'PSA', content: [] }, + { type: 'chapter', number: '3', sid: 'PSA 3' }, + { + type: 'para', + marker: 'q1', + content: [ + { type: 'verse', sid: 'PSA 3:0' }, + 'A Psalm by David.', + { type: 'verse', sid: 'PSA 3:1' }, + 'Yahweh, how my adversaries have increased!', + ], + }, + ], + }; + const { verses } = extractBookFromUsj(usj, WS); + expect(verses).toHaveLength(2); + expect(verses[0]).toEqual({ sid: 'PSA 3:0', text: 'A Psalm by David.' }); + expect(verses[1]).toEqual({ + sid: 'PSA 3:1', + text: 'Yahweh, how my adversaries have increased!', + }); + }); + + it('throws when an explicit verse-0 marker duplicates a non-empty synthetic verse 0', () => { + const usj: UsjDocument = { + content: [ + { type: 'book', code: 'PSA', content: [] }, + { type: 'chapter', number: '3', sid: 'PSA 3' }, + // A `d` descriptive title opens a non-empty synthetic verse 0 (PSA 3:0)... + { type: 'para', marker: 'd', content: ['A Psalm by David.'] }, + // ...then an explicit verse-0 marker with the same SID must be rejected as a duplicate + // rather than silently emitting two PSA 3:0 verses. + { type: 'para', marker: 'q1', content: [{ type: 'verse', sid: 'PSA 3:0' }, 'Yahweh.'] }, + ], + }; + expect(() => extractBookFromUsj(usj, WS)).toThrow('duplicate verse SID "PSA 3:0"'); + }); }); diff --git a/src/components/InterlinearNavContext.tsx b/src/components/InterlinearNavContext.tsx index 10d9228f..379a1b13 100644 --- a/src/components/InterlinearNavContext.tsx +++ b/src/components/InterlinearNavContext.tsx @@ -44,19 +44,6 @@ export type FadePhase = 'idle' | 'out' | 'in'; */ export type NavOrigin = 'internal' | 'external'; -/** - * Normalizes a chapter-level reference (verse 0, as the scripture controls emit for a chapter - * selection) to the chapter's first verse. The host echoes navigations back through this same - * mapping ({@link InterlinearNavProvider}'s `liveScrRef`), so applying it everywhere a reference is - * keyed or compared keeps the stamped-at-click identity and the delivered identity in lockstep. - * - * @param ref - The reference to normalize. - * @returns `ref` unchanged, or a copy with `verseNum` 0 mapped to 1. - */ -export function normalizeScrRef(ref: SerializedVerseRef): SerializedVerseRef { - return ref.verseNum === 0 ? { ...ref, verseNum: 1 } : ref; -} - /** * Compares the verse coordinate of two serialized references: book, chapter, and verse number. Used * to detect the host's duplicate deliveries — the scripture picker fires each external navigation @@ -76,16 +63,15 @@ function areScrRefsEqual(a: SerializedVerseRef, b: SerializedVerseRef): boolean /** * Builds a stable string key identifying the verse a reference names. Used to match an internal * navigation against the `liveScrRef` the host later delivers, so the segment window can tell an - * internally-originated change apart from an external one. Keys the {@link normalizeScrRef}-mapped - * reference so a stamp and the later delivered (already-normalized) reference can never diverge on - * the verse-0 boundary. + * internally-originated change apart from an external one. Verse 0 is keyed verbatim (as its own + * verse) so a deliberate navigation to a chapter's verse-0 superscription is distinct from verse + * 1. * * @param ref - The scripture reference to key. * @returns A `book:chapter:verse` string uniquely identifying the verse. */ export function verseKey(ref: SerializedVerseRef): string { - const normalized = normalizeScrRef(ref); - return `${normalized.book}:${normalized.chapterNum}:${normalized.verseNum}`; + return `${ref.book}:${ref.chapterNum}:${ref.verseNum}`; } /** @@ -127,12 +113,20 @@ export interface InterlinearNav { */ rawScrRef: SerializedVerseRef; /** - * `rawScrRef` with a chapter-level (verse 0) reference normalized to verse 1. Selecting a chapter - * in the scripture controls yields `verseNum: 0`, which names the chapter rather than a verse — - * no segment has verse 0, so the active-verse lookup, the `isActive` highlight, and the - * continuous strip's focus resolution would all miss, leaving the list parked on the book's first - * phrase with nothing highlighted. Mapping verse 0 to the chapter's first verse makes every - * downstream consumer resolve the intended verse. + * The active reference, equal to `rawScrRef` except that a verse-0 reference naming the chapter + * already shown is held sticky on the current verse — unless it matches a fresh internal-nav + * marker (the extension's own move to that chapter's superscription), which passes through. A + * verse-0 reference is otherwise passed through verbatim: when it names a chapter with a verse-0 + * superscription segment, that segment becomes the active verse. Whether a given chapter actually + * has verse-0 content is unknown here (the book is not loaded at this layer), so the loader + * resolves a verse-0 reference with no matching segment back to the chapter's first numbered + * verse before rendering. + * + * The sticky exception exists because, after a verse navigation, the host re-broadcasts the + * chapter as a separate `verseNum: 0` reference (an echo of the current location, not a real + * move); treating that as a jump to verse 0 would yank the view off the verse the user is on. The + * marker carve-out distinguishes that spurious echo from a deliberate verse-0 navigation the + * extension itself just made (which is shaped identically). */ liveScrRef: SerializedVerseRef; /** @@ -235,28 +229,48 @@ export function InterlinearNavProvider({ * The last committed {@link liveScrRef}, mirrored so the verse-0 stickiness below can compare the * incoming `rawScrRef` against the verse currently shown. */ - const liveScrRefRef = useRef(normalizeScrRef(rawScrRef)); + const liveScrRefRef = useRef(rawScrRef); + + /** + * Verse keys of internal navigations still awaiting their host round-trip, each mapped to its + * `Date.now()` stamp. `navigate(ref, 'internal')` records `verseKey(ref)`; `consumeInternalNav` + * removes it on match. Keyed (not a single value) so that rapid successive clicks both stay + * pending and neither host delivery is misread as external. The stamp gives each marker a TTL + * ({@link INTERNAL_NAV_TTL_MS} — see its doc for why stranded markers must expire), honored by ALL + * readers: `consumeInternalNav` (which also evicts expired markers), the verse-0 stickiness + * exception below, and the render-phase mid-reveal guard (both pure reads — no eviction during + * render). + */ + const pendingInternalNavRef = useRef>(new Map()); // After a verse navigation the host re-broadcasts the *chapter* to the scroll group as a separate - // `verseNum: 0` reference (an echo of the current location, not a real move). Normalizing that to - // verse 1 unconditionally would read as a fresh navigation to the chapter's first verse, fading and - // recentering the views off the verse the user is actually on. So a verse-0 reference that names the - // book+chapter already shown is treated as sticky: keep the current `liveScrRef` (its real verse) - // rather than snapping to verse 1. A verse-0 reference for a *different* chapter is a genuine - // chapter jump and normalizes to verse 1 as before. When two *different* deliveries normalize to - // the same verse (a verse-0 chapter jump followed by its verse-1 form), the previously committed - // object is reused so the second delivery never reads as a fresh navigation downstream. + // `verseNum: 0` reference (an echo of the current location, not a real move). Treating that as a + // jump to verse 0 would yank the views off the verse the user is actually on, so a verse-0 + // reference that names the book+chapter already shown is held sticky: keep the current `liveScrRef` + // (its real verse). + // + // The exception is a verse-0 reference the extension itself just navigated to — selecting a + // chapter's verse-0 superscription segment writes `verseNum: 0` for the chapter already shown, + // which is shaped exactly like the spurious echo. A fresh internal-nav marker for that verse-0 key + // distinguishes the two: when one exists, this is our own deliberate move to the superscription, so + // it passes through (and `consumeInternalNav` clears the marker downstream). A pure read here — no + // eviction during render, matching the render-phase mid-reveal guard. + // + // Every other reference — including a verse-0 reference for a *different* chapter (a genuine chapter + // jump, which the loader resolves to the verse-0 superscription when one exists, else to verse 1) — + // passes through verbatim. The duplicate-delivery guard reuses the previously committed object when + // a re-send is value-equal, so a duplicate never reads as a fresh navigation. const liveScrRef = useMemo(() => { const prev = liveScrRefRef.current; if ( rawScrRef.verseNum === 0 && rawScrRef.book === prev.book && - rawScrRef.chapterNum === prev.chapterNum + rawScrRef.chapterNum === prev.chapterNum && + !isInternalNavMarkerFresh(pendingInternalNavRef.current.get(verseKey(rawScrRef))) ) { return prev; } - const normalized = normalizeScrRef(rawScrRef); - return areScrRefsEqual(normalized, prev) ? prev : normalized; + return areScrRefsEqual(rawScrRef, prev) ? prev : rawScrRef; }, [rawScrRef]); /** * The {@link liveScrRef} committed on the previous render, captured before the mirror update below @@ -266,19 +280,12 @@ export function InterlinearNavProvider({ const prevLiveScrRef = liveScrRefRef.current; liveScrRefRef.current = liveScrRef; - /** - * Verse keys of internal navigations still awaiting their host round-trip, each mapped to its - * `Date.now()` stamp. `navigate(ref, 'internal')` records `verseKey(ref)`; `consumeInternalNav` - * removes it on match. Keyed (not a single value) so that rapid successive clicks both stay - * pending and neither host delivery is misread as external. The stamp gives each marker a TTL - * ({@link INTERNAL_NAV_TTL_MS} — see its doc for why stranded markers must expire), honored by - * BOTH readers: `consumeInternalNav` (which also evicts expired markers) and the render-phase - * mid-reveal guard (a pure read — no eviction during render). - */ - const pendingInternalNavRef = useRef>(new Map()); - const navigate = useCallback( (newScrRef: SerializedVerseRef, origin: NavOrigin = 'external') => { + // Invariant: a marker write is ALWAYS paired with the `setScrRef` below. The `liveScrRef` memo + // reads this marker but lists only `[rawScrRef]` as a dependency, relying on every marker write + // also pushing a new ref through the host (which re-runs the memo). Never set a marker here + // without the accompanying `setScrRef`, or the memo could read a stale marker. if (origin === 'internal') pendingInternalNavRef.current.set(verseKey(newScrRef), Date.now()); setScrRef(newScrRef); }, diff --git a/src/components/Interlinearizer.tsx b/src/components/Interlinearizer.tsx index 4fd1106a..d1281246 100644 --- a/src/components/Interlinearizer.tsx +++ b/src/components/Interlinearizer.tsx @@ -35,10 +35,11 @@ type InterlinearizerProps = Readonly<{ /** When true, the horizontal token strip is shown above the segment list. */ continuousScroll: boolean; /** - * Current scripture reference used to highlight the active verse. Must carry a normalized verse - * number (>= 1): production refs flow through `normalizeScrRef` in `InterlinearNavContext`, which - * maps a verse-0 (chapter heading) selection to verse 1 before it reaches this component. A raw - * verse-0 ref would match no segment, leaving focus unseeded and nothing highlighted. + * Current scripture reference used to highlight the active verse. Always names a verse that has a + * segment in `book`: the loader keeps `verseNum: 0` when the active chapter has a verse-0 + * superscription segment (so the superscription becomes the active verse) and otherwise resolves + * a whole-chapter (verse 0) selection to the chapter's first numbered verse. So this is normally + * verse >= 1, and is verse 0 only when a matching verse-0 segment exists. */ scrRef: SerializedVerseRef; /** @@ -211,6 +212,10 @@ function InterlinearizerInner({ * reference. (The loader's `viewScrRef` freeze normally keeps the two in sync, so this guards a * transient.) * + * A verse-0 segment (a chapter superscription) navigates like any other: the write records an + * internal-nav marker so the host's chapter echo is recognized as our own move rather than + * bouncing the view back. (See the verse-0 stickiness exception in `InterlinearNavContext`.) + * * @param tokenRef - The word-token ref to focus. */ const focusToken = useCallback( @@ -231,7 +236,9 @@ function InterlinearizerInner({ /** * Updates the active scripture reference (when the verse actually changed) and, when a specific * token was clicked, focuses that token. Skips the write to PAPI when the clicked verse matches - * the current one, avoiding a gratuitous echo round-trip. + * the current one, avoiding a gratuitous echo round-trip. A verse-0 segment (a chapter + * superscription) writes like any other verse — the internal-nav marker keeps the host's chapter + * echo from bouncing the view (see the verse-0 stickiness exception in `InterlinearNavContext`). * * @param ref - The verse coordinate that was selected. * @param tokenRef - The token that was clicked; omitted when the whole segment was selected. @@ -312,8 +319,8 @@ function InterlinearizerInner({ * @param props - Component props * @param props.book - Book data used by the continuous view and segment window * @param props.continuousScroll - Whether the continuous scroll view is shown - * @param props.scrRef - Current scripture reference; must be normalized (verse >= 1, see - * {@link InterlinearizerProps}). + * @param props.scrRef - Current scripture reference; always names a verse with a segment (verse 0 + * only when a superscription segment exists, see {@link InterlinearizerProps}). * @param props.initialAnalysis - Seed analysis data for the store; not reactive after mount * @param props.analysisLanguage - BCP 47 tag for gloss read/write * @param props.onSaveAnalysis - Called after each gloss write with the updated `TextAnalysis` diff --git a/src/components/InterlinearizerLoader.tsx b/src/components/InterlinearizerLoader.tsx index dbe140cf..8db29fd0 100644 --- a/src/components/InterlinearizerLoader.tsx +++ b/src/components/InterlinearizerLoader.tsx @@ -21,6 +21,7 @@ import { WipeModal, type WipeScope } from './modals/WipeModal'; import ScriptureNavControls from './controls/ScriptureNavControls'; import { InterlinearNavProvider, useInterlinearNav } from './InterlinearNavContext'; import { RECENTER_FADE_TRANSITION_STYLE } from './recenter-fade'; +import { isSameVerse } from '../utils/verse-ref'; /** Host-injected callback to update this WebView's definition (used to toggle the tab title). */ type UpdateWebViewDefinition = WebViewProps['updateWebViewDefinition']; @@ -235,6 +236,18 @@ function InterlinearizerLoaderInner({ scrRef, }); + // The active reference handed to the interlinearizer. The host emits `verseNum: 0` both for a + // chapter's verse-0 superscription (which has its own segment) and for a plain whole-chapter + // selection (which does not). Keep verse 0 when the loaded book actually has a verse-0 segment for + // that chapter — so a Psalm superscription becomes the active verse — and otherwise fall back to + // the chapter's first numbered verse, so an ordinary chapter selection still lands on verse 1 + // rather than leaving nothing highlighted. Non-verse-0 references pass through unchanged. + const activeScrRef = useMemo(() => { + if (scrRef.verseNum !== 0 || !book) return scrRef; + const hasVerseZero = book.segments.some((segment) => isSameVerse(segment.startRef, scrRef)); + return hasVerseZero ? scrRef : { ...scrRef, verseNum: 1 }; + }, [scrRef, book]); + const hasError = !!bookError || !!tokenizeError; const isSettingLoading = isContinuousScrollLoading || @@ -472,7 +485,7 @@ function InterlinearizerLoaderInner({ key={`${draftVersion}:${book.bookRef}`} book={book} continuousScroll={continuousScroll} - scrRef={scrRef} + scrRef={activeScrRef} analysisLanguage={analysisLanguage} initialAnalysis={draft?.analysis} onSaveAnalysis={autosaveAnalysis} diff --git a/src/components/SegmentListView.tsx b/src/components/SegmentListView.tsx index d6e73aed..3786581e 100644 --- a/src/components/SegmentListView.tsx +++ b/src/components/SegmentListView.tsx @@ -181,6 +181,16 @@ export default function SegmentListView({ recenterOnActive(); }, [continuousScroll, recenterOnActive]); + // Segment that wears the active highlight. It follows the focused token's segment so the highlight + // lands on the segment whose token is focused — including a verse-0 superscription. Normal + // navigation keeps the focused token inside the active verse, so this resolves to the same segment + // as the `displayScrRef` verse; it can diverge briefly when a focus move and the host echo it + // triggers are not yet reconciled. Falls back to the active verse when nothing is focused (e.g. the + // active verse has no word token). + const activeSegmentId = displayFocusedTokenRef + ? tokenSegmentMap.get(displayFocusedTokenRef) + : undefined; + return (
void; /** The segment to render. */ @@ -93,9 +95,9 @@ type SegmentViewProps = Readonly<{ * focused state; only meaningful in `token-chip` mode. * @param props.isActive - Whether this segment is the currently selected verse * @param props.onSelect - Required callback invoked when the segment or one of its word tokens is - * interacted with. In `baseline-text` mode the whole segment is clickable and `tokenRef` is - * omitted. In `token-chip` mode only word tokens trigger this callback and `tokenRef` is always - * provided. + * interacted with. In `baseline-text` mode the whole segment is clickable and selecting it passes + * the segment's first word token; in `token-chip` mode only word tokens trigger this callback + * with the clicked token. `tokenRef` is omitted only when the segment has no word token. * @param props.segment - The segment to render * @param props.phraseMode - Current phrase-interaction mode * @param props.setPhraseMode - Setter for `phraseMode` @@ -374,18 +376,21 @@ export function SegmentView({ }, [firstWordTokenRef, onSelect, ref]); /** - * Selects this segment when its baseline-text body is clicked. Clicks that originate inside the - * free-translation input are ignored: that input fires its own {@link handleFreeTranslationFocus} - * on focus, so letting the container also fire would double-select the verse. + * Selects this segment when its baseline-text body is clicked, focusing its first word token so + * the segment gains focus (and the active highlight) even when it is verse 0 — a superscription + * that cannot be written back to the host as the active verse, and so would otherwise never + * become active from a bare-ref select. Clicks that originate inside the free-translation input + * are ignored: that input fires its own {@link handleFreeTranslationFocus} on focus, so letting + * the container also fire would double-select the verse. * * @param event - The click event on the baseline-text container. */ const handleBaselineClick = useCallback( (event: MouseEvent) => { if (event.target instanceof Element && event.target.closest('input')) return; - onSelect(ref); + onSelect(ref, firstWordTokenRef); }, - [onSelect, ref], + [firstWordTokenRef, onSelect, ref], ); // Measure phrase boxes inside this segment and compute arcs. Disabled in baseline-text mode, diff --git a/src/parsers/papi/usjBookExtractor.ts b/src/parsers/papi/usjBookExtractor.ts index 0340a8a8..bf2f7ab0 100644 --- a/src/parsers/papi/usjBookExtractor.ts +++ b/src/parsers/papi/usjBookExtractor.ts @@ -64,9 +64,13 @@ export interface UsjDocument { // --------------------------------------------------------------------------- /** - * Para markers whose content is not part of the verse baseline text (headings, titles, spacing, - * speaker IDs, acrostic headings, etc.). Verse-content para markers (p, m, pi, q*, etc.) are absent - * from this set and have their text accumulated as usual. + * Para markers whose content is not part of the verse baseline text (headings, spacing, speaker + * IDs, acrostic headings, etc.). Verse-content para markers (p, m, pi, q*, etc.) are absent from + * this set and have their text accumulated as usual. + * + * The descriptive-title marker `d` (a Psalm superscription, e.g. "A Psalm of David") is + * deliberately absent: its text is genuine scripture that the source omits a verse marker for, so + * it is accumulated as the chapter's verse-0 content (see {@link handleChapterNode}). */ const HEADING_PARA_MARKERS = new Set([ // Major section headings and reference ranges @@ -75,7 +79,7 @@ const HEADING_PARA_MARKERS = new Set([ 'ms2', 'ms3', 'mr', - // Section headings, reference ranges, and descriptive titles + // Section headings and reference ranges 's', 's1', 's2', @@ -83,7 +87,6 @@ const HEADING_PARA_MARKERS = new Set([ 's4', 'sr', 'r', - 'd', // Speaker, acrostic heading, blank lines 'sp', 'qa', @@ -110,10 +113,41 @@ interface TraversalState { seenVerseIds: Set; /** The verse currently being accumulated; `undefined` when outside a verse scope. */ currentVerse: { sid: string; text: string } | undefined; + /** + * `true` when `currentVerse` is the synthetic verse-0 scope opened at a chapter boundary (see + * {@link handleChapterNode}). A synthetic verse-0 is emitted only when it accumulates text, so + * chapters with no superscription don't produce an empty verse-0; real verse markers are always + * emitted even when empty. + */ + currentVerseIsSynthetic: boolean; /** Completed verses in document order. */ verses: RawVerse[]; } +/** + * Closes the verse currently being accumulated (if any): trims trailing whitespace and pushes it to + * the completed-verses list, then clears the open-verse state. A synthetic verse-0 scope is dropped + * rather than pushed when it accumulated no text, so chapters without a superscription emit no + * spurious empty verse-0 segment. Real verse markers are pushed even when empty. + * + * Every emitted verse's SID is recorded in `seenVerseIds`. Real verse markers are already recorded + * when opened (see {@link handleVerseNode}), so this matters for synthetic verse-0 scopes: it lets a + * later explicit verse marker with the same SID be rejected as a duplicate rather than silently + * emitting two verses with the same ID. + * + * @param state - Shared traversal state updated in place. + */ +function closeCurrentVerse(state: TraversalState): void { + if (state.currentVerse === undefined) return; + state.currentVerse.text = state.currentVerse.text.trimEnd(); + if (!(state.currentVerseIsSynthetic && state.currentVerse.text.length === 0)) { + state.verses.push(state.currentVerse); + state.seenVerseIds.add(state.currentVerse.sid); + } + state.currentVerse = undefined; + state.currentVerseIsSynthetic = false; +} + /** * Captures the book code from a `book` node, then recurses into its content. * @@ -126,17 +160,24 @@ function handleBookNode(node: UsjNode, state: TraversalState): void { } /** - * Closes the current open verse (if any) when a `chapter` node is encountered, then recurses into - * the chapter's content to pick up verses inside it. + * Closes the current open verse (if any) when a `chapter` node is encountered, then opens a + * synthetic verse-0 scope for the new chapter before recursing into its content. + * + * The verse-0 scope captures content that precedes the chapter's first `verse` marker — chiefly the + * `d` descriptive title (a Psalm superscription). Heading paragraphs (see + * {@link HEADING_PARA_MARKERS}) are still skipped while it is open, so a chapter with only a section + * heading before verse 1 accumulates nothing and the empty scope is dropped on close. The scope's + * SID is `" :0"`, parsed downstream into a verse-0 `Segment`. When the chapter node + * carries no `number` the scope cannot be named, so it is not opened. * - * @param node - The `chapter` USJ node. + * @param node - The `chapter` USJ node; `node.number` is the chapter number (e.g. `"3"`). * @param state - Shared traversal state updated in place. */ function handleChapterNode(node: UsjNode, state: TraversalState): void { - if (state.currentVerse !== undefined) { - state.currentVerse.text = state.currentVerse.text.trimEnd(); - state.verses.push(state.currentVerse); - state.currentVerse = undefined; + closeCurrentVerse(state); + if (node.number) { + state.currentVerse = { sid: `${state.bookCode} ${node.number}:0`, text: '' }; + state.currentVerseIsSynthetic = true; } if (node.content) traverse(node.content, state); } @@ -150,10 +191,7 @@ function handleChapterNode(node: UsjNode, state: TraversalState): void { * @throws {SyntaxError} If the `verse` SID has already been seen (duplicate verse SID). */ function handleVerseNode(node: UsjNode, state: TraversalState): void { - if (state.currentVerse !== undefined) { - state.currentVerse.text = state.currentVerse.text.trimEnd(); - state.verses.push(state.currentVerse); - } + closeCurrentVerse(state); if (!node.sid) throw new SyntaxError('Invalid USJ: verse marker missing required sid attribute'); if (state.seenVerseIds.has(node.sid)) throw new SyntaxError(`Invalid USJ: duplicate verse SID "${node.sid}"`); @@ -261,6 +299,10 @@ function fnv1a32(s: string): string { * verse scope are accumulated into `RawVerse.text`; `note` nodes are skipped entirely. Verse * markers with no following text produce an empty `RawVerse` (`text: ""`). * + * Content preceding a chapter's first `verse` marker — chiefly a `d` descriptive title (Psalm + * superscription) — is captured as a synthetic verse-0 `RawVerse` with SID `" :0"`, + * but only when it has text; see {@link handleChapterNode}. + * * @param usj - USJ document returned by `useProjectData('platformScripture.USJ_Book', ...)`. * @param writingSystem - BCP 47 tag for the baseline, from `platform.languageTag`. * @returns A `RawBook` with `bookCode`, `writingSystem`, `contentHash`, and `verses` populated. @@ -272,15 +314,13 @@ export function extractBookFromUsj(usj: UsjDocument, writingSystem: string): Raw bookCode: '', seenVerseIds: new Set(), currentVerse: undefined, + currentVerseIsSynthetic: false, verses: [], }; traverse(usj.content, state); - if (state.currentVerse !== undefined) { - state.currentVerse.text = state.currentVerse.text.trimEnd(); - state.verses.push(state.currentVerse); - } + closeCurrentVerse(state); if (!state.bookCode) throw new SyntaxError('Invalid USJ: no book marker with a code attribute found'); diff --git a/src/types/interlinearizer.d.ts b/src/types/interlinearizer.d.ts index 8ed9194e..52dc08cb 100644 --- a/src/types/interlinearizer.d.ts +++ b/src/types/interlinearizer.d.ts @@ -335,7 +335,11 @@ declare module 'interlinearizer' { book: string; /** 1-based chapter number. */ chapter: number; - /** 1-based verse number. */ + /** + * Verse number. Normally 1-based; `0` denotes a chapter's verse-0 content (a Psalm + * superscription / descriptive title that precedes verse 1), which the parser captures as its + * own segment. + */ verse: number; /** * Zero-based character offset within the verse's baseline text. Absent when the reference is