Fix/cog 2139 deepeval retry

[malinosqui]

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.

---

This pull request introduces several updates and improvements across multiple files in the kodustech/cognee repository. Key changes include:

1. GitHub Actions Workflow: The Memgraph integration testing workflow is updated to re-enable triggers for pull requests and manual dispatch. Memgraph is integrated as a Docker service, the actions/checkout action is updated to v3, and the Poetry install command now uses the neo4j extra. The test environment is configured to connect to the local Memgraph service using default credentials.

2. Documentation Enhancements:

   • The main README.md is refined to emphasize the project's capability to replace RAG systems, improving clarity and consistency.
   • The cognee-mcp/README.md is enhanced with a direct link to external documentation and a new 'Running the Server' section with command-line instructions for server startup.

3. Code Improvements:

   • In metrics_calculator.py, a check is added to filter out None scores during metrics extraction, preventing errors in numerical calculations.
   • The DeepEvalAdapter in deep_eval_adapter.py now includes a retry mechanism with exponential backoff for metric calculations, encapsulated in a new private method _calculate_metric.
   • The memgraph_adapter.py file sees a method renaming for consistency and the introduction of new methods get_node and get_nodes, which may duplicate existing functionality.
   • Neo4j Cypher queries in neo4j_driver/adapter.py are refactored to use a BASE_LABEL for node matching and creation, with a unique constraint on node.id. However, issues such as an un-awaited asynchronous call and Cypher query errors are identified.
   • Cypher queries in neo4j_metrics_utils.py are updated for performance but pose a security risk due to potential Cypher injection, suggesting the need for parameterized queries.

4. User Module Update: The get_authenticated_user.py file improves JWT payload processing by using safer access methods and default values to prevent KeyError exceptions.

5. Example Scripts:

   • The relational_database_migration_example.py script is refined with better abstraction, improved search functionality, and enhanced user guidance through comments.
   • A new script, relational_db_and_dlt.py, demonstrates an end-to-end data processing pipeline using dlt, SQLite, and Cognee's knowledge graph, with asyncio for orchestration and a patch for SqlalchemyCredentials.

These changes aim to enhance functionality, robustness, and clarity across the project.

[kodus-qa]

Code Review Completed! 🔥

The code review was successfully completed based on your current configurations.

Kody Guide: Usage and Configuration

Interacting with Kody

• Request a Review: Ask Kody to review your PR manually by adding a comment with the @kody start-review command at the root of your PR.

• Provide Feedback: Help Kody learn and improve by reacting to its comments with a 👍 for helpful suggestions or a 👎 if improvements are needed.

Current Kody Configuration

Review Options

The following review options are enabled or disabled:

Options	Enabled
Security	✅
Code Style	✅
Kody Rules	✅
Refactoring	✅
Error Handling	✅
Maintainability	✅
Potential Issues	✅
Documentation And Comments	✅
Performance And Optimization	✅
Breaking Changes	✅

Access your configuration settings here.

​

[kodus-qa]

# In get_num_connected_components, modify the query definition and execution:
# Original f-string query construction:
# query = f"""
# CALL gds.wcc.stats('{graph_name}')
# YIELD componentCount
# RETURN componentCount AS num_connected_components;
# """

# Suggested approach with parameterized query:
query_template = """
CALL gds.wcc.stats($graph_name_param)
YIELD componentCount
RETURN componentCount AS num_connected_components;
"""
# And call the adapter with parameters (exact syntax depends on your Neo4jAdapter):
# result = await adapter.query(query_template, params={"graph_name_param": graph_name})

The graph_name parameter is directly embedded into Cypher queries using f-strings in multiple locations (lines 60 and 184 in neo4j_metrics_utils.py), which can lead to Cypher injection vulnerabilities if graph_name is derived from untrusted input.

This issue appears in multiple locations:

• cognee/infrastructure/databases/graph/neo4j_driver/neo4j_metrics_utils.py: Lines 60-60
• cognee/infrastructure/databases/graph/neo4j_driver/neo4j_metrics_utils.py: Lines 184-184
  Please use parameterized queries for all instances where graph_name is embedded in Cypher queries to prevent injection vulnerabilities.

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[kodus-qa]

        MATCH (predecessor)-[r:`{edge_label}`]->(node:`{BASE_LABEL}` {id: id})

Multiple Cypher queries incorrectly use node IDs as labels and have incorrect relationship direction patterns (lines 734 and 763 in neo4j_driver/adapter.py), which will lead to incorrect graph operations.

This issue appears in multiple locations:

• cognee/infrastructure/databases/graph/neo4j_driver/adapter.py: Lines 734-734
• cognee/infrastructure/databases/graph/neo4j_driver/adapter.py: Lines 763-763
  Please correct all Cypher queries to use proper node labeling with BASE_LABEL and ensure relationship directions match the intended operation (predecessors vs. successors).

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[kodus-qa]

        image: memgraph/memgraph-mage:X.Y.Z # Replace X.Y.Z with a specific stable version

