change(web): stage construction of CorrectionPredictionTuple instances

[jahorton]

Build-bot: skip build:web
Test-bot: skip

[keymanapp-test-bot]

User Test Results

Test specification and instructions

User tests are not required

Test Artifacts

• Web
  • KeymanWeb Test Home

[mcdurdin]

There is a fair bit of stuff moving around here but I am not entirely sure why. Why did you extract matchLevel and preservationTransform into a separate PredictionMetadata interface? It seems like additional it adds extra verbosity to the code but the interface is not used outside that one place.

Why is CorrectionPredictionTuple separated from CorrectionPredictionTupleCore? Why can't metadata just be an optional property of CorrectionPredictionTuple as it was before? What is the benefit? Does this restructuring make the code more robust, or easier to understand, or faster?

(FWIW, we usually try to avoid using terms like Tuple because it doesn't add useful information about the type. CorrectionPredictionProbability seems closer? or SuggestionProbability? But I'm not entirely sure if those suggestions are right.)

Otherwise, the changes LGTM

[jahorton]

There is a fair bit of stuff moving around here but I am not entirely sure why. Why did you extract matchLevel and preservationTransform into a separate PredictionMetadata interface? It seems like additional it adds extra verbosity to the code but the interface is not used outside that one place.

For this PR, yes. The next PR - #16436 - will do a little more with the extracted type. Furthermore, epic/boundary-correction already has the type, which is part of why I decided to go ahead and leverage similar structure for this purpose.

Why is CorrectionPredictionTuple separated from CorrectionPredictionTupleCore? Why can't metadata just be an optional property of CorrectionPredictionTuple as it was before? What is the benefit? Does this restructuring make the code more robust, or easier to understand, or faster?

It becomes clearer where and when the .metadata property should and should not be optional. In particular, the inner-most core of prediction-generation doesn't have access to the values needed to populate .metadata, and now it is clearly a different code section's responsibility. (.matchLevel's value there is merely a default, to be overwritten later.) This will now be enforced by the TS type-inference engine.

New fields added to the .metadata type in the next PR - #16436 - similarly were only reasonable to initialize outside of that "inner-most core".

[keyman-server]

Changes in this pull request will be available for download in Keyman version 19.0.279-alpha