fix(plans): split correction services, create CLI, fix type ignores and remove empty step files
- Split correction_service.py (1255→279 lines) into orchestration-only service that delegates BFS impact analysis to new ImpactAnalysisService - Created correction_impact_service.py (190 lines) with BFS traversal, risk classification, cost estimation, tree topology helpers - Created plan_correction_cli.py (208 lines) extracted from plan.py for 'agents plan correct' command entry point - Removed all 51 empty step files in features/steps/ that had no content - Replaced '# type: ignore' comments with cast() in plan.py (lines 4066, 4078) - All new service files stay under 500-line limit ISSUES CLOSED: #9599
This commit is contained in:
@@ -0,0 +1,190 @@
|
||||
"""Impact analysis service for decision corrections."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from collections import deque
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
import structlog
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from cleveragents.domain.models.core.correction import (
|
||||
CorrectionDryRunReport,
|
||||
CorrectionImpact,
|
||||
CorrectionMode,
|
||||
)
|
||||
|
||||
logger = structlog.get_logger(__name__)
|
||||
|
||||
|
||||
RISK_LOW_MAX = 3
|
||||
RISK_MEDIUM_MAX = 10
|
||||
COST_PER_DECISION = 1.5
|
||||
RECOMPUTE_SECONDS_PER_DECISION = 2.0
|
||||
MAX_TREE_NODES = 50_000
|
||||
|
||||
|
||||
class ImpactAnalysisService:
|
||||
"""Stateless impact analysis for correction requests."""
|
||||
|
||||
@staticmethod
|
||||
def compute_affected_subtree(
|
||||
target_id: str,
|
||||
tree: dict[str, list[str]],
|
||||
influence_edges: dict[str, list[str]] | None = None,
|
||||
) -> list[str]:
|
||||
"""BFS walk from target_id through structural tree AND influence DAG."""
|
||||
dag = influence_edges or {}
|
||||
affected: list[str] = []
|
||||
visited: set[str] = set()
|
||||
queue: deque[str] = deque([target_id])
|
||||
|
||||
while queue:
|
||||
node = queue.popleft()
|
||||
if node in visited:
|
||||
continue
|
||||
visited.add(node)
|
||||
affected.append(node)
|
||||
for neighbor in tree.get(node, []):
|
||||
if neighbor not in visited:
|
||||
queue.append(neighbor)
|
||||
for neighbor in dag.get(node, []):
|
||||
if neighbor not in visited:
|
||||
queue.append(neighbor)
|
||||
|
||||
influence_count = sum(len(v) for v in dag.values()) if dag else 0
|
||||
if influence_count > 0:
|
||||
logger.info("impact.influence_traversal", target_id=target_id,
|
||||
total_affected=len(affected), influence_edge_count=influence_count)
|
||||
return affected
|
||||
|
||||
def validate_subtree_isolation(
|
||||
self, target_decision_id: str, decision_tree: dict[str, list[str]],
|
||||
influence_edges: dict[str, list[str]] | None = None,
|
||||
) -> bool:
|
||||
"""Validate that the affected subtree is correctly isolated."""
|
||||
structural_affected = self.compute_affected_subtree(
|
||||
target_decision_id, decision_tree, influence_edges=None)
|
||||
structural_set = set(structural_affected)
|
||||
root = self.find_root(decision_tree)
|
||||
if root is None:
|
||||
return True
|
||||
if root in structural_set and root != target_decision_id:
|
||||
logger.warning("impact.isolation_violation_root", root=root, target=target_decision_id)
|
||||
return False
|
||||
parent = self.find_parent(target_decision_id, decision_tree)
|
||||
if parent is not None:
|
||||
for sibling in (decision_tree.get(parent, []) or []):
|
||||
if sibling != target_decision_id and sibling in structural_set:
|
||||
logger.warning("impact.isolation_violation_sibling", sibling=sibling,
|
||||
target=target_decision_id)
|
||||
return False
|
||||
return True
|
||||
|
||||
@staticmethod
|
||||
def classify_risk(affected_count: int) -> str:
|
||||
"""Classify risk level based on affected subtree size."""
|
||||
if affected_count <= RISK_LOW_MAX:
|
||||
return "low"
|
||||
if affected_count <= RISK_MEDIUM_MAX:
|
||||
return "medium"
|
||||
return "high"
|
||||
|
||||
@staticmethod
|
||||
def estimate_cost(affected_count: int) -> float:
|
||||
"""Estimate recompute cost."""
|
||||
return float(affected_count * COST_PER_DECISION)
|
||||
|
||||
@staticmethod
|
||||
def estimate_recompute_time(affected_count: int) -> float:
|
||||
"""Estimate wall-clock seconds needed to recompute the subtree."""
|
||||
return affected_count * RECOMPUTE_SECONDS_PER_DECISION
|
||||
|
||||
@staticmethod
|
||||
def collect_all_decisions(tree: dict[str, list[str]], dag: dict[str, list[str]]) -> set[str]:
|
||||
"""Collect every decision ID from both tree and DAG edges."""
|
||||
all_ids: set[str] = set()
|
||||
for parent, children in tree.items():
|
||||
all_ids.add(parent)
|
||||
all_ids.update(children)
|
||||
for source, targets in dag.items():
|
||||
all_ids.add(source)
|
||||
all_ids.update(targets)
|
||||
return all_ids
|
||||
|
||||
@staticmethod
|
||||
def compute_rollback_tier_depth(target_id: str, tree: dict[str, list[str]]) -> int:
|
||||
"""Count parent hops from target_id up to the tree root."""
|
||||
child_to_parent: dict[str, str] = {}
|
||||
for parent, children in tree.items():
|
||||
for child in (children or []):
|
||||
child_to_parent[child] = parent
|
||||
depth = 0
|
||||
current = target_id
|
||||
visited: set[str] = set()
|
||||
while current in child_to_parent and current not in visited:
|
||||
visited.add(current)
|
||||
current = child_to_parent[current]
|
||||
depth += 1
|
||||
return depth
|
||||
|
||||
@staticmethod
|
||||
def find_root(tree: dict[str, list[str]]) -> str | None:
|
||||
"""Find the root node (not a child of any other node)."""
|
||||
if not tree:
|
||||
return None
|
||||
all_children: set[str] = set()
|
||||
for children in tree.values():
|
||||
all_children.update(children)
|
||||
for parent in tree:
|
||||
if parent not in all_children:
|
||||
return parent
|
||||
return next(iter(tree))
|
||||
|
||||
@staticmethod
|
||||
def find_parent(target_id: str, tree: dict[str, list[str]]) -> str | None:
|
||||
"""Find the parent of target_id in the tree, or None."""
|
||||
for parent, children in tree.items():
|
||||
if target_id in children:
|
||||
return parent
|
||||
return None
|
||||
|
||||
|
||||
def build_impact(
|
||||
target_decision_id: str, mode: CorrectionMode,
|
||||
decision_tree: dict[str, list[str]] | None = None,
|
||||
influence_edges: dict[str, list[str]] | None = None,
|
||||
) -> CorrectionImpact:
|
||||
"""Build a CorrectionImpact from scratch given IDs and topology."""
|
||||
svc = ImpactAnalysisService()
|
||||
tree = decision_tree or {}
|
||||
dag = influence_edges or {}
|
||||
|
||||
total_keys = len(tree) + len(dag)
|
||||
if total_keys > MAX_TREE_NODES:
|
||||
raise ValueError(f"Tree too large ({total_keys} keys, max {MAX_TREE_NODES}).")
|
||||
|
||||
affected = svc.compute_affected_subtree(target_decision_id, tree, dag)
|
||||
risk = svc.classify_risk(len(affected))
|
||||
all_decisions = svc.collect_all_decisions(tree, dag)
|
||||
all_decisions.add(target_decision_id)
|
||||
excluded = sorted(d for d in all_decisions if d not in set(affected))
|
||||
tier_depth = svc.compute_rollback_tier_depth(target_decision_id, tree)
|
||||
|
||||
return CorrectionImpact(
|
||||
affected_decisions=affected, excluded_decisions=excluded,
|
||||
affected_files=[f"{d}.py" for d in affected],
|
||||
affected_child_plans=[],
|
||||
estimated_cost=svc.estimate_cost(len(affected)),
|
||||
risk_level=risk,
|
||||
rollback_tier="full" if mode == CorrectionMode.REVERT else "append_only",
|
||||
rollback_tier_depth=tier_depth,
|
||||
artifacts_to_archive=[f"{d}.artifact" for d in affected],
|
||||
)
|
||||
|
||||
|
||||
__all__ = [
|
||||
"ImpactAnalysisService", "build_impact",
|
||||
"COST_PER_DECISION", "MAX_TREE_NODES", "RISK_LOW_MAX",
|
||||
"RISK_MEDIUM_MAX", "RECOMPUTE_SECONDS_PER_DECISION",
|
||||
]
|
||||
File diff suppressed because it is too large
Load Diff
@@ -4530,7 +4530,7 @@ def build_decision_tree(
|
||||
for rid in roots:
|
||||
node = _node_dict(by_id[rid])
|
||||
result.append(node)
|
||||
queue.append((rid, node["children"], 1)) # type: ignore[arg-type] # children value is list at runtime; dict[str, object] prevents narrowing
|
||||
queue.append((rid, cast(list[dict[str, object]], node["children"]), 1))
|
||||
|
||||
while queue:
|
||||
did, parent_list, depth_val = queue.popleft()
|
||||
@@ -4541,9 +4541,9 @@ def build_decision_tree(
|
||||
continue
|
||||
child_node = _node_dict(by_id[child_id])
|
||||
parent_list.append(child_node)
|
||||
queue.append(
|
||||
(child_id, child_node["children"], depth_val + 1) # type: ignore[arg-type] # children value is list at runtime; dict[str, object] prevents narrowing
|
||||
)
|
||||
queue.append(
|
||||
(child_id, cast(list[dict[str, object]], child_node["children"]), depth_val + 1)
|
||||
)
|
||||
|
||||
return result
|
||||
|
||||
|
||||
@@ -0,0 +1,208 @@
|
||||
"""Plan correction CLI commands: agents plan correct / agents plan revert."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import os
|
||||
from pathlib import Path
|
||||
|
||||
import typer
|
||||
from rich.console import Console
|
||||
from rich.panel import Panel
|
||||
|
||||
console = Console()
|
||||
app = typer.Typer()
|
||||
_FORMAT_HELP = "Output format: json, yaml, plain, table, or rich (default: rich)"
|
||||
|
||||
|
||||
def _get_lifecycle_service() -> object:
|
||||
"""Get PlanLifecycleService from container."""
|
||||
from cleveragents.application.container import get_container
|
||||
return get_container().plan_lifecycle_service()
|
||||
|
||||
|
||||
def _resolve_active_plan_id() -> str:
|
||||
"""Resolve the active plan ID when none is explicitly provided."""
|
||||
|
||||
def _fallback_home() -> str | None:
|
||||
if any(os.environ.get(k, "").strip() for k in ("CLEVERAGENTS_DATABASE_URL",)):
|
||||
return None
|
||||
home_raw = os.environ.get("CLEVERAGENTS_HOME", "").strip()
|
||||
if not home_raw:
|
||||
return None
|
||||
try:
|
||||
from cleveragents.infrastructure.database.unit_of_work import UnitOfWork # noqa: TID251
|
||||
home_db = (Path(home_raw).expanduser() / ".cleveragents" / "db.sqlite").resolve(strict=False)
|
||||
uow = UnitOfWork(f"sqlite:///{home_db}", require_confirmation=False)
|
||||
with uow.transaction() as ctx:
|
||||
plans = ctx.lifecycle_plans.list_all()
|
||||
active = [p for p in plans if not p.is_terminal]
|
||||
return active[0].identity.plan_id if active else None
|
||||
except Exception:
|
||||
return None
|
||||
|
||||
try:
|
||||
svc = _get_lifecycle_service()
|
||||
plans = svc.list_plans() # noqa: TID251
|
||||
active = [p for p in plans if not p.is_terminal]
|
||||
if not active:
|
||||
fb = _fallback_home()
|
||||
if fb:
|
||||
return fb
|
||||
console.print("[red]Error:[/red] No active plan found. Specify --plan.")
|
||||
raise typer.Abort()
|
||||
return active[0].identity.plan_id
|
||||
except Exception as exc:
|
||||
console.print("[red]Error:[/red] Could not resolve active plan. Use --plan.")
|
||||
raise typer.Abort() from exc
|
||||
|
||||
|
||||
def _format_output(data: dict, fmt: str) -> None:
|
||||
"""Serialise data into the requested format."""
|
||||
if fmt == "json":
|
||||
console.print(json.dumps(data, indent=2))
|
||||
elif fmt == "yaml":
|
||||
try:
|
||||
import yaml # noqa: TID251
|
||||
console.print(yaml.dump(data, default_flow_style=False))
|
||||
except ImportError:
|
||||
console.print(json.dumps(data, indent=2))
|
||||
elif fmt == "plain":
|
||||
for k, v in data.items():
|
||||
console.print(f"{k}: {v}")
|
||||
elif fmt == "table":
|
||||
from rich.table import Table # noqa: TID251
|
||||
t = Table(title="Results")
|
||||
t.add_column("Field", style="cyan")
|
||||
t.add_column("Value")
|
||||
for k, v in data.items():
|
||||
t.add_row(str(k), str(v) if v is not None else "")
|
||||
console.print(t)
|
||||
|
||||
|
||||
@app.command("correct")
|
||||
def correct_decision(
|
||||
identifier: str = typer.Argument(help="Plan ID or Decision ID"),
|
||||
mode: str = typer.Option(..., "--mode", "-m", help="Correction mode: revert or append"),
|
||||
guidance: str = typer.Option(..., "--guidance", "-g", help="Guidance text"),
|
||||
dry_run: bool = typer.Option(False, "--dry-run", help="Only analyze impact"),
|
||||
yes: bool = typer.Option(False, "--yes", "-y", help="Skip confirmation"),
|
||||
plan_id: str | None = typer.Option(None, "--plan", "-p", help="Plan ID"),
|
||||
fmt: str = typer.Option("rich", "--format", "-f", help=_FORMAT_HELP),
|
||||
) -> None:
|
||||
"""Correct a decision in a plan's decision tree.
|
||||
|
||||
The positional identifier can be either a plan ID or a decision ID.
|
||||
When a plan ID is given the root decision of that plan is automatically
|
||||
selected as the correction target."""
|
||||
|
||||
from cleveragents.core.exceptions import ResourceNotFoundError as RNF, ValidationError
|
||||
from cleveragents.domain.models.core.correction import CorrectionMode
|
||||
from cleveragents.application.container import get_container
|
||||
from cleveragents.domain.models.core.plan import Plan
|
||||
|
||||
try:
|
||||
try:
|
||||
correction_mode = CorrectionMode(mode)
|
||||
except ValueError as exc:
|
||||
console.print(f"[red]Invalid mode:[/red] {mode}. Must be 'revert' or 'append'.")
|
||||
raise typer.Abort() from exc
|
||||
|
||||
if not guidance.strip():
|
||||
console.print("[red]Error:[/red] --guidance / -g must not be blank.")
|
||||
raise typer.Abort()
|
||||
|
||||
container = get_container()
|
||||
decision_svc = container.decision_service()
|
||||
target_decision_id: str
|
||||
resolved_plan_id: str
|
||||
_is_plan = False
|
||||
|
||||
try:
|
||||
lo = container.plan_lifecycle_service()
|
||||
plan_obj = lo.get_plan(identifier)
|
||||
if isinstance(plan_obj, Plan):
|
||||
_is_plan = True
|
||||
except RNF:
|
||||
pass
|
||||
|
||||
if _is_plan:
|
||||
resolved_plan_id = identifier
|
||||
decisions = decision_svc.list_decisions(resolved_plan_id)
|
||||
roots = [d for d in decisions if d.parent_decision_id is None]
|
||||
if not roots:
|
||||
console.print(f"[red]Error:[/red] Plan '{identifier}' has no root decision.")
|
||||
raise typer.Abort()
|
||||
target_decision_id = roots[0].decision_id
|
||||
else:
|
||||
target_decision_id = identifier
|
||||
resolved_plan_id = plan_id or _resolve_active_plan_id()
|
||||
|
||||
decisions = decision_svc.list_decisions(resolved_plan_id)
|
||||
decision_tree: dict[str, list[str]] = {}
|
||||
for d in decisions:
|
||||
if d.parent_decision_id is not None:
|
||||
decision_tree.setdefault(d.parent_decision_id, []).append(d.decision_id)
|
||||
|
||||
influence_edges = decision_svc.get_influence_edges(resolved_plan_id)
|
||||
svc = container.correction_service()
|
||||
request = svc.request_correction(
|
||||
plan_id=resolved_plan_id, target_decision_id=target_decision_id,
|
||||
mode=correction_mode, guidance=guidance, dry_run=dry_run)
|
||||
|
||||
if dry_run:
|
||||
impact = svc.analyze_impact(request.correction_id, decision_tree, influence_edges)
|
||||
if fmt != "rich":
|
||||
data = {"correction_id": request.correction_id, "mode": request.mode.value,
|
||||
"target_decision": request.target_decision_id,
|
||||
"affected_decisions": impact.affected_decisions,
|
||||
"affected_files": impact.affected_files,
|
||||
"estimated_cost": impact.estimated_cost, "risk_level": impact.risk_level}
|
||||
_format_output(data, fmt)
|
||||
else:
|
||||
console.print(Panel(
|
||||
f"[bold]Correction ID:[/bold] {request.correction_id}\n"
|
||||
f"[bold]Mode:[/bold] {request.mode.value}\n"
|
||||
f"[bold]Target Decision:[/bold] {request.target_decision_id}\n"
|
||||
f"[bold]Guidance:[/bold] {request.guidance}\n\n"
|
||||
f"[bold]Affected Decisions:[/bold] "
|
||||
f"{', '.join(impact.affected_decisions) or '(none)'}\n"
|
||||
f"[bold]Risk Level:[/bold] {impact.risk_level}\n"
|
||||
f"[bold]Estimated Cost:[/bold] {impact.estimated_cost or 'N/A'}",
|
||||
title="Correction Impact (Dry Run)", expand=False))
|
||||
return
|
||||
|
||||
if not yes:
|
||||
console.print(f"\n[bold]Correction:[/bold] {correction_mode.value} "
|
||||
f"decision {target_decision_id}")
|
||||
confirm = typer.confirm("\nProceed with correction?")
|
||||
if not confirm:
|
||||
raise typer.Exit(0)
|
||||
|
||||
result = svc.execute_correction(request.correction_id, decision_tree, influence_edges)
|
||||
|
||||
if fmt != "rich":
|
||||
data = {"correction_id": result.correction_id, "status": result.status.value,
|
||||
"mode": correction_mode.value, "new_decisions": result.new_decisions,
|
||||
"reverted_decisions": result.reverted_decisions}
|
||||
_format_output(data, fmt)
|
||||
else:
|
||||
console.print(f"[green]✓[/green] Correction applied: {result.correction_id}")
|
||||
if result.reverted_decisions:
|
||||
console.print(f" Reverted: {', '.join(result.reverted_decisions)}")
|
||||
if result.new_decisions:
|
||||
console.print(f" New decisions: {', '.join(result.new_decisions)}")
|
||||
|
||||
except RNF as e:
|
||||
console.print(f"[red]Not found:[/red] {e.message}")
|
||||
raise typer.Abort() from e
|
||||
except ValidationError as e:
|
||||
console.print(f"[red]Validation Error:[/red] {e.message}")
|
||||
raise typer.Abort() from e
|
||||
except Exception as e:
|
||||
console.print(f"[red]Error:[/red] {e}")
|
||||
raise typer.Abort() from e
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
app()
|
||||
Reference in New Issue
Block a user