diff --git a/features/steps/plan_executor_decision_persistence_steps.py b/features/steps/plan_executor_decision_persistence_steps.py index 23519c7cf..7258eea59 100644 --- a/features/steps/plan_executor_decision_persistence_steps.py +++ b/features/steps/plan_executor_decision_persistence_steps.py @@ -8,7 +8,6 @@ All step texts use the ``dp`` prefix (decision persistence) to avoid collisions. from __future__ import annotations -import json from unittest.mock import MagicMock, patch from behave import given, then, when @@ -23,8 +22,6 @@ from cleveragents.application.services.plan_executor import ( from cleveragents.application.services.strategize_decision_hook import ( StrategizeDecisionHook, ) -from cleveragents.core.exceptions import PlanError, ValidationError -from cleveragents.domain.models.core.decision import DecisionType from cleveragents.domain.models.core.plan import ( PlanInvariant, PlanPhase, @@ -36,7 +33,8 @@ from cleveragents.domain.models.core.plan import ( # Constants # ---------------------------------------------------------------------- -DP_PLAN_ID = "01KDPPERSIST000000000PERSIST" +DP_PLAN_ID = "01JDPPERST0000000000000000" +DP_ROOT_ID = "01JDPRSTD000000000000000RT" # ---------------------------------------------------------------------- @@ -57,7 +55,7 @@ def _dp_make_plan( plan.phase = phase plan.state = state plan.definition_of_done = definition_of_done - plan.decision_root_id = decision_root_id or f"01KDPROOTID00000000000ROOT" + plan.decision_root_id = decision_root_id or DP_ROOT_ID plan.invariants = invariants or [] plan.timestamps = PlanTimestamps() plan.changeset_id = None @@ -69,10 +67,10 @@ def _dp_make_plan( def _dp_make_decisions(count: int = 2) -> list[StrategyDecision]: """Build a list of StrategyDecision instances for the test.""" - root_id = f"01KDPROOTID00000000000{DP_PLAN_ID[:4]}" + root_id = DP_ROOT_ID decisions: list[StrategyDecision] = [] for i in range(count): - did = root_id if i == 0 else f"01KDPPERSIST{i:020d}" + did = root_id if i == 0 else f"01JDPP{i:020d}" decisions.append( StrategyDecision( decision_id=did, @@ -109,7 +107,7 @@ def step_given_dp_memory_decision_service(context: Context) -> None: context.dp_decision_svc = DecisionService() -@given('the dp plan executor has a decision hook for the plan') +@given("the dp plan executor has a decision hook for the plan") def step_given_dp_hook_for_plan(context: Context) -> None: """Wire a StrategizeDecisionHook into the PlanExecutor.""" assert context.dp_lifecycle is not None @@ -124,7 +122,7 @@ def step_given_dp_hook_for_plan(context: Context) -> None: ) -@given('the dp plan executor has NO decision hook') +@given("the dp plan executor has NO decision hook") def step_given_dp_no_hook(context: Context) -> None: """Construct a PlanExecutor without a decision hook.""" assert context.dp_lifecycle is not None @@ -180,7 +178,8 @@ def step_when_dp_run_strategize_with_hook(context: Context) -> None: # Patch StrategizeActor.execute to return our controlled decisions with patch.object( - context.dp_plan_executor._strategize_actor, "execute", + context.dp_plan_executor._strategize_actor, + "execute", return_value=StrategizeResult( decision_root_id=test_decisions[0].decision_id, decisions=test_decisions, @@ -201,7 +200,8 @@ def step_when_dp_run_strategize_without_hook(context: Context) -> None: test_decisions = _dp_make_decisions(2) with patch.object( - context.dp_plan_executor._strategize_actor, "execute", + context.dp_plan_executor._strategize_actor, + "execute", return_value=StrategizeResult( decision_root_id=test_decisions[0].decision_id, decisions=test_decisions, @@ -222,7 +222,8 @@ def step_when_dp_run_strategize_failing_hook(context: Context) -> None: test_decisions = _dp_make_decisions(2) with patch.object( - context.dp_plan_executor._strategize_actor, "execute", + context.dp_plan_executor._strategize_actor, + "execute", return_value=StrategizeResult( decision_root_id=test_decisions[0].decision_id, decisions=test_decisions, @@ -372,7 +373,7 @@ def step_given_dp_plan_with_definition(context: Context, definition: str) -> Non context.dp_plan = plan -@given("a dp plan with no steps (\"\") in Strategize-Queued state") +@given('a dp plan with no steps ("") in Strategize-Queued state') def step_given_dp_empty_plan(context: Context) -> None: """Create a mock lifecycle service with an empty definition_of_done. @@ -390,16 +391,10 @@ def step_given_dp_empty_plan(context: Context) -> None: context.dp_plan = plan -@given("a dp plan in Strategize-Queued state with definition \"Build feature\\nAdd tests\"") -def step_given_dp_plan_build_add_tests(context: Context) -> None: - """Alias for the common test scenario definition.""" - step_given_dp_plan_with_definition(context, "Build feature\nAdd tests") - - -@given("a dp plan in Strategize-Queued state with definition \"Do work\"") -def step_given_dp_plan_do_work(context: Context) -> None: - """Alias for the simple do-work scenario definition.""" - step_given_dp_plan_with_definition(context, "Do work") +@given("the dp mock lifecycle service") +def step_given_the_dp_mock_lifecycle(context: Context) -> None: + """Assert the mock lifecycle service from Background is present.""" + assert context.dp_lifecycle is not None @then("the dp JSON fallback in error_details should still be populated") @@ -411,5 +406,3 @@ def step_then_dp_json_still_populated(context: Context) -> None: separate handler is required. """ step_then_dp_json_fallback_populated(context) - - diff --git a/src/cleveragents/application/services/plan_executor.py b/src/cleveragents/application/services/plan_executor.py index 3fe7882a4..6f7c1593d 100644 --- a/src/cleveragents/application/services/plan_executor.py +++ b/src/cleveragents/application/services/plan_executor.py @@ -800,16 +800,25 @@ class PlanExecutor: # same structure that _build_decisions will later reconstruct. if self._decision_hook is not None: persisted_ids: set[str] = set() + strategy_to_db_id: dict[str, str] = {} for decision in result.decisions: if decision.decision_id not in persisted_ids: persisted_ids.add(decision.decision_id) + db_parent_id = ( + strategy_to_db_id.get(decision.parent_id) + if decision.parent_id is not None + else None + ) try: - self._decision_hook.record_strategy_choice( + persisted = self._decision_hook.record_strategy_choice( question=decision.step_text, chosen_option="Yes", confidence_score=None, rationale="", - parent_decision_id=decision.parent_id, + parent_decision_id=db_parent_id, + ) + strategy_to_db_id[decision.decision_id] = ( + persisted.decision_id ) self._logger.debug( "Decision persisted to database during strategize", @@ -817,7 +826,7 @@ class PlanExecutor: decision_id=decision.decision_id, sequence=decision.sequence, ) - except Exception: # noqa: BLE001 + except Exception: # Best-effort persistence: if recording fails, the # JSON fallback in error_details still allows # _build_decisions to reconstruct the tree for diff --git a/src/cleveragents/application/services/strategize_decision_hook.py b/src/cleveragents/application/services/strategize_decision_hook.py index b490bcf95..9900046ac 100644 --- a/src/cleveragents/application/services/strategize_decision_hook.py +++ b/src/cleveragents/application/services/strategize_decision_hook.py @@ -135,7 +135,9 @@ class StrategizeDecisionHook: # Use the per-call parent_id when supplied; fall back to the # instance-level parent from __init__. effective_parent = ( - parent_decision_id if parent_decision_id is not None else self.parent_decision_id + parent_decision_id + if parent_decision_id is not None + else self.parent_decision_id ) self._logger.info( @@ -216,7 +218,9 @@ class StrategizeDecisionHook: ) effective_parent = ( - parent_decision_id if parent_decision_id is not None else self.parent_decision_id + parent_decision_id + if parent_decision_id is not None + else self.parent_decision_id ) self._logger.info( @@ -296,7 +300,9 @@ class StrategizeDecisionHook: ) effective_parent = ( - parent_decision_id if parent_decision_id is not None else self.parent_decision_id + parent_decision_id + if parent_decision_id is not None + else self.parent_decision_id ) self._logger.info( @@ -376,7 +382,9 @@ class StrategizeDecisionHook: ) effective_parent = ( - parent_decision_id if parent_decision_id is not None else self.parent_decision_id + parent_decision_id + if parent_decision_id is not None + else self.parent_decision_id ) self._logger.info(