Skip to content

Fix FP termination in step3_azure_ai_agent_group_chat.py #10771

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Fix FP termination in step3_azure_ai_agent_group_chat.py #10771

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7bd2958
Stronger termination condition for step3_azure_ai_agent_group_chat.
thecsw Mar 2, 2025
631d367
Merge branch 'microsoft:main' into sandy/step3_azure_ai_agent_group_c…
thecsw Mar 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion python/samples/getting_started_with_agents/azure_ai_agent/step3_azure_ai_agent_group_chat.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,12 @@ class ApprovalTerminationStrategy(TerminationStrategy):

async def should_agent_terminate(self, agent, history):
"""Check if the agent should terminate."""
return "approved" in history[-1].content.lower()
resp = history[-1].content.lower()

# The agent would sometimes respond with simple "Not approved," which
# would trigger the termination. Even if the prompt clearly states not
# to use the word, it fails on 4o. This is a simple check to avoid that.
return "approved" in resp and not "not approved" in resp
Copy link
Contributor

Choose a reason for hiding this comment

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

This probably won't work for all cases. For example, this will trigger the termination too: "It wasn't approved."

I wonder if tuning the REVIEWER INSTRUCTIONS prompt will be a better solution.

Copy link
Author

@thecsw thecsw Mar 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Good note, I tried doing that first with updating the instruction blob with,

If not, provide insight on how to refine suggested copy without example, do NOT say it wasn't simply not approved.

LLMs being LLMs, they don't take commands and would still produce consistently,

# User: a slogan for a new line of electric cars.
# CopyWriter: "Shockingly Smooth. Quietly Powerful."
# ArtDirector: Not approved. While "Shockingly Smooth. Quietly Powerful." plays with the electric theme and contrasts the smoothness and quietness, it feels expected and perhaps too tame for a groundbreaking electric car line. A slogan should reflect the unique essence of the brand—why these cars matter within a crowded market.

etc.

I haven't seen it saying something with "It wasn't approved", not putting it behind the model to generate, though.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can adjust the termination keyword to something like TERMINATE. This can depend on the model used, too -- for example - gpt-4o-mini may handle it differently compared to gpt-4o.

Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to what @TaoChenOSU said, I'm more keen on adjusting the Reviewer's instructions to better communicate the termination criteria -- that's either telling it to switch to using TERMINATE if approved. Or respond with APPROVED and instructing it to use all caps.

I understand we should have samples that work, but this is a sample. :) It should guide users towards what's possible in applications and it doesn't need to be the end-all-be-all.

Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't been seeing issues with the original code. As an exercise, I did the following:

TERMINATATION_KEYWORD = "approved"


class ApprovalTerminationStrategy(TerminationStrategy):
    """A strategy for determining when an agent should terminate."""

    async def should_agent_terminate(self, agent, history):
        """Check if the agent should terminate."""
        return TERMINATATION_KEYWORD in history[-1].content.lower()


REVIEWER_NAME = "ArtDirector"
REVIEWER_INSTRUCTIONS = f"""
You are an art director who has opinions about copywriting born of a love for David Ogilvy.
The goal is to determine if the given copy is acceptable to print.
If the copy is acceptable, state your approval with the single word "{TERMINATATION_KEYWORD}." 
Do not use the word "{TERMINATATION_KEYWORD}" unless you are giving approval. 
If not, provide insight on how to refine suggested copy without example.
There is no need to be nit-picky, if the slogan is acceptable, say so and complete the chat.
"""

I get these types of results:

# AuthorRole.USER: 'a slogan for a new line of electric cars.'
# AuthorRole.ASSISTANT - CopyWriter: '"Drive the Future: Shockingly Efficient."'
# AuthorRole.ASSISTANT - ArtDirector: 'The slogan needs refinement. Consider simplifying the message for clarity and impact. Emphasize the benefits of electric cars more directly, and avoid clichés. Focus on uniqueness and a strong call to action instead.'
# AuthorRole.ASSISTANT - CopyWriter: '"Electric Life: Join the Charge."'
# AuthorRole.ASSISTANT - ArtDirector: 'Approved.'

Copy link
Contributor

Choose a reason for hiding this comment

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

@thecsw I'm not convinced there are updates required for the sample. As I mentioned before, these are supposed to be "getting started" samples and should provide some inspiration for how one can start -- the dev can absolutely take the sample and improve it on their own.

Copy link
Author

Choose a reason for hiding this comment

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

Hi, @moonbox3! Apologies for a delay. Yes, thinking about it more, it works as is. "Stochastic nature of LLMs, etc." sometimes it writes "Not approved" and sometimes it goes through and gives expected nudges. In any case, even if this is an issue—since this is an onboarding script, if they do encounter this, they'd have a great opportunity to debug and learn more.



REVIEWER_NAME = "ArtDirector"
Expand Down
Loading