Merge pull request #16450 from keymanapp/fix/web/support-transposition-autocorrect

fix(web): support autocorrection of transposed text

web/src/engine/predictive-text/worker-thread/src/main/correction/distance-modeler.ts

@@ -103,6 +103,12 @@ export interface PartialSearchEdge {
  * more steps until valid search endpoints are reached.
  */
 export class SearchNode {
+  /**
+   * Denotes any additional edit-cost components not modeled by the core edit-distance
+   * computation object.
+   */
+  private addedEditCost = 0;
+
   /**
    * The search-term keying method used by the active LexicalModel
    * @param str
@@ -188,6 +194,7 @@ export class SearchNode {
       // This is unique at each level, though it will reuse a previous ID if no new
       // one is provided (say, for 'insert' edits).
       this.spaceId = spaceId ?? priorNode.spaceId;
+      this.addedEditCost = priorNode.addedEditCost;
     } else {
       this.calculation = new ClassicalDistanceCalculation();
       this.matchedTraversals = [param1];
@@ -206,7 +213,7 @@ export class SearchNode {
    * by the current node.
    */
   get editCount(): number {
-    return this.calculation.getHeuristicFinalCost() + this.deleteAfterInsertEditPairs;
+    return this.calculation.getHeuristicFinalCost() + this.deleteAfterInsertEditPairs + this.addedEditCost;
   }
 
   /**
@@ -271,6 +278,10 @@ export class SearchNode {
     return EDIT_DISTANCE_COST_SCALE * this.editCount + this.inputSamplingCost;
   }
 
+  addEdit() {
+    this.addedEditCost++;
+  }
+
   /**
    * Adds outbound paths from the current Node that model the insertion of a
    * character not seen in the input, as if the user accidentally skipped typing

web/src/engine/predictive-text/worker-thread/src/main/correction/legacy-quotient-spur.ts

@@ -9,9 +9,9 @@
  */
 
 import { LexicalModelTypes } from '@keymanapp/common-types';
-import { KMWString } from 'keyman/common/web-utils';
+import { KMWString, PriorityQueue } from 'keyman/common/web-utils';
 
-import { PathResult } from './correction-searchable.js';
+import { CORRECTION_QUEUE_COMPARATOR, PathResult } from './correction-searchable.js';
 import { SearchNode } from './distance-modeler.js';
 import { SearchQuotientNode, PathInputProperties } from './search-quotient-node.js';
 import { SearchQuotientSpur } from './search-quotient-spur.js';
@@ -24,6 +24,9 @@ import Transform = LexicalModelTypes.Transform;
 // The set of search spaces corresponding to the same 'context' for search.
 // Whenever a wordbreak boundary is crossed, a new instance should be made.
 export class LegacyQuotientSpur extends SearchQuotientSpur {
+  private transposeQueue: PriorityQueue<SearchNode> = new PriorityQueue(CORRECTION_QUEUE_COMPARATOR);
+  private incomingTransposeRootNodes: TokenResultMapping[] = [];
+
   public readonly insertLength: number;
   public readonly leftDeleteLength: number;
 
@@ -44,33 +47,28 @@ export class LegacyQuotientSpur extends SearchQuotientSpur {
     super(space, inputs, inputSource, codepointLength);
     this.insertLength = insertLength;
     this.leftDeleteLength = inputSample.deleteLeft;
-    return;
+
+    // Link to the grandparent node if it exists; transposes start construction rooted there.
+    const grandparentNode = this.parents[0].parents[0];
+    if(grandparentNode) {
+      this.incomingTransposeRootNodes = [...grandparentNode.previousResults];
+      this.linkAndQueueFromParent(grandparentNode, this.incomingTransposeRootNodes);
+    }
   }
 
   construct(parentNode: SearchQuotientNode, inputs?: Distribution<Transform>, inputSource?: PathInputProperties): this {
     return new LegacyQuotientSpur(parentNode, inputs, inputSource) as this;
   }
 
-  protected buildEdgesFromResults(priorResults: ReadonlyArray<TokenResultMapping>): SearchNode[] {
-    // With a newly-available input, we can extend new input-dependent paths from
-    // our previously-reached 'extractedResults' nodes.
-    let outboundNodes = priorResults.map((result) => {
-      // Hard restriction:  no further edits will be supported.  This helps keep the search
-      // more narrowly focused.
-      const substitutionsOnly = result.editCount == 2;
+  protected buildEdgesFromResults(priorResults: ReadonlyArray<TokenResultMapping>, inputs?: Distribution<Transform>): SearchNode[] {
+    return buildEdgesFromResults(priorResults, inputs ?? this.inputs, this.spaceId);
+  }
 
-      let deletionEdges: SearchNode[] = [];
-      if(!substitutionsOnly) {
-        deletionEdges         = result.buildDeletionEdges(this.inputs, this.spaceId);
-      }
-      const substitutionEdges = result.buildSubstitutionEdges(this.inputs, this.spaceId);
+  get currentCost() {
+    const defaultCost = super.currentCost;
+    const transposeCost = this.transposeQueue.peek()?.currentCost ?? Number.POSITIVE_INFINITY;
 
-      // Skip the queue for the first pass; there will ALWAYS be at least one pass,
-      // and queue-enqueing does come with a cost - avoid unnecessary overhead here.
-      return substitutionEdges.flatMap(e => e.processSubsetEdge()).concat(deletionEdges);
-    }).flat();
-
-    return outboundNodes;
+    return Math.min(transposeCost, defaultCost);
   }
 
   /**
@@ -80,6 +78,52 @@ export class LegacyQuotientSpur extends SearchQuotientSpur {
    * @returns
    */
   public handleNextNode(): PathResult<TokenResultMapping> {
+    this.processPendingRoots();
+    const transposeCost = this.transposeQueue.peek()?.currentCost ?? Number.POSITIVE_INFINITY;
+
+    // Handle transposition cases
+    if(transposeCost < super.currentCost) {
+      let currentNode = this.transposeQueue.dequeue();
+
+      let unmatchedResult: PathResult<TokenResultMapping> = {
+        type: 'intermediate',
+        cost: currentNode.currentCost
+      }
+
+      // Stage 1:  filter out nodes/edges we want to prune
+
+      // Forbid a raw edit-distance of greater than 2.
+      // Note:  .knownCost is not scaled, while its contribution to .currentCost _is_ scaled.
+      if(currentNode.editCount > 2) {
+        return unmatchedResult;
+      }
+
+      // Stage 2:  process subset further OR build remaining edges
+
+      if(currentNode.hasPartialInput) {
+        // Re-use the current queue; the number of total inputs considered still
+        // holds.
+        const nextProcessingStepNodes = currentNode
+          .processSubsetEdge()
+          .filter(e => e.editCount == currentNode.editCount);
+        this.transposeQueue.enqueueAll(nextProcessingStepNodes);
+        return unmatchedResult;
+      }
+
+      // If here, we've properly done the first half of a transpose.  Now for
+      // the other half...
+      const transposeSecondHalfNodes = this.buildEdgesFromResults(
+        [new TokenResultMapping(this, currentNode)],
+        (this.parents[0] as LegacyQuotientSpur).inputs
+      ).filter(
+        // Ignore all paths that charge a new insert/delete edit while modeling
+        // a transposition edit.
+        e => e.editCount == currentNode.editCount
+      );
+      this.queueNodes(transposeSecondHalfNodes);
+      return unmatchedResult;
+    }
+
     const result = super.handleNextNode();
 
     if(result.type == 'complete') {
@@ -95,4 +139,55 @@ export class LegacyQuotientSpur extends SearchQuotientSpur {
 
     return result;
   }
+
+  protected processPendingRoots(): void {
+    super.processPendingRoots();
+
+    if(this.incomingTransposeRootNodes.length > 0) {
+      const transpositionFirstHalves = processTransposeRoots(this.incomingTransposeRootNodes, this.inputs, this.spaceId);
+
+      this.incomingTransposeRootNodes.splice(0, this.incomingTransposeRootNodes.length);
+      this.transposeQueue.enqueueAll(transpositionFirstHalves);
+    }
+  }
+}
+
+export function processTransposeRoots(priorResults: TokenResultMapping[], inputs: Distribution<Transform>, spaceId: number) {
+  // Build only substitution edges from these.
+  const transpositionFirstHalves = priorResults.flatMap((entry) => {
+    const entryFirstHalves = entry.buildSubstitutionEdges(inputs, spaceId)
+      // Perform the first step of processing the edge...
+      .flatMap(e => e.processSubsetEdge())
+      // and eliminate any that cost a non-transpose edit.  We don't blend
+      // transpose edits with insert or delete edits.
+      .filter(e => e.editCount == entry.editCount);
+
+    // Now, add the unit of edit cost charged for modeling a transposition.
+    entryFirstHalves.forEach(e => e.addEdit());
+    return entryFirstHalves;
+  });
+
+  return transpositionFirstHalves;
+}
+
+export function buildEdgesFromResults(priorResults: ReadonlyArray<TokenResultMapping>, inputs: Distribution<Transform>, spaceId: number): SearchNode[] {
+  // With a newly-available input, we can extend new input-dependent paths from
+  // our previously-reached 'extractedResults' nodes.
+  let outboundNodes = priorResults.map((result) => {
+    // Hard restriction:  no further edits will be supported.  This helps keep the search
+    // more narrowly focused.
+    const substitutionsOnly = result.editCount == 2;
+
+    let deletionEdges: SearchNode[] = [];
+    if(!substitutionsOnly) {
+      deletionEdges         = result.buildDeletionEdges(inputs, spaceId);
+    }
+    const substitutionEdges = result.buildSubstitutionEdges(inputs, spaceId);
+
+    // Skip the queue for the first pass; there will ALWAYS be at least one pass,
+    // and queue-enqueing does come with a cost - avoid unnecessary overhead here.
+    return substitutionEdges.flatMap(e => e.processSubsetEdge()).concat(deletionEdges);
+  }).flat();
+
+  return outboundNodes;
 }

web/src/engine/predictive-text/worker-thread/src/main/predict-helpers.ts

@@ -21,7 +21,7 @@ import { ContextState, determineContextSlideTransform } from './correction/conte
 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';
+import { EDIT_DISTANCE_COST_SCALE, getBestTokenMatches } from './correction/distance-modeler.js';
 
 import CasingForm = LexicalModelTypes.CasingForm;
 import Context = LexicalModelTypes.Context;
@@ -78,7 +78,12 @@ export const CORRECTION_SEARCH_THRESHOLDS = {
    * in log-space, the search would stop at a total cost of 1 + this value if
    * a "full" set of suggestions had already been found.
    */
-  REPLACEMENT_SEARCH_THRESHOLD: 4 as const // e^-4 = 0.0183156388.  Allows "80%" of an extra edit.
+
+  // Ensure at least one "edit distance cost unit" so that even heavily
+  // fat-fingered transpositions have a chance.  Note that the level is this
+  // applied, wordlist weightings have no effect and cannot prevent correction
+  // thresholding!
+  REPLACEMENT_SEARCH_THRESHOLD: EDIT_DISTANCE_COST_SCALE * 1.1
 }
 
 /**
@@ -662,7 +667,17 @@ export async function correctAndEnumerate(
       continue;
     }
 
-    if(match.editCount > 0 && !searchModules.find(s => s.correctionsEnabled)) {
+    // In the case of a backspace, we wipe out the original form of the search
+    // module and replace it with a format that also signals that corrections
+    // aren't enabled.
+    //
+    // To resolve this, we check the pre-transition form in order to check if
+    // corrections were enabled before a backspace.
+    const correctionsWereEnabled = transition.base.displayTokenization.tail.searchModule.correctionsEnabled;
+    if(match.editCount > 0
+      && !searchModules.find(s => s.correctionsEnabled)
+      && !(TransformUtils.isBackspace(inputTransform) && correctionsWereEnabled)
+    ) {
       continue;
     }
 
@@ -1078,19 +1093,6 @@ export function predictionAutoSelect(suggestionDistribution: CorrectionPredictio
     return;
   }
 
-  // Find the highest probability for any correction that led to a valid prediction.
-  // No need to full-on re-sort everything, though.
-  const bestCorrection = suggestionDistribution.reduce(
-    (prev, current) => prev?.correction.p > current.correction.p ? prev : current,
-    null
-  ).correction;
-  if(bestCorrection.p > bestSuggestion.correction.p) {
-    // Here, the best suggestion didn't come from the best correction.
-    // Is it actually reasonable to auto-correct?  We're probably just very
-    // biased toward its frequency.  (Maybe a threshold should be considered?)
-    return;
-  }
-
   // If we allow an option to allow same-key suggestions to replace context automatically
   // - such as replacing `cant` with `can't` if the latter is much more frequent -
   // we may wish to group matchLevel values below by 'mapping' them with an appropriate

web/src/test/auto/headless/engine/predictive-text/worker-thread/correction-search/early-correction-search-stopping.tests.ts

@@ -38,7 +38,11 @@ describe('correction-search: shouldStopSearchingEarly', () => {
   });
 
   it('stops checking corrections earlier when enough predictions have been found', () => {
-    const predictionProbs = [.010, .009, .008, .008, .0075, .0075, .007, .007, .006, .006, .005, .005];
+    // Thresholding is performed in log-space.
+    const baseCost = 1;
+    const expectedThreshold = CORRECTION_SEARCH_THRESHOLDS.REPLACEMENT_SEARCH_THRESHOLD;
+
+    const predictionProbs = [.010, .009, .008, .008, .0075, .007, .006, .005, .004, .003, .002, Math.exp(- baseCost - expectedThreshold)];
     assert.isAtLeast(predictionProbs.length, ModelCompositor.MAX_SUGGESTIONS, "test setup no longer valid");
 
     // The only part for each entry we actually care about here:  .totalProb.
@@ -49,12 +53,8 @@ describe('correction-search: shouldStopSearchingEarly', () => {
       } as CorrectionPredictionTupleCore
     });
 
-    const baseCost = 1;
-
-    // Thresholding is performed in log-space.
-    const expectedThreshold = CORRECTION_SEARCH_THRESHOLDS.REPLACEMENT_SEARCH_THRESHOLD;
-
+    // The actual assertions.
     assert.isFalse(shouldStopSearchingEarly(baseCost, baseCost + expectedThreshold - 0.01, predictions));
-    assert.isTrue(shouldStopSearchingEarly( baseCost, baseCost + expectedThreshold + 0.01, predictions));
+    assert.isTrue(shouldStopSearchingEarly(baseCost, baseCost + expectedThreshold + 0.01, predictions));
   });
 });

web/src/test/auto/headless/engine/predictive-text/worker-thread/correction-search/getBestMatches.tests.ts

@@ -76,14 +76,16 @@ describe('Correction Searching', () => {
         // 't' -> 'b' (sub)
         'beh',
         // '' -> 'c' (insertion)
-        'tech'
+        'tech',
+        // 'eh' -> 'he' (transposition)
+        'the'
       ];
 
       await checkBatch(thirdBatch, secondCost);
 
       // All replace the low-likelihood case for the third input.
       const fourthBatch = [
-        'the', 'thi', 'tho', 'thr',
+        'thi', 'tho', 'thr',
         'thu', 'tha'
       ];
 

web/src/test/auto/headless/engine/predictive-text/worker-thread/correction-search/legacy-quotient-spur.tests.ts

@@ -9,17 +9,24 @@
 
 import { assert } from 'chai';
 
+import { LexicalModelTypes } from '@keymanapp/common-types';
 import { jsonFixture } from '@keymanapp/common-test-resources/model-helpers.mjs';
 import {
+  buildEdgesFromResults,
+  CORRECTION_SEARCH_THRESHOLDS,
   generateSubsetId,
   LegacyQuotientRoot,
   LegacyQuotientSpur,
-  models
+  models,
+  processTransposeRoots,
+  TokenResultMapping
 } from '@keymanapp/lm-worker/test-index';
 
 import { buildCantLinearFixture } from '../../helpers/buildCantLinearFixture.js';
 import { analyzeQuotientNodeResults } from '../../helpers/analyzeQuotientNodeResults.js';
 
+import Distribution = LexicalModelTypes.Distribution;
+import Transform = LexicalModelTypes.Transform;
 import TrieModel = models.TrieModel;
 
 const testModel = new TrieModel(jsonFixture('models/tries/english-1000'));
@@ -322,4 +329,116 @@ describe('LegacyQuotientSpur', () => {
       assert.isEmpty(analysis.foundWithDuplicates);
     });
   });
+
+  describe('transposition handling', () => {
+    const tehDistributions: Distribution<Transform>[] = [
+      [
+        { sample: { insert: 't', deleteLeft: 0, id: 1 }, p: .55},
+        { sample: { insert: 'r', deleteLeft: 0, id: 1 }, p: .45}
+      ], [
+        { sample: { insert: 'e', deleteLeft: 0, id: 1 }, p: .9},
+        { sample: { insert: 's', deleteLeft: 0, id: 1 }, p: .1}
+      ], [
+        { sample: { insert: 'h', deleteLeft: 0, id: 1 }, p: .9},
+        { sample: { insert: 'n', deleteLeft: 0, id: 1 }, p: .1}
+      ]
+    ];
+
+    it('corrects to `the` for a targeted, deep search for a `teh` transposition', () => {
+      const root = new LegacyQuotientRoot(testModel);
+
+      const rootResults: TokenResultMapping[] = [];
+      while(root.currentCost < Number.POSITIVE_INFINITY) {
+        const result = root.handleNextNode();
+        if(result.type == 'complete') {
+          rootResults.push(result.mapping);
+        }
+      }
+
+      const entry_empty = rootResults.find((entry) => entry.matchString == '')
+      assert.isOk(entry_empty);
+
+      const firstSpur = new LegacyQuotientSpur(root, tehDistributions[0], tehDistributions[0][0]);
+      const edgesFromEmpty = buildEdgesFromResults([entry_empty], firstSpur.inputs, firstSpur.spaceId);
+      const edge_t = edgesFromEmpty.find((entry) => entry.resultKey == 't');
+      assert.isOk(edge_t);
+
+      // now, try to do something with entry_t.
+      const secondSpur = new LegacyQuotientSpur(firstSpur,  tehDistributions[1], tehDistributions[1][0]);
+      const thirdSpur  = new LegacyQuotientSpur(secondSpur, tehDistributions[2], tehDistributions[2][0]);
+
+      const entry_t = new TokenResultMapping(firstSpur, edge_t);
+      const transposeFirstHalves = processTransposeRoots([entry_t], thirdSpur.inputs, thirdSpur.spaceId);
+
+      const edge_th = transposeFirstHalves.find((entry) => entry.resultKey == 'th' && entry.editCount == 1);
+      assert.isOk(edge_th);
+
+      const entry_th = new TokenResultMapping(thirdSpur, edge_th);
+      const transposeSecondHalves = buildEdgesFromResults([entry_th], secondSpur.inputs, thirdSpur.spaceId);
+      const edge_the = transposeSecondHalves.find((entry) => entry.resultKey == 'the' && entry.editCount == 1);
+      assert.isOk(edge_the);
+    });
+
+    it('corrects to `the` for an sequential, broad search for a `teh` transposition', () => {
+      const root = new LegacyQuotientRoot(testModel);
+
+      const rootResults: TokenResultMapping[] = [];
+      while(root.currentCost < Number.POSITIVE_INFINITY) {
+        const result = root.handleNextNode();
+        if(result.type == 'complete') {
+          rootResults.push(result.mapping);
+        }
+      }
+
+      const entry_empty = rootResults.find((entry) => entry.matchString == '')
+      assert.isOk(entry_empty);
+
+      const firstSpur = new LegacyQuotientSpur(root, tehDistributions[0], tehDistributions[0][0]);
+      const firstResults: TokenResultMapping[] = [];
+      while(firstSpur.currentCost < Number.POSITIVE_INFINITY) {
+        const result = firstSpur.handleNextNode();
+        if(result.type == 'complete') {
+          firstResults.push(result.mapping);
+        }
+      }
+
+      const entry_t = firstResults.find((entry) => entry.matchString == 't' && entry.editCount == 0);
+      assert.isOk(entry_t);
+
+      // now, try to do something with entry_t.
+      const secondSpur = new LegacyQuotientSpur(firstSpur,  tehDistributions[1], tehDistributions[1][0]);
+      const secondResults: TokenResultMapping[] = [];
+      while(secondSpur.currentCost < Number.POSITIVE_INFINITY) {
+        const result = secondSpur.handleNextNode();
+        if(result.type == 'complete') {
+          secondResults.push(result.mapping);
+        }
+      }
+
+      const thirdSpur  = new LegacyQuotientSpur(secondSpur, tehDistributions[2], tehDistributions[2][0]);
+      const thirdResults: TokenResultMapping[] = [];
+      while(thirdSpur.currentCost < Number.POSITIVE_INFINITY) {
+        const result = thirdSpur.handleNextNode();
+        if(result.type == 'complete') {
+          thirdResults.push(result.mapping);
+        }
+      }
+
+      const entry_the = thirdResults.find((entry) => entry.matchString == 'the' && entry.editCount == 1);
+      assert.isOk(entry_the);
+
+      thirdResults.sort((a, b) => a.totalCost - b.totalCost);
+      const the_index = thirdResults.findIndex((entry) => entry.matchString == 'the' && entry.editCount == 1);
+      // `teh` should appear fairly early as a viable correction.
+      assert.isBelow(the_index, 10);
+
+      // This test portion is a bit "white box" - it should verify that a
+      // specific conditional within `shouldStopSearchingEarly` returns true.
+      //
+      // We want to make sure we don't auto-ignore transposition cases by
+      // accident by failing that conditional.
+      const the_entry = thirdResults[the_index];
+      assert.isBelow(the_entry.totalCost - thirdResults[0].totalCost, CORRECTION_SEARCH_THRESHOLDS.REPLACEMENT_SEARCH_THRESHOLD);
+    });
+  });
 });

web/src/test/auto/headless/engine/predictive-text/worker-thread/prediction-helpers/auto-correct.tests.ts

@@ -564,85 +564,6 @@ describe('predictionAutoSelect', () => {
     assert.equal(autoselected, expectedSuggestion);
   });
 
-  // The idea:  avoid "over-correcting" when a potential correction has a
-  // super-high-frequency word.
-  it('does not auto-select suggestion if its root correction is not most likely', () => {
-    const keepSuggestion: CorrectionPredictionTuple= {
-      correction: {
-        sample: 'thi',
-        p: .7
-      },
-      prediction: {
-        sample: {
-          tag: 'keep',
-          transform: {  // can be null / "mocked out"
-            insert: 'i',
-            deleteLeft: 0
-          },
-          displayAs: '"thi"',
-          matchesModel: false
-        },
-        p: .05
-      },
-      totalProb: .035,
-      metadata: {...defaultMetadata}
-    }
-
-    const highestCorrectionSuggestion: CorrectionPredictionTuple= {
-      correction: {
-        sample: 'thi',
-        p: .7
-      },
-      prediction: {
-        sample: {
-          transform: {  // can be null / "mocked out"
-            insert: 'in',
-            deleteLeft: 0
-          },
-          displayAs: 'thin'
-        },
-        p: .1
-      },
-      totalProb: .07,
-      metadata: {...defaultMetadata}
-    };
-
-    const highestNonKeepSuggestion: CorrectionPredictionTuple= {
-      correction: {
-        sample: 'the',
-        p: .3
-      },
-      prediction: {
-        sample: {
-          transform: {  // can be null / "mocked out"
-            insert: 'e',
-            deleteLeft: 0
-          },
-          displayAs: 'the'
-        },
-        p: 1
-      },
-      totalProb: .3,
-      metadata: {...defaultMetadata}
-    };
-
-    const predictions: CorrectionPredictionTuple[] = [
-      keepSuggestion,
-      highestNonKeepSuggestion,
-      highestCorrectionSuggestion
-    ];
-
-    const totalProb = predictions.reduce((accum, current) => accum + current.totalProb, 0);
-    assert.isAbove(highestNonKeepSuggestion.totalProb, totalProb * AUTOSELECT_PROPORTION_THRESHOLD, 'test setup is no longer valid');
-
-    const originalPredictions = [].concat(predictions);
-    assert.doesNotThrow(() => predictionAutoSelect(predictions));
-    assert.sameDeepMembers(predictions, originalPredictions);
-
-    const autoselected = predictions.find((entry) => entry.prediction.sample.autoAccept);
-    assert.isNotOk(autoselected);
-  });
-
   // // If we add a setting allowing 'exact', 'sameText', and 'sameKey' tiers to
   // // all compete equally, rather than having each instantly win over those
   // // after it, we'd want to add a test such as this.