airvzxf / airvzxf/moagan

refactor(llm): migrate CLI (cli/probe.rs + cli/run.rs minimax short-circuit)

Offen
#926 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
area:llm enhancement priority:P1 size:M
Vorherrschende Sprache
Rust
Sterne
0
Forks
1
Ø Merge
35 Min.
Gemergte PRs (30 T.)
238

Beschreibung

## Goal

Migrate the **CLI boundary** — `src/cli/probe.rs` and `src/cli/run.rs` — from `Arc` to `Arc`. Both files construct `Provider` instances for CLI-side use (the `moagan probe ...` subcommand and the `moagan run ...` startup path). Migrate the construction sites and the per-section wrapper logic to use `LlmClient` and the new dispatcher (issue #4).

## Why now

After #7 migrates the inner probe subsystem, the only remaining `Provider::send` consumer outside `phase.rs` is the CLI. Migrating the CLI closes the `Provider`-using code surface and unblocks issue #15 (deletion of `Provider`).

## Scope

**In scope:**
- `src/cli/probe.rs::build_provider_for_spec` (lines 692-759): replace the section-name + `WireFormatId` `match` with a single call to `client::dispatcher::build_client` (introduced in #4).
- `src/cli/probe.rs::build_provider_for_probe` (lines 692-744): same.
- `src/cli/probe.rs::tests` (the test at `:737` that asserts the section-name branch): migrate to use `build_client` and assert via `sdk_type()`.
- `src/cli/run.rs::build_registry_for_with_active` (lines around `:936`): the `minimax` short-circuit that hand-rolls a `MinimaxProvider` when `--api-key` is supplied. Replace with a call to `client::dispatcher::build_client` that picks the SDK from the URL.
- `src/cli/telemetry_cmd.rs` (the test at `:2566-2569` that filters by provider name): migrate to filter by `sdk_type()` instead of `name`.

**Out of scope:**
- Migration of other phase files / discovery / integration tests — issues #9-#11.
- New `top_p_auto` / `top_k_auto` CLI verbs — issue #13.
- Cascade absorption (D9) / audit-hash kill (D8) — issue #14.
- Legacy `Provider` deletion — issue #15.

## Approach

### 1. `build_provider_for_spec` → `build_client_for_spec`

The 30-line `match` at `src/cli/probe.rs:728-744` collapses to:

```rust
fn build_client_for_spec(spec: &ProviderConfig, model_id: &str, api_key: SecretString) -> Result> {
let endpoint = spec.endpoint.as_deref()
.or_else(|| /* fallback to model_id's endpoint */)
.ok_or_else(|| Error::InvalidArgs(format!("provider section '{}' has no endpoint", spec.name)))?;
client::dispatcher::build_client(endpoint, &spec.name, model_id, api_key, spec)
}
```

This deletes the section-name special cases (`minimax`, `deepseek`) — D2 says no `sdk` knob, and the URL alone decides. The two `minimax` / `deepseek` kind-cap field initialisations that the legacy code inlined (`MINIMAX_MAX_TOKENS_CAP`, `DEEPSEEK_MAX_TOKENS_CAP`) move into the `AnthropicClient::new` / `OpenAIClient::new_chat` constructors (issues #2 + #3).

### 2. `build_registry_for_with_active` → drop the minimax short-circuit

The `--api-key` flag's minimax short-circuit at `src/cli/run.rs:936`:

```rust
if section == "minimax" && api_key.is_some() {
let resolved = cfg.resolved_model(§ion, &model_id)?;
let provider = crate::llm::minimax::MinimaxProvider::from_resolved(&resolved)?;
...
}
```

becomes:

```rust
if let Some(api_key) = api_key {
// Build via the dispatcher regardless of section name.
let resolved = cfg.resolved_model(§ion, &model_id)?;
let endpoint = resolved.endpoint.as_deref()
.ok_or_else(|| Error::InvalidArgs(format!("provider section '{}' has no endpoint", section)))?;
let client = client::dispatcher::build_client(endpoint, §ion, &model_id, api_key, &spec)?;
// Wrap + insert into registry.
...
}
```

The section-name special case goes away — the URL decides, per D2.

### 3. `telemetry_cmd.rs` test migration

The test at `src/cli/telemetry_cmd.rs:2566-2569`:

```rust
let minimax = rows.iter().find(|r| r.provider == "minimax").unwrap();
let oc = rows.iter().find(|r| r.provider == "opencode").unwrap();
```

becomes:

```rust
let minimax = rows.iter().find(|r| r.sdk_type() == "anthropic").unwrap(); // matches the section's wire format
let oc = rows.iter().find(|r| r.sdk_type() == "openai_responses").unwrap();
```

Wait — that's wrong. The `provider_usage` table is keyed by the **section name** (`minimax`, `opencode`), not the wire format. The migration is:

```rust
let minimax = rows.iter().find(|r| r.name() == "minimax").unwrap();
let oc = rows.iter().find(|r| r.name() == "opencode").unwrap();
```

`name()` is on `LlmClient` too — same accessor, same return value. No semantic change.

### 4. Test migration

- `src/cli/probe.rs::tests::build_provider_for_spec_*` (3 cases): migrate to assert `build_client_for_spec` returns the right SDK (`assert!(client.sdk_type() == "anthropic")`).
- `src/cli/run.rs::tests::*` (4 cases that touch the minimax short-circuit): migrate to assert the dispatcher path picks `AnthropicClient` when the URL ends in `/v1/messages`.
- `src/cli/telemetry_cmd.rs::tests::*` (1 case): migrate to `name()`.

## Acceptance criteria

- [ ] `cargo build --release --all-features` succeeds with **zero warnings**.
- [ ] `make fmt-check guard-deps lint build test-ci` green.
- [ ] `src/cli/probe.rs::build_provider_for_spec` collapses to a single call to `client::dispatcher::build_client`. No `match section_name { "deepseek" | "minimax" | _ => match wire_format { ... } }` anywhere in `src/cli/`.
- [ ] `src/cli/run.rs::build_registry_for_with_active` no longer has a `minimax` short-circuit. The `--api-key` flag goes through the dispatcher.
- [ ] The 8 CLI test cases (3 + 4 + 1) pass.
- [ ] Smoke: `moagan run --mode fast --provider mock:mock-model --api-key dummy` produces the same artefacts as v0.17.6.
- [ ] Smoke: `moagan probe temperature --provider mock:mock-model` and `moagan probe max_tokens --provider mock:mock-model` continue to work (their sidecars are identical to v0.17.6).

## Merge order

```
#1-#7 (foundation + phase.rs + probe subsystem)

#8 (this issue — CLI migration)

#9-#11 (other migrations)
```

Can be merged in parallel with #7 if both PRs agree on the test-stub API surface. Practically: ship #7 first, then #8.

## Validation

- `make fmt-check guard-deps lint build test-ci` green after the PR lands.
- 8 CLI test cases pass.
- Three smoke probes run (`moagan run`, `moagan probe temperature`, `moagan probe max_tokens`) produce v0.17.6-equivalent artefacts.

## Version target

v0.18.0.

## References

- [EPIC #847](https://github.com/airvzxf/moagan/issues/847) — the umbrella.
- [#900 D2](https://github.com/airvzxf/moagan/issues/900) — dispatcher by URL path, no `sdk` knob.
- [`src/cli/probe.rs:692-759`](../blob/main/src/cli/probe.rs) — `build_provider_for_spec` / `build_provider_for_probe`.
- [`src/cli/run.rs:920-960`](../blob/main/src/cli/run.rs) — `build_registry_for_with_active` minimax short-circuit.
- [`src/cli/telemetry_cmd.rs:2566-2569`](../blob/main/src/cli/telemetry_cmd.rs) — provider-name filter test.
- Issue #4 — `client::dispatcher::build_client` factory.

Beitragsleitfaden

Beitragsleitfaden öffnen

Bewertung

Dieses Issue wurde noch nicht bewertet.

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.