fix(llm): bound extract retry cascade to one timeout budget per chunk - #3157
fix(llm): bound extract retry cascade to one timeout budget per chunk#3157Ashfaqbs wants to merge 1 commit into
Conversation
…hunk (Graphify-Labs#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.
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.
Graphify review — findings
Adds a shared wall-clock deadline to _extract_with_adaptive_retry so a subtree of timeout-driven splits shares one GRAPHIFY_API_TIMEOUT budget anchored at the top-level call rather than each split re-paying the full timeout. Once that deadline passes, a further timeout now gives up on the remaining chunk immediately (returning empty results with finish_reason="stop" and a stderr warning) instead of bisecting into up to 2**max_depth more full-length attempts.
Worth a look
- Deadline anchored to _resolve_api_timeout() but each split re-pays a full timeout, so budget may never be exceeded before first split —
graphify/llm.py:2378· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 823 functions depend on the 297 functions this change touches.
Health — this change adds coupling hotspots:
- new:
deduplicate_entities()— 63 callers, 21 callees - new:
build_merge()— 53 callers, 13 callees - new:
extract_files_direct()— 17 callers, 20 callees - new:
extract_corpus_parallel()— 26 callers, 11 callees - new:
_call_claude_cli()— 31 callers, 9 callees - new:
_extract_with_adaptive_retry()— 23 callers, 11 callees - new:
dispatch_command()— 2 callers, 122 callees - new:
_call_llm()— 11 callers, 18 callees - …and 16 more — each is listed as a finding
Verification — 823 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 532 function(s) in the blast radius were not formally verified this run
· 24 more finding(s) on lines outside this diff (see the check run).
43c71c3 to
24e84c4
Compare
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 3 advisory finding(s) below merit a look before merge.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Adds a shared wall-clock deadline to _extract_with_adaptive_retry so a chunk that keeps timing out no longer re-pays the full GRAPHIFY_API_TIMEOUT on every bisection: the top-level call anchors one budget (time.monotonic() + _resolve_api_timeout()) and all splits inherit the same absolute deadline. Once that budget is spent, a further timeout gives up on the remaining chunk — returning an empty result with finish_reason="stop" and a stderr warning — instead of spawning up to 2**max_depth more full-length attempts (previously up to ~2.5h for the 600s default at max_depth=3). Context-exceeded splits are unaffected; only timeouts honor the deadline.
Worth a look
- Deadline check gives up before ever splitting when timeout exceeds budget —
graphify/llm.py:2378· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Expired shared deadline still allows sibling retry to start —
graphify/llm.py:2422· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Expired timeout budget still allows right sibling extraction —
graphify/llm.py:2422· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 823 functions depend on the 297 functions this change touches.
Health — this change adds coupling hotspots:
- new:
deduplicate_entities()— 63 callers, 21 callees - new:
build_merge()— 53 callers, 13 callees - new:
extract_files_direct()— 17 callers, 20 callees - new:
extract_corpus_parallel()— 26 callers, 11 callees - new:
_call_claude_cli()— 31 callers, 9 callees - new:
_extract_with_adaptive_retry()— 23 callers, 11 callees - new:
dispatch_command()— 2 callers, 122 callees - new:
_call_llm()— 11 callers, 18 callees - …and 16 more — each is listed as a finding
Verification — 823 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 532 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify \_extract\_with\_adaptive\_retry.
The verifier did not have enough to check \_extract\_with\_adaptive\_retry, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `root` is annotated `Path` — outside the synthesizable primitive/collection set
· 24 more finding(s) on lines outside this diff (see the check run).
Fixes #3142.
Bug
_extract_with_adaptive_retrybisects a chunk onTimeoutExpired(and otherrecognized timeout errors) and retries each half. Every retry re-pays the
full
GRAPHIFY_API_TIMEOUTfrom scratch, so a chunk that keeps timing outacross the whole cascade can burn up to
2**max_depthfull timeouts —600s x (1+2+4+8) = 9,000s(~2.5h) at the default timeout andmax_depth=3.This is most visible on
--backend claude-cli, where each call spawns a fullnested
claude -psession and is far slower than a plain HTTP completion, sothe default 600s is exceeded in normal use rather than as an edge case. It
reads as a hang: the only signal between "chunk N/M done" and the eventual
give-up is silence.
The recursion depth was already capped, so this isn't unbounded — it's a
bounded cascade whose bound is much larger than anyone intends.
Fix
Give the whole split subtree for one original chunk a single shared
wall-clock deadline (anchored to one
GRAPHIFY_API_TIMEOUTallowance),instead of handing every split a fresh full timeout:
_deadline = time.monotonic() + GRAPHIFY_API_TIMEOUTand threads it through every recursive call in the subtree (both the
timeout-bisection path and the truncation/length-retry path, so a later
timeout deeper in an already-truncating subtree still respects it).
cascade gives up immediately instead of committing to another full-length
attempt — same outcome as hitting
max_depth, just reached by budgetinstead of depth.
this only changes what happens after a
TimeoutExpired-class exception.Worst case is now bounded by roughly one timeout's worth of wall time per
original chunk instead of
2**max_depthtimeouts.Testing
test_adaptive_retry_stops_when_timeout_budget_exhausted: mockstime.monotonic()to simulate the deadline being exceeded right after thefirst timeout, and asserts the cascade gives up after exactly 1 attempt
(previously it would have kept splitting and retrying).
tests/test_llm_backends.pysuite (104 tests, including the4 existing timeout/adaptive-retry tests) — all pass unchanged, since those
fakes complete instantly and never approach the default 600s budget.
ruff checkon both changed files — clean.a large, pre-existing batch of unrelated failures there trace back to a
missing
tree-sitterinstall in my environment, reproducible on a cleancheckout of
v8with no changes applied, not to this diff.I used AI-assisted tooling to help navigate the codebase and draft this
change, but the diagnosis of the fix approach and the change itself were
reviewed and verified by me before opening this PR.