feat: add TOOLUSE_MODEL offloading and Qwen 3.5 reasoning support#17
feat: add TOOLUSE_MODEL offloading and Qwen 3.5 reasoning support#17jasagiri wants to merge 11 commits into
Conversation
- Implement Planner-Actor offloading to delegate tool execution to a specialist model - Add system prompt optimization for reasoning models (Qwen 3.5, R1, etc.) - Enhance JSON/XML tool call extraction from text - Add optimized Modelfile for Qwen 3.5 tool use
ochyai
left a comment
ochyai
left a comment
There was a problem hiding this comment.
コードレビュー: PR #17 — TOOLUSE_MODEL offloading & Qwen 3.5 reasoning support
@jasagiri レビューします。Planner-Actor パターンのアイデアは面白いですが、いくつか設計上の懸念があります。
1. _is_reasoning_model() のマッチングが広すぎる (要修正)
return any(k in m for k in ["qwen3.5", "r1", "o1", "o3", "thinking", "reasoning", "tooluse"])"r1"→"error1","gpt4-turbo-pr1eview"等にマッチしてしまう"o1"→"solo1","photo1"等にマッチ"o3"→"proto3"にマッチ
修正案: ワードバウンダリを考慮する or モデル名のプレフィックス/サフィックスパターンで判定：
import re
_REASONING_PATTERNS = re.compile(
r"(?:^|[:/])(?:qwen3\.5|deepseek-r1|o[13](?:-|$)|thinking|reasoning)", re.I
)
def _is_reasoning_model(model_name):
return bool(model_name and _REASONING_PATTERNS.search(model_name))また "tooluse" がリストに含まれていますが、これはユーザーがModelfileで作成したカスタムモデル名を想定していますか？ その場合、ドキュメントに記載が必要です。
2. Offloading ヒューリスティックが脆弱 (要修正)
is_summary = any(kw in text for kw in ["正常に", "完了", "成功", ...])
needs_action = any(kw in text.lower() or kw in text for kw in ["Write", "Edit", "Bash", "実行", ...])問題点:
kw in text.lower() or kw in text—"Write"はtext.lower()に存在しない（"write"になる）が、or kw in textで元テキストの"Write"にはマッチする。ロジックとしてtext.lower()と大文字キーワードの比較は常に False になるので、実質kw in textだけが有効- 日本語キーワードと英語ツール名が混在しており、false positive が起きやすい（例: ユーザーのソースコードに "Write" が含まれている場合）
is_summaryのネガティブリストも不完全（例: 「終わりました」「処理済み」等は未カバー）
提案: キーワードマッチではなく、LLMの出力にツール呼び出しの「意図」があるかを構造的に判定する：
<tool_call>タグの存在チェック（パースは失敗したがタグはある場合）- JSON構造の部分的なマッチ（
{"name":パターン） - これなら言語非依存で確実
3. SilentTUI クラスがループ内で毎回定義される (要修正)
class SilentTUI:
def show_sync_response(self, data, known_tools):
...このクラスがエージェントループの各イテレーション内で再定義されています。モジュールレベルまたは Agent.__init__ 時に一度だけ定義すべきです。
4. _consecutive_denies が未使用 (要修正)
_consecutive_denies = 0 # track how many times user has denied in this turn初期化されていますが、どこでもインクリメントされていません。意図した機能が未実装であれば削除するか、実装を追加してください。
5. ask_permission の "abort" 返却値 — API互換性 (要確認)
ask_permission() が "abort" を返すようになりましたが、"abort" は truthy な文字列です。
現在の Agent.run() 内では正しく処理されていますが、permissions.check() 経由で return result する箇所（L5029, L5064）を通ると、他の呼び出し元（例: SubAgentTool L4202 の if not self._permissions.check(...) パターン）で not "abort" = False → 許可扱いになるリスクがあります。
SubAgentTool は tui=None を渡すため現時点では安全ですが、将来的な呼び出し元追加時に罠になります。
提案: permissions.check() 内で "abort" を False に変換するか、返却型を Enum にする。
6. Popen エラーハンドリング改善 ✅ Good
except FileNotFoundError as e:
return f"Error: Command execution failed. Program not found: {e}."これは良い改善です。shell=True でも稀にシェル自体が見つからないケースがあるため。
7. Permission deny 時の動作改善 ✅ Good
results.append(ToolResult(tc_id, "Error: user has denied tool execution. STOP this approach...", True))
break # Stop the loop earlydeny 後に continue ではなく break + 明確なメッセージで、LLMが同じ操作をリトライし続ける問題を防いでいます。ユーザー体験の改善として正しいです。
8. _norm_args のパス正規化 — パフォーマンス懸念 (軽微)
obj[k] = os.path.normpath(os.path.join(os.getcwd(), obj[k]))os.getcwd() はシステムコール。ループ検知のために毎回呼ぶのは過剰。cwd をキャッシュするか、正規化自体を省略して元の json.dumps(sort_keys=True) に戻すことを検討してください。ループ検出で同じパスの相対/絶対表記違いが問題になるケースは稀です。
9. Modelfile について (要修正)
FROM qwen35-vision:latest
qwen35-vision:latest はユーザーの環境に存在しない可能性が高いです。README または Modelfile 内にセットアップ手順（ollama pull コマンド等）を記載してください。また、このModelfileの位置づけ（実験的・参考用等）をドキュメントに明記してください。
まとめ
| 項目 | 判定 |
|---|---|
_is_reasoning_model マッチング |
❌ 要修正 |
| Offloading ヒューリスティック | ❌ 要修正 |
| SilentTUI ループ内定義 | ❌ 要修正 |
_consecutive_denies 未使用 |
❌ 要修正 |
| "abort" API互換性 | |
_norm_args パフォーマンス |
|
| Modelfile ドキュメント | |
| Popen エラーハンドリング | ✅ |
| Permission deny 動作改善 | ✅ |
上記の ❌ 項目を修正した上で再レビューをお願いします。コンセプト自体は有用なので、実装の堅牢性を上げれば良いPRになると思います。
…nd reviewer fixes - Improve _is_reasoning_model with regex word boundaries - Move _SilentTUI to module level - Strengthen Planner-Actor offloading trigger with structural detection - Performance: cache os.getcwd() in _norm_args - Robustness: handle 'abort' as None in permission check - Logic: add _consecutive_denies counter (stop turn after 3 denies) - Issue ochyai#19: Add Qwen 3.5 models to tiers/context sizes - Issue ochyai#19: Update install.sh and README.md for Qwen 3.5 recommendations
|
ご指摘いただき、ありがとうございます。指摘いただいた点をすべて修正し、本PRを更新しました。 また、関連する Issue #19 (Qwen 3.5サポート) についても、本PRのロジック修正（Planner-Actor）と密接に関連しているため、本PRに統合する形で最小限の更新を行いました。これにより、マージ後すぐにユーザーが Qwen 3.5 の恩恵を受けられるようになります。 主な変更点と修正内容:
検証内容:
補足: ご確認のほど、よろしくお願いいたします。 |
ochyai
left a comment
ochyai
left a comment
There was a problem hiding this comment.
再レビュー: PR #17 — 修正後の確認（v2）
@jasagiri 迅速な対応ありがとうございます。前回指摘した項目のほとんどが修正されていることを確認しました。以下、修正済みの項目と残存する問題を整理します。
✅ 修正確認済み
| 項目 | 状態 | コメント |
|---|---|---|
_is_reasoning_model() のワードバウンダリ |
✅ 修正済み | \b 正規表現で photo1, error1, proto3 等の誤マッチを防止。テストも追加されています |
SilentTUI のループ内再定義 |
✅ 修正済み | モジュールレベルの _SilentTUI に移動 |
_consecutive_denies の未使用 |
✅ 修正済み | カウンタが正しくインクリメント・チェックされ、3回連続拒否でループ停止 |
"abort" API互換性 |
✅ 修正済み | permissions.check() が None を返すようになり、bool(result) で安全に変換 |
_norm_args のパフォーマンス |
✅ 修正済み | _turn_cwd でキャッシュ済み |
| Modelfile のベースモデル | ✅ 修正済み | FROM qwen3.5:latest に変更 |
| deny 後の制御 | ✅ Good | break + 明確なメッセージで無意味なリトライを防止 |
❌ 残存する問題（要修正）
1. Ollamaモデルタグの不一致（致命的）
PR内で使用されているモデルサイズが、実際のOllamaレジストリに存在しないタグになっています：
| PR で使用 | 実際のOllamaタグ（Issue #19 調査） |
|---|---|
qwen3.5:32b |
qwen3.5:27b |
qwen3.5:14b |
qwen3.5:9b |
qwen3.5:7b |
qwen3.5:4b |
qwen3.5:3b |
qwen3.5:2b |
qwen3.5:1.5b |
qwen3.5:0.8b |
README、install.sh、MODEL_TIERS、MODEL_CONTEXT_SIZES すべてに影響します。@shigel さんの Issue #19 の調査結果と突き合わせて、正しいタグに修正してください。
2. Offloading ヒューリスティックの混合比較バグ（前回指摘の残存）
needs_action = any(kw in text.lower() or kw in text for kw in ["Write", "Edit", "Bash", "実行", "作成", "呼び出"])kw in text.lower() で "Write" を比較すると、text.lower() は全て小文字なので常に False。結果として or kw in text のみが有効で、大文字小文字の区別なし検索が意図通りに動いていません。
修正案:
text_lower = text.lower()
needs_action = any(kw.lower() in text_lower for kw in ["Write", "Edit", "Bash", "実行", "作成", "呼び出"])ただし、前回レビューでも指摘した通り、キーワードベースのヒューリスティック自体に根本的な脆弱性があります。構造的な判定（has_tags チェック）を主軸にし、キーワードマッチは補助に留めることを推奨します。
3. actor_messages の境界条件
actor_messages = [hist[0]] + hist[-2:] + [...]hist が1要素のみの場合（初回の応答等）、hist[0] と hist[-2:] が重複します。hist の長さを事前にチェックするか、重複を排除するロジックを追加してください。
⚠️ 軽微な指摘
4. _SIDECAR_CANDIDATES への qwen3.5 追加が見当たらない
MODEL_TIERS と MODEL_CONTEXT_SIZES には qwen3.5 エントリが追加されていますが、_SIDECAR_CANDIDATES（サイドカー自動選択用）への追加がdiffに含まれていません。サイドカーとして qwen3.5 の小型モデルを自動選択させるには追加が必要です。
5. Issue #19 の変更をPR #17に統合する判断について
コンセプトは理解できますが、PRのスコープが広がりすぎています。Qwen 3.5 のモデルエントリ追加は最小限に留め、install.sh のモデル選択ロジック変更は Issue #19 対応の別PRとして分離することを推奨します。理由:
- モデルタグの正確性を Issue #19 の @shigel さんと確認しやすくなる
- Planner-Actor 機能と モデル選択ロジック変更を分離レビューできる
- Ollama のツール呼び出しブロッカー（ollama#14493）が未解決のため、自動選択で qwen3.5 を推すのは時期尚早
まとめ
| 項目 | 判定 |
|---|---|
| Ollamaモデルタグ不一致 | ❌ 致命的・要修正 |
| Offloading キーワード比較バグ | ❌ 要修正 |
| actor_messages 境界条件 | ❌ 要修正 |
| _SIDECAR_CANDIDATES 追加漏れ | |
| Issue #19 スコープ分離 | |
| ワードバウンダリ修正 | ✅ |
| SilentTUI 移動 | ✅ |
| consecutive_denies 実装 | ✅ |
| abort 互換性 | ✅ |
| _norm_args キャッシュ | ✅ |
Planner-Actor パターンのコア実装は着実に改善されています。❌ 項目（特にモデルタグの修正）を対応いただければ、マージに近づきます。
また、Issue #19 のスコープと分離について @shigel さんとも相談の上、Qwen 3.5 対応部分をどちらのPRで進めるか決めていただけると助かります。
- Improve _is_reasoning_model with robust regex using word boundaries - Strengthen Planner-Actor offloading heuristics (structural detection over keywords) - Refine Actor system prompt and history management (avoid duplication) - Enhance _extract_tool_calls_from_text with raw JSON and nested object support - Support tool call extraction from reasoning blocks - Adjust system prompt instructions for reasoning models
- Implement Planner-Actor offloading trigger for reasoning models - Enhance _extract_tool_calls_from_text to support nested and raw JSON - Improve tool extraction from within code blocks for reasoning models - Add word-boundary aware reasoning model detection (Qwen 3.5, R1, o1) - Add Qwen 3.5 context window definitions (MODEL_CONTEXT_SIZES) - Fix agent loop to handle user abort and consecutive denies correctly - Cache os.getcwd() in _norm_args for better loop detection performance
2 participants
Planner-Actor パターンエンジンと堅牢なツール呼び出し抽出の導入
このプルリクエストでは、高推論モデル（Qwen 3.5、DeepSeek-R1 など）をコーディングエージェントとして効果的に機能させるため、実際のツール実行を専門モデルに委託する「Planner-Actor パターン」の基盤エンジンを導入します。本 PR は「仕組み（メカニズム）」の構築に焦点を当てており、将来のモデル固有のアップデートがスムーズに動作するための堅牢な土台を提供します。
主な変更点 (初期投稿時)
1. Planner-Actor オフローディング (TOOLUSE_MODEL)
</think>）の完了状態に基づく構造的な検知に切り替え、精度を向上させました。2. 堅牢なツール呼び出し抽出
<think>タグ内に含まれるツール呼び出しもスキャン対象化。2026-03-29 更新：指摘事項の完全解消と Qwen 3.5 への基盤対応
再度動作確認を行い、これまでの議論や指摘を完全に反映させる形で内容を更新しました。
今回のアップデート内容
1. 堅牢なツール呼び出し抽出の完成 (Issue #17)
```json等のブロック内にツール呼び出しを記述するケースに対応し、全テキストからの正確な抽出をサポートしました。2. 推論型モデルへの基盤対応（Qwen 3.5 等）
qwen3.5、deepseek-r1、o1等を正確に識別します。MODEL_CONTEXT_SIZESに Qwen 3.5 系の定義を追加し、大規模コンテキストを手動利用時でも適切に扱えるようにしました。3. 安定性の向上
abort(Ctrl+C 等) や、3 回連続のツール実行拒否時に、エージェントループが即座かつ確実に停止するよう修正しました。_norm_args内でのos.getcwd()呼び出しを最適化し、ループ検知のパフォーマンスを向上させました。検証結果
tests/test_vibe_coder.pyの全 857 テストをパス。