fix(agentchat): persist sources filter in TextMentionTerminationConfig#7650
Open
xodn348 wants to merge 1 commit intomicrosoft:mainfrom
Open
fix(agentchat): persist sources filter in TextMentionTerminationConfig#7650xodn348 wants to merge 1 commit intomicrosoft:mainfrom
xodn348 wants to merge 1 commit intomicrosoft:mainfrom
Conversation
TextMentionTermination accepted a sources whitelist via __init__ but TextMentionTerminationConfig only stored text, so the filter was silently dropped on every serialize/deserialize round-trip. Components loaded from YAML/JSON would therefore check all messages including the initial user task prompt, causing spurious early termination when the task description happened to contain the stop phrase. Adds sources: List[str] | None = None to the config model, updates _to_config/_from_config to round-trip the field, improves the docstring to explain the use-case, and adds a round-trip assertion to the declarative component test.
Contributor
|
@xodn348 please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
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.
Summary
TextMentionTerminationhas accepted asourceswhitelist parameter since its introduction, allowing callers to restrict termination checks to specific agent names. However,TextMentionTerminationConfig— the Pydantic model used for component serialization — only stored thetextfield. Every serialize → deserialize round-trip (e.g. loading a team from YAML or JSON) silently dropped thesourcesfilter, restoring the "check all messages" behaviour. This meant a component configuration that worked correctly in memory would produce different, broken behaviour after persistence.One observable consequence (reported in #6826) is that the termination phrase placed in the initial user task prompt triggers immediate termination: the user passes
sources=["assistant"]to avoid checking their own prompt, but after loading from configsourcesis gone and the user message fires the condition instead.The fix adds
sources: List[str] | None = NonetoTextMentionTerminationConfig, updates_to_configto serialize the value, updates_from_configto pass it back to__init__, and improves the docstring with a concrete guidance note. A round-trip assertion is added totest_declarative_components.py.Issue
Refs #6826
Local verification
Also ran ruff on the changed files — 1 import-ordering fix applied automatically, 0 remaining errors.
Risk
The only change to the runtime
__call__path is in_to_config/_from_config; in-memory behaviour of existing code that never round-trips through the config is unaffected. Callers who rely on the old (broken) serialisation omittingsourceswould see the field appear in their config JSON with valuenull, which is schema-compatible. No breaking API change.