feat(agentic): Phase 0 — IChatClient<T> model abstraction (epic #1544)

[ooples]

Phase 0 (foundations) — slice 1 of the agentic-orchestration epic

Part of #1544 (Epic: Agentic Orchestration — surpass Semantic Kernel & LangGraph).

This PR lands the model-abstraction layer — the message-based chat-model contract everything else in Phase 0 builds on. It supersedes the legacy text-in/text-out IChatModel<T> / ILanguageModel<T> (which cannot express native tool calls, streaming, or structured output).

What's in this slice

New namespace AiDotNet.Agentic.Models (src/Agentic/Models/):

• IChatClient<T> — message-based, native tool-calling, streaming (IAsyncEnumerable), structured output.
• Enums (no magic strings): ChatRole, ChatFinishReason, ToolChoiceMode, ChatResponseFormatKind.
• Multimodal content parts: AiContent + TextContent / ImageContent / ToolCallContent / ToolResultContent.
• ChatMessage (System/User/Assistant/Tool factories, .Text / .ToolCalls projections).
• AiToolDefinition (JSON-Schema JObject), ChatOptions, ChatUsage, ChatResponse, ChatResponseUpdate, StreamingToolCallUpdate.

Not in this slice (subsequent Phase 0 PRs)

Tool/function abstraction + source-generated JSON schema; reimplemented OpenAI/Anthropic/Azure connectors on IChatClient<T>; Microsoft.Extensions.AI adapter; typed structured-output helper.

Verification

• Core AiDotNet.csproj builds clean on net10.0 + net471, 0 errors (warnings-as-errors on).
• UnitTests/Agentic/Models/ChatModelAbstractionTests.cs: 15/15 pass on net10.0 and net471, including a fake-client streaming round-trip (confirms IAsyncEnumerable on the .NET Framework polyfill path).

Conventions followed: generic <T>, Guard.* checks, enums-not-strings, "For Beginners" XML docs, #nullable disable (not !) for null-guard tests.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • New provider-neutral chat client API with native connectors for OpenAI, Anthropic, Azure, and MEAI; supports streaming, non-streaming, multimodal inputs, and structured/JSON-schema outputs.
  • Agent tooling system: register/invoke tools, scan methods as tools, generate tool schemas at compile time, and execute tools with structured parameters.
  • Improved reasoning/benchmarking now accept external chat clients.

• Breaking Changes

  • Legacy agent framework, prompt-chain and several old model/tool interfaces removed; APIs migrated to new chat/tool abstractions.

• Chores

  • Pinned Microsoft.Extensions.AI.Abstractions to 10.6.0.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments

Project	Deployment	Actions	Updated (UTC)
aidotnet_website	Ignored	Preview	Jun 11, 2026 2:25am
aidotnet-playground-api	Ignored	Preview	Jun 11, 2026 2:25am

[coderabbitai]

Warning

Review limit reached

@ooples, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 21 minutes and 9 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c0b51289-56ce-4d00-a35f-7174f5f8661e

📥 Commits

Reviewing files that changed from the base of the PR and between 03a68eb and 2f4e9e5.

📒 Files selected for processing (6)

• src/Agentic/Models/ImageMediaTypeExtensions.cs
• src/Agentic/Models/ToolCallContent.cs
• src/Agentic/Tools/FunctionAgentTool.cs
• src/Agentic/Tools/JsonSchemaGenerator.cs
• src/AiDotNet.Generators/AgentToolSchemaGenerator.cs
• src/Reasoning/ReasoningStrategyBase.cs

Walkthrough

Adds agentic chat/message/tool contracts, a connector base plus OpenAI/Anthropic/Azure/MEAI adapters, tool runtime/schema generation and a source generator, and migrates reasoning, benchmarking, and builder code to use IChatClient<T>/IAgentTool; removes legacy agent/chain/tool interfaces and implementations.

Changes

Agentic chat stack migration

Layer / File(s)

Summary

All changes (single checkpoint) src/Agentic/**, src/Agentic/Models/Connectors/**, src/Agentic/Tools/**, src/AiDotNet.Generators/**, src/Reasoning/**, src/Models/**, src/AiModelBuilder*.cs, src/Benchmarking/**, src/Interfaces/**, Directory.Packages.props, src/AiDotNet.csproj

Introduces agentic contracts (AiContent, ChatMessage, ChatResponse, ChatResponseUpdate, ChatOptions, ChatRole, ChatFinishReason, ChatUsage, ChatResponseFormatKind, image/media types, streaming/tool content types) and IChatClient<T>; adds tooling runtime (IAgentTool, AgentToolBase, DelegateAgentTool, FunctionAgentTool, ToolCollection, ToolInvocationResult, AgentToolAttribute, ToolParameterAttribute, JsonSchemaGenerator, AgentToolFactory, StructuredOutputExtensions) and a Roslyn AgentToolSchemaGenerator; adds ChatClientBase<T> and OpenAI/Azure/Anthropic/MEAI connectors with streaming and tool mapping; migrates reasoning/training/benchmarks/builder/APIs to depend on IChatClient<T>/IAgentTool and to accept external chat clients; removes legacy agent stack, prompt-chain/chain types, older language-models, prompt-engineering chain/tools, and many tool implementations; updates project/package wiring.

Sequence Diagram

sequenceDiagram
  participant AiModelResult
  participant IChatClientT
  participant ChatClientBaseT
  participant ChatConnector
  participant IAgentTool
  participant ToolCollection

  AiModelResult->>IChatClientT: ReasonAsync/EvaluateBenchmarkAsync
  IChatClientT->>ChatClientBaseT: GetResponseAsync/GetStreamingResponseAsync
  ChatClientBaseT->>ChatConnector: provider-specific request/response flow
  ChatConnector->>IAgentTool: InvokeAsync for tool calls
  ToolCollection->>IAgentTool: dispatch tool invocation
  ChatConnector-->>IChatClientT: ChatResponse/ChatResponseUpdate
  IChatClientT-->>AiModelResult: response text / reasoning output

Loading

Estimated code review effort
🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

• ooples/AiDotNet#423: Adds the legacy agent stack that this PR removes from src/Agents/*.
• ooples/AiDotNet#828: Touches agent hyperparameter and registry components related to removals in this PR.
• ooples/AiDotNet#1368: Adjusts AiModelBuilder/AiModelResult wiring related to builder result pipelines.

Suggested labels
feature, documentation, testing

A new chat stack finds its voice,
Tools learn schemas, generators rejoice.
Old chains depart, new clients call,
Streams and tool-calls answer all.
Review sharp eyes — production ready or fall.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feature/agentic-phase0-model-abstractions

[vercel]

Deployment failed with the following error:

Resource is limited - try again in 24 hours (more than 100, code: "api-deployments-free-per-day").

Learn More: https://vercel.com/franklins-projects-02a0b5a0?upgradeToPro=build-rate-limit

[coderabbitai]

Actionable comments posted: 12

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (15)

src/Interfaces/IAiModelBuilder.cs (1)

1467-1479: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Blocking: the public ConfigureReasoning example still references removed ConfigureAgentAssistance.

This XML doc now advertises a call chain that no longer exists on IAiModelBuilder, so the published API example is broken the moment users paste it. Production-ready docs here should compile against the new chat-client/tooling surface or remove the outdated step entirely.

🛠 Suggested fix

-    /// var agentConfig = new AgentConfiguration<double>
-    /// {
-    ///     ApiKey = "sk-...",
-    ///     Provider = LLMProvider.OpenAI,
-    ///     IsEnabled = true
-    /// };
-    ///
     /// var result = await new AiModelBuilder<double, Matrix<double>, Vector<double>>()
-    ///     .ConfigureAgentAssistance(agentConfig)
     ///     .ConfigureReasoning()
     ///     .BuildAsync();

If reasoning now requires a different configuration path, replace the removed step with that new supported setup instead of leaving the stale example in place.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Interfaces/IAiModelBuilder.cs` around lines 1467 - 1479, The XML doc
example for IAiModelBuilder is stale: it shows chaining
ConfigureAgentAssistance(...) before ConfigureReasoning(), but
ConfigureAgentAssistance no longer exists; update the example under
ConfigureReasoning to use the current initialization/configuration flow for
IAiModelBuilder (e.g., remove the ConfigureAgentAssistance call or replace it
with the new supported method used to enable agent-related settings), ensuring
the snippet compiles against AiModelBuilder<TModel,...> and references only
existing public APIs (e.g., ConfigureReasoning, the current agent/config setup
method) so the example is valid.

Source: Coding guidelines

src/Models/Results/AiModelResult.cs (1)

1441-1456: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Refresh the public XML docs for the new IChatClient<T> contract.

EvaluateBenchmarksAsync adds chatClient without documenting it, and the benchmark/reasoning docs still describe the removed agent-configured flow and examples that omit the new required argument. That leaves the facade docs out of sync with the public API, and some sample calls no longer compile. Please update the XML comments/examples and call out this break in migration notes.

📝 Suggested doc updates

-    /// <param name="cancellationToken">Cancellation token.</param>
+    /// <param name="chatClient">Chat client used for benchmark suites that need model-generated answers.</param>
+    /// <param name="cancellationToken">Cancellation token.</param>

-    /// <exception cref="InvalidOperationException">Thrown when agent configuration is not set up.</exception>
+    /// <exception cref="ArgumentNullException">Thrown when <paramref name="chatClient"/> is null.</exception>

-    /// - Agent configuration must be set (ConfigureAgentAssistance during building)
+    /// - A chat client must be supplied

-    /// var result = await modelResult.ReasonAsync(
-    ///     "If a train travels 60 mph for 2.5 hours, how far does it go?",
-    ///     ReasoningMode.ChainOfThought
-    /// );
+    /// var result = await modelResult.ReasonAsync(
+    ///     "If a train travels 60 mph for 2.5 hours, how far does it go?",
+    ///     chatClient,
+    ///     ReasoningMode.ChainOfThought
+    /// );

Based on learnings: for this repo’s public API surface, prefer clean breaking changes rather than shims, but document the break in release notes and/or migration notes so downstream consumers can update.

Also applies to: 5911-5927, 6562-6600, 6619-6640, 6652-6682, 6694-6726

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Models/Results/AiModelResult.cs` around lines 1441 - 1456, Update the
public XML documentation for EvaluateBenchmarksAsync to describe the new
IChatClient<T> parameter (its purpose, when to provide null, and how it affects
evaluation), update method summary/params/returns to reference
BenchmarkingOptions, IChatClient<T>, and BenchmarkReport, refresh any sample XML
example calls to include a chatClient argument or show how to call with
null/default, and add a brief migration note indicating this is a breaking
change so downstream callers must supply an IChatClient<T> or adjust their call
sites; apply the same documentation updates to the other affected
overloads/locations referenced in the review (the ranges noted) to keep the
public API docs consistent.

Source: Learnings

src/Reasoning/DomainSpecific/CodeReasoner.cs (1)

74-81: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Document this public API break across the domain reasoners.

These constructors now require IChatClient<T>/IAgentTool> instead of the legacy abstractions, so downstream callers will hit both source and binary breaks when upgrading. Please add explicit migration or release-note guidance alongside this PR.

Based on learnings, clean breaking changes are acceptable here, but they should be called out in release or migration notes so downstream consumers can update intentionally.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Reasoning/DomainSpecific/CodeReasoner.cs` around lines 74 - 81, The
public constructor for CodeReasoner now takes IChatClient<T> and
IEnumerable<IAgentTool> causing a source/binary break; update the PR to include
clear migration guidance and release-note text explaining the API change and how
consumers should migrate (e.g., replace legacy chat/agent abstractions with
IChatClient<T> and IAgentTool, update construction of ChainOfThoughtStrategy<T>,
TreeOfThoughtsStrategy<T>, CriticModel<T>, and SelfRefinementEngine<T> usages),
and add a short example snippet or checklist for downstream maintainers to
follow when upgrading their codebases.

Source: Learnings

src/Reasoning/Components/ThoughtEvaluator.cs (2)

66-67: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Blocking: the IChatClient<T> migration dropped cancellation propagation in two reasoning components.

Both components accept a CancellationToken, but neither passes it into the outbound model call. That leaves canceled reasoning runs waiting on the remote LLM anyway. Thread the token through both GenerateResponseAsync calls.

As per coding guidelines, missing error handling at system boundaries is a blocking production-readiness issue.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Reasoning/Components/ThoughtEvaluator.cs` around lines 66 - 67, The
GenerateResponseAsync calls dropped CancellationToken propagation after the
IChatClient<T> migration; update the calls in ThoughtEvaluator (and the other
reasoning component that calls IChatClient<T>.GenerateResponseAsync) to pass the
incoming CancellationToken through to GenerateResponseAsync so cancellations
abort the remote LLM call (e.g., change string response = await
_chatModel.GenerateResponseAsync(prompt); to call the overload that accepts the
token), and ensure you use the same token parameter name (CancellationToken
token) from the method signature so cancellation flows end-to-end.

Source: Coding guidelines

---

120-124: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Blocking: validate the "score" token before converting it.

This is parsing external model output. If the model returns "score": "high" or an object/array, Value<double>() throws and the evaluator fails instead of falling back to the regex/default path.

Suggested fix

             var scoreToken = root["score"];
             if (scoreToken != null)
             {
-                double score = scoreToken.Value<double>();
-                return MathHelper.Clamp(score, 0.0, 1.0);
+                try
+                {
+                    double score = scoreToken.Value<double>();
+                    return MathHelper.Clamp(score, 0.0, 1.0);
+                }
+                catch (FormatException)
+                {
+                    // Continue to fallback parsing below.
+                }
+                catch (InvalidCastException)
+                {
+                    // Continue to fallback parsing below.
+                }
             }

As per coding guidelines, missing validation of external inputs is a blocking production-readiness issue.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Reasoning/Components/ThoughtEvaluator.cs` around lines 120 - 124, The
code reads root["score"] and calls scoreToken.Value<double>() directly; validate
the token before conversion to avoid exceptions on non-numeric model output. In
the block where scoreToken is obtained (root["score"]), check scoreToken.Type
(or use scoreToken.ToString()) and attempt a safe parse with
double.TryParse(..., CultureInfo.InvariantCulture) or only accept
JTokenType.Float/Integer; if parsing fails, do not call Value<double>() and
instead fall back to the existing regex/default path; then clamp the parsed
double with MathHelper.Clamp as before. Ensure you reference scoreToken,
Value<double>(), MathHelper.Clamp and the fallback path in the ThoughtEvaluator
logic.

Source: Coding guidelines

src/Reasoning/DomainSpecific/MathematicalReasoner.cs (1)

129-132: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Blocking: guard ReasoningChain before entering verification.

VerifyAndRefineAsync immediately iterates chain.Steps, but the caller only checks result.Success. The other reasoners in this PR already treat ReasoningChain as nullable, so a successful result without a populated chain will throw here.

Suggested fix

-        if (useVerification && result.Success)
+        if (useVerification && result.Success && result.ReasoningChain != null)
         {
             result = await VerifyAndRefineAsync(result, config, cancellationToken);
         }

     private async Task<ReasoningResult<T>> VerifyAndRefineAsync(
         ReasoningResult<T> result,
         ReasoningConfig config,
         CancellationToken cancellationToken)
     {
+        if (result.ReasoningChain == null)
+            return result;
+
         var chain = result.ReasoningChain;
         bool hasCalculationErrors = false;
         var verificationResults = new List<string>();

As per coding guidelines, missing null checks on boundary data are blocking production-readiness issues.

Also applies to: 146-156

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Reasoning/DomainSpecific/MathematicalReasoner.cs` around lines 129 - 132,
The code calls VerifyAndRefineAsync whenever result.Success is true but doesn't
ensure result.ReasoningChain is non-null, which causes iteration over
chain.Steps to throw; update the guard to require both result.Success and
result.ReasoningChain != null before calling VerifyAndRefineAsync (and mirror
the same null-check where similar verification is triggered around the 146-156
block), or alternatively add a defensive null check at the start of
VerifyAndRefineAsync to return/skip if the incoming ReasoningChain is null so
iteration over chain.Steps is never attempted on a null object.

Source: Coding guidelines

src/Reasoning/Components/ThoughtGenerator.cs (1)

141-165: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Blocking: don't silently return zero thoughts for valid-but-unusable JSON.

Right now only JsonException triggers the line-based fallback. A response like { "items": [...] } or { "thoughts": "single string" } parses successfully, but ParseThoughts returns an empty list and the generator silently does nothing.

Suggested fix

     private List<string> ParseThoughts(string response, int expectedCount)
     {
         var thoughts = new List<string>();

         try
         {
             // Try JSON parsing
             string jsonContent = ExtractJsonFromResponse(response);
             var root = JObject.Parse(jsonContent);

             if (root["thoughts"] is JArray thoughtsArray)
             {
                 foreach (var thought in thoughtsArray)
                 {
                     string thoughtText = thought.Value<string>() ?? "";
                     if (!string.IsNullOrWhiteSpace(thoughtText))
                     {
                         thoughts.Add(thoughtText.Trim());
                     }
                 }
             }
         }
         catch (JsonException)
         {
-            // Fallback to line-based parsing
-            thoughts = ParseThoughtsFromLines(response, expectedCount);
+            thoughts = ParseThoughtsFromLines(response, expectedCount);
         }
+
+        if (thoughts.Count == 0)
+        {
+            thoughts = ParseThoughtsFromLines(response, expectedCount);
+        }

         return thoughts;
     }

As per coding guidelines, half-implemented code paths that silently do nothing are blocking issues.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Reasoning/Components/ThoughtGenerator.cs` around lines 141 - 165, The
current JSON parsing block in ThoughtGenerator (using ExtractJsonFromResponse
and JObject.Parse) only falls back to ParseThoughtsFromLines on JsonException,
which lets valid-but-unusable JSON (e.g., missing "thoughts" key or "thoughts"
not being a JArray) produce an empty thoughts list silently; update the logic so
that after parsing the JObject you verify that root["thoughts"] is a non-empty
JArray and contains string items, and if not (null, wrong type, or yields no
valid strings) call ParseThoughtsFromLines(response, expectedCount) as the
fallback; preserve throwing/parsing errors, but ensure you do not return an
empty list silently—optionally emit a warning via the existing logging mechanism
when falling back.

Source: Coding guidelines

src/Reasoning/Strategies/ChainOfThoughtStrategy.cs (2)

149-157: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Blocking: remove the fake verification path.

When EnableVerification is on, this branch ships a TODO and marks every step as verified anyway. That creates false-positive verification and contaminates downstream confidence/metrics. Production-ready code here should either call a real verifier or fail fast until one is wired.

💡 Suggested fix

         if (config.EnableVerification && parsedSteps.Count > 0)
         {
-            AppendTrace("Verification enabled - checking reasoning quality...");
-            // TODO: Implement verification when ICriticModel concrete implementation is available
-            // For now, mark all steps as verified with high confidence
-            foreach (var step in chain.Steps)
-            {
-                step.IsVerified = true;
-            }
+            throw new NotSupportedException(
+                "Chain-of-thought verification requires a concrete verifier. " +
+                "Disable ReasoningConfig.EnableVerification until verification is implemented.");
         }

As per coding guidelines, “Every PR must contain production-ready code” and both // TODO placeholders and “hardcoded values instead of proper logic” are blocking issues.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Reasoning/Strategies/ChainOfThoughtStrategy.cs` around lines 149 - 157,
The code in ChainOfThoughtStrategy currently fakes verification when
config.EnableVerification is true by marking all chain.Steps as verified; remove
this fake path and instead fail fast or call the real verifier: if
config.EnableVerification is true and no concrete ICriticModel/verifier is
configured, throw a clear NotImplementedException (or a specific
InvalidOperationException) with context via AppendTrace rather than setting
step.IsVerified, otherwise invoke the real verifier API (the ICriticModel
implementation) to set each step's IsVerified and confidence; ensure the branch
references config.EnableVerification, ChainOfThoughtStrategy, AppendTrace, and
chain.Steps so reviewers can locate and wire the proper verifier or enforce the
fail-fast behavior.

Source: Coding guidelines

---

201-216: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

The JSON example is malformed.

The literal ... inside reasoning_steps makes the sample invalid JSON, so the model is more likely to echo invalid syntax and force the regex fallback even when _useJsonFormat is enabled.

💡 Suggested fix

 Respond in JSON format:
 {{
   ""reasoning_steps"": [
     ""Step 1: [your first reasoning step with explanation]"",
-    ""Step 2: [your second reasoning step with explanation]"",
-    ...
+    ""Step 2: [your second reasoning step with explanation]""
   ],
   ""final_answer"": ""your final answer to the query""
 }}
+
+Add more items to `reasoning_steps` as needed, up to the configured maximum.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Reasoning/Strategies/ChainOfThoughtStrategy.cs` around lines 201 - 216,
The JSON example in ChainOfThoughtStrategy.cs is malformed because the literal
"..." inside the reasoning_steps array makes the sample invalid; update the
prompt template used by the method (search for the string starting with "Respond
in JSON format:" in ChainOfThoughtStrategy or the variable that contains this
template, and references to _useJsonFormat) to provide a valid JSON
example—remove the "..." and either include a fixed small valid example entry or
annotate with a JSON-safe placeholder (e.g., an explicit array with up to
{config.MaxSteps} numbered step examples) and ensure the MaxSteps placeholder is
rendered correctly so the template produces valid JSON when sent to the model.

src/Reasoning/Training/ReinforcementLearner.cs (2)

477-489: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Blocking: the checkpoint API does not actually checkpoint model state.

These methods are presented as save/load for resumable training, but they only persist counters/config/data-collector state. The injected model’s weights/state are never saved, so loading resumes bookkeeping against whatever model instance happens to be in memory.

💡 Suggested fix

+internal interface ICheckpointableModel
+{
+    Task<byte[]> SaveStateAsync(CancellationToken cancellationToken = default);
+    Task LoadStateAsync(byte[] state, CancellationToken cancellationToken = default);
+}
+
 internal class TrainingCheckpoint<T>
 {
@@
     public TrainingDataCollector<T>? TrainingData { get; set; }
+    public string? ModelStateBase64 { get; set; }
 }
@@
     public async Task SaveCheckpointAsync(string filePath, CancellationToken cancellationToken = default)
     {
+        if (_model is not ICheckpointableModel checkpointableModel)
+            throw new NotSupportedException("The configured model does not support checkpointing.");
+
         var checkpoint = new TrainingCheckpoint<T>
         {
@@
-            TrainingData = _dataCollector
+            TrainingData = _dataCollector,
+            ModelStateBase64 = Convert.ToBase64String(
+                await checkpointableModel.SaveStateAsync(cancellationToken))
         };
@@
     public async Task LoadCheckpointAsync(string filePath, CancellationToken cancellationToken = default)
     {
@@
+        if (_model is not ICheckpointableModel checkpointableModel || checkpoint.ModelStateBase64 == null)
+            throw new NotSupportedException("The checkpoint does not contain model state.");
+
+        await checkpointableModel.LoadStateAsync(
+            Convert.FromBase64String(checkpoint.ModelStateBase64),
+            cancellationToken);

As per coding guidelines, “Future enhancement” comments and incomplete features that only partially implement the advertised behavior are blocking issues.

Also applies to: 494-505, 524-556

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Reasoning/Training/ReinforcementLearner.cs` around lines 477 - 489, The
SaveCheckpointAsync currently only persists training counters/config/data
collector but omits the injected model weights, so make SaveCheckpointAsync
serialize the model state too (e.g., call the injected model's Save/Export
method or implement a GetWeights/Serialize method on the model interface) and
write that bytes/blob into the same checkpoint (or alongside with a clear
filename/version); update the corresponding LoadCheckpointAsync to restore those
serialized weights into the injected model (e.g., call model.Load/Import or
SetWeights) before restoring epoch/bestAccuracy/earlyStopping counters and
TrainingDataCollector state, and include versioning/validation so mismatched
model formats throw a clear error and respect the cancellationToken during I/O.

Source: Coding guidelines

---

247-285: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Blocking: remove Console.WriteLine from the library training path.

This library code writes progress directly to stdout during normal operation. That is a non-production pattern here and gives hosts no control over formatting, routing, or suppression. Use the existing events for progress, and push any human-readable output behind an injected logger abstraction.

💡 Suggested fix

-            Console.WriteLine($"\n=== Epoch {epoch + 1}/{_config.Epochs} ===");
@@
-                Console.WriteLine($"Validation Accuracy: {evalMetrics.Accuracy:P2}");
@@
-                    Console.WriteLine($"Early stopping at epoch {epoch + 1}");
@@
-            Console.WriteLine($"\n=== STaR Epoch {epoch + 1}/{_config.Epochs} ===");
@@
-            Console.WriteLine($"STaR Chains Collected: {trainingMetrics.ChainCount}");
-            Console.WriteLine($"Average Reward: {Convert.ToDouble(trainingMetrics.AverageReward):F3}");
@@
-                Console.WriteLine($"Validation Accuracy: {evalMetrics.Accuracy:P2}");
@@
-                Console.WriteLine($"  Batch {batchIdx + 1}/{batches.Count}");

As per coding guidelines, “Using Console.WriteLine for logging instead of proper logging abstractions” is a blocking non-production pattern.

Also applies to: 317-347, 424-425

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Reasoning/Training/ReinforcementLearner.cs` around lines 247 - 285,
Replace direct Console.WriteLine calls in the training loop with the library's
logging/event mechanism: remove Console.WriteLine calls in the epoch loop (the
lines around the Epoch header, validation accuracy, and early-stopping message)
and instead emit the existing progress event or call the injected logger (use
the same abstraction other parts of the class use). Specifically, in the method
containing TrainEpochAsync and EvaluateAsync, replace prints for the epoch
start, Validation Accuracy, and "Early stopping" with event invocations or
ILogger methods (use the same event name or logger instance already present on
the class), and ensure SaveCheckpointAsync and metrics updates remain unchanged;
run unit/integration tests to confirm no Console.WriteLine remains in the
indicated regions.

Source: Coding guidelines

src/Reasoning/Training/PolicyGradientTrainer.cs (1)

142-163: ⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy lift

Blocking: this “trainer” still cannot train an IChatClient<T>.

TrainBatchAsync computes rewards and losses, but it never uses _model or applies any policy update. After this migration the dependency is explicitly an inference client, so the class now advertises RL training while performing zero model-state changes.

💡 Suggested fix

-internal class PolicyGradientTrainer<T>
+internal interface ITrainableChatPolicy<T> : IChatClient<T>
+{
+    Task ApplyPolicyUpdateAsync(
+        IReadOnlyList<ReasoningChain<T>> chains,
+        Vector<T> advantages,
+        double learningRate,
+        CancellationToken cancellationToken = default);
+}
+
+internal class PolicyGradientTrainer<T>
 {
-    private readonly IChatClient<T> _model;
+    private readonly ITrainableChatPolicy<T> _model;
@@
-    public PolicyGradientTrainer(
-        IChatClient<T> model,
+    public PolicyGradientTrainer(
+        ITrainableChatPolicy<T> model,
         IRewardModel<T>? rewardModel = null,
@@
         var totalLoss = _numOps.Add(
             policyLoss,
             _numOps.Multiply(_numOps.FromDouble(_entropyCoefficient), entropy)
         );
         metrics.TotalLoss = totalLoss;
+
+        await _model.ApplyPolicyUpdateAsync(
+            chains,
+            advantages,
+            _learningRate,
+            cancellationToken);

As per coding guidelines, “All methods have complete, production-ready implementations” and simplified implementations that stop at metrics instead of doing the real work are blocking issues.

Also applies to: 219-245

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Reasoning/Training/PolicyGradientTrainer.cs` around lines 142 - 163,
TrainBatchAsync currently computes rewards and losses but never updates model
parameters; modify the trainer to perform actual policy updates by deriving
gradients from the computed loss and applying them to the model via the _model
field: (1) if IChatClient<T> already exposes parameter update APIs, call the
appropriate method (e.g., ApplyGradients/UpdateParameters) with gradients scaled
by _learningRate, including entropy regularization using _entropyCoefficient and
discounting with _discountFactor; (2) if IChatClient<T> is inference-only,
introduce/require a trainable interface (e.g., ITrainableChatClient<T> or
methods like GetParameters/SetParameters/ApplyGradients) and cast or accept that
interface in the constructor; (3) update the running baseline (_baseline) using
returns and _useBaseline to compute advantages before applying gradients; and
(4) ensure TrainBatchAsync updates model state synchronously/atomically and
handles _numOps for numeric ops when computing gradients and advantages so the
trainer actually mutates the model rather than only reporting metrics.

Source: Coding guidelines

src/Reasoning/Verification/CriticModel.cs (1)

182-188: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

The sample JSON in the prompt is malformed.

"feedback" runs directly into "strengths" here, so the example object is not valid JSON. That makes the model more likely to return invalid JSON and fall back to the text parser.

💡 Suggested fix

 Respond in JSON format:
 {{
   ""score"": 0.85,
-  ""feedback"": ""The reasoning chain...,""strengths"": [...],
+  ""feedback"": ""The reasoning chain..."",
+  ""strengths"": [...],
   ""weaknesses"": [...],
   ""suggestions"": [...]
 }}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Reasoning/Verification/CriticModel.cs` around lines 182 - 188, The sample
JSON in the prompt inside CriticModel is malformed because the "feedback" value
runs into the "strengths" key; update the prompt construction in the CriticModel
class (where the JSON example is assembled) so the example is valid JSON —
include the missing comma and ensure arrays for "strengths", "weaknesses", and
"suggestions" are properly bracketed and quoted and that the entire object is
well-formed; adjust the string literal or formatter used in the method that
builds the prompt to emit a syntactically correct JSON example (e.g.,
{"score":0.85,"feedback":"...","strengths":[],"weaknesses":[],"suggestions":[]})
so downstream parsers receive valid JSON.

src/Reasoning/Verification/OutcomeRewardModel.cs (1)

216-226: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

BLOCKING: Remove TODO comment and implement verification results or remove incomplete code section.

This TODO comment with commented-out code is not production-ready. The CalculateUnsupervisedRewardAsync method is incomplete—it promises a 4-component heuristic ("Verification: Did it pass any verifiers?") but only implements 3 components (completeness, confidence, coherence) and leaves the verification scoring as a placeholder.

Per the production readiness guidelines, TODO comments and incomplete implementations are blocking issues. Either:

1. Implement the verification results integration now, or
2. Remove the commented-out code and the TODO, and update the method documentation to accurately reflect that only 3 heuristics are used

Production-ready code should not ship with known gaps documented in TODO comments.

🔧 Suggested fix: Remove incomplete code section

         }
 
         // 3. Coherence: Are steps logically connected?
         if (chain.Steps.Count > 1)
         {
             bool hasCoherence = chain.Steps.All(s => !string.IsNullOrWhiteSpace(s.Content));
             if (hasCoherence)
             {
                 reward += 0.2;
             }
         }
 
-        // 4. Verification: Did it pass any verifiers?
-        // TODO: ReasoningChain<T> doesn't have VerificationResults property yet
-        // if (chain.VerificationResults?.Count > 0)
-        // {
-        //     double verificationScore = chain.VerificationResults
-        //         .Where(v => v.Passed)
-        //         .Select(v => Convert.ToDouble(v.Confidence))
-        //         .DefaultIfEmpty(0.0)
-        //         .Average();
-        //
-        //     reward += verificationScore * 0.1;
-        // }
-
         return _numOps.FromDouble(MathHelper.Clamp(reward, 0.0, 1.0));
     }

Also update the method's logic comments (lines 178-213) to remove references to the 4th component, or implement the verification results integration if ReasoningChain<T> now supports it.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Reasoning/Verification/OutcomeRewardModel.cs` around lines 216 - 226, The
CalculateUnsupervisedRewardAsync method currently contains a TODO and
commented-out verification logic; either remove the TODO and the entire
commented block and update the method summary/comments to state it uses only
three heuristics (completeness, confidence, coherence), or implement the
verification integration by reading ReasoningChain<T>.VerificationResults
(ensure the property exists), compute verificationScore as the average
Confidence for Passed results (default 0.0), multiply by the intended weight
(0.1) and add to reward, and remove the TODO/comment once implemented; reference
the CalculateUnsupervisedRewardAsync method and
ReasoningChain<T>.VerificationResults when making the change.

Source: Coding guidelines

src/Reasoning/Verification/SelfRefinementEngine.cs (1)

75-75: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Consider propagating cancellationToken to the chat client call.

The RefineStepAsync method accepts a CancellationToken parameter (line 60), but line 75 calls GenerateResponseAsync(prompt) without passing it. Propagating the token would allow refinement operations to be canceled during the LLM call.

♻️ Suggested enhancement

-        string refinedContent = await _chatModel.GenerateResponseAsync(prompt);
+        string refinedContent = await _chatModel.GenerateResponseAsync(prompt, cancellationToken);

Note: ProcessRewardModel.cs line 85 already follows this pattern.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Reasoning/Verification/SelfRefinementEngine.cs` at line 75,
RefineStepAsync is not propagating the received CancellationToken to the LLM
call; update the call to _chatModel.GenerateResponseAsync to pass the
CancellationToken parameter (the same token passed into RefineStepAsync) so the
LLM call can be canceled (match the pattern used in ProcessRewardModel.cs where
GenerateResponseAsync(..., cancellationToken) is used); ensure you use the
correct parameter name (cancellationToken) and overload/signature on
_chatModel.GenerateResponseAsync.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/Agentic/Models/Connectors/ChatClientBase.cs`:
- Around line 58-62: The ChatClientBase constructor currently overwrites a
caller-owned HttpClient.Timeout; change protected ChatClientBase(HttpClient?
httpClient) so it sets Timeout only when it creates the HttpClient internally
(i.e., when httpClient is null) and do not mutate the passed-in HttpClient; use
per-call cancellation tokens instead of changing caller-owned Timeout. Also
tighten IsRetryable(HttpRequestException ex): on frameworks lacking
HttpRequestException.StatusCode (net471) inspect ex.InnerException (e.g.,
WebException/HttpWebResponse) and return true only for network timeouts and HTTP
status codes 408, 429, or any 5xx; return false for other permanent failures.
Ensure you modify the ChatClientBase constructor, the HttpClient property usage,
and the IsRetryable(HttpRequestException) implementation accordingly.
- Around line 151-163: ChatClientBase.IsRetryable currently returns true for all
HttpRequestException on non-NET5 builds; update the error flow so
OpenAIChatClient and AnthropicChatClient preserve HTTP status codes (do not
swallow them into plain HttpRequestException) by introducing and throwing a
custom exception (e.g., HttpResponseException containing an HttpStatusCode
property) when a non-success response is received, then change
ChatClientBase.IsRetryable(HttpRequestException) to detect that custom exception
(or check ex for an embedded StatusCode) and return true only for genuine
network errors or when status is 429, 408, or >=500; update unit tests and call
sites to throw/handle the new exception type accordingly.

In `@src/Agentic/Models/Connectors/MeaiChatClient.cs`:
- Around line 172-173: The current cast of usage.InputTokenCount and
usage.OutputTokenCount to int (variables input and output) can overflow; change
these to use long (e.g., declare input and output as long and assign
usage.InputTokenCount ?? 0 and usage.OutputTokenCount ?? 0) or, if downstream
APIs require int, clamp values to int.MaxValue (or throw) instead of direct
casting; update any downstream uses in MeaiChatClient (references to input,
output, InputTokenCount, OutputTokenCount) accordingly to preserve correctness
and avoid overflow.

In `@src/Agentic/Models/Connectors/OpenAIChatClient.cs`:
- Around line 420-438: BuildResponseFormat currently returns null when
ResponseFormat == ChatResponseFormatKind.JsonSchema but ResponseJsonSchema is
null, allowing requests to proceed with no response_format; instead validate
this combination early and fail fast. In BuildResponseFormat (and where
BuildRequest is assembled) check if options.ResponseFormat ==
ChatResponseFormatKind.JsonSchema and options.ResponseJsonSchema is null, and
throw an ArgumentException or InvalidOperationException with a clear message;
update callers so they cannot omit response_format for JsonSchema and ensure the
error surfaces before sending the request (referencing BuildResponseFormat and
BuildRequest to locate changes).
- Around line 73-108: In GetResponseCoreAsync, stop fabricating an empty
assistant message when the OpenAI 200 payload is missing required fields:
validate that root["choices"] exists, that choices.FirstOrDefault() and its
"message" are non-null, and if they are missing throw a descriptive exception
instead of returning ChatResponse with TextContent(string.Empty) (affecting the
code that builds contents and the final return of new
ChatResponse(ChatMessage.Assistant(contents), ...)). In BuildResponseFormat,
ensure that when response format kind is JsonSchema you validate
options.ResponseJsonSchema is non-null and throw an error if it is missing
instead of returning null/omitting response_format; reference ResponseJsonSchema
and the enum ChatResponseFormatKind.JsonSchema so callers fail fast on
misconfiguration.

In `@src/Agentic/Models/ImageMediaTypeExtensions.cs`:
- Around line 34-55: Change the fallback assignment in TryParseMimeType so it
sets the out parameter to the enum default instead of a specific image type;
replace the current mediaType = ImageMediaType.Png (in the default branch of
TryParseMimeType) with mediaType = default(ImageMediaType) to avoid misleading
non-default values when the method returns false.

In `@src/Agentic/Models/ToolCallContent.cs`:
- Around line 30-36: The ToolCallContent constructor currently assigns
ArgumentsJson without verifying JSON; update the constructor in class
ToolCallContent (the ToolCallContent(string callId, string toolName, string?
argumentsJson = null) method) to validate non-empty argumentsJson by attempting
to parse it (e.g., with JObject.Parse or JsonDocument) and on parse failure
throw an ArgumentException (including the original JsonException as inner
exception), while preserving the existing behavior of defaulting null/whitespace
to "{}".

In `@src/Agentic/Tools/FunctionAgentTool.cs`:
- Around line 47-51: Wrap the invocation inside
FunctionAgentTool.InvokeCoreAsync in a try-catch: call _invoker(arguments,
cancellationToken) as before but catch Exception, and on error return
ToolInvocationResult.Error (including the exception message/stack or an
appropriate diagnostic string) instead of letting the exception propagate; keep
ConfigureAwait(false) and preserve cancellationToken handling so the behavior
matches DelegateAgentTool's exception-to-ToolInvocationResult.Error pattern.

In `@src/Agentic/Tools/StructuredOutputExtensions.cs`:
- Around line 30-34: The deserialization is too lenient: update
JsonSchemaGenerator.ForType to populate a "required" array for
non-nullable/required CLR members (e.g., for properties without nullable
annotation or marked required) and then change
JsonSettings.MissingMemberHandling from MissingMemberHandling.Ignore to
MissingMemberHandling.Error so
StructuredOutputExtensions.GetStructuredResponseAsync will fail when JSON is
missing required fields; also update GetStructuredResponseAsync to catch and
surface those deserialization errors (or rethrow with context) so missing fields
are reported clearly.

In `@src/AiDotNet.Generators/AgentToolSchemaGenerator.cs`:
- Around line 164-169: The generated binder in AgentToolSchemaGenerator.cs
currently uses default({pType}) when an argument is missing, losing declared
parameter defaults; implement a helper (e.g.,
GetParameterDefaultLiteral(IParameterSymbol p)) that returns the C# literal for
p.ExplicitDefaultValue when p.HasExplicitDefaultValue (handle null,
strings/chars with escaping and quoting, numeric/bool with invariant formatting,
enums by emitting named constant or casted value), and change the invoker
generation (the lines that build local/__p_{p.Name} and token/__t_{p.Name}) to
use that helper’s result instead of default({pType}); ensure the fallback
remains $"default({pType})" if no explicit default exists.
- Around line 271-283: IsRequired currently only treats System_Nullable_T as
nullable; update IsRequired to also consider nullable-reference annotations by
checking p.NullableAnnotation == NullableAnnotation.Annotated (in addition to
the existing INamedTypeSymbol nullable value-type check) so parameters declared
like string? or MyDto? are not marked required. In EmitTool's binder generation,
replace the unconditional fallback ": default({pType});" with logic that emits
the method-declared optional default when present: if
(p.HasExplicitDefaultValue) emit the literal for p.ExplicitDefaultValue
(preserving null/primitive/string values and any symbol-to-literal formatting
used elsewhere) else emit default({pType}); so runtime invocation honors
declared optional defaults.

In `@src/Reasoning/ReasoningStrategyBase.cs`:
- Around line 309-323: The current ExecuteToolAsync forces every tool to receive
a single {"input": string} property and lets InvokeAsync exceptions bubble up;
change ExecuteToolAsync (and use FindTool, _tools, InvokeAsync, AppendTrace) to
preserve schema-shaped arguments by attempting to parse the provided input
string as JSON (JToken.Parse) and, if it yields a JObject/JArray, pass that
token directly to InvokeAsync; otherwise fall back to wrapping the raw string
under ["input"]. Also wrap the call to tool.InvokeAsync in a try/catch, on
exception AppendTrace a clear "Tool '{toolName}' exception: {ex.Message}" and
return an error string instead of letting the exception escape so one bad tool
call doesn't fail the whole reasoning run.

---

Outside diff comments:
In `@src/Interfaces/IAiModelBuilder.cs`:
- Around line 1467-1479: The XML doc example for IAiModelBuilder is stale: it
shows chaining ConfigureAgentAssistance(...) before ConfigureReasoning(), but
ConfigureAgentAssistance no longer exists; update the example under
ConfigureReasoning to use the current initialization/configuration flow for
IAiModelBuilder (e.g., remove the ConfigureAgentAssistance call or replace it
with the new supported method used to enable agent-related settings), ensuring
the snippet compiles against AiModelBuilder<TModel,...> and references only
existing public APIs (e.g., ConfigureReasoning, the current agent/config setup
method) so the example is valid.

In `@src/Models/Results/AiModelResult.cs`:
- Around line 1441-1456: Update the public XML documentation for
EvaluateBenchmarksAsync to describe the new IChatClient<T> parameter (its
purpose, when to provide null, and how it affects evaluation), update method
summary/params/returns to reference BenchmarkingOptions, IChatClient<T>, and
BenchmarkReport, refresh any sample XML example calls to include a chatClient
argument or show how to call with null/default, and add a brief migration note
indicating this is a breaking change so downstream callers must supply an
IChatClient<T> or adjust their call sites; apply the same documentation updates
to the other affected overloads/locations referenced in the review (the ranges
noted) to keep the public API docs consistent.

In `@src/Reasoning/Components/ThoughtEvaluator.cs`:
- Around line 66-67: The GenerateResponseAsync calls dropped CancellationToken
propagation after the IChatClient<T> migration; update the calls in
ThoughtEvaluator (and the other reasoning component that calls
IChatClient<T>.GenerateResponseAsync) to pass the incoming CancellationToken
through to GenerateResponseAsync so cancellations abort the remote LLM call
(e.g., change string response = await _chatModel.GenerateResponseAsync(prompt);
to call the overload that accepts the token), and ensure you use the same token
parameter name (CancellationToken token) from the method signature so
cancellation flows end-to-end.
- Around line 120-124: The code reads root["score"] and calls
scoreToken.Value<double>() directly; validate the token before conversion to
avoid exceptions on non-numeric model output. In the block where scoreToken is
obtained (root["score"]), check scoreToken.Type (or use scoreToken.ToString())
and attempt a safe parse with double.TryParse(..., CultureInfo.InvariantCulture)
or only accept JTokenType.Float/Integer; if parsing fails, do not call
Value<double>() and instead fall back to the existing regex/default path; then
clamp the parsed double with MathHelper.Clamp as before. Ensure you reference
scoreToken, Value<double>(), MathHelper.Clamp and the fallback path in the
ThoughtEvaluator logic.

In `@src/Reasoning/Components/ThoughtGenerator.cs`:
- Around line 141-165: The current JSON parsing block in ThoughtGenerator (using
ExtractJsonFromResponse and JObject.Parse) only falls back to
ParseThoughtsFromLines on JsonException, which lets valid-but-unusable JSON
(e.g., missing "thoughts" key or "thoughts" not being a JArray) produce an empty
thoughts list silently; update the logic so that after parsing the JObject you
verify that root["thoughts"] is a non-empty JArray and contains string items,
and if not (null, wrong type, or yields no valid strings) call
ParseThoughtsFromLines(response, expectedCount) as the fallback; preserve
throwing/parsing errors, but ensure you do not return an empty list
silently—optionally emit a warning via the existing logging mechanism when
falling back.

In `@src/Reasoning/DomainSpecific/CodeReasoner.cs`:
- Around line 74-81: The public constructor for CodeReasoner now takes
IChatClient<T> and IEnumerable<IAgentTool> causing a source/binary break; update
the PR to include clear migration guidance and release-note text explaining the
API change and how consumers should migrate (e.g., replace legacy chat/agent
abstractions with IChatClient<T> and IAgentTool, update construction of
ChainOfThoughtStrategy<T>, TreeOfThoughtsStrategy<T>, CriticModel<T>, and
SelfRefinementEngine<T> usages), and add a short example snippet or checklist
for downstream maintainers to follow when upgrading their codebases.

In `@src/Reasoning/DomainSpecific/MathematicalReasoner.cs`:
- Around line 129-132: The code calls VerifyAndRefineAsync whenever
result.Success is true but doesn't ensure result.ReasoningChain is non-null,
which causes iteration over chain.Steps to throw; update the guard to require
both result.Success and result.ReasoningChain != null before calling
VerifyAndRefineAsync (and mirror the same null-check where similar verification
is triggered around the 146-156 block), or alternatively add a defensive null
check at the start of VerifyAndRefineAsync to return/skip if the incoming
ReasoningChain is null so iteration over chain.Steps is never attempted on a
null object.

In `@src/Reasoning/Strategies/ChainOfThoughtStrategy.cs`:
- Around line 149-157: The code in ChainOfThoughtStrategy currently fakes
verification when config.EnableVerification is true by marking all chain.Steps
as verified; remove this fake path and instead fail fast or call the real
verifier: if config.EnableVerification is true and no concrete
ICriticModel/verifier is configured, throw a clear NotImplementedException (or a
specific InvalidOperationException) with context via AppendTrace rather than
setting step.IsVerified, otherwise invoke the real verifier API (the
ICriticModel implementation) to set each step's IsVerified and confidence;
ensure the branch references config.EnableVerification, ChainOfThoughtStrategy,
AppendTrace, and chain.Steps so reviewers can locate and wire the proper
verifier or enforce the fail-fast behavior.
- Around line 201-216: The JSON example in ChainOfThoughtStrategy.cs is
malformed because the literal "..." inside the reasoning_steps array makes the
sample invalid; update the prompt template used by the method (search for the
string starting with "Respond in JSON format:" in ChainOfThoughtStrategy or the
variable that contains this template, and references to _useJsonFormat) to
provide a valid JSON example—remove the "..." and either include a fixed small
valid example entry or annotate with a JSON-safe placeholder (e.g., an explicit
array with up to {config.MaxSteps} numbered step examples) and ensure the
MaxSteps placeholder is rendered correctly so the template produces valid JSON
when sent to the model.

In `@src/Reasoning/Training/PolicyGradientTrainer.cs`:
- Around line 142-163: TrainBatchAsync currently computes rewards and losses but
never updates model parameters; modify the trainer to perform actual policy
updates by deriving gradients from the computed loss and applying them to the
model via the _model field: (1) if IChatClient<T> already exposes parameter
update APIs, call the appropriate method (e.g., ApplyGradients/UpdateParameters)
with gradients scaled by _learningRate, including entropy regularization using
_entropyCoefficient and discounting with _discountFactor; (2) if IChatClient<T>
is inference-only, introduce/require a trainable interface (e.g.,
ITrainableChatClient<T> or methods like
GetParameters/SetParameters/ApplyGradients) and cast or accept that interface in
the constructor; (3) update the running baseline (_baseline) using returns and
_useBaseline to compute advantages before applying gradients; and (4) ensure
TrainBatchAsync updates model state synchronously/atomically and handles _numOps
for numeric ops when computing gradients and advantages so the trainer actually
mutates the model rather than only reporting metrics.

In `@src/Reasoning/Training/ReinforcementLearner.cs`:
- Around line 477-489: The SaveCheckpointAsync currently only persists training
counters/config/data collector but omits the injected model weights, so make
SaveCheckpointAsync serialize the model state too (e.g., call the injected
model's Save/Export method or implement a GetWeights/Serialize method on the
model interface) and write that bytes/blob into the same checkpoint (or
alongside with a clear filename/version); update the corresponding
LoadCheckpointAsync to restore those serialized weights into the injected model
(e.g., call model.Load/Import or SetWeights) before restoring
epoch/bestAccuracy/earlyStopping counters and TrainingDataCollector state, and
include versioning/validation so mismatched model formats throw a clear error
and respect the cancellationToken during I/O.
- Around line 247-285: Replace direct Console.WriteLine calls in the training
loop with the library's logging/event mechanism: remove Console.WriteLine calls
in the epoch loop (the lines around the Epoch header, validation accuracy, and
early-stopping message) and instead emit the existing progress event or call the
injected logger (use the same abstraction other parts of the class use).
Specifically, in the method containing TrainEpochAsync and EvaluateAsync,
replace prints for the epoch start, Validation Accuracy, and "Early stopping"
with event invocations or ILogger methods (use the same event name or logger
instance already present on the class), and ensure SaveCheckpointAsync and
metrics updates remain unchanged; run unit/integration tests to confirm no
Console.WriteLine remains in the indicated regions.

In `@src/Reasoning/Verification/CriticModel.cs`:
- Around line 182-188: The sample JSON in the prompt inside CriticModel is
malformed because the "feedback" value runs into the "strengths" key; update the
prompt construction in the CriticModel class (where the JSON example is
assembled) so the example is valid JSON — include the missing comma and ensure
arrays for "strengths", "weaknesses", and "suggestions" are properly bracketed
and quoted and that the entire object is well-formed; adjust the string literal
or formatter used in the method that builds the prompt to emit a syntactically
correct JSON example (e.g.,
{"score":0.85,"feedback":"...","strengths":[],"weaknesses":[],"suggestions":[]})
so downstream parsers receive valid JSON.

In `@src/Reasoning/Verification/OutcomeRewardModel.cs`:
- Around line 216-226: The CalculateUnsupervisedRewardAsync method currently
contains a TODO and commented-out verification logic; either remove the TODO and
the entire commented block and update the method summary/comments to state it
uses only three heuristics (completeness, confidence, coherence), or implement
the verification integration by reading ReasoningChain<T>.VerificationResults
(ensure the property exists), compute verificationScore as the average
Confidence for Passed results (default 0.0), multiply by the intended weight
(0.1) and add to reward, and remove the TODO/comment once implemented; reference
the CalculateUnsupervisedRewardAsync method and
ReasoningChain<T>.VerificationResults when making the change.

In `@src/Reasoning/Verification/SelfRefinementEngine.cs`:
- Line 75: RefineStepAsync is not propagating the received CancellationToken to
the LLM call; update the call to _chatModel.GenerateResponseAsync to pass the
CancellationToken parameter (the same token passed into RefineStepAsync) so the
LLM call can be canceled (match the pattern used in ProcessRewardModel.cs where
GenerateResponseAsync(..., cancellationToken) is used); ensure you use the
correct parameter name (cancellationToken) and overload/signature on
_chatModel.GenerateResponseAsync.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/Agentic/Models/Connectors/AnthropicChatClient.cs (1)

107-110: 🧹 Nitpick | 🔵 Trivial | 💤 Low value

Consider throwing on empty response content for consistency with OpenAI connector.

OpenAIChatClient throws InvalidOperationException when the response contains neither text content nor tool calls (lines 113-117 in that file), treating it as a malformed response or API contract violation. Here, the Anthropic connector silently returns an assistant message with empty text, which could hide API contract changes or malformed responses from the caller.

This inconsistency may surprise users who expect uniform error behavior across connectors.

♻️ Suggested change for stricter validation

         if (contents.Count == 0)
         {
-            contents.Add(new TextContent(string.Empty));
+            throw new InvalidOperationException(
+                $"Anthropic response from '{ModelId}' contained no content blocks.");
         }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Agentic/Models/Connectors/AnthropicChatClient.cs` around lines 107 - 110,
The Anthropic connector currently silently substitutes an empty TextContent when
the parsed response has no contents; change this to throw an
InvalidOperationException to match OpenAIChatClient behavior. In
AnthropicChatClient (the method that builds/parses the assistant response where
the local variable contents is populated), remove the contents.Add(new
TextContent(string.Empty)) fallback and instead throw new
InvalidOperationException with a clear message indicating the response contained
neither text content nor tool calls (mirroring OpenAIChatClient's validation).
Ensure the exception message references the Anthropic response being malformed
so callers can detect API contract violations consistently across connectors.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/Agentic/Models/Connectors/AnthropicChatClient.cs`:
- Around line 107-110: The Anthropic connector currently silently substitutes an
empty TextContent when the parsed response has no contents; change this to throw
an InvalidOperationException to match OpenAIChatClient behavior. In
AnthropicChatClient (the method that builds/parses the assistant response where
the local variable contents is populated), remove the contents.Add(new
TextContent(string.Empty)) fallback and instead throw new
InvalidOperationException with a clear message indicating the response contained
neither text content nor tool calls (mirroring OpenAIChatClient's validation).
Ensure the exception message references the Anthropic response being malformed
so callers can detect API contract violations consistently across connectors.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3edebcce-06c6-4b4c-9f19-784a50ee6568

📥 Commits

Reviewing files that changed from the base of the PR and between bc4fa02 and 03a68eb.

📒 Files selected for processing (5)

• src/Agentic/Models/Connectors/AnthropicChatClient.cs
• src/Agentic/Models/Connectors/ChatClientBase.cs
• src/Agentic/Models/Connectors/HttpResponseException.cs
• src/Agentic/Models/Connectors/MeaiChatClient.cs
• src/Agentic/Models/Connectors/OpenAIChatClient.cs