mirror of
https://github.com/paperless-ngx/paperless-ngx.git
synced 2026-08-11 21:33:19 +00:00
Compare commits
5
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
6a02b87dde | ||
|
|
59a2651804 | ||
|
|
a99f63e059 | ||
|
|
939cb52f6e | ||
|
|
855669ddf9 |
@@ -948,10 +948,11 @@ for display in the web interface.
|
||||
|
||||
!!! note
|
||||
|
||||
The **remote OCR parser** (Azure AI) always produces a searchable
|
||||
PDF and stores it as the archive copy, regardless of this setting.
|
||||
`ARCHIVE_FILE_GENERATION=never` has no effect when the remote
|
||||
parser handles a document.
|
||||
The **remote OCR parser** (Azure AI) also honors this setting: when
|
||||
no archive is requested (`never`, or `auto` with a born-digital PDF),
|
||||
the remote engine is skipped entirely and locally-extracted text is
|
||||
used instead, avoiding an unnecessary API call and a duplicate text
|
||||
layer.
|
||||
|
||||
#### [`PAPERLESS_OCR_CLEAN=<mode>`](#PAPERLESS_OCR_CLEAN) {#PAPERLESS_OCR_CLEAN}
|
||||
|
||||
|
||||
@@ -187,10 +187,11 @@ PAPERLESS_ARCHIVE_FILE_GENERATION=auto
|
||||
|
||||
### Remote OCR parser
|
||||
|
||||
If you use the **remote OCR parser** (Azure AI), note that it always produces a
|
||||
searchable PDF and stores it as the archive copy. `ARCHIVE_FILE_GENERATION=never`
|
||||
has no effect for documents handled by the remote parser - the archive is produced
|
||||
unconditionally by the remote engine.
|
||||
If you use the **remote OCR parser** (Azure AI), `ARCHIVE_FILE_GENERATION` is
|
||||
honored the same way as for the local engine: when no archive is requested
|
||||
(`never`, or `auto` with a born-digital PDF), the remote engine is skipped
|
||||
entirely and locally-extracted text is used instead, avoiding an unnecessary
|
||||
API call and a duplicate text layer.
|
||||
|
||||
## Search Index (Whoosh -> Tantivy)
|
||||
|
||||
|
||||
+3
-1
@@ -576,7 +576,9 @@ The following workflow action types are available:
|
||||
- Tags, correspondent, document type and storage path
|
||||
- Document owner
|
||||
- View and / or edit permissions to users or groups
|
||||
- Custom fields. Note that no value for the field will be set
|
||||
- Custom fields, optionally with a value. If no value is set, the field is only added to the
|
||||
document and any value it may already have is left untouched. If a value is set, it will
|
||||
overwrite an existing value of that field on the document.
|
||||
|
||||
##### Removal {#workflow-action-removal}
|
||||
|
||||
|
||||
+8
-8
@@ -599,7 +599,7 @@
|
||||
</context-group>
|
||||
<context-group purpose="location">
|
||||
<context context-type="sourcefile">src/app/components/manage/saved-views/saved-views.component.html</context>
|
||||
<context context-type="linenumber">84,85</context>
|
||||
<context context-type="linenumber">85,86</context>
|
||||
</context-group>
|
||||
</trans-unit>
|
||||
<trans-unit id="3768927257183755959" datatype="html">
|
||||
@@ -670,7 +670,7 @@
|
||||
</context-group>
|
||||
<context-group purpose="location">
|
||||
<context context-type="sourcefile">src/app/components/manage/saved-views/saved-views.component.html</context>
|
||||
<context context-type="linenumber">85,86</context>
|
||||
<context context-type="linenumber">86,87</context>
|
||||
</context-group>
|
||||
</trans-unit>
|
||||
<trans-unit id="5079885666748292382" datatype="html">
|
||||
@@ -9907,7 +9907,7 @@
|
||||
</context-group>
|
||||
<context-group purpose="location">
|
||||
<context context-type="sourcefile">src/app/components/manage/saved-views/saved-views.component.ts</context>
|
||||
<context context-type="linenumber">296</context>
|
||||
<context context-type="linenumber">314</context>
|
||||
</context-group>
|
||||
</trans-unit>
|
||||
<trans-unit id="2620006875434695386" datatype="html">
|
||||
@@ -10234,7 +10234,7 @@
|
||||
</context-group>
|
||||
<context-group purpose="location">
|
||||
<context context-type="sourcefile">src/app/components/manage/saved-views/saved-views.component.ts</context>
|
||||
<context context-type="linenumber">290</context>
|
||||
<context context-type="linenumber">308</context>
|
||||
</context-group>
|
||||
</trans-unit>
|
||||
<trans-unit id="3501895737484542570" datatype="html">
|
||||
@@ -10346,28 +10346,28 @@
|
||||
<source>Saved view "<x id="PH" equiv-text="savedView.name"/>" deleted.</source>
|
||||
<context-group purpose="location">
|
||||
<context context-type="sourcefile">src/app/components/manage/saved-views/saved-views.component.ts</context>
|
||||
<context context-type="linenumber">160</context>
|
||||
<context context-type="linenumber">178</context>
|
||||
</context-group>
|
||||
</trans-unit>
|
||||
<trans-unit id="1660419335376265526" datatype="html">
|
||||
<source>Views saved successfully.</source>
|
||||
<context-group purpose="location">
|
||||
<context context-type="sourcefile">src/app/components/manage/saved-views/saved-views.component.ts</context>
|
||||
<context context-type="linenumber">237</context>
|
||||
<context context-type="linenumber">255</context>
|
||||
</context-group>
|
||||
</trans-unit>
|
||||
<trans-unit id="1699877326523238632" datatype="html">
|
||||
<source>Error while saving views.</source>
|
||||
<context-group purpose="location">
|
||||
<context context-type="sourcefile">src/app/components/manage/saved-views/saved-views.component.ts</context>
|
||||
<context context-type="linenumber">242</context>
|
||||
<context context-type="linenumber">260</context>
|
||||
</context-group>
|
||||
</trans-unit>
|
||||
<trans-unit id="4919025779187821586" datatype="html">
|
||||
<source>Note: Sharing saved views does not share the underlying documents.</source>
|
||||
<context-group purpose="location">
|
||||
<context context-type="sourcefile">src/app/components/manage/saved-views/saved-views.component.ts</context>
|
||||
<context context-type="linenumber">278</context>
|
||||
<context context-type="linenumber">296</context>
|
||||
</context-group>
|
||||
</trans-unit>
|
||||
<trans-unit id="1229748338333965418" datatype="html">
|
||||
|
||||
+4
-4
@@ -52,10 +52,10 @@ describe('CustomFieldsValuesComponent', () => {
|
||||
})
|
||||
|
||||
it('should set selectedFields and map values correctly', () => {
|
||||
component.value = { 1: 'value1' }
|
||||
component.selectedFields = [1, 2]
|
||||
expect(component.selectedFields).toEqual([1, 2])
|
||||
expect(component.value).toEqual({ 1: 'value1', 2: null })
|
||||
component.value = { 1: 'value1', 3: 0, 4: false }
|
||||
component.selectedFields = [1, 2, 3, 4]
|
||||
expect(component.selectedFields).toEqual([1, 2, 3, 4])
|
||||
expect(component.value).toEqual({ 1: 'value1', 2: null, 3: 0, 4: false })
|
||||
})
|
||||
|
||||
it('should return the correct custom field by id', () => {
|
||||
|
||||
+1
-1
@@ -77,7 +77,7 @@ export class CustomFieldsValuesComponent extends AbstractInputComponent<Object>
|
||||
this._selectedFields = newFields
|
||||
// map the selected fields to an object with field_id as key and value as value
|
||||
this.value = newFields.reduce((acc, fieldId) => {
|
||||
acc[fieldId] = this.value?.[fieldId] || null
|
||||
acc[fieldId] = this.value?.[fieldId] ?? null
|
||||
return acc
|
||||
}, {})
|
||||
this.onChange(this.value)
|
||||
|
||||
@@ -7,7 +7,7 @@
|
||||
</pngx-page-header>
|
||||
<form [formGroup]="savedViewsForm" (ngSubmit)="save()">
|
||||
<ul class="list-group mb-3" formGroupName="savedViews">
|
||||
@for (view of savedViews(); track view) {
|
||||
@for (view of pagedSavedViews(); track view) {
|
||||
<li class="list-group-item py-3">
|
||||
<div [formGroupName]="view.id">
|
||||
<div class="row">
|
||||
@@ -81,6 +81,11 @@
|
||||
}
|
||||
</ul>
|
||||
|
||||
<button type="button" (click)="reset()" class="btn btn-outline-secondary mb-2" [disabled]="(isDirty$ | async) === false" i18n>Cancel</button>
|
||||
<button type="submit" class="btn btn-primary ms-2 mb-2" [disabled]="(isDirty$ | async) === false" i18n>Save</button>
|
||||
<div class="d-flex align-items-center mb-3">
|
||||
<button type="button" (click)="reset()" class="btn btn-outline-secondary mb-2" [disabled]="(isDirty$ | async) === false" i18n>Cancel</button>
|
||||
<button type="submit" class="btn btn-primary ms-2 mb-2" [disabled]="(isDirty$ | async) === false" i18n>Save</button>
|
||||
@if (savedViews()?.length > pageSize) {
|
||||
<ngb-pagination class="ms-auto" [pageSize]="pageSize" [collectionSize]="savedViews().length" [page]="page()" [maxSize]="5" (pageChange)="page.set($event)" size="sm" aria-label="Pagination"></ngb-pagination>
|
||||
}
|
||||
</div>
|
||||
</form>
|
||||
|
||||
@@ -4,6 +4,7 @@ import { provideHttpClientTesting } from '@angular/common/http/testing'
|
||||
import { signal } from '@angular/core'
|
||||
import { ComponentFixture, TestBed } from '@angular/core/testing'
|
||||
import { FormsModule, ReactiveFormsModule } from '@angular/forms'
|
||||
import { By } from '@angular/platform-browser'
|
||||
import { NgbModal, NgbModule } from '@ng-bootstrap/ng-bootstrap'
|
||||
import { NgxBootstrapIconsModule, allIcons } from 'ngx-bootstrap-icons'
|
||||
import { Subject, of, throwError } from 'rxjs'
|
||||
@@ -222,6 +223,44 @@ describe('SavedViewsComponent', () => {
|
||||
).toEqual(view.show_on_dashboard)
|
||||
})
|
||||
|
||||
it('should page saved views, clamp the page if views are removed', () => {
|
||||
const manyViews = Array.from({ length: 30 }, (_, i) => ({
|
||||
id: i + 1,
|
||||
name: `view${i + 1}`,
|
||||
})) as SavedView[]
|
||||
const listSpy = jest.spyOn(savedViewService, 'list').mockReturnValue(
|
||||
of({
|
||||
all: manyViews.map((v) => v.id),
|
||||
count: manyViews.length,
|
||||
results: manyViews.concat([]),
|
||||
})
|
||||
)
|
||||
component.ngOnInit()
|
||||
fixture.detectChanges()
|
||||
expect(listSpy).toHaveBeenCalledWith(1, 100000, null, false, {
|
||||
full_perms: true,
|
||||
})
|
||||
expect(component.pagedSavedViews()).toHaveLength(25)
|
||||
expect(fixture.debugElement.query(By.css('ngb-pagination'))).not.toBeNull()
|
||||
// all views have controls, not just the current page
|
||||
expect(
|
||||
Object.keys(component.savedViewsForm.get('savedViews').value)
|
||||
).toHaveLength(30)
|
||||
|
||||
component.page.set(2)
|
||||
expect(component.pagedSavedViews()).toHaveLength(5)
|
||||
|
||||
listSpy.mockReturnValue(
|
||||
of({
|
||||
all: manyViews.slice(0, 25).map((v) => v.id),
|
||||
count: 25,
|
||||
results: manyViews.slice(0, 25),
|
||||
})
|
||||
)
|
||||
component.ngOnInit()
|
||||
expect(component.page()).toEqual(1)
|
||||
})
|
||||
|
||||
it('should support editing permissions', () => {
|
||||
const confirmClicked = new Subject<any>()
|
||||
const modalRef = {
|
||||
|
||||
@@ -1,12 +1,19 @@
|
||||
import { AsyncPipe } from '@angular/common'
|
||||
import { Component, OnDestroy, OnInit, inject, signal } from '@angular/core'
|
||||
import {
|
||||
Component,
|
||||
OnDestroy,
|
||||
OnInit,
|
||||
computed,
|
||||
inject,
|
||||
signal,
|
||||
} from '@angular/core'
|
||||
import {
|
||||
FormControl,
|
||||
FormGroup,
|
||||
FormsModule,
|
||||
ReactiveFormsModule,
|
||||
} from '@angular/forms'
|
||||
import { NgbModal } from '@ng-bootstrap/ng-bootstrap'
|
||||
import { NgbModal, NgbPaginationModule } from '@ng-bootstrap/ng-bootstrap'
|
||||
import { dirtyCheck } from '@ngneat/dirty-check-forms'
|
||||
import { NgxBootstrapIconsModule } from 'ngx-bootstrap-icons'
|
||||
import { BehaviorSubject, Observable, of, switchMap, takeUntil } from 'rxjs'
|
||||
@@ -42,6 +49,7 @@ import { LoadingComponentWithPermissions } from '../../loading-component/loading
|
||||
FormsModule,
|
||||
ReactiveFormsModule,
|
||||
AsyncPipe,
|
||||
NgbPaginationModule,
|
||||
NgxBootstrapIconsModule,
|
||||
],
|
||||
})
|
||||
@@ -58,6 +66,14 @@ export class SavedViewsComponent
|
||||
DisplayMode = DisplayMode
|
||||
|
||||
readonly savedViews = signal<SavedView[]>(undefined)
|
||||
readonly page = signal(1)
|
||||
public readonly pageSize = 25
|
||||
// All views are loaded at init, so paging is only for display
|
||||
readonly pagedSavedViews = computed(() => {
|
||||
const start = (this.page() - 1) * this.pageSize
|
||||
return this.savedViews()?.slice(start, start + this.pageSize)
|
||||
})
|
||||
|
||||
private savedViewsGroup = new FormGroup({})
|
||||
public savedViewsForm: FormGroup = new FormGroup({
|
||||
savedViews: this.savedViewsGroup,
|
||||
@@ -84,9 +100,11 @@ export class SavedViewsComponent
|
||||
private reloadViews(): void {
|
||||
this.loading.set(true)
|
||||
this.savedViewService
|
||||
.list(null, null, null, false, { full_perms: true })
|
||||
.list(1, 100000, null, false, { full_perms: true })
|
||||
.subscribe((r) => {
|
||||
this.savedViews.set(r.results)
|
||||
const pageCount = Math.ceil(r.results.length / this.pageSize)
|
||||
this.page.update((page) => Math.min(page, Math.max(1, pageCount)))
|
||||
this.initialize()
|
||||
})
|
||||
}
|
||||
|
||||
@@ -3213,6 +3213,13 @@ class WorkflowActionSerializer(serializers.ModelSerializer[WorkflowAction]):
|
||||
{"assign_title": f'Invalid f-string detected: "{e.args[0]}"'},
|
||||
)
|
||||
|
||||
if attrs.get("assign_custom_fields_values"):
|
||||
# Empty strings treated as None to avoid unexpected behavior
|
||||
attrs["assign_custom_fields_values"] = {
|
||||
field_id: (None if value == "" else value)
|
||||
for field_id, value in attrs["assign_custom_fields_values"].items()
|
||||
}
|
||||
|
||||
if (
|
||||
"type" in attrs
|
||||
and attrs["type"] == WorkflowAction.WorkflowActionType.EMAIL
|
||||
|
||||
@@ -422,6 +422,11 @@ class TestApiWorkflows(DirectoriesMixin, APITestCase):
|
||||
json.dumps(
|
||||
{
|
||||
"assign_title": "",
|
||||
"assign_custom_fields": [self.cf1.id, self.cf2.id],
|
||||
"assign_custom_fields_values": {
|
||||
str(self.cf1.id): "",
|
||||
str(self.cf2.id): 0,
|
||||
},
|
||||
},
|
||||
),
|
||||
content_type="application/json",
|
||||
@@ -429,6 +434,10 @@ class TestApiWorkflows(DirectoriesMixin, APITestCase):
|
||||
self.assertEqual(response.status_code, status.HTTP_201_CREATED)
|
||||
action = WorkflowAction.objects.get(id=response.data["id"])
|
||||
self.assertIsNone(action.assign_title)
|
||||
self.assertEqual(
|
||||
action.assign_custom_fields_values,
|
||||
{str(self.cf1.id): None, str(self.cf2.id): 0},
|
||||
)
|
||||
|
||||
response = self.client.post(
|
||||
self.ENDPOINT_TRIGGERS,
|
||||
|
||||
@@ -2000,6 +2000,55 @@ class TestWorkflows(
|
||||
r"Doc added in \w{3,}",
|
||||
) # Match any 3-letter month name
|
||||
|
||||
def test_document_updated_workflow_existing_custom_field_empty_value(self) -> None:
|
||||
"""
|
||||
GIVEN:
|
||||
- Existing workflow with UPDATED trigger and action that assigns a custom field
|
||||
with an empty value
|
||||
WHEN:
|
||||
- Document is updated that already contains the field with a value
|
||||
THEN:
|
||||
- The existing value is left untouched, see GH #13627
|
||||
"""
|
||||
trigger = WorkflowTrigger.objects.create(
|
||||
type=WorkflowTrigger.WorkflowTriggerType.DOCUMENT_UPDATED,
|
||||
filter_has_document_type=self.dt,
|
||||
)
|
||||
action = WorkflowAction.objects.create()
|
||||
action.assign_custom_fields.add(self.cf1)
|
||||
action.assign_custom_fields_values = {self.cf1.pk: ""}
|
||||
action.save()
|
||||
w = Workflow.objects.create(
|
||||
name="Workflow 1",
|
||||
order=0,
|
||||
)
|
||||
w.triggers.add(trigger)
|
||||
w.actions.add(action)
|
||||
w.save()
|
||||
|
||||
doc = Document.objects.create(
|
||||
title="sample test",
|
||||
correspondent=self.c,
|
||||
original_filename="sample.pdf",
|
||||
)
|
||||
CustomFieldInstance.objects.create(
|
||||
document=doc,
|
||||
field=self.cf1,
|
||||
value_text="existing value",
|
||||
)
|
||||
|
||||
superuser = User.objects.create_superuser("superuser")
|
||||
self.client.force_authenticate(user=superuser)
|
||||
|
||||
self.client.patch(
|
||||
f"/api/documents/{doc.id}/",
|
||||
{"document_type": self.dt.id},
|
||||
format="json",
|
||||
)
|
||||
|
||||
doc.refresh_from_db()
|
||||
self.assertEqual(doc.custom_fields.get(field=self.cf1).value, "existing value")
|
||||
|
||||
def test_document_updated_workflow_existing_custom_field(self) -> None:
|
||||
"""
|
||||
GIVEN:
|
||||
|
||||
@@ -2267,7 +2267,7 @@ class ChatStreamingView(GenericAPIView[Any]):
|
||||
if not has_perms_owner_aware(request.user, "view_document", document):
|
||||
return HttpResponseForbidden("Insufficient permissions")
|
||||
|
||||
documents = [document]
|
||||
documents = Document.objects.filter(pk=document.pk)
|
||||
else:
|
||||
documents = Document.objects.filter(
|
||||
id__in=permitted_document_ids(request.user),
|
||||
|
||||
@@ -105,7 +105,8 @@ def apply_assignment_to_document(
|
||||
field=field,
|
||||
document=document,
|
||||
).first()
|
||||
if instance and args[value_field_name] is not None:
|
||||
# empty string is indistinguishable from no value in the UI
|
||||
if instance and args[value_field_name] not in (None, ""):
|
||||
setattr(instance, value_field_name, args[value_field_name])
|
||||
instance.save()
|
||||
elif not instance:
|
||||
|
||||
@@ -3,7 +3,9 @@ Built-in remote-OCR document parser.
|
||||
|
||||
Handles documents by sending them to a configured remote OCR engine
|
||||
(currently Azure AI Vision / Document Intelligence) and retrieving both
|
||||
the extracted text and a searchable PDF with an embedded text layer.
|
||||
the extracted text and a searchable PDF with an embedded text layer. For
|
||||
born-digital PDFs that need no archive copy, the remote call is skipped
|
||||
entirely in favor of locally-extracted text (see ``RemoteDocumentParser.parse``).
|
||||
|
||||
When no engine is configured, ``score()`` returns ``None`` so the parser
|
||||
is effectively invisible to the registry — the tesseract parser handles
|
||||
@@ -22,6 +24,8 @@ from typing import Self
|
||||
from django.conf import settings
|
||||
|
||||
from documents.parsers import ParseError
|
||||
from paperless.parsers.utils import extract_pdf_text
|
||||
from paperless.parsers.utils import post_process_text
|
||||
from paperless.version import __full_version_str__
|
||||
|
||||
if TYPE_CHECKING:
|
||||
@@ -70,8 +74,11 @@ class RemoteDocumentParser:
|
||||
"""Parse documents via a remote OCR API (currently Azure AI Vision).
|
||||
|
||||
This parser sends documents to a remote engine that returns both
|
||||
extracted text and a searchable PDF with an embedded text layer.
|
||||
It does not depend on Tesseract or ocrmypdf.
|
||||
extracted text and a searchable PDF with an embedded text layer,
|
||||
except when ``parse()`` is called with ``produce_archive=False`` for
|
||||
a PDF, in which case the remote call is skipped and only locally
|
||||
extracted text is returned (no archive). It does not depend on
|
||||
Tesseract or ocrmypdf.
|
||||
|
||||
Class attributes
|
||||
----------------
|
||||
@@ -160,8 +167,11 @@ class RemoteDocumentParser:
|
||||
Returns
|
||||
-------
|
||||
bool
|
||||
Always True — the remote engine always returns a PDF with an
|
||||
embedded text layer that serves as the archive copy.
|
||||
Always True — the remote engine is capable of returning a PDF
|
||||
with an embedded text layer to serve as the archive copy.
|
||||
Whether it actually does so for a given document depends on
|
||||
``produce_archive`` passed to :meth:`parse` (see there for when
|
||||
the remote engine call, and thus archive generation, is skipped).
|
||||
"""
|
||||
return True
|
||||
|
||||
@@ -218,6 +228,12 @@ class RemoteDocumentParser:
|
||||
) -> None:
|
||||
"""Send the document to the remote engine and store results.
|
||||
|
||||
When *produce_archive* is False for a PDF, the caller (via
|
||||
``documents.consumer.should_produce_archive``) has already determined
|
||||
that the document is born-digital and needs no archive — skip the
|
||||
remote engine entirely rather than re-OCRing it and creating a
|
||||
duplicate text layer.
|
||||
|
||||
Parameters
|
||||
----------
|
||||
document_path:
|
||||
@@ -225,8 +241,8 @@ class RemoteDocumentParser:
|
||||
mime_type:
|
||||
Detected MIME type of the document.
|
||||
produce_archive:
|
||||
Ignored — the remote engine always returns a searchable PDF,
|
||||
which is stored as the archive copy regardless of this flag.
|
||||
Whether an archive copy is wanted. For PDFs, False skips the
|
||||
remote engine and uses locally-extracted text instead.
|
||||
"""
|
||||
config = RemoteEngineConfig(
|
||||
engine=settings.REMOTE_OCR_ENGINE,
|
||||
@@ -241,6 +257,16 @@ class RemoteDocumentParser:
|
||||
self._text = ""
|
||||
return
|
||||
|
||||
if not produce_archive and mime_type == "application/pdf":
|
||||
logger.debug(
|
||||
"Remote OCR: skipped — no archive requested, "
|
||||
"using locally-extracted text",
|
||||
)
|
||||
self._text = (
|
||||
post_process_text(extract_pdf_text(document_path, log=logger)) or ""
|
||||
)
|
||||
return
|
||||
|
||||
if config.engine == "azureai":
|
||||
self._text = self._azure_ai_vision_parse(document_path, config)
|
||||
|
||||
|
||||
@@ -337,6 +337,117 @@ class TestRemoteParserParse:
|
||||
assert remote_parser.get_date() is None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# parse() — produce_archive=False skips the remote engine (PDFs only)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestRemoteParserSkipsWhenNoArchiveWanted:
|
||||
"""When the caller has already decided no archive is needed for a PDF
|
||||
(documents.consumer.should_produce_archive), the remote engine call is
|
||||
skipped entirely in favor of locally-extracted text.
|
||||
"""
|
||||
|
||||
def test_pdf_skips_azure_when_no_archive_requested(
|
||||
self,
|
||||
remote_parser: RemoteDocumentParser,
|
||||
simple_digital_pdf_file: Path,
|
||||
azure_client: Mock,
|
||||
) -> None:
|
||||
"""
|
||||
GIVEN: produce_archive=False for a PDF
|
||||
WHEN: parse() is called
|
||||
THEN: Azure is never invoked, no archive is produced, and text
|
||||
comes from local pdftotext extraction
|
||||
"""
|
||||
remote_parser.parse(
|
||||
simple_digital_pdf_file,
|
||||
"application/pdf",
|
||||
produce_archive=False,
|
||||
)
|
||||
|
||||
azure_client.begin_analyze_document.assert_not_called()
|
||||
assert remote_parser.get_archive_path() is None
|
||||
assert remote_parser.get_text() != ""
|
||||
|
||||
def test_pdf_no_archive_requested_text_matches_local_extraction(
|
||||
self,
|
||||
remote_parser: RemoteDocumentParser,
|
||||
simple_digital_pdf_file: Path,
|
||||
azure_client: Mock,
|
||||
mocker: MockerFixture,
|
||||
) -> None:
|
||||
"""
|
||||
GIVEN: produce_archive=False for a PDF
|
||||
WHEN: parse() is called
|
||||
THEN: the returned text is exactly the locally-extracted text,
|
||||
not anything from the (unused) Azure mock
|
||||
"""
|
||||
mocker.patch(
|
||||
"paperless.parsers.remote.extract_pdf_text",
|
||||
return_value="Local digital text.",
|
||||
)
|
||||
|
||||
remote_parser.parse(
|
||||
simple_digital_pdf_file,
|
||||
"application/pdf",
|
||||
produce_archive=False,
|
||||
)
|
||||
|
||||
assert remote_parser.get_text() == "Local digital text."
|
||||
|
||||
def test_pdf_no_archive_requested_closes_no_client(
|
||||
self,
|
||||
remote_parser: RemoteDocumentParser,
|
||||
simple_digital_pdf_file: Path,
|
||||
azure_client: Mock,
|
||||
) -> None:
|
||||
remote_parser.parse(
|
||||
simple_digital_pdf_file,
|
||||
"application/pdf",
|
||||
produce_archive=False,
|
||||
)
|
||||
|
||||
azure_client.close.assert_not_called()
|
||||
|
||||
def test_non_pdf_still_calls_azure_when_no_archive_requested(
|
||||
self,
|
||||
remote_parser: RemoteDocumentParser,
|
||||
simple_digital_pdf_file: Path,
|
||||
azure_client: Mock,
|
||||
) -> None:
|
||||
"""
|
||||
Images have no local-text fallback, so produce_archive=False does
|
||||
not skip the remote engine for non-PDF MIME types.
|
||||
"""
|
||||
remote_parser.parse(
|
||||
simple_digital_pdf_file,
|
||||
"image/png",
|
||||
produce_archive=False,
|
||||
)
|
||||
|
||||
azure_client.begin_analyze_document.assert_called_once()
|
||||
assert remote_parser.get_text() == _DEFAULT_TEXT
|
||||
|
||||
@pytest.mark.usefixtures("no_engine_settings")
|
||||
def test_unconfigured_engine_takes_precedence_over_skip(
|
||||
self,
|
||||
remote_parser: RemoteDocumentParser,
|
||||
simple_digital_pdf_file: Path,
|
||||
) -> None:
|
||||
"""An unconfigured engine still short-circuits before the
|
||||
produce_archive check, returning empty text as before.
|
||||
"""
|
||||
remote_parser.parse(
|
||||
simple_digital_pdf_file,
|
||||
"application/pdf",
|
||||
produce_archive=False,
|
||||
)
|
||||
|
||||
assert remote_parser.get_text() == ""
|
||||
assert remote_parser.get_archive_path() is None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# parse() — Azure failure path
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -2,6 +2,8 @@ import json
|
||||
import logging
|
||||
import sys
|
||||
|
||||
from django.db.models import QuerySet
|
||||
|
||||
from documents.models import Document
|
||||
from paperless.config import AIConfig
|
||||
from paperless_ai.client import AIClient
|
||||
@@ -82,10 +84,21 @@ def _build_document_reference(
|
||||
|
||||
|
||||
def _get_document_references(
|
||||
documents: list[Document],
|
||||
documents: QuerySet[Document],
|
||||
top_nodes: list,
|
||||
) -> list[dict[str, int | str]]:
|
||||
allowed_documents = {doc.pk: doc for doc in documents}
|
||||
candidate_ids: set[int] = set()
|
||||
for node in top_nodes:
|
||||
try:
|
||||
candidate_ids.add(int(node.metadata["document_id"]))
|
||||
except (KeyError, TypeError, ValueError): # pragma: no cover
|
||||
continue
|
||||
|
||||
if not candidate_ids:
|
||||
return []
|
||||
|
||||
allowed_documents = {doc.pk: doc for doc in documents.filter(pk__in=candidate_ids)}
|
||||
|
||||
references: list[dict[str, int | str]] = []
|
||||
seen_document_ids: set[int] = set()
|
||||
|
||||
@@ -119,7 +132,7 @@ def _format_chat_metadata_trailer(references: list[dict[str, int | str]]) -> str
|
||||
|
||||
def stream_chat_with_documents(
|
||||
query_str: str,
|
||||
documents: list[Document],
|
||||
documents: QuerySet[Document],
|
||||
output_language: str | None = None,
|
||||
):
|
||||
try:
|
||||
@@ -135,10 +148,10 @@ def stream_chat_with_documents(
|
||||
|
||||
def _stream_chat_with_documents(
|
||||
query_str: str,
|
||||
documents: list[Document],
|
||||
documents: QuerySet[Document],
|
||||
output_language: str | None = None,
|
||||
):
|
||||
if not documents:
|
||||
if not documents.exists():
|
||||
yield CHAT_NO_CONTENT_MESSAGE
|
||||
return
|
||||
|
||||
@@ -148,7 +161,9 @@ def _stream_chat_with_documents(
|
||||
from llama_index.core.retrievers import VectorIndexRetriever
|
||||
|
||||
config = AIConfig()
|
||||
filters = _document_id_filters(str(doc.pk) for doc in documents)
|
||||
filters = _document_id_filters(
|
||||
str(pk) for pk in documents.values_list("pk", flat=True)
|
||||
)
|
||||
|
||||
# Hold the shared read lock for the whole operation: the query engine
|
||||
# retrieves from the vector store again during synthesis, so the connection
|
||||
|
||||
@@ -3,10 +3,12 @@ from unittest.mock import MagicMock
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
from django.db.models.signals import post_init
|
||||
from llama_index.core import settings as llama_settings
|
||||
from llama_index.core.embeddings.mock_embed_model import MockEmbedding
|
||||
from llama_index.core.schema import TextNode
|
||||
|
||||
from documents.models import Document
|
||||
from documents.tests.factories import DocumentFactory
|
||||
from paperless_ai import chat
|
||||
from paperless_ai import indexing
|
||||
@@ -36,16 +38,6 @@ def patch_embed_nodes():
|
||||
yield mock_embed_nodes
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def mock_document():
|
||||
doc = MagicMock()
|
||||
doc.pk = 1
|
||||
doc.title = "Test Document"
|
||||
doc.filename = "test_file.pdf"
|
||||
doc.content = "This is the document content."
|
||||
return doc
|
||||
|
||||
|
||||
def assert_chat_output(
|
||||
output: list[str],
|
||||
*,
|
||||
@@ -61,6 +53,13 @@ def assert_chat_output(
|
||||
}
|
||||
|
||||
|
||||
def _fake_documents_queryset(pks: list[int]) -> MagicMock:
|
||||
qs = MagicMock()
|
||||
qs.exists.return_value = bool(pks)
|
||||
qs.values_list.return_value = pks
|
||||
return qs
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("output_language", "expected_language_line"),
|
||||
[
|
||||
@@ -107,9 +106,10 @@ def test_build_refine_prompt(
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_stream_chat_with_one_document_retrieval(
|
||||
mock_document,
|
||||
patch_embed_nodes,
|
||||
) -> None:
|
||||
document = DocumentFactory.create(title="Test Document", content="ignored")
|
||||
documents = Document.objects.filter(pk=document.pk)
|
||||
with (
|
||||
patch("paperless_ai.chat.AIClient") as mock_client_cls,
|
||||
patch("paperless_ai.chat.load_or_build_index") as mock_load_index,
|
||||
@@ -124,22 +124,19 @@ def test_stream_chat_with_one_document_retrieval(
|
||||
mock_client_cls.return_value = mock_client
|
||||
mock_client.llm = MagicMock()
|
||||
|
||||
mock_node = TextNode(
|
||||
text="This is node content.",
|
||||
metadata={"document_id": str(mock_document.pk), "title": "Test Document"},
|
||||
)
|
||||
mock_index = MagicMock()
|
||||
# Simulate get_nodes returning nodes (content exists)
|
||||
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()
|
||||
mock_retriever_instance.retrieve.return_value = [
|
||||
MagicMock(
|
||||
metadata={
|
||||
"document_id": str(mock_document.pk),
|
||||
"title": "Test Document",
|
||||
},
|
||||
metadata={"document_id": str(document.pk), "title": "Test Document"},
|
||||
),
|
||||
]
|
||||
|
||||
@@ -153,7 +150,7 @@ def test_stream_chat_with_one_document_retrieval(
|
||||
"llama_index.core.retrievers.VectorIndexRetriever",
|
||||
return_value=mock_retriever_instance,
|
||||
):
|
||||
output = list(stream_chat_with_documents("What is this?", [mock_document]))
|
||||
output = list(stream_chat_with_documents("What is this?", documents))
|
||||
|
||||
mock_query_engine.query.assert_called_once_with("What is this?")
|
||||
synthesizer_kwargs = mock_get_response_synthesizer.call_args.kwargs
|
||||
@@ -166,13 +163,16 @@ def test_stream_chat_with_one_document_retrieval(
|
||||
output,
|
||||
expected_chunks=["chunk1", "chunk2"],
|
||||
expected_references=[
|
||||
{"id": mock_document.pk, "title": "Test Document"},
|
||||
{"id": document.pk, "title": "Test Document"},
|
||||
],
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_stream_chat_with_multiple_documents_retrieval(patch_embed_nodes) -> None:
|
||||
doc1 = DocumentFactory.create(title="Document 1", content="ignored")
|
||||
doc2 = DocumentFactory.create(title="Document 2", content="ignored")
|
||||
documents = Document.objects.filter(pk__in=[doc1.pk, doc2.pk])
|
||||
with (
|
||||
patch("paperless_ai.chat.AIClient") as mock_client_cls,
|
||||
patch("paperless_ai.chat.load_or_build_index") as mock_load_index,
|
||||
@@ -184,23 +184,23 @@ def test_stream_chat_with_multiple_documents_retrieval(patch_embed_nodes) -> Non
|
||||
mock_client_cls.return_value = mock_client
|
||||
mock_client.llm = MagicMock()
|
||||
|
||||
mock_node1 = TextNode(
|
||||
text="Content for doc 1.",
|
||||
metadata={"document_id": "1", "title": "Document 1"},
|
||||
)
|
||||
mock_node2 = TextNode(
|
||||
text="Content for doc 2.",
|
||||
metadata={"document_id": "2", "title": "Document 2"},
|
||||
)
|
||||
mock_index = MagicMock()
|
||||
# Simulate get_nodes returning nodes (content exists)
|
||||
mock_index.vector_store.get_nodes.return_value = [mock_node1, mock_node2]
|
||||
mock_index.vector_store.get_nodes.return_value = [
|
||||
TextNode(
|
||||
text="Content for doc 1.",
|
||||
metadata={"document_id": str(doc1.pk), "title": "Document 1"},
|
||||
),
|
||||
TextNode(
|
||||
text="Content for doc 2.",
|
||||
metadata={"document_id": str(doc2.pk), "title": "Document 2"},
|
||||
),
|
||||
]
|
||||
mock_load_index.return_value = mock_index
|
||||
|
||||
mock_retriever_instance = MagicMock()
|
||||
mock_retriever_instance.retrieve.return_value = [
|
||||
MagicMock(metadata={"document_id": "1", "title": "Document 1"}),
|
||||
MagicMock(metadata={"document_id": "2", "title": "Document 2"}),
|
||||
MagicMock(metadata={"document_id": str(doc1.pk), "title": "Document 1"}),
|
||||
MagicMock(metadata={"document_id": str(doc2.pk), "title": "Document 2"}),
|
||||
]
|
||||
|
||||
mock_response_stream = MagicMock()
|
||||
@@ -210,14 +210,11 @@ def test_stream_chat_with_multiple_documents_retrieval(patch_embed_nodes) -> Non
|
||||
mock_query_engine_cls.return_value = mock_query_engine
|
||||
mock_query_engine.query.return_value = mock_response_stream
|
||||
|
||||
doc1 = MagicMock(pk=1, title="Document 1", filename="doc1.pdf")
|
||||
doc2 = MagicMock(pk=2, title="Document 2", filename="doc2.pdf")
|
||||
|
||||
with patch(
|
||||
"llama_index.core.retrievers.VectorIndexRetriever",
|
||||
return_value=mock_retriever_instance,
|
||||
):
|
||||
output = list(stream_chat_with_documents("What's up?", [doc1, doc2]))
|
||||
output = list(stream_chat_with_documents("What's up?", documents))
|
||||
|
||||
mock_query_engine.query.assert_called_once_with("What's up?")
|
||||
patch_embed_nodes.assert_not_called()
|
||||
@@ -225,15 +222,15 @@ def test_stream_chat_with_multiple_documents_retrieval(patch_embed_nodes) -> Non
|
||||
output,
|
||||
expected_chunks=["chunk1", "chunk2"],
|
||||
expected_references=[
|
||||
{"id": 1, "title": "Document 1"},
|
||||
{"id": 2, "title": "Document 2"},
|
||||
{"id": doc1.pk, "title": "Document 1"},
|
||||
{"id": doc2.pk, "title": "Document 2"},
|
||||
],
|
||||
)
|
||||
|
||||
|
||||
def test_stream_chat_empty_document_list() -> None:
|
||||
with patch("paperless_ai.chat.load_or_build_index") as mock_load_index:
|
||||
output = list(stream_chat_with_documents("Any info?", []))
|
||||
output = list(stream_chat_with_documents("Any info?", Document.objects.none()))
|
||||
mock_load_index.assert_not_called()
|
||||
assert output == ["Sorry, I couldn't find any content to answer your question."]
|
||||
|
||||
@@ -253,7 +250,9 @@ def test_stream_chat_no_matching_nodes() -> None:
|
||||
mock_index.vector_store.get_nodes.return_value = []
|
||||
mock_load_index.return_value = mock_index
|
||||
|
||||
output = list(stream_chat_with_documents("Any info?", [MagicMock(pk=1)]))
|
||||
output = list(
|
||||
stream_chat_with_documents("Any info?", _fake_documents_queryset([1])),
|
||||
)
|
||||
|
||||
assert output == ["Sorry, I couldn't find any content to answer your question."]
|
||||
|
||||
@@ -282,7 +281,9 @@ def test_stream_chat_unexpected_failure_returns_generic_error(caplog) -> None:
|
||||
)
|
||||
mock_retriever_cls.return_value = mock_retriever
|
||||
|
||||
output = list(stream_chat_with_documents("Any info?", [MagicMock(pk=1)]))
|
||||
output = list(
|
||||
stream_chat_with_documents("Any info?", _fake_documents_queryset([1])),
|
||||
)
|
||||
|
||||
assert output == [CHAT_ERROR_MESSAGE]
|
||||
assert "Failed to stream document chat response" in caplog.text
|
||||
@@ -298,7 +299,12 @@ class TestStreamChatRetrieval:
|
||||
) -> None:
|
||||
doc = DocumentFactory.create(content="hello world")
|
||||
# Nothing indexed for this document yet.
|
||||
out = list(chat.stream_chat_with_documents("question?", [doc]))
|
||||
out = list(
|
||||
chat.stream_chat_with_documents(
|
||||
"question?",
|
||||
Document.objects.filter(pk=doc.pk),
|
||||
),
|
||||
)
|
||||
assert chat.CHAT_NO_CONTENT_MESSAGE in out
|
||||
|
||||
def test_chat_filter_contains_only_requested_document_ids(
|
||||
@@ -332,7 +338,12 @@ class TestStreamChatRetrieval:
|
||||
side_effect=capture_retriever,
|
||||
)
|
||||
|
||||
list(chat.stream_chat_with_documents("question?", [included]))
|
||||
list(
|
||||
chat.stream_chat_with_documents(
|
||||
"question?",
|
||||
Document.objects.filter(pk=included.pk),
|
||||
),
|
||||
)
|
||||
|
||||
assert captured_filters, "VectorIndexRetriever was never constructed"
|
||||
filt = captured_filters[0]
|
||||
@@ -340,3 +351,47 @@ class TestStreamChatRetrieval:
|
||||
filter_values = filt.filters[0].value
|
||||
assert str(included.pk) in filter_values
|
||||
assert str(excluded.pk) not in filter_values
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_get_document_references_only_queries_referenced_documents(
|
||||
self,
|
||||
django_assert_num_queries,
|
||||
) -> None:
|
||||
"""Building references must not hydrate every document the caller is
|
||||
permitted to see -- only the (<= CHAT_RETRIEVER_TOP_K) documents that
|
||||
the retriever actually returned nodes for.
|
||||
"""
|
||||
referenced = DocumentFactory.create(title="Referenced Document")
|
||||
# Many more documents are "accessible" but never referenced by a node.
|
||||
DocumentFactory.create_batch(200)
|
||||
|
||||
documents = Document.objects.all()
|
||||
top_nodes = [
|
||||
MagicMock(
|
||||
metadata={
|
||||
"document_id": str(referenced.pk),
|
||||
"title": "Referenced Document",
|
||||
},
|
||||
),
|
||||
]
|
||||
|
||||
hydrated_count = 0
|
||||
|
||||
def _count_hydration(sender, instance, **kwargs):
|
||||
nonlocal hydrated_count
|
||||
hydrated_count += 1
|
||||
|
||||
post_init.connect(_count_hydration, sender=Document)
|
||||
try:
|
||||
# One query: `documents.filter(pk__in=candidate_ids)` for the single
|
||||
# referenced id. No query should scale with the 200 unreferenced documents.
|
||||
with django_assert_num_queries(1):
|
||||
references = chat._get_document_references(documents, top_nodes)
|
||||
finally:
|
||||
post_init.disconnect(_count_hydration, sender=Document)
|
||||
|
||||
# The bug this guards against: the old code hydrated all 201 accessible
|
||||
# documents via `{doc.pk: doc for doc in documents}` before filtering by
|
||||
# top_nodes. Only the referenced document should ever be constructed.
|
||||
assert hydrated_count == 1
|
||||
assert references == [{"id": referenced.pk, "title": "Referenced Document"}]
|
||||
|
||||
Reference in New Issue
Block a user