75 lines
7.0 KiB
Markdown
75 lines
7.0 KiB
Markdown
# Review & Roadblock UX Implementation Plan
|
||
|
||
> **For agentic workers:** execute task-by-task (subagent-driven-development). Steps use `- [ ]`.
|
||
|
||
**Goal:** Move the task-row review actions into the Details panel, give the Details panel a real `WaitingForReview` state + a populated diff meter, and add a glanceable yellow roadblock indicator on the task card.
|
||
|
||
**Architecture:** Persist a `RoadblockCount` on `TaskEntity` (set by the runner when it folds in `CLAUDEDO_BLOCKED` markers). The row shows a warning badge when count > 0; review controls relocate to `DetailsIslandView`.
|
||
|
||
**Tech Stack:** .NET 8, Avalonia, EF Core (one migration), xUnit.
|
||
|
||
**Coordination:** A second session (`claudedo-childloop`) is building the child-tasks/improvement-loop in a worktree and will rebase onto main *after* these commits. It also touches `DetailsIslandViewModel`, `TaskRowView.axaml`, `TaskStateService`, `TaskStatus`. This plan deliberately stays OUT of `TaskStateService` and the `TaskStatus` enum (persisting `RoadblockCount` from the runner via the repository instead).
|
||
|
||
Build/test (per-project, .NET 8):
|
||
```bash
|
||
dotnet build src/ClaudeDo.App/ClaudeDo.App.csproj -c Release
|
||
dotnet build src/ClaudeDo.Worker/ClaudeDo.Worker.csproj -c Release
|
||
dotnet test tests/ClaudeDo.Data.Tests/ClaudeDo.Data.Tests.csproj -c Release
|
||
dotnet test tests/ClaudeDo.Worker.Tests/ClaudeDo.Worker.Tests.csproj -c Release
|
||
```
|
||
|
||
---
|
||
|
||
## Task A — Persist RoadblockCount (Data + Worker, no UI)
|
||
|
||
**Files:** `TaskEntity.cs`, `TaskEntityConfiguration.cs`, new migration, `TaskRepository.cs`, `TaskRunner.cs`; test in `tests/ClaudeDo.Data.Tests`.
|
||
|
||
- Add `public int RoadblockCount { get; set; }` to `TaskEntity` (default 0).
|
||
- Map it in `TaskEntityConfiguration` to column `roadblock_count` (default 0). Mirror the pattern used by an existing scalar column (e.g. how `DailyPrepMaxTasks`/other ints are configured).
|
||
- Create EF migration `AddRoadblockCount` (run `dotnet ef migrations add AddRoadblockCount` against `src/ClaudeDo.Data`; if EF tooling is unavailable, hand-author the migration + Designer + snapshot edit mirroring the most recent migration). One column, default 0, no backfill needed.
|
||
- Add `TaskRepository.SetRoadblockCountAsync(string taskId, int count, CancellationToken ct)` using `ExecuteUpdateAsync` on `RoadblockCount`.
|
||
- In `TaskRunner.HandleSuccess`, BEFORE the terminal state write (`SubmitForReviewAsync`/`CompleteAsync`), call `SetRoadblockCountAsync(task.Id, result.Blocks.Count, CancellationToken.None)` so the `TaskUpdated` broadcast reflects it. (Do NOT route this through `TaskStateService`.)
|
||
- Test: a `TaskRepository` test that sets a count and reads it back.
|
||
- Commit: `feat(roadblock): persist roadblock count on the task`.
|
||
|
||
**Acceptance:** a finished run with N roadblocks leaves `tasks.roadblock_count = N`; a clean run leaves 0.
|
||
|
||
---
|
||
|
||
## Task B — Detail panel: host review actions + real WaitingForReview state + diff meter
|
||
|
||
**Files:** `DetailsIslandViewModel.cs`, `DetailsIslandView.axaml` (+ `.axaml.cs` if needed), locales if new keys; reuse `IWorkerClient.ApproveReview/RejectReviewToQueue/RejectReviewToIdle/CancelReview` (already exist).
|
||
|
||
1. **WaitingForReview state:**
|
||
- In `StatusToStateKey` map `WaitingForReview => "review"` (was `"running"`); in `FinishedStatusToStateKey` map `"waiting_for_review" => "review"`.
|
||
- Add `public bool IsWaitingForReview => AgentState == "review";` and raise it in `OnAgentStateChanged`.
|
||
- Add a `vm.agentStatus.review` locale key (en + de, parity) for the status label.
|
||
- Confirm `IsAgentSectionEnabled => !IsRunning` still holds (review is no longer "running", so the agent settings section re-enables in review — correct).
|
||
2. **Review actions (moved from the row):** add commands to `DetailsIslandViewModel` that call the worker for the selected task: `ApproveReviewCommand`, `RejectReviewCommand` (takes feedback text → `RejectReviewToQueueAsync`), `ParkReviewCommand` (`RejectReviewToIdleAsync`), `CancelReviewCommand` (`CancelReviewAsync`). Add a `ReviewFeedback` string property for the rejection comment. Mirror how the row's code-behind currently invokes these (see `TaskRowView.axaml.cs`).
|
||
- In `DetailsIslandView.axaml`, add a review section (visible when `IsWaitingForReview` and `IsTaskDetailVisible`) with Approve / Reject(+feedback box) / Park / Cancel, reusing the existing `tasks.approve/reject/park/cancel` + `tasks.feedback*` locale keys.
|
||
3. **Diff meter:** in `RefreshWorktreeAsync`, after setting `row.DiffStat`, parse the `--stat` summary into additions/deletions and assign `DiffAdditions`/`DiffDeletions` (drives `DiffMeterRatio`). Add a small static parser `ParseDiffStat(string?) -> (int add, int del)` reading the "N insertions(+), M deletions(-)" tail; unit-test it.
|
||
- Commit: `feat(ui): host review actions in the details panel; show review state and diff meter`.
|
||
|
||
**Acceptance:** selecting a `WaitingForReview` task shows a "review" status (not "running"), the four review actions work from the detail panel, and the diff meter reflects real additions/deletions.
|
||
|
||
---
|
||
|
||
## Task C — Task row: remove review buttons, add roadblock badge
|
||
|
||
**Files:** `TaskRowView.axaml`, `TaskRowView.axaml.cs`, `TaskRowViewModel.cs`; warning icon resource if missing.
|
||
|
||
- Remove the review-actions `StackPanel` (lines ~142–157) and the now-unused `RejectAnchor` flyout (~250–279) from `TaskRowView.axaml`, and the corresponding click handlers (`OnApproveReviewClick`, `OnRejectReviewClick`, `OnParkReviewClick`, `OnCancelReviewClick`, reject-flyout handlers) from the code-behind. (Review now lives in the detail panel — Task B.)
|
||
- `TaskRowViewModel`: add `int RoadblockCount` + `bool HasRoadblock => RoadblockCount > 0` + `string RoadblockTooltip` (e.g. `"{n} roadblock(s) reported — see details"`); map `RoadblockCount` in `FromEntity`.
|
||
- `TaskRowView.axaml`: add a yellow warning `PathIcon` immediately left of the action area (in the chip row, before the status chip or before the star — pick the spot that reads as "left of the Done/action button"), `IsVisible="{Binding HasRoadblock}"`, `ToolTip.Tip="{Binding RoadblockTooltip}"`. Use a filled-geometry warning icon (PathIcon fills geometry — a stroke path renders invisible); if no `Icon.Warning` resource exists, add one (filled triangle + exclamation) to the icon resources, colored with a yellow/amber brush.
|
||
- Commit: `feat(ui): roadblock badge on the task card; relocate review actions`.
|
||
|
||
**Acceptance:** rows no longer show the four review buttons; a task with `RoadblockCount > 0` shows a yellow ⚠ left of the action button with a tooltip; review still fully works via the detail panel.
|
||
|
||
---
|
||
|
||
## Task D — Build + visual-check
|
||
|
||
- Full build (`App` + `Worker`) and run Data + Worker test suites; all green.
|
||
- **Manual (flag for user):** start the app, take a `WaitingForReview` task (the deploy roadblock task qualifies), confirm: row shows the ⚠ badge + no row review buttons; detail panel shows "review" state, working review actions, and a non-zero diff meter for the farewell/README tasks. The agent cannot verify GUI — ask the user.
|
||
- Then ping `claudedo-childloop` via mailbox with the exact shared-file diffs so it can rebase.
|