langgenius / langgenius/dify

`document_indexing_task` swallows "missing document" warning after SQLAlchemy 2.0 refactor

Open
#38,413 1 comment 1 reaction 0 assignees View on GitHub
🐞 bug project#dify
Dominant language
TypeScript
Stars
156k
Forks
24.6k
Avg merge
22h 9m
Merged PRs (30d)
610

Description

### Self Checks

- [x] I have read the [Contributing Guide](https://github.com/langgenius/dify/blob/main/CONTRIBUTING.md) and [Language Policy](https://github.com/langgenius/dify/issues/1542).
- [x] This is only for bug report, if you would like to ask a question, please head to [Discussions](https://github.com/langgenius/dify/discussions/categories/general).
- [x] I have searched for existing issues [search for existing issues](https://github.com/langgenius/dify/issues), including closed ones.
- [x] I confirm that I am using English to submit this report, otherwise it will be closed.
- [x] 【中文用户 & Non English User】请使用英语提交,否则会被关闭 :)
- [x] Please do not modify this template :) and fill in all the required fields.

### Dify version

main branch (uncommitted)

### Cloud or Self Hosted

Self Hosted (Source)

### Steps to reproduce

Discovered by peaks-loop code-sweep on 2026-07-02.

1. In a dataset, queue 10 documents for indexing.
2. Mid-flight (between Phase 1 transaction commit and Phase 2 summary-index scan), delete 2 of those documents from another shell (`DELETE FROM documents WHERE id IN (...)`).
3. Trigger the summary-index pass.
4. Observe the logs: **no warning is emitted** for the 2 missing documents. Operators have no signal that the dataset/queue drifted.

The bug was introduced by the SQLAlchemy 2.0 refactor in #34968 (commit `f67297688` "refactor(tasks): migrate `document_indexing_task` and `remove_app_and_related_data_task` to SQLAlchemy 2.0 select()"). The loop variable changed from a string id to the ORM `Document` object, which made the legacy `if document:` guard unreachable and the `else: logger.warning('... document_id: ')` branch dead.

### ✔️ Expected Behavior

The summary-indexing phase should log one `logger.warning(...)` per missing document id so operators can detect dataset/queue drift. The original intent of "warn when requested documents were deleted before we got to them" must be restored.

### ❌ Actual Behavior

After the SQLAlchemy 2.0 refactor in #34968 (commit `f67297688`), the loop variable in `_document_indexing` was changed from a string id to the ORM `Document` object. The legacy `if document: guard became unreachable (`session.scalars(select(Document...).all())` never yields None) and the `else: logger.warning('... document_id: ')` branch could never fire.

More importantly, the original intent of warning about documents that were requested but no longer present was silently lost. Any document id deleted between the Phase 1 transaction and the Phase 2 summary-index scan produced no log signal at all.

**Proposed change:**

- Drop the dead `if document:` guards in both Phase 1 and the Phase 2 summary-task sub-block of `_document_indexing`.
- Replace the unreachable warning branch with a single `set(document_ids) - found_ids` loop that emits one `logger.warning(...)` per missing id (now correctly typed as a string).
- Promote the per-document `Checking document X for summary generation` info log out of the dead conditional so it fires for every present document, matching the original intent.
- Add a regression test `test_missing_documents_are_logged_with_warning` in `TestBatchProcessing` asserting one warning per missing id and no warnings for present ids.

**Risk:** When all `document_ids` are present, `set(document_ids) - found_ids` is empty, so no warning fires. The per-document update loop runs unchanged. `IndexingRunner.run(documents)` is called the same way as before — no caller change. `DocumentsIsPausedError` and other exception paths are untouched. `_document_indexing_with_tenant_queue`, `normal_document_indexing_task`, `priority_document_indexing_task` are unchanged (they only delegate to `_document_indexing`). The Celery task signature `document_indexing_task(dataset_id: str, document_ids: list)` is unchanged. No schema, migration, controller, or frontend changes.

Introduced by: #34968 (`f67297688`).

Contributor guide

Open the contributing guide

Research direction

Start by reading the `_document_indexing` entry point and the SQLAlchemy 2.0 refactor in #34968, then run the `TestBatchProcessing` suite. Add the named `test_missing_documents_are_logged_with_warning` regression test and verify that each missing document id produces one warning while present ids produce none.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend, databases
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
74/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.