diff --git a/common/core/web/input-processor/src/text/prediction/languageProcessor.ts b/common/core/web/input-processor/src/text/prediction/languageProcessor.ts index e3f781f903..b02384eab2 100644 --- a/common/core/web/input-processor/src/text/prediction/languageProcessor.ts +++ b/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 { + public applySuggestion(suggestion: Suggestion, outputTarget: OutputTarget): Promise { 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 = new Promise(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 = 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; } diff --git a/common/models/templates/src/common.ts b/common/models/templates/src/common.ts index 73d360a2e7..88dbc31ad1 100644 --- a/common/models/templates/src/common.ts +++ b/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) } } diff --git a/common/models/templates/test/test-common.js b/common/models/templates/test/test-common.js index 61892ad985..299cb25443 100644 --- a/common/models/templates/test/test-common.js +++ b/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 = { diff --git a/common/models/types/index.d.ts b/common/models/types/index.d.ts index 543365cb81..084bd08591 100644 --- a/common/models/types/index.d.ts +++ b/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). diff --git a/common/predictive-text/docs/worker-communication-protocol.md b/common/predictive-text/docs/worker-communication-protocol.md index ed4f72a07b..f109ed5c7c 100644 --- a/common/predictive-text/docs/worker-communication-protocol.md +++ b/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 diff --git a/common/predictive-text/index.ts b/common/predictive-text/index.ts index f4ec5e1567..1d20c13ddf 100644 --- a/common/predictive-text/index.ts +++ b/common/predictive-text/index.ts @@ -53,6 +53,7 @@ namespace com.keyman.text.prediction { private _declareLMLayerReady: (conf: Configuration) => void; private _predictPromises: PromiseStore; private _wordbreakPromises: PromiseStore; + private _acceptPromises: PromiseStore; 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(); + 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 { + 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 diff --git a/common/predictive-text/message.d.ts b/common/predictive-text/message.d.ts index 1cf4ce1fcd..8765a616e9 100644 --- a/common/predictive-text/message.d.ts +++ b/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. */ diff --git a/common/predictive-text/unit_tests/headless/worker-dummy-integration.js b/common/predictive-text/unit_tests/headless/worker-dummy-integration.js index bea5941d0e..6f6c34d860 100644 --- a/common/predictive-text/unit_tests/headless/worker-dummy-integration.js +++ b/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(); diff --git a/common/predictive-text/unit_tests/headless/worker-model-compositor.js b/common/predictive-text/unit_tests/headless/worker-model-compositor.js index 1ba4ba0497..8769a71849 100644 --- a/common/predictive-text/unit_tests/headless/worker-model-compositor.js +++ b/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»"); + }); + }); }); diff --git a/common/predictive-text/unit_tests/headless/worker-predict.js b/common/predictive-text/unit_tests/headless/worker-predict.js index 8f935c7804..c9d7eb42d7 100644 --- a/common/predictive-text/unit_tests/headless/worker-predict.js +++ b/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); diff --git a/common/predictive-text/unit_tests/in_browser/cases/worker-dummy-integration.js b/common/predictive-text/unit_tests/in_browser/cases/worker-dummy-integration.js index 4702709d03..80f0f095f5 100644 --- a/common/predictive-text/unit_tests/in_browser/cases/worker-dummy-integration.js +++ b/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(); diff --git a/common/predictive-text/worker/index.ts b/common/predictive-text/worker/index.ts index ffa1df429b..a828033dd6 100644 --- a/common/predictive-text/worker/index.ts +++ b/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 diff --git a/common/predictive-text/worker/model-compositor.ts b/common/predictive-text/worker/model-compositor.ts index 5b4971edc7..0e46b9e4e2 100644 --- a/common/predictive-text/worker/model-compositor.ts +++ b/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; + } } /** diff --git a/common/predictive-text/worker/worker-interfaces.ts b/common/predictive-text/worker/worker-interfaces.ts index 32e5c60515..3422c5db89 100644 --- a/common/predictive-text/worker/worker-interfaces.ts +++ b/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. */ diff --git a/web/source/osk/banner.ts b/web/source/osk/banner.ts index a1e9252450..c8110840dc 100644 --- a/web/source/osk/banner.ts +++ b/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 { + public apply(target?: text.OutputTarget): Promise { 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; + private revertAcceptancePromise: Promise; private preAccept: text.Transcription = null; private swallowPrediction: boolean = false;