diff --git a/docs/dev/editing-strategy.md b/docs/dev/editing-strategy.md index 7e0baf7..de706e7 100644 --- a/docs/dev/editing-strategy.md +++ b/docs/dev/editing-strategy.md @@ -97,15 +97,15 @@ The run_start trace records: Use replace_text when all of these are true: - the target is an existing regular UTF-8 text file; -- the intended change is localized; -- current source text supplies an exact, unique anchor; -- the anchor is reasonably bounded. +- the intended change fits one localized code region; +- current source text supplies the smallest exact anchor that matches once. Use apply_patch directly when: - creating or deleting a file; - applying a large structural rewrite; -- the exact anchor would reproduce an impractically large source block; +- changing multiple separated regions or rewriting a large definition; +- making the exact anchor unique would require an impractically large source block; - replace_text does not support the target operation. Issue #10 also names renames as a patch case. The current Issue #8 patch diff --git a/src/yada/agents/prompts.py b/src/yada/agents/prompts.py index 815a222..750f99f 100644 --- a/src/yada/agents/prompts.py +++ b/src/yada/agents/prompts.py @@ -45,10 +45,11 @@ _REPLACE_FIRST_POLICY = """ Editing strategy: replace-first. -- For a localized change to an existing text file, prefer replace_text with an exact, - unique, reasonably bounded old_text. -- Use apply_patch for new or deleted files, broad structural changes, or edits that - cannot be expressed with a reasonably sized exact anchor. +- Prefer replace_text when the change fits one localized code region and can use the + smallest exact old_text that matches once. +- Use apply_patch for new or deleted files, broad structural changes, multiple separated + regions, large definition rewrites, or edits that need a large source block only to + make old_text unique. - After a failure, follow the structured recovery instruction. Retry or switch tools only in a later turn after observing the result. """ @@ -82,11 +83,9 @@ def task_prompt(task: str) -> str: The stable user-message template used to start an agent run. """ - return f"""Workspace: the tool root (shown as `.`). - -Task: + return f"""Task: {task.strip()} -Complete the task autonomously. Existing visible tests may be used, but hidden tests are -not available. Preserve existing behavior outside the requested change. +Complete the task autonomously. Use only files and tests available in the workspace. +Preserve existing behavior outside the requested change. """ diff --git a/src/yada/tools/schemas.py b/src/yada/tools/schemas.py index 3cd11df..d0b74c4 100644 --- a/src/yada/tools/schemas.py +++ b/src/yada/tools/schemas.py @@ -9,11 +9,17 @@ "type": "function", "function": { "name": "search_code", - "description": "Search text or regex in workspace files when the target location is unclear.", + "description": "Search a regular expression in workspace files when the target location is unclear.", "parameters": { "type": "object", "properties": { - "query": {"type": "string", "description": "Regex search pattern."}, + "query": { + "type": "string", + "description": ( + "Regular expression understood by ripgrep/Python re; " + "escape regex metacharacters when searching for literal text." + ), + }, "path": { "type": "string", "description": "Workspace-relative path; default '.'.", @@ -40,9 +46,21 @@ "parameters": { "type": "object", "properties": { - "path": {"type": "string"}, - "start_line": {"type": "integer"}, - "end_line": {"type": "integer"}, + "path": { + "type": "string", + "description": "Workspace-relative path to an existing file.", + }, + "start_line": { + "type": "integer", + "description": "One-based first line, inclusive; default 1.", + }, + "end_line": { + "type": "integer", + "description": ( + "One-based final line, inclusive; defaults to a 200-line " + "window from start_line. At most 400 lines may be read." + ), + }, }, "required": ["path"], "additionalProperties": False, @@ -53,21 +71,40 @@ "type": "function", "function": { "name": "apply_patch", - "description": "Apply a git-style unified diff transactionally. Every touched existing file needs its read_file SHA-256; new files use NEW.", + "description": ( + "Apply a git-style unified diff transactionally. Hunk old/new line " + "counts are recalculated automatically and need not be exact, but " + "headers must be valid and context must match. expected_files must " + "list exactly every patch target with no extra or missing paths; " + "existing files use their read_file SHA-256 and new files use NEW." + ), "parameters": { "type": "object", "properties": { "patch": { "type": "string", - "description": "Unified diff with diff --git headers.", + "description": "Unified diff with one diff --git header per target.", }, "expected_files": { "type": "array", + "description": ( + "Exact set of files touched by patch, with one entry per " + "target." + ), "items": { "type": "object", "properties": { - "path": {"type": "string"}, - "sha256": {"type": "string"}, + "path": { + "type": "string", + "description": "Workspace-relative patch target path.", + }, + "sha256": { + "type": "string", + "description": ( + "Current read_file SHA-256 for an existing " + "file, or NEW for a new file." + ), + }, }, "required": ["path", "sha256"], "additionalProperties": False, @@ -83,21 +120,47 @@ "type": "function", "function": { "name": "replace_text", - "description": "Replace exact unique text in existing UTF-8 files as one SHA-bound transaction. Edits to the same file run in declared order.", + "description": ( + "Replace exact unique text in existing UTF-8 files as one SHA-bound " + "transaction. Cannot create or delete files. Edits to the same file " + "run in declared order." + ), "parameters": { "type": "object", "properties": { "edits": { "type": "array", + "description": ( + "One to 100 exact replacements applied as one transaction." + ), "minItems": 1, "maxItems": 100, "items": { "type": "object", "properties": { - "path": {"type": "string"}, - "sha256": {"type": "string"}, - "old_text": {"type": "string"}, - "new_text": {"type": "string"}, + "path": { + "type": "string", + "description": "Workspace-relative path to an existing regular UTF-8 file.", + }, + "sha256": { + "type": "string", + "description": ( + "SHA-256 from read_file; all edits for one " + "file use the same starting hash." + ), + }, + "old_text": { + "type": "string", + "description": ( + "Exact non-empty Unicode text, including " + "whitespace and line breaks; must match exactly " + "once at this point in the ordered transaction." + ), + }, + "new_text": { + "type": "string", + "description": "Literal replacement text; may be empty.", + }, }, "required": [ "path", @@ -122,13 +185,30 @@ "parameters": { "type": "object", "properties": { - "argv": {"type": "array", "items": {"type": "string"}}, + "argv": { + "type": "array", + "description": "Executable and arguments as separate strings; no shell parsing.", + "items": {"type": "string"}, + }, "purpose": { "type": "string", "enum": ["inspect", "test", "build"], + "description": ( + "Command intent; only a successful test or build satisfies " + "the verification gate." + ), + }, + "cwd": { + "type": "string", + "description": "Workspace-relative working directory; default '.'.", + }, + "timeout_seconds": { + "type": "integer", + "description": ( + "Timeout from 1 to 1800 seconds; a timeout is returned as " + "a structured non-verification result." + ), }, - "cwd": {"type": "string"}, - "timeout_seconds": {"type": "integer"}, }, "required": ["argv", "purpose"], "additionalProperties": False, @@ -142,7 +222,12 @@ "description": "Submit the completed task. Rejected unless a patch exists and a relevant test/build passed after the latest patch.", "parameters": { "type": "object", - "properties": {"summary": {"type": "string"}}, + "properties": { + "summary": { + "type": "string", + "description": "Concise description of the completed change.", + } + }, "required": ["summary"], "additionalProperties": False, }, diff --git a/tests/agents/test_default.py b/tests/agents/test_default.py index 3320de0..94e37fa 100644 --- a/tests/agents/test_default.py +++ b/tests/agents/test_default.py @@ -272,12 +272,20 @@ def test_strategy_prompts_are_explicit_and_stable() -> None: assert "Use apply_patch for every workspace edit" in patch_prompt assert "follow the structured recovery instruction" in patch_prompt assert "Editing strategy: replace-first" in replace_prompt - assert "prefer replace_text with an exact" in replace_prompt + assert "Prefer replace_text when the change fits one localized" in replace_prompt + assert "multiple separated" in replace_prompt + assert "smallest exact old_text that matches once" in replace_prompt assert "Once the target and intended edit are clear" in replace_prompt assert "Do not repeat" in replace_prompt assert "Retry or switch tools" in replace_prompt assert replace_prompt == replace_planner.initial_messages("Fix it")[0]["content"] + task_message = replace_planner.initial_messages("Fix it")[1]["content"] + assert task_message.startswith("Task:\nFix it\n") + assert "Use only files and tests available in the workspace" in task_message + assert "Workspace: the tool root" not in task_message + assert "hidden tests" not in task_message + def test_agent_rejects_mismatched_strategy_components(tmp_path: Path) -> None: runner = ToolRunner(tmp_path, approver=CommandApprover("allow")) diff --git a/tests/tools/test_schemas.py b/tests/tools/test_schemas.py new file mode 100644 index 0000000..f010718 --- /dev/null +++ b/tests/tools/test_schemas.py @@ -0,0 +1,51 @@ +from __future__ import annotations + +from typing import Any + +from yada.tools.schemas import TOOL_SCHEMAS + + +def _functions_by_name() -> dict[str, dict[str, Any]]: + return {schema["function"]["name"]: schema["function"] for schema in TOOL_SCHEMAS} + + +def _assert_property_descriptions(schema: dict[str, Any]) -> None: + for property_schema in schema.get("properties", {}).values(): + assert property_schema.get("description") + items = property_schema.get("items") + if isinstance(items, dict): + _assert_property_descriptions(items) + + +def test_all_tool_parameters_describe_their_model_visible_semantics() -> None: + for function in _functions_by_name().values(): + _assert_property_descriptions(function["parameters"]) + + +def test_high_risk_tool_descriptions_expose_recovery_relevant_contracts() -> None: + functions = _functions_by_name() + + search = functions["search_code"] + assert "regular expression" in search["description"] + assert ( + "escape regex metacharacters" + in search["parameters"]["properties"]["query"]["description"] + ) + + patch = functions["apply_patch"] + assert "counts are recalculated automatically" in patch["description"] + assert "exactly every patch target" in patch["description"] + assert "no extra or missing paths" in patch["description"] + + replace = functions["replace_text"] + assert "Cannot create or delete files" in replace["description"] + old_text = replace["parameters"]["properties"]["edits"]["items"]["properties"][ + "old_text" + ]["description"] + assert "whitespace and line breaks" in old_text + assert "match exactly once" in old_text + + command = functions["run_command"]["parameters"]["properties"] + assert "no shell parsing" in command["argv"]["description"] + assert "verification gate" in command["purpose"]["description"] + assert "Workspace-relative" in command["cwd"]["description"]