refactor(common/models): move core trie-compilation code into common/models/templates 📖 #12083

jahorton · 2024-08-02T05:16:52Z

Note: this only moves Trie construction code, rather than TrieModel construction code.

Wordlist file loading, parsing, etc are still part of kmc-model.
This does not write the results out to a file or provide the TrieModel structure and additional parameters, such as punctuation, search term keying, etc.

In support of #10973.

At runtime - be it within the worker or within a separate process hosted by the mobile apps - we'll want a way to collate and/or compile user-specific words and names into a model that we can use for correction and prediction. The simplest such model is a wordlist... and we already know how to build those.

The key detail here is that we need Trie compilation code, at a minimum, to become common code. I believe we can no longer limit its use to within Developer.

@keymanapp-test-bot skip

keymanapp-test-bot · 2024-08-02T05:16:56Z

User Test Results

Test specification and instructions

User tests are not required

Test Artifacts

Android
Developer
iOS
- Keyman for iOS (simulator image)
- FirstVoices Keyboards for iOS (simulator image)
- TestFlight internal PR build version - 18.0.85 (0.12083.11714)
Keyboards
- Test Keyboards
Web
- KeymanWeb Test Home
Windows

jahorton · 2024-08-02T05:20:21Z

developer/src/kmc-model/src/build-trie.ts

  }
+  trieCollator.sort();


This still calls sortTrie, just via the object's member function. (See line 162 of trie-builder.ts.)

…on/models/templates We'll likely want to dynamically build a Trie to represent user-specific entries made available by the active OS. We'll then blend _that_ with the 'static' distributed model.

jahorton · 2024-08-02T05:58:08Z

developer/src/kmc-model/src/build-trie.ts

-   * Unicode will never assign semantics to these characters, as they are
-   * intended to be used internally as sentinel values.
-   */
-  const INTERNAL_VALUE = '\uFDD0';


This exactly matches the value of SENTINEL_CODE_UNIT in common/models/templates, so I replaced all migrated instances of it with SENTINEL_CODE_UNIT.

jahorton · 2024-08-02T06:04:45Z

common/models/templates/src/trie.ts

@@ -88,12 +98,26 @@ export class TrieTraversal implements LexiconTraversal {
    return traversal;
  }

