Files
cleveragents-core/features/steps/decision_service_coverage_steps.py
khyari hamza 0e36755db9
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Successful in 14s
CI / build (pull_request) Successful in 14s
CI / quality (pull_request) Successful in 16s
CI / security (pull_request) Successful in 31s
CI / typecheck (pull_request) Successful in 43s
CI / unit_tests (pull_request) Successful in 2m18s
CI / integration_tests (pull_request) Successful in 2m53s
CI / docker (pull_request) Successful in 39s
CI / coverage (pull_request) Successful in 3m58s
CI / benchmark-regression (pull_request) Successful in 24m44s
fix(service): address review findings for decision service
- Rehydrate sequence counter from DB on restart (BUG-1)
- Add uniqueness guard raising SequenceConflictError (BUG-2)
- Fix delete_decision consistency between persisted/in-memory (BUG-3)
- Validate new_decision_id exists in mark_superseded (BUG-4)
- Include orphaned subtrees in get_tree output (BUG-5)
- Remove dead _decision_seq/_next_seq from plan_lifecycle (BUG-7)
- Export SequenceConflictError from services __init__ (SPEC-2)
- Replace list[Any] with list[ArtifactRef] typing (SPEC-4)
- Add actor_reasoning max_length validation (SEC-1)
- Use SELECT COUNT(*) in count_decisions (PERF-1)
- Replace MagicMock with create_autospec(Settings) (TEST-3)
- Eliminate UnitOfWork.__new__() anti-pattern (TEST-4)
- Use exact assertion counts in DI tests (TEST-5)
- Add restart rehydration, BFS order, invalid type, and
  confidence boundary test scenarios (TEST-1/2/6/7)
- Fix decision_service_coverage.feature to match actual API

ISSUES CLOSED: #172
2026-03-03 12:53:28 +00:00

503 lines
17 KiB
Python

"""Step definitions for decision_service_coverage.feature.
Exercises every method of ``DecisionService`` using mocked UnitOfWork
and repository objects so that decision_service.py achieves full
line-rate and branch-rate coverage.
All step names use the ``dsvc-`` prefix to avoid collisions with
existing step definitions in other feature files.
"""
from __future__ import annotations
from contextlib import contextmanager
from typing import Any
from unittest.mock import MagicMock
from behave import given, then, when
from behave.runner import Context
from ulid import ULID
from cleveragents.application.services.decision_service import (
DecisionNotFoundError,
DecisionService,
)
from cleveragents.domain.models.core.decision import (
Decision,
DecisionType,
)
# ---------------------------------------------------------------------------
# Helpers
# ---------------------------------------------------------------------------
_PLAN_ID = str(ULID())
_DECISION_ID = str(ULID())
_NEW_DECISION_ID = str(ULID())
_LEAF_ID = str(ULID())
_ROOT_ID = str(ULID())
def _make_decision(
decision_id: str | None = None,
plan_id: str = _PLAN_ID,
parent_decision_id: str | None = None,
sequence_number: int = 0,
decision_type: DecisionType = DecisionType.PROMPT_DEFINITION,
superseded_by: str | None = None,
) -> Decision:
"""Create a minimal valid Decision for testing."""
kwargs: dict[str, Any] = {
"plan_id": plan_id,
"sequence_number": sequence_number,
"decision_type": decision_type,
"question": "Which approach?",
"chosen_option": "REST API",
}
if decision_id is not None:
kwargs["decision_id"] = decision_id
if parent_decision_id is not None:
kwargs["parent_decision_id"] = parent_decision_id
if superseded_by is not None:
kwargs["superseded_by"] = superseded_by
return Decision(**kwargs)
def _build_mock_uow() -> tuple[MagicMock, MagicMock, MagicMock]:
"""Build a mock UnitOfWork with a mock transaction context manager.
Returns:
(mock_uow, mock_ctx, mock_decisions_repo)
"""
mock_decisions = MagicMock(name="decisions_repo")
mock_ctx = MagicMock(name="uow_context")
mock_ctx.decisions = mock_decisions
mock_uow = MagicMock(name="unit_of_work")
@contextmanager
def _fake_transaction():
yield mock_ctx
mock_uow.transaction = _fake_transaction
return mock_uow, mock_ctx, mock_decisions
def _build_service(mock_uow: MagicMock) -> tuple[DecisionService, MagicMock]:
"""Construct a DecisionService with mock settings and the given UoW.
Returns:
(service, mock_settings)
"""
mock_settings = MagicMock(name="settings")
service = DecisionService(settings=mock_settings, unit_of_work=mock_uow)
return service, mock_settings
# ---------------------------------------------------------------------------
# Constructor
# ---------------------------------------------------------------------------
@given("dsvc- a mock settings object and a mock UnitOfWork")
def step_dsvc_mock_deps(context: Context) -> None:
context.dsvc_settings = MagicMock(name="settings")
context.dsvc_uow = MagicMock(name="unit_of_work")
@when("dsvc- I construct a DecisionService with those dependencies")
def step_dsvc_construct(context: Context) -> None:
context.dsvc_service = DecisionService(
settings=context.dsvc_settings,
unit_of_work=context.dsvc_uow,
)
@then("dsvc- the service should store the settings")
def step_dsvc_check_settings(context: Context) -> None:
assert context.dsvc_service.settings is context.dsvc_settings, (
"Service did not store settings"
)
@then("dsvc- the service should store the unit_of_work")
def step_dsvc_check_uow(context: Context) -> None:
assert context.dsvc_service.unit_of_work is context.dsvc_uow, (
"Service did not store unit_of_work"
)
@then("dsvc- the service should have a bound structlog logger")
def step_dsvc_check_logger(context: Context) -> None:
assert hasattr(context.dsvc_service, "_logger"), "Service missing _logger attribute"
# ---------------------------------------------------------------------------
# Shared Given: service + mock UoW
# ---------------------------------------------------------------------------
@given("dsvc- a DecisionService with a mocked UnitOfWork")
def step_dsvc_service_with_mock_uow(context: Context) -> None:
mock_uow, mock_ctx, mock_decisions = _build_mock_uow()
service, _ = _build_service(mock_uow)
# Attach a mock logger so non-logger scenarios can still verify calls
mock_logger = MagicMock(name="structlog_logger")
mock_logger.bind = MagicMock(return_value=mock_logger)
service._logger = mock_logger
context.dsvc_service = service
context.dsvc_uow = mock_uow
context.dsvc_ctx = mock_ctx
context.dsvc_decisions = mock_decisions
context.dsvc_logger = mock_logger
@given("dsvc- a DecisionService with a mocked UnitOfWork and captured logger")
def step_dsvc_service_with_logger(context: Context) -> None:
mock_uow, mock_ctx, mock_decisions = _build_mock_uow()
service, _ = _build_service(mock_uow)
# Replace the _logger with a MagicMock so we can assert calls
mock_logger = MagicMock(name="structlog_logger")
mock_logger.bind = MagicMock(return_value=mock_logger)
service._logger = mock_logger
context.dsvc_service = service
context.dsvc_uow = mock_uow
context.dsvc_ctx = mock_ctx
context.dsvc_decisions = mock_decisions
context.dsvc_logger = mock_logger
# ---------------------------------------------------------------------------
# record_decision
# ---------------------------------------------------------------------------
@when("dsvc- I call record_decision with plan_id and required args")
def step_dsvc_call_record(context: Context) -> None:
context.dsvc_result = context.dsvc_service.record_decision(
plan_id=_PLAN_ID,
decision_type=DecisionType.PROMPT_DEFINITION,
question="Which approach?",
chosen_option="REST API",
)
@then("dsvc- the UoW transaction should have been entered")
def step_dsvc_txn_entered(context: Context) -> None:
assert context.dsvc_decisions.create.called, (
"Transaction context was not entered — create was never called"
)
@then("dsvc- ctx.decisions.create should have been called once")
def step_dsvc_create_called(context: Context) -> None:
assert context.dsvc_decisions.create.call_count == 1, (
f"Expected create to be called once, got {context.dsvc_decisions.create.call_count}"
)
@then("dsvc- the returned decision should have the correct plan_id")
def step_dsvc_record_returns_correct_plan(context: Context) -> None:
assert context.dsvc_result.plan_id == _PLAN_ID, (
f"Expected plan_id '{_PLAN_ID}', got '{context.dsvc_result.plan_id}'"
)
# ---------------------------------------------------------------------------
# record_decision logging
# ---------------------------------------------------------------------------
@then('dsvc- the logger should have recorded an info call with "{event}"')
def step_dsvc_logger_info(context: Context, event: str) -> None:
info_calls = context.dsvc_logger.info.call_args_list
events = [c.args[0] if c.args else c.kwargs.get("event", "") for c in info_calls]
assert event in events, f"Expected info event '{event}' in {events}"
# ---------------------------------------------------------------------------
# get_decision
# ---------------------------------------------------------------------------
@given("dsvc- the mock repo get method returns a Decision")
def step_dsvc_mock_get_returns(context: Context) -> None:
context.dsvc_expected_decision = _make_decision(
decision_id=_DECISION_ID,
plan_id=_PLAN_ID,
)
context.dsvc_decisions.get.return_value = context.dsvc_expected_decision
@given("dsvc- the mock repo get method returns None")
def step_dsvc_mock_get_returns_none(context: Context) -> None:
context.dsvc_decisions.get.return_value = None
@when("dsvc- I call get_decision with a known ID")
def step_dsvc_call_get(context: Context) -> None:
context.dsvc_result = context.dsvc_service.get_decision(_DECISION_ID)
@when("dsvc- I call get_decision with an unknown ID expecting not-found")
def step_dsvc_call_get_unknown(context: Context) -> None:
try:
context.dsvc_service.get_decision("NONEXISTENT_ID_12345678")
context.dsvc_error = None
except DecisionNotFoundError as exc:
context.dsvc_error = exc
@then("dsvc- ctx.decisions.get should have been called with the ID")
def step_dsvc_get_called(context: Context) -> None:
context.dsvc_decisions.get.assert_called_once_with(_DECISION_ID)
@then("dsvc- the returned value should be the expected Decision")
def step_dsvc_get_returns_expected(context: Context) -> None:
assert context.dsvc_result is context.dsvc_expected_decision, (
"get_decision did not return the expected Decision"
)
@then("dsvc- a DecisionNotFoundError should have been raised")
def step_dsvc_not_found_raised(context: Context) -> None:
assert context.dsvc_error is not None, "Expected DecisionNotFoundError but got None"
assert isinstance(context.dsvc_error, DecisionNotFoundError)
# ---------------------------------------------------------------------------
# list_decisions
# ---------------------------------------------------------------------------
@given("dsvc- the mock repo get_by_plan method returns {count:d} decisions")
def step_dsvc_mock_get_by_plan(context: Context, count: int) -> None:
decisions = [
_make_decision(
plan_id=_PLAN_ID,
sequence_number=i,
decision_type=(
DecisionType.PROMPT_DEFINITION
if i == 0
else DecisionType.STRATEGY_CHOICE
),
parent_decision_id=None if i == 0 else str(ULID()),
)
for i in range(count)
]
context.dsvc_decisions.get_by_plan.return_value = decisions
context.dsvc_expected_count = count
@when("dsvc- I call list_decisions with a plan ID")
def step_dsvc_call_list_decisions(context: Context) -> None:
context.dsvc_result = context.dsvc_service.list_decisions(_PLAN_ID)
@then("dsvc- ctx.decisions.get_by_plan should have been called with the plan ID")
def step_dsvc_get_by_plan_called(context: Context) -> None:
context.dsvc_decisions.get_by_plan.assert_called_once_with(_PLAN_ID)
@then("dsvc- the returned list should have {count:d} decisions")
def step_dsvc_list_count(context: Context, count: int) -> None:
assert len(context.dsvc_result) == count, (
f"Expected {count} decisions, got {len(context.dsvc_result)}"
)
# ---------------------------------------------------------------------------
# get_tree
# ---------------------------------------------------------------------------
_CHILD1_ID = str(ULID())
_CHILD2_ID = str(ULID())
_GRANDCHILD_ID = str(ULID())
@given("dsvc- the mock repo get_by_plan method returns 4 decisions as a tree")
def step_dsvc_mock_get_tree(context: Context) -> None:
root = _make_decision(
decision_id=_ROOT_ID,
plan_id=_PLAN_ID,
sequence_number=0,
)
child1 = _make_decision(
decision_id=_CHILD1_ID,
plan_id=_PLAN_ID,
sequence_number=1,
decision_type=DecisionType.STRATEGY_CHOICE,
parent_decision_id=_ROOT_ID,
)
child2 = _make_decision(
decision_id=_CHILD2_ID,
plan_id=_PLAN_ID,
sequence_number=2,
decision_type=DecisionType.STRATEGY_CHOICE,
parent_decision_id=_ROOT_ID,
)
grandchild = _make_decision(
decision_id=_GRANDCHILD_ID,
plan_id=_PLAN_ID,
sequence_number=3,
decision_type=DecisionType.IMPLEMENTATION_CHOICE,
parent_decision_id=_CHILD1_ID,
)
context.dsvc_decisions.get_by_plan.return_value = [root, child1, child2, grandchild]
@when("dsvc- I call get_tree with a plan ID")
def step_dsvc_call_get_tree(context: Context) -> None:
context.dsvc_result = context.dsvc_service.get_tree(_PLAN_ID)
@then("dsvc- the returned tree list should have {count:d} decisions")
def step_dsvc_tree_count(context: Context, count: int) -> None:
assert len(context.dsvc_result) == count, (
f"Expected {count} tree nodes, got {len(context.dsvc_result)}"
)
# ---------------------------------------------------------------------------
# get_path_to_root
# ---------------------------------------------------------------------------
@given("dsvc- the mock repo get_path_to_root method returns {count:d} decisions")
def step_dsvc_mock_get_path(context: Context, count: int) -> None:
decisions = [
_make_decision(
plan_id=_PLAN_ID,
sequence_number=count - 1 - i,
decision_type=(
DecisionType.PROMPT_DEFINITION
if i == count - 1
else DecisionType.STRATEGY_CHOICE
),
parent_decision_id=None if i == count - 1 else str(ULID()),
)
for i in range(count)
]
context.dsvc_decisions.get_path_to_root.return_value = decisions
context.dsvc_expected_path_count = count
@when("dsvc- I call get_path_to_root with a leaf ID")
def step_dsvc_call_get_path(context: Context) -> None:
context.dsvc_result = context.dsvc_service.get_path_to_root(_LEAF_ID)
@then("dsvc- ctx.decisions.get_path_to_root should have been called with the leaf ID")
def step_dsvc_get_path_called(context: Context) -> None:
context.dsvc_decisions.get_path_to_root.assert_called_once_with(_LEAF_ID)
@then("dsvc- the returned path list should have {count:d} decisions")
def step_dsvc_path_count(context: Context, count: int) -> None:
assert len(context.dsvc_result) == count, (
f"Expected {count} path nodes, got {len(context.dsvc_result)}"
)
# ---------------------------------------------------------------------------
# mark_superseded
# ---------------------------------------------------------------------------
@given("dsvc- the mock repo has the replacement decision in cache")
def step_dsvc_seed_replacement_in_cache(context: Context) -> None:
"""Seed the replacement decision into the service's in-memory cache.
``mark_superseded`` now validates that ``new_decision_id`` exists
before proceeding. We add the replacement to ``_decisions`` so
the validation lookup succeeds.
"""
replacement = _make_decision(
decision_id=_NEW_DECISION_ID,
plan_id=_PLAN_ID,
sequence_number=99,
decision_type=DecisionType.STRATEGY_CHOICE,
)
context.dsvc_service._decisions[_NEW_DECISION_ID] = replacement
@given("dsvc- the mock repo update_superseded_by method returns a superseded Decision")
def step_dsvc_mock_superseded(context: Context) -> None:
context.dsvc_superseded_decision = _make_decision(
decision_id=_DECISION_ID,
plan_id=_PLAN_ID,
superseded_by=_NEW_DECISION_ID,
)
context.dsvc_decisions.update_superseded_by.return_value = (
context.dsvc_superseded_decision
)
@when("dsvc- I call mark_superseded with old and new IDs")
def step_dsvc_call_mark_superseded(context: Context) -> None:
context.dsvc_result = context.dsvc_service.mark_superseded(
_DECISION_ID,
_NEW_DECISION_ID,
)
@then("dsvc- ctx.decisions.update_superseded_by should have been called with both IDs")
def step_dsvc_update_superseded_called(context: Context) -> None:
context.dsvc_decisions.update_superseded_by.assert_called_once_with(
_DECISION_ID,
_NEW_DECISION_ID,
)
@then("dsvc- the returned decision should be the superseded one")
def step_dsvc_superseded_returned(context: Context) -> None:
assert context.dsvc_result is context.dsvc_superseded_decision, (
"mark_superseded did not return the expected superseded Decision"
)
# ---------------------------------------------------------------------------
# list_by_type
# ---------------------------------------------------------------------------
@given("dsvc- the mock repo list_by_type method returns {count:d} decisions")
def step_dsvc_mock_list_by_type(context: Context, count: int) -> None:
decisions = [
_make_decision(
plan_id=_PLAN_ID,
sequence_number=i,
decision_type=DecisionType.STRATEGY_CHOICE,
parent_decision_id=str(ULID()),
)
for i in range(count)
]
context.dsvc_decisions.list_by_type.return_value = decisions
context.dsvc_expected_type_count = count
@when('dsvc- I call list_by_type with plan ID and type "{dtype}"')
def step_dsvc_call_list_by_type(context: Context, dtype: str) -> None:
context.dsvc_list_type_arg = dtype
context.dsvc_result = context.dsvc_service.list_by_type(_PLAN_ID, dtype)
@then("dsvc- ctx.decisions.list_by_type should have been called with plan ID and type")
def step_dsvc_list_by_type_called(context: Context) -> None:
context.dsvc_decisions.list_by_type.assert_called_once_with(
_PLAN_ID,
context.dsvc_list_type_arg,
)
@then("dsvc- the returned type list should have {count:d} decisions")
def step_dsvc_type_list_count(context: Context, count: int) -> None:
assert len(context.dsvc_result) == count, (
f"Expected {count} decisions by type, got {len(context.dsvc_result)}"
)