Skip to content

Conversation

@RomneyDa
Copy link
Collaborator

@RomneyDa RomneyDa commented May 23, 2025

When edit tool is complete, read the new file contents and feed it back to the model. For now, just read the entire file. In the future, could send back diffs, ranges, etc but my hunch is that would confuse more than help the model.


Summary by cubic

The edit tool now reads the updated file contents after an edit and sends them back to the model for further processing.

  • New Features
    • Reads the entire edited file and returns its contents as context to the model.
    • Adds a helper to format and package the file output for the model.

@RomneyDa RomneyDa requested a review from a team as a code owner May 23, 2025 18:18
@RomneyDa RomneyDa requested review from sestinj and removed request for a team May 23, 2025 18:18
@netlify
Copy link

netlify bot commented May 23, 2025

Deploy Preview for continuedev failed. Why did it fail? →

Name Link
🔨 Latest commit 1ce32fd
🔍 Latest deploy log https://app.netlify.com/projects/continuedev/deploys/6830bbece2083b00083cb564

@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label May 23, 2025
@sestinj
Copy link
Contributor

sestinj commented May 26, 2025

@RomneyDa I feel like I'm just missing a bit of context on this, but doesn't the model already basically see the new file contents from the input to its tool call? Not full output, but it can probably piece this together

@RomneyDa
Copy link
Collaborator Author

RomneyDa commented May 26, 2025

@sestinj agreed that the model can somewhat infer new file contents based on changes. This is for case of only some or none of the diffs being accepted. Currently if you reject diffs Agent mode continues on as if the edits were all accepted. This is a heuristic solution that might be a bit token expensive but is a safe start and works well with smoke testing.

Reasoning for this approach came from convo with @Patrick-Erichsen, we landed on these reasons

  • We could add diff info to apply state and feed it back to the model, but it would be a bigger change and has some problems
    • Apply state doesn't contain diff info which is a non trivial add
    • even if it did the model wouldn't confidently be able to correlate it's change suggestions (lazy) to diffs (apply model)
    • users can edit diffs before accepting/rejecting, which complicates things

One improvement would be if all rejected don't send new file contents just say user rejected try something else.

Another improvement would be to only send the updated range, e.g. beginning of first diff to end of last diff or similar to save tokens on huge files.

Finally we could add some meta comments like of not all diffs were accepted add a cautionary: "here is the file after changes, note that the user did not accept all changes" or similar

Think any of these are important before merging or thoughts on different approaches?

@chezsmithy
Copy link
Contributor

This seems like a reasonable solution but you are right it will be token expensive if you make a number of small edits to a larger file? If the previous version of the file is in the chat history I wonder if there is some way to compress the chat history somehow after the edit is complete. Maybe store it like this?

History:
v1: full snapshot
v2: diff from v1
v3: diff from v2
v4: full snapshot (latest)

@sestinj
Copy link
Contributor

sestinj commented Jun 1, 2025

I think cost is actually a major concern here (enough so to avoid merging), and the point about small edits in large files I expect will become increasingly true with the direction of models using find/replace for edits

@chezsmithy
Copy link
Contributor

Could we solve for the full reject by updating the tool call noting the edit was cancelled as an early change while we solve for the bigger state issue of partial rejections?

@recurseml
Copy link

recurseml bot commented Jun 13, 2025

✨ No issues found! Your code is sparkling clean! ✨

@sestinj
Copy link
Contributor

sestinj commented Jun 16, 2025

@RomneyDa Not sure where you're at with this, but I'm going to close for hygiene's sake. Can be re-opened

@sestinj sestinj closed this Jun 16, 2025
@github-project-automation github-project-automation bot moved this from Todo to Done in Issues and PRs Jun 16, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Jun 16, 2025
@RomneyDa
Copy link
Collaborator Author

@chezsmithy retackling this today!

@sestinj sestinj deleted the dallin/edit-tool-results branch October 17, 2025 22:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

size:M This PR changes 30-99 lines, ignoring generated files.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants