airvzxf / airvzxf/moagan

feat(llm): add CLI subcommands moagan probe top_p + moagan probe top_k (D7)

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

Description

## Goal

Add the CLI verbs `moagan probe top_p` and `moagan probe top_k` per #900 D7. Mirror the existing `moagan probe temperature` and `moagan probe max_tokens` verbs. Both verbs run the auto-probe, persist the result to `/top_p_auto.toml` and `/top_k_auto.toml`, and accept `--persist-min` / `--persist-union` flags (D10: fixed safe set — these are now permanent code, not user-configurable).

## Why now

#12 lands the underlying `TopPTable` / `TopKTable` infrastructure. This issue exposes them via the CLI, completing the two-plane naming contract from #900 D7 (wire field `top_p` / `top_k`; CLI verb `moagan probe top_p` / `moagan probe top_k`; sidecar file `top_p_auto.toml` / `top_k_auto.toml`).

## Scope

**In scope:**
- `src/cli/probe.rs` (already has `ProbeCmd` enum at `:65-84`): add two new variants:
- `ProbeCmd::TopP(ProbeTopPCmd)`
- `ProbeCmd::TopK(ProbeTopKCmd)`
- Each variant mirrors the existing `ProbeMaxTokensCmd` / `ProbeTemperatureCmd` shape (`:87-157`):
- `--provider PROVIDER:MODEL` (required, repeatable).
- `--persist-min` (for `top_p` / `top_k` — pins the operator cap to the minimum accepted value).
- `--batch-size N` (for `top_p` — defaults to the per-verb batch size constant).
- `--dry-run` (don't persist to disk).
- `dispatch(cmd: &ProbeCmd)` at `src/cli/probe.rs:160-166`: add the two new arms.
- `dispatch_top_p` / `dispatch_top_k` functions (mirror `dispatch_temperature` at `:353-502`):
- Build the underlying `LlmClient` via `client::dispatcher::build_client` (issue #4).
- Wrap in `LlmClientProbeTransport` (introduced in #7).
- Run the probe via `TopPTable::probe_and_store(...)` / `TopKTable::probe_and_store(...)`.
- On `--persist-min`, call `TopPTable::set_operator_cap(...)` / `TopKTable::set_operator_cap(...)`.
- Unit tests at `src/cli/probe.rs::tests` that pin the new CLI verbs.
- Update `src/cli/probe.rs::tests::build_provider_for_spec_*` to assert the two new verbs route through the dispatcher.

**Out of scope:**
- The underlying `TopPTable` / `TopKTable` infrastructure — issue #12.
- Cascade absorption (D9) / audit-hash kill (D8) — issue #14.
- Legacy `Provider` deletion — issue #15.

## Approach

### 1. CLI verb shape

```rust
#[derive(Debug, Clone, clap::Args)]
pub struct ProbeTopPCmd {
#[arg(long = "provider", value_name = "PROVIDER:MODEL", required = true, num_args = 1..)]
pub providers: Vec,
#[arg(long, default_value_t = false)]
pub persist_min: bool,
#[arg(long, default_value_t = TOP_P_PROBE_BATCH_SIZE)]
pub batch_size: usize,
#[arg(long, default_value_t = false)]
pub dry_run: bool,
}

#[derive(Debug, Clone, clap::Args)]
pub struct ProbeTopKCmd {
#[arg(long = "provider", value_name = "PROVIDER:MODEL", required = true, num_args = 1..)]
pub providers: Vec,
#[arg(long, default_value_t = false)]
pub persist_min: bool,
#[arg(long, default_value_t = false)]
pub dry_run: bool,
}
```

(`top_k` doesn't take `--batch-size` because the algorithm walks powers of 2 — the batch is fixed.)

### 2. `dispatch_top_p` function

Mirror `dispatch_temperature` at `src/cli/probe.rs:353-502`:

```rust
pub async fn dispatch_top_p(cmd: &ProbeTopPCmd) -> Result {
// 1. Resolve each `provider:model` pair via `cfg.providers_by_section`.
// 2. Build an LlmClient via `client::dispatcher::build_client` (issue #4).
// 3. Wrap in `LlmClientProbeTransport` (issue #7).
// 4. Load `TopPTable::from_home(&home, floor, save)`.
// 5. Run `TopPTable::probe_and_store(transport, ...)` with cmd.batch_size.
// 6. On `--persist-min`, call `TopPTable::set_operator_cap(...)`.
// 7. Print the discovered top_p set + operator cap to stdout.
// 8. Exit 0 on success, 1 on probe failure.
}
```

### 3. Unit tests

- `dispatch_top_p_accepts_provider` — wiremock returns `200` for `top_p=0.5`, `400` for `top_p=0.55`. Probe returns `0.5`.
- `dispatch_top_p_persist_min_writes_cap` — `--persist-min` → sidecar has `operator_caps..auto = false, .top_p = 0.5`.
- `dispatch_top_p_dry_run_does_not_persist` — `--dry-run` → sidecar unchanged.
- `dispatch_top_k_walks_powers_of_2` — wiremock accepts `1, 2, 4, 8, 16`, rejects `32`. Probe returns `16`.
- `dispatch_top_k_persist_min_writes_cap` — same as top_p.

## Acceptance criteria

- [ ] `cargo build --release --all-features` succeeds with **zero warnings**.
- [ ] `make fmt-check guard-deps lint build test-ci` green.
- [ ] `moagan probe top_p --help` and `moagan probe top_k --help` print the new args.
- [ ] `moagan probe top_p --provider mock:mock-model --dry-run` runs without error.
- [ ] `moagan probe top_k --provider mock:mock-model --dry-run` runs without error.
- [ ] `moagan probe top_p --provider mock:mock-model --persist-min` writes `/top_p_auto.toml` with `operator_caps.mock.mock-model.auto = false`.
- [ ] `moagan probe top_k --provider mock:mock-model --persist-min` writes `/top_k_auto.toml` similarly.
- [ ] New unit tests at `src/cli/probe.rs::tests` cover the 5 listed cases and pass.
- [ ] `cli-reference.md` documents the two new verbs.

## Merge order

```
#1-#11 (foundation + migration wave)
#12 (top_p_auto + top_k_auto infrastructure)

#13 (this issue — CLI verbs)

#14-#15 (cleanup wave)
```

## Validation

- `make fmt-check guard-deps lint build test-ci` green after the PR lands.
- 5 new unit tests pass.
- Manual: `moagan probe top_p --provider mock:mock-model --dry-run` and `moagan probe top_k --provider mock:mock-model --dry-run` run successfully.

## Version target

v0.18.0 (or v0.18.x patch — additive feature, no breaking changes). Per the operator's preference in the EPIC body comment, batching into v0.18.0 is the natural target.

## References

- [EPIC #847](https://github.com/airvzxf/moagan/issues/847) — the umbrella.
- [#900 D7, D10](https://github.com/airvzxf/moagan/issues/900) — `_auto` naming (D7), fixed safe set (D10).
- [`src/cli/probe.rs:65-166`](../blob/main/src/cli/probe.rs) — existing `ProbeCmd` enum and `dispatch` function.
- [`src/cli/probe.rs:353-502`](../blob/main/src/cli/probe.rs) — `dispatch_temperature` (mirrored by `dispatch_top_p`).
- [`src/cli/probe.rs:168-320`](../blob/main/src/cli/probe.rs) — `dispatch_max_tokens` (mirrored by `dispatch_top_k`).
- [Issue #4](../issues/921) (when created) — `client::dispatcher::build_client`.
- [Issue #7](../issues/924) (when created) — `LlmClientProbeTransport`.
- [Issue #12](../issues/929) (when created) — `TopPTable` / `TopKTable`.

Contributor guide

Open the contributing guide

Research direction

Start in src/cli/probe.rs, reading ProbeCmd, dispatch, dispatch_max_tokens, and dispatch_temperature. Then inspect the referenced TopPTable and TopKTable infrastructure and run the existing probe tests. Done means both verbs expose the specified flags, route through the dispatcher, persist or skip sidecars correctly, pass the five listed unit tests, and are documented in cli-reference.md.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
cli
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.