airvzxf / airvzxf/moagan

refactor(llm): migrate probe subsystem (probe.rs + temperature_probe.rs ProviderProbeTransport)

Cerrado
#925 0 comentarios 0 reacciones 0 asignados Ver en GitHub
area:llm enhancement priority:P1 size:M
Lenguaje dominante
Rust
Estrellas
0
Forks
1
Merge medio
29 min
PR fusionados (30 d)
246

Descripción

## 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).

Guía de contribución

Abrir la guía de contribución

Línea de trabajo

Start by reading src/llm/probe.rs and src/llm/temperature_probe.rs, then inspect the LlmClient and send_probe APIs from the prerequisite migration issues. Run the existing probe tests before changing the ScriptedProvider stubs. Done means both transports and their 16 tests use LlmClient, the listed validation commands pass, and both smoke probes preserve their v0.17.6-equivalent sidecars.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
rust
Área
ai, backend
Tipo de issue
Refactorización
Dificultad
5/5
Tiempo estimado
Más de una semana
Estado de actividad
Activo
Claridad
Bastante claro
Aptitud para principiantes
35/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.