mirror of
https://github.com/paperless-ngx/paperless-ngx.git
synced 2026-10-14 12:07:12 +00:00
Fix: index document versions as their root document (#14422)
* Fix: index document versions as their root document The search index and the LLM index hold root documents only, a root being indexed with its newest version's content. Every write path therefore had to remember to hand them the root. Several did not. WriteBatch.add_or_update, WriteBatch.add_or_update_ids and llm_index_add_or_update_document now resolve a version to its root themselves. The reindex command and update_llm_index only walk root documents, and an incremental LLM update is scoped to the roots of the given ids through versioning.root_document_ids, which add_or_update_ids shares. more_like_id returns the root's id for a version. The reprocess task no longer picks the root for the indexes and only still clears the caches of both documents. * Fix: rebuild old search indexes and look versions up as their root for similar documents An index built before versions were indexed as their root can still hold version entries under their own id and metadata, and only a rebuild clears them. Bump the schema version so the index is rebuilt on upgrade. more_like_this_ids looked the given id up in the index as is, so asking for documents like a version, as the AI suggestions endpoint can, silently found nothing. It now looks the root up instead.
This commit is contained in:
1 parent
96535991a3
commit
9f2f809131
12 files changed
+337
-44
No files matched your search
@@ -293,6 +293,80 @@ class TestAddOrUpdateIds:
|
||||
assert backend.search_ids("updated", user=None) == [doc.pk]
|
||||
|
||||
|
||||
class TestVersionsAreIndexedAsTheirRoot:
|
||||
"""Only root documents are indexed, with their effective content, so
|
||||
every write path that is handed a version indexes its root instead."""
|
||||
|
||||
@staticmethod
|
||||
def _root_with_version() -> tuple[Document, Document]:
|
||||
root = DocumentFactory(title="Statement", content="stale text")
|
||||
version = DocumentFactory(
|
||||
title="Statement",
|
||||
content="latest text",
|
||||
root_document=root,
|
||||
version_index=1,
|
||||
)
|
||||
return root, version
|
||||
|
||||
def test_add_or_update_indexes_the_root_of_a_version(
|
||||
self,
|
||||
backend: TantivyBackend,
|
||||
) -> None:
|
||||
"""
|
||||
GIVEN:
|
||||
- A root document with a version
|
||||
WHEN:
|
||||
- The version is passed to add_or_update
|
||||
THEN:
|
||||
- The root is indexed with the version's text, and the version is not
|
||||
"""
|
||||
root, version = self._root_with_version()
|
||||
|
||||
backend.add_or_update(version)
|
||||
|
||||
assert backend.search_ids("latest", user=None) == [root.pk]
|
||||
assert backend.search_ids("stale", user=None) == []
|
||||
|
||||
def test_add_or_update_ids_indexes_each_root_once(
|
||||
self,
|
||||
backend: TantivyBackend,
|
||||
) -> None:
|
||||
"""
|
||||
GIVEN:
|
||||
- A root document with a version
|
||||
WHEN:
|
||||
- Both ids are passed to add_or_update_ids
|
||||
THEN:
|
||||
- The root is indexed once and the version is not indexed
|
||||
"""
|
||||
root, version = self._root_with_version()
|
||||
|
||||
with backend.batch_update() as batch:
|
||||
batch.add_or_update_ids([version.pk, root.pk])
|
||||
|
||||
assert backend.search_ids("Statement", user=None) == [root.pk]
|
||||
assert backend.search_ids("latest", user=None) == [root.pk]
|
||||
|
||||
def test_add_or_update_ids_resolves_a_lone_version(
|
||||
self,
|
||||
backend: TantivyBackend,
|
||||
) -> None:
|
||||
"""
|
||||
GIVEN:
|
||||
- A root document with a version
|
||||
WHEN:
|
||||
- Only the version's id is passed to add_or_update_ids
|
||||
THEN:
|
||||
- The root is indexed
|
||||
"""
|
||||
root, version = self._root_with_version()
|
||||
|
||||
with backend.batch_update() as batch:
|
||||
batch.add_or_update_ids([version.pk])
|
||||
|
||||
assert backend.search_ids("latest", user=None) == [root.pk]
|
||||
|
||||
|
||||
class TestSearch:
|
||||
"""Test search query parsing and matching via search_ids."""
|
||||
|
||||
@@ -1062,6 +1136,34 @@ class TestMoreLikeThis:
|
||||
assert 150 not in ids
|
||||
assert 151 in ids
|
||||
|
||||
def test_more_like_this_ids_seeded_by_version_uses_root(
|
||||
self,
|
||||
backend: TantivyBackend,
|
||||
) -> None:
|
||||
"""A version is not indexed, so it must be looked up as its root."""
|
||||
root = DocumentFactory.create(
|
||||
title="Important document",
|
||||
content="financial information report",
|
||||
)
|
||||
version = DocumentFactory.create(
|
||||
title="Important document",
|
||||
content="financial information report",
|
||||
root_document=root,
|
||||
version_index=1,
|
||||
)
|
||||
other = DocumentFactory.create(
|
||||
title="Another document",
|
||||
content="financial information report",
|
||||
)
|
||||
backend.add_or_update(root)
|
||||
backend.add_or_update(other)
|
||||
|
||||
ids = backend.more_like_this_ids(doc_id=version.pk, user=None)
|
||||
|
||||
assert other.pk in ids
|
||||
assert root.pk not in ids
|
||||
assert version.pk not in ids
|
||||
|
||||
|
||||
class TestSingleton:
|
||||
"""Test get_backend() and reset_backend() singleton lifecycle."""
|
||||
|
||||
@@ -1196,6 +1196,59 @@ class TestDocumentSearchApi(DirectoriesMixin, APITestCase):
|
||||
self.assertIn(d3.id, result_ids)
|
||||
self.assertNotIn(d4.id, result_ids)
|
||||
|
||||
def test_search_more_like_version_uses_its_root(self) -> None:
|
||||
"""
|
||||
GIVEN:
|
||||
- A document similar in content to a root document, and one that is not
|
||||
- A version of the root document, which is never indexed
|
||||
WHEN:
|
||||
- API request for more like the version
|
||||
THEN:
|
||||
- The documents similar to the root are returned, not the version
|
||||
"""
|
||||
indexed = {}
|
||||
for name, title, content, day in (
|
||||
("root", "bank statement 1", "things i paid for in august", (2019, 3, 4)),
|
||||
(
|
||||
"similar",
|
||||
"bank statement 3",
|
||||
"things i paid for in september",
|
||||
(2020, 7, 9),
|
||||
),
|
||||
(
|
||||
"other",
|
||||
"Quarterly Report",
|
||||
"quarterly revenue profit margin",
|
||||
(2021, 11, 30),
|
||||
),
|
||||
):
|
||||
with time_machine.travel(
|
||||
timezone.make_aware(datetime.datetime(*day)),
|
||||
tick=False,
|
||||
):
|
||||
indexed[name] = DocumentFactory(
|
||||
title=title,
|
||||
content=content,
|
||||
created=datetime.date(*day),
|
||||
added=timezone.make_aware(datetime.datetime(*day)),
|
||||
)
|
||||
version = DocumentFactory(
|
||||
root_document=indexed["root"],
|
||||
version_index=1,
|
||||
content="things i paid for in august",
|
||||
)
|
||||
backend = get_backend()
|
||||
for document in indexed.values():
|
||||
backend.add_or_update(document)
|
||||
|
||||
response = self.client.get(f"/api/documents/?more_like_id={version.id}")
|
||||
|
||||
self.assertEqual(response.status_code, status.HTTP_200_OK)
|
||||
result_ids = [r["id"] for r in response.data["results"]]
|
||||
self.assertIn(indexed["similar"].id, result_ids)
|
||||
self.assertNotIn(indexed["other"].id, result_ids)
|
||||
self.assertNotIn(version.id, result_ids)
|
||||
|
||||
def test_more_like_requires_id_of_existing_document(self) -> None:
|
||||
"""
|
||||
GIVEN:
|
||||
|
||||
@@ -22,6 +22,7 @@ from documents.models import Document
|
||||
from documents.tasks import update_document_content_maybe_archive_file
|
||||
from paperless_testing.assertions import FileSystemAssertsMixin
|
||||
from paperless_testing.dirs import DirectoriesMixin
|
||||
from paperless_testing.factories import DocumentFactory
|
||||
|
||||
sample_file: Path = Path(__file__).parent / "samples" / "simple.pdf"
|
||||
|
||||
@@ -116,6 +117,27 @@ class TestMakeIndex:
|
||||
call_command("document_index", "reindex", skip_checks=True)
|
||||
mock_get_backend.return_value.rebuild.assert_called_once()
|
||||
|
||||
def test_reindex_skips_versions(self, mocker: MockerFixture) -> None:
|
||||
"""
|
||||
GIVEN:
|
||||
- A root document with a version
|
||||
WHEN:
|
||||
- The reindex command runs
|
||||
THEN:
|
||||
- Only the root document is handed to the rebuild, since a version
|
||||
is indexed as its root
|
||||
"""
|
||||
root = DocumentFactory()
|
||||
DocumentFactory(root_document=root, version_index=1)
|
||||
mock_get_backend = mocker.patch(
|
||||
"documents.management.commands.document_index.get_backend",
|
||||
)
|
||||
|
||||
call_command("document_index", "reindex", skip_checks=True)
|
||||
|
||||
documents = mock_get_backend.return_value.rebuild.call_args.args[0]
|
||||
assert list(documents.values_list("pk", flat=True)) == [root.pk]
|
||||
|
||||
def test_optimize(self) -> None:
|
||||
"""Optimize command must execute without error (Tantivy handles optimization automatically)."""
|
||||
call_command("document_index", "optimize", skip_checks=True)
|
||||
|
||||
@@ -310,7 +310,7 @@ class TestUpdateContent(DirectoriesMixin, TestCase):
|
||||
|
||||
@mock.patch("documents.tasks.clear_document_caches")
|
||||
@mock.patch("documents.search.get_backend")
|
||||
def test_update_content_version_indexes_root(
|
||||
def test_update_content_version_clears_caches_for_root(
|
||||
self,
|
||||
mock_get_backend: mock.Mock,
|
||||
mock_clear_caches: mock.Mock,
|
||||
@@ -321,8 +321,8 @@ class TestUpdateContent(DirectoriesMixin, TestCase):
|
||||
WHEN:
|
||||
- Update content task is called for the version
|
||||
THEN:
|
||||
- The version's content is updated
|
||||
- The root document is indexed rather than the version
|
||||
- The version's content is updated, not the root's
|
||||
- The document is indexed
|
||||
- Caches are cleared for both
|
||||
"""
|
||||
root, version = self._create_root_with_version()
|
||||
@@ -334,8 +334,7 @@ class TestUpdateContent(DirectoriesMixin, TestCase):
|
||||
"my document",
|
||||
)
|
||||
self.assertEqual(Document.objects.get(pk=root.pk).content, "root content")
|
||||
indexed = mock_get_backend.return_value.add_or_update.call_args.args[0]
|
||||
self.assertEqual(indexed.pk, root.pk)
|
||||
mock_get_backend.return_value.add_or_update.assert_called_once()
|
||||
mock_clear_caches.assert_has_calls(
|
||||
[mock.call(version.pk), mock.call(root.pk)],
|
||||
)
|
||||
@@ -343,7 +342,7 @@ class TestUpdateContent(DirectoriesMixin, TestCase):
|
||||
@override_settings(AI_ENABLED=True, LLM_EMBEDDING_BACKEND="huggingface")
|
||||
@mock.patch("documents.tasks.llm_index_add_or_update_document")
|
||||
@mock.patch("documents.search.get_backend")
|
||||
def test_update_content_version_updates_llm_index_for_root(
|
||||
def test_update_content_version_updates_llm_index(
|
||||
self,
|
||||
mock_get_backend: mock.Mock,
|
||||
mock_llm_index: mock.Mock,
|
||||
@@ -355,14 +354,13 @@ class TestUpdateContent(DirectoriesMixin, TestCase):
|
||||
WHEN:
|
||||
- Update content task is called for the version
|
||||
THEN:
|
||||
- The LLM index is updated for the root document, not the version
|
||||
- The LLM index is updated
|
||||
"""
|
||||
root, version = self._create_root_with_version()
|
||||
_, version = self._create_root_with_version()
|
||||
|
||||
tasks.update_document_content_maybe_archive_file(version.pk)
|
||||
|
||||
mock_llm_index.assert_called_once()
|
||||
self.assertEqual(mock_llm_index.call_args.args[0].pk, root.pk)
|
||||
|
||||
|
||||
class TestUpdateContentRemoteOCR(DirectoriesMixin, TestCase):
|
||||
|
||||
Reference in new issue
Block a user