diff --git a/TRANSCRIPT_RENDERING.md b/TRANSCRIPT_RENDERING.md index c06fb70..541d24f 100644 --- a/TRANSCRIPT_RENDERING.md +++ b/TRANSCRIPT_RENDERING.md @@ -2,7 +2,8 @@ Written 2026-09-03 at the end of a week of work on the session screen's transcript, so the next session can start from here rather than from a -compacted context. `AGENTS.md` holds the one-paragraph conventions; this is +compacted context. Work that is finished lives in "the architecture, as +built"; the running log of how each piece got there has been dropped. `AGENTS.md` holds the one-paragraph conventions; this is the longer record: the measurements that drove each decision, the techniques that worked, the ones that did not, and the order to do the rest in. `PLAN.md` remains the design source of truth; nothing here contradicts @@ -40,10 +41,13 @@ re-parsing substrings, which cost a parse per piece and broke reference links defined at the foot of a message. `ParsedReplies` caches the parse and the piece list per text (`of`, `piecesOf`), warmed off the composing thread by `TranscriptItems.warm`. The parser is still intellij-markdown via -the mikepenz renderer; the renderer's `Markdown()` composable is kept only -as the provider of its `Local*` environment (`MarkdownRoot` in -`Markdown.kt`), and `MarkdownElement` dispatches a whole block through our -component table. +the mikepenz renderer, but its `Markdown()` composable is not called at all: +`MarkdownRoot` in `Markdown.kt` provides the `Local*` environment itself -- +reference links from the parse, padding, dimens, colours, typography, a +no-op image transformer, animations, components -- and `MarkdownElement` +dispatches a whole block through our component table. Nothing between a +piece and the screen is the library's now except the leaf composables that +table names. **Lists are drawn an item at a time, by us.** The renderer has no element for a single list item, so `MarkdownListItem` draws one: marker, then the @@ -87,8 +91,52 @@ reparses only the tail block per delta. Markdown's block rules make later text unable to alter an earlier block, with the single exception of a late reference definition, which is accepted. Measured on a 58-word stream of list, fence, table and quote: 47 tail reparses at 1.7ms mean. A -single-list stream still reparses per delta because the whole list is one -tail block. +single-list stream would reparse the whole list per delta, since it is one +tail block; that is what the rule below cuts. + +**A streaming list becomes a unit per item.** `LiveParse.advanceTo` cuts at +the last item of a multi-item list (`openPiece`), provided that item has +content beyond its marker -- a bare `-` is an empty item now and the first +character of a paragraph line once `-x` arrives, so cutting on it would draw +that line as a new item. The cut is at the start of the item's line, so the +indentation the reparse reads its nesting from survives. `Segment.continues` +marks a tail that carries on a list, and `MarkdownPiece`'s +`continuesList`/`listContinues` keep an inner item's padding at the seam, so +nothing moves when the seam does. Forty linked bullets streamed a word at a +time went from 2412ms of reparsing to 674ms, and `record: one block` from +1.8ms worst to 0.7ms. + +**Fences are highlighted off the drawing thread, and a fence still being +written is drawn plain.** `CodeFence.kt` holds `highlight` (shared with a +tool call's input, so the same code is the same colours wherever it +appears), the `fenceLanguage` alias table, and `fenceContent`. A word not in +the table stays plain, because a fence coloured by the wrong language's +rules looks highlighted and is wrong in a way the reader cannot see. +Highlighting is warmed and cached exactly as parsing is +(`ParsedReplies.highlighted`, filled by `warm` from `fences(parse)`), and +`highlight` takes no colour from the theme, which is what lets it run off +the drawing thread: a two-hundred-line Kotlin fence costs 174ms to lex, and +a `remember` inside the fence was charged that again every time the block +scrolled back into composition. Because the warming has to ask for the same +string the drawing does, `fenceContent` extracts the code and the language +word itself -- two extractions would be two keys, and the warmed answer +would be missed at every fence with nothing saying so. A fence still +arriving is the same stall in a second place, and warming cannot reach it: +the tail was re-lexed at every delta, on the composing thread, for colours +on text being replaced as fast as they were computed -- 211 lexes and 13.7 +seconds across one turn. So `MarkdownRoot`'s `streaming`, true only for a +live reply's last segment, draws the block plain until it freezes; a +finished fence colours as soon as the next block starts, and the settling +lex happens once, in `warm`. + +**Markers, and images.** `MarkdownListItem`'s `Marker` draws the bullet by +depth, cycling past the third, in `listMarkerColor` (Theme.kt). The colour +is the same at every depth on purpose: depth is said by the glyph and the +indent, and a colour per depth would make a difference in degree look like +one in kind. The app has no image loader and the renderer's transformer was +the no-op one, so an image in a reply drew as *nothing at all*; an `IMAGE` +node is now appended by `appendPlainLink` as a link carrying its alt text +(the address when there is none), which says what was there and opens it. **Expansion anchors the edge that was tapped, and the list never moves under the reader** except when pinned to the bottom with new content @@ -114,6 +162,25 @@ and are the reason several tempting simplifications were rejected. (`adb logcat -d -s ai-app:I`), and it includes the last crash's stack (`CrashLog.kt`), which is how a crash on the phone reaches a session here. +- **`app/stream-bench.sh [-k] FILE`** is `transcript-bench.sh` for a reply + still arriving: opens the first session, taps "Jump to latest" so the list + is pinned to the newest end, resets the report, sends FILE, waits for the + transcript to stop growing, prints the report. Both of those last two are + corrections to a first version that measured nothing -- a transcript parked + further back never redraws while a reply streams into it, and a session is + idle at *both* ends of a turn, so polling for idle answers before the turn + has started. Fixtures live in `/tmp` and are regenerated from the shapes + named here: `fixture.md` (lists four deep, ordered and nested, fences in + kotlin/rust/sh/none, a table with a link, a quote with a list, an inline + and a standalone image, a reference link), `longfence.md` (200-line Kotlin + fence), `longlist.md` (40 linked items). +- **Two traps in the emulator loop**, each of which cost a bench run. + `adb shell pm clear` removes the enrolment and the notification permission + along with the saved anchors, so the next run measures a permission + dialog; re-enrol with the command `ui-sandbox.sh` prints and + `pm grant ... POST_NOTIFICATIONS`. And a saved anchor is per session id, + so the only way two builds start a scroll from the same place is a *fresh + session for each*. - **`DebugStats`/`FrameStats`** time our own phases (`record: one block`, `measure: the app root`) and count events (`markdown reparsed while streaming`, `markdown cut into pieces`). Add a counter before guessing. @@ -142,9 +209,25 @@ and are the reason several tempting simplifications were rejected. - **Compose `DropdownMenu` in an edge-to-edge activity** needs `PopupProperties(clippingEnabled = false)` or it opens a status bar's height away from its anchor (`~/.claude/TOOLCHAIN.md`). -- **highlights 1.1.0's shell lexer** returns a span whose end precedes its - start for `x '*/a/*'`; `ToolInput.kt` drops such spans. The fix belongs - upstream and has not been filed. +- **highlights 1.1.0 finds comments before it knows the language, and pairs + `/*` with `*/` by ordinal.** Measured against the library directly with + `app/highlights-repro.sh`, which asks it for a piece of code outside the + app and prints every span with the text under it -- the mistakes are + invisible on a phone, where a line greyed out as a comment looks like a + comment. In `MultilineCommentLocator` and `CommentLocator`: it collects every `/*` and + every `*/` in the code, zips the two lists by position and never checks + that the end follows the start, so `x '*/a/*'` yields `start=6, end=5` -- + a range `AnnotatedString` rejects, which crashed a card holding + `-path '*/.git/*'` until `highlight` started dropping such spans. The same + delimiters are used for every language, so a shell glob is read as a + comment opener and, worse, `//` in any URL comments out the rest of its + line: in `curl https://example.com/x && echo done` the comment runs to the + end and takes `echo` with it, and in Kotlin `val url = "https://..."` the + string span disappears inside it. Comments are located before strings and + win over them. What we can reach is `getCodeStructure()`, which is public, + plus a nine-line reconstruction of the library's private + `constructHighlights`; the locators themselves are `internal`. 1.1.0 is + the newest release, so there is nothing to upgrade to. ## Rejected, and why @@ -160,151 +243,6 @@ and are the reason several tempting simplifications were rejected. catch at 120Hz is a bug; corrections must be structurally impossible to see. -## Session of 2026-09-03: the list above, worked through - -Items 1-5 of the previous list are done and checked on the emulator, and so -is the AGP bump from item 7. Two things were found while measuring them: a -174ms stall this work introduced and then removed, and a restore bug that -predates it. Both are described below with their numbers. - -**1. Styled markers per depth.** `MarkdownListItem`'s `Marker` draws `•`, -`◦`, `▪` by depth, cycling past the third, in `listMarkerColor` (Theme.kt, -Lavender -- the scheme's secondary accent, which nothing else used, so it -now means "structure"). Ordered numbers take the same colour. All three -glyphs were checked on screen at four depths: they render from the system -fonts, no missing-glyph boxes. The colour is the same at every depth on -purpose -- depth is said by the glyph and the indent, and a colour per depth -would make a difference in degree look like one in kind. - -**2. Syntax highlighting inside fences.** `CodeFence.kt` holds it: -`highlight` (moved out of `ToolInput.kt`, so a reply's code and a tool -call's command are the same colours), the `fenceLanguage` alias table -(extension or name to the highlights lexer; a word not in it stays plain, -because a fence coloured by the wrong language's rules looks highlighted and -is wrong in a way the reader cannot see), `fenceContent`, and the -`codeFence`/`codeBlock` entries of the component table. - -The measurement is the reason there is a cache. Highlighted naively, with -the answer held by a `remember` inside the fence, a two-hundred-line Kotlin -fence cost **174ms to lex** and the lazy list charged it again every time -the block scrolled back into composition -- six times in one bench run, -1043ms of lexing, and the scroll's draw phase up at 1.29ms per frame. So -highlighting is now warmed and cached exactly as parsing is -(`ParsedReplies.highlighted`, filled by `warm` from `fences(parse)`), and -`highlight` is a plain function taking no colour from the theme, which is -what lets it run off the drawing thread. - -Because the warming has to ask for the same string the drawing does, -`fenceContent` extracts the code and the language word itself -- the rule -copied from the library's `MarkdownCodeFence`, which is a composable and so -cannot be called from `warm`. Two extractions would be two keys, and the -warmed answer would be missed at every fence with nothing saying so. - -A fence *still arriving* was the same stall in a second place, and the -warming does not reach it: the tail is re-lexed at every delta, on the -composing thread, for colours on text that is being replaced as fast as they -are computed. Measured streaming the same fence: **211 lexes, 13.7 seconds** -across the turn, the worst 177ms. So a block that is still being written is -drawn plain and takes its colours when it freezes -- `MarkdownRoot`'s -`streaming`, which is true only for a live reply's *last* segment, so a -finished fence colours as soon as the next block starts. It is the bargain -[LiveParse] already makes for a reference link defined at the foot of a -message, and the settle-time lex then happens in `warm`, off the drawing -thread: one lex, 374ms, and the row redraws coloured on the tick that -follows it. - -Clean pair, two fresh sessions of the same 200-line Kotlin fence, no saved -anchor, same gestures (`transcript-bench.sh`): - -| | before (no highlighting) | after (warmed) | -|---|---|---| -| draw phase per frame | 0.74ms | 0.77ms | -| the transcript's share | 0.33ms | 0.33ms | -| lexing during the scroll | none | none | - -Streaming that fence in (`stream-bench.sh /tmp/longfence.md`), before and -after the plain-while-writing rule: - -| | before | after | -|---|---|---| -| lexes during the turn | 211 | 1 (in `warm`, off-thread) | -| time in them | 13,665ms | 374ms | -| worst single lex | 177.1ms | -- | -| draw phase per frame | 1.51ms | 1.01ms | - -**3. `MarkdownRoot` no longer calls the library's `Markdown()`.** It -provides the locals itself -- reference links from the parse, padding, -dimens, colours, typography, a no-op image transformer, animations, -components. Nothing between a piece and the screen is the library's now -except the leaf composables named in the component table. - -**4. Paragraphs with images -- done differently from the plan.** The plan -said draw the image as its own piece; measuring first showed the app has no -image loader and the renderer's transformer was the no-op one, so an image -in a reply drew as *nothing at all*. An `IMAGE` node is now appended by -`appendPlainLink` as a link carrying its alt text (the address when there is -none), which says what was there and opens it. With no image to place, the -`hasImage` branch and the renderer's `MarkdownText` are gone: every -paragraph is the platform `BasicText`. - -**5. Per-item units for a streaming list.** `LiveParse.advanceTo` now cuts -at the last item of a multi-item list (`openPiece`), provided that item has -content beyond its marker -- a bare `-` is an empty item now and the first -character of a paragraph line once `-x` arrives, so cutting on it would draw -that line as a new item. The cut is at the start of the item's line, so the -indentation the reparse reads its nesting from survives. -`Segment.continues` marks a tail that carries on a list, and -`MarkdownPiece`'s `continuesList`/`listContinues` keep an inner item's -padding at the seam, so nothing moves when the seam does. - -Clean pair, two fresh sessions, forty linked bullet items streamed word at a -time (`stream-bench.sh /tmp/longlist.md`): - -| | before | after | -|---|---|---| -| reparses while streaming | 482 | 483 | -| total time in them | 2412ms | 674ms | -| mean / worst | 5.0ms / 8.9ms | 1.4ms / 7.9ms | -| `record: one block` worst | 1.8ms | 0.7ms | - -The worst case moves least, which is the shape to expect: the first reparse -of a tail still covers whatever has arrived, and the last item can be long. -What changes is that every reparse after it covers one item instead of the -whole list. - -**The restore walked back one event per request.** Found while benching, and -older than this work. `savedAnchor`'s loop asked for -`oldestSeq - anchor.seq + RESTORE_PAGE_CUSHION` events; when the anchor's row -is already loaded but is the oldest half-row (which `anchorRow` refuses, -correctly -- it grows when the page behind it lands), that span is negative -and was coerced to 1. So the restore fetched one event, then one more, at a -round trip each: six hundred requests walking a long reply back a word at a -time, with the screen on its spinner the whole way and the sandbox log -printing `limit=1` once a second. It now asks for a page counted in rows, -which is the only kind that can promise to reach the row behind the anchor. - -**Harness.** `app/stream-bench.sh [-k] FILE` is `transcript-bench.sh` for a -reply still arriving: opens the first session, taps the app's own "Jump to -latest" so the list is pinned to the newest end, resets the report, sends -FILE, waits for the transcript to stop growing, prints the report. Both of -those last two are corrections to a first version that measured nothing: -a transcript parked further back never redraws while a reply streams into it -(the list must not move under a reader), and a session is idle at *both* -ends of a turn, so polling for idle answers before the turn has started. -Fixtures live in `/tmp` and are regenerated from the shapes named here: -`fixture.md` (lists four deep, ordered and nested, fences in -kotlin/rust/sh/none, a table with a link, a quote with a list, an inline and -a standalone image, a reference link), `longfence.md` (200-line Kotlin -fence), `longlist.md` (40 linked items). - -**Two traps in the emulator loop**, both of which cost a bench run here. -`adb shell pm clear` removes the enrolment and the notification permission -along with the saved anchors, so the next run measures a permission dialog; -re-enrol with the command `ui-sandbox.sh` prints and -`pm grant … POST_NOTIFICATIONS`. And a saved anchor is per session id, so -the only way two builds start a scroll from the same place is a *fresh -session for each*. - ## What is next, in order 1. **The reconnect loop.** Restarting the app onto a session with a saved @@ -313,15 +251,21 @@ session for each*. `events?after=N` more than `CATCH_UP_LIMIT` (200) behind answers `reset` plus the newest 200 *raw* deltas -- a window starting mid-message -- and the reset clears `items`, which is the state the restore loop then pages - against. The one-event-per-request bug above was part of what made it so - visible; whether it survives that fix is the first thing to find out. -2. **File the highlights range bug upstream.** There is no `gh` and no - GitHub credential in this VM, so it needs Bryan or a token. One-line - repro: lexing `x '*/a/*'` as `SyntaxLanguage.SHELL` in highlights 1.1.0 - returns a highlight whose `location.end` precedes its `location.start`. - Until then `highlight` drops such spans, which is why the shell fence in - `fixture.md` -- whose command contains `'*/.git/*'` -- draws plain while - an ordinary shell fence colours. + against. The restore's one-event-per-request bug was part of what made it + so visible and has been fixed; whether this survives that fix is the + first thing to find out. +2. **Decide what to do about the highlighting bug above.** Two options, both + ours: post-process `getCodeStructure()` -- pair `/*` with the next `*/` + after it, and only for languages that have them, ignore `#` and `//` + where the language does not use them -- then build the annotated string + from the corrected structure, which costs no extra lexing but cannot + recover keywords and strings the wrong comment range already suppressed; + or fork the twenty-file library and fix the locators. Filing it upstream + needs Bryan or a token, since there is no `gh` and no GitHub credential + in this VM. One-line repro: lexing `x '*/a/*'` as `SyntaxLanguage.SHELL` + in highlights 1.1.0 returns a highlight whose `location.end` precedes its + `location.start`. Whichever is chosen, `app/highlights-repro.sh` is what + checks it. 3. **Regression runs.** `transcript-bench.sh` and `stream-bench.sh` before and after any change to the files above, with the report in the commit. The numbers to watch are the worst `record: one block`, the reparse mean diff --git a/app/highlights-repro.sh b/app/highlights-repro.sh new file mode 100755 index 0000000..b3d9039 --- /dev/null +++ b/app/highlights-repro.sh @@ -0,0 +1,59 @@ +#!/usr/bin/env bash +# What dev.snipme:highlights actually answers for a piece of code, outside the app. +# +# The lexer's mistakes are invisible on a phone -- a line greyed out as a comment looks +# like a comment -- so this asks it directly and prints every span with the text under +# it. It is how the `/*`-pairing and URL-comment bugs in TRANSCRIPT_RENDERING.md were +# measured, and it is what any fix has to be checked against. +# +# Usage: ./highlights-repro.sh [LANGUAGE FILE] (default: the known-bad cases) +set -euo pipefail + +cache=~/.gradle/caches/modules-2/files-2.1 +jar() { find "$cache/$1" -name "$2" ! -name '*sources*' | sort | tail -1; } + +cp=$(jar dev.snipme/highlights-jvm/1.1.0 '*.jar') +cp=$cp:$(jar org.jetbrains.kotlin/kotlin-stdlib '*[0-9].jar') +cp=$cp:$(jar org.jetbrains.kotlinx/kotlinx-coroutines-core-jvm '*.jar') +cp=$cp:$(jar org.jetbrains.kotlinx/kotlinx-serialization-core-jvm '*.jar') +cp=$cp:$(jar org.jetbrains.kotlinx/kotlinx-serialization-json-jvm '*.jar') +for entry in ${cp//:/ }; do + [ -f "$entry" ] || { echo "missing jar in the gradle cache: build the app once first" >&2; exit 1; } +done + +out=$(mktemp -d) +trap 'rm -rf "$out"' EXIT +cat > "$out/Repro.java" <<'JAVA' +import dev.snipme.highlights.Highlights; +import dev.snipme.highlights.model.*; +import java.nio.file.*; + +public class Repro { + public static void main(String[] args) throws Exception { + if (args.length == 2) { + report(SyntaxLanguage.valueOf(args[0].toUpperCase()), Files.readString(Path.of(args[1]))); + return; + } + report(SyntaxLanguage.SHELL, "x '*/a/*'"); + report(SyntaxLanguage.SHELL, "find . -path '*/.git/*' -prune -o -name '*.kt' -print"); + report(SyntaxLanguage.SHELL, "curl https://example.com/x && echo done"); + report(SyntaxLanguage.KOTLIN, "val url = \"https://example.com\"\nfun f() = 1"); + } + + static void report(SyntaxLanguage language, String code) { + System.out.println("== [" + language + "] " + code.replace("\n", "\\n")); + for (CodeHighlight highlight : new Highlights.Builder().code(code).language(language) + .build().getHighlights()) { + PhraseLocation at = highlight instanceof ColorHighlight + ? ((ColorHighlight) highlight).getLocation() + : ((BoldHighlight) highlight).getLocation(); + boolean sane = at.getStart() >= 0 && at.getStart() <= at.getEnd() + && at.getEnd() <= code.length(); + System.out.println((sane ? " " : " BAD ") + at + " " + + (sane ? "\"" + code.substring(at.getStart(), at.getEnd()) + "\"" : "")); + } + } +} +JAVA +javac -cp "$cp" -d "$out" "$out/Repro.java" +java -cp "$cp:$out" Repro "$@"