Add engine to generate token gloss suggestions from previous glosses#131
Conversation
Phase 0.1–0.5 of the suggestion engine: move token analyses from "one payload per token" to shared payloads with dedupe-on-write and link-based cleanup, without deriving any suggestions yet. Behavior is unchanged for single-occurrence words. - 0.1: add utils/analysis-identity.ts — normalizeSurfaceForm (NFC + locale-independent lowercase) and analysesAreIdentical (content identity on surface form, gloss, pos, features, and morphemes by form/gloss/refs; ignores id and writingSystem). - 0.2: appendApprovedAnalysis does find-or-create, so identical glosses converge onto one shared payload while homographs stay distinct. Link snapshots record the linking token's own surface text. - 0.3: replace removeTokenAnalysis with detachTokenAnalysisLink — drop the editing token's link and reclaim the payload only once its last link is gone, so an edit/clear on one token never orphans another's. - 0.4: add forkAnalysisForToken reducer — clone a shared payload and repoint only this token's approved link, leaving others shared. - 0.5: add memoized selectApprovedLinkCountForPayload — the blast radius of a global edit (0 / 1 / N tokens). Preserves the existing findLast orphan-repair, the at-most-one-approved invariant, and empty-record cleanup. Full suite green at 100% coverage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…gine 0.6)
Editing a TokenAnalysis payload shared by more than one token now prompts
before the change fans out, so a single gloss edit never silently rewrites
many occurrences.
The gate lives in AnalysisStoreProvider rather than TokenChip, so one modal
serves every gloss input: useGlossDispatch reads the blast radius
(selectApprovedLinkCountForPayload) and, when >1 token shares the payload,
holds the edit for GlobalEditConfirmationModal instead of committing. The
modal offers three choices:
- Update all -> commit the global writeGloss
- Make separate -> fork the payload (forkAnalysisForToken), then write to
the clone so only this token changes
- Cancel -> drop the held edit
The provider's confirmGlobalEdits prop defaults off (opt-in) so existing
consumers are unaffected. A removable, session-local "Confirm shared-analysis
edits" toggle (default on) is threaded InterlinearizerLoader -> Interlinearizer
-> provider via the view-options dropdown, demonstrating the open UX question
on edit friction.
Scope: only the gloss edit path is gated; editing a shared payload's morphemes
still commits globally. Deferred deliberately to keep this step focused.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Compute token suggestions as a read-only view over the pool of approved analyses — nothing is persisted and nothing renders yet. New pure engine module `utils/suggestion-engine.ts`: - buildPoolIndex: groups approved payloads by normalized surface form with their approval frequency (homographs share a key as competing entries) - rankPoolEntries: best-first by descending frequency, ties broken by lowest analysis id so the elected suggestion never flickers - deriveTokenSuggestion / deriveSuggestions: match an un-approved token to the pool, electing the top payload as `suggested` and the rest as `candidate`; punctuation and already-approved tokens are skipped New selectors in `store/analysisSlice.ts`: - selectPoolIndex: memoized pool built from the existing approved-count and by-id base selectors (only approved links enter the pool) - selectResolvedTokenAnalysis: the single merged read the renderer will use — the approved decision if present, else the derived suggestion, else nothing Keying on the normalized surface form alone is correct for the v1 single-project pool; cross-project writing-system discrimination is deferred. No reducers or components changed. Suite green at 100% coverage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review follow-up: the merged read freshly allocates its result object (and a fresh candidates array in the suggested branch) on every call, unlike the reference-stable sibling reads selectApprovedGloss/selectApprovedMorphemes. Warn that a useSelector consumer must subscribe via a per-token memoized selector or a shallow/custom equalityFn (or useMemo the result), so the Phase 2 renderer is not wired naively into per-store-change re-renders and the react-redux "selector returned a different result" warning. Doc-only; no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Surface the engine's derived suggestion on un-approved tokens and let users accept it (or promote a homograph candidate) into an approved analysis. Built behind a removable demo toggle since the prominence / candidate-review UX is still open. Store / engine: - approveAnalysisForToken reducer: links a token to an existing shared payload with status approved (no new payload), so accepting raises that payload's approval frequency and the derived suggestion disappears. Exposed via useApproveAnalysisDispatch, which auto-saves and bypasses the global-edit gate (accepting adds a link, it does not rewrite shared content) - resolvedTokenAnalysisEqual: a useSelector equalityFn so the per-token useResolvedTokenAnalysis subscription stays referentially stable across pool rebuilds, since selectResolvedTokenAnalysis freshly allocates its result - remove the now-redundant batch deriveSuggestions — the renderer derives per-token through useResolvedTokenAnalysis instead Rendering (TokenChip): - show the suggested gloss as a green "accept" button beneath the gloss input, with blue "promote" buttons for homograph candidates (capped); italic + color keep them subordinate to an approved (foreground) gloss - gated on the new showSuggestions flag, an editable chip, an empty draft, and an active-language gloss (blank-gloss matches are not shown as empty buttons) - status colors centralized as gloss-suggested / gloss-candidate / gloss-rejected / gloss-stale utilities in tailwind.css, reusing the theme's green (success-foreground) and red (destructive) and adding raw blue/orange where the theme has none; each light/dark aware. Mapped from new utils/status-colors.ts Demo toggle: - session-local "Show suggestions" switch (default on), threaded InterlinearizerLoader -> Interlinearizer -> AnalysisStoreProvider (showSuggestions prop -> useShowSuggestions) via the view-options dropdown, demonstrating the open suggestion-prominence question Suite green at 100% coverage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- TokenChip: when the top-ranked suggestion has no active-language gloss, fall through to the highest-ranked candidate that does, instead of hiding the whole suggestion block (a blank homograph no longer masks a usable lower-ranked alternative). Skips blank analyses individually. - approveAnalysisForToken: no-op when the token already has an approved analysis, so a stray double-dispatch can't append a second approved link; also heals an orphaned approved link rather than being blocked by it. - status-colors.ts: Prettier formatting. New tests for both behaviors; suite green at 100% coverage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
3) deriveTokenSuggestion re-sorted a bucket on every per-token render via rankPoolEntries. Move ranking into buildPoolIndex, which sorts each bucket in place once when the memoized selectPoolIndex rebuilds (only on an approved write), so the per-token derive just reads the pre-ranked head as the suggested pick and the tail as the candidates. - Add an internal comparePoolEntries comparator (descending frequency, ascending analysis.id tiebreak to keep the elected pick stable). - Tighten PoolIndex to ReadonlyMap<string, readonly PoolEntry[]> to encode the "ranked once, never re-sorted per token" contract. - Remove the now-redundant exported rankPoolEntries; its behavioral coverage moves into the buildPoolIndex ranking tests. Suite green at 100% coverage; lint clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fixes surfaced while reviewing the suggestion-engine branch. Correctness: - Clearing a gloss or deleting morphemes on a shared payload now forks the editing token onto a private clone first, so co-linked tokens keep their content instead of being stranded on an emptied payload. Adds forkSharedAnalysis / isPayloadSharedByOtherLinks; strengthens the cleanup tests to assert the co-token's content is preserved (they previously only checked the payload still existed). - Canceling the global-edit modal no longer leaves the abandoned draft stuck in the gloss / morpheme-gloss input: the edit dispatch reports whether it was held, and the input reverts to the committed value. - An in-place gloss edit re-converges onto an identical existing payload (mergeIntoIdenticalPayload), so editing can't leave the duplicate the create path's find-or-create avoids. - analysesAreIdentical now compares glossSenseRef; normalizeSurfaceForm re-normalizes to NFC after case-folding. Global-edit gate: - Generalize the provider gate into one gateEdit(commit, forkAndCommit) seam and route morpheme-breakdown and morpheme-gloss edits through it, so every fan-out edit prompts consistently. Clear/delete fork per-token in the reducer and intentionally bypass the gate. Cleanup: - Skip per-token pool resolution when suggestions are off; memoize the ranked-suggestion list so typing doesn't recompute it; reuse one shared empty candidates array. - Add the missing v8-ignore reason; drop brittle hardcoded counts from comments; document why deepEqual is kept local. Suite green at 100% coverage; lint clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 10 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds a “Show suggestions” toggle and a suggestion combobox for token glosses. It deduplicates identical approved analyses, builds ranked suggestion pools, adds provider hooks and store actions, and updates ChangesShared analysis suggestions
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
writeMorphemes/deleteMorphemes now re-converge onto an identical sibling payload after an in-place breakdown edit, matching writeGloss's existing mergeIntoIdenticalPayload step so morphology-only edits don't leave duplicate payloads. approveAnalysisForToken rejects an analysisId that resolves to no stored payload, so it can't append an orphaned approved link. GlobalEditConfirmationModal renders the blocking ModalShell overlay while localized strings load instead of returning nothing, keeping the held edit protected during the load window.
alex-rawlings-yyc
left a comment
alex-rawlings-yyc
left a comment
There was a problem hiding this comment.
@alex-rawlings-yyc resolved 3 discussions.
Reviewable status: 0 of 23 files reviewed, all discussions resolved (waiting on alex-rawlings-yyc).
Correctness: - Don't route gloss clears through the global-edit modal: a blank value commits directly, so the reducer's fork-on-clear isn't misrepresented as an "Update all" the clear can't honor. - Guard gateEdit against overwriting an already-held pendingEdit; a second gated edit while the modal is open is dropped (and its input reverts) instead of silently replacing the parked edit. Efficiency: - Memoize normalizeSurfaceForm by input so the per-token derive path doesn't re-run three Unicode passes per store dispatch. Altitude / cleanup: - Extract glossedSuggestionEntries into the engine; TokenChip delegates the active-language flatten/blank-skip ranking instead of inlining it. - gateEdit now owns the fork (gateEdit(tokenRef, commit)), removing the triplicated fork-then-commit closure from the dispatch hooks. - Collapse selectApprovedLinkCountForPayload to a single `?? 0` return. - Gate candidateEntries on acceptEntry rather than re-deriving showSuggestionUi. Adds tests for the blank-clear bypass and the overwrite guard; coverage 100%. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pressing Down in a focused gloss input now opens the suggestion dropdown when the token has suggestions but it isn't already open — the keyboard equivalent of the chevron. Covers approved tokens, whose dropdown doesn't auto-open on focus but may hold pool alternatives. Also show the top suggestion as green ghost-text placeholder in empty inputs, so a glance across the row reveals which tokens have a suggestion without focusing or hovering.
The clear branch returned before mergeIntoIdenticalPayload, so clearing a morpheme gloss back to a sibling's state left a duplicate payload the write path would have collapsed (double-counting the form in the suggestion pool). Merge on the clear path too, making dedupe symmetric across both directions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
scroll Guard the Enter handler against an absent pick when glossedRanked empties while suggestionsOpen is stale-true. Switch SuggestionDropdown positioning to useLayoutEffect to avoid a top-left flash, and scroll the active row into view during keyboard navigation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
alex-rawlings-yyc
left a comment
alex-rawlings-yyc
left a comment
There was a problem hiding this comment.
@alex-rawlings-yyc resolved 1 discussion.
Reviewable status: 0 of 27 files reviewed, all discussions resolved (waiting on alex-rawlings-yyc).
Gloss, morpheme, and morpheme-gloss edits now fork a shared TokenAnalysis before writing, so editing one token never rewrites its co-linked siblings. This matches the existing clear-path behavior and removes the blank-path asymmetry between token and morpheme gloss clears. Deletes the global-edit confirmation apparatus that the old fan-out model needed: GlobalEditConfirmationModal, the gateEdit/pendingEdit gate, the confirmGlobalEdits prop and demo toggle, and the now-unused forkAnalysisForToken action and selectApprovedLinkCountForPayload selector. Editing every occurrence of a shared analysis is deferred to a dedicated control; see user-questions.md "separating per-token edits from global analysis edits". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
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.
imnasnainaec
changed the title
imnasnainaec
left a comment
imnasnainaec
left a comment
There was a problem hiding this comment.
@imnasnainaec reviewed 9 files and all commit messages, and made 1 comment.
Reviewable status: 9 of 27 files reviewed, 1 unresolved discussion (waiting on alex-rawlings-yyc).
src/components/TokenChip.tsx line 482 at r3 (raw file):
type="text" /> {hasSuggestions && (inputFocused || chipHovered) && (
I'm finding the appearance/disappearance of this chevron, which changes the width of the parent box on/off hover, fairly jarring.
|
Previously, imnasnainaec (D. Ror.) wrote…
What would you prefer would happen? You don't need to have an answer, I just don't have a concrete idea of how this should behave or how the suggestions should be accessible |
Minor suggested cleanup (#133)
@alex-rawlings-yyc Perhaps it could go inside the gloss textfield, some sort of end decoration for which we'd want sufficient end-padding to prevent length increase when it appears. I don't know if that's supported by the shadcn component library. |
imnasnainaec
left a comment
imnasnainaec
left a comment
There was a problem hiding this comment.
@imnasnainaec reviewed 4 files and all commit messages, and made 4 comments.
Reviewable status: 12 of 27 files reviewed, 4 unresolved discussions (waiting on alex-rawlings-yyc).
user-questions.md line 114 at r4 (raw file):
3. **Status colors.** Approved = default foreground, suggested = green, candidate = blue, rejected = orange, stale = red. Does this mapping read correctly to users (e.g. green commonly means "approved/done", which here is the plain foreground state)?
Jason reminded me that we want the colors to match PT9 colors as much as make sense. There it is default for approved and blue for suggested with default for the alternative candidates in the dropdown. We should match the blue, then for the candidates, maybe purple or grey?
src/__tests__/utils/analysis-identity.test.ts line 16 at r4 (raw file):
const decomposed = 'café'; // "café" as e + combining acute (U+0301) expect(decomposed).not.toBe(composed); // distinct raw strings... expect(normalizeSurfaceForm(decomposed)).toBe(normalizeSurfaceForm(composed)); // ...equal once NFC.
⛏️ This test just confirms that that same normalization occurs, not NFC in particular.
src/components/TokenChip.tsx line 485 at r4 (raw file):
onMouseDown={(e) => e.preventDefault()} > <ChevronDown className={`tw:h-3 tw:w-3${dropdownShown ? ' tw:rotate-180' : ''}`} />
Two things that would align this more to PT9's interlinearizer:
+instead of^- only showing up when there is more than one suggestion
(The latter makes more sense if clicking anywhere in the gloss opens the menu of suggestions, and less sense if clicking on the icon is required.)
src/utils/status-colors.ts line 24 at r4 (raw file):
* - `stale` — red (drifted text needing re-review). */ const STATUS_TEXT_COLOR_CLASS: Record<AssignmentStatus, string> = {
If you export this record you can:
- use it directly
- delete the function below
- move this file to the
types/folder - delete the associated test file.
Replace the gloss suggestion chevron with a "+" button and render it only for tokens with more than one suggestion. Open the dropdown on focus of the gloss (click-to-open) regardless of whether it's empty, so single-suggestion and approved-token alternatives stay reachable without the button. Center the button on the input by moving gloss-input's top margin to the wrapper span. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Suggested glosses now render blue (matching PT9) and candidates grey (subordinate, via the muted-foreground theme token) instead of green/blue. Export STATUS_TEXT_COLOR_CLASS and index it directly at the call site, dropping the statusTextColorClass wrapper. Move the map to src/types/ (data, not logic) and delete its test, since src/types/** is excluded from coverage. Retitle the analysis-identity normalization test to state what it actually verifies (same key for decomposed/composed forms, not NFC specifically).
alex-rawlings-yyc
left a comment
alex-rawlings-yyc
left a comment
There was a problem hiding this comment.
@imnasnainaec I've addressed your reviews so far
@alex-rawlings-yyc made 1 comment.
Reviewable status: 12 of 27 files reviewed, 4 unresolved discussions.
imnasnainaec
left a comment
imnasnainaec
left a comment
There was a problem hiding this comment.
@imnasnainaec reviewed 10 files and made 3 comments.
Reviewable status: 17 of 29 files reviewed, 6 unresolved discussions (waiting on alex-rawlings-yyc).
user-questions.md line 114 at r4 (raw file):
Previously, imnasnainaec (D. Ror.) wrote…
Jason reminded me that we want the colors to match PT9 colors as much as make sense. There it is default for approved and blue for suggested with default for the alternative candidates in the dropdown. We should match the blue, then for the candidates, maybe purple or grey?
The colors here are stale.
a discussion (no related file):
Small behavior quirk:
- identify a token that occurs 4 times
- give the same gloss in 3 of them and a different one in the 4th
- click on one of the 3 same glosses, and see the dropdown appear with the other option (good)
- delete the gloss and see that the suggestion is populated with the other option (even though it's not the most-used option... questionable)
- once you blur away, the option list on that gloss is refreshed and it switches the suggestion
I think we should: either re-add a suggestion when a confirmed gloss is deleted; or keep the gloss suggestion blank if the gloss was just deleted. I'm leaning toward the former, so that it's consistant with what the user will see after blur.
src/__tests__/components/TokenChip.suggestions.test.tsx line 203 at r5 (raw file):
describe('TokenChip suggestion dropdown', () => { it('opens on focus of an empty input and shows the suggested gloss in green', async () => {
stale color reference--one of several "green" and "blue" references in the tests
Deleting an approved gloss commits only on blur, so until then the dropdown showed just that payload's alternatives (e.g. the minority homograph) and snapped to the pool's best pick once the empty value committed. Derive the suggestion as if the token's approval were already removed so the placeholder and dropdown stay consistent across the blur: - deriveTokenSuggestion gains an excludeAnalysisId that discounts the token's own approval (dropping the payload when it was its last) and re-ranks the bucket. - New selectSuggestionAfterClearing selector + useSuggestionAfterClearing hook, gated so only the token being cleared does the extra pool lookup. Also refresh stale status-color references (suggested green->blue, candidate blue->grey) in comments and tests, and update user-questions.md to the shipped suggestion dropdown affordances plus the clearing decision. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
imnasnainaec
left a comment
imnasnainaec
left a comment
There was a problem hiding this comment.
@imnasnainaec reviewed 10 files, made 1 comment, and resolved 6 discussions.
Reviewable status: 24 of 29 files reviewed, 1 unresolved discussion (waiting on alex-rawlings-yyc).
src/utils/analysis-identity.ts line 106 at r6 (raw file):
* `features`, `glossSenseRef` (the lexicon sense the gloss resolves through), and `morphemes` * (compared on form + gloss + refs only — see {@link morphemeIdentity}). A missing field and an * empty one of the same kind compare equal. `glossSenseRef` is part of identity because
⛏️ the missing=empty is not technically true for gloss; either the doc needs updating or
a.gloss ?? {}, b.gloss ?? {} (if that's appropriate for this function's usage)
imnasnainaec
left a comment
imnasnainaec
left a comment
There was a problem hiding this comment.
@imnasnainaec reviewed 4 files and all commit messages.
Reviewable status: 28 of 29 files reviewed, 1 unresolved discussion (waiting on alex-rawlings-yyc).
imnasnainaec
left a comment
imnasnainaec
left a comment
There was a problem hiding this comment.
@imnasnainaec reviewed 1 file and all commit messages, and resolved 1 discussion.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on alex-rawlings-yyc).
2 participants
This change is
Summary by CodeRabbit
New Features
Bug Fixes