feat(agents): add virtual provider backends config and round-robin routing #157
No reviewers
Labels
No labels
auto/blocked-by-deps
auto/ci-timeout
auto/claimed-implementer
auto/claimed-merge
auto/claimed-reviewer
auto/driver-down
auto/invariant-violation
auto/last-attempt-tier-0
auto/last-attempt-tier-1
auto/last-attempt-tier-2
auto/last-attempt-tier-min
Automation Tracking
auto/needs-conflict-resolution
auto/needs-implementer
auto/postmortem
auto/ready-to-merge
auto/restart-throttled
auto/revert
auto/sentinel
auto/stale-inactivity
auto/unstable
Blocked
Bounty
$100
Bounty
$1000
Bounty
$10000
Bounty
$20
Bounty
$2000
Bounty
$250
Bounty
$50
Bounty
$500
Bounty
$5000
Bounty
$750
MoSCoW
Could have
MoSCoW
Must have
MoSCoW
Should have
Needs Feedback
Points
1
Points
13
Points
2
Points
21
Points
3
Points
34
Points
5
Points
55
Points
8
Points
88
Priority
Backlog
Priority
CI Blocker
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Signed-off: Owner
Signed-off: Scrum Master
Signed-off: Tech Lead
Spike
State
Completed
State
Duplicate
State
In Progress
State
In Review
State
Paused
State
Unverified
State
Verified
State
Wont Do
Type
Automation
Type
Bug
Type
Discussion
Type
Documentation
Type
Epic
Type
Feature
Type
Legendary
Type
Refactor
Type
Support
Type
Task
Type
Testing
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Blocks
#155 Add backends/strategy configuration and round-robin routing for virtual provider LLM agents
cleveragents/cleveractors-core
Reference
cleveragents/cleveractors-core!157
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feature/m1-virtual-provider-round-robin"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Implements issue #155 (the first increment of Epic #154, ADR-2038): an optional
backendslist (>= 2 entries) andstrategyfield (currently onlyround_robin) ontype: llmagents, so a single logical agent can pool several backend credential sets — e.g. multiple OAuth subscriptions for the same provider — and round-robin requests across them to multiply effective capacity.Key design points (full rationale in ADR-2038, status: approved):
VirtualChatModelis a genuineBaseChatModelsubclass (same precedent asReasoningChatModel, ADR-2036) — Liskov-substitutable, no changes needed anywhere else in the agent/tool-loop/graph dispatch stack.LLMAgent._retry_ainvoke/_run_pruning_pass/stream_messagerequired no changes.reasoning_feedback(ADR-2036) is read once at the virtual-provider level and applied identically to every backend, so reasoning-aware backend switching still round-tripsreasoning_contentcorrectly.reasoning_provider_routing_steps.py's_reasoning_unavailablecontext flag wasn't reset between scenarios, silently leaking into later scenarios that reuse its shared "the chat model is constructed" step.Test plan
nox -s lint— greennox -s typecheck— green (Pyright strict, zero suppressions)nox -s unit_tests— green, full suite (155 features / 3144 scenarios)nox -s coverage_report— 96.9% (threshold 96.5%)nox -s security_scan— green (bandit + semgrep)nox -s dead_code— green (vulture)nox -s integration_tests -- robot/virtual_provider_routing.robot— green (Executor → LLMAgent → VirtualChatModel end-to-end through a real tool-call loop, >= 2 backends)benchmarks/virtual_provider_benchmark.py) for the per-invocation backend-selection hot pathnox -s docs— mkdocs build green with the new guide pagedocs/index.md§4.4/§4.4.7 updated, Version bumped 1.7.0 → 1.8.0, §21.1 revision-history row addedexamples/packages/virtual-provider-pooled-agent.yamladdeddocs/guides/virtual-provider-backend-routing.mdadded (practical guide, mirroring the reasoning-aware-llm-agents guide)Closes #155
9a16cc5955fd8f9629eePR Review: !157 (Ticket #155)
Verdict: Approve
No critical or major issues found. The retry-pinning design is sound (verified against
call_with_retry's control flow: an innerExecutionErrorfrom exhausted retries is re-raised immediately, never re-retried or re-routed by the outer wrapper), round-robin wraps correctly, per-backend credential isolation is real, and the Behave/Robot suites assert actual behavior. The remaining findings are minor documentation/edge-case items — none block merge, but a few are trivial to fold in before it.Scope note: no review comments, PR comments, or ticket comments exist anywhere, so there are no author decisions to honor. The
randomstrategy is explicitly deferred to issue #156 and was not flagged. Red-tape items (commit footer format, labels) excluded per review instructions.Critical Issues
None
Major Issues
None
Minor Issues
docs/index.mdlines 320–324 — duplicated table rows in the normative standard. The §4.4 field table contains thebackendsandstrategyrows twice (four consecutive rows instead of two, exact duplicates). Recommendation: delete the second pair.examples/packages/virtual-provider-pooled-agent.yamlline 10 — example advises an unsupported value. The comment says "setstrategy: randomfor uniform random selection instead", butrandomis not supported (VALID_VIRTUAL_PROVIDER_STRATEGIES = {"round_robin"}); following the shipped example produces aConfigurationError. It also contradicts the guide ("round_robinis currently the only supported value"). Recommendation: reword to state thatrandomis planned as a follow-up (issue #156).src/cleveractors/agents/llm_virtual_provider.pylines 365–366 — explicitprovider: null/model: nullin a backend entry is inconsistent between validation and construction. The validation layer treatsNoneas absent (skips checks), but construction usesraw_backend.get("provider", provider), which returnsNonewhen the key exists with a null value — soNone.lower()raisesAttributeError, wrapped as the confusingFailed to initialize backends[i] (None/gpt-4)instead of inheriting the agent's default. Recommendation: normalizeNoneto the agent default (or reject it explicitly in validation).src/cleveractors/agents/factory.pylines 533–544 — credential-injection-mode gap for virtual agents. In ADR-2026 injection mode,AgentFactoryrequirescredentials[provider]to exist for anyllmagent and raisesmissing credentials for provider— yet that looked-up slice is then entirely ignored by the virtual path (build_chat_modeldispatches tobuild_virtual_chat_modelbefore touching it). Abackends-only agent (per-backend credentials, no top-level provider entry) cannot run in injection mode without an otherwise-unused dummy entry. Pre-existing gate, but the interaction is new, untested, and undocumented. Recommendation: skip the provider-credentials requirement whenbackendsis present, or document it.Coverage 96.9% vs the 97% figure (ticket subtask /
noxfile.pydocstring lines 157–160; actual gate constant is the pre-existing 96.5, so CI is green). The gap is the new sync pathsVirtualChatModel._generate/_stream(lines 184–204), which no Behave scenario exercises — all steps useainvoke/astream. Recommendation: add two cheap scenarios using the sync.invoke()/.stream()(fakes already implement both), or explicitly reconcile the 96.5 vs 97.0 mismatch in the noxfile docstring.Nits
docs/index.mdline 511 (§4.4.7) — the body listsrandomas a strategy without a "not yet supported" marker; only the revision-history row and the guide clarify that. A one-word qualifier would keep the normative body consistent with shipped behavior.src/cleveractors/agents/llm_virtual_provider.pyline 43 — first cross-layer import of the privatecleveractors.validation._vocabularyfrom the agents layer. Consider a neutral home for the two constants (or the publiccleveractors.validationsurface).llm_virtual_provider.pylines 251–267 — on a transient first-chunk failure, the abandoned async generator is neveraclose()d before the retry creates a new one. This exactly mirrors the pre-existingLLMAgent.stream_messagepattern (llm.py lines 2134–2148), so it is consistent, not a regression.llm_virtual_provider.pylines 384–390 — the non-ConfigurationErrorwrap omits the DEBUG-level log of the exception type thatbuild_chat_model's sanitize path emits (llm_client.py lines 150–162); a small diagnostics loss.llm.pylines 823–834 —_run_pruning_passconstructs a freshVirtualChatModelper pruning call, so pruning always lands on backend 0 (fresh round-robin counter each pass). Deterministic and harmless, but an undocumented emergent behavior; pruning timeout messages also lose the provider URL (_get_provider_urlfinds no URL attrs on the wrapper).features/virtual_provider.feature— no scenario for explicitstrategy: round_robin(only the omitted-default case is tested end-to-end).ReasoningChatModelpayloads directly, bypassingVirtualChatModel. The mechanism it proves (reasoning travels in message kwargs +reasoning_feedbackapplied identically per backend) is what makes the virtual case work, so it is a legitimate proxy — an end-to-end variant would just be stronger.fd8f962) rather than the atomic feature commit, so neither commit alone fully implements the issue. Linear history with no merge commits, otherwise clean.Summary
A well-executed, carefully-scoped increment.
VirtualChatModelis a genuineBaseChatModelsubclass that composes correctly with the existing stack: backend selection happens exactly once per invocation and is pinned through both the ADR-2032 retry loop and the first-chunk-only streaming retry; the outer_retry_ainvokewrapping degenerates to a no-op exactly as the ticket requires; per-backend construction reuses the unmodified_build_from_credentialspath (including SSRF checks and error sanitization); and the temperature-sync in_select()correctly makes the existing_temperature_overrideflow work through the virtual layer. All ticket acceptance criteria have corresponding, meaningful Behave scenarios, the Robot test drives the real Executor → LLMAgent → VirtualChatModel chain with only the network boundary stubbed, and the accompanying test-isolation fix infeatures/environment.pyis a genuine pre-existing bug fix. The findings above are polish: fix the two documentation defects (trivial), and consider the null-handling and injection-mode items as fast-follows or fold-ins.fd8f9629ee9631ee670a