[Theia AI] Coder and ChangeSet Epic

[JonasHelming]

Prioritized List:

• Improve Changeset Diff Editor Experience #14785
• [Theia AI] Coder switched to attempt tool calls in text like this "Tool Call:" #14905
• [Theia AI] Speed up the generation of changes of coder agent #14773
• [Theia AI] Show diff on click in ChangeSets #14783
• [Theia AI] Override existing change elements per file #14791
• Feature - Chat Agent Pinning #14716
• [Theia AI] Refine Coder prompt #14886
• Review context (feat(ai): Enable Context Variables for Chat Requests #14787 )
• [Theia AI] Make search-replace prompt the default of Coder #14869
• Enable change navigation in diff editors #14888
• [Theia AI] Diff Opening Slow #14916
• [Theia AI] Diff editor does not update to new changes #14900
• [Theia AI] Close diff editors of accepted or cleared changes | they are still open in dirty state which is very confusing Improve AI diff UX #14910
• [Theia AI] Diff editor asks for file location for new files | When saving a diff editor for a new file, it should not open the “save” file system dialog. And even if it does, then not on the root system location Improve AI diff UX #14910
• [Theia AI] Diff editor behaves weird with auto save on | changes are directly applied Improve AI diff UX #14910
• [Theia AI] Run ScanOSS on changeset #14915
• [Theia AI] Integrate context feature with Coder #14895
• Use unsaved state of files in workspace functions #14884
• [Theia AI] Add function to validate a file | open it in an editor and send all entries from the problem view as a function result

[colin-grant-work]

[Theia AI] Check diff approach | Are we uing the right diff editor? It does not allow typical features such as navigating between the different changes

This is true of diff editors opened by e.g. the Git extension for changes to the working tree, as well. We likely need to implement the relevant commands for all diff editors, and we'll get them on AI diff editors for free.

[colin-grant-work]

[Theia AI] Diff editor does not update to new changes | When a diff editor is still open for a file and a new change for the same file was generated, then clicking on the change set diff will just focus the editor, although it does not contain the changes. I need to close the old editor and open it again via the new changeset to see the diff.

The basic problem here is that the resources we create for changes don't have a mechanism to inform holders of the resource of changes, so the editor never learns that anything has changed.

We can:

• Try to pipe the change set's change event through to the corresponding resource. This would be the cleanest, but it doesn't really jive with the lifecycle currently in place. Currently, the content of the resource is tied to the ChangeSetElement, but when the content changes, we actually replace the ChangeSetElement in its host ChangeSet rather than updating it. That means we could:
  • Look the ChangeSetElement up dynamically at read time, rather than fixing it when we create the resource.
  • Change the lifecycle of ChangeSetElements so that they can be updated, rather than replaced.

@planger, it looks like you did some of the implementation work here, so I'm curious what your thoughts are.

• Add a change event to the resource and attempt to cache the resource so that a change set / owner of a change set could trigger a change event on the resource held by the editor. Cons: In most cases (including the change set resource resolver), resource resolvers return new resources every time a given URI is resolved, so it isn't easy to make sure that we have the same resource as the editor refers to without implementing some kind of cache whose lifecycle we'd then be responsible for.

• When we believe that the content of a given URI may have changed, we can go through the Monaco service to get the text model and update its content. Cons: treats Monaco as the source of truth, rather than the originating chat service.

But there is a subsidiary problem: since we treat the editor as dirty, firing a change on the resource doesn't actually cause the changes to be reflected in the editor, because Monaco believes that its state is fresher than the state 'on disk.'

That means that we can:

• Say that the resource is not initially dirty.
• Use the Monaco approach above. But this will discard any changes that the user has made to the 'dirty' version in the meantime. Maybe that's ok if the user has gone back to the chat anyway.
• Make a more thorough rewrite and adopt the approach used by the Cline extension, where they basically
  • Use a custom URI for the left side diff that just reflects the content of the file at the time the change set was opened.
  • Update the content of the actual file that would be created, and then do some work to revert if the user rejects the changes.
  • (More optionally, in my view) do some work to try to avoid 'distractions' like other (writable) tabs with the same file in view.

@tsmaeder, it looks like you did some of the work on the current implementation with a custom URI on the right side of the diff, so I'd be curious about your thoughts here.

[colin-grant-work]

[Theia AI] Close diff editors of accepted or cleared changes | they are still open in dirty state which is very confusing

[Theia AI] Diff editor asks for file location for new files | When saving a diff editor for a new file, it should not open the “save” file system dialog. And even if it does, then not on the root system location

[Theia AI] Diff editor behaves weird with auto save on | changes are directly applied

Fundamentally, what's the UX we want to get to re: the pending changes. The flow chart I see looks like something like:
Agent proposes changes, and we display them. Then...

• User can open individual files
  • Interact and save --> counts as acceptance of state of editor at the time. Should remove that file from proposed changes(?)
  • (Interact and) close without saving --> neutral, no action re: change set node.
• User can click accept on node --> If file not open, enact suggestion. If changes open, is that accepting the current state or the original suggestion?
• User can click reject on node --> If file not open, just remove suggestion. If changes open and dirty --> revert to state before suggestions? Keep dirty editor available in case the user meant that they didn't want the suggestions as proposed, but maybe they've made progress in the dirty editor.
• User can click Accept on change set representation --> delegate to individual acceptance actions.
• User leaves suggestions around for a long time while they work on the relevant files in other ways (maybe even other chats) --> how to handle staleness if they come back?

In any case, it seems clear that autosaving in this context shouldn't count as 'normal' saving: we shouldn't count that as the user having actually accepted the changes, however acceptance is handled in our case. This is one argument against our current dirty default.

[JonasHelming]

[Theia AI] Close diff editors of accepted or cleared changes | they are still open in dirty state which is very confusing
[Theia AI] Diff editor asks for file location for new files | When saving a diff editor for a new file, it should not open the “save” file system dialog. And even if it does, then not on the root system location
[Theia AI] Diff editor behaves weird with auto save on | changes are directly applied

Fundamentally, what's the UX we want to get to re: the pending changes. The flow chart I see looks like something like: Agent proposes changes, and we display them. Then...

• User can open individual files

  • Interact and save --> counts as acceptance of state of editor at the time. Should remove that file from proposed changes(?)
    => Set them to applied. Whether we remove them should be handled in the changeset I believe. There are currently different opinions about this, so we might introduce a user setting or make the changeset view filterable.
  • (Interact and) close without saving --> neutral, no action re: change set node.
    => yes, no action

• User can click accept on node --> If file not open, enact suggestion
  => yes.
  If changes open, is that accepting the current state or the original suggestion?
  => Tricky, what's your take?

• User can click reject on node --> If file not open, just remove suggestion.
  => yes
  If changes open and dirty --> revert to state before suggestions? Keep dirty editor available in case the user meant that they didn't want the suggestions as proposed, but maybe they've made progress in the dirty editor.
  Hmm, ideally, we would check whether the user has changed anything and in this case ask, otherwise close. If this is very complicated, it could be a follow up, then I would just close, WDYT?

• User can click Accept on change set representation --> delegate to individual acceptance actions.
  yes

• User leaves suggestions around for a long time while they work on the relevant files in other ways (maybe even other chats) --> how to handle staleness if they come back?
  How this behave if you do them same with regular diff editors?

In any case, it seems clear that autosaving in this context shouldn't count as 'normal' saving: we shouldn't count that as the user having actually accepted the changes, however acceptance is handled in our case. This is one argument against our current dirty default.
maybe just turn off auto save for this editor?

[planger]

@colin-grant-work Thanks for looking into this!

The basic problem here is that the resources we create for changes don't have a mechanism to inform holders of the resource of changes, so the editor never learns that anything has changed.

@planger, it looks like you did some of the implementation work here, so I'm curious what your thoughts are.

Please note that I haven't explored the implementation details of these approaches in depth, so please consider my comments as unconfirmed thoughts. :-)

I second your statement that the cleanest approach would be to pipe the change set's change events through the corresponding resource. I think we can return a MutableResource from the resource provider, which seems to support change notifications. I'm not exactly sure though in which direction and how this would behave in the diff editor, but this looks promising.

It is true that we currently replace the change set elements instead of updating them. However, I think we should use the stability of the URIs (chat-session-id/uri). To Theia and the user, this is what counts, not particularly the objects representing them. So if the agent or tool function replaces the change set elements with new elements that have the same URI, internally we actually may want to rather represent that as an update instead of a replace, to ensure a consistent life cycle of a change suggestion for a file, identified by chat-session-id/uri.

This leaves us with three states of a resource:

1. Normal file URI: Current state (on disk or Monaco workspace)
2. Chat-session-file-URI: Suggested version of the file
3. In-memory state of Chat-session-file-URI (2) in the diff-editor, which may include user edits

The suggestion-diff-editor shows a diff between 1 and 3. The AI only may see 1 and 2. So the tricky situation is how to handle a scenario where AI suggests a new version of 2, not being aware of 3. (Alternatively, we may think about considering 3 the actual new state of 1, when the AI requests the current (in-memory) state of 1 -- because this is also somewhat how we represent it to the user. But let's leave this aspect what the AI sees out of the picture for now.)

With the MutableResource we may be able to notify the suggestion-diff-editor that state 2 has changed and either:

1. Replace the state 3 with the new state 2 -- but this would throw away potential user edits
2. Try to apply the suggested changes on top of state 3, by computing edits from the diff between 1 and 2, but I feel this might be a bit over the top.

I think in the first step 1 might be good enough, as long as we do it in the command stack, so users can undo.

What do you think?

If changes open, is that accepting the current state or the original suggestion?
=> Tricky, what's your take?

Related to what I wrote above, my feeling is that this should reset the state of the editor to the AI suggestion within the command stack. Users can still undo if they like, but if they go to the chat and then click the button there, I'd expect it to reset the state to its suggestion as is (to be symmetric to the discard action). But this is tricky indeed :-)

[tsmaeder]

Sorry for the late reply, I had thought you had invoked @planger, not me. Anyway a couple of thoughts:

1. If we have multiple changesets active at the same time, be it in the same or different chats, it will be difficult to find a UI that is convincing with a single diff editor for the same target workspace resource.
2. As for applying the changes in the changeset. Maybe we need to think more like a "staging area" instead of applying the changes into a workspace resource directly. That way, the user can have multiple editors for the same target resource in the workspace open. When the user applies a change to the workspace (either explicitly or by saving the changeset-specific target resource), all other active changesets are updated to show the diff to the now current workspace resource. Maybe we can even rerun the prompts to create new, better changes.