From a78c67dd8424b4958df9dfde2e9c510c54a3aa9c Mon Sep 17 00:00:00 2001 From: Yibo Cai Date: Thu, 10 Sep 2026 23:59:41 -0700 Subject: [PATCH 1/2] fix(query): stop tool-failure-loop guard from conflating distinct Bash failures The generic fallback error-category keyed off the first 120 chars of a tool result. Bash results are stdout+stderr+exit-code sentence in that order, so a script that prints a startup banner before crashing produced an identical 120-char prefix for genuinely different bugs, tripping the loop guard after 3 attempts even when each attempt fixed the prior bug and hit a new one. Now prefers the tail (and a matched traceback's tail specifically), where the actual exception line lives, so distinct failures behind a shared stdout prefix are no longer collapsed into one signature. --- src/query/tool_failure_loop_guard.py | 35 ++++++++++++++++++++++- tests/test_tool_failure_loop_guard.py | 40 +++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/src/query/tool_failure_loop_guard.py b/src/query/tool_failure_loop_guard.py index 223eb5fdc..43d97960b 100644 --- a/src/query/tool_failure_loop_guard.py +++ b/src/query/tool_failure_loop_guard.py @@ -37,6 +37,22 @@ always eventually reached, never suppressed, and ``max_turns`` bounds the worst case. Deliberate: a model succeeding a quarter of the time is making progress, not looping. + +Third divergence (intentional): the generic fallback category (no named +error pattern matched) is keyed off the TAIL of the tool result, not the +head. TS and the original Python port both took ``text[:120]``. For Bash +specifically, tool_result content is stdout + stderr + an exit-code +sentence in that order, so the head is dominated by the command's own +stdout (a startup banner, progress output) while the actual differentiator +-- a traceback's exception line, a compiler's final error -- sits at the +tail. Observed in practice: three genuinely different bugs in a script +that prints an identical banner before crashing each time were categorized +as ONE recurring signature and tripped the guard after attempt 3, even +though each attempt fixed the prior bug and hit a new one. Taking the tail +(and preferring a matched ``Traceback (most recent call last):`` block's +tail when present, since that isolates the exception line from any stderr +preamble) fixes this without weakening detection of an actually-recurring +error, since a truly identical failure has an identical tail too. """ from __future__ import annotations @@ -376,8 +392,25 @@ def _normalize_error_category(content: str) -> str: if re.search(r"Error writing file", normalized, re.IGNORECASE): return "FileWriteError" + # Generic fallback. Bash results are stdout + stderr + an exit-code + # sentence, in that order (bash_tool.py:_assemble_bash_body / + # _bash_map_result_to_api), so a long-running command's own stdout + # (banner/progress text, often near-identical across genuinely + # different failures) sits at the head while the actual differentiator + # -- a traceback's exception line, or a compiler's final error -- sits + # at the tail, just before the exit-code sentence. Strip that sentence, + # then prefer the tail over the head so distinct failures don't get + # collapsed into one signature by a shared stdout prefix. + without_exit_code = re.sub( + r"\s*Command failed with exit code \d+\s*$", "", normalized, flags=re.IGNORECASE + ) + traceback_match = re.search( + r"Traceback \(most recent call last\):.*$", without_exit_code, re.IGNORECASE + ) + signal = traceback_match.group(0) if traceback_match else without_exit_code + return ( - normalized.lower()[:MAX_FALLBACK_CATEGORY_LENGTH] or "unknown error" + signal.lower()[-MAX_FALLBACK_CATEGORY_LENGTH:] or "unknown error" ) diff --git a/tests/test_tool_failure_loop_guard.py b/tests/test_tool_failure_loop_guard.py index a370d1300..c6921c7de 100644 --- a/tests/test_tool_failure_loop_guard.py +++ b/tests/test_tool_failure_loop_guard.py @@ -250,6 +250,46 @@ def test_tool_use_error_tags_stripped(self): "PermissionError", ) + def test_fallback_prefers_traceback_tail_over_shared_stdout_banner(self): + """Two distinct bugs behind an identical stdout banner must not + collapse into the same fallback signature (the bug this test + guards: the old head-slice made every Bash failure of a script + that prints a banner before crashing look identical).""" + banner = "Loading MNIST dataset...\n" * 3 + "Starting training run\n" + run1 = ( + banner + + "Traceback (most recent call last):\n" + + ' File "train.py", line 42, in \n' + + "ConcretizationTypeError: Abstract tracer value\n" + + "Command failed with exit code 1" + ) + run2 = ( + banner + + "Traceback (most recent call last):\n" + + ' File "train.py", line 58, in \n' + + "TypeError: unsupported operand type(s)\n" + + "Command failed with exit code 1" + ) + self.assertNotEqual( + _normalize_error_category(run1), _normalize_error_category(run2) + ) + + def test_fallback_no_traceback_uses_tail_not_head(self): + text = ("x" * 200) + "distinct tail content that must survive" + out = _normalize_error_category(text) + self.assertTrue(out.endswith("distinct tail content that must survive")) + + def test_fallback_same_traceback_still_matches(self): + """A genuinely recurring failure must still be recognized as the + same signature -- the fix must not weaken real-loop detection.""" + text = ( + "irrelevant preamble\n" + "Traceback (most recent call last):\n" + "ValueError: same bug every time\n" + "Command failed with exit code 1" + ) + self.assertEqual(_normalize_error_category(text), _normalize_error_category(text)) + class TestPathHandling(unittest.TestCase): def test_field_precedence(self): From 625c517e1a797cca880c30011b0c127f0b2bc068 Mon Sep 17 00:00:00 2001 From: Yibo Cai Date: Sat, 12 Sep 2026 11:30:19 -0700 Subject: [PATCH 2/2] test(query): differentially fuzz the fallback-category tail-slice fix 2000 seeded trials each, in the spirit of this guard's own per-batch counting fix (differential fuzz against the prior behavior rather than one hand-picked example): 1. Reproduces the bug at scale: a >=120-char shared banner collides almost every trial under the old head-slice, and zero times under the new tail-preferring slice, for otherwise-distinct tracebacks. 2. No-regression property: an identical traceback tail is still recognized as the same category no matter how much random, unrelated stdout noise varies in the head -- the real-loop case the fix must not weaken. --- tests/test_tool_failure_loop_guard.py | 100 ++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/tests/test_tool_failure_loop_guard.py b/tests/test_tool_failure_loop_guard.py index c6921c7de..c96fdcdd4 100644 --- a/tests/test_tool_failure_loop_guard.py +++ b/tests/test_tool_failure_loop_guard.py @@ -291,6 +291,106 @@ def test_fallback_same_traceback_still_matches(self): self.assertEqual(_normalize_error_category(text), _normalize_error_category(text)) +class TestFallbackCategoryFuzz(unittest.TestCase): + """Differential fuzz against the old head-slice behavior (2000 seeded + randomized trials), in the spirit of the guard's own per-batch-counting + fix: quantify the bug at scale rather than trust one hand-picked example, + and bound the fix's downside risk rather than assert it away. + + ``_normalize_error_category`` only diverges from the old + ``text.lower()[:MAX_FALLBACK_CATEGORY_LENGTH]`` when the (exit-code- + stripped) text is longer than the 120-char window -- shorter text has + ``[:120] == [-120:]`` for both slicings, so there is nothing to test + there. Both properties below hold that length fixed above the window. + """ + + SEED = 20260910 # date this fix landed; keeps trials reproducible. + TRIALS = 2000 + + @staticmethod + def _old_fallback_category(text: str) -> str: + """The pre-fix behavior this module used to have, reconstructed + verbatim for differential comparison (not imported from the module, + since the module no longer contains it).""" + import re as _re + + normalized = _re.sub(r"\s+", " ", text).strip() + normalized = _re.sub( + r"", "", normalized, flags=_re.IGNORECASE + ).strip() + return normalized.lower()[:120] or "unknown error" + + def _random_banner(self, rng: "__import__('random').Random", min_len: int) -> str: + words = ["Loading", "dataset", "Initializing", "model", "epoch", "step", + "progress:", "===", "Starting", "run", "batch", "checkpoint"] + out = "" + while len(out) < min_len: + out += rng.choice(words) + " " + return out + + def _random_traceback(self, rng: "__import__('random').Random", tag: int) -> str: + exceptions = [ + "ValueError: bad shape", "TypeError: unsupported operand", + "KeyError: 'params'", "IndexError: out of bounds", + "RuntimeError: device mismatch", "ConcretizationTypeError: tracer", + ] + return ( + "Traceback (most recent call last):\n" + f' File "train_{tag}.py", line {rng.randint(1, 999)}, in \n' + + rng.choice(exceptions) + ) + + def test_old_scheme_collides_on_long_shared_banners_new_scheme_resolves_them(self): + """At scale: whenever the shared preamble is >= 120 chars, the old + head-slice collapses genuinely distinct tracebacks into one category + far more often than the new tail-preferring scheme does.""" + import random + + rng = random.Random(self.SEED) + old_collisions = 0 + new_collisions = 0 + for _ in range(self.TRIALS): + banner = self._random_banner(rng, min_len=rng.randint(120, 400)) + text1 = banner + self._random_traceback(rng, 1) + "\nCommand failed with exit code 1" + text2 = banner + self._random_traceback(rng, 2) + "\nCommand failed with exit code 1" + if self._old_fallback_category(text1) == self._old_fallback_category(text2): + old_collisions += 1 + if _normalize_error_category(text1) == _normalize_error_category(text2): + new_collisions += 1 + self.assertGreater( + old_collisions, self.TRIALS * 0.9, + "sanity check: the old scheme should collide on almost every " + "trial here, or this fuzz setup isn't reproducing the bug", + ) + self.assertEqual( + new_collisions, 0, + "the new scheme must not collapse distinct tracebacks behind a " + "long shared banner", + ) + + def test_identical_tail_is_recognized_regardless_of_random_head_noise(self): + """No-regression property: a truly recurring failure (identical + traceback tail) must still compare equal under the new scheme no + matter what unrelated, randomly-varying stdout noise precedes it -- + this is the real-loop case the fix must not weaken.""" + import random + + rng = random.Random(self.SEED + 1) + fixed_tail = ( + "Traceback (most recent call last):\n" + ' File "train.py", line 77, in \n' + "ValueError: same bug every time\n" + "Command failed with exit code 1" + ) + for _ in range(self.TRIALS): + head_a = self._random_banner(rng, min_len=rng.randint(0, 500)) + head_b = self._random_banner(rng, min_len=rng.randint(0, 500)) + self.assertEqual( + _normalize_error_category(head_a + fixed_tail), + _normalize_error_category(head_b + fixed_tail), + ) + + class TestPathHandling(unittest.TestCase): def test_field_precedence(self): """guard:301 — file_path, then path, then notebook_path."""