diff --git a/agent/context_references.py b/agent/context_references.py index 1f4e01ec2bfe0..6f428bf87badd 100644 --- a/agent/context_references.py +++ b/agent/context_references.py @@ -13,7 +13,7 @@ from typing import Awaitable, Callable from agent.model_metadata import estimate_tokens_rough from hermes_cli._subprocess_compat import IS_WINDOWS, windows_hide_flags -from hermes_cli.sizefmt import format_bytes as _human_bytes +from hermes_cli.sizefmt import format_bytes _QUOTED_REFERENCE_VALUE = r'(?:`[^`\n]+`|"[^"\n]+"|\'[^\'\n]+\')' REFERENCE_PATTERN = re.compile( @@ -559,7 +559,7 @@ def _binary_reference_block(ref: ContextReference, path: Path) -> str: mime, _ = mimetypes.guess_type(path.name) mime = mime or "application/octet-stream" try: - size = _human_bytes(path.stat().st_size) + size = format_bytes(path.stat().st_size) except OSError: size = "unknown size" return ( diff --git a/agent/curator_backup.py b/agent/curator_backup.py index 4d17f3743c0a3..d7ea51755cfa4 100644 --- a/agent/curator_backup.py +++ b/agent/curator_backup.py @@ -50,10 +50,7 @@ from typing import Any, Dict, List, Optional, Set, Tuple from hermes_constants import get_hermes_home from agent.skill_utils import is_excluded_skill_path - -# Shared byte formatter; public name ``format_size`` is part of this module's -# established surface, so alias rather than churn the callers. -from hermes_cli.sizefmt import format_bytes as format_size +from hermes_cli.sizefmt import format_bytes logger = logging.getLogger(__name__) @@ -749,6 +746,6 @@ def summarize_backups() -> str: f"{r.get('id','?'):<24} " f"{(r.get('reason','?') or '?')[:40]:<40} " f"{r.get('skill_files', 0):>6} " - f"{format_size(int(r.get('archive_bytes', 0))):>8}" + f"{format_bytes(int(r.get('archive_bytes', 0))):>8}" ) return "\n".join(lines) diff --git a/hermes_cli/checkpoints.py b/hermes_cli/checkpoints.py index cecd22df2d7e0..0d7c866b40028 100644 --- a/hermes_cli/checkpoints.py +++ b/hermes_cli/checkpoints.py @@ -27,14 +27,7 @@ from datetime import datetime from pathlib import Path from typing import Any, Optional -from hermes_cli.sizefmt import format_bytes - - -def _fmt_bytes(n: Optional[int]) -> str: - # Delegates to the shared formatter; ``or 0`` preserves this module's - # historical None/0 -> "0 B" display (the shared helper renders None - # as "?", which is wrong for a size total that is genuinely zero). - return format_bytes(n or 0) +from hermes_cli.sizefmt import format_bytes as _fmt_bytes def _fmt_ts(ts: Any) -> str: diff --git a/hermes_cli/doctor.py b/hermes_cli/doctor.py index 202942b476d4f..88ce3ecba73bf 100644 --- a/hermes_cli/doctor.py +++ b/hermes_cli/doctor.py @@ -320,8 +320,8 @@ def check_info(text: str): STATE_DB_SIZE_WARN_BYTES = 1 * 1024 * 1024 * 1024 # 1 GiB logical size -# Shared byte formatter (private alias: this module's rendering helpers and -# tests refer to ``_human_bytes``). +# Shared byte formatter, aliased to the name this module's three rendering +# call sites already use. from hermes_cli.sizefmt import format_bytes as _human_bytes diff --git a/hermes_cli/sizefmt.py b/hermes_cli/sizefmt.py index 99cf9e8c83634..f0680f4229d85 100644 --- a/hermes_cli/sizefmt.py +++ b/hermes_cli/sizefmt.py @@ -1,11 +1,8 @@ """Small shared size-formatting helpers for CLI/agent output. -Public home for the human-readable byte formatter that previously existed -as five near-identical private copies (``hermes_cli/backup.py``, -``hermes_cli/checkpoints.py``, ``hermes_cli/doctor.py``, -``agent/context_references.py``, ``agent/curator_backup.py``). Sibling of -``hermes_cli.timefmt``, and kept dependency-free for the same reason: -lightweight consumers must not drag in the whole CLI surface. +Sibling of ``hermes_cli.timefmt`` (same extraction rationale: a tiny +purpose-named module lightweight consumers can import without dragging in +the CLI surface). Replaces six near-identical private byte formatters. Two in-repo formatters intentionally do NOT delegate here: @@ -19,20 +16,21 @@ Two in-repo formatters intentionally do NOT delegate here: from __future__ import annotations -def format_bytes(n, *, fallback: str = "?") -> str: +def format_bytes(n) -> str: """1234567 -> '1.2 MB' (B/KB/MB/GB/TB; integer bytes, one decimal above). - Accepts anything ``float()`` accepts; returns *fallback* for None or - unparseable input so display call sites never raise. + Accepts anything ``float()`` accepts; returns ``"?"`` for None or + unparseable input so display call sites never raise (contract inherited + from doctor's original copy — its stats dict tolerates None fields). """ try: size = float(n) except (TypeError, ValueError): - return fallback + return "?" if size < 1024: return f"{int(size)} B" - for unit in ("KB", "MB", "GB", "TB"): + for unit in ("KB", "MB", "GB"): size /= 1024.0 - if size < 1024 or unit == "TB": + if size < 1024: return f"{size:.1f} {unit}" - return f"{size:.1f} TB" # unreachable; keeps type-checkers satisfied + return f"{size / 1024.0:.1f} TB" diff --git a/hermes_cli/update_cmd.py b/hermes_cli/update_cmd.py index c99d56a81e18b..6d6c139633e09 100644 --- a/hermes_cli/update_cmd.py +++ b/hermes_cli/update_cmd.py @@ -2692,12 +2692,9 @@ def _run_pre_update_backup(args) -> Optional[str]: size_bytes = 0 # Human-readable size - size_str = f"{size_bytes} B" - for unit in ("KB", "MB", "GB"): - if size_bytes < 1024: - break - size_bytes /= 1024 - size_str = f"{size_bytes:.1f} {unit}" + from hermes_cli.sizefmt import format_bytes + + size_str = format_bytes(size_bytes) # Render path using display_hermes_home so the user sees ~/.hermes/... try: diff --git a/tests/hermes_cli/test_sizefmt.py b/tests/hermes_cli/test_sizefmt.py index f2ad56a927b21..17fe0d0e7f0b9 100644 --- a/tests/hermes_cli/test_sizefmt.py +++ b/tests/hermes_cli/test_sizefmt.py @@ -1,10 +1,10 @@ """Tests for the shared byte formatter (hermes_cli.sizefmt). -Consolidates five near-identical private formatters (backup, checkpoints, -doctor, context_references, curator_backup). The contract below locks the -shared behavior, including the two deliberate changes vs the old copies: -a real TB tier (doctor/context_references/curator_backup previously -rendered 1 TiB as '1024.0 GB') and non-raising fallback for None/garbage. +Consolidates six near-identical formatters (backup, checkpoints, doctor, +update_cmd, context_references, curator_backup). The contract below locks +the shared behavior, including the one deliberate change vs the old +copies: a real TB tier (doctor/context_references/curator_backup +previously rendered 1 TiB as '1024.0 GB'). """ import pytest @@ -40,24 +40,23 @@ def test_format_bytes_tb_tier_not_gb_overflow(): def test_format_bytes_never_raises(): + """Doctor's stats dict tolerates None in every field; the formatter must + render (not raise) for None/garbage.""" assert format_bytes(None) == "?" assert format_bytes("garbage") == "?" assert format_bytes("2048") == "2.0 KB" # numeric strings accepted - assert format_bytes(None, fallback="unknown") == "unknown" -def test_delegating_aliases_share_the_implementation(): - """The five migrated call sites must all resolve to the shared helper - (checkpoints wraps it to preserve its None -> '0 B' display).""" - from agent.context_references import _human_bytes as ctx - from agent.curator_backup import format_size as curator +def test_migrated_sites_render_through_the_shared_helper(): + """Behavior contract for the aliased call sites: the module-local names + must render byte-identically to the shared helper (how they delegate is + an implementation detail — only the rendering is pinned).""" from hermes_cli.backup import _format_size as backup from hermes_cli.checkpoints import _fmt_bytes as checkpoints from hermes_cli.doctor import _human_bytes as doctor - assert backup is format_bytes - assert ctx is format_bytes - assert curator is format_bytes - assert doctor is format_bytes - assert checkpoints(None) == "0 B" - assert checkpoints(2048) == format_bytes(2048) + for n in (0, 512, 2048, 1234567, 1024**3, 1024**4): + expected = format_bytes(n) + assert backup(n) == expected + assert checkpoints(n) == expected + assert doctor(n) == expected