refactor(correction): eliminate redundant fields in CorrectionDryRunReport (#1278)

Remove three top-level fields from CorrectionDryRunReport that duplicated data already present in the embedded CorrectionImpact object:

- excluded_decisions → now accessed via report.impact.excluded_decisions
- rollback_tier_depth → now accessed via report.impact.rollback_tier_depth
- child_plans_to_rollback → now accessed via report.impact.affected_child_plans

The redundant fields created a divergence risk: both models are frozen=True Pydantic models, so post-construction mutation is impossible, but nothing prevented constructing an instance where the top-level copies disagreed with the impact sub-object.

Approach: Option B (nest-only) — remove the duplicated top-level fields and update all consumers to use the impact object's fields instead.

Closes #1087

Co-authored-by: Jeffrey Phillips Freeman <the@jeffreyfreeman.me>
Co-committed-by: Jeffrey Phillips Freeman <the@jeffreyfreeman.me>

CHANGELOG.md

@@ -92,6 +92,14 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 ## Unreleased (pre-3.7.0)
 
+- Eliminated redundant fields (`excluded_decisions`, `rollback_tier_depth`,
+  `child_plans_to_rollback`) from `CorrectionDryRunReport` that duplicated data
+  already present in the embedded `CorrectionImpact` object. Consumers now
+  access these values via `report.impact.excluded_decisions`,
+  `report.impact.rollback_tier_depth`, and `report.impact.affected_child_plans`
+  respectively. Updated `CorrectionService.generate_dry_run_report()`, Behave
+  step definitions, and Robot Framework helpers to use the canonical `impact`
+  sub-object. (#1087)
 - Added a context-sensitive TUI help panel overlay toggled by `F1`, with
   help content that varies for main-screen, slash-command, reference, and
   shell prompt modes. Updated Behave and Robot coverage for help-panel

features/steps/correction_subtree_isolation_steps.py

@@ -199,19 +199,19 @@ def step_subtree_warnings_contain(context: Context, text: str) -> None:
 
 @then("the subtree isolate dry-run report should include excluded decisions")
 def step_subtree_report_has_excluded(context: Context) -> None:
-    """Check that the dry-run report includes excluded decisions."""
+    """Check that the dry-run report includes excluded decisions via impact."""
     report = context.subtree_report
-    assert len(report.excluded_decisions) > 0, (
-        f"Expected non-empty excluded_decisions, got {report.excluded_decisions}"
+    assert len(report.impact.excluded_decisions) > 0, (
+        f"Expected non-empty excluded_decisions, got {report.impact.excluded_decisions}"
     )
 
 
 @then("the subtree isolate dry-run report rollback tier depth should be {expected:d}")
 def step_subtree_report_tier(context: Context, expected: int) -> None:
-    """Check the rollback tier depth in the dry-run report."""
+    """Check the rollback tier depth in the dry-run report via impact."""
     report = context.subtree_report
-    assert report.rollback_tier_depth == expected, (
-        f"Expected report tier_depth={expected}, got {report.rollback_tier_depth}"
+    assert report.impact.rollback_tier_depth == expected, (
+        f"Expected report tier_depth={expected}, got {report.impact.rollback_tier_depth}"
     )
 
 
@@ -219,10 +219,10 @@ def step_subtree_report_tier(context: Context, expected: int) -> None:
     'the subtree isolate dry-run report excluded decisions should contain "{decision_id}"'
 )
 def step_subtree_report_excluded_contains(context: Context, decision_id: str) -> None:
-    """Check the dry-run report's excluded decisions list."""
+    """Check the dry-run report's excluded decisions list via impact."""
     report = context.subtree_report
-    assert decision_id in report.excluded_decisions, (
-        f"Expected '{decision_id}' in report excluded={report.excluded_decisions}"
+    assert decision_id in report.impact.excluded_decisions, (
+        f"Expected '{decision_id}' in report excluded={report.impact.excluded_decisions}"
     )
 
 

robot/helper_correction_subtree_isolation.py

@@ -117,19 +117,22 @@ def _rollback_tier_computation() -> None:
 
 
 def _dry_run_report_enhanced() -> None:
-    """Verify dry-run report includes excluded decisions and tier depth."""
+    """Verify dry-run report includes excluded decisions and tier depth via impact."""
     svc = CorrectionService()
     tree = _sample_tree()
     req = svc.request_correction(_PLAN_ID, "A", CorrectionMode.REVERT)
     report = svc.generate_dry_run_report(req.correction_id, tree)
 
-    assert report.rollback_tier_depth == 1, (
-        f"Expected tier_depth=1, got {report.rollback_tier_depth}"
+    assert report.impact.rollback_tier_depth == 1, (
+        f"Expected tier_depth=1, got {report.impact.rollback_tier_depth}"
+    )
+    assert len(report.impact.excluded_decisions) > 0, (
+        "Expected non-empty excluded_decisions"
     )
-    assert len(report.excluded_decisions) > 0, "Expected non-empty excluded_decisions"
     for excluded in ["root", "B", "E"]:
-        assert excluded in report.excluded_decisions, (
-            f"Expected '{excluded}' in report excluded={report.excluded_decisions}"
+        assert excluded in report.impact.excluded_decisions, (
+            f"Expected '{excluded}' in report "
+            f"excluded={report.impact.excluded_decisions}"
         )
     print("dry-run-report-enhanced-ok")
 
@@ -143,7 +146,7 @@ def _dry_run_tier_zero_warning() -> None:
 
     all_warnings = " ".join(report.warnings)
     assert "Tier 0" in all_warnings, f"Expected 'Tier 0' in warnings: {report.warnings}"
-    assert report.rollback_tier_depth == 0
+    assert report.impact.rollback_tier_depth == 0
     print("dry-run-tier-zero-warning-ok")
 
 

robot/helper_m6_e2e_verification.py

@@ -416,7 +416,6 @@ def correction_affected_subtree() -> None:
         mode=CorrectionMode.REVERT,
         impact=impact,
         decisions_to_invalidate=[target_id],
-        child_plans_to_rollback=[],
     )
 
     if len(report.decisions_to_invalidate) != 1:

src/cleveragents/application/services/correction_service.py

@@ -391,9 +391,6 @@ class CorrectionService:
             decisions_to_invalidate=impact.affected_decisions
             if request.mode == CorrectionMode.REVERT
             else [],
-            child_plans_to_rollback=impact.affected_child_plans,
-            excluded_decisions=list(impact.excluded_decisions),
-            rollback_tier_depth=impact.rollback_tier_depth,
             estimated_recompute_time_seconds=recompute_seconds,
             warnings=warnings,
         )

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

@@ -295,6 +295,18 @@ class CorrectionDryRunReport(BaseModel):
 
     Generated by ``CorrectionService.generate_dry_run_report`` without
     actually modifying any state.
+
+    Design rationale (Option B — nest-only):
+    The embedded ``impact`` object is the single authoritative source for
+    impact data.  Previously, ``excluded_decisions``, ``rollback_tier_depth``,
+    and ``child_plans_to_rollback`` were duplicated at the top level, creating
+    a divergence risk: both models are ``frozen=True``, so post-construction
+    mutation is impossible, but nothing prevented constructing an instance
+    where the top-level copies disagreed with ``impact``.  Removing the
+    redundant fields eliminates that ambiguity.  Consumers should access
+    these values via ``report.impact.excluded_decisions``,
+    ``report.impact.rollback_tier_depth``, and
+    ``report.impact.affected_child_plans`` respectively.
     """
 
     model_config = ConfigDict(frozen=True)
@@ -309,24 +321,16 @@ class CorrectionDryRunReport(BaseModel):
     )
     impact: CorrectionImpact = Field(
         ...,
-        description="Full impact analysis.",
+        description=(
+            "Full impact analysis.  Use ``impact.excluded_decisions``, "
+            "``impact.rollback_tier_depth``, and ``impact.affected_child_plans`` "
+            "to access data that was previously duplicated at the top level."
+        ),
     )
     decisions_to_invalidate: list[str] = Field(
         default_factory=list,
         description="Decisions that would be marked invalid.",
     )
-    child_plans_to_rollback: list[str] = Field(
-        default_factory=list,
-        description="Child plans that would be rolled back.",
-    )
-    excluded_decisions: list[str] = Field(
-        default_factory=list,
-        description="Decisions NOT affected by the correction.",
-    )
-    rollback_tier_depth: int = Field(
-        default=0,
-        description="Depth of target decision from tree root.",
-    )
     estimated_recompute_time_seconds: float | None = Field(
         default=None,
         description="Estimated wall-clock seconds to recompute.",