fix: retry invalid goal patch contract once
This commit is contained in:
@@ -277,6 +277,49 @@ def validate_write_output(text: str) -> str:
|
||||
return extract_patch(text)
|
||||
|
||||
|
||||
def patch_repair_attempts() -> int:
|
||||
"""Return the bounded number of chances to repair an invalid model diff."""
|
||||
try:
|
||||
configured = int(os.environ.get("CASAN_GOAL_PATCH_REPAIR_ATTEMPTS", "1"))
|
||||
except ValueError:
|
||||
configured = 1
|
||||
return min(max(configured, 0), 1)
|
||||
|
||||
|
||||
def repair_write_output(model: str, original_prompt: str, invalid_output: str):
|
||||
"""Ask the producing model to repair format only, without relaxing H2.
|
||||
|
||||
A write-intent goal may never advance to approval without a checked unified
|
||||
diff. Models occasionally answer with an implementation plan despite the
|
||||
contract, so one bounded repair avoids treating a recoverable formatting
|
||||
lapse as a final H2 failure. The caller still H4-scans and validates the
|
||||
result before accepting it.
|
||||
"""
|
||||
if patch_repair_attempts() == 0:
|
||||
return False, "", {}, "goal_patch_missing"
|
||||
repair_prompt = (
|
||||
"Your previous response violated the required write-output contract because it did not contain a complete unified git diff. "
|
||||
"Return ONLY one complete patch inside a ```diff fence. The first non-empty line inside the fence MUST be `diff --git `. "
|
||||
"Do not explain, plan, summarize, use placeholders, or perform side effects. Preserve the original objective and workspace restrictions.\n\n"
|
||||
f"ORIGINAL CONTRACT:\n{original_prompt}\n\nPREVIOUS INVALID RESPONSE:\n{invalid_output[:8000]}"
|
||||
)
|
||||
ok, output, metadata, reason = call_model(model, repair_prompt, False)
|
||||
if not ok:
|
||||
return False, "", metadata, f"goal_patch_repair_failed:{reason}"
|
||||
return True, output, metadata, "ok"
|
||||
|
||||
|
||||
def merge_usage(primary: dict, additional: dict) -> dict:
|
||||
"""Preserve provider metadata while adding numeric usage from a repair call."""
|
||||
merged = dict(primary)
|
||||
for key, value in additional.items():
|
||||
if isinstance(value, (int, float)) and isinstance(merged.get(key), (int, float)):
|
||||
merged[key] = merged[key] + value
|
||||
elif key not in merged:
|
||||
merged[key] = value
|
||||
return merged
|
||||
|
||||
|
||||
def validate_and_store_patch(job_path: str, job: dict, patch: str) -> dict:
|
||||
roots = [str(value).strip("/") for value in job.get("workspace", {}).get("context_roots", [])]
|
||||
changed = []
|
||||
@@ -653,11 +696,27 @@ def run(job_path: str) -> int:
|
||||
if write_intent:
|
||||
try:
|
||||
validate_write_output(safe_local)
|
||||
except ValueError as error:
|
||||
reason = str(error)
|
||||
except ValueError as first_error:
|
||||
stage(job_path, "local-worker", "running", "Repairing invalid patch output contract", job.get("local_provider", ""), local_model)
|
||||
emit(goal_id, "H2-tool", "running", "Local worker is repairing patch output contract", {"reason": str(first_error), "max_attempts": patch_repair_attempts()})
|
||||
repaired, repaired_output, repaired_meta, repair_reason = repair_write_output(local_model, local_prompt, safe_local)
|
||||
local_meta = merge_usage(local_meta, repaired_meta)
|
||||
if not repaired:
|
||||
reason = repair_reason
|
||||
else:
|
||||
allowed, safe_local = scan(repaired_output, "output")
|
||||
if not allowed:
|
||||
emit(goal_id, "H4-security", "blocked", "Repaired local worker output rejected")
|
||||
raise ValueError("local_output_security_blocked")
|
||||
try:
|
||||
validate_write_output(safe_local)
|
||||
reason = ""
|
||||
except ValueError as repair_error:
|
||||
reason = f"goal_patch_repair_invalid:{repair_error}"
|
||||
if reason:
|
||||
stage(job_path, "local-worker", "error", reason, job.get("local_provider", ""), local_model)
|
||||
emit(goal_id, "H2-tool", "error", "Local worker violated patch output contract", {"reason": reason})
|
||||
raise
|
||||
emit(goal_id, "H2-tool", "error", "Local worker violated patch output contract after repair", {"reason": reason})
|
||||
raise ValueError(reason)
|
||||
stage(job_path, "local-worker", "pass", "Primary solution prepared", job.get("local_provider", ""), local_model)
|
||||
update_job(job_path, local_draft=safe_local, local_usage=local_meta)
|
||||
emit(goal_id, "H2-tool", "pass", "Local solution prepared", {"provider": job.get("local_provider", ""), "model": local_model, **local_meta})
|
||||
|
||||
@@ -38,6 +38,25 @@ class GoalPatchWorkflowTests(unittest.TestCase):
|
||||
value = ORCHESTRATOR.extract_patch("```diff\ndiff --git a/apps/okr/frontend/a.ts b/apps/okr/frontend/a.ts\n--- a/apps/okr/frontend/a.ts\n+++ b/apps/okr/frontend/a.ts\n@@ -1 +1 @@\n-a\n+b\n```")
|
||||
self.assertTrue(value.startswith("diff --git"))
|
||||
|
||||
def test_invalid_write_output_gets_one_bounded_repair_attempt(self):
|
||||
repaired = "```diff\ndiff --git a/apps/okr/frontend/a.ts b/apps/okr/frontend/a.ts\n--- a/apps/okr/frontend/a.ts\n+++ b/apps/okr/frontend/a.ts\n@@ -1 +1 @@\n-a\n+b\n```"
|
||||
with patch.dict(os.environ, {"CASAN_GOAL_PATCH_REPAIR_ATTEMPTS": "1"}, clear=False), \
|
||||
patch.object(ORCHESTRATOR, "call_model", return_value=(True, repaired, {"output_tokens": 9}, "ok")) as call:
|
||||
ok, value, usage, reason = ORCHESTRATOR.repair_write_output("ollama:test", "original prompt", "implementation plan")
|
||||
self.assertTrue(ok)
|
||||
self.assertEqual(reason, "ok")
|
||||
self.assertEqual(usage["output_tokens"], 9)
|
||||
self.assertIn("diff --git", value)
|
||||
self.assertEqual(call.call_count, 1)
|
||||
|
||||
def test_write_output_repair_can_be_disabled(self):
|
||||
with patch.dict(os.environ, {"CASAN_GOAL_PATCH_REPAIR_ATTEMPTS": "0"}, clear=False), \
|
||||
patch.object(ORCHESTRATOR, "call_model") as call:
|
||||
ok, _, _, reason = ORCHESTRATOR.repair_write_output("ollama:test", "original prompt", "implementation plan")
|
||||
self.assertFalse(ok)
|
||||
self.assertEqual(reason, "goal_patch_missing")
|
||||
call.assert_not_called()
|
||||
|
||||
def test_patch_outside_workspace_is_denied(self):
|
||||
job = {"workspace": {"context_roots": ["apps/okr/frontend"]}}
|
||||
content = "diff --git a/package.json b/package.json\n--- a/package.json\n+++ b/package.json\n@@ -1 +1 @@\n-a\n+b\n"
|
||||
|
||||
Reference in New Issue
Block a user