diff --git a/.github/workflows/dotnet-build-and-test.yml b/.github/workflows/dotnet-build-and-test.yml index fcfe26fc0e..0b589e2c55 100644 --- a/.github/workflows/dotnet-build-and-test.yml +++ b/.github/workflows/dotnet-build-and-test.yml @@ -273,6 +273,8 @@ jobs: -c ${{ matrix.configuration }} ` --no-build -v Normal ` --report-xunit-trx ` + --report-junit ` + --results-directory ../IntegrationTestResults/ ` --ignore-exit-code 8 ` --filter-not-trait "Category=IntegrationDisabled" ` --filter-not-trait "Category=FoundryHostedAgents" ` @@ -294,6 +296,10 @@ jobs: AZURE_AI_PROJECT_ENDPOINT: ${{ vars.AZURE_AI_PROJECT_ENDPOINT }} AZURE_AI_MODEL_DEPLOYMENT_NAME: ${{ vars.AZURE_AI_MODEL_DEPLOYMENT_NAME }} AZURE_AI_BING_CONNECTION_ID: ${{ vars.AZURE_AI_BING_CONNECTION_ID }} + # Anthropic Models + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + ANTHROPIC_CHAT_MODEL_NAME: ${{ vars.ANTHROPIC_CHAT_MODEL_NAME }} + ANTHROPIC_REASONING_MODEL_NAME: ${{ vars.ANTHROPIC_REASONING_MODEL_NAME }} # Generate test reports and check coverage - name: Generate test reports @@ -316,6 +322,14 @@ jobs: shell: pwsh run: ./dotnet/eng/scripts/dotnet-check-coverage.ps1 -JsonReportPath "TestResults/Reports/Summary.json" -CoverageThreshold $env:COVERAGE_THRESHOLD + - name: Upload integration test results + if: always() && github.event_name != 'pull_request' && matrix.integration-tests + uses: actions/upload-artifact@v7 + with: + name: dotnet-test-results-${{ matrix.targetFramework }}-${{ matrix.os }} + path: IntegrationTestResults/**/*.junit + if-no-files-found: ignore + # The Foundry hosted-agent IT is costly (it builds a container, pushes to ACR, and provisions # live agents on a separate Foundry project). Running it in its own job keeps the overall # workflow time roughly flat: it executes in parallel to dotnet-build and dotnet-test and is @@ -379,6 +393,14 @@ jobs: # We rebuild and push the test container image on every IT run so framework code changes # are picked up; the image tag is content-hashed across the test container source AND its # framework project references, so identical content is a no-op push. + # + # `-UsePrebuiltProjectReferences` opts into the no-rebuild fast path: publish skips + # rebuilding ProjectReferences and consumes the DLLs the prior "Build Foundry hosted IT + # (and its deps)" step already produced. This avoids MSB3026 ("file is being used by + # another process") collisions caused by the previous build's shared-compilation server + # still holding file handles to those DLLs. Safe in CI because the prebuild step ran in + # the same job against the same source. Do not remove the prebuild step (the subsequent + # `dotnet test --no-build` step depends on it too). - name: Build and push Foundry Hosted Agents test container id: build-foundry-hosted-image shell: pwsh @@ -388,7 +410,7 @@ jobs: if ([string]::IsNullOrWhiteSpace($registry)) { throw "IT_HOSTED_AGENT_REGISTRY not set in the integration environment." } - & "${{ github.workspace }}/dotnet/tests/Foundry.Hosting.IntegrationTests/scripts/it-build-image.ps1" -Registry $registry | Tee-Object -FilePath $env:GITHUB_ENV -Append + & "${{ github.workspace }}/dotnet/tests/Foundry.Hosting.IntegrationTests/scripts/it-build-image.ps1" -Registry $registry -UsePrebuiltProjectReferences | Tee-Object -FilePath $env:GITHUB_ENV -Append - name: Run Foundry Hosted Agents Integration Tests shell: pwsh @@ -448,3 +470,64 @@ jobs: uses: actions/github-script@v8 with: script: core.setFailed('Integration Tests Cancelled!') + + # Integration test trend report (aggregates JUnit XML results from dotnet test jobs) + dotnet-integration-test-report: + name: Integration Test Report + if: > + always() && + github.event_name != 'pull_request' && + (contains(join(needs.*.result, ','), 'success') || + contains(join(needs.*.result, ','), 'failure')) + needs: [dotnet-test] + runs-on: ubuntu-latest + defaults: + run: + working-directory: python + steps: + - uses: actions/checkout@v6 + with: + persist-credentials: false + sparse-checkout: | + .github/actions/python-setup + python + - name: Set up python and install the project + uses: ./.github/actions/python-setup + with: + python-version: "3.13" + os: ${{ runner.os }} + - name: Download all test results from current run + uses: actions/download-artifact@v4 + with: + pattern: dotnet-test-results-* + path: dotnet-test-results/ + - name: Restore report history cache + uses: actions/cache/restore@v4 + with: + path: python/dotnet-integration-report-history.json + key: dotnet-integration-report-history-${{ github.run_id }} + restore-keys: | + dotnet-integration-report-history- + - name: Generate trend report + run: > + uv run python scripts/integration_test_report/aggregate.py + ../dotnet-test-results/ + dotnet-integration-report-history.json + dotnet-integration-test-report.md + - name: Post to Job Summary + if: always() + run: cat dotnet-integration-test-report.md >> $GITHUB_STEP_SUMMARY + - name: Save report history cache + if: always() + uses: actions/cache/save@v4 + with: + path: python/dotnet-integration-report-history.json + key: dotnet-integration-report-history-${{ github.run_id }} + - name: Upload trend report + if: always() + uses: actions/upload-artifact@v7 + with: + name: dotnet-integration-test-report + path: | + python/dotnet-integration-test-report.md + python/dotnet-integration-report-history.json diff --git a/dotnet/README.md b/dotnet/README.md index 328dfdf684..2edb402a94 100644 --- a/dotnet/README.md +++ b/dotnet/README.md @@ -33,3 +33,4 @@ Console.WriteLine(await agent.RunAsync("Write a haiku about Microsoft Agent Fram - [Design Documents](../docs/design) - [Architectural Decision Records](../docs/decisions) - [MSFT Learn Docs](https://learn.microsoft.com/agent-framework/overview/agent-framework-overview) + diff --git a/dotnet/samples/01-get-started/04_memory/Program.cs b/dotnet/samples/01-get-started/04_memory/Program.cs index ebafc60b45..961066682a 100644 --- a/dotnet/samples/01-get-started/04_memory/Program.cs +++ b/dotnet/samples/01-get-started/04_memory/Program.cs @@ -50,12 +50,12 @@ Console.WriteLine(await agent.RunAsync("My name is Ruaidhrí", session)); Console.WriteLine(await agent.RunAsync("I am 20 years old", session)); // We can serialize the session. The serialized state will include the state of the memory component. -JsonElement sesionElement = await agent.SerializeSessionAsync(session); +JsonElement sessionElement = await agent.SerializeSessionAsync(session); Console.WriteLine("\n>> Use deserialized session with previously created memories\n"); // Later we can deserialize the session and continue the conversation with the previous memory component state. -var deserializedSession = await agent.DeserializeSessionAsync(sesionElement); +var deserializedSession = await agent.DeserializeSessionAsync(sessionElement); Console.WriteLine(await agent.RunAsync("What is my name and age?", deserializedSession)); Console.WriteLine("\n>> Read memories using memory component\n"); diff --git a/dotnet/src/Microsoft.Agents.AI.Hosting.OpenAI/Responses/Streaming/SequenceNumber.cs b/dotnet/src/Microsoft.Agents.AI.Hosting.OpenAI/Responses/Streaming/SequenceNumber.cs index d119275f71..e125c4269e 100644 --- a/dotnet/src/Microsoft.Agents.AI.Hosting.OpenAI/Responses/Streaming/SequenceNumber.cs +++ b/dotnet/src/Microsoft.Agents.AI.Hosting.OpenAI/Responses/Streaming/SequenceNumber.cs @@ -13,5 +13,5 @@ internal sealed class SequenceNumber /// Gets the next sequence number. /// /// The next sequence number. - public int Increment() => this._sequenceNumber++; + public int Increment() => System.Threading.Interlocked.Increment(ref this._sequenceNumber) - 1; } diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/ExecutorEvent.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/ExecutorEvent.cs index a0d4dd73b4..3d590ea571 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows/ExecutorEvent.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/ExecutorEvent.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. using System.Text.Json.Serialization; +using Microsoft.Agents.AI.Workflows.Specialized.Magentic; namespace Microsoft.Agents.AI.Workflows; @@ -10,6 +11,7 @@ namespace Microsoft.Agents.AI.Workflows; [JsonDerivedType(typeof(ExecutorInvokedEvent))] [JsonDerivedType(typeof(ExecutorCompletedEvent))] [JsonDerivedType(typeof(ExecutorFailedEvent))] +[JsonDerivedType(typeof(MagenticOrchestratorEvent))] public class ExecutorEvent(string executorId, object? data) : WorkflowEvent(data) { /// diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/MagenticPlanReviewRequest.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/MagenticPlanReviewRequest.cs new file mode 100644 index 0000000000..fd7b82afe6 --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/MagenticPlanReviewRequest.cs @@ -0,0 +1,44 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System.Collections.Generic; +using System.Linq; +using Microsoft.Extensions.AI; + +namespace Microsoft.Agents.AI.Workflows; + +/// +/// Request for human review of a proposed plan. +/// +/// The proposed plan. +/// The current progress ledger, if available. During the initial plan review, +/// this will be . In subsequent reviews after replanning (due to stalls), this will +/// contain the latest progress ledger that determined that no progress has been made or the workflow was in +/// a loop. +/// Whether the workflow is currently stalled. +public record MagenticPlanReviewRequest(ChatMessage Plan, MagenticProgressLedger? CurrentProgress, bool IsStalled) +{ + /// + /// Create an approving . + /// + /// + public MagenticPlanReviewResponse Approve() => new([]); + + /// + /// Create a with revisions. + /// + /// + public MagenticPlanReviewResponse Revise(string message) => new([new(ChatRole.User, message)]); + + /// + /// Create a with revisions. + /// + /// + public MagenticPlanReviewResponse Revise(ChatMessage message) => new([message]); + + /// + /// Create a with revisions. + /// + /// + public MagenticPlanReviewResponse Revise(IEnumerable messages) + => new(messages is List messageList ? messageList : messages.ToList()); +} diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/MagenticPlanReviewResponse.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/MagenticPlanReviewResponse.cs new file mode 100644 index 0000000000..0a72ccfa0f --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/MagenticPlanReviewResponse.cs @@ -0,0 +1,18 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System.Collections.Generic; +using Microsoft.Extensions.AI; + +namespace Microsoft.Agents.AI.Workflows; + +/// +/// Review feedback for a proposed plan, including any revisions if the plan is not approved as-is. An +/// empty list of review messages indicates approval of the proposed plan without any revisions. +/// +/// +/// Review feedback for a generated plan. Empty if the plan is approved as-is and changes are requested. +/// +public record MagenticPlanReviewResponse(List Review) +{ + internal bool IsApproved => this.Review.Count == 0; +} diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/MagenticProgressLedger.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/MagenticProgressLedger.cs new file mode 100644 index 0000000000..445007d3f3 --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/MagenticProgressLedger.cs @@ -0,0 +1,269 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using System.Linq; +using System.Text; +using System.Text.Json; +using System.Text.Json.Serialization; +using System.Text.Json.Serialization.Metadata; + +namespace Microsoft.Agents.AI.Workflows; + +/// +/// Maintains a ledger of progress made by the Magentic workflow. +/// +public class MagenticProgressLedger +{ + internal static readonly BooleanProgressLedgerSlot IsRequestSatisfiedSlot = new("is_request_satisfied", + "Is the request fully satisfied? (True if complete, or False if the original request has yet to be SUCCESSFULLY and FULLY addressed)"); + + internal static readonly BooleanProgressLedgerSlot IsInLoopSlot = new("is_in_loop", + "Are we in a loop where we are repeating the same requests and or getting the same responses as before? " + + "Loops can span multiple turns, and can include repeated actions like scrolling up or down more than a handful of times."); + + internal static readonly BooleanProgressLedgerSlot IsProgressBeingMadeSlot = new("is_progress_being_made", + "Are we making forward progress? (True if just starting, or recent messages are adding value. False if recent " + + "messages show evidence of being stuck in a loop or if there is evidence of significant barriers to success " + + "such as the inability to read from a required file)"); + + internal readonly StringProgressLedgerSlot NextSpeakerSlot; + + internal static readonly StringProgressLedgerSlot InstructionOrQuestionSlot = new("instruction_or_question", + "What instruction or question would you give this team member? (Phrase as if speaking directly to them, and " + + "include any specific information they may need)"); + + internal MagenticProgressLedger(string teamNames, IEnumerable additionalQuestions, JsonElement? state = null) + { + this.NextSpeakerSlot = new("next_speaker", $"Who should speak next? (select from: {teamNames})"); + this.AdditionalQuestions = additionalQuestions as ProgressLedgerSlot[] ?? additionalQuestions.ToArray(); + + if (state != null) + { + this.TryUpdateState(state.Value); + } + } + + internal ProgressLedgerSlot[] AdditionalQuestions { get; } + + internal bool TryUpdateState(JsonElement element) + { + // In principle all of these should be inlineable, but the CodeAnalysis fails to properly chain through the and-chain to realize that + // all must be true for `requiredQuestionsAnswered` to be true, meaning all of the out parameters would be initialized properly. + bool isInLoop = false; + bool isProgressBeingMade = false; + string? nextSpeaker = string.Empty; + string? instructionOrQuestion = string.Empty; + + bool requiredQuestionsAnswered = + IsRequestSatisfiedSlot.TryGetValueFrom(element, out bool isRequestSatisfied) && + IsInLoopSlot.TryGetValueFrom(element, out isInLoop) && + IsProgressBeingMadeSlot.TryGetValueFrom(element, out isProgressBeingMade) && + this.NextSpeakerSlot.TryGetValueFrom(element, out nextSpeaker) && + InstructionOrQuestionSlot.TryGetValueFrom(element, out instructionOrQuestion); + + if (requiredQuestionsAnswered) + { + this.State = element; + + this.IsRequestSatisfied = isRequestSatisfied; + this.IsInLoop = isInLoop; + this.IsProgressBeingMade = isProgressBeingMade; + + this.NextSpeaker = nextSpeaker!; + this.InstructionOrQuestion = instructionOrQuestion!; + } + + // TODO: To what extent do we want to enforce that the additional questions are also answered? + + return requiredQuestionsAnswered; + } + + [JsonInclude] + internal JsonElement? State; + + /// + /// Specifies whether plan execution has started. + /// + [JsonIgnore] + public bool IsStarted => this.State != null; + + /// + /// Specifies whether the task has been fully satisfied. + /// + [JsonIgnore] + public bool IsRequestSatisfied { get; private set; } + + /// + /// Specifies whether the team is in a loop. + /// + [JsonIgnore] + public bool IsInLoop { get; private set; } + + /// + /// Specifies whether the team is making progress on the task. + /// + [JsonIgnore] + public bool IsProgressBeingMade { get; private set; } + + /// + /// Gets the next team member to take a turn. + /// + [JsonIgnore] + public string NextSpeaker { get; private set; } = string.Empty; + + /// + /// Gets the instruction or question to send to the next team member. + /// + [JsonIgnore] + public string InstructionOrQuestion { get; private set; } = string.Empty; + + [JsonIgnore] + internal IEnumerable Slots => + [ + IsRequestSatisfiedSlot, + IsInLoopSlot, + IsProgressBeingMadeSlot, + this.NextSpeakerSlot, + InstructionOrQuestionSlot, + .. this.AdditionalQuestions + ]; + + internal bool TryGetCurrentSlotValue(ProgressLedgerSlot slot, [NotNullWhen(true)] out T? value) + { + if (!this.State.HasValue) + { + value = default; + return false; + } + + return slot.TryGetValueFrom(this.State.Value, out value); + } + + private (string QuestionBlock, string AnswerSchema)? _questionFormatCache; + internal (string QuestionBlock, string AnswerSchema) FormatQuestions() + { + if (!this._questionFormatCache.HasValue) + { + StringBuilder questionBuilder = new(), schemaBuilder = new(); + + schemaBuilder.AppendLine("{"); + foreach (ProgressLedgerSlot slot in this.Slots) + { + questionBuilder.AppendLine(slot.FormattedQuestion); + + schemaBuilder.AppendLine($"\"{slot.Key}\": {{") + .AppendLine($" \"{ProgressLedgerSlot.ValueKey}\": {slot.SchemaType}{slot.SuffixString},") + .AppendLine($" \"{ProgressLedgerSlot.ReasonKey}\": string") + .AppendLine("}"); + } + schemaBuilder.AppendLine("}"); + + this._questionFormatCache = (questionBuilder.ToString(), schemaBuilder.ToString()); + } + + return this._questionFormatCache.Value; + } +} + +internal abstract record ProgressLedgerSlot(string Key, string Question, string? SchemaTypeSuffix = null) +{ + public const string ValueKey = "answer"; + public const string ReasonKey = "reason"; + + internal string SuffixString => this.SchemaTypeSuffix == null ? string.Empty : $"({this.SchemaTypeSuffix})"; + + protected internal abstract string SchemaType { get; } + + public string FormattedQuestion + { + get + { + if (field == null) + { + IEnumerable questionLines = this.Question.Split(['\r', '\n'], StringSplitOptions.RemoveEmptyEntries) + .Select(line => line.TrimEnd()); + + field = $" - {string.Join("\n ", questionLines)}"; + } + + return field; + } + } +} + +internal abstract record ProgressLedgerSlot(string Key, string Question, string? SchemaTypeSuffix = null, JsonSerializerOptions? SerializerOptions = null) + : ProgressLedgerSlot(Key, Question, SchemaTypeSuffix) +{ + protected internal virtual JsonTypeInfo GetJsonTypeInfo() => + ((this.SerializerOptions ?? WorkflowsJsonUtilities.DefaultOptions).TryGetTypeInfo(typeof(T), out JsonTypeInfo? typeInfo) + ? typeInfo as JsonTypeInfo : null) + ?? throw new InvalidOperationException($"Cannot get TypeInfo for {typeof(T)} from {(this.SerializerOptions == null ? "provided" : "default")} SerializationOptions."); + + public bool TryGetValueFrom(JsonElement answers, [NotNullWhen(true)] out T? value) + { + if (answers.TryGetProperty(this.Key, out JsonElement slotElement) && + slotElement.ValueKind != JsonValueKind.Null && + slotElement.TryGetProperty(ValueKey, out JsonElement answerValue)) + { + try + { + T? result = answerValue.Deserialize(this.GetJsonTypeInfo()); + if (result != null) + { + value = result; + return true; + } + } + catch + { + } + } + + value = default; + return false; + } + + public bool TryGetReasonFrom(JsonElement answers, [NotNullWhen(true)] out string? value) + { + if (answers.TryGetProperty(this.Key, out JsonElement slotElement) && + slotElement.ValueKind != JsonValueKind.Null && + slotElement.TryGetProperty(ReasonKey, out JsonElement reasonValue)) + { + try + { + string? result = reasonValue.Deserialize(WorkflowsJsonUtilities.JsonContext.Default.String); + if (result != null) + { + value = result; + return true; + } + } + catch + { + } + } + + value = default; + return false; + } +} + +internal sealed record BooleanProgressLedgerSlot(string Key, string Question, string? SchemaTypeSuffix = null) : ProgressLedgerSlot(Key, Question, SchemaTypeSuffix) +{ + // Since we know the type statically, we can directly return the JsonTypeInfo for string from our JsonContext, + // which is more efficient than looking it up via the options. + protected internal override JsonTypeInfo GetJsonTypeInfo() => WorkflowsJsonUtilities.JsonContext.Default.Boolean; + + protected internal override string SchemaType => "boolean"; +} + +internal sealed record StringProgressLedgerSlot(string Key, string Question, string? SchemaTypeSuffix = null) : ProgressLedgerSlot(Key, Question, SchemaTypeSuffix) +{ + // Since we know the type statically, we can directly return the JsonTypeInfo for string from our JsonContext, + // which is more efficient than looking it up via the options. + protected internal override JsonTypeInfo GetJsonTypeInfo() => WorkflowsJsonUtilities.JsonContext.Default.String; + + protected internal override string SchemaType => "string"; +} diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/MagenticWorkflowBuilder.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/MagenticWorkflowBuilder.cs new file mode 100644 index 0000000000..f5b8091dc2 --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/MagenticWorkflowBuilder.cs @@ -0,0 +1,158 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System.Collections.Generic; +using System.Threading.Tasks; +using Microsoft.Agents.AI.Workflows.Specialized.Magentic; + +using ExecutorFactoryFunc = System.Func, + string, + System.Threading.Tasks.ValueTask>; + +namespace Microsoft.Agents.AI.Workflows; + +/// +/// Fluent builder for creating Magentic One multi-agent orchestration workflows. +/// +/// Magentic One workflows use an LLM-powered manager to coordinate multiple agents through dynamic task planning, progress tracking, +/// and adaptive replanning.The manager creates plans, selects agents, monitors progress, and determines when to replan or complete. +/// +/// The builder provides a fluent API for configuring participants, the manager, optional plan review, checkpointing, and event +/// callbacks. +/// +/// Human-in-the-loop Support: Magentic provides specialized HITL mechanisms via: +/// - `RequirePlanSignoff` - Review and approve/revise plans before execution +/// - Tool approval via `function_approval_request`: Approve individual tool calls on participating agents. Note that tool calls are +/// not supported on the ManagerAgent. +/// +/// +public class MagenticWorkflowBuilder(AIAgent managerAgent) +{ + private readonly List _team = new(); + private string? _name; + private string? _description; + private int _maxStalls = TaskLimits.DefaultMaxStallCount; + private int? _maxRounds; + private int? _maxResets; + private bool _requirePlanSignoff = true; + + /// + public MagenticWorkflowBuilder AddParticipants(params IEnumerable agents) + { + this._team.AddRange(agents); + return this; + } + + /// + public MagenticWorkflowBuilder WithName(string name) + { + this._name = name; + return this; + } + + /// + public MagenticWorkflowBuilder WithDescription(string description) + { + this._description = description; + return this; + } + + /// + /// Set the maximum number of coordination rounds. means unlimited. + /// + /// + public MagenticWorkflowBuilder WithMaxRounds(int? maxRounds = null) + { + this._maxRounds = maxRounds; + return this; + } + + /// + /// Set the maximum number ofnumber of resets allowed. means unlimited. + /// + /// + public MagenticWorkflowBuilder WithMaxResets(int? maxResets = null) + { + this._maxResets = maxResets; + return this; + } + + /// + /// Set the maximum number of consecutive rounds without progress before replan (default 3). + /// + /// + public MagenticWorkflowBuilder WithMaxStalls(int maxStalls = TaskLimits.DefaultMaxStallCount) + { + this._maxStalls = maxStalls; + return this; + } + + /// + /// If , requires human approval of the initial plan or any updates before proceeding. True by default. + /// + /// + /// + public MagenticWorkflowBuilder RequirePlanSignoff(bool requirePlanSignoff = true) + { + this._requirePlanSignoff = requirePlanSignoff; + return this; + } + + private WorkflowBuilder ReduceToWorkflowBuilder() + { + // Create a copy of the team so that improper modifications by using the builder after .Build() do not affect the + // workflow in unexpected ways. + List team = [.. this._team]; + + ExecutorBinding orchestrator = CreateOrchestratorBinding(managerAgent, team, this.Limits, this._requirePlanSignoff); + WorkflowBuilder result = new(orchestrator); + + AIAgentHostOptions options = new() + { + ReassignOtherAgentsAsUsers = true, + ForwardIncomingMessages = false + }; + + List teamBindings = []; + foreach (AIAgent agent in team) + { + ExecutorBinding binding = agent.BindAsExecutor(options); + teamBindings.Add(binding); + + result.AddEdge(binding, orchestrator); + } + + result.AddFanOutEdge(orchestrator, teamBindings) + .WithOutputFrom(orchestrator); + + if (!string.IsNullOrWhiteSpace(this._name)) + { + result.WithName(this._name); + } + + if (!string.IsNullOrWhiteSpace(this._description)) + { + result.WithDescription(this._description); + } + + return result; + } + + /// + public Workflow Build() => this.ReduceToWorkflowBuilder().Build(); + + private TaskLimits Limits => new( + MaxRoundCount: this._maxRounds, + MaxResetCount: this._maxResets, + MaxStallCount: this._maxStalls); + + private static ExecutorBinding CreateOrchestratorBinding(AIAgent managerAgent, List team, TaskLimits limits, bool requirePlanSignoff) + { + ExecutorFactoryFunc factory = CreateOrchestratorAsync; + return factory.BindExecutor(nameof(MagenticOrchestrator)); + + ValueTask CreateOrchestratorAsync(ExecutorConfig options, string sessionId) + { + return new(new MagenticOrchestrator(managerAgent, team, limits, requirePlanSignoff)); + } + } +} diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/ResetChatSignal.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/ResetChatSignal.cs new file mode 100644 index 0000000000..c8013ded8f --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/ResetChatSignal.cs @@ -0,0 +1,9 @@ +// Copyright (c) Microsoft. All rights reserved. + +namespace Microsoft.Agents.AI.Workflows; + +/// +/// Notifies an AIAgent-hosting executor that it should reset its conversation state, and start a new session, if appropriate. +/// Note that for Agent Orchestrations, only Magentic makes use of this functionality. +/// +public sealed record ResetChatSignal(); diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/AIAgentHostExecutor.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/AIAgentHostExecutor.cs index 885dbc4f57..cd20fc4336 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/AIAgentHostExecutor.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/AIAgentHostExecutor.cs @@ -24,7 +24,7 @@ internal static class TurnExtensions => handoffState.TurnToken.ShouldEmitStreamingEvents(agentSetting); } -internal sealed class AIAgentHostExecutor : ChatProtocolExecutor +internal class AIAgentHostExecutor : ChatProtocolExecutor { private readonly AIAgent _agent; private readonly AIAgentHostOptions _options; @@ -40,7 +40,9 @@ internal sealed class AIAgentHostExecutor : ChatProtocolExecutor StringMessageChatRole = ChatRole.User }; - public AIAgentHostExecutor(AIAgent agent, AIAgentHostOptions options) : base(id: agent.GetDescriptiveId(), + public static string IdFor(AIAgent agent) => agent.GetDescriptiveId(); + + public AIAgentHostExecutor(AIAgent agent, AIAgentHostOptions options) : base(id: IdFor(agent), s_defaultChatProtocolOptions, declareCrossRunShareable: false) // Explicitly false, because we maintain turn state on the instance { @@ -67,7 +69,14 @@ internal sealed class AIAgentHostExecutor : ChatProtocolExecutor protected override ProtocolBuilder ConfigureProtocol(ProtocolBuilder protocolBuilder) { - return this.ConfigureUserInputHandling(base.ConfigureProtocol(protocolBuilder)); + return this.ConfigureUserInputHandling(base.ConfigureProtocol(protocolBuilder)) + .ConfigureRoutes(routeBuilder => routeBuilder.AddHandler(this.ResetChat)); + } + + internal void ResetChat(ResetChatSignal signal, IWorkflowContext context) + { + this._session = null; + this._currentTurnEmitEvents = null; } private ValueTask HandleUserInputResponseAsync( diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/ChatMessageExtensions.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/ChatMessageExtensions.cs new file mode 100644 index 0000000000..6230711077 --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/ChatMessageExtensions.cs @@ -0,0 +1,175 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; +using System.Text; +using System.Text.Json; +using System.Text.RegularExpressions; +using Microsoft.Extensions.AI; + +namespace Microsoft.Agents.AI.Workflows.Specialized.Magentic; + +internal static partial class ChatMessageExtensions +{ + private static void ProcessAIContents(StringBuilder resultBuilder, IEnumerable contents, StreamingToolCallResultPairMatcher? pairMatcher = null) + { + pairMatcher ??= new(); + + foreach (AIContent content in contents) + { + switch (content) + { + case TextContent textContent: + resultBuilder.AppendLine(textContent.Text); + break; + + //case DataContent dataContent: + // // We really do not know how to deal with anything other than image data with descriptions, which is not + // // a well-defined concept in MEAI (as contrasted with AutoGen's ImageContent type) + // break; + + case ErrorContent errorContent: + resultBuilder.AppendLine($"[ERROR{(errorContent.ErrorCode != null ? $"(Code={errorContent.ErrorCode})" : string.Empty)}]"); + resultBuilder.AppendLine(errorContent.Message); + + if (errorContent.Details != null) + { + resultBuilder.Append("Details:").AppendLine(errorContent.Details); + } + + break; + + case FunctionCallContent functionCallContent: + pairMatcher.CollectFunctionCall(functionCallContent); + break; + + case FunctionResultContent functionResultContent: + pairMatcher.TryResolveFunctionCall(functionResultContent, out string? functionName); + string result = functionResultContent.Result?.ToString() ?? string.Empty; + + resultBuilder.AppendLine($"[Tool Call '{functionName ?? functionResultContent.CallId}' Result]") + .AppendLine(result); + + break; + + case McpServerToolCallContent mstContent: + pairMatcher.CollectMcpServerToolCall(mstContent); + break; + + case McpServerToolResultContent mstResultContent: + if (mstResultContent.Outputs?.Any() is true) + { + pairMatcher.TryResolveMcpServerToolCall(mstResultContent, out string? mcpServerToolName); + resultBuilder.AppendLine($"[Start MCP Server Tool Call '{mcpServerToolName ?? mstResultContent.CallId}' Results]"); + + ProcessAIContents(resultBuilder, mstResultContent.Outputs!); + + resultBuilder.AppendLine($"[End MCP Server Tool Call '{mcpServerToolName ?? mstResultContent.CallId}']"); + } + + break; + case TextReasoningContent reasoningContent: + if (!string.IsNullOrWhiteSpace(reasoningContent.Text)) + { + resultBuilder.Append("[Reasoning] ") + .AppendLine(reasoningContent.Text); + } + + break; + + case UriContent uriContent: + resultBuilder.AppendLine(uriContent.Uri.ToString()); + break; + } + } + } + + public static string GetText(this List messages) + { + if (messages.Count == 0) + { + return string.Empty; + } + + StringBuilder builder = new(); + StreamingToolCallResultPairMatcher pairMatcher = new(); + foreach (ChatMessage message in messages) + { + ProcessAIContents(builder, message.Contents, pairMatcher); + } + + return builder.ToString(); + } + + private const string FencedJsonRegexPattern = @"```(?[a-z]+)?\s*(?\{[\s\S]*?\})\s*```"; +#if NET + [GeneratedRegex(FencedJsonRegexPattern, RegexOptions.IgnoreCase | RegexOptions.CultureInvariant | RegexOptions.ExplicitCapture)] + public static partial Regex FencedJsonRegex(); +#else + public static Regex FencedJsonRegex() => s_fencedJsonRegex; + private static readonly Regex s_fencedJsonRegex = + new(FencedJsonRegexPattern, RegexOptions.Compiled | RegexOptions.CultureInvariant | RegexOptions.ExplicitCapture); +#endif + + internal static JsonElement ExtractJson(string messageText) + { + Match match = FencedJsonRegex().Match(messageText); + if (match.Success) + { + return JsonElement.Parse(match.Groups["json"].Value); + } + + int start = messageText.IndexOf('{'), scanHead = start; + int? end = null; + + if (scanHead < 0) + { + throw new InvalidOperationException("No JSON object found."); + } + + int depth = 0; + bool inQuotes = false, inEscape = false; + for (; scanHead < messageText.Length && end is null; scanHead++) + { + if (inEscape) + { + inEscape = false; + continue; + } + + switch (messageText[scanHead]) + { + case '{' when !inQuotes: + depth++; + break; + case '}' when !inQuotes: + depth--; + if (depth == 0) + { + end = scanHead; + } + + break; + case '\"': + // We already handled inEscape, so we can always flip inQuotes here + inQuotes = !inQuotes; + break; + case '\\': + Debug.Assert(!inEscape); + inEscape = true; + break; + } + } + + if (end is null) + { + throw new InvalidOperationException("Unbalanced JSON braces."); + } + + return JsonElement.Parse(messageText.Substring(start, end.Value - start + 1)); + } + + public static JsonElement ExtractJson(this ChatMessage message) => ExtractJson(message.Text); +} diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/ExecutorAgentHarness.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/ExecutorAgentHarness.cs new file mode 100644 index 0000000000..858e91bd3e --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/ExecutorAgentHarness.cs @@ -0,0 +1,72 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System.Collections.Generic; +using System.Text.Json; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.AI; + +namespace Microsoft.Agents.AI.Workflows.Specialized.Magentic; + +internal sealed class ExecutorAgentHarness(AIAgent agent, AIAgentUnservicedRequestsCollector collector) +{ + internal const string AgentSessionKey = nameof(AgentSession); + private AgentSession? _session; + + private async ValueTask EnsureSessionAsync(IWorkflowContext context, CancellationToken cancellationToken) => + this._session ??= await agent.CreateSessionAsync(cancellationToken).ConfigureAwait(false); + + public async ValueTask InvokeAgentAsync(IEnumerable messages, IWorkflowContext context, bool emitUpdateEvents, CancellationToken cancellationToken = default) + { + AgentResponse response; + + if (emitUpdateEvents) + { + // Run the agent in streaming mode only when agent run update events are to be emitted. + IAsyncEnumerable agentStream = agent.RunStreamingAsync( + messages, + await this.EnsureSessionAsync(context, cancellationToken).ConfigureAwait(false), + cancellationToken: cancellationToken); + + List updates = []; + await foreach (AgentResponseUpdate update in agentStream.ConfigureAwait(false)) + { + await context.YieldOutputAsync(update, cancellationToken).ConfigureAwait(false); + collector.ProcessAgentResponseUpdate(update); + updates.Add(update); + } + + response = updates.ToAgentResponse(); + } + else + { + // Otherwise, run the agent in non-streaming mode. + response = await agent.RunAsync(messages, + await this.EnsureSessionAsync(context, cancellationToken).ConfigureAwait(false), + cancellationToken: cancellationToken) + .ConfigureAwait(false); + + collector.ProcessAgentResponse(response); + } + + return response; + } + + public async ValueTask SerializeSessionAsync(CancellationToken cancellationToken) + => this._session == null + ? null + : await agent.SerializeSessionAsync(this._session, cancellationToken: cancellationToken).ConfigureAwait(false); + + public async ValueTask DeserializeSessionAsync(JsonElement? serializedSession, CancellationToken cancellationToken) + { + this._session = serializedSession == null + ? null + : await agent.DeserializeSessionAsync(serializedSession.Value, cancellationToken: cancellationToken) + .ConfigureAwait(false); + } + + public void ResetSession() + { + this._session = null; + } +} diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticConstants.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticConstants.cs new file mode 100644 index 0000000000..2ff41cc43d --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticConstants.cs @@ -0,0 +1,8 @@ +// Copyright (c) Microsoft. All rights reserved. + +namespace Microsoft.Agents.AI.Workflows.Specialized.Magentic; + +internal static class MagenticConstants +{ + public const string MagenticTaskContextKey = nameof(MagenticTaskContextKey); +} diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticManager.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticManager.cs new file mode 100644 index 0000000000..936d9d951e --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticManager.cs @@ -0,0 +1,122 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Collections.Generic; +using System.Runtime.ExceptionServices; +using System.Text.Json; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.AI; + +namespace Microsoft.Agents.AI.Workflows.Specialized.Magentic; + +internal class MagenticManager(AIAgent managerAgent) +{ + private static async ValueTask CheckResponseAsync(Task responseTask, IWorkflowContext context, CancellationToken cancellationToken) + { + AgentResponse response = await responseTask.ConfigureAwait(false); + + if (response.Messages.Count == 0) + { + throw new InvalidOperationException("Planner Agent did not return any messages."); + } + + if (response.Messages.Count > 1) + { + await context.AddEventAsync(new WorkflowWarningEvent("Planner Agent returned multiple messages; using the last one."), cancellationToken) + .ConfigureAwait(false); + } + + return response.Messages[response.Messages.Count - 1]; + } + + private ValueTask InvokeAgentAsync(IEnumerable messages, IWorkflowContext context, CancellationToken cancellationToken, AgentSession? session = null) + => CheckResponseAsync(managerAgent.RunAsync(messages, session, cancellationToken: cancellationToken), context, cancellationToken); + + public async ValueTask UpdatePlanAsync(MagenticTaskContext taskContext, IWorkflowContext context, CancellationToken cancellationToken) + { + // If we already have a TaskLedger, we need to update the facts based on the existing factset; otherwise, we use the initial facts construction + bool isReplan = taskContext.TaskLedger != null; + + AgentSession localSession = await managerAgent.CreateSessionAsync(cancellationToken).ConfigureAwait(false); + + ChatMessage factsRequest = new(ChatRole.User, isReplan ? taskContext.ToTaskLedgerFactsUpdatePrompt() : taskContext.ToTaskLedgerFactsPrompt()); + ChatMessage updatedFacts = await this.InvokeAgentAsync( + messages: [.. taskContext.ChatHistory, factsRequest], + context, + cancellationToken, + localSession) + .ConfigureAwait(false); + + ChatMessage planRequest = new(ChatRole.User, isReplan ? taskContext.ToTaskLedgerPlanUpdatePrompt() : taskContext.ToTaskLedgerPlanPrompt()); + ChatMessage updatedPlan = await this.InvokeAgentAsync( + // We rely on the AgentSession to maintain the context of the conversation, so we don't include the + // history, facts request, or updated facts in the messages list. + messages: [planRequest], + context, + cancellationToken, + localSession) + .ConfigureAwait(false); + + taskContext.ChatHistory.AddRange([factsRequest, updatedFacts, planRequest, updatedPlan]); + + return new(updatedFacts, updatedPlan); + } + + public async ValueTask UpdateProgressLedgerAsync(MagenticTaskContext taskContext, IWorkflowContext context, CancellationToken cancellationToken) + { + ChatMessage progressRequest = new(ChatRole.User, taskContext.ToProgressLedgerPrompt()); + + ExceptionDispatchInfo? lastException = null; + int maxRetryCount = taskContext.TaskLimits.MaxProgressLedgerRetryCount; + for (int attempts = 0; attempts < maxRetryCount; attempts++) + { + ChatMessage progressUpdateMessage = await this.InvokeAgentAsync( + messages: [.. taskContext.ChatHistory, progressRequest], + context, + cancellationToken) + .ConfigureAwait(false); + + try + { + lastException = null; + JsonElement stateUpdateJson = progressUpdateMessage.ExtractJson(); + if (!taskContext.ProgressLedger.TryUpdateState(stateUpdateJson)) + { + throw new InvalidOperationException("Could not answer progress ledger questions with provided JSON."); + } + + break; + } + catch (Exception e) + { + lastException = ExceptionDispatchInfo.Capture(e); + + string warnString = $"Progress ledger JSON parse failed (attempt {attempts}/{maxRetryCount}): {e}"; + await context.AddEventAsync(new WorkflowWarningEvent(warnString), cancellationToken).ConfigureAwait(false); + + if (attempts < maxRetryCount) + { + await Task.Delay(250 * attempts, cancellationToken).ConfigureAwait(false); + } + } + } + + lastException?.Throw(); + } + + public async ValueTask PrepareFinalAnswerAsync(MagenticTaskContext taskContext, IWorkflowContext context, CancellationToken cancellationToken) + { + ChatMessage finalAnswerRequest = new(ChatRole.User, taskContext.ToFinalAnswerPrompt()); + ChatMessage finalAnswer = await this.InvokeAgentAsync([.. taskContext.ChatHistory, finalAnswerRequest], context, cancellationToken) + .ConfigureAwait(false); + + return new(ChatRole.Assistant, finalAnswer.Text) + { + AuthorName = finalAnswer.AuthorName ?? nameof(MagenticManager), + MessageId = finalAnswer.MessageId ?? Guid.NewGuid().ToString("N"), + CreatedAt = finalAnswer.CreatedAt ?? DateTimeOffset.UtcNow, + RawRepresentation = finalAnswer.RawRepresentation, + }; + } +} diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticOrchestrator.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticOrchestrator.cs new file mode 100644 index 0000000000..b272ed102d --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticOrchestrator.cs @@ -0,0 +1,326 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text.Json.Serialization; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.AI; + +namespace Microsoft.Agents.AI.Workflows.Specialized.Magentic; + +/// +/// Base type for Magentic Orchestration Events +/// +/// +[JsonDerivedType(typeof(MagenticPlanCreatedEvent))] +[JsonDerivedType(typeof(MagenticReplannedEvent))] +[JsonDerivedType(typeof(MagenticProgressLedgerUpdatedEvent))] +public abstract class MagenticOrchestratorEvent(object? data) : WorkflowEvent(data) +{ +} + +/// +/// Represents the creation of the initial plan +/// +/// +public sealed class MagenticPlanCreatedEvent(ChatMessage fullTaskLeger) : MagenticOrchestratorEvent(fullTaskLeger) +{ + /// + /// A containing the initial plan. + /// + public ChatMessage FullTaskLedger { get; } = fullTaskLeger; +} + +/// +/// Represents the creation of a new plan in response to a stall. +/// +/// +public sealed class MagenticReplannedEvent(ChatMessage fullTaskLeger) : MagenticOrchestratorEvent(fullTaskLeger) +{ + /// + /// A containing the new plan. + /// + public ChatMessage FullTaskLedger { get; } = fullTaskLeger; +} + +/// +/// Represents an update to the when running a coordination round. +/// +/// +public sealed class MagenticProgressLedgerUpdatedEvent(MagenticProgressLedger progressLedger) : MagenticOrchestratorEvent(progressLedger) +{ + /// + /// The new state of the + /// + public MagenticProgressLedger ProgressLedger { get; } = progressLedger; +} + +/// +/// Magentic orchestrator that defines the workflow structure. +/// +/// This orchestrator manages the overall Magentic workflow in the following structure: +/// +/// 1. Upon receiving the task(a list of messages), it creates the plan using the manager then runs the inner loop. +/// 2. The inner loop is distributed and implementation is decentralized. In the orchestrator, it is responsible for: +/// - Creating the progress ledger using the manager. +/// - Checking for task completion. +/// - Detecting stalling or looping and triggering replanning if needed. +/// - Sending requests to participants based on the progress ledger's next speaker. +/// - Issue requests for human intervention if enabled and needed. +/// 3. The inner loop waits for responses from the selected participant, then continues the loop. +/// 4. The orchestrator breaks out of the inner loop when the replanning or final answer conditions are met. +/// 5. The outer loop handles replanning and reenters the inner loop. +/// +/// +/// +/// +/// +internal class MagenticOrchestrator(AIAgent managerAgent, List team, TaskLimits limits, bool requirePlanSignoff) + : ChatProtocolExecutor(nameof(MagenticOrchestrator), s_options, declareCrossRunShareable: false) +{ + private readonly MagenticManager _manager = new(managerAgent); + + private static readonly ChatProtocolExecutorOptions s_options = new() + { + StringMessageChatRole = ChatRole.User, + AutoSendTurnToken = false + }; + + private MagenticTaskContext? _taskContext; + private PortBinding? _planReviewPort; + + protected override ProtocolBuilder ConfigureProtocol(ProtocolBuilder protocolBuilder) + { + return base.ConfigureProtocol(protocolBuilder).ConfigureRoutes(ConfigureRoutes); + + void ConfigureRoutes(RouteBuilder routeBuilder) => routeBuilder.AddPortHandler( + "RequestPlanReview", + this.ProcessPlanReviewAsync, + out this._planReviewPort); + } + + private ValueTask SubmitPlanReviewRequestAsync(MagenticTaskContext taskContext, IWorkflowContext workflowContext) + { + MagenticProgressLedger? progressLedger = taskContext.ProgressLedger; + if (progressLedger?.IsStarted is not true) + { + progressLedger = null; + } + + MagenticPlanReviewRequest request = new(taskContext.TaskLedger!.CurrentPlan, progressLedger, taskContext.IsStalled); + + return this._planReviewPort!.PostRequestAsync(request); + } + + private async ValueTask ProcessPlanReviewAsync(MagenticPlanReviewResponse response, IWorkflowContext context, CancellationToken cancellationToken) + { + /* + Handle the human response to the plan review request. + + Logic: + There are code paths which will trigger a plan review request to the human: + - Initial plan creation if `require_plan_signoff` is True. + - Potentially during the inner loop if stalling is detected (resetting and replanning). + + The human can either approve the plan or request revisions with comments. + - If approved, proceed to run the outer loop, which simply adds the task ledger + to the conversation and enters the inner loop. + - If revision requested, append the review comments to the chat history, + trigger replanning via the manager, emit a REPLANNED event, then run the outer loop. + + */ + if (this._taskContext == null || this._taskContext.TaskLedger == null) + { + throw new InvalidOperationException("Magentic Orchestration was not initialized correctly."); + } + + if (this._taskContext.IsTerminated) + { + throw new InvalidOperationException("Magentic Orchestration has already been terminated and cannot process new messages. Please start a new session."); + } + + if (response.IsApproved) + { + await this.DelegateToTeamAsync(this._taskContext, context, cancellationToken).ConfigureAwait(false); + } + else + { + this._taskContext.ChatHistory.AddRange(response.Review); + + await this.UpdatePlanAndDelegateAsync(this._taskContext, context, cancellationToken).ConfigureAwait(false); + } + } + + private async ValueTask UpdatePlanAndDelegateAsync(MagenticTaskContext taskContext, IWorkflowContext context, CancellationToken cancellationToken) + { + bool isReplan = taskContext.TaskLedger != null; + + taskContext.TaskLedger = await this._manager.UpdatePlanAsync(taskContext, context, cancellationToken) + .ConfigureAwait(false); + + this._fullTaskLedgerMessage = new(ChatRole.User, taskContext.ToTaskLedgerFullPrompt()); + taskContext.ChatHistory.Add(this._fullTaskLedgerMessage); + + await context.AddEventAsync(isReplan + ? new MagenticReplannedEvent(this._fullTaskLedgerMessage) + : new MagenticPlanCreatedEvent(this._fullTaskLedgerMessage), cancellationToken).ConfigureAwait(false); + + if (requirePlanSignoff) + { + await this.SubmitPlanReviewRequestAsync(taskContext, context).ConfigureAwait(false); + } + else + { + await this.DelegateToTeamAsync(taskContext, context, cancellationToken).ConfigureAwait(false); + } + } + + protected override async ValueTask TakeTurnAsync(List messages, IWorkflowContext context, bool? emitEvents, CancellationToken cancellationToken = default) + { + // First Turn: Initialize the task context and send the initial messages to the planner agent + this._taskContext ??= new(messages, team, limits, emitEvents, []); + await this.UpdatePlanAndDelegateAsync(this._taskContext, context, cancellationToken).ConfigureAwait(false); + } + + private ChatMessage? _fullTaskLedgerMessage; + private ValueTask DelegateToTeamAsync(MagenticTaskContext taskContext, IWorkflowContext context, CancellationToken cancellationToken) + { + return this.RunCoordinationRoundAsync(taskContext, context, cancellationToken); + } + + private async ValueTask RunCoordinationRoundAsync(MagenticTaskContext taskContext, IWorkflowContext context, CancellationToken cancellationToken) + { + (bool hitRoundLimit, bool hitResetLimit) = taskContext.CheckLimits(); + + if (hitRoundLimit || hitResetLimit) + { + string limitType = hitRoundLimit ? "round" : "reset"; + + List messages = [new(ChatRole.Assistant, $"Task execution stopped due to hitting the maximum {limitType} count limit.")]; + await context.YieldOutputAsync(messages, cancellationToken).ConfigureAwait(false); + taskContext.IsTerminated = true; + + return; + } + + taskContext.TaskCounters.RoundCount++; + + // Update the Progress Ledger + try + { + await this._manager.UpdateProgressLedgerAsync(taskContext, context, cancellationToken).ConfigureAwait(false); + + await context.AddEventAsync(new MagenticProgressLedgerUpdatedEvent(taskContext.ProgressLedger), cancellationToken) + .ConfigureAwait(false); + } + // Retry on exception to max retry count, unless it is OperationCancelledException - in that case exit the loop right away + catch (Exception ex) when (ex is not OperationCanceledException) + { + await context.AddEventAsync(new WorkflowWarningEvent($"Magentic Orchestrator: Progress ledger creation failed, triggering reset: {ex}"), cancellationToken) + .ConfigureAwait(false); + + await this.ResetAndReplanAsync(taskContext, context, cancellationToken).ConfigureAwait(false); + return; + } + + // Check and handle finish condition + if (taskContext.ProgressLedger.IsRequestSatisfied) + { + await this.PrepareFinalAnswerAsync(taskContext, context, cancellationToken).ConfigureAwait(false); + return; + } + + // Check and handle stalls + if (taskContext.ProgressLedger.IsInLoop || !taskContext.ProgressLedger.IsProgressBeingMade) + { + taskContext.TaskCounters.StallCount++; + } + else + { + taskContext.TaskCounters.StallCount = Math.Max(0, taskContext.TaskCounters.StallCount - 1); + } + + if (taskContext.IsStalled) + { + await this.ResetAndReplanAsync(taskContext, context, cancellationToken).ConfigureAwait(false); + return; + } + + // Prepare to delegate to the next speaker + string nextSpeaker = taskContext.ProgressLedger.NextSpeaker; + if (string.IsNullOrEmpty(nextSpeaker)) + { + await context.AddEventAsync(new WorkflowWarningEvent("Next speaker answer empty; selecting first participant as fallback"), cancellationToken) + .ConfigureAwait(false); + nextSpeaker = team.First().Name!; + } + + AIAgent? nextAgent = team.FirstOrDefault(agent => agent.Name == nextSpeaker); + if (nextAgent == null) + { + await context.AddEventAsync(new WorkflowWarningEvent($"Invalid next speaker: {nextSpeaker}"), cancellationToken) + .ConfigureAwait(false); + await this.PrepareFinalAnswerAsync(taskContext, context, cancellationToken).ConfigureAwait(false); + return; + } + + if (!string.IsNullOrWhiteSpace(taskContext.ProgressLedger.InstructionOrQuestion)) + { + ChatMessage instruction = new(ChatRole.Assistant, taskContext.ProgressLedger.InstructionOrQuestion); + taskContext.ChatHistory.Add(instruction); + + await context.SendMessageAsync(instruction, cancellationToken).ConfigureAwait(false); + } + + string nextExecutorId = AIAgentHostExecutor.IdFor(nextAgent); + await context.SendMessageAsync(new TurnToken(taskContext.EmitUpdateEvents), nextExecutorId, cancellationToken).ConfigureAwait(false); + } + + private async ValueTask ResetAndReplanAsync(MagenticTaskContext taskContext, IWorkflowContext context, CancellationToken cancellationToken) + { + taskContext.Reset(); + await context.SendMessageAsync(new ResetChatSignal(), cancellationToken: cancellationToken).ConfigureAwait(false); + + await this.UpdatePlanAndDelegateAsync(taskContext, context, cancellationToken).ConfigureAwait(false); + } + + private async ValueTask PrepareFinalAnswerAsync(MagenticTaskContext taskContext, IWorkflowContext context, CancellationToken cancellationToken) + { + List messages = [await this._manager.PrepareFinalAnswerAsync(taskContext, context, cancellationToken).ConfigureAwait(false)]; + await context.YieldOutputAsync(messages, cancellationToken).ConfigureAwait(false); + taskContext.IsTerminated = true; + } + + private const string CurrentTurnEmitUpdateEventsKey = nameof(CurrentTurnEmitUpdateEventsKey); + protected internal override async ValueTask OnCheckpointingAsync(IWorkflowContext context, CancellationToken cancellationToken = default) + { + Task contextStateTask = this._taskContext == null + ? Task.CompletedTask + : context.QueueStateUpdateAsync(MagenticConstants.MagenticTaskContextKey, + this._taskContext.ExportState(), + cancellationToken: cancellationToken) + .AsTask(); + + await Task.WhenAll(base.OnCheckpointingAsync(context, cancellationToken).AsTask(), + contextStateTask).ConfigureAwait(false); + } + + protected internal override async ValueTask OnCheckpointRestoredAsync(IWorkflowContext context, CancellationToken cancellationToken = default) + { + await Task.WhenAll(base.OnCheckpointRestoredAsync(context, cancellationToken).AsTask(), LoadContextStateAsync()) + .ConfigureAwait(false); + + async Task LoadContextStateAsync() + { + MagenticTaskState? state = await context.ReadStateAsync(MagenticConstants.MagenticTaskContextKey, cancellationToken: cancellationToken) + .ConfigureAwait(false); + + if (state != null) + { + this._taskContext = new MagenticTaskContext(state, team, limits, []); + } + } + } +} diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticTaskContext.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticTaskContext.cs new file mode 100644 index 0000000000..0db289126e --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticTaskContext.cs @@ -0,0 +1,95 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text.Json; +using Microsoft.Extensions.AI; + +namespace Microsoft.Agents.AI.Workflows.Specialized.Magentic; + +internal record TaskLimits(int MaxStallCount = TaskLimits.DefaultMaxStallCount, + int? MaxRoundCount = null, + int? MaxResetCount = null, + int MaxProgressLedgerRetryCount = TaskLimits.DefaultMaxProgressLedgerRetryCount) +{ + public const int DefaultMaxStallCount = 3; + public const int DefaultMaxProgressLedgerRetryCount = 3; +} + +internal record TaskLedger(ChatMessage CurrentFacts, ChatMessage CurrentPlan); + +internal class TaskCounters +{ + public int RoundCount { get; set; } + public int StallCount { get; set; } + public int ResetCount { get; set; } +} + +internal record MagenticTaskState(List TaskDefinition, List ChatHistory, TaskLedger? TaskLedger, JsonElement? ProgressLedgerState, TaskCounters Counters, bool Terminated, bool? EmitUpdateEvents) +{ +} + +internal class MagenticTaskContext(List taskDefinition, List team, TaskLimits limits, bool? emitUpdateEvents, IEnumerable additionalProgressQuestions) +{ + internal MagenticTaskContext(MagenticTaskState state, List team, TaskLimits limits, IEnumerable additionalProgressQuestions) + : this(state.TaskDefinition, team, limits, state.EmitUpdateEvents, additionalProgressQuestions) + { + this.TaskLedger = state.TaskLedger; + this.TaskCounters = state.Counters; + this.ChatHistory = state.ChatHistory; + this.IsTerminated = state.Terminated; + + if (state.ProgressLedgerState.HasValue && !this.ProgressLedger.TryUpdateState(state.ProgressLedgerState.Value)) + { + throw new InvalidOperationException("Could not load progress ledger state value"); + } + } + + public string Task { get; } = taskDefinition.GetText(); + + public string TeamDescription { get; } = GetTeamDescription(team); + + public List ChatHistory { get; internal set; } = new(); + + public TaskLedger? TaskLedger { get; internal set; } + + public TaskLimits TaskLimits => limits; + + public bool IsTerminated { get; internal set; } + + public bool IsStalled => this.TaskCounters.StallCount >= this.TaskLimits.MaxStallCount; + + public (bool HitRoundLimit, bool HitResetLimit) CheckLimits() + { + return (this.TaskLimits.MaxRoundCount.HasValue && this.TaskLimits.MaxRoundCount.Value <= this.TaskCounters.RoundCount, + this.TaskLimits.MaxResetCount.HasValue && this.TaskLimits.MaxResetCount.Value <= this.TaskCounters.ResetCount); + } + + public TaskCounters TaskCounters { get; internal set; } = new(); + + public MagenticProgressLedger ProgressLedger { get; } = new(GetTeamNames(team), additionalProgressQuestions); + public bool? EmitUpdateEvents => emitUpdateEvents; + + public static string GetTeamDescription(IEnumerable team) + { + return string.Join("\n", team.Select(agent => $"- {agent.Name}: {agent.Description}")); + } + + public static string GetTeamNames(IEnumerable team) + { + return string.Join(", ", team.Select(agent => agent.Name)); + } + + public MagenticTaskState ExportState() + { + return new(taskDefinition, this.ChatHistory, this.TaskLedger, this.ProgressLedger.State, this.TaskCounters, this.IsTerminated, this.EmitUpdateEvents); + } + + internal void Reset() + { + this.ChatHistory.Clear(); + this.TaskCounters.ResetCount++; + this.TaskCounters.StallCount = 0; + } +} diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/PromptTemplates.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/PromptTemplates.cs new file mode 100644 index 0000000000..17176a97d4 --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/PromptTemplates.cs @@ -0,0 +1,151 @@ +// Copyright (c) Microsoft. All rights reserved. + +using Microsoft.Extensions.AI; + +namespace Microsoft.Agents.AI.Workflows.Specialized.Magentic; + +internal static class PromptTemplateExtensions +{ + public static string ToTaskLedgerFactsPrompt(this MagenticTaskContext taskContext) + { + return $""" +Below I will present you a request. + +Before we begin addressing the request, please answer the following pre-survey to the best of your ability. + Keep in mind that you are Ken Jennings-level with trivia, and Mensa-level with puzzles, so there should be + a deep well to draw from. + + Here is the request: + +{taskContext.Task} + + Here is the pre-survey: + + 1. Please list any specific facts or figures that are GIVEN in the request itself.It is possible that + there are none. + 2. Please list any facts that may need to be looked up, and WHERE SPECIFICALLY they might be found. + In some cases, authoritative sources are mentioned in the request itself. + 3. Please list any facts that may need to be derived(e.g., via logical deduction, simulation, or computation) + 4. Please list any facts that are recalled from memory, hunches, well-reasoned guesses, etc. + +When answering this survey, keep in mind that "facts" will typically be specific names, dates, statistics, etc. +Your answer should use headings: + + 1. GIVEN OR VERIFIED FACTS + 2. FACTS TO LOOK UP + 3. FACTS TO DERIVE + 4. EDUCATED GUESSES + +DO NOT include any other headings or sections in your response.DO NOT list next steps or plans until asked to do so. +"""; + } + + public static string ToTaskLedgerFactsUpdatePrompt(this MagenticTaskContext taskContext) + { + return $""" +As a reminder, we are working to solve the following task: + +{taskContext.Task} + +It is clear we are not making as much progress as we would like, but we may have learned something new. +Please rewrite the following fact sheet, updating it to include anything new we have learned that may be helpful. + +Example edits can include (but are not limited to) adding new guesses, moving educated guesses to verified facts +if appropriate, etc. Updates may be made to any section of the fact sheet, and more than one section of the fact +sheet can be edited. This is an especially good time to update educated guesses, so please at least add or update +one educated guess or hunch, and explain your reasoning. + +Here is the old fact sheet: + +{taskContext.TaskLedger?.CurrentFacts ?? new(ChatRole.Assistant, string.Empty)} +"""; + } + + public static string ToTaskLedgerPlanPrompt(this MagenticTaskContext taskContext) + { + return $""" +Fantastic. To address this request we have assembled the following team: + +{taskContext.TeamDescription} + +Based on the team composition, and known and unknown facts, please devise a short bullet-point plan for addressing the +original request. Remember, there is no requirement to involve all team members. A team member's particular expertise +may not be needed for this task. +"""; + } + + public static string ToTaskLedgerPlanUpdatePrompt(this MagenticTaskContext taskContext) + { + return $""" +Please briefly explain what went wrong on this last run +(the root cause of the failure), and then come up with a new plan that takes steps and includes hints to overcome prior +challenges and especially avoids repeating the same mistakes. As before, the new plan should be concise, expressed in +bullet-point form, and consider the following team composition: + +{taskContext.TeamDescription} +"""; + } + + public static string ToTaskLedgerFullPrompt(this MagenticTaskContext taskContext) + { + return $""" +We are working to address the following user request: + +{taskContext.Task} + + +To answer this request we have assembled the following team: + +{taskContext.TeamDescription} + + +Here is an initial fact sheet to consider: + +{taskContext.TaskLedger!.CurrentFacts ?? new(ChatRole.Assistant, string.Empty)} + + +Here is the plan to follow as best as possible: + +{taskContext.TaskLedger!.CurrentPlan} +"""; + } + + public static string ToProgressLedgerPrompt(this MagenticTaskContext taskContext) + { + (string questions, string schema) = taskContext.ProgressLedger.FormatQuestions(); + + return $""" +Recall we are working on the following request: + +{taskContext.Task} + +And we have assembled the following team: + +{taskContext.TeamDescription} + +To make progress on the request, please answer the following questions, including necessary reasoning: + +{questions} + +Please output an answer in pure JSON format according to the following schema. The JSON object must be parsable as-is. +DO NOT OUTPUT ANYTHING OTHER THAN JSON, AND DO NOT DEVIATE FROM THIS SCHEMA: + +{schema} +"""; + } + + public static string ToFinalAnswerPrompt(this MagenticTaskContext taskContext) + { + return $""" +We are working on the following task: +{taskContext.Task} + +We have completed the task. + +The above messages contain the conversation that took place to complete the task. + +Based on the information gathered, provide the final answer to the original request. +The answer should be phrased as if you were speaking to the user. +"""; + } +} diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/StreamingToolCallResultPairMatcher.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/StreamingToolCallResultPairMatcher.cs new file mode 100644 index 0000000000..bb72b8390a --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/StreamingToolCallResultPairMatcher.cs @@ -0,0 +1,77 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using Microsoft.Extensions.AI; + +namespace Microsoft.Agents.AI.Workflows.Specialized.Magentic; + +internal sealed class StreamingToolCallResultPairMatcher +{ + private enum CallType + { + Function, + McpServerTool + } + + private record CallSummaryKey(CallType Type, string CallId); + + private struct ToolCallSummary(CallType callType, string callId, string name) + { + public CallType CallType => callType; + + public string? CallId => callId; + + public string Name => name; + } + + private readonly Dictionary _callSummaries = new(); + + private void Collect(CallType callType, string callId, string name, string callContentTypeName, string resultContentTypeName) + { + CallSummaryKey key = new(callType, callId); + if (this._callSummaries.ContainsKey(key)) + { + throw new InvalidOperationException($"Duplicate {callContentTypeName} with CallId '{callId}' without corresponding {resultContentTypeName}."); + } + + this._callSummaries[key] = new ToolCallSummary(callType, callId, name); + } + + public void CollectFunctionCall(FunctionCallContent callContent) + { + const string FunctionCallContentTypeName = nameof(FunctionCallContent); + const string FunctionResultContentTypeName = nameof(FunctionResultContent); + + this.Collect(CallType.Function, callContent.CallId, callContent.Name, FunctionCallContentTypeName, FunctionResultContentTypeName); + } + + public void CollectMcpServerToolCall(McpServerToolCallContent callContent) + { + const string McpServerToolCallContentTypeName = nameof(McpServerToolCallContent); + const string McpServerToolResultContentTypeName = nameof(McpServerToolResultContent); + + this.Collect(CallType.McpServerTool, callContent.CallId, callContent.Name, McpServerToolCallContentTypeName, McpServerToolResultContentTypeName); + } + + private bool TryResolve(CallType callType, string callId, [NotNullWhen(true)] out string? name) + { + CallSummaryKey key = new(callType, callId); + + bool hasMatchingCall = this._callSummaries.TryGetValue(key, out ToolCallSummary callSummary); + if (hasMatchingCall) + { + this._callSummaries.Remove(key); + } + + name = hasMatchingCall ? callSummary.Name : null; + return hasMatchingCall; + } + + public bool TryResolveFunctionCall(FunctionResultContent resultContent, [NotNullWhen(true)] out string? name) + => this.TryResolve(CallType.Function, resultContent.CallId, out name); + + public bool TryResolveMcpServerToolCall(McpServerToolResultContent resultContent, [NotNullWhen(true)] out string? name) + => this.TryResolve(CallType.McpServerTool, resultContent.CallId, out name); +} diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/WorkflowEvent.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/WorkflowEvent.cs index 76b379a611..0095e86000 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows/WorkflowEvent.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/WorkflowEvent.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. using System.Text.Json.Serialization; +using Microsoft.Agents.AI.Workflows.Specialized.Magentic; namespace Microsoft.Agents.AI.Workflows; @@ -14,6 +15,8 @@ namespace Microsoft.Agents.AI.Workflows; [JsonDerivedType(typeof(WorkflowWarningEvent))] [JsonDerivedType(typeof(WorkflowOutputEvent))] [JsonDerivedType(typeof(RequestInfoEvent))] +[JsonDerivedType(typeof(MagenticOrchestratorEvent))] + public class WorkflowEvent(object? data = null) { /// diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/WorkflowsJsonUtilities.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/WorkflowsJsonUtilities.cs index 6978cc1c93..8b3d3e4ce8 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows/WorkflowsJsonUtilities.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/WorkflowsJsonUtilities.cs @@ -7,6 +7,7 @@ using System.Text.Json.Serialization; using Microsoft.Agents.AI.Workflows.Checkpointing; using Microsoft.Agents.AI.Workflows.Execution; using Microsoft.Agents.AI.Workflows.Specialized; +using Microsoft.Agents.AI.Workflows.Specialized.Magentic; using Microsoft.Extensions.AI; namespace Microsoft.Agents.AI.Workflows; @@ -97,6 +98,10 @@ internal static partial class WorkflowsJsonUtilities [JsonSerializable(typeof(AIAgentHostState))] [JsonSerializable(typeof(HandoffSharedState))] [JsonSerializable(typeof(HandoffAgentHostState))] + [JsonSerializable(typeof(MagenticPlanReviewRequest))] + [JsonSerializable(typeof(MagenticPlanReviewResponse))] + [JsonSerializable(typeof(MagenticTaskState))] + [JsonSerializable(typeof(ResetChatSignal))] // Event Types //[JsonSerializable(typeof(WorkflowEvent))] diff --git a/dotnet/tests/AnthropicChatCompletion.IntegrationTests/AnthropicChatCompletionFixture.cs b/dotnet/tests/AnthropicChatCompletion.IntegrationTests/AnthropicChatCompletionFixture.cs index af98629237..7c2a0c3b6c 100644 --- a/dotnet/tests/AnthropicChatCompletion.IntegrationTests/AnthropicChatCompletionFixture.cs +++ b/dotnet/tests/AnthropicChatCompletion.IntegrationTests/AnthropicChatCompletionFixture.cs @@ -17,9 +17,6 @@ namespace AnthropicChatCompletion.IntegrationTests; public class AnthropicChatCompletionFixture : IChatClientAgentFixture { - // All tests for Anthropic are intended to be ran locally as the CI pipeline for Anthropic is not setup. - internal const string SkipReason = "Integrations tests for local execution only"; - private readonly bool _useReasoningModel; private readonly bool _useBeta; @@ -105,7 +102,22 @@ public class AnthropicChatCompletionFixture : IChatClientAgentFixture public async ValueTask InitializeAsync() { - Assert.SkipWhen(SkipReason is not null, SkipReason ?? string.Empty); + // Temporarily disabled: Anthropic SDK has a binary incompatibility with the current + // Microsoft.Extensions.AI version (WebSearchToolResultContent.Results method not found). + // See: https://github.com/microsoft/agent-framework/pull/5515 + Assert.Skip("Anthropic integration tests temporarily disabled due to SDK incompatibility with Microsoft.Extensions.AI"); + + try + { + _ = TestConfiguration.GetRequiredValue(TestSettings.AnthropicApiKey); + _ = TestConfiguration.GetRequiredValue(TestSettings.AnthropicChatModelName); + _ = TestConfiguration.GetRequiredValue(TestSettings.AnthropicReasoningModelName); + } + catch (InvalidOperationException ex) + { + Assert.Skip("Anthropic configuration could not be loaded. Error:" + ex.Message); + } + this._agent = await this.CreateChatClientAgentAsync(); } diff --git a/dotnet/tests/AnthropicChatCompletion.IntegrationTests/AnthropicSkillsIntegrationTests.cs b/dotnet/tests/AnthropicChatCompletion.IntegrationTests/AnthropicSkillsIntegrationTests.cs index 452b0c6cf2..82b3511993 100644 --- a/dotnet/tests/AnthropicChatCompletion.IntegrationTests/AnthropicSkillsIntegrationTests.cs +++ b/dotnet/tests/AnthropicChatCompletion.IntegrationTests/AnthropicSkillsIntegrationTests.cs @@ -1,5 +1,6 @@ // Copyright (c) Microsoft. All rights reserved. +using System; using System.Threading.Tasks; using AgentConformance.IntegrationTests.Support; using Anthropic; @@ -17,19 +18,28 @@ namespace AnthropicChatCompletion.IntegrationTests; /// Integration tests for Anthropic Skills functionality. /// These tests are designed to be run locally with a valid Anthropic API key. /// +/// +/// Temporarily disabled due to Anthropic SDK binary incompatibility with +/// the current Microsoft.Extensions.AI version (WebSearchToolResultContent.Results). +/// +[Trait("Category", "IntegrationDisabled")] public sealed class AnthropicSkillsIntegrationTests { - // All tests for Anthropic are intended to be ran locally as the CI pipeline for Anthropic is not setup. - private const string SkipReason = "Integrations tests for local execution only"; - [Fact] public async Task CreateAgentWithPptxSkillAsync() { - Assert.SkipWhen(SkipReason is not null, SkipReason ?? string.Empty); - - // Arrange - AnthropicClient anthropicClient = new() { ApiKey = TestConfiguration.GetRequiredValue(TestSettings.AnthropicApiKey) }; - string model = TestConfiguration.GetRequiredValue(TestSettings.AnthropicChatModelName); + AnthropicClient? anthropicClient; + string? model; + try + { + anthropicClient = new() { ApiKey = TestConfiguration.GetRequiredValue(TestSettings.AnthropicApiKey) }; + model = TestConfiguration.GetRequiredValue(TestSettings.AnthropicChatModelName); + } + catch (InvalidOperationException ex) + { + Assert.Skip("Anthropic configuration could not be loaded. Error:" + ex.Message); + return; + } BetaSkillParams pptxSkill = new() { @@ -56,10 +66,16 @@ public sealed class AnthropicSkillsIntegrationTests [Fact] public async Task ListAnthropicManagedSkillsAsync() { - Assert.SkipWhen(SkipReason is not null, SkipReason ?? string.Empty); - - // Arrange - AnthropicClient anthropicClient = new() { ApiKey = TestConfiguration.GetRequiredValue(TestSettings.AnthropicApiKey) }; + AnthropicClient? anthropicClient; + try + { + anthropicClient = new() { ApiKey = TestConfiguration.GetRequiredValue(TestSettings.AnthropicApiKey) }; + } + catch (InvalidOperationException ex) + { + Assert.Skip("Anthropic configuration could not be loaded. Error:" + ex.Message); + return; + } // Act SkillListPage skills = await anthropicClient.Beta.Skills.List( diff --git a/dotnet/tests/Foundry.Hosting.IntegrationTests/scripts/it-build-image.ps1 b/dotnet/tests/Foundry.Hosting.IntegrationTests/scripts/it-build-image.ps1 index 2bb4670a76..857ea3be0a 100644 --- a/dotnet/tests/Foundry.Hosting.IntegrationTests/scripts/it-build-image.ps1 +++ b/dotnet/tests/Foundry.Hosting.IntegrationTests/scripts/it-build-image.ps1 @@ -41,7 +41,14 @@ param( [string] $Repository = "foundry-hosting-it", - [string] $TestContainerProject = "dotnet/tests/Foundry.Hosting.IntegrationTests.TestContainer" + [string] $TestContainerProject = "dotnet/tests/Foundry.Hosting.IntegrationTests.TestContainer", + + # Explicit opt-in for the no-rebuild fast path. CI sets this after running the + # "Build Foundry hosted IT (and its deps)" step, which guarantees the prebuilt + # library DLLs match current source. Off by default so local invocations always + # let publish rebuild ProjectReferences and never produce an image whose tag is + # computed from current source while the contents come from a stale build. + [switch] $UsePrebuiltProjectReferences ) $ErrorActionPreference = "Stop" @@ -100,7 +107,60 @@ if (Test-Path $out) { Remove-Item -Recurse -Force $out } -dotnet publish $TestContainerProject -c Release -f net10.0 -r linux-musl-x64 --self-contained false -o $out --tl:off | Out-Host +# Conditionally tell publish to skip rebuilding ProjectReferences and consume the +# prebuilt library DLLs in place. This avoids two failure modes that arise when +# the CI workflow runs a `dotnet build` of the same library projects immediately +# before this script: +# 1) MSB3026 "file is being used by another process" when publish's MSBuild +# tries to overwrite src//bin/Release/net10.0/.dll while the +# previous build's shared-compilation server still holds a file handle. +# 2) Publish needlessly rebuilding identical managed (RID-agnostic) library +# DLLs that prebuild already produced. +# Gated on -UsePrebuiltProjectReferences (a strict opt-in) instead of marker +# detection, because a developer machine may have a stale Release build of the +# libraries from days ago; using those would silently produce an image whose +# content is older than the source the tag is computed from. +$publishExtraArgs = @() +if ($UsePrebuiltProjectReferences) { + Write-Host "-UsePrebuiltProjectReferences: skipping ProjectReference rebuild." -ForegroundColor DarkGray + $publishExtraArgs += "-p:BuildProjectReferences=false" +} else { + # Preflight: in default (rebuild) mode, publish propagates RuntimeIdentifier=linux-musl-x64 + # to library ProjectReferences and writes their intermediates to a RID-suffixed obj path + # (e.g. obj/Release/net10.0/linux-musl-x64/). DefaultItemExcludes follows the new + # IntermediateOutputPath, so any *.AssemblyInfo.cs left in obj/Release/net10.0/ from a + # prior `dotnet build` is no longer excluded and gets picked up by the **/*.cs Compile + # glob, producing CS0579 "duplicate attribute" errors. Detect that state up front and + # tell the user exactly how to recover. + $staleObjProbes = @( + "dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/obj/Release/net10.0", + "dotnet/src/Microsoft.Agents.AI.Foundry/obj/Release/net10.0", + "dotnet/src/Microsoft.Agents.AI/obj/Release/net10.0", + "dotnet/src/Microsoft.Agents.AI.Abstractions/obj/Release/net10.0" + ) + $stale = @($staleObjProbes | Where-Object { Test-Path (Join-Path $_ "*.AssemblyInfo.cs") }) + if ($stale.Count -gt 0) { + $msg = @( + "Detected prior Release/net10.0 build outputs in:" + ($stale | ForEach-Object { " - $_" }) + "" + "Publish would propagate -r linux-musl-x64 to those ProjectReferences and the" + "leftover obj/Release/net10.0/*.AssemblyInfo.cs files would cause CS0579 duplicate" + "attribute errors. Pick one:" + " (a) Pass -UsePrebuiltProjectReferences (skips ProjectReference rebuild and" + " uses the existing src//bin/Release/net10.0/*.dll outputs in place)." + " Only safe when you know those DLLs match current source - this is the path" + " CI uses immediately after its 'Build Foundry hosted IT (and its deps)' step." + " (b) Remove the stale obj/Release trees, e.g.:" + " Remove-Item -Recurse -Force dotnet/src/Microsoft.Agents.AI*/obj/Release" + " and re-run." + ) -join "`n" + throw $msg + } + Write-Host "Letting publish build ProjectReferences (pass -UsePrebuiltProjectReferences in CI to skip)." -ForegroundColor DarkGray +} + +dotnet publish $TestContainerProject -c Release -f net10.0 -r linux-musl-x64 --self-contained false -o $out @publishExtraArgs --tl:off | Out-Host if ($LASTEXITCODE -ne 0) { throw "dotnet publish failed with exit code $LASTEXITCODE." } diff --git a/dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/ConsoleAppSamplesValidation.cs b/dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/ConsoleAppSamplesValidation.cs index 7b8fa3a8f9..5e1142f027 100644 --- a/dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/ConsoleAppSamplesValidation.cs +++ b/dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/ConsoleAppSamplesValidation.cs @@ -13,8 +13,6 @@ namespace Microsoft.Agents.AI.DurableTask.IntegrationTests; [Trait("Category", "SampleValidation")] public sealed class ConsoleAppSamplesValidation(ITestOutputHelper outputHelper) : SamplesValidationBase(outputHelper) { - private const string SkipFlakyTimingTest = "Flaky: timing-dependent LLM test, see https://github.com/microsoft/agent-framework/issues/4971"; - private static readonly string s_samplesPath = Path.GetFullPath( Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "..", "..", "..", "..", "..", "samples", "04-hosting", "DurableAgents", "ConsoleApps")); @@ -69,7 +67,7 @@ public sealed class ConsoleAppSamplesValidation(ITestOutputHelper outputHelper) }); } - [Fact] + [RetryFact(2, 5000)] public async Task SingleAgentOrchestrationChainingSampleValidationAsync() { using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(); @@ -105,7 +103,7 @@ public sealed class ConsoleAppSamplesValidation(ITestOutputHelper outputHelper) }); } - [Fact] + [RetryFact(2, 5000)] public async Task MultiAgentConcurrencySampleValidationAsync() { using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(); @@ -160,7 +158,7 @@ public sealed class ConsoleAppSamplesValidation(ITestOutputHelper outputHelper) }); } - [Fact] + [RetryFact(2, 5000)] public async Task MultiAgentConditionalSampleValidationAsync() { using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(); @@ -237,14 +235,14 @@ public sealed class ConsoleAppSamplesValidation(ITestOutputHelper outputHelper) Assert.True(foundSuccess, "Orchestration did not complete successfully."); } - [Fact(Skip = SkipFlakyTimingTest)] + [RetryFact(2, 5000)] public async Task SingleAgentOrchestrationHITLSampleValidationAsync() { string samplePath = Path.Combine(s_samplesPath, "05_AgentOrchestration_HITL"); await this.RunSampleTestAsync(samplePath, async (process, logs) => { - using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(); + using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(TimeSpan.FromSeconds(180)); // Start the HITL orchestration following the happy path from README await this.WriteInputAsync(process, "The Future of Artificial Intelligence", testTimeoutCts.Token); @@ -260,7 +258,7 @@ public sealed class ConsoleAppSamplesValidation(ITestOutputHelper outputHelper) while ((line = this.ReadLogLine(logs, testTimeoutCts.Token)) != null) { // Look for notification that content is ready. The first time we see this, we should send a rejection. - // The second time we see this, we should send approval. + // Subsequent times we see this, we should send approval (LLM may produce extra review cycles). if (line.Contains("Content is ready for review", StringComparison.OrdinalIgnoreCase)) { if (!rejectionSent) @@ -275,20 +273,15 @@ public sealed class ConsoleAppSamplesValidation(ITestOutputHelper outputHelper) testTimeoutCts.Token); rejectionSent = true; } - else if (!approvalSent) + else { - // Prompt: Approve? (y/n): + // Approve any subsequent draft (LLM non-determinism may produce extra review cycles) await this.WriteInputAsync(process, "y", testTimeoutCts.Token); // Prompt: Feedback (optional): await this.WriteInputAsync(process, "Looks good!", testTimeoutCts.Token); approvalSent = true; } - else - { - // This should never happen - Assert.Fail("Unexpected message found."); - } } // Look for success message @@ -311,14 +304,14 @@ public sealed class ConsoleAppSamplesValidation(ITestOutputHelper outputHelper) }); } - [Fact(Skip = SkipFlakyTimingTest)] + [RetryFact(2, 5000)] public async Task LongRunningToolsSampleValidationAsync() { string samplePath = Path.Combine(s_samplesPath, "06_LongRunningTools"); await this.RunSampleTestAsync(samplePath, async (process, logs) => { // This test takes a bit longer to run due to the multiple agent interactions and the lengthy content generation. - using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(TimeSpan.FromSeconds(90)); + using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(TimeSpan.FromSeconds(180)); // Test starting an agent that schedules a content generation orchestration await this.WriteInputAsync( @@ -335,7 +328,7 @@ public sealed class ConsoleAppSamplesValidation(ITestOutputHelper outputHelper) while ((line = this.ReadLogLine(logs, testTimeoutCts.Token)) != null) { // Look for notification that content is ready. The first time we see this, we should send a rejection. - // The second time we see this, we should send approval. + // Subsequent times we see this, we should send approval (LLM may produce extra review cycles). if (line.Contains("NOTIFICATION: Please review the following content for approval", StringComparison.OrdinalIgnoreCase)) { // Wait for the notification to be fully written to the console @@ -350,20 +343,15 @@ public sealed class ConsoleAppSamplesValidation(ITestOutputHelper outputHelper) testTimeoutCts.Token); rejectionSent = true; } - else if (!approvalSent) + else { - // Approve the content. Note that we need to send a newline character to the console first before sending the input. + // Approve any subsequent draft (LLM non-determinism may produce extra review cycles) await this.WriteInputAsync( process, "\nApprove the content", testTimeoutCts.Token); approvalSent = true; } - else - { - // This should never happen - Assert.Fail("Unexpected message found."); - } } // Look for success message @@ -396,14 +384,14 @@ public sealed class ConsoleAppSamplesValidation(ITestOutputHelper outputHelper) }); } - [Fact(Skip = SkipFlakyTimingTest)] + [RetryFact(2, 5000)] public async Task ReliableStreamingSampleValidationAsync() { string samplePath = Path.Combine(s_samplesPath, "07_ReliableStreaming"); await this.RunSampleTestAsync(samplePath, async (process, logs) => { // This test takes a bit longer to run due to the multiple agent interactions and the lengthy content generation. - using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(TimeSpan.FromSeconds(90)); + using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(TimeSpan.FromSeconds(150)); // Test the agent endpoint with a simple prompt await this.WriteInputAsync(process, "Plan a 5-day trip to Seattle. Include daily activities.", testTimeoutCts.Token); diff --git a/dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/ExternalClientTests.cs b/dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/ExternalClientTests.cs index 134a12e688..aa1edab7da 100644 --- a/dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/ExternalClientTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/ExternalClientTests.cs @@ -19,11 +19,9 @@ namespace Microsoft.Agents.AI.DurableTask.IntegrationTests; [Trait("Category", "Integration")] public sealed class ExternalClientTests(ITestOutputHelper outputHelper) : IDisposable { - private const string SkipFlakyTimingTest = "Flaky: timing-dependent LLM test, see https://github.com/microsoft/agent-framework/issues/4971"; - private static readonly TimeSpan s_defaultTimeout = Debugger.IsAttached ? TimeSpan.FromMinutes(5) - : TimeSpan.FromSeconds(60); + : TimeSpan.FromSeconds(120); private static readonly IConfiguration s_configuration = new ConfigurationBuilder() @@ -38,7 +36,7 @@ public sealed class ExternalClientTests(ITestOutputHelper outputHelper) : IDispo public void Dispose() => this._cts.Dispose(); - [Fact] + [RetryFact(2, 5000)] public async Task SimplePromptAsync() { // Setup @@ -77,7 +75,7 @@ public sealed class ExternalClientTests(ITestOutputHelper outputHelper) : IDispo Assert.Contains(agentLogs, log => log.EventId.Name == "LogAgentResponse"); } - [Fact(Skip = SkipFlakyTimingTest)] + [RetryFact(2, 5000)] public async Task CallFunctionToolsAsync() { int weatherToolInvocationCount = 0; @@ -129,7 +127,7 @@ public sealed class ExternalClientTests(ITestOutputHelper outputHelper) : IDispo Assert.Equal(1, packingListToolInvocationCount); } - [Fact(Skip = SkipFlakyTimingTest)] + [RetryFact(2, 5000)] public async Task CallLongRunningFunctionToolsAsync() { [Description("Starts a greeting workflow and returns the workflow instance ID")] diff --git a/dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/SamplesValidationBase.cs b/dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/SamplesValidationBase.cs index f5ecf0354d..3f01b83e54 100644 --- a/dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/SamplesValidationBase.cs +++ b/dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/SamplesValidationBase.cs @@ -217,7 +217,7 @@ public abstract class SamplesValidationBase : IAsyncLifetime /// protected CancellationTokenSource CreateTestTimeoutCts(TimeSpan? timeout = null) { - TimeSpan testTimeout = Debugger.IsAttached ? TimeSpan.FromMinutes(5) : timeout ?? TimeSpan.FromSeconds(60); + TimeSpan testTimeout = Debugger.IsAttached ? TimeSpan.FromMinutes(5) : timeout ?? TimeSpan.FromSeconds(120); return new CancellationTokenSource(testTimeout); } diff --git a/dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/WorkflowConsoleAppSamplesValidation.cs b/dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/WorkflowConsoleAppSamplesValidation.cs index f137e4abd9..390b3586ce 100644 --- a/dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/WorkflowConsoleAppSamplesValidation.cs +++ b/dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/WorkflowConsoleAppSamplesValidation.cs @@ -22,7 +22,7 @@ public sealed class WorkflowConsoleAppSamplesValidation(ITestOutputHelper output /// protected override string TaskHubPrefix => "workflow"; - [Fact] + [RetryFact(2, 5000)] public async Task SequentialWorkflowSampleValidationAsync() { using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(s_testTimeout); @@ -71,7 +71,7 @@ public sealed class WorkflowConsoleAppSamplesValidation(ITestOutputHelper output }); } - [Fact] + [RetryFact(2, 5000)] public async Task ConcurrentWorkflowSampleValidationAsync() { using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(s_testTimeout); @@ -120,7 +120,7 @@ public sealed class WorkflowConsoleAppSamplesValidation(ITestOutputHelper output }); } - [Fact] + [RetryFact(2, 5000)] public async Task ConditionalEdgesWorkflowSampleValidationAsync() { using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(s_testTimeout); @@ -182,7 +182,7 @@ public sealed class WorkflowConsoleAppSamplesValidation(ITestOutputHelper output } } - [Fact] + [RetryFact(2, 5000)] public async Task WorkflowEventsSampleValidationAsync() { using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(s_testTimeout); @@ -278,7 +278,7 @@ public sealed class WorkflowConsoleAppSamplesValidation(ITestOutputHelper output }); } - [Fact] + [RetryFact(2, 5000)] public async Task WorkflowSharedStateSampleValidationAsync() { using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(s_testTimeout); @@ -376,7 +376,7 @@ public sealed class WorkflowConsoleAppSamplesValidation(ITestOutputHelper output }); } - [Fact] + [RetryFact(2, 5000)] public async Task SubWorkflowsSampleValidationAsync() { using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(s_testTimeout); @@ -452,7 +452,7 @@ public sealed class WorkflowConsoleAppSamplesValidation(ITestOutputHelper output }); } - [Fact] + [RetryFact(2, 5000)] public async Task WorkflowHITLSampleValidationAsync() { using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(s_testTimeout); @@ -505,7 +505,7 @@ public sealed class WorkflowConsoleAppSamplesValidation(ITestOutputHelper output }); } - [Fact] + [RetryFact(2, 5000)] public async Task WorkflowAndAgentsSampleValidationAsync() { using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(s_testTimeout); diff --git a/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/AgentFrameworkResponseHandlerTelemetryTests.cs b/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/AgentFrameworkResponseHandlerTelemetryTests.cs index dcb4e7e212..360c065eee 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/AgentFrameworkResponseHandlerTelemetryTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/AgentFrameworkResponseHandlerTelemetryTests.cs @@ -37,7 +37,7 @@ public class AgentFrameworkResponseHandlerTelemetryTests public async Task CreateAsync_DefaultAgent_EmitsInvokeAgentSpanAsync() { // Arrange - var activities = new List(); + var activities = new ConcurrentActivityList(); using var tracerProvider = Sdk.CreateTracerProviderBuilder() .AddSource(ResponsesSourceName) .AddInMemoryExporter(activities) @@ -56,7 +56,7 @@ public class AgentFrameworkResponseHandlerTelemetryTests await foreach (var _ in handler.CreateAsync(request, context, CancellationToken.None)) { } // Assert — filter by agent name to isolate this test's span from any parallel test spans - var mySpan = Assert.Single(activities.Where(a => TelemetryTestAgent.AgentName.Equals(a.GetTagItem("gen_ai.agent.name"))).ToList()); + var mySpan = Assert.Single(activities.Snapshot().Where(a => TelemetryTestAgent.AgentName.Equals(a.GetTagItem("gen_ai.agent.name"))).ToList()); Assert.Equal("invoke_agent", mySpan.GetTagItem("gen_ai.operation.name")); Assert.NotNull(mySpan.GetTagItem("gen_ai.agent.id")); } @@ -65,7 +65,7 @@ public class AgentFrameworkResponseHandlerTelemetryTests public async Task CreateAsync_KeyedAgent_EmitsInvokeAgentSpanAsync() { // Arrange - var activities = new List(); + var activities = new ConcurrentActivityList(); using var tracerProvider = Sdk.CreateTracerProviderBuilder() .AddSource(ResponsesSourceName) .AddInMemoryExporter(activities) @@ -84,7 +84,7 @@ public class AgentFrameworkResponseHandlerTelemetryTests await foreach (var _ in handler.CreateAsync(request, context, CancellationToken.None)) { } // Assert — filter by agent name to isolate this test's span - var mySpan = Assert.Single(activities.Where(a => TelemetryTestAgent.AgentName.Equals(a.GetTagItem("gen_ai.agent.name"))).ToList()); + var mySpan = Assert.Single(activities.Snapshot().Where(a => TelemetryTestAgent.AgentName.Equals(a.GetTagItem("gen_ai.agent.name"))).ToList()); Assert.Equal("invoke_agent", mySpan.GetTagItem("gen_ai.operation.name")); } @@ -95,8 +95,8 @@ public class AgentFrameworkResponseHandlerTelemetryTests // If ApplyOpenTelemetry double-wraps, an extra span would appear on ResponsesSourceName. // If it correctly skips wrapping, only the pre-wrap's unique source emits spans. var preWrapSource = Guid.NewGuid().ToString(); - var preWrapActivities = new List(); - var responsesActivities = new List(); + var preWrapActivities = new ConcurrentActivityList(); + var responsesActivities = new ConcurrentActivityList(); using var preWrapProvider = Sdk.CreateTracerProviderBuilder() .AddSource(preWrapSource) @@ -125,18 +125,19 @@ public class AgentFrameworkResponseHandlerTelemetryTests await foreach (var _ in handler.CreateAsync(request, context, CancellationToken.None)) { } // Assert — pre-wrap source emits exactly 1 span (agent ran) - Assert.Single(preWrapActivities); - Assert.Equal("invoke_agent", preWrapActivities[0].GetTagItem("gen_ai.operation.name")); + var preWrapSnapshot = preWrapActivities.Snapshot(); + Assert.Single(preWrapSnapshot); + Assert.Equal("invoke_agent", preWrapSnapshot[0].GetTagItem("gen_ai.operation.name")); // ResponsesSourceName emits 0 spans — ApplyOpenTelemetry skipped wrapping the pre-instrumented agent - Assert.DoesNotContain(responsesActivities, a => TelemetryTestAgent.AgentName.Equals(a.GetTagItem("gen_ai.agent.name"))); + Assert.DoesNotContain(responsesActivities.Snapshot(), a => TelemetryTestAgent.AgentName.Equals(a.GetTagItem("gen_ai.agent.name"))); } [Fact] public async Task CreateAsync_DefaultAgent_SpanDisplayNameContainsAgentNameAsync() { // Arrange - var activities = new List(); + var activities = new ConcurrentActivityList(); using var tracerProvider = Sdk.CreateTracerProviderBuilder() .AddSource(ResponsesSourceName) .AddInMemoryExporter(activities) @@ -155,7 +156,7 @@ public class AgentFrameworkResponseHandlerTelemetryTests await foreach (var _ in handler.CreateAsync(request, context, CancellationToken.None)) { } // Assert — display name follows "invoke_agent {Name}({Id})" convention; filter by agent name to isolate - var mySpan = Assert.Single(activities.Where(a => TelemetryTestAgent.AgentName.Equals(a.GetTagItem("gen_ai.agent.name"))).ToList()); + var mySpan = Assert.Single(activities.Snapshot().Where(a => TelemetryTestAgent.AgentName.Equals(a.GetTagItem("gen_ai.agent.name"))).ToList()); Assert.Contains("invoke_agent", mySpan.DisplayName, StringComparison.Ordinal); Assert.Contains(TelemetryTestAgent.AgentName, mySpan.DisplayName, StringComparison.Ordinal); } @@ -231,4 +232,35 @@ public class AgentFrameworkResponseHandlerTelemetryTests } private sealed class TelemetryAgentSession : AgentSession; + + /// + /// Thread-safe used by OTel's InMemoryExporter to capture + /// activities emitted on globally-listened sources. Required because the exporter writes into + /// the supplied collection from background Activity completion callbacks while the test thread + /// may be enumerating it for assertions, and other tests in the same assembly may emit on the + /// same source concurrently. A plain trips + /// "Collection was modified; enumeration operation may not execute." in that scenario. + /// + private sealed class ConcurrentActivityList : ICollection + { + private readonly List _items = new(); + private readonly object _gate = new(); + + public int Count { get { lock (this._gate) { return this._items.Count; } } } + public bool IsReadOnly => false; + + public void Add(Activity item) { lock (this._gate) { this._items.Add(item); } } + public void Clear() { lock (this._gate) { this._items.Clear(); } } + public bool Contains(Activity item) { lock (this._gate) { return this._items.Contains(item); } } + public void CopyTo(Activity[] array, int arrayIndex) { lock (this._gate) { this._items.CopyTo(array, arrayIndex); } } + public bool Remove(Activity item) { lock (this._gate) { return this._items.Remove(item); } } + + public Activity[] Snapshot() + { + lock (this._gate) { return this._items.ToArray(); } + } + + public IEnumerator GetEnumerator() => ((IEnumerable)this.Snapshot()).GetEnumerator(); + System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() => this.GetEnumerator(); + } } diff --git a/dotnet/tests/Microsoft.Agents.AI.Hosting.AzureFunctions.IntegrationTests/SamplesValidation.cs b/dotnet/tests/Microsoft.Agents.AI.Hosting.AzureFunctions.IntegrationTests/SamplesValidation.cs index 078b6af790..be9d2b7434 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Hosting.AzureFunctions.IntegrationTests/SamplesValidation.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Hosting.AzureFunctions.IntegrationTests/SamplesValidation.cs @@ -15,8 +15,6 @@ namespace Microsoft.Agents.AI.Hosting.AzureFunctions.IntegrationTests; [Trait("Category", "SampleValidation")] public sealed class SamplesValidation(ITestOutputHelper outputHelper) : IAsyncLifetime { - private const string SkipFlakyTimingTest = "Flaky: timing-dependent LLM test, see https://github.com/microsoft/agent-framework/issues/4971"; - private const string AzureFunctionsPort = "7071"; private const string AzuritePort = "10000"; private const string DtsPort = "8080"; @@ -37,7 +35,7 @@ public sealed class SamplesValidation(ITestOutputHelper outputHelper) : IAsyncLi .Build(); private static bool s_infrastructureStarted; - private static readonly TimeSpan s_orchestrationTimeout = TimeSpan.FromMinutes(2); + private static readonly TimeSpan s_orchestrationTimeout = TimeSpan.FromMinutes(3); // In CI, `dotnet run` builds the Functions project from scratch before the host starts, so 60s is not enough. private static readonly TimeSpan s_functionsReadyTimeout = TimeSpan.FromSeconds(180); @@ -62,7 +60,7 @@ public sealed class SamplesValidation(ITestOutputHelper outputHelper) : IAsyncLi await Task.CompletedTask; } - [Fact] + [RetryFact(2, 5000)] public async Task SingleAgentSampleValidationAsync() { string samplePath = Path.Combine(s_samplesPath, "01_SingleAgent"); @@ -107,7 +105,7 @@ public sealed class SamplesValidation(ITestOutputHelper outputHelper) : IAsyncLi }); } - [Fact] + [Fact(Skip = "Flaky: LLM non-determinism can produce null orchestration results")] public async Task SingleAgentOrchestrationChainingSampleValidationAsync() { string samplePath = Path.Combine(s_samplesPath, "02_AgentOrchestration_Chaining"); @@ -150,7 +148,7 @@ public sealed class SamplesValidation(ITestOutputHelper outputHelper) : IAsyncLi }); } - [Fact] + [RetryFact(2, 5000)] public async Task MultiAgentOrchestrationConcurrentSampleValidationAsync() { string samplePath = Path.Combine(s_samplesPath, "03_AgentOrchestration_Concurrency"); @@ -200,7 +198,7 @@ public sealed class SamplesValidation(ITestOutputHelper outputHelper) : IAsyncLi }); } - [Fact] + [RetryFact(2, 5000)] public async Task MultiAgentOrchestrationConditionalsSampleValidationAsync() { string samplePath = Path.Combine(s_samplesPath, "04_AgentOrchestration_Conditionals"); @@ -218,7 +216,7 @@ public sealed class SamplesValidation(ITestOutputHelper outputHelper) : IAsyncLi }); } - [Fact] + [RetryFact(2, 5000)] public async Task SingleAgentOrchestrationHITLSampleValidationAsync() { string samplePath = Path.Combine(s_samplesPath, "05_AgentOrchestration_HITL"); @@ -274,7 +272,7 @@ public sealed class SamplesValidation(ITestOutputHelper outputHelper) : IAsyncLi }); } - [Fact(Skip = SkipFlakyTimingTest)] + [RetryFact(2, 5000)] public async Task LongRunningToolsSampleValidationAsync() { string samplePath = Path.Combine(s_samplesPath, "06_LongRunningTools"); @@ -316,7 +314,7 @@ public sealed class SamplesValidation(ITestOutputHelper outputHelper) : IAsyncLi } }, message: "Orchestration is requesting human feedback", - timeout: TimeSpan.FromSeconds(60)); + timeout: TimeSpan.FromSeconds(180)); // Approve the content Uri approvalUri = new($"{runAgentUri}?thread_id={sessionId}"); @@ -336,7 +334,7 @@ public sealed class SamplesValidation(ITestOutputHelper outputHelper) : IAsyncLi } }, message: "Content published notification is logged", - timeout: TimeSpan.FromSeconds(60)); + timeout: TimeSpan.FromSeconds(180)); // Verify the final orchestration status by asking the agent for the status Uri statusUri = new($"{runAgentUri}?thread_id={sessionId}"); @@ -360,11 +358,11 @@ public sealed class SamplesValidation(ITestOutputHelper outputHelper) : IAsyncLi return isCompleted && hasContent; }, message: "Orchestration is completed", - timeout: TimeSpan.FromSeconds(60)); + timeout: TimeSpan.FromSeconds(180)); }); } - [Fact] + [RetryFact(2, 5000)] public async Task AgentAsMcpToolAsync() { string samplePath = Path.Combine(s_samplesPath, "07_AgentAsMcpTool"); @@ -404,7 +402,7 @@ public sealed class SamplesValidation(ITestOutputHelper outputHelper) : IAsyncLi }); } - [Fact(Skip = SkipFlakyTimingTest)] + [RetryFact(2, 5000)] public async Task ReliableStreamingSampleValidationAsync() { string samplePath = Path.Combine(s_samplesPath, "08_ReliableStreaming"); diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/MagenticManagerTests.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/MagenticManagerTests.cs new file mode 100644 index 0000000000..9014463133 --- /dev/null +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/MagenticManagerTests.cs @@ -0,0 +1,153 @@ +// Copyright (c) Microsoft. All rights reserved. + +//using System.Collections.Generic; +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using FluentAssertions; +using Microsoft.Agents.AI.Workflows.Specialized.Magentic; +using Microsoft.Extensions.AI; + +namespace Microsoft.Agents.AI.Workflows.UnitTests; + +public class MagenticManagerTests +{ + private static void CheckMessage(ChatMessage message, string expectedText, bool runPropertySmokeTest = false, bool skipCreatedAt = true) + { + message.Text.Should().Be(expectedText); + + if (runPropertySmokeTest) + { + message.AuthorName.Should().Be(nameof(MagenticOrchestrator)); + + if (!skipCreatedAt) + { + message.CreatedAt.Should().NotBeNull().And.NotBeBefore(DateTimeOffset.UtcNow.AddDays(-1)); + } + + message.Role.Should().Be(ChatRole.Assistant); + message.MessageId.Should().NotBeNull(); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Test_MagenticManager_UpdatePlanAsync(bool hasExistingPlan) + { + TestReplayAgent testAgent = new(name: nameof(MagenticOrchestrator), + messages: + [ + [new(ChatRole.Assistant, "Facts")], + [new(ChatRole.Assistant, "Plan")], + ]); + + TestEchoAgent participant = new(name: "Echo"); + MagenticManager manager = new(testAgent); + + MagenticTaskContext taskContext = new([new(ChatRole.User, "Task")], [participant], new TaskLimits(), null, []); + if (hasExistingPlan) + { + taskContext.TaskLedger = new(new(ChatRole.Assistant, "OldFacts"), new(ChatRole.Assistant, "OldPlan")); + } + + TestRunContext runContext = new(); + IWorkflowContext workflowContext = runContext.BindWorkflowContext(nameof(MagenticOrchestrator)); + + TaskLedger newPlan = await manager.UpdatePlanAsync(taskContext, workflowContext, CancellationToken.None); + CheckMessage(newPlan.CurrentFacts, "Facts"); + CheckMessage(newPlan.CurrentPlan, "Plan"); + + taskContext.ChatHistory.Should().HaveCount(4); + + if (hasExistingPlan) + { + ChatMessage factsRequest = taskContext.ChatHistory[0]; + factsRequest.Text.Should().Contain("OldFacts"); + } + + ChatMessage facts = taskContext.ChatHistory[1]; + facts.Should().Be(newPlan.CurrentFacts); + + ChatMessage plan = taskContext.ChatHistory[3]; + plan.Should().Be(newPlan.CurrentPlan); + } + + [Theory] + [InlineData(0)] + [InlineData(1)] + [InlineData(2)] + [InlineData(3)] + [InlineData(4)] + public async Task Test_MagenticManager_UpdateProgressLedgerAsync(int failures) + { + List> turns = + TestProgressLedgerState.MissingRequired.Take(failures) + .Select>( + state => [new ChatMessage(ChatRole.Assistant, state.ToJsonString())]) + .ToList(); + + turns.Should().HaveCount(failures); + turns.Add([new ChatMessage(ChatRole.Assistant, TestProgressLedgerState.Default.ToJsonString())]); + + TestReplayAgent testAgent = new(name: nameof(MagenticOrchestrator), + messages: turns); + + TestEchoAgent participant = new(name: "Echo"); + MagenticManager manager = new(testAgent); + + MagenticTaskContext taskContext = new([new(ChatRole.User, "Task")], [participant], new TaskLimits(), null, []); + taskContext.TaskLedger = new(new(ChatRole.Assistant, "OldFacts"), new(ChatRole.Assistant, "OldPlan")); + + TestRunContext runContext = new(); + IWorkflowContext workflowContext = runContext.BindWorkflowContext(nameof(MagenticOrchestrator)); + + // Precondition check: ProgressLedger should be not "started" + taskContext.ProgressLedger.IsStarted.Should().BeFalse(); + + Func action = () => manager.UpdateProgressLedgerAsync(taskContext, workflowContext, CancellationToken.None).AsTask(); + + if (failures >= taskContext.TaskLimits.MaxProgressLedgerRetryCount) + { + // We expect to see an exception if the number of failures exceeds the maximum retry count + await action.Should().ThrowAsync(); + taskContext.ProgressLedger.IsStarted.Should().BeFalse(); + } + else + { + await action.Should().NotThrowAsync(); + taskContext.ProgressLedger.IsStarted.Should().BeTrue(); + TestProgressLedgerState.Default.Validate(taskContext.ProgressLedger); + } + + int expectedWarnings = Math.Min(failures, 3); + + runContext.Events.Should().HaveCount(expectedWarnings).And.AllBeOfType(); + } + + [Fact] + public async Task Test_MagenticManager_PrepareFinalAnswerAsync() + { + TestReplayAgent testAgent = new(name: nameof(MagenticOrchestrator), + messages: + [ + [ + new(ChatRole.Assistant, "FinalAnswer") + ], + ]); + + TestEchoAgent participant = new(name: "Echo"); + MagenticManager manager = new(testAgent); + + MagenticTaskContext taskContext = new([new(ChatRole.User, "Task")], [participant], new TaskLimits(), null, []); + + TestRunContext runContext = new(); + IWorkflowContext workflowContext = runContext.BindWorkflowContext(nameof(MagenticOrchestrator)); + + ChatMessage answer = await manager.PrepareFinalAnswerAsync(taskContext, workflowContext, CancellationToken.None); + + CheckMessage(answer, "FinalAnswer", true, false); + } +} diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/MagenticProgressLedgerTests.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/MagenticProgressLedgerTests.cs new file mode 100644 index 0000000000..fdc66f69d9 --- /dev/null +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/MagenticProgressLedgerTests.cs @@ -0,0 +1,214 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Text.Json; +using FluentAssertions; +using Microsoft.Agents.AI.Workflows.Specialized.Magentic; +using Microsoft.Extensions.AI; + +namespace Microsoft.Agents.AI.Workflows.UnitTests; + +public class MagenticProgressLedgerTests +{ + public record KVPair(string key); + public record AnswerReasonPair(bool answer, string reason); + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void Test_ExtractJson_SucceedsWhenInBlockQuote(bool isTagged) + { + // Arrange + string json = isTagged + ? "```json\n{\"key\": \"value\"}\n```" + : "```{\"key\": \"value\"}```"; + + string embedded = $"Some text before the JSON block.\n{json}\nSome text after the JSON block."; + ChatMessage message = new(ChatRole.Assistant, embedded); + + // Act + JsonElement element = message.ExtractJson(); + + // Assert + KVPair? result = element.Deserialize(); + + result.Should().NotBeNull(); + result.key.Should().Be("value"); + } + + [Fact] + public void Test_ExtractJson_SucceedsWhenScanning() + { + // Arrange + ChatMessage message = new(ChatRole.Assistant, +""" + Some text before the JSON embed. + {"key": "value"} + + Some text after the JSON embed. +"""); + + // Act + JsonElement element = message.ExtractJson(); + + // Assert + KVPair? result = element.Deserialize(); + + result.Should().NotBeNull(); + result.key.Should().Be("value"); + } + + [Fact] + public void Test_ExtractJson_FailsWhenUnbalanced() + { + // Arrange + ChatMessage message = new(ChatRole.Assistant, +""" + Some text before the JSON embed. + {"key": { "key2": "value" } + + Some text after the JSON embed. +"""); + + // Act + Func action = () => message.ExtractJson(); + + // Assert + action.Should().Throw(); + } + + [Fact] + public void Test_ExtractJson_FailsWhenNoJson() + { + // Arrange + ChatMessage message = new(ChatRole.Assistant, +""" + Some text, without JSON +"""); + + // Act + Func action = () => message.ExtractJson(); + + // Assert + action.Should().Throw(); + } + + [Fact] + public void Test_ExtractJson_SuceedsWithQuotesBrackets() + { + // Arrange + ChatMessage message = new(ChatRole.Assistant, +""" + {"reason":"the output contained }", "answer": false} +"""); + + // Act + JsonElement element = message.ExtractJson(); + + // Assert + AnswerReasonPair? result = element.Deserialize(); + + result.Should().NotBeNull(); + result.reason.Should().Be("the output contained }"); + result.answer.Should().BeFalse(); + } + + public static readonly string TestTeamNames = string.Join(", ", ["CodingAgent", "CodeExecutor", "WebSurferAgent", "FileSurferAgent"]); + + [Fact] + public void Test_ProgressLedgerState_IsEmptyWhenStarted() + { + // Arrange/Act + MagenticProgressLedger ledger = new(TestTeamNames, []); + + // Assert + ledger.State.Should().BeNull(); + ledger.IsStarted.Should().BeFalse(); + + ledger.TryGetCurrentSlotValue(TestProgressLedgerState.CustomSlot1, out _).Should().BeFalse(); + ledger.TryGetCurrentSlotValue(TestProgressLedgerState.CustomSlot2, out _).Should().BeFalse(); + } + + [Theory] + [InlineData(0, "RequiredOnly")] + [InlineData(1, "IncludeCustom")] + public void Test_ProgressLedgerState_IsNotEmptyWhenRestored(int caseIndex, string _) + { + // Arrange + TestProgressLedgerState state = TestProgressLedgerState.Working[caseIndex]; + JsonElement element = state.ToJson(); + + // Act + MagenticProgressLedger ledger = new(TestTeamNames, [], element); + + // Assert + ledger.State.Should().Be(element); + state.Validate(ledger); + } + + [Theory] + [InlineData(0, "RequiredOnly")] + [InlineData(1, "IncludeCustom")] + public void Test_ProgressLedgerState_SwitchesToStartedWhenStateUpdates(int caseIndex, string _) + { + // Arrange + MagenticProgressLedger ledger = new(TestTeamNames, []); + TestProgressLedgerState targetState = TestProgressLedgerState.Working[caseIndex]; + JsonElement element = targetState.ToJson(); + ledger.State.Should().BeNull(); + + // Act + ledger.TryUpdateState(element).Should().BeTrue(); + + // Assert + ledger.State.Should().Be(element); + targetState.Validate(ledger); + } + + [Theory] + [InlineData(0, "is_request_satisfied")] + [InlineData(1, "is_in_loop")] + [InlineData(2, "is_progress_being_made")] + [InlineData(3, "instruction_or_question")] + [InlineData(4, "next_speaker")] + public void Test_ProgressLedgerState_FailsToUpdateWhenRequiredAnswersMissing(int caseIndex, string _) + { + // Arrange + MagenticProgressLedger ledger = new(TestTeamNames, []); + TestProgressLedgerState targetState = TestProgressLedgerState.MissingRequired[caseIndex]; + JsonElement element = targetState.ToJson(); + ledger.State.Should().BeNull(); + + // Act + ledger.TryUpdateState(element).Should().BeFalse(); + ledger.State.Should().BeNull(); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void Test_ProgressLedgerState_GeneratesCorrectSchema(bool includeCustom) + { + // Arrange + MagenticProgressLedger ledger = new(TestTeamNames, includeCustom + ? [TestProgressLedgerState.CustomSlot1, TestProgressLedgerState.CustomSlot2] + : []); + + // Act + (string questionBlock, string answerSchema) = ledger.FormatQuestions(); + + foreach (ProgressLedgerSlot slot in ledger.Slots) + { + // Best-efforts validation: I do not want to make it super-brittle and check for 1:1: with the template + // since that is effectively checking that string formatting works right to some extent. + questionBlock.Should().Contain(slot.Question); + answerSchema.Should().Contain(slot.Key); + answerSchema.Should().Contain(slot.SchemaType); + + if (!string.IsNullOrWhiteSpace(slot.SchemaTypeSuffix)) + { + answerSchema.Should().Contain(slot.SchemaTypeSuffix); + } + } + } +} diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/TestProgressLedgerState.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/TestProgressLedgerState.cs new file mode 100644 index 0000000000..fc7e9ded2d --- /dev/null +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/TestProgressLedgerState.cs @@ -0,0 +1,88 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System.Text.Json; +using System.Text.Json.Serialization; +using FluentAssertions; + +namespace Microsoft.Agents.AI.Workflows.UnitTests; + +public sealed record Slot(T? answer, string? reason); + +public record TestProgressLedgerState(Slot? is_request_satisfied = null, + Slot? is_in_loop = null, + Slot? is_progress_being_made = null, + Slot? instruction_or_question = null, + Slot? next_speaker = null, + Slot? custom1 = null, + Slot? custom2 = null) +{ + public TestProgressLedgerState() : this(new Slot(false, "is_request_satisfied_reason"), + new Slot(false, "is_in_loop_reason"), + new Slot(false, "is_progress_being_made_reason"), + new Slot("Answer", "instruction_or_question_reason"), + new Slot("Lorem Ipsum", "next_speaker_reason"), + new Slot(false, "custom1_reason"), + new Slot("Custom2", "custom2_reason")) + { } + + public string ToJsonString() => this.ToJson().ToString(); + + private static readonly JsonSerializerOptions s_options = new() + { + DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull + }; + + public JsonElement ToJson() => JsonSerializer.SerializeToElement(this, s_options); + + internal static BooleanProgressLedgerSlot CustomSlot1 = new("custom1", "Custom Slot 1"); + internal static StringProgressLedgerSlot CustomSlot2 = new("custom2", "Custom Slot 2"); + + public static bool TryGetCustom1(MagenticProgressLedger state, out bool result) + => state.TryGetCurrentSlotValue(CustomSlot1, out result); + + public static bool TryGetCustom2(MagenticProgressLedger state, out string? result) + => state.TryGetCurrentSlotValue(CustomSlot2, out result); + + public void Validate(MagenticProgressLedger state) + { + state.IsRequestSatisfied.Should().Be(this.is_request_satisfied!.answer!.Value); + state.IsInLoop.Should().Be(this.is_in_loop!.answer!.Value); + state.IsProgressBeingMade.Should().Be(this.is_progress_being_made!.answer!.Value); + state.InstructionOrQuestion.Should().Be(this.instruction_or_question!.answer); + state.NextSpeaker.Should().Be(this.next_speaker!.answer); + + if (this.custom1 != null) + { + TryGetCustom1(state, out bool custom1Value).Should().BeTrue(); + custom1Value.Should().Be(this.custom1.answer!.Value); + } + else + { + TryGetCustom1(state, out _).Should().BeFalse(); + } + + if (this.custom2 != null) + { + TryGetCustom2(state, out string? custom2Value).Should().BeTrue(); + custom2Value.Should().Be(this.custom2.answer); + } + else + { + TryGetCustom2(state, out _).Should().BeFalse(); + } + } + + public static readonly TestProgressLedgerState Default = new(); + public static readonly TestProgressLedgerState RequiredOnly = Default with { custom1 = null, custom2 = null }; + + public static readonly TestProgressLedgerState[] Working = [RequiredOnly, Default]; + + public static readonly TestProgressLedgerState[] MissingRequired = + [ + Default with { is_request_satisfied = null }, + Default with { is_in_loop = null}, + Default with { is_progress_being_made = null}, + Default with { instruction_or_question = null}, + Default with { next_speaker = null}, + ]; +} diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/TestReplayAgent.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/TestReplayAgent.cs index 3a117b9492..8373b844ed 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/TestReplayAgent.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/TestReplayAgent.cs @@ -11,8 +11,14 @@ using Microsoft.Extensions.AI; namespace Microsoft.Agents.AI.Workflows.UnitTests; -public class TestReplayAgent(List? messages = null, string? id = null, string? name = null) : AIAgent +public class TestReplayAgent(List> messages, string? id = null, string? name = null) : AIAgent { + public TestReplayAgent(List messages, string? id = null, string? name = null) : this([messages ?? []], id, name) + { } + + public TestReplayAgent(string? id = null, string? name = null) : this([[]], id, name) + { } + protected override string? IdCore => id; public override string? Name => name; @@ -57,46 +63,55 @@ public class TestReplayAgent(List? messages = null, string? id = nu public static TestReplayAgent FromStrings(params string[] messages) => new(ToChatMessages(messages)); - public List Messages { get; } = Validate(messages) ?? []; + public List> Messages { get; } = Validate(messages) ?? []; protected override Task RunCoreAsync(IEnumerable messages, AgentSession? session = null, AgentRunOptions? options = null, CancellationToken cancellationToken = default) => this.RunStreamingAsync(messages, session, options, cancellationToken).ToAgentResponseAsync(cancellationToken); + public int Turn { get; set; } + protected override async IAsyncEnumerable RunCoreStreamingAsync(IEnumerable messages, AgentSession? session = null, AgentRunOptions? options = null, [EnumeratorCancellation] CancellationToken cancellationToken = default) { string responseId = Guid.NewGuid().ToString("N"); - foreach (ChatMessage message in this.Messages) + + if (this.Turn < this.Messages.Count) { - foreach (AIContent content in message.Contents) + foreach (ChatMessage message in this.Messages[this.Turn++]) { - yield return new AgentResponseUpdate() + foreach (AIContent content in message.Contents) { - AgentId = this.Id, - AuthorName = this.Name, - MessageId = message.MessageId, - ResponseId = responseId, - Contents = [content], - Role = message.Role, - }; + yield return new AgentResponseUpdate() + { + AgentId = this.Id, + AuthorName = this.Name, + MessageId = message.MessageId, + ResponseId = responseId, + Contents = [content], + Role = message.Role, + }; + } } } } - private static List? Validate(List? candidateMessages) + private static List>? Validate(List>? candidateMessages) { - string? currentMessageId = null; + string? lastMessageId = null; - if (candidateMessages is not null) + if (candidateMessages != null) { - foreach (ChatMessage message in candidateMessages) + foreach (List candidateMessagesTurn in candidateMessages) { - if (currentMessageId is null) + foreach (ChatMessage message in candidateMessagesTurn) { - currentMessageId = message.MessageId; - } - else if (currentMessageId == message.MessageId) - { - throw new ArgumentException("Duplicate consecutive message ids"); + if (lastMessageId is null || lastMessageId != message.MessageId) + { + lastMessageId = message.MessageId; + } + else if (lastMessageId == message.MessageId) + { + throw new ArgumentException("Duplicate consecutive message ids"); + } } } } diff --git a/python/packages/core/agent_framework/__init__.py b/python/packages/core/agent_framework/__init__.py index 4592f8c716..db1c43abfe 100644 --- a/python/packages/core/agent_framework/__init__.py +++ b/python/packages/core/agent_framework/__init__.py @@ -135,6 +135,7 @@ from ._sessions import ( from ._settings import SecretString, load_settings from ._skills import ( AggregatingSkillsSource, + ClassSkill, DeduplicatingSkillsSource, DelegatingSkillsSource, FileSkill, @@ -345,6 +346,7 @@ __all__ = [ "ChatResponseUpdate", "CheckResult", "CheckpointStorage", + "ClassSkill", "CompactionProvider", "CompactionStrategy", "Content", diff --git a/python/packages/core/agent_framework/_skills.py b/python/packages/core/agent_framework/_skills.py index 06612b4df0..a1d5634e18 100644 --- a/python/packages/core/agent_framework/_skills.py +++ b/python/packages/core/agent_framework/_skills.py @@ -5,7 +5,7 @@ Defines the core data model classes for the agent skills system: - **Skills:** :class:`Skill` (abstract base), :class:`InlineSkill` (code-defined), - and :class:`FileSkill` (filesystem-backed). + :class:`ClassSkill` (class-based), and :class:`FileSkill` (filesystem-backed). - **Resources:** :class:`SkillResource` (abstract base), :class:`InlineSkillResource` (static content or callable). - **Scripts:** :class:`SkillScript` (abstract base), :class:`InlineSkillScript` @@ -27,6 +27,9 @@ Skills can come from different sources: Represented as :class:`FileSkill` instances. - **Code-defined** — created as :class:`InlineSkill` instances in Python code, with optional callable resources attached via the ``@skill.resource`` decorator. +- **Class-based** — created by subclassing :class:`ClassSkill` to define + self-contained, reusable skill types with ``create_resource()`` and + ``create_script()`` factory methods. - **Custom sources** — any :class:`SkillsSource` implementation that provides skills from arbitrary origins (REST APIs, databases, etc.). @@ -570,6 +573,65 @@ def _validate_skill_description(name: str, description: str) -> None: ) +def _build_skill_content( + name: str, + description: str, + instructions: str, + resources: Sequence[SkillResource] | None = None, + scripts: Sequence[SkillScript] | None = None, +) -> str: + """Build XML-structured content for code-defined and class-based skills. + + Produces an XML document containing name, description, instructions, + resources, and scripts elements. Used by both :class:`InlineSkill` + and :class:`ClassSkill` to generate their ``content`` property. + + Args: + name: The skill name. + description: The skill description. + instructions: The raw instructions text. + resources: Optional resources associated with the skill. + scripts: Optional scripts associated with the skill. + + Returns: + An XML-structured content string. + """ + result = ( + f"{xml_escape(name)}\n" + f"{xml_escape(description)}\n" + "\n" + "\n" + f"{instructions}\n" + "" + ) + + if resources: + resource_lines = "\n".join(_create_resource_element(r) for r in resources) + result += f"\n\n\n{resource_lines}\n" + + if scripts: + script_lines = "\n".join(_create_script_element(s) for s in scripts) + result += f"\n\n\n{script_lines}\n" + + return result + + +def _create_resource_element(resource: SkillResource) -> str: + """Create a self-closing ```` XML element from a :class:`SkillResource`. + + Args: + resource: The resource to create the element from. + + Returns: + A single indented XML element string with ``name`` and optional + ``description`` attributes. + """ + attrs = f'name="{xml_escape(resource.name, quote=True)}"' + if resource.description: + attrs += f' description="{xml_escape(resource.description, quote=True)}"' + return f" " + + @experimental(feature_id=ExperimentalFeature.SKILLS) class InlineSkill(Skill): """A skill defined entirely in code with resources and scripts. @@ -634,25 +696,10 @@ class InlineSkill(Skill): if self._cached_content is not None: return self._cached_content - result = ( - f"{xml_escape(self.name)}\n" - f"{xml_escape(self.description)}\n" - "\n" - "\n" - f"{self.instructions}\n" - "" + self._cached_content = _build_skill_content( + self.name, self.description, self.instructions, self._resources, self._scripts ) - - if self._resources: - resource_lines = "\n".join(self._create_resource_element(r) for r in self._resources) - result += f"\n\n\n{resource_lines}\n" - - if self._scripts: - script_lines = "\n".join(_create_script_element(s) for s in self._scripts) - result += f"\n\n\n{script_lines}\n" - - self._cached_content = result - return result + return self._cached_content @property def resources(self) -> list[SkillResource]: @@ -664,22 +711,6 @@ class InlineSkill(Skill): """Mutable list of :class:`SkillScript` instances.""" return self._scripts - @staticmethod - def _create_resource_element(resource: SkillResource) -> str: - """Create a self-closing ```` XML element from an :class:`SkillResource`. - - Args: - resource: The resource to create the element from. - - Returns: - A single indented XML element string with ``name`` and optional - ``description`` attributes. - """ - attrs = f'name="{xml_escape(resource.name, quote=True)}"' - if resource.description: - attrs += f' description="{xml_escape(resource.description, quote=True)}"' - return f" " - def resource( self, func: Callable[..., Any] | None = None, @@ -700,8 +731,7 @@ class InlineSkill(Skill): Keyword Args: name: Resource name override. Defaults to ``func.__name__``. - description: Resource description override. Defaults to the - function's docstring (via :func:`inspect.getdoc`). + description: Resource description override. Defaults to ``None``. Returns: The original function unchanged, or a secondary decorator when @@ -727,7 +757,7 @@ class InlineSkill(Skill): def decorator(f: Callable[..., Any]) -> Callable[..., Any]: resource_name = name or f.__name__ - resource_description = description or (inspect.getdoc(f) or None) + resource_description = description self._resources.append( InlineSkillResource( name=resource_name, @@ -761,8 +791,7 @@ class InlineSkill(Skill): Keyword Args: name: Script name override. Defaults to ``func.__name__``. - description: Script description override. Defaults to the - function's docstring (via :func:`inspect.getdoc`). + description: Script description override. Defaults to ``None``. Returns: The original function unchanged, or a secondary decorator when @@ -789,7 +818,7 @@ class InlineSkill(Skill): def decorator(f: Callable[..., Any]) -> Callable[..., Any]: script_name = name or f.__name__ - script_description = description or (inspect.getdoc(f) or None) + script_description = description self._scripts.append( InlineSkillScript( name=script_name, @@ -804,6 +833,420 @@ class InlineSkill(Skill): return decorator(func) +def _make_method_name(method_name: str) -> str: + """Convert a Python method name to a skill resource/script name. + + Replaces underscores with hyphens to match the skill naming convention. + + Args: + method_name: The Python method name (e.g. ``"conversion_table"``). + + Returns: + The converted name (e.g. ``"conversion-table"``). + """ + return method_name.replace("_", "-").strip("-") + + +def _validate_member_name(name: str, kind: str) -> None: + """Validate a resource or script name at decoration time. + + Args: + name: The name to validate. + kind: ``"resource"`` or ``"script"`` — used in error messages. + + Raises: + ValueError: If the name is empty, too long, or contains invalid characters. + """ + if not name or not name.strip(): + raise ValueError(f"@ClassSkill.{kind} name cannot be empty.") + if len(name) > MAX_NAME_LENGTH or not VALID_NAME_RE.match(name): + raise ValueError( + f"Invalid @ClassSkill.{kind} name '{name}': Must be {MAX_NAME_LENGTH} characters or fewer, " + "using only lowercase letters, numbers, and hyphens, and must not start or end with a hyphen " + "or contain consecutive hyphens." + ) + + +def _discover_marked_members(cls: type, marker_attr: str) -> list[tuple[str, dict[str, Any]]]: + """Scan a class for methods or properties stamped with a marker attribute. + + Checks both regular callable attributes (via ``dir``) and ``property`` + descriptors (via ``cls.__dict__``) whose ``fget`` carries the marker. + + Args: + cls: The class to scan. + marker_attr: The marker attribute name to look for (e.g. + ``"_skill_resource_marker"``). + + Returns: + A list of ``(member_name, marker_dict)`` tuples. + """ + results: list[tuple[str, dict[str, Any]]] = [] + seen: set[str] = set() + + # Walk the MRO so that property-resources defined on a parent class + # are also discovered. ``cls.__dict__`` only sees the leaf class. + for klass in cls.__mro__: + for attr_name, attr_value in klass.__dict__.items(): + if attr_name in seen: + continue + if ( + isinstance(attr_value, property) + and attr_value.fget is not None + and hasattr(attr_value.fget, marker_attr) + ): + results.append((attr_name, getattr(attr_value.fget, marker_attr))) + seen.add(attr_name) + + # Check regular callable attributes. + for attr_name in dir(cls): + if attr_name in seen: + continue + try: + attr = getattr(cls, attr_name, None) + except Exception: + # Some descriptors (e.g. abstract properties) may raise on access. + logger.warning("Skipping '%s' during skill discovery: descriptor raised on access", attr_name) + attr = None + if attr is not None and callable(attr) and hasattr(attr, marker_attr): + results.append((attr_name, getattr(attr, marker_attr))) + return results + + +@experimental(feature_id=ExperimentalFeature.SKILLS) +class ClassSkill(Skill, ABC): + """Abstract base class for defining skills as reusable Python classes. + + Inherit from this class to create a self-contained skill definition. + Override :attr:`instructions` to provide the skill body. + + Resources and scripts can be defined in two ways: + + - **Decorator-based (recommended):** Mark methods with + :meth:`ClassSkill.resource` and :meth:`ClassSkill.script` decorators + for automatic discovery. + - **Explicit override:** Override the :attr:`resources` and + :attr:`scripts` properties, constructing :class:`InlineSkillResource` + and :class:`InlineSkillScript` instances directly. + + Class-based skills can be distributed via shared libraries or PyPI + packages, making them easy to reuse across projects. + + Attributes: + name: Skill name (lowercase letters, numbers, hyphens only). + description: Human-readable description of the skill. + + Examples: + Decorator-based (recommended): + + .. code-block:: python + + class UnitConverterSkill(ClassSkill): + def __init__(self) -> None: + super().__init__( + name="unit-converter", + description="Convert between common units.", + ) + + @property + def instructions(self) -> str: + return "Use this skill to convert units..." + + @ClassSkill.resource(name="table") + def conversion_table(self) -> str: + return "| From | To | Factor |..." + + @ClassSkill.script(name="convert") + def convert(self, value: float, factor: float) -> str: + return json.dumps({"result": round(value * factor, 4)}) + + Explicit override: + + .. code-block:: python + + class UnitConverterSkill(ClassSkill): + def __init__(self) -> None: + super().__init__( + name="unit-converter", + description="Convert between common units.", + ) + + @property + def instructions(self) -> str: + return "Use this skill to convert units..." + + @property + def resources(self) -> list[SkillResource]: + return [ + InlineSkillResource(name="table", content="| From | To | Factor |..."), + ] + + @property + def scripts(self) -> list[SkillScript]: + return [InlineSkillScript(name="convert", function=convert_fn)] + """ + + def __init__( + self, + *, + name: str, + description: str, + ) -> None: + """Initialize a ClassSkill. + + Args: + name: Skill name (lowercase letters, numbers, hyphens only; + max 64 characters). + description: Human-readable description of the skill + (≤1024 characters). + """ + super().__init__(name=name, description=description) + self._cached_content: str | None = None + self._cached_resources: list[SkillResource] | None = None + self._cached_scripts: list[SkillScript] | None = None + + @staticmethod + def resource( + func: Callable[..., Any] | None = None, + *, + name: str | None = None, + description: str | None = None, + ) -> Any: + """Decorator that marks a method or property as a skill resource for auto-discovery. + + When applied to a method or property on a :class:`ClassSkill` subclass, + it is automatically discovered and registered as an + :class:`InlineSkillResource`. Methods are invoked each time the + resource is read. Properties are evaluated via their getter. + + Can be applied to a method directly, or stacked with ``@property`` + (place ``@property`` first, ``@ClassSkill.resource`` second). + + Supports bare usage (``@ClassSkill.resource``) and parameterized usage + (``@ClassSkill.resource(name="custom", description="...")``). + + Args: + func: The function being decorated. Populated automatically when + the decorator is applied without parentheses. + + Keyword Args: + name: Resource name override. Defaults to the method name with + underscores replaced by hyphens. + description: Resource description. Defaults to ``None``. + + Examples: + On a method: + + .. code-block:: python + + @ClassSkill.resource(name="conversion-table") + def get_table(self) -> str: + return "..." + + On a property: + + .. code-block:: python + + @property + @ClassSkill.resource + def conversion_table(self) -> str: + return "..." + """ + + def decorator(f: Callable[..., Any]) -> Callable[..., Any]: + if isinstance(f, (property, classmethod, staticmethod)): + raise TypeError( + "@ClassSkill.resource must be applied before @property, @classmethod, or @staticmethod. " + "Place @property first, then @ClassSkill.resource." + ) + if name is not None: + _validate_member_name(name, "resource") + f._skill_resource_marker = { # type: ignore[attr-defined] + "name": name, + "description": description, + } + return f + + if func is None: + return decorator + return decorator(func) + + @staticmethod + def script( + func: Callable[..., Any] | None = None, + *, + name: str | None = None, + description: str | None = None, + ) -> Any: + """Decorator that marks a method as a skill script for auto-discovery. + + When applied to a method on a :class:`ClassSkill` subclass, the method is + automatically discovered and registered as an :class:`InlineSkillScript`. + The method's parameters (excluding ``self``) are used to generate a JSON + schema, and the method is invoked in-process when the script is run. + + Supports bare usage (``@ClassSkill.script``) and parameterized usage + (``@ClassSkill.script(name="custom", description="...")``). + + Args: + func: The function being decorated. Populated automatically when + the decorator is applied without parentheses. + + Keyword Args: + name: Script name override. Defaults to the method name with + underscores replaced by hyphens. + description: Script description. Defaults to ``None``. + + Examples: + .. code-block:: python + + @ClassSkill.script(name="convert") + def convert(self, value: float, factor: float) -> str: + return json.dumps({"result": round(value * factor, 4)}) + """ + + def decorator(f: Callable[..., Any]) -> Callable[..., Any]: + if isinstance(f, (property, classmethod, staticmethod)): + raise TypeError( + "@ClassSkill.script must be applied before @property, @classmethod, or @staticmethod." + ) + if name is not None: + _validate_member_name(name, "script") + f._skill_script_marker = { # type: ignore[attr-defined] + "name": name, + "description": description, + } + return f + + if func is None: + return decorator + return decorator(func) + + @property + @abstractmethod + def instructions(self) -> str: + """The raw instructions text for this skill. + + Subclasses must override this property to provide the skill body. + """ + ... + + @property + def resources(self) -> list[SkillResource]: + """Resources discovered from :meth:`ClassSkill.resource`-decorated methods. + + On first access, scans the class for methods marked with the + :meth:`ClassSkill.resource` decorator and instantiates + :class:`InlineSkillResource` instances from them. + The result is cached after the first access. + + Override this property to provide resources explicitly instead of + using decorator-based discovery. + """ + if self._cached_resources is not None: + return list(self._cached_resources) + + resources: list[SkillResource] = [] + seen_names: set[str] = set() + + for attr_name, attr in _discover_marked_members(type(self), "_skill_resource_marker"): + marker: dict[str, Any] = attr + resource_name = marker.get("name") or _make_method_name(attr_name) + if resource_name in seen_names: + raise ValueError( + f"Skill '{self.name}' already has a resource named '{resource_name}'. " + "Ensure each @ClassSkill.resource has a unique name." + ) + seen_names.add(resource_name) + + # Use inspect.getattr_static to check the descriptor type without + # triggering it, and walk the MRO so inherited properties are found. + static_attr = inspect.getattr_static(self, attr_name, None) + is_property = isinstance(static_attr, property) + resource_description = marker.get("description") + + if is_property: + # Property — use a lambda that reads the property value each time. + # We capture attr_name to avoid late-binding issues. + # Do NOT call getattr here to avoid triggering the getter during discovery. + resource_func = (lambda name: lambda: getattr(self, name))(attr_name) + resources.append( + InlineSkillResource( + name=resource_name, + function=resource_func, + description=resource_description, + ) + ) + else: + # Regular method — use the bound method directly. + bound_method = getattr(self, attr_name) + resources.append( + InlineSkillResource( + name=resource_name, + function=bound_method, + description=resource_description, + ) + ) + + self._cached_resources = resources + return list(self._cached_resources) + + @property + def scripts(self) -> list[SkillScript]: + """Scripts discovered from :meth:`ClassSkill.script`-decorated methods. + + On first access, scans the class for methods marked with the + :meth:`ClassSkill.script` decorator and instantiates + :class:`InlineSkillScript` instances from them. + The result is cached after the first access. + + Override this property to provide scripts explicitly instead of + using decorator-based discovery. + """ + if self._cached_scripts is not None: + return list(self._cached_scripts) + + scripts: list[SkillScript] = [] + seen_names: set[str] = set() + + for attr_name, attr in _discover_marked_members(type(self), "_skill_script_marker"): + marker: dict[str, Any] = attr + script_name = marker.get("name") or _make_method_name(attr_name) + if script_name in seen_names: + raise ValueError( + f"Skill '{self.name}' already has a script named '{script_name}'. " + "Ensure each @ClassSkill.script has a unique name." + ) + seen_names.add(script_name) + + bound_method = getattr(self, attr_name) + script_description = marker.get("description") + scripts.append( + InlineSkillScript( + name=script_name, + function=bound_method, + description=script_description, + ) + ) + + self._cached_scripts = scripts + return list(self._cached_scripts) + + @property + def content(self) -> str: + """Synthesized XML content containing name, description, instructions, resources, and scripts. + + The result is cached after the first access. + """ + if self._cached_content is not None: + return self._cached_content + + self._cached_content = _build_skill_content( + self.name, self.description, self.instructions, self.resources, self.scripts + ) + return self._cached_content + + @experimental(feature_id=ExperimentalFeature.SKILLS) class FileSkill(Skill): """A :class:`Skill` discovered from a filesystem directory backed by a SKILL.md file. diff --git a/python/packages/core/tests/core/test_skills.py b/python/packages/core/tests/core/test_skills.py index 8e8c6a8aed..b268b31551 100644 --- a/python/packages/core/tests/core/test_skills.py +++ b/python/packages/core/tests/core/test_skills.py @@ -5,6 +5,7 @@ from __future__ import annotations import os +from abc import ABC from collections.abc import Sequence from pathlib import Path from typing import Any @@ -14,6 +15,7 @@ import pytest from agent_framework import ( AggregatingSkillsSource, + ClassSkill, DeduplicatingSkillsSource, FileSkill, FileSkillScript, @@ -32,6 +34,7 @@ from agent_framework._skills import ( DEFAULT_SCRIPT_EXTENSIONS, InlineSkillResource, InlineSkillScript, + _create_resource_element, _create_script_element, _FileSkillResource, ) @@ -1004,7 +1007,7 @@ class TestInlineSkill: assert len(skill.resources) == 1 assert skill.resources[0].name == "get_schema" - assert skill.resources[0].description == "Get the database schema." + assert skill.resources[0].description is None assert isinstance(skill.resources[0], InlineSkillResource) assert skill.resources[0].function is get_schema @@ -1677,22 +1680,22 @@ class TestCreateResourceElement: def test_name_only(self) -> None: r = InlineSkillResource(name="my-ref", content="data") - elem = InlineSkill._create_resource_element(r) + elem = _create_resource_element(r) assert elem == ' ' def test_with_description(self) -> None: r = InlineSkillResource(name="my-ref", description="A reference.", content="data") - elem = InlineSkill._create_resource_element(r) + elem = _create_resource_element(r) assert elem == ' ' def test_xml_escapes_name(self) -> None: r = InlineSkillResource(name='ref"special', content="data") - elem = InlineSkill._create_resource_element(r) + elem = _create_resource_element(r) assert """ in elem def test_xml_escapes_description(self) -> None: r = InlineSkillResource(name="ref", description='Uses & "quotes"', content="data") - elem = InlineSkill._create_resource_element(r) + elem = _create_resource_element(r) assert "<tags>" in elem assert "&" in elem assert """ in elem @@ -2136,8 +2139,8 @@ class TestSkillResourceDecoratorEdgeCases: return "data" assert skill.resources[0].name == "custom-name" - # description falls back to docstring - assert skill.resources[0].description == "Some docs." + # description is None when not explicitly provided + assert skill.resources[0].description is None def test_decorator_with_description_only(self) -> None: skill = InlineSkill(name="my-skill", description="A skill.", instructions="Body") @@ -2320,7 +2323,7 @@ class TestSkillScriptDecorator: assert len(skill.scripts) == 1 assert skill.scripts[0].name == "analyze" - assert skill.scripts[0].description == "Run analysis." + assert skill.scripts[0].description is None assert isinstance(skill.scripts[0], InlineSkillScript) assert skill.scripts[0].function is analyze @@ -3177,6 +3180,757 @@ class TestLoadSkillWithScripts: result = provider._load_skill(_raw_skills(provider), "my-skill") assert "" not in result + +# --------------------------------------------------------------------------- +# Tests: ClassSkill +# --------------------------------------------------------------------------- + + +class _MinimalClassSkill(ClassSkill): + """A minimal class-based skill with no resources or scripts.""" + + def __init__(self) -> None: + super().__init__(name="minimal-skill", description="A minimal skill.") + + @property + def instructions(self) -> str: + return "Do minimal things." + + +class _FullClassSkill(ClassSkill): + """A class-based skill with resources and scripts.""" + + def __init__(self) -> None: + super().__init__(name="full-skill", description="A full skill.") + self._resources: list[SkillResource] | None = None + self._scripts: list[SkillScript] | None = None + + @property + def instructions(self) -> str: + return "Use this skill for full tasks." + + @property + def resources(self) -> list[SkillResource]: + if self._resources is None: + self._resources = [ + InlineSkillResource(name="test-resource", content="Static resource content."), + ] + return self._resources + + @property + def scripts(self) -> list[SkillScript]: + if self._scripts is None: + self._scripts = [ + InlineSkillScript(name="test-script", function=_class_skill_test_fn), + ] + return self._scripts + + +def _class_skill_test_fn(value: float, factor: float) -> str: + """Multiply value by factor.""" + import json as _json + + return _json.dumps({"result": round(value * factor, 4)}) + + +class TestClassSkill: + """Tests for ClassSkill abstract base class.""" + + def test_minimal_skill_has_no_resources(self) -> None: + skill = _MinimalClassSkill() + assert skill.resources == [] + + def test_minimal_skill_has_no_scripts(self) -> None: + skill = _MinimalClassSkill() + assert skill.scripts == [] + + def test_minimal_skill_content_contains_name(self) -> None: + skill = _MinimalClassSkill() + assert "minimal-skill" in skill.content + + def test_minimal_skill_content_contains_description(self) -> None: + skill = _MinimalClassSkill() + assert "A minimal skill." in skill.content + + def test_minimal_skill_content_contains_instructions(self) -> None: + skill = _MinimalClassSkill() + assert "Do minimal things." in skill.content + + def test_minimal_skill_content_no_resources_element(self) -> None: + skill = _MinimalClassSkill() + assert "" not in skill.content + + def test_minimal_skill_content_no_scripts_element(self) -> None: + skill = _MinimalClassSkill() + assert "" not in skill.content + + def test_full_skill_has_resources(self) -> None: + skill = _FullClassSkill() + assert len(skill.resources) == 1 + assert skill.resources[0].name == "test-resource" + + def test_full_skill_has_scripts(self) -> None: + skill = _FullClassSkill() + assert len(skill.scripts) == 1 + assert skill.scripts[0].name == "test-script" + + def test_full_skill_content_contains_resources(self) -> None: + skill = _FullClassSkill() + assert "" in skill.content + assert 'name="test-resource"' in skill.content + + def test_full_skill_content_contains_scripts(self) -> None: + skill = _FullClassSkill() + assert "" in skill.content + assert 'name="test-script"' in skill.content + + def test_content_is_cached(self) -> None: + skill = _MinimalClassSkill() + content1 = skill.content + content2 = skill.content + assert content1 is content2 + + def test_resources_are_lazy_cached(self) -> None: + skill = _FullClassSkill() + resources1 = skill.resources + resources2 = skill.resources + assert resources1 is resources2 + + def test_scripts_are_lazy_cached(self) -> None: + skill = _FullClassSkill() + scripts1 = skill.scripts + scripts2 = skill.scripts + assert scripts1 is scripts2 + + def test_script_has_parameters_schema(self) -> None: + skill = _FullClassSkill() + script = skill.scripts[0] + assert isinstance(script, InlineSkillScript) + schema = script.parameters_schema + assert schema is not None + assert "value" in schema.get("properties", {}) + assert "factor" in schema.get("properties", {}) + + async def test_provider_with_class_skill(self) -> None: + skill = _FullClassSkill() + provider = SkillsProvider([skill]) + await _init_provider(provider) + + skills = _raw_skills(provider) + assert len(skills) == 1 + assert skills[0].name == "full-skill" + + async def test_provider_loads_class_skill_content(self) -> None: + skill = _FullClassSkill() + provider = SkillsProvider([skill]) + await _init_provider(provider) + + result = provider._load_skill(_raw_skills(provider), "full-skill") + assert "Use this skill for full tasks." in result + assert "" in result + assert "" in result + + async def test_in_memory_source_with_class_skill(self) -> None: + skill = _MinimalClassSkill() + source = InMemorySkillsSource([skill]) + skills = await source.get_skills() + assert len(skills) == 1 + assert skills[0].name == "minimal-skill" + + async def test_mixed_inline_and_class_skills(self) -> None: + inline = InlineSkill(name="inline-skill", description="Inline", instructions="inline body") + class_skill = _MinimalClassSkill() + provider = SkillsProvider([inline, class_skill]) + await _init_provider(provider) + + skills = _raw_skills(provider) + names = {s.name for s in skills} + assert names == {"inline-skill", "minimal-skill"} + + async def test_class_skill_script_runs(self) -> None: + skill = _FullClassSkill() + script = skill.scripts[0] + result = await script.run(skill, {"value": 10.0, "factor": 2.5}) + import json as _json + + parsed = _json.loads(result) + assert parsed["result"] == 25.0 + + async def test_class_skill_resource_reads(self) -> None: + skill = _FullClassSkill() + resource = skill.resources[0] + content = await resource.read() + assert content == "Static resource content." + + +# --------------------------------------------------------------------------- +# Tests: ClassSkill with decorator-based discovery +# --------------------------------------------------------------------------- + + +class _DecoratorClassSkill(ClassSkill): + """A class-based skill using @ClassSkill.resource and @ClassSkill.script decorators.""" + + def __init__(self) -> None: + super().__init__(name="decorator-skill", description="A decorator-discovered skill.") + + @property + def instructions(self) -> str: + return "Use this skill for decorator tests." + + @ClassSkill.resource(name="lookup-table") + def get_table(self) -> str: + """Conversion lookup table.""" + return "| From | To | Factor |" + + @ClassSkill.script(name="convert") + def run_convert(self, value: float, factor: float) -> str: + """Convert a value.""" + import json as _json + + return _json.dumps({"result": round(value * factor, 4)}) + + +class _BareDecoratorSkill(ClassSkill): + """Skill using bare decorators (no arguments) — name/description from method.""" + + def __init__(self) -> None: + super().__init__(name="bare-skill", description="Bare decorator skill.") + + @property + def instructions(self) -> str: + return "Bare instructions." + + @ClassSkill.resource + def my_table(self) -> str: + """The table docs.""" + return "table content" + + @ClassSkill.script + def my_script(self, x: int) -> int: + """Double x.""" + return x * 2 + + +class _DuplicateResourceSkill(ClassSkill): + """Skill with duplicate resource names — should raise.""" + + def __init__(self) -> None: + super().__init__(name="dup-skill", description="Dup.") + + @property + def instructions(self) -> str: + return "x" + + @ClassSkill.resource(name="same-name") + def res_a(self) -> str: + return "a" + + @ClassSkill.resource(name="same-name") + def res_b(self) -> str: + return "b" + + +class _DuplicateScriptSkill(ClassSkill): + """Skill with duplicate script names — should raise.""" + + def __init__(self) -> None: + super().__init__(name="dup-script-skill", description="Dup.") + + @property + def instructions(self) -> str: + return "x" + + @ClassSkill.script(name="same-name") + def script_a(self, x: int) -> int: + return x + + @ClassSkill.script(name="same-name") + def script_b(self, x: int) -> int: + return x + + +class _SelfAccessSkill(ClassSkill): + """Skill where resource/script access instance state via self.""" + + def __init__(self, multiplier: int = 10) -> None: + super().__init__(name="self-access", description="Self access skill.") + self.multiplier = multiplier + + @property + def instructions(self) -> str: + return "Use multiplier." + + @ClassSkill.resource(name="config") + def get_config(self) -> str: + return f"multiplier={self.multiplier}" + + @ClassSkill.script(name="multiply") + def multiply(self, value: int) -> int: + return value * self.multiplier + + +class TestClassSkillDecoratorDiscovery: + """Tests for decorator-based resource/script discovery on ClassSkill.""" + + def test_discovers_resources(self) -> None: + skill = _DecoratorClassSkill() + assert len(skill.resources) == 1 + assert skill.resources[0].name == "lookup-table" + + def test_discovers_scripts(self) -> None: + skill = _DecoratorClassSkill() + assert len(skill.scripts) == 1 + assert skill.scripts[0].name == "convert" + + def test_resource_description_from_decorator(self) -> None: + skill = _DecoratorClassSkill() + assert skill.resources[0].description is None + + def test_script_description_from_decorator(self) -> None: + skill = _DecoratorClassSkill() + assert skill.scripts[0].description is None + + def test_bare_decorator_name_from_method(self) -> None: + skill = _BareDecoratorSkill() + assert skill.resources[0].name == "my-table" + assert skill.scripts[0].name == "my-script" + + def test_bare_decorator_description_is_none(self) -> None: + skill = _BareDecoratorSkill() + assert skill.resources[0].description is None + assert skill.scripts[0].description is None + + async def test_resource_reads(self) -> None: + skill = _DecoratorClassSkill() + content = await skill.resources[0].read() + assert content == "| From | To | Factor |" + + async def test_script_runs(self) -> None: + skill = _DecoratorClassSkill() + import json as _json + + result = await skill.scripts[0].run(skill, {"value": 10.0, "factor": 2.5}) + parsed = _json.loads(result) + assert parsed["result"] == 25.0 + + def test_script_schema_excludes_self(self) -> None: + skill = _DecoratorClassSkill() + script = skill.scripts[0] + assert isinstance(script, InlineSkillScript) + schema = script.parameters_schema + assert schema is not None + props = schema.get("properties", {}) + assert "self" not in props + assert "value" in props + assert "factor" in props + + def test_resources_cached(self) -> None: + skill = _DecoratorClassSkill() + r1 = skill.resources + r2 = skill.resources + assert r1 == r2 + assert r1 is not r2 # defensive copy + + def test_scripts_cached(self) -> None: + skill = _DecoratorClassSkill() + s1 = skill.scripts + s2 = skill.scripts + assert s1 == s2 + assert s1 is not s2 # defensive copy + + def test_content_includes_discovered_resources(self) -> None: + skill = _DecoratorClassSkill() + assert "" in skill.content + assert 'name="lookup-table"' in skill.content + + def test_content_includes_discovered_scripts(self) -> None: + skill = _DecoratorClassSkill() + assert "" in skill.content + assert 'name="convert"' in skill.content + + def test_duplicate_resource_name_raises(self) -> None: + skill = _DuplicateResourceSkill() + with pytest.raises(ValueError, match="already has a resource named"): + _ = skill.resources + + def test_duplicate_script_name_raises(self) -> None: + skill = _DuplicateScriptSkill() + with pytest.raises(ValueError, match="already has a script named"): + _ = skill.scripts + + async def test_self_access_resource(self) -> None: + skill = _SelfAccessSkill(multiplier=42) + content = await skill.resources[0].read() + assert content == "multiplier=42" + + async def test_self_access_script(self) -> None: + skill = _SelfAccessSkill(multiplier=3) + result = await skill.scripts[0].run(skill, {"value": 7}) + assert result == 21 + + def test_no_decorators_yields_empty(self) -> None: + skill = _MinimalClassSkill() + assert skill.resources == [] + assert skill.scripts == [] + + async def test_provider_with_decorator_skill(self) -> None: + skill = _DecoratorClassSkill() + provider = SkillsProvider([skill]) + await _init_provider(provider) + + skills = _raw_skills(provider) + assert len(skills) == 1 + assert skills[0].name == "decorator-skill" + + def test_manual_override_wins(self) -> None: + """A subclass that overrides resources/scripts bypasses decorator discovery.""" + skill = _FullClassSkill() + assert len(skill.resources) == 1 + assert skill.resources[0].name == "test-resource" + + async def test_property_resource_reads(self) -> None: + """@ClassSkill.resource on a @property works correctly.""" + skill = _PropertyResourceSkill() + assert len(skill.resources) == 1 + assert skill.resources[0].name == "static-table" + content = await skill.resources[0].read() + assert "miles" in content + + def test_property_resource_description_is_none_without_explicit(self) -> None: + skill = _PropertyResourceSkill() + assert skill.resources[0].description is None + + def test_property_resource_in_content(self) -> None: + skill = _PropertyResourceSkill() + assert 'name="static-table"' in skill.content + + async def test_mixed_property_and_method_resources(self) -> None: + """Property and method resources can coexist.""" + skill = _MixedPropertyMethodSkill() + names = {r.name for r in skill.resources} + assert names == {"prop-data", "method-data"} + for r in skill.resources: + content = await r.read() + assert "content" in content.lower() + + def test_explicit_resource_description_in_object(self) -> None: + """Explicit description= on @ClassSkill.resource is stored on the object.""" + skill = _ExplicitDescriptionSkill() + res = next(r for r in skill.resources if r.name == "described-res") + assert res.description == "A described resource." + + def test_explicit_script_description_in_object(self) -> None: + """Explicit description= on @ClassSkill.script is stored on the object.""" + skill = _ExplicitDescriptionSkill() + scr = next(s for s in skill.scripts if s.name == "described-scr") + assert scr.description == "A described script." + + def test_explicit_description_in_content_xml(self) -> None: + """Explicit descriptions appear in the skill content XML.""" + skill = _ExplicitDescriptionSkill() + assert 'description="A described resource."' in skill.content + assert 'description="A described script."' in skill.content + + def test_property_getter_not_called_during_discovery(self) -> None: + """Property getter must NOT be evaluated when resources are discovered.""" + skill = _PropertyCallCountSkill() + assert skill.getter_call_count == 0 + _ = skill.resources # discovery should NOT call the getter + assert skill.getter_call_count == 0 + + async def test_property_getter_called_on_read(self) -> None: + """Property getter IS evaluated when the resource is read.""" + skill = _PropertyCallCountSkill() + _ = skill.resources + assert skill.getter_call_count == 0 + await skill.resources[0].read() + assert skill.getter_call_count == 1 + + def test_make_method_name_strips_leading_trailing_hyphens(self) -> None: + """_make_method_name strips leading/trailing underscores turned to hyphens.""" + from agent_framework._skills import _make_method_name + + assert _make_method_name("my_method") == "my-method" + assert _make_method_name("_private_method_") == "private-method" + assert _make_method_name("__dunder__") == "dunder" + assert _make_method_name("already_good") == "already-good" + + def test_inherited_decorated_resources_are_discovered(self) -> None: + """Decorated resources from a parent class are discovered on subclass.""" + skill = _ChildSkill() + names = {r.name for r in skill.resources} + assert "parent-data" in names + + def test_inherited_decorated_scripts_are_discovered(self) -> None: + """Decorated scripts from a parent class are discovered on subclass.""" + skill = _ChildSkill() + names = {s.name for s in skill.scripts} + assert "parent-action" in names + + def test_child_can_add_own_resources(self) -> None: + """A child class can add resources alongside inherited ones.""" + skill = _ChildSkill() + names = {r.name for r in skill.resources} + assert "parent-data" in names + assert "child-data" in names + + async def test_script_receives_kwargs(self) -> None: + """ClassSkill scripts receive **kwargs forwarded from the runtime.""" + skill = _KwargsSkill() + script = skill.scripts[0] + result = await script.run(skill, {"x": 5}, custom_key="hello") + assert result == "5-hello" + + def test_wrong_decorator_order_resource_raises(self) -> None: + """@ClassSkill.resource above @property raises TypeError at class definition.""" + with pytest.raises(TypeError, match="must be applied before @property"): + + class _BadOrder(ClassSkill): + def __init__(self) -> None: + super().__init__(name="bad", description="bad") + + @property + def instructions(self) -> str: + return "x" + + @ClassSkill.resource(name="oops") # wrong: should be below @property + @property + def bad_prop(self) -> str: + return "x" + + def test_wrong_decorator_order_script_raises(self) -> None: + """@ClassSkill.script on a property raises TypeError.""" + with pytest.raises(TypeError, match="must be applied before"): + + class _BadOrder(ClassSkill): + def __init__(self) -> None: + super().__init__(name="bad", description="bad") + + @property + def instructions(self) -> str: + return "x" + + @ClassSkill.script(name="oops") + @property + def bad_prop(self) -> str: + return "x" + + def test_invalid_explicit_resource_name_raises(self) -> None: + """Invalid name= on @ClassSkill.resource raises ValueError at decoration.""" + with pytest.raises(ValueError, match="Invalid @ClassSkill.resource name"): + + class _BadName(ClassSkill): + def __init__(self) -> None: + super().__init__(name="bad", description="bad") + + @property + def instructions(self) -> str: + return "x" + + @ClassSkill.resource(name="UPPER CASE!") + def res(self) -> str: + return "x" + + def test_invalid_explicit_script_name_raises(self) -> None: + """Invalid name= on @ClassSkill.script raises ValueError at decoration.""" + with pytest.raises(ValueError, match="Invalid @ClassSkill.script name"): + + class _BadName(ClassSkill): + def __init__(self) -> None: + super().__init__(name="bad", description="bad") + + @property + def instructions(self) -> str: + return "x" + + @ClassSkill.script(name="has spaces") + def scr(self, x: int) -> int: + return x + + def test_empty_explicit_name_raises(self) -> None: + """Empty name= on @ClassSkill.resource raises ValueError.""" + with pytest.raises(ValueError, match="name cannot be empty"): + + class _EmptyName(ClassSkill): + def __init__(self) -> None: + super().__init__(name="bad", description="bad") + + @property + def instructions(self) -> str: + return "x" + + @ClassSkill.resource(name="") + def res(self) -> str: + return "x" + + def test_resources_copy_prevents_cache_mutation(self) -> None: + """Mutating the returned resources list does not affect the cache.""" + skill = _DecoratorClassSkill() + r1 = skill.resources + r1.clear() + r2 = skill.resources + assert len(r2) == 1 # original cached list is intact + + def test_scripts_copy_prevents_cache_mutation(self) -> None: + """Mutating the returned scripts list does not affect the cache.""" + skill = _DecoratorClassSkill() + s1 = skill.scripts + s1.clear() + s2 = skill.scripts + assert len(s2) == 1 # original cached list is intact + + async def test_inherited_property_resource_discovered(self) -> None: + """A @property @ClassSkill.resource on a parent class is discovered on child.""" + skill = _ChildWithInheritedPropertySkill() + names = {r.name for r in skill.resources} + assert "parent-prop" in names + content = await next(r for r in skill.resources if r.name == "parent-prop").read() + assert content == "parent property content" + + +# --------------------------------------------------------------------------- +# Helper skills for additional tests +# --------------------------------------------------------------------------- + + +class _ExplicitDescriptionSkill(ClassSkill): + """Skill with explicit descriptions on decorator.""" + + def __init__(self) -> None: + super().__init__(name="desc-skill", description="Explicit desc.") + + @property + def instructions(self) -> str: + return "x" + + @ClassSkill.resource(name="described-res", description="A described resource.") + def res(self) -> str: + return "data" + + @ClassSkill.script(name="described-scr", description="A described script.") + def scr(self, x: int) -> int: + return x + + +class _PropertyCallCountSkill(ClassSkill): + """Tracks how many times the property getter is called.""" + + def __init__(self) -> None: + super().__init__(name="callcount-skill", description="Tracks calls.") + self.getter_call_count = 0 + + @property + def instructions(self) -> str: + return "x" + + @property + @ClassSkill.resource(name="counted") + def counted_resource(self) -> str: + self.getter_call_count += 1 + return "counted" + + +class _ParentSkill(ClassSkill, ABC): + """Parent with decorated resources/scripts.""" + + @ClassSkill.resource(name="parent-data") + def parent_resource(self) -> str: + return "parent" + + @ClassSkill.script(name="parent-action") + def parent_script(self, x: int) -> int: + return x + + +class _ChildSkill(_ParentSkill): + """Child inheriting parent resources and adding its own.""" + + def __init__(self) -> None: + super().__init__(name="child-skill", description="Child.") + + @property + def instructions(self) -> str: + return "child" + + @ClassSkill.resource(name="child-data") + def child_resource(self) -> str: + return "child" + + +class _KwargsSkill(ClassSkill): + """Skill that uses **kwargs from runtime.""" + + def __init__(self) -> None: + super().__init__(name="kwargs-skill", description="Kwargs.") + + @property + def instructions(self) -> str: + return "x" + + @ClassSkill.script(name="echo") + def echo(self, x: int, **kwargs: Any) -> str: + return f"{x}-{kwargs.get('custom_key', 'none')}" + + +class _ParentWithPropertyResource(ClassSkill, ABC): + """Parent with a property-based resource.""" + + @property + @ClassSkill.resource(name="parent-prop") + def parent_property(self) -> str: + return "parent property content" + + +class _ChildWithInheritedPropertySkill(_ParentWithPropertyResource): + """Child that should discover inherited property resource.""" + + def __init__(self) -> None: + super().__init__(name="child-prop-skill", description="Child prop.") + + @property + def instructions(self) -> str: + return "x" + + +class _PropertyResourceSkill(ClassSkill): + """Skill with a property-based resource.""" + + def __init__(self) -> None: + super().__init__(name="prop-skill", description="Property skill.") + + @property + def instructions(self) -> str: + return "Use this skill." + + @property + @ClassSkill.resource(name="static-table") + def conversion_table(self) -> str: + """Static conversion table.""" + return "| miles | km | 1.60934 |" + + +class _MixedPropertyMethodSkill(ClassSkill): + """Skill with both property and method resources.""" + + def __init__(self) -> None: + super().__init__(name="mixed-prop", description="Mixed.") + + @property + def instructions(self) -> str: + return "x" + + @property + @ClassSkill.resource(name="prop-data") + def static_data(self) -> str: + """Static content.""" + return "Property Content" + + @ClassSkill.resource(name="method-data") + def dynamic_data(self) -> str: + """Dynamic content.""" + return "Method Content" + async def test_code_skill_scripts_element_contains_parameters(self) -> None: """Scripts XML includes parameters schema when the function has typed parameters.""" diff --git a/python/samples/02-agents/skills/README.md b/python/samples/02-agents/skills/README.md index b401278577..6e9bb08202 100644 --- a/python/samples/02-agents/skills/README.md +++ b/python/samples/02-agents/skills/README.md @@ -10,7 +10,8 @@ Start with file-based or code-defined skills, then explore combining them and ad |--------|-------------| | [**file_based_skill**](file_based_skill/) | Define skills as `SKILL.md` files on disk with reference documents and executable scripts. Uses the unit-converter skill. | | [**code_defined_skill**](code_defined_skill/) | Define skills entirely in Python code using `Skill`, `@skill.resource`, and `@skill.script` decorators. Uses a code-defined unit-converter skill. | -| [**mixed_skills**](mixed_skills/) | Combine code-defined and file-based skills in a single agent. Uses a code-defined volume-converter and a file-based unit-converter. | +| [**class_based_skill**](class_based_skill/) | Define skills as Python classes using `ClassSkill` with `@ClassSkill.resource` and `@ClassSkill.script` decorators for auto-discovery. Uses a class-based unit-converter skill. | +| [**mixed_skills**](mixed_skills/) | Combine code-defined, class-based, and file-based skills in a single agent. Uses a code-defined volume-converter, a class-based temperature-converter, and a file-based unit-converter. | | [**script_approval**](script_approval/) | Require human-in-the-loop approval before executing skill scripts | ## Key Concepts @@ -23,17 +24,18 @@ Skills use a three-step interaction model to minimize token usage: 2. **Load** — Full instructions are loaded on-demand via the `load_skill` tool 3. **Access** — Resources are read via `read_skill_resource`; scripts are executed via `run_skill_script` -### File-Based vs Code-Defined Skills +### File-Based vs Code-Defined vs Class-Based Skills -| Aspect | File-Based | Code-Defined | -|--------|-----------|--------------| -| Definition | `SKILL.md` files on disk | `Skill` instances in Python | -| Resources | Static files in `references/` and `assets/` directories | Callable functions via `@skill.resource` decorator | -| Scripts | Python files in `scripts/` directory (executed via subprocess) | Callable functions via `@skill.script` decorator (executed in-process) | -| Discovery | Automatic via `skill_paths` parameter | Explicit via `skills` parameter | -| Dynamic content | No (static files only) | Yes (functions can generate content at runtime) | +| Aspect | File-Based | Code-Defined | Class-Based | +|--------|-----------|--------------|-------------| +| Definition | `SKILL.md` files on disk | `Skill` instances in Python | Classes extending `ClassSkill` | +| Resources | Static files in `references/` and `assets/` directories | Callable functions via `@skill.resource` decorator | `@ClassSkill.resource` decorator (auto-discovered) | +| Scripts | Python files in `scripts/` directory (executed via subprocess) | Callable functions via `@skill.script` decorator (executed in-process) | `@ClassSkill.script` decorator (executed in-process) | +| Discovery | Automatic via `skill_paths` parameter | Explicit via `skills` parameter | Explicit via `skills` parameter | +| Dynamic content | No (static files only) | Yes (functions can generate content at runtime) | Yes (functions can generate content at runtime) | +| Sharing pattern | Copy skill directory | Inline or shared instances | Package in shared libraries/PyPI | -Both types can be combined in a single `SkillsProvider` — see the [mixed_skills](mixed_skills/) sample. +All three types can be combined in a single `SkillsProvider` — see the [mixed_skills](mixed_skills/) sample. ### Script Execution diff --git a/python/samples/02-agents/skills/class_based_skill/README.md b/python/samples/02-agents/skills/class_based_skill/README.md new file mode 100644 index 0000000000..bf70e35db8 --- /dev/null +++ b/python/samples/02-agents/skills/class_based_skill/README.md @@ -0,0 +1,71 @@ +# Class-Based Agent Skills + +This sample demonstrates how to define **Agent Skills as Python classes** using `ClassSkill`. + +## What's Demonstrated + +- Creating skills as classes that extend `ClassSkill` +- Bundling name, description, instructions, resources, and scripts into a single class +- Using `@ClassSkill.resource` decorator for automatic resource discovery +- Using `@ClassSkill.script` decorator for automatic script discovery +- Lazy-loading and caching of resources and scripts +- Registering class-based skills with `SkillsProvider` + +## Skills Included + +### unit-converter (class-based) + +A `UnitConverterSkill` class that converts between common units. Defined in `class_based_skill.py`: + +- `conversion-table` — Static resource with factor table +- `convert` — Script that performs `value × factor` conversion + +## Project Structure + +``` +class_based_skill/ +├── class_based_skill.py +└── README.md +``` + +## Running the Sample + +### Prerequisites + +- An [Azure AI Foundry](https://ai.azure.com/) project with a deployed model (e.g. `gpt-4o-mini`) + +### Environment Variables + +Set the required environment variables in a `.env` file (see `python/.env.example`): + +- `FOUNDRY_PROJECT_ENDPOINT`: Your Azure AI Foundry project endpoint +- `FOUNDRY_MODEL`: The name of your model deployment (defaults to `gpt-4o-mini`) + +### Authentication + +This sample uses `AzureCliCredential` for authentication. Run `az login` in your terminal before running the sample. + +### Run + +```bash +cd python +uv run samples/02-agents/skills/class_based_skill/class_based_skill.py +``` + +### Expected Output + +``` +Converting units with class-based skills +------------------------------------------------------------ +Agent: Here are your conversions: + +1. **26.2 miles → 42.16 km** (a marathon distance) +2. **75 kg → 165.35 lbs** +``` + +## Learn More + +- [Agent Skills Specification](https://agentskills.io/) +- [Code-Defined Skills Sample](../code_defined_skill/) +- [Mixed Skills Sample](../mixed_skills/) +- [Microsoft Agent Framework Documentation](../../../../../docs/) diff --git a/python/samples/02-agents/skills/class_based_skill/class_based_skill.py b/python/samples/02-agents/skills/class_based_skill/class_based_skill.py new file mode 100644 index 0000000000..e2b480864f --- /dev/null +++ b/python/samples/02-agents/skills/class_based_skill/class_based_skill.py @@ -0,0 +1,145 @@ +# Copyright (c) Microsoft. All rights reserved. + +import asyncio +import json +import os + +# Uncomment this filter to suppress the experimental Skills warning before +# using the sample's Skills APIs. +# import warnings # isort: skip +# warnings.filterwarnings("ignore", message=r"\[SKILLS\].*", category=FutureWarning) +from textwrap import dedent + +from agent_framework import Agent, ClassSkill, SkillsProvider +from agent_framework.foundry import FoundryChatClient +from azure.identity import AzureCliCredential +from dotenv import load_dotenv + +""" +Class-Based Agent Skills — Define skills as Python classes + +This sample demonstrates how to define Agent Skills as reusable Python classes +by subclassing ``ClassSkill``. Class-based skills bundle all components (name, +description, instructions, resources, scripts) into a single class, making +them easy to package and distribute via shared libraries or PyPI. + +Key concepts shown: +- Subclassing ``ClassSkill`` to create a self-contained skill +- Using ``@property`` + ``@ClassSkill.resource`` (bare) — name defaults to method name +- Using ``@ClassSkill.script(name=..., description=...)`` — explicit name and description +- Lazy-loading and caching of resources and scripts +""" + +# Load environment variables from .env file +load_dotenv() + + +# --------------------------------------------------------------------------- +# Class-Based Skill: UnitConverterSkill +# --------------------------------------------------------------------------- + + +class UnitConverterSkill(ClassSkill): + """A unit-converter skill defined as a Python class. + + Converts between common units (miles↔km, pounds↔kg) using a + conversion factor. Resources and scripts are discovered automatically + via decorators. + """ + + def __init__(self) -> None: + super().__init__( + name="unit-converter", + description=( + "Convert between common units using a multiplication factor. " + "Use when asked to convert miles, kilometers, pounds, or kilograms." + ), + ) + + @property + def instructions(self) -> str: + return dedent("""\ + Use this skill when the user asks to convert between units. + + 1. Review the conversion-table resource to find the factor for the requested conversion. + 2. Use the convert script, passing the value and factor from the table. + 3. Present the result clearly with both units. + """) + + # 1. Property with bare decorator — name defaults to the method name + # ("conversion_table" → "conversion-table"), no description. + # Place @property first, then @ClassSkill.resource. + @property + @ClassSkill.resource + def conversion_table(self) -> str: + """Lookup table of multiplication factors for common unit conversions.""" + return dedent("""\ + # Conversion Tables + + Formula: **result = value × factor** + + | From | To | Factor | + |-------------|-------------|----------| + | miles | kilometers | 1.60934 | + | kilometers | miles | 0.621371 | + | pounds | kilograms | 0.453592 | + | kilograms | pounds | 2.20462 | + """) + + # 2. Explicit name — overrides the method name + # 3. Explicit description — provides a description for the script + @ClassSkill.script(name="convert", description="Multiplies a value by a conversion factor.") + def convert_units(self, value: float, factor: float) -> str: + """Convert a value using a multiplication factor: result = value × factor. + + Args: + value: The numeric value to convert. + factor: Conversion factor from the conversion table. + + Returns: + JSON string with the inputs and converted result. + """ + result = round(value * factor, 4) + return json.dumps({"value": value, "factor": factor, "result": result}) + + +async def main() -> None: + """Run the class-based skills demo.""" + endpoint = os.environ["FOUNDRY_PROJECT_ENDPOINT"] + deployment = os.environ.get("FOUNDRY_MODEL", "gpt-4o-mini") + + client = FoundryChatClient( + project_endpoint=endpoint, + model=deployment, + credential=AzureCliCredential(), + ) + + # Instantiate the class-based skill and pass it to the provider + unit_converter = UnitConverterSkill() + + async with Agent( + client=client, + instructions="You are a helpful assistant that can convert units.", + context_providers=[SkillsProvider(unit_converter)], + ) as agent: + print("Converting units with class-based skills") + print("-" * 60) + response = await agent.run( + "How many kilometers is a marathon (26.2 miles)? And how many pounds is 75 kilograms?" + ) + print(f"Agent: {response}\n") + + +if __name__ == "__main__": + asyncio.run(main()) + +""" +Sample output: + +Converting units with class-based skills +------------------------------------------------------------ +Agent: Here are your conversions: + +1. **26.2 miles → 42.16 km** (a marathon distance) +2. **75 kg → 165.35 lbs** +""" diff --git a/python/samples/02-agents/skills/mixed_skills/README.md b/python/samples/02-agents/skills/mixed_skills/README.md index 8e5111266c..3b6832827e 100644 --- a/python/samples/02-agents/skills/mixed_skills/README.md +++ b/python/samples/02-agents/skills/mixed_skills/README.md @@ -1,17 +1,18 @@ -# Mixed Skills — Code Skills and File Skills +# Mixed Skills — Code, Class, and File Skills -This sample demonstrates how to combine **code-defined skills** and -**file-based skills** in a single agent using a `SkillScriptRunner` callable -and `SkillsProvider`. +This sample demonstrates how to combine **code-defined skills**, +**class-based skills**, and **file-based skills** in a single agent using +`SkillsProvider`. ## Concepts | Concept | Description | |---------|-------------| | **Code skill** | A `Skill` created in Python with `@skill.script` decorators for in-process callable functions and `@skill.resource` for dynamic content | +| **Class skill** | A self-contained skill class extending `ClassSkill`, bundling instructions, resources, and scripts | | **File skill** | A skill discovered from a `SKILL.md` file on disk, with reference documents and executable script files | | **`script_runner`** | A callable (sync or async) satisfying the `SkillScriptRunner` protocol — required when file skills have scripts | -| **`SkillsProvider`** | Registers both code-defined and file-based skills in a single provider | +| **`SkillsProvider`** | Registers code-defined, class-based, and file-based skills in a single provider | ## Skills in This Sample @@ -24,6 +25,15 @@ Defined entirely in Python code using decorators: Code scripts run **in-process** — no subprocess or external runner needed. +### temperature-converter (class skill) + +Defined as a `TemperatureConverterSkill` class extending `ClassSkill`: + +- **`@ClassSkill.resource`** — `temperature-conversion-formulas`: °F↔°C↔K formulas +- **`@ClassSkill.script`** — `convert-temperature`: converts between temperature scales + +Class-based scripts run **in-process** — no subprocess or external runner needed. + ### unit-converter (file skill) Discovered from `skills/unit-converter/SKILL.md`: @@ -43,7 +53,10 @@ File scripts are executed as **local Python subprocesses** via the │ AggregatingSkillsSource([ │ │ FileSkillsSource("./skills", # file skills │ │ script_runner=runner), │ -│ InMemorySkillsSource([skill]), # code skills │ +│ InMemorySkillsSource([ │ +│ volume_skill, # code skill │ +│ temp_converter, # class skill │ +│ ]), │ │ ]) │ │ ) │ │ ) │ @@ -54,6 +67,7 @@ File scripts are executed as **local Python subprocesses** via the │ script_runner(skill, script, args) │ │ │ │ • Code scripts (@skill.script) → in-process call │ +│ • Class scripts (@ClassSkill.script) → in-process call │ │ • File scripts (scripts/*.py) → subprocess via │ │ the callback function │ └─────────────────────────────────────────────────────────────┘ diff --git a/python/samples/02-agents/skills/mixed_skills/mixed_skills.py b/python/samples/02-agents/skills/mixed_skills/mixed_skills.py index 998dc6e692..2b10fc0c2a 100644 --- a/python/samples/02-agents/skills/mixed_skills/mixed_skills.py +++ b/python/samples/02-agents/skills/mixed_skills/mixed_skills.py @@ -16,6 +16,7 @@ from typing import Any from agent_framework import ( Agent, AggregatingSkillsSource, + ClassSkill, DeduplicatingSkillsSource, FileSkillsSource, InlineSkill, @@ -34,28 +35,32 @@ if _SKILLS_ROOT not in sys.path: from subprocess_script_runner import subprocess_script_runner # noqa: E402 """ -Mixed Skills — Code skills and file skills in a single agent +Mixed Skills — Code, class, and file skills in a single agent This sample demonstrates how to combine **code-defined skills** (with -``@skill.script`` and ``@skill.resource`` decorators) and **file-based skills** -(discovered from ``SKILL.md`` files on disk) in a single agent using -``SkillsProvider`` and a ``SkillScriptRunner`` callable. +``@skill.script`` and ``@skill.resource`` decorators), **class-based skills** +(subclassing ``ClassSkill``), and **file-based skills** (discovered from +``SKILL.md`` files on disk) in a single agent using ``SkillsProvider`` and +a ``SkillScriptRunner`` callable. Key concepts shown: - Code skills with ``@skill.script``: executable Python functions the agent can invoke directly in-process. - Code skills with ``@skill.resource``: dynamic content the agent can read on demand. +- Class skills: self-contained skill classes extending ``ClassSkill``. - File skills from disk: ``SKILL.md`` files with reference documents and executable script files. - ``script_runner``: routes **file-based** script execution through a callback, enabling custom handling (e.g. subprocess calls). - Code-defined scripts (``@skill.script``) run in-process automatically. + Code-defined and class-based scripts run in-process automatically. -The sample registers two skills: +The sample registers three skills: 1. **volume-converter** (code skill) — converts between gallons and liters using ``@skill.script`` for conversion and ``@skill.resource`` for the factor table. -2. **unit-converter** (file skill) — converts between common units (miles↔km, +2. **temperature-converter** (class skill) — converts between temperature scales + (°F↔°C↔K) using a ``ClassSkill`` subclass. +3. **unit-converter** (file skill) — converts between common units (miles↔km, pounds↔kg) via a subprocess-executed Python script discovered from ``skills/unit-converter/SKILL.md``. """ @@ -110,9 +115,68 @@ def convert_volume(value: float, factor: float) -> str: # --------------------------------------------------------------------------- -# 2. Wire everything together and run the agent +# 2. Define a class-based skill for temperature conversion # --------------------------------------------------------------------------- +class TemperatureConverterSkill(ClassSkill): + """A temperature-converter skill defined as a Python class. + + Converts between temperature scales (Fahrenheit, Celsius, Kelvin). + Resources and scripts are discovered automatically via decorators. + """ + + def __init__(self) -> None: + super().__init__( + name="temperature-converter", + description="Convert between temperature scales (Fahrenheit, Celsius, Kelvin).", + ) + + @property + def instructions(self) -> str: + return dedent("""\ + Use this skill when the user asks to convert temperatures. + + 1. Read the temperature-conversion-formulas resource to find the factor and offset + for the requested conversion. + 2. Use the convert-temperature script, passing value, factor, and offset. + 3. Present the result clearly with both temperature scales. + """) + + @ClassSkill.resource(name="temperature-conversion-formulas") + def formulas(self) -> str: + """Temperature conversion formulas reference table.""" + return dedent("""\ + # Temperature Conversion Formulas + + Formula: **result = value × factor + offset** + + | From | To | Factor | Offset | + |-------------|-------------|----------|-----------| + | Fahrenheit | Celsius | 0.555556 | -17.7778 | + | Celsius | Fahrenheit | 1.8 | 32 | + | Celsius | Kelvin | 1 | 273.15 | + | Kelvin | Celsius | 1 | -273.15 | + """) + + @ClassSkill.script(name="convert-temperature") + def convert_temperature(self, value: float, factor: float, offset: float = 0) -> str: + """Convert a temperature value using factor and offset from the formulas resource. + + Args: + value: The numeric temperature value to convert. + factor: Conversion factor from the formulas resource. + offset: Offset to add after multiplying (default 0). + + Returns: + JSON string with the conversion result. + """ + result = round(value * factor + offset, 4) + return json.dumps({"value": value, "factor": factor, "offset": offset, "result": result}) + + +# --------------------------------------------------------------------------- +# 3. Wire everything together and run the agent +# --------------------------------------------------------------------------- async def main() -> None: """Run the combined skills demo.""" @@ -126,9 +190,11 @@ async def main() -> None: credential=AzureCliCredential(), ) - # Create the SkillsProvider with both code and file skills. - # The script_runner handles file-based scripts; code-defined scripts - # (@skill.script) run in-process automatically. + # Create the SkillsProvider with code, class, and file skills. + # The script_runner handles file-based scripts; code-defined and + # class-based scripts run in-process automatically. + temperature_converter = TemperatureConverterSkill() + skills_provider = SkillsProvider( DeduplicatingSkillsSource( AggregatingSkillsSource([ @@ -136,7 +202,7 @@ async def main() -> None: str(Path(__file__).parent / "skills"), script_runner=subprocess_script_runner, ), - InMemorySkillsSource([volume_converter_skill]), + InMemorySkillsSource([volume_converter_skill, temperature_converter]), ]) ) ) @@ -144,14 +210,17 @@ async def main() -> None: # Run the agent async with Agent( client=client, - instructions="You are a helpful assistant that can convert units.", + instructions="You are a helpful assistant that can convert units, volumes, and temperatures.", context_providers=[skills_provider], ) as agent: - # Ask the agent to use both skills - print("Converting units") + # Ask the agent to use all three skills + print("Converting with mixed skills (file + code + class)") print("-" * 60) response = await agent.run( - "How many kilometers is a marathon (26.2 miles)? And how many liters is a 5-gallon bucket?" + "I need three conversions: " + "1) How many kilometers is a marathon (26.2 miles)? " + "2) How many liters is a 5-gallon bucket? " + "3) What is 98.6°F in Celsius?" ) print(f"Agent: {response}\n") @@ -162,12 +231,11 @@ if __name__ == "__main__": """ Sample output: -Converting units +Converting with mixed skills (file + code + class) ------------------------------------------------------------ Agent: Here are your conversions: 1. **26.2 miles → 42.16 km** (a marathon distance) 2. **5 gallons → 18.93 liters** - -I used the conversion factors from each skill's reference table. +3. **98.6°F → 37.0°C** """ diff --git a/python/scripts/integration_test_report/aggregate.py b/python/scripts/integration_test_report/aggregate.py index e803add730..708f47fcf0 100644 --- a/python/scripts/integration_test_report/aggregate.py +++ b/python/scripts/integration_test_report/aggregate.py @@ -2,16 +2,18 @@ """Aggregate per-provider JUnit XML test results and generate a trend report. -Parses ``pytest.xml`` (JUnit XML) files produced by each CI job, merges them -into a single run, combines with historical data, and generates a markdown -trend table — the same pattern used by ``scripts/sample_validation/aggregate.py``. +Parses JUnit XML files produced by CI jobs — both ``pytest.xml`` (Python) and +xunit v3 ``*.junit`` (dotnet) — merges them into a single run, combines +with historical data, and generates a markdown trend table. Usage (from CI): python aggregate.py -The reports directory is expected to contain subdirectories named -``test-results-/`` each containing a ``pytest.xml`` file -(created by ``actions/download-artifact``). +The reports directory is expected to contain artifact subdirectories. Two +layouts are supported: + +- **Python (pytest):** ``test-results-/pytest.xml`` +- **Dotnet (xunit):** ``dotnet-test-results--/*.junit`` """ from __future__ import annotations @@ -46,9 +48,21 @@ def _format_run_label(timestamp: str) -> str: def _derive_provider(directory_name: str) -> str: """Derive a provider label from a report directory name. - ``test-results-openai`` → ``OpenAI`` - ``test-results-azure-openai`` → ``Azure OpenAI`` + Handles both Python and dotnet naming conventions: + - ``test-results-openai`` → ``OpenAI`` + - ``test-results-azure-openai`` → ``Azure OpenAI`` + - ``dotnet-test-results-net10.0-ubuntu-latest`` → ``net10.0 (ubuntu)`` """ + # Dotnet convention: dotnet-test-results-- + if directory_name.startswith("dotnet-test-results-"): + raw = directory_name.replace("dotnet-test-results-", "") + # e.g. "net10.0-ubuntu-latest" → framework="net10.0", os="ubuntu-latest" + parts = raw.split("-", 1) + framework = parts[0] + os_label = parts[1].split("-")[0] if len(parts) > 1 else "" + return f"{framework} ({os_label})" if os_label else framework + + # Python convention: test-results- raw = directory_name.replace("test-results-", "") known = { "openai": "OpenAI", @@ -102,11 +116,21 @@ def _parse_junit_xml(xml_path: Path) -> list[dict[str, str]]: # it appends the class name, e.g.: # "packages.foundry.tests.foundry.test_foundry_embedding_client.TestFoundryEmbeddingIntegration" # We want the file-level module: "test_foundry_embedding_client" + # + # xunit (dotnet) writes classname as the full C# type, e.g.: + # "OpenAIChatCompletion.IntegrationTests.ChatCompletionTests" + # We want the project prefix: "OpenAIChatCompletion" if classname: parts = classname.rsplit(".", 2) # If the last segment starts with uppercase it's a class name — take the one before it if len(parts) >= 2 and parts[-1][0:1].isupper(): - module = parts[-2] + # For dotnet: if the penultimate part is "IntegrationTests" or "UnitTests", + # use the part before that (the project name) instead + if parts[-2] in ("IntegrationTests", "UnitTests") and len(parts) >= 3: + # parts[0] may contain dots — take the last segment of it + module = parts[0].rsplit(".", 1)[-1] + else: + module = parts[-2] else: module = parts[-1] else: @@ -148,28 +172,61 @@ def _parse_junit_xml(xml_path: Path) -> list[dict[str, str]]: # --------------------------------------------------------------------------- +def _discover_xml_files(reports_dir: Path) -> list[tuple[str, Path]]: + """Discover JUnit XML test result files in artifact subdirectories. + + Handles two directory layouts: + - **Python (pytest):** ``test-results-/pytest.xml`` + - **Dotnet (xunit):** ``dotnet-test-results--/*.junit`` + + Returns: + List of ``(directory_name, xml_path)`` tuples. + """ + xml_files: list[tuple[str, Path]] = [] + if not reports_dir.is_dir(): + return xml_files + + for subdir in sorted(reports_dir.iterdir()): + if not subdir.is_dir(): + continue + + # Python layout: single pytest.xml per artifact + pytest_xml = subdir / "pytest.xml" + if pytest_xml.exists(): + xml_files.append((subdir.name, pytest_xml)) + continue + + # Dotnet layout: multiple *.junit files per artifact + junit_files = sorted(subdir.rglob("*.junit")) + for jf in junit_files: + xml_files.append((subdir.name, jf)) + + # Fallback: any .xml file that looks like JUnit (not .trx, not cobertura) + if not junit_files: + for xf in sorted(subdir.rglob("*.xml")): + if xf.suffix == ".xml" and not xf.name.endswith(".cobertura.xml"): + xml_files.append((subdir.name, xf)) + + return xml_files + + def load_current_run(reports_dir: Path) -> dict[str, Any]: """Load per-provider JUnit XML reports from the current CI run and merge. + Supports both pytest (Python) and xunit v3 (dotnet) JUnit XML formats. + Args: - reports_dir: Directory containing ``test-results-/`` subdirs. + reports_dir: Directory containing artifact subdirectories with XML reports. Returns: Merged run dict with ``timestamp``, ``summary``, ``results``. """ combined_results: dict[str, dict[str, str]] = {} # nodeid → {status, provider} - # actions/download-artifact creates: reports_dir/test-results-openai/pytest.xml - xml_files: list[tuple[str, Path]] = [] - if reports_dir.is_dir(): - for subdir in sorted(reports_dir.iterdir()): - if subdir.is_dir(): - xml_file = subdir / "pytest.xml" - if xml_file.exists(): - xml_files.append((subdir.name, xml_file)) + xml_files = _discover_xml_files(reports_dir) if not xml_files: - print(f"Warning: No pytest.xml files found in {reports_dir}") + print(f"Warning: No JUnit XML files found in {reports_dir}") return { "timestamp": datetime.now(timezone.utc).isoformat(), "summary": { @@ -181,19 +238,42 @@ def load_current_run(reports_dir: Path) -> dict[str, Any]: "results": {}, } + # Dotnet tests always run under multiple frameworks, so we always + # qualify their keys with the provider to ensure deterministic, + # stable keys across runs regardless of file parse order. + is_dotnet = any(d.startswith("dotnet-test-results-") for d, _ in xml_files) + for dir_name, xml_file in xml_files: print(f" Loading: {xml_file}") provider = _derive_provider(dir_name) tests = _parse_junit_xml(xml_file) for test in tests: - combined_results[test["nodeid"]] = { + raw_id = test["nodeid"] + key = f"{provider}::{raw_id}" if is_dotnet else raw_id + + combined_results[key] = { "status": test["status"], "provider": provider, "module": test.get("module", ""), } - # Build summary counts using mutually exclusive status buckets. - # Errors are folded into the failed count for display purposes. + # Build per-provider summary counts so the report can show one row per + # framework (dotnet) or per provider (Python). + provider_counts: dict[str, dict[str, int]] = {} + for r in combined_results.values(): + prov = r.get("provider", "Unknown") + if prov not in provider_counts: + provider_counts[prov] = {"total": 0, "passed": 0, "failed": 0, "skipped": 0} + provider_counts[prov]["total"] += 1 + st = r["status"] + if st == "passed": + provider_counts[prov]["passed"] += 1 + elif st in ("failed", "error"): + provider_counts[prov]["failed"] += 1 + elif st == "skipped": + provider_counts[prov]["skipped"] += 1 + + # Overall summary (sum across all providers). statuses = [r["status"] for r in combined_results.values()] summary = { "total": len(statuses), @@ -205,6 +285,7 @@ def load_current_run(reports_dir: Path) -> dict[str, Any]: return { "timestamp": datetime.now(timezone.utc).isoformat(), "summary": summary, + "provider_summaries": provider_counts, "results": combined_results, } @@ -253,7 +334,29 @@ def generate_trend_report(runs: list[dict[str, Any]]) -> str: "", ] - # --- Overall status table (most recent first) --- + # Detect whether this is a dotnet report (provider-qualified keys). + is_dotnet = False + for run in runs: + provider_sums = run.get("provider_summaries", {}) + if any(p.startswith("net") for p in provider_sums): + is_dotnet = True + break + + if is_dotnet: + _generate_dotnet_report(lines, runs) + else: + _generate_python_report(lines, runs) + + lines.append("") + lines.append("**Legend:** ✅ Passed · ❌ Failed · ⏭️ Skipped · ⚠️ Expected Failure (xfail) · N/A Not available") + lines.append("") + + return "\n".join(lines) + + +def _generate_python_report(lines: list[str], runs: list[dict[str, Any]]) -> None: + """Generate the original single-table Python report format.""" + # --- Overall status table --- lines.append("## Overall Status (Last 5 Runs)") lines.append("") lines.append("| Run | Total | ✅ Passed | ❌ Failed | ⏭️ Skipped |") @@ -276,27 +379,91 @@ def generate_trend_report(runs: list[dict[str, Any]]) -> str: lines.append("") - # --- Per-test results table --- - lines.append("## Per-Test Results") - lines.append("") + # --- Single per-test results table --- + _generate_per_test_table(lines, runs, "## Per-Test Results") - # Collect all test nodeids, providers, and modules across all runs - all_tests: dict[str, str] = {} # nodeid → provider (from most recent run) - all_modules: dict[str, str] = {} # nodeid → module (from most recent run) + +def _generate_dotnet_report(lines: list[str], runs: list[dict[str, Any]]) -> None: + """Generate per-framework tables for dotnet (net10.0, net472, etc.).""" + # Collect all providers seen across all runs, sorted for stable ordering + all_providers: set[str] = set() + for run in runs: + all_providers.update(run.get("provider_summaries", {}).keys()) + providers = sorted(all_providers) + + for provider in providers: + lines.append(f"## {provider}") + lines.append("") + + # --- Per-provider summary table --- + lines.append("| Run | Total | ✅ Passed | ❌ Failed | ⏭️ Skipped |") + lines.append("|-----|-------|-----------|-----------|------------|") + + for run in reversed(runs): + ps = run.get("provider_summaries", {}).get(provider, {}) + total = ps.get("total", 0) + label = _format_run_label(run["timestamp"]) + if total == 0: + lines.append(f"| {label} | N/A | N/A | N/A | N/A |") + else: + lines.append( + f"| {label} " + f"| {total} " + f"| {ps.get('passed', 0)}/{total} " + f"| {ps.get('failed', 0)}/{total} " + f"| {ps.get('skipped', 0)}/{total} |" + ) + + for _ in range(MAX_HISTORY - len(runs)): + lines.append("| N/A | N/A | N/A | N/A | N/A |") + + lines.append("") + + # --- Per-test table filtered to this provider --- + _generate_per_test_table( + lines, runs, + heading=None, + provider_filter=provider, + ) + + +def _generate_per_test_table( + lines: list[str], + runs: list[dict[str, Any]], + heading: str | None = None, + provider_filter: str | None = None, +) -> None: + """Emit a per-test trend table, optionally filtered to a single provider.""" + if heading: + lines.append(heading) + lines.append("") + + # Collect all test nodeids (and metadata) across all runs + all_tests: dict[str, str] = {} # nodeid → provider + all_modules: dict[str, str] = {} # nodeid → module for run in runs: for nodeid, info in run.get("results", {}).items(): - provider = info.get("provider", "Unknown") if isinstance(info, dict) else "Unknown" - module = info.get("module", "") if isinstance(info, dict) else "" - all_tests[nodeid] = provider + if not isinstance(info, dict): + continue + prov = info.get("provider", "Unknown") + if provider_filter and prov != provider_filter: + continue + module = info.get("module", "") + all_tests[nodeid] = prov all_modules[nodeid] = module if not all_tests: lines.append("*No test results available.*") - return "\n".join(lines) + lines.append("") + return - # Build header (most recent run first) - header = "| Test | File | Provider |" - separator = "|------|------|----------|" + # Build header + if provider_filter: + header = "| Test | File |" + separator = "|------|------|" + else: + header = "| Test | File | Provider |" + separator = "|------|------|----------|" for run in reversed(runs): label = _format_run_label(run["timestamp"]) header += f" {label} |" @@ -308,12 +475,15 @@ def generate_trend_report(runs: list[dict[str, Any]]) -> str: lines.append(header) lines.append(separator) - # Sort by provider then test name - for nodeid in sorted(all_tests, key=lambda n: (all_tests[n], n)): - provider = all_tests[nodeid] + # Sort by module then test name + for nodeid in sorted(all_tests, key=lambda n: (all_modules.get(n, ""), n)): module = all_modules.get(nodeid, "") short = _short_name(nodeid) - row = f"| `{short}` | `{module}` | {provider} |" + if provider_filter: + row = f"| `{short}` | `{module}` |" + else: + provider = all_tests[nodeid] + row = f"| `{short}` | `{module}` | {provider} |" for run in reversed(runs): result = run.get("results", {}).get(nodeid) @@ -330,10 +500,6 @@ def generate_trend_report(runs: list[dict[str, Any]]) -> str: lines.append(row) lines.append("") - lines.append("**Legend:** ✅ Passed · ❌ Failed · ⏭️ Skipped · ⚠️ Expected Failure (xfail) · N/A Not available") - lines.append("") - - return "\n".join(lines) # ---------------------------------------------------------------------------