-
Notifications
You must be signed in to change notification settings - Fork 7k
Optimize the selection NextSpeaker mechanism of RolePlayOrchestrator … #6688
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
base: main
Are you sure you want to change the base?
Conversation
…to improve the accuracy of name hits Improvement content: 1. Enforce the use of the JSON standard return format. 2. Introduce a one-time retry mechanism to reduce the error rate (currently, all tests pass 100%).
…to improve the accuracy of name hits Improvement content: 1. Enforce the use of the JSON standard return format. 2. Introduce a one-time retry mechanism to reduce the error rate (currently, all tests pass 100%).
…to improve the accuracy of name hits
var name = response.GetContent() ?? throw new ArgumentException("No name is returned."); | ||
var responseMessageStr = response.GetContent() ?? throw new ArgumentException("No name is returned."); | ||
|
||
RolePlayOrchestratorResponse? responseMessage; |
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.
nit:
var responseMessage = JsonSerializer.Deserialize<RolePlayOrchestratorResponse>(responseMessageStr) ?? throw new InvalidOperationException("Incorrect RolePlayOrchestratorResponse JSON format.");
|
||
var reaginCandidate = candidates.FirstOrDefault(x => x.Name!.ToUpper() == regainResponseMessage.Speaker!.ToUpper()); | ||
|
||
if (reaginCandidate != null) |
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.
reaginCandidate
-> regainCadidate
namespace AutoGen.Core.Orchestrator; | ||
internal class RolePlayOrchestratorResponse | ||
{ | ||
internal string? Speaker { get; set; } |
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 access modifier of property doesn't have to be also internal
here?
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.
public is also fine.
Each message will start with 'From name:', e.g: | ||
From {agentNames.First()}: | ||
//your message//."); | ||
## Available Speaker Names |
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.
Maybe adding a class-level summary on top?
/// <summary>
/// This orchestrator uses a robust two-step strategy to select the next speaker in a roleplay conversation:
/// 1. It first prompts the LLM to select the next speaker from the list of valid candidate names, requiring output in a strict JSON format.
/// 2. If the LLM's chosen name does not exactly match any candidate (e.g., due to hallucination, abbreviation, or formatting issues),
/// the orchestrator issues a second prompt to the LLM, instructing it to map the provided name to the closest valid candidate name from the original list.
/// This approach ensures that the selected speaker always corresponds to an authorized candidate and guards against LLM output errors.
/// </summary>
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.
PR LGTM, minor changes requested before merging
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6688 +/- ##
=======================================
Coverage 79.71% 79.71%
=======================================
Files 232 232
Lines 17323 17323
=======================================
Hits 13809 13809
Misses 3514 3514
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…to improve the accuracy of name hits
Improvement content:
Checks