feat(web): check for low-probability exact + exact-key correction matches 📚 #11876

jahorton · 2024-06-26T08:39:40Z

One long-time pet peeve of mine when it comes to auto-correct: it should never correct away from a perfectly-valid word of the language. Even if it's is a more common word in English than its, I believe that its should be left in place. (I find it quite the pain on iOS, as I've had to fight iOS to leave its before.)

Even if we were to auto-correct away, we should ensure that its is, at least, easily accessible on the banner as an option so that a user may at least prevent auto-correction that way. (Admittedly, iOS does present it... I probably need to adapt.) That is... we need to detect exact matches + exact-key matches and ensure they're always visible, with priority. This is something our engine currently doesn't do well, but thanks to #11869, we now have the perfect tool to remedy the issue.

Secondly... the ModelCompositor.predict method is long and rather "monolithic". It'd be nice to spin off as much "keep"-related handling as possible into its own method.

Unit tests for the new functionality will be added with #11944. Ideally, this PR should not be merged until all PRs in the sequence up to and including #11944 are ready to go.

@keymanapp-test-bot skip

keymanapp-test-bot · 2024-06-26T08:42:05Z

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.60 (0.11876.11423)
Keyboards
- Test Keyboards
Web
- KeymanWeb Test Home
Windows

…refix for Trie models

…ches

…g, not displayAs

mcdurdin · 2024-07-05T07:23:51Z

One long-time pet peeve of mine when it comes to auto-correct: it should never correct away from a perfectly-valid word of the language. Even if it's is a more common word in English than its, I believe that its should be left in place. (I find it quite the pain on iOS, as I've had to fight iOS to leave its before.)

I am tempted to make you feel better by saying "there, their, they're"...

Interestingly, I've seen other users who love this. its vs it's is impossible to get right without some grammatical awareness of course... but in the more general case, I wonder if this should be surfaced as an option for users?

jahorton · 2024-07-05T08:15:06Z

One long-time pet peeve of mine when it comes to auto-correct: it should never correct away from a perfectly-valid word of the language. Even if it's is a more common word in English than its, I believe that its should be left in place. (I find it quite the pain on iOS, as I've had to fight iOS to leave its before.)

I am tempted to make you feel better by saying "there, their, they're"...

Interestingly, I've seen other users who love this. its vs it's is impossible to get right without some grammatical awareness of course... but in the more general case, I wonder if this should be surfaced as an option for users?

I was actually thinking something similar, making it a configurable option.

mcdurdin · 2024-07-05T08:20:13Z

I was actually thinking something similar, making it a configurable option.

feat(ios,android,web): add option to allow auto-correction away from valid words in predictive text #11931

mcdurdin

I think this LGTM; I haven't reviewed the changes to the tests in detail as my flight is about to start descent and I wanted to get this in before that!

mcdurdin · 2024-07-05T08:21:30Z

common/models/templates/src/trie-model.ts

+    }
+
+    const directEntries = rootTraversal.entries;
+    // `Set` requires Chrome 38+, which is more recent than Chrome 35.


