Skip to content

Commit 3516127

Browse files
committed
refactor: Fix markdown streaming race conditions
- Add asyncio locks to CopyableMarkdown update methods - Serialize fragment appends to prevent stale state - Handle fenced code blocks across async boundaries - Update streaming logic to ignore stale snapshots - Add tests for fenced block serialization
1 parent 4cb3f1b commit 3516127

5 files changed

Lines changed: 118 additions & 17 deletions

File tree

‎src/mother/runtime_coordinator.py‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ def _show_error(self, response: Response, error_text: str) -> None:
249249
self.callbacks.update_response_output, response, error_text
250250
)
251251
_ = self.callbacks.call_from_thread(response.stop_stream)
252-
response.reset_state(error_text)
252+
_ = self.callbacks.call_from_thread(response.reset_state, error_text)
253253

254254
async def run_runtime_request(
255255
self,
@@ -342,7 +342,7 @@ async def run_runtime_request(
342342

343343
_ = self.callbacks.call_from_thread(response.stop_stream)
344344
session.conversation_state.message_history = list(runtime_response.all_messages)
345-
response.reset_state(stream_state.visible_text)
345+
_ = self.callbacks.call_from_thread(response.reset_state, stream_state.visible_text)
346346
if runtime_response.tool_limit_recovery_used:
347347
session.record_session_event(
348348
"tool_limit_recovery",
@@ -511,7 +511,7 @@ def on_progress(message: str) -> None:
511511
stream_state.visible_text,
512512
)
513513
_ = self.callbacks.call_from_thread(response.stop_stream)
514-
response.reset_state(stream_state.visible_text)
514+
_ = self.callbacks.call_from_thread(response.reset_state, stream_state.visible_text)
515515
session.record_session_message("assistant", stream_state.visible_text)
516516
return stream_state.visible_text
517517

@@ -589,7 +589,7 @@ def on_progress(update: CouncilProgressUpdate) -> None:
589589
)
590590
_ = self.callbacks.call_from_thread(self.callbacks.show_council_trace, result)
591591
_ = self.callbacks.call_from_thread(response.stop_stream)
592-
response.reset_state(final_text)
592+
_ = self.callbacks.call_from_thread(response.reset_state, final_text)
593593
session.conversation_state.append_synthetic_turn(user_question, final_text)
594594
session.record_session_message("assistant", final_text)
595595
session.record_session_event("council_completed", result.to_event_details())

‎src/mother/runtime_presentation.py‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -271,13 +271,7 @@ async def update_response_output(self, response: Response, text: str) -> None:
271271
had_waiting_animation = self.has_waiting_animation(response)
272272
self.clear_response_waiting_animation(response)
273273
should_follow = self.host.should_follow_chat_updates()
274-
current_text = response.raw_markdown
275-
if had_waiting_animation:
276-
await response.replace_markdown(text)
277-
elif text.startswith(current_text):
278-
await response.append_fragment(text[len(current_text) :])
279-
else:
280-
await response.replace_markdown(text)
274+
await response.update_streamed_markdown(text, force_replace=had_waiting_animation)
281275
if should_follow:
282276
self.host.scroll_chat_to_end(force=True)
283277

‎src/mother/widgets.py‎

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
"""Reusable TUI widget classes for Mother."""
22

3+
import asyncio
34
import re
45
from dataclasses import dataclass
56
from itertools import pairwise
@@ -835,6 +836,7 @@ def __init__(self, markdown: str = "") -> None:
835836
self._cursor: int = 0
836837
self._raw: str = markdown
837838
self._stream: MarkdownStream | None = None
839+
self._update_lock: asyncio.Lock = asyncio.Lock()
838840

839841
@property
840842
def raw_markdown(self) -> str:
@@ -850,31 +852,61 @@ def stream(self) -> MarkdownStream:
850852

851853
async def append_fragment(self, fragment: str) -> None:
852854
"""Append a streamed markdown fragment without reparsing the full response."""
855+
async with self._update_lock:
856+
await self._append_fragment_locked(fragment)
857+
858+
async def _append_fragment_locked(self, fragment: str) -> None:
859+
"""Append a fragment while the markdown update lock is held."""
853860
if not fragment:
854861
return
855862
previous_raw = self._raw
856863
self._raw += fragment
857864
if (
858-
_ends_inside_fenced_block(previous_raw)
865+
_contains_fenced_block_marker(previous_raw)
866+
or _ends_inside_fenced_block(previous_raw)
859867
or _ends_inside_fenced_block(self._raw)
860868
or _contains_fenced_block_marker(fragment)
861869
):
862-
# Textual's incremental Markdown.append can lose fenced code contents
863-
# or produce stale block layout when streamed fragments contain fences.
864-
# Reparse the full response whenever a fence is active or appears.
865-
await self.stop_stream()
870+
# Textual's incremental Markdown.append can lose fenced code contents,
871+
# append following paragraphs into the preceding fence, or produce stale
872+
# block layout when streamed fragments contain fences. Once a response
873+
# has seen a fence, reparse the full response for all later fragments.
874+
await self._stop_stream_locked()
866875
await self.update(self._raw)
867876
return
868877
await self.stream.write(fragment)
869878

870879
async def replace_markdown(self, markdown: str) -> None:
871880
"""Replace the rendered markdown, stopping any active incremental stream first."""
872-
await self.stop_stream()
881+
async with self._update_lock:
882+
await self._replace_markdown_locked(markdown)
883+
884+
async def _replace_markdown_locked(self, markdown: str) -> None:
885+
"""Replace markdown while the markdown update lock is held."""
886+
await self._stop_stream_locked()
873887
self._raw = markdown
874888
await self.update(markdown)
875889

890+
async def update_streamed_markdown(self, markdown: str, *, force_replace: bool = False) -> None:
891+
"""Render a streamed full-text snapshot in order, ignoring stale prefix snapshots."""
892+
async with self._update_lock:
893+
current_text = self._raw
894+
if not force_replace and current_text.startswith(markdown) and markdown != current_text:
895+
return
896+
if force_replace:
897+
await self._replace_markdown_locked(markdown)
898+
elif markdown.startswith(current_text):
899+
await self._append_fragment_locked(markdown[len(current_text) :])
900+
else:
901+
await self._replace_markdown_locked(markdown)
902+
876903
async def stop_stream(self) -> None:
877904
"""Stop the active incremental markdown stream, flushing any pending fragments."""
905+
async with self._update_lock:
906+
await self._stop_stream_locked()
907+
908+
async def _stop_stream_locked(self) -> None:
909+
"""Stop the active incremental markdown stream while the update lock is held."""
878910
if self._stream is None:
879911
return
880912
stream = self._stream

‎tests/test_markdown_streaming.py‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from unittest.mock import AsyncMock
66

77
from textual.app import App, ComposeResult
8+
from textual.await_complete import AwaitComplete
89
from textual.widgets.markdown import MarkdownFence
910

1011
from mother.widgets import Response
@@ -25,6 +26,53 @@ def compose(self) -> ComposeResult:
2526
yield Response("")
2627

2728

29+
@final
30+
class _RecordingResponse(Response):
31+
"""Response test double that records rendered markdown with configurable delay."""
32+
33+
def __init__(self) -> None:
34+
super().__init__("")
35+
self.rendered_markdown: list[str] = []
36+
37+
@override
38+
def update(self, markdown: str) -> AwaitComplete:
39+
async def record() -> None:
40+
if markdown == "```bash\n":
41+
await asyncio.sleep(0.02)
42+
self.rendered_markdown.append(markdown)
43+
44+
return AwaitComplete(record())
45+
46+
47+
def test_response_streaming_serializes_fenced_code_reparses() -> None:
48+
async def run() -> None:
49+
response = _RecordingResponse()
50+
51+
first_update = asyncio.create_task(response.append_fragment("```bash\n"))
52+
await asyncio.sleep(0)
53+
final_update = asyncio.create_task(response.append_fragment("echo ok\n```\n"))
54+
_ = await asyncio.gather(first_update, final_update)
55+
56+
assert response.raw_markdown == "```bash\necho ok\n```\n"
57+
assert response.rendered_markdown[-1] == response.raw_markdown
58+
59+
asyncio.run(run())
60+
61+
62+
def test_response_streaming_ignores_stale_prefix_snapshots() -> None:
63+
async def run() -> None:
64+
response = _RecordingResponse()
65+
final_markdown = "```bash\necho ok\n```\n"
66+
67+
await response.update_streamed_markdown(final_markdown)
68+
await response.update_streamed_markdown("```bash\n")
69+
70+
assert response.raw_markdown == final_markdown
71+
assert response.rendered_markdown[-1] == final_markdown
72+
73+
asyncio.run(run())
74+
75+
2876
def test_response_streaming_reparses_fragments_that_contain_complete_fences() -> None:
2977
async def run() -> None:
3078
app = _MarkdownStreamingApp()
@@ -47,6 +95,25 @@ async def run() -> None:
4795
asyncio.run(run())
4896

4997

98+
def test_response_streaming_keeps_text_after_closed_fence_out_of_code_block() -> None:
99+
async def run() -> None:
100+
app = _MarkdownStreamingApp()
101+
102+
async with app.run_test() as pilot:
103+
response = app.query_one(Response)
104+
105+
await response.append_fragment("```bash\n")
106+
await response.append_fragment("echo ok\n")
107+
await response.append_fragment("```\n")
108+
await response.append_fragment("\nAfter\n")
109+
await pilot.pause()
110+
111+
assert response.query_one(MarkdownFence).code == "echo ok"
112+
assert response.raw_markdown.endswith("After\n")
113+
114+
asyncio.run(run())
115+
116+
50117
def test_response_streaming_preserves_fenced_code_blocks_across_fragments() -> None:
51118
async def run() -> None:
52119
app = _MarkdownStreamingApp()

‎tests/test_mother_send_prompt.py‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,14 @@ async def replace_markdown(self, markdown: str) -> None:
6767
self.raw_markdown = markdown
6868
self.updated_texts.append(markdown)
6969

70+
async def update_streamed_markdown(self, markdown: str, *, force_replace: bool = False) -> None:
71+
if force_replace:
72+
await self.replace_markdown(markdown)
73+
elif markdown.startswith(self.raw_markdown):
74+
await self.append_fragment(markdown[len(self.raw_markdown) :])
75+
else:
76+
await self.replace_markdown(markdown)
77+
7078
async def stop_stream(self) -> None:
7179
self.stop_stream_calls += 1
7280

0 commit comments

Comments
 (0)