Fix TUI serial line duplication after soft-wrap rendering

[aebrer]

Closes #301

Fix TUI serial line duplication after soft-wrap rendering by addressing raw-newline markdown list output during assistant streaming.

Implementation plan posted as a comment below.

[aebrer]

Implementation Plan

Problem Analysis

Issue 301 is now reproducible. The visible duplicate rows are not caused by the soft-wrap row math alone; they are caused by a component-render contract violation that the soft-wrap/live renderer exposed.

The TUI assumes Component.render(width): string[] returns one logical terminal line per array entry. During fast assistant streaming, partial markdown list syntax can make Markdown.render() return a single array entry containing a raw newline. The terminal interprets that embedded newline as an extra physical row, but TUI state (previousLines, row maps, hardwareCursorRow, and diff boundaries) still counts it as one logical row. On the next render, when markdown parsing stabilizes and the list item becomes multiple array entries, the diff renderer writes from stale row state and leaves a duplicate line behind.

The fix should normalize markdown list output so raw CR/LF never appears inside a returned line, and add guardrails/tests so future components cannot silently violate the renderer invariant.

Deliverables

1. Markdown list newline normalization

   • Ensure packages/tui/src/components/markdown.ts never returns list-item lines containing raw \n or \r.
   • Normalize multiline text/paragraph/inline fallback results in renderListItem() before renderList() applies bullets, continuation indentation, and markWrappable().
   • Preserve existing soft-wrap semantics: each resulting list/continuation line remains a clean logical line and is marked wrappable where appropriate.

2. Renderer invariant guardrail

   • Add a loud TUI assertion for raw \n/\r inside rendered line entries before emission/row accounting can drift.
   • The guard should report the offending line index and render context similarly to the existing over-width guard.
   • This is a defensive check; the expected production path is that Markdown no longer trips it.

3. Regression tests for root cause and visible artifact

   • Add a focused Markdown soft-wrap unit test for partial/nested list continuation output.
   • Add an xterm-backed TUI integration regression using the reproduced Proton Bridge-style streaming sample at 170 columns by 45 rows.
   • Add a thin assistant-component smoke test so the real coding-agent caller remains newline-safe.

4. Documentation/contract clarification

   • Update the TUI component contract documentation to state that each render-array entry must be one logical line and must not contain raw CR/LF.

Files to Modify

• packages/tui/src/components/markdown.ts

  • Add or reuse a small helper to split inline-rendered list item text on CR/LF into separate line entries.
  • Apply it in renderListItem() for text, paragraph, and fallback inline-token branches.
  • Review heading/list/block quote paths for the same invariant while keeping the implementation narrowly scoped.

• packages/tui/src/tui.ts

  • Document the no-raw-newline invariant on Component.render().
  • Add a guard in the render validation path so any future raw newline in an emitted line fails loudly instead of corrupting terminal state.

• packages/tui/test/markdown-softwrap.test.ts

  • Add unit tests proving soft-wrapped list items with continuation lines and partial nested-list syntax return no raw \n/\r inside any rendered entry.
  • Verify the resulting plain lines preserve bullet/continuation content.

• packages/tui/test/wrap-soft.test.ts

  • Add the visible regression test using VirtualTerminal/LoggingVirtualTerminal:
    • terminal size 170x45;
    • stream the Proton Bridge-style markdown list sample in small chunks;
    • include enough prior/committed context to match the reproduced failure shape;
    • assert viewport rows match expected rendered output and do not contain adjacent duplicate list rows;
    • assert the fix does not rely on recommitAll() or scrollback clearing.

• packages/coding-agent/test/assistant-message-softwrap.test.ts

  • Add a smoke test that AssistantMessageComponent rendering streamed markdown list content produces no raw newlines in returned lines.

• packages/tui/README.md

  • Clarify the component rendering contract and soft-wrap invariant if the existing documentation does not already state it explicitly.

Testing Approach

1. Focused unit tests

   • node --test --import tsx packages/tui/test/markdown-softwrap.test.ts
   • Cases:
     • partial list continuation such as - first\n continuation\n- next;
     • numbered list item with an indented bullet appearing mid-stream;
     • inline hard break or soft break inside a list item;
     • no rendered line contains \n or \r.

2. TUI integration regression

   • node --test --import tsx packages/tui/test/wrap-soft.test.ts
   • Simulate streaming by repeatedly updating a component and flushing VirtualTerminal.
   • Verify no duplicated adjacent list rows and no use of whole-scrollback clearing as a workaround.

3. Coding-agent caller smoke test

   • npx vitest --run packages/coding-agent/test/assistant-message-softwrap.test.ts
   • Ensure the real assistant message component path remains newline-safe.

4. Full validation after implementation

   • Run targeted tests above.
   • Run npm run build before any manual test with the real dreb binary, per project rules.
   • If touched files trigger formatting/lint changes, run npx biome check --write <files> and rerun affected tests.

Acceptance Criteria

• The reproduced Proton Bridge/list streaming sample no longer duplicates rows in the xterm-backed TUI test.
• No Markdown.render() output entry contains raw \n or \r for the covered list cases.
• AssistantMessageComponent.render() does not return raw-newline-containing entries for streamed markdown list content.
• The TUI fails loudly if any component returns a raw newline inside a rendered line entry.
• Existing soft-wrap guarantees remain intact: copy-clean logical lines, committed scrollback preservation, and no unnecessary full recommit during normal live updates.
• Targeted tests and build pass.

Risks and Open Questions

• List indentation: Splitting multiline list-item content must preserve readable continuation indentation for ordered and nested lists. Tests should assert content preservation without over-specifying exact ANSI styling.
• Nested lists: Current nested-list detection relies on styled bullet patterns; normalization should avoid broad rewrites unless tests prove they are needed.
• Theme highlighters: Highlight callbacks can theoretically return strings containing newlines. The TUI guard will catch this, but the first implementation should avoid expanding scope unless tests reveal an actual issue.
• Guard strictness: The invariant should reject both \n and \r. If an existing component violates it, fix that component rather than weakening the guard.

---

Plan created by mach6

[aebrer]

Progress Update

Implemented the issue 301 plan:

• Normalized Markdown list/heading/paragraph rendered text so raw CR/LF cannot survive inside a single render-array entry during streaming.
• Added a loud TUI guard for raw CR/LF in rendered lines before terminal row state can drift.
• Added regression coverage for:
  • Markdown soft-wrap list continuations and partial nested-list streaming states.
  • Assistant message streamed list output.
  • The reproduced 170x45 Proton-style streaming duplicate-row case.
• Clarified the TUI component render contract in packages/tui/README.md.
• Updated a stale model-switch test fixture from removed Anthropic Haiku 3.5 to OpenAI gpt-4o-mini so the full suite passes after the model registry refresh.

Verification completed:

• node --test --import tsx packages/tui/test/markdown-softwrap.test.ts packages/tui/test/wrap-soft.test.ts
• npx vitest --run packages/coding-agent/test/assistant-message-softwrap.test.ts
• npx vitest --run packages/coding-agent/test/agent-session-model-switch-thinking.test.ts
• npm run build
• npm test
• npm run check

Commit: f64d39b

---

Progress tracked by mach6

[github-actions]

Vitest coverage

Metric	Covered	Total	Coverage
Statements	18136	34377	52.75%
Branches	9851	21309	46.22%
Functions	2852	5392	52.89%
Lines	16026	30581	52.4%

View full coverage run

[aebrer]

Code Review

Critical

None.

Important

None.

Suggestions

Finding 1 — Streamed thinking output is not directly regression-tested
Severity: medium. Confidence: 92.
The new incremental AssistantMessageComponent smoke test only streams text content. Issue acceptance explicitly calls out agent text and thought output during streaming/incremental updates, but current thinking coverage is static wrappability only. A stepwise thinking update test would close the gap and catch raw-newline or row-drift regressions in thought rendering.

Finding 2 — Raw-line-break guard is bypassed for mixed image/text lines
Severity: medium. Confidence: 91.
packages/tui/src/tui.ts skips the raw CR/LF guard when isImageLine(line) is true. isImageLine() is broad and returns true if an image escape appears anywhere in the string, so a mixed text/image entry containing a raw newline can still desynchronize terminal row tracking. The raw CR/LF invariant should be enforced before the image exemption, or image escape payloads should be parsed/stripped before validation.

Finding 3 — Streaming duplicate assertion is viewport-local
Severity: low. Confidence: 84.
The new streaming regression checks only adjacent duplicate rows in the visible viewport after each chunk. That covers the reproduced symptom, but could miss non-adjacent drift or scrollback corruption with a unique-looking viewport. A stronger assertion would compare the rendered viewport/logical transcript against expected output and/or assert committed scrollback contents after the stream.

