fix: guard against non-LLM recipients in speak_to tool(#158)#161
fix: guard against non-LLM recipients in speak_to tool(#158)#161BhoomiAgrawal12 wants to merge 4 commits intomesa:mainfrom
Conversation
for more information, see https://pre-commit.ci
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #161 +/- ##
==========================================
+ Coverage 90.08% 90.67% +0.59%
==========================================
Files 19 19
Lines 1503 1545 +42
==========================================
+ Hits 1354 1401 +47
+ Misses 149 144 -5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Fixes a crash in speak_to when recipients include non-LLM agents that don’t implement the expected memory interface.
Changes:
- Added a guard in
speak_toto skip recipients without amemoryattribute and log a warning. - Added a unit test to ensure
speak_todoesn’t crash when a non-LLM recipient is included.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
mesa_llm/tools/inbuilt_tools.py |
Skips recipients missing memory and emits a warning instead of crashing. |
tests/test_tools/test_inbuilt_tools.py |
Adds regression coverage for non-LLM recipients lacking memory. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
|
Thanks for your work ! Can you address the copilot suggestions (accept/refuse with justification), we can probably merge soon :) |
@colinfrisch thanks for approving! I have addressed the copilot suggestion of adding another test. |
f7a2213 to
d33b841
Compare
for more information, see https://pre-commit.ci
Summary
speak_tocrashed withAttributeErrorwhen any of its named recipients was a non-LLM agent with no memory attribute.speak_tobuilds its recipient list fromagent.model.agentsand does not require recipients to be LLMAgent instances.The subsequent
recipient.memory.add_to_memory()call then crashed on any agentthat was not an
LLMAgent.Bug / Issue
Fixes #158
Implementation
hasattrguard insidespeak_to's recipient loop.Testing
New test added in
tests/test_tools/test_inbuilt_tools.pyand all previous 11 tests passed as well.Additional Notes
Existing
test_speak_to_records_on_recipientsis unchanged and still passes.