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

Documents uploaded with global scope not searched by CoPilot memory #421

Closed
arvashis opened this issue Sep 29, 2023 · 5 comments
Closed
Assignees
Labels
bug Something isn't working

Comments

@arvashis
Copy link

Describe the bug
Any documents uploaded with global scope via the console app ImportDocument in not included in documentmemory search. If the document is uploaded in the context of a chat via the UI then everything works fine.

To Reproduce
Steps to reproduce the behavior:

  1. Upload a supported document via ImprtDocument console app as per the README.md.
  2. Start a new chat OR even use an ongoing chat with the copilot and ask a specific question about the content from the document you uploaded.
  3. CoPilot will return a generic answer without any context to the uploaded document.
  4. Now, upload the same / other document from the upload tab on the copilot
  5. Now Ask Copilot as in This repo is missing important files #2
  6. This time CoPilot returns the expected response.

Expected behavior
Documents uploaded with Global scope are used by the copilot to respond to the query in any chat thread.

Platform

  • OS: Windows
  • IDE: Visual Studio
  • Language: C#

Additional context
Add any other context about the problem here.

@dehoward
Copy link
Contributor

@TaoChenOSU can you help take a look at this? we should be seeing the same results no matter where how the document was uploaded.

@dehoward dehoward added the bug Something isn't working label Sep 29, 2023
@arvashis
Copy link
Author

I believe ISemanticMemoryExtensions clss sets filter with chatid and since the globally uploaded doc is not associated with this chatid, the filter leaves them out. If the memory has a chatid=Guid.Empty then that also should be included as it would seem global.

public static async Task SearchMemoryAsync(
this ISemanticMemoryClient memoryClient,
string indexName,
string query,
float relevanceThreshold,
int resultCount,
string chatId,
string? memoryName = null,
CancellationToken cancellationToken = default)
{
var filter =
new MemoryFilter
{
MinRelevance = relevanceThreshold,
};

 filter.ByTag(MemoryTags.TagChatId, chatId);

 if (!string.IsNullOrWhiteSpace(memoryName))
 {
     filter.ByTag(MemoryTags.TagMemory, memoryName);
 }

 var searchResult =
     await memoryClient.SearchAsync(
         query,
         indexName,
         filter,
         resultCount,
         cancellationToken);

 return searchResult;

}

@liebert100
Copy link

liebert100 commented Oct 3, 2023

Changing the code in chat-copilot\webapi\Skills\ChatSkills\SemanticMemoryRetriever.cs fixed it for me.
Calling with chatId only for WorkingMemory

async Task SearchMemoryAsync(string memoryName)
{
var searchResult =
await this._memoryClient.SearchMemoryAsync(
this._promptOptions.MemoryIndexName,
query,
this.CalculateRelevanceThreshold(memoryName, chatSession!.MemoryBalance),
memoryName == this._promptOptions.WorkingMemoryName ? chatId : Guid.Empty.ToString(),
memoryName)
.ConfigureAwait(false);

@arvashis
Copy link
Author

arvashis commented Oct 3, 2023

That worked for me - Thanks!

@TaoChenOSU
Copy link
Collaborator

Hello @arvashis,

Thanks for reporting the bug!

The solution provided above is an ideal solution. We are working to fix this bug properly.

@dehoward dehoward removed their assignment Oct 4, 2023
github-merge-queue bot pushed a commit that referenced this issue Oct 4, 2023
### Motivation and Context

<!-- Thank you for your contribution to the chat-copilot repo!
Please help reviewers and future users, providing the following
information:
  1. Why is this change required?
  2. What problem does it solve?
  3. What scenario does it contribute to?
  4. If it fixes an open issue, please link to the issue here.
-->
1. Issue reported by the community:
#421
2. Citation was accidentally removed by a previous PR.

### Description
1. Fix the issue.
2. Add back citations.

<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [ ] 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
bug Something isn't working
Projects
No open projects
Development

No branches or pull requests

4 participants