feat(change): add diff review artifacts

benchmarks/diff_review_bench.py

@@ -0,0 +1,198 @@
+"""ASV benchmarks for diff review artifact building and serialization.
+
+Measures the performance of:
+- DiffBuilder.build() with various changeset sizes
+- DiffSerializer.to_plain() / to_json() / to_rich()
+- Rename detection with content hash matching
+- ReviewArtifact and DiffEntry construction
+"""
+
+from __future__ import annotations
+
+import sys
+from pathlib import Path
+
+try:
+    from cleveragents.domain.models.core.change import (
+        ChangeEntry,
+        ChangeOperation,
+        SpecChangeSet,
+    )
+    from cleveragents.domain.models.core.diff_review import (
+        DiffBuilder,
+        DiffEntry,
+        DiffSerializer,
+        ResourceDiffGroup,
+        ReviewArtifact,
+    )
+except ModuleNotFoundError:
+    sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "src"))
+    from cleveragents.domain.models.core.change import (
+        ChangeEntry,
+        ChangeOperation,
+        SpecChangeSet,
+    )
+    from cleveragents.domain.models.core.diff_review import (
+        DiffBuilder,
+        DiffEntry,
+        DiffSerializer,
+        ResourceDiffGroup,
+        ReviewArtifact,
+    )
+
+
+def _make_changeset(
+    n_entries: int = 10,
+) -> tuple[SpecChangeSet, dict[str, str], dict[str, str]]:
+    """Create a changeset with n_entries MODIFY entries and content maps."""
+    cs = SpecChangeSet(plan_id="BENCH-PLAN")
+    before_map: dict[str, str] = {}
+    after_map: dict[str, str] = {}
+    for i in range(n_entries):
+        path = f"src/module_{i:04d}.py"
+        cs.add_change(
+            ChangeEntry(
+                plan_id="BENCH-PLAN",
+                resource_id=f"RES{i % 5:03d}",
+                tool_name="builtin/bench-tool",
+                operation=ChangeOperation.MODIFY,
+                path=path,
+                before_hash=f"before-{i}",
+                after_hash=f"after-{i}",
+            )
+        )
+        before_map[path] = f"# module {i}\nx = {i}\n"
+        after_map[path] = f"# module {i}\nx = {i + 1}\ny = {i}\n"
+    return cs, before_map, after_map
+
+
+class DiffBuildSuite:
+    """Benchmark DiffBuilder.build() with various changeset sizes."""
+
+    def setup(self) -> None:
+        """Prepare changesets of different sizes."""
+        self.builder = DiffBuilder()
+        self.cs_small, self.bm_small, self.am_small = _make_changeset(5)
+        self.cs_medium, self.bm_medium, self.am_medium = _make_changeset(50)
+        self.cs_large, self.bm_large, self.am_large = _make_changeset(200)
+
+    def time_build_small(self) -> None:
+        """Benchmark building artifact from 5-entry changeset."""
+        self.builder.build(
+            self.cs_small,
+            before_contents=self.bm_small,
+            after_contents=self.am_small,
+        )
+
+    def time_build_medium(self) -> None:
+        """Benchmark building artifact from 50-entry changeset."""
+        self.builder.build(
+            self.cs_medium,
+            before_contents=self.bm_medium,
+            after_contents=self.am_medium,
+        )
+
+    def time_build_large(self) -> None:
+        """Benchmark building artifact from 200-entry changeset."""
+        self.builder.build(
+            self.cs_large,
+            before_contents=self.bm_large,
+            after_contents=self.am_large,
+        )
+
+
+class DiffSerializationSuite:
+    """Benchmark DiffSerializer output formats."""
+
+    def setup(self) -> None:
+        """Build an artifact to serialize."""
+        cs, bm, am = _make_changeset(20)
+        builder = DiffBuilder()
+        self.artifact = builder.build(cs, before_contents=bm, after_contents=am)
+
+    def time_to_plain(self) -> None:
+        """Benchmark plain text serialization."""
+        DiffSerializer.to_plain(self.artifact)
+
+    def time_to_json(self) -> None:
+        """Benchmark JSON serialization."""
+        DiffSerializer.to_json(self.artifact)
+
+    def time_to_rich(self) -> None:
+        """Benchmark rich text serialization."""
+        DiffSerializer.to_rich(self.artifact)
+
+
+class DiffRenameSuite:
+    """Benchmark rename detection with content hash matching."""
+
+    def setup(self) -> None:
+        """Prepare a changeset with rename pairs."""
+        self.builder = DiffBuilder()
+        self.cs = SpecChangeSet(plan_id="BENCH-PLAN")
+        for i in range(20):
+            self.cs.add_change(
+                ChangeEntry(
+                    plan_id="BENCH-PLAN",
+                    resource_id="RES001",
+                    tool_name="builtin/bench-tool",
+                    operation=ChangeOperation.DELETE,
+                    path=f"old/module_{i:04d}.py",
+                    before_hash=f"hash-{i}",
+                )
+            )
+            self.cs.add_change(
+                ChangeEntry(
+                    plan_id="BENCH-PLAN",
+                    resource_id="RES001",
+                    tool_name="builtin/bench-tool",
+                    operation=ChangeOperation.CREATE,
+                    path=f"new/module_{i:04d}.py",
+                    after_hash=f"hash-{i}",
+                )
+            )
+
+    def time_rename_detection(self) -> None:
+        """Benchmark rename detection for 20 pairs."""
+        self.builder.build(self.cs)
+
+
+class DiffModelSuite:
+    """Benchmark DiffEntry and ReviewArtifact construction."""
+
+    def time_diff_entry_construction(self) -> None:
+        """Benchmark creating a DiffEntry."""
+        DiffEntry(
+            resource_id="RES001",
+            resource_name="test-resource",
+            path="src/main.py",
+            change_type="modify",
+            diff_text="--- a/src/main.py\n+++ b/src/main.py\n",
+        )
+
+    def time_resource_diff_group_construction(self) -> None:
+        """Benchmark creating a ResourceDiffGroup."""
+        ResourceDiffGroup(
+            resource_id="RES001",
+            resource_name="test-resource",
+            entries=[
+                DiffEntry(
+                    resource_id="RES001",
+                    path="a.py",
+                    change_type="create",
+                ),
+                DiffEntry(
+                    resource_id="RES001",
+                    path="b.py",
+                    change_type="modify",
+                ),
+            ],
+        )
+
+    def time_review_artifact_construction(self) -> None:
+        """Benchmark creating a ReviewArtifact."""
+        ReviewArtifact(
+            plan_id="PLAN001",
+            changeset_id="CS001",
+            total_changes=5,
+        )

docs/reference/diff_review.md

@@ -0,0 +1,87 @@
+# Diff Review
+
+The diff review module builds reviewable diff artifacts from `SpecChangeSet`
+records, grouping changes by resource and producing unified diff text for
+CLI display.
+
+## Overview
+
+When a plan accumulates changes through tool executions, the diff review
+system transforms the raw `ChangeEntry` records into a structured
+`ReviewArtifact` suitable for human review before applying changes.
+
+## Domain Models
+
+### DiffEntry
+
+A single diff entry for CLI review. Contains:
+
+| Field | Type | Description |
+|---|---|---|
+| `resource_id` | `str` | Resource ULID |
+| `resource_name` | `str` | Human-readable resource name |
+| `path` | `str` | File path |
+| `change_type` | `str` | Operation type (create/modify/delete/rename) |
+| `before_content` | `str \| None` | Content before the change |
+| `after_content` | `str \| None` | Content after the change |
+| `before_hash` | `str \| None` | SHA-256 before |
+| `after_hash` | `str \| None` | SHA-256 after |
+| `file_mode` | `int \| None` | File permission mode |
+| `is_binary` | `bool` | Whether the file is binary |
+| `is_rename` | `bool` | Whether this is a rename/move |
+| `rename_from` | `str \| None` | Original path for renames |
+| `rename_to` | `str \| None` | Destination path for renames |
+| `diff_text` | `str` | Unified diff text |
+| `truncated` | `bool` | Whether the diff text was truncated |
+
+### ResourceDiffGroup
+
+Groups diff entries for a single resource. Provides summary counts
+via properties: `added`, `modified`, `deleted`, `renamed`.
+
+### ReviewArtifact
+
+Complete diff review artifact for a plan. Contains grouped diff entries,
+total change count, and truncation status.
+
+## DiffBuilder
+
+`DiffBuilder` takes a `SpecChangeSet` and optional content maps to produce
+a `ReviewArtifact`.
+
+```python
+from cleveragents.domain.models.core.diff_review import DiffBuilder
+
+builder = DiffBuilder(max_diff_size=50_000, context_lines=3)
+artifact = builder.build(
+    changeset,
+    before_contents={"src/main.py": "old content"},
+    after_contents={"src/main.py": "new content"},
+    resource_names={"RES001": "my-resource"},
+)
+```
+
+### Features
+
+- **Unified diff generation** using `difflib.unified_diff`
+- **Binary detection** via null byte checking
+- **Rename detection** using `SequenceMatcher` path similarity (>0.6)
+  combined with matching content hashes
+- **Truncation** when total diff text exceeds `max_diff_size` (applies
+  uniformly to all entry types including renames)
+- **Stable ordering** by resource ID, then path within each group
+
+## DiffSerializer
+
+Renders a `ReviewArtifact` to various output formats:
+
+- `to_plain(artifact)` — plain text for terminal output
+- `to_json(artifact)` — structured JSON for programmatic use
+- `to_rich(artifact)` — rich markup with `[bold]`, `[green]`, `[red]` tags
+
+## Constants
+
+| Name | Default | Description |
+|---|---|---|
+| `DEFAULT_MAX_DIFF_SIZE` | 50,000 | Maximum total diff text characters |
+| `RENAME_SIMILARITY_THRESHOLD` | 0.6 | Minimum path similarity for rename detection |

