From 208f04ebda32e3688a36c0620ca855ffdab335fc Mon Sep 17 00:00:00 2001 From: Himanshu Dongre Date: Mon, 13 Apr 2026 16:28:50 +0530 Subject: [PATCH] Fix stats honesty: compute from actual rendered sections, never silently no-op --- cli/smriti_cli/formatters.py | 55 ++++++++++++++++++---------- cli/tests/test_state_multi_branch.py | 24 +++++++----- 2 files changed, 51 insertions(+), 28 deletions(-) diff --git a/cli/smriti_cli/formatters.py b/cli/smriti_cli/formatters.py index cdd5ebd..cf14d19 100644 --- a/cli/smriti_cli/formatters.py +++ b/cli/smriti_cli/formatters.py @@ -103,35 +103,47 @@ def _artifact_section( return "\n".join(lines) + "\n" -def _artifact_compact_stats(artifacts: list[dict]) -> dict | None: - """Compute savings from compact mode. Returns None if no artifacts.""" +def _artifact_compact_stats( + artifacts: list[dict], + checkpoint_id: str = "", +) -> dict | None: + """Compute savings by rendering both full and compact artifact sections + and comparing their actual char counts. Returns None only if there are + no artifacts at all.""" if not artifacts: return None - full_chars = sum(len(a.get("content") or "") for a in artifacts) - if full_chars == 0: - return None - # Compact replaces content with labels (~20 chars each + overhead) - compact_chars = sum(len(f"- {a.get('label', 'Untitled')}") + 1 for a in artifacts) + 80 + full_rendered = _artifact_section(artifacts, full=True) + compact_rendered = _artifact_section( + artifacts, compact=True, checkpoint_id=checkpoint_id, + ) + full_chars = len(full_rendered) + compact_chars = len(compact_rendered) saved = full_chars - compact_chars - if saved <= 0: - return None - pct = round(saved * 100 / full_chars) + pct = round(saved * 100 / full_chars) if full_chars > 0 else 0 return { "artifacts_omitted": len(artifacts), - "chars_saved": saved, - "reduction_pct": pct, + "full_chars": full_chars, + "compact_chars": compact_chars, + "chars_saved": max(saved, 0), + "reduction_pct": max(pct, 0), } -def _format_compact_stats_footer(stats: dict | None) -> str: - """One-line footer for --stats. Empty string if no stats.""" +def _format_compact_stats_footer(stats: dict | None, compact: bool = True) -> str: + """Footer for --stats. Always returns a line when called — either + savings data or an explicit 'nothing to report' message.""" + if not compact: + return "\n---\ncompact stats: no artifact savings to report (--compact not used)\n" if not stats: - return "" + return "\n---\ncompact stats: no artifact savings to report (0 artifacts)\n" + if stats["chars_saved"] <= 0: + return "\n---\ncompact stats: no artifact savings to report (artifacts too small)\n" return ( f"\n---\n" f"compact stats: {stats['artifacts_omitted']} artifact(s) omitted " f"· {stats['chars_saved']} chars saved " - f"· {stats['reduction_pct']}% reduction in artifact section\n" + f"· {stats['reduction_pct']}% smaller artifact section " + f"(full: {stats['full_chars']} chars → compact: {stats['compact_chars']} chars)\n" ) @@ -302,9 +314,14 @@ def format_state_brief( result = "\n".join(p for p in parts if p).rstrip() + "\n" - if stats and compact: - compact_stats = _artifact_compact_stats(artifacts) - result += _format_compact_stats_footer(compact_stats) + if stats: + if compact: + compact_stats = _artifact_compact_stats( + artifacts, checkpoint_id=str(checkpoint_id), + ) + result += _format_compact_stats_footer(compact_stats, compact=True) + else: + result += _format_compact_stats_footer(None, compact=False) return result diff --git a/cli/tests/test_state_multi_branch.py b/cli/tests/test_state_multi_branch.py index 3e29fb4..fe6e279 100644 --- a/cli/tests/test_state_multi_branch.py +++ b/cli/tests/test_state_multi_branch.py @@ -419,8 +419,8 @@ def test_mcp_smriti_state_compact_mode(mock_client): assert "compact — content omitted" in out -def test_compact_stats_footer(): - """--compact --stats should append a savings footer.""" +def test_compact_stats_footer_shows_actual_rendered_sizes(): + """--compact --stats shows full and compact rendered char counts.""" commit = _commit_with_artifacts() out = format_state_brief( _base_space(), _base_head(), commit, compact=True, stats=True, @@ -429,24 +429,30 @@ def test_compact_stats_footer(): assert "compact stats:" in out assert "artifact(s) omitted" in out assert "chars saved" in out - assert "reduction in artifact section" in out + assert "smaller artifact section" in out + # Must show actual rendered sizes, not heuristic estimates + assert "full:" in out + assert "compact:" in out + assert "chars)" in out -def test_stats_without_compact_produces_no_footer(): - """Stats without compact should not show a footer.""" +def test_stats_without_compact_shows_explicit_message(): + """--stats without --compact should say so, not silently produce nothing.""" commit = _commit_with_artifacts() out = format_state_brief( _base_space(), _base_head(), commit, stats=True, ) - assert "compact stats:" not in out + assert "compact stats:" in out + assert "--compact not used" in out -def test_stats_with_no_artifacts_produces_no_footer(): - """Compact+stats on a commit with no artifacts: no footer.""" +def test_stats_with_no_artifacts_shows_explicit_message(): + """Compact+stats on a commit with no artifacts: explicit message.""" commit = _base_commit() out = format_state_brief( _base_space(), _base_head(), commit, compact=True, stats=True, ) - assert "compact stats:" not in out + assert "compact stats:" in out + assert "0 artifacts" in out