From 5daebcd87a034088ad2e36a950905682e945a991 Mon Sep 17 00:00:00 2001 From: Anas Rashid Date: Sun, 4 Oct 2026 19:49:15 +0200 Subject: [PATCH] Show the assistant's answers in the page, and make a couplet's options findable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Translations appear under the summary they translate. Leaving the poem to read one — to a sheet over the top, or to another app entirely — breaks the reading, which is the thing the feature is meant to help with. Ganjoor prints a summary under each couplet as well as under the poem, so both now carry a translate button, the couplet one only once a server is configured: a button under every couplet earns its space only if it can answer. A couplet's own actions were effectively unreachable. They opened on a tap that landed between words, and on a full line of poetry a tap almost always lands on a word, which opens the dictionary instead — three attempts here failed before one worked. There is now a chevron at the end of every couplet, and the same actions sit at the foot of the word sheet, inside its scroll rather than after it, where they were pushed past the bottom of the sheet with no way to reach them. Answers are remembered. A LazyColumn disposes what scrolls out of view, which restarted the request behind it: one tap on translate sent two, confirmed against a stub server, and scrolling away and back would have sent more. On a metered API that is money. Co-Authored-By: Claude Opus 5 (1M context) --- .../com/ganjoor/android/data/Assistant.kt | 21 +++ .../android/ui/AssistantResultSheet.kt | 167 +++++++++++------- .../java/com/ganjoor/android/ui/PoemScreen.kt | 73 ++++++-- .../java/com/ganjoor/android/ui/WordSheet.kt | 43 ++++- app/src/main/res/values-fa/strings.xml | 2 + app/src/main/res/values-ur/strings.xml | 2 + app/src/main/res/values/strings.xml | 2 + .../java/com/ganjoor/android/AssistantTest.kt | 28 +++ 8 files changed, 249 insertions(+), 89 deletions(-) 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) }