Skip to content

.NET: Should we exclude cases where ConversationId="_agent_local_chat_history"? #8133

Closed

Description

@ZeeLyn

Should we exclude cases where ConversationId="_agent_local_chat_history"?

if (chatClientSession is not null &&
!string.IsNullOrWhiteSpace(chatClientSession.ConversationId))

Activity

  1. added
    .NETUsage: [Issues, PRs], Target: .Net
    triageUsage: [Issues], Target: All issues that still need to be triaged
    on Sep 8, 2026
  2. changed the title [-]Should we exclude cases where ConversationId="_agent_local_chat_history"?[/-] [+].NET: Should we exclude cases where ConversationId="_agent_local_chat_history"?[/+] on Sep 8, 2026
  3. removed
    triageUsage: [Issues], Target: All issues that still need to be triaged
    on Sep 8, 2026
  4. 1picassoai commented on Sep 10, 2026

    @1picassoai

    Yes, I think it should, and I can confirm the path that makes it bite.

    The sentinel is written onto the session, not just the response. In PerServiceCallChatHistoryPersistingChatClient.cs, the streaming path sets it directly on the session at line 280:

    session.ConversationId = LocalHistoryConversationId;   // "_agent_local_chat_history"

    and the non-streaming path reaches the same state via SetSentinelConversationId (L312-315), which sets it on both the response and the session.

    So by the time CompactionProvider.InvokingCoreAsync runs, the check at L134-135 is:

    ChatClientAgentSession? chatClientSession = session.GetService<ChatClientAgentSession>();
    if (chatClientSession is not null &&
        !string.IsNullOrWhiteSpace(chatClientSession.ConversationId))
    {
        logger.LogCompactionProviderSkipped("session managed by remote service");
        return context.AIContext;
    }

    "_agent_local_chat_history" is not whitespace, so the guard fires and compaction is skipped with the reason "session managed by remote service" — which is the exact opposite of the truth. The sentinel exists precisely to signal that history is local. That means locally-persisted conversations silently stop compacting after the first service call, and the only trace is a debug-level skip log most people never have enabled.

    The fix that matches intent is to treat the sentinel as absent:

    ChatClientAgentSession? chatClientSession = session.GetService<ChatClientAgentSession>();
    string? conversationId = chatClientSession?.ConversationId;
    
    bool isServiceManaged =
        !string.IsNullOrWhiteSpace(conversationId) &&
        !string.Equals(conversationId, "_agent_local_chat_history", StringComparison.Ordinal);
    
    if (isServiceManaged)
    {
        logger.LogCompactionProviderSkipped("session managed by remote service");
        return context.AIContext;
    }

    LocalHistoryConversationId is internal const on PerServiceCallChatHistoryPersistingChatClient, so within Microsoft.Agents.AI it can be referenced by name rather than duplicated as a literal. It would probably be worth hoisting it to a shared internal constant so the two files cannot drift, and adding a test that asserts a session carrying the sentinel still compacts.

    A related point worth deciding at the same time: the skip is logged, but there is no event or property a caller can inspect to find out that compaction did not run. Silent skips and silent compactions are both hard to notice — from the outside, a conversation that quietly stopped compacting and one that compacted away its opening instructions look identical until the model starts behaving oddly.

    On that last point I should disclose an interest: I wrote a small MIT package, Picasso.AgentFramework.Persistence, which is a SQLite ChatHistoryProvider that records every compaction it observes — including whether the first user message survived it — because I hit the same class of silent-history problem and could not see it. Detection has to sit in InvokingCoreAsync rather than StoreChatHistoryAsync, since the base class strips provider-contributed messages before the latter runs. Treat that as a disclosure rather than a recommendation; the fix above stands on its own.

  5. westey-m commented on Sep 29, 2026

    @westey-m
    Collaborator

    Should we exclude cases where ConversationId="_agent_local_chat_history"?

    if (chatClientSession is not null &&
    !string.IsNullOrWhiteSpace(chatClientSession.ConversationId))

    Yes, we should exclude skipping for cases where ConversationId="_agent_local_chat_history", since if the provider is used as a chat client decorator below the per-service-call chat history provider in the stack, it should be able to compact chat history coming from the per-service-call chat history provider. This may have the side-affect of a the provider trying to act upon new input only if it was inserted in a different place in the stack where there is no chat history added to the input yet. This is unlikely to achieve any compaction due to input typically being constrained already. But this can easily be avoided by not inserting the provider in those places.

    We may also want to consider never skipping and leaving it up to the developer to choose where they want to insert the provider so that it can act upon whatever is available at that location.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

.NETUsage: [Issues, PRs], Target: .NetagentsUsage: [Issues, PRs], Target: Single agent

Type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions