Merge pull request #14558 from keymanapp/fix/web/post-bksp-applies

fix(web): handle suggestion application after backspacing 🚂

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

@@ -13,28 +13,42 @@ import { ClassicalDistanceCalculation, EditOperation } from "./classical-calcula
 /**
  * Determines the proper 'last match' index for a tokenized sequence based on its edit path.
  *
- * In particular, this method is designed to handle the following case:
- * ['to', 'apple', ' ', ''] => ['to', 'apply', ' ', 'n']
+ * In particular, this method is designed to handle the following cases:
+ * - ['to', ' ', 'apple', ' ', ''] => ['to', ' ', 'apply', ' ', '']
+ * - ['to', ' ', 'apple', ' ', ''] => ['to', ' ', 'apply', ' ', 'n']
  *
- * Edit path for this example case:
- * ['match', 'substitute', 'match', 'substitute']
+ * Edit path for these example cases:
+ * - ['match', 'match', 'substitute', 'match', 'match']
+ * - ['match', 'match', 'substitute', 'match', 'substitute']
  *
- * In cases such as these, the whitespace match should be considered 'edited'. While the ' '
- * is unedited, it follows the edited 'apple' => 'apply', so it must have been deleted and
- * then re-inserted.  As a result, 'to' is the true "last matched" token.
+ * In cases such as these, the late whitespace match should be considered 'edited'. While the
+ * ' ' is unedited, it follows the edited 'apple' => 'apply', so it must have been deleted and
+ * then re-inserted.  As a result, the whitespace after 'to' is the true "last matched" token.
+ *
+ * Returns -1 if an unexpected edit other than 'substitute' occurs in the middle of the big
+ * 'match' block.
  * @param editPath
  * @returns
  */
 export function getEditPathLastMatch(editPath: EditOperation[]) {
-  const editLength = editPath.length;
-  // Special handling: appending whitespace to whitespace with the default wordbreaker.
-  // The default wordbreaker currently adds an empty token after whitespace; this would
-  // show up with 'substitute', 'match' at the end of the edit path.  (This should remain.)
-  if(editLength >= 2 && editPath[editLength - 2] == 'substitute' && editPath[editLength - 1] == 'match') {
-    return editPath.lastIndexOf('match', editLength - 2);
-  } else {
-    return editPath.lastIndexOf('match');
+  // Assertion:  for a long context, the bulk of the edit path should be a
+  // continuous block of 'match' entries.  If there's anything but a substitution
+  // in the middle, we have a context mismatch.
+  //
+  // That said, it is possible to apply a suggestion after a backspace.  Anything
+  // after the substitution needs to be treated as a substitution rather than
+  // a match.
+  const firstMatch = editPath.indexOf('match');
+  const lastMatch = editPath.lastIndexOf('match');
+  if(firstMatch > -1) {
+    for(let i = firstMatch+1; i <= lastMatch; i++) {
+      if(editPath[i] != 'match') {
+        return (editPath[i] == 'substitute') ? (i - 1) : -1;
+      }
+    }
   }
+
+  return lastMatch;
 }
 
 /**

web/src/engine/predictive-text/worker-thread/src/main/correction/context-tokenization.ts

@@ -161,7 +161,7 @@ export class ContextTokenization {
     // From here on assumes that at least one 'match' exists on the path.
     // It all works great... once the context is long enough for at least one stable token.
     const firstMatch = editPath.indexOf('match');
-    const lastMatch = getEditPathLastMatch(editPath);
+
     if(firstMatch == -1) {
       // If there are no matches, there's no alignment.
       return {
@@ -176,6 +176,17 @@ export class ContextTokenization {
       };
     }
 
+    const lastMatch = getEditPathLastMatch(editPath);
+
+    // Assertion:  for a long context, the bulk of the edit path should be a
+    // continuous block of 'match' entries.  If there's anything else in
+    // the middle, we have a context mismatch.
+    if(lastMatch == -1) {
+      return {
+        canAlign: false
+      };
+    }
+
     let matchLength = lastMatch - firstMatch + 1;
     let tailInsertLength = 0;
     let tailDeleteLength = 0;
@@ -195,19 +206,6 @@ export class ContextTokenization {
     }
     const tailSubstituteLength = (editPath.length - 1 - lastMatch) - tailInsertLength - tailDeleteLength;
 
-    // Assertion:  for a long context, the bulk of the edit path should be a
-    // continuous block of 'match' entries.  If there's anything else in
-    // the middle, we have a context mismatch.
-    if(firstMatch > -1) {
-      for(let i = firstMatch+1; i < lastMatch; i++) {
-        if(editPath[i] != 'match') {
-          return {
-            canAlign: false
-          };
-        }
-      }
-    }
-
     // If we have a perfect match with a pre-existing context, no mutations have
     // happened; we have a 100% perfect match.
     if(firstMatch == 0 && lastMatch == editPath.length - 1) {
@@ -414,7 +412,7 @@ export class ContextTokenization {
         token = new ContextToken(matchedToken);
         // Erase any applied-suggestion transition ID; it is no longer valid.
         token.appliedTransitionId = undefined;
-        token.searchSpace.addInput(tokenDistribution.map((seq) => seq[tailIndex]));
+        token.searchSpace.addInput(tokenDistribution.map((seq) => seq[tailIndex] ?? { sample: { insert: '', deleteLeft: 0 }, p: 1 }));
       }
 
       tokenization[incomingIndex] = token;

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

@@ -14,20 +14,32 @@ import { EditOperation, getEditPathLastMatch, isSubstitutionAlignable } from '@k
 describe('getEditPathLastMatch', () => {
   it('returns the last match when no substitutions exist', () => {
     const path: EditOperation[] = ['delete', 'delete', 'match', 'match', 'match', 'match', 'insert'];
-    assert.equal(getEditPathLastMatch(path), path.lastIndexOf('match'));
+    assert.equal(path.lastIndexOf('match'), 5);
+    assert.equal(getEditPathLastMatch(path), 5);
   });
 
   it('returns the last match when no substitutions exist left of a "match"', () => {
     const path: EditOperation[] = ['delete', 'delete', 'match', 'match', 'match', 'match', 'substitute', 'insert'];
-    assert.equal(getEditPathLastMatch(path), path.lastIndexOf('match'));
+    assert.equal(path.lastIndexOf('match'), 5);
+    assert.equal(getEditPathLastMatch(path), 5);
   });
 
   // is intended to handle application of suggestions.
-  it('returns the second-to-last match when a substitution exists before final "match"', () => {
+  it('returns the last match before a substitute occurring after the first match', () => {
     // limitation:  if there is _anything_ after that last match, the first assertion will fail.
-    const path: EditOperation[] = ['delete', 'delete', 'match', 'match', 'match', 'substitute', 'match'];
-    assert.notEqual(getEditPathLastMatch(path), path.lastIndexOf('match'));
-    assert.equal(getEditPathLastMatch(path), path.lastIndexOf('match', path.lastIndexOf('match')-1));
+    //                                0         1         2        3           4          5        6
+    const path: EditOperation[] = ['delete', 'delete', 'match', 'match', 'substitute', 'match', 'match'];
+    assert.notEqual(getEditPathLastMatch(path), 6);
+    assert.equal(getEditPathLastMatch(path), 3);
+  });
+
+  // is intended to handle complex transforms that include a whitespace and affect prior tokens.
+  it('returns the last match before a substitute occurring after the first match', () => {
+    // limitation:  if there is _anything_ after that last match, the first assertion will fail.
+    //                                0         1         2        3           4          5           6
+    const path: EditOperation[] = ['delete', 'delete', 'match', 'match', 'substitute', 'match', 'substitute'];
+    assert.notEqual(getEditPathLastMatch(path), 5);
+    assert.equal(getEditPathLastMatch(path), 3);
   });
 });
 

web/src/test/auto/headless/engine/predictive-text/worker-thread/context/context-tokenization.tests.ts

@@ -491,6 +491,26 @@ describe('ContextTokenization', function() {
 
       assert.deepEqual(computedAlignment, {canAlign: false});
     });
+
+    it("handles late-context suggestion application after backspace", () => {
+      const baseContext = [
+        'quick', ' ', 'brown', ' ', 'fox', ' ', 'jumped', ' ', 'oven', ' ', ''
+      ];
+      const newContext = [
+        'quick', ' ', 'brown', ' ', 'fox', ' ', 'jumped', ' ', 'over', ' ', ''
+      ];
+
+      const baseTokenization = buildBaseTokenization(baseContext);
+      const computedAlignment = baseTokenization.computeAlignment(newContext, false);
+
+      assert.deepEqual(computedAlignment, {
+        canAlign: true,
+        leadTokenShift: 0,
+        matchLength: 8,
+        tailEditLength: 3,
+        tailTokenShift: 0
+      });
+    });
   });
 
   describe('transitionTo', function() {