-
Notifications
You must be signed in to change notification settings - Fork 295
enhance: avoid context limit #832
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
Conversation
Signed-off-by: Grant Linville <[email protected]>
Signed-off-by: Grant Linville <[email protected]>
Signed-off-by: Grant Linville <[email protected]>
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 very important thing is that we never drop the system role message. I believe that is still that case.
@ibuildthecloud correct. The way I implemented this will never change how the system role messages are handled. Those will always stick around. |
pkg/openai/client.go
Outdated
@@ -317,6 +320,14 @@ func (c *Client) Call(ctx context.Context, messageRequest types.CompletionReques | |||
} | |||
|
|||
if messageRequest.Chat { | |||
// Check the last message. If it is from a tool call, and if it takes up more than 80% of the budget on its own, reject it. | |||
lastMessage := msgs[len(msgs)-1] | |||
if lastMessage.Role == string(types.CompletionMessageRoleTypeTool) && countMessage(lastMessage) > int(math.Round(float64(getBudget(messageRequest.MaxTokens))*0.8)) { |
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.
nit: probably don't need to math.Round
here. Not much of a difference between 102,399 and 102,400.
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.
Fixed
pkg/openai/client.go
Outdated
|
||
// If we got back a context length exceeded error, keep retrying and shrinking the message history until we pass. | ||
var apiError *openai.APIError | ||
if err != nil && errors.As(err, &apiError) && apiError.Code == "context_length_exceeded" && messageRequest.Chat { |
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.
nit: errors.As
takes care of the err != nil
check
if err != nil && errors.As(err, &apiError) && apiError.Code == "context_length_exceeded" && messageRequest.Chat { | |
if errors.As(err, &apiError) && apiError.Code == "context_length_exceeded" && messageRequest.Chat { |
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.
Fixed
err error | ||
) | ||
|
||
for range 10 { // maximum 10 tries |
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.
Is this the first use in our code base?
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.
I think so!
pkg/openai/count.go
Outdated
|
||
func decreaseTenPercent(maxTokens int) int { | ||
maxTokens = getBudget(maxTokens) | ||
return int(math.Round(float64(maxTokens) * 0.9)) |
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.
same nit about math.Round
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.
Fixed
Signed-off-by: Grant Linville <[email protected]>
for gptscript-ai/desktop#283
This PR introduces three new measures to try to help the user avoid hitting the context length error:
With these three measures, I am fairly confident that users will no longer encounter the error.