Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(web): extract the correct-and-raw-predict blocks into their own method 📚 #11888

Merged
merged 2 commits into from
Jul 25, 2024

Conversation

jahorton
Copy link
Contributor

@jahorton jahorton commented Jun 28, 2024

Continuing from #11899 (spun off from this one), this PR makes the correct-and-predict section its own method. Even on its own, the new method is pretty large, but at least its bounds are made distinct from the rest of the sections by doing so.

We can also shift to a simpler + clearer pattern for 12.0 models vs 14.0 models by returning early when the more complicated process isn't viable, which I believe does result better clarity here.

Note: reviewing with "ignore whitespace" appears to make the reviewing process much simpler for this PR.

@keymanapp-test-bot skip

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

keymanapp-test-bot bot commented Jun 28, 2024

User Test Results

Test specification and instructions

User tests are not required

@keymanapp-test-bot keymanapp-test-bot bot added this to the A18S5 milestone Jun 28, 2024
@mcdurdin mcdurdin changed the title refactor(web): refactors lm-worker predict into smaller methods refactor(web): refactor lm-worker predict() into smaller methods Jul 1, 2024
@github-actions github-actions bot added web/ and removed web/ labels Jul 2, 2024
@jahorton jahorton force-pushed the refactor/web/lm-worker-predict branch from 9d98a4a to b485b68 Compare July 2, 2024 02:56
@github-actions github-actions bot added web/ and removed web/ labels Jul 2, 2024
@jahorton jahorton changed the base branch from fix/web/low-probability-exact-matching to refactor/web/predict-suggestion-finalization July 2, 2024 02:58
@jahorton jahorton force-pushed the refactor/web/lm-worker-predict branch from b485b68 to f84c76c Compare July 2, 2024 03:41
@github-actions github-actions bot added web/ and removed web/ labels Jul 2, 2024
@jahorton jahorton changed the base branch from refactor/web/predict-suggestion-finalization to refactor/web/dedupe-and-similarity July 2, 2024 03:42
@jahorton jahorton changed the title refactor(web): refactor lm-worker predict() into smaller methods refactor(web): extract the correct-and-raw-predict blocks into their own method 📚 Jul 2, 2024
@keymanapp-test-bot keymanapp-test-bot bot removed the user-test-missing User tests have not yet been defined for the PR label Jul 2, 2024
@jahorton jahorton marked this pull request as ready for review July 2, 2024 03:50
@jahorton jahorton requested a review from mcdurdin as a code owner July 2, 2024 03:50
@darcywong00 darcywong00 modified the milestones: A18S5, A18S6 Jul 5, 2024
@darcywong00 darcywong00 modified the milestones: A18S6, A18S7 Jul 19, 2024
Copy link
Member

@mcdurdin mcdurdin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RSLGTM

@github-actions github-actions bot removed the web/ label Jul 25, 2024
@github-actions github-actions bot added the web/ label Jul 25, 2024
Base automatically changed from refactor/web/dedupe-and-similarity to master July 25, 2024 06:00
@jahorton jahorton merged commit 7839618 into master Jul 25, 2024
18 of 19 checks passed
@jahorton jahorton deleted the refactor/web/lm-worker-predict branch July 25, 2024 06:00
@keyman-server
Copy link
Collaborator

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

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.

4 participants