Merge branch 'main' of git.arirex.me:iris/ai-app

TODO.md

@@ -5,16 +5,29 @@ one in place when it turns out to need a decision.
 
 ## App — transcript
 
-- [ ] Tapping an expanded agent message card should close it. — a tap that
+- [ ] Text inside code blocks does not highlight when selected. **Measured, and
-      clears a selection now spends itself on that and nothing else
+      it does** — the selection is drawn, but over the near-black surface a code
-      (`SessionScreen.expanding`); still to check on the emulator whether a tap
+      block and a tool's output sit on, Material's default 40%-alpha tint
-      with *no* selection reaches an opened peer card at all.
+      composites to a barely-there smudge, much weaker than the same selection
-- [ ] Text inside code blocks does not highlight when selected (selection
+      over a reply. The app now states its own selection colours
-      itself works — only the highlight is missing). Waiting on the app-c7
+      (`AiAppSelectionColors`), which took the fill from #5B4C73 to #776394 on
-      session, which is replacing the highlighter (`HIGHLIGHTER_PLAN.md`) and
+      that surface. Worth confirming this was the complaint rather than a
-      owns `CodeFence.kt` until it pushes; it confirmed this item is ours.
+      selection that draws *nothing* on the phone.
+- [ ] Text inside an opened peer message or memory note cannot be selected at
+      all — the heading of the same card can, and so can a tool call's output,
+      so it is the markdown text specifically. Pre-existing (measured against
+      the build before this session's changes, by stashing them). It
+      contradicts AGENTS.md's "all transcript text is selectable".
 - [ ] Messages received from other agents are inconsistent — sometimes they
-      appear, sometimes they don't.
+      appear, sometimes they don't. **Needs a rig.** Read the code rather than
+      measured: a live Claude session only learns of a peer message from the
+      `origin` object on a turn's `result`
+      (`session/claude/translate.rs`), which the CLI attaches to a turn the
+      message *started*. So a message that arrives mid-turn, or a second one
+      within one turn, has nowhere to be reported — while an imported session,
+      which syncs from the CLI's own file, picks up every one of them. That
+      would show exactly as "sometimes". Confirming it means driving a real
+      stream-json session and sending it messages in both states.
 
 ## Session settings
 

app/androidApp/src/main/kotlin/com/example/aiapp/Ansi.kt

@@ -71,8 +71,10 @@ fun ansiStyled(text: String, palette: AnsiPalette): AnnotatedString {
                         if (final == 'm') sgr = sgr.apply(params, palette)
                     }
             }
