hermes-admin/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 28 Packages Projects Releases Wiki Activity

refactor(tools): compact file_tools companions; one eviction helper in file_state

Browse Source
This commit is contained in:
Teknium
2026-09-02 22:44:46 -07:00
parent b15ac86686
commit 8d6232878e
4 changed files with 192 additions and 392 deletions
Download Patch File Download Diff File Expand all files Collapse all files

149
tools/file_tools_read_tracking.py
View File

@@ -1,23 +1,12 @@
"""Per-task read/search bookkeeping for the file tools.
Process-lifetime state behind ``read_file`` / ``search_files`` / ``write_file`` /
``patch``; ``tools.file_tools`` re-imports every name here. Per task_id
``_read_tracker`` stores:
last_key / consecutive most recent read-or-search key and its repeat count
(loop detection; reset by any OTHER tool call).
read_history set of (path, offset, limit) — diagnostic summaries only.
dedup (resolved_path, offset, limit) -> mtime; skip identical
re-reads of unchanged files. Cleared on context
compression (the original content was summarised away).
dedup_hits per-key count of stub returns, to break stub loops.
read_timestamps resolved_path -> mtime at last read/write by this task;
write/patch warn when the file changed underneath.
not_found (op, resolved_path) -> (monotonic, cached error JSON);
short-TTL negative cache for retried missing paths.
Every container is hard-capped (``_cap_read_tracker_data``) so a long CLI
session accretes a few hundred KB at most instead of ~1.5MB per 10k reads.
Process-lifetime state behind read_file/search_files/write_file/patch;
``tools.file_tools`` re-imports every name here. Per task_id ``_read_tracker``
stores: ``last_key``/``consecutive`` (loop detection; reset by any OTHER tool
call), ``read_history`` (diagnostics), ``dedup`` (key -> mtime; cleared on
context compression), ``dedup_hits`` (stub-loop breaker), ``read_timestamps``
(staleness warnings) and ``not_found`` (short-TTL negative cache). Every
container is hard-capped (``_cap_read_tracker_data``) so long sessions stay small.
"""
import logging
@@ -25,6 +14,7 @@ import os
import threading
import time
from tools.file_state import _evict_oldest
from tools.file_tools_paths import _authoritative_workspace_root, _resolve_path_for_task
logger = logging.getLogger("tools.file_tools")
@@ -33,8 +23,7 @@ _read_tracker_lock = threading.Lock()
_read_tracker: dict = {}
# Consecutive patch failures per (task_id, resolved_path); escalates the hint
# when the model keeps failing the same file (stale view, ambiguous old_string).
# Reset on a successful patch to that path.
# when the model keeps failing the same file. Reset on a successful patch.
_patch_failure_lock = threading.Lock()
_patch_failure_tracker: dict = {} # {task_id: {resolved_path: count}}
_PATCH_FAILURE_PATHS_CAP = 64
@@ -48,24 +37,17 @@ _NOT_FOUND_CAP = 500
_NOT_FOUND_TTL_SECONDS = 60.0 # a path that didn't exist may be created soon
def _new_task_data() -> dict:
return {
"last_key": None, "consecutive": 0,
"read_history": set(), "dedup": {},
"dedup_hits": {}, "read_timestamps": {},
}
def _task_data(task_id: str) -> dict:
"""Get-or-create the tracker entry for *task_id*, back-filling any missing keys.
Must be called with ``_read_tracker_lock`` held. Entries created by older
code paths (or injected by tests) may lack the newer containers.
"""
task_data = _read_tracker.setdefault(task_id, _new_task_data())
for key, factory in (("dedup", dict), ("dedup_hits", dict), ("read_timestamps", dict)):
if key not in task_data:
task_data[key] = factory()
task_data = _read_tracker.setdefault(task_id, {
"last_key": None, "consecutive": 0, "read_history": set(),
})
for key in ("dedup", "dedup_hits", "read_timestamps"):
task_data.setdefault(key, {})
return task_data
@@ -74,11 +56,8 @@ def _record_patch_failure(task_id: str, resolved_path: str) -> int:
with _patch_failure_lock:
task_failures = _patch_failure_tracker.setdefault(task_id, {})
# Evict the oldest entry once a task has failed on many distinct files.
if len(task_failures) >= _PATCH_FAILURE_PATHS_CAP and resolved_path not in task_failures:
try:
del task_failures[next(iter(task_failures))]
except StopIteration:
pass
if resolved_path not in task_failures:
_evict_oldest(task_failures, _PATCH_FAILURE_PATHS_CAP - 1)
task_failures[resolved_path] = task_failures.get(resolved_path, 0) + 1
return task_failures[resolved_path]
@@ -89,29 +68,10 @@ def _reset_patch_failures(task_id: str, resolved_paths: list) -> None:
return
with _patch_failure_lock:
task_failures = _patch_failure_tracker.get(task_id)
if not task_failures:
return
for rp in resolved_paths:
for rp in resolved_paths if task_failures else ():
task_failures.pop(rp, None)
def _evict_oldest(container, cap: int) -> None:
"""Pop entries until *container* is within *cap*.
Sets pop arbitrary entries (they only feed diagnostic summaries); dicts pop
oldest by insertion order. An evicted entry costs one redundant re-send
(dedup) or one non-mtime staleness check — graceful degradation, not a bug.
"""
for _ in range(len(container) - cap):
try:
if isinstance(container, set):
container.pop()
else:
container.pop(next(iter(container)))
except (StopIteration, KeyError):
break
def _cap_read_tracker_data(task_data: dict) -> None:
"""Enforce size caps on the per-task sub-containers. Call with ``_read_tracker_lock`` held."""
# Caps are read at call time so tests can monkeypatch the module constants.
@@ -127,6 +87,14 @@ def _cap_read_tracker_data(task_data: dict) -> None:
_evict_oldest(container, cap)
def _pop_not_found(op: str, resolved_str: str, task_id: str) -> None:
"""Drop the negative-cache entry for *(op, resolved_str)*. Lock must be held."""
task_data = _read_tracker.get(task_id)
nf = task_data.get("not_found") if task_data else None
if nf:
nf.pop((op, resolved_str), None)
def _check_not_found_cache(op: str, resolved_str: str, task_id: str) -> str | None:
"""Return cached not-found JSON for *(op, resolved_str)* if still fresh.
@@ -136,28 +104,20 @@ def _check_not_found_cache(op: str, resolved_str: str, task_id: str) -> str | No
"""
with _read_tracker_lock:
task_data = _read_tracker.get(task_id)
if not task_data:
return None
nf = task_data.get("not_found")
if not nf:
return None
entry = nf.get((op, resolved_str))
entry = (task_data.get("not_found") or {}).get((op, resolved_str)) if task_data else None
if entry is None:
return None
ts, cached_json = entry
if time.monotonic() - ts > _NOT_FOUND_TTL_SECONDS:
nf.pop((op, resolved_str), None)
_pop_not_found(op, resolved_str, task_id)
return None
# The path may have been created since the miss was cached (terminal,
# another agent, ...) — the "check → create → read" pattern is common, so
# serving a stale miss breaks it. The stat runs OUTSIDE the global tracker
# lock: a hung stat on a dead network mount must not stall every task.
# another agent, ...) — "check → create → read" is common, so serving a
# stale miss breaks it. The stat runs OUTSIDE the global tracker lock: a
# hung stat on a dead network mount must not stall every task.
if os.path.exists(resolved_str):
with _read_tracker_lock:
task_data = _read_tracker.get(task_id)
nf = task_data.get("not_found") if task_data else None
if nf:
nf.pop((op, resolved_str), None)
_pop_not_found(op, resolved_str, task_id)
return None
return cached_json
@@ -166,8 +126,7 @@ def _record_not_found(op: str, resolved_str: str, task_id: str, error_json: str)
"""Cache a not-found error so the next *op* call for *resolved_str* skips I/O."""
with _read_tracker_lock:
task_data = _task_data(task_id)
nf = task_data.setdefault("not_found", {})
nf[(op, resolved_str)] = (time.monotonic(), error_json)
task_data.setdefault("not_found", {})[(op, resolved_str)] = (time.monotonic(), error_json)
_cap_read_tracker_data(task_data)
@@ -212,11 +171,9 @@ def notify_other_tool_call(task_id: str = "default"):
if task_data:
task_data["last_key"] = None
task_data["consecutive"] = 0
if "dedup_hits" in task_data:
task_data["dedup_hits"].clear()
nf = task_data.get("not_found")
if nf:
nf.clear()
for key in ("dedup_hits", "not_found"):
if task_data.get(key):
task_data[key].clear()
def _invalidate_dedup_for_path(filepath: str, task_id: str) -> None:
@@ -237,10 +194,8 @@ def _invalidate_dedup_for_path(filepath: str, task_id: str) -> None:
if dedup:
for k in [k for k in dedup if k[0] == resolved]:
del dedup[k]
nf = task_data.get("not_found")
if nf:
nf.pop(("read", resolved), None)
nf.pop(("search", resolved), None)
_pop_not_found("read", resolved, task_id)
_pop_not_found("search", resolved, task_id)
def _update_read_timestamp(filepath: str, task_id: str) -> None:
@@ -271,9 +226,7 @@ def _check_file_staleness(filepath: str, task_id: str) -> str | None:
return None
with _read_tracker_lock:
task_data = _read_tracker.get(task_id)
if not task_data:
return None
read_mtime = task_data.get("read_timestamps", {}).get(resolved)
read_mtime = task_data.get("read_timestamps", {}).get(resolved) if task_data else None
if read_mtime is None:
return None
try:
@@ -289,11 +242,8 @@ def _check_file_staleness(filepath: str, task_id: str) -> str | None:
return None
def _mark_verification_stale(
task_id: str,
resolved_paths: list[str],
session_id: str | None = None,
) -> None:
def _mark_verification_stale(task_id: str, resolved_paths: list[str],
session_id: str | None = None) -> None:
"""Best-effort note that successful edits made prior verification stale.
The workspace cwd is the first edited path's project root when one is
@@ -308,22 +258,9 @@ def _mark_verification_stale(
from agent.coding_context import project_facts_for
from agent.verification_evidence import mark_workspace_edited
cwd = None
for path in paths:
try:
candidate = str(Path(path).parent)
except Exception:
continue
if project_facts_for(candidate):
cwd = candidate
break
if cwd is None:
cwd = _authoritative_workspace_root(task_id)
if cwd is None:
try:
cwd = str(Path(paths[0]).parent)
except Exception:
cwd = None
parents = [str(Path(p).parent) for p in paths]
cwd = (next((c for c in parents if project_facts_for(c)), None)
or _authoritative_workspace_root(task_id) or parents[0])
mark_workspace_edited(session_id=session_id or task_id, cwd=cwd, paths=paths)
except Exception:
logger.debug("verification stale marker failed", exc_info=True)