semantic-kernel icon indicating copy to clipboard operation
semantic-kernel copied to clipboard

TextMemorySkill Never Resolves [dotnet]

Open WretchedDade opened this issue 2 years ago • 1 comments

Describe the bug I'm replicating the GitHub Repo Summary and GitHub Memory Query skills and functions but for Azure DevOps repos. I am able to summarize and stored embedding in memory and manually search them using SearchAsync. However, when I try to use the TextMemorySkill.Recall() function either manually or from a prompt, the function gets stuck in async land when calling FirstOrDefault() on the returned memories.

Locally I copied the skill and made the following changes:

var memories = context.Memory
+    .SearchAsync(collection, text, limitInt, minRelevanceScore: float.Parse(relevance, CultureInfo.InvariantCulture));
-    .SearchAsync(collection, text, limitInt, minRelevanceScore: float.Parse(relevance, CultureInfo.InvariantCulture))
-    .ToEnumerable

context.Log.LogTrace("Done looking for memories in collection '{0}')", collection);

string resultString;

if (limitInt == 1)
{
-    var memory = memories.FirstOrDefault();
+    var memory = await memories.FirstOrDefaultAsync();
    resultString = (memory != null) ? memory.Metadata.Text : string.Empty;
}
else
{
-    resultString = JsonSerializer.Serialize(memories.Select(x => x.Metadata.Text));
+    resultString = JsonSerializer.Serialize(await memories.Select(x => x.Metadata.Text).ToListAsync());
}

When using my "patched" skill, it works correctly and resolves.

WretchedDade avatar Apr 26 '23 16:04 WretchedDade

I have opened a PR (#673) to merge the changes I called out above.

WretchedDade avatar Apr 26 '23 16:04 WretchedDade

Hey @lemillermicrosoft, just wanted to close the loop.

I dug deeper and noticed some differences between the latest code in the repo and the code in the Nuget package version I was using. Thankfully, there was a new package version published yesterday and after updating, my code works as expected.

I'm closing this issue now.

WretchedDade avatar Apr 26 '23 19:04 WretchedDade