-            // A bare carriage return rewrites the line; one before a newline is the other half of
+            // A bare carriage return rewrites the line. One before a newline is the other half
-            // a Windows line ending and has nothing to rewrite.
+            // of a Windows line ending: it rewrites nothing, and it is dropped rather than kept,
+            // since that pair is one line break and the return itself would draw as a stray
+            // control character.
             c == '\r' && text.getOrNull(at + 1) != '\n' -> {
                 flush()
                 dropLine(runs)

app/androidApp/src/main/kotlin/com/example/aiapp/MainActivity.kt

@@ -13,8 +13,10 @@ import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.fillMaxSize
 import androidx.compose.foundation.layout.navigationBarsPadding
 import androidx.compose.foundation.layout.statusBarsPadding
+import androidx.compose.foundation.text.selection.LocalTextSelectionColors
 import androidx.compose.material3.MaterialTheme
 import androidx.compose.material3.Surface
+import androidx.compose.runtime.CompositionLocalProvider
 import androidx.compose.runtime.getValue
 import androidx.compose.runtime.mutableIntStateOf
 import androidx.compose.runtime.mutableStateOf
@@ -92,7 +94,12 @@ class MainActivity : ComponentActivity() {
         NotificationService.sync(this)
 
         setContent {
+            // Selection colours with the theme rather than at each place text is drawn: the
+            // transcript is one selection container, and a selection that ran from a reply into
+            // the code block under it would otherwise change colour halfway. See
+            // [AiAppSelectionColors].
             MaterialTheme(colorScheme = AiAppColors) {
+                CompositionLocalProvider(LocalTextSelectionColors provides AiAppSelectionColors) {
                     Surface(modifier = Modifier.fillMaxSize()) {
                         Box(
                             modifier =
@@ -149,6 +156,7 @@ class MainActivity : ComponentActivity() {
                 }
             }
         }
+    }
 
     // launchMode="singleTop": an enrollment scan, or a notification tapped
     // while the app is open, lands here rather than in a second activity

app/androidApp/src/main/kotlin/com/example/aiapp/MarkdownLinks.kt

@@ -1,9 +1,13 @@
 package com.example.aiapp
 
-import androidx.compose.foundation.gestures.detectTapGestures
+import androidx.compose.foundation.gestures.awaitEachGesture
+import androidx.compose.foundation.gestures.awaitFirstDown
+import androidx.compose.foundation.gestures.waitForUpOrCancellation
 import androidx.compose.foundation.text.BasicText
 import androidx.compose.runtime.Composable
+import androidx.compose.runtime.compositionLocalOf
 import androidx.compose.runtime.remember
+import androidx.compose.runtime.rememberUpdatedState
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.geometry.Offset
 import androidx.compose.ui.graphics.isSpecified
@@ -88,17 +92,41 @@ fun LinkedText(content: String, node: ASTNode, style: TextStyle, modifier: Modif
             content.buildMarkdownAnnotatedString(node, style, settings)
         }
     val uriHandler = LocalUriHandler.current
+    val onPlainTap = LocalMarkdownTap.current
     val layout = remember { Ref<TextLayoutResult>() }
     // The renderer's own rule for a style that names no colour: the theme's text colour.
     val color = if (style.color.isSpecified) style.color else LocalMarkdownColors.current.text
     BasicText(
         text = text,
         modifier =
-            modifier.pointerInput(text) {
+            // A tap here is either a link or the card's; see [LocalMarkdownTap] for why the
-                detectTapGestures { position ->
+            // second one has to be answered from inside the text rather than left to the card.
-                    val url = text.linkAt(layout.value, position) ?: return@detectTapGestures
+            modifier.pointerInput(text, onPlainTap) {
+                awaitEachGesture {
+                    // Unconsumed is not required: something outside may already be tracking this
+                    // press, and it is still the press that may land on a link.
+                    awaitFirstDown(requireUnconsumed = false)
+                    // A tap and nothing else. Null when the gesture became something somebody
+                    // else's -- a scroll, or a press held past the long-press timeout, which is
+                    // how a selection starts. The timeout is the load-bearing half: without it a
+                    // press held for a second and released was still an up with nothing consumed,
+                    // so holding a peer message to select from it shut the card instead.
+                    val up =
+                        withTimeoutOrNull(viewConfiguration.longPressTimeoutMillis) {
+                            waitForUpOrCancellation()
+                        } ?: return@awaitEachGesture
+                    val url = text.linkAt(layout.value, up.position)
+                    when {
+                        url != null -> {
+                            up.consume()
                             uriHandler.openUri(url)
                         }
+                        onPlainTap != null -> {
+                            up.consume()
+                            onPlainTap()
+                        }
+                    }
+                }
             },
         style = style,
         color = { color },
@@ -106,6 +134,39 @@ fun LinkedText(content: String, node: ASTNode, style: TextStyle, modifier: Modif
     )
 }
 
+/**
+ * What a tap on markdown text means when it lands on no link -- shutting the card it is drawn in,
+ * usually -- or null where a plain tap means nothing.
+ *
+ * A composition local because there is nowhere else to put it. The paragraphs of a message are
+ * composed by the renderer's own dispatch out of its component table, so nothing between a card and
+ * the text inside it is ours to pass a parameter through; the renderer already hands its colours,
+ * its typography and its components down the same way.
+ *
+ * It exists because a pointer-input node over the glyphs takes the tap and the card's own click
+ * handler never sees it. Measured on the emulator against an opened peer message: with a handler on
+ * the text -- consuming or not -- a tap on its words did nothing at all, and with the handler
+ * removed entirely the same tap shut the card. So a card whose body is markdown cannot be shut by
+ * pressing its words unless the words do the shutting, and "nothing happens when I press it" is
+ * indistinguishable from a card that has stopped working.
+ *
+ * Provided as a value that outlives a recomposition (see [rememberMarkdownTap]), since a fresh
+ * lambda per composition would invalidate every paragraph reading it.
+ */
+val LocalMarkdownTap = compositionLocalOf<(() -> Unit)?> { null }
+
+/**
+ * [onTap] as a stable value to provide for [LocalMarkdownTap].
+ *
+ * The identity stays put while the behaviour follows the latest [onTap], which is what keeps
+ * providing it from invalidating the text under it on every recomposition of the card.
+ */
+@Composable
+fun rememberMarkdownTap(onTap: () -> Unit): () -> Unit {
+    val latest = rememberUpdatedState(onTap)
+    return remember { { latest.value() } }
+}
+
 /**
  * The address under [position], if a link's glyph is there rather than merely nearest to it.
  *

app/androidApp/src/main/kotlin/com/example/aiapp/MemoryNote.kt

@@ -12,6 +12,7 @@ import androidx.compose.material3.Card
 import androidx.compose.material3.MaterialTheme
 import androidx.compose.material3.Text
 import androidx.compose.runtime.Composable
+import androidx.compose.runtime.CompositionLocalProvider
 import androidx.compose.runtime.remember
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
@@ -123,7 +124,12 @@ fun MemoryNote(
                     )
                 }
             }
-            if (expanded) MarkdownText(note.text, replies, Modifier.padding(top = 4.dp))
+            // The words shut the card too, and have to do it themselves -- see [LocalMarkdownTap].
+            if (expanded) {
+                CompositionLocalProvider(LocalMarkdownTap provides rememberMarkdownTap(onToggle)) {
+                    MarkdownText(note.text, replies, Modifier.padding(top = 4.dp))
+                }
+            }
         }
     }
 }

app/androidApp/src/main/kotlin/com/example/aiapp/PeerMessage.kt

@@ -13,6 +13,7 @@ import androidx.compose.material3.CardDefaults
 import androidx.compose.material3.MaterialTheme
 import androidx.compose.material3.Text
 import androidx.compose.runtime.Composable
+import androidx.compose.runtime.CompositionLocalProvider
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.draw.clip
@@ -87,11 +88,16 @@ fun PeerBlockRow(unit: TranscriptUnit.PeerBlock, replies: ParsedReplies, onToggl
             onPress = onToggle,
         )
     ) {
-        // The gap the card's own column used to provide between its heading and its prose, and
+        // The words shut the card too, and have to do it themselves -- see [LocalMarkdownTap].
-        // between one block and the next -- inside the piece, so the card's fill runs through it.
+        // Without this the card closes everywhere except on the text, which is most of it.
+        CompositionLocalProvider(LocalMarkdownTap provides rememberMarkdownTap(onToggle)) {
+            // The gap the card's own column used to provide between its heading and its prose,
+            // and between one block and the next -- inside the piece, so the card's fill runs
+            // through it.
             MarkdownPiece(unit.text, unit.piece, replies, Modifier.padding(top = unit.spacing))
         }
     }
+}
 
 /**
  * One piece of a card drawn in slices: the fill, the corners it owns, and the room inside it.

app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt

@@ -281,6 +281,13 @@ fun SessionScreen(
     // have to ask whether anything is selected before they treat a tap as their own -- see
     // [expanding].
     val selection = rememberSelectionState()
+    // Read here, at composition, rather than inside [expanding] at the moment of the click. The
+    // container clears the selection from the very press that a card then reads as its own, a few
+    // milliseconds earlier and in the same event -- so a card asking the live state at its click
+    // always hears "nothing is selected", and a tap meant to put a selection away also shut the
+    // tool call the words were in. This value is whatever was true as of the last frame, which is
+    // what the reader was looking at when they touched the screen.
+    val selecting = selection.selectedTexts.isNotEmpty()
     var expandedTools by remember { mutableStateOf(setOf<String>()) }
     // Which runs of adjacent tool calls are open. Keyed by the first call's
     // id, so a group survives more calls arriving after it.
@@ -509,7 +516,7 @@ fun SessionScreen(
      * hold for only some of them.
      */
     fun expanding(toggle: () -> Unit) {
-        if (selection.selectedTexts.isNotEmpty()) {
+        if (selecting) {
             selection.clear()
             return
         }

app/androidApp/src/main/kotlin/com/example/aiapp/Theme.kt

@@ -1,5 +1,6 @@
 package com.example.aiapp
 
+import androidx.compose.foundation.text.selection.TextSelectionColors
 import androidx.compose.material3.ButtonColors
 import androidx.compose.material3.ButtonDefaults
 import androidx.compose.material3.MaterialTheme
@@ -257,6 +258,26 @@ fun ansiPalette(): AnsiPalette =
         background = Mocha.Crust,
     )
 
+/**
+ * What a selection looks like, stated rather than left to Material's default.
+ *
+ * The default is `primary` at 40% alpha, which is a tint of whatever is behind it -- and this app
+ * draws text on surfaces two full steps apart. Over a reply, on Base, that reads clearly. Over a
+ * code block or a tool's output, on Crust, the same 40% composites to a barely-there smudge, so
+ * selecting a line of code looks like nothing happened even though the selection is there and
+ * copies correctly.
+ *
+ * Fixed and stronger, because "this is selected" is a meaning rather than decoration: a colour that
+ * means something must carry its own contrast instead of borrowing it from the surface it happens
+ * to land on. Raised only as far as it takes to read on the darkest of them -- past this the fill
+ * starts competing with the syntax colours it sits behind, which are the thing being read.
+ */
+val AiAppSelectionColors =
+    TextSelectionColors(
+        handleColor = Mocha.Mauve,
+        backgroundColor = Mocha.Mauve.copy(alpha = 0.55f),
+    )
+
 /**
  * A link. Blue is what a link is on every Catppuccin surface, and the one colour to leave alone.
  */

app/androidApp/src/test/kotlin/com/example/aiapp/AnsiTest.kt

@@ -0,0 +1,87 @@
+package com.example.aiapp
+
+import androidx.compose.ui.graphics.Color
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertNull
+import kotlin.test.assertTrue
+
+/**
+ * What survives a terminal's escape sequences, and what the styling reads as.
+ *
+ * Asserted as the plain text and as the style over a named substring, rather than as span offsets,
+ * so a failure prints the output that was got wrong instead of a pair of numbers.
+ */
+class AnsiTest {
+    private val palette =
+        AnsiPalette(
+            colours = (0..15).map { Color(it, 0, 0) },
+            foreground = Color(1f, 1f, 1f),
+            background = Color(0f, 0f, 0f),
+        )
+
+    private fun styled(text: String) = ansiStyled(text, palette)
+
+    /** The style covering the first character of [word], or null where nothing styles it. */
+    private fun styleOver(text: String, word: String) =
+        styled(text).let { annotated ->
+            val at = annotated.text.indexOf(word)
+            assertTrue(at >= 0, "no \"$word\" in ${annotated.text}")
+            annotated.spanStyles.firstOrNull { at >= it.start && at < it.end }?.item
+        }
+
+    private val esc = '\u001B'
+
+    @Test
+    fun `a colour becomes a span and the sequence itself disappears`() {
+        val text = "plain ${esc}[31mred${esc}[0m plain"
+        assertEquals("plain red plain", styled(text).text)
+        assertEquals(palette.colours[1], styleOver(text, "red")?.color)
+        assertNull(styleOver(text, "plain"))
+    }
+
+    @Test
+    fun `bright, background and 256-colour forms all reach the same table`() {
+        assertEquals(palette.colours[9], styleOver("${esc}[91mx", "x")?.color)
+        assertEquals(palette.colours[4], styleOver("${esc}[44mx", "x")?.background)
+        // The first sixteen of the 256-colour table are the palette's own, so a program that
+        // spells a colour either way gets the same one.
+        assertEquals(palette.colours[1], styleOver("${esc}[38;5;1mx", "x")?.color)
+        // And past them, xterm's cube: 16 is its black corner, 231 its white one.
+        assertEquals(Color(0, 0, 0), styleOver("${esc}[38;5;16mx", "x")?.color)
+        assertEquals(Color(255, 255, 255), styleOver("${esc}[38;5;231mx", "x")?.color)
+        assertEquals(Color(10, 20, 30), styleOver("${esc}[38;2;10;20;30mx", "x")?.color)
+    }
+
+    @Test
+    fun `everything that is not styling is dropped rather than printed`() {
+        // A cursor move, an erase, an OSC window title with its bell, and a bare two-character
+        // escape. None of them mean anything in a scrolling document, and all of them would be
+        // line noise if the escape alone were stripped and the body left behind.
+        val text = "a${esc}[2Jb${esc}[Kc${esc}]0;a titled${esc}=e"
+        assertEquals("abcde", styled(text).text)
+    }
+
+    @Test
+    fun `a carriage return rewrites its line, as it does on a terminal`() {
+        // What a progress bar looks like: every state it passed through, ending on the last.
+        assertEquals("done\n", styled("10%\r50%\rdone\n").text)
+        // The line before it is untouched. A Windows line ending rewrites nothing and is not
+        // kept either: it is one line break, and passing the carriage return through would draw
+        // a stray control character in the middle of the output.
+        assertEquals("kept\nlast", styled("kept\r\nfirst\rlast").text)
+    }
+
+    @Test
+    fun `a sequence cut off mid-stream takes no text with it`() {
+        // Output still arriving ends anywhere, including inside an escape. The fragment goes and
+        // the whole sequence arrives with the next delta.
+        assertEquals("text ", styled("text ${esc}[3").text)
+    }
+
+    @Test
+    fun `unstyled text costs no spans at all`() {
+        assertEquals(0, styled("nothing to do here").spanStyles.size)
+        assertEquals(0, styled("a${esc}[2Jb").spanStyles.size)
+    }
+}

app/androidApp/src/test/kotlin/com/example/aiapp/DurationsTest.kt

@@ -0,0 +1,56 @@
+package com.example.aiapp
+
+import java.time.Duration
+import kotlin.test.Test
+import kotlin.test.assertEquals
+
+/**
+ * The two ways a span of time is written here, and the rule each of them follows.
+ *
+ * Both are read off a screen to make a decision -- how long a tool call may take, how long a quota
+ * has left -- so what matters is that the shortest form that answers the question is what appears.
+ */
+class DurationsTest {
+    @Test
+    fun `under a minute is the largest unit alone`() {
+        assertEquals("30ms", formatMillis(30))
+        assertEquals("999ms", formatMillis(999))
+        assertEquals("1s", formatMillis(1000))
+        assertEquals("2.5s", formatMillis(2500))
+        // One decimal, rounded rather than cut: 2.46s is nearer two and a half than two and four.
+        assertEquals("2.5s", formatMillis(2460))
+        assertEquals("59.9s", formatMillis(59_900))
+    }
+
+    @Test
+    fun `a minute or more is every unit that has something in it`() {
+        // The figure this rule was written for: a tool timeout, which arrives as milliseconds and
+        // is unreadable as 480000.
+        assertEquals("8m", formatMillis(480_000))
+        assertEquals("1m", formatMillis(60_000))
+        assertEquals("1m 30s", formatMillis(90_000))
+        assertEquals("5d 12h 4m", formatMillis(475_440_000))
+        // Empty units are left out rather than written as zero: the labels say which is which,
+        // and "5d 0h 4m" is only longer.
+        assertEquals("5d 4m", formatMillis(432_240_000))
+    }
+
+    @Test
+    fun `only a whole number of milliseconds is rewritten`() {
+        assertEquals("8m", formatMillisText(" 480000 "))
+        // A timeout a tool expressed some other way is its own words, passed through rather than
+        // guessed at.
+        assertEquals("2 minutes", formatMillisText("2 minutes"))
+        assertEquals("", formatMillisText(""))
+    }
+
+    @Test
+    fun `a countdown rounds up, so it never reports a minute already spent`() {
+        assertEquals("3h 13m", formatSpan(Duration.ofMinutes(192).plusSeconds(50)))
+        // Exactly on a minute is already the answer and is not pushed past it.
+        assertEquals("3h 12m", formatSpan(Duration.ofMinutes(192)))
+        assertEquals("12m", formatSpan(Duration.ofMinutes(12)))
+        // Rounding up carries, so a day's worth of minutes reads as a day.
+        assertEquals("1d 0h", formatSpan(Duration.ofHours(23).plusMinutes(59).plusSeconds(30)))
+    }
+}