Keep the image on screen when its tool call joins a group
A `Read` that returns an image is a row of one call, and the moment the session makes its next call the two become a group -- which is a different composable in a different part of the tree, so the old subtree goes and everything it remembered goes with it. The full-screen viewer was inside that subtree, so somebody looking at a screenshot was thrown back to the transcript because the session carried on working. A page of history landing does the same thing to the same row. What is open is a property of the screen rather than of whichever row happened to draw the thumbnail, so it is held there now and drawn beside the other two dialogs. Nothing that happens to rows can reach it. The cost is one fetch when it opens, since the thumbnail's decoded bitmap belongs to a row this no longer goes through. Paid deliberately rather than plumbed around: it is one request for a picture somebody asked to see, and the viewer draws the same two empty states the thumbnail does -- still coming, and never coming -- which it previously could not have, since it only ever opened on a bitmap already in hand. `/tools n gap` now puts a screenshot on its first call, so the case is reproducible rather than argued about: that command already existed to make a run *grow* while somebody watches, and the image is what made growing matter. Checked on the emulator with `/tools 3 30` -- opened the image on the lone call, and it was still open a minute later with the row by then inside a group of three, and back returned to the transcript rather than leaving the app.
This commit is contained in:
1 parent
ed88bdb31f
commit
bfaf5e6f38
4 files changed
+138
-34
No files matched your search
@@ -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
|
||||
|
||||
@@ -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),
|
||||
) {
|
||||
private fun ZoomableImage(image: ImageBitmap) {
|
||||
var scale by remember { mutableFloatStateOf(1f) }
|
||||
var offsetX by remember { mutableFloatStateOf(0f) }
|
||||
var offsetY by remember { mutableFloatStateOf(0f) }
|
||||
Box(
|
||||
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
|
||||
}
|
||||
}
|
||||
},
|
||||
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 {
|
||||
}
|
||||
.graphicsLayer {
|
||||
scaleX = scale
|
||||
scaleY = scale
|
||||
translationX = offsetX
|
||||
translationY = offsetY
|
||||
},
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -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<String>()) }
|
||||
// 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<String?>(null) }
|
||||
// Uploaded-but-not-yet-sent attachment ids; sent with the next message.
|
||||
var pendingAttachments by remember { mutableStateOf(listOf<String>()) }
|
||||
// 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<String> = 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))
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in new issue
Block a user