refactor(kanban): share one _retention_seconds helper across both gc sweeps

gc_events and gc_worker_logs each carried the same three lines (int()
coerce, `< 0` check, raise ValueError(_NEGATIVE_RETENTION_MSG.format(...)));
only the message string was hoisted. Replace the constant with a
sibling-local helper `_retention_seconds(older_than_seconds) -> int` that
owns coerce + check + raise, and call it from both sweeps.

WHY: the comment explaining the rule ("a negative window puts the cutoff in
the future, so 'older than cutoff' matches everything") is the reason for
the check, so it belongs on the check rather than on a format string; and
a third sweep now has one place to reuse instead of a block to paste.
Message text is unchanged, so the existing
`pytest.raises(ValueError, match="older_than_seconds")` tests pass as-is.

Finding: simplify/D.reuse.md #1 + simplify/D.quality.md #1
(hermes_cli/kanban_db.py:4285-4287, :4303-4305).

Proof: mutating the helper's `< 0` to `< -10**12` fails
test_gc_events_rejects_negative_window and
test_gc_worker_logs_rejects_negative_window (DID NOT RAISE); head green.
This commit is contained in:
kshitijk4poor
2026-09-22 13:01:33 +05:30
committed by kshitij
parent 1be926dbcd
commit e8ae808bd3

View File

@@ -4271,9 +4271,20 @@ def task_age(task: Task) -> dict:
# --- Retention + garbage collection ---
# Shared by both gc sweeps: a negative window puts the cutoff in the future, so
# "older than cutoff" matches every row / file instead of none.
_NEGATIVE_RETENTION_MSG = "older_than_seconds must be >= 0, got {!r}: a negative retention selects everything."
def _retention_seconds(older_than_seconds: int) -> int:
"""Normalise a gc retention window, rejecting negatives.
Shared by both gc sweeps: a negative window puts the cutoff in the future,
so "older than cutoff" would match every row / file instead of none —
refuse before any sweep runs.
"""
older_than_seconds = int(older_than_seconds)
if older_than_seconds < 0:
raise ValueError(
f"older_than_seconds must be >= 0, got {older_than_seconds!r}: "
"a negative retention selects everything."
)
return older_than_seconds
def gc_events(conn: sqlite3.Connection, *, older_than_seconds: int = 30 * 24 * 3600) -> int:
@@ -4282,10 +4293,7 @@ def gc_events(conn: sqlite3.Connection, *, older_than_seconds: int = 30 * 24 * 3
``older_than_seconds=0`` means everything older than now; the CLI maps
``--event-retention-days 0`` to "disabled" before calling this.
"""
older_than_seconds = int(older_than_seconds)
if older_than_seconds < 0:
raise ValueError(_NEGATIVE_RETENTION_MSG.format(older_than_seconds))
cutoff = int(time.time()) - older_than_seconds
cutoff = int(time.time()) - _retention_seconds(older_than_seconds)
with write_txn(conn):
cur = conn.execute(
"DELETE FROM task_events WHERE created_at < ? AND kind != 'decomposed' AND task_id IN "
@@ -4300,9 +4308,7 @@ def gc_worker_logs(*, older_than_seconds: int = 30 * 24 * 3600, board: Optional[
``older_than_seconds=0`` means everything older than now; the CLI maps
``--log-retention-days 0`` to "disabled" before calling this.
"""
older_than_seconds = int(older_than_seconds)
if older_than_seconds < 0:
raise ValueError(_NEGATIVE_RETENTION_MSG.format(older_than_seconds))
older_than_seconds = _retention_seconds(older_than_seconds)
log_dir = worker_logs_dir(board=board)
if not log_dir.exists():
return 0