This commit is contained in:
@@ -44,6 +44,8 @@ def evaluate_changes(
|
|||||||
errors: list[str] = []
|
errors: list[str] = []
|
||||||
if not summary.files:
|
if not summary.files:
|
||||||
errors.append("Agent produced no file changes")
|
errors.append("Agent produced no file changes")
|
||||||
|
elif all(filename.startswith("tests/") for filename in summary.files):
|
||||||
|
errors.append("Agent changed only tests; production behavior was not fixed")
|
||||||
if len(summary.files) > settings.max_changed_files:
|
if len(summary.files) > settings.max_changed_files:
|
||||||
errors.append(f"Changed {len(summary.files)} files; limit is {settings.max_changed_files}")
|
errors.append(f"Changed {len(summary.files)} files; limit is {settings.max_changed_files}")
|
||||||
if summary.changed_lines > settings.max_changed_lines:
|
if summary.changed_lines > settings.max_changed_lines:
|
||||||
|
|||||||
@@ -91,6 +91,16 @@ For an unknown-column failure, compare every alias.column in the observed SQL wi
|
|||||||
schema and identify the owning table before editing. Teach the exact ownership, join, grouping,
|
schema and identify the owning table before editing. Teach the exact ownership, join, grouping,
|
||||||
and real column names in semantics config, then add a reference SQL example for the reported
|
and real column names in semantics config, then add a reference SQL example for the reported
|
||||||
question shape. Never replace one missing column with an unverified synonym.
|
question shape. Never replace one missing column with an unverified synonym.
|
||||||
|
"""
|
||||||
|
placeholder_guidance = ''
|
||||||
|
if '__LAI_' in response:
|
||||||
|
placeholder_guidance = """
|
||||||
|
Privacy placeholder repair guidance:
|
||||||
|
Trace the actual answer path through AIService::generateDatabaseResponse, issued token
|
||||||
|
restoration, and ChatController's local fallback. A token can only be restored when it exists
|
||||||
|
in the exact token map returned by protect(); never test a made-up token against an unrelated
|
||||||
|
map. Cover model-invented or malformed __LAI_...__ output with an HTTP-backed AIService test
|
||||||
|
and verify the user receives a real local database-result fallback, not a raw placeholder.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
return f"""You are fixing failure report #{report.id} in the Laravel AI Chat repository.
|
return f"""You are fixing failure report #{report.id} in the Laravel AI Chat repository.
|
||||||
@@ -131,6 +141,7 @@ Generated SQL, if relevant:
|
|||||||
{sql or '[not available]'}
|
{sql or '[not available]'}
|
||||||
{tenant_scope}
|
{tenant_scope}
|
||||||
{sql_semantics}
|
{sql_semantics}
|
||||||
|
{placeholder_guidance}
|
||||||
|
|
||||||
Run focused tests while working. Leave the repository with the fix and regression test applied.
|
Run focused tests while working. Leave the repository with the fix and regression test applied.
|
||||||
"""
|
"""
|
||||||
|
|||||||
@@ -77,16 +77,21 @@ class AutomationWorker:
|
|||||||
)
|
)
|
||||||
policy = evaluate_changes(summary, self.settings, workspace.path)
|
policy = evaluate_changes(summary, self.settings, workspace.path)
|
||||||
if not policy.allowed:
|
if not policy.allowed:
|
||||||
if not summary.files and attempt < self.settings.max_agent_attempts:
|
retryable = not summary.files or all(
|
||||||
|
filename.startswith("tests/") for filename in summary.files
|
||||||
|
)
|
||||||
|
if retryable and attempt < self.settings.max_agent_attempts:
|
||||||
LOG.warning(
|
LOG.warning(
|
||||||
"Agent produced no changes for report %s on attempt %s; requesting correction",
|
"Policy rejected retryable changes for report %s on attempt %s: %s",
|
||||||
report.id,
|
report.id,
|
||||||
attempt,
|
attempt,
|
||||||
|
"; ".join(policy.errors),
|
||||||
)
|
)
|
||||||
task = build_retry_task(
|
task = build_retry_task(
|
||||||
report,
|
report,
|
||||||
RuntimeError(
|
RuntimeError(
|
||||||
"No file changes were applied. Make the smallest concrete code and regression-test edits now."
|
"Policy rejected changes: " + "; ".join(policy.errors)
|
||||||
|
+ ". Modify the owning production code and add a focused regression test."
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
continue
|
continue
|
||||||
|
|||||||
@@ -23,6 +23,16 @@ def test_policy_rejects_secrets_and_vendor_paths() -> None:
|
|||||||
assert len(result.errors) == 3
|
assert len(result.errors) == 3
|
||||||
|
|
||||||
|
|
||||||
|
def test_policy_rejects_test_only_repair() -> None:
|
||||||
|
result = evaluate_changes(
|
||||||
|
ChangeSummary(("tests/Unit/ExampleTest.php",), 20, 2),
|
||||||
|
settings(),
|
||||||
|
)
|
||||||
|
|
||||||
|
assert not result.allowed
|
||||||
|
assert "production behavior was not fixed" in result.errors[0]
|
||||||
|
|
||||||
|
|
||||||
def test_direct_main_blocks_validator_and_migrations() -> None:
|
def test_direct_main_blocks_validator_and_migrations() -> None:
|
||||||
config = replace(
|
config = replace(
|
||||||
settings(),
|
settings(),
|
||||||
|
|||||||
@@ -152,3 +152,27 @@ def test_customer_database_task_explains_tenant_scope() -> None:
|
|||||||
assert "must not" in task
|
assert "must not" in task
|
||||||
assert "guessed column" in task
|
assert "guessed column" in task
|
||||||
assert "tenant_type" in task
|
assert "tenant_type" in task
|
||||||
|
|
||||||
|
|
||||||
|
def test_unresolved_placeholder_task_guides_real_answer_path() -> None:
|
||||||
|
report = FailureReport(
|
||||||
|
id=49,
|
||||||
|
chat_message_id=1,
|
||||||
|
source="user_feedback",
|
||||||
|
category="incorrect_answer",
|
||||||
|
user_feedback="Token was not restored",
|
||||||
|
feedback_type="wrong_result",
|
||||||
|
expected_behavior="Return the database value",
|
||||||
|
prompt="Average price",
|
||||||
|
response="Price: __LAI_UNKNOWN_VALUE__",
|
||||||
|
sql_query="SELECT AVG(price)",
|
||||||
|
error_message=None,
|
||||||
|
context={"database_type": "erp"},
|
||||||
|
)
|
||||||
|
|
||||||
|
task = build_task(report)
|
||||||
|
|
||||||
|
assert "AIService::generateDatabaseResponse" in task
|
||||||
|
assert "exact token map" in task
|
||||||
|
assert "HTTP-backed AIService test" in task
|
||||||
|
assert "local database-result fallback" in task
|
||||||
|
|||||||
@@ -131,7 +131,9 @@ def test_failed_validation_gets_one_correction_before_pr() -> None:
|
|||||||
workspace = Mock()
|
workspace = Mock()
|
||||||
workspace.path = Path("/tmp/nonexistent-workspace")
|
workspace.path = Path("/tmp/nonexistent-workspace")
|
||||||
workspace.branch = "ai/failure-13"
|
workspace.branch = "ai/failure-13"
|
||||||
workspace.summary.return_value = ChangeSummary(("tests/Feature/FixTest.php",), 5, 0)
|
workspace.summary.return_value = ChangeSummary(
|
||||||
|
("app/Services/Fix.php", "tests/Feature/FixTest.php"), 10, 1
|
||||||
|
)
|
||||||
workspace.commit.return_value = "commit-id"
|
workspace.commit.return_value = "commit-id"
|
||||||
|
|
||||||
with patch("lai_automation.worker.GitWorkspace", return_value=workspace):
|
with patch("lai_automation.worker.GitWorkspace", return_value=workspace):
|
||||||
@@ -172,8 +174,8 @@ def test_no_change_response_gets_one_correction() -> None:
|
|||||||
workspace.branch = "ai/failure-14"
|
workspace.branch = "ai/failure-14"
|
||||||
workspace.summary.side_effect = [
|
workspace.summary.side_effect = [
|
||||||
ChangeSummary((), 0, 0),
|
ChangeSummary((), 0, 0),
|
||||||
ChangeSummary(("tests/Feature/FixTest.php",), 5, 0),
|
ChangeSummary(("app/Services/Fix.php", "tests/Feature/FixTest.php"), 10, 1),
|
||||||
ChangeSummary(("tests/Feature/FixTest.php",), 5, 0),
|
ChangeSummary(("app/Services/Fix.php", "tests/Feature/FixTest.php"), 10, 1),
|
||||||
]
|
]
|
||||||
workspace.commit.return_value = "commit-id"
|
workspace.commit.return_value = "commit-id"
|
||||||
|
|
||||||
@@ -181,6 +183,47 @@ def test_no_change_response_gets_one_correction() -> None:
|
|||||||
assert worker.run_once()
|
assert worker.run_once()
|
||||||
|
|
||||||
assert worker.agent.run.call_count == 2
|
assert worker.agent.run.call_count == 2
|
||||||
assert "No file changes" in worker.agent.run.call_args_list[1].args[1]
|
assert "Agent produced no file changes" in worker.agent.run.call_args_list[1].args[1]
|
||||||
worker._run_tests.assert_called_once()
|
worker._run_tests.assert_called_once()
|
||||||
workspace.push_branch.assert_called_once()
|
workspace.push_branch.assert_called_once()
|
||||||
|
|
||||||
|
|
||||||
|
def test_test_only_change_gets_correction_before_validation() -> None:
|
||||||
|
report = FailureReport(
|
||||||
|
id=49,
|
||||||
|
chat_message_id=1,
|
||||||
|
source="user_feedback",
|
||||||
|
category="incorrect_answer",
|
||||||
|
user_feedback="Token was not restored",
|
||||||
|
feedback_type="wrong_result",
|
||||||
|
expected_behavior="Return the real value",
|
||||||
|
prompt="Average price",
|
||||||
|
response="__LAI_UNKNOWN__",
|
||||||
|
sql_query="SELECT AVG(price)",
|
||||||
|
error_message=None,
|
||||||
|
context={},
|
||||||
|
)
|
||||||
|
config = settings(mode="pr", dry_run=False, gitea_token="token")
|
||||||
|
worker = AutomationWorker(config)
|
||||||
|
worker.reports = Mock()
|
||||||
|
worker.reports.claim_next.return_value = report
|
||||||
|
worker.agent = Mock()
|
||||||
|
worker.agent.run.side_effect = ["tests only", "production fix"]
|
||||||
|
worker.gitea = Mock()
|
||||||
|
worker._run_tests = Mock(return_value=[])
|
||||||
|
workspace = Mock()
|
||||||
|
workspace.path = Path("/tmp/nonexistent-workspace")
|
||||||
|
workspace.branch = "ai/failure-49"
|
||||||
|
workspace.summary.side_effect = [
|
||||||
|
ChangeSummary(("tests/Unit/FixTest.php",), 8, 0),
|
||||||
|
ChangeSummary(("app/Services/Fix.php", "tests/Unit/FixTest.php"), 15, 1),
|
||||||
|
ChangeSummary(("app/Services/Fix.php", "tests/Unit/FixTest.php"), 15, 1),
|
||||||
|
]
|
||||||
|
workspace.commit.return_value = "commit-id"
|
||||||
|
|
||||||
|
with patch("lai_automation.worker.GitWorkspace", return_value=workspace):
|
||||||
|
assert worker.run_once()
|
||||||
|
|
||||||
|
assert worker.agent.run.call_count == 2
|
||||||
|
assert "changed only tests" in worker.agent.run.call_args_list[1].args[1]
|
||||||
|
worker._run_tests.assert_called_once()
|
||||||
|
|||||||
Reference in New Issue
Block a user