refactor(llm): migrate probe subsystem (probe.rs + temperature_probe.rs ProviderProbeTransport)
- Lingua principale
- Rust
- Stelle
- 0
- Fork
- 1
- Merge medio
- 35m
- PR unite (30g)
- 238
Descrizione
## Goal
Migrate the **probe subsystem** — `src/llm/probe.rs` and `src/llm/temperature_probe.rs` — from `Arc` to `Arc`. Both files define a `ProviderProbeTransport` (or `ProviderTemperatureProbeTransport`) adapter that wraps a `Provider` for the auto-probe algorithm. Migrate the wrappers to take `Arc` instead, keeping the algorithm bodies unchanged.
## Why now
The probe subsystem runs **outside** the normal `dispatch_to_provider` path: it directly calls `provider.send_probe(...)` on the underlying client to bypass the safety wire-clamp. Until #5 + #6 land (which move `phase.rs` to `LlmClient`), `provider.rs` itself stays `Provider`-based, so the probe subsystem's adapter has nothing to wrap. But once the rest of the migration runs, the probe subsystem is the only `Provider::send_probe` consumer left outside `phase.rs` — making it the natural next migration target so the `ProbeTransport` trait can stay generic over a future `LlmClient::send_probe` instead of `Provider::send_probe`.
## Scope
**In scope:**
- `src/llm/probe.rs::ProviderProbeTransport` (struct at `:50-100`, impl at `:100-200`): wrap `Arc` instead of `Arc`.
- `src/llm/temperature_probe.rs::ProviderTemperatureProbeTransport` (struct at `:362-407`): same.
- The two `ProbeTransport` traits (one per file, used by the auto-probe algorithms) get a parallel set of methods that take `Arc` — or, more cleanly, the trait methods are generic and the algorithm body calls `client.send_probe(...)` directly.
- Tests at `src/llm/probe.rs::tests` and `src/llm/temperature_probe.rs::tests` migrate their `ScriptedProvider` test stubs to `ScriptedLlmClient`.
**Out of scope:**
- Migration of `phase.rs` — issues #5 + #6.
- Migration of CLI — issue #8.
- Migration of other phase files / discovery / integration tests — issues #9-#11.
- New `top_p_auto` / `top_k_auto` probes — issues #12 + #13.
- Cascade absorption (D9) / audit-hash kill (D8) — issue #14.
- Legacy `Provider` deletion — issue #15.
## Approach
### 1. `ProviderProbeTransport` → `LlmClientProbeTransport`
The struct fields change from:
```rust
pub struct ProviderProbeTransport<'a> {
pub provider: &'a Arc,
pub name: &'a str,
pub model: &'a str,
}
```
to:
```rust
pub struct LlmClientProbeTransport<'a> {
pub client: &'a Arc,
pub name: &'a str,
pub model: &'a str,
}
```
The `probe_send_with_body` method (the only one the algorithm calls) becomes:
```rust
async fn probe_send_with_body(&self, body: &serde_json::Value) -> Result<(u16, Response)> {
// Build a minimal LlmRequest from the probe body.
let req = LlmRequest::from_probe_body(body, self.name, self.model)?;
let resp = self.client.send_probe(&req).await?;
Ok((200, resp.into_legacy())) // 200 is a placeholder; the algorithm only inspects .text + .usage
}
```
Wait — the algorithm inspects `(status, response)` to detect probe outcomes. `LlmResponse` doesn't carry an HTTP status (the status is the outer `Result<(u16, Response)>` in the legacy API). Two options:
- **Option A (chosen):** keep the `(u16, Response)` return shape. `LlmResponse` gains an `http_status: u16` field for probe-internal use. Documented as probe-only; production `send` returns `LlmResponse` without it.
Actually that's adding complexity to `LlmResponse`. Simpler:
- **Option B (chosen):** `LlmClient::send_probe` returns `Result` (no status). The algorithm body is updated to inspect `LlmResponse.text` and `LlmResponse.finish_reason` for probe outcomes, not the HTTP status. The 4xx-vs-2xx distinction is irrelevant for the probe — only the response content matters (was the body accepted or rejected?).
This is a clean break with the legacy API and aligns with the new trait shape. The algorithm body's "status" check becomes a "did the call succeed?" check (`Result::is_ok()`).
### 2. `ProviderTemperatureProbeTransport` → `LlmClientTemperatureProbeTransport`
Same shape as #1. The temperature probe algorithm inspects `response.text` for rejection patterns like `"temperature"`, `"temperature_value"`, etc. — no HTTP status needed.
### 3. Tests migration
`src/llm/probe.rs::tests` has ~6 test cases using a `ScriptedProvider` stub. Each gets a parallel `ScriptedLlmClient` stub. Same for `temperature_probe.rs::tests` (~10 test cases).
The new test stubs live in `src/llm/client/test_stubs.rs` (new file, exported by `src/llm/client/mod.rs` under `#[cfg(test)]` so it doesn't bloat the release binary).
## Acceptance criteria
- [ ] `cargo build --release --all-features` succeeds with **zero warnings**.
- [ ] `make fmt-check guard-deps lint build test-ci` green.
- [ ] `src/llm/probe.rs` no longer imports `crate::llm::Provider`. All `provider.send_probe(...)` calls become `client.send_probe(...)`.
- [ ] `src/llm/temperature_probe.rs` no longer imports `crate::llm::Provider`.
- [ ] The probe algorithm tests (6 in `probe.rs`, 10 in `temperature_probe.rs`) pass against `ScriptedLlmClient`.
- [ ] Smoke: `moagan probe temperature --provider mock:mock-model` produces the same sidecar (`/temperatures_auto.toml`) as v0.17.6 for the same mock input.
- [ ] Smoke: `moagan probe max_tokens --provider mock:mock-model` produces the same sidecar (`/max_tokens_auto.toml`) as v0.17.6 for the same mock input.
- [ ] The CLI handler in `src/cli/probe.rs` is **untouched** in this issue — it still builds a `Provider` (via `build_provider_for_spec`); only the inner probe transport switches to `LlmClient`.
## Merge order
```
#1-#6 (foundation + phase.rs)
↓
#7 (this issue — probe subsystem migration)
↓
#8 (CLI migration)
↓
#9-#11 (other migrations)
```
Can be merged in parallel with #8 if both PRs agree on the test-stub API surface.
## Validation
- `make fmt-check guard-deps lint build test-ci` green after the PR lands.
- 16 test cases migrated (6 in probe.rs + 10 in temperature_probe.rs).
- Two smoke probes (`moagan probe temperature`, `moagan probe max_tokens`) produce v0.17.6-equivalent sidecars.
## Version target
v0.18.0.
## References
- [EPIC #847](https://github.com/airvzxf/moagan/issues/847) — the umbrella.
- [`src/llm/probe.rs:50-200`](../blob/main/src/llm/probe.rs) — `ProviderProbeTransport`.
- [`src/llm/temperature_probe.rs:362-407`](../blob/main/src/llm/temperature_probe.rs) — `ProviderTemperatureProbeTransport`.
- [`src/cli/probe.rs:692-759`](../blob/main/src/cli/probe.rs) — `build_provider_for_spec` (unchanged in this issue; migrated in #8).
Guida per i contributori
Apri la guida per i contributori
Valutazione
Questa issue non è ancora stata valutata.