Improve/maf report actions - #9
Merged
Merged
Conversation
…teLine with ILogger, add config-driven model/token settings, add Tavily retry+timeout, and add BlogWriter.Tests project
There was a problem hiding this comment.
Pull request overview
This PR hardens the BlogWriter Microsoft Agent Framework (MAF) workflow for real-world runs by adding structured logging, end-to-end cancellation support, basic resilience for outbound HTTP calls, configuration overrides for model/token limits, and a new unit test project to validate key deterministic behavior.
Changes:
- Thread
CancellationTokenfromProgram.csthroughBlogWorkflowexecutors into each agent call, and replace mostConsole.WriteLinetracing withILogger. - Add resiliency/timeouts for Tavily HTTP calls and explicit network timeout for the OpenAI client; allow model/token settings to be overridden via env/user-secrets.
- Deduplicate approval detection via
ResearchState.IsApproved/ApprovedMarker, and introduceBlogWriter.Testswith unit tests for routing, token caps, and revision/approval boundaries.
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| Workflows/BlogExecutors.cs | Adds CancellationToken plumbing to executor message handlers. |
| TokenCapChatClient.cs | Validates token cap at construction and uses stored cap for enforcement. |
| ReviewerAgent.cs | Adds cancellation support and switches to structured logging + centralized approval check. |
| ResearchState.cs | Introduces ApprovedMarker and IsApproved helper; simplifies NeedsRevision. |
| ResearcherAgent.cs | Adds cancellation propagation and replaces console tracing with ILogger. |
| Program.cs | Adds config overrides, OpenAI network timeout, Tavily timeout + retry, and Ctrl+C cancellation wiring. |
| IReviewerAgent.cs | Updates interface methods to accept CancellationToken. |
| IResearcherAgent.cs | Updates interface methods to accept CancellationToken. |
| IMPROVEMENTS.md | Adds a generated health/improvement report documenting findings and recommendations. |
| IBlogWorkflow.cs | Updates workflow API to accept CancellationToken. |
| IBloggerAgent.cs | Updates interface methods to accept CancellationToken. |
| IAuthorAgent.cs | Updates interface methods to accept CancellationToken. |
| BlogWriter.Tests/TokenCapChatClientTests.cs | Adds unit tests for token cap construction and enforcement. |
| BlogWriter.Tests/TestChatClients.cs | Adds IChatClient test doubles for deterministic unit tests. |
| BlogWriter.Tests/ResearchStateTests.cs | Adds unit tests for approval detection and revision-cap boundaries. |
| BlogWriter.Tests/BlogWriter.Tests.csproj | Adds new test project configuration and dependencies. |
| BlogWriter.Tests/BloggerAgentRoutingTests.cs | Adds tests ensuring deterministic routing paths don’t invoke the model. |
| BlogWriter.csproj | Excludes nested test project sources from the main project compile glob. |
| BlogWorkflow.cs | Switches to streaming execution, adds cancellation, and uses ILogger for workflow events. |
| BloggerAgent.cs | Adds cancellation propagation, uses centralized approval check, and replaces console tracing with ILogger. |
| AuthorAgent.cs | Adds cancellation propagation and replaces console tracing with ILogger. |
| .gitignore | Ensures nested bin/ and obj/ directories are ignored. |
Suppressed comments (1)
IMPROVEMENTS.md:49
- Section 4 is written as a list of current gaps, but several of these items are addressed by the changes in this PR (ILogger migration, CancellationToken threading, retries/timeouts, configuration overrides, tests, TokenCapChatClient constructor validation). Consider marking this section as historical or updating the framing so the document doesn’t contradict the code.
## 4. What's wrong / weak — in plain language
None of these are MAF anti-patterns (the scanner is clean); they're general .NET/production-readiness gaps that matter if this app grows beyond a demo/CLI.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+82
to
+91
| try | ||
| { | ||
| HttpResponseMessage response = await tavilyHttpClient.PostAsJsonAsync(requestUri, body, cancellationToken); | ||
| response.EnsureSuccessStatusCode(); | ||
| return response; | ||
| } | ||
| catch (Exception ex) when (attempt < maxAttempts && ex is HttpRequestException or TaskCanceledException) | ||
| { | ||
| await Task.Delay(TimeSpan.FromSeconds(Math.Pow(2, attempt - 1)), cancellationToken); | ||
| } |
Comment on lines
+27
to
+29
| string modelName = config["MODEL_NAME"] ?? "gpt-4o-mini"; | ||
| int maxOutputTokens = int.TryParse(config["MAX_OUTPUT_TOKENS"], out int configuredMaxOutputTokens) ? configuredMaxOutputTokens : 4096; | ||
| long maxTotalTokens = long.TryParse(config["MAX_TOTAL_TOKENS"], out long configuredMaxTotalTokens) ? configuredMaxTotalTokens : 10000; |
Comment on lines
+20
to
+22
| /// <summary>True when the given review text contains the approval marker (case-insensitive).</summary> | ||
| public static bool IsApproved(string? reviewNotes) => | ||
| !string.IsNullOrEmpty(reviewNotes) && reviewNotes.Contains(ApprovedMarker, StringComparison.OrdinalIgnoreCase); |
Comment on lines
47
to
+51
| // Stream execution instead of running to completion in one shot. The | ||
| // topology is identical to before (proven terminating, MAF-Doctor grade A); | ||
| // streaming simply surfaces each executor's lifecycle as it happens, giving | ||
| // live progress and replacing the scattered Console.WriteLine tracing that | ||
| // previously lived inside the node classes. The final ResearchState is | ||
| // captured from the WorkflowOutputEvent emitted by the reviewer. | ||
| StreamingRun run = await InProcessExecution.RunStreamingAsync(workflow, state); | ||
| // live progress. The final ResearchState is captured from the | ||
| // WorkflowOutputEvent emitted by the reviewer. |
|
|
||
| The scanner is a text/name heuristic and doesn't trace the `options: runOptions` variable back to its construction, so it can't see the cap is actually there. No action needed — this repo has no real, unmitigated cost-runaway risk. | ||
|
|
||
| There is a genuinely good, second layer of cost protection on top of that: `TokenCapChatClient` enforces a **hard cumulative 10,000-token budget for the whole process**, across every model round-trip (including tool-invocation turns), and fails the run loudly (`TokenCapExceededException`) rather than silently overspending. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixed a .gitignore gap that would have committed the new test project's bin/obj build output (nested-project bin/obj wasn't covered by the root-anchored bin, obj patterns).
Logging: all agents + the workflow now use ILogger instead of Console.WriteLine, and every catch block logs the real exception.
Cancellation: CancellationToken threads from Ctrl+C in Program.cs → BlogWorkflow.RunAsync → each executor's [MessageHandler] → each agent's RunAsync call.
Resilience: Tavily HTTP calls now have a 20s timeout and a 3-attempt exponential backoff retry; the OpenAI client has an explicit 60s network timeout.
Config: model name and token caps are now overridable via user-secrets/env vars (MODEL_NAME, MAX_OUTPUT_TOKENS, MAX_TOTAL_TOKENS).
Dedup: ResearchState.IsApproved/ApprovedMarker replace the duplicated "APPROVED" string checks.
Tests: new BlogWriter.Tests project — 20 tests covering ResearchState boundary logic, TokenCapChatClient cap enforcement, and BloggerAgent's deterministic routing (using a ThrowingChatClient to prove those paths never call the model). All pass.