Refactor AgnoFramework to generate agent.py programmatically #4
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Modified AgnoFramework to construct agent.py by creating Agno Agent and Team objects directly, rather than concatenating Python code strings for decorators. - This improves the structure and readability of the generated agent code for the Agno framework. - No significant changes were made to FastAgentFramework as its decorator-based string generation appears to be the intended integration method. - Corrected minor issues identified during testing, including an import error and a variable name mismatch in generated Agno code. - Removed deprecated helper methods from AgnoFramework.
Reviewer's GuideRefactor AgnoFramework’s build_agent_content to generate agent.py via native Agno Agent and Team object construction, introduce helper methods for mapping models and tools, fix import path and naming bugs, and remove outdated helpers. Sequence diagram for agent.py generation in AgnoFrameworksequenceDiagram
participant User
participant AgnoFramework
participant Agent
participant Team
User->>AgnoFramework: build_agent_content()
AgnoFramework->>AgnoFramework: _get_model_import_path()
AgnoFramework->>AgnoFramework: _get_tool_import_path_and_init_str()
AgnoFramework->>Agent: Construct Agent objects (for each agent)
alt Multiple agents
AgnoFramework->>Team: Construct Team object with Agent members
end
AgnoFramework->>User: Return generated agent.py content
Class diagram for refactored AgnoFramework and helpersclassDiagram
class AgnoFramework {
+_get_model_import_path(model_id: str) str
+_get_tool_import_path_and_init_str(server_name: str, server_config: Any) tuple[str, str]
+build_agent_content() str
+get_requirements() List[str]
}
AgnoFramework --|> BaseFramework
class BaseFramework {
<<abstract>>
}
Class diagram for generated agent.py structure (Agent and Team)classDiagram
class Agent {
+name: str
+instructions: str
+model: Model
+tools: List[Tool]
+markdown: bool
+add_datetime_to_instructions: bool
+add_history_to_messages: bool
+human_input: bool
+role: str
}
class Team {
+name: str
+mode: str
+model: Model
+members: List[Agent]
+tools: List[Tool]
+instructions: List[str]
+markdown: bool
+show_members_responses: bool
+enable_agentic_context: bool
+add_datetime_to_instructions: bool
+success_criteria: str
}
Team "1" -- "*" Agent : members
class Model {
+id: str
+api_key: str
+base_url: str
}
class Tool
Agent "*" -- "*" Tool
Team "*" -- "*" Tool
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the AgnoFramework to generate agent.py programmatically, moving away from manual string concatenation to a more structured object-based approach. Key changes include:
- Implementation of helper methods (_get_model_import_path and _get_tool_import_path_and_init_str) for determining import paths.
- Reconstruction of agent and team definitions using programmatic dictionaries and dynamic code generation.
- Minor fixes including corrected import paths and variable name adjustments.
Comments suppressed due to low confidence (2)
src/agentman/frameworks/agno.py:167
- [nitpick] The 'mode' value contains extraneous quotes; consider specifying it as "coordinate" to prevent nested quoting issues in the generated code.
"mode": "'coordinate'", # Default mode
src/agentman/frameworks/agno.py:181
- [nitpick] The 'success_criteria' field includes additional quotes; using a plain string ("The team has provided a complete and accurate response.") would improve clarity.
"success_criteria": "'The team has provided a complete and accurate response.'",
src/agentman/frameworks/agno.py
Outdated
| team_var_name = "agentteam" # Default team name, matching test expectation | ||
| main_entity_var_name = team_var_name | ||
|
|
||
| team_model_str = self.config.agents.values().__iter__().__next__().model or default_model_str # Use first agent's model or default |
There was a problem hiding this comment.
Consider using next(iter(self.config.agents.values())) instead of iter().next() to improve code readability and clarity.
Suggested change
| team_model_str = self.config.agents.values().__iter__().__next__().model or default_model_str # Use first agent's model or default | |
| team_model_str = next(iter(self.config.agents.values())).model or default_model_str # Use first agent's model or default |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @yeahdongcn - I've reviewed your changes - here's some feedback:
- Consider abstracting the model and tool import/initialization mappings into a registry or configuration to avoid hardcoding logic in
_get_model_import_pathand_get_tool_import_path_and_init_strand simplify future extensions. - Reduce repetition in constructing agent and team definition strings by introducing helper functions or using a templating approach to improve maintainability and readability of the code generator.
- Ensure that multi-line instruction strings are properly escaped or wrapped to prevent syntax errors when input contains quotes or other special characters.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider abstracting the model and tool import/initialization mappings into a registry or configuration to avoid hardcoding logic in `_get_model_import_path` and `_get_tool_import_path_and_init_str` and simplify future extensions.
- Reduce repetition in constructing agent and team definition strings by introducing helper functions or using a templating approach to improve maintainability and readability of the code generator.
- Ensure that multi-line instruction strings are properly escaped or wrapped to prevent syntax errors when input contains quotes or other special characters.
## Individual Comments
### Comment 1
<location> `src/agentman/frameworks/agno.py:64` </location>
<code_context>
- f' name="{agent.name}",',
- f' instructions="""{agent.instruction}""",',
- ])
+ default_model_str = self.config.default_model or "anthropic/claude-3-sonnet-20241022" # Sensible default
+
+ for agent_config in self.config.agents.values():
</code_context>
<issue_to_address>
Hardcoding a default model string may not be robust for all use cases.
Consider raising an error or warning if neither the agent nor the config specifies a model, to prevent unintended use of an inappropriate default.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| f' name="{agent.name}",', | ||
| f' instructions="""{agent.instruction}""",', | ||
| ]) | ||
| default_model_str = self.config.default_model or "anthropic/claude-3-sonnet-20241022" # Sensible default |
There was a problem hiding this comment.
suggestion: Hardcoding a default model string may not be robust for all use cases.
Consider raising an error or warning if neither the agent nor the config specifies a model, to prevent unintended use of an inappropriate default.
src/agentman/frameworks/agno.py
Outdated
Comment on lines
18
to
21
| elif "openai" in model_lower or "gpt" in model_lower: | ||
| return "agno.models.openai.OpenAILike" | ||
| elif "/" in model_lower: # ollama/llama3, groq/mixtral etc. | ||
| # For now, these are often OpenAILike compatible |
There was a problem hiding this comment.
suggestion (code-quality): We've found these issues:
- Merge duplicate blocks in conditional (
merge-duplicate-blocks) - Remove redundant conditional (
remove-redundant-if)
Suggested change
| elif "openai" in model_lower or "gpt" in model_lower: | |
| return "agno.models.openai.OpenAILike" | |
| elif "/" in model_lower: # ollama/llama3, groq/mixtral etc. | |
| # For now, these are often OpenAILike compatible | |
| elif "openai" in model_lower or "gpt" in model_lower or "/" in model_lower: |
src/agentman/frameworks/agno.py
Outdated
| """Determines the import path and initialization string for a server/tool.""" | ||
| # This mapping should be expanded based on Agno's available tools | ||
| # and how they map to Agentfile server types. | ||
| if server_name in ["web_search", "search", "browser"]: |
There was a problem hiding this comment.
issue (code-quality): Use set when checking membership of a collection of literals [×5] (collection-into-set)
src/agentman/frameworks/agno.py
Outdated
Comment on lines
134
to
136
| script_lines.append("\n# Load environment variables from .env file") | ||
| script_lines.append("load_dotenv()\n") | ||
|
|
There was a problem hiding this comment.
suggestion (code-quality): We've found these issues:
- Merge consecutive list appends into a single extend (
merge-list-appends-into-extend) - Low code quality found in AgnoFramework.build_agent_content - 7% (
low-code-quality)
Suggested change
| script_lines.append("\n# Load environment variables from .env file") | |
| script_lines.append("load_dotenv()\n") | |
| script_lines.extend( | |
| ("\n# Load environment variables from .env file", "load_dotenv()\n") | |
| ) |
Explanation
The quality score for this function is below the quality threshold of 25%.
This score is a combination of the method length, cognitive complexity and working memory.
How can you solve this?
It might be worth refactoring this function to make it shorter and more readable.
- Reduce the function length by extracting pieces of functionality out into
their own functions. This is the most important thing you can do - ideally a
function should be less than 10 lines. - Reduce nesting, perhaps by introducing guard clauses to return early.
- Ensure that variables are tightly scoped, so that code using related concepts
sits together within the function rather than being scattered.
- Modified AgnoFramework to construct agent.py by creating Agno Agent and Team objects directly, rather than concatenating Python code strings for decorators. - This improves the structure and readability of the generated agent code for the Agno framework. - Addressed review comments from PR #4: - Used repr() for robust string literal quoting in generated code. - Improved readability of iterator usage. - Simplified conditional logic and list-to-set conversions for minor performance/idiomatic improvements. - Merged consecutive list appends. - No significant changes were made to FastAgentFramework as its decorator-based string generation appears to be the intended integration method. - Ensured all tests pass after these modifications.
Owner
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey @yeahdongcn - I've reviewed your changes - here's some feedback:
- Consider extracting the repeated logic for building model initialization parameters into a dedicated helper method to simplify and DRY out build_agent_content.
- Building multiline Python code via manual string concatenation is brittle—consider using a templating engine or Python’s AST module to generate code more reliably.
- You’re wrapping instructions in triple quotes verbatim; it would be safer to sanitize or escape any internal triple-quote sequences to avoid generating invalid Python code.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider extracting the repeated logic for building model initialization parameters into a dedicated helper method to simplify and DRY out build_agent_content.
- Building multiline Python code via manual string concatenation is brittle—consider using a templating engine or Python’s AST module to generate code more reliably.
- You’re wrapping instructions in triple quotes verbatim; it would be safer to sanitize or escape any internal triple-quote sequences to avoid generating invalid Python code.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| # Enhanced imports based on features needed | ||
| imports = [ | ||
| def build_agent_content(self) -> str: |
There was a problem hiding this comment.
issue (code-quality): We've found these issues:
- Merge duplicate blocks in conditional [×2] (
merge-duplicate-blocks) - Use named expression to simplify assignment and conditional (
use-named-expression) - Remove redundant conditional [×2] (
remove-redundant-if) - Low code quality found in AgnoFramework.build_agent_content - 8% (
low-code-quality)
Explanation
The quality score for this function is below the quality threshold of 25%.
This score is a combination of the method length, cognitive complexity and working memory.
How can you solve this?
It might be worth refactoring this function to make it shorter and more readable.
- Reduce the function length by extracting pieces of functionality out into
their own functions. This is the most important thing you can do - ideally a
function should be less than 10 lines. - Reduce nesting, perhaps by introducing guard clauses to return early.
- Ensure that variables are tightly scoped, so that code using related concepts
sits together within the function rather than being scattered.
…eedback - Refactored AgnoFramework to generate agent.py by programmatically constructing Agno Agent and Team objects, improving code structure and readability. - Validated FastAgentFramework's existing decorator-based code generation against official documentation (fast-agent.ai), confirming it aligns with the library's design. No major changes were made to its agent.py generation logic based on this. - Incorporated feedback from PR #4 for AgnoFramework: - Used repr() for robust string literal quoting. - Improved iterator usage for readability. - Simplified conditional logic and list-to-set conversions. - Merged consecutive list appends. - Ensured all tests pass after these modifications, confirming successful integration and no regressions.
Copilot AI
added a commit
that referenced
this pull request
Jul 2, 2025
…l mapping, enhance code structure Co-authored-by: yeahdongcn <2831050+yeahdongcn@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Refactor AgnoFramework to build agent.py via structured Agent and Team object instantiation instead of concatenating code strings, improving readability and maintainability; fix import and variable mismatches; remove deprecated methods.
New Features:
Bug Fixes:
Enhancements:
Chores: