airvzxf / airvzxf/moagan

refactor(llm): migrate phase.rs explicit-pair dispatch (dispatch_to_provider_for + cascade loops)

Aberta
#924 0 comentários 0 reações 0 responsáveis Ver no GitHub
area:llm enhancement priority:P1 size:L
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

Abrir o guia de contribuição

Avaliação

Esta issue ainda não foi avaliada.

Receba novas issues na sua caixa de entrada

Um resumo curto de issues do GitHub para quem está começando.