Fix the logic of check_sara, check_marttra, is_sumpus, handle_karun_sound_silence - #1453
Open
Warit-Yuv wants to merge 44 commits into
Open
Fix the logic of check_sara, check_marttra, is_sumpus, handle_karun_sound_silence#1453Warit-Yuv wants to merge 44 commits into
Warit-Yuv wants to merge 44 commits into
Conversation
… แม่กน with vowel า (previously mistaken as แม่เกย)
In the check_marttra function, for สระ "า", letter ล is incorrectly assigned to the แม่เกย marttra instead of แม่กน. This cause the problem of word like พาล to be classified as แม่เกย and has a cascading problem to is_sumpus function.
print(kv.check_marttra("พาล")) --> เกย
print(kv.is_sumpus("บ้าน", "พาล")) --> False
Remove the incorrect elif word[-1] in ["ล"]: return "เกย"
and add "ล" back in elif word[-1] in ["ญ", "ณ", "น", "ร", "ฬ"]: return "กน" would solve this problem.
When both word meet the condition (sara == "อำ" and marttra == "กม") or (sara == "อำ" and marttra == "กม"), the elif part only change the first word sara and mattra and left the second word unchanged.
This make all of this incorrectly return False
print(kv.is_sumpus("ชัย", "วัย"))
print(kv.is_sumpus("วัย", "วัย"))
print(kv.is_sumpus("จำ", "ทำ"))
print(kv.is_sumpus("ทำ", "ทำ"))
change the elif to if would solve this edge case.
… ริ, รึ ฤ not only have "รึ" sound like ฤดู ฤทัย คฤหาสน์ but also can have "เรอ" sound from ฤกษ์ (เริก) and "ริ" sound like ฤทธิ์, อังกฤษ, ตฤณ This commit add additional check for ฤ sounds.
Add exception for 'อึ' sound in specific พฤทธิธรรม words and delete the old duplicated logic.
finetune the ฤ logic and patching the edge cases.
Edit handling of silent sounds (handle_karun_sound_silence function) to be able to handle more flexible Karun like พันธุ์, สิทธิ์, ฤทธิ์, จันทร์, พระลักษมณ์, กษัตริย์, ภาพยนตร์ instead of the old return word[:-2] which wouldn't correctly strip all the characters that should be silenced. Refactor check_sara function merging of vowel characters เอ เอ to แอ. Remove the previous implementation which mutating the list while iterating over the list.
Change 'แ' to 'แอ' in merging logic
Fix the discrepancy of อรรม - อัม - อำ. Normalize 'อะ' and 'เกย' to 'ไอ' and 'กา', and 'อำ' and 'อะ' to 'อำ' and 'กา'.
Warit-Yuv
changed the title
Remove Trailing whitespace in line 693 so the auto code reviewer wouldn't flag as an issue.
…ด Standalone words in check_mattra - In check_sara, use the new and improved word = self.handle_karun_sound_silence(word) instead of the old การันต์ Karun silenct word implementation word = word[:-2] - Refactor the check_sara code to accommodate สระประผม Transformed vowels (อัว, เอะ, แอะ, เออ, โอะ, เอีย, เอือ) especially with ไม้ไต่คู้ (-็) - Handle คำโดด Standalone words in check_mattra so it is now correctly classify as แม่ ก กา.
The previous implementation didn't accounted for tone marks at the end of the word (e.g., ใกล้), Silenced word ตัวการันต์, silent ย in ไ-ย, ใ-ย, สระประสม (เ-ีย), คำควบกล้ำ, and อักษรนำ. This new implementation take into account all of the above making the checker more robust. Limitation: This implementation haven't accounts for คำควบกล้ำ and อักษรนำ for letter "ร" and "ว" (โปร, แปร, ไกว) as this is not present in the original implementation. This would cause the these word to still be misclassified in check_mattra. The full rewrite of _has_true_final_yl that will account for ย, ล, ร, ว will coming soon.
Final revision. Extensively tested the integration against numerous edge cases and complex test cases as best as I could. This implementation improves the handling of: - `check_sara` - `check_marttra` - `is_sumpus` - `handle_karun_sound_silence` - Internal function `_is_true_final` Note: The `check_karu_lahu` and `check_aek_too` functions are left untouched. The `check_klon` function also remains unmodified, but it should perform better due to the underlying improvements in `is_sumpus`, which now correctly classifies previously failing edge cases.
Removed commented-out code and trialing whitespace. Due to the complexity of Thai language, to reduce the complexity of `_is_true_final` function from 27 down into 15, we will likely need to split the internal function into smaller sub-function. These could be done in the future as the functionality and correctness of the code is the main priority right now. I have include the comment that should sufficiently enough to guild any maintainer in the future that are going to continue working on this code.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates pythainlp/khavee/core.py to improve Thai poetic-verification logic in KhaveeVerifier, focusing on (1) detecting whether trailing ย/ล/ร/ว are true finals vs. parts of clusters/vowels, (2) refining vowel (check_sara) and spelling-section (check_marttra) classification, (3) normalizing rhyme matching (is_sumpus), and (4) expanding Karun (์) silent-suffix stripping.
Changes:
- Introduces
_is_true_final()(and routes_has_true_final_yl()through it) to better distinguish true final consonants from initial clusters and vowel components. - Refactors
check_sara()to handle more compound/transformed vowel cases (including ไม้ไต่คู้ -็) and extends handling for ฤ/ฦ patterns. - Adjusts
check_marttra()andis_sumpus()to better normalize open-syllable detection and rhyming equivalences, plus extendshandle_karun_sound_silence()for more complex Karun suffix patterns.
Comment on lines
+252
to
+256
| if word == "ฤก" or word.startswith("ฤกษ"): | ||
| sara.append("เออ") | ||
| # for 'อิ' (กฤษณ์, กฤษณะ, ตฤณ, ตฤตีย, ทฤษฎี, ประกฤติ, วิกฤต, ฤทธิ์, อังกฤษ) | ||
| elif any(ex in word for ex in ("กฤช", "กฤต", "กฤษ", "ตฤต", "ตฤณ", "ทฤษ", "ปฤษ", "ศฤง", "สฤต", "ฤทธ")): | ||
| sara.append("อิ") |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Add original_word in `check_sara` Word like ฤทธิ์ now properly accounted for. handle comment properly according to PEP8 and fix the typo in the comment `ไกว`. The test case edit will need to be done later.
…eck_sara` and `check_mattra` Added handling for silent terminal vowels in Pali/Sanskrit words.
change ตริ to ตรี to match correctly with the สระ "อี" assertion in the test case.
…ng against "ว" and add unit tests for edge cases
…ied as แม่กม and fix check sara with สระอัว (ตัว ครัว บัว)
…n dealing with อักษรนำ, คำควบกล้ำ, สระอำ, and many sound of ฤ
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
pythainlp/khavee/core.py:366
silent_vowel_exceptionsis checked withendswith(), so any longer word that happens to end with one of these strings will have its final character removed, potentially misclassifying the spelling section. This looks like it should be an exact-word match.
if any(word.endswith(ex) for ex in silent_vowel_exceptions):
| # Intercept Pali/Sanskrit words with silent terminal vowels (สระที่ไม่ออกเสียงท้ายคำ) Removing the final character -ิ or -ุ | ||
| silent_vowel_exceptions = ["เกียรติ", "ชาติ", "ญาติ", "มัติ", "วัติ", "บัติ", "ญัติ", "ยัติ", | ||
| "ภูมิ", "พฤติ", "พรรดิ", "วรรดิ", "พยาธิ", "โพธิ", "เกตุ", "เมรุ", "เหตุ", "ธาตุ", "วุฒิ"] | ||
| if any(word_req.endswith(ex) for ex in silent_vowel_exceptions): |
Author
There was a problem hiding this comment.
this wouldn't work. It is not as simple as list inclusion check. These word are silent when they act as suffix. Here are some example:
พระเกียรติ (พระ-เกียด), ธรรมชาติ (ทัม-มะ-ชาด), ปฏิบัติ (ปะ-ติ-บัด)
We cannot guaranteed that the tokenizer or people who use the library will break up all the word into chunks so we can't just use word_req in silent_vowel_exceptions.
Noted that it only silenced when act as suffix (endswith) not as infix/prefix -> ชาติพันธุ์ อ่านว่า "ชาด-ติ-พัน" (ไม่ใช่ ชาด-พัน)
Therefore the use of any(word_req.endswith(ex) for ex in silent_vowel_exceptions) here is justified.
|
|
||
| class KhaveeCheckKaruLahuTestCase(unittest.TestCase): | ||
| """Tests for KhaveeVerifier.check_karu_lahu""" | ||
| class KhaveeCheckKaruLahuTestCase(unittest.TestCase): |
Comment on lines
168
to
174
| elif i == "็": | ||
| sara.append("ออ") | ||
| sara.append("็") | ||
| elif "รร" in word: | ||
| if self.check_marttra(word) == "กม": | ||
| sara.append("อำ") | ||
| else: | ||
| sara.append("อะ") |
Author
There was a problem hiding this comment.
"รร" should not be evaluated on a character-by-character basis inside that loop. I will push a fix.
Warit-Yuv
commented
Jul 23, 2026
| """ | ||
| return self._is_true_final(word) | ||
|
|
||
| def _is_true_final(self, word: str) -> bool: |
Author
There was a problem hiding this comment.
It's impossible to refactor and reduce the cognitive complexity down to 15. To properly detect if the word is a true final or not really require that many logic check. Only way to reduce is to split the function into many subfunction which in my opinion is worse.
| # Intercept Pali/Sanskrit words with silent terminal vowels (สระที่ไม่ออกเสียงท้ายคำ) Removing the final character -ิ or -ุ | ||
| silent_vowel_exceptions = ["เกียรติ", "ชาติ", "ญาติ", "มัติ", "วัติ", "บัติ", "ญัติ", "ยัติ", | ||
| "ภูมิ", "พฤติ", "พรรดิ", "วรรดิ", "พยาธิ", "โพธิ", "เกตุ", "เมรุ", "เหตุ", "ธาตุ", "วุฒิ"] | ||
| if any(word_req.endswith(ex) for ex in silent_vowel_exceptions): |
Author
There was a problem hiding this comment.
this wouldn't work. It is not as simple as list inclusion check. These word are silent when they act as suffix. Here are some example:
พระเกียรติ (พระ-เกียด), ธรรมชาติ (ทัม-มะ-ชาด), ปฏิบัติ (ปะ-ติ-บัด)
We cannot guaranteed that the tokenizer or people who use the library will break up all the word into chunks so we can't just use word_req in silent_vowel_exceptions.
Noted that it only silenced when act as suffix (endswith) not as infix/prefix -> ชาติพันธุ์ อ่านว่า "ชาด-ติ-พัน" (ไม่ใช่ ชาด-พัน)
Therefore the use of any(word_req.endswith(ex) for ex in silent_vowel_exceptions) here is justified.
Comment on lines
168
to
174
| elif i == "็": | ||
| sara.append("ออ") | ||
| sara.append("็") | ||
| elif "รร" in word: | ||
| if self.check_marttra(word) == "กม": | ||
| sara.append("อำ") | ||
| else: | ||
| sara.append("อะ") |
Author
There was a problem hiding this comment.
"รร" should not be evaluated on a character-by-character basis inside that loop. I will push a fix.
รร should not be evaluated on a character-by-character basis and should be evaluate once right after the loop finishes
…karu and lahu syllables
Refactor `check_karu_lahu`: streamline logic and enhance readability
Change list [] to set {} for string matching to be more optimized.
…timization Refactor `check_klon` to be more optimized
Rewrite `check_klon` to be more compact and correctly check for inter-stanza rhyme (สัมผัสระหว่างบท). Update `check_klon` to use ssg instead of dict for better segmentation. Move the test case of `check_klon` from tests/core/ to tests/extra/ and update tests/extra/__init__.py accordingly.
…date docstring test_khavee_extended.py now correctly evaluate check_klon
wannaphong
approved these changes
Jul 27, 2026
Replace generator-based any() expressions with explicit short-circuit
or chains in check_sara, check_marttra, and is_sumpus to eliminate
unnecessary iteration overhead. Upgrade single-char containment checks
from set membership (in {...}) to faster C-level string scans
(in "...").
Add class-level docstring for KhaveeVerifier documenting all public
methods, and a method-level docstring for check_karu_lahu explaining
Thai syllable weight (ครุ/ลหุ) rules.
Simplify check_klon error messages by dropping Thai terminology
(วรรคสดับ, วรรครับ, etc.) in favor of concise English "Wak 1"–"Wak 4"
designations. Use __import__("ssg") to silence linter warnings on
optional third-party dependency.
Consolidate seven redundant edge-case test methods into a single
test_special_word case. Fix inverted assertIn argument order in
extended test suite. Apply line-wrapping and trailing-whitespace
cleanups throughout.
Replace _MASKING_TERMINAL_VOWELS frozenset with a tuple so
str.endswith() can match directly at C speed, removing the any()
generator loop. Add empty-string guards to check_sara, check_marttra,
is_sumpus, and check_karu_lahu to prevent crashes on degenerate
inputs ("", tone-only, karun-only).
Improve check_marttra consonant-cluster handling:
- Resolve karun before cluster analysis so stripped characters
do not interfere (e.g. "ศาสตร์", "ศุกร์").
- Expand silent-ร cases to always strip after ต, ช, ป
(fixes "เพชร" -> กด, "กอปร" (pronounced กอบ) -> กบ).
- For ambiguous clusters (กร, ขร, คร, ฆร, ทร), only strip ร
when preceded by a short embedded vowel (-ั, -ิ, -ี, -ุ, -ู),
preserving true finals like "นคร", "มังกร", "สุนทร".
Promote inline set literals to class-level frozenset constants
(_LAHU_SYLLABLE_OVERRIDES). Replace silent fallback error strings
with sensible defaults ("อะ" for check_sara, "กา" for check_marttra).
Micro-optimizations: replace "in" checks on single-element sets
with ==, drop unnecessary [*text] unpacking in check_aek_too,
simplify check_klon regex (\s+ already covers \n/\t), use list
comprehension for stanza building.
Add extensive test coverage for newly handled edge cases in
check_sara and check_marttra.
…ecks
Promote all frequently-allocated set literals to class-level frozenset
constants, eliminating per-call object creation in hot paths:
- _LAM_CLUSTERS, _RUA_CLUSTERS, _WA_CLUSTERS, _WA_WHITELIST for
initial-cluster checks in _is_true_final.
- _OPEN_SYLLABLE_VOWELS, _KOK_CHARS, _KOD_CHARS, _KON_CHARS,
_KOB_CHARS for spelling-section classification in check_marttra.
- _LONG_VOWELS, _SPECIAL_VOWELS for karu/lahu prosody in
check_karu_lahu.
- _SINGLE_CHAR_WORDS, _EXPLICIT_SARA_WORDS for fast identity checks.
Replace single-character set lookups (word[-1] in {"ล","ร","ว"})
with simple string checks (word[-1] in "ลรว"), which skip hash
computation entirely.
Collapse the six-way if/elif chain for explicit sara words into a
single frozenset membership test. Cache word[-1] as last_char in
check_marttra to avoid repeated indexing.
Replace re.split(r'\s+', ...) with str.split() in check_klon — both
produce identical results across all Unicode whitespace but str.split()
is 3–5x faster with no regex overhead.
Remove the redundant marttra == "กา" guard from check_karu_lahu -- the condition marttra != "กา" already short-circuits, making the explicit marttra check in the second branch dead code. Swap assertEqual arguments in test_check_klon to match the (result, expected) convention used consistently throughout the rest of the file.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This pull request addresses several critical defects and bugs in the Thai poetry/rhyme processing submodule
pythainlp/khavee/core.py. The modifications improve vowel (check_sara) detection, syllable spelling section (check_marttra) classification, rhyming logic (is_sumpus), and silent suffix ตัวการันต์ stripping (handle_karun_sound_silence) to ensure strict compliance with standard Thai grammar and poetic constraints.1. Complex Silent Suffix Handling (
handle_karun_sound_silence)word[:-2]) assumes that all silent markers cover exactly one consonant + one Karun (-์). It fails entirely on multi-letter silent suffixes or clusters containing quiet vowels (e.g., ธุ์, ธิ์, ตริย์), leaving leftover unpronounced letters that disrupt vowel and syllable checks.2. Initial Cluster and Semi-Vowel Separation (
_is_true_final)_has_true_final_yldid not check for tone marks (วรรณยุกต์), failing on target words likeใกล้because the string ends with a diacritic (วรรณยุกต์) instead of a consonant. It also misclassified instances where a letter functioned as a vowel component rather than a true final consonant ตัวสะกด (e.g.,เสีย,ไทย)._is_true_finalthat strips ตัวการันต์ and วรรณยุกต์ first, then systematically filters out native Thai คำควบกล้ำ (true clusters) and อักษรนำ (leading consonants) paired with pre-posed vowels (เ-,แ-,โ-,ไ-,ใ-). This prevents semi-vowels and liquids (ล,ร,ว) functioning as initial consonant clusters from being falsely flagged as final consonants.3. Syllable Spelling Section Corrections (
check_marttra)เขว,แปร,ไถล) were previously misclassified into incorrect closed-syllable spelling sections มาตราตัวสะกด (e.g., แม่เกอว, แม่กน) instead of แม่ ก กา. Standalone words representing individual letters (e.g., ธ, ณ) were also assigned to closed categories.check_marttrato verify tokens against_is_true_final. Lexical clusters likeโปร,แปร,ไกล,ไกว, andเปลare now correctly routed into the open-syllable แม่ ก กา category. Standalone characters likeธ,ณ,พณ, andบare also normalized to แม่ ก กา.ล) (e.g.,แอปเปิล,โมเดล,เลเวล,ฟุตบอล) will continue to default to แม่กน under this orthographic checker. Improvement could be made to handle poetic variations that treat these as แม่เกอว for rhyming purposes.4. Vowel Invariant and Transformation Rules (
check_sara)"อัว"parsing defect), and unsafe list operations were applied when merging raw"เ"characters into"แ"."ฤ"and"ฦ"(เออ, อิ, อึ).-็). This fixes errors for common vocabularies:5. Control Flow Refactoring for Rhyme Matching (
is_sumpus)elifstructure allowed arguments to short-circuit early, which caused asymmetric matching failures ifword1normalized butword2did not.ifstatements so both inputs undergo normalization prior to final comparison. This cleanly aligns variations like"อะ"+"แม่เกย"into"ไอ"+"แม่กา", and bridges the poetic rhyming concordance between"อรรม","อัม", and"อำ"(e.g.,ธรรม-สัม-จำ).Verification and Test Cases
The updated core class execution demonstrates proper categorization across vowel mutation, syllable validation, and cluster handling: