From e01d424079df4972f14d5b84de82bd3b07817641 Mon Sep 17 00:00:00 2001 From: Joshua Horton Date: Fri, 18 Sep 2026 15:49:14 -0500 Subject: [PATCH] fix(web): add "result sanitizing" wrapper for custom wordbreakers Fixes: #16587 Fixes: #16585 Build-bot: skip release:web,android,ios --- common/web/types/src/lexical-model-types.ts | 3 +- .../wordbreakers/src/main/default/index.ts | 2 +- .../wordbreakers/src/main/index.ts | 15 +- .../wordbreakers/src/main/sanitize-results.ts | 110 ++++++++++ .../worker-thread/src/main/model-helpers.ts | 4 +- .../wordbreakers/sanitize-results.tests.ts | 192 ++++++++++++++++++ 6 files changed, 320 insertions(+), 6 deletions(-) create mode 100644 web/src/engine/predictive-text/wordbreakers/src/main/sanitize-results.ts create mode 100644 web/src/test/auto/headless/engine/predictive-text/wordbreakers/sanitize-results.tests.ts diff --git a/common/web/types/src/lexical-model-types.ts b/common/web/types/src/lexical-model-types.ts index a3c8df1c98b..5939005308b 100644 --- a/common/web/types/src/lexical-model-types.ts +++ b/common/web/types/src/lexical-model-types.ts @@ -572,9 +572,10 @@ export interface Configuration { * phrase, each span which representing a word. */ export interface WordBreakingFunction { - // invariant: span[i].end <= span[i + 1].start + // invariant: span[i].end = span[i + 1].start // invariant: for all span[i] and span[i + 1], there does not exist a span[k] // where span[i].end <= span[k].start AND span[k].end <= span[i + 1].start + // except for a context-final empty token. (phrase: string): Span[]; } diff --git a/web/src/engine/predictive-text/wordbreakers/src/main/default/index.ts b/web/src/engine/predictive-text/wordbreakers/src/main/default/index.ts index eb49219d9ee..8fdc4324775 100644 --- a/web/src/engine/predictive-text/wordbreakers/src/main/default/index.ts +++ b/web/src/engine/predictive-text/wordbreakers/src/main/default/index.ts @@ -77,7 +77,7 @@ export default def; /** * A span that does not cut out the substring until it absolutely has to! */ -class LazySpan implements LexicalModelTypes.Span { +export class LazySpan implements LexicalModelTypes.Span { private _source: string; readonly start: number; readonly end: number; diff --git a/web/src/engine/predictive-text/wordbreakers/src/main/index.ts b/web/src/engine/predictive-text/wordbreakers/src/main/index.ts index da5e7abd2a0..abc890c2786 100644 --- a/web/src/engine/predictive-text/wordbreakers/src/main/index.ts +++ b/web/src/engine/predictive-text/wordbreakers/src/main/index.ts @@ -1,8 +1,19 @@ import { placeholder } from "./placeholder.js"; import { ascii } from "./ascii.js"; -import { default_ } from "./default/index.js"; +import { default_, LazySpan } from "./default/index.js"; import { WordBreakProperty } from "./default/data.inc.js"; import { searchForProperty } from "./default/searchForProperty.js"; +import { sanitizeResults } from "./sanitize-results.js"; + +export { + placeholder, + ascii, + default_ as default, + default_ as defaultWordbreaker, + LazySpan, + sanitizeResults, + searchForProperty, + WordBreakProperty +}; -export { placeholder, ascii, default_ as default, default_ as defaultWordbreaker, searchForProperty, WordBreakProperty }; export { type BreakerContext } from "./default/index.js"; diff --git a/web/src/engine/predictive-text/wordbreakers/src/main/sanitize-results.ts b/web/src/engine/predictive-text/wordbreakers/src/main/sanitize-results.ts new file mode 100644 index 00000000000..4019493f5c3 --- /dev/null +++ b/web/src/engine/predictive-text/wordbreakers/src/main/sanitize-results.ts @@ -0,0 +1,110 @@ +/* + * Keyman is copyright (C) SIL Global. MIT License. + * + * Created by jahorton on 2026-09-18 + * + * This file defines a wordbreaker wrapper that corrects custom wordbreaker + * whitespace handling and span indexing issues. + */ + +import { LexicalModelTypes } from '@keymanapp/common-types'; + +import Span = LexicalModelTypes.Span; +import WordBreakingFunction = LexicalModelTypes.WordBreakingFunction; + +/** + * Acts as a wrapper that helps the provided wordbreaker report tokens for _all_ + * text in range, not just the parts that aren't whitespace. + * + * @param phrase + */ +export function sanitizeResults(breaker: WordBreakingFunction): WordBreakingFunction { + const wrappedBreaker: WordBreakingFunction = (str) => { + const sourceSpans = breaker(str); + const finalSpans: Span[] = []; + let currentIndex = 0; + + // Note: spans are based on code units, not code points. + while(currentIndex <= str.length) { + let nextSpan = sourceSpans[0]; + + // If we're out of spans, then all remaining text must be whitespace. We don't + // return that, but we do return one final empty-token Span + if(!nextSpan) { + const missingFinalSpan: Span = { + start: str.length, + end: str.length, + length: 0, + text: '' + }; + finalSpans.push(missingFinalSpan); + + break; + } else { + // ait.mnw.mon's custom breaker does not specify the length property! + if(nextSpan.length === undefined) { + nextSpan = { + ...nextSpan, + length: nextSpan.end - nextSpan.start + }; + } + } + + // Remove any context-starting & non-final empty spans from the detected-spans list. + // Empty contexts should still report one of them! + if( + // If it's an empty span at the start of context... + (nextSpan.length == 0 && nextSpan.start == 0) && + // and there are more remaining spans (or text that results in a new span) + (sourceSpans.length > 1 || str.length > 0) + ) { + // then remove the empty span. + sourceSpans.shift(); + continue; + } + + // Naive approaches, like with ait.mnw.mon, may not properly index the spans at all times! + // We can improve the indexing as follows. + if(nextSpan.start < currentIndex) { + const trueStart = str.indexOf(nextSpan.text, currentIndex); + + const replacementSpan: Span = { + start: trueStart, + end: trueStart + nextSpan.length, + length: nextSpan.length, + text: nextSpan.text + }; + + sourceSpans[0] = replacementSpan; + nextSpan = replacementSpan; + } + + // The easy case: a span already exists! Easy mode! + if(nextSpan.start == currentIndex) { + sourceSpans.shift(); + finalSpans.push(nextSpan); + currentIndex = nextSpan.end; + + // If the span's end is located at the end of the string, we can terminate now. + if(nextSpan.end == str.length) { + break; + } + continue; + } + + // Handling intermediate missing spans + // const missingSpan: Span = { + // start: currentIndex, + // end: nextSpan.start, + // length: nextSpan.start - currentIndex, + // text: str.substring(currentIndex, nextSpan.start) + // }; + // finalSpans.push(missingSpan); + currentIndex = nextSpan.start; + } + + return finalSpans; + }; + + return wrappedBreaker; +} diff --git a/web/src/engine/predictive-text/worker-thread/src/main/model-helpers.ts b/web/src/engine/predictive-text/worker-thread/src/main/model-helpers.ts index 7008a74be55..3e3a2c9e5e0 100644 --- a/web/src/engine/predictive-text/worker-thread/src/main/model-helpers.ts +++ b/web/src/engine/predictive-text/worker-thread/src/main/model-helpers.ts @@ -52,7 +52,7 @@ export function determineModelWordbreaker(model: LexicalModel): (context: Contex // We're either relying on defaults or on the 14.0+ wordbreaker spec. let wordbreaker = model.wordbreaker || wordBreakers.default; - return models.wordbreak(wordbreaker, context); + return models.wordbreak(wordBreakers.sanitizeResults(wordbreaker), context); /* c8 ignore start */ } else { // 1. This model does not provide a model following the 14.0+ wordbreaking spec @@ -69,7 +69,7 @@ export function determineModelWordbreaker(model: LexicalModel): (context: Contex export function determineModelTokenizer(model: LexicalModel) { return (context: Context) => { if(model.wordbreaker) { - return models.tokenize(model.wordbreaker, context); + return models.tokenize(wordBreakers.sanitizeResults(model.wordbreaker), context); } else { return null; } diff --git a/web/src/test/auto/headless/engine/predictive-text/wordbreakers/sanitize-results.tests.ts b/web/src/test/auto/headless/engine/predictive-text/wordbreakers/sanitize-results.tests.ts new file mode 100644 index 00000000000..e076183fc78 --- /dev/null +++ b/web/src/test/auto/headless/engine/predictive-text/wordbreakers/sanitize-results.tests.ts @@ -0,0 +1,192 @@ +/* + * Keyman is copyright (C) SIL Global. MIT License. + * + * Created by jahorton on 2026-09-18 + * + * This file defines tests for our backward-compatibility wrapper for custom + * model wordbreakers that don't properly handle/report whitespace tokens and/or + * don't robustly mark the start of their spans correctly. + */ + +import { assert } from 'chai'; + +import { LexicalModelTypes } from '@keymanapp/common-types'; +import { defaultWordbreaker, LazySpan, sanitizeResults } from '@keymanapp/models-wordbreakers'; + +import Span = LexicalModelTypes.Span; +import WordBreakingFunction = LexicalModelTypes.WordBreakingFunction; + +// Taken from `ait.mnw.mon` v1.1 +const simpleNaiveCustomBreaker: WordBreakingFunction = (str: string) => { + return str.split(/\s|\u200b/).map(function(token) { + // Is actually missing the 'length' portion! + return { + left: str.indexOf(token), + start: str.indexOf(token), + right: str.indexOf(token) + token.length, + end: str.indexOf(token) + token.length, + text: token + } as undefined as Span; + }); +}; + +// Taken from `sil.km.gcc` v2.0 +const extendedCustomBreaker: WordBreakingFunction = (str) => { + const whitespaceRegex = /\s|\u200b|\n|\r/; + const tokens = str.split(whitespaceRegex); + + for(let i=0; i < tokens.length; i++) { + const token = tokens[i]; + if(token.length == 0) { + tokens.splice(i, 1); + i--; + continue; + } else if(token.length == 1 && whitespaceRegex.test(token)) { + tokens.splice(i, 1); + i--; + continue; + } + + // Certain punctuation marks should be considered a separate token from the word they're next to. + const punctuationMarks = ['«', '»', '$', '#' /* add extras here */]; + const punctSplitIndices = []; + + // Find if and where each mark exists within the token + for(let i = 0; i < punctuationMarks.length; i++) { + const split = token.indexOf(punctuationMarks[i]); + if(split >= 0) { + punctSplitIndices.push(split); + } + } + + // Sort and pick the earliest mark's location. If none exists, use -1. + punctSplitIndices.sort(); + const splitPoint = punctSplitIndices[0] === undefined ? -1 : punctSplitIndices[0]; + + if(splitPoint > -1) { + const left = token.substring(0, splitPoint); // (0, -1) => '' + const punct = token.substring(splitPoint, splitPoint+1); + const right = token.substring(splitPoint+1); // Starting past the end of the string => '' + + if(left) { + tokens.splice(i++, 0, left); + } + tokens.splice(i++, 1, punct); + if(right) { + tokens.splice(i, 0, right); + } + // Ensure that the next iteration puts `i` immediately after the punctuation token... even if + // there was a `right` portion, as it may have extra marks that also need to be spun off. + i--; + } + } + + let latestIndex = 0; + return tokens.map(function(token) { + const start = str.indexOf(token, latestIndex); + latestIndex = start + token.length; + return { + left: start, + start: start, + right: start + token.length, + end: start + token.length, + length: token.length, + text: token + } + }); +}; + +const u = (code: number) => String.fromCodePoint(code); + +// Uses both BMP & non-BMP chars that essentially spell out "apple". +const mixedPlaneApple = 'a' + u(0x1d5c9) + 'p' + 'l' + u(0x1d5be); + +describe('Custom wordbreaker whitespace restoration', function () { + describe('wrapping the Unicode default wordbreaker', () => { + const breaker = sanitizeResults(defaultWordbreaker); + + this.beforeAll(() => { + assert.isOk(breaker); + }); + + it('properly breaks a context with two spaces', () => { + const context = ' '; + const spans = breaker(context); + assert.deepEqual(spans.map((s) => new LazySpan(context, s.start, s.end)), defaultWordbreaker(context)); + assert.deepEqual(spans.map((s) => s.text), ['']); + }); + + it('properly breaks a context with text tokens and a context-final space', () => { + const context = ' apple '; + const spans = breaker(context); + assert.deepEqual(spans.map((s) => new LazySpan(context, s.start, s.end)), defaultWordbreaker(context)); + assert.deepEqual(spans.map((s) => s.text), ['apple', '']); + }); + + it('properly handles tokens with non-BMP text', () => { + const context = ` ${mixedPlaneApple} `; + const spans = breaker(context); + assert.deepEqual(spans.map((s) => new LazySpan(context, s.start, s.end)), defaultWordbreaker(context)); + assert.deepEqual(spans.map((s) => s.text), [mixedPlaneApple, '']); + }); + }); + + describe('ait.mnw.mon 1.1', () => { + const breaker = sanitizeResults(simpleNaiveCustomBreaker); + + this.beforeAll(() => { + assert.isOk(breaker); + }); + + it('properly breaks a context with two spaces', () => { + const spans = breaker(' '); + assert.deepEqual(spans.map((s) => s.text), ['']); + assert.deepEqual(spans.map((s) => s.start), [' '.length]); + }); + + it('properly breaks a context with text tokens and a context-final space', () => { + const spans = breaker(' ကခဗံၚ် '); + assert.deepEqual(spans.map((s) => s.text), ['ကခဗံၚ်', '']); + assert.deepEqual(spans.map((s) => s.start), [' '.length, ' ကခဗံၚ် '.length]); + }); + + it('properly breaks a context with a single-char final token', () => { + const spans = breaker('ကၚ လိက်အုပ် အ'); + assert.deepEqual(spans.map((s) => s.text), ['ကၚ', 'လိက်အုပ်', 'အ']); + assert.deepEqual(spans.map((s) => s.start), [''.length, 'ကၚ '.length, 'ကၚ လိက်အုပ် '.length]); + }); + + it('properly handles tokens with non-BMP text', () => { + const spans = breaker(` ${mixedPlaneApple} `); + assert.deepEqual(spans.map((s) => s.text), [mixedPlaneApple, '']); + assert.deepEqual(spans.map((s) => s.start), [' '.length, ` ${mixedPlaneApple} `.length]); + }); + }); + + describe('sil.km.gcc 2.0', () => { + const breaker = sanitizeResults(extendedCustomBreaker); + + this.beforeAll(() => { + assert.isOk(breaker); + }); + + it('properly breaks a context with two spaces', () => { + const spans = breaker(' '); + assert.deepEqual(spans.map((s) => s.text), ['']); + assert.deepEqual(spans.map((s) => s.start), [' '.length]); + + }); + + it('properly breaks a context with text tokens and a context-final space', () => { + const spans = breaker(' ការ '); + assert.deepEqual(spans.map((s) => s.text), ['ការ', '']); + assert.deepEqual(spans.map((s) => s.start), [' '.length, ' ការ '.length]); + }); + + it('properly handles tokens with non-BMP text', () => { + const spans = breaker(` ${mixedPlaneApple} `); + assert.deepEqual(spans.map((s) => s.text), [mixedPlaneApple, '']); + assert.deepEqual(spans.map((s) => s.start), [' '.length, ` ${mixedPlaneApple} `.length]); + }); + }); +});