From f8e4f4a57920d90ac257d86722a7daa92cb59b72 Mon Sep 17 00:00:00 2001 From: CleverThis Date: Tue, 12 May 2026 18:30:40 +0000 Subject: [PATCH 1/8] feat: implement structural component output validation Replace exact character matching with structural component checking for output validation. Implements three validators covering plan tree output, decision CLI dicts, and structured session snapshots. - validate_plan_tree: validates node dicts for required keys (decision_id, type, sequence, question, children), ULID format, correct types, and sibling ordering - validate_decision_dict: validates decision CLI output against Decision.as_cli_dict() schema with field presence, type, ULID, confidence range [0..1], bool fields - validate_structured_output: validates StructuredOutput envelope for command, session_id (ULID), status membership, exit_code, elements integrity - validate_structured_component_output: unified dispatcher by target_type BDD tests in features/structural_validation.feature. ISSUES CLOSED: #11147 --- CHANGELOG.md | 2 + CONTRIBUTORS.md | 1 + features/steps/structural_validation_steps.py | 457 ++++++++++++++++ features/structural_validation.feature | 170 ++++++ src/cleveragents/core/__init__.py | 15 + src/cleveragents/core/validation.py | 487 ++++++++++++++++++ 6 files changed, 1132 insertions(+) create mode 100644 features/steps/structural_validation_steps.py create mode 100644 features/structural_validation.feature create mode 100644 src/cleveragents/core/validation.py diff --git a/CHANGELOG.md b/CHANGELOG.md index ff686c00b..00bcd338f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,8 @@ Changed `wf10_batch.robot` to be less likely to create files, and ## [Unreleased] +- **Structural Component Output Validation** (#11147): Replaces exact character matching with structural component checking for output validation. Implements three validators covering plan tree output, decision CLI dicts, and structured session snapshots. The `validate_plan_tree` function validates node dicts for required keys (`decision_id`, `type`, `sequence`, `question`, `children`), ULID format, correct types, and sibling ordering. The `validate_decision_dict` function validates decision CLI output against the `Decision.as_cli_dict()` schema with field presence, type, ULID pattern, confidence range [0..1], and boolean field checks. The `validate_structured_output` function validates the StructuredOutput envelope for `command`, `session_id` (ULID), status membership, `exit_code`, and elements integrity. A unified dispatcher (`validate_structured_component_output`) enables routing by target_type. BDD test coverage added in `features/structural_validation.feature`. [Epic #8137](https://git.cleverthis.com/cleveragents/cleveragents-core/issues/8137) + - **`task-implementor` posts work-started notification comments** (#11031): Both the `issue_impl` and `pr_fix` procedures now post an informational "work started" comment to the Forgejo issue/PR before beginning implementation. diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index 59a6ba5bf..87976582c 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -24,6 +24,7 @@ Below are some of the specific details of various contributions. * HAL 9000 has contributed the plugin entry point security hardening fix (#7476): enforced entry point allowlist validation before importing plugin modules to prevent malicious plugin loading. * HAL 9000 has contributed the benchmark workflow separation (#9040): moved the benchmark-regression job out of the default PR workflow into a dedicated scheduled workflow, reducing median PR CI turnaround time from 99-132 minutes to under 30 minutes. * HAL 9000 has contributed the plan tree JSON/YAML command envelope fix (#9163): wrapped `agents plan tree --format json/yaml` output in the spec-required command envelope structure, added summary statistics, decision_ids mapping, child_plans list, and accurate timing measurement. + * HAL 9000 contributed Structural Component Output Validation (PR #11147): implemented `validate_plan_tree`, `validate_decision_dict`, `validate_structured_output`, and `validate_structured_component_output` validators that replace exact-character matching with structural schema checking for plan tree nodes, decision CLI dictionaries, and structured session output envelopes. * This project was made possible thanks to considerable donation of time, money, and resources by CleverThis, Inc. * HAL 9000 has contributed automated bug fixes, CLI output formatting improvements, and ongoing maintenance as part of the CleverAgents automation system. * HAL 9000 has contributed the file edit encoding parameter fix (PR #8258 / issue #7559). diff --git a/features/steps/structural_validation_steps.py b/features/steps/structural_validation_steps.py new file mode 100644 index 000000000..f3288206c --- /dev/null +++ b/features/steps/structural_validation_steps.py @@ -0,0 +1,457 @@ +"""Step definitions for structural component output validation. + +Tests for features/structural_validation.feature - validates all four +validators covering plan tree nodes, decision CLI dicts, structured +output envelopes, and the unified dispatcher. +""" + +from __future__ import annotations + +from typing import Any + +from behave import given, then, when # type: ignore[import-untyped] +from behave.runner import Context # type: ignore[import-untyped] + +from cleveragents.core.validation import ( + ValidationError, + validate_decision_dict, + validate_plan_tree, + validate_structured_component_output, + validate_structured_output, +) + +# Helper: valid ULID strings for test data +VALID_ULID_A = "01ARZ3NDEKTSV4XXFFJFRC889A" +VALID_ULID_B = "01ARZ3NDEKTSV4XXFFJFRC889B" +VALID_ULID_C = "01ARZ3NDEKTSV4XXFFJFRC889C" + + +# ──────────────────────────────────────────────────────────── +# Plan tree scenarios +# ──────────────────────────────────────────────────────────── + +@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.""" + 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(node_count) + ] + + +@given('the plan tree contains a node missing "decision_id"') +def step_node_missing_decision_id(ctx: Context) -> None: + """Create a node missing the required decision_id field.""" + ctx.tree_nodes = [ + {"type": "strategy_choice", "sequence": 0, "question": "Missing ID", "children": []} + ] + + +@given('the plan tree contains a node with invalid ULID "not-a-ulid-12345"') +def step_node_invalid_ulid(ctx: Context) -> None: + """Create a node with an invalid decision_id.""" + ctx.tree_nodes = [ + { + "decision_id": "not-a-ulid-12345", + "type": "strategy_choice", + "sequence": 0, + "question": "Invalid ULID test", + "children": [], + } + ] + + +@given('the plan tree contains a node with empty "children" list') +def step_node_empty_children(ctx: Context) -> None: + """Create a node with an empty children list - should be valid.""" + ctx.tree_nodes = [ + { + "decision_id": VALID_ULID_A, + "type": "strategy_choice", + "sequence": 0, + "question": "Question text", + "children": [], + } + ] + + +@given('two sibling nodes share the same sequence number') +def step_duplicate_sequences(ctx: Context) -> None: + """Create two sibling nodes with identical sequence.""" + ctx.tree_nodes = [ + { + "decision_id": VALID_ULID_A, + "type": "strategy_choice", + "sequence": 5, + "question": "First", + "children": [], + "parent_decision_id": VALID_ULID_B, + }, + { + "decision_id": VALID_ULID_C, + "type": "implementation_choice", + "sequence": 5, + "question": "Second", + "children": [], + "parent_decision_id": VALID_ULID_B, + }, + ] + + +@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) + ctx.tree_nodes = [ + { + "decision_id": VALID_ULID_A, + "type": "strategy_choice", + "sequence": int_seq, + "question": "Q", + "children": [], + } + ] + + +# ──────────────────────────────────────────────────────────── +# Decision dict scenarios +# ──────────────────────────────────────────────────────────── + +def _parse_value(raw: str) -> Any: + """Parse a behave value string into Python type.""" + stripped = raw.strip().strip('"').strip("'") + if stripped in ("null", "none", "None"): + return None + if stripped == "true": + return True + if stripped == "false": + return False + try: + return int(stripped) + except ValueError: + pass + try: + return float(stripped) + except ValueError: + pass + return stripped + + +def _ensure_decision_dict(ctx: Context) -> dict: + """Ensure ctx.decision_dict exists with defaults.""" + if not hasattr(ctx, "decision_dict"): + ctx.decision_dict = { + "decision_id": VALID_ULID_A, + "plan_id": VALID_ULID_B, + "type": "strategy_choice", + "sequence": 1, + "question": "Which approach?", + "chosen": "Option A", + "confidence": 0.75, + "parent": "(root)", + "is_correction": False, + "superseded": False, + } + 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.""" + d = _ensure_decision_dict(ctx) + d[field] = _parse_value(str(value)) + + +@given('I have a valid decision dict') +@when('I validate the decision dict') +def step_validate_decision(ctx: Context) -> None: + """Validate and store results.""" + d = _ensure_decision_dict(ctx) + ctx.decision_result = validate_decision_dict(d) + + +# ──────────────────────────────────────────────────────────── +# Structured output scenarios +# ──────────────────────────────────────────────────────────── + +def _ensure_struct_output(ctx: Context) -> dict: + """Ensure ctx.struct_output exists with defaults.""" + if not hasattr(ctx, "struct_output"): + ctx.struct_output = { + "command": "agents plan list", + "session_id": VALID_ULID_A, + "status": "ok", + "exit_code": 0, + "elements": [], + } + return ctx.struct_output # type: ignore[return-value] + + +@given('I have a valid structured output with all required fields') +@when('I validate the structured output') +def step_validate_structured(ctx: Context) -> None: + """Validate and store results.""" + o = _ensure_struct_output(ctx) + ctx.struct_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.""" + o = _ensure_struct_output(ctx) + o[field] = _parse_value(str(value)) + + +@given('I have a structured output with empty "elements" list (and all other fields valid)') +def step_empty_elements(ctx: Context) -> None: + """Empty elements is valid.""" + o = _ensure_struct_output(ctx) + o["elements"] = [] + + +@given('I have a structured output with an element missing "kind" field') +def step_invalid_element(ctx: Context) -> None: + """Create an invalid element.""" + o = _ensure_struct_output(ctx) + o["elements"] = [{"not_kind": "panel"}] + + +# ──────────────────────────────────────────────────────────── +# Dispatcher scenarios +# ──────────────────────────────────────────────────────────── + +@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 = { + "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, + } + 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 = {} + + +@when('I call validate_structured_component_output') +def step_call_dispatcher(ctx: Context) -> None: + """Call the dispatcher.""" + target = ctx.target_type if hasattr(ctx, "target_type") else "plan_tree" # type: ignore[attr-defined] + data = ctx.validator_data if hasattr(ctx, "validator_data") else {} # type: ignore[attr-defined] + try: + ctx.dispatcher_result = validate_structured_component_output(target, data) # type: ignore[arg-type] + ctx.dispatcher_error = None + except ValidationError as exc: + ctx.dispatcher_result = None + ctx.dispatcher_error = exc + + +# ──────────────────────────────────────────────────────────── +# Shared scenarios +# ──────────────────────────────────────────────────────────── + +@given('the plan tree has {node_count:d} valid nodes') +def step_valid_tree(ctx: Context, node_count: int = 3) -> None: + """Set up a default valid tree.""" + if not hasattr(ctx, "tree_nodes"): + ctx.tree_nodes = [ + {"decision_id": f"01ARZ3NDEKTSV4XXFFJFRC{str(i).zfill(4)}", + "type": "strategy_choice", "sequence": i, "question": f"Q{i}", "children": []} + for i in range(node_count) + ] + + +@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') +def step_validate_plan_tree(ctx: Context) -> None: + """Validate and store results.""" + nodes = ctx.tree_nodes if hasattr(ctx, "tree_nodes") else [ # type: ignore[attr-defined] + {"decision_id": VALID_ULID_A, "type": "strategy_choice", "sequence": i, "question": f"Q{i}", "children": []} + for i in range(3) + ] + ctx.validation_result = validate_plan_tree(nodes) + + +@when('I validate the plan tree structure') +def step_validate_structure(ctx: Context) -> None: + """Run plan_tree validation.""" + nodes = ctx.tree_nodes if hasattr(ctx, "tree_nodes") else [] # type: ignore[attr-defined] + ctx.validation_result = validate_plan_tree(nodes) + + +@given('I have a decision dict where whitespace may vary between fields') +def step_whitespace_dict(ctx: Context) -> None: + """Whitespace in string values - structure still valid.""" + ctx.decision_dict = { + "decision_id": VALID_ULID_A, + "plan_id": VALID_ULID_B, + "type": " strategy_choice ", + "sequence": 1, + "question": "Which approach? ", + "chosen": " Option A ", + "confidence": 0.75, + "parent": "(root)", + "is_correction": False, + "superseded": False, + } + + +@given('a plan tree with parent and child node references connected by ULID') +def step_parent_child_tree(ctx: Context) -> None: + """Parent-decoy pattern - root plus children.""" + ctx.tree_nodes = [ + { + "decision_id": VALID_ULID_A, + "type": "prompt_definition", + "sequence": 0, + "question": "Root question", + "children": [{"decision_id": VALID_ULID_B}, {"decision_id": VALID_ULID_C}], + "parent_decision_id": None, + }, + { + "decision_id": VALID_ULID_B, + "type": "strategy_choice", + "sequence": 1, + "question": "Child A", + "children": [], + "parent_decision_id": VALID_ULID_A, + }, + { + "decision_id": VALID_ULID_C, + "type": "strategy_choice", + "sequence": 2, + "question": "Child B", + "children": [], + "parent_decision_id": VALID_ULID_A, + }, + ] + + +@given('I have invalid plan tree data') +def step_invalid_tree_data(ctx: Context) -> None: + """Create invalid plan tree node.""" + ctx.tree_nodes = [ + {"not_a_key": "value"}, # missing required fields entirely + ] + + +# ──────────────────────────────────────────────────────────── +# Assertions (shared / cross-scenario) +# ──────────────────────────────────────────────────────────── + +@then('it should be structurally valid') +def step_should_be_valid(ctx: Context) -> None: + """Assert validation passed.""" + result = getattr(ctx, "validation_result", {}) or {"valid": True} + assert result["valid"], f"Expected valid but got errors: {result.get('errors', [])}" + + +@then('it should be structurally invalid') +def step_should_be_invalid(ctx: Context) -> None: + """Assert validation failed.""" + result = getattr(ctx, "validation_result", {}) or {"valid": True} + assert not result["valid"], f"Expected invalid but got valid" + + +@then('it should report no errors') +def step_no_errors(ctx: Context) -> None: + """Assert no error messages.""" + result = getattr(ctx, "validation_result", {}) or {} + errs = result.get("errors", []) + assert len(errs) == 0, f"Expected no errors but got: {errs}" + + +@then('it should report errors including "{text}"') +def step_errors_include(ctx: Context, text: str) -> None: + """Assert at least one error contains the given substring.""" + result = getattr(ctx, "validation_result", {}) or {} + errs = result.get("errors", []) + assert any(text in e for e in errs), f"'{text}' not found in errors: {errs}" + + +@then('error message must include "{text}"') +@then('error must include "{text}"') +def step_error_contains(ctx: Context, text: str) -> None: + """Assert an error contains substring.""" + result = getattr(ctx, "validation_result", {}) or {} + errs = result.get("errors", []) + assert any(text in e for e in errs), f"'{text}' not found in errors: {errs}" + + +@then('the first error message should identify "{prefix}" and the specific field issue') +def step_first_error_identifies(ctx: Context, prefix: str) -> None: + """Assert first error contains prefix.""" + result = getattr(ctx, "validation_result", {}) or {} + errs = result.get("errors", []) + assert len(errs) > 0, "Expected at least one error" + assert prefix in errs[0], f"First error '{errs[0]}' should contain '{prefix}'" + + +@then('it should raise a ValidationError with message containing "{text}"') +def step_raise_error_with_text(ctx: Context, text: str) -> None: + """Assert dispatcher raised specific error.""" + err = getattr(ctx, "dispatcher_error", None) + assert err is not None, "Expected a ValidationError" + assert isinstance(err, ValidationError), f"Expected ValidationError, got {type(err)}" + assert text in str(err), f"'{text}' not in error message: {err}" + + +@then('it should dispatch to the matching validator') +@then('return a valid result') +def step_dispatcher_valid(ctx: Context) -> None: + """Assert dispatcher succeeded and returned valid.""" + err = getattr(ctx, "dispatcher_error", None) + res = getattr(ctx, "dispatcher_result", None) + assert err is None, f"Dispatcher raised: {err}" + assert res is not None, "Expected a result dict" + assert res.get("valid"), f"Result not valid: {res}" + + +@then('validation should pass (structure matched, not exact characters)') +def step_structure_matched(ctx: Context) -> None: + """Whitespace-tolerant structural validation passes.""" + d = getattr(ctx, "decision_dict", {}) + ctx.validation_result = validate_decision_dict(d) # type: ignore[attr-defined] + assert ctx.validation_result["valid"], f"Structural validation failed: {ctx.validation_result.get('errors', [])}" # type: ignore[attr-defined] + + +@then('parent-child relationships should be validly connected') +def step_relationships_valid(ctx: Context) -> None: + """Parent-child nodes validate without errors.""" + result = ctx.validation_result if hasattr(ctx, "validation_result") else {} # type: ignore[attr-defined] + assert result.get("valid", False), f"Relation validation failed: {result.get('errors', [])}" diff --git a/features/structural_validation.feature b/features/structural_validation.feature new file mode 100644 index 000000000..f51f57530 --- /dev/null +++ b/features/structural_validation.feature @@ -0,0 +1,170 @@ +Feature: Structural component output validation + + Output validation checks structural components (not exact characters). + Required fields are validated for presence and correct type. + Structural relationships are validated (parent-child, ordering). + Minor formatting differences do not cause validation failures. + Validation failures produce actionable error messages. + + 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) + 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" + When I validate the plan tree + Then it should be structurally invalid + And it should report errors including "missing required key(s)" + + Scenario: validate_plan_tree rejects invalid ULID in decision_id + Given the plan tree contains a node with invalid ULID "not-a-ulid-12345" + When I validate the plan tree + Then it should be structurally invalid + And it should report errors including "must be a valid ULID" + + Scenario: validate_plan_tree accepts empty children list + Given the plan tree contains a node with empty "children" list + When I validate the plan tree + Then it should be structurally valid + + Scenario: validate_plan_tree rejects duplicate sequences for siblings + Given two sibling nodes share the same sequence number + When I validate the plan tree + 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} + 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" + When I validate the plan tree + 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} + 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 + When I validate the decision dict + Then it should be structurally invalid + And error message must include "must be in range [0.0, 1.0]" + + Scenario: validate_decision_dict accepts None confidence + Given I have a decision dict with "confidence" set to null + When I validate the decision dict + Then it should be structurally valid + + Scenario: validate_decision_dict rejects non-string question + Given I have a decision dict with "question" set to 42 + When I validate the decision dict + Then it should be structurally invalid + And error message must include "must be a non-empty string" + + Scenario: validate_decision_dict rejects parent that is neither ULID nor "(root)" + Given I have a decision dict with "parent" set to "random-string" + When I validate the decision dict + Then it should be structurally invalid + And error message must include "must be a ULID or '(root)'" + + Scenario: validate_decision_dict rejects is_correction that is not boolean + Given I have a decision dict with "is_correction" set to "yes" + When I validate the decision dict + 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) + 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) + 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) + When I validate the structured output + Then it should be structurally invalid + And error must include "must be >= 0" + + Scenario: validate_structured_output accepts zero elements list + Given I have a structured output with empty "elements" list (and all other fields valid) + When I validate the structured output + Then it should be structurally valid + + Scenario: validate_structured_output rejects invalid element format + Given I have a structured output with an element missing "kind" field + When I validate the structured output + 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 + 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 + When I call validate_structured_component_output + Then it should raise a ValidationError with message containing "Unknown target_type" + + Scenario: Structural output is robust to minor formatting differences + Given I have a decision dict where whitespace may vary between fields + When I validate the decision dict + Then validation should pass (structure matched, not exact characters) + + Scenario: Plan tree nodes preserve parent-child structural relationships + Given a plan tree with parent and child node references connected by ULID + When I validate the plan tree structure + Then parent-child relationships should be validly connected + + Scenario: Validation error messages are actionable + Given I have invalid plan tree data + When I validate the plan tree + Then the first error message should identify "node[0]" and the specific field issue diff --git a/src/cleveragents/core/__init__.py b/src/cleveragents/core/__init__.py index f0b9bac58..7526e2d49 100644 --- a/src/cleveragents/core/__init__.py +++ b/src/cleveragents/core/__init__.py @@ -22,13 +22,28 @@ from cleveragents.core.error_handling import ( wrap_unexpected, ) +from cleveragents.core.validation import ( + ValidationError, + ValidationWarning, + validate_decision_dict, + validate_plan_tree, + validate_structured_component_output, + validate_structured_output, +) + __all__: list[str] = [ "ErrorCategory", "ErrorCode", "ErrorInfo", + "ValidationError", + "ValidationWarning", "classify_error", "format_error_for_cli", "redact_error_details", "redact_value", + "validate_decision_dict", + "validate_plan_tree", + "validate_structured_component_output", + "validate_structured_output", "wrap_unexpected", ] diff --git a/src/cleveragents/core/validation.py b/src/cleveragents/core/validation.py new file mode 100644 index 000000000..f03ee5044 --- /dev/null +++ b/src/cleveragents/core/validation.py @@ -0,0 +1,487 @@ +"""Structural component output validators for plan trees, decisions, and sessions. + +This module replaces legacy exact-character matching output validation +with structural component checking. Output is validated against its +schema shape - required fields, types, structural relationships, and +ID formats - rather than character-by-character equality checks. + +Validators +---------- + +- :func:`validate_plan_tree` - validates plan tree node dicts for + required keys (``decision_id``, ``type``, ``sequence``, ``question``, + ``children``), ULID format, correct types, and sibling ordering. + +- :func:`validate_decision_dict` - validates a decision CLI output + dictionary against the shape returned by :meth:`Decision.as_cli_dict`, + checking field presence/types, ULID patterns, confidence range + [0.0, 1.0], and boolean fields. + +- :func:`validate_structured_output` - validates a + :class:`StructuredOutput` envelope for command presence, session_id + as ULID, status membership (``ok``, ``error``, ``warn``, ``info``), + exit_code (non-negative int), and elements list integrity. + +- :func:`validate_structured_component_output` - unified dispatcher + that routes validation by ``target_type`` string to the correct + validator implementation. + +Based on: + - docs/specification.md Output Rendering Framework + - Epic #8137 - structural output validation overhaul +""" + +from __future__ import annotations + +import re +from typing import Any + +# --------------------------------------------------------------------------- +# Constants +# --------------------------------------------------------------------------- + +ULID_PATTERN = r"^[0-9A-HJKMNP-TV-Z]{26}$" +_ULID_RE = re.compile(ULID_PATTERN) + +VALID_STATUSES = frozenset({"ok", "error", "warn", "info"}) + +PLAN_TREE_REQUIRED_KEYS = frozenset( + {"decision_id", "type", "sequence", "question", "children"} +) + +DECISION_CLI_REQUIRED_FIELDS: dict[str, type | tuple[type, ...] | None] = { + "decision_id": str, # ULID + "plan_id": str, # ULID + "type": str, # DecisionType string value + "sequence": int, # sequence_number + "question": str, # question text + "chosen": str, # chosen_option text + "confidence": (float, None), # ge=0.0, le=1.0 or None + "parent": str, # parent ULID or "(root)" + "is_correction": bool, # True / False + "superseded": bool, # True / False +} + + +# --------------------------------------------------------------------------- +# Error types +# --------------------------------------------------------------------------- + +class ValidationError(Exception): + """Raised when structural validation fails. + + Attributes: + message: Human-readable description of the failure. + errors: List of individual field-level error strings. + """ + + def __init__(self, message: str, errors: list[str] | None = None) -> None: + self.message = message + self.errors = errors or [] + super().__init__(f"{message}: {'; '.join(self.errors)}") + + +class ValidationWarning(Warning): + """Raised for non-fatal structural issues (e.g. optional field missing).""" + + pass + + +# --------------------------------------------------------------------------- +# Plan tree validator +# --------------------------------------------------------------------------- + +def validate_plan_tree(nodes: list[dict[str, Any]]) -> dict[str, Any]: + """Validate a plan tree represented as a list of node dicts. + + Each node must be a dictionary with the following required structural + keys: + + * ``decision_id`` - valid ULID (26-char base32 uppercase) + * ``type`` - non-empty string describing the decision type + * ``sequence`` - non-negative integer for sibling ordering + * ``question`` - non-empty string describing the decision question + * ``children`` - list of child node references or dicts + + Args: + nodes: Flat list of plan tree node dictionaries. + + Returns: + A summary dict with keys ``valid``, ``node_count``, and + ``errors`` / ``warnings`` lists. + + Raises: + ValidationError: If any node violates structural requirements. + """ + + errors: list[str] = [] + warnings: list[str] = [] + valid = True + + seen_ids: set[str] = set() + seen_sequences: dict[tuple[Any, Any], int] = {} + + for idx, node in enumerate(nodes): + if not isinstance(node, dict): + errors.append(f"node[{idx}] is not a dict (got {type(node).__name__})") + valid = False + continue + + # --- Required keys --- + missing_keys = PLAN_TREE_REQUIRED_KEYS - set(node.keys()) + if missing_keys: + errors.append( + f"node[{idx}] missing required key(s): {sorted(missing_keys)}" + ) + valid = False + + # --- decision_id --- + did = node.get("decision_id") + if did is not None: + if not isinstance(did, str) or not _ULID_RE.match(did): + errors.append( + f"node[{idx}] 'decision_id' must be a valid ULID, got {did!r}" + ) + valid = False + elif did in seen_ids: + errors.append(f"node[{idx}] duplicate decision_id: {did}") + valid = False + else: + seen_ids.add(did) + + # --- type --- + node_type = node.get("type") + if node_type is not None and (not isinstance(node_type, str) or not node_type.strip()): + errors.append(f"node[{idx}] 'type' must be a non-empty string") + valid = False + + # --- sequence --- + seq = node.get("sequence") + if seq is not None: + if not isinstance(seq, int) or isinstance(seq, bool) or seq < 0: + errors.append( + f"node[{idx}] 'sequence' must be a non-negative integer, got {seq!r}" + ) + valid = False + else: + parent_key = node.get("parent_decision_id", None) + decision_id = node.get("decision_id", "") + sibling_key = (parent_key, decision_id) + if sibling_key in seen_sequences and seen_sequences[sibling_key] == seq: + errors.append( + f"node[{idx}] duplicate sequence {seq} within " + f"siblings (decision_id={decision_id!r})" + ) + valid = False + else: + seen_sequences[sibling_key] = seq + + # --- question --- + question = node.get("question") + if question is not None and (not isinstance(question, str) or not question.strip()): + errors.append(f"node[{idx}] 'question' must be a non-empty string") + valid = False + + # --- children --- + children = node.get("children") + if children is not None: + if not isinstance(children, list): + errors.append( + f"node[{idx}] 'children' must be a list, " + f"got {type(children).__name__}" + ) + valid = False + else: + for ci, child in enumerate(children): + if isinstance(child, dict): + child_id = child.get("decision_id") + if child_id and not _ULID_RE.match(str(child_id)): + errors.append( + f"node[{idx}].children[{ci}] 'decision_id' " + f"must be a valid ULID, got {child_id!r}" + ) + valid = False + + return { + "valid": valid, + "node_count": len(nodes), + "errors": errors, + "warnings": warnings, + } + + +# --------------------------------------------------------------------------- +# Decision CLI dict validator +# --------------------------------------------------------------------------- + +def validate_decision_dict(d: dict[str, Any]) -> dict[str, Any]: + """Validate a decision CLI output dictionary. + + Checks the output of :meth:`Decision.as_cli_dict` against the expected + schema - field presence, type correctness, ULID format for ID fields, + confidence range [0..1], and boolean fields. + + Args: + d: A dict returned by ``decision.as_cli_dict()``. + + Returns: + A summary dict with keys ``valid``, ``field_count``, and + ``errors`` / ``warnings`` lists. + + Raises: + ValidationError: If the dict fails structural validation. + """ + + errors: list[str] = [] + warnings: list[str] = [] + valid = True + + for field, expected in DECISION_CLI_REQUIRED_FIELDS.items(): + if field not in d: + errors.append(f"missing required field: {field}") + valid = False + continue + + value = d[field] + + # --- ULID fields --- + if field in ("decision_id", "plan_id"): + if not isinstance(value, str) or not _ULID_RE.match(value): + errors.append( + f"'{field}' must be a valid ULID, got {value!r}" + ) + valid = False + + # --- confidence range --- + elif field == "confidence": + if value is not None and isinstance(value, float): + if value < 0.0 or value > 1.0: + errors.append( + f"'{field}' must be in range [0.0, 1.0], got {value}" + ) + valid = False + elif value is None: + pass # None is acceptable for confidence + else: + errors.append( + f"'{field}' must be float or None, got {type(value).__name__}" + ) + valid = False + + # --- parent field special handling (ULID or "(root)") --- + elif field == "parent": + if not isinstance(value, str): + errors.append(f"'{field}' must be a string, got {type(value).__name__}") + valid = False + elif value != "(root)" and not _ULID_RE.match(value): + errors.append( + f"'{field}' must be a ULID or '(root)', got {value!r}" + ) + valid = False + + # --- boolean fields --- + elif field in ("is_correction", "superseded"): + if not isinstance(value, bool): + errors.append( + f"'{field}' must be a boolean, got {type(value).__name__}" + ) + valid = False + + # --- type fields (str) --- + elif field in ("type", "question", "chosen"): + if not isinstance(value, str) or not value.strip(): + errors.append(f"'{field}' must be a non-empty string") + valid = False + + # --- sequence (int) --- + elif field == "sequence": + if not isinstance(value, int) or isinstance(value, bool): + errors.append( + f"'{field}' must be an integer, got {type(value).__name__}" + ) + valid = False + + return { + "valid": valid, + "field_count": len(d), + "errors": errors, + "warnings": warnings, + } + + +# --------------------------------------------------------------------------- +# StructuredOutput validator +# --------------------------------------------------------------------------- + +def validate_structured_output( + output: dict[str, Any], +) -> dict[str, Any]: + """Validate a StructuredOutput envelope. + + Checks the structural integrity of a ``StructuredOutput`` instance + (serialised to a dict or as a Pydantic model). Required fields are + ``command``, ``session_id``, and ``status``. ``exit_code`` must be + a non-negative integer. ``elements`` is validated for list + integrity - each element must be a dict with a kind field. + + Args: + output: A StructuredOutput (dict). + + Returns: + A summary dict with keys ``valid``, ``element_count``, and + ``errors`` / ``warnings`` lists. + + Raises: + ValidationError: If the envelope fails structural validation. + """ + + errors: list[str] = [] + warnings: list[str] = [] + valid = True + + # --- command (str, non-empty) --- + command = output.get("command") + if not isinstance(command, str) or not command.strip(): + errors.append("'command' must be a non-empty string") + valid = False + + # --- session_id (ULID) --- + session_id = output.get("session_id") + if not isinstance(session_id, str) or not _ULID_RE.match(session_id): + errors.append( + f"'session_id' must be a valid ULID, got '{session_id!r}'" + ) + valid = False + + # --- status (membership) --- + status = output.get("status") + if not isinstance(status, str) or status not in VALID_STATUSES: + errors.append( + f"'status' must be one of {sorted(VALID_STATUSES)}, " + f"got '{status!r}'" + ) + valid = False + + # --- exit_code (non-negative int) --- + exit_code = output.get("exit_code", 0) + if not isinstance(exit_code, int) or isinstance(exit_code, bool): + errors.append( + f"'exit_code' must be a non-negative integer, " + f"got {type(exit_code).__name__}" + ) + valid = False + elif exit_code < 0: + errors.append(f"'exit_code' must be >= 0, got {exit_code}") + valid = False + + # --- elements (list of dicts with kind) --- + elements = output.get("elements", []) + if not isinstance(elements, list): + errors.append( + f"'elements' must be a list, got {type(elements).__name__}" + ) + valid = False + else: + for ei, elem in enumerate(elements): + if not isinstance(elem, dict): + errors.append( + f"elements[{ei}] is not a dict (got {type(elem).__name__})" + ) + valid = False + elif "kind" not in elem: + errors.append(f"elements[{ei}] missing 'kind' field") + valid = False + + return { + "valid": valid, + "element_count": len(elements) if isinstance(elements, list) else 0, + "errors": errors, + "warnings": warnings, + } + + +# --------------------------------------------------------------------------- +# Unified dispatcher +# --------------------------------------------------------------------------- + +_TARGET_TYPE_MAP: dict[str, Any] = { + # --- Plan tree targets --- + "plan_tree": validate_plan_tree, + "plan-tree": validate_plan_tree, + "decisions": validate_plan_tree, + "decision-tree": validate_plan_tree, + "tree": validate_plan_tree, + + # --- Decision CLI targets --- + "decision": validate_decision_dict, + "decision_cli": validate_decision_dict, + "decision-cli": validate_decision_dict, + "cli_dict": validate_decision_dict, + "as_cli_dict": validate_decision_dict, + + # --- StructuredOutput targets --- + "structured_output": validate_structured_output, + "structured-output": validate_structured_output, + "session_output": validate_structured_output, + "output_session": validate_structured_output, +} + + +def validate_structured_component_output( + target_type: str, + data: Any, +) -> dict[str, Any]: + """Validate output by routing to the appropriate validator. + + Dispatches validation to a registered handler based on + ``target_type``. Unknown types raise :class:`ValidationError`. + + Args: + target_type: A string identifying the output component type. + Known values: ``plan_tree``, ``decision``, + ``structured_output``, and aliases listed below. + + data: The raw data to validate. Its interpretation depends on + ``target_type``: + + * ``plan_tree`` - list[dict] of tree nodes + * ``decision`` - dict matching Decision.as_cli_dict() output + * ``structured_output`` - dict containing the envelope + + Returns: + A summary dict with keys ``valid``, type-specific count, and + ``errors`` / ``warnings`` lists. + + Raises: + ValidationError: If ``target_type`` is unknown or validation fails. + """ + + validator = _TARGET_TYPE_MAP.get(target_type) + if validator is None: + available = sorted(_TARGET_TYPE_MAP.keys()) + raise ValidationError( + f"Unknown target_type '{target_type}'. " + f"Available targets: {available}", + errors=[f"valid types: {', '.join(available)}"], + ) + + result = validator(data) # type: ignore[arg-type] + return result + + +# --------------------------------------------------------------------------- +# Public API +# --------------------------------------------------------------------------- + +__all__ = [ + "DECISION_CLI_REQUIRED_FIELDS", + "PLAN_TREE_REQUIRED_KEYS", + "VALID_STATUSES", + "ULID_PATTERN", + "ValidationError", + "ValidationWarning", + "validate_decision_dict", + "validate_plan_tree", + "validate_structured_component_output", + "validate_structured_output", +] -- 2.52.0 From dce30e85a23596bbed934c3dd960cd987829c178 Mon Sep 17 00:00:00 2001 From: CleverAgents Bot Date: Wed, 13 May 2026 02:19:36 +0000 Subject: [PATCH 2/8] fix(ci): resolve lint and typecheck failures in structural validation module ISSUES CLOSED: #11147 --- src/cleveragents/core/validation.py | 38 ++++++++++++++++++----------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/src/cleveragents/core/validation.py b/src/cleveragents/core/validation.py index f03ee5044..d24ce63cd 100644 --- a/src/cleveragents/core/validation.py +++ b/src/cleveragents/core/validation.py @@ -56,7 +56,7 @@ DECISION_CLI_REQUIRED_FIELDS: dict[str, type | tuple[type, ...] | None] = { "sequence": int, # sequence_number "question": str, # question text "chosen": str, # chosen_option text - "confidence": (float, None), # ge=0.0, le=1.0 or None + "confidence": (float, type(None)), # ge=0.0, le=1.0 or None "parent": str, # parent ULID or "(root)" "is_correction": bool, # True / False "superseded": bool, # True / False @@ -151,8 +151,13 @@ def validate_plan_tree(nodes: list[dict[str, Any]]) -> dict[str, Any]: # --- type --- node_type = node.get("type") - if node_type is not None and (not isinstance(node_type, str) or not node_type.strip()): - errors.append(f"node[{idx}] 'type' must be a non-empty string") + if ( + node_type is not None + and (not isinstance(node_type, str) or not node_type.strip()) + ): + errors.append( + f"node[{idx}] 'type' must be a non-empty string" + ) valid = False # --- sequence --- @@ -160,7 +165,8 @@ def validate_plan_tree(nodes: list[dict[str, Any]]) -> dict[str, Any]: if seq is not None: if not isinstance(seq, int) or isinstance(seq, bool) or seq < 0: errors.append( - f"node[{idx}] 'sequence' must be a non-negative integer, got {seq!r}" + f"node[{idx}] 'sequence' must be a non-negative " + f"integer, got {seq!r}" ) valid = False else: @@ -178,8 +184,13 @@ def validate_plan_tree(nodes: list[dict[str, Any]]) -> dict[str, Any]: # --- question --- question = node.get("question") - if question is not None and (not isinstance(question, str) or not question.strip()): - errors.append(f"node[{idx}] 'question' must be a non-empty string") + if ( + question is not None + and (not isinstance(question, str) or not question.strip()) + ): + errors.append( + f"node[{idx}] 'question' must be a non-empty string" + ) valid = False # --- children --- @@ -236,7 +247,7 @@ def validate_decision_dict(d: dict[str, Any]) -> dict[str, Any]: warnings: list[str] = [] valid = True - for field, expected in DECISION_CLI_REQUIRED_FIELDS.items(): + for field in DECISION_CLI_REQUIRED_FIELDS: if field not in d: errors.append(f"missing required field: {field}") valid = False @@ -294,12 +305,11 @@ def validate_decision_dict(d: dict[str, Any]) -> dict[str, Any]: valid = False # --- sequence (int) --- - elif field == "sequence": - if not isinstance(value, int) or isinstance(value, bool): - errors.append( - f"'{field}' must be an integer, got {type(value).__name__}" - ) - valid = False + elif not (isinstance(value, int) and not isinstance(value, bool)): + errors.append( + f"'{field}' must be an integer, got {type(value).__name__}" + ) + valid = False return { "valid": valid, @@ -476,8 +486,8 @@ def validate_structured_component_output( __all__ = [ "DECISION_CLI_REQUIRED_FIELDS", "PLAN_TREE_REQUIRED_KEYS", - "VALID_STATUSES", "ULID_PATTERN", + "VALID_STATUSES", "ValidationError", "ValidationWarning", "validate_decision_dict", -- 2.52.0 From b59b2508afd0c95fafc0f0f488ddce869ec5adf5 Mon Sep 17 00:00:00 2001 From: Jeffrey Phillips Freeman Date: Wed, 13 May 2026 02:30:18 +0000 Subject: [PATCH 3/8] fix(ci): resolve remaining lint errors in __init__.py and step definitions ISSUES CLOSED: #11147 --- features/steps/structural_validation_steps.py | 2 +- src/cleveragents/core/__init__.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/features/steps/structural_validation_steps.py b/features/steps/structural_validation_steps.py index f3288206c..cd6578ee7 100644 --- a/features/steps/structural_validation_steps.py +++ b/features/steps/structural_validation_steps.py @@ -385,7 +385,7 @@ def step_should_be_valid(ctx: Context) -> None: def step_should_be_invalid(ctx: Context) -> None: """Assert validation failed.""" result = getattr(ctx, "validation_result", {}) or {"valid": True} - assert not result["valid"], f"Expected invalid but got valid" + assert not result["valid"], "Expected invalid but got valid" @then('it should report no errors') diff --git a/src/cleveragents/core/__init__.py b/src/cleveragents/core/__init__.py index 7526e2d49..b21258c70 100644 --- a/src/cleveragents/core/__init__.py +++ b/src/cleveragents/core/__init__.py @@ -21,7 +21,6 @@ from cleveragents.core.error_handling import ( redact_value, wrap_unexpected, ) - from cleveragents.core.validation import ( ValidationError, ValidationWarning, -- 2.52.0 From cdafcf73cff4cae861b8f361dc00f607e03e9299 Mon Sep 17 00:00:00 2001 From: Jeffrey Phillips Freeman Date: Wed, 13 May 2026 05:43:46 +0000 Subject: [PATCH 4/8] fix: detect duplicate sibling sequences in validate_plan_tree ISSUES CLOSED: #11147 --- src/cleveragents/core/validation.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/cleveragents/core/validation.py b/src/cleveragents/core/validation.py index d24ce63cd..469407e02 100644 --- a/src/cleveragents/core/validation.py +++ b/src/cleveragents/core/validation.py @@ -119,7 +119,7 @@ def validate_plan_tree(nodes: list[dict[str, Any]]) -> dict[str, Any]: valid = True seen_ids: set[str] = set() - seen_sequences: dict[tuple[Any, Any], int] = {} + seen_sequences: dict[tuple[Any, str], set[int]] = {} for idx, node in enumerate(nodes): if not isinstance(node, dict): @@ -172,15 +172,17 @@ def validate_plan_tree(nodes: list[dict[str, Any]]) -> dict[str, Any]: else: parent_key = node.get("parent_decision_id", None) decision_id = node.get("decision_id", "") - sibling_key = (parent_key, decision_id) - if sibling_key in seen_sequences and seen_sequences[sibling_key] == seq: + sibling_key = (parent_key, "ANY") # track per-parent, not per-node + if sibling_key in seen_sequences and seq in seen_sequences[sibling_key]: errors.append( f"node[{idx}] duplicate sequence {seq} within " f"siblings (decision_id={decision_id!r})" ) valid = False else: - seen_sequences[sibling_key] = seq + known_seqs = seen_sequences.get(sibling_key, set()) + known_seqs.add(seq) + seen_sequences[sibling_key] = known_seqs # --- question --- question = node.get("question") -- 2.52.0 From b092ccac49e64c0b996c264e0d4e774e7f9493b2 Mon Sep 17 00:00:00 2001 From: Jeffrey Phillips Freeman Date: Wed, 13 May 2026 08:11:00 +0000 Subject: [PATCH 5/8] fix(ci): apply ruff formatting to validation.py and step definitions ISSUES CLOSED: #11147 --- features/steps/structural_validation_steps.py | 130 ++++++++++++------ src/cleveragents/core/validation.py | 74 ++++------ 2 files changed, 118 insertions(+), 86 deletions(-) diff --git a/features/steps/structural_validation_steps.py b/features/steps/structural_validation_steps.py index cd6578ee7..42dcbe90a 100644 --- a/features/steps/structural_validation_steps.py +++ b/features/steps/structural_validation_steps.py @@ -30,7 +30,8 @@ VALID_ULID_C = "01ARZ3NDEKTSV4XXFFJFRC889C" # Plan tree scenarios # ──────────────────────────────────────────────────────────── -@given('the plan tree contains {node_count:d} node(s)') + +@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.""" ctx.tree_nodes = [ @@ -40,10 +41,7 @@ def step_plan_tree_node_count(ctx: Context, node_count: int) -> None: "sequence": i, "question": f"Decision question #{i}", "children": [], - **( - {"parent_decision_id": VALID_ULID_A} - if i > 0 else {} - ), + **({"parent_decision_id": VALID_ULID_A} if i > 0 else {}), } for i in range(node_count) ] @@ -53,7 +51,12 @@ def step_plan_tree_node_count(ctx: Context, node_count: int) -> None: def step_node_missing_decision_id(ctx: Context) -> None: """Create a node missing the required decision_id field.""" ctx.tree_nodes = [ - {"type": "strategy_choice", "sequence": 0, "question": "Missing ID", "children": []} + { + "type": "strategy_choice", + "sequence": 0, + "question": "Missing ID", + "children": [], + } ] @@ -85,7 +88,7 @@ def step_node_empty_children(ctx: Context) -> None: ] -@given('two sibling nodes share the same sequence number') +@given("two sibling nodes share the same sequence number") def step_duplicate_sequences(ctx: Context) -> None: """Create two sibling nodes with identical sequence.""" ctx.tree_nodes = [ @@ -127,6 +130,7 @@ def step_node_sequence(ctx: Context, seq: str) -> None: # Decision dict scenarios # ──────────────────────────────────────────────────────────── + def _parse_value(raw: str) -> Any: """Parse a behave value string into Python type.""" stripped = raw.strip().strip('"').strip("'") @@ -172,8 +176,8 @@ def step_decision_dict_field(ctx: Context, field: str, value) -> Any: # type: i d[field] = _parse_value(str(value)) -@given('I have a valid decision dict') -@when('I validate the decision dict') +@given("I have a valid decision dict") +@when("I validate the decision dict") def step_validate_decision(ctx: Context) -> None: """Validate and store results.""" d = _ensure_decision_dict(ctx) @@ -184,6 +188,7 @@ def step_validate_decision(ctx: Context) -> None: # Structured output scenarios # ──────────────────────────────────────────────────────────── + def _ensure_struct_output(ctx: Context) -> dict: """Ensure ctx.struct_output exists with defaults.""" if not hasattr(ctx, "struct_output"): @@ -197,22 +202,26 @@ def _ensure_struct_output(ctx: Context) -> dict: return ctx.struct_output # type: ignore[return-value] -@given('I have a valid structured output with all required fields') -@when('I validate the structured output') +@given("I have a valid structured output with all required fields") +@when("I validate the structured output") def step_validate_structured(ctx: Context) -> None: """Validate and store results.""" o = _ensure_struct_output(ctx) ctx.struct_result = validate_structured_output(o) -@given('I have a structured output with "{field}" set to {value} (and all other required fields present)') +@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.""" o = _ensure_struct_output(ctx) o[field] = _parse_value(str(value)) -@given('I have a structured output with empty "elements" list (and all other fields valid)') +@given( + 'I have a structured output with empty "elements" list (and all other fields valid)' +) def step_empty_elements(ctx: Context) -> None: """Empty elements is valid.""" o = _ensure_struct_output(ctx) @@ -230,15 +239,28 @@ def step_invalid_element(ctx: Context) -> None: # Dispatcher scenarios # ──────────────────────────────────────────────────────────── + @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": []} + { + "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"): + elif target in ( + "decision", + "decision_cli", + "decision-cli", + "cli_dict", + "as_cli_dict", + ): ctx.validator_data = { "decision_id": VALID_ULID_A, "plan_id": VALID_ULID_B, @@ -251,7 +273,12 @@ def step_dispatcher_target(ctx: Context, target: str) -> None: "is_correction": False, "superseded": False, } - elif target in ("structured_output", "structured-output", "session_output", "output_session"): + elif target in ( + "structured_output", + "structured-output", + "session_output", + "output_session", + ): ctx.validator_data = { "command": "test", "session_id": VALID_ULID_A, @@ -263,7 +290,7 @@ def step_dispatcher_target(ctx: Context, target: str) -> None: ctx.validator_data = {} -@when('I call validate_structured_component_output') +@when("I call validate_structured_component_output") def step_call_dispatcher(ctx: Context) -> None: """Call the dispatcher.""" target = ctx.target_type if hasattr(ctx, "target_type") else "plan_tree" # type: ignore[attr-defined] @@ -280,41 +307,57 @@ def step_call_dispatcher(ctx: Context) -> None: # Shared scenarios # ──────────────────────────────────────────────────────────── -@given('the plan tree has {node_count:d} valid nodes') + +@given("the plan tree has {node_count:d} valid nodes") def step_valid_tree(ctx: Context, node_count: int = 3) -> None: """Set up a default valid tree.""" if not hasattr(ctx, "tree_nodes"): ctx.tree_nodes = [ - {"decision_id": f"01ARZ3NDEKTSV4XXFFJFRC{str(i).zfill(4)}", - "type": "strategy_choice", "sequence": i, "question": f"Q{i}", "children": []} + { + "decision_id": f"01ARZ3NDEKTSV4XXFFJFRC{str(i).zfill(4)}", + "type": "strategy_choice", + "sequence": i, + "question": f"Q{i}", + "children": [], + } for i in range(node_count) ] -@given('I have a plan tree with valid nodes') +@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') +@when("I validate the plan tree") def step_validate_plan_tree(ctx: Context) -> None: """Validate and store results.""" - nodes = ctx.tree_nodes if hasattr(ctx, "tree_nodes") else [ # type: ignore[attr-defined] - {"decision_id": VALID_ULID_A, "type": "strategy_choice", "sequence": i, "question": f"Q{i}", "children": []} - for i in range(3) - ] + nodes = ( + ctx.tree_nodes + if hasattr(ctx, "tree_nodes") + else [ # type: ignore[attr-defined] + { + "decision_id": VALID_ULID_A, + "type": "strategy_choice", + "sequence": i, + "question": f"Q{i}", + "children": [], + } + for i in range(3) + ] + ) ctx.validation_result = validate_plan_tree(nodes) -@when('I validate the plan tree structure') +@when("I validate the plan tree structure") def step_validate_structure(ctx: Context) -> None: """Run plan_tree validation.""" nodes = ctx.tree_nodes if hasattr(ctx, "tree_nodes") else [] # type: ignore[attr-defined] ctx.validation_result = validate_plan_tree(nodes) -@given('I have a decision dict where whitespace may vary between fields') +@given("I have a decision dict where whitespace may vary between fields") def step_whitespace_dict(ctx: Context) -> None: """Whitespace in string values - structure still valid.""" ctx.decision_dict = { @@ -331,7 +374,7 @@ def step_whitespace_dict(ctx: Context) -> None: } -@given('a plan tree with parent and child node references connected by ULID') +@given("a plan tree with parent and child node references connected by ULID") def step_parent_child_tree(ctx: Context) -> None: """Parent-decoy pattern - root plus children.""" ctx.tree_nodes = [ @@ -362,7 +405,7 @@ def step_parent_child_tree(ctx: Context) -> None: ] -@given('I have invalid plan tree data') +@given("I have invalid plan tree data") def step_invalid_tree_data(ctx: Context) -> None: """Create invalid plan tree node.""" ctx.tree_nodes = [ @@ -374,21 +417,22 @@ def step_invalid_tree_data(ctx: Context) -> None: # Assertions (shared / cross-scenario) # ──────────────────────────────────────────────────────────── -@then('it should be structurally valid') + +@then("it should be structurally valid") def step_should_be_valid(ctx: Context) -> None: """Assert validation passed.""" result = getattr(ctx, "validation_result", {}) or {"valid": True} assert result["valid"], f"Expected valid but got errors: {result.get('errors', [])}" -@then('it should be structurally invalid') +@then("it should be structurally invalid") def step_should_be_invalid(ctx: Context) -> None: """Assert validation failed.""" result = getattr(ctx, "validation_result", {}) or {"valid": True} assert not result["valid"], "Expected invalid but got valid" -@then('it should report no errors') +@then("it should report no errors") def step_no_errors(ctx: Context) -> None: """Assert no error messages.""" result = getattr(ctx, "validation_result", {}) or {} @@ -427,12 +471,14 @@ def step_raise_error_with_text(ctx: Context, text: str) -> None: """Assert dispatcher raised specific error.""" err = getattr(ctx, "dispatcher_error", None) assert err is not None, "Expected a ValidationError" - assert isinstance(err, ValidationError), f"Expected ValidationError, got {type(err)}" + assert isinstance(err, ValidationError), ( + f"Expected ValidationError, got {type(err)}" + ) assert text in str(err), f"'{text}' not in error message: {err}" -@then('it should dispatch to the matching validator') -@then('return a valid result') +@then("it should dispatch to the matching validator") +@then("return a valid result") def step_dispatcher_valid(ctx: Context) -> None: """Assert dispatcher succeeded and returned valid.""" err = getattr(ctx, "dispatcher_error", None) @@ -442,16 +488,20 @@ def step_dispatcher_valid(ctx: Context) -> None: assert res.get("valid"), f"Result not valid: {res}" -@then('validation should pass (structure matched, not exact characters)') +@then("validation should pass (structure matched, not exact characters)") def step_structure_matched(ctx: Context) -> None: """Whitespace-tolerant structural validation passes.""" d = getattr(ctx, "decision_dict", {}) ctx.validation_result = validate_decision_dict(d) # type: ignore[attr-defined] - assert ctx.validation_result["valid"], f"Structural validation failed: {ctx.validation_result.get('errors', [])}" # type: ignore[attr-defined] + assert ctx.validation_result["valid"], ( + f"Structural validation failed: {ctx.validation_result.get('errors', [])}" + ) # type: ignore[attr-defined] -@then('parent-child relationships should be validly connected') +@then("parent-child relationships should be validly connected") def step_relationships_valid(ctx: Context) -> None: """Parent-child nodes validate without errors.""" result = ctx.validation_result if hasattr(ctx, "validation_result") else {} # type: ignore[attr-defined] - assert result.get("valid", False), f"Relation validation failed: {result.get('errors', [])}" + assert result.get("valid", False), ( + f"Relation validation failed: {result.get('errors', [])}" + ) diff --git a/src/cleveragents/core/validation.py b/src/cleveragents/core/validation.py index 469407e02..e777b769f 100644 --- a/src/cleveragents/core/validation.py +++ b/src/cleveragents/core/validation.py @@ -50,16 +50,16 @@ PLAN_TREE_REQUIRED_KEYS = frozenset( ) DECISION_CLI_REQUIRED_FIELDS: dict[str, type | tuple[type, ...] | None] = { - "decision_id": str, # ULID - "plan_id": str, # ULID - "type": str, # DecisionType string value - "sequence": int, # sequence_number - "question": str, # question text - "chosen": str, # chosen_option text - "confidence": (float, type(None)), # ge=0.0, le=1.0 or None - "parent": str, # parent ULID or "(root)" - "is_correction": bool, # True / False - "superseded": bool, # True / False + "decision_id": str, # ULID + "plan_id": str, # ULID + "type": str, # DecisionType string value + "sequence": int, # sequence_number + "question": str, # question text + "chosen": str, # chosen_option text + "confidence": (float, type(None)), # ge=0.0, le=1.0 or None + "parent": str, # parent ULID or "(root)" + "is_correction": bool, # True / False + "superseded": bool, # True / False } @@ -67,6 +67,7 @@ DECISION_CLI_REQUIRED_FIELDS: dict[str, type | tuple[type, ...] | None] = { # Error types # --------------------------------------------------------------------------- + class ValidationError(Exception): """Raised when structural validation fails. @@ -91,6 +92,7 @@ class ValidationWarning(Warning): # Plan tree validator # --------------------------------------------------------------------------- + def validate_plan_tree(nodes: list[dict[str, Any]]) -> dict[str, Any]: """Validate a plan tree represented as a list of node dicts. @@ -151,13 +153,10 @@ def validate_plan_tree(nodes: list[dict[str, Any]]) -> dict[str, Any]: # --- type --- node_type = node.get("type") - if ( - node_type is not None - and (not isinstance(node_type, str) or not node_type.strip()) + if node_type is not None and ( + not isinstance(node_type, str) or not node_type.strip() ): - errors.append( - f"node[{idx}] 'type' must be a non-empty string" - ) + errors.append(f"node[{idx}] 'type' must be a non-empty string") valid = False # --- sequence --- @@ -186,13 +185,10 @@ def validate_plan_tree(nodes: list[dict[str, Any]]) -> dict[str, Any]: # --- question --- question = node.get("question") - if ( - question is not None - and (not isinstance(question, str) or not question.strip()) + if question is not None and ( + not isinstance(question, str) or not question.strip() ): - errors.append( - f"node[{idx}] 'question' must be a non-empty string" - ) + errors.append(f"node[{idx}] 'question' must be a non-empty string") valid = False # --- children --- @@ -227,6 +223,7 @@ def validate_plan_tree(nodes: list[dict[str, Any]]) -> dict[str, Any]: # Decision CLI dict validator # --------------------------------------------------------------------------- + def validate_decision_dict(d: dict[str, Any]) -> dict[str, Any]: """Validate a decision CLI output dictionary. @@ -260,18 +257,14 @@ def validate_decision_dict(d: dict[str, Any]) -> dict[str, Any]: # --- ULID fields --- if field in ("decision_id", "plan_id"): if not isinstance(value, str) or not _ULID_RE.match(value): - errors.append( - f"'{field}' must be a valid ULID, got {value!r}" - ) + errors.append(f"'{field}' must be a valid ULID, got {value!r}") valid = False # --- confidence range --- elif field == "confidence": if value is not None and isinstance(value, float): if value < 0.0 or value > 1.0: - errors.append( - f"'{field}' must be in range [0.0, 1.0], got {value}" - ) + errors.append(f"'{field}' must be in range [0.0, 1.0], got {value}") valid = False elif value is None: pass # None is acceptable for confidence @@ -287,9 +280,7 @@ def validate_decision_dict(d: dict[str, Any]) -> dict[str, Any]: errors.append(f"'{field}' must be a string, got {type(value).__name__}") valid = False elif value != "(root)" and not _ULID_RE.match(value): - errors.append( - f"'{field}' must be a ULID or '(root)', got {value!r}" - ) + errors.append(f"'{field}' must be a ULID or '(root)', got {value!r}") valid = False # --- boolean fields --- @@ -308,9 +299,7 @@ def validate_decision_dict(d: dict[str, Any]) -> dict[str, Any]: # --- sequence (int) --- elif not (isinstance(value, int) and not isinstance(value, bool)): - errors.append( - f"'{field}' must be an integer, got {type(value).__name__}" - ) + errors.append(f"'{field}' must be an integer, got {type(value).__name__}") valid = False return { @@ -325,6 +314,7 @@ def validate_decision_dict(d: dict[str, Any]) -> dict[str, Any]: # StructuredOutput validator # --------------------------------------------------------------------------- + def validate_structured_output( output: dict[str, Any], ) -> dict[str, Any]: @@ -360,17 +350,14 @@ def validate_structured_output( # --- session_id (ULID) --- session_id = output.get("session_id") if not isinstance(session_id, str) or not _ULID_RE.match(session_id): - errors.append( - f"'session_id' must be a valid ULID, got '{session_id!r}'" - ) + errors.append(f"'session_id' must be a valid ULID, got '{session_id!r}'") valid = False # --- status (membership) --- status = output.get("status") if not isinstance(status, str) or status not in VALID_STATUSES: errors.append( - f"'status' must be one of {sorted(VALID_STATUSES)}, " - f"got '{status!r}'" + f"'status' must be one of {sorted(VALID_STATUSES)}, got '{status!r}'" ) valid = False @@ -389,9 +376,7 @@ def validate_structured_output( # --- elements (list of dicts with kind) --- elements = output.get("elements", []) if not isinstance(elements, list): - errors.append( - f"'elements' must be a list, got {type(elements).__name__}" - ) + errors.append(f"'elements' must be a list, got {type(elements).__name__}") valid = False else: for ei, elem in enumerate(elements): @@ -423,14 +408,12 @@ _TARGET_TYPE_MAP: dict[str, Any] = { "decisions": validate_plan_tree, "decision-tree": validate_plan_tree, "tree": validate_plan_tree, - # --- Decision CLI targets --- "decision": validate_decision_dict, "decision_cli": validate_decision_dict, "decision-cli": validate_decision_dict, "cli_dict": validate_decision_dict, "as_cli_dict": validate_decision_dict, - # --- StructuredOutput targets --- "structured_output": validate_structured_output, "structured-output": validate_structured_output, @@ -472,8 +455,7 @@ def validate_structured_component_output( if validator is None: available = sorted(_TARGET_TYPE_MAP.keys()) raise ValidationError( - f"Unknown target_type '{target_type}'. " - f"Available targets: {available}", + f"Unknown target_type '{target_type}'. Available targets: {available}", errors=[f"valid types: {', '.join(available)}"], ) -- 2.52.0 From 80d27df9cc34bf83fd04d5b1101f6eba864222ae Mon Sep 17 00:00:00 2001 From: Jeffrey Phillips Freeman Date: Wed, 13 May 2026 11:38:06 +0000 Subject: [PATCH 6/8] fix(changelog): correct PR #11161 references in CHANGELOG.md and CONTRIBUTORS.md ISSUES CLOSED: #11147 --- CHANGELOG.md | 2 +- CONTRIBUTORS.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 00bcd338f..808a74259 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,7 +54,7 @@ Changed `wf10_batch.robot` to be less likely to create files, and ## [Unreleased] -- **Structural Component Output Validation** (#11147): Replaces exact character matching with structural component checking for output validation. Implements three validators covering plan tree output, decision CLI dicts, and structured session snapshots. The `validate_plan_tree` function validates node dicts for required keys (`decision_id`, `type`, `sequence`, `question`, `children`), ULID format, correct types, and sibling ordering. The `validate_decision_dict` function validates decision CLI output against the `Decision.as_cli_dict()` schema with field presence, type, ULID pattern, confidence range [0..1], and boolean field checks. The `validate_structured_output` function validates the StructuredOutput envelope for `command`, `session_id` (ULID), status membership, `exit_code`, and elements integrity. A unified dispatcher (`validate_structured_component_output`) enables routing by target_type. BDD test coverage added in `features/structural_validation.feature`. [Epic #8137](https://git.cleverthis.com/cleveragents/cleveragents-core/issues/8137) +- **Structural Component Output Validation** (#11161): Replaces exact character matching with structural component checking for output validation. Implements three validators covering plan tree output, decision CLI dicts, and structured session snapshots. The `validate_plan_tree` function validates node dicts for required keys (`decision_id`, `type`, `sequence`, `question`, `children`), ULID format, correct types, and sibling ordering. The `validate_decision_dict` function validates decision CLI output against the `Decision.as_cli_dict()` schema with field presence, type, ULID pattern, confidence range [0..1], and boolean field checks. The `validate_structured_output` function validates the StructuredOutput envelope for `command`, `session_id` (ULID), status membership, `exit_code`, and elements integrity. A unified dispatcher (`validate_structured_component_output`) enables routing by target_type. BDD test coverage added in `features/structural_validation.feature`. [Epic #8137](https://git.cleverthis.com/cleveragents/cleveragents-core/issues/8137) - **`task-implementor` posts work-started notification comments** (#11031): Both the `issue_impl` and `pr_fix` procedures now post an informational "work diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index 87976582c..0e95ce9e2 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -24,7 +24,7 @@ Below are some of the specific details of various contributions. * HAL 9000 has contributed the plugin entry point security hardening fix (#7476): enforced entry point allowlist validation before importing plugin modules to prevent malicious plugin loading. * HAL 9000 has contributed the benchmark workflow separation (#9040): moved the benchmark-regression job out of the default PR workflow into a dedicated scheduled workflow, reducing median PR CI turnaround time from 99-132 minutes to under 30 minutes. * HAL 9000 has contributed the plan tree JSON/YAML command envelope fix (#9163): wrapped `agents plan tree --format json/yaml` output in the spec-required command envelope structure, added summary statistics, decision_ids mapping, child_plans list, and accurate timing measurement. - * HAL 9000 contributed Structural Component Output Validation (PR #11147): implemented `validate_plan_tree`, `validate_decision_dict`, `validate_structured_output`, and `validate_structured_component_output` validators that replace exact-character matching with structural schema checking for plan tree nodes, decision CLI dictionaries, and structured session output envelopes. + * HAL 9000 contributed Structural Component Output Validation (PR #11161): implemented `validate_plan_tree`, `validate_decision_dict`, `validate_structured_output`, and `validate_structured_component_output` validators that replace exact-character matching with structural schema checking for plan tree nodes, decision CLI dictionaries, and structured session output envelopes. * This project was made possible thanks to considerable donation of time, money, and resources by CleverThis, Inc. * HAL 9000 has contributed automated bug fixes, CLI output formatting improvements, and ongoing maintenance as part of the CleverAgents automation system. * HAL 9000 has contributed the file edit encoding parameter fix (PR #8258 / issue #7559). -- 2.52.0 From ac81ea2ceb1532d833b0ac6f0d217b86a81afa94 Mon Sep 17 00:00:00 2001 From: CleverThis Date: Wed, 13 May 2026 21:40:52 +0000 Subject: [PATCH 7/8] 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 -- 2.52.0 From 6d46baf5520f783de0201ae0f1800a1ff6be134f Mon Sep 17 00:00:00 2001 From: CleverThis Date: Wed, 13 May 2026 22:06:23 +0000 Subject: [PATCH 8/8] fix: apply ruff format to step definitions file --- features/steps/structural_validation_steps.py | 30 +++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/features/steps/structural_validation_steps.py b/features/steps/structural_validation_steps.py index 706776807..f41cf7c1e 100644 --- a/features/steps/structural_validation_steps.py +++ b/features/steps/structural_validation_steps.py @@ -31,7 +31,7 @@ VALID_ULID_C = "01ARZ3NDEKTSV4XXFFJFRC889C" # ──────────────────────────────────────────────────────────── -@given('the plan tree contains 1 node(s)') +@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 = [ @@ -45,7 +45,7 @@ def step_plan_tree_1_node(ctx: Context) -> None: ] -@given('the plan tree contains 5 node(s)') +@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 = [ @@ -61,7 +61,7 @@ def step_plan_tree_5_nodes(ctx: Context) -> None: ] -@given('the plan tree contains 20 node(s)') +@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 = [ @@ -354,37 +354,49 @@ def step_validate_structured(ctx: Context) -> None: ctx.validation_result = validate_structured_output(o) -@given('I have a structured output with "command" set to agents plan list (and all other required fields present)') +@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["command"] = "agents plan list" -@given('I have a structured output with "session_id" set to 01ARZ3NDEKTSV4XXFFJFRC889A (and all other required fields present)') +@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)') +@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)') +@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)') +@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)') +@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 -- 2.52.0