From 29f2383e2d0aba0b1dcfe90ec34f8ce24076f27b Mon Sep 17 00:00:00 2001 From: CleverThis Date: Tue, 14 Apr 2026 12:55:06 +0000 Subject: [PATCH] fix(plan-lifecycle): record prompt_definition as root decision during Strategize Changed PlanLifecycleService.start_strategize() to record a prompt_definition decision as the root instead of strategy_choice. The decision now uses question='What is the plan prompt?' and chosen_option=plan.description, conforming to the decision model spec. Added BDD tests verifying the root decision type, question, and chosen_option match the plan description. ISSUES CLOSED: #9061 --- CHANGELOG.md | 7 + features/plan_lifecycle_bug9061.feature | 25 +++ .../steps/plan_lifecycle_bug9061_steps.py | 174 ++++++++++++++++++ .../services/plan_lifecycle_service.py | 6 +- 4 files changed, 209 insertions(+), 3 deletions(-) create mode 100644 features/plan_lifecycle_bug9061.feature create mode 100644 features/steps/plan_lifecycle_bug9061_steps.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 0cf791e12..fac1dc95d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,13 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **Plan Lifecycle Root Decision Type** (#9061): Fixed `PlanLifecycleService.start_strategize()` + to record a `prompt_definition` decision as the root of the decision tree instead of + `strategy_choice`. The root decision now correctly uses `question="What is the plan prompt?"` + and `chosen_option=plan.description`, ensuring the decision tree accurately reflects the + plan's prompt definition as the foundational decision during the Strategize phase, conforming + to the decision model specification. + - **Automation Profile Silent Fallback** (#8232): `_resolve_profile_for_plan` in `PlanLifecycleService` now raises a clear `ValidationError` when a plan's automation profile name is not a known built-in profile, instead of silently diff --git a/features/plan_lifecycle_bug9061.feature b/features/plan_lifecycle_bug9061.feature new file mode 100644 index 000000000..2738d6354 --- /dev/null +++ b/features/plan_lifecycle_bug9061.feature @@ -0,0 +1,25 @@ +@plan_lifecycle @bug9061 +Feature: Plan lifecycle records prompt_definition as root decision during Strategize + As a developer + I want the plan lifecycle to record a prompt_definition decision as the root during Strategize + So that the decision tree conforms to the specification + + Background: + Given I have a fresh plan lifecycle service for bug 9061 + + Scenario: start_strategize records prompt_definition as root decision + Given a plan lifecycle service with a capturing decision service for bug 9061 + And an action "local/bug9061-action" exists for bug 9061 + And a plan created from "local/bug9061-action" for bug 9061 + When I start strategize for bug 9061 + Then the bug9061 recorded decision type should be "prompt_definition" + And the bug9061 recorded decision should have no parent (root) + And the bug9061 recorded decision question should be "What is the plan prompt?" + And the bug9061 recorded decision chosen_option should equal the plan description + + Scenario: start_strategize root decision chosen_option matches plan description + Given a plan lifecycle service with a capturing decision service for bug 9061 + And an action "local/bug9061-desc" exists for bug 9061 with description "Deploy the application to production" + And a plan created from "local/bug9061-desc" for bug 9061 + When I start strategize for bug 9061 + Then the bug9061 recorded decision chosen_option should be "Deploy the application to production" diff --git a/features/steps/plan_lifecycle_bug9061_steps.py b/features/steps/plan_lifecycle_bug9061_steps.py new file mode 100644 index 000000000..18b9feb8d --- /dev/null +++ b/features/steps/plan_lifecycle_bug9061_steps.py @@ -0,0 +1,174 @@ +"""Step definitions for plan_lifecycle_bug9061.feature. + +Verifies that PlanLifecycleService.start_strategize() records a +prompt_definition decision as the root (not strategy_choice). + +Bug: #9061 - Plan lifecycle records strategy_choice as root decision +instead of prompt_definition during Strategize phase. +""" + +from __future__ import annotations + +from unittest.mock import MagicMock + +from behave import given, then, when +from behave.runner import Context + +from cleveragents.application.services.plan_lifecycle_service import ( + PlanLifecycleService, +) +from cleveragents.config.settings import Settings +from cleveragents.domain.models.core.plan import ( + ProjectLink, +) + +# Background + + +@given("I have a fresh plan lifecycle service for bug 9061") +def step_create_fresh_service_bug9061(context: Context) -> None: + """Create a clean PlanLifecycleService for bug 9061 tests.""" + Settings._instance = None + settings = Settings() + context.service = PlanLifecycleService(settings=settings) + context.error = None + context.captured_calls = [] + + +def _create_action_bug9061( + context: Context, name: str, description: str | None = None +) -> object: + """Create a basic action with sensible defaults.""" + return context.service.create_action( + name=name, + description=description or f"Action {name}", + definition_of_done="Tests pass", + strategy_actor="openai/gpt-4", + execution_actor="openai/gpt-4", + ) + + +@given("a plan lifecycle service with a capturing decision service for bug 9061") +def step_create_service_with_capturing_ds(context: Context) -> None: + """Create a PlanLifecycleService with a DecisionService that captures calls.""" + Settings._instance = None + settings = Settings() + mock_ds = MagicMock() + context.captured_calls = [] + + def _capture_record_decision(**kwargs: object) -> MagicMock: + context.captured_calls.append(dict(kwargs)) + result = MagicMock() + result.decision_id = "01ABCDEFGHIJKLMNOPQRSTUVWX" + result.decision_type = kwargs.get("decision_type") + result.parent_decision_id = kwargs.get("parent_decision_id") + result.question = kwargs.get("question") + result.chosen_option = kwargs.get("chosen_option") + return result + + mock_ds.record_decision.side_effect = _capture_record_decision + context.service = PlanLifecycleService( + settings=settings, + decision_service=mock_ds, + ) + context.mock_decision_service = mock_ds + context.error = None + + +@given('an action "{name}" exists for bug 9061') +def step_create_named_action_bug9061(context: Context, name: str) -> None: + """Create an action with the given name.""" + context.action = _create_action_bug9061(context, name) + context.plan_description = context.action.description + + +@given('an action "{name}" exists for bug 9061 with description "{description}"') +def step_create_named_action_with_desc_bug9061( + context: Context, name: str, description: str +) -> None: + """Create an action with the given name and description.""" + context.action = _create_action_bug9061(context, name, description=description) + context.plan_description = description + + +@given('a plan created from "{action_name}" for bug 9061') +def step_create_plan_from_action_bug9061(context: Context, action_name: str) -> None: + """Create a plan from the named action.""" + context.plan = context.service.use_action( + action_name=action_name, + project_links=[ProjectLink(project_name="proj-bug9061")], + ) + + +@when("I start strategize for bug 9061") +def step_start_strategize_bug9061(context: Context) -> None: + """Start strategize phase.""" + context.error = None + try: + pid = context.plan.identity.plan_id + context.plan = context.service.start_strategize(pid) + except Exception as e: + context.error = e + + +@then('the bug9061 recorded decision type should be "{expected_type}"') +def step_verify_recorded_decision_type(context: Context, expected_type: str) -> None: + """Verify the first recorded decision has the expected type.""" + assert context.error is None, f"Unexpected error: {context.error}" + assert context.captured_calls, "No decision was recorded" + first_call = context.captured_calls[0] + actual_type = str(first_call.get("decision_type", "")) + assert actual_type == expected_type, ( + f"Expected decision_type={expected_type!r}, got {actual_type!r}" + ) + + +@then("the bug9061 recorded decision should have no parent (root)") +def step_verify_recorded_decision_no_parent(context: Context) -> None: + """Verify the first recorded decision has no parent (is root).""" + assert context.captured_calls, "No decision was recorded" + first_call = context.captured_calls[0] + parent_id = first_call.get("parent_decision_id") + assert parent_id is None, ( + f"Expected parent_decision_id=None (root), got {parent_id!r}" + ) + + +@then('the bug9061 recorded decision question should be "{expected_question}"') +def step_verify_recorded_decision_question( + context: Context, expected_question: str +) -> None: + """Verify the first recorded decision has the expected question.""" + assert context.captured_calls, "No decision was recorded" + first_call = context.captured_calls[0] + actual_question = first_call.get("question", "") + assert actual_question == expected_question, ( + f"Expected question={expected_question!r}, got {actual_question!r}" + ) + + +@then("the bug9061 recorded decision chosen_option should equal the plan description") +def step_verify_chosen_option_equals_description( + context: Context, +) -> None: + """Verify the first recorded decision chosen_option equals the plan description.""" + assert context.captured_calls, "No decision was recorded" + first_call = context.captured_calls[0] + actual_chosen = first_call.get("chosen_option", "") + expected = context.plan.description + assert actual_chosen == expected, ( + f"Expected chosen_option={expected!r}, got {actual_chosen!r}" + ) + + +@then('the bug9061 recorded decision chosen_option should be "{expected_option}"') +def step_verify_recorded_decision_chosen_option( + context: Context, expected_option: str +) -> None: + """Verify the first recorded decision chosen_option matches expected value.""" + assert context.captured_calls, "No decision was recorded" + first_call = context.captured_calls[0] + actual_chosen = first_call.get("chosen_option", "") + assert actual_chosen == expected_option, ( + f"Expected chosen_option={expected_option!r}, got {actual_chosen!r}" + ) diff --git a/src/cleveragents/application/services/plan_lifecycle_service.py b/src/cleveragents/application/services/plan_lifecycle_service.py index b3b28aa2d..0adb8b048 100644 --- a/src/cleveragents/application/services/plan_lifecycle_service.py +++ b/src/cleveragents/application/services/plan_lifecycle_service.py @@ -1400,9 +1400,9 @@ class PlanLifecycleService: self._try_record_decision( plan_id=plan_id, - decision_type="strategy_choice", - question="Which strategy should the plan follow?", - chosen_option=f"Begin strategize phase for plan {plan_id}", + decision_type=DecisionType.PROMPT_DEFINITION, + question="What is the plan prompt?", + chosen_option=plan.description, ) return plan -- 2.52.0