From 28ade8991d3487ac2ab39a80541b614549905026 Mon Sep 17 00:00:00 2001 From: Joshua Horton Date: Sun, 7 Sep 2025 21:32:08 -0500 Subject: [PATCH] fix(web): better delete-left handing for suggestions when crossing word boundaries --- .../src/main/correction/context-state.ts | 9 +++-- .../worker-thread/src/main/predict-helpers.ts | 14 +++++--- .../worker-model-compositor.tests.ts | 34 +++++++++++++++++++ 3 files changed, 51 insertions(+), 6 deletions(-) diff --git a/web/src/engine/predictive-text/worker-thread/src/main/correction/context-state.ts b/web/src/engine/predictive-text/worker-thread/src/main/correction/context-state.ts index b80c96fa17..f2b67bb149 100644 --- a/web/src/engine/predictive-text/worker-thread/src/main/correction/context-state.ts +++ b/web/src/engine/predictive-text/worker-thread/src/main/correction/context-state.ts @@ -20,7 +20,7 @@ import { ContextTokenization } from './context-tokenization.js'; import { ContextTransition } from './context-transition.js'; import { determineModelTokenizer } from '#./model-helpers.js'; import { tokenizeAndFilterDistribution } from './transform-tokenization.js'; -import { applyTransform, buildMergedTransform } from '@keymanapp/models-templates'; +import { applyTransform } from '@keymanapp/models-templates'; /** * Represents a state of the active context at some point in time along with the @@ -267,7 +267,12 @@ export class ContextState { for(let i of transformKeys) { const primaryInput = transformSequenceDistribution[0].sample.get(i); - preservationTransform = preservationTransform ? buildMergedTransform(preservationTransform, primaryInput): primaryInput; + if(!preservationTransform) { + preservationTransform = primaryInput; + } else { + preservationTransform.insert += primaryInput.insert; + preservationTransform.deleteLeft += primaryInput.deleteLeft; + } } } 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 abae4a9845..36e21a062c 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 @@ -187,7 +187,6 @@ export async function correctAndEnumerate( const postContext = models.applyTransform(inputTransform, context); let rawPredictions: CorrectionPredictionTuple[] = []; - const inputIsBksp = TransformUtils.isBackspace(inputTransform); // If `this.contextTracker` does not exist, we don't have the // `LexiconTraversal` pattern available to us. We're unable to efficiently // iterate through the lexicon as a result, so we use a far lazier pattern - @@ -200,6 +199,7 @@ export async function correctAndEnumerate( // Only allow new-word suggestions if space was the most likely keypress. const allowSpace = TransformUtils.isWhitespace(inputTransform); + const allowBksp = TransformUtils.isBackspace(inputTransform); // Generates raw prediction distributions for each valid input. Can only 'correct' // against the final input. @@ -215,7 +215,7 @@ export async function correctAndEnumerate( // Filter out special keys unless they're expected. if(TransformUtils.isWhitespace(transform) && !allowSpace) { return null; - } else if(TransformUtils.isBackspace(transform) && !inputIsBksp) { + } else if(TransformUtils.isBackspace(transform) && !allowBksp) { return null; } @@ -340,7 +340,7 @@ export async function correctAndEnumerate( const alignment = postContextState.tokenization.alignment; // If the context now has more tokens, the token we'll be 'predicting' didn't originally exist. - if(transition.preservationTransform && !inputIsBksp) { + if(transition.preservationTransform && alignment?.canAlign && alignment.tailTokenShift > 0) { // As the word/token being corrected/predicted didn't originally exist, there's no // part of it to 'replace'. (Suggestions are applied to the pre-transform state.) deleteLeft = 0; @@ -911,7 +911,13 @@ export function finalizeSuggestions( // // Note: may need adjustment if/when supporting phrase-level correction. if(tuple.preservationTransform) { - let mergedTransform = models.buildMergedTransform(tuple.preservationTransform, prediction.sample.transform); + const presDL = tuple.preservationTransform.deleteLeft; + const mergedTransform = models.buildMergedTransform(tuple.preservationTransform, prediction.sample.transform); + // Any preserved delete-left is applied early because it directly affects the suggestion + // root; we need to remove that preserved delete-left here. + if(presDL > 0) { + mergedTransform.deleteLeft -= presDL; + } mergedTransform.id = prediction.sample.transformId; // Temporarily and locally drops 'readonly' semantics so that we can reassign the transform. diff --git a/web/src/test/auto/headless/engine/predictive-text/worker-thread/worker-model-compositor.tests.ts b/web/src/test/auto/headless/engine/predictive-text/worker-thread/worker-model-compositor.tests.ts index d780e84971..1134f5b0a7 100644 --- a/web/src/test/auto/headless/engine/predictive-text/worker-thread/worker-model-compositor.tests.ts +++ b/web/src/test/auto/headless/engine/predictive-text/worker-thread/worker-model-compositor.tests.ts @@ -159,6 +159,40 @@ describe('ModelCompositor', function() { }; let suggestions = await compositor.predict(inputTransform, context); + // Lots of suggestions are rooted on 'the'. We should have more than + // just a single 'keep' suggestion. + assert.isAbove(suggestions.length, 1); + + // Verify the deleteLeft length - gotta make sure we get that right. + suggestions.forEach(function(suggestion) { + // Suggestions always delete the full root of the suggestion. + // + // After a backspace, that means the text 'the' - 3 chars. + // Char 4 is for the original backspace, as suggestions are built + // based on the context state BEFORE the triggering input - + // here, a backspace. + assert.equal(suggestion.transform.deleteLeft, 4); + }); + }); + + it('properly handles complex transforms that change the root token', async function() { + let compositor = new ModelCompositor(plainModel, true); + let context = { + left: 'the ', startOfBuffer: true, endOfBuffer: true, + }; + + let inputTransform = { + insert: 'r', + deleteLeft: 1 + }; + + let suggestions = await compositor.predict(inputTransform, context); + // Lots of suggestions are rooted on 'the'. We should have more than + // just a single 'keep' suggestion. + assert.isAbove(suggestions.length, 1); + assert.isOk(suggestions.find((s => s.displayAs.startsWith("ther")))); + + // Verify the deleteLeft length - gotta make sure we get that right. suggestions.forEach(function(suggestion) { // Suggestions always delete the full root of the suggestion. //