Add engine to generate gloss suggestions based on previous glosses#131
Draft
alex-rawlings-yyc wants to merge 15 commits into
Draft
Add engine to generate gloss suggestions based on previous glosses#131alex-rawlings-yyc wants to merge 15 commits into
alex-rawlings-yyc wants to merge 15 commits into
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>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR adds shared-analysis deduping and suggestion pooling, wires confirmation-aware edit flows through the store and UI, and updates token and morpheme inputs to render suggestion actions and revert held drafts. It also adds localized copy, Tailwind status classes, and tests. ChangesShared-analysis suggestions and global-edit confirmation
Sequence Diagram(s)sequenceDiagram
participant TokenChip
participant AnalysisStoreProvider
participant GlobalEditConfirmationModal
participant analysisSlice
TokenChip->>AnalysisStoreProvider: requestGlossEdit(tokenRef, surfaceText, value)
AnalysisStoreProvider->>GlobalEditConfirmationModal: render pendingEdit for shared payload
GlobalEditConfirmationModal->>AnalysisStoreProvider: onUpdateAll / onForkInstead / onCancel
AnalysisStoreProvider->>analysisSlice: writeGloss(...) or forkAnalysisForToken(...)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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
Contributor
Author
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
Contributor
Author
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change is
Summary by CodeRabbit
New Features
Bug Fixes