+  private sortNodeIfNeeded(node: Node) {


I found this by referencing the original original implementation:

keyman/developer/src/kmc-model/src/build-trie.ts

Lines 207 to 208 in dd12e73

// The following trie implementation has been (heavily) derived from trie-ing

// by Conrad Irwin.

keyman/developer/src/kmc-model/src/build-trie.ts

Line 234 in dd12e73

// See: https://github.com/ConradIrwin/trie-ing/blob/df55d7af7068d357829db9e0a7faa8a38add1d1d/LICENSE

This may seem "pulled up a tad bit too early", but there's a reason. If we 'construct' the Trie while live, modifications can make part of the Trie unsorted. We want to track that and correct for that.

The followup then changes this block, integrating it within the Trie traversal structure. Pulling it in early like this allows for code comparison between the two approaches.

jahorton · 2024-08-02T06:34:54Z

Keyman Core test failures:

13:07:10   [developer/src/kmc-model] Skipping dependency common/models/templates for build
13:07:12   
13:07:12   > build
13:07:12   > tsc -b
13:07:12   
13:07:14   src/build-trie.ts(4,29): error TS2307: Cannot find module '@keymanapp/models-templates' or its corresponding type declarations.
13:07:14   npm error Lifecycle script `build` failed with error:
13:07:14   npm error code 1
13:07:14   npm error path /var/lib/TeamCity/work/99b311828f4ee7c/keyman/developer/src/kmc-model
13:07:14   npm error workspace @keymanapp/kmc-model
13:07:14   npm error location /var/lib/TeamCity/work/99b311828f4ee7c/keyman/developer/src/kmc-model
13:07:14   npm error command failed
13:07:14   npm error command sh -c tsc -b

Yeah, that tracks. I probably forgot to set a dependency link.

mcdurdin

One important dependency change required (otherwise I suspect this will break at runtime)

developer/src/kmc-model/build.sh

…to refactor/common/models/trie-construction

…mon/models/trie-construction

mcdurdin

LGTM

jahorton requested review from mcdurdin and darcywong00 as code owners August 2, 2024 05:16

keymanapp-test-bot bot added this to the A18S7 milestone Aug 2, 2024

github-actions bot added developer/ common/ common/models/ developer/compilers/ common/models/templates/ refactor labels Aug 2, 2024

jahorton commented Aug 2, 2024

View reviewed changes

jahorton marked this pull request as draft August 2, 2024 05:32

refactor(common/models): move primary trie-compilation code into comm…

54bdf73

…on/models/templates We'll likely want to dynamically build a Trie to represent user-specific entries made available by the active OS. We'll then blend _that_ with the 'static' distributed model.

jahorton mentioned this pull request Aug 2, 2024

refactor(developer/compilers): prepare for trie-compiler split 📖 #12084

Merged

jahorton force-pushed the refactor/common/models/trie-construction branch from f28f7b9 to 54bdf73 Compare August 2, 2024 05:54

jahorton changed the base branch from refactor/common/models/trie-model to refactor/developer/build-trie August 2, 2024 05:54

jahorton marked this pull request as ready for review August 2, 2024 05:57

jahorton commented Aug 2, 2024

View reviewed changes

fix(developer): add 'build' dependency link to models/templates

32a31bc

darcywong00 modified the milestones: A18S7, A18S8 Aug 2, 2024

mcdurdin requested changes Aug 5, 2024

View reviewed changes

developer/src/kmc-model/build.sh Show resolved Hide resolved

chore(common/models): Merge branch 'refactor/developer/build-trie' in…

c95b414

…to refactor/common/models/trie-construction

Base automatically changed from refactor/developer/build-trie to epic/user-dict August 9, 2024 02:23

keymanapp-test-bot bot added the epic-user-dict label Aug 9, 2024

chore(common/models): Merge branch 'epic/user-dict' into refactor/com…

a568ebb

…mon/models/trie-construction

jahorton requested a review from mcdurdin August 9, 2024 02:27

fix(developer): sets models-templates as package.json dependency

f6e87f8

mcdurdin approved these changes Aug 9, 2024

View reviewed changes

chore(common): update package-lock.json

894721d

jahorton merged commit 85e0157 into epic/user-dict Aug 12, 2024
14 of 15 checks passed

jahorton deleted the refactor/common/models/trie-construction branch August 12, 2024 02:08

jahorton mentioned this pull request Aug 23, 2024

chore(models/templates): seed epic/model-encoding with cherry-pick from epic/user-dict 💾 #12268

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(common/models): move core trie-compilation code into common/models/templates 📖 #12083

refactor(common/models): move core trie-compilation code into common/models/templates 📖 #12083

jahorton commented Aug 2, 2024 •

edited

Loading

keymanapp-test-bot bot commented Aug 2, 2024 •

edited

Loading

jahorton Aug 2, 2024

jahorton Aug 2, 2024

jahorton Aug 2, 2024

jahorton commented Aug 2, 2024

mcdurdin left a comment

mcdurdin left a comment

	// The following trie implementation has been (heavily) derived from trie-ing
	// by Conrad Irwin.

refactor(common/models): move core trie-compilation code into common/models/templates 📖 #12083

refactor(common/models): move core trie-compilation code into common/models/templates 📖 #12083

Conversation

jahorton commented Aug 2, 2024 • edited Loading

keymanapp-test-bot bot commented Aug 2, 2024 • edited Loading

User Test Results

Test Artifacts

jahorton Aug 2, 2024

Choose a reason for hiding this comment

jahorton Aug 2, 2024

Choose a reason for hiding this comment

jahorton Aug 2, 2024

Choose a reason for hiding this comment

jahorton commented Aug 2, 2024

mcdurdin left a comment

Choose a reason for hiding this comment

mcdurdin left a comment

Choose a reason for hiding this comment

jahorton commented Aug 2, 2024 •

edited

Loading

keymanapp-test-bot bot commented Aug 2, 2024 •

edited

Loading