-
Notifications
You must be signed in to change notification settings - Fork 5
feat: e2e CLI validation infrastructure + ReAct engine bug fix #371
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
f261f0d
feat: e2e CLI validation infrastructure + ReAct engine bug fix (#353)
dc266e1
fix: remove cache_dir from pytest.ini to avoid strict-config error
c034ca3
fix: lint errors in e2e test code
94a0656
fix: address PR review feedback
8075a37
fix: address second round of PR review feedback
81e7542
fix: address remaining PR review feedback
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,185 @@ | ||
| # Phase 2.5-F: End-to-End CLI Validation Report | ||
|
|
||
| **Date**: 2026-02-10 | ||
| **Issue**: #353 | ||
| **Engine**: ReAct (`react_agent.py`) | ||
| **Target project**: `~/projects/cf-test` (Task Tracker CLI) | ||
|
|
||
| --- | ||
|
|
||
| ## Summary | ||
|
|
||
| The ReAct engine was validated by running the full Golden Path workflow against the `cf-test` project. The workflow pipeline (init, PRD, task generation, marking ready) works correctly. However, the ReAct agent achieved **0% task completion** — all 10 generated tasks failed after exhausting the 30-iteration limit. | ||
|
|
||
| A critical bug was found and fixed during validation: the `_react_loop` method started with an empty messages list, causing all real API calls to fail with `BadRequestError`. | ||
|
|
||
| --- | ||
|
|
||
| ## Test Infrastructure | ||
|
|
||
| Reusable e2e test infrastructure was created in `tests/e2e/cli/`: | ||
|
|
||
| | File | Purpose | | ||
| |------|---------| | ||
| | `conftest.py` | Fixtures, markers, API key loading | | ||
| | `golden_path_runner.py` | Reusable `GoldenPathRunner` class | | ||
| | `validators.py` | Validation functions for success criteria | | ||
| | `test_react_engine_validation.py` | ReAct engine validation tests | | ||
| | `test_engine_comparison.py` | Side-by-side engine comparison | | ||
|
|
||
| **Running the tests:** | ||
| ```bash | ||
| # Run ReAct validation (requires ANTHROPIC_API_KEY, ~30 min) | ||
| uv run pytest tests/e2e/cli/test_react_engine_validation.py -v -s | ||
|
|
||
| # Run engine comparison | ||
| uv run pytest tests/e2e/cli/test_engine_comparison.py -v -s | ||
|
|
||
| # Run all e2e LLM tests | ||
| uv run pytest -m e2e_llm -v -s | ||
| ``` | ||
|
|
||
| --- | ||
|
|
||
| ## Bug Fix: Empty Messages in `_react_loop` | ||
|
|
||
| **File**: `codeframe/core/react_agent.py` | ||
| **Root cause**: `_react_loop()` initialized `messages: list[dict] = []` then called the Anthropic API, which requires at least one user message. | ||
|
|
||
| **Impact**: Every real API call raised `anthropic.BadRequestError: 'messages: at least one message is required'`. This was masked in unit tests because `MockLLMProvider` doesn't enforce this constraint. | ||
|
|
||
| **Fix**: Added an initial user message that instructs the agent to begin implementation: | ||
| ```python | ||
| messages: list[dict] = [ | ||
| { | ||
| "role": "user", | ||
| "content": ( | ||
| "Implement the task described in the system prompt. " | ||
| "Start by reading relevant files to understand the current " | ||
| "codebase, then make the necessary changes. " | ||
| "When you are done, respond with a brief summary." | ||
| ), | ||
| } | ||
| ] | ||
| ``` | ||
|
|
||
| **Regression check**: All 1316 core tests + 23 adapter tests pass after the fix. | ||
|
|
||
| --- | ||
|
|
||
| ## Validation Results | ||
|
|
||
| ### Workflow Pipeline | ||
|
|
||
| | Step | Status | Duration | | ||
| |------|--------|----------| | ||
| | `cf init --detect` | PASS | ~1s | | ||
| | `cf prd add requirements.md` | PASS | <1s | | ||
| | `cf tasks generate` | PASS | ~15s | | ||
| | Mark all tasks READY | PASS | <1s | | ||
| | Task execution (10 tasks) | **0/10 PASS** | ~1573s total | | ||
|
|
||
| ### Per-Task Breakdown | ||
|
|
||
| All 10 tasks hit the 30-iteration maximum and were marked FAILED. The agent did generate substantial code but could not get verification gates to pass within the iteration budget. | ||
|
|
||
| | # | Task | Iterations | Duration | Result | | ||
| |---|------|-----------|----------|--------| | ||
| | 1 | Data models (Task, Priority, Status) | 30 | ~160s | FAILED | | ||
| | 2 | Storage layer (JSON persistence) | 30 | ~160s | FAILED | | ||
| | 3 | CLI entry point (Click) | 30 | ~160s | FAILED | | ||
| | 4 | Add task command | 30 | ~160s | FAILED | | ||
| | 5 | List tasks with filtering | 30 | ~160s | FAILED | | ||
| | 6 | Update task command | 30 | ~160s | FAILED | | ||
| | 7 | Delete task command | 30 | ~160s | FAILED | | ||
| | 8 | Status management | 30 | ~160s | FAILED | | ||
| | 9 | Input validation & error handling | 30 | ~160s | FAILED | | ||
| | 10 | Test suite | 30 | ~160s | FAILED | | ||
|
|
||
| ### Success Criteria Assessment | ||
|
|
||
| | Criterion | Pass? | Notes | | ||
| |-----------|-------|-------| | ||
| | Build working CLI on first attempt | NO | All tasks failed | | ||
| | 0 ruff lint errors | YES | `ruff check` reports 0 errors on generated code | | ||
| | pyproject.toml preserved | YES | Hash unchanged | | ||
| | No cross-file naming mismatches | YES | No import errors at package level | | ||
| | Each task within 30 iterations | YES | All tasks hit exactly 30 (the limit) | | ||
| | Generated tests pass | NO | `ModuleNotFoundError: No module named 'task_tracker'` | | ||
|
|
||
| ### Generated Artifacts | ||
|
|
||
| The agent did produce code in `cf-test`: | ||
|
|
||
| **Source files** (`src/task_tracker/`): | ||
| - `cli.py` (27KB) - Click-based CLI with all commands | ||
| - `models.py` - Pydantic data models | ||
| - `schema.py` - JSON schema definitions | ||
| - `storage.py` - JSON file persistence layer | ||
|
|
||
| **Test files** (`tests/`): | ||
| - `test_cli.py`, `test_models.py`, `test_schema.py` | ||
| - `test_status_management.py`, `test_storage.py` | ||
|
|
||
| Tests fail because the `task_tracker` package is not installed in the venv (`pip install -e .` was never run). | ||
|
|
||
| --- | ||
|
|
||
| ## Failure Analysis | ||
|
|
||
| ### Primary Failure Mode | ||
|
|
||
| The ReAct agent enters a **verification gate loop**: it generates code, the verification gate (pytest/ruff) fails, it tries to fix the failure, the fix introduces a new failure, and the cycle continues until the 30-iteration limit is reached. | ||
|
|
||
| ### Contributing Factors | ||
|
|
||
| 1. **Package not installed**: The generated tests import `task_tracker` but the package is never installed in the venv. The agent doesn't run `pip install -e .` as part of its workflow. | ||
|
|
||
| 2. **Accumulated complexity**: Each task builds on the previous, but the agent starts each task fresh without understanding prior artifacts. By task 3-4, the generated code needs to be consistent with earlier tasks' output. | ||
|
|
||
| 3. **No task dependency awareness**: Tasks execute independently. The agent doesn't know task 1 already created `models.py` when working on task 2. | ||
|
|
||
| 4. **Gate strictness vs. iteration budget**: 30 iterations is not enough to converge when each fix attempt can introduce new failures, especially with pytest running the full test suite. | ||
|
|
||
| ### Recommendations for ReAct Engine Improvements | ||
|
|
||
| 1. **Package installation step**: Add `pip install -e .` (or equivalent) as a standard setup step before running pytest gates. | ||
| 2. **Cross-task context**: Carry over file inventory from previous tasks so the agent knows what already exists. | ||
| 3. **Incremental gate scope**: Run only tests related to the current task, not the entire suite. | ||
| 4. **Iteration budget tuning**: Consider adaptive budgets based on task complexity. | ||
|
|
||
| --- | ||
|
|
||
| ## pytest Results | ||
|
|
||
| ```text | ||
| 17 collected tests: | ||
| 15 passed (workflow steps, ruff lint, pyproject preserved, metrics) | ||
| 2 failed: | ||
| - test_all_tasks_succeed (0% completion rate) | ||
| - test_tests_pass (ModuleNotFoundError in generated tests) | ||
| ``` | ||
|
|
||
| --- | ||
|
|
||
| ## Comparison with Plan-and-Execute Engine | ||
|
|
||
| The Plan-and-Execute engine comparison was **skipped** for this validation run. With a 0% success rate on the ReAct engine, the comparison would not yield meaningful insights until the verification gate loop issue is addressed. | ||
|
|
||
| This can be revisited as a follow-up once the ReAct engine's task completion rate improves. | ||
|
|
||
| --- | ||
|
|
||
| ## Files Changed in This PR | ||
|
|
||
| | File | Change | | ||
| |------|--------| | ||
| | `codeframe/core/react_agent.py` | Bug fix: added initial user message to `_react_loop` | | ||
| | `pytest.ini` | Added `e2e_llm` marker registration | | ||
| | `tests/e2e/cli/__init__.py` | New: package init | | ||
| | `tests/e2e/cli/conftest.py` | New: fixtures, markers, API key loading | | ||
| | `tests/e2e/cli/golden_path_runner.py` | New: reusable Golden Path workflow runner | | ||
| | `tests/e2e/cli/validators.py` | New: validation functions | | ||
| | `tests/e2e/cli/test_react_engine_validation.py` | New: ReAct validation tests | | ||
| | `tests/e2e/cli/test_engine_comparison.py` | New: engine comparison tests | | ||
| | `docs/PHASE_25_VALIDATION_REPORT.md` | New: this report | | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Empty file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,150 @@ | ||
| """E2E CLI test fixtures and markers. | ||
|
|
||
| Tests in this directory exercise the full CLI → core → adapter pipeline | ||
| against a real project (cf-test). Tests marked with `e2e_llm` make real | ||
| API calls and should be run explicitly: `uv run pytest -m e2e_llm`. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import hashlib | ||
| import os | ||
| import shutil | ||
| from pathlib import Path | ||
|
|
||
| import pytest | ||
|
|
||
| CF_TEST_PROJECT = Path( | ||
| os.getenv("CF_TEST_PROJECT", Path.home() / "projects" / "cf-test") | ||
| ) | ||
| CODEFRAME_ROOT = Path( | ||
| os.getenv("CODEFRAME_ROOT", Path.home() / "projects" / "codeframe") | ||
| ) | ||
|
|
||
|
|
||
| def _ensure_api_key() -> None: | ||
| """Eagerly load ANTHROPIC_API_KEY from .env if not already set.""" | ||
| if os.environ.get("ANTHROPIC_API_KEY"): | ||
| return | ||
| env_file = CODEFRAME_ROOT / ".env" | ||
| if env_file.exists(): | ||
| for line in env_file.read_text().splitlines(): | ||
| line = line.strip() | ||
| if line.startswith("ANTHROPIC_API_KEY="): | ||
| key = line.split("=", 1)[1].strip().strip('"').strip("'") | ||
| os.environ["ANTHROPIC_API_KEY"] = key | ||
| return | ||
|
|
||
|
|
||
| # Load API key at import time so subprocesses inherit it | ||
| _ensure_api_key() | ||
|
|
||
|
|
||
| def pytest_collection_modifyitems(config, items): | ||
| """Auto-mark all tests in this directory as e2e.""" | ||
| for item in items: | ||
| if "e2e/cli" in str(item.fspath): | ||
| item.add_marker(pytest.mark.e2e) | ||
|
|
||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # Fixtures | ||
| # --------------------------------------------------------------------------- | ||
|
|
||
|
|
||
| @pytest.fixture(scope="session") | ||
| def cf_test_path() -> Path: | ||
| """Path to the cf-test project.""" | ||
| if not CF_TEST_PROJECT.exists(): | ||
| pytest.skip(f"cf-test project not found at {CF_TEST_PROJECT}") | ||
| return CF_TEST_PROJECT | ||
|
|
||
|
|
||
| @pytest.fixture(scope="session") | ||
| def codeframe_root() -> Path: | ||
| """Path to the codeframe project root.""" | ||
| return CODEFRAME_ROOT | ||
|
|
||
|
|
||
| @pytest.fixture(scope="session") | ||
| def anthropic_api_key() -> str: | ||
| """Load ANTHROPIC_API_KEY from codeframe .env or environment.""" | ||
| key = os.environ.get("ANTHROPIC_API_KEY") | ||
| if key: | ||
| return key | ||
|
|
||
| env_file = CODEFRAME_ROOT / ".env" | ||
| if env_file.exists(): | ||
| for line in env_file.read_text().splitlines(): | ||
| line = line.strip() | ||
| if line.startswith("ANTHROPIC_API_KEY="): | ||
| key = line.split("=", 1)[1].strip().strip('"').strip("'") | ||
| if key: | ||
| os.environ["ANTHROPIC_API_KEY"] = key | ||
| return key | ||
|
|
||
| pytest.skip("ANTHROPIC_API_KEY not available") | ||
|
|
||
|
|
||
| @pytest.fixture(scope="module") | ||
| def pyproject_snapshot(cf_test_path: Path) -> dict: | ||
| """Capture a snapshot of pyproject.toml for preservation checks.""" | ||
| toml_path = cf_test_path / "pyproject.toml" | ||
| content = toml_path.read_text() | ||
| return { | ||
| "path": toml_path, | ||
| "content": content, | ||
| "hash": hashlib.sha256(content.encode()).hexdigest(), | ||
| } | ||
|
|
||
|
|
||
| @pytest.fixture(scope="module") | ||
| def clean_cf_test(cf_test_path: Path) -> Path: | ||
| """Clean the cf-test project, preserving only config and requirements. | ||
|
|
||
| Removes: .codeframe/, src/task_tracker/ contents (not __init__.py), | ||
| tests/ contents (not __init__.py), __pycache__ dirs. | ||
|
|
||
| Preserves: pyproject.toml, requirements.md, .gitignore, .python-version, | ||
| .venv/, uv.lock, README.md. | ||
| """ | ||
| # Remove .codeframe workspace | ||
| codeframe_dir = cf_test_path / ".codeframe" | ||
| if codeframe_dir.exists(): | ||
| shutil.rmtree(codeframe_dir) | ||
|
|
||
| # Remove generated source files (recursively, keep directory structure) | ||
| src_dir = cf_test_path / "src" / "task_tracker" | ||
| if src_dir.exists(): | ||
| for f in sorted(src_dir.rglob("*"), reverse=True): | ||
| if f.name == "__pycache__": | ||
| shutil.rmtree(f) | ||
| elif f.name != "__init__.py" and f.is_file(): | ||
| f.unlink() | ||
| elif f.is_dir() and not any(f.iterdir()): | ||
| f.rmdir() | ||
| # Reset __init__.py to empty | ||
| init_file = src_dir / "__init__.py" | ||
| init_file.write_text("") | ||
|
|
||
| # Remove generated test files (recursively, keep directory structure) | ||
| tests_dir = cf_test_path / "tests" | ||
| if tests_dir.exists(): | ||
| for f in sorted(tests_dir.rglob("*"), reverse=True): | ||
| if f.name == "__pycache__": | ||
| shutil.rmtree(f) | ||
| elif f.name != "__init__.py" and f.is_file(): | ||
| f.unlink() | ||
| elif f.is_dir() and not any(f.iterdir()): | ||
| f.rmdir() | ||
| init_file = tests_dir / "__init__.py" | ||
| if not init_file.exists(): | ||
| init_file.write_text("") | ||
|
|
||
| # Remove pytest/ruff caches | ||
| for cache_dir in [".pytest_cache", ".ruff_cache"]: | ||
| cache_path = cf_test_path / cache_dir | ||
| if cache_path.exists(): | ||
| shutil.rmtree(cache_path) | ||
|
|
||
| return cf_test_path |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.