Skip to content

refactor(web): extract buildAndMapPredictions as new prediction-helper 🚂#15781

Merged
jahorton merged 2 commits intoepic/autocorrectfrom
refactor/web/build-and-map-predictions
Apr 20, 2026
Merged

refactor(web): extract buildAndMapPredictions as new prediction-helper 🚂#15781
jahorton merged 2 commits intoepic/autocorrectfrom
refactor/web/build-and-map-predictions

Conversation

@jahorton
Copy link
Copy Markdown
Contributor

@jahorton jahorton commented Mar 25, 2026

While addressing support for corrections and predictions under whitespace fat-fingering scenarios, I came to recognize a helper that would be needed... that was previously written in two separate sections within the correctAndEnumerate helper function. Fortunately, the two sections are actually possible to condense into just one with a little effort, simplifying the code a bit in the process.

Build-bot: skip build:web
Test-bot: skip

@keymanapp-test-bot
Copy link
Copy Markdown

keymanapp-test-bot Bot commented Mar 25, 2026

User Test Results

Test specification and instructions

User tests are not required

@keymanapp-test-bot keymanapp-test-bot Bot changed the title refactor(web): extract buildAndMapPredictions as new prediction-helper refactor(web): extract buildAndMapPredictions as new prediction-helper 🚂 Mar 25, 2026
@keymanapp-test-bot keymanapp-test-bot Bot added this to the A19S25 milestone Mar 25, 2026
*/
rootCost *= ModelCompositor.SINGLE_CHAR_KEY_PROB_EXPONENT; // note the `Math.exp` below.
if(match.node.editCount > 0 && !searchModules.find(s => s.correctionsEnabled)) {
continue;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is all we need to enforce a "disabled corrections" state - we actually don't need an entirely separate control flow / code block.

@keyman-server keyman-server modified the milestones: A19S25, A19S26 Mar 28, 2026
@jahorton jahorton force-pushed the change/web/prep-suggestion-application branch 2 times, most recently from 3802382 to 0e33686 Compare April 9, 2026 20:47
@jahorton jahorton force-pushed the refactor/web/build-and-map-predictions branch from 7163830 to 04e6479 Compare April 9, 2026 20:58
@jahorton jahorton requested a review from ermshiperete April 9, 2026 21:06
@jahorton jahorton marked this pull request as ready for review April 9, 2026 21:06
@keyman-server keyman-server modified the milestones: A19S26, A19S27 Apr 14, 2026
Copy link
Copy Markdown
Contributor

@ermshiperete ermshiperete left a comment

Choose a reason for hiding this comment

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

LGTM

// Only set 'best correction' cost when a correction ACTUALLY YIELDS predictions.
if(predictions.length > 0 && bestCorrectionCost === undefined) {
bestCorrectionCost = rootCost;
bestCorrectionCost = predictions[0].correction.p;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

from devin.ai:

bestCorrectionCost stores a probability instead of a cost, breaking early search termination

The old code set bestCorrectionCost = rootCost where rootCost was match.totalCost (a log-space cost value, e.g. 0–20+). The new code sets bestCorrectionCost = predictions[0].correction.p, which is Math.exp(-rootCost) — a probability in the range (0, 1]. However, shouldStopSearchingEarly at predict-helpers.ts:594 compares this against match.totalCost (a cost) plus CORRECTION_SEARCH_THRESHOLDS (8 or 4). Since bestCorrectionCost is now always ≤ 1 instead of being a cost in the same domain, the effective stop threshold becomes ~8 regardless of the first match's actual cost. For example, when the first viable correction has cost 5, the old code would stop at cost 13 (5+8), but the new code stops at ~8.007 (exp(-5)+8), terminating the search much too early and potentially missing better predictions.

Suggested change
bestCorrectionCost = predictions[0].correction.p;
bestCorrectionCost = match.totalCost;

Base automatically changed from change/web/prep-suggestion-application to epic/autocorrect April 20, 2026 20:05
@jahorton jahorton merged commit 4be1643 into epic/autocorrect Apr 20, 2026
7 of 8 checks passed
@jahorton jahorton deleted the refactor/web/build-and-map-predictions branch April 20, 2026 20:05
@github-project-automation github-project-automation Bot moved this from Todo to Done in Keyman Apr 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants