diff --git a/app/src/main/java/com/ganjoor/android/data/Assistant.kt b/app/src/main/java/com/ganjoor/android/data/Assistant.kt index a69c3a5..8429895 100644 --- a/app/src/main/java/com/ganjoor/android/data/Assistant.kt +++ b/app/src/main/java/com/ganjoor/android/data/Assistant.kt @@ -166,6 +166,27 @@ object Assistant { .build() } + /** + * Answers already received, so scrolling a poem does not ask twice for the same thing. A + * LazyColumn disposes what scrolls out of view, which restarts the request behind it; on a + * metered API that is money, and on a model running locally it is a wait the reader already + * sat through. Access-ordered, so the oldest falls out first. + */ + private val answers = object : LinkedHashMap(16, 0.75f, true) { + override fun removeEldestEntry(eldest: Map.Entry) = size > 32 + } + + internal fun cacheKey(prompt: String, language: AssistantLanguage, text: String) = + "$prompt|${language.code}|$text" + + @Synchronized + internal fun cached(key: String): String? = answers[key] + + @Synchronized + internal fun remember(key: String, reply: String) { + answers[key] = reply + } + /** Anthropic is the one service that does not speak the OpenAI shape. */ internal fun isClaude(baseUrl: String) = baseUrl.contains("anthropic.com", ignoreCase = true) diff --git a/app/src/main/java/com/ganjoor/android/ui/AssistantResultSheet.kt b/app/src/main/java/com/ganjoor/android/ui/AssistantResultSheet.kt index 27a8448..246df20 100644 --- a/app/src/main/java/com/ganjoor/android/ui/AssistantResultSheet.kt +++ b/app/src/main/java/com/ganjoor/android/ui/AssistantResultSheet.kt @@ -1,17 +1,19 @@ package com.ganjoor.android.ui +import androidx.compose.animation.AnimatedVisibility import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.navigationBarsPadding import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size import androidx.compose.foundation.rememberScrollState import androidx.compose.foundation.verticalScroll -import androidx.compose.foundation.layout.size import androidx.compose.material.icons.Icons import androidx.compose.material.icons.filled.Share import androidx.compose.material3.CircularProgressIndicator -import androidx.compose.material3.Icon import androidx.compose.material3.ExperimentalMaterial3Api +import androidx.compose.material3.Icon import androidx.compose.material3.MaterialTheme import androidx.compose.material3.ModalBottomSheet import androidx.compose.material3.Text @@ -20,10 +22,10 @@ import androidx.compose.material3.rememberModalBottomSheetState import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf -import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.produceState -import androidx.compose.runtime.setValue import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue +import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.res.painterResource @@ -36,57 +38,59 @@ import com.ganjoor.android.data.LocalAssistant import com.ganjoor.android.data.Prompts /** - * Asks the reader's own assistant and shows what comes back. + * Asking the reader's own assistant, and showing what comes back. * * The answer isn't stored: it's a reading aid, and a model's paraphrase of a thousand-year-old * poem has no business being cached next to the poem itself. Sharing it on is one tap, for anyone * who wants to keep it. */ -@OptIn(ExperimentalMaterial3Api::class) -@Composable -fun AssistantResultSheet(prompt: String, text: String, onDismiss: () -> Unit) { - ModalBottomSheet( - onDismissRequest = onDismiss, - sheetState = rememberModalBottomSheetState(skipPartiallyExpanded = true), - ) { - AssistantAnswer(text = text, prompt = prompt) - } -} /** - * A button that asks the reader's own assistant, or hands the question to another app when no - * server is configured. Both paths are one tap and neither requires any setting up, which is the - * point: the feature is optional, so it cannot assume anyone switched it on. + * The reply, in the page. A translation belongs under the thing it translates — leaving the poem + * to read it, whether to a sheet over the top or to another app entirely, breaks the reading. + * + * With a server configured the answer arrives here. Without one there is nothing to ask, so the + * question goes to whichever assistant is installed; that hand-off is the only path that leaves + * the app, and only because the alternative is no answer at all. */ @Composable -fun AssistantAction(prompt: String, text: String, label: Int, instruction: Int) { +fun AssistantInline(prompt: String, text: String, label: Int, instruction: Int) { val assistant = LocalAssistant.current val context = LocalContext.current val ask = stringResource(instruction) - var open by remember { mutableStateOf(false) } + var asked by remember(text) { mutableStateOf(false) } - TextButton(onClick = { - if (assistant.serverReady) open = true else context.shareText("$ask\n\n$text") - }) { - Icon( - painter = painterResource(R.drawable.ic_ask), - contentDescription = null, - modifier = Modifier.size(18.dp).padding(end = 4.dp), - ) - Text(stringResource(label)) + Column(modifier = Modifier.fillMaxWidth()) { + if (!asked) { + TextButton(onClick = { + if (assistant.serverReady) asked = true else context.shareText("$ask\n\n$text") + }) { + Icon( + painter = painterResource(R.drawable.ic_ask), + contentDescription = null, + modifier = Modifier.size(18.dp).padding(end = 4.dp), + ) + Text(stringResource(label)) + } + } + AnimatedVisibility(visible = asked) { + AssistantReply(prompt = prompt, text = text) + } } - if (open) AssistantResultSheet(prompt = prompt, text = text, onDismiss = { open = false }) } -/** The same answer as a plain screen, for the selection-menu activity. */ +/** The question and its answer, with nothing around them. */ @Composable -fun AssistantAnswer(text: String, prompt: String = "translate") { +fun AssistantReply(prompt: String, text: String, modifier: Modifier = Modifier) { val settings = LocalAssistant.current val context = LocalContext.current val language = settings.language - val answer by produceState?>(null, prompt, text, language) { - value = Assistant.ask( + val key = Assistant.cacheKey(prompt, language, text) + val answer by produceState?>(Assistant.cached(key)?.let(Result.Companion::success), key) { + // Already answered once: an item scrolling back into view must not ask again. + if (value != null) return@produceState + val result = Assistant.ask( settings = settings, system = Prompts.system(language), user = when (prompt) { @@ -97,14 +101,68 @@ fun AssistantAnswer(text: String, prompt: String = "translate") { else -> text }, ) + result.getOrNull()?.let { Assistant.remember(key, it) } + value = result } - Column( - modifier = Modifier - .navigationBarsPadding() - .verticalScroll(rememberScrollState()) - .padding(horizontal = 24.dp), + Column(modifier = modifier.fillMaxWidth().padding(top = 4.dp)) { + when (val result = answer) { + null -> Row(verticalAlignment = Alignment.CenterVertically) { + CircularProgressIndicator( + strokeWidth = 2.dp, + modifier = Modifier.size(16.dp).padding(end = 8.dp), + ) + Text( + text = stringResource(R.string.assistant_working), + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } + + else -> if (result.isSuccess) { + // A reply in Urdu or English needs its own direction, not the reader's. + val reply = result.getOrDefault("") + InDirectionOf(reply) { + Text( + text = reply, + style = MaterialTheme.typography.bodyMedium, + modifier = Modifier.fillMaxWidth(), + ) + } + TextButton(onClick = { context.shareText(reply) }) { + Icon( + imageVector = Icons.Default.Share, + contentDescription = null, + modifier = Modifier.size(18.dp).padding(end = 4.dp), + ) + Text(stringResource(R.string.share)) + } + } else { + Text( + text = result.exceptionOrNull()?.message + ?: stringResource(R.string.assistant_not_set_up), + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.error, + ) + } + } + } +} + +/** The same reply as a sheet, for the selection-menu screen, which has no page to sit in. */ +@OptIn(ExperimentalMaterial3Api::class) +@Composable +fun AssistantResultSheet(prompt: String, text: String, onDismiss: () -> Unit) { + ModalBottomSheet( + onDismissRequest = onDismiss, + sheetState = rememberModalBottomSheetState(skipPartiallyExpanded = true), ) { + Column( + modifier = Modifier + .navigationBarsPadding() + .verticalScroll(rememberScrollState()) + .padding(horizontal = 24.dp), + ) { Text( text = text, style = MaterialTheme.typography.bodySmall, @@ -112,34 +170,7 @@ fun AssistantAnswer(text: String, prompt: String = "translate") { maxLines = 4, modifier = Modifier.fillMaxWidth().padding(bottom = 12.dp), ) - val result = answer - when { - result == null -> Column { - CircularProgressIndicator(modifier = Modifier.padding(bottom = 8.dp)) - Text(stringResource(R.string.assistant_working)) - } - - result.isSuccess -> { - // A reply in Urdu or English needs its own direction, not the reader's. - val reply = result.getOrDefault("") - InDirectionOf(reply) { - Text(reply, style = MaterialTheme.typography.bodyLarge) - } - TextButton(onClick = { context.shareText(reply) }) { - Icon( - imageVector = Icons.Default.Share, - contentDescription = null, - modifier = Modifier.size(18.dp).padding(end = 4.dp), - ) - Text(stringResource(R.string.share)) - } - } - - else -> Text( - text = result.exceptionOrNull()?.message - ?: stringResource(R.string.assistant_not_set_up), - color = MaterialTheme.colorScheme.error, - ) - } + AssistantReply(prompt = prompt, text = text) + } } } diff --git a/app/src/main/java/com/ganjoor/android/ui/PoemScreen.kt b/app/src/main/java/com/ganjoor/android/ui/PoemScreen.kt index b6fd91f..504eb97 100644 --- a/app/src/main/java/com/ganjoor/android/ui/PoemScreen.kt +++ b/app/src/main/java/com/ganjoor/android/ui/PoemScreen.kt @@ -16,6 +16,8 @@ import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.items import androidx.compose.foundation.text.selection.SelectionContainer import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.filled.KeyboardArrowDown +import androidx.compose.material.icons.filled.KeyboardArrowUp import androidx.compose.material.icons.filled.Share import androidx.compose.material.icons.automirrored.filled.ArrowBack import androidx.compose.material.icons.filled.Favorite @@ -53,6 +55,7 @@ import com.ganjoor.android.data.Bookmark import com.ganjoor.android.data.breadcrumbs import com.ganjoor.android.data.wordAt import com.ganjoor.android.data.Ganjoor +import com.ganjoor.android.data.LocalAssistant import com.ganjoor.android.data.LocalBookmarks import com.ganjoor.android.data.Poem import com.ganjoor.android.data.PoemRef @@ -71,7 +74,9 @@ fun PoemScreen( ) { BackHandler(onBack = onUp) - var tappedWord by remember { mutableStateOf(null) } + // The couplet travels with the word: the dictionary sheet is the one gesture every reader + // finds, so the couplet's own actions live at its foot rather than behind a tap between words. + var tapped by remember { mutableStateOf?>(null) } Load(key = fullUrl, block = { Ganjoor.poem(fullUrl) }) { poem -> val prefs = LocalSettings.current.value @@ -148,7 +153,7 @@ fun PoemScreen( style = style, showSummaries = prefs.showSummaries, source = Bookmark(fullUrl, poem.title, poem.fullTitle), - onWord = { tappedWord = it }, + onWord = { word, passage -> tapped = word to passage }, ) } @@ -181,8 +186,12 @@ fun PoemScreen( } } - tappedWord?.let { word -> - WordSheet(word = word, onDismiss = { tappedWord = null }) + tapped?.let { (word, passage) -> + WordSheet( + word = word, + passage = passage, + onDismiss = { tapped = null }, + ) } } @@ -262,17 +271,18 @@ private fun Couplet( style: androidx.compose.ui.text.TextStyle, showSummaries: Boolean, source: Bookmark, - onWord: (String) -> Unit, + onWord: (String, Bookmark) -> Unit, ) { // Tap, not long-press: long-press belongs to the text selection this sits inside. var actionsOpen by remember(couplet) { mutableStateOf(false) } + val passage = source.copy(excerpt = couplet.joinToString("\n") { it.text }) Column(modifier = Modifier.fillMaxWidth().padding(vertical = 6.dp)) { couplet.forEach { verse -> VerseText( verse = verse, style = style, - onWord = onWord, + onWord = { onWord(it, passage) }, // A tap that lands between words still opens the couplet's own actions. onElsewhere = { actionsOpen = !actionsOpen }, ) @@ -285,12 +295,49 @@ private fun Couplet( text = summary, style = MaterialTheme.typography.bodySmall, color = MaterialTheme.colorScheme.onSurfaceVariant, - modifier = Modifier.padding(top = 4.dp, bottom = 8.dp), + modifier = Modifier.padding(top = 4.dp), ) + // Ganjoor writes these in Persian. Offered only once an assistant is set up: + // a button under every couplet earns its space only if it can answer, and the + // couplet's own actions are behind a tap between words that few will find. + if (LocalAssistant.current.serverReady) { + AssistantInline( + prompt = "summary", + text = summary, + label = R.string.assistant_translate, + instruction = R.string.assistant_translate_prompt, + ) + } } } + // A visible way in. The tap between words still works, but on a full line of poetry it + // almost never lands there, so the actions were effectively unreachable without this. + Row( + modifier = Modifier.fillMaxWidth(), + horizontalArrangement = Arrangement.End, + ) { + IconButton( + onClick = { actionsOpen = !actionsOpen }, + modifier = Modifier.size(32.dp), + ) { + Icon( + imageVector = if (actionsOpen) Icons.Default.KeyboardArrowUp + else Icons.Default.KeyboardArrowDown, + contentDescription = stringResource(R.string.couplet_options), + tint = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.size(20.dp), + ) + } + } if (actionsOpen) { - PassageActions(source.copy(excerpt = couplet.joinToString("\n") { it.text })) + PassageActions(passage) + // Below the buttons rather than among them: the answer needs the full width. + AssistantInline( + prompt = "explain", + text = passage.excerpt.orEmpty(), + label = R.string.assistant_explain, + instruction = R.string.assistant_ask_prompt, + ) } } } @@ -358,7 +405,7 @@ internal fun wordTappedAt( /** Save this passage, or copy it. Saving keeps the link back to the poem; copying doesn't. */ @Composable -private fun PassageActions(passage: Bookmark) { +internal fun PassageActions(passage: Bookmark) { val bookmarks = LocalBookmarks.current val clipboard = LocalClipboardManager.current val context = LocalContext.current @@ -395,12 +442,6 @@ private fun PassageActions(passage: Bookmark) { ) Text(stringResource(R.string.share)) } - AssistantAction( - prompt = "explain", - text = passage.excerpt.orEmpty(), - label = R.string.assistant_explain, - instruction = R.string.assistant_ask_prompt, - ) } } @@ -419,7 +460,7 @@ private fun PoemSummary(summary: String) { style = MaterialTheme.typography.bodyMedium, color = MaterialTheme.colorScheme.onSurfaceVariant, ) - AssistantAction( + AssistantInline( prompt = "summary", text = summary, label = R.string.assistant_translate, diff --git a/app/src/main/java/com/ganjoor/android/ui/WordSheet.kt b/app/src/main/java/com/ganjoor/android/ui/WordSheet.kt index 4a5919f..350499b 100644 --- a/app/src/main/java/com/ganjoor/android/ui/WordSheet.kt +++ b/app/src/main/java/com/ganjoor/android/ui/WordSheet.kt @@ -2,6 +2,7 @@ package com.ganjoor.android.ui import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.ColumnScope import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.navigationBarsPadding @@ -31,7 +32,9 @@ import androidx.compose.ui.unit.LayoutDirection import androidx.compose.ui.unit.dp import com.ganjoor.android.R import com.ganjoor.android.data.Definition +import com.ganjoor.android.data.Bookmark import com.ganjoor.android.data.Dictionary +import com.ganjoor.android.data.LocalAssistant import com.ganjoor.android.data.Pronunciation import com.ganjoor.android.ui.theme.readingStyle @@ -62,11 +65,38 @@ private fun sourceLabel(source: String) = when (source) { else -> R.string.source_daneshjoo } -/** What the dictionary knows about a tapped word, as a sheet over the poem. */ +/** + * What the dictionary knows about a tapped word, as a sheet over the poem. + * + * When the word came from a couplet, that couplet's own actions sit at the foot. Tapping a word + * is the one gesture every reader finds; saving, copying or sharing the line used to be behind a + * tap that landed between words, which on a full line of poetry almost never happens. + */ @OptIn(ExperimentalMaterial3Api::class) @Composable -fun WordSheet(word: String, onDismiss: () -> Unit) { - ModalBottomSheet(onDismissRequest = onDismiss) { WordLookup(word) } +fun WordSheet(word: String, onDismiss: () -> Unit, passage: Bookmark? = null) { + ModalBottomSheet(onDismissRequest = onDismiss) { + // Inside the lookup's own scroll, not after it: the lookup scrolls, so anything placed + // below it is pushed past the bottom of the sheet with no way to reach it. + WordLookup(word) { + if (passage == null) return@WordLookup + HorizontalDivider(modifier = Modifier.padding(top = 8.dp)) + Text( + text = stringResource(R.string.this_couplet), + style = MaterialTheme.typography.labelMedium, + color = MaterialTheme.colorScheme.primary, + ) + PassageActions(passage) + if (LocalAssistant.current.serverReady) { + AssistantInline( + prompt = "explain", + text = passage.excerpt.orEmpty(), + label = R.string.assistant_explain, + instruction = R.string.assistant_ask_prompt, + ) + } + } + } } /** @@ -75,7 +105,7 @@ fun WordSheet(word: String, onDismiss: () -> Unit) { * to sit over, and a scrim with no content under it is just a grey window. */ @Composable -fun WordLookup(word: String) { +fun WordLookup(word: String, footer: @Composable ColumnScope.() -> Unit = {}) { val prefs = LocalSettings.current.value // A suggestion replaces what is being looked up, so the sheet can be followed like a trail. var current by remember(word) { mutableStateOf(word) } @@ -191,5 +221,8 @@ fun WordLookup(word: String) { } } } - } + + footer() + } + } diff --git a/app/src/main/res/values-fa/strings.xml b/app/src/main/res/values-fa/strings.xml index 78de97b..1de8dfa 100644 --- a/app/src/main/res/values-fa/strings.xml +++ b/app/src/main/res/values-fa/strings.xml @@ -105,4 +105,6 @@ این شعر فارسی را ترجمه کن و معنایش را توضیح بده: هوش مصنوعی این متن را ترجمه کن: + این بیت + گزینه‌های این بیت diff --git a/app/src/main/res/values-ur/strings.xml b/app/src/main/res/values-ur/strings.xml index eefe67c..88b6294 100644 --- a/app/src/main/res/values-ur/strings.xml +++ b/app/src/main/res/values-ur/strings.xml @@ -105,4 +105,6 @@ اس فارسی کلام کا ترجمہ کریں اور اس کا مطلب بیان کریں: مصنوعی ذہانت اس کا ترجمہ کریں: + یہ شعر + اس شعر کے اختیارات diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index b6287d8..44c7804 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -110,4 +110,6 @@ Translate this Persian poetry and explain what it means: AI assistant Translate this: + This couplet + Options for this couplet diff --git a/app/src/test/java/com/ganjoor/android/AssistantTest.kt b/app/src/test/java/com/ganjoor/android/AssistantTest.kt index 8212628..5e67eb4 100644 --- a/app/src/test/java/com/ganjoor/android/AssistantTest.kt +++ b/app/src/test/java/com/ganjoor/android/AssistantTest.kt @@ -1,8 +1,11 @@ package com.ganjoor.android import com.ganjoor.android.data.Assistant +import com.ganjoor.android.data.AssistantLanguage import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotEquals +import org.junit.Assert.assertNull import org.junit.Assert.assertThrows import org.junit.Assert.assertTrue import org.junit.Test @@ -30,6 +33,31 @@ class AssistantTest { assertEquals("the rose", Assistant.reply(body, claude = true)) } + @Test + fun `the cache keeps answers apart by prompt, language and text`() { + val a = Assistant.cacheKey("translate", AssistantLanguage.Urdu, "x") + assertEquals(a, Assistant.cacheKey("translate", AssistantLanguage.Urdu, "x")) + assertNotEquals(a, Assistant.cacheKey("explain", AssistantLanguage.Urdu, "x")) + assertNotEquals(a, Assistant.cacheKey("translate", AssistantLanguage.English, "x")) + assertNotEquals(a, Assistant.cacheKey("translate", AssistantLanguage.Urdu, "y")) + } + + /** Scrolling a long poem must not re-ask for something already answered. */ + @Test + fun `a remembered answer comes back`() { + val key = Assistant.cacheKey("summary", AssistantLanguage.Urdu, "remembered") + assertNull(Assistant.cached(key)) + Assistant.remember(key, "the answer") + assertEquals("the answer", Assistant.cached(key)) + } + + @Test + fun `the cache does not grow without bound`() { + repeat(40) { Assistant.remember("bulk-$it", "reply $it") } + assertNull(Assistant.cached("bulk-0")) + assertEquals("reply 39", Assistant.cached("bulk-39")) + } + @Test fun `an empty reply is an error, not an empty answer`() { assertThrows(IOException::class.java) { Assistant.reply("""{"choices":[]}""", false) }