refactor: fold simplify findings — 6th copy in update_cmd, drop dead wrapper + speculative kwarg, behavior-contract tests
- Migrate the missed 6th inline formatter (update_cmd.py backup-size display) to the shared helper. - checkpoints._fmt_bytes: plain alias instead of a None-guard wrapper — every caller feeds ints from checkpoint_manager (all size fields initialize to 0), so the None path was dead defensive code. - Drop the fallback= kwarg (zero production callers; '?' default is the real inherited contract and stays). - curator_backup + context_references: call format_bytes directly (single internal call site each, zero external importers — alias was churn avoidance with nothing to avoid). backup/_format_size and doctor/_human_bytes keep their aliases (claw.py + tests pin the former; three call sites use the latter). - Reshape the loop so the trailing TB return is reachable (no dead line). - Tests: replace alias-identity assertions (ossified the delegation mechanism) with behavior-contract equality over a value sweep; mutation-checked red-green. - update_cmd parity: byte-identical B-GB vs the old inline loop; gains the TB tier.
This commit is contained in:
parent
7289898494
commit
973c14b57c
|
|
@ -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 (
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in New Issue