From adf939c1723af505e40a550b16daf5889ecc5078 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 22 May 2026 03:47:49 +0000 Subject: [PATCH 01/15] fix(streaming-markdown): list-marker awareness + table chunk-boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three production Slack streaming bugs from issue #69 comment: 1. `_remend("* item one\n")` appended a stray `*`. `_close_emphasis` counted the line-leading bullet as an italic opener. Now skips single-`*` runs preceded by line-leading whitespace and followed by a space/tab (list-marker shape), matching upstream `remend`'s awareness of list markers. 2. `StreamingMarkdownRenderer.finish()` on an odd-count bullet list produced `...item three\n*` — same root cause as #1; the fix covers both. 3. `_get_committable_prefix` released a confirmed table the moment a separator arrived, even with zero body rows. Slack `chat.appendStream` saw header+separator alone as broken syntax and then bare body rows in subsequent appends, never reconnecting them as a single table. Now the header+separator block is held until at least one body row arrives, then released atomically. The original tests pinned the buggy "commit on separator" behavior; updated to require a body row before committing the table block, and added a TestIssue69Regressions class with the exact reproducers from the issue comment plus invariants for the new contract. Closes part of #69 (the streaming bugs). The broader hand-rolled parser question (Options A/B/C in the issue) remains open. --- src/chat_sdk/shared/streaming_markdown.py | 51 ++++++- tests/test_streaming_markdown.py | 155 ++++++++++++++++++++-- 2 files changed, 193 insertions(+), 13 deletions(-) diff --git a/src/chat_sdk/shared/streaming_markdown.py b/src/chat_sdk/shared/streaming_markdown.py index ede4b0ec..4c315d10 100644 --- a/src/chat_sdk/shared/streaming_markdown.py +++ b/src/chat_sdk/shared/streaming_markdown.py @@ -54,6 +54,30 @@ def _strip_fenced_code(text: str) -> str: return "\n".join(result_lines) +def _is_list_marker(stripped: str, run_start: int, run_len: int, ch: str) -> bool: + """Whether a run of *ch* at *run_start* is a list marker, not emphasis. + + Only ``*`` is ambiguous -- it's both an inline emphasis marker and a + bullet-list marker. A list marker is a single character preceded only + by horizontal whitespace (or start of text) since the previous newline, + and followed by a horizontal whitespace character. + """ + if ch != "*" or run_len != 1: + return False + j = run_start - 1 + while j >= 0: + c = stripped[j] + if c == "\n": + break + if c not in (" ", "\t"): + return False + j -= 1 + after = run_start + run_len + if after >= len(stripped): + return False + return stripped[after] in (" ", "\t") + + def _close_emphasis(result: str, stripped: str, ch: str) -> str: """Close unclosed bold/italic emphasis for a single marker character. @@ -66,6 +90,10 @@ def _close_emphasis(result: str, stripped: str, ch: str) -> str: level: a run of 2+ characters opens/closes bold first, then any remaining single character opens/closes italic. + Line-leading ``*`` runs followed by whitespace are treated as list + markers and excluded from the emphasis tally -- mirroring upstream + ``remend``'s list-marker awareness (issue #69). + To guarantee idempotency the suffix is separated from any trailing marker run by a zero-width space so it cannot merge with existing characters and create new openers on re-scan. @@ -78,10 +106,13 @@ def _close_emphasis(result: str, stripped: str, ch: str) -> str: i += 2 continue if stripped[i] == ch: + run_start = i run_len = 0 while i < len(stripped) and stripped[i] == ch: run_len += 1 i += 1 + if _is_list_marker(stripped, run_start, run_len, ch): + continue runs.append(run_len) else: i += 1 @@ -231,6 +262,7 @@ def _get_committable_prefix(text: str) -> str: # Walk backward to find consecutive table-like lines at the end held_count = 0 separator_found = False + separator_idx = -1 for i in range(len(lines) - 1, -1, -1): trimmed = lines[i].strip() @@ -241,6 +273,7 @@ def _get_committable_prefix(text: str) -> str: if TABLE_SEPARATOR_RE.match(trimmed): separator_found = True + separator_idx = i break if TABLE_ROW_RE.match(trimmed): @@ -248,10 +281,24 @@ def _get_committable_prefix(text: str) -> str: else: break - if separator_found or held_count == 0: + if separator_found and held_count == 0: + # Separator present but no body row beneath it yet. Append-only + # consumers (Slack `chat.appendStream`, etc.) parse each delta + # independently; emitting a header+separator without rows produces + # broken syntax. Hold the entire pre-separator table block until a + # row arrives (issue #69). + table_start = separator_idx + for i in range(separator_idx - 1, -1, -1): + trimmed = lines[i].strip() + if trimmed == "" or TABLE_ROW_RE.match(trimmed) is None: + break + table_start = i + commit_line_count = table_start + elif separator_found or held_count == 0: return text + else: + commit_line_count = len(lines) - held_count - commit_line_count = len(lines) - held_count committed_lines = lines[:commit_line_count] result = "\n".join(committed_lines) diff --git a/tests/test_streaming_markdown.py b/tests/test_streaming_markdown.py index 571d832a..cbcc3d32 100644 --- a/tests/test_streaming_markdown.py +++ b/tests/test_streaming_markdown.py @@ -122,15 +122,25 @@ def test_should_hold_back_trailing_table_header_lines(self): assert "| A | B |" not in result assert "Text" in result - def test_should_confirm_table_when_separator_arrives(self): + def test_should_confirm_table_when_separator_and_body_row_arrive(self): r = StreamingMarkdownRenderer() r.push("Text\n\n| A | B |\n") assert "| A | B |" not in r.render() + # Separator alone with no body row is still held back: append-only + # consumers (e.g. Slack chat.appendStream) parse each delta + # independently and a header+separator emission with zero rows is + # broken syntax. See issue #69. r.push("|---|---|\n") + assert "| A | B |" not in r.render() + assert "|---|---|" not in r.render() + + # First body row releases the entire confirmed table atomically. + r.push("| 1 | 2 |\n") result = r.render() assert "| A | B |" in result assert "|---|---|" in result + assert "| 1 | 2 |" in result def test_should_release_held_lines_when_next_line_is_not_a_table_row(self): r = StreamingMarkdownRenderer() @@ -240,20 +250,34 @@ def test_should_confirm_table_with_alignment_markers_in_separator(self): r.push("| Left | Center | Right |\n") assert "| Left |" not in r.render() + # Separator alone is held; first body row releases the table. r.push("|:---|:---:|---:|\n") + assert "| Left |" not in r.render() + + r.push("| 1 | 2 | 3 |\n") result = r.render() assert "| Left | Center | Right |" in result assert "|:---|:---:|---:|" in result + assert "| 1 | 2 | 3 |" in result def test_should_not_hold_data_rows_after_confirmed_separator(self): r = StreamingMarkdownRenderer() + # Header+separator alone is held until a body row arrives (issue #69). r.push("| A | B |\n|---|---|\n") - assert "|---|---|" in r.render() + assert "|---|---|" not in r.render() + # First body row releases the entire table block atomically. r.push("| 1 | 2 |\n") result = r.render() + assert "| A | B |" in result + assert "|---|---|" in result assert "| 1 | 2 |" in result + # Subsequent rows commit immediately. + r.push("| 3 | 4 |\n") + result = r.render() + assert "| 3 | 4 |" in result + def test_should_handle_multiple_push_calls_before_single_render(self): r = StreamingMarkdownRenderer() r.push("| A ") @@ -273,7 +297,12 @@ def test_should_handle_table_header_split_across_chunks(self): r.push(" | B |\n") assert "| A | B |" not in r.render() + # Separator alone keeps the table held (issue #69). r.push("|---|---|\n") + assert "| A | B |" not in r.render() + + # First body row releases the assembled table. + r.push("| 1 | 2 |\n") assert "| A | B |" in r.render() @@ -450,22 +479,23 @@ def test_getcommittabletext_delta_should_stream_table_in_code_fence(self): delta = committable[len(last_appended) :] assert delta == "" - # Push separator -- table confirmed + # Push separator -- still held; header+separator without a body row + # would be broken markup for append-only consumers (issue #69). r.push("|---|---|\n") committable = r.get_committable_text() delta = committable[len(last_appended) :] - assert "```" in delta - assert "| A | B |" in delta - assert "|---|---|" in delta - last_appended = committable + assert delta == "" - # Push data row + # First data row releases header+separator+row atomically. r.push("| 1 | 2 |\n") committable = r.get_committable_text() delta = committable[len(last_appended) :] + assert "```" in delta + assert "| A | B |" in delta + assert "|---|---|" in delta assert "| 1 | 2 |" in delta # Should NOT have a closing ``` - assert "```" not in delta + assert "```\n```" not in delta last_appended = committable # Blank line ends table -- closes code fence @@ -740,13 +770,17 @@ def test_should_render_realworld_table_with_singledash_separators_progressively( assert "| ID |" not in result assert "Here's a table" in result + # Separator alone is held -- need a body row to confirm (issue #69). r.push("| - | - | - | - | - | - | - | - |\n") result = r.render() - assert "| ID |" in result - assert "| - |" in result + assert "| ID |" not in result + assert "| - |" not in result + # First body row releases header+separator+row atomically. r.push("| 1 | Sarah Johnson | Engineering | 32 | $95,000 | Seattle | 2019-03-15 | Active |\n") result = r.render() + assert "| ID |" in result + assert "| - |" in result assert "Sarah Johnson" in result r.push("| 2 | Michael") @@ -910,3 +944,102 @@ def test_getcommittabletext_is_always_clean_remend_would_not_add_markers(self): f'("{self.COMPLEX_MARKDOWN[: i + 1][-20:]}") has unclosed markers: ' f'"{committable[-40:]}"' ) + + +# ============================================================================ +# Issue #69 regressions: list-marker awareness + table chunk-boundary +# ============================================================================ + + +class TestIssue69Regressions: + """Pin the three production bugs from issue #69 (comment 4514752058). + + The hand-rolled ``_remend`` previously confused line-leading bullet + markers with italic openers, and ``_get_committable_prefix`` emitted + table header+separator without a body row -- both produced visible + corruption in Slack streaming. + """ + + def test_remend_does_not_treat_line_leading_star_as_italic(self): + # Single bullet item -- a literal `* item one\n` is unchanged. + assert _remend("* item one\n") == "* item one\n" + + def test_remend_does_not_close_italic_on_multi_bullet_list(self): + # Three bullets, odd count: previously appended a stray `*`. + text = "* item one\n* item two\n* item three\n" + assert _remend(text) == text + + def test_finish_on_odd_count_bullet_list_does_not_corrupt(self): + r = StreamingMarkdownRenderer(wrap_tables_for_append=False) + r.push("* item one\n* item two\n* item three\n") + assert r.finish() == "* item one\n* item two\n* item three\n" + + def test_remend_still_closes_genuine_italic(self): + # *hello (no following whitespace) is genuine emphasis, not a list. + assert _remend("*hello") == "*hello*" + + def test_remend_still_closes_genuine_bold(self): + assert _remend("**bold") == "**bold**" + + def test_remend_handles_bold_inside_list_item(self): + # Bullet marker is skipped; bold inside the item is balanced. + text = "* **important** item\n" + assert _remend(text) == text + + def test_remend_closes_unclosed_bold_inside_list_item(self): + # Bullet skipped, then an unclosed `**` should still get closed. + assert _remend("* **important") == "* **important**" + + def test_remend_handles_indented_bullet(self): + # Up to leading whitespace before the bullet -- still a list marker. + assert _remend(" * nested item\n") == " * nested item\n" + + def test_table_header_plus_separator_alone_is_held(self): + # Header+separator without a body row would be emitted as broken + # markup to append-only consumers. Hold the whole block. + r = StreamingMarkdownRenderer(wrap_tables_for_append=False) + r.push("| ID | Status |\n|---|---|\n") + assert r.get_committable_text() == "" + + def test_table_chunk_boundary_emits_atomic_header_separator_row(self): + # Reproduces the exact chunk sequence from issue #69 comment. + r = StreamingMarkdownRenderer(wrap_tables_for_append=False) + chunks = [ + "Header:\n\n", + "| ID", + " | Status |\n", + "|---|---|\n", + "| 1 | Open |\n", + "| 2 | Closed |\n", + ] + last = "" + deltas: list[str] = [] + for c in chunks: + r.push(c) + cur = r.get_committable_text() + deltas.append(cur[len(last) :]) + last = cur + + assert deltas[0] == "Header:\n\n" + # Header line, separator line, and split-header chunk all held. + assert deltas[1] == "" + assert deltas[2] == "" + assert deltas[3] == "" + # First body row releases the assembled table atomically. + assert deltas[4] == "| ID | Status |\n|---|---|\n| 1 | Open |\n" + # Subsequent rows commit immediately. + assert deltas[5] == "| 2 | Closed |\n" + + def test_table_with_preceding_text_holds_only_table_block(self): + # Prose above an unconfirmed table commits; only the table holds. + r = StreamingMarkdownRenderer(wrap_tables_for_append=False) + r.push("Intro paragraph.\n\n| H |\n|---|\n") + assert r.get_committable_text() == "Intro paragraph.\n\n" + + def test_table_held_block_flushes_on_finish_even_without_body_row(self): + # finish() is the terminal flush -- even an incomplete table is emitted. + r = StreamingMarkdownRenderer(wrap_tables_for_append=False) + r.push("| H |\n|---|\n") + final = r.finish() + assert "| H |" in final + assert "|---|" in final From 7eda0400d2c2c4c657e15840c76a77fe9f342984 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 22 May 2026 04:07:47 +0000 Subject: [PATCH 02/15] refactor(_remend): match remend's flanking check for excluded asterisks The previous `_is_list_marker` skipped only line-leading single `*` followed by horizontal whitespace. Tighten to match upstream remend's `shouldSkipAsterisk` (packages/remend/src/emphasis-handlers.ts): exclude any single `*` flanked by whitespace (or text boundary) on both sides, which is what CommonMark says isn't a valid emphasis delimiter anyway. The line-leading bullet case still falls out naturally. Picks up three additional cases the previous narrower check missed: - `text * more` -- whitespace-flanked mid-line - `trailing *\n` -- asterisk at end of line - `partial *` -- asterisk at end of buffer Same `_remend` over-counting failure mode as the original issue #69 bugs, just different surface forms. Renamed the helper to `_is_excluded_asterisk` to reflect the broader scope. Remaining remend divergences (word-internal asterisks, math-block contents, escaped sequences, multi-backtick spans, setext headings, indented code, raw HTML, footnotes) are tracked on issue #69 for the parser-strategy discussion -- out of scope for this PR. --- src/chat_sdk/shared/streaming_markdown.py | 46 +++++++++++------------ tests/test_streaming_markdown.py | 13 +++++++ 2 files changed, 36 insertions(+), 23 deletions(-) diff --git a/src/chat_sdk/shared/streaming_markdown.py b/src/chat_sdk/shared/streaming_markdown.py index 4c315d10..57cf8a57 100644 --- a/src/chat_sdk/shared/streaming_markdown.py +++ b/src/chat_sdk/shared/streaming_markdown.py @@ -54,28 +54,27 @@ def _strip_fenced_code(text: str) -> str: return "\n".join(result_lines) -def _is_list_marker(stripped: str, run_start: int, run_len: int, ch: str) -> bool: - """Whether a run of *ch* at *run_start* is a list marker, not emphasis. - - Only ``*`` is ambiguous -- it's both an inline emphasis marker and a - bullet-list marker. A list marker is a single character preceded only - by horizontal whitespace (or start of text) since the previous newline, - and followed by a horizontal whitespace character. +def _is_excluded_asterisk(stripped: str, run_start: int, run_len: int, ch: str) -> bool: + """Whether a single-``*`` run at *run_start* should be excluded from the + emphasis tally per CommonMark's flanking rules. + + A ``*`` flanked by whitespace (space, tab, newline, or text boundary) + on both sides is not a valid emphasis delimiter, and this same check + covers line-leading bullet list markers (``* item``). Mirrors + ``shouldSkipAsterisk`` in upstream remend's ``emphasis-handlers.ts``. + + Only single-``*`` runs are subject to this check -- ``**`` / ``***`` + runs are evaluated by the bold pairing logic and have their own + word-boundary semantics. ``_``-emphasis isn't ambiguous with list + markers, so it's not affected here either. """ if ch != "*" or run_len != 1: return False - j = run_start - 1 - while j >= 0: - c = stripped[j] - if c == "\n": - break - if c not in (" ", "\t"): - return False - j -= 1 - after = run_start + run_len - if after >= len(stripped): - return False - return stripped[after] in (" ", "\t") + prev_char = stripped[run_start - 1] if run_start > 0 else None + next_char = stripped[run_start + run_len] if run_start + run_len < len(stripped) else None + prev_ws = prev_char is None or prev_char in (" ", "\t", "\n") + next_ws = next_char is None or next_char in (" ", "\t", "\n") + return prev_ws and next_ws def _close_emphasis(result: str, stripped: str, ch: str) -> str: @@ -90,9 +89,10 @@ def _close_emphasis(result: str, stripped: str, ch: str) -> str: level: a run of 2+ characters opens/closes bold first, then any remaining single character opens/closes italic. - Line-leading ``*`` runs followed by whitespace are treated as list - markers and excluded from the emphasis tally -- mirroring upstream - ``remend``'s list-marker awareness (issue #69). + Whitespace-flanked single ``*`` runs are excluded via + :func:`_is_excluded_asterisk` -- this covers line-leading bullets + (``* item``) and any other non-delimiter asterisks per CommonMark's + flanking rules (issue #69). To guarantee idempotency the suffix is separated from any trailing marker run by a zero-width space so it cannot merge with existing @@ -111,7 +111,7 @@ def _close_emphasis(result: str, stripped: str, ch: str) -> str: while i < len(stripped) and stripped[i] == ch: run_len += 1 i += 1 - if _is_list_marker(stripped, run_start, run_len, ch): + if _is_excluded_asterisk(stripped, run_start, run_len, ch): continue runs.append(run_len) else: diff --git a/tests/test_streaming_markdown.py b/tests/test_streaming_markdown.py index cbcc3d32..30afc478 100644 --- a/tests/test_streaming_markdown.py +++ b/tests/test_streaming_markdown.py @@ -994,6 +994,19 @@ def test_remend_handles_indented_bullet(self): # Up to leading whitespace before the bullet -- still a list marker. assert _remend(" * nested item\n") == " * nested item\n" + def test_remend_skips_whitespace_flanked_asterisk_mid_line(self): + # CommonMark: `*` flanked by whitespace on both sides isn't a valid + # delimiter. Previously counted as an italic opener. + assert _remend("use the * operator") == "use the * operator" + + def test_remend_skips_trailing_asterisk_at_end_of_line(self): + # `*\n` -- next char is whitespace, prev is whitespace -- not a delimiter. + assert _remend("trailing star *\nmore text") == "trailing star *\nmore text" + + def test_remend_skips_bare_asterisk_at_end_of_buffer(self): + # `*` at end of stream with whitespace before -- not a delimiter yet. + assert _remend("partial *") == "partial *" + def test_table_header_plus_separator_alone_is_held(self): # Header+separator without a body row would be emitted as broken # markup to append-only consumers. Hold the whole block. From b7d7f838c59c59826bf8f6ccfbccc19ae1314da8 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 22 May 2026 05:29:24 +0000 Subject: [PATCH 03/15] fix(markdown): chat-scoped completeness (escaped chars, task lists, math/escape in remend) Pulls in the chat-realistic subset of the issue #69 gap list, scoped to what LLMs actually emit in agent/chat contexts. Builds on #99's _is_excluded_asterisk helper rather than restating it. Parser side (markdown_parser.py): - Inline regexes get `(? Content: - """Create a list item node.""" - return {"type": "listItem", "children": children} +def make_list_item(children: list[Content], *, checked: bool | None = None) -> Content: + """Create a list item node. + + *checked* follows the mdast GFM task-list extension: ``True`` for + ``- [x]``, ``False`` for ``- [ ]``, ``None`` for a regular list item. + """ + node: Content = {"type": "listItem", "children": children} + if checked is not None: + node["checked"] = checked + return node def make_thematic_break() -> Content: @@ -162,43 +169,75 @@ def get_node_value(node: Content) -> str: # Inline parser # --------------------------------------------------------------------------- +# ASCII-punctuation characters that may be backslash-escaped per +# CommonMark. A preceding `\` makes the next character literal -- not a +# delimiter -- which the lookbehinds in the patterns below honour. +_ESCAPABLE_PUNCT = set("!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~") + # Regex patterns for inline elements, ordered by priority. -# Each pattern captures a full inline construct. +# Each pattern captures a full inline construct. Every opening delimiter +# uses a ``(? str: + """Resolve backslash-escapes of ASCII punctuation to their literal char. + + Applied at text-leaf emission only -- inline delimiters were already + consumed by the patterns above with escape-aware lookbehinds, so by + the time we reach a text leaf, any remaining ``\\X`` pair where X is + ASCII punct is unambiguously a literal X. + """ + if "\\" not in text: + return text + out: list[str] = [] + i = 0 + while i < len(text): + if text[i] == "\\" and i + 1 < len(text) and text[i + 1] in _ESCAPABLE_PUNCT: + out.append(text[i + 1]) + i += 2 + continue + out.append(text[i]) + i += 1 + return "".join(out) + + def _parse_inline_plain(text: str) -> list[Content]: """Parse plain text that contains no inline formatting. - Handles hard line breaks (two trailing spaces + newline). + Handles hard line breaks (two trailing spaces + newline) and resolves + backslash escapes of ASCII punctuation to their literal character. """ if " \n" in text: parts: list[Content] = [] segments = text.split(" \n") for i, seg in enumerate(segments): if seg: - parts.append(make_text(seg)) + parts.append(make_text(_unescape_punct(seg))) if i < len(segments) - 1: parts.append(make_break()) - return parts if parts else [make_text(text)] - return [make_text(text)] + return parts if parts else [make_text(_unescape_punct(text))] + return [make_text(_unescape_punct(text))] def _parse_inline(text: str) -> list[Content]: @@ -242,7 +281,7 @@ def _parse_inline(text: str) -> list[Content]: # The matched construct (content recursion is bounded by match length) if best_kind == "image": - alt = best_match.group(1) + alt = _unescape_punct(best_match.group(1)) url = best_match.group(2) title = best_match.group(3) nodes.append(make_image(url, alt, title)) @@ -319,6 +358,9 @@ def _collect_list_items(lines: list[str], start: int, ordered: bool) -> tuple[li items: list[Content] = [] i = start item_re = re.compile(r"^(\d+)[.)]\s+(.*)") if ordered else re.compile(r"^[-*+]\s+(.*)") + # GFM task list extension: `- [ ]` or `- [x]` at the start of an + # unordered item maps to `listItem.checked = False` / `True`. + task_re = re.compile(r"^\[([ xX])\]\s+(.*)") while i < len(lines): line = lines[i] @@ -326,6 +368,13 @@ def _collect_list_items(lines: list[str], start: int, ordered: bool) -> tuple[li if m: item_text = m.group(2) if ordered else m.group(1) + checked: bool | None = None + if not ordered: + task_m = task_re.match(item_text) + if task_m: + checked = task_m.group(1) in ("x", "X") + item_text = task_m.group(2) + item_children_lines = [item_text] i += 1 @@ -350,7 +399,7 @@ def _collect_list_items(lines: list[str], start: int, ordered: bool) -> tuple[li # Parse nested content in the item nested_children = _parse_list_item_content(item_children_lines) - items.append(make_list_item(nested_children)) + items.append(make_list_item(nested_children, checked=checked)) else: break diff --git a/src/chat_sdk/shared/streaming_markdown.py b/src/chat_sdk/shared/streaming_markdown.py index 57cf8a57..ee7efa66 100644 --- a/src/chat_sdk/shared/streaming_markdown.py +++ b/src/chat_sdk/shared/streaming_markdown.py @@ -54,6 +54,54 @@ def _strip_fenced_code(text: str) -> str: return "\n".join(result_lines) +# ASCII-punctuation characters that can be backslash-escaped per CommonMark. +# When `\X` precedes one of these, the resulting char is literal, not a +# delimiter -- so the inline-marker counters in `_remend` should ignore it. +_ESCAPABLE_PUNCT = frozenset("!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~") + + +def _strip_escape_sequences(text: str) -> str: + """Remove ``\\X`` pairs from *text* so escape-aware counters work. + + For each ``\\X`` where X is an ASCII punctuation char, drop both + characters from the result. The cleaned text is only used for + *counting* delimiters (``*``, ``~~``, `` ` ``, ``[``, ``]``); + positions in the cleaned text don't map back to the original, so + truncating is safe. + """ + if "\\" not in text: + return text + out: list[str] = [] + i = 0 + while i < len(text): + if text[i] == "\\" and i + 1 < len(text) and text[i + 1] in _ESCAPABLE_PUNCT: + i += 2 + continue + out.append(text[i]) + i += 1 + return "".join(out) + + +def _strip_math_regions(text: str) -> str: + """Replace ``$$...$$`` and ``$...$`` math spans with empty strings. + + LLMs frequently emit TeX-style math in technical chat content. Inline + markers inside math (e.g. ``$a^* + b^*$``) are literal, not delimiters, + so they shouldn't count toward the unclosed-emphasis tally. Math is + line-scoped here -- a ``$`` that doesn't close on the same line is + left alone so it doesn't gobble unrelated prose. The double-dollar + pass runs first so ``$$`` blocks don't get mis-paired by the single + pass. + """ + if "$" not in text: + return text + # Display math: $$...$$ may span multiple lines. + text = re.sub(r"\$\$.*?\$\$", "", text, flags=re.DOTALL) + # Inline math: $...$ within a single line, non-greedy, no embedded $. + text = re.sub(r"\$[^\$\n]+\$", "", text) + return text + + def _is_excluded_asterisk(stripped: str, run_start: int, run_len: int, ch: str) -> bool: """Whether a single-``*`` run at *run_start* should be excluded from the emphasis tally per CommonMark's flanking rules. @@ -63,6 +111,12 @@ def _is_excluded_asterisk(stripped: str, run_start: int, run_len: int, ch: str) covers line-leading bullet list markers (``* item``). Mirrors ``shouldSkipAsterisk`` in upstream remend's ``emphasis-handlers.ts``. + Note: a simpler word-internal exclusion (e.g. ``5*3=15``) was + considered but breaks paired emphasis like ``text*foo*`` whose + asterisks are also word-flanked. Distinguishing the two needs a + proper CommonMark delimiter-stack algorithm, tracked under issue #69 + Option A as the next chat-completeness item. + Only single-``*`` runs are subject to this check -- ``**`` / ``***`` runs are evaluated by the bold pairing logic and have their own word-boundary semantics. ``_``-emphasis isn't ambiguous with list @@ -180,6 +234,12 @@ def _remend(text: str) -> str: # Strip fenced code blocks so their contents don't affect inline counts. outside_fences = _strip_fenced_code(result) + # Drop math regions ($...$ and $$...$$) -- markers inside math are + # literal, not delimiters (issue #69 follow-up). + outside_fences = _strip_math_regions(outside_fences) + # Drop ASCII-punct escape sequences (`\*`, `\~`, `\[`, etc.) so the + # tilde / backtick / bracket counters below see the post-escape text. + outside_fences = _strip_escape_sequences(outside_fences) # --- inline code backticks --- # Count total backtick characters outside code fences. If odd, one code diff --git a/tests/test_markdown_faithful.py b/tests/test_markdown_faithful.py index f5ad5c93..2054725c 100644 --- a/tests/test_markdown_faithful.py +++ b/tests/test_markdown_faithful.py @@ -189,6 +189,121 @@ def test_handles_multiple_paragraphs(self): assert ast["children"][1]["type"] == "paragraph" +# ============================================================================ +# Chat / AI agent completeness (issue #69): escaped chars + task lists +# ============================================================================ + + +class TestEscapedCharacters: + """Backslash-escape support for chat content. + + LLMs routinely emit ``\\*`` / ``\\[`` / ``\\\\`` when explaining + syntax, displaying shell commands, or showing literal markdown. + Previously these slipped past the inline regexes and the visible + output dropped the backslashes but mis-parsed the surrounding + delimiters as emphasis / links. + """ + + def _para_children(self, md): + ast = parse_markdown(md) + assert ast["children"][0]["type"] == "paragraph" + return ast["children"][0]["children"] + + def test_escaped_asterisk_is_literal_not_italic(self): + children = self._para_children(r"\*not italic\*") + assert len(children) == 1 + assert children[0] == {"type": "text", "value": "*not italic*"} + + def test_escaped_underscore_is_literal_not_italic(self): + children = self._para_children(r"\_not italic\_") + assert len(children) == 1 + assert children[0] == {"type": "text", "value": "_not italic_"} + + def test_escaped_brackets_do_not_form_a_link(self): + children = self._para_children(r"\[not a link\](https://x.com)") + assert all(c.get("type") != "link" for c in children) + assert any("[not a link]" in c.get("value", "") for c in children if c.get("type") == "text") + + def test_escaped_tilde_does_not_form_strikethrough(self): + children = self._para_children(r"\~~not strike\~~") + assert all(c.get("type") != "delete" for c in children) + + def test_escaped_backslash_yields_single_backslash(self): + children = self._para_children(r"path with \\backslash") + assert children == [{"type": "text", "value": "path with \\backslash"}] + + def test_mixed_escaped_and_real_emphasis(self): + children = self._para_children(r"\*literal\* and *real italic*") + types = [c.get("type") for c in children] + assert "emphasis" in types + text_values = [c.get("value", "") for c in children if c.get("type") == "text"] + assert any("*literal*" in v for v in text_values) + + def test_escaped_alt_in_image(self): + children = self._para_children(r"![my \[image\]](pic.png)") + assert children[0]["type"] == "image" + assert children[0]["alt"] == "my [image]" + + def test_inline_code_contents_are_not_unescaped(self): + # Per CommonMark, backslash inside `code` is literal. + children = self._para_children(r"a `\*literal\*` b") + code_nodes = [c for c in children if c.get("type") == "inlineCode"] + assert len(code_nodes) == 1 + assert code_nodes[0]["value"] == r"\*literal\*" + + +class TestTaskListItems: + """GFM task list checkbox extraction. + + Maps ``- [ ] item`` / ``- [x] item`` to mdast ``listItem`` with a + ``checked`` attribute. Plain list items stay as before (no attr). + """ + + def _list(self, md): + ast = parse_markdown(md) + lists = [c for c in ast["children"] if c.get("type") == "list"] + assert len(lists) == 1 + return lists[0] + + def test_unchecked_task_item_sets_checked_false(self): + lst = self._list("- [ ] todo") + assert lst["children"][0]["checked"] is False + + def test_checked_task_item_sets_checked_true(self): + lst = self._list("- [x] done") + assert lst["children"][0]["checked"] is True + + def test_uppercase_x_also_counts_as_checked(self): + lst = self._list("- [X] done") + assert lst["children"][0]["checked"] is True + + def test_plain_item_has_no_checked_attr(self): + lst = self._list("- regular item") + assert "checked" not in lst["children"][0] + + def test_mixed_list_preserves_per_item_state(self): + lst = self._list("- [x] done\n- [ ] pending\n- regular") + items = lst["children"] + assert items[0]["checked"] is True + assert items[1]["checked"] is False + assert "checked" not in items[2] + + def test_task_marker_strips_from_content(self): + lst = self._list("- [x] do the thing") + para = lst["children"][0]["children"][0] + # Content should be "do the thing", not "[x] do the thing". + text_concat = "".join(c.get("value", "") for c in para["children"]) + assert text_concat == "do the thing" + + def test_ordered_list_with_bracket_content_is_not_a_task(self): + # `1. [x] foo` should NOT be treated as a task -- task lists are + # a bullet-only convention in GFM. + ast = parse_markdown("1. [x] foo") + lst = ast["children"][0] + assert lst["type"] == "list" and lst["ordered"] is True + assert "checked" not in lst["children"][0] + + # ============================================================================ # stringifyMarkdown Tests # ============================================================================ diff --git a/tests/test_streaming_markdown.py b/tests/test_streaming_markdown.py index 30afc478..d237cac1 100644 --- a/tests/test_streaming_markdown.py +++ b/tests/test_streaming_markdown.py @@ -1056,3 +1056,61 @@ def test_table_held_block_flushes_on_finish_even_without_body_row(self): final = r.finish() assert "| H |" in final assert "|---|" in final + + +# ============================================================================ +# Chat / AI agent completeness (issue #69 follow-up) +# ============================================================================ + + +class TestRemendChatCompleteness: + """``_remend`` parity with upstream remend on chat-content patterns. + + Each gap below comes from the [issue #69 follow-up + catalog](https://github.com/Chinchill-AI/chat-sdk-python/issues/69#issuecomment-4514898801). + Word-internal asterisks (``5*3=15``) are tracked separately under + Option A -- they need a proper CommonMark delimiter-stack algorithm + to avoid breaking paired-emphasis cases like ``text*foo*``. + """ + + # --- Math regions ($...$, $$...$$) ----------------------------------- + + def test_remend_skips_markers_inside_inline_math(self): + assert _remend("$a^* + b^*$") == "$a^* + b^*$" + + def test_remend_skips_markers_inside_display_math(self): + text = "$$\\int_0^* e^{-x} dx$$" + assert _remend(text) == text + + def test_remend_still_closes_emphasis_outside_math(self): + # Math region is stripped for counting; the trailing italic still + # gets closed normally. + assert _remend("$a^2$ and *italic") == "$a^2$ and *italic*" + + def test_remend_skips_strike_marker_inside_math(self): + assert _remend("$a ~~ b$") == "$a ~~ b$" + + def test_remend_does_not_open_bracket_inside_math(self): + # Math regions are dropped from the bracket walk too -- a literal + # `[` inside math shouldn't add a phantom `]` closer. + assert _remend("note $f[x]$ here") == "note $f[x]$ here" + + # --- Escape-aware tilde / backtick / bracket counters ---------------- + + def test_remend_does_not_add_strike_for_escaped_tilde_pair(self): + assert _remend(r"foo \~~bar") == r"foo \~~bar" + + def test_remend_does_not_add_bracket_for_escaped_open(self): + assert _remend(r"see \[item") == r"see \[item" + + def test_remend_does_not_add_backtick_for_escaped(self): + assert _remend(r"foo \` bar") == r"foo \` bar" + + def test_remend_handles_mixed_escaped_and_real_unclosed(self): + # `\~~` is literal; the trailing real `~~` is unclosed -> close it. + assert _remend(r"a \~~b and ~~real") == r"a \~~b and ~~real~~" + + def test_remend_does_not_affect_escape_outside_relevant_counters(self): + # An escaped delimiter that wasn't going to imbalance anything + # is still left untouched. + assert _remend(r"foo \* bar") == r"foo \* bar" From ef085c3cba16f0d923fe5987354447d821c4b616 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 22 May 2026 08:41:59 +0000 Subject: [PATCH 04/15] fix(markdown): address PR #101 review findings Independent review surfaced three actionable items: 1. **Math/escape ordering** (review #1) -- `_strip_math_regions` ran BEFORE `_strip_escape_sequences`, so input like `\$opener *unclosed text closer\$` had its two escaped `$`s paired as math, eating the `*` opener inside. Reordered: escape strip first, then math strip. Verified the original bug repro now correctly produces a closing `*`. 2. **Empty task items** (review #5) -- `r"^\[([ xX])\]\s+(.*)"` required at least one whitespace AFTER `]`, so `- [ ]` with no trailing content silently fell through to plain text. Loosened to `r"^\[([ xX])\](?:\s+(.*))?$"` -- trailing whitespace+content is now optional. `- [ ]` and `- [x]` (no trailing) both produce `listItem(checked=False/True, children=[])`. 3. **Strikethrough test strengthened** (review #7) -- the existing `test_escaped_tilde_does_not_form_strikethrough` only asserted the absence of a `delete` node. A buggy impl that dropped the tildes entirely would have passed. Added a content-shape assertion that `~~not strike~~` appears in the text leaves. Reviewer's #3 (the `r"a *b\* * c"` case) was investigated and determined NOT to be a bug -- the trailing `*` between two spaces is not a valid CommonMark closer (whitespace-flanked), so italic correctly stays open. Pre-fix behavior happened to balance by ignoring flanking; post-fix is CommonMark-correct. Pre-existing edge cases #4 (`\\*text*`) and #6 (link-text unescape implicit) are documented in the existing code; not blockers. Tests added: - `test_empty_unchecked_task_item_no_content` and `_checked_` variant - `test_remend_escaped_dollar_does_not_pair_with_unescaped_dollar` (the exact #1 repro) - `test_remend_escaped_dollar_does_not_create_phantom_math_region` - Strengthened `test_escaped_tilde_does_not_form_strikethrough` 3,625 pass / 1 pre-existing failure unrelated. --- src/chat_sdk/shared/markdown_parser.py | 10 +++++++--- src/chat_sdk/shared/streaming_markdown.py | 10 ++++++---- tests/test_markdown_faithful.py | 16 ++++++++++++++++ tests/test_streaming_markdown.py | 15 +++++++++++++++ 4 files changed, 44 insertions(+), 7 deletions(-) diff --git a/src/chat_sdk/shared/markdown_parser.py b/src/chat_sdk/shared/markdown_parser.py index d2297e11..a55993a5 100644 --- a/src/chat_sdk/shared/markdown_parser.py +++ b/src/chat_sdk/shared/markdown_parser.py @@ -359,8 +359,10 @@ def _collect_list_items(lines: list[str], start: int, ordered: bool) -> tuple[li i = start item_re = re.compile(r"^(\d+)[.)]\s+(.*)") if ordered else re.compile(r"^[-*+]\s+(.*)") # GFM task list extension: `- [ ]` or `- [x]` at the start of an - # unordered item maps to `listItem.checked = False` / `True`. - task_re = re.compile(r"^\[([ xX])\]\s+(.*)") + # unordered item maps to `listItem.checked = False` / `True`. The + # trailing whitespace + content group is optional so an empty task + # item (``- [ ]`` with nothing after) still produces ``checked``. + task_re = re.compile(r"^\[([ xX])\](?:\s+(.*))?$") while i < len(lines): line = lines[i] @@ -373,7 +375,9 @@ def _collect_list_items(lines: list[str], start: int, ordered: bool) -> tuple[li task_m = task_re.match(item_text) if task_m: checked = task_m.group(1) in ("x", "X") - item_text = task_m.group(2) + # group(2) is None for an empty task item (`- [ ]`) + # with no trailing whitespace/content. + item_text = task_m.group(2) or "" item_children_lines = [item_text] i += 1 diff --git a/src/chat_sdk/shared/streaming_markdown.py b/src/chat_sdk/shared/streaming_markdown.py index ee7efa66..899fdd1b 100644 --- a/src/chat_sdk/shared/streaming_markdown.py +++ b/src/chat_sdk/shared/streaming_markdown.py @@ -234,12 +234,14 @@ def _remend(text: str) -> str: # Strip fenced code blocks so their contents don't affect inline counts. outside_fences = _strip_fenced_code(result) + # Drop ASCII-punct escape sequences BEFORE math-region stripping -- + # otherwise an escaped ``\$`` could pair with a later unescaped ``$`` + # and the inline-math regex would eat content between them, including + # any emphasis openers (PR #101 review finding). + outside_fences = _strip_escape_sequences(outside_fences) # Drop math regions ($...$ and $$...$$) -- markers inside math are - # literal, not delimiters (issue #69 follow-up). + # literal, not delimiters. outside_fences = _strip_math_regions(outside_fences) - # Drop ASCII-punct escape sequences (`\*`, `\~`, `\[`, etc.) so the - # tilde / backtick / bracket counters below see the post-escape text. - outside_fences = _strip_escape_sequences(outside_fences) # --- inline code backticks --- # Count total backtick characters outside code fences. If odd, one code diff --git a/tests/test_markdown_faithful.py b/tests/test_markdown_faithful.py index 2054725c..63919912 100644 --- a/tests/test_markdown_faithful.py +++ b/tests/test_markdown_faithful.py @@ -227,6 +227,10 @@ def test_escaped_brackets_do_not_form_a_link(self): def test_escaped_tilde_does_not_form_strikethrough(self): children = self._para_children(r"\~~not strike\~~") assert all(c.get("type") != "delete" for c in children) + # Also pin the actual text content -- a buggy impl that dropped + # the tildes entirely would pass the absence-of-`delete` check. + text_concat = "".join(c.get("value", "") for c in children if c.get("type") == "text") + assert "~~not strike~~" in text_concat def test_escaped_backslash_yields_single_backslash(self): children = self._para_children(r"path with \\backslash") @@ -303,6 +307,18 @@ def test_ordered_list_with_bracket_content_is_not_a_task(self): assert lst["type"] == "list" and lst["ordered"] is True assert "checked" not in lst["children"][0] + def test_empty_unchecked_task_item_no_content(self): + # `- [ ]` with no trailing whitespace or content should still + # produce a checked listItem (PR #101 review finding). + lst = self._list("- [ ]") + assert lst["children"][0]["checked"] is False + assert lst["children"][0]["children"] == [] + + def test_empty_checked_task_item_no_content(self): + lst = self._list("- [x]") + assert lst["children"][0]["checked"] is True + assert lst["children"][0]["children"] == [] + # ============================================================================ # stringifyMarkdown Tests diff --git a/tests/test_streaming_markdown.py b/tests/test_streaming_markdown.py index d237cac1..4fad5ba6 100644 --- a/tests/test_streaming_markdown.py +++ b/tests/test_streaming_markdown.py @@ -1114,3 +1114,18 @@ def test_remend_does_not_affect_escape_outside_relevant_counters(self): # An escaped delimiter that wasn't going to imbalance anything # is still left untouched. assert _remend(r"foo \* bar") == r"foo \* bar" + + # --- Escape-before-math ordering (PR #101 review #1) --------------------- + + def test_remend_escaped_dollar_does_not_pair_with_unescaped_dollar(self): + # Without the escape-strip-before-math-strip ordering, the math + # regex would pair these two `$`s and eat the `*` opener inside, + # leaving italic unclosed. + text = r"\$opener *unclosed text closer\$" + assert _remend(text) == text + "*" + + def test_remend_escaped_dollar_does_not_create_phantom_math_region(self): + # `\$5` is a literal dollar amount; the `$10` later is not part of + # any math region (one un-escaped `$` doesn't form `$...$`). + # The italic at the end still gets closed normally. + assert _remend(r"\$5 and $10 *italic") == r"\$5 and $10 *italic*" From 33b2eb5f49e8df88399d519c82876996479cc8c4 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 22 May 2026 09:42:52 +0000 Subject: [PATCH 05/15] fix(markdown): stringify round-trip for escapes + task list checked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Downstream-impact subagent found two blockers in PR #101's new AST semantics: `stringify_markdown` and `_render_list` were not updated to emit the new state, causing data loss when adapters round-trip through the AST. Blocker 1: `listItem.checked` silently dropped. parse_markdown("- [x] done") → stringify_markdown → "* done\n" Affects every adapter that goes markdown → AST → stringify (linear/github/telegram/whatsapp + the generic BaseFormatConverter.to_markdown). On GitHub and Linear especially (native task-list rendering), this is observable data loss. Blocker 2: `_unescape_punct` + naive stringify produces re-parse drift. parse_markdown(r"Use \*literal\*") → text "Use *literal*" → stringify → "Use *literal*\n" → reparse → emphasis(literal) Telegram MarkdownV2 / WhatsApp / Slack image alt / Google Chat all render the un-escaped `*literal*` as bold/italic in the user's view. Fixes in `markdown_parser.py`: - New `_escape_text_leaf` re-escapes the seven delimiter chars (`* _ ~ \` [ ] \\`) on text-leaf emission. Inverse of `_unescape_punct`. Conservative: doesn't over-escape ordinary punct like `.` / `,` / `!`. - `_stringify_node("text")` calls `_escape_text_leaf` on the value. - `_stringify_node("image")` calls it on the `alt` attribute (alt is stored as a raw string, doesn't go through inline recursion). - `_stringify_node("list")` emits `[x] ` / `[ ] ` prefix when `item.checked` is set (unordered items only -- task lists are a bullet-only GFM convention). - Empty task items (`- [ ]` with no children) still get a line so the checkbox isn't lost on round-trip. Fixes in `base_format_converter.py`: - `_render_list` emits the same `[x] ` / `[ ] ` prefix so platform output (Slack mrkdwn, Discord, Teams, gchat, etc.) keeps the checkbox. Most platforms render the literal `[x]` as text; GitHub and Linear render it natively. Tests added (TestStringifyMarkdown): - `test_roundtrip_preserves_escaped_punctuation_in_text` (5 inputs: asterisk, bracket, backtick, tilde, backslash) - `test_roundtrip_preserves_task_list_state` (checked, unchecked, mixed) - `test_roundtrip_preserves_empty_task_item` - `test_stringify_does_not_over_escape_ordinary_text` 3,629 pass / 1 pre-existing unrelated failure. --- src/chat_sdk/shared/base_format_converter.py | 18 +++++++- src/chat_sdk/shared/markdown_parser.py | 44 ++++++++++++++++++-- tests/test_markdown_faithful.py | 38 +++++++++++++++++ 3 files changed, 95 insertions(+), 5 deletions(-) diff --git a/src/chat_sdk/shared/base_format_converter.py b/src/chat_sdk/shared/base_format_converter.py index 0f6f7e46..6cd43e27 100644 --- a/src/chat_sdk/shared/base_format_converter.py +++ b/src/chat_sdk/shared/base_format_converter.py @@ -100,7 +100,11 @@ def _render_list( ) -> str: """Render a list node with proper indentation. - Handles ordered and unordered lists and recurses into nested lists. + Handles ordered and unordered lists, recurses into nested lists, + and emits GFM task-list checkbox markers (``[ ]`` / ``[x]``) for + items carrying a ``checked`` attribute. Most chat platforms + either render the marker natively (GitHub, Linear) or display it + as literal text -- either way, dropping it would lose user data. """ indent = " " * depth start = node.get("start", 1) @@ -109,6 +113,13 @@ def _render_list( for i, item in enumerate(get_node_children(node)): prefix = f"{start + i}." if ordered else unordered_bullet + # GFM task-list marker, only on unordered items. + checked = item.get("checked") if not ordered else None + task_marker = "" + if checked is True: + task_marker = "[x] " + elif checked is False: + task_marker = "[ ] " is_first_content = True for child in get_node_children(item): if child.get("type") == "list": @@ -118,10 +129,13 @@ def _render_list( if not text.strip(): continue if is_first_content: - lines.append(f"{indent}{prefix} {text}") + lines.append(f"{indent}{prefix} {task_marker}{text}") is_first_content = False else: lines.append(f"{indent} {text}") + # Empty task item -- still emit a line so the marker isn't lost. + if is_first_content and task_marker: + lines.append(f"{indent}{prefix} {task_marker}".rstrip()) return "\n".join(lines) diff --git a/src/chat_sdk/shared/markdown_parser.py b/src/chat_sdk/shared/markdown_parser.py index a55993a5..d99422f0 100644 --- a/src/chat_sdk/shared/markdown_parser.py +++ b/src/chat_sdk/shared/markdown_parser.py @@ -222,6 +222,32 @@ def _unescape_punct(text: str) -> str: return "".join(out) +# Inverse of `_unescape_punct`: characters that, if present literally in +# a text leaf, would be parsed as the start of a markdown construct on +# the next ``parse_markdown`` call. Re-escape these at stringify time so +# the AST round-trips. Over-escaping ordinary punctuation produces noisy +# output without improving correctness -- only delimiters need it. +_TEXT_DELIMITERS = frozenset("*_~`[]\\") + + +def _escape_text_leaf(value: str) -> str: + """Re-escape delimiter characters so a text leaf round-trips through + parse_markdown unchanged. + + Inverse of :func:`_unescape_punct` -- a text node carrying the value + ``"*literal*"`` came from input ``\\*literal\\*``; stringify must + emit the backslashes or re-parse will form an emphasis node. + """ + if not any(c in _TEXT_DELIMITERS for c in value): + return value + out: list[str] = [] + for c in value: + if c in _TEXT_DELIMITERS: + out.append("\\") + out.append(c) + return "".join(out) + + def _parse_inline_plain(text: str) -> list[Content]: """Parse plain text that contains no inline formatting. @@ -632,7 +658,7 @@ def _stringify_node(node: Content, *, emphasis: str = "*", bullet: str = "*") -> node_type = node.get("type") if node_type == "text": - return node.get("value", "") + return _escape_text_leaf(node.get("value", "")) if node_type == "break": return "\n" @@ -680,7 +706,7 @@ def _stringify_node(node: Content, *, emphasis: str = "*", bullet: str = "*") -> return f"[{text}]({url})" if node_type == "image": - alt = node.get("alt", "") + alt = _escape_text_leaf(node.get("alt", "")) url = node.get("url", "") title = node.get("title") if title: @@ -694,7 +720,19 @@ def _stringify_node(node: Content, *, emphasis: str = "*", bullet: str = "*") -> lines: list[str] = [] for idx, item in enumerate(items): prefix = f"{start + idx}." if ordered else bullet + # GFM task-list marker, only on unordered items with `checked` set. + checked = item.get("checked") if not ordered else None + task_marker = "" + if checked is True: + task_marker = "[x] " + elif checked is False: + task_marker = "[ ] " item_children = item.get("children", []) + if not item_children: + # Empty item -- still emit the prefix (and any task marker) + # so the round-trip preserves an empty `- [ ]` task. + lines.append(f"{prefix} {task_marker}".rstrip()) + continue for ci, child in enumerate(item_children): child_type = child.get("type") if child_type == "list": @@ -706,7 +744,7 @@ def _stringify_node(node: Content, *, emphasis: str = "*", bullet: str = "*") -> else: text = _stringify_node(child, emphasis=emphasis, bullet=bullet) or "" if ci == 0: - lines.append(f"{prefix} {text}") + lines.append(f"{prefix} {task_marker}{text}") else: lines.append(f" {text}") return "\n".join(lines) diff --git a/tests/test_markdown_faithful.py b/tests/test_markdown_faithful.py index 63919912..e299484b 100644 --- a/tests/test_markdown_faithful.py +++ b/tests/test_markdown_faithful.py @@ -360,6 +360,44 @@ def test_roundtrips_markdown_correctly(self): reparsed = parse_markdown(result) assert len(reparsed["children"]) == len(ast["children"]) + def test_roundtrip_preserves_escaped_punctuation_in_text(self): + # PR #101 downstream review: text leaves carrying delimiter chars + # (post-unescape) must be re-escaped on stringify so re-parse + # doesn't form a new emphasis / link / strike node. + for src in [ + r"Use \*literal\*", + r"see \[brackets\] here", + r"backtick \` literal", + r"\~\~not strike\~\~", + r"path with \\backslash", + ]: + ast = parse_markdown(src) + ast2 = parse_markdown(stringify_markdown(ast)) + assert ast == ast2, f"AST diverged after roundtrip on {src!r}" + + def test_roundtrip_preserves_task_list_state(self): + # PR #101 downstream review: listItem.checked must be re-emitted + # so the checkbox isn't silently dropped on stringify. + for src in ["- [x] done", "- [ ] todo", "- [x] a\n- [ ] b\n- regular"]: + ast = parse_markdown(src) + ast2 = parse_markdown(stringify_markdown(ast)) + assert ast == ast2, f"task-list AST diverged after roundtrip on {src!r}" + + def test_roundtrip_preserves_empty_task_item(self): + ast = parse_markdown("- [ ]") + out = stringify_markdown(ast) + # The "[ ]" marker must survive the roundtrip + assert "[ ]" in out + ast2 = parse_markdown(out) + assert ast == ast2 + + def test_stringify_does_not_over_escape_ordinary_text(self): + # Only delimiter characters get backslash-prefixed; ordinary + # punctuation (periods, commas, !) stays clean. + ast = parse_markdown("Hello, world! It is fine.") + out = stringify_markdown(ast) + assert "\\" not in out + # ============================================================================ # toPlainText Tests From 797a360580657da9434f55e1be019dcab9a9e84f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 22 May 2026 09:52:32 +0000 Subject: [PATCH 06/15] fix(markdown): re-escape block-level markers at paragraph start Round-trip verification subagent surfaced a residual gap in the stringify path: while inline delimiter chars (`*`, `_`, `~`, `\``, `[`, `]`, `\`) are now re-escaped on text-leaf emission, block-level markers at paragraph start (`#`, `>`, `-`, `+`, ordered-list `digits.`, thematic-break `---`) weren't. Result: AST drift on roundtrip for inputs like `\# not heading` -> paragraph -> "# not heading" -> heading. The gap is narrow but real: it only fires when the very first text leaf of a paragraph starts with a block-marker character, which happens when a user typed an escape sequence to disambiguate from a block construct. Five cases drifted (heading / blockquote / unordered list / ordered list / thematic break). The sixth (`*`) was already covered by `_escape_text_leaf` since `*` is in the inline-delimiter set. Fix in `markdown_parser.py`: - New `_escape_paragraph_block_markers` helper applied to the joined output of `paragraph` nodes only. Mid-text occurrences of `#` / `>` / `-` / `1.` are safe and left alone (block markers only fire at line start). - Heading / blockquote / unordered-list / thematic-break: prepend `\`. - Ordered list: insert `\` between the digit run and the marker char (`.` or `)`), since digits aren't in CommonMark's escapable-punct set so `\1` wouldn't round-trip via `_unescape_punct`. Tests added (TestStringifyMarkdown): - `test_roundtrip_preserves_escaped_block_markers_at_paragraph_start` (8 inputs: all six block-marker families plus already-handled `*` and `---`) - `test_stringify_does_not_over_escape_mid_text_block_markers` (4 inputs verifying `\` doesn't appear when block-marker chars occur mid-text) 3,631 pass / 1 pre-existing unrelated failure. --- src/chat_sdk/shared/markdown_parser.py | 45 +++++++++++++++++++++++++- tests/test_markdown_faithful.py | 32 ++++++++++++++++++ 2 files changed, 76 insertions(+), 1 deletion(-) diff --git a/src/chat_sdk/shared/markdown_parser.py b/src/chat_sdk/shared/markdown_parser.py index d99422f0..37aca903 100644 --- a/src/chat_sdk/shared/markdown_parser.py +++ b/src/chat_sdk/shared/markdown_parser.py @@ -248,6 +248,48 @@ def _escape_text_leaf(value: str) -> str: return "".join(out) +# Block-level constructs only fire at line start. When a paragraph's +# joined text begins with one of these patterns -- typically after +# `_unescape_punct` resolved a ``\#`` or ``\>`` -- naive stringify would +# re-form the construct on the next parse. These regexes match the +# minimal opener pattern; `_escape_paragraph_block_markers` inserts the +# backslash at the right position. +_BLOCK_HEADING_RE = re.compile(r"^#{1,6}(?=\s|$)") +_BLOCK_BLOCKQUOTE_RE = re.compile(r"^>") +_BLOCK_UNORDERED_RE = re.compile(r"^[-+](?=\s)") +_BLOCK_ORDERED_RE = re.compile(r"^(\d+)([.)])(?=\s)") +_BLOCK_THEMATIC_DASH_RE = re.compile(r"^(-\s*){3,}\s*$") + + +def _escape_paragraph_block_markers(text: str) -> str: + """Re-escape block-level markers at the start of a paragraph. + + Applied to the joined output of a `paragraph` node only. Block-level + constructs (heading, blockquote, list, thematic break) only fire at + line start, so mid-text occurrences of ``#`` / ``>`` / ``-`` / ``+`` + are safe and don't need escaping. ``*``-based thematic breaks and + ``_``-based ones are already handled by :func:`_escape_text_leaf` + since both characters are in `_TEXT_DELIMITERS`. + """ + if not text: + return text + if _BLOCK_HEADING_RE.match(text): + return "\\" + text + if _BLOCK_BLOCKQUOTE_RE.match(text): + return "\\" + text + if _BLOCK_UNORDERED_RE.match(text): + return "\\" + text + m = _BLOCK_ORDERED_RE.match(text) + if m: + # Escape the . or ) marker, not the digits -- `\1` isn't a + # CommonMark escape (digits aren't in escapable punct), so a + # leading-backslash placement wouldn't survive `_unescape_punct`. + return f"{m.group(1)}\\{m.group(2)}{text[m.end() :]}" + if _BLOCK_THEMATIC_DASH_RE.match(text): + return "\\" + text + return text + + def _parse_inline_plain(text: str) -> list[Content]: """Parse plain text that contains no inline formatting. @@ -665,7 +707,8 @@ def _stringify_node(node: Content, *, emphasis: str = "*", bullet: str = "*") -> if node_type == "paragraph": children = node.get("children", []) - return "".join(_stringify_node(c, emphasis=emphasis, bullet=bullet) or "" for c in children) + joined = "".join(_stringify_node(c, emphasis=emphasis, bullet=bullet) or "" for c in children) + return _escape_paragraph_block_markers(joined) if node_type == "heading": depth = node.get("depth", 1) diff --git a/tests/test_markdown_faithful.py b/tests/test_markdown_faithful.py index e299484b..5d8256ae 100644 --- a/tests/test_markdown_faithful.py +++ b/tests/test_markdown_faithful.py @@ -398,6 +398,38 @@ def test_stringify_does_not_over_escape_ordinary_text(self): out = stringify_markdown(ast) assert "\\" not in out + def test_roundtrip_preserves_escaped_block_markers_at_paragraph_start(self): + # Block-level constructs (heading, blockquote, list, thematic + # break, ordered list) only fire at line start. When the user + # writes `\#`, `\>`, `\-`, etc. at paragraph start, stringify + # must re-emit the backslash or re-parse promotes the paragraph + # into the corresponding block construct. + for src in [ + r"\# not heading", + r"\> not quote", + r"\- not list", + r"\+ also not list", + r"1\. not ordered", + r"2\) other ordered", + r"\* not bullet", + r"\---", + ]: + ast = parse_markdown(src) + ast2 = parse_markdown(stringify_markdown(ast)) + assert ast == ast2, f"paragraph-start drift on {src!r}" + + def test_stringify_does_not_over_escape_mid_text_block_markers(self): + # `#` / `>` / `-` / digits-with-dot are only block markers at + # line start. Mid-text they're literal -- shouldn't be escaped. + for src in [ + "see > arrow operator", + "Step 1. Then 2. Then 3.", + "use - dash here", + "ordinary # hash inline", + ]: + out = stringify_markdown(parse_markdown(src)) + assert "\\" not in out, f"over-escape on {src!r}: {out!r}" + # ============================================================================ # toPlainText Tests From e67aa14f531d5c1a6a07d9604fb2a25d08ec004b Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 22 May 2026 10:00:09 +0000 Subject: [PATCH 07/15] fix(markdown): escape block markers on every line of a paragraph Fourth-round review subagent found that the previous fix only checked position 0 of the joined paragraph text, but paragraph text leaves can contain embedded `\n` (from `_unescape_punct` resolving an escaped marker after a soft line break, or from joining text leaves around a hard break). Block markers at the start of line 2+ of a paragraph silently re-form constructs on re-parse. Concrete drift cases the previous fix missed: - `"Here is a paragraph\n\\# 1 is not a heading"` -> paragraph -> stringify -> "Here is a paragraph\n# 1 is not a heading\n" -> re-parse -> paragraph + heading. - Same for `\>`, `\-`, `\+`, ordered-list `1\.`, thematic-break `\---` appearing on line 2+ of a multi-line paragraph. Fix splits the joined paragraph text on `\n` and applies the single-line escape per line, then rejoins. Single-line paragraphs (the common case) take a no-`\n` fast path. Adds a one-test `test_roundtrip_preserves_escaped_block_markers_after_soft_break` pinning all five drift cases. 3,632 pass / 1 pre-existing unrelated failure. --- src/chat_sdk/shared/markdown_parser.py | 58 ++++++++++++++++---------- tests/test_markdown_faithful.py | 16 +++++++ 2 files changed, 53 insertions(+), 21 deletions(-) diff --git a/src/chat_sdk/shared/markdown_parser.py b/src/chat_sdk/shared/markdown_parser.py index 37aca903..b1152cfe 100644 --- a/src/chat_sdk/shared/markdown_parser.py +++ b/src/chat_sdk/shared/markdown_parser.py @@ -261,33 +261,49 @@ def _escape_text_leaf(value: str) -> str: _BLOCK_THEMATIC_DASH_RE = re.compile(r"^(-\s*){3,}\s*$") +def _escape_block_marker_line(line: str) -> str: + """Re-escape a block-level marker at the start of a single line.""" + if not line: + return line + if _BLOCK_HEADING_RE.match(line): + return "\\" + line + if _BLOCK_BLOCKQUOTE_RE.match(line): + return "\\" + line + if _BLOCK_UNORDERED_RE.match(line): + return "\\" + line + m = _BLOCK_ORDERED_RE.match(line) + if m: + # Escape the . or ) marker, not the digits -- `\1` isn't a + # CommonMark escape (digits aren't in escapable punct), so a + # leading-backslash placement wouldn't survive `_unescape_punct`. + return f"{m.group(1)}\\{m.group(2)}{line[m.end() :]}" + if _BLOCK_THEMATIC_DASH_RE.match(line): + return "\\" + line + return line + + def _escape_paragraph_block_markers(text: str) -> str: - """Re-escape block-level markers at the start of a paragraph. + """Re-escape block-level markers at the start of every line in a + paragraph's joined output. - Applied to the joined output of a `paragraph` node only. Block-level + Applied to the joined output of a `paragraph` node. Block-level constructs (heading, blockquote, list, thematic break) only fire at - line start, so mid-text occurrences of ``#`` / ``>`` / ``-`` / ``+`` - are safe and don't need escaping. ``*``-based thematic breaks and - ``_``-based ones are already handled by :func:`_escape_text_leaf` - since both characters are in `_TEXT_DELIMITERS`. + line start, so mid-line occurrences of ``#`` / ``>`` / ``-`` / ``+`` + are safe. But a paragraph text leaf may contain ``\\n`` (from + `_unescape_punct` resolving an escaped marker after a soft line + break, or from joining text leaves around a hard break); each line + of the joined output needs the same escape treatment as line 1, or + re-parse will split the paragraph at the embedded marker. + + ``*``-based and ``_``-based thematic breaks are already handled by + :func:`_escape_text_leaf` since both characters are in + `_TEXT_DELIMITERS`. """ if not text: return text - if _BLOCK_HEADING_RE.match(text): - return "\\" + text - if _BLOCK_BLOCKQUOTE_RE.match(text): - return "\\" + text - if _BLOCK_UNORDERED_RE.match(text): - return "\\" + text - m = _BLOCK_ORDERED_RE.match(text) - if m: - # Escape the . or ) marker, not the digits -- `\1` isn't a - # CommonMark escape (digits aren't in escapable punct), so a - # leading-backslash placement wouldn't survive `_unescape_punct`. - return f"{m.group(1)}\\{m.group(2)}{text[m.end() :]}" - if _BLOCK_THEMATIC_DASH_RE.match(text): - return "\\" + text - return text + if "\n" not in text: + return _escape_block_marker_line(text) + return "\n".join(_escape_block_marker_line(line) for line in text.split("\n")) def _parse_inline_plain(text: str) -> list[Content]: diff --git a/tests/test_markdown_faithful.py b/tests/test_markdown_faithful.py index 5d8256ae..81c2caf3 100644 --- a/tests/test_markdown_faithful.py +++ b/tests/test_markdown_faithful.py @@ -430,6 +430,22 @@ def test_stringify_does_not_over_escape_mid_text_block_markers(self): out = stringify_markdown(parse_markdown(src)) assert "\\" not in out, f"over-escape on {src!r}: {out!r}" + def test_roundtrip_preserves_escaped_block_markers_after_soft_break(self): + # A paragraph text leaf can contain `\n` (from a soft break + a + # following line that doesn't qualify as a block construct + # because the marker was originally escaped). The line-2 marker + # also needs re-escaping or stringify drops it. + for src in [ + "Here is a paragraph\n\\# 1 is not a heading", + "first line\n\\> not quote", + "line one\n\\- not list", + "alpha\n1\\. not ordered", + "alpha\n\\---", + ]: + ast = parse_markdown(src) + ast2 = parse_markdown(stringify_markdown(ast)) + assert ast == ast2, f"soft-break drift on {src!r}" + # ============================================================================ # toPlainText Tests From 0bb035324174b215f3a2a9853444ed4dc741fe86 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 23 May 2026 00:42:13 +0000 Subject: [PATCH 08/15] fix(streaming-markdown): preserve monotonicity on back-to-back tables PR #99 review (chatgpt-codex P1): in `_get_committable_prefix`, the "hold pre-separator block" backward walk uses `TABLE_ROW_RE` to extend `table_start`, but separator lines also match that pattern. For a stream like `|A|B|\n|---|---|\n|1|2|\n|C|D|\n|---|---|\n` (two tables with no blank line between them), the walk crosses the first table's separator and collapses `_get_committable_prefix` back to "", violating the monotonic append-only contract of `get_committable_text()`. Adapters that compute deltas from prior committed length then emit incorrect/no deltas. Fix: the backward walk now stops at empty lines (existing), non-row content (existing), AND at prior separators (new). When it hits a prior separator -- meaning the candidate "new header" row above it was already committed as a body row of the prior table -- the function falls back to `return text` instead of holding. That emits one "stray separator" on the rollback delta, which is broken markup but the lesser evil compared to non-monotonic rollback. The fix preserves the well-formed multi-table case where the second table is separated from the first by a blank line; the empty-line break in the walk fires before reaching the prior separator. Tests added: - `test_back_to_back_tables_keep_committable_monotonic` (the exact reviewer repro) - `test_second_table_after_blank_line_still_holds_header` (verifies the fix doesn't regress the blank-line-separated case) The outdated comment from gemini-code-assist on `_is_excluded_asterisk` suggesting we also include `\n` and end-of-string in the list-marker exclusion is already addressed in the second commit on this branch, which broadened the helper to match remend's `shouldSkipAsterisk` exactly. 79 streaming tests pass / 3598 total / 1 pre-existing unrelated failure. --- src/chat_sdk/shared/streaming_markdown.py | 14 ++++++++++ tests/test_streaming_markdown.py | 33 +++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/src/chat_sdk/shared/streaming_markdown.py b/src/chat_sdk/shared/streaming_markdown.py index 57cf8a57..6a4f8809 100644 --- a/src/chat_sdk/shared/streaming_markdown.py +++ b/src/chat_sdk/shared/streaming_markdown.py @@ -287,12 +287,26 @@ def _get_committable_prefix(text: str) -> str: # independently; emitting a header+separator without rows produces # broken syntax. Hold the entire pre-separator table block until a # row arrives (issue #69). + # + # The backward walk must also stop at empty lines (end of the + # current block) and at prior separators -- prior separators mean + # the candidate "new header" rows above them were already + # committed as body rows of a previously confirmed table. + # Rolling them back would violate the monotonic append-only + # contract; in that case fall through to "commit everything" + # instead (PR #99 review finding). table_start = separator_idx + rolled_back = False for i in range(separator_idx - 1, -1, -1): trimmed = lines[i].strip() if trimmed == "" or TABLE_ROW_RE.match(trimmed) is None: break + if TABLE_SEPARATOR_RE.match(trimmed): + rolled_back = True + break table_start = i + if rolled_back: + return text commit_line_count = table_start elif separator_found or held_count == 0: return text diff --git a/tests/test_streaming_markdown.py b/tests/test_streaming_markdown.py index 30afc478..392a55a1 100644 --- a/tests/test_streaming_markdown.py +++ b/tests/test_streaming_markdown.py @@ -1056,3 +1056,36 @@ def test_table_held_block_flushes_on_finish_even_without_body_row(self): final = r.finish() assert "| H |" in final assert "|---|" in final + + def test_back_to_back_tables_keep_committable_monotonic(self): + # PR #99 review #2: when a second table's separator arrives in a + # stream that already had one confirmed table (and no blank line + # between them), the backward "hold pre-separator block" walk + # would roll back into the previously-committed first-table + # body. Verify get_committable_text() never shrinks. + r = StreamingMarkdownRenderer(wrap_tables_for_append=False) + chunks = [ + "|A|B|\n|---|---|\n|1|2|\n", # full first table + "|C|D|\n", # second table's header (committed as body of first) + "|---|---|\n", # second table's separator -- must not roll back + ] + last = "" + for chunk in chunks: + r.push(chunk) + cur = r.get_committable_text() + assert cur.startswith(last), ( + f"monotonicity violated: prior committed prefix={last!r} not a prefix of new committed={cur!r}" + ) + last = cur + + def test_second_table_after_blank_line_still_holds_header(self): + # The fix above must NOT regress the well-formed multi-table + # case where tables are separated by a blank line. + r = StreamingMarkdownRenderer(wrap_tables_for_append=False) + r.push("| A | B |\n|---|---|\n| 1 | 2 |\n") + r.push("\n| X | Y |\n") # blank line then new header + r.push("|---|---|\n") # new separator + # `| X | Y |` is still held -- second table has no body row yet. + committable = r.get_committable_text() + assert "| X | Y |" not in committable + assert "| 1 | 2 |" in committable From 12fed105f82e0a50a0676d8055b6e4b5fbef8283 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 23 May 2026 00:45:45 +0000 Subject: [PATCH 09/15] fix(markdown): address PR #101 review findings (math, task nested) Three findings actioned: 1. **Math regex too broad** (gemini-code-assist, high). The inline-math pattern `\$[^\$\n]+\$` matched plain currency text like `prices are $5 and $10`, eating the `$5 and $10` portion and corrupting downstream inline-marker counts. Tightened to require non-whitespace immediately after the opening `$` and before the closing `$` (standard pandoc / markdown-it heuristic). Display math also tightened (`.+?` instead of `.*?`) to require non-empty content and avoid matching bare `$$$$`. 2. **Task item with only nested list children loses parent prefix** (chatgpt-codex P1). `parse_markdown("- [ ]\n - child")` produces a listItem with `checked: False` and only a nested-list child. Stringify hit the nested-list branch first, indented and appended, never wrote the `* [ ]` parent line. Re-parse then dropped the parent entirely. Fixed by tracking `prefix_emitted` across the item's children -- if the first non-skipped child is a nested list, emit the parent prefix line first. 3. **Same scenario in `_render_list` produces reversed output** (chatgpt-codex P2). `is_first_content` stayed True through the nested-list branch, so the empty-task fallback appended the checkbox line AFTER the nested rendering. Fixed by emitting the prefix before recursing into the nested list when it's the first content, then dropping the `if task_marker` qualifier on the fallback so any item with zero rendered children still gets a line (was previously only firing for task items). Tests added: - `test_task_item_with_only_nested_list_keeps_parent_prefix` - `test_remend_unescaped_currency_does_not_pair_as_math` 3,634 pass / 1 pre-existing unrelated failure. --- src/chat_sdk/shared/base_format_converter.py | 12 ++++++++++-- src/chat_sdk/shared/markdown_parser.py | 16 +++++++++++++--- src/chat_sdk/shared/streaming_markdown.py | 12 +++++++++--- tests/test_markdown_faithful.py | 14 ++++++++++++++ tests/test_streaming_markdown.py | 7 +++++++ 5 files changed, 53 insertions(+), 8 deletions(-) diff --git a/src/chat_sdk/shared/base_format_converter.py b/src/chat_sdk/shared/base_format_converter.py index 6cd43e27..2ddc5aa4 100644 --- a/src/chat_sdk/shared/base_format_converter.py +++ b/src/chat_sdk/shared/base_format_converter.py @@ -123,6 +123,13 @@ def _render_list( is_first_content = True for child in get_node_children(item): if child.get("type") == "list": + if is_first_content: + # No text child before this nested list -- emit + # the parent prefix on its own line BEFORE the + # nested list, or the checkbox/parent ordering + # ends up reversed (PR #101 review finding). + lines.append(f"{indent}{prefix} {task_marker}".rstrip()) + is_first_content = False lines.append(self._render_list(child, depth + 1, node_converter, unordered_bullet)) continue text = node_converter(child) @@ -133,8 +140,9 @@ def _render_list( is_first_content = False else: lines.append(f"{indent} {text}") - # Empty task item -- still emit a line so the marker isn't lost. - if is_first_content and task_marker: + # Item with zero rendered children -- still emit a line so the + # listItem (and any task marker) isn't lost. + if is_first_content: lines.append(f"{indent}{prefix} {task_marker}".rstrip()) return "\n".join(lines) diff --git a/src/chat_sdk/shared/markdown_parser.py b/src/chat_sdk/shared/markdown_parser.py index b1152cfe..e472cb55 100644 --- a/src/chat_sdk/shared/markdown_parser.py +++ b/src/chat_sdk/shared/markdown_parser.py @@ -792,18 +792,28 @@ def _stringify_node(node: Content, *, emphasis: str = "*", bullet: str = "*") -> # so the round-trip preserves an empty `- [ ]` task. lines.append(f"{prefix} {task_marker}".rstrip()) continue - for ci, child in enumerate(item_children): + # Track whether the parent prefix has been written so a + # listItem whose only children are nested lists still gets + # its own line (PR #101 review finding). + prefix_emitted = False + for child in item_children: child_type = child.get("type") if child_type == "list": + if not prefix_emitted: + # No text child before this nested list -- emit + # the parent prefix on its own line so the + # listItem and any checkbox state survive. + lines.append(f"{prefix} {task_marker}".rstrip()) + prefix_emitted = True nested = _stringify_node(child, emphasis=emphasis, bullet=bullet) if nested: - # Indent nested list for nl in nested.split("\n"): lines.append(f" {nl}") else: text = _stringify_node(child, emphasis=emphasis, bullet=bullet) or "" - if ci == 0: + if not prefix_emitted: lines.append(f"{prefix} {task_marker}{text}") + prefix_emitted = True else: lines.append(f" {text}") return "\n".join(lines) diff --git a/src/chat_sdk/shared/streaming_markdown.py b/src/chat_sdk/shared/streaming_markdown.py index 899fdd1b..a7dc4c93 100644 --- a/src/chat_sdk/shared/streaming_markdown.py +++ b/src/chat_sdk/shared/streaming_markdown.py @@ -92,13 +92,19 @@ def _strip_math_regions(text: str) -> str: left alone so it doesn't gobble unrelated prose. The double-dollar pass runs first so ``$$`` blocks don't get mis-paired by the single pass. + + Inline math requires a non-whitespace char immediately after the + opening ``$`` and immediately before the closing ``$`` (standard + pandoc/markdown-it heuristic). This prevents false positives on + currency text like ``"prices are $5 and $10"`` which would otherwise + pair up the two dollar signs as math (PR #101 review finding). """ if "$" not in text: return text # Display math: $$...$$ may span multiple lines. - text = re.sub(r"\$\$.*?\$\$", "", text, flags=re.DOTALL) - # Inline math: $...$ within a single line, non-greedy, no embedded $. - text = re.sub(r"\$[^\$\n]+\$", "", text) + text = re.sub(r"\$\$.+?\$\$", "", text, flags=re.DOTALL) + # Inline math: $X...Y$ on a single line where X and Y are non-whitespace. + text = re.sub(r"\$(?!\s)[^\$\n]+?(? Date: Sat, 23 May 2026 02:15:30 +0000 Subject: [PATCH 10/15] docs(markdown): note fixed-width lookbehind limitation PR #101 review (chatgpt-codex P2): the inline regex lookbehinds only check one character back, so an escaped backslash followed by a real delimiter (e.g. `\\*foo*`) is treated as an escaped delimiter and falls through to plain text -- losing the intended emphasis. Fixing this properly requires variable-width lookbehind (not supported by Python's `re`) or a placeholder-based pre-pass that substitutes escape sequences before the regex runs. Both are moderate refactors of the inline parser. The pattern is rare in LLM / chat output (technical docs explaining backslash + emphasis typically use code fences), so deferring to a future chat-completeness pass. Added a comment block near `_INLINE_PATTERNS` documenting the limitation and the recommended fix path so the next person to touch this code sees it immediately. --- src/chat_sdk/shared/markdown_parser.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/chat_sdk/shared/markdown_parser.py b/src/chat_sdk/shared/markdown_parser.py index e472cb55..b515a5af 100644 --- a/src/chat_sdk/shared/markdown_parser.py +++ b/src/chat_sdk/shared/markdown_parser.py @@ -180,6 +180,17 @@ def get_node_value(node: Content) -> str: # is treated as literal text rather than as a delimiter run. Bracket # contents (link / image) also tolerate inner ``\]`` / ``\[`` via the # ``(?:[^\]\\]|\\.)*`` pattern. +# +# Known limitation (issue #69 follow-up): the fixed-width lookbehind +# can only check the immediately preceding character, so ``\\*foo*`` +# (an escaped backslash followed by genuine emphasis) is treated as +# escaped emphasis and falls through to plain text -- the resulting +# AST loses the italic semantics. Fixing this requires either a +# variable-width lookbehind (Python's ``re`` doesn't support that) or +# a placeholder-based pre-pass that protects escape sequences before +# the regex runs. Tracked for the next chat-completeness pass; the +# pattern is rare enough in LLM/chat output to defer (LLMs explaining +# regex syntax usually use code fences). _INLINE_PATTERNS = [ # Images: ![alt](url) or ![alt](url "title") ("image", re.compile(r'(? Date: Sat, 23 May 2026 02:30:51 +0000 Subject: [PATCH 11/15] fix(markdown): doubled-backslash escape via sentinel-based pre-pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the PR #101 review #4 finding (chatgpt-codex P2) properly rather than deferring. `\\*foo*` (escaped backslash + real italic) now parses correctly as text(`\`) + emphasis(`foo`); previously the fixed-width `(?X` where SENTINEL = U+0001 (a control codepoint that never legitimately appears in chat markdown). The inline regex lookbehinds now check for the sentinel instead of `\`, which trivially handles any number of preceding backslashes: - `\*` -> `*`. Regex lookbehind sees SENTINEL, skips. - `\\*` -> `\*`. The `*` is preceded by `\` (not SENTINEL), so the lookbehind passes and emphasis opens correctly. - `\\\*` -> `\*`. The `*` is preceded by SENTINEL, lookbehind skips. Correct (3 backslashes = escaped backslash + escaped asterisk = both literal). The complementary restore functions handle the two emission modes: - `_restore_escapes(text)`: SENTINEL pairs -> literal X (text leaves and image alt -- the backslash is dropped because the escape resolved to a literal char). - `_restore_escapes_as_literal_pair(text)`: SENTINEL pairs -> `\X` (inline code -- CommonMark preserves backslashes inside code spans). Changes: - New helpers `_protect_escapes`, `_restore_escapes`, `_restore_escapes_as_literal_pair`. ~50 LOC. - `_INLINE_PATTERNS` lookbehinds changed from `(? str: # ASCII-punctuation characters that may be backslash-escaped per # CommonMark. A preceding `\` makes the next character literal -- not a -# delimiter -- which the lookbehinds in the patterns below honour. +# delimiter -- which `_protect_escapes` consumes into a sentinel-prefixed +# pair before the inline regexes run. _ESCAPABLE_PUNCT = set("!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~") -# Regex patterns for inline elements, ordered by priority. -# Each pattern captures a full inline construct. Every opening delimiter -# uses a ``(?X``. The sentinel is a control codepoint +# (U+0001) that never legitimately appears in chat-content markdown, so +# any occurrence in the protected text unambiguously marks an escape. +# The inline-pattern lookbehinds reject delimiters preceded by this +# sentinel; everything else (including any number of literal +# backslashes) is treated as a real delimiter -- which closes the +# doubled-backslash gap that fixed-width lookbehind on ``\`` couldn't. +_ESCAPE_SENTINEL = "\x01" + + +def _protect_escapes(text: str) -> str: + """Replace every ``\\X`` escape pair with ``X`` in *text*. + + Used as a pre-pass before the inline regex matching. Each escaped + delimiter character is marked so the regex lookbehinds can skip it + via ``(? str: - """Resolve backslash-escapes of ASCII punctuation to their literal char. +def _restore_escapes(text: str) -> str: + """Inverse of :func:`_protect_escapes` for text-leaf emission. - Applied at text-leaf emission only -- inline delimiters were already - consumed by the patterns above with escape-aware lookbehinds, so by - the time we reach a text leaf, any remaining ``\\X`` pair where X is - ASCII punct is unambiguously a literal X. + Replaces each ``X`` pair with literal ``X``. The + backslash is dropped because the escape sequence resolved to a + literal character at the AST text leaf. """ - if "\\" not in text: + if _ESCAPE_SENTINEL not in text: return text out: list[str] = [] i = 0 while i < len(text): - if text[i] == "\\" and i + 1 < len(text) and text[i + 1] in _ESCAPABLE_PUNCT: + if text[i] == _ESCAPE_SENTINEL and i + 1 < len(text): + out.append(text[i + 1]) + i += 2 + continue + out.append(text[i]) + i += 1 + return "".join(out) + + +def _restore_escapes_as_literal_pair(text: str) -> str: + """Inverse for inline-code emission: ``X`` -> ``\\X``. + + Per CommonMark, backslashes inside inline code spans are preserved + literally -- ``\\*`` inside a code span stays as ``\\*``, not ``*``. + """ + if _ESCAPE_SENTINEL not in text: + return text + out: list[str] = [] + i = 0 + while i < len(text): + if text[i] == _ESCAPE_SENTINEL and i + 1 < len(text): + out.append("\\") out.append(text[i + 1]) i += 2 continue @@ -233,11 +254,53 @@ def _unescape_punct(text: str) -> str: return "".join(out) -# Inverse of `_unescape_punct`: characters that, if present literally in -# a text leaf, would be parsed as the start of a markdown construct on -# the next ``parse_markdown`` call. Re-escape these at stringify time so -# the AST round-trips. Over-escaping ordinary punctuation produces noisy -# output without improving correctness -- only delimiters need it. +# Regex patterns for inline elements, ordered by priority. Each pattern +# captures a full inline construct. +# +# The patterns operate on text that has already been through +# `_protect_escapes`, so escaped delimiters appear as ``X`` +# (where SENTINEL = U+0001). The ``(? ``\\``) -- are left as literal ``\`` and don't +# trigger the lookbehind, so the following delimiter is correctly +# recognised as a real opener (closing the doubled-backslash gap). +# +# Bracket content groups still need ``(?:[^\]\x01]|\x01.)+?`` so that +# escaped ``\]`` / ``\[`` inside link text don't prematurely terminate +# the match -- the alternation consumes a ``X`` pair as a +# single unit. +_INLINE_PATTERNS = [ + # Images: ![alt](url) or ![alt](url "title") + ("image", re.compile(r'(? str: """Re-escape delimiter characters so a text leaf round-trips through parse_markdown unchanged. - Inverse of :func:`_unescape_punct` -- a text node carrying the value - ``"*literal*"`` came from input ``\\*literal\\*``; stringify must - emit the backslashes or re-parse will form an emphasis node. + Counterpart of :func:`_restore_escapes` -- a text node carrying the + value ``"*literal*"`` came from input ``\\*literal\\*``; stringify + must emit the backslashes or re-parse will form an emphasis node. """ if not any(c in _TEXT_DELIMITERS for c in value): return value @@ -261,10 +324,10 @@ def _escape_text_leaf(value: str) -> str: # Block-level constructs only fire at line start. When a paragraph's # joined text begins with one of these patterns -- typically after -# `_unescape_punct` resolved a ``\#`` or ``\>`` -- naive stringify would -# re-form the construct on the next parse. These regexes match the -# minimal opener pattern; `_escape_paragraph_block_markers` inserts the -# backslash at the right position. +# `_restore_escapes` resolved a ``\#`` or ``\>`` to its literal char -- +# naive stringify would re-form the construct on the next parse. These +# regexes match the minimal opener pattern; `_escape_paragraph_block_markers` +# inserts the backslash at the right position. _BLOCK_HEADING_RE = re.compile(r"^#{1,6}(?=\s|$)") _BLOCK_BLOCKQUOTE_RE = re.compile(r"^>") _BLOCK_UNORDERED_RE = re.compile(r"^[-+](?=\s)") @@ -286,7 +349,7 @@ def _escape_block_marker_line(line: str) -> str: if m: # Escape the . or ) marker, not the digits -- `\1` isn't a # CommonMark escape (digits aren't in escapable punct), so a - # leading-backslash placement wouldn't survive `_unescape_punct`. + # leading-backslash placement wouldn't survive `_restore_escapes`. return f"{m.group(1)}\\{m.group(2)}{line[m.end() :]}" if _BLOCK_THEMATIC_DASH_RE.match(line): return "\\" + line @@ -301,7 +364,7 @@ def _escape_paragraph_block_markers(text: str) -> str: constructs (heading, blockquote, list, thematic break) only fire at line start, so mid-line occurrences of ``#`` / ``>`` / ``-`` / ``+`` are safe. But a paragraph text leaf may contain ``\\n`` (from - `_unescape_punct` resolving an escaped marker after a soft line + `_restore_escapes` resolving an escaped marker after a soft line break, or from joining text leaves around a hard break); each line of the joined output needs the same escape treatment as line 1, or re-parse will split the paragraph at the embedded marker. @@ -320,27 +383,33 @@ def _escape_paragraph_block_markers(text: str) -> str: def _parse_inline_plain(text: str) -> list[Content]: """Parse plain text that contains no inline formatting. - Handles hard line breaks (two trailing spaces + newline) and resolves - backslash escapes of ASCII punctuation to their literal character. + Handles hard line breaks (two trailing spaces + newline) and + restores escape-sentinel pairs to their literal character. The + caller is expected to have run ``_protect_escapes`` on the input. """ if " \n" in text: parts: list[Content] = [] segments = text.split(" \n") for i, seg in enumerate(segments): if seg: - parts.append(make_text(_unescape_punct(seg))) + parts.append(make_text(_restore_escapes(seg))) if i < len(segments) - 1: parts.append(make_break()) - return parts if parts else [make_text(_unescape_punct(text))] - return [make_text(_unescape_punct(text))] + return parts if parts else [make_text(_restore_escapes(text))] + return [make_text(_restore_escapes(text))] -def _parse_inline(text: str) -> list[Content]: +def _parse_inline(text: str, *, _already_protected: bool = False) -> list[Content]: """Parse inline markdown elements into AST nodes. Handles: strong, emphasis, delete, inlineCode, link, image. Returns a list of inline Content nodes. + Runs ``_protect_escapes`` on entry so the inline regex patterns can + use sentinel-based lookbehinds. Recursive calls (e.g. into link + text or emphasis content) pass ``_already_protected=True`` because + the captured group is already in the protected form. + The suffix (text after a match) is processed iteratively to avoid unbounded recursion on long strings. Content *inside* a match (e.g. bold text, link text) still recurses, but that depth is @@ -348,6 +417,8 @@ def _parse_inline(text: str) -> list[Content]: """ if not text: return [] + if not _already_protected: + text = _protect_escapes(text) nodes: list[Content] = [] remaining = text @@ -376,23 +447,30 @@ def _parse_inline(text: str) -> list[Content]: # The matched construct (content recursion is bounded by match length) if best_kind == "image": - alt = _unescape_punct(best_match.group(1)) - url = best_match.group(2) + alt = _restore_escapes(best_match.group(1)) + url = _restore_escapes(best_match.group(2)) title = best_match.group(3) + if title is not None: + title = _restore_escapes(title) nodes.append(make_image(url, alt, title)) elif best_kind == "link": link_text = best_match.group(1) - url = best_match.group(2) + url = _restore_escapes(best_match.group(2)) title = best_match.group(3) - nodes.append(make_link(url, _parse_inline(link_text), title)) + if title is not None: + title = _restore_escapes(title) + nodes.append(make_link(url, _parse_inline(link_text, _already_protected=True), title)) elif best_kind == "inlineCode": - nodes.append(make_inline_code(best_match.group(1))) + # CommonMark: backslashes inside inline code are literal, + # so restore sentinel pairs as `\X` rather than dropping the + # backslash. + nodes.append(make_inline_code(_restore_escapes_as_literal_pair(best_match.group(1)))) elif best_kind in ("strong_star", "strong_under"): - nodes.append(make_strong(_parse_inline(best_match.group(1)))) + nodes.append(make_strong(_parse_inline(best_match.group(1), _already_protected=True))) elif best_kind == "delete": - nodes.append(make_delete(_parse_inline(best_match.group(1)))) + nodes.append(make_delete(_parse_inline(best_match.group(1), _already_protected=True))) elif best_kind in ("emphasis_star", "emphasis_under"): - nodes.append(make_emphasis(_parse_inline(best_match.group(1)))) + nodes.append(make_emphasis(_parse_inline(best_match.group(1), _already_protected=True))) # Advance past the match (iterative, not recursive) remaining = remaining[best_match.end() :] diff --git a/tests/test_markdown_faithful.py b/tests/test_markdown_faithful.py index 6e182d75..ef22b4e0 100644 --- a/tests/test_markdown_faithful.py +++ b/tests/test_markdown_faithful.py @@ -255,6 +255,29 @@ def test_inline_code_contents_are_not_unescaped(self): assert len(code_nodes) == 1 assert code_nodes[0]["value"] == r"\*literal\*" + def test_escaped_backslash_followed_by_real_emphasis(self): + # PR #101 review #4 (chatgpt-codex P2): ``\\*foo*`` is literal + # backslash + italic. The fixed-width ``(? Date: Sat, 23 May 2026 04:50:09 +0000 Subject: [PATCH 12/15] fix(markdown): harden escape sentinel + pin URL escape contract Second round of subagent review on commit 48fa4f3 surfaced three issues: 1. **U+0001 sentinel could be mangled by adapter-supplied input** (blocker). The pre-pass used `\x01` as the escape sentinel and assumed it would never appear in chat content -- but no adapter strips control characters, so input like `before\x01*after*` would have the sentinel paired with the asterisk and dropped, corrupting the visible text. Switched the sentinel to U+FDD0 (a Unicode noncharacter -- one of the 32 BMP codepoints permanently reserved by the Unicode standard for process-internal application use; never assigned to a real character). Also added a strip step at `_protect_escapes` entry: any literal U+FDD0 in input is removed before the escape walk runs, as defense-in-depth. Updated all the `(? str: _ESCAPABLE_PUNCT = set("!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~") # Sentinel character used by `_protect_escapes` to mark every ``\X`` -# escape sequence as ``X``. The sentinel is a control codepoint -# (U+0001) that never legitimately appears in chat-content markdown, so -# any occurrence in the protected text unambiguously marks an escape. -# The inline-pattern lookbehinds reject delimiters preceded by this -# sentinel; everything else (including any number of literal -# backslashes) is treated as a real delimiter -- which closes the -# doubled-backslash gap that fixed-width lookbehind on ``\`` couldn't. -_ESCAPE_SENTINEL = "\x01" +# escape sequence as ``X``. U+FDD0 is one of the 32 BMP +# "noncharacters" permanently reserved by the Unicode standard for +# process-internal application use -- by definition, no real text +# interchange uses these codepoints. `_protect_escapes` also strips +# any pre-existing occurrence from the input as a defense-in-depth +# measure, so the sentinel-based round trip is robust even when an +# adversarial caller smuggles the codepoint in. +_ESCAPE_SENTINEL = "﷐" def _protect_escapes(text: str) -> str: @@ -191,12 +191,20 @@ def _protect_escapes(text: str) -> str: Used as a pre-pass before the inline regex matching. Each escaped delimiter character is marked so the regex lookbehinds can skip it - via ``(?)``. Unlike the previous ``(? str: # # The patterns operate on text that has already been through # `_protect_escapes`, so escaped delimiters appear as ``X`` -# (where SENTINEL = U+0001). The ``(? ``\\``) -- are left as literal ``\`` and don't -# trigger the lookbehind, so the following delimiter is correctly -# recognised as a real opener (closing the doubled-backslash gap). +# (where SENTINEL = U+FDD0 -- the f-string ``﷐`` below). The +# ``(? ``\\``) -- are left +# as literal ``\`` and don't trigger the lookbehind, so the following +# delimiter is correctly recognised as a real opener (closing the +# doubled-backslash gap). # -# Bracket content groups still need ``(?:[^\]\x01]|\x01.)+?`` so that -# escaped ``\]`` / ``\[`` inside link text don't prematurely terminate -# the match -- the alternation consumes a ``X`` pair as a -# single unit. +# Bracket content groups still need ``(?:[^\]﷐]|﷐.)+?`` so +# that escaped ``\]`` / ``\[`` inside link text don't prematurely +# terminate the match -- the alternation consumes a ``X`` +# pair as a single unit. _INLINE_PATTERNS = [ # Images: ![alt](url) or ![alt](url "title") - ("image", re.compile(r'(? Date: Sat, 23 May 2026 15:05:18 +0000 Subject: [PATCH 13/15] fix(markdown): don't leak escape sentinel from code spans ending in backslash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Holistic re-review blocker: a code span ending in a backslash (e.g. `` `C:\` ``, `` `\d+\` ``) left the escape sentinel (U+FDD0) dangling in the captured content, leaking the noncharacter into stringify_markdown / ast_to_plain_text output as a replacement glyph. Root cause: `_protect_escapes` rewrites `\X` -> `X`. For `` `C:\` `` the `\`+backtick becomes ``+backtick, and the inline-code regex `` (?` -- the sentinel's paired backtick falls outside the group as the code-span closer. Both restore functions guarded on `i + 1 < len(text)`, so a trailing sentinel with no following char was emitted raw. Fix: both restore functions now handle a dangling sentinel explicitly. `_restore_escapes_as_literal_pair` (code spans) emits a bare backslash -- matching CommonMark's "backslash is literal inside code spans" rule, so `` `C:\` `` correctly yields code content `C:\`. `_restore_escapes` (text leaves / URLs) likewise emits a backslash rather than the raw sentinel. Neither can ever emit U+FDD0 now. Test added: `test_code_span_ending_in_backslash_does_not_leak_sentinel` covers three realistic code-span-ending-in-backslash inputs and asserts the sentinel codepoint never reaches output. 3,638 pass / 1 pre-existing unrelated failure. --- src/chat_sdk/shared/markdown_parser.py | 35 +++++++++++++++++++++----- tests/test_markdown_faithful.py | 18 +++++++++++++ 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/src/chat_sdk/shared/markdown_parser.py b/src/chat_sdk/shared/markdown_parser.py index f4503e1d..570265e5 100644 --- a/src/chat_sdk/shared/markdown_parser.py +++ b/src/chat_sdk/shared/markdown_parser.py @@ -226,15 +226,28 @@ def _restore_escapes(text: str) -> str: Replaces each ``X`` pair with literal ``X``. The backslash is dropped because the escape sequence resolved to a literal character at the AST text leaf. + + A *dangling* sentinel (one with no following char, which arises when + the escaped character was consumed as a delimiter -- e.g. a code + span ending in ``\\``, where the backtick that followed the sentinel + became the closing delimiter) resolves to a literal backslash. This + must never emit the raw sentinel codepoint, which would leak U+FDD0 + into user-visible output. """ if _ESCAPE_SENTINEL not in text: return text out: list[str] = [] i = 0 while i < len(text): - if text[i] == _ESCAPE_SENTINEL and i + 1 < len(text): - out.append(text[i + 1]) - i += 2 + if text[i] == _ESCAPE_SENTINEL: + if i + 1 < len(text): + out.append(text[i + 1]) + i += 2 + else: + # Dangling sentinel: the escaped char was consumed + # elsewhere; keep the backslash, never the sentinel. + out.append("\\") + i += 1 continue out.append(text[i]) i += 1 @@ -246,16 +259,26 @@ def _restore_escapes_as_literal_pair(text: str) -> str: Per CommonMark, backslashes inside inline code spans are preserved literally -- ``\\*`` inside a code span stays as ``\\*``, not ``*``. + + A dangling sentinel (e.g. from a code span ending in ``\\`` where the + closing backtick was the sentinel's paired char) resolves to a bare + backslash, matching CommonMark's "backslash is literal in code" + rule. Never emits the raw sentinel codepoint. """ if _ESCAPE_SENTINEL not in text: return text out: list[str] = [] i = 0 while i < len(text): - if text[i] == _ESCAPE_SENTINEL and i + 1 < len(text): + if text[i] == _ESCAPE_SENTINEL: out.append("\\") - out.append(text[i + 1]) - i += 2 + if i + 1 < len(text): + out.append(text[i + 1]) + i += 2 + else: + # Dangling sentinel (e.g. code span ending in `\`): the + # paired char became a delimiter; keep the backslash only. + i += 1 continue out.append(text[i]) i += 1 diff --git a/tests/test_markdown_faithful.py b/tests/test_markdown_faithful.py index 6150476f..e1f62a3c 100644 --- a/tests/test_markdown_faithful.py +++ b/tests/test_markdown_faithful.py @@ -271,6 +271,24 @@ def test_inline_code_contents_are_not_unescaped(self): assert len(code_nodes) == 1 assert code_nodes[0]["value"] == r"\*literal\*" + def test_code_span_ending_in_backslash_does_not_leak_sentinel(self): + # PR #101 re-review (holistic): a code span ending in `\` (e.g. + # `` `C:\` ``) used to leave the escape sentinel (U+FDD0) + # dangling in the captured content, leaking the noncharacter + # into output. The dangling sentinel now resolves to a literal + # backslash (CommonMark: backslash is literal inside code spans). + for src, expected_code in [ + ("Use `C:\\` as root", "C:\\"), + ("regex `\\d+\\` here", "\\d+\\"), + ("a `b\\` c", "b\\"), + ]: + children = self._para_children(src) + code_nodes = [c for c in children if c.get("type") == "inlineCode"] + assert len(code_nodes) == 1, f"expected 1 code span in {src!r}" + assert code_nodes[0]["value"] == expected_code + # The sentinel codepoint must never reach output. + assert "﷐" not in code_nodes[0]["value"] + def test_escaped_backslash_followed_by_real_emphasis(self): # PR #101 review #4 (chatgpt-codex P2): ``\\*foo*`` is literal # backslash + italic. The fixed-width ``(? Date: Thu, 28 May 2026 03:06:39 +0000 Subject: [PATCH 14/15] ci: retrigger after pre-existing test_github_webhook flake From 2856d95c3a4cb8a86c3ec19cb592d36363e943d9 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 28 May 2026 04:13:41 +0000 Subject: [PATCH 15/15] fix(markdown): list-item continuation indent + hard break stringify Adversarial re-review surfaced two pre-existing stringify round-trip bugs that are commonly hit by LLM-produced markdown. Fixing them in this PR rather than filing follow-ups (per the team preference for clean PRs). Both bugs predate PR #101 -- the same drift reproduces on `main`. They were never exercised by the chat-completeness work specifically, but they sit next to the escape-roundtrip code this PR added and break the same general "parse -> stringify -> parse stable" contract the rest of the PR pins. Fix 1 (list-item continuation indent): - `- item1\n para2` parsed to one list with one item containing one paragraph `"item1\npara2"`. Stringify emitted `* item1\npara2\n` -- no indent on `para2` -- so re-parse split into `[list, paragraph]`. - `_stringify_node`'s list branch now splits each child's stringified text on `\n` and applies the standard 2-space continuation indent to lines after the first. Empty continuation lines stay empty so intra-item blank lines round-trip too. - Affects every adapter that uses `from_markdown` / `to_markdown` to translate (linear, github, telegram, whatsapp + the generic `BaseFormatConverter.to_markdown`). Fix 2 (hard break stringify): - A `break` node (parsed from `Step \nline2`) stringified as just `\n`, which re-parses as a soft break -- collapsing the `[text, break, text]` triple into a single text leaf with embedded `\n`. The semantic distinction between hard and soft breaks was silently lost on round-trip. - `_stringify_node`'s break branch now emits `" \n"` per CommonMark. Tests added: - `test_roundtrip_preserves_list_item_continuation_indent` (4 inputs: single-line continuation, multi-line continuation, multiple items with continuations, ordered list with continuation) - `test_roundtrip_preserves_hard_break_in_paragraph` (asserts both AST stability AND that the `break` node survives) Docstring note added near `_INLINE_PATTERNS` for the third finding (O(n^2) on adversarial bracket-heavy input). Pre-existing in the underlying linear-search-per-position; this PR's alternation regex adds a constant-factor slowdown only on ~1000+ unclosed brackets, sub-millisecond on typical chat content. Deferred to a future character-level walker if a non-chat input surface ever lands. 3,643 pass / 1 pre-existing unrelated failure. --- src/chat_sdk/shared/markdown_parser.py | 25 ++++++++++++++++++--- tests/test_markdown_faithful.py | 31 ++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/src/chat_sdk/shared/markdown_parser.py b/src/chat_sdk/shared/markdown_parser.py index 570265e5..c933992c 100644 --- a/src/chat_sdk/shared/markdown_parser.py +++ b/src/chat_sdk/shared/markdown_parser.py @@ -302,6 +302,14 @@ def _restore_escapes_as_literal_pair(text: str) -> str: # that escaped ``\]`` / ``\[`` inside link text don't prematurely # terminate the match -- the alternation consumes a ``X`` # pair as a single unit. +# +# Performance note: the alternation makes the link / image content +# match O(n^2) on adversarial inputs with many unclosed ``[`` (e.g. +# 5000 unclosed brackets -> ~1-2s parse). Typical chat content has a +# handful of brackets so this is sub-millisecond in practice. If a +# future surface ever feeds untrusted markdown with adversarial +# bracket counts, switch to a character-level walker for link/image +# content (the rest of `_parse_inline` is bounded by message size). _INLINE_PATTERNS = [ # Images: ![alt](url) or ![alt](url "title") ("image", re.compile(r'(? return _escape_text_leaf(node.get("value", "")) if node_type == "break": - return "\n" + # Hard line break in CommonMark is two trailing spaces + newline. + # Emitting just `\n` would degrade to a soft break on re-parse. + return " \n" if node_type == "paragraph": children = node.get("children", []) @@ -932,11 +942,20 @@ def _stringify_node(node: Content, *, emphasis: str = "*", bullet: str = "*") -> lines.append(f" {nl}") else: text = _stringify_node(child, emphasis=emphasis, bullet=bullet) or "" + # A paragraph's stringified text may contain `\n` from + # soft breaks or hard breaks. Continuation lines must + # be indented by 2 spaces so the round-trip stays + # inside the list item rather than splitting back out + # to a sibling paragraph. + text_lines = text.split("\n") if not prefix_emitted: - lines.append(f"{prefix} {task_marker}{text}") + lines.append(f"{prefix} {task_marker}{text_lines[0]}") + for cont in text_lines[1:]: + lines.append(f" {cont}" if cont else "") prefix_emitted = True else: - lines.append(f" {text}") + for cont in text_lines: + lines.append(f" {cont}" if cont else "") return "\n".join(lines) if node_type == "listItem": diff --git a/tests/test_markdown_faithful.py b/tests/test_markdown_faithful.py index e1f62a3c..c1b326cf 100644 --- a/tests/test_markdown_faithful.py +++ b/tests/test_markdown_faithful.py @@ -529,6 +529,37 @@ def test_roundtrip_preserves_escaped_block_markers_after_soft_break(self): ast2 = parse_markdown(stringify_markdown(ast)) assert ast == ast2, f"soft-break drift on {src!r}" + def test_roundtrip_preserves_list_item_continuation_indent(self): + # PR #101 adversarial review: a multi-line list item like + # `- item1\n para2` (LLMs frequently emit wrapped bullets) + # parses to one list with one item containing one paragraph + # `item1\npara2`. The stringifier must indent the continuation + # line by 2 spaces or re-parse drops out of the list and + # produces `[list, paragraph]`. + for src in [ + "- item1\n para2", + "- a\n b\n c", + "- first wrapped\n second line\n- second item", + "1. ordered\n continuation", + ]: + ast = parse_markdown(src) + ast2 = parse_markdown(stringify_markdown(ast)) + assert ast == ast2, f"list-item continuation drift on {src!r}" + + def test_roundtrip_preserves_hard_break_in_paragraph(self): + # PR #101 adversarial review: a `break` node (hard line break) + # previously stringified to a bare `\n`, which re-parses as a + # soft break -- collapsing `[text, break, text]` into a single + # text leaf with embedded `\n`. Must emit `" \n"` per CommonMark. + ast = parse_markdown("Step \nline2") + ast2 = parse_markdown(stringify_markdown(ast)) + assert ast == ast2 + # And the children should still include a `break` node, not + # just a single merged text leaf. + para = ast2["children"][0] + types = [c.get("type") for c in para["children"]] + assert "break" in types + # ============================================================================ # toPlainText Tests