From f916f88181f1892dee740d5ba28ff008d2800b46 Mon Sep 17 00:00:00 2001 From: CleverThis Date: Wed, 6 May 2026 08:57:22 +0000 Subject: [PATCH] fix(plan): use structured alternatives objects in plan explain output per spec Convert alternatives_considered list of strings to structured objects with index (1-based), description, and chosen fields in _build_explain_dict(). Rename output field from alternatives_considered to alternatives. Update BDD tests in plan_explain.feature, plan_explain_cli_coverage.feature, and plan_explain_steps.py to validate the new structured format. ISSUES CLOSED: #9166 --- CHANGELOG.md | 5 +++++ CONTRIBUTORS.md | 1 + features/plan_explain.feature | 7 +++--- features/plan_explain_cli_coverage.feature | 2 +- features/steps/plan_explain_steps.py | 25 +++++++++++++++++++++- robot/helper_plan_explain.py | 3 ++- src/cleveragents/cli/commands/plan.py | 9 +++++++- 7 files changed, 45 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 082e0f8df..6b69fb38c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,11 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). `session show`, `session delete`, and `session export`, all of which require the full 26-character identifier. The full ULID is now displayed in all output formats (Rich, plain, JSON, YAML, table). +- **`plan explain` output uses structured alternatives objects** (#9166): The `alternatives_considered` + field is replaced with a structured `alternatives` array where each element contains + an `"index"` (1-based), `"description"`, and `"chosen"` (boolean) key. This satisfies the + plan explain specification requiring per-alternative metadata rather than a flat list of + decision strings. ### Security diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index 1b5c41879..bdf4c4ea6 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -31,3 +31,4 @@ Below are some of the specific details of various contributions. * HAL 9000 has contributed comprehensive milestone documentation for v3.6.0 (Advanced Concepts & Deferred Features) and v3.7.0 (TUI Implementation) (PR #9903): split into sub-documents covering context strategies, LLM backends, resource types, A2A rename, container tool execution, scope chain resolution, cost/safety budgets, E2E workflow tests, code review examples, plugin architecture, TUI layout, persona system, reference/command input, session management, configuration, and TuiMaterializer integration. * HAL 9000 has contributed the LLMTraceRepository data-integrity fix (PR #8185 / issue #7505): replaced the unconditional `session.commit()` in `LLMTraceRepository.save()` with a dual-path implementation that respects the UnitOfWork pattern — flushing only when an external session is provided, and flushing + committing + closing when operating standalone. This eliminates premature transaction commits, loss of rollback capability, and a docstring/implementation mismatch. * HAL 9000 has contributed the ACMS Index Data Model and File Traversal Engine (PR #9664 / issue #9579): foundational data structures for indexed context entries with hot/warm/cold/archive storage tier classification, tag system, and a timeout-safe chunked file traversal engine for large projects with 10,000+ files. +* HAL 9000 has contributed the `plan explain` structured alternatives output fix (#9407 / #9166): replaced the flat `alternatives_considered` list with structured `alternatives` objects containing `index`, `description`, and `chosen` fields per plan explain specification, updated BDD step definitions and Robot integration tests accordingly. diff --git a/features/plan_explain.feature b/features/plan_explain.feature index 1332eb511..4e8cb1f5b 100644 --- a/features/plan_explain.feature +++ b/features/plan_explain.feature @@ -13,7 +13,7 @@ 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 not contain key "rationale" And the explain dict should not contain key "context_snapshot" @@ -45,8 +45,9 @@ 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" - And the alternatives list should have 2 items + Then the explain dict should contain key "alternatives" + And each alternative must have keys "index", "description", and "chosen" + And exactly one alternative has chosen=true with 2 total alternatives # ------------------------------------------------------------------ # plan explain - json format diff --git a/features/plan_explain_cli_coverage.feature b/features/plan_explain_cli_coverage.feature index 7c87f6498..406ece3c1 100644 --- a/features/plan_explain_cli_coverage.feature +++ b/features/plan_explain_cli_coverage.feature @@ -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" # ------------------------------------------------------------------ # tree_decisions_cmd - rich tree format diff --git a/features/steps/plan_explain_steps.py b/features/steps/plan_explain_steps.py index f27416a56..f950c5476 100644 --- a/features/steps/plan_explain_steps.py +++ b/features/steps/plan_explain_steps.py @@ -322,10 +322,33 @@ def step_snapshot_has_key(context: Context, key: str) -> None: @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( + 'each alternative must have keys "{keys_csv}"' +) +def step_alternatives_have_required_keys(context: Context, keys_csv: str) -> None: + """Verify all alternatives contain the specified keys.""" + keys = [k.strip() for k in keys_csv.split(",")] + alts = context.pe_explain_dict["alternatives"] + for alt in alts: + for key in keys: + assert key in alt, f"Expected key '{key}' in alternative {alt}" + + +@then("exactly one alternative has chosen=true with {total:d} total alternatives") +def step_exactly_one_chosen(context: Context, total: int) -> None: + """Verify exactly one alternative is marked as chosen.""" + alts = context.pe_explain_dict["alternatives"] + assert len(alts) == total, f"Expected {total} alternatives, got {len(alts)}" + chosen_count = sum(1 for alt in alts if alt.get("chosen") is True) + assert chosen_count == 1, ( + f"Expected exactly 1 chosen alternative, got {chosen_count}" + ) + + @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/robot/helper_plan_explain.py b/robot/helper_plan_explain.py index 215940953..0a416f38a 100644 --- a/robot/helper_plan_explain.py +++ b/robot/helper_plan_explain.py @@ -49,7 +49,8 @@ def _test_explain_format() -> None: assert "decision_id" in data assert "context_snapshot" in data assert "rationale" in data - assert "alternatives_considered" in data + assert "alternatives" in data + assert isinstance(data["alternatives"], list) assert data["question"] == "What to build?" print("plan-explain-ok") diff --git a/src/cleveragents/cli/commands/plan.py b/src/cleveragents/cli/commands/plan.py index b2a7f64d0..f40bc597b 100644 --- a/src/cleveragents/cli/commands/plan.py +++ b/src/cleveragents/cli/commands/plan.py @@ -3909,7 +3909,14 @@ 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": [ + { + "index": i + 1, + "description": alt, + "chosen": alt == decision.chosen_option, + } + for i, alt in enumerate(decision.alternatives_considered) + ], } if show_reasoning: data["rationale"] = decision.rationale -- 2.52.0