Conversation
Implement a structured patch application tool that allows the LLM to add, delete, and update files in the codebase. - Implement `apply_patch` tool logic with support for `*** Add File`, `*** Delete File`, and `*** Update File` (including `*** Move to`) operations. - Add a robust seeking mechanism for updates using a fallback hierarchy: Exact Match $\rightarrow$ RStrip $\rightarrow$ Trim $\rightarrow$ Normalized Unicode. - Register the tool in `config.lua` and provide a comprehensive system prompt. - Add unit tests covering parsing, file creation, deletion, updates, and renames. - Include `apply_patch_plan.md` detailing the tool specification.
- Add type annotations for the `ApplyPatch` tool. - Validate normalized paths and move paths to prevent invalid file operations. - Reorganize imports to the top of the file for better consistency. - Enhance error messages when patch context is not found.
Extract hunk handling logic into dedicated helper functions (`handle_add`, `handle_delete`, and `handle_update`) to improve readability and maintainability. Update associated tests to align with the refactored structure and use more representative test cases.
Implement a check to ensure that patch hunks match exactly one location in the target file. If multiple matches are found for the old lines, an error is returned requesting more context to resolve the ambiguity. - Add `count_sequences` helper to detect multiple pattern occurrences. - Update `handle_update` to validate match uniqueness. - Add test case to verify failure on ambiguous matches.
| child.lua([=[ | ||
| local apply_patch = require('codecompanion.interactions.chat.tools.builtin.apply_patch') | ||
| -- Setup: create file with duplicate lines | ||
| vim.fn.mkdir('test_patch_dir', 'p') |
There was a problem hiding this comment.
Use vim.fn.tempname() to prevent the workspace getting polluted with test artifacts:
I typically do:
local tmpdir = child.lua("return vim.fn.tempname()")
child.fn.mkdir(tmpdir)
local f1 = vim.fs.joinpath(tmpdir, "one.txt")
local f2 = vim.fs.joinpath(tmpdir, "two.md")
child.fn.writefile({ "first file" }, f1)
child.fn.writefile({ "second file" }, f2)
| local file_utils = require("codecompanion.utils.files") | ||
| local fmt = string.format | ||
| local tool_helpers = require("codecompanion.interactions.chat.tools.builtin.helpers") | ||
|
|
There was a problem hiding this comment.
I recommend including a large comment header in this file to explain how the patching process works. Anyone coming to this cold will have to work hard to understand how it functions. Method names like seek_sequence aren't immediately clear as to why and what they're doing
| return s | ||
| end | ||
|
|
||
| local function try_match(lines, pattern, start_index, compare_fn, eof) |
There was a problem hiding this comment.
As this is a key function, I generally prefer parameters to be in an opts table. This makes it pretty trivial to update in the future. Something like:
try_match(opts)
-- Or
try_match(lines, opts)gives you a bit of versatility and matches what we try and do throughout CodeCompanion. Surprised Opencode didn't pick this up, as it is in AGENTS.md.
| if vim.fn.getfsize(path) == -1 then | ||
| return { status = "error", data = "File to delete does not exist: " .. path } | ||
| end | ||
| vim.fn.delete(path, "rf") |
There was a problem hiding this comment.
use file_utils.delete(path) in place of vim functions. They should be faster and utilize existing utilities.
olimorris
left a comment
olimorris
left a comment
There was a problem hiding this comment.
This looks really awesome. I'm going to test it out tonightand see how it performs with Anthropic, OpenAI, Copilot etc.
I've put some inline comments just afte a quick pass. My overarching feedback would be to ensure that we have LuaLS annotations and descriptions for each function/method. This is noted in AGENTS.md so looks like OpenCode missed it. Could probably do with more comments in apply_patch.lua as well, explaining why we're doing things.
You could also consider separating the file into tools/builtin/apply_patch/*.lua files if that would make it more managable.
|
Hello! Given that I had worked on the previous version of the edit tool, I thought I'd share a couple of observations that could prove useful during the validation process of the proposed algorithm. Some edge cases that need to be considered (off the top of my head and I haven't read the code):
Thanks |
3 participants
Description
AI Usage
opencode: gemma-4-31B-it
Related Issue(s)
N/A
Screenshots
N/A
Checklist
make allto ensure docs are generated, tests pass and StyLua has formatted the code