From b1d057e2205cc597c983a5f28c5bc67c79f838df Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 13 May 2026 19:55:58 +0000 Subject: [PATCH] Document follow-up changes for the Magentic .NET sample Agent-Logs-Url: https://github.com/microsoft/agent-framework/sessions/caa3488f-d6f5-494d-a928-a45d6a98b3c3 Co-authored-by: lokitoth <6936551+lokitoth@users.noreply.github.com> --- .../Orchestration/Magentic/CHANGES.md | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100644 dotnet/samples/03-workflows/Orchestration/Magentic/CHANGES.md diff --git a/dotnet/samples/03-workflows/Orchestration/Magentic/CHANGES.md b/dotnet/samples/03-workflows/Orchestration/Magentic/CHANGES.md new file mode 100644 index 0000000000..9a6fea5357 --- /dev/null +++ b/dotnet/samples/03-workflows/Orchestration/Magentic/CHANGES.md @@ -0,0 +1,113 @@ +# Proposed Changes to the Magentic .NET Sample + +This document captures the changes recommended for `dotnet/samples/03-workflows/Orchestration/Magentic/Program.cs` +following a deep review of the framework code that backs `MagenticWorkflowBuilder`, +`MagenticOrchestrator`, `AIAgentHostExecutor`, and the `ChatProtocolExecutor` pipeline. + +The sample compiles cleanly (`dotnet build` → 0 warnings, 0 errors), the workflow is wired correctly, +and it should run end-to-end against a properly configured Azure AI Foundry project. The items below are +either parity gaps with the Python original (`python/samples/03-workflows/orchestrations/magentic.py`) +or small robustness improvements — none are blockers. + +--- + +## 1. Final transcript only contains the manager's final answer (parity gap) + +### Observation + +`MagenticOrchestrator.PrepareFinalAnswerAsync` yields a single-element `List` containing +only the manager's synthesized final answer +(`dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticOrchestrator.cs:297-302`). +The orchestrator is the only registered output executor (`MagenticWorkflowBuilder.cs:127`, +`WithOutputFrom(orchestrator)`), and the per-turn `AgentResponseUpdate` values from the team agents +bypass the output filter and surface only as streamed `AgentResponseUpdateEvent` instances +(`InProcessRunnerContext.cs:244-256`). + +The Python sample uses `intermediate_outputs=True` and prints the **full multi-agent transcript** +at the end. The .NET sample's "Final Conversation Transcript" section therefore prints only +the manager's summary, which is misleading given the section title. + +### Recommended change + +Accumulate the streamed agent text yourself during the run and print that as the transcript, instead +of relying on the terminal `WorkflowOutputEvent`. Concretely: + +- Maintain a `List<(string Speaker, StringBuilder Text)>` keyed off `ResponseId` change + (the same boundary `WriteStreamingUpdate` already uses). +- On each `AgentResponseUpdateEvent`, append `update.Update.Text` to the current speaker's buffer, + using `updateEvent.Update.AuthorName ?? updateEvent.ExecutorId` as the speaker. +- After the `await foreach` completes, print the accumulated buffers, followed by the manager's + final answer captured from `WorkflowOutputEvent`. + +This matches the Python sample's `outputs = cast(list[Message], output_event.data)` printout and +makes the section title accurate. + +## 2. Rename or repurpose the "Final Conversation Transcript" header + +If we choose **not** to implement (1), rename the printed header to something accurate such as +**"Final Answer (from MagenticManager)"** so users do not expect the per-agent turns there. The +per-agent text is already streamed live as `- {executorId}: …` lines during execution. + +## 3. `PauseIfInteractive()` blocks the workflow scheduler thread + +### Observation + +`Console.ReadLine()` is called synchronously from inside the `await foreach (WorkflowEvent …)` loop. +Because that loop drives the workflow's superstep pump, blocking it pauses the entire workflow until +the user hits Enter. This matches the documented intent ("block to allow user to read the plan +before continuing"), and the Python sample does the same with `run_in_executor(None, input, …)`, +so behavior is consistent. However: + +- A long pause can exceed the lifetime of any externally injected `CancellationToken` and the user has + no way to cancel from the prompt. +- On CI the prompt is skipped via `Console.IsInputRedirected`, which is correct. + +### Recommended change (optional polish) + +Wrap the prompt in `await Task.Run(() => Console.ReadLine())` so the Read happens off the scheduler +thread and is awaitable. This is a cosmetic robustness improvement and not required for correctness. + +## 4. Document the disabled plan-signoff path + +`RequirePlanSignoff(false)` means the orchestrator never raises a `RequestInfoEvent` for plan approval, +so the sample legitimately does not handle one. Add a short comment near the +`.RequirePlanSignoff(false)` call explaining that switching this to `true` would also require handling +`RequestInfoEvent` in the event loop and replying via `run.SendResponseAsync(...)`. This keeps the +sample self-explanatory for users who copy it as a starting point. + +## 5. Consider explicitly logging `WorkflowWarningEvent` + +`MagenticOrchestrator` and `MagenticManager` both surface non-fatal issues (e.g., progress-ledger +JSON parse retries, empty next-speaker fallback) via `WorkflowWarningEvent` +(`MagenticManager.cs:96`, `MagenticOrchestrator.cs:229`, `:263`, `:271`). The current sample +silently ignores them. Adding a `case WorkflowWarningEvent warn: Console.WriteLine($"[warn] {warn.Message}");` +arm to the `switch` would make the sample more useful for debugging when the model returns malformed +JSON for the progress ledger. + +## 6. No change required for the following items (validated) + +- **Input typing**: `RunStreamingAsync(workflow, new List { … })` is accepted by the + orchestrator's `ChatProtocolExecutor` route for `IEnumerable` + (`ChatProtocolExecutor.cs:84-86`). +- **TurnToken trigger**: required because `MagenticOrchestrator` sets `AutoSendTurnToken = false` + (`MagenticOrchestrator.cs:92-93`); `emitEvents: true` correctly enables streaming for participants + (`MagenticOrchestrator.cs:286`). +- **Switch ordering**: `AgentResponseUpdateEvent` derives from `WorkflowOutputEvent`; matching it + first is correct, and the typed `WorkflowOutputEvent when outputEvent.Is>()` + arm only catches the orchestrator's terminal yield. +- **Limit-exhausted path**: `RunCoordinationRoundAsync` (line 209-210) also yields a + `List`, so `finalOutput` is populated even if max rounds/resets are hit. +- **Code Interpreter tool**: `HostedCodeInterpreterTool` on the coder is supported by the Foundry + Responses path; same pattern is used in `Agent_Step24_CodeInterpreterFileDownload`. +- **`OPENAI001` suppression**: handled at the project level via `` in `Magentic.csproj`. + +--- + +## Suggested order of work + +1. (1) Capture streamed updates and print the full transcript — biggest UX/parity win. +2. (5) Surface `WorkflowWarningEvent`s — small change, large debugging value. +3. (4) Add the plan-signoff comment — documentation only. +4. (3) Off-thread the `Console.ReadLine` — cosmetic. + +Item (2) is only needed if (1) is not done.