Using the :latest tag for the memgraph/memgraph-mage service image can lead to unexpected build failures if a new latest version introduces breaking changes or incompatibilities. Pinning to a specific image version (e.g., memgraph/memgraph-mage:2.5.0 or another known stable version) ensures more reproducible and stable CI runs.

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[kodus-qa]

            # Ensure score is numeric (int or float)
            if isinstance(score, (int, float)):
                processed_score = float(score) # Convert to float for List[float] consistency
                metrics_data[metric].append(processed_score)
                if "reason" in values:
                    metric_details[metric].append(
                        {
                            "question": entry["question"],
                            "answer": entry["answer"],
                            "golden_answer": entry["golden_answer"],
                            "reason": values["reason"],
                            "score": processed_score, # Use the consistent processed score
                        }
                    )
            # Optional: else if score is not None (i.e., it's non-numeric and not None):
            #     # Log or handle unexpected non-numeric scores
            #     # e.g., import logging; logging.warning(f"Skipping non-numeric score '{score}' for metric '{metric}'.")

The current check if score is not None: effectively filters out None values. However, the function extract_metrics_and_details is type-hinted to produce metrics_data as Dict[str, List[float]]. To ensure type consistency and prevent potential downstream errors if score were a non-None, non-numeric type (e.g., a string), consider also verifying that score is numeric (e.g., int or float) before appending. This would make the data handling more robust and strictly align with the type hints, preventing errors in np.mean and ensuring metrics_data contains only floats.

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[kodus-qa]

                    await asyncio.sleep(2**attempt)  # Exponential backoff

The synchronous time.sleep() on line 43 will block the asyncio event loop because _calculate_metric (defined on line 27) is a synchronous method called from the async method evaluate_answers (call on line 75). To prevent event loop blockage and improve concurrency, _calculate_metric should be converted to an async method. This involves changing its definition to async def, using await asyncio.sleep() instead of time.sleep(), and awaiting the call to self._calculate_metric. An import asyncio statement will be necessary. Furthermore, if metric_to_calculate.measure(test_case) (line 33) performs blocking I/O operations (common for evaluation metrics involving external calls), it should also be handled asynchronously, for instance, by running it in a thread pool executor using await asyncio.get_event_loop().run_in_executor(None, metric_to_calculate.measure, test_case).

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[kodus-qa]

        # This async call needs to be properly handled in the synchronous __init__.
        # For example, by moving it to an async setup method:
        # async def setup_database(self):
        #     await self.query(f"CREATE CONSTRAINT IF NOT EXISTS FOR (n:`{BASE_LABEL}`) REQUIRE n.id IS UNIQUE;")
        # Or by using a synchronous Neo4j call if available for this operation.
        # The current call `self.query(...)` will not execute the constraint creation.

The self.query method is asynchronous, but it's being called from the synchronous __init__ method without being awaited. This means the Cypher query to create the unique constraint on n.id will not actually be executed; self.query(...) will only return a coroutine object. The constraint will not be created in the database. This critical setup step needs to be performed in an async context, for example, by moving it to an async def initialize(self) method that is called and awaited after object instantiation, or by using a synchronous Neo4j driver call if one is available for constraint creation.

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[kodus-qa]

            MATCH (from_node: `{BASE_LABEL}`)-[r:`{edge_label}`]->(to_node: `{BASE_LABEL}`)

The Cypher query's MATCH clause on line 295 defines an anonymous relationship [:{edge_label}]. However, the subsequent RETURN statement (on line 297, which is context code not changed in this diff) refers to COUNT(relationship). This will cause a Cypher error as relationship is not bound. You need to assign a variable to the relationship in the MATCH pattern on line 295, for example, [r:{edge_label}], and then use COUNT(r).

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[kodus-qa]

            user_id_str = payload.get("user_id")
            if not user_id_str: # Check for None or empty string
                raise HTTPException(status_code=401, detail="'user_id' missing or empty in token payload.")
            try:
                user_uuid = UUID(user_id_str)
            except ValueError:
                raise HTTPException(status_code=401, detail="'user_id' in token payload is not a valid UUID.")
            auth_data = SimpleNamespace(id=user_uuid, tenant_id=None, roles=[])

The code on line 32 directly accesses payload["user_id"] and passes it to UUID(). If the user_id key is missing from the JWT payload, this will raise a KeyError. If user_id is present but is an empty string or not a valid UUID format, UUID() will raise a ValueError. Both scenarios would likely lead to unhandled exceptions and a 500 error. It's recommended to use payload.get("user_id") for safer access, then validate its presence and format, raising a specific HTTPException (e.g., 401) if it's missing or invalid. This aligns with the safer handling already implemented for tenant_id.

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[kodus-qa]

        sql_script_path = "examples/relational_db_with_dlt/fix_foreign_keys.sql"
        try:
            with open(sql_script_path, "r", encoding="utf-8") as f:
                sql_script_content = f.read()
            raw_conn.executescript(sql_script_content)
            print("[apply_foreign_key_fixes] Applied foreign key fixes")
        except FileNotFoundError:
            print(f"[apply_foreign_key_fixes] Warning: '{sql_script_path}' not found. Foreign key fixes skipped.")
        except Exception as e: # Catch other potential errors from executescript
            print(f"[apply_foreign_key_fixes] Error applying fixes from '{sql_script_path}': {e}. Fixes might be incomplete.")

The script attempts to open and execute fix_foreign_keys.sql without explicit error handling for FileNotFoundError. If the SQL file is missing or not accessible, the script will raise an unhandled FileNotFoundError and terminate. It's recommended to wrap the file operations and SQL execution in a try-except block to gracefully handle cases where the file cannot be found, allowing the script to potentially continue or exit with a more informative message.

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​