-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for change sets via tool functions #14715
Conversation
fixed #14678 Signed-off-by: Jonas Helming <jhelming@eclipsesource.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty powerful and nicely implemented, thank you! While I didn't test this in detail --- just a few smaller changes --- it actually performed pretty well.
I've added a few suggestions below, but they are all very minor things.
packages/ai-workspace-agent/src/browser/content-change-applier-service.ts
Outdated
Show resolved
Hide resolved
packages/ai-workspace-agent/src/browser/content-change-applier-service.ts
Show resolved
Hide resolved
packages/ai-workspace-agent/src/browser/file-changeset-service.ts
Outdated
Show resolved
Hide resolved
packages/ai-workspace-agent/src/common/coder-replace-template.ts
Outdated
Show resolved
Hide resolved
packages/ai-workspace-agent/src/browser/replace-changeset-functions.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found the file name replace-changeset-functions.ts
confusing as it reads like we replace a change set.
While I understand where the replace
comes from in this name, I think it isn't so relevant. How about naming this file write-change-to-file-provider.ts
(see also my comment below about the name WriteToFileChangeProvider
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went for 'replace-content-changeset-functions'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think for now I'd move the content of this file into replace-changeset-functions.ts
as it is only used here and rather local. Not sure if it is intended to be used outside of the scope of the tool function WriteToFileChangeProvider
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think both files will grow a bit, at least with a "search and replace" function. Therefore and in the spirit of keeping files small to work with this replace provider, I would keep it separate :-)
…-service.ts Co-authored-by: Philip Langer <planger@eclipsesource.com>
Co-authored-by: Philip Langer <planger@eclipsesource.com>
fixed #14678
What it does
Introduce a set of tool functions, which allow agents to create file based change sets. Change sets collect changes, an agent wants to conduct, the user can review them and then decide to apply them.
ChangeSets are managed in a service that supports generic operations to apply the changes. This allows to implement various strategies, e.g. replace content or to work with diffs. To support a new strategy, you need to:
The current PR contains a simple strategy to replace files completely.
The PR introduces also a new agent 'coder' that uses the new tool functions.
The PR renames the workspace functions and template to 'workspace-'
How to test
Ask something like this in the chat:
@Coder I want to add a property "autostart" to the mcp server preferences (packages/ai-mcp/src/browser/mcp-preferences.ts).
Then ask to apply the changes.
Follow-ups
Breaking changes
Attribution
Review checklist
Reminder for reviewers