Merge pull request #15816 from keymanapp/refactor/web/extract-duplicate-result-filter

refactor(web): extract token-search result duplicate-filtering method 🚂

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

@@ -6,7 +6,7 @@ import { LexicalModelTypes } from '@keymanapp/common-types';
 import { ClassicalDistanceCalculation } from './classical-calculation.js';
 import { ExecutionTimer, STANDARD_TIME_BETWEEN_DEFERS } from './execution-timer.js';
 import { SearchQuotientNode } from './search-quotient-node.js';
-import { TokenResultMapping } from './token-result-mapping.js';
+import { initTokenResultFilterer, TokenResultMapping } from './token-result-mapping.js';
 import { subsetByChar, subsetByInterval, mergeSubset, TransformSubset } from '../transform-subsets.js';
 import TransformUtils from '../transformUtils.js';
 
@@ -574,6 +574,18 @@ export class SearchNode {
   }
 }
 
+/**
+ * Searches for the best available token corrections from among the provided
+ * SearchSpaces, ending after the configured timer has elapsed or all available
+ * corrections have been enumerated.
+ * @param searchModules
+ * @param timer
+ * @returns
+ */
+export const getBestTokenMatches = (searchModules: SearchQuotientNode[], timer: ExecutionTimer) => {
+  return getBestMatches(searchModules, timer, initTokenResultFilterer());
+}
+
 /**
  * Searches for the best available corrections from among the provided
  * SearchSpaces, ending after the configured timer has elapsed or all available
@@ -582,7 +594,13 @@ export class SearchNode {
  * @param timer
  * @returns
  */
