Merge pull request #133 from sillsdev/se2

Minor suggested cleanup (#133)

Author: alex-rawlings-yyc

src/__tests__/store/analysisSlice.test.ts

@@ -11,8 +11,6 @@ import type {
   TokenSnapshot,
 } from 'interlinearizer';
 import { createAnalysisStore } from '../../store';
-import { makePhraseLink } from '../test-helpers';
-import { emptyAnalysis } from '../../types/empty-factories';
 import {
   approveAnalysisForToken,
   createPhrase,
@@ -35,6 +33,8 @@ import {
   writeSegmentFreeTranslation,
   type AnalysisState,
 } from '../../store/analysisSlice';
+import { emptyAnalysis } from '../../types/empty-factories';
+import { makePhraseLink } from '../test-helpers';
 
 /**
  * Builds an approved `TokenAnalysisLink` for `tok-1` pointing at the given `TokenAnalysis`.
@@ -65,10 +65,8 @@ function makeAnalysis(ta: TokenAnalysis): TextAnalysis {
 }
 
 /**
- * Counts how many approved `TokenAnalysisLink`s point at the same payload that `tokenRef`'s own
- * approved link points at — the shared count of the payload a token sits on. A local test helper
- * standing in for the former production selector, used to assert sharing without reaching into the
- * link arrays at each call site.
+ * Counts approved links on the same payload as `tokenRef`'s own approved link, used to assert
+ * sharing without reaching into the link arrays at each call site.
  *
  * @param state - The analysis slice state.
  * @param tokenRef - The `Token.ref` whose payload's shared count is wanted.

src/components/AnalysisStore.tsx

@@ -8,6 +8,7 @@ import type {
 import { createContext, useCallback, useContext, useEffect, useMemo, useRef } from 'react';
 import type { ReactNode } from 'react';
 import { Provider as ReduxProvider, useDispatch, useSelector, useStore } from 'react-redux';
+import { createAnalysisStore, type AnalysisDispatch, type AnalysisRootState } from '../store';
 import {
   approveAnalysisForToken,
   createPhrase,
@@ -30,7 +31,6 @@ import {
   writePhraseGloss,
   writeSegmentFreeTranslation,
 } from '../store/analysisSlice';
-import { createAnalysisStore, type AnalysisDispatch, type AnalysisRootState } from '../store';
 import { emptyAnalysis } from '../types/empty-factories';
 import { resolvedTokenAnalysisEqual, type ResolvedTokenAnalysis } from '../utils/suggestion-engine';
 
@@ -166,14 +166,11 @@ export function AnalysisStoreProvider({
   }, []);
 
   /**
-   * The gloss-write entry point exposed to inputs. Dispatches {@link writeGloss} so the reducer
-   * applies a per-token edit — it forks a shared payload before writing (blank clears, non-blank
-   * edits), so editing one token never rewrites its co-linked siblings — then persists the updated
-   * analysis via `onSave` and notifies the `onGlossChange` spy with the edited token and value.
+   * Dispatches {@link writeGloss} (forking any shared payload before writing), then persists via
+   * `onSave` and notifies the `onGlossChange` spy.
    *
    * @param tokenRef - `Token.ref` of the token being glossed.
-   * @param surfaceText - Surface text of the token, recorded so the analysis can detect baseline
-   *   drift.
+   * @param surfaceText - Surface text, recorded so the analysis can detect baseline drift.
    * @param value - New gloss string; blank (empty or whitespace) clears the active language's
    *   gloss.
    */
@@ -286,14 +283,13 @@ export function useGloss(tokenRef: string): string {
 /**
  * Returns the merged analysis the renderer shows for a token — its approved decision, else the
  * engine's derived suggestion, else `undefined` ({@link selectResolvedTokenAnalysis}). Subscribes
- * through {@link resolvedTokenAnalysisEqual} so the freshly-allocated result stays referentially
- * stable across unrelated store changes: the token re-renders only when its approved decision or
- * suggestion actually changes, never on every pool rebuild.
+ * through {@link resolvedTokenAnalysisEqual} so the result stays referentially stable across
+ * unrelated store changes, re-rendering only when the decision or suggestion actually changes.
  *
  * When `enabled` is `false` the selector short-circuits to `undefined` without consulting the pool,
  * so a chip that is not currently showing suggestions does no per-token pool lookup or
- * normalization on each store change. The only consumer ({@link TokenChip}) reads `resolved` solely
- * to render suggestions, so it passes its `showSuggestions` flag here.
+ * normalization on each store change. The only consumer ({@link TokenChip}) passes its
+ * `showSuggestions` flag here.
  *
  * @param tokenRef - The `Token.ref` to resolve.
  * @param surfaceText - The token's current surface text, matched against the pool when the token is

src/components/SuggestionDropdown.tsx

@@ -7,8 +7,8 @@
  */
 import { useLayoutEffect, useRef, useState } from 'react';
 import { createPortal } from 'react-dom';
-import type { GlossedSuggestionEntry } from '../utils/suggestion-engine';
 import { statusTextColorClass } from '../utils/status-colors';
+import type { GlossedSuggestionEntry } from '../utils/suggestion-engine';
 
 /** Props for {@link SuggestionDropdown}. */
 type SuggestionDropdownProps = Readonly<{
@@ -41,13 +41,9 @@ type SuggestionDropdownProps = Readonly<{
 /**
  * Renders the portaled suggestion listbox for a token's gloss combobox. Each row is colored and
  * labeled by its own `status` — `'suggested'` (green, "accept") or `'candidate'` (blue, "promote")
- * — carried on the entry rather than inferred from row position, so a dropped blank-in-language
- * pick can never leave a candidate masquerading as the accept row. The keyboard-active row gets the
- * same `bg-accent` background hovering applies, and hovering a row sets the active index so only
- * one row is ever highlighted. Each row suppresses its mouse-down default so clicking it never
- * blurs the input — focus stays in the input and the click selects instead. The panel closes itself
- * on outer scrolling (the anchor would drift); scrolling the panel's own overflow is ignored so a
- * long list can be scrolled without dismissing it.
+ * — carried on the entry rather than inferred from position, so a dropped blank-in-language pick
+ * can never leave a candidate masquerading as the accept row. Each row suppresses its mouse-down
+ * default so clicking it never blurs the input.
  *
  * @param props - Component props (see {@link SuggestionDropdownProps}).
  * @returns A `document.body` portal containing the listbox, positioned under the anchor input.
@@ -77,15 +73,12 @@ export default function SuggestionDropdown({
     listRef.current = el ?? undefined;
   };
 
-  // Position the panel under the anchor on open and keep it glued there across window resizes and
-  // outer scrolling. The continuous view smooth-scrolls the token strip to center a phrase whenever
-  // a token's gloss input is focused — the same focus that opens this dropdown — so closing on outer
-  // scroll would dismiss the panel the instant it appeared. Instead we reposition under the anchor as
-  // it moves, then close only once the anchor has scrolled out of the viewport (a far user scroll
-  // that abandons this token). A capture listener catches scrolls of ancestor viewports; scrolls of
-  // the panel's own overflow are ignored so a long list can be scrolled without moving or closing it.
-  // Layout effect so the first measurement runs before paint — otherwise the portaled panel flashes
-  // at the default top-left before snapping under the anchor.
+  // Position the panel under the anchor and keep it glued there across resizes and outer scrolling.
+  // The continuous view smooth-scrolls the token strip on focus — the same focus that opens this
+  // dropdown — so we reposition as the anchor moves rather than close immediately; we close only
+  // once the anchor leaves the viewport (a far scroll that abandons this token). Layout effect so
+  // the first measurement runs before paint — otherwise the portaled panel flashes at the default
+  // top-left before snapping under the anchor.
   useLayoutEffect(() => {
     const anchor = anchorRef.current;
     /* v8 ignore next -- the chip only mounts this while the input (the anchor) is rendered */

src/components/TokenChip.tsx

@@ -13,6 +13,8 @@ import {
   useRef,
   useState,
 } from 'react';
+import { resolvedOrEmpty } from '../utils/localized-strings';
+import { glossedSuggestionEntries } from '../utils/suggestion-engine';
 import {
   useAnalysisLanguage,
   useApproveAnalysisDispatch,
@@ -27,8 +29,6 @@ import {
 } from './AnalysisStore';
 import { MorphemeBreakdownPopover, MorphemeGlossInput } from './MorphemeEditor';
 import SuggestionDropdown from './SuggestionDropdown';
-import { resolvedOrEmpty } from '../utils/localized-strings';
-import { glossedSuggestionEntries } from '../utils/suggestion-engine';
 
 const STRING_KEYS = [
   '%interlinearizer_tokenChip_editMorphemes%',
@@ -174,58 +174,50 @@ export function TokenChip({
 
   const hasMorphemes = morphemes.length > 0;
 
-  // The pool entries to offer via the suggestion dropdown, with their accept/promote status. The
-  // engine flattens the resolved read into ranked, status-tagged rows: for a suggested token its
-  // pick (green "accept") plus candidates (blue "promote"); for an approved token the pool
-  // alternatives only (all blue "promote", the already-approved payload excluded). Blank-in-active-
-  // language entries are dropped individually rather than shown as empty rows (see
-  // `user-questions.md` display #3). Memoized on the (reference-stable) resolved read and active
-  // language so typing a gloss — which only changes local draft state — never re-runs the flatten/
-  // filter; it recomputes only when the resolved read or language actually changes.
+  // Pool entries for the suggestion dropdown: for a suggested token, the top pick (green "accept")
+  // plus candidates (blue "promote"); for an approved token, the pool alternatives only (the
+  // already-approved payload excluded). Blank-in-active-language entries are dropped rather than
+  // shown as empty rows. Memoized on the (reference-stable) resolved read and active language so
+  // typing a gloss — which only changes local draft state — never re-runs the flatten/filter.
   const glossedRanked = useMemo(
     () => glossedSuggestionEntries(resolved, analysisLanguage),
     [resolved, analysisLanguage],
   );
   // Whether this token has anything to suggest: gated on the demo toggle (via the resolve short-
   // circuit) and editability. The chevron and dropdown only ever appear when this is true.
   const hasSuggestions = showSuggestions && !disabled && glossedRanked.length > 0;
-  // The engine's top pick (row 0, the green "suggested"), shown as ghost placeholder text in the
-  // empty input so the row reveals at a glance which tokens already have a suggestion — without
-  // needing focus or hover. Only meaningful while the draft is empty; once the user types, the
-  // typed value replaces it. Undefined when this token has no suggestion to offer.
+  // Top pick (row 0, the green "suggested") shown as ghost placeholder text so the row reveals
+  // which tokens have a suggestion at a glance — without focus or hover. Once the user types, the
+  // typed value replaces it.
   const suggestedGloss = hasSuggestions ? glossedRanked[0].gloss : undefined;
   const showSuggestedPlaceholder = suggestedGloss !== undefined && draft === '';
 
   /**
-   * Builds the listbox option element id for a row, kept in sync with the input's
-   * `aria-activedescendant` so assistive tech can follow the keyboard-highlighted row.
+   * Returns the listbox option id for `index`; kept in sync with `aria-activedescendant` so
+   * assistive tech follows the keyboard-highlighted row.
    *
    * @param index - The row's index in {@link glossedRanked}.
    * @returns The option element id.
    */
-  const optionId = (index: number) => `${listboxId}-opt-${index}`;
+  const optionId = useCallback((index: number) => `${listboxId}-opt-${index}`, [listboxId]);
 
   /** Closes the suggestion dropdown and clears the keyboard highlight, leaving focus untouched. */
   const closeSuggestions = useCallback(() => {
     setSuggestionsOpen(false);
     setActiveIndex(-1);
   }, []);
 
-  /**
-   * Commits the current draft gloss the same way blur does: only when it differs from the committed
-   * value. Only called from the (disabled-gated) blur and key-down handlers, so it needs no
-   * disabled check.
-   */
+  /** Commits the draft gloss only when it differs from the committed value. */
   const commitDraft = () => {
     if (draft !== committedGloss) {
       onGlossChange(token.ref, token.surfaceText, draft);
     }
   };
 
   /**
-   * Approves the chosen suggestion payload for this token and closes the dropdown. The typed draft
-   * (if any) is discarded: approving updates committedGloss, which the sync effect mirrors back
-   * into the input, so the selection wins over the abandoned draft.
+   * Approves the chosen suggestion payload for this token and closes the dropdown. Any typed draft
+   * is discarded: the approval updates `committedGloss`, which the sync effect mirrors back into
+   * the input so the selection wins.
    *
    * @param id - The chosen payload's id (the suggested pick or a promoted candidate).
    */
@@ -236,12 +228,10 @@ export function TokenChip({
 
   /**
    * Drives the gloss input as a combobox. While the dropdown is open, arrow keys move the highlight
-   * (stopping at the ends, with Up returning to the no-highlight state), Enter commits the
-   * highlight or the top row, and Escape closes without committing. While it is closed, ArrowDown
-   * opens the dropdown (the keyboard equivalent of the chevron) when this token has suggestions —
-   * so an approved token whose dropdown does not auto-open on focus can still summon its pool
-   * alternatives from the keyboard — and Enter commits the typed draft. Other keys fall through to
-   * the input.
+   * (stopping at the ends; Up returns to the no-highlight state), Enter commits the highlighted row
+   * or the top row, and Escape closes without committing. While closed, ArrowDown opens the
+   * dropdown (keyboard equivalent of the chevron — lets an approved token summon pool alternatives
+   * without focusing away) and Enter commits the typed draft.
    *
    * @param e - The gloss input's key-down event.
    */

src/components/__mocks__/AnalysisStore.tsx

@@ -151,14 +151,13 @@ export function useMorphemeGlossDispatch(): (
 export function useReportGlossEditing(_isEditing: boolean): void {}
 
 /**
- * Returns the merged token analysis in mock context. The mock pool is empty and tracks only
- * approved glosses, so it never derives a suggestion — always `undefined`. Suggestion behavior is
- * covered separately against the real store.
+ * Returns the merged token analysis in mock context. The mock pool is empty, so it never derives
+ * a suggestion — always `undefined`. Suggestion behavior is covered against the real store.
  *
- * @param _tokenRef - The token reference key (unused in mock).
- * @param _surfaceText - The token's surface text (unused in mock).
- * @param _enabled - Whether to resolve (unused in mock; it never surfaces suggestions anyway).
- * @returns `undefined` — the mock surfaces no suggestions.
+ * @param _tokenRef - Token ref (unused in mock).
+ * @param _surfaceText - Surface text (unused in mock).
+ * @param _enabled - Whether to resolve (unused in mock).
+ * @returns `undefined`.
  */
 export function useResolvedTokenAnalysis(
   _tokenRef: string,
@@ -169,8 +168,7 @@ export function useResolvedTokenAnalysis(
 }
 
 /**
- * Returns whether suggestions should render in mock context. The mock never surfaces suggestions,
- * so this is always `false`.
+ * Returns whether suggestions should render in mock context — always `false`.
  *
  * @returns `false`.
  */

src/store/analysisSlice.ts

@@ -11,8 +11,8 @@ import type {
   TokenSnapshot,
 } from 'interlinearizer';
 import { emptyAnalysis } from '../types/empty-factories';
-import { isEmptyMultiString } from '../utils/multi-string';
 import { analysesAreIdentical } from '../utils/analysis-identity';
+import { isEmptyMultiString } from '../utils/multi-string';
 import {
   buildPoolIndex,
   deriveTokenSuggestion,
@@ -303,7 +303,9 @@ function isPayloadSharedByOtherLinks(
   link: TokenAnalysisLink,
   analysisId: string,
 ): boolean {
-  return state.analysis.tokenAnalysisLinks.some((l) => l !== link && l.analysisId === analysisId);
+  return state.analysis.tokenAnalysisLinks.some(
+    (l) => l !== link && l.status === 'approved' && l.analysisId === analysisId,
+  );
 }
 
 /**
@@ -446,8 +448,8 @@ const analysisSlice = createSlice({
           // onto a private clone first and mutate that, so the co-linked tokens keep the shared gloss
           // instead of being rewritten or stranded by an edit aimed at this one. (Global "edit every
           // occurrence" is deferred; see user-questions.md "separating per-token edits from global
-          // analysis edits".) Surface text is refreshed on the fork (not the shared original) so a
-          // co-linked sibling's payload is never rewritten — mirroring writeMorphemes.
+          // analysis edits".) Surface text is refreshed on the fork (not the shared original)
+          // so a co-linked sibling's payload is never rewritten.
           const target = isPayloadSharedByOtherLinks(state, link, analysis.id)
             ? forkSharedAnalysis(state, link, analysis, id)
             : analysis;
@@ -705,10 +707,7 @@ const analysisSlice = createSlice({
         }
         // A morpheme gloss is part of analysis identity (see analysesAreIdentical), so editing or
         // clearing one can make this payload identical to an existing one (e.g. a homograph whose
-        // only difference was this morpheme's gloss); re-converge so the dedupe the create path
-        // guarantees on first write also holds after morpheme gloss edits (mirrors writeGloss).
-        // Symmetric across write and clear, so a clear back to a sibling's state never leaves a
-        // duplicate the suggestion pool would double-count.
+        // only difference was this morpheme's gloss); re-converge so dedupe holds after edits too.
         mergeIntoIdenticalPayload(state, target);
       },
     },

src/utils/analysis-identity.ts

@@ -12,10 +12,8 @@ import type { MorphemeAnalysis, TokenAnalysis } from 'interlinearizer';
 const NORMALIZED_FORM_CACHE_MAX = 50_000;
 
 /**
- * Memoizes {@link normalizeSurfaceForm} by raw input string. The same surface forms are normalized
- * over and over — once per visible un-approved token on every store dispatch (the suggestion derive
- * path) plus the dedupe scans on every write — yet the result is a pure function of the input, so
- * caching is safe and entries never go stale (normalization is deterministic). The cache is module-
+ * Memoizes {@link normalizeSurfaceForm} by raw input string — called once per visible un-approved
+ * token on every store dispatch, so the same forms are normalized repeatedly. The cache is module-
  * global rather than project-scoped, so a bound is needed: when it reaches
  * {@link NORMALIZED_FORM_CACHE_MAX} the oldest entry is evicted (insertion-order FIFO, free from
  * `Map`), keeping memory flat across a session that opens many projects rather than retaining every