From 84eb518343fa83110d57b25eb02d81741dc908e0 Mon Sep 17 00:00:00 2001 From: Levy Tate <78818969+iLevyTate@users.noreply.github.com> Date: Wed, 11 Nov 2026 20:01:34 -0500 Subject: [PATCH] fix(discover): reanalyze files where they are, not through the smart-folder watcher Reanalyze on a Discover file card and "Reanalyze N" in Organize sent the file to SmartFolderWatcher.reanalyzeFile, which refuses any file outside a watched Smart Folder. Discover's files are source files, so the action always ended in "This file is not inside a configured Smart Folder". Both controls now re-run the Discover analysis run for those paths with bypassCache, so the analyzers skip the cached result: - analysis IPC: the single-file analyze payload accepts bypassCache and passes it to the document, image and media analyzers - preload: files.analyze forwards bypassCache - useAnalysis: reanalyzeFiles(paths) runs the per-file path with the cache bypassed, does not auto-advance to Organize, and reports reanalysis results in its own words; refused while a run is active - Organize: the button is disabled while an analysis runs The watcher's embeddings.reanalyzeFile route is left in place; it no longer has a renderer caller. --- src/main/ipc/analysis.js | 63 ++++++--- src/preload/preload.js | 15 +- .../organize/UncategorizedActionsBar.jsx | 84 +++-------- src/renderer/phases/DiscoverPhase.jsx | 4 +- src/renderer/phases/OrganizePhase.jsx | 4 + .../phases/discover/AnalysisRunProvider.jsx | 3 + src/renderer/phases/discover/useAnalysis.js | 118 +++++++++++++--- .../phases/discover/useFileActions.js | 43 ++---- test/analysisIpc.realZod.test.js | 44 ++++++ test/components/OrganizePhase.test.js | 4 + .../UncategorizedActionsBar.test.js | 34 ++++- test/preload.coverage.test.js | 22 +++ test/useAnalysis.test.js | 133 ++++++++++++++++++ test/useFileActions.test.js | 29 ++++ 14 files changed, 463 insertions(+), 137 deletions(-) diff --git a/src/main/ipc/analysis.js b/src/main/ipc/analysis.js index bcf38b0..4a8ae57 100644 --- a/src/main/ipc/analysis.js +++ b/src/main/ipc/analysis.js @@ -292,7 +292,9 @@ function registerAnalysisIpc(servicesOrParams) { stringSchema, z.object({ filePath: z.string().min(1).max(4096), - namingSettings: namingSettingsSchema.optional() + namingSettings: namingSettingsSchema.optional(), + // A reanalysis the user asked for: skip the cached result. + bypassCache: z.boolean().optional() }) ]) : { @@ -316,15 +318,16 @@ function registerAnalysisIpc(servicesOrParams) { /** @param {any} payload */ function normalizeAnalyzeFilePayload(payload) { if (typeof payload === 'string') { - return { filePath: payload, namingOverride: null }; + return { filePath: payload, namingOverride: null, bypassCache: false }; } if (payload && typeof payload === 'object') { return { filePath: payload.filePath, - namingOverride: payload.namingSettings || null + namingOverride: payload.namingSettings || null, + bypassCache: payload.bypassCache === true }; } - return { filePath: '', namingOverride: null }; + return { filePath: '', namingOverride: null, bypassCache: false }; } // Defense-in-depth path refine for batch entries: the preload validator // rejects UNC/URL/etc, but a compromised renderer could bypass the preload @@ -513,8 +516,14 @@ function registerAnalysisIpc(servicesOrParams) { * @param {string} filePath * @param {AbortSignal|undefined} abortSignal * @param {Record|null} [namingOverride] + * @param {boolean} [bypassCache] */ - async function performDocumentAnalysis(filePath, abortSignal, namingOverride = null) { + async function performDocumentAnalysis( + filePath, + abortSignal, + namingOverride = null, + bypassCache = false + ) { const serviceIntegration = getServiceIntegration?.(); const cleanPath = await validateAnalysisPath(filePath); @@ -551,7 +560,10 @@ function registerAnalysisIpc(servicesOrParams) { ); const statsBefore = await statBeforeAnalysis(cleanPath); - const result = await analyzeDocumentFile(cleanPath, folderCategories, { abortSignal }); + const result = await analyzeDocumentFile(cleanPath, folderCategories, { + abortSignal, + bypassCache + }); await applyUserNamingFromSettings( cleanPath, result, @@ -622,9 +634,9 @@ function registerAnalysisIpc(servicesOrParams) { context: 'Analysis', schema: analyzeFilePayloadSchema, handler: (event, payload) => { - const { filePath, namingOverride } = normalizeAnalyzeFilePayload(payload); + const { filePath, namingOverride, bypassCache } = normalizeAnalyzeFilePayload(payload); return withUserAnalysis(filePath, () => - performDocumentAnalysis(filePath, event?.abortSignal, namingOverride) + performDocumentAnalysis(filePath, event?.abortSignal, namingOverride, bypassCache) ); } }); @@ -638,8 +650,14 @@ function registerAnalysisIpc(servicesOrParams) { * @param {string} filePath * @param {AbortSignal|undefined} abortSignal * @param {Record|null} [namingOverride] + * @param {boolean} [bypassCache] */ - async function performImageAnalysis(filePath, abortSignal, namingOverride = null) { + async function performImageAnalysis( + filePath, + abortSignal, + namingOverride = null, + bypassCache = false + ) { const serviceIntegration = getServiceIntegration?.(); const cleanPath = await validateAnalysisPath(filePath); @@ -676,7 +694,10 @@ function registerAnalysisIpc(servicesOrParams) { ); const statsBefore = await statBeforeAnalysis(cleanPath); - const result = await analyzeImageFile(cleanPath, folderCategories, { abortSignal }); + const result = await analyzeImageFile(cleanPath, folderCategories, { + abortSignal, + bypassCache + }); await applyUserNamingFromSettings( cleanPath, result, @@ -747,9 +768,9 @@ function registerAnalysisIpc(servicesOrParams) { context: 'Analysis', schema: analyzeFilePayloadSchema, handler: (event, payload) => { - const { filePath, namingOverride } = normalizeAnalyzeFilePayload(payload); + const { filePath, namingOverride, bypassCache } = normalizeAnalyzeFilePayload(payload); return withUserAnalysis(filePath, () => - performImageAnalysis(filePath, event?.abortSignal, namingOverride) + performImageAnalysis(filePath, event?.abortSignal, namingOverride, bypassCache) ); } }); @@ -766,8 +787,14 @@ function registerAnalysisIpc(servicesOrParams) { * @param {string} filePath * @param {AbortSignal|undefined} abortSignal * @param {Record|null} [namingOverride] + * @param {boolean} [bypassCache] */ - async function performMediaAnalysis(filePath, abortSignal, namingOverride = null) { + async function performMediaAnalysis( + filePath, + abortSignal, + namingOverride = null, + bypassCache = false + ) { const serviceIntegration = getServiceIntegration?.(); const cleanPath = await validateAnalysisPath(filePath); @@ -807,7 +834,11 @@ function registerAnalysisIpc(servicesOrParams) { ); const isCancelled = () => Boolean(abortSignal?.aborted); const statsBefore = await statBeforeAnalysis(cleanPath); - const result = await analyzer(cleanPath, folderCategories, { isCancelled, abortSignal }); + const result = await analyzer(cleanPath, folderCategories, { + isCancelled, + abortSignal, + bypassCache + }); await applyUserNamingFromSettings( cleanPath, result, @@ -878,9 +909,9 @@ function registerAnalysisIpc(servicesOrParams) { context: 'Analysis', schema: analyzeFilePayloadSchema, handler: (event, payload) => { - const { filePath, namingOverride } = normalizeAnalyzeFilePayload(payload); + const { filePath, namingOverride, bypassCache } = normalizeAnalyzeFilePayload(payload); return withUserAnalysis(filePath, () => - performMediaAnalysis(filePath, event?.abortSignal, namingOverride) + performMediaAnalysis(filePath, event?.abortSignal, namingOverride, bypassCache) ); } }); diff --git a/src/preload/preload.js b/src/preload/preload.js index a91e30a..c836b3a 100644 --- a/src/preload/preload.js +++ b/src/preload/preload.js @@ -1781,9 +1781,20 @@ contextBridge.exposeInMainWorld('electronAPI', { let ext = ''; if (lastDot > lastSlash && lastDot > 0) ext = filePath.slice(lastDot + 1).toLowerCase(); - const payload = + const namingSettings = options?.namingSettings && typeof options.namingSettings === 'object' - ? { filePath, namingSettings: options.namingSettings } + ? options.namingSettings + : null; + // bypassCache: a reanalysis the user asked for must not return the + // cached result of the analysis it replaces. + const bypassCache = options?.bypassCache === true; + const payload = + namingSettings || bypassCache + ? { + filePath, + ...(namingSettings ? { namingSettings } : {}), + ...(bypassCache ? { bypassCache: true } : {}) + } : filePath; // Fix 1B: Use IMAGE_EXT_SET derived from constants.js instead of hardcoded list diff --git a/src/renderer/components/organize/UncategorizedActionsBar.jsx b/src/renderer/components/organize/UncategorizedActionsBar.jsx index ee6352d..015bb75 100644 --- a/src/renderer/components/organize/UncategorizedActionsBar.jsx +++ b/src/renderer/components/organize/UncategorizedActionsBar.jsx @@ -75,11 +75,12 @@ function UncategorizedActionsBar({ defaultLocation, existingFolders, onFolderCreated, + onReanalyze, + isAnalyzing = false, addNotification }) { const [isModalOpen, setIsModalOpen] = useState(false); const [prefillName, setPrefillName] = useState(''); - const [isReanalyzing, setIsReanalyzing] = useState(false); const earlierFiles = useEarlierUncategorizedFiles(); // Derive uncategorized files + pattern suggestions in one pass. @@ -165,68 +166,16 @@ function UncategorizedActionsBar({ [addNotification, onFolderCreated] ); - const handleReanalyzeAll = useCallback(async () => { - if (!window?.electronAPI?.embeddings?.reanalyzeFile) { + // The analysis run does the work and reports the outcome. The button is + // disabled while any analysis runs: the run refuses a reanalysis then. + const handleReanalyzeAll = useCallback(() => { + if (typeof onReanalyze !== 'function') { addNotification?.('Reanalyze is not available in this build.', 'warning'); return; } - if (uncategorizedFiles.length === 0) return; - - setIsReanalyzing(true); - let succeeded = 0; - let outOfScope = 0; - let otherFailed = 0; - try { - // Sequential rather than parallel: the embedding queue serializes - // anyway, and parallel calls just queue depth without finishing faster. - // Sequential also keeps the notification count predictable. - for (const file of uncategorizedFiles) { - if (!file?.path) { - otherFailed += 1; - continue; - } - try { - const result = await window.electronAPI.embeddings.reanalyzeFile(file.path); - if (result?.success !== false) { - succeeded += 1; - } else if (result?.errorCode === 'NOT_IN_SMART_FOLDER') { - outOfScope += 1; - } else { - otherFailed += 1; - } - } catch (err) { - logger.warn('Reanalyze single file failed', { - path: file.path, - error: err?.message - }); - otherFailed += 1; - } - } - // Tailor the toast to what actually happened so users understand the - // scope guard: out-of-scope rejections need a different remediation - // (add the source folder as a Smart Folder) than transient failures. - if (succeeded > 0 && outOfScope === 0 && otherFailed === 0) { - addNotification?.( - `Reanalyzed ${succeeded} ${succeeded === 1 ? 'file' : 'files'}.`, - 'success' - ); - } else if (outOfScope > 0 && otherFailed === 0) { - const skipNote = `${outOfScope} skipped (not inside a configured Smart Folder). Add the source folder as a Smart Folder destination to enable reanalysis.`; - addNotification?.( - succeeded > 0 ? `Reanalyzed ${succeeded}. ${skipNote}` : skipNote, - 'warning' - ); - } else { - const parts = []; - if (succeeded > 0) parts.push(`Reanalyzed ${succeeded}`); - if (outOfScope > 0) parts.push(`${outOfScope} not in a Smart Folder`); - if (otherFailed > 0) parts.push(`${otherFailed} failed`); - addNotification?.(parts.join(', ') + '.', 'warning'); - } - } finally { - setIsReanalyzing(false); - } - }, [uncategorizedFiles, addNotification]); + const paths = uncategorizedFiles.map((file) => file?.path).filter(Boolean); + if (paths.length > 0) onReanalyze(paths); + }, [uncategorizedFiles, onReanalyze, addNotification]); if (uncategorizedFiles.length === 0) return null; @@ -288,15 +237,11 @@ function UncategorizedActionsBar({ variant="ghost" size="sm" onClick={handleReanalyzeAll} - disabled={isReanalyzing} - aria-label={ - isReanalyzing - ? 'Reanalyzing…' - : `Reanalyze ${uncategorizedFiles.length} uncategorized files` - } + disabled={isAnalyzing} + aria-label={`Reanalyze ${uncategorizedFiles.length} uncategorized files`} > - - {isReanalyzing ? 'Reanalyzing…' : `Reanalyze ${uncategorizedFiles.length}`} + + Reanalyze {uncategorizedFiles.length} @@ -326,6 +271,9 @@ UncategorizedActionsBar.propTypes = { defaultLocation: PropTypes.string, existingFolders: PropTypes.array, onFolderCreated: PropTypes.func, + // Analyzes the given paths again (the analysis run's reanalyzeFiles). + onReanalyze: PropTypes.func, + isAnalyzing: PropTypes.bool, addNotification: PropTypes.func }; diff --git a/src/renderer/phases/DiscoverPhase.jsx b/src/renderer/phases/DiscoverPhase.jsx index 0f0b286..9e74b12 100644 --- a/src/renderer/phases/DiscoverPhase.jsx +++ b/src/renderer/phases/DiscoverPhase.jsx @@ -280,6 +280,7 @@ function DiscoverPhase() { cancelAnalysis, clearAnalysisQueue, retryFailedFiles, + reanalyzeFiles, pruneBatchResultsForPath, totalAnalysisFailure, setTotalAnalysisFailure @@ -306,7 +307,8 @@ function DiscoverPhase() { addNotification, showConfirm, phaseData: stablePhaseData, - onFileRemoved: pruneBatchResultsForPath + onFileRemoved: pruneBatchResultsForPath, + onReanalyze: reanalyzeFiles }); const handleFileDrop = useCallback( diff --git a/src/renderer/phases/OrganizePhase.jsx b/src/renderer/phases/OrganizePhase.jsx index 612343b..cbf1e7d 100644 --- a/src/renderer/phases/OrganizePhase.jsx +++ b/src/renderer/phases/OrganizePhase.jsx @@ -60,10 +60,12 @@ import { PHASE_PREREQUISITE_REASONS } from '../store/slices/uiSlice'; import { formatDisplayPath } from '../utils/pathDisplay'; import { toCanonicalPath } from '../utils/canonicalPath'; import { useFileActions } from '../hooks/useFileActions'; +import { useAnalysisRun } from './discover/AnalysisRunProvider'; const logger = createLogger('OrganizePhase'); function OrganizePhase() { const { addNotification } = useNotification(); + const { reanalyzeFiles } = useAnalysisRun(); const { executeAction } = useUndoRedo(); const [viewingFileDetails, setViewingFileDetails] = React.useState(null); const redactPaths = useAppSelector(selectRedactPaths); @@ -716,6 +718,8 @@ function OrganizePhase() { defaultLocation={defaultLocation} existingFolders={safeSmartFolders} onFolderCreated={reloadSmartFolders} + onReanalyze={reanalyzeFiles} + isAnalyzing={isAnalysisRunning} addNotification={addNotification} /> diff --git a/src/renderer/phases/discover/AnalysisRunProvider.jsx b/src/renderer/phases/discover/AnalysisRunProvider.jsx index 56a24d7..319157d 100644 --- a/src/renderer/phases/discover/AnalysisRunProvider.jsx +++ b/src/renderer/phases/discover/AnalysisRunProvider.jsx @@ -63,6 +63,7 @@ export function AnalysisRunProvider({ children }) { cancelAnalysis, clearAnalysisQueue, retryFailedFiles, + reanalyzeFiles, pruneBatchResultsForPath } = useAnalysis({ selectedFiles, @@ -143,6 +144,7 @@ export function AnalysisRunProvider({ children }) { cancelAnalysis, clearAnalysisQueue, retryFailedFiles, + reanalyzeFiles, pruneBatchResultsForPath, totalAnalysisFailure, setTotalAnalysisFailure @@ -152,6 +154,7 @@ export function AnalysisRunProvider({ children }) { cancelAnalysis, clearAnalysisQueue, retryFailedFiles, + reanalyzeFiles, pruneBatchResultsForPath, totalAnalysisFailure ] diff --git a/src/renderer/phases/discover/useAnalysis.js b/src/renderer/phases/discover/useAnalysis.js index e14c73d..21bb3d3 100644 --- a/src/renderer/phases/discover/useAnalysis.js +++ b/src/renderer/phases/discover/useAnalysis.js @@ -82,6 +82,7 @@ function clearAutoAdvanceTimeoutRef(autoAdvanceTimeoutRef) { * @param {any} [abortSignal] * @param {any} [namingSettings] * @param {number} [mediaBusyWaitedMs] - time already spent waiting for an earlier media request + * @param {boolean} [bypassCache] - a reanalysis: do not return the cached result * @returns {Promise} Analysis result */ async function analyzeWithRetry( @@ -89,7 +90,8 @@ async function analyzeWithRetry( attempt = 1, abortSignal = null, namingSettings = null, - mediaBusyWaitedMs = 0 + mediaBusyWaitedMs = 0, + bypassCache = false ) { // Ensure electronAPI is available before attempting analysis if (!window.electronAPI?.files?.analyze) { @@ -99,8 +101,14 @@ async function analyzeWithRetry( } try { + const hasNaming = namingSettings && typeof namingSettings === 'object'; const analyzeOptions = - namingSettings && typeof namingSettings === 'object' ? { namingSettings } : undefined; + hasNaming || bypassCache + ? { + ...(hasNaming ? { namingSettings } : {}), + ...(bypassCache ? { bypassCache: true } : {}) + } + : undefined; return await window.electronAPI.files.analyze(filePath, analyzeOptions); } catch (error) { // Analysis is fully local (in-process node-llama-cpp via main IPC); the @@ -132,7 +140,8 @@ async function analyzeWithRetry( attempt, abortSignal, namingSettings, - mediaBusyWaitedMs + BATCH_BUSY_RETRY_MS + mediaBusyWaitedMs + BATCH_BUSY_RETRY_MS, + bypassCache ); } @@ -154,7 +163,8 @@ async function analyzeWithRetry( attempt + 1, abortSignal, namingSettings, - mediaBusyWaitedMs + mediaBusyWaitedMs, + bypassCache ); } throw error; @@ -363,7 +373,7 @@ function scheduleAutoAdvance({ /** * Show appropriate notification based on analysis results. * - * @param {{ successCount: number, failureCount: number, addNotification: Function, actions: any, getCurrentPhase: Function, autoAdvanceTimeoutRef: any, isMoreAnalysisPending?: () => boolean }} params + * @param {{ successCount: number, failureCount: number, addNotification: Function, actions: any, getCurrentPhase: Function, autoAdvanceTimeoutRef: any, isMoreAnalysisPending?: () => boolean, isReanalysis?: boolean }} params */ function showAnalysisCompletionNotification({ successCount, @@ -372,8 +382,28 @@ function showAnalysisCompletionNotification({ actions, getCurrentPhase, autoAdvanceTimeoutRef, - isMoreAnalysisPending + isMoreAnalysisPending, + isReanalysis = false }) { + if (isReanalysis) { + // The user asked for these files again from where they are: say what + // happened, and neither move them on to Organize nor call the whole + // analysis failed. + const plural = (/** @type {number} */ n) => `${n} ${n === 1 ? 'file' : 'files'}`; + if (failureCount === 0) { + addNotification(`Reanalyzed ${plural(successCount)}`, 'success', 4000, 'analysis-complete'); + } else { + addNotification( + successCount > 0 + ? `Reanalyzed ${plural(successCount)}; ${failureCount} failed` + : `Reanalysis failed for ${plural(failureCount)}`, + successCount > 0 ? 'warning' : 'error', + 6000, + 'analysis-complete' + ); + } + return; + } if (successCount > 0 && failureCount === 0) { addNotification( `Analysis complete! ${successCount} files ready`, @@ -1005,7 +1035,10 @@ export function useAnalysis(options = {}) { * Main analysis function */ const analyzeFiles = useCallback( - async (/** @type {any} */ files) => { + async (/** @type {any} */ files, /** @type {{ bypassCache?: boolean }} */ runOptions = {}) => { + // A reanalysis: every file goes through the per-file request, which + // carries bypassCache, and the run ends without auto-advance. + const bypassCache = runOptions?.bypassCache === true; if (!files || files.length === 0) { addNotification('No files queued for analysis.', 'info', 2000, 'analysis-empty'); return { started: false, reason: 'empty' }; @@ -1395,7 +1428,9 @@ export function useAnalysis(options = {}) { }); logger.info(`Starting analysis of ${uniqueFiles.length} files`); addNotification( - `Starting AI analysis of ${uniqueFiles.length} files...`, + bypassCache + ? `Reanalyzing ${uniqueFiles.length} ${uniqueFiles.length === 1 ? 'file' : 'files'}...` + : `Starting AI analysis of ${uniqueFiles.length} files...`, 'info', 3000, 'analysis-start' @@ -1736,7 +1771,7 @@ export function useAnalysis(options = {}) { try { const analysis = await withTimeout( - analyzeWithRetry(file.path, 1, fileSignal, namingSettings), + analyzeWithRetry(file.path, 1, fileSignal, namingSettings, 0, bypassCache), PER_FILE_LIMIT_MS, `Analysis for ${fileName}` ).catch((/** @type {any} */ error) => { @@ -1814,12 +1849,9 @@ export function useAnalysis(options = {}) { } }; - const useBatchPath = shouldUseBatchAnalysis( - uniqueFiles.length, - concurrency, - systemRecommended, - hasMedia - ); + const useBatchPath = + !bypassCache && + shouldUseBatchAnalysis(uniqueFiles.length, concurrency, systemRecommended, hasMedia); if (useBatchPath) { try { logger.info('Using batch analysis path', { @@ -1948,7 +1980,8 @@ export function useAnalysis(options = {}) { // says analyzing) or is still waiting in pendingFilesRef. isMoreAnalysisPending: () => pendingFilesRef.current.length > 0 || - Boolean(/** @type {any} */ (store.getState())?.analysis?.isAnalyzing) + Boolean(/** @type {any} */ (store.getState())?.analysis?.isAnalyzing), + isReanalysis: bypassCache }); } catch (error) { if (error.message !== 'Analysis cancelled by user') { @@ -2223,6 +2256,58 @@ export function useAnalysis(options = {}) { } }, [analysisResults, fileStates, setFileStates, addNotification]); + /** + * Analyze files again that already have a result, where they are, without + * the cached result. This is what the Reanalyze controls in Discover and + * Organize run; it works for any file in the list, inside a Smart Folder + * or not. + * @returns {{ started: boolean, reason?: string }} + */ + const reanalyzeFiles = useCallback( + (/** @type {string[]} */ filePaths) => { + const wanted = new Set((Array.isArray(filePaths) ? filePaths : []).map(toCanonicalPath)); + const seen = new Set(); + const files = [...analysisResults, ...selectedFiles].filter((/** @type {any} */ f) => { + const key = toCanonicalPath(f?.path); + if (!f?.path || !wanted.has(key) || seen.has(key)) return false; + seen.add(key); + return true; + }); + if (files.length === 0) { + addNotification('No files to reanalyze', 'info', 2000, 'reanalyze'); + return { started: false, reason: 'empty' }; + } + // A running analysis would queue these behind it as an ordinary + // analysis, which returns the cached result: refuse instead. + if ( + analysisLockRef.current || + isAnalyzingRef.current || + /** @type {any} */ (store.getState())?.analysis?.isAnalyzing + ) { + addNotification( + 'Analysis is still running. Reanalyze when it finishes.', + 'info', + 3000, + 'reanalyze' + ); + return { started: false, reason: 'busy' }; + } + + setFileStates((/** @type {any} */ prev) => { + const keyByCanonical = new Map(Object.keys(prev).map((p) => [toCanonicalPath(p), p])); + const updated = { ...prev }; + files.forEach((/** @type {any} */ file) => { + const key = keyByCanonical.get(toCanonicalPath(file.path)) || file.path; + updated[key] = { ...updated[key], state: 'pending', error: null }; + }); + return updated; + }); + void analyzeFilesRef.current?.(files, { bypassCache: true }); + return { started: true }; + }, + [analysisResults, selectedFiles, setFileStates, addNotification, store] + ); + useEffect(() => { // Reset on StrictMode re-mount — refs persist across unmount/remount isMountedRef.current = true; @@ -2337,6 +2422,7 @@ export function useAnalysis(options = {}) { cancelAnalysis, clearAnalysisQueue, retryFailedFiles, + reanalyzeFiles, resetAnalysisState, generatePreviewName }; diff --git a/src/renderer/phases/discover/useFileActions.js b/src/renderer/phases/discover/useFileActions.js index d81e2f2..28e7097 100644 --- a/src/renderer/phases/discover/useFileActions.js +++ b/src/renderer/phases/discover/useFileActions.js @@ -13,7 +13,7 @@ import { mapErrorToNotification } from '../../utils/errorMapping'; /** * Custom hook for file action operations - * @param {{ setAnalysisResults: Function, setSelectedFiles: Function, setFileStates: Function, addNotification: Function, showConfirm: Function, phaseData: any, onFileRemoved?: Function }} options + * @param {{ setAnalysisResults: Function, setSelectedFiles: Function, setFileStates: Function, addNotification: Function, showConfirm: Function, phaseData: any, onFileRemoved?: Function, onReanalyze?: (filePaths: string[]) => unknown }} options * @returns {Record} File action functions */ export function useFileActions({ @@ -23,7 +23,8 @@ export function useFileActions({ addNotification, showConfirm, phaseData, - onFileRemoved + onFileRemoved, + onReanalyze }) { /** * Handle file action (open, reveal, delete, reanalyze) @@ -121,40 +122,15 @@ export function useFileActions({ break; } - case 'reanalyze': { - if (!window.electronAPI?.embeddings?.reanalyzeFile) { + case 'reanalyze': + // The analysis run owns this: it analyzes the file again where + // it is and reports the outcome itself. + if (typeof onReanalyze !== 'function') { addNotification('Reanalysis unavailable', 'error', 3000, 'file-actions'); return; } - const result = await window.electronAPI.embeddings.reanalyzeFile(filePath); - if (result?.success) { - addNotification( - `Reanalysis queued: ${filePath.split(/[\\/]/).pop()}`, - 'success', - 2500, - 'file-actions' - ); - } else if (result?.errorCode === 'NOT_IN_SMART_FOLDER') { - // The scope guard rejects files outside any configured Smart - // Folder root for security (see semantic.js:1723-1740). The - // generic "not eligible" copy didn't tell the user how to - // unblock — name the cause and the fix explicitly. - addNotification( - 'This file is not inside a configured Smart Folder. Add its source folder as a Smart Folder destination to enable reanalysis.', - 'warning', - 6000, - 'file-actions' - ); - } else { - addNotification( - result?.error || 'Reanalysis not eligible for this file', - 'warning', - 4000, - 'file-actions' - ); - } + onReanalyze([filePath]); break; - } case 'delete': { const fileName = filePath.split(/[\\/]/).pop(); @@ -234,7 +210,8 @@ export function useFileActions({ setFileStates, showConfirm, phaseData, - onFileRemoved + onFileRemoved, + onReanalyze ] ); diff --git a/test/analysisIpc.realZod.test.js b/test/analysisIpc.realZod.test.js index 2994c0d..7ef7a63 100644 --- a/test/analysisIpc.realZod.test.js +++ b/test/analysisIpc.realZod.test.js @@ -307,6 +307,50 @@ describe('registerAnalysisIpc (real Zod)', () => { }); }); + describe('single-file analyze payload with bypassCache', () => { + const analyzerOptions = (mock) => mock.mock.calls[0][2]; + + test('ANALYZE_DOCUMENT passes bypassCache to the analyzer', async () => { + await handlers[ANALYSIS_CHANNELS.ANALYZE_DOCUMENT]( + {}, + { filePath: '/x/report.pdf', bypassCache: true } + ); + + expect(analyzerOptions(mockAnalyzeDocumentFile)).toEqual( + expect.objectContaining({ bypassCache: true }) + ); + }); + + test('ANALYZE_IMAGE passes bypassCache to the analyzer', async () => { + await handlers[ANALYSIS_CHANNELS.ANALYZE_IMAGE]( + {}, + { filePath: '/x/photo.jpg', bypassCache: true } + ); + + expect(analyzerOptions(mockAnalyzeImageFile)).toEqual( + expect.objectContaining({ bypassCache: true }) + ); + }); + + test('an ordinary analysis still uses the cache', async () => { + await handlers[ANALYSIS_CHANNELS.ANALYZE_DOCUMENT]({}, '/x/report.pdf'); + + expect(analyzerOptions(mockAnalyzeDocumentFile)).toEqual( + expect.objectContaining({ bypassCache: false }) + ); + }); + + test('a bypassCache that is not a boolean is rejected', async () => { + const result = await handlers[ANALYSIS_CHANNELS.ANALYZE_DOCUMENT]( + {}, + { filePath: '/x/report.pdf', bypassCache: 'yes' } + ); + + expect(mockAnalyzeDocumentFile).not.toHaveBeenCalled(); + expect(result?.success).toBe(false); + }); + }); + describe('CANCEL_BATCH validation', () => { test('rejects payloads missing batchId', async () => { const result = await handlers[ANALYSIS_CHANNELS.CANCEL_BATCH]({}, {}); diff --git a/test/components/OrganizePhase.test.js b/test/components/OrganizePhase.test.js index 985a9da..4ad074f 100644 --- a/test/components/OrganizePhase.test.js +++ b/test/components/OrganizePhase.test.js @@ -11,6 +11,10 @@ jest.mock('../../src/renderer/contexts/NotificationContext', () => ({ useNotification: jest.fn(() => ({ addNotification: mockAddNotification })) })); +jest.mock('../../src/renderer/phases/discover/AnalysisRunProvider', () => ({ + useAnalysisRun: () => ({ reanalyzeFiles: jest.fn() }) +})); + jest.mock('../../src/renderer/components/UndoRedoSystem', () => ({ UndoRedoToolbar: () =>
UNDO_REDO
, useUndoRedo: jest.fn(() => ({ executeAction: jest.fn() })) diff --git a/test/components/UncategorizedActionsBar.test.js b/test/components/UncategorizedActionsBar.test.js index ce0f834..259b283 100644 --- a/test/components/UncategorizedActionsBar.test.js +++ b/test/components/UncategorizedActionsBar.test.js @@ -3,7 +3,7 @@ * too: three invoices from one vendor rarely arrive in the same batch. */ import React from 'react'; -import { render, screen } from '@testing-library/react'; +import { fireEvent, render, screen } from '@testing-library/react'; jest.mock('../../src/shared/logger', () => { const logger = { info: jest.fn(), debug: jest.fn(), warn: jest.fn(), error: jest.fn() }; @@ -87,6 +87,38 @@ describe('UncategorizedActionsBar', () => { expect(container.textContent).not.toContain('Create folder for'); }); + // Reanalyze used to go to the Smart Folder watcher, which refuses files + // outside a Smart Folder: every file still waiting in Organize. + test('Reanalyze hands the uncategorized files to the analysis run', async () => { + const onReanalyze = jest.fn(); + window.electronAPI = { analysisHistory: { getUncategorized: jest.fn() } }; + render( + null} + onReanalyze={onReanalyze} + /> + ); + + fireEvent.click(screen.getByRole('button', { name: 'Reanalyze 2 uncategorized files' })); + + expect(onReanalyze).toHaveBeenCalledWith(['/in/a.pdf', '/in/b.pdf']); + }); + + test('Reanalyze is disabled while an analysis runs', async () => { + window.electronAPI = { analysisHistory: { getUncategorized: jest.fn() } }; + render( + null} + onReanalyze={jest.fn()} + isAnalyzing + /> + ); + + expect(screen.getByRole('button', { name: 'Reanalyze 1 uncategorized files' })).toBeDisabled(); + }); + test('nothing uncategorized on screen, no bar', async () => { const { container } = renderBar([], [earlier('acme-jan.pdf', ['acme'])]); await Promise.resolve(); diff --git a/test/preload.coverage.test.js b/test/preload.coverage.test.js index 8ac20f9..a873f19 100644 --- a/test/preload.coverage.test.js +++ b/test/preload.coverage.test.js @@ -197,6 +197,28 @@ describe('Preload Coverage', () => { ); }); + test('a reanalysis carries bypassCache to the handler', async () => { + await electronAPI.files.analyze('C:\\docs\\report.pdf', { bypassCache: true }); + expect(mockInvoke).toHaveBeenCalledWith( + 'analysis:analyze-document', + { filePath: 'C:\\docs\\report.pdf', bypassCache: true }, + IPC_META_ENVELOPE + ); + }); + + test('bypassCache travels with naming settings', async () => { + const namingSettings = { convention: 'subject-date' }; + await electronAPI.files.analyze('C:\\photos\\test.jpg', { + namingSettings, + bypassCache: true + }); + expect(mockInvoke).toHaveBeenCalledWith( + 'analysis:analyze-image', + { filePath: 'C:\\photos\\test.jpg', namingSettings, bypassCache: true }, + IPC_META_ENVELOPE + ); + }); + test('rejects empty file path', async () => { await expect(electronAPI.files.analyze('')).rejects.toThrow('Invalid file path'); }); diff --git a/test/useAnalysis.test.js b/test/useAnalysis.test.js index ffb0614..93957a7 100644 --- a/test/useAnalysis.test.js +++ b/test/useAnalysis.test.js @@ -251,6 +251,139 @@ describe('useAnalysis', () => { }); }); + // Reanalyze in Discover and Organize: the file is analyzed again where it + // is. It used to go to the Smart Folder watcher, which refuses any file + // outside a Smart Folder, which is where Discover's files are. + describe('reanalyzeFiles', () => { + const { useStore } = require('react-redux'); + let storeState; + const analyzed = { path: '/in/report.pdf', name: 'report.pdf', analysis: { category: 'old' } }; + + beforeEach(() => { + storeState = { analysis: { isAnalyzing: false } }; + useStore.mockReturnValue({ getState: () => storeState }); + }); + + afterEach(() => { + useStore.mockReset(); + useStore.mockImplementation(() => ({ + getState: jest.fn(() => ({ analysis: { isAnalyzing: false, progress: null } })) + })); + }); + + const reanalyze = async (result, paths) => { + let outcome; + await act(async () => { + outcome = result.current.reanalyzeFiles(paths); + await jest.advanceTimersByTimeAsync(500); + }); + return outcome; + }; + const messages = () => mockAddNotification.mock.calls.map((call) => call[0]); + + test('analyzes the file again without the cached result', async () => { + const { result } = renderHook(() => + useAnalysis(createMockOptions({ selectedFiles: [], analysisResults: [analyzed] })) + ); + + const outcome = await reanalyze(result, ['/in/report.pdf']); + + expect(outcome).toEqual({ started: true }); + expect(mockElectronAPI.files.analyze).toHaveBeenCalledWith( + '/in/report.pdf', + expect.objectContaining({ bypassCache: true }) + ); + expect(messages()).toContain('Reanalyzed 1 file'); + }); + + test('finds the file whichever slash or case its path was recorded with', async () => { + const recorded = { ...analyzed, path: 'C:\\In\\Report.pdf' }; + const { result } = renderHook(() => + useAnalysis(createMockOptions({ selectedFiles: [], analysisResults: [recorded] })) + ); + + await reanalyze(result, ['c:/in/report.pdf']); + + expect(mockElectronAPI.files.analyze).toHaveBeenCalledWith( + 'C:\\In\\Report.pdf', + expect.objectContaining({ bypassCache: true }) + ); + }); + + test('leaves the user on the screen they asked from', async () => { + const { result } = renderHook(() => + useAnalysis( + createMockOptions({ + selectedFiles: [], + analysisResults: [analyzed], + getCurrentPhase: () => 'discover' + }) + ) + ); + + await reanalyze(result, ['/in/report.pdf']); + await act(async () => { + await jest.advanceTimersByTimeAsync(2500); + }); + + expect(mockActions.advancePhase).not.toHaveBeenCalled(); + }); + + test('a failed reanalysis is reported as that, not as a failed analysis run', async () => { + mockElectronAPI.files.analyze.mockRejectedValue(new Error('model unavailable')); + const { result } = renderHook(() => + useAnalysis(createMockOptions({ selectedFiles: [], analysisResults: [analyzed] })) + ); + + await act(async () => { + result.current.reanalyzeFiles(['/in/report.pdf']); + await jest.advanceTimersByTimeAsync(60000); + }); + + expect(messages()).toContain('Reanalysis failed for 1 file'); + expect(mockActions.setPhaseData).not.toHaveBeenCalledWith('totalAnalysisFailure', true); + }); + + test('sends a recording through the per-file request, which carries bypassCache', async () => { + const recording = { path: '/in/call.mp3', name: 'call.mp3', analysis: {} }; + const { result } = renderHook(() => + useAnalysis(createMockOptions({ selectedFiles: [], analysisResults: [recording] })) + ); + + await reanalyze(result, ['/in/call.mp3']); + + expect(mockElectronAPI.analysis.batch).not.toHaveBeenCalled(); + expect(mockElectronAPI.files.analyze).toHaveBeenCalledWith( + '/in/call.mp3', + expect.objectContaining({ bypassCache: true }) + ); + }); + + test('is refused while an analysis is running', async () => { + storeState.analysis.isAnalyzing = true; + const { result } = renderHook(() => + useAnalysis(createMockOptions({ selectedFiles: [], analysisResults: [analyzed] })) + ); + + const outcome = await reanalyze(result, ['/in/report.pdf']); + + expect(outcome).toEqual({ started: false, reason: 'busy' }); + expect(mockElectronAPI.files.analyze).not.toHaveBeenCalled(); + expect(messages()).toContain('Analysis is still running. Reanalyze when it finishes.'); + }); + + test('says so when none of the paths is in the list', async () => { + const { result } = renderHook(() => + useAnalysis(createMockOptions({ selectedFiles: [], analysisResults: [analyzed] })) + ); + + const outcome = await reanalyze(result, ['/elsewhere/other.pdf']); + + expect(outcome).toEqual({ started: false, reason: 'empty' }); + expect(mockElectronAPI.files.analyze).not.toHaveBeenCalled(); + }); + }); + describe('initialization', () => { test('returns analyzeFiles function', () => { const { result } = renderHook(() => useAnalysis(createMockOptions())); diff --git a/test/useFileActions.test.js b/test/useFileActions.test.js index 5370744..45abc6c 100644 --- a/test/useFileActions.test.js +++ b/test/useFileActions.test.js @@ -371,6 +371,35 @@ describe('useFileActions', () => { }); }); + describe('reanalyze action', () => { + test('hands the file to the analysis run', async () => { + const onReanalyze = jest.fn(); + const { result } = renderHook(() => useFileActions(createMockOptions({ onReanalyze }))); + + await act(async () => { + await result.current.handleFileAction('reanalyze', '/path/to/file.txt'); + }); + + expect(onReanalyze).toHaveBeenCalledWith(['/path/to/file.txt']); + expect(mockAddNotification).not.toHaveBeenCalled(); + }); + + test('says reanalysis is unavailable when no run is wired in', async () => { + const { result } = renderHook(() => useFileActions(createMockOptions())); + + await act(async () => { + await result.current.handleFileAction('reanalyze', '/path/to/file.txt'); + }); + + expect(mockAddNotification).toHaveBeenCalledWith( + 'Reanalysis unavailable', + 'error', + 3000, + 'file-actions' + ); + }); + }); + describe('unknown action', () => { test('shows error for unknown action', async () => { const { result } = renderHook(() => useFileActions(createMockOptions()));