From 3f37f49dd028221c642a1008b75af6433366461c Mon Sep 17 00:00:00 2001 From: Trenton Holmes <797416+stumpylog@users.noreply.github.com> Date: Sun, 9 Aug 2026 14:33:09 -0700 Subject: [PATCH] Docs: make chat unbounded-scan plan self-contained Inline the bug diagnosis into a Background section and drop references to the now-deleted CHAT_UNBOUNDED_DOCUMENT_SCAN.md and the throwaway worktree, so the plan can be executed on its own. Co-Authored-By: Claude Sonnet 5 --- ...2026-07-26-chat-unbounded-document-scan.md | 224 +++++++++--------- 1 file changed, 111 insertions(+), 113 deletions(-) diff --git a/docs/superpowers/plans/2026-07-26-chat-unbounded-document-scan.md b/docs/superpowers/plans/2026-07-26-chat-unbounded-document-scan.md index 050c8f4f1..ad618e145 100644 --- a/docs/superpowers/plans/2026-07-26-chat-unbounded-document-scan.md +++ b/docs/superpowers/plans/2026-07-26-chat-unbounded-document-scan.md @@ -2,48 +2,77 @@ > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. -**Revision note (2026-08-08):** This plan was originally written 2026-07-26 against an older -version of `views.py`/`chat.py`. Both files have since changed (the `permitted_document_ids` -perf refactor landed, and `stream_chat_with_documents` grew an `output_language` parameter). -The bug itself is unchanged and still present; this revision updates line numbers, signatures, -and code snippets to match the code as of commit `fc242bb57` (current `dev` tip). Re-verify -line numbers again before implementing if more commits have landed on `chat.py` or `views.py` -in the meantime. - -**Existing partial work (found 2026-08-09):** a worktree already exists at -`.claude/worktrees/chat-unbounded-scan` on branch `worktree-chat-unbounded-scan`, with one -commit: `085040f18 Test: rewrite chat tests to use QuerySets, add bounded-lookup regression -test` -- this is Task 1 below, already done, and its diff matches this plan's Task 1 almost -exactly. However that worktree branched off `dev` **before** the `permitted_document_ids` perf -refactor landed, so its copy of `views.py` (and possibly other files) predates the current -`dev` tip and will conflict if merged as-is. Before starting Task 1, check whether to -`git rebase dev` that worktree branch (or cherry-pick just `085040f18`'s `test_chat.py` change -onto a fresh branch off current `dev`) rather than redoing the work from scratch -- do not -blindly re-implement Task 1 without first inspecting whether the existing commit can be -reused. - -**Benchmark/profiling status:** none exists yet for this specific bug. The original -`CHAT_UNBOUNDED_DOCUMENT_SCAN.md` explicitly called for benchmarking (query count + wall time, -scaling with library size) before designing the fix, matching the methodology used for the -sibling `perf/13314-*` branches -- this was missing from the task breakdown below until this -revision. See new **Task 0** and the addition to **Task 4**. - **Goal:** Stop `ChatStreamingView`'s "chat with my whole archive" path from materializing every accessible `Document` into Python memory on every chat message; bound the cost to the vector-store `IN`-filter id list plus at most `CHAT_RETRIEVER_TOP_K` (5) documents for the reference/permission lookup. **Architecture:** Change `documents` from a materialized `list[Document]` to a lazy `QuerySet[Document]` threaded through `ChatStreamingView.post` -> `stream_chat_with_documents` -> `_stream_chat_with_documents` -> `_get_document_references`. Build the vector-store `IN` filter from `documents.values_list("pk", flat=True)` (ids only, no row hydration) instead of iterating full `Document` instances. Reorder `_get_document_references` to run `retriever.retrieve()` first, then permission-check/hydrate only the (≤5) documents that `top_nodes` actually reference via `documents.filter(pk__in=candidate_ids)`, instead of hydrating every accessible document up front. **Tech Stack:** Django ORM (QuerySet), llama-index (`MetadataFilters`, `VectorIndexRetriever`), pytest + pytest-django. +## Background + +`ChatStreamingView.post` (`src/documents/views.py`), when the request has no `document_id` +(i.e. "chat with my whole archive" rather than "chat with this one document"), builds a +`QuerySet` of every `Document` the requesting user is permitted to view and passes it straight +into `stream_chat_with_documents(query_str, documents)` +(`src/paperless_ai/chat.py`), which calls into `_stream_chat_with_documents`. Two places there +force-materialize the entire queryset into Python objects, on **every single chat message**: + +1. `_document_id_filters(str(doc.pk) for doc in documents)` -- iterates every accessible + document just to build a `MetadataFilter(key="document_id", operator=IN, +value=sorted(doc_ids))` for the vector-store query. +2. `_get_document_references`'s `allowed_documents = {doc.pk: doc for doc in documents}` -- + hydrates every accessible `Document` row into a dict, just to look up at most + `MAX_CHAT_REFERENCES = 3` of them later. + +Meanwhile the actual retrieval only ever wants `CHAT_RETRIEVER_TOP_K = 5` nodes, and shows at +most 3 references. So the cost of _every_ chat message -- not a background job, an interactive +request a user is staring at a spinner for -- scales with total accessible-document count, not +with the ~5 documents that actually matter to the answer. This is worse than an equivalent +scan in a background Celery task: a user is waiting on it in real time, on every message, and +the cost grows as the library grows regardless of how good or bad the actual answer needs to +be. + +**What this plan fixes (and what it deliberately doesn't):** + +1. Stop materializing full `Document` rows for the filter step -- `_document_id_filters` only + needs a list of ids, not hydrated rows (Task 2, Step 3). +2. Stop permission-checking/hydrating the whole accessible set before knowing which documents + were even retrieved -- flip the order so retrieval happens first (bounded by + `CHAT_RETRIEVER_TOP_K = 5`), then permission-check only those results (Task 2, Step 4). The + permission check itself is unchanged in substance -- a document is only surfaced if it's in + the caller's permission-scoped queryset -- only its timing and the amount of data it touches + change. +3. **Out of scope:** the vector-store-side `IN (...)` filter still needs the full list of + accessible document ids to constrain the KNN search to permitted documents -- that's + inherent to "chat with my whole (permitted) archive" and can't be avoided by filtering after + the fact (doing so would leak un-permitted document content into the LLM context). Whether + that `IN`-list itself is a performance problem for the vector store at very large scale is a + separate, unimplemented investigation and is explicitly not addressed by this plan. + +## Global Constraints + +- Backend lint/format: ruff, line length 88, double quotes, single-line isort imports (from `CLAUDE.md`). +- Type checking: mypy + pyrefly; do not introduce new violations beyond the frozen baseline (`.mypy-baseline.txt`, `.pyrefly-baseline.json`). +- Tests: pytest/pytest-django; match the style of the file being edited (`src/paperless_ai/tests/test_chat.py` is already idiomatic pytest with fixtures). +- The existing permission check semantics MUST be preserved exactly: a document referenced by a retrieved node is only surfaced/cited if it is in the caller's permission-scoped `documents` queryset. No behavior change to what a user is allowed to see, only to when/how much is loaded to check it. +- Preserve `output_language` threading through `stream_chat_with_documents` / `_stream_chat_with_documents` unchanged -- it is unrelated to this fix but must not be dropped by a careless signature rewrite. +- Do not touch the vector-store-side `IN (...)` filter question (see Background, point 3) -- out of scope for this plan. + **Suggested delegation (Claude Code `Agent` tool `subagent_type` + model tier):** - Task 0 (benchmark baseline -- open-ended: choosing a harness, interpreting numbers, deciding what "proves the bug" means): `python-pro` or `django-developer` at **Sonnet** tier. Not mechanical enough for Haiku -- it requires judgment about what to measure and whether the resulting numbers actually support the claimed scaling behavior, and it's the evidence the rest of the plan's justification rests on. -- Task 1 (test rewrite -- mechanical: swap list literals for querysets/MagicMocks per the exact snippets already written out in this plan): `django-developer` at **Haiku** tier. The transformations are fully specified here (copy-paste-adjacent), so a fast/cheap model is sufficient; escalate to Sonnet only if the agent reports the current file has drifted from what this plan quotes. Before dispatching, check whether `.claude/worktrees/chat-unbounded-scan` commit `085040f18` can be rebased/cherry-picked instead of redone (see "Existing partial work" note above). +- Task 1 (test rewrite -- mechanical: swap list literals for querysets/MagicMocks per the exact snippets already written out in this plan): `django-developer` at **Haiku** tier. The transformations are fully specified here (copy-paste-adjacent), so a fast/cheap model is sufficient; escalate to Sonnet only if the agent reports the current file has drifted from what this plan quotes. - Task 2 (`chat.py` rework -- the actual bug fix, changes runtime permission-check ordering): `django-developer` at **Sonnet** tier (or whatever the session's default is). This is the correctness-sensitive core of the change -- worth the stronger model even though the code is also fully specified, because a subtle mistake here (e.g. querying `documents` before `.filter(pk__in=...)` narrows it) reintroduces the exact bug being fixed. -- Task 3 (`views.py` one-line change + locating/running the right view tests): `django-developer` at **Haiku** tier for the one-line edit; if the test-discovery grep in Step 2 turns up ambiguity (unlike Task 1, this step isn't fully pre-solved -- see the verification note below naming `test_permission_filtering_security.py`), let it escalate or hand off rather than guessing. -- Task 4 (full verification, lint/type baselines, doc status update, before/after benchmark comparison): a `code-reviewer` subagent (or the `code-review` skill) at **Sonnet** tier or above for the correctness/permission-scoping review, paired with whichever agent ran Task 0 (same one, if possible, so it can compare against numbers it already understands) for the benchmark re-run in Step 0. Not a good candidate for Haiku -- both the permission-scoping check and the benchmark interpretation require judgment. +- Task 3 (`views.py` one-line change + locating/running the right view tests): `django-developer` at **Haiku** tier for the one-line edit; if the test-discovery grep in Step 2 turns up ambiguity, let it escalate or hand off rather than guessing. +- Task 4 (full verification, lint/type baselines, before/after benchmark comparison): a `code-reviewer` subagent (or the `code-review` skill) at **Sonnet** tier or above for the correctness/permission-scoping review, paired with whichever agent ran Task 0 (same one, if possible, so it can compare against numbers it already understands) for the benchmark re-run in Step 0. Not a good candidate for Haiku -- both the permission-scoping check and the benchmark interpretation require judgment. - Use `superpowers:subagent-driven-development` to run Tasks 0-3 as independent-but-ordered subagent dispatches with review checkpoints between them, per this plan's header. -## Current code (as of `fc242bb57`, for reference while implementing) +--- + +## Current code (as of `dev` commit `fc242bb57`, for reference while implementing) + +Re-verify these line numbers against the live files before editing -- they will drift as other +work lands on `dev`. `src/documents/views.py:2245-2286` (`ChatStreamingView.post`): @@ -93,12 +122,14 @@ class ChatStreamingView(GenericAPIView[Any]): ``` Note: the whole-library `else` branch already returns a `QuerySet` (`permitted_document_ids` -returns a lazy `QuerySet[int]` per `documents/permissions.py:223-237`) — the bug is entirely +returns a lazy `QuerySet[int]`, see `src/documents/permissions.py`) -- the bug is entirely inside `chat.py`, which force-materializes it. Only the single-document `if` branch needs to change (`[document]` -> a one-row `QuerySet`), purely so both branches share the same type. -`src/paperless_ai/chat.py:84-176` (`_get_document_references`, `stream_chat_with_documents`, -`_stream_chat_with_documents`): +`src/paperless_ai/chat.py` (`_get_document_references`, `stream_chat_with_documents`, +`_stream_chat_with_documents` -- abridged excerpt, elisions and inline comments below are +annotations for this plan, not literal source; re-read the live file rather than treating this +as a copy-paste-ready contiguous block): ```python def _get_document_references( @@ -139,25 +170,8 @@ def _stream_chat_with_documents( references = _get_document_references(documents, top_nodes) ``` -All three signatures need to carry `output_language: str | None = None` through unchanged — -this parameter did not exist when the plan was first drafted; do not drop it. - -(Note: the excerpt above is abridged for readability — elisions (`...`) and inline -`# <-- full materialization` comments are annotations added for this plan, not literal source. -`_stream_chat_with_documents` actually continues to line 209 in the real file. Each quoted line -does match the source verbatim; re-read the live file rather than treating this block as a -copy-paste-ready contiguous excerpt.) - -## Global Constraints - -- Backend lint/format: ruff, line length 88, double quotes, single-line isort imports (from `CLAUDE.md`). -- Type checking: mypy + pyrefly; do not introduce new violations beyond the frozen baseline (`.mypy-baseline.txt`, `.pyrefly-baseline.json`). -- Tests: pytest/pytest-django; match the style of the file being edited (`src/paperless_ai/tests/test_chat.py` is already idiomatic pytest with fixtures). -- The existing permission check semantics MUST be preserved exactly: a document referenced by a retrieved node is only surfaced/cited if it is in the caller's permission-scoped `documents` queryset. No behavior change to what a user is allowed to see, only to when/how much is loaded to check it. -- Preserve `output_language` threading through all three functions — it is unrelated to this fix but must not be dropped by a careless signature rewrite. -- Do not touch the vector-store-side `IN (...)` filter question (whether a large id list is itself a KNN scaling concern) -- that is flagged as a separate, unimplemented follow-up in `CHAT_UNBOUNDED_DOCUMENT_SCAN.md` and is out of scope for this plan. - ---- +All three signatures need to carry `output_language: str | None = None` through unchanged -- +this parameter is unrelated to the fix but must not be dropped. ## File Structure @@ -184,7 +198,7 @@ copy-paste-ready contiguous excerpt.) Seed libraries at a few sizes (e.g. 10, 100, 1000 documents) via `DocumentFactory.create_batch(n)` (see `src/documents/tests/factories.py`), matching the -pattern already used in this file's own `test_get_document_references_only_queries_referenced_documents` +pattern already used in this plan's own `test_get_document_references_only_queries_referenced_documents` test (Task 1, Step 3) which seeds 200. Wrap the call path in Django's `django.test.utils.CaptureQueriesContext` (or the `django_assert_num_queries` fixture for a fixed expected count, but here you want the _actual_ count at each size, not just an @@ -192,13 +206,13 @@ assertion) plus `time.perf_counter()` for wall time. `src/profiling.py`'s `profi context manager already bundles both (query count/time + memory) if you'd rather reuse it than hand-roll `CaptureQueriesContext`. -- [ ] **Step 2: Run the benchmark against the two hot spots identified in `CHAT_UNBOUNDED_DOCUMENT_SCAN.md`** +- [ ] **Step 2: Run the benchmark against the two hot spots described in Background** Specifically measure, at each library size: -1. `_document_id_filters(str(doc.pk) for doc in documents)` (`chat.py:151`) -- the filter-list +1. `_document_id_filters(str(doc.pk) for doc in documents)` (`chat.py`) -- the filter-list build. -2. `_get_document_references(documents, top_nodes)` (`chat.py:84-110`) -- the reference +2. `_get_document_references(documents, top_nodes)` (`chat.py`) -- the reference lookup, with `top_nodes` fixed at a small constant (e.g. 1-3 nodes) regardless of library size, to isolate the effect of accessible-library size on this specific function (this is the function the fix changes the most). @@ -240,9 +254,9 @@ just keep the recorded numbers from Step 3. - [ ] **Step 1: Replace list-based `documents` fixtures with `QuerySet`-shaped values** -In `src/paperless_ai/tests/test_chat.py`, the `mock_document` fixture (line 39-46) is a +In `src/paperless_ai/tests/test_chat.py`, the `mock_document` fixture (around line 39-46) is a `MagicMock`, not a real row, so it cannot be used with a real `QuerySet.filter(pk=...)` -lookup. Replace its use in `test_stream_chat_with_one_document_retrieval` (line 108) with a +lookup. Replace its use in `test_stream_chat_with_one_document_retrieval` with a real `DocumentFactory.create()` instance and pass `Document.objects.filter(pk=document.pk)`: ```python @@ -267,12 +281,13 @@ def test_stream_chat_with_one_document_retrieval(patch_embed_nodes) -> None: mock_client_cls.return_value = mock_client mock_client.llm = MagicMock() - mock_node = TextNode( - text="This is node content.", - metadata={"document_id": str(document.pk), "title": "Test Document"}, - ) mock_index = MagicMock() - mock_index.vector_store.get_nodes.return_value = [mock_node] + mock_index.vector_store.get_nodes.return_value = [ + TextNode( + text="This is node content.", + metadata={"document_id": str(document.pk), "title": "Test Document"}, + ), + ] mock_load_index.return_value = mock_index mock_retriever_instance = MagicMock() @@ -313,16 +328,16 @@ def test_stream_chat_with_one_document_retrieval(patch_embed_nodes) -> None: Remove the `mock_document` fixture only if nothing else in the file still uses it (check with `rg -n "mock_document" src/paperless_ai/tests/test_chat.py` after this step). -Apply the equivalent change to `test_stream_chat_with_multiple_documents_retrieval` (line 174): +Apply the equivalent change to `test_stream_chat_with_multiple_documents_retrieval`: replace `doc1 = MagicMock(pk=1, ...)` / `doc2 = MagicMock(pk=2, ...)` with two `DocumentFactory.create(...)` instances, and pass `documents = Document.objects.filter(pk__in=[doc1.pk, doc2.pk])` to `stream_chat_with_documents`. Update the node/reference metadata to use the real created pks instead of hardcoded `"1"`/`"2"`. -For the three non-DB tests (`test_stream_chat_empty_document_list` line 234, -`test_stream_chat_no_matching_nodes` line 241, -`test_stream_chat_unexpected_failure_returns_generic_error` line 261), replace the list +For the three non-DB tests (`test_stream_chat_empty_document_list`, +`test_stream_chat_no_matching_nodes`, +`test_stream_chat_unexpected_failure_returns_generic_error`), replace the list arguments with values that behave like an (unevaluated) `QuerySet` without touching the database: @@ -358,9 +373,11 @@ Add this helper near the top of the file (after `assert_chat_output`) and use Add the necessary import: `from documents.models import Document` at the top of the file. -- [ ] **Step 2: Rewrite `test_no_nodes_yields_no_content_message` and `test_chat_filter_contains_only_requested_document_ids` (in `TestStreamChatRetrieval`, line 292) to pass a QuerySet** +- [ ] **Step 2: Rewrite the two `TestStreamChatRetrieval` tests to pass a QuerySet** -Both already use real `DocumentFactory` documents and `django_db`. Change the calls: +Both `test_no_nodes_yields_no_content_message` and +`test_chat_filter_contains_only_requested_document_ids` (in class `TestStreamChatRetrieval`) +already use real `DocumentFactory` documents and `django_db`. Change the calls: ```python out = list(chat.stream_chat_with_documents("question?", Document.objects.filter(pk=doc.pk))) @@ -374,7 +391,7 @@ list(chat.stream_chat_with_documents("question?", Document.objects.filter(pk=inc Add a new test proving `_get_document_references` only touches documents that `top_nodes` actually reference, not every document in the passed queryset. This is the direct regression -test for the bug described in `CHAT_UNBOUNDED_DOCUMENT_SCAN.md`: +test for the bug described in this plan's Background section: ```python @pytest.mark.django_db @@ -469,8 +486,8 @@ short-circuits without hitting the database at all.) ``` This still touches every accessible document's id (inherent to scoping the vector-store `IN` -filter to the permitted set -- see `CHAT_UNBOUNDED_DOCUMENT_SCAN.md`'s point 3, which remains -out of scope), but no longer loads full `Document` rows -- just a flat list of integers. +filter to the permitted set -- see Background, point 3, which remains out of scope), but no +longer loads full `Document` rows -- just a flat list of integers. - [ ] **Step 4: Rework `_get_document_references` to hydrate only referenced documents** @@ -541,7 +558,7 @@ git commit -m "Fix: bound chat document reference lookup to retrieved nodes inst **Files:** -- Modify: `src/documents/views.py` (`ChatStreamingView.post`, currently lines 2245-2286 -- re-check with `rg -n "class ChatStreamingView" src/documents/views.py` before editing, in case other changes shifted it) +- Modify: `src/documents/views.py` (`ChatStreamingView.post` -- re-locate with `rg -n "class ChatStreamingView" src/documents/views.py` before editing, in case other changes shifted it) **Interfaces:** @@ -564,24 +581,23 @@ changes inside `chat.py` are what stop that queryset from being force-materializ - [ ] **Step 2: Run the view tests** -Three test locations cover this view (confirmed by an independent verification pass, see -plan revision note at the bottom -- re-check with +Three test locations cover this view (re-check with `rg -n "ChatStreamingView|/api/chat|stream_chat_with_documents" src/documents/tests/*.py` if -more time has passed): +more time has passed since this plan was written): 1. `src/documents/tests/test_views.py`, class `TestAIChatStreamingView` -- patches `stream_chat_with_documents` entirely, doesn't inspect `documents`' type. 2. `src/documents/tests/test_api_chat.py`, class `TestChatStreamingViewInputValidation` -- input-validation only, doesn't reach `documents` construction. -3. `src/documents/tests/test_permission_filtering_security.py:177`, class +3. `src/documents/tests/test_permission_filtering_security.py`, class `TestAiChatAllDocumentsPermissionBoundary`, test - `test_chat_all_documents_excludes_unshared_document` (~line 190-215) -- **this is the one - that actually matters for this change**: it asserts on `kwargs["documents"]` from the - mocked `stream_chat_with_documents` call (`{doc.pk for doc in kwargs["documents"]}`), - pinning the permission-scoping behavior this plan touches. Read this test specifically - before/after the change, not just via a blind `-k chat` filter -- iterating a `QuerySet` - with a set comprehension works the same as iterating a `list`, so it should keep passing - unchanged, but confirm rather than assume. + `test_chat_all_documents_excludes_unshared_document` -- **this is the one that actually + matters for this change**: it asserts on `kwargs["documents"]` from the mocked + `stream_chat_with_documents` call (`{doc.pk for doc in kwargs["documents"]}`), pinning the + permission-scoping behavior this plan touches. Read this test specifically before/after the + change, not just via a blind `-k chat` filter -- iterating a `QuerySet` with a set + comprehension works the same as iterating a `list`, so it should keep passing unchanged, but + confirm rather than assume. Run: @@ -615,15 +631,13 @@ that tests pass -- prove the improvement, don't assume it. Expect: `CHAT_RETRIEVER_TOP_K = 5`) instead of scaling with library size. - `_document_id_filters`' cost is unchanged in shape (Task 2 only avoids hydrating full `Document` rows there, via `.values_list("pk", flat=True)`; it still touches every accessible - id -- see `CHAT_UNBOUNDED_DOCUMENT_SCAN.md` point 3, still out of scope) but should show - reduced wall time/memory from not loading full rows. + id -- see Background, point 3, still out of scope) but should show reduced wall time/memory + from not loading full rows. Record the before/after comparison (e.g. as a small table: library size, before query -count/time, after query count/time) either back into Task 0's section of this plan or into -`CHAT_UNBOUNDED_DOCUMENT_SCAN.md`'s status update (Step 4 below) -- whichever reads better as a -permanent record for a future reader. If the numbers do NOT show the expected improvement, -stop and treat that as a signal the fix is incomplete or wrong before proceeding to the rest of -this task's steps. +count/time, after query count/time) back into Task 0's section of this plan. If the numbers do +NOT show the expected improvement, stop and treat that as a signal the fix is incomplete or +wrong before proceeding to the rest of this task's steps. - [ ] **Step 1: Run the full `paperless_ai` and relevant `documents` test suites** @@ -644,35 +658,19 @@ uv run prek run --all-files Expected: clean, and no new violations beyond `.mypy-baseline.txt` / `.pyrefly-baseline.json`. -- [ ] **Step 3: Re-read `CHAT_UNBOUNDED_DOCUMENT_SCAN.md`'s "What a fix probably looks like" section and confirm points 1 and 2 are addressed** +- [ ] **Step 3: Confirm both in-scope fixes from Background are addressed** Point 1 (don't materialize full `Document` rows for the filter step) -- addressed by Task 2 Step 3. Point 2 (permission-check only `top_nodes`, bounded by `CHAT_RETRIEVER_TOP_K`) -- addressed by Task 2 Step 4. -Point 3 (whether the vector-store `IN (...)` filter itself is a KNN scaling concern) -- explicitly out of scope for this plan; leave `CHAT_UNBOUNDED_DOCUMENT_SCAN.md` in place (do not delete it) and update its "Status" line to reflect that points 1-2 are fixed and point 3 remains open, so a future session picking up the sqlite-vec investigation has accurate context. - -- [ ] **Step 4: Update `CHAT_UNBOUNDED_DOCUMENT_SCAN.md` status** - -Change the header: - -```markdown -**Status:** points 1-2 fixed (see git log for this file's directory); point 3 -(large `IN (...)` clause inside the vector-store KNN query) remains open and -unimplemented -- see "What a fix probably looks like", item 3. -``` - -- [ ] **Step 5: Commit the doc update** - -```bash -git add CHAT_UNBOUNDED_DOCUMENT_SCAN.md -git commit -m "Docs: update chat unbounded scan status after partial fix" -``` +Point 3 (whether the vector-store `IN (...)` filter itself is a KNN scaling concern) remains +explicitly out of scope for this plan -- if it needs tracking as future work, open a fresh +issue/note for it rather than reviving old diagnosis documents. --- ## Self-Review Notes -- **Spec coverage:** `CHAT_UNBOUNDED_DOCUMENT_SCAN.md` items 1 and 2 under "What a fix probably looks like" are both implemented (Task 2). Item 3 (vector-store `IN` filter scaling) is explicitly called out as needing separate source-level investigation before any code change, and is left open per the doc's own guidance not to guess -- tracked via the Task 4 doc update rather than silently dropped. +- **Spec coverage:** both in-scope points from Background ("don't materialize full `Document` rows for the filter step" and "permission-check only `top_nodes`, bounded by `CHAT_RETRIEVER_TOP_K`") are implemented in Task 2. The vector-store `IN` filter scaling question is explicitly out of scope and not silently dropped -- it's called out in Background, Global Constraints, and Task 4 Step 3. - **Placeholder scan:** no TBD/TODO markers; every step has literal code. - **Type consistency:** `documents: QuerySet[Document]` is consistent across `stream_chat_with_documents`, `_stream_chat_with_documents`, `_get_document_references`, and both call sites in `views.py`. `_build_document_reference`'s signature is unchanged (still takes a hydrated `Document`). `output_language` threading is preserved unchanged throughout. -- **2026-08-08 revision:** original plan referenced a `get_objects_for_user_owner_aware`-based whole-library branch and an `output_language`-less signature, neither of which match current `dev`. This revision was cross-checked line-by-line against `src/paperless_ai/chat.py` and `src/documents/views.py` at commit `fc242bb57` and against the current `src/paperless_ai/tests/test_chat.py`. The underlying bug (full materialization in `_document_id_filters` call and `_get_document_references`) is unchanged and still present. A follow-up `django-developer` agent independently re-verified this revision against the live repo: all line numbers in `test_chat.py` and the `chat.py`/`views.py` snippets matched verbatim; it additionally surfaced that `src/documents/tests/test_permission_filtering_security.py::TestAiChatAllDocumentsPermissionBoundary` (not originally named in the plan) is the test that actually pins the permission-scoping behavior this change touches -- folded into Task 3 Step 2 above. - +- **Self-contained:** this plan does not depend on any other document, branch, or worktree existing -- all context needed to execute it (bug diagnosis, current code, fix design) is inlined above.