diff --git a/HIGHLIGHTER_PLAN.md b/HIGHLIGHTER_PLAN.md new file mode 100644 index 0000000..8acd0d6 --- /dev/null +++ b/HIGHLIGHTER_PLAN.md @@ -0,0 +1,246 @@ +# Replacing the code highlighter with a scanner of our own + +Written 2026-09-03 for whichever session does the work. Bryan decided on +this after the assessment below; `TRANSCRIPT_RENDERING.md` records the +measurements that led here and stays the record of the transcript work, +and `PLAN.md` stays the design source of truth for the app as a whole. +Nothing here contradicts either. Work goes on `main`, committed and pushed +when it is verified, as usual. + +## Why + +The app colours code in two places -- a fenced block in a reply and the +command in a tool call's card -- through one function, `highlight` in +`app/androidApp/src/main/kotlin/com/example/aiapp/CodeFence.kt`, backed by +the library `dev.snipme:highlights` 1.1.0. That library finds comments +before it knows the language, using `//`, `#` and `/* */` for every +language, then suppresses keywords and strings under whatever it found; +it pairs quotes by ordinal per quote character; and it pairs `/*` with +`*/` by ordinal too. Measured against the library directly on 2026-09-03: + +- Any language: `//` inside a URL comments out the rest of the line, and a + string holding a URL loses its colour inside the comment. +- Rust: every attribute line, `#[derive(Debug)]`, is greyed as a comment. +- Kotlin: a `#` inside a string (`"#FF0000"`) comments out the rest of the + line; an apostrophe inside a double-quoted string (`"don't"`) opens a + bogus single-quoted string that runs to the next apostrophe in the block. +- Shell: `x '*/a/*'` produced a span whose end preceded its start, which + crashed the app until `highlight` started dropping such spans. + +Upstream merged a one-line fix for the last of these in August 2026 and has +not released it; nothing else moved. Post-processing the library's output +cannot restore what it already suppressed, and forking it means rewriting +its analyzer, which is the part that is wrong. No maintained pure-Kotlin +alternative exists on Maven Central; tree-sitter would cost the NDK on +every machine that builds the APK and one to five megabytes per language +per ABI. So the highlighter becomes ours: one sequential scanner and a +table of languages. The rule in `prefer-libraries-for-specs` still holds +in general; this is the case it allows, where the only library is a worse +hand-rolled scanner than the one we would write. + +Counting fence words across the Claude Code transcripts on the dev VM +says which languages matter: shell in its variants 181, kotlin 119, rust +98, ron 67, then toml, python, java, js, groovy, json and diff at three to +eight each. RON, TOML and fish are not known to the library at all today, +so those fences are drawn plain; the scanner colours them from day one. + +## Shape + +Two new files under `app/androidApp/src/main/kotlin/com/example/aiapp/`, +and the library removed. + +**`Highlighter.kt`** holds the scanner, the palette type, and `highlight`. +The signature stays `fun highlight(code: String, language: Language?): +AnnotatedString`, returning plain text for a null language, so the two +callers and the cache do not change shape. It is not a composable and +takes no colour from the theme, for the reason `CodeFence.kt` already +states: `ParsedReplies.warm` runs it off the drawing thread. The palette +is a plain `SyntaxPalette` value class of our own -- keyword, string, +literal, comment, metadata, punctuation, mark, as `Color` -- returned by +`catppuccinSyntax()` in `Theme.kt`, which currently builds the library's +`SyntaxTheme` from ARGB ints. The library's separate multiline-comment +colour collapses into comment; the palette already gave both `Overlay0`. + +The scanner is one pass over the code with a small state: at each +position it is in a line comment, a block comment (with a nesting depth +where the language nests them), a string (with its closing delimiter and +whether backslash escapes apply), or ordinary code. In ordinary code it +recognises, in this order, a comment opener, a string opener, an +attribute, a number, a word, and finally single punctuation or mark +characters. Every span it emits is produced by advancing an index +forward, so spans are non-overlapping and ordered by construction and the +reversed-range filter goes with the library. Spans map onto the palette +as: keyword, string, literal (numbers), comment, metadata (attributes and +annotations), punctuation (`, . : ;`) and mark (`( ) = { } < > - + [ ] | +&`). Those two character sets are the library's, kept so nothing the +reader is used to changes colour except where it was wrong; punctuation +and marks are only coloured in ordinary code, never inside a string or a +comment, which is the one visible difference on correct input (a `.` in a +URL inside a string used to take the punctuation colour). + +A word is `[A-Za-z_][A-Za-z0-9_]*` and is a keyword if the language's set +contains it. A number is a word starting with a digit and continuing +through letters, digits, `_` and `.`, which covers `0xFF`, `1_000`, +`1u32` and `3.14` without a grammar. A date in TOML comes out as digits +with marks between; leave that, it is untidy rather than misleading. + +**`Languages.kt`** holds `enum class Language` and a `Rules` value per +member. Keep the vocabulary the code already uses: the enum replaces the +library's `SyntaxLanguage` under the name `Language`, and `fenceLanguage` +in `CodeFence.kt` keeps its name and its alias table, retargeted. `Rules` +is data, not code, and is what makes a new language a table row: + +- `lineComments`: the tokens that open a comment to end of line, and + whether they count only at the start of a word. Shell and fish need + the word-start rule, since `$#`, `${#x}` and `a#b` are not comments; a + `#` counts there when it is preceded by nothing, whitespace, or one of + `;`, `|`, `&`, `(`. Python, Ruby, Perl, TOML: `#` anywhere. C-family, + Kotlin, Rust, RON, Go, Swift, Dart, JavaScript, TypeScript, Java, + C#, PHP: `//`; PHP also `#`. +- `blockComment`: the pair, or none, and whether it nests. `/* */` for + the C-family and friends; Rust, Kotlin and RON nest; C, Java, + JavaScript, Go do not. Python, Ruby, shell, fish, TOML: none. +- `quotes`: each string form as opener, closer, and whether backslash + escapes the closer. Double quotes with escapes nearly everywhere; + single quotes with escapes in the C-family (a character literal reads + fine as a string); single quotes with *no* escapes in shell and TOML; + fish single quotes escape only `\'` and `\\`; triple quotes in Kotlin + (`"""`, no escapes), Python (`"""` and `'''`) and TOML (`"""` and + `'''`). Longest opener wins, so `"""` is tried before `"`. +- Two string forms need a line of code rather than a row, and belong in + the scanner behind a flag on `Rules`. Rust and RON raw strings: + optional `b`, then `r`, then `n` hashes, then `"`, closing at `"` + followed by `n` hashes, no escapes. Rust lifetimes: a `'` followed by + a backslash or by one character and a `'` is a character literal; + otherwise it is a lifetime or label and no string starts. Without that + rule `'a` opens a string that runs to the next apostrophe. +- `attributes`: what opens a metadata span. `@` followed by a word for + Kotlin, Java, Dart, Swift, TypeScript, PHP, Python (decorators); + `#[` or `#![` through the matching `]` for Rust and RON; a `#` at the + start of a line through end of line for C and C++ (preprocessor + directives, which the library greyed out as comments); a `[` at the + start of a line through `]` for TOML (table headers). Nothing for the + rest. +- `keywords`: the set. Bring the lists over from the library's + `SyntaxTokens.kt` (Apache-2.0; say so in a comment at the table), for + every language the alias table names today, so no fence that is + coloured now turns plain. Drop entries that are not plain words -- + Kotlin's `as?`, `!in`, `!is`; Swift's `#if` family; Ruby's `defined?`; + CoffeeScript's `=` and `->` -- since the word scanner cannot reach + them and the library only matched them by luck. Add rows for RON + (`true false Some None inf NaN`), TOML (`true false inf nan`), fish + (`and begin break builtin case command continue else end exec for + function if in not or return switch while set echo test string math + read source`) and JSON (`true false null`). Add `ron`, `toml`, `fish` + and `json` to the alias table in `CodeFence.kt`. + +The `streaming` arrangement stays as it is: a fence still being written +is drawn plain and coloured when it freezes. That decision was made +against a lexer costing 174ms for two hundred lines; a linear scanner may +make live colouring affordable, but that is a follow-up to measure with +`app/stream-bench.sh`, not something to assume here. + +## Files + +- `Highlighter.kt`, new: scanner, `SyntaxPalette`, `highlight`. The + `DebugStats.timed("code highlighted")` wrapper stays, as the number the + before-and-after is read from. +- `Languages.kt`, new: `Language`, `Rules`, the rows and keyword sets. +- `CodeFence.kt`: remove `highlight` and the library imports; retarget + `fenceLanguage` and `FENCE_LANGUAGES`; add the four aliases. +- `ToolInput.kt`: `SyntaxLanguage` becomes `Language` in `ToolInput` and + `SUBJECTS`. `ToolInputView` keeps calling `highlight` uncached. +- `Markdown.kt`: the import at line 58 and the signature of + `ParsedReplies.highlighted`. The cache key `"$language\n$code"` works + unchanged with an enum. +- `Theme.kt`: `catppuccinSyntax()` returns `SyntaxPalette` of `Color`s + rather than the library's theme of ints. +- `app/gradle/libs.versions.toml` and `app/androidApp/build.gradle.kts`: + remove `highlights` and its comment. +- `app/highlights-repro.sh`: delete. It exists only to interrogate the + library, and its cases move into the test below; this is the harness + being replaced by a better one, not thrown away. +- `app/androidApp/src/test/kotlin/com/example/aiapp/HighlighterTest.kt`, + new: see next section. +- Docs, in the same commit: in `TRANSCRIPT_RENDERING.md` rewrite the + fences paragraph's mention of the library, replace the "highlights + 1.1.0 finds comments before it knows the language" bullet with a short + description of the scanner and where its rows live, and drop item 2 of + "What is next". In `AGENTS.md`, add `:androidApp:testDebugUnitTest` to + the app line under "Checking your work". Prune rather than append. + +## Tests + +The app has no unit test source set today. The scanner is pure logic +with no Android imports, which is exactly the case CODE_RULES names for +adding tests, and the cost of the harness is one line, +`testImplementation(kotlin("test"))`, run by +`./gradlew :androidApp:testDebugUnitTest`. Keep `Highlighter.kt` and +`Languages.kt` free of Android and Compose imports other than +`AnnotatedString`, `SpanStyle` and `Color` (which are plain JVM classes), +so the test runs on the JVM without Robolectric. Better still, have the +scanner return a list of (range, kind) spans and let `highlight` be the +thin wrapper that turns them into an `AnnotatedString`: the test then +asserts on the list, which prints legibly on failure, and the test does +not depend on Compose loading at all. + +Cases, each asserting the spans against the code so a failure prints +what was coloured: + +- The four inputs from the deleted script: `x '*/a/*'` (shell), the + `find . -path '*/.git/*'` line, `curl https://example.com/x && echo + done`, and `val url = "https://example.com"\nfun f() = 1`. +- `#[derive(Debug)]` then a struct in Rust: metadata, then keywords. +- `"#FF0000"` and `"don't"` in Kotlin: two strings, no comment. +- `'a` as a Rust lifetime beside `'x'` as a character: one string. +- `r#"a "quoted" b"#` in Rust: one string. +- Nested `/* a /* b */ c */` in Rust and Kotlin: one comment to the end; + the same in C: comment ends at the first `*/`. +- `${#x} $# a#b # real` in shell: one comment, starting at the last `#`. +- `"""a "b" c"""` in Kotlin: one string. +- A TOML table header and a key with a `#` in a quoted value. +- A language the alias table does not know: `highlight` returns the + plain string. +- Every emitted span is inside the code and starts before it ends, over + a handful of nasty inputs (unterminated string, unterminated block + comment, a lone `'`, empty code). The scanner must never throw; an + unterminated thing runs to the end of the code. + +## Verification + +1. From `app/`: `. ./android-env.sh && ./gradlew :androidApp:ktfmtFormat + :androidApp:compileDebugKotlin :androidApp:lintDebug + :androidApp:testDebugUnitTest`. Lint must stay clean; an unused + dependency or import it reports is a finding. +2. Timing. `TRANSCRIPT_RENDERING.md` gives the library at 174ms for a + two-hundred-line Kotlin fence on the emulator. Regenerate + `/tmp/longfence.md` from that description and read `code highlighted` + from the render report after `app/transcript-bench.sh`; expect single + milliseconds. Run `app/transcript-bench.sh` and `app/stream-bench.sh` + before and after, and put the report lines named in that doc's item 3 + into the commit message. +3. Look at it. `app/run-android.sh`, send `/tmp/fixture.md` through + `app/ui-sandbox.sh` (both described in `TRANSCRIPT_RENDERING.md`), and + screenshot a fence holding a URL in a string, a Rust attribute, a + shell line with a glob and a `$#`, a RON fence, and a TOML fence. Check + a tool card's Bash command still colours, since that is the other + caller. Check a fence in an unknown language is plain, and that a + fence still streaming is plain and colours when the next block starts. + Per the code rule about trying a fix on the half that cannot fail, + look at a fence the change had no reason to touch -- a plain Python + function -- and confirm it looks as it did. +4. Commit on `main` with `git add -A`, and push. + +## Traps + +- `fenceContent` is shared by the drawing and the warming and must stay + the single extraction; nothing here should touch it. +- The `Color` constructor in `highlight` currently ORs in an alpha byte + because the library's ints had none. With `Color` values in the palette + that goes away; do not carry the mask over. +- Keyword lists are per language in the library's `DEFAULT` case merged + into one; there is no `DEFAULT` here, since a null language is plain. +- The two callers pass different things: fences pass what the alias + table answered for the info word, tool cards pass `SHELL` for Bash and + null for everything else. Both must keep compiling without a cast. +- ktfmt will reflow the keyword tables; let it.