From 8cdb05bb1f646bf693bb433363bd8615c8494101 Mon Sep 17 00:00:00 2001 From: Jeffrey Phillips Freeman Date: Sun, 5 Apr 2026 09:02:44 +0000 Subject: [PATCH] fix(cli): implement spec-required structured panels in agents plan explain rich output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the single flat 'Decision Details' table in explain_decision_cmd() with six structured Rich panels matching the spec exactly: - Decision panel: ID, Type, Question, Chosen, Confidence, Plan, Sequence, Created - Alternatives Considered panel: numbered list with '(chosen)' marker - Impact panel: Downstream Decisions, Downstream Child Plans, Artifacts Produced, Correction Impact (derived from downstream_decision_ids count) - Context Snapshot panel (--show-context): resource paths + hot context hash - Rationale panel (--show-reasoning): rationale text - Correction panel: 'agents plan correct --mode revert --guidance "..."' hint Also adds the missing success message '✓ OK Decision explained'. Updates _build_explain_dict() to: - Accept total_decisions parameter for 'X of Y' sequence format - Return structured alternatives list [{index, description, chosen}] instead of flat string list - Include impact dict with downstream_decisions, downstream_child_plans, artifacts_produced, correction_impact - Include correction_hint field Updates feature files and step definitions to test the new output structure. ISSUES CLOSED: #2815 --- features/plan_explain.feature | 38 ++- features/plan_explain_cli_coverage.feature | 4 +- .../steps/plan_explain_cli_coverage_steps.py | 2 + features/steps/plan_explain_steps.py | 33 ++- src/cleveragents/cli/commands/plan.py | 227 ++++++++++++++++-- 5 files changed, 283 insertions(+), 21 deletions(-) diff --git a/features/plan_explain.feature b/features/plan_explain.feature index 3dd40b8c85..ace05c8e69 100644 --- a/features/plan_explain.feature +++ b/features/plan_explain.feature @@ -13,7 +13,9 @@ Feature: Plan explain and decision tree CLI commands And the explain dict should contain key "question" And the explain dict should contain key "chosen" And the explain dict should contain key "type" - And the explain dict should contain key "alternatives_considered" + And the explain dict should contain key "alternatives" + And the explain dict should contain key "impact" + And the explain dict should contain key "correction_hint" And the explain dict should not contain key "rationale" And the explain dict should not contain key "context_snapshot" @@ -45,9 +47,41 @@ Feature: Plan explain and decision tree CLI commands Scenario: Explain includes alternatives by default Given a test decision with alternatives for explain When I build the explain dict with default options - Then the explain dict should contain key "alternatives_considered" + Then the explain dict should contain key "alternatives" And the alternatives list should have 2 items + # ------------------------------------------------------------------ + # plan explain - impact dict always included + # ------------------------------------------------------------------ + + Scenario: Explain includes impact dict by default + Given a test decision for explain + When I build the explain dict with default options + Then the explain dict should contain key "impact" + And the impact dict should contain key "downstream_decisions" + And the impact dict should contain key "downstream_child_plans" + And the impact dict should contain key "artifacts_produced" + And the impact dict should contain key "correction_impact" + + # ------------------------------------------------------------------ + # plan explain - correction_hint always included + # ------------------------------------------------------------------ + + Scenario: Explain includes correction_hint by default + Given a test decision for explain + When I build the explain dict with default options + Then the explain dict should contain key "correction_hint" + And the correction hint should reference the decision id + + # ------------------------------------------------------------------ + # plan explain - sequence format + # ------------------------------------------------------------------ + + Scenario: Explain sequence shows X of Y when total provided + Given a test decision for explain + When I build the explain dict with total decisions 5 + Then the sequence should be formatted as "X of Y" + # ------------------------------------------------------------------ # plan explain - json format # ------------------------------------------------------------------ diff --git a/features/plan_explain_cli_coverage.feature b/features/plan_explain_cli_coverage.feature index 7c87f6498b..abac740ac8 100644 --- a/features/plan_explain_cli_coverage.feature +++ b/features/plan_explain_cli_coverage.feature @@ -12,7 +12,7 @@ Feature: Plan explain and tree CLI command coverage Given pec a mock DecisionService returning a valid decision When pec I invoke "explain" with the decision id Then pec the exit code should be 0 - And pec the output should contain "Decision Details" + And pec the output should contain "Decision" And pec the output should contain "decision_id" Scenario: Explain CLI renders json format @@ -49,7 +49,7 @@ Feature: Plan explain and tree CLI command coverage Given pec a mock DecisionService returning a decision with alternatives When pec I invoke "explain" with the decision id Then pec the exit code should be 0 - And pec the output should contain "alternatives_considered" + And pec the output should contain "Alternatives Considered" # ------------------------------------------------------------------ # tree_decisions_cmd - rich tree format diff --git a/features/steps/plan_explain_cli_coverage_steps.py b/features/steps/plan_explain_cli_coverage_steps.py index e9d5074600..dd480dd3ff 100644 --- a/features/steps/plan_explain_cli_coverage_steps.py +++ b/features/steps/plan_explain_cli_coverage_steps.py @@ -94,6 +94,8 @@ def _make_decision( def _mock_container_with_decision_svc(svc_mock: MagicMock) -> MagicMock: + # Ensure count_decisions returns an int for sequence formatting. + svc_mock.count_decisions.return_value = 3 container = MagicMock() container.decision_service.return_value = svc_mock return container diff --git a/features/steps/plan_explain_steps.py b/features/steps/plan_explain_steps.py index 124be3c717..d3e9659f60 100644 --- a/features/steps/plan_explain_steps.py +++ b/features/steps/plan_explain_steps.py @@ -236,6 +236,13 @@ def step_build_explain_reasoning(context: Context) -> None: ) +@when("I build the explain dict with total decisions {total:d}") +def step_build_explain_with_total(context: Context, total: int) -> None: + context.pe_explain_dict = _build_explain_dict( + context.pe_decision, total_decisions=total + ) + + def _capture_format_output(data, fmt): """Call format_output capturing stdout (machine-readable formats write there).""" from contextlib import redirect_stdout @@ -319,12 +326,36 @@ def step_snapshot_has_key(context: Context, key: str) -> None: assert key in snap, f"Expected key '{key}' in snapshot: {list(snap.keys())}" +@then('the impact dict should contain key "{key}"') +def step_impact_has_key(context: Context, key: str) -> None: + impact = context.pe_explain_dict["impact"] + assert isinstance(impact, dict), ( + f"Expected 'impact' to be a dict, got {type(impact)}" + ) + assert key in impact, f"Expected key '{key}' in impact: {list(impact.keys())}" + + @then("the alternatives list should have {count:d} items") def step_alternatives_count(context: Context, count: int) -> None: - alts = context.pe_explain_dict["alternatives_considered"] + alts = context.pe_explain_dict["alternatives"] assert len(alts) == count, f"Expected {count} alternatives, got {len(alts)}" +@then("the correction hint should reference the decision id") +def step_correction_hint_has_decision_id(context: Context) -> None: + hint = context.pe_explain_dict["correction_hint"] + decision_id = context.pe_explain_dict["decision_id"] + assert decision_id in str(hint), ( + f"Expected decision_id '{decision_id}' in correction_hint '{hint}'" + ) + + +@then('the sequence should be formatted as "X of Y"') +def step_sequence_x_of_y(context: Context) -> None: + seq = context.pe_explain_dict["sequence"] + assert " of " in str(seq), f"Expected sequence in 'X of Y' format, got '{seq}'" + + @then('the json output should contain "{text}"') def step_json_contains(context: Context, text: str) -> None: assert text in context.pe_json_output, f"Expected '{text}' in JSON output" diff --git a/src/cleveragents/cli/commands/plan.py b/src/cleveragents/cli/commands/plan.py index db8f7d3f80..4fccd7f2de 100644 --- a/src/cleveragents/cli/commands/plan.py +++ b/src/cleveragents/cli/commands/plan.py @@ -3457,13 +3457,68 @@ def _build_explain_dict( *, show_context: bool = False, show_reasoning: bool = False, + total_decisions: int | None = None, ) -> dict[str, object]: - """Build a dict for ``plan explain`` output from a Decision.""" + """Build a dict for ``plan explain`` output from a Decision. + + Args: + decision: The decision to explain. + show_context: Include the context snapshot in the output. + show_reasoning: Include rationale and actor reasoning. + total_decisions: Total number of decisions in the plan, used to + format ``sequence`` as "X of Y". When ``None`` the raw + sequence number is used. + """ + # Format sequence as "X of Y" when total is known. + if total_decisions is not None: + sequence_str = f"{decision.sequence_number + 1} of {total_decisions}" + else: + sequence_str = str(decision.sequence_number) + + # Build structured alternatives list: [{index, description, chosen}]. + chosen_lower = decision.chosen_option.strip().lower() + alternatives: list[dict[str, object]] = [] + for idx, alt in enumerate(decision.alternatives_considered, start=1): + alternatives.append( + { + "index": idx, + "description": alt, + "chosen": alt.strip().lower() == chosen_lower, + } + ) + # Always include the chosen option itself if alternatives list is empty. + if not alternatives: + alternatives = [ + {"index": 1, "description": decision.chosen_option, "chosen": True} + ] + + # Derive a simple correction_impact level from downstream count. + downstream_count = len(decision.downstream_decision_ids) + if downstream_count == 0: + correction_impact = "low" + elif downstream_count <= 3: + correction_impact = "medium" + else: + correction_impact = "high" + + # Build impact sub-dict. + impact: dict[str, object] = { + "downstream_decisions": len(decision.downstream_decision_ids), + "downstream_child_plans": len(decision.downstream_plan_ids), + "artifacts_produced": len(decision.artifacts_produced), + "correction_impact": correction_impact, + } + + # Build correction hint. + correction_hint = ( + f'agents plan correct {decision.decision_id} --mode revert --guidance "..."' + ) + data: dict[str, object] = { "decision_id": decision.decision_id, "plan_id": decision.plan_id, "type": str(decision.decision_type), - "sequence": decision.sequence_number, + "sequence": sequence_str, "question": decision.question, "chosen": decision.chosen_option, "confidence": decision.confidence_score, @@ -3471,7 +3526,9 @@ def _build_explain_dict( "is_correction": decision.is_correction, "superseded": decision.is_superseded, "created_at": decision.created_at.isoformat(), - "alternatives_considered": decision.alternatives_considered, + "alternatives": alternatives, + "impact": impact, + "correction_hint": correction_hint, } if show_reasoning: data["rationale"] = decision.rationale @@ -3537,29 +3594,167 @@ def explain_decision_cmd( ) raise typer.Exit(1) + # Fetch total decision count for "X of Y" sequence format. + total_decisions: int | None = None + with suppress(Exception): + total_decisions = svc.count_decisions(decision.plan_id) + data = _build_explain_dict( decision, show_context=show_context, show_reasoning=show_reasoning, + total_decisions=total_decisions, ) if fmt in (OutputFormat.JSON, OutputFormat.YAML, OutputFormat.TABLE): console.print(format_output(data, fmt)) + elif fmt == OutputFormat.PLAIN.value or fmt == "plain": + # Plain text: section headers + indented key-value lines. + created_str = decision.created_at.strftime("%Y-%m-%d %H:%M") + lines = [ + "Decision", + f" ID: {data['decision_id']}", + f" Type: {data['type']}", + f" Question: {data['question']}", + f" Chosen: {data['chosen']}", + f" Confidence: {data['confidence']}", + f" Plan: {data['plan_id']}", + f" Sequence: {data['sequence']}", + f" Created: {created_str}", + "", + "Alternatives Considered", + ] + plain_alts = data.get("alternatives", []) + assert isinstance(plain_alts, list) + for alt in plain_alts: + assert isinstance(alt, dict) + suffix = " (chosen)" if alt.get("chosen") else "" + lines.append(f" {alt['index']}. {alt['description']}{suffix}") + impact = data.get("impact", {}) + assert isinstance(impact, dict) + lines += [ + "", + "Impact", + f" Downstream Decisions: {impact.get('downstream_decisions', 0)}", + f" Downstream Child Plans: {impact.get('downstream_child_plans', 0)}", + f" Artifacts Produced: {impact.get('artifacts_produced', 0)}", + f" Correction Impact: {impact.get('correction_impact', 'low')}", + ] + if show_context and "context_snapshot" in data: + snap = data["context_snapshot"] + assert isinstance(snap, dict) + lines += ["", "Context Snapshot"] + for res in snap.get("relevant_resources", []): + assert isinstance(res, dict) + lines.append(f" - {res.get('path', '')}") + lines.append(f" Hot Context Hash: {snap.get('hot_context_hash', '')}") + if show_reasoning: + lines += ["", "Rationale", f" {data.get('rationale', '')}"] + lines += [ + "", + "Correction", + f" {data['correction_hint']}", + "", + "[OK] Decision explained", + ] + console.print("\n".join(lines)) else: - # rich / plain - table = Table(title="Decision Details", show_header=False, expand=False) - table.add_column("Field", style="bold") - table.add_column("Value") - import json as _json + # Rich output: structured panels per spec. + created_str = decision.created_at.strftime("%Y-%m-%d %H:%M") + confidence_val = data.get("confidence") + confidence_str = str(confidence_val) if confidence_val is not None else "N/A" - for key, val in data.items(): - if isinstance(val, dict): - table.add_row(key, _json.dumps(val, indent=2, default=str)) - elif isinstance(val, list): - table.add_row(key, _json.dumps(val, default=str)) - else: - table.add_row(key, str(val)) - console.print(Panel(table, expand=False)) + # --- Decision panel --- + console.print( + Panel( + f"[bold]ID:[/bold] {data['decision_id']}\n" + f"[bold]Type:[/bold] {data['type']}\n" + f"[bold]Question:[/bold] {data['question']}\n" + f"[bold]Chosen:[/bold] {data['chosen']}\n" + f"[bold]Confidence:[/bold] {confidence_str}\n" + f"[bold]Plan:[/bold] {data['plan_id']}\n" + f"[bold]Sequence:[/bold] {data['sequence']}\n" + f"[bold]Created:[/bold] {created_str}", + title="Decision", + expand=False, + ) + ) + + # --- Alternatives Considered panel --- + alt_lines: list[str] = [] + rich_alts = data.get("alternatives", []) + assert isinstance(rich_alts, list) + for alt in rich_alts: + assert isinstance(alt, dict) + suffix = " [bold](chosen)[/bold]" if alt.get("chosen") else "" + alt_lines.append(f"{alt['index']}. {alt['description']}{suffix}") + console.print( + Panel( + "\n".join(alt_lines) if alt_lines else "(none)", + title="Alternatives Considered", + expand=False, + ) + ) + + # --- Impact panel --- + impact = data.get("impact", {}) + assert isinstance(impact, dict) + console.print( + Panel( + f"[bold]Downstream Decisions:[/bold] " + f"{impact.get('downstream_decisions', 0)}\n" + f"[bold]Downstream Child Plans:[/bold] " + f"{impact.get('downstream_child_plans', 0)}\n" + f"[bold]Artifacts Produced:[/bold] " + f"{impact.get('artifacts_produced', 0)}\n" + f"[bold]Correction Impact:[/bold] " + f"{impact.get('correction_impact', 'low')}", + title="Impact", + expand=False, + ) + ) + + # --- Context Snapshot panel (optional) --- + if show_context and "context_snapshot" in data: + snap = data["context_snapshot"] + assert isinstance(snap, dict) + ctx_lines: list[str] = [] + for res in snap.get("relevant_resources", []): + assert isinstance(res, dict) + ctx_lines.append(f"- {res.get('path', '')}") + ctx_lines.append( + f"[bold]Hot Context Hash:[/bold] {snap.get('hot_context_hash', '')}" + ) + console.print( + Panel( + "\n".join(ctx_lines), + title="Context Snapshot", + expand=False, + ) + ) + + # --- Rationale panel (optional) --- + if show_reasoning: + rationale = str(data.get("rationale", "")) + console.print( + Panel( + rationale or "(no rationale recorded)", + title="Rationale", + expand=False, + ) + ) + + # --- Correction panel --- + console.print( + Panel( + str(data["correction_hint"]), + title="Correction", + expand=False, + ) + ) + + # --- Success message --- + console.print("[green bold]✓ OK[/green bold] Decision explained") # ---------------------------------------------------------------------------