Finding 4 — Simplify CR/CRLF normalization
Severity: low. Confidence: 99.
splitRenderedTextLines() currently normalizes \r\n and bare \r with two replacements. text.replace(/\r\n?/g, "\n") expresses the same behavior in one pass.

Finding 5 — Extract duplicated crash-log/stop/throw plumbing
Severity: medium. Confidence: 90.
The new raw-CR/LF guard and existing over-width guard both build a crash log, write it, stop the TUI, and throw. A shared helper for this render-contract failure path would make assertLineFits() shorter while preserving the two distinct messages and line formatting.

Finding 6 — Remove pass-through streaming test wrapper
Severity: low. Confidence: 97.
StreamingMarkdownComponent in packages/tui/test/wrap-soft.test.ts only forwards setText, render, and invalidate to an inner Markdown. The test can use new Markdown("", 0, 0, defaultMarkdownTheme, undefined, true) directly.

Strengths

• The implementation addresses the reproduced root cause narrowly by preventing raw CR/LF from surviving inside Markdown render entries.
• The TUI guard fails loudly before row accounting can drift and writes useful crash context.
• Regression coverage spans Markdown unit behavior, the assistant-message caller, and the reproduced 170x45 Proton-style streaming sample.
• Drew’s real terminal validation passed, and the known bad sample reproduced the bug before the fix but not after.

Agents run: code-reviewer, error-auditor, test-reviewer, completeness-checker, simplifier

---

Reviewed by mach6

[aebrer]

Review Assessment

Review comment: #303 (comment)

Classifications

Finding	Classification	Reasoning
Streamed thinking output is not directly regression-tested	genuine	assistant-message-softwrap.test.ts has static thinking wrappability coverage, but the stepwise streaming test only updates text content. Thinking uses a separate Markdown rendering path, so the issue acceptance item for streamed thought output is not directly covered.
Raw-line-break guard is bypassed for mixed image/text lines	genuine	assertLineFits() skips raw CR/LF validation when isImageLine(line) is true, and isImageLine() matches image escapes anywhere in a string. A mixed image/text entry with a raw newline could still bypass the new invariant and desynchronize row accounting.
Streaming duplicate assertion is viewport-local	nitpick	The Proton-style test checks raw CR/LF and adjacent duplicate rows in the visible viewport after each chunk, covering the reproduced symptom. Full transcript/scrollback comparison would be stronger, but this is an incremental test-quality improvement rather than a demonstrated gap.
Simplify CR/CRLF normalization	nitpick	text.replace(/\r\n/g, "\n").replace(/\r/g, "\n") can be written as text.replace(/\r\n?/g, "\n"), but behavior is equivalent and stylistic.
Extract duplicated crash-log/stop/throw plumbing	nitpick	The raw CR/LF and over-width guard branches duplicate crash-log/stop/throw plumbing, but both are clear and correct. This is a maintainability refactor, not a merge blocker.
Remove pass-through streaming test wrapper	nitpick	StreamingMarkdownComponent only forwards to Markdown; direct Markdown use would be shorter, but the wrapper is harmless test scaffolding.

Action Plan

1. Fix the raw CR/LF guard so image-line detection does not exempt rendered entries from newline validation; only the width guard should keep the image exemption.
2. Add a stepwise streamed thinking regression test through AssistantMessageComponent, analogous to the current streamed text markdown-list test.

---

Assessment by mach6

[aebrer]

Progress Update

Fixed the two genuine review findings from the latest assessment:

• Fixed finding 1 by adding stepwise streamed thinking coverage in packages/coding-agent/test/assistant-message-softwrap.test.ts, matching the existing streamed text markdown-list smoke test.
• Fixed finding 2 by moving the raw CR/LF render-contract guard in packages/tui/src/tui.ts ahead of image-line detection, so mixed image/text entries cannot bypass newline validation.
• Added a regression test in packages/tui/test/wrap-soft.test.ts proving a rendered line containing Kitty image escapes plus a raw newline still trips the raw-line-break guard.

Verification completed:

• node --test --import tsx packages/tui/test/wrap-soft.test.ts
• npx vitest --run packages/coding-agent/test/assistant-message-softwrap.test.ts
• npx biome check --write packages/tui/src/tui.ts packages/tui/test/wrap-soft.test.ts packages/coding-agent/test/assistant-message-softwrap.test.ts
• npm run build
• npm test
• npm run check

Commit: 66b1a53

---

Progress tracked by mach6