chore(audoedit): decouple codeToReplaceData from getPromptForModelType
#6474
Conversation
| @@ -155,6 +156,13 @@ export class AutoeditsProvider implements vscode.InlineCompletionItemProvider, v | |||
| maxSuffixLength: tokensToChars(autoeditsProviderConfig.tokenLimit.suffixTokens), | |||
| }) | |||
|
|
|||
| const { fileWithMarkerPrompt, areaPrompt, codeToReplaceData } = getCurrentFilePromptComponents({ | |||
There was a problem hiding this comment.
Why do we need this decoupling, do autoedits-provider need any additional fields from the codeToReplaceData?
I think fileWithMarkerPrompt and areaPrompt are the prompt components which are used to construct the final prompt. Earlier structure abstracts the individual prompt components from the autoedits-provider so that, autoedits-provider only has to know about the final prompt and not the individual components, and final prompt-strateges (eg: default-prompt-strategy.ts or short-term-diff-prompt-strategy.ts) can call and use these prompt component fileWithMarkerPrompt and integrate/manipulate however it sees fit.
autoedit-providerfirst receiving the these prompt components, then it sending to the prompt constructors (likedefault-prompt-strategy) imposes some risk, for example what if the prompt gets modified in theautoedits-provideritself.- Also, this introduces another layer in the final prompt construction, where I need to aware of how the prompt flows in the
autoedits-provideras well, where earlier, I could just readprompt-constructorfor the final prompt creation and had to not at all look at theautoedits-provider.
I think if we need some additional fields from the prompt constructor we could send them in the codeToReplaceData itself, but can keep the responsibility of creating final prompt with individual prompt constructors.
There was a problem hiding this comment.
Yeah, I agree. I started moving fileWithMarkerPrompt, areaPrompt out of getCurrentFilePromptComponents so that we can get only codeToReplaceData and other parts would be calculated inside of the getPromptForModelType method but it involved too many additional changes.
For now, my goal is to extract the codeToReplaceData calculation because it's helpful to have it early. I can hide fileWithMarkerPrompt, areaPrompt from the autoedits provider in a follow-up PR. WDYT?
There was a problem hiding this comment.
yeah totally, we can do that in a follow up PR.
Approving to unblock !!
There was a problem hiding this comment.
Created an issue to follow-up https://linear.app/sourcegraph/issue/CODY-4596/hide-prompt-component-management-logic-from-the-autoedits-provider
There was a problem hiding this comment.
We can address the comment in the follow up PR.
Otherwise, looks good to me !!
valerybugakov
deleted the
vb/autoedits-analytics-logger-integration-2
branch
codeToReplaceDatacomputation fromgetPromptForModelType. This is necessary becausecodeToReplaceDatawill be used to create the analytics logger request early in the autoedits generation code path. By calculating it independently, we can have it ready earlier in the pipeline.Test plan
CI + updated unit tests.