refactor(patch): V4A mode gated to OpenAI-family mains — base schema is replace-only with real required (365 -> 195 for everyone else, -149; handler accepts both shapes from any model) (#97403)

2026-08-28 12:59:41 -07:00
parent 6da0ae1cf5
commit dadbfd8990
3 changed files with 201 additions and 34 deletions
@@ -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

@@ -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()
@@ -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)