Skip to content

Commit 318b225

Browse files
bloveclaude
andcommitted
fix(chat): stop streaming tables rendering as raw pipes mid-stream
ChatStreamingMdComponent called parser.finish() on every render where its [streaming] input was false. finish() is destructive: an incomplete table (a header row before its delimiter row) reverts to a CommonMark paragraph — raw "| a | b |" text. The [streaming] flag (agent.isLoading() && i===last) is not a reliable "still arriving" signal: it reads false at cold start and flaps false mid-stream, so the parser kept being finalized while tokens were still arriving and the whole table rendered as raw pipes until the message completed. (Prose hid this because finalized prose looks identical to streaming prose; tables are the first place finished != streaming is visible. partial-markdown 0.5.2 renders streaming tables correctly when the parser is not finished early.) Fix: never finalize while content is still growing — the live parser.root projection already renders everything (incl. streaming tables) since 0.5.x. Finalize via a debounce: only after streaming is false AND no new content has arrived for 600ms; any token or streaming=true flap re-arms it, so finish() fires exactly once after the stream truly stops. No markdown view depends on node status, so the delay has no visual cost. Also guard against pushing into an already-finished parser. Tests: new streaming-markdown.table-stream.spec (reproduces the bug + a fake-timer flap test); updated the one variants assertion that depended on synchronous finalize. Live-verified in examples/chat: a streamed table renders a <table> every frame, never raw pipes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent b5962e7 commit 318b225

3 files changed

Lines changed: 177 additions & 29 deletions

File tree

libs/chat/src/lib/streaming/streaming-markdown.component.ts

