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..d8b2566c8 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,85 @@ 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, 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 + * 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; + } + + 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 + // 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; + } + + /** + * 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/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..5d129f37e 100644 --- a/tests/backend/Modules/Language/Infrastructure/Parser/ParserRegistryTest.php +++ b/tests/backend/Modules/Language/Infrastructure/Parser/ParserRegistryTest.php @@ -116,4 +116,153 @@ 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 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']); + + $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]); + } + } +}