From ffac6be3262cfc1f604ed9ddf2b66fca36f62729 Mon Sep 17 00:00:00 2001 From: drew Date: Tue, 16 Jun 2026 17:35:57 -0400 Subject: [PATCH] fix(tui): count conversation separators for pruning --- .../tui_conversation_module_coverage_steps.py | 24 +++++++++ .../tui_conversation_module_coverage.feature | 9 ++++ robot/helper_tui_conversation_pruning.py | 39 +++++++++++++- robot/tui_conversation_pruning.robot | 11 ++++ src/cleveragents/tui/conversation.py | 54 ++++++++++++++++--- 5 files changed, 128 insertions(+), 9 deletions(-) diff --git a/features/steps/tui_conversation_module_coverage_steps.py b/features/steps/tui_conversation_module_coverage_steps.py index 80266c666..43374daf1 100644 --- a/features/steps/tui_conversation_module_coverage_steps.py +++ b/features/steps/tui_conversation_module_coverage_steps.py @@ -186,6 +186,30 @@ def step_assert_lines_bound(context, limit): ) +def _rendered_line_count(stream: ConversationStream) -> int: + rendered = stream.render() + if not rendered: + return 0 + return rendered.count("\n") + 1 + + +@then("the stream total_lines should equal the rendered line count") +def step_assert_total_matches_rendered_lines(context): + rendered_line_count = _rendered_line_count(context._conv_stream) + assert context._conv_stream.total_lines == rendered_line_count, ( + f"expected total_lines to match rendered line count {rendered_line_count}, " + f"got {context._conv_stream.total_lines}" + ) + + +@then("the stream rendered line count should be less than or equal to {limit:d}") +def step_assert_rendered_line_count_bound(context, limit): + rendered_line_count = _rendered_line_count(context._conv_stream) + assert rendered_line_count <= limit, ( + f"expected rendered line count <= {limit}, got {rendered_line_count}" + ) + + @then('the stream rendered text should contain "{text}"') def step_assert_render_contains(context, text): rendered = context._conv_stream.render() diff --git a/features/tui_conversation_module_coverage.feature b/features/tui_conversation_module_coverage.feature index 754d91a75..b02f5166c 100644 --- a/features/tui_conversation_module_coverage.feature +++ b/features/tui_conversation_module_coverage.feature @@ -89,6 +89,15 @@ Feature: TUI conversation module coverage Then the stream rendered text should contain "Protected welcome" And the stream total_lines should be less than or equal to 350 + @tdd_issue @tdd_issue_6350 + Scenario: ConversationStream pruning counts rendered separators between blocks + Given a fresh ConversationStream with low_mark 100 excess 50 preserve_recent_lines 0 + When I deliver 100 plain blocks of 1 lines each through add_block + Then the stream should have exactly 1 note block + And the stream rendered text should not contain "plain-1-line-1" + And the stream total_lines should equal the rendered line count + And the stream rendered line count should be less than or equal to 150 + @tdd_issue @tdd_issue_6350 Scenario: ConversationStream pruning updates the note in place rather than re-inserting Given a fresh ConversationStream with low_mark 200 excess 50 preserve_recent_lines 100 diff --git a/robot/helper_tui_conversation_pruning.py b/robot/helper_tui_conversation_pruning.py index 87d62c90e..427ed418d 100644 --- a/robot/helper_tui_conversation_pruning.py +++ b/robot/helper_tui_conversation_pruning.py @@ -40,6 +40,13 @@ def _multiline_block(n_lines: int, label: str = "line") -> str: return "\n".join(f"{label} {i}" for i in range(n_lines)) +def _rendered_line_count(stream: ConversationStream) -> int: + rendered = stream.render() + if not rendered: + return 0 + return rendered.count("\n") + 1 + + # --------------------------------------------------------------------------- # Subcommands # --------------------------------------------------------------------------- @@ -48,9 +55,9 @@ def _multiline_block(n_lines: int, label: str = "line") -> str: def cmd_prune_trigger() -> None: """Verify pruning fires and total_lines returns to <= prune_low_mark.""" stream = _make_stream(prune_low_mark=100, prune_excess=50) - # trigger_line_count = 150; push 210 lines to trigger pruning twice, + # trigger_line_count = 150; push enough rendered lines to trigger pruning twice, # ensuring total_lines ends at prune_low_mark or below. - for i in range(21): + for i in range(24): stream.add_block(_multiline_block(10, f"msg{i}"), block_type="message") assert stream.total_lines <= 100, ( @@ -59,6 +66,33 @@ def cmd_prune_trigger() -> None: print("tui-prune-trigger-ok") +def cmd_separator_aware_prune() -> None: + """Verify rendered inter-block separators count toward pruning thresholds.""" + stream = _make_stream( + prune_low_mark=100, + prune_excess=50, + preserve_recent_lines=0, + ) + + for i in range(100): + stream.add_block(f"one-line-{i}", block_type="message") + + rendered = stream.render() + assert stream.total_lines == _rendered_line_count(stream), ( + "Expected total_lines to match rendered line count, " + f"got total_lines={stream.total_lines}, rendered={_rendered_line_count(stream)}" + ) + assert stream.total_lines <= 150, ( + "Expected rendered lines to stay within trigger threshold, " + f"got {stream.total_lines}" + ) + assert "one-line-0" not in rendered, "Expected oldest one-line block to be pruned" + assert any(block.block_type == "note" for block in stream.blocks), ( + "Expected pruning note after separator-aware pruning" + ) + print("tui-separator-aware-prune-ok") + + def cmd_prune_note_inserted() -> None: """Verify a note block is at index 0 with the expected text after pruning.""" stream = _make_stream(prune_low_mark=100, prune_excess=50) @@ -140,6 +174,7 @@ def cmd_settings_defaults() -> None: COMMANDS = { "prune-trigger": cmd_prune_trigger, + "separator-aware-prune": cmd_separator_aware_prune, "prune-note-inserted": cmd_prune_note_inserted, "prune-protected": cmd_prune_protected, "clear-resets-state": cmd_clear_resets_state, diff --git a/robot/tui_conversation_pruning.robot b/robot/tui_conversation_pruning.robot index 75f8e93d3..27fd35f1d 100644 --- a/robot/tui_conversation_pruning.robot +++ b/robot/tui_conversation_pruning.robot @@ -27,6 +27,17 @@ ConversationStream Prunes Oldest Blocks When Threshold Exceeded Should Be Equal As Integers ${result.rc} 0 Should Contain ${result.stdout} tui-prune-trigger-ok +ConversationStream Counts Rendered Separators When Pruning + [Documentation] Verify many one-line blocks prune based on rendered blank + ... separators, not only block-local line counts. + [Tags] tui_conversation_pruning tdd_issue tdd_issue_6350 + ${result}= Run Process ${PYTHON} ${HELPER} separator-aware-prune + ... cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} tui-separator-aware-prune-ok + ConversationStream Inserts Note Block After Pruning [Documentation] Verify a pruned-note block is inserted at index 0 after pruning. [Tags] tui_conversation_pruning tdd_issue tdd_issue_6350 diff --git a/src/cleveragents/tui/conversation.py b/src/cleveragents/tui/conversation.py index 9bfa00070..2008a2290 100644 --- a/src/cleveragents/tui/conversation.py +++ b/src/cleveragents/tui/conversation.py @@ -83,6 +83,7 @@ class ConversationStream: self._settings = settings or ConversationSettings() self._blocks: list[ConversationBlock] = [] self._total_lines = 0 + self._visible_blocks = 0 # Invariant: when True, _blocks[0] is a ConversationBlock with # block_type == "note". Maintained by _insert_pruned_note(). self._note_active = False @@ -105,6 +106,7 @@ class ConversationStream: self._blocks = [ ConversationBlock(text=welcome_text, block_type="welcome", protected=True) ] + self._visible_blocks = 1 if welcome_text else 0 self._total_lines = self._blocks[0].line_count() self._note_active = False @@ -113,6 +115,7 @@ class ConversationStream: self._blocks.clear() self._total_lines = 0 + self._visible_blocks = 0 self._note_active = False def extend(self, blocks: Iterable[ConversationBlock]) -> None: @@ -143,7 +146,7 @@ class ConversationStream: markup=markup, ) self._blocks.append(block) - self._total_lines += block.line_count() + self._add_block_lines(block) self._prune_if_needed() def render(self) -> str: @@ -165,6 +168,15 @@ class ConversationStream: if self._total_lines <= self._settings.trigger_line_count: return + pruned = self._prune_oldest_blocks_to_low_mark() + + if pruned: + self._insert_pruned_note() + self._prune_oldest_blocks_to_low_mark() + + def _prune_oldest_blocks_to_low_mark(self) -> bool: + """Remove oldest eligible blocks until rendered lines reach the low mark.""" + preserve_start = self._determine_preserve_start() pruned = False index = 0 @@ -175,13 +187,11 @@ class ConversationStream: if block.protected: index += 1 continue - self._total_lines -= block.line_count() + self._remove_block_lines(block) self._blocks.pop(index) preserve_start -= 1 pruned = True - - if pruned: - self._insert_pruned_note() + return pruned def _determine_preserve_start(self) -> int: """Return the index from which blocks must be preserved.""" @@ -190,9 +200,15 @@ class ConversationStream: return len(self._blocks) preserved_lines = 0 + preserved_visible_blocks = 0 index = len(self._blocks) - 1 while index >= 0 and preserved_lines < self._settings.preserve_recent_lines: - preserved_lines += self._blocks[index].line_count() + block = self._blocks[index] + if block.text: + preserved_lines += block.line_count() + if preserved_visible_blocks: + preserved_lines += 1 + preserved_visible_blocks += 1 index -= 1 return max(0, index + 1) @@ -204,8 +220,10 @@ class ConversationStream: if self._note_active and self._blocks: first = self._blocks[0] if first.block_type == "note": + previous_line_count = first.line_count() first.text = note_markup first.markup = True + self._total_lines += first.line_count() - previous_line_count return note_block = ConversationBlock( @@ -215,9 +233,31 @@ class ConversationStream: markup=True, ) self._blocks.insert(0, note_block) - self._total_lines += note_block.line_count() + self._add_block_lines(note_block) self._note_active = True + def _add_block_lines(self, block: ConversationBlock) -> None: + """Add a block's rendered line contribution to the running total.""" + + if not block.text: + return + + self._total_lines += block.line_count() + if self._visible_blocks: + self._total_lines += 1 + self._visible_blocks += 1 + + def _remove_block_lines(self, block: ConversationBlock) -> None: + """Remove a block's rendered line contribution from the running total.""" + + if not block.text: + return + + self._total_lines -= block.line_count() + if self._visible_blocks > 1: + self._total_lines -= 1 + self._visible_blocks -= 1 + def load_conversation_settings(config_path: Path | None = None) -> ConversationSettings: """Load pruning settings from the TUI settings file if available.