Skip to content

_ChannelIdFieldMixin removal - #490

Merged
Rodrigo Brandão (rodrigobr-msft) merged 10 commits into
mainfrom
users/robrandao/channel-id-2
Jul 22, 2026
Merged

_ChannelIdFieldMixin removal#490
Rodrigo Brandão (rodrigobr-msft) merged 10 commits into
mainfrom
users/robrandao/channel-id-2

Conversation

@rodrigobr-msft

Copy link
Copy Markdown
Contributor

This pull request removes the _ChannelIdFieldMixin mixin and refactors the handling of channel_id throughout the codebase to use the ChannelId class directly. It also improves the ChannelId implementation to provide static methods for extracting the channel and sub-channel, and updates all usages to leverage these utility methods. These changes simplify the code, improve type safety, and centralize channel ID logic.

Channel ID Refactoring

  • Removed the _ChannelIdFieldMixin mixin and its usage from Activity and ConversationReference classes; channel_id is now a direct field of type Optional[ChannelId] in both classes. [1] [2] [3] [4] [5]
  • Updated all imports and __init__.py exports to remove references to _ChannelIdFieldMixin. [1] [2] [3]

ChannelId Utilities and Implementation

  • Enhanced the ChannelId class to include static methods get_channel and get_sub_channel for extracting the base channel and sub-channel, and refactored its constructor logic for clarity and correctness. [1] [2] [3] [4]

Usage Updates

  • Replaced manual extraction and normalization of channel IDs throughout the codebase with calls to ChannelId.get_channel(), including in the UserTokenClient and related authentication methods. [1] [2] [3] [4] [5] [6] [7] [8]
  • Updated logic in Activity.get_conversation_reference to use ChannelId.get_channel() for extracting the base channel.

Cleanup

  • Removed now-unnecessary code and comments related to the mixin and computed field serialization/validation. [1] [2] [3]

These changes make channel ID handling more robust and maintainable by consolidating logic into the ChannelId class and simplifying model definitions.

Copilot AI review requested due to automatic review settings July 22, 2026 16:20
@rodrigobr-msft Rodrigo Brandão (rodrigobr-msft) changed the title Users/robrandao/channel id 2 _ChannelIdFieldMixin removal Jul 22, 2026
@rodrigobr-msft
Rodrigo Brandão (rodrigobr-msft) marked this pull request as ready for review July 22, 2026 16:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request refactors channel ID handling across the Microsoft 365 Agents SDK for Python to remove the _ChannelIdFieldMixin and centralize parsing/normalization in the ChannelId type, aiming to simplify model definitions while keeping Bot Framework-compatible serialization behavior.

Changes:

  • Removed _ChannelIdFieldMixin and migrated Activity / ConversationReference to explicit channel_id: Optional[ChannelId] fields.
  • Enhanced ChannelId with constructor normalization and new utility methods (get_channel, get_sub_channel) and updated core usages to rely on them.
  • Updated affected tests and internal call sites (e.g., token client, adapter activity creation) to work with the refactor.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/activity/test_channel_id.py Adds coverage for ChannelId constructor behavior (instance reuse).
tests/activity/pydantic/test_channel_id_field_mixin.py Removes mixin-specific tests after mixin deletion.
tests/activity/pydantic/test_activity_io.py Adjusts Activity IO tests to align with the new channel_id field approach.
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/connector/client/user_token_client.py Switches base-channel extraction to ChannelId.get_channel.
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/channel_service_adapter.py Wraps adapter-provided channel_id into ChannelId when building activities.
libraries/microsoft-agents-activity/microsoft_agents/activity/conversation_reference.py Removes mixin inheritance and adds channel_id field typed as ChannelId.
libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py Refactors ChannelId construction and introduces get_channel / get_sub_channel.
libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py Removes mixin inheritance, adds channel_id field, and updates base-channel extraction logic.
libraries/microsoft-agents-activity/microsoft_agents/activity/_channel_id_field_mixin.py Deletes the mixin implementation.
libraries/microsoft-agents-activity/microsoft_agents/activity/init.py Removes _ChannelIdFieldMixin exports.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot AI review requested due to automatic review settings July 22, 2026 16:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py:156

  • Activity.channel_id is now a plain field, but AgentsModel does not enable assignment validation. Existing code/tests set activity.channel_id = "msteams:..." after initialization, which will leave a raw str on the model; _serialize_sub_channel_data then unconditionally accesses .sub_channel and will raise AttributeError during model_dump(_json). Enabling assignment validation on Activity restores the previous behavior of coercing strings into ChannelId.

    type: NonEmptyString
    channel_id: Optional[ChannelId] = None