features/diff_review.feature

@@ -0,0 +1,263 @@
+Feature: Diff review artifacts
+  As a developer reviewing plan changes
+  I want to build diff review artifacts from changesets
+  So that I can inspect changes before applying them
+
+  Background:
+    Given a plan ID "01PLAN000000000000000001"
+    And an empty changeset for the plan
+
+  Scenario: Empty changeset produces empty review artifact
+    When I build a review artifact from the changeset
+    Then the review artifact should have 0 total changes
+    And the review artifact should have 0 groups
+    And the review artifact should not be truncated
+
+  Scenario: Single CREATE change produces a diff entry
+    Given a change entry with operation "create" for path "src/main.py" in resource "RES001"
+    And after content for "src/main.py" is "print('hello')\n"
+    When I build a review artifact from the changeset
+    Then the review artifact should have 1 total changes
+    And the review artifact should have 1 groups
+    And group 0 should have 1 entries
+    And entry 0 in group 0 should have change type "create"
+    And entry 0 in group 0 should have path "src/main.py"
+
+  Scenario: MODIFY change generates unified diff
+    Given a change entry with operation "modify" for path "src/app.py" in resource "RES001"
+    And before content for "src/app.py" is "x = 1\ny = 2\n"
+    And after content for "src/app.py" is "x = 1\ny = 3\n"
+    When I build a review artifact from the changeset
+    Then entry 0 in group 0 should have change type "modify"
+    And the diff text in entry 0 of group 0 should contain "---"
+    And the diff text in entry 0 of group 0 should contain "+++"
+
+  Scenario: DELETE change entry
+    Given a change entry with operation "delete" for path "old.txt" in resource "RES001"
+    And before content for "old.txt" is "old content\n"
+    When I build a review artifact from the changeset
+    Then entry 0 in group 0 should have change type "delete"
+    And the diff text in entry 0 of group 0 should contain "---"
+
+  Scenario: Multiple changes across different resources
+    Given a change entry with operation "create" for path "a.py" in resource "RES001"
+    And a change entry with operation "modify" for path "b.py" in resource "RES002"
+    And after content for "a.py" is "new\n"
+    And before content for "b.py" is "old\n"
+    And after content for "b.py" is "new\n"
+    When I build a review artifact from the changeset
+    Then the review artifact should have 2 total changes
+    And the review artifact should have 2 groups
+
+  Scenario: Changes grouped by resource
+    Given a change entry with operation "create" for path "a.py" in resource "RES001"
+    And a change entry with operation "modify" for path "b.py" in resource "RES001"
+    And after content for "a.py" is "new\n"
+    And before content for "b.py" is "old\n"
+    And after content for "b.py" is "new\n"
+    When I build a review artifact from the changeset
+    Then the review artifact should have 1 groups
+    And group 0 should have 2 entries
+
+  Scenario: Binary file detection
+    Given a change entry with operation "create" for path "image.bin" in resource "RES001"
+    And after content for "image.bin" is "binary\x00data"
+    When I build a review artifact from the changeset
+    Then entry 0 in group 0 should be binary
+    And the diff text in entry 0 of group 0 should contain "[binary file]"
+
+  Scenario: Rename detection with matching content hash
+    Given a delete entry for path "old/file.py" in resource "RES001" with before_hash "abc123"
+    And a create entry for path "new/file.py" in resource "RES001" with after_hash "abc123"
+    When I build a review artifact from the changeset
+    Then the review artifact should have 1 groups
+    And group 0 should have 1 entries
+    And entry 0 in group 0 should be a rename
+    And entry 0 in group 0 rename should be from "old/file.py" to "new/file.py"
+    And group 0 should have 1 renamed
+
+  Scenario: Rename not detected when hashes differ
+    Given a delete entry for path "old/file.py" in resource "RES001" with before_hash "abc123"
+    And a create entry for path "new/file.py" in resource "RES001" with after_hash "xyz789"
+    When I build a review artifact from the changeset
+    Then group 0 should have 2 entries
+    And entry 0 in group 0 should not be a rename
+
+  Scenario: Max diff size truncation
+    Given a change entry with operation "create" for path "big.py" in resource "RES001"
+    And after content for "big.py" is a string of 100000 characters
+    When I build a review artifact with max diff size 100
+    Then the review artifact should be truncated
+    And entry 0 in group 0 should be truncated
+
+  Scenario: Stable ordering by resource then path
+    Given a change entry with operation "create" for path "z.py" in resource "RES002"
+    And a change entry with operation "create" for path "a.py" in resource "RES001"
+    And after content for "z.py" is "z\n"
+    And after content for "a.py" is "a\n"
+    When I build a review artifact from the changeset
+    Then group 0 should have resource id "RES001"
+    And group 1 should have resource id "RES002"
+
+  Scenario: Before and after content hashes in metadata
+    Given a hashed change entry "modify" path "f.py" resource "RES001" before_hash "hash1" after_hash "hash2"
+    And before content for "f.py" is "old\n"
+    And after content for "f.py" is "new\n"
+    When I build a review artifact from the changeset
+    Then entry 0 in group 0 should have before hash "hash1"
+    And entry 0 in group 0 should have after hash "hash2"
+
+  Scenario: File mode in metadata
+    Given a moded change entry "modify" path "script.sh" resource "RES001" after_mode 0o755
+    And before content for "script.sh" is "#!/bin/sh\n"
+    And after content for "script.sh" is "#!/bin/bash\n"
+    When I build a review artifact from the changeset
+    Then entry 0 in group 0 should have file mode 493
+
+  Scenario: Serialize to plain text
+    Given a change entry with operation "create" for path "new.py" in resource "RES001"
+    And after content for "new.py" is "hello\n"
+    And resource name "RES001" is "my-resource"
+    When I build a review artifact from the changeset
+    And I serialize the artifact to plain text
+    Then the plain text should contain "my-resource"
+    And the plain text should contain "new.py"
+    And the plain text should contain "Total changes: 1"
+
+  Scenario: Serialize to JSON
+    Given a change entry with operation "create" for path "new.py" in resource "RES001"
+    And after content for "new.py" is "hello\n"
+    When I build a review artifact from the changeset
+    And I serialize the artifact to JSON
+    Then the JSON output should be valid JSON
+    And the JSON output should contain key "total_changes"
+    And the JSON output should contain key "groups"
+
+  Scenario: Serialize to rich format
+    Given a change entry with operation "create" for path "new.py" in resource "RES001"
+    And after content for "new.py" is "hello\n"
+    When I build a review artifact from the changeset
+    And I serialize the artifact to rich format
+    Then the rich text should contain "[bold]"
+    And the rich text should contain "new.py"
+
+  Scenario: ResourceDiffGroup summary counts
+    Given a change entry with operation "create" for path "a.py" in resource "RES001"
+    And a change entry with operation "modify" for path "b.py" in resource "RES001"
+    And a change entry with operation "delete" for path "c.py" in resource "RES001"
+    And after content for "a.py" is "new\n"
+    And before content for "b.py" is "old\n"
+    And after content for "b.py" is "new\n"
+    And before content for "c.py" is "old\n"
+    When I build a review artifact from the changeset
+    Then group 0 should have 1 added
+    And group 0 should have 1 modified
+    And group 0 should have 1 deleted
+
+  Scenario: ReviewArtifact has plan and changeset IDs
+    When I build a review artifact from the changeset
+    Then the review artifact plan ID should match the changeset plan ID
+    And the review artifact changeset ID should match the changeset ID
+
+  Scenario: Empty content produces empty diff
+    Given a change entry with operation "modify" for path "empty.py" in resource "RES001"
+    When I build a review artifact from the changeset
+    Then the diff text in entry 0 of group 0 should be empty
+
+  Scenario: Multiple entries in same resource sorted by path
+    Given a change entry with operation "create" for path "z.py" in resource "RES001"
+    And a change entry with operation "create" for path "a.py" in resource "RES001"
+    And after content for "z.py" is "z\n"
+    And after content for "a.py" is "a\n"
+    When I build a review artifact from the changeset
+    Then entry 0 in group 0 should have path "a.py"
+    And entry 1 in group 0 should have path "z.py"
+
+  Scenario: Second entry fully truncated when budget exhausted
+    Given a change entry with operation "create" for path "a.py" in resource "RES001"
+    And a change entry with operation "create" for path "b.py" in resource "RES001"
+    And after content for "a.py" is a string of 1000 characters
+    And after content for "b.py" is a string of 1000 characters
+    When I build a review artifact with max diff size 50
+    Then the review artifact should be truncated
+    And entry 1 in group 0 should be truncated
+    And the diff text in entry 1 of group 0 should contain "[truncated]"
+
+  Scenario: JSON serialization includes hashes and file mode
+    Given a hashed change entry "modify" path "f.py" resource "RES001" before_hash "aaa" after_hash "bbb"
+    And a moded change entry "modify" path "g.py" resource "RES001" after_mode 0o644
+    And before content for "f.py" is "old\n"
+    And after content for "f.py" is "new\n"
+    And before content for "g.py" is "a\n"
+    And after content for "g.py" is "b\n"
+    When I build a review artifact from the changeset
+    And I serialize the artifact to JSON
+    Then the JSON output should be valid JSON
+    And the JSON output should have entry with before hash "aaa"
+    And the JSON output should have entry with after hash "bbb"
+    And the JSON output should have entry with file mode
+
+  Scenario: JSON serialization includes rename fields
+    Given a delete entry for path "old/r.py" in resource "RES001" with before_hash "h1"
+    And a create entry for path "new/r.py" in resource "RES001" with after_hash "h1"
+    When I build a review artifact from the changeset
+    And I serialize the artifact to JSON
+    Then the JSON output should be valid JSON
+    And the JSON output should have entry with rename from "old/r.py"
+
+  Scenario: Rich format shows truncation warning
+    Given a change entry with operation "create" for path "big.py" in resource "RES001"
+    And after content for "big.py" is a string of 100000 characters
+    When I build a review artifact with max diff size 100
+    And I serialize the artifact to rich format
+    Then the rich text should contain "[yellow]"
+    And the rich text should contain "truncated"
+
+  Scenario: Rich format shows binary marker
+    Given a change entry with operation "create" for path "bin.dat" in resource "RES001"
+    And after content for "bin.dat" is "binary\x00data"
+    When I build a review artifact from the changeset
+    And I serialize the artifact to rich format
+    Then the rich text should contain "[binary]"
+
+  Scenario: Rich format colorizes diff lines
+    Given a change entry with operation "modify" for path "c.py" in resource "RES001"
+    And before content for "c.py" is "old\n"
+    And after content for "c.py" is "new\n"
+    When I build a review artifact from the changeset
+    And I serialize the artifact to rich format
+    Then the rich text should contain "[green]"
+    And the rich text should contain "[red]"
+
+  Scenario: Cross-resource delete and create are not renamed
+    Given a delete entry for path "old/x.py" in resource "RES001" with before_hash "same"
+    And a create entry for path "new/x.py" in resource "RES002" with after_hash "same"
+    When I build a review artifact from the changeset
+    Then the review artifact should have 2 groups
+    And entry 0 in group 0 should not be a rename
+    And entry 0 in group 1 should not be a rename
+
+  Scenario: Multiple renames consume creates only once
+    Given a delete entry for path "old/a.py" in resource "RES001" with before_hash "h1"
+    And a delete entry for path "old/b.py" in resource "RES001" with before_hash "h1"
+    And a create entry for path "new/a.py" in resource "RES001" with after_hash "h1"
+    When I build a review artifact from the changeset
+    Then group 0 should have 2 entries
+
+  Scenario: Rename entry truncated when budget exhausted
+    Given a change entry with operation "create" for path "a.py" in resource "RES001"
+    And after content for "a.py" is a string of 1000 characters
+    And a delete entry for path "old/r.py" in resource "RES001" with before_hash "rh1"
+    And a create entry for path "new/r.py" in resource "RES001" with after_hash "rh1"
+    When I build a review artifact with max diff size 50
+    Then the review artifact should be truncated
+    And entry 1 in group 0 should be a rename
+    And entry 1 in group 0 should be truncated
+    And the diff text in entry 1 of group 0 should contain "[truncated]"
+
+  Scenario: Plain serialization shows binary marker
+    Given a change entry with operation "create" for path "img.png" in resource "RES001"
+    And after content for "img.png" is "png\x00header"
+    When I build a review artifact from the changeset
+    And I serialize the artifact to plain text
+    Then the plain text should contain "[binary file]"

features/steps/diff_review_steps.py

@@ -0,0 +1,456 @@
+"""Step definitions for diff review artifact BDD tests."""
+
+from __future__ import annotations
+
+import json
+from typing import Any
+
+from behave import given, then, when
+from behave.runner import Context
+
+from cleveragents.domain.models.core.change import (
+    ChangeEntry,
+    ChangeOperation,
+    SpecChangeSet,
+)
+from cleveragents.domain.models.core.diff_review import (
+    DiffBuilder,
+    DiffSerializer,
+    ReviewArtifact,
+)
+
+__all__: list[str] = []
+
+
+# ---------------------------------------------------------------------------
+# Helpers
+# ---------------------------------------------------------------------------
+
+
+def _make_entry(
+    operation: str,
+    path: str,
+    resource_id: str,
+    plan_id: str,
+    *,
+    before_hash: str | None = None,
+    after_hash: str | None = None,
+    after_mode: int | None = None,
+) -> ChangeEntry:
+    """Build a ChangeEntry with sensible defaults."""
+    kwargs: dict[str, Any] = {
+        "plan_id": plan_id,
+        "resource_id": resource_id,
+        "tool_name": "builtin/test-tool",
+        "operation": ChangeOperation(operation),
+        "path": path,
+    }
+    if before_hash is not None:
+        kwargs["before_hash"] = before_hash
+    if after_hash is not None:
+        kwargs["after_hash"] = after_hash
+    if after_mode is not None:
+        kwargs["after_mode"] = after_mode
+    return ChangeEntry(**kwargs)
+
+
+# ---------------------------------------------------------------------------
+# Background
+# ---------------------------------------------------------------------------
+
+
+@given('a plan ID "{plan_id}"')
+def step_given_plan_id(context: Context, plan_id: str) -> None:
+    context.plan_id = plan_id
+    context.before_contents = {}  # dict[str, str]
+    context.after_contents = {}  # dict[str, str]
+    context.resource_names = {}  # dict[str, str]
+
+
+@given("an empty changeset for the plan")
+def step_given_empty_changeset(context: Context) -> None:
+    context.changeset = SpecChangeSet(plan_id=context.plan_id)
+
+
+# ---------------------------------------------------------------------------
+# Change entry creation
+# ---------------------------------------------------------------------------
+
+
+@given('a change entry with operation "{op}" for path "{path}" in resource "{res}"')
+def step_add_change_entry(context: Context, op: str, path: str, res: str) -> None:
+    entry = _make_entry(op, path, res, context.plan_id)
+    context.changeset.add_change(entry)
+
+
+@given(
+    'a delete entry for path "{path}" in resource "{res}" with before_hash "{bhash}"'
+)
+def step_add_delete_with_hash(
+    context: Context, path: str, res: str, bhash: str
+) -> None:
+    entry = _make_entry("delete", path, res, context.plan_id, before_hash=bhash)
+    context.changeset.add_change(entry)
+
+
+@given('a create entry for path "{path}" in resource "{res}" with after_hash "{ahash}"')
+def step_add_create_with_hash(
+    context: Context, path: str, res: str, ahash: str
+) -> None:
+    entry = _make_entry("create", path, res, context.plan_id, after_hash=ahash)
+    context.changeset.add_change(entry)
+
+
+@given(
+    'a hashed change entry "{op}" path "{path}" '
+    'resource "{res}" before_hash "{bhash}" after_hash "{ahash}"'
+)
+def step_add_entry_with_hashes(
+    context: Context,
+    op: str,
+    path: str,
+    res: str,
+    bhash: str,
+    ahash: str,
+) -> None:
+    entry = _make_entry(
+        op, path, res, context.plan_id, before_hash=bhash, after_hash=ahash
+    )
+    context.changeset.add_change(entry)
+
+
+@given('a moded change entry "{op}" path "{path}" resource "{res}" after_mode {mode}')
+def step_add_entry_with_mode(
+    context: Context, op: str, path: str, res: str, mode: str
+) -> None:
+    # mode comes as e.g. "0o755" — parse it
+    mode_int = int(mode, 0)
+    entry = _make_entry(op, path, res, context.plan_id, after_mode=mode_int)
+    context.changeset.add_change(entry)
+
+
+# ---------------------------------------------------------------------------
+# Content setup
+# ---------------------------------------------------------------------------
+
+
+@given('before content for "{path}" is "{content}"')
+def step_before_content(context: Context, path: str, content: str) -> None:
+    # Interpret escape sequences
+    context.before_contents[path] = content.replace("\\n", "\n").replace(
+        "\\x00", "\x00"
+    )
+
+
+@given('after content for "{path}" is "{content}"')
+def step_after_content(context: Context, path: str, content: str) -> None:
+    context.after_contents[path] = content.replace("\\n", "\n").replace("\\x00", "\x00")
+
+
+@given('after content for "{path}" is a string of {count} characters')
+def step_after_content_large(context: Context, path: str, count: str) -> None:
+    context.after_contents[path] = "x" * int(count)
+
+
+@given('resource name "{res_id}" is "{name}"')
+def step_resource_name(context: Context, res_id: str, name: str) -> None:
+    context.resource_names[res_id] = name
+
+
+# ---------------------------------------------------------------------------
+# Build actions
+# ---------------------------------------------------------------------------
+
+
+@when("I build a review artifact from the changeset")
+def step_build_artifact(context: Context) -> None:
+    builder = DiffBuilder()
+    context.artifact = builder.build(
+        context.changeset,
+        before_contents=context.before_contents,
+        after_contents=context.after_contents,
+        resource_names=context.resource_names,
+    )
+
+
+@when("I build a review artifact with max diff size {size}")
+def step_build_artifact_limited(context: Context, size: str) -> None:
+    builder = DiffBuilder(max_diff_size=int(size))
+    context.artifact = builder.build(
+        context.changeset,
+        before_contents=context.before_contents,
+        after_contents=context.after_contents,
+        resource_names=context.resource_names,
+    )
+
+
+# ---------------------------------------------------------------------------
+# Serialization actions
+# ---------------------------------------------------------------------------
+
+
+@when("I serialize the artifact to plain text")
+def step_serialize_plain(context: Context) -> None:
+    context.plain_text = DiffSerializer.to_plain(context.artifact)
+
+
+@when("I serialize the artifact to JSON")
+def step_serialize_json(context: Context) -> None:
+    context.json_text = DiffSerializer.to_json(context.artifact)
+
+
+@when("I serialize the artifact to rich format")
+def step_serialize_rich(context: Context) -> None:
+    context.rich_text = DiffSerializer.to_rich(context.artifact)
+
+
+# ---------------------------------------------------------------------------
+# Review artifact assertions
+# ---------------------------------------------------------------------------
+
+
+@then("the review artifact should have {n} total changes")
+def step_assert_total_changes(context: Context, n: str) -> None:
+    artifact: ReviewArtifact = context.artifact
+    assert artifact.total_changes == int(n), (
+        f"Expected {n} total changes, got {artifact.total_changes}"
+    )
+
+
+@then("the review artifact should have {n} groups")
+def step_assert_groups(context: Context, n: str) -> None:
+    artifact: ReviewArtifact = context.artifact
+    assert len(artifact.groups) == int(n), (
+        f"Expected {n} groups, got {len(artifact.groups)}"
+    )
+
+
+@then("the review artifact should not be truncated")
+def step_assert_not_truncated(context: Context) -> None:
+    assert not context.artifact.truncated, "Artifact should not be truncated"
+
+
+@then("the review artifact should be truncated")
+def step_assert_truncated(context: Context) -> None:
+    assert context.artifact.truncated, "Artifact should be truncated"
+
+
+@then("the review artifact plan ID should match the changeset plan ID")
+def step_plan_id_matches(context: Context) -> None:
+    assert context.artifact.plan_id == context.changeset.plan_id
+
+
+@then("the review artifact changeset ID should match the changeset ID")
+def step_changeset_id_matches(context: Context) -> None:
+    assert context.artifact.changeset_id == context.changeset.changeset_id
+
+
+# ---------------------------------------------------------------------------
+# Group assertions
+# ---------------------------------------------------------------------------
+
+
+@then("group {g} should have {n} entries")
+def step_group_entries(context: Context, g: str, n: str) -> None:
+    group = context.artifact.groups[int(g)]
+    assert len(group.entries) == int(n), (
+        f"Expected {n} entries in group {g}, got {len(group.entries)}"
+    )
+
+
+@then('group {g} should have resource id "{rid}"')
+def step_group_resource_id(context: Context, g: str, rid: str) -> None:
+    group = context.artifact.groups[int(g)]
+    assert group.resource_id == rid, (
+        f"Expected resource id '{rid}', got '{group.resource_id}'"
+    )
+
+
+@then("group {g} should have {n} added")
+def step_group_added(context: Context, g: str, n: str) -> None:
+    group = context.artifact.groups[int(g)]
+    assert group.added == int(n), f"Expected {n} added, got {group.added}"
+
+
+@then("group {g} should have {n} modified")
+def step_group_modified(context: Context, g: str, n: str) -> None:
+    group = context.artifact.groups[int(g)]
+    assert group.modified == int(n), f"Expected {n} modified, got {group.modified}"
+
+
+@then("group {g} should have {n} deleted")
+def step_group_deleted(context: Context, g: str, n: str) -> None:
+    group = context.artifact.groups[int(g)]
+    assert group.deleted == int(n), f"Expected {n} deleted, got {group.deleted}"
+
+
+@then("group {g} should have {n} renamed")
+def step_group_renamed(context: Context, g: str, n: str) -> None:
+    group = context.artifact.groups[int(g)]
+    assert group.renamed == int(n), f"Expected {n} renamed, got {group.renamed}"
+
+
+# ---------------------------------------------------------------------------
+# Entry assertions
+# ---------------------------------------------------------------------------
+
+
+@then('entry {e} in group {g} should have change type "{ct}"')
+def step_entry_change_type(context: Context, e: str, g: str, ct: str) -> None:
+    entry = context.artifact.groups[int(g)].entries[int(e)]
+    assert entry.change_type == ct, (
+        f"Expected change_type '{ct}', got '{entry.change_type}'"
+    )
+
+
+@then('entry {e} in group {g} should have path "{path}"')
+def step_entry_path(context: Context, e: str, g: str, path: str) -> None:
+    entry = context.artifact.groups[int(g)].entries[int(e)]
+    assert entry.path == path, f"Expected path '{path}', got '{entry.path}'"
+
+
+@then("entry {e} in group {g} should be binary")
+def step_entry_binary(context: Context, e: str, g: str) -> None:
+    entry = context.artifact.groups[int(g)].entries[int(e)]
+    assert entry.is_binary, "Entry should be binary"
+
+
+@then("entry {e} in group {g} should be a rename")
+def step_entry_is_rename(context: Context, e: str, g: str) -> None:
+    entry = context.artifact.groups[int(g)].entries[int(e)]
+    assert entry.is_rename, "Entry should be a rename"
+
+
+@then("entry {e} in group {g} should not be a rename")
+def step_entry_not_rename(context: Context, e: str, g: str) -> None:
+    entry = context.artifact.groups[int(g)].entries[int(e)]
+    assert not entry.is_rename, "Entry should not be a rename"
+
+
+@then('entry {e} in group {g} rename should be from "{src}" to "{dst}"')
+def step_entry_rename_paths(
+    context: Context, e: str, g: str, src: str, dst: str
+) -> None:
+    entry = context.artifact.groups[int(g)].entries[int(e)]
+    assert entry.rename_from == src, (
+        f"Expected rename_from '{src}', got '{entry.rename_from}'"
+    )
+    assert entry.rename_to == dst, (
+        f"Expected rename_to '{dst}', got '{entry.rename_to}'"
+    )
+
+
+@then("entry {e} in group {g} should be truncated")
+def step_entry_truncated(context: Context, e: str, g: str) -> None:
+    entry = context.artifact.groups[int(g)].entries[int(e)]
+    assert entry.truncated, "Entry should be truncated"
+
+
+@then('entry {e} in group {g} should have before hash "{h}"')
+def step_entry_before_hash(context: Context, e: str, g: str, h: str) -> None:
+    entry = context.artifact.groups[int(g)].entries[int(e)]
+    assert entry.before_hash == h, (
+        f"Expected before_hash '{h}', got '{entry.before_hash}'"
+    )
+
+
+@then('entry {e} in group {g} should have after hash "{h}"')
+def step_entry_after_hash(context: Context, e: str, g: str, h: str) -> None:
+    entry = context.artifact.groups[int(g)].entries[int(e)]
+    assert entry.after_hash == h, f"Expected after_hash '{h}', got '{entry.after_hash}'"
+
+
+@then("entry {e} in group {g} should have file mode {mode}")
+def step_entry_file_mode(context: Context, e: str, g: str, mode: str) -> None:
+    entry = context.artifact.groups[int(g)].entries[int(e)]
+    assert entry.file_mode == int(mode), (
+        f"Expected file_mode {mode}, got {entry.file_mode}"
+    )
+
+
+# ---------------------------------------------------------------------------
+# Diff text assertions
+# ---------------------------------------------------------------------------
+
+
+@then('the diff text in entry {e} of group {g} should contain "{text}"')
+def step_diff_contains(context: Context, e: str, g: str, text: str) -> None:
+    entry = context.artifact.groups[int(g)].entries[int(e)]
+    assert text in entry.diff_text, (
+        f"Expected diff_text to contain '{text}', got: {entry.diff_text!r}"
+    )
+
+
+@then("the diff text in entry {e} of group {g} should be empty")
+def step_diff_empty(context: Context, e: str, g: str) -> None:
+    entry = context.artifact.groups[int(g)].entries[int(e)]
+    assert entry.diff_text == "", f"Expected empty diff_text, got: {entry.diff_text!r}"
+
+
+# ---------------------------------------------------------------------------
+# Serialization assertions
+# ---------------------------------------------------------------------------
+
+
+@then('the plain text should contain "{text}"')
+def step_plain_contains(context: Context, text: str) -> None:
+    assert text in context.plain_text, f"Expected plain text to contain '{text}'"
+
+
+@then("the JSON output should be valid JSON")
+def step_json_valid(context: Context) -> None:
+    try:
+        json.loads(context.json_text)
+    except json.JSONDecodeError as exc:
+        raise AssertionError(f"Invalid JSON: {exc}") from exc
+
+
+@then('the JSON output should contain key "{key}"')
+def step_json_has_key(context: Context, key: str) -> None:
+    data = json.loads(context.json_text)
+    assert key in data, f"Expected key '{key}' in JSON output"
+
+
+@then('the rich text should contain "{text}"')
+def step_rich_contains(context: Context, text: str) -> None:
+    assert text in context.rich_text, f"Expected rich text to contain '{text}'"
+
+
+@then('the JSON output should have entry with before hash "{h}"')
+def step_json_entry_before_hash(context: Context, h: str) -> None:
+    data = json.loads(context.json_text)
+    for group in data["groups"]:
+        for entry in group["entries"]:
+            if entry.get("before_hash") == h:
+                return
+    raise AssertionError(f"No entry with before_hash '{h}' in JSON output")
+
+
+@then('the JSON output should have entry with after hash "{h}"')
+def step_json_entry_after_hash(context: Context, h: str) -> None:
+    data = json.loads(context.json_text)
+    for group in data["groups"]:
+        for entry in group["entries"]:
+            if entry.get("after_hash") == h:
+                return
+    raise AssertionError(f"No entry with after_hash '{h}' in JSON output")
+
+
+@then("the JSON output should have entry with file mode")
+def step_json_entry_file_mode(context: Context) -> None:
+    data = json.loads(context.json_text)
+    for group in data["groups"]:
+        for entry in group["entries"]:
+            if "file_mode" in entry:
+                return
+    raise AssertionError("No entry with file_mode in JSON output")
+
+
+@then('the JSON output should have entry with rename from "{path}"')
+def step_json_entry_rename_from(context: Context, path: str) -> None:
+    data = json.loads(context.json_text)
+    for group in data["groups"]:
+        for entry in group["entries"]:
+            if entry.get("rename_from") == path:
+                return
+    raise AssertionError(f"No entry with rename_from '{path}' in JSON output")

implementation_plan.md

@@ -2670,10 +2670,10 @@ No standalone Q0-Advanced commits planned. Advanced QA enhancements are bundled
   - [X] Quality [Luis]: Run `nox` (all default sessions, including benchmark).
   - [X] Git [Luis]: `git add .`
   - [X] Git [Luis]: `git commit -m "feat(session): add session persistence and repositories"`
-  - [ ] Quality [Luis]: Verify coverage >=97% via `nox -s coverage_report`. If coverage is <97% then review the current unit test coverage report at `build/coverage.xml` and use it to write new Behave based unit tests to improve code coverage. Specifically, write Behave style unit tests that are descriptively named and specifically improves coverage on whichever file has the most uncovered lines by writing tests that will target the uncovered lines in the report. Once that is done rerun `nox -s coverage_report` to verify all tests pass and coverage is above >=97%. Only mark this as complete once coverage is >=97%, if not repeat this task as many times as is needed until coverage reaches >=97%.
-  - [ ] Quality [Luis]: Run `nox` (all default sessions, including benchmark), fix any errors if needed ensuring nox passes across **entire** code base, do not ignore any failure even if it seems unrelated to this commit, fix it.
-  - [ ] Git [Luis]: `git push -u origin feature/m3-session-persistence`
-  - [ ] Forgejo PR [Luis]: Open PR from `feature/m3-session-persistence` to `master` with a suitable and thorough description
+  - [X] Quality [Luis]: Verify coverage >=97% via `nox -s coverage_report`. If coverage is <97% then review the current unit test coverage report at `build/coverage.xml` and use it to write new Behave based unit tests to improve code coverage. Specifically, write Behave style unit tests that are descriptively named and specifically improves coverage on whichever file has the most uncovered lines by writing tests that will target the uncovered lines in the report. Once that is done rerun `nox -s coverage_report` to verify all tests pass and coverage is above >=97%. Only mark this as complete once coverage is >=97%, if not repeat this task as many times as is needed until coverage reaches >=97%.
+   - [X] Quality [Luis]: Run `nox` (all default sessions, including benchmark), fix any errors if needed ensuring nox passes across **entire** code base, do not ignore any failure even if it seems unrelated to this commit, fix it.
+   - [X] Git [Luis]: `git push -u origin feature/m3-session-persistence`
+   - [X] Forgejo PR [Luis]: Open PR from `feature/m3-session-persistence` to `master` with a suitable and thorough description
   **Notes (A7.persistence)**:
   - Migration `a7_001_session_persistence` depends on `b1_001_resource_registry`. Tables: `sessions` (9 cols, ULID PK, namespace default `'local'`) and `session_messages` (8 cols, ULID PK, CASCADE from sessions).
   - `SessionModel.to_domain()` returns full `Session` domain object (not a dict) with parsed `SessionTokenUsage` and hydrated child `SessionMessage` list, unlike `ToolModel.to_domain()` which returns a dict.
