refactor(web): utilize new tokenization-transition methodology 🚂 #14877
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
User Test ResultsTest specification and instructions User tests are not required Test Artifacts |
keymanapp-test-bot
bot
changed the title
jahorton
force-pushed
the
feat/web/link-new-tokenization-transitioner
branch
from
04c3e63 to
e8583bc
Compare
jahorton
force-pushed
the
feat/web/evaluate-precomputed-tokenization
branch
from
fad631a to
68c8c0c
Compare
jahorton
force-pushed
the
feat/web/link-new-tokenization-transitioner
branch
from
870b785 to
a9b6970
Compare
jahorton
force-pushed
the
feat/web/evaluate-precomputed-tokenization
branch
from
68c8c0c to
7ca6f27
Compare
jahorton
changed the base branch from
feat/web/evaluate-precomputed-tokenization
to
feat/web/interim-legacy-keyer
jahorton
force-pushed
the
feat/web/link-new-tokenization-transitioner
branch
from
a9b6970 to
c96a604
Compare
jahorton
changed the title
Note that we still need to keep the previous tokenization method's alignment style around for now - it's referenced by quite a number of unit tests for suggestions and reversions, which are currently written to leverage that. Updating the related methods will be its own unit of work.
…ion-transition method
jahorton
force-pushed
the
feat/web/link-new-tokenization-transitioner
branch
from
c96a604 to
0cf7930
Compare
jahorton
commented
Oct 7, 2025
Comment on lines
278
to
288
| // We actually will want to build `preservationTransform`s based on the path | ||
| // leading to each correction/suggestion. But, until now, we've just built | ||
| // it based upon the actual input transform - so we'll maintain (temporarily) | ||
| // as a transitional state. | ||
|
|
||
| const bestResultAnalysis = tokenizationAnalysis; | ||
| // inputTransform is the ideal transform we found. | ||
|
|
||
| // If tokens were inserted, emit an empty transform; this prevents | ||
| // suggestions from replacing the "current" token. | ||
| if(bestResultAnalysis.inputs[0].sample.has(1)) { |
Contributor
Author
There was a problem hiding this comment.
Self note: this section could use a rework and reword.
Base automatically changed from
feat/web/interim-legacy-keyer
to
epic/autocorrect
October 16, 2025 14:49
jahorton
changed the base branch from
epic/autocorrect
to
fix/web/base-context-state-validation
…nk-new-tokenization-transitioner
Base automatically changed from
fix/web/base-context-state-validation
to
epic/autocorrect
October 16, 2025 15:36
ermshiperete
approved these changes
Oct 17, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We're now at a place where we can reasonably integrate all the new parts designed to help with token-merge and token-split scenarios. By swapping our tokenization strategy to the one introduced over the last several PRs, we gain the ability to handle such cases when assuming that there is only one true tokenization. (Handling for cases where we drop that assumption will come later - see #14970 for the start of multi-tokenization support.)
Note that we still need to keep the previous tokenization method's alignment style around for a PR - it's referenced by quite a number of unit tests for suggestions and reversions, which are currently written to leverage that. Updating the related methods will be its own unit of work... but fortunately, we can "frankenstein" the two parts together for now until those changes are made.
That said, all unit tests are passing with the new tokenization internals in place - I consider this a good sign! Just gotta update suggestions and reversions to better leverage the data too.
The user tests will come in #14880, two PRs after this one.
Build-bot: skip release:web
Test-bot: skip