From 13af7821c3ec3a1742ffd2901b58ec938bbd4467 Mon Sep 17 00:00:00 2001 From: Joshua Horton Date: Mon, 20 Jul 2026 13:17:05 -0500 Subject: [PATCH 1/7] fix(web): prevent backspace from clustering with output keys --- .../src/main/correction/tokenization-subsets.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/web/src/engine/predictive-text/worker-thread/src/main/correction/tokenization-subsets.ts b/web/src/engine/predictive-text/worker-thread/src/main/correction/tokenization-subsets.ts index bfbadef182e..a8617a7c0f6 100644 --- a/web/src/engine/predictive-text/worker-thread/src/main/correction/tokenization-subsets.ts +++ b/web/src/engine/predictive-text/worker-thread/src/main/correction/tokenization-subsets.ts @@ -135,7 +135,8 @@ export function legacySubsetKeyer(tokenizationEdits: TokenizationTransitionEdits // Now, based on the transform tokenization. We want to force uniqueness for // all variations of result length on each tokenized transform resulting from // the precomputation's represented keystroke. - for(const {0: relativeIndex} of tokenizedTransform.entries()) { + for(const {0: relativeIndex, 1: transform} of tokenizedTransform.entries()) { + const insertLen = KMWString.length(transform.insert); if(relativeIndex > 0) { // The true boundary lie before the insert if the value is non-zero; // don't differentiate here! @@ -148,10 +149,10 @@ export function legacySubsetKeyer(tokenizationEdits: TokenizationTransitionEdits // // IMPORTANT: update unit tests manually if the BI marker here changes // or the use of SENTINEL_CODE_UNIT as a key component separator changes. - components.push(`BI@${relativeIndex}`); + components.push(`BI@${relativeIndex}-${boundaryTextLen + insertLen}`); boundaryTextLen = 0; } else { - components.push(`I@${relativeIndex}`); + components.push(`I@${relativeIndex}-${boundaryTextLen + insertLen}`); } } From 5a1e23d72711275122b5d303bccd83dd1978b84b Mon Sep 17 00:00:00 2001 From: Joshua Horton Date: Tue, 21 Jul 2026 13:39:11 -0500 Subject: [PATCH 2/7] fix(web): handle prediction whitespace-input, backspace-input edge cases better Fixes: #16271 Fixes: #16272 Build-bot: skip release:web,android,ios --- .../src/main/correction/context-tokenization.ts | 13 ++++++++++++- .../worker-thread/src/main/predict-helpers.ts | 14 +++++++++----- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/web/src/engine/predictive-text/worker-thread/src/main/correction/context-tokenization.ts b/web/src/engine/predictive-text/worker-thread/src/main/correction/context-tokenization.ts index 52c365dd7eb..4ccd8cb86dc 100644 --- a/web/src/engine/predictive-text/worker-thread/src/main/correction/context-tokenization.ts +++ b/web/src/engine/predictive-text/worker-thread/src/main/correction/context-tokenization.ts @@ -708,8 +708,19 @@ export class ContextTokenization { affectedToken = null; } + // Backspace handling - emptying context via backspace or erasing _part_ of + // a whitespace token can erase the tokenization-final empty token usually + // used for word-initial suggestions. + // + // We re-add it here so that suggestions can be presented to the user as + // normal. + const tokenSequence = this.tokens.slice(0, sliceIndex).concat(tailTokenization); + if(tokenSequence.length == 0 || tokenSequence[tokenSequence.length - 1]?.isWhitespace) { + tokenSequence.push(new ContextToken(new LegacyQuotientRoot(lexicalModel))); + } + return new ContextTokenization( - this.tokens.slice(0, sliceIndex).concat(tailTokenization), + tokenSequence, null, determineTaillessTrueKeystroke(transitionEdge) ); diff --git a/web/src/engine/predictive-text/worker-thread/src/main/predict-helpers.ts b/web/src/engine/predictive-text/worker-thread/src/main/predict-helpers.ts index ac0de6c5039..665db610ef5 100644 --- a/web/src/engine/predictive-text/worker-thread/src/main/predict-helpers.ts +++ b/web/src/engine/predictive-text/worker-thread/src/main/predict-helpers.ts @@ -578,11 +578,6 @@ export async function correctAndEnumerate( // Corrections obtained: now to predict from them! const tokenization = tokenizations.find(t => t.spaceId == match.spaceId); - // If our 'match' results in fully deleting the new token, reject it and try again. - if(match.matchSequence.length == 0 && match.inputSequence.length != 0) { - continue; - } - // If our 'match' fully replaces the token, reject it and try again. if(match.matchSequence.length != 0 && match.matchSequence.length == match.knownCost) { continue; @@ -609,6 +604,12 @@ export async function correctAndEnumerate( const predictions = buildAndMapPredictions(transition, tokenization, match, costFactor); + // Backspaces that cause the whole context to become empty do not reflect the backspace transform + // within the search space. We correct for that here. + if(tokenization.tail.searchModule.codepointLength == 0 && inputTransform.deleteLeft > 0) { + predictions.forEach((p) => p.prediction.sample.transform.deleteLeft += inputTransform.deleteLeft); + } + // Only set 'best correction' cost when a correction ACTUALLY YIELDS predictions. if(predictions.length > 0 && bestCorrectionCost === undefined) { bestCorrectionCost = match.totalCost * costFactor; @@ -1077,6 +1078,9 @@ export function finalizeSuggestions( if(presDL > 0) { mergedTransform.deleteLeft -= presDL; } + if(prediction.sample.transform.id !== undefined) { + mergedTransform.id = prediction.sample.transform.id; + } // Temporarily and locally drops 'readonly' semantics so that we can reassign the transform. // See https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-8.html#improved-control-over-mapped-type-modifiers From 26703a4e6d097264dd64033600be9b638e6e2728 Mon Sep 17 00:00:00 2001 From: Joshua Horton Date: Wed, 22 Jul 2026 15:39:53 -0500 Subject: [PATCH 3/7] change(web): double the permitted transcription-cache size This will help in cases where a user wishes to revert a suggestion after numerous other edits. --- web/src/engine/src/main/headless/transcriptionCache.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/engine/src/main/headless/transcriptionCache.ts b/web/src/engine/src/main/headless/transcriptionCache.ts index 4a3e6408a65..7ff3832b9b9 100644 --- a/web/src/engine/src/main/headless/transcriptionCache.ts +++ b/web/src/engine/src/main/headless/transcriptionCache.ts @@ -1,7 +1,7 @@ import { Transcription } from "keyman/engine/keyboard"; import { RewindableCache } from "keyman/common/web-utils"; -const TRANSCRIPTION_BUFFER_SIZE = 10; +const TRANSCRIPTION_BUFFER_SIZE = 20; export class TranscriptionCache extends RewindableCache { constructor() { From e225b26f10b7c2a0bc9fcbd6a429d4ff978e6bfd Mon Sep 17 00:00:00 2001 From: Joshua Horton Date: Thu, 23 Jul 2026 15:52:53 -0500 Subject: [PATCH 4/7] change(web): add unit tests for issues fixed by this PR --- .../worker-thread/src/main/predict-helpers.ts | 17 +- .../context/context-tokenization.tests.ts | 37 +++ .../context/tokenization-subsets.tests.ts | 51 +++- .../build-and-map-predictions.tests.ts | 220 ++++++++++++++++++ 4 files changed, 316 insertions(+), 9 deletions(-) create mode 100644 web/src/test/auto/headless/engine/predictive-text/worker-thread/prediction-helpers/build-and-map-predictions.tests.ts diff --git a/web/src/engine/predictive-text/worker-thread/src/main/predict-helpers.ts b/web/src/engine/predictive-text/worker-thread/src/main/predict-helpers.ts index 665db610ef5..5aa73200eee 100644 --- a/web/src/engine/predictive-text/worker-thread/src/main/predict-helpers.ts +++ b/web/src/engine/predictive-text/worker-thread/src/main/predict-helpers.ts @@ -13,7 +13,6 @@ import { ContextTransition } from './correction/context-transition.js'; import { ExecutionTimer } from './correction/execution-timer.js'; import { ModelCompositor } from './model-compositor.js'; import { getBestTokenMatches } from './correction/distance-modeler.js'; -import { TokenResultMapping } from './correction/token-result-mapping.js'; import CasingForm = LexicalModelTypes.CasingForm; import Context = LexicalModelTypes.Context; @@ -459,7 +458,8 @@ export function determineSuggestionRange( export function buildAndMapPredictions( transition: ContextTransition, tokenization: ContextTokenization, - match: Readonly, + // Originally, Readonly - but we only need these two components here. + match: Readonly<{matchString: string, totalCost: number}>, costFactor: number ): CorrectionPredictionTuple[] { const model = transition.final.model; @@ -493,6 +493,13 @@ export function buildAndMapPredictions( // entry.baseTokenization = transition.final.tokenizationSourceMap.get(tokenization); }); + // Backspaces that shorten a multi-codepoint whitespace token are not handled well by default. + // As a new empty token is placed at the end for such cases, we can detect and handle such cases. + const inputTransform = transition.inputDistribution?.[0].sample ?? { insert: '', deleteLeft: 0 }; + if(tokenization.tokens.length > 1 && tokenization.tail.searchModule.codepointLength == 0 && inputTransform.deleteLeft > 0) { + predictions.forEach((p) => p.prediction.sample.transform.deleteLeft += inputTransform.deleteLeft); + } + return predictions; } @@ -604,12 +611,6 @@ export async function correctAndEnumerate( const predictions = buildAndMapPredictions(transition, tokenization, match, costFactor); - // Backspaces that cause the whole context to become empty do not reflect the backspace transform - // within the search space. We correct for that here. - if(tokenization.tail.searchModule.codepointLength == 0 && inputTransform.deleteLeft > 0) { - predictions.forEach((p) => p.prediction.sample.transform.deleteLeft += inputTransform.deleteLeft); - } - // Only set 'best correction' cost when a correction ACTUALLY YIELDS predictions. if(predictions.length > 0 && bestCorrectionCost === undefined) { bestCorrectionCost = match.totalCost * costFactor; diff --git a/web/src/test/auto/headless/engine/predictive-text/worker-thread/context/context-tokenization.tests.ts b/web/src/test/auto/headless/engine/predictive-text/worker-thread/context/context-tokenization.tests.ts index dcef6f8f615..8f93bd20f4a 100644 --- a/web/src/test/auto/headless/engine/predictive-text/worker-thread/context/context-tokenization.tests.ts +++ b/web/src/test/auto/headless/engine/predictive-text/worker-thread/context/context-tokenization.tests.ts @@ -397,6 +397,43 @@ describe('ContextTokenization', function() { ); }); + it('handles simple case - deletion of final context content via backspace', () => { + const baseTokens = ['a']; + const baseTokenization = new ContextTokenization(baseTokens.map(t => toToken(t))); + + const targetTokens = [''].map((t) => ({text: t, isWhitespace: t == ' '})); + const inputTransform = { insert: '', deleteLeft: 1, deleteRight: 0, id: 42 }; + const inputTransformMap: Map = new Map(); + inputTransformMap.set(0, { insert: '', deleteLeft: 1, id: 42 }); + + const edgeWindow = buildEdgeWindow(baseTokenization.tokens, inputTransform, false, testEdgeWindowSpec); + const tokenization = baseTokenization.evaluateTransition({ + alignment: { + merges: [], + splits: [], + unmappedEdits: [], + edgeWindow: { + ...edgeWindow, + // The range within the window constructed by the prior call for its parameterization. + // Any adjustments on the boundary token itself are included here. + retokenization: [...targetTokens.slice(edgeWindow.sliceIndex).map(t => t.text)] + }, + removedTokenCount: 1 + }, + inputs: [{ sample: inputTransformMap, p: 1 }], + inputSubsetId: generateSubsetId() + }, + inputTransform.id, + 1 + ); + + assert.isOk(tokenization); + assert.equal(tokenization.tokens.length, targetTokens.length); + assert.deepEqual(tokenization.tokens.map((t) => ({text: t.exampleInput, isWhitespace: t.isWhitespace})), + targetTokens + ); + }); + it('handles simple case - new character added to last token', () => { const baseTokens = ['an', ' ', 'apple', ' ', 'a', ' ', 'da']; const baseTokenization = new ContextTokenization(baseTokens.map(t => toToken(t))); diff --git a/web/src/test/auto/headless/engine/predictive-text/worker-thread/context/tokenization-subsets.tests.ts b/web/src/test/auto/headless/engine/predictive-text/worker-thread/context/tokenization-subsets.tests.ts index 04647123c02..b924d9422cb 100644 --- a/web/src/test/auto/headless/engine/predictive-text/worker-thread/context/tokenization-subsets.tests.ts +++ b/web/src/test/auto/headless/engine/predictive-text/worker-thread/context/tokenization-subsets.tests.ts @@ -21,6 +21,7 @@ import { ContextToken, ContextTokenization, generateSubsetId, + legacySubsetKeyer, models, precomputationSubsetKeyer, TokenizationTransitionEdits, @@ -31,7 +32,7 @@ import Distribution = LexicalModelTypes.Distribution; import Transform = LexicalModelTypes.Transform; import TrieModel = models.TrieModel; -var plainModel = new TrieModel(jsonFixture('models/tries/english-1000'), +const plainModel = new TrieModel(jsonFixture('models/tries/english-1000'), {wordBreaker: defaultBreaker}); function toToken(text: string) { @@ -41,6 +42,54 @@ function toToken(text: string) { return token; } +describe('legacySubsetKeyer', () => { + it('does not map backspace inputs to same result as standard key inputs', () => { + const appleToken = ContextToken.fromRawText(plainModel, 'apple', true); + + const bksp = { insert: '', deleteLeft: 1, deleteRight: 0, id: 3 }; + const bkspKey = legacySubsetKeyer({ + alignment: { + merges: [], + splits: [], + unmappedEdits: [], + edgeWindow: { + ...buildEdgeWindow([appleToken], bksp, false), + retokenization: ['appl'], + retokenizationText: 'appl' + }, + removedTokenCount: 0 + }, + tokenizedTransform: (() => { + const map: Map = new Map(); + map.set(0, bksp); + return map; + })() + }); + + const input = { insert: 's', deleteLeft: 0, deleteRight: 0, id: bksp.id }; + const inputKey = legacySubsetKeyer({ + alignment: { + merges: [], + splits: [], + unmappedEdits: [], + edgeWindow: { + ...buildEdgeWindow([appleToken], input, false), + retokenization: ['apples'], + retokenizationText: 'apples' + }, + removedTokenCount: 0 + }, + tokenizedTransform: (() => { + const map: Map = new Map(); + map.set(0, input); + return map; + })() + }); + + assert.notEqual(bkspKey, inputKey); + }); +}); + describe('precomputationSubsetKeyer', function() { it("safely generates keys for empty transition + empty contexts", () => { const rawTextTokens = ['']; diff --git a/web/src/test/auto/headless/engine/predictive-text/worker-thread/prediction-helpers/build-and-map-predictions.tests.ts b/web/src/test/auto/headless/engine/predictive-text/worker-thread/prediction-helpers/build-and-map-predictions.tests.ts new file mode 100644 index 00000000000..73d8276e132 --- /dev/null +++ b/web/src/test/auto/headless/engine/predictive-text/worker-thread/prediction-helpers/build-and-map-predictions.tests.ts @@ -0,0 +1,220 @@ + +/* + * Keyman is copyright (C) SIL Global. MIT License. + * + * Created by jahorton on 2026-07-23 + * + * This file contains tests designed to validate the behavior of the + * buildAndMapPredictions helper function class and its integration with the + * lower-level predictive-text helpers. + */ + + +import { assert } from 'chai'; + +import { default as defaultBreaker } from '@keymanapp/models-wordbreakers'; +import { LexicalModelTypes } from '@keymanapp/common-types'; +import { jsonFixture } from '@keymanapp/common-test-resources/model-helpers.mjs'; +import { TrieModel } from '@keymanapp/models-templates'; + +import { buildAndMapPredictions, buildEdgeWindow, ContextState, ContextToken, ContextTokenization, ContextTransition, generateSubsetId, LegacyQuotientRoot, LegacyQuotientSpur, models, predictFromCorrections } from "@keymanapp/lm-worker/test-index"; + +import Context = LexicalModelTypes.Context; +import Distribution = LexicalModelTypes.Distribution; +import ProbabilityMass = LexicalModelTypes.ProbabilityMass; +import Transform = LexicalModelTypes.Transform; + +const plainModel = new TrieModel(jsonFixture('models/tries/english-1000'), + {wordBreaker: defaultBreaker}); + +describe('buildAndMapPredictions', () => { + it('adds the preservation transform to all generated predictions', () => { + const context: Context = { + left: 'th', + right: '', + startOfBuffer: true, + endOfBuffer: true + }; + + const correctionDistribution: Distribution = [{ + sample: { + insert: 'e', + deleteLeft: 0 + }, + p: 0.6 + } + ]; + + const basePredictions = predictFromCorrections(plainModel, correctionDistribution, context); + basePredictions.forEach((entry) => assert.isNotOk(entry.preservationTransform)); + + // must construct the taillessTrueKeystroke appropriately. + const tailless = { insert: 'TEST', deleteLeft: 0 }; + const tokenization = new ContextTokenization([ContextToken.fromRawText(plainModel, 'th', true)], null, tailless); + const transition = new ContextTransition(new ContextState(context, plainModel, tokenization), 0); + + const targetTokenization = new ContextTokenization([new ContextToken(new LegacyQuotientSpur(tokenization.tail.searchModule, correctionDistribution, correctionDistribution[0]))]); + transition.finalize(new ContextState(models.applyTransform(correctionDistribution[0].sample, context), plainModel, targetTokenization), correctionDistribution); + + const mappedPredictions = buildAndMapPredictions( + transition, + transition.base.displayTokenization, + {matchString: 'the', totalCost: 0}, + 1 + ); + + assert.deepEqual(mappedPredictions.map((tuple) => tuple.prediction), basePredictions.map((tuple) => tuple.prediction)); + mappedPredictions.forEach((tuple) => assert.isOk(tuple.preservationTransform)); + mappedPredictions.forEach((tuple) => tuple.preservationTransform == tailless); + }); + + it('properly handles empty prediction roots from deleted same-token codepoints', () => { + const context: Context = { + left: 'the a', + right: '', + startOfBuffer: true, + endOfBuffer: true + }; + + const correctionDistribution: Distribution = [{ + sample: { + insert: '', + deleteLeft: 1 + }, + p: 1 + } + ]; + + const basePredictions = predictFromCorrections(plainModel, correctionDistribution, context); + + // must construct the taillessTrueKeystroke appropriately. + const tokenization = new ContextTokenization([ + ContextToken.fromRawText(plainModel, 'the', false), + ContextToken.fromRawText(plainModel, ' ', false), + ContextToken.fromRawText(plainModel, 'a', true) + ]); + const transition = new ContextTransition(new ContextState(context, plainModel, tokenization), 0); + + const targetTokenization = new ContextTokenization([ + tokenization.tokens[0], + tokenization.tokens[1], + new ContextToken(new LegacyQuotientRoot(plainModel)) + ]); + transition.finalize(new ContextState(models.applyTransform(correctionDistribution[0].sample, context), plainModel, targetTokenization), correctionDistribution); + + const mappedPredictions = buildAndMapPredictions( + transition, + transition.base.displayTokenization, + {matchString: '', totalCost: 0}, + 1 + ); + + assert.deepEqual(mappedPredictions.map((tuple) => tuple.prediction), basePredictions.map((tuple) => tuple.prediction)); + }); + + it('properly handles empty prediction roots caused by backspacing one of multiple spaces', () => { + const context: Context = { + left: 'the ', + right: '', + startOfBuffer: true, + endOfBuffer: true + }; + + const correctionDistribution: Distribution = [{ + sample: { + insert: '', + deleteLeft: 1 + }, + p: 1 + } + ]; + + const basePredictions = predictFromCorrections(plainModel, correctionDistribution, context); + + // must construct the taillessTrueKeystroke appropriately. + const tokenization = new ContextTokenization([ + ContextToken.fromRawText(plainModel, 'the', false), + ContextToken.fromRawText(plainModel, ' ', false), + ContextToken.fromRawText(plainModel, '', true) + ]); + const transition = new ContextTransition(new ContextState(context, plainModel, tokenization), 0); + + const targetTokenization = new ContextTokenization([ + tokenization.tokens[0], + new ContextToken(new LegacyQuotientSpur(tokenization.tokens[1].searchModule, correctionDistribution, correctionDistribution[0])), + new ContextToken(new LegacyQuotientRoot(plainModel)) + ]); + transition.finalize(new ContextState(models.applyTransform(correctionDistribution[0].sample, context), plainModel, targetTokenization), correctionDistribution); + + const mappedPredictions = buildAndMapPredictions( + transition, + transition.base.displayTokenization, + {matchString: '', totalCost: 0}, + 1 + ); + + assert.deepEqual(mappedPredictions.map((tuple) => tuple.prediction), basePredictions.map((tuple) => tuple.prediction)); + }); + + it('properly handles contexts made empty by input backspace', () => { + const context: Context = { + left: 't', + right: '', + startOfBuffer: true, + endOfBuffer: true + }; + + const correctionDistribution = [{ + sample: { + insert: '', + deleteLeft: 1, + deleteRight: 0 + }, + p: 1 + } + ]; + + // must construct the taillessTrueKeystroke appropriately. + const tokenization = new ContextTokenization([ + ContextToken.fromRawText(plainModel, 't', true) + ]); + const transition = new ContextTransition(new ContextState(context, plainModel, tokenization), 0); + + const targetTokenization = new ContextTokenization([ + new ContextToken(new LegacyQuotientRoot(plainModel)) + ], { + alignment: { + merges: [], + splits: [], + unmappedEdits: [], + edgeWindow: { + ...buildEdgeWindow(tokenization.tokens, correctionDistribution[0].sample, false), + retokenization: [''], + retokenizationText: '' + }, + removedTokenCount: 0 + }, + inputs: (() => { + const val: ProbabilityMass>[] = [{ + sample: new Map(), + p: correctionDistribution[0].p + }]; + + val[0].sample.set(0, correctionDistribution[0].sample); + + return val; + })(), + inputSubsetId: generateSubsetId() + }, null); + transition.finalize(new ContextState(models.applyTransform(correctionDistribution[0].sample, context), plainModel, targetTokenization), correctionDistribution); + + const mappedPredictions = buildAndMapPredictions( + transition, + transition.final.displayTokenization, + {matchString: '', totalCost: 0}, + 1 + ); + + mappedPredictions.forEach((tuple) => assert.equal(tuple.prediction.sample.transform.deleteLeft, 1)); + }); +}); \ No newline at end of file From f864bf293a473b3eeccf0b0d23af10cecfac3375 Mon Sep 17 00:00:00 2001 From: Joshua Horton Date: Mon, 27 Jul 2026 12:11:50 -0500 Subject: [PATCH 5/7] change(web): spin off method, add unit tests to verify handling of keep vs revert suggestions --- .../src/main/correction/context-transition.ts | 9 ++ .../src/main/model-compositor.ts | 20 +--- .../worker-thread/src/main/predict-helpers.ts | 33 +++++- .../worker-thread/src/main/test-index.ts | 2 +- .../prepend-reversion.tests.ts | 101 ++++++++++++++++++ 5 files changed, 148 insertions(+), 17 deletions(-) create mode 100644 web/src/test/auto/headless/engine/predictive-text/worker-thread/prediction-helpers/prepend-reversion.tests.ts diff --git a/web/src/engine/predictive-text/worker-thread/src/main/correction/context-transition.ts b/web/src/engine/predictive-text/worker-thread/src/main/correction/context-transition.ts index c65f3489963..ede7125a9c0 100644 --- a/web/src/engine/predictive-text/worker-thread/src/main/correction/context-transition.ts +++ b/web/src/engine/predictive-text/worker-thread/src/main/correction/context-transition.ts @@ -19,6 +19,15 @@ import Reversion = LexicalModelTypes.Reversion; import Suggestion = LexicalModelTypes.Suggestion; import Transform = LexicalModelTypes.Transform; +export interface TransitionReversionView extends Pick { + /** + * Gets the context state resulting from the context transition event, + * including any generated suggestions and data regarding potential + * application thereof. + */ + final: Pick +} + /** * Represents the transition between two context states as triggered * by input keystrokes or applied suggestions. diff --git a/web/src/engine/predictive-text/worker-thread/src/main/model-compositor.ts b/web/src/engine/predictive-text/worker-thread/src/main/model-compositor.ts index 8e12b96ed49..1a514cad649 100644 --- a/web/src/engine/predictive-text/worker-thread/src/main/model-compositor.ts +++ b/web/src/engine/predictive-text/worker-thread/src/main/model-compositor.ts @@ -2,7 +2,7 @@ import * as models from '@keymanapp/models-templates'; import { LexicalModelTypes } from '@keymanapp/common-types'; import { TransformUtils } from './transformUtils.js'; -import { applySuggestionCasing, correctAndEnumerate, createDefaultKeep, dedupeSuggestions, finalizeSuggestions, predictionAutoSelect, processSimilarity, toAnnotatedSuggestion, tupleDisplayOrderSort } from './predict-helpers.js'; +import { applySuggestionCasing, correctAndEnumerate, createDefaultKeep, dedupeSuggestions, finalizeSuggestions, predictionAutoSelect, prependReversion, processSimilarity, toAnnotatedSuggestion, tupleDisplayOrderSort } from './predict-helpers.js'; import { detectCurrentCasing, determineModelTokenizer, determineModelWordbreaker, determinePunctuationFromModel } from './model-helpers.js'; import { ContextTracker } from './correction/context-tracker.js'; @@ -206,18 +206,8 @@ export class ModelCompositor { this.SUGGESTION_ID_SEED++; }); - if(revertableTransitionId) { - const reversion = this.contextTracker.peek(revertableTransitionId)?.reversion; - if(reversion) { - if(suggestions[0]?.tag == 'keep') { - const keep = suggestions.shift(); - suggestions.unshift(reversion); - suggestions.unshift(keep); - } else { - suggestions.unshift(reversion) - } - } - } + const transitionToRevert = this.contextTracker?.peek(revertableTransitionId); + prependReversion(suggestions, transitionToRevert); // Store the suggestions on the final token of the current context state (if it exists). // Or, once phrase-level suggestions are possible, on whichever token serves as each prediction's root. @@ -239,8 +229,8 @@ export class ModelCompositor { // Step 1: re-use the original input Transform as the reversion's Transform. // The Web engine will restore the original state of the context before accepting // and before reverting; all we need to do is put the original keystroke back in place. - let reversionTransform: Transform = originalInput - ? { ...originalInput } + let reversionTransform: Transform = originalInput + ? { ...originalInput } : { insert: '', deleteLeft: 0, id: suggestion.transform.id }; // Step 2: building the proper 'displayAs' string for the Reversion diff --git a/web/src/engine/predictive-text/worker-thread/src/main/predict-helpers.ts b/web/src/engine/predictive-text/worker-thread/src/main/predict-helpers.ts index 5aa73200eee..9047e80a831 100644 --- a/web/src/engine/predictive-text/worker-thread/src/main/predict-helpers.ts +++ b/web/src/engine/predictive-text/worker-thread/src/main/predict-helpers.ts @@ -9,7 +9,7 @@ import { ContextTokenization } from './correction/context-tokenization.js'; import { ContextTracker } from './correction/context-tracker.js'; import { ContextToken } from './correction/context-token.js'; import { ContextState, determineContextSlideTransform } from './correction/context-state.js'; -import { ContextTransition } from './correction/context-transition.js'; +import { ContextTransition, TransitionReversionView } from './correction/context-transition.js'; import { ExecutionTimer } from './correction/execution-timer.js'; import { ModelCompositor } from './model-compositor.js'; import { getBestTokenMatches } from './correction/distance-modeler.js'; @@ -1196,4 +1196,35 @@ export function toAnnotatedSuggestion( } return result; +} + +/** + * For applicable scenarios, this mutates the passed-in suggestion array by + * prepending a predictive-text reversion that restores the context to a prior + * state. Otherwise, it leaves the suggestion array unaltered. + * @param suggestions + * @param transitionToRevert + * @returns + */ +export function prependReversion(suggestions: Suggestion[], transitionToRevert: TransitionReversionView) { + if(transitionToRevert) { + const reversion = transitionToRevert.reversion; + if(reversion) { + if(suggestions[0]?.tag == 'keep') { + const appliedId = -reversion.id; + const appliedSuggestion = transitionToRevert.final.suggestions.find((s) => s.id == appliedId); + // If the selected suggestion was itself a `keep`, we don't need a + // reversion. They'd do the same thing. + if(appliedSuggestion.tag != 'keep') { + const keep = suggestions.shift(); + suggestions.unshift(reversion); + suggestions.unshift(keep); + } + } else { + suggestions.unshift(reversion); + } + } + } + + return suggestions; } \ No newline at end of file diff --git a/web/src/engine/predictive-text/worker-thread/src/main/test-index.ts b/web/src/engine/predictive-text/worker-thread/src/main/test-index.ts index e0c5d513f95..bcaf4692bf7 100644 --- a/web/src/engine/predictive-text/worker-thread/src/main/test-index.ts +++ b/web/src/engine/predictive-text/worker-thread/src/main/test-index.ts @@ -3,7 +3,7 @@ export * from './correction/context-state.js'; export * from './correction/context-token.js'; export * from './correction/context-tokenization.js'; export { ContextTracker } from './correction/context-tracker.js'; -export { ContextTransition } from './correction/context-transition.js'; +export * from './correction/context-transition.js'; export * from './correction/correction-searchable.js'; export * from './correction/correction-result-mapping.js'; export * from './correction/distance-modeler.js'; diff --git a/web/src/test/auto/headless/engine/predictive-text/worker-thread/prediction-helpers/prepend-reversion.tests.ts b/web/src/test/auto/headless/engine/predictive-text/worker-thread/prediction-helpers/prepend-reversion.tests.ts new file mode 100644 index 00000000000..5215705ed6c --- /dev/null +++ b/web/src/test/auto/headless/engine/predictive-text/worker-thread/prediction-helpers/prepend-reversion.tests.ts @@ -0,0 +1,101 @@ + +/* + * Keyman is copyright (C) SIL Global. MIT License. + * + * Created by jahorton on 2026-07-27 + * + * This file contains tests designed to ensure predictive text does not + * provide matching 'keep' and 'revert' suggestions in any context. + */ + + +import { assert } from 'chai'; + +import { LexicalModelTypes } from '@keymanapp/common-types'; +import { prependReversion, type TransitionReversionView } from "@keymanapp/lm-worker/test-index"; + +import Suggestion = LexicalModelTypes.Suggestion; + +describe('prependReversion', () => { + it(`prepends reversions when reverting a non-'keep' suggestion`, () => { + // context: Original was appl+u, corrected to apply. Reached via bksp. + const suggestions: Suggestion[] = [{ + tag: 'keep', + transform: { insert: 'apply', deleteLeft: 6, id: 3 }, + displayAs: '"apply"', + id: 5, + matchesModel: false + } as Suggestion]; + + const revertable: TransitionReversionView = { + reversion: { + tag: 'revert', + transform: { insert: 'u', deleteLeft: 0, id: 3 }, + id: -3, + displayAs: '"applu"' + }, + final: { + suggestions: [{ + tag: 'keep', + transform: { insert: 'applu', deleteLeft: 4, id: 3 }, + displayAs: '"applu"', + id: 2, + matchesModel: false + } as Suggestion, { + transform: { insert: 'apply', deleteLeft: 4, id: 3 }, + displayAs: 'apply', + id: 3 + }, { + transform: { insert: 'applied', deleteLeft: 4, id: 3 }, + displayAs: 'applied', + id: 4 + }] + } + }; + + prependReversion(suggestions, revertable); + + assert.includeMembers(suggestions, [revertable.reversion]); + }); + + it(`does not prepend reversions when reverting a 'keep' suggestion`, () => { + // context: Original was appl+u, corrected to apply + const suggestions: Suggestion[] = [{ + tag: 'keep', + transform: { insert: 'applu', deleteLeft: 5, id: 3 }, + displayAs: '"applu"', + id: 5, + matchesModel: false + } as Suggestion]; + + const revertable: TransitionReversionView = { + reversion: { + tag: 'revert', + transform: { insert: 'u', deleteLeft: 0, id: 3 }, + id: -2, + displayAs: '"applu"' + }, + final: { + suggestions: [{ + tag: 'keep', + transform: { insert: 'applu', deleteLeft: 4, id: 3 }, + displayAs: '"applu"', + id: 2, + matchesModel: false + } as Suggestion, { + transform: { insert: 'apply', deleteLeft: 4, id: 3 }, + displayAs: 'apply', + id: 3 + }, { + transform: { insert: 'applied', deleteLeft: 4, id: 3 }, + displayAs: 'applied', + id: 4 + }] + } + }; + + prependReversion(suggestions, revertable); + + assert.notIncludeMembers(suggestions, [revertable.reversion]); + }); +}); \ No newline at end of file From 7db38f93ec303f0c07774964b42237090b0c9792 Mon Sep 17 00:00:00 2001 From: Joshua Horton Date: Tue, 28 Jul 2026 08:40:44 -0500 Subject: [PATCH 6/7] fix(web): do not remove applied transition ID from original token when editing --- .../src/main/correction/context-tokenization.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/web/src/engine/predictive-text/worker-thread/src/main/correction/context-tokenization.ts b/web/src/engine/predictive-text/worker-thread/src/main/correction/context-tokenization.ts index 4ccd8cb86dc..61b06092abd 100644 --- a/web/src/engine/predictive-text/worker-thread/src/main/correction/context-tokenization.ts +++ b/web/src/engine/predictive-text/worker-thread/src/main/correction/context-tokenization.ts @@ -673,9 +673,6 @@ export class ContextTokenization { tailTokenization.splice(tokenIndex, 1, affectedToken); } - affectedToken.isPartial = true; - delete affectedToken.appliedTransitionId; - // If we are completely replacing a token via delete left, erase the deleteLeft; // that part applied to a _previous_ token that no longer exists. // We start at index 0 in the insert string for the "new" token. @@ -699,6 +696,11 @@ export class ContextTokenization { affectedToken = new ContextToken(affectedToken); affectedToken.addInput(inputSource, distribution); + // Do not adjust the original token, as it may be used by other transitions. + // Only adjust the new, extended token. + affectedToken.isPartial = true; + delete affectedToken.appliedTransitionId; + const tokenize = determineModelTokenizer(lexicalModel); affectedToken.isWhitespace = tokenize({left: affectedToken.exampleInput, startOfBuffer: false, endOfBuffer: false}).left[0]?.isWhitespace ?? false; // Do not re-use the previous token; the mutation may have unexpected From f2e160c232d5ecb6b600c2e86f5961096f7d3f17 Mon Sep 17 00:00:00 2001 From: Joshua Horton Date: Tue, 28 Jul 2026 12:01:03 -0500 Subject: [PATCH 7/7] feat(web): add unit tests for prior commit --- .../context/context-tokenization.tests.ts | 192 ++++++++++++++++++ 1 file changed, 192 insertions(+) diff --git a/web/src/test/auto/headless/engine/predictive-text/worker-thread/context/context-tokenization.tests.ts b/web/src/test/auto/headless/engine/predictive-text/worker-thread/context/context-tokenization.tests.ts index 8f93bd20f4a..0b10f90c892 100644 --- a/web/src/test/auto/headless/engine/predictive-text/worker-thread/context/context-tokenization.tests.ts +++ b/web/src/test/auto/headless/engine/predictive-text/worker-thread/context/context-tokenization.tests.ts @@ -860,6 +860,198 @@ describe('ContextTokenization', function() { assert.equal(preTail.exampleInput, '\''); assert.equal(tail.exampleInput, '.'); }); + + describe('properly handles previously-applied transition IDs', () => { + it('does not preserve applied transition IDs on edited tokens', () => { + const baseTokens = ['an', ' ', 'apple', ' ', 'a', ' ', 'day', ' ', 'can']; + const baseTokenization = new ContextTokenization(baseTokens.map(t => toToken(t))); + + const REVERTABLE_TRANSITION_ID = 31415; + baseTokenization.tail.appliedTransitionId = REVERTABLE_TRANSITION_ID; + const NEW_TRANSITION_ID = REVERTABLE_TRANSITION_ID + 1; + + const dist = [ + { + sample: { insert: 't', deleteLeft: 0, deleteRight: 0, id: NEW_TRANSITION_ID }, + p: 1 + } + ]; + + const resultTokenization = baseTokenization.evaluateTransition( + { + alignment: { + merges: [], + splits: [], + unmappedEdits: [], + edgeWindow: { + ...buildEdgeWindow(baseTokenization.tokens, dist[0].sample, false), + retokenization: baseTokenization.tokens.map((t) => t.exampleInput), + retokenizationText: baseTokenization.tokens.map((t) => t.exampleInput).reduce((accum, curr) => accum + curr, '') + }, + removedTokenCount: 0 + }, + inputs: (() => { + const map: Map = new Map(); + map.set(0, dist[0].sample); + + return [ + {sample: map, p: 1} + ]; + })(), + inputSubsetId: 0 + }, + NEW_TRANSITION_ID, + 1 + ) + + resultTokenization.tokens.forEach((t) => { + assert.isUndefined(t.appliedTransitionId); + }); + }); + + it('preserves applied transition IDs on applicable tokens', () => { + const baseTokens = ['an', ' ', 'apple', ' ', 'a', ' ', 'day', ' ', 'can']; + const baseTokenization = new ContextTokenization(baseTokens.map(t => toToken(t))); + + const REVERTABLE_TRANSITION_ID = 31415; + baseTokenization.tail.appliedTransitionId = REVERTABLE_TRANSITION_ID; + const NEW_TRANSITION_ID = REVERTABLE_TRANSITION_ID + 1; + + + const dist = [ + { + sample: { insert: ' ', deleteLeft: 0, deleteRight: 0, id: NEW_TRANSITION_ID }, + p: 1 + } + ]; + + const resultTokenization = baseTokenization.evaluateTransition( + { + alignment: { + merges: [], + splits: [], + unmappedEdits: [], + edgeWindow: { + ...buildEdgeWindow(baseTokenization.tokens, dist[0].sample, false), + retokenization: baseTokenization.tokens.map((t) => t.exampleInput), + retokenizationText: baseTokenization.tokens.map((t) => t.exampleInput).reduce((accum, curr) => accum + curr, '') + }, + removedTokenCount: 0 + }, + inputs: (() => { + const map: Map = new Map(); + map.set(1, dist[0].sample); + + return [ + {sample: map, p: 1} + ]; + })(), + inputSubsetId: 0 + }, + NEW_TRANSITION_ID, + 1 + ) + + const resultTokenLength = resultTokenization.tokens.length; + + resultTokenization.tokens.forEach((t, i) => { + // The space will add TWO tokens. + if(i == resultTokenLength - 3) { + assert.equal(t.appliedTransitionId, REVERTABLE_TRANSITION_ID); + } else { + assert.isUndefined(t.appliedTransitionId); + } + }); + }); + + // Performs the two above in sequence in a manner that could cause cross-effects + // if implemented incorrectly. + it('does not conflate effects between different tokenization transitions', () => { + const baseTokens = ['an', ' ', 'apple', ' ', 'a', ' ', 'day', ' ', 'can']; + const baseTokenization = new ContextTokenization(baseTokens.map(t => toToken(t))); + + const REVERTABLE_TRANSITION_ID = 31415; + baseTokenization.tail.appliedTransitionId = REVERTABLE_TRANSITION_ID; + const NEW_TRANSITION_ID = REVERTABLE_TRANSITION_ID + 1; + + const dist = [ + { + sample: { insert: ' ', deleteLeft: 0, deleteRight: 0, id: NEW_TRANSITION_ID }, + p: .8 + }, { + sample: { insert: ' ', deleteLeft: 0, deleteRight: 0, id: NEW_TRANSITION_ID }, + p: .2 + } + ]; + + const baseTransitionEdge: TransitionEdge = { + alignment: { + merges: [], + splits: [], + unmappedEdits: [], + edgeWindow: { + ...buildEdgeWindow(baseTokenization.tokens, dist[0].sample, false), + retokenization: baseTokenization.tokens.map((t) => t.exampleInput), + retokenizationText: baseTokenization.tokens.map((t) => t.exampleInput).reduce((accum, curr) => accum + curr, '') + }, + removedTokenCount: 0 + }, + inputs: (() => { + const map: Map = new Map(); + map.set(1, dist[0].sample); + + return [ + {sample: map, p: dist[0].p} + ]; + })(), + inputSubsetId: 0 + }; + + // We don't care about the results here. What we care about is that + // this call doesn't remove the appliedTransitionId from the source + // token, preventing it from being marked on later tokenization + // transitions. + baseTokenization.evaluateTransition( + { + alignment: { + ...baseTransitionEdge.alignment, + edgeWindow: { + ...baseTransitionEdge.alignment.edgeWindow, + ...buildEdgeWindow(baseTokenization.tokens, dist[1].sample, false) + } + }, + inputs: (() => { + const map: Map = new Map(); + map.set(0, dist[1].sample); + + return [ + {sample: map, p: dist[1].p} + ]; + })(), + inputSubsetId: 0 + }, + NEW_TRANSITION_ID, + dist[1].p + ) + + const resultTokenization = baseTokenization.evaluateTransition( + baseTransitionEdge, + NEW_TRANSITION_ID, + dist[0].p + ); + + const resultTokenLength = resultTokenization.tokens.length; + + resultTokenization.tokens.forEach((t, i) => { + // The space will add TWO tokens. + if(i == resultTokenLength - 3) { + assert.equal(t.appliedTransitionId, REVERTABLE_TRANSITION_ID); + } else { + assert.isUndefined(t.appliedTransitionId); + } + }); + }); + }); }); describe('buildEdgeWindow', () => {