From eeadb0ca61369db7b500c148f707eadb0f0de09c Mon Sep 17 00:00:00 2001 From: "Brent E. Edwards" Date: Wed, 18 Mar 2026 19:30:33 +0000 Subject: [PATCH] fix(cli): make plan explain accept plan_id for plan-level explanation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When given an identifier, explain_decision_cmd now tries it as a decision_id first (preserving backward compatibility) and, if not found, falls back to treating it as a plan_id—explaining the root decision of that plan. The parameter is renamed from decision_id to identifier and the help text updated to reflect dual usage. The existing BDD step mock for the "not found" path is updated to also stub list_decisions so the fallback resolves correctly. ISSUES CLOSED: #968 --- .../steps/plan_explain_cli_coverage_steps.py | 1 + features/tdd_plan_explain_plan_id.feature | 2 +- robot/tdd_plan_explain_plan_id.robot | 4 +-- src/cleveragents/cli/commands/plan.py | 26 +++++++++++++++---- 4 files changed, 25 insertions(+), 8 deletions(-) diff --git a/features/steps/plan_explain_cli_coverage_steps.py b/features/steps/plan_explain_cli_coverage_steps.py index 558ea6a0f..36919e9f7 100644 --- a/features/steps/plan_explain_cli_coverage_steps.py +++ b/features/steps/plan_explain_cli_coverage_steps.py @@ -159,6 +159,7 @@ def step_pec_mock_decision_none(context: Context) -> None: context.pec_decision_id = str(ULID()) svc = MagicMock() svc.get_decision.return_value = None + svc.list_decisions.return_value = [] context.pec_container = _mock_container_with_decision_svc(svc) diff --git a/features/tdd_plan_explain_plan_id.feature b/features/tdd_plan_explain_plan_id.feature index 77bb8c4f3..c2dd8579b 100644 --- a/features/tdd_plan_explain_plan_id.feature +++ b/features/tdd_plan_explain_plan_id.feature @@ -1,4 +1,4 @@ -@tdd_expected_fail @tdd_bug @tdd_bug_968 @mock_only +@tdd_bug @tdd_bug_968 @mock_only Feature: TDD Bug #968 — plan explain expects decision_id but test passes plan_id As a developer I want to verify that `plan explain ` succeeds when given a plan ID diff --git a/robot/tdd_plan_explain_plan_id.robot b/robot/tdd_plan_explain_plan_id.robot index ad3e4116c..8f3ca514e 100644 --- a/robot/tdd_plan_explain_plan_id.robot +++ b/robot/tdd_plan_explain_plan_id.robot @@ -17,7 +17,7 @@ TDD Plan Explain Succeeds With Plan ID [Documentation] Verify that ``plan explain `` exits with rc=0 ... when given a plan ID that has associated decisions. ... Bug #968: the command currently exits with rc=1. - [Tags] tdd_expected_fail tdd_bug tdd_bug_968 + [Tags] tdd_bug tdd_bug_968 ${result}= Run Process ${PYTHON} ${HELPER} explain-with-plan-id cwd=${WORKSPACE} timeout=60s on_timeout=kill Log ${result.stdout} Log ${result.stderr} @@ -28,7 +28,7 @@ TDD Plan Explain With Plan ID Shows Root Question [Documentation] Verify that ``plan explain `` output contains ... the root decision question when given a plan ID. ... Bug #968: the command fails before rendering any output. - [Tags] tdd_expected_fail tdd_bug tdd_bug_968 + [Tags] tdd_bug tdd_bug_968 ${result}= Run Process ${PYTHON} ${HELPER} explain-plan-id-shows-question cwd=${WORKSPACE} timeout=60s on_timeout=kill Log ${result.stdout} Log ${result.stderr} diff --git a/src/cleveragents/cli/commands/plan.py b/src/cleveragents/cli/commands/plan.py index 9f16ff103..4c71fcc6e 100644 --- a/src/cleveragents/cli/commands/plan.py +++ b/src/cleveragents/cli/commands/plan.py @@ -3123,9 +3123,9 @@ def _build_explain_dict( @app.command("explain") def explain_decision_cmd( - decision_id: Annotated[ + identifier: Annotated[ str, - typer.Argument(help="Decision ULID to explain"), + typer.Argument(help="Decision or Plan ULID to explain"), ], fmt: Annotated[ str, @@ -3140,14 +3140,30 @@ def explain_decision_cmd( typer.Option("--show-reasoning", help="Include rationale and actor reasoning"), ] = False, ) -> None: - """Explain a single decision in the plan decision tree.""" + """Explain a single decision or the root decision of a plan.""" from cleveragents.application.container import get_container + from cleveragents.application.services.decision_service import ( + DecisionNotFoundError, + ) container = get_container() svc = container.decision_service() - decision = svc.get_decision(decision_id) + + # First, try treating the identifier as a decision_id (backward compat). + decision = None + with suppress(DecisionNotFoundError): + decision = svc.get_decision(identifier) + + # If not found as a decision, try as a plan_id. if decision is None: - console.print(f"[red]Error:[/red] Decision '{decision_id}' not found.") + decisions = svc.list_decisions(identifier) + if decisions: + decision = decisions[0] + + if decision is None: + console.print( + f"[red]Error:[/red] '{identifier}' not found as a decision or plan." + ) raise typer.Exit(1) data = _build_explain_dict( -- 2.52.0