Skip to content

Track 0: refresh/ask reliability + opt-in multi-provider LLM failover - #84

Merged
study8677 merged 2 commits into
mainfrom
fix/track0-reliability
May 24, 2026
Merged

Track 0: refresh/ask reliability + opt-in multi-provider LLM failover#84
study8677 merged 2 commits into
mainfrom
fix/track0-reliability

Conversation

@study8677

Copy link
Copy Markdown
Owner

What & why

Sustained LLM-provider outages (the shared-proxy 503s) invalidated several recent
benchmark runs, and FastAPI's all-test module overflowed the provider instruction
limit during refresh. This branch stops that bleeding.

Commits

fix(hub): chunk all-test modules, fix underscore module paths, retry transient ask failures

  • module_grouping: route all-test modules through chunking (+ per-group file cap)
    so large test suites no longer build a single ~2.4M-char agent instruction that
    exceeds the 1.05M provider limit.
  • scanner: resolve_module_path tries the longest existing parent prefix first,
    fixing underscored parents like docs_src_additional_responses.
  • ask_pipeline: same-provider exponential-backoff retry for transient failures
    (503 / timeout / litellm ServiceUnavailableError).

feat(hub): opt-in multi-provider LLM failover for the ask path

  • New _providers.py: provider chain (primary + AG_LLM_FALLBACKS), a shared
    retryable-error classifier, and a wrapper that fails over to the next provider
    and re-runs the answer when the active one keeps failing.
  • ask_pipeline split into a failover wrapper + _ask_pipeline_once.
  • Default behaviour unchanged: with no AG_LLM_FALLBACKS, the chain is length 1
    and the wrapper is a pass-through (no env mutation).

Testing

  • Focused failover suite + affected hub tests green (113 passed in the relevant subset).
  • Full engine suite: 202 passed; the single failure is a pre-existing repo-walk test
    that trips on untracked local agent-worktree copies, not a regression (clean on CI).

Out of scope

  • Cross-provider failover for refresh (partial-success semantics) — folded into the
    upcoming incremental-refresh work.

Config (opt-in)

AG_LLM_FALLBACKS=[{"base_url":"https://api.openai.com/v1","api_key":"sk-...","model":"gpt-4o","label":"openai"}]

🤖 Generated with Claude Code

study8677 and others added 2 commits May 24, 2026 18:19
…transient ask failures

- module_grouping: route all-test modules through chunking and cap files
  per group so large test suites don't overflow the provider instruction
  limit (FastAPI tests module hit 2.4M chars > 1.05M).
- scanner: resolve_module_path tries the longest existing parent prefix
  first, fixing underscored parents like docs_src_additional_responses.
- ask_pipeline: retry transient provider failures (503/timeout/litellm
  ServiceUnavailableError) with exponential backoff on the ask path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sustained provider outages (e.g. the shared-proxy 503s that invalidated
several benchmark runs) cannot be recovered by same-provider retries —
every retry hits the same dead host. Add an ordered provider chain and
wrap the ask pipeline so a transient/provider error on the active
endpoint fails over to the next provider and re-runs the answer.

- _providers.py: ProviderConfig, get_provider_chain (parses
  AG_LLM_FALLBACKS, degrades to primary on bad input), activate_provider,
  and the run_with_provider_failover wrapper, plus one shared
  retryable-error classifier (is_retryable_provider_error).
- ask_pipeline: split into a failover wrapper + _ask_pipeline_once;
  _is_retryable_ask_error now delegates to the shared classifier.
- Unchanged when AG_LLM_FALLBACKS is unset (chain length 1 => pass-through,
  no env mutation).
- Tests for chain parsing, inheritance, bad-JSON degradation, failover
  rotation, and non-transient pass-through.
- Document AG_ASK_RETRY_* and AG_LLM_FALLBACKS in .env.example.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@study8677 study8677 left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

总体评价

PR 整体质量较高,解决了真实的痛点(provider 宕机导致基准测试失效)。模块分块修复和下划线路径解析是干净的 bug fix,重试和故障转移机制设计为可选项且默认行为不变,测试覆盖较充分。存在两个值得修复的问题:关键词匹配存在子字符串误判风险,以及 os.environ 全局突变在并发场景下不安全。整体倾向 Comment(可合并,但建议跟进修复)。


问题清单

