diff --git a/contributions/localizedStrings.json b/contributions/localizedStrings.json index 37c22347b..8916b195b 100644 --- a/contributions/localizedStrings.json +++ b/contributions/localizedStrings.json @@ -101,6 +101,9 @@ "%interlinearizer_boundaryControl_merge%": "Join these two segments", "%interlinearizer_boundaryControl_mergeAltHint%": "Join these two segments. Hold {key} and click between words to split.", "%interlinearizer_boundaryControl_split%": "Split segment here", + "%interlinearizer_segmentation_lostBoundaries%": "The source text changed, so {count} of your segment boundary changes no longer fit it and aren't applied.", + "%interlinearizer_segmentation_lostBoundaries_one%": "The source text changed, so one of your segment boundary changes no longer fits it and isn't applied.", + "%interlinearizer_segmentation_lostBoundaries_dismiss%": "Dismiss this warning", "%interlinearizer_phraseBox_glossLabel%": "Phrase gloss", "%interlinearizer_phraseBox_edit%": "Edit phrase", "%interlinearizer_phraseBox_unlink%": "Unlink phrase", diff --git a/src/__tests__/components/InterlinearizerLoader.test.tsx b/src/__tests__/components/InterlinearizerLoader.test.tsx index 91c455a04..9f52a7fe7 100644 --- a/src/__tests__/components/InterlinearizerLoader.test.tsx +++ b/src/__tests__/components/InterlinearizerLoader.test.tsx @@ -13,7 +13,9 @@ import { useGlossDispatch } from '../../components/AnalysisStore'; import InterlinearizerLoader from '../../components/InterlinearizerLoader'; import { RECENTER_FADE_MS } from '../../components/recenter-fade'; import useInterlinearizerBookData from '../../hooks/useInterlinearizerBookData'; +import useLostBoundaryDismissal from '../../hooks/useLostBoundaryDismissal'; import useOptimisticBooleanSetting from '../../hooks/useOptimisticBooleanSetting'; +import type { OpenableProject } from '../../hooks/useDraftProject'; import { emptyAnalysis, emptyDraft } from '../../types/empty-factories'; import { PT9_MANIFEST_TIMEOUT_MS } from '../../utils/pt9-manifest'; import type { PhraseMode } from '../../types/phrase-mode'; @@ -33,6 +35,7 @@ import { import { mockKeyAsValueLocalizedStrings } from './test-helpers'; jest.mock('../../hooks/useInterlinearizerBookData'); +jest.mock('../../hooks/useLostBoundaryDismissal'); jest.mock('../../hooks/useOptimisticBooleanSetting'); jest.mock('../../components/controls/ViewOptionsDropdown', () => ({ @@ -277,6 +280,15 @@ const STUB_IMPORT_PROJECT: MockProject = { pt9Import: { fileHashes: { 'Lexicon.xml': 'aaaa1111' }, importedAt: '2026-08-01T00:00:00Z' }, }; +/** + * The project the stub picker's "Open project" button loads into the draft. Mutable so a test can + * choose the boundaries the opened project carries. + */ +let openableProjectForStub: OpenableProject = { + analysis: emptyAnalysis(), + analysisLanguages: ['en'], +}; + jest.mock('../../components/modals/ProjectModals', () => ({ __esModule: true, /** @@ -293,6 +305,7 @@ jest.mock('../../components/modals/ProjectModals', () => ({ activeProject, defaultAnalysisLanguage, hasUnsavedWork, + loadFromProject, onImportPt9, onOpenImport, openRequest, @@ -304,7 +317,7 @@ jest.mock('../../components/modals/ProjectModals', () => ({ defaultAnalysisLanguage?: string; hasUnsavedWork: boolean; getDraftSnapshot: () => DraftProject | undefined; - loadFromProject: (project: unknown) => void; + loadFromProject: (project: OpenableProject) => void; markSynced: () => void; onImportPt9: () => void; onOpenImport: (project: MockProject) => void; @@ -366,6 +379,16 @@ jest.mock('../../components/modals/ProjectModals', () => ({ > View info + )} {modal === 'create' && ( @@ -535,13 +558,26 @@ function mockSettings( }); } +/** + * Stubs {@link useLostBoundaryDismissal} to report the given lost anchors as undismissed. + * + * @returns The dismiss callback the stub hands the banner, so the wiring can be asserted on. + */ +function mockLostBoundaries(undismissedLostBoundaries: readonly string[]): jest.Mock { + const onDismiss = jest.fn(); + jest.mocked(useLostBoundaryDismissal).mockReturnValue({ undismissedLostBoundaries, onDismiss }); + return onDismiss; +} + describe('InterlinearizerLoader', () => { beforeEach(() => { capturedInterlinearizerProps = undefined; capturedStoreProps = undefined; interlinearizerMountCount = 0; + openableProjectForStub = { analysis: emptyAnalysis(), analysisLanguages: ['en'] }; mockBookData(); mockOptimisticSetting(); + mockLostBoundaries([]); // The loader's draft hook calls `interlinearizer.getDraft` on mount; default to a valid empty // draft so the editor renders. Individual tests override with mockResolvedValueOnce. mockSendCommand.mockResolvedValue(JSON.stringify(emptyDraft(testProjectId))); @@ -1143,6 +1179,22 @@ describe('InterlinearizerLoader', () => { expect(screen.getByTestId('pt9-copy-button')).toBeInTheDocument(); }); + it('holds the banner back until the localized strings resolve', async () => { + mockImportCommands(); + jest + .mocked(useLocalizedStrings) + .mockImplementation((keys: readonly string[]) => [ + Object.fromEntries(keys.map((k) => [k, k])), + true, + ]); + + await act(async () => + renderLoader({ useWebViewState: makeWebViewState({ activeProject: STUB_IMPORT_PROJECT }) }), + ); + + expect(screen.queryByTestId('pt9-import-banner')).not.toBeInTheDocument(); + }); + it('silences Save, Save As, and Wipe while an import is open', async () => { mockImportCommands(); await renderImportView(); @@ -2541,6 +2593,115 @@ describe('InterlinearizerLoader', () => { }); }); + describe('lost segment boundaries', () => { + /** + * Renders the loader on a loaded book, parked on GEN unless a `scrRef` elsewhere asks for a + * cross-book swap. + */ + async function renderOnLoadedBook(scrRef?: SerializedVerseRef): Promise { + mockBookData({ book: GEN_1_1_BOOK }); + await act(async () => { + renderLoader({ useWebViewScrollGroupScrRef: makeScrollGroupHook(scrRef) }); + }); + } + + it('shows the banner when the hook reports an undismissed loss', async () => { + mockLostBoundaries(['GEN 1:9:0']); + + await renderOnLoadedBook(); + + expect(screen.getByTestId('lost-boundaries-banner')).toBeInTheDocument(); + }); + + it('does not show the banner when the hook reports no loss', async () => { + await renderOnLoadedBook(); + + expect(screen.queryByTestId('lost-boundaries-banner')).not.toBeInTheDocument(); + }); + + it('holds the banner back until the localized strings resolve', async () => { + jest + .mocked(useLocalizedStrings) + .mockImplementation((keys: readonly string[]) => [ + Object.fromEntries(keys.map((k) => [k, k])), + true, + ]); + mockLostBoundaries(['GEN 1:9:0']); + + await renderOnLoadedBook(); + + expect(screen.queryByTestId('lost-boundaries-banner')).not.toBeInTheDocument(); + }); + + it('holds the banner back during a cross-book swap', async () => { + // A swap is mid-flight when scrRef already names EXO but the loaded book is still GEN, whose + // anchors are the ones lost. + mockLostBoundaries(['GEN 1:9:0']); + + await renderOnLoadedBook({ book: 'EXO', chapterNum: 1, verseNum: 1 }); + + expect(screen.queryByTestId('lost-boundaries-banner')).not.toBeInTheDocument(); + }); + + it('interpolates the lost-anchor count into the banner text', async () => { + jest + .mocked(useLocalizedStrings) + .mockImplementation((keys: readonly string[]) => [ + Object.fromEntries( + keys.map((k) => [ + k, + k === '%interlinearizer_segmentation_lostBoundaries%' ? '{count} boundaries lost' : k, + ]), + ), + false, + ]); + mockLostBoundaries(['GEN 1:9:0', 'GEN 1:1:99']); + + await renderOnLoadedBook(); + + expect(screen.getByTestId('lost-boundaries-banner')).toHaveTextContent('2 boundaries lost'); + }); + + it('uses the singular string for a single lost anchor', async () => { + jest + .mocked(useLocalizedStrings) + .mockImplementation((keys: readonly string[]) => [ + Object.fromEntries( + keys.map((k) => [ + k, + k === '%interlinearizer_segmentation_lostBoundaries_one%' ? 'just the one' : k, + ]), + ), + false, + ]); + mockLostBoundaries(['GEN 1:9:0']); + + await renderOnLoadedBook(); + + expect(screen.getByTestId('lost-boundaries-banner')).toHaveTextContent('just the one'); + }); + + it('dismisses through the hook when the banner close button is clicked', async () => { + const onDismiss = mockLostBoundaries(['GEN 1:9:0']); + await renderOnLoadedBook(); + + await userEvent.click(screen.getByTestId('lost-boundaries-dismiss')); + + expect(onDismiss).toHaveBeenCalledTimes(1); + }); + + it('leaves the draft untouched when the banner is dismissed', async () => { + mockLostBoundaries(['GEN 1:9:0']); + await renderOnLoadedBook(); + + await userEvent.click(screen.getByTestId('lost-boundaries-dismiss')); + + // The banner is read-only: the anchors themselves stay for a source that reverts. + const saves = mockSendCommand.mock.calls.filter(([c]) => c === 'interlinearizer.saveDraft'); + expect(saves).toHaveLength(0); + }); + }); + describe('save command', () => { it('saves the draft analysis to the active project when Save is clicked with an active project', async () => { const draftAnalysis = emptyAnalysis(); @@ -3369,6 +3530,7 @@ describe('analysis store lifetime', () => { interlinearizerMountCount = 0; mockBookData(); mockOptimisticSetting(); + mockLostBoundaries([]); mockSendCommand.mockResolvedValue(JSON.stringify(emptyDraft(testProjectId))); jest .mocked(useData) diff --git a/src/__tests__/hooks/useLostBoundaryDismissal.test.ts b/src/__tests__/hooks/useLostBoundaryDismissal.test.ts new file mode 100644 index 000000000..6dfc2245a --- /dev/null +++ b/src/__tests__/hooks/useLostBoundaryDismissal.test.ts @@ -0,0 +1,276 @@ +/// + +import { act, renderHook } from '@testing-library/react'; +import type { Book, SegmentationDelta } from 'interlinearizer'; +import useLostBoundaryDismissal from '../../hooks/useLostBoundaryDismissal'; +import { GEN_1_1_BOOK, makeSegment, makeWebViewState, makeWordToken } from '../test-helpers'; + +/** A two-verse book, with a mid-verse token, that the deltas below anchor into. */ +const TWO_VERSE_BOOK: Book = { + ...GEN_1_1_BOOK, + segments: [ + makeSegment('GEN 1:1', 'Alpha beta.', [ + makeWordToken('GEN 1:1:0', 'Alpha'), + makeWordToken('GEN 1:1:6', 'beta', 6), + ]), + makeSegment('GEN 1:2', 'Gamma.', [makeWordToken('GEN 1:2:0', 'Gamma')]), + ], +}; + +/** {@link TWO_VERSE_BOOK} with the verse carrying the anchor the deltas below remove. */ +const THREE_VERSE_BOOK: Book = { + ...TWO_VERSE_BOOK, + segments: [ + ...TWO_VERSE_BOOK.segments, + makeSegment('GEN 1:3', 'Delta.', [makeWordToken('GEN 1:3:0', 'Delta')]), + ], +}; + +/** Another book entirely, whose own anchors are all intact. */ +const OTHER_BOOK: Book = { + ...TWO_VERSE_BOOK, + id: 'EXO', + bookRef: 'EXO', + segments: [makeSegment('EXO 1:1', 'Epsilon.', [makeWordToken('EXO 1:1:0', 'Epsilon')])], +}; + +/** The hook options a test varies between renders; the rest stay fixed. */ +type HookInput = { + verseBook?: Book | undefined; + segmentation?: SegmentationDelta | undefined; + draftVersion?: number; + isImportView?: boolean; +}; + +/** + * Renders the hook on {@link TWO_VERSE_BOOK} with one WebView-state store held across rerenders, so + * a dismissal persists exactly as it does for a tab. Rerendering takes the whole input afresh + * rather than a patch, keeping each step's book and delta stated where it is asserted on. + */ +function renderDismissal(initial: HookInput, webViewSeed: Record = {}) { + const useWebViewState = makeWebViewState(webViewSeed); + const { result, rerender } = renderHook( + (input: HookInput) => + useLostBoundaryDismissal({ + verseBook: 'verseBook' in input ? input.verseBook : TWO_VERSE_BOOK, + segmentation: input.segmentation, + segmentationVersion: 0, + draftVersion: input.draftVersion ?? 0, + isDraftLoading: false, + isImportView: input.isImportView ?? false, + useWebViewState, + }), + { initialProps: initial }, + ); + return { result, rerenderWith: (input: HookInput) => act(() => rerender(input)) }; +} + +describe('useLostBoundaryDismissal', () => { + describe('finding the lost anchors', () => { + it('reports the anchors the source no longer carries', () => { + const { result } = renderDismissal({ + segmentation: { removedVerseStarts: ['GEN 1:9:0'], addedStarts: ['GEN 1:1:99'] }, + }); + + expect(result.current.undismissedLostBoundaries).toEqual(['GEN 1:9:0', 'GEN 1:1:99']); + }); + + it('reports nothing when every anchor still resolves', () => { + const { result } = renderDismissal({ + segmentation: { removedVerseStarts: ['GEN 1:2:0'], addedStarts: ['GEN 1:1:6'] }, + }); + + expect(result.current.undismissedLostBoundaries).toEqual([]); + }); + + it('reports nothing for the default segmentation', () => { + const { result } = renderDismissal({ segmentation: undefined }); + + expect(result.current.undismissedLostBoundaries).toEqual([]); + }); + + it('reports nothing while no book is loaded', () => { + const { result } = renderDismissal({ + verseBook: undefined, + segmentation: { removedVerseStarts: ['GEN 1:9:0'], addedStarts: [] }, + }); + + expect(result.current.undismissedLostBoundaries).toEqual([]); + }); + + it('reports nothing for an import view, which the draft boundaries never reach', () => { + const { result } = renderDismissal({ + isImportView: true, + segmentation: { removedVerseStarts: ['GEN 1:9:0'], addedStarts: [] }, + }); + + expect(result.current.undismissedLostBoundaries).toEqual([]); + }); + + it('ignores anchors in a book other than the loaded one', () => { + const { result } = renderDismissal({ + segmentation: { removedVerseStarts: ['EXO 1:5:0'], addedStarts: ['EXO 1:1:6'] }, + }); + + expect(result.current.undismissedLostBoundaries).toEqual([]); + }); + }); + + describe('dismissal', () => { + it('clears the flag for exactly the anchors it was raised for', () => { + const { result } = renderDismissal({ + segmentation: { removedVerseStarts: ['GEN 1:9:0'], addedStarts: [] }, + }); + + act(() => result.current.onDismiss()); + + expect(result.current.undismissedLostBoundaries.length > 0).toBe(false); + }); + + it('stays down while the same anchors stay lost', () => { + const segmentation = { removedVerseStarts: ['GEN 1:9:0'], addedStarts: [] }; + const { result, rerenderWith } = renderDismissal({ segmentation }); + act(() => result.current.onDismiss()); + + // A source edit re-tokenizes to a fresh Book carrying the same text. + rerenderWith({ verseBook: { ...TWO_VERSE_BOOK }, segmentation }); + + expect(result.current.undismissedLostBoundaries.length > 0).toBe(false); + }); + + it('stays down for a tab whose stored dismissal covers every lost anchor', () => { + const { result } = renderDismissal( + { segmentation: { removedVerseStarts: ['GEN 1:9:0'], addedStarts: ['GEN 1:1:99'] } }, + { dismissedLostBoundaries: ['GEN 1:9:0', 'GEN 1:1:99'] }, + ); + + expect(result.current.undismissedLostBoundaries.length > 0).toBe(false); + }); + + it('comes back for an anchor lost after the dismissal, reporting only that one', () => { + // The stored dismissal covers one of the two anchors the loaded source strands. + const { result } = renderDismissal( + { segmentation: { removedVerseStarts: ['GEN 1:9:0'], addedStarts: ['GEN 1:1:99'] } }, + { dismissedLostBoundaries: ['GEN 1:9:0'] }, + ); + + expect(result.current.undismissedLostBoundaries).toEqual(['GEN 1:1:99']); + }); + + it('keeps another book’s dismissal when dismissing in this one', () => { + // One delta spans the draft, so each book's dismissal covers only the anchors it reports. + const segmentation = { removedVerseStarts: ['GEN 1:9:0', 'EXO 1:9:0'], addedStarts: [] }; + const { result, rerenderWith } = renderDismissal({ segmentation }); + act(() => result.current.onDismiss()); + + rerenderWith({ verseBook: OTHER_BOOK, segmentation }); + expect(result.current.undismissedLostBoundaries).toEqual(['EXO 1:9:0']); + act(() => result.current.onDismiss()); + + rerenderWith({ verseBook: { ...TWO_VERSE_BOOK }, segmentation }); + + expect(result.current.undismissedLostBoundaries).toEqual([]); + }); + + it('stays down when an anchor comes back but the rest are dismissed', () => { + // 'GEN 1:2:0' resolves in this book, so only the dismissed anchor is still lost. + const { result } = renderDismissal( + { segmentation: { removedVerseStarts: ['GEN 1:9:0', 'GEN 1:2:0'], addedStarts: [] } }, + { dismissedLostBoundaries: ['GEN 1:9:0', 'GEN 1:1:99'] }, + ); + + expect(result.current.undismissedLostBoundaries.length > 0).toBe(false); + }); + }); + + describe('dropping a spent dismissal', () => { + it('comes back when a recovered anchor is stranded again', () => { + const segmentation = { removedVerseStarts: ['GEN 1:3:0'], addedStarts: [] }; + const { result, rerenderWith } = renderDismissal({ segmentation }); + act(() => result.current.onDismiss()); + + rerenderWith({ verseBook: THREE_VERSE_BOOK, segmentation }); + expect(result.current.undismissedLostBoundaries.length > 0).toBe(false); + + // The recovery ended the loss the dismissal acknowledged, so losing it again is a fresh one. + rerenderWith({ verseBook: { ...TWO_VERSE_BOOK }, segmentation }); + + expect(result.current.undismissedLostBoundaries.length > 0).toBe(true); + }); + + it('keeps a dismissal across a visit to another book', () => { + const segmentation = { removedVerseStarts: ['GEN 1:9:0'], addedStarts: [] }; + const { result, rerenderWith } = renderDismissal({ segmentation }); + act(() => result.current.onDismiss()); + + // The other book reports none of GEN's anchors, which is not the same as their recovery. + rerenderWith({ verseBook: OTHER_BOOK, segmentation }); + rerenderWith({ verseBook: { ...TWO_VERSE_BOOK }, segmentation }); + + expect(result.current.undismissedLostBoundaries.length > 0).toBe(false); + }); + + it('comes back when the anchor recovers in its own book after a visit elsewhere', () => { + const segmentation = { removedVerseStarts: ['GEN 1:3:0'], addedStarts: [] }; + const { result, rerenderWith } = renderDismissal({ segmentation }); + act(() => result.current.onDismiss()); + + rerenderWith({ verseBook: OTHER_BOOK, segmentation }); + // Back in GEN the anchor resolves, so the dismissal it covered is spent. + rerenderWith({ verseBook: THREE_VERSE_BOOK, segmentation }); + rerenderWith({ verseBook: { ...TWO_VERSE_BOOK }, segmentation }); + + expect(result.current.undismissedLostBoundaries.length > 0).toBe(true); + }); + + it('reads no recovery from an import view, which reports no anchors of its own', () => { + const segmentation = { removedVerseStarts: ['GEN 1:9:0'], addedStarts: [] }; + const { result, rerenderWith } = renderDismissal({ segmentation }); + act(() => result.current.onDismiss()); + + rerenderWith({ isImportView: true, segmentation }); + rerenderWith({ segmentation }); + + expect(result.current.undismissedLostBoundaries.length > 0).toBe(false); + }); + + it('drops the dismissal when the draft is replaced wholesale', () => { + const segmentation = { removedVerseStarts: ['GEN 1:9:0'], addedStarts: [] }; + const { result, rerenderWith } = renderDismissal({ segmentation }); + act(() => result.current.onDismiss()); + + // The replacement carries the same delta, so the same anchor is lost afresh. + rerenderWith({ segmentation, draftVersion: 1 }); + + expect(result.current.undismissedLostBoundaries.length > 0).toBe(true); + }); + + it('drops the dismissal when a replacement keeps one lost anchor and recovers another', () => { + const { result, rerenderWith } = renderDismissal({ + segmentation: { removedVerseStarts: ['GEN 1:9:0', 'GEN 1:8:0'], addedStarts: [] }, + }); + act(() => result.current.onDismiss()); + + // The replacement strands only the first anchor, whose loss the user has not seen in it. + rerenderWith({ + segmentation: { removedVerseStarts: ['GEN 1:9:0'], addedStarts: [] }, + draftVersion: 1, + }); + + expect(result.current.undismissedLostBoundaries.length > 0).toBe(true); + }); + + it('keeps a stored dismissal through the mount pass, so a restored tab stays down', () => { + const { result } = renderDismissal( + { + segmentation: { removedVerseStarts: ['GEN 1:9:0'], addedStarts: [] }, + // A restored tab mounts at whatever draft version it left off at, not at zero. + draftVersion: 4, + }, + { dismissedLostBoundaries: ['GEN 1:9:0'] }, + ); + + expect(result.current.undismissedLostBoundaries.length > 0).toBe(false); + }); + }); +}); diff --git a/src/__tests__/parsers/papi/resegmentBook.test.ts b/src/__tests__/parsers/papi/resegmentBook.test.ts index 4a517b209..a84e8ed93 100644 --- a/src/__tests__/parsers/papi/resegmentBook.test.ts +++ b/src/__tests__/parsers/papi/resegmentBook.test.ts @@ -28,6 +28,14 @@ describe('resegmentBook', () => { expect(resegmentBook(BOOK, { removedVerseStarts: [], addedStarts: [] })).toBe(BOOK); }); + it('returns the same book reference when the delta only names other books', () => { + // One delta spans the draft, so an Exodus-only delta must not cost Genesis a token-stream walk + // and a fresh Book object that no boundary of its own justifies. + expect( + resegmentBook(BOOK, { removedVerseStarts: ['EXO 1:5:0'], addedStarts: ['EXO 1:1:6'] }), + ).toBe(BOOK); + }); + it('reuses untouched verse Segment objects by reference when a delta is active elsewhere', () => { // Merge verses 1+2; verse 3 is untouched and should be the same object. const result = resegmentBook(BOOK, { removedVerseStarts: ['GEN 1:2:0'], addedStarts: [] }); diff --git a/src/__tests__/utils/segmentation.test.ts b/src/__tests__/utils/segmentation.test.ts index df6fe933e..da30edb5e 100644 --- a/src/__tests__/utils/segmentation.test.ts +++ b/src/__tests__/utils/segmentation.test.ts @@ -5,7 +5,9 @@ import { addBoundaryBefore, defaultVerseStarts, effectiveStarts, - isDefaultSegmentation, + isDefaultSegmentationForBook, + isEmptyDelta, + lostBoundaries, mergeSegments, moveBoundary, removeBoundaryAt, @@ -43,6 +45,23 @@ const VZ0_START = 'GEN 2:0:0'; const VZ0_INTERIOR = 'GEN 2:0:4'; const VZ_NEXT_START = 'GEN 2:1:0'; +/** + * A fixture whose middle verse carries no token (an empty verse marker), leaving verse 3 with no + * preceding token run to be merged into. + */ +const EMPTY_MIDDLE_VERSE = makeVerseBook([ + { sid: 'GEN 1:1', number: '1', text: 'Alpha beta.' }, + { sid: 'GEN 1:2', number: '2', text: ' ' }, + { sid: 'GEN 1:3', number: '3', text: 'Epsilon.' }, +]); + +/** The same, with the token-less verse opening the book, so verse 2 begins the first token run. */ +const EMPTY_FIRST_VERSE = makeVerseBook([ + { sid: 'GEN 1:1', number: '1', text: ' ' }, + { sid: 'GEN 1:2', number: '2', text: 'Gamma delta.' }, + { sid: 'GEN 1:3', number: '3', text: 'Epsilon.' }, +]); + describe('defaultVerseStarts', () => { it('returns the first-token ref of every verse', () => { expect(defaultVerseStarts(THREE_VERSES)).toEqual(new Set([V1_START, V2_START, V3_START])); @@ -57,21 +76,151 @@ describe('defaultVerseStarts', () => { }); }); -describe('isDefaultSegmentation', () => { +describe('isEmptyDelta', () => { it('is true for undefined', () => { - expect(isDefaultSegmentation(undefined)).toBe(true); + expect(isEmptyDelta(undefined)).toBe(true); }); it('is true for empty arrays', () => { - expect(isDefaultSegmentation({ removedVerseStarts: [], addedStarts: [] })).toBe(true); + expect(isEmptyDelta({ removedVerseStarts: [], addedStarts: [] })).toBe(true); }); it('is false when a boundary is removed', () => { - expect(isDefaultSegmentation({ removedVerseStarts: [V2_START], addedStarts: [] })).toBe(false); + expect(isEmptyDelta({ removedVerseStarts: [V2_START], addedStarts: [] })).toBe(false); }); it('is false when a boundary is added', () => { - expect(isDefaultSegmentation({ removedVerseStarts: [], addedStarts: [V1_BETA] })).toBe(false); + expect(isEmptyDelta({ removedVerseStarts: [], addedStarts: [V1_BETA] })).toBe(false); + }); +}); + +describe('isDefaultSegmentationForBook', () => { + it('is true for undefined', () => { + expect(isDefaultSegmentationForBook(THREE_VERSES, undefined)).toBe(true); + }); + + it('is true when every anchor names another book', () => { + expect( + isDefaultSegmentationForBook(THREE_VERSES, { + removedVerseStarts: ['EXO 1:5:0'], + addedStarts: ['EXO 1:1:6'], + }), + ).toBe(true); + }); + + it('is false when this book has a removed boundary', () => { + expect( + isDefaultSegmentationForBook(THREE_VERSES, { + removedVerseStarts: [V2_START], + addedStarts: ['EXO 1:1:6'], + }), + ).toBe(false); + }); + + it('is false when this book has an added boundary', () => { + expect( + isDefaultSegmentationForBook(THREE_VERSES, { + removedVerseStarts: ['EXO 1:5:0'], + addedStarts: [V1_BETA], + }), + ).toBe(false); + }); +}); + +describe('lostBoundaries', () => { + it('is empty for undefined', () => { + expect(lostBoundaries(THREE_VERSES, undefined)).toEqual([]); + }); + + it('is empty when every anchor still names a token', () => { + const delta: SegmentationDelta = { removedVerseStarts: [V2_START], addedStarts: [V1_BETA] }; + expect(lostBoundaries(THREE_VERSES, delta)).toEqual([]); + }); + + it('reports a removed verse start whose token is gone', () => { + const delta: SegmentationDelta = { removedVerseStarts: ['GEN 1:9:0'], addedStarts: [] }; + expect(lostBoundaries(THREE_VERSES, delta)).toEqual(['GEN 1:9:0']); + }); + + it('reports an added start whose char offset no longer exists', () => { + const delta: SegmentationDelta = { removedVerseStarts: [], addedStarts: ['GEN 1:1:99'] }; + expect(lostBoundaries(THREE_VERSES, delta)).toEqual(['GEN 1:1:99']); + }); + + it('reports losses from both arrays, keeping the surviving anchors out', () => { + const delta: SegmentationDelta = { + removedVerseStarts: [V2_START, 'GEN 1:9:0'], + addedStarts: [V1_BETA, 'GEN 1:1:99'], + }; + expect(lostBoundaries(THREE_VERSES, delta)).toEqual(['GEN 1:9:0', 'GEN 1:1:99']); + }); + + it('ignores anchors naming a book other than the one loaded', () => { + // One delta spans the whole draft, so a boundary set in Exodus is intact, not lost. + const delta: SegmentationDelta = { + removedVerseStarts: ['EXO 1:5:0'], + addedStarts: ['EXO 1:1:6'], + }; + expect(lostBoundaries(THREE_VERSES, delta)).toEqual([]); + }); + + it('still reports this book’s losses when another book’s anchors are present', () => { + const delta: SegmentationDelta = { + removedVerseStarts: ['EXO 1:5:0', 'GEN 1:9:0'], + addedStarts: ['EXO 1:1:6'], + }; + expect(lostBoundaries(THREE_VERSES, delta)).toEqual(['GEN 1:9:0']); + }); + + it('reports a removed start whose token survived but no longer begins a verse', () => { + // A mid-verse ref leaves the merge nothing to remove, the drifted source having moved the + // verse start off it. + const delta: SegmentationDelta = { removedVerseStarts: [V1_BETA], addedStarts: [] }; + expect(lostBoundaries(THREE_VERSES, delta)).toEqual([V1_BETA]); + }); + + it('reports nothing for an added start whose token has become a verse’s own first token', () => { + const delta: SegmentationDelta = { removedVerseStarts: [], addedStarts: [V2_START] }; + expect(effectiveStarts(THREE_VERSES, delta).has(V2_START)).toBe(true); + expect(lostBoundaries(THREE_VERSES, delta)).toEqual([]); + }); + + it('reports a merge that drift turned into the book’s first token', () => { + // Verse 1 has gone missing upstream, leaving the merged-away start of verse 2 to begin the book. + const droppedFirstVerse = makeVerseBook([ + { sid: 'GEN 1:2', number: '2', text: 'Gamma delta.' }, + { sid: 'GEN 1:3', number: '3', text: 'Epsilon.' }, + ]); + const delta: SegmentationDelta = { removedVerseStarts: [V2_START], addedStarts: [] }; + expect(effectiveStarts(droppedFirstVerse, delta).has(V2_START)).toBe(true); + expect(lostBoundaries(droppedFirstVerse, delta)).toEqual([V2_START]); + }); + + it('reports a removed start that drift left on the book’s first token', () => { + // No edit records this anchor, so its presence means earlier source text went missing. + const delta: SegmentationDelta = { removedVerseStarts: [V1_START], addedStarts: [] }; + expect(lostBoundaries(THREE_VERSES, delta)).toEqual([V1_START]); + }); + + it('reports a merge whose preceding verse drift left token-less', () => { + const delta: SegmentationDelta = { removedVerseStarts: [V3_START], addedStarts: [] }; + expect(lostBoundaries(EMPTY_MIDDLE_VERSE, delta)).toEqual([V3_START]); + }); + + it('reports a merge into a token-less verse that opens the book', () => { + const delta: SegmentationDelta = { removedVerseStarts: [V2_START], addedStarts: [] }; + expect(lostBoundaries(EMPTY_FIRST_VERSE, delta)).toEqual([V2_START]); + }); + + it('keeps reporting nothing for a merge whose preceding verse still has tokens', () => { + const book = makeVerseBook([ + { sid: 'GEN 1:1', number: '1', text: 'Alpha beta.' }, + { sid: 'GEN 1:2', number: '2', text: ' ' }, + { sid: 'GEN 1:3', number: '3', text: 'Epsilon here.' }, + { sid: 'GEN 1:4', number: '4', text: 'Zeta.' }, + ]); + const delta: SegmentationDelta = { removedVerseStarts: ['GEN 1:4:0'], addedStarts: [] }; + expect(lostBoundaries(book, delta)).toEqual([]); }); }); @@ -113,6 +262,22 @@ describe('effectiveStarts', () => { }); expect(starts.has(V1_START)).toBe(true); }); + + it('keeps a start whose preceding verse is token-less, matching resegmentBook', () => { + const starts = effectiveStarts(EMPTY_MIDDLE_VERSE, { + removedVerseStarts: [V3_START], + addedStarts: [], + }); + expect(starts.has(V3_START)).toBe(true); + }); + + it('keeps the first token-bearing start when a token-less verse opens the book', () => { + const starts = effectiveStarts(EMPTY_FIRST_VERSE, { + removedVerseStarts: [V2_START], + addedStarts: [], + }); + expect(starts.has(V2_START)).toBe(true); + }); }); describe('addBoundaryBefore', () => { @@ -180,6 +345,21 @@ describe('removeBoundaryAt', () => { addedStarts: [], }); }); + + it('is a no-op for a start whose preceding verse carries no token', () => { + // Recording the removal would store an entry lostBoundaries immediately reports as lost. + expect(removeBoundaryAt(EMPTY_MIDDLE_VERSE, undefined, V3_START)).toEqual({ + removedVerseStarts: [], + addedStarts: [], + }); + }); + + it('is a no-op for the first token-bearing start when a token-less verse opens the book', () => { + expect(removeBoundaryAt(EMPTY_FIRST_VERSE, undefined, V2_START)).toEqual({ + removedVerseStarts: [], + addedStarts: [], + }); + }); }); describe('moveBoundary', () => { @@ -227,20 +407,156 @@ describe('normalization', () => { expect(result).toEqual({ removedVerseStarts: [V2_START, V3_START], addedStarts: [V1_BETA] }); }); - it('strips a removed ref that is not a default verse start', () => { - const bogus: SegmentationDelta = { removedVerseStarts: [V1_BETA], addedStarts: [] }; - // V1_BETA is mid-verse, not a default start, so it is not a valid removal. - expect(removeBoundaryAt(THREE_VERSES, bogus, V3_START)).toEqual({ - removedVerseStarts: [V3_START], + it('keeps a removed ref whose token drifted off a verse start', () => { + // V1_BETA is mid-verse, so this source honors no removal there, but its token is still present. + const drifted: SegmentationDelta = { removedVerseStarts: [V1_BETA], addedStarts: [] }; + expect(removeBoundaryAt(THREE_VERSES, drifted, V3_START)).toEqual({ + removedVerseStarts: [V3_START, V1_BETA], addedStarts: [], }); }); - it('strips an added ref that is actually a default verse start', () => { - const bogus: SegmentationDelta = { removedVerseStarts: [], addedStarts: [V2_START] }; - expect(addBoundaryBefore(THREE_VERSES, bogus, V1_BETA)).toEqual({ + it('keeps an added ref whose token drifted onto a default verse start', () => { + const drifted: SegmentationDelta = { removedVerseStarts: [], addedStarts: [V2_START] }; + expect(addBoundaryBefore(THREE_VERSES, drifted, V1_BETA)).toEqual({ + removedVerseStarts: [], + addedStarts: [V1_BETA, V2_START], + }); + }); + + it('keeps a removed ref that drift left on the book’s first token', () => { + // The merge returns if the missing earlier text does, so the anchor outlives the edit. + const drifted: SegmentationDelta = { removedVerseStarts: [V1_START], addedStarts: [] }; + expect(removeBoundaryAt(THREE_VERSES, drifted, V3_START)).toEqual({ + removedVerseStarts: [V3_START, V1_START], + addedStarts: [], + }); + }); + + it('keeps another book’s added start when splitting in this one', () => { + // One delta spans the draft, so an Exodus split must survive an edit made while Genesis is + // loaded — its ref cannot resolve here, but that is absence of evidence, not a dead anchor. + const withExodus: SegmentationDelta = { removedVerseStarts: [], addedStarts: ['EXO 1:1:6'] }; + expect(addBoundaryBefore(THREE_VERSES, withExodus, V1_BETA)).toEqual({ + removedVerseStarts: [], + addedStarts: [V1_BETA, 'EXO 1:1:6'], + }); + }); + + it('keeps another book’s removed verse start when merging in this one', () => { + const withExodus: SegmentationDelta = { removedVerseStarts: ['EXO 1:5:0'], addedStarts: [] }; + expect(removeBoundaryAt(THREE_VERSES, withExodus, V2_START)).toEqual({ + removedVerseStarts: [V2_START, 'EXO 1:5:0'], + addedStarts: [], + }); + }); + + it('keeps another book’s anchors when merging the book-first token is a no-op', () => { + const withExodus: SegmentationDelta = { + removedVerseStarts: ['EXO 1:5:0'], + addedStarts: ['EXO 1:1:6'], + }; + expect(removeBoundaryAt(THREE_VERSES, withExodus, V1_START)).toEqual(withExodus); + }); + + it('sorts other books’ anchors by ref, whichever book is loaded', () => { + // The foreign tail has no token stream to sort by, so ref order stands in. + const foreign: SegmentationDelta = { + removedVerseStarts: ['REV 1:1:0', 'EXO 1:5:0', 'EXO 1:2:0', 'LEV 1:1:0'], + addedStarts: [], + }; + expect(removeBoundaryAt(THREE_VERSES, foreign, V2_START)).toEqual({ + removedVerseStarts: [V2_START, 'EXO 1:2:0', 'EXO 1:5:0', 'LEV 1:1:0', 'REV 1:1:0'], + addedStarts: [], + }); + }); + + it('sorts this book’s unhonored anchors by ref, whatever order the edits arrived in', () => { + const unhonored: SegmentationDelta = { + removedVerseStarts: ['GEN 1:9:0', 'GEN 1:11:0', 'GEN 1:10:0'], + addedStarts: [], + }; + expect(removeBoundaryAt(THREE_VERSES, unhonored, V2_START)).toEqual({ + removedVerseStarts: [V2_START, 'GEN 1:10:0', 'GEN 1:11:0', 'GEN 1:9:0'], + addedStarts: [], + }); + }); + + it('keeps this book’s drift-hidden added start through an unrelated split', () => { + // A ref this source cannot resolve: unhonored for now, but recoverable if the source reverts. + const hidden: SegmentationDelta = { removedVerseStarts: [], addedStarts: ['GEN 1:1:99'] }; + expect(addBoundaryBefore(THREE_VERSES, hidden, V1_BETA)).toEqual({ + removedVerseStarts: [], + addedStarts: [V1_BETA, 'GEN 1:1:99'], + }); + }); + + it('keeps this book’s drift-hidden removed start through an unrelated merge', () => { + const hidden: SegmentationDelta = { removedVerseStarts: ['GEN 1:9:0'], addedStarts: [] }; + expect(removeBoundaryAt(THREE_VERSES, hidden, V2_START)).toEqual({ + removedVerseStarts: [V2_START, 'GEN 1:9:0'], + addedStarts: [], + }); + }); + + it('sorts drift-hidden anchors after the resolvable ones, before other books’', () => { + const mixed: SegmentationDelta = { + removedVerseStarts: ['EXO 1:5:0', 'GEN 1:9:0', V3_START], + addedStarts: [], + }; + expect(removeBoundaryAt(THREE_VERSES, mixed, V2_START)).toEqual({ + removedVerseStarts: [V2_START, V3_START, 'GEN 1:9:0', 'EXO 1:5:0'], + addedStarts: [], + }); + }); + + it('keeps every boundary lostBoundaries reports through an unrelated edit', () => { + // A boundary reported as a recoverable loss is not recoverable at all if a later edit deletes + // the entry recording it. + const drifted: SegmentationDelta = { + removedVerseStarts: [V1_START, V1_BETA, 'GEN 1:9:0'], + addedStarts: [V2_START, 'GEN 1:1:99'], + }; + const lost = lostBoundaries(THREE_VERSES, drifted); + // V2_START is unhonored but not lost — a verse start carries the split it asked for. + expect(lost).toEqual([V1_START, V1_BETA, 'GEN 1:9:0', 'GEN 1:1:99']); + const after = addBoundaryBefore(THREE_VERSES, drifted, 'GEN 1:2:6'); + const survivors = [...after.removedVerseStarts, ...after.addedStarts]; + lost.forEach((ref) => expect(survivors).toContain(ref)); + }); + + it('clears a drifted added start when merging at that same ref', () => { + // Drift moved V2_START's token onto a verse start while an added split still names it. + const drifted: SegmentationDelta = { removedVerseStarts: [], addedStarts: [V2_START] }; + const merged = removeBoundaryAt(THREE_VERSES, drifted, V2_START); + expect(merged).toEqual({ removedVerseStarts: [V2_START], addedStarts: [] }); + expect(effectiveStarts(THREE_VERSES, merged).has(V2_START)).toBe(false); + }); + + it('clears a drifted removed start when splitting at that same ref', () => { + // The mirror case: a removal naming a ref that drift left mid-verse, un-done by a split there. + const drifted: SegmentationDelta = { removedVerseStarts: [V1_BETA], addedStarts: [] }; + expect(addBoundaryBefore(THREE_VERSES, drifted, V1_BETA)).toEqual({ removedVerseStarts: [], addedStarts: [V1_BETA], }); }); + + it('moves a boundary off a ref a drifted added start also names', () => { + const drifted: SegmentationDelta = { removedVerseStarts: [], addedStarts: [V2_START] }; + const moved = moveBoundary(THREE_VERSES, drifted, V2_START, V1_BETA); + expect(moved).toEqual({ removedVerseStarts: [V2_START], addedStarts: [V1_BETA] }); + expect(effectiveStarts(THREE_VERSES, moved).has(V2_START)).toBe(false); + }); + + it('still dedupes and sorts this book’s anchors alongside another book’s', () => { + const messy: SegmentationDelta = { + removedVerseStarts: ['EXO 1:5:0', V3_START, V2_START, V2_START], + addedStarts: [], + }; + expect(removeBoundaryAt(THREE_VERSES, messy, V2_START)).toEqual({ + removedVerseStarts: [V2_START, V3_START, 'EXO 1:5:0'], + addedStarts: [], + }); + }); }); diff --git a/src/components/InterlinearizerLoader.tsx b/src/components/InterlinearizerLoader.tsx index 8042b1c0d..2f7a0e54f 100644 --- a/src/components/InterlinearizerLoader.tsx +++ b/src/components/InterlinearizerLoader.tsx @@ -13,6 +13,7 @@ import { TabToolbar, } from 'platform-bible-react'; import type { SelectMenuItemHandler } from 'platform-bible-react'; +import { X } from 'lucide-react'; import { formatReplacementString, isPlatformError } from 'platform-bible-utils'; import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import type { ComponentProps, ReactNode, RefObject } from 'react'; @@ -20,9 +21,10 @@ import type { TextAnalysis } from 'interlinearizer'; import { resegmentBook } from 'parsers/papi/resegmentBook'; import useDraftProject from '../hooks/useDraftProject'; import useInterlinearizerBookData from '../hooks/useInterlinearizerBookData'; +import useLostBoundaryDismissal from '../hooks/useLostBoundaryDismissal'; import useOptimisticBooleanSetting from '../hooks/useOptimisticBooleanSetting'; import { - isDefaultSegmentation, + isEmptyDelta, mergeSegments, moveBoundary, splitSegmentBefore, @@ -161,8 +163,15 @@ const STRING_KEYS = [ '%interlinearizer_banner_pt9Import%', '%interlinearizer_banner_sync%', '%interlinearizer_banner_copy%', + '%interlinearizer_segmentation_lostBoundaries%', + '%interlinearizer_segmentation_lostBoundaries_one%', + '%interlinearizer_segmentation_lostBoundaries_dismiss%', ] as const satisfies `%${string}%`[]; +/** The full-width strip every banner above the view area shares. */ +const BANNER_STRIP_CLASS = + 'tw:flex tw:items-center tw:gap-2 tw:border-b tw:border-border tw:bg-muted/40 tw:px-3 tw:py-1.5'; + /** * How long the first-open data probe may stay unanswered before the checking dialog shows. A fast * answer - which every project without Paratext 9 data gives - never shows one. @@ -232,7 +241,7 @@ function InterlinearizerLoaderInner({ }>) { const { scrRef, navigate, scrollGroupId, setScrollGroupId, fadePhase, cancelFade } = useInterlinearNav(); - const [localizedStrings] = useLocalizedStrings(STRING_KEYS); + const [localizedStrings, stringsLoading] = useLocalizedStrings(STRING_KEYS); const [interfaceMode] = useSetting('platform.interfaceMode', 'simple'); const [interfaceLanguages] = useSetting('platform.interfaceLanguage', ['und']); @@ -456,9 +465,9 @@ function InterlinearizerLoaderInner({ /** * The book the views render: the verse-tokenized book re-grouped into the user's custom segments. - * Identical (by reference) to `verseBook` when no custom boundaries are set, so the common case - * incurs no extra work. `verseBook` is retained separately because the segmentation operations - * need the default verse boundaries it carries. + * Identical (by reference) to `verseBook` when no custom boundaries are set in it, so the common + * case incurs no extra work. `verseBook` is retained separately because the segmentation + * operations need the default verse boundaries it carries. * * `draft.segmentation` is read fresh from the ref-held draft at recompute time; the deps are the * two version counters that cover every path that can change it — `segmentationVersion` for @@ -478,6 +487,17 @@ function InterlinearizerLoaderInner({ [verseBook, segmentationVersion, draftVersion, isDraftLoading, isImportView], ); + const { undismissedLostBoundaries, onDismiss: handleDismissLostBoundaries } = + useLostBoundaryDismissal({ + verseBook, + segmentation: draft?.segmentation, + segmentationVersion, + draftVersion, + isDraftLoading, + isImportView, + useWebViewState, + }); + /** * Maps each merged-away default verse boundary's word-token split anchor — the verse's first word * token, the ref the boundary slots are keyed by — to the removed default start ref (the verse's @@ -518,7 +538,7 @@ function InterlinearizerLoaderInner({ * `undefined` when the edit restores the default verse segmentation. */ const apply = (next: ReturnType) => { - autosaveSegmentation(isDefaultSegmentation(next) ? undefined : next); + autosaveSegmentation(isEmptyDelta(next) ? undefined : next); }; return { merge: (secondSegmentStartRef) => { @@ -1073,18 +1093,23 @@ function InterlinearizerLoaderInner({
{bookError && (
-

- {localizedStrings['%interlinearizer_error_load_book_heading%']} -

+ {/* The error text is not localized, so it shows here and below without waiting. */} + {!stringsLoading && ( +

+ {localizedStrings['%interlinearizer_error_load_book_heading%']} +

+ )}
{bookError}
)} {tokenizeError && (
-

- {localizedStrings['%interlinearizer_error_process_book_heading%']} -

+ {!stringsLoading && ( +

+ {localizedStrings['%interlinearizer_error_process_book_heading%']} +

+ )}
{tokenizeError.message}
)} @@ -1096,7 +1121,7 @@ function InterlinearizerLoaderInner({

)} - {!hasError && !showLoading && importLoadFailed && ( + {!hasError && !showLoading && importLoadFailed && !stringsLoading && (

{localizedStrings['%interlinearizer_error_pt9Import_load_failed%']}

@@ -1263,11 +1288,10 @@ function InterlinearizerLoaderInner({ }} /> - {isImportView && activeProject?.pt9Import && ( -
+ {/* The strip waits on localization whole: its button labels are localized too, so an + unresolved render would leave Sync and Copy with no label at all. */} + {isImportView && activeProject?.pt9Import && !stringsLoading && ( +
{formatReplacementString(localizedStrings['%interlinearizer_banner_pt9Import%'], { date: new Date(activeProject.pt9Import.importedAt).toLocaleString(), @@ -1289,6 +1313,29 @@ function InterlinearizerLoaderInner({
)} + {isLoaded && undismissedLostBoundaries.length > 0 && !stringsLoading && ( +
+ + {undismissedLostBoundaries.length === 1 + ? localizedStrings['%interlinearizer_segmentation_lostBoundaries_one%'] + : formatReplacementString( + localizedStrings['%interlinearizer_segmentation_lostBoundaries%'], + { count: undismissedLostBoundaries.length }, + )} + + +
+ )} +
{viewArea}
{ // Treat the default segmentation (undefined or a delta with both arrays empty) the same as // `undefined`: clear the field rather than persisting a redundant custom object. - const hasCustomBoundaries = !isDefaultSegmentation(segmentation); + const hasCustomBoundaries = !isEmptyDelta(segmentation); const applied = autosaveDraft((current) => { const next: DraftProject = { ...current, dirty: true }; // Store custom boundaries when present; clear the field for the default segmentation so the diff --git a/src/hooks/useLostBoundaryDismissal.ts b/src/hooks/useLostBoundaryDismissal.ts new file mode 100644 index 000000000..8615cb8f7 --- /dev/null +++ b/src/hooks/useLostBoundaryDismissal.ts @@ -0,0 +1,132 @@ +import type { UseWebViewStateHook } from '@papi/core'; +import type { Book, SegmentationDelta } from 'interlinearizer'; +import { useCallback, useEffect, useMemo, useRef } from 'react'; +import { lostBoundaries as findLostBoundaries } from '../utils/segmentation'; + +type LostBoundaryDismissalOptions = { + /** The verse-tokenized loaded book, `undefined` while none is loaded. */ + verseBook: Book | undefined; + /** The draft's boundary edits, whose anchors are the ones checked against the source. */ + segmentation: SegmentationDelta | undefined; + /** Bumped by every boundary edit. */ + segmentationVersion: number; + /** Bumped by every wholesale draft replacement (New / Open / Wipe). */ + draftVersion: number; + /** Whether the initial draft load is still outstanding, which bumps neither counter. */ + isDraftLoading: boolean; + /** Whether a read-only Paratext 9 import is showing, which the draft's boundaries never reach. */ + isImportView: boolean; + /** Scopes the dismissal to one tab. */ + useWebViewState: UseWebViewStateHook; +}; + +type LostBoundaryDismissal = { + /** The boundaries the loaded source text no longer carries and the user has not dismissed. */ + undismissedLostBoundaries: readonly string[]; + /** Dismisses the banner for exactly the boundaries it is currently reporting. */ + onDismiss: () => void; +}; + +/** + * Finds the draft's segment boundaries the loaded source text no longer carries — a loss that + * reverts those regions to one segment per verse, silently without this — and tracks which of them + * the user has dismissed the banner for. + * + * A dismissal names the boundaries it covered rather than setting a flag, so a later edit that + * strands further boundaries raises the banner again while a shrinking loss leaves it down. It is + * dropped when the draft is replaced wholesale, and singly when one recovers, on the grounds that + * either makes the next loss one the user has not seen. + */ +export default function useLostBoundaryDismissal({ + verseBook, + segmentation, + segmentationVersion, + draftVersion, + isDraftLoading, + isImportView, + useWebViewState, +}: LostBoundaryDismissalOptions): LostBoundaryDismissal { + // Keying on the draft itself would re-run the search after every gloss auto-save, which replaces + // its identity without touching the boundaries. + const lostBoundaries = useMemo( + () => (verseBook && !isImportView ? findLostBoundaries(verseBook, segmentation) : []), + // eslint-disable-next-line react-hooks/exhaustive-deps -- the version counters track segmentation, a ref value + [verseBook, segmentationVersion, draftVersion, isDraftLoading, isImportView], + ); + + /** + * The lost boundaries the user has dismissed the banner for — an acknowledgement of a message, + * tab-scoped rather than persisted into the draft alongside the analysis. + */ + const [dismissedLostBoundaries, setDismissedLostBoundaries] = useWebViewState( + 'dismissedLostBoundaries', + [], + ); + + const undismissedLostBoundaries = useMemo(() => { + const dismissed = new Set(dismissedLostBoundaries); + return lostBoundaries.filter((ref) => !dismissed.has(ref)); + }, [lostBoundaries, dismissedLostBoundaries]); + + /** + * The stored list spans the whole draft while a loaded book reports only its own boundaries, so a + * dismissal has to leave every other book's acknowledgement standing. + */ + const onDismiss = useCallback(() => { + setDismissedLostBoundaries([...new Set([...dismissedLostBoundaries, ...lostBoundaries])]); + }, [dismissedLostBoundaries, lostBoundaries, setDismissedLostBoundaries]); + + /** + * The draft the dismissal acknowledged, so a wholesale replacement drops it: the acknowledgement + * was of one draft's message, and a replacement stranding the same boundary is a loss the user + * has not seen. + */ + const dismissedDraftVersionRef = useRef(draftVersion); + /** Whether the dismissal has just been dropped wholesale, leaving nothing to drop singly. */ + const draftReplacedRef = useRef(false); + useEffect(() => { + // Skipping the mount pass leaves a dismissal restored with the tab in place. + if (dismissedDraftVersionRef.current === draftVersion) return; + dismissedDraftVersionRef.current = draftVersion; + draftReplacedRef.current = true; + setDismissedLostBoundaries([]); + }, [draftVersion, setDismissedLostBoundaries]); + + /** + * Drops a dismissed boundary once it recovers, so stranding it again raises the banner rather + * than carrying the earlier acknowledgement across the recovery. + * + * Only a recovery seen in this tab counts: a dismissal restored alongside the tab names + * boundaries from whatever the source looked like when it was made, so absence alone implies no + * recovery. + * + * A recovery is only observable in the book that owns the boundary: a book that never reports + * one, like an unloaded or import view, says nothing about whether it came back. + */ + const observedLostBoundariesRef = useRef(new Map()); + useEffect(() => { + const observableBookRef = isImportView ? undefined : verseBook?.bookRef; + if (!observableBookRef) return; + const observed = observedLostBoundariesRef.current; + const previous = observed.get(observableBookRef); + observed.set(observableBookRef, lostBoundaries); + if (draftReplacedRef.current) { + draftReplacedRef.current = false; + return; + } + if (previous === undefined) return; + const stillLost = new Set(lostBoundaries); + const recovered = previous.filter((ref) => !stillLost.has(ref)); + if (recovered.length === 0) return; + const recoveredSet = new Set(recovered); + setDismissedLostBoundaries(dismissedLostBoundaries.filter((ref) => !recoveredSet.has(ref))); + }, [ + lostBoundaries, + dismissedLostBoundaries, + setDismissedLostBoundaries, + verseBook, + isImportView, + ]); + + return { undismissedLostBoundaries, onDismiss }; +} diff --git a/src/parsers/papi/resegmentBook.ts b/src/parsers/papi/resegmentBook.ts index 5ae20fad6..c29206eac 100644 --- a/src/parsers/papi/resegmentBook.ts +++ b/src/parsers/papi/resegmentBook.ts @@ -7,7 +7,7 @@ import type { VerseStart, } from 'interlinearizer'; -import { effectiveStarts, isDefaultSegmentation } from '../../utils/segmentation'; +import { effectiveStarts, isDefaultSegmentationForBook } from '../../utils/segmentation'; /** Separator inserted between two verses' baseline text when they are merged into one segment. */ const MERGE_SEPARATOR = ' '; @@ -97,9 +97,9 @@ function buildSegment(run: SourcedToken[]): Segment { * Re-groups a verse-tokenized {@link Book} into the user's custom segments, without touching the * text-layer tokenizer. * - * The book is returned unchanged, by reference, for the default segmentation, so the common - * no-custom-boundaries case incurs no work and no identity churn. Otherwise the flat document-order - * token stream is cut at the delta's effective boundaries. A run that is exactly one original verse + * The book is returned unchanged, by reference, when the delta sets no boundary in it, so a book no + * boundary names incurs no work and no identity churn. Otherwise the flat document-order token + * stream is cut at the delta's effective boundaries. A run that is exactly one original verse * reuses that verse's segment verbatim, so analyses keep resolving and React memoization is * undisturbed; only merged or split runs are rebuilt, with baseline text and char offsets * recomputed so the `baselineText.slice(charStart, charEnd) === surfaceText` invariant still @@ -109,7 +109,7 @@ function buildSegment(run: SourcedToken[]): Segment { * they survive a custom segmentation exactly as they do the default one. */ export function resegmentBook(book: Book, delta: SegmentationDelta | undefined): Book { - if (isDefaultSegmentation(delta)) return book; + if (isDefaultSegmentationForBook(book, delta)) return book; const starts = effectiveStarts(book, delta); diff --git a/src/utils/segmentation.ts b/src/utils/segmentation.ts index 37f6c2c85..1514a4e29 100644 --- a/src/utils/segmentation.ts +++ b/src/utils/segmentation.ts @@ -6,6 +6,7 @@ * that is what the default verse starts are derived from, and returns a normalized delta. */ import type { Book, SegmentationDelta } from 'interlinearizer'; +import { bookOfRef } from './analysis-book'; /** An empty delta — equivalent to the default verse segmentation. */ const EMPTY_DELTA: SegmentationDelta = { removedVerseStarts: [], addedStarts: [] }; @@ -24,8 +25,12 @@ type BookLookups = Readonly<{ all: ReadonlySet; /** Document-order index for every token ref, used to keep delta arrays canonically sorted. */ order: ReadonlyMap; - /** The book's very first token ref — the start of the first segment, never merged leftward. */ - first: string | undefined; + /** + * The default starts a removal can actually merge leftward — those whose verse directly follows a + * token-bearing one. A verse opening the book or following a token-less verse marker has no + * preceding run to be absorbed into. + */ + mergeable: ReadonlySet; }>; /** @@ -40,22 +45,23 @@ function bookLookups(verseBook: Book): BookLookups { const defaults = new Set(); const all = new Set(); const order = new Map(); + const mergeable = new Set(); let i = 0; + let precededByTokens = false; verseBook.segments.forEach((seg) => { const firstToken = seg.tokens[0]; - if (firstToken) defaults.add(firstToken.ref); + if (firstToken) { + defaults.add(firstToken.ref); + if (precededByTokens) mergeable.add(firstToken.ref); + } seg.tokens.forEach((t) => { all.add(t.ref); order.set(t.ref, i); i += 1; }); + precededByTokens = seg.tokens.length > 0; }); - const lookups: BookLookups = { - defaults, - all, - order, - first: verseBook.segments[0]?.tokens[0]?.ref, - }; + const lookups: BookLookups = { defaults, all, order, mergeable }; bookLookupsCache.set(verseBook, lookups); return lookups; } @@ -71,53 +77,90 @@ export function defaultVerseStarts(verseBook: Book): ReadonlySet { /** * The token refs that begin a segment once the delta is applied to the default verse starts: * `(defaults \ removedVerseStarts) ∪ addedStarts`. Added anchors whose token no longer exists are - * dropped, and the book's first token is always forced to be a start. This is the single definition - * of where a segment begins, so no two boundary operations can disagree. + * dropped, and only a removal with a preceding run to merge into takes effect. This is the single + * definition of where a segment begins, so no two boundary operations can disagree. */ export function effectiveStarts( verseBook: Book, delta: SegmentationDelta | undefined, ): Set { - const { defaults, all, first } = bookLookups(verseBook); + const { defaults, all, mergeable } = bookLookups(verseBook); const removed = new Set(delta?.removedVerseStarts ?? []); const starts = new Set(); defaults.forEach((ref) => { - if (!removed.has(ref)) starts.add(ref); + // A start with nothing to merge leftward into survives its own removal. + if (!removed.has(ref) || !mergeable.has(ref)) starts.add(ref); }); if (delta) { delta.addedStarts.forEach((ref) => { if (all.has(ref)) starts.add(ref); }); } - // The first segment can never be merged away, so its start is always present. - if (first !== undefined) starts.add(first); return starts; } +/** + * A predicate per kind of delta entry, each answering whether this book's loaded source still lets + * that entry change where a segment begins — the single definition of that question. + * + * Drift unhonors an entry either by dropping its token or by moving the token into a role the entry + * no longer fits, which includes leaving a removal with no preceding run to merge into. + */ +function honorsAnchor({ defaults, all, mergeable }: BookLookups) { + return { + removal: (ref: string) => all.has(ref) && defaults.has(ref) && mergeable.has(ref), + addition: (ref: string) => all.has(ref) && !defaults.has(ref), + }; +} + /** * Canonicalizes a delta so that equal segmentations serialize identically: each array is deduped, - * stripped of no-op entries, and sorted by document order. + * stripped of no-op entries, and sorted. + * + * One delta spans every book of its draft, so anchors naming a book other than `verseBook` are + * carried through untouched — dropping them would delete boundaries the user set in a book they + * merely navigated away from. They sort after this book's. + * + * Anchors that this book's loaded source does not honor survive for the same reason — a drifted + * source may yet revert, and no edit elsewhere in the book should be what makes that loss + * permanent. + * + * Only this book's honored anchors have a document order to sort by; the two tails sort by ref. */ function normalize(verseBook: Book, delta: SegmentationDelta): SegmentationDelta { - const { defaults, all, order, first } = bookLookups(verseBook); + const lookups = bookLookups(verseBook); + const { order } = lookups; + const honors = honorsAnchor(lookups); const byOrder = (a: string, b: string) => /* v8 ignore next -- ?? 0 fallback for refs absent from order; filtered arrays only hold real refs */ (order.get(a) ?? 0) - (order.get(b) ?? 0); + const byRef = (a: string, b: string) => a.localeCompare(b); - const removedVerseStarts = [...new Set(delta.removedVerseStarts)] - .filter((ref) => defaults.has(ref) && ref !== first) - .sort(byOrder); - const addedStarts = [...new Set(delta.addedStarts)] - .filter((ref) => all.has(ref) && !defaults.has(ref)) - .sort(byOrder); + /** Orders this book's honored refs canonically, keeping the unhonored ones after them. */ + const canonicalize = (refs: string[], isHonored: (ref: string) => boolean) => { + const deduped = [...new Set(refs)]; + const mine = deduped.filter((ref) => bookOfRef(ref) === verseBook.bookRef); + const foreign = deduped.filter((ref) => bookOfRef(ref) !== verseBook.bookRef); + return [ + ...mine.filter(isHonored).sort(byOrder), + ...mine.filter((ref) => !isHonored(ref)).sort(byRef), + ...foreign.sort(byRef), + ]; + }; - return { removedVerseStarts, addedStarts }; + return { + removedVerseStarts: canonicalize(delta.removedVerseStarts, honors.removal), + addedStarts: canonicalize(delta.addedStarts, honors.addition), + }; } /** * Makes a token begin a segment — that is, splits before it. A default verse start that had been * merged away is un-merged; any other token is recorded as an added start. Already being a segment * start is a no-op. + * + * An edit at a ref is authoritative over any anchor drift has left there, so the token begins a + * segment whichever kind of anchor already named it. */ export function addBoundaryBefore( verseBook: Book, @@ -126,22 +169,20 @@ export function addBoundaryBefore( ): SegmentationDelta { const current = delta ?? EMPTY_DELTA; const { defaults } = bookLookups(verseBook); - if (defaults.has(ref)) { - return normalize(verseBook, { - removedVerseStarts: current.removedVerseStarts.filter((r) => r !== ref), - addedStarts: current.addedStarts, - }); - } - return normalize(verseBook, { - removedVerseStarts: current.removedVerseStarts, - addedStarts: [...current.addedStarts, ref], - }); + const removedVerseStarts = current.removedVerseStarts.filter((r) => r !== ref); + const addedStarts = current.addedStarts.filter((r) => r !== ref); + if (defaults.has(ref)) return normalize(verseBook, { removedVerseStarts, addedStarts }); + return normalize(verseBook, { removedVerseStarts, addedStarts: [...addedStarts, ref] }); } /** * Stops a token from beginning a segment, merging it into the preceding one. A default verse start - * is recorded as removed; a previously added split is dropped. Merging the book's first token is a - * no-op, since the first segment cannot merge leftward. + * is recorded as removed; a previously added split is dropped. Removing a default start with + * nothing to merge into is a no-op, which covers the book's first verse and any verse following a + * token-less verse marker. + * + * An edit at a ref is authoritative over any anchor drift has left there, so the token stops + * beginning a segment whichever kind of anchor already named it. */ export function removeBoundaryAt( verseBook: Book, @@ -149,18 +190,14 @@ export function removeBoundaryAt( ref: string, ): SegmentationDelta { const current = delta ?? EMPTY_DELTA; - const { defaults, first } = bookLookups(verseBook); - if (ref === first) return normalize(verseBook, current); - if (defaults.has(ref)) { - return normalize(verseBook, { - removedVerseStarts: [...current.removedVerseStarts, ref], - addedStarts: current.addedStarts, - }); - } - return normalize(verseBook, { - removedVerseStarts: current.removedVerseStarts, - addedStarts: current.addedStarts.filter((r) => r !== ref), - }); + const lookups = bookLookups(verseBook); + const { defaults, mergeable } = lookups; + if (defaults.has(ref) && !mergeable.has(ref)) return normalize(verseBook, current); + const removedVerseStarts = current.removedVerseStarts.filter((r) => r !== ref); + const addedStarts = current.addedStarts.filter((r) => r !== ref); + if (defaults.has(ref)) + return normalize(verseBook, { removedVerseStarts: [...removedVerseStarts, ref], addedStarts }); + return normalize(verseBook, { removedVerseStarts, addedStarts }); } /** @@ -179,8 +216,8 @@ export function moveBoundary( /** * Merges a segment into the one before it, identified by the first-token ref of the _second_ * segment — the one absorbed into its predecessor. Clearing that token's segment start is the whole - * operation, so merging the book's first token is a no-op; the separate name states the merge - * intent. + * operation, so a segment with no predecessor to merge into is a no-op; the separate name states + * the merge intent. */ export function mergeSegments( verseBook: Book, @@ -203,7 +240,53 @@ export function splitSegmentBefore( return addBoundaryBefore(verseBook, delta, ref); } -/** Whether the delta represents the default verse segmentation: absent, or both arrays empty. */ -export function isDefaultSegmentation(delta: SegmentationDelta | undefined): boolean { +/** + * Whether the delta records no boundary edit at all, in any book: absent, or both arrays empty. + * Such a delta leaves every book on the default verse segmentation. + */ +export function isEmptyDelta(delta: SegmentationDelta | undefined): boolean { return !delta || (delta.removedVerseStarts.length === 0 && delta.addedStarts.length === 0); } + +/** + * Whether the delta leaves `verseBook` on the default verse segmentation. One delta spans every + * book of its draft, so a delta that is custom overall may still say nothing about this book. + */ +export function isDefaultSegmentationForBook( + verseBook: Book, + delta: SegmentationDelta | undefined, +): boolean { + if (!delta) return true; + return ![...delta.removedVerseStarts, ...delta.addedStarts].some( + (ref) => bookOfRef(ref) === verseBook.bookRef, + ); +} + +/** + * The user's boundaries the loaded book no longer carries, named by the delta ref that recorded + * each, in delta order — the boundaries {@link effectiveStarts} silently drops, which a reversified + * or upstream-edited source produces because both re-key the token refs the delta is written + * against. + * + * What counts is whether the boundary is absent, not whether its delta entry still changes + * anything: a merge stranded mid-verse is lost, while a split whose token has become a verse start + * is merely redundant. + * + * Only refs naming `verseBook` are considered, one delta spanning every book of its draft. The + * delta is left intact either way, so a source that reverts brings its boundaries back. + */ +export function lostBoundaries( + verseBook: Book, + delta: SegmentationDelta | undefined, +): readonly string[] { + if (!delta) return []; + const lookups = bookLookups(verseBook); + const honors = honorsAnchor(lookups); + const { all } = lookups; + const isMine = (ref: string) => bookOfRef(ref) === verseBook.bookRef; + return [ + ...delta.removedVerseStarts.filter((ref) => isMine(ref) && !honors.removal(ref)), + // A token that has become a default start carries the boundary itself. + ...delta.addedStarts.filter((ref) => isMine(ref) && !all.has(ref)), + ]; +}