Studio: simplify tool-call dedup and replace html2text with builtin converter #4722
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,295 @@ | ||
| # SPDX-License-Identifier: AGPL-3.0-only | ||
| # Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0 | ||
|
|
||
| """ | ||
| Minimal HTML-to-Markdown converter using only the standard library. | ||
|
|
||
| Replaces the external ``html2text`` (GPL-3.0) dependency with a ~180-line | ||
| ``html.parser.HTMLParser`` subclass. Covers headings, links, bold/italic, | ||
| lists, tables, blockquotes, code blocks, and entity decoding. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import html | ||
| import re | ||
| from html.parser import HTMLParser | ||
|
|
||
| __all__ = ["html_to_markdown"] | ||
|
|
||
| _SKIP_TAGS = frozenset({"script", "style", "head", "noscript", "svg", "math"}) | ||
| _BLOCK_TAGS = frozenset( | ||
| { | ||
| "p", | ||
| "div", | ||
| "section", | ||
| "article", | ||
| "header", | ||
| "footer", | ||
| "main", | ||
| "aside", | ||
| "nav", | ||
| "figure", | ||
| "figcaption", | ||
| "details", | ||
| "summary", | ||
| } | ||
| ) | ||
| _HEADING_TAGS = frozenset({"h1", "h2", "h3", "h4", "h5", "h6"}) | ||
| _INLINE_EMPHASIS = {"strong": "**", "b": "**", "em": "*", "i": "*"} | ||
|
|
||
|
|
||
| class _MarkdownRenderer(HTMLParser): | ||
| """HTMLParser subclass that emits Markdown tokens into a list.""" | ||
|
|
||
| def __init__(self): | ||
| super().__init__(convert_charrefs = False) | ||
| self._out: list[str] = [] | ||
| self._skip_depth: int = 0 | ||
|
|
||
| # Link state | ||
| self._link_href: str | None = None | ||
| self._link_text_parts: list[str] = [] | ||
| self._in_link: bool = False | ||
|
|
||
| # List state | ||
| self._list_stack: list[str] = [] # "ul" or "ol" | ||
| self._ol_counter: list[int] = [] | ||
|
|
||
| # Table state | ||
| self._in_table: bool = False | ||
| self._current_row: list[str] = [] | ||
| self._cell_parts: list[str] = [] | ||
| self._in_cell: bool = False | ||
| self._header_row_done: bool = False | ||
| self._row_has_th: bool = False | ||
|
|
||
| # Pre/code state | ||
| self._in_pre: bool = False | ||
| self._pre_parts: list[str] = [] | ||
|
|
||
| # Blockquote depth | ||
| self._bq_depth: int = 0 | ||
|
|
||
| # ------------------------------------------------------------------ | ||
| def _emit(self, text: str) -> None: | ||
| if self._in_link: | ||
| self._link_text_parts.append(text) | ||
| elif self._in_cell: | ||
| self._cell_parts.append(text) | ||
| elif self._in_pre: | ||
| self._pre_parts.append(text) | ||
| else: | ||
| # Prefix newlines with blockquote markers when inside <blockquote> | ||
| if self._bq_depth and "\n" in text: | ||
| prefix = "> " * self._bq_depth | ||
| text = text.replace("\n", "\n" + prefix) | ||
| self._out.append(text) | ||
|
|
||
| # ------------------------------------------------------------------ | ||
| # Tag handlers | ||
| # ------------------------------------------------------------------ | ||
| def handle_starttag(self, tag: str, attrs: list[tuple[str, str | None]]) -> None: | ||
| tag = tag.lower() | ||
|
|
||
| if tag in _SKIP_TAGS: | ||
| self._skip_depth += 1 | ||
| return | ||
| if self._skip_depth: | ||
| return | ||
|
|
||
| attr_dict = dict(attrs) | ||
|
|
||
| if tag in _HEADING_TAGS: | ||
| level = int(tag[1]) | ||
| self._emit("\n\n" + "#" * level + " ") | ||
|
|
||
| elif tag == "a": | ||
| self._link_href = attr_dict.get("href") | ||
| self._link_text_parts = [] | ||
| self._in_link = True | ||
|
|
||
| elif tag in _INLINE_EMPHASIS: | ||
| self._emit(_INLINE_EMPHASIS[tag]) | ||
|
|
||
| elif tag == "br": | ||
| self._emit("\n") | ||
|
|
||
| elif tag in _BLOCK_TAGS: | ||
| self._emit("\n\n") | ||
|
|
||
| elif tag == "hr": | ||
| self._emit("\n\n---\n\n") | ||
|
|
||
| elif tag == "blockquote": | ||
| self._bq_depth += 1 | ||
| self._emit("\n\n" + "> " * self._bq_depth) | ||
|
Contributor
There was a problem hiding this comment. Markdown blockquotes require the > prefix on every line of the quoted content. The current implementation only emits the prefix once at the start of the blockquote. For multi-line or multi-paragraph content within a blockquote, subsequent lines will lack the necessary prefix, breaking the formatting. |
||
|
|
||
| elif tag == "ul": | ||
| self._list_stack.append("ul") | ||
| self._emit("\n") | ||
|
|
||
| elif tag == "ol": | ||
| self._list_stack.append("ol") | ||
| self._ol_counter.append(0) | ||
| self._emit("\n") | ||
|
|
||
| elif tag == "li": | ||
| indent = " " * max(0, len(self._list_stack) - 1) | ||
| if self._list_stack and self._list_stack[-1] == "ol": | ||
| self._ol_counter[-1] += 1 | ||
| self._emit(f"\n{indent}{self._ol_counter[-1]}. ") | ||
| else: | ||
| self._emit(f"\n{indent}* ") | ||
|
|
||
| elif tag == "pre": | ||
| self._in_pre = True | ||
| self._pre_parts = [] | ||
| self._emit("\n\n```\n") | ||
|
|
||
| elif tag == "code" and not self._in_pre: | ||
| self._emit("`") | ||
|
|
||
| elif tag == "table": | ||
| self._in_table = True | ||
| self._header_row_done = False | ||
| self._emit("\n\n") | ||
|
|
||
| elif tag == "tr": | ||
| self._current_row = [] | ||
|
|
||
| elif tag in ("th", "td"): | ||
| self._cell_parts = [] | ||
| self._in_cell = True | ||
| if tag == "th": | ||
| self._row_has_th = True | ||
|
|
||
| elif tag == "img": | ||
| alt = attr_dict.get("alt", "") | ||
| src = attr_dict.get("src", "") | ||
| if alt or src: | ||
| self._emit(f"") | ||
|
|
||
| def handle_endtag(self, tag: str) -> None: | ||
| tag = tag.lower() | ||
|
|
||
| if tag in _SKIP_TAGS: | ||
| self._skip_depth = max(0, self._skip_depth - 1) | ||
| return | ||
| if self._skip_depth: | ||
| return | ||
|
|
||
| if tag in _HEADING_TAGS: | ||
| self._emit("\n\n") | ||
|
|
||
| elif tag == "a": | ||
| text = "".join(self._link_text_parts).strip() | ||
| href = self._link_href or "" | ||
| self._in_link = False | ||
| if href and text: | ||
| self._emit(f"[{text}]({href})") | ||
| elif text: | ||
| self._emit(text) | ||
|
|
||
| elif tag in _INLINE_EMPHASIS: | ||
| self._emit(_INLINE_EMPHASIS[tag]) | ||
|
|
||
| elif tag in _BLOCK_TAGS: | ||
| self._emit("\n\n") | ||
|
|
||
| elif tag == "blockquote": | ||
| self._bq_depth = max(0, self._bq_depth - 1) | ||
| self._emit("\n\n") | ||
|
|
||
| elif tag == "ul": | ||
| if self._list_stack and self._list_stack[-1] == "ul": | ||
| self._list_stack.pop() | ||
| self._emit("\n") | ||
|
|
||
| elif tag == "ol": | ||
| if self._list_stack and self._list_stack[-1] == "ol": | ||
| self._list_stack.pop() | ||
| if self._ol_counter: | ||
| self._ol_counter.pop() | ||
| self._emit("\n") | ||
|
|
||
| elif tag == "pre": | ||
| raw = "".join(self._pre_parts) | ||
| self._out.append(raw) | ||
| self._in_pre = False | ||
| self._emit("\n```\n\n") | ||
|
|
||
| elif tag == "code" and not self._in_pre: | ||
| self._emit("`") | ||
|
|
||
| elif tag in ("th", "td"): | ||
| self._in_cell = False | ||
| cell_text = "".join(self._cell_parts).strip().replace("\n", " ") | ||
| self._current_row.append(cell_text) | ||
|
|
||
| elif tag == "tr": | ||
| if self._current_row: | ||
| line = "| " + " | ".join(self._current_row) + " |" | ||
| self._emit(line + "\n") | ||
| if self._row_has_th and not self._header_row_done: | ||
| sep = "| " + " | ".join("---" for _ in self._current_row) + " |" | ||
| self._emit(sep + "\n") | ||
| self._header_row_done = True | ||
| self._current_row = [] | ||
| self._row_has_th = False | ||
|
|
||
| elif tag == "table": | ||
| self._in_table = False | ||
| self._emit("\n") | ||
|
|
||
| # ------------------------------------------------------------------ | ||
| # Text / entity handlers | ||
| # ------------------------------------------------------------------ | ||
| def handle_data(self, data: str) -> None: | ||
| if self._skip_depth: | ||
| return | ||
| if self._in_pre: | ||
| self._pre_parts.append(data) | ||
| return | ||
| # Collapse whitespace for non-pre content | ||
| text = re.sub(r"[ \t]+", " ", data) | ||
| self._emit(text) | ||
|
|
||
| def handle_entityref(self, name: str) -> None: | ||
| if self._skip_depth: | ||
| return | ||
| self._emit(html.unescape(f"&{name};")) | ||
|
|
||
| def handle_charref(self, name: str) -> None: | ||
| if self._skip_depth: | ||
| return | ||
| self._emit(html.unescape(f"&#{name};")) | ||
|
|
||
|
|
||
| # ------------------------------------------------------------------ | ||
| # Post-processing | ||
| # ------------------------------------------------------------------ | ||
| def _cleanup(text: str) -> str: | ||
| """Normalize whitespace and blank lines in the final output.""" | ||
| # Collapse runs of 3+ newlines into 2 | ||
| text = re.sub(r"\n{3,}", "\n\n", text) | ||
| # Remove trailing spaces on each line | ||
| text = re.sub(r"[ \t]+$", "", text, flags = re.MULTILINE) | ||
| return text.strip() | ||
|
|
||
|
|
||
| # ------------------------------------------------------------------ | ||
| # Public API | ||
| # ------------------------------------------------------------------ | ||
| def html_to_markdown(source_html: str) -> str: | ||
| """Convert an HTML string to Markdown. | ||
|
|
||
| Handles headings, links, bold/italic, lists (ordered and unordered), | ||
| tables, blockquotes, code blocks, and HTML entities. ``<script>``, | ||
| ``<style>``, and ``<head>`` sections are stripped entirely. | ||
| """ | ||
| renderer = _MarkdownRenderer() | ||
| renderer.feed(source_html) | ||
| renderer.close() | ||
| raw = "".join(renderer._out) | ||
|
Comment on lines
+436
to
+438
There was a problem hiding this comment.
Useful? React with 👍 / 👎. |
||
| return _cleanup(raw) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,6 @@ | |
|
|
||
| import atexit | ||
| import contextlib | ||
| import json | ||
| import re | ||
| import struct | ||
|
|
@@ -2181,22 +2180,6 @@ def _strip_tool_markup(text: str, *, final: bool = False) -> str: | |
| # identical call succeeded). | ||
| _tool_call_history: list[tuple[str, bool]] = [] # (key, failed) | ||
|
|
||
| for iteration in range(max_tool_iterations): | ||
| if cancel_event is not None and cancel_event.is_set(): | ||
| return | ||
|
|
@@ -2692,7 +2675,9 @@ def _record_tool_call(name: str, args: dict, failed: bool) -> None: | |
| } | ||
|
|
||
| # ── Duplicate call detection ────────────── | ||
| _tc_key = tool_name + str(arguments) | ||
|
There was a problem hiding this comment.
Building Useful? React with 👍 / 👎.
Contributor
There was a problem hiding this comment. The change from json.dumps(..., sort_keys=True) to str(arguments) introduces a regression in robustness. While Python 3.7+ dictionaries preserve insertion order, str(dict) is sensitive to that order. If the model generates the same tool arguments but with keys in a different order, they will be semantically identical but produce different strings, causing the duplicate detection to fail. I recommend using json.dumps with sort_keys=True for the key generation. _tc_key = tool_name + json.dumps(arguments, sort_keys=True)
Member
Author
There was a problem hiding this comment. Good catch -- switched to
Member
Author
There was a problem hiding this comment. Reverted back to |
||
| _prev = _tool_call_history[-1] if _tool_call_history else None | ||
| if _prev and _prev[0] == _tc_key and not _prev[1]: | ||
|
There was a problem hiding this comment.
Building the dedup key with Useful? React with 👍 / 👎. |
||
| result = ( | ||
| "You already made this exact call. " | ||
| "Do not repeat the same tool call. " | ||
|
|
@@ -2734,7 +2719,7 @@ def _record_tool_call(name: str, args: dict, failed: bool) -> None: | |
| _is_error = isinstance(result, str) and result.lstrip().startswith( | ||
| _error_prefixes | ||
| ) | ||
| _tool_call_history.append((_tc_key, _is_error)) | ||
| _result_content = result | ||
| if _is_error: | ||
| _result_content = ( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
</head>is omittedhtml_to_markdown()treats<head>as a skip-depth region, but HTML end tags forheadare optional; many pages validly switch to<body>without emitting</head>. In this branch,_skip_depthis only decremented on explicit end tags, so once<head>is seen, all subsequent body tags/data are ignored and_fetch_page_text()can return(page returned no readable text)for otherwise normal documents. This is a functional regression in page extraction for common HTML5 markup.Useful? React with 👍 / 👎.