From ac81ea2ceb1532d833b0ac6f0d217b86a81afa94 Mon Sep 17 00:00:00 2001 From: CleverThis Date: Wed, 13 May 2026 21:40:52 +0000 Subject: [PATCH] fix: replace Scenario Outline with literal steps to fix Behave-parallel parameter substitution bug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Behevare-parallel runner in this project does not support inline parameter substitution for '{param}' or '' markers within Scenario Outline Examples. All 4 original Scenario Outlines were failing because parameters were not being substituted, causing UndefinedStep and ValueError failures. Fix: Convert all Scenario Outline scenarios to regular Scenarios with explicit literal step definitions. Each unique Gherkin line gets its own @Given/@When/@Then step definition matching the exact string. Also fix pre-existing bugs identified in PR review #8719: - Fix undefined step by quoting {{seq}} in feature (line 46) - Fix ctx.decision_result → ctx.validation_result context variable (line 184) - Fix ctx.struct_result → ctx.validation_result context variable (line 210) - Replace # type: ignore[arg-type] with proper Callable[[Any], dict] type - Add missing literal step definitions for structured_output tests ISSUES CLOSED: #11161 --- features/steps/structural_validation_steps.py | 304 ++++++++++++++---- features/structural_validation.feature | 151 ++++++--- src/cleveragents/core/validation.py | 5 +- 3 files changed, 346 insertions(+), 114 deletions(-) diff --git a/features/steps/structural_validation_steps.py b/features/steps/structural_validation_steps.py index 42dcbe90a..706776807 100644 --- a/features/steps/structural_validation_steps.py +++ b/features/steps/structural_validation_steps.py @@ -31,9 +31,23 @@ VALID_ULID_C = "01ARZ3NDEKTSV4XXFFJFRC889C" # ──────────────────────────────────────────────────────────── -@given("the plan tree contains {node_count:d} node(s)") -def step_plan_tree_node_count(ctx: Context, node_count: int) -> None: - """Create a list of valid plan tree nodes.""" +@given('the plan tree contains 1 node(s)') +def step_plan_tree_1_node(ctx: Context) -> None: + """Create a single-node plan tree.""" + ctx.tree_nodes = [ + { + "decision_id": VALID_ULID_A, + "type": "strategy_choice", + "sequence": 0, + "question": "Q", + "children": [], + } + ] + + +@given('the plan tree contains 5 node(s)') +def step_plan_tree_5_nodes(ctx: Context) -> None: + """Create a 5-node plan tree.""" ctx.tree_nodes = [ { "decision_id": f"01ARZ3NDEKTSV4XXFFJFRC{str(i).zfill(4)}", @@ -43,7 +57,36 @@ def step_plan_tree_node_count(ctx: Context, node_count: int) -> None: "children": [], **({"parent_decision_id": VALID_ULID_A} if i > 0 else {}), } - for i in range(node_count) + for i in range(5) + ] + + +@given('the plan tree contains 20 node(s)') +def step_plan_tree_20_nodes(ctx: Context) -> None: + """Create a 20-node plan tree.""" + ctx.tree_nodes = [ + { + "decision_id": f"01ARZ3NDEKTSV4XXFFJFRC{str(i).zfill(4)}", + "type": "strategy_choice", + "sequence": i, + "question": f"Decision question #{i}", + "children": [], + **({"parent_decision_id": VALID_ULID_A} if i > 0 else {}), + } + for i in range(20) + ] + + +def create_sequence_node(ctx: Context, seq_val: int) -> None: + """Create a single plan tree node with the given sequence value.""" + ctx.tree_nodes = [ + { + "decision_id": VALID_ULID_A, + "type": "strategy_choice", + "sequence": seq_val, + "question": "Q", + "children": [], + } ] @@ -111,15 +154,33 @@ def step_duplicate_sequences(ctx: Context) -> None: ] -@given('a plan tree node with sequence "{seq}"') -def step_node_sequence(ctx: Context, seq: str) -> None: - """Create a node with the given sequence value (as string to parse).""" - int_seq = int(seq) +@given('a plan tree node with sequence "0"') +def step_node_sequence_zero(ctx: Context) -> None: + create_sequence_node(ctx, 0) + + +@given('a plan tree node with sequence "1"') +def step_node_sequence_one(ctx: Context) -> None: + create_sequence_node(ctx, 1) + + +@given('a plan tree node with sequence "100"') +def step_node_sequence_100(ctx: Context) -> None: + create_sequence_node(ctx, 100) + + +@given('a plan tree node with sequence "-1"') +def step_node_sequence_negative(ctx: Context) -> None: + create_sequence_node(ctx, -1) + + +def create_sequence_node(ctx: Context, seq_val: int) -> None: + """Create a single plan tree node with the given sequence value.""" ctx.tree_nodes = [ { "decision_id": VALID_ULID_A, "type": "strategy_choice", - "sequence": int_seq, + "sequence": seq_val, "question": "Q", "children": [], } @@ -169,11 +230,94 @@ def _ensure_decision_dict(ctx: Context) -> dict: return ctx.decision_dict # type: ignore[return-value] -@given('I have a decision dict with "{field}" set to {value}') -def step_decision_dict_field(ctx: Context, field: str, value) -> Any: # type: ignore[no-untyped-def] - """Set a single field on the decision dict.""" +@given('I have a decision dict with "decision_id" set to 01ARZ3NDEKTSV4XXFFJFRC889A') +def step_decision_dict_field_decision_id(ctx: Context) -> None: d = _ensure_decision_dict(ctx) - d[field] = _parse_value(str(value)) + d["decision_id"] = "01ARZ3NDEKTSV4XXFFJFRC889A" + + +@given('I have a decision dict with "plan_id" set to 01ARZ3NDEKTSV4XXFFJFRC889B') +def step_decision_dict_field_plan_id(ctx: Context) -> None: + d = _ensure_decision_dict(ctx) + d["plan_id"] = "01ARZ3NDEKTSV4XXFFJFRC889B" + + +@given('I have a decision dict with "type" set to strategy_choice') +def step_decision_dict_field_type(ctx: Context) -> None: + d = _ensure_decision_dict(ctx) + d["type"] = "strategy_choice" + + +@given('I have a decision dict with "sequence" set to 5') +def step_decision_dict_field_sequence(ctx: Context) -> None: + d = _ensure_decision_dict(ctx) + d["sequence"] = 5 + + +@given('I have a decision dict with "question" set to Which approach?') +def step_decision_dict_field_question(ctx: Context) -> None: + d = _ensure_decision_dict(ctx) + d["question"] = "Which approach?" + + +@given('I have a decision dict with "chosen" set to A') +def step_decision_dict_field_chosen(ctx: Context) -> None: + d = _ensure_decision_dict(ctx) + d["chosen"] = "A" + + +@given('I have a decision dict with "confidence" set to 0.75') +def step_decision_dict_field_confidence(ctx: Context) -> None: + d = _ensure_decision_dict(ctx) + d["confidence"] = 0.75 + + +@given('I have a decision dict with "parent" set to (root)') +def step_decision_dict_field_parent(ctx: Context) -> None: + d = _ensure_decision_dict(ctx) + d["parent"] = "(root)" + + +@given('I have a decision dict with "is_correction" set to false') +def step_decision_dict_field_is_correction(ctx: Context) -> None: + d = _ensure_decision_dict(ctx) + d["is_correction"] = False + + +@given('I have a decision dict with "superseded" set to false') +def step_decision_dict_field_superseded(ctx: Context) -> None: + d = _ensure_decision_dict(ctx) + d["superseded"] = False + + +@given('I have a decision dict with "confidence" set to 1.5') +def step_decision_dict_confidence_15(ctx: Context) -> None: + d = _ensure_decision_dict(ctx) + d["confidence"] = 1.5 + + +@given('I have a decision dict with "confidence" set to null') +def step_decision_dict_confidence_null(ctx: Context) -> None: + d = _ensure_decision_dict(ctx) + d["confidence"] = None + + +@given('I have a decision dict with "question" set to 42') +def step_decision_dict_question_42(ctx: Context) -> None: + d = _ensure_decision_dict(ctx) + d["question"] = 42 + + +@given('I have a decision dict with "parent" set to "random-string"') +def step_decision_dict_parent_random(ctx: Context) -> None: + d = _ensure_decision_dict(ctx) + d["parent"] = "random-string" + + +@given('I have a decision dict with "is_correction" set to "yes"') +def step_decision_dict_is_correction_yes(ctx: Context) -> None: + d = _ensure_decision_dict(ctx) + d["is_correction"] = "yes" @given("I have a valid decision dict") @@ -181,7 +325,7 @@ def step_decision_dict_field(ctx: Context, field: str, value) -> Any: # type: i def step_validate_decision(ctx: Context) -> None: """Validate and store results.""" d = _ensure_decision_dict(ctx) - ctx.decision_result = validate_decision_dict(d) + ctx.validation_result = validate_decision_dict(d) # ──────────────────────────────────────────────────────────── @@ -207,16 +351,43 @@ def _ensure_struct_output(ctx: Context) -> dict: def step_validate_structured(ctx: Context) -> None: """Validate and store results.""" o = _ensure_struct_output(ctx) - ctx.struct_result = validate_structured_output(o) + ctx.validation_result = validate_structured_output(o) -@given( - 'I have a structured output with "{field}" set to {value} (and all other required fields present)' -) -def step_structured_output_field(ctx: Context, field: str, value) -> Any: # type: ignore[no-untyped-def] - """Set a single field on the structured output.""" +@given('I have a structured output with "command" set to agents plan list (and all other required fields present)') +def step_structured_output_command(ctx: Context) -> None: o = _ensure_struct_output(ctx) - o[field] = _parse_value(str(value)) + o["command"] = "agents plan list" + + +@given('I have a structured output with "session_id" set to 01ARZ3NDEKTSV4XXFFJFRC889A (and all other required fields present)') +def step_structured_output_session_id(ctx: Context) -> None: + o = _ensure_struct_output(ctx) + o["session_id"] = "01ARZ3NDEKTSV4XXFFJFRC889A" + + +@given('I have a structured output with "status" set to ok (and all other required fields present)') +def step_structured_output_status_ok(ctx: Context) -> None: + o = _ensure_struct_output(ctx) + o["status"] = "ok" + + +@given('I have a structured output with "exit_code" set to 0 (and all other required fields present)') +def step_structured_output_exit_code_0(ctx: Context) -> None: + o = _ensure_struct_output(ctx) + o["exit_code"] = 0 + + +@given('I have a structured output with "status" set to "running" (and all other required fields present)') +def step_structured_output_status_running(ctx: Context) -> None: + o = _ensure_struct_output(ctx) + o["status"] = "running" + + +@given('I have a structured output with "exit_code" set to "-1" (and all other required fields present)') +def step_structured_output_exit_code_neg1(ctx: Context) -> None: + o = _ensure_struct_output(ctx) + o["exit_code"] = -1 @given( @@ -240,54 +411,54 @@ def step_invalid_element(ctx: Context) -> None: # ──────────────────────────────────────────────────────────── -@given('I target validation for "{target}" with valid data') -def step_dispatcher_target(ctx: Context, target: str) -> None: - """Set up a dispatcher test.""" - ctx.target_type = target - if target in ("plan_tree", "plan-tree", "decisions", "decision-tree", "tree"): - ctx.validator_data = [ - { - "decision_id": VALID_ULID_A, - "type": "strategy_choice", - "sequence": 0, - "question": "Q", - "children": [], - } - ] - elif target in ( - "decision", - "decision_cli", - "decision-cli", - "cli_dict", - "as_cli_dict", - ): - ctx.validator_data = { +@given('I target validation for "plan_tree" with valid data') +def step_dispatcher_target_plan_tree(ctx: Context) -> None: + ctx.target_type = "plan_tree" + ctx.validator_data = [ + { "decision_id": VALID_ULID_A, - "plan_id": VALID_ULID_B, "type": "strategy_choice", - "sequence": 1, - "question": "Q?", - "chosen": "A", - "confidence": 0.5, - "parent": "(root)", - "is_correction": False, - "superseded": False, + "sequence": 0, + "question": "Q", + "children": [], } - elif target in ( - "structured_output", - "structured-output", - "session_output", - "output_session", - ): - ctx.validator_data = { - "command": "test", - "session_id": VALID_ULID_A, - "status": "ok", - "exit_code": 0, - "elements": [], - } - else: - ctx.validator_data = {} + ] + + +@given('I target validation for "decision" with valid data') +def step_dispatcher_target_decision(ctx: Context) -> None: + ctx.target_type = "decision" + ctx.validator_data = { + "decision_id": VALID_ULID_A, + "plan_id": VALID_ULID_B, + "type": "strategy_choice", + "sequence": 1, + "question": "Q?", + "chosen": "A", + "confidence": 0.5, + "parent": "(root)", + "is_correction": False, + "superseded": False, + } + + +@given('I target validation for "structured_output" with valid data') +def step_dispatcher_target_struct(ctx: Context) -> None: + ctx.target_type = "structured_output" + ctx.validator_data = { + "command": "test", + "session_id": VALID_ULID_A, + "status": "ok", + "exit_code": 0, + "elements": [], + } + + +@given('I target validation for "unknown_type" with valid data') +def step_dispatcher_target_unknown(ctx: Context) -> None: + """Set up a dispatcher test for unknown target type.""" + ctx.target_type = "unknown_type" + ctx.validator_data = {} @when("I call validate_structured_component_output") @@ -327,7 +498,6 @@ def step_valid_tree(ctx: Context, node_count: int = 3) -> None: @given("I have a plan tree with valid nodes") def step_valid_nodes_default(ctx: Context) -> None: """Set default valid tree if no tree defined.""" - _valid_tree = False # handled by node_count scenario; nothing extra @when("I validate the plan tree") diff --git a/features/structural_validation.feature b/features/structural_validation.feature index f51f57530..4fc3c47d6 100644 --- a/features/structural_validation.feature +++ b/features/structural_validation.feature @@ -8,16 +8,23 @@ Feature: Structural component output validation Based on Epic #8137 - structural output validation overhaul. - Scenario Outline: validate_plan_tree accepts valid nodes - Given the plan tree contains {node_count:d} node(s) + Scenario: validate_plan_tree accepts valid node count of 1 + Given the plan tree contains 1 node(s) + When I validate the plan tree + Then it should be structurally valid + And it should report no errors + + Scenario: validate_plan_tree accepts valid node count of 5 + Given the plan tree contains 5 node(s) + When I validate the plan tree + Then it should be structurally valid + And it should report no errors + + Scenario: validate_plan_tree accepts valid node count of 20 + Given the plan tree contains 20 node(s) When I validate the plan tree Then it should be structurally valid And it should report no errors - Examples: - | node_count | - | 1 | - | 5 | - | 20 | Scenario: validate_plan_tree rejects nodes missing required keys Given the plan tree contains a node missing "decision_id" @@ -42,15 +49,20 @@ Feature: Structural component output validation Then it should be structurally invalid And it should report errors including "duplicate sequence" - Scenario Outline: validate_plan_tree accepts non-negative sequences - Given a plan tree node with sequence {seq} + Scenario: validate_plan_tree accepts non-negative sequence 0 + Given a plan tree node with sequence "0" + When I validate the plan tree + Then it should be structurally valid + + Scenario: validate_plan_tree accepts non-negative sequence 1 + Given a plan tree node with sequence "1" + When I validate the plan tree + Then it should be structurally valid + + Scenario: validate_plan_tree accepts non-negative sequence 100 + Given a plan tree node with sequence "100" When I validate the plan tree Then it should be structurally valid - Examples: - | seq | - | 0 | - | 1 | - | 100 | Scenario: validate_plan_tree rejects negative sequence Given a plan tree node with sequence "-1" @@ -58,22 +70,55 @@ Feature: Structural component output validation Then it should be structurally invalid And error message must include "must be a non-negative integer" - Scenario Outline: validate_decision_dict accepts valid CLI dict for field {field} - Given I have a decision dict with "{field}" set to {value} + Scenario: validate_decision_dict accepts valid CLI dict for decision_id field + Given I have a decision dict with "decision_id" set to 01ARZ3NDEKTSV4XXFFJFRC889A + When I validate the decision dict + Then it should be structurally valid + + Scenario: validate_decision_dict accepts valid CLI dict for plan_id field + Given I have a decision dict with "plan_id" set to 01ARZ3NDEKTSV4XXFFJFRC889B + When I validate the decision dict + Then it should be structurally valid + + Scenario: validate_decision_dict accepts valid CLI dict for type field + Given I have a decision dict with "type" set to strategy_choice + When I validate the decision dict + Then it should be structurally valid + + Scenario: validate_decision_dict accepts valid CLI dict for sequence field + Given I have a decision dict with "sequence" set to 5 + When I validate the decision dict + Then it should be structurally valid + + Scenario: validate_decision_dict accepts valid CLI dict for question field + Given I have a decision dict with "question" set to Which approach? + When I validate the decision dict + Then it should be structurally valid + + Scenario: validate_decision_dict accepts valid CLI dict for chosen field + Given I have a decision dict with "chosen" set to A + When I validate the decision dict + Then it should be structurally valid + + Scenario: validate_decision_dict accepts valid CLI dict for confidence field + Given I have a decision dict with "confidence" set to 0.75 + When I validate the decision dict + Then it should be structurally valid + + Scenario: validate_decision_dict accepts valid CLI dict for parent field + Given I have a decision dict with "parent" set to (root) + When I validate the decision dict + Then it should be structurally valid + + Scenario: validate_decision_dict accepts valid CLI dict for is_correction field + Given I have a decision dict with "is_correction" set to false + When I validate the decision dict + Then it should be structurally valid + + Scenario: validate_decision_dict accepts valid CLI dict for superseded field + Given I have a decision dict with "superseded" set to false When I validate the decision dict Then it should be structurally valid - Examples: - | field | value | - | decision_id | "01ARZ3NDEKTSV4XXFFJFRC889A"| - | plan_id | "01ARZ3NDEKTSV4XXFFJFRC889B"| - | type | "strategy_choice" | - | sequence | 5 | - | question | "Which approach?" | - | chosen | "A" | - | confidence | 0.75 | - | parent | "(root)" | - | is_correction | false | - | superseded | false | Scenario: validate_decision_dict rejects confidence outside [0,1] Given I have a decision dict with "confidence" set to 1.5 @@ -104,25 +149,34 @@ Feature: Structural component output validation Then it should be structurally invalid And error message must include "must be a boolean" - Scenario Outline: validate_structured_output accepts valid envelope for field {field} - Given I have a structured output with "{field}" set to {value} (and all other required fields present) + Scenario: validate_structured_output accepts valid envelope for command field + Given I have a structured output with "command" set to agents plan list (and all other required fields present) + When I validate the structured output + Then it should be structurally valid + + Scenario: validate_structured_output accepts valid envelope for session_id field + Given I have a structured output with "session_id" set to 01ARZ3NDEKTSV4XXFFJFRC889A (and all other required fields present) + When I validate the structured output + Then it should be structurally valid + + Scenario: validate_structured_output accepts valid envelope for status field + Given I have a structured output with "status" set to ok (and all other required fields present) + When I validate the structured output + Then it should be structurally valid + + Scenario: validate_structured_output accepts valid envelope for exit_code field + Given I have a structured output with "exit_code" set to 0 (and all other required fields present) When I validate the structured output Then it should be structurally valid - Examples: - | field | value | - | command | "agents plan list" | - | session_id| "01ARZ3NDEKTSV4XXFFJFRC889A"| - | status | "ok" | - | exit_code | 0 | Scenario: validate_structured_output rejects invalid status value - Given I have a structured output with "status" set to "running" (and all other fields valid) + Given I have a structured output with "status" set to "running" (and all other required fields present) When I validate the structured output Then it should be structurally invalid And error must include "must be one of" Scenario: validate_structured_output rejects negative exit_code - Given I have a structured output with "exit_code" set to "-1" (and all other fields valid) + Given I have a structured output with "exit_code" set to "-1" (and all other required fields present) When I validate the structured output Then it should be structurally invalid And error must include "must be >= 0" @@ -138,16 +192,23 @@ Feature: Structural component output validation Then it should be structurally invalid And error must include "missing 'kind' field" - Scenario Outline: validate_structured_component_output routes to correct validator for type {target} - Given I target validation for "{target}" with valid data + Scenario: validate_structured_component_output routes plan_tree to correct validator + Given I target validation for "plan_tree" with valid data + When I call validate_structured_component_output + Then it should dispatch to the matching validator + And return a valid result + + Scenario: validate_structured_component_output routes decision to correct validator + Given I target validation for "decision" with valid data + When I call validate_structured_component_output + Then it should dispatch to the matching validator + And return a valid result + + Scenario: validate_structured_component_output routes structured_output to correct validator + Given I target validation for "structured_output" with valid data When I call validate_structured_component_output Then it should dispatch to the matching validator And return a valid result - Examples: - | target | - | plan_tree | - | decision | - | structured_output| Scenario: validate_structured_component_output rejects unknown target type Given I target validation for "unknown_type" with valid data diff --git a/src/cleveragents/core/validation.py b/src/cleveragents/core/validation.py index e777b769f..045bfccc0 100644 --- a/src/cleveragents/core/validation.py +++ b/src/cleveragents/core/validation.py @@ -34,6 +34,7 @@ Based on: from __future__ import annotations import re +from collections.abc import Callable from typing import Any # --------------------------------------------------------------------------- @@ -401,7 +402,7 @@ def validate_structured_output( # Unified dispatcher # --------------------------------------------------------------------------- -_TARGET_TYPE_MAP: dict[str, Any] = { +_TARGET_TYPE_MAP: dict[str, Callable[[Any], dict[str, Any]]] = { # --- Plan tree targets --- "plan_tree": validate_plan_tree, "plan-tree": validate_plan_tree, @@ -459,7 +460,7 @@ def validate_structured_component_output( errors=[f"valid types: {', '.join(available)}"], ) - result = validator(data) # type: ignore[arg-type] + result = validator(data) return result