2434253c1a
CI / build (push) Successful in 16s
CI / lint (push) Successful in 18s
CI / quality (push) Successful in 28s
CI / typecheck (push) Successful in 1m0s
CI / security (push) Successful in 1m0s
CI / benchmark-regression (push) Has been skipped
CI / unit_tests (push) Successful in 3m14s
CI / integration_tests (push) Successful in 3m38s
CI / docker (push) Successful in 55s
CI / e2e_tests (push) Successful in 5m15s
CI / coverage (push) Successful in 7m8s
CI / benchmark-publish (push) Successful in 20m34s
## Summary Implement the 6 missing element handle types (Tree, Text, Code, Diff, Separator, ActionHint) and ensure all 6 materialization strategies (rich, color, table, plain, json, yaml) support all 10 element types. This completes the output rendering framework per the specification (§25417-27276). Closes #550 ## Changes ### Source (restructured into smaller modules) #### `handles/` package (was `handles.py` — 1062 lines → 5 files, all ≤500 lines) - **`_models.py`**: All Pydantic data models, constants (MAX_TREE_DEPTH, MAX_ELEMENTS_PER_SESSION, MAX_TABLE_ROWS), exceptions (ElementClosedError), event classes, ElementSnapshot union. P2-1: `DiffLine.type` renamed to `line_type` with backward-compatible alias. - **`_base.py`**: Generic `ElementHandle[E]` base class with thread-safe `element_copy()` (lock-protected). - **`_panel_table.py`**: PanelHandle, TableHandle, StatusHandle. - **`_concrete.py`**: ProgressHandle (P1-1: validates `total` non-negative), TreeHandle, TextHandle (P3-2: close() delegates to super()), CodeHandle, DiffHandle, SeparatorHandle, ActionHintHandle. P3-8: `increment(delta=0)` now rejected. - **`__init__.py`**: Re-exports all public symbols. #### `_renderers.py` — plain renderers, sanitization, and shared helpers - Plain render functions for all 10 element types. - Terminal escape sanitization (`strip_terminal_escapes`). - P2-3: `_sort_table_rows` now consults `ColumnDef.col_type` for numeric sorting. - P2-6: `compute_column_widths()` shared helper extracted (DRY fix). #### `_color_renderers.py` — ANSI colour renderers - Colour-coded render functions for all 10 element types. - P2-2: TextBlock now gets color treatment (`_render_text_color`) instead of falling through to plain. #### `_boxdraw.py` — box-drawing renderers - P2-5: Upgraded from ASCII `+-|` to Unicode box-drawing `╭─╮│╰╯` with rounded corners per spec §26821. #### `_ids.py` — ID generation helpers - P3-1: Session and handle IDs now use separate counters, making IDs monotonic within their namespace. #### `materializers.py` — strategy protocol and 6 concrete strategies - P1-2: `_snapshot_to_dict` now includes `timing` field in JSON/YAML output per spec §27022. - P2-4: `_column_def_to_dict` always includes all fields unconditionally for stable JSON schemas. #### `selection.py` — materializer selection with fallback - P1-4: `NO_COLOR` environment variable now respected (https://no-color.org/). When set, all visual formats fall back to plain. Precedence: explicit flag > NO_COLOR > terminal capability fallback. #### `session.py` - P1-1: `session.progress()` factory validates `total >= 0`. - P1-2: `snapshot()` includes `timing` when available. #### `__init__.py` — package docstring - P1-3: SD-29 corrected to reflect actual Table → Color → Plain fallback chain. - SD-14 marked as implemented (NO_COLOR support added). ### Spec Deviations (Documented) 28 deliberate deviations documented in `__init__.py` module docstring (SD-1 through SD-29, with SD-14 now implemented). SD-29 corrected. ### Tests (updated) - **+23 new BDD scenarios** covering: P1-1 total validation, P1-4 NO_COLOR, P2-1 DiffLine.line_type alias, P2-2 text color, P2-3 numeric sorting, P2-4 ColumnDef serialization, P2-5 Unicode box-drawing, P3-3 add_rows limit, P3-5 10-thread stress test, P3-6 summary truncation, P3-7 explicit format for color/table, P3-8 zero delta. - **Robot tests**: Updated box-drawing assertion for Unicode chars. ## Verification | Check | Result | |-------|--------| | Pyright | 0 errors, 1 pre-existing warning | | Ruff lint | All passed | | Unit tests | 393 features, 11,344 scenarios, 0 failures | | Integration tests | All passed | | E2E tests | All passed | | Coverage | 97% overall (threshold: 97%) | ## Review Fixes Applied (Luis Review #2412) | ID | Severity | Fix | |----|----------|-----| | P1-1 | High | `set_progress()` and `session.progress()` validate `total >= 0` | | P1-2 | High | `_snapshot_to_dict` includes `timing` field | | P1-3 | High | SD-29 documentation corrected | | P1-4 | High | `NO_COLOR` env var respected | | P2-1 | Medium | `DiffLine.type` → `line_type` with alias | | P2-2 | Medium | TextBlock gets color treatment | | P2-3 | Medium | Numeric column sorting | | P2-4 | Medium | ColumnDef always serializes all fields | | P2-5 | Medium | Unicode box-drawing characters | | P2-6 | Medium | Shared `compute_column_widths()` helper | | P3-1 | Low | Separate ID counters | | P3-2 | Low | TextHandle.close() delegates to super() | | P3-3 | Low | add_rows batch limit test | | P3-5 | Low | 10-thread stress test | | P3-6 | Low | Summary truncation test | | P3-7 | Low | Explicit format tests for color/table | | P3-8 | Low | Zero delta rejected | ### Deferred Items | ID | Reason | |----|--------| | P3-9 | snapshot() lock scope — acceptable correctness trade-off | | P3-10 | CLEVERAGENTS_FORMAT env var — documented as SD-15, requires CLI framework changes | Reviewed-on: #812 Reviewed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com> Co-authored-by: Rui Hu <rui.hu@cleverthis.com> Co-committed-by: Rui Hu <rui.hu@cleverthis.com>
441 lines
16 KiB
Python
441 lines
16 KiB
Python
"""Step definitions for materializers_coverage_boost.feature.
|
|
|
|
These steps target uncovered lines in materializers.py, specifically:
|
|
- Lines 466-469: on_session_end force-flush of remaining buffered elements
|
|
- Line 169: _render_element_plain fallback for unknown element types
|
|
- Line 269: _render_element_color fallback for unknown element types
|
|
- Line 372: _element_to_dict fallback for unknown element types
|
|
- Line 440: _BaseBufferStrategy._render_element default
|
|
- Line 445: on_element_closed early return for unknown handle_id
|
|
- Line 551: _AccumulateStrategy._serialise default
|
|
- Line 576: _AccumulateStrategy._serialise_dict default
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import re
|
|
|
|
from behave import given, then, when
|
|
from behave.runner import Context
|
|
|
|
from cleveragents.cli.output._color_renderers import (
|
|
render_element_color as _render_element_color,
|
|
)
|
|
from cleveragents.cli.output._renderers import (
|
|
render_element_plain as _render_element_plain,
|
|
)
|
|
from cleveragents.cli.output.handles import (
|
|
ElementClosed,
|
|
Panel,
|
|
PanelEntry,
|
|
SessionEnd,
|
|
)
|
|
from cleveragents.cli.output.materializers import (
|
|
ColorMaterializer,
|
|
PlainMaterializer,
|
|
RichMaterializer,
|
|
TableMaterializer,
|
|
_AccumulateStrategy,
|
|
_BaseBufferStrategy,
|
|
_element_to_dict,
|
|
)
|
|
from cleveragents.cli.output.session import OutputSession, StructuredOutput
|
|
|
|
# ===========================================================================
|
|
# Scenario 1: Force-flush remaining buffers on session end
|
|
# Targets lines 466-469
|
|
# ===========================================================================
|
|
|
|
|
|
@given("a PlainMaterializer buffer strategy")
|
|
def step_given_plain_materializer(context: Context) -> None:
|
|
context.mat_strategy = PlainMaterializer()
|
|
|
|
|
|
@given("an OutputSession wired to that buffer strategy")
|
|
def step_given_session_with_plain_strategy(context: Context) -> None:
|
|
context.mat_session = OutputSession(
|
|
format="plain", command="test-flush", strategy=context.mat_strategy
|
|
)
|
|
|
|
|
|
@when("I create three panels in declaration order")
|
|
def step_create_three_panels(context: Context) -> None:
|
|
context.panel_0 = context.mat_session.panel("Panel-Zero")
|
|
context.panel_0.set_entry("key0", "val0")
|
|
context.panel_1 = context.mat_session.panel("Panel-One")
|
|
context.panel_1.set_entry("key1", "val1")
|
|
context.panel_2 = context.mat_session.panel("Panel-Two")
|
|
context.panel_2.set_entry("key2", "val2")
|
|
|
|
|
|
@when("I close only the third and second panels")
|
|
def step_close_third_and_second(context: Context) -> None:
|
|
# Close out of declaration order: index 2 first, then index 1
|
|
# This means index 0 is still open, so _flush_ready won't drain
|
|
# anything (it waits for _next_render_index=0 which isn't closed).
|
|
# Items 1 and 2 accumulate in self._buffers.
|
|
context.panel_2.close()
|
|
context.panel_1.close()
|
|
|
|
|
|
@when("I end the session without closing the first panel")
|
|
def step_end_session_without_first(context: Context) -> None:
|
|
# session.close() force-closes open handles, but the first panel
|
|
# (index 0) gets closed last by the session. By the time
|
|
# on_session_end fires, the buffers for index 1 and 2 should
|
|
# still be present (since _flush_ready would have flushed 0,
|
|
# then potentially 1 and 2 in sequence).
|
|
# Actually, let's be precise: session.close() calls handle.close()
|
|
# for open handles. Panel 0 close triggers on_element_closed with
|
|
# idx=0, which flushes 0, then 1, then 2 via _flush_ready.
|
|
# To truly leave items in buffers at session_end time, we need to
|
|
# avoid the session force-closing the handle. Instead, we directly
|
|
# invoke on_session_end while items are still buffered.
|
|
#
|
|
# Strategy: Manually invoke on_session_end before session.close()
|
|
# to exercise the force-flush path while buffers are non-empty.
|
|
event = SessionEnd(
|
|
event_type="session_end",
|
|
handle_id="",
|
|
element_kind="session",
|
|
exit_code=0,
|
|
)
|
|
# At this point: panel_0 is open (not closed), so _flush_ready hasn't
|
|
# flushed anything. Panels 1 and 2 are closed but stuck in _buffers
|
|
# because _next_render_index is 0 and index 0 isn't in _closed_indices.
|
|
# But wait - when panel_1 and panel_2 closed, on_element_closed stored
|
|
# them in _buffers. _flush_ready starts at _next_render_index=0, and
|
|
# since 0 is NOT in _closed_indices, it doesn't flush anything.
|
|
# So _buffers has {1: rendered_1, 2: rendered_2}.
|
|
# Calling on_session_end now will iterate sorted(self._buffers.keys())
|
|
# = [1, 2] and write their content → covers lines 466-469.
|
|
context.mat_strategy.on_session_end(event)
|
|
|
|
|
|
@then("the strategy output includes content from all three panels")
|
|
def step_verify_all_panels_in_output(context: Context) -> None:
|
|
output = context.mat_strategy.get_output()
|
|
assert "Panel-One" in output, f"Panel-One not found in output:\n{output}"
|
|
assert "Panel-Two" in output, f"Panel-Two not found in output:\n{output}"
|
|
# Panel-Zero might not be in the output since it was never closed
|
|
# (no rendered content for it). That's expected - we're testing that
|
|
# the force-flush path writes the buffered panels 1 and 2.
|
|
|
|
|
|
# ===========================================================================
|
|
# Scenario 2: Session end with non-empty buffers (color strategy)
|
|
# Also targets lines 466-469
|
|
# ===========================================================================
|
|
|
|
|
|
@given("a ColorMaterializer buffer strategy")
|
|
def step_given_color_materializer(context: Context) -> None:
|
|
context.color_mat_strategy = ColorMaterializer()
|
|
|
|
|
|
@given("an OutputSession wired to that color buffer strategy")
|
|
def step_given_session_with_color_strategy(context: Context) -> None:
|
|
context.color_mat_session = OutputSession(
|
|
format="color", command="test-color-flush", strategy=context.color_mat_strategy
|
|
)
|
|
|
|
|
|
@when("I create two status elements in order")
|
|
def step_create_two_statuses(context: Context) -> None:
|
|
context.status_0 = context.color_mat_session.status("First status", level="info")
|
|
context.status_1 = context.color_mat_session.status("Second status", level="ok")
|
|
|
|
|
|
@when("I close only the second status element")
|
|
def step_close_second_status(context: Context) -> None:
|
|
context.status_1.close()
|
|
|
|
|
|
@when("the session is closed via context exit")
|
|
def step_session_closed_via_exit(context: Context) -> None:
|
|
# Manually fire session_end while buffer has items.
|
|
# Status 0 is open, so _next_render_index=0 hasn't advanced.
|
|
# Status 1 is closed but in _buffers at index 1.
|
|
event = SessionEnd(
|
|
event_type="session_end",
|
|
handle_id="",
|
|
element_kind="session",
|
|
exit_code=0,
|
|
)
|
|
context.color_mat_strategy.on_session_end(event)
|
|
|
|
|
|
@then("the color strategy output includes both status messages")
|
|
def step_color_output_has_both(context: Context) -> None:
|
|
output = context.color_mat_strategy.get_output()
|
|
# Second status was buffered and force-flushed
|
|
assert "Second status" in output, f"Second status not in output:\n{output}"
|
|
|
|
|
|
# ===========================================================================
|
|
# Scenario 3: _render_element_plain fallback (line 169)
|
|
# ===========================================================================
|
|
|
|
|
|
@given("an unknown element type object")
|
|
def step_given_unknown_element(context: Context) -> None:
|
|
# Create an object that is not Panel, Table, StatusMessage, or ProgressIndicator
|
|
context.unknown_element = {"custom": "data", "id": 42}
|
|
|
|
|
|
@when("I render it with the plain element dispatcher")
|
|
def step_render_plain_unknown(context: Context) -> None:
|
|
context.plain_render_result = _render_element_plain(context.unknown_element)
|
|
|
|
|
|
@then("the result is the string representation of the object")
|
|
def step_verify_plain_str_fallback(context: Context) -> None:
|
|
expected = str(context.unknown_element)
|
|
assert context.plain_render_result == expected, (
|
|
f"Expected {expected!r}, got {context.plain_render_result!r}"
|
|
)
|
|
|
|
|
|
# ===========================================================================
|
|
# Scenario 4: _render_element_color fallback (line 269)
|
|
# ===========================================================================
|
|
|
|
|
|
@when("I render it with the color element dispatcher")
|
|
def step_render_color_unknown(context: Context) -> None:
|
|
context.color_render_result = _render_element_color(context.unknown_element)
|
|
|
|
|
|
@then("the color result is the string representation of the object")
|
|
def step_verify_color_str_fallback(context: Context) -> None:
|
|
expected = str(context.unknown_element)
|
|
assert context.color_render_result == expected, (
|
|
f"Expected {expected!r}, got {context.color_render_result!r}"
|
|
)
|
|
|
|
|
|
# ===========================================================================
|
|
# Scenario 5: _element_to_dict fallback (line 372)
|
|
# ===========================================================================
|
|
|
|
|
|
@when("I convert it to a dict using element_to_dict")
|
|
def step_convert_to_dict_unknown(context: Context) -> None:
|
|
context.element_dict_result = _element_to_dict(context.unknown_element)
|
|
|
|
|
|
@then("the result dict has type equal to unknown")
|
|
def step_verify_unknown_type_dict(context: Context) -> None:
|
|
assert context.element_dict_result == {"type": "unknown"}, (
|
|
f"Expected {{'type': 'unknown'}}, got {context.element_dict_result!r}"
|
|
)
|
|
|
|
|
|
# ===========================================================================
|
|
# Scenario 6: _BaseBufferStrategy._render_element default (line 440)
|
|
# ===========================================================================
|
|
|
|
|
|
@given("a raw BaseBufferStrategy instance")
|
|
def step_given_base_buffer_strategy(context: Context) -> None:
|
|
context.base_strategy = _BaseBufferStrategy()
|
|
|
|
|
|
@given("a simple Panel element for rendering")
|
|
def step_given_simple_panel(context: Context) -> None:
|
|
context.simple_panel = Panel(
|
|
title="Base Panel",
|
|
entries=[PanelEntry(key="host", value="localhost")],
|
|
)
|
|
|
|
|
|
@when("I call the base strategy render_element with the panel")
|
|
def step_call_base_render(context: Context) -> None:
|
|
context.base_render_result = context.base_strategy._render_element(
|
|
context.simple_panel
|
|
)
|
|
|
|
|
|
@then("the rendered output contains the panel title")
|
|
def step_verify_base_render(context: Context) -> None:
|
|
assert "Base Panel" in context.base_render_result, (
|
|
f"Expected 'Base Panel' in {context.base_render_result!r}"
|
|
)
|
|
|
|
|
|
# ===========================================================================
|
|
# Scenario 7: _AccumulateStrategy._serialise default (line 551)
|
|
# ===========================================================================
|
|
|
|
|
|
@given("a raw AccumulateStrategy instance")
|
|
def step_given_accumulate_strategy(context: Context) -> None:
|
|
context.accum_strategy = _AccumulateStrategy()
|
|
|
|
|
|
@given("a mock StructuredOutput snapshot")
|
|
def step_given_mock_snapshot(context: Context) -> None:
|
|
context.mock_snapshot = StructuredOutput(
|
|
command="test",
|
|
session_id="ses-000001",
|
|
elements=[],
|
|
exit_code=0,
|
|
metadata={},
|
|
)
|
|
|
|
|
|
@when("I call the base strategy serialise method")
|
|
def step_call_base_serialise(context: Context) -> None:
|
|
context.serialise_result = context.accum_strategy._serialise(context.mock_snapshot)
|
|
|
|
|
|
@then("the result is an empty string")
|
|
def step_verify_empty_serialise(context: Context) -> None:
|
|
assert context.serialise_result == "", (
|
|
f"Expected empty string, got {context.serialise_result!r}"
|
|
)
|
|
|
|
|
|
# ===========================================================================
|
|
# Scenario 8: _AccumulateStrategy._serialise_dict default (line 576)
|
|
# ===========================================================================
|
|
|
|
|
|
@when("I call the base strategy serialise_dict with a sample dict")
|
|
def step_call_base_serialise_dict(context: Context) -> None:
|
|
context.serialise_dict_result = context.accum_strategy._serialise_dict(
|
|
{"key": "value"}
|
|
)
|
|
|
|
|
|
@then("the serialise_dict result is an empty string")
|
|
def step_verify_empty_serialise_dict(context: Context) -> None:
|
|
assert context.serialise_dict_result == "", (
|
|
f"Expected empty string, got {context.serialise_dict_result!r}"
|
|
)
|
|
|
|
|
|
# ===========================================================================
|
|
# Scenario 9: on_element_closed with unknown handle_id (line 445)
|
|
# ===========================================================================
|
|
|
|
|
|
@when("I dispatch an ElementClosed event with an unknown handle id")
|
|
def step_dispatch_unknown_handle_closed(context: Context) -> None:
|
|
event = ElementClosed(
|
|
event_type="closed",
|
|
handle_id="hdl-nonexistent-999",
|
|
element_kind="panel",
|
|
final_state=Panel(title="Ghost"),
|
|
)
|
|
context.mat_strategy.on_element_closed(event)
|
|
|
|
|
|
@then("the strategy output is empty")
|
|
def step_verify_empty_output(context: Context) -> None:
|
|
output = context.mat_strategy.get_output()
|
|
assert output == "", f"Expected empty output, got {output!r}"
|
|
|
|
|
|
# ===========================================================================
|
|
# Scenario 10: TableMaterializer renders non-table element via plain
|
|
# Targets line 327 (_render_element_table -> _render_element_plain)
|
|
# ===========================================================================
|
|
|
|
|
|
@given("a TableMaterializer buffer strategy")
|
|
def step_given_table_materializer(context: Context) -> None:
|
|
context.table_mat_strategy = TableMaterializer()
|
|
|
|
|
|
@given("an OutputSession wired to that table buffer strategy")
|
|
def step_given_session_with_table_strategy(context: Context) -> None:
|
|
context.table_mat_session = OutputSession(
|
|
format="table",
|
|
command="test-table-fallback",
|
|
strategy=context.table_mat_strategy,
|
|
)
|
|
|
|
|
|
@when('I create and close a status element with message "{msg}" and level "{level}"')
|
|
def step_create_close_status(context: Context, msg: str, level: str) -> None:
|
|
handle = context.table_mat_session.status(msg, level=level)
|
|
handle.close()
|
|
|
|
|
|
@then('the table strategy output contains "{text}"')
|
|
def step_table_strategy_contains(context: Context, text: str) -> None:
|
|
output = context.table_mat_strategy.get_output()
|
|
assert text in output, f"Expected {text!r} in:\n{output}"
|
|
|
|
|
|
# ===========================================================================
|
|
# Scenario 11: Session end with all elements already flushed
|
|
# ===========================================================================
|
|
|
|
|
|
@when('I create and close a panel with title "{title}" and entry "{key}" "{val}"')
|
|
def step_create_close_panel(context: Context, title: str, key: str, val: str) -> None:
|
|
handle = context.mat_session.panel(title)
|
|
handle.set_entry(key, val)
|
|
handle.close()
|
|
|
|
|
|
@when("the session is ended cleanly")
|
|
def step_end_session_cleanly(context: Context) -> None:
|
|
context.mat_session.close()
|
|
|
|
|
|
@then('the strategy output contains "{text}" exactly once')
|
|
def step_output_contains_once(context: Context, text: str) -> None:
|
|
output = context.mat_strategy.get_output()
|
|
count = output.count(text)
|
|
assert count == 1, (
|
|
f"Expected '{text}' exactly once, found {count} times in:\n{output}"
|
|
)
|
|
|
|
|
|
# ===========================================================================
|
|
# Scenario 12: RichMaterializer renders with ANSI codes
|
|
# ===========================================================================
|
|
|
|
ANSI_RE = re.compile(r"\033\[")
|
|
|
|
|
|
@given("a RichMaterializer buffer strategy")
|
|
def step_given_rich_materializer(context: Context) -> None:
|
|
context.rich_mat_strategy = RichMaterializer()
|
|
|
|
|
|
@given("an OutputSession wired to that rich buffer strategy")
|
|
def step_given_session_with_rich_strategy(context: Context) -> None:
|
|
context.rich_mat_session = OutputSession(
|
|
format="rich",
|
|
command="test-rich",
|
|
strategy=context.rich_mat_strategy,
|
|
)
|
|
|
|
|
|
@when('I create and close a rich panel with title "{title}" and entry "{key}" "{val}"')
|
|
def step_create_close_rich_panel(
|
|
context: Context, title: str, key: str, val: str
|
|
) -> None:
|
|
handle = context.rich_mat_session.panel(title)
|
|
handle.set_entry(key, val)
|
|
handle.close()
|
|
|
|
|
|
@then("the rich strategy output contains ANSI escape codes")
|
|
def step_rich_has_ansi(context: Context) -> None:
|
|
output = context.rich_mat_strategy.get_output()
|
|
assert ANSI_RE.search(output), f"No ANSI codes found in output:\n{output}"
|
|
|
|
|
|
@then('the rich strategy output contains "{text}"')
|
|
def step_rich_output_contains(context: Context, text: str) -> None:
|
|
output = context.rich_mat_strategy.get_output()
|
|
# Strip ANSI codes for text comparison
|
|
clean = re.sub(r"\033\[[0-9;]*m", "", output)
|
|
assert text in output or text in clean, f"Expected {text!r} in:\n{output}"
|