@@ -3824,8 +3824,8 @@ No standalone Q0-Advanced commits planned. Advanced QA enhancements are bundled
    - [X] Quality [Luis]: Run `nox` (all default sessions, including benchmark), fix any errors if needed ensuring nox passes across **entire** code base, do not ignore any failure even if it seems unrelated to this commit, fix it.
    - [X] Git [Luis]: Perform an appropriate `git add` command to add all the files that should be part of this commit to the git index
    - [X] Git [Luis]: `git commit -m "feat(skill): add git operation skills"`
-   - [ ] Git [Luis]: `git push -u origin feature/m3-skill-git`
-   - [ ] Forgejo PR [Luis]: Open PR from `feature/m3-skill-git` to `master` with description "Add read-only git operation skills with safety guards and tests.".
+   - [X] Git [Luis]: `git push -u origin feature/m3-skill-git`
+   - [X] Forgejo PR [Luis]: Open PR from `feature/m3-skill-git` to `master` with description "Add read-only git operation skills with safety guards and tests.".
 
 **Parallel Group C5: Tool Routing & Change Tracking [Luis + Jeff]** (depends on C3/C4)
 - [X] **COMMIT (Owner: Luis | Group: C5.model | Branch: feature/m3-change-model | Done: Day 20, February 19, 2026) - Commit message: "feat(change): add ChangeSet models and invocation tracker"**
@@ -3849,8 +3849,8 @@ No standalone Q0-Advanced commits planned. Advanced QA enhancements are bundled
    - [X] Quality [Luis]: Run `nox` (all default sessions), all pass.
    - [X] Git [Luis]: Perform an appropriate `git add` command to add all the files that should be part of this commit to the git index
    - [X] Git [Luis]: `git commit -m "feat(change): add ChangeSet models and invocation tracker"`
-   - [ ] Git [Luis]: `git push -u origin feature/m3-change-model`
-   - [ ] Forgejo PR [Luis]: Open PR from `feature/m3-change-model` to `master` with description "Add ChangeSet models, invocation tracking, and tests.".
+   - [X] Git [Luis]: `git push -u origin feature/m3-change-model`
+   - [X] Forgejo PR [Luis]: Open PR from `feature/m3-change-model` to `master` with description "Add ChangeSet models, invocation tracking, and tests.".
 - [X] **COMMIT (Owner: Jeff | Group: C5.router | Branch: feature/m3-tool-router | Planned: Day 20 | Expected: Day 20) - Commit message: "feat(change): add tool router for providers"**
   - [X] Git [Jeff]: `git checkout master`
   - [X] Git [Jeff]: `git pull origin master`
@@ -3874,27 +3874,27 @@ No standalone Q0-Advanced commits planned. Advanced QA enhancements are bundled
   - [X] Git [Jeff]: `git commit -m "feat(change): add tool router for providers"`
   - [X] Git [Jeff]: `git push -u origin feature/m3-tool-router`
   - [ ] Forgejo PR [Jeff]: Open PR from `feature/m3-tool-router` to `master` with a suitable and thorough description
-- [ ] **COMMIT (Owner: Luis | Group: C5.diff | Branch: feature/m3-diff-review | Planned: Day 20 | Expected: Day 22) - Commit message: "feat(change): add diff review artifacts"**
-  - [ ] Git [Luis]: `git checkout master`
-  - [ ] Git [Luis]: `git pull origin master`
-  - [ ] Git [Luis]: `git checkout -b feature/m3-diff-review`
-  - [ ] Git [Luis]: `git fetch origin && git merge origin/master` (run before final tests and before commit)
-  - [ ] Code [Luis]: Implement DiffBuilder and ReviewArtifact models for CLI review.
-  - [ ] Code [Luis]: Add support for multi-resource diffs and per-resource grouping.
-  - [ ] Code [Luis]: Add diff output serializers for rich/plain/json formats.
-  - [ ] Code [Luis]: Include before/after content hashes and file modes in diff metadata for integrity checks.
-  - [ ] Code [Luis]: Detect binary files and include a binary marker instead of inline diff content.
-  - [ ] Code [Luis]: Add rename/move detection using path similarity + content hash and render as rename events.
-  - [ ] Code [Luis]: Add max diff size guard with truncation notes and a `truncated=true` flag in JSON output.
-  - [ ] Code [Luis]: Ensure diff output ordering is stable (resource name, path, change type).
-  - [ ] Docs [Luis]: Add `docs/reference/diff_review.md` with output format.
-  - [ ] Tests (Behave) [Luis]: Add `features/diff_review.feature` for diff generation.
-  - [ ] Tests (Robot) [Luis]: Add `robot/diff_review.robot` for review artifacts.
-  - [ ] Tests (ASV) [Luis]: Add `benchmarks/diff_review_bench.py` for diff building performance.
-  - [ ] Quality [Luis]: Verify coverage >=97% via `nox -s coverage_report`. If coverage is <97% then review the current unit test coverage report at `build/coverage.xml` and use it to write new Behave based unit tests to improve code coverage. Specifically, write Behave style unit tests that are descriptively named and specifically improves coverage on whichever file has the most uncovered lines by writing tests that will target the uncovered lines in the report. Once that is done rerun `nox -s coverage_report` to verify all tests pass and coverage is above >=97%. Only mark this as complete once coverage is >=97%, if not repeat this task as many times as is needed until coverage reaches >=97%.
-  - [ ] Quality [Luis]: Run `nox` (all default sessions, including benchmark), fix any errors if needed ensuring nox passes across **entire** code base, do not ignore any failure even if it seems unrelated to this commit, fix it.
-  - [ ] Git [Luis]: Perform an appropriate `git add` command to add all the files that should be part of this commit to the git index
-  - [ ] Git [Luis]: `git commit -m "feat(change): add diff review artifacts"`
+- [X] **COMMIT (Owner: Luis | Group: C5.diff | Branch: feature/m3-diff-review | Planned: Day 20 | Expected: Day 22) - Commit message: "feat(change): add diff review artifacts"**
+  - [X] Git [Luis]: `git checkout master`
+  - [X] Git [Luis]: `git pull origin master`
+  - [X] Git [Luis]: `git checkout -b feature/m3-diff-review`
+  - [X] Git [Luis]: `git fetch origin && git merge origin/master` (run before final tests and before commit)
+  - [X] Code [Luis]: Implement DiffBuilder and ReviewArtifact models for CLI review.
+  - [X] Code [Luis]: Add support for multi-resource diffs and per-resource grouping.
+  - [X] Code [Luis]: Add diff output serializers for rich/plain/json formats.
+  - [X] Code [Luis]: Include before/after content hashes and file modes in diff metadata for integrity checks.
+  - [X] Code [Luis]: Detect binary files and include a binary marker instead of inline diff content.
+  - [X] Code [Luis]: Add rename/move detection using path similarity + content hash and render as rename events.
+  - [X] Code [Luis]: Add max diff size guard with truncation notes and a `truncated=true` flag in JSON output.
+  - [X] Code [Luis]: Ensure diff output ordering is stable (resource name, path, change type).
+  - [X] Docs [Luis]: Add `docs/reference/diff_review.md` with output format.
+  - [X] Tests (Behave) [Luis]: Add `features/diff_review.feature` for diff generation.
+  - [X] Tests (Robot) [Luis]: Add `robot/diff_review.robot` for review artifacts.
+  - [X] Tests (ASV) [Luis]: Add `benchmarks/diff_review_bench.py` for diff building performance.
+  - [X] Quality [Luis]: Verify coverage >=97% via `nox -s coverage_report`. If coverage is <97% then review the current unit test coverage report at `build/coverage.xml` and use it to write new Behave based unit tests to improve code coverage. Specifically, write Behave style unit tests that are descriptively named and specifically improves coverage on whichever file has the most uncovered lines by writing tests that will target the uncovered lines in the report. Once that is done rerun `nox -s coverage_report` to verify all tests pass and coverage is above >=97%. Only mark this as complete once coverage is >=97%, if not repeat this task as many times as is needed until coverage reaches >=97%.
+  - [X] Quality [Luis]: Run `nox` (all default sessions, including benchmark), fix any errors if needed ensuring nox passes across **entire** code base, do not ignore any failure even if it seems unrelated to this commit, fix it.
+  - [X] Git [Luis]: Perform an appropriate `git add` command to add all the files that should be part of this commit to the git index
+  - [X] Git [Luis]: `git commit -m "feat(change): add diff review artifacts"`
   - [ ] Git [Luis]: `git push -u origin feature/m3-diff-review`
   - [ ] Forgejo PR [Luis]: Open PR from `feature/m3-diff-review` to `master` with a suitable and thorough description
 