Comment thread libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py Outdated
Comment thread tests/activity/test_channel_id.py
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 22, 2026 16:46
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py:124

  • ChannelId.get_channel() does not strip whitespace when the input has no ':' (e.g. " msteams "), because it only strips the split result when a colon is present. This contradicts the new unit test expectations and can leak unnormalized channel IDs to callers.
        if not channel_id or not channel_id.strip():
            return channel_id
        if isinstance(channel_id, ChannelId):
            return channel_id.channel

libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py:114

  • ChannelId.get_sub_channel() returns the raw ChannelId.sub_channel when given a ChannelId instance. For values like "msteams:", ChannelId.sub_channel can be "" (empty string), so get_sub_channel(ChannelId("msteams:")) returns "" instead of None, which is inconsistent with get_sub_channel("msteams:") and the method contract.
        if not channel_id or not channel_id.strip():
            return None
        if isinstance(channel_id, ChannelId):
            return channel_id.sub_channel
        value = channel_id.strip()

Copilot AI review requested due to automatic review settings July 22, 2026 16:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py:156

  • Activity.channel_id is now a plain Pydantic field, but AgentsModel does not enable validate_assignment. This means activity.channel_id = "msteams:sub" will leave a raw str on the model, and _serialize_sub_channel_data later assumes a ChannelId (accesses .sub_channel / .channel), which can raise at runtime. Consider re-enabling assignment validation to preserve the previous mixin’s coercion behavior and keep the serializer’s assumptions safe.
    type: NonEmptyString
    channel_id: Optional[ChannelId] = None

tests/activity/test_channel_id.py:36

  • The old UserToken._base_channel_id tests covered edge cases like ":COPILOT" and whitespace inputs. After moving logic to ChannelId.get_channel(), those edge cases are no longer covered here. Adding them to this test helps prevent regressions in normalization behavior for odd/malformed channel IDs.
    def test_get_channel_strips_and_drops_sub_channel(self):
        assert ChannelId.get_channel(" msteams ") == "msteams"
        assert ChannelId.get_channel("msteams:sub") == "msteams"
        assert ChannelId.get_channel("msteams:") == "msteams"
        assert ChannelId.get_channel(None) is None

Copilot AI review requested due to automatic review settings July 22, 2026 16:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py:78

  • ChannelId._normalize currently returns the original stripped input string as the instance value even when the parsed sub_channel is empty (e.g., "msteams:"), which leaves a trailing ':' in the canonical string value while sub_channel is None. This can lead to serializing "msteams:" without a ProductInfo entity and makes equality/round-trips inconsistent with get_channel/get_sub_channel semantics.
            split = value.split(":", 1)
            channel = split[0].strip()
            if not channel:
                raise ValueError(str(activity_errors.ChannelIdValueMustBeNonEmpty))
            sub_channel = (split[1].strip() or None) if len(split) == 2 else None
            return value, channel, sub_channel

libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py:159

  • Activity.channel_id is now a plain Optional[ChannelId] field, but AgentsModel does not enable assignment validation. As a result, common usage like activity.channel_id = "msteams:sub" will leave a raw str on the model, and later _serialize_sub_channel_data will crash when it accesses self.channel_id.sub_channel. Restoring setter-style coercion for channel_id avoids this runtime error and keeps backward-compatible assignment behavior.
    type: NonEmptyString
    channel_id: Optional[ChannelId] = None
    id: Optional[NonEmptyString] = None
    timestamp: datetime = None
    local_timestamp: datetime = None

Copilot AI review requested due to automatic review settings July 22, 2026 17:27
@rodrigobr-msft
Rodrigo Brandão (rodrigobr-msft) merged commit 8cb8e42 into main Jul 22, 2026
10 of 11 checks passed
@rodrigobr-msft
Rodrigo Brandão (rodrigobr-msft) deleted the users/robrandao/channel-id-2 branch July 22, 2026 17:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py:78

  • ChannelId normalization currently preserves a trailing ':' when the input has no sub-channel (e.g., "msteams:"). This means ChannelId("msteams:") != "msteams", and any code comparing activity.channel_id (a ChannelId/str) to channel constants like "msteams" will fail even though .channel is "msteams". Consider canonicalizing the stored string value to omit the ':' when sub_channel is empty, so equality checks and serialization are consistent.
            split = value.split(":", 1)
            channel = split[0].strip()
            if not channel:
                raise ValueError(str(activity_errors.ChannelIdValueMustBeNonEmpty))
            sub_channel = (split[1].strip() or None) if len(split) == 2 else None
            return value, channel, sub_channel

libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py:54

  • Activity._serialize_sub_channel_data and _validate_channel_id assume self.channel_id is a ChannelId instance (accessing .sub_channel/.channel), but AgentsModel does not enable validate_assignment. With the mixin removed, activity.channel_id = "msteams:copilot-web" will leave a plain str on the model, which can later raise AttributeError during validation/serialization. To preserve the prior setter behavior and avoid runtime crashes, enable assignment validation for Activity (so str assignments are coerced to ChannelId).
class Activity(AgentsModel):

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.

3 participants