Skip to content

fix: trim history by logical turn boundaries instead of message count since history is always a clean sequence of alternating user and assistant messages#71

Open
Danchi-1 wants to merge 1 commit intomofa-org:mainfrom
Danchi-1:main
Open

Conversation

@Danchi-1
Copy link
Copy Markdown

📋 Summary

The previous history trimming logic used a raw message count (1 + max_exchanges * 2) based on the assumption that every conversation turn is exactly two messages: one user, one assistant. That assumption breaks down when tool-calling is involved, because a single logical turn can span four or more messages. The new logic groups messages into logical turns by identifying User message boundaries, then drops only complete oldest turns until the session is within the max_exchanges limit.

🔗 Related Issues

Closes #30

Related to #


🧠 Context

The change is needed because manage_history trimmed conversation history by raw message count, which assumes every turn is exactly two messages, but tool-calling turns can span four or more. It solves the problem of history trimming slicing through the middle of a tool-call sequence, leaving orphaned or mismatched messages that corrupt the context sent to the AI provider. The fix adopts a turn-boundary approach by grouping everything between User messages as one atomic unit because User messages are the only reliable structural delimiter in the OpenAI message format that marks where one logical exchange ends and another begins.


🛠️ Changes

  • Replaced raw message-count trimming in manage_history with logical turn-boundary trimming
  • History is now grouped by User message boundaries, where each group represents one complete exchange including any tool calls and results
  • Oldest complete turns are dropped as a unit instead of individual messages being drained by count
  • System prompt at index 0 is still always preserved and never touched

🧪 How you Tested

    1. Plain chat: no tools
      Started a session and sent more messages than max_history_exchanges allows. Verify that the oldest turns are dropped cleanly, the system prompt is always the first message, and responses remain coherent.
  1. Single tool call turn
    Configure a session with enable_local_mcp = true and trigger a single tool call. Let it complete fully (user -> assistant+tool_calls -> tool result -> final assistant), then send enough subsequent messages to push it out of the history window. Verify the entire tool-call turn is dropped as one unit with no orphaned messages remaining.
  2. Multiple tool results in one turn
    Trigger a turn where the assistant calls more than one tool. After history trimming occurs, inspect session.messages and confirm all tool result messages from that turn were dropped together alongside their parent assistant tool-call message.
  3. Boundary condition: exactly at the limit
    Send exactly max_history_exchanges turns. Verify nothing is trimmed and all messages are preserved.
  4. Boundary condition: one over the limit
    Send max_history_exchanges + 1 turns. Verify only the single oldest complete turn is dropped and nothing else.
  5. Inspect message list directly
    Add a temporary debug log after each manage_history call that prints session.messages and manually confirm no message of type Tool or Assistant+tool_calls exists without its corresponding counterpart.

📸 Screenshots / Logs (if applicable)


⚠️ Breaking Changes

  • No breaking changes

🧹 Checklist

Code Quality

  • Code follows Rust idioms and project conventions
  • cargo fmt run
  • cargo clippy passes without warnings

Testing

  • Tests added/updated
  • cargo test passes locally without any error

Documentation

  • Public APIs documented
  • README / docs updated (if needed)

PR Hygiene

  • PR is small and focused (one logical change)
  • Branch is up to date with main
  • No unrelated commits
  • Commit messages explain why, not only what

🚀 Deployment Notes (if applicable)


🧩 Additional Notes for Reviewers

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(maas-client): ChatSession history trimming can break tool-call context

1 participant