{{ message }}
Repository navigation
Python: Fix prepend_instructions_to_messages inverting instruction order on partial dedup - #8360
Closed
Sanjay Santhanam (Sanjays2402) wants to merge 1 commit into
Closed
Sanjay Santhanam (Sanjays2402) wants to merge 1 commit into
Sanjay Santhanam (Sanjays2402) wants to merge 1 commit into
Conversation
…ons_to_messages The positional dedup from microsoft#5051 skipped instructions already present as leading messages but then prepended the remaining ones in front of the whole message list. When only part of the instructions were already present, the remaining instructions landed before the skipped ones, silently reordering the system prompt: prepend_instructions_to_messages( [Message("system", ["First"])], ["First", "Second"]) -> ["Second", "First"] # wrong Insert the remaining instruction messages after the leading matched messages instead, so the original instruction order is preserved regardless of how much of it is already present: -> ["First", "Second"] # right Fixes microsoft#8352. Tests: added test_prepend_instructions_partial_dedup_preserves_order and test_prepend_instructions_partial_dedup_multiline to tests/core/test_types.py; all 310 tests in test_types.py and 32 in test_clients.py pass, ruff check and ruff format clean.
Sanjay Santhanam (Sanjays2402)
temporarily deployed
to
github-app-auth
September 13, 2026 23:29 — with
GitHub Actions
Inactive
Sanjay Santhanam (Sanjays2402)
temporarily deployed
to
github-app-auth
September 13, 2026 23:29 — with
GitHub Actions
Inactive
Sanjay Santhanam (Sanjays2402)
temporarily deployed
to
github-app-auth
September 13, 2026 23:29 — with
GitHub Actions
Inactive
Sanjay Santhanam (Sanjays2402)
temporarily deployed
to
github-app-auth
September 13, 2026 23:29 — with
GitHub Actions
Inactive
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
A moderate deduplication-ordering issue remains, and a regression test is needed.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR fixes instruction ordering during partial deduplication in the Python message helper.
Changes:
- Inserts remaining instructions after the matched leading prefix.
- Adds regression tests for partial-deduplication ordering.
File summaries
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
2001
to
+2002
| instruction_messages = [Message(role, [instr]) for instr in deduplicated] | ||
| return [*instruction_messages, *messages] | ||
| return [*messages[:insert_at], *instruction_messages, *messages[insert_at:]] |
Contributor
There was a problem hiding this comment.
please have a look at this when you can thanks
Contributor
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Motivation & Context
prepend_instructions_to_messagesgains a partial-dedup path from #5051: instructions already present as leading messages (same role and text) are skipped to avoid duplicate system messages when instructions are injected by multiple layers (e.g. Agent + chat client). But the remaining instructions were then prepended in front of the whole message list, so on a partial match they landed ahead of the skipped duplicates and the instruction order got inverted:Description & Review Guide
prepend_instructions_to_messages(python/packages/core/agent_framework/_types.py), the non-duplicated instruction messages are now inserted right after the leading matched messages instead of in front of the whole list. This preserves the caller-supplied instruction order no matter how much of it is already present as leading messages. Two regression tests added inpython/packages/core/tests/core/test_types.py(test_prepend_instructions_partial_dedup_preserves_order,test_prepend_instructions_partial_dedup_multiline).Related Issue
Fixes #8352
Contribution Checklist