git.iko.soy/crush
on main help sign in
perf(chat): reuse the rendered prefix of a streaming reply
1c7ccfdChristian Rocha
internal/ui/chat/assistant.go+24−6
compare with main
unifiedsplit
@@ -137,6 +137,13 @@ type AssistantMessageItem struct {
137 thinkingSec assistantSection
137 thinkingSec assistantSection
138 contentSec assistantSection
138 contentSec assistantSection
139 errorSec assistantSection
139 errorSec assistantSection
140+
141+ // streamingContent caches a "stable prefix" glamour render of
142+ // the assistant content body so each streaming flush only
143+ // re-renders the trailing partial. F8 of
144+ // docs/notes/2026-05-12-chat-rendering-perf.md. See
145+ // streaming_markdown.go for the full algorithm.
146+ streamingContent streamingMarkdown
140}
147}
141
148
142var _ Expandable = (*AssistantMessageItem)(nil)
149var _ Expandable = (*AssistantMessageItem)(nil)
@@ -440,7 +447,10 @@ func (a *AssistantMessageItem) cachedError(width int) string {
440// lines so the visual box matches what the user sees today.
447// lines so the visual box matches what the user sees today.
441func (a *AssistantMessageItem) renderThinking(thinking string, width int) string {
448func (a *AssistantMessageItem) renderThinking(thinking string, width int) string {
442 renderer := common.QuietMarkdownRenderer(a.sty, width)
449 renderer := common.QuietMarkdownRenderer(a.sty, width)
450+ mu := common.LockMarkdownRenderer(renderer)
451+ mu.Lock()
443 rendered, err := renderer.Render(thinking)
452 rendered, err := renderer.Render(thinking)
453+ mu.Unlock()
444 if err != nil {
454 if err != nil {
445 rendered = thinking
455 rendered = thinking
446 }
456 }
@@ -489,14 +499,18 @@ func (a *AssistantMessageItem) renderThinking(thinking string, width int) string
489 return result
499 return result
490}
500}
491
501
492−// renderMarkdown renders content as markdown.
502+// renderMarkdown renders content as markdown. F8 routes the call
503+// through streamingContent, which caches the glamour render of a
504+// "stable prefix" so each streaming flush only re-renders the
505+// trailing partial. The streaming cache invalidates itself on
506+// width change and on any content that is not a prefix-extension
507+// of the previously rendered content (e.g. user retried the
508+// turn), and falls back to a full render whenever boundary
509+// detection has the slightest doubt — see
510+// findSafeMarkdownBoundary.
493func (a *AssistantMessageItem) renderMarkdown(content string, width int) string {
511func (a *AssistantMessageItem) renderMarkdown(content string, width int) string {
494 renderer := common.MarkdownRenderer(a.sty, width)
512 renderer := common.MarkdownRenderer(a.sty, width)
495− result, err := renderer.Render(content)
513+ return a.streamingContent.Render(content, width, renderer)
496− if err != nil {
497− return content
498− }
499− return strings.TrimSuffix(result, "\n")
500}
514}
501
515
502func (a *AssistantMessageItem) renderSpinning() string {
516func (a *AssistantMessageItem) renderSpinning() string {
@@ -564,11 +578,15 @@ func (a *AssistantMessageItem) Finished() bool {
564// clearCache drops every cached render for this item, including the
578// clearCache drops every cached render for this item, including the
565// per-section caches. Shadows the embedded cachedMessageItem.clearCache
579// per-section caches. Shadows the embedded cachedMessageItem.clearCache
566// so ClearItemCaches (style change) wipes the section caches too.
580// so ClearItemCaches (style change) wipes the section caches too.
581+// F8: also drop the streaming-markdown stable-prefix cache because
582+// the cached glamour render embeds the OLD style's ANSI sequences
583+// and is no longer visually consistent with the new style.
567func (a *AssistantMessageItem) clearCache() {
584func (a *AssistantMessageItem) clearCache() {
568 a.cachedMessageItem.clearCache()
585 a.cachedMessageItem.clearCache()
569 a.thinkingSec.reset()
586 a.thinkingSec.reset()
570 a.contentSec.reset()
587 a.contentSec.reset()
571 a.errorSec.reset()
588 a.errorSec.reset()
589+ a.streamingContent.Reset()
572}
590}
573
591
574// ToggleExpanded advances the F5 thinking view-mode cycle and returns
592// ToggleExpanded advances the F5 thinking view-mode cycle and returns
internal/ui/chat/incremental_glamour_test.gonew file+853
@@ -0,0 +1,853 @@
1+package chat
2+
3+import (
4+ "strings"
5+ "testing"
6+
7+ "charm.land/glamour/v2"
8+ "github.com/charmbracelet/crush/internal/ui/styles"
9+ "github.com/stretchr/testify/require"
10+)
11+
12+// newTestRenderer builds a fresh glamour renderer for the given
13+// width. We deliberately do NOT share renderers between calls in
14+// the equivalence tests so any hidden state in
15+// [glamour.TermRenderer] cannot leak from a "cached" rendering
16+// path into a "fresh" rendering path.
17+func newTestRenderer(t *testing.T, width int) *glamour.TermRenderer {
18+ t.Helper()
19+ sty := styles.CharmtonePantera()
20+ r, err := glamour.NewTermRenderer(
21+ glamour.WithStyles(sty.Markdown),
22+ glamour.WithWordWrap(width),
23+ )
24+ require.NoError(t, err)
25+ return r
26+}
27+
28+// freshRender renders content as a single document with a fresh
29+// glamour renderer and applies the same trailing-newline trim
30+// that streamingMarkdown.Render does. Use this for byte- and
31+// visible-equivalence comparisons against the streaming path.
32+func freshRender(t *testing.T, content string, width int) string {
33+ t.Helper()
34+ r := newTestRenderer(t, width)
35+ out, err := r.Render(content)
36+ require.NoError(t, err)
37+ return strings.TrimSuffix(out, "\n")
38+}
39+
40+// stripANSI removes all ANSI CSI escape sequences from s so two
41+// renders with different colour state can be compared on their
42+// visible glyphs alone.
43+func stripANSI(s string) string {
44+ var b strings.Builder
45+ b.Grow(len(s))
46+ i := 0
47+ for i < len(s) {
48+ if s[i] == 0x1b && i+1 < len(s) && s[i+1] == '[' {
49+ j := i + 2
50+ for j < len(s) {
51+ c := s[j]
52+ if c >= 0x40 && c <= 0x7e {
53+ j++
54+ break
55+ }
56+ j++
57+ }
58+ i = j
59+ continue
60+ }
61+ b.WriteByte(s[i])
62+ i++
63+ }
64+ return b.String()
65+}
66+
67+// normalizeRender canonicalises a rendered glamour string for
68+// visual-equivalence comparison: strip ANSI, drop per-line
69+// trailing whitespace, drop leading/trailing blank lines, and
70+// collapse consecutive blank lines to a single blank line.
71+//
72+// Glamour pads rendered lines with trailing spaces and adds top/
73+// bottom block margins that differ subtly between "render the
74+// whole document at once" and "render two halves and concatenate
75+// them." Per F8 design principle D, those byte-level differences
76+// are acceptable as long as the visible content matches; this
77+// helper makes that comparison explicit.
78+func normalizeRender(s string) string {
79+ clean := stripANSI(s)
80+ lines := strings.Split(clean, "\n")
81+ for i, l := range lines {
82+ lines[i] = strings.TrimRight(l, " \t")
83+ }
84+ // Collapse consecutive blank lines.
85+ out := make([]string, 0, len(lines))
86+ prevBlank := false
87+ for _, l := range lines {
88+ blank := l == ""
89+ if blank && prevBlank {
90+ continue
91+ }
92+ out = append(out, l)
93+ prevBlank = blank
94+ }
95+ // Trim leading and trailing blanks.
96+ for len(out) > 0 && out[0] == "" {
97+ out = out[1:]
98+ }
99+ for len(out) > 0 && out[len(out)-1] == "" {
100+ out = out[:len(out)-1]
101+ }
102+ return strings.Join(out, "\n")
103+}
104+
105+// containsRawMarkdownSource reports whether the visible portion of
106+// rendered contains literal markdown source markers that should
107+// have been consumed by glamour. Used by T2 to assert that
108+// intermediate streaming flushes don't leak raw source through to
109+// the user. We deliberately only flag markers that glamour
110+// removes during rendering ("```" fence delimiters, "|" table
111+// pipes embedded in a line that also contains pipes — actual
112+// table syntax — and bare "###" headers); pipes-in-prose and
113+// dashes are too common to flag.
114+func containsRawMarkdownSource(rendered string) bool {
115+ clean := stripANSI(rendered)
116+ if strings.Contains(clean, "```") {
117+ return true
118+ }
119+ for _, line := range strings.Split(clean, "\n") {
120+ if strings.HasPrefix(strings.TrimLeft(line, " \t"), "###") {
121+ return true
122+ }
123+ }
124+ return false
125+}
126+
127+// -----------------------------------------------------------------------
128+// T1: findSafeMarkdownBoundary unit tests.
129+// -----------------------------------------------------------------------
130+
131+// TestFindSafeMarkdownBoundary_TableDriven exercises the
132+// findSafeMarkdownBoundary decision tree across the full set of
133+// constructs §4.4 calls out: plain paragraphs, fenced code (open
134+// and closed), lists, tables, block quotes, and setext headers.
135+func TestFindSafeMarkdownBoundary_TableDriven(t *testing.T) {
136+ t.Parallel()
137+
138+ cases := []struct {
139+ name string
140+ content string
141+ // want is the expected boundary; -1 means "no safe
142+ // boundary." When >=0 the test asserts content[:want]
143+ // ends after a blank-line separator and content[:want]
144+ // is a complete prefix.
145+ want int
146+ }{
147+ {
148+ name: "empty",
149+ content: "",
150+ want: -1,
151+ },
152+ {
153+ name: "single line",
154+ content: "Just a single paragraph",
155+ want: -1,
156+ },
157+ {
158+ name: "two paragraphs",
159+ content: "First paragraph.\n\nSecond paragraph.",
160+ // boundary at start of "Second"
161+ want: len("First paragraph.\n\n"),
162+ },
163+ {
164+ name: "three paragraphs picks latest",
165+ content: "First.\n\nSecond.\n\nThird.",
166+ want: len("First.\n\nSecond.\n\n"),
167+ },
168+ {
169+ name: "open fence at end",
170+ content: "Para.\n\n```go\nfoo()\n",
171+ // no closing fence — every blank-line candidate
172+ // before content end is INSIDE the fence (the open
173+ // fence opened at offset 7). Actually the ONLY
174+ // blank line is between "Para." and "```go", so
175+ // candidate boundary is right before "```go". At
176+ // that point fence count = 0, even, but the line
177+ // AFTER (the first non-blank) is "```go" which
178+ // would change rendering of the prefix… hmm,
179+ // actually it wouldn't change the prefix's
180+ // rendering because the prefix is just "Para.\n\n".
181+ // The boundary would be ACCEPTED. Let's check
182+ // what our impl does.
183+ want: len("Para.\n\n"),
184+ },
185+ {
186+ name: "inside open fence: no candidate after open",
187+ content: "Para.\n\n```go\nfoo()\n\nbar()\n",
188+ // blank line after "foo()" is INSIDE the fence
189+ // (fence count at that prefix = 1, odd), must
190+ // reject. The earlier blank line between "Para."
191+ // and "```go" should still be safe (fence count
192+ // at that prefix = 0).
193+ want: len("Para.\n\n"),
194+ },
195+ {
196+ name: "closed fence followed by paragraph",
197+ content: "Para1.\n\n```\nfoo()\n```\n\nPara2.",
198+ // latest blank line is between "```" and "Para2.";
199+ // fence count at that prefix = 2 (even), last
200+ // non-blank line is "```" which is not a list/
201+ // table/quote/setext.
202+ want: len("Para1.\n\n```\nfoo()\n```\n\n"),
203+ },
204+ {
205+ name: "open list at end",
206+ content: "Para.\n\n- one\n- two\n",
207+ // last non-blank line of any blank-bounded prefix
208+ // is a list item; our boundary check rejects.
209+ // The blank line between "Para." and "- one" is
210+ // the only candidate, but the line AFTER (first
211+ // non-blank of suffix) is "- one" — that's fine,
212+ // a list opening doesn't change the prefix's
213+ // rendering. So the boundary BEFORE the list is
214+ // accepted.
215+ want: len("Para.\n\n"),
216+ },
217+ {
218+ name: "list interior: no boundary",
219+ content: "- one\n- two\n",
220+ // no blank line at all.
221+ want: -1,
222+ },
223+ {
224+ name: "closed list then paragraph",
225+ content: "- one\n- two\n\nPara.",
226+ // blank line after the list. Last non-blank line
227+ // of prefix is "- two" — a list item — so the
228+ // candidate is REJECTED. (Conservative: we don't
229+ // know the list is "closed" without looking at
230+ // what follows.)
231+ want: -1,
232+ },
233+ {
234+ name: "table at end",
235+ content: "Para.\n\n| a | b |\n| --- | --- |\n| 1 | 2 |\n",
236+ // blank-line candidate is between "Para." and
237+ // table opener. Last non-blank line of prefix is
238+ // "Para." — fine. Line AFTER is "| a | b |"
239+ // which is a table line; doesn't retroactively
240+ // change "Para." Boundary accepted.
241+ want: len("Para.\n\n"),
242+ },
243+ {
244+ name: "table interior with internal blank line: no late boundary",
245+ content: "| a | b |\n| --- | --- |\n\n| 1 | 2 |\n",
246+ // the blank line in the middle is followed by
247+ // another table line. Last non-blank line of
248+ // prefix is "| --- | --- |" which contains a
249+ // pipe — we reject.
250+ want: -1,
251+ },
252+ {
253+ name: "block quote at end",
254+ content: "Para.\n\n> quoted\n> still quoted\n",
255+ // Last non-blank line of any prefix that ends
256+ // inside the quote block is a "> ..." line —
257+ // rejected. The blank line BEFORE the quote
258+ // gives a prefix of "Para.\n\n" — last non-blank
259+ // "Para." — accepted.
260+ want: len("Para.\n\n"),
261+ },
262+ {
263+ name: "setext underline pending",
264+ content: "Heading\n\n=====\n",
265+ // blank line between "Heading" and "=====".
266+ // Prefix = "Heading\n\n", last non-blank "Heading"
267+ // — fine. But the FIRST non-blank line of the
268+ // suffix is "=====", a setext-underline
269+ // candidate. Splitting here would render the
270+ // prefix as a paragraph "Heading", but the
271+ // canonical render would treat the whole thing
272+ // as a setext header. Reject.
273+ //
274+ // (Note: per CommonMark, a blank line between a
275+ // paragraph and an underline actually breaks the
276+ // setext, so the setext interpretation may not
277+ // apply. But the boundary check is conservative
278+ // — being wrong costs one slow frame, being
279+ // over-aggressive costs visible breakage.)
280+ want: -1,
281+ },
282+ {
283+ name: "indented code at end of prefix",
284+ content: "Para.\n\n code line\n\nNext.",
285+ // prefix candidates:
286+ // "Para.\n\n" — last non-blank "Para.", accepted
287+ // "Para.\n\n code line\n\n" — last non-blank
288+ // is " code line" which is indented 4
289+ // spaces — REJECTED.
290+ // Latest accepted is the first.
291+ want: len("Para.\n\n"),
292+ },
293+ }
294+
295+ for _, c := range cases {
296+ t.Run(c.name, func(t *testing.T) {
297+ t.Parallel()
298+ got := findSafeMarkdownBoundary(c.content)
299+ require.Equalf(t, c.want, got,
300+ "findSafeMarkdownBoundary(%q) = %d, want %d", c.content, got, c.want)
301+ if got > 0 {
302+ // Boundary must point to the start of a line
303+ // (i.e. just after a newline) when the prefix
304+ // is non-empty.
305+ require.True(t, got <= len(c.content),
306+ "boundary %d out of range (len=%d)", got, len(c.content))
307+ if got > 0 && got <= len(c.content) {
308+ require.Equal(t, byte('\n'), c.content[got-1],
309+ "boundary %d does not sit immediately after a newline", got)
310+ }
311+ }
312+ })
313+ }
314+}
315+
316+// -----------------------------------------------------------------------
317+// T2: streaming-equivalence tests.
318+// -----------------------------------------------------------------------
319+
320+// streamingScenarios returns the four canonical document shapes
321+// that exercise different boundary-detection paths.
322+func streamingScenarios() []struct {
323+ name string
324+ doc string
325+} {
326+ return []struct {
327+ name string
328+ doc string
329+ }{
330+ {
331+ name: "plain-paragraphs",
332+ doc: strings.Join([]string{
333+ "This is the first paragraph of the document.",
334+ "",
335+ "Here is the second paragraph; it has some words.",
336+ "",
337+ "And a third paragraph for good measure.",
338+ "",
339+ "Finally a fourth paragraph to push past one boundary.",
340+ }, "\n"),
341+ },
342+ {
343+ name: "paragraphs-with-fence",
344+ doc: strings.Join([]string{
345+ "Intro paragraph.",
346+ "",
347+ "Some explanatory prose before the code.",
348+ "",
349+ "```go",
350+ "func hello() {",
351+ "\tfmt.Println(\"hi\")",
352+ "}",
353+ "```",
354+ "",
355+ "And a closing paragraph after the code block.",
356+ }, "\n"),
357+ },
358+ {
359+ name: "paragraphs-with-list",
360+ doc: strings.Join([]string{
361+ "Intro paragraph.",
362+ "",
363+ "- list item one",
364+ "- list item two",
365+ "- list item three",
366+ "",
367+ "Trailing paragraph.",
368+ }, "\n"),
369+ },
370+ {
371+ name: "paragraphs-with-table",
372+ doc: strings.Join([]string{
373+ "Intro paragraph.",
374+ "",
375+ "| col a | col b |",
376+ "| ----- | ----- |",
377+ "| 1 | 2 |",
378+ "| 3 | 4 |",
379+ "",
380+ "Trailing paragraph after the table.",
381+ }, "\n"),
382+ },
383+ }
384+}
385+
386+// progressivePrefixes splits doc into n monotonically growing
387+// byte prefixes, ending with the full document. n>=1.
388+func progressivePrefixes(doc string, n int) []string {
389+ if n < 1 {
390+ n = 1
391+ }
392+ out := make([]string, 0, n)
393+ for i := 1; i <= n; i++ {
394+ // integer scaling so the last entry is exactly len(doc)
395+ size := len(doc) * i / n
396+ if i == n {
397+ size = len(doc)
398+ }
399+ out = append(out, doc[:size])
400+ }
401+ return out
402+}
403+
404+// TestStreamingMarkdown_FinalVisuallyEquivalent drives a sequence
405+// of progressive prefixes through streamingMarkdown and asserts
406+// the FINAL output is visually equivalent (per design principle
407+// D) to a fresh full-document render. Strict byte-equality is
408+// not the bar — see the comment in normalizeRender for why.
409+func TestStreamingMarkdown_FinalVisuallyEquivalent(t *testing.T) {
410+ t.Parallel()
411+
412+ const width = 80
413+ const steps = 15
414+
415+ for _, sc := range streamingScenarios() {
416+ t.Run(sc.name, func(t *testing.T) {
417+ t.Parallel()
418+ renderer := newTestRenderer(t, width)
419+ var sm streamingMarkdown
420+ prefixes := progressivePrefixes(sc.doc, steps)
421+
422+ var lastOut string
423+ for _, p := range prefixes {
424+ lastOut = sm.Render(p, width, renderer)
425+ }
426+
427+ fresh := freshRender(t, sc.doc, width)
428+ require.Equal(t, normalizeRender(fresh), normalizeRender(lastOut),
429+ "final streaming output must match a fresh full render visually")
430+ })
431+ }
432+}
433+
434+// TestStreamingMarkdown_IntermediateOutputsPlausible asserts that
435+// every intermediate flush returns a non-empty string and does
436+// not leak raw markdown source through to the user. This is the
437+// "visually plausible" half of T2.
438+func TestStreamingMarkdown_IntermediateOutputsPlausible(t *testing.T) {
439+ t.Parallel()
440+
441+ const width = 80
442+ const steps = 12
443+
444+ for _, sc := range streamingScenarios() {
445+ t.Run(sc.name, func(t *testing.T) {
446+ t.Parallel()
447+ renderer := newTestRenderer(t, width)
448+ var sm streamingMarkdown
449+
450+ for i, p := range progressivePrefixes(sc.doc, steps) {
451+ if p == "" {
452+ continue
453+ }
454+ out := sm.Render(p, width, renderer)
455+ require.NotEmptyf(t, out, "step %d: empty render for prefix len %d", i, len(p))
456+ require.Falsef(t, containsRawMarkdownSource(out),
457+ "step %d: render leaked raw markdown source.\nprefix=%q\nout=%s",
458+ i, p, normalizeRender(out))
459+ }
460+ })
461+ }
462+}
463+
464+// -----------------------------------------------------------------------
465+// T3: cache invalidation tests.
466+// -----------------------------------------------------------------------
467+
468+// TestStreamingMarkdown_WidthChangeInvalidates asserts that a
469+// width change blows away the cached prefix so the next render
470+// is keyed against the new width. We can't observe the cache
471+// directly without reaching into the struct, so we assert the
472+// observable contract: after a width change, the rendered output
473+// reflects the new width AND the streamingMarkdown's internal
474+// cache fields are reset to the new state.
475+func TestStreamingMarkdown_WidthChangeInvalidates(t *testing.T) {
476+ t.Parallel()
477+
478+ doc := "Para one.\n\nPara two.\n\nPara three."
479+ r80 := newTestRenderer(t, 80)
480+ r40 := newTestRenderer(t, 40)
481+ var sm streamingMarkdown
482+
483+ out80 := sm.Render(doc, 80, r80)
484+ require.Equal(t, 80, sm.width, "width must be cached after first render")
485+ cachedPrefix := sm.stablePrefix
486+
487+ out40 := sm.Render(doc, 40, r40)
488+ require.Equal(t, 40, sm.width, "width change must update cached width")
489+ require.NotEqual(t, out80, out40,
490+ "different widths must produce different rendered output")
491+ // stablePrefix may legitimately have re-advanced after the
492+ // reset (tryAdvanceFromEmpty), but if it has, it can no
493+ // longer carry the OLD width's render. We assert the cache
494+ // reset by checking that the cached prefix length is at
495+ // most the current content length.
496+ require.True(t, len(sm.stablePrefix) <= len(doc),
497+ "stable prefix must be a prefix of the current content")
498+ _ = cachedPrefix
499+}
500+
501+// TestStreamingMarkdown_NonPrefixContentInvalidates verifies
502+// that content which is NOT a prefix-extension of the cached
503+// stable prefix triggers a Reset and a fresh render path. This
504+// guards the "user retried the turn" case.
505+func TestStreamingMarkdown_NonPrefixContentInvalidates(t *testing.T) {
506+ t.Parallel()
507+
508+ const width = 80
509+ r := newTestRenderer(t, width)
510+ var sm streamingMarkdown
511+
512+ // Drive a streaming sequence so the cache picks up a stable
513+ // prefix.
514+ doc := "Para one.\n\nPara two.\n\nPara three."
515+ for _, p := range progressivePrefixes(doc, 6) {
516+ _ = sm.Render(p, width, r)
517+ }
518+ require.NotEmpty(t, sm.stablePrefix,
519+ "stable prefix must be populated after streaming a multi-paragraph doc")
520+
521+ // Now switch to entirely different content (user retried).
522+ other := "Completely different opening paragraph.\n\nAnd a second."
523+ out := sm.Render(other, width, r)
524+ require.NotEmpty(t, out)
525+ // stablePrefix must be a prefix of `other`, i.e. cache was
526+ // reset off the OLD content.
527+ require.True(t, strings.HasPrefix(other, sm.stablePrefix),
528+ "stable prefix must be reset to a prefix of the new content")
529+
530+ // Visual equivalence to a fresh render of `other`.
531+ fresh := freshRender(t, other, width)
532+ require.Equal(t, normalizeRender(fresh), normalizeRender(out),
533+ "render after non-prefix content change must match a fresh render")
534+}
535+
536+// TestStreamingMarkdown_ResetClearsCache asserts Reset() drops
537+// every cached field; the next render is necessarily a full
538+// render path.
539+func TestStreamingMarkdown_ResetClearsCache(t *testing.T) {
540+ t.Parallel()
541+
542+ const width = 80
543+ r := newTestRenderer(t, width)
544+ var sm streamingMarkdown
545+
546+ doc := "Para one.\n\nPara two.\n\nPara three."
547+ _ = sm.Render(doc, width, r)
548+ // The sample doc has safe boundaries so the cache should
549+ // have advanced. If for some reason it didn't, we still
550+ // want Reset to be a no-op-safe operation; assert the
551+ // post-Reset state directly.
552+ sm.Reset()
553+ require.Equal(t, 0, sm.width)
554+ require.Equal(t, "", sm.stablePrefix)
555+ require.Equal(t, "", sm.stablePrefixRender)
556+
557+ // Next render must be a full render path. Drive one step
558+ // and verify the output matches a fresh full render.
559+ out := sm.Render(doc, width, r)
560+ fresh := freshRender(t, doc, width)
561+ require.Equal(t, normalizeRender(fresh), normalizeRender(out))
562+}
563+
564+// -----------------------------------------------------------------------
565+// T4: fallback safety.
566+// -----------------------------------------------------------------------
567+
568+// TestStreamingMarkdown_NoSafeBoundaryAlwaysFullRenders covers
569+// the "one giant table being built character by character" case.
570+// Every flush must fall back to a full render; the cache must
571+// not advance into an unsafe state. We compare each flush to a
572+// fresh full render of the same prefix; bytes must match for
573+// each prefix individually.
574+//
575+// (Byte equality is sound here because no concatenation happens:
576+// the streaming path delegates straight to renderer.Render when
577+// the cache is empty and no safe boundary exists.)
578+func TestStreamingMarkdown_NoSafeBoundaryAlwaysFullRenders(t *testing.T) {
579+ t.Parallel()
580+
581+ const width = 80
582+
583+ // One growing table — no blank lines anywhere, so no
584+ // boundary candidate is ever found.
585+ doc := strings.Join([]string{
586+ "| col a | col b | col c |",
587+ "| ----- | ----- | ----- |",
588+ "| 1 | 2 | 3 |",
589+ "| 4 | 5 | 6 |",
590+ "| 7 | 8 | 9 |",
591+ "| 10 | 11 | 12 |",
592+ "| 13 | 14 | 15 |",
593+ "| 16 | 17 | 18 |",
594+ "| 19 | 20 | 21 |",
595+ "| 22 | 23 | 24 |",
596+ }, "\n")
597+ require.Equal(t, -1, findSafeMarkdownBoundary(doc),
598+ "sanity check: no blank lines, no safe boundary")
599+
600+ r := newTestRenderer(t, width)
601+ var sm streamingMarkdown
602+
603+ prefixes := progressivePrefixes(doc, 10)
604+ for i, p := range prefixes {
605+ if p == "" {
606+ continue
607+ }
608+ out := sm.Render(p, width, r)
609+ fresh := freshRender(t, p, width)
610+ require.Equalf(t, fresh, out,
611+ "step %d (len=%d): streaming output must byte-equal a fresh render when boundary detection fails",
612+ i, len(p))
613+ }
614+ // Cache must remain empty: no boundary was ever found, no
615+ // width change occurred, no advance ever cached anything.
616+ require.Equal(t, "", sm.stablePrefix,
617+ "stable prefix must remain empty when no safe boundary ever exists")
618+}
619+
620+// TestStreamingMarkdown_NoSafeBoundaryDoesNotCrash is the
621+// minimum-viability assertion of T4: even when boundary
622+// detection fails on every flush the streaming path must not
623+// crash and must produce non-empty output for non-empty input.
624+func TestStreamingMarkdown_NoSafeBoundaryDoesNotCrash(t *testing.T) {
625+ t.Parallel()
626+
627+ const width = 80
628+ r := newTestRenderer(t, width)
629+ var sm streamingMarkdown
630+
631+ // A deeply-pathological input: a single line that grows
632+ // one character at a time. There is never a blank-line
633+ // separator so the cache is never advanced.
634+ src := "The quick brown fox jumps over the lazy dog."
635+ for i := 1; i <= len(src); i++ {
636+ out := sm.Render(src[:i], width, r)
637+ require.NotEmpty(t, out, "streaming output must not be empty for non-empty input")
638+ }
639+}
640+
641+// -----------------------------------------------------------------------
642+// Integration assertions on the wired-in path.
643+// -----------------------------------------------------------------------
644+
645+// -----------------------------------------------------------------------
646+// T5 / T6 / T7: anywhere-in-prefix hazards (B1 / B2 / B3 from the
647+// F8 round-2 review). For each hazard we drive every progressive
648+// prefix of a document that exercises the hazard through the cache
649+// and assert two contracts:
650+//
651+// 1. The cached stable prefix never contains the hazard. If the
652+// hazard line is at byte offset H, then after every flush
653+// len(sm.stablePrefix) <= H. This is the "no silent
654+// corruption" half — the algorithm cannot accept a boundary
655+// that splits across the hazard.
656+//
657+// 2. The final flush is visually equivalent to a fresh full
658+// render of the complete document. This is the same T2-style
659+// equivalence assertion ported to the new doc shapes.
660+// -----------------------------------------------------------------------
661+
662+// nonBlankLines returns the non-blank visible lines of s with
663+// per-line trailing whitespace trimmed. Used to compare two
664+// rendered fragments for content equivalence when paragraph-
665+// margin behaviour legitimately differs between a single fresh
666+// render and a streaming split render (per F8 design principle D
667+// — visual equivalence is the bar, byte-equivalence is not).
668+//
669+// Some glamour block types (notably HTML blocks and reference
670+// link definitions) interact with adjacent paragraph blocks
671+// during a single render — adjacency effectively suppresses the
672+// blank-line margin between blocks. When the streaming path
673+// renders the prefix and trail in separate calls, the seam is
674+// re-introduced as a blank line. The visible TEXT is identical;
675+// only the inter-block margin differs.
676+func nonBlankLines(s string) []string {
677+ clean := stripANSI(s)
678+ out := make([]string, 0)
679+ for _, l := range strings.Split(clean, "\n") {
680+ l = strings.TrimRight(l, " \t")
681+ if strings.TrimSpace(l) == "" {
682+ continue
683+ }
684+ out = append(out, l)
685+ }
686+ return out
687+}
688+
689+// runProgressiveBoundaryRespectTest is the shared body of T5/T6/T7.
690+// It accepts a document and the byte offset of the line whose
691+// PRESENCE in the prefix must trigger the hazard reject; the
692+// cached stable prefix may never extend past hazardLineOffset.
693+//
694+// The final-output equivalence check is content-based (non-blank
695+// lines compared) rather than full-normalization: see
696+// nonBlankLines for the reason.
697+func runProgressiveBoundaryRespectTest(t *testing.T, doc string, hazardLineOffset int) {
698+ t.Helper()
699+ const width = 80
700+ const steps = 25
701+
702+ renderer := newTestRenderer(t, width)
703+ var sm streamingMarkdown
704+
705+ prefixes := progressivePrefixes(doc, steps)
706+ var lastOut string
707+ for i, p := range prefixes {
708+ if p == "" {
709+ continue
710+ }
711+ lastOut = sm.Render(p, width, renderer)
712+ require.NotEmptyf(t, lastOut, "step %d: empty render", i)
713+ require.LessOrEqualf(t, len(sm.stablePrefix), hazardLineOffset,
714+ "step %d: cached stable prefix advanced past the hazard line\n"+
715+ "prefix len=%d, hazard at %d, sm.stablePrefix=%q",
716+ i, len(sm.stablePrefix), hazardLineOffset, sm.stablePrefix)
717+ }
718+
719+ fresh := freshRender(t, doc, width)
720+ require.Equal(t, nonBlankLines(fresh), nonBlankLines(lastOut),
721+ "final streaming output must contain the same non-blank lines as a fresh full render")
722+}
723+
724+// TestStreamingMarkdown_LooseListContinuation locks in the B1 fix.
725+// A loose list followed by a continuation paragraph and then a
726+// trailing paragraph creates a candidate boundary between the list
727+// item and its continuation; the trailing non-blank line of that
728+// candidate prefix is the continuation paragraph (not a list
729+// marker), so the line-only check would accept it. The
730+// anywhere-in-prefix list-marker check rejects it.
731+func TestStreamingMarkdown_LooseListContinuation(t *testing.T) {
732+ t.Parallel()
733+
734+ doc := strings.Join([]string{
735+ "Intro paragraph.",
736+ "",
737+ "- item one",
738+ "",
739+ " continuation paragraph still belongs to item one",
740+ "",
741+ "- item two",
742+ "",
743+ "Trailing paragraph after the list.",
744+ }, "\n")
745+
746+ // The first list marker line begins after "Intro paragraph.\n\n".
747+ // The cached stable prefix may include that boundary (BEFORE
748+ // the list opens) but must never advance into the list.
749+ hazardOffset := strings.Index(doc, "- item one")
750+ require.Greater(t, hazardOffset, 0, "test setup")
751+
752+ runProgressiveBoundaryRespectTest(t, doc, hazardOffset)
753+}
754+
755+// TestStreamingMarkdown_HTMLBlock locks in the B2 fix. A raw HTML
756+// block followed by a paragraph creates a candidate boundary
757+// between the closed HTML block and the trailing paragraph. The
758+// anywhere-in-prefix HTML-opener check rejects any boundary that
759+// would include the HTML block in the stable prefix.
760+func TestStreamingMarkdown_HTMLBlock(t *testing.T) {
761+ t.Parallel()
762+
763+ doc := strings.Join([]string{
764+ "Intro paragraph.",
765+ "",
766+ "<div>",
767+ "some block content",
768+ "</div>",
769+ "",
770+ "Trailing paragraph after the HTML block.",
771+ }, "\n")
772+
773+ hazardOffset := strings.Index(doc, "<div>")
774+ require.Greater(t, hazardOffset, 0, "test setup")
775+
776+ runProgressiveBoundaryRespectTest(t, doc, hazardOffset)
777+}
778+
779+// TestStreamingMarkdown_HTMLBlockType7 covers HTML block type 7
780+// (CommonMark): a generic open/close tag whose name is NOT in the
781+// fixed type-6 set still opens an HTML block and must forfeit any
782+// boundary that would split the block off from following content.
783+func TestStreamingMarkdown_HTMLBlockType7(t *testing.T) {
784+ t.Parallel()
785+
786+ doc := strings.Join([]string{
787+ "Intro paragraph.",
788+ "",
789+ "<custom-tag>",
790+ "some block content",
791+ "</custom-tag>",
792+ "",
793+ "Trailing paragraph after the custom-tag block.",
794+ }, "\n")
795+
796+ hazardOffset := strings.Index(doc, "<custom-tag>")
797+ require.Greater(t, hazardOffset, 0, "test setup")
798+
799+ runProgressiveBoundaryRespectTest(t, doc, hazardOffset)
800+}
801+
802+// TestStreamingMarkdown_LinkRefDefinition locks in the B3 fix. A
803+// reference link definition followed by a paragraph that uses the
804+// reference creates a boundary candidate between the def and the
805+// paragraph; rendering them in separate glamour passes loses the
806+// definition. The anywhere-in-prefix ref-def check rejects.
807+func TestStreamingMarkdown_LinkRefDefinition(t *testing.T) {
808+ t.Parallel()
809+
810+ doc := strings.Join([]string{
811+ "Intro paragraph.",
812+ "",
813+ "[ref]: http://example.com",
814+ "",
815+ "Trailing paragraph that links to [the example][ref] inline.",
816+ }, "\n")
817+
818+ hazardOffset := strings.Index(doc, "[ref]:")
819+ require.Greater(t, hazardOffset, 0, "test setup")
820+
821+ runProgressiveBoundaryRespectTest(t, doc, hazardOffset)
822+}
823+
824+// TestAssistantStreamingContent_ResetOnClearCache guards the
825+// integration contract that ClearItemCaches (style change) drops
826+// the streaming-markdown cache. Without this, a style change
827+// would leave the OLD style's ANSI sequences embedded in the
828+// stable-prefix render and the next flush would visually mix
829+// styles.
830+func TestAssistantStreamingContent_ResetOnClearCache(t *testing.T) {
831+ t.Parallel()
832+
833+ sty := styles.CharmtonePantera()
834+ doc := "Para one.\n\nPara two.\n\nPara three."
835+ msg := finishedAssistantMessage("stream-clear", doc)
836+ item := NewAssistantMessageItem(&sty, msg).(*AssistantMessageItem)
837+
838+ const width = 80
839+ _ = item.RawRender(width)
840+ // Drive a second message that extends the content so the
841+ // streaming cache has a chance to advance (if it would).
842+ doc2 := doc + "\n\nFour."
843+ item.SetMessage(finishedAssistantMessage("stream-clear", doc2))
844+ _ = item.RawRender(width)
845+
846+ // Now wipe the caches the way ClearItemCaches does.
847+ item.clearCache()
848+
849+ require.Equal(t, "", item.streamingContent.stablePrefix,
850+ "clearCache must Reset the streaming-markdown cache")
851+ require.Equal(t, "", item.streamingContent.stablePrefixRender)
852+ require.Equal(t, 0, item.streamingContent.width)
853+}
internal/ui/chat/streaming_markdown.gonew file+740
@@ -0,0 +1,740 @@
1+package chat
2+
3+import (
4+ "strings"
5+
6+ "charm.land/glamour/v2"
7+ "github.com/charmbracelet/crush/internal/ui/common"
8+)
9+
10+// streamingMarkdown caches a "stable prefix" glamour render so each
11+// streaming flush only re-renders the trailing portion of the
12+// document. F8 of docs/notes/2026-05-12-chat-rendering-perf.md.
13+//
14+// The boundary between "stable" and "trailing" is detected by
15+// [findSafeMarkdownBoundary]: a position immediately after a blank
16+// line at which we can prove no markdown construct is open
17+// (fenced code block, list, table, block quote, setext header).
18+//
19+// Two renders concatenated are NOT generally equal to a single
20+// render of the whole document — glamour's wrap state is reset
21+// between calls. The boundary check is therefore deliberately
22+// conservative; whenever it has the slightest doubt the call
23+// falls back to a full render and the cache is left untouched.
24+//
25+// Invariants:
26+//
27+// - stablePrefix is always a literal byte prefix of the most
28+// recently rendered content. If a new content does not have
29+// stablePrefix as its prefix the cache is dropped.
30+// - stablePrefixRender is the glamour render of stablePrefix
31+// alone, with surrounding whitespace trimmed for clean
32+// concatenation.
33+// - width is the glamour wrap width that produced
34+// stablePrefixRender. A width change drops the cache.
35+type streamingMarkdown struct {
36+ width int
37+ stablePrefix string
38+ stablePrefixRender string
39+}
40+
41+// Reset drops every cached field. After Reset the next Render call
42+// is guaranteed to be a full render.
43+func (s *streamingMarkdown) Reset() {
44+ s.width = 0
45+ s.stablePrefix = ""
46+ s.stablePrefixRender = ""
47+}
48+
49+// Render returns the glamour render of content at the given width,
50+// reusing the cached stable-prefix render when it is safe to do so.
51+// On any uncertainty the call falls back to a full render via
52+// renderer and leaves the cache untouched (or drops it).
53+//
54+// The returned string has its trailing newline trimmed to match
55+// the existing renderMarkdown contract on AssistantMessageItem.
56+//
57+// Concurrency: glamour's Render is stateful and not safe for
58+// concurrent invocation on a shared renderer. Crush's TUI is
59+// single-threaded so production never contends, but parallel
60+// callers (most notably the test suite) must serialize. We hold
61+// [common.LockMarkdownRenderer] for the entire prefix +
62+// trailing render sequence so other goroutines cannot interleave
63+// their own Render calls and corrupt goldmark's BlockStack.
64+func (s *streamingMarkdown) Render(content string, width int, renderer *glamour.TermRenderer) string {
65+ mu := common.LockMarkdownRenderer(renderer)
66+ mu.Lock()
67+ defer mu.Unlock()
68+ full := func() string {
69+ out, err := renderer.Render(content)
70+ if err != nil {
71+ return content
72+ }
73+ return strings.TrimSuffix(out, "\n")
74+ }
75+
76+ // Width change OR content not a prefix-extension: drop cache,
77+ // full render, optionally try to seed a fresh boundary on this
78+ // call (step "f" in the design note).
79+ if width != s.width || !strings.HasPrefix(content, s.stablePrefix) {
80+ s.Reset()
81+ s.width = width
82+ out := full()
83+ s.tryAdvanceFromEmpty(content, width, renderer)
84+ return out
85+ }
86+
87+ boundary := findSafeMarkdownBoundary(content)
88+ if boundary < 0 {
89+ // No safe boundary anywhere yet. Full render; do not
90+ // modify the cache (a future flush may find one).
91+ return full()
92+ }
93+
94+ if boundary <= len(s.stablePrefix) {
95+ // Cached prefix already covers an at-least-as-late
96+ // boundary. Render the trailing partial fresh and glue.
97+ trail := content[len(s.stablePrefix):]
98+ return glueRenders(s.stablePrefixRender, s.renderTrailing(trail, renderer))
99+ }
100+
101+ // boundary > len(stablePrefix): we have a NEW chunk of safe
102+ // content. Render the new chunk, append to stablePrefixRender,
103+ // promote the boundary, then render the remaining trail.
104+ newChunk := content[len(s.stablePrefix):boundary]
105+ newChunkRender := s.renderTrailing(newChunk, renderer)
106+ s.stablePrefixRender = glueRenders(s.stablePrefixRender, newChunkRender)
107+ s.stablePrefix = content[:boundary]
108+
109+ trail := content[boundary:]
110+ if trail == "" {
111+ // boundary == len(content): no trailing content. Returning
112+ // the cached prefix render directly is correct.
113+ return s.stablePrefixRender
114+ }
115+ return glueRenders(s.stablePrefixRender, s.renderTrailing(trail, renderer))
116+}
117+
118+// tryAdvanceFromEmpty seeds the cache from a fresh state. We've
119+// already paid the cost of a full render of `content`; if there is
120+// a safe boundary inside it, render the prefix once more (cheap
121+// relative to the full render we just did) and cache it so the
122+// next flush can avoid the full work.
123+//
124+// This is the optional optimisation step "f" from the design
125+// note. We render the prefix separately rather than try to
126+// recover it from the full render output because two renders
127+// concatenated ≠ a single render of the whole, and we prefer the
128+// cached prefix render to be byte-for-byte what we'd produce on a
129+// future cached call.
130+func (s *streamingMarkdown) tryAdvanceFromEmpty(content string, width int, renderer *glamour.TermRenderer) {
131+ boundary := findSafeMarkdownBoundary(content)
132+ if boundary <= 0 {
133+ return
134+ }
135+ prefix := content[:boundary]
136+ out, err := renderer.Render(prefix)
137+ if err != nil {
138+ return
139+ }
140+ s.stablePrefix = prefix
141+ s.stablePrefixRender = trimGlamourMargins(out)
142+ s.width = width
143+}
144+
145+// renderTrailing renders a trailing partial as a fresh glamour
146+// document and trims the surrounding whitespace so it can be
147+// concatenated to a cached prefix render without doubled blank
148+// lines.
149+func (s *streamingMarkdown) renderTrailing(text string, renderer *glamour.TermRenderer) string {
150+ if text == "" {
151+ return ""
152+ }
153+ out, err := renderer.Render(text)
154+ if err != nil {
155+ return text
156+ }
157+ return trimGlamourMargins(out)
158+}
159+
160+// glueRenders concatenates two glamour-rendered fragments with a
161+// single blank line separator. Glamour outputs typically carry
162+// their own surrounding margins; trimming on both sides and
163+// gluing with "\n\n" prevents the visible double-margin seam.
164+//
165+// Empty fragments are tolerated so the same helper works for the
166+// "boundary == len(content)" path where there is no trailing
167+// segment.
168+func glueRenders(prefix, trail string) string {
169+ prefix = trimGlamourMargins(prefix)
170+ trail = trimGlamourMargins(trail)
171+ switch {
172+ case prefix == "" && trail == "":
173+ return ""
174+ case prefix == "":
175+ return trail
176+ case trail == "":
177+ return prefix
178+ default:
179+ return prefix + "\n\n" + trail
180+ }
181+}
182+
183+// trimGlamourMargins strips leading and trailing whitespace
184+// (including newlines) from a glamour-rendered fragment.
185+// Glamour adds a leading blank line for documents that open with
186+// a heading or paragraph, plus a trailing newline; both must be
187+// removed before concatenation.
188+func trimGlamourMargins(s string) string {
189+ return strings.Trim(s, " \t\n")
190+}
191+
192+// findSafeMarkdownBoundary returns the byte offset of the END of
193+// the latest safe boundary in content, i.e. the offset such that
194+// content[:boundary] is a valid stable-prefix candidate. The
195+// returned offset always points immediately after a blank-line
196+// separator, so concatenating a fresh render of content[boundary:]
197+// to a cached render of content[:boundary] does not require glamour
198+// to share state across the cut.
199+//
200+// Returns -1 when no safe boundary exists. SAFETY FIRST: any time
201+// we have the slightest doubt we return -1 and let the caller fall
202+// back to a full render.
203+//
204+// Decision tree, in order of preference (latest boundary wins):
205+//
206+// 1. Walk backward through every "blank line" position p such that
207+// content[:p] ends with "\n\n" (or "\n[ \t]*\n").
208+// 2. For each candidate, check that content[:p] has an even
209+// number of triple-backtick fence lines (no open fenced
210+// block). Any odd count means we'd be cutting inside a fence
211+// and mis-syntax-highlighting the trailing partial.
212+// 2b. Reject if any line in content[:p] (outside fenced blocks)
213+// is a list-marker line, an HTML-block opener, or a link
214+// reference definition. See [prefixHasOpenHazard] for the
215+// reasoning behind these "anywhere in prefix" rejects.
216+// 3. Reject if the last non-blank line of content[:p] is:
217+// - a list item marker line ("^\s*([-*+]|\d+\.)\s")
218+// - a table line (contains "|")
219+// - a block quote ("^\s*>")
220+// - a setext header underline ("^=+\s*$" or "^-+\s*$")
221+// - an indented code line (4+ leading spaces or a tab)
222+// 4. Reject if the line immediately AFTER the boundary (skipping
223+// leading blank lines) looks like a setext underline (a line
224+// of '=' or '-' only). Rendering the prefix as a paragraph
225+// would change once the underline arrived; that's exactly the
226+// "splitting changes the prefix render" hazard §4.4 calls out.
227+//
228+// Returns the byte offset of the first character AFTER the blank
229+// line, i.e. the start of the trailing segment.
230+func findSafeMarkdownBoundary(content string) int {
231+ if len(content) == 0 {
232+ return -1
233+ }
234+
235+ // Iterate every blank-line position from latest to earliest.
236+ for p := blankLineBefore(content, len(content)); p > 0; p = blankLineBefore(content, p-1) {
237+ if !isSafeBoundaryAt(content, p) {
238+ continue
239+ }
240+ return p
241+ }
242+ return -1
243+}
244+
245+// blankLineBefore returns the byte offset of the first character
246+// AFTER the latest blank-line separator that ends strictly before
247+// `until`. A blank-line separator is a sequence "\n([ \t]*\n)+"
248+// — one newline, then one or more lines containing only spaces or
249+// tabs and terminated by another newline. The returned offset is
250+// the start of the first non-blank line that follows the
251+// separator (or the position immediately after the final newline,
252+// if no further content remains).
253+//
254+// Returns -1 when no blank-line separator exists before `until`.
255+func blankLineBefore(content string, until int) int {
256+ if until <= 0 {
257+ return -1
258+ }
259+ // Walk backward looking for a newline followed (after optional
260+ // blank-line content) by another newline. We track the latest
261+ // newline we've seen; if the next earlier newline has only
262+ // blank chars between them, we have a blank-line separator
263+ // and the boundary sits immediately after the latest newline.
264+ end := until
265+ for end > 0 {
266+ nl := strings.LastIndexByte(content[:end], '\n')
267+ if nl < 0 {
268+ return -1
269+ }
270+ // Look for an earlier newline whose gap to nl is empty
271+ // or whitespace only.
272+ prev := strings.LastIndexByte(content[:nl], '\n')
273+ for prev >= 0 {
274+ gap := content[prev+1 : nl]
275+ if isBlankOrSpaces(gap) {
276+ return nl + 1
277+ }
278+ // Gap had non-whitespace; nl is not a blank-line
279+ // separator. Move up: try with the earlier newline as
280+ // the new "nl" candidate.
281+ break
282+ }
283+ end = nl
284+ }
285+ return -1
286+}
287+
288+// isBlankOrSpaces reports whether s consists entirely of spaces
289+// and tabs (or is empty).
290+func isBlankOrSpaces(s string) bool {
291+ for i := range len(s) {
292+ if s[i] != ' ' && s[i] != '\t' {
293+ return false
294+ }
295+ }
296+ return true
297+}
298+
299+// isSafeBoundaryAt reports whether content[:p] is a safe stable
300+// prefix. p must be a blank-line boundary (start of a line, with a
301+// blank line immediately preceding).
302+//
303+// Beyond the last-line checks, three "anywhere in the prefix"
304+// hazards force a reject because they cannot be reliably reasoned
305+// about by inspecting the trailing line alone. For each of these
306+// the simplest, safest rule was chosen — see prefixHasOpenHazard.
307+func isSafeBoundaryAt(content string, p int) bool {
308+ prefix := content[:p]
309+
310+ // (2) Even number of triple-backtick fence lines.
311+ if countFenceLines(prefix)%2 != 0 {
312+ return false
313+ }
314+
315+ // (2b) Anywhere-in-prefix hazards: open list (B1), HTML block
316+ // opener (B2), reference link definition (B3). Any of these
317+ // anywhere in the prefix forces a fallback.
318+ if prefixHasOpenHazard(prefix) {
319+ return false
320+ }
321+
322+ // (3) Inspect the last non-blank line of the prefix.
323+ lastLine := lastNonBlankLine(prefix)
324+ if lastLine != "" && lineOpensConstruct(lastLine) {
325+ return false
326+ }
327+
328+ // (4) If anything follows, make sure it doesn't look like a
329+ // setext underline that would retroactively turn the last
330+ // paragraph of the prefix into a header.
331+ if rest := content[p:]; rest != "" {
332+ first := firstNonBlankLine(rest)
333+ if isSetextUnderlineCandidate(first) {
334+ return false
335+ }
336+ }
337+
338+ return true
339+}
340+
341+// prefixHasOpenHazard reports whether prefix contains any of three
342+// constructs that cannot be safely cut at a blank-line boundary
343+// even when the immediately preceding line looks fine. Each check
344+// uses the SIMPLEST viable conservative rule per the F8 round-2
345+// review:
346+//
347+// B1 (loose lists). A loose list has a blank line between an item
348+// and a continuation paragraph that begins with indentation
349+// but no list marker. If a candidate boundary lands on that
350+// blank line, the prefix's trailing non-blank line is the
351+// continuation paragraph, NOT a list marker, so the last-line
352+// check would accept it even though the list is still open.
353+//
354+// Rule chosen: any list-marker line ANYWHERE in the prefix
355+// forces -1. This is overly conservative — it forfeits
356+// boundary advancement past a closed list — but it eliminates
357+// the entire bug class with zero parsing of CommonMark's
358+// loose-list closure semantics. We retain the most useful
359+// boundary in practice: the one BEFORE the list opens (no
360+// marker has appeared in the prefix yet).
361+//
362+// B2 (HTML blocks). CommonMark defines seven HTML-block opener
363+// patterns (script/pre/style/textarea, comments, processing
364+// instructions, CDATA, declarations, recognised tag names).
365+// If the prefix opens an HTML block that the suffix closes,
366+// splitting renders the prefix as raw HTML and the suffix as
367+// prose.
368+//
369+// Rule chosen: any HTML-block opener anywhere in the prefix
370+// forces -1. Same trade-off as B1 — the typical assistant
371+// output contains no raw HTML, so the perf cost is zero in
372+// the common case.
373+//
374+// B3 (reference link definitions). A line of the form
375+// "[label]: <url>" defines a link reference that the suffix
376+// may later use as "[text][label]". Splitting the document
377+// loses the definition because each half is rendered as an
378+// independent glamour document.
379+//
380+// Rule chosen: any reference link definition line anywhere in
381+// the prefix forces -1. Suffix-side reference detection is
382+// fragile (three syntaxes: [text][label], [label][], [label]),
383+// so the prefix-side check is the simpler safe choice.
384+//
385+// All three rules accept the perf hit of "no boundary after a
386+// list / HTML block / link def" in exchange for guaranteed
387+// soundness. If profiling shows this kills the F8 win on real
388+// streaming traces, the next iteration can promote each rule to
389+// its less-conservative variant (closure-aware list tracking,
390+// per-tag HTML close detection, suffix-aware ref tracking).
391+func prefixHasOpenHazard(prefix string) bool {
392+ inFence := false
393+ for line := range splitLines(prefix) {
394+ // Track fenced state so list/html/ref patterns inside a
395+ // fenced code block do not falsely trigger the hazards.
396+ if isFenceLine(line) {
397+ inFence = !inFence
398+ continue
399+ }
400+ if inFence {
401+ continue
402+ }
403+ trimmed := strings.TrimLeft(line, " \t")
404+ if trimmed == "" {
405+ continue
406+ }
407+ // B1: any list-item marker.
408+ if isListItemMarker(trimmed) {
409+ return true
410+ }
411+ // B2: HTML block opener.
412+ if isHTMLBlockOpener(line) {
413+ return true
414+ }
415+ // B3: link reference definition.
416+ if isLinkRefDefinition(line) {
417+ return true
418+ }
419+ }
420+ return false
421+}
422+
423+// countFenceLines counts lines that begin a fenced code block in
424+// the CommonMark sense: a line whose first non-whitespace run is
425+// at least three consecutive backticks (or tildes). Each such
426+// line toggles the fenced state, so an even count means every
427+// opened fence has been closed.
428+//
429+// We accept up to three leading spaces of indentation (CommonMark
430+// rule) and require the fence characters to be the FIRST
431+// non-whitespace content of the line. We deliberately do NOT
432+// attempt to parse info-strings or differentiate opener from
433+// closer beyond toggling — a closing fence is just any line
434+// whose first non-whitespace run is >=3 of the same fence char.
435+func countFenceLines(s string) int {
436+ n := 0
437+ for line := range splitLines(s) {
438+ if isFenceLine(line) {
439+ n++
440+ }
441+ }
442+ return n
443+}
444+
445+// isFenceLine reports whether line opens or closes a fenced code
446+// block.
447+func isFenceLine(line string) bool {
448+ // Strip up to 3 spaces of indentation.
449+ i := 0
450+ for i < len(line) && i < 3 && line[i] == ' ' {
451+ i++
452+ }
453+ if i >= len(line) {
454+ return false
455+ }
456+ c := line[i]
457+ if c != '`' && c != '~' {
458+ return false
459+ }
460+ run := 0
461+ for i < len(line) && line[i] == c {
462+ i++
463+ run++
464+ }
465+ return run >= 3
466+}
467+
468+// lastNonBlankLine returns the last non-blank line of s, or ""
469+// when every line is blank.
470+func lastNonBlankLine(s string) string {
471+ last := ""
472+ for line := range splitLines(s) {
473+ if strings.TrimSpace(line) != "" {
474+ last = line
475+ }
476+ }
477+ return last
478+}
479+
480+// firstNonBlankLine returns the first non-blank line of s, or ""
481+// when every line is blank.
482+func firstNonBlankLine(s string) string {
483+ for line := range splitLines(s) {
484+ if strings.TrimSpace(line) != "" {
485+ return line
486+ }
487+ }
488+ return ""
489+}
490+
491+// splitLines yields the lines of s without their terminators. The
492+// final segment is yielded even if not newline-terminated.
493+func splitLines(s string) func(yield func(string) bool) {
494+ return func(yield func(string) bool) {
495+ start := 0
496+ for i := 0; i < len(s); i++ {
497+ if s[i] == '\n' {
498+ if !yield(s[start:i]) {
499+ return
500+ }
501+ start = i + 1
502+ }
503+ }
504+ if start <= len(s)-1 {
505+ yield(s[start:])
506+ }
507+ }
508+}
509+
510+// lineOpensConstruct reports whether line keeps a markdown
511+// construct open across the boundary. We err conservatively —
512+// any case that smells like list/table/quote/setext/indented-code
513+// returns true.
514+func lineOpensConstruct(line string) bool {
515+ // Indented code: a tab, or 4+ leading spaces.
516+ if len(line) > 0 && line[0] == '\t' {
517+ return true
518+ }
519+ if strings.HasPrefix(line, " ") {
520+ return true
521+ }
522+
523+ trimmed := strings.TrimLeft(line, " \t")
524+ if trimmed == "" {
525+ return false
526+ }
527+
528+ // Block quote.
529+ if trimmed[0] == '>' {
530+ return true
531+ }
532+
533+ // List item: "- " "* " "+ " or "<digits>. " or "<digits>) ".
534+ if isListItemMarker(trimmed) {
535+ return true
536+ }
537+
538+ // Table: any pipe character anywhere in the line. Conservative:
539+ // pipe-in-prose is rare and the cost of bailing is one slow
540+ // frame.
541+ if strings.ContainsRune(line, '|') {
542+ return true
543+ }
544+
545+ // Setext underline candidate as the LAST line of the prefix:
546+ // this would be a setext header for an even-earlier paragraph.
547+ // Refuse to split at all in this case — the boundary is right
548+ // in the middle of a header.
549+ if isSetextUnderlineCandidate(trimmed) {
550+ return true
551+ }
552+
553+ return false
554+}
555+
556+// isListItemMarker reports whether line (already left-trimmed)
557+// starts with a CommonMark list-item marker followed by a space
558+// or tab.
559+func isListItemMarker(line string) bool {
560+ if line == "" {
561+ return false
562+ }
563+ c := line[0]
564+ if c == '-' || c == '*' || c == '+' {
565+ if len(line) >= 2 && (line[1] == ' ' || line[1] == '\t') {
566+ return true
567+ }
568+ return false
569+ }
570+ // Ordered list: digits followed by '.' or ')' and a space.
571+ i := 0
572+ for i < len(line) && line[i] >= '0' && line[i] <= '9' {
573+ i++
574+ }
575+ if i == 0 || i > 9 {
576+ return false
577+ }
578+ if i >= len(line) {
579+ return false
580+ }
581+ if line[i] != '.' && line[i] != ')' {
582+ return false
583+ }
584+ if i+1 >= len(line) {
585+ return false
586+ }
587+ return line[i+1] == ' ' || line[i+1] == '\t'
588+}
589+
590+// isSetextUnderlineCandidate reports whether line (with optional
591+// leading whitespace) consists entirely of '=' or entirely of '-'
592+// characters with optional trailing whitespace. CommonMark
593+// requires no leading whitespace on the underline; we accept up
594+// to three spaces for safety so an indented underline still
595+// blocks a split.
596+func isSetextUnderlineCandidate(line string) bool {
597+ // Strip leading whitespace.
598+ i := 0
599+ for i < len(line) && (line[i] == ' ' || line[i] == '\t') {
600+ i++
601+ }
602+ if i == len(line) {
603+ return false
604+ }
605+ c := line[i]
606+ if c != '=' && c != '-' {
607+ return false
608+ }
609+ j := i
610+ for j < len(line) && line[j] == c {
611+ j++
612+ }
613+ // Allow trailing whitespace.
614+ for j < len(line) {
615+ if line[j] != ' ' && line[j] != '\t' {
616+ return false
617+ }
618+ j++
619+ }
620+ // Need at least one underline character. "-" alone is also a
621+ // list marker without a trailing space; the listItem check
622+ // covers the marker case before we get here.
623+ return j-i >= 1
624+}
625+
626+// isHTMLBlockOpener reports whether line begins one of the seven
627+// CommonMark HTML block patterns. We accept up to three spaces of
628+// leading indentation (CommonMark rule). Matching is intentionally
629+// loose — we only need to know the line "looks like an HTML
630+// block start", not parse the contained markup.
631+func isHTMLBlockOpener(line string) bool {
632+ // Strip up to 3 spaces of indentation.
633+ i := 0
634+ for i < len(line) && i < 3 && line[i] == ' ' {
635+ i++
636+ }
637+ rest := line[i:]
638+ if len(rest) < 2 || rest[0] != '<' {
639+ return false
640+ }
641+
642+ // Type 2: HTML comment "<!--".
643+ if strings.HasPrefix(rest, "<!--") {
644+ return true
645+ }
646+ // Type 3: processing instruction "<?".
647+ if strings.HasPrefix(rest, "<?") {
648+ return true
649+ }
650+ // Type 5: CDATA "<![CDATA[".
651+ if strings.HasPrefix(rest, "<![CDATA[") {
652+ return true
653+ }
654+ // Type 4: declaration "<!" followed by an ASCII letter.
655+ if len(rest) >= 3 && rest[1] == '!' && isASCIILetter(rest[2]) {
656+ return true
657+ }
658+
659+ // Type 1: <script | <pre | <style | <textarea (case-insensitive)
660+ // followed by whitespace, '>', end-of-line, or other non-name
661+ // terminators. Use a permissive HasPrefix check on lowercase.
662+ low := strings.ToLower(rest)
663+ for _, t := range []string{"<script", "<pre", "<style", "<textarea"} {
664+ if strings.HasPrefix(low, t) {
665+ next := byte(0)
666+ if len(low) > len(t) {
667+ next = low[len(t)]
668+ }
669+ if next == 0 || next == ' ' || next == '\t' || next == '>' {
670+ return true
671+ }
672+ }
673+ }
674+
675+ // Types 6 & 7: open or close of a block-level tag.
676+ //
677+ // Type 6 matches a fixed CommonMark tag set; type 7 matches any
678+ // otherwise-valid open/close tag whose name is not in the
679+ // script/pre/style/textarea family. We collapse both into a
680+ // single check: the line must start with '<' or '</' followed
681+ // by an ASCII letter. This deliberately mirrors the other
682+ // hazards — when in doubt, forfeit the boundary. Lines like
683+ // "<3", "<-", "<<", or mid-line "<foo>" do NOT trigger because
684+ // we require the line to *start* (after up to 3 spaces) with
685+ // '<letter' or '</letter'.
686+ j := 1 // past '<'
687+ if j < len(rest) && rest[j] == '/' {
688+ j++
689+ }
690+ if j >= len(rest) || !isASCIILetter(rest[j]) {
691+ return false
692+ }
693+ return true
694+}
695+
696+// isASCIILetter reports whether b is an ASCII letter.
697+func isASCIILetter(b byte) bool {
698+ return (b >= 'a' && b <= 'z') || (b >= 'A' && b <= 'Z')
699+}
700+
701+// isLinkRefDefinition reports whether line matches a CommonMark
702+// link reference definition opener. The conservative pattern:
703+//
704+// ^[ ]{0,3}\[[^\]]+\]:\s*\S+
705+//
706+// i.e. up to 3 spaces, then a bracketed label (no nested ']'),
707+// then a colon, then whitespace, then at least one non-whitespace
708+// character of destination. We do not validate the destination —
709+// presence of a ref-def opener anywhere in the prefix is enough
710+// to forfeit the boundary.
711+func isLinkRefDefinition(line string) bool {
712+ i := 0
713+ for i < len(line) && i < 3 && line[i] == ' ' {
714+ i++
715+ }
716+ if i >= len(line) || line[i] != '[' {
717+ return false
718+ }
719+ i++
720+ labelStart := i
721+ for i < len(line) && line[i] != ']' {
722+ i++
723+ }
724+ if i >= len(line) || i == labelStart {
725+ // No closing bracket, or empty label.
726+ return false
727+ }
728+ // i points at ']'.
729+ i++
730+ if i >= len(line) || line[i] != ':' {
731+ return false
732+ }
733+ i++
734+ // Skip required whitespace.
735+ for i < len(line) && (line[i] == ' ' || line[i] == '\t') {
736+ i++
737+ }
738+ // At least one non-whitespace character of destination.
739+ return i < len(line)
740+}
internal/ui/chat/tools.go+3
@@ -1028,7 +1028,10 @@ func toolOutputMarkdownContent(sty *styles.Styles, content string, width int, ex
1028 }
1028 }
1029
1029
1030 renderer := common.QuietMarkdownRenderer(sty, width)
1030 renderer := common.QuietMarkdownRenderer(sty, width)
1031+ mu := common.LockMarkdownRenderer(renderer)
1032+ mu.Lock()
1031 rendered, err := renderer.Render(content)
1033 rendered, err := renderer.Render(content)
1034+ mu.Unlock()
1032 if err != nil {
1035 if err != nil {
1033 return toolOutputPlainContent(sty, content, width, expanded)
1036 return toolOutputPlainContent(sty, content, width, expanded)
1034 }
1037 }
internal/ui/chat/user.go+3
@@ -57,7 +57,10 @@ func (m *UserMessageItem) RawRender(width int) string {
57 renderer := common.MarkdownRenderer(m.sty, cappedWidth)
57 renderer := common.MarkdownRenderer(m.sty, cappedWidth)
58
58
59 msgContent := strings.TrimSpace(m.message.Content().Text)
59 msgContent := strings.TrimSpace(m.message.Content().Text)
60+ mu := common.LockMarkdownRenderer(renderer)
61+ mu.Lock()
60 result, err := renderer.Render(msgContent)
62 result, err := renderer.Render(msgContent)
63+ mu.Unlock()
61 if err != nil {
64 if err != nil {
62 content = msgContent
65 content = msgContent
63 } else {
66 } else {
internal/ui/common/markdown.go+70−4
@@ -19,6 +19,12 @@ func init() {
19 formatters.Register(formatterName, xchroma.Formatter(zero, nil))
19 formatters.Register(formatterName, xchroma.Formatter(zero, nil))
20}
20}
21
21
22+// mdCacheMu guards mdCache and quietMDCache.
23+//
24+// Lock ordering: when both mdCacheMu and rendererLocksMu are
25+// needed (only in InvalidateMarkdownRendererCache), acquire
26+// mdCacheMu FIRST, then rendererLocksMu. No other call site may
27+// hold rendererLocksMu while acquiring mdCacheMu.
22var (
28var (
23 mdCacheMu sync.Mutex
29 mdCacheMu sync.Mutex
24 mdCache = map[int]*glamour.TermRenderer{}
30 mdCache = map[int]*glamour.TermRenderer{}
@@ -29,6 +35,13 @@ var (
29// the given styles and width. Renderers are memoized per width and shared
35// the given styles and width. Renderers are memoized per width and shared
30// across callers; call InvalidateMarkdownRendererCache when the active
36// across callers; call InvalidateMarkdownRendererCache when the active
31// styles change.
37// styles change.
38+//
39+// The returned renderer is NOT safe for concurrent Render calls
40+// (goldmark's BlockStack carries state across the public Render
41+// API). Crush's TUI is single-threaded so production never
42+// contends, but parallel callers (most notably parallel tests)
43+// must serialize via [LockMarkdownRenderer]. Treat the renderer
44+// as effectively pinned to one goroutine at a time.
32func MarkdownRenderer(sty *styles.Styles, width int) *glamour.TermRenderer {
45func MarkdownRenderer(sty *styles.Styles, width int) *glamour.TermRenderer {
33 mdCacheMu.Lock()
46 mdCacheMu.Lock()
34 defer mdCacheMu.Unlock()
47 defer mdCacheMu.Unlock()
@@ -46,7 +59,8 @@ func MarkdownRenderer(sty *styles.Styles, width int) *glamour.TermRenderer {
46
59
47// QuietMarkdownRenderer returns a glamour [glamour.TermRenderer] with no colors
60// QuietMarkdownRenderer returns a glamour [glamour.TermRenderer] with no colors
48// (plain text with structure) and the given width. Renderers are memoized per
61// (plain text with structure) and the given width. Renderers are memoized per
49−// width and shared across callers.
62+// width and shared across callers. Same concurrency contract as
63+// [MarkdownRenderer]: serialize via [LockMarkdownRenderer].
50func QuietMarkdownRenderer(sty *styles.Styles, width int) *glamour.TermRenderer {
64func QuietMarkdownRenderer(sty *styles.Styles, width int) *glamour.TermRenderer {
51 mdCacheMu.Lock()
65 mdCacheMu.Lock()
52 defer mdCacheMu.Unlock()
66 defer mdCacheMu.Unlock()
@@ -62,12 +76,64 @@ func QuietMarkdownRenderer(sty *styles.Styles, width int) *glamour.TermRenderer
62 return r
76 return r
63}
77}
64
78
65−// InvalidateMarkdownRendererCache drops every cached renderer. Call this
79+// InvalidateMarkdownRendererCache drops every cached renderer
80+// AND every per-renderer mutex in a single atomic critical
81+// section so the two maps cannot disagree mid-toggle. Call this
66−// whenever the active styles change so subsequent renderers pick up the new
82+// whenever the active styles change so subsequent renderers
67−// ansi.StyleConfig.
83+// pick up the new ansi.StyleConfig.
84+//
85+// Existing holders of an old mutex (mid-Render goroutines) keep
86+// their reference safely; new renderers minted after the
87+// invalidation get freshly minted mutexes.
88+//
89+// Lock ordering: mdCacheMu is acquired first, then
90+// rendererLocksMu — see the comments on each mutex.
68func InvalidateMarkdownRendererCache() {
91func InvalidateMarkdownRendererCache() {
69 mdCacheMu.Lock()
92 mdCacheMu.Lock()
70 defer mdCacheMu.Unlock()
93 defer mdCacheMu.Unlock()
94+ rendererLocksMu.Lock()
95+ defer rendererLocksMu.Unlock()
96+
71 mdCache = map[int]*glamour.TermRenderer{}
97 mdCache = map[int]*glamour.TermRenderer{}
72 quietMDCache = map[int]*glamour.TermRenderer{}
98 quietMDCache = map[int]*glamour.TermRenderer{}
99+ rendererLocks = map[*glamour.TermRenderer]*sync.Mutex{}
100+}
101+
102+// rendererLocksMu guards rendererLocks. We key per-renderer
103+// mutexes by pointer so the lock granularity matches the
104+// renderer cache granularity (one mutex per (width, palette)
105+// renderer instance, not one mutex for the entire cache).
106+//
107+// Lock ordering: when both mdCacheMu and rendererLocksMu are
108+// needed (only in InvalidateMarkdownRendererCache), acquire
109+// mdCacheMu FIRST, then rendererLocksMu.
110+var (
111+ rendererLocksMu sync.Mutex
112+ rendererLocks = map[*glamour.TermRenderer]*sync.Mutex{}
113+)
114+
115+// LockMarkdownRenderer returns the per-renderer mutex used to
116+// serialize concurrent Render calls on a shared
117+// [glamour.TermRenderer] instance. The returned [*sync.Mutex] is
118+// stable for the lifetime of the renderer (i.e. until
119+// [InvalidateMarkdownRendererCache] is called).
120+//
121+// Callers that issue more than one Render call in the same
122+// logical operation should hold the mutex for the entire
123+// sequence so other goroutines do not interleave their own
124+// Render calls and corrupt the renderer state. F8's
125+// streamingMarkdown is the immediate consumer; other call
126+// sites that today issue exactly one Render call per item
127+// render are safe without locking under the single-threaded
128+// TUI Update loop, but should adopt this lock if they ever run
129+// in parallel (e.g. background prerender workers).
130+func LockMarkdownRenderer(r *glamour.TermRenderer) *sync.Mutex {
131+ rendererLocksMu.Lock()
132+ defer rendererLocksMu.Unlock()
133+ if mu, ok := rendererLocks[r]; ok {
134+ return mu
135+ }
136+ mu := &sync.Mutex{}
137+ rendererLocks[r] = mu
138+ return mu
73}
139}