Files
cleveragents-core/features/steps/tdd_session_shared_steps.py
T
CoreRasurae 7f29874156
CI / push-validation (pull_request) Successful in 23s
CI / helm (pull_request) Successful in 37s
CI / build (pull_request) Successful in 3m48s
CI / lint (pull_request) Successful in 3m55s
CI / quality (pull_request) Successful in 4m18s
CI / typecheck (pull_request) Successful in 4m32s
CI / security (pull_request) Successful in 4m35s
CI / integration_tests (pull_request) Successful in 6m47s
CI / e2e_tests (pull_request) Successful in 6m56s
CI / unit_tests (pull_request) Successful in 7m23s
CI / docker (pull_request) Successful in 1m30s
CI / coverage (pull_request) Successful in 14m31s
CI / benchmark-regression (push) Waiting to run
CI / benchmark-publish (push) Waiting to run
CI / status-check (pull_request) Successful in 3s
CI / helm (push) Successful in 27s
CI / push-validation (push) Successful in 26s
CI / build (push) Successful in 3m43s
CI / lint (push) Successful in 3m54s
CI / quality (push) Successful in 4m16s
CI / typecheck (push) Successful in 4m28s
CI / security (push) Successful in 4m43s
CI / integration_tests (push) Successful in 7m3s
CI / e2e_tests (push) Successful in 7m13s
CI / unit_tests (push) Successful in 7m25s
CI / docker (push) Successful in 1m36s
CI / coverage (push) Successful in 13m40s
CI / status-check (push) Successful in 3s
CI / benchmark-publish (pull_request) Has been skipped
CI / benchmark-regression (pull_request) Failing after 14m31s
fix: address code review findings from PR #1175 (issue #10267)
Implement comprehensive fixes for all P2:should-fix and P3:nit findings
from the Strategy Actor code review:

Update 5 step functions that were calling the private _execute_with_llm() method
directly to instead use the new strategy_tree field from StrategizeResult:

 - step_execute_and_inspect_tree (line 619)
 - step_parse_self_dep (line 877)
 - step_parse_duplicate_step_numbers (line 968)
 - step_parse_non_sequential_steps (line 1075)
 - step_parse_non_sequential_steps_and_inspect (line 1755)
 - step_build_decisions_from_llm_tree (line 1299) - also added execute() call

This eliminates the fragile double-execution pattern that produced divergent ULIDs
and couples tests to private implementation details. Tests now use the public
StrategizeResult.strategy_tree field.

P2:should-fix Fixes:
 - Move json import to module level in plan_executor_coverage_steps.py
 - Remove redundant Exception clause in plan_executor.py exception handler
 - Add logging to silent config service fallback in plan.py
 - Improve structured content block handling in _extract_content() to properly
   extract text from LangChain MessageContent dicts
 - Remove duplicated _DEFAULT_ACTOR_NAME constant and import from canonical
   source (strategy_resolution.py)
 - Add strategy_tree field to StrategizeResult to expose tree for test inspection
   without coupling to private _execute_with_llm() method

P3:nit Fixes:
 - Improved docstrings and code clarity

Refs: #10267
2026-04-21 15:36:09 +01:00

153 lines
5.4 KiB
Python

"""Shared step definitions for TDD session DI bug tests.
Steps in this module are used by both ``tdd_session_create_di.feature`` and
``tdd_session_list_di.feature``. Behave loads all step files globally, so
placing shared steps here avoids duplication and satisfies the CONTRIBUTING.md
rule that feature-specific steps live in matching ``*_steps.py`` files while
shared steps live in clearly named reusable modules.
"""
from __future__ import annotations
import json
import logging
import os
import tempfile
from pathlib import Path
from behave import given, then
from behave.runner import Context
from typer.testing import CliRunner
from cleveragents.application.container import reset_container
from cleveragents.cli.commands import session as session_mod
from cleveragents.config.settings import Settings
def _suppress_structlog_stdout() -> tuple[int, bool]:
"""Prevent structlog debug lines from contaminating CLI stdout.
When ``behave-parallel`` runs multiple features in one process,
another feature may (re)configure structlog with a ``ConsoleRenderer``
or leave it at the default ``PrintLogger`` — both write to
``sys.stdout``. The ``@database_retry`` decorator logs a *debug*
event on every attempt, and ``CliRunner.invoke()`` captures
``sys.stdout``, so those debug lines end up in the captured output,
breaking JSON/YAML assertions.
Returns the previous root-logger level and ``cache_logger_on_first_use``
flag so that the caller's cleanup function can restore them.
"""
import structlog
root = logging.getLogger()
prev_level = root.level
root.setLevel(logging.CRITICAL)
# Force structlog to route through stdlib logging (not PrintLogger)
# and disable logger caching so the level change takes effect for
# loggers that were already instantiated.
prev_config = structlog.get_config()
prev_cache: bool = prev_config.get("cache_logger_on_first_use", True) # type: ignore[assignment]
structlog.configure(
logger_factory=structlog.stdlib.LoggerFactory(),
wrapper_class=structlog.stdlib.BoundLogger,
cache_logger_on_first_use=False,
)
return prev_level, prev_cache
def _restore_structlog(prev_level: int, prev_cache: bool) -> None:
"""Undo the suppression applied by :func:`_suppress_structlog_stdout`."""
import structlog
logging.getLogger().setLevel(prev_level)
structlog.configure(cache_logger_on_first_use=prev_cache)
@given("a CLI runner using the real session DI path")
def step_real_di_runner(context: Context) -> None:
"""Set up a CLI runner that does NOT mock the session service.
By ensuring ``session_mod._service`` is ``None``, the CLI will call
``_get_session_service()`` which hits the real DI container and
resolves the ``session_service`` provider.
"""
context.runner = CliRunner()
# Ensure we go through the real DI path — no mock service.
session_mod._service = None
# Reset singletons before setup so stale state from a parallel
# scenario does not leak in.
reset_container()
Settings._instance = None # type: ignore[attr-defined]
# Suppress structlog debug output so CliRunner captures clean output.
prev_level, prev_cache = _suppress_structlog_stdout()
# Provide a database URL so the container can be constructed.
fd, db_path = tempfile.mkstemp(suffix=".db")
os.close(fd)
context._tdd_db_path = db_path
db_url = f"sqlite:///{db_path}"
os.environ["CLEVERAGENTS_DATABASE_URL"] = db_url
# Create the schema so the database file is ready for use.
from sqlalchemy import create_engine
from cleveragents.infrastructure.database.models import Base
engine = create_engine(db_url, echo=False)
try:
Base.metadata.create_all(engine)
finally:
engine.dispose()
def _cleanup() -> None:
session_mod._service = None
os.environ.pop("CLEVERAGENTS_DATABASE_URL", None)
reset_container()
# Reset the Settings singleton so stale database URLs from this
# scenario do not leak into subsequent scenarios.
Settings._instance = None # type: ignore[attr-defined]
Path(db_path).unlink(missing_ok=True)
_restore_structlog(prev_level, prev_cache)
context.add_cleanup(_cleanup)
@then("the session {subcommand} command should exit successfully")
def step_session_exits_ok(context: Context, subcommand: str) -> None:
"""Assert the command exits with code 0."""
assert context.result.exit_code == 0, (
f"Expected exit code 0, got {context.result.exit_code}.\n"
f"Output:\n{context.result.output}"
)
@then("the session {subcommand} output should be valid JSON")
def step_session_output_json(context: Context, subcommand: str) -> None:
"""Assert the output is parseable JSON."""
output = context.result.output
# Strip ANSI color codes and MCP health check warning that may prepend to JSON
output = output.lstrip()
lines = output.split("\n")
# Skip leading lines that are not JSON (like MCP warnings)
json_start_idx = 0
for i, line in enumerate(lines):
stripped = line.strip()
if stripped.startswith("{") or stripped.startswith("["):
json_start_idx = i
break
output = "\n".join(lines[json_start_idx:])
try:
json.loads(output)
except json.JSONDecodeError as exc:
raise AssertionError(
f"Output is not valid JSON:\n{context.result.output}"
) from exc