-
Notifications
You must be signed in to change notification settings - Fork 966
fix: get default tasks #700
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
Conversation
Please make sure all the checkboxes are checked:
|
WalkthroughThis pull request updates the task retrieval functions in the default task getter module. Both Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant get_no_summary_tasks
participant BaseTasks
participant GraphTask
participant AddDataPointsTask
Client->>get_no_summary_tasks: Call get_no_summary_tasks(user, graph_model,\nontology_file_path, ...)
get_no_summary_tasks->>BaseTasks: Retrieve base tasks (classify, check permissions, extract chunks)
get_no_summary_tasks->>GraphTask: Create graph_task using extract_graph_from_data
GraphTask->>TaskConfig: Configure dependencies
get_no_summary_tasks->>AddDataPointsTask: Create add_data_points_task (dependency on extract_graph_from_data)
get_no_summary_tasks-->>Client: Return combined tasks list
sequenceDiagram
participant Client
participant get_just_chunks_tasks
participant BaseTasks
participant AddDataPointsTask
Client->>get_just_chunks_tasks: Call get_just_chunks_tasks(user, ...)
get_just_chunks_tasks->>BaseTasks: Retrieve base tasks (classify, check permissions, extract chunks)
get_just_chunks_tasks->>AddDataPointsTask: Create add_data_points_task (dependency on extract_chunks_from_documents)
get_just_chunks_tasks-->>Client: Return combined tasks list
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 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: 0
🧹 Nitpick comments (5)
cognee/eval_framework/corpus_builder/task_getters/get_default_tasks_by_indices.py (5)
6-10: Remove unused imports
classify_documentsandcheck_permissions_on_documentsappear never used in this file. Consider removing them for cleanliness.🧰 Tools
🪛 Ruff (0.8.2)
7-7:
cognee.tasks.documents.classify_documentsimported but unusedRemove unused import
(F401)
8-8:
cognee.tasks.documents.check_permissions_on_documentsimported but unusedRemove unused import
(F401)
13-13: Unused import
get_default_useris not referenced in this file. Consider removing it.🧰 Tools
🪛 Ruff (0.8.2)
13-13:
cognee.modules.users.methods.get_default_userimported but unusedRemove unused import:
cognee.modules.users.methods.get_default_user(F401)
15-15: Remove unused imports
run_tasksandmerge_needsare unused here. Consider removing these imports.🧰 Tools
🪛 Ruff (0.8.2)
15-15:
cognee.modules.pipelines.run_tasksimported but unusedRemove unused import
(F401)
15-15:
cognee.modules.pipelines.merge_needsimported but unusedRemove unused import
(F401)
16-16: Unused import
get_cognify_configis never used in this file. Removing it would reduce clutter.🧰 Tools
🪛 Ruff (0.8.2)
16-16:
cognee.modules.cognify.config.get_cognify_configimported but unusedRemove unused import:
cognee.modules.cognify.config.get_cognify_config(F401)
18-18: Unused import
get_max_chunk_tokensis not invoked here. Please remove it.🧰 Tools
🪛 Ruff (0.8.2)
18-18:
cognee.infrastructure.llm.get_max_chunk_tokensimported but unusedRemove unused import:
cognee.infrastructure.llm.get_max_chunk_tokens(F401)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cognee/eval_framework/corpus_builder/task_getters/get_default_tasks_by_indices.py(2 hunks)examples/python/pokemon_datapoints_example.py(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
cognee/eval_framework/corpus_builder/task_getters/get_default_tasks_by_indices.py (2)
cognee/api/v1/cognify/cognify.py (1)
cognify(35-69)cognee/api/v1/cognify/routers/get_cognify_router.py (1)
cognify(20-27)
🪛 Ruff (0.8.2)
cognee/eval_framework/corpus_builder/task_getters/get_default_tasks_by_indices.py
7-7: cognee.tasks.documents.classify_documents imported but unused
Remove unused import
(F401)
8-8: cognee.tasks.documents.check_permissions_on_documents imported but unused
Remove unused import
(F401)
13-13: cognee.modules.users.methods.get_default_user imported but unused
Remove unused import: cognee.modules.users.methods.get_default_user
(F401)
15-15: cognee.modules.pipelines.run_tasks imported but unused
Remove unused import
(F401)
15-15: cognee.modules.pipelines.merge_needs imported but unused
Remove unused import
(F401)
16-16: cognee.modules.cognify.config.get_cognify_config imported but unused
Remove unused import: cognee.modules.cognify.config.get_cognify_config
(F401)
18-18: cognee.infrastructure.llm.get_max_chunk_tokens imported but unused
Remove unused import: cognee.infrastructure.llm.get_max_chunk_tokens
(F401)
⏰ Context from checks skipped due to timeout of 90000ms (35)
- GitHub Check: Test on macos-15
- GitHub Check: run_networkx_metrics_test / test
- GitHub Check: Test on macos-13
- GitHub Check: run_notebook_test / test
- GitHub Check: run_eval_framework_test / test
- GitHub Check: run_dynamic_steps_example_test / test
- GitHub Check: run_simple_example_test / test
- GitHub Check: run_multimedia_example_test / test
- GitHub Check: Test on ubuntu-22.04
- GitHub Check: test
- GitHub Check: Test on macos-15
- GitHub Check: run_simple_example_test / test
- GitHub Check: chromadb test
- GitHub Check: run_notebook_test / test
- GitHub Check: run_notebook_test / test
- GitHub Check: Test on macos-15
- GitHub Check: run_notebook_test / test
- GitHub Check: Test on macos-13
- GitHub Check: test
- GitHub Check: Test on macos-13
- GitHub Check: Test on ubuntu-22.04
- GitHub Check: test
- GitHub Check: Test on ubuntu-22.04
- GitHub Check: test
- GitHub Check: windows-latest
- GitHub Check: test
- GitHub Check: lint (ubuntu-latest, 3.11.x)
- GitHub Check: test
- GitHub Check: test
- GitHub Check: Test on ubuntu-22.04
- GitHub Check: run_simple_example_test
- GitHub Check: lint (ubuntu-latest, 3.10.x)
- GitHub Check: test
- GitHub Check: docker-compose-test
- GitHub Check: Build Cognee Backend Docker App Image
🔇 Additional comments (5)
examples/python/pokemon_datapoints_example.py (1)
178-179: Potential performance impact due to removal of batch_size
By removing thetask_config={"batch_size": 50}parameter, tasks now process data in a single unbatched manner, which may affect memory usage or performance if the dataset is large. Consider reintroducing a batching approach if needed or confirm that single-batch processing is sufficient for your use case.cognee/eval_framework/corpus_builder/task_getters/get_default_tasks_by_indices.py (4)
35-41: Unused parameter 'user'
useris never utilized inget_no_summary_tasks. Confirm if this is intentional or remove it to keep the signature concise.
43-60: Function logic looks good
The flow for adding new tasks and their dependencies appears correct.
63-65: Unused parameter 'user'
useris not employed inget_just_chunks_tasks. Please verify if needed or consider removing it.
67-75: Implementation looks consistent
The approach to creatingadd_data_points_taskwith the needed dependency is properly structured.
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.