Fix the logic of check_sara, check_marttra, is_sumpus, handle_karun_sound_silence#1453
Open
Warit-Yuv wants to merge 39 commits into
Open
Fix the logic of check_sara, check_marttra, is_sumpus, handle_karun_sound_silence#1453Warit-Yuv wants to merge 39 commits into
Warit-Yuv wants to merge 39 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 ฤ
…al require word before tonemark get stripped to evaluate some tone-dependent structures.
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
|
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: