-
Notifications
You must be signed in to change notification settings - Fork 5.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Makes select_speaker more robust by checking for mentions anywhere. #669
Conversation
…where in the selection string. Addresses 663.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #669 +/- ##
==========================================
+ Coverage 28.89% 37.36% +8.46%
==========================================
Files 27 27
Lines 3395 3407 +12
Branches 764 767 +3
==========================================
+ Hits 981 1273 +292
+ Misses 2342 2017 -325
- Partials 72 117 +45
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change looks good to me.
I made a suggestion to use "set", but feel free to ignore it.
Thanks. I'm happy either way, but personally find complex list compressions take longer to read/understand. If it's all the same to you, I'd rather just leave it step by step. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a test to cover the changed function?
…er to own function.
Yes done. To test this effectively, I needed to separate some of the logic out as a helper function |
) # Finds agent mentions, taking word boundaries into account | ||
count = len(re.findall(regex, " " + message_content + " ")) # Pad the message to help with matching | ||
if count > 0: | ||
mentions[agent.name] = count |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Early stop is still applicable here.
…icrosoft#669) * Makes select_speaker more robust by checking for agents mentioned anywhere in the selection string. Addresses 663. * Added test coverage for group chat mentions. Refactored mention counter to own function. * Fixed pre-commit formatting.
Co-authored-by: Jack Gerrits <[email protected]>
Makes select_speaker more robust by checking for agent name mentions anywhere in the string returned by the LLM.
Why are these changes needed?
Selecting the speaker fails when the LLM returns anything other than an exact agent name.
Related issue number
Closes #663, and perhaps a few others.
Edit: I just saw #668, which is a different problem, but might be related?
Checks