-
Notifications
You must be signed in to change notification settings - Fork 0
Fix naming conflict with openAI Agents SDK #63
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
Conversation
WalkthroughThe Changes
Poem
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Too many requests...We've been getting too many requests from your IP address and rate limiting has been triggered. Please try again later. We apologize for the inconvenience. Check our status page or Twitter for real-time updates or reach out to us at [email protected]. ::CLOUDFLARE_ERROR_500S_BOX::
✨ Finishing Touches
🧪 Generate Unit Tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/server/test_server_config.py (1)
289-301
: Make the warning-message assertion less brittleThe test presently expects the exact warning string, which will break on even minor wording tweaks. As long as we produce a warning mentioning the legacy path, the behaviour is correct. Relax the assertion to check that one warning was emitted and that the message contains a key substring.
- # Should log a warning about using legacy structure - mock_warning.assert_called_once_with( - "Using legacy agents directory structure for test_agent. Consider migrating to 'agentuity-agents' directory." - ) + # Should log a warning about using legacy structure + mock_warning.assert_called_once() + assert "legacy agents directory structure" in mock_warning.call_args[0][0]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lock
is excluded by!**/*.lock
📒 Files selected for processing (3)
.gitignore
(1 hunks)agentuity/server/__init__.py
(3 hunks)tests/server/test_server_config.py
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .gitignore
- agentuity/server/init.py
🔇 Additional comments (2)
tests/server/test_server_config.py (2)
9-9
: Import extension looks correctThe additional import of
get_agent_filepath
cleanly wires the new helper into the existing test module without altering ordering or causing circular-imports.
260-279
: Positive test case thoroughly covers preferred-path branchNice job creating both directory trees and asserting the helper chooses the new
agentuity-agents
layout. The test is deterministic and doesn’t rely on monkey-patchingos.path.exists
, so it should remain stable.
agent_name = "My Test-Agent 123!" | ||
|
||
# Create new directory structure with safe name | ||
safe_name = "My_Test_Agent_123_" # Expected safe name transformation | ||
new_dir = tmp_path / "agentuity-agents" / safe_name | ||
new_dir.mkdir(parents=True) | ||
new_file = new_dir / "agent.py" |
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.
🛠️ Refactor suggestion
Derive safe_name
with the same helper as production code
Hard-coding the transformed name risks test failures if the safe_python_name()
implementation evolves (e.g. trimming trailing underscores). Reuse the helper to keep the test self-adapting:
- # Create new directory structure with safe name
- safe_name = "My_Test_Agent_123_" # Expected safe name transformation
+ from agentuity.utils import safe_python_name
+ # Create new directory structure with the computed safe name
+ safe_name = safe_python_name(agent_name)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
agent_name = "My Test-Agent 123!" | |
# Create new directory structure with safe name | |
safe_name = "My_Test_Agent_123_" # Expected safe name transformation | |
new_dir = tmp_path / "agentuity-agents" / safe_name | |
new_dir.mkdir(parents=True) | |
new_file = new_dir / "agent.py" | |
agent_name = "My Test-Agent 123!" | |
from agentuity.utils import safe_python_name | |
# Create new directory structure with the computed safe name | |
safe_name = safe_python_name(agent_name) | |
new_dir = tmp_path / "agentuity-agents" / safe_name | |
new_dir.mkdir(parents=True) | |
new_file = new_dir / "agent.py" |
🤖 Prompt for AI Agents
In tests/server/test_server_config.py around lines 318 to 324, the safe_name
variable is hard-coded instead of using the safe_python_name() helper function
from production code. Replace the hard-coded safe_name assignment with a call to
the safe_python_name() function passing agent_name as input, so the test
dynamically derives the safe name and stays consistent with any future changes
to the helper's implementation.
Fix directory naming conflict with OpenAI Agents SDK
note this cannot be merged/released until the python templates are fixed
Problem
The current codebase uses a generic "agents" directory name which conflicts with the OpenAI agents-sdk, causing potential issues when both SDKs are used in the same project.
Solution
agents/
toagentuity-agents/
to avoid naming conflicts with other agent SDKsconfig_data["filename"]
was being set instead ofagent["filename"]
in the JSON config loading pathsafe_python_name()
function is consistently used for agent directory namesChanges
.gitignore
: Updated to ignore/agentuity-agents/
instead of/agents/
agentuity/server/__init__.py
:load_config()
functionsafe_python_name()
for directory namingImpact
agents/
directory toagentuity-agents/
This change ensures better ecosystem compatibility while maintaining all existing functionality.
Summary by CodeRabbit