From 72898984946607c443f563a368f15d78c24c17c8 Mon Sep 17 00:00:00 2001 From: kshitij <82637225+kshitijk4poor@users.noreply.github.com> Date: Sat, 8 Aug 2026 14:41:36 +0530 Subject: [PATCH] refactor: consolidate five duplicate byte formatters into hermes_cli.sizefmt Five modules each carried a private near-identical human-readable byte formatter (backup._format_size, checkpoints._fmt_bytes, doctor._human_bytes, context_references._human_bytes, curator_backup.format_size). Three of them silently topped out at GB and rendered a 1 TiB value as '1024.0 GB'. All five now alias one shared format_bytes in hermes_cli/sizefmt.py (sibling of timefmt.py, same zero-dependency rationale), keeping each module's established local name so no caller churns. Deliberately NOT migrated (behavior differs on purpose): - session_recovery._format_bytes: binary suffixes (KiB/MiB/GiB) - qqbot chunked_upload.format_size: '100.0 B' one-decimal style, pinned by its protocol tests Net -33 production LOC before the new module; parity verified over a 16-value corpus against all five verbatim originals (only divergence: the TB tier fix). Contract tests mutation-checked red-green. --- agent/context_references.py | 10 +---- agent/curator_backup.py | 11 ++---- hermes_cli/backup.py | 13 ++----- hermes_cli/checkpoints.py | 17 ++++----- hermes_cli/doctor.py | 16 ++------ hermes_cli/sizefmt.py | 38 +++++++++++++++++++ tests/hermes_cli/test_sizefmt.py | 63 ++++++++++++++++++++++++++++++++ 7 files changed, 120 insertions(+), 48 deletions(-) create mode 100644 hermes_cli/sizefmt.py create mode 100644 tests/hermes_cli/test_sizefmt.py diff --git a/agent/context_references.py b/agent/context_references.py index ab370a5a59240..1f4e01ec2bfe0 100644 --- a/agent/context_references.py +++ b/agent/context_references.py @@ -13,6 +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 _QUOTED_REFERENCE_VALUE = r'(?:`[^`\n]+`|"[^"\n]+"|\'[^\'\n]+\')' REFERENCE_PATTERN = re.compile( @@ -554,15 +555,6 @@ def _rg_files(path: Path, cwd: Path, limit: int) -> list[Path] | None: return files[:limit] -def _human_bytes(n: int) -> str: - size = float(n) - for unit in ("B", "KB", "MB", "GB"): - if size < 1024 or unit == "GB": - return f"{int(size)} {unit}" if unit == "B" else f"{size:.1f} {unit}" - size /= 1024 - return f"{size:.1f} GB" - - def _binary_reference_block(ref: ContextReference, path: Path) -> str: mime, _ = mimetypes.guess_type(path.name) mime = mime or "application/octet-stream" diff --git a/agent/curator_backup.py b/agent/curator_backup.py index 8a65825464e49..4d17f3743c0a3 100644 --- a/agent/curator_backup.py +++ b/agent/curator_backup.py @@ -51,6 +51,10 @@ 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 + logger = logging.getLogger(__name__) @@ -733,13 +737,6 @@ def rollback(backup_id: Optional[str] = None) -> Tuple[bool, str, Optional[Path] # Human-readable summary for CLI # --------------------------------------------------------------------------- -def format_size(n: int) -> str: - for unit in ("B", "KB", "MB", "GB"): - if n < 1024 or unit == "GB": - return f"{n:.1f} {unit}" if unit != "B" else f"{n} B" - n /= 1024 - return f"{n:.1f} GB" - def summarize_backups() -> str: rows = list_backups() diff --git a/hermes_cli/backup.py b/hermes_cli/backup.py index 43748fd3e207c..98d0f41df1c1f 100644 --- a/hermes_cli/backup.py +++ b/hermes_cli/backup.py @@ -25,6 +25,10 @@ from typing import Any, Dict, List, Optional from hermes_constants import get_default_hermes_root, get_hermes_home, display_hermes_home +# Shared formatter; the private alias is kept because claw.py and the backup +# tests import ``_format_size`` from this module. +from hermes_cli.sizefmt import format_bytes as _format_size + logger = logging.getLogger(__name__) @@ -574,15 +578,6 @@ def copy_db_and_verify(src: Path, dst: Path) -> bool: # Backup # --------------------------------------------------------------------------- -def _format_size(nbytes: int) -> str: - """Human-readable file size.""" - for unit in ("B", "KB", "MB", "GB"): - if nbytes < 1024: - return f"{nbytes:.1f} {unit}" if unit != "B" else f"{nbytes} {unit}" - nbytes /= 1024 - return f"{nbytes:.1f} TB" - - def run_backup(args) -> None: """Create a zip backup of the Hermes home directory.""" hermes_root = get_default_hermes_root() diff --git a/hermes_cli/checkpoints.py b/hermes_cli/checkpoints.py index 5e9cbe1bdc9c1..cecd22df2d7e0 100644 --- a/hermes_cli/checkpoints.py +++ b/hermes_cli/checkpoints.py @@ -27,17 +27,14 @@ from datetime import datetime from pathlib import Path from typing import Any, Optional +from hermes_cli.sizefmt import format_bytes -def _fmt_bytes(n: int) -> str: - units = ("B", "KB", "MB", "GB", "TB") - size = float(n or 0) - for unit in units: - if size < 1024 or unit == units[-1]: - if unit == "B": - return f"{int(size)} {unit}" - return f"{size:.1f} {unit}" - size /= 1024 - return f"{size:.1f} TB" + +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) def _fmt_ts(ts: Any) -> str: diff --git a/hermes_cli/doctor.py b/hermes_cli/doctor.py index b6dd56c8780dc..202942b476d4f 100644 --- a/hermes_cli/doctor.py +++ b/hermes_cli/doctor.py @@ -320,19 +320,9 @@ def check_info(text: str): STATE_DB_SIZE_WARN_BYTES = 1 * 1024 * 1024 * 1024 # 1 GiB logical size -def _human_bytes(n) -> str: - """1234567 → '1.2 MB' (GB/MB/KB/B).""" - try: - n = int(n) - except (TypeError, ValueError): - return "?" - if n >= 1024 ** 3: - return f"{n / (1024 ** 3):.1f} GB" - if n >= 1024 ** 2: - return f"{n / (1024 ** 2):.1f} MB" - if n >= 1024: - return f"{n / 1024:.1f} KB" - return f"{n} B" +# Shared byte formatter (private alias: this module's rendering helpers and +# tests refer to ``_human_bytes``). +from hermes_cli.sizefmt import format_bytes as _human_bytes def _render_state_db_stats(stats: dict, holders=None) -> list: diff --git a/hermes_cli/sizefmt.py b/hermes_cli/sizefmt.py new file mode 100644 index 0000000000000..99cf9e8c83634 --- /dev/null +++ b/hermes_cli/sizefmt.py @@ -0,0 +1,38 @@ +"""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. + +Two in-repo formatters intentionally do NOT delegate here: + +* ``hermes_cli/session_recovery.py`` uses binary suffixes (KiB/MiB/GiB) + throughout its recovery report — a deliberate, self-consistent style. +* ``gateway/platforms/qqbot/chunked_upload.py`` renders bytes with one + decimal ("100.0 B", pinned by tests) inside a self-contained upload + protocol module. +""" + +from __future__ import annotations + + +def format_bytes(n, *, fallback: str = "?") -> 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. + """ + try: + size = float(n) + except (TypeError, ValueError): + return fallback + if size < 1024: + return f"{int(size)} B" + for unit in ("KB", "MB", "GB", "TB"): + size /= 1024.0 + if size < 1024 or unit == "TB": + return f"{size:.1f} {unit}" + return f"{size:.1f} TB" # unreachable; keeps type-checkers satisfied diff --git a/tests/hermes_cli/test_sizefmt.py b/tests/hermes_cli/test_sizefmt.py new file mode 100644 index 0000000000000..f2ad56a927b21 --- /dev/null +++ b/tests/hermes_cli/test_sizefmt.py @@ -0,0 +1,63 @@ +"""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. +""" + +import pytest + +from hermes_cli.sizefmt import format_bytes + + +@pytest.mark.parametrize( + "n,expected", + [ + (0, "0 B"), + (1, "1 B"), + (512, "512 B"), + (1023, "1023 B"), + (1024, "1.0 KB"), + (1536, "1.5 KB"), + (1024**2, "1.0 MB"), + (1234567, "1.2 MB"), + (1024**3, "1.0 GB"), + (1024**4, "1.0 TB"), + (2 * 1024**4, "2.0 TB"), + ], +) +def test_format_bytes_tiers(n, expected): + assert format_bytes(n) == expected + + +def test_format_bytes_tb_tier_not_gb_overflow(): + """The old doctor/context_references/curator_backup copies topped out at + GB and rendered 1 TiB as '1024.0 GB' — the shared helper must not.""" + assert "TB" in format_bytes(1024**4) + assert "1024" not in format_bytes(1024**4) + + +def test_format_bytes_never_raises(): + 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 + 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)