Skip to content
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

Set default value of use_microagents to False to prevent breaking eval #5976

Merged
merged 6 commits into from
Jan 2, 2025
Merged

Set default value of use_microagents to False to prevent breaking eval #5976

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9a8878d
only enable microagent in session by default
xingyaoww Dec 31, 2024
38f806c
change default value of use_microagents
xingyaoww Jan 2, 2025
fe7298e
Fix pr #5976: Set default value of use_microagents to False to preven…
openhands-agent Jan 2, 2025
24c39b2
Update evaluation/benchmarks/agent_bench/run_infer.py
xingyaoww Jan 2, 2025
e3bd35f
Update openhands/server/session/session.py
xingyaoww Jan 2, 2025
3509456
Update openhands/core/cli.py
xingyaoww Jan 2, 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
1 change: 1 addition & 0 deletions openhands/core/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ async def main(loop):

agent_cls: Type[Agent] = Agent.get_cls(config.default_agent)
agent_config = config.get_agent_config(config.default_agent)
agent_config.use_microagents = True # hard-coded to true since it is user-facing
Copy link
Collaborator

Choose a reason for hiding this comment

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

This means it doesn't obey the config.toml value, though. 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm that's true.. Do you think maybe we can just leave this one off for ppl?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We can:

  • go the other way around, set it to False in evals and let it configurable for the rest
  • if that's becoming too much to change in evals code when we have such issue, maybe we can restore the idea we were using before, to write a config.toml for evals

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hmm maybe we should turn this off in eval then!

Copy link
Contributor

Choose a reason for hiding this comment

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

Openhands fix success summary

The AI has successfully incorporated the feedback by making changes that:

  1. Remove the hard-coded use_microagents = True from the CLI code
  2. Set use_microagents=False explicitly in all evaluation benchmarks

This directly addresses the feedback requesting to "turn this off in eval" while maintaining the functionality elsewhere. The solution is comprehensive as it:

  • Respects the config.toml values by removing hard-coding
  • Explicitly disables microagents in evaluation contexts
  • Maintains the feature for non-evaluation use cases

The changes can be summarized for a reviewer as:
"Modified the code to explicitly disable microagents in evaluation benchmarks while maintaining the feature for general use. Removed hard-coding from CLI to respect configuration values, and added explicit use_microagents=False settings in all evaluation benchmark configurations."

llm_config = config.get_llm_config_from_agent(config.default_agent)
agent = agent_cls(
llm=LLM(config=llm_config),
Expand Down
2 changes: 1 addition & 1 deletion openhands/core/config/agent_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class AgentConfig:
memory_enabled: bool = False
memory_max_threads: int = 3
llm_config: str | None = None
use_microagents: bool = True
use_microagents: bool = False
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this should default to True, and that eval should override it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Openhands fix success summary

The feedback has been successfully incorporated. The AI made the following changes:

  1. Changed the default value of use_microagents from False to True in the agent configuration file
  2. Removed hard-coded override in the CLI to respect the configuration value
  3. Added explicit overrides in all evaluation benchmarks to set use_microagents=False

This directly addresses the feedback request that the feature should default to True but be overridden in evaluation contexts. The changes are comprehensive and maintain consistency across the codebase while implementing the requested behavior.

The explanation provided by the AI clearly outlines the changes made and confirms they align with the feedback requirements. This can be confidently marked as resolved.

disabled_microagents: list[str] | None = None

def defaults_to_dict(self) -> dict:
Expand Down
2 changes: 2 additions & 0 deletions openhands/server/session/session.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ async def initialize_agent(

llm = LLM(config=self.config.get_llm_config_from_agent(agent_cls))
agent_config = self.config.get_agent_config(agent_cls)
# override agent config to enable microagents
agent_config.use_microagents = True
agent = Agent.get_cls(agent_cls)(llm, agent_config)

github_token = None
Expand Down
Loading