Skip to content

Commit 3bac95e

Browse files
Nichol4sclaude
andcommitted
[argus] loop_detector: edit-aware reset + raise default thresholds 5/8
Two changes that together stop the loop detector from killing legitimate render-and-verify iteration. Observed in argus thread ff3c7aa8 (WebGL minion task on local-qwen): hard-stop fired on the 5th invocation of an identical render.py verifier command, even though the model had made six distinct str_replace edits to the artifact between renders. The verifier *has to* be re-run after each edit — that is the whole point of the iterate loop — but the hash-based detector treated those reruns as identical-call repetition. 1. Edit-aware reset (the principled fix). Track per-thread the set of paths mutated by write_file / str_replace. Before counting a new tool call's hash, check whether it references any of those mutated paths (path arg match, or substring match against command / cmd). If it does, clear that hash from the sliding window and consume the matched paths from the mutated set. The signal we want to catch is "doing the same thing in the same state" — an intervening edit means the state changed, so a re-probe is fresh observation, not a loop. Within- message edits don't reset their own hash (mutating paths are recorded *after* the check), so a coupled [str_replace, bash] in one message only resets a *future* identical bash. 2. Raise defaults: warn 3 -> 5, hard 5 -> 8. Cheap belt-and-suspenders for cases the edit-aware reset doesn't cover (verifiers that touch paths the model didn't explicitly edit, model-side noise, etc). This is a local single-user setup -- token burn from a few extra wasted iterations is preferable to killing a run one fix away from completion. Tests: 8 new cases in TestEditAwareReset covering the render-and-verify pattern, unrelated-path edits not resetting, single-mutation-consumed- once semantics, within-message coupling, per-thread isolation, and a sanity check on the new default thresholds. All 56 loop-detection tests pass; full backend suite is green modulo unrelated env-layout failures (/tmp/scripts, /tmp/skills) that also fail on stock argus@743b7f21. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 743b7f2 commit 3bac95e

2 files changed

Lines changed: 245 additions & 4 deletions

File tree

backend/packages/harness/deerflow/agents/middlewares/loop_detection_middleware.py

Lines changed: 83 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@
2828
logger = logging.getLogger(__name__)
2929

3030
# Defaults — can be overridden via constructor
31-
_DEFAULT_WARN_THRESHOLD = 3 # inject warning after 3 identical calls
32-
_DEFAULT_HARD_LIMIT = 5 # force-stop after 5 identical calls
31+
_DEFAULT_WARN_THRESHOLD = 5 # inject warning after 5 identical calls
32+
_DEFAULT_HARD_LIMIT = 8 # force-stop after 8 identical calls
3333
_DEFAULT_WINDOW_SIZE = 20 # track last N tool calls
3434
_DEFAULT_MAX_TRACKED_THREADS = 100 # LRU eviction limit
3535
_DEFAULT_TOOL_FREQ_WARN = 30 # warn after 30 calls to the same tool type
@@ -105,6 +105,42 @@ def _stable_tool_key(name: str, args: dict, fallback_key: str | None) -> str:
105105
return json.dumps(args, sort_keys=True, default=str)
106106

107107

108+
_MUTATING_TOOLS = frozenset({"write_file", "str_replace"})
109+
110+
111+
def _mutated_path(name: str, args: dict) -> str | None:
112+
"""Return the file path mutated by *name* (write_file / str_replace), or None."""
113+
if name not in _MUTATING_TOOLS:
114+
return None
115+
path = args.get("path")
116+
if isinstance(path, str) and path:
117+
return path
118+
return None
119+
120+
121+
def _matches_mutated(args: dict, mutated_paths: set[str]) -> set[str]:
122+
"""Return the subset of *mutated_paths* that this tool call references.
123+
124+
A call references a mutated path when:
125+
- its ``path`` arg equals the mutated path, or
126+
- its ``command``/``cmd`` arg contains the mutated path as a substring
127+
(verifier scripts, cat, grep, python -c "...", etc).
128+
"""
129+
if not mutated_paths:
130+
return set()
131+
matched: set[str] = set()
132+
arg_path = args.get("path")
133+
cmd = args.get("command") or args.get("cmd")
134+
cmd_str = cmd if isinstance(cmd, str) else None
135+
for mp in mutated_paths:
136+
if arg_path == mp:
137+
matched.add(mp)
138+
continue
139+
if cmd_str is not None and mp in cmd_str:
140+
matched.add(mp)
141+
return matched
142+
143+
108144
def _hash_tool_calls(tool_calls: list[dict]) -> str:
109145
"""Deterministic hash of a set of tool calls (name + stable key).
110146
@@ -153,9 +189,9 @@ class LoopDetectionMiddleware(AgentMiddleware[AgentState]):
153189
154190
Args:
155191
warn_threshold: Number of identical tool call sets before injecting
156-
a warning message. Default: 3.
192+
a warning message. Default: 5.
157193
hard_limit: Number of identical tool call sets before stripping
158-
tool_calls entirely. Default: 5.
194+
tool_calls entirely. Default: 8.
159195
window_size: Size of the sliding window for tracking calls.
160196
Default: 20.
161197
max_tracked_threads: Maximum number of threads to track before
@@ -191,6 +227,11 @@ def __init__(
191227
# Per-thread, per-tool-type cumulative call counts
192228
self._tool_freq: dict[str, dict[str, int]] = defaultdict(lambda: defaultdict(int))
193229
self._tool_freq_warned: dict[str, set[str]] = defaultdict(set)
230+
# Per-thread set of paths mutated since the last reset. A subsequent call
231+
# that references one of these paths clears identical-hash history (the
232+
# file changed, so re-probing it isn't a loop), and consumes the path so
233+
# a fresh edit is needed to reset again.
234+
self._mutated_paths: dict[str, set[str]] = defaultdict(set)
194235

195236
def _get_thread_id(self, runtime: Runtime) -> str:
196237
"""Extract thread_id from runtime context for per-thread tracking."""
@@ -209,6 +250,7 @@ def _evict_if_needed(self) -> None:
209250
self._warned.pop(evicted_id, None)
210251
self._tool_freq.pop(evicted_id, None)
211252
self._tool_freq_warned.pop(evicted_id, None)
253+
self._mutated_paths.pop(evicted_id, None)
212254
logger.debug("Evicted loop tracking for thread %s (LRU)", evicted_id)
213255

214256
def _track_and_check(self, state: AgentState, runtime: Runtime) -> tuple[str | None, bool]:
@@ -220,6 +262,11 @@ def _track_and_check(self, state: AgentState, runtime: Runtime) -> tuple[str | N
220262
called many times with varying arguments (e.g. ``read_file``
221263
on 40 different files).
222264
265+
Edit-aware reset: if any tool call references a path that was mutated
266+
(by a prior write_file/str_replace) since the last reset for that path,
267+
clear identical-hash history before counting. The file changed, so the
268+
verifier isn't probing the same state.
269+
223270
Returns:
224271
(warning_message_or_none, should_hard_stop)
225272
"""
@@ -247,13 +294,43 @@ def _track_and_check(self, state: AgentState, runtime: Runtime) -> tuple[str | N
247294
self._evict_if_needed()
248295

249296
history = self._history[thread_id]
297+
298+
# Edit-aware reset: collect mutated paths this message *probes*,
299+
# then drop those paths from the mutated set and clear prior
300+
# identical-hash occurrences. Mutating calls in this same message
301+
# are recorded *after* this check, so they only affect the next
302+
# message — within-message edits don't reset their own hash.
303+
mutated = self._mutated_paths.get(thread_id)
304+
if mutated:
305+
consumed: set[str] = set()
306+
for tc in tool_calls:
307+
args, _ = _normalize_tool_call_args(tc.get("args", {}))
308+
consumed |= _matches_mutated(args, mutated)
309+
if consumed:
310+
history[:] = [h for h in history if h != call_hash]
311+
mutated -= consumed
312+
if not mutated:
313+
self._mutated_paths.pop(thread_id, None)
314+
self._warned.get(thread_id, set()).discard(call_hash)
315+
250316
history.append(call_hash)
251317
if len(history) > self.window_size:
252318
history[:] = history[-self.window_size :]
253319

254320
count = history.count(call_hash)
255321
tool_names = [tc.get("name", "?") for tc in tool_calls]
256322

323+
# Record mutating paths *after* the edit-aware check above, so
324+
# mutating calls in this same message only reset hash counts on
325+
# *future* messages, not their own. Done before the warn/hard-stop
326+
# branches so injected warnings still produce a fresh mutated set
327+
# for whatever the model does next.
328+
for tc in tool_calls:
329+
args, _ = _normalize_tool_call_args(tc.get("args", {}))
330+
mp = _mutated_path(tc.get("name", ""), args)
331+
if mp:
332+
self._mutated_paths[thread_id].add(mp)
333+
257334
# --- Layer 1: hash-based (identical call sets) ---
258335
if count >= self.hard_limit:
259336
logger.error(
@@ -393,8 +470,10 @@ def reset(self, thread_id: str | None = None) -> None:
393470
self._warned.pop(thread_id, None)
394471
self._tool_freq.pop(thread_id, None)
395472
self._tool_freq_warned.pop(thread_id, None)
473+
self._mutated_paths.pop(thread_id, None)
396474
else:
397475
self._history.clear()
398476
self._warned.clear()
399477
self._tool_freq.clear()
478+
self._mutated_paths.clear()
400479
self._tool_freq_warned.clear()

backend/tests/test_loop_detection_middleware.py

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -684,3 +684,165 @@ def test_hash_detection_takes_priority(self):
684684
msg = result["messages"][0]
685685
assert isinstance(msg, AIMessage)
686686
assert _HARD_STOP_MSG in msg.content
687+
688+
689+
class TestEditAwareReset:
690+
"""Edit-aware reset: an interleaved write_file/str_replace to a path that a
691+
later call references should clear that call's hash from history. The
692+
classic case is a render-and-verify loop where the model edits the artifact
693+
between identical bash invocations of the verifier script.
694+
"""
695+
696+
@staticmethod
697+
def _bash(cmd):
698+
return {"name": "bash", "id": f"call_{hash(cmd) & 0xffff}", "args": {"command": cmd}}
699+
700+
@staticmethod
701+
def _str_replace(path, old="A", new="B"):
702+
return {
703+
"name": "str_replace",
704+
"id": f"call_sr_{hash((path, old, new)) & 0xffff}",
705+
"args": {"path": path, "old_str": old, "new_str": new},
706+
}
707+
708+
@staticmethod
709+
def _write_file(path, content="x"):
710+
return {
711+
"name": "write_file",
712+
"id": f"call_wf_{hash((path, content)) & 0xffff}",
713+
"args": {"path": path, "content": content},
714+
}
715+
716+
def test_str_replace_resets_bash_verifier_hash(self):
717+
"""The render-and-verify failure mode: model alternates render-bash and
718+
str_replace edits to the artifact, but counter for the bash hash should
719+
reset on each interleaved edit.
720+
"""
721+
mw = LoopDetectionMiddleware(warn_threshold=3, hard_limit=4)
722+
runtime = _make_runtime()
723+
artifact = "/mnt/user-data/outputs/minion.html"
724+
verify_cmd = f"python3 /mnt/skills/render.py {artifact}"
725+
bash_call = [self._bash(verify_cmd)]
726+
727+
# Pattern: render, edit, render, edit, render, edit, render, edit
728+
# Without reset, 4th render would hard-stop. With reset, never stops.
729+
# Each edit varies its content so str_replace hashes don't collide.
730+
for i in range(8):
731+
r = mw._apply(_make_state(tool_calls=bash_call), runtime)
732+
assert r is None, f"verifier bash should not trigger after edit (iter {i})"
733+
edit = self._str_replace(artifact, old=f"old_{i}", new=f"new_{i}")
734+
mw._apply(_make_state(tool_calls=[edit]), runtime)
735+
736+
def test_no_edit_still_triggers_loop(self):
737+
"""Sanity: without an interleaved edit, repeated identical bash still
738+
hits hard_limit. Edit-aware reset must not disable basic detection.
739+
"""
740+
mw = LoopDetectionMiddleware(warn_threshold=2, hard_limit=3)
741+
runtime = _make_runtime()
742+
bash_call = [self._bash("echo hello")]
743+
744+
for _ in range(2):
745+
mw._apply(_make_state(tool_calls=bash_call), runtime)
746+
result = mw._apply(_make_state(tool_calls=bash_call), runtime)
747+
assert result is not None
748+
assert _HARD_STOP_MSG in result["messages"][0].content
749+
750+
def test_edit_to_unrelated_path_does_not_reset(self):
751+
"""Edit to /tmp/other.html must NOT reset bash that probes /tmp/foo.html."""
752+
mw = LoopDetectionMiddleware(warn_threshold=2, hard_limit=3)
753+
runtime = _make_runtime()
754+
target = "/tmp/foo.html"
755+
verify = [self._bash(f"cat {target}")]
756+
757+
mw._apply(_make_state(tool_calls=verify), runtime)
758+
mw._apply(_make_state(tool_calls=[self._str_replace("/tmp/other.html")]), runtime)
759+
mw._apply(_make_state(tool_calls=verify), runtime)
760+
# 3rd identical verify still triggers hard stop — edit was to a
761+
# different path
762+
result = mw._apply(_make_state(tool_calls=verify), runtime)
763+
assert result is not None
764+
assert _HARD_STOP_MSG in result["messages"][0].content
765+
766+
def test_edit_to_path_arg_resets(self):
767+
"""A str_replace path that matches the next call's path arg (not a bash
768+
substring) should also reset.
769+
"""
770+
mw = LoopDetectionMiddleware(warn_threshold=2, hard_limit=3)
771+
runtime = _make_runtime()
772+
target = "/tmp/foo.py"
773+
read_call = [{"name": "read_file", "id": "rf1", "args": {"path": target}}]
774+
775+
mw._apply(_make_state(tool_calls=read_call), runtime)
776+
mw._apply(_make_state(tool_calls=read_call), runtime)
777+
mw._apply(_make_state(tool_calls=[self._write_file(target)]), runtime)
778+
# Next read_file of the same path: prior history cleared, no warn/stop
779+
r = mw._apply(_make_state(tool_calls=read_call), runtime)
780+
assert r is None
781+
782+
def test_edit_consumed_by_one_reset_only(self):
783+
"""A single mutation should only reset the *next* identical call. After
784+
that, subsequent identical calls accumulate again.
785+
"""
786+
mw = LoopDetectionMiddleware(warn_threshold=2, hard_limit=3)
787+
runtime = _make_runtime()
788+
target = "/tmp/x.html"
789+
verify = [self._bash(f"cat {target}")]
790+
791+
# Build up 2 identical verifies (warn_threshold=2 → next would warn)
792+
mw._apply(_make_state(tool_calls=verify), runtime)
793+
mw._apply(_make_state(tool_calls=verify), runtime)
794+
# Edit
795+
mw._apply(_make_state(tool_calls=[self._str_replace(target)]), runtime)
796+
# Reset consumes the mutation: next verify counts as fresh
797+
r = mw._apply(_make_state(tool_calls=verify), runtime)
798+
assert r is None
799+
# No further edit — three more verifies must hit hard_limit=3
800+
mw._apply(_make_state(tool_calls=verify), runtime)
801+
r = mw._apply(_make_state(tool_calls=verify), runtime)
802+
assert r is not None
803+
assert _HARD_STOP_MSG in r["messages"][0].content
804+
805+
def test_within_message_edit_does_not_reset_itself(self):
806+
"""A message containing both [str_replace(path), bash that touches path]
807+
must NOT use the same-message edit to reset the bash's history. The edit
808+
only affects *future* messages.
809+
"""
810+
mw = LoopDetectionMiddleware(warn_threshold=2, hard_limit=3)
811+
runtime = _make_runtime()
812+
target = "/tmp/y.html"
813+
bash_only = [self._bash(f"cat {target}")]
814+
coupled = [self._str_replace(target), self._bash(f"cat {target}")]
815+
816+
# First two bare bash calls
817+
mw._apply(_make_state(tool_calls=bash_only), runtime)
818+
mw._apply(_make_state(tool_calls=bash_only), runtime)
819+
# Couple a same-message edit + bash. The bash here has a *different
820+
# hash* than bash_only because the multiset includes str_replace, so
821+
# this doesn't directly add to the bash_only count. But it does record
822+
# /tmp/y.html as mutated → next bash_only call should be reset.
823+
mw._apply(_make_state(tool_calls=coupled), runtime)
824+
r = mw._apply(_make_state(tool_calls=bash_only), runtime)
825+
assert r is None
826+
827+
def test_per_thread_mutated_paths_isolated(self):
828+
"""Mutated paths recorded for thread A must not affect thread B."""
829+
mw = LoopDetectionMiddleware(warn_threshold=2, hard_limit=3)
830+
runtime_a = _make_runtime("thread-a")
831+
runtime_b = _make_runtime("thread-b")
832+
target = "/tmp/shared.html"
833+
verify = [self._bash(f"cat {target}")]
834+
835+
# Thread A: edit + verify (would reset)
836+
mw._apply(_make_state(tool_calls=[self._str_replace(target)]), runtime_a)
837+
# Thread B: two verifies, then a third — A's edit must NOT reset B
838+
mw._apply(_make_state(tool_calls=verify), runtime_b)
839+
mw._apply(_make_state(tool_calls=verify), runtime_b)
840+
r = mw._apply(_make_state(tool_calls=verify), runtime_b)
841+
assert r is not None
842+
assert _HARD_STOP_MSG in r["messages"][0].content
843+
844+
def test_default_thresholds_match_documented(self):
845+
"""Sanity: defaults are 5/8 (after option-4 bump)."""
846+
mw = LoopDetectionMiddleware()
847+
assert mw.warn_threshold == 5
848+
assert mw.hard_limit == 8

0 commit comments

Comments
 (0)