Prune the transcript doc to what is still live, and measure the highlighting bug
The rendering doc's running log of finished work is folded into the architecture section and the rest dropped: what remains is why the code is the way it is, the harness, and what is next. The highlights bug is bigger than the backwards span we already drop. The library pairs every `/*` with an `*/` by ordinal position and uses the same delimiters for every language, so a shell glob opens a comment and `//` in any URL comments out the rest of its line, taking the keywords and strings inside it with it. highlights-repro.sh asks the library directly, outside the app, since none of this is visible on a phone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
b0b91ad28f
commit
c0121d2b5a
2 files changed
+167
-164
No files matched your search
+108
-164
@@ -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
|
||||
|
||||
Reference in new issue
Block a user