feat: add PDF OCR support with validation and page limits - #87
Merged
Conversation
- Implement `/extract_pdf` endpoint for PDF text extraction. - Use `pdf2image` to convert PDF pages to images for robust OCR. - Enforce a default 10-page limit with `force_processing` override. - Add strict PDF validation using magic bytes. - Update Dockerfile to include `poppler-utils`. - Update README with PDF endpoint documentation. Co-authored-by: gomesrocha <3893269+gomesrocha@users.noreply.github.com>
📝 WalkthroughWalkthroughThis pull request introduces PDF extraction capability, enhances OCR with language and mode parameters, implements file validation, adopts multi-stage Docker builds, and expands documentation. New endpoints for PDF processing and updated text extraction API with configurable OCR modes are added alongside supporting domain logic. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant TextAPI as Text Extract<br/>Endpoint
participant ImageValidator as Image<br/>Validator
participant OCREngine as OCR Engine<br/>(Tesseract)
participant FileSystem as File<br/>System
Client->>TextAPI: POST /extract_text with images,<br/>lang, mode, auto_detect
TextAPI->>ImageValidator: validate_image_file(img)
ImageValidator->>FileSystem: Read file header
ImageValidator-->>TextAPI: Validation result
alt Invalid Image
TextAPI-->>Client: HTTP 400 Error
else Valid Image
TextAPI->>OCREngine: read_image(path, lang,<br/>mode, auto)
alt mode == 'accurate'
OCREngine->>OCREngine: Preprocess: grayscale,<br/>upscale 2x, autocontrast
else mode == 'fast'
OCREngine->>OCREngine: Use image as-is
end
OCREngine->>OCREngine: Apply PSM config<br/>(1 if auto, else 3)
OCREngine-->>TextAPI: Extracted text
TextAPI-->>Client: HTTP 200 with text results
end
sequenceDiagram
participant Client
participant PDFExtractAPI as PDF Extract<br/>Endpoint
participant PDFValidator as PDF<br/>Validator
participant PDFProcessor as PDF<br/>Processor
participant FileSystem as File<br/>System
participant OCREngine as OCR Engine
Client->>PDFExtractAPI: POST /extract_pdf with file,<br/>lang, mode, force_processing
PDFExtractAPI->>PDFValidator: validate_pdf_file(file)
PDFValidator->>FileSystem: Read file header
PDFValidator-->>PDFExtractAPI: Validation result
alt Invalid PDF
PDFExtractAPI-->>Client: HTTP 400 Error
else Valid PDF
PDFExtractAPI->>PDFProcessor: process_pdf(path, lang,<br/>mode, auto, force_processing)
PDFProcessor->>FileSystem: Get page count
alt pages > 10 AND<br/>not force_processing
PDFProcessor-->>PDFExtractAPI: HTTP 400 Page limit exceeded
else Process allowed
PDFProcessor->>FileSystem: Convert PDF to images<br/>(DPI: 300 if accurate, else 200)
loop For each page
PDFProcessor->>FileSystem: Save page image
PDFProcessor->>OCREngine: read_image(page, lang, mode, auto)
OCREngine-->>PDFProcessor: Page text
PDFProcessor->>FileSystem: Cleanup temp image
end
PDFProcessor-->>PDFExtractAPI: Concatenated text<br/>with page headers
PDFExtractAPI-->>Client: HTTP 200 with results
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/pdf_extract.py`:
- Around line 47-55: process_pdf currently lets pdf2image.convert_from_path
materialize all pages at once when force_processing=True, which OOMs for large
PDFs; change process_pdf in app/domain/pdf_ocr.py to load and process pages in
bounded batches or one-at-a-time using pdf2image's first_page/last_page (or
equivalent single-page conversion) rather than converting the whole PDF at once,
and wire that behavior to the existing force_processing flag so large documents
are processed incrementally (iterate page ranges, call convert_from_path per
batch, and free images after OCR) to cap memory usage.
- Around line 29-36: Extract the duplicated mode/lang logic into a shared helper
(e.g. normalize_ocr_params) and replace the duplicated blocks in both
pdf_extract.py and text_extract.py with a call to that helper; the helper should
accept (lang: str, mode: str, auto_detect: bool) validate mode against ("fast",
"accurate") raising HTTPException on invalid input, set auto_detect=True and
lang="eng+por" when lang == "auto", and return the normalized (lang, mode,
auto_detect) so callers (where variables named lang, mode, auto_detect are
currently used) simply assign the returned values.
In `@app/api/text_extract.py`:
- Around line 38-46: Replace the ad-hoc print with structured logging by using a
module logger (logging.getLogger(__name__).info(...)) when logging the
processing of each image (in the loop that iterates over input_images and
currently calls print for img.filename); also stop calling the module-private
helper fileUpload._save_file_to_server from this module (and pdf_extract.py) —
rename the function to save_file_to_server (remove the leading underscore) and
update all callers (the loop that currently calls
fileUpload._save_file_to_server after validate_image_file) to call
fileUpload.save_file_to_server so the helper becomes part of the public API.
In `@app/domain/fileUpload.py`:
- Around line 35-38: Update the catch-all exception handlers in
app/domain/fileUpload.py to chain the original exception when converting to an
HTTPException: in the except block that currently does "raise
HTTPException(...)" (the one around line 35) and in the validate_pdf_file
handler (around line 75), change the re-raise to "raise
HTTPException(status_code=..., detail=...) from e" so the original traceback is
preserved; locate the handlers by the surrounding symbols/logic that check
"isinstance(e, HTTPException)" and the function validate_pdf_file to apply the
change.
- Around line 40-75: The validate_pdf_file function currently duplicates the
%PDF- fallback in both the kind is None and kind.mime != "application/pdf"
branches; refactor by checking header.startswith(b"%PDF-") once immediately
after reading the header (before calling or instead of duplicate checks around
filetype.guess) and return True if it matches, then proceed to use
filetype.guess and validate kind.mime == "application/pdf" otherwise; keep
existing HTTPException raises and the exception handling logic intact while
removing the duplicated fallback checks (refer to validate_pdf_file, header
variable, and the %PDF- fallback).
In `@app/domain/ocr.py`:
- Around line 26-43: The opened PIL Image from Image.open(img_path) is never
closed; refactor to use a context manager so the file handle is released after
OCR: open the image with a with Image.open(img_path) as image: block (including
the preprocessing steps when mode == 'accurate') and run the OCR inside that
block (ensure run_ocr and the call to asyncio.to_thread(lambda:
pytesseract.image_to_string(image, lang=lang, config=custom_config)) execute
while the context is active) so the image is automatically closed once OCR
completes.
- Around line 44-45: The read_image function currently catches all exceptions
and returns an error string, which masks failures for callers like
pdf_ocr.process_pdf; change read_image to re-raise the exception instead of
returning a string so callers can handle errors appropriately (e.g., remove the
except-return block and either let the exception propagate or raise a specific
exception with context including img_path), and update any callers that relied
on the error string (such as pdf_ocr.process_pdf) to handle exceptions or catch
and convert to desired output there.
In `@app/domain/pdf_ocr.py`:
- Around line 43-48: The current convert() wrapper calls convert_from_path which
loads all pages into memory (images) at once; change the logic in pdf_ocr.py to
iterate over pages (or process in small batches) by calling convert_from_path
with first_page and last_page parameters and handling each returned page before
requesting the next batch, e.g., replace the single convert()/images await with
a loop that uses asyncio.to_thread to convert one page or a batch at a time,
process/append the results (or stream them) and free references immediately to
avoid keeping the entire PDF in memory; update references to images and
convert() accordingly so downstream code consumes per-page outputs rather than
assuming a full-image list.
- Around line 69-73: The catch-all exception handler in app/domain/pdf_ocr.py
(the "except Exception as e:" block) currently raises a new HTTPException
without chaining the original error; change that raise to use exception chaining
by using "raise HTTPException(status_code=500, detail=f'Error processing PDF:
{str(e)}') from e" so the original traceback is preserved for debugging while
keeping the same status and detail message.
In `@Dockerfile`:
- Around line 1-38: Add a non-root runtime user and switch to it: after the
runtime packages are installed (the RUN apt-get ... line) create a non-root
user/group (e.g., appuser) with a fixed UID/GID, set its HOME, and ensure /app
and the copied virtualenv are owned by that user (use chown on /app and
/app/.venv after the COPY --from=builder /app/.venv /app/.venv and COPY . .
steps or change ownership right after copying), then add a USER directive (USER
appuser) before EXPOSE/CMD so the container runs as that non-root user;
reference the Dockerfile directives RUN apt-get, COPY --from=builder /app/.venv,
COPY . ., chown, and USER when making the changes.
In `@tests/test_api.py`:
- Around line 24-26: Remove the leftover debug conditional that prints
response.json() when response.status_code == 400; instead rely on the existing
assertion assert response.status_code == 200 (or, if desired, make the assertion
include diagnostics by changing it to assert response.status_code == 200,
response.json()). Update the code around the response variable (the conditional
checking response.status_code == 400 and the print(response.json()) call) to
eliminate the debug print so tests don't emit extraneous output.
In `@tests/test_file_validation.py`:
- Around line 28-39: Add a symmetric test for empty PDFs by creating a new test
function (e.g., test_validate_empty_pdf) in the same test module that constructs
an UploadFile with filename "test.pdf" and an empty BytesIO (b'' or BytesIO()),
calls validate_pdf_file, and asserts it raises an HTTPException with status_code
400; reference the existing tests test_validate_valid_pdf and
test_validate_invalid_pdf and the UploadFile/validate_pdf_file symbols to mirror
the empty-file check you already have for validate_image_file.
In `@tests/test_pdf_extract.py`:
- Around line 77-85: The test function test_extract_pdf_invalid_file is using
the unnecessary mock_pdf_tools fixture even though validation fails before
process_pdf is called; remove mock_pdf_tools from the test function signature
(leaving mock_ocr_read_image if still needed) and delete any unused
references/imports related to mock_pdf_tools so the test clearly focuses on
validation without mocking the PDF processing layer.
- Around line 1-85: Add a new test in tests/test_pdf_extract.py to cover the
invalid mode branch by posting to the /extract_pdf endpoint with a valid PDF
file payload but data={"mode": "invalid"} (e.g., name the test
test_extract_pdf_invalid_mode); assert the response.status_code is 400 and that
response.json()["detail"] contains the expected validation message (e.g.,
mentions "invalid" or "mode"), using the existing TestClient and the mock
fixtures (mock_ocr_read_image, mock_pdf_tools) so the request exercises the
mode-validation logic in the extract_pdf handler.
- Around line 29-43: In the tests test_extract_pdf_success and
test_extract_pdf_page_limit_exceeded, the fixture tuple is unpacked into
mock_convert and mock_info but those variables are unused; to fix the RUF059
warnings either prefix the unused names with an underscore (e.g., _mock_convert,
_mock_info) or avoid unpacking entirely and keep the fixture as a single unused
name so the fixtures still run (reference the mock tuple used in the tests:
mock_pdf_tools and the local names mock_convert/mock_info).
- Around line 59-75: The test test_extract_pdf_force_processing should also
assert that all 15 pages were processed: after posting to "/extract_pdf" with
force_processing, inspect response.json()[0]["text"] (or the relevant field
returned by the endpoint) and assert it contains page-specific markers from the
mocked conversion such as "--- Page 15 ---" and/or that splitting the text
yields 15 pages; update the assertions to check both presence of the last page
marker and that the extracted page count equals 15 using the existing mocks
mock_convert/mock_info to ensure force_processing bypassed the page limit.
🧹 Nitpick comments (12)
🤖 Fix all nitpicks with AI agents
Verify each finding against the current code and only fix it if needed. In `@app/api/pdf_extract.py`: - Around line 29-36: Extract the duplicated mode/lang logic into a shared helper (e.g. normalize_ocr_params) and replace the duplicated blocks in both pdf_extract.py and text_extract.py with a call to that helper; the helper should accept (lang: str, mode: str, auto_detect: bool) validate mode against ("fast", "accurate") raising HTTPException on invalid input, set auto_detect=True and lang="eng+por" when lang == "auto", and return the normalized (lang, mode, auto_detect) so callers (where variables named lang, mode, auto_detect are currently used) simply assign the returned values. In `@app/api/text_extract.py`: - Around line 38-46: Replace the ad-hoc print with structured logging by using a module logger (logging.getLogger(__name__).info(...)) when logging the processing of each image (in the loop that iterates over input_images and currently calls print for img.filename); also stop calling the module-private helper fileUpload._save_file_to_server from this module (and pdf_extract.py) — rename the function to save_file_to_server (remove the leading underscore) and update all callers (the loop that currently calls fileUpload._save_file_to_server after validate_image_file) to call fileUpload.save_file_to_server so the helper becomes part of the public API. In `@app/domain/fileUpload.py`: - Around line 35-38: Update the catch-all exception handlers in app/domain/fileUpload.py to chain the original exception when converting to an HTTPException: in the except block that currently does "raise HTTPException(...)" (the one around line 35) and in the validate_pdf_file handler (around line 75), change the re-raise to "raise HTTPException(status_code=..., detail=...) from e" so the original traceback is preserved; locate the handlers by the surrounding symbols/logic that check "isinstance(e, HTTPException)" and the function validate_pdf_file to apply the change. - Around line 40-75: The validate_pdf_file function currently duplicates the %PDF- fallback in both the kind is None and kind.mime != "application/pdf" branches; refactor by checking header.startswith(b"%PDF-") once immediately after reading the header (before calling or instead of duplicate checks around filetype.guess) and return True if it matches, then proceed to use filetype.guess and validate kind.mime == "application/pdf" otherwise; keep existing HTTPException raises and the exception handling logic intact while removing the duplicated fallback checks (refer to validate_pdf_file, header variable, and the %PDF- fallback). In `@app/domain/ocr.py`: - Around line 44-45: The read_image function currently catches all exceptions and returns an error string, which masks failures for callers like pdf_ocr.process_pdf; change read_image to re-raise the exception instead of returning a string so callers can handle errors appropriately (e.g., remove the except-return block and either let the exception propagate or raise a specific exception with context including img_path), and update any callers that relied on the error string (such as pdf_ocr.process_pdf) to handle exceptions or catch and convert to desired output there. In `@app/domain/pdf_ocr.py`: - Around line 69-73: The catch-all exception handler in app/domain/pdf_ocr.py (the "except Exception as e:" block) currently raises a new HTTPException without chaining the original error; change that raise to use exception chaining by using "raise HTTPException(status_code=500, detail=f'Error processing PDF: {str(e)}') from e" so the original traceback is preserved for debugging while keeping the same status and detail message. In `@tests/test_api.py`: - Around line 24-26: Remove the leftover debug conditional that prints response.json() when response.status_code == 400; instead rely on the existing assertion assert response.status_code == 200 (or, if desired, make the assertion include diagnostics by changing it to assert response.status_code == 200, response.json()). Update the code around the response variable (the conditional checking response.status_code == 400 and the print(response.json()) call) to eliminate the debug print so tests don't emit extraneous output. In `@tests/test_file_validation.py`: - Around line 28-39: Add a symmetric test for empty PDFs by creating a new test function (e.g., test_validate_empty_pdf) in the same test module that constructs an UploadFile with filename "test.pdf" and an empty BytesIO (b'' or BytesIO()), calls validate_pdf_file, and asserts it raises an HTTPException with status_code 400; reference the existing tests test_validate_valid_pdf and test_validate_invalid_pdf and the UploadFile/validate_pdf_file symbols to mirror the empty-file check you already have for validate_image_file. In `@tests/test_pdf_extract.py`: - Around line 77-85: The test function test_extract_pdf_invalid_file is using the unnecessary mock_pdf_tools fixture even though validation fails before process_pdf is called; remove mock_pdf_tools from the test function signature (leaving mock_ocr_read_image if still needed) and delete any unused references/imports related to mock_pdf_tools so the test clearly focuses on validation without mocking the PDF processing layer. - Around line 1-85: Add a new test in tests/test_pdf_extract.py to cover the invalid mode branch by posting to the /extract_pdf endpoint with a valid PDF file payload but data={"mode": "invalid"} (e.g., name the test test_extract_pdf_invalid_mode); assert the response.status_code is 400 and that response.json()["detail"] contains the expected validation message (e.g., mentions "invalid" or "mode"), using the existing TestClient and the mock fixtures (mock_ocr_read_image, mock_pdf_tools) so the request exercises the mode-validation logic in the extract_pdf handler. - Around line 29-43: In the tests test_extract_pdf_success and test_extract_pdf_page_limit_exceeded, the fixture tuple is unpacked into mock_convert and mock_info but those variables are unused; to fix the RUF059 warnings either prefix the unused names with an underscore (e.g., _mock_convert, _mock_info) or avoid unpacking entirely and keep the fixture as a single unused name so the fixtures still run (reference the mock tuple used in the tests: mock_pdf_tools and the local names mock_convert/mock_info). - Around line 59-75: The test test_extract_pdf_force_processing should also assert that all 15 pages were processed: after posting to "/extract_pdf" with force_processing, inspect response.json()[0]["text"] (or the relevant field returned by the endpoint) and assert it contains page-specific markers from the mocked conversion such as "--- Page 15 ---" and/or that splitting the text yields 15 pages; update the assertions to check both presence of the last page marker and that the extracted page count equals 15 using the existing mocks mock_convert/mock_info to ensure force_processing bypassed the page limit.app/domain/fileUpload.py (2)
35-38: Chain the original exception for better traceability.When re-raising as
HTTPExceptionin the catch-all handler, useraise ... from eso the original traceback is preserved. Same applies tovalidate_pdf_fileat Line 75.Proposed fix
- raise HTTPException(status_code=400, detail=f"Error validating file: {str(e)}") + raise HTTPException(status_code=400, detail=f"Error validating file: {str(e)}") from eAnd at line 75:
- raise HTTPException(status_code=400, detail=f"Error validating file: {str(e)}") + raise HTTPException(status_code=400, detail=f"Error validating file: {str(e)}") from e🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/domain/fileUpload.py` around lines 35 - 38, Update the catch-all exception handlers in app/domain/fileUpload.py to chain the original exception when converting to an HTTPException: in the except block that currently does "raise HTTPException(...)" (the one around line 35) and in the validate_pdf_file handler (around line 75), change the re-raise to "raise HTTPException(status_code=..., detail=...) from e" so the original traceback is preserved; locate the handlers by the surrounding symbols/logic that check "isinstance(e, HTTPException)" and the function validate_pdf_file to apply the change.
40-75: Duplicated%PDF-fallback logic across two branches.The
header.startswith(b"%PDF-")fallback is checked identically in both thekind is Noneandkind.mime != "application/pdf"branches (Lines 61–62 and 67–68). Consider checking the%PDF-header once up front to simplify the flow.Proposed simplification
def validate_pdf_file(file: UploadFile): try: header = file.file.read(2048) file.file.seek(0) if not header: raise HTTPException(status_code=400, detail="Empty file uploaded.") + # Quick check for PDF magic bytes + if header.startswith(b"%PDF-"): + return True + kind = filetype.guess(header) - if kind is None: - if header.startswith(b"%PDF-"): - return True - raise HTTPException(status_code=400, detail="Could not determine file type. Please upload a valid PDF.") - - if kind.mime != "application/pdf": - if header.startswith(b"%PDF-"): - return True - raise HTTPException(status_code=400, detail=f"Invalid file type: {kind.mime}. Only PDF files are allowed.") + if kind is None or kind.mime != "application/pdf": + detail = "Could not determine file type. Please upload a valid PDF." if kind is None else f"Invalid file type: {kind.mime}. Only PDF files are allowed." + raise HTTPException(status_code=400, detail=detail) return True except Exception as e:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/domain/fileUpload.py` around lines 40 - 75, The validate_pdf_file function currently duplicates the %PDF- fallback in both the kind is None and kind.mime != "application/pdf" branches; refactor by checking header.startswith(b"%PDF-") once immediately after reading the header (before calling or instead of duplicate checks around filetype.guess) and return True if it matches, then proceed to use filetype.guess and validate kind.mime == "application/pdf" otherwise; keep existing HTTPException raises and the exception handling logic intact while removing the duplicated fallback checks (refer to validate_pdf_file, header variable, and the %PDF- fallback).tests/test_file_validation.py (1)
28-39: Consider adding an empty PDF file test for symmetry.You test empty file handling for
validate_image_filebut not forvalidate_pdf_file. Adding atest_validate_empty_pdfwould ensure both validators handle this edge case consistently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_file_validation.py` around lines 28 - 39, Add a symmetric test for empty PDFs by creating a new test function (e.g., test_validate_empty_pdf) in the same test module that constructs an UploadFile with filename "test.pdf" and an empty BytesIO (b'' or BytesIO()), calls validate_pdf_file, and asserts it raises an HTTPException with status_code 400; reference the existing tests test_validate_valid_pdf and test_validate_invalid_pdf and the UploadFile/validate_pdf_file symbols to mirror the empty-file check you already have for validate_image_file.app/domain/pdf_ocr.py (1)
69-73: Chain the original exception for traceability.Use
raise ... from ein the catch-all handler (Line 73) to preserve the original traceback, consistent with the Ruff B904 hint.Proposed fix
except HTTPException as he: - raise he + raise except Exception as e: - raise HTTPException(status_code=500, detail=f"Error processing PDF: {str(e)}") + raise HTTPException(status_code=500, detail=f"Error processing PDF: {str(e)}") from e🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/domain/pdf_ocr.py` around lines 69 - 73, The catch-all exception handler in app/domain/pdf_ocr.py (the "except Exception as e:" block) currently raises a new HTTPException without chaining the original error; change that raise to use exception chaining by using "raise HTTPException(status_code=500, detail=f'Error processing PDF: {str(e)}') from e" so the original traceback is preserved for debugging while keeping the same status and detail message.app/domain/ocr.py (1)
44-45: Silently returning an error string masks failures from callers.
read_imagereturns"[ERROR] ..."on exception instead of raising. Callers (e.g.,pdf_ocr.process_pdf) concatenate the result as page text, so OCR failures will silently appear in the output. Consider raising so the caller can decide how to handle it. This is a pre-existing pattern, so flagging for awareness rather than as a blocker.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/domain/ocr.py` around lines 44 - 45, The read_image function currently catches all exceptions and returns an error string, which masks failures for callers like pdf_ocr.process_pdf; change read_image to re-raise the exception instead of returning a string so callers can handle errors appropriately (e.g., remove the except-return block and either let the exception propagate or raise a specific exception with context including img_path), and update any callers that relied on the error string (such as pdf_ocr.process_pdf) to handle exceptions or catch and convert to desired output there.tests/test_pdf_extract.py (4)
77-85:mock_pdf_toolsfixture is unnecessary for the invalid-file test.Validation rejects the file before
process_pdfis ever called, so the PDF tool mocks are never exercised. Removing the fixture from this test's parameters makes the test's intent clearer — it's purely testing the validation layer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_pdf_extract.py` around lines 77 - 85, The test function test_extract_pdf_invalid_file is using the unnecessary mock_pdf_tools fixture even though validation fails before process_pdf is called; remove mock_pdf_tools from the test function signature (leaving mock_ocr_read_image if still needed) and delete any unused references/imports related to mock_pdf_tools so the test clearly focuses on validation without mocking the PDF processing layer.
1-85: Missing test: invalidmodeparameter.There's no test asserting that a request with
mode="invalid"returns HTTP 400. This is a simple edge case that would cover the mode-validation branch inextract_pdf.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_pdf_extract.py` around lines 1 - 85, Add a new test in tests/test_pdf_extract.py to cover the invalid mode branch by posting to the /extract_pdf endpoint with a valid PDF file payload but data={"mode": "invalid"} (e.g., name the test test_extract_pdf_invalid_mode); assert the response.status_code is 400 and that response.json()["detail"] contains the expected validation message (e.g., mentions "invalid" or "mode"), using the existing TestClient and the mock fixtures (mock_ocr_read_image, mock_pdf_tools) so the request exercises the mode-validation logic in the extract_pdf handler.
29-43: Consider prefixing unused unpacked variables with_.In
test_extract_pdf_success,mock_convertandmock_infoare unpacked but neither is referenced in the test body. The same pattern appears intest_extract_pdf_page_limit_exceeded(line 46). Prefix with_to signal intent and silence the Ruff RUF059 warnings:- mock_convert, mock_info = mock_pdf_tools + _mock_convert, _mock_info = mock_pdf_toolsAlternatively, don't unpack at all when you only need the fixture to be active.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_pdf_extract.py` around lines 29 - 43, In the tests test_extract_pdf_success and test_extract_pdf_page_limit_exceeded, the fixture tuple is unpacked into mock_convert and mock_info but those variables are unused; to fix the RUF059 warnings either prefix the unused names with an underscore (e.g., _mock_convert, _mock_info) or avoid unpacking entirely and keep the fixture as a single unused name so the fixtures still run (reference the mock tuple used in the tests: mock_pdf_tools and the local names mock_convert/mock_info).
59-75: Force-processing test could assert on page content.The test verifies
status_code == 200andlen(data) == 1, but doesn't check that all 15 pages were actually processed. Adding an assertion on the text content (e.g."--- Page 15 ---" in data[0]["text"]) would strengthen confidence thatforce_processingtruly bypassed the limit and processed every page.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_pdf_extract.py` around lines 59 - 75, The test test_extract_pdf_force_processing should also assert that all 15 pages were processed: after posting to "/extract_pdf" with force_processing, inspect response.json()[0]["text"] (or the relevant field returned by the endpoint) and assert it contains page-specific markers from the mocked conversion such as "--- Page 15 ---" and/or that splitting the text yields 15 pages; update the assertions to check both presence of the last page marker and that the extracted page count equals 15 using the existing mocks mock_convert/mock_info to ensure force_processing bypassed the page limit.app/api/pdf_extract.py (1)
29-36: Duplicated input-handling logic — extract a shared helper.The mode validation (lines 29-31) and
lang == "auto"handling (lines 33-36) are copy-pasted fromtext_extract.py. If the set of valid modes or the auto-lang behavior changes, both files must be updated in lockstep.Consider extracting a small shared function, e.g. in a utils module:
def normalize_ocr_params(lang: str, mode: str, auto_detect: bool): if mode not in ("fast", "accurate"): raise HTTPException(status_code=400, detail="Invalid mode. Use 'fast' or 'accurate'.") if lang == "auto": auto_detect = True lang = "eng+por" return lang, mode, auto_detect🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/pdf_extract.py` around lines 29 - 36, Extract the duplicated mode/lang logic into a shared helper (e.g. normalize_ocr_params) and replace the duplicated blocks in both pdf_extract.py and text_extract.py with a call to that helper; the helper should accept (lang: str, mode: str, auto_detect: bool) validate mode against ("fast", "accurate") raising HTTPException on invalid input, set auto_detect=True and lang="eng+por" when lang == "auto", and return the normalized (lang, mode, auto_detect) so callers (where variables named lang, mode, auto_detect are currently used) simply assign the returned values.app/api/text_extract.py (1)
38-46: Replace_save_file_to_server.Two concerns in this block:
- Line 40:
print()should be replaced withlogging.getLogger(__name__).info(...)for proper log-level control in production.- Line 46:
_save_file_to_serveris prefixed with_, signaling it's a module-private function. Both this file andpdf_extract.pycall it directly. Consider renaming it tosave_file_to_server(dropping the underscore) to make it part of the public API.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/text_extract.py` around lines 38 - 46, Replace the ad-hoc print with structured logging by using a module logger (logging.getLogger(__name__).info(...)) when logging the processing of each image (in the loop that iterates over input_images and currently calls print for img.filename); also stop calling the module-private helper fileUpload._save_file_to_server from this module (and pdf_extract.py) — rename the function to save_file_to_server (remove the leading underscore) and update all callers (the loop that currently calls fileUpload._save_file_to_server after validate_image_file) to call fileUpload.save_file_to_server so the helper becomes part of the public API.tests/test_api.py (1)
24-26: Remove debugging artifact.This conditional print is a leftover debug aid — the
assert response.status_code == 200on line 28 already covers the failure case. If you want better diagnostics on failure, pytest'sassertwill show the full response by default, or you can use a message:assert response.status_code == 200, response.json().Proposed fix
- # If status code is 400, it means validation failed. - if response.status_code == 400: - print(response.json()) - - assert response.status_code == 200 + assert response.status_code == 200, f"Unexpected 400: {response.json()}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_api.py` around lines 24 - 26, Remove the leftover debug conditional that prints response.json() when response.status_code == 400; instead rely on the existing assertion assert response.status_code == 200 (or, if desired, make the assertion include diagnostics by changing it to assert response.status_code == 200, response.json()). Update the code around the response variable (the conditional checking response.status_code == 400 and the print(response.json()) call) to eliminate the debug print so tests don't emit extraneous output.
Comment on lines
+47
to
+55
| try: | ||
| # Process PDF | ||
| text = await pdf_ocr.process_pdf( | ||
| file_path=temp_file, | ||
| lang=lang, | ||
| mode=mode, | ||
| auto=auto_detect, | ||
| force_processing=force_processing | ||
| ) |
There was a problem hiding this comment.
Large PDFs with force_processing=True will load all page images into memory at once.
Looking at app/domain/pdf_ocr.py (lines 48-51 in the relevant snippet), convert_from_path materializes every page as a PIL Image simultaneously. For a 100-page PDF at 300 DPI, this can easily consume multiple GB of RAM and risk an OOM kill.
Consider converting pages in batches (pdf2image supports first_page/last_page parameters) or streaming one page at a time to bound memory usage.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/pdf_extract.py` around lines 47 - 55, process_pdf currently lets
pdf2image.convert_from_path materialize all pages at once when
force_processing=True, which OOMs for large PDFs; change process_pdf in
app/domain/pdf_ocr.py to load and process pages in bounded batches or
one-at-a-time using pdf2image's first_page/last_page (or equivalent single-page
conversion) rather than converting the whole PDF at once, and wire that behavior
to the existing force_processing flag so large documents are processed
incrementally (iterate page ranges, call convert_from_path per batch, and free
images after OCR) to cap memory usage.
Comment on lines
+26
to
43
| image = Image.open(img_path) | ||
|
|
||
| if mode == 'accurate': | ||
| # Preprocessing for accuracy: | ||
| # 1. Convert to grayscale | ||
| image = image.convert('L') | ||
| # 2. Resize (Scale 2x) for better character recognition | ||
| width, height = image.size | ||
| image = image.resize((width * 2, height * 2), Image.Resampling.LANCZOS) | ||
| # 3. Enhance contrast (optional, but histogram equalization often helps) | ||
| image = ImageOps.autocontrast(image) | ||
|
|
||
| # Run OCR in a separate thread | ||
| def run_ocr(): | ||
| return pytesseract.image_to_string(image, lang=lang, config=custom_config) | ||
|
|
||
| text = await asyncio.to_thread(run_ocr) | ||
| return text |
There was a problem hiding this comment.
Opened Image is never closed — potential resource leak.
Image.open(img_path) acquires a file handle that is never explicitly released. For long-running server processes, this can accumulate file descriptors. Use a context manager or explicitly close the image after OCR completes.
Proposed fix
- # Load image
- image = Image.open(img_path)
-
- if mode == 'accurate':
- image = image.convert('L')
- width, height = image.size
- image = image.resize((width * 2, height * 2), Image.Resampling.LANCZOS)
- image = ImageOps.autocontrast(image)
-
- def run_ocr():
- return pytesseract.image_to_string(image, lang=lang, config=custom_config)
-
- text = await asyncio.to_thread(run_ocr)
- return text
+ # Load image
+ image = Image.open(img_path)
+ try:
+ if mode == 'accurate':
+ image = image.convert('L')
+ width, height = image.size
+ image = image.resize((width * 2, height * 2), Image.Resampling.LANCZOS)
+ image = ImageOps.autocontrast(image)
+
+ def run_ocr():
+ return pytesseract.image_to_string(image, lang=lang, config=custom_config)
+
+ text = await asyncio.to_thread(run_ocr)
+ return text
+ finally:
+ image.close()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| image = Image.open(img_path) | |
| if mode == 'accurate': | |
| # Preprocessing for accuracy: | |
| # 1. Convert to grayscale | |
| image = image.convert('L') | |
| # 2. Resize (Scale 2x) for better character recognition | |
| width, height = image.size | |
| image = image.resize((width * 2, height * 2), Image.Resampling.LANCZOS) | |
| # 3. Enhance contrast (optional, but histogram equalization often helps) | |
| image = ImageOps.autocontrast(image) | |
| # Run OCR in a separate thread | |
| def run_ocr(): | |
| return pytesseract.image_to_string(image, lang=lang, config=custom_config) | |
| text = await asyncio.to_thread(run_ocr) | |
| return text | |
| image = Image.open(img_path) | |
| try: | |
| if mode == 'accurate': | |
| # Preprocessing for accuracy: | |
| # 1. Convert to grayscale | |
| image = image.convert('L') | |
| # 2. Resize (Scale 2x) for better character recognition | |
| width, height = image.size | |
| image = image.resize((width * 2, height * 2), Image.Resampling.LANCZOS) | |
| # 3. Enhance contrast (optional, but histogram equalization often helps) | |
| image = ImageOps.autocontrast(image) | |
| # Run OCR in a separate thread | |
| def run_ocr(): | |
| return pytesseract.image_to_string(image, lang=lang, config=custom_config) | |
| text = await asyncio.to_thread(run_ocr) | |
| return text | |
| finally: | |
| image.close() |
🧰 Tools
🪛 Ruff (0.15.0)
[warning] 43-43: Consider moving this statement to an else block
(TRY300)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/domain/ocr.py` around lines 26 - 43, The opened PIL Image from
Image.open(img_path) is never closed; refactor to use a context manager so the
file handle is released after OCR: open the image with a with
Image.open(img_path) as image: block (including the preprocessing steps when
mode == 'accurate') and run the OCR inside that block (ensure run_ocr and the
call to asyncio.to_thread(lambda: pytesseract.image_to_string(image, lang=lang,
config=custom_config)) execute while the context is active) so the image is
automatically closed once OCR completes.
Comment on lines
+43
to
+48
| # Convert PDF to images | ||
| # We run this in a thread because it's CPU bound | ||
| def convert(): | ||
| return convert_from_path(file_path, dpi=dpi) | ||
|
|
||
| images = await asyncio.to_thread(convert) |
There was a problem hiding this comment.
All PDF pages are loaded into memory at once — high memory risk.
convert_from_path materializes every page as a PIL Image simultaneously. For large PDFs (enabled via force_processing=True), this can consume several GB of RAM and potentially OOM the process. Consider processing pages in batches or one at a time using the first_page/last_page parameters of convert_from_path.
Sketch: page-by-page conversion
- def convert():
- return convert_from_path(file_path, dpi=dpi)
-
- images = await asyncio.to_thread(convert)
-
extracted_text = []
- for i, image in enumerate(images):
+ for i in range(page_count):
+ page_num = i + 1
+ def convert_page(p=page_num):
+ return convert_from_path(file_path, dpi=dpi, first_page=p, last_page=p)
+
+ pages = await asyncio.to_thread(convert_page)
+ image = pages[0]
+
with tempfile.NamedTemporaryFile(suffix=".png", delete=False) as temp_img:
image.save(temp_img.name)
temp_img_path = temp_img.name
try:
page_text = await ocr.read_image(temp_img_path, lang=lang, mode=mode, auto=auto)
- extracted_text.append(f"--- Page {i+1} ---\n{page_text}")
+ extracted_text.append(f"--- Page {page_num} ---\n{page_text}")
finally:
+ image.close()
if os.path.exists(temp_img_path):
os.remove(temp_img_path)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/domain/pdf_ocr.py` around lines 43 - 48, The current convert() wrapper
calls convert_from_path which loads all pages into memory (images) at once;
change the logic in pdf_ocr.py to iterate over pages (or process in small
batches) by calling convert_from_path with first_page and last_page parameters
and handling each returned page before requesting the next batch, e.g., replace
the single convert()/images await with a loop that uses asyncio.to_thread to
convert one page or a batch at a time, process/append the results (or stream
them) and free references immediately to avoid keeping the entire PDF in memory;
update references to images and convert() accordingly so downstream code
consumes per-page outputs rather than assuming a full-image list.
Comment on lines
+1
to
38
| # Build stage | ||
| FROM ghcr.io/astral-sh/uv:python3.12-bookworm-slim AS builder | ||
|
|
||
| WORKDIR /app | ||
|
|
||
| # Install system dependencies | ||
| # Copy dependency files | ||
| COPY pyproject.toml uv.lock ./ | ||
|
|
||
| # Install dependencies into .venv | ||
| RUN uv sync --frozen --no-dev | ||
|
|
||
| # Runtime stage | ||
| FROM python:3.12-slim-bookworm | ||
|
|
||
| WORKDIR /app | ||
|
|
||
| # Install runtime dependencies (tesseract, languages, and poppler) | ||
| RUN apt-get update && apt-get install -y --no-install-recommends \ | ||
| tesseract-ocr \ | ||
| tesseract-ocr-por \ | ||
| tesseract-ocr-eng \ | ||
| poppler-utils \ | ||
| && rm -rf /var/lib/apt/lists/* | ||
|
|
||
| # Copy pyproject.toml and uv.lock | ||
| COPY pyproject.toml uv.lock ./ | ||
|
|
||
| # Install dependencies | ||
| RUN uv sync --frozen --no-dev | ||
| # Copy virtual environment from builder | ||
| COPY --from=builder /app/.venv /app/.venv | ||
|
|
||
| # Copy the application code | ||
| # Copy application code | ||
| COPY . . | ||
|
|
||
| # Expose port | ||
| EXPOSE 8080 | ||
|
|
||
| # Run the application | ||
| # We need to use the virtual environment created by uv, usually at .venv | ||
| # Set environment variables | ||
| ENV PATH="/app/.venv/bin:$PATH" | ||
|
|
||
| # Run the application | ||
| CMD ["uvicorn", "app.main:app", "--host", "0.0.0.0", "--port", "8080"] |
There was a problem hiding this comment.
Container runs as root — add a non-root user.
The entire container runs as root because no USER directive is specified. This widens the blast radius of any container escape or application-level exploit. Add a non-root user after installing system packages.
🛡️ Proposed fix
# Copy virtual environment from builder
COPY --from=builder /app/.venv /app/.venv
+# Create non-root user
+RUN useradd --create-home appuser
+
# Copy application code
COPY . .
+# Switch to non-root user
+RUN chown -R appuser:appuser /app
+USER appuser
+
# Expose port
EXPOSE 8080📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Build stage | |
| FROM ghcr.io/astral-sh/uv:python3.12-bookworm-slim AS builder | |
| WORKDIR /app | |
| # Install system dependencies | |
| # Copy dependency files | |
| COPY pyproject.toml uv.lock ./ | |
| # Install dependencies into .venv | |
| RUN uv sync --frozen --no-dev | |
| # Runtime stage | |
| FROM python:3.12-slim-bookworm | |
| WORKDIR /app | |
| # Install runtime dependencies (tesseract, languages, and poppler) | |
| RUN apt-get update && apt-get install -y --no-install-recommends \ | |
| tesseract-ocr \ | |
| tesseract-ocr-por \ | |
| tesseract-ocr-eng \ | |
| poppler-utils \ | |
| && rm -rf /var/lib/apt/lists/* | |
| # Copy pyproject.toml and uv.lock | |
| COPY pyproject.toml uv.lock ./ | |
| # Install dependencies | |
| RUN uv sync --frozen --no-dev | |
| # Copy virtual environment from builder | |
| COPY --from=builder /app/.venv /app/.venv | |
| # Copy the application code | |
| # Copy application code | |
| COPY . . | |
| # Expose port | |
| EXPOSE 8080 | |
| # Run the application | |
| # We need to use the virtual environment created by uv, usually at .venv | |
| # Set environment variables | |
| ENV PATH="/app/.venv/bin:$PATH" | |
| # Run the application | |
| CMD ["uvicorn", "app.main:app", "--host", "0.0.0.0", "--port", "8080"] | |
| # Build stage | |
| FROM ghcr.io/astral-sh/uv:python3.12-bookworm-slim AS builder | |
| WORKDIR /app | |
| # Copy dependency files | |
| COPY pyproject.toml uv.lock ./ | |
| # Install dependencies into .venv | |
| RUN uv sync --frozen --no-dev | |
| # Runtime stage | |
| FROM python:3.12-slim-bookworm | |
| WORKDIR /app | |
| # Install runtime dependencies (tesseract, languages, and poppler) | |
| RUN apt-get update && apt-get install -y --no-install-recommends \ | |
| tesseract-ocr \ | |
| tesseract-ocr-por \ | |
| tesseract-ocr-eng \ | |
| poppler-utils \ | |
| && rm -rf /var/lib/apt/lists/* | |
| # Copy virtual environment from builder | |
| COPY --from=builder /app/.venv /app/.venv | |
| # Create non-root user | |
| RUN useradd --create-home appuser | |
| # Copy application code | |
| COPY . . | |
| # Switch to non-root user | |
| RUN chown -R appuser:appuser /app | |
| USER appuser | |
| # Expose port | |
| EXPOSE 8080 | |
| # Set environment variables | |
| ENV PATH="/app/.venv/bin:$PATH" | |
| # Run the application | |
| CMD ["uvicorn", "app.main:app", "--host", "0.0.0.0", "--port", "8080"] |
🧰 Tools
🪛 Trivy (0.69.1)
[error] 1-1: Image user should not be 'root'
Specify at least 1 USER command in Dockerfile with non-root user as argument
Rule: DS-0002
(IaC/Dockerfile)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Dockerfile` around lines 1 - 38, Add a non-root runtime user and switch to
it: after the runtime packages are installed (the RUN apt-get ... line) create a
non-root user/group (e.g., appuser) with a fixed UID/GID, set its HOME, and
ensure /app and the copied virtualenv are owned by that user (use chown on /app
and /app/.venv after the COPY --from=builder /app/.venv /app/.venv and COPY . .
steps or change ownership right after copying), then add a USER directive (USER
appuser) before EXPOSE/CMD so the container runs as that non-root user;
reference the Dockerfile directives RUN apt-get, COPY --from=builder /app/.venv,
COPY . ., chown, and USER when making the changes.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
/extract_pdfendpoint for PDF text extraction.pdf2imageto convert PDF pages to images for robust OCR.force_processingoverride.poppler-utils.Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests
Chores