[黄耀鑫(Huang Yaoxin) (YaoxinHuang)]

Motivation and Context

ChatHistory.replace clears its messages before consuming the replacement iterable. Passing the history itself, its messages list, or a generator filtering the history therefore silently empties it. For example, removing assistant messages with history.replace(m for m in history if m.role != AuthorRole.ASSISTANT) also loses the system and user messages.

Description

Materialize the iterable before clearing the history, while preserving the existing messages list. Add regression coverage for the history, its backing list, an iterator, and a filtered generator.

Validated the contents unit suite, repository pre-commit hooks for both changed files, scoped mypy, and a wheel build. The full optional-provider test matrix was not run.

Contribution Checklist

• The code builds clean without any errors or warnings
• The PR follows the SK Contribution Guidelines and the pre-submission formatting script raises no violations
• All unit tests pass, and I have added new tests where possible
• I didn't break anyone 😄

[Copilot]

Copilot review overview

🟢 Approval recommended

Reviewed changes address the reported replacement behavior with regression coverage.

Review effort: Lite
Findings: None

What changed in this PR

Updates ChatHistory.replace to preserve messages when replacing history from itself or related iterables.

Changes:

• Materializes replacement messages before clearing history.
• Adds regression tests for histories, lists, iterators, and filtered generators.

File	Description
python/​tests/​unit/​contents/​test_chat_history.py	Adds regression coverage for self-referential replacements.
python/​semantic_kernel/​contents/​chat_history.py	Safely materializes replacement messages while preserving list identity.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[黄耀鑫(Huang Yaoxin) (YaoxinHuang)]

@microsoft-github-policy-service agree

[PRABHU KIRAN VANDRANKI (VANDRANKI)]

Community review, not a maintainer review, so this does not clear the merge gate. I read the full diff and ran ChatHistory.replace on this branch and on origin/main (Python 3.13, python/ of a local worktree).

The bug is real and the fix is correct for what I ran. I built a history of three messages (system s, user q, assistant a) and called replace with different sources:

• replace(history), replace(history.messages) and replace(iter(history)): main leaves the history empty ([]), because clear() empties the very list the iterable reads from. This branch leaves ['s', 'q', 'a'].
• replace(m for m in history if m.role != assistant): main gives [], this branch gives ['s', 'q'].
• replace(list(history.messages)) (a copy): ['s', 'q', 'a'] on both, so existing callers that already pass a copy see no change.

Tests: tests/unit/contents/test_chat_history.py gives 59 passed on this branch. With main's chat_history.py and the PR's test file, 4 fail (the three test_replace_from_same_history ids and test_replace_from_filtered_history), so the new tests do catch the bug.

Reading notes, nothing blocking:

• The new messages is messages assertion in the test checks that the list object is kept and cleared in place. That matches clear() calling self.messages.clear(), so anyone holding a reference to the old list still sees the new content.
• list(messages) copies the input once. That is the whole cost of the change.
• In python/semantic_kernel my grep (for def replace, .replace(messages, .replace(self and .replace(chat) found only the ChatHistory.replace definition and no caller that passes a history to itself. That grep is narrow, so it does not rule out other call shapes. I did not search the samples or other packages.

The change is small and does what it says.