From 24e84c40c5059fd6f8457a3bb2f21173251003ae Mon Sep 17 00:00:00 2001 From: Ashfaq <105435085+Ashfaqbs@users.noreply.github.com> Date: Fri, 28 Aug 2026 08:58:58 +0530 Subject: [PATCH 1/3] fix(llm): bound the extract retry cascade to one timeout budget per chunk (#3142) A chunk that times out at every recursion depth used to re-pay the full GRAPHIFY_API_TIMEOUT on each of up to 2**max_depth attempts (600s x 15 attempts = up to 2.5h at the default settings for claude-cli). Track a shared wall-clock deadline for the whole split subtree instead of granting each split a fresh full timeout. Once the deadline passes, a further timeout gives up immediately rather than committing to another full-length attempt. --- graphify/llm.py | 37 +++++++++++++++++++++++++++++++------ tests/test_llm_backends.py | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 6 deletions(-) diff --git a/graphify/llm.py b/graphify/llm.py index ae2119773a..ba95a2fe51 100644 --- a/graphify/llm.py +++ b/graphify/llm.py @@ -2265,6 +2265,7 @@ def _extract_with_adaptive_retry( _depth: int = 0, *, deep_mode: bool = False, + _deadline: float | None = None, ) -> dict: """Extract a chunk; if the response is truncated (`finish_reason="length"`), the API rejects the prompt as too large for the model's context window, or @@ -2307,13 +2308,24 @@ def _extract_with_adaptive_retry( a splittable document: the slice is bisected and retried (#1369). A whole non-splittable file (e.g. one huge code file) can't be made smaller than itself, so we return what we got and warn. + + Timeouts additionally carry a subtree wall-clock budget (``_deadline``): the + top-level call anchors it to one `GRAPHIFY_API_TIMEOUT` allowance, and every + split inherits the same absolute deadline rather than each getting a fresh + full timeout. Before #3142, a chunk that timed out at every depth re-paid + the full timeout on each of up to ``2**max_depth`` attempts — up to 2.5h for + the default 600s timeout at max_depth=3. Once the shared deadline passes, + a further timeout gives up immediately instead of splitting again. """ + if _deadline is None: + _deadline = time.monotonic() + _resolve_api_timeout() + def _merge_two(left_units, right_units) -> dict: left = _extract_with_adaptive_retry( - left_units, backend, api_key, model, root, max_depth, _depth + 1, deep_mode=deep_mode + left_units, backend, api_key, model, root, max_depth, _depth + 1, deep_mode=deep_mode, _deadline=_deadline ) right = _extract_with_adaptive_retry( - right_units, backend, api_key, model, root, max_depth, _depth + 1, deep_mode=deep_mode + right_units, backend, api_key, model, root, max_depth, _depth + 1, deep_mode=deep_mode, _deadline=_deadline ) return { "nodes": left.get("nodes", []) + right.get("nodes", []), @@ -2363,6 +2375,19 @@ def _split_lone_slice() -> "tuple[FileSlice, FileSlice] | None": if not (_looks_like_context_exceeded(exc) or is_timeout): raise reason = "timed out" if is_timeout else "exceeded context" + if is_timeout and time.monotonic() >= _deadline: + # The subtree's shared timeout budget is spent — every prior split + # in this cascade already re-paid the full per-attempt timeout, so + # granting yet another one here is how a single slow chunk used to + # burn up to 2**max_depth timeouts (#3142). Give up on whatever is + # left rather than committing to another full-length attempt. + print( + f"[graphify] chunk of {len(chunk)} timed out at depth {_depth} and " + f"the subtree's {_resolve_api_timeout():g}s timeout budget is spent " + f"— giving up on this chunk instead of splitting further", + file=sys.stderr, + ) + return {"nodes": [], "edges": [], "hyperedges": [], "input_tokens": 0, "output_tokens": 0, "model": model, "finish_reason": "stop"} if len(chunk) <= 1: halves = _split_lone_slice() if halves is not None: @@ -2394,10 +2419,10 @@ def _split_lone_slice() -> "tuple[FileSlice, FileSlice] | None": ) mid = len(chunk) // 2 left = _extract_with_adaptive_retry( - chunk[:mid], backend, api_key, model, root, max_depth, _depth + 1, deep_mode=deep_mode + chunk[:mid], backend, api_key, model, root, max_depth, _depth + 1, deep_mode=deep_mode, _deadline=_deadline ) right = _extract_with_adaptive_retry( - chunk[mid:], backend, api_key, model, root, max_depth, _depth + 1, deep_mode=deep_mode + chunk[mid:], backend, api_key, model, root, max_depth, _depth + 1, deep_mode=deep_mode, _deadline=_deadline ) return { "nodes": left.get("nodes", []) + right.get("nodes", []), @@ -2482,10 +2507,10 @@ def _split_lone_slice() -> "tuple[FileSlice, FileSlice] | None": ) mid = len(chunk) // 2 left = _extract_with_adaptive_retry( - chunk[:mid], backend, api_key, model, root, max_depth, _depth + 1, deep_mode=deep_mode + chunk[:mid], backend, api_key, model, root, max_depth, _depth + 1, deep_mode=deep_mode, _deadline=_deadline ) right = _extract_with_adaptive_retry( - chunk[mid:], backend, api_key, model, root, max_depth, _depth + 1, deep_mode=deep_mode + chunk[mid:], backend, api_key, model, root, max_depth, _depth + 1, deep_mode=deep_mode, _deadline=_deadline ) return { diff --git a/tests/test_llm_backends.py b/tests/test_llm_backends.py index 4480eff629..bb65c22da7 100644 --- a/tests/test_llm_backends.py +++ b/tests/test_llm_backends.py @@ -445,6 +445,42 @@ def fake_extract(chunk, *_, **__): assert "exceeded context" not in err +def test_adaptive_retry_stops_when_timeout_budget_exhausted(tmp_path, capsys): + """Regression test for #3142: before this fix, every split re-paid the + full per-attempt timeout, so a chunk that keeps timing out could burn + 2**max_depth timeouts (up to 2.5h for the 600s default at max_depth=3). + Once the shared subtree deadline has passed, a further timeout must give + up immediately instead of committing another chunk to a fresh timeout.""" + import subprocess + + files = [tmp_path / f"f{i}.md" for i in range(4)] + for f in files: + f.write_text("hello") + + calls = {"n": 0} + + def always_timeout(chunk, *_, **__): + calls["n"] += 1 + raise subprocess.TimeoutExpired(["claude", "-p"], 600) + + # First monotonic() call anchors the deadline at t=0 (+600s default + # budget). The second call, made right after the first timeout while + # deciding whether to split, reports t=700 -- past the budget -- so the + # cascade must give up rather than commit to another 600s attempt. + clock = iter([0.0, 700.0]) + with patch("graphify.llm.extract_files_direct", side_effect=always_timeout), \ + patch("graphify.llm.time.monotonic", side_effect=lambda: next(clock)): + result = llm._extract_with_adaptive_retry( + files, backend="claude-cli", api_key=None, model=None, root=tmp_path, max_depth=3 + ) + + assert result["nodes"] == [] + assert result["finish_reason"] == "stop" + assert calls["n"] == 1 # only the original attempt -- no bisection paid for + err = capsys.readouterr().err + assert "subtree" in err and "budget is spent" in err + + def test_adaptive_retry_timeout_caps_at_max_depth(tmp_path, capsys): import subprocess From 7cc0f14f68120df4b754282272e115f27eb6b95b Mon Sep 17 00:00:00 2001 From: Ashfaq <105435085+Ashfaqbs@users.noreply.github.com> Date: Wed, 2 Sep 2026 22:19:22 +0530 Subject: [PATCH 2/3] fix(llm): skip an already-budget-exhausted split before attempting it Graphify's own review bot flagged this PR: the shared subtree deadline was only checked reactively, after a timeout occurred. A sibling split reached after the budget was already spent elsewhere in the same subtree would still start (and pay for) its own fresh full-length attempt before its own reactive check caught up and gave up on it -- weakening the "one shared budget per subtree" guarantee this PR set out to establish. Add a proactive check at the top of each split: if the shared deadline has already passed before this split's own attempt has even started, skip the attempt outright instead of starting a new one the budget can no longer afford. (A related finding, that the very first depth-0 attempt "gives up before ever splitting" when it alone consumes the whole budget, turned out on investigation to be inherent to the single-timeout-budget design rather than separately fixable: if the original attempt's own client-side timeout reaches the full shared budget, there is by definition no time left for any further split regardless of where in the call tree that's decided. Not changed.) --- graphify/llm.py | 26 ++++++++++++++++++-- tests/test_llm_backends.py | 49 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/graphify/llm.py b/graphify/llm.py index ba95a2fe51..a7b2d40673 100644 --- a/graphify/llm.py +++ b/graphify/llm.py @@ -2314,11 +2314,33 @@ def _extract_with_adaptive_retry( split inherits the same absolute deadline rather than each getting a fresh full timeout. Before #3142, a chunk that timed out at every depth re-paid the full timeout on each of up to ``2**max_depth`` attempts — up to 2.5h for - the default 600s timeout at max_depth=3. Once the shared deadline passes, - a further timeout gives up immediately instead of splitting again. + the default 600s timeout at max_depth=3. A timeout checks the budget + reactively, after the attempt, and gives up rather than splitting further + once it is spent. + + A split whose *own* attempt has not yet started also checks the budget + proactively, before making that attempt: if an earlier sibling elsewhere + in the same subtree already exhausted the budget with a real timeout of + its own, this split is skipped outright rather than paying for a fresh + full-length attempt that the shared budget can no longer afford. """ if _deadline is None: _deadline = time.monotonic() + _resolve_api_timeout() + elif _depth > 0 and time.monotonic() >= _deadline: + # An earlier split in this subtree already used up the shared budget + # with a real timeout of its own (the reactive check below, on a + # prior call in this recursion). Skip this attempt entirely rather + # than paying for one more full-length call the budget can no longer + # afford — without this, a sibling reached after the budget is spent + # would still start (and pay for) its own fresh timeout, since the + # reactive check only fires after that sibling's own attempt fails. + print( + f"[graphify] chunk of {len(chunk)} at depth {_depth}: the subtree's " + f"{_resolve_api_timeout():g}s timeout budget is spent — skipping " + f"this split rather than starting a fresh attempt", + file=sys.stderr, + ) + return {"nodes": [], "edges": [], "hyperedges": [], "input_tokens": 0, "output_tokens": 0, "model": model, "finish_reason": "stop"} def _merge_two(left_units, right_units) -> dict: left = _extract_with_adaptive_retry( diff --git a/tests/test_llm_backends.py b/tests/test_llm_backends.py index bb65c22da7..ed915e8aa9 100644 --- a/tests/test_llm_backends.py +++ b/tests/test_llm_backends.py @@ -481,6 +481,55 @@ def always_timeout(chunk, *_, **__): assert "subtree" in err and "budget is spent" in err +def test_adaptive_retry_skips_sibling_attempt_after_budget_exhausted_mid_tree(tmp_path, capsys): + """Regression test: the reactive check above only catches a timeout + *after* the attempt that caused it. Once some split elsewhere in the + subtree has already spent the shared budget with a real timeout of its + own, a sibling split reached afterwards must not still pay for a brand + new full-length attempt before discovering the same thing reactively -- + it should never start that attempt at all. + + The depth-0 chunk gets an instant (non-timeout) truncated response, so it + splits immediately with the budget untouched. The left half then times + out for real, spending the whole shared budget. The right half must be + skipped proactively, without ever calling extract_files_direct.""" + import subprocess + + files = [tmp_path / f"f{i}.md" for i in range(4)] + for f in files: + f.write_text("hello") + + calls = [] + + def truncated_then_timeout(chunk, *_, **__): + calls.append(len(chunk)) + if len(chunk) == 4: + return {"nodes": [], "edges": [], "hyperedges": [], "input_tokens": 1, + "output_tokens": 1, "model": "m", "finish_reason": "length"} + raise subprocess.TimeoutExpired(["claude", "-p"], 600) + + # t=0: depth-0 anchors the deadline (+600s). Its own attempt returns + # "length" instantly, so no further monotonic() call happens at depth 0. + # t=5: the left child's proactive check, well inside the budget -- it + # proceeds to attempt, and that attempt times out for real. + # t=605: the left child's reactive check, past the deadline -- it gives + # up and returns to depth 0. + # t=605: the right child's proactive check, also past the deadline -- it + # must skip its attempt entirely rather than starting a fresh one. + clock = iter([0.0, 5.0, 605.0, 605.0]) + with patch("graphify.llm.extract_files_direct", side_effect=truncated_then_timeout), \ + patch("graphify.llm.time.monotonic", side_effect=lambda: next(clock)): + result = llm._extract_with_adaptive_retry( + files, backend="claude-cli", api_key=None, model=None, root=tmp_path, max_depth=3 + ) + + assert result["nodes"] == [] + # depth-0 (len 4) + left's real attempt (len 2) -- right never attempted. + assert calls == [4, 2] + err = capsys.readouterr().err + assert "subtree" in err and "budget is spent" in err + + def test_adaptive_retry_timeout_caps_at_max_depth(tmp_path, capsys): import subprocess From 4b37e4d678542fc697f9ca66d6992eea2f5d14f6 Mon Sep 17 00:00:00 2001 From: Ashfaq <105435085+Ashfaqbs@users.noreply.github.com> Date: Tue, 15 Sep 2026 17:15:08 +0530 Subject: [PATCH 3/3] fix(llm): only skip a split once a real timeout has occurred The proactive budget check in _extract_with_adaptive_retry skipped a not-yet-started split purely based on wall-clock time (time.monotonic() >= _deadline), regardless of whether any real timeout had ever happened in the subtree. A context-exceeded or truncation ("length") split could therefore be skipped -- returning an empty result instead of recovering via bisection -- just because enough time had elapsed since the deadline was anchored, contradicting the documented intent that only a real timeout should spend the shared budget. Thread a _timeout_hit flag (a one-element list shared by reference across the recursion) that is only set True when a real timeout exception is observed, and gate the proactive skip on it. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01PhuCVHf7XKmFmUMULkczFa --- graphify/llm.py | 28 +++++++++++++----- tests/test_llm_backends.py | 60 +++++++++++++++++++++++++++++++++----- 2 files changed, 73 insertions(+), 15 deletions(-) diff --git a/graphify/llm.py b/graphify/llm.py index a7b2d40673..5d26a8d909 100644 --- a/graphify/llm.py +++ b/graphify/llm.py @@ -2266,6 +2266,7 @@ def _extract_with_adaptive_retry( *, deep_mode: bool = False, _deadline: float | None = None, + _timeout_hit: "list[bool] | None" = None, ) -> dict: """Extract a chunk; if the response is truncated (`finish_reason="length"`), the API rejects the prompt as too large for the model's context window, or @@ -2322,11 +2323,17 @@ def _extract_with_adaptive_retry( proactively, before making that attempt: if an earlier sibling elsewhere in the same subtree already exhausted the budget with a real timeout of its own, this split is skipped outright rather than paying for a fresh - full-length attempt that the shared budget can no longer afford. + full-length attempt that the shared budget can no longer afford. This + proactive skip only fires once a real timeout has actually been observed + somewhere in the subtree (``_timeout_hit``, a one-element list shared by + reference across the whole recursion) — plain wall-clock time passing + from ordinary successful calls, or from a context-exceeded or truncation + split, never trips it on its own. """ if _deadline is None: _deadline = time.monotonic() + _resolve_api_timeout() - elif _depth > 0 and time.monotonic() >= _deadline: + _timeout_hit = [False] + elif _timeout_hit[0] and _depth > 0 and time.monotonic() >= _deadline: # An earlier split in this subtree already used up the shared budget # with a real timeout of its own (the reactive check below, on a # prior call in this recursion). Skip this attempt entirely rather @@ -2344,10 +2351,10 @@ def _extract_with_adaptive_retry( def _merge_two(left_units, right_units) -> dict: left = _extract_with_adaptive_retry( - left_units, backend, api_key, model, root, max_depth, _depth + 1, deep_mode=deep_mode, _deadline=_deadline + left_units, backend, api_key, model, root, max_depth, _depth + 1, deep_mode=deep_mode, _deadline=_deadline, _timeout_hit=_timeout_hit ) right = _extract_with_adaptive_retry( - right_units, backend, api_key, model, root, max_depth, _depth + 1, deep_mode=deep_mode, _deadline=_deadline + right_units, backend, api_key, model, root, max_depth, _depth + 1, deep_mode=deep_mode, _deadline=_deadline, _timeout_hit=_timeout_hit ) return { "nodes": left.get("nodes", []) + right.get("nodes", []), @@ -2397,6 +2404,11 @@ def _split_lone_slice() -> "tuple[FileSlice, FileSlice] | None": if not (_looks_like_context_exceeded(exc) or is_timeout): raise reason = "timed out" if is_timeout else "exceeded context" + if is_timeout: + # A real timeout happened, not just elapsed wall-clock time from + # ordinary successful calls — only now is the shared budget + # allowed to skip a not-yet-started sibling proactively. + _timeout_hit[0] = True if is_timeout and time.monotonic() >= _deadline: # The subtree's shared timeout budget is spent — every prior split # in this cascade already re-paid the full per-attempt timeout, so @@ -2441,10 +2453,10 @@ def _split_lone_slice() -> "tuple[FileSlice, FileSlice] | None": ) mid = len(chunk) // 2 left = _extract_with_adaptive_retry( - chunk[:mid], backend, api_key, model, root, max_depth, _depth + 1, deep_mode=deep_mode, _deadline=_deadline + chunk[:mid], backend, api_key, model, root, max_depth, _depth + 1, deep_mode=deep_mode, _deadline=_deadline, _timeout_hit=_timeout_hit ) right = _extract_with_adaptive_retry( - chunk[mid:], backend, api_key, model, root, max_depth, _depth + 1, deep_mode=deep_mode, _deadline=_deadline + chunk[mid:], backend, api_key, model, root, max_depth, _depth + 1, deep_mode=deep_mode, _deadline=_deadline, _timeout_hit=_timeout_hit ) return { "nodes": left.get("nodes", []) + right.get("nodes", []), @@ -2529,10 +2541,10 @@ def _split_lone_slice() -> "tuple[FileSlice, FileSlice] | None": ) mid = len(chunk) // 2 left = _extract_with_adaptive_retry( - chunk[:mid], backend, api_key, model, root, max_depth, _depth + 1, deep_mode=deep_mode, _deadline=_deadline + chunk[:mid], backend, api_key, model, root, max_depth, _depth + 1, deep_mode=deep_mode, _deadline=_deadline, _timeout_hit=_timeout_hit ) right = _extract_with_adaptive_retry( - chunk[mid:], backend, api_key, model, root, max_depth, _depth + 1, deep_mode=deep_mode, _deadline=_deadline + chunk[mid:], backend, api_key, model, root, max_depth, _depth + 1, deep_mode=deep_mode, _deadline=_deadline, _timeout_hit=_timeout_hit ) return { diff --git a/tests/test_llm_backends.py b/tests/test_llm_backends.py index ed915e8aa9..8eb59a15a7 100644 --- a/tests/test_llm_backends.py +++ b/tests/test_llm_backends.py @@ -510,13 +510,16 @@ def truncated_then_timeout(chunk, *_, **__): # t=0: depth-0 anchors the deadline (+600s). Its own attempt returns # "length" instantly, so no further monotonic() call happens at depth 0. - # t=5: the left child's proactive check, well inside the budget -- it - # proceeds to attempt, and that attempt times out for real. - # t=605: the left child's reactive check, past the deadline -- it gives - # up and returns to depth 0. - # t=605: the right child's proactive check, also past the deadline -- it - # must skip its attempt entirely rather than starting a fresh one. - clock = iter([0.0, 5.0, 605.0, 605.0]) + # The left child's proactive check makes no monotonic() call either: no + # real timeout has happened anywhere in the subtree yet, so the check is + # short-circuited before it ever looks at the clock -- it proceeds + # straight to its attempt, and that attempt times out for real. + # t=605: the left child's reactive check (now the second clock call), + # past the deadline -- it gives up and returns to depth 0. + # t=605: the right child's proactive check, now armed because a real + # timeout was just observed -- also past the deadline, so it must skip + # its attempt entirely rather than starting a fresh one. + clock = iter([0.0, 605.0, 605.0]) with patch("graphify.llm.extract_files_direct", side_effect=truncated_then_timeout), \ patch("graphify.llm.time.monotonic", side_effect=lambda: next(clock)): result = llm._extract_with_adaptive_retry( @@ -530,6 +533,49 @@ def truncated_then_timeout(chunk, *_, **__): assert "subtree" in err and "budget is spent" in err +def test_adaptive_retry_deadline_only_skips_after_a_real_timeout(tmp_path): + """Regression test: the proactive budget check must only skip a split + once a real timeout has actually happened somewhere in the subtree, not + merely because time.monotonic() reports the deadline has passed. Before + this fix, the check was purely wall-clock-based, so a subtree that never + raised a single timeout -- e.g. one that recovered via truncation splits + only -- could still have a later sibling skipped outright just because + enough time had elapsed since the deadline was anchored. + + The depth-0 chunk truncates instantly (finish_reason='length', no + exception raised) and splits into two depth-1 chunks. time.monotonic() + is patched to report far past the deadline for every call after the + anchor, simulating elapsed wall-clock time with zero real timeouts. Both + depth-1 children must still be attempted.""" + from itertools import chain, repeat + + files = [tmp_path / f"f{i}.md" for i in range(4)] + for f in files: + f.write_text("hello") + + calls = [] + + def truncate_once_then_succeed(chunk, *_, **__): + calls.append(len(chunk)) + finish = "length" if len(chunk) == 4 else "stop" + return _ok(nodes=[{"id": f"n_{len(calls)}"}]) | {"finish_reason": finish} + + # t=0 anchors the deadline at 0 + 600s. Every later monotonic() call, no + # matter how many the implementation makes, reports t=700 -- past the + # deadline -- with no timeout ever having occurred. + clock = chain([0.0], repeat(700.0)) + with patch("graphify.llm.extract_files_direct", side_effect=truncate_once_then_succeed), \ + patch("graphify.llm.time.monotonic", side_effect=lambda: next(clock)): + result = llm._extract_with_adaptive_retry( + files, backend="claude-cli", api_key=None, model=None, root=tmp_path, max_depth=3 + ) + + # depth-0 (len 4) + both depth-1 halves (len 2, len 2) -- neither half + # was skipped by the elapsed-time-only deadline. + assert calls == [4, 2, 2] + assert len(result["nodes"]) == 2 + + def test_adaptive_retry_timeout_caps_at_max_depth(tmp_path, capsys): import subprocess