feat(web): Review of predictive text code and tests 🚂

[markcsinclair]

Comments on study of predictive-text\worker-thread\src\main\correction prior to current changes (i.e. commit 69d1626).

[markcsinclair]

It would be good to have an SIL boilerplate header on all source files (@mcdurdin says not strictly necessary):

• classical-calculation.ts
• context-tracker.ts
• distance-modeler.ts
• execution-timer.ts
• index.ts
• segmentation-calculation.ts
• tokenization-subsets.ts

[markcsinclair]

Starting by looking at classical-calculation.ts.

Could we add a references to Hamming, Levenshtein and Damerau-Levenshtein in the references for ClassicalDistanceCalculation() in classical-calculation.ts? The Hamming distance isn't strictly needed, but some people will be familiar with this, and there is a progression of complexity from Hamming -> Levenshtein -> Damerau-Levenshtein.

[markcsinclair]

If possible, could we add Javadoc comments to some additional functions/classes, such as:

• getTrueIndex()
• getCostAt()
• initialCostAt()
• getSubset()
• addInputChar()
• _addInputChar()
• addMatchChar()
• _addMatchChar()
• increaseMaxDistance()
• propagateUpdateFrom()
• getTransposeParent()
• findTransposeEdges()
• EdgeFinder
• PathBuilder
  • addEdgeFinder()
  • backtracePath()

[markcsinclair]

As hasFinalCostWithin() is only used in tests, should it be in a source file?

Similarly, should visualizeCalculation() and the commented-out visualize() be in a source file? Perh aps they could be moved to a file in the tests folder?

[markcsinclair]

Can you give some extra explanation for validate the match? Is this just the check on line 974, or the input == match on 978?

[markcsinclair]

It would be good to see some unit tests on:

• findBaseEdges()
• findTransposeEdges()
• getSubset()
• _addInputChar()
• _addMatchChar()
• forNewIndices()
• increaseMaxDistance()
• propagateUpdateFrom()

[markcsinclair]

The code relating to 'merge' (here and here) seems unused.

Why is 'split' not one of the EditOperations?

I see that there is an ExtendedEditOperation in segmentable-calculation.ts, but this is not used in classical-calculation.ts.

[markcsinclair]

The local variable totalId seems potentially confusing because of the usual meaning of Id. Might something like totalInsertDelete be clearer?