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

Edit: Handle conflicting diff decorations #6501

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
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
11 changes: 8 additions & 3 deletions vscode/src/autoedits/autoedits-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { ContextMixer } from '../completions/context/context-mixer'
import { DefaultContextStrategyFactory } from '../completions/context/context-strategy'
import { getCurrentDocContext } from '../completions/get-current-doc-context'

import type { FixupController } from '../non-stop/FixupController'
import type { AutoeditsModelAdapter, AutoeditsPrompt } from './adapters/base'
import { createAutoeditsModelAdapter } from './adapters/create-adapter'
import { getTimeNowInMillis } from './analytics-logger'
Expand Down Expand Up @@ -74,7 +75,7 @@ export class AutoeditsProvider implements vscode.InlineCompletionItemProvider, v
dataCollectionEnabled: false,
})

constructor(chatClient: ChatClient) {
constructor(chatClient: ChatClient, fixupController: FixupController) {
autoeditsOutputChannelLogger.logDebug('Constructor', 'Constructing AutoEditsProvider')
this.modelAdapter = createAutoeditsModelAdapter({
providerName: autoeditsProviderConfig.provider,
Expand All @@ -88,9 +89,13 @@ export class AutoeditsProvider implements vscode.InlineCompletionItemProvider, v

this.rendererManager =
enabledRenderer === 'inline'
? new AutoEditsInlineRendererManager(editor => new InlineDiffDecorator(editor))
? new AutoEditsInlineRendererManager(
editor => new InlineDiffDecorator(editor),
fixupController
)
: new AutoEditsDefaultRendererManager(
(editor: vscode.TextEditor) => new DefaultDecorator(editor)
(editor: vscode.TextEditor) => new DefaultDecorator(editor),
fixupController
)

this.onSelectionChangeDebounced = debounce(
Expand Down
29 changes: 28 additions & 1 deletion vscode/src/autoedits/renderer/manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import {
extractInlineCompletionFromRewrittenCode,
} from '../utils'

import type { FixupController } from '../../non-stop/FixupController'
import { CodyTaskState } from '../../non-stop/state'
import type { AutoEditsDecorator, DecorationInfo } from './decorators/base'

/**
Expand Down Expand Up @@ -92,7 +94,10 @@ export class AutoEditsDefaultRendererManager implements AutoEditsRendererManager
protected activeEdit: ProposedChange | null = null
protected disposables: vscode.Disposable[] = []

constructor(protected createDecorator: (editor: vscode.TextEditor) => AutoEditsDecorator) {
constructor(
protected createDecorator: (editor: vscode.TextEditor) => AutoEditsDecorator,
protected fixupController: FixupController
) {
this.disposables.push(
vscode.commands.registerCommand('cody.supersuggest.accept', () => this.acceptEdit()),
vscode.commands.registerCommand('cody.supersuggest.dismiss', () => this.dismissEdit()),
Expand Down Expand Up @@ -122,6 +127,9 @@ export class AutoEditsDefaultRendererManager implements AutoEditsRendererManager
if (!editor || document !== editor.document) {
return
}
if (this.hasConflictingDecorations(document, range)) {
return
}
this.activeEdit = {
uri: document.uri.toString(),
range: range,
Expand Down Expand Up @@ -250,6 +258,25 @@ export class AutoEditsDefaultRendererManager implements AutoEditsRendererManager
return { inlineCompletions: null, updatedDecorationInfo: decorationInfo }
}

private hasConflictingDecorations(document: vscode.TextDocument, range: vscode.Range): boolean {
const existingFixupFile = this.fixupController.maybeFileForUri(document.uri)
if (!existingFixupFile) {
// No Edits in this file, no conflicts
return false
}

const existingFixupTasks = this.fixupController.tasksForFile(existingFixupFile)
if (existingFixupTasks.length === 0) {
// No Edits in this file, no conflicts
return false
}

// Validate that the decoration position does not conflict with an existing Edit diff
return existingFixupTasks.some(
task => task.state === CodyTaskState.Applied && task.selectionRange.intersection(range)
)
}

public dispose(): void {
this.dismissEdit()
for (const disposable of this.disposables) {
Expand Down
28 changes: 27 additions & 1 deletion vscode/src/commands/GhostHintDecorator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import { telemetryRecorder } from '@sourcegraph/cody-shared'
import { type DebouncedFunc, throttle } from 'lodash'
import * as vscode from 'vscode'
import type { SyntaxNode } from 'web-tree-sitter'
import type { FixupController } from '../non-stop/FixupController'
import { CodyTaskState } from '../non-stop/state'
import { execQueryWrapper } from '../tree-sitter/query-sdk'

const EDIT_SHORTCUT_LABEL = isMacOS() ? 'Opt+K' : 'Alt+K'
Expand Down Expand Up @@ -176,7 +178,7 @@ export class GhostHintDecorator implements vscode.Disposable {
/** Store the last line that the user typed on, we want to avoid showing the text here */
private lastLineTyped: number | null = null

constructor() {
constructor(private options: { fixupController: FixupController }) {
this.setThrottledGhostText = throttle(this.setGhostText.bind(this), GHOST_TEXT_THROTTLE, {
leading: false,
trailing: true,
Expand Down Expand Up @@ -358,6 +360,11 @@ export class GhostHintDecorator implements vscode.Disposable {
return
}

if (this.hasConflictingDecorations(editor, position)) {
// Decoration will conflict, so do nothing
return
}

this.fireThrottledDisplayEvent(variant)

const decorationHint = HINT_DECORATIONS[variant]
Expand All @@ -372,6 +379,25 @@ export class GhostHintDecorator implements vscode.Disposable {
])
}

private hasConflictingDecorations(editor: vscode.TextEditor, position: vscode.Position): boolean {
const existingFixupFile = this.options.fixupController.maybeFileForUri(editor.document.uri)
if (!existingFixupFile) {
// No Edits in this file, no conflicts
return false
}

const existingFixupTasks = this.options.fixupController.tasksForFile(existingFixupFile)
if (existingFixupTasks.length === 0) {
// No Edits in this file, no conflicts
return false
}

// Validate that the decoration position does not conflict with an existing Edit diff
return existingFixupTasks.some(
task => task.state === CodyTaskState.Applied && task.selectionRange.contains(position)
)
}

private getDocumentableSymbol(
document: vscode.TextDocument,
position: vscode.Position
Expand Down
21 changes: 10 additions & 11 deletions vscode/src/edit/manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
import type { GhostHintDecorator } from '../commands/GhostHintDecorator'
import { getEditor } from '../editor/active-editor'
import type { VSCodeEditor } from '../editor/vscode-editor'
import { FixupController } from '../non-stop/FixupController'
import type { FixupController } from '../non-stop/FixupController'
import type { FixupTask } from '../non-stop/FixupTask'

import { context } from '@opentelemetry/api'
Expand All @@ -38,18 +38,17 @@ export interface EditManagerOptions {
chat: ChatClient
ghostHintDecorator: GhostHintDecorator
extensionClient: ExtensionClient
controller: FixupController
}

// EditManager handles translating specific edit intents (document, edit) into
// generic FixupTasks, and pairs a FixupTask with an EditProvider to generate
// a completion.
export class EditManager implements vscode.Disposable {
private readonly controller: FixupController
private disposables: vscode.Disposable[] = []
private editProviders = new WeakMap<FixupTask, EditProvider>()

constructor(public options: EditManagerOptions) {
this.controller = new FixupController(options.extensionClient)
/**
* Entry point to triggering a new Edit.
* Given a set or arguments, this will create a new LLM interaction
Expand Down Expand Up @@ -86,7 +85,7 @@ export class EditManager implements vscode.Disposable {
provider.startEdit()
}
)
this.disposables.push(this.controller, editCommand, smartApplyCommand, startCommand)
this.disposables.push(this.options.controller, editCommand, smartApplyCommand, startCommand)
}

public async executeEdit(args: ExecuteEditArguments = {}): Promise<FixupTask | undefined> {
Expand Down Expand Up @@ -148,7 +147,7 @@ export class EditManager implements vscode.Disposable {

let task: FixupTask | null
if (configuration.instruction && configuration.instruction.trim().length > 0) {
task = await this.controller.createTask(
task = await this.options.controller.createTask(
document,
configuration.instruction,
configuration.userContextFiles ?? [],
Expand All @@ -163,7 +162,7 @@ export class EditManager implements vscode.Disposable {
configuration.id
)
} else {
task = await this.controller.promptUserForTask(
task = await this.options.controller.promptUserForTask(
configuration.preInstruction,
document,
range,
Expand All @@ -184,7 +183,7 @@ export class EditManager implements vscode.Disposable {
* Checks if there is already an active task for the given fixup file
* that has the same instruction and selection range as the current task.
*/
const activeTask = this.controller.tasksForFile(task.fixupFile).find(activeTask => {
const activeTask = this.options.controller.tasksForFile(task.fixupFile).find(activeTask => {
return (
ACTIVE_TASK_STATES.includes(activeTask.state) &&
activeTask.instruction.toString() === task.instruction.toString() &&
Expand All @@ -193,7 +192,7 @@ export class EditManager implements vscode.Disposable {
})

if (activeTask) {
this.controller.cancel(task)
this.options.controller.cancel(task)
return
}

Expand Down Expand Up @@ -273,7 +272,7 @@ export class EditManager implements vscode.Disposable {
if (args.configuration?.isNewFile) {
// We are creating a new file, this means we are only _adding_ new code and _inserting_ it into the document.
// We do not need to re-prompt the LLM for this, let's just add the code directly.
const task = await this.controller.createTask(
const task = await this.options.controller.createTask(
document,
configuration.instruction,
[],
Expand Down Expand Up @@ -384,7 +383,7 @@ export class EditManager implements vscode.Disposable {
// We determined a selection, but it was empty. This means that we will be _adding_ new code
// and _inserting_ it into the document. We do not need to re-prompt the LLM for this, let's just
// add the code directly.
const task = await this.controller.createTask(
const task = await this.options.controller.createTask(
document,
configuration.instruction,
[],
Expand Down Expand Up @@ -446,7 +445,7 @@ export class EditManager implements vscode.Disposable {
let provider = this.editProviders.get(task)

if (!provider) {
provider = new EditProvider({ task, controller: this.controller, ...this.options })
provider = new EditProvider({ task, ...this.options })
this.editProviders.set(task, provider)
}

Expand Down
37 changes: 23 additions & 14 deletions vscode/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ import { isRunningInsideAgent } from './jsonrpc/isRunningInsideAgent'
import type { SymfRunner } from './local-context/symf'
import { MinionOrchestrator } from './minion/MinionOrchestrator'
import { PoorMansBash } from './minion/environment'
import { FixupController } from './non-stop/FixupController'
import { CodyProExpirationNotifications } from './notifications/cody-pro-expiration'
import { showSetupNotification } from './notifications/setup-notification'
import { logDebug, logError } from './output-channel-logger'
Expand Down Expand Up @@ -262,9 +263,20 @@ const register = async (
},
disposables
)
disposables.push(chatsController)
const fixupController = new FixupController(platform.extensionClient)
const ghostHintDecorator = new GhostHintDecorator({ fixupController })
const editManager = new EditManager({
controller: fixupController,
chat: chatClient,
editor,
ghostHintDecorator,
extensionClient: platform.extensionClient,
})

disposables.push(
chatsController,
ghostHintDecorator,
editManager,
subscriptionDisposable(
exposeOpenCtxClient(context, platform.createOpenCtxController).subscribe({})
)
Expand All @@ -284,7 +296,7 @@ const register = async (
registerAutocomplete(platform, statusBar, disposables)
const tutorialSetup = tryRegisterTutorial(context, disposables)

await registerCodyCommands(statusBar, chatClient, disposables)
await registerCodyCommands(statusBar, chatClient, fixupController, disposables)
registerAuthCommands(disposables)
registerChatCommands(disposables)
disposables.push(...registerSidebarCommands())
Expand Down Expand Up @@ -410,6 +422,7 @@ async function registerOtherCommands(disposables: vscode.Disposable[]) {
async function registerCodyCommands(
statusBar: CodyStatusBar,
chatClient: ChatClient,
fixupController: FixupController,
disposables: vscode.Disposable[]
): Promise<void> {
// Execute Cody Commands and Cody Custom Commands
Expand Down Expand Up @@ -455,7 +468,7 @@ async function registerCodyCommands(
)

// Initialize autoedit provider if experimental feature is enabled
registerAutoEdits(chatClient, disposables)
registerAutoEdits(chatClient, fixupController, disposables)

// Initialize autoedit tester
disposables.push(
Expand Down Expand Up @@ -701,7 +714,11 @@ async function tryRegisterTutorial(
}
}

function registerAutoEdits(chatClient: ChatClient, disposables: vscode.Disposable[]): void {
function registerAutoEdits(
chatClient: ChatClient,
fixupController: FixupController,
disposables: vscode.Disposable[]
): void {
disposables.push(
subscriptionDisposable(
combineLatest(
Expand All @@ -714,7 +731,7 @@ function registerAutoEdits(chatClient: ChatClient, disposables: vscode.Disposabl
.pipe(
map(([config, authStatus, autoeditEnabled]) => {
if (shouldEnableExperimentalAutoedits(config, autoeditEnabled, authStatus)) {
const provider = new AutoeditsProvider(chatClient)
const provider = new AutoeditsProvider(chatClient, fixupController)
const completionRegistration =
vscode.languages.registerInlineCompletionItemProvider(
[{ scheme: 'file', language: '*' }, { notebookType: '*' }],
Expand Down Expand Up @@ -896,16 +913,8 @@ function registerChat(
platform.extensionClient
)
chatsController.registerViewsAndCommands()

const ghostHintDecorator = new GhostHintDecorator()
const editorManager = new EditManager({
chat: chatClient,
editor,
ghostHintDecorator,
extensionClient: platform.extensionClient,
})
const promptsManager = new PromptsManager({ chatsController })
disposables.push(ghostHintDecorator, editorManager, new CodeActionProvider(), promptsManager)
disposables.push(new CodeActionProvider(), promptsManager)

// Register a serializer for reviving the chat panel on reload
if (vscode.window.registerWebviewPanelSerializer) {
Expand Down
Loading