worker-generation-settings完了
This commit is contained in:
parent
8114d3c4fd
commit
2e38a24ac2
1
TODO.md
1
TODO.md
|
|
@ -18,4 +18,3 @@
|
|||
- [ ] Phase 2 consolidation → [tickets/memory-phase2-consolidation.md](tickets/memory-phase2-consolidation.md)
|
||||
- [ ] 使用頻度メトリクス + Knowledge 化候補レポート → [tickets/memory-usage-metrics.md](tickets/memory-usage-metrics.md)
|
||||
- [ ] GC（定期再評価） → [tickets/memory-gc.md](tickets/memory-gc.md)
|
||||
- [ ] LLM 生成設定の manifest 露出整理 → [tickets/worker-generation-settings.md](tickets/worker-generation-settings.md)
|
||||
|
|
|
|||
|
|
@ -1,56 +0,0 @@
|
|||
# LLM 生成設定の manifest 露出整理
|
||||
|
||||
## 背景
|
||||
|
||||
`llm-worker::RequestConfig` には LLM 生成に関する共通設定として `max_tokens` / `temperature` / `top_p` / `top_k` / `stop_sequences` / `reasoning` がある。一方、Pod manifest の `[worker]` まで露出しているのは現状 `max_tokens` と `temperature` のみで、`top_p` / `top_k` / `stop_sequences` は manifest から指定できない。
|
||||
|
||||
`reasoning` は `tickets/model-reasoning-control.md` で内部抽象と manifest 経路を整理するため、本チケットではそれ以外の既存 `RequestConfig` 項目を manifest から指定できるようにする。
|
||||
|
||||
## 要件
|
||||
|
||||
### `[worker]` への生成設定追加
|
||||
|
||||
Pod manifest の `[worker]` セクションで、既存 `RequestConfig` に存在する以下の項目を指定できるようにする。
|
||||
|
||||
```toml
|
||||
[worker]
|
||||
top_p = 0.9
|
||||
top_k = 40
|
||||
stop_sequences = ["\n\n", "</stop>"]
|
||||
```
|
||||
|
||||
- `top_p`: `Option<f32>` として扱い、指定時のみ request に渡す
|
||||
- `top_k`: `Option<u32>` として扱い、指定時のみ request に渡す
|
||||
- `stop_sequences`: `Vec<String>` として扱い、未指定時は空配列と同等にする
|
||||
|
||||
Provider / scheme によって効かない値がある場合でも、既存の scheme 投影に任せる。値の厳密な provider 別検証は本チケットでは行わない。
|
||||
|
||||
### manifest cascade / restore 経路
|
||||
|
||||
`WorkerManifestConfig` / `WorkerManifest` に上記フィールドを追加し、manifest cascade の merge 後に `pod::apply_worker_manifest()` から `RequestConfig` へ渡す。
|
||||
|
||||
`stop_sequences` は cascade 時に上位 manifest が指定した場合の意味を明確にする。初期方針は、`max_tokens` や `temperature` と同様に「上位指定があれば置き換え」とし、配列の追記マージは行わない。
|
||||
|
||||
### 既存挙動の保持
|
||||
|
||||
未指定時は現在と同じ request body になること。特に、既存 manifest で `top_p` / `top_k` / `stop_sequences` を省略している場合、wire request に新しい値が出ない。
|
||||
|
||||
## 範囲外
|
||||
|
||||
- `reasoning` / thinking 制御の manifest 露出。これは `tickets/model-reasoning-control.md` で扱う
|
||||
- `presence_penalty` / `frequency_penalty` / `seed` / `response_format` / `tool_choice` / `parallel_tool_calls` 等、まだ `RequestConfig` に存在しない新規生成パラメータの追加
|
||||
- provider ごとの値域検証や推奨値テーブル
|
||||
- UI での設定編集画面
|
||||
|
||||
## 完了条件
|
||||
|
||||
- `[worker].top_p` / `[worker].top_k` / `[worker].stop_sequences` を manifest TOML で指定できる
|
||||
- cascade merge 後の `WorkerManifest` がこれらの値を保持する
|
||||
- `pod::apply_worker_manifest()` が `RequestConfig.top_p` / `top_k` / `stop_sequences` へ値を渡す
|
||||
- 未指定時の既存挙動が変わらない
|
||||
- manifest parse / merge / apply のテストが追加または更新されている
|
||||
|
||||
## Review
|
||||
- 状態: Approve
|
||||
- レビュー詳細: [./worker-generation-settings.review.md](./worker-generation-settings.review.md)
|
||||
- 日付: 2026-04-28
|
||||
|
|
@ -1,47 +0,0 @@
|
|||
# Review: LLM 生成設定の manifest 露出整理
|
||||
|
||||
## 前提・要件の確認
|
||||
|
||||
- `[worker]` への `top_p` / `top_k` / `stop_sequences` 追加: 満たされている。
|
||||
- `WorkerManifestConfig` に `top_p: Option<f32>` / `top_k: Option<u32>` / `stop_sequences: Option<Vec<String>>` を追加 (`crates/manifest/src/config.rs:65-73`)。
|
||||
- `WorkerManifest` 側にも `top_p` / `top_k` / `stop_sequences: Vec<String>`（解決後は確定値、未指定は空 Vec）として露出 (`crates/manifest/src/lib.rs:104-109`)。
|
||||
- `TryFrom<PodManifestConfig>` で `cfg.worker.stop_sequences.unwrap_or_default()` により未指定時は空配列に正規化 (`crates/manifest/src/config.rs:351-353`)。
|
||||
- cascade merge の置換意味論: 満たされている。`upper.top_p.or(self.top_p)` 等のスカラーと、`upper.stop_sequences.or(self.stop_sequences)` の配列もまるごと置換になっている (`crates/manifest/src/config.rs:235-237`)。要件「`stop_sequences` は配列の追記マージをしない」と一致。
|
||||
- `apply_worker_manifest()` から `RequestConfig` への反映: 満たされている。`request_config_from_worker_manifest` ヘルパーに切り出し、`top_p` / `top_k` を `Some` のときだけ代入し、`stop_sequences` はクローンして代入 (`crates/pod/src/pod.rs:1395-1411`)。
|
||||
- 未指定時の wire 互換: 満たされている。
|
||||
- `RequestConfig.stop_sequences: Vec<String>` (`crates/llm-worker/src/llm_client/types.rs:596`) は各 scheme 側で `#[serde(skip_serializing_if = "Vec::is_empty")]` 付きフィールドへ複製される（anthropic `request.rs:33-34`、gemini `request.rs:141-142`、openai_chat も同様）。空 Vec のとき body から欠落するため、新フィールド未指定時は今までと同じ wire になる。
|
||||
- `top_p` / `top_k` も `Option` のまま `skip_serializing_if = "Option::is_none"` 経路で従来通り省略される。
|
||||
- parse / merge / apply の三段テスト: 揃っている。
|
||||
- parse: `crates/manifest/src/config.rs:773` `from_toml_accepts_worker_generation_settings` と `crates/manifest/src/lib.rs:316-358` の TOML 統合テスト。
|
||||
- merge: `crates/manifest/src/config.rs:601` `merge_worker_generation_settings_upper_wins`。`top_k` は upper=None で lower=Some(20) が残ること、`stop_sequences` は upper 配列が下位を完全に置換することを検証。
|
||||
- apply: `crates/pod/src/pod.rs:1630` `worker_manifest_generation_settings_become_request_config`。
|
||||
- 既存テストの未指定アサーション: `crates/manifest/src/lib.rs:308-310` で `top_p.is_none() / top_k.is_none() / stop_sequences.is_empty()` を確認。defaults 経路の不変も明示されている。
|
||||
|
||||
## アーキテクチャ・スコープ
|
||||
|
||||
- 既存の cascade パターン（`Option<T>::or`）にそのまま乗っており、`stop_sequences` を `Option<Vec<String>>` で持って `unwrap_or_default()` で `Vec` に展開する実装は、他の `worker.*` フィールドと意味論を揃えるための妥当な選択。`Vec` 直持ちにすると「未指定」と「明示的な空配列」が区別できなくなり cascade で下位を上書きする意味が出せなくなるため、この選択は適切。
|
||||
- `apply_worker_manifest` から `request_config_from_worker_manifest` を抽出した小さなリファクタは、`RequestConfig` ビルドが純粋関数になりテスト容易性が上がる範囲に収まっている。過剰な抽象化はなく YAGNI 違反なし。
|
||||
- `llm-worker` の `RequestConfig` 自体には変更なし、既に存在するフィールドへ繋ぐだけで manifest 側を上の層から制御している。`feedback_llm_worker_scope` の方針（高レベル機能は上の層）に沿う。
|
||||
- 範囲外項目（`reasoning` の manifest 露出、`presence_penalty` 等の新規パラメータ、provider 別値域検証）には踏み込んでいない。
|
||||
- `cargo add` 経路の懸念なし（依存追加なし）。
|
||||
- README (`crates/manifest/README.md`) のサマリも「生成設定、reasoning」と整合の取れた表現に更新されている。
|
||||
|
||||
## 指摘事項
|
||||
|
||||
### Blocking
|
||||
|
||||
- なし。
|
||||
|
||||
### Non-blocking / Follow-up
|
||||
|
||||
- docs の範囲外編集が混在している。`docs/architecture.md` の `pwd` 行削除、`docs/pod-factory.md` の overlay 例から `pod.pwd` を外す変更、`compaction.provider` → `compaction.model` への置換、旧 `compact_retained_turns` → `compact_request_threshold` / `compact_retained_tokens` / `compact_auto_read_budget` / `compact_worker_max_input_tokens` への差し替え、パス解決例の更新（`provider.api_key_file` → `model.auth.file`）は、既に main 側で実装が進んでいた仕様に docs を追従させる修正であって、本チケットのスコープではない。動作上は健全だが、別チケットとして括り出すか commit 粒度を分けると履歴の追跡性が上がる。今回はチケット完了の判断を妨げない。
|
||||
- `docs/pod-factory.md` の新設「`[worker]` 設定」テーブルにある `top_k` 説明「未対応 scheme では warning または provider 側挙動に任せる」は、本チケットの範囲外の値域検証文言に踏み込み気味。実際 `validate_config` 経路で warning を出すかは scheme 実装に依存するため、軽い表現緩和（例: 「scheme が未対応の場合は scheme 側の投影に従う」）の検討余地あり。
|
||||
|
||||
### Nits
|
||||
|
||||
- `request_config_from_worker_manifest` 内の `if let Some(top_p) = wm.top_p { config.top_p = Some(top_p); }` は `config.top_p = wm.top_p;` でも等価。`max_tokens` / `temperature` の既存記法に合わせている意図と思われるので変更は必須でないが、`config.top_p = wm.top_p; config.top_k = wm.top_k;` のほうが意図（「未指定なら未指定のまま伝搬」）が読みやすい。
|
||||
- `merge_worker_generation_settings_upper_wins` テストで `top_k` を upper 未指定にして lower の `Some(20)` が残ることを確認しているのは良い設計。ここに `stop_sequences` の「upper 未指定なら lower がそのまま残る」ケースが加わるとカバレッジが完全になる（現在は upper が指定されたケースのみ）。
|
||||
|
||||
## 判断
|
||||
|
||||
Approve — 要件・完了条件をいずれも満たし、cascade の置換意味論・未指定時の wire 互換・三段テストの整備すべて確認済み。docs の範囲外編集は履歴粒度の指摘に留まり、コードベースを歪める変更はない。
|
||||
Loading…
Reference in New Issue
Block a user