fix: paged, extracted and post-compaction reads count as a write_file baseline
The stale-overwrite refusal made write_file permanently unusable for any existing file it could not show in one read_file page: every >2000-line (or >100K-char) page was recorded as partial, no full baseline ever existed, and the refusal told the model to "re-read the whole file", which the tool cannot do. Track the line ranges each task pages through per path at one mtime; contiguous pages from line 1 to total_lines are a full read (a new mtime between pages restarts the coverage). The same gap hit two siblings: the extracted-document branch (.ipynb, text-authorable) returned before any read bookkeeping, so an existing notebook could never be overwritten; and reset_file_dedup dropped every baseline on compaction while keeping read_timestamps, so every write after compaction was refused even for files unchanged on disk. Baselines now survive compaction exactly like the dedup mtime map does — only while the recorded mtime still matches. Refusal texts no longer embed the pre-PR "Warning: … Consider re-reading" copy inside "Refusing to overwrite", and every refusal names a recovery the model can perform: read the remaining pages, or use patch.
This commit is contained in:
@@ -7,10 +7,12 @@ call), ``read_history`` (diagnostics), ``dedup`` (key -> mtime; survives context
|
||||
compression), ``dedup_generation_reads`` (keys whose full content was served since
|
||||
the last compaction boundary; cleared on compression so one recovery read returns
|
||||
full content), ``dedup_hits`` (stub-loop breaker), ``read_timestamps``
|
||||
(staleness warnings), ``full_write_baselines`` (resolved paths whose whole-file
|
||||
content this task saw via a full unredacted read_file or wrote via write_file;
|
||||
required before write_file may overwrite an existing file — patch never
|
||||
qualifies) and ``not_found`` (short-TTL negative cache). Every
|
||||
(staleness warnings), ``read_coverage`` (per resolved path: the line ranges the
|
||||
task has paged through at one mtime — contiguous pages that reach the last line
|
||||
count as a whole-file read), ``full_write_baselines`` (resolved paths whose
|
||||
whole-file content this task saw via unredacted read_file page(s) or wrote via
|
||||
write_file; required before write_file may overwrite an existing file — patch
|
||||
never qualifies) and ``not_found`` (short-TTL negative cache). Every
|
||||
container is hard-capped (``_cap_read_tracker_data``) so long sessions stay small.
|
||||
"""
|
||||
|
||||
@@ -19,7 +21,7 @@ import os
|
||||
import threading
|
||||
import time
|
||||
|
||||
from tools.file_state import _evict_oldest
|
||||
from tools.file_state import _evict_oldest, _mtime_or_none
|
||||
from tools.file_tools_paths import _authoritative_workspace_root, _resolve_path_for_task
|
||||
|
||||
logger = logging.getLogger("tools.file_tools")
|
||||
@@ -48,7 +50,7 @@ def _task_data(task_id: str) -> dict:
|
||||
(search_tool / tests create partial entries). Lock must be held."""
|
||||
task_data = _read_tracker.setdefault(task_id, {
|
||||
"last_key": None, "consecutive": 0, "read_history": set()})
|
||||
for key in ("dedup", "dedup_hits", "read_timestamps"):
|
||||
for key in ("dedup", "dedup_hits", "read_timestamps", "read_coverage"):
|
||||
task_data.setdefault(key, {})
|
||||
for key in ("dedup_generation_reads", "full_write_baselines"):
|
||||
task_data.setdefault(key, set())
|
||||
@@ -85,6 +87,7 @@ def _cap_read_tracker_data(task_data: dict) -> None:
|
||||
("dedup_hits", _DEDUP_CAP),
|
||||
("dedup_generation_reads", _DEDUP_CAP),
|
||||
("read_timestamps", _READ_TIMESTAMPS_CAP),
|
||||
("read_coverage", _READ_TIMESTAMPS_CAP),
|
||||
("full_write_baselines", _FULL_WRITE_BASELINES_CAP),
|
||||
("not_found", _NOT_FOUND_CAP)):
|
||||
container = task_data.get(key)
|
||||
@@ -156,7 +159,10 @@ def reset_file_dedup(task_id: str = None):
|
||||
files keep returning stubs instead of re-bloating the reclaimed context; the
|
||||
generation-read set is cleared so the FIRST unchanged read of each key after
|
||||
compaction returns full content the summary may have dropped. Stub-hit counters
|
||||
are cleared so the hard block restarts fresh."""
|
||||
are cleared so the hard block restarts fresh. write_file baselines survive
|
||||
exactly like the dedup map does — for files whose mtime still matches the
|
||||
stamp this task recorded; a baseline whose file changed underneath is dropped
|
||||
(the stat runs outside the lock so a hung mount cannot stall other tasks)."""
|
||||
with _read_tracker_lock:
|
||||
if task_id:
|
||||
targets = [_read_tracker[task_id]] if _read_tracker.get(task_id) else []
|
||||
@@ -166,9 +172,13 @@ def reset_file_dedup(task_id: str = None):
|
||||
if "dedup_hits" in task_data:
|
||||
task_data["dedup_hits"].clear()
|
||||
task_data.setdefault("dedup_generation_reads", set()).clear()
|
||||
# The summary may have dropped the exact bytes the baseline vouched
|
||||
# for: a full overwrite needs a fresh read_file after compaction.
|
||||
task_data.setdefault("full_write_baselines", set()).clear()
|
||||
candidates = [(task_data, list(task_data.get("full_write_baselines", ())),
|
||||
dict(task_data.get("read_timestamps", {}))) for task_data in targets]
|
||||
for task_data, baselines, stamps in candidates:
|
||||
changed = {p for p in baselines if _mtime_or_none(p) != stamps.get(p)}
|
||||
if changed:
|
||||
with _read_tracker_lock:
|
||||
task_data.get("full_write_baselines", set()).difference_update(changed)
|
||||
|
||||
|
||||
def notify_other_tool_call(task_id: str = "default"):
|
||||
@@ -244,22 +254,54 @@ def _has_full_write_baseline(resolved: str, task_id: str) -> bool:
|
||||
return str(resolved) in task_data.get("full_write_baselines", set())
|
||||
|
||||
|
||||
def _check_file_staleness(filepath: str, task_id: str) -> str | None:
|
||||
"""Warn (don't block) when the file's mtime changed since this task last read it.
|
||||
``None`` when never read, fresh, or unstattable (a deleted file is the write's problem)."""
|
||||
_READ_COVERAGE_RANGES_CAP = 256
|
||||
|
||||
|
||||
def _note_read_coverage(task_data: dict, resolved: str, mtime: float, start: int, end: int,
|
||||
total_lines, redacted: bool) -> tuple[bool, bool]:
|
||||
"""Merge the page ``start..end`` into this task's coverage of *resolved* and return
|
||||
``(complete, redacted_any)``: whether pages taken at this same *mtime* now reach from
|
||||
line 1 to *total_lines*, and whether any of them came back redacted. A file too large
|
||||
for one read_file page (>2000 lines / the char budget) can only ever be seen this
|
||||
way, so paging through it must count as a whole-file read. A new mtime restarts the
|
||||
coverage (the earlier pages describe a file that no longer exists). Lock must be held."""
|
||||
coverage = task_data.setdefault("read_coverage", {})
|
||||
entry = coverage.get(resolved)
|
||||
if entry is None or entry["mtime"] != mtime or len(entry["ranges"]) > _READ_COVERAGE_RANGES_CAP:
|
||||
entry = coverage[resolved] = {"mtime": mtime, "ranges": [], "redacted": False}
|
||||
entry["redacted"] = entry["redacted"] or redacted
|
||||
merged: list[tuple[int, int]] = []
|
||||
for s, e in sorted(entry["ranges"] + [(start, end)]):
|
||||
if merged and s <= merged[-1][1] + 1:
|
||||
merged[-1] = (merged[-1][0], max(merged[-1][1], e))
|
||||
else:
|
||||
merged.append((s, e))
|
||||
entry["ranges"] = merged
|
||||
complete = (isinstance(total_lines, int) and total_lines > 0
|
||||
and merged[0][0] <= 1 and merged[0][1] >= total_lines)
|
||||
return complete, entry["redacted"]
|
||||
|
||||
|
||||
def _read_mtime_drifted(filepath: str, task_id: str) -> bool:
|
||||
"""True when the file's mtime changed since this task last read it. False when
|
||||
never read, fresh, or unstattable (a deleted file is the write's problem)."""
|
||||
resolved = _resolved_or_none(filepath, task_id)
|
||||
if resolved is None:
|
||||
return None
|
||||
return False
|
||||
with _read_tracker_lock:
|
||||
task_data = _read_tracker.get(task_id)
|
||||
read_mtime = task_data.get("read_timestamps", {}).get(resolved) if task_data else None
|
||||
if read_mtime is None:
|
||||
return None
|
||||
return False
|
||||
try:
|
||||
current_mtime = os.path.getmtime(resolved)
|
||||
return os.path.getmtime(resolved) != read_mtime
|
||||
except OSError:
|
||||
return None
|
||||
if current_mtime != read_mtime:
|
||||
return False
|
||||
|
||||
|
||||
def _check_file_staleness(filepath: str, task_id: str) -> str | None:
|
||||
"""Warn (don't block) when the file's mtime changed since this task last read it."""
|
||||
if _read_mtime_drifted(filepath, task_id):
|
||||
return (
|
||||
f"Warning: {filepath} was modified since you last read it "
|
||||
"(external edit or concurrent agent). The content you read may be "
|
||||
|
||||
Reference in New Issue
Block a user