mirror of
https://github.com/tiennm99/DocsGPT.git
synced 2026-10-19 00:14:40 +00:00
fix: error type fixes and docling parsing improvements
This commit is contained in:
1 parent
fcef3e68e5
commit
0ecb421954
15 files changed
+938
-59
No files matched your search
@@ -4,6 +4,7 @@ Covers: SimpleDirectoryReader (init, file discovery, load_data, directory
|
||||
structure building), get_default_file_extractor.
|
||||
"""
|
||||
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
@@ -158,6 +159,119 @@ class TestSimpleDirectoryReaderLoadData:
|
||||
for doc in docs:
|
||||
assert isinstance(doc, Document)
|
||||
|
||||
def test_unparseable_file_is_skipped_not_fatal(self, tmp_path):
|
||||
"""One bad file must not cost the user the other 99.
|
||||
|
||||
Parsers raise ``DocumentParseError`` rather than returning their own
|
||||
traceback as the document text, so an unguarded loop turns a single
|
||||
corrupt PDF into a failed ingest for the whole zip/folder/sync.
|
||||
"""
|
||||
from application.parser.file.base_parser import DocumentParseError
|
||||
from application.parser.file.bulk import SimpleDirectoryReader
|
||||
|
||||
(tmp_path / "good1.md").write_text("first")
|
||||
(tmp_path / "bad.md").write_text("corrupt")
|
||||
(tmp_path / "good2.md").write_text("second")
|
||||
|
||||
def _parse(path, **kwargs):
|
||||
if path.name == "bad.md":
|
||||
raise DocumentParseError("Failed to parse bad.md with docling: boom")
|
||||
return f"parsed {path.name}"
|
||||
|
||||
mock_parser = MagicMock()
|
||||
mock_parser.parser_config_set = True
|
||||
mock_parser.parse_file.side_effect = _parse
|
||||
mock_parser.get_file_metadata.return_value = {}
|
||||
|
||||
reader = SimpleDirectoryReader(
|
||||
input_dir=str(tmp_path),
|
||||
file_extractor={".md": mock_parser},
|
||||
recursive=False,
|
||||
exclude_hidden=True,
|
||||
)
|
||||
docs = reader.load_data()
|
||||
|
||||
texts = sorted(doc.text for doc in docs)
|
||||
assert texts == ["parsed good1.md", "parsed good2.md"]
|
||||
# The skip is recorded, not silent: callers can surface it.
|
||||
assert [Path(p).name for p, _ in reader.failed_files] == ["bad.md"]
|
||||
assert "boom" in reader.failed_files[0][1]
|
||||
|
||||
def test_single_unparseable_file_still_raises(self, tmp_path):
|
||||
"""The attachment path parses exactly one file and must fail loudly.
|
||||
|
||||
Skipping there would hand ``load_data()[0]`` an empty list and turn a
|
||||
clear "this PDF could not be read" into an IndexError.
|
||||
"""
|
||||
from application.parser.file.base_parser import DocumentParseError
|
||||
from application.parser.file.bulk import SimpleDirectoryReader
|
||||
|
||||
(tmp_path / "bad.pdf").write_text("not really a pdf")
|
||||
|
||||
mock_parser = MagicMock()
|
||||
mock_parser.parser_config_set = True
|
||||
mock_parser.parse_file.side_effect = DocumentParseError(
|
||||
"Failed to parse bad.pdf with docling: InvalidCxxCompiler"
|
||||
)
|
||||
|
||||
reader = SimpleDirectoryReader(
|
||||
input_files=[str(tmp_path / "bad.pdf")],
|
||||
file_extractor={".pdf": mock_parser},
|
||||
)
|
||||
# The parser's own message reaches the user verbatim, unwrapped.
|
||||
with pytest.raises(DocumentParseError, match="InvalidCxxCompiler"):
|
||||
reader.load_data()
|
||||
|
||||
def test_all_files_unparseable_raises(self, tmp_path):
|
||||
"""Nothing parsed is a failed ingest, not an empty success."""
|
||||
from application.parser.file.base_parser import DocumentParseError
|
||||
from application.parser.file.bulk import SimpleDirectoryReader
|
||||
|
||||
(tmp_path / "a.md").write_text("x")
|
||||
(tmp_path / "b.md").write_text("y")
|
||||
|
||||
mock_parser = MagicMock()
|
||||
mock_parser.parser_config_set = True
|
||||
mock_parser.parse_file.side_effect = DocumentParseError("nope")
|
||||
|
||||
reader = SimpleDirectoryReader(
|
||||
input_dir=str(tmp_path),
|
||||
file_extractor={".md": mock_parser},
|
||||
recursive=False,
|
||||
exclude_hidden=True,
|
||||
)
|
||||
with pytest.raises(DocumentParseError) as excinfo:
|
||||
reader.load_data()
|
||||
assert "None of the 2 files" in str(excinfo.value)
|
||||
|
||||
def test_skipped_file_still_advances_progress(self, tmp_path):
|
||||
"""Progress must not stall on a skipped file."""
|
||||
from application.parser.file.base_parser import DocumentParseError
|
||||
from application.parser.file.bulk import SimpleDirectoryReader
|
||||
|
||||
(tmp_path / "good.md").write_text("ok")
|
||||
(tmp_path / "bad.md").write_text("bad")
|
||||
|
||||
def _parse(path, **kwargs):
|
||||
if path.name == "bad.md":
|
||||
raise DocumentParseError("boom")
|
||||
return "ok"
|
||||
|
||||
mock_parser = MagicMock()
|
||||
mock_parser.parser_config_set = True
|
||||
mock_parser.parse_file.side_effect = _parse
|
||||
mock_parser.get_file_metadata.return_value = {}
|
||||
|
||||
reader = SimpleDirectoryReader(
|
||||
input_dir=str(tmp_path),
|
||||
file_extractor={".md": mock_parser},
|
||||
recursive=False,
|
||||
exclude_hidden=True,
|
||||
)
|
||||
calls = []
|
||||
reader.load_data(progress_callback=lambda done, total: calls.append((done, total)))
|
||||
assert [c[0] for c in calls] == [1, 2]
|
||||
|
||||
def test_load_data_progress_callback_fires_per_file(self, temp_dir):
|
||||
from application.parser.file.bulk import SimpleDirectoryReader
|
||||
|
||||
|
||||
@@ -270,7 +270,16 @@ class TestDoclingParserParseFile:
|
||||
result = parser.parse_file(Path("test.pdf"))
|
||||
assert "content" in result
|
||||
|
||||
def test_parse_file_error_ignore(self):
|
||||
def test_parse_file_error_ignore_raises_instead_of_returning_error_text(self):
|
||||
"""A failed conversion must never become the document's text.
|
||||
|
||||
Regression: ``errors="ignore"`` used to return
|
||||
``"[Error parsing file with docling: ...]"``, which the attachment
|
||||
worker then stored as ``attachments.content`` and handed to the LLM
|
||||
as if it were the PDF. ``errors`` controls *decoding* leniency, not
|
||||
"substitute the traceback for the document".
|
||||
"""
|
||||
from application.parser.file.base_parser import DocumentParseError
|
||||
from application.parser.file.docling_parser import DoclingParser
|
||||
|
||||
parser = DoclingParser()
|
||||
@@ -278,8 +287,37 @@ class TestDoclingParserParseFile:
|
||||
mock_converter.convert.side_effect = Exception("Parse failed")
|
||||
parser._converter = mock_converter
|
||||
|
||||
result = parser.parse_file(Path("bad.pdf"), errors="ignore")
|
||||
assert "Error" in result
|
||||
with pytest.raises(DocumentParseError) as excinfo:
|
||||
parser.parse_file(Path("bad.pdf"), errors="ignore")
|
||||
|
||||
# The message identifies the file and preserves the cause for triage…
|
||||
assert "bad.pdf" in str(excinfo.value)
|
||||
assert "Parse failed" in str(excinfo.value)
|
||||
# …and the original exception is chained, not swallowed.
|
||||
assert isinstance(excinfo.value.__cause__, Exception)
|
||||
assert "Parse failed" in str(excinfo.value.__cause__)
|
||||
|
||||
def test_parse_file_error_ignore_never_returns_a_string(self):
|
||||
"""Belt-and-braces: no code path may hand back error text as content."""
|
||||
from application.parser.file.base_parser import DocumentParseError
|
||||
from application.parser.file.docling_parser import DoclingParser
|
||||
|
||||
parser = DoclingParser()
|
||||
mock_converter = MagicMock()
|
||||
mock_converter.convert.side_effect = RuntimeError(
|
||||
"Conversion failed for: x.pdf with status: failure. Errors: "
|
||||
"InvalidCxxCompiler: No working C++ compiler found"
|
||||
)
|
||||
parser._converter = mock_converter
|
||||
|
||||
try:
|
||||
result = parser.parse_file(Path("x.pdf"), errors="ignore")
|
||||
except DocumentParseError:
|
||||
return # expected
|
||||
pytest.fail(
|
||||
f"parse_file returned {result!r} instead of raising; error text "
|
||||
"must not be usable as document content"
|
||||
)
|
||||
|
||||
def test_parse_file_error_raise(self):
|
||||
from application.parser.file.docling_parser import DoclingParser
|
||||
|
||||
Reference in new issue
Block a user