change(web): remove duplicate correction-search path filtering 🚂 #14996
+160
−70
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
github-actions
bot
added
web/
web/predictive-text/
change
jahorton
force-pushed
the
change/web/remove-duplicate-path-filtering
branch
from
8af2407 to
3f3382a
Compare
jahorton
force-pushed
the
refactor/web/search-batcher
branch
from
fac1c30 to
9920f5e
Compare
jahorton
force-pushed
the
change/web/remove-duplicate-path-filtering
branch
from
3f3382a to
1f3b029
Compare
jahorton
force-pushed
the
refactor/web/search-batcher
branch
from
9920f5e to
7e5d78c
Compare
jahorton
force-pushed
the
change/web/remove-duplicate-path-filtering
branch
from
1f3b029 to
9c197bd
Compare
jahorton
changed the base branch from
refactor/web/search-batcher
to
feat/web/search-space-identifiers
mcdurdin
approved these changes
Nov 15, 2025
Member
mcdurdin
left a comment
mcdurdin
left a comment
There was a problem hiding this comment.
Apart from the negatively-named variable, the rest looks fine. Approving with the inversion of the variable name please (and obviously fixing up usage)
| * Notes if the most recent edit processed in the node's represented search path | ||
| * was not a deletion. Root nodes are considered to meet this condition. | ||
| */ | ||
| readonly nonDeletion: boolean; |
Member
There was a problem hiding this comment.
Please avoid variables with negative names:
Suggested change
| readonly nonDeletion: boolean; | |
| readonly isDeletion: boolean; |
jahorton
force-pushed
the
feat/web/search-space-identifiers
branch
from
d2d5c67 to
43f5498
Compare
jahorton
force-pushed
the
change/web/remove-duplicate-path-filtering
branch
2 times, most recently
from
86fc4ef to
8043ef3
Compare
Upon investigation into the code being removed, this mostly triggered whenever an empty transform appeared in the input - during a context reset, at initialization, or when a deadkey is typed. (Deadkeys aren't sent to the pred-text engine.) This is generally not a common case within the engine, and there exists other filtering that helps prevent duplicating search results. Suppose a token that begins tagged with an empty transform. When the search performs an 'insertion' to better lookup words down a lexical path, the 'insertion' is identical whether before or after an empty transform. Similarly, insertions after a deletion appear no different than substitutions (or the same insertion before the deletion, then the deletion itself). The code likely didn't gain us much, as it likely carried some performance overhead - it built a large object that required memory and lookup time that involved constructing and processing strings for hashing. Build-bot: skip build:web Test-bot: skip
jahorton
force-pushed
the
feat/web/search-space-identifiers
branch
from
f26fd31 to
db77619
Compare
jahorton
force-pushed
the
change/web/remove-duplicate-path-filtering
branch
from
0bc1f96 to
4f98dbf
Compare
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.
Upon investigation into the code being removed, multiple paths to the same search correction mostly occur whenever an empty transform appeares in the input - during a context reset, at initialization, or when a deadkey is typed. (Deadkeys aren't sent to the pred-text engine.) This is generally not a common case within the engine, and even then, there exists other filtering to prevent displaying duplicated search results to our users.
Suppose a token that begins tagged with an empty transform. When the search performs an 'insertion' to better lookup words down a lexical path, the 'insertion' is identical whether before or after an empty transform. Similarly, insertions after a deletion appear no different than substitutions (or the same insertion before the deletion, then the deletion itself).
The prior anti-duplication code likely didn't gain us much, as it likely carried some performance overhead - it built a large object that required memory and lookup time that involved constructing and processing strings for hashing.
This change is not actually dependent on autocorrect and could easily be 🍒-picked to
masterif we so desire. It's here largely because the changes made here also helps with streamlining theSearchPath/SearchSpace/ etc rework happening on epic/autocorrect.Build-bot: skip build:web
Test-bot: skip