Merge pull request #3647 from keymanapp/feat/common/models/model-dependent-revert-annotation

feat(common/models): 'revert' now uses model's punctuation

common/core/web/input-processor/src/text/prediction/languageProcessor.ts

@@ -186,7 +186,7 @@ namespace com.keyman.text.prediction {
       return this.predict_internal(transcription);
     }
 
-    public applySuggestion(suggestion: Suggestion, outputTarget: OutputTarget): Promise<Suggestion> {
+    public applySuggestion(suggestion: Suggestion, outputTarget: OutputTarget): Promise<Reversion> {
       if(!outputTarget) {
         throw "Accepting suggestions requires a destination OutputTarget instance."
       }
@@ -220,25 +220,34 @@ namespace com.keyman.text.prediction {
         let reversionTranscription = preApply.buildTranscriptionFrom(outputTarget, null);
         this.recordTranscription(reversionTranscription);
 
-        // TODO:  Also tell lm-layer we accepted the suggestion
-        // The 'reversion promise' should be returned from the lm-layer, not constructed here.
-        // (Needed for proper displayAs syntax!)
-        let reversionPromise: Promise<Suggestion> = new Promise<Suggestion>(function(resolveFunc, rejectFunc) {
-          wordbreakPromise.then(function(token) {
-            let suggestion: Suggestion = {
-              // TODO:  This needs to be sourced from the lm-layer side, from the model's
-              // punctuation spec.  Hence the 'outer' TODO.
-              displayAs: '"' + token + '"',
-              transform: reversionTranscription.transform,
-              transformId: reversionTranscription.token
-            }
+        // Builds the reversion option according to the loaded lexical model's known
+        // syntactic properties.
+        let suggestionContext = new TranscriptionContext(original.preInput, this.configuration);
 
-            resolveFunc(suggestion);
-          });
-        });
+        // We must accept the Suggestion from its original context, which was before
+        // `original.transform` was applied.
+        let reversionPromise: Promise<Reversion> = this.lmEngine.acceptSuggestion(suggestion, suggestionContext, original.transform);
 
         // Also, request new prediction set based on the resulting context.
-        this.predictFromTarget(outputTarget);
+        let lp = this;
+        reversionPromise = reversionPromise.then(function(reversion) {
+          let mappedReversion: Reversion = {
+            // The lm-layer's generated transform deletes more than is necessary,
+            // even if it re-inserts it afterward.  This may cause loss of 
+            // restorable deadkeys, so it's best to use Web's pre-calculated
+            // version above.
+            transform: reversionTranscription.transform,
+            // The ID part is critical; the reversion can't be applied without it.
+            transformId: reversionTranscription.token,
+            displayAs: reversion.displayAs,
+            tag: reversion.tag
+          }
+          // // If using the version from lm-layer:
+          // let mappedReversion = reversion;
+          // mappedReversion.transformId = reversionTranscription.token;
+          lp.predictFromTarget(outputTarget);
+          return mappedReversion;
+        });
 
         return reversionPromise;
       }

common/models/templates/src/common.ts

@@ -24,16 +24,33 @@ namespace models {
   }
 
   /**
-   *
-   * @param transform Merges one transform into another, mutating the first parameter to
-   *                  include the effects of the second.
-   * @param prefix
+   * Merges two Transforms as if they were applied to a `Context` successively.
+   * @param first 
+   * @param second 
    */
-  export function prependTransform(transform: Transform, prefix: Transform) {
-    transform.insert = prefix.insert + transform.insert;
-    transform.deleteLeft += prefix.deleteLeft;
-    if(prefix.deleteRight) {
-      transform.deleteRight = (transform.deleteRight || 0) + prefix.deleteRight;
+  export function buildMergedTransform(first: Transform, second: Transform): Transform {
+    // These exist to avoid parameter mutation.
+    let mergedFirstInsert: string = first.insert;
+    let mergedSecondDelete: number = second.deleteLeft;
+
+    // The 'fun' case:  the second Transform wants to delete something from the first.
+    if(second.deleteLeft) {
+      let firstLength = first.insert.kmwLength();
+      if(firstLength <= second.deleteLeft) {
+        mergedFirstInsert = '';
+        mergedSecondDelete = second.deleteLeft - firstLength;
+      } else {
+        mergedFirstInsert = first.insert.kmwSubstr(0, firstLength - second.deleteLeft);
+        mergedSecondDelete = 0;
+      }
+    }
+
+    return {
+      insert: mergedFirstInsert + second.insert,
+      deleteLeft: first.deleteLeft + mergedSecondDelete,
+      // As `first` would affect the context before `second` could take effect,
+      // this is the correct way to merge `deleteRight`.
+      deleteRight: (first.deleteRight || 0) + (second.deleteRight || 0)
     }
   }
 

common/models/templates/test/test-common.js

@@ -8,6 +8,185 @@ var models = require('../').models;
 describe('Common utility functions', function() {
   // TODO:  unit tests for other common utility functions
 
+  describe('buildMergedTransform', function() {
+    it("simple case:  no deletions", function() {
+      let apple = {
+        insert: 'apple',
+        deleteLeft: 0
+      };
+
+      let banana = {
+        insert: 'banana',
+        deleteLeft: 0
+      };
+
+      let final = {
+        insert: 'applebanana',
+        deleteLeft: 0,
+        deleteRight: 0
+      };
+
+      let mergedTransform = models.buildMergedTransform(apple, banana);
+      assert.deepEqual(mergedTransform, final);
+    });
+
+    it("first.deleteLeft > 0, second.deleteLeft = 0", function() {
+      let apple = {
+        insert: 'apple',
+        deleteLeft: 2
+      };
+
+      let banana = {
+        insert: 'banana',
+        deleteLeft: 0
+      };
+
+      let final = {
+        insert: 'applebanana',
+        deleteLeft: 2,
+        deleteRight: 0
+      };
+
+      let mergedTransform = models.buildMergedTransform(apple, banana);
+      assert.deepEqual(mergedTransform, final);
+    });
+
+    it("first.deleteLeft = 0, second.deleteLeft > 0", function() {
+      let banana = {
+        insert: 'banana',
+        deleteLeft: 0
+      };
+
+      let apple = {
+        insert: 'apple',
+        deleteLeft: 1
+      };
+
+      let final = {
+        insert: 'bananapple',  // the 'apple' transform removes the final 'a' from 'banana'.
+        deleteLeft: 0,
+        deleteRight: 0
+      };
+
+      let mergedTransform = models.buildMergedTransform(banana, apple);
+      assert.deepEqual(mergedTransform, final);
+    });
+
+    it("first.deleteLeft > 0, second.deleteLeft > 0", function() {
+      let banana = {
+        insert: 'banana',
+        deleteLeft: 2
+      };
+
+      let apple = {
+        insert: 'apple',
+        deleteLeft: 1
+      };
+
+      let final = {
+        insert: 'bananapple',  // the 'apple' transform removes the final 'a' from 'banana'.
+        deleteLeft: 2,
+        deleteRight: 0
+      };
+
+      let mergedTransform = models.buildMergedTransform(banana, apple);
+      assert.deepEqual(mergedTransform, final);
+    });
+
+    it("first.deleteRight > 0, second.deleteRight = 0 (implied)", function() {
+      let apple = {
+        insert: 'apple',
+        deleteLeft: 0,
+        deleteRight: 2 
+      };
+
+      let banana = {
+        insert: 'banana',
+        deleteLeft: 0
+      };
+
+      let final = {
+        insert: 'applebanana',  // the 'apple' transform does NOT remove the front 'ba' from 'banana'.
+                                // 'banana' is considered 'later' in time, after application of 'apple'
+        deleteLeft: 0,
+        deleteRight: 2
+      };
+
+      let mergedTransform = models.buildMergedTransform(apple, banana);
+      assert.deepEqual(mergedTransform, final);
+    });
+
+    it("first.deleteRight = 0, second.deleteRight > 0", function() {
+      let apple = {
+        insert: 'apple',
+        deleteLeft: 0
+      };
+
+      let banana = {
+        insert: 'banana',
+        deleteLeft: 0,
+        deleteRight: 3
+      };
+
+      let final = {
+        insert: 'applebanana',  // the 'apple' transform does NOT remove the front 'ba' from 'banana'.
+                                // 'banana' is considered 'later' in time, after application of 'apple'
+        deleteLeft: 0,
+        deleteRight: 3
+      };
+
+      let mergedTransform = models.buildMergedTransform(apple, banana);
+      assert.deepEqual(mergedTransform, final);
+    });
+
+    it("first.deleteRight > 0, second.deleteRight > 0", function() {
+      let apple = {
+        insert: 'apple',
+        deleteLeft: 0,
+        deleteRight: 2 
+      };
+
+      let banana = {
+        insert: 'banana',
+        deleteLeft: 0,
+        deleteRight: 2
+      };
+
+      let final = {
+        insert: 'applebanana',  // the 'apple' transform does NOT remove the front 'ba' from 'banana'.
+                                // 'banana' is considered 'later' in time, after application of 'apple'
+        deleteLeft: 0,
+        deleteRight: 4
+      };
+
+      let mergedTransform = models.buildMergedTransform(apple, banana);
+      assert.deepEqual(mergedTransform, final);
+    });
+
+    it("complex case:  first.deleteRight > 0, second.deleteLeft > 0", function() {
+      let apple = {
+        insert: 'apple',
+        deleteLeft: 0,
+        deleteRight: 2 
+      };
+
+      let banana = {
+        insert: 'banana',
+        deleteLeft: 2,
+        deleteRight: 0
+      };
+
+      let final = {
+        insert: 'appbanana',
+        deleteLeft: 0,
+        deleteRight: 2
+      };
+
+      let mergedTransform = models.buildMergedTransform(apple, banana);
+      assert.deepEqual(mergedTransform, final);
+    });
+  });
+
   describe('transformToSuggestion', function() {
     it('p: undefined', function() {
       let suggestion = {

common/models/types/index.d.ts

@@ -238,6 +238,10 @@ declare interface Suggestion {
   tag?: SuggestionTag;
 }
 
+interface Reversion extends Suggestion {
+  tag: 'revert';
+}
+
 /**
  * A tag indicating the nature of the current suggestion.
  * 
@@ -251,8 +255,7 @@ declare interface Suggestion {
  *  
  * If left undefined, the consumers will assume this is a prediction.
  */
-type SuggestionTag = undefined | 'keep' | 'correction' | 'emoji';
-
+type SuggestionTag = undefined | 'keep' | 'revert' | 'correction' | 'emoji';
 
 /**
  * The text and environment surrounding the insertion point (text cursor).

common/predictive-text/docs/worker-communication-protocol.md

@@ -125,6 +125,10 @@ Message       | Direction          | Parameters          | Expected reply      |
 `suggestions` | LMLayer → keyboard | suggestions         | No                  | Yes
 `wordbreak`   | LMLayer → worker   | context             | Yes - `currentword` | Yes
 `currentword` | LMLayer → keyboard | string              | No                  | Yes
+`accept`      | keyboard → LMLayer | suggestion,         | Yes - `postaccept`  | Yes
+                                   | context, transform  |                     |
+`postaccept`  | LMLayer → keyboard | reversion           | No                  | Yes
+              
 
 ### Message: `config`
 
@@ -459,6 +463,53 @@ point.  If no such wordform exists, this returns an empty string.
 This message **MUST** be in response to a `wordbreak` message, and it
 **MUST** respond with the corresponding [token][Tokens].
 
+### Message: `accept`
+
+The `accept` message is sent from the keyboard to the LMLayer.  This
+message tells the LMLayer that a previously-returned `suggestion` 
+(from a `predict`-`suggestions` pair) has been accepted by the user.  
+The LMLayer **SHOULD** respond to each `accept` message with a 
+`postaccept` message providing a `reversion` capable of undoing it.
+
+The keyboard **MUST** send the `suggestion` and `context` parameters.
+The keyboard **MUST** send a unique token.  The `postTransform` parameter is 
+optional, but highly suggested.
+
+The semantics of the `accept` message **MUST** be from the perspective
+of this sequence of events:
+
+1. A user has just selected the `suggestion` as valid.
+2. The keystroke triggering the `suggestion` has NOT been committed to 
+   the `context`.  Its Transform data is sent as the `postTransform`
+   parameter.  (The "post" nomenclature component signifies that 
+   `postTransform` comes temporally "after" this context state.)
+3. Before the user inputs any additional keystrokes, which would trigger
+   new suggestions.
+
+The `postTransform` parameter allows the base keystroke, which has **NOT**
+been applied to the provided `context`, to be restored by the `reversion` 
+that undoes acceptance of the `suggestion`.  
+
+For reference, compare this to the ["Message: `predict`" section](#message-predict)
+For Suggestions returned by a `predict`->`suggestions` message sequence:
+
+* `predict`'s `context` there should match `context` here.
+* `predict`'s `transform` there should match `postTransform` here.
+    - In the case that a distribution of Transforms was specified, rather than 
+    just one, only the 'base' keystroke's Transform should be used.
+
+These serve as a snapshot in time of the state in which the Suggestion was
+generated.
+
+### Message: `postaccept`
+
+The `postaccept` message is sent from the keyboard to the LMLayer.  This
+message sends a `reversion` capable of undoing acceptance of the `suggestion`
+just accepted by the triggering `accept` message.
+
+This message **MUST** be in response to a `postaccept` message, and it
+**MUST** respond with the corresponding [token][Tokens].
+
 #### Examples
 
 ```javascript

common/predictive-text/index.ts

@@ -53,6 +53,7 @@ namespace com.keyman.text.prediction {
     private _declareLMLayerReady: (conf: Configuration) => void;
     private _predictPromises: PromiseStore<Suggestion[]>;
     private _wordbreakPromises: PromiseStore<USVString>;
+    private _acceptPromises: PromiseStore<Reversion>;
     private _nextToken: number;
     private capabilities: Capabilities;
 
@@ -67,8 +68,9 @@ namespace com.keyman.text.prediction {
       this._worker = worker || DefaultWorker.constructInstance();
       this._worker.onmessage = this.onMessage.bind(this)
       this._declareLMLayerReady = null;
-      this._predictPromises = new PromiseStore;
-      this._wordbreakPromises = new PromiseStore<USVString>();
+      this._predictPromises = new PromiseStore();
+      this._wordbreakPromises = new PromiseStore();
+      this._acceptPromises = new PromiseStore();
       this._nextToken = Number.MIN_SAFE_INTEGER;
 
       this.sendConfig(capabilities);
@@ -138,6 +140,20 @@ namespace com.keyman.text.prediction {
       });
     }
 
+    acceptSuggestion(suggestion: Suggestion, context: Context, postTransform: Transform): Promise<Reversion> {
+      let token = this._nextToken++;
+      return new Promise((resolve, reject) => {
+        this._acceptPromises.make(token, resolve, reject);
+        this._worker.postMessage({
+          message: 'accept',
+          token: token,
+          suggestion: suggestion,
+          context: context,
+          postTransform: postTransform
+        });
+      });
+    }
+
     // TODO: asynchronous close() method.
     //       Worker code must recognize message and call self.close().
 
@@ -155,6 +171,8 @@ namespace com.keyman.text.prediction {
         this._predictPromises.keep(payload.token, payload.suggestions);
       } else if (payload.message === 'currentword') {
         this._wordbreakPromises.keep(payload.token, payload.word);
+      } else if (payload.message === 'postaccept') {
+        this._acceptPromises.keep(payload.token, payload.reversion);
       } else {
         // This branch should never execute, but just in case...
         //@ts-ignore

common/predictive-text/message.d.ts

@@ -30,8 +30,8 @@ type Token = number;
 /**
  * The valid outgoing message kinds.
  */
-type OutgoingMessageKind = 'error' | 'ready' | 'suggestions' | 'currentword';
-type OutgoingMessage = ErrorMessage | ReadyMessage | SuggestionMessage | CurrentWordMessage;
+type OutgoingMessageKind = 'error' | 'ready' | 'suggestions' | 'currentword' | 'postaccept';
+type OutgoingMessage = ErrorMessage | ReadyMessage | SuggestionMessage | CurrentWordMessage | PostAcceptMessage;
 
 interface ErrorMessage {
   message: 'error';
@@ -86,6 +86,25 @@ interface CurrentWordMessage {
   word: USVString;
 }
 
+/**
+ * Returns the results of a 'wordbreak' request:  a 'reversion' Suggestion and an
+ * array of new, word-initial Suggestions.
+ */
+interface PostAcceptMessage {
+  message: 'postaccept';
+
+  /**
+   * Opaque, unique token that pairs this message
+   * with the wordbreak message that initiated it.
+   */
+  token: Token;
+
+  /**
+   * A 'Reversion' that will return the context to its prior state.
+   */
+  reversion: Reversion;
+}
+
 /**
  * Describes what kind of model to instantiate.
  */

common/predictive-text/unit_tests/headless/worker-dummy-integration.js

@@ -14,6 +14,12 @@ describe('LMLayer using dummy model', function () {
     it('will predict future suggestions', function () {
       var lmLayer = new LMLayer(capabilities());
 
+      var stripIDs = function(suggestions) {
+        suggestions.forEach(function(suggestion) {
+          delete suggestion.id;
+        });
+      }
+
       // We're testing many as asynchronous messages in a row.
       // this would be cleaner using async/await syntax.
       // Not done yet, as this test case is a slightly-edited copy of the in-browser version.
@@ -25,15 +31,19 @@ describe('LMLayer using dummy model', function () {
       }).then(function () {
         return lmLayer.predict(zeroTransform(), emptyContext());
       }).then(function (suggestions) {
+        stripIDs(suggestions);
         assert.deepEqual(suggestions, iGotDistractedByHazel()[0]);
         return lmLayer.predict(zeroTransform(), emptyContext());
       }).then(function (suggestions) {
+        stripIDs(suggestions);
         assert.deepEqual(suggestions, iGotDistractedByHazel()[1]);
         return lmLayer.predict(zeroTransform(), emptyContext());
       }).then(function (suggestions) {
+        stripIDs(suggestions);
         assert.deepEqual(suggestions, iGotDistractedByHazel()[2]);
         return lmLayer.predict(zeroTransform(), emptyContext());
       }).then(function (suggestions) {
+        stripIDs(suggestions);
         assert.deepEqual(suggestions, iGotDistractedByHazel()[3]);
         lmLayer.shutdown();
         return Promise.resolve();

common/predictive-text/unit_tests/headless/worker-model-compositor.js

@@ -102,9 +102,9 @@ describe('ModelCompositor', function() {
   
         var keep;
         if(quoteStyle) {
-          keep = compositor.toAnnotatedKeepSuggestion(baseSuggestion, quoteStyle);
+          keep = compositor.toAnnotatedSuggestion(baseSuggestion, 'keep', quoteStyle);
         } else {
-          keep = compositor.toAnnotatedKeepSuggestion(baseSuggestion);
+          keep = compositor.toAnnotatedSuggestion(baseSuggestion, 'keep');
         }
 
         // Make sure we didn't accidentally leak any mutations to the parameter.
@@ -134,4 +134,180 @@ describe('ModelCompositor', function() {
       });
     });
   });
+
+  describe('acceptSuggestion', function() {
+    let acceptanceTest = function(punctuation, suggestion, context, postTransform) {
+      let options = {
+        punctuation: punctuation
+      };
+
+      let model = new models.DummyModel(options);
+      let compositor = new ModelCompositor(model);
+
+      return compositor.acceptSuggestion(suggestion, context, postTransform);
+    }
+
+    let englishPunctuation = {
+      quotesForKeepSuggestion: { open: `“`, close: `”`},
+      insertAfterWord: ' '
+    };
+
+    let angledPunctuation = {
+      quotesForKeepSuggestion: { open: `«`, close: `»`},
+      insertAfterWord: " "
+    }
+
+    it('first word of context, postTransform provided, .deleteLeft = 0', function() {
+      let baseSuggestion = {
+        transform: {
+          insert: 'hello ',
+          deleteLeft: 2,
+          id: 0
+        },
+        transformId: 0,
+        displayAs: 'hello'
+      };
+  
+      let baseContext = {
+        left: 'he', startOfBuffer: true, endOfBuffer: true
+      }
+    
+      // Represents the keystroke that triggered the suggestion.  It's not technically part
+      // of the Context when the suggestion is built.
+      let postTransform = {
+        insert: 'l',
+        deleteLeft: 0
+      }
+
+      let reversion = acceptanceTest(englishPunctuation, baseSuggestion, baseContext, postTransform);
+
+      // Check #1:  Does the returned reversion properly revert the context to its pre-application state?
+      //            Does this include characters not considered when the Suggestion was built?
+      let unappliedContext = models.applyTransform(postTransform, baseContext);
+      let appliedContext = models.applyTransform(baseSuggestion.transform, baseContext);
+      assert.equal(appliedContext.left, "hello ");
+
+      let revertedContext = models.applyTransform(reversion.transform, appliedContext);
+      assert.deepEqual(revertedContext, unappliedContext);
+
+      // Check #2:  Are the correct display strings built, depending on the active model's punctuation?
+      assert.equal(reversion.displayAs, "“hel”"); // text should _basically_ be a quoted version of `preApplyContext.left`
+
+      let angledReversion = acceptanceTest(angledPunctuation, baseSuggestion, baseContext, postTransform);
+      assert.equal(angledReversion.displayAs, "«hel»");
+    });
+
+    it('second word of context, postTransform provided, .deleteLeft = 0', function() {
+      let baseSuggestion = {
+        transform: {
+          insert: 'world ',
+          deleteLeft: 3,
+          id: 0
+        },
+        transformId: 0,
+        displayAs: 'world'
+      };
+  
+      let baseContext = {
+        left: 'hello wot', startOfBuffer: true, endOfBuffer: true
+      }
+    
+      // Represents the keystroke that triggered the suggestion.  It's not technically part
+      // of the Context when the suggestion is built.
+      let postTransform = {
+        insert: 'l',
+        deleteLeft: 0
+      }
+
+      let reversion = acceptanceTest(englishPunctuation, baseSuggestion, baseContext, postTransform);
+
+      // Check #1:  Does the returned reversion properly revert the context to its pre-application state?
+      //            Does this include characters not considered when the Suggestion was built?
+      let unappliedContext = models.applyTransform(postTransform, baseContext);
+      let appliedContext = models.applyTransform(baseSuggestion.transform, baseContext);
+      assert.equal(appliedContext.left, "hello world ");
+
+      let revertedContext = models.applyTransform(reversion.transform, appliedContext);
+      assert.deepEqual(revertedContext, unappliedContext);
+
+      // Check #2:  Are the correct display strings built, depending on the active model's punctuation?
+      assert.equal(reversion.displayAs, "“wotl”"); // text should _basically_ be a quoted version of `preApplyContext.left`
+
+      let angledReversion = acceptanceTest(angledPunctuation, baseSuggestion, baseContext, postTransform);
+      assert.equal(angledReversion.displayAs, "«wotl»");
+    });
+
+    it('second word of context, postTransform undefined', function() {
+      let baseSuggestion = {
+        transform: {
+          insert: 'world ',
+          deleteLeft: 3,
+          id: 0
+        },
+        transformId: 0,
+        displayAs: 'world'
+      };
+  
+      let baseContext = {
+        left: 'hello wot', startOfBuffer: true, endOfBuffer: true
+      }
+
+      let reversion = acceptanceTest(englishPunctuation, baseSuggestion, baseContext);
+
+      // Check #1:  Does the returned reversion properly revert the context to its pre-application state?
+      //            Does this include characters not considered when the Suggestion was built?
+      let unappliedContext = models.applyTransform({insert: '', deleteLeft: 0}, baseContext); // to clone the original context.
+      let appliedContext = models.applyTransform(baseSuggestion.transform, baseContext);
+      assert.equal(appliedContext.left, "hello world ");
+
+      let revertedContext = models.applyTransform(reversion.transform, appliedContext);
+      assert.deepEqual(revertedContext, unappliedContext);
+
+      // Check #2:  Are the correct display strings built, depending on the active model's punctuation?
+      assert.equal(reversion.displayAs, "“wot”"); // text should _basically_ be a quoted version of `preApplyContext.left`
+
+      let angledReversion = acceptanceTest(angledPunctuation, baseSuggestion, baseContext);
+      assert.equal(angledReversion.displayAs, "«wot»");
+    });
+
+    it('first word of context + postTransform provided, .deleteLeft > 0', function() {
+      let baseSuggestion = {
+        transform: {
+          insert: 'hello ',
+          deleteLeft: 2,
+          id: 0
+        },
+        transformId: 0,
+        displayAs: 'hello'
+      };
+  
+      let baseContext = {
+        left: 'he', startOfBuffer: true, endOfBuffer: true
+      }
+    
+      // Represents the keystroke that triggered the suggestion.  It's not technically part
+      // of the Context when the suggestion is built.
+      let postTransform = {
+        insert: 'i',
+        deleteLeft: 1
+      }
+
+      let reversion = acceptanceTest(englishPunctuation, baseSuggestion, baseContext, postTransform);
+
+      // Check #1:  Does the returned reversion properly revert the context to its pre-application state?
+      //            Does this include characters not considered when the Suggestion was built?
+      let unappliedContext = models.applyTransform(postTransform, baseContext);
+      let appliedContext = models.applyTransform(baseSuggestion.transform, baseContext);
+      assert.equal(appliedContext.left, "hello ");
+
+      let revertedContext = models.applyTransform(reversion.transform, appliedContext);
+      assert.deepEqual(revertedContext, unappliedContext);
+
+      // Check #2:  Are the correct display strings built, depending on the active model's punctuation?
+      assert.equal(reversion.displayAs, "“hi”"); // text should _basically_ be a quoted version of `preApplyContext.left`
+
+      let angledReversion = acceptanceTest(angledPunctuation, baseSuggestion, baseContext, postTransform);
+      assert.equal(angledReversion.displayAs, "«hi»");
+    });
+  });
 });

common/predictive-text/unit_tests/headless/worker-predict.js

@@ -17,8 +17,20 @@ describe('LMLayerWorker', function () {
 
       // Initialize the worker with a model that will produce one suggestion.
       var fakePostMessage = sinon.fake();
+      var filteredFakePostMessage = function(event) {
+        if(event.message == 'suggestions') {
+          let suggestions = event.suggestions;
+
+          // Strip any IDs set by the model compositor.
+          suggestions.forEach(function(suggestion) {
+            delete suggestion.id;
+          });
+        }
+
+        fakePostMessage(event);
+      }
       var context = {
-        postMessage: fakePostMessage
+        postMessage: filteredFakePostMessage
       };
       context.importScripts = importScriptsWith(context);
 

common/predictive-text/unit_tests/in_browser/cases/worker-dummy-integration.js

@@ -18,6 +18,12 @@ describe('LMLayer using dummy model', function () {
                                                   // the WebWorker boundary, so we should be generous here.
       var lmLayer = new LMLayer(helpers.defaultCapabilities);
 
+      var stripIDs = function(suggestions) {
+        suggestions.forEach(function(suggestion) {
+          delete suggestion.id;
+        });
+      }
+
       // We're testing many as asynchronous messages in a row.
       // this would be cleaner using async/await syntax, but
       // alas some of our browsers don't support it.
@@ -29,15 +35,19 @@ describe('LMLayer using dummy model', function () {
       }).then(function () {
         return lmLayer.predict(zeroTransform(), emptyContext());
       }).then(function (suggestions) {
+        stripIDs(suggestions);
         assert.deepEqual(suggestions, iGotDistractedByHazel()[0]);
         return lmLayer.predict(zeroTransform(), emptyContext());
       }).then(function (suggestions) {
+        stripIDs(suggestions);
         assert.deepEqual(suggestions, iGotDistractedByHazel()[1]);
         return lmLayer.predict(zeroTransform(), emptyContext());
       }).then(function (suggestions) {
+        stripIDs(suggestions);
         assert.deepEqual(suggestions, iGotDistractedByHazel()[2]);
         return lmLayer.predict(zeroTransform(), emptyContext());
       }).then(function (suggestions) {
+        stripIDs(suggestions);
         assert.deepEqual(suggestions, iGotDistractedByHazel()[3]);
         lmLayer.shutdown();
         return Promise.resolve();

common/predictive-text/worker/index.ts

@@ -279,7 +279,7 @@ class LMLayerWorker {
       handleMessage: (payload) => {
         switch(payload.message) {
           case 'predict':
-            let {transform, context} = payload;
+            var {transform, context} = payload;
             let suggestions = compositor.predict(transform, context);
 
             // Now that the suggestions are ready, send them out!
@@ -299,8 +299,17 @@ class LMLayerWorker {
           case 'unload':
             this.unloadModel();
             break;
+          case 'accept':
+            var {suggestion, context, postTransform} = payload;
+            var reversion = compositor.acceptSuggestion(suggestion, context, postTransform);
+
+            this.cast('postaccept', {
+              token: payload.token,
+              reversion: reversion
+            });
+            break;
           default:
-          throw new Error(`invalid message; expected one of {'predict', 'unload'} but got ${payload.message}`);
+          throw new Error(`invalid message; expected one of {'predict', 'wordbreak', 'accept', 'unload'} but got ${payload.message}`);
         }
       },
       compositor: compositor

common/predictive-text/worker/model-compositor.ts

@@ -90,6 +90,7 @@ class ModelCompositor {
 
     // Used to restore whitespaces if operations would remove them.
     let prefixTransform: Transform;
+    let contextState: correction.TrackedContextState = null;
 
     // Section 1:  determining 'prediction roots'.
     if(!this.contextTracker) {
@@ -124,12 +125,12 @@ class ModelCompositor {
       // Running in bulk over all suggestions, duplicate entries may be possible.
       rawPredictions = this.predictFromCorrections(predictionRoots, context);
     } else {
-      let contextState = this.contextTracker.analyzeState(this.lexicalModel, 
-                                                          postContext, 
-                                                          !this.isEmpty(inputTransform) ? 
-                                                                        transformDistribution: 
-                                                                        [{sample: inputTransform, p: 1.0}]
-                                                          );
+      contextState = this.contextTracker.analyzeState(this.lexicalModel, 
+                                                      postContext, 
+                                                      !this.isEmpty(inputTransform) ? 
+                                                                    transformDistribution: 
+                                                                    [{sample: inputTransform, p: 1.0}]
+                                                      );
 
       // TODO:  Should we filter backspaces & whitespaces out of the transform distribution?
       //        Ideally, the answer (in the future) will be no, but leaving it in right now may pose an issue.
@@ -228,7 +229,7 @@ class ModelCompositor {
         }
 
         keepOption = models.transformToSuggestion(keepTransform, prediction.p);
-        keepOption = this.toAnnotatedKeepSuggestion(keepOption, models.QuoteBehavior.noQuotes);
+        keepOption = this.toAnnotatedSuggestion(keepOption, 'keep',  models.QuoteBehavior.noQuotes);
       } else {
         let existingSuggestion = suggestionDistribMap[displayText];
         if(existingSuggestion) {
@@ -245,7 +246,7 @@ class ModelCompositor {
       // This is the one case where the transform doesn't insert the full word; we need to override the displayAs param.
       keepTransform.displayAs = keepOptionText;
 
-      keepOption = this.toAnnotatedKeepSuggestion(keepTransform);
+      keepOption = this.toAnnotatedSuggestion(keepTransform, 'keep');
     }
 
     // Section 3:  Finalize suggestions, truncate list to the N (MAX_SUGGESTIONS) most optimal, return.
@@ -279,32 +280,58 @@ class ModelCompositor {
       suggestions = [ keepOption ].concat(suggestions);
     }
 
-    // Apply 'after word' punctuation.  We delay until now so that utility functions relying on the
-    // unmodified Transform may execute properly.
+    // Apply 'after word' punctuation and set suggestion IDs.  
+    // We delay until now so that utility functions relying on the unmodified Transform may execute properly.
     suggestions.forEach(function(suggestion) {
       if (suggestion.transform.insert.length > 0) {
         suggestion.transform.insert += punctuation.insertAfterWord;
 
         // If this is a suggestion after wordbreak input, make sure we preserve the wordbreak transform!
         if(prefixTransform) {
-          models.prependTransform(suggestion.transform, prefixTransform);
+          let mergedTransform = models.buildMergedTransform(prefixTransform, suggestion.transform);
+          mergedTransform.id = suggestion.transformId;
+
+          // 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
+          let mutableSuggestion = suggestion as {-readonly [transform in keyof Suggestion]: Suggestion[transform]};
+          
+          // Assignment via by-reference behavior, as suggestion is an object
+          mutableSuggestion.transform = mergedTransform;
         }
       }
     });
 
+    // 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.
+    if(contextState) {
+      // TODO:  context tracking enhancements
+    }
+
     return suggestions;
   }
 
-  private toAnnotatedKeepSuggestion(suggestion: Suggestion & {p?: number}, 
-                                    quoteBehavior: models.QuoteBehavior = models.QuoteBehavior.default): Suggestion & {p?: number} {
+  private toAnnotatedSuggestion(suggestion: Suggestion & {p?: number}, 
+    annotationType: SuggestionTag,
+    quoteBehavior?: models.QuoteBehavior): Suggestion & {p?: number};
+  private toAnnotatedSuggestion(suggestion: Suggestion & {p?: number}, 
+    annotationType: 'revert',
+    quoteBehavior?: models.QuoteBehavior): Reversion & {p?: number};
+  private toAnnotatedSuggestion(suggestion: Suggestion & {p?: number}, 
+                                annotationType: SuggestionTag,
+                                quoteBehavior: models.QuoteBehavior = models.QuoteBehavior.default): Suggestion & {p?: number} {
     // A method-internal 'import' of the enum.
     let QuoteBehavior = models.QuoteBehavior;
 
+    let defaultQuoteBehavior = QuoteBehavior.noQuotes;
+    if(annotationType == 'keep' || annotationType == 'revert') {
+      defaultQuoteBehavior = QuoteBehavior.useQuotes;
+    } 
+
     return {
       transform: suggestion.transform,
       transformId: suggestion.transformId,
-      displayAs: QuoteBehavior.apply(quoteBehavior, suggestion.displayAs, this.punctuation, QuoteBehavior.useQuotes),
-      tag: 'keep',
+      displayAs: QuoteBehavior.apply(quoteBehavior, suggestion.displayAs, this.punctuation, defaultQuoteBehavior),
+      tag: annotationType,
       p: suggestion.p
     };
   }
@@ -337,6 +364,48 @@ class ModelCompositor {
       insertAfterWord, quotesForKeepSuggestion, isRTL
     }
   }
+
+  acceptSuggestion(suggestion: Suggestion, context: Context, postTransform?: Transform): Reversion {
+    // Step 1:  generate and save the reversion's Transform.
+    let sourceTransform = suggestion.transform;
+    let deletedLeftChars = context.left.kmwSubstr(-sourceTransform.deleteLeft, sourceTransform.deleteLeft);
+    // right deletion is currently not implemented.
+    let insertedLength = sourceTransform.insert.kmwLength();
+
+    let reversionTransform: Transform = {
+      insert: deletedLeftChars,
+      deleteLeft: insertedLength
+    };
+
+    // Step 2:  building the proper 'displayAs' string for the Reversion
+    if(postTransform) {
+      // The code above restores the state to the context at the time the `Suggestion` was created.
+      // `postTransform` handles any missing context that came later.
+      reversionTransform = models.buildMergedTransform(reversionTransform, postTransform);
+
+      // Now that we've built the reversion based upon the Suggestion's original context,
+      // we may safely manipulate it in order to get a proper 'displayAs' string.
+      context = models.applyTransform(postTransform, context);
+    }
+
+    let postContextTokens = this.lexicalModel.tokenize(context); //.wordbreak(postContext);
+    let revertedPrefix = postContextTokens[postContextTokens.length - 1];
+
+    let firstConversion = models.transformToSuggestion(reversionTransform);
+    firstConversion.displayAs = revertedPrefix;
+    
+    // Build the actual Reversion, which is technically an annotated Suggestion.
+    // Since we're outside of the standard `predict` control path, we'll need to
+    // set the Reversion's ID directly.
+    let reversion = this.toAnnotatedSuggestion(firstConversion, 'revert');
+    
+    // Step 3:  if we track Contexts, update the tracking data as appropriate.
+    if(this.contextTracker) {
+      // TODO:  implement.
+    }
+
+    return reversion;
+  }
 }
 
 /**

common/predictive-text/worker/worker-interfaces.ts

@@ -38,8 +38,8 @@ type ImportScripts = typeof DedicatedWorkerGlobalScope.prototype.importScripts;
 /**
  * The valid incoming message kinds.
  */
-type IncomingMessageKind = 'config' | 'load' | 'predict' | 'unload' | 'wordbreak';
-type IncomingMessage = ConfigMessage | LoadMessage | PredictMessage | UnloadMessage | WordbreakMessage;
+type IncomingMessageKind = 'config' | 'load' | 'predict' | 'unload' | 'wordbreak' | 'accept';
+type IncomingMessage = ConfigMessage | LoadMessage | PredictMessage | UnloadMessage | WordbreakMessage | AcceptMessage;
 
 /**
  * The structure of a config message.  It should include the platform's supported
@@ -118,6 +118,37 @@ interface WordbreakMessage {
   context: Context;
 }
 
+interface AcceptMessage {
+  message: 'accept';
+
+  /**
+   * Opaque, unique token that pairs this accept message with its return message.
+   */
+  token: Token;
+
+  /**
+   * The Suggestion being accepted.  The ID must be assigned.
+   */
+  suggestion: Suggestion;
+
+  /**
+   * The context (text to the left and text to right) at the
+   * insertion point/text cursor, at the moment the Suggestion
+   * was generated.
+   */
+  context: Context;
+
+  /**
+   * A Transform representing any text manipulations applied to 
+   * the Context after the `suggestion` was generated.  
+   * 
+   * Necessary, as Suggestions are generated without applying their
+   * triggering keystroke to the Context.  (The current context is 
+   * thus likely to differ.)
+   */
+  postTransform?: Transform;
+}
+
 /**
  * The LMLayer can be in one of the following states. The LMLayer can only produce predictions in the 'ready' state.
  */

web/source/osk/banner.ts

@@ -244,7 +244,7 @@ namespace com.keyman.osk {
      * @param target (Optional) The OutputTarget to which the `Suggestion` ought be applied.
      * Description  Applies the predictive `Suggestion` represented by this `BannerSuggestion`.
      */
-    public apply(target?: text.OutputTarget): Promise<Suggestion> {
+    public apply(target?: text.OutputTarget): Promise<Reversion> {
       let keyman = com.keyman.singleton;
 
       if(this.isEmpty()) {
@@ -531,13 +531,13 @@ namespace com.keyman.osk {
 
     private currentSuggestions: Suggestion[] = [];
     private keepSuggestion: Suggestion;
-    private revertSuggestion: Suggestion;
+    private revertSuggestion: Reversion;
 
     private currentTranscriptionID: number;
 
     private recentAccept: boolean = false;
     private recentAccepted: Suggestion;
-    private revertAcceptancePromise: Promise<Suggestion>;
+    private revertAcceptancePromise: Promise<Reversion>;
 
     private preAccept: text.Transcription = null;
     private swallowPrediction: boolean = false;