From 582a6328680557426e8c4a159ea4a97d10065a1b Mon Sep 17 00:00:00 2001 From: Hyeonsang Cho Date: Thu, 17 Sep 2026 10:36:55 +0900 Subject: [PATCH] fix(agents): key read_file loop detection on its exact line window (#5486) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Layer 1 quantized read_file's line range into 200-line buckets, which erased the offset inside a bucket: every read shorter than a bucket collapsed onto its neighbours. Five sequential 40-line reads hashed identically and tripped the hard stop, ending the run with a forced final answer and stop_reason=loop_capped — on exactly the ranged reads that read_file's own truncation notice tells the model to make. Bucketing cannot separate progress from repetition in general: an equality key can only approximate range overlap, and the approximation was erasing the offset that distinguishes the two. Key on the exact window instead, with an omitted end_line kept open-ended so a bare read and an explicit start_line=1 still share one key. Repeating a single range is still caught at the same threshold, and a read loop that varies its bounds remains covered by the per-tool frequency layer. Co-authored-by: Willem Jiang --- CHANGELOG.md | 10 +++ CHANGELOG_zh.md | 6 ++ .../middlewares/loop_detection_middleware.py | 62 +++++++++----- .../tests/test_loop_detection_middleware.py | 81 +++++++++++++++++++ 4 files changed, 140 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f1cb7e44..e4ff6f5fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -582,6 +582,16 @@ This section accumulates work toward the **2.1.0** milestone ### Fixed +- **middleware:** Stop loop detection from cutting off an agent that pages + through a file. `read_file` calls were keyed by 200-line buckets, so every + read shorter than a bucket collapsed onto its neighbours: five sequential + 40-line reads hashed identically and tripped the hard stop, ending the run + with a forced final answer and `stop_reason=loop_capped` — on exactly the + ranged reads `read_file`'s own truncation notice tells the model to make. + The key now uses the exact line window, with an omitted `end_line` kept + open-ended so a bare read and an explicit `start_line=1` still share one key. + Repeating a single range is still caught at the same threshold, and a read + loop that varies its bounds remains covered by the per-tool frequency layer. - **subagents:** Give `max_turns` the meaning operators read it as. It was handed to LangGraph as `recursion_limit`, which counts super-steps — one per graph node — while `create_agent` compiles a node for every middleware diff --git a/CHANGELOG_zh.md b/CHANGELOG_zh.md index adf47711f..7826f5223 100644 --- a/CHANGELOG_zh.md +++ b/CHANGELOG_zh.md @@ -397,6 +397,12 @@ ### 修复 +- **中间件：** 循环检测不再中断正在分段读取文件的智能体。此前 `read_file` 调用按 200 行分桶作为 + 键，因此任何短于一个桶的读取都会与相邻读取塌缩到同一个键：连续五次 40 行读取会哈希成相同值并 + 触发硬停止，运行被迫给出最终答复并带上 `stop_reason=loop_capped`——而这恰恰是 `read_file` 自身 + 的截断提示要求模型去做的分段读取。现在键使用精确的行区间，省略 `end_line` 时保持"读到末行"的 + 开放语义，因此不带范围的读取与显式 `start_line=1` 仍共用同一个键。重复同一区间依然会在原有阈值 + 被拦下，边界抖动的读取循环仍由按工具类型计数的频率层覆盖。 - **子智能体：** `max_turns` 现在真正表示运维人员理解的"轮次"。此前它被直接当作 LangGraph 的 `recursion_limit` 传入，而后者统计的是 super-step——每个图节点一步，且 `create_agent` 会为每个 中间件生命周期钩子编译出一个节点，因此在子智能体的中间件链上一轮要花掉 7~8 步：内置 diff --git a/backend/packages/harness/deerflow/agents/middlewares/loop_detection_middleware.py b/backend/packages/harness/deerflow/agents/middlewares/loop_detection_middleware.py index 9e1fc8571..1ab721ac1 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/loop_detection_middleware.py +++ b/backend/packages/harness/deerflow/agents/middlewares/loop_detection_middleware.py @@ -98,6 +98,9 @@ _DEFAULT_MAX_TRACKED_THREADS = 100 # LRU limit for tracked thread/run scopes _DEFAULT_TOOL_FREQ_WARN = 30 # warn after 30 calls to the same tool type _DEFAULT_TOOL_FREQ_HARD_LIMIT = 50 # force-stop after 50 calls to the same tool type _MAX_PENDING_WARNINGS_PER_RUN = 4 +# Stands in for ``read_file``'s omitted ``end_line`` in a call key: the read +# runs to the last line, which is not the same window as any numbered bound. +_OPEN_ENDED_READ = "end" type _RunScopeKey = tuple[str, str | None] @@ -128,29 +131,50 @@ def _normalize_tool_call_args(raw_args: object) -> tuple[dict, str | None]: return {}, json.dumps(raw_args, sort_keys=True, default=str) +def _coerce_line_number(value: object) -> int | None: + """Parse one ``read_file`` line bound, or ``None`` when absent or unusable.""" + try: + line = int(value) # type: ignore[arg-type] + except (TypeError, ValueError): + return None + return max(line, 1) + + +def _normalized_read_range(args: dict) -> tuple[int, int | None]: + """Normalize ``read_file``'s line range into one comparable window. + + Omitting ``end_line`` reads through the last line, so it normalizes to + ``None`` (open-ended) rather than collapsing onto ``start_line``: that keeps + ``read_file(path)`` and ``read_file(path, start_line=1)`` — the same read, + written two ways — on a single key. A reversed range is ordered, so one + window written either way also produces a single key. + + The window is otherwise kept exact. Quantizing it into 200-line buckets (the + original heuristic) collapsed every read shorter than a bucket onto its + neighbours, so an agent paging a file in 40-line chunks tripped the hard + stop on its fifth *distinct* read — while ``read_file``'s own truncation + notice tells the model to page with ``start_line``/``end_line``. Bucketing + cannot separate progress from repetition in general: equality keys can only + approximate range overlap, and the approximation was erasing the offset that + distinguishes the two. An exact window still catches the loop this layer + exists for — the same read emitted over and over — and a loop that jitters + its bounds is what Layer 2's per-tool frequency window covers. + """ + start_line = _coerce_line_number(args.get("start_line")) + end_line = _coerce_line_number(args.get("end_line")) + if start_line is None: + start_line = 1 + if end_line is not None and end_line < start_line: + start_line, end_line = end_line, start_line + return start_line, end_line + + def _stable_tool_key(name: str, args: dict, fallback_key: str | None) -> str: """Derive a stable key from salient args without overfitting to noise.""" if name == "read_file" and fallback_key is None: path = args.get("path") or "" - start_line = args.get("start_line") - end_line = args.get("end_line") - - bucket_size = 200 - try: - start_line = int(start_line) if start_line is not None else 1 - except (TypeError, ValueError): - start_line = 1 - try: - end_line = int(end_line) if end_line is not None else start_line - except (TypeError, ValueError): - end_line = start_line - - start_line, end_line = sorted((start_line, end_line)) - bucket_start = max(start_line, 1) - bucket_end = max(end_line, 1) - bucket_start = (bucket_start - 1) // bucket_size - bucket_end = (bucket_end - 1) // bucket_size - return f"{path}:{bucket_start}-{bucket_end}" + start_line, end_line = _normalized_read_range(args) + return f"{path}:{start_line}-{end_line if end_line is not None else _OPEN_ENDED_READ}" # write_file / str_replace are content-sensitive: same path may be updated # with different payloads during iteration. Using only salient fields (path) diff --git a/backend/tests/test_loop_detection_middleware.py b/backend/tests/test_loop_detection_middleware.py index 16d2a1ce9..c9d4be5ad 100644 --- a/backend/tests/test_loop_detection_middleware.py +++ b/backend/tests/test_loop_detection_middleware.py @@ -196,6 +196,87 @@ class TestHashToolCalls: assert _hash_tool_calls([a]) != _hash_tool_calls([b]) +class TestReadFileRangeKey: + """``read_file`` keys must separate paging progress from re-reading. + + Line ranges used to be quantized into 200-line buckets, which erased the + offset inside a bucket: every read shorter than 200 lines collapsed onto its + neighbours, so paging one file in 40-line chunks looked like five identical + calls and tripped the hard stop — on reads ``read_file``'s own truncation + notice tells the model to make. + """ + + @staticmethod + def _read_call(path="/w/app.py", **range_args): + return {"name": "read_file", "id": "call_read", "args": {"path": path, **range_args}} + + def test_adjacent_pages_are_distinct_calls(self): + pages = [self._read_call(start_line=start, end_line=start + 39) for start in (1, 41, 81, 121, 161)] + + hashes = {_hash_tool_calls([page]) for page in pages} + + assert len(hashes) == len(pages) + + def test_paging_through_a_file_does_not_hard_stop(self): + """Regression: five sequential 40-line reads used to force a final answer.""" + mw = LoopDetectionMiddleware(warn_threshold=3, hard_limit=5) + runtime = _make_runtime() + + for start in range(1, 401, 40): + decision = mw._apply(_make_state(tool_calls=[self._read_call(start_line=start, end_line=start + 39)]), runtime) + assert decision is None, f"read of lines {start}-{start + 39} was treated as a loop" + assert mw.consume_stop_reason("test-run") is None + + def test_repeating_one_range_still_hard_stops(self): + mw = LoopDetectionMiddleware(warn_threshold=3, hard_limit=5) + runtime = _make_runtime() + call = [self._read_call(start_line=1, end_line=40)] + + for _ in range(4): + assert mw._apply(_make_state(tool_calls=call), runtime) is None + hard_stop = mw._apply(_make_state(tool_calls=call), runtime) + + assert hard_stop is not None + assert hard_stop["messages"][0].tool_calls == [] + assert mw.consume_stop_reason("test-run") == "loop_capped" + + def test_rereading_the_same_page_counts_even_between_new_pages(self): + """Interleaving fresh pages must not hide a repeated read: the window counts by key.""" + mw = LoopDetectionMiddleware(warn_threshold=3, hard_limit=3) + runtime = _make_runtime() + repeated = [self._read_call(start_line=1, end_line=40)] + + assert mw._apply(_make_state(tool_calls=repeated), runtime) is None + assert mw._apply(_make_state(tool_calls=[self._read_call(start_line=41, end_line=80)]), runtime) is None + assert mw._apply(_make_state(tool_calls=repeated), runtime) is None + assert mw._apply(_make_state(tool_calls=[self._read_call(start_line=81, end_line=120)]), runtime) is None + + assert mw._apply(_make_state(tool_calls=repeated), runtime) is not None + + def test_omitted_end_line_matches_a_bare_read_of_the_same_file(self): + """Both read to the last line, so they are one read written two ways.""" + open_ended = self._read_call(start_line=1) + bare = self._read_call() + + assert _hash_tool_calls([open_ended]) == _hash_tool_calls([bare]) + + def test_omitted_end_line_is_not_a_single_line_read(self): + open_ended = self._read_call(start_line=10) + single_line = self._read_call(start_line=10, end_line=10) + + assert _hash_tool_calls([open_ended]) != _hash_tool_calls([single_line]) + + def test_line_bounds_are_clamped_to_the_first_line(self): + assert _hash_tool_calls([self._read_call(start_line=0, end_line=40)]) == _hash_tool_calls([self._read_call(start_line=1, end_line=40)]) + + def test_unparsable_end_line_reads_as_open_ended(self): + """The tool rejects such a call anyway; the key must stay stable, not crash.""" + assert _hash_tool_calls([self._read_call(start_line=5, end_line="oops")]) == _hash_tool_calls([self._read_call(start_line=5)]) + + def test_different_paths_stay_distinct(self): + assert _hash_tool_calls([self._read_call(path="/w/a.py", start_line=1, end_line=40)]) != _hash_tool_calls([self._read_call(path="/w/b.py", start_line=1, end_line=40)]) + + class TestLoopDetection: def test_no_tool_calls_returns_none(self): mw = LoopDetectionMiddleware()