fix: 第5章の書籍整合性向上、思考ループの最適化、およびサポートサイトの更新

[laiso]

第5章の書籍原稿と配布コードの整合性向上のための修正およびサポートサイトの更新、単体テストの追加。

[github-actions]

コードレビューしました。

全体として、Agent の思考ループ・ツール実行・承認ゲートに対するテストが追加されており、今回の変更で意図した挙動をかなり追いやすくなっています。executeTool としてツール実行処理を切り出した点も、例外時に LLM へエラー文字列を返す責務が明確になっていて良い整理だと思います。bun test も実行し、既存/追加テストが通ることを確認しました。

気になった点は 1 点です。

• src/core/agent.ts の「ツールが一度も使用されずに終了しました」警告条件が toolCallCount === 0 && currentStep === 1 になっていますが、コメントでは「ツールが一度も使われなかった場合」と説明されているため、finishReason === 'length' などで複数ステップ継続したあと結局ツール未使用で終了したケースでは警告されません。意図が「1ステップで通常終了した場合のみ警告」ならコメントを合わせる、意図がコメント通りなら toolCallCount === 0 のみ、または finishReason も含めた条件にするのがよさそうです。

ドキュメント更新も含めて大きな問題は見当たりませんでした。

[github-actions]

コードレビューしました。テスト追加により Agent の基本ループ・承認ゲートがカバーされている点は良いと思います。一方で、既存機能の回帰と思われる点がいくつかあります。

指摘事項

1. useStreaming が完全に無視され、--stream が機能しなくなっています

src/core/agent.ts で collectStreamResult / generate-stream の利用が削除され、this.useStreaming はコンストラクタで保持されるだけで generate() 内では参照されていません。

そのため、bin/cli.ts の --stream オプションは受け取られるものの、実際には常に非ストリーミングの generateText() が呼ばれます。以前は model.doStream があれば逐次出力され、未対応時のみ通常 API にフォールバックしていたため、これは既存 CLI 機能の回帰です。

対応案:

• 以前のように this.useStreaming && this.model.doStream の場合は collectStreamResult() を使う
• もし意図的にストリーミングを廃止するなら、AgentConfig.useStreaming、CLI の --stream、関連ドキュメントも削除・修正する

2. finishReason の扱いが失われ、length / content_filter / error が stop として扱われます

現在の generate() は、ツール呼び出しがない場合に常に finishReason = 'stop' として終了しています。

messages.push({ role: 'assistant', content: response.text });
finishReason = 'stop';
break;

このため、モデルが finishReason: 'length' を返して応答が途中で切れた場合や、content_filter / error を返した場合でも、呼び出し側には正常終了のように見えてしまいます。以前の実装では少なくとも length は警告して継続、content_filter はその理由で終了していました。

対応案:

• finishReason = response.finishReason を基本にする
• length の場合は継続するのか中断するのかを明示的に処理する
• content_filter / error は呼び出し側が判別できる形で返す
• この挙動のテストを追加する

3. CLI の通常実行でエージェントの応答が表示されなくなっています

旧実装では各ステップの response.text を console.log(response.text) していましたが、新実装では verbose のときだけ出力されます。

一方で bin/cli.ts / bin/review.ts は verbose: true を渡しておらず、agent.generate() の戻り値 result.text も表示していません。そのため、ローカルで CLI を実行した場合に最終応答がコンソールに出ない可能性があります。特に --stream も上記の通り無効化されているため、ユーザーから見ると「実行したが何も返ってこない」挙動になり得ます。

対応案:

• Agent 側で従来通り通常出力する
• もしくは CLI 側で result.text を表示する
• ログ量を抑えたい場合は verbose と「最終応答の表示」を分けて制御する

良い点

• src/core/agent.test.ts に、通常のツール実行、maxSteps、承認・拒否の主要ケースが追加されており、Agent の中核挙動を確認しやすくなっています。
• executeTool() でツール例外をツール結果として返す設計は、LLM にリカバリさせる目的では有用です。

検証

• bun test を実行し、既存・追加テストが通ることを確認しました。

ただし上記の回帰は現状のテストでは検出されていないため、特に useStreaming と finishReason 周りのテスト追加をおすすめします。

[github-actions]

コードレビューしました。Agent の思考ループをシンプルにしつつ、承認ゲートや maxSteps のテストを追加している点は良いと思います。bun test と bun x tsc --noEmit はどちらも通りました。

気になった点です。

1. --stream / useStreaming がサイレントに無効化されています

   • AgentConfig とコンストラクタには useStreaming が残っていますが、generate() 内では参照されていません。
   • そのため bin/cli.ts --stream はログ上はストリーミングモードになりますが、実際には常に generateText() が呼ばれます。
   • 既存実装では collectStreamResult() + model.doStream にフォールバック付きで対応していたため、これは機能退行に見えます。意図的にストリーミングを外すなら CLI オプションやドキュメントも合わせて削除/変更するのがよさそうです。互換性を維持するなら、旧実装と同様に this.useStreaming 分岐を戻すことをおすすめします。

2. Agent.generate() の戻り値から finishReason / usage が消えており、呼び出し側の情報表示が壊れています

   • 旧実装は { text, finishReason, usage } を返していましたが、新実装は { text } のみです。
   • bin/cli.ts / bin/review.ts 側では result as any にして usage を読もうとしていますが、現在の Agent は usage を返さないため CI の使用トークン表示は常に出なくなります。
   • また finishReason が返らないことで、length / content_filter / max_steps などの終了理由を呼び出し側が判定できなくなっています。型を緩めるより、Agent.generate() の戻り値型を維持して usage を累積する方が安全です。

3. finishReason: 'length' などの終了制御が失われています

   • 新実装ではツール呼び出しがない場合、response.finishReason を見ずに break しています。
   • 旧実装では length の場合に警告して次ステップへ進み、content_filter では中断理由を保持していました。長い応答が途中で切れたケースでも「完了」として扱われる可能性があります。
   • 少なくとも length / content_filter / error の扱いは復元するか、意図的な仕様変更としてテストとドキュメントで明示した方がよいです。

4. ドキュメントに文字化けらしき箇所があります

   • workspace/docs/index.html の bin/cli.ts --responses の説明付近に （付録 B）��試そう という文字化けが入っています。
   • （付録 B）を試そう などに修正をお願いします。

追加された src/core/agent.test.ts は、通常のツール実行・承認/拒否・maxSteps を押さえていて良いです。上記の退行を防ぐため、ストリーミング有効時に doStream が使われること、usage が返ること、finishReason: 'length' の扱いについてもテストを追加すると安心です。

[github-actions]

レビューしました。Agent の基本ループや承認ゲートに対するテストが追加されている点は良いと思います。bun test でも既存・追加テストは通っています。

一方で、今回の src/core/agent.ts の置き換えにより既存 CLI から見える挙動にいくつか回帰がありそうです。

指摘事項

1. --stream / useStreaming が実質無効になっています

   • AgentConfig と Agent には useStreaming が残っていますが、generate() 内では常に generateText() が呼ばれており、collectStreamResult() / model.doStream() が使われなくなっています。
   • bin/cli.ts では --stream を受け取って useStreaming: streamMode を渡しているため、ユーザーから見るとオプションは存在するのにストリーミングされない状態になります。
   • 対応案: 旧実装と同様に this.useStreaming && this.model.doStream の場合は collectStreamResult() を使う、またはこのPRの目的が第5章サンプルへの簡略化であれば CLI 側の --stream 表示・オプションやドキュメントも合わせて調整すると良さそうです。

2. usage / finishReason が返らなくなり、CLI の使用トークン表示が壊れています

   • 旧 Agent.generate() は { text, finishReason, usage } を返していましたが、新実装は { text } のみです。
   • その結果、bin/cli.ts / bin/review.ts は result as any で型エラーを回避していますが、実際には使用トークンが出力されなくなります。
   • 対応案: GenerateTextResult の usage.totalTokens を累積して従来互換の戻り値を返すか、CLI 側から使用トークン表示を削除して仕様変更として明示するのがよいと思います。as any は型安全性を下げるため、できれば避けたいです。

3. finishReason: 'length' / content_filter 等の終了制御が失われています

   • 新実装ではツール呼び出しがない場合、response.finishReason に関係なく会話を終了します。
   • 旧実装では length の場合に警告して次ステップへ進み、content_filter は明示的に終了理由として返していました。
   • 出力が途中で切れたケースを正常完了扱いしてしまう可能性があるため、少なくとも length / content_filter / error の扱いは戻した方が安全です。

4. コンテキスト管理が削除されています

   • 旧実装にあった manageContext() がなくなったため、長いツール結果が続くと会話履歴が肥大化して API エラーやコスト増につながりやすくなります。
   • ドキュメントには「実用上の差異」として触れられていますが、実際の CLI 実行でも Agent は使われるため、実用コードとしては保持した方がよい機能だと思います。

5. chapters/05-coding-agent.ts が実行時に外部 API を呼ぶトップレベル await 付きサンプルになっています

   • サンプルとしては分かりやすいですが、単に import しただけで codingAgent.generate(...) が実行されます。
   • 再利用やテスト時の副作用を避けるため、if (import.meta.main) { ... } のように直接実行時だけ走る形にするか、エクスポート用ファイルと実行用ファイルを分けると安全です。

良い点

• src/core/agent.test.ts でツール実行、maxSteps、承認・拒否の主要パスをカバーしている点は良いです。
• ツール実行例外を executeTool() に分離して、例外をツール結果としてモデルへ返す構成はシンプルで読みやすいです。

検証

• bun test を実行し、テストは通過しました（出力は長いため一部省略されました）。

上記のうち特に 1〜3 は既存 CLI の利用者に影響する回帰になりやすいため、マージ前に対応または仕様変更として明示することをおすすめします。

[laiso]

📋 コードレビュー

全体的に非常に質の高い PR で、Agent クラスの実装簡素化とテストの充実が印象的です。以下、指摘と提案を記載します。

✅ 良い点

1. Agent.ts のリファクタリング

   • 戻り値を { text: string } に統一し、API をシンプルにしたのは優れた設計
   • executeTool を独立関数として抽出し、エラーハンドリングを統一
   • 各ステップを詳細にコメント化（5.3節など）し、可読性が高い

2. テストの充実 🎯

   • src/core/agent.test.ts は非常に包括的
   • 思考ループ、maxSteps 制御、承認ゲート、ストリーミング、finishReason など主要な機能を網羅
   • モデルのモック実装が現実的で、複数段階のシナリオ検証が可能

3. ドキュメント更新

   • API 不整合エラー対策（6.5 節）の説明が詳しく、各プロバイダでの対処方法を明示
   • 新しい Agent API の変更点を丁寧に記載

4. 実用的な問題への対処

   • cleanMessages をコメントアウトで提供することで、本書の簡潔さを保ちつつ実用的な問題へ対応
   • tsconfig.json に strictFunctionTypes: false を追加して型チェックエラーを解消

⚠️ 改善提案

1. 型安全性の強化

// bin/cli.ts, Line 176
const res = result as any;  // ← `any` は型チェック を無効化

提案: Agent.generate() の戻り値に以下を検討：

interface AgentResponse {
  text: string;
  // 必要に応じて usage を統計目的で保持
}

これにより as any キャストが不要に。

2. Message.content の安全なアクセス

// src/core/agent.ts, Line 212
let totalLength = messages.reduce((sum, m) => sum + m.content.length, 0);

懸念: toolCalls を持つアシスタント応答で content が undefined の可能性
提案:

let totalLength = messages.reduce((sum, m) => sum + (m.content?.length ?? 0), 0);

3. API 不整合エラー対策の実装統一

各プロバイダ（openai.ts, anthropic.ts, google.ts）の cleanMessages 実装が重複しています。

提案: 共通の cleanMessages 関数を src/utils/ に配置し、各プロバイダから呼び出す方式に統一すると保守性が向上します。

4. ストリーミング出力制御の単純化

// src/core/agent.ts, Line 104-108
if (!this.useStreaming || !this.model.doStream) {
    console.log(response.text);
}

ストリーミング時の改行が Line 90 で既に追加されているため、条件がやや複雑に見えます。

提案: コメント追加で意図を明示

// ストリーミング時は collectStreamResult 内で逐次出力済みなのでスキップ
if (!this.useStreaming || !this.model.doStream) {
    console.log(response.text);
}

5. テストの追加検討

テストは全体的に良いですが、以下の追加を検討：

• ツール呼び出し-結果ペア整合性テスト: manageContext で古いメッセージを削除する際、ツール呼び出しと結果の対応が壊れるケースのテスト
• collectStreamResult のエラーケース: ストリーミング中の例外発生シナリオ

📝 質問・確認事項

1. bin/review.ts での usage 削除の意図

   - if (result.usage) {
   -    console.log(`[使用トークン] ${result.usage.totalTokens} tokens`);
   - }

   削除した理由がコメントに記載されていません。API 設計の変更に伴うものなら、コメント追加を検討してください。

2. chapters/05-coding-agent.ts の import.meta.main
   Bun 固有の拡張機能ですが、互換性に関する注記があると良いかもしれません。

🎉 総評

この PR は、Agent クラスの API 簡潔化と実装の品質向上を同時に実現しています。テストカバレッジも充実しており、マージ後の保守性が高いコードになると予想されます。

提案した改善は、いずれも「強制」ではなく「検討」レベルの内容です。現在の実装でも機能上に問題はありません。

---

Reviewed by CodeReviewBot

[github-actions]

レビューしました。テスト（bun test）と型チェック（bun tsc --noEmit）はいずれも成功しています。コメント追加やプロバイダ実装の整理、コンテキスト管理周りのテスト追加は良い改善だと思います。

ただし、1点修正をお願いしたいです。

必須修正: cleanMessages が複数 tool call を持つ assistant を分割してしまう可能性があります

bin/review.ts の cleanMessages で、孤立した tool メッセージに対して直前までの finalMessages から同じ toolCallId を持つ assistant を探していますが、後続にある assistant の toolCalls からは existingToolCallIds に存在しない tool call を削除しています。

このため、例えば元の履歴が以下のような場合:

assistant(toolCalls: [callA, callB])
tool(callA)
// tool(callB) がコンテキスト削減などで欠落

現在の実装では assistant が toolCalls: [callA] に書き換えられます。OpenAI/Anthropic のツール呼び出し履歴では、assistant が複数の tool calls を返した場合、それぞれに対応する tool result が続く必要がある一方で、assistant 側の tool calls だけを削ると、モデルが実際に発行した tool call の内容が改変されます。さらにプロバイダによっては assistant/tool の対応関係や順序に厳密なため、履歴の意味が変わったり API エラーの原因になり得ます。

孤立した tool を補完する方向に合わせるなら、assistant の toolCalls を削除するのではなく、欠落している tool result 側をダミーの tool メッセージで補完する、または assistant メッセージごと安全に落とす、など会話履歴の整合性が崩れない方法にするのがよさそうです。少なくとも「複数 toolCalls の一部だけ tool response が欠落している」ケースのテストを追加して期待動作を固定してください。