Extract attachment text off the event loop; reject text-less files
- extract_text (PyMuPDF for PDFs) now runs via asyncio.to_thread in both /upload and stream-query, so a large PDF doesn't stall other streams. - /upload returns 422 when extraction yields no text: a scanned PDF without a text layer, or an empty text file. - Mock server: *scan*.pdf uploads return that 422. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -228,7 +228,8 @@ async def _load_attached_file(file_key: str) -> tuple[str, str]:
|
||||
data, metadata = await s3_get_file(file_key)
|
||||
name = unquote(metadata.get("filename", "")) or file_key.rsplit("/", 1)[-1]
|
||||
ext = file_extension(file_key, None) or ""
|
||||
return name, extract_text(data, ext)
|
||||
# PyMuPDF is CPU-bound; keep it off the event loop so other streams don't stall.
|
||||
return name, await asyncio.to_thread(extract_text, data, ext)
|
||||
|
||||
|
||||
@conversation_blueprint.post("/upload")
|
||||
@@ -278,9 +279,16 @@ async def upload_attachment():
|
||||
)
|
||||
|
||||
try:
|
||||
text = extract_text(file_bytes, ext)
|
||||
text = await asyncio.to_thread(extract_text, file_bytes, ext)
|
||||
except FileExtractionError as e:
|
||||
return jsonify({"error": str(e)}), 400
|
||||
if not text.strip():
|
||||
message = (
|
||||
"This PDF has no text layer (scanned?), so Simba can't read it yet."
|
||||
if ext == "pdf"
|
||||
else "This file is empty, so there's nothing for Simba to read."
|
||||
)
|
||||
return jsonify({"error": message}), 422
|
||||
|
||||
key = f"conversations/{conversation.id}/files/{uuid.uuid4()}.{ext}"
|
||||
await s3_upload_file(file_bytes, key, FILE_TYPES[ext], {"filename": quote(name)})
|
||||
|
||||
@@ -203,6 +203,36 @@ class TestUploadRoute:
|
||||
resp = await _upload(app, headers, b"nope", "a.pdf", "application/pdf")
|
||||
assert resp.status_code == 400
|
||||
|
||||
async def test_pdf_without_text_layer_is_422(self, app, headers, s3):
|
||||
doc = fitz.open()
|
||||
doc.new_page() # blank page: no text, like a scan without OCR
|
||||
resp = await _upload(app, headers, doc.tobytes(), "scan.pdf", "application/pdf")
|
||||
assert resp.status_code == 422
|
||||
assert "no text layer" in (await resp.get_json())["error"]
|
||||
s3.upload_file.assert_not_awaited()
|
||||
|
||||
async def test_empty_text_file_is_422(self, app, headers, s3):
|
||||
resp = await _upload(app, headers, b" \n", "empty.txt", "text/plain")
|
||||
assert resp.status_code == 422
|
||||
s3.upload_file.assert_not_awaited()
|
||||
|
||||
async def test_extraction_runs_off_the_event_loop(self, app, headers, s3):
|
||||
calls = []
|
||||
|
||||
async def fake_to_thread(fn, *args):
|
||||
calls.append(fn)
|
||||
return fn(*args)
|
||||
|
||||
with patch.object(conv.asyncio, "to_thread", fake_to_thread):
|
||||
resp = await _upload(
|
||||
app, headers, _pdf_bytes("hi"), "a.pdf", "application/pdf"
|
||||
)
|
||||
get = AsyncMock(return_value=(b"x", {}))
|
||||
with patch.object(conv, "s3_get_file", get):
|
||||
await conv._load_attached_file(f"conversations/{CONV_ID}/files/u.txt")
|
||||
assert resp.status_code == 200
|
||||
assert calls == [conv.extract_text, conv.extract_text]
|
||||
|
||||
async def test_other_users_conversation_is_404(self, app, headers, s3):
|
||||
resp = await _upload(
|
||||
app, headers, b"hi", "a.txt", "text/plain", conversation_id=OTHER_ID
|
||||
|
||||
Reference in New Issue
Block a user