fix(llm): honour GRAPHIFY_API_TIMEOUT in the claude-cli backend, not just HTTP#1111
Closed
yaanfpv wants to merge 1 commit into
Closed
fix(llm): honour GRAPHIFY_API_TIMEOUT in the claude-cli backend, not just HTTP#1111yaanfpv wants to merge 1 commit into
yaanfpv wants to merge 1 commit into
Conversation
…just HTTP GRAPHIFY_API_TIMEOUT (and the --api-timeout flag that sets it) bounded only the OpenAI-compatible HTTP client. The two claude-cli subprocess calls, extraction and community labeling, hardcoded timeout=600 and ignored the knob entirely, so a slow chunk on claude-cli could not be given more headroom without patching the source. Factor the parse the HTTP path already used into a _resolve_api_timeout helper (mirroring _resolve_max_tokens) and route all three call sites through it. Behaviour for the HTTP backend is unchanged. The claude-cli extraction and labeling subprocesses now respect the same env var and flag, defaulting to 600s when unset and falling back to the default on non-positive or unparseable values.
Collaborator
|
Implemented from our end in 75c4de7 with two additions over the PR: (1) the Anthropic SDK path (_call_anthropic) now also honours the timeout, so the README claim 'every backend' is accurate; (2) added a test for the _call_llm claude-cli branch separately from _call_claude_cli. Thanks for the diagnosis — your root cause analysis was exactly right. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1112.
GRAPHIFY_API_TIMEOUT, and the--api-timeoutflag that sets it, only ever reached the OpenAI-compatible HTTP client. The twoclaude-clisubprocess calls, extraction (_call_claude_cli) and community labeling (theclaude-clibranch of_call_llm), hardcodedtimeout=600and ignored the knob completely.That bites on long
claude-cliruns. A dense chunk or a large 1M-context model can legitimately run past ten minutes, and today there is no way to give it more headroom without editing the source. The HTTP backends already advertise the escape hatch (the env-var table documents it, and #792 wired it in); the CLI path just never picked it up.What changed
_resolve_api_timeout()helper, placed next to and mirroring the existing_resolve_max_tokens()._call_claude_cli(extract), and theclaude-clibranch of_call_llm(labeling).The HTTP backend behaves exactly as before (same env var, same 600s default, same parsing). The
claude-cliextraction and labeling subprocesses now respectGRAPHIFY_API_TIMEOUT, and since--api-timeoutsimply exports that env, the flag now applies toclaude-clias well. Non-positive or unparseable values fall back to 600s, matching the HTTP path.Tests
Added to
tests/test_claude_cli_backend.py: four unit tests for_resolve_api_timeout(default, env override, invalid string, non-positive) and two that assert the resolved value reachessubprocess.run'stimeout=argument for both the default and an env override. The file passes (19 tests) andruff checkis clean.