UAT: `agents plan diff` output missing spec-required structured sections — diff_summary, patch_preview with hunks, risk_assessment, and per-file insertions/deletions counts #2480

New issue

Open

opened 2026-04-03 18:37:15 +00:00 by freemo · 1 comment

freemo commented

2026-04-03 18:37:15 +00:00

Owner

Metadata

Commit message: fix(cli): add structured diff output with patch preview and risk assessment to agents plan diff
Branch: fix/plan-diff-structured-output
Parent Epic: #397 (Epic: Server & Autonomy Infrastructure)

Bug Description

The agents plan diff command does not produce the spec-required structured output. The spec (§ agents plan diff, lines 15322–15521) defines a rich diff output with a Diff Summary panel, Files table with per-file insertion/deletion counts, Patch Preview with unified diff hunks, and a Risk Assessment panel. The current implementation renders a simplified hash-based diff without actual content.

Expected Behavior (from spec)

The spec requires the following output structure:

Rich format:

Diff Summary panel: Plan ID, Project, Files Changed, Insertions, Deletions, Net Change
Files table: Path, Change (+N -M), Status (modified/added/deleted)
Patch Preview panel: Actual unified diff with --- a/file, +++ b/file, @@ ... @@ hunks showing line-level changes
Risk Assessment panel: API Compatibility, Test Coverage, Breaking Changes

JSON format (spec lines 15424–15472):

{
  "data": {
    "diff_summary": {
      "plan": "...", "project": "...", "files_changed": 2,
      "insertions": 12, "deletions": 4, "net_change": "+8 lines"
    },
    "files": [
      { "path": "src/auth/session.py", "change": "+8 -2", "status": "modified" }
    ],
    "patch_preview": [
      {
        "file": "src/auth/session.py",
        "hunks": [
          { "range": "@@ -12,4 +12,10 @@", "deletions": [...], "insertions": [...] }
        ]
      }
    ],
    "risk_assessment": {
      "api_compatibility": "preserved",
      "test_coverage": "maintained",
      "breaking_changes": "none detected"
    }
  }
}

Actual Behavior

The agents plan diff command (lines 2673–2726 in src/cleveragents/cli/commands/plan.py) delegates to PlanApplyService.diff() which calls _render_diff_rich() / _render_diff_plain() / _render_diff_json() in plan_apply_service.py. These renderers:

Do NOT compute actual insertions/deletions counts — they only show content hashes (before_hash, after_hash)
Do NOT produce patch preview with unified diff hunks — no --- a/file, +++ b/file, @@ ... @@ hunk structure
Do NOT show per-file change summary (+8 -2 format)
Do NOT include risk assessment (API compatibility, test coverage, breaking changes)
Do NOT include project name in the diff summary

The DiffBuilder in domain/models/core/diff_review.py does produce proper unified diffs via _unified_diff(), but this builder is not used by PlanApplyService.diff(). The apply service uses its own simpler renderers that only show hash values.

Code Location

src/cleveragents/application/services/plan_apply_service.py, lines 80–155 (_render_diff_plain, _render_diff_rich, _render_diff_json)
src/cleveragents/domain/models/core/diff_review.py — DiffBuilder exists but is unused by the CLI diff command
src/cleveragents/cli/commands/plan.py, lines 2673–2726 (plan_diff command)

Root Cause

PlanApplyService.diff() uses its own simplified renderers instead of the DiffBuilder + DiffSerializer classes that already implement proper unified diff generation. The DiffBuilder requires before_contents and after_contents maps (actual file content) which need to be fetched from the sandbox or changeset store.

Subtasks

Wire DiffBuilder into PlanApplyService.diff() with actual file content from sandbox/changeset store
Add diff_summary with insertions/deletions counts computed from actual content
Add patch_preview with unified diff hunks (using DiffSerializer)
Add risk_assessment section (static analysis or heuristic-based)
Add per-file +N -M change summary in Files table
Add project name to diff summary
Add unit tests (Behave) for diff output rendering

Definition of Done

agents plan diff <PLAN_ID> renders Diff Summary, Files table, Patch Preview, and Risk Assessment panels in rich format
JSON/YAML/plain formats include diff_summary, files, patch_preview, and risk_assessment fields
Per-file insertion/deletion counts are computed from actual file content
Test coverage ≥ 97%

Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: ca-uat-tester

## Metadata **Commit message**: `fix(cli): add structured diff output with patch preview and risk assessment to agents plan diff` **Branch**: `fix/plan-diff-structured-output` **Parent Epic**: #397 (Epic: Server & Autonomy Infrastructure) ## Bug Description The `agents plan diff` command does not produce the spec-required structured output. The spec (§ `agents plan diff`, lines 15322–15521) defines a rich diff output with a Diff Summary panel, Files table with per-file insertion/deletion counts, Patch Preview with unified diff hunks, and a Risk Assessment panel. The current implementation renders a simplified hash-based diff without actual content. ## Expected Behavior (from spec) The spec requires the following output structure: **Rich format:** 1. **Diff Summary** panel: Plan ID, Project, Files Changed, Insertions, Deletions, Net Change 2. **Files** table: Path, Change (+N -M), Status (modified/added/deleted) 3. **Patch Preview** panel: Actual unified diff with `--- a/file`, `+++ b/file`, `@@ ... @@` hunks showing line-level changes 4. **Risk Assessment** panel: API Compatibility, Test Coverage, Breaking Changes **JSON format** (spec lines 15424–15472): ```json { "data": { "diff_summary": { "plan": "...", "project": "...", "files_changed": 2, "insertions": 12, "deletions": 4, "net_change": "+8 lines" }, "files": [ { "path": "src/auth/session.py", "change": "+8 -2", "status": "modified" } ], "patch_preview": [ { "file": "src/auth/session.py", "hunks": [ { "range": "@@ -12,4 +12,10 @@", "deletions": [...], "insertions": [...] } ] } ], "risk_assessment": { "api_compatibility": "preserved", "test_coverage": "maintained", "breaking_changes": "none detected" } } } ``` ## Actual Behavior The `agents plan diff` command (lines 2673–2726 in `src/cleveragents/cli/commands/plan.py`) delegates to `PlanApplyService.diff()` which calls `_render_diff_rich()` / `_render_diff_plain()` / `_render_diff_json()` in `plan_apply_service.py`. These renderers: 1. **Do NOT compute actual insertions/deletions counts** — they only show content hashes (`before_hash`, `after_hash`) 2. **Do NOT produce patch preview with unified diff hunks** — no `--- a/file`, `+++ b/file`, `@@ ... @@` hunk structure 3. **Do NOT show per-file change summary** (`+8 -2` format) 4. **Do NOT include risk assessment** (API compatibility, test coverage, breaking changes) 5. **Do NOT include project name** in the diff summary The `DiffBuilder` in `domain/models/core/diff_review.py` does produce proper unified diffs via `_unified_diff()`, but this builder is **not used** by `PlanApplyService.diff()`. The apply service uses its own simpler renderers that only show hash values. ## Code Location - `src/cleveragents/application/services/plan_apply_service.py`, lines 80–155 (`_render_diff_plain`, `_render_diff_rich`, `_render_diff_json`) - `src/cleveragents/domain/models/core/diff_review.py` — `DiffBuilder` exists but is unused by the CLI diff command - `src/cleveragents/cli/commands/plan.py`, lines 2673–2726 (`plan_diff` command) ## Root Cause `PlanApplyService.diff()` uses its own simplified renderers instead of the `DiffBuilder` + `DiffSerializer` classes that already implement proper unified diff generation. The `DiffBuilder` requires `before_contents` and `after_contents` maps (actual file content) which need to be fetched from the sandbox or changeset store. ## Subtasks - [ ] Wire `DiffBuilder` into `PlanApplyService.diff()` with actual file content from sandbox/changeset store - [ ] Add `diff_summary` with insertions/deletions counts computed from actual content - [ ] Add `patch_preview` with unified diff hunks (using `DiffSerializer`) - [ ] Add `risk_assessment` section (static analysis or heuristic-based) - [ ] Add per-file `+N -M` change summary in Files table - [ ] Add project name to diff summary - [ ] Add unit tests (Behave) for diff output rendering ## Definition of Done - `agents plan diff <PLAN_ID>` renders Diff Summary, Files table, Patch Preview, and Risk Assessment panels in rich format - JSON/YAML/plain formats include `diff_summary`, `files`, `patch_preview`, and `risk_assessment` fields - Per-file insertion/deletion counts are computed from actual file content - Test coverage ≥ 97% --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: ca-uat-tester

freemo referenced this issue

2026-04-03 18:43:10 +00:00

[CA-GROOMER] Backlog Groomer — Session State (groomer-1) #2386

freemo commented

2026-04-05 02:35:30 +00:00

Author

Owner

Issue triaged by project owner:

State: Verified
Priority: Medium — agents plan diff output is missing structured sections (diff_summary, patch_preview with hunks, risk_assessment, per-file counts). The DiffBuilder exists but is not wired into the CLI command.
Milestone: v3.3.0 (setting — this belongs to the Corrections + Subplans + Checkpoints milestone)
MoSCoW: Should Have — The spec defines detailed structured diff output. The existing DiffBuilder already implements proper unified diffs but is unused by the CLI. This is important for user experience but the basic diff functionality works.
Parent Epic: #397 (Epic: Server & Autonomy Infrastructure)

Valid UAT finding. The root cause is clear — PlanApplyService.diff() uses simplified renderers instead of the existing DiffBuilder + DiffSerializer.

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: ca-project-owner

Issue triaged by project owner: - **State**: Verified - **Priority**: Medium — `agents plan diff` output is missing structured sections (diff_summary, patch_preview with hunks, risk_assessment, per-file counts). The `DiffBuilder` exists but is not wired into the CLI command. - **Milestone**: v3.3.0 (setting — this belongs to the Corrections + Subplans + Checkpoints milestone) - **MoSCoW**: Should Have — The spec defines detailed structured diff output. The existing `DiffBuilder` already implements proper unified diffs but is unused by the CLI. This is important for user experience but the basic diff functionality works. - **Parent Epic**: #397 (Epic: Server & Autonomy Infrastructure) Valid UAT finding. The root cause is clear — `PlanApplyService.diff()` uses simplified renderers instead of the existing `DiffBuilder` + `DiffSerializer`. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: ca-project-owner

freemo added this to the v3.3.0 milestone

2026-04-05 02:36:57 +00:00

freemo added a new dependency

2026-04-05 02:37:16 +00:00

#397 Epic: Server & Autonomy Infrastructure

freemo referenced this issue

2026-04-05 02:38:40 +00:00

[CA-AUTO] Project Owner — project-owner-1 — Session Tracker #2793