compact() ran SELECT count(*) on the vec0 table itself purely to compute
the bloat ratio, on every update_llm_index() call -- including small
scoped ones from bulk_update_documents. Like any other document_id-scale
lookup, count(*) has no point-lookup fast path on vec0's own table, so
this is a full scan regardless of index size.
document_chunks is kept in lockstep with the vec0 table by construction
and is a plain indexed table, so it gives an identical count far more
cheaply. Only affects the bloat-ratio heuristic and a log line -- the
actual rebuild already streams and counts real vec0 rows independently
(_copy_rows()'s own return value), so this can't desync compaction's
correctness even in the brief window before a store's document_chunks is
fully backfilled by migration.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
_rebuild_into()/_create_vec_table() always reflect whatever the *current*
schema is -- correct for compact() (which only ever runs on an
already-current-schema store), but wrong for a migration that isn't the
latest one anymore. m0001 delegated to them, which worked fine as long as
v2 was current, but silently breaks the moment a v2 -> v3 migration is
added: m0001 would then produce a v3-shaped table (whatever columns
happen to be current) instead of its actual v2 shape, and the next
migration in the chain would find the columns it expects to migrate from
already gone.
Spell out m0001's own v2-shaped CREATE VIRTUAL TABLE and row copy instead,
so it keeps producing the same output forever, independent of later
schema changes -- the same reasoning already documented on the test
helper _copying_apply(), just not, until now, applied to the real
migration.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Prompted by an automated simplification review of the branch. Applies the
highest-value findings, all behavior-preserving (full suite still green):
- vector_store.py: extract _rebuild_file() (temp-file lifecycle: open,
populate, swap-in-or-discard-on-failure) and _rebuild_into() (create
table, copy meta, stream rows) out of compact() and the v1->v2
migration, which had duplicated both almost verbatim. The migration
module shrinks to a single _rebuild_into() call instead of reaching
into four PaperlessSqliteVecVectorStore privates.
- vector_store.py: extract _stored_schema_version() out of
has_pending_migration() and check_and_run_migrations(), which
duplicated the same schema_version read and its "missing key means
current" default.
- indexing.py: extract _with_exclusive_access(), collapsing three
identical _exclude_readers()/Timeout/log-and-skip blocks (compaction
in update_llm_index() and llm_index_compact(), the migration check in
_check_and_run_migrations()) to one line each. Also trims
_check_and_run_migrations()'s docstring, which had grown to restate
content already documented on has_pending_migration() and
check_and_run_migrations(), including a claim that was now stale
(llm_index_migrate() is a second consumer of the re-embed signal).
- test_ai_indexing.py: add a mock_store fixture for the
write_store()-yields-a-MagicMock pattern that 8 tests were hand-rolling
identically.
- migrations/__init__.py: consolidate the "how to add a migration"
instructions to one place instead of three.
- docs/administration.md: fix two now-inaccurate claims -- the bare-metal
migrate step doesn't run "automatically" (it's the manual step being
documented), and the self-contradictory "if enabled... no-op if
disabled" phrasing on the same step.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Move schema migrations into their own paperless_ai/migrations package,
one module per migration (mNNNN_description.py -- a leading digit isn't
a valid Python identifier, unlike Django's own numbered migrations,
which load via importlib.import_module() rather than a static import
statement), establishing the pattern before more migrations accumulate
in vector_store.py.
- Drop the redundant "if the LLM index is enabled..." clause from the
Docker upgrade note in docs/administration.md -- it's already covered
by the LLM index section below, and calling out just one of several
auto-applied migrations there was incomplete.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
bulk_edit.py's tag/correspondent/document_type/storage_path/custom_field
helpers write via queryset.update() or direct M2M/through-model bulk
operations, none of which call Document.save() -- so Document.modified's
auto_now never fires. Comparing against get_modified_times() for a
document_ids-scoped update_llm_index() call was silently skipping reindex
of documents whose embedded metadata had just changed via a bulk edit.
The modified-time comparison is now only used for the unscoped,
full-library incremental scan, where it is still needed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Add document_llmindex migrate, a cheap check-only path (no reindex) safe
to run unconditionally: has_pending_migration() short-circuits to a
metadata-only read once the store is current, so a healthy install pays
almost nothing. If a pending migration would require re-embedding, it
only logs a warning and leaves the index as-is -- re-embedding can be
slow and, for a metered embedding backend, cost money, so it stays a
deliberate manual action (document_llmindex rebuild), never automatic.
Wire it into the Docker image as a new init-llmindex-migrate oneshot
(modeled on init-search-index, gated on init-migrations), and document
the equivalent manual step for bare-metal upgrades.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Only update_llm_index()'s nightly rebuild ran check_and_run_migrations(),
but llm_index_remove_document()/llm_index_add_or_update_document() call
delete()/upsert_document() directly and immediately (on every document
delete/edit). On upgrade, every pre-existing document is "pre-migration"
until the next scheduled rebuild -- up to 24h by default -- so a delete or
edit in that window would find zero document_chunks rows and silently
leave stale/orphaned vec0 rows behind.
Add has_pending_migration(), a cheap metadata-only read that needs no
exclusive access, so normal writes only pay for check_and_run_migrations()'s
exclusive locking when a migration is actually pending -- otherwise they'd
be gated by the same lock compaction uses, which
test_normal_write_is_not_gated_by_the_compaction_lock exists to forbid.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
document_chunks is a plain SQLite table, not a vec0 virtual table, so
(unlike vec0's own TEXT metadata column) it gets normal type-affinity
coercion between the TEXT document ids used elsewhere in this module and
an INTEGER column -- verified empirically. INTEGER keys make the
per-document delete lookup this table exists for cheaper: smaller index
entries and integer rather than text B-tree comparisons.
Cover compact()'s handling of a churned document's final chunk generation,
delete() on a never-indexed document, upsert_document() clearing to an empty
node list, and the v1->v2 migration's embed_model preservation and
idempotency, per repo convention.
vec0 only gets an efficient lookup on a metadata column inside a KNN
query; a plain document_id-filtered DELETE is a full table scan
regardless of index size. Add a document_chunks side table (plain
SQLite, real index) to look up chunk ids for a document, then delete
each by its id primary key instead. Includes a v1 -> v2 migration to
backfill document_chunks for indexes created before this existed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
build_llm_index_text queried Note and CustomFieldInstance (plus its
field FK) per document, uncovered by the earlier correspondent/type/
storage_path/tags prefetch fix. Add notes and custom_fields__field to
the rebuild and scoped document querysets, and read from doc.notes.all()
instead of a fresh Note.objects.filter() so the prefetch is actually used.
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
The permission filter OR'd three querysets together on top of a
queryset that could already carry two independent tags__id__all
joins, letting a document that matched more than one branch (e.g.
unowned + group-permissioned) come back twice. Replaced it with a
single id__in filter against the existing permitted_document_ids
helper, which is join-free and can't hit this.
* Perf: scope bulk_update_documents' LLM index refresh to the edited document ids instead of the whole library
* Perf: select_related/prefetch_related for the scoped LLM index batch
* Perf: select_related/prefetch_related for the full LLM index rebuild path
* Fix: address Copilot review findings on LLM index scoping
Gotenberg's LibreOffice route enables updateIndexes by default, which refreshes dynamic fields (e.g. auto-dates) to the current date. Disable it so field values are preserved as authored.
* Fix: selection_data re-derives the filtered document set 5 times over
_get_selection_data_for_queryset() (powers ?include_selection_data=true on
the document list and search endpoints) computed document_count for
Correspondent/DocumentType/StoragePath/Tag/CustomField by embedding the
caller's full filtered queryset -- filters plus the permission check -- as
a subquery inside 5 separate Count(filter=Q(documents__in=queryset), ...)
calls. Each one re-evaluates that whole queryset from scratch.
Resolve the document ids once into a concrete list and reuse it across all
five annotations instead. For Tag/CustomField specifically (M2M via a
through-model table), also route through annotate_document_count_by_ids()
-- extracted from the tag/custom-field document_count fix (#13203) -- to
avoid the same Count(filter=Q(id__in=...), distinct=True)-on-an-M2M-relation
anti-pattern diagnosed there.
On a 400k-document/1,000-tag corpus, the correspondents portion alone
previously didn't finish within several minutes (killed twice while
investigating, including one run that left a zombie query still consuming
a CPU 30+ minutes later). All five queries together now complete in
~30-35s. Root-caused from a real report (paperless-ngx#13201) via a
different, already-fixed query (#13205) -- this one hasn't been reported
in the wild yet, found by auditing the same call path.
Depends on #13203 for annotate_document_count_by_ids().
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
* Address review feedback: drop unnecessary ordering before collecting ids
queryset passed into _get_selection_data_for_queryset() carries the
default/user-specified ordering, which is irrelevant once we're only
collecting a flat id list. Clearing it removes a pointless sort.
No measurable change in benchmarking at 400k documents -- the id-list
materialization/IN-clause cost still dominates -- but it's a free,
strictly-correct cleanup, not just noise-neutral in the other direction.
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
query_similar_documents() never excluded the querying document itself, so a document could appear in its own "similar documents" context, duplicating its content into the AI-suggestions prompt and inflating prompt size/tokens unnecessarily. Add NE filter support to the vector store and exclude the source document's id at query time.
Avoids a Postgres error where the clamp UPDATE and the following ALTER TABLE on paperless_mail_mailrule share a transaction, but the table's FKs are deferrable, so pending trigger events block the ALTER.
Also fixes a verbose_name mismatch in migration 0013.
DocumentViewSet.get_queryset() carried a blanket .distinct() left over from
an older query shape (predating the version-file feature and the
correlated-subquery rewrite of num_notes/effective_content). Nothing in the
current base queryset can produce duplicate document rows: select_related
is all FK-to-PK, and the permission filter is a boolean id__in predicate,
not a join. id (the PK) is also always selected and inherently unique, so
.distinct() was a structural no-op for correctness.
It was not a no-op for cost. EXPLAIN showed it forcing a full sort-then-
dedupe over the entire permission-visible document set before LIMIT could
apply -- 340k rows for a 25-row page, with the effective_content/num_notes
correlated subqueries re-executed once per row as a result. This was the
dominant cost behind the original report's document overview slowness,
well beyond the get_user_can_change N+1 fixed separately.
Removing it isn't safe on its own, though -- auditing found two existing
filters that rely on an M2M join *without* deduping themselves, previously
papered over by the blanket .distinct():
- InboxFilter: a document with two tags both flagged is_inbox_tag=True
(nothing prevents that) would be returned twice for ?is_in_inbox=true.
- CustomFieldsFilter: a document with multiple custom field instances
matching different OR-ed branches would be returned once per match.
Fixed both locally, matching the pattern ObjectFilter already used for
tags__id__in/custom_fields__id__in. Added regression tests for both --
confirmed they fail without the local .distinct() calls.
Benchmarked against the same 400k-document/1,000-tag corpus: document list
wall-clock for a permission-restricted user drops from ~15.8s to ~2.25s
(~7x), closing most of the previous gap to a superuser's ~1.8s.
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
* Fix (beta): tag/custom-field document_count scales badly with tag count
TagViewSet and CustomFieldViewSet's document_count annotation used a
per-row correlated subquery (annotate_document_count_for_related_queryset),
executed once per tag/custom-field row. Fine at a few dozen rows, but at
~1,000 tags it degrades to a full per-tag GroupAggregate over the tags M2M
table -- 6s+ in production reports, confirmed via EXPLAIN (loops=1000).
Replaced with a single, independent GROUP BY over the through table
(permission filter expressed as a plain WHERE, not an aggregate FILTER),
then injected via Case/When. Also tried a more "obvious" fix -- a plain
Count(filter=Q(id__in=permitted_ids), distinct=True) directly on the M2M
relation -- but that's worse: Postgres fails to plan the id__in check as a
semi-join once a large M2M bridge table is involved, and instead re-checks
subquery membership once per joined row.
Benchmarked against a synthetic corpus (400k documents, 1,000 tags @ ~5/doc,
matching real-world reports in discussion #13161): tag list wall-clock drops
from ~180s to ~8s for a permission-restricted user, ~21s to ~8s for a
superuser. No measurable change at typical home-instance scale (tens of
tags).
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
* Address review feedback: restore validation, scope aggregation to queryset
- Restore the document_count_source_field validation helper dropped during
the refactor -- misconfiguring a viewset (document_count_through set
without document_count_source_field) now fails fast with a clear error
again instead of an obscure runtime failure.
- Restrict annotate_document_count_for_related_queryset()'s through-table
aggregation to rows whose related_object_field is one of the annotated
queryset's pks. No-op for the main tag-list call site (queryset is all
tags), but avoids unnecessary work for narrower callers like the tag
descendants branch in TagViewSet.list().
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
* Fix (beta): batch document user_can_change checks to avoid per-row N+1
DocumentSerializer.get_user_can_change() built a fresh
ObjectPermissionChecker and issued a guardian permission-table query for
every document row not owned by the requesting user -- correct, but O(N)
per page load for any non-superuser viewing documents owned by others.
BulkPermissionMixin already batches this exact lookup (2 queries total,
regardless of page size) for Correspondent/Tag/DocumentType/CustomField,
but was gated behind the rarely-used `full_perms` flag and DocumentViewSet
didn't inherit it at all. Changed the gate to "any list action" (cheap:
just two extra queries per page) and added BulkPermissionMixin to
DocumentViewSet, then updated get_user_can_change to consult that batched
context before falling back to a fresh guardian check.
Preserves guardian's own superuser shortcut explicitly (has_perm() special-
cases is_superuser without a query; the batched-context path doesn't, so it
needed its own check) -- covered by a new regression test, since no
existing test exercised a superuser viewing an other-owned document.
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
* Fix (beta): don't batch permissions for tantivy search results
UnifiedSearchViewSet.list() returns SearchHit/dict-like objects for
text/title/query/more_like_id search requests, not Document ORM instances.
Adding BulkPermissionMixin to DocumentViewSet (previous commit) meant its
get_serializer_context() ran for search responses too, and its
_get_object_perms() -- which expects real model instances with .pk --
crashed on the dict-like hits with AttributeError, turning every search
request into a 400.
Skip the batching specifically for search requests (existing
_is_search_request() check) by calling past BulkPermissionMixin in the
MRO; non-search list() calls (which return a real Document queryset) are
unaffected and still get the batching.
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
stumpylogandClaude Sonnet 5
shamoon
Matthias MastandGitHub
