Address copilot issues

This commit is contained in:
Reece Browne 2025-08-21 11:39:07 +01:00
parent c8b3db295c
commit 3afbf8fb4d
6 changed files with 78 additions and 36 deletions

View File

@ -19,6 +19,40 @@ import { buildQuickKeySet, buildQuickKeySetFromMetadata } from './fileSelectors'
const DEBUG = process.env.NODE_ENV === 'development'; const DEBUG = process.env.NODE_ENV === 'development';
/**
* Simple mutex to prevent race conditions in addFiles
*/
class SimpleMutex {
private locked = false;
private queue: Array<() => void> = [];
async lock(): Promise<void> {
if (!this.locked) {
this.locked = true;
return Promise.resolve();
}
return new Promise<void>((resolve) => {
this.queue.push(() => {
this.locked = true;
resolve();
});
});
}
unlock(): void {
if (this.queue.length > 0) {
const next = this.queue.shift()!;
next();
} else {
this.locked = false;
}
}
}
// Global mutex for addFiles operations
const addFilesMutex = new SimpleMutex();
/** /**
* Helper to create ProcessedFile metadata structure * Helper to create ProcessedFile metadata structure
*/ */
@ -63,6 +97,10 @@ export async function addFiles(
dispatch: React.Dispatch<FileContextAction>, dispatch: React.Dispatch<FileContextAction>,
lifecycleManager: FileLifecycleManager lifecycleManager: FileLifecycleManager
): Promise<Array<{ file: File; id: FileId; thumbnail?: string }>> { ): Promise<Array<{ file: File; id: FileId; thumbnail?: string }>> {
// Acquire mutex to prevent race conditions
await addFilesMutex.lock();
try {
const fileRecords: FileRecord[] = []; const fileRecords: FileRecord[] = [];
const addedFiles: Array<{ file: File; id: FileId; thumbnail?: string }> = []; const addedFiles: Array<{ file: File; id: FileId; thumbnail?: string }> = [];
@ -99,7 +137,7 @@ export async function addFiles(
const result = await generateThumbnailWithMetadata(file); const result = await generateThumbnailWithMetadata(file);
thumbnail = result.thumbnail; thumbnail = result.thumbnail;
pageCount = result.pageCount; pageCount = result.pageCount;
if (DEBUG) console.log(`📄 Generated PDF metadata for ${file.name}: ${pageCount} pages, thumbnail: ${!!thumbnail}`); if (DEBUG) console.log(`📄 Generated PDF metadata for ${file.name}: ${pageCount} pages, thumbnail: SUCCESS`);
} catch (error) { } catch (error) {
if (DEBUG) console.warn(`📄 Failed to generate PDF metadata for ${file.name}:`, error); if (DEBUG) console.warn(`📄 Failed to generate PDF metadata for ${file.name}:`, error);
} }
@ -110,7 +148,7 @@ export async function addFiles(
const { generateThumbnailForFile } = await import('../../utils/thumbnailUtils'); const { generateThumbnailForFile } = await import('../../utils/thumbnailUtils');
thumbnail = await generateThumbnailForFile(file); thumbnail = await generateThumbnailForFile(file);
pageCount = 0; // Non-PDFs have no page count pageCount = 0; // Non-PDFs have no page count
if (DEBUG) console.log(`📄 Generated simple thumbnail for ${file.name}: no page count, thumbnail: ${!!thumbnail}`); if (DEBUG) console.log(`📄 Generated simple thumbnail for ${file.name}: no page count, thumbnail: SUCCESS`);
} catch (error) { } catch (error) {
if (DEBUG) console.warn(`📄 Failed to generate simple thumbnail for ${file.name}:`, error); if (DEBUG) console.warn(`📄 Failed to generate simple thumbnail for ${file.name}:`, error);
} }
@ -255,6 +293,10 @@ export async function addFiles(
} }
return addedFiles; return addedFiles;
} finally {
// Always release mutex even if error occurs
addFilesMutex.unlock();
}
} }
/** /**

View File

@ -50,13 +50,8 @@ export const useToolResources = () => {
try { try {
console.log(`🖼️ Generating thumbnail for: ${file.name} (${file.type}, ${file.size} bytes)`); console.log(`🖼️ Generating thumbnail for: ${file.name} (${file.type}, ${file.size} bytes)`);
const thumbnail = await generateThumbnailForFile(file); const thumbnail = await generateThumbnailForFile(file);
console.log(`🖼️ Generated thumbnail for ${file.name}:`, thumbnail ? 'SUCCESS' : 'FAILED (no thumbnail returned)'); console.log(`🖼️ Generated thumbnail for ${file.name}: SUCCESS`);
if (thumbnail) {
thumbnails.push(thumbnail); thumbnails.push(thumbnail);
} else {
console.warn(`🖼️ No thumbnail returned for ${file.name}`);
thumbnails.push('');
}
} catch (error) { } catch (error) {
console.warn(`🖼️ Failed to generate thumbnail for ${file.name}:`, error); console.warn(`🖼️ Failed to generate thumbnail for ${file.name}:`, error);
thumbnails.push(''); thumbnails.push('');
@ -74,9 +69,7 @@ export const useToolResources = () => {
try { try {
console.log(`🖼️ Generating thumbnail with metadata for: ${file.name} (${file.type}, ${file.size} bytes)`); console.log(`🖼️ Generating thumbnail with metadata for: ${file.name} (${file.type}, ${file.size} bytes)`);
const result = await generateThumbnailWithMetadata(file); const result = await generateThumbnailWithMetadata(file);
console.log(`🖼️ Generated thumbnail with metadata for ${file.name}:`, console.log(`🖼️ Generated thumbnail with metadata for ${file.name}: SUCCESS, ${result.pageCount} pages`);
result.thumbnail ? 'SUCCESS' : 'FAILED (no thumbnail)',
`${result.pageCount} pages`);
results.push(result); results.push(result);
} catch (error) { } catch (error) {
console.warn(`🖼️ Failed to generate thumbnail with metadata for ${file.name}:`, error); console.warn(`🖼️ Failed to generate thumbnail with metadata for ${file.name}:`, error);
@ -84,7 +77,7 @@ export const useToolResources = () => {
} }
} }
console.log(`🖼️ useToolResources.generateThumbnailsWithMetadata: Complete. Generated ${results.filter(r => r.thumbnail).length}/${files.length} thumbnails with metadata`); console.log(`🖼️ useToolResources.generateThumbnailsWithMetadata: Complete. Generated ${results.length}/${files.length} thumbnails with metadata`);
return results; return results;
}, []); }, []);

View File

@ -64,10 +64,8 @@ export function useIndexedDBThumbnail(file: FileMetadata | undefined | null): {
// Use the universal thumbnail generator // Use the universal thumbnail generator
const thumbnail = await generateThumbnailForFile(fileObject); const thumbnail = await generateThumbnailForFile(fileObject);
if (!cancelled && thumbnail) { if (!cancelled) {
setThumb(thumbnail); setThumb(thumbnail);
} else if (!cancelled) {
setThumb(null);
} }
} catch (error) { } catch (error) {
console.warn('Failed to generate thumbnail for file', file.name, error); console.warn('Failed to generate thumbnail for file', file.name, error);

View File

@ -107,18 +107,8 @@ class FileProcessingService {
throw new Error('Processing cancelled'); throw new Error('Processing cancelled');
} }
} catch (pdfError) { } catch (pdfError) {
console.warn(`📁 FileProcessingService: PDF.js failed for ${file.name}, trying fallback:`, pdfError); console.warn(`📁 FileProcessingService: PDF.js failed for ${file.name}, setting pages to 0:`, pdfError);
totalPages = 0; // Unknown page count - UI will hide page count display
// Fallback to text analysis (reuse same arrayBuffer)
try {
const text = new TextDecoder('latin1').decode(arrayBuffer);
const pageMatches = text.match(/\/Type\s*\/Page[^s]/g);
totalPages = pageMatches ? pageMatches.length : 1;
console.log(`📁 FileProcessingService: Text analysis discovered ${totalPages} pages for ${file.name}`);
} catch (textError) {
console.warn(`📁 FileProcessingService: Text analysis also failed for ${file.name}:`, textError);
totalPages = 1;
}
} }
} }

View File

@ -117,6 +117,19 @@ export class ThumbnailGenerationService {
options: ThumbnailGenerationOptions = {}, options: ThumbnailGenerationOptions = {},
onProgress?: (progress: { completed: number; total: number; thumbnails: ThumbnailResult[] }) => void onProgress?: (progress: { completed: number; total: number; thumbnails: ThumbnailResult[] }) => void
): Promise<ThumbnailResult[]> { ): Promise<ThumbnailResult[]> {
// Input validation
if (!fileId || typeof fileId !== 'string' || fileId.trim() === '') {
throw new Error('generateThumbnails: fileId must be a non-empty string');
}
if (!pdfArrayBuffer || pdfArrayBuffer.byteLength === 0) {
throw new Error('generateThumbnails: pdfArrayBuffer must not be empty');
}
if (!pageNumbers || pageNumbers.length === 0) {
throw new Error('generateThumbnails: pageNumbers must not be empty');
}
const { const {
scale = 0.2, scale = 0.2,
quality = 0.8 quality = 0.8

View File

@ -1,7 +1,7 @@
import { pdfWorkerManager } from '../services/pdfWorkerManager'; import { pdfWorkerManager } from '../services/pdfWorkerManager';
export interface ThumbnailWithMetadata { export interface ThumbnailWithMetadata {
thumbnail: string | undefined; thumbnail: string; // Always returns a thumbnail (placeholder if needed)
pageCount: number; pageCount: number;
} }
@ -325,9 +325,9 @@ async function generatePDFThumbnail(arrayBuffer: ArrayBuffer, file: File, scale:
} }
/** /**
* Generate thumbnail for any file type * Generate thumbnail for any file type - always returns a thumbnail (placeholder if needed)
*/ */
export async function generateThumbnailForFile(file: File): Promise<string | undefined> { export async function generateThumbnailForFile(file: File): Promise<string> {
// Skip very large files // Skip very large files
if (file.size >= 100 * 1024 * 1024) { if (file.size >= 100 * 1024 * 1024) {
return generatePlaceholderThumbnail(file); return generatePlaceholderThumbnail(file);
@ -351,11 +351,17 @@ export async function generateThumbnailForFile(file: File): Promise<string | und
return await generatePDFThumbnail(arrayBuffer, file, scale); return await generatePDFThumbnail(arrayBuffer, file, scale);
} catch (error) { } catch (error) {
if (error instanceof Error && error.name === 'InvalidPDFException') { if (error instanceof Error && error.name === 'InvalidPDFException') {
console.warn(`PDF structure issue for ${file.name} - using fallback thumbnail`); console.warn(`PDF structure issue for ${file.name} - trying with full file`);
try {
// Try with full file instead of chunk // Try with full file instead of chunk
const fullArrayBuffer = await file.arrayBuffer(); const fullArrayBuffer = await file.arrayBuffer();
return await generatePDFThumbnail(fullArrayBuffer, file, scale); return await generatePDFThumbnail(fullArrayBuffer, file, scale);
} catch (fullFileError) {
console.warn(`Full file PDF processing also failed for ${file.name} - using placeholder`);
return generatePlaceholderThumbnail(file);
} }
}
console.warn(`PDF processing failed for ${file.name} - using placeholder:`, error);
return generatePlaceholderThumbnail(file); return generatePlaceholderThumbnail(file);
} }
} }
@ -365,7 +371,7 @@ export async function generateThumbnailForFile(file: File): Promise<string | und
} }
/** /**
* Generate thumbnail and extract page count for a PDF file * Generate thumbnail and extract page count for a PDF file - always returns a valid thumbnail
*/ */
export async function generateThumbnailWithMetadata(file: File): Promise<ThumbnailWithMetadata> { export async function generateThumbnailWithMetadata(file: File): Promise<ThumbnailWithMetadata> {
// Non-PDF files have no page count // Non-PDF files have no page count