级别 文件 & 行号 描述 建议
🟡 建议 hub/_providers.py L38-57 _RETRYABLE_KEYWORDS 中的纯数字字符串("500", "502", "503", "504", "429")作为子字符串匹配会产生误判。例如 RuntimeError("processed 15000 tokens") 的 msg 为 "builtins.runtimeerror: processed 15000 tokens",其中 "500""15000" 的子串,导致本不应重试的真实逻辑错误被静默重试 3 次,浪费最多 35s 才失败 用正则 \b(429|500|502|503|504)\b 匹配数字码,文字关键词保持子串匹配
🟡 建议 hub/_providers.py activate_provider os.environ 突变不是协程安全的。若有多个并发 ask_pipeline 调用(web server 场景),其中一个触发 failover 后,os.environ["OPENAI_BASE_URL"]reset_settings() 会影响所有其他正在进行的请求,导致它们无声地切换 provider 在代码注释中明确说明"当前实现假设同时只有一个 ask 在飞";或将 provider config 通过参数传递而非 mutate 全局 env(可作后续跟进)
🟡 建议 hub/ask_pipeline.py & hub/_providers.py 重试/failover 的警告信息使用 print(..., file=sys.stderr) 而非 logger.warning(),与模块其他地方的日志风格不一致,也绕过了外部日志配置 改用 logger.warning(...)
🟢 优化 hub/_providers.py L183-188 assert last_exc is not None 在 Python -O(优化模式)下会被跳过,之后 raise last_exc 可能触发 UnboundLocalError 改为普通 if last_exc is None: raise RuntimeError("unreachable")
🟢 优化 hub/scanner.py L788-789 _find_venv_dirs(root) 现在在进入循环前就调用,即使找不到任何有效父目录也会执行 影响极小,可低优先级跟进
🟢 优化 hub/_providers.py get_provider_chain 每次 ask 调用都解析 AG_LLM_FALLBACKS JSON,若 ask 频繁可考虑缓存 可加 @functools.lru_cache 或在 Settings 初始化时解析一次

亮点

  • _RETRYABLE_KEYWORDS 作为单一真相来源:同一个分类器同时服务同 provider 重试路径和跨 provider failover 路径,避免了两处维护不同步的风险。
  • run_with_provider_failover 单 provider 快路径len(providers) <= 1 时直接透传,不触碰 os.environ,确保默认行为零开销零副作用。
  • module_grouping.pyif non_test_files else []:避免在纯测试模块下生成空的 "main" 组,小而准确。
  • scanner.py 最长前缀优先:从 len(parts)-1 向下遍历,正确处理父目录名含下划线的情况,逻辑清晰。
  • 测试覆盖test_hub_providers.py 对分类器、链解析、failover 逻辑分别做了参数化测试,质量较高。

修改示例

🟡 关键词匹配误判修复

import re

_RETRYABLE_CODE_RE = re.compile(r"\b(429|500|502|503|504)\b")
_RETRYABLE_TEXT_KEYWORDS = (
    "timeout",
    "gateway time-out",
    "connection",
    "network",
    "unreachable",
    "refused",
    "rate limit",
    "ratelimit",
    "serviceunavailable",
    "service unavailable",
    "service temporarily unavailable",
    "temporarily unavailable",
    "bad gateway",
    "internalservererror",
    "internal server error",
)

def is_retryable_provider_error(exc: Exception) -> bool:
    if isinstance(exc, (TimeoutError, asyncio.TimeoutError)):
        return True
    msg = f"{type(exc).__module__}.{type(exc).__name__}: {exc}".lower()
    if _RETRYABLE_CODE_RE.search(msg):
        return True
    return any(kw in msg for kw in _RETRYABLE_TEXT_KEYWORDS)

🟡 并发安全说明注释(在 activate_provider docstring 末尾追加)

    # NOTE: This mutates the process-wide os.environ and is NOT coroutine-safe.
    # Assumes at most one ask_pipeline call is in flight at any time.
    # Concurrent asks with failover will silently share the same provider switch.

🟡 print → logger

# ask_pipeline.py
logger.warning(
    "Ask attempt %d failed%s: %s. Retrying in %.1fs...",
    attempt + 1, label, error_msg, delay,
)

# _providers.py
logger.warning(
    "Provider '%s' failed for %s: %s. Failing over to '%s'...",
    provider.label, label, raw_msg or type(exc).__name__, next_label,
)

🟢 assert → 普通检查

if last_exc is None:
    raise RuntimeError("unreachable: loop exited without result or exception")
raise last_exc

Generated by Claude Code

@study8677
study8677 merged commit f6d30a1 into main May 24, 2026
8 checks passed
@study8677
study8677 deleted the fix/track0-reliability branch May 24, 2026 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant