Audit docs, clean up tests, minor adjustments

Author: alex-rawlings-yyc

src/__tests__/components/ArcOverlay.test.tsx

@@ -367,9 +367,24 @@ describe('ArcOverlay', () => {
       />,
     );
     const btns = screen.getAllByTestId('split-arc-btn');
-    // Hover the first split button — tok-b's arc goes through focusedPhraseId branch (returns 2).
+    // Hover the first split button — tok-b's arc goes through focusedPhraseId branch.
     await userEvent.hover(btns[0]);
-    expect(document.querySelectorAll('path')).toHaveLength(2);
+
+    // The free-split-hovered tok-a arc is promoted to the z-5 (hovered) layer.
+    const hoveredLayer = document.querySelector('svg.tw\\:z-5');
+    const hoveredPaths = hoveredLayer?.querySelectorAll('path') ?? [];
+    expect(hoveredPaths).toHaveLength(1);
+    expect(hoveredPaths[0].getAttribute('d')).toContain('tok-a');
+
+    // tok-b's arc is classified focused via the focusedPhraseId branch, so it sits in the z-3
+    // (focused) layer — not the z-1 unfocused layer it would fall to if that branch regressed.
+    const focusedLayer = document.querySelector('svg.tw\\:z-3');
+    const focusedPaths = focusedLayer?.querySelectorAll('path') ?? [];
+    expect(focusedPaths).toHaveLength(1);
+    expect(focusedPaths[0].getAttribute('d')).toContain('tok-b');
+
+    // Nothing falls through to the unfocused (z-1) layer.
+    expect(document.querySelector('svg.tw\\:z-1')?.querySelectorAll('path')).toHaveLength(0);
   });
 
   it('assigns candidate priority to an arc via candidatePhraseIds when split-hover misses its splitAfterTokenRef and focusedPhraseId does not match', async () => {
@@ -395,7 +410,21 @@ describe('ArcOverlay', () => {
     );
     const btns = screen.getAllByTestId('split-arc-btn');
     await userEvent.hover(btns[0]);
-    expect(document.querySelectorAll('path')).toHaveLength(2);
+
+    // Both arcs land in the z-5 (hovered) layer: tok-a via the free-split promotion, and tok-b via
+    // the candidatePhraseIds.has('p1') branch. If that candidate clause regressed, tok-b would drop
+    // to the z-1 (unfocused) layer and would no longer appear here.
+    const hoveredLayer = document.querySelector('svg.tw\\:z-5');
+    const hoveredDs = Array.from(hoveredLayer?.querySelectorAll('path') ?? []).map((p) =>
+      p.getAttribute('d'),
+    );
+    expect(hoveredDs).toHaveLength(2);
+    expect(hoveredDs).toEqual(
+      expect.arrayContaining([expect.stringContaining('tok-a'), expect.stringContaining('tok-b')]),
+    );
+
+    // No arc is demoted to the unfocused (z-1) layer.
+    expect(document.querySelector('svg.tw\\:z-1')?.querySelectorAll('path')).toHaveLength(0);
   });
 
   it('highlights the phrase via onHoverPhrase on enter when the split would not free any token', async () => {

src/__tests__/components/ContinuousView.test.tsx

@@ -88,12 +88,22 @@ jest.mock('../../components/TokenLinkIcon', () => ({
 
 jest.mock('../../components/ArcOverlay', () => ({
   __esModule: true,
+  // Surface the props ContinuousView derives and forwards (hoveredPhraseId, candidatePhraseIds) as
+  // data attributes so DOM queries can assert on values that otherwise only live inside ArcOverlay.
   default: ({
     onArcSplit,
-  }: Readonly<{ onArcSplit: (phraseId: string, splitAfterTokenRef: string) => void }>) => (
+    hoveredPhraseId,
+    candidatePhraseIds,
+  }: Readonly<{
+    onArcSplit: (phraseId: string, splitAfterTokenRef: string) => void;
+    hoveredPhraseId: string | undefined;
+    candidatePhraseIds: ReadonlySet<string>;
+  }>) => (
     <button
       type="button"
       data-testid="arc-split-btn"
+      data-hovered-phrase-id={hoveredPhraseId ?? ''}
+      data-candidate-phrase-ids={[...candidatePhraseIds].join(',')}
       onClick={() => onArcSplit('phrase-1', 'tok-0')}
     >
       split
@@ -1267,26 +1277,69 @@ describe('ContinuousView phrase grouping', () => {
     expect(phraseBoxes[1]).toHaveAttribute('data-show-gloss', 'false');
   });
 
-  it('fires mouse-leave on the token strip without throwing', async () => {
+  it('clears the hovered phrase highlight when the pointer leaves the token strip', async () => {
+    // Group tok-0/tok-1 into one hoverable phrase so hovering it sets hoveredPhraseId, which
+    // ContinuousView forwards to ArcOverlay. Leaving the strip runs clearAllHoverState, which must
+    // reset hoveredPhraseId to undefined.
+    const phraseLink: PhraseAnalysisLink = {
+      analysisId: 'phrase-1',
+      status: 'approved',
+      tokens: [
+        { tokenRef: 'tok-0', surfaceText: 'In' },
+        { tokenRef: 'tok-1', surfaceText: 'the' },
+      ],
+    };
+    phraseLinkMap.set('tok-0', phraseLink);
+    phraseLinkMap.set('tok-1', phraseLink);
     const book = makeBook();
     render(<ContinuousView {...requiredProps(book)} />, withAnalysisStore);
-    const strip = screen.getByTestId('token-strip');
-    await userEvent.unhover(strip);
-    // No throw = pass
+
+    // Hover the phrase group to set hoveredPhraseId='phrase-1'.
+    const phraseGroupSpan = document.querySelector('[data-phrase-box="true"]')?.parentElement;
+    if (!phraseGroupSpan) throw new Error('Expected a phrase group wrapper span');
+    await userEvent.hover(phraseGroupSpan);
+    expect(screen.getByTestId('arc-split-btn')).toHaveAttribute(
+      'data-hovered-phrase-id',
+      'phrase-1',
+    );
+
+    // Leaving the strip itself (not the group) must clear the highlight via clearAllHoverState.
+    fireEvent.mouseLeave(screen.getByTestId('token-strip'));
+    expect(screen.getByTestId('arc-split-btn')).toHaveAttribute('data-hovered-phrase-id', '');
   });
 
   it('applies the internal focus transition when the parent reflects a click-driven ref change', async () => {
     // Simulate: ContinuousView clicks Next, sets internalFocusedTokenRefRef, calls
     // onFocusedTokenRefChange. The parent then passes the new focusedTokenRef back. This exercises
-    // the isInternal=true path (lines 306-308) of the pending-jump effect.
+    // the isInternal=true branch of the focus-change effect, which applies the new ref *immediately*
+    // (setDisplayFocusedTokenRef) without fading the strip out. The external (non-internal) branch
+    // would instead defer the display update behind a fade timeout, so the focused box would still
+    // be 'In' (tok-0) right after the rerender.
     const book = makeBook();
     const props = requiredProps(book, { focusedTokenRef: 'tok-0' });
     const { rerender } = render(<ContinuousView {...props} />, withAnalysisStore);
 
+    // Sanity: tok-0's box ('In') is focused before the click.
+    expect(screen.getByText('In').closest('[data-phrase-box="true"]')).toHaveAttribute(
+      'data-focus-state',
+      'focused',
+    );
+
     await userEvent.click(screen.getByRole('button', { name: 'Next token' }));
-    // Now reflect the new ref back as a prop change (as a real parent would do).
+    // Now reflect the new ref back as a prop change (as a real parent would do). Because the click
+    // stamped tok-1 as internally-originated, the echo is recognized as internal and applied at once.
     rerender(<ContinuousView {...props} focusedTokenRef="tok-1" />);
-    // No throw = the isInternal path ran successfully.
+
+    // The displayed focus moved synchronously to tok-1's box ('the') — the internal path, not the
+    // fade-then-snap external path (which would leave 'In' focused until the fade timeout fires).
+    expect(screen.getByText('the').closest('[data-phrase-box="true"]')).toHaveAttribute(
+      'data-focus-state',
+      'focused',
+    );
+    expect(screen.getByText('In').closest('[data-phrase-box="true"]')).toHaveAttribute(
+      'data-focus-state',
+      'default',
+    );
   });
 
   it('scrolls to the first token of the active phrase when entering edit mode', async () => {
@@ -1400,6 +1453,25 @@ describe('ContinuousView phrase grouping', () => {
     mockCandidateTokenRefs.current = new Set(['tok-0']);
     const book = makeBook();
     render(<ContinuousView {...requiredProps(book)} />, withAnalysisStore);
-    expect(screen.getByTestId('arc-split-btn')).toBeInTheDocument();
+    // useCandidatePhraseIds resolves the hovered candidate ref (tok-0) to the phrase that contains
+    // it, and ContinuousView forwards the set to ArcOverlay. The mock surfaces it as a data attr.
+    expect(screen.getByTestId('arc-split-btn')).toHaveAttribute(
+      'data-candidate-phrase-ids',
+      'phrase-1',
+    );
+  });
+
+  it('computes an empty candidatePhraseIds set when no candidate tokens are hovered', () => {
+    const phraseLink: PhraseAnalysisLink = {
+      analysisId: 'phrase-1',
+      status: 'approved',
+      tokens: [{ tokenRef: 'tok-0', surfaceText: 'In' }],
+    };
+    phraseLinkMap.set('tok-0', phraseLink);
+    // No hovered candidate refs: the phrase exists, but nothing should resolve to it.
+    mockCandidateTokenRefs.current = new Set();
+    const book = makeBook();
+    render(<ContinuousView {...requiredProps(book)} />, withAnalysisStore);
+    expect(screen.getByTestId('arc-split-btn')).toHaveAttribute('data-candidate-phrase-ids', '');
   });
 });

src/__tests__/components/Interlinearizer.test.tsx

@@ -1064,7 +1064,13 @@ describe('Interlinearizer', () => {
         throw new Error('Expected GEN 1:7 onSelect to be a function');
       act(() => select({ book: 'GEN', chapter: 1, verse: 7 }, 'GEN 1:7:0'));
 
-      expect(container.querySelector('.tw\\:transition-opacity')).toHaveStyle({ opacity: '1' });
+      // Target the inner list wrapper (tw:gap-2) — the one that fades on external nav via isFaded —
+      // matching the positive-fade test above. The bare .tw:transition-opacity selector would return
+      // the outer mode-toggle wrapper instead, whose opacity is never driven here, masking a regressed
+      // (non-suppressed) recenter fade of the list.
+      expect(container.querySelector('.tw\\:gap-2.tw\\:transition-opacity')).toHaveStyle({
+        opacity: '1',
+      });
     } finally {
       jest.useRealTimers();
     }

src/__tests__/components/InterlinearizerLoader.test.tsx

@@ -241,6 +241,7 @@ jest.mock('../../components/modals/ProjectModals', () => ({
         data-testid="project-modals"
         data-modal={modal}
         data-default-lang={defaultAnalysisLanguage}
+        data-active-project-name={activeProject?.name}
       >
         {modal === 'select' && (
           <div data-testid="select-modal">
@@ -313,7 +314,10 @@ jest.mock('../../components/modals/ProjectModals', () => ({
             <button
               type="button"
               data-testid="metadata-modal-saved"
-              onClick={() => setModal('none')}
+              onClick={() => {
+                setActiveProject({ ...STUB_ACTIVE_PROJECT, name: 'Renamed Project' });
+                setModal('none');
+              }}
             >
               Save
             </button>
@@ -834,6 +838,9 @@ describe('InterlinearizerLoader', () => {
       await userEvent.click(screen.getByTestId('metadata-modal-deleted'));
 
       expect(screen.queryByTestId('metadata-modal')).not.toBeInTheDocument();
+      // The deleted project was the active one, so the loader should now pass `activeProject:
+      // undefined` down to ProjectModals (the stub omits the attribute when there is no name).
+      expect(screen.getByTestId('project-modals')).not.toHaveAttribute('data-active-project-name');
     });
 
     it('updates the active project name when its metadata is saved', async () => {
@@ -847,6 +854,12 @@ describe('InterlinearizerLoader', () => {
       await userEvent.click(screen.getByTestId('metadata-modal-saved'));
 
       expect(screen.queryByTestId('metadata-modal')).not.toBeInTheDocument();
+      // Saving renamed the active project; the loader must reflect the new name it reads back from
+      // WebView state by passing it down to ProjectModals.
+      expect(screen.getByTestId('project-modals')).toHaveAttribute(
+        'data-active-project-name',
+        'Renamed Project',
+      );
     });
 
     it('renders without error when useData provides a topMenu with items', async () => {

src/__tests__/components/MorphemeEditor.test.tsx

@@ -53,7 +53,12 @@ describe('MorphemeBreakdownPopover', () => {
 
   it('auto-focuses and selects the input on mount', () => {
     renderPopover({ initialValue: 'word' });
-    expect(screen.getByRole('textbox')).toHaveFocus();
+    const input = screen.getByRole('textbox');
+    expect(input).toHaveFocus();
+    // The mount effect calls .select(), so the whole value is selected and a fresh keystroke
+    // replaces it. Asserting the selection range catches a regression that drops the .select() call.
+    expect(input).toHaveProperty('selectionStart', 0);
+    expect(input).toHaveProperty('selectionEnd', 'word'.length);
   });
 
   it('calls onSave and onClose when Done button is clicked', async () => {
@@ -150,7 +155,13 @@ describe('MorphemeBreakdownPopover', () => {
 
   it('does not save on outside interaction when the input is only whitespace', async () => {
     const onSave = jest.fn();
-    renderPopover({ initialValue: '   ', onSave });
+    // Start from a real word and edit it down to whitespace so the draft differs from initialValue
+    // (isUnedited is false). This forces handleInteractOutside past the unedited guard into
+    // handleSave, where the isMeaningless check is what rejects the empty breakdown — the behavior
+    // this test names. If isMeaningless were removed, handleSave would call onSave and this fails.
+    renderPopover({ initialValue: 'word', onSave, surfaceText: 'whole' });
+    await userEvent.clear(screen.getByRole('textbox'));
+    await userEvent.type(screen.getByRole('textbox'), '   ');
     await userEvent.click(screen.getByTestId('popover-outside'));
     expect(onSave).not.toHaveBeenCalled();
   });

src/__tests__/components/PhraseBox.test.tsx

@@ -933,10 +933,13 @@ describe('PhraseBox', () => {
     renderBox(<PhraseBox {...requiredProps()} />);
     const box = document.querySelector('[data-phrase-box="true"]');
     expect(box).not.toBeNull();
-    // Focus the box container, then press Enter → should focus the first input.
+    // Focus the box container, then press Enter → the keydown handler forwards focus to the first
+    // gloss input. Asserting toHaveFocus makes the test fail if the Enter branch of
+    // focusFirstGlossOnSelfKeyDown is broken or removed.
     if (box instanceof HTMLElement) box.focus();
     await userEvent.keyboard('{Enter}');
-    // No throw = pass (jsdom doesn't fire input focus in this setup, but the handler runs).
+
+    expect(screen.getByRole('textbox', { name: 'Gloss for Hello' })).toHaveFocus();
   });
 
   it('pops out a middle token from a 3+ token phrase in view mode (updatePhrase)', async () => {
@@ -1049,19 +1052,6 @@ describe('PhraseBox', () => {
     expect(screen.getByRole('button', { name: 'Remove World' })).toBeInTheDocument();
   });
 
-  it('pops out a token from a 3-token phrase by deleting when only 1 would remain (edge case)', async () => {
-    // Actually a 3-token phrase removes a middle token to leave 2 (≥2), so updatePhrase is called.
-    // For deletePhrase to be called, we need to go from 2 tokens to ≤1.
-    // The onRemove is only wired for middle tokens when length > 2. So we simulate handleViewPopOut
-    // via the phraseLink having exactly 2 tokens (which means the mock won't show Remove button).
-    // Instead, let's test the deletePhrase path by using a 3-token phrase removing to leave 1 token
-    // by mocking phraseLink.tokens to have 2 entries while tokens prop has 3, and removing the first.
-    // Actually, the easiest way is to test through phraseLink with 2 tokens where the condition
-    // doesn't show the Remove button. Skip this path via v8 ignore instead.
-    // This test just documents the behavior.
-    expect(true).toBe(true);
-  });
-
   it('writes phrase gloss on blur when draft differs from committed', async () => {
     mockUsePhraseGloss.mockReturnValue('');
     const dispatchSpy = jest.fn();
@@ -1075,12 +1065,14 @@ describe('PhraseBox', () => {
   });
 
   it('ignores non-Enter/Space keys on the box container', async () => {
-    const setPhraseMode = jest.fn();
-    renderBox(<PhraseBox {...requiredProps()} />, { setPhraseMode });
+    renderBox(<PhraseBox {...requiredProps()} />);
     const box = document.querySelector('[data-phrase-box="true"]');
     if (box instanceof HTMLElement) box.focus();
-    await userEvent.keyboard('{Tab}');
-    expect(setPhraseMode).not.toHaveBeenCalled();
+    // ArrowRight is neither Enter nor Space and does not move focus by default, so
+    // focusFirstGlossOnSelfKeyDown must return early without focusing the first gloss input. If the
+    // Enter/Space guard were dropped, this key would forward focus to the gloss input and fail here.
+    await userEvent.keyboard('{ArrowRight}');
+    expect(screen.getByRole('textbox', { name: 'Gloss for Hello' })).not.toHaveFocus();
   });
 
   it('renders disabled style for a token in the wrong segment during edit mode', () => {

src/__tests__/components/PhraseStripParts.test.tsx

@@ -22,7 +22,9 @@ import { makePhraseLink, makePhraseStripContext, makeWordToken } from '../test-h
 
 jest.mock('../../components/TokenLinkIcon', () => ({
   __esModule: true,
-  default: () => <span data-testid="link-icon" />,
+  default: ({ isPhraseRevealed }: Readonly<{ isPhraseRevealed: boolean }>) => (
+    <span data-testid="link-icon" data-phrase-revealed={String(isPhraseRevealed)} />
+  ),
 }));
 
 jest.mock('../../components/TokenChip', () => ({
@@ -155,12 +157,29 @@ describe('PhraseSlot', () => {
       punctuationBetween: [],
     };
     const slot: LinkSlot = { prevGroup, nextGroup, punctuation: [] };
-    // PhrasedRevealed means the unlink button is shown — but TokenLinkIcon is mocked to undefined,
-    // so just check no errors are thrown when hoveredPhraseId matches.
-    const { container } = render(
-      withProvider(<PhraseSlot {...slotProps(slot)} hoveredPhraseId="p1" />),
-    );
-    expect(container.firstChild).not.toBeNull();
+    // phraseRevealed (source lines 70-73) flows into TokenLinkIcon as isPhraseRevealed; the mock
+    // surfaces it as data-phrase-revealed so we can assert the computation directly.
+    render(withProvider(<PhraseSlot {...slotProps(slot)} hoveredPhraseId="p1" />));
+    expect(screen.getByTestId('link-icon')).toHaveAttribute('data-phrase-revealed', 'true');
+  });
+
+  it('does not set phraseRevealed when the hovered phrase differs from the neighbors', () => {
+    const link = makePhraseLink('p1', ['tok-a', 'tok-b']);
+    const prevGroup: TokenGroup = {
+      tokens: [makeWordToken('tok-a')],
+      phraseLink: link,
+      firstIndex: 0,
+      punctuationBetween: [],
+    };
+    const nextGroup: TokenGroup = {
+      tokens: [makeWordToken('tok-b')],
+      phraseLink: link,
+      firstIndex: 1,
+      punctuationBetween: [],
+    };
+    const slot: LinkSlot = { prevGroup, nextGroup, punctuation: [] };
+    render(withProvider(<PhraseSlot {...slotProps(slot)} hoveredPhraseId="other-phrase" />));
+    expect(screen.getByTestId('link-icon')).toHaveAttribute('data-phrase-revealed', 'false');
   });
 
   it('sets phraseRevealed via focusedPhraseId when both neighbors are in the same focused phrase', () => {
@@ -185,12 +204,14 @@ describe('PhraseSlot', () => {
       focusedSegmentId: 'seg-1',
       focusedPhraseId: 'p1',
     };
-    const { container } = render(
+    // With no hover, the only way phraseRevealed becomes true is the focus.focusedPhraseId branch
+    // (source line 73); the mock exposes it via data-phrase-revealed.
+    render(
       withProvider(
         <PhraseSlot {...slotProps(slot)} focus={focusedContext} hoveredPhraseId={undefined} />,
       ),
     );
-    expect(container.firstChild).not.toBeNull();
+    expect(screen.getByTestId('link-icon')).toHaveAttribute('data-phrase-revealed', 'true');
   });
 
   it('renders the link icon when hideInactiveLinkButtons is off', () => {
@@ -228,7 +249,6 @@ describe('PhraseSlot', () => {
     );
     // Icon stays mounted but invisible (opacity:0 hides it while the min-height preserves layout space).
     const icon = screen.getByTestId('link-icon');
-    expect(icon.parentElement?.style.visibility).toBeFalsy();
     expect(icon.parentElement?.style.opacity).toBe('0');
   });
 
@@ -274,7 +294,6 @@ describe('PhraseSlot', () => {
     );
     // Icon stays mounted but invisible (opacity:0 hides it while the min-height preserves layout space).
     const icon = screen.getByTestId('link-icon');
-    expect(icon.parentElement?.style.visibility).toBeFalsy();
     expect(icon.parentElement?.style.opacity).toBe('0');
   });
 });