Repository navigation
Python: FIX: honor overwrite=False for default execution settings in PromptTemplateConfig - #14586
Merged
Eduard van Valkenburg (eavanvalkenburg) merged 5 commits intoOct 27, 2026
Conversation
…mplateConfig add_execution_settings() checked membership with settings.service_id (None for the default service) but stored under settings.service_id or DEFAULT_SERVICE_NAME. The mismatch meant the None key never matched, so overwrite=False never protected the "default" entry. Use the same resolved key for both the check and the store, and emit the "already exist" warning only when the add is actually refused. fixes microsoft#14582
inchang-ing
temporarily deployed
to
github-app-auth
October 13, 2026 16:11 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
inchang-ing
temporarily deployed
to
github-app-auth
October 13, 2026 16:12 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
Author
|
@microsoft-github-policy-service agree |
semantic-kernel-automation
Bot
added
the
python
github-actions
Bot
changed the title
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The implementation and tests correctly address the reported defect; only a minor comment-language issue remains.
1 open finding
What changed in this PR
Fixes default execution settings being overwritten despite overwrite=False.
Changes:
- Resolves the effective service key before checking and storing settings.
- Logs warnings only when an addition is refused.
- Adds regression tests for default settings and warning behavior.
| File | Description |
|---|---|
python/semantic_kernel/prompt_template/prompt_template_config.py |
Corrects overwrite handling and warning placement. |
python/tests/unit/prompt_template/test_prompt_templates.py |
Adds regression coverage. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
inchang-ing
temporarily deployed
to
github-app-auth
October 22, 2026 19:11 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Eduard van Valkenburg (eavanvalkenburg)
enabled auto-merge
October 26, 2026 12:52
Keep code comments under python/semantic_kernel English-only so the maintenance context stays accessible to every contributor.
auto-merge was automatically disabled
October 26, 2026 19:07
Head branch was pushed to by a user without write access
Eduard van Valkenburg (eavanvalkenburg)
enabled auto-merge
October 27, 2026 08:48
Eduard van Valkenburg (eavanvalkenburg)
approved these changes
Oct 27, 2026
This branch was successfully 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.
Why
PromptTemplateConfig.add_execution_settings(settings, overwrite=False)still replaced the existing entry whensettings.service_idis None (the common case). Fixes #14582.Root cause
The membership check used
settings.service_id(None) while the entry is stored undersettings.service_id or DEFAULT_SERVICE_NAME("default").None in self.execution_settingsis always False, sooverwrite=Falsenever protected the default entry. The "already exist" warning was also logged after every successful add, and never when an add was actually refused.Fix
Use the same resolved key (
settings.service_id or DEFAULT_SERVICE_NAME) for both the membership check and the store, and emit the warning only on the refused path.Tests
test_add_execution_settings_no_overwrite_default_service:overwrite=Falsekeeps the original default settings.test_add_execution_settings_warning_only_when_refused: warning appears only when an add is refused, not on a fresh add.Both fail on the unpatched code and pass with the fix; the 18 pre-existing tests in the file still pass.