Skip to content

chore: remove stale TODO and legacy comments (#2316)#2345

Open
ajuijas wants to merge 2 commits intotopoteretes:mainfrom
ajuijas:fix/remove-stale-todos-2316
Open

chore: remove stale TODO and legacy comments (#2316)#2345
ajuijas wants to merge 2 commits intotopoteretes:mainfrom
ajuijas:fix/remove-stale-todos-2316

Conversation

@ajuijas
Copy link

@ajuijas ajuijas commented Mar 9, 2026

Description

This PR resolves issue #2316 by fully auditing and triaging the TODO comments across the codebase.

Changes:

  1. Removes Stale TODOs: Deleted 6 outdated TODO comments related to legacy caching and exception handling that were no longer relevant.
  2. References New GitHub Issues: For the 9 remaining actionable TODOs, we created dedicated GitHub issues. I then updated the inline comments in the Python files to reference these new issue IDs (e.g., changing TODO: to TODO(#2350):).

This cleans up the code and makes sure all pending work is tracked in GitHub issues instead.

Acceptance Criteria

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Code refactoring
  • Other (please specify):

Pre-submission Checklist

  • I have tested my changes thoroughly before submitting this PR (See CONTRIBUTING.md)
  • This PR contains minimal changes necessary to address the issue/feature
  • My code follows the project's coding standards and style guidelines
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if applicable)
  • All new and existing tests pass
  • I have searched existing PRs to ensure this change hasn't been submitted already
  • I have linked any relevant issues in the description
  • My commits have clear and descriptive messages

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.

- Removed 5 legacy caching TODOs from cache_db_interface.py, test_fs_adapter_crud.py, and test_redis_adapter_crud.py referencing backward compatibility that was completed in PR topoteretes#2077.

- Removed 1 outdated dynamic exception handling TODO attached to dead code in exceptions.py from PR topoteretes#567.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 9, 2026

Walkthrough

Mostly comment/TODO text updates across multiple modules and tests; one behavioral-affecting change: removal of the base-class initializer call from EntityNotFoundError.init in exceptions.py.

Changes

Cohort / File(s) Summary
Comment/TODO updates (multiple modules)
cognee/api/v1/cognify/cognify.py, cognee/context_global_variables.py, cognee/infrastructure/databases/graph/neo4j_driver/.../Neo4jAuraDevDatasetDatabaseHandler.py, cognee/infrastructure/databases/hybrid/neptune_analytics/NeptuneAnalyticsAdapter.py, cognee/infrastructure/databases/vector/chromadb/ChromaDBAdapter.py, cognee/infrastructure/llm/structured_output_framework/.../openai/adapter.py, cognee/tests/integration/retrieval/test_rag_completion_retriever.py, cognee/tests/test_cognee_server_start.py, cognee/tests/unit/entity_extraction/regex_entity_extraction_test.py, examples/pocs/single_add_datapoints/single_add_datapoints_pipeline.py
Updated inline comments / TODOs to reference issue numbers or reformat text; no functional or signature changes.
Cache interface documentation
cognee/infrastructure/databases/cache/cache_db_interface.py
Removed backward-compatibility ":TODO: delete when retrievers are updated" notes from three docstrings; behavior unchanged.
Exception constructor change
cognee/infrastructure/databases/exceptions/exceptions.py
Removed super().__init__(message, name, status_code) call from EntityNotFoundError.init; initializer now only assigns instance attributes (possible behavioral/initialization impact).
Test comment cleanup
cognee/tests/unit/infrastructure/databases/cache/test_fs_adapter_crud.py, cognee/tests/unit/infrastructure/databases/cache/test_redis_adapter_crud.py
Deleted backward-compatibility header comment lines; test logic unchanged.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • Merge main vol7 #1891: Touches cognee/api/v1/cognify/cognify.py's get_default_tasks (related to TODO/comment edits in same function).

Suggested labels

run-checks

Suggested reviewers

  • lxobr
  • dexters1
  • Vasilije1990
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The PR title 'chore: remove stale TODO and legacy comments (#2316)' clearly and concisely summarizes the main change: removing outdated TODO comments to address issue #2316.
Docstring Coverage ✅ Passed Docstring coverage is 82.35% which is sufficient. The required threshold is 80.00%.
Description check ✅ Passed The PR description is complete and follows the template structure with all required sections filled out, including clear explanation of changes, acceptance criteria, type of change, pre-submission checklist confirmation, and DCO affirmation.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
cognee/infrastructure/databases/cache/cache_db_interface.py (1)

56-56: LGTM! TODO cleanup aligns with PR objectives.

The removal of stale TODO comments from these backward-compatibility wrapper docstrings is appropriate and keeps the documentation concise. The functional delegation logic remains unchanged.


Optional enhancement for future consideration:

These backward-compatibility wrapper methods could benefit from more complete docstrings documenting their parameters and return values, similar to the abstract methods they delegate to (e.g., create_qa_entry, get_latest_qa_entries, get_all_qa_entries). However, this is out of scope for this cleanup PR.

As per coding guidelines: "undocumented function definitions and class definitions in the project's Python code are assumed incomplete. Please consider suggesting a short summary of the code for any of these incomplete definitions as docstrings when reviewing."

Also applies to: 85-85, 96-96

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cognee/infrastructure/databases/cache/cache_db_interface.py` at line 56, Add
brief, properly formatted docstrings to the backward-compatibility wrapper
methods that currently lack documentation (the wrapper that delegates to
create_qa_entry and the similar wrappers for get_latest_qa_entries and
get_all_qa_entries) describing their parameters, return value, and that they
delegate to the corresponding abstract methods; reference the wrapper method
names (the create_qa_entry wrapper, get_latest_qa_entries wrapper,
get_all_qa_entries wrapper) and mirror the essential param/return info from the
delegated abstract methods to satisfy the project's undocumented-definition
guideline.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@cognee/infrastructure/databases/cache/cache_db_interface.py`:
- Line 56: Add brief, properly formatted docstrings to the
backward-compatibility wrapper methods that currently lack documentation (the
wrapper that delegates to create_qa_entry and the similar wrappers for
get_latest_qa_entries and get_all_qa_entries) describing their parameters,
return value, and that they delegate to the corresponding abstract methods;
reference the wrapper method names (the create_qa_entry wrapper,
get_latest_qa_entries wrapper, get_all_qa_entries wrapper) and mirror the
essential param/return info from the delegated abstract methods to satisfy the
project's undocumented-definition guideline.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 3c3d756d-0d69-4736-a41c-e69675e4b917

📥 Commits

Reviewing files that changed from the base of the PR and between c9370a8 and df8c437.

📒 Files selected for processing (4)
  • cognee/infrastructure/databases/cache/cache_db_interface.py
  • cognee/infrastructure/databases/exceptions/exceptions.py
  • cognee/tests/unit/infrastructure/databases/cache/test_fs_adapter_crud.py
  • cognee/tests/unit/infrastructure/databases/cache/test_redis_adapter_crud.py
💤 Files with no reviewable changes (3)
  • cognee/tests/unit/infrastructure/databases/cache/test_redis_adapter_crud.py
  • cognee/infrastructure/databases/exceptions/exceptions.py
  • cognee/tests/unit/infrastructure/databases/cache/test_fs_adapter_crud.py

@pull-checklist
Copy link

Please make sure all the checkboxes are checked:

  • I have tested these changes locally.
  • I have reviewed the code changes.
  • I have added end-to-end and unit tests (if applicable).
  • I have updated the documentation and README.md file (if necessary).
  • I have removed unnecessary code and debug statements.
  • PR title is clear and follows the convention.
  • I have tagged reviewers or team members for feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant