diff --git a/BlogWorkflow.cs b/BlogWorkflow.cs index 3d27b6f..3bb0591 100644 --- a/BlogWorkflow.cs +++ b/BlogWorkflow.cs @@ -44,19 +44,21 @@ public async Task RunAsync( Workflow workflow = new WorkflowBuilder(bloggerExecutor) .AddEdge(bloggerExecutor, researcherExecutor) .AddEdge(researcherExecutor, authorExecutor) - .AddEdge(authorExecutor, reviewerExecutor) + // Review only the initial author pass. A rejected review can route back + // to the author once, but the capped revision is terminal output. + .AddEdge(authorExecutor, reviewerExecutor, + condition: s => s?.RevisionNumber < ResearchState.MaxRevisions) // Bounded revision loop: route back to the author only while the draft - // still needs work and the revision cap has not been reached. When the - // condition is false the reviewer instead yields the final output. + // still needs work and the revision cap has not been reached. .AddEdge(reviewerExecutor, authorExecutor, condition: s => s?.NeedsRevision == true) - .WithOutputFrom(reviewerExecutor) + .WithOutputFrom(reviewerExecutor, authorExecutor) .Build(); // 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. The final ResearchState is captured from the - // WorkflowOutputEvent emitted by the reviewer. + // topology is proven terminating; streaming simply surfaces each + // executor's lifecycle as it happens, giving live progress. The final + // ResearchState is captured from the WorkflowOutputEvent emitted by the + // reviewer after approval or the author after the single revision. StreamingRun run = await InProcessExecution.RunStreamingAsync(workflow, state, cancellationToken: cancellationToken); ResearchState? result = null; @@ -90,7 +92,7 @@ public async Task RunAsync( new InvalidOperationException($"Workflow executor '{failed.ExecutorId}' failed."); case WorkflowOutputEvent { Data: ResearchState finalState }: - // The reviewer yielded the final, approved (or revision-capped) state. + // Reviewer approval or the capped author revision yields the final state. result = finalState; publisher.PublishLifecycle(WorkflowOutputOutcome.Success, "Writing workflow completed."); break; diff --git a/BlogWriter.Tests/BlogWorkflowTests.cs b/BlogWriter.Tests/BlogWorkflowTests.cs index 4defff7..a8d2f57 100644 --- a/BlogWriter.Tests/BlogWorkflowTests.cs +++ b/BlogWriter.Tests/BlogWorkflowTests.cs @@ -8,11 +8,13 @@ public sealed class BlogWorkflowTests [Fact] public async Task RunAsync_EmitsLifecycleAndReviewerUpdatesWithoutChangingFinalState() { + var author = new TestAuthor("draft"); + var reviewer = new TestReviewer("APPROVED"); var workflow = new BlogWorkflow( new TestBlogger(), new TestResearcher(), - new TestAuthor(), - new TestReviewer(), + author, + reviewer, NullLogger.Instance); var output = new WorkflowOutputCollector(); @@ -32,6 +34,53 @@ public async Task RunAsync_EmitsLifecycleAndReviewerUpdatesWithoutChangingFinalS update.Kind == WorkflowOutputKind.Lifecycle && update.Outcome == WorkflowOutputOutcome.Success); Assert.Equal(output.Updates.Count, output.Updates.Select(update => update.Sequence).Distinct().Count()); + Assert.Equal(1, author.Calls); + Assert.Equal(1, reviewer.Calls); + } + + [Fact] + public async Task RunAsync_RejectedInitialDraftGetsOneRevisionAndNoSecondReview() + { + var author = new TestAuthor("draft-1", "draft-2"); + var reviewer = new TestReviewer("Please revise the introduction."); + var workflow = new BlogWorkflow( + new TestBlogger(), + new TestResearcher(), + author, + reviewer, + NullLogger.Instance); + var output = new WorkflowOutputCollector(); + + var service = new BlogWriterSessionService(workflow, new RecordingStore()); + BlogSession session = await service.StartAsync("topic", output: output); + + Assert.Equal("draft-2", session.State.Draft); + Assert.Equal("Please revise the introduction.", session.State.ReviewNotes); + Assert.Equal(2, author.Calls); + Assert.Equal(1, reviewer.Calls); + Assert.Contains(output.Updates, update => + update.Kind == WorkflowOutputKind.Lifecycle && + update.Outcome == WorkflowOutputOutcome.Success); + } + + [Fact] + public async Task RunAsync_RejectedRevisionWithoutReplacementKeepsLatestDraftAndNoSecondReview() + { + var author = new TestAuthor("draft-1", null); + var reviewer = new TestReviewer("Please revise the introduction."); + var workflow = new BlogWorkflow( + new TestBlogger(), + new TestResearcher(), + author, + reviewer, + NullLogger.Instance); + + var service = new BlogWriterSessionService(workflow, new RecordingStore()); + BlogSession session = await service.StartAsync("topic"); + + Assert.Equal("draft-1", session.State.Draft); + Assert.Equal(2, author.Calls); + Assert.Equal(1, reviewer.Calls); } private sealed class TestBlogger : IBloggerAgent @@ -59,26 +108,40 @@ public Task ResearchNodeAsync(ResearchState state, CancellationTo } } - private sealed class TestAuthor : IAuthorAgent + private sealed class TestAuthor(params string?[] drafts) : IAuthorAgent { + private readonly IReadOnlyList _drafts = drafts; + + public int Calls { get; private set; } + public Task InvokeAsync(ResearchState state, CancellationToken cancellationToken = default) => - Task.FromResult("draft"); + Task.FromResult(_drafts[Math.Min(Calls, _drafts.Count - 1)]); public Task AuthorNodeAsync(ResearchState state, CancellationToken cancellationToken = default) { - state.Draft = "draft"; + string? draft = _drafts[Math.Min(Calls, _drafts.Count - 1)]; + Calls++; + state.RevisionNumber++; + if (!string.IsNullOrEmpty(draft)) + { + state.Draft = draft; + } + return Task.FromResult(state); } } - private sealed class TestReviewer : IReviewerAgent + private sealed class TestReviewer(string reviewNotes) : IReviewerAgent { + public int Calls { get; private set; } + public Task InvokeAsync(ResearchState state, CancellationToken cancellationToken = default) => - Task.FromResult("APPROVED"); + Task.FromResult(reviewNotes); public Task ReviewerNodeAsync(ResearchState state, CancellationToken cancellationToken = default) { - state.ReviewNotes = "APPROVED"; + Calls++; + state.ReviewNotes = reviewNotes; return Task.FromResult(state); } } diff --git a/ResearchState.cs b/ResearchState.cs index 22a5ca6..b56696c 100644 --- a/ResearchState.cs +++ b/ResearchState.cs @@ -3,7 +3,7 @@ namespace BlogWriter; /// State for the research workflow. public class ResearchState { - /// Hard upper bound on author/review revision cycles. Guarantees the workflow terminates. + /// Hard upper bound on author passes: the initial draft plus one revision. Guarantees the workflow terminates. public const int MaxRevisions = 2; /// The single source-of-truth marker written to on approval. diff --git a/Workflows/BlogExecutors.cs b/Workflows/BlogExecutors.cs index 84f593c..2814882 100644 --- a/Workflows/BlogExecutors.cs +++ b/Workflows/BlogExecutors.cs @@ -27,12 +27,21 @@ internal sealed partial class AuthorExecutor(IAuthorAgent author) : Executor("Au { [MessageHandler] private async ValueTask HandleAsync(ResearchState state, IWorkflowContext context, CancellationToken cancellationToken) - => await author.AuthorNodeAsync(state, cancellationToken); + { + state = await author.AuthorNodeAsync(state, cancellationToken); + + if (state.RevisionNumber >= ResearchState.MaxRevisions) + { + await context.YieldOutputAsync(state); + } + + return state; + } } /// -/// Reviews the draft and records approval / revision notes. Acts as the terminal -/// output node: when no further revision is needed it yields the final state. +/// Reviews the initial draft and records approval / revision notes. It yields +/// approved output; a rejected draft routes to the author for one revision. /// internal sealed partial class ReviewerExecutor( IReviewerAgent reviewer, @@ -50,12 +59,12 @@ private async ValueTask HandleAsync(ResearchState state, IWorkflo if (!state.NeedsRevision) { - // Approved, or the revision cap was hit — emit the final result. + // Approval is terminal here. A capped revision is emitted by the author. await context.YieldOutputAsync(state); } - // Returned state is routed back to the author only when the loop edge - // condition (NeedsRevision) is satisfied; otherwise it goes nowhere. + // Rejected initial state is routed back to the author only when the loop + // edge condition (NeedsRevision) is satisfied. return state; } } diff --git a/specs/013-single-review-round/checklists/requirements.md b/specs/013-single-review-round/checklists/requirements.md new file mode 100644 index 0000000..51dcbfe --- /dev/null +++ b/specs/013-single-review-round/checklists/requirements.md @@ -0,0 +1,35 @@ +# Specification Quality Checklist: Single Review Round + +**Purpose**: Validate specification completeness and quality before proceeding to planning +**Created**: 2026-09-27 +**Feature**: [spec.md](../spec.md) + +## Content Quality + +- [x] No implementation details (languages, frameworks, APIs) +- [x] Focused on user value and business needs +- [x] Written for non-technical stakeholders +- [x] All mandatory sections completed + +## Requirement Completeness + +- [x] No [NEEDS CLARIFICATION] markers remain +- [x] Requirements are testable and unambiguous +- [x] Success criteria are measurable +- [x] Success criteria are technology-agnostic (no implementation details) +- [x] All acceptance scenarios are defined +- [x] Edge cases are identified +- [x] Scope is clearly bounded +- [x] Dependencies and assumptions identified + +## Feature Readiness + +- [x] All functional requirements have clear acceptance criteria +- [x] User scenarios cover primary flows +- [x] Feature meets measurable outcomes defined in Success Criteria +- [x] No implementation details leak into specification + +## Notes + +- Validation passed on the initial specification review. +- The feature is ready for `/speckit-plan`. diff --git a/specs/013-single-review-round/data-model.md b/specs/013-single-review-round/data-model.md new file mode 100644 index 0000000..8019fd6 --- /dev/null +++ b/specs/013-single-review-round/data-model.md @@ -0,0 +1,34 @@ +# Data Model: Single Review Round + +## ResearchState + +Existing workflow state remains the single source of truth. + +| Field | Role in this feature | Rules | +|---|---|---| +| `Draft` | Latest draft displayed to the user | Initial author output is replaced only by the one permitted revision when replacement content exists. | +| `ReviewNotes` | Reviewer decision and suggested changes | Approval uses the existing `APPROVED` marker; rejection notes are passed to the one revision and remain available for final display. | +| `RevisionNumber` | Number of author passes | Initial draft is `1`; one revision may reach `2`; no author pass beyond `2` is allowed for this workflow run. | +| `MaxRevisions` | Existing upper bound | Remains `2`, representing the initial draft plus one revision. | +| `NextStep` | Existing workflow status marker | Existing values remain unchanged unless current stage behavior requires the terminal revised-author path to mark completion. | + +## State transitions + +```text +Initial state + -> Author pass 1: Draft populated, RevisionNumber = 1 + -> Reviewer pass 1 + -> APPROVED: yield final state from ReviewerExecutor + -> Rejected: retain ReviewNotes and route to AuthorExecutor + -> Author pass 2: revise using ReviewNotes, RevisionNumber = 2 + -> yield final state from AuthorExecutor + +No transition from Author pass 2 to ReviewerExecutor exists. +``` + +## Validation rules + +- Reviewer call count is at most one per workflow run. +- Author call count is one for approval and two for rejection followed by revision. +- The final state is emitted after initial approval or after the single revision attempt. +- If the revision produces no replacement content, the existing latest draft remains the displayed final draft, consistent with current author fallback behavior. diff --git a/specs/013-single-review-round/plan.md b/specs/013-single-review-round/plan.md new file mode 100644 index 0000000..d2290a0 --- /dev/null +++ b/specs/013-single-review-round/plan.md @@ -0,0 +1,105 @@ +# Implementation Plan: Single Review Round + +**Branch**: `013-single-review-round` | **Date**: 2026-09-27 | **Spec**: [spec.md](spec.md) + +**Input**: Feature specification from `/specs/013-single-review-round/spec.md` + +**Note**: This template is filled in by the `/speckit-plan` command; its definition describes the execution workflow. + +## Summary + +Bound the blog workflow to one reviewer pass and, when needed, one author revision. The existing revision counter remains the state boundary: the initial draft is author pass 1, a rejected draft may return to the author for pass 2, and pass 2 becomes a terminal output without a second reviewer invocation. + +## Technical Context + + + +**Language/Version**: C# on .NET 10 with nullable reference types enabled + +**Primary Dependencies**: Microsoft Agent Framework Workflows 1.10.0, workflow generators 1.10.0, Microsoft.Extensions.AI 10.9.0, xUnit test projects + +**Storage**: Existing session stores persist `ResearchState`; no schema or storage format change is planned + +**Testing**: Focused xUnit tests in `BlogWriter.Tests`, followed by the affected project suite and MAF topology validation + +**Target Platform**: Existing console and hosted web application workflow runtime + +**Project Type**: .NET console workflow service with a hosted web presentation and independently deployed Foundry agents + +**Performance Goals**: Rejected drafts must complete with no more than one revision and one reviewer call; approved drafts must complete without a revision + +**Constraints**: Preserve the existing `ResearchState` persistence contract, token-budget handling, cancellation, output publishing, and bounded termination. Do not add credentials, raw HTTP, retained agent session state, or new message-handler shapes. + +**Scale/Scope**: One workflow run at a time per session; changes are limited to the workflow topology, terminal output routing, and focused regression tests + +## Constitution Check + +*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.* + +- **Hosted-Agent Boundaries**: PASS. The change only routes existing Blogger, Researcher, Author, and Reviewer stages; it does not move model logic or add transport calls. +- **MAF-Native Workflow Composition**: PASS. Routing remains explicit through `ResearchState`, `WorkflowBuilder` edges, executor handlers, and workflow output events. +- **Identity, Secrets, and Budget Control**: PASS. No authentication or secret changes are introduced; existing cancellation and token-cap propagation remain intact. +- **Testable and Observable Behavior**: PASS. Add call-count and final-output tests using existing test doubles and preserve lifecycle/reviewer output publishing. +- **Simple, Compatible Evolution**: PASS. Reuse `RevisionNumber` and `MaxRevisions`; do not add a persisted flag or change session serialization. +- **MAF baseline**: MAF Doctor currently reports four pre-existing credential errors, three observability warnings, and six heuristic uncapped-call findings. These are outside this feature's scope and are not worsened by the planned changes. + +## Project Structure + +### Documentation (this feature) + +```text +specs/013-single-review-round/ +├── plan.md # This file (/speckit-plan command output) +├── research.md # Phase 0 output (/speckit-plan command) +├── data-model.md # Phase 1 output (/speckit-plan command) +├── quickstart.md # Phase 1 output (/speckit-plan command) +└── tasks.md # Phase 2 output (/speckit-tasks command - NOT created by /speckit-plan) +``` + +### Source Code (repository root) + +```text +BlogWorkflow.cs # Conditional author/reviewer edges and output sources +Workflows/BlogExecutors.cs # Terminal output after the single revised author pass +ResearchState.cs # Existing revision-boundary semantics and documentation +BlogWriter.Tests/BlogWorkflowTests.cs # Approval/rejection call-count and output regressions +BlogWriter.Tests/ResearchStateTests.cs # Revision-boundary state assertions if semantics change +``` + +**Structure Decision**: Keep the existing root workflow implementation and focused unit-test project. No new source project, API contract, persistence model, or frontend component is needed. + + +## Phase 0: Research Summary + +Research findings are recorded in [research.md](research.md). The design uses the existing counter and explicit conditional edges. The installed MAF API supports multiple output sources through `WithOutputFrom(ExecutorBinding[])`, and the relevant APIs are safe according to the current registry. + +## Phase 1: Design Summary + +- [data-model.md](data-model.md) defines the existing state fields and the author/reviewer lifecycle. +- No `contracts/` artifact is required because this is an internal workflow topology change with no external interface. +- [quickstart.md](quickstart.md) defines focused approval, rejection/revision, fallback, and broader test validation. + +## Implementation Design + +1. In `BlogWorkflow.cs`, keep the reviewer-to-author edge conditioned on `NeedsRevision`, make the author-to-reviewer edge conditional on `RevisionNumber < MaxRevisions`, and register both `ReviewerExecutor` and `AuthorExecutor` as output sources. +2. In `Workflows/BlogExecutors.cs`, have `AuthorExecutor` yield the state when its author pass reaches `MaxRevisions`; preserve the existing reviewer yield for initial approval and existing reviewer feedback publishing. +3. In `ResearchState.cs`, retain the value `MaxRevisions = 2` and clarify that it bounds total author passes for this workflow, if the current wording is misleading. Preserve `NeedsRevision` and `RevisionLimitReached` behavior unless focused tests show a necessary adjustment. +4. In `BlogWriter.Tests/BlogWorkflowTests.cs`, add counting test doubles and assertions for initial approval, initial rejection followed by one revision, and no second reviewer call. Verify final draft and output events. +5. Update `ResearchStateTests.cs` only for any directly affected boundary wording or state invariant; do not broaden unrelated state tests. + +## Validation Plan + +- Run the focused `BlogWorkflowTests` command in [quickstart.md](quickstart.md). +- Run the full `BlogWriter.Tests` project suite. +- Run MAF workflow topology simulation and confirm both terminal-capable paths complete without silent starvation. +- Build the affected projects and confirm nullable/compiler diagnostics remain clean. + +## Complexity Tracking + +| Violation | Why Needed | Simpler Alternative Rejected Because | +|-----------|------------|-------------------------------------| +| None | N/A | The existing workflow and test projects are sufficient. | diff --git a/specs/013-single-review-round/quickstart.md b/specs/013-single-review-round/quickstart.md new file mode 100644 index 0000000..a7b84c5 --- /dev/null +++ b/specs/013-single-review-round/quickstart.md @@ -0,0 +1,41 @@ +# Quickstart: Single Review Round + +## Prerequisites + +- .NET 10 SDK +- Repository dependencies restored +- No live Foundry service is required; the focused tests use test doubles for Blogger, Researcher, Author, and Reviewer. + +## Focused validation + +From the repository root: + +```powershell +dotnet test .\BlogWriter.Tests\BlogWriter.Tests.csproj --filter FullyQualifiedName~BlogWorkflowTests --no-restore -p:BaseOutputPath=.\obj\single-review-test\ +``` + +Expected result: all `BlogWorkflowTests` pass. + +## Scenarios to verify + +1. **Initial approval** + - Reviewer returns `APPROVED` for the initial draft. + - Expected: the initial draft is emitted, the reviewer is called once, and the author is called once. + +2. **Initial rejection and one revision** + - Reviewer returns revision feedback for the initial draft. + - Expected: the author receives the feedback, produces one revised draft, the revised draft is emitted, the reviewer is still called only once, and the author is called twice. + +3. **Revision fallback** + - The author returns no replacement content on the revision pass. + - Expected: the latest available draft remains final and no second review is attempted. + +## Broader validation + +After the focused tests pass, run the affected project test suite: + +```powershell +dotnet test .\BlogWriter.Tests\BlogWriter.Tests.csproj --no-restore -p:BaseOutputPath=.\obj\single-review-test\ +``` + +The implementation should also be checked with the repository's MAF workflow topology simulation to confirm that both terminal output sources and the conditional edges complete without silent starvation. diff --git a/specs/013-single-review-round/research.md b/specs/013-single-review-round/research.md new file mode 100644 index 0000000..1a366f9 --- /dev/null +++ b/specs/013-single-review-round/research.md @@ -0,0 +1,39 @@ +# Research: Single Review Round + +## Decision: Use the existing bounded state counter and add a terminal revised-author path + +- Keep `ResearchState.RevisionNumber` as the count of author passes: the initial draft is pass 1 and the single permitted revision is pass 2. +- Keep `ResearchState.MaxRevisions` at 2 so the existing state cap represents the initial draft plus one revision. +- Make the author-to-reviewer edge conditional on the author pass being below the cap. +- Allow `AuthorExecutor` to yield the state directly when it completes the capped revision. +- Configure both `ReviewerExecutor` and `AuthorExecutor` as workflow output sources. The reviewer yields an accepted initial draft; the author yields the final revised draft. +- Retain the reviewer-to-author edge only when `ResearchState.NeedsRevision` is true. + +## Rationale + +The current topology always sends every author result to the reviewer, so a rejected initial draft is reviewed twice. Conditional routing at the author edge prevents the second reviewer invocation while preserving the existing reviewer decision and feedback state. Explicit output from the author executor makes the revised draft observable through the same `WorkflowOutputEvent` path used by the reviewer. + +## API and compatibility findings + +- The installed Microsoft Agent Framework workflow API exposes `WorkflowBuilder.WithOutputFrom(ExecutorBinding[])`, allowing both terminal-capable executors to be registered. +- `WorkflowBuilder.AddEdge`, `IWorkflowContext.YieldOutputAsync`, and `WorkflowBuilder.WithOutputFrom` are marked SAFE by maf-doctor for the current tracked MAF release. +- Existing handlers return `ValueTask`, which the workflow topology simulator reports as complete and fan-in safe. +- No new agent, credential, session-state, or message-handler pattern is required. + +## Alternatives considered + +### Keep the existing unconditional author-to-reviewer edge and lower the cap + +Rejected because lowering the cap alone still invokes the reviewer after the revision; it only changes whether that second review yields output. + +### Add a separate boolean revision-complete flag + +Rejected because the existing revision counter already distinguishes the initial author pass from the single revised pass and adding another state field would expand persistence and cloning requirements unnecessarily. + +### Bypass the reviewer after rejection by changing the reviewer executor to invoke the author + +Rejected because it would combine stage responsibilities, obscure workflow topology, and make author/reviewer call counts harder to test independently. + +## Baseline risks to track separately + +MAF Doctor currently reports pre-existing repository findings: four production `DefaultAzureCredential` errors, three observability warnings, and six heuristic uncapped-call findings. This feature does not alter those areas. The plan preserves the existing token-budget and observability behavior and does not include unrelated remediation. diff --git a/specs/013-single-review-round/spec.md b/specs/013-single-review-round/spec.md new file mode 100644 index 0000000..3588546 --- /dev/null +++ b/specs/013-single-review-round/spec.md @@ -0,0 +1,80 @@ +# Feature Specification: Single Review Round + +**Feature Branch**: `013-single-review-round` + +**Created**: 2026-09-27 + +**Status**: Draft + +**Input**: User description: "At most one round of revision should occur. The reviewer either accepts the initial draft (in which case it is displayed and the round ends) or the reviewer rejects the draft and sends it back to the author with suggested changes. In that event, the author revises the draft and it is immediately displayed -- it does not go back to the reviewer for a second review." + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 - Display an accepted initial draft (Priority: P1) + +As a blog writer, I want an initially generated draft that passes review to be displayed immediately so I can read the completed result without unnecessary additional processing. + +**Why this priority**: Approval is the shortest successful path and must remain reliable for every accepted draft. + +**Independent Test**: Start a writing workflow with a reviewer that accepts the initial draft and verify that the initial draft is displayed and no revision is requested. + +**Acceptance Scenarios**: + +1. **Given** an initial draft has been generated, **When** the reviewer accepts it, **Then** the system displays that initial draft as the final result. +2. **Given** the reviewer accepts the initial draft, **When** the workflow completes, **Then** no author revision is requested and no additional review occurs. + +--- + +### User Story 2 - Display one revision after rejection (Priority: P1) + +As a blog writer, I want reviewer feedback to produce one revised draft so that clear improvements are incorporated without creating an open-ended review loop. + +**Why this priority**: A rejected initial draft needs one opportunity to incorporate actionable feedback while the workflow remains bounded and predictable. + +**Independent Test**: Start a writing workflow with a reviewer that rejects the initial draft and supplies suggested changes, then verify that the author receives the feedback once and the revised draft is displayed without a second review. + +**Acceptance Scenarios**: + +1. **Given** an initial draft has been generated, **When** the reviewer rejects it with suggested changes, **Then** the author receives the rejected draft and the suggestions for one revision. +2. **Given** the author receives the review suggestions, **When** the author completes the revision, **Then** the revised draft is displayed as the final result. +3. **Given** the revised draft is displayed, **When** the workflow completes, **Then** the reviewer is not invoked again. + +--- + +### Edge Cases + +- If the reviewer rejects the initial draft without usable suggestions, the author still receives one revision opportunity and the workflow remains bounded. +- If the author cannot produce a different draft after rejection, the latest available draft is displayed and the workflow ends without another review attempt. +- A reviewer approval on the initial draft must not consume or trigger a revision round. +- A rejected initial draft must never cause more than one author revision. + +## Requirements *(mandatory)* + +### Functional Requirements + +- **FR-001**: The system MUST submit the initial draft to the reviewer exactly once before deciding whether a revision is needed. +- **FR-002**: When the reviewer accepts the initial draft, the system MUST display that draft as the final result and end the workflow. +- **FR-003**: When the reviewer rejects the initial draft, the system MUST provide the author's next revision with the reviewer's suggested changes. +- **FR-004**: The system MUST allow no more than one author revision after rejection of the initial draft. +- **FR-005**: After the single revision is produced, the system MUST display the revised draft as the final result without submitting it for a second review. +- **FR-006**: The workflow MUST terminate after either initial approval or display of the single revised draft. +- **FR-007**: The system MUST preserve the latest available draft for display when a requested revision cannot produce replacement content. +- **FR-008**: The system MUST make the completed result and workflow termination observable to the user through the existing result and status presentation. + +## Success Criteria *(mandatory)* + +### Measurable Outcomes + +- **SC-001**: 100% of workflows with an accepted initial draft display that draft and invoke no revision. +- **SC-002**: 100% of workflows with a rejected initial draft invoke at most one author revision and at most one review. +- **SC-003**: 100% of workflows display a final draft after either initial approval or the single revision attempt, including when replacement content is unavailable. +- **SC-004**: In usability checks, users can identify the final displayed draft and completion state on the first result view in at least 95% of attempts. +- **SC-005**: No workflow remains active or requests further review after the initial approval or single revision result is displayed. + +## Assumptions + +- The existing reviewer determines acceptance or rejection and provides textual suggestions when rejecting a draft. +- "One round of revision" means at most one author revision after review of the initial draft; it does not include generating the initial draft. +- The revised draft is considered the final displayed result without a second reviewer decision, as explicitly requested. +- Existing workflow cancellation, error reporting, token-budget, and session behavior remain unchanged except where needed to terminate the review loop. +- This feature changes review-loop bounds and result presentation only; it does not change research, initial drafting, or reviewer evaluation criteria. diff --git a/specs/013-single-review-round/tasks.md b/specs/013-single-review-round/tasks.md new file mode 100644 index 0000000..2b17b4e --- /dev/null +++ b/specs/013-single-review-round/tasks.md @@ -0,0 +1,138 @@ +--- + +description: "Task list for the single review round feature" +--- + +# Tasks: Single Review Round + +**Input**: Design documents from `/specs/013-single-review-round/` + +**Prerequisites**: `plan.md`, `spec.md`, `research.md`, `data-model.md`, and `quickstart.md` + +**Organization**: Tasks are grouped by user story. Both stories are P1; User Story 1 is the MVP approval path and User Story 2 adds the rejected-draft revision path. + +## Phase 1: Setup (Shared Infrastructure) + +**Purpose**: Establish the focused validation baseline before changing the workflow. + +- [X] T001 Run the focused baseline command from `specs/013-single-review-round/quickstart.md` against `BlogWriter.Tests/BlogWriter.Tests.csproj` and record the current approval-path result before implementation. + +--- + +## Phase 2: Foundational (Blocking Prerequisites) + +**Purpose**: Preserve the shared state and output contracts used by both user stories. + +- [X] T002 [P] Verify and, only if wording is misleading, update the `MaxRevisions` and `RevisionNumber` documentation in `ResearchState.cs` so the cap explicitly represents author passes 1 and 2 without changing the persisted state shape. +- [X] T003 [P] Add reusable author/reviewer call-count and output-capture test support in `BlogWriter.Tests/BlogWorkflowTests.cs` without changing production interfaces. + +**Checkpoint**: Shared state semantics and test instrumentation are ready; user story work can proceed in priority order. + +--- + +## Phase 3: User Story 1 - Display an accepted initial draft (Priority: P1) 🎯 MVP + +**Goal**: An initially approved draft is emitted as the final result with no revision. + +**Independent Test**: Configure the test reviewer to return `APPROVED`; verify the initial draft is emitted, the author runs once, the reviewer runs once, and the workflow publishes completion. + +### Tests for User Story 1 + +- [X] T004 [US1] Add an approval-path regression test in `BlogWriter.Tests/BlogWorkflowTests.cs` that asserts the initial draft remains final, the author and reviewer call counts are both one, and the final output event is emitted. + +### Implementation for User Story 1 + +- [X] T005 [US1] Update `BlogWorkflow.cs` to register `ReviewerExecutor` as the approval terminal output and preserve the existing reviewer-to-author conditional edge for rejected drafts. +- [X] T006 [US1] Preserve the approval behavior in `Workflows/BlogExecutors.cs` so `ReviewerExecutor` yields an approved initial state and publishes reviewer feedback exactly once. + +**Checkpoint**: User Story 1 is independently testable as the MVP approval flow. + +--- + +## Phase 4: User Story 2 - Display one revision after rejection (Priority: P1) + +**Goal**: A rejected initial draft receives exactly one author revision, which is displayed immediately without a second review. + +**Independent Test**: Configure the reviewer to reject the initial draft with feedback; verify the author runs twice, the reviewer runs once, the revised/latest draft is emitted, and no second review occurs. + +### Tests for User Story 2 + +- [X] T007 [US2] Add rejection-and-revision regression tests in `BlogWriter.Tests/BlogWorkflowTests.cs` covering reviewer feedback delivery, exactly one author revision, exactly one reviewer call, final revised draft output, and the no-replacement-content fallback. + +### Implementation for User Story 2 + +- [X] T008 [US2] Make the author-to-reviewer edge conditional on `RevisionNumber < ResearchState.MaxRevisions` and register `AuthorExecutor` as the second terminal output source in `BlogWorkflow.cs`. +- [X] T009 [US2] Update `AuthorExecutor` in `Workflows/BlogExecutors.cs` to yield the state when the single revision reaches `ResearchState.MaxRevisions`, while retaining the existing author node and fallback-draft behavior. +- [X] T010 [US2] Update `BlogWriter.Tests/ResearchStateTests.cs` only if the implementation changes a revision-boundary invariant, asserting that the capped revised state cannot request another review or revision. No state invariant changed, so existing tests remain sufficient. + +**Checkpoint**: User Stories 1 and 2 both terminate with a displayed final draft and no workflow review loop beyond the requested single revision. + +--- + +## Phase 5: Polish & Cross-Cutting Validation + +**Purpose**: Validate the complete feature and preserve documented operational behavior. + +- [X] T011 Run the full `BlogWriter.Tests/BlogWriter.Tests.csproj` suite using the isolated output command in `specs/013-single-review-round/quickstart.md` and resolve only regressions caused by this feature. +- [X] T012 Run the MAF workflow topology simulation and affected-project build for `BlogWorkflow.cs` and `Workflows/BlogExecutors.cs`; confirm both terminal output paths complete without silent starvation and nullable/compiler diagnostics remain clean. + +--- + +## Dependencies & Execution Order + +### Phase Dependencies + +- **Phase 1 (Setup)**: T001 has no dependencies and establishes the baseline. +- **Phase 2 (Foundational)**: T002 and T003 depend on T001 and can run in parallel because they touch different files. +- **Phase 3 (US1 MVP)**: T004 depends on T003; T005 and T006 depend on T004 and can be completed together as the approval-path implementation. +- **Phase 4 (US2)**: T007 depends on the shared test support and the US1 baseline; T008 and T009 depend on T007; T010 is conditional on whether the state invariant changes. +- **Phase 5 (Polish)**: T011 and T012 depend on the completed desired user stories. + +### User Story Dependencies + +- **User Story 1 (P1)**: Depends on Foundational tasks; independently validates the accepted initial draft and is the MVP. +- **User Story 2 (P1)**: Extends the same workflow topology after User Story 1; it depends on the approval output path but remains independently testable with rejection-focused doubles. + +### Parallel Opportunities + +- T002 and T003 can run in parallel after T001. +- Within User Story 1, T005 and T006 can be implemented in parallel after T004 when the shared topology contract is agreed. +- T011 and T012 can run in parallel after both story checkpoints. + +## Parallel Example: User Story 1 + +```text +Task: T005 Update BlogWorkflow.cs for the reviewer terminal output path +Task: T006 Preserve ReviewerExecutor approval output in Workflows/BlogExecutors.cs +``` + +## Parallel Example: User Story 2 + +```text +Task: T008 Update conditional edges and output sources in BlogWorkflow.cs +Task: T009 Add terminal output after the capped author revision in Workflows/BlogExecutors.cs +``` + +## Implementation Strategy + +### MVP First (User Story 1 Only) + +1. Complete T001 through T003. +2. Complete T004 through T006. +3. Run the User Story 1 focused test and stop for validation if only approval behavior is required. + +### Incremental Delivery + +1. Establish the baseline and shared test/state contracts. +2. Deliver User Story 1 as the approval-path MVP. +3. Add User Story 2's conditional routing and terminal revised-author output. +4. Run full tests and MAF topology/build validation. + +### Notes + +- Every task is independently actionable, uses the required checkbox/ID format, and names the file or validation artifact it affects. +- No new project, storage migration, external contract, credential, or prompt change is required. + +## Phase 6: Convergence + +- [X] T013 Update the stale final-output comment in `BlogWorkflow.cs` to document that `WorkflowOutputEvent` may be emitted by either `ReviewerExecutor` after approval or `AuthorExecutor` after the single revision per plan: Implementation Design 1-2 (partial)