feat(validation): implement Fix-then-Revalidate orchestration loop for required validations
CI / benchmark-publish (pull_request) Has been skipped
CI / build (pull_request) Successful in 15s
CI / lint (pull_request) Successful in 3m40s
CI / quality (pull_request) Successful in 4m13s
CI / security (pull_request) Successful in 4m16s
CI / typecheck (pull_request) Successful in 4m16s
CI / integration_tests (pull_request) Successful in 9m11s
CI / unit_tests (pull_request) Successful in 9m23s
CI / docker (pull_request) Successful in 1m18s
CI / coverage (pull_request) Successful in 11m48s
CI / status-check (pull_request) Successful in 7s
CI / e2e_tests (pull_request) Successful in 8m24s
CI / lint (push) Successful in 3m18s
CI / build (push) Failing after 13s
CI / typecheck (push) Successful in 3m54s
CI / benchmark-regression (push) Has been skipped
CI / quality (push) Successful in 4m15s
CI / security (push) Successful in 4m39s
CI / integration_tests (push) Successful in 7m21s
CI / unit_tests (push) Successful in 7m44s
CI / docker (push) Successful in 1m9s
CI / e2e_tests (push) Successful in 11m9s
CI / coverage (push) Successful in 13m41s
CI / status-check (push) Successful in 2s
CI / benchmark-regression (pull_request) Successful in 48m49s
CI / benchmark-publish (push) Successful in 26m25s
CI / benchmark-publish (pull_request) Has been skipped
CI / build (pull_request) Successful in 15s
CI / lint (pull_request) Successful in 3m40s
CI / quality (pull_request) Successful in 4m13s
CI / security (pull_request) Successful in 4m16s
CI / typecheck (pull_request) Successful in 4m16s
CI / integration_tests (pull_request) Successful in 9m11s
CI / unit_tests (pull_request) Successful in 9m23s
CI / docker (pull_request) Successful in 1m18s
CI / coverage (pull_request) Successful in 11m48s
CI / status-check (pull_request) Successful in 7s
CI / e2e_tests (pull_request) Successful in 8m24s
CI / lint (push) Successful in 3m18s
CI / build (push) Failing after 13s
CI / typecheck (push) Successful in 3m54s
CI / benchmark-regression (push) Has been skipped
CI / quality (push) Successful in 4m15s
CI / security (push) Successful in 4m39s
CI / integration_tests (push) Successful in 7m21s
CI / unit_tests (push) Successful in 7m44s
CI / docker (push) Successful in 1m9s
CI / e2e_tests (push) Successful in 11m9s
CI / coverage (push) Successful in 13m41s
CI / status-check (push) Successful in 2s
CI / benchmark-regression (pull_request) Successful in 48m49s
CI / benchmark-publish (push) Successful in 26m25s
Implemented FixThenRevalidateOrchestrator with full diagnosis → self-fix → re-validation → retry limit → strategy revision → user escalation → terminal failure flow. Added retry counting per validation per plan, configurable retry limit (default 3), auto_strategy_revision flag support, and validation_fix_history recording in plan execution metadata. Review fixes applied (round 1): - Added try/except around fix_callback and revalidate_callback (spec: validation errors treated as required failure regardless of mode) - Wired optional EventBus for VALIDATION_FIX_ATTEMPTED/SUCCEEDED/EXHAUSTED events - Fixed total_attempts derivation from fix_history length instead of cumulative retry counts - Updated module docstring to reflect implemented vs caller-responsible steps - Added exhausted-retry logging on re-invocation - Replaced bare assert in Robot helper with explicit _check() function - Added 10 new Behave scenarios covering exception paths, multi-failure, reset, field validation, boundary values, event_bus property, and exhausted re-invocation Review fixes applied (round 2 — PR #711): - B1+B7: Fixed TOCTOU race in _fix_single_validation; atomic claim-per-iteration under RLock; eliminated defaultdict auto-vivification with .get() reads - B2: Added validation_name mismatch check on revalidate_callback return - B3: Added fix_description truncation to 2000 chars before FixAttemptRecord - T1: Switched threading.Lock to threading.RLock for defensive reentrancy - A1: Added model_validator preventing escalated+terminal_failure both True - R1: Added max_length=255 to FixAttemptRecord.validation_name - S7: Fixed misleading docstring about automation_profile integration - D4: Fixed timestamp field description to specify UTC - D1: Renamed misleading benchmark time_fix_after_two_retries - Added 5 new Behave scenarios for truncation, model constraints, name mismatch ISSUES CLOSED: #583
This commit was merged in pull request #711.
This commit is contained in:
@@ -0,0 +1,304 @@
|
||||
"""Helper script for Robot Framework fix-then-revalidate integration tests.
|
||||
|
||||
Self-contained Python helper that exercises the FixThenRevalidateOrchestrator
|
||||
and its models, printing sentinel strings on success and exiting
|
||||
with code 1 on failure.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import sys
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
# Ensure src is importable when run from workspace root
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "src"))
|
||||
|
||||
from cleveragents.application.services.fix_then_revalidate import (
|
||||
FixAttemptRecord,
|
||||
FixThenRevalidateOrchestrator,
|
||||
FixThenRevalidateResult,
|
||||
)
|
||||
from cleveragents.application.services.validation_pipeline import (
|
||||
ValidationPipeline,
|
||||
ValidationResult,
|
||||
)
|
||||
from cleveragents.domain.models.core.tool import ValidationMode
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Mock helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _mock_executor(validation_name: str, arguments: dict[str, Any]) -> dict[str, Any]:
|
||||
return {"passed": True, "message": f"{validation_name} ok"}
|
||||
|
||||
|
||||
def _make_failed_result(
|
||||
name: str,
|
||||
mode: ValidationMode = ValidationMode.REQUIRED,
|
||||
) -> ValidationResult:
|
||||
return ValidationResult(
|
||||
validation_name=name,
|
||||
resource_id="RES0001",
|
||||
resource_name="test-resource",
|
||||
mode=mode,
|
||||
passed=False,
|
||||
message=f"{name} failed",
|
||||
data={"hint": "check config"},
|
||||
duration_ms=10.0,
|
||||
)
|
||||
|
||||
|
||||
def _always_fix(result: ValidationResult) -> str:
|
||||
return f"Fixed {result.validation_name}"
|
||||
|
||||
|
||||
class _CountingRevalidate:
|
||||
def __init__(self, pass_after: int) -> None:
|
||||
self._pass_after = pass_after
|
||||
self._count = 0
|
||||
|
||||
def __call__(self, result: ValidationResult) -> ValidationResult:
|
||||
self._count += 1
|
||||
passed = self._count >= self._pass_after
|
||||
return ValidationResult(
|
||||
validation_name=result.validation_name,
|
||||
resource_id=result.resource_id,
|
||||
resource_name=result.resource_name,
|
||||
mode=result.mode,
|
||||
passed=passed,
|
||||
message="passed" if passed else "still failing",
|
||||
data=result.data,
|
||||
duration_ms=5.0,
|
||||
)
|
||||
|
||||
|
||||
class _NeverPassRevalidate:
|
||||
def __call__(self, result: ValidationResult) -> ValidationResult:
|
||||
return ValidationResult(
|
||||
validation_name=result.validation_name,
|
||||
resource_id=result.resource_id,
|
||||
resource_name=result.resource_name,
|
||||
mode=result.mode,
|
||||
passed=False,
|
||||
message="still failing",
|
||||
data=result.data,
|
||||
duration_ms=5.0,
|
||||
)
|
||||
|
||||
|
||||
def _check(condition: bool, message: str) -> None:
|
||||
"""Explicit check that works even with Python -O optimization."""
|
||||
if not condition:
|
||||
raise AssertionError(message)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Sub-commands
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_models() -> None:
|
||||
"""Verify FixAttemptRecord and FixThenRevalidateResult models."""
|
||||
record = FixAttemptRecord(
|
||||
attempt_number=1,
|
||||
validation_name="check-syntax",
|
||||
fix_description="Adjusted indentation",
|
||||
success=True,
|
||||
)
|
||||
_check(record.attempt_number == 1, "attempt_number should be 1")
|
||||
_check(record.success is True, "success should be True")
|
||||
|
||||
result = FixThenRevalidateResult(
|
||||
validation_attempts=2,
|
||||
final_passed=True,
|
||||
validation_fix_history=[record],
|
||||
escalated=False,
|
||||
terminal_failure=False,
|
||||
)
|
||||
_check(result.validation_attempts == 2, "validation_attempts should be 2")
|
||||
_check(result.final_passed is True, "final_passed should be True")
|
||||
_check(
|
||||
len(result.validation_fix_history) == 1,
|
||||
"validation_fix_history should have 1 record",
|
||||
)
|
||||
print("models-ok")
|
||||
|
||||
|
||||
def test_fix_succeeds_first_attempt() -> None:
|
||||
"""Fix succeeds on first attempt."""
|
||||
pipeline = ValidationPipeline(commands=[], executor=_mock_executor)
|
||||
orchestrator = FixThenRevalidateOrchestrator(
|
||||
validation_pipeline=pipeline,
|
||||
max_retries=3,
|
||||
)
|
||||
failed = [_make_failed_result("check-syntax")]
|
||||
result = orchestrator.run_fix_loop(
|
||||
plan_id="PLAN001",
|
||||
failed_results=failed,
|
||||
fix_callback=_always_fix,
|
||||
revalidate_callback=_CountingRevalidate(pass_after=1),
|
||||
)
|
||||
_check(result.final_passed is True, "final_passed should be True")
|
||||
_check(result.validation_attempts == 1, "validation_attempts should be 1")
|
||||
_check(
|
||||
len(result.validation_fix_history) == 1,
|
||||
"validation_fix_history should have 1 record",
|
||||
)
|
||||
_check(
|
||||
result.validation_fix_history[0].success is True,
|
||||
"first fix history record should be success",
|
||||
)
|
||||
print("fix-first-ok")
|
||||
|
||||
|
||||
def test_fix_succeeds_after_multiple() -> None:
|
||||
"""Fix succeeds after multiple attempts."""
|
||||
pipeline = ValidationPipeline(commands=[], executor=_mock_executor)
|
||||
orchestrator = FixThenRevalidateOrchestrator(
|
||||
validation_pipeline=pipeline,
|
||||
max_retries=3,
|
||||
)
|
||||
failed = [_make_failed_result("check-lint")]
|
||||
result = orchestrator.run_fix_loop(
|
||||
plan_id="PLAN002",
|
||||
failed_results=failed,
|
||||
fix_callback=_always_fix,
|
||||
revalidate_callback=_CountingRevalidate(pass_after=2),
|
||||
)
|
||||
_check(result.final_passed is True, "final_passed should be True")
|
||||
_check(result.validation_attempts == 2, "validation_attempts should be 2")
|
||||
print("fix-multiple-ok")
|
||||
|
||||
|
||||
def test_terminal_failure() -> None:
|
||||
"""Terminal failure when all attempts exhausted and no strategy revision."""
|
||||
pipeline = ValidationPipeline(commands=[], executor=_mock_executor)
|
||||
orchestrator = FixThenRevalidateOrchestrator(
|
||||
validation_pipeline=pipeline,
|
||||
max_retries=3,
|
||||
auto_strategy_revision=1.0,
|
||||
)
|
||||
failed = [_make_failed_result("check-deploy")]
|
||||
result = orchestrator.run_fix_loop(
|
||||
plan_id="PLAN003",
|
||||
failed_results=failed,
|
||||
fix_callback=_always_fix,
|
||||
revalidate_callback=_NeverPassRevalidate(),
|
||||
)
|
||||
_check(result.final_passed is False, "final_passed should be False")
|
||||
_check(result.needs_user_escalation is True, "needs_user_escalation should be True")
|
||||
_check(result.escalated is False, "escalated should be False")
|
||||
print("terminal-failure-ok")
|
||||
|
||||
|
||||
def test_escalation() -> None:
|
||||
"""Escalation to strategy revision when auto_strategy_revision is 0.0."""
|
||||
pipeline = ValidationPipeline(commands=[], executor=_mock_executor)
|
||||
orchestrator = FixThenRevalidateOrchestrator(
|
||||
validation_pipeline=pipeline,
|
||||
max_retries=3,
|
||||
auto_strategy_revision=0.0,
|
||||
)
|
||||
failed = [_make_failed_result("check-security")]
|
||||
result = orchestrator.run_fix_loop(
|
||||
plan_id="PLAN004",
|
||||
failed_results=failed,
|
||||
fix_callback=_always_fix,
|
||||
revalidate_callback=_NeverPassRevalidate(),
|
||||
)
|
||||
_check(result.final_passed is False, "final_passed should be False")
|
||||
_check(result.escalated is True, "escalated should be True")
|
||||
_check(result.terminal_failure is False, "terminal_failure should be False")
|
||||
print("escalation-ok")
|
||||
|
||||
|
||||
def test_informational_skipped() -> None:
|
||||
"""Informational validations are skipped by the fix loop."""
|
||||
pipeline = ValidationPipeline(commands=[], executor=_mock_executor)
|
||||
orchestrator = FixThenRevalidateOrchestrator(
|
||||
validation_pipeline=pipeline,
|
||||
max_retries=3,
|
||||
)
|
||||
failed = [_make_failed_result("check-style", ValidationMode.INFORMATIONAL)]
|
||||
result = orchestrator.run_fix_loop(
|
||||
plan_id="PLAN005",
|
||||
failed_results=failed,
|
||||
fix_callback=_always_fix,
|
||||
revalidate_callback=_CountingRevalidate(pass_after=1),
|
||||
)
|
||||
_check(result.final_passed is True, "final_passed should be True")
|
||||
_check(result.validation_attempts == 0, "validation_attempts should be 0")
|
||||
_check(
|
||||
len(result.validation_fix_history) == 0,
|
||||
"validation_fix_history should be empty",
|
||||
)
|
||||
print("informational-skipped-ok")
|
||||
|
||||
|
||||
def test_retry_counting() -> None:
|
||||
"""Verify retry counting per validation per plan."""
|
||||
pipeline = ValidationPipeline(commands=[], executor=_mock_executor)
|
||||
orchestrator = FixThenRevalidateOrchestrator(
|
||||
validation_pipeline=pipeline,
|
||||
max_retries=3,
|
||||
)
|
||||
_check(
|
||||
orchestrator.get_retry_count("P1", "check-a") == 0,
|
||||
"initial retry count should be 0",
|
||||
)
|
||||
failed = [_make_failed_result("check-a")]
|
||||
orchestrator.run_fix_loop(
|
||||
plan_id="P1",
|
||||
failed_results=failed,
|
||||
fix_callback=_always_fix,
|
||||
revalidate_callback=_CountingRevalidate(pass_after=2),
|
||||
)
|
||||
_check(
|
||||
orchestrator.get_retry_count("P1", "check-a") == 0,
|
||||
"retry count should be 0 after run (auto-reset)",
|
||||
)
|
||||
orchestrator.reset_retry_counts("P1")
|
||||
_check(
|
||||
orchestrator.get_retry_count("P1", "check-a") == 0,
|
||||
"retry count should be 0 after explicit reset (no-op)",
|
||||
)
|
||||
print("retry-counting-ok")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Dispatch
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
COMMANDS = {
|
||||
"models": test_models,
|
||||
"fix_first": test_fix_succeeds_first_attempt,
|
||||
"fix_multiple": test_fix_succeeds_after_multiple,
|
||||
"terminal_failure": test_terminal_failure,
|
||||
"escalation": test_escalation,
|
||||
"informational_skipped": test_informational_skipped,
|
||||
"retry_counting": test_retry_counting,
|
||||
}
|
||||
|
||||
|
||||
def main() -> None:
|
||||
if len(sys.argv) < 2:
|
||||
print("Usage: helper_fix_then_revalidate.py <command>", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
|
||||
cmd = sys.argv[1]
|
||||
func = COMMANDS.get(cmd)
|
||||
if func is None:
|
||||
print(f"Unknown command: {cmd}", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
|
||||
try:
|
||||
func()
|
||||
except Exception as exc:
|
||||
print(f"FAIL: {exc}", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
main()
|
||||
Reference in New Issue
Block a user