fix(ocr): address code review feedback on multi-language OCR

- Fix _get_pipeline_ocr_language: remove redundant `or None` in step_config.get()
- Add Session type hint to _get_pipeline_ocr_language db parameter via TYPE_CHECKING
- Update process_with_ocr to use modern str | None syntax instead of Optional[str]
- Fix test_get_pipeline_ocr_language_explicit_pipeline_takes_priority: properly add
  sys_step to db_session so the system pipeline step is persisted in the test DB

Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
This commit is contained in:
copilot-swe-agent[bot]
2026-03-08 21:49:49 +00:00
parent a2a4c6fc9a
commit 0b291995b9
3 changed files with 14 additions and 7 deletions
+8 -2
View File
@@ -1,11 +1,14 @@
#!/usr/bin/env python3 #!/usr/bin/env python3
from __future__ import annotations
import json import json
import logging import logging
import mimetypes import mimetypes
import os import os
import shutil import shutil
import uuid import uuid
from typing import TYPE_CHECKING
import pypdf # Upgraded from PyPDF2 to fix CVE-2023-36464 import pypdf # Upgraded from PyPDF2 to fix CVE-2023-36464
from pypdf.errors import PdfReadError from pypdf.errors import PdfReadError
@@ -21,10 +24,13 @@ from app.utils import get_unique_filepath_with_counter, hash_file, log_task_prog
from app.utils.step_manager import initialize_file_steps from app.utils.step_manager import initialize_file_steps
from app.utils.text_quality import check_text_quality, detect_pdf_text_source from app.utils.text_quality import check_text_quality, detect_pdf_text_source
if TYPE_CHECKING:
from sqlalchemy.orm import Session
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
def _get_pipeline_ocr_language(db, file_record: FileRecord, owner_id: str | None) -> str | None: def _get_pipeline_ocr_language(db: "Session", file_record: FileRecord, owner_id: str | None) -> str | None:
"""Look up the OCR language override from the file's pipeline OCR step config. """Look up the OCR language override from the file's pipeline OCR step config.
Resolution order: Resolution order:
@@ -80,7 +86,7 @@ def _get_pipeline_ocr_language(db, file_record: FileRecord, owner_id: str | None
try: try:
step_config = json.loads(ocr_step.config) step_config = json.loads(ocr_step.config)
lang = step_config.get("ocr_language") or None lang = step_config.get("ocr_language")
# "auto" is treated as no override # "auto" is treated as no override
return lang if lang and lang != "auto" else None return lang if lang and lang != "auto" else None
except Exception: except Exception:
+3 -4
View File
@@ -17,7 +17,6 @@ task with a multi-engine OCR pipeline that:
import logging import logging
import os import os
from typing import Optional
from app.celery_app import celery from app.celery_app import celery
from app.config import settings from app.config import settings
@@ -36,9 +35,9 @@ logger = logging.getLogger(__name__)
def process_with_ocr( def process_with_ocr(
self, self,
filename: str, filename: str,
file_id: Optional[int] = None, file_id: int | None = None,
original_text: Optional[str] = None, original_text: str | None = None,
language: Optional[str] = None, language: str | None = None,
): ):
"""Run the configured OCR providers on *filename* and continue the pipeline. """Run the configured OCR providers on *filename* and continue the pipeline.
+3 -1
View File
@@ -977,13 +977,15 @@ def test_get_pipeline_ocr_language_explicit_pipeline_takes_priority(db_session):
db_session.add(sys_pipeline) db_session.add(sys_pipeline)
db_session.commit() db_session.commit()
PipelineStep( sys_step = PipelineStep(
pipeline_id=sys_pipeline.id, pipeline_id=sys_pipeline.id,
position=0, position=0,
step_type="ocr", step_type="ocr",
config=json.dumps({"ocr_language": "eng"}), config=json.dumps({"ocr_language": "eng"}),
enabled=True, enabled=True,
) )
db_session.add(sys_step)
db_session.commit()
# Explicit pipeline with "fra" # Explicit pipeline with "fra"
explicit_pipeline = Pipeline( explicit_pipeline = Pipeline(