From 8114d3c4fdaf48acea17f1908aba74a1b32a7a67 Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 28 Apr 2026 09:37:22 +0900 Subject: [PATCH] =?UTF-8?q?=E7=94=9F=E6=88=90=E8=A8=AD=E5=AE=9A=E3=81=AEma?= =?UTF-8?q?nifest=E5=8C=96=E3=81=AE=E5=AE=9F=E8=A3=85?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/manifest/README.md | 2 +- crates/manifest/src/config.rs | 59 ++++++++++++++++++++ crates/manifest/src/lib.rs | 15 +++++ crates/pod/src/pod.rs | 46 +++++++++++++-- docs/architecture.md | 2 +- docs/pod-factory.md | 46 ++++++++++++--- tickets/worker-generation-settings.md | 5 ++ tickets/worker-generation-settings.review.md | 47 ++++++++++++++++ 8 files changed, 206 insertions(+), 16 deletions(-) create mode 100644 tickets/worker-generation-settings.review.md diff --git a/crates/manifest/README.md b/crates/manifest/README.md index 53a4bacb..21d612da 100644 --- a/crates/manifest/README.md +++ b/crates/manifest/README.md @@ -9,6 +9,6 @@ Pod の宣言的設定を TOML マニフェストとして定義・パースす - `ModelConfig` — LLM モデル設定（scheme、base_url、model_id、auth） - `SchemeKind` — wire scheme 種別（`Anthropic`, `OpenaiChat`, `OpenaiResponses`, `Gemini`） - `AuthRef` — 認証参照（`None`, `ApiKey { env, file }`, `CodexOAuth`） -- `WorkerManifest` — ワーカー設定（システムプロンプト、max_tokens、temperature） +- `WorkerManifest` — ワーカー設定（システムプロンプト、生成設定、reasoning） - `ScopeConfig` / `ScopeRule` / `Permission` — allow / deny の宣言的スコープ設定 - `Scope` — 実行時スコープ。`from_config(&ScopeConfig, pwd)` で構築し、`is_readable` / `is_writable` / `permission_at` で問い合わせる diff --git a/crates/manifest/src/config.rs b/crates/manifest/src/config.rs index f47c2935..d4656460 100644 --- a/crates/manifest/src/config.rs +++ b/crates/manifest/src/config.rs @@ -65,6 +65,12 @@ pub struct WorkerManifestConfig { #[serde(default)] pub temperature: Option, #[serde(default)] + pub top_p: Option, + #[serde(default)] + pub top_k: Option, + #[serde(default)] + pub stop_sequences: Option>, + #[serde(default)] pub reasoning: Option, #[serde(default)] pub tool_output: ToolOutputLimitsPartial, @@ -226,6 +232,9 @@ impl WorkerManifestConfig { max_tokens: upper.max_tokens.or(self.max_tokens), max_turns: upper.max_turns.or(self.max_turns), temperature: upper.temperature.or(self.temperature), + top_p: upper.top_p.or(self.top_p), + top_k: upper.top_k.or(self.top_k), + stop_sequences: upper.stop_sequences.or(self.stop_sequences), reasoning: upper.reasoning.or(self.reasoning), tool_output: self.tool_output.merge(upper.tool_output), } @@ -339,6 +348,9 @@ impl TryFrom for PodManifest { max_tokens: cfg.worker.max_tokens, max_turns: cfg.worker.max_turns, temperature: cfg.worker.temperature, + top_p: cfg.worker.top_p, + top_k: cfg.worker.top_k, + stop_sequences: cfg.worker.stop_sequences.unwrap_or_default(), reasoning: cfg.worker.reasoning, tool_output: ToolOutputLimits { default_max_bytes: cfg @@ -586,6 +598,33 @@ mod tests { ); } + #[test] + fn merge_worker_generation_settings_upper_wins() { + let lower = PodManifestConfig { + worker: WorkerManifestConfig { + top_p: Some(0.8), + top_k: Some(20), + stop_sequences: Some(vec!["lower".into()]), + ..Default::default() + }, + ..Default::default() + }; + let upper = PodManifestConfig { + worker: WorkerManifestConfig { + top_p: Some(0.9), + stop_sequences: Some(vec!["upper".into()]), + ..Default::default() + }, + ..Default::default() + }; + + let merged = lower.merge(upper); + + assert_eq!(merged.worker.top_p, Some(0.9)); + assert_eq!(merged.worker.top_k, Some(20)); + assert_eq!(merged.worker.stop_sequences, Some(vec!["upper".into()])); + } + #[test] fn merge_scope_accumulates_allow_and_deny() { let lower = PodManifestConfig { @@ -732,6 +771,26 @@ reasoning = -1 ); } + #[test] + fn from_toml_accepts_worker_generation_settings() { + let cfg = PodManifestConfig::from_toml( + r#" +[worker] +top_p = 0.9 +top_k = 40 +stop_sequences = ["\n\n", ""] +"#, + ) + .unwrap(); + + assert_eq!(cfg.worker.top_p, Some(0.9)); + assert_eq!(cfg.worker.top_k, Some(40)); + assert_eq!( + cfg.worker.stop_sequences, + Some(vec!["\n\n".into(), "".into()]) + ); + } + #[test] fn from_toml_partial_layer_succeeds() { // A project-layer manifest with only scope set must parse fine. diff --git a/crates/manifest/src/lib.rs b/crates/manifest/src/lib.rs index 4a668ddd..8d949a8e 100644 --- a/crates/manifest/src/lib.rs +++ b/crates/manifest/src/lib.rs @@ -102,6 +102,12 @@ pub struct WorkerManifest { #[serde(default)] pub temperature: Option, #[serde(default)] + pub top_p: Option, + #[serde(default)] + pub top_k: Option, + #[serde(default)] + pub stop_sequences: Vec, + #[serde(default)] pub reasoning: Option, /// Byte-size caps applied to tool `content` before it reaches the /// conversation history. The section is optional in TOML — when @@ -299,6 +305,9 @@ permission = "write" assert_eq!(manifest.scope.allow.len(), 1); assert!(manifest.scope.deny.is_empty()); assert_eq!(manifest.worker.instruction, defaults::DEFAULT_INSTRUCTION); + assert!(manifest.worker.top_p.is_none()); + assert!(manifest.worker.top_k.is_none()); + assert!(manifest.worker.stop_sequences.is_empty()); } #[test] @@ -316,6 +325,9 @@ auth = { kind = "api_key", file = "/abs/keys/anthropic" } instruction = "$user/reviewer" max_tokens = 4096 temperature = 0.3 +top_p = 0.9 +top_k = 40 +stop_sequences = ["\n\n", ""] reasoning = "medium" [[scope.allow]] @@ -341,6 +353,9 @@ permission = "write" assert_eq!(manifest.worker.instruction, "$user/reviewer"); assert_eq!(manifest.worker.max_tokens, Some(4096)); assert_eq!(manifest.worker.temperature, Some(0.3)); + assert_eq!(manifest.worker.top_p, Some(0.9)); + assert_eq!(manifest.worker.top_k, Some(40)); + assert_eq!(manifest.worker.stop_sequences, vec!["\n\n", ""]); assert_eq!( manifest.worker.reasoning, Some(ReasoningControl::Effort(ReasoningEffort::Medium)) diff --git a/crates/pod/src/pod.rs b/crates/pod/src/pod.rs index d3cb9072..696a2153 100644 --- a/crates/pod/src/pod.rs +++ b/crates/pod/src/pod.rs @@ -1384,6 +1384,15 @@ impl Pod, St> { /// minijinja template that is parsed by `Pod::from_manifest` and /// rendered once at first turn in `ensure_system_prompt_materialized`. pub fn apply_worker_manifest(worker: &mut Worker, wm: &WorkerManifest) { + worker.set_request_config(request_config_from_worker_manifest(wm)); + worker.set_max_turns(wm.max_turns.map(|n| n.get())); + worker.set_tool_output_limits(Some(ToolOutputLimits { + default_max_bytes: wm.tool_output.default_max_bytes, + per_tool: wm.tool_output.per_tool.clone(), + })); +} + +fn request_config_from_worker_manifest(wm: &WorkerManifest) -> RequestConfig { let mut config = RequestConfig::new(); if let Some(max_tokens) = wm.max_tokens { config.max_tokens = Some(max_tokens); @@ -1391,13 +1400,15 @@ pub fn apply_worker_manifest(worker: &mut Worker, wm: &WorkerMa if let Some(temperature) = wm.temperature { config.temperature = Some(temperature); } + if let Some(top_p) = wm.top_p { + config.top_p = Some(top_p); + } + if let Some(top_k) = wm.top_k { + config.top_k = Some(top_k); + } + config.stop_sequences = wm.stop_sequences.clone(); config.reasoning = wm.reasoning.clone(); - worker.set_request_config(config); - worker.set_max_turns(wm.max_turns.map(|n| n.get())); - worker.set_tool_output_limits(Some(ToolOutputLimits { - default_max_bytes: wm.tool_output.default_max_bytes, - per_tool: wm.tool_output.per_tool.clone(), - })); + config } /// Result of a Pod run. @@ -1616,6 +1627,29 @@ mod build_summary_prompt_tests { assert!(!prompt.contains("deliberation")); } + #[test] + fn worker_manifest_generation_settings_become_request_config() { + let manifest = WorkerManifest { + instruction: "unused".into(), + max_tokens: Some(1024), + max_turns: None, + temperature: Some(0.2), + top_p: Some(0.9), + top_k: Some(40), + stop_sequences: vec!["\n\n".into(), "".into()], + reasoning: None, + tool_output: manifest::ToolOutputLimits::default(), + }; + + let config = request_config_from_worker_manifest(&manifest); + + assert_eq!(config.max_tokens, Some(1024)); + assert_eq!(config.temperature, Some(0.2)); + assert_eq!(config.top_p, Some(0.9)); + assert_eq!(config.top_k, Some(40)); + assert_eq!(config.stop_sequences, vec!["\n\n", ""]); + } + #[test] fn keeps_user_and_assistant_messages() { let items = vec![ diff --git a/docs/architecture.md b/docs/architecture.md index d785b440..061bd811 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -73,7 +73,6 @@ Pod の宣言的構成。TOML で記述。 ```toml [pod] name = "agent" -pwd = "/abs/path" [model] ref = "anthropic/claude-sonnet-4-6" @@ -81,6 +80,7 @@ ref = "anthropic/claude-sonnet-4-6" [worker] instruction = "$insomnia/default" max_tokens = 4096 +temperature = 0.3 [[scope.allow]] target = "/abs/path" diff --git a/docs/pod-factory.md b/docs/pod-factory.md index a30b0231..fe8776af 100644 --- a/docs/pod-factory.md +++ b/docs/pod-factory.md @@ -15,7 +15,7 @@ overlay をマージして、検証済みの `PodManifest` と `PromptLoader` | 1 | ビルトインのデフォルト | `manifest::defaults` モジュールの `pub const` 群を `PodManifestConfig::builtin_defaults()` が cascade 層として注入 | `tool_output.default_max_bytes = 16KB` など | | 2 | ユーザー manifest | `/manifest.toml`（解決ルールは `manifest::paths`） | プロバイダ指定、デフォルトモデル、常用ツール設定 | | 3 | プロジェクト manifest | 起動ディレクトリから上方向に探索した最初の `/.insomnia/manifest.toml` | scope、compaction、プロジェクト固有の instruction | -| 4 | プログラマティック overlay | CLI / GUI / 別 Pod からの spawn 等 | `pod.name`、`pod.pwd` のような Pod 固有値 | +| 4 | プログラマティック overlay | CLI / GUI / 別 Pod からの spawn 等 | `pod.name`、spawn 時の `worker.instruction` のような Pod 固有値 | デフォルト値はすべて `crates/manifest/src/defaults.rs` の `pub const` として集約 されており、serde `#[default = "..."]` 経路（`PodManifest` の直接 deserialize） @@ -31,6 +31,7 @@ overlay をマージして、検証済みの `PodManifest` と `PromptLoader` |---|---| | スカラー（`String`, `u32`, `bool` 等） | 上層に値があれば丸ごと置換 | | `Option` | 上層が `Some` なら置換、`None` なら据え置き | +| 配列スカラー（`worker.stop_sequences` 等） | 上層に値があれば配列ごと置換。追記マージはしない | | マップ（`tool_output.per_tool` 等） | キー単位でマージ、同一キーは上層優先 | | `scope.allow` / `scope.deny` | **union**（各層から全部足す）。上位層は `deny` で下位層の `allow` を必ず削れる | @@ -40,8 +41,8 @@ for PodManifest` が必須フィールド検証と絶対パス検証をかけて ## パス解決 -manifest 中のパス（`provider.api_key_file` / `scope.*.target` / -`compaction.provider.api_key_file`）は相対記述を許容する。相対パスは +manifest 中のパス（`model.auth.file` / `scope.*.target` / +`compaction.model.auth.file`）は相対記述を許容する。相対パスは **各層のベース基準**で層ごとに絶対化され、そのあとで cascade merge に かかる。層をまたいだ相対の意味ブレ（user 層の `./keys` が project 層の どこを指すのか曖昧）を避けるための設計。 @@ -135,6 +136,9 @@ instruction = "$user/reviewer" max_tokens = 4096 max_turns = 50 temperature = 0.3 +top_p = 0.9 +top_k = 40 +stop_sequences = ["\n\n", ""] reasoning = "medium" # 文字列 = effort label / 整数 = thinking budget tokens。詳細は docs/reasoning.md [worker.tool_output] @@ -161,16 +165,42 @@ permission = "write" prune_protected_turns = 3 prune_min_savings = 4096 compact_threshold = 80000 -compact_retained_turns = 2 +compact_request_threshold = 90000 +compact_retained_tokens = 8000 +compact_auto_read_budget = 8000 +compact_worker_max_input_tokens = 50000 -[compaction.provider] -kind = "gemini" -model = "gemini-2.0-flash" -api_key_file = "/home/you/.config/insomnia/keys/gemini" +[compaction.model] +scheme = "gemini" +model_id = "gemini-2.0-flash" +auth = { kind = "api_key", file = "/home/you/.config/insomnia/keys/gemini" } ``` --- +## `[worker]` 設定 + +`[worker]` は Pod 内の `llm_worker::RequestConfig` とターン制御へ渡す設定を持つ。 +Provider ごとの wire 名の違い（OpenAI の `max_completion_tokens` / +Responses の `max_output_tokens` / Gemini の `generation_config` など）は +scheme 側が吸収する。 + +| key | 型 | 既定 | 内容 | +|---|---|---|---| +| `instruction` | `String` | `$insomnia/default` | システムプロンプト本体として使う prompt asset 参照 | +| `max_tokens` | `u32` | 未指定 | 1 request の最大出力 token。scheme が provider の該当 wire field に投影 | +| `max_turns` | `NonZeroU32` | 未指定 | 1 run 内で Worker が進められる最大 turn 数 | +| `temperature` | `f32` | 未指定 | sampling temperature | +| `top_p` | `f32` | 未指定 | nucleus sampling | +| `top_k` | `u32` | 未指定 | top-k sampling。未対応 scheme では warning または provider 側挙動に任せる | +| `stop_sequences` | `Vec` | `[]` | stop sequence。cascade では上層指定が配列ごと置換する | +| `reasoning` | `String` または `i32` | 未指定 | reasoning / thinking 制御。詳細は `docs/reasoning.md` | +| `tool_output.default_max_bytes` | `usize` | `16384` | tool result `content` の既定 byte cap | +| `tool_output.per_tool` | `Map` | `{}` | tool 名ごとの byte cap override | + +生成設定は provider 別の値域検証を行わない。型が TOML と合わない場合は manifest +parse error になるが、provider が受け付けない値や組み合わせは API 応答で検出する。 + ## instruction とプロンプト資産 ### `worker.instruction` フィールド diff --git a/tickets/worker-generation-settings.md b/tickets/worker-generation-settings.md index 3b7939d8..f03967fc 100644 --- a/tickets/worker-generation-settings.md +++ b/tickets/worker-generation-settings.md @@ -49,3 +49,8 @@ Provider / scheme によって効かない値がある場合でも、既存の s - `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 diff --git a/tickets/worker-generation-settings.review.md b/tickets/worker-generation-settings.review.md new file mode 100644 index 00000000..6f0f275a --- /dev/null +++ b/tickets/worker-generation-settings.review.md @@ -0,0 +1,47 @@ +# Review: LLM 生成設定の manifest 露出整理 + +## 前提・要件の確認 + +- `[worker]` への `top_p` / `top_k` / `stop_sequences` 追加: 満たされている。 + - `WorkerManifestConfig` に `top_p: Option` / `top_k: Option` / `stop_sequences: Option>` を追加 (`crates/manifest/src/config.rs:65-73`)。 + - `WorkerManifest` 側にも `top_p` / `top_k` / `stop_sequences: Vec`（解決後は確定値、未指定は空 Vec）として露出 (`crates/manifest/src/lib.rs:104-109`)。 + - `TryFrom` で `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` (`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::or`）にそのまま乗っており、`stop_sequences` を `Option>` で持って `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 の範囲外編集は履歴粒度の指摘に留まり、コードベースを歪める変更はない。