Skip to content

change(common/models): context tracking within the lm-worker 🐵#9855

Merged
jahorton merged 3 commits intofeature-gesturesfrom
change/common/models/pred-text-context-tracking
Nov 6, 2023
Merged

change(common/models): context tracking within the lm-worker 🐵#9855
jahorton merged 3 commits intofeature-gesturesfrom
change/common/models/pred-text-context-tracking

Conversation

@jahorton
Copy link
Copy Markdown
Contributor

This PR changes the underlying assumptions of the predictive-text worker, generalizing the cases it can handle.

In particular, the worker no longer assumes perfect continuity and synchronization of context with its KeymanWeb host. This is necessary due to interactions with multitaps, as those need "context rewind" operations that the worker is not privy to.

Many of the changes are due to significant instance reuse that was "fine" under the assumptions we held until now, but that are no longer viable. Thus, the new clone-constructors. We can still make significant re-use of previous calculations... but we need to make sure those calculations don't have unwanted cross-effects should an unexpected context-rewind occur.


In previous versions of Keyman and our predictive-text worker, we assumed that...

  1. We always had perfectly synchronized context between KeymanWeb's actual context and the worker's most-recently handled context.
  2. KeymanWeb could always signal the worker whenever a "context rewind" occurred.

Previously, step 2 above was only triggered by two causes:

  1. A predictive-text reversion was applied.
  2. KeymanWeb's context changed, requiring a full context dump.

The issue is: a multitap...

  1. Uses context rewinds.
  2. Does not use any sort of predictive-text reversion to do so.

Thus, supporting multitaps invalidates the assumptions at the top if things are left as they are... and my experience with testing #9740 shows that moderate multitap use will rapidly break predictive text if we don't correct these assumptions.

While it would be possible to instead update the worker on the results of a multitap... I personally find that path to be significantly less straightforward. (For one thing, reverting a suggestion always crossed 'context token' boundaries, thanks to whitespace. Not so with multitaps!) There's a significant chance said path would require new APIs into the worker, unlike this solution.

@keymanapp-test-bot skip

@jahorton jahorton requested a review from mcdurdin as a code owner October 26, 2023 02:46
@mcdurdin mcdurdin modified the milestones: A17S24, A17S25 Oct 27, 2023
…to change/common/models/pred-text-context-tracking
Base automatically changed from feat/web/test-kbd-prototyping to feature-gestures November 6, 2023 01:27
@jahorton jahorton merged commit a01a48f into feature-gestures Nov 6, 2023
@jahorton jahorton deleted the change/common/models/pred-text-context-tracking branch November 6, 2023 01:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants