diff --git a/dotnet/samples/03-workflows/Orchestration/Magentic/CHANGES.md b/dotnet/samples/03-workflows/Orchestration/Magentic/CHANGES.md deleted file mode 100644 index 9a6fea5357..0000000000 --- a/dotnet/samples/03-workflows/Orchestration/Magentic/CHANGES.md +++ /dev/null @@ -1,113 +0,0 @@ -# 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.