If we are moving away from ES5 (#11881), does that bump our minimum version of Chrome?

It does. This PR's main content was written months ago, before we were looking at directly dropping it. #11881 isn't yet merged, either.

common/models/templates/src/trie-model.ts

common/web/lm-worker/src/main/model-compositor.ts

mcdurdin · 2024-07-05T08:28:58Z

common/web/lm-worker/src/main/model-compositor.ts

+      if(keyed(tuple.correction.sample) == keyedPrefix) {
+        if(predictedWord == truePrefix) {
+          tuple.matchLevel = SuggestionSimilarity.exact;
+          keepOption = this.toAnnotatedSuggestion(tuple.prediction.sample, 'keep',  models.QuoteBehavior.noQuotes);


It would be good to add a comment for this and the next 3 lines of code because it's not entirely obvious what's happening here, even with the comments on ll.426-428

common/web/lm-worker/src/main/model-compositor.ts

…-through-traversal' into fix/web/low-probability-exact-matching

jahorton · 2024-07-09T03:25:01Z

Noticed this while working on focused unit tests for the followup to #11940: even with this PR in place, we're not currently auto-selecting something like can't with priority when the current context is cant - where there's no exact context match, but there is an exact-key match. I'll want to fix that, whether it be within this PR or within a descendant.

The probability-ratio requirement should probably only apply to suggestions within the same "similarity tier". If it's a lower tier, we should probably straight-up ignore its probability component for the sum used in the thresholding ratio.

keyman-server · 2024-07-25T18:05:17Z

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

github-actions bot added common/ common/web/ feat web/ labels Jun 26, 2024

keymanapp-test-bot bot added the user-test-missing User tests have not yet been defined for the PR label Jun 26, 2024

keymanapp-test-bot bot added this to the A18S5 milestone Jun 26, 2024

jahorton force-pushed the feat/web/auto-prediction branch from 3632a12 to 7779125 Compare June 27, 2024 02:52

jahorton mentioned this pull request Jun 27, 2024

feat: 17.0 staging branch keymanapp/lexical-models#227

Merged

jahorton added 3 commits June 28, 2024 12:51

chore(web): update mtnt manual-text fixture

4822b27

change(common/models): directly return predictions that match keyed p…

b473ada

…refix for Trie models

feat(web): check for low-probability exact + exact-key correction mat…

0e8816c

…ches

jahorton force-pushed the fix/web/low-probability-exact-matching branch from a1ed21f to c6dd3bf Compare June 28, 2024 05:52

github-actions bot added common/models/ common/models/templates/ labels Jun 28, 2024

change(web): perform suggestion dupe-detection based on applied strin…

ae02e7d

…g, not displayAs

jahorton force-pushed the fix/web/low-probability-exact-matching branch from c6dd3bf to ae02e7d Compare June 28, 2024 07:13

jahorton mentioned this pull request Jul 2, 2024

refactor(web): extract suggestion-finalization block into its own function 📚 #11899

Merged

jahorton marked this pull request as ready for review July 5, 2024 08:15

jahorton requested review from ermshiperete and mcdurdin as code owners July 5, 2024 08:15

mcdurdin mentioned this pull request Jul 5, 2024

feat(ios,android,web): add option to allow auto-correction away from valid words in predictive text #11931

Open

mcdurdin approved these changes Jul 5, 2024

View reviewed changes

darcywong00 modified the milestones: A18S5, A18S6 Jul 5, 2024

jahorton added 2 commits July 8, 2024 10:29

chore(web): Merge branch 'change/common/models/templates/trie-results…

067d033

…-through-traversal' into fix/web/low-probability-exact-matching

chore(web): incorporate suggestions, address concerns from PR review

946ec38

github-actions bot added the common/models/types/ label Jul 8, 2024

Base automatically changed from feat/web/auto-prediction to master July 8, 2024 07:37

jahorton mentioned this pull request Jul 9, 2024

feat(web): add unit tests for predict auto-selection method 📚 #11941

Merged

darcywong00 modified the milestones: A18S6, A18S7 Jul 19, 2024

keymanapp-test-bot bot removed the user-test-missing User tests have not yet been defined for the PR label Jul 23, 2024

jahorton merged commit 980ed88 into master Jul 25, 2024
18 of 19 checks passed

jahorton deleted the fix/web/low-probability-exact-matching branch July 25, 2024 03:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(web): check for low-probability exact + exact-key correction matches 📚 #11876

feat(web): check for low-probability exact + exact-key correction matches 📚 #11876

jahorton commented Jun 26, 2024 •

edited

Loading

keymanapp-test-bot bot commented Jun 26, 2024 •

edited

Loading

mcdurdin commented Jul 5, 2024

jahorton commented Jul 5, 2024

mcdurdin commented Jul 5, 2024

mcdurdin left a comment

mcdurdin Jul 5, 2024

jahorton Jul 8, 2024

mcdurdin Jul 5, 2024

jahorton commented Jul 9, 2024

keyman-server commented Jul 25, 2024

feat(web): check for low-probability exact + exact-key correction matches 📚 #11876

feat(web): check for low-probability exact + exact-key correction matches 📚 #11876

Conversation

jahorton commented Jun 26, 2024 • edited Loading

keymanapp-test-bot bot commented Jun 26, 2024 • edited Loading

User Test Results

Test Artifacts

mcdurdin commented Jul 5, 2024

jahorton commented Jul 5, 2024

mcdurdin commented Jul 5, 2024

mcdurdin left a comment

Choose a reason for hiding this comment

mcdurdin Jul 5, 2024

Choose a reason for hiding this comment

jahorton Jul 8, 2024

Choose a reason for hiding this comment

mcdurdin Jul 5, 2024

Choose a reason for hiding this comment

jahorton commented Jul 9, 2024

keyman-server commented Jul 25, 2024

jahorton commented Jun 26, 2024 •

edited

Loading

keymanapp-test-bot bot commented Jun 26, 2024 •

edited

Loading