mirror of
https://github.com/tiennm99/DocsGPT.git
synced 2026-10-14 16:16:48 +00:00
fix: handle non text uploads more carefully
This commit is contained in:
1 parent
82691314ca
commit
3030aba39d
21 files changed
+729
-40
No files matched your search
@@ -2394,3 +2394,71 @@ class TestAttachmentExtractionGate:
|
||||
assert "truncated" in prompt
|
||||
assert "100,000" in prompt
|
||||
assert "250,000" in prompt
|
||||
|
||||
def test_scanned_pdf_row_tells_the_model_it_cannot_read_the_file(self):
|
||||
"""A no-text PDF reaches this path only on a model that reads neither
|
||||
PDFs nor images. Dropping it silently lets the model answer as if
|
||||
nothing were attached; naming it lets the model tell the user."""
|
||||
handler = ConcreteHandler()
|
||||
messages = [{"role": "system", "content": "sys"}]
|
||||
attachments = [{
|
||||
"id": "a1",
|
||||
"filename": "bylaws.pdf",
|
||||
"mime_type": "application/pdf",
|
||||
"content": "",
|
||||
"metadata": {"extraction": {"status": "no_text"}},
|
||||
}]
|
||||
|
||||
prompt = handler._append_unsupported_attachments(messages, attachments)[0]["content"]
|
||||
|
||||
assert "bylaws.pdf" in prompt
|
||||
assert "cannot read" in prompt
|
||||
assert "Attached file content" not in prompt
|
||||
|
||||
def test_image_without_text_is_named_instead_of_an_empty_block(self):
|
||||
handler = ConcreteHandler()
|
||||
messages = [{"role": "system", "content": "sys"}]
|
||||
attachments = [{
|
||||
"id": "a1",
|
||||
"filename": "photo.webp",
|
||||
"mime_type": "image/webp",
|
||||
"content": "",
|
||||
"metadata": {"extraction": {"status": "ok"}},
|
||||
}]
|
||||
|
||||
prompt = handler._append_unsupported_attachments(messages, attachments)[0]["content"]
|
||||
|
||||
assert "photo.webp" in prompt
|
||||
assert "cannot read" in prompt
|
||||
assert "Attached file content" not in prompt
|
||||
|
||||
def test_image_with_extracted_text_is_still_appended(self):
|
||||
handler = ConcreteHandler()
|
||||
messages = [{"role": "system", "content": "sys"}]
|
||||
attachments = [{
|
||||
"id": "a1",
|
||||
"filename": "receipt.png",
|
||||
"mime_type": "image/png",
|
||||
"content": "Total due: 42 EUR",
|
||||
"metadata": {"extraction": {"status": "ok"}},
|
||||
}]
|
||||
|
||||
prompt = handler._append_unsupported_attachments(messages, attachments)[0]["content"]
|
||||
|
||||
assert "Total due: 42 EUR" in prompt
|
||||
assert "cannot read" not in prompt
|
||||
|
||||
def test_failed_scan_row_is_still_skipped_silently(self):
|
||||
handler = ConcreteHandler()
|
||||
messages = [{"role": "system", "content": "sys"}]
|
||||
attachments = [{
|
||||
"id": "a1",
|
||||
"filename": "broken.pdf",
|
||||
"mime_type": "application/pdf",
|
||||
"content": None,
|
||||
"metadata": {"extraction": {"status": "failed", "error": "boom"}},
|
||||
}]
|
||||
|
||||
result = handler._append_unsupported_attachments(messages, attachments)
|
||||
|
||||
assert result[0]["content"] == "sys"
|
||||
@@ -527,22 +527,28 @@ def test_tableize_never_touches_docling_reroute(monkeypatch):
|
||||
def test_scanned_pdf_with_near_empty_fallback_fails_loudly_when_ocr_off(tmp_path):
|
||||
"""A scan whose fallback (OCR off) extracts almost nothing must fail with
|
||||
an actionable message, not be stored as an empty document."""
|
||||
from docsgpt.parser.file.base_parser import NoTextLayerError
|
||||
|
||||
path = _scanned_pdf(tmp_path / "scan.pdf")
|
||||
fallback = _RecordingFallback(result=" ")
|
||||
fallback.ocr_enabled = False
|
||||
parser = AnydocParser(fallback_parser=fallback)
|
||||
|
||||
with pytest.raises(DocumentParseError, match="OCR_ENABLED"):
|
||||
# Still a DocumentParseError for source ingestion, but typed, so the
|
||||
# attachment worker can keep the file for models that read it natively.
|
||||
with pytest.raises(NoTextLayerError, match="OCR_ENABLED"):
|
||||
parser.parse_file(path)
|
||||
assert parser.last_engine is None
|
||||
|
||||
|
||||
def test_scanned_pdf_with_near_empty_fallback_and_ocr_on_reports_it(tmp_path):
|
||||
from docsgpt.parser.file.base_parser import NoTextLayerError
|
||||
|
||||
path = _scanned_pdf(tmp_path / "scan.pdf")
|
||||
fallback = _RecordingFallback(result="x")
|
||||
fallback.ocr_enabled = True
|
||||
|
||||
with pytest.raises(DocumentParseError, match="even with OCR enabled"):
|
||||
with pytest.raises(NoTextLayerError, match="even with OCR enabled"):
|
||||
AnydocParser(fallback_parser=fallback).parse_file(path)
|
||||
|
||||
|
||||
|
||||
@@ -222,6 +222,31 @@ class TestSimpleDirectoryReaderLoadData:
|
||||
with pytest.raises(DocumentParseError, match="InvalidCxxCompiler"):
|
||||
reader.load_data()
|
||||
|
||||
def test_single_unparseable_file_keeps_the_parsers_error_type(self, tmp_path):
|
||||
"""The attachment worker tells a scan from a broken file by the error's type.
|
||||
|
||||
Re-wrapping the parser's exception in a plain ``DocumentParseError``
|
||||
would erase ``NoTextLayerError`` and turn every scanned PDF back into
|
||||
a failed upload.
|
||||
"""
|
||||
from docsgpt.parser.file.base_parser import NoTextLayerError
|
||||
from docsgpt.parser.file.bulk import SimpleDirectoryReader
|
||||
|
||||
(tmp_path / "scan.pdf").write_bytes(b"%PDF-1.7")
|
||||
|
||||
mock_parser = MagicMock()
|
||||
mock_parser.parser_config_set = True
|
||||
mock_parser.parse_file.side_effect = NoTextLayerError(
|
||||
"scan.pdf appears to be a scanned PDF (no text layer)"
|
||||
)
|
||||
|
||||
reader = SimpleDirectoryReader(
|
||||
input_files=[str(tmp_path / "scan.pdf")],
|
||||
file_extractor={".pdf": mock_parser},
|
||||
)
|
||||
with pytest.raises(NoTextLayerError, match="scanned PDF"):
|
||||
reader.load_data()
|
||||
|
||||
def test_all_files_unparseable_raises(self, tmp_path):
|
||||
"""Nothing parsed is a failed ingest, not an empty success."""
|
||||
from docsgpt.parser.file.base_parser import DocumentParseError
|
||||
@@ -588,6 +613,21 @@ class TestParserEngineSwitch:
|
||||
assert type(extractor[".xlsx"].fallback_parser).__name__ == "ExcelParser"
|
||||
assert type(extractor[".png"]).__name__ == "ImageParser"
|
||||
|
||||
def test_anydoc_without_docling_admits_every_image_format(self, monkeypatch):
|
||||
"""The production image ships neither docling nor OCR. An image is
|
||||
sent to the model as an image, so .webp/.tiff/.bmp need a parser
|
||||
entry or the worker refuses them before any model sees them."""
|
||||
pytest.importorskip("anydoc")
|
||||
import sys
|
||||
|
||||
from docsgpt.parser.file.bulk import get_default_file_extractor
|
||||
|
||||
monkeypatch.setitem(sys.modules, "docling", None)
|
||||
extractor = get_default_file_extractor(engine="anydoc")
|
||||
|
||||
for suffix in (".webp", ".tiff", ".tif", ".bmp"):
|
||||
assert type(extractor[suffix]).__name__ == "ImageParser", suffix
|
||||
|
||||
def test_docling_engine_keeps_docling_map(self):
|
||||
pytest.importorskip("docling")
|
||||
from docsgpt.parser.file.bulk import get_default_file_extractor
|
||||
|
||||
@@ -0,0 +1,64 @@
|
||||
"""Re-encoding images the model providers reject (TIFF, BMP) as PNG.
|
||||
|
||||
Chat attachments reach vision models as the stored file itself. OpenAI and
|
||||
Anthropic accept png/jpeg/webp/gif only, so a TIFF fax or a BMP screenshot has
|
||||
to be re-encoded before it can be sent at all.
|
||||
"""
|
||||
|
||||
import io
|
||||
|
||||
import pytest
|
||||
from PIL import Image
|
||||
|
||||
from docsgpt.parser.file.base_parser import DocumentParseError
|
||||
from docsgpt.parser.file.image_parser import (
|
||||
VISION_CONVERTIBLE_MIME_TYPES,
|
||||
convert_image_to_png,
|
||||
)
|
||||
|
||||
|
||||
def _encoded(fmt, frames=1, size=(40, 24)):
|
||||
"""An in-memory image of ``frames`` pages, each a different red level."""
|
||||
images = [Image.new("RGB", size, color=(i * 60, 20, 200)) for i in range(frames)]
|
||||
buf = io.BytesIO()
|
||||
if frames > 1:
|
||||
images[0].save(buf, format=fmt, save_all=True, append_images=images[1:])
|
||||
else:
|
||||
images[0].save(buf, format=fmt)
|
||||
buf.seek(0)
|
||||
return buf
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestConvertImageToPng:
|
||||
def test_tiff_becomes_png(self):
|
||||
png, frames = convert_image_to_png(_encoded("TIFF"))
|
||||
|
||||
decoded = Image.open(io.BytesIO(png))
|
||||
assert decoded.format == "PNG"
|
||||
assert decoded.size == (40, 24)
|
||||
assert frames == 1
|
||||
|
||||
def test_multi_page_tiff_keeps_the_first_page_and_reports_the_count(self):
|
||||
png, frames = convert_image_to_png(_encoded("TIFF", frames=3))
|
||||
|
||||
decoded = Image.open(io.BytesIO(png)).convert("RGB")
|
||||
assert frames == 3
|
||||
assert decoded.getpixel((0, 0))[0] == 0 # page one's red level
|
||||
|
||||
def test_bmp_becomes_png(self):
|
||||
png, frames = convert_image_to_png(_encoded("BMP"))
|
||||
|
||||
assert Image.open(io.BytesIO(png)).format == "PNG"
|
||||
assert frames == 1
|
||||
|
||||
def test_unreadable_bytes_raise_a_parse_error(self):
|
||||
with pytest.raises(DocumentParseError):
|
||||
convert_image_to_png(io.BytesIO(b"not an image"))
|
||||
|
||||
def test_only_formats_the_providers_reject_are_converted(self):
|
||||
assert {"image/tiff", "image/bmp"} <= VISION_CONVERTIBLE_MIME_TYPES
|
||||
assert not (
|
||||
{"image/png", "image/jpeg", "image/webp", "image/gif"}
|
||||
& VISION_CONVERTIBLE_MIME_TYPES
|
||||
)
|
||||
@@ -536,13 +536,16 @@ class TestNativeOcrImageParser:
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestExtractorWiring:
|
||||
def test_legacy_map_without_ocr_is_unchanged(self):
|
||||
def test_legacy_map_without_ocr_hands_every_image_to_image_parser(self):
|
||||
"""Without OCR an image still reaches the model as an image, so every
|
||||
image suffix the upload gate admits needs an entry. Only png/jpg/jpeg
|
||||
had one, and a .webp was refused outright (prod 2026-09-10)."""
|
||||
from docsgpt.parser.file.bulk import _legacy_file_extractor
|
||||
|
||||
extractor = _legacy_file_extractor()
|
||||
assert type(extractor[".pdf"]).__name__ == "PDFParser"
|
||||
assert type(extractor[".png"]).__name__ == "ImageParser"
|
||||
assert ".tiff" not in extractor
|
||||
for suffix in op.IMAGE_SUFFIXES:
|
||||
assert type(extractor[suffix]).__name__ == "ImageParser", suffix
|
||||
|
||||
def test_legacy_map_with_ocr_uses_native_parsers(self):
|
||||
from docsgpt.parser.file.bulk import _legacy_file_extractor
|
||||
|
||||
@@ -145,23 +145,24 @@ def test_enforce_parseable_attachment_rejects_binary_behind_a_bom(bom, tmp_path)
|
||||
def test_enforce_parseable_attachment_uses_the_extractor_it_is_given(tmp_path):
|
||||
"""The worker holds the live parser table; a trimmed install must not admit on trust.
|
||||
|
||||
Without docling the fallback extractor has no .webp handler, so a .webp
|
||||
would otherwise skip the content check and be read as plain text.
|
||||
Without docling the fallback extractor has no .vtt handler, so a binary
|
||||
file named .vtt would otherwise skip the content check and be read as
|
||||
plain text.
|
||||
"""
|
||||
from docsgpt.upload_limits import (
|
||||
enforce_parseable_attachment,
|
||||
UnsupportedUploadTypeError,
|
||||
)
|
||||
|
||||
path = tmp_path / "scan.webp"
|
||||
path.write_bytes(b"RIFF\x00\x00\x00\x00WEBPVP8 " + bytes(range(256)))
|
||||
path = tmp_path / "subs.vtt"
|
||||
path.write_bytes(b"\x00\x01\x02\x03" + bytes(range(256)))
|
||||
|
||||
# Default list: .webp is parser-backed, admitted on its name.
|
||||
enforce_parseable_attachment(path, "scan.webp")
|
||||
# Default list: .vtt is parser-backed, admitted on its name.
|
||||
enforce_parseable_attachment(path, "subs.vtt")
|
||||
|
||||
# The extractor actually loaded has no .webp parser.
|
||||
# The extractor actually loaded has no .vtt parser.
|
||||
with pytest.raises(UnsupportedUploadTypeError):
|
||||
enforce_parseable_attachment(path, "scan.webp", {".pdf", ".docx"})
|
||||
enforce_parseable_attachment(path, "subs.vtt", {".pdf", ".docx"})
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
|
||||
@@ -384,14 +384,14 @@ class TestAttachmentTypeGuard:
|
||||
):
|
||||
"""The guard judges against the parser table actually loaded.
|
||||
|
||||
Without docling the fallback extractor has no .webp parser, so a
|
||||
.webp the route admitted on its name would otherwise be opened as
|
||||
plain text here.
|
||||
The route admits ``.vtt`` by name, but a docling-less install has no
|
||||
VTT parser, so a binary file named ``.vtt`` would otherwise be opened
|
||||
as plain text here.
|
||||
"""
|
||||
from docsgpt import worker
|
||||
|
||||
local_path = tmp_path / "scan.webp"
|
||||
local_path.write_bytes(b"RIFF\x00\x00\x00\x00WEBPVP8 " + bytes(range(256)))
|
||||
local_path = tmp_path / "subs.vtt"
|
||||
local_path.write_bytes(b"\x00\x01\x02\x03" + bytes(range(256)))
|
||||
|
||||
events = []
|
||||
fake_storage = MagicMock(name="storage")
|
||||
@@ -399,7 +399,7 @@ class TestAttachmentTypeGuard:
|
||||
str(local_path)
|
||||
)
|
||||
monkeypatch.setattr(worker.StorageCreator, "get_storage", lambda: fake_storage)
|
||||
# The docling-less fallback table: images are handled, .webp is not.
|
||||
# A trimmed parser table: .png is handled, .vtt is not.
|
||||
monkeypatch.setattr(
|
||||
worker,
|
||||
"get_default_file_extractor",
|
||||
@@ -412,19 +412,176 @@ class TestAttachmentTypeGuard:
|
||||
)
|
||||
|
||||
file_info = {
|
||||
"filename": "scan.webp",
|
||||
"filename": "subs.vtt",
|
||||
"attachment_id": "507f1f77bcf86cd799439014",
|
||||
"path": "uploads/user1/attachments/scan.webp",
|
||||
"path": "uploads/user1/attachments/subs.vtt",
|
||||
"metadata": {"source": "chat"},
|
||||
}
|
||||
|
||||
with pytest.raises(
|
||||
worker.AttachmentRejectedError, match=r"Unsupported file type: \.webp"
|
||||
worker.AttachmentRejectedError, match=r"Unsupported file type: \.vtt"
|
||||
):
|
||||
worker.attachment_worker(task_self, file_info, "user1")
|
||||
|
||||
failed = [payload for name, payload in events if name == "attachment.failed"]
|
||||
assert failed and failed[0]["error"] == "Unsupported file type: .webp"
|
||||
assert failed and failed[0]["error"] == "Unsupported file type: .vtt"
|
||||
|
||||
def test_scanned_pdf_completes_without_text_for_models_that_read_it_natively(
|
||||
self, pg_conn, patch_worker_db, task_self, monkeypatch
|
||||
):
|
||||
"""A PDF with no text layer is still a usable attachment.
|
||||
|
||||
Models that read PDFs natively (or as page images) are sent the
|
||||
stored file, not its extracted text, so failing the upload only kept
|
||||
the file from a model that could read it (prod 2026-09-12: a paying
|
||||
user's scanned club bylaws). The row is kept with ``status: no_text``
|
||||
— text inlining still skips it — and the user is told it completed.
|
||||
"""
|
||||
from docsgpt import worker
|
||||
from docsgpt.parser.file.base_parser import NoTextLayerError
|
||||
|
||||
published: list[tuple[str, dict]] = []
|
||||
monkeypatch.setattr(
|
||||
worker,
|
||||
"publish_user_event",
|
||||
lambda user, event, payload, **kw: published.append((event, payload)),
|
||||
)
|
||||
fake_storage = MagicMock(name="storage")
|
||||
fake_storage.process_file.side_effect = NoTextLayerError(
|
||||
"bylaws.pdf appears to be a scanned PDF (no text layer), and "
|
||||
"PDFParser extracted almost nothing"
|
||||
)
|
||||
monkeypatch.setattr(worker.StorageCreator, "get_storage", lambda: fake_storage)
|
||||
monkeypatch.setattr(
|
||||
worker, "get_default_file_extractor", lambda ocr_enabled=False, pdf_text_fast_path=False: {}
|
||||
)
|
||||
|
||||
file_info = {
|
||||
"filename": "bylaws.pdf",
|
||||
"attachment_id": "507f1f77bcf86cd799439021",
|
||||
"path": "uploads/user1/attachments/bylaws.pdf",
|
||||
"metadata": {"source": "chat"},
|
||||
}
|
||||
|
||||
result = worker.attachment_worker(task_self, file_info, "user1")
|
||||
|
||||
assert result["token_count"] == 0
|
||||
assert result["mime_type"] == "application/pdf"
|
||||
extraction = result["metadata"]["extraction"]
|
||||
assert extraction["status"] == "no_text"
|
||||
assert "scanned PDF" in extraction["reason"]
|
||||
|
||||
row = AttachmentsRepository(pg_conn).get_by_legacy_id(
|
||||
file_info["attachment_id"], "user1"
|
||||
)
|
||||
assert row["upload_path"] == file_info["path"]
|
||||
assert row["content"] == ""
|
||||
assert row["metadata"]["extraction"]["status"] == "no_text"
|
||||
|
||||
events = [event for event, _ in published]
|
||||
assert "attachment.failed" not in events
|
||||
completed = [payload for event, payload in published if event == "attachment.completed"]
|
||||
assert completed[0]["extraction_status"] == "no_text"
|
||||
assert completed[0]["mime_type"] == "application/pdf"
|
||||
|
||||
def test_no_text_on_a_type_models_cannot_read_natively_still_fails(
|
||||
self, pg_conn, patch_worker_db, task_self, monkeypatch
|
||||
):
|
||||
"""Only PDFs and images have a native path; anything else without text stays a failure."""
|
||||
from docsgpt import worker
|
||||
from docsgpt.parser.file.base_parser import NoTextLayerError
|
||||
|
||||
published: list[tuple[str, dict]] = []
|
||||
monkeypatch.setattr(
|
||||
worker,
|
||||
"publish_user_event",
|
||||
lambda user, event, payload, **kw: published.append((event, payload)),
|
||||
)
|
||||
fake_storage = MagicMock(name="storage")
|
||||
fake_storage.process_file.side_effect = NoTextLayerError("notes.docx has no text")
|
||||
monkeypatch.setattr(worker.StorageCreator, "get_storage", lambda: fake_storage)
|
||||
monkeypatch.setattr(
|
||||
worker, "get_default_file_extractor", lambda ocr_enabled=False, pdf_text_fast_path=False: {}
|
||||
)
|
||||
|
||||
file_info = {
|
||||
"filename": "notes.docx",
|
||||
"attachment_id": "507f1f77bcf86cd799439022",
|
||||
"path": "uploads/user1/attachments/notes.docx",
|
||||
"metadata": {"source": "chat"},
|
||||
}
|
||||
|
||||
with pytest.raises(NoTextLayerError):
|
||||
worker.attachment_worker(task_self, file_info, "user1")
|
||||
|
||||
events = [event for event, _ in published]
|
||||
assert "attachment.failed" in events
|
||||
assert "attachment.completed" not in events
|
||||
|
||||
def test_tiff_is_stored_as_png_so_providers_accept_it(
|
||||
self, pg_conn, patch_worker_db, task_self, monkeypatch
|
||||
):
|
||||
"""OpenAI and Anthropic reject TIFF, so the row must point at a PNG copy."""
|
||||
import io
|
||||
|
||||
from PIL import Image
|
||||
|
||||
from docsgpt import worker
|
||||
|
||||
buf = io.BytesIO()
|
||||
Image.new("RGB", (32, 20), color=(10, 120, 200)).save(buf, format="TIFF")
|
||||
tiff_bytes = buf.getvalue()
|
||||
|
||||
published: list[tuple[str, dict]] = []
|
||||
saved: dict[str, bytes] = {}
|
||||
|
||||
def _save(data, path, **kwargs):
|
||||
saved[path] = data.read()
|
||||
return {"storage_type": "local"}
|
||||
|
||||
fake_storage = MagicMock(name="storage")
|
||||
fake_storage.process_file.return_value = Document(text="", extra_info={})
|
||||
fake_storage.get_file.side_effect = lambda path: io.BytesIO(tiff_bytes)
|
||||
fake_storage.save_file.side_effect = _save
|
||||
monkeypatch.setattr(worker.StorageCreator, "get_storage", lambda: fake_storage)
|
||||
monkeypatch.setattr(
|
||||
worker, "get_default_file_extractor", lambda ocr_enabled=False, pdf_text_fast_path=False: {}
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
worker,
|
||||
"publish_user_event",
|
||||
lambda user, event, payload, **kw: published.append((event, payload)),
|
||||
)
|
||||
|
||||
file_info = {
|
||||
"filename": "fax.tiff",
|
||||
"attachment_id": "507f1f77bcf86cd799439023",
|
||||
"path": "uploads/user1/attachments/abc/fax.tiff",
|
||||
"metadata": {"source": "chat"},
|
||||
}
|
||||
|
||||
result = worker.attachment_worker(task_self, file_info, "user1")
|
||||
|
||||
png_path = "uploads/user1/attachments/abc/fax.png"
|
||||
assert list(saved) == [png_path]
|
||||
assert Image.open(io.BytesIO(saved[png_path])).format == "PNG"
|
||||
assert result["path"] == png_path
|
||||
assert result["mime_type"] == "image/png"
|
||||
assert result["metadata"]["image_conversion"] == {
|
||||
"from": "image/tiff",
|
||||
"to": "image/png",
|
||||
"frames": 1,
|
||||
}
|
||||
|
||||
row = AttachmentsRepository(pg_conn).get_by_legacy_id(
|
||||
file_info["attachment_id"], "user1"
|
||||
)
|
||||
assert row["filename"] == "fax.tiff"
|
||||
assert row["upload_path"] == png_path
|
||||
assert row["mime_type"] == "image/png"
|
||||
|
||||
completed = [payload for event, payload in published if event == "attachment.completed"]
|
||||
assert completed[0]["mime_type"] == "image/png"
|
||||
|
||||
def test_text_without_a_parser_is_parsed(
|
||||
self, pg_conn, patch_worker_db, task_self, monkeypatch, tmp_path
|
||||
|
||||
Reference in new issue
Block a user