Steer stale patch repairs
Authored by
mfwolffe <wolffemf@dukes.jmu.edu>
- SHA
c7a21e5bf274b1c377dcfb2be7cc6c67c85dbbc0- Parents
-
7f616be - Tree
d6c12e5
c7a21e5
c7a21e5bf274b1c377dcfb2be7cc6c67c85dbbc07f616be
d6c12e5| Status | File | + | - |
|---|---|---|---|
| M |
src/loader/runtime/recovery.py
|
72 | 0 |
| M |
tests/test_recovery.py
|
77 | 0 |
src/loader/runtime/recovery.pymodified@@ -523,6 +523,17 @@ def categorize_error(error_message: str) -> ErrorCategory: | |||
| 523 | ): | 523 | ): |
| 524 | return ErrorCategory.INVALID_ARGUMENTS | 524 | return ErrorCategory.INVALID_ARGUMENTS |
| 525 | 525 | ||
| 526 | + if any( | ||
| 527 | + token in error_lower | ||
| 528 | + for token in [ | ||
| 529 | + "structured patch", | ||
| 530 | + "patch hunks are missing", | ||
| 531 | + "hunks must not be empty", | ||
| 532 | + "old_string not found", | ||
| 533 | + ] | ||
| 534 | + ): | ||
| 535 | + return ErrorCategory.INVALID_ARGUMENTS | ||
| 536 | + | ||
| 526 | if any( | 537 | if any( |
| 527 | token in error_lower | 538 | token in error_lower |
| 528 | for token in [ | 539 | for token in [ |
@@ -893,6 +904,54 @@ def _edit_old_string_mismatch_hints( | |||
| 893 | ] | 904 | ] |
| 894 | 905 | ||
| 895 | 906 | ||
| 907 | +def _is_patch_context_failure(error: str) -> bool: | ||
| 908 | + error_lower = str(error or "").lower() | ||
| 909 | + return any( | ||
| 910 | + token in error_lower | ||
| 911 | + for token in ( | ||
| 912 | + "structured patch hunk consumed", | ||
| 913 | + "structured patch context mismatch", | ||
| 914 | + "structured patch references lines past the end", | ||
| 915 | + "structured patch hunks overlap", | ||
| 916 | + ) | ||
| 917 | + ) | ||
| 918 | + | ||
| 919 | + | ||
| 920 | +def _patch_context_failure_hints( | ||
| 921 | + tool_name: str, | ||
| 922 | + args: dict[str, Any] | None, | ||
| 923 | + error: str, | ||
| 924 | +) -> list[str]: | ||
| 925 | + """Return recovery hints for brittle structured patch coordinates.""" | ||
| 926 | + | ||
| 927 | + if tool_name != "patch" or not _is_patch_context_failure(error): | ||
| 928 | + return [] | ||
| 929 | + | ||
| 930 | + target = str( | ||
| 931 | + (args or {}).get("file_path") or (args or {}).get("path") or "" | ||
| 932 | + ).strip() | ||
| 933 | + target_suffix = f" for `{target}`" if target else "" | ||
| 934 | + return [ | ||
| 935 | + ( | ||
| 936 | + "`patch` hunk coordinates/context did not match the current file" | ||
| 937 | + f"{target_suffix}; do not retry line-number hunks from memory." | ||
| 938 | + ), | ||
| 939 | + ( | ||
| 940 | + "Displayed wrapping is not source line numbering; line-number hunks " | ||
| 941 | + "past the real file length will fail." | ||
| 942 | + ), | ||
| 943 | + ( | ||
| 944 | + "For generated-file repairs, prefer " | ||
| 945 | + "`write(file_path=..., content=...)` with the full corrected file, " | ||
| 946 | + "or `edit` using exact current on-disk text." | ||
| 947 | + ), | ||
| 948 | + ( | ||
| 949 | + "If more evidence is truly needed, read only the target file; do not " | ||
| 950 | + "consult notepad/project memory or unrelated references." | ||
| 951 | + ), | ||
| 952 | + ] | ||
| 953 | + | ||
| 954 | + | ||
| 896 | RECOVERY_PROMPT = """## TOOL FAILURE - INVESTIGATE AND ADAPT | 955 | RECOVERY_PROMPT = """## TOOL FAILURE - INVESTIGATE AND ADAPT |
| 897 | 956 | ||
| 898 | The command failed. You MUST analyze the error and take a DIFFERENT action. | 957 | The command failed. You MUST analyze the error and take a DIFFERENT action. |
@@ -935,6 +994,9 @@ def format_recovery_prompt( | |||
| 935 | mismatch_hints = _edit_old_string_mismatch_hints(tool_name, args, error) | 994 | mismatch_hints = _edit_old_string_mismatch_hints(tool_name, args, error) |
| 936 | if mismatch_hints: | 995 | if mismatch_hints: |
| 937 | hints = "\n".join(f"- {hint}" for hint in mismatch_hints) + "\n" + hints | 996 | hints = "\n".join(f"- {hint}" for hint in mismatch_hints) + "\n" + hints |
| 997 | + patch_hints = _patch_context_failure_hints(tool_name, args, error) | ||
| 998 | + if patch_hints: | ||
| 999 | + hints = "\n".join(f"- {hint}" for hint in patch_hints) + "\n" + hints | ||
| 938 | args_str = ", ".join(f"{key}={value!r}" for key, value in args.items()) | 1000 | args_str = ", ".join(f"{key}={value!r}" for key, value in args.items()) |
| 939 | 1001 | ||
| 940 | return RECOVERY_PROMPT.format( | 1002 | return RECOVERY_PROMPT.format( |
@@ -1039,6 +1101,16 @@ def format_failure_message(context: RecoveryContext) -> str: | |||
| 1039 | "Use exact current on-disk text for `edit`, or switch to `patch`/`write`", | 1101 | "Use exact current on-disk text for `edit`, or switch to `patch`/`write`", |
| 1040 | "For large generated-file repairs, replace the whole file or anchor a patch to current content", | 1102 | "For large generated-file repairs, replace the whole file or anchor a patch to current content", |
| 1041 | ] | 1103 | ] |
| 1104 | + elif ( | ||
| 1105 | + last_attempt is not None | ||
| 1106 | + and last_attempt.tool_name == "patch" | ||
| 1107 | + and _is_patch_context_failure(last_attempt.error) | ||
| 1108 | + ): | ||
| 1109 | + specific_suggestions = [ | ||
| 1110 | + "Patch hunk coordinates/context were stale; do not retry line-number hunks from memory", | ||
| 1111 | + "Use `write(file_path=..., content=...)` for a full generated-file replacement", | ||
| 1112 | + "Or use `edit` with exact current on-disk text from the target file", | ||
| 1113 | + ] | ||
| 1042 | else: | 1114 | else: |
| 1043 | specific_suggestions = suggestions.get( | 1115 | specific_suggestions = suggestions.get( |
| 1044 | last_category, | 1116 | last_category, |
tests/test_recovery.pymodified@@ -44,6 +44,13 @@ class TestCategorizeError: | |||
| 44 | ) | 44 | ) |
| 45 | == ErrorCategory.INVALID_ARGUMENTS | 45 | == ErrorCategory.INVALID_ARGUMENTS |
| 46 | ) | 46 | ) |
| 47 | + assert ( | ||
| 48 | + categorize_error( | ||
| 49 | + "Error patching file: structured patch hunk consumed a different " | ||
| 50 | + "number of original lines than declared" | ||
| 51 | + ) | ||
| 52 | + == ErrorCategory.INVALID_ARGUMENTS | ||
| 53 | + ) | ||
| 47 | 54 | ||
| 48 | def test_network_error(self): | 55 | def test_network_error(self): |
| 49 | assert categorize_error("Network unreachable") == ErrorCategory.NETWORK_ERROR | 56 | assert categorize_error("Network unreachable") == ErrorCategory.NETWORK_ERROR |
@@ -250,6 +257,44 @@ class TestFormatRecoveryPrompt: | |||
| 250 | assert "current on-disk file" in prompt | 257 | assert "current on-disk file" in prompt |
| 251 | assert "write` with the complete replacement content" in prompt | 258 | assert "write` with the complete replacement content" in prompt |
| 252 | 259 | ||
| 260 | + def test_format_recovery_prompt_for_patch_context_miss_prefers_replacement(self): | ||
| 261 | + ctx = RecoveryContext( | ||
| 262 | + original_tool="patch", | ||
| 263 | + original_args={ | ||
| 264 | + "file_path": "~/Loader/guides/nginx/chapters/05-load-balancing.html", | ||
| 265 | + "hunks": [ | ||
| 266 | + { | ||
| 267 | + "old_start": 64, | ||
| 268 | + "old_lines": 1, | ||
| 269 | + "lines": ["<h2>More</h2>"], | ||
| 270 | + } | ||
| 271 | + ], | ||
| 272 | + }, | ||
| 273 | + ) | ||
| 274 | + ctx.add_attempt( | ||
| 275 | + "patch", | ||
| 276 | + ctx.original_args, | ||
| 277 | + ( | ||
| 278 | + "Error patching file: structured patch hunk consumed a different " | ||
| 279 | + "number of original lines than declared (0 vs 1)" | ||
| 280 | + ), | ||
| 281 | + ) | ||
| 282 | + | ||
| 283 | + prompt = format_recovery_prompt( | ||
| 284 | + ctx, | ||
| 285 | + "patch", | ||
| 286 | + ctx.original_args, | ||
| 287 | + ( | ||
| 288 | + "Error patching file: structured patch hunk consumed a different " | ||
| 289 | + "number of original lines than declared (0 vs 1)" | ||
| 290 | + ), | ||
| 291 | + ) | ||
| 292 | + | ||
| 293 | + assert "did not match the current file" in prompt | ||
| 294 | + assert "Displayed wrapping is not source line numbering" in prompt | ||
| 295 | + assert "write(file_path=..., content=...)" in prompt | ||
| 296 | + assert "notepad/project memory" in prompt | ||
| 297 | + | ||
| 253 | 298 | ||
| 254 | class TestFormatFailureMessage: | 299 | class TestFormatFailureMessage: |
| 255 | """Tests for failure message formatting.""" | 300 | """Tests for failure message formatting.""" |
@@ -289,3 +334,35 @@ class TestFormatFailureMessage: | |||
| 289 | assert "`old_string` was stale" in msg | 334 | assert "`old_string` was stale" in msg |
| 290 | assert "exact current on-disk text" in msg | 335 | assert "exact current on-disk text" in msg |
| 291 | assert "Try a completely different approach" not in msg | 336 | assert "Try a completely different approach" not in msg |
| 337 | + | ||
| 338 | + def test_format_failure_message_for_patch_context_miss_is_specific(self): | ||
| 339 | + ctx = RecoveryContext( | ||
| 340 | + original_tool="patch", | ||
| 341 | + original_args={ | ||
| 342 | + "file_path": "guide.html", | ||
| 343 | + "hunks": [ | ||
| 344 | + { | ||
| 345 | + "old_start": 12, | ||
| 346 | + "old_lines": 1, | ||
| 347 | + "lines": ["<h2>More</h2>"], | ||
| 348 | + } | ||
| 349 | + ], | ||
| 350 | + }, | ||
| 351 | + max_retries=2, | ||
| 352 | + ) | ||
| 353 | + ctx.add_attempt( | ||
| 354 | + "patch", | ||
| 355 | + ctx.original_args, | ||
| 356 | + "Error patching file: structured patch references lines past the end of the file", | ||
| 357 | + ) | ||
| 358 | + ctx.add_attempt( | ||
| 359 | + "patch", | ||
| 360 | + ctx.original_args, | ||
| 361 | + "Error patching file: structured patch references lines past the end of the file", | ||
| 362 | + ) | ||
| 363 | + | ||
| 364 | + msg = format_failure_message(ctx) | ||
| 365 | + | ||
| 366 | + assert "Patch hunk coordinates/context were stale" in msg | ||
| 367 | + assert "write(file_path=..., content=...)" in msg | ||
| 368 | + assert "Try a completely different approach" not in msg | ||