From 3adcc6441916c40f0c5135e65194ff9642c99f29 Mon Sep 17 00:00:00 2001 From: briandevans <252620095+briandevans@users.noreply.github.com> Date: Sat, 25 Apr 2026 07:26:37 -0700 Subject: [PATCH] fix(patch-tool): advertise per-mode required params in schema descriptions Models that enforce required-only constraints (e.g. kimi-k2.x) were omitting old_string/new_string for replace mode and patch for patch mode because the schema only declared required: ["mode"]. Add explicit "REQUIRED when mode='X'" markers to each conditionally-required property description and a top-level "REQUIRED PARAMETERS: ..." summary for each mode. Avoids anyOf/oneOf which break Anthropic, Fireworks, and Kimi/Moonshot providers. Add TestPatchSchemaShape to lock the shape. Fixes #15524 Co-Authored-By: Claude Sonnet 4.6 --- tests/tools/test_file_tools.py | 45 ++++++++++++++++++++++++++++++++ tools/file_tools.py | 47 +++++++++++++++++++++++++++------- 2 files changed, 83 insertions(+), 9 deletions(-) diff --git a/tests/tools/test_file_tools.py b/tests/tools/test_file_tools.py index 0ee0270fdf..3cc8ffa81e 100644 --- a/tests/tools/test_file_tools.py +++ b/tests/tools/test_file_tools.py @@ -361,4 +361,49 @@ class TestSearchHints: assert "offset=100" in raw +# --------------------------------------------------------------------------- +# PATCH_SCHEMA shape tests (issue #15524) +# --------------------------------------------------------------------------- +class TestPatchSchemaShape: + """PATCH_SCHEMA must advertise per-mode required params so strict models + (e.g. kimi-k2.x) don't silently omit old_string / new_string for replace + mode, or patch content for patch mode.""" + + def test_required_only_contains_mode(self): + # anyOf is incompatible with several providers (Anthropic, Fireworks, + # Kimi/Moonshot). The only safe approach is description-level guidance. + assert PATCH_SCHEMA["parameters"]["required"] == ["mode"] + + def test_top_level_description_documents_replace_mode_required_params(self): + desc = PATCH_SCHEMA["description"] + assert "REQUIRED PARAMETERS: mode, path, old_string, new_string" in desc + + def test_top_level_description_documents_patch_mode_required_params(self): + desc = PATCH_SCHEMA["description"] + assert "REQUIRED PARAMETERS: mode, patch" in desc + + def test_path_property_advertises_required_for_replace(self): + desc = PATCH_SCHEMA["parameters"]["properties"]["path"]["description"] + assert "REQUIRED when mode='replace'" in desc + + def test_old_string_property_advertises_required_for_replace(self): + desc = PATCH_SCHEMA["parameters"]["properties"]["old_string"]["description"] + assert "REQUIRED when mode='replace'" in desc + + def test_new_string_property_advertises_required_for_replace(self): + desc = PATCH_SCHEMA["parameters"]["properties"]["new_string"]["description"] + assert "REQUIRED when mode='replace'" in desc + + def test_patch_property_advertises_required_for_patch_mode(self): + desc = PATCH_SCHEMA["parameters"]["properties"]["patch"]["description"] + assert "REQUIRED when mode='patch'" in desc + + def test_no_anyof_at_parameters_level(self): + assert "anyOf" not in PATCH_SCHEMA["parameters"] + assert "oneOf" not in PATCH_SCHEMA["parameters"] + + def test_schema_is_provider_compatible_object(self): + params = PATCH_SCHEMA["parameters"] + assert params["type"] == "object" + assert isinstance(params["properties"], dict) diff --git a/tools/file_tools.py b/tools/file_tools.py index 200287dcbd..c197061ade 100644 --- a/tools/file_tools.py +++ b/tools/file_tools.py @@ -1055,19 +1055,48 @@ WRITE_FILE_SCHEMA = { PATCH_SCHEMA = { "name": "patch", - "description": "Targeted find-and-replace edits in files. Use this instead of sed/awk in terminal. Uses fuzzy matching (9 strategies) so minor whitespace/indentation differences won't break it. Returns a unified diff. Auto-runs syntax checks after editing.\n\nReplace mode (default): find a unique string and replace it.\nPatch mode: apply V4A multi-file patches for bulk changes.", + "description": ( + "Targeted find-and-replace edits in files. Use this instead of sed/awk in terminal. " + "Uses fuzzy matching (9 strategies) so minor whitespace/indentation differences won't break it. " + "Returns a unified diff. Auto-runs syntax checks after editing.\n\n" + "REPLACE MODE (mode='replace', default): find a unique string and replace it. " + "REQUIRED PARAMETERS: mode, path, old_string, new_string.\n" + "PATCH MODE (mode='patch'): apply V4A multi-file patches for bulk changes. " + "REQUIRED PARAMETERS: mode, patch." + ), "parameters": { "type": "object", "properties": { - "mode": {"type": "string", "enum": ["replace", "patch"], "description": "Edit mode: 'replace' for targeted find-and-replace, 'patch' for V4A multi-file patches", "default": "replace"}, - "path": {"type": "string", "description": "File path to edit (required for 'replace' mode)"}, - "old_string": {"type": "string", "description": "Text to find in the file (required for 'replace' mode). Must be unique in the file unless replace_all=true. Include enough surrounding context to ensure uniqueness."}, - "new_string": {"type": "string", "description": "Replacement text (required for 'replace' mode). Can be empty string to delete the matched text."}, - "replace_all": {"type": "boolean", "description": "Replace all occurrences instead of requiring a unique match (default: false)", "default": False}, - "patch": {"type": "string", "description": "V4A format patch content (required for 'patch' mode). Format:\n*** Begin Patch\n*** Update File: path/to/file\n@@ context hint @@\n context line\n-removed line\n+added line\n*** End Patch"} + "mode": { + "type": "string", + "enum": ["replace", "patch"], + "description": "Edit mode. 'replace' (default): requires path + old_string + new_string. 'patch': requires patch content only.", + "default": "replace", + }, + "path": { + "type": "string", + "description": "REQUIRED when mode='replace'. File path to edit.", + }, + "old_string": { + "type": "string", + "description": "REQUIRED when mode='replace'. Exact text to find and replace. Must be unique in the file unless replace_all=true. Include surrounding context lines to ensure uniqueness.", + }, + "new_string": { + "type": "string", + "description": "REQUIRED when mode='replace'. Replacement text. Pass empty string '' to delete the matched text.", + }, + "replace_all": { + "type": "boolean", + "description": "Replace all occurrences instead of requiring a unique match (default: false)", + "default": False, + }, + "patch": { + "type": "string", + "description": "REQUIRED when mode='patch'. V4A format patch content. Format:\n*** Begin Patch\n*** Update File: path/to/file\n@@ context hint @@\n context line\n-removed line\n+added line\n*** End Patch", + }, }, - "required": ["mode"] - } + "required": ["mode"], + }, } SEARCH_FILES_SCHEMA = {