Skip to content

Conversation

@randombet
Copy link
Collaborator

@randombet randombet commented Oct 16, 2025

Why are these changes needed?

The content normalization pattern is repeated ~15+ times across the codebase. Extract this into a helper function to improve maintainability.

Locations: chat.py:136-138, conversable_agent.py:1082-1087, conversable_agent.py:3984-3987, group_tool_executor.py:208, enforcer.py (multiple), groupchat.py (multiple)

Related issue number

Checks

@joggrbot
Copy link

joggrbot bot commented Oct 16, 2025

📝 Documentation Analysis

All docs are up to date! 🎉


✅ Latest commit analyzed: 044f5df | Powered by Joggr

@randombet randombet changed the title Replace duplicate logic with normalize_content and normalize_message_… Replace duplicate logic with normalize_content and normalize_message_to_dict Oct 16, 2025
@codecov
Copy link

codecov bot commented Oct 16, 2025

Codecov Report

❌ Patch coverage is 81.81818% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
autogen/agentchat/group/safeguards/enforcer.py 20.00% 4 Missing ⚠️
Files with missing lines Coverage Δ
autogen/agentchat/chat.py 74.07% <100.00%> (+0.16%) ⬆️
autogen/agentchat/conversable_agent.py 62.53% <100.00%> (-1.57%) ⬇️
autogen/agentchat/group/group_tool_executor.py 88.78% <100.00%> (+1.16%) ⬆️
autogen/agentchat/utils.py 88.57% <100.00%> (+1.33%) ⬆️
autogen/agentchat/group/safeguards/enforcer.py 23.67% <20.00%> (+0.30%) ⬆️

... and 40 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@randombet randombet marked this pull request as ready for review October 20, 2025 17:31

def test_normalize_content_with_string():
"""Test normalize_content with string input."""
from autogen.agentchat.utils import normalize_content
Copy link
Member

Choose a reason for hiding this comment

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

Why do you import function in each test? Can we import it on top of the file?

assert original_str == "Original string"


if __name__ == "__main__":
Copy link
Member

Choose a reason for hiding this comment

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

Please, remove manual-run block from the test file

Copy link
Member

@Lancetnik Lancetnik left a comment

Choose a reason for hiding this comment

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

There are a lot of pretty same tests for each function. Can we group some of them to parametrized run?

@pytest.mark.parametrize(
    ("value", "expected"),
    (
        pytest.param("Hello world", "Hello world", id="regular string")
        pytest.param("", "", id="emepty string")
        pytest.param(None, "", id="none")
        ...  # etc
    )
)
def test_normalize_content(value: Any, expected: str) -> None:
    assert normalize_content(value) == expected

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