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

Removed token count of user message #836

Conversation

tdechanterac
Copy link

Motivation and Context

  1. Why is this change required?
    The user message is not added to the GPT completion request when the token count of the prompt exceeds 50% of the configuration.
  2. What problem does it solve?
    We can now use large prompts in ChatCopilot.

Description

The method GetChatContextTokenLimit from ChatPlugin was used to count the user message, indicating that it needed to be added. However, it was never added directly; instead, it was added by the GetAllowedChatHistoryAsync method.

For example, if you have a configuration with:

  • CompletionTokenLimit: 4096
  • ResponseTokenLimit: 1024

and the following prompt:

You are a specialist in HR and recruitment.
You analyze the transcript of an interview in relation to a job offer announcement.
Here is the announcement
[START OFFER]
{{Offer content that has more than 1000 tokens}}
[END OFFER]
Here is the transcript of the interview
[START TRANSCRIPT]
{{meeting transcript that has more than 1000 tokens}}
[END TRANSCRIPT]
Give me the list of 5 positive and negative points about the candidate in relation to the announcement
Give me a rating out of 5 for the candidate

You end up with a completion request that only contains the user intent, and the bot responds with:

Sure, I can help with that. Can you provide me with more details about the candidate and the job posting?

Note: We could also address this issue by adding the user message directly into the chat history and ensuring that the message is not added a second time in GetAllowedChatHistoryAsync.

@github-actions github-actions bot added the webapi Pull requests that update .net code label Mar 4, 2024
@glahaye glahaye mentioned this pull request Mar 6, 2024
4 tasks
@glahaye
Copy link
Contributor

glahaye commented Mar 6, 2024

@tdechanterac Thanks for figuring this out! The code in ChatPlugin.cs is quite "dense" (to use an euphemism). I had to spend quite some time reading it to fully understand the impact of that one-line change.

I think your fix would make the code better. But the code would still not be guaranteed to include the user prompt because a big enough portion of the token budget could be taken by memories.

So I came up with bigger change that I think is "safer" and more closely respects the original intent of the code: #839

Please take a look at it and feel free to comment on it. This new PR would make your change no longer needed.

While I was at it, I made a few related changes to make ChatPlugin.cs more readable too.

@glahaye glahaye self-assigned this Mar 6, 2024
@glahaye glahaye added the PR: paused For PRs that have been converted to draft, are facing blockers or have no active development planned label Mar 6, 2024
@tdechanterac
Copy link
Author

Thank you !

github-merge-queue bot pushed a commit that referenced this pull request Mar 9, 2024
### Motivation and Context
The user message is not added to the GPT completion request when the
token count of the prompt exceeds 50% of the configuration.

This fix is inspired by #836

### Description
Fix budget calculation and clarify / simplify code.

### Contribution Checklist
- [ ] The code builds clean without any errors or warnings
- [ ] The PR follows the [Contribution
Guidelines](https://github.com/microsoft/chat-copilot/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/chat-copilot/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [ ] All unit tests pass, and I have added new tests where possible
- [ ] I didn't break anyone 😄
jlubken pushed a commit to pennsignals/chat-copilot that referenced this pull request Mar 11, 2024
### Motivation and Context
The user message is not added to the GPT completion request when the
token count of the prompt exceeds 50% of the configuration.

This fix is inspired by microsoft#836

### Description
Fix budget calculation and clarify / simplify code.

### Contribution Checklist
- [ ] The code builds clean without any errors or warnings
- [ ] The PR follows the [Contribution
Guidelines](https://github.com/microsoft/chat-copilot/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/chat-copilot/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [ ] All unit tests pass, and I have added new tests where possible
- [ ] I didn't break anyone 😄
teamleader-dev pushed a commit to vlink-group/chat-copilot that referenced this pull request Oct 7, 2024
### Motivation and Context
The user message is not added to the GPT completion request when the
token count of the prompt exceeds 50% of the configuration.

This fix is inspired by microsoft#836

### Description
Fix budget calculation and clarify / simplify code.

### Contribution Checklist
- [ ] The code builds clean without any errors or warnings
- [ ] The PR follows the [Contribution
Guidelines](https://github.com/microsoft/chat-copilot/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/chat-copilot/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [ ] All unit tests pass, and I have added new tests where possible
- [ ] I didn't break anyone 😄
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: paused For PRs that have been converted to draft, are facing blockers or have no active development planned webapi Pull requests that update .net code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants