refactor(cli): deduplicate session list summary logic in list_sessions
CI / lint (pull_request) Successful in 21s
CI / typecheck (pull_request) Successful in 46s
CI / security (pull_request) Successful in 1m1s
CI / quality (pull_request) Successful in 35s
CI / build (pull_request) Successful in 16s
CI / helm (pull_request) Successful in 23s
CI / unit_tests (pull_request) Successful in 7m4s
CI / e2e_tests (pull_request) Successful in 18m8s
CI / integration_tests (pull_request) Successful in 22m40s
CI / docker (pull_request) Successful in 1m19s
CI / coverage (pull_request) Successful in 10m36s
CI / status-check (pull_request) Successful in 1s
CI / benchmark-publish (pull_request) Has been skipped
CI / benchmark-regression (pull_request) Successful in 57m8s
CI / lint (pull_request) Successful in 21s
CI / typecheck (pull_request) Successful in 46s
CI / security (pull_request) Successful in 1m1s
CI / quality (pull_request) Successful in 35s
CI / build (pull_request) Successful in 16s
CI / helm (pull_request) Successful in 23s
CI / unit_tests (pull_request) Successful in 7m4s
CI / e2e_tests (pull_request) Successful in 18m8s
CI / integration_tests (pull_request) Successful in 22m40s
CI / docker (pull_request) Successful in 1m19s
CI / coverage (pull_request) Successful in 10m36s
CI / status-check (pull_request) Successful in 1s
CI / benchmark-publish (pull_request) Has been skipped
CI / benchmark-regression (pull_request) Successful in 57m8s
Refactor list_sessions to reuse the summary dict already computed by _session_list_dict instead of recalculating total_msgs, sorted_sessions, most_recent, and oldest independently. The rich table summary panel now reads directly from data["summary"], ensuring JSON and rich table output are always consistent and the summary logic lives in exactly one place (_session_list_dict). Add Behave feature (session_list_summary_dedup.feature) with 6 scenarios verifying that JSON and rich table summary values are consistent across all required fields: total, most_recent, oldest, total_messages, storage. ISSUES CLOSED: #3046
This commit is contained in:
@@ -0,0 +1,44 @@
|
||||
@tdd_issue @tdd_issue_3046
|
||||
Feature: Session list summary consistency between JSON and rich table output
|
||||
As a developer
|
||||
I want the session list summary values to be consistent between JSON and rich table output
|
||||
So that the deduplicated summary logic in _session_list_dict is the single source of truth
|
||||
|
||||
Background:
|
||||
Given a session-list-summary mock service with two sessions
|
||||
|
||||
Scenario: JSON output summary matches rich table summary for total count
|
||||
When I invoke session list with JSON format
|
||||
And I invoke session list with rich format
|
||||
Then the JSON summary total should match the rich table total
|
||||
|
||||
Scenario: JSON output summary matches rich table summary for total messages
|
||||
When I invoke session list with JSON format
|
||||
And I invoke session list with rich format
|
||||
Then the JSON summary total_messages should match the rich table total messages
|
||||
|
||||
Scenario: JSON output summary matches rich table summary for most recent session
|
||||
When I invoke session list with JSON format
|
||||
And I invoke session list with rich format
|
||||
Then the JSON summary most_recent should appear in the rich table summary panel
|
||||
|
||||
Scenario: JSON output summary matches rich table summary for oldest session
|
||||
When I invoke session list with JSON format
|
||||
And I invoke session list with rich format
|
||||
Then the JSON summary oldest should appear in the rich table summary panel
|
||||
|
||||
Scenario: Rich table summary panel contains all required fields
|
||||
When I invoke session list with rich format
|
||||
Then the rich table output contains "Total:"
|
||||
And the rich table output contains "Most Recent:"
|
||||
And the rich table output contains "Oldest:"
|
||||
And the rich table output contains "Total Messages:"
|
||||
And the rich table output contains "Storage:"
|
||||
|
||||
Scenario: JSON output summary contains all required fields
|
||||
When I invoke session list with JSON format
|
||||
Then the JSON output has a summary key with total
|
||||
And the JSON output has a summary key with most_recent
|
||||
And the JSON output has a summary key with oldest
|
||||
And the JSON output has a summary key with total_messages
|
||||
And the JSON output has a summary key with storage
|
||||
@@ -0,0 +1,251 @@
|
||||
"""Step definitions for session_list_summary_dedup.feature (issue #3046).
|
||||
|
||||
Verifies that the summary values in the JSON output and the rich table output
|
||||
are consistent — i.e. that the deduplicated ``_session_list_dict`` helper is
|
||||
the single source of truth for both rendering paths.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from datetime import UTC, datetime
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from behave import given, then, when
|
||||
from behave.runner import Context
|
||||
from typer.testing import CliRunner
|
||||
|
||||
from cleveragents.cli.commands.session import app as session_app
|
||||
from cleveragents.domain.models.core.session import (
|
||||
MessageRole,
|
||||
Session,
|
||||
SessionMessage,
|
||||
SessionService,
|
||||
SessionTokenUsage,
|
||||
)
|
||||
|
||||
_runner = CliRunner()
|
||||
|
||||
# Stable ULIDs for deterministic test output
|
||||
_ULID_NEWER = "01SCVBST000000000000000001"
|
||||
_ULID_OLDER = "01SCVBST000000000000000002"
|
||||
|
||||
# Newer session updated more recently
|
||||
_DT_NEWER = datetime(2025, 8, 1, 12, 0, 0, tzinfo=UTC)
|
||||
_DT_OLDER = datetime(2025, 7, 1, 10, 0, 0, tzinfo=UTC)
|
||||
|
||||
|
||||
def _make_session(
|
||||
session_id: str,
|
||||
actor_name: str | None,
|
||||
updated_at: datetime,
|
||||
message_count: int = 0,
|
||||
name: str | None = None,
|
||||
) -> Session:
|
||||
messages = [
|
||||
SessionMessage(
|
||||
message_id=f"01SCVBST0000000000000000{i + 1:02d}",
|
||||
role=MessageRole.USER,
|
||||
content=f"msg {i}",
|
||||
sequence=i,
|
||||
timestamp=updated_at,
|
||||
)
|
||||
for i in range(message_count)
|
||||
]
|
||||
return Session(
|
||||
session_id=session_id,
|
||||
actor_name=actor_name,
|
||||
namespace="local",
|
||||
messages=messages,
|
||||
linked_plan_ids=[],
|
||||
token_usage=SessionTokenUsage(),
|
||||
created_at=updated_at,
|
||||
updated_at=updated_at,
|
||||
metadata={},
|
||||
cost_budget=None,
|
||||
name=name,
|
||||
)
|
||||
|
||||
|
||||
def _mock_service_with_sessions() -> MagicMock:
|
||||
svc = MagicMock(spec=SessionService)
|
||||
sessions = [
|
||||
_make_session(
|
||||
session_id=_ULID_NEWER,
|
||||
actor_name="openai/gpt-4",
|
||||
updated_at=_DT_NEWER,
|
||||
message_count=3,
|
||||
name="newer-session",
|
||||
),
|
||||
_make_session(
|
||||
session_id=_ULID_OLDER,
|
||||
actor_name=None,
|
||||
updated_at=_DT_OLDER,
|
||||
message_count=2,
|
||||
name=None,
|
||||
),
|
||||
]
|
||||
svc.list.return_value = sessions
|
||||
return svc
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Background
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@given("a session-list-summary mock service with two sessions")
|
||||
def step_setup_mock_service(context: Context) -> None:
|
||||
context.sls_svc = _mock_service_with_sessions()
|
||||
context.sls_json_result = None
|
||||
context.sls_rich_result = None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# When
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@when("I invoke session list with JSON format")
|
||||
def step_invoke_json(context: Context) -> None:
|
||||
with patch(
|
||||
"cleveragents.cli.commands.session._get_session_service",
|
||||
return_value=context.sls_svc,
|
||||
):
|
||||
context.sls_json_result = _runner.invoke(
|
||||
session_app, ["list", "--format", "json"]
|
||||
)
|
||||
|
||||
|
||||
@when("I invoke session list with rich format")
|
||||
def step_invoke_rich(context: Context) -> None:
|
||||
with patch(
|
||||
"cleveragents.cli.commands.session._get_session_service",
|
||||
return_value=context.sls_svc,
|
||||
):
|
||||
context.sls_rich_result = _runner.invoke(
|
||||
session_app, ["list", "--format", "rich"]
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Then — cross-format consistency
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _get_json_summary(context: Context) -> dict:
|
||||
result = context.sls_json_result
|
||||
assert result is not None, "JSON result not set — invoke JSON format first"
|
||||
assert result.exit_code == 0, (
|
||||
f"JSON command failed (exit {result.exit_code}):\n{result.output}"
|
||||
)
|
||||
data = json.loads(result.output)
|
||||
assert "summary" in data, f"No 'summary' key in JSON output: {data}"
|
||||
return data["summary"]
|
||||
|
||||
|
||||
def _get_rich_output(context: Context) -> str:
|
||||
result = context.sls_rich_result
|
||||
assert result is not None, "Rich result not set — invoke rich format first"
|
||||
assert result.exit_code == 0, (
|
||||
f"Rich command failed (exit {result.exit_code}):\n{result.output}"
|
||||
)
|
||||
return result.output
|
||||
|
||||
|
||||
@then("the JSON summary total should match the rich table total")
|
||||
def step_total_matches(context: Context) -> None:
|
||||
summary = _get_json_summary(context)
|
||||
rich_output = _get_rich_output(context)
|
||||
expected_total = str(summary["total"])
|
||||
assert "Total:" in rich_output, f"'Total:' not found in rich output:\n{rich_output}"
|
||||
assert expected_total in rich_output, (
|
||||
f"Expected total '{expected_total}' not found in rich output:\n{rich_output}"
|
||||
)
|
||||
|
||||
|
||||
@then("the JSON summary total_messages should match the rich table total messages")
|
||||
def step_total_messages_matches(context: Context) -> None:
|
||||
summary = _get_json_summary(context)
|
||||
rich_output = _get_rich_output(context)
|
||||
expected = str(summary["total_messages"])
|
||||
assert "Total Messages:" in rich_output, (
|
||||
f"'Total Messages:' not found in rich output:\n{rich_output}"
|
||||
)
|
||||
assert expected in rich_output, (
|
||||
f"Expected total_messages '{expected}' not found in rich output:\n{rich_output}"
|
||||
)
|
||||
|
||||
|
||||
@then("the JSON summary most_recent should appear in the rich table summary panel")
|
||||
def step_most_recent_matches(context: Context) -> None:
|
||||
summary = _get_json_summary(context)
|
||||
rich_output = _get_rich_output(context)
|
||||
most_recent = summary["most_recent"]
|
||||
assert most_recent is not None, "most_recent is None in JSON summary"
|
||||
assert most_recent in rich_output, (
|
||||
f"Expected most_recent '{most_recent}' not found in rich output:\n{rich_output}"
|
||||
)
|
||||
|
||||
|
||||
@then("the JSON summary oldest should appear in the rich table summary panel")
|
||||
def step_oldest_matches(context: Context) -> None:
|
||||
summary = _get_json_summary(context)
|
||||
rich_output = _get_rich_output(context)
|
||||
oldest = summary["oldest"]
|
||||
assert oldest is not None, "oldest is None in JSON summary"
|
||||
assert oldest in rich_output, (
|
||||
f"Expected oldest '{oldest}' not found in rich output:\n{rich_output}"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Then — rich table field presence
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@then('the rich table output contains "{text}"')
|
||||
def step_rich_contains(context: Context, text: str) -> None:
|
||||
rich_output = _get_rich_output(context)
|
||||
assert text in rich_output, f"Expected '{text}' in rich output:\n{rich_output}"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Then — JSON summary field presence
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@then("the JSON output has a summary key with total")
|
||||
def step_json_has_total(context: Context) -> None:
|
||||
summary = _get_json_summary(context)
|
||||
assert "total" in summary, f"'total' not in summary: {summary}"
|
||||
assert isinstance(summary["total"], int), (
|
||||
f"'total' should be int, got {type(summary['total'])}"
|
||||
)
|
||||
|
||||
|
||||
@then("the JSON output has a summary key with most_recent")
|
||||
def step_json_has_most_recent(context: Context) -> None:
|
||||
summary = _get_json_summary(context)
|
||||
assert "most_recent" in summary, f"'most_recent' not in summary: {summary}"
|
||||
|
||||
|
||||
@then("the JSON output has a summary key with oldest")
|
||||
def step_json_has_oldest(context: Context) -> None:
|
||||
summary = _get_json_summary(context)
|
||||
assert "oldest" in summary, f"'oldest' not in summary: {summary}"
|
||||
|
||||
|
||||
@then("the JSON output has a summary key with total_messages")
|
||||
def step_json_has_total_messages(context: Context) -> None:
|
||||
summary = _get_json_summary(context)
|
||||
assert "total_messages" in summary, f"'total_messages' not in summary: {summary}"
|
||||
assert isinstance(summary["total_messages"], int), (
|
||||
f"'total_messages' should be int, got {type(summary['total_messages'])}"
|
||||
)
|
||||
|
||||
|
||||
@then("the JSON output has a summary key with storage")
|
||||
def step_json_has_storage(context: Context) -> None:
|
||||
summary = _get_json_summary(context)
|
||||
assert "storage" in summary, f"'storage' not in summary: {summary}"
|
||||
@@ -324,21 +324,18 @@ def list_sessions(
|
||||
console.print(table)
|
||||
console.print()
|
||||
|
||||
# Summary panel with all required fields
|
||||
total_msgs = sum(s.message_count for s in sessions)
|
||||
# Find most recent and oldest sessions
|
||||
sorted_sessions = sorted(sessions, key=lambda x: x.updated_at, reverse=True)
|
||||
most_recent = sorted_sessions[0].name or sorted_sessions[0].session_id[:8]
|
||||
oldest = sorted_sessions[-1].name or sorted_sessions[-1].session_id[:8]
|
||||
# Reuse the summary dict already computed by _session_list_dict to avoid
|
||||
# duplicating the total_msgs / sorted_sessions / most_recent / oldest logic.
|
||||
summary = data["summary"]
|
||||
|
||||
summary_table = Table.grid(padding=(0, 1))
|
||||
summary_table.add_column(style="cyan bold", justify="left")
|
||||
summary_table.add_column(style="white", justify="left")
|
||||
summary_table.add_row("Total:", str(len(sessions)))
|
||||
summary_table.add_row("Most Recent:", most_recent)
|
||||
summary_table.add_row("Oldest:", oldest)
|
||||
summary_table.add_row("Total Messages:", str(total_msgs))
|
||||
summary_table.add_row("Storage:", "0 KB") # Placeholder
|
||||
summary_table.add_row("Total:", str(summary["total"]))
|
||||
summary_table.add_row("Most Recent:", summary["most_recent"] or "")
|
||||
summary_table.add_row("Oldest:", summary["oldest"] or "")
|
||||
summary_table.add_row("Total Messages:", str(summary["total_messages"]))
|
||||
summary_table.add_row("Storage:", summary["storage"])
|
||||
|
||||
console.print(Panel(summary_table, title="Summary", border_style="blue"))
|
||||
console.print()
|
||||
|
||||
Reference in New Issue
Block a user