robot/diff_review.robot

@@ -0,0 +1,50 @@
+*** Settings ***
+Documentation    Smoke tests for diff review artifact building,
+...              rename detection, serialization, and truncation.
+Resource         ${CURDIR}/common.resource
+Suite Setup      Setup Test Environment
+Suite Teardown   Cleanup Test Environment
+
+*** Variables ***
+${HELPER}    ${CURDIR}/helper_diff_review.py
+
+*** Test Cases ***
+Build Empty Changeset Artifact
+    [Documentation]    Build a ReviewArtifact from an empty SpecChangeSet
+    ${result}=    Run Process    ${PYTHON}    ${HELPER}    build-empty    cwd=${WORKSPACE}
+    Log    ${result.stdout}
+    Log    ${result.stderr}
+    Should Be Equal As Integers    ${result.rc}    0
+    Should Contain    ${result.stdout}    build-empty-ok
+
+Build Create Change Artifact
+    [Documentation]    Build a ReviewArtifact with a single CREATE change entry
+    ${result}=    Run Process    ${PYTHON}    ${HELPER}    build-create    cwd=${WORKSPACE}
+    Log    ${result.stdout}
+    Log    ${result.stderr}
+    Should Be Equal As Integers    ${result.rc}    0
+    Should Contain    ${result.stdout}    build-create-ok
+
+Detect Rename From Delete And Create
+    [Documentation]    Detect renames by matching content hashes across DELETE/CREATE pairs
+    ${result}=    Run Process    ${PYTHON}    ${HELPER}    build-rename    cwd=${WORKSPACE}
+    Log    ${result.stdout}
+    Log    ${result.stderr}
+    Should Be Equal As Integers    ${result.rc}    0
+    Should Contain    ${result.stdout}    build-rename-ok
+
+Serialize Artifact To All Formats
+    [Documentation]    Serialize a ReviewArtifact to plain text, JSON, and rich format
+    ${result}=    Run Process    ${PYTHON}    ${HELPER}    serialize-all    cwd=${WORKSPACE}
+    Log    ${result.stdout}
+    Log    ${result.stderr}
+    Should Be Equal As Integers    ${result.rc}    0
+    Should Contain    ${result.stdout}    serialize-all-ok
+
+Truncate Large Diff
+    [Documentation]    Verify diff truncation when content exceeds max_diff_size
+    ${result}=    Run Process    ${PYTHON}    ${HELPER}    truncation    cwd=${WORKSPACE}
+    Log    ${result.stdout}
+    Log    ${result.stderr}
+    Should Be Equal As Integers    ${result.rc}    0
+    Should Contain    ${result.stdout}    truncation-ok

robot/helper_diff_review.py

@@ -0,0 +1,200 @@
+"""Robot Framework helper for diff review artifact smoke tests.
+
+Provides a CLI-style interface for Robot to invoke diff review
+model creation, serialization, and builder operations.
+Exit code 0 = success, 1 = failure.
+
+Usage:
+    python robot/helper_diff_review.py build-empty
+    python robot/helper_diff_review.py build-create
+    python robot/helper_diff_review.py build-rename
+    python robot/helper_diff_review.py serialize-all
+    python robot/helper_diff_review.py truncation
+"""
+
+from __future__ import annotations
+
+import json
+import sys
+from pathlib import Path
+from typing import Any
+
+# Ensure the src directory is on the import path.
+_SRC = str(Path(__file__).resolve().parents[1] / "src")
+if _SRC not in sys.path:
+    sys.path.insert(0, _SRC)
+
+from cleveragents.domain.models.core.change import (  # noqa: E402
+    ChangeEntry,
+    ChangeOperation,
+    SpecChangeSet,
+)
+from cleveragents.domain.models.core.diff_review import (  # noqa: E402
+    DiffBuilder,
+    DiffSerializer,
+)
+
+
+def _make_entry(
+    plan_id: str,
+    resource_id: str,
+    operation: str,
+    path: str,
+    *,
+    before_hash: str | None = None,
+    after_hash: str | None = None,
+) -> ChangeEntry:
+    """Build a ChangeEntry with sensible defaults."""
+    kwargs: dict[str, Any] = {
+        "plan_id": plan_id,
+        "resource_id": resource_id,
+        "tool_name": "builtin/test-tool",
+        "operation": ChangeOperation(operation),
+        "path": path,
+    }
+    if before_hash is not None:
+        kwargs["before_hash"] = before_hash
+    if after_hash is not None:
+        kwargs["after_hash"] = after_hash
+    return ChangeEntry(**kwargs)
+
+
+def _cmd_build_empty() -> int:
+    """Build an artifact from an empty changeset."""
+    cs = SpecChangeSet(plan_id="PLAN001")
+    builder = DiffBuilder()
+    artifact = builder.build(cs)
+    assert artifact.total_changes == 0
+    assert len(artifact.groups) == 0
+    assert not artifact.truncated
+    assert artifact.plan_id == "PLAN001"
+    print("build-empty-ok")
+    return 0
+
+
+def _cmd_build_create() -> int:
+    """Build an artifact with a CREATE change."""
+    cs = SpecChangeSet(plan_id="PLAN001")
+    entry = _make_entry("PLAN001", "RES001", "create", "src/main.py")
+    cs.add_change(entry)
+
+    builder = DiffBuilder()
+    artifact = builder.build(
+        cs,
+        after_contents={"src/main.py": "print('hello')\n"},
+    )
+    assert artifact.total_changes == 1
+    assert len(artifact.groups) == 1
+    group = artifact.groups[0]
+    assert group.resource_id == "RES001"
+    assert len(group.entries) == 1
+    assert group.entries[0].change_type == "create"
+    assert group.entries[0].path == "src/main.py"
+    assert "+++" in group.entries[0].diff_text
+    print("build-create-ok")
+    return 0
+
+
+def _cmd_build_rename() -> int:
+    """Build an artifact with rename detection."""
+    cs = SpecChangeSet(plan_id="PLAN001")
+    del_entry = _make_entry(
+        "PLAN001", "RES001", "delete", "old/file.py", before_hash="abc123"
+    )
+    cre_entry = _make_entry(
+        "PLAN001", "RES001", "create", "new/file.py", after_hash="abc123"
+    )
+    cs.add_change(del_entry)
+    cs.add_change(cre_entry)
+
+    builder = DiffBuilder()
+    artifact = builder.build(cs)
+    assert len(artifact.groups) == 1
+    group = artifact.groups[0]
+    assert len(group.entries) == 1
+    assert group.entries[0].is_rename
+    assert group.entries[0].rename_from == "old/file.py"
+    assert group.entries[0].rename_to == "new/file.py"
+    print("build-rename-ok")
+    return 0
+
+
+def _cmd_serialize_all() -> int:
+    """Build an artifact and serialize to all formats."""
+    cs = SpecChangeSet(plan_id="PLAN001")
+    entry = _make_entry("PLAN001", "RES001", "create", "new.py")
+    cs.add_change(entry)
+
+    builder = DiffBuilder()
+    artifact = builder.build(
+        cs,
+        after_contents={"new.py": "hello\n"},
+        resource_names={"RES001": "my-resource"},
+    )
+
+    # Plain text
+    plain = DiffSerializer.to_plain(artifact)
+    assert "my-resource" in plain
+    assert "Total changes: 1" in plain
+
+    # JSON
+    json_text = DiffSerializer.to_json(artifact)
+    data = json.loads(json_text)
+    assert "total_changes" in data
+    assert "groups" in data
+    assert data["total_changes"] == 1
+
+    # Rich
+    rich = DiffSerializer.to_rich(artifact)
+    assert "[bold]" in rich
+    assert "new.py" in rich
+
+    print("serialize-all-ok")
+    return 0
+
+
+def _cmd_truncation() -> int:
+    """Verify truncation with a small max_diff_size."""
+    cs = SpecChangeSet(plan_id="PLAN001")
+    entry = _make_entry("PLAN001", "RES001", "create", "big.py")
+    cs.add_change(entry)
+
+    builder = DiffBuilder(max_diff_size=50)
+    artifact = builder.build(
+        cs,
+        after_contents={"big.py": "x" * 10000},
+    )
+    assert artifact.truncated
+    assert artifact.groups[0].entries[0].truncated
+    print("truncation-ok")
+    return 0
+
+
+def main() -> int:
+    """Entry point called by Robot Framework ``Run Process``."""
+    if len(sys.argv) < 2:
+        print(
+            "Usage: helper_diff_review.py "
+            "<build-empty|build-create|build-rename|serialize-all|truncation>"
+        )
+        return 1
+
+    command = sys.argv[1]
+
+    if command == "build-empty":
+        return _cmd_build_empty()
+    if command == "build-create":
+        return _cmd_build_create()
+    if command == "build-rename":
+        return _cmd_build_rename()
+    if command == "serialize-all":
+        return _cmd_serialize_all()
+    if command == "truncation":
+        return _cmd_truncation()
+
+    print(f"Unknown command: {command}")
+    return 1
+
+
+if __name__ == "__main__":
+    sys.exit(main())

