feat: disable langflow ingestion via UI#1623
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR adds a runtime ChangesTraditional Document Ingestion Toggle
🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
… through an exception' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/config/config_manager.py (1)
3-8:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix import ordering to unblock CI.
ruffis failing withI001on this import block; sorting/formatting imports here is required before merge.Proposed import block ordering
-import os -import yaml -from pathlib import Path -from typing import Dict, Any, Optional -from dataclasses import dataclass, asdict, field +import os +from dataclasses import asdict, dataclass, field +from pathlib import Path +from typing import Any, Dict, Optional + +import yaml from utils.logging_config import get_logger🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/config/config_manager.py` around lines 3 - 8, The import block in config_manager.py is not sorted per ruff (I001); reorder imports into standard-library, third-party, then local groups with a blank line between groups and alphabetize within groups. Specifically move dataclasses/Path/os/typing imports into the stdlib group (e.g., from dataclasses import dataclass, asdict, field; import os; from pathlib import Path; from typing import Any, Dict, Optional), keep yaml as the third-party import, and place from utils.logging_config import get_logger in the local group; ensure single blank lines separate groups to satisfy ruff/isort expectations.
🧹 Nitpick comments (1)
src/services/default_docs_service.py (1)
69-69: ⚡ Quick winSnapshot
disable_ingest_with_langflowonce per ingestion run.This flag is read multiple times across one flow. If it changes during execution, URL and file ingestion can route through different pipelines in the same request. Read it once at entry and pass it through helper calls.
♻️ Suggested refactor
async def ingest_openrag_docs_when_ready( document_service, models_service, task_service, langflow_file_service, session_manager, + disable_langflow_ingest: bool, jwt_token=None, ): @@ - if get_openrag_config().knowledge.disable_ingest_with_langflow: + if disable_langflow_ingest: task_id = await _ingest_default_documents_url( @@ async def ingest_default_documents_when_ready( @@ ): @@ + disable_langflow_ingest = ( + get_openrag_config().knowledge.disable_ingest_with_langflow + ) logger.info( "Ingesting default documents when ready", - disable_langflow_ingest=get_openrag_config().knowledge.disable_ingest_with_langflow, + disable_langflow_ingest=disable_langflow_ingest, ingest_source=DEFAULT_DOCS_INGEST_SOURCE, ) @@ task_id = await ingest_openrag_docs_when_ready( document_service, models_service, task_service, langflow_file_service, session_manager, + disable_langflow_ingest=disable_langflow_ingest, jwt_token=jwt_token, ) @@ - if get_openrag_config().knowledge.disable_ingest_with_langflow: + if disable_langflow_ingest: new_task_id = await _ingest_default_documents_openrag(Also applies to: 113-113, 146-146
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/default_docs_service.py` at line 69, Read get_openrag_config().knowledge.disable_ingest_with_langflow once at the start of the ingestion entrypoint (e.g., public methods that begin URL/file ingestion such as ingest_urls, ingest_files or the top-level process_ingestion in default_docs_service.py), store it in a local boolean (disable_ingest_with_langflow) and pass that boolean into all downstream helper calls instead of calling get_openrag_config() repeatedly; update helper signatures that currently read the flag internally to accept the boolean and propagate it through the URL and file ingestion pipeline so the same snapshot is used for the entire run.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/api/router.py`:
- Around line 117-125: The temp files in temp_file_paths are only removed on
error and are left behind after a successful call to
task_service.create_upload_task; after create_upload_task returns (i.e., when
task_id is obtained) you must unlink each path in temp_file_paths to avoid
leaking bytes. Modify the upload handler around create_upload_task to, on
success, iterate over temp_file_paths and remove/unlink them (ensure you use
safe remove semantics and log failures), referencing the create_upload_task call
and temp_file_paths list so cleanup always runs for the successful path without
changing DocumentFileProcessor or TaskService behavior.
- Around line 97-110: The issue is that safe_filename in the upload loop
produces non-unique temp_path values causing overwrites and collapsing keys into
file_path_to_original_filename and UploadTask.file_tasks; fix it by generating a
unique temp path per upload (e.g., use tempfile.NamedTemporaryFile(delete=False)
or tempfile.mkstemp to obtain a unique filename or append a uuid4/unique suffix
to safe_filename) in the loop where upload_files are processed, write the file
to that unique temp_path, and keep using that unique path as the key when
building file_path_to_original_filename and when creating UploadTask.file_tasks.
In `@src/config/config_manager.py`:
- Line 97: The system_prompt string contains incorrect repeated Langflow text
for OpenSearch and Docling in the "What is OpenRAG" answer; update the
system_prompt (variable name: system_prompt) so the three project descriptions
are accurate and distinct: set OpenSearch to a short description like
"OpenSearch – a community-driven, open source search and analytics suite
(https://opensearch.org/)" and Docling to a correct description like "Docling –
a tool for document ingestion and semantic search (https://www.docling.ai/)",
keeping Langflow's description and the embedded links intact and ensuring the
final quoted "What is OpenRAG" paragraph reflects these corrected descriptions.
- Around line 306-309: Remove the direct os.getenv(...) call in config_manager
(the block that sets config_data["knowledge"]["disable_ingest_with_langflow"])
and instead consume the value exported by config/settings.py (e.g., import the
Settings instance or constant like DISABLE_INGEST_WITH_LANGFLOW from
config.settings). Use the typed/settings value (already boolean or convert once
in settings) to assign config_data["knowledge"]["disable_ingest_with_langflow"]
rather than reading environment variables here. Ensure you reference the
config_manager function/class where config_data is built and import the
appropriate symbol from config.settings.
In `@src/models/processors.py`:
- Around line 459-462: When replace_duplicates is true and you delete the
existing document via delete_document_by_filename, the subsequent call to
process_document_standard may still see the old chunks because OpenSearch hasn't
refreshed; call a refresh on the index (using opensearch_client.indices.refresh
or the equivalent client method) right after delete_document_by_filename and
before process_document_standard calls check_document_exists(file_hash, ...) so
the deletion is visible. Apply the same refresh-after-delete step in the other
delete-before-replace branch referenced around the
process_document_standard/check_document_exists logic (the block covering lines
~474-485) to ensure the existence check reflects the deletion.
In `@tests/integration/core/test_api_endpoints.py`:
- Around line 231-236: Update the test to require the task-based ingestion
status (202) instead of accepting 201: in the test block that reads the response
body and assigns task_id, change the status assertion to expect 202 and keep the
subsequent checks for task_id (string), file_count == 1 and the call to
_wait_for_task_completion(client, task_id); this enforces the “always task-based
ingestion” contract referenced by the router test.
---
Outside diff comments:
In `@src/config/config_manager.py`:
- Around line 3-8: The import block in config_manager.py is not sorted per ruff
(I001); reorder imports into standard-library, third-party, then local groups
with a blank line between groups and alphabetize within groups. Specifically
move dataclasses/Path/os/typing imports into the stdlib group (e.g., from
dataclasses import dataclass, asdict, field; import os; from pathlib import
Path; from typing import Any, Dict, Optional), keep yaml as the third-party
import, and place from utils.logging_config import get_logger in the local
group; ensure single blank lines separate groups to satisfy ruff/isort
expectations.
---
Nitpick comments:
In `@src/services/default_docs_service.py`:
- Line 69: Read get_openrag_config().knowledge.disable_ingest_with_langflow once
at the start of the ingestion entrypoint (e.g., public methods that begin
URL/file ingestion such as ingest_urls, ingest_files or the top-level
process_ingestion in default_docs_service.py), store it in a local boolean
(disable_ingest_with_langflow) and pass that boolean into all downstream helper
calls instead of calling get_openrag_config() repeatedly; update helper
signatures that currently read the flag internally to accept the boolean and
propagate it through the URL and file ingestion pipeline so the same snapshot is
used for the entire run.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b932e0d1-92ad-4721-991e-d8a8f6590222
📒 Files selected for processing (18)
frontend/app/api/mutations/useUpdateSettingsMutation.tsfrontend/app/api/queries/useGetSettingsQuery.tsfrontend/app/settings/_components/ingest-settings-section.tsxsrc/api/connector_router.pysrc/api/router.pysrc/api/settings/endpoints.pysrc/api/settings/models.pysrc/app/container.pysrc/config/config_manager.pysrc/config/settings.pysrc/models/processors.pysrc/services/default_docs_service.pysrc/services/flows_service.pysrc/services/startup_orchestrator.pysrc/services/task_service.pytests/integration/core/test_api_endpoints.pytests/unit/config/test_disable_ingest_setting.pytests/unit/test_traditional_duplicate_handling.py
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 5517 🏁 Script executed: Length of output: 4147 🏁 Script executed: Length of output: 36279
Sync parity analysis1.
|
| Aspect | LangflowConnectorService |
ConnectorService (traditional) |
Status |
|---|---|---|---|
| List files kwarg | connector.list_files(page_token, limit=page_size) |
connector.list_files(page_token, max_files=page_size) |
|
| Filename filter | Not supported | filename_filter: set param — skips files deleted from the connector |
❌ Feature absent in Langflow path |
| Task creation | create_custom_task(...) |
create_custom_task(...) |
✅ |
original_filenames mapping |
✅ | ✅ | ✅ |
LangflowConnectorService passes limit=page_size while ConnectorService passes max_files=page_size when calling connector.list_files(). Unless the individual connectors accept both kwargs, one of these paths silently ignores the page-size cap.
❌ filename_filter absent in Langflow path: ConnectorService.sync_connector_files accepts a filename_filter: set that prevents re-ingesting files that have been deleted from the connector (used during reconciliation). LangflowConnectorService has no equivalent — deleted-file prevention relies entirely on pre-delete logic at ingestion time.
2. sync_specific_files — Gaps found
| Aspect | LangflowConnectorService |
ConnectorService |
Status |
|---|---|---|---|
| Processor type | LangflowConnectorFileProcessor |
ConnectorFileProcessor |
✅ (correct per path) |
ingest_settings forwarded to processor |
✅ | ✅ | ✅ |
cfg is None guard (IBM COS skip) |
if cfg is not None: — skips expansion |
if hasattr(connector, "cfg"): — same intent |
✅ |
ValueError re-raise |
❌ Falls back silently | ✅ if isinstance(e, ValueError): raise |
❌ Langflow path swallows intentional errors |
isFolder filtering in fallback file_infos |
❌ Missing | ✅ Filters f.get("isFolder") before fallback |
❌ Langflow path may try to ingest folder items |
finally block restores cfg |
✅ | ✅ | ✅ |
3. process_connector_document — Gaps found
| Aspect | LangflowConnectorService |
ConnectorService |
Status |
|---|---|---|---|
ingest_settings accepted |
✅ (chunkSize, chunkOverlap via merge_ui_ingest_settings_into_tweaks) |
❌ No ingest_settings param |
❌ |
| Pre-delete existing chunks before re-ingestion | ✅ Step 0 — delete_by_query on filename |
❌ Missing | ❌ |
Post-process metadata update (source_url, connector_type, created_time, modified_time, metadata) |
Via Langflow flow variables | ✅ _update_connector_metadata (explicit update_by_query) |
✅ Traditional path has richer metadata write-back |
| ACL update | Via allowed_users/allowed_groups to Langflow |
✅ update_document_acl in _update_connector_metadata |
✅ |
document_id propagation |
✅ Explicit kwarg to run_ingestion_flow |
✅ Used as file_hash in process_document_standard |
✅ |
Summary of actionable gaps
| # | File | Gap | Severity |
|---|---|---|---|
| 1 | src/connectors/service.py process_connector_document |
Missing ingest_settings forwarding → chunkSize/chunkOverlap/embeddingModel ignored |
🔴 High |
| 2 | src/connectors/service.py process_connector_document |
No pre-delete of existing chunks before re-ingestion — causes duplicate chunks on resync | 🔴 High |
| 3 | src/connectors/langflow_connector_service.py sync_connector_files |
limit= kwarg vs max_files= in traditional — page-size cap may be silently ignored in one path |
🟡 Medium |
| 4 | src/connectors/langflow_connector_service.py sync_specific_files |
Swallows ValueError on folder-only selection instead of re-raising |
🟡 Medium |
| 5 | src/connectors/langflow_connector_service.py sync_specific_files |
Fallback does not filter isFolder items from file_infos — may attempt to ingest folders |
🟡 Medium |
| 6 | src/connectors/langflow_connector_service.py sync_connector_files |
filename_filter feature missing — cannot exclude deleted files during reconciliation |
🟠 Low-Medium |
Would you like me to open a GitHub issue to track these gaps?
|
@lucaseduoli I am testing this PR ATM. |
|
Getting Error: |
…on_disable # Conflicts: # src/api/connector_router.py # src/connectors/langflow_connector_service.py # src/connectors/service.py # src/models/processors.py # src/models/tasks.py # src/session_manager.py
|
updating brnach to create PR into it |
Ensure ACL and metadata updates target both Langflow and non-Langflow chunks by matching document IDs against multiple fields. Introduce _build_id_query and id_fields parameters to should_update_acl, update_document_acl, and batch_update_acls so updates can match document_id and connector_file_id. Stop overwriting chunk owner during ACL syncs (only allowed_users/allowed_groups are updated) and set owner at ingest to the syncing user in TaskProcessor. Adjust tests to expect the combined bool query and make minor typing/import/formatting cleanups.
This pull request introduces a new feature allowing users to disable document ingestion via Langflow and instead use a traditional OpenRAG ingestion pipeline. The change is implemented as a new
disable_ingest_with_langflowsetting, which is configurable through both the frontend settings UI and backend configuration/environment variables. The backend logic, API endpoints, and processors have been updated to respect this setting, ensuring that document ingestion routes and processing behavior adapt accordingly.Frontend: Ingestion Settings UI and API
IngestSettingsSection) to allow users to enable or disable Langflow ingestion. This setting is now part of the settings API requests and state management. [1] [2] [3] [4] [5] [6] [7]Backend: Configuration and Models
KnowledgeConfig, config manager, and environment variable parsing) to support the newdisable_ingest_with_langflowboolean, including propagation through API models and settings endpoints. [1] [2] [3] [4] [5] [6]Backend: Routing and Service Logic
_traditional_upload_ingest_taskfor handling traditional ingestion as a background task. [1] [2] [3] [4] [5]Backend: Document Processing
replace_duplicatesflag and the session manager for OpenSearch client access. This ensures that when Langflow is disabled, duplicate handling is robust. [1] [2] [3] [4]Miscellaneous and Clean-up
These changes collectively provide a flexible ingestion pipeline, allowing administrators and users to switch between Langflow and traditional methods as needed.
Summary by CodeRabbit
New Features
Improvements
Tests