From 4fc17941da6cedd069d596a8ce6f211cb0cbfcd2 Mon Sep 17 00:00:00 2001 From: seanturner83 Date: Fri, 17 Jul 2026 08:50:46 +0100 Subject: [PATCH 1/2] fix(tui): don't double-render finish_scan section headings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The finish_scan renderer prints a styled section label ("Executive Summary", "Methodology", etc.) above each field's value. But the finish_scan tool prompts the model to "use markdown in every field" and its own example opens each field with a `#
` heading — so the models routinely start executive_summary with `# Executive Summary`, and the heading renders twice (styled label + the markdown heading in the body). Strip a leading markdown heading from a field value when it just repeats that field's section label (case-insensitive), so the label appears once. A leading heading that says something else, a value with no heading, and a lone heading with no body are all left untouched. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../tui/renderers/finish_renderer.py | 26 ++++++-- tests/test_finish_renderer.py | 65 +++++++++++++++++++ 2 files changed, 87 insertions(+), 4 deletions(-) create mode 100644 tests/test_finish_renderer.py diff --git a/strix/interface/tui/renderers/finish_renderer.py b/strix/interface/tui/renderers/finish_renderer.py index 62c21288d..3c7d03ecf 100644 --- a/strix/interface/tui/renderers/finish_renderer.py +++ b/strix/interface/tui/renderers/finish_renderer.py @@ -1,3 +1,4 @@ +import re from typing import Any, ClassVar from rich.text import Text @@ -10,6 +11,23 @@ FIELD_STYLE = "bold #4ade80" +def _strip_leading_heading(value: str, section: str) -> str: + """Drop a leading markdown heading that just repeats the section label. + + The finish_scan tool prompts the model to write markdown in every field, and + the models routinely open each with a ``#
`` heading (e.g. + ``# Executive Summary``). This renderer also prints its own styled section + label above the value, so that heading renders twice. Strip a leading + ``#``-heading whose text matches this section (case-insensitively) so the + label isn't duplicated; leave all other content — including headings that + say something else — untouched. + """ + stripped = value.lstrip() + pattern = rf"^#{{1,6}}\s+{re.escape(section)}\s*\n+" + m = re.match(pattern, stripped, flags=re.IGNORECASE) + return stripped[m.end() :] if m else value + + @register_tool_renderer class FinishScanRenderer(BaseToolRenderer): tool_name: ClassVar[str] = "finish_scan" @@ -32,25 +50,25 @@ def render(cls, tool_data: dict[str, Any]) -> Static: text.append("\n\n") text.append("Executive Summary", style=FIELD_STYLE) text.append("\n") - text.append(executive_summary) + text.append(_strip_leading_heading(executive_summary, "Executive Summary")) if methodology: text.append("\n\n") text.append("Methodology", style=FIELD_STYLE) text.append("\n") - text.append(methodology) + text.append(_strip_leading_heading(methodology, "Methodology")) if technical_analysis: text.append("\n\n") text.append("Technical Analysis", style=FIELD_STYLE) text.append("\n") - text.append(technical_analysis) + text.append(_strip_leading_heading(technical_analysis, "Technical Analysis")) if recommendations: text.append("\n\n") text.append("Recommendations", style=FIELD_STYLE) text.append("\n") - text.append(recommendations) + text.append(_strip_leading_heading(recommendations, "Recommendations")) if not (executive_summary or methodology or technical_analysis or recommendations): text.append("\n ") diff --git a/tests/test_finish_renderer.py b/tests/test_finish_renderer.py new file mode 100644 index 000000000..9eece5203 --- /dev/null +++ b/tests/test_finish_renderer.py @@ -0,0 +1,65 @@ +"""Tests for the finish_scan TUI renderer (section-heading de-duplication).""" + +from __future__ import annotations + +from rich.text import Text + +from strix.interface.tui.renderers.finish_renderer import ( + FinishScanRenderer, + _strip_leading_heading, +) + + +def _plain(static: object) -> str: + content = static.content # type: ignore[attr-defined] + return content.plain if isinstance(content, Text) else str(content) + + +def _render(**args: str) -> str: + return _plain(FinishScanRenderer.render({"status": "completed", "args": args})) + + +# --- _strip_leading_heading ------------------------------------------------- + + +def test_strips_matching_leading_heading() -> None: + assert _strip_leading_heading("# Executive Summary\n\nBody", "Executive Summary") == "Body" + assert _strip_leading_heading("## Methodology\nSteps", "Methodology") == "Steps" + + +def test_strip_is_case_and_whitespace_insensitive() -> None: + assert _strip_leading_heading("# technical analysis \n\nX", "Technical Analysis") == "X" + + +def test_keeps_a_different_leading_heading() -> None: + # A heading that isn't the section label is real content — leave it. + val = "# Findings Overview\nY" + assert _strip_leading_heading(val, "Executive Summary") == val + + +def test_keeps_body_when_no_heading() -> None: + assert _strip_leading_heading("No heading here", "Methodology") == "No heading here" + + +def test_keeps_lone_heading_with_no_body() -> None: + # No trailing newline => not a section split; don't strip to empty. + assert _strip_leading_heading("# Recommendations", "Recommendations") == "# Recommendations" + + +# --- end-to-end render ------------------------------------------------------ + + +def test_section_heading_rendered_once_not_twice() -> None: + # The model is prompted to write markdown and routinely opens each field + # with a `#
` heading; the renderer also prints a styled label. + # Regression: both showed, doubling the heading. Now the field's leading + # heading is stripped so "Executive Summary" appears exactly once. + out = _render(executive_summary="# Executive Summary\n\nThe app is sound.") + assert out.count("Executive Summary") == 1 + assert "The app is sound." in out + + +def test_render_preserves_body_without_heading() -> None: + out = _render(methodology="White-box review of the diff.") + assert "Methodology" in out + assert "White-box review of the diff." in out From 7091b7be063138ca9481872cccc8ba48533201fe Mon Sep 17 00:00:00 2001 From: seanturner83 Date: Fri, 17 Jul 2026 09:18:07 +0100 Subject: [PATCH 2/2] review: don't blank a heading-only field; match closing-ATX headings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two Greptile findings: - P1: a field that is ONLY the heading + a trailing newline (e.g. "# Recommendations\n") matched the strip pattern and returned empty, hiding the field's sole content — contradicting the "keep a lone heading" intent. Now: if stripping leaves nothing non-whitespace, keep the original value. - P2: the closed-ATX form "# Executive Summary #" didn't match, so its duplicate heading still rendered. Pattern now accepts an optional closing "#+" run. +2 tests (heading-only-with-newline kept; closing-ATX stripped). Co-Authored-By: Claude Opus 4.8 (1M context) --- strix/interface/tui/renderers/finish_renderer.py | 12 +++++++++--- tests/test_finish_renderer.py | 16 ++++++++++++++-- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/strix/interface/tui/renderers/finish_renderer.py b/strix/interface/tui/renderers/finish_renderer.py index 3c7d03ecf..d2f868703 100644 --- a/strix/interface/tui/renderers/finish_renderer.py +++ b/strix/interface/tui/renderers/finish_renderer.py @@ -20,12 +20,18 @@ def _strip_leading_heading(value: str, section: str) -> str: label above the value, so that heading renders twice. Strip a leading ``#``-heading whose text matches this section (case-insensitively) so the label isn't duplicated; leave all other content — including headings that - say something else — untouched. + say something else — untouched. Also matches the closing-``#`` ATX form + (``# Executive Summary #``). If stripping the heading would leave nothing + (the field was ONLY the heading, e.g. ``# Recommendations\n``), keep the + original rather than render the field's sole content as blank. """ stripped = value.lstrip() - pattern = rf"^#{{1,6}}\s+{re.escape(section)}\s*\n+" + pattern = rf"^#{{1,6}}\s+{re.escape(section)}(?:\s+#+)?\s*\n+" m = re.match(pattern, stripped, flags=re.IGNORECASE) - return stripped[m.end() :] if m else value + if not m: + return value + remainder = stripped[m.end() :] + return remainder if remainder.strip() else value @register_tool_renderer diff --git a/tests/test_finish_renderer.py b/tests/test_finish_renderer.py index 9eece5203..28a2e5dae 100644 --- a/tests/test_finish_renderer.py +++ b/tests/test_finish_renderer.py @@ -41,11 +41,23 @@ def test_keeps_body_when_no_heading() -> None: assert _strip_leading_heading("No heading here", "Methodology") == "No heading here" -def test_keeps_lone_heading_with_no_body() -> None: - # No trailing newline => not a section split; don't strip to empty. +def test_keeps_lone_heading_no_trailing_newline() -> None: + # No trailing newline => doesn't match the pattern; kept. assert _strip_leading_heading("# Recommendations", "Recommendations") == "# Recommendations" +def test_keeps_lone_heading_with_trailing_newline() -> None: + # Trailing newline DOES match, but stripping leaves nothing — the field's + # only content would vanish. Keep the original (Greptile P1). + assert _strip_leading_heading("# Recommendations\n", "Recommendations") == "# Recommendations\n" + assert _strip_leading_heading("## Methodology\n\n", "Methodology") == "## Methodology\n\n" + + +def test_strips_closing_atx_heading() -> None: + # Closed-ATX form `# Section #` must also dedupe (Greptile P2). + assert _strip_leading_heading("# Executive Summary #\n\nBody", "Executive Summary") == "Body" + + # --- end-to-end render ------------------------------------------------------