Skip to content

.Net: Chat History Reducer is possibly in the wrong spot #10203

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

Closed
wmeints opened this issue Jan 16, 2025 · 3 comments · Fixed by #10285
Closed

.Net: Chat History Reducer is possibly in the wrong spot #10203

wmeints opened this issue Jan 16, 2025 · 3 comments · Fixed by #10285
Assignees
Labels
.NET Issue or Pull requests regarding .NET code

Comments

@wmeints
Copy link

wmeints commented Jan 16, 2025

I noticed that the IChatHistoryReducer was graduated from a sample to the agents layer of the framework. However, I feel that this is the wrong place to put it.

On the one hand, it makes sense to add it to an agent because agents are often used in chat scenarios. I understand that. However, I'm using chat history in non-chat scenarios too, and I don't want to use the agent framework in my application. I consider many of my applications with Semantic Kernel workflows rather than agents.

Would it be possible to consider adding the chat history reduction logic to the core of Semantic Kernel rather than the agent layer?

@moonbox3 moonbox3 changed the title Chat History Reducer is possibly in the wrong spot .Net: Chat History Reducer is possibly in the wrong spot Jan 16, 2025
@moonbox3 moonbox3 added the .NET Issue or Pull requests regarding .NET code label Jan 16, 2025
@sphenry
Copy link
Member

sphenry commented Jan 21, 2025

I think this makes sense to associate with ChatHistory

@sphenry sphenry removed the triage label Jan 21, 2025
@markwallace-microsoft markwallace-microsoft moved this to Sprint: Planned in Semantic Kernel Jan 21, 2025
@dmytrostruk dmytrostruk moved this from Sprint: Planned to Sprint: In Progress in Semantic Kernel Jan 22, 2025
@dmytrostruk
Copy link
Member

@wmeints Thanks for creating this issue!

Before doing any changes in the codebase, it would be great to learn more about your use case. For example, when exactly do you want to reduce a chat history, should it be a manual call such as chatHistory.Reduce(...) on your side or something automatic on SK side?

Also, do you want just an abstraction (e.g., the IChatHistoryReducer interface) as part of the Microsoft.SemanticKernel.Abstractions package, allowing you to implement your own custom reducer? Or are you also interested in existing implementations from the Agents.Core package, such as ChatHistorySummarizationReducer and ChatHistoryTruncationReducer?

If you have any code samples (even pseudocode will be helpful) or any ideas how IChatHistoryReducer could be used on your side - it will help to understand how to place this functionality in SK Abstractions/Core packages properly. Thank you!

@wmeints
Copy link
Author

wmeints commented Jan 23, 2025

Hey @dmytrostruk, let me explain my scenario a bit more.

I want to store the full chat history in a database like Cosmos or MongoDB. But I can't send it to the LLM in one piece after a while because it can become too large. That's where the reducer logic comes in. Usually, I just need to truncate the old messages from the history and use the remainder. But I can imagine others need to summarize their history. So, in short, I'd like to have the option to implement my reducer, but I also like the idea of using one of the standard reducers.

I would love to have a method on the ChatHistory to reduce it using IChatHistoryReducer so I can control when I call the reduction logic.

@dmytrostruk dmytrostruk moved this from Sprint: In Progress to Sprint: In Review in Semantic Kernel Jan 24, 2025
github-merge-queue bot pushed a commit that referenced this issue Jan 27, 2025
### Motivation and Context

<!-- Thank you for your contribution to the semantic-kernel 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.
-->

Resolves: #10203

This PR:
- Moves `IChatHistoryReducer` from `Agents.Core` to
`Microsoft.SemanticKernel.Abstractions` package.
- Moves implementations `ChatHistorySummarizationReducer` and
`ChatHistoryTruncationReducer` from `Agents.Core` to
`Microsoft.SemanticKernel.Core` package.

The functionality is marked as experimental.

### Contribution Checklist

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

- [x] The code builds clean without any errors or warnings
- [x] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [x] All unit tests pass, and I have added new tests where possible
- [x] I didn't break anyone 😄
@dmytrostruk dmytrostruk moved this from Sprint: In Review to Sprint: Done in Semantic Kernel Jan 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
.NET Issue or Pull requests regarding .NET code
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants