From bb98afac826a955770744c93141e22a671f08d94 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 28 Feb 2026 12:56:01 +0000 Subject: [PATCH] fix(api): use processed_file_path from DB in upload retry path resolution The upload retry logic now checks file_record.processed_file_path first (the GPT-suggested filename stored during finalization), before falling back to legacy hash-based and original-filename-based path patterns. This fixes the case where the processed file has a different name than the original (e.g., '2023-10-01_Unknown.pdf' vs 'cable_graphic.pdf') and the retry couldn't find the file on disk. Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- app/api/files.py | 50 ++++++++++++------- tests/test_retry_subtask_logging.py | 75 +++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 17 deletions(-) diff --git a/app/api/files.py b/app/api/files.py index be341239..b40fbaf3 100644 --- a/app/api/files.py +++ b/app/api/files.py @@ -763,28 +763,44 @@ def retry_subtask( logger.info( f"Retrying upload task {subtask_name} for file {file_id}: " f"original_filename={file_record.original_filename!r}, " - f"filehash={file_record.filehash!r}" + f"filehash={file_record.filehash!r}, " + f"processed_file_path={file_record.processed_file_path!r}" ) - workdir = settings.workdir - processed_dir = os.path.join(workdir, "processed") - - # Try to find the processed file - base_filename = os.path.splitext(file_record.original_filename)[0] - potential_paths = [ - os.path.join(processed_dir, f"{file_record.filehash}.pdf"), - os.path.join(processed_dir, f"{base_filename}_processed.pdf"), - os.path.join(processed_dir, file_record.original_filename), - ] + # Try to find the processed file using multiple strategies: + # 1. DB-stored processed_file_path (most reliable - set during finalization) + # 2. Hash-based path in processed dir + # 3. Original filename with _processed suffix + # 4. Original filename in processed dir file_path = None checked_paths = [] - for path in potential_paths: - exists = os.path.exists(path) - checked_paths.append(f"{path!r} (exists={exists})") - logger.info(f"Checking processed file path: {path!r}, exists={exists}") + + # Check 1: processed_file_path from DB + if file_record.processed_file_path: + exists = os.path.exists(file_record.processed_file_path) + checked_paths.append(f"processed_file_path={file_record.processed_file_path!r} (exists={exists})") + logger.info(f"Checking processed_file_path: {file_record.processed_file_path!r}, exists={exists}") if exists: - file_path = path - break + file_path = file_record.processed_file_path + + # Check 2-4: Legacy path patterns in processed directory + if not file_path: + workdir = settings.workdir + processed_dir = os.path.join(workdir, "processed") + base_filename = os.path.splitext(file_record.original_filename)[0] + legacy_paths = [ + os.path.join(processed_dir, f"{file_record.filehash}.pdf"), + os.path.join(processed_dir, f"{base_filename}_processed.pdf"), + os.path.join(processed_dir, file_record.original_filename), + ] + + for path in legacy_paths: + exists = os.path.exists(path) + checked_paths.append(f"{path!r} (exists={exists})") + logger.info(f"Checking processed file path: {path!r}, exists={exists}") + if exists: + file_path = path + break if not file_path: paths_detail = "; ".join(checked_paths) diff --git a/tests/test_retry_subtask_logging.py b/tests/test_retry_subtask_logging.py index 321541dd..7b8bbcb4 100644 --- a/tests/test_retry_subtask_logging.py +++ b/tests/test_retry_subtask_logging.py @@ -168,6 +168,7 @@ class TestRetrySubtaskEnhancedLogging: filehash="upload_error_test", original_filename="missing.pdf", local_filename="/nonexistent/tmp/missing.pdf", + processed_file_path="/nonexistent/processed/missing_gpt.pdf", file_size=1024, mime_type="application/pdf", ) @@ -183,8 +184,82 @@ class TestRetrySubtaskEnhancedLogging: # Verify the error message contains diagnostic information assert "Cannot retry upload" in error_detail assert "Paths checked:" in error_detail + assert "processed_file_path" in error_detail assert "exists=False" in error_detail + def test_upload_retry_uses_processed_file_path_from_db( + self, client: TestClient, db_session, sample_pdf_path, tmp_path + ): + """Test that upload retry uses processed_file_path from DB when legacy paths don't exist.""" + mock_task = MagicMock() + mock_task.id = "upload-processed-path-task" + + # Create processed file at the DB-stored path (GPT-suggested filename) + processed_file = tmp_path / "2023-10-01_Unknown.pdf" + shutil.copy(sample_pdf_path, processed_file) + + # Create file record where only processed_file_path exists + # (simulates the real scenario: original filename != GPT-suggested filename) + file_record = FileRecord( + filehash="upload_db_path_test", + original_filename="cable_graphic.pdf", + local_filename="/nonexistent/tmp/uuid.pdf", + processed_file_path=str(processed_file), + file_size=1024, + mime_type="application/pdf", + ) + db_session.add(file_record) + db_session.commit() + db_session.refresh(file_record) + + with patch("app.tasks.upload_to_onedrive.upload_to_onedrive") as mock_upload: + mock_upload.delay.return_value = mock_task + response = client.post(f"/api/files/{file_record.id}/retry-subtask?subtask_name=upload_to_onedrive") + + assert response.status_code == 200 + data = response.json() + assert data["status"] == "success" + assert data["subtask_name"] == "upload_to_onedrive" + + # Verify the task was called with the processed_file_path from DB + mock_upload.delay.assert_called_once() + call_args = mock_upload.delay.call_args + assert call_args[0][0] == str(processed_file) + + def test_upload_retry_processed_file_path_takes_priority( + self, client: TestClient, db_session, sample_pdf_path, tmp_path + ): + """Test that processed_file_path from DB takes priority over legacy hash-based paths.""" + mock_task = MagicMock() + mock_task.id = "upload-priority-task" + + # Create the DB-stored processed file + processed_file = tmp_path / "2024-01-01_Invoice.pdf" + shutil.copy(sample_pdf_path, processed_file) + + file_record = FileRecord( + filehash="upload_priority_test", + original_filename="scan001.pdf", + local_filename="/nonexistent/tmp/uuid.pdf", + processed_file_path=str(processed_file), + file_size=1024, + mime_type="application/pdf", + ) + db_session.add(file_record) + db_session.commit() + db_session.refresh(file_record) + + with patch("app.tasks.upload_to_dropbox.upload_to_dropbox") as mock_upload: + mock_upload.delay.return_value = mock_task + response = client.post(f"/api/files/{file_record.id}/retry-subtask?subtask_name=upload_to_dropbox") + + assert response.status_code == 200 + + # Verify the task was called with the processed_file_path (first priority for uploads) + mock_upload.delay.assert_called_once() + call_args = mock_upload.delay.call_args + assert call_args[0][0] == str(processed_file) + def test_embed_metadata_path_order(self, client: TestClient, db_session, sample_pdf_path, tmp_path): """Test that embed_metadata_into_pdf checks paths in the correct order.""" mock_task = MagicMock()