fix(action): address review feedback — repair BDD steps and remove pytest file
Fix all blocking issues identified in reviews 8578 and 8584:
1. Remove duplicate @then('action argument {idx:d} type should be "{expected}"')
step registered twice (lines 432 & 517) — caused AmbiguousStep error.
2. Remove duplicate @when('I validate the action schema expecting failure')
registered at lines 281 and 616 — caused AmbiguousStep error.
3. Remove duplicate @given('the environment variable "{env_var}" is set to "{value}"')
at action_schema_steps.py — already defined in settings_steps.py.
4. Fix Behave docstring API: step_given_yaml_multiline had extra yaml_content
parameter; Behave docstrings are read from context.text, not passed as args.
5. Fix logic bug in step_then_argument_defaults: was asserting all args have
defaults instead of counting args with non-None defaults.
6. Fix type mismatch in step_then_argument_with_default: int default 42 was
compared to string '42'; added numeric coercion before comparison.
7. Add missing step definitions: the actual type should be mentioned,
the expected type should be mentioned, and the boolean-default-true Given.
8. Fix @tdd_issue_8322 -> @tdd_issue_9105 in action_schema.feature.
9. Fix CHANGELOG issue references: #8322 -> #9178.
10. Remove tests/action/test_action_argument_schema.py (pytest in wrong dir/
wrong framework; project mandates Behave in features/ only).
11. Fix lint errors in schema.py and action.py (E501 line-too-long, SIM102
nested-if); fix Pyright type-narrowing for numeric comparisons.
12. Rewrite action_schema.feature step calls to use action schema error should
mention and correct docstring indentation (triple-quoted YAML blocks).
nox -s lint, format --check, and typecheck all pass.
ISSUES CLOSED: #9105
This commit is contained in:
+2
-2
@@ -319,7 +319,7 @@ ensuring data is stored with proper parameter values.
|
||||
`AUTO-REV-SUP` prefix. The agent was already using `AUTO-REV-SUP` in production; this change
|
||||
aligns the documentation with the actual runtime behaviour.
|
||||
|
||||
- **ActionArgumentSchema default value type validation** (#8322, #9105): Added
|
||||
- **ActionArgumentSchema default value type validation** (#9178, #9105): Added
|
||||
a `@model_validator` to `ActionArgumentSchema` that validates the default value
|
||||
matches its declared argument type. Type mappings enforced:
|
||||
* "string" → str
|
||||
@@ -332,7 +332,7 @@ ensuring data is stored with proper parameter values.
|
||||
messages. This fixes issue #9105 where invalid configurations could pass
|
||||
silently. Also validates min_value/max_value bounds against the default when present.
|
||||
|
||||
- **ActionArgument domain model default value type validation** (#8322): Added
|
||||
- **ActionArgument domain model default value type validation** (#9178, #9105): Added
|
||||
a matching `@model_validator` to `ActionArgument` in the domain model so that
|
||||
CLI-parsed and YAML-mapped arguments are also validated at instance construction
|
||||
time, not only during schema loading. This prevents inconsistent argument definitions
|
||||
|
||||
@@ -27,102 +27,102 @@ Feature: Action Schema Validation
|
||||
# Invalid default: string where integer expected
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@tdd_issue @tdd_issue_8322
|
||||
@tdd_issue @tdd_issue_9105
|
||||
Scenario: String default for integer argument fails validation
|
||||
Given an action YAML with integer arg as string "forty-two"
|
||||
When I validate the action schema expecting failure
|
||||
Then the action schema validation should fail
|
||||
And the error should mention 'my_arg'
|
||||
And the action schema error should mention "count"
|
||||
And the actual type should be mentioned
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Invalid default: integer where string expected
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@tdd_issue @tdd_issue_8322
|
||||
@tdd_issue @tdd_issue_9105
|
||||
Scenario: Integer default for string argument fails validation
|
||||
Given an action YAML string with argument named "my_arg" as string type and integer default 123
|
||||
When I validate the action schema expecting failure
|
||||
Then the action schema validation should fail
|
||||
And the error should mention 'argument'
|
||||
And the action schema error should mention "my_arg"
|
||||
And the expected type should be mentioned
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Invalid default: string where boolean expected
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@tdd_issue @tdd_issue_8322
|
||||
@tdd_issue @tdd_issue_9105
|
||||
Scenario: String "true" default for boolean argument fails validation
|
||||
Given an action YAML with integer arg as string "forty-two"
|
||||
Given an action YAML string with argument named "verbose" as boolean type and string default "true"
|
||||
When I validate the action schema expecting failure
|
||||
Then the action schema validation should fail
|
||||
And the error should mention 'my_arg'
|
||||
And the action schema error should mention "Default value type mismatch"
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Invalid default: string where float expected
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@tdd_issue @tdd_issue_8322
|
||||
@tdd_issue @tdd_issue_9105
|
||||
Scenario: String default for float argument fails validation
|
||||
Given an action YAML with integer arg as string "forty-two"
|
||||
Given an action YAML string with argument named "rate" as float type and string default "high"
|
||||
When I validate the action schema expecting failure
|
||||
Then the action schema validation should fail
|
||||
And the error should mention 'my_arg'
|
||||
And the action schema error should mention "Default value type mismatch"
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Invalid default: string where list expected
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@tdd_issue @tdd_issue_8322
|
||||
@tdd_issue @tdd_issue_9105
|
||||
Scenario: String default for list argument fails validation
|
||||
Given an action YAML with integer arg as string "forty-two"
|
||||
Given an action YAML string with argument named "targets" as list type and string default "single-value"
|
||||
When I validate the action schema expecting failure
|
||||
Then the action schema validation should fail
|
||||
And the error should mention 'my_arg'
|
||||
And the action schema error should mention "Default value type mismatch"
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Min/max bounds validation against defaults
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@tdd_issue @tdd_issue_8322
|
||||
@tdd_issue @tdd_issue_9105
|
||||
Scenario: Default below min_value is rejected
|
||||
Given an action YAML string:
|
||||
"""\
|
||||
name: local/below-min-default
|
||||
description: Action with default below min_value
|
||||
strategy_actor: openai/gpt-4
|
||||
execution_actor: openai/gpt-4
|
||||
definition_of_done: Done
|
||||
arguments:
|
||||
- name: priority
|
||||
type: integer
|
||||
default: -10
|
||||
min_value: 0
|
||||
max_value: 100
|
||||
"""
|
||||
name: local/below-min-default
|
||||
description: Action with default below min_value
|
||||
strategy_actor: openai/gpt-4
|
||||
execution_actor: openai/gpt-4
|
||||
definition_of_done: Done
|
||||
arguments:
|
||||
- name: priority
|
||||
type: integer
|
||||
default: -10
|
||||
min_value: 0
|
||||
max_value: 100
|
||||
"""
|
||||
When I validate the action schema expecting failure
|
||||
Then the action schema validation should fail
|
||||
And the error should mention 'min_value'
|
||||
And the action schema error should mention "min_value"
|
||||
|
||||
@tdd_issue @tdd_issue_8322
|
||||
@tdd_issue @tdd_issue_9105
|
||||
Scenario: Default above max_value is rejected
|
||||
Given an action YAML string:
|
||||
"""\
|
||||
name: local/above-max-default
|
||||
description: Action with default above max_value
|
||||
strategy_actor: openai/gpt-4
|
||||
execution_actor: openai/gpt-4
|
||||
definition_of_done: Done
|
||||
arguments:
|
||||
- name: priority
|
||||
type: integer
|
||||
default: 200
|
||||
min_value: 0
|
||||
max_value: 100
|
||||
"""
|
||||
name: local/above-max-default
|
||||
description: Action with default above max_value
|
||||
strategy_actor: openai/gpt-4
|
||||
execution_actor: openai/gpt-4
|
||||
definition_of_done: Done
|
||||
arguments:
|
||||
- name: priority
|
||||
type: integer
|
||||
default: 200
|
||||
min_value: 0
|
||||
max_value: 100
|
||||
"""
|
||||
When I validate the action schema expecting failure
|
||||
Then the action schema validation should fail
|
||||
And the error should mention 'max_value'
|
||||
And the action schema error should mention "max_value"
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Valid min/max bounds — defaults within ranges pass
|
||||
@@ -138,12 +138,12 @@ arguments:
|
||||
# Bool vs int edge case — Python bool is subclass of int
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@tdd_issue @tdd_issue_8322
|
||||
@tdd_issue @tdd_issue_9105
|
||||
Scenario: Boolean default for integer argument is rejected (bool is subclass of int)
|
||||
Given an action YAML string with argument named "count" as {type_} type and boolean default true
|
||||
Given an action YAML string with argument named "count" as integer type and boolean default true
|
||||
When I validate the action schema expecting failure
|
||||
Then the action schema validation should fail
|
||||
And the error should mention 'boolean'
|
||||
And the action schema error should mention "not a boolean"
|
||||
And the expected type should be mentioned
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -153,19 +153,19 @@ arguments:
|
||||
@happy_path
|
||||
Scenario: Arguments with min/max but no default still validate normally
|
||||
Given an action YAML string:
|
||||
"""\
|
||||
name: local/min-max-no-default
|
||||
description: Action with min/max but no default
|
||||
strategy_actor: openai/gpt-4
|
||||
execution_actor: openai/gpt-4
|
||||
definition_of_done: Done
|
||||
arguments:
|
||||
- name: count
|
||||
type: integer
|
||||
required: true
|
||||
description: Count must be between 1 and 100
|
||||
min_value: 1
|
||||
max_value: 100
|
||||
"""
|
||||
name: local/min-max-no-default
|
||||
description: Action with min/max but no default
|
||||
strategy_actor: openai/gpt-4
|
||||
execution_actor: openai/gpt-4
|
||||
definition_of_done: Done
|
||||
arguments:
|
||||
- name: count
|
||||
type: integer
|
||||
required: true
|
||||
description: Count must be between 1 and 100
|
||||
min_value: 1
|
||||
max_value: 100
|
||||
"""
|
||||
When I validate the action schema
|
||||
Then the action schema validation should succeed
|
||||
|
||||
@@ -909,7 +909,7 @@ arguments:
|
||||
"""
|
||||
When I validate the action schema expecting failure
|
||||
Then the action schema validation should fail
|
||||
And the error should mention "not a boolean"
|
||||
And the action schema error should mention "not a boolean"
|
||||
|
||||
|
||||
Scenario: Integer default that is a string fails type validation
|
||||
@@ -917,15 +917,15 @@ arguments:
|
||||
When I validate the action schema expecting failure
|
||||
Then the action schema validation should fail
|
||||
And the action schema error should mention "Default value type mismatch"
|
||||
And the error should mention "'integer'"
|
||||
And the action schema error should mention "'integer'"
|
||||
|
||||
|
||||
Scenario: String default that is an integer fails type validation
|
||||
Given an action YAML string with argument named "my_arg" as string type and integer default 123
|
||||
When I validate the action schema expecting failure
|
||||
Then the action schema validation should fail
|
||||
And the error should mention "argument"
|
||||
And the error should mention "'string'"
|
||||
And the action schema error should mention "my_arg"
|
||||
And the action schema error should mention "'string'"
|
||||
|
||||
|
||||
Scenario: Boolean default that is a string fails type validation
|
||||
@@ -953,6 +953,6 @@ arguments:
|
||||
Given an action YAML string with argument named "my_arg" as integer type and string default "nope"
|
||||
When I validate the action schema expecting failure
|
||||
Then the action schema validation should fail
|
||||
And the error should mention "argument"
|
||||
And the error should mention "'integer'"
|
||||
And the action schema error should mention "my_arg"
|
||||
And the action schema error should mention "'integer'"
|
||||
|
||||
|
||||
@@ -489,19 +489,24 @@ arguments:
|
||||
"""
|
||||
|
||||
|
||||
@then('the action config should have {count:d} arguments with defaults')
|
||||
@then("the action config should have {count:d} arguments with defaults")
|
||||
def step_then_argument_defaults(context: Context, count: int) -> None:
|
||||
"""Assert the number of parsed arguments that have non-None defaults."""
|
||||
assert context.action_config is not None
|
||||
for arg in context.action_config.arguments:
|
||||
if count == 0:
|
||||
assert arg.default is None
|
||||
else:
|
||||
assert arg.default is not None
|
||||
actual = sum(
|
||||
1 for arg in context.action_config.arguments if arg.default is not None
|
||||
)
|
||||
assert actual == count, (
|
||||
f"Expected {count} args with non-None defaults, got {actual}"
|
||||
)
|
||||
|
||||
|
||||
@then('action argument {idx:d} name should be "{expected}" with default "{default_value}"')
|
||||
def step_then_argument_with_default(context: Context, idx: int, expected: str, default_value: str) -> None:
|
||||
@then(
|
||||
'action argument {idx:d} name should be "{expected}" with default "{default_value}"'
|
||||
)
|
||||
def step_then_argument_with_default(
|
||||
context: Context, idx: int, expected: str, default_value: str
|
||||
) -> None:
|
||||
"""Assert an argument has a specific default value."""
|
||||
assert context.action_config is not None
|
||||
arg = context.action_config.arguments[idx]
|
||||
@@ -511,18 +516,41 @@ def step_then_argument_with_default(context: Context, idx: int, expected: str, d
|
||||
elif default_value.lower() == "false":
|
||||
assert arg.default is False
|
||||
else:
|
||||
assert arg.default == default_value
|
||||
# Attempt numeric coercion so integer/float defaults compare correctly
|
||||
try:
|
||||
coerced: int | float = int(default_value)
|
||||
assert arg.default == coerced
|
||||
except ValueError:
|
||||
try:
|
||||
coerced_f: float = float(default_value)
|
||||
assert arg.default == coerced_f
|
||||
except ValueError:
|
||||
assert arg.default == default_value
|
||||
|
||||
|
||||
@then('action argument {idx:d} type should be "{expected}"')
|
||||
def step_then_argument_type(context: Context, idx: int, expected: str) -> None:
|
||||
"""Assert an argument type by index."""
|
||||
assert context.action_config is not None
|
||||
arg = context.action_config.arguments[idx]
|
||||
assert arg.type == expected
|
||||
@then("the actual type should be mentioned")
|
||||
def step_then_actual_type_mentioned(context: Context) -> None:
|
||||
"""Assert the error message includes a Python type name (str, int, float, bool, list)."""
|
||||
assert context.action_schema_error is not None, "No error was captured"
|
||||
assert any(
|
||||
t in context.action_schema_error
|
||||
for t in ["str", "int", "float", "bool", "list"]
|
||||
), (
|
||||
f"Expected an actual Python type name in error, got: {context.action_schema_error}"
|
||||
)
|
||||
|
||||
|
||||
@then('the action schema error should mention {match_text}')
|
||||
@then("the expected type should be mentioned")
|
||||
def step_then_expected_type_mentioned(context: Context) -> None:
|
||||
"""Assert the error message includes a declared type name (string, integer, float, boolean, list)."""
|
||||
assert context.action_schema_error is not None, "No error was captured"
|
||||
assert any(
|
||||
t in context.action_schema_error
|
||||
for t in ["string", "integer", "float", "boolean", "list"]
|
||||
), f"Expected a declared type name in error, got: {context.action_schema_error}"
|
||||
|
||||
|
||||
@then("the action schema error should mention {match_text}")
|
||||
def step_then_error_mentions_match(context: Context, match_text: str) -> None:
|
||||
"""Assert the error message contains a specific pattern (supports regex-like matching)."""
|
||||
assert context.action_schema_error is not None, "No error was captured"
|
||||
@@ -553,59 +581,20 @@ def step_then_none_arg_passthrough(context: Context) -> None:
|
||||
assert arg.default is None
|
||||
|
||||
|
||||
@then('the error should mention {pattern}')
|
||||
def step_then_error_mentions_pattern(context: Context, pattern: str) -> None:
|
||||
"""Assert the error message mentions a specific pattern."""
|
||||
assert context.action_schema_error is not None, "No error was captured"
|
||||
|
||||
if 'argument' in pattern and "'my_arg'" in pattern:
|
||||
assert "my_arg" in context.action_schema_error, (
|
||||
f"Expected error to mention 'my_arg', got: {context.action_schema_error}"
|
||||
)
|
||||
elif 'expected type' in pattern.lower() or "'string'" in pattern:
|
||||
assert "string" in context.action_schema_error.lower(), (
|
||||
f"Expected error to mention 'string', got: {context.action_schema_error}"
|
||||
)
|
||||
elif 'actual type' in pattern.lower() or "got" in pattern:
|
||||
assert "int" in context.action_schema_error, (
|
||||
f"Expected error to mention an actual type, got: {context.action_schema_error}"
|
||||
)
|
||||
|
||||
|
||||
|
||||
# ── Additional given steps for default value type tests ────────────────
|
||||
|
||||
|
||||
@given("an action YAML with typed arguments and defaults")
|
||||
def step_given_yaml_with_typed_defaults(context: Context) -> None:
|
||||
"""Provide YAML with all valid default types."""
|
||||
context.action_yaml_string = _VALID_YAML_WITH_DEFAULTS
|
||||
|
||||
|
||||
@given('the environment variable "{env_var}" is set to "{value}"')
|
||||
def step_given_env_var_set(context: Context, env_var: str, value: str) -> None:
|
||||
"""Set an environment variable for interpolation tests."""
|
||||
import os
|
||||
os.environ[env_var] = value
|
||||
|
||||
|
||||
@when("I validate the action schema expecting failure")
|
||||
def step_when_validate_expecting_failure_default(context: Context) -> None:
|
||||
"""Validate the action YAML, expecting it to fail.
|
||||
|
||||
Alias: reuses the existing step function.
|
||||
"""
|
||||
try:
|
||||
context.action_config = ActionConfigSchema.from_yaml(context.action_yaml_string)
|
||||
context.action_schema_error = None
|
||||
except (ValidationError, ValueError) as exc:
|
||||
context.action_config = None
|
||||
context.action_schema_error = str(exc)
|
||||
|
||||
|
||||
@given('an action YAML string:')
|
||||
def step_given_yaml_multiline(context: Context, yaml_content: str) -> None:
|
||||
"""Provide a multi-line YAML string from the scenario."""
|
||||
context.action_yaml_string = yaml_content.strip()
|
||||
@given("an action YAML string:")
|
||||
def step_given_yaml_multiline(context: Context) -> None:
|
||||
"""Provide a multi-line YAML string from the scenario docstring."""
|
||||
assert context.text is not None, "Expected a docstring in the scenario"
|
||||
context.action_yaml_string = context.text.strip()
|
||||
|
||||
|
||||
# ── Steps for specific bad-default scenarios ───────────────────────────
|
||||
@@ -677,32 +666,62 @@ def step_given_bad_int_for_string(context: Context) -> None:
|
||||
context.action_yaml_string = _BAD_INT_STRING_YAML
|
||||
|
||||
|
||||
@given('an action YAML string with argument named "{arg_name}" as {type_} type and integer default 123')
|
||||
@given(
|
||||
'an action YAML string with argument named "{arg_name}" as {type_} type and integer default 123'
|
||||
)
|
||||
def step_given_bad_str_for_int(context: Context, arg_name: str, type_: str) -> None:
|
||||
"""Provide YAML with an int default where a string is expected."""
|
||||
context.action_yaml_string = _BAD_STR_INT_YAML
|
||||
|
||||
|
||||
@given('an action YAML string with argument named "{arg_name}" as {type_} type and string default "true"')
|
||||
@given(
|
||||
'an action YAML string with argument named "{arg_name}" as {type_} type and string default "true"'
|
||||
)
|
||||
def step_given_bad_str_for_bool(context: Context, arg_name: str, type_: str) -> None:
|
||||
"""Provide YAML with a string default where bool is expected."""
|
||||
context.action_yaml_string = _BAD_BOOL_STR_YAML
|
||||
|
||||
|
||||
@given('an action YAML string with argument named "{arg_name}" as {type_} type and string default "high"')
|
||||
@given(
|
||||
'an action YAML string with argument named "{arg_name}" as {type_} type and string default "high"'
|
||||
)
|
||||
def step_given_bad_str_for_float(context: Context, arg_name: str, type_: str) -> None:
|
||||
"""Provide YAML with a string default where float is expected."""
|
||||
context.action_yaml_string = _BAD_FLOAT_STR_YAML
|
||||
|
||||
|
||||
@given('an action YAML string with argument named "{arg_name}" as {type_} type and string default "single-value"')
|
||||
@given(
|
||||
'an action YAML string with argument named "{arg_name}" as {type_} type and string default "single-value"'
|
||||
)
|
||||
def step_given_bad_str_for_list(context: Context, arg_name: str, type_: str) -> None:
|
||||
"""Provide YAML with a string default where list is expected."""
|
||||
context.action_yaml_string = _BAD_LIST_STR_YAML
|
||||
|
||||
|
||||
@given('an action YAML string with argument named "{arg_name}" as integer type and string default "nope"')
|
||||
@given(
|
||||
'an action YAML string with argument named "{arg_name}" as integer type and string default "nope"'
|
||||
)
|
||||
def step_given_bad_int_for_str_pattern(context: Context, arg_name: str) -> None:
|
||||
"""Provide YAML for error pattern testing."""
|
||||
context.action_yaml_string = _BAD_INT_STRING_YAML
|
||||
|
||||
|
||||
_BAD_BOOL_INT_YAML = """\
|
||||
name: local/bad-bool-int
|
||||
description: Action with boolean default for integer arg
|
||||
strategy_actor: openai/gpt-4
|
||||
execution_actor: openai/gpt-4
|
||||
definition_of_done: Done
|
||||
arguments:
|
||||
- name: count
|
||||
type: integer
|
||||
default: true
|
||||
"""
|
||||
|
||||
|
||||
@given(
|
||||
'an action YAML string with argument named "{arg_name}" as integer type and boolean default true'
|
||||
)
|
||||
def step_given_bad_bool_for_int(context: Context, arg_name: str) -> None:
|
||||
"""Provide YAML with a boolean default where integer is expected."""
|
||||
context.action_yaml_string = _BAD_BOOL_INT_YAML
|
||||
|
||||
@@ -175,7 +175,9 @@ class ActionArgumentSchema(BaseModel):
|
||||
|
||||
case "integer":
|
||||
# Python bool is a subclass of int — reject booleans explicitly
|
||||
if isinstance(default_value, bool) or not isinstance(default_value, int):
|
||||
is_bool = isinstance(default_value, bool)
|
||||
is_not_int = is_bool or not isinstance(default_value, int)
|
||||
if is_not_int:
|
||||
actual = type(default_value).__name__
|
||||
raise ValueError(
|
||||
f"Default value type mismatch for argument '{self.name}': "
|
||||
@@ -218,24 +220,17 @@ class ActionArgumentSchema(BaseModel):
|
||||
"The default value must be a list."
|
||||
)
|
||||
|
||||
# Validate min/max bounds when default is present and numeric
|
||||
if (
|
||||
self.min_value is not None
|
||||
and isinstance(default_value, (int, float))
|
||||
and not isinstance(default_value, bool)
|
||||
# Validate min/max bounds when default is present and numeric.
|
||||
# Use isinstance directly so Pyright can narrow the type for comparisons.
|
||||
if isinstance(default_value, (int, float)) and not isinstance(
|
||||
default_value, bool
|
||||
):
|
||||
if default_value < self.min_value:
|
||||
if self.min_value is not None and default_value < self.min_value:
|
||||
raise ValueError(
|
||||
f"Argument '{self.name}': default {default_value} "
|
||||
f"is less than min_value {self.min_value}."
|
||||
)
|
||||
|
||||
if (
|
||||
self.max_value is not None
|
||||
and isinstance(default_value, (int, float))
|
||||
and not isinstance(default_value, bool)
|
||||
):
|
||||
if default_value > self.max_value:
|
||||
if self.max_value is not None and default_value > self.max_value:
|
||||
raise ValueError(
|
||||
f"Argument '{self.name}': default {default_value} "
|
||||
f"is greater than max_value {self.max_value}."
|
||||
|
||||
@@ -219,7 +219,9 @@ class ActionArgument(BaseModel):
|
||||
|
||||
case ArgumentType.INTEGER:
|
||||
# Python bool is a subclass of int — reject booleans explicitly
|
||||
if isinstance(default_value, bool) or not isinstance(default_value, int):
|
||||
is_bool = isinstance(default_value, bool)
|
||||
is_not_int = is_bool or not isinstance(default_value, int)
|
||||
if is_not_int:
|
||||
actual = type(default_value).__name__
|
||||
raise ValueError(
|
||||
f"Default value type mismatch for argument '{self.name}': "
|
||||
@@ -262,24 +264,17 @@ class ActionArgument(BaseModel):
|
||||
"The default value must be a list."
|
||||
)
|
||||
|
||||
# Validate min/max bounds when default is present
|
||||
if (
|
||||
self.min_value is not None
|
||||
and isinstance(default_value, (int, float))
|
||||
and not isinstance(default_value, bool)
|
||||
# Validate min/max bounds when default is present and numeric.
|
||||
# Use isinstance directly so Pyright can narrow the type for comparisons.
|
||||
if isinstance(default_value, (int, float)) and not isinstance(
|
||||
default_value, bool
|
||||
):
|
||||
if default_value < self.min_value:
|
||||
if self.min_value is not None and default_value < self.min_value:
|
||||
raise ValueError(
|
||||
f"Argument '{self.name}': default {default_value} "
|
||||
f"is less than min_value {self.min_value}."
|
||||
)
|
||||
|
||||
if (
|
||||
self.max_value is not None
|
||||
and isinstance(default_value, (int, float))
|
||||
and not isinstance(default_value, bool)
|
||||
):
|
||||
if default_value > self.max_value:
|
||||
if self.max_value is not None and default_value > self.max_value:
|
||||
raise ValueError(
|
||||
f"Argument '{self.name}': default {default_value} "
|
||||
f"is greater than max_value {self.max_value}."
|
||||
|
||||
@@ -1,279 +0,0 @@
|
||||
"""Unit tests for ActionArgumentSchema default value type validation.
|
||||
|
||||
Tests cover the model_validator added in PR #9178 / issue #9105 that ensures
|
||||
default values match their declared types:
|
||||
- "string" → str
|
||||
- "integer" → int (not bool)
|
||||
- "float" → float or int
|
||||
- "boolean" → bool
|
||||
- "list" → list[constrained-str]
|
||||
|
||||
None defaults are always valid.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
from pydantic import ValidationError
|
||||
|
||||
from cleveragents.action.schema import ActionArgumentSchema
|
||||
|
||||
|
||||
class TestDefaultStringValue:
|
||||
"""Tests for string type default value validation."""
|
||||
|
||||
def test_valid_string_default(self) -> None:
|
||||
"""A str default with string type should pass."""
|
||||
arg = ActionArgumentSchema.model_validate(
|
||||
{"name": "target", "type": "string", "default": "hello"}
|
||||
)
|
||||
assert arg.default == "hello"
|
||||
|
||||
def test_invalid_integer_default_for_string_type_fails(self) -> None:
|
||||
"""An int default with string type should raise ValidationError."""
|
||||
with pytest.raises(ValidationError, match="Default value type mismatch"):
|
||||
ActionArgumentSchema.model_validate(
|
||||
{"name": "target", "type": "string", "default": 42}
|
||||
)
|
||||
|
||||
def test_invalid_bool_default_for_string_type_fails(self) -> None:
|
||||
"""A bool default with string type should raise ValidationError."""
|
||||
with pytest.raises(ValidationError, match="Default value type mismatch"):
|
||||
ActionArgumentSchema.model_validate(
|
||||
{"name": "target", "type": "string", "default": True}
|
||||
)
|
||||
|
||||
def test_invalid_list_default_for_string_type_fails(self) -> None:
|
||||
"""A list default with string type should raise ValidationError.
|
||||
|
||||
Note: lists get rejected earlier by union coercion, but the error
|
||||
still validates that a non-string was supplied. We accept either our
|
||||
custom message or pydantic's built-in type mismatch.
|
||||
"""
|
||||
with pytest.raises(ValidationError):
|
||||
ActionArgumentSchema.model_validate(
|
||||
{"name": "target", "type": "string", "default": [1, 2, 3]}
|
||||
)
|
||||
|
||||
|
||||
class TestDefaultIntegerValue:
|
||||
"""Tests for integer type default value validation."""
|
||||
|
||||
def test_valid_integer_default(self) -> None:
|
||||
"""An int default with integer type should pass."""
|
||||
arg = ActionArgumentSchema.model_validate(
|
||||
{"name": "count", "type": "integer", "default": 10}
|
||||
)
|
||||
assert arg.default == 10
|
||||
|
||||
def test_invalid_float_default_for_integer_type_fails(self) -> None:
|
||||
"""A float default with integer type should raise ValidationError."""
|
||||
with pytest.raises(ValidationError, match="Default value type mismatch"):
|
||||
ActionArgumentSchema.model_validate(
|
||||
{"name": "count", "type": "integer", "default": 10.5}
|
||||
)
|
||||
|
||||
def test_invalid_bool_default_for_integer_type_fails(self) -> None:
|
||||
"""A bool default with integer type should raise ValidationError (bool != int)."""
|
||||
with pytest.raises(ValidationError, match="not a boolean"):
|
||||
ActionArgumentSchema.model_validate(
|
||||
{"name": "count", "type": "integer", "default": True}
|
||||
)
|
||||
|
||||
def test_invalid_string_default_for_integer_type_fails(self) -> None:
|
||||
"""A string default with integer type should raise ValidationError."""
|
||||
with pytest.raises(ValidationError, match="Default value type mismatch"):
|
||||
ActionArgumentSchema.model_validate(
|
||||
{"name": "count", "type": "integer", "default": "forty-two"}
|
||||
)
|
||||
|
||||
|
||||
class TestDefaultFloatValue:
|
||||
"""Tests for float type default value validation."""
|
||||
|
||||
def test_valid_float_default(self) -> None:
|
||||
"""A float default with float type should pass."""
|
||||
arg = ActionArgumentSchema.model_validate(
|
||||
{"name": "rate", "type": "float", "default": 3.14}
|
||||
)
|
||||
assert arg.default == 3.14
|
||||
|
||||
def test_valid_int_default_for_float_type(self) -> None:
|
||||
"""An int default with float type should pass (int coerces to float)."""
|
||||
arg = ActionArgumentSchema.model_validate(
|
||||
{"name": "rate", "type": "float", "default": 2}
|
||||
)
|
||||
assert arg.default == 2
|
||||
|
||||
def test_invalid_bool_default_for_float_type_fails(self) -> None:
|
||||
"""A bool default with float type should raise ValidationError."""
|
||||
with pytest.raises(ValidationError, match="Default value type mismatch"):
|
||||
ActionArgumentSchema.model_validate(
|
||||
{"name": "rate", "type": "float", "default": True}
|
||||
)
|
||||
|
||||
def test_invalid_string_default_for_float_type_fails(self) -> None:
|
||||
"""A string default with float type should raise ValidationError."""
|
||||
with pytest.raises(ValidationError, match="Default value type mismatch"):
|
||||
ActionArgumentSchema.model_validate(
|
||||
{"name": "rate", "type": "float", "default": "high"}
|
||||
)
|
||||
|
||||
|
||||
class TestDefaultValueBoolean:
|
||||
"""Tests for boolean type default value validation."""
|
||||
|
||||
def test_valid_true_default(self) -> None:
|
||||
"""A True default with boolean type should pass."""
|
||||
arg = ActionArgumentSchema.model_validate(
|
||||
{"name": "verbose", "type": "boolean", "default": True}
|
||||
)
|
||||
assert arg.default is True
|
||||
|
||||
def test_valid_false_default(self) -> None:
|
||||
"""A False default with boolean type should pass."""
|
||||
arg = ActionArgumentSchema.model_validate(
|
||||
{"name": "debug", "type": "boolean", "default": False}
|
||||
)
|
||||
assert arg.default is False
|
||||
|
||||
def test_invalid_string_default_for_boolean_type_fails(self) -> None:
|
||||
"""A string default with boolean type should raise ValidationError."""
|
||||
with pytest.raises(ValidationError, match="Default value type mismatch"):
|
||||
ActionArgumentSchema.model_validate(
|
||||
{"name": "verbose", "type": "boolean", "default": "true"}
|
||||
)
|
||||
|
||||
def test_invalid_int_default_for_boolean_type_fails(self) -> None:
|
||||
"""An int default with boolean type should raise ValidationError."""
|
||||
with pytest.raises(ValidationError, match="Default value type mismatch"):
|
||||
ActionArgumentSchema.model_validate(
|
||||
{"name": "verbose", "type": "boolean", "default": 1}
|
||||
)
|
||||
|
||||
|
||||
class TestDefaultValueList:
|
||||
"""Tests for list type default value validation."""
|
||||
|
||||
def test_valid_list_default(self) -> None:
|
||||
"""A list default with list type should pass."""
|
||||
arg = ActionArgumentSchema.model_validate(
|
||||
{"name": "targets", "type": "list", "default": ["a", "b"]}
|
||||
)
|
||||
assert arg.default == ["a", "b"]
|
||||
|
||||
def test_valid_empty_list_default(self) -> None:
|
||||
"""An empty list default with list type should pass."""
|
||||
arg = ActionArgumentSchema.model_validate(
|
||||
{"name": "targets", "type": "list", "default": []}
|
||||
)
|
||||
assert arg.default == []
|
||||
|
||||
def test_invalid_string_default_for_list_type_fails(self) -> None:
|
||||
"""A string default with list type should raise ValidationError."""
|
||||
with pytest.raises(ValidationError, match="Default value type mismatch"):
|
||||
ActionArgumentSchema.model_validate(
|
||||
{"name": "targets", "type": "list", "default": "single-value"}
|
||||
)
|
||||
|
||||
def test_invalid_int_default_for_list_type_fails(self) -> None:
|
||||
"""An int default with list type should raise ValidationError."""
|
||||
with pytest.raises(ValidationError, match="Default value type mismatch"):
|
||||
ActionArgumentSchema.model_validate(
|
||||
{"name": "targets", "type": "list", "default": 42}
|
||||
)
|
||||
|
||||
|
||||
class TestNoneDefaults:
|
||||
"""Tests confirming None defaults always pass validation regardless of type."""
|
||||
|
||||
@pytest.mark.parametrize("arg_type", ["string", "integer", "float", "boolean", "list"])
|
||||
def test_none_defaults_always_valid(self, arg_type: str) -> None:
|
||||
"""A None default is always valid. No error should be raised."""
|
||||
arg = ActionArgumentSchema.model_validate(
|
||||
{"name": "arg", "type": arg_type, "default": None}
|
||||
)
|
||||
assert arg.default is None
|
||||
|
||||
|
||||
class TestErrorMessages:
|
||||
"""Tests for clear, actionable error messages."""
|
||||
|
||||
def test_error_message_name_included(self) -> None:
|
||||
"""The argument name should appear in the error message."""
|
||||
with pytest.raises(ValidationError, match="my_arg"):
|
||||
ActionArgumentSchema.model_validate(
|
||||
{"name": "my_arg", "type": "string", "default": 123}
|
||||
)
|
||||
|
||||
def test_error_message_expected_type(self) -> None:
|
||||
"""The expected type should appear in the error message."""
|
||||
with pytest.raises(ValidationError, match="'string'"):
|
||||
ActionArgumentSchema.model_validate(
|
||||
{"name": "my_arg", "type": "string", "default": 123}
|
||||
)
|
||||
|
||||
def test_error_message_actual_type(self) -> None:
|
||||
"""The actual type should appear in the error message."""
|
||||
with pytest.raises(ValidationError, match="got 'int'"):
|
||||
ActionArgumentSchema.model_validate(
|
||||
{"name": "my_arg", "type": "string", "default": 123}
|
||||
)
|
||||
|
||||
def test_integer_error_mentions_not_boolean(self) -> None:
|
||||
"""Integer mismatch with bool should mention 'not a boolean'."""
|
||||
with pytest.raises(ValidationError, match="not a boolean"):
|
||||
ActionArgumentSchema.model_validate(
|
||||
{"name": "count", "type": "integer", "default": True}
|
||||
)
|
||||
|
||||
|
||||
class TestIntegrationActionConfigSchema:
|
||||
"""Tests verifying the validation works end-to-end through ActionConfigSchema."""
|
||||
|
||||
def test_action_yaml_with_valid_default_passes(self) -> None:
|
||||
"""An action YAML with matching default types should validate."""
|
||||
yaml_str = """\
|
||||
name: local/test-action
|
||||
description: Test action
|
||||
strategy_actor: openai/gpt-4
|
||||
execution_actor: openai/gpt-4
|
||||
definition_of_done: Done
|
||||
arguments:
|
||||
- name: target_score
|
||||
type: integer
|
||||
default: 80
|
||||
- name: verbose
|
||||
type: boolean
|
||||
default: true
|
||||
- name: prefix
|
||||
type: string
|
||||
default: "test_"
|
||||
"""
|
||||
from cleveragents.action.schema import ActionConfigSchema
|
||||
|
||||
schema = ActionConfigSchema.from_yaml(yaml_str)
|
||||
target_score = schema.arguments[0]
|
||||
assert target_score.default == 80
|
||||
verbose_arg = schema.arguments[1]
|
||||
assert verbose_arg.default is True
|
||||
prefix = schema.arguments[2]
|
||||
assert prefix.default == "test_"
|
||||
|
||||
def test_action_yaml_with_type_mismatch_fails(self) -> None:
|
||||
"""An action YAML with mismatched default type should raise ValidationError."""
|
||||
yaml_str = """\
|
||||
name: local/test-action
|
||||
description: Test action
|
||||
strategy_actor: openai/gpt-4
|
||||
execution_actor: openai/gpt-4
|
||||
definition_of_done: Done
|
||||
arguments:
|
||||
- name: target_score
|
||||
type: integer
|
||||
default: "not a number"
|
||||
"""
|
||||
from cleveragents.action.schema import ActionConfigSchema
|
||||
|
||||
with pytest.raises(ValidationError):
|
||||
ActionConfigSchema.from_yaml(yaml_str)
|
||||
Reference in New Issue
Block a user