-export async function *getBestMatches(searchModules: SearchQuotientNode[], timer: ExecutionTimer): AsyncGenerator<Readonly<TokenResultMapping>> {
+export async function *getBestMatches(
+  searchModules: SearchQuotientNode[],
+  timer: ExecutionTimer,
+  filter?: (searchResult: TokenResultMapping) => boolean
+): AsyncGenerator<Readonly<TokenResultMapping>> {
+  filter ??= () => true;
+
   const spaceQueue = new PriorityQueue<SearchQuotientNode>((a, b) => a.currentCost - b.currentCost);
 
   // Stage 1 - if we already have extracted results, build a queue just for them
@@ -597,15 +615,31 @@ export async function *getBestMatches(searchModules: SearchQuotientNode[], timer
   // With potential prior results re-queued, NOW enqueue.  (Not before - the heap may reheapify!)
   spaceQueue.enqueueAll(searchModules);
 
-  let currentReturns: {[resultKey: string]: TokenResultMapping} = {};
-
   // Stage 2:  the fun part; actually searching!
   do {
     const entry: TokenResultMapping = timer.time(() => {
       if((priorResultsQueue.peek()?.totalCost ?? Number.POSITIVE_INFINITY) <= spaceQueue.peek().currentCost) {
         const result = priorResultsQueue.dequeue();
-        currentReturns[result.matchString] = result;
-        return result;
+
+        // There's no guarantee that the filter closure is the same instance as
+        // before.
+        //
+        // As a filter function may contain caching and/or deduplication
+        // components, we pass pre-existing results through the filter so that
+        // it may reconstruct related state and thus cache/deduplicate new
+        // results based upon old results.
+        //
+        // See `initTokenResultFilterer()`, which maintains a map used for
+        // deduplication.
+        //
+        // As these _are_ pre-existing results, we know that they previously
+        // passed through the filter with a `true` response.  However, as #14366
+        // isn't implemented, it IS technically possible that a lower-cost
+        // result was found after a higher-cost result in some cases; therefore
+        // there is a chance such a duplicate may exist.  On that basis,
+        // re-filtering even for prior results is reasonably motivated at this
+        // time.
+        return filter(result) ? result : null;
       }
 
       let lowestCostSource = spaceQueue.dequeue();
@@ -616,25 +650,7 @@ export async function *getBestMatches(searchModules: SearchQuotientNode[], timer
         return null;
       } else if(newResult.type == 'complete') {
         const mapping = newResult.mapping;
-
-        // Is the entry a reasonable result?
-        if(mapping.isFullReplacement) {
-          // If the entry's 'match' fully replaces the input string, we consider it
-          // unreasonable and ignore it.  Also, if we've reached this point...
-          // we can(?) assume that everything thereafter is as well.
-          return null;
-        }
-
-        // As we can't guarantee a monotonically-increasing cost during the search -
-        // due to effects from keystrokes with deleteLeft > 0 - it's technically
-        // possible to find a lower-cost path later in such cases.
-        //
-        // If it occurs, we should re-emit it - it'll show up earlier in the
-        // suggestions that way, as it should.
-        if((currentReturns[mapping.matchString]?.totalCost ?? Number.MAX_VALUE) > newResult.cost) {
-          currentReturns[mapping.matchString] = newResult.mapping;
-          return newResult.mapping;
-        }
+        return filter(mapping) ? mapping : null;
       }
 
       return null;
@@ -652,4 +668,4 @@ export async function *getBestMatches(searchModules: SearchQuotientNode[], timer
   } while(!timer.elapsed && spaceQueue.peek().currentCost < Number.POSITIVE_INFINITY);
 
   return null;
-}
+}

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

@@ -51,7 +51,7 @@ export class LegacyQuotientSpur extends SearchQuotientSpur {
     return new LegacyQuotientSpur(parentNode, inputs, inputSource) as this;
   }
 
-  protected buildEdgesFromResults(priorResults: ReadonlyArray<TokenResultMapping>) {
+  protected buildEdgesFromResults(priorResults: ReadonlyArray<TokenResultMapping>): SearchNode[] {
     // With a newly-available input, we can extend new input-dependent paths from
     // our previously-reached 'extractedResults' nodes.
     let outboundNodes = priorResults.map((result) => {
@@ -83,12 +83,12 @@ export class LegacyQuotientSpur extends SearchQuotientSpur {
     const result = super.handleNextNode();
 
     if(result.type == 'complete') {
-      const currentMapping = result.mapping;
+      const parentResult = result.mapping;
 
       // Forbid a raw edit-distance of greater than 2.
       // Note:  .knownCost is not scaled, while its contribution to .currentCost _is_ scaled.
-      if(currentMapping.editCount < 2) {
-        let insertionEdges = currentMapping.buildInsertionEdges();
+      if(parentResult.editCount < 2) {
+        let insertionEdges = parentResult.buildInsertionEdges();
         this.queueNodes(insertionEdges);
       }
     }

web/src/engine/predictive-text/worker-thread/src/main/correction/search-quotient-cluster.ts

@@ -142,11 +142,16 @@ export class SearchQuotientCluster implements SearchQuotientNode {
     const currentResult = bestPath.handleNextNode();
     this.selectionQueue.enqueue(bestPath);
 
+    let finalResult = currentResult;
     if(currentResult.type == 'complete') {
-      this.completedPaths.push(new TokenResultMapping(currentResult.mapping, this.spaceId));
+      finalResult = {
+        ...currentResult,
+        mapping: new TokenResultMapping(currentResult.mapping, this.spaceId)
+      };
+      this.completedPaths.push(finalResult.mapping);
     }
 
-    return currentResult;
+    return finalResult;
   }
 
   public get previousResults(): TokenResultMapping[] {

web/src/engine/predictive-text/worker-thread/src/main/correction/token-result-mapping.ts

@@ -17,6 +17,29 @@ import LexiconTraversal = LexicalModelTypes.LexiconTraversal;
 import ProbabilityMass = LexicalModelTypes.ProbabilityMass;
 import Transform = LexicalModelTypes.Transform;
 
+export function initTokenResultFilterer() {
+  const priorReturnCosts: Map<string, number> = new Map();
+
+  const closure = (searchResult: TokenResultMapping) => {
+    if(searchResult.isFullReplacement) {
+      // If the entry's 'match' fully replaces the input string, we consider it
+      // unreasonable and ignore it.  Also, if we've reached this point...
+      // we can(?) assume that everything thereafter is as well.
+      return false;
+    }
+
+    if((priorReturnCosts.get(searchResult.matchString) ?? Number.MAX_VALUE) > searchResult.totalCost) {
+      priorReturnCosts.set(searchResult.matchString, searchResult.totalCost);
+
+      return true;
+    } else {
+      return false;
+    }
+  };
+
+  return closure;
+}
+
 export class TokenResultMapping {
   private readonly node: SearchNode;
   readonly spaceId: number;
@@ -56,7 +79,6 @@ export class TokenResultMapping {
   get isFullReplacement(): boolean {
     return this.node.isFullReplacement;
   }
-
   /**
    * Gets the number of Damerau-Levenshtein edits needed to reach the node's
    * matchString from the output induced by the input sequence used to reach it.

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

@@ -11,7 +11,7 @@ import { ContextState, determineContextSlideTransform } from './correction/conte
 import { ContextTransition } from './correction/context-transition.js';
 import { ExecutionTimer } from './correction/execution-timer.js';
 import ModelCompositor from './model-compositor.js';
-import { getBestMatches } from './correction/distance-modeler.js';
+import { getBestTokenMatches } from './correction/distance-modeler.js';
 import { TokenResultMapping } from './correction/token-result-mapping.js';
 
 const searchForProperty = defaultWordbreaker.searchForProperty;
@@ -518,7 +518,7 @@ export async function correctAndEnumerate(
   let rawPredictions: CorrectionPredictionTuple[] = [];
   let bestCorrectionCost: number;
   const correctionPredictionMap: Record<string, Distribution<Suggestion>> = {};
-  for await(const match of getBestMatches(searchModules, timer)) {
+  for await(const match of getBestTokenMatches(searchModules, timer)) {
     // Corrections obtained:  now to predict from them!
     const tokenization = tokenizations.find(t => t.spaceId == match.spaceId);
 

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

@@ -10,7 +10,7 @@
 import { assert } from 'chai';
 
 import { jsonFixture } from '@keymanapp/common-test-resources/model-helpers.mjs';
-import { correction, getBestMatches, LegacyQuotientSpur, models, LegacyQuotientRoot, TokenResultMapping } from '@keymanapp/lm-worker/test-index';
+import { correction, getBestTokenMatches, LegacyQuotientSpur, models, LegacyQuotientRoot, TokenResultMapping } from '@keymanapp/lm-worker/test-index';
 
 import TrieModel = models.TrieModel;
 
@@ -97,7 +97,7 @@ describe('getBestMatches', () => {
 
     const searchSpace = new LegacyQuotientRoot(testModel);
     const timer = buildTestTimer();
-    const iter = getBestMatches([searchSpace], timer);
+    const iter = getBestTokenMatches([searchSpace], timer);
 
     // While there's no input, insertion operations can produce suggestions.
     const resultState = await iter.next();
@@ -152,7 +152,7 @@ describe('getBestMatches', () => {
     assert.notEqual(searchPath2.spaceId, searchPath1.spaceId);
     assert.notEqual(searchPath3.spaceId, searchPath2.spaceId);
 
-    const iter = getBestMatches([searchPath3], buildTestTimer()); // disables the correction-search timeout.
+    const iter = getBestTokenMatches([searchPath3], buildTestTimer()); // disables the correction-search timeout.
     await checkRepeatableResults_teh(iter);
   });
 
@@ -186,12 +186,12 @@ describe('getBestMatches', () => {
     assert.notEqual(searchPath2.spaceId, searchPath1.spaceId);
     assert.notEqual(searchPath3.spaceId, searchPath2.spaceId);
 
-    const iter = getBestMatches([searchPath3], buildTestTimer()); // disables the correction-search timeout.
+    const iter = getBestTokenMatches([searchPath3], buildTestTimer()); // disables the correction-search timeout.
     await checkRepeatableResults_teh(iter);
 
     // The key: do we get the same results the second time?
     // Reset the iterator first...
-    const iter2 = getBestMatches([searchPath3], buildTestTimer()); // disables the correction-search timeout.
+    const iter2 = getBestTokenMatches([searchPath3], buildTestTimer()); // disables the correction-search timeout.
     await checkRepeatableResults_teh(iter2);
   });
 });