Lines changed: 57 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
effect,
99
inject,
1010
input,
11+
signal,
1112
} from '@angular/core';
1213
import {
1314
createPartialMarkdownParser,
@@ -22,6 +23,16 @@ import { MarkdownChildrenComponent } from '../markdown/markdown-children.compone
2223
import { cacheplaneMarkdownViews } from '../markdown/cacheplane-markdown-views';
2324
import { CitationsResolverService } from '../markdown/citations-resolver.service';
2425

26+
// How long streaming must be false AND content stable before we finalize the
27+
// parser. finish() is only needed to mark final node status (not used visually)
28+
// and to revert a genuinely-truncated trailing construct to CommonMark — the
29+
// live `parser.root` projection already renders everything during streaming, so
30+
// this delay has NO visual cost. It must comfortably exceed real inter-chunk
31+
// gaps (e.g. the pause between a table's header row and its delimiter row) and
32+
// any `streaming` flag flap, so finalize never fires mid-stream and reverts an
33+
// in-progress table to raw "| a | b |" text.
34+
const FINALIZE_DEBOUNCE_MS = 600;
35+
2536
/**
2637
* Renders streaming markdown by walking a @cacheplane/partial-markdown AST
2738
* through @threadplane/render's view registry.
@@ -77,6 +88,42 @@ export class ChatStreamingMdComponent {
7788
this.resolver.markdownDefs.set(r.citations ?? new Map());
7889
}
7990
});
91+
92+
// Debounced finalization. `finish()` is terminal and DESTRUCTIVE: it reverts
93+
// an incomplete trailing construct to its CommonMark fallback — e.g. a table
94+
// header before its delimiter row becomes raw "| a | b |" paragraph text. We
95+
// must therefore never finalize while tokens are still arriving. The
96+
// `streaming` input is not a reliable "still arriving" signal — it can flap
97+
// false mid-stream, and at cold start it can read false for an entire live
98+
// stream — so we finalize only once streaming is false AND no new content
99+
// has arrived for a short, imperceptible window. Any new content or a
100+
// streaming=true flap re-arms the timer, so finalize fires exactly once,
101+
// after the stream truly stops. Until then the live `parser.root` projection
102+
// (0.5.x) renders the in-progress content, including streaming tables.
103+
let timer: ReturnType<typeof setTimeout> | null = null;
104+
effect((onCleanup) => {
105+
const isStreaming = this.streaming();
106+
this.content(); // re-arm whenever new content arrives
107+
if (timer) {
108+
clearTimeout(timer);
109+
timer = null;
110+
}
111+
if (isStreaming || this.finished) return;
112+
timer = setTimeout(() => {
113+
timer = null;
114+
if (this.streaming() || this.finished) return;
115+
if (!this.prior.endsWith('\n')) this.parser.push('\n');
116+
this.parser.finish();
117+
this.finished = true;
118+
this.finalizeTick.update((v) => v + 1);
119+
}, FINALIZE_DEBOUNCE_MS);
120+
onCleanup(() => {
121+
if (timer) {
122+
clearTimeout(timer);
123+
timer = null;
124+
}
125+
});
126+
});
80127
}
81128

82129
// Parser instance is rebuilt only when content diverges from the prior
@@ -85,36 +132,27 @@ export class ChatStreamingMdComponent {
85132
private parser: PartialMarkdownParser = createPartialMarkdownParser();
86133
private prior = '';
87134
private finished = false;
135+
// Bumped by the debounced finalizer so the `root` computed re-materializes
136+
// the now-finished parser tree.
137+
private readonly finalizeTick = signal(0);
88138

89139
readonly root = computed<MarkdownDocumentNode | null>(() => {
90140
const c = this.content();
91-
const isStreaming = this.streaming();
141+
this.finalizeTick(); // re-materialize after a debounced finalize
92142
if (c !== this.prior) {
93-
if (c.startsWith(this.prior)) {
143+
// Re-parse from scratch when the content diverged from the prior prefix,
144+
// OR when the parser was already finalized — finish() is terminal, so
145+
// pushing further deltas into a finished parser corrupts its state. A
146+
// transient `streaming=false` mid-stream that finalized early thus
147+
// recovers here: new content rebuilds an open, projecting parser.
148+
if (c.startsWith(this.prior) && !this.finished) {
94149
this.parser.push(c.slice(this.prior.length));
95150
} else {
96-
// Content shrank or diverged — reset.
97151
this.parser = createPartialMarkdownParser();
98152
this.finished = false;
99153
if (c.length > 0) this.parser.push(c);
100154
}
101-
if (!isStreaming && !this.finished) {
102-
// @cacheplane/partial-markdown@0.3 does not flush trailing text on
103-
// finish() unless the buffer ends with a newline. Plain LLM
104-
// responses often omit the trailing newline, which causes the
105-
// parser to emit a document with zero children — i.e. the message
106-
// renders empty. Push a sentinel newline first to force the open
107-
// paragraph closed before we finalize.
108-
if (!c.endsWith('\n')) this.parser.push('\n');
109-
this.parser.finish();
110-
this.finished = true;
111-
}
112155
this.prior = c;
113-
} else if (!isStreaming && !this.finished) {
114-
// Streaming flipped to false without new content; ensure parser is finalized.
115-
if (!this.prior.endsWith('\n')) this.parser.push('\n');
116-
this.parser.finish();
117-
this.finished = true;
118156
}
119157
// Materialize for Angular reactivity: produces a NEW root reference when
120158
// any descendant subtree changed; same reference when nothing changed
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
// libs/chat/src/lib/streaming/streaming-markdown.table-stream.spec.ts
2+
// SPDX-License-Identifier: MIT
3+
//
4+
// Regression: a streaming table must render as a <table> as it arrives, not as
5+
// raw "| a | b |" paragraph text. The bug was that ChatStreamingMdComponent
6+
// called parser.finish() on every render where [streaming] was false — and
7+
// finish() reverts an incomplete table (header with no delimiter row yet) to a
8+
// CommonMark paragraph (raw pipes). Because the [streaming] flag is unreliable
9+
// (observed false for an entire live stream at cold start), the whole table
10+
// rendered as raw pipes until the message completed. The fix: do not finalize
11+
// the parser while content is still growing; finalize only once it settles.
12+
import { describe, it, expect, beforeEach, vi } from 'vitest';
13+
import { TestBed } from '@angular/core/testing';
14+
import { Component, signal } from '@angular/core';
15+
import { ChatStreamingMdComponent } from './streaming-markdown.component';
16+
17+
@Component({
18+
standalone: true,
19+
imports: [ChatStreamingMdComponent],
20+
template: `<chat-streaming-md [content]="content()" [streaming]="streaming()" />`,
21+
})
22+
class HostComponent {
23+
content = signal<string>('');
24+
streaming = signal<boolean>(true);
25+
}
26+
27+
describe('ChatStreamingMdComponent — streaming table rendering', () => {
28+
let fixture: ReturnType<typeof TestBed.createComponent<HostComponent>>;
29+
let host: HostComponent;
30+
let el: HTMLElement;
31+
beforeEach(() => {
32+
TestBed.configureTestingModule({ imports: [HostComponent] });
33+
fixture = TestBed.createComponent(HostComponent);
34+
host = fixture.componentInstance;
35+
el = fixture.nativeElement as HTMLElement;
36+
});
37+
const grow = (c: string) => { host.content.set(c); fixture.detectChanges(); };
38+
39+
it('renders a <table> as a table streams in even when [streaming] lags false', () => {
40+
// The cold-start race: content is actively growing but streaming is false.
41+
host.streaming.set(false);
42+
grow('Here is a table:\n\n| Name ');
43+
grow('Here is a table:\n\n| Name | Age |'); // header on the open line, no delimiter
44+
// Before the fix: finish() reverted this to raw-pipe paragraphs.
45+
expect(el.querySelector('table'), 'header should render as a table, not raw pipes').toBeTruthy();
46+
const paras = [...el.querySelectorAll('p')].map((p) => p.textContent || '');
47+
expect(paras.some((t) => t.includes('| Name | Age |')), 'no raw-pipe paragraph').toBe(false);
48+
});
49+
50+
it('renders a <table> while streaming (flag true), through the delimiter wait', () => {
51+
grow('| Name | Age |');
52+
expect(el.querySelector('table')).toBeTruthy();
53+
grow('| Name | Age |\n'); // header committed, awaiting delimiter
54+
expect(el.querySelector('table')).toBeTruthy();
55+
});
56+
57+
it('finalizes the table once the stream settles (streaming -> false)', () => {
58+
host.streaming.set(true);
59+
grow('| Name | Age |\n| --- | --- |\n| Ada | 36 |\n');
60+
expect(el.querySelector('table')).toBeTruthy();
61+
host.streaming.set(false); // settle
62+
fixture.detectChanges();
63+
const table = el.querySelector('table');
64+
expect(table).toBeTruthy();
65+
expect(el.querySelectorAll('thead th').length).toBe(2);
66+
expect(el.querySelectorAll('tbody tr').length).toBe(1);
67+
});
68+
69+
it('renders a complete one-shot (non-streaming) table message', () => {
70+
host.streaming.set(false);
71+
grow('| Name | Age |\n| --- | --- |\n| Ada | 36 |\n');
72+
expect(el.querySelector('table')).toBeTruthy();
73+
expect(el.querySelectorAll('thead th').length).toBe(2);
74+
});
75+
76+
it('does not flash raw pipes when [streaming] flaps false mid-stream', () => {
77+
vi.useFakeTimers();
78+
try {
79+
host.streaming.set(true);
80+
grow('| Name | Age |'); // streaming header → table
81+
expect(el.querySelector('table')).toBeTruthy();
82+
// Flap: streaming reads false for a moment with no new content.
83+
host.streaming.set(false);
84+
fixture.detectChanges();
85+
vi.advanceTimersByTime(60); // less than the debounce — must NOT finalize
86+
expect(el.querySelector('table'), 'table must survive the flap').toBeTruthy();
87+
expect(
88+
[...el.querySelectorAll('p')].some((p) => (p.textContent || '').includes('|')),
89+
'no raw-pipe paragraph during the flap',
90+
).toBe(false);
91+
// Flap recovers: streaming true again + more content arrives.
92+
host.streaming.set(true);
93+
grow('| Name | Age |\n| --- | --- |\n');
94+
vi.advanceTimersByTime(300);
95+
expect(el.querySelector('table')).toBeTruthy();
96+
} finally {
97+
vi.useRealTimers();
98+
}
99+
});
100+
});

libs/chat/src/lib/streaming/streaming-markdown.variants.spec.ts

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// SPDX-License-Identifier: MIT
2-
import { describe, it, expect } from 'vitest';
2+
import { describe, it, expect, vi } from 'vitest';
33
import { TestBed } from '@angular/core/testing';
44
import { Component, signal } from '@angular/core';
55
import { ChatStreamingMdComponent } from './streaming-markdown.component';
@@ -76,14 +76,24 @@ const midStreamRows: MidStreamRow[] = [
7676

7777
describe('ChatStreamingMdComponent — mid-stream input variance', () => {
7878
it.each(midStreamRows)('$name', (row) => {
79-
TestBed.configureTestingModule({ imports: [HostComponent] });
80-
const fixture = TestBed.createComponent(HostComponent);
81-
fixture.componentInstance.content.set(row.midStream);
82-
fixture.componentInstance.streaming.set(true);
83-
fixture.detectChanges();
84-
fixture.componentInstance.content.set(row.onFinish ?? row.midStream);
85-
fixture.componentInstance.streaming.set(false);
86-
fixture.detectChanges();
87-
expect(normalize(fixture.nativeElement.textContent ?? '')).toBe(row.expectedText);
79+
// Finalization is debounced (the component must not finalize on a transient
80+
// streaming=false), so an unclosed construct only reverts to its literal
81+
// CommonMark form once the debounce elapses. Drive fake timers to settle.
82+
vi.useFakeTimers();
83+
try {
84+
TestBed.configureTestingModule({ imports: [HostComponent] });
85+
const fixture = TestBed.createComponent(HostComponent);
86+
fixture.componentInstance.content.set(row.midStream);
87+
fixture.componentInstance.streaming.set(true);
88+
fixture.detectChanges();
89+
fixture.componentInstance.content.set(row.onFinish ?? row.midStream);
90+
fixture.componentInstance.streaming.set(false);
91+
fixture.detectChanges();
92+
vi.advanceTimersByTime(800); // elapse the finalize debounce
93+
fixture.detectChanges();
94+
expect(normalize(fixture.nativeElement.textContent ?? '')).toBe(row.expectedText);
95+
} finally {
96+
vi.useRealTimers();
97+
}
8898
});
8999
});

0 commit comments

Comments
 (0)