From 831045305313666cd9c653351b6b90e2d3a1590b Mon Sep 17 00:00:00 2001 From: Jeff Boek Date: Mon, 15 Jun 2026 17:11:54 -0700 Subject: [PATCH] Bug 2047013 - Allows rating of summarizations --- .../feature/summarize/SummarizationAction.kt | 7 ++ .../feature/summarize/SummarizationReducer.kt | 11 +- .../feature/summarize/SummarizationScreen.kt | 2 + .../feature/summarize/SummarizationState.kt | 21 +++- .../summarize/ui/SummaryContentLoaded.kt | 101 +++++++++++++++- .../summarize/src/main/res/values/strings.xml | 6 + .../summarize/SummarizationStoreTest.kt | 108 ++++++++++++++++++ mobile/android/fenix/app/metrics.yaml | 24 ++++ .../SummarizationTelemetryMiddleware.kt | 16 +++ .../SummarizationTelemetryMiddlewareTest.kt | 28 +++++ 10 files changed, 315 insertions(+), 9 deletions(-) diff --git a/mobile/android/android-components/components/feature/summarize/src/main/java/mozilla/components/feature/summarize/SummarizationAction.kt b/mobile/android/android-components/components/feature/summarize/src/main/java/mozilla/components/feature/summarize/SummarizationAction.kt index cb90214189b26..184a23e774949 100644 --- a/mobile/android/android-components/components/feature/summarize/src/main/java/mozilla/components/feature/summarize/SummarizationAction.kt +++ b/mobile/android/android-components/components/feature/summarize/src/main/java/mozilla/components/feature/summarize/SummarizationAction.kt @@ -24,6 +24,13 @@ data class ViewDismissed(val isEngineAvailable: Boolean) : SummarizationAction /** The user tapped the settings cog. */ data object SettingsClicked : SummarizationAction +/** + * The user rated the generated summary using the feedback control. + * + * @param feedback The [SummaryFeedback] the user selected. + */ +data class SummaryFeedbackProvided(val feedback: SummaryFeedback) : SummarizationAction + /** The user tapped the back button from settings. */ data object SettingsBackClicked : SummarizationAction diff --git a/mobile/android/android-components/components/feature/summarize/src/main/java/mozilla/components/feature/summarize/SummarizationReducer.kt b/mobile/android/android-components/components/feature/summarize/src/main/java/mozilla/components/feature/summarize/SummarizationReducer.kt index 0239d0a7aab99..baff6c15d6b5f 100644 --- a/mobile/android/android-components/components/feature/summarize/src/main/java/mozilla/components/feature/summarize/SummarizationReducer.kt +++ b/mobile/android/android-components/components/feature/summarize/src/main/java/mozilla/components/feature/summarize/SummarizationReducer.kt @@ -24,11 +24,18 @@ fun summarizationReducer(state: SummarizationState, action: SummarizationAction) is SummarizationFailed -> SummarizationState.Error(SummarizationError.SummarizationFailed(action.exception)) is ReceivedParsedDocument -> state.updateDocument(action.document) is SettingsClicked -> when (state) { - is SummarizationState.Summarized -> SummarizationState.Settings(info = state.info, document = state.document) + is SummarizationState.Summarized -> + SummarizationState.Settings(info = state.info, document = state.document, feedback = state.feedback) else -> state } is SettingsBackClicked -> when (state) { - is SummarizationState.Settings -> SummarizationState.Summarized(info = state.info, document = state.document) + is SummarizationState.Settings -> + SummarizationState.Summarized(info = state.info, document = state.document, feedback = state.feedback) + else -> state + } + is SummaryFeedbackProvided -> when (state) { + is SummarizationState.Summarized -> + if (state.feedback == null) state.copy(feedback = action.feedback) else state else -> state } else -> state diff --git a/mobile/android/android-components/components/feature/summarize/src/main/java/mozilla/components/feature/summarize/SummarizationScreen.kt b/mobile/android/android-components/components/feature/summarize/src/main/java/mozilla/components/feature/summarize/SummarizationScreen.kt index 40e5bf7ea4b26..c057b92020616 100644 --- a/mobile/android/android-components/components/feature/summarize/src/main/java/mozilla/components/feature/summarize/SummarizationScreen.kt +++ b/mobile/android/android-components/components/feature/summarize/src/main/java/mozilla/components/feature/summarize/SummarizationScreen.kt @@ -194,6 +194,8 @@ private fun SummarizationScreenContent( info = state.info, document = state.document, onSettingsClicked = { store.dispatch(SettingsClicked) }, + feedback = state.feedback, + onFeedbackClicked = { store.dispatch(SummaryFeedbackProvided(it)) }, ) is SummarizationState.Settings -> { diff --git a/mobile/android/android-components/components/feature/summarize/src/main/java/mozilla/components/feature/summarize/SummarizationState.kt b/mobile/android/android-components/components/feature/summarize/src/main/java/mozilla/components/feature/summarize/SummarizationState.kt index 2b381191042ac..a72f516f24147 100644 --- a/mobile/android/android-components/components/feature/summarize/src/main/java/mozilla/components/feature/summarize/SummarizationState.kt +++ b/mobile/android/android-components/components/feature/summarize/src/main/java/mozilla/components/feature/summarize/SummarizationState.kt @@ -57,10 +57,13 @@ sealed class SummarizationState : State { * * @param info metadata about the LLM that generated the summary * @param document The generated document. + * @param feedback The rating the user gave the summary, or `null` if they have not rated it. + * The rating is single-use: once set it is never cleared, disabling further feedback. */ data class Summarized( val info: LlmProvider.Info, val document: RichDocument = RichDocument(listOf()), + val feedback: SummaryFeedback? = null, ) : SummarizationState() /** @@ -75,8 +78,13 @@ sealed class SummarizationState : State { * * @param info metadata about the LLM that generated the summary * @param document The document to return to when navigating back. + * @param feedback The rating to restore when navigating back to the summary. */ - data class Settings(val info: LlmProvider.Info, val document: RichDocument) : SummarizationState() + data class Settings( + val info: LlmProvider.Info, + val document: RichDocument, + val feedback: SummaryFeedback? = null, + ) : SummarizationState() /** User is finished with the Summarization Flow */ sealed class Finished : SummarizationState() { @@ -106,6 +114,17 @@ sealed class SummarizationError { data class SummarizationFailed(val exception: Throwable) : SummarizationError() } +/** + * The rating a user can give a generated summary via the feedback control. + */ +enum class SummaryFeedback { + /** The user found the summary helpful. */ + POSITIVE, + + /** The user found the summary unhelpful. */ + NEGATIVE, +} + val SummarizationState.isLoading get() = this is SummarizationState.Loading val SummarizationState.isSummarizing get() = this is SummarizationState.Summarizing val SummarizationState.isSummarized get() = this is SummarizationState.Summarized diff --git a/mobile/android/android-components/components/feature/summarize/src/main/java/mozilla/components/feature/summarize/ui/SummaryContentLoaded.kt b/mobile/android/android-components/components/feature/summarize/src/main/java/mozilla/components/feature/summarize/ui/SummaryContentLoaded.kt index 1216b755b35f7..df92c6c52cf49 100644 --- a/mobile/android/android-components/components/feature/summarize/src/main/java/mozilla/components/feature/summarize/ui/SummaryContentLoaded.kt +++ b/mobile/android/android-components/components/feature/summarize/src/main/java/mozilla/components/feature/summarize/ui/SummaryContentLoaded.kt @@ -4,6 +4,7 @@ package mozilla.components.feature.summarize.ui +import androidx.annotation.DrawableRes import androidx.compose.foundation.Image import androidx.compose.foundation.background import androidx.compose.foundation.layout.Column @@ -33,18 +34,28 @@ import mozilla.components.compose.base.button.IconButton import mozilla.components.compose.base.theme.AcornTheme import mozilla.components.concept.llm.LlmProvider import mozilla.components.feature.summarize.R +import mozilla.components.feature.summarize.SummaryFeedback import mozilla.components.ui.richtext.RichText import mozilla.components.ui.richtext.ir.RichDocument import mozilla.components.ui.icons.R as iconsR /** - * Content being shown after the page summary has been generated + * Content being shown after the page summary has been generated. + * + * @param document The generated summary to render. + * @param info Metadata about the LLM that generated the summary. + * @param onSettingsClicked Invoked when the user taps the settings cog. + * @param feedback The rating the user has given the summary, or `null` if unrated. + * @param onFeedbackClicked Invoked when the user rates the summary. When `null` the feedback + * control is hidden (e.g. while the summary is still streaming). */ @Composable internal fun SummaryContentLoaded( document: RichDocument, info: LlmProvider.Info, onSettingsClicked: () -> Unit = {}, + feedback: SummaryFeedback? = null, + onFeedbackClicked: ((SummaryFeedback) -> Unit)? = null, ) { Column( modifier = Modifier @@ -61,7 +72,7 @@ internal fun SummaryContentLoaded( .verticalScroll(rememberScrollState()), ) Spacer(Modifier.height(AcornTheme.layout.space.static200)) - DisclaimerMessage() + SummaryFooter(feedback = feedback, onFeedbackClicked = onFeedbackClicked) Spacer(Modifier.height(AcornTheme.layout.space.static200)) } } @@ -144,13 +155,91 @@ private fun SummarizedContent(document: RichDocument, modifier: Modifier = Modif } @Composable -private fun DisclaimerMessage() { +private fun SummaryFooter( + feedback: SummaryFeedback?, + onFeedbackClicked: ((SummaryFeedback) -> Unit)?, + modifier: Modifier = Modifier, +) { + Row( + modifier = modifier.fillMaxWidth(), + verticalAlignment = Alignment.CenterVertically, + ) { + DisclaimerMessage(modifier = Modifier.weight(1f)) + + if (onFeedbackClicked != null) { + SummaryFeedbackControl(feedback = feedback, onFeedbackClicked = onFeedbackClicked) + } + } +} + +@Composable +private fun DisclaimerMessage(modifier: Modifier = Modifier) { Text( text = stringResource(R.string.mozac_feature_summarize_disclaimer_message), fontSize = 14.sp, - modifier = Modifier - .height(24.dp) - .width(AcornTheme.layout.size.containerMaxWidth), + modifier = modifier, color = MaterialTheme.colorScheme.onSurfaceVariant, ) } + +@Composable +private fun SummaryFeedbackControl( + feedback: SummaryFeedback?, + onFeedbackClicked: (SummaryFeedback) -> Unit, + modifier: Modifier = Modifier, +) { + val hasRated = feedback != null + Row( + modifier = modifier, + verticalAlignment = Alignment.CenterVertically, + ) { + FeedbackButton( + selected = feedback == SummaryFeedback.POSITIVE, + enabled = !hasRated, + selectedIcon = iconsR.drawable.mozac_ic_thumbs_up_fill_24, + unselectedIcon = iconsR.drawable.mozac_ic_thumbs_up_24, + contentDescription = stringResource( + R.string.mozac_feature_summarize_feedback_positive_content_description, + ), + onClick = { onFeedbackClicked(SummaryFeedback.POSITIVE) }, + ) + FeedbackButton( + selected = feedback == SummaryFeedback.NEGATIVE, + enabled = !hasRated, + selectedIcon = iconsR.drawable.mozac_ic_thumbs_down_fill_24, + unselectedIcon = iconsR.drawable.mozac_ic_thumbs_down_24, + contentDescription = stringResource( + R.string.mozac_feature_summarize_feedback_negative_content_description, + ), + onClick = { onFeedbackClicked(SummaryFeedback.NEGATIVE) }, + ) + } +} + +@Composable +private fun FeedbackButton( + selected: Boolean, + enabled: Boolean, + @DrawableRes selectedIcon: Int, + @DrawableRes unselectedIcon: Int, + contentDescription: String, + onClick: () -> Unit, + modifier: Modifier = Modifier, +) { + IconButton( + onClick = onClick, + contentDescription = contentDescription, + modifier = modifier, + enabled = enabled, + ) { + Icon( + painter = painterResource(if (selected) selectedIcon else unselectedIcon), + contentDescription = null, + tint = if (selected) { + MaterialTheme.colorScheme.primary + } else { + MaterialTheme.colorScheme.onSurfaceVariant + }, + ) + } +} diff --git a/mobile/android/android-components/components/feature/summarize/src/main/res/values/strings.xml b/mobile/android/android-components/components/feature/summarize/src/main/res/values/strings.xml index 461f1b4f2a550..827ebbe8c1b69 100644 --- a/mobile/android/android-components/components/feature/summarize/src/main/res/values/strings.xml +++ b/mobile/android/android-components/components/feature/summarize/src/main/res/values/strings.xml @@ -110,6 +110,12 @@ AI can make mistakes + + Good summary + + + Bad summary + Page summaries diff --git a/mobile/android/android-components/components/feature/summarize/src/test/java/mozilla/components/feature/summarize/SummarizationStoreTest.kt b/mobile/android/android-components/components/feature/summarize/src/test/java/mozilla/components/feature/summarize/SummarizationStoreTest.kt index 86eefff48856a..33c6aca10672d 100644 --- a/mobile/android/android-components/components/feature/summarize/src/test/java/mozilla/components/feature/summarize/SummarizationStoreTest.kt +++ b/mobile/android/android-components/components/feature/summarize/src/test/java/mozilla/components/feature/summarize/SummarizationStoreTest.kt @@ -13,6 +13,7 @@ import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.toList import kotlinx.coroutines.launch import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.runTest import mozilla.components.concept.llm.CloudLlmProvider import mozilla.components.concept.llm.Llm @@ -40,6 +41,7 @@ import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import kotlin.time.Duration.Companion.seconds +import mozilla.components.feature.summarize.SummarizationState.Settings as SettingsState class SummarizationStoreTest { @@ -635,4 +637,110 @@ class SummarizationStoreTest { ) assertEquals(expected, states) } + + @Test + fun `when feedback is provided on a summary, it is stored`() = runTest { + val provider = FakeCloudProvider(preparedState = CloudLlmProvider.State.Ready(FakeLlm.successful)) + val pageTitle = "Article Headline" + val store = summarizedStore(provider, pageTitle) + + val states = mutableListOf() + backgroundScope.launch { + store.stateFlow.toList(states) + } + + store.dispatch(ViewAppeared) + testScheduler.advanceTimeBy(15.seconds) + store.dispatch(SummaryFeedbackProvided(SummaryFeedback.POSITIVE)) + testScheduler.advanceTimeBy(1.seconds) + + val document = parser.parse("# $pageTitle\nThis is the article\nThis is some content...\nThis is some *bold* content.\n") + assertEquals(Summarized(provider.info, document, SummaryFeedback.POSITIVE), states.last()) + } + + @Test + fun `when feedback has already been provided, providing it again is ignored`() = runTest { + val provider = FakeCloudProvider(preparedState = CloudLlmProvider.State.Ready(FakeLlm.successful)) + val pageTitle = "Article Headline" + val store = summarizedStore(provider, pageTitle) + + val states = mutableListOf() + backgroundScope.launch { + store.stateFlow.toList(states) + } + + store.dispatch(ViewAppeared) + testScheduler.advanceTimeBy(15.seconds) + store.dispatch(SummaryFeedbackProvided(SummaryFeedback.POSITIVE)) + testScheduler.advanceTimeBy(1.seconds) + store.dispatch(SummaryFeedbackProvided(SummaryFeedback.NEGATIVE)) + testScheduler.advanceTimeBy(1.seconds) + + val document = parser.parse("# $pageTitle\nThis is the article\nThis is some content...\nThis is some *bold* content.\n") + assertEquals(Summarized(provider.info, document, SummaryFeedback.POSITIVE), states.last()) + } + + @Test + fun `when feedback is provided before a summary is ready, the state is unchanged`() = runTest { + val hangingLlm = object : Llm { + override suspend fun prompt(prompt: Prompt): Flow = flow { awaitCancellation() } + } + val provider = FakeCloudProvider(preparedState = CloudLlmProvider.State.Ready(hangingLlm)) + val store = summarizedStore(provider, "Article Headline") + + val states = mutableListOf() + backgroundScope.launch { + store.stateFlow.toList(states) + } + + store.dispatch(ViewAppeared) + testScheduler.advanceTimeBy(1.seconds) + store.dispatch(SummaryFeedbackProvided(SummaryFeedback.POSITIVE)) + testScheduler.advanceTimeBy(1.seconds) + + assertEquals(Loading(provider.info), states.last()) + } + + @Test + fun `feedback is preserved when navigating to settings and back`() = runTest { + val provider = FakeCloudProvider(preparedState = CloudLlmProvider.State.Ready(FakeLlm.successful)) + val pageTitle = "Article Headline" + val store = summarizedStore(provider, pageTitle) + + val states = mutableListOf() + backgroundScope.launch { + store.stateFlow.toList(states) + } + + store.dispatch(ViewAppeared) + testScheduler.advanceTimeBy(15.seconds) + store.dispatch(SummaryFeedbackProvided(SummaryFeedback.NEGATIVE)) + testScheduler.advanceTimeBy(1.seconds) + store.dispatch(SettingsClicked) + testScheduler.advanceTimeBy(1.seconds) + store.dispatch(SettingsBackClicked) + testScheduler.advanceTimeBy(1.seconds) + + val document = parser.parse("# $pageTitle\nThis is the article\nThis is some content...\nThis is some *bold* content.\n") + assertTrue(states.contains(SettingsState(provider.info, document, SummaryFeedback.NEGATIVE))) + assertEquals(Summarized(provider.info, document, SummaryFeedback.NEGATIVE), states.last()) + } + + private fun TestScope.summarizedStore( + provider: FakeCloudProvider, + pageTitle: String, + ) = SummarizationStore( + initialState = Inert(true), + reducer = ::summarizationReducer, + middleware = listOf( + SummarizationMiddleware( + llmProvider = provider, + settings = SummarizationSettings.inMemory(hasConsentedToShake = true), + contentProvider = { Result.success(Content(PageMetadata(pageTitle = pageTitle))) }, + errorReporter = noopReporter, + scope = backgroundScope, + dispatcher = StandardTestDispatcher(testScheduler), + ), + ), + ) } diff --git a/mobile/android/fenix/app/metrics.yaml b/mobile/android/fenix/app/metrics.yaml index 8d5d8accd918d..5a6ebfc7be50d 100644 --- a/mobile/android/fenix/app/metrics.yaml +++ b/mobile/android/fenix/app/metrics.yaml @@ -15596,6 +15596,30 @@ ai_summarize: notification_emails: - android-probes@mozilla.com expires: never + feedback: + type: event + description: | + Recorded when the user rates a generated summary using the thumbs + up/down feedback control. Recorded at most once per summarization, as + the control is disabled after the first rating. + extra_keys: + rating: + description: | + The user's rating of the summary. Possible values: + "positive", "negative". + type: string + model: + description: The name of the model used for summarization. + type: string + bugs: + - https://bugzilla.mozilla.org/show_bug.cgi?id=2047013 + data_reviews: + - https://bugzilla.mozilla.org/show_bug.cgi?id=2047013 + data_sensitivity: + - interaction + notification_emails: + - android-probes@mozilla.com + expires: never duration: type: timing_distribution time_unit: millisecond diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/summarization/SummarizationTelemetryMiddleware.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/summarization/SummarizationTelemetryMiddleware.kt index dc9f19d12ea5a..9929c05345955 100644 --- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/summarization/SummarizationTelemetryMiddleware.kt +++ b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/summarization/SummarizationTelemetryMiddleware.kt @@ -13,6 +13,8 @@ import mozilla.components.feature.summarize.SummarizationCompleted import mozilla.components.feature.summarize.SummarizationFailed import mozilla.components.feature.summarize.SummarizationRequested import mozilla.components.feature.summarize.SummarizationState +import mozilla.components.feature.summarize.SummaryFeedback +import mozilla.components.feature.summarize.SummaryFeedbackProvided import mozilla.components.feature.summarize.ViewAppeared import mozilla.components.feature.summarize.ViewDismissed import mozilla.components.feature.summarize.content.Content @@ -81,6 +83,7 @@ class SummarizationTelemetryMiddleware( is ContentExtracted -> handleExtractedContent(action.content) is SummarizationCompleted -> recordSummarizationCompleted() is SummarizationFailed -> recordSummarizationCompleted(success = false, action.exception) + is SummaryFeedbackProvided -> recordFeedback(action.feedback) is ViewDismissed -> { AiSummarize.closed.record( AiSummarize.ClosedExtra( @@ -154,6 +157,19 @@ class SummarizationTelemetryMiddleware( ) } + private fun recordFeedback(feedback: SummaryFeedback) { + val rating = when (feedback) { + SummaryFeedback.POSITIVE -> "positive" + SummaryFeedback.NEGATIVE -> "negative" + } + AiSummarize.feedback.record( + AiSummarize.FeedbackExtra( + model = sessionTelemetry.model, + rating = rating, + ), + ) + } + /** * Identifier for the failure in telemetry. For [Llm.Exception] subtypes we log the qualified * class name so provider attribution survives (e.g. MLPA's `RateLimited` vs a hypothetical diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/summarization/SummarizationTelemetryMiddlewareTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/summarization/SummarizationTelemetryMiddlewareTest.kt index 0615e74af45d0..90b8c41b19bf8 100644 --- a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/summarization/SummarizationTelemetryMiddlewareTest.kt +++ b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/summarization/SummarizationTelemetryMiddlewareTest.kt @@ -15,6 +15,8 @@ import mozilla.components.feature.summarize.SummarizationCompleted import mozilla.components.feature.summarize.SummarizationFailed import mozilla.components.feature.summarize.SummarizationRequested import mozilla.components.feature.summarize.SummarizationState +import mozilla.components.feature.summarize.SummaryFeedback +import mozilla.components.feature.summarize.SummaryFeedbackProvided import mozilla.components.feature.summarize.ViewAppeared import mozilla.components.feature.summarize.ViewDismissed import mozilla.components.feature.summarize.content.Content @@ -274,6 +276,32 @@ class SummarizationTelemetryMiddlewareTest { assertEquals("CELLULAR", extras["connection_type"]) } + @Test + fun `WHEN positive feedback is provided THEN feedback is recorded with rating positive and model`() { + assertNull(AiSummarize.feedback.testGetValue()) + + setupFullSession() + invokeMiddleware(SummaryFeedbackProvided(SummaryFeedback.POSITIVE)) + + val snapshot = AiSummarize.feedback.testGetValue()!! + assertEquals(1, snapshot.size) + + val extras = snapshot.first().extra!! + assertEquals("positive", extras["rating"]) + assertEquals(TEST_MODEL, extras["model"]) + } + + @Test + fun `WHEN negative feedback is provided THEN feedback is recorded with rating negative`() { + assertNull(AiSummarize.feedback.testGetValue()) + + setupFullSession() + invokeMiddleware(SummaryFeedbackProvided(SummaryFeedback.NEGATIVE)) + + val extras = AiSummarize.feedback.testGetValue()!!.first().extra!! + assertEquals("negative", extras["rating"]) + } + private fun setupFullSession() { every { store.state } returns SummarizationState.Inert(initializedWithShake = false) invokeMiddleware(ViewAppeared)