diff --git a/CHANGELOG.md b/CHANGELOG.md index d590f89c1..7a76a7292 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,6 +77,13 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Changed +- **Decision Tree Full ULID Display** (#5825): The `agents plan tree` command now + displays full 26-character ULIDs for all decisions instead of truncating them to + 8 characters. This enables users to copy decision IDs directly from tree output + and use them in follow-up CLI commands like `agents plan correct` without manual + ID reconstruction. Added "Decision IDs (for correction)" section with human-readable + labels for easy reference. Applies to both table and rich/plain text output formats. + - **Automation Tracking Format**: All automation tracking issues now use a standardized header format with mandatory `Reporting Interval: (Next report expected: )` declarations, enabling precise staleness detection. diff --git a/features/plan_explain.feature b/features/plan_explain.feature index be1e51320..b3b637e5d 100644 --- a/features/plan_explain.feature +++ b/features/plan_explain.feature @@ -141,3 +141,32 @@ Feature: Plan explain and decision tree CLI commands Given a set of test decisions with superseded entries When I build the decision tree with default options Then the tree should not include superseded decision nodes + + # ------------------------------------------------------------------ + # plan tree - per-type ordinals in decision labels + # ------------------------------------------------------------------ + + Scenario: Decision labels use per-type ordinals + Given a set of test decisions with multiple types for ordinal testing + When I generate decision labels with per-type ordinals + Then the first invariant should be labeled "Invariant 1" + And the second invariant should be labeled "Invariant 2" + And the first strategy should be labeled "Strategy" + And the first parallel should be labeled "Parallel 1" + + Scenario: Decision labels restart counting per decision type + Given a set of test decisions with mixed types + When I generate decision labels with per-type ordinals + Then each decision type should start counting from 1 + And invariants should have sequential numbers + And parallel spawns should have sequential numbers + And spawn decisions should have sequential numbers + + Scenario: Full tree output includes per-type ordinal labels + Given a set of test decisions forming a tree with multiple types + When I build the plain format tree output with decision labels + Then the decision tree output should contain "Invariant 1" + And the decision tree output should contain "Invariant 2" + And the decision tree output should contain "Strategy" + And the decision tree output should not contain "Invariant 6" + And the decision tree output should not contain "Strategy 7" diff --git a/features/steps/plan_explain_steps.py b/features/steps/plan_explain_steps.py index 124be3c71..f27416a56 100644 --- a/features/steps/plan_explain_steps.py +++ b/features/steps/plan_explain_steps.py @@ -14,6 +14,7 @@ from ulid import ULID from cleveragents.cli.commands.plan import ( _build_explain_dict, + _get_decision_label, build_decision_tree, ) from cleveragents.cli.formatting import format_output @@ -448,3 +449,411 @@ def _collect_ids(nodes: list[dict[str, object]], out: list[str]) -> None: for child in children: assert isinstance(child, dict) queue.append(child) + + +# --------------------------------------------------------------------------- +# Given steps - per-type ordinals +# --------------------------------------------------------------------------- + + +@given("a set of test decisions with multiple types for ordinal testing") +def step_decisions_multiple_types(context: Context) -> None: + """Create decisions with multiple invariants, strategies, and parallel spawns.""" + root_id = str(ULID()) + inv1_id = str(ULID()) + inv2_id = str(ULID()) + strat1_id = str(ULID()) + par1_id = str(ULID()) + + context.pe_decisions = [ + Decision( + decision_id=root_id, + plan_id=_PLAN_ID, + sequence_number=0, + decision_type=DecisionType.PROMPT_DEFINITION, + question="What to build?", + chosen_option="REST API", + ), + Decision( + decision_id=inv1_id, + plan_id=_PLAN_ID, + parent_decision_id=root_id, + sequence_number=1, + decision_type=DecisionType.INVARIANT_ENFORCED, + question="Enforce authentication?", + chosen_option="OAuth2", + ), + Decision( + decision_id=inv2_id, + plan_id=_PLAN_ID, + parent_decision_id=root_id, + sequence_number=2, + decision_type=DecisionType.INVARIANT_ENFORCED, + question="Enforce rate limiting?", + chosen_option="100 requests per minute", + ), + Decision( + decision_id=strat1_id, + plan_id=_PLAN_ID, + parent_decision_id=root_id, + sequence_number=3, + decision_type=DecisionType.STRATEGY_CHOICE, + question="Which strategy?", + chosen_option="Microservices", + ), + Decision( + decision_id=par1_id, + plan_id=_PLAN_ID, + parent_decision_id=root_id, + sequence_number=4, + decision_type=DecisionType.SUBPLAN_PARALLEL_SPAWN, + question="Run in parallel?", + chosen_option="Yes", + ), + ] + + +@given("a set of test decisions with mixed types") +def step_decisions_mixed_types(context: Context) -> None: + """Create decisions with invariants, strategies, and spawns.""" + root_id = str(ULID()) + inv1_id = str(ULID()) + inv2_id = str(ULID()) + strat1_id = str(ULID()) + strat2_id = str(ULID()) + spawn1_id = str(ULID()) + spawn2_id = str(ULID()) + + context.pe_decisions = [ + Decision( + decision_id=root_id, + plan_id=_PLAN_ID, + sequence_number=0, + decision_type=DecisionType.PROMPT_DEFINITION, + question="What to build?", + chosen_option="REST API", + ), + Decision( + decision_id=inv1_id, + plan_id=_PLAN_ID, + parent_decision_id=root_id, + sequence_number=1, + decision_type=DecisionType.INVARIANT_ENFORCED, + question="Enforce auth?", + chosen_option="OAuth2", + ), + Decision( + decision_id=inv2_id, + plan_id=_PLAN_ID, + parent_decision_id=root_id, + sequence_number=2, + decision_type=DecisionType.INVARIANT_ENFORCED, + question="Enforce encryption?", + chosen_option="TLS", + ), + Decision( + decision_id=strat1_id, + plan_id=_PLAN_ID, + parent_decision_id=root_id, + sequence_number=3, + decision_type=DecisionType.STRATEGY_CHOICE, + question="Which architecture?", + chosen_option="Microservices", + ), + Decision( + decision_id=strat2_id, + plan_id=_PLAN_ID, + parent_decision_id=root_id, + sequence_number=4, + decision_type=DecisionType.STRATEGY_CHOICE, + question="Which deployment?", + chosen_option="Kubernetes", + ), + Decision( + decision_id=spawn1_id, + plan_id=_PLAN_ID, + parent_decision_id=root_id, + sequence_number=5, + decision_type=DecisionType.SUBPLAN_SPAWN, + question="Spawn auth service?", + chosen_option="Yes", + ), + Decision( + decision_id=spawn2_id, + plan_id=_PLAN_ID, + parent_decision_id=root_id, + sequence_number=6, + decision_type=DecisionType.SUBPLAN_SPAWN, + question="Spawn payment service?", + chosen_option="Yes", + ), + ] + + +@given("a set of test decisions forming a tree with multiple types") +def step_tree_multiple_types(context: Context) -> None: + """Create a complete tree with different decision types.""" + root_id = str(ULID()) + inv1_id = str(ULID()) + inv2_id = str(ULID()) + strat1_id = str(ULID()) + par1_id = str(ULID()) + + context.pe_decisions = [ + Decision( + decision_id=root_id, + plan_id=_PLAN_ID, + sequence_number=0, + decision_type=DecisionType.PROMPT_DEFINITION, + question="What to build?", + chosen_option="REST API", + ), + Decision( + decision_id=inv1_id, + plan_id=_PLAN_ID, + parent_decision_id=root_id, + sequence_number=1, + decision_type=DecisionType.INVARIANT_ENFORCED, + question="Enforce authentication?", + chosen_option="OAuth2", + ), + Decision( + decision_id=inv2_id, + plan_id=_PLAN_ID, + parent_decision_id=root_id, + sequence_number=2, + decision_type=DecisionType.INVARIANT_ENFORCED, + question="Enforce rate limiting?", + chosen_option="100 req/min", + ), + Decision( + decision_id=strat1_id, + plan_id=_PLAN_ID, + parent_decision_id=root_id, + sequence_number=3, + decision_type=DecisionType.STRATEGY_CHOICE, + question="Which framework?", + chosen_option="FastAPI", + ), + Decision( + decision_id=par1_id, + plan_id=_PLAN_ID, + parent_decision_id=root_id, + sequence_number=4, + decision_type=DecisionType.SUBPLAN_PARALLEL_SPAWN, + question="Run in parallel?", + chosen_option="Yes", + ), + ] + + +# --------------------------------------------------------------------------- +# When steps - per-type ordinals +# --------------------------------------------------------------------------- + + +@when("I generate decision labels with per-type ordinals") +def step_generate_labels(context: Context) -> None: + """Generate labels for decisions using per-type ordinals.""" + # Build the per-type ordinal counter (same logic as in the CLI) + type_counts: dict[str, int] = {} + decision_labels: dict[str, str] = {} + + for d in context.pe_decisions: + type_counts[d.decision_type] = type_counts.get(d.decision_type, 0) + 1 + per_type_ordinal = type_counts[d.decision_type] + decision_labels[d.decision_id] = _get_decision_label( + d.decision_type, per_type_ordinal + ) + + context.pe_decision_labels = decision_labels + + +@when("I build the plain format tree output with decision labels") +def step_build_tree_with_labels(context: Context) -> None: + """Build tree output and capture the decision labels section.""" + # Build the tree + filtered = context.pe_decisions + + # Build per-type ordinals (same logic as in the CLI command) + type_counts: dict[str, int] = {} + decision_ordinals: dict[str, int] = {} + for d in filtered: + type_counts[d.decision_type] = type_counts.get(d.decision_type, 0) + 1 + decision_ordinals[d.decision_id] = type_counts[d.decision_type] + + # Capture the decision labels output + labels_section = [] + labels_section.append("Decision IDs (for correction)") + for d in filtered: + type_label = _get_decision_label( + d.decision_type, decision_ordinals[d.decision_id] + ) + labels_section.append(f" {type_label}: {d.decision_id}") + + context.pe_tree_output = "\n".join(labels_section) + + +# --------------------------------------------------------------------------- +# Then steps - per-type ordinals +# --------------------------------------------------------------------------- + + +@then('the first invariant should be labeled "{label}"') +def step_first_invariant_label(context: Context, label: str) -> None: + """Verify the first invariant has the correct label.""" + inv_ids = [ + d.decision_id + for d in context.pe_decisions + if d.decision_type == DecisionType.INVARIANT_ENFORCED + ] + assert len(inv_ids) > 0, "No invariant decisions found" + first_inv = inv_ids[0] + assert context.pe_decision_labels[first_inv] == label, ( + f"Expected first invariant to be '{label}', " + f"got '{context.pe_decision_labels[first_inv]}'" + ) + + +@then('the second invariant should be labeled "{label}"') +def step_second_invariant_label(context: Context, label: str) -> None: + """Verify the second invariant has the correct label.""" + inv_ids = [ + d.decision_id + for d in context.pe_decisions + if d.decision_type == DecisionType.INVARIANT_ENFORCED + ] + assert len(inv_ids) > 1, "Fewer than 2 invariant decisions found" + second_inv = inv_ids[1] + assert context.pe_decision_labels[second_inv] == label, ( + f"Expected second invariant to be '{label}', " + f"got '{context.pe_decision_labels[second_inv]}'" + ) + + +@then('the first strategy should be labeled "{label}"') +def step_first_strategy_label(context: Context, label: str) -> None: + """Verify the first strategy has the correct label (no ordinal).""" + strat_ids = [ + d.decision_id + for d in context.pe_decisions + if d.decision_type == DecisionType.STRATEGY_CHOICE + ] + assert len(strat_ids) > 0, "No strategy decisions found" + first_strat = strat_ids[0] + actual_label = context.pe_decision_labels[first_strat] + assert actual_label == label, ( + f"Expected first strategy to be '{label}', got '{actual_label}'" + ) + + +@then('the first parallel should be labeled "{label}"') +def step_first_parallel_label(context: Context, label: str) -> None: + """Verify the first parallel spawn has the correct label.""" + par_ids = [ + d.decision_id + for d in context.pe_decisions + if d.decision_type == DecisionType.SUBPLAN_PARALLEL_SPAWN + ] + assert len(par_ids) > 0, "No parallel spawn decisions found" + first_par = par_ids[0] + assert context.pe_decision_labels[first_par] == label, ( + f"Expected first parallel to be '{label}', " + f"got '{context.pe_decision_labels[first_par]}'" + ) + + +@then("each decision type should start counting from 1") +def step_each_type_starts_from_one(context: Context) -> None: + """Verify each decision type starts counting from 1.""" + # Check that for each decision type, the ordinal numbers are sequential + type_ordinals: dict[str, list[int]] = {} + + for d in context.pe_decisions: + label = context.pe_decision_labels[d.decision_id] + # Extract the ordinal number from the label (e.g., "Invariant 2" -> 2) + parts = label.split() + if len(parts) > 1 and parts[-1].isdigit(): + ordinal = int(parts[-1]) + type_ordinals.setdefault(d.decision_type, []).append(ordinal) + + # For each type with ordinals, verify they start from 1 + for dtype, ordinals in type_ordinals.items(): + sorted_ordinals = sorted(set(ordinals)) + expected = list(range(1, len(sorted_ordinals) + 1)) + assert sorted_ordinals == expected, ( + f"Decision type {dtype} ordinals {sorted_ordinals} != expected {expected}" + ) + + +@then("invariants should have sequential numbers") +def step_invariants_sequential(context: Context) -> None: + """Verify invariant ordinals are sequential.""" + inv_labels = [ + context.pe_decision_labels[d.decision_id] + for d in context.pe_decisions + if d.decision_type == DecisionType.INVARIANT_ENFORCED + ] + # Extract ordinals from labels + ordinals = [] + for label in inv_labels: + parts = label.split() + if len(parts) > 1 and parts[-1].isdigit(): + ordinals.append(int(parts[-1])) + # Should be [1, 2, ...] in order + expected = list(range(1, len(ordinals) + 1)) + assert ordinals == expected, f"Invariant ordinals {ordinals} != {expected}" + + +@then("parallel spawns should have sequential numbers") +def step_parallel_spawns_sequential(context: Context) -> None: + """Verify parallel spawn ordinals are sequential.""" + par_labels = [ + context.pe_decision_labels[d.decision_id] + for d in context.pe_decisions + if d.decision_type == DecisionType.SUBPLAN_PARALLEL_SPAWN + ] + # Extract ordinals from labels + ordinals = [] + for label in par_labels: + parts = label.split() + if len(parts) > 1 and parts[-1].isdigit(): + ordinals.append(int(parts[-1])) + # Should be [1, 2, ...] in order + expected = list(range(1, len(ordinals) + 1)) + assert ordinals == expected, f"Parallel ordinals {ordinals} != {expected}" + + +@then("spawn decisions should have sequential numbers") +def step_spawns_sequential(context: Context) -> None: + """Verify spawn ordinals are sequential.""" + spawn_labels = [ + context.pe_decision_labels[d.decision_id] + for d in context.pe_decisions + if d.decision_type == DecisionType.SUBPLAN_SPAWN + ] + # Extract ordinals from labels + ordinals = [] + for label in spawn_labels: + parts = label.split() + if len(parts) > 1 and parts[-1].isdigit(): + ordinals.append(int(parts[-1])) + # Should be [1, 2, ...] in order + expected = list(range(1, len(ordinals) + 1)) + assert ordinals == expected, f"Spawn ordinals {ordinals} != {expected}" + + +@then('the decision tree output should contain "{text}"') +def step_decision_tree_output_contains(context: Context, text: str) -> None: + """Verify text is in decision tree output.""" + assert text in context.pe_tree_output, ( + f"Expected '{text}' in output:\n{context.pe_tree_output}" + ) + + +@then('the decision tree output should not contain "{text}"') +def step_decision_tree_output_not_contains(context: Context, text: str) -> None: + """Verify text does not appear in decision tree output.""" + assert text not in context.pe_tree_output, ( + f"Unexpected '{text}' in output:\n{context.pe_tree_output}" + ) diff --git a/src/cleveragents/cli/commands/plan.py b/src/cleveragents/cli/commands/plan.py index d224fa39b..f3a249d6b 100644 --- a/src/cleveragents/cli/commands/plan.py +++ b/src/cleveragents/cli/commands/plan.py @@ -4338,6 +4338,42 @@ def build_decision_tree( return result +def _get_decision_label(decision_type: str, per_type_ordinal: int = 0) -> str: + """Generate a human-readable label for a decision type. + + Args: + decision_type: The type of decision (e.g., "strategy_choice", + "invariant_enforced") + per_type_ordinal: The ordinal number within this decision type + (1st invariant, 2nd invariant, etc., restarting per type). + Defaults to 0 for non-repeating types. + + Returns: + A human-readable label for the decision, formatted as + "{TypeName}" or "{TypeName} {ordinal}" for repeating types. + """ + type_map = { + "prompt_definition": "Root", + "invariant_enforced": "Invariant", + "strategy_choice": "Strategy", + "implementation_choice": "Implementation", + "subplan_spawn": "Spawn", + "subplan_parallel_spawn": "Parallel", + } + + base_label = type_map.get(decision_type, str(decision_type)) + + # For types that can repeat multiple times, add per-type ordinal + if decision_type in ( + "invariant_enforced", + "subplan_spawn", + "subplan_parallel_spawn", + ): + return f"{base_label} {per_type_ordinal}" + + return base_label + + @app.command("tree") def tree_decisions_cmd( plan_id: Annotated[ @@ -4384,7 +4420,7 @@ def tree_decisions_cmd( ) by_id = {d.decision_id: d for d in filtered} table = Table(title="Decision Tree", show_header=True) - table.add_column("ID", max_width=8) + table.add_column("ID", max_width=26) table.add_column("Type") table.add_column("Seq") table.add_column("Question", max_width=40) @@ -4405,7 +4441,7 @@ def tree_decisions_cmd( if node_depth >= depth: continue table.add_row( - d.decision_id[:8], + d.decision_id, str(d.decision_type), str(d.sequence_number), d.question[:40] if len(d.question) > 40 else d.question, @@ -4422,7 +4458,7 @@ def tree_decisions_cmd( # rich / plain - use Rich Tree from rich.tree import Tree as RichTree - rich_tree = RichTree(f"[bold]Plan {plan_id[:8]}...[/bold]") + rich_tree = RichTree(f"[bold]Plan {plan_id}[/bold]") filtered = ( decisions @@ -4430,6 +4466,13 @@ def tree_decisions_cmd( else [d for d in decisions if not d.is_superseded] ) + # Build per-type ordinals (count restarts for each decision type) + type_counts: dict[str, int] = {} + decision_ordinals: dict[str, int] = {} + for d in filtered: + type_counts[d.decision_type] = type_counts.get(d.decision_type, 0) + 1 + decision_ordinals[d.decision_id] = type_counts[d.decision_type] + from collections import deque as _deque filt_ids = {d.decision_id for d in filtered} @@ -4441,7 +4484,7 @@ def tree_decisions_cmd( tree_queue: _deque[tuple[Decision, RichTree, int]] = _deque() for root_d in roots: label = ( - f"[bold]{root_d.decision_id[:8]}[/bold] " + f"[bold]{root_d.decision_id}[/bold] " f"({root_d.decision_type}) " f"Q: {root_d.question[:40]}" ) @@ -4458,11 +4501,43 @@ def tree_decisions_cmd( continue for child in children_map.get(current.decision_id, []): clabel = ( - f"[bold]{child.decision_id[:8]}[/bold] " + f"[bold]{child.decision_id}[/bold] " f"({child.decision_type}) " f"Q: {child.question[:40]}" ) cbranch = parent_branch.add(clabel) tree_queue.append((child, cbranch, cur_depth + 1)) - console.print(rich_tree) + # Output tree + if fmt == OutputFormat.PLAIN: + console.print(rich_tree) + # Add "Decision IDs (for correction)" section for plain format + console.print("\nDecision IDs (for correction)") + for d in filtered: + type_label = _get_decision_label( + d.decision_type, decision_ordinals[d.decision_id] + ) + console.print(f" {type_label}: {d.decision_id}") + else: + # Rich format - output tree and add Decision IDs panel + console.print(rich_tree) + + from rich.panel import Panel + + decision_ids_content = [] + for d in filtered: + type_label = _get_decision_label( + d.decision_type, decision_ordinals[d.decision_id] + ) + decision_ids_content.append( + f"[bold]{type_label}:[/bold] {d.decision_id}" + ) + + if decision_ids_content: + console.print( + Panel( + "\n".join(decision_ids_content), + title="Decision IDs (for correction)", + expand=False, + ) + )