Merge pull request #91 from m1sk9/fix-conversion-eng

fix: Fix kana converter valid romaji

5 files changed, 466 insertions, 3 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 892cf86..4ec85b1 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,7 @@
 
 - Velocity integration is now available.
 - Fixed the header design for certain commands such as `/lc channel`.
+- When converting to Roman letters, if kana characters are included, those conversions should be skipped.
 
 ### v0.7.0
 
diff --git a/engine/src/main/kotlin/dev/m1sk9/lunaticChat/engine/converter/KanaConverter.kt b/engine/src/main/kotlin/dev/m1sk9/lunaticChat/engine/converter/KanaConverter.kt
index 1b3c67d..24e3560 100644
--- a/engine/src/main/kotlin/dev/m1sk9/lunaticChat/engine/converter/KanaConverter.kt
+++ b/engine/src/main/kotlin/dev/m1sk9/lunaticChat/engine/converter/KanaConverter.kt
@@ -254,6 +254,75 @@ object KanaConverter {
     }
 
     /**
+     * Checks if the input can be fully converted to hiragana without any remaining alphabetic characters.
+     * This validates that the input is valid romaji before attempting conversion.
+     *
+     * @param input The text to validate
+     * @return true if the input is valid romaji, false otherwise
+     */
+    fun isValidRomaji(input: String): Boolean {
+        val lowerInput = input.lowercase()
+        var i = 0
+
+        while (i < lowerInput.length) {
+            val char = lowerInput[i]
+
+            // Non-alphabetic characters are allowed (spaces, numbers, symbols)
+            if (char !in 'a'..'z') {
+                i++
+                continue
+            }
+
+            // Check for double consonant (valid in romaji for っ)
+            if (i + 1 < lowerInput.length) {
+                val next = lowerInput[i + 1]
+                if (char == next && char in "bcdfghjklmpqrstvwxyz") {
+                    i++
+                    continue
+                }
+            }
+
+            // Try to find the longest match in the trie
+            var node: TrieNode = romanjiTrie
+            var matchLength = 0
+            var j = i
+
+            while (j < lowerInput.length && lowerInput[j] in 'a'..'z') {
+                node =
+                    when (node) {
+                        is TrieNode.Branch -> {
+                            if (node.value != null) {
+                                matchLength = j - i
+                            }
+                            node.children[lowerInput[j]] ?: break
+                        }
+                        is TrieNode.Leaf -> {
+                            matchLength = j - i
+                            break
+                        }
+                    }
+                j++
+            }
+
+            // Check for terminal match
+            if (node is TrieNode.Leaf) {
+                matchLength = j - i
+            } else if (node is TrieNode.Branch && node.value != null) {
+                matchLength = j - i
+            }
+
+            // If no match found, this character cannot be converted - not valid romaji
+            if (matchLength == 0) {
+                return false
+            }
+
+            i += matchLength
+        }
+
+        return true
+    }
+
+    /**
      * Converts romanji text to hiragana.
      *
      * @param input The romanji text to convert
diff --git a/engine/src/test/kotlin/dev/m1sk9/lunaticChat/engine/converter/KanaConverterTest.kt b/engine/src/test/kotlin/dev/m1sk9/lunaticChat/engine/converter/KanaConverterTest.kt
index a348fa3..5ba7f0e 100644
--- a/engine/src/test/kotlin/dev/m1sk9/lunaticChat/engine/converter/KanaConverterTest.kt
+++ b/engine/src/test/kotlin/dev/m1sk9/lunaticChat/engine/converter/KanaConverterTest.kt
@@ -2,6 +2,8 @@ package dev.m1sk9.lunaticChat.engine.converter
 
 import kotlin.test.Test
 import kotlin.test.assertEquals
+import kotlin.test.assertFalse
+import kotlin.test.assertTrue
 
 class KanaConverterTest {
     @Test
@@ -290,4 +292,87 @@ class KanaConverterTest {
         val expected = "わたし わ にほんご を べんきょう しています"
         assertEquals(expected, KanaConverter.toHiragana(input))
     }
+
+    // isValidRomaji tests
+
+    @Test
+    fun `isValidRomaji should return true for valid romaji words`() {
+        assertTrue(KanaConverter.isValidRomaji("konnichiwa"))
+        assertTrue(KanaConverter.isValidRomaji("arigatou"))
+        assertTrue(KanaConverter.isValidRomaji("ohayou"))
+        assertTrue(KanaConverter.isValidRomaji("sumimasen"))
+        assertTrue(KanaConverter.isValidRomaji("ganbatte"))
+    }
+
+    @Test
+    fun `isValidRomaji should return true for single vowels`() {
+        assertTrue(KanaConverter.isValidRomaji("a"))
+        assertTrue(KanaConverter.isValidRomaji("i"))
+        assertTrue(KanaConverter.isValidRomaji("u"))
+        assertTrue(KanaConverter.isValidRomaji("e"))
+        assertTrue(KanaConverter.isValidRomaji("o"))
+    }
+
+    @Test
+    fun `isValidRomaji should return true for double consonants`() {
+        assertTrue(KanaConverter.isValidRomaji("kitte"))
+        assertTrue(KanaConverter.isValidRomaji("gakkou"))
+        assertTrue(KanaConverter.isValidRomaji("sappari"))
+    }
+
+    @Test
+    fun `isValidRomaji should return true for phrases with spaces`() {
+        assertTrue(KanaConverter.isValidRomaji("watashi wa nihongo wo benkyou shiteimasu"))
+    }
+
+    @Test
+    fun `isValidRomaji should return true for input with numbers and symbols`() {
+        assertTrue(KanaConverter.isValidRomaji("arigatou123"))
+        assertTrue(KanaConverter.isValidRomaji("konnichiwa!"))
+        assertTrue(KanaConverter.isValidRomaji("hai?"))
+    }
+
+    @Test
+    fun `isValidRomaji should return false for English words`() {
+        // These contain consonants that cannot be converted (e.g., 's' alone, 'v', 'c' without vowel pattern)
+        assertFalse(KanaConverter.isValidRomaji("This"))
+        assertFalse(KanaConverter.isValidRomaji("server"))
+        assertFalse(KanaConverter.isValidRomaji("version"))
+        assertFalse(KanaConverter.isValidRomaji("running"))
+        assertFalse(KanaConverter.isValidRomaji("Paper"))
+    }
+
+    @Test
+    fun `isValidRomaji should return false for words with unconvertable consonant endings`() {
+        // 's', 'r', 'c', 'v', 'l', 'x' alone or in non-romaji patterns cannot be converted
+        assertFalse(KanaConverter.isValidRomaji("test"))
+        assertFalse(KanaConverter.isValidRomaji("cat"))
+        assertFalse(KanaConverter.isValidRomaji("fix"))
+    }
+
+    @Test
+    fun `isValidRomaji should return true for n at word end`() {
+        assertTrue(KanaConverter.isValidRomaji("san"))
+        assertTrue(KanaConverter.isValidRomaji("nihon"))
+        assertTrue(KanaConverter.isValidRomaji("ramen"))
+    }
+
+    @Test
+    fun `isValidRomaji should handle uppercase input`() {
+        assertTrue(KanaConverter.isValidRomaji("KONNICHIWA"))
+        assertFalse(KanaConverter.isValidRomaji("THIS"))
+        assertFalse(KanaConverter.isValidRomaji("SERVER"))
+    }
+
+    @Test
+    fun `isValidRomaji should return true for empty string`() {
+        assertTrue(KanaConverter.isValidRomaji(""))
+    }
+
+    @Test
+    fun `isValidRomaji should return true for only numbers and symbols`() {
+        assertTrue(KanaConverter.isValidRomaji("123"))
+        assertTrue(KanaConverter.isValidRomaji("!@#"))
+        assertTrue(KanaConverter.isValidRomaji("1.21.11-110"))
+    }
 }
diff --git a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/converter/RomanjiConverter.kt b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/converter/RomanjiConverter.kt
index 9e54f24..c961af8 100644
--- a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/converter/RomanjiConverter.kt
+++ b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/converter/RomanjiConverter.kt
@@ -48,11 +48,18 @@ class RomanjiConverter(
                 continue
             }
 
+            // Pre-validate: Check if the word is valid romaji before attempting conversion
+            // This prevents partial conversion of English words (e.g., "This" -> "てぃs")
+            if (!KanaConverter.isValidRomaji(word)) {
+                if (debugMode) {
+                    logger.info("Word is not valid romaji, keeping original: $word")
+                }
+                results.add(word)
+                continue
+            }
+
             // Step 1: Romanji -> Hiragana
             val hiragana = KanaConverter.toHiragana(word)
-            if (debugMode) {
-                logger.info("Romanji -> Hiragana: $word -> $hiragana")
-            }
 
             // Step 2: Hiragana -> Kanji/Kana
             val converted =
diff --git a/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/converter/RomanjiConverterTest.kt b/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/converter/RomanjiConverterTest.kt
new file mode 100644
index 0000000..1e6d3c0
--- /dev/null
+++ b/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/converter/RomanjiConverterTest.kt
@@ -0,0 +1,301 @@
+package dev.m1sk9.lunaticChat.paper.converter
+
+import dev.m1sk9.lunaticChat.engine.converter.GoogleIMEClient
+import dev.m1sk9.lunaticChat.paper.TestUtils
+import io.mockk.coEvery
+import io.mockk.coVerify
+import io.mockk.every
+import io.mockk.mockk
+import io.mockk.verify
+import kotlinx.coroutines.runBlocking
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertNull
+
+/**
+ * Tests for RomanjiConverter.
+ * Verifies conversion logic, English word handling, and caching behavior.
+ *
+ * Note: The alphabetic character check after KanaConverter determines whether
+ * a word is treated as English or romaji:
+ * - "version" -> "vえrしおん" (has 'v', 'r') -> kept as "version"
+ * - "hello" -> "へっぉ" (no alphabetic chars) -> treated as romaji, converted
+ * - "API" -> "あぴ" (no alphabetic chars) -> treated as romaji, converted
+ */
+class RomanjiConverterTest {
+    private fun createConverter(debugMode: Boolean = false): Triple<RomanjiConverter, ConversionCache, GoogleIMEClient> {
+        val cache = mockk<ConversionCache>(relaxed = true)
+        val apiClient = mockk<GoogleIMEClient>(relaxed = true)
+        val logger = TestUtils.TestLogger()
+
+        every { cache.get(any()) } returns null
+
+        val converter = RomanjiConverter(cache, apiClient, logger, debugMode)
+        return Triple(converter, cache, apiClient)
+    }
+
+    // ===== English words with unconvertible letters should NOT be converted =====
+    // Words containing letters like 'v', 'l', 'x', 'q' that have no direct hiragana mapping
+    // will retain those letters after KanaConverter, triggering the English word detection.
+
+    @Test
+    fun `English word 'version' should not be converted (has v and r)`() =
+        runBlocking {
+            val (converter, _, _) = createConverter()
+            // "version" -> "vえrしおん" (v, r remain)
+
+            val result = converter.convert("version")
+
+            assertEquals("version", result)
+        }
+
+    @Test
+    fun `English word 'server' should not be converted (has r and v)`() =
+        runBlocking {
+            val (converter, _, _) = createConverter()
+            // "server" -> "せrvえr" (r, v, r remain)
+
+            val result = converter.convert("server")
+
+            assertEquals("server", result)
+        }
+
+    @Test
+    fun `English word 'latest' should not be converted (has s and t)`() =
+        runBlocking {
+            val (converter, _, _) = createConverter()
+            // "latest" -> "ぁてst" (s, t remain)
+
+            val result = converter.convert("latest")
+
+            assertEquals("latest", result)
+        }
+
+    @Test
+    fun `English word 'running' should not be converted (has g)`() =
+        runBlocking {
+            val (converter, _, _) = createConverter()
+            // "running" -> "るんいんg" (g remains)
+
+            val result = converter.convert("running")
+
+            assertEquals("running", result)
+        }
+
+    @Test
+    fun `English word 'world' should not be converted (has r, l, d)`() =
+        runBlocking {
+            val (converter, _, _) = createConverter()
+            // "world" -> "をrld" (r, l, d remain)
+
+            val result = converter.convert("world")
+
+            assertEquals("world", result)
+        }
+
+    @Test
+    fun `English words should not call API`() =
+        runBlocking {
+            val (converter, _, apiClient) = createConverter()
+
+            // These words have unconvertible letters
+            converter.convert("version")
+            converter.convert("server")
+            converter.convert("world")
+
+            coVerify(exactly = 0) { apiClient.convert(any()) }
+        }
+
+    // ===== Words that happen to be valid romaji will be converted =====
+    // Some English words like "hello" (h->へ, ll->っ, o->ぉ) or "API" (a->あ, p->ぴ, i is consumed)
+    // completely convert to hiragana, so they are treated as romaji.
+
+    @Test
+    fun `'hello' converts completely to hiragana so it is treated as romaji`() =
+        runBlocking {
+            val (converter, _, apiClient) = createConverter()
+            // "hello" -> "へっぉ" (no alphabetic chars remain)
+            coEvery { apiClient.convert("へっぉ") } returns "へっぉ"
+
+            val result = converter.convert("hello")
+
+            assertEquals("へっぉ", result)
+            coVerify(exactly = 1) { apiClient.convert("へっぉ") }
+        }
+
+    @Test
+    fun `'API' converts completely to hiragana so it is treated as romaji`() =
+        runBlocking {
+            val (converter, _, apiClient) = createConverter()
+            // "API" -> "あぴ" (no alphabetic chars remain)
+            coEvery { apiClient.convert("あぴ") } returns "あぴ"
+
+            val result = converter.convert("API")
+
+            assertEquals("あぴ", result)
+            coVerify(exactly = 1) { apiClient.convert("あぴ") }
+        }
+
+    // ===== Pure romaji should be converted =====
+
+    @Test
+    fun `romaji 'konnichiwa' should be converted`() =
+        runBlocking {
+            val (converter, _, apiClient) = createConverter()
+            // KanaConverter.toHiragana("konnichiwa") returns "こんいちわ"
+            coEvery { apiClient.convert("こんいちわ") } returns "こんにちは"
+
+            val result = converter.convert("konnichiwa")
+
+            assertEquals("こんにちは", result)
+            coVerify(exactly = 1) { apiClient.convert("こんいちわ") }
+        }
+
+    @Test
+    fun `romaji 'ohayou' should be converted`() =
+        runBlocking {
+            val (converter, _, apiClient) = createConverter()
+            coEvery { apiClient.convert("おはよう") } returns "おはよう"
+
+            val result = converter.convert("ohayou")
+
+            assertEquals("おはよう", result)
+            coVerify(exactly = 1) { apiClient.convert("おはよう") }
+        }
+
+    @Test
+    fun `romaji 'arigatou' should be converted`() =
+        runBlocking {
+            val (converter, _, apiClient) = createConverter()
+            coEvery { apiClient.convert("ありがとう") } returns "ありがとう"
+
+            val result = converter.convert("arigatou")
+
+            assertEquals("ありがとう", result)
+            coVerify(exactly = 1) { apiClient.convert("ありがとう") }
+        }
+
+    // ===== Mixed text (romaji + English) =====
+
+    @Test
+    fun `mixed text should convert romaji and keep English words with unconvertible letters`() =
+        runBlocking {
+            val (converter, _, apiClient) = createConverter()
+            // "konnichiwa" -> "こんいちわ" (pure romaji, converted)
+            // "server" -> "せrvえr" (has r, v, r, kept as English)
+            coEvery { apiClient.convert("こんいちわ") } returns "こんにちは"
+
+            val result = converter.convert("konnichiwa server")
+
+            assertEquals("こんにちは server", result)
+            coVerify(exactly = 1) { apiClient.convert("こんいちわ") }
+        }
+
+    @Test
+    fun `multiple English words with unconvertible letters should all be preserved`() =
+        runBlocking {
+            val (converter, _, apiClient) = createConverter()
+            // "version" has v, r
+            // "server" has r, v, r
+            // "world" has r, l, d
+
+            val result = converter.convert("version server world")
+
+            assertEquals("version server world", result)
+            coVerify(exactly = 0) { apiClient.convert(any()) }
+        }
+
+    // ===== Edge cases =====
+
+    @Test
+    fun `empty string should return empty string`() =
+        runBlocking {
+            val (converter, _, _) = createConverter()
+
+            val result = converter.convert("")
+
+            assertEquals("", result)
+        }
+
+    @Test
+    fun `non-ASCII input should return null`() =
+        runBlocking {
+            val (converter, _, _) = createConverter()
+
+            val result = converter.convert("こんにちは")
+
+            assertNull(result)
+        }
+
+    @Test
+    fun `numbers only should return as-is`() =
+        runBlocking {
+            val (converter, _, apiClient) = createConverter()
+            // Numbers pass through KanaConverter unchanged, no alphabetic chars
+            coEvery { apiClient.convert("123") } returns "123"
+
+            val result = converter.convert("123")
+
+            assertEquals("123", result)
+        }
+
+    @Test
+    fun `spaces only should return empty due to word filtering`() =
+        runBlocking {
+            val (converter, _, _) = createConverter()
+
+            val result = converter.convert("   ")
+
+            assertEquals("", result)
+        }
+
+    // ===== Cache behavior =====
+
+    @Test
+    fun `cached word should be retrieved from cache`() =
+        runBlocking {
+            val (converter, cache, apiClient) = createConverter()
+            every { cache.get("konnichiwa") } returns "こんにちは"
+
+            val result = converter.convert("konnichiwa")
+
+            assertEquals("こんにちは", result)
+            verify(exactly = 1) { cache.get("konnichiwa") }
+            coVerify(exactly = 0) { apiClient.convert(any()) }
+        }
+
+    @Test
+    fun `converted word should be stored in cache`() =
+        runBlocking {
+            val (converter, cache, apiClient) = createConverter()
+            coEvery { apiClient.convert("おはよう") } returns "おはよう"
+
+            converter.convert("ohayou")
+
+            verify(exactly = 1) { cache.put("ohayou", "おはよう") }
+        }
+
+    @Test
+    fun `English words with unconvertible letters should not be cached`() =
+        runBlocking {
+            val (converter, cache, _) = createConverter()
+
+            converter.convert("version")
+
+            verify(exactly = 0) { cache.put(any(), any()) }
+        }
+
+    // ===== API error handling =====
+
+    @Test
+    fun `API failure should fallback to hiragana`() =
+        runBlocking {
+            val (converter, cache, apiClient) = createConverter()
+            coEvery { apiClient.convert("おはよう") } throws Exception("API Error")
+
+            val result = converter.convert("ohayou")
+
+            assertEquals("おはよう", result)
+            verify(exactly = 1) { cache.put("ohayou", "おはよう") }
+        }
+}