refactor(llm): migrate phase.rs explicit-pair dispatch (dispatch_to_provider_for + cascade loops)
- Linguagem predominante
- Rust
- Estrelas
- 0
- Forks
- 1
- Merge médio
- 35min
- PRs com merge (30d)
- 238
Descrição
## Goal
Migrate `src/phases/phase.rs::dispatch_to_provider_for` (the **explicit-pair** dispatch — takes `section` and `model_id` as parameters, used by the discovery matrix / coordinator for multi-provider fan-out) from `Arc` to `Arc`. Includes the two cascade loops at `phase.rs:1968-1985` (preflight) and `:2011-2057` (retry). Lands alongside the existing `Provider`-using code path.
## Why now
The explicit-pair dispatch is the call-site used by the discovery subsystem (`src/discovery/coordinator.rs`, `src/discovery/matrix.rs`, etc.) when fanning out across multiple `(provider, model)` pairs. It mirrors `dispatch_to_provider` structurally — same cascade pattern, just with caller-supplied routing instead of `self.default_provider`. Migrating it after #5 (default-pair) is the natural next step because both functions share the same `BreakeredClient` adapter and the same `convert_request`/`convert_response` helpers from #5.
## Scope
**In scope:**
- `phase.rs::dispatch_to_provider_for` (lines 1898-2244): switch from `Arc` to `Arc`.
- The two cascade loops in this function (preflight at `:1968-1985`, retry at `:2011-2057`): same shape as #5's migration.
- The audit-hash branch at `phase.rs:1994` (`if section == "minimax"`) **stays** — #14 removes it.
- The `RunContext::provider_for(section, model_id)` accessor gets a sibling `RunContext::llm_client_for(section, model_id)` that returns `Arc`. The old accessor stays.
- New unit tests at `phase.rs::tests::v2_for::*` that pin the explicit-pair cascade contract.
**Out of scope:**
- Default-pair dispatch migration — issue #5.
- Migration of probe / CLI / other phases / discovery / tests — issues #7-#11.
- Cascade absorption into `LlmClient::send` (D9) — issue #14.
- Removal of the `if section == "minimax"` audit-hash branch (D8) — issue #14.
- Deletion of `dispatch_to_provider_for`'s legacy `Provider`-based code path — issue #15.
## Approach
### 1. `llm_client_for` accessor
```rust
impl RunContext {
pub(crate) async fn llm_client_for(&self, section: &str, model_id: &str) -> Arc {
// Resolves (section, model_id) via the same lookup path as provider_for(),
// wraps the result in a BreakeredClient adapter (introduced in #5).
...
}
}
```
Same dual-path strategy as #5: both `provider_for` and `llm_client_for` exist; `llm_client_for` wraps the legacy `BreakeredProvider` in `BreakeredClient`.
### 2. `dispatch_to_provider_for` migration
The function signature changes from:
```rust
async fn dispatch_to_provider_for(&self, section, model_id, req, cache_key, started_unix, retry_count) -> Result
```
to:
```rust
async fn dispatch_to_provider_for(&self, section, model_id, req, cache_key, started_unix, retry_count) -> Result
```
Internally:
- `self.provider_for(section, model_id)` → `self.llm_client_for(section, model_id)`.
- `provider.send(&hash_input)` → `client.send(&hash_input)`.
- The 4xx cascade loop now calls `client.send` (no section-name branching).
- The `if section == "minimax"` audit-hash branch **stays** (D8 = #14).
- All 4 `provider.effective_max_tokens(&req)` calls drop in favour of `client.body_sha256(&req)`.
### 3. Unit tests migration
Add 3 new tests in `phase.rs::tests::v2_for::*`:
- `dispatch_for_recovers_from_three_param_cascade` — mirror of the existing `dispatch_recovers_from_three_param_cascade` (which tests default-pair), but for explicit-pair.
- `dispatch_for_aborts_when_detector_returns_none` — mirror.
- `dispatch_for_caps_at_param_names_len` — mirror.
Each uses a `ScriptedLlmClient` stub that returns programmable 4xx bodies.
The legacy explicit-pair tests (using `ScriptedProvider`) stay green throughout — both code paths run until #15.
## Acceptance criteria
- [ ] `cargo build --release --all-features` succeeds with **zero warnings**.
- [ ] `make fmt-check guard-deps lint build test-ci` green.
- [ ] `phase.rs::dispatch_to_provider_for` is fully migrated to `Arc`.
- [ ] The 3 new `v2_for::*` unit tests pass.
- [ ] The legacy `ScriptedProvider`-based explicit-pair tests still pass.
- [ ] The integration tests in `tests/integration_phase_*.rs`, `tests/integration_discover_legacy_paths.rs`, `tests/integration_pr09_provider_pool.rs` continue to pass.
- [ ] Smoke: `moagan run --mode fast --provider mock:mock-model` produces `final/portfolio.md` and `rankings/ranking.json` byte-identical to v0.17.6.
- [ ] The `Provider`-using helpers (`provider_for`, `BreakeredProvider` direct construction) are untouched in the legacy code paths.
## Merge order
```
#1-#4 (foundation wave)
↓
#5 (phase.rs default-pair migration)
↓
#6 (this issue — phase.rs explicit-pair migration — parallel to #5 is fine, but #6 should land after #5's BreakeredClient adapter exists)
↓
#7-#11 (other migrations)
```
Can be merged in parallel with #5 if both PRs agree on the `BreakeredClient` API surface. Practically: ship #5 first, then #6.
## Validation
- `make fmt-check guard-deps lint build test-ci` green after the PR lands.
- 3 new `v2_for::*` unit tests in `phase.rs::tests` pass.
- Legacy `ScriptedProvider`-based tests still pass.
- Smoke (`moagan run --mode fast --provider mock:mock-model`) produces the same artefacts.
## Version target
v0.18.0.
## References
- [EPIC #847](https://github.com/airvzxf/moagan/issues/847) — the umbrella.
- [#900 D8, D9](https://github.com/airvzxf/moagan/issues/900) — D8 + D9 deferred to #14.
- [`src/phases/phase.rs:1898-2244`](../blob/main/src/phases/phase.rs) — `dispatch_to_provider_for` and its cascade loops.
- [`src/discovery/coordinator.rs`](../blob/main/src/discovery/coordinator.rs) — primary caller of `dispatch_to_provider_for` (via `call_with_retry_at_temp_for`).
Guia de contribuição
Avaliação
Esta issue ainda não foi avaliada.