Add auxiliary same-provider model fallbacks - #17235
getyolkboy wants to merge 1 commit into
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused sync/async fallback coverage. Current main now has a shared per-task auxiliary fallback architecture, but it intentionally excludes a fallback whose provider matches the failed provider (agent/auxiliary_client.py:3911-3920). That means the same-provider/different-model use case in this PR is still not present, but it should be integrated as a policy decision in the existing chain rather than added as a parallel fallback path.
Problems
- Current fallback selection and dispatch live in
agent/auxiliary_client.py:3889-3957,:6908-6928, and:7413-7433; the April patch predates those paths and cannot be salvaged mechanically. - The current documented contract also says matching-provider entries are skipped (
website/docs/user-guide/configuration.md:1095-1109). The change needs to define when a different model on the same provider is safe to treat as a distinct target.
Suggested changes
- Extend the shared
fallback_chainselector to distinguish a different model/endpoint from the exact failed target, with sync and async regression coverage for ordered same-provider traversal and exact-target skipping.
Automated hermes-sweeper review.
| @@ -2767,6 +2785,26 @@ def _get_auxiliary_task_config(task: str) -> Dict[str, Any]: | |||
| return task_config if isinstance(task_config, dict) else {} | |||
There was a problem hiding this comment.
Current main now centralizes per-task fallback selection in _try_configured_fallback_chain, which deliberately skips candidates matching the failed provider (agent/auxiliary_client.py:3911-3920). Please integrate same-provider/different-model semantics into that shared policy rather than add a parallel fallback_models path, and preserve an explicit guard against retrying the identical failed target.
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Two PRs address same-provider auxiliary model fallback behavior. #17235 adds a separate fallback_models retry path for selected transient errors, while #59561 fixes the shared fallback_chain so sibling models on the failed provider remain eligible for model-specific failures without weakening provider-wide auth/payment handling.
Related pull requests
- #17235
related— (+276/-0) — close as duplicate of #59561: the patch adds a parallelfallback_modelsselector and sync/async dispatch path, but the later #59561 change implements the same-provider/different-model behavior inside the shared fallback architecture. Despite the keep_open review on #17235, its requested salvage path—integrating the policy intofallback_chainrather than retaining a parallel path—was subsequently implemented through #59561 and carried onto main via #72468. - #59561 [closed]
duplicate— (+199/-7) — relevant merged reference implementation via #72468: it narrows skipping to the exact failed(provider, model)for model-specific failures while retaining provider-wide skipping for auth and payment failures, with sync/async regression coverage. The automated keep_open verdict was superseded by the contributor's later confirmation that #59561's two commits were cherry-picked into #72468 with authorship preserved, so the closed PR remains the implementation reference.
Duplicates
#17235 and #59561 target substantially the same same-provider sibling-model fallback outcome, but through competing mechanisms; use the chain #17235 → duplicate of #59561 → incorporated into #72468.
Suggested consolidation
Close #17235 as duplicate of #59561, with the explicit chain that #59561 was incorporated into #72468. The later shared-chain implementation addresses the policy concern raised in the keep_open review on #17235 and preserves the important auth/payment provider-wide carve-out, whereas retaining #17235 would introduce a second fallback configuration and dispatch path; keep #59561 closed as the superseded implementation record.
Complex graph
flowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
subgraph Dup17235 ["PRs duplicating each other"]
P17235["PR #17235 (open)"]
P59561["PR #59561 (closed)"]
end
class P17235 open
class P59561 closed
class P17235 target
click P17235 "https://github.com/NousResearch/hermes-agent/pull/17235"
click P59561 "https://github.com/NousResearch/hermes-agent/pull/59561"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).
Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 33 kB of PR diffs, 9 kB of issue/PR text, 3 kB of discussion (3 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
What does this PR do?