airvzxf / airvzxf/moagan

refactor(llm)!: delete legacy Provider trait + 5 impls + BreakeredProvider + ProviderRegistry + wire.rs + wire_format.rs (BREAKING v0.18.0)

Closed
#933 0 comments 0 reactions 0 assignees View on GitHub
area:llm enhancement priority:P1 size:M
Dominant language
Rust
Stars
0
Forks
1
Avg merge
29m
Merged PRs (30d)
246

Description

## Goal

Delete the legacy `Provider` trait + the 5 concrete `Provider` impls + the `ProviderRegistry` + the `BreakeredProvider` wrapper. After this issue lands, `src/llm/provider.rs` is gone; `src/llm/minimax.rs`, `src/llm/deepseek.rs`, `src/llm/anthropic_compat.rs`, `src/llm/openai_compat.rs`, `src/llm/openai_compatible.rs`, `src/llm/mock.rs` are gone. Only `src/llm/client/*` (introduced across #1-#14) remains.

## Why now

After #5-#14 finish migrating every `Provider::` consumer in `src/` to `LlmClient`, the legacy types have zero production consumers. The only remaining references are the test stubs (`ScriptedProvider`, etc.) and the `ProviderRegistry::from_config` chain — both migrated by #11. Deleting the legacy files is the **only** breaking change in the EPIC, and it's the change that justifies the v0.18.0 minor bump per #900 D3.

## Scope

**In scope (delete):**

- `src/llm/provider.rs` (4,449 LOC) — `Provider` trait, `ProviderRegistry`, `BreakeredProvider`, `ProviderPoolEntry`, `ProviderPool`, `registry_from_config` family.
- `src/llm/minimax.rs` (1,423 LOC).
- `src/llm/deepseek.rs` (340 LOC).
- `src/llm/anthropic_compat.rs` (1,119 LOC).
- `src/llm/openai_compat.rs` (1,951 LOC).
- `src/llm/openai_compatible.rs` (1,520 LOC).
- `src/llm/mock.rs` (589 LOC).
- `src/llm/wire.rs` (781 LOC) — `Request`/`Response`/`CallRecord`/`Usage` types, replaced by `LlmRequest`/`LlmResponse`.
- `src/llm/wire_format.rs` (894 LOC) — `WireFormat` trait + `WireFormatId` enum + `wire_format_from_url` (replaced by `client::dispatcher::pick_sdk` from #4).
- `src/llm/probe_table.rs::MaxTokensTable` (move into `src/llm/client/` or `src/llm/probe_max_tokens.rs`).
- `src/llm/temperature_probe.rs::TemperatureTable` (move into `src/llm/client/` or `src/llm/probe_temperature.rs`).
- `src/llm/param_rejections.rs::ParamRejectionsTable` is **kept** (used by the new `LlmClient::send` cascade).
- The `BreakeredClient` adapter (introduced in #5) becomes the primary circuit-breaker wrapper — rename to just `BreakeredClient` (already that name) and stop being an adapter.
- `ScriptedProvider` test stubs (kept under `#[cfg(test)]` annotations only if other test files still reference them — otherwise deleted).

**Out of scope (kept):**

- `src/llm/param_rejections.rs` — the cascade detection logic stays.
- `src/llm/capabilities.rs` — the capability matrix is reused by `LlmCapabilities`.
- `src/llm/http.rs` — the wire-body hash function stays (used by `LlmClient::body_sha256`).
- `src/llm/cache/`, `src/llm/embed/`, `src/llm/prompts/`, etc. — unrelated subsystems.
- `src/llm/probe_top_p.rs` and `src/llm/probe_top_k.rs` (introduced in #12) — these are new files; the deletion here is only of legacy code.

## Approach

### 1. Pre-flight: confirm zero `Provider::` consumers

```bash
rg 'use crate::llm::Provider|use moagan::llm::Provider|use crate::llm::provider::Provider' src/ tests/
```

Expected output: no matches. If any match remains, it's a missed migration — fix before this issue lands.

```bash
rg 'use crate::llm::provider::|use crate::llm::minimax|use crate::llm::deepseek|use crate::llm::anthropic_compat|use crate::llm::openai_compat|use crate::llm::openai_compatible|use crate::llm::mock' src/ tests/
```

Expected output: no matches.

### 2. `src/llm/mod.rs` rewrite

The new `src/llm/mod.rs` exports only the new modules:

```rust
//! LLM module: client trait + SDK impls, role enum, wire types, mock + minimax
//! implementations, cache, rate limiter, circuit breaker, and the
//! versioned prompt registry.

pub mod cache;
pub mod capabilities;
pub mod circuit_breaker;
pub mod client; // NEW (issues #1-#14)
pub mod control_tokens;
pub mod cost;
pub mod embed;
pub mod governor;
pub mod http;
pub mod json_extractor;
pub mod json_strategy;
pub mod max_tokens;
pub mod modal_gate;
pub mod models_dev;
pub mod param_rejections;
pub mod probe_max_tokens; // moved from probe_table.rs
pub mod probe_temperature; // moved from temperature_probe.rs
pub mod probe_top_p; // NEW (issue #12)
pub mod probe_top_k; // NEW (issue #12)
pub mod prompt_cache;
pub mod prompts;
pub mod rate_limiter;
pub mod response_format_opt_out;
pub mod retry_budget;
pub mod role;
pub mod size_limits;
pub mod sse_parser;

pub use client::{LlmClient, LlmRequest, LlmResponse, LlmCapabilities, LlmError, MockClient, AnthropicClient, OpenAIClient, OpenAIVariant};
pub use models_dev::{...}; // unchanged
pub use role::Role;
pub use probe_max_tokens::MaxTokensTable;
pub use probe_temperature::TemperatureTable;
pub use probe_top_p::TopPTable;
pub use probe_top_k::TopKTable;
```

The 7 legacy modules (`provider`, `minimax`, `deepseek`, `anthropic_compat`, `openai_compat`, `openai_compatible`, `mock`, `wire`, `wire_format`) are removed from the export list. Their files are deleted from disk.

### 3. `BreakeredClient` becomes the primary wrapper

`BreakeredClient` (introduced in #5 as an adapter over `BreakeredProvider`) becomes the only wrapper. It owns the breaker, rate limiter, max_tokens_table — same fields as today's `BreakeredProvider`, but wraps `Arc` instead of `Arc`.

The wrapper is constructed in `client::dispatcher::build_client` (from #4):

```rust
pub fn build_client(...) -> Result> {
let inner = match pick_sdk(endpoint)? {
SdkKind::Anthropic => Arc::new(AnthropicClient::new(cfg, api_key)?),
SdkKind::OpenAIChat => Arc::new(OpenAIClient::new_chat(cfg, api_key)?),
SdkKind::OpenAIResponses => Arc::new(OpenAIClient::new_responses(cfg, api_key)?),
SdkKind::Mock => Arc::new(MockClient::empty()),
};
let breaker = CircuitBreaker::new(breaker_cfg);
let wrapped = BreakeredClient::new(inner, breaker);
Ok(Arc::new(wrapped))
}
```

### 4. `LlmClientRegistry` becomes the primary registry

The new `LlmClientRegistry` (introduced in #11) becomes the only registry. The legacy `ProviderRegistry` is deleted.

### 5. ScriptedProvider stubs deletion

After the migration wave + tests migration (#11), `ScriptedProvider` should have zero remaining consumers. Verify:

```bash
rg 'ScriptedProvider' src/ tests/
```

Expected: no matches. If matches remain, they're test stubs that need migration or deletion.

## Acceptance criteria

- [ ] `cargo build --release --all-features` succeeds with **zero warnings** (the legacy code is gone, no dead references).
- [ ] `make fmt-check guard-deps lint build test-ci` green.
- [ ] `src/llm/provider.rs` deleted from disk.
- [ ] `src/llm/minimax.rs` deleted from disk.
- [ ] `src/llm/deepseek.rs` deleted from disk.
- [ ] `src/llm/anthropic_compat.rs` deleted from disk.
- [ ] `src/llm/openai_compat.rs` deleted from disk.
- [ ] `src/llm/openai_compatible.rs` deleted from disk.
- [ ] `src/llm/mock.rs` deleted from disk.
- [ ] `src/llm/wire.rs` deleted from disk.
- [ ] `src/llm/wire_format.rs` deleted from disk.
- [ ] `rg 'use crate::llm::Provider|use crate::llm::provider' src/ tests/` returns no matches.
- [ ] `rg 'use crate::llm::minimax|use crate::llm::deepseek|use crate::llm::anthropic_compat|use crate::llm::openai_compat|use crate::llm::openai_compatible|use crate::llm::mock' src/ tests/` returns no matches.
- [ ] `src/llm/client/` is the only LLM-traits module.
- [ ] `BreakeredClient` is the only circuit-breaker wrapper.
- [ ] `LlmClientRegistry` is the only registry.
- [ ] Smoke: `moagan run --mode fast --provider mock:mock-model` produces `final/portfolio.md` and `rankings/ranking.json`.
- [ ] Smoke: `moagan run --mode deep --provider mock:mock-model` produces all the deep-mode artefacts.
- [ ] Smoke: `moagan probe temperature --provider mock:mock-model` produces `/temperatures_auto.toml`.
- [ ] Smoke: `moagan probe max_tokens --provider mock:mock-model` produces `/max_tokens_auto.toml`.
- [ ] CHANGELOG entry: **BREAKING**: removed `Provider` trait and 5 concrete impls (`MinimaxProvider`, `DeepSeekProvider`, `AnthropicCompatProvider`, `OpenAICompatibleProvider`, `OpenAICompatProvider`). Operators with custom `Provider` impls (downstream of `crate::llm::Provider`) need to migrate to `LlmClient`. The 3 SDK impls (`AnthropicClient`, `OpenAIClient`, `MockClient`) cover all current use cases.

## Merge order

```
#1-#14 (foundation + migration + features + cleanup)

#15 (this issue — legacy deletion)

#16 (docs + close EPIC)
```

**Breaking change** — this issue is the only one in the EPIC that breaks the public API. Per #900 D3, no soft-landing: operators with custom `Provider` impls must migrate in lockstep with this release.

## Validation

- `make fmt-check guard-deps lint build test-ci` green after the PR lands.
- 11 files deleted from disk.
- 4 smoke runs (fast mode, deep mode, probe temperature, probe max_tokens) all succeed.
- CHANGELOG entry marked **BREAKING**.

## Version target

**v0.18.0.** This is the breaking-change release.

## References

- [EPIC #847](https://github.com/airvzxf/moagan/issues/847) — the umbrella.
- [#900 D3, D8](https://github.com/airvzxf/moagan/issues/900) — D3 (no soft-landing, breaking changes allowed), D8 (no compat layer, `BreakeredProvider` deleted).
- [ADR-0010](../blob/main/docs/adr/0010-llm-client-trait-impl-plan.md) — the deferred plan this issue finalises.

Contributor guide

Open the contributing guide

Research direction

Start with the pre-flight rg checks in src/ and tests/, then inspect src/llm/mod.rs and client::dispatcher::build_client to confirm the migration prerequisites and current exports. Remove only the listed legacy modules and move the probe tables as specified, then run make fmt-check guard-deps lint build test-ci plus the four documented mock-provider smoke commands; done means the files and legacy references are gone and all acceptance checks pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
ai
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.