Merge branch 'change/web/suggestion-tracking' into refactor/web/tokenization-transition

web/src/engine/common/web-utils/src/rewindable-cache.ts

@@ -14,12 +14,10 @@
  * reaching a specified capacity and that provides the ability to
  * forget _all_ entries more recent than a specified entry.
  */
-export class RewindableCache<Key, Value> {
+export class RewindableCache<Value> {
   static readonly DEFAULT_MAX_SIZE = 5;
 
-  private cacheOrderSet: Set<Key> = new Set();
-  private transitionMap: Map<Key, Value> = new Map();
-
+  private map: Map<number, Value> = new Map();
   private maxSize: number;
 
   /**
@@ -34,7 +32,7 @@ export class RewindableCache<Key, Value> {
    * Gets the current number of entries in the cache.
    */
   get size() {
-    return this.cacheOrderSet.size;
+    return this.map.size;
   }
 
   /**
@@ -43,53 +41,31 @@ export class RewindableCache<Key, Value> {
    * @param key
    * @returns
    */
-  peek(key: Key): Value {
-    const entry = this.transitionMap.get(key);
+  get(key: number): Value {
+    const entry = this.map.get(key);
     return entry;
   }
 
-  /**
-   * Fetches the cache entry with the specified key and also updates its
-   * most recent access time.
-   * @param key
-   * @returns
-   */
-  get(key: Key): Value {
-    const set = this.cacheOrderSet;
-    if(!set.has(key)) {
-      return null;
-    }
-
-    set.delete(key);
-    set.add(key);
-
-    return this.peek(key);
-  }
-
   /**
    * Adds the specified key-value pair as a cache entry, overwriting any
    * previous value for the specified key.
    * @param key
    * @returns
    */
-  add(key: Key, value: Value) {
-    const set = this.cacheOrderSet;
-    const map = this.transitionMap;
+  add(key: number, value: Value) {
+    const map = this.map;
 
-    if(set.has(key)) {
-      set.delete(key);
-      set.add(key);
+    if(map.has(key)) {
+      map.delete(key);
+      map.set(key, value);
     } else {
-      set.add(key);
+      map.set(key, value);
 
-      if(set.size > this.maxSize) {
-        const oldest = set.keys().next().value as Key;
-        set.delete(oldest);
+      if(map.size > this.maxSize) {
+        const oldest = map.keys().next().value as number;
         map.delete(oldest);
       }
     }
-
-    this.transitionMap.set(key, value);
   }
 
   /**
@@ -99,44 +75,38 @@ export class RewindableCache<Key, Value> {
    * Note:  returns an Array and is accordingly O(N).
    * @returns
    */
-  keys(): Key[] {
-    return [...this.cacheOrderSet.keys()].reverse();
+  keys(): number[] {
+    return [...this.map.keys()].reverse();
   }
 
   /**
    * Deletes all entries from the cache.
    */
   clear() {
-    this.cacheOrderSet = new Set();
-    this.transitionMap = new Map();
+    this.map = new Map();
   }
 
   /**
-   * Deletes all entries more recent than the specified key, leaving
-   * the specified key as the most recent entry.
+   * Deletes all entries with a key greater than the specified key (which indicates
+   * the entry was first created more recently), leaving the specified key as
+   * the most recent entry.
    * @param key
    */
-  rewindTo(key: Key) {
-    const set = this.cacheOrderSet;
-    const map = this.transitionMap;
+  rewindTo(key: number) {
+    const map = this.map;
 
-    if(!set.has(key)) {
+    if(!map.has(key)) {
       throw new Error("Key not found; cannot clear entries appearing afterward.");
     }
 
-    let keyFound: boolean = false;
     // Iteration:  by insertion order
     // The most recent entries will appear after the specified key.
-    for(const k of set.keys()) {
-      if(!keyFound) {
-        keyFound = k == key;
-        continue;
+    for(const k of map.keys()) {
+      if(k > key) {
+        // It is safe to delete a key from a Set or Map while iterating over it in ES6+.
+        // https://stackoverflow.com/a/28306768
+        map.delete(k);
       }
-
-      // It is safe to delete a key from a Set while iterating over it in ES6+.
-      // https://stackoverflow.com/a/28306768
-      set.delete(k);
-      map.delete(k);
     }
   }
 }

web/src/engine/common/web-utils/src/tests/rewindableCache.tests.js

@@ -62,11 +62,8 @@ describe("RewindableCache", () => {
     cache.add(4, 'd');
     cache.add(5, 'e');
 
-    assert.sameMembers([...cache.keys()], [4, 5]);
-    assert.sameMembers([...cache.keys().map((k) => cache.peek(k))], ['d', 'e']);
-
     assert.sameOrderedMembers([...cache.keys()], [5, 4]);
-    assert.sameOrderedMembers([...cache.keys().map((k) => cache.peek(k))], ['e', 'd']);
+    assert.sameOrderedMembers([...cache.keys().map((k) => cache.get(k))], ['e', 'd']);
     assert.equal(cache.size, 2);
   });
 
@@ -78,26 +75,23 @@ describe("RewindableCache", () => {
     cache.add(4, 'd');
     cache.add(5, 'e');
 
-    cache.get(2);
+    // make it more recently-accessed, but don't remove it.
     cache.rewindTo(3);
 
-    assert.sameMembers([...cache.keys()], [1, 3]);
-    assert.sameMembers([...cache.keys().map((k) => cache.peek(k))], ['a', 'c']);
-
-    assert.sameOrderedMembers([...cache.keys()], [3, 1]);
-    assert.sameOrderedMembers([...cache.keys().map((k) => cache.peek(k))], ['c', 'a']);
-    assert.equal(cache.size, 2);
+    cache.add(2, 'b'); // 2 is still ORIGINALLY older than 3, even if accessed more recently.
+    assert.sameOrderedMembers([...cache.keys()], [2, 3, 1]);
+    assert.sameOrderedMembers([...cache.keys().map((k) => cache.get(k))], ['b', 'c', 'a']);
+    assert.equal(cache.size, 3);
   });
 
   it("forgets no entries if invalid key is provided", () => {
     const cache = new RewindableCache(2);
     cache.add(1, 'a');
     cache.add(2, 'b');
-    cache.add(3, 'c');
-    cache.add(4, 'd');
-    cache.add(5, 'e');
+    cache.add(3, 'c'); // Entry 1 no longer valid
+    cache.add(4, 'd'); //       2
+    cache.add(5, 'e'); //       3
 
-    assert.doesNotThrow(() => cache.get(2));
     assert.throws(() => cache.rewindTo(3));
   });
 
@@ -105,11 +99,10 @@ describe("RewindableCache", () => {
     const cache = new RewindableCache(2);
     cache.add(1, 'a');
     cache.add(2, 'b');
-    cache.add(3, 'c');
-    cache.add(4, 'd');
-    cache.add(5, 'e');
+    cache.add(3, 'c'); // Entry 1 no longer valid
+    cache.add(4, 'd'); //       2
+    cache.add(5, 'e'); //       3
 
-    assert.doesNotThrow(() => cache.peek(2));
-    assert.doesNotThrow(() => cache.get(2));
+    assert.isUndefined(cache.get(2), 'outdated cache entry should be forgotten');
   });
 });

web/src/engine/main/src/headless/inputProcessor.ts

@@ -103,7 +103,7 @@ export class InputProcessor {
       // Support for multitap context reversion; multitap keys should act as if they were
       // the first thing typed since `preInput`, the state before the original base key.
       if(keyEvent.baseTranscriptionToken) {
-        const transcription = this.contextCache.peek(keyEvent.baseTranscriptionToken);
+        const transcription = this.contextCache.get(keyEvent.baseTranscriptionToken);
         if(transcription) {
           this.contextCache.rewindTo(keyEvent.baseTranscriptionToken);
           // Has there been a context change at any point during the multitap?  If so, we need

web/src/engine/main/src/headless/languageProcessor.ts

@@ -380,7 +380,7 @@ export class LanguageProcessor extends EventEmitter<LanguageProcessorEventMap> {
    * @returns The matching `Transcription`, or `null` none is found.
    */
   public getPredictionState(id: number): Transcription {
-    return this.recentTranscriptions.peek(id);
+    return this.recentTranscriptions.get(id);
   }
 
   public shutdown() {

web/src/engine/main/src/headless/transcriptionCache.ts

@@ -3,7 +3,7 @@ import { RewindableCache } from "@keymanapp/web-utils";
 
 const TRANSCRIPTION_BUFFER_SIZE = 10;
 
-export class TranscriptionCache extends RewindableCache<number, Transcription> {
+export class TranscriptionCache extends RewindableCache<Transcription> {
   constructor() {
     super(TRANSCRIPTION_BUFFER_SIZE);
   }
@@ -17,7 +17,7 @@ export class TranscriptionCache extends RewindableCache<number, Transcription> {
     const entries = [...this.keys()].map((key) => {
       return {
         key: key,
-        entry: this.peek(key)
+        entry: this.get(key)
       }
     });
 

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

@@ -12,7 +12,7 @@ import { ContextState } from './context-state.js';
 import { ContextTransition } from './context-transition.js';
 
 export class ContextTracker {
-  readonly cache = new RewindableCache<number, ContextTransition>(5);
+  readonly cache = new RewindableCache<ContextTransition>(5);
 
   /**
    * As written, this method attempts to match an incoming context with a candidate
@@ -139,7 +139,7 @@ export class ContextTracker {
 
     if(tokenizedPostContext.left.length > 0) {
       for(const id of this.cache.keys()) {
-        const priorMatchState = this.cache.peek(id);
+        const priorMatchState = this.cache.get(id);
 
         // Skip intermediate multitap-produced contexts.
         // When multitapping, we skip all contexts from prior taps within the same interaction,
@@ -175,7 +175,8 @@ export class ContextTracker {
             if(result.final != result.base) {
               this.cache.add(transitionId, result);
             } else {
-              return this.cache.peek(priorMatchState.transitionId);
+              this.cache.add(priorMatchState.transitionId, priorMatchState);
+              return this.cache.get(priorMatchState.transitionId);
             }
           }
 
@@ -203,7 +204,7 @@ export class ContextTracker {
     if(key === undefined) {
       return undefined;
     } else {
-      return this.cache.peek(key);
+      return this.cache.get(key);
     }
   }
 

web/src/engine/predictive-text/worker-thread/src/main/model-compositor.ts

@@ -299,7 +299,7 @@ export class ModelCompositor {
     }
 
     // When the context is tracked, we prefer the tracked information.
-    let originalTransition = this.contextTracker.cache.peek(-reversion.transformId);
+    let originalTransition = this.contextTracker.cache.get(-reversion.transformId);
 
     if(!originalTransition) {
       // We forgot the original base state... and what suggestions we gave.  We can make

web/src/test/auto/headless/engine/predictive-text/worker-thread/worker-model-compositor.tests.ts

@@ -894,6 +894,7 @@ describe('ModelCompositor', function() {
       assert.equal(compositor.contextTracker.cache.size, 2);
 
       let contextIds = compositor.contextTracker.cache.keys();
+      let transitionInstances = compositor.contextTracker.cache.keys().map((key) => compositor.contextTracker.cache.get(key));
 
       let baseSuggestion = initialSuggestions[1];
       let reversion = compositor.acceptSuggestion(baseSuggestion, baseContext, postTransform);
@@ -906,6 +907,7 @@ describe('ModelCompositor', function() {
       // Accepting the suggestion rewrites the latest context transition.
       assert.equal(compositor.contextTracker.cache.size, 3);
       assert.sameMembers(compositor.contextTracker.cache.keys(), [15, ...contextIds]);
+      assert.notSameDeepMembers(compositor.contextTracker.cache.keys().map((key) => compositor.contextTracker.cache.get(key)), transitionInstances);
 
       // The replacement should be marked on the context-tracking token for the applied version of the results.
       assert.equal(suggestionContextState.final.appliedSuggestionId, undefined);
@@ -914,7 +916,7 @@ describe('ModelCompositor', function() {
       let appliedContext = models.applyTransform(baseSuggestion.transform, baseContext);
       await compositor.applyReversion(reversion, appliedContext);
       assert.equal(compositor.contextTracker.cache.size, 2);
-      assert.isUndefined(compositor.contextTracker.cache.peek(13).final.appliedSuggestionId);
+      assert.isUndefined(compositor.contextTracker.cache.get(13).final.appliedSuggestionId);
     });
   });
 });