-
Notifications
You must be signed in to change notification settings - Fork 86
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
Fix: deleting incorrect repo path #426
Conversation
WalkthroughThe changes involve modifications to the Changes
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
evals/eval_swe_bench.py (1)
17-17
: Remove unused importretrieved_edges_to_string
The
retrieved_edges_to_string
function is imported but never used in this file. Consider removing it to maintain clean imports.-from evals.eval_utils import download_github_repo, retrieved_edges_to_string +from evals.eval_utils import download_github_repo
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
evals/eval_swe_bench.py
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (18)
- GitHub Check: run_simple_example_test / test
- GitHub Check: run_dynamic_steps_example_test / test
- GitHub Check: run_notebook_test / test
- GitHub Check: run_notebook_test / test
- GitHub Check: run_notebook_test / test
- GitHub Check: run_multimedia_example_test / test
- GitHub Check: test
- GitHub Check: test
- GitHub Check: test
- GitHub Check: test
- GitHub Check: test
- GitHub Check: test
- GitHub Check: test
- GitHub Check: test
- GitHub Check: test
- GitHub Check: profiler
- GitHub Check: docker-compose-test
- GitHub Check: Publish Cognee Docker image
🔇 Additional comments (1)
evals/eval_swe_bench.py (1)
37-37
: Verify removal of all hardcoded repository pathsSince this PR aims to fix incorrect repository paths, let's verify that all instances have been addressed.
✅ Verification successful
All hardcoded repository paths have been addressed
The verification confirms that the modified line in
evals/eval_swe_bench.py
was the only instance of hardcoded repository paths that needed to be addressed. Other path traversal patterns found in the codebase are either comments or legitimate use cases unrelated to repository paths.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for potential hardcoded repository paths in the codebase echo "Searching for potential hardcoded repository paths..." rg -i "RAW_GIT_REPOS|repo_path\s*=\s*['\"]" --type py # Search for path traversal patterns echo "Searching for path traversal patterns..." rg -i "\.\./" --type pyLength of output: 743
@@ -33,12 +34,11 @@ def check_install_package(package_name): | |||
|
|||
|
|||
async def generate_patch_with_cognee(instance): | |||
"""repo_path = download_github_repo(instance, "../RAW_GIT_REPOS")""" | |||
repo_path = download_github_repo(instance, "../RAW_GIT_REPOS") |
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
Improve repository path handling and add error handling
Several improvements needed for the repository download:
-
The hardcoded relative path "../RAW_GIT_REPOS" is fragile and traverses outside the package directory. Consider:
- Using an absolute path from a configuration
- Using a path within the package directory
- Using a temporary directory
-
Add error handling for download failures
Example implementation:
- repo_path = download_github_repo(instance, "../RAW_GIT_REPOS")
+ try:
+ # Option 1: Use package directory
+ from pathlib import Path
+ package_root = Path(__file__).parent.parent
+ repos_dir = package_root / "data" / "repos"
+ repo_path = download_github_repo(instance, repos_dir)
+
+ # Option 2: Use temporary directory
+ # import tempfile
+ # with tempfile.TemporaryDirectory() as repos_dir:
+ # repo_path = download_github_repo(instance, repos_dir)
+ except Exception as e:
+ raise RuntimeError(f"Failed to download repository: {e}")
Committable suggestion skipped: line range outside the PR's diff.
Summary by CodeRabbit
New Features
Chores