freemo/cleveragents-core
0
0
Fork 0
forked from cleveragents/cleveragents-core
Code Pull Requests Activity

Merge remote-tracking branch 'origin/master' into feature/m3-fix-actor-list-empty

Browse Source 
# Conflicts:
#	features/mocks/fake_provider.py
This commit is contained in:
Brent Edwards
2026-03-11 03:40:37 +00:00
parent 205e94eb52 5b9f0554c0
commit f2e44b2cf1
7 changed files with 508 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
benchmarks/tdd_actor_list_validation_bench.py
+74
View File
@@ -0,0 +1,74 @@
"""ASV benchmarks for TDD Bug #592 — actor list validation error.
Measures the performance of ``ActorRegistry.list_actors()`` when a provider
has a default model containing ``/`` characters. Establishes a baseline
before and after the bug fix.
Root cause: ``_actor_name()`` builds names via ``f"{provider}/{model}"``.
Multi-slash model names produce actor names with 2+ slashes, which
``ActorService._normalize_name()`` rejects with a ``ValidationError``.
"""
from __future__ import annotations
import contextlib
import sys
from pathlib import Path
from pydantic import ValidationError
# Ensure the local *source* tree is importable even when ASV has an
# older build of the package installed.
_SRC = str(Path(__file__).resolve().parents[1] / "src")
if _SRC not in sys.path:
sys.path.insert(0, _SRC)
# Ensure the features directory is importable so we can reuse shared
# mock stubs instead of duplicating them locally.
_FEATURES = str(Path(__file__).resolve().parents[1] / "features")
if _FEATURES not in sys.path:
sys.path.insert(0, _FEATURES)
from mocks.fake_provider import FakeProviderInfo, make_registry # noqa: E402
class TDDActorListValidationSuite:
"""Benchmark actor list with multi-slash provider models (TDD bug #592)."""
timeout = 60
def setup(self) -> None:
_, self._empty_registry = make_registry()
_, self._slash_registry = make_registry(
providers=[
FakeProviderInfo(
name="Openrouter",
default_model="anthropic/claude-sonnet-4-20250514",
),
]
)
def time_list_empty(self) -> None:
"""List actors with zero configured providers."""
self._empty_registry.list_actors()
def time_list_multi_slash_provider(self) -> None:
"""List actors when a multi-slash provider is configured."""
with contextlib.suppress(ValidationError):
self._slash_registry.list_actors()
def track_multi_slash_succeeds(self) -> int:
"""Track whether listing with a multi-slash provider succeeds.
Returns 1 when ``list_actors()`` completes without error; 0 if
``ValidationError`` is raised.
"""
try:
self._slash_registry.list_actors()
return 1
except ValidationError:
return 0
_track = TDDActorListValidationSuite.track_multi_slash_succeeds
_track.unit = "success" # type: ignore[attr-defined]
benchmarks/tdd_session_create_di_bench.py
+2 -3
View File
@@ -10,9 +10,8 @@ from __future__ import annotations
from unittest.mock import MagicMock
from benchmarks._session_bench_common import mock_session, runner
from cleveragents.cli.commands import session as session_mod # noqa: E402
from cleveragents.cli.commands.session import app as session_app # noqa: E402
from cleveragents.cli.commands import session as session_mod
from cleveragents.cli.commands.session import app as session_app
class TDDSessionCreateDISuite:
features/mocks/fake_provider.py
+48
View File
@@ -11,6 +11,8 @@ from dataclasses import dataclass, field
from typing import Any
from unittest.mock import MagicMock
from cleveragents.actor.registry import ActorRegistry
from cleveragents.domain.models.core.actor import Actor
from cleveragents.providers.registry import ProviderCapabilities
@@ -22,6 +24,7 @@ class FakeProviderInfo:
default_model: str
provider_type: Any = None
capabilities: Any = None
api_key_env_var: str = ""
def __post_init__(self) -> None:
if self.provider_type is None:
@@ -38,3 +41,48 @@ class FakeProviderRegistry:
def get_configured_providers(self) -> list[FakeProviderInfo]:
return self.providers
def make_registry(
providers: list[FakeProviderInfo] | None = None,
) -> tuple[MagicMock, ActorRegistry]:
"""Build a real ``ActorRegistry`` with mocked service/settings.
The ``ActorService`` is mocked so there is no database dependency, but
the ``ActorRegistry`` code (including ``_actor_name()``) runs for real.
Actors successfully created by ``upsert_actor`` are captured so that
``list_actors`` returns them — ensuring Scenario 3 ("valid JSON") can
detect when the bug is actually fixed.
"""
provider_reg = FakeProviderRegistry(providers=providers or [])
captured_actors: list[Actor] = []
def _capturing_upsert(**kwargs: Any) -> Actor:
actor = Actor(
name=kwargs.get("name", "mock/actor"),
provider=kwargs.get("provider", "mock"),
model=kwargs.get("model", "actor"),
config_blob={},
config_hash=Actor.compute_hash({}),
)
captured_actors.append(actor)
return actor
mock_service = MagicMock()
mock_service.list_actors.side_effect = lambda: list(captured_actors)
mock_service.get_default_actor.return_value = None
mock_service.upsert_actor.side_effect = _capturing_upsert
mock_settings = MagicMock()
mock_settings.resolve_provider_defaults.return_value = MagicMock(
provider=None, model=None
)
registry = ActorRegistry(
actor_service=mock_service,
provider_registry=provider_reg,
settings=mock_settings,
)
return mock_service, registry
features/steps/tdd_actor_list_validation_steps.py
+140
View File
@@ -0,0 +1,140 @@
"""Step definitions for TDD Bug #592 — actor list validation error.
These steps exercise the *real* ``ActorRegistry._actor_name()`` code path
with a provider whose default model name contains ``/`` characters. The
``@tdd_expected_fail`` tag on the scenarios inverts the result: these tests
**pass** CI while the bug is present and will **fail** once the bug is fixed
(signalling that the tag should be removed).
Root cause
~~~~~~~~~~
``ActorRegistry._actor_name()`` builds names via
``f"{provider}/{model}"``. For providers whose default model already
contains ``/`` (e.g. OpenRouter's ``anthropic/claude-sonnet-4-20250514``),
the result has 2+ slashes. ``ActorService._normalize_name()`` then raises
``ValidationError("Actor names must include exactly one '/' separator")``.
"""
from __future__ import annotations
import json
from typing import Any
from unittest.mock import patch
from behave import given, then, when
from typer.testing import CliRunner
from cleveragents.cli.commands.actor import app as actor_app
from features.mocks.fake_provider import FakeProviderInfo, make_registry
runner = CliRunner()
_PATCH_GET_SERVICES = "cleveragents.cli.commands.actor._get_services"
# ---------------------------------------------------------------------------
# Given steps
# ---------------------------------------------------------------------------
@given("an actor registry with a multi-slash model provider for tdd-actor-validation")
def step_multi_slash_provider(context: Any) -> None:
"""Set up a real ``ActorRegistry`` with a multi-slash model provider.
The provider name is ``Openrouter`` and the default model is
``anthropic/claude-sonnet-4-20250514`` — reproducing the exact
combination that triggers bug #592.
"""
fake_provider = FakeProviderInfo(
name="Openrouter",
default_model="anthropic/claude-sonnet-4-20250514",
)
service, registry = make_registry([fake_provider])
context.tdd_actor_service = service
context.tdd_actor_registry = registry
# ---------------------------------------------------------------------------
# When steps
# ---------------------------------------------------------------------------
@when("I run actor list via the tdd-actor-validation CLI")
def step_run_actor_list(context: Any) -> None:
"""Invoke ``actor list`` with the prepared registry."""
with patch(
_PATCH_GET_SERVICES,
return_value=(
context.tdd_actor_service,
context.tdd_actor_registry,
),
):
context.tdd_actor_result = runner.invoke(actor_app, ["list"])
@when("I run actor list with format json via the tdd-actor-validation CLI")
def step_run_actor_list_json(context: Any) -> None:
"""Invoke ``actor list --format json`` with the prepared registry."""
with patch(
_PATCH_GET_SERVICES,
return_value=(
context.tdd_actor_service,
context.tdd_actor_registry,
),
):
context.tdd_actor_result = runner.invoke(
actor_app, ["list", "--format", "json"]
)
# ---------------------------------------------------------------------------
# Then steps
# ---------------------------------------------------------------------------
@then("the tdd-actor-validation exit code should be {code:d}")
def step_exit_code(context: Any, code: int) -> None:
result = context.tdd_actor_result
assert result is not None, "actor list was not invoked"
actual = result.exit_code
assert actual == code, (
f"Expected exit code {code}, got {actual}. Output:\n{result.output}"
)
@then('the tdd-actor-validation output should not contain "{text}"')
def step_output_not_contains(context: Any, text: str) -> None:
result = context.tdd_actor_result
assert result is not None, "actor list was not invoked"
output = result.output
assert text.lower() not in output.lower(), (
f"Did NOT expect '{text}' in output but found it:\n{output}"
)
@then("the tdd-actor-validation upserted actor name should contain exactly one slash")
def step_upserted_name_single_slash(context: Any) -> None:
"""Verify that every name passed to ``upsert_actor`` has exactly one ``/``.
With the bug present, ``_actor_name()`` produces names like
``Openrouter/anthropic/claude-sonnet-4-20250514`` (2+ slashes), so this
assertion fails — which the ``@tdd_expected_fail`` tag inverts to a pass.
"""
service = context.tdd_actor_service
assert service.upsert_actor.call_count > 0, "upsert_actor was never called"
for call in service.upsert_actor.call_args_list:
name = call.kwargs.get("name") or call.args[0]
slash_count = name.count("/")
assert slash_count == 1, (
f"Expected exactly 1 slash in actor name '{name}', found {slash_count}"
)
@then("the tdd-actor-validation output should be valid JSON")
def step_output_valid_json(context: Any) -> None:
result = context.tdd_actor_result
assert result is not None, "actor list was not invoked"
try:
json.loads(result.output)
except json.JSONDecodeError as exc:
raise AssertionError(f"Output is not valid JSON:\n{result.output}") from exc
features/tdd_actor_list_validation.feature
+32
View File
@@ -0,0 +1,32 @@
@tdd_bug @tdd_bug_592
Feature: TDD Bug #592 — actor list validation rejects multi-slash model names
As a developer
I want to verify that `agents actor list` raises a validation error when
a provider has a model name containing `/` separators
So that the bug is captured and will be caught by a regression test
The root cause is that `ActorRegistry._actor_name()` builds actor names
via `f"{provider}/{model}"`. When a provider's default model already
contains `/` (e.g. `anthropic/claude-sonnet-4-20250514`), the resulting
name has 2+ slashes and `ActorService._normalize_name()` rejects it with
`ValidationError("Actor names must include exactly one '/' separator")`.
@tdd_expected_fail
Scenario: Actor list with multi-slash model does not raise validation error
Given an actor registry with a multi-slash model provider for tdd-actor-validation
When I run actor list via the tdd-actor-validation CLI
Then the tdd-actor-validation exit code should be 0
And the tdd-actor-validation output should not contain "VALIDATION_FAILED"
@tdd_expected_fail
Scenario: Built-in actor names have exactly one slash separator
Given an actor registry with a multi-slash model provider for tdd-actor-validation
When I run actor list via the tdd-actor-validation CLI
Then the tdd-actor-validation upserted actor name should contain exactly one slash
@tdd_expected_fail
Scenario: Actor list JSON format with multi-slash model succeeds
Given an actor registry with a multi-slash model provider for tdd-actor-validation
When I run actor list with format json via the tdd-actor-validation CLI
Then the tdd-actor-validation exit code should be 0
And the tdd-actor-validation output should be valid JSON
robot/helper_tdd_actor_list_validation.py
+169
View File
@@ -0,0 +1,169 @@
"""Helper script for tdd_actor_list_validation.robot smoke tests.
Each subcommand exercises the real ``ActorRegistry._actor_name()`` code path
with a provider whose default model contains ``/`` characters to reproduce
bug #592. The helper reports the **real** outcome: it exits 0 and prints
the sentinel when the operation succeeds (bug is fixed), and exits 1 when
the bug is still present. The ``tdd_expected_fail_listener`` on the Robot
side handles pass/fail inversion while the bug remains open.
"""
from __future__ import annotations
import sys
from collections.abc import Callable
from pathlib import Path
from typing import Any
from unittest.mock import patch
# Ensure local source tree is importable
_SRC = str(Path(__file__).resolve().parents[1] / "src")
if _SRC not in sys.path:
sys.path.insert(0, _SRC)
# Ensure the features directory is importable for shared mocks
_FEATURES = str(Path(__file__).resolve().parents[1] / "features")
if _FEATURES not in sys.path:
sys.path.insert(0, _FEATURES)
from mocks.fake_provider import FakeProviderInfo, make_registry # noqa: E402
from typer.testing import CliRunner # noqa: E402
from cleveragents.cli.commands.actor import app as actor_app # noqa: E402
runner = CliRunner()
_PATCH_GET_SERVICES = "cleveragents.cli.commands.actor._get_services"
_MULTI_SLASH_PROVIDERS = [
FakeProviderInfo(
name="Openrouter",
default_model="anthropic/claude-sonnet-4-20250514",
),
]
# ---------------------------------------------------------------------------
# Subcommands
# ---------------------------------------------------------------------------
def list_validation_error() -> None:
"""Invoke ``actor list`` with a multi-slash model provider.
Exits 0 with sentinel when the command succeeds with valid names (bug
fixed). Exits 1 when the command fails or produces bad names (bug
still present).
"""
mock_service, registry = make_registry(_MULTI_SLASH_PROVIDERS)
with patch(
_PATCH_GET_SERVICES,
return_value=(mock_service, registry),
):
result = runner.invoke(actor_app, ["list"])
assert mock_service.upsert_actor.call_count > 0, (
"upsert_actor was never called — patch may not have taken effect"
)
if result.exit_code != 0:
# Bug present — command failed.
print(
f"actor list failed with exit code {result.exit_code}",
file=sys.stderr,
)
sys.exit(1)
# Command succeeded — check if names are valid (single slash).
for call in mock_service.upsert_actor.call_args_list:
name: Any = call.kwargs.get("name", "")
if name.count("/") != 1:
print(
f"actor list produced bad name {name!r} with {name.count('/')} slashes",
file=sys.stderr,
)
sys.exit(1)
# Bug fixed — command succeeded with valid names.
print("tdd-actor-list-validation-error-ok")
def name_slash_check() -> None:
"""Verify that upserted actor names have exactly one ``/``.
Exits 0 with sentinel when all names have exactly one slash (bug
fixed). Exits 1 when any name has 0 or 2+ slashes (bug still
present).
"""
mock_service, registry = make_registry(_MULTI_SLASH_PROVIDERS)
with patch(
_PATCH_GET_SERVICES,
return_value=(mock_service, registry),
):
runner.invoke(actor_app, ["list"])
assert mock_service.upsert_actor.call_count > 0, (
"upsert_actor was never called — patch may not have taken effect"
)
for call in mock_service.upsert_actor.call_args_list:
name: Any = call.kwargs.get("name", "")
if name.count("/") != 1:
# Bug present — name construction produces bad names.
print(
f"actor name {name!r} has {name.count('/')} slashes",
file=sys.stderr,
)
sys.exit(1)
# Bug fixed — all names have exactly one slash.
print("tdd-actor-name-slash-check-ok")
def list_json_validation() -> None:
"""Invoke ``actor list --format json`` with a multi-slash model provider.
Exits 0 with sentinel when the command succeeds with valid names (bug
fixed). Exits 1 when the command fails or produces bad names (bug
still present).
"""
mock_service, registry = make_registry(_MULTI_SLASH_PROVIDERS)
with patch(
_PATCH_GET_SERVICES,
return_value=(mock_service, registry),
):
result = runner.invoke(actor_app, ["list", "--format", "json"])
assert mock_service.upsert_actor.call_count > 0, (
"upsert_actor was never called — patch may not have taken effect"
)
if result.exit_code != 0:
# Bug present — command failed.
print(
f"actor list --format json failed with exit code {result.exit_code}",
file=sys.stderr,
)
sys.exit(1)
# Command succeeded — check if names are valid (single slash).
for call in mock_service.upsert_actor.call_args_list:
name: Any = call.kwargs.get("name", "")
if name.count("/") != 1:
print(
f"actor list --format json produced bad name {name!r} with "
f"{name.count('/')} slashes",
file=sys.stderr,
)
sys.exit(1)
# Bug fixed — command succeeded with valid names.
print("tdd-actor-list-json-validation-ok")
# ---------------------------------------------------------------------------
# Dispatcher
# ---------------------------------------------------------------------------
_COMMANDS: dict[str, Callable[[], None]] = {
"list-validation-error": list_validation_error,
"name-slash-check": name_slash_check,
"list-json-validation": list_json_validation,
}
if __name__ == "__main__":
if len(sys.argv) < 2 or sys.argv[1] not in _COMMANDS:
print(f"Usage: {sys.argv[0]} <{'|'.join(_COMMANDS)}>", file=sys.stderr)
sys.exit(1)
cmd = _COMMANDS[sys.argv[1]]
cmd()
robot/tdd_actor_list_validation.robot
+43
View File
@@ -0,0 +1,43 @@
*** Settings ***
Documentation TDD Bug #592 — actor list validation rejects multi-slash model names
... Integration smoke tests verifying that the actor list command
... raises a validation error when a provider has a multi-slash
... default model name. These tests are tagged ``tdd_expected_fail``
... and are expected to fail until bug #592 is fixed.
Resource ${CURDIR}/common.resource
Suite Setup Setup Test Environment
Suite Teardown Cleanup Test Environment
*** Variables ***
${HELPER} ${CURDIR}/helper_tdd_actor_list_validation.py
*** Test Cases ***
TDD Actor List Validation Error With Multi-Slash Model
[Documentation] Verify that ``actor list`` triggers a validation error
... when a provider's default model contains ``/`` characters.
[Tags] tdd_bug tdd_bug_592 tdd_expected_fail
${result}= Run Process ${PYTHON} ${HELPER} list-validation-error cwd=${WORKSPACE}
Log ${result.stdout}
Log ${result.stderr}
Should Be Equal As Integers ${result.rc} 0
Should Contain ${result.stdout} tdd-actor-list-validation-error-ok
TDD Actor Name Slash Count Check
[Documentation] Verify that built-in actor names have exactly one ``/``
... separator after construction by ``_actor_name()``.
[Tags] tdd_bug tdd_bug_592 tdd_expected_fail
${result}= Run Process ${PYTHON} ${HELPER} name-slash-check cwd=${WORKSPACE}
Log ${result.stdout}
Log ${result.stderr}
Should Be Equal As Integers ${result.rc} 0
Should Contain ${result.stdout} tdd-actor-name-slash-check-ok
TDD Actor List JSON With Multi-Slash Model
[Documentation] Verify that ``actor list --format json`` triggers a
... validation error with a multi-slash model provider.
[Tags] tdd_bug tdd_bug_592 tdd_expected_fail
${result}= Run Process ${PYTHON} ${HELPER} list-json-validation cwd=${WORKSPACE}
Log ${result.stdout}
Log ${result.stderr}
Should Be Equal As Integers ${result.rc} 0
Should Contain ${result.stdout} tdd-actor-list-json-validation-ok