Skip to content

Conversation

@SongChiYoung
Copy link
Contributor

Why are these changes needed?

SocietyOfMindAgent has multiple system message, however many client/model does not support it.

Related issue number

Related #6290

Checks

@codecov
Copy link

codecov bot commented Apr 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.91%. Comparing base (4a5dd9e) to head (aa3ef16).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6327      +/-   ##
==========================================
+ Coverage   77.90%   77.91%   +0.01%     
==========================================
  Files         209      209              
  Lines       15180    15187       +7     
==========================================
+ Hits        11826    11833       +7     
  Misses       3354     3354              
Flag Coverage Δ
unittests 77.91% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 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.

@ekzhu ekzhu enabled auto-merge (squash) April 18, 2025 05:14
@ekzhu ekzhu merged commit b13264a into microsoft:main Apr 18, 2025
60 checks passed
ekzhu added a commit that referenced this pull request Apr 21, 2025
…system_messages”] instead of `startswith("gemini-")` (#6345)

The current implementation of consecutive `SystemMessage` merging
applies only to models where `model_info.family` starts with
`"gemini-"`.

Since PR #6327 introduced the `multiple_system_messages` field in
`model_info`, we can now generalize this logic by checking whether the
field is explicitly set to `False`.

This change replaces the hardcoded family check with a conditional that
merges consecutive `SystemMessage` blocks whenever
`multiple_system_messages` is set to `False`.

Test cases that previously depended on the `"gemini"` model family have
been updated to reflect this configuration flag, and renamed accordingly
for clarity.

In addition, for consistency across conditional logic, a follow-up PR is
planned to refactor the Claude-specific transformation condition
(currently implemented via `create_args.get("model",
"unknown").startswith("claude-")`)
to instead use the existing `is_claude()`.

Co-authored-by: Eric Zhu <ekzhu@users.noreply.github.com>
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.

2 participants