You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Stage 2: LLM calls have no retry/backoff on 429 or timeout; default concurrency 10 is unconfigurable #10
The Stage 2 LLM calls have no retry on transient failures. A single HTTP 429 or a single request timeout fails the call outright, and (because of the related abort-everything behaviour) takes the whole semantic pass down with it. There is also no backoff, and the default concurrency is high enough that hosted tiers 429 almost immediately.
None of these are wrapped in a retry. A RateLimitError (429) or an APITimeoutError raises on the first attempt and that is the end of it.
The default concurrency makes this easy to hit, :368:
max_concurrency: int=10,
Ten parallel requests with no backoff is more than several hosted tiers will accept. On the NVIDIA build tier I saw nearly every call 429, and there is no env knob to turn the concurrency down without editing the source.
Why this matters
429 and timeout are the normal, expected failure modes of every hosted inference API; they are not exceptional. Treating the first one as fatal means the semantic pass is fragile exactly where it should be resilient. Combined with the all-or-nothing fallback, the practical result is that on a hosted tier the LLM filter frequently does not run at all, and the user is not told.
I do recognise that adding retries trades wall-clock for reliability, and that nobody wants an unbounded retry storm. A small bounded exponential backoff (a handful of attempts) is the usual middle ground and is plenty here.
Reproduce
Use a provider/tier with a low requests-per-minute ceiling.
Run skillspector scan <dir> --verbose over a tree with enough files to exceed the ceiling at concurrency 10.
Observe repeated 429s, no retry, and (via the related abort bug) a fallback to static-only.
Suggested direction
Add a bounded exponential-backoff retry around the four invoke sites, scoped to genuinely transient errors only: rate-limit (429) and timeout. Do not retry 4xx like the context-overflow 400, because retrying that is pointless; let it fail the batch and be handled by per-batch isolation.
Make max_concurrency configurable by environment variable (something like SKILLSPECTOR_MAX_CONCURRENCY) and consider a lower default than 10, since the safe ceiling varies a lot by provider and tier.
Detect the retryable cases defensively across SDKs (check for a 429 status, a RateLimit/Timeout error name, or the matching text) rather than importing one provider's exception types.
Related: this compounds with the abort-everything fallback. Retries reduce how often a transient error fires; per-batch isolation limits the damage when one finally does.
Reproducing on a hosted free tier (v2.1.3). Scanning a ~35-file skill package with SKILLSPECTOR_PROVIDER=nv_build (NVIDIA build.nvidia.com free tier, ~40 req/min):
Default concurrency burst-trips the free-tier limit immediately; all three semantic analyzers 429 and the pass falls back to static-only (#9). Net effect: the package scores 58/100 HIGH / DO NOT INSTALL purely on static false-positives, even though the LLM calls that did complete returned findings=0. A retry + exponential backoff on 429/timeout, plus configurable concurrency (--concurrency or env), would let the semantic pass finish on rate-limited tiers and clear those FPs.
The remaining transient-failure retry work is in progress in comprehensive PR #555, which covers provider-neutral 5xx, timeout, throttling, Bedrock, bounded backoff, and Retry-After; the older narrower 429 implementation is PR #403. Keeping this issue open until an accepted fix merges and the original hosted-tier cases are verified.
Resolved on main by PR #555, merged September 21. It adds bounded retries for transient provider failures, including 429/throttling, timeouts, transport errors, and 5xx responses; honors bounded Retry-After; and keeps deterministic request/context-overflow errors non-retryable. Configurable concurrency was already delivered by PR #305.
The sync/async retry, deadline, retry-exhaustion, and global-concurrency regression tests pass on main at 6685a1b. This validation uses controlled provider failures, not live hosted-tier calls. Closing as completed on main; the retry change is not yet in the latest published release, v2.11.2.
Activity
HetCreep commentedon Jun 13, 2026
Reproducing on a hosted free tier (v2.1.3). Scanning a ~35-file skill package with
SKILLSPECTOR_PROVIDER=nv_build(NVIDIA build.nvidia.com free tier, ~40 req/min):Default concurrency burst-trips the free-tier limit immediately; all three semantic analyzers 429 and the pass falls back to static-only (#9). Net effect: the package scores 58/100 HIGH / DO NOT INSTALL purely on static false-positives, even though the LLM calls that did complete returned
findings=0. A retry + exponential backoff on 429/timeout, plus configurable concurrency (--concurrencyor env), would let the semantic pass finish on rate-limited tiers and clear those FPs.rng1995 commentedon Sep 16, 2026
The remaining transient-failure retry work is in progress in comprehensive PR #555, which covers provider-neutral 5xx, timeout, throttling, Bedrock, bounded backoff, and
Retry-After; the older narrower 429 implementation is PR #403. Keeping this issue open until an accepted fix merges and the original hosted-tier cases are verified.rng1995 commentedon Sep 21, 2026
Resolved on
mainby PR #555, merged September 21. It adds bounded retries for transient provider failures, including 429/throttling, timeouts, transport errors, and 5xx responses; honors boundedRetry-After; and keeps deterministic request/context-overflow errors non-retryable. Configurable concurrency was already delivered by PR #305.The sync/async retry, deadline, retry-exhaustion, and global-concurrency regression tests pass on
mainat6685a1b. This validation uses controlled provider failures, not live hosted-tier calls. Closing as completed onmain; the retry change is not yet in the latest published release, v2.11.2.