feat(plan): decision correction recomputes only affected subtree #1075
@@ -55,6 +55,56 @@
|
||||
hides them from `resource add` scaffolding (user_addable: false). Includes
|
||||
YAML configurations, Behave BDD tests (47 scenarios), Robot Framework
|
||||
integration tests, ASV benchmarks, and reference documentation. (#331)
|
||||
- Enhanced `CorrectionService` subtree isolation: `analyze_impact()` now
|
||||
populates `excluded_decisions` and `rollback_tier_depth`; added
|
||||
`compute_rollback_tier()`, `validate_subtree_isolation()`, and dry-run
|
||||
report enhancements with tier-0 root-targeted warnings. Fixed status
|
||||
state-machine regression in `execute_revert()` where `analyze_impact()`
|
||||
overwrote status back to ANALYZING; `execute_revert()` now transitions
|
||||
through ANALYZING before EXECUTING for correct lifecycle ordering.
|
||||
Fixed `validate_subtree_isolation()` to check structural-only BFS for
|
||||
sibling invariant so influence-DAG-caused sibling reachability is not
|
||||
misreported as a violation. Fixed false-positive cycle-detection warnings
|
||||
from convergent (diamond) topologies by using a global enqueued set in
|
||||
BFS instead of per-node seen_this_round. Added `dry_run` enforcement
|
||||
guard in `_assert_executable()` to prevent execution of dry-run-only
|
||||
corrections per spec (§ plan correct --dry-run). Added terminal-state
|
||||
guard in `analyze_impact()` to reject re-analysis after execution.
|
||||
Added mode validation in `execute_revert()`/`execute_append()` to
|
||||
prevent mode-mismatched execution. Fixed `generate_dry_run_report()`
|
||||
to preserve request status (dry-run is non-mutating). Fixed tier-0
|
||||
warning to only trigger when target is genuinely in the structural
|
||||
tree. Fixed `_collect_all_decisions()` to always include the target
|
||||
decision in the universe. Improved cycle-detection log message
|
||||
accuracy. Extracted cost/time estimation constants. Fixed
|
||||
`generate_dry_run_report()` to use try/finally for status restoration
|
||||
so that an exception during `analyze_impact()` does not leave the
|
||||
request stuck in ANALYZING status. Promoted terminal-status set to
|
||||
a module-level `_TERMINAL_STATUSES` frozenset constant. Review-cycle
|
||||
fixes: fixed `generate_dry_run_report()` to also restore `_impacts`
|
||||
dict (not only status) so dry-run is fully non-mutating; fixed tier-0
|
||||
warning to compare against the actual root via `_find_root()` rather
|
||||
than key-in-tree heuristic, preventing false warnings for forest
|
||||
topologies; fixed BFS cycle detection to log at enqueue time so the
|
||||
warning is reachable (previously dead code due to redundant visited
|
||||
vs enqueued sets); added `_EXECUTABLE_STATUSES` module-level
|
||||
frozenset for `_assert_executable()` and `cancel_correction()`;
|
||||
added `_MAX_TREE_NODES` guard in `analyze_impact()` to reject
|
||||
pathologically large inputs; `execute_revert()` now reuses cached
|
||||
impact from `_impacts` when available to avoid redundant O(V+E)
|
||||
recomputation; `execute_append()` now transitions through ANALYZING
|
||||
before EXECUTING for consistent lifecycle across both modes; event
|
||||
emission (`_emit_correction_applied`) now includes `attempt_id` and
|
||||
logs failures at error level; added `max_length=10000` to
|
||||
`CorrectionRequest.guidance` field; moved status transition after
|
||||
attempt creation in both execution paths. Includes Behave BDD
|
||||
scenarios (influence DAG, append mode, negative isolation validation,
|
||||
dry-run enforcement, execute-revert end-to-end, status guard, mode
|
||||
mismatch, single-node tree, terminal state guard, exact-match
|
||||
affected count, convergent diamond DAG topology, dry-run exception
|
||||
recovery, execute-revert with influence edges, DAG-only nodes in
|
||||
excluded set), Robot Framework integration tests, and updated
|
||||
dry-run report model fields. (#845)
|
||||
- Added deferred physical resource types for git object taxonomy
|
||||
(`git`, `git-remote`, `git-branch`, `git-tag`, `git-commit`, `git-tree`,
|
||||
`git-tree-entry`, `git-stash`, `git-submodule`) and filesystem link types
|
||||
|
||||
@@ -0,0 +1,254 @@
|
||||
Feature: Decision correction recomputes only affected subtree
|
||||
As a developer using CleverAgents correction flows
|
||||
I want corrections to recompute only the affected subtree
|
||||
So that sibling and ancestor decisions are preserved unchanged
|
||||
|
||||
Background:
|
||||
Given a subtree isolation correction service
|
||||
And a subtree isolation decision tree with root "root" and children
|
||||
| parent | children |
|
||||
| root | A,B |
|
||||
| A | C,D |
|
||||
| B | E |
|
||||
|
||||
# --- Leaf targeting ---
|
||||
|
||||
Scenario: Correction targets leaf node - only leaf affected
|
||||
When I subtree isolate target decision "D" with mode "revert"
|
||||
And I subtree isolate analyze the impact
|
||||
Then the subtree isolate affected decisions should be "D"
|
||||
And the subtree isolate excluded decisions should contain "root"
|
||||
And the subtree isolate excluded decisions should contain "A"
|
||||
And the subtree isolate excluded decisions should contain "B"
|
||||
And the subtree isolate excluded decisions should contain "C"
|
||||
And the subtree isolate excluded decisions should contain "E"
|
||||
|
||||
# --- Middle node targeting ---
|
||||
|
||||
Scenario: Correction targets middle node - middle and descendants affected
|
||||
When I subtree isolate target decision "A" with mode "revert"
|
||||
And I subtree isolate analyze the impact
|
||||
Then the subtree isolate affected decisions should contain "A"
|
||||
And the subtree isolate affected decisions should contain "C"
|
||||
And the subtree isolate affected decisions should contain "D"
|
||||
And the subtree isolate excluded decisions should contain "root"
|
||||
And the subtree isolate excluded decisions should contain "B"
|
||||
And the subtree isolate excluded decisions should contain "E"
|
||||
|
||||
# --- Root targeting ---
|
||||
|
||||
Scenario: Correction targets root - all decisions affected with tier 0 warning
|
||||
When I subtree isolate target decision "root" with mode "revert"
|
||||
And I subtree isolate generate a dry-run report
|
||||
Then the subtree isolate affected decisions should contain "root"
|
||||
And the subtree isolate affected decisions should contain "A"
|
||||
And the subtree isolate affected decisions should contain "B"
|
||||
And the subtree isolate affected decisions should contain "C"
|
||||
And the subtree isolate affected decisions should contain "D"
|
||||
And the subtree isolate affected decisions should contain "E"
|
||||
And the subtree isolate excluded decisions should be empty
|
||||
And the subtree isolate rollback tier depth should be 0
|
||||
And the subtree isolate dry-run warnings should contain "Tier 0"
|
||||
|
||||
# --- Rollback tier computation ---
|
||||
|
||||
Scenario: Rollback tier is 0 for root
|
||||
When I subtree isolate compute rollback tier for "root"
|
||||
Then the subtree isolate rollback tier should be 0
|
||||
|
||||
Scenario: Rollback tier is 1 for direct child
|
||||
When I subtree isolate compute rollback tier for "A"
|
||||
Then the subtree isolate rollback tier should be 1
|
||||
|
||||
Scenario: Rollback tier is 1 for second direct child
|
||||
When I subtree isolate compute rollback tier for "B"
|
||||
Then the subtree isolate rollback tier should be 1
|
||||
|
||||
Scenario: Rollback tier is 2 for grandchild
|
||||
When I subtree isolate compute rollback tier for "C"
|
||||
Then the subtree isolate rollback tier should be 2
|
||||
|
||||
Scenario: Rollback tier depth 3 for deep tree
|
||||
Given a subtree isolation deep tree with depth 4
|
||||
When I subtree isolate compute rollback tier for "depth_3"
|
||||
Then the subtree isolate rollback tier should be 3
|
||||
|
||||
# --- Dry-run report enhancements ---
|
||||
|
||||
Scenario: Dry-run report includes excluded decisions and rollback tier
|
||||
When I subtree isolate target decision "A" with mode "revert"
|
||||
And I subtree isolate generate a dry-run report
|
||||
Then the subtree isolate dry-run report should include excluded decisions
|
||||
And the subtree isolate dry-run report rollback tier depth should be 1
|
||||
And the subtree isolate dry-run report excluded decisions should contain "root"
|
||||
And the subtree isolate dry-run report excluded decisions should contain "B"
|
||||
And the subtree isolate dry-run report excluded decisions should contain "E"
|
||||
|
||||
# --- Subtree isolation validation ---
|
||||
|
||||
Scenario: Subtree isolation is valid for non-root target
|
||||
When I subtree isolate validate isolation for target "A"
|
||||
Then the subtree isolate isolation should be valid
|
||||
|
||||
Scenario: Subtree isolation is valid for leaf target
|
||||
When I subtree isolate validate isolation for target "D"
|
||||
Then the subtree isolate isolation should be valid
|
||||
|
||||
# --- Edge cases for coverage ---
|
||||
|
||||
Scenario: Subtree isolation with empty tree returns valid
|
||||
Given a subtree isolation empty tree
|
||||
When I subtree isolate validate isolation for target "X"
|
||||
Then the subtree isolate isolation should be valid
|
||||
|
||||
Scenario: Rollback tier for unknown node is 0
|
||||
When I subtree isolate compute rollback tier for "UNKNOWN"
|
||||
Then the subtree isolate rollback tier should be 0
|
||||
|
||||
Scenario: Find parent returns None for root node
|
||||
When I subtree isolate check parent of "root"
|
||||
Then the subtree isolate parent should be none
|
||||
|
||||
# --- Influence DAG traversal ---
|
||||
|
||||
Scenario: Influence edge adds extra decision to affected set
|
||||
Given a subtree isolate influence edge from "D" to "E"
|
||||
When I subtree isolate target decision "D" with mode "revert"
|
||||
And I subtree isolate analyze the impact with influence edges
|
||||
Then the subtree isolate affected decisions should contain "D"
|
||||
And the subtree isolate affected decisions should contain "E"
|
||||
And the subtree isolate excluded decisions should contain "root"
|
||||
And the subtree isolate excluded decisions should contain "A"
|
||||
And the subtree isolate excluded decisions should contain "B"
|
||||
And the subtree isolate excluded decisions should contain "C"
|
||||
|
||||
Scenario: Influence DAG cycle does not cause infinite loop
|
||||
Given a subtree isolate influence cycle from "C" to "D" back to "C"
|
||||
When I subtree isolate target decision "C" with mode "revert"
|
||||
And I subtree isolate analyze the impact with influence edges
|
||||
Then the subtree isolate affected decisions should contain "C"
|
||||
And the subtree isolate affected decisions should contain "D"
|
||||
|
||||
Scenario: Isolation validation passes when influence DAG reaches sibling
|
||||
Given a subtree isolate influence edge from "D" to "E"
|
||||
When I subtree isolate validate isolation for target "D" with influence edges
|
||||
Then the subtree isolate isolation should be valid
|
||||
|
||||
# --- Append mode ---
|
||||
|
||||
Scenario: Append correction spawns child plan
|
||||
When I subtree isolate target decision "A" with mode "append"
|
||||
And I subtree isolate execute append correction
|
||||
Then the subtree isolate append result should have a spawned child plan
|
||||
And the subtree isolate append result status should be "applied"
|
||||
|
||||
# --- Dry-run enforcement ---
|
||||
|
||||
Scenario: Executing a dry-run revert correction raises ValidationError
|
||||
When I subtree isolate target decision "A" with mode "revert" and dry_run
|
||||
Then subtree isolate executing the correction should raise a dry-run error
|
||||
|
||||
Scenario: Executing a dry-run append correction raises ValidationError
|
||||
When I subtree isolate target decision "A" with mode "append" and dry_run
|
||||
Then subtree isolate executing the correction should raise a dry-run error
|
||||
|
||||
# --- Negative isolation validation ---
|
||||
|
||||
Scenario: Isolation validation detects structural root contamination
|
||||
Given a subtree isolation tree where child structurally reaches root
|
||||
| parent | children |
|
||||
| root | A |
|
||||
| A | root |
|
||||
When I subtree isolate validate isolation for target "A"
|
||||
Then the subtree isolate isolation should be invalid
|
||||
|
||||
# --- Execute revert (non-dry-run) ---
|
||||
|
||||
Scenario: Execute revert correction succeeds for middle node
|
||||
When I subtree isolate target decision "A" with mode "revert"
|
||||
And I subtree isolate execute revert correction
|
||||
Then the subtree isolate revert result status should be "applied"
|
||||
And the subtree isolate revert result should contain reverted decisions "A,C,D"
|
||||
And the subtree isolate revert result should have archived artifacts
|
||||
|
||||
# --- Status guard enforcement ---
|
||||
|
||||
Scenario: Executing an already-applied correction raises ValidationError
|
||||
When I subtree isolate target decision "A" with mode "revert"
|
||||
And I subtree isolate execute revert correction
|
||||
Then subtree isolate executing the correction again should raise a status error
|
||||
|
||||
Scenario: Executing a cancelled correction raises ValidationError
|
||||
When I subtree isolate target decision "A" with mode "revert"
|
||||
And I subtree isolate cancel the correction
|
||||
Then subtree isolate executing the correction should raise a status error
|
||||
|
||||
# --- Mode mismatch enforcement ---
|
||||
|
||||
Scenario: Calling execute_revert on an append correction raises ValidationError
|
||||
When I subtree isolate target decision "A" with mode "append"
|
||||
Then subtree isolate calling execute_revert should raise a mode error
|
||||
|
||||
Scenario: Calling execute_append on a revert correction raises ValidationError
|
||||
When I subtree isolate target decision "A" with mode "revert"
|
||||
Then subtree isolate calling execute_append should raise a mode error
|
||||
|
||||
# --- Single-node tree edge case ---
|
||||
|
||||
Scenario: Single-node tree has no excluded decisions
|
||||
Given a subtree isolation single node tree
|
||||
When I subtree isolate target decision "root" with mode "revert"
|
||||
And I subtree isolate analyze the impact
|
||||
Then the subtree isolate affected decisions should be "root"
|
||||
And the subtree isolate excluded decisions should be empty
|
||||
And the subtree isolate rollback tier depth should be 0
|
||||
|
||||
# --- Terminal state guard ---
|
||||
|
||||
Scenario: Re-analyzing an applied correction raises ValidationError
|
||||
When I subtree isolate target decision "A" with mode "revert"
|
||||
And I subtree isolate execute revert correction
|
||||
Then subtree isolate re-analyzing the impact should raise a terminal state error
|
||||
|
||||
# --- Exact-match affected set for middle node ---
|
||||
|
||||
Scenario: Middle node affected count is exactly 3
|
||||
When I subtree isolate target decision "A" with mode "revert"
|
||||
And I subtree isolate analyze the impact
|
||||
Then the subtree isolate affected decisions count should be 3
|
||||
|
||||
# --- Convergent (diamond) DAG topology ---
|
||||
|
||||
Scenario: Convergent diamond topology does not duplicate affected nodes
|
||||
Given a subtree isolate diamond influence where "C" and "D" both target "E"
|
||||
When I subtree isolate target decision "A" with mode "revert"
|
||||
And I subtree isolate analyze the impact with influence edges
|
||||
Then the subtree isolate affected decisions should contain "C"
|
||||
And the subtree isolate affected decisions should contain "D"
|
||||
And the subtree isolate affected decisions should contain "E"
|
||||
And the subtree isolate affected decision "E" should appear exactly once
|
||||
|
||||
# --- Dry-run exception recovery ---
|
||||
|
||||
Scenario: Dry-run report restores status when analyze_impact fails
|
||||
When I subtree isolate target decision "A" with mode "revert"
|
||||
And I subtree isolate generate a dry-run report that fails internally
|
||||
Then the subtree isolate correction status should be "pending"
|
||||
|
||||
# --- Execute revert with influence edges ---
|
||||
|
||||
Scenario: Execute revert with influence edges includes DAG-reachable decisions
|
||||
Given a subtree isolate influence edge from "D" to "E"
|
||||
When I subtree isolate target decision "A" with mode "revert"
|
||||
And I subtree isolate execute revert correction with influence edges
|
||||
Then the subtree isolate revert result status should be "applied"
|
||||
And the subtree isolate revert result should contain reverted decision "E"
|
||||
|
||||
# --- DAG-only nodes in excluded set ---
|
||||
|
||||
Scenario: Nodes only in influence DAG appear in excluded set
|
||||
Given a subtree isolate influence edge from "X_EXT" to "Y_EXT" only in DAG
|
||||
When I subtree isolate target decision "D" with mode "revert"
|
||||
And I subtree isolate analyze the impact with influence edges
|
||||
Then the subtree isolate excluded decisions should contain "X_EXT"
|
||||
And the subtree isolate excluded decisions should contain "Y_EXT"
|
||||
@@ -273,14 +273,3 @@ Feature: Deferred Physical Resource Types
|
||||
Scenario: as_cli_dict works for trigger-based auto_discovery for phys_deferred
|
||||
Given the built-in "devcontainer-instance" type from registry for phys_deferred
|
||||
Then as_cli_dict should succeed and include auto_discovery for phys_deferred
|
||||
|
||||
# ── as_cli_dict regression: both auto_discovery schemas ──
|
||||
|
||||
Scenario: as_cli_dict works for rules-based auto_discovery for phys_deferred
|
||||
Given the built-in "git" YAML for phys_deferred
|
||||
When I load the YAML via from_config for phys_deferred
|
||||
Then as_cli_dict should succeed and include auto_discovery for phys_deferred
|
||||
|
||||
Scenario: as_cli_dict works for trigger-based auto_discovery for phys_deferred
|
||||
Given the built-in "devcontainer-instance" type from registry for phys_deferred
|
||||
Then as_cli_dict should succeed and include auto_discovery for phys_deferred
|
||||
|
||||
@@ -0,0 +1,678 @@
|
||||
"""Step definitions for correction subtree isolation tests.
|
||||
|
||||
All step names are prefixed with ``subtree isolat`` to avoid
|
||||
``AmbiguousStep`` conflicts with existing correction steps.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import patch
|
||||
|
||||
from behave import given, then, when
|
||||
from behave.runner import Context
|
||||
|
||||
from cleveragents.application.services.correction_service import CorrectionService
|
||||
from cleveragents.core.exceptions import ValidationError
|
||||
from cleveragents.domain.models.core.correction import CorrectionMode, CorrectionStatus
|
||||
|
||||
_PLAN_ID = "01SUBTREEPLAN000000000000001"
|
||||
|
||||
|
||||
def _parse_children(text: str) -> list[str]:
|
||||
"""Split a comma-separated children string into a list."""
|
||||
return [c.strip() for c in text.split(",") if c.strip()]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Background
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@given("a subtree isolation correction service")
|
||||
def step_subtree_service(context: Context) -> None:
|
||||
"""Set up a fresh CorrectionService for subtree tests."""
|
||||
context.subtree_svc = CorrectionService()
|
||||
context.subtree_tree = {}
|
||||
context.subtree_impact = None
|
||||
context.subtree_report = None
|
||||
context.subtree_tier = None
|
||||
context.subtree_isolation_valid = None
|
||||
|
||||
|
||||
@given('a subtree isolation decision tree with root "{root}" and children')
|
||||
def step_subtree_tree(context: Context, root: str) -> None:
|
||||
"""Build the decision tree from the table."""
|
||||
tree: dict[str, list[str]] = {}
|
||||
for row in context.table:
|
||||
parent = row["parent"]
|
||||
children = _parse_children(row["children"])
|
||||
tree[parent] = children
|
||||
context.subtree_tree = tree
|
||||
|
||||
|
||||
@given("a subtree isolation deep tree with depth {depth:d}")
|
||||
def step_subtree_deep_tree(context: Context, depth: int) -> None:
|
||||
"""Build a linear chain tree with given depth."""
|
||||
tree: dict[str, list[str]] = {}
|
||||
for i in range(depth):
|
||||
parent = f"depth_{i}" if i > 0 else "root"
|
||||
child = f"depth_{i + 1}"
|
||||
tree[parent] = [child]
|
||||
context.subtree_tree = tree
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# When steps
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@when('I subtree isolate target decision "{decision_id}" with mode "{mode}"')
|
||||
def step_subtree_target(context: Context, decision_id: str, mode: str) -> None:
|
||||
"""Create a correction request targeting the given decision."""
|
||||
svc: CorrectionService = context.subtree_svc
|
||||
correction_mode = CorrectionMode(mode)
|
||||
req = svc.request_correction(
|
||||
plan_id=_PLAN_ID,
|
||||
target_decision_id=decision_id,
|
||||
mode=correction_mode,
|
||||
)
|
||||
context.subtree_correction_id = req.correction_id
|
||||
|
||||
|
||||
@when("I subtree isolate analyze the impact")
|
||||
def step_subtree_analyze(context: Context) -> None:
|
||||
"""Analyze impact for the current correction."""
|
||||
svc: CorrectionService = context.subtree_svc
|
||||
impact = svc.analyze_impact(
|
||||
context.subtree_correction_id,
|
||||
context.subtree_tree,
|
||||
)
|
||||
context.subtree_impact = impact
|
||||
|
||||
|
||||
@when("I subtree isolate generate a dry-run report")
|
||||
def step_subtree_dry_run(context: Context) -> None:
|
||||
"""Generate a dry-run report for the current correction."""
|
||||
svc: CorrectionService = context.subtree_svc
|
||||
report = svc.generate_dry_run_report(
|
||||
context.subtree_correction_id,
|
||||
context.subtree_tree,
|
||||
)
|
||||
context.subtree_report = report
|
||||
context.subtree_impact = report.impact
|
||||
|
||||
|
||||
@when('I subtree isolate compute rollback tier for "{decision_id}"')
|
||||
def step_subtree_compute_tier(context: Context, decision_id: str) -> None:
|
||||
"""Compute the rollback tier for a decision."""
|
||||
svc: CorrectionService = context.subtree_svc
|
||||
context.subtree_tier = svc.compute_rollback_tier(
|
||||
decision_id, _PLAN_ID, context.subtree_tree
|
||||
)
|
||||
|
||||
|
||||
@when('I subtree isolate validate isolation for target "{decision_id}"')
|
||||
def step_subtree_validate(context: Context, decision_id: str) -> None:
|
||||
"""Validate subtree isolation for a target decision."""
|
||||
svc: CorrectionService = context.subtree_svc
|
||||
context.subtree_isolation_valid = svc.validate_subtree_isolation(
|
||||
decision_id, context.subtree_tree
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Then steps: affected / excluded decisions
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@then('the subtree isolate affected decisions should be "{expected}"')
|
||||
def step_subtree_affected_exact(context: Context, expected: str) -> None:
|
||||
"""Check that affected decisions match exactly."""
|
||||
impact = context.subtree_impact
|
||||
expected_list = _parse_children(expected)
|
||||
assert impact.affected_decisions == expected_list, (
|
||||
f"Expected affected={expected_list}, got {impact.affected_decisions}"
|
||||
)
|
||||
|
||||
|
||||
@then('the subtree isolate affected decisions should contain "{decision_id}"')
|
||||
def step_subtree_affected_contains(context: Context, decision_id: str) -> None:
|
||||
"""Check that a specific decision is in the affected set."""
|
||||
impact = context.subtree_impact
|
||||
assert decision_id in impact.affected_decisions, (
|
||||
f"Expected '{decision_id}' in affected={impact.affected_decisions}"
|
||||
)
|
||||
|
||||
|
||||
@then('the subtree isolate excluded decisions should contain "{decision_id}"')
|
||||
def step_subtree_excluded_contains(context: Context, decision_id: str) -> None:
|
||||
"""Check that a specific decision is in the excluded set."""
|
||||
impact = context.subtree_impact
|
||||
assert decision_id in impact.excluded_decisions, (
|
||||
f"Expected '{decision_id}' in excluded={impact.excluded_decisions}"
|
||||
)
|
||||
|
||||
|
||||
@then("the subtree isolate excluded decisions should be empty")
|
||||
def step_subtree_excluded_empty(context: Context) -> None:
|
||||
"""Check that excluded decisions is empty."""
|
||||
impact = context.subtree_impact
|
||||
assert len(impact.excluded_decisions) == 0, (
|
||||
f"Expected empty excluded, got {impact.excluded_decisions}"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Then steps: rollback tier
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@then("the subtree isolate rollback tier depth should be {expected:d}")
|
||||
def step_subtree_tier_depth(context: Context, expected: int) -> None:
|
||||
"""Check the rollback tier depth from impact analysis."""
|
||||
impact = context.subtree_impact
|
||||
assert impact.rollback_tier_depth == expected, (
|
||||
f"Expected tier_depth={expected}, got {impact.rollback_tier_depth}"
|
||||
)
|
||||
|
||||
|
||||
@then("the subtree isolate rollback tier should be {expected:d}")
|
||||
def step_subtree_tier_value(context: Context, expected: int) -> None:
|
||||
"""Check the rollback tier from direct computation."""
|
||||
assert context.subtree_tier == expected, (
|
||||
f"Expected tier={expected}, got {context.subtree_tier}"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Then steps: dry-run report
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@then('the subtree isolate dry-run warnings should contain "{text}"')
|
||||
def step_subtree_warnings_contain(context: Context, text: str) -> None:
|
||||
"""Check that dry-run warnings contain specific text."""
|
||||
report = context.subtree_report
|
||||
all_warnings = " ".join(report.warnings)
|
||||
assert text in all_warnings, f"Expected '{text}' in warnings: {report.warnings}"
|
||||
|
||||
|
||||
@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."""
|
||||
report = context.subtree_report
|
||||
assert len(report.excluded_decisions) > 0, (
|
||||
f"Expected non-empty excluded_decisions, got {report.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."""
|
||||
report = context.subtree_report
|
||||
assert report.rollback_tier_depth == expected, (
|
||||
f"Expected report tier_depth={expected}, got {report.rollback_tier_depth}"
|
||||
)
|
||||
|
||||
|
||||
@then(
|
||||
'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."""
|
||||
report = context.subtree_report
|
||||
assert decision_id in report.excluded_decisions, (
|
||||
f"Expected '{decision_id}' in report excluded={report.excluded_decisions}"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Then steps: subtree isolation validation
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@then("the subtree isolate isolation should be valid")
|
||||
def step_subtree_isolation_valid(context: Context) -> None:
|
||||
"""Check that subtree isolation validation passed."""
|
||||
assert context.subtree_isolation_valid is True, (
|
||||
"Expected isolation to be valid, but it was not"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Edge-case steps for additional coverage
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@given("a subtree isolation empty tree")
|
||||
def step_subtree_empty_tree(context: Context) -> None:
|
||||
"""Set the decision tree to empty for edge-case testing."""
|
||||
context.subtree_tree = {}
|
||||
|
||||
|
||||
@when('I subtree isolate check parent of "{decision_id}"')
|
||||
def step_subtree_check_parent(context: Context, decision_id: str) -> None:
|
||||
"""Check the parent of a decision in the tree."""
|
||||
parent = CorrectionService._find_parent(decision_id, context.subtree_tree)
|
||||
context.subtree_parent_result = parent
|
||||
|
||||
|
||||
@then("the subtree isolate parent should be none")
|
||||
def step_subtree_parent_none(context: Context) -> None:
|
||||
"""Verify that the parent lookup returned None."""
|
||||
assert context.subtree_parent_result is None, (
|
||||
f"Expected None, got {context.subtree_parent_result}"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Influence DAG steps
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@given('a subtree isolate influence edge from "{src}" to "{dst}"')
|
||||
def step_subtree_influence_edge(context: Context, src: str, dst: str) -> None:
|
||||
"""Set up a single influence edge."""
|
||||
context.subtree_influence_edges = {src: [dst]}
|
||||
|
||||
|
||||
@given('a subtree isolate influence cycle from "{src}" to "{mid}" back to "{back}"')
|
||||
def step_subtree_influence_cycle(
|
||||
context: Context,
|
||||
src: str,
|
||||
mid: str,
|
||||
back: str,
|
||||
) -> None:
|
||||
"""Set up cyclic influence edges."""
|
||||
context.subtree_influence_edges = {src: [mid], mid: [back]}
|
||||
|
||||
|
||||
@when("I subtree isolate analyze the impact with influence edges")
|
||||
def step_subtree_analyze_with_influence(context: Context) -> None:
|
||||
"""Analyze impact using both tree and influence edges."""
|
||||
svc: CorrectionService = context.subtree_svc
|
||||
impact = svc.analyze_impact(
|
||||
context.subtree_correction_id,
|
||||
context.subtree_tree,
|
||||
influence_edges=context.subtree_influence_edges,
|
||||
)
|
||||
context.subtree_impact = impact
|
||||
|
||||
|
||||
@when(
|
||||
'I subtree isolate validate isolation for target "{decision_id}"'
|
||||
" with influence edges"
|
||||
)
|
||||
def step_subtree_validate_with_influence(
|
||||
context: Context,
|
||||
decision_id: str,
|
||||
) -> None:
|
||||
"""Validate subtree isolation with influence edges."""
|
||||
svc: CorrectionService = context.subtree_svc
|
||||
context.subtree_isolation_valid = svc.validate_subtree_isolation(
|
||||
decision_id,
|
||||
context.subtree_tree,
|
||||
influence_edges=context.subtree_influence_edges,
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Append mode steps
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@when("I subtree isolate execute append correction")
|
||||
def step_subtree_execute_append(context: Context) -> None:
|
||||
"""Execute an append correction."""
|
||||
svc: CorrectionService = context.subtree_svc
|
||||
result = svc.execute_append(context.subtree_correction_id)
|
||||
context.subtree_append_result = result
|
||||
|
||||
|
||||
@then("the subtree isolate append result should have a spawned child plan")
|
||||
def step_subtree_append_has_child_plan(context: Context) -> None:
|
||||
"""Check that the append result has a spawned child plan."""
|
||||
result = context.subtree_append_result
|
||||
assert result.spawned_child_plan_id is not None, (
|
||||
"Expected spawned_child_plan_id, got None"
|
||||
)
|
||||
|
||||
|
||||
@then('the subtree isolate append result status should be "{status}"')
|
||||
def step_subtree_append_status(context: Context, status: str) -> None:
|
||||
"""Check the append result status."""
|
||||
result = context.subtree_append_result
|
||||
assert result.status.value == status, (
|
||||
f"Expected status='{status}', got '{result.status.value}'"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Dry-run enforcement steps
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@when(
|
||||
'I subtree isolate target decision "{decision_id}" with mode "{mode}" and dry_run'
|
||||
)
|
||||
def step_subtree_target_dry_run(
|
||||
context: Context,
|
||||
decision_id: str,
|
||||
mode: str,
|
||||
) -> None:
|
||||
"""Create a dry-run correction request."""
|
||||
svc: CorrectionService = context.subtree_svc
|
||||
correction_mode = CorrectionMode(mode)
|
||||
req = svc.request_correction(
|
||||
plan_id=_PLAN_ID,
|
||||
target_decision_id=decision_id,
|
||||
mode=correction_mode,
|
||||
dry_run=True,
|
||||
)
|
||||
context.subtree_correction_id = req.correction_id
|
||||
|
||||
|
||||
@then("subtree isolate executing the correction should raise a dry-run error")
|
||||
def step_subtree_execute_raises_dry_run(context: Context) -> None:
|
||||
"""Verify that executing a dry-run correction raises ValidationError."""
|
||||
svc: CorrectionService = context.subtree_svc
|
||||
raised = False
|
||||
try:
|
||||
svc.execute_correction(context.subtree_correction_id, context.subtree_tree)
|
||||
except ValidationError as exc:
|
||||
raised = True
|
||||
assert "dry-run" in str(exc).lower(), (
|
||||
f"Expected 'dry-run' in error message, got: {exc}"
|
||||
)
|
||||
assert raised, "Expected ValidationError for dry-run execution, but none was raised"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Negative isolation validation steps
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@given("a subtree isolation tree where child structurally reaches root")
|
||||
def step_subtree_cyclic_tree(context: Context) -> None:
|
||||
"""Build a tree where a child has the root as structural child."""
|
||||
tree: dict[str, list[str]] = {}
|
||||
for row in context.table:
|
||||
parent = row["parent"]
|
||||
children = _parse_children(row["children"])
|
||||
tree[parent] = children
|
||||
context.subtree_tree = tree
|
||||
|
||||
|
||||
@then("the subtree isolate isolation should be invalid")
|
||||
def step_subtree_isolation_invalid(context: Context) -> None:
|
||||
"""Check that subtree isolation validation failed."""
|
||||
assert context.subtree_isolation_valid is False, (
|
||||
"Expected isolation to be invalid, but it was valid"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Execute revert (non-dry-run) steps
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@when("I subtree isolate execute revert correction")
|
||||
def step_subtree_execute_revert(context: Context) -> None:
|
||||
"""Execute a revert correction."""
|
||||
svc: CorrectionService = context.subtree_svc
|
||||
result = svc.execute_revert(
|
||||
context.subtree_correction_id,
|
||||
context.subtree_tree,
|
||||
)
|
||||
context.subtree_revert_result = result
|
||||
|
||||
|
||||
@then('the subtree isolate revert result status should be "{status}"')
|
||||
def step_subtree_revert_status(context: Context, status: str) -> None:
|
||||
"""Check the revert result status."""
|
||||
result = context.subtree_revert_result
|
||||
assert result.status.value == status, (
|
||||
f"Expected status='{status}', got '{result.status.value}'"
|
||||
)
|
||||
|
||||
|
||||
@then(
|
||||
'the subtree isolate revert result should contain reverted decisions "{expected}"'
|
||||
)
|
||||
def step_subtree_revert_decisions(context: Context, expected: str) -> None:
|
||||
"""Check the reverted decisions list."""
|
||||
result = context.subtree_revert_result
|
||||
expected_list = sorted(_parse_children(expected))
|
||||
actual_list = sorted(result.reverted_decisions)
|
||||
assert actual_list == expected_list, (
|
||||
f"Expected reverted={expected_list}, got {actual_list}"
|
||||
)
|
||||
|
||||
|
||||
@then("the subtree isolate revert result should have archived artifacts")
|
||||
def step_subtree_revert_has_artifacts(context: Context) -> None:
|
||||
"""Check that archived artifacts is non-empty."""
|
||||
result = context.subtree_revert_result
|
||||
assert len(result.archived_artifacts) > 0, (
|
||||
f"Expected non-empty archived_artifacts, got {result.archived_artifacts}"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Status guard enforcement steps
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@then("subtree isolate executing the correction again should raise a status error")
|
||||
def step_subtree_execute_again_raises(context: Context) -> None:
|
||||
"""Verify re-executing an applied correction raises ValidationError."""
|
||||
svc: CorrectionService = context.subtree_svc
|
||||
raised = False
|
||||
try:
|
||||
svc.execute_correction(context.subtree_correction_id, context.subtree_tree)
|
||||
except ValidationError:
|
||||
raised = True
|
||||
assert raised, "Expected ValidationError when re-executing an applied correction"
|
||||
|
||||
|
||||
@when("I subtree isolate cancel the correction")
|
||||
def step_subtree_cancel(context: Context) -> None:
|
||||
"""Cancel the current correction."""
|
||||
svc: CorrectionService = context.subtree_svc
|
||||
svc.cancel_correction(context.subtree_correction_id)
|
||||
|
||||
|
||||
@then("subtree isolate executing the correction should raise a status error")
|
||||
def step_subtree_execute_cancelled_raises(context: Context) -> None:
|
||||
"""Verify executing a cancelled correction raises ValidationError."""
|
||||
svc: CorrectionService = context.subtree_svc
|
||||
raised = False
|
||||
try:
|
||||
svc.execute_correction(context.subtree_correction_id, context.subtree_tree)
|
||||
except ValidationError:
|
||||
raised = True
|
||||
assert raised, "Expected ValidationError when executing a cancelled correction"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Mode mismatch enforcement steps
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@then("subtree isolate calling execute_revert should raise a mode error")
|
||||
def step_subtree_revert_on_append_raises(context: Context) -> None:
|
||||
"""Verify calling execute_revert on an append correction raises."""
|
||||
svc: CorrectionService = context.subtree_svc
|
||||
raised = False
|
||||
try:
|
||||
svc.execute_revert(context.subtree_correction_id, context.subtree_tree)
|
||||
except ValidationError as exc:
|
||||
raised = True
|
||||
assert "REVERT" in str(exc), f"Expected 'REVERT' in error, got: {exc}"
|
||||
assert raised, "Expected ValidationError for mode mismatch"
|
||||
|
||||
|
||||
@then("subtree isolate calling execute_append should raise a mode error")
|
||||
def step_subtree_append_on_revert_raises(context: Context) -> None:
|
||||
"""Verify calling execute_append on a revert correction raises."""
|
||||
svc: CorrectionService = context.subtree_svc
|
||||
raised = False
|
||||
try:
|
||||
svc.execute_append(context.subtree_correction_id)
|
||||
except ValidationError as exc:
|
||||
raised = True
|
||||
assert "APPEND" in str(exc), f"Expected 'APPEND' in error, got: {exc}"
|
||||
assert raised, "Expected ValidationError for mode mismatch"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Single-node tree steps
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@given("a subtree isolation single node tree")
|
||||
def step_subtree_single_node_tree(context: Context) -> None:
|
||||
"""Set a tree with only a root node and no children."""
|
||||
context.subtree_tree = {"root": []}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Terminal state guard steps
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@then("subtree isolate re-analyzing the impact should raise a terminal state error")
|
||||
def step_subtree_reanalyze_raises(context: Context) -> None:
|
||||
"""Verify re-analyzing an applied correction raises ValidationError."""
|
||||
svc: CorrectionService = context.subtree_svc
|
||||
raised = False
|
||||
try:
|
||||
svc.analyze_impact(context.subtree_correction_id, context.subtree_tree)
|
||||
except ValidationError as exc:
|
||||
raised = True
|
||||
assert "terminal" in str(exc).lower(), (
|
||||
f"Expected 'terminal' in error, got: {exc}"
|
||||
)
|
||||
assert raised, "Expected ValidationError when re-analyzing in terminal state"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Exact-match affected count steps
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@then("the subtree isolate affected decisions count should be {expected:d}")
|
||||
def step_subtree_affected_count(context: Context, expected: int) -> None:
|
||||
"""Check the exact count of affected decisions."""
|
||||
impact = context.subtree_impact
|
||||
assert len(impact.affected_decisions) == expected, (
|
||||
f"Expected {expected} affected decisions, "
|
||||
f"got {len(impact.affected_decisions)}: {impact.affected_decisions}"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Convergent (diamond) DAG topology steps
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@given(
|
||||
'a subtree isolate diamond influence where "{src1}" and "{src2}" both target "{dst}"'
|
||||
)
|
||||
def step_subtree_diamond_influence(
|
||||
context: Context,
|
||||
src1: str,
|
||||
src2: str,
|
||||
dst: str,
|
||||
) -> None:
|
||||
"""Set up a convergent (diamond) influence DAG."""
|
||||
context.subtree_influence_edges = {src1: [dst], src2: [dst]}
|
||||
|
||||
|
||||
@then(
|
||||
'the subtree isolate affected decision "{decision_id}" should appear exactly once'
|
||||
)
|
||||
def step_subtree_affected_appears_once(context: Context, decision_id: str) -> None:
|
||||
"""Verify a decision appears exactly once in the affected list."""
|
||||
impact = context.subtree_impact
|
||||
count = impact.affected_decisions.count(decision_id)
|
||||
assert count == 1, (
|
||||
f"Expected '{decision_id}' exactly once in affected, "
|
||||
f"found {count} times: {impact.affected_decisions}"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Dry-run exception recovery steps
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@when("I subtree isolate generate a dry-run report that fails internally")
|
||||
def step_subtree_dry_run_fails(context: Context) -> None:
|
||||
"""Generate a dry-run report where analyze_impact raises internally."""
|
||||
svc: CorrectionService = context.subtree_svc
|
||||
try:
|
||||
with patch.object(
|
||||
CorrectionService,
|
||||
"_compute_affected_subtree",
|
||||
side_effect=RuntimeError("simulated BFS failure"),
|
||||
):
|
||||
svc.generate_dry_run_report(
|
||||
context.subtree_correction_id,
|
||||
context.subtree_tree,
|
||||
)
|
||||
except RuntimeError:
|
||||
pass # Expected — the exception propagates from analyze_impact
|
||||
|
||||
|
||||
@then('the subtree isolate correction status should be "{expected_status}"')
|
||||
def step_subtree_correction_status(context: Context, expected_status: str) -> None:
|
||||
"""Verify the correction request's current status."""
|
||||
svc: CorrectionService = context.subtree_svc
|
||||
request = svc.get_correction(context.subtree_correction_id)
|
||||
expected = CorrectionStatus(expected_status)
|
||||
assert request.status == expected, (
|
||||
f"Expected status='{expected}', got '{request.status}'"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Execute revert with influence edges steps
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@when("I subtree isolate execute revert correction with influence edges")
|
||||
def step_subtree_execute_revert_with_influence(context: Context) -> None:
|
||||
"""Execute a revert correction with influence edges."""
|
||||
svc: CorrectionService = context.subtree_svc
|
||||
result = svc.execute_revert(
|
||||
context.subtree_correction_id,
|
||||
context.subtree_tree,
|
||||
influence_edges=context.subtree_influence_edges,
|
||||
)
|
||||
context.subtree_revert_result = result
|
||||
|
||||
|
||||
@then(
|
||||
'the subtree isolate revert result should contain reverted decision "{decision_id}"'
|
||||
)
|
||||
def step_subtree_revert_contains_decision(
|
||||
context: Context,
|
||||
decision_id: str,
|
||||
) -> None:
|
||||
"""Check that a specific decision is in the reverted set."""
|
||||
result = context.subtree_revert_result
|
||||
assert decision_id in result.reverted_decisions, (
|
||||
f"Expected '{decision_id}' in reverted={result.reverted_decisions}"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# DAG-only nodes in excluded set steps
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@given('a subtree isolate influence edge from "{src}" to "{dst}" only in DAG')
|
||||
def step_subtree_dag_only_influence(context: Context, src: str, dst: str) -> None:
|
||||
"""Set up influence edges with nodes that are NOT in the structural tree."""
|
||||
context.subtree_influence_edges = {src: [dst]}
|
||||
@@ -0,0 +1,137 @@
|
||||
*** Settings ***
|
||||
Documentation Integration tests for correction subtree isolation.
|
||||
... Verifies that decision corrections recompute only the
|
||||
... affected subtree while excluding root and sibling nodes.
|
||||
Resource ${CURDIR}/common.resource
|
||||
Suite Setup Setup Test Environment
|
||||
Suite Teardown Cleanup Test Environment
|
||||
|
||||
*** Variables ***
|
||||
${HELPER} robot/helper_correction_subtree_isolation.py
|
||||
|
||||
*** Test Cases ***
|
||||
Leaf Node Isolation
|
||||
[Documentation] Target a leaf — only the leaf is affected
|
||||
[Tags] phase2 correction subtree isolation
|
||||
${result}= Run Process ${PYTHON} ${HELPER} leaf-isolation cwd=${WORKSPACE} on_timeout=kill
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} leaf-isolation-ok
|
||||
|
||||
Middle Node Isolation
|
||||
[Documentation] Target a middle node — middle and descendants affected
|
||||
[Tags] phase2 correction subtree isolation
|
||||
${result}= Run Process ${PYTHON} ${HELPER} middle-node-isolation cwd=${WORKSPACE} on_timeout=kill
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} middle-node-isolation-ok
|
||||
|
||||
Root Targets All Decisions
|
||||
[Documentation] Target root — all decisions affected, none excluded
|
||||
[Tags] phase2 correction subtree isolation
|
||||
${result}= Run Process ${PYTHON} ${HELPER} root-targets-all cwd=${WORKSPACE} on_timeout=kill
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} root-targets-all-ok
|
||||
|
||||
Rollback Tier Computation
|
||||
[Documentation] Verify tier depth at depth 0, 1, 2, and 3
|
||||
[Tags] phase2 correction subtree tier
|
||||
${result}= Run Process ${PYTHON} ${HELPER} rollback-tier-computation cwd=${WORKSPACE} on_timeout=kill
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} rollback-tier-computation-ok
|
||||
|
||||
Enhanced Dry Run Report
|
||||
[Documentation] Dry-run report includes excluded decisions and tier
|
||||
[Tags] phase2 correction subtree dryrun
|
||||
${result}= Run Process ${PYTHON} ${HELPER} dry-run-report-enhanced cwd=${WORKSPACE} on_timeout=kill
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} dry-run-report-enhanced-ok
|
||||
|
||||
Dry Run Tier Zero Warning
|
||||
[Documentation] Root targeting triggers tier-0 warning
|
||||
[Tags] phase2 correction subtree dryrun tier
|
||||
${result}= Run Process ${PYTHON} ${HELPER} dry-run-tier-zero-warning cwd=${WORKSPACE} on_timeout=kill
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} dry-run-tier-zero-warning-ok
|
||||
|
||||
Subtree Isolation Validation
|
||||
[Documentation] Isolation validation passes for non-root targets
|
||||
[Tags] phase2 correction subtree validation
|
||||
${result}= Run Process ${PYTHON} ${HELPER} subtree-isolation-validation cwd=${WORKSPACE} on_timeout=kill
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} subtree-isolation-validation-ok
|
||||
|
||||
Edge Case Empty Tree
|
||||
[Documentation] Edge cases: empty tree, unknown node, root parent
|
||||
[Tags] phase2 correction subtree edge
|
||||
${result}= Run Process ${PYTHON} ${HELPER} edge-case-empty-tree cwd=${WORKSPACE} on_timeout=kill
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} edge-case-empty-tree-ok
|
||||
|
||||
Influence DAG Traversal
|
||||
[Documentation] Influence edges pull additional decisions into affected set
|
||||
[Tags] phase2 correction subtree influence
|
||||
${result}= Run Process ${PYTHON} ${HELPER} influence-dag-traversal cwd=${WORKSPACE} on_timeout=kill
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} influence-dag-traversal-ok
|
||||
|
||||
Append Mode Correction
|
||||
[Documentation] Append mode spawns a child plan without impact analysis
|
||||
[Tags] phase2 correction subtree append
|
||||
${result}= Run Process ${PYTHON} ${HELPER} append-mode-correction cwd=${WORKSPACE} on_timeout=kill
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} append-mode-correction-ok
|
||||
|
||||
Dry Run Enforcement
|
||||
[Documentation] Dry-run corrections cannot be executed
|
||||
[Tags] phase2 correction subtree dryrun enforcement
|
||||
${result}= Run Process ${PYTHON} ${HELPER} dry-run-enforcement cwd=${WORKSPACE} on_timeout=kill
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} dry-run-enforcement-ok
|
||||
|
||||
Execute Revert Non Dry Run
|
||||
[Documentation] Revert correction executes end-to-end with correct results
|
||||
[Tags] phase2 correction subtree revert
|
||||
${result}= Run Process ${PYTHON} ${HELPER} execute-revert-non-dry-run cwd=${WORKSPACE} on_timeout=kill
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} execute-revert-non-dry-run-ok
|
||||
|
||||
Status Guard Enforcement
|
||||
[Documentation] Applied and cancelled corrections cannot be re-executed
|
||||
[Tags] phase2 correction subtree guard
|
||||
${result}= Run Process ${PYTHON} ${HELPER} status-guard-enforcement cwd=${WORKSPACE} on_timeout=kill
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} status-guard-enforcement-ok
|
||||
|
||||
Mode Mismatch Enforcement
|
||||
[Documentation] execute_revert on append and execute_append on revert both raise
|
||||
[Tags] phase2 correction subtree guard
|
||||
${result}= Run Process ${PYTHON} ${HELPER} mode-mismatch-enforcement cwd=${WORKSPACE} on_timeout=kill
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} mode-mismatch-enforcement-ok
|
||||
|
||||
Diamond DAG Topology
|
||||
[Documentation] Convergent (diamond) DAG topology does not cause duplicate visits
|
||||
[Tags] phase2 correction subtree influence diamond
|
||||
${result}= Run Process ${PYTHON} ${HELPER} diamond-dag-topology cwd=${WORKSPACE} on_timeout=kill
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} diamond-dag-topology-ok
|
||||
|
||||
Dry Run Exception Recovery
|
||||
[Documentation] Dry-run report restores status even when analyze_impact fails
|
||||
[Tags] phase2 correction subtree dryrun recovery
|
||||
${result}= Run Process ${PYTHON} ${HELPER} dry-run-exception-recovery cwd=${WORKSPACE} on_timeout=kill
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} dry-run-exception-recovery-ok
|
||||
|
||||
Execute Revert With Influence Edges
|
||||
[Documentation] Revert correction works correctly with influence DAG edges
|
||||
[Tags] phase2 correction subtree revert influence
|
||||
${result}= Run Process ${PYTHON} ${HELPER} execute-revert-with-influence cwd=${WORKSPACE} on_timeout=kill
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} execute-revert-with-influence-ok
|
||||
|
||||
DAG Only Nodes In Excluded Set
|
||||
[Documentation] Nodes appearing only in the influence DAG are in the excluded set
|
||||
[Tags] phase2 correction subtree influence excluded
|
||||
${result}= Run Process ${PYTHON} ${HELPER} dag-only-nodes-excluded cwd=${WORKSPACE} on_timeout=kill
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} dag-only-nodes-excluded-ok
|
||||
@@ -0,0 +1,457 @@
|
||||
"""Helper script for correction subtree isolation Robot Framework tests.
|
||||
|
||||
Validates that CorrectionService correctly isolates correction scope:
|
||||
- excluded_decisions populated after impact analysis
|
||||
- rollback_tier_depth computation
|
||||
- dry-run report enhancements
|
||||
- subtree isolation validation
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import sys
|
||||
from collections.abc import Callable
|
||||
from unittest.mock import patch
|
||||
|
||||
from cleveragents.application.services.correction_service import CorrectionService
|
||||
from cleveragents.core.exceptions import ValidationError
|
||||
from cleveragents.domain.models.core.correction import CorrectionMode, CorrectionStatus
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Shared fixtures
|
||||
# ---------------------------------------------------------------------------
|
||||
_PLAN_ID = "01ROBOTSUBTREEPLAN0000000001"
|
||||
|
||||
|
||||
def _sample_tree() -> dict[str, list[str]]:
|
||||
return {
|
||||
"root": ["A", "B"],
|
||||
"A": ["C", "D"],
|
||||
"B": ["E"],
|
||||
}
|
||||
|
||||
|
||||
def _deep_tree(depth: int) -> dict[str, list[str]]:
|
||||
tree: dict[str, list[str]] = {}
|
||||
for i in range(depth):
|
||||
parent = f"depth_{i}" if i > 0 else "root"
|
||||
child = f"depth_{i + 1}"
|
||||
tree[parent] = [child]
|
||||
return tree
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Command handlers
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _leaf_isolation() -> None:
|
||||
"""Target a leaf node and verify only it is affected."""
|
||||
svc = CorrectionService()
|
||||
tree = _sample_tree()
|
||||
req = svc.request_correction(_PLAN_ID, "D", CorrectionMode.REVERT)
|
||||
impact = svc.analyze_impact(req.correction_id, tree)
|
||||
|
||||
assert impact.affected_decisions == ["D"], (
|
||||
f"Expected ['D'], got {impact.affected_decisions}"
|
||||
)
|
||||
for excluded in ["root", "A", "B", "C", "E"]:
|
||||
assert excluded in impact.excluded_decisions, (
|
||||
f"Expected '{excluded}' in excluded={impact.excluded_decisions}"
|
||||
)
|
||||
print("leaf-isolation-ok")
|
||||
|
||||
|
||||
def _middle_node_isolation() -> None:
|
||||
"""Target a middle node and verify subtree is affected."""
|
||||
svc = CorrectionService()
|
||||
tree = _sample_tree()
|
||||
req = svc.request_correction(_PLAN_ID, "A", CorrectionMode.REVERT)
|
||||
impact = svc.analyze_impact(req.correction_id, tree)
|
||||
|
||||
for affected in ["A", "C", "D"]:
|
||||
assert affected in impact.affected_decisions, (
|
||||
f"Expected '{affected}' in affected={impact.affected_decisions}"
|
||||
)
|
||||
for excluded in ["root", "B", "E"]:
|
||||
assert excluded in impact.excluded_decisions, (
|
||||
f"Expected '{excluded}' in excluded={impact.excluded_decisions}"
|
||||
)
|
||||
print("middle-node-isolation-ok")
|
||||
|
||||
|
||||
def _root_targets_all() -> None:
|
||||
"""Target root and verify all decisions are affected."""
|
||||
svc = CorrectionService()
|
||||
tree = _sample_tree()
|
||||
req = svc.request_correction(_PLAN_ID, "root", CorrectionMode.REVERT)
|
||||
impact = svc.analyze_impact(req.correction_id, tree)
|
||||
|
||||
for node in ["root", "A", "B", "C", "D", "E"]:
|
||||
assert node in impact.affected_decisions, (
|
||||
f"Expected '{node}' in affected={impact.affected_decisions}"
|
||||
)
|
||||
assert len(impact.excluded_decisions) == 0, (
|
||||
f"Expected empty excluded, got {impact.excluded_decisions}"
|
||||
)
|
||||
assert impact.rollback_tier_depth == 0
|
||||
print("root-targets-all-ok")
|
||||
|
||||
|
||||
def _rollback_tier_computation() -> None:
|
||||
"""Verify rollback tier at various depths."""
|
||||
svc = CorrectionService()
|
||||
tree = _sample_tree()
|
||||
|
||||
assert svc.compute_rollback_tier("root", _PLAN_ID, tree) == 0
|
||||
assert svc.compute_rollback_tier("A", _PLAN_ID, tree) == 1
|
||||
assert svc.compute_rollback_tier("B", _PLAN_ID, tree) == 1
|
||||
assert svc.compute_rollback_tier("C", _PLAN_ID, tree) == 2
|
||||
assert svc.compute_rollback_tier("D", _PLAN_ID, tree) == 2
|
||||
assert svc.compute_rollback_tier("E", _PLAN_ID, tree) == 2
|
||||
|
||||
# Deep tree test
|
||||
deep = _deep_tree(4)
|
||||
assert svc.compute_rollback_tier("depth_3", _PLAN_ID, deep) == 3
|
||||
print("rollback-tier-computation-ok")
|
||||
|
||||
|
||||
def _dry_run_report_enhanced() -> None:
|
||||
"""Verify dry-run report includes excluded decisions and tier depth."""
|
||||
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 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}"
|
||||
)
|
||||
print("dry-run-report-enhanced-ok")
|
||||
|
||||
|
||||
def _dry_run_tier_zero_warning() -> None:
|
||||
"""Verify tier-0 warning when root is targeted."""
|
||||
svc = CorrectionService()
|
||||
tree = _sample_tree()
|
||||
req = svc.request_correction(_PLAN_ID, "root", CorrectionMode.REVERT)
|
||||
report = svc.generate_dry_run_report(req.correction_id, tree)
|
||||
|
||||
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
|
||||
print("dry-run-tier-zero-warning-ok")
|
||||
|
||||
|
||||
def _subtree_isolation_validation() -> None:
|
||||
"""Verify subtree isolation validation passes for non-root targets."""
|
||||
svc = CorrectionService()
|
||||
tree = _sample_tree()
|
||||
|
||||
assert svc.validate_subtree_isolation("A", tree) is True
|
||||
assert svc.validate_subtree_isolation("D", tree) is True
|
||||
assert svc.validate_subtree_isolation("E", tree) is True
|
||||
print("subtree-isolation-validation-ok")
|
||||
|
||||
|
||||
def _edge_case_empty_tree() -> None:
|
||||
"""Verify edge cases with empty tree."""
|
||||
svc = CorrectionService()
|
||||
|
||||
# validate_subtree_isolation with empty tree
|
||||
assert svc.validate_subtree_isolation("X", {}) is True
|
||||
|
||||
# _find_root with empty tree
|
||||
assert CorrectionService._find_root({}) is None
|
||||
|
||||
# _find_parent for root node (not a child of anything)
|
||||
tree = _sample_tree()
|
||||
assert CorrectionService._find_parent("root", tree) is None
|
||||
|
||||
# compute_rollback_tier for unknown node
|
||||
assert svc.compute_rollback_tier("UNKNOWN", _PLAN_ID, tree) == 0
|
||||
print("edge-case-empty-tree-ok")
|
||||
|
||||
|
||||
def _influence_dag_traversal() -> None:
|
||||
"""Verify influence edges pull extra decisions into affected set."""
|
||||
svc = CorrectionService()
|
||||
tree = _sample_tree()
|
||||
|
||||
# D -> E via influence DAG (E is a sibling-subtree node)
|
||||
req = svc.request_correction(_PLAN_ID, "D", CorrectionMode.REVERT)
|
||||
influence = {"D": ["E"]}
|
||||
impact = svc.analyze_impact(req.correction_id, tree, influence_edges=influence)
|
||||
|
||||
assert "D" in impact.affected_decisions, (
|
||||
f"Expected 'D' in affected={impact.affected_decisions}"
|
||||
)
|
||||
assert "E" in impact.affected_decisions, (
|
||||
f"Expected 'E' in affected (via influence)={impact.affected_decisions}"
|
||||
)
|
||||
assert "root" in impact.excluded_decisions
|
||||
assert "A" in impact.excluded_decisions
|
||||
|
||||
# Cycle detection: C -> D -> C via influence DAG
|
||||
svc2 = CorrectionService()
|
||||
req2 = svc2.request_correction(_PLAN_ID, "C", CorrectionMode.REVERT)
|
||||
cycle_influence = {"C": ["D"], "D": ["C"]}
|
||||
impact2 = svc2.analyze_impact(
|
||||
req2.correction_id, tree, influence_edges=cycle_influence
|
||||
)
|
||||
assert "C" in impact2.affected_decisions
|
||||
assert "D" in impact2.affected_decisions
|
||||
|
||||
# Isolation validation passes even when influence edges reach sibling
|
||||
assert svc.validate_subtree_isolation("D", tree, influence_edges=influence) is True
|
||||
|
||||
print("influence-dag-traversal-ok")
|
||||
|
||||
|
||||
def _append_mode_correction() -> None:
|
||||
"""Verify append mode spawns a child plan."""
|
||||
svc = CorrectionService()
|
||||
req = svc.request_correction(_PLAN_ID, "A", CorrectionMode.APPEND)
|
||||
result = svc.execute_append(req.correction_id)
|
||||
|
||||
assert result.status.value == "applied", (
|
||||
f"Expected status='applied', got '{result.status.value}'"
|
||||
)
|
||||
assert result.spawned_child_plan_id is not None, (
|
||||
"Expected spawned_child_plan_id, got None"
|
||||
)
|
||||
assert len(result.new_decisions) > 0, (
|
||||
f"Expected new_decisions, got {result.new_decisions}"
|
||||
)
|
||||
print("append-mode-correction-ok")
|
||||
|
||||
|
||||
def _dry_run_enforcement() -> None:
|
||||
"""Verify that executing a dry-run correction raises ValidationError."""
|
||||
svc = CorrectionService()
|
||||
|
||||
# Revert dry-run should not be executable
|
||||
req_revert = svc.request_correction(
|
||||
_PLAN_ID, "A", CorrectionMode.REVERT, dry_run=True
|
||||
)
|
||||
try:
|
||||
svc.execute_correction(req_revert.correction_id, _sample_tree())
|
||||
raise AssertionError("Expected ValidationError for dry-run revert execution")
|
||||
except ValidationError as exc:
|
||||
assert "dry-run" in str(exc).lower(), (
|
||||
f"Expected 'dry-run' in error message, got: {exc}"
|
||||
)
|
||||
|
||||
# Append dry-run should not be executable
|
||||
svc2 = CorrectionService()
|
||||
req_append = svc2.request_correction(
|
||||
_PLAN_ID, "A", CorrectionMode.APPEND, dry_run=True
|
||||
)
|
||||
try:
|
||||
svc2.execute_correction(req_append.correction_id)
|
||||
raise AssertionError("Expected ValidationError for dry-run append execution")
|
||||
except ValidationError as exc:
|
||||
assert "dry-run" in str(exc).lower(), (
|
||||
f"Expected 'dry-run' in error message, got: {exc}"
|
||||
)
|
||||
|
||||
print("dry-run-enforcement-ok")
|
||||
|
||||
|
||||
def _execute_revert_non_dry_run() -> None:
|
||||
"""Verify execute_revert works end-to-end for a non-dry-run correction."""
|
||||
svc = CorrectionService()
|
||||
tree = _sample_tree()
|
||||
req = svc.request_correction(_PLAN_ID, "A", CorrectionMode.REVERT)
|
||||
result = svc.execute_revert(req.correction_id, tree)
|
||||
|
||||
assert result.status.value == "applied", (
|
||||
f"Expected status='applied', got '{result.status.value}'"
|
||||
)
|
||||
assert sorted(result.reverted_decisions) == sorted(["A", "C", "D"]), (
|
||||
f"Expected reverted=['A','C','D'], got {result.reverted_decisions}"
|
||||
)
|
||||
assert len(result.archived_artifacts) > 0, (
|
||||
f"Expected non-empty archived_artifacts, got {result.archived_artifacts}"
|
||||
)
|
||||
print("execute-revert-non-dry-run-ok")
|
||||
|
||||
|
||||
def _status_guard_enforcement() -> None:
|
||||
"""Verify status guards prevent re-execution and cancelled execution."""
|
||||
svc = CorrectionService()
|
||||
tree = _sample_tree()
|
||||
|
||||
# Re-execution of applied correction
|
||||
req = svc.request_correction(_PLAN_ID, "A", CorrectionMode.REVERT)
|
||||
svc.execute_revert(req.correction_id, tree)
|
||||
try:
|
||||
svc.execute_correction(req.correction_id, tree)
|
||||
raise AssertionError("Expected ValidationError for re-execution")
|
||||
except ValidationError:
|
||||
pass
|
||||
|
||||
# Execution of cancelled correction
|
||||
svc2 = CorrectionService()
|
||||
req2 = svc2.request_correction(_PLAN_ID, "B", CorrectionMode.REVERT)
|
||||
svc2.cancel_correction(req2.correction_id)
|
||||
try:
|
||||
svc2.execute_correction(req2.correction_id, tree)
|
||||
raise AssertionError("Expected ValidationError for cancelled execution")
|
||||
except ValidationError:
|
||||
pass
|
||||
|
||||
print("status-guard-enforcement-ok")
|
||||
|
||||
|
||||
def _mode_mismatch_enforcement() -> None:
|
||||
"""Verify mode mismatch raises ValidationError."""
|
||||
svc = CorrectionService()
|
||||
|
||||
# execute_revert on an append correction
|
||||
req_a = svc.request_correction(_PLAN_ID, "A", CorrectionMode.APPEND)
|
||||
try:
|
||||
svc.execute_revert(req_a.correction_id, _sample_tree())
|
||||
raise AssertionError("Expected ValidationError for mode mismatch")
|
||||
except ValidationError as exc:
|
||||
assert "REVERT" in str(exc), f"Expected 'REVERT' in error, got: {exc}"
|
||||
|
||||
# execute_append on a revert correction
|
||||
svc2 = CorrectionService()
|
||||
req_r = svc2.request_correction(_PLAN_ID, "A", CorrectionMode.REVERT)
|
||||
try:
|
||||
svc2.execute_append(req_r.correction_id)
|
||||
raise AssertionError("Expected ValidationError for mode mismatch")
|
||||
except ValidationError as exc:
|
||||
assert "APPEND" in str(exc), f"Expected 'APPEND' in error, got: {exc}"
|
||||
|
||||
print("mode-mismatch-enforcement-ok")
|
||||
|
||||
|
||||
def _diamond_dag_topology() -> None:
|
||||
"""Verify convergent (diamond) DAG topology does not duplicate visits."""
|
||||
svc = CorrectionService()
|
||||
tree = _sample_tree()
|
||||
|
||||
# Both C and D (children of A) influence E via DAG — diamond pattern
|
||||
req = svc.request_correction(_PLAN_ID, "A", CorrectionMode.REVERT)
|
||||
influence = {"C": ["E"], "D": ["E"]}
|
||||
impact = svc.analyze_impact(req.correction_id, tree, influence_edges=influence)
|
||||
|
||||
assert "E" in impact.affected_decisions, (
|
||||
f"Expected 'E' in affected={impact.affected_decisions}"
|
||||
)
|
||||
count = impact.affected_decisions.count("E")
|
||||
assert count == 1, (
|
||||
f"Expected 'E' exactly once, found {count} times: {impact.affected_decisions}"
|
||||
)
|
||||
print("diamond-dag-topology-ok")
|
||||
|
||||
|
||||
def _dry_run_exception_recovery() -> None:
|
||||
"""Verify dry-run report restores status when analyze_impact fails."""
|
||||
svc = CorrectionService()
|
||||
tree = _sample_tree()
|
||||
req = svc.request_correction(_PLAN_ID, "A", CorrectionMode.REVERT)
|
||||
|
||||
assert req.status == CorrectionStatus.PENDING, (
|
||||
f"Expected PENDING before dry-run, got {req.status}"
|
||||
)
|
||||
|
||||
try:
|
||||
with patch.object(
|
||||
CorrectionService,
|
||||
"_compute_affected_subtree",
|
||||
side_effect=RuntimeError("simulated BFS failure"),
|
||||
):
|
||||
svc.generate_dry_run_report(req.correction_id, tree)
|
||||
except RuntimeError:
|
||||
pass # Expected — exception propagates from analyze_impact
|
||||
|
||||
assert req.status == CorrectionStatus.PENDING, (
|
||||
f"Expected PENDING after failed dry-run, got {req.status}"
|
||||
)
|
||||
print("dry-run-exception-recovery-ok")
|
||||
|
||||
|
||||
def _execute_revert_with_influence() -> None:
|
||||
"""Verify execute_revert works with influence edges."""
|
||||
svc = CorrectionService()
|
||||
tree = _sample_tree()
|
||||
|
||||
# D influences E via DAG — revert of A should include E
|
||||
influence = {"D": ["E"]}
|
||||
req = svc.request_correction(_PLAN_ID, "A", CorrectionMode.REVERT)
|
||||
result = svc.execute_revert(req.correction_id, tree, influence_edges=influence)
|
||||
|
||||
assert result.status.value == "applied", (
|
||||
f"Expected status='applied', got '{result.status.value}'"
|
||||
)
|
||||
assert "E" in result.reverted_decisions, (
|
||||
f"Expected 'E' in reverted={result.reverted_decisions}"
|
||||
)
|
||||
print("execute-revert-with-influence-ok")
|
||||
|
||||
|
||||
def _dag_only_nodes_excluded() -> None:
|
||||
"""Verify nodes in the DAG (not in tree) appear in excluded set."""
|
||||
svc = CorrectionService()
|
||||
tree = _sample_tree()
|
||||
|
||||
# X_EXT and Y_EXT exist only in the influence DAG, not in the tree
|
||||
influence = {"X_EXT": ["Y_EXT"]}
|
||||
req = svc.request_correction(_PLAN_ID, "D", CorrectionMode.REVERT)
|
||||
impact = svc.analyze_impact(req.correction_id, tree, influence_edges=influence)
|
||||
|
||||
assert "X_EXT" in impact.excluded_decisions, (
|
||||
f"Expected 'X_EXT' in excluded={impact.excluded_decisions}"
|
||||
)
|
||||
assert "Y_EXT" in impact.excluded_decisions, (
|
||||
f"Expected 'Y_EXT' in excluded={impact.excluded_decisions}"
|
||||
)
|
||||
print("dag-only-nodes-excluded-ok")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Dispatch
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
COMMANDS: dict[str, Callable[[], None]] = {
|
||||
"leaf-isolation": _leaf_isolation,
|
||||
"middle-node-isolation": _middle_node_isolation,
|
||||
"root-targets-all": _root_targets_all,
|
||||
"rollback-tier-computation": _rollback_tier_computation,
|
||||
"dry-run-report-enhanced": _dry_run_report_enhanced,
|
||||
"dry-run-tier-zero-warning": _dry_run_tier_zero_warning,
|
||||
"subtree-isolation-validation": _subtree_isolation_validation,
|
||||
"edge-case-empty-tree": _edge_case_empty_tree,
|
||||
"influence-dag-traversal": _influence_dag_traversal,
|
||||
"append-mode-correction": _append_mode_correction,
|
||||
"dry-run-enforcement": _dry_run_enforcement,
|
||||
"execute-revert-non-dry-run": _execute_revert_non_dry_run,
|
||||
"status-guard-enforcement": _status_guard_enforcement,
|
||||
"mode-mismatch-enforcement": _mode_mismatch_enforcement,
|
||||
"diamond-dag-topology": _diamond_dag_topology,
|
||||
"dry-run-exception-recovery": _dry_run_exception_recovery,
|
||||
"execute-revert-with-influence": _execute_revert_with_influence,
|
||||
"dag-only-nodes-excluded": _dag_only_nodes_excluded,
|
||||
}
|
||||
|
||||
|
||||
def main() -> None:
|
||||
"""Dispatch command from sys.argv."""
|
||||
if len(sys.argv) < 2:
|
||||
raise SystemExit("Expected command argument")
|
||||
command = sys.argv[1]
|
||||
if command not in COMMANDS:
|
||||
raise SystemExit(f"Unknown command: {command}")
|
||||
func = COMMANDS[command]
|
||||
func()
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
main()
|
||||
@@ -41,6 +41,36 @@ logger = structlog.get_logger(__name__)
|
||||
_RISK_LOW_MAX = 3
|
||||
_RISK_MEDIUM_MAX = 10
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Cost / time estimation constants
|
||||
# ---------------------------------------------------------------------------
|
||||
_COST_PER_DECISION = 1.5
|
||||
_RECOMPUTE_SECONDS_PER_DECISION = 2.0
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Terminal correction statuses (immutable after execution)
|
||||
# ---------------------------------------------------------------------------
|
||||
_TERMINAL_STATUSES: frozenset[CorrectionStatus] = frozenset(
|
||||
{
|
||||
CorrectionStatus.APPLIED,
|
||||
CorrectionStatus.FAILED,
|
||||
CorrectionStatus.CANCELLED,
|
||||
CorrectionStatus.REJECTED,
|
||||
}
|
||||
)
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Executable / cancellable statuses (shared by guards)
|
||||
# ---------------------------------------------------------------------------
|
||||
_EXECUTABLE_STATUSES: frozenset[CorrectionStatus] = frozenset(
|
||||
{CorrectionStatus.PENDING, CorrectionStatus.ANALYZING}
|
||||
)
|
||||
|
||||
|
|
||||
# ---------------------------------------------------------------------------
|
||||
# Maximum decision tree size (DoS protection)
|
||||
# ---------------------------------------------------------------------------
|
||||
freemo
commented
Minor: This parameter is accepted "for API consistency" per the docstring, but silently discarded (structural-only BFS is used). This could mislead callers into believing influence edges affect isolation validation. Consider either **Minor**: This parameter is accepted "for API consistency" per the docstring, but silently discarded (structural-only BFS is used). This could mislead callers into believing influence edges affect isolation validation.
Consider either `_influence_edges` prefix to signal it's unused, or omitting it if the spec is clear that isolation is structural-only.
|
||||
_MAX_TREE_NODES = 50_000
|
||||
|
||||
|
||||
class CorrectionService:
|
||||
"""Service for creating, analysing, and executing decision corrections.
|
||||
@@ -75,6 +105,7 @@ class CorrectionService:
|
||||
correction_id: str,
|
||||
request: CorrectionRequest,
|
||||
result: CorrectionResult,
|
||||
attempt_id: str | None = None,
|
||||
) -> None:
|
||||
"""Emit a ``CORRECTION_APPLIED`` event when the result is successful."""
|
||||
if self._event_bus is not None and result.status == CorrectionStatus.APPLIED:
|
||||
@@ -85,6 +116,7 @@ class CorrectionService:
|
||||
plan_id=request.plan_id,
|
||||
details={
|
||||
"correction_id": correction_id,
|
||||
"attempt_id": attempt_id,
|
||||
"target_decision_id": request.target_decision_id,
|
||||
"mode": request.mode.value
|
||||
if hasattr(request.mode, "value")
|
||||
@@ -94,7 +126,7 @@ class CorrectionService:
|
||||
)
|
||||
)
|
||||
except Exception:
|
||||
logger.warning(
|
||||
logger.error(
|
||||
"event_bus_emit_failed",
|
||||
event_type="CORRECTION_APPLIED",
|
||||
correction_id=correction_id,
|
||||
@@ -170,6 +202,11 @@ class CorrectionService:
|
||||
corrections cascade through influence relationships as required
|
||||
CoreRasurae
commented
L2 — Performance: This set is allocated on every **L2 — Performance: `_TERMINAL` set recreated on every call**
This set is allocated on every `analyze_impact` invocation. Consider promoting it to a module-level constant (e.g., `_TERMINAL_STATUSES`) following the pattern of `_RISK_LOW_MAX` and `_RISK_MEDIUM_MAX` already at module level.
|
||||
by the specification (§ Affected Subtree Computation).
|
||||
|
||||
After computing the affected subtree, populates
|
||||
``excluded_decisions`` with all plan decisions that are **not**
|
||||
in the affected set, enabling callers to understand which parts
|
||||
of the tree remain untouched.
|
||||
|
||||
Args:
|
||||
correction_id: Previously created correction request ID.
|
||||
decision_tree: Adjacency list mapping parent → children.
|
||||
@@ -177,31 +214,69 @@ class CorrectionService:
|
||||
in the influence DAG (``decision_dependencies``).
|
||||
|
||||
Returns:
|
||||
``CorrectionImpact`` with affected nodes and risk level.
|
||||
``CorrectionImpact`` with affected nodes, excluded nodes,
|
||||
rollback tier depth, and risk level.
|
||||
|
||||
Raises:
|
||||
ResourceNotFoundError: If the correction does not exist.
|
||||
"""
|
||||
request = self._get_request_or_raise(correction_id)
|
||||
request.status = CorrectionStatus.ANALYZING
|
||||
|
||||
# Guard against pathologically large inputs that could cause
|
||||
# unbounded memory / CPU consumption during BFS traversal.
|
||||
tree = decision_tree or {}
|
||||
dag = influence_edges or {}
|
||||
total_keys = len(tree) + len(dag)
|
||||
if total_keys > _MAX_TREE_NODES:
|
||||
raise ValidationError(
|
||||
f"Decision tree + influence DAG too large ({total_keys} keys, "
|
||||
f"max {_MAX_TREE_NODES}). Reduce tree size before analyzing."
|
||||
)
|
||||
|
||||
# Reject analysis on terminal states to prevent audit-data
|
||||
# corruption (the stored impact must match what was used during
|
||||
# execution).
|
||||
if request.status in _TERMINAL_STATUSES:
|
||||
raise ValidationError(
|
||||
f"Cannot analyze correction in terminal '{request.status}' "
|
||||
"status. Impact data is immutable after execution."
|
||||
)
|
||||
|
||||
if request.status == CorrectionStatus.PENDING:
|
||||
request.status = CorrectionStatus.ANALYZING
|
||||
|
||||
affected = self._compute_affected_subtree(request.target_decision_id, tree, dag)
|
||||
risk = self._classify_risk(len(affected))
|
||||
|
||||
# Collect all decisions known in the plan from the adjacency lists
|
||||
all_decisions = self._collect_all_decisions(tree, dag)
|
||||
# Guarantee the target is in the universe even if it does not
|
||||
# appear in any adjacency list (isolated single-node plan).
|
||||
all_decisions.add(request.target_decision_id)
|
||||
affected_set = set(affected)
|
||||
excluded = sorted(d for d in all_decisions if d not in affected_set)
|
||||
|
||||
# Compute rollback tier depth (hops from target to root)
|
||||
tier_depth = self._compute_rollback_tier_depth(request.target_decision_id, tree)
|
||||
|
||||
# Derive artefacts from decision IDs (convention: <id>.artifact)
|
||||
artifacts = [f"{d}.artifact" for d in affected]
|
||||
|
||||
impact = CorrectionImpact(
|
||||
affected_decisions=affected,
|
||||
excluded_decisions=excluded,
|
||||
# TODO: affected_files and artifacts_to_archive use synthetic
|
||||
# placeholders derived from decision IDs. Replace with real
|
||||
# file / artifact tracking once the resource-rollback layer
|
||||
# is integrated (see spec § Mid-Execute Correction).
|
||||
affected_files=[f"{d}.py" for d in affected],
|
||||
affected_child_plans=[],
|
||||
estimated_cost=float(len(affected)) * 1.5,
|
||||
estimated_cost=float(len(affected)) * _COST_PER_DECISION,
|
||||
risk_level=risk,
|
||||
rollback_tier="full"
|
||||
if request.mode == CorrectionMode.REVERT
|
||||
else "append_only",
|
||||
rollback_tier_depth=tier_depth,
|
||||
artifacts_to_archive=artifacts,
|
||||
)
|
||||
self._impacts[correction_id] = impact
|
||||
@@ -210,6 +285,8 @@ class CorrectionService:
|
||||
"correction.impact_analyzed",
|
||||
correction_id=correction_id,
|
||||
affected_count=len(affected),
|
||||
excluded_count=len(excluded),
|
||||
rollback_tier_depth=tier_depth,
|
||||
risk_level=risk,
|
||||
)
|
||||
return impact
|
||||
@@ -224,10 +301,16 @@ class CorrectionService:
|
||||
decision_tree: dict[str, list[str]] | None = None,
|
||||
influence_edges: dict[str, list[str]] | None = None,
|
||||
) -> CorrectionDryRunReport:
|
||||
"""Generate a dry-run report without mutating state.
|
||||
"""Generate a dry-run report without executing the correction.
|
||||
|
||||
Calls ``analyze_impact`` internally, then assembles warnings
|
||||
and estimated recompute time.
|
||||
Calls ``analyze_impact`` internally to compute the affected
|
||||
subtree, then assembles warnings, excluded decisions, rollback
|
||||
tier depth, and estimated recompute time. The request status
|
||||
is preserved (not mutated) so that generating a preview does
|
||||
not advance the correction lifecycle.
|
||||
|
||||
A tier-0 warning is emitted when the root decision is targeted,
|
||||
indicating the entire decision tree will be affected.
|
||||
|
||||
Args:
|
||||
correction_id: Correction request ID.
|
||||
@@ -238,7 +321,20 @@ class CorrectionService:
|
||||
``CorrectionDryRunReport`` describing what *would* happen.
|
||||
"""
|
||||
request = self._get_request_or_raise(correction_id)
|
||||
impact = self.analyze_impact(correction_id, decision_tree, influence_edges)
|
||||
|
||||
# Preserve original status and impact — dry-run is conceptually
|
||||
# read-only. The try/finally ensures both are restored even when
|
||||
# analyze_impact raises after transitioning the status.
|
||||
original_status = request.status
|
||||
original_impact = self._impacts.get(correction_id)
|
||||
try:
|
||||
impact = self.analyze_impact(correction_id, decision_tree, influence_edges)
|
||||
finally:
|
||||
request.status = original_status
|
||||
if original_impact is None:
|
||||
self._impacts.pop(correction_id, None)
|
||||
else:
|
||||
self._impacts[correction_id] = original_impact
|
||||
|
||||
warnings: list[str] = []
|
||||
if impact.risk_level == "high":
|
||||
@@ -246,15 +342,33 @@ class CorrectionService:
|
||||
"High risk: more than 10 decisions affected. "
|
||||
"Review carefully before executing."
|
||||
)
|
||||
if impact.risk_level == "medium":
|
||||
elif impact.risk_level == "medium":
|
||||
warnings.append("Medium risk: 4-10 decisions affected.")
|
||||
if request.mode == CorrectionMode.REVERT and len(impact.affected_decisions) > 1:
|
||||
warnings.append(
|
||||
f"Revert will invalidate {len(impact.affected_decisions)} decisions "
|
||||
"and archive associated artifacts."
|
||||
)
|
||||
# Only emit a tier-0 warning when the target is genuinely the
|
||||
# tree root. Compare against the actual root determined by
|
||||
# _find_root to avoid false positives for non-root subtree
|
||||
# heads in forest (disconnected) topologies.
|
||||
tree = decision_tree or {}
|
||||
actual_root = self._find_root(tree)
|
||||
if (
|
||||
impact.rollback_tier_depth == 0
|
||||
and len(impact.affected_decisions) > 1
|
||||
and actual_root is not None
|
||||
and request.target_decision_id == actual_root
|
||||
):
|
||||
warnings.append(
|
||||
"Tier 0: root decision targeted — entire decision tree "
|
||||
"will be affected."
|
||||
)
|
||||
|
||||
recompute_seconds = float(len(impact.affected_decisions)) * 2.0
|
||||
recompute_seconds = (
|
||||
float(len(impact.affected_decisions)) * _RECOMPUTE_SECONDS_PER_DECISION
|
||||
)
|
||||
|
||||
report = CorrectionDryRunReport(
|
||||
correction_id=correction_id,
|
||||
@@ -264,6 +378,8 @@ class CorrectionService:
|
||||
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,
|
||||
)
|
||||
@@ -272,6 +388,8 @@ class CorrectionService:
|
||||
"correction.dry_run_generated",
|
||||
correction_id=correction_id,
|
||||
warning_count=len(warnings),
|
||||
rollback_tier_depth=impact.rollback_tier_depth,
|
||||
excluded_count=len(impact.excluded_decisions),
|
||||
)
|
||||
return report
|
||||
|
||||
@@ -301,17 +419,32 @@ class CorrectionService:
|
||||
|
||||
Raises:
|
||||
ResourceNotFoundError: If correction does not exist.
|
||||
ValidationError: If correction is not in PENDING or ANALYZING status.
|
||||
ValidationError: If correction is not in PENDING or ANALYZING
|
||||
status, or if the correction mode is not REVERT.
|
||||
"""
|
||||
request = self._get_request_or_raise(correction_id)
|
||||
if request.mode != CorrectionMode.REVERT:
|
||||
raise ValidationError(
|
||||
f"execute_revert requires mode=REVERT, got mode={request.mode.value!r}."
|
||||
)
|
||||
self._assert_executable(request)
|
||||
request.status = CorrectionStatus.EXECUTING
|
||||
|
||||
attempt = CorrectionAttempt(correction_id=correction_id)
|
||||
self._attempts[correction_id].append(attempt)
|
||||
|
||||
# Transition through ANALYZING before EXECUTING so the full
|
||||
# state-machine lifecycle is honoured.
|
||||
request.status = CorrectionStatus.ANALYZING
|
||||
|
||||
try:
|
||||
impact = self.analyze_impact(correction_id, decision_tree, influence_edges)
|
||||
# Use previously cached impact when available to avoid
|
||||
# redundant O(V+E) recomputation.
|
||||
impact = self._impacts.get(correction_id)
|
||||
if impact is None:
|
||||
impact = self.analyze_impact(
|
||||
correction_id, decision_tree, influence_edges
|
||||
)
|
||||
request.status = CorrectionStatus.EXECUTING
|
||||
|
||||
result = CorrectionResult(
|
||||
correction_id=correction_id,
|
||||
@@ -322,6 +455,11 @@ class CorrectionService:
|
||||
request.status = CorrectionStatus.APPLIED
|
||||
attempt.success = True
|
||||
except Exception as exc:
|
||||
logger.error(
|
||||
"correction.revert_failed",
|
||||
correction_id=correction_id,
|
||||
exc_info=True,
|
||||
)
|
||||
result = CorrectionResult(
|
||||
correction_id=correction_id,
|
||||
status=CorrectionStatus.FAILED,
|
||||
@@ -339,7 +477,9 @@ class CorrectionService:
|
||||
correction_id=correction_id,
|
||||
status=result.status,
|
||||
)
|
||||
self._emit_correction_applied(correction_id, request, result)
|
||||
self._emit_correction_applied(
|
||||
correction_id, request, result, attempt_id=attempt.attempt_id
|
||||
)
|
||||
return result
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
@@ -363,15 +503,24 @@ class CorrectionService:
|
||||
|
||||
Raises:
|
||||
ResourceNotFoundError: If correction does not exist.
|
||||
ValidationError: If correction is not in PENDING or ANALYZING status.
|
||||
ValidationError: If correction is not in PENDING or ANALYZING
|
||||
status, or if the correction mode is not APPEND.
|
||||
"""
|
||||
request = self._get_request_or_raise(correction_id)
|
||||
if request.mode != CorrectionMode.APPEND:
|
||||
raise ValidationError(
|
||||
f"execute_append requires mode=APPEND, got mode={request.mode.value!r}."
|
||||
)
|
||||
self._assert_executable(request)
|
||||
request.status = CorrectionStatus.EXECUTING
|
||||
|
||||
attempt = CorrectionAttempt(correction_id=correction_id)
|
||||
self._attempts[correction_id].append(attempt)
|
||||
|
||||
# Transition through ANALYZING → EXECUTING for consistent
|
||||
# lifecycle across both correction modes.
|
||||
request.status = CorrectionStatus.ANALYZING
|
||||
request.status = CorrectionStatus.EXECUTING
|
||||
|
||||
try:
|
||||
child_plan_id = str(ULID())
|
||||
new_decision_id = str(ULID())
|
||||
@@ -389,6 +538,11 @@ class CorrectionService:
|
||||
"new_decision_id": new_decision_id,
|
||||
}
|
||||
except Exception as exc:
|
||||
logger.error(
|
||||
"correction.append_failed",
|
||||
correction_id=correction_id,
|
||||
exc_info=True,
|
||||
)
|
||||
result = CorrectionResult(
|
||||
correction_id=correction_id,
|
||||
status=CorrectionStatus.FAILED,
|
||||
@@ -406,7 +560,9 @@ class CorrectionService:
|
||||
correction_id=correction_id,
|
||||
status=result.status,
|
||||
)
|
||||
self._emit_correction_applied(correction_id, request, result)
|
||||
self._emit_correction_applied(
|
||||
correction_id, request, result, attempt_id=attempt.attempt_id
|
||||
)
|
||||
return result
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
@@ -473,11 +629,10 @@ class CorrectionService:
|
||||
ValidationError: If correction is not in a cancellable status.
|
||||
"""
|
||||
request = self._get_request_or_raise(correction_id)
|
||||
cancellable = {CorrectionStatus.PENDING, CorrectionStatus.ANALYZING}
|
||||
if request.status not in cancellable:
|
||||
if request.status not in _EXECUTABLE_STATUSES:
|
||||
raise ValidationError(
|
||||
f"Cannot cancel correction in '{request.status}' status. "
|
||||
f"Cancellation is only allowed in: {sorted(cancellable)}"
|
||||
f"Cancellation is only allowed in: {sorted(_EXECUTABLE_STATUSES)}"
|
||||
)
|
||||
request.status = CorrectionStatus.CANCELLED
|
||||
logger.info(
|
||||
@@ -501,14 +656,145 @@ class CorrectionService:
|
||||
return request
|
||||
|
||||
def _assert_executable(self, request: CorrectionRequest) -> None:
|
||||
"""Ensure the correction is in an executable status."""
|
||||
executable = {CorrectionStatus.PENDING, CorrectionStatus.ANALYZING}
|
||||
if request.status not in executable:
|
||||
"""Ensure the correction is in an executable status.
|
||||
|
||||
Raises:
|
||||
ValidationError: If the correction was created as dry-run
|
||||
only, or if its status is not in the executable set.
|
||||
"""
|
||||
if request.dry_run:
|
||||
raise ValidationError(
|
||||
"Cannot execute a dry-run correction. "
|
||||
"Use generate_dry_run_report() or analyze_impact() instead."
|
||||
)
|
||||
if request.status not in _EXECUTABLE_STATUSES:
|
||||
raise ValidationError(
|
||||
f"Cannot execute correction in '{request.status}' status. "
|
||||
f"Execution requires status in: {sorted(executable)}"
|
||||
f"Execution requires status in: {sorted(_EXECUTABLE_STATUSES)}"
|
||||
)
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
# Rollback tier computation
|
||||
# ------------------------------------------------------------------
|
||||
|
||||
def compute_rollback_tier(
|
||||
self,
|
||||
target_decision_id: str,
|
||||
plan_id: str,
|
||||
decision_tree: dict[str, list[str]] | None = None,
|
||||
) -> int:
|
||||
"""Compute the rollback tier (depth) for a target decision.
|
||||
|
||||
The tier is the number of parent hops from the target to the
|
||||
tree root:
|
||||
|
||||
- **Tier 0**: the root decision itself is targeted.
|
||||
- **Tier 1**: a direct child of the root is targeted.
|
||||
- **Tier N**: the target is *N* levels below the root.
|
||||
|
||||
Args:
|
||||
target_decision_id: Decision to compute the tier for.
|
||||
plan_id: Plan owning the decision tree (reserved for
|
||||
future repository look-ups; currently unused beyond
|
||||
logging).
|
||||
decision_tree: Adjacency list (parent → children).
|
||||
|
||||
Returns:
|
||||
Non-negative integer representing the tier depth.
|
||||
"""
|
||||
tree = decision_tree or {}
|
||||
depth = self._compute_rollback_tier_depth(target_decision_id, tree)
|
||||
logger.info(
|
||||
"correction.rollback_tier_computed",
|
||||
target_decision_id=target_decision_id,
|
||||
plan_id=plan_id,
|
||||
tier_depth=depth,
|
||||
)
|
||||
return depth
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
# Subtree isolation validation
|
||||
# ------------------------------------------------------------------
|
||||
|
||||
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.
|
||||
|
||||
Confirms two invariants for non-root corrections:
|
||||
|
||||
1. The **root decision** (the node with no parent in the tree)
|
||||
is never in the *structural* affected set unless it is
|
||||
explicitly the *target* decision.
|
||||
2. **Sibling decisions** (children of the same parent as the
|
||||
target, excluding the target itself) are not in the
|
||||
*structural* affected set.
|
||||
|
||||
Both invariants are checked against the **structural-only**
|
||||
affected set (tree traversal without influence edges). If the
|
||||
influence DAG legitimately pulls in a sibling or the root,
|
||||
that is expected behaviour per the spec (§ Affected Subtree
|
||||
Computation) and is not an isolation violation.
|
||||
|
||||
Args:
|
||||
target_decision_id: Decision node that was targeted.
|
||||
decision_tree: Structural tree adjacency list.
|
||||
influence_edges: Optional influence DAG adjacency list.
|
||||
Accepted for API consistency but **not used** in
|
||||
isolation checks — structural-only BFS is used per
|
||||
the specification (§ Affected Subtree Computation).
|
||||
|
||||
Returns:
|
||||
``True`` if isolation invariants hold, ``False`` otherwise.
|
||||
"""
|
||||
tree = decision_tree
|
||||
|
||||
# Use structural-only BFS for isolation invariant checks so
|
||||
# that influence-DAG-caused reachability is not misreported
|
||||
# as a violation.
|
||||
structural_affected = self._compute_affected_subtree(
|
||||
target_decision_id,
|
||||
tree,
|
||||
influence_edges=None,
|
||||
)
|
||||
structural_set = set(structural_affected)
|
||||
|
||||
# Find the root (node that never appears as a child)
|
||||
root = self._find_root(tree)
|
||||
if root is None:
|
||||
# Empty or flat tree — nothing to validate
|
||||
return True
|
||||
|
||||
# Invariant 1: root not in structural affected set unless
|
||||
# explicitly targeted
|
||||
if root in structural_set and root != target_decision_id:
|
||||
logger.warning(
|
||||
"correction.isolation_violation_root",
|
||||
root=root,
|
||||
target=target_decision_id,
|
||||
)
|
||||
return False
|
||||
|
||||
# Invariant 2: siblings of target not in structural affected set
|
||||
parent = self._find_parent(target_decision_id, tree)
|
||||
if parent is not None:
|
||||
siblings = [
|
||||
child for child in tree.get(parent, []) if child != target_decision_id
|
||||
]
|
||||
for sibling in siblings:
|
||||
if sibling in structural_set:
|
||||
logger.warning(
|
||||
"correction.isolation_violation_sibling",
|
||||
sibling=sibling,
|
||||
target=target_decision_id,
|
||||
)
|
||||
return False
|
||||
|
||||
return True
|
||||
|
||||
@staticmethod
|
||||
def _compute_affected_subtree(
|
||||
target_id: str,
|
||||
@@ -540,33 +826,50 @@ class CorrectionService:
|
||||
"""
|
||||
dag = influence_edges or {}
|
||||
affected: list[str] = []
|
||||
# A single ``visited`` set tracks every node that has been
|
||||
# dequeued and processed. New neighbors are only enqueued
|
||||
# when not yet in ``visited``, preventing both duplicate
|
||||
# processing and infinite loops from cycles. When a neighbor
|
||||
# is already visited it means we encountered a back-edge
|
||||
# (cycle in the structural tree or influence DAG); we log the
|
||||
# cycle for operational observability.
|
||||
visited: set[str] = set()
|
||||
queue: deque[str] = deque([target_id])
|
||||
|
||||
while queue:
|
||||
node = queue.popleft()
|
||||
if node in visited:
|
||||
logger.warning(
|
||||
"correction.cycle_detected",
|
||||
node=node,
|
||||
msg="Skipping already-visited node during BFS "
|
||||
"(possible cycle in influence DAG)",
|
||||
)
|
||||
# Node was already enqueued by a different parent
|
||||
# (convergent / diamond topology) — skip silently.
|
||||
continue
|
||||
visited.add(node)
|
||||
affected.append(node)
|
||||
|
||||
# Follow structural tree children
|
||||
structural_children = tree.get(node, [])
|
||||
# Follow influence DAG dependents
|
||||
influence_dependents = dag.get(node, [])
|
||||
|
||||
# Union of both edge sets
|
||||
for neighbor in structural_children:
|
||||
if neighbor not in visited:
|
||||
# Follow structural tree children first, then influence DAG
|
||||
# dependents.
|
||||
for neighbor in tree.get(node, []):
|
||||
if neighbor in visited:
|
||||
logger.warning(
|
||||
"correction.cycle_detected",
|
||||
node=neighbor,
|
||||
source=node,
|
||||
edge_type="structural",
|
||||
msg="Back-edge detected during BFS "
|
||||
"(cycle in decision tree or influence DAG)",
|
||||
)
|
||||
elif neighbor not in visited:
|
||||
queue.append(neighbor)
|
||||
for neighbor in influence_dependents:
|
||||
if neighbor not in visited:
|
||||
for neighbor in dag.get(node, []):
|
||||
if neighbor in visited:
|
||||
logger.warning(
|
||||
"correction.cycle_detected",
|
||||
node=neighbor,
|
||||
source=node,
|
||||
edge_type="influence",
|
||||
msg="Back-edge detected during BFS "
|
||||
"(cycle in decision tree or influence DAG)",
|
||||
)
|
||||
elif neighbor not in visited:
|
||||
queue.append(neighbor)
|
||||
|
||||
influence_count = sum(len(v) for v in dag.values()) if dag else 0
|
||||
@@ -589,6 +892,95 @@ class CorrectionService:
|
||||
return "medium"
|
||||
return "high"
|
||||
|
||||
@staticmethod
|
||||
def _collect_all_decisions(
|
||||
tree: dict[str, list[str]],
|
||||
dag: dict[str, list[str]],
|
||||
) -> set[str]:
|
||||
"""Collect every decision ID from both the tree and DAG edges.
|
||||
|
||||
Gathers all nodes that appear as either keys or values in the
|
||||
structural tree and influence DAG adjacency lists.
|
||||
"""
|
||||
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.
|
||||
|
||||
Builds a child → parent mapping from the adjacency list, then
|
||||
walks upward from the target. Returns ``0`` when the target
|
||||
is the root **or** when the target is not present in the tree.
|
||||
|
||||
.. note::
|
||||
|
||||
A return value of ``0`` is ambiguous: it can mean either
|
||||
"the target is the tree root" or "the target was not found
|
||||
in the tree." Callers that need to distinguish these cases
|
||||
should check whether the target appears in the tree before
|
||||
calling this method.
|
||||
"""
|
||||
child_to_parent: dict[str, str] = {}
|
||||
for parent, children in tree.items():
|
||||
for child in children:
|
||||
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 of the tree (not a child of any node).
|
||||
|
||||
Returns ``None`` if the tree is empty.
|
||||
|
||||
.. note::
|
||||
|
||||
If the tree is a forest (multiple disconnected subtrees)
|
||||
the first parent that is not a child of any other node is
|
||||
returned, following Python dict insertion order. For
|
||||
degenerate cases where every node appears as someone's
|
||||
child (cycles), the first key is returned as a fallback.
|
||||
"""
|
||||
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
|
||||
# Fallback: return the first key (degenerate tree)
|
||||
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
|
||||
|
||||
|
||||
__all__ = [
|
||||
"CorrectionService",
|
||||
|
||||
@@ -73,6 +73,7 @@ class CorrectionRequest(BaseModel):
|
||||
)
|
||||
guidance: str = Field(
|
||||
default="",
|
||||
max_length=10_000,
|
||||
description="Human-supplied guidance for the correction.",
|
||||
)
|
||||
dry_run: bool = Field(
|
||||
@@ -121,6 +122,10 @@ class CorrectionImpact(BaseModel):
|
||||
default_factory=list,
|
||||
description="Decision IDs in the affected subtree (BFS order).",
|
||||
)
|
||||
excluded_decisions: list[str] = Field(
|
||||
default_factory=list,
|
||||
description="Decision IDs NOT in the affected subtree (preserved).",
|
||||
)
|
||||
affected_files: list[str] = Field(
|
||||
default_factory=list,
|
||||
description="Files touched by affected decisions.",
|
||||
@@ -141,6 +146,14 @@ class CorrectionImpact(BaseModel):
|
||||
default="full",
|
||||
description="Rollback granularity: full, phase, or append_only.",
|
||||
)
|
||||
rollback_tier_depth: int = Field(
|
||||
default=0,
|
||||
description=(
|
||||
"Depth of the targeted decision from the tree root. "
|
||||
"Tier 0 means the root itself is targeted; tier N means "
|
||||
"the target is N hops below the root."
|
||||
),
|
||||
)
|
||||
artifacts_to_archive: list[str] = Field(
|
||||
default_factory=list,
|
||||
description="Artifact paths that should be archived on revert.",
|
||||
@@ -265,6 +278,14 @@ class CorrectionDryRunReport(BaseModel):
|
||||
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.",
|
||||
|
||||
Reference in New Issue
Block a user
Minor: These two consecutive assignments mean the ANALYZING state is never observable — no work runs between them, no event is emitted, nothing is persisted.
Contrast with
execute_revertwhere ANALYZING is meaningful becauseanalyze_impact()runs between the transitions. Here it's purely ceremonial.Consider either: