Skip to content
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

Improve behavior of AI Changeset diff editor #14786

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion dependency-check-baseline.json
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
{
"npm/npmjs/-/unicode-property-aliases-ecmascript/2.1.0": "Manually approved"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change seems unrelated

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's not needed anymore

}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
} from '@theia/ai-chat';
import { ChangeSetFileElementFactory } from '@theia/ai-chat/lib/browser/change-set-file-element';
import { Agent, PromptTemplate } from '@theia/ai-core';
import { URI } from '@theia/core';
import { inject, injectable, interfaces } from '@theia/core/shared/inversify';
import { FileService } from '@theia/filesystem/lib/browser/file-service';
import { WorkspaceService } from '@theia/workspace/lib/browser';
Expand Down Expand Up @@ -94,13 +95,14 @@ export class ChangeSetChatAgent extends AbstractStreamParsingChatAgent implement
chatSessionId
})
);

if (fileToChange && fileToChange.resource) {
changeSet.addElement(
this.fileChangeFactory({
uri: fileToChange.resource,
type: 'modify',
state: 'pending',
targetState: 'Hello World Modify!',
targetState: await this.computeTargetState(fileToChange.resource),
changeSet,
chatSessionId
})
Expand All @@ -124,6 +126,30 @@ export class ChangeSetChatAgent extends AbstractStreamParsingChatAgent implement
));
request.response.complete();
}
async computeTargetState(resource: URI): Promise<string> {
const content = await this.fileService.read(resource);
if (content.value.length < 20) {
return 'HelloWorldModify';
}
let readLocation = Math.random() * 0.1 * content.value.length;
let oldLocation = 0;
let output = '';
while (readLocation < content.value.length) {
output += content.value.substring(oldLocation, readLocation);
oldLocation = readLocation;
const type = Math.random();
if (type < 0.33) {
// insert
output += `this is an insert at ${readLocation}`;
} else {
// delete
oldLocation += 20;
}

readLocation += Math.random() * 0.1 * content.value.length;
}
return output;
}

protected override async getSystemMessageDescription(): Promise<SystemMessageDescription | undefined> {
return undefined;
Expand Down
1 change: 1 addition & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 8 additions & 4 deletions packages/ai-chat/src/browser/change-set-file-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ export class ChangeSetFileElement implements ChangeSetElement {
return this.elementProps.uri;
}

get changedUri(): URI {
return createChangeSetFileUri(this.elementProps.chatSessionId, this.uri);
}

get name(): string {
return this.elementProps.name ?? this.changeSetFileService.getName(this.uri);
}
Expand Down Expand Up @@ -98,25 +102,25 @@ export class ChangeSetFileElement implements ChangeSetElement {
}

async open(): Promise<void> {
this.changeSetFileService.open(this.uri, this.targetState);
this.changeSetFileService.open(this);
}

async openChange(): Promise<void> {
this.changeSetFileService.openDiff(
this.uri,
createChangeSetFileUri(this.elementProps.chatSessionId, this.uri)
this.changedUri
);
}

async accept(): Promise<void> {
async accept(contents?: string): Promise<void> {
this.state = 'applied';
if (this.type === 'delete') {
await this.changeSetFileService.delete(this.uri);
this.state = 'applied';
return;
}

await this.changeSetFileService.write(this.uri, this.targetState);
await this.changeSetFileService.write(this.uri, contents !== undefined ? contents : this.targetState);
}

async discard(): Promise<void> {
Expand Down
12 changes: 8 additions & 4 deletions packages/ai-chat/src/browser/change-set-file-resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
// SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0
// *****************************************************************************

import { Resource, ResourceResolver, URI } from '@theia/core';
import { Resource, ResourceResolver, ResourceSaveOptions, URI } from '@theia/core';
import { inject, injectable } from '@theia/core/shared/inversify';
import { ChatService } from '../common';
import { ChangeSetFileElement } from './change-set-file-element';
Expand All @@ -23,7 +23,7 @@ export const CHANGE_SET_FILE_RESOURCE_SCHEME = 'changeset-file';
const QUERY = 'uri=';

export function createChangeSetFileUri(chatSessionId: string, elementUri: URI): URI {
return new URI(CHANGE_SET_FILE_RESOURCE_SCHEME + ':/' + chatSessionId).withQuery(QUERY + encodeURIComponent(elementUri.path.toString()));
return new URI(CHANGE_SET_FILE_RESOURCE_SCHEME + '://' + chatSessionId + '/' + elementUri.path).withQuery(QUERY + encodeURIComponent(elementUri.path.toString()));
}

/**
Expand All @@ -41,7 +41,7 @@ export class ChangeSetFileResourceResolver implements ResourceResolver {
throw new Error('The given uri is not a change set file uri: ' + uri);
}

const chatSessionId = uri.path.name;
const chatSessionId = uri.authority;
const session = this.chatService.getSession(chatSessionId);
if (!session) {
throw new Error('Chat session not found: ' + chatSessionId);
Expand All @@ -60,8 +60,12 @@ export class ChangeSetFileResourceResolver implements ResourceResolver {

return {
uri,
readOnly: true,
readOnly: false,
initiallyDirty: true,
readContents: async () => element.targetState ?? '',
saveContents: async (content: string, options?: ResourceSaveOptions): Promise<void> => {
element.accept(content);
},
dispose: () => { }
};
}
Expand Down
26 changes: 7 additions & 19 deletions packages/ai-chat/src/browser/change-set-file-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { EditorManager } from '@theia/editor/lib/browser';
import { FileService } from '@theia/filesystem/lib/browser/file-service';
import { MonacoWorkspace } from '@theia/monaco/lib/browser/monaco-workspace';
import { WorkspaceService } from '@theia/workspace/lib/browser/workspace-service';
import { ChangeSetFileElement } from './change-set-file-element';

@injectable()
export class ChangeSetFileService {
Expand Down Expand Up @@ -82,28 +83,15 @@ export class ChangeSetFileService {
return this.labelProvider.getLongName(uri.parent);
}

async open(uri: URI, targetState: string): Promise<void> {
const exists = await this.fileService.exists(uri);
async open(element: ChangeSetFileElement): Promise<void> {
const exists = await this.fileService.exists(element.uri);
if (exists) {
open(this.openerService, uri);
await open(this.openerService, element.uri);
return;
}
const editor = await this.editorManager.open(uri.withScheme(UNTITLED_SCHEME), {
await this.editorManager.open(element.changedUri, {
mode: 'reveal'
});
editor.editor.executeEdits([{
newText: targetState,
range: {
start: {
character: 1,
line: 1,
},
end: {
character: 1,
line: 1,
},
}
}]);
}

async openDiff(originalUri: URI, suggestedUri: URI): Promise<void> {
Expand All @@ -112,7 +100,7 @@ export class ChangeSetFileService {
// Currently we don't have a great way to show the suggestions in a diff editor with accept/reject buttons
// So we just use plain diffs with the suggestions as original and the current state as modified, so users can apply changes in their current state
// But this leads to wrong colors and wrong label (revert change instead of accept change)
const diffUri = DiffUris.encode(suggestedUri, openedUri,
const diffUri = DiffUris.encode(openedUri, suggestedUri,
`AI Changes: ${this.labelProvider.getName(originalUri)}`,
);
open(this.openerService, diffUri);
Expand All @@ -139,7 +127,7 @@ export class ChangeSetFileService {
await this.monacoWorkspace.applyBackgroundEdit(document, [{
range: document.textEditorModel.getFullModelRange(),
text
}], true);
}], (editor, wasDirty) => editor === undefined || !wasDirty);
} else {
await this.fileService.write(uri, text);
}
Expand Down
3 changes: 3 additions & 0 deletions packages/core/src/browser/saveable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -392,4 +392,7 @@ export class ShouldSaveDialog extends AbstractDialog<boolean> {
return this.shouldSave;
}

override async open(disposeOnResolve?: boolean): Promise<boolean | undefined> {
return super.open(disposeOnResolve);
}
}
4 changes: 4 additions & 0 deletions packages/core/src/common/resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ export interface Resource extends Disposable {
readonly onDidChangeReadOnly?: Event<boolean | MarkdownString>;

readonly readOnly?: boolean | MarkdownString;

readonly initiallyDirty?: boolean;
/**
* Reads latest content of this resource.
*
Expand Down Expand Up @@ -378,11 +380,13 @@ export class UntitledResourceResolver implements ResourceResolver {
export class UntitledResource implements Resource {

protected readonly onDidChangeContentsEmitter = new Emitter<void>();
initiallyDirty: boolean;
get onDidChangeContents(): Event<void> {
return this.onDidChangeContentsEmitter.event;
}

constructor(private resources: Map<string, UntitledResource>, public uri: URI, private content?: string) {
this.initiallyDirty = (content !== undefined && content.length > 0);
this.resources.set(this.uri.toString(), this);
}

Expand Down
6 changes: 3 additions & 3 deletions packages/monaco/src/browser/monaco-editor-model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { TextEditorDocument, EncodingMode, FindMatchesOptions, FindMatch, Editor
import { DisposableCollection, Disposable } from '@theia/core/lib/common/disposable';
import { Emitter, Event } from '@theia/core/lib/common/event';
import { CancellationTokenSource, CancellationToken } from '@theia/core/lib/common/cancellation';
import { Resource, ResourceError, ResourceVersion, UNTITLED_SCHEME } from '@theia/core/lib/common/resource';
import { Resource, ResourceError, ResourceVersion } from '@theia/core/lib/common/resource';
import { Saveable, SaveOptions } from '@theia/core/lib/browser/saveable';
import { MonacoToProtocolConverter } from './monaco-to-protocol-converter';
import { ProtocolToMonacoConverter } from './protocol-to-monaco-converter';
Expand Down Expand Up @@ -202,7 +202,7 @@ export class MonacoEditorModel implements IResolvedTextEditorModel, TextEditorDo
const languageSelection = StandaloneServices.get(ILanguageService).createByFilepathOrFirstLine(uri, firstLine);
this.model = StandaloneServices.get(IModelService).createModel(value, languageSelection, uri);
this.resourceVersion = this.resource.version;
this.setDirty(this._dirty || (this.resource.uri.scheme === UNTITLED_SCHEME && this.model.getValueLength() > 0));
this.setDirty(this._dirty || (!!this.resource.initiallyDirty));
this.updateSavedVersionId();
this.toDispose.push(this.model);
this.toDispose.push(this.model.onDidChangeContent(event => this.fireDidChangeContent(event)));
Expand Down Expand Up @@ -553,7 +553,7 @@ export class MonacoEditorModel implements IResolvedTextEditorModel, TextEditorDo
}

const changes = [...this.contentChanges];
if (changes.length === 0 && !overwriteEncoding && reason !== TextDocumentSaveReason.Manual) {
if ((changes.length === 0 && !this.resource.initiallyDirty) && !overwriteEncoding && reason !== TextDocumentSaveReason.Manual) {
return;
}

Expand Down
6 changes: 4 additions & 2 deletions packages/monaco/src/browser/monaco-workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,14 +221,16 @@ export class MonacoWorkspace {
* Applies given edits to the given model.
* The model is saved if no editors is opened for it.
*/
applyBackgroundEdit(model: MonacoEditorModel, editOperations: monaco.editor.IIdentifiedSingleEditOperation[], shouldSave = true): Promise<void> {
applyBackgroundEdit(model: MonacoEditorModel, editOperations: monaco.editor.IIdentifiedSingleEditOperation[],
shouldSave?: boolean | ((openEditor: MonacoEditor | undefined, wasDirty: boolean) => boolean)): Promise<void> {
return this.suppressOpenIfDirty(model, async () => {
const editor = MonacoEditor.findByDocument(this.editorManager, model)[0];
const wasDirty = !!editor?.document.dirty;
const cursorState = editor && editor.getControl().getSelections() || [];
model.textEditorModel.pushStackElement();
model.textEditorModel.pushEditOperations(cursorState, editOperations, () => cursorState);
model.textEditorModel.pushStackElement();
if (!editor && shouldSave) {
if ((typeof shouldSave === 'function' && shouldSave(editor, wasDirty)) || (!editor && shouldSave)) {
await model.save();
}
});
Expand Down
Loading