-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Add second check for prompt length before autocomplete #6451
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
base: main
Are you sure you want to change the base?
Conversation
👷 Deploy request for continuedev pending review.Visit the deploys page to approve it
|
All contributors have signed the CLA ✍️ ✅ |
recheck |
I have read the CLA Document and I hereby sign the CLA |
Passing this to @jpoly1219 who is working on autocomplete things at the moment |
@@ -126,3 +135,163 @@ export function renderPrompt({ | |||
}, | |||
}; | |||
} | |||
|
|||
/** Builds the final prompt by applying prefix/suffix compilation or snippet formatting, then rendering the template. */ | |||
function buildPrompt( |
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.
Can we add unit tests for this new function?
@@ -126,3 +135,163 @@ export function renderPrompt({ | |||
}, | |||
}; | |||
} | |||
|
|||
/** Builds the final prompt by applying prefix/suffix compilation or snippet formatting, then rendering the template. */ | |||
function buildPrompt( |
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.
The core logic of this is a duplicate of renderPrompt
. I think a good next step to take here is to refactor renderPrompt
using buildPrompt
. Also +1 to adding unit tests.
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.
It looks good overall. I think reducing repetitive logic and adding unit tests would be greatly appreciated!
Description
There has been reports showing that continue will truncate prefix when sending request to LLM for autocompletion, See #3372 and #6003. The reason is that the renderPrompt function does not strictly follow the contextLength restriction, which causes the prompt to be truncated before sending to LLM.
To fix the issue, I
and that's it!
Checklist
My first PR, not sure how to test the code here. Would be happy to update if any guidance are provided :)
Summary by cubic
Added a second check to ensure prompt length stays within model limits before sending autocomplete requests, preventing prompt truncation.