Add basic tui #1826
Add basic tui #1826
Conversation
Please make sure all the checkboxes are checked:
|
WalkthroughThe PR removes permission validation from Cognify and cascade graph task pipelines, deletes the unused check_permissions_on_dataset module, adds a new TUI entry point to the CLI for lightweight terminal UI access, and extends .gitignore to exclude the .venvsource directory. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
cognee/api/v1/cognify/cognify.py (2)
113-121: Update docstring to reflect removed permission validation step.The docstring lists "Permission Validation" as step 2 in the processing pipeline (line 115), but the
check_permissions_on_datasettask has been removed from bothget_default_tasks()andget_temporal_tasks(). This creates a misleading documentation-code mismatch.Apply this diff to update the docstring:
Processing Pipeline: 1. **Document Classification**: Identifies document types and structures - 2. **Permission Validation**: Ensures user has processing rights - 3. **Text Chunking**: Breaks content into semantically meaningful segments - 4. **Entity Extraction**: Identifies key concepts, people, places, organizations - 5. **Relationship Detection**: Discovers connections between entities - 6. **Graph Construction**: Builds semantic knowledge graph with embeddings - 7. **Content Summarization**: Creates hierarchical summaries for navigation + 2. **Text Chunking**: Breaks content into semantically meaningful segments + 3. **Entity Extraction**: Identifies key concepts, people, places, organizations + 4. **Relationship Detection**: Discovers connections between entities + 5. **Graph Construction**: Builds semantic knowledge graph with embeddings + 6. **Content Summarization**: Creates hierarchical summaries for navigation
333-369: Update docstring to reflect removed permission check.The docstring (lines 340-342) mentions "Dataset permission checks (requires 'write' access)" as step 2, but the
check_permissions_on_datasettask has been removed from the temporal tasks pipeline.Apply this diff to update the docstring:
The pipeline includes: 1. Document classification. - 2. Dataset permission checks (requires "write" access). - 3. Document chunking with a specified or default chunk size. - 4. Event and timestamp extraction from chunks. - 5. Knowledge graph extraction from events. - 6. Batched insertion of data points. + 2. Document chunking with a specified or default chunk size. + 3. Event and timestamp extraction from chunks. + 4. Knowledge graph extraction from events. + 5. Batched insertion of data points.
🧹 Nitpick comments (1)
cognee/cli/minimal_cli.py (1)
73-112: Consider refining error handling and user experience.The TUI implementation is functional but could be improved:
Blind exception catch (line 108): Catching
Exceptionbroadly masks different failure modes. Consider catching specific exceptions or at least including the exception type in the error message.Ignored return codes (line 107): The return value from
full_main()is discarded. Users won't know if commands succeeded or failed.UX consideration: After executing a command, the menu reappears immediately. Users might not see command output before the menu overwrites it. Consider adding a pause (e.g., "Press Enter to continue...") after command execution.
Here's a refined version addressing these concerns:
- # Temporarily override sys.argv to reuse the existing CLI original_argv = sys.argv try: sys.argv = cmd_map[choice] - full_main() - except Exception as e: - print(f"Error while running command: {e}") + exit_code = full_main() + if exit_code != 0: + print(f"\n⚠ Command exited with code {exit_code}") + except KeyboardInterrupt: + print("\n\nCommand interrupted by user.") + except Exception as e: + print(f"\n❌ Error while running command ({type(e).__name__}): {e}") finally: sys.argv = original_argv + input("\nPress Enter to continue...")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.gitignore(1 hunks)cognee/api/v1/cognify/cognify.py(2 hunks)cognee/cli/minimal_cli.py(4 hunks)cognee/eval_framework/corpus_builder/task_getters/get_cascade_graph_tasks.py(0 hunks)cognee/tasks/documents/__init__.py(1 hunks)cognee/tasks/documents/check_permissions_on_dataset.py(0 hunks)
💤 Files with no reviewable changes (2)
- cognee/eval_framework/corpus_builder/task_getters/get_cascade_graph_tasks.py
- cognee/tasks/documents/check_permissions_on_dataset.py
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Vattikuti-Manideep-Sitaram
Repo: topoteretes/cognee PR: 1529
File: cognee/api/v1/cognify/ontology_graph_pipeline.py:69-74
Timestamp: 2025-10-11T04:18:24.594Z
Learning: The code_graph_pipeline.py and ontology_graph_pipeline.py both follow an established pattern of calling cognee.prune.prune_data() and cognee.prune.prune_system(metadata=True) at the start of pipeline execution. This appears to be intentional behavior for pipeline operations in the cognee codebase.
🧬 Code graph analysis (1)
cognee/cli/minimal_cli.py (1)
cognee/cli/_cognee.py (1)
main(170-342)
🪛 Ruff (0.14.5)
cognee/cli/minimal_cli.py
108-108: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (6)
.gitignore (1)
200-200: LGTM!Adding
.venvsource/to gitignore is appropriate for excluding virtual environment directories.cognee/tasks/documents/__init__.py (1)
1-3: LGTM!The removal of
check_permissions_on_datasetexport aligns with the deletion of the corresponding module.cognee/api/v1/cognify/cognify.py (2)
21-25: LGTM!The removal of
check_permissions_on_datasetimport is consistent with its removal from the processing pipeline.
309-330: LGTM!The default tasks pipeline correctly implements the processing flow without the removed permission check.
cognee/cli/minimal_cli.py (2)
46-46: LGTM!Adding the "tui" command to the available commands is appropriate.
125-127: LGTM!The TUI routing logic integrates cleanly with the existing CLI structure without disrupting other commands.
|
1 participant
Pre-submission Checklist