From 80bb9cec101e5dd61043a9f7864fc2a80ca16f24 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 21:00:30 +0000 Subject: [PATCH 1/4] Initial plan From 42d35c7c6f7bf2391a9a4314e35d4e652ba02bba Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 21:06:20 +0000 Subject: [PATCH 2/4] fix: handle file in processed directory when retrying embed_metadata_into_pdf - Update _retry_pipeline_step to check for file in tmp, processed, and fallback locations - Pass full path to extract_metadata_with_gpt instead of just basename - Update extract_metadata_with_gpt to handle both basename and full path parameters - Add test case for retrying when file is in processed directory Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- app/api/files.py | 32 ++++++++++++++++--- app/tasks/extract_metadata_with_gpt.py | 21 ++++++++++--- tests/test_file_detail_endpoints.py | 43 ++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 10 deletions(-) diff --git a/app/api/files.py b/app/api/files.py index d852e65a..526d9498 100644 --- a/app/api/files.py +++ b/app/api/files.py @@ -525,13 +525,35 @@ def _retry_pipeline_step(file_record: FileRecord, step_name: str, db: Session) - # Retrying embed requires re-running metadata extraction first, because # embed_metadata_into_pdf needs the actual metadata dict (not empty). # Re-trigger extract_metadata_with_gpt which will chain into embed_metadata_into_pdf. - if not file_record.local_filename or not os.path.exists(file_record.local_filename): + + # Check for file in multiple locations: + # 1. Original location in tmp (file_record.local_filename) + # 2. Processed location (file_record.processed_file_path) + # 3. Fallback to workdir/tmp/ + file_path = None + if file_record.local_filename and os.path.exists(file_record.local_filename): + file_path = file_record.local_filename + elif file_record.processed_file_path and os.path.exists(file_record.processed_file_path): + # File has been processed and moved to processed directory + file_path = file_record.processed_file_path + else: + # Try fallback path in workdir/tmp + if file_record.local_filename: + workdir = settings.workdir + tmp_dir = os.path.join(workdir, "tmp") + fallback_path = os.path.join(tmp_dir, os.path.basename(file_record.local_filename)) + if os.path.exists(fallback_path): + file_path = fallback_path + + if not file_path: raise HTTPException( - status_code=400, detail="Local file not found on disk. Cannot retry metadata embedding." + status_code=400, + detail="File not found in tmp or processed directory. Cannot retry metadata embedding." ) - extracted_text = _extract_text_from_pdf(file_record.local_filename) - filename = os.path.basename(file_record.local_filename) - task = extract_metadata_task.delay(filename, extracted_text, file_id) + + extracted_text = _extract_text_from_pdf(file_path) + # Pass the full path to the task so it can locate the file + task = extract_metadata_task.delay(file_path, extracted_text, file_id) else: raise HTTPException(status_code=400, detail=f"Unsupported pipeline step: {step_name}") diff --git a/app/tasks/extract_metadata_with_gpt.py b/app/tasks/extract_metadata_with_gpt.py index 58aca4cf..7c917a51 100644 --- a/app/tasks/extract_metadata_with_gpt.py +++ b/app/tasks/extract_metadata_with_gpt.py @@ -47,17 +47,28 @@ def extract_json_from_text(text): @celery.task(base=BaseTaskWithRetry, bind=True) def extract_metadata_with_gpt(self, filename: str, cleaned_text: str, file_id: int = None): - """Uses OpenAI to classify document metadata.""" + """ + Uses OpenAI to classify document metadata. + + Args: + filename: Can be either a basename (e.g., "file.pdf") or a full path (e.g., "/workdir/processed/file.pdf") + cleaned_text: The extracted text from the document + file_id: Optional file ID for tracking + """ task_id = self.request.id logger.info(f"[{task_id}] Starting metadata extraction for: {filename}") log_task_progress( - task_id, "extract_metadata_with_gpt", "in_progress", f"Extracting metadata for {filename}", file_id=file_id + task_id, "extract_metadata_with_gpt", "in_progress", f"Extracting metadata for {os.path.basename(filename)}", file_id=file_id ) # Get file_id from database if not provided if file_id is None: tmp_dir = os.path.join(settings.workdir, "tmp") - file_path = os.path.join(tmp_dir, filename) + # Handle both basename and full path + if os.path.isabs(filename): + file_path = filename + else: + file_path = os.path.join(tmp_dir, filename) if os.path.exists(file_path): with SessionLocal() as db: file_record = db.query(FileRecord).filter_by(local_filename=file_path).first() @@ -162,14 +173,14 @@ def extract_metadata_with_gpt(self, filename: str, cleaned_text: str, file_id: i ) # Trigger the next step: embedding metadata into the PDF - # Pass the original filename (UUID-based) so embed_metadata_into_pdf can find the file on disk + # Pass the filename (can be basename or full path) so embed_metadata_into_pdf can find the file on disk logger.info(f"[{task_id}] Queueing metadata embedding task") log_task_progress( task_id, "extract_metadata_with_gpt", "success", "Metadata extracted, queuing embed task", file_id=file_id ) embed_metadata_into_pdf.delay(filename, cleaned_text, metadata, file_id) - return {"s3_file": filename, "metadata": metadata} + return {"s3_file": os.path.basename(filename), "metadata": metadata} except Exception as e: logger.exception(f"[{task_id}] OpenAI classification failed for {filename}: {e}") diff --git a/tests/test_file_detail_endpoints.py b/tests/test_file_detail_endpoints.py index 1f02fb47..46580554 100644 --- a/tests/test_file_detail_endpoints.py +++ b/tests/test_file_detail_endpoints.py @@ -257,6 +257,49 @@ class TestSubtaskRetry: assert response.status_code == 400 assert "not found on disk" in response.json()["detail"].lower() + def test_retry_embed_metadata_with_processed_file(self, client: TestClient, db_session, sample_pdf_path, tmp_path): + """Test retrying embed_metadata_into_pdf when file is in processed directory.""" + mock_task = MagicMock() + mock_task.id = "embed-processed-retry-task" + + # Create processed directory and copy file there + processed_dir = tmp_path / "processed" + processed_dir.mkdir(exist_ok=True) + processed_file = processed_dir / "processed_doc.pdf" + + # Copy the sample PDF to processed directory + import shutil + shutil.copy(sample_pdf_path, processed_file) + + # Create file record with non-existent local_filename but existing processed_file_path + file_record = FileRecord( + filehash="pipeline_retry6", + original_filename="processed_doc.pdf", + local_filename="/nonexistent/tmp/doc.pdf", # File no longer in tmp + processed_file_path=str(processed_file), # But exists in processed + file_size=1024, + mime_type="application/pdf", + ) + db_session.add(file_record) + db_session.commit() + db_session.refresh(file_record) + + with patch("app.tasks.extract_metadata_with_gpt.extract_metadata_with_gpt") as mock_extract: + mock_extract.delay.return_value = mock_task + response = client.post(f"/api/files/{file_record.id}/retry-subtask?subtask_name=embed_metadata_into_pdf") + + # Should succeed because file exists in processed directory + assert response.status_code == 200 + data = response.json() + assert data["status"] == "success" + assert data["subtask_name"] == "embed_metadata_into_pdf" + # Verify extract_metadata_with_gpt.delay was called with full path + mock_extract.delay.assert_called_once() + call_args = mock_extract.delay.call_args + # First argument should be the full path to the processed file + assert str(processed_file) in str(call_args[0][0]) + + @pytest.mark.integration class TestFilePreview: From bcf5f652fca6ad53084e6c2b3be9e8d42addff1e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 21:07:23 +0000 Subject: [PATCH 3/4] style: fix linting issues in retry and metadata extraction code Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- app/api/files.py | 23 +++++++++++------------ app/tasks/extract_metadata_with_gpt.py | 2 +- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/app/api/files.py b/app/api/files.py index 526d9498..83875746 100644 --- a/app/api/files.py +++ b/app/api/files.py @@ -525,7 +525,7 @@ def _retry_pipeline_step(file_record: FileRecord, step_name: str, db: Session) - # Retrying embed requires re-running metadata extraction first, because # embed_metadata_into_pdf needs the actual metadata dict (not empty). # Re-trigger extract_metadata_with_gpt which will chain into embed_metadata_into_pdf. - + # Check for file in multiple locations: # 1. Original location in tmp (file_record.local_filename) # 2. Processed location (file_record.processed_file_path) @@ -536,21 +536,20 @@ def _retry_pipeline_step(file_record: FileRecord, step_name: str, db: Session) - elif file_record.processed_file_path and os.path.exists(file_record.processed_file_path): # File has been processed and moved to processed directory file_path = file_record.processed_file_path - else: - # Try fallback path in workdir/tmp - if file_record.local_filename: - workdir = settings.workdir - tmp_dir = os.path.join(workdir, "tmp") - fallback_path = os.path.join(tmp_dir, os.path.basename(file_record.local_filename)) - if os.path.exists(fallback_path): - file_path = fallback_path - + # Try fallback path in workdir/tmp + elif file_record.local_filename: + workdir = settings.workdir + tmp_dir = os.path.join(workdir, "tmp") + fallback_path = os.path.join(tmp_dir, os.path.basename(file_record.local_filename)) + if os.path.exists(fallback_path): + file_path = fallback_path + if not file_path: raise HTTPException( - status_code=400, + status_code=400, detail="File not found in tmp or processed directory. Cannot retry metadata embedding." ) - + extracted_text = _extract_text_from_pdf(file_path) # Pass the full path to the task so it can locate the file task = extract_metadata_task.delay(file_path, extracted_text, file_id) diff --git a/app/tasks/extract_metadata_with_gpt.py b/app/tasks/extract_metadata_with_gpt.py index 7c917a51..c9db5025 100644 --- a/app/tasks/extract_metadata_with_gpt.py +++ b/app/tasks/extract_metadata_with_gpt.py @@ -49,7 +49,7 @@ def extract_json_from_text(text): def extract_metadata_with_gpt(self, filename: str, cleaned_text: str, file_id: int = None): """ Uses OpenAI to classify document metadata. - + Args: filename: Can be either a basename (e.g., "file.pdf") or a full path (e.g., "/workdir/processed/file.pdf") cleaned_text: The extracted text from the document From d259d5843163100a2c327a8b06dd85bd9a78444c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 21:08:48 +0000 Subject: [PATCH 4/4] refactor: move shutil import to top of test file and fix whitespace Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- tests/test_file_detail_endpoints.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_file_detail_endpoints.py b/tests/test_file_detail_endpoints.py index 46580554..837d058d 100644 --- a/tests/test_file_detail_endpoints.py +++ b/tests/test_file_detail_endpoints.py @@ -2,6 +2,7 @@ Tests for file detail view improvements including reprocessing and preview endpoints. """ +import shutil from unittest.mock import MagicMock, patch import pytest @@ -266,11 +267,10 @@ class TestSubtaskRetry: processed_dir = tmp_path / "processed" processed_dir.mkdir(exist_ok=True) processed_file = processed_dir / "processed_doc.pdf" - + # Copy the sample PDF to processed directory - import shutil shutil.copy(sample_pdf_path, processed_file) - + # Create file record with non-existent local_filename but existing processed_file_path file_record = FileRecord( filehash="pipeline_retry6", @@ -287,7 +287,7 @@ class TestSubtaskRetry: with patch("app.tasks.extract_metadata_with_gpt.extract_metadata_with_gpt") as mock_extract: mock_extract.delay.return_value = mock_task response = client.post(f"/api/files/{file_record.id}/retry-subtask?subtask_name=embed_metadata_into_pdf") - + # Should succeed because file exists in processed directory assert response.status_code == 200 data = response.json()