src/cleveragents/domain/models/core/__init__.py

@@ -38,6 +38,13 @@ from cleveragents.domain.models.core.context_policy import (
     ProjectContextPolicy,
 )
 from cleveragents.domain.models.core.debug_attempt import DebugAttempt
+from cleveragents.domain.models.core.diff_review import (
+    DiffBuilder,
+    DiffEntry,
+    DiffSerializer,
+    ResourceDiffGroup,
+    ReviewArtifact,
+)
 from cleveragents.domain.models.core.org import (
     CloudBillingFields,
     CreditsTransaction,
@@ -170,6 +177,9 @@ __all__ = [
     "CreditsTransaction",
     "CreditsTransactionType",
     "DebugAttempt",
+    "DiffBuilder",
+    "DiffEntry",
+    "DiffSerializer",
     "InMemoryChangeSetStore",
     "InMemoryInvocationTracker",
     "InvariantSource",
@@ -207,11 +217,13 @@ __all__ = [
     "Resource",
     "ResourceAccessMode",
     "ResourceCapabilities",
+    "ResourceDiffGroup",
     "ResourceKind",
     "ResourceSlot",
     "ResourceTypeArgument",
     "ResourceTypeSandboxStrategy",
     "ResourceTypeSpec",
+    "ReviewArtifact",
     "SandboxStrategy",
     "Session",
     "SessionExportError",

src/cleveragents/domain/models/core/diff_review.py

@@ -0,0 +1,537 @@
+"""Diff review domain models for CleverAgents.
+
+Builds reviewable diff artifacts from SpecChangeSet records,
+grouping changes by resource and producing unified diff text
+for CLI display in rich, plain, and JSON formats.
+"""
+
+from __future__ import annotations
+
+import difflib
+import json
+from datetime import UTC, datetime
+from difflib import SequenceMatcher
+from typing import Any
+
+from pydantic import BaseModel, ConfigDict, Field
+from ulid import ULID
+
+from cleveragents.domain.models.core.change import (
+    ChangeEntry,
+    ChangeOperation,
+    SpecChangeSet,
+)
+
+DEFAULT_MAX_DIFF_SIZE: int = 50_000
+"""Maximum total diff text characters before truncation."""
+
+RENAME_SIMILARITY_THRESHOLD: float = 0.6
+"""Minimum path similarity ratio for rename detection."""
+
+
+def _new_ulid() -> str:
+    return str(ULID())
+
+
+def _is_binary(content: str | None) -> bool:
+    """Detect binary content by checking for null bytes."""
+    if content is None:
+        return False
+    return "\x00" in content
+
+
+def _unified_diff(
+    before: str | None,
+    after: str | None,
+    path: str,
+    *,
+    context_lines: int = 3,
+) -> str:
+    """Generate a unified diff string between before and after content."""
+    before_lines = (before or "").splitlines(keepends=True)
+    after_lines = (after or "").splitlines(keepends=True)
+    diff_lines = difflib.unified_diff(
+        before_lines,
+        after_lines,
+        fromfile=f"a/{path}",
+        tofile=f"b/{path}",
+        n=context_lines,
+    )
+    return "".join(diff_lines)
+
+
+class DiffEntry(BaseModel):
+    """A single diff entry for CLI review."""
+
+    resource_id: str = Field(..., description="Resource ULID")
+    resource_name: str = Field(
+        default="",
+        description="Human-readable resource name",
+    )
+    path: str = Field(..., min_length=1, description="File path")
+    change_type: str = Field(
+        ...,
+        description="Operation type (create/modify/delete/rename)",
+    )
+    before_content: str | None = Field(
+        default=None,
+        description="Content before the change",
+    )
+    after_content: str | None = Field(
+        default=None,
+        description="Content after the change",
+    )
+    before_hash: str | None = Field(
+        default=None,
+        description="SHA-256 before",
+    )
+    after_hash: str | None = Field(
+        default=None,
+        description="SHA-256 after",
+    )
+    file_mode: int | None = Field(
+        default=None,
+        description="File permission mode",
+    )
+    is_binary: bool = Field(
+        default=False,
+        description="Whether the file is binary",
+    )
+    is_rename: bool = Field(
+        default=False,
+        description="Whether this is a rename/move",
+    )
+    rename_from: str | None = Field(
+        default=None,
+        description="Original path for renames",
+    )
+    rename_to: str | None = Field(
+        default=None,
+        description="Destination path for renames",
+    )
+    diff_text: str = Field(
+        default="",
+        description="Unified diff text",
+    )
+    truncated: bool = Field(
+        default=False,
+        description="Whether the diff text was truncated",
+    )
+
+    model_config = ConfigDict(
+        str_strip_whitespace=True,
+        validate_assignment=True,
+    )
+
+
+class ResourceDiffGroup(BaseModel):
+    """Group of diff entries for a single resource."""
+
+    resource_id: str = Field(..., description="Resource ULID")
+    resource_name: str = Field(
+        default="",
+        description="Human-readable resource name",
+    )
+    entries: list[DiffEntry] = Field(
+        default_factory=list,
+        description="Diff entries for this resource",
+    )
+
+    model_config = ConfigDict(
+        str_strip_whitespace=True,
+        validate_assignment=True,
+    )
+
+    @property
+    def added(self) -> int:
+        """Count of CREATE entries."""
+        return sum(1 for e in self.entries if e.change_type == "create")
+
+    @property
+    def modified(self) -> int:
+        """Count of MODIFY entries."""
+        return sum(1 for e in self.entries if e.change_type == "modify")
+
+    @property
+    def deleted(self) -> int:
+        """Count of DELETE entries."""
+        return sum(1 for e in self.entries if e.change_type == "delete")
+
+    @property
+    def renamed(self) -> int:
+        """Count of RENAME entries."""
+        return sum(1 for e in self.entries if e.change_type == "rename")
+
+
+class ReviewArtifact(BaseModel):
+    """Complete diff review artifact for a plan."""
+
+    artifact_id: str = Field(
+        default_factory=_new_ulid,
+        description="ULID for this review artifact",
+    )
+    plan_id: str = Field(..., description="Plan ULID")
+    changeset_id: str = Field(..., description="ChangeSet ULID")
+    created_at: datetime = Field(
+        default_factory=lambda: datetime.now(UTC),
+        description="When this artifact was created",
+    )
+    groups: list[ResourceDiffGroup] = Field(
+        default_factory=list,
+        description="Diff groups by resource",
+    )
+    total_changes: int = Field(
+        default=0,
+        description="Total number of change entries",
+    )
+    truncated: bool = Field(
+        default=False,
+        description="Whether any diff was truncated",
+    )
+    max_diff_size: int = Field(
+        default=DEFAULT_MAX_DIFF_SIZE,
+        description="Max diff text chars before truncation",
+    )
+
+    model_config = ConfigDict(
+        str_strip_whitespace=True,
+        validate_assignment=True,
+    )
+
+
+class DiffBuilder:
+    """Builds a ReviewArtifact from a SpecChangeSet.
+
+    Takes optional content maps (``before_contents`` and
+    ``after_contents``) keyed by file path to generate unified
+    diffs.  A ``resource_names`` map provides human-readable names
+    for resource IDs.
+    """
+
+    def __init__(
+        self,
+        *,
+        max_diff_size: int = DEFAULT_MAX_DIFF_SIZE,
+        context_lines: int = 3,
+    ) -> None:
+        self._max_diff_size = max_diff_size
+        self._context_lines = context_lines
+
+    def build(
+        self,
+        changeset: SpecChangeSet,
+        *,
+        before_contents: dict[str, str] | None = None,
+        after_contents: dict[str, str] | None = None,
+        resource_names: dict[str, str] | None = None,
+    ) -> ReviewArtifact:
+        """Build a ReviewArtifact from a changeset."""
+        before_map = before_contents or {}
+        after_map = after_contents or {}
+        name_map = resource_names or {}
+
+        grouped = changeset.grouped_by_resource()
+        total_diff_size = 0
+        artifact_truncated = False
+        groups: list[ResourceDiffGroup] = []
+
+        # Detect renames across all entries
+        rename_pairs = self._detect_renames(
+            changeset.entries,
+            before_map,
+            after_map,
+        )
+        consumed_entries: set[str] = set()
+        for del_id, cre_id in rename_pairs:
+            consumed_entries.add(del_id)
+            consumed_entries.add(cre_id)
+
+        for resource_id in sorted(grouped.keys()):
+            entries_for_resource = grouped[resource_id]
+            res_name = name_map.get(resource_id, resource_id)
+            diff_entries: list[DiffEntry] = []
+
+            for entry in entries_for_resource:
+                if entry.entry_id in consumed_entries:
+                    # Skip individually consumed rename entries
+                    continue
+
+                before = before_map.get(entry.path)
+                after = after_map.get(entry.path)
+                binary = _is_binary(before) or _is_binary(after)
+
+                diff_text = ""
+                if binary:
+                    diff_text = "[binary file]"
+                else:
+                    diff_text = _unified_diff(
+                        before,
+                        after,
+                        entry.path,
+                        context_lines=self._context_lines,
+                    )
+
+                entry_truncated = False
+                remaining = self._max_diff_size - total_diff_size
+                if remaining <= 0:
+                    diff_text = "[truncated]"
+                    entry_truncated = True
+                    artifact_truncated = True
+                elif len(diff_text) > remaining:
+                    diff_text = diff_text[:remaining] + "\n[truncated]"
+                    entry_truncated = True
+                    artifact_truncated = True
+
+                total_diff_size += len(diff_text)
+
+                diff_entries.append(
+                    DiffEntry(
+                        resource_id=resource_id,
+                        resource_name=res_name,
+                        path=entry.path,
+                        change_type=entry.operation,
+                        before_content=before,
+                        after_content=after,
+                        before_hash=entry.before_hash,
+                        after_hash=entry.after_hash,
+                        file_mode=entry.after_mode or entry.before_mode,
+                        is_binary=binary,
+                        is_rename=False,
+                        diff_text=diff_text,
+                        truncated=entry_truncated,
+                    )
+                )
+
+            # Add rename entries
+            for del_id, cre_id in rename_pairs:
+                del_entry = self._find_entry(entries_for_resource, del_id)
+                cre_entry = self._find_entry(entries_for_resource, cre_id)
+                if del_entry is None or cre_entry is None:
+                    continue
+
+                after = after_map.get(cre_entry.path)
+                binary = _is_binary(after)
+                diff_text = f"renamed: {del_entry.path} -> {cre_entry.path}"
+
+                entry_truncated = False
+                remaining = self._max_diff_size - total_diff_size
+                if remaining <= 0:
+                    diff_text = "[truncated]"
+                    entry_truncated = True
+                    artifact_truncated = True
+                elif len(diff_text) > remaining:
+                    diff_text = diff_text[:remaining] + "\n[truncated]"
+                    entry_truncated = True
+                    artifact_truncated = True
+
+                total_diff_size += len(diff_text)
+
+                diff_entries.append(
+                    DiffEntry(
+                        resource_id=resource_id,
+                        resource_name=res_name,
+                        path=cre_entry.path,
+                        change_type="rename",
+                        before_content=None,
+                        after_content=after,
+                        before_hash=del_entry.before_hash,
+                        after_hash=cre_entry.after_hash,
+                        file_mode=cre_entry.after_mode,
+                        is_binary=binary,
+                        is_rename=True,
+                        rename_from=del_entry.path,
+                        rename_to=cre_entry.path,
+                        diff_text=diff_text,
+                        truncated=entry_truncated,
+                    )
+                )
+
+            # Sort entries by (path, change_type)
+            diff_entries.sort(key=lambda e: (e.path, e.change_type))
+
+            groups.append(
+                ResourceDiffGroup(
+                    resource_id=resource_id,
+                    resource_name=res_name,
+                    entries=diff_entries,
+                )
+            )
+
+        return ReviewArtifact(
+            plan_id=changeset.plan_id,
+            changeset_id=changeset.changeset_id,
+            groups=groups,
+            total_changes=len(changeset.entries),
+            truncated=artifact_truncated,
+            max_diff_size=self._max_diff_size,
+        )
+
+    def _detect_renames(
+        self,
+        entries: list[ChangeEntry],
+        before_map: dict[str, str],
+        after_map: dict[str, str],
+    ) -> list[tuple[str, str]]:
+        """Detect rename pairs using path similarity + content hash."""
+        deletes = [e for e in entries if e.operation == ChangeOperation.DELETE]
+        creates = [e for e in entries if e.operation == ChangeOperation.CREATE]
+
+        pairs: list[tuple[str, str]] = []
+        used_creates: set[str] = set()
+
+        for del_entry in deletes:
+            best_ratio = 0.0
+            best_create: ChangeEntry | None = None
+
+            for cre_entry in creates:
+                if cre_entry.entry_id in used_creates:
+                    continue
+                if cre_entry.resource_id != del_entry.resource_id:
+                    continue
+
+                # Check content hash match
+                del_hash = del_entry.before_hash
+                cre_hash = cre_entry.after_hash
+                if del_hash is None or cre_hash is None:
+                    continue
+                if del_hash != cre_hash:
+                    continue
+
+                ratio = SequenceMatcher(
+                    None,
+                    del_entry.path,
+                    cre_entry.path,
+                ).ratio()
+                if ratio > best_ratio:
+                    best_ratio = ratio
+                    best_create = cre_entry
+
+            if best_create is not None and best_ratio >= RENAME_SIMILARITY_THRESHOLD:
+                pairs.append((del_entry.entry_id, best_create.entry_id))
+                used_creates.add(best_create.entry_id)
+
+        return pairs
+
+    @staticmethod
+    def _find_entry(
+        entries: list[ChangeEntry],
+        entry_id: str,
+    ) -> ChangeEntry | None:
+        """Find a ChangeEntry by ID in a list."""
+        for e in entries:
+            if e.entry_id == entry_id:
+                return e
+        return None
+
+
+class DiffSerializer:
+    """Serialize a ReviewArtifact to various output formats."""
+
+    @staticmethod
+    def to_plain(artifact: ReviewArtifact) -> str:
+        """Render as plain text."""
+        lines: list[str] = []
+        lines.append(
+            f"Review: plan={artifact.plan_id} changeset={artifact.changeset_id}"
+        )
+        lines.append(f"Total changes: {artifact.total_changes}")
+        if artifact.truncated:
+            lines.append("[some diffs truncated]")
+        lines.append("")
+
+        for group in artifact.groups:
+            lines.append(f"Resource: {group.resource_name} ({group.resource_id})")
+            lines.append(
+                f"  +{group.added} ~{group.modified} -{group.deleted} r{group.renamed}"
+            )
+            for entry in group.entries:
+                lines.append(f"  --- {entry.path} [{entry.change_type}]")
+                if entry.is_binary:
+                    lines.append("  [binary file]")
+                elif entry.diff_text:
+                    for diff_line in entry.diff_text.splitlines():
+                        lines.append(f"  {diff_line}")
+            lines.append("")
+
+        return "\n".join(lines)
+
+    @staticmethod
+    def to_json(artifact: ReviewArtifact) -> str:
+        """Render as JSON string."""
+        data: dict[str, Any] = {
+            "artifact_id": artifact.artifact_id,
+            "plan_id": artifact.plan_id,
+            "changeset_id": artifact.changeset_id,
+            "created_at": artifact.created_at.isoformat(),
+            "total_changes": artifact.total_changes,
+            "truncated": artifact.truncated,
+            "groups": [],
+        }
+        for group in artifact.groups:
+            group_data: dict[str, Any] = {
+                "resource_id": group.resource_id,
+                "resource_name": group.resource_name,
+                "added": group.added,
+                "modified": group.modified,
+                "deleted": group.deleted,
+                "renamed": group.renamed,
+                "entries": [],
+            }
+            for entry in group.entries:
+                entry_data: dict[str, Any] = {
+                    "path": entry.path,
+                    "change_type": entry.change_type,
+                    "is_binary": entry.is_binary,
+                    "is_rename": entry.is_rename,
+                    "truncated": entry.truncated,
+                    "diff_text": entry.diff_text,
+                }
+                if entry.before_hash:
+                    entry_data["before_hash"] = entry.before_hash
+                if entry.after_hash:
+                    entry_data["after_hash"] = entry.after_hash
+                if entry.file_mode is not None:
+                    entry_data["file_mode"] = entry.file_mode
+                if entry.rename_from:
+                    entry_data["rename_from"] = entry.rename_from
+                if entry.rename_to:
+                    entry_data["rename_to"] = entry.rename_to
+                group_data["entries"].append(entry_data)
+            data["groups"].append(group_data)
+
+        return json.dumps(data, indent=2)
+
+    @staticmethod
+    def to_rich(artifact: ReviewArtifact) -> str:
+        """Render as rich-formatted text with color markers."""
+        lines: list[str] = []
+        lines.append(f"[bold]Review[/bold]: plan={artifact.plan_id}")
+        lines.append(f"Changes: {artifact.total_changes}")
+        if artifact.truncated:
+            lines.append("[yellow]Some diffs truncated[/yellow]")
+        lines.append("")
+
+        for group in artifact.groups:
+            lines.append(f"[cyan]{group.resource_name}[/cyan] ({group.resource_id})")
+            lines.append(
+                f"  [green]+{group.added}[/green] "
+                f"[yellow]~{group.modified}[/yellow] "
+                f"[red]-{group.deleted}[/red] "
+                f"r{group.renamed}"
+            )
+            for entry in group.entries:
+                if entry.is_binary:
+                    lines.append(f"  {entry.path} [dim][binary][/dim]")
+                else:
+                    lines.append(f"  {entry.path} [{entry.change_type}]")
+                    if entry.diff_text:
+                        for dl in entry.diff_text.splitlines():
+                            if dl.startswith("+"):
+                                lines.append(f"  [green]{dl}[/green]")
+                            elif dl.startswith("-"):
+                                lines.append(f"  [red]{dl}[/red]")
+                            else:
+                                lines.append(f"  {dl}")
+            lines.append("")
+
+        return "\n".join(lines)