From b5c669f3fea60f434c73864e82d08adac4edb44b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Feb 2026 20:04:57 +0000 Subject: [PATCH 1/5] Initial plan From 73a0222e564324370841b077aa70bf4ff1f4d3f5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Feb 2026 20:10:19 +0000 Subject: [PATCH 2/5] feat(storage): implement immutable original/processed file storage and collision handling - Add original_file_path and processed_file_path columns to FileRecord model - Create database migration for new fields - Implement get_unique_filepath_with_counter() with -0001 suffix format - Update process_document to save immutable copy to /workdir/original - Add force_cloud_ocr parameter to process_document for forced OCR - Update embed_metadata to use new collision handling - Update metadata JSON to include file path references - Add /files/{file_id}/reprocess-with-cloud-ocr API endpoint - Update processed_file_path in database during embedding Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- app/api/files.py | 66 ++++++++++++++++++++++ app/models.py | 8 +++ app/tasks/embed_metadata_into_pdf.py | 67 +++++++++++++++-------- app/tasks/process_document.py | 64 ++++++++++++++++++++-- app/utils/__init__.py | 3 +- app/utils/filename_utils.py | 51 +++++++++++++++++ migrations/versions/002_add_file_paths.py | 29 ++++++++++ 7 files changed, 258 insertions(+), 30 deletions(-) create mode 100644 migrations/versions/002_add_file_paths.py diff --git a/app/api/files.py b/app/api/files.py index abdc4ed5..7da4f6ac 100644 --- a/app/api/files.py +++ b/app/api/files.py @@ -392,6 +392,72 @@ def reprocess_single_file(request: Request, file_id: int, db: DbSession): raise HTTPException(status_code=500, detail=f"Error reprocessing file: {str(e)}") +@router.post("/files/{file_id}/reprocess-with-cloud-ocr") +@require_login +def reprocess_with_cloud_ocr(request: Request, file_id: int, db: DbSession): + """ + Reprocess a single file with forced Cloud OCR processing. + + This endpoint forces Azure Document Intelligence OCR processing regardless + of whether the PDF contains embedded text. Useful for documents with + low-quality embedded text or when higher quality OCR is needed. + + Args: + file_id: ID of the file to reprocess with Cloud OCR + + Returns: + Task ID and status information + """ + try: + # Find the file record + file_record = db.query(FileRecord).filter(FileRecord.id == file_id).first() + + if not file_record: + raise HTTPException(status_code=404, detail=f"File with ID {file_id} not found") + + # Prefer using the original_file_path if available, otherwise fall back to local_filename + source_file = None + if file_record.original_file_path and os.path.exists(file_record.original_file_path): + source_file = file_record.original_file_path + logger.info(f"Using original file for Cloud OCR reprocessing: {source_file}") + elif file_record.local_filename and os.path.exists(file_record.local_filename): + source_file = file_record.local_filename + logger.info(f"Using local file for Cloud OCR reprocessing: {source_file}") + else: + raise HTTPException( + status_code=400, + detail="Neither original nor local file found on disk. Cannot reprocess." + ) + + # Queue the file for processing with force_cloud_ocr=True + task = process_document.delay( + source_file, + original_filename=file_record.original_filename, + file_id=file_record.id, + force_cloud_ocr=True + ) + + logger.info( + f"Reprocessing file with Cloud OCR: ID={file_record.id}, " + f"Filename={file_record.original_filename}, TaskID={task.id}" + ) + + return { + "status": "success", + "message": "File queued for Cloud OCR reprocessing", + "file_id": file_record.id, + "filename": file_record.original_filename, + "task_id": task.id, + "force_cloud_ocr": True, + } + + except HTTPException: + raise + except Exception as e: + logger.exception(f"Error reprocessing file {file_id} with Cloud OCR: {str(e)}") + raise HTTPException(status_code=500, detail=f"Error reprocessing file with Cloud OCR: {str(e)}") + + def _extract_text_from_pdf(file_path: str) -> str: """ Extract text from a PDF file using PyPDF2. diff --git a/app/models.py b/app/models.py index 9be67422..9ca3c859 100644 --- a/app/models.py +++ b/app/models.py @@ -30,6 +30,14 @@ class FileRecord(Base): # The name/path we store on disk (e.g. /workdir/tmp/.pdf) local_filename = Column(String, nullable=False) + # Immutable original copy path (e.g. /workdir/original/.pdf) + # This is the first copy made when the file is ingested + original_file_path = Column(String) + + # Processed copy path (e.g. /workdir/processed/2024-01-01_Invoice.pdf) + # This is the final file with embedded metadata before upload + processed_file_path = Column(String) + # Size of the file in bytes file_size = Column(Integer, nullable=False) diff --git a/app/tasks/embed_metadata_into_pdf.py b/app/tasks/embed_metadata_into_pdf.py index ba3b9b3a..fd8ec487 100644 --- a/app/tasks/embed_metadata_into_pdf.py +++ b/app/tasks/embed_metadata_into_pdf.py @@ -16,7 +16,7 @@ from app.database import SessionLocal from app.models import FileRecord from app.tasks.finalize_document_storage import finalize_document_storage from app.tasks.retry_config import BaseTaskWithRetry -from app.utils import log_task_progress +from app.utils import get_unique_filepath_with_counter, log_task_progress from app.utils.filename_utils import sanitize_filename logger = logging.getLogger(__name__) @@ -30,32 +30,35 @@ TMP_SUBDIR = "tmp" PROCESSED_SUBDIR = "processed" -def unique_filepath(directory, base_filename, extension=".pdf"): - """ - Returns a unique filepath in the specified directory. - If 'base_filename.pdf' exists, it will append an underscore and counter. - """ - candidate = os.path.join(directory, base_filename + extension) - if not os.path.exists(candidate): - return candidate - counter = 1 - while True: - candidate = os.path.join(directory, f"{base_filename}_{counter}{extension}") - if not os.path.exists(candidate): - return candidate - counter += 1 - - -def persist_metadata(metadata, final_pdf_path): +def persist_metadata(metadata, final_pdf_path, original_file_path=None, processed_file_path=None): """ Saves the metadata dictionary to a JSON file with the same base name as the final PDF. For example, if final_pdf_path is "/processed/MyFile.pdf", the metadata will be saved as "/processed/MyFile.json". + + Optionally augments the metadata with file path references for traceability. + + Args: + metadata: Dictionary of metadata to save + final_pdf_path: Path to the final PDF file + original_file_path: Optional path to the immutable original file + processed_file_path: Optional path to the processed file + + Returns: + str: Path to the created JSON file """ base, _ = os.path.splitext(final_pdf_path) json_path = base + ".json" + + # Augment metadata with file path references if provided + metadata_with_paths = metadata.copy() + if original_file_path: + metadata_with_paths["original_file_path"] = original_file_path + if processed_file_path: + metadata_with_paths["processed_file_path"] = processed_file_path + with open(json_path, "w", encoding="utf-8") as f: - json.dump(metadata, f, ensure_ascii=False, indent=2) + json.dump(metadata_with_paths, f, ensure_ascii=False, indent=2) return json_path @@ -159,15 +162,15 @@ def embed_metadata_into_pdf(self, local_file_path: str, extracted_text: str, met # Define the final directory based on settings.workdir and ensure it exists. final_dir = os.path.join(settings.workdir, PROCESSED_SUBDIR) os.makedirs(final_dir, exist_ok=True) - # Get a unique filepath in case of collisions. - final_file_path = unique_filepath(final_dir, suggested_filename, extension=".pdf") + # Get a unique filepath in case of collisions using -0001, -0002 suffix format + final_file_path = get_unique_filepath_with_counter(final_dir, suggested_filename, extension=".pdf") logger.info(f"[{task_id}] Moving file to: {final_file_path}") log_task_progress( task_id, "move_to_processed", "in_progress", - f"Moving to processed: {suggested_filename}.pdf", + f"Moving to processed: {os.path.basename(final_file_path)}", file_id=file_id, ) # Move the processed file using shutil.move to handle cross-device moves. @@ -179,10 +182,28 @@ def embed_metadata_into_pdf(self, local_file_path: str, extracted_text: str, met task_id, "move_to_processed", "success", f"Moved to: {os.path.basename(final_file_path)}", file_id=file_id ) + # Get the original_file_path from the database + original_file_path = None + with SessionLocal() as db: + if file_id: + file_record = db.query(FileRecord).filter_by(id=file_id).first() + if file_record: + original_file_path = file_record.original_file_path + # Update the processed_file_path in the database + file_record.processed_file_path = final_file_path + db.commit() + logger.info(f"[{task_id}] Updated database with processed_file_path: {final_file_path}") + # Persist the metadata into a JSON file with the same base name. + # Include file path references for traceability logger.info(f"[{task_id}] Persisting metadata to JSON") log_task_progress(task_id, "save_metadata_json", "in_progress", "Saving metadata JSON", file_id=file_id) - json_path = persist_metadata(metadata, final_file_path) + json_path = persist_metadata( + metadata, + final_file_path, + original_file_path=original_file_path, + processed_file_path=final_file_path + ) logger.info(f"[{task_id}] Metadata persisted to {json_path}") log_task_progress( task_id, "save_metadata_json", "success", f"Saved: {os.path.basename(json_path)}", file_id=file_id diff --git a/app/tasks/process_document.py b/app/tasks/process_document.py index e0a72da1..e214e2e0 100644 --- a/app/tasks/process_document.py +++ b/app/tasks/process_document.py @@ -17,13 +17,13 @@ from app.tasks.process_with_azure_document_intelligence import ( process_with_azure_document_intelligence, ) from app.tasks.retry_config import BaseTaskWithRetry -from app.utils import hash_file, log_task_progress +from app.utils import get_unique_filepath_with_counter, hash_file, log_task_progress logger = logging.getLogger(__name__) @celery.task(base=BaseTaskWithRetry, bind=True) -def process_document(self, original_local_file: str, original_filename: str = None, file_id: int = None): +def process_document(self, original_local_file: str, original_filename: str = None, file_id: int = None, force_cloud_ocr: bool = False): """ Process a document file and trigger appropriate text extraction. @@ -32,14 +32,18 @@ def process_document(self, original_local_file: str, original_filename: str = No original_filename: Optional original filename (if different from path basename) file_id: Optional existing file record ID. When provided, skips duplicate detection and reuses the existing record (used for reprocessing). + force_cloud_ocr: If True, forces Azure Document Intelligence OCR processing + regardless of embedded text quality. Used for re-processing. Steps: 1. Check if we have a FileRecord entry (via SHA-256 hash). If found, skip re-processing. (Skipped when file_id is provided for reprocessing.) 2. If not found, insert a new DB row and continue with the pipeline: - - Copy file to /workdir/tmp + - Save immutable copy to /workdir/original + - Copy file to /workdir/tmp for processing - Check for embedded text. If present, run local GPT extraction - Otherwise, queue Azure Document Intelligence processing + 3. If force_cloud_ocr is True, skip local text extraction and use cloud OCR """ task_id = self.request.id logger.info(f"[{task_id}] Starting document processing: {original_local_file}") @@ -142,16 +146,43 @@ def process_document(self, original_local_file: str, original_filename: str = No file_id=new_record.id, ) - # 1. Generate a UUID-based filename and place it in /workdir/tmp + # 1. Generate a UUID-based filename for storage file_ext = os.path.splitext(original_local_file)[1] file_uuid = str(uuid.uuid4()) new_filename = f"{file_uuid}{file_ext}" + # 2. Save immutable copy to /workdir/original + # This copy serves as the permanent, untouched reference of the ingested file + original_dir = os.path.join(settings.workdir, "original") + os.makedirs(original_dir, exist_ok=True) + + # Use collision-resistant naming with -0001, -0002 suffixes + base_name = os.path.splitext(new_filename)[0] + original_file_path = get_unique_filepath_with_counter(original_dir, base_name, file_ext) + + logger.info(f"[{task_id}] Saving immutable original to: {original_file_path}") + log_task_progress( + task_id, + "save_original", + "in_progress", + f"Saving original to {os.path.basename(original_file_path)}", + file_id=new_record.id, + ) + shutil.copy(original_local_file, original_file_path) + log_task_progress( + task_id, + "save_original", + "success", + f"Original saved: {os.path.basename(original_file_path)}", + file_id=new_record.id, + ) + + # 3. Copy to /workdir/tmp for processing tmp_dir = os.path.join(settings.workdir, "tmp") os.makedirs(tmp_dir, exist_ok=True) new_local_path = os.path.join(tmp_dir, new_filename) - logger.info(f"[{task_id}] Copying file to: {new_local_path}") + logger.info(f"[{task_id}] Copying file to processing area: {new_local_path}") log_task_progress( task_id, "copy_file", @@ -169,14 +200,35 @@ def process_document(self, original_local_file: str, original_filename: str = No file_id=new_record.id, ) - # Update the DB with final local filename + # Update the DB with file paths new_record.local_filename = new_local_path + new_record.original_file_path = original_file_path db.commit() # Store file_id before session closes to avoid DetachedInstanceError file_id = new_record.id # 2. Check for embedded text (outside the DB session to avoid long open transactions) + # Skip local text extraction if force_cloud_ocr is True + if force_cloud_ocr: + logger.info(f"[{task_id}] Force Cloud OCR requested, skipping embedded text check") + log_task_progress( + task_id, + "check_text", + "success", + "Force Cloud OCR requested, queuing OCR", + file_id=file_id, + ) + log_task_progress( + task_id, + "process_document", + "success", + "Queued for forced OCR processing", + file_id=file_id, + ) + process_with_azure_document_intelligence.delay(new_filename, file_id) + return {"file": new_local_path, "status": "Queued for forced OCR", "file_id": file_id} + logger.info(f"[{task_id}] Checking for embedded text in PDF") log_task_progress( task_id, diff --git a/app/utils/__init__.py b/app/utils/__init__.py index 8a4ad1a3..8a8c0d7f 100644 --- a/app/utils/__init__.py +++ b/app/utils/__init__.py @@ -4,7 +4,8 @@ Utility functions and helpers for the document processor application. # Import functions to make them available through the package from app.utils.file_operations import hash_file +from app.utils.filename_utils import get_unique_filepath_with_counter, sanitize_filename from app.utils.logging import log_task_progress # Export all the functions that should be available when importing from app.utils -__all__ = ["hash_file", "log_task_progress"] +__all__ = ["hash_file", "log_task_progress", "get_unique_filepath_with_counter", "sanitize_filename"] diff --git a/app/utils/filename_utils.py b/app/utils/filename_utils.py index 83b8baff..498475c7 100644 --- a/app/utils/filename_utils.py +++ b/app/utils/filename_utils.py @@ -69,6 +69,57 @@ def get_unique_filename(original_path, check_exists_func=None): return new_path +def get_unique_filepath_with_counter(directory, base_filename, extension=".pdf"): + """ + Returns a unique filepath in the specified directory using a numeric counter suffix. + If 'base_filename.pdf' exists, it will append '-0001', '-0002', etc. + + This function implements robust collision handling with zero-padded numeric suffixes + as required for document storage organization. + + Args: + directory (str): Directory path where the file will be stored + base_filename (str): Base name for the file (without extension) + extension (str): File extension including the dot (default: ".pdf") + + Returns: + str: Full path to a unique filename + + Examples: + >>> get_unique_filepath_with_counter("/workdir/original", "2024-01-01_Invoice") + "/workdir/original/2024-01-01_Invoice.pdf" # If doesn't exist + + >>> get_unique_filepath_with_counter("/workdir/original", "2024-01-01_Invoice") + "/workdir/original/2024-01-01_Invoice-0001.pdf" # If original exists + """ + # Try the base filename first + candidate = os.path.join(directory, base_filename + extension) + if not os.path.exists(candidate): + return candidate + + # If base exists, try with counter suffix + counter = 1 + while True: + # Use zero-padded 4-digit counter: -0001, -0002, etc. + suffix = f"-{counter:04d}" + candidate = os.path.join(directory, f"{base_filename}{suffix}{extension}") + if not os.path.exists(candidate): + return candidate + counter += 1 + + # Sanity check to prevent infinite loops (very unlikely to reach) + if counter > 9999: + # Fall back to timestamp + UUID if somehow we have 10000 collisions + timestamp = datetime.now().strftime("%Y%m%d_%H%M%S") + uuid_str = str(uuid.uuid4())[:8] + candidate = os.path.join(directory, f"{base_filename}-{timestamp}-{uuid_str}{extension}") + logger.warning( + f"Exceeded 9999 file collisions for {base_filename}, " + f"using timestamp+UUID: {os.path.basename(candidate)}" + ) + return candidate + + def sanitize_filename(filename): r""" Sanitize a filename to ensure it's valid across different file systems diff --git a/migrations/versions/002_add_file_paths.py b/migrations/versions/002_add_file_paths.py new file mode 100644 index 00000000..e05d9a8b --- /dev/null +++ b/migrations/versions/002_add_file_paths.py @@ -0,0 +1,29 @@ +"""Add original_file_path and processed_file_path to FileRecord + +Revision ID: 002_add_file_paths +Revises: 001_file_processing_steps +Create Date: 2026-02-11 + +""" + +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = "002_add_file_paths" +down_revision: Union[str, None] = "001_file_processing_steps" +depends_on: Union[str, None] = None + + +def upgrade() -> None: + """Add original_file_path and processed_file_path columns to files table.""" + op.add_column("files", sa.Column("original_file_path", sa.String(), nullable=True)) + op.add_column("files", sa.Column("processed_file_path", sa.String(), nullable=True)) + + +def downgrade() -> None: + """Remove original_file_path and processed_file_path columns from files table.""" + op.drop_column("files", "processed_file_path") + op.drop_column("files", "original_file_path") From 952e27b33901f15466ecf93eb2fbd5200bcf3fd7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Feb 2026 20:13:17 +0000 Subject: [PATCH 3/5] test: add tests for storage reorganization features - Add tests for get_unique_filepath_with_counter collision handling - Add tests for immutable original storage - Add tests for metadata augmentation with file paths - Add tests for force_cloud_ocr parameter - Verify existing process_document tests still pass Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- app/tasks/process_document.py | 68 +++--- tests/test_filename_utils.py | 88 ++++++++ tests/test_storage_reorganization.py | 316 +++++++++++++++++++++++++++ 3 files changed, 445 insertions(+), 27 deletions(-) create mode 100644 tests/test_storage_reorganization.py diff --git a/app/tasks/process_document.py b/app/tasks/process_document.py index e214e2e0..9eae16d6 100644 --- a/app/tasks/process_document.py +++ b/app/tasks/process_document.py @@ -151,31 +151,46 @@ def process_document(self, original_local_file: str, original_filename: str = No file_uuid = str(uuid.uuid4()) new_filename = f"{file_uuid}{file_ext}" - # 2. Save immutable copy to /workdir/original - # This copy serves as the permanent, untouched reference of the ingested file - original_dir = os.path.join(settings.workdir, "original") - os.makedirs(original_dir, exist_ok=True) - - # Use collision-resistant naming with -0001, -0002 suffixes - base_name = os.path.splitext(new_filename)[0] - original_file_path = get_unique_filepath_with_counter(original_dir, base_name, file_ext) - - logger.info(f"[{task_id}] Saving immutable original to: {original_file_path}") - log_task_progress( - task_id, - "save_original", - "in_progress", - f"Saving original to {os.path.basename(original_file_path)}", - file_id=new_record.id, - ) - shutil.copy(original_local_file, original_file_path) - log_task_progress( - task_id, - "save_original", - "success", - f"Original saved: {os.path.basename(original_file_path)}", - file_id=new_record.id, - ) + # 2. Save immutable copy to /workdir/original (only for new files, not reprocessing) + # For reprocessing, the original_file_path should already exist in the database + if file_id is None: # New file - save original copy + # This copy serves as the permanent, untouched reference of the ingested file + original_dir = os.path.join(settings.workdir, "original") + os.makedirs(original_dir, exist_ok=True) + + # Use collision-resistant naming with -0001, -0002 suffixes + base_name = os.path.splitext(new_filename)[0] + original_file_path = get_unique_filepath_with_counter(original_dir, base_name, file_ext) + + logger.info(f"[{task_id}] Saving immutable original to: {original_file_path}") + log_task_progress( + task_id, + "save_original", + "in_progress", + f"Saving original to {os.path.basename(original_file_path)}", + file_id=new_record.id, + ) + shutil.copy(original_local_file, original_file_path) + log_task_progress( + task_id, + "save_original", + "success", + f"Original saved: {os.path.basename(original_file_path)}", + file_id=new_record.id, + ) + + # Update the DB with original_file_path + new_record.original_file_path = original_file_path + else: + # Reprocessing - original should already exist + logger.info(f"[{task_id}] Reprocessing: original file already saved at {new_record.original_file_path}") + log_task_progress( + task_id, + "save_original", + "success", + "Reprocessing: using existing original", + file_id=new_record.id, + ) # 3. Copy to /workdir/tmp for processing tmp_dir = os.path.join(settings.workdir, "tmp") @@ -200,9 +215,8 @@ def process_document(self, original_local_file: str, original_filename: str = No file_id=new_record.id, ) - # Update the DB with file paths + # Update the DB with local_filename new_record.local_filename = new_local_path - new_record.original_file_path = original_file_path db.commit() # Store file_id before session closes to avoid DetachedInstanceError diff --git a/tests/test_filename_utils.py b/tests/test_filename_utils.py index 3fb9df7d..89a32ee6 100644 --- a/tests/test_filename_utils.py +++ b/tests/test_filename_utils.py @@ -257,3 +257,91 @@ class TestFilenameUtilsEdgeCases: assert '"' not in result assert "|" not in result assert "?" not in result + + + +@pytest.mark.unit +class TestUniqueFilepathWithCounter: + """Test unique filepath generation with numeric counter suffix""" + + def test_get_unique_filepath_with_counter_no_collision(self, tmp_path): + """Test that original filename is returned when no collision exists""" + from app.utils.filename_utils import get_unique_filepath_with_counter + + result = get_unique_filepath_with_counter(str(tmp_path), "document") + assert result == str(tmp_path / "document.pdf") + + def test_get_unique_filepath_with_counter_single_collision(self, tmp_path): + """Test that -0001 suffix is added on first collision""" + from app.utils.filename_utils import get_unique_filepath_with_counter + + # Create the base file + (tmp_path / "document.pdf").touch() + + result = get_unique_filepath_with_counter(str(tmp_path), "document") + assert result == str(tmp_path / "document-0001.pdf") + + def test_get_unique_filepath_with_counter_multiple_collisions(self, tmp_path): + """Test that counter increments correctly for multiple collisions""" + from app.utils.filename_utils import get_unique_filepath_with_counter + + # Create files with base name and first two counter suffixes + (tmp_path / "document.pdf").touch() + (tmp_path / "document-0001.pdf").touch() + (tmp_path / "document-0002.pdf").touch() + + result = get_unique_filepath_with_counter(str(tmp_path), "document") + assert result == str(tmp_path / "document-0003.pdf") + + def test_get_unique_filepath_with_counter_custom_extension(self, tmp_path): + """Test with custom file extension""" + from app.utils.filename_utils import get_unique_filepath_with_counter + + (tmp_path / "data.json").touch() + + result = get_unique_filepath_with_counter(str(tmp_path), "data", extension=".json") + assert result == str(tmp_path / "data-0001.json") + + def test_get_unique_filepath_with_counter_zero_padded(self, tmp_path): + """Test that counter uses zero-padded 4-digit format""" + from app.utils.filename_utils import get_unique_filepath_with_counter + + (tmp_path / "invoice.pdf").touch() + + result = get_unique_filepath_with_counter(str(tmp_path), "invoice") + # Should be -0001, not -1 + assert result == str(tmp_path / "invoice-0001.pdf") + assert "-1.pdf" not in result + + def test_get_unique_filepath_with_counter_preserves_filename(self, tmp_path): + """Test that complex filenames are preserved""" + from app.utils.filename_utils import get_unique_filepath_with_counter + + filename = "2024-01-01_Invoice_Company-Name" + (tmp_path / f"{filename}.pdf").touch() + + result = get_unique_filepath_with_counter(str(tmp_path), filename) + assert filename in result + assert result == str(tmp_path / f"{filename}-0001.pdf") + + def test_get_unique_filepath_with_counter_high_count(self, tmp_path): + """Test that function handles high counter values""" + from app.utils.filename_utils import get_unique_filepath_with_counter + + # Create files up to -0099 + (tmp_path / "test.pdf").touch() + for i in range(1, 100): + (tmp_path / f"test-{i:04d}.pdf").touch() + + result = get_unique_filepath_with_counter(str(tmp_path), "test") + assert result == str(tmp_path / "test-0100.pdf") + + def test_get_unique_filepath_with_counter_directory_creation(self, tmp_path): + """Test with directory that already exists""" + from app.utils.filename_utils import get_unique_filepath_with_counter + + # Directory already exists (tmp_path) + result = get_unique_filepath_with_counter(str(tmp_path), "newfile") + assert result == str(tmp_path / "newfile.pdf") + # File shouldn't be created, just path returned + assert not os.path.exists(result) diff --git a/tests/test_storage_reorganization.py b/tests/test_storage_reorganization.py new file mode 100644 index 00000000..8dd30d13 --- /dev/null +++ b/tests/test_storage_reorganization.py @@ -0,0 +1,316 @@ +""" +Tests for document storage reorganization features. + +Tests the new functionality for storing immutable originals and processed copies, +collision handling, and forced Cloud OCR reprocessing. +""" + +import os +import json +from unittest.mock import MagicMock, patch + +import pytest +from sqlalchemy.orm import Session + +from app.models import FileRecord + + +@pytest.mark.unit +@pytest.mark.requires_db +class TestImmutableOriginalStorage: + """Test that original files are saved immutably to /workdir/original""" + + def test_new_file_saves_original_copy(self, db_session, tmp_path): + """Test that a new file creates an immutable original copy""" + from app.tasks.process_document import process_document + + # Create test PDF + test_pdf = tmp_path / "test_input.pdf" + pdf_content = b"""%PDF-1.4 +1 0 obj +<< +/Type /Catalog +/Pages 2 0 R +>> +endobj +2 0 obj +<< +/Type /Pages +/Kids [3 0 R] +/Count 1 +>> +endobj +3 0 obj +<< +/Type /Page +/Parent 2 0 R +/MediaBox [0 0 612 792] +/Resources << +/Font << +/F1 << +/Type /Font +/Subtype /Type1 +/BaseFont /Helvetica +>> +>> +>> +/Contents 4 0 R +>> +endobj +4 0 obj +<< +/Length 44 +>> +stream +BT +/F1 12 Tf +100 700 Td +(Test content) Tj +ET +endstream +endobj +xref +0 5 +0000000000 65535 f +0000000009 00000 n +0000000058 00000 n +0000000115 00000 n +0000000306 00000 n +trailer +<< +/Size 5 +/Root 1 0 R +>> +startxref +399 +%%EOF +""" + test_pdf.write_bytes(pdf_content) + + # Setup mocks + with ( + patch("app.tasks.process_document.SessionLocal") as mock_session_local, + patch("app.tasks.process_document.settings") as mock_settings, + patch("app.tasks.process_document.log_task_progress"), + patch("app.tasks.process_document.extract_metadata_with_gpt") as mock_extract, + ): + mock_settings.workdir = str(tmp_path) + mock_session_local.return_value.__enter__.return_value = db_session + mock_session_local.return_value.__exit__.return_value = None + mock_extract.delay = MagicMock() + + # Call process_document + result = process_document(str(test_pdf), original_filename="test_input.pdf") + + # Verify original directory was created + original_dir = tmp_path / "original" + assert original_dir.exists() + + # Verify an original file was saved + original_files = list(original_dir.glob("*.pdf")) + assert len(original_files) > 0 + + # Verify database record has original_file_path + file_record = db_session.query(FileRecord).first() + assert file_record is not None + assert file_record.original_file_path is not None + assert "original" in file_record.original_file_path + + def test_reprocessing_preserves_original(self, db_session, tmp_path): + """Test that reprocessing doesn't create a duplicate original""" + from app.tasks.process_document import process_document + + # Create test file and original + test_pdf = tmp_path / "test.pdf" + test_pdf.write_bytes(b"%PDF-1.4\ntest") + + original_dir = tmp_path / "original" + original_dir.mkdir() + original_file = original_dir / "existing-original.pdf" + original_file.write_bytes(b"%PDF-1.4\noriginal") + + # Create existing file record + file_record = FileRecord( + filehash="abc123", + original_filename="test.pdf", + local_filename=str(test_pdf), + original_file_path=str(original_file), + file_size=100, + mime_type="application/pdf" + ) + db_session.add(file_record) + db_session.commit() + + # Setup mocks + with ( + patch("app.tasks.process_document.SessionLocal") as mock_session_local, + patch("app.tasks.process_document.settings") as mock_settings, + patch("app.tasks.process_document.log_task_progress"), + patch("app.tasks.process_document.extract_metadata_with_gpt") as mock_extract, + ): + mock_settings.workdir = str(tmp_path) + mock_session_local.return_value.__enter__.return_value = db_session + mock_session_local.return_value.__exit__.return_value = None + mock_extract.delay = MagicMock() + + # Reprocess with file_id + original_count = len(list(original_dir.glob("*.pdf"))) + process_document(str(test_pdf), file_id=file_record.id) + + # Should not create new original file + new_count = len(list(original_dir.glob("*.pdf"))) + assert new_count == original_count + + +@pytest.mark.unit +class TestCollisionHandling: + """Test filename collision handling with -0001 suffix format""" + + def test_collision_handling_in_processed_dir(self, tmp_path): + """Test that collision handling works in processed directory""" + from app.utils.filename_utils import get_unique_filepath_with_counter + + processed_dir = tmp_path / "processed" + processed_dir.mkdir() + + # Create first file + (processed_dir / "2024-01-01_Invoice.pdf").touch() + + # Get unique path for same filename + result = get_unique_filepath_with_counter(str(processed_dir), "2024-01-01_Invoice") + + assert "2024-01-01_Invoice-0001.pdf" in result + assert os.path.exists(str(processed_dir / "2024-01-01_Invoice.pdf")) + + +@pytest.mark.unit +class TestMetadataAugmentation: + """Test that metadata JSON includes file path references""" + + def test_metadata_includes_file_paths(self, tmp_path): + """Test that persisted metadata includes original and processed paths""" + from app.tasks.embed_metadata_into_pdf import persist_metadata + + metadata = { + "filename": "2024-01-01_Invoice", + "document_type": "Invoice", + "tags": ["finance", "2024"] + } + + processed_file = tmp_path / "processed" / "2024-01-01_Invoice.pdf" + processed_file.parent.mkdir(parents=True) + processed_file.touch() + + original_path = "/workdir/original/abc123.pdf" + processed_path = str(processed_file) + + json_path = persist_metadata( + metadata, + str(processed_file), + original_file_path=original_path, + processed_file_path=processed_path + ) + + # Verify JSON was created + assert os.path.exists(json_path) + + # Verify content + with open(json_path, 'r') as f: + saved_metadata = json.load(f) + + assert "original_file_path" in saved_metadata + assert saved_metadata["original_file_path"] == original_path + assert "processed_file_path" in saved_metadata + assert saved_metadata["processed_file_path"] == processed_path + assert saved_metadata["filename"] == "2024-01-01_Invoice" + + +@pytest.mark.unit +@pytest.mark.requires_db +class TestForceCloudOCR: + """Test forced Cloud OCR reprocessing functionality""" + + def test_force_cloud_ocr_parameter(self, db_session, tmp_path): + """Test that force_cloud_ocr parameter skips local text extraction""" + from app.tasks.process_document import process_document + + # Create PDF with embedded text + test_pdf = tmp_path / "with_text.pdf" + pdf_content = b"""%PDF-1.4 +1 0 obj +<< +/Type /Catalog +/Pages 2 0 R +>> +endobj +2 0 obj +<< +/Type /Pages +/Kids [3 0 R] +/Count 1 +>> +endobj +3 0 obj +<< +/Type /Page +/Parent 2 0 R +/MediaBox [0 0 612 792] +/Resources << +/Font << +/F1 << +/Type /Font +/Subtype /Type1 +/BaseFont /Helvetica +>> +>> +>> +/Contents 4 0 R +>> +endobj +4 0 obj +<< +/Length 44 +>> +stream +BT +/F1 12 Tf +100 700 Td +(Embedded text here) Tj +ET +endstream +endobj +xref +0 5 +0000000000 65535 f +0000000009 00000 n +0000000058 00000 n +0000000115 00000 n +0000000306 00000 n +trailer +<< +/Size 5 +/Root 1 0 R +>> +startxref +399 +%%EOF +""" + test_pdf.write_bytes(pdf_content) + + with ( + patch("app.tasks.process_document.SessionLocal") as mock_session_local, + patch("app.tasks.process_document.settings") as mock_settings, + patch("app.tasks.process_document.log_task_progress"), + patch("app.tasks.process_document.process_with_azure_document_intelligence") as mock_azure, + ): + mock_settings.workdir = str(tmp_path) + mock_session_local.return_value.__enter__.return_value = db_session + mock_session_local.return_value.__exit__.return_value = None + mock_azure.delay = MagicMock() + + # Process with force_cloud_ocr=True + result = process_document(str(test_pdf), force_cloud_ocr=True) + + # Should queue Azure OCR, not local extraction + mock_azure.delay.assert_called_once() + assert result["status"] == "Queued for forced OCR" From c7dff41492c47227867803e295fbfc51d38dab67 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Feb 2026 20:14:56 +0000 Subject: [PATCH 4/5] docs: add comprehensive documentation for storage reorganization - Create StorageArchitecture.md with detailed architecture explanation - Document collision handling with -0001 suffix format - Explain immutable original and processed file storage - Document forced Cloud OCR reprocessing feature - Update API.md with new reprocess-with-cloud-ocr endpoint - Update UserGuide.md with user-facing feature descriptions - Include database schema, lifecycle diagrams, and examples Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- docs/API.md | 26 +++- docs/StorageArchitecture.md | 302 ++++++++++++++++++++++++++++++++++++ docs/UserGuide.md | 13 +- 3 files changed, 338 insertions(+), 3 deletions(-) create mode 100644 docs/StorageArchitecture.md diff --git a/docs/API.md b/docs/API.md index 36f421f5..6eeead7a 100644 --- a/docs/API.md +++ b/docs/API.md @@ -240,6 +240,28 @@ Reprocess a specific file. This queues the file for complete reprocessing throug - `404`: File not found - `400`: Local file not found on disk (cannot reprocess) +**POST** `/api/files/{file_id}/reprocess-with-cloud-ocr` + +Reprocess a specific file with forced Cloud OCR, regardless of embedded text quality. This is useful for documents with low-quality embedded text or when higher quality OCR is needed. + +**Response**: +```json +{ + "status": "success", + "message": "File queued for Cloud OCR reprocessing", + "file_id": 123, + "filename": "invoice.pdf", + "task_id": "a1b2c3d4-e5f6-7g8h-9i0j-k1l2m3n4o5p6", + "force_cloud_ocr": true +} +``` + +**Error Responses**: +- `404`: File not found +- `400`: Neither original nor local file found on disk (cannot reprocess) + +**Note**: This endpoint forces Azure Document Intelligence OCR processing even if the PDF contains embedded text. The original file (if available) is used for reprocessing to ensure the highest quality result. + ### File Preview **GET** `/api/files/{file_id}/preview` @@ -248,8 +270,8 @@ Retrieve the file content for preview purposes. **Parameters**: - `version` (required): Either `original` or `processed` - - `original`: Returns the file as it was uploaded (from tmp directory) - - `processed`: Returns the file after metadata embedding (from processed directory) + - `original`: Returns the immutable original file from the original directory + - `processed`: Returns the file after metadata embedding from the processed directory **Response**: Returns the file content with appropriate MIME type for browser display. diff --git a/docs/StorageArchitecture.md b/docs/StorageArchitecture.md new file mode 100644 index 00000000..f37f76d2 --- /dev/null +++ b/docs/StorageArchitecture.md @@ -0,0 +1,302 @@ +# Document Storage Architecture + +## Overview + +DocuElevate implements a robust document storage architecture that maintains immutable originals, processed copies, and comprehensive traceability throughout the document lifecycle. + +## Storage Structure + +### Directory Layout + +``` +workdir/ +├── original/ # Immutable original files (never modified) +│ ├── .pdf +│ ├── -0001.pdf +│ └── ... +├── tmp/ # Temporary processing area +│ ├── .pdf +│ └── ... +└── processed/ # Final processed files with metadata + ├── 2024-01-01_Invoice.pdf + ├── 2024-01-01_Invoice.json + ├── 2024-01-15_Contract-0001.pdf + └── ... +``` + +### Directory Purposes + +#### `/workdir/original` +- **Purpose**: Immutable storage of files as they were first ingested +- **When Created**: When a new file is uploaded or processed +- **Naming**: UUID-based to prevent collisions (e.g., `a1b2c3d4-e5f6.pdf`) +- **Immutability**: Files in this directory are never modified or deleted +- **Database Reference**: `FileRecord.original_file_path` + +#### `/workdir/tmp` +- **Purpose**: Temporary working directory for document processing +- **When Created**: During processing pipeline +- **Lifecycle**: Files are copied here during processing and may be deleted after successful completion +- **Database Reference**: `FileRecord.local_filename` + +#### `/workdir/processed` +- **Purpose**: Final processed files with embedded metadata +- **When Created**: After successful metadata extraction and embedding +- **Naming**: Human-readable names from metadata (e.g., `2024-01-01_Invoice.pdf`) +- **Collision Handling**: Automatic `-0001`, `-0002` suffix when names collide +- **Database Reference**: `FileRecord.processed_file_path` +- **Companion Files**: Each PDF has a corresponding `.json` file with metadata + +## Collision Handling + +### Naming Strategy + +When a file name collision occurs in the `processed` directory, DocuElevate automatically appends a zero-padded numeric suffix: + +``` +2024-01-01_Invoice.pdf # First file +2024-01-01_Invoice-0001.pdf # First collision +2024-01-01_Invoice-0002.pdf # Second collision +2024-01-01_Invoice-0003.pdf # Third collision +... +2024-01-01_Invoice-9999.pdf # Max numeric suffix +``` + +### Features +- **Zero-padded**: Always uses 4-digit format (`-0001`, not `-1`) +- **Automatic**: No user intervention required +- **Deterministic**: Same base name always gets next available number +- **Scalable**: Supports up to 10,000 variations of the same filename + +### Implementation + +The collision handling is implemented in `app/utils/filename_utils.py`: + +```python +from app.utils import get_unique_filepath_with_counter + +# Get unique path with automatic collision handling +unique_path = get_unique_filepath_with_counter( + directory="/workdir/processed", + base_filename="2024-01-01_Invoice", + extension=".pdf" +) +# Returns: "/workdir/processed/2024-01-01_Invoice.pdf" (or -0001, -0002, etc.) +``` + +## Document Lifecycle + +### 1. Initial Upload + +``` +User uploads document.pdf + ↓ +Saved to /workdir/.pdf + ↓ +File hash computed (SHA-256) + ↓ +Database record created +``` + +### 2. Processing Pipeline + +``` +Original saved to /workdir/original/.pdf + ↓ +Working copy to /workdir/tmp/.pdf + ↓ +Text extraction (local or Cloud OCR) + ↓ +Metadata extraction (GPT) + ↓ +Metadata embedding into PDF + ↓ +Move to /workdir/processed/.pdf + ↓ +Save metadata JSON + ↓ +Queue for upload to destinations + ↓ +Cleanup /workdir/tmp/.pdf +``` + +### 3. Reprocessing + +When reprocessing an existing file: + +``` +User triggers reprocess (file_id provided) + ↓ +Retrieve existing FileRecord + ↓ +Use original_file_path (immutable original) + ↓ +Skip saving new original (already exists) + ↓ +Continue with processing pipeline + ↓ +New processed file may get -0001 suffix +``` + +## Metadata JSON Structure + +Each processed PDF has a companion JSON file with the same base name: + +**File**: `/workdir/processed/2024-01-01_Invoice.json` + +```json +{ + "filename": "2024-01-01_Invoice", + "document_type": "Invoice", + "absender": "ACME Corp", + "empfaenger": "John Doe", + "tags": ["finance", "2024", "Q1"], + "language": "en", + "confidence_score": 95, + "original_file_path": "/workdir/original/a1b2c3d4-e5f6.pdf", + "processed_file_path": "/workdir/processed/2024-01-01_Invoice.pdf" +} +``` + +### Metadata Fields + +#### Core Metadata (from GPT extraction) +- `filename`: Suggested filename from metadata +- `document_type`: Classification (Invoice, Contract, etc.) +- `absender`: Sender +- `empfaenger`: Recipient +- `tags`: Thematic keywords +- `language`: ISO 639-1 language code +- `confidence_score`: Extraction confidence (0-100) + +#### File Path References (added by DocuElevate) +- `original_file_path`: Path to immutable original +- `processed_file_path`: Path to processed file with metadata + +## Forced Cloud OCR + +### Use Cases + +Force Cloud OCR reprocessing when: +1. PDF has poor quality embedded text +2. OCR accuracy is insufficient +3. Embedded text is corrupted or garbled +4. Higher quality extraction is needed + +### API Endpoint + +```bash +POST /api/files/{file_id}/reprocess-with-cloud-ocr +``` + +### Behavior + +1. Bypasses local text extraction +2. Always uses Azure Document Intelligence OCR +3. Processes from `original_file_path` if available +4. Creates new processed file (may get collision suffix) +5. Updates database with new `processed_file_path` + +### Example + +```bash +curl -X POST "http://localhost:8000/api/files/123/reprocess-with-cloud-ocr" \ + -H "Authorization: Bearer YOUR_TOKEN" +``` + +**Response**: +```json +{ + "status": "success", + "message": "File queued for Cloud OCR reprocessing", + "file_id": 123, + "filename": "invoice.pdf", + "task_id": "task-uuid", + "force_cloud_ocr": true +} +``` + +## Database Schema + +### FileRecord Model + +```python +class FileRecord(Base): + __tablename__ = "files" + + id = Column(Integer, primary_key=True) + filehash = Column(String, unique=True, nullable=False) + original_filename = Column(String) # User's original name + local_filename = Column(String) # /workdir/tmp/.pdf + original_file_path = Column(String) # /workdir/original/.pdf + processed_file_path = Column(String) # /workdir/processed/.pdf + file_size = Column(Integer) + mime_type = Column(String) + created_at = Column(DateTime) +``` + +## File Operations Safety + +### Immutability Guarantees + +1. **Original Directory**: Files are never modified or deleted +2. **Processed Directory**: Files are never modified after creation +3. **Path Validation**: All file operations use path validation to prevent traversal +4. **Database Integrity**: File paths are stored in database for traceability + +### Cleanup Policy + +- **Original**: Never deleted (permanent archive) +- **Tmp**: Deleted after successful processing +- **Processed**: Kept until explicitly deleted by user or retention policy + +## Benefits + +### Traceability +- Every file has a permanent, unmodified original +- Complete processing history tracked in database +- Metadata JSON provides audit trail + +### Flexibility +- Reprocessing uses original for best quality +- Forced Cloud OCR option for quality improvements +- Multiple processed versions can coexist + +### Reliability +- Collision handling prevents file overwrites +- Immutable originals enable recovery +- Database references ensure consistency + +## Migration + +For existing installations, the new fields are added via database migration: + +```bash +# Migration creates nullable columns +alembic upgrade head + +# Existing files will have NULL for new paths +# Future processing will populate these fields +``` + +### Backfilling + +To populate file paths for existing records: + +```python +from app.models import FileRecord +from app.database import SessionLocal + +with SessionLocal() as db: + for record in db.query(FileRecord).filter( + FileRecord.original_file_path.is_(None) + ): + # Logic to backfill based on local_filename if needed + pass +``` + +## See Also + +- [API Documentation](API.md) - API endpoints for file operations +- [User Guide](UserGuide.md) - User-facing documentation +- [Configuration Guide](ConfigurationGuide.md) - Storage configuration options diff --git a/docs/UserGuide.md b/docs/UserGuide.md index b8be88af..0f18a82a 100644 --- a/docs/UserGuide.md +++ b/docs/UserGuide.md @@ -115,6 +115,12 @@ View the complete processing history with a timeline showing: - Network connectivity was lost during processing - Configuration has been updated and you want to reprocess with new settings +**Force Cloud OCR**: For files with low-quality embedded text, you can use the "Reprocess with Cloud OCR" option to force high-quality Azure Document Intelligence OCR processing, even if the PDF already contains embedded text. This is useful when: +- The embedded text quality is poor or contains errors +- OCR accuracy needs to be improved +- The embedded text is corrupted or garbled +- You need the highest quality text extraction possible + #### Process Flow Visualization The process flow visualization shows a visual representation of the document processing pipeline: - **Green indicators**: Successful stages @@ -128,7 +134,7 @@ This helps you understand: #### File Previews If your file is still available on disk, you can preview it directly in the browser: -- **Original File**: The file as it was uploaded +- **Original File**: The immutable original file as it was first ingested - **Processed File**: The file after metadata has been embedded (if processing completed) Both previews support: @@ -136,6 +142,11 @@ Both previews support: - Opening in a new tab for full-screen viewing - Side-by-side comparison of original and processed versions +**Note**: The original file is stored in an immutable archive and is never modified. This ensures you always have access to the file exactly as it was uploaded, which is valuable for: +- Auditing and compliance +- Debugging processing issues +- Reprocessing with improved algorithms + ## Document Processing Features Depending on the system configuration, DocuElevate can perform: From a247393e18f6fc6b44ec02ac066903192b9e0d09 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Feb 2026 20:17:03 +0000 Subject: [PATCH 5/5] fix: update test to use new collision handling function - Replace unique_filepath with get_unique_filepath_with_counter in tests - Update test expectations for -0001 suffix format - All 40 tests passing successfully Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- tests/test_embed_metadata.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/tests/test_embed_metadata.py b/tests/test_embed_metadata.py index e15a94a8..8457af3b 100644 --- a/tests/test_embed_metadata.py +++ b/tests/test_embed_metadata.py @@ -3,31 +3,32 @@ import os import pytest from unittest.mock import patch, MagicMock -from app.tasks.embed_metadata_into_pdf import unique_filepath, persist_metadata +from app.tasks.embed_metadata_into_pdf import persist_metadata +from app.utils.filename_utils import get_unique_filepath_with_counter @pytest.mark.unit class TestUniqueFilepath: - """Tests for unique_filepath function.""" + """Tests for unique filepath collision handling - now using get_unique_filepath_with_counter.""" def test_returns_path_when_no_conflict(self, tmp_path): """Test returns original path when no conflict.""" - result = unique_filepath(str(tmp_path), "test", ".pdf") + result = get_unique_filepath_with_counter(str(tmp_path), "test", ".pdf") assert result == str(tmp_path / "test.pdf") def test_appends_counter_on_conflict(self, tmp_path): - """Test appends counter when file already exists.""" + """Test appends -0001 counter when file already exists.""" # Create the initial file (tmp_path / "test.pdf").touch() - result = unique_filepath(str(tmp_path), "test", ".pdf") - assert result == str(tmp_path / "test_1.pdf") + result = get_unique_filepath_with_counter(str(tmp_path), "test", ".pdf") + assert result == str(tmp_path / "test-0001.pdf") def test_increments_counter(self, tmp_path): """Test increments counter for multiple conflicts.""" (tmp_path / "test.pdf").touch() - (tmp_path / "test_1.pdf").touch() - result = unique_filepath(str(tmp_path), "test", ".pdf") - assert result == str(tmp_path / "test_2.pdf") + (tmp_path / "test-0001.pdf").touch() + result = get_unique_filepath_with_counter(str(tmp_path), "test", ".pdf") + assert result == str(tmp_path / "test-0002.pdf") @pytest.mark.unit