-
Notifications
You must be signed in to change notification settings - Fork 113
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: notebooks errors #565
Conversation
WalkthroughThe changes introduce an asynchronous method in the NetworkX adapter for initializing and saving an empty graph, refactoring error handling in Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant A as NetworkXAdapter
participant FS as File System
C->>A: load_graph_from_file(file_path)
alt File exists
A->>FS: Read graph file
FS-->>A: Return file content
else File missing
A->>A: create_empty_graph(file_path)
A->>FS: Check or create directory
A->>A: Instantiate MultiDiGraph
A->>FS: Write graph to file
end
A-->>C: Return graph
sequenceDiagram
participant N as Notebook
participant S as SearchService
N->>S: search(query_type=CODE, query_text="def create_graphrag_config")
S-->>N: Return search results
N->>N: Print results
Suggested reviewers
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 (5)
notebooks/cognee_code_graph_demo.ipynb (3)
12-13
: Add input validation for credentials.Consider adding validation for the input values to ensure they are not empty or malformed before setting them as environment variables.
-os.environ["GRAPHISTRY_PASSWORD"] = input("Please enter your graphistry password") -os.environ["OPENAI_API_KEY"] = input("Please enter your OpenAI API key") +graphistry_password = input("Please enter your graphistry password").strip() +openai_key = input("Please enter your OpenAI API key").strip() +if not graphistry_password or not openai_key: + raise ValueError("Credentials cannot be empty") +os.environ["GRAPHISTRY_PASSWORD"] = graphistry_password +os.environ["OPENAI_API_KEY"] = openai_key
79-83
: Document the purpose and impact ofdetailed_extraction
.The
detailed_extraction
flag significantly affects task behavior and batch sizes. Please add documentation explaining:
- The purpose of this flag
- The rationale behind different batch sizes (100 vs 500)
- The impact on performance and resource usage
+# detailed_extraction: When True, performs a more thorough analysis with smaller batch sizes +# to ensure accuracy. When False, uses larger batches for faster processing. detailed_extraction = True tasks = [ Task(get_repo_file_dependencies, detailed_extraction=detailed_extraction), Task(add_data_points, task_config={"batch_size": 100 if detailed_extraction else 500}), ]
126-130
: Enhance search functionality with error handling and result formatting.The current implementation could be improved by:
- Adding error handling for the search operation
- Formatting the results for better readability
- Making the search query configurable
from cognee import search, SearchType -results = await search(query_type=SearchType.CODE, query_text="def create_graphrag_config") - -print(results) +async def perform_code_search(query: str): + try: + results = await search(query_type=SearchType.CODE, query_text=query) + if not results: + print("No results found") + return + print("\nSearch Results:") + print("-" * 50) + for idx, result in enumerate(results, 1): + print(f"\nResult {idx}:") + print(result) + except Exception as e: + print(f"Error during search: {e}") + +# Make the search query configurable +query = "def create_graphrag_config" # Could be taken as input +await perform_code_search(query)cognee/infrastructure/databases/graph/networkx/adapter.py (2)
246-254
: Add docstring and type hints tocreate_empty_graph
.The new method would benefit from documentation and proper type hints.
- async def create_empty_graph(self, file_path: str) -> None: + async def create_empty_graph(self, file_path: str) -> None: + """Create and save an empty MultiDiGraph. + + Args: + file_path (str): Path where the empty graph will be saved + + Raises: + OSError: If directory creation fails + IOError: If saving the graph fails + """ self.graph = nx.MultiDiGraph() file_dir = os.path.dirname(file_path) if not os.path.exists(file_dir): os.makedirs(file_dir, exist_ok=True) await self.save_graph_to_file(file_path)
336-339
: Improve error handling specificity.The current error handling catches all exceptions. Consider catching specific exceptions for better error handling.
- except Exception: + except (json.JSONDecodeError, IOError) as e: logger.error("Failed to load graph from file: %s", file_path) + logger.debug("Error details: %s", str(e)) await self.create_empty_graph(file_path)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lock
is excluded by!**/*.lock
📒 Files selected for processing (4)
cognee/infrastructure/databases/graph/networkx/adapter.py
(2 hunks)notebooks/cognee_code_graph_demo.ipynb
(4 hunks)notebooks/cognee_graphiti_demo.ipynb
(6 hunks)pyproject.toml
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (25)
- GitHub Check: test
- GitHub Check: Test on macos-15
- GitHub Check: Test on macos-15
- GitHub Check: Test on macos-13
- GitHub Check: Test on macos-15
- GitHub Check: Test on macos-13
- GitHub Check: Test on ubuntu-22.04
- GitHub Check: Test on macos-13
- GitHub Check: run_eval_framework_test / test
- GitHub Check: run_multimedia_example_test / test
- GitHub Check: test
- GitHub Check: Test on ubuntu-22.04
- GitHub Check: run_simple_example_test / test
- GitHub Check: Test on ubuntu-22.04
- GitHub Check: run_networkx_metrics_test / test
- GitHub Check: test
- GitHub Check: run_dynamic_steps_example_test / test
- GitHub Check: test
- GitHub Check: run_notebook_test / test
- GitHub Check: test
- GitHub Check: windows-latest
- GitHub Check: run_notebook_test / test
- GitHub Check: run_notebook_test / test
- GitHub Check: run_notebook_test / test
- GitHub Check: docker-compose-test
🔇 Additional comments (4)
notebooks/cognee_code_graph_demo.ipynb (1)
110-110
:❓ Verification inconclusive
Verify default behavior of
render_graph
.The parameters
include_nodes=True
andinclude_labels=True
were removed. Please verify that these are the default values in therender_graph
implementation to ensure the behavior remains unchanged.
🏁 Script executed:
#!/bin/bash # Description: Check the default parameter values in render_graph implementation ast-grep --pattern $'def render_graph($$$) { $$$ }'Length of output: 54
Action Required: Verify Default Behavior of
render_graph
The recent changes removed the explicit parameters (
include_nodes=True
andinclude_labels=True
) from the call, so we need to ensure that therender_graph
implementation still defaults to these values. The initial verification command didn’t yield any useful results. Please run the commands below to extract the function signature and search for the default parameter settings:#!/bin/bash echo "Extracting render_graph function signature..." rg -o "def render_graph\s*\([^)]*\)" -n . echo "Searching for default 'include_nodes=True' setting..." rg -n "include_nodes\s*=\s*True" . echo "Searching for default 'include_labels=True' setting..." rg -n "include_labels\s*=\s*True" .Review the output to confirm that the defaults for
include_nodes
andinclude_labels
are indeed set toTrue
. If they aren’t, please update the implementation accordingly.notebooks/cognee_graphiti_demo.ipynb (1)
226-226
:❓ Verification inconclusive
Verify Python version compatibility.
The Python version has been updated from 3.9.6 to 3.11.8. Please verify that this version is compatible with all project dependencies and requirements.
🌐 Web query:
What are the breaking changes between Python 3.9 and 3.11?
💡 Result:
Python 3.11 introduces several breaking changes compared to Python 3.9 that developers should consider when upgrading:
Enum Behavior Changes
- String formatting with mixed Enums now returns member names instead of values in f-strings/format() when using
str
/int
mixins:Fix: Useclass Foo(str, Enum): BAR = "bar" # Python ≤3.10: "bar" # Python ≥3.11: "Foo.BAR"StrEnum
/IntEnum
instead of manual mixins[2]System Path Handling
- Bytes paths prohibited:
sys.path
no longer accepts byte strings for filesystem paths[3][5]Deprecated Features Removed
1️⃣ Distutils components:
- Removed
bdist_msi
command (deprecated since Python 3.9)[5]2️⃣ Inspect module:
- Removed deprecated functions:
getargspec()
→ Usesignature()
/getfullargspec()
formatargspec()
→ UseSignature
objects[5]Windows Specific Changes
- Import behavior: Default _PTH file removed in Windows distributions to fix relative imports[6]
Other Notable Changes
- Exception formatting: Tracebacks now show exact error locations using
^
markers[1][4]- Math functions: Combines improved precision for large arguments with stricter input validation[5]
- String formatting: New 'z' specifier normalizes signed zeros (
f"{num:z}"
)[3]Upgrade recommendation: Test codebases thoroughly for Enum usage patterns and deprecated feature dependencies before migrating[2][5].
Citations:
- 1: https://www.datacamp.com/blog/whats-new-in-python-311-and-should-you-even-bother-with-it
- 2: https://blog.pecar.me/python-enum
- 3: https://guide.sigtech.com/release-notes/v9-intro/python/python-3-11/
- 4: https://realpython.com/python311-new-features/
- 5: https://docs.python.org/3/whatsnew/3.11.html
- 6: https://discuss.python.org/t/behaviour-change-in-python-3-11-compatibility-break-or-genuine-fix/54729
- 7: https://www.nicholashairs.com/posts/major-changes-between-python-versions/
- 8: https://www.python.org/downloads/release/python-31110/
- 9: https://docs.python.org/3/whatsnew/3.9.html
- 10: https://docs.python.org/3/whatsnew/index.html
Python 3.11.8 Upgrade – Verify Dependency Compatibility
The project’s Python version has been updated from 3.9.6 to 3.11.8. Note that Python 3.11 brings several breaking changes compared to 3.9, including:
- Enum Behavior: Mixed-base Enums now format differently—member names may be used instead of values.
- Deprecated Features Removed: Functions like
getargspec()
and commands related todistutils
(e.g.,bdist_msi
) have been removed.- System Path Handling: The use of byte strings in
sys.path
is prohibited.Please review and test all project dependencies to ensure they are compatible with these changes. In particular, verify that any code or libraries relying on the old Enum behaviors, deprecated functions, or non-standard sys.path usage are updated.
pyproject.toml (2)
113-113
: LGTM!The graphiti extra group is properly defined and follows the same pattern as other extra groups in the file.
89-89
:✅ Verification successful
Verify graphiti-core version compatibility.
Please verify that version 0.7.0 of graphiti-core is the latest stable version and is compatible with the project's requirements.
🌐 Web query:
What is the latest stable version of graphiti-core?
💡 Result:
The latest stable version of graphiti-core is v0.7.0, released approximately five days ago[2][9]. This release introduced support for custom entity types using Pydantic BaseModels during data ingestion[2].
For context:
- The graphiti-core Python library focuses on building temporally-aware knowledge graphs[9].
- Previous versions include v0.6.x (released last week) and v0.5.x (released in early February 2025)[2][4].
Note: While PyPI listings show v0.5.3 as the latest indexed version[4], the project's GitHub repository confirms active development with newer releases[2][9]. Always verify versions directly from official sources when possible[2][9].
Citations:
- 1: https://eclipse.dev/graphiti/download.html
- 2: https://github.com/getzep/graphiti/releases
- 3: https://download.eclipse.org/graphiti/updates/nightly/index.html
- 4: https://pydigger.com/pypi/graphiti-core
- 5: https://projects.eclipse.org/projects/modeling.graphiti/releases/0.19.2
- 6: https://eclipse.dev/graphiti/
- 7: https://www.piwheels.org/project/graphiti-core/
- 8: https://github.com/graphiti-api/graphiti/blob/master/CHANGELOG.md
- 9: https://github.com/getzep/graphiti
- 10: https://help.getzep.com/graphiti/graphiti/overview
Graphiti-core version compatibility confirmed.
After verifying, version 0.7.0 is indeed the latest stable release according to the GitHub repository. The dependency declaration with
^0.7.0
inpyproject.toml
correctly captures this version and allows for compatible minor updates. No further changes are required.
Description
DCO Affirmation
I affirm that all code in every commit of this pull request conforms to the terms of the Topoteretes Developer Certificate of Origin
Summary by CodeRabbit
New Features
Refactor
Chores