From 4c78c01a10483d907e8d08204a57748a38109533 Mon Sep 17 00:00:00 2001 From: HugoFara Date: Sun, 23 Aug 2026 13:28:21 +0200 Subject: [PATCH 1/2] fix(language): make the parser setting real, and Chinese use jieba (#278) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Chinese language created from the built-in preset produced a text with no clickable words at all: sentences=1, words=0. Nothing to look up, nothing to track. Three separate gaps stacked up to that. The parser setting was decorative. LgParserType was written by the language form and read by nothing — TextParsing::tokenize() branched only on the legacy MECAB magic word and otherwise went straight to StandardTextParser, so setting a language to jieba, mecab or even a bogus value parsed identically. The whole ParserRegistry existed only to populate the dropdown. It is now consulted before the built-in pipeline, and its tokens are adapted to ParsedToken; the two shapes differ only in sentence numbering and token ordering. External parsers never reached the dropdown either. ParserRegistry's constructor took an optional loader, and both direct call sites passed none, so registerExternalParsers() returned immediately: jieba was installed in the Docker image, working, and unlistable. It now builds a loader when given none. The presets could not express a parser. langdefs.json has carried parserType for the CJK languages all along, but LanguagePresets flattened it into an eight-slot tuple that dropped the field — which is why the Japanese preset, declaring mecab, silently resolved to character parsing. The tuple carries it as slot 8, the API exposes it, and both preset appliers set it. Chinese (Simplified) and (Traditional) now ask for jieba, keeping makeCharacterWord so the fallback stays meaningful. Opting in is deliberate and the fallback is safe. Only an explicit, non-default LgParserType routes to the registry; the legacy signals it also understands are ignored, so every language that exists today — all of which store no parser type — parses byte-identically. A parser the server cannot run falls back to the built-in pipeline, never to the regex parser, which for a CJK language yields zero words. The form gained an "Automatic" option so that "infer from the flags" stays expressible. Verified end to end against a real database with jieba installed: preset (jieba) LgParserType='jieba' sentences=2 words=7 no parser type LgParserType=NULL sentences=2 words=13 unavailable parser LgParserType='sudachi' sentences=2 words=13 where 7 is jieba word segmentation and 13 is one token per character. --- CHANGELOG.md | 16 +++ locale/de/language.json | 1 + locale/en/language.json | 1 + locale/es/language.json | 1 + locale/fr/language.json | 1 + locale/it/language.json | 1 + locale/ja/language.json | 1 + locale/pt/language.json | 1 + locale/ru/language.json | 1 + locale/zh/language.json | 1 + .../Admin/Application/Services/TtsService.php | 6 +- .../Language/Application/LanguageFacade.php | 2 +- .../Application/UseCases/GetLanguageCode.php | 2 +- .../Language/Http/LanguageApiHandler.php | 3 +- .../Infrastructure/Data/langdefs.json | 4 +- .../Infrastructure/Parser/ParserRegistry.php | 43 +++++++- src/Modules/Language/Views/form.php | 1 + .../Infrastructure/Database/TextParsing.php | 99 ++++++++++++++++++- .../Language/LanguagePresets.php | 20 ++-- .../js/modules/language/api/languages_api.ts | 2 + .../modules/language/pages/language_form.ts | 15 ++- .../language/stores/language_form_store.ts | 4 + .../Parser/ParserRegistryTest.php | 90 +++++++++++++++++ .../LanguagePresetsParserTypeTest.php | 67 +++++++++++++ 24 files changed, 361 insertions(+), 22 deletions(-) create mode 100644 tests/backend/Shared/Infrastructure/Language/LanguagePresetsParserTypeTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index c944083a5..f4459be7e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -86,6 +86,22 @@ ones are marked like "v1.0.0-fork". unchanged; the work behind them no longer grows with the size of your vocabulary. +* **Chinese texts could not be read** (#278). A Chinese language created from + the built-in preset produced a text with no clickable words at all — nothing + to look up, nothing to track. Chinese and Japanese now come out of the preset + already pointed at a real tokenizer: **jieba** segments Chinese into words + rather than characters, and Japanese asks for MeCab as it always claimed to. + Where that tokenizer is not installed the language falls back to + character-by-character parsing, so it stays readable either way. + +* **Choosing a parser in the language form did nothing** (#278). The Parser + Type menu wrote its value to the database and no part of the parsing pipeline + ever read it — picking "Jieba (Chinese)" or "MeCab Python" parsed exactly as + before. The setting is now honoured. It also lists the parsers from + `config/parsers.php`, which it never did, so jieba and MeCab Python appear on + an install that has them. A language that names no parser — every language + that exists today — parses exactly as it did. + ## [3.4.2-fork] - 2026-08-16 ### Fixed diff --git a/locale/de/language.json b/locale/de/language.json index 1e29ed2f4..2262f31a9 100644 --- a/locale/de/language.json +++ b/locale/de/language.json @@ -79,6 +79,7 @@ "form.text_size_example": "Der Text wird diese Größe haben", "form.section_text_processing": "Textverarbeitung", "form.parser_type": "Parser-Typ", + "form.parser_automatic": "Automatisch", "form.parser_unavailable": " (nicht verfügbar)", "form.character_substitutions": "Zeichenersetzungen", "form.character_substitutions_help": "Zeichen vor dem Parsen ersetzen (Format: von=zu, getrennt durch |)", diff --git a/locale/en/language.json b/locale/en/language.json index 60aa10df7..a0a0719b5 100644 --- a/locale/en/language.json +++ b/locale/en/language.json @@ -79,6 +79,7 @@ "form.text_size_example": "Text will be this size", "form.section_text_processing": "Text Processing", "form.parser_type": "Parser Type", + "form.parser_automatic": "Automatic", "form.parser_unavailable": " (unavailable)", "form.character_substitutions": "Character Substitutions", "form.character_substitutions_help": "Replace characters before parsing (format: from=to, separated by |)", diff --git a/locale/es/language.json b/locale/es/language.json index 636397649..64f99c57a 100644 --- a/locale/es/language.json +++ b/locale/es/language.json @@ -79,6 +79,7 @@ "form.text_size_example": "El texto tendrá este tamaño", "form.section_text_processing": "Procesamiento de texto", "form.parser_type": "Tipo de analizador", + "form.parser_automatic": "Automático", "form.parser_unavailable": " (no disponible)", "form.character_substitutions": "Sustituciones de caracteres", "form.character_substitutions_help": "Reemplaza caracteres antes del análisis (formato: de=a, separados por |)", diff --git a/locale/fr/language.json b/locale/fr/language.json index db7d99ab3..97dd59675 100644 --- a/locale/fr/language.json +++ b/locale/fr/language.json @@ -79,6 +79,7 @@ "form.text_size_example": "Le texte aura cette taille", "form.section_text_processing": "Traitement du texte", "form.parser_type": "Type d'analyseur", + "form.parser_automatic": "Automatique", "form.parser_unavailable": " (indisponible)", "form.character_substitutions": "Substitutions de caractères", "form.character_substitutions_help": "Remplacer les caractères avant l'analyse (format : de=à, séparés par |)", diff --git a/locale/it/language.json b/locale/it/language.json index e52ac10d8..d61ac7527 100644 --- a/locale/it/language.json +++ b/locale/it/language.json @@ -79,6 +79,7 @@ "form.text_size_example": "Il testo avrà questa dimensione", "form.section_text_processing": "Elaborazione del testo", "form.parser_type": "Tipo di parser", + "form.parser_automatic": "Automatico", "form.parser_unavailable": " (non disponibile)", "form.character_substitutions": "Sostituzioni di caratteri", "form.character_substitutions_help": "Sostituisci i caratteri prima dell'analisi (formato: da=a, separati da |)", diff --git a/locale/ja/language.json b/locale/ja/language.json index ddec97ee7..de13cb01b 100644 --- a/locale/ja/language.json +++ b/locale/ja/language.json @@ -79,6 +79,7 @@ "form.text_size_example": "テキストはこのサイズになります", "form.section_text_processing": "テキスト処理", "form.parser_type": "パーサータイプ", + "form.parser_automatic": "自動", "form.parser_unavailable": "（利用不可）", "form.character_substitutions": "文字置換", "form.character_substitutions_help": "解析前に文字を置換します（形式：from=to、|で区切り）", diff --git a/locale/pt/language.json b/locale/pt/language.json index 8d0ff3750..df70e57fc 100644 --- a/locale/pt/language.json +++ b/locale/pt/language.json @@ -79,6 +79,7 @@ "form.text_size_example": "O texto terá este tamanho", "form.section_text_processing": "Processamento de Texto", "form.parser_type": "Tipo de Analisador", + "form.parser_automatic": "Automático", "form.parser_unavailable": " (indisponível)", "form.character_substitutions": "Substituições de Caracteres", "form.character_substitutions_help": "Substituir caracteres antes da análise (formato: de=para, separados por |)", diff --git a/locale/ru/language.json b/locale/ru/language.json index 232c1d4f3..e8580dfb5 100644 --- a/locale/ru/language.json +++ b/locale/ru/language.json @@ -79,6 +79,7 @@ "form.text_size_example": "Текст будет такого размера", "form.section_text_processing": "Обработка текста", "form.parser_type": "Тип парсера", + "form.parser_automatic": "Автоматически", "form.parser_unavailable": " (недоступно)", "form.character_substitutions": "Замены символов", "form.character_substitutions_help": "Заменять символы перед парсингом (формат: from=to, через |)", diff --git a/locale/zh/language.json b/locale/zh/language.json index 2e03ffe00..022daabbf 100644 --- a/locale/zh/language.json +++ b/locale/zh/language.json @@ -79,6 +79,7 @@ "form.text_size_example": "文本将以此大小显示", "form.section_text_processing": "文本处理", "form.parser_type": "解析器类型", + "form.parser_automatic": "自动", "form.parser_unavailable": "（不可用）", "form.character_substitutions": "字符替换", "form.character_substitutions_help": "解析前替换字符（格式：from=to，用 | 分隔）", diff --git a/src/Modules/Admin/Application/Services/TtsService.php b/src/Modules/Admin/Application/Services/TtsService.php index b4e67afe5..5d8fd9b4e 100644 --- a/src/Modules/Admin/Application/Services/TtsService.php +++ b/src/Modules/Admin/Application/Services/TtsService.php @@ -57,7 +57,7 @@ public function __construct(?LanguageFacade $languageService = null) * * @param int $lgId Language ID * @param array $langArray Languages array from langdefs * * @return string Two-letter language code @@ -94,7 +94,7 @@ public function getLanguageIdFromCode(string $code, array $langArray): int * Get language options for TTS form. * * @param array $langArray Languages array from langdefs * * @return string HTML-formatted options string @@ -117,7 +117,7 @@ public function getLanguageOptions(array $langArray): string * Get current language code for TTS settings. * * @param array $langArray Languages array from langdefs * * @return string Current language code diff --git a/src/Modules/Language/Application/LanguageFacade.php b/src/Modules/Language/Application/LanguageFacade.php index da312e25f..1c6ddbc25 100644 --- a/src/Modules/Language/Application/LanguageFacade.php +++ b/src/Modules/Language/Application/LanguageFacade.php @@ -354,7 +354,7 @@ public function getLanguageName($id): string * * @param int $id Language ID * @param array $languagesTable + * 4: string, 5: bool, 6: bool, 7: bool, 8: string}> $languagesTable * Table of languages, usually from LanguagePresets::getAll() * * @return string Two-letter code (e.g., BCP 47) or empty string diff --git a/src/Modules/Language/Application/UseCases/GetLanguageCode.php b/src/Modules/Language/Application/UseCases/GetLanguageCode.php index bd4d55c8e..002125e3a 100644 --- a/src/Modules/Language/Application/UseCases/GetLanguageCode.php +++ b/src/Modules/Language/Application/UseCases/GetLanguageCode.php @@ -63,7 +63,7 @@ public function getLanguageName($id): string * * @param int $id Language ID * @param array $languagesTable + * 4: string, 5: bool, 6: bool, 7: bool, 8: string}> $languagesTable * Table of languages, usually from LanguagePresets::getAll() * * @return string Two-letter code (e.g., BCP 47) or empty string diff --git a/src/Modules/Language/Http/LanguageApiHandler.php b/src/Modules/Language/Http/LanguageApiHandler.php index 2e08c4bac..cb0c8bd25 100644 --- a/src/Modules/Language/Http/LanguageApiHandler.php +++ b/src/Modules/Language/Http/LanguageApiHandler.php @@ -482,7 +482,8 @@ public function formatGetDefinitions(): array 'sentSplRegExp' => $def[4], 'makeCharacterWord' => $def[5], 'removeSpaces' => $def[6], - 'rightToLeft' => $def[7] + 'rightToLeft' => $def[7], + 'parserType' => $def[8] ]; } diff --git a/src/Modules/Language/Infrastructure/Data/langdefs.json b/src/Modules/Language/Infrastructure/Data/langdefs.json index c74a7f990..be716f007 100644 --- a/src/Modules/Language/Infrastructure/Data/langdefs.json +++ b/src/Modules/Language/Infrastructure/Data/langdefs.json @@ -58,7 +58,7 @@ "makeCharacterWord": true, "removeSpaces": true, "rightToLeft": false, - "parserType": "character" + "parserType": "jieba" }, "Chinese (Traditional)": { "glosbeIso": "zh", @@ -69,7 +69,7 @@ "makeCharacterWord": true, "removeSpaces": true, "rightToLeft": false, - "parserType": "character" + "parserType": "jieba" }, "Croatian": { "glosbeIso": "hr", diff --git a/src/Modules/Language/Infrastructure/Parser/ParserRegistry.php b/src/Modules/Language/Infrastructure/Parser/ParserRegistry.php index 70139a413..c46544588 100644 --- a/src/Modules/Language/Infrastructure/Parser/ParserRegistry.php +++ b/src/Modules/Language/Infrastructure/Parser/ParserRegistry.php @@ -42,11 +42,16 @@ class ParserRegistry /** * Create a new parser registry with default parsers. * - * @param ExternalParserLoader|null $externalLoader Optional loader for external parsers + * Builds its own loader when none is given, so that a registry constructed + * directly still sees the parsers in config/parsers.php. It did not before, + * which is why jieba never reached the language form's parser list even on + * an install where it was installed and working. + * + * @param ExternalParserLoader|null $externalLoader Loader for external parsers */ public function __construct(?ExternalParserLoader $externalLoader = null) { - $this->externalLoader = $externalLoader; + $this->externalLoader = $externalLoader ?? new ExternalParserLoader(); $this->registerDefaultParsers(); $this->registerExternalParsers(); } @@ -176,6 +181,40 @@ public function getDefaultType(): string return self::DEFAULT_PARSER; } + /** + * The parser a language deliberately asked for, if any. + * + * Only an explicit, non-default `LgParserType` counts. The legacy signals + * resolveParserTypeFromRow() also understands — the MECAB magic word and + * the split-each-character flag — are deliberately ignored here: the + * pipeline has always handled those itself, and every language predating + * this field carries no parser type at all, so returning null for them + * keeps their parsing byte-identical. + * + * @param array $row Database row with Lg* prefixed columns + * + * @return ParserInterface|null The chosen parser, or null to leave the + * language on the built-in pipeline + */ + public function getOptedInParserFromRow(array $row): ?ParserInterface + { + $type = trim((string) ($row['LgParserType'] ?? '')); + if ($type === '' || $type === self::DEFAULT_PARSER) { + return null; + } + + $parser = $this->get($type); + if ($parser === null || !$parser->isAvailable()) { + // An unavailable parser must not drop the language onto the regex + // parser: for the CJK languages that ask for jieba or mecab, that + // yields a text with no words at all. The built-in pipeline still + // honours their split-each-character setting, so fall back to it. + return null; + } + + return $parser; + } + /** * Resolve the parser type for a language. * diff --git a/src/Modules/Language/Views/form.php b/src/Modules/Language/Views/form.php index 122d44868..e3c5aa16c 100644 --- a/src/Modules/Language/Views/form.php +++ b/src/Modules/Language/Views/form.php @@ -432,6 +432,7 @@ class="input"
on change, not input + input.dispatchEvent(new Event('change', { bubbles: true })); } /** @@ -514,6 +519,7 @@ function applyWizardPreset(): void { makeCharacterWord: boolean; removeSpaces: boolean; rightToLeft: boolean; + parserType?: string; } >; }; @@ -591,6 +597,13 @@ function applyWizardPreset(): void { l2Def.rightToLeft ); + // Tokenizer the preset asks for — jieba for Chinese, MeCab for Japanese. + // An empty value leaves it inferred from the flags above. + setBoundValue( + lgForm.elements.namedItem('LgParserType') as HTMLSelectElement | null, + l2Def.parserType ?? '' + ); + console.log(`Applied wizard preset for ${data.l2} (L1: ${data.l1})`); } catch (e) { console.error('Failed to apply wizard preset:', e); diff --git a/src/frontend/js/modules/language/stores/language_form_store.ts b/src/frontend/js/modules/language/stores/language_form_store.ts index 33a820afc..25ab2f195 100644 --- a/src/frontend/js/modules/language/stores/language_form_store.ts +++ b/src/frontend/js/modules/language/stores/language_form_store.ts @@ -38,6 +38,7 @@ export interface LanguageFormData { regexpWordCharacters: string; removeSpaces: boolean; splitEachChar: boolean; + parserType: string; rightToLeft: boolean; ttsVoiceApi: string; showRomanization: boolean; @@ -122,6 +123,7 @@ function createEmptyFormData(): LanguageFormData { regexpWordCharacters: 'a-zA-ZÀ-ÖØ-öø-ȳ', removeSpaces: false, splitEachChar: false, + parserType: '', rightToLeft: false, ttsVoiceApi: '', showRomanization: true @@ -247,6 +249,7 @@ function createLanguageFormStore(): LanguageFormStoreState { regexpWordCharacters: lang.regexpWordCharacters, removeSpaces: lang.removeSpaces, splitEachChar: lang.splitEachChar, + parserType: lang.parserType ?? '', rightToLeft: lang.rightToLeft, ttsVoiceApi: lang.ttsVoiceApi, showRomanization: lang.showRomanization @@ -324,6 +327,7 @@ function createLanguageFormStore(): LanguageFormStoreState { this.formData.regexpSplitSentences = l2Def.sentSplRegExp; this.formData.regexpWordCharacters = l2Def.wordCharRegExp; this.formData.splitEachChar = l2Def.makeCharacterWord; + this.formData.parserType = l2Def.parserType ?? ''; this.formData.removeSpaces = l2Def.removeSpaces; this.formData.rightToLeft = l2Def.rightToLeft; }, diff --git a/tests/backend/Modules/Language/Infrastructure/Parser/ParserRegistryTest.php b/tests/backend/Modules/Language/Infrastructure/Parser/ParserRegistryTest.php index 8ef2cd3da..044458a8c 100644 --- a/tests/backend/Modules/Language/Infrastructure/Parser/ParserRegistryTest.php +++ b/tests/backend/Modules/Language/Infrastructure/Parser/ParserRegistryTest.php @@ -116,4 +116,94 @@ public function testRegisterCustomParser(): void $this->assertTrue($this->registry->has('custom')); $this->assertSame($customParser, $this->registry->get('custom')); } + + // ========================================================================= + // Opted-in parser resolution (#278) + // ========================================================================= + + public function testNoParserTypeLeavesTheLanguageOnTheBuiltInPipeline(): void + { + // Every language predating the field stores nothing here, and their + // parsing must not change + $this->assertNull($this->registry->getOptedInParserFromRow([])); + $this->assertNull($this->registry->getOptedInParserFromRow(['LgParserType' => null])); + $this->assertNull($this->registry->getOptedInParserFromRow(['LgParserType' => ''])); + $this->assertNull($this->registry->getOptedInParserFromRow(['LgParserType' => ' '])); + } + + public function testTheDefaultParserIsNotAnOptIn(): void + { + // "regex" is what the built-in pipeline already does + $this->assertNull($this->registry->getOptedInParserFromRow(['LgParserType' => 'regex'])); + } + + public function testLegacySignalsAreNotOptIns(): void + { + // resolveParserTypeFromRow() reads both of these; the pipeline has + // always handled them itself, so they must not route anywhere new + $this->assertNull($this->registry->getOptedInParserFromRow([ + 'LgRegexpWordCharacters' => 'MECAB', + ])); + $this->assertNull($this->registry->getOptedInParserFromRow([ + 'LgSplitEachChar' => 1, + ])); + } + + public function testAnExplicitAvailableParserIsReturned(): void + { + $parser = $this->registry->getOptedInParserFromRow(['LgParserType' => 'character']); + + $this->assertInstanceOf(CharacterParser::class, $parser); + } + + public function testAnUnknownParserFallsBackToTheBuiltInPipeline(): void + { + $this->assertNull( + $this->registry->getOptedInParserFromRow(['LgParserType' => 'no-such-parser']) + ); + } + + public function testAnUnavailableParserFallsBackToTheBuiltInPipeline(): void + { + // Not to the regex parser: for a language that asked for jieba or + // mecab, regex yields a text with no words at all + $this->registry->register(new UnavailableTestParser()); + + $this->assertNull( + $this->registry->getOptedInParserFromRow(['LgParserType' => 'unavailable-test']) + ); + } +} + +/** + * A registered parser that the server cannot run. + */ +class UnavailableTestParser implements ParserInterface +{ + public function getType(): string + { + return 'unavailable-test'; + } + + public function getName(): string + { + return 'Unavailable Test Parser'; + } + + public function isAvailable(): bool + { + return false; + } + + public function getAvailabilityMessage(): string + { + return 'not installed'; + } + + public function parse( + string $text, + \Lwt\Modules\Language\Domain\Parser\ParserConfig $config + ): \Lwt\Modules\Language\Domain\Parser\ParserResult { + throw new \RuntimeException('never called'); + } } diff --git a/tests/backend/Shared/Infrastructure/Language/LanguagePresetsParserTypeTest.php b/tests/backend/Shared/Infrastructure/Language/LanguagePresetsParserTypeTest.php new file mode 100644 index 000000000..09addf79a --- /dev/null +++ b/tests/backend/Shared/Infrastructure/Language/LanguagePresetsParserTypeTest.php @@ -0,0 +1,67 @@ + + */ + +declare(strict_types=1); + +namespace Lwt\Tests\Shared\Infrastructure\Language; + +use Lwt\Shared\Infrastructure\Language\LanguagePresets; +use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\TestCase; + +/** + * The tokenizer a preset asks for reaches the caller (#278). + * + * langdefs.json has carried a parserType for the CJK languages since the + * parser modules were added, but loadFromJson() built an eight-slot tuple + * that dropped it, so nothing could ever read it. + */ +#[CoversClass(LanguagePresets::class)] +class LanguagePresetsParserTypeTest extends TestCase +{ + public function testChinesePresetsAskForJieba(): void + { + $all = LanguagePresets::getAll(); + + $this->assertSame('jieba', $all['Chinese (Simplified)'][8]); + $this->assertSame('jieba', $all['Chinese (Traditional)'][8]); + } + + public function testJapanesePresetAsksForMecab(): void + { + $this->assertSame('mecab', LanguagePresets::getAll()['Japanese'][8]); + } + + public function testChinesePresetsStillSplitEachCharacter(): void + { + // The fallback when jieba is not installed: per-character parsing, + // which is what these languages did before jieba was reachable + $all = LanguagePresets::getAll(); + + $this->assertTrue($all['Chinese (Simplified)'][5]); + $this->assertTrue($all['Chinese (Traditional)'][5]); + } + + public function testALanguageWithoutAParserTypeGetsAnEmptyString(): void + { + // Empty means "infer", which is what every non-CJK preset wants + $this->assertSame('', LanguagePresets::getAll()['English'][8]); + } + + public function testEveryPresetCarriesTheSlot(): void + { + foreach (LanguagePresets::getAll() as $name => $def) { + $this->assertArrayHasKey(8, $def, "preset $name lost its parser type"); + $this->assertIsString($def[8]); + } + } +} From 17262f57390b9f0e281e1c6a428e6673b15c135e Mon Sep 17 00:00:00 2001 From: HugoFara Date: Thu, 27 Aug 2026 09:30:41 +0200 Subject: [PATCH 2/2] fix(language): treat a backfilled parser type as legacy, not intent The opt-in test asked whether LgParserType was set, on the reasoning that no language stores one. They do. 20251223_120000_add_parser_type.sql backfills the column from the very legacy signals the opt-in test excludes: 'mecab' wherever the magic word sits in LgRegexpWordCharacters, 'character' wherever LgSplitEachChar is set. Every upgraded install with a CJK language therefore carries a parser type nobody chose, and routing it to the registry retokenizes the language. Measured with TextParsing::checkText() on a real 3.4.2 database, same texts, same settings, before and after: lg=2 Chinese (character, split=1) 12 sent / 103 words -> 13 / 122 lg=5 Japanese (character, split=1) 13 sent / 46 words -> 14 / 60 lg=1 French, lg=3 German, lg=4, 6, 7, 8 (no type) unchanged Existing texts keep their stored parse, so nothing breaks at upgrade; it diverges later and quietly, when UpdateLanguage re-parses on a settings change or a new text is imported. The language then holds texts split two different ways, and terms link by string, so saved vocabulary stops matching new occurrences. Read a type that only restates the flag beside it as the flag, not as a choice. 'character' with LgSplitEachChar, and 'mecab' with the magic word, carry no information the built-in pipeline is not already acting on. Anything else could only have been picked in the form: jieba, an external tokenizer, or 'character' on a language whose split flag is off, since the backfill never wrote that combination. Re-measured after the change: every language matches the pre-#281 baseline exactly. A language set to jieba still routes to jieba, and one naming a parser the server cannot run still falls back to the built-in pipeline. Deriving intent this way is a workaround for the magic word overloading LgRegexpWordCharacters, and can go once that is retired. --- .../Infrastructure/Parser/ParserRegistry.php | 49 ++++++++++++++- .../Parser/ParserRegistryTest.php | 59 +++++++++++++++++++ 2 files changed, 106 insertions(+), 2 deletions(-) diff --git a/src/Modules/Language/Infrastructure/Parser/ParserRegistry.php b/src/Modules/Language/Infrastructure/Parser/ParserRegistry.php index c46544588..d8b2566c8 100644 --- a/src/Modules/Language/Infrastructure/Parser/ParserRegistry.php +++ b/src/Modules/Language/Infrastructure/Parser/ParserRegistry.php @@ -187,10 +187,24 @@ public function getDefaultType(): string * Only an explicit, non-default `LgParserType` counts. The legacy signals * resolveParserTypeFromRow() also understands — the MECAB magic word and * the split-each-character flag — are deliberately ignored here: the - * pipeline has always handled those itself, and every language predating - * this field carries no parser type at all, so returning null for them + * pipeline has always handled those itself, so returning null for them * keeps their parsing byte-identical. * + * A stored parser type is not by itself evidence of a choice, which is the + * subtle half. 20251223_120000_add_parser_type.sql *backfilled* the column + * from those same legacy signals — 'mecab' for the magic word, 'character' + * for LgSplitEachChar — so on every upgraded install the CJK languages + * already carry a type nobody picked. Routing those to the registry + * retokenizes them: measured on a real database, a character-split Chinese + * text goes from 103 words to 122 and Japanese from 46 to 60, which would + * silently desynchronise saved terms from new text occurrences. A value + * that merely restates the legacy signal beside it is therefore read as the + * legacy signal, not as intent. + * + * Deriving intent this way is a workaround for the magic word overloading + * LgRegexpWordCharacters; it goes away once that is retired and the column + * means only what a user chose. + * * @param array $row Database row with Lg* prefixed columns * * @return ParserInterface|null The chosen parser, or null to leave the @@ -203,6 +217,10 @@ public function getOptedInParserFromRow(array $row): ?ParserInterface return null; } + if (self::restatesALegacySignal($type, $row)) { + return null; + } + $parser = $this->get($type); if ($parser === null || !$parser->isAvailable()) { // An unavailable parser must not drop the language onto the regex @@ -215,6 +233,33 @@ public function getOptedInParserFromRow(array $row): ?ParserInterface return $parser; } + /** + * Whether a stored parser type only repeats the legacy flag beside it. + * + * The backfill wrote 'mecab' where the magic word was and 'character' where + * LgSplitEachChar was set, so those two combinations carry no more + * information than the flags do, and the built-in pipeline already acts on + * the flags. Anything else — jieba, an external tokenizer, or 'character' + * on a language whose split flag is off — could only have been chosen. + * + * @param string $type Trimmed, non-empty LgParserType + * @param array $row Database row with Lg* prefixed columns + * + * @return bool True when the value is derived rather than chosen + */ + private static function restatesALegacySignal(string $type, array $row): bool + { + if ($type === 'mecab') { + return strtoupper(trim((string) ($row['LgRegexpWordCharacters'] ?? ''))) === 'MECAB'; + } + + if ($type === 'character') { + return (int) ($row['LgSplitEachChar'] ?? 0) === 1; + } + + return false; + } + /** * Resolve the parser type for a language. * diff --git a/tests/backend/Modules/Language/Infrastructure/Parser/ParserRegistryTest.php b/tests/backend/Modules/Language/Infrastructure/Parser/ParserRegistryTest.php index 044458a8c..5d129f37e 100644 --- a/tests/backend/Modules/Language/Infrastructure/Parser/ParserRegistryTest.php +++ b/tests/backend/Modules/Language/Infrastructure/Parser/ParserRegistryTest.php @@ -149,6 +149,65 @@ public function testLegacySignalsAreNotOptIns(): void ])); } + public function testABackfilledCharacterTypeIsNotAnOptIn(): void + { + // 20251223_120000_add_parser_type.sql set LgParserType='character' on + // every LgSplitEachChar language, so an upgraded install carries a type + // nobody chose. Routing it to CharacterParser retokenizes the language: + // measured on a real database, Chinese went from 103 words to 122. + $this->assertNull($this->registry->getOptedInParserFromRow([ + 'LgParserType' => 'character', + 'LgSplitEachChar' => 1, + ])); + } + + public function testABackfilledMecabTypeIsNotAnOptIn(): void + { + // Same backfill, from the magic word this time. The built-in pipeline + // already routes these to JapaneseTextParser. + $this->assertNull($this->registry->getOptedInParserFromRow([ + 'LgParserType' => 'mecab', + 'LgRegexpWordCharacters' => 'MECAB', + ])); + $this->assertNull($this->registry->getOptedInParserFromRow([ + 'LgParserType' => 'mecab', + 'LgRegexpWordCharacters' => ' mecab ', + ])); + } + + public function testACharacterTypeWithoutTheSplitFlagIsAChoice(): void + { + // The backfill only ever wrote 'character' alongside the flag, so this + // combination could only have come from the language form. + $this->assertInstanceOf( + CharacterParser::class, + $this->registry->getOptedInParserFromRow([ + 'LgParserType' => 'character', + 'LgSplitEachChar' => 0, + ]) + ); + } + + public function testAParserUnrelatedToTheLegacyFlagsIsAlwaysAChoice(): void + { + // A language asking for jieba means it, whatever its other flags say + $this->registry->register(new UnavailableTestParser()); + + $this->assertNull($this->registry->getOptedInParserFromRow([ + 'LgParserType' => 'unavailable-test', + 'LgSplitEachChar' => 1, + ]), 'an unavailable parser still falls back, but for its own reason'); + + $this->assertInstanceOf( + CharacterParser::class, + $this->registry->getOptedInParserFromRow([ + 'LgParserType' => 'character', + 'LgRegexpWordCharacters' => 'MECAB', + ]), + 'the magic word only excuses a mecab type, not any other' + ); + } + public function testAnExplicitAvailableParserIsReturned(): void { $parser = $this->registry->getOptedInParserFromRow(['LgParserType' => 'character']);