forked from HAL9000/cleveragents-core
01b6eb1804
## Summary Add M6 parallel-scaling coverage for 10+ concurrent subplans: - **15-subplan parallel scenario** with explicit peak-concurrency bound checks (`max_parallel=10`) and thread-safe concurrency tracking via `_build_executor()`. - **Deep hierarchical decomposition** coverage (4+ levels) with adjusted leaf condition that only stops early when hitting `max_depth` or when the workset is trivially small (`min_files_per_subplan`). - **Non-progress guard** in `_build_hierarchy` to prevent pathological recursion when clustering cannot meaningfully split the file set. - **Small-project regression test** (< 50 files) verifying decomposition depth does not increase unexpectedly with the relaxed leaf condition. - **ASV benchmark** for 15-subplan parallel execution with `max_parallel=10` to track scaling behavior. ### Removed from this PR The `_build_hierarchy` child-linkage correctness fix (returning `node_id` from recursive calls instead of using `nodes[-1].node_id`) has been **removed** per review feedback — it is a separate bug fix and will be submitted as an independent issue/PR per CONTRIBUTING.md §Atomic Commits. ## Approach - **Concurrency tracking:** The `_build_executor()` closure in step definitions detects `context.concurrency_counter` / `context.concurrency_lock` and performs thread-safe peak tracking in a try/finally block. - **Leaf condition:** Replaced the `max_files_per_subplan` / `max_tokens_per_subplan` leaf check with a `min_files_per_subplan` check to allow deeper decomposition for large projects. Added a non-progress guard so clustering that cannot split the file set terminates immediately rather than recursing to `max_depth`. - **Deterministic IDs:** `_ids_for_count()` preserves legacy fixed IDs for the first 5 subplans and generates additional deterministic IDs for scale scenarios. ## Validation ### Passing - `nox -s lint` — all checks passed - `nox -s typecheck` — 0 errors, 0 warnings - `nox -s unit_tests` — 12,988 scenarios passed, 0 failed - `nox -s coverage_report` — 97% (passes `--fail-under=97`) Closes #855 Reviewed-on: cleveragents/cleveragents-core#1201 Co-authored-by: Brent E. Edwards <brent.edwards@cleverthis.com> Co-committed-by: Brent E. Edwards <brent.edwards@cleverthis.com>
233 lines
8.0 KiB
Python
233 lines
8.0 KiB
Python
"""Helper script for tdd_plan_explain_plan_id.robot integration tests.
|
|
|
|
Each subcommand exercises the ``plan explain <plan_id>`` CLI path to reproduce
|
|
bug #968. The ``plan explain`` command currently declares its first positional
|
|
argument as ``decision_id``. When passed a plan ID, ``svc.get_decision(plan_id)``
|
|
raises ``DecisionNotFoundError`` (the plan ID is not a decision ID) and the
|
|
command exits with rc=1 and "Decision not found" error.
|
|
|
|
The helper exits 0 with a sentinel when the command succeeds (bug 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 os
|
|
import shutil
|
|
import subprocess
|
|
import sys
|
|
import tempfile
|
|
from collections.abc import Callable
|
|
from pathlib import Path
|
|
from typing import NoReturn
|
|
|
|
# Ensure local source tree AND robot/ directory are importable.
|
|
_ROOT: Path = Path(__file__).resolve().parents[1]
|
|
_SRC: str = str(_ROOT / "src")
|
|
_ROBOT: str = str(_ROOT / "robot")
|
|
for _p in (_SRC, _ROBOT):
|
|
if _p not in sys.path:
|
|
sys.path.insert(0, _p)
|
|
|
|
from ulid import ULID # noqa: E402
|
|
|
|
from cleveragents.application.container import ( # noqa: E402
|
|
get_container,
|
|
reset_container,
|
|
)
|
|
from cleveragents.config.settings import Settings # noqa: E402
|
|
from cleveragents.domain.models.core.decision import DecisionType # noqa: E402
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Helpers
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def _fail(msg: str) -> NoReturn:
|
|
"""Print failure message to stderr and exit with code 1."""
|
|
print(msg, file=sys.stderr)
|
|
sys.exit(1)
|
|
|
|
|
|
def _make_subprocess_env() -> dict[str, str]:
|
|
"""Build environment for subprocess calls with ``NO_COLOR=1``."""
|
|
env: dict[str, str] = os.environ.copy()
|
|
env["NO_COLOR"] = "1"
|
|
return env
|
|
|
|
|
|
def _setup_isolated_home() -> tuple[str, str | None]:
|
|
"""Create and activate an isolated CLEVERAGENTS_HOME for this run."""
|
|
tmp_home = tempfile.mkdtemp(prefix="tdd_plan_explain_968_")
|
|
previous_home = os.environ.get("CLEVERAGENTS_HOME")
|
|
os.environ["CLEVERAGENTS_HOME"] = tmp_home
|
|
reset_container()
|
|
Settings._instance = None
|
|
return tmp_home, previous_home
|
|
|
|
|
|
def _restore_isolated_home(tmp_home: str, previous_home: str | None) -> None:
|
|
"""Restore prior environment and clean up temporary home directory."""
|
|
if previous_home is not None:
|
|
os.environ["CLEVERAGENTS_HOME"] = previous_home
|
|
else:
|
|
os.environ.pop("CLEVERAGENTS_HOME", None)
|
|
reset_container()
|
|
Settings._instance = None
|
|
shutil.rmtree(tmp_home, ignore_errors=True)
|
|
|
|
|
|
def _setup_plan_with_decisions() -> str:
|
|
"""Create a plan_id and record decisions against it.
|
|
|
|
Uses the DecisionService directly with a synthetic plan ID.
|
|
The decision service does not require an actual Plan object to exist —
|
|
it records decisions keyed by plan_id string.
|
|
|
|
Returns the plan_id with recorded decisions.
|
|
"""
|
|
container = get_container()
|
|
decision_svc = container.decision_service()
|
|
|
|
plan_id: str = str(ULID())
|
|
|
|
# Record a root decision against this plan
|
|
decision_svc.record_decision(
|
|
plan_id=plan_id,
|
|
decision_type=DecisionType.PROMPT_DEFINITION,
|
|
question="What should we build?",
|
|
chosen_option="A REST API",
|
|
rationale="REST API is the most common pattern",
|
|
)
|
|
|
|
# Defensive check: verify that the decision was persisted before the
|
|
# subprocess reads it (distinguishes setup failures from the actual bug).
|
|
decisions = decision_svc.list_decisions(plan_id)
|
|
if not decisions:
|
|
_fail(
|
|
f"Setup failure: record_decision succeeded but list_decisions "
|
|
f"returned no decisions for plan_id={plan_id}. "
|
|
f"This is a test setup problem, not bug #968."
|
|
)
|
|
|
|
return plan_id
|
|
|
|
|
|
def _run_plan_explain(plan_id: str) -> subprocess.CompletedProcess[str]:
|
|
"""Run ``plan explain <plan_id>`` via subprocess.
|
|
|
|
Handles timeout with a descriptive error and sets ``NO_COLOR=1`` to
|
|
prevent ANSI escape codes in captured output.
|
|
"""
|
|
try:
|
|
return subprocess.run(
|
|
[
|
|
sys.executable,
|
|
"-m",
|
|
"cleveragents",
|
|
"plan",
|
|
"explain",
|
|
plan_id,
|
|
"--format",
|
|
"plain",
|
|
],
|
|
capture_output=True,
|
|
text=True,
|
|
timeout=45,
|
|
cwd=str(_ROOT),
|
|
env=_make_subprocess_env(),
|
|
)
|
|
except subprocess.TimeoutExpired:
|
|
_fail(
|
|
f"plan explain {plan_id} timed out after 45 seconds. "
|
|
f"Bug #968: subprocess exceeded inner timeout."
|
|
)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Subcommands
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def explain_with_plan_id() -> None:
|
|
"""Verify that ``plan explain <plan_id>`` succeeds (rc=0).
|
|
|
|
Bug #968: The command currently treats the argument as a decision_id,
|
|
calls ``svc.get_decision(plan_id)`` which raises DecisionNotFoundError,
|
|
and exits with rc=1. When the fix is applied, the command should fall
|
|
back to looking up decisions for the plan via ``list_decisions(plan_id)``
|
|
and explain the root decision.
|
|
"""
|
|
tmp_home, previous_home = _setup_isolated_home()
|
|
try:
|
|
plan_id: str = _setup_plan_with_decisions()
|
|
result: subprocess.CompletedProcess[str] = _run_plan_explain(plan_id)
|
|
|
|
if result.returncode != 0:
|
|
_fail(
|
|
f"plan explain {plan_id} exited with rc={result.returncode}. "
|
|
f"stdout: {result.stdout}\n"
|
|
f"stderr: {result.stderr}\n"
|
|
f"Bug #968: explain treats the argument as a decision_id, "
|
|
f"get_decision(plan_id) raises DecisionNotFoundError, command fails."
|
|
)
|
|
|
|
# Verify the output contains decision-related content — both keywords
|
|
# must be present (mirrors the AND-based assertion in the Behave test).
|
|
combined: str = result.stdout + result.stderr
|
|
if "decision" not in combined.lower() or "question" not in combined.lower():
|
|
_fail(
|
|
f"plan explain output does not contain decision details. "
|
|
f"stdout: {result.stdout}"
|
|
)
|
|
|
|
print("tdd-plan-explain-plan-id-ok")
|
|
finally:
|
|
_restore_isolated_home(tmp_home, previous_home)
|
|
|
|
|
|
def explain_plan_id_shows_question() -> None:
|
|
"""Verify that ``plan explain <plan_id>`` shows the root decision question.
|
|
|
|
Bug #968: Since the command fails with rc=1 before any output is
|
|
rendered, the root decision question is never displayed.
|
|
"""
|
|
tmp_home, previous_home = _setup_isolated_home()
|
|
try:
|
|
plan_id: str = _setup_plan_with_decisions()
|
|
result: subprocess.CompletedProcess[str] = _run_plan_explain(plan_id)
|
|
|
|
if result.returncode != 0:
|
|
_fail(
|
|
f"plan explain {plan_id} exited with rc={result.returncode}. "
|
|
f"Bug #968: command fails before rendering any output."
|
|
)
|
|
|
|
if "What should we build?" not in result.stdout:
|
|
_fail(f"Expected root decision question in output. stdout: {result.stdout}")
|
|
|
|
print("tdd-plan-explain-plan-id-question-ok")
|
|
finally:
|
|
_restore_isolated_home(tmp_home, previous_home)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Dispatcher
|
|
# ---------------------------------------------------------------------------
|
|
|
|
_COMMANDS: dict[str, Callable[[], None]] = {
|
|
"explain-with-plan-id": explain_with_plan_id,
|
|
"explain-plan-id-shows-question": explain_plan_id_shows_question,
|
|
}
|
|
|
|
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: Callable[[], None] = _COMMANDS[sys.argv[1]]
|
|
cmd()
|