feat(agents): add random backend-selection strategy for virtual provider LLM agents #158

Merged
CoreRasurae merged 1 commit from feature/m1-virtual-provider-random-strategy into develop 2026-10-20 08:44:01 +00:00
Member

Summary

Implements issue #156 (the second increment of Epic #154, ADR-2038), extending the backends/strategy virtual-provider routing introduced in #155 (PR #157) with the second required selection strategy: strategy: random.

  • strategy: random is now accepted by both validation paths (validate_agents and build_virtual_chat_model's own defensive validation) -- VALID_VIRTUAL_PROVIDER_STRATEGIES now includes "random" alongside "round_robin".
  • RandomStrategy (new SelectionStrategy implementation, Strategy pattern, same precedent as RoundRobinStrategy) selects a backend index via random.randrange, with a # nosec B311 justification comment -- this is a capacity-distribution concern, not a security boundary (ADR-2038 "Selection strategies"). Unlike RoundRobinStrategy, no concurrency guard is needed: there is no shared position to advance.
  • strategy values other than round_robin/random still raise ConfigurationError naming the unsupported value -- two existing scenarios that used "random" as an example of an unsupported value now use "weighted" instead.
  • Selection-and-forward semantics (retry pinning, streaming pinning, error propagation, provider/model defaulting, credential isolation) are identical to round_robin and untouched by this change -- only next_index's algorithm differs.
  • docs/index.md: Version bumped 1.8.0 -> 1.9.0, new §21.1 revision-history row (the §4.4.7 body text already documented random's semantics ahead of time).
  • docs/guides/virtual-provider-backend-routing.md and examples/packages/virtual-provider-pooled-agent.yaml updated to show random is now supported (previously documented as "planned as a follow-up").
  • ASV benchmark (benchmarks/virtual_provider_benchmark.py): added RandomStrategyBenchmark and a random-strategy VirtualChatModel._select case.

Test plan

  • nox -s lint -- green
  • nox -s format -- --check -- green
  • nox -s typecheck -- green (Pyright strict, zero suppressions)
  • nox -s unit_tests -- green, full suite (155 features / 3152 scenarios)
  • nox -s coverage_report -- 97% (threshold 96.5%)
  • nox -s security_scan -- green (bandit + semgrep); bandit B311 suppressed with a justification comment
  • nox -s dead_code -- green (vulture)
  • nox -s integration_tests -- green, full suite (368 tests), including a new Robot test case exercising strategy: random through the library API (Executor -> LLMAgent -> VirtualChatModel -> RandomStrategy)
  • nox -s docs -- mkdocs build green
  • CHANGELOG.md updated

Closes #156

## Summary Implements issue #156 (the second increment of Epic #154, ADR-2038), extending the `backends`/`strategy` virtual-provider routing introduced in #155 (PR #157) with the second required selection strategy: `strategy: random`. - `strategy: random` is now accepted by both validation paths (`validate_agents` and `build_virtual_chat_model`'s own defensive validation) -- `VALID_VIRTUAL_PROVIDER_STRATEGIES` now includes `"random"` alongside `"round_robin"`. - `RandomStrategy` (new `SelectionStrategy` implementation, Strategy pattern, same precedent as `RoundRobinStrategy`) selects a backend index via `random.randrange`, with a `# nosec B311` justification comment -- this is a capacity-distribution concern, not a security boundary (ADR-2038 "Selection strategies"). Unlike `RoundRobinStrategy`, no concurrency guard is needed: there is no shared position to advance. - `strategy` values other than `round_robin`/`random` still raise `ConfigurationError` naming the unsupported value -- two existing scenarios that used `"random"` as an *example* of an unsupported value now use `"weighted"` instead. - Selection-and-forward semantics (retry pinning, streaming pinning, error propagation, provider/model defaulting, credential isolation) are identical to `round_robin` and untouched by this change -- only `next_index`'s algorithm differs. - `docs/index.md`: Version bumped 1.8.0 -> 1.9.0, new §21.1 revision-history row (the §4.4.7 body text already documented `random`'s semantics ahead of time). - `docs/guides/virtual-provider-backend-routing.md` and `examples/packages/virtual-provider-pooled-agent.yaml` updated to show `random` is now supported (previously documented as "planned as a follow-up"). - ASV benchmark (`benchmarks/virtual_provider_benchmark.py`): added `RandomStrategyBenchmark` and a random-strategy `VirtualChatModel._select` case. ## Test plan - [x] `nox -s lint` -- green - [x] `nox -s format -- --check` -- green - [x] `nox -s typecheck` -- green (Pyright strict, zero suppressions) - [x] `nox -s unit_tests` -- green, full suite (155 features / 3152 scenarios) - [x] `nox -s coverage_report` -- 97% (threshold 96.5%) - [x] `nox -s security_scan` -- green (bandit + semgrep); bandit B311 suppressed with a justification comment - [x] `nox -s dead_code` -- green (vulture) - [x] `nox -s integration_tests` -- green, full suite (368 tests), including a new Robot test case exercising `strategy: random` through the library API (Executor -> LLMAgent -> VirtualChatModel -> RandomStrategy) - [x] `nox -s docs` -- mkdocs build green - [x] CHANGELOG.md updated Closes #156
feat(agents): add random backend-selection strategy for virtual provider LLM agents
Some checks failed
CI / lint (pull_request) Successful in 1m32s
CI / quality (pull_request) Successful in 1m10s
CI / typecheck (pull_request) Successful in 1m34s
CI / security (pull_request) Successful in 1m54s
CI / build (pull_request) Successful in 1m16s
CI / integration_tests (pull_request) Successful in 2m19s
CI / unit_tests (pull_request) Successful in 4m22s
CI / coverage (pull_request) Successful in 5m36s
CI / status-check (pull_request) Successful in 2s
CI / benchmark (pull_request) Failing after 25m9s
4e2cb1af79
Extends the backends/strategy virtual-provider routing (#155, ADR-2038)
with the second required selection strategy: strategy: random selects a
backend uniformly at random, via Python's random module, with identical
selection-and-forward semantics to round_robin -- only how the backend
index is chosen differs.

RandomStrategy needs no concurrency guard (unlike RoundRobinStrategy's
position counter): random.randrange has no shared mutable state to
protect. The non-cryptographic random source is deliberate -- this is a
capacity-distribution concern, not a security boundary -- documented with
a `# nosec B311` justification comment for the expected low-severity
bandit finding.

Two existing scenarios used "random" as an example of an *unsupported*
strategy value; both now use "weighted" instead, since random is no
longer unsupported.

ISSUES CLOSED: #156
CoreRasurae added this to the v2.1.0 milestone 2026-10-19 21:40:51 +00:00
hurui200320 left a comment

PR Review: !158 (Ticket #156)

Verdict: Approve

Reviewed the full branch (single atomic commit 4e2cb1a, first line matching the ticket's Commit Message exactly, rebase-clean on develop), the ticket #156 acceptance criteria, Epic #154, ADR-2038, and docs/index.md §4.4.7. There are no prior review comments or author responses on this PR (and none on the ticket), so nothing was decided out-of-scope or deferred — the full review stands.

The implementation satisfies every acceptance criterion: strategy: random is accepted by both validation paths (VALID_VIRTUAL_PROVIDER_STRATEGIES and build_virtual_chat_model's defensive check); RandomStrategy selects uniformly via random.randrange with the required # nosec B311 justification comment and correctly needs no concurrency guard (stateless, unlike RoundRobinStrategy); unsupported values still fail fast naming the value (the two scenarios that used "random" as the unsupported example were correctly switched to "weighted"); statistical-coverage and exactly-one-backend Behave scenarios plus a full-chain Robot integration test are added; docs are consistent throughout (standard bumped 1.8.0 → 1.9.0 in both places, §21.1 revision-history row added, guide/example/CHANGELOG updated, no stale "planned as a follow-up" references remain anywhere). Benchmarks cover the new strategy. No correctness, security, or performance issues found.

Critical Issues

None.

Major Issues

None.

Minor Issues

  1. Random-path tests never assert the response is forwarded unmodified — features/virtual_provider.feature:129-132 ("Random strategy forwards each call to exactly one backend") asserts only call counts, and robot/virtual_provider_routing.robot:28-39 asserts only Result Is Valid Actor Result plus per-backend request counts — yet the Robot keyword's docstring claims it proves "that backend's response is returned unmodified". The ticket's acceptance criterion explicitly requires "forwards the call/response unmodified". Both layers already hold the data to assert this cheaply: in Behave, assert context.invocation_order == [<the called backend's name>]; in Robot, assert the response matches whichever backend received the request (e.g. final-answer-from-backend-0/-1). The forwarding code is shared with round_robin (whose tests do assert content), so this is a test-strengthening suggestion, not a bug.

Nits

  1. benchmarks/virtual_provider_benchmark.py:47-49 — _make_virtual_chat_model's strategy parameter is typed as the concrete union RoundRobinStrategy | RandomStrategy | None; the SelectionStrategy ABC (importable from the same module) is the natural type and keeps a future third strategy from touching this signature (the extensibility rationale in ADR-2038 A-5).
  2. src/cleveractors/agents/llm_virtual_provider.py:130 — at 121 characters this is now the longest line in src/. Lint/format pass (E501 is ignored project-wide and the formatter doesn't split trailing comments), but putting the # nosec B311 justification on its own line above the return would keep the file within the configured 88-column style.
  3. src/cleveractors/agents/llm_virtual_provider.py:102-105 and 126-129 — the num_backends <= 0 → ConfigurationError("num_backends must be positive, ...") guard is duplicated across both strategies; hoisting it into SelectionStrategy (or a shared helper) would prevent a third copy when weighted lands.
  4. Docstring staleness — robot/virtual_provider_routing.robot:3-7 and robot/VirtualProviderRoutingTestLib.py:1-25 still describe the suite as exercising "the real round-robin routing code" / RoundRobinStrategy only; it now also covers RandomStrategy.
  5. For completeness: there is no dedicated error-propagation scenario with strategy: random. This is defensible as-is — retry/streaming pinning lives in VirtualChatModel/call_with_retry and is strategy-independent, and it is covered by the round_robin scenarios — so no action is strictly required; noting it only because it maps to an explicit acceptance criterion of #156.

Summary

A clean, well-scoped increment that completes the second (and final) child issue of the virtual-provider Epic #154. The implementation follows the established Strategy pattern exactly, reuses the #155 selection-and-forward machinery without modification (so retry pinning, streaming pinning, error propagation, provider/model defaulting, and credential isolation all carry over unchanged), and the test additions are meaningful — statistical coverage at a failure probability of ~1e-35, a fresh zero-backends guard scenario, strategy build assertions, and an end-to-end Robot case through the real build_chat_model → build_virtual_chat_model chain. Documentation, standard versioning, revision history, changelog, and benchmarks are all consistent. The minor and nit items above are non-blocking polish. Nice work.

## PR Review: !158 (Ticket #156) ### Verdict: Approve Reviewed the full branch (single atomic commit `4e2cb1a`, first line matching the ticket's Commit Message exactly, rebase-clean on `develop`), the ticket #156 acceptance criteria, Epic #154, ADR-2038, and `docs/index.md` §4.4.7. There are no prior review comments or author responses on this PR (and none on the ticket), so nothing was decided out-of-scope or deferred — the full review stands. The implementation satisfies every acceptance criterion: `strategy: random` is accepted by both validation paths (`VALID_VIRTUAL_PROVIDER_STRATEGIES` and `build_virtual_chat_model`'s defensive check); `RandomStrategy` selects uniformly via `random.randrange` with the required `# nosec B311` justification comment and correctly needs no concurrency guard (stateless, unlike `RoundRobinStrategy`); unsupported values still fail fast naming the value (the two scenarios that used `"random"` as the unsupported example were correctly switched to `"weighted"`); statistical-coverage and exactly-one-backend Behave scenarios plus a full-chain Robot integration test are added; docs are consistent throughout (standard bumped 1.8.0 → 1.9.0 in both places, §21.1 revision-history row added, guide/example/CHANGELOG updated, no stale "planned as a follow-up" references remain anywhere). Benchmarks cover the new strategy. No correctness, security, or performance issues found. ### Critical Issues None. ### Major Issues None. ### Minor Issues 1. **Random-path tests never assert the response is forwarded unmodified** — `features/virtual_provider.feature:129-132` ("Random strategy forwards each call to exactly one backend") asserts only call counts, and `robot/virtual_provider_routing.robot:28-39` asserts only `Result Is Valid Actor Result` plus per-backend request counts — yet the Robot keyword's docstring claims it proves "that backend's response is returned unmodified". The ticket's acceptance criterion explicitly requires "forwards the call/response unmodified". Both layers already hold the data to assert this cheaply: in Behave, assert `context.invocation_order == [<the called backend's name>]`; in Robot, assert the response matches whichever backend received the request (e.g. `final-answer-from-backend-0`/`-1`). The forwarding code is shared with round_robin (whose tests do assert content), so this is a test-strengthening suggestion, not a bug. ### Nits 1. `benchmarks/virtual_provider_benchmark.py:47-49` — `_make_virtual_chat_model`'s `strategy` parameter is typed as the concrete union `RoundRobinStrategy | RandomStrategy | None`; the `SelectionStrategy` ABC (importable from the same module) is the natural type and keeps a future third strategy from touching this signature (the extensibility rationale in ADR-2038 A-5). 2. `src/cleveractors/agents/llm_virtual_provider.py:130` — at 121 characters this is now the longest line in `src/`. Lint/format pass (E501 is ignored project-wide and the formatter doesn't split trailing comments), but putting the `# nosec B311` justification on its own line above the `return` would keep the file within the configured 88-column style. 3. `src/cleveractors/agents/llm_virtual_provider.py:102-105 and 126-129` — the `num_backends <= 0` → `ConfigurationError("num_backends must be positive, ...")` guard is duplicated across both strategies; hoisting it into `SelectionStrategy` (or a shared helper) would prevent a third copy when `weighted` lands. 4. Docstring staleness — `robot/virtual_provider_routing.robot:3-7` and `robot/VirtualProviderRoutingTestLib.py:1-25` still describe the suite as exercising "the real round-robin routing code" / `RoundRobinStrategy` only; it now also covers `RandomStrategy`. 5. For completeness: there is no dedicated error-propagation scenario with `strategy: random`. This is defensible as-is — retry/streaming pinning lives in `VirtualChatModel`/`call_with_retry` and is strategy-independent, and it is covered by the round_robin scenarios — so no action is strictly required; noting it only because it maps to an explicit acceptance criterion of #156. ### Summary A clean, well-scoped increment that completes the second (and final) child issue of the virtual-provider Epic #154. The implementation follows the established Strategy pattern exactly, reuses the #155 selection-and-forward machinery without modification (so retry pinning, streaming pinning, error propagation, provider/model defaulting, and credential isolation all carry over unchanged), and the test additions are meaningful — statistical coverage at a failure probability of ~1e-35, a fresh zero-backends guard scenario, strategy build assertions, and an end-to-end Robot case through the real `build_chat_model` → `build_virtual_chat_model` chain. Documentation, standard versioning, revision history, changelog, and benchmarks are all consistent. The minor and nit items above are non-blocking polish. Nice work.
CoreRasurae force-pushed feature/m1-virtual-provider-random-strategy from 4e2cb1af79
Some checks failed
CI / lint (pull_request) Successful in 1m32s
CI / quality (pull_request) Successful in 1m10s
CI / typecheck (pull_request) Successful in 1m34s
CI / security (pull_request) Successful in 1m54s
CI / build (pull_request) Successful in 1m16s
CI / integration_tests (pull_request) Successful in 2m19s
CI / unit_tests (pull_request) Successful in 4m22s
CI / coverage (pull_request) Successful in 5m36s
CI / status-check (pull_request) Successful in 2s
CI / benchmark (pull_request) Failing after 25m9s
to 4b98ce0067
Some checks failed
CI / lint (pull_request) Successful in 1m13s
CI / security (pull_request) Successful in 2m12s
CI / build (pull_request) Successful in 2m9s
CI / typecheck (pull_request) Successful in 2m50s
CI / quality (pull_request) Successful in 2m56s
CI / integration_tests (pull_request) Successful in 4m1s
CI / unit_tests (pull_request) Successful in 6m49s
CI / coverage (pull_request) Successful in 6m11s
CI / status-check (pull_request) Successful in 2s
CI / benchmark (pull_request) Failing after 17m14s
CI / lint (push) Successful in 40s
CI / typecheck (push) Successful in 2m2s
CI / quality (push) Successful in 2m8s
CI / security (push) Successful in 2m37s
CI / build (push) Successful in 1m26s
CI / integration_tests (push) Successful in 4m12s
CI / unit_tests (push) Successful in 6m26s
CI / coverage (push) Successful in 5m33s
CI / status-check (push) Successful in 6s
CI / benchmark (push) Failing after 24m16s
2026-10-20 08:22:51 +00:00
Compare
Author
Member

Addressed rui.hu's review (approved, with Minor/Nit polish items) by amending the commit:

Minor Issue 1 (fixed) — Random-path tests now assert the response is forwarded unmodified, not just that a backend was called:

  • features/steps/virtual_provider_steps.py: exactly one configured backend should have been called now also asserts context.invocation_order == [<called backend>].
  • robot/VirtualProviderRoutingTestLib.py: Exactly One Backend Should Have Received A Request now also asserts the response contains final-answer-from-backend-{N} for whichever backend was called.

Nit 1 (fixed) — benchmarks/virtual_provider_benchmark.py: _make_virtual_chat_model's strategy parameter is now typed as the SelectionStrategy ABC instead of the concrete union.

Nit 2 (not applied) — Verified via nox -s security_scan: bandit's # nosec suppression only works on the same physical line as the flagged call; moving it to the line above (as suggested) makes bandit re-flag B311 and fails the security gate. Left as-is.

Nit 3 (fixed) — The duplicated num_backends <= 0 guard is hoisted into a shared SelectionStrategy._validate_num_backends helper, called by both RoundRobinStrategy and RandomStrategy.

Nit 4 (fixed) — robot/virtual_provider_routing.robot and robot/VirtualProviderRoutingTestLib.py docstrings now mention RandomStrategy alongside RoundRobinStrategy.

Nit 5 (skipped) — Per your own note, no action required (strategy-independent, already covered by round_robin scenarios).

Verified via nox (scoped to this feature): lint, typecheck, security_scan, dead_code (full suite, all green); unit_tests -- features/virtual_provider.feature (43/43 scenarios passed); integration_tests -- robot/virtual_provider_routing.robot (2/2 passed); coverage_report -- features/virtual_provider.feature (99% on llm_virtual_provider.py, the one pre-existing uncovered line is an unrelated exception-reraise branch); benchmark (VirtualChatModelSelectBenchmark, no regressions).

Commit amended in place (no force-push risk to shared history — single-commit feature branch).

Addressed rui.hu's review (approved, with Minor/Nit polish items) by amending the commit: **Minor Issue 1 (fixed)** — Random-path tests now assert the response is forwarded unmodified, not just that a backend was called: - `features/steps/virtual_provider_steps.py`: `exactly one configured backend should have been called` now also asserts `context.invocation_order == [<called backend>]`. - `robot/VirtualProviderRoutingTestLib.py`: `Exactly One Backend Should Have Received A Request` now also asserts the response contains `final-answer-from-backend-{N}` for whichever backend was called. **Nit 1 (fixed)** — `benchmarks/virtual_provider_benchmark.py`: `_make_virtual_chat_model`'s `strategy` parameter is now typed as the `SelectionStrategy` ABC instead of the concrete union. **Nit 2 (not applied)** — Verified via `nox -s security_scan`: bandit's `# nosec` suppression only works on the *same physical line* as the flagged call; moving it to the line above (as suggested) makes bandit re-flag B311 and fails the security gate. Left as-is. **Nit 3 (fixed)** — The duplicated `num_backends <= 0` guard is hoisted into a shared `SelectionStrategy._validate_num_backends` helper, called by both `RoundRobinStrategy` and `RandomStrategy`. **Nit 4 (fixed)** — `robot/virtual_provider_routing.robot` and `robot/VirtualProviderRoutingTestLib.py` docstrings now mention `RandomStrategy` alongside `RoundRobinStrategy`. **Nit 5 (skipped)** — Per your own note, no action required (strategy-independent, already covered by round_robin scenarios). Verified via nox (scoped to this feature): `lint`, `typecheck`, `security_scan`, `dead_code` (full suite, all green); `unit_tests -- features/virtual_provider.feature` (43/43 scenarios passed); `integration_tests -- robot/virtual_provider_routing.robot` (2/2 passed); `coverage_report -- features/virtual_provider.feature` (99% on `llm_virtual_provider.py`, the one pre-existing uncovered line is an unrelated exception-reraise branch); `benchmark` (VirtualChatModelSelectBenchmark, no regressions). Commit amended in place (no force-push risk to shared history — single-commit feature branch).
CoreRasurae deleted branch feature/m1-virtual-provider-random-strategy 2026-10-20 08:44:01 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Reference
cleveragents/cleveractors-core!158
No description provided.