Skip to content

Commit f227daf

Browse files
Tag suggestion rows by status; repoint approval on promote
Carry each suggestion row's accept/promote status on the entry instead of inferring it from row position, so dropping a blank-in-language pick can never leave a candidate masquerading as the accept row. glossedSuggestionEntries now flattens the resolved read directly (folding in the approved-payload exclusion), and the dropdown and suggested-placeholder color from a shared gloss-suggested utility. Make approveAnalysisForToken repoint the one approved link when promoting an already-approved token, reclaiming the old payload if it was its last reference, rather than no-opping. This unblocks promoting an approved homograph to a different pool analysis while keeping the single-approved invariant. Bound normalizedFormCache with a FIFO eviction cap so the module-global cache stays flat across a session that opens many projects.
1 parent 31a141f commit f227daf

10 files changed

Lines changed: 269 additions & 116 deletions

File tree

contributions/localizedStrings.json

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -86,12 +86,6 @@
8686
"%interlinearizer_confirm_discard_ok%": "Discard",
8787
"%interlinearizer_confirm_discard_cancel%": "Cancel",
8888

89-
"%interlinearizer_globalEdit_title%": "Update analysis used by {count} tokens?",
90-
"%interlinearizer_globalEdit_body%": "This analysis is shared by {count} tokens, so editing it updates all of them. Make a separate analysis to change only this token.",
91-
"%interlinearizer_globalEdit_updateAll%": "Update all {count}",
92-
"%interlinearizer_globalEdit_fork%": "Make a separate analysis for just this one",
93-
"%interlinearizer_globalEdit_cancel%": "Cancel",
94-
9589
"%interlinearizer_modal_saveAs_title%": "Save As",
9690
"%interlinearizer_modal_saveAs_new_section%": "Save as a new project",
9791
"%interlinearizer_modal_saveAs_save_new%": "Save as New Project",

src/__tests__/components/TokenChip.suggestions.test.tsx

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -163,15 +163,15 @@ describe('TokenChip suggested placeholder', () => {
163163
const input = screen.getByLabelText('Gloss for logos');
164164
// Visible at a glance — no focus or hover — so the row reveals which tokens have a suggestion.
165165
expect(input).toHaveAttribute('placeholder', 'word');
166-
expect(input.className).toContain('tw:placeholder:text-success-foreground');
166+
expect(input.className).toContain('tw:placeholder:gloss-suggested');
167167
});
168168

169169
it('falls back to the generic placeholder when the token has no suggestion', () => {
170170
renderChip(wordToken('tok-x', 'unseen'), { initialAnalysis: poolWithOneApproved('word') });
171171

172172
const input = screen.getByLabelText('Gloss for unseen');
173173
expect(input).toHaveAttribute('placeholder', 'gloss');
174-
expect(input.className).not.toContain('tw:placeholder:text-success-foreground');
174+
expect(input.className).not.toContain('tw:placeholder:gloss-suggested');
175175
});
176176

177177
it('uses the generic placeholder when suggestions are turned off', () => {
@@ -182,7 +182,7 @@ describe('TokenChip suggested placeholder', () => {
182182

183183
const input = screen.getByLabelText('Gloss for logos');
184184
expect(input).toHaveAttribute('placeholder', 'gloss');
185-
expect(input.className).not.toContain('tw:placeholder:text-success-foreground');
185+
expect(input.className).not.toContain('tw:placeholder:gloss-suggested');
186186
});
187187

188188
it('reverts to the generic placeholder once the user types a gloss', async () => {
@@ -194,7 +194,7 @@ describe('TokenChip suggested placeholder', () => {
194194

195195
// With a non-empty draft the typed value shows, so the suggested ghost text no longer applies.
196196
expect(input).toHaveAttribute('placeholder', 'gloss');
197-
expect(input.className).not.toContain('tw:placeholder:text-success-foreground');
197+
expect(input.className).not.toContain('tw:placeholder:gloss-suggested');
198198
});
199199
});
200200

@@ -250,8 +250,12 @@ describe('TokenChip suggestion dropdown', () => {
250250
const chevron = screen.getByTestId('suggestion-chevron');
251251
await userEvent.click(chevron);
252252

253-
const accept = screen.getByTestId('suggestion-accept');
254-
expect(accept).toHaveTextContent('finance');
253+
// On an already-approved token every alternative is a blue "promote" row, not the green "accept"
254+
// row — there is no suggestion to accept, only candidates to promote to.
255+
expect(screen.queryByTestId('suggestion-accept')).not.toBeInTheDocument();
256+
const promote = screen.getByTestId('suggestion-candidate');
257+
expect(promote).toHaveTextContent('finance');
258+
expect(promote.className).toContain('tw:gloss-candidate');
255259
// The already-approved 'riverbank' is excluded — only the alternative is offered.
256260
expect(screen.queryByText('riverbank')).not.toBeInTheDocument();
257261
});
@@ -531,10 +535,12 @@ describe('TokenChip suggestion keyboard navigation', () => {
531535

532536
await userEvent.keyboard('{ArrowDown}');
533537

534-
// The dropdown opens with nothing highlighted, offering the alternative payload.
538+
// The dropdown opens with nothing highlighted, offering the alternative as a promote candidate
539+
// (an approved token has no "accept the suggestion" row, only promotions).
535540
expect(input).toHaveAttribute('aria-expanded', 'true');
536-
expect(screen.getByTestId('suggestion-accept')).toHaveTextContent('finance');
537-
expect(screen.getByTestId('suggestion-accept')).toHaveAttribute('aria-selected', 'false');
541+
expect(screen.queryByTestId('suggestion-accept')).not.toBeInTheDocument();
542+
expect(screen.getByTestId('suggestion-candidate')).toHaveTextContent('finance');
543+
expect(screen.getByTestId('suggestion-candidate')).toHaveAttribute('aria-selected', 'false');
538544
expect(onGlossChange).not.toHaveBeenCalled();
539545
});
540546

src/__tests__/store/analysisSlice.test.ts

Lines changed: 83 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1899,8 +1899,11 @@ describe('approveAnalysisForToken', () => {
18991899
expect(links.some((l) => l.token.tokenRef === 'tok-2')).toBe(false);
19001900
});
19011901

1902-
it('is a no-op when the token already has an approved analysis (single-approved invariant)', () => {
1903-
// Guards against a stray double-dispatch creating a second approved link for one token.
1902+
it('repoints the existing approved link when promoting an already-approved token (no second link)', () => {
1903+
// Promoting an already-approved homograph to a different pool analysis must swap the one approved
1904+
// link in place rather than appending a second (preserving the single-approved invariant) or
1905+
// no-opping (which would silently fail the re-promotion the UI offers). A co-linked sibling keeps
1906+
// ta-1 alive, so the old payload survives.
19041907
const approved: TokenAnalysis = { id: 'ta-1', surfaceText: 'logos', gloss: { en: 'word' } };
19051908
const other: TokenAnalysis = { id: 'ta-2', surfaceText: 'logos', gloss: { en: 'other' } };
19061909
const store = createAnalysisStore({
@@ -1914,6 +1917,11 @@ describe('approveAnalysisForToken', () => {
19141917
status: 'approved',
19151918
token: { tokenRef: 'tok-1', surfaceText: 'logos' },
19161919
},
1920+
{
1921+
analysisId: 'ta-1',
1922+
status: 'approved',
1923+
token: { tokenRef: 'tok-sibling', surfaceText: 'logos' },
1924+
},
19171925
],
19181926
},
19191927
analysisLanguage: 'en',
@@ -1924,11 +1932,83 @@ describe('approveAnalysisForToken', () => {
19241932
approveAnalysisForToken({ tokenRef: 'tok-1', surfaceText: 'logos', analysisId: 'ta-2' }),
19251933
);
19261934

1927-
// The existing approved link stands; no second approved link was appended.
1935+
// The one approved link for tok-1 is repointed to ta-2 — not duplicated.
1936+
const links = store
1937+
.getState()
1938+
.analysis.analysis.tokenAnalysisLinks.filter((l) => l.token.tokenRef === 'tok-1');
1939+
expect(links).toHaveLength(1);
1940+
expect(links[0].analysisId).toBe('ta-2');
1941+
// The sibling still approves ta-1, so the old payload survives.
1942+
expect(store.getState().analysis.analysis.tokenAnalyses.map((ta) => ta.id)).toEqual([
1943+
'ta-1',
1944+
'ta-2',
1945+
]);
1946+
});
1947+
1948+
it('reclaims the old payload when promoting an already-approved token off its last reference', () => {
1949+
// When the promoted-away payload has no other approved link, repointing leaves it orphaned, so
1950+
// it is dropped — a promotion never strands an empty payload.
1951+
const approved: TokenAnalysis = { id: 'ta-1', surfaceText: 'logos', gloss: { en: 'word' } };
1952+
const other: TokenAnalysis = { id: 'ta-2', surfaceText: 'logos', gloss: { en: 'other' } };
1953+
const store = createAnalysisStore({
1954+
analysis: {
1955+
analysis: {
1956+
...emptyAnalysis(),
1957+
tokenAnalyses: [approved, other],
1958+
tokenAnalysisLinks: [
1959+
{
1960+
analysisId: 'ta-1',
1961+
status: 'approved',
1962+
token: { tokenRef: 'tok-1', surfaceText: 'logos' },
1963+
},
1964+
],
1965+
},
1966+
analysisLanguage: 'en',
1967+
},
1968+
});
1969+
1970+
store.dispatch(
1971+
approveAnalysisForToken({ tokenRef: 'tok-1', surfaceText: 'logos', analysisId: 'ta-2' }),
1972+
);
1973+
1974+
const links = store
1975+
.getState()
1976+
.analysis.analysis.tokenAnalysisLinks.filter((l) => l.token.tokenRef === 'tok-1');
1977+
expect(links).toHaveLength(1);
1978+
expect(links[0].analysisId).toBe('ta-2');
1979+
// ta-1 had no other approved reference, so it was reclaimed.
1980+
expect(store.getState().analysis.analysis.tokenAnalyses.map((ta) => ta.id)).toEqual(['ta-2']);
1981+
});
1982+
1983+
it('is a no-op when promoting an already-approved token to the analysis it already approves', () => {
1984+
// Re-approving the same payload changes nothing — the link already points there.
1985+
const approved: TokenAnalysis = { id: 'ta-1', surfaceText: 'logos', gloss: { en: 'word' } };
1986+
const store = createAnalysisStore({
1987+
analysis: {
1988+
analysis: {
1989+
...emptyAnalysis(),
1990+
tokenAnalyses: [approved],
1991+
tokenAnalysisLinks: [
1992+
{
1993+
analysisId: 'ta-1',
1994+
status: 'approved',
1995+
token: { tokenRef: 'tok-1', surfaceText: 'logos' },
1996+
},
1997+
],
1998+
},
1999+
analysisLanguage: 'en',
2000+
},
2001+
});
2002+
2003+
store.dispatch(
2004+
approveAnalysisForToken({ tokenRef: 'tok-1', surfaceText: 'logos', analysisId: 'ta-1' }),
2005+
);
2006+
19282007
const links = store
19292008
.getState()
19302009
.analysis.analysis.tokenAnalysisLinks.filter((l) => l.token.tokenRef === 'tok-1');
19312010
expect(links).toHaveLength(1);
19322011
expect(links[0].analysisId).toBe('ta-1');
2012+
expect(store.getState().analysis.analysis.tokenAnalyses).toHaveLength(1);
19332013
});
19342014
});

src/components/AnalysisStore.tsx

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -165,9 +165,18 @@ export function AnalysisStoreProvider({
165165
if (wasPending !== isPending) onPendingEditsChangeRef.current?.(isPending);
166166
}, []);
167167

168-
// The gloss-write entry point exposed to inputs. A gloss edit is per-token: the reducer forks a
169-
// shared payload before writing (blank clears, non-blank edits), so editing one token never
170-
// rewrites the others. Write, persist via `onSave`, then notify the `onGlossChange` spy.
168+
/**
169+
* The gloss-write entry point exposed to inputs. Dispatches {@link writeGloss} so the reducer
170+
* applies a per-token edit — it forks a shared payload before writing (blank clears, non-blank
171+
* edits), so editing one token never rewrites its co-linked siblings — then persists the updated
172+
* analysis via `onSave` and notifies the `onGlossChange` spy with the edited token and value.
173+
*
174+
* @param tokenRef - `Token.ref` of the token being glossed.
175+
* @param surfaceText - Surface text of the token, recorded so the analysis can detect baseline
176+
* drift.
177+
* @param value - New gloss string; blank (empty or whitespace) clears the active language's
178+
* gloss.
179+
*/
171180
const requestGlossEdit = useCallback(
172181
(tokenRef: string, surfaceText: string, value: string) => {
173182
store.dispatch(writeGloss(tokenRef, surfaceText, value));

src/components/SuggestionDropdown.tsx

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,15 @@ type SuggestionDropdownProps = Readonly<{
3939
}>;
4040

4141
/**
42-
* Renders the portaled suggestion listbox for a token's gloss combobox. Row 0 is the suggested pick
43-
* (green); the rest are candidates (blue), matching the inline coloring this replaced. The
44-
* keyboard-active row gets the same `bg-accent` background hovering applies, and hovering a row
45-
* sets the active index so only one row is ever highlighted. Each row suppresses its mouse-down
46-
* default so clicking it never blurs the input — focus stays in the input and the click selects
47-
* instead. The panel closes itself on outer scrolling (the anchor would drift); scrolling the
48-
* panel's own overflow is ignored so a long list can be scrolled without dismissing it.
42+
* Renders the portaled suggestion listbox for a token's gloss combobox. Each row is colored and
43+
* labeled by its own `status` — `'suggested'` (green, "accept") or `'candidate'` (blue, "promote")
44+
* — carried on the entry rather than inferred from row position, so a dropped blank-in-language
45+
* pick can never leave a candidate masquerading as the accept row. The keyboard-active row gets the
46+
* same `bg-accent` background hovering applies, and hovering a row sets the active index so only
47+
* one row is ever highlighted. Each row suppresses its mouse-down default so clicking it never
48+
* blurs the input — focus stays in the input and the click selects instead. The panel closes itself
49+
* on outer scrolling (the anchor would drift); scrolling the panel's own overflow is ignored so a
50+
* long list can be scrolled without dismissing it.
4951
*
5052
* @param props - Component props (see {@link SuggestionDropdownProps}).
5153
* @returns A `document.body` portal containing the listbox, positioned under the anchor input.
@@ -132,13 +134,13 @@ export default function SuggestionDropdown({
132134
<li
133135
key={entry.id}
134136
aria-label={
135-
index === 0
137+
entry.status === 'suggested'
136138
? `Accept suggestion ${entry.gloss} for ${surfaceText}`
137139
: `Promote ${entry.gloss} for ${surfaceText}`
138140
}
139141
aria-selected={index === activeIndex}
140-
className={`tw:cursor-pointer tw:whitespace-nowrap tw:px-2 tw:py-0.5 tw:text-sm tw:italic ${index === 0 ? statusTextColorClass('suggested') : statusTextColorClass('candidate')}${index === activeIndex ? ' tw:bg-accent' : ''}`}
141-
data-testid={index === 0 ? 'suggestion-accept' : 'suggestion-candidate'}
142+
className={`tw:cursor-pointer tw:whitespace-nowrap tw:px-2 tw:py-0.5 tw:text-sm tw:italic ${statusTextColorClass(entry.status)}${index === activeIndex ? ' tw:bg-accent' : ''}`}
143+
data-testid={entry.status === 'suggested' ? 'suggestion-accept' : 'suggestion-candidate'}
142144
id={optionId(index)}
143145
role="option"
144146
// Select on mouse-down, suppressing its default focus shift, so choosing a row never blurs

src/components/TokenChip.tsx

Lines changed: 14 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -174,29 +174,17 @@ export function TokenChip({
174174

175175
const hasMorphemes = morphemes.length > 0;
176176

177-
// The pool entries to offer via the suggestion dropdown. For a suggested token this is the engine's
178-
// derived pick plus candidates. For an approved token this is the pool alternatives only —
179-
// the already-approved payload is filtered out so the dropdown only shows genuinely different
180-
// choices (re-approving the same analysis via the dropdown would be a no-op). Row 0 is the green
181-
// "accept"; the rest are blue "promote" candidates. Blank-in-active-language entries are dropped
182-
// individually rather than shown as empty rows (see `user-questions.md` display #3).
183-
const suggestion = useMemo(() => {
184-
if (!resolved) return undefined;
185-
if (resolved.status === 'suggested') return resolved;
186-
const pool = resolved.poolSuggestion;
187-
if (!pool) return undefined;
188-
const approvedId = resolved.analysis.id;
189-
// Keep only pool entries that are not the already-approved analysis.
190-
const alternatives = [pool.suggested, ...pool.candidates].filter((a) => a.id !== approvedId);
191-
if (alternatives.length === 0) return undefined;
192-
return { suggested: alternatives[0], candidates: alternatives.slice(1) };
193-
}, [resolved]);
194-
// Memoized on the (reference-stable) suggestion and active language so typing a gloss — which only
195-
// changes local draft state — never re-runs the engine's flatten/filter; it recomputes only when
196-
// the suggestion or language actually changes.
177+
// The pool entries to offer via the suggestion dropdown, with their accept/promote status. The
178+
// engine flattens the resolved read into ranked, status-tagged rows: for a suggested token its
179+
// pick (green "accept") plus candidates (blue "promote"); for an approved token the pool
180+
// alternatives only (all blue "promote", the already-approved payload excluded). Blank-in-active-
181+
// language entries are dropped individually rather than shown as empty rows (see
182+
// `user-questions.md` display #3). Memoized on the (reference-stable) resolved read and active
183+
// language so typing a gloss — which only changes local draft state — never re-runs the flatten/
184+
// filter; it recomputes only when the resolved read or language actually changes.
197185
const glossedRanked = useMemo(
198-
() => glossedSuggestionEntries(suggestion, analysisLanguage),
199-
[suggestion, analysisLanguage],
186+
() => glossedSuggestionEntries(resolved, analysisLanguage),
187+
[resolved, analysisLanguage],
200188
);
201189
// Whether this token has anything to suggest: gated on the demo toggle (via the resolve short-
202190
// circuit) and editability. The chevron and dropdown only ever appear when this is true.
@@ -274,9 +262,6 @@ export function TokenChip({
274262
// non-empty while open, but glossedRanked can empty out after open (a row approved away),
275263
// leaving suggestionsOpen stale while the dropdown is unmounted — guard so Enter closes
276264
// rather than dereferencing an absent pick.
277-
// activeIndex -1 (nothing highlighted) falls back to the top row. The list is normally
278-
// non-empty while open, but glossedRanked can empty out after open (a row approved away)
279-
// while suggestionsOpen is still true, so guard rather than dereference an absent pick.
280265
const pick = glossedRanked[activeIndex] ?? glossedRanked[0];
281266
if (pick) selectSuggestion(pick.id);
282267
/* v8 ignore next -- defensive: the empty-pick race above is not reachable from the call sites */ else
@@ -465,9 +450,10 @@ export function TokenChip({
465450
aria-expanded={hasSuggestions ? dropdownShown : undefined}
466451
aria-label={`Gloss for ${token.surfaceText}`}
467452
// When the empty input is showing a suggested gloss as its placeholder, color that ghost
468-
// text green (the same success color the dropdown's accept row uses) and italicize it, at
469-
// full opacity, so it reads clearly as a suggestion rather than a faint generic hint.
470-
className={`tw:gloss-input${showSuggestedPlaceholder ? ' tw:placeholder:text-success-foreground tw:placeholder:italic tw:placeholder:opacity-100' : ''}`}
453+
// text via the same `gloss-suggested` utility the dropdown's accept row uses (one source
454+
// of truth for the suggested green) and italicize it, at full opacity, so it reads
455+
// clearly as a suggestion rather than a faint generic hint.
456+
className={`tw:gloss-input${showSuggestedPlaceholder ? ' tw:placeholder:gloss-suggested tw:placeholder:italic tw:placeholder:opacity-100' : ''}`}
471457
disabled={disabled}
472458
id={glossInputId}
473459
placeholder={

src/components/__mocks__/AnalysisStore.tsx

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,12 @@ import type { ResolvedTokenAnalysis } from '../../utils/suggestion-engine';
88
type GlossMap = Record<string, string>;
99
type MockCtxValue = {
1010
glosses: GlossMap;
11-
dispatch: (tokenRef: string, surfaceText: string, value: string) => boolean;
11+
dispatch: (tokenRef: string, surfaceText: string, value: string) => void;
1212
language: string;
1313
};
1414
const MockCtx = createContext<MockCtxValue>({
1515
glosses: {},
16-
dispatch: () => false,
16+
dispatch: () => {},
1717
language: 'und',
1818
});
1919

@@ -54,8 +54,6 @@ export function AnalysisStoreProvider({
5454
(tokenRef: string, _surfaceText: string, value: string) => {
5555
setGlosses((prev) => ({ ...prev, [tokenRef]: value }));
5656
onGlossChange?.(tokenRef, value);
57-
// The mock has no global-edit gate, so an edit is never held — always reports "committed".
58-
return false;
5957
},
6058
[onGlossChange],
6159
);
@@ -132,17 +130,16 @@ export function useMorphemeDeleteDispatch(): (tokenRef: string) => void {
132130
}
133131

134132
/**
135-
* Returns a no-op dispatch for writing morpheme glosses in mock context. Reports "committed" (never
136-
* held), matching the real hook's boolean return.
133+
* Returns a no-op dispatch for writing morpheme glosses in mock context.
137134
*
138135
* @returns A no-op function matching the real signature.
139136
*/
140137
export function useMorphemeGlossDispatch(): (
141138
tokenRef: string,
142139
morphemeId: string,
143140
value: string,
144-
) => boolean {
145-
return () => false;
141+
) => void {
142+
return () => {};
146143
}
147144

148145
/**

0 commit comments

Comments
 (0)