diff --git a/AGENTS.md b/AGENTS.md index 9760f76..435ac11 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -259,6 +259,14 @@ first if a remote spawn ever mangles an argument. card that remembered for itself forgets the moment the lazy list stops composing it, so a note opened and scrolled past would shut behind the reader. +- **The full-screen image lives on the screen, not in the row that drew the + thumbnail** (`SessionImageViewer`). A `Read` whose result is an image is a + row of one call until the next call arrives and makes it a group -- a + different composable in a different part of the tree, so the old subtree + and everything it remembered goes, the open dialog included. Somebody + looking at a screenshot was thrown back to the transcript because the + session made another tool call. `/tools n gap` puts an image on its first + call so this is reproducible: open it, wait a gap, watch the row regroup. - **All transcript text is selectable, from one `SelectionContainer` around the whole list** (`TranscriptList.kt`). Not per row: a transcript is one body of text to a reader, so a selection has to be able to run from a diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionImage.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionImage.kt index e44b2d5..027be8f 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionImage.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionImage.kt @@ -78,12 +78,18 @@ fun rememberSessionBitmap(settings: ServerSettings, sessionId: String, ref: Stri * is what it should be. * * Four lines of body text, so a screenshot reads as an attachment beside the conversation rather - * than as a page of its own. Full size is one tap away. + * than as a page of its own. Full size is one tap away -- but the full-size view itself is not + * here. [onOpen] hands the ref to the screen, which draws [SessionImageViewer] outside the list; + * see that function for the reason. */ @Composable -fun SessionImage(settings: ServerSettings, sessionId: String, ref: String) { +fun SessionImage( + settings: ServerSettings, + sessionId: String, + ref: String, + onOpen: (String) -> Unit, +) { val (bitmap, failed) = rememberSessionBitmap(settings, sessionId, ref) - var full by remember(ref) { mutableStateOf(false) } val height = thumbnailHeight() val heightPx = with(LocalDensity.current) { height.roundToPx() } Box(Modifier.fillMaxWidth().height(height), contentAlignment = Alignment.CenterStart) { @@ -102,12 +108,60 @@ fun SessionImage(settings: ServerSettings, sessionId: String, ref: String) { contentDescription = "Attached image, tap to view full screen", contentScale = ContentScale.Fit, filterQuality = enlargingFilter(image.height, heightPx), - modifier = Modifier.fillMaxSize().clickable { full = true }, + modifier = Modifier.fillMaxSize().clickable { onOpen(ref) }, alignment = Alignment.CenterStart, ) } } - if (full) bitmap?.let { image -> ImageViewer(image) { full = false } } +} + +/** + * The image somebody opened, drawn by the screen rather than by the row it was tapped in. + * + * The row is the wrong place to hold this, and it took a real fault to see why: an image from a + * `Read` on its own is a row of one call, and the moment the next call arrives the two become a + * group -- a different composable in a different part of the tree, so everything the old subtree + * remembered goes, the dialog included. Somebody looking at a screenshot was thrown back to the + * transcript because the session made another tool call. The same happens to a row regrouped by a + * page of history landing. + * + * Held by the screen, none of that reaches it: what is open is a property of the screen, not of + * whichever row happened to draw the thumbnail. + * + * The cost is one fetch, since the thumbnail's decoded bitmap belongs to a row this does not go + * through. Paid deliberately rather than plumbed around: it is one request for a picture somebody + * asked to see, and the loading and unavailable states below are the same two the thumbnail draws. + */ +@Composable +fun SessionImageViewer( + settings: ServerSettings, + sessionId: String, + ref: String, + onClose: () -> Unit, +) { + val (bitmap, failed) = rememberSessionBitmap(settings, sessionId, ref) + Dialog( + onDismissRequest = onClose, + properties = DialogProperties(usePlatformDefaultWidth = false), + ) { + Box( + Modifier.fillMaxSize().background(Color.Black).clickable(onClick = onClose), + contentAlignment = Alignment.Center, + ) { + when (val image = bitmap) { + // Two states, not one, exactly as the thumbnail has them: still coming, and never + // coming. Stated in white because this box paints its own black behind them and a + // theme colour would be picked against a surface that is not there. + null -> + Text( + if (failed) "Image $ref is unavailable" else "Loading image…", + color = Color.White, + style = MaterialTheme.typography.bodyMedium, + ) + else -> ZoomableImage(image) + } + } + } } /** @@ -139,23 +193,23 @@ private fun enlargingFilter(sourceHeight: Int, drawnHeight: Int): FilterQuality /** * The image on its own, as large as it fits, with pinch to zoom. * - * A dialog rather than a screen, so the platform's back gesture returns to the transcript instead - * of leaving the app. It opens fitted -- the whole image visible, which is the thing a reader wants - * first -- and zoom is theirs from there. + * Inside a dialog rather than a screen -- see [SessionImageViewer] -- so the platform's back + * gesture returns to the transcript instead of leaving the app. It opens fitted, the whole image + * visible, which is the thing a reader wants first; zoom is theirs from there. */ @Composable -private fun ImageViewer(image: ImageBitmap, onClose: () -> Unit) { - Dialog( - onDismissRequest = onClose, - properties = DialogProperties(usePlatformDefaultWidth = false), - ) { - var scale by remember { mutableFloatStateOf(1f) } - var offsetX by remember { mutableFloatStateOf(0f) } - var offsetY by remember { mutableFloatStateOf(0f) } - Box( +private fun ZoomableImage(image: ImageBitmap) { + var scale by remember { mutableFloatStateOf(1f) } + var offsetX by remember { mutableFloatStateOf(0f) } + var offsetY by remember { mutableFloatStateOf(0f) } + Image( + bitmap = image, + contentDescription = "Attached image", + contentScale = ContentScale.Fit, + // Zoomed in, the reader is looking at pixels on purpose. + filterQuality = FilterQuality.None, + modifier = Modifier.fillMaxSize() - .background(Color.Black) - .clickable(onClick = onClose) .pointerInput(Unit) { detectTransformGestures { _, pan, zoom, _ -> // Floor of 1 so the image cannot be pinched smaller than fitted, which is @@ -169,23 +223,12 @@ private fun ImageViewer(image: ImageBitmap, onClose: () -> Unit) { offsetY = 0f } } + } + .graphicsLayer { + scaleX = scale + scaleY = scale + translationX = offsetX + translationY = offsetY }, - contentAlignment = Alignment.Center, - ) { - Image( - bitmap = image, - contentDescription = "Attached image", - contentScale = ContentScale.Fit, - // Zoomed in, the reader is looking at pixels on purpose. - filterQuality = FilterQuality.None, - modifier = - Modifier.fillMaxSize().graphicsLayer { - scaleX = scale - scaleY = scale - translationX = offsetX - translationY = offsetY - }, - ) - } - } + ) } diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt index 508cd48..d03afce 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt @@ -264,6 +264,10 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () // like everything else new in this transcript, and held here rather than in the card so a // note opened and scrolled past is still open on the way back. var openMemories by remember { mutableStateOf(setOf()) } + // The image being looked at full screen, by ref. Here rather than in the row that drew the + // thumbnail: a row regrouped underneath the reader takes its whole subtree with it, and the + // dialog with it -- see [SessionImageViewer]. + var fullImage by remember { mutableStateOf(null) } // Uploaded-but-not-yet-sent attachment ids; sent with the next message. var pendingAttachments by remember { mutableStateOf(listOf()) } // What this session is set to now, seeded from the row that opened it and @@ -930,6 +934,11 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () } } + /** Opens one image full screen, from whichever row drew it; see [SessionImageViewer]. */ + fun openImage(ref: String) { + fullImage = ref + } + /** Opens or closes one memory note, wherever it is drawn; see [MemoryNote]. */ fun toggleMemory(text: String) { openMemories = if (text in openMemories) openMemories - text else openMemories + text @@ -1205,6 +1214,7 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () sessionId = summary.id, text = waiting.text, images = waiting.images, + onOpenImage = ::openImage, pending = true, refusal = waiting.refusal, // The bubble goes away on the `messageDropped` this @@ -1315,7 +1325,12 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () } }, image = { ref -> - SessionImage(settings, summary.id, ref) + SessionImage( + settings, + summary.id, + ref, + ::openImage, + ) }, ) is TranscriptRow.Single -> @@ -1326,6 +1341,7 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () sessionId = summary.id, text = item.text, images = item.images, + onOpenImage = ::openImage, ) is TranscriptItem.AssistantMsg -> // A whole assistant row is only ever the reply @@ -1368,7 +1384,12 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () } }, image = { ref -> - SessionImage(settings, summary.id, ref) + SessionImage( + settings, + summary.id, + ref, + ::openImage, + ) }, ) is TranscriptItem.QuestionCard -> @@ -1389,7 +1410,12 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () style = MaterialTheme.typography.bodyMedium, ) is TranscriptItem.ImageItem -> - SessionImage(settings, summary.id, item.ref) + SessionImage( + settings, + summary.id, + item.ref, + ::openImage, + ) is TranscriptItem.Note -> Text( item.text, @@ -1689,6 +1715,9 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () } } + // Beside the other two dialogs, and outside the list for the same reason as them: what is + // open is the screen's business rather than any row's. See [SessionImageViewer]. + fullImage?.let { ref -> SessionImageViewer(settings, summary.id, ref) { fullImage = null } } if (usageOpen) { UsageDialog(settings = settings, onDismiss = { usageOpen = false }) } @@ -1762,6 +1791,7 @@ private fun UserBubble( sessionId: String, text: String, images: List = emptyList(), + onOpenImage: (String) -> Unit, pending: Boolean = false, refusal: String? = null, onTakeBack: (() -> Unit)? = null, @@ -1808,7 +1838,7 @@ private fun UserBubble( // same place down the transcript, whether or not there is an image in it. images.forEachIndexed { index, ref -> if (index > 0 || text.isNotEmpty()) Spacer(Modifier.height(4.dp)) - SessionImage(settings, sessionId, ref) + SessionImage(settings, sessionId, ref, onOpenImage) } refusal?.let { Spacer(Modifier.height(6.dp)) diff --git a/server/src/session/echo.rs b/server/src/session/echo.rs index 6e57e8f..877518f 100644 --- a/server/src/session/echo.rs +++ b/server/src/session/echo.rs @@ -11,7 +11,9 @@ //! looks like when a screen groups them. `gap` is seconds between one //! call and the next, default none: it is what makes a run *grow* while //! somebody is looking at it, which is the only way to reach the state -//! where a call opened on its own gains a neighbour. +//! where a call opened on its own gains a neighbour. The first call +//! carries a screenshot, so that state can also be reached with an image +//! open full screen -- which is where it used to close itself. //! - `/question [text]` -- a question, exercising the answer path. //! - `/ask` -- an AskUserQuestion call: two questions on one tool call, //! with descriptions, a preview and a multi-select, which is the shape @@ -482,6 +484,27 @@ impl EchoDriver { "timeout": 5000, }), }); + // The first call carries a screenshot, and only the + // first. That is what makes this rig cover the case a + // growing run is actually about: an image opened full + // screen from a call that is alone, and then a second + // call arriving and turning that row into a group. The + // dialog used to be inside the row, so the reader was + // thrown back to the transcript by the session making + // another tool call. Any of the calls would do; the + // first is the one that is on its own for a whole + // `gap`, which is the window somebody can open it in. + if i == 1 { + let part = serde_json::json!({ + "source": {"media_type": "image/png", "data": SAMPLE_PNG} + }); + if let Some(name) = super::claude::translate::save_image(&dir, &part) { + send(Event::Image { + image: name, + about: Some(id.clone()), + }); + } + } tokio::time::sleep(DELTA_DELAY).await; send(Event::ToolEnd { id,