From dadbfd89901de2db4011e09a5fa66bf133832533 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Fri, 28 Aug 2026 12:59:41 -0700 Subject: [PATCH] =?UTF-8?q?refactor(patch):=20V4A=20mode=20gated=20to=20Op?= =?UTF-8?q?enAI-family=20mains=20=E2=80=94=20base=20schema=20is=20replace-?= =?UTF-8?q?only=20with=20real=20required=20(365=20->=20195=20for=20everyon?= =?UTF-8?q?e=20else,=20-149;=20handler=20accepts=20both=20shapes=20from=20?= =?UTF-8?q?any=20model)=20(#97403)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/tools/test_file_tools.py | 39 ++++++---- tests/tools/test_patch_v4a_gate.py | 85 ++++++++++++++++++++++ tools/file_tools.py | 111 +++++++++++++++++++++++------ 3 files changed, 201 insertions(+), 34 deletions(-) create mode 100644 tests/tools/test_patch_v4a_gate.py diff --git a/tests/tools/test_file_tools.py b/tests/tools/test_file_tools.py index 068b3d9f2b..58fb91ac2b 100644 --- a/tests/tools/test_file_tools.py +++ b/tests/tools/test_file_tools.py @@ -494,25 +494,36 @@ class TestSensitivePathCheck: class TestPatchSchemaShape: - """PATCH_SCHEMA must advertise per-mode required params via description - text (not JSON-schema ``required``), so strict models like kimi-k2.x stop - silently omitting old_string / new_string / patch content.""" + """The BASE schema is replace-only (V4A layers on for OpenAI-family + mains via _patch_schema_overrides — see test_patch_v4a_gate.py). The + kimi-k2.x per-mode-description concern now applies to the V4A LAYER, + whose composed variant still documents both modes' requirements.""" - def test_per_mode_required_params_documented_in_descriptions(self): + def test_base_schema_replace_only_with_real_required(self): desc = PATCH_SCHEMA["description"] - assert "REQUIRED PARAMETERS: mode, path, old_string, new_string" in desc - assert "REQUIRED PARAMETERS: mode, patch" in desc + assert "V4A" not in desc props = PATCH_SCHEMA["parameters"]["properties"] - for name in ("path", "old_string", "new_string"): - assert "REQUIRED when mode='replace'" in props[name]["description"] - assert "REQUIRED when mode='patch'" in props["patch"]["description"] + assert "mode" not in props and "patch" not in props + # replace-only means required can finally be the REAL contract — + # no per-mode description hedging needed on the base. + assert PATCH_SCHEMA["parameters"]["required"] == ["path", "old_string", "new_string"] assert "must differ from old_string" in props["new_string"]["description"] - def test_no_anyof_required_stays_mode_only(self): - # anyOf/oneOf at parameters level break Anthropic, Fireworks, and the - # Moonshot/Kimi schema sanitizer — description-level guidance is the - # only provider-safe signalling mechanism. - params = PATCH_SCHEMA["parameters"] + def test_v4a_layer_keeps_per_mode_documentation(self): + """When the V4A layer IS rendered (OpenAI-family), the strict-model + guidance survives: per-mode requirements in description text, no + anyOf/oneOf (breaks Anthropic/Fireworks/Kimi sanitizers).""" + from unittest.mock import patch as _p + + import tools.file_tools as ft + + with _p("agent.auxiliary_client._read_main_provider", return_value="openai"), \ + _p("agent.auxiliary_client._read_main_model", return_value="gpt-5.2"): + o = ft._patch_schema_overrides() + desc = o["description"] + assert "REQUIRED PARAMETERS: mode, path, old_string, new_string" in desc + assert "REQUIRED PARAMETERS: mode, patch" in desc + params = o["parameters"] assert params["required"] == ["mode"] assert "anyOf" not in params and "oneOf" not in params diff --git a/tests/tools/test_patch_v4a_gate.py b/tests/tools/test_patch_v4a_gate.py new file mode 100644 index 0000000000..7153ff57bc --- /dev/null +++ b/tests/tools/test_patch_v4a_gate.py @@ -0,0 +1,85 @@ +"""patch V4A provider gate (#95681). + +V4A is the OpenAI apply_patch dialect; the dual-mode schema taxed every +non-OpenAI session ~149 tok/call for a format their models weren't +trained on. Base schema = replace-only (mode gone, path/old/new required); +the V4A layer renders only for OpenAI-family mains. Handler accepts both +shapes from any model (replay compat; mode defaults to replace). +""" +import json +import os +import sys +import tempfile +import unittest +from unittest.mock import patch + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "..")) + +import tools.file_tools as ft + + +def _family(prov, model): + with patch("agent.auxiliary_client._read_main_provider", return_value=prov), \ + patch("agent.auxiliary_client._read_main_model", return_value=model): + return ft._is_openai_family_main() + + +def _override(prov, model): + with patch("agent.auxiliary_client._read_main_provider", return_value=prov), \ + patch("agent.auxiliary_client._read_main_model", return_value=model): + return ft._patch_schema_overrides() + + +class TestPatchV4AGate(unittest.TestCase): + def test_family_detector(self): + for prov, model, want in [ + ("openai", "gpt-5.2", True), + ("openai-codex", "codex-large", True), + ("azure-openai", "deploy-x", True), + ("openrouter", "openai/gpt-5.2", True), + ("nous", "openai/o5-mini", True), + ("openrouter", "anthropic/claude-sonnet-4", False), + ("anthropic", "claude-fable-5", False), + ("nous", "hermes-4-405b", False), + ("", "", False), + ]: + self.assertEqual(_family(prov, model), want, (prov, model)) + + def test_base_schema_is_replace_only(self): + props = ft.PATCH_SCHEMA["parameters"]["properties"] + self.assertNotIn("mode", props) + self.assertNotIn("patch", props) + self.assertEqual(ft.PATCH_SCHEMA["parameters"]["required"], + ["path", "old_string", "new_string"]) + self.assertNotIn("V4A", ft.PATCH_SCHEMA["description"]) + + def test_openai_family_gets_v4a_layer(self): + o = _override("openai", "gpt-5.2") + self.assertIn("V4A", o["description"]) + self.assertIn("mode", o["parameters"]["properties"]) + self.assertIn("patch", o["parameters"]["properties"]) + self.assertEqual(o["parameters"]["required"], ["mode"]) + + def test_non_openai_gets_no_override(self): + self.assertEqual(_override("anthropic", "claude-fable-5"), {}) + + def test_handler_accepts_both_shapes_regardless(self): + """Wire compat: replace works without mode; V4A applies even from + sessions whose schema never advertised it.""" + work = tempfile.mkdtemp(prefix="v4a_t_") + f1 = os.path.join(work, "a.txt") + open(f1, "w").write("alpha beta\n") + r = json.loads(ft.patch_tool(path=f1, old_string="beta", new_string="B")) + self.assertFalse(r.get("error"), r) + v4a = ( + "*** Begin Patch\n" + f"*** Update File: {f1}\n@@\n-alpha B\n+A B\n" + "*** End Patch" + ) + r = json.loads(ft.patch_tool(mode="patch", patch=v4a)) + self.assertFalse(r.get("error"), r) + self.assertEqual(open(f1).read().strip(), "A B") + + +if __name__ == "__main__": + unittest.main() diff --git a/tools/file_tools.py b/tools/file_tools.py index a83d73e13f..eef53d0895 100644 --- a/tools/file_tools.py +++ b/tools/file_tools.py @@ -2688,52 +2688,101 @@ WRITE_FILE_SCHEMA = { PATCH_SCHEMA = { "name": "patch", + # BASE = replace-only (what nearly every model family was trained on). + # The V4A patch mode (mode + patch params, dual-mode description) is + # LAYERED ON dynamically for OpenAI-family mains only — V4A is the + # OpenAI apply_patch dialect their models emit natively; advertising + # it to everyone cost every other session ~148 tok/call + # (_patch_schema_overrides below). The handler accepts BOTH shapes + # from any model regardless (replay compat + strong models that know + # V4A anyway): mode defaults to 'replace' when omitted. "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." + "Returns a unified diff. Auto-runs syntax checks after editing. " + "Finds a unique string and replaces it." ), "parameters": { "type": "object", "properties": { - "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.", + "description": "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.", + "description": "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'. Changed replacement text; it must differ from old_string. Pass empty string '' to delete the matched text.", + "description": "Changed replacement text; it must differ from old_string. 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", - }, # NOTE: handler still accepts `cross_profile` — see write_file's # NOTE (mirror-guard bypass only; unadvertised by design). + # NOTE: handler still accepts `mode` + `patch` (V4A) from ANY + # model — the schema just doesn't advertise them off-family. }, - "required": ["mode"], + "required": ["path", "old_string", "new_string"], }, } + +# V4A layer, rendered only for OpenAI-family main models (see PATCH_SCHEMA +# comment). Kept as data so the override composes it deterministically. +_PATCH_V4A_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." +) + +_PATCH_V4A_PARAMS = { + "mode": { + "type": "string", + "enum": ["replace", "patch"], + "description": "Edit mode. 'replace' (default): requires path + old_string + new_string. 'patch': requires patch content only.", + "default": "replace", + }, + "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", + }, +} + + +def _is_openai_family_main() -> bool: + """Whether the active main provider/model is the OpenAI/codex family — + the population trained on the V4A apply_patch dialect. + + Provider-family-coarse on purpose (no per-model training-diet table to + go stale): direct OpenAI providers always qualify; on aggregators + (openrouter/nous/azure...) the MODEL slug decides (gpt-*/o-series/ + codex). Fail-closed to the universal replace-only schema. + """ + try: + from agent.auxiliary_client import _read_main_model, _read_main_provider + + provider = (_read_main_provider() or "").strip().lower() + model = (_read_main_model() or "").strip().lower() + except Exception: # noqa: BLE001 + return False + if provider in {"openai", "openai-chat", "openai-codex", "azure-openai", "codex"}: + return True + # Aggregators: the model slug carries the family. + slug = model.split("/", 1)[-1] + if slug.startswith(("gpt-", "gpt.", "chatgpt", "codex", "o1", "o3", "o4", "o5")): + return True + return "openai/" in model + + SEARCH_FILES_SCHEMA = { "name": "search_files", "description": "Search file contents or find files by name. Use this instead of grep/rg/find/ls in terminal. Ripgrep-backed, faster than shell equivalents. On macOS, broad searches above the user home automatically skip TCC-protected folders (Desktop, Documents, Downloads, Library, Movies, Music, Pictures); target one directly when access is intentional.\n\nContent search (target='content'): Regex search inside files. Output modes: full matches with line numbers, file paths only, or match counts.\n\nFile search (target='files'): Find files by glob pattern (e.g., '*.py', '*config*'). Also use this instead of ls — results sorted by modification time.", @@ -2831,5 +2880,27 @@ def _read_file_schema_overrides(): registry.register(name="read_file", toolset="file", schema=READ_FILE_SCHEMA, handler=_handle_read_file, check_fn=_check_file_reqs, emoji="📖", max_result_size_chars=100_000, dynamic_schema_overrides=_read_file_schema_overrides) registry.register(name="write_file", toolset="file", schema=WRITE_FILE_SCHEMA, handler=_handle_write_file, check_fn=_check_file_reqs, emoji="✍️", max_result_size_chars=100_000) -registry.register(name="patch", toolset="file", schema=PATCH_SCHEMA, handler=_handle_patch, check_fn=_check_file_reqs, emoji="🔧", max_result_size_chars=100_000) +def _patch_schema_overrides(): + """Layer the V4A patch mode onto the base replace-only schema for + OpenAI-family mains (see PATCH_SCHEMA comment). Config/context probe + only — no I/O at schema-build time; compaction's tool refresh + (#97073) re-evaluates on model switches.""" + try: + if not _is_openai_family_main(): + return {} + params = { + "type": "object", + "properties": { + "mode": _PATCH_V4A_PARAMS["mode"], + **PATCH_SCHEMA["parameters"]["properties"], + "patch": _PATCH_V4A_PARAMS["patch"], + }, + "required": ["mode"], + } + return {"description": _PATCH_V4A_DESCRIPTION, "parameters": params} + except Exception: # noqa: BLE001 + return {} + + +registry.register(name="patch", toolset="file", schema=PATCH_SCHEMA, handler=_handle_patch, check_fn=_check_file_reqs, emoji="🔧", max_result_size_chars=100_000, dynamic_schema_overrides=_patch_schema_overrides) registry.register(name="search_files", toolset="file", schema=SEARCH_FILES_SCHEMA, handler=_handle_search_files, check_fn=_check_file_reqs, emoji="🔎", max_result_size_chars=100_000)