From 7ff1c66d09f88a7acbbedcb36436b79fb4391773 Mon Sep 17 00:00:00 2001 From: James Brunton Date: Wed, 17 Sep 2025 15:11:36 +0100 Subject: [PATCH] Remove custom response handler from Merge (#4457) # Description of Changes Remove custom response handler from Merge. Also make `filePrefix` mandatory for `multiFile` tools to make the output more visually different since you get a new 'V1' file rather than 'V2' of the current file. --- CLAUDE.md | 10 ++++------ .../tools/merge/useMergeOperation.test.ts | 18 ------------------ .../src/hooks/tools/merge/useMergeOperation.ts | 8 +------- .../src/hooks/tools/shared/useToolOperation.ts | 14 ++++++++++---- 4 files changed, 15 insertions(+), 35 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index a806d0098..bc6af38c9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -39,7 +39,7 @@ Frontend designed for **stateful document processing**: #### FileContext - Central State Management **Location**: `src/contexts/FileContext.tsx` - **Active files**: Currently loaded PDFs and their variants -- **Tool navigation**: Current mode (viewer/pageEditor/fileEditor/toolName) +- **Tool navigation**: Current mode (viewer/pageEditor/fileEditor/toolName) - **Memory management**: PDF document cleanup, blob URL lifecycle, Web Worker management - **IndexedDB persistence**: File storage with thumbnail caching - **Preview system**: Tools can preview results (e.g., Split → Viewer → back to Split) without context pollution @@ -89,7 +89,6 @@ return useToolOperation({ endpoint: '/api/v1/misc/compress-pdf', buildFormData: (params, file: File) => { /* single file */ }, multiFileEndpoint: false, - filePrefix: 'compressed_' }); ``` @@ -103,7 +102,7 @@ return useToolOperation({ endpoint: '/api/v1/general/split-pages', buildFormData: (params, files: File[]) => { /* all files */ }, multiFileEndpoint: true, - filePrefix: 'split_' + filePrefix: 'split_', }); ``` @@ -115,13 +114,12 @@ return useToolOperation({ return useToolOperation({ operationType: 'convert', customProcessor: async (params, files) => { /* custom logic */ }, - filePrefix: 'converted_' }); ``` **Benefits**: - **No Timeouts**: Operations run until completion (supports 100GB+ files) -- **Consistent**: All tools follow same pattern and interface +- **Consistent**: All tools follow same pattern and interface - **Maintainable**: Single responsibility hooks, easy to test and modify - **i18n Ready**: Built-in internationalization support - **Type Safe**: Full TypeScript support with generic interfaces @@ -185,7 +183,7 @@ return useToolOperation({ ## Frontend Architecture Status - **Core Status**: React SPA architecture complete with multi-tool workflow support -- **State Management**: FileContext handles all file operations and tool navigation +- **State Management**: FileContext handles all file operations and tool navigation - **File Processing**: Production-ready with memory management for large PDF workflows (up to 100GB+) - **Tool Integration**: Modular hook architecture with `useToolOperation` orchestrator - Individual hooks: `useToolState`, `useToolApiCalls`, `useToolResources` diff --git a/frontend/src/hooks/tools/merge/useMergeOperation.test.ts b/frontend/src/hooks/tools/merge/useMergeOperation.test.ts index f811febcf..4d41c5162 100644 --- a/frontend/src/hooks/tools/merge/useMergeOperation.test.ts +++ b/frontend/src/hooks/tools/merge/useMergeOperation.test.ts @@ -81,24 +81,6 @@ describe('useMergeOperation', () => { expect(formData.get('generateToc')).toBe('false'); }); - test('should handle response correctly', () => { - renderHook(() => useMergeOperation()); - - const config = getToolConfig(); - const mockBlob = new Blob(['merged content'], { type: 'application/pdf' }); - const mockFiles = [ - new File(['content1'], 'file1.pdf', { type: 'application/pdf' }), - new File(['content2'], 'file2.pdf', { type: 'application/pdf' }) - ]; - - const result = config.responseHandler!(mockBlob, mockFiles) as File[]; - - expect(result).toHaveLength(1); - expect(result[0].name).toBe('merged_file1.pdf'); - expect(result[0].type).toBe('application/pdf'); - expect(result[0].size).toBe(mockBlob.size); - }); - test('should return the hook result from useToolOperation', () => { const { result } = renderHook(() => useMergeOperation()); diff --git a/frontend/src/hooks/tools/merge/useMergeOperation.ts b/frontend/src/hooks/tools/merge/useMergeOperation.ts index a356be443..ea630ea0d 100644 --- a/frontend/src/hooks/tools/merge/useMergeOperation.ts +++ b/frontend/src/hooks/tools/merge/useMergeOperation.ts @@ -1,5 +1,5 @@ import { useTranslation } from 'react-i18next'; -import { useToolOperation, ResponseHandler, ToolOperationConfig, ToolType } from '../shared/useToolOperation'; +import { useToolOperation, ToolOperationConfig, ToolType } from '../shared/useToolOperation'; import { createStandardErrorHandler } from '../../../utils/toolErrorHandler'; import { MergeParameters } from './useMergeParameters'; @@ -16,11 +16,6 @@ const buildFormData = (parameters: MergeParameters, files: File[]): FormData => return formData; }; -const mergeResponseHandler: ResponseHandler = (blob: Blob, originalFiles: File[]): File[] => { - const filename = `merged_${originalFiles[0].name}` - return [new File([blob], filename, { type: 'application/pdf' })]; -}; - // Operation configuration for automation export const mergeOperationConfig: ToolOperationConfig = { toolType: ToolType.multiFile, @@ -28,7 +23,6 @@ export const mergeOperationConfig: ToolOperationConfig = { operationType: 'merge', endpoint: '/api/v1/general/merge-pdfs', filePrefix: 'merged_', - responseHandler: mergeResponseHandler, }; export const useMergeOperation = () => { diff --git a/frontend/src/hooks/tools/shared/useToolOperation.ts b/frontend/src/hooks/tools/shared/useToolOperation.ts index b50d1c175..0b7ba75b6 100644 --- a/frontend/src/hooks/tools/shared/useToolOperation.ts +++ b/frontend/src/hooks/tools/shared/useToolOperation.ts @@ -31,7 +31,10 @@ interface BaseToolOperationConfig { /** Operation identifier for tracking and logging */ operationType: string; - /** Prefix added to processed filenames (e.g., 'compressed_', 'split_') */ + /** + * Prefix added to processed filenames (e.g., 'compressed_', 'split_'). + * Only generally useful for multiFile interfaces. + */ filePrefix?: string; /** @@ -68,6 +71,9 @@ export interface MultiFileToolOperationConfig extends BaseToolOperation /** This tool processes multiple files at once. */ toolType: ToolType.multiFile; + /** Prefix added to processed filename (e.g., 'merged_', 'split_') */ + filePrefix: string; + /** Builds FormData for API request. */ buildFormData: ((params: TParams, files: File[]) => FormData); @@ -214,9 +220,9 @@ export const useToolOperation = ( processedFiles = await config.responseHandler(response.data, filesForAPI); } else if (response.data.type === 'application/pdf' || (response.headers && response.headers['content-type'] === 'application/pdf')) { - // Single PDF response (e.g. split with merge option) - use original filename - const originalFileName = filesForAPI[0]?.name || 'document.pdf'; - const singleFile = new File([response.data], originalFileName, { type: 'application/pdf' }); + // Single PDF response (e.g. split with merge option) - add prefix to first original filename + const filename = `${config.filePrefix}${filesForAPI[0]?.name || 'document.pdf'}`; + const singleFile = new File([response.data], filename, { type: 'application/pdf' }); processedFiles = [singleFile]; } else { // Default: assume ZIP response for multi-file endpoints