From b613528043f4684f5199c61b5b3468d752e66906 Mon Sep 17 00:00:00 2001 From: Levy Tate <78818969+iLevyTate@users.noreply.github.com> Date: Mon, 9 Nov 2026 23:55:50 -0500 Subject: [PATCH 1/2] fix: review sweep of the analysis path (runs, model memory, vision, Whisper, watcher, dates) Discover runs - A run with audio or video always goes through the batch; the batch keeps reporting during one long file and is bounded by its own checks instead of a fixed 30-minute request timeout and a 5-minute "stuck" check. - A cancelled batch reports nothing more; a cancelled file is neither failed nor recorded. A progress update held by the renderer's throttle is dropped when its analysis ends ("Analysis paused" after Stop). - Only a Discover analysis writes analysis progress, and only the running batch's own events are accepted. - A run that waited for an earlier batch shows the counts of the whole run. - A run that holds the lock and was not aborted counts as active from the moment it took the lock: a second call during its start-up is queued instead of resetting it as stale. - A run replaced during its start-up pause stops there, and a heartbeat clears only its own interval (StrictMode resume lost its heartbeat). - setFileStates(prev => next) reads prev from the store, not from the last render, in Discover and Organize. - The Dashboard waits for a running analysis. Models - Memory counts from the start of a load; a second caller shares the load; external consumers reserve before they start. - The vision runtime is not shut down under a running image, a failed start is reported, a timed-out request releases its pin, and a connection closed mid-answer settles the request. - Cancelling one analysis does not cancel another that shares its transcription. A recording past the transcription limit is recorded once. Watcher - Checks the history again before analyzing, records the file as it was when the analysis started, pauses an image batch for a Discover run, and keeps a reanalysis request across a change to the same file. Dates - Suggested names, the file date given to the model, dates read from content and the settings export name use the local calendar day. The history date-range walk steps through UTC months, as the index is keyed. Tests - New suites for pending loads, per-zone dates (child process per zone), the live-store updater, image cancellation and the progress middleware. - useAnalysis tests clear the module-level resume flag after each test; a test that unmounted mid-run started a resume in the next one. --- CHANGELOG.md | 87 ++ CLAUDE.md | 2 +- scripts/verify-checkjs-ratchet.js | 2 +- src/main/analysis/analysisCancellation.js | 76 ++ src/main/analysis/documentAnalysis.js | 14 +- src/main/analysis/imageAnalysis.js | 47 +- src/main/analysis/modelAnalysisResult.js | 8 +- src/main/ipc/analysis.js | 48 +- src/main/ipc/semantic.js | 9 +- src/main/ipc/settings.js | 3 +- src/main/services/BatchAnalysisService.js | 23 +- src/main/services/DegradationManager.js | 17 + src/main/services/MediaTranscoder.js | 12 +- src/main/services/ModelMemoryManager.js | 226 ++++-- src/main/services/SmartFolderWatcher.js | 78 +- src/main/services/VisionService.js | 206 ++++- src/main/services/WhisperService.js | 54 +- src/main/services/analysisHistory/queries.js | 10 +- src/main/services/mediaAnalyzerAdapter.js | 39 + src/renderer/components/NavigationBar.jsx | 32 +- .../search/hooks/useSearchModalStats.js | 3 +- .../settings/EmbeddingRebuildSection.jsx | 3 +- .../settings/GraphRetrievalSection.jsx | 3 +- src/renderer/phases/discover/useAnalysis.js | 225 ++++-- .../phases/discover/useDiscoverState.js | 13 +- .../phases/organize/useOrganizeState.js | 17 +- src/renderer/store/analysisRunBase.js | 43 + src/renderer/store/hooks.js | 5 +- .../store/middleware/ipcMiddleware.js | 73 +- src/shared/contentDateSanity.js | 30 +- src/shared/ipcEventSchemas.js | 1 + src/shared/ipcInvokeTimeout.js | 22 +- src/shared/namingConventions.js | 15 +- src/shared/performanceConstants.js | 8 + test/BatchAnalysisService.test.js | 40 +- test/DegradationManager.test.js | 22 + test/MediaTranscoder.test.js | 2 +- ...odelMemoryManager.externalConsumer.test.js | 26 + test/ModelMemoryManager.pendingLoads.test.js | 268 +++++++ test/SmartFolderWatcher.migration.test.js | 36 + ...martFolderWatcher.queuePersistence.test.js | 11 +- ...artFolderWatcher.userAnalysisDefer.test.js | 58 ++ ...martFolderWatcher.visionBatchOwner.test.js | 84 ++ test/VisionService.test.js | 498 ++++++++++++ test/WhisperService.test.js | 83 ++ test/analysis-ipc-handlers.test.js | 196 +++++ test/analysisHistory-queries.test.js | 28 + test/calendarDayLocalTime.test.js | 190 +++++ .../NavigationBar.regression.test.js | 55 ++ test/fileStatesUpdaterLiveStore.test.js | 104 +++ test/imageAnalysis.cancellation.test.js | 245 ++++++ .../analyzeNameOrganize.e2e.test.js | 12 +- test/ipcEventSchemas.test.js | 10 + test/ipcInvokeTimeout.test.js | 26 +- test/ipcMiddleware.extended.test.js | 43 +- test/ipcMiddleware.progressAfterStop.test.js | 90 +++ test/ipcMiddleware.runBase.test.js | 122 +++ test/mediaAnalyzerAdapter.test.js | 34 + test/namingConventionMedia.test.js | 6 +- test/semanticIpc.handlers.test.js | 2 +- test/useAnalysis.test.js | 749 +++++++++++++++++- test/useDiscoverState.test.js | 6 +- test/useOrganizeState.test.js | 6 +- 63 files changed, 4225 insertions(+), 281 deletions(-) create mode 100644 src/main/analysis/analysisCancellation.js create mode 100644 src/renderer/store/analysisRunBase.js create mode 100644 test/ModelMemoryManager.pendingLoads.test.js create mode 100644 test/calendarDayLocalTime.test.js create mode 100644 test/fileStatesUpdaterLiveStore.test.js create mode 100644 test/imageAnalysis.cancellation.test.js create mode 100644 test/ipcMiddleware.progressAfterStop.test.js create mode 100644 test/ipcMiddleware.runBase.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 578676c..40e968a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -203,6 +203,93 @@ Changes you will notice: ### Fixed +- **A Discover run with a long recording is no longer cut off or started over.** The batch request + ended after a fixed 30 minutes, and five minutes without a finished file was taken for a stuck + analysis. Either way the rest of the run went one file at a time, where a recording gets five + minutes and is then transcribed again from the start, and where Stop does not reach the work in + progress. A run with audio or video now always goes through the batch, the batch keeps reporting + while one long file is transcribed, and the run is bounded by its own checks (no progress, and at + most six hours). +- **Stop leaves nothing behind.** After a cancel, a file that was in flight could still report + progress, which put back counts that read as a paused analysis; a document that was mid-analysis + was listed as failed; and an image whose vision request was cancelled was stored with a + filename-only "Limited analysis" result. A cancelled batch now reports nothing more, and a + cancelled file is neither failed nor recorded. +- **Leaving Discover during an analysis no longer says "Analysis cancelled by user".** The notice + now appears only when you stop the run. A run left behind this way also no longer writes over the + progress of the run a returning Discover started. +- **The Dashboard waits for a running analysis.** Switching to the Dashboard closed Discover, which + stopped the run without saying so. The Dashboard toggle and **Back to dashboard** now explain that + the dashboard opens when the analysis finishes; the way back to Guided stays open. +- **Progress from other work no longer shows as analysis progress.** Organize batches, model + downloads and **Reanalyze all** report on the same channel as a Discover batch, and their counts + were written into the analysis state: "Analysis continuing in background: 3/7 files" could be an + organize batch, and a finished reanalysis could leave a paused analysis behind. Only a Discover + analysis updates that state now, and another sender can no longer take over the running batch + (after which its progress was dropped and Stop did nothing). +- **A file that finishes right after another appears without waiting for the next one.** The second + result was held back until another file finished, which behind a long recording could be many + minutes. +- **The completion notice counts every file of a run that waited for an earlier batch,** not only + the files the run analyzed itself. +- **Dates follow your calendar day, not the UTC day.** A file saved in the evening west of Greenwich + was named and dated for the next day, and a date written in a document or file name moved back a + day east of it. Suggested names, the file date given to the model, dates the model reads from the + content and the settings export file name now use the local day. A date-range search of the + analysis history also no longer skips the first or last month of the range. +- **A recording longer than the transcription limit is recorded once instead of failing every + time.** Each attempt decoded the audio up to the limit before failing, and the smart-folder + watcher retried it. The file is now kept as "not transcribed", with the limit in its description. +- **Two models are no longer loaded onto a GPU that has room for one.** A model counted against the + memory budget only once it had finished loading, and the vision runtime only once it had started + (about a minute on a laptop GPU), so a text model and the vision runtime started moments apart + both fit on paper. Memory now counts from the moment a load begins, a second caller for the same + model shares the load, and background work does not start on memory that is spoken for. +- **An image being analyzed keeps the vision runtime.** The runtime could be shut down under an + image in three ways: by its idle timer while another image was still running, by a text model + taking its memory between its start and the request, and by a start that failed without reporting + it (the app then polled a dead port until the startup timeout). Each left the image with an + OCR-only result or the batch waiting. A request that timed out also kept the runtime pinned in + memory for the rest of the session, and a runtime that crashed part way through an answer left its + image waiting for good. +- **Cancelling one analysis no longer cancels another that was waiting for the same transcription.** + Two analyses of one recording share a transcription; cancelling the first failed the second as + cancelled, and cancelling the second did nothing until the transcription ended. Each now follows + its own cancel. +- **The smart-folder watcher checks again before it analyzes.** A file queued while a Discover run + held the queue was analyzed a second time although the run had just analyzed it, and a file saved + while it was being analyzed was recorded as up to date with the old content. The watcher now asks + the history when a file's turn comes and records the file as it was when the analysis started. An + image batch (up to 20 images) also pauses when a Discover run starts, instead of finishing beside + it, and a reanalysis you asked for is no longer turned into a plain update by a change to the same + file. +- **A batch replaced as stale is stopped,** instead of running beside its replacement, and when it + ends late it no longer makes the replacement impossible to cancel. +- **Quieter logs.** A model load refused because another model is in use is no longer logged as an + unclassified failure with a stack trace, waiting for a model that is in use warns once instead of + every two seconds, and the batch concurrency in the memory-pressure warning is the one that + applies. +- **A file's status is no longer put back by a later change.** Each change to the list of file + statuses was worked out from the list as the page last drew it. A second change in the same + moment, or one made after you had left the page (a Discover run ends when its last file is let go, + which can be minutes later), put that older list back over everything changed since. Every change + in Discover and Organize now starts from the current list. +- **"Analysis paused" no longer appears after Stop.** Progress reaches the screen at most every 80 + ms, and an update that arrived inside that window was written when it ended, also when the + analysis had been stopped or reset in between. Counts without a running analysis read as a paused + one. The held update is now dropped when its analysis ends. +- **The progress of a run that waited for an earlier batch counts the whole run.** Such a run sends + only the files the earlier batch did not finish, and the count shown restarted with them: "1 of + 10" on a run of 40 with 30 done. It now continues from what was done, "31 of 40". +- **A second Analyze right after the first no longer replaces it.** A run records its first progress + a moment after it starts, and until then it looked like a run that had stopped responding: a + second request in that moment reset it and took its place. The second request now waits its turn + behind the first. +- **Development builds: a resumed analysis keeps its activity signal.** Under React StrictMode the + run that was aborted while starting carried on as if it had started, reported the analysis as + started a second time, and switched off the 30-second activity signal of the run that replaced it + (and of every run after that), so a long first file could be taken for a stuck analysis. A run + replaced while starting now stops there, and a run only ever stops its own signal. - **Analyzed files show up again after Discover remounts mid-start.** A run that Discover's unmount (or React StrictMode in development) aborted during its first moments still sent its batch, then ignored every result: the list stayed on "0 ready" with loading placeholders, and five minutes diff --git a/CLAUDE.md b/CLAUDE.md index e12c768..799e2f4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -103,7 +103,7 @@ tree. `ci`) enforces a monotonic floor and blocks new .js under `COVERED_DIRS` (`src/shared`, `src/preload`, `src/main/{ipc,services,analysis,errors,utils,workers,transports,core}`, `src/renderer/{services,hooks,phases,store,utils,components}` today (every `.js` under `src/` is - now opted in — 489/489 as of 2026-11-09, and `GLOBAL_DIRECTIVE_FLOOR` matches at 489) — see + now opted in — 493/493 as of 2026-11-09, and `GLOBAL_DIRECTIVE_FLOOR` matches at 493) — see `scripts/verify-checkjs-ratchet.js` for the authoritative list) from landing without the directive. When adding a file to a covered dir: put `// @ts-check` on line 1 and fix the errors before commit. When opting in a new file elsewhere: bump `GLOBAL_DIRECTIVE_FLOOR` in diff --git a/scripts/verify-checkjs-ratchet.js b/scripts/verify-checkjs-ratchet.js index e36c531..d80eb91 100644 --- a/scripts/verify-checkjs-ratchet.js +++ b/scripts/verify-checkjs-ratchet.js @@ -43,7 +43,7 @@ const COVERED_DIRS = [ 'src/renderer/utils', 'src/renderer/components' ]; -const GLOBAL_DIRECTIVE_FLOOR = 491; +const GLOBAL_DIRECTIVE_FLOOR = 493; // ──────────────────────────────────────────────────────────────────────── const DIRECTIVE_SCAN_LINES = 5; diff --git a/src/main/analysis/analysisCancellation.js b/src/main/analysis/analysisCancellation.js new file mode 100644 index 0000000..4d66b24 --- /dev/null +++ b/src/main/analysis/analysisCancellation.js @@ -0,0 +1,76 @@ +// @ts-check +/** + * Ending an analysis the caller cancelled. + * + * The analyzers turn almost every failure into a filename fallback, so a file + * always gets some result. A cancellation is not a failure: the caller asked + * for the work to stop and will not use a result. Returned as a fallback, it + * was recorded in the analysis history and the ready queue as if the file had + * been analyzed. + * + * @module analysis/analysisCancellation + */ + +const { isFallbackAnalysis } = require('../../shared/analysisProvenance'); + +/** + * @param {string} [message] + * @returns {Error & { code: string }} + */ +function createCancellationError(message = 'Analysis cancelled') { + return Object.assign(new Error(message), { code: 'OPERATION_CANCELLED' }); +} + +/** + * Whether the caller of an analyzer has cancelled: its abort signal fired, or + * its isCancelled() poll says so. + * @param {{ abortSignal?: AbortSignal | null, signal?: AbortSignal | null, isCancelled?: (() => boolean) | null } | null | undefined} options + * @returns {boolean} + */ +function callerCancelled(options) { + if (options?.abortSignal?.aborted || options?.signal?.aborted) return true; + if (typeof options?.isCancelled !== 'function') return false; + try { + return options.isCancelled() === true; + } catch { + return false; + } +} + +/** + * The error to throw for a failure caught while the caller has cancelled: + * the failure itself when it already is a cancellation, otherwise a + * cancellation that names it. + * @param {any} error + * @returns {Error & { code?: string }} + */ +function asCancellationError(error) { + if (error?.code === 'OPERATION_CANCELLED') return error; + const cancellation = createCancellationError(); + /** @type {any} */ (cancellation).cause = error; + return cancellation; +} + +/** + * A fallback produced after the caller cancelled is the cancellation's doing + * (the engine reported the abort as an ordinary failure and the analyzer fell + * back): throw instead of returning it. A real analysis that finished anyway + * is kept, the work is done. + * @template T + * @param {T} result + * @param {Parameters[0]} options + * @returns {T} + */ +function rejectFallbackOfCancelledRun(result, options) { + if (callerCancelled(options) && isFallbackAnalysis(/** @type {any} */ (result))) { + throw createCancellationError(); + } + return result; +} + +module.exports = { + createCancellationError, + callerCancelled, + asCancellationError, + rejectFallbackOfCancelledRun +}; diff --git a/src/main/analysis/documentAnalysis.js b/src/main/analysis/documentAnalysis.js index 3474d95..318c877 100644 --- a/src/main/analysis/documentAnalysis.js +++ b/src/main/analysis/documentAnalysis.js @@ -19,6 +19,7 @@ const { TRUNCATION, TIMEOUTS } = require('../../shared/performanceConstants'); const { withTimeout, delay } = require('../../shared/promiseUtils'); const { createLogger } = require('../../shared/logger'); const { isMemoryBudgetExceeded } = require('../../shared/analysisProvenance'); +const { toLocalCalendarDay } = require('../../shared/contentDateSanity'); const { getTextModel, getLlamaService } = require('../llamaUtils'); const { AppConfig } = require('./documentLlm'); @@ -56,6 +57,7 @@ const { safeSuggestedName, createFallbackAnalysis } = require('./fallbackUtils'); +const { createCancellationError } = require('./analysisCancellation'); const { analysisQueue } = require('./embeddingQueue/stageQueues'); const { globalDeduplicator } = require('../utils/llmOptimization'); const { @@ -368,19 +370,21 @@ async function analyzeDocumentFile(filePath, smartFolders = [], options = {}) { // Get file date from stats or default to today // Some test fixtures (and some fs polyfills) may not provide a Date-valued mtime. const fileDate = (() => { - const today = new Date().toISOString().split('T')[0]; + // The day the user sees in the file manager, not the UTC day: a file + // saved in the evening west of Greenwich is not from tomorrow. + const today = toLocalCalendarDay(Date.now()); if (!fileStats) return today; // Prefer Date-valued mtime when available const { mtime } = fileStats; - if (mtime && typeof mtime.toISOString === 'function') { - return mtime.toISOString().split('T')[0]; + if (mtime && typeof mtime.getTime === 'function' && Number.isFinite(mtime.getTime())) { + return toLocalCalendarDay(mtime.getTime()); } // Fall back to numeric mtimeMs when present const { mtimeMs } = fileStats; if (typeof mtimeMs === 'number' && Number.isFinite(mtimeMs) && mtimeMs > 0) { - return new Date(mtimeMs).toISOString().split('T')[0]; + return toLocalCalendarDay(mtimeMs); } return today; @@ -908,7 +912,7 @@ async function analyzeDocumentFile(filePath, smartFolders = [], options = {}) { // Check for cancellation before expensive LLM call if (typeof options.isCancelled === 'function' && options.isCancelled()) { - throw new Error('Analysis cancelled'); + throw createCancellationError(); } // Backend Caching & Deduplication: diff --git a/src/main/analysis/imageAnalysis.js b/src/main/analysis/imageAnalysis.js index e5e7778..1654d3a 100644 --- a/src/main/analysis/imageAnalysis.js +++ b/src/main/analysis/imageAnalysis.js @@ -6,7 +6,11 @@ const crypto = require('crypto'); const { getInstance: getLlamaService } = require('../services/LlamaService'); const { globalDeduplicator } = require('../utils/llmOptimization'); const { extractAndParseJSON } = require('../utils/jsonRepair'); -const { isPlausibleContentDate } = require('../../shared/contentDateSanity'); +const { + isPlausibleContentDate, + toLocalCalendarDay, + toWrittenCalendarDay +} = require('../../shared/contentDateSanity'); const { attemptJsonRepairWithLlama, attemptProseExtractionWithLlama @@ -33,6 +37,12 @@ const { safeSuggestedName, createFallbackAnalysis } = require('./fallbackUtils'); +const { + createCancellationError, + callerCancelled, + asCancellationError, + rejectFallbackOfCancelledRun +} = require('./analysisCancellation'); const FolderMatchingService = require('../services/FolderMatchingService'); const { analysisQueue } = require('./embeddingQueue/stageQueues'); const embeddingQueueManager = require('./embeddingQueue/queueManager'); @@ -658,12 +668,12 @@ STRICT OUTPUT REQUIREMENT: // Validate and structure the date if (parsedJson.date) { - const dateObj = new Date(parsedJson.date); - if (isNaN(dateObj.getTime())) { + const writtenDay = toWrittenCalendarDay(parsedJson.date); + if (!writtenDay) { delete parsedJson.date; logger.warn('AI engine returned an invalid date for image, omitting.'); } else { - parsedJson.date = dateObj.toISOString().split('T')[0]; + parsedJson.date = writtenDay; // Same plausibility gate as documentLlm: a hallucinated year // would poison the timeline sort and stats-card year range. if (!isPlausibleContentDate(parsedJson.date)) { @@ -1275,11 +1285,24 @@ async function preprocessImageBuffer(imageBuffer, fileExtension, options = {}) { // ============================================================================ /** + * Analyze an image. Rejects with OPERATION_CANCELLED when the caller + * cancelled (abort signal or isCancelled) and no analysis was completed; + * every other failure resolves to a filename fallback. * @param {any} filePath * @param {any[]} [smartFolders] * @param {{ bypassCache?: boolean, isCancelled?: () => boolean, abortSignal?: AbortSignal, signal?: AbortSignal, smartFolderId?: string | null, visionModel?: string }} [options] */ async function analyzeImageFile(filePath, smartFolders = [], options = {}) { + const result = await runImageAnalysis(filePath, smartFolders, options); + return rejectFallbackOfCancelledRun(result, options); +} + +/** + * @param {any} filePath + * @param {any[]} smartFolders + * @param {{ bypassCache?: boolean, isCancelled?: () => boolean, abortSignal?: AbortSignal, signal?: AbortSignal, smartFolderId?: string | null, visionModel?: string }} options + */ +async function runImageAnalysis(filePath, smartFolders, options) { logger.info(`Analyzing image file`, { path: filePath }); const bypassCache = Boolean(options?.bypassCache); const fileExtension = path.extname(filePath).toLowerCase(); @@ -1428,7 +1451,7 @@ async function analyzeImageFile(filePath, smartFolders = [], options = {}) { // "By filename and date" means the date too: the file's modification // time is in hand from the stat above, and without it the Timeline // filed a whole photo library under the day the batch ran. - const fileDay = new Date(stats.mtimeMs || Date.now()).toISOString().split('T')[0]; + const fileDay = toLocalCalendarDay(stats.mtimeMs || Date.now()); const undecodable = createVisionUnavailableFallback( { fileName, @@ -1590,9 +1613,7 @@ async function analyzeImageFile(filePath, smartFolders = [], options = {}) { logger.debug('[IMAGE] Failed to get naming context', { error: ncError.message }); } - const fileDateForText = exifDate - ? exifDate - : new Date(stats.mtimeMs || Date.now()).toISOString().split('T')[0]; + const fileDateForText = exifDate ? exifDate : toLocalCalendarDay(stats.mtimeMs || Date.now()); const attemptTextFallback = async () => { let ocrText = extractedText; if (!ocrText || ocrText.length < 20) { @@ -1650,8 +1671,8 @@ async function analyzeImageFile(filePath, smartFolders = [], options = {}) { }; // Check for cancellation before expensive vision/LLM call - if (typeof options.isCancelled === 'function' && options.isCancelled()) { - throw new Error('Analysis cancelled'); + if (callerCancelled(options)) { + throw createCancellationError(); } /** @type {AnalysisResult | undefined} */ @@ -1819,6 +1840,7 @@ async function analyzeImageFile(filePath, smartFolders = [], options = {}) { analysis = retriedAnalysis; } } catch (retryError) { + if (retryError?.code === 'OPERATION_CANCELLED') throw retryError; logger.warn('[IMAGE] Vision retry failed', { filePath, error: retryError?.message || String(retryError) @@ -2287,6 +2309,11 @@ async function analyzeImageFile(filePath, smartFolders = [], options = {}) { ) { throw error; } + // A cancellation is not a failed analysis: the caller stops, and no + // filename fallback is recorded for a file nobody analyzed. + if (error?.code === 'OPERATION_CANCELLED' || callerCancelled(options)) { + throw asCancellationError(error); + } logger.error('Error processing image', { filePath, error: error.message }); const fallback = createFallbackAnalysis({ fileName, diff --git a/src/main/analysis/modelAnalysisResult.js b/src/main/analysis/modelAnalysisResult.js index 3817cee..7c86632 100644 --- a/src/main/analysis/modelAnalysisResult.js +++ b/src/main/analysis/modelAnalysisResult.js @@ -15,7 +15,7 @@ */ const path = require('path'); -const { isPlausibleContentDate } = require('../../shared/contentDateSanity'); +const { isPlausibleContentDate, toWrittenCalendarDay } = require('../../shared/contentDateSanity'); const { sanitizeSuggestedBaseName } = require('../../shared/pathSanitization'); const FolderMatchingService = require('../services/FolderMatchingService'); @@ -46,11 +46,11 @@ function buildAnalysisFromModelJson(parsedJson, context) { // Validate and sanitize date field if (parsedJson.date) { try { - const dateObj = new Date(parsedJson.date); - if (isNaN(dateObj.getTime())) { + const writtenDay = toWrittenCalendarDay(parsedJson.date); + if (!writtenDay) { delete parsedJson.date; } else { - parsedJson.date = dateObj.toISOString().split('T')[0]; + parsedJson.date = writtenDay; // A parseable-but-implausible year (hallucinated "2103", // OCR-misread "0201") would pin the timeline sort and poison // the stats card's year range — drop to the fileDate fallback. diff --git a/src/main/ipc/analysis.js b/src/main/ipc/analysis.js index 0b409a2..aec3ecb 100644 --- a/src/main/ipc/analysis.js +++ b/src/main/ipc/analysis.js @@ -11,6 +11,7 @@ const { normalizeNamingConventionState } = require('../../shared/namingConventio const { safeFilePath } = require('../utils/safeAccess'); const { validateFileOperationPath } = require('../../shared/pathSanitization'); const { isFallbackAnalysis, isMemoryBudgetExceeded } = require('../../shared/analysisProvenance'); +const { isMediaPath } = require('../../shared/fileIdUtils'); const { mapFoldersToCategories, getFolderNamesString } = require('../../shared/folderUtils'); const { recognizeIfAvailable } = require('../utils/tesseractUtils'); const { @@ -852,6 +853,14 @@ function registerAnalysisIpc(servicesOrParams) { startedAt: existing.startedAt, ageMs: Date.now() - existing.startedAt }); + // The evicted run may still be working. Stop it, so it does not + // analyze beside the run that takes its id. + existing.cancelled = true; + if (!existing.reason) { + existing.reason = 'Replaced by a new batch with the same id'; + existing.requestedAt = Date.now(); + } + existing.abortController?.abort(); activeBatchRuns.delete(batchId); } else { logger.warn(`${LOG_PREFIX} Duplicate active batch id rejected`, { batchId }); @@ -989,6 +998,27 @@ function registerAnalysisIpc(servicesOrParams) { } } + // The service reports only when a file starts or finishes, and one long + // recording is many minutes of work. The renderer takes 5 minutes without + // a report for a stuck analysis and resets it. So the last report is + // repeated while the batch runs: silence then means main stopped + // answering. Only for as long as one file may take, so a file that hangs + // is still noticed. + const quietBudgetMs = validatedPaths.some((/** @type {string} */ p) => isMediaPath(p)) + ? TIMEOUTS.BATCH_MEDIA_QUIET_MAX + : TIMEOUTS.BATCH_FILE_QUIET_MAX; + /** @type {Record | null} */ + let lastProgressPayload = null; + let lastProgressAt = 0; + const progressHeartbeat = setInterval(() => { + if (!lastProgressPayload || runState.cancelled) return; + if (Date.now() - lastProgressAt > quietBudgetMs) return; + if (typeof getMainWindow === 'function') { + sendOperationProgress(getMainWindow, lastProgressPayload); + } + }, TIMEOUTS.HEARTBEAT_INTERVAL); + progressHeartbeat.unref?.(); + // The smart-folder watcher defers these files until the batch is done, // so a file it also sees is not analyzed twice at once. const releaseUserAnalysis = claimUserAnalysis(validatedPaths); @@ -1010,12 +1040,14 @@ function registerAnalysisIpc(servicesOrParams) { sectionOrder: options.sectionOrder, enableVisionBatchMode: options.enableVisionBatchMode, onProgress: (/** @type {any} */ progress) => { + // The renderer cleared its progress when it cancelled. A later + // report (a file that was in flight, the closing flush) would + // put counts back that read as a paused analysis. + if (runState.cancelled) return; + lastProgressPayload = { type: 'batch_analyze', batchId, ...progress }; + lastProgressAt = Date.now(); if (typeof getMainWindow === 'function') { - sendOperationProgress(getMainWindow, { - type: 'batch_analyze', - batchId, - ...progress - }); + sendOperationProgress(getMainWindow, lastProgressPayload); } }, onFileComplete: (/** @type {any} */ item) => { @@ -1230,9 +1262,13 @@ function registerAnalysisIpc(servicesOrParams) { batchId }; } finally { + clearInterval(progressHeartbeat); releaseUserAnalysis(); removeAbortListener?.(); - activeBatchRuns.delete(batchId); + // Only this run's own entry: an evicted run that finishes late must + // not remove the run that replaced it, or that run can no longer be + // cancelled. + if (activeBatchRuns.get(batchId) === runState) activeBatchRuns.delete(batchId); } } }); diff --git a/src/main/ipc/semantic.js b/src/main/ipc/semantic.js index 8b09af4..326bc46 100644 --- a/src/main/ipc/semantic.js +++ b/src/main/ipc/semantic.js @@ -2435,6 +2435,9 @@ async function handleReanalyzeAll( const totalToProcess = result.queued; if (totalToProcess > 0) { const batchId = `reanalyze-all-${Date.now()}`; + // Its own type, not 'batch_analyze': that one is a Discover batch's + // progress, and the renderer shows it as the running analysis. + const progressType = 'reanalyze'; // Clear any existing polling interval to prevent duplicate progress bars if (/** @type {any} */ (global).__reanalyzePollInterval) { @@ -2451,7 +2454,7 @@ async function handleReanalyzeAll( const win = getMainWindow(); if (win && !win.isDestroyed()) { safeSend(win.webContents, IPC_EVENTS.OPERATION_PROGRESS, { - type: 'batch_analyze', + type: progressType, batchId, completed: totalToProcess, total: totalToProcess, @@ -2474,7 +2477,7 @@ async function handleReanalyzeAll( const win = getMainWindow(); if (win && !win.isDestroyed()) { safeSend(win.webContents, IPC_EVENTS.OPERATION_PROGRESS, { - type: 'batch_analyze', + type: progressType, batchId, completed, total: totalToProcess, @@ -2491,7 +2494,7 @@ async function handleReanalyzeAll( const win = getMainWindow(); if (win && !win.isDestroyed()) { safeSend(win.webContents, IPC_EVENTS.OPERATION_PROGRESS, { - type: 'batch_analyze', + type: progressType, batchId, completed: totalToProcess, total: totalToProcess, diff --git a/src/main/ipc/settings.js b/src/main/ipc/settings.js index b319241..2cbfc8b 100644 --- a/src/main/ipc/settings.js +++ b/src/main/ipc/settings.js @@ -47,6 +47,7 @@ const { applyExperienceModeSettings } = require('../../shared/experienceMode'); const { SETTINGS_SCHEMA_VERSION } = require('../../shared/constants'); +const { toLocalCalendarDay } = require('../../shared/contentDateSanity'); const { validateAiSettingsForSave } = require('../services/ai/aiSettings'); /** @@ -781,7 +782,7 @@ function registerSettingsIpc(servicesOrParams) { // Always show save dialog for security const result = await dialog.showSaveDialog({ title: 'Export Settings', - defaultPath: `stratosort-config-${new Date().toISOString().split('T')[0]}.json`, + defaultPath: `stratosort-config-${toLocalCalendarDay(Date.now())}.json`, filters: [ { name: 'JSON Files', extensions: ['json'] }, { name: 'All Files', extensions: ['*'] } diff --git a/src/main/services/BatchAnalysisService.js b/src/main/services/BatchAnalysisService.js index 95b6a38..b26ffd3 100644 --- a/src/main/services/BatchAnalysisService.js +++ b/src/main/services/BatchAnalysisService.js @@ -131,17 +131,23 @@ class BatchAnalysisService { let concurrency = Math.max(2, Math.floor(cpuCores * 0.75)); // Reduce if memory pressure is high (>85% usage) - if (memUsage > 0.85) { + const underMemoryPressure = memUsage > 0.85; + if (underMemoryPressure) { concurrency = Math.max(2, Math.floor(concurrency * 0.5)); + } + + // Cap at global AI semaphore limit (3) to avoid holding memory while waiting for slots + concurrency = Math.min(concurrency, 3); + + // Logged after the cap so the number is the one that applies (it used to + // report the pre-cap value, e.g. 6 on a 16-core machine). + if (underMemoryPressure) { logger.warn('[BATCH-ANALYSIS] High memory usage detected, reducing concurrency', { memUsage: `${(memUsage * 100).toFixed(1)}%`, reducedConcurrency: concurrency }); } - // Cap at global AI semaphore limit (3) to avoid holding memory while waiting for slots - concurrency = Math.min(concurrency, 3); - logger.debug('[BATCH-ANALYSIS] Calculated optimal concurrency', { cpuCores, memUsage: `${(memUsage * 100).toFixed(1)}%`, @@ -739,8 +745,13 @@ class BatchAnalysisService { } catch (error) { // User cancellation: treat like the skip path (null result, progress // finalized) instead of recording a per-file failure for every file - // that was mid-inference when the user hit cancel. - if (error?.code === 'OPERATION_CANCELLED') { + // that was mid-inference when the user hit cancel. Document analysis + // rejects with an AbortError when its signal fires; that is the same + // cancel only while this run was asked to stop. + if ( + error?.code === 'OPERATION_CANCELLED' || + (error?.name === 'AbortError' && typeof isCancelled === 'function' && isCancelled()) + ) { logger.info('[BATCH-ANALYSIS] File analysis cancelled', { path: filePath, section }); return null; } diff --git a/src/main/services/DegradationManager.js b/src/main/services/DegradationManager.js index fe6e822..108f46a 100644 --- a/src/main/services/DegradationManager.js +++ b/src/main/services/DegradationManager.js @@ -135,6 +135,23 @@ class DegradationManager { // text so the patterns can match. const message = String(error?.message ?? error ?? '').toLowerCase(); + // The memory manager refused the load because the models already resident + // are in use. That is a sizing decision it has logged itself and callers + // handle; it used to fall through to the unknown-error branch below and + // be reported, with a stack, as an unclassified failure. + if ( + /** @type {any} */ (error)?.code === 'MODEL_MEMORY_BUDGET_EXCEEDED' || + message.includes('memory budget exceeded') + ) { + return { + action: 'none', + reason: 'memory_budget', + message: 'Not enough model memory while another model is in use.', + shouldNotifyUser: false, + originalError: error + }; + } + // Persistent GPU memory pressure errors should switch to CPU fallback. // Use the shared classifier so main and renderer stay aligned on which // signatures count as VRAM exhaustion. The legacy substring list below diff --git a/src/main/services/MediaTranscoder.js b/src/main/services/MediaTranscoder.js index 343f11b..ae7a566 100644 --- a/src/main/services/MediaTranscoder.js +++ b/src/main/services/MediaTranscoder.js @@ -345,9 +345,15 @@ async function transcodeToWhisperPcm(inputPath, options = {}) { child.stdout.on('data', (chunk) => { pcmBytes += chunk.length; if (pcmBytes > maxPcmBytes) { - const error = new Error( - `Media exceeds maximum supported duration (${maxDurationSeconds}s). ` + - `Split the file into smaller chunks before transcribing.` + // maxDurationSeconds marks this as a limit of the file itself, so + // callers record it once instead of retrying a transcode that + // buffers hours of audio only to fail the same way. + const error = Object.assign( + new Error( + `Media exceeds maximum supported duration (${maxDurationSeconds}s). ` + + `Split the file into smaller chunks before transcribing.` + ), + { maxDurationSeconds } ); fail(attachErrorCode(error, ERROR_CODES.INVALID_INPUT)); return; diff --git a/src/main/services/ModelMemoryManager.js b/src/main/services/ModelMemoryManager.js index 0705b49..02abeb4 100644 --- a/src/main/services/ModelMemoryManager.js +++ b/src/main/services/ModelMemoryManager.js @@ -47,6 +47,10 @@ class ModelMemoryManager { this._gpuInfo = options.gpuInfo || null; this._maxMemoryUsage = this._calculateMaxMemory(); this._currentMemoryUsage = 0; + // Loads in progress: memory that is being taken but is not resident yet + // (see reserveLoad). + /** @type {Map, settle: () => void }>} */ + this._pendingLoads = new Map(); // A pinned text or vision model can stay busy for a whole inference. // Queue the next load behind it instead of refusing after a few seconds // and turning the file into a stored fallback. @@ -211,19 +215,99 @@ class ModelMemoryManager { */ canLoadModel(/** @type {any} */ modelType) { const estimatedSize = this._modelSizeEstimates[modelType] || 0; - const projectedUsage = this._currentMemoryUsage + estimatedSize; + const projectedUsage = this._currentMemoryUsage + this._pendingLoadBytes() + estimatedSize; return projectedUsage < this._maxMemoryUsage; } + /** + * Count memory that a load in progress is about to take. + * + * A model used to enter the budget only once it had finished loading, and + * the vision runtime only once its health check had passed (about a minute + * on a laptop GPU). Whatever asked "does this fit?" in between was told + * the memory was free, so a text model and the vision runtime could be + * started half a second apart and sit together at twice the budget. + * + * A reservation cannot be evicted; callers that need its memory wait for + * it to end. It ends when the load becomes a resident (ensureModelLoaded, + * registerExternalConsumer) or fails. + * + * @param {string} type + * @param {number} sizeBytes + * @returns {() => void} ends the reservation; calling it again does nothing + */ + reserveLoad(type, sizeBytes) { + /** @type {() => void} */ + let settle = () => {}; + /** @type {Promise} */ + const settled = new Promise((resolve) => { + settle = resolve; + }); + const record = { sizeBytes: Math.max(0, Number(sizeBytes) || 0), settled, settle }; + const replaced = this._pendingLoads.get(type); + this._pendingLoads.set(type, record); + replaced?.settle(); + return () => { + if (this._pendingLoads.get(type) === record) this._pendingLoads.delete(type); + record.settle(); + }; + } + + /** + * Loads in progress that are not residents yet. A type that is both + * reserved and registered (the instant between the two) counts once, as + * the resident. + * @param {string} [excludeType] + * @returns {Array<[string, { sizeBytes: number, settled: Promise }]>} + */ + _pendingLoadEntries(excludeType) { + return [...this._pendingLoads].filter( + ([type]) => type !== excludeType && !this._loadedModels.has(type) + ); + } + + /** + * @param {string} [excludeType] + * @returns {number} + */ + _pendingLoadBytes(excludeType) { + let total = 0; + for (const [, pending] of this._pendingLoadEntries(excludeType)) total += pending.sizeBytes; + return total; + } + + /** + * Wait for the loads in progress to end, for at most the time a load + * waits on a pinned resident. + * @param {string} [excludeType] + * @returns {Promise} false when one is still in progress + */ + async _pendingLoadsSettled(excludeType) { + const waits = this._pendingLoadEntries(excludeType).map(([, pending]) => pending.settled); + if (waits.length === 0) return true; + if (!(this._pinnedResidentWaitMs > 0)) return false; + /** @type {ReturnType | undefined} */ + let timer; + const timedOut = new Promise((resolve) => { + timer = setTimeout(() => resolve(false), this._pinnedResidentWaitMs); + }); + try { + return await Promise.race([Promise.all(waits).then(() => true), timedOut]); + } finally { + clearTimeout(timer); + } + } + /** * Whether a consumer that loads outside this manager (the vision * llama-server child, registered afterwards as an external consumer) fits * the budget next to what is resident now. Mirrors ensureModelLoaded's * rule: with nothing resident a single oversized estimate is allowed * through (the runtime may still manage with mmap or GPU offload); with - * residents pinned by in-flight work the answer is honest. `excludeType` - * leaves out a registration the caller is about to replace, so a resident - * vision runtime is not counted twice. + * residents pinned by in-flight work the answer is honest. A load in + * progress counts like a resident. `excludeType` leaves out a registration + * or reservation the caller is about to replace, so a resident vision + * runtime is not counted twice. * * @param {number} sizeBytes * @param {{ excludeType?: string }} [options] @@ -232,9 +316,13 @@ class ModelMemoryManager { canFitExternalLoad(sizeBytes, { excludeType } = {}) { this._refreshMemoryBudget(); const excluded = excludeType ? this._loadedModels.get(excludeType) : null; - const residentCount = this._loadedModels.size - (excluded ? 1 : 0); + const residentCount = + this._loadedModels.size - (excluded ? 1 : 0) + this._pendingLoadEntries(excludeType).length; if (residentCount <= 0) return true; - const usage = this._currentMemoryUsage - (excluded ? excluded.sizeBytes || 0 : 0); + const usage = + this._currentMemoryUsage - + (excluded ? excluded.sizeBytes || 0 : 0) + + this._pendingLoadBytes(excludeType); return usage + Math.max(0, Number(sizeBytes) || 0) < this._maxMemoryUsage; } @@ -253,6 +341,12 @@ class ModelMemoryManager { */ async makeRoomForExternalLoad(sizeBytes, { excludeType } = {}) { while (!this.canFitExternalLoad(sizeBytes, { excludeType })) { + // A load in progress cannot be evicted. Wait for it to become a + // resident (or fail), then judge it like any other. + if (this._pendingLoadEntries(excludeType).length > 0) { + if (!(await this._pendingLoadsSettled(excludeType))) return false; + continue; + } const evictable = [...this._loadedModels.entries()] .filter(([type]) => type !== excludeType && (this._activeRefs.get(type) || 0) === 0) .sort((a, b) => (a[1].lastUsed || 0) - (b[1].lastUsed || 0)); @@ -281,7 +375,10 @@ class ModelMemoryManager { canRunOptionalLoad(modelType) { this._refreshMemoryBudget(); if (this._loadedModels.has(modelType)) { - const remaining = Math.max(0, this._maxMemoryUsage - this._currentMemoryUsage); + const remaining = Math.max( + 0, + this._maxMemoryUsage - this._currentMemoryUsage - this._pendingLoadBytes() + ); return remaining >= OPTIONAL_INFERENCE_HEADROOM_BYTES; } return this.canLoadModel(modelType); @@ -289,13 +386,14 @@ class ModelMemoryManager { /** * Ensure model is loaded, unloading others if necessary + * @returns {Promise} */ async ensureModelLoaded(/** @type {any} */ modelType, options = {}) { // Already loaded? If a dispose is mid-flight for this type, wait for it // to settle first: returning the context now would hand the caller a // context whose native teardown is in progress (use-after-dispose). If // the dispose fails, the entry stays tracked and is safe to return. - { + for (;;) { let entry = this._loadedModels.get(modelType); if (entry?.disposing) { await entry.disposePromise; @@ -305,6 +403,12 @@ class ModelMemoryManager { entry.lastUsed = Date.now(); return entry.context; } + // Another caller is loading this model (the load lock can be dropped + // from under a load that waits longer than its timeout). Wait for that + // load instead of starting a second copy; if it fails, try here. + const inFlight = this._pendingLoads.get(modelType); + if (!inFlight) break; + await inFlight.settled; } // Refresh budget from current system state before making load decisions @@ -320,22 +424,34 @@ class ModelMemoryManager { // embedding model "did not fit" beside the 6 GB vision runtime and the // runtime was shut down and restarted for every image of the batch. const besideType = /** @type {any} */ (options)?.besideResident; - const besideEntry = - typeof besideType === 'string' && /** @type {any} */ (options)?.gpuLayersOverride === 0 - ? this._loadedModels.get(besideType) - : null; + const cpuBeside = + typeof besideType === 'string' && /** @type {any} */ (options)?.gpuLayersOverride === 0; // keepBeside: the caller chose this CPU load so an idle resident (the // text model between files) stays; without it only a pinned one does. - const besidePinned = Boolean( - besideEntry && - ((this._activeRefs.get(besideType) || 0) > 0 || - /** @type {any} */ (options)?.keepBeside === true) - ); - const fits = () => - besidePinned - ? this._currentMemoryUsage - (besideEntry?.sizeBytes || 0) + estimatedSize < - this._maxMemoryUsage + // A resident that is still starting cannot be evicted and is pinned by + // the work that started it, so it is left out the same way. Read on + // every check: the resident can finish starting during the wait below. + const besideBytes = () => { + if (!cpuBeside) return 0; + const besideEntry = this._loadedModels.get(besideType); + if (!besideEntry) return this._pendingLoads.get(besideType)?.sizeBytes || 0; + const pinned = + (this._activeRefs.get(besideType) || 0) > 0 || + /** @type {any} */ (options)?.keepBeside === true; + return pinned ? besideEntry.sizeBytes || 0 : 0; + }; + const fits = () => { + // Loaded or being loaded by another caller since this one started + // waiting: nothing to make room for, the check before the load joins it. + if (this._loadedModels.has(modelType) || this._pendingLoads.has(modelType)) return true; + const beside = besideBytes(); + return beside > 0 + ? this._currentMemoryUsage + this._pendingLoadBytes() - beside + estimatedSize < + this._maxMemoryUsage : this.canLoadModel(modelType); + }; + // Residents, or loads in progress that will become residents. + const othersHoldMemory = () => this._loadedModels.size > 0 || this._pendingLoads.size > 0; while (!fits() && this._loadedModels.size > 0) { const sizeBefore = this._loadedModels.size; @@ -354,22 +470,27 @@ class ModelMemoryManager { // remaining residents are pinned by in-flight inference. Wait for those // refs to drain and retry eviction so a document queued behind a vision // batch (or an embedding queued behind a text inference) runs once the - // resident releases, instead of stacking a second multi-GB model. - if (!fits() && this._loadedModels.size > 0 && this._pinnedResidentWaitMs > 0) { + // resident releases, instead of stacking a second multi-GB model. A load + // in progress is waited out the same way: it becomes a resident that is + // evicted here once it is idle. + if (!fits() && othersHoldMemory() && this._pinnedResidentWaitMs > 0) { const deadline = Date.now() + this._pinnedResidentWaitMs; while (!fits() && Date.now() < deadline) { await delay(this._pinnedResidentPollMs); this._refreshMemoryBudget(); while (!fits() && this._loadedModels.size > 0) { const sizeBefore = this._loadedModels.size; - await this._unloadLeastRecentlyUsed(); + // Quiet: the pass above already logged that nothing can be + // evicted; repeating it every poll wrote a warning every 2 s for + // as long as the wait lasted. + await this._unloadLeastRecentlyUsed({ quiet: true }); if (this._loadedModels.size === sizeBefore) break; } } } if (!fits()) { - if (this._loadedModels.size > 0) { + if (othersHoldMemory()) { // Fail closed under concurrent pressure: stacking another multi-GB // model on top of pinned residents risks a native OOM that takes the // whole process down. Callers already handle load rejections. @@ -384,7 +505,8 @@ class ModelMemoryManager { estimatedSizeMB: Math.round(estimatedSize / 1024 / 1024), maxUsableMB: Math.round(this._maxMemoryUsage / 1024 / 1024), currentUsageMB: Math.round(this._currentMemoryUsage / 1024 / 1024), - loadedModels: Array.from(this._loadedModels.keys()) + loadedModels: Array.from(this._loadedModels.keys()), + pendingLoads: Array.from(this._pendingLoads.keys()) }); throw budgetError; } @@ -400,32 +522,45 @@ class ModelMemoryManager { }); } - // Load the model via the bound callback - const context = await this._loadModelFn(modelType, options); + // Another caller loaded this model, or began to, while this one waited + // for room: use that load. + if (this._loadedModels.has(modelType) || this._pendingLoads.has(modelType)) { + return this.ensureModelLoaded(modelType, options); + } - // The load callback may have refined the estimate from the real file - // size (updateModelSizeEstimate); record what this model actually - // costs so later budget decisions are not held to the fixed guess. - const recordedSize = this._modelSizeEstimates[modelType] || estimatedSize; - this._loadedModels.set(modelType, { - context, - lastUsed: Date.now(), - sizeBytes: recordedSize - }); - this._currentMemoryUsage += recordedSize; + // The memory counts as taken from here, not from when the load finishes. + const endReservation = this.reserveLoad(modelType, estimatedSize); + try { + // Load the model via the bound callback + const context = await this._loadModelFn(modelType, options); + + // The load callback may have refined the estimate from the real file + // size (updateModelSizeEstimate); record what this model actually + // costs so later budget decisions are not held to the fixed guess. + const recordedSize = this._modelSizeEstimates[modelType] || estimatedSize; + this._loadedModels.set(modelType, { + context, + lastUsed: Date.now(), + sizeBytes: recordedSize + }); + this._currentMemoryUsage += recordedSize; - logger.info('[Memory] Model loaded', { - type: modelType, - currentUsageMB: Math.round(this._currentMemoryUsage / 1024 / 1024) - }); + logger.info('[Memory] Model loaded', { + type: modelType, + currentUsageMB: Math.round(this._currentMemoryUsage / 1024 / 1024) + }); - return context; + return context; + } finally { + endReservation(); + } } /** * Unload least recently used model, skipping models with active references. + * @param {{ quiet?: boolean }} [options] quiet: no warning when nothing can be evicted */ - async _unloadLeastRecentlyUsed() { + async _unloadLeastRecentlyUsed({ quiet = false } = {}) { let oldest = null; let oldestTime = Infinity; @@ -441,7 +576,7 @@ class ModelMemoryManager { if (oldest) { await this.unloadModel(oldest); - } else { + } else if (!quiet) { logger.warn('[Memory] Cannot evict any model -- all loaded models are in active use'); } } @@ -607,6 +742,7 @@ class ModelMemoryManager { maxMemoryMB: Math.round(this._maxMemoryUsage / 1024 / 1024), currentUsageMB: Math.round(this._currentMemoryUsage / 1024 / 1024), loadedModels: Array.from(this._loadedModels.keys()), + pendingLoads: Array.from(this._pendingLoads.keys()), systemFreeMemoryMB: Math.round(os.freemem() / 1024 / 1024) }; } diff --git a/src/main/services/SmartFolderWatcher.js b/src/main/services/SmartFolderWatcher.js index ec2b387..72d4edf 100644 --- a/src/main/services/SmartFolderWatcher.js +++ b/src/main/services/SmartFolderWatcher.js @@ -140,6 +140,21 @@ function analysisRetryDelayMs(retryCount) { return Math.min(ANALYSIS_RETRY_MAX_DELAY_MS, ANALYSIS_RETRY_BASE_DELAY_MS * 4 ** exponent); } +/** + * A reanalysis the user asked for stays one when a newer item for the same + * file takes its place. Replaced by a plain file event, it would run as a + * content update (original name kept, folder kept) or be skipped as already + * analyzed. + * @param {any} replaced the item that gives way + * @param {any} replacement the item that stays in the queue + */ +function keepReanalysisRequest(replaced, replacement) { + if (replaced?.eventType !== 'reanalyze' || replacement.eventType === 'reanalyze') return; + replacement.eventType = 'reanalyze'; + replacement.applyNaming = replaced.applyNaming; + if (replaced.bulk) replacement.bulk = true; +} + // CRITICAL: Prevent unbounded memory growth under heavy file activity (e.g., large archive extraction). // We bound the analysis queue and apply light deduplication by filePath. const MAX_ANALYSIS_QUEUE_SIZE = LIMITS.ANALYSIS_QUEUE_SIZE ?? 1000; @@ -1440,6 +1455,7 @@ class SmartFolderWatcher { (q) => (q.dedupKey || q.filePath) === dedupKey || q.filePath === filePath ); if (existingIndex !== -1) { + keepReanalysisRequest(this.analysisQueue[existingIndex], item); this.analysisQueue[existingIndex] = item; this._scheduleQueuePersist(); return; @@ -2388,11 +2404,38 @@ class SmartFolderWatcher { try { await runInBatch(async () => { - for (const item of batch) { + for (const [index, item] of batch.entries()) { // A stop since this pass began ends the batch: the remaining // items are in the snapshot stop() flushed. Checked by epoch, // not only _isStopping, because stop() may already have returned. if (this._isStopping || epoch !== this._stopEpoch) break; + // A Discover analysis began during the batch. The hold above is + // only looked at before a batch starts, and a batch is up to 20 + // images: the ones not started go back to the front of the queue. + if (hasUserAnalysisInFlight()) { + // A file that changed meanwhile has a newer item in the queue + // already: that one stays, as enqueueing over it would. + const queuedByKey = new Map( + this.analysisQueue.map((queuedItem) => [procKey(queuedItem), queuedItem]) + ); + const rest = batch.slice(index); + const handedBack = []; + for (const waiting of rest) { + const newer = queuedByKey.get(procKey(waiting)); + if (newer) keepReanalysisRequest(waiting, newer); + else handedBack.push(waiting); + this._inFlightItems.delete(procKey(waiting)); + } + this.analysisQueue.unshift(...handedBack); + this._scheduleQueuePersist(); + logger.info( + '[SMART-FOLDER-WATCHER] Discover analysis started; image batch paused', + { + returned: rest.length + } + ); + break; + } try { await this._analyzeWithTimeout(item); } catch (error) { @@ -2710,20 +2753,22 @@ class SmartFolderWatcher { // Verify file still exists and capture stats for reuse below const fileStats = await fs.stat(filePath); - // Restored items skipped the _needsAnalysis gate that _handleFileEvent - // and the startup scan both apply — they came straight off disk. Between - // the crash and this restart the file may well have been analyzed - // another way (Discover, a manual run), so re-check now rather than - // spending a full LLM pass. Done lazily here, not at restore time, so a - // 50k-item backlog doesn't stat and hash every file up front. Explicit - // reanalyze requests intentionally bypass the gate. - if ((item.restored || item.recheck) && !isReanalyze) { + // Ask the history again, now that the file's turn has come. The answer + // from when it was queued can be minutes old: the queue is held while a + // Discover run is in progress (which may have analyzed this very file), + // a change event that arrived during our own analysis of the file was + // queued before that analysis was recorded, and restored items came + // straight off disk without ever being checked. One history lookup is + // cheap next to a model pass, and the file is statted here anyway. + // Explicit reanalyze requests bypass the gate, and an embedding retry + // already has its analysis in the history. + if (!isReanalyze && !item.cachedAnalysis) { const stillNeedsAnalysis = await this._needsAnalysis(filePath, item.mtime, fileStats); if (!stillNeedsAnalysis) { - logger.debug( - '[SMART-FOLDER-WATCHER] Restored item already analyzed elsewhere; skipping', - { filePath } - ); + logger.debug('[SMART-FOLDER-WATCHER] Already analyzed since it was queued; skipping', { + filePath, + eventType + }); return; } } @@ -3038,7 +3083,12 @@ class SmartFolderWatcher { processingTime: Number(result.processingTimeMs || result.processingTime || 0), modelType: isImagePath(filePath) ? 'vision' : 'llm', analysisHistory: this.analysisHistoryService, - logger + logger, + // The file as it was when this analysis started. Statting again + // here would stamp an analysis of the old content with the size + // and time of a save made while the model was running, and the + // gate above would then skip the file as up to date. + fileStats }); // This triggers a rebuild on the next search instead of waiting 15 minutes diff --git a/src/main/services/VisionService.js b/src/main/services/VisionService.js index 525032e..45a05f5 100644 --- a/src/main/services/VisionService.js +++ b/src/main/services/VisionService.js @@ -867,9 +867,25 @@ async function getAvailablePort() { }); } +/** + * The caller gave up before the request was sent; nothing reached the runtime. + * Named AbortError so LlamaResilience does not count it against the breaker. + */ +function createVisionAbortError() { + const err = new Error('Vision request aborted'); + err.name = 'AbortError'; + return err; +} + async function requestJson( /** @type {any} */ { method, port, path: endpointPath, body, timeoutMs, signal, apiKey } ) { + // A caller that has already given up gets its rejection without a request + // being created. This used to destroy a fresh request before its 'error' + // listener was attached: the error surfaced as an uncaught exception and + // the promise never settled, so analyzeImage kept its in-flight count and + // the runtime stayed pinned in memory until the app was restarted. + if (signal?.aborted) throw createVisionAbortError(); const payload = body ? JSON.stringify(body) : null; const options = { method, @@ -925,7 +941,20 @@ async function requestJson( settleReject(error); } }); + // A runtime that crashes or is stopped after its headers went out closes + // the response without 'end', and the request reports no error for it. + // Nothing else would settle this: the image would stay in flight and + // the batch would wait on it. A timeout or an abort has rejected already + // by the time this runs, so their reason is the one the caller sees. + res.on('close', () => { + if (!res.complete) { + settleReject(new Error('Vision response ended before it was complete')); + } + }); }); + // Attached before anything can destroy the request: a destroy with no + // 'error' listener is an uncaught exception and leaves this unsettled. + req.on('error', settleReject); if (timeoutMs) { req.setTimeout(timeoutMs, () => { @@ -934,10 +963,6 @@ async function requestJson( } if (signal) { - if (signal.aborted) { - req.destroy(new Error('Vision request aborted')); - return; - } // Removable listener: { once: true } only fires-and-removes on abort, // which means the success path leaves the closure attached to the // signal forever. Reused AbortControllers across many requests would @@ -949,7 +974,6 @@ async function requestJson( signal.addEventListener('abort', abortListener, { once: true }); } - req.on('error', settleReject); if (payload) req.write(payload); req.end(); }); @@ -1019,6 +1043,12 @@ class VisionService { // tear the server down while one is running (audit row ML-05), and the // memory manager must not evict it either: each request holds a ref. this._inFlightRequests = 0; + /** + * One entry per request in flight. `manager` is the memory manager whose + * ref this request holds, null while it holds none. + * @type {Set<{ manager: any }>} + */ + this._requestHolds = new Set(); /** @type {{ manager: any, type: string } | null} */ this._memoryRegistration = null; /** @type {any} */ @@ -1088,12 +1118,18 @@ class VisionService { * objection. Eviction under pressure runs shutdown(); a request in flight * pins the entry via acquireRef so eviction skips it. * @param {any} config + * @param {any} [child] the process the registration is for, when known */ - async _registerWithMemoryManager(config) { + async _registerWithMemoryManager(config, child) { const manager = this._getMemoryManager(); if (!manager || typeof manager.registerExternalConsumer !== 'function') return; if (this._memoryRegistration) return; const sizeBytes = await this.estimateRuntimeMemoryBytes(config); + // The runtime can go away while its files are measured (a crash, or a + // text load that took its memory). Registering it then would leave the + // budget charged for a process that no longer exists. + if (child && this._process !== child) return; + if (this._memoryRegistration) return; try { const registered = manager.registerExternalConsumer({ type: VISION_RUNTIME_MEMORY_TYPE, @@ -1108,8 +1144,10 @@ class VisionService { }); if (registered) { this._memoryRegistration = { manager, type: VISION_RUNTIME_MEMORY_TYPE }; - for (let i = 0; i < this._inFlightRequests; i += 1) { + for (const hold of this._requestHolds) { + if (hold.manager) continue; manager.acquireRef?.(VISION_RUNTIME_MEMORY_TYPE); + hold.manager = manager; } } } catch (error) { @@ -1132,14 +1170,33 @@ class VisionService { } } + /** + * Count a request as in flight and pin the runtime in the memory budget. + * + * The returned function ends the request. It releases the ref on the + * manager it was taken from, not on whatever is registered by then: a + * request that times out shuts the runtime down, which drops the + * registration before the request is over. Releasing through + * `_memoryRegistration` skipped the release in that case and the runtime + * stayed pinned, unevictable, for the rest of the session. + * @returns {() => void} ends the request; calling it again does nothing + */ _beginRequest() { + /** @type {{ manager: any }} */ + const hold = { manager: null }; + this._requestHolds.add(hold); this._inFlightRequests += 1; - this._memoryRegistration?.manager.acquireRef?.(this._memoryRegistration.type); - } - - _endRequest() { - this._inFlightRequests = Math.max(0, this._inFlightRequests - 1); - this._memoryRegistration?.manager.releaseRef?.(this._memoryRegistration.type); + const manager = this._memoryRegistration?.manager; + if (manager) { + manager.acquireRef?.(VISION_RUNTIME_MEMORY_TYPE); + hold.manager = manager; + } + return () => { + if (!this._requestHolds.delete(hold)) return; + this._inFlightRequests = Math.max(0, this._inFlightRequests - 1); + hold.manager?.releaseRef?.(VISION_RUNTIME_MEMORY_TYPE); + hold.manager = null; + }; } _extractDecodePressureHint() { @@ -1212,6 +1269,13 @@ class VisionService { this._idleShutdownTimer = setTimeout(() => { this._idleShutdownTimer = null; + // Idle means idle: a request still running when the timer fires (one + // image finished and armed it while another was mid-inference) keeps + // the runtime, and the timer starts over. + if (this._inFlightRequests > 0) { + this.scheduleIdleShutdown(reason); + return; + } // Do NOT track() here — shutdown() calls drain(), which would await // this very promise, creating a circular wait with a 5s timeout stall. this.shutdown().catch((error) => { @@ -1614,7 +1678,48 @@ class VisionService { return DEFAULT_STARTUP_TIMEOUT_MS; } - async _startServer(/** @type {any} */ config) { + /** + * Start the runtime with its memory counted from the first moment. + * + * The runtime entered the memory budget only after its health check passed + * (about a minute on a laptop GPU). A model load that asked for room during + * that minute was told the memory was free and loaded beside the runtime. + * The reservation ends when the start does: by then the runtime is + * registered as a resident, or it failed and holds nothing. + * @param {any} config + */ + async _startServer(config) { + // Before the reservation: with no runtime staged for this backend this + // is a download, and a model load would wait on memory that nothing is + // using yet. _spawnServer gets the same path back from the cache. + await this._ensureBinary(); + const endReservation = await this._reserveRuntimeMemory(config); + try { + await this._spawnServer(config); + } finally { + endReservation(); + } + } + + /** + * @param {any} config + * @returns {Promise<() => void>} ends the reservation + */ + async _reserveRuntimeMemory(config) { + const manager = this._getMemoryManager(); + if (!manager || typeof manager.reserveLoad !== 'function') return () => {}; + try { + const sizeBytes = await this.estimateRuntimeMemoryBytes(config); + return manager.reserveLoad(VISION_RUNTIME_MEMORY_TYPE, sizeBytes); + } catch (error) { + logger.debug('[VisionService] Memory reservation failed', { + error: /** @type {any} */ (error)?.message + }); + return () => {}; + } + } + + async _spawnServer(/** @type {any} */ config) { const binaryPath = await this._ensureBinary(); const port = await getAvailablePort(); this._apiKey = crypto.randomBytes(32).toString('hex'); @@ -1673,7 +1778,10 @@ class VisionService { // preventing both a stale listener and an unhandled promise rejection when the // process later exits normally. let earlyExitHandler; + /** @type {(error: Error) => void} */ + let failStartup = () => {}; const earlyExitPromise = new Promise((_, reject) => { + failStartup = reject; earlyExitHandler = (/** @type {any} */ code, /** @type {any} */ signal) => { abortController.abort(); // Signal health check to stop polling const stderr = stderrChunks.join('').slice(-500); @@ -1690,9 +1798,14 @@ class VisionService { logger.debug('[VisionService] Early exit promise rejected', { error: e.message }); }); - // Handle spawn-level errors (e.g. ENOENT when binary doesn't exist) + // Handle spawn-level errors (e.g. ENOENT when binary doesn't exist). A + // child that never started may emit no 'exit', so this has to end the + // start itself; otherwise the health check polls a dead port until the + // startup timeout. Once the start has settled this is only a log line. child.on('error', (err) => { logger.error('[VisionService] Failed to spawn vision runtime', { error: err.message }); + abortController.abort(); + failStartup(new Error(`Vision runtime failed to start: ${err.message}`)); }); // Clean up state when the process exits (for both startup failures and runtime crashes) @@ -1751,7 +1864,7 @@ class VisionService { // spurious rejection when the process later exits normally. child.removeListener('exit', earlyExitHandler); this._isReady = true; - await this._registerWithMemoryManager(config); + await this._registerWithMemoryManager(config, child); // Multimodal capability probe. When the model declares mmprojRequired, // llama-server emits `clip_model_load:` lines during projector init. @@ -1901,6 +2014,9 @@ class VisionService { throw new Error('Vision model not found: missing multimodal projector'); } + // Cancelled while it waited its turn: do not start a runtime for it. + if (signal?.aborted) throw createVisionAbortError(); + // Fail fast while a recent startup failure is inside its cooldown so a // queued batch degrades to OCR-only in milliseconds (the message contains // "vision runtime" on purpose — imageAnalysis.js routes it through @@ -1926,6 +2042,51 @@ class VisionService { } this._clearIdleShutdownTimer(); + // In flight from here, not from the HTTP request. A text load that finds + // the runtime idle shuts it down for its memory; counting only the HTTP + // request left a gap (runtime just became ready, image file still being + // read) in which the runtime was taken from under the image that had + // started it, and the image fell back to OCR. + const endRequest = this._beginRequest(); + try { + return await this._analyzeImageWithRuntime({ + imageBase64, + imagePath, + prompt, + systemPrompt, + maxTokens, + temperature, + timeoutMs, + signal, + config + }); + } finally { + endRequest(); + // Every way out re-arms the idle timer cleared above. A failure between + // the runtime starting and the request going out (image file gone, a + // cancel) used to skip this and leave the runtime resident for good. + this.scheduleIdleShutdown('analyzeImage'); + } + } + + /** + * analyzeImage after its cheap checks: start or reuse the runtime, then + * send the request. + * @param {any} request + */ + async _analyzeImageWithRuntime(request) { + const { + imageBase64, + imagePath, + prompt, + systemPrompt, + maxTokens, + temperature, + timeoutMs, + signal, + config + } = request; + try { await this._ensureServer(config); } catch (err) { @@ -1952,6 +2113,10 @@ class VisionService { throw err; } + // Cancelled while the runtime was starting, which can take a minute. The + // runtime itself is healthy, so it stays up for the next image. + if (signal?.aborted) throw createVisionAbortError(); + // If the startup probe determined the runtime can't actually decode // images, fail fast with a clear error rather than letting llama-server // return its generic "Invalid url value" 500 — that response burns a @@ -2003,7 +2168,10 @@ class VisionService { const dataUrl = `data:${mimeType};base64,${imageData}`; const requestTimeoutMs = normalizeTimeoutMs(timeoutMs, DEFAULT_REQUEST_TIMEOUT_MS); - this._beginRequest(); + // Last check before the request goes out (the image file was read since + // the previous one). Past this point an abort interrupts a running + // inference and the catch below restarts the runtime. + if (signal?.aborted) throw createVisionAbortError(); try { const response = await requestJson({ method: 'POST', @@ -2066,9 +2234,6 @@ class VisionService { } } throw error; - } finally { - this._endRequest(); - this.scheduleIdleShutdown('analyzeImage'); } } @@ -2205,6 +2370,7 @@ module.exports = { _createUtf8ChunkDecoder: createUtf8ChunkDecoder, _detectBase64Mime: detectBase64Mime, _detectMimeFromPath: detectMimeFromPath, + _requestJson: requestJson, confirmGpuBackend, // Exposed so WhisperService can locate the on-disk vision runtime root // (which holds the bundled CUDA 12 runtime DLLs whisper.node depends on) diff --git a/src/main/services/WhisperService.js b/src/main/services/WhisperService.js index c6af321..ef67189 100644 --- a/src/main/services/WhisperService.js +++ b/src/main/services/WhisperService.js @@ -668,6 +668,32 @@ function whisperVariantFor(gpuInfo) { return []; } +/** + * Wait for a transcription another caller started, for as long as this + * caller wants to wait: its own cancel ends the wait and leaves the pass + * running for the caller that started it. + * @param {Promise} pending + * @param {AbortSignal} [signal] + * @returns {Promise} + */ +async function waitUnlessAborted(pending, signal) { + if (!signal) return pending; + const cancelled = () => + attachErrorCode(new Error('Transcription cancelled'), ERROR_CODES.OPERATION_CANCELLED); + if (signal.aborted) throw cancelled(); + /** @type {() => void} */ + let onAbort = () => {}; + const aborted = new Promise((_resolve, reject) => { + onAbort = () => reject(cancelled()); + signal.addEventListener('abort', onAbort, { once: true }); + }); + try { + return await Promise.race([pending, aborted]); + } finally { + signal.removeEventListener('abort', onAbort); + } +} + class WhisperService extends EventEmitter { /** * @param {Object} [options] @@ -1181,11 +1207,21 @@ class WhisperService extends EventEmitter { // Skipped when useCache is false because a forced re-analysis caller // explicitly wants its own pass; joining an in-flight call would // return a stale (cached-by-the-other-caller) transcript. - const pending = this._pendingTranscriptions.get(cacheKey); - if (pending) { - const result = await pending; - // Clone: callers may mutate (e.g., normalize for downstream stages). - return { ...result, cached: true }; + let pending = this._pendingTranscriptions.get(cacheKey); + while (pending) { + try { + const result = await waitUnlessAborted(pending, signal); + // Clone: callers may mutate (e.g., normalize for downstream stages). + return { ...result, cached: true }; + } catch (error) { + // The pass runs under the signal of the caller that started it + // (the watcher, say, which yields to a Discover run). That caller + // stopping is not this one stopping: join whoever took the file + // over, or run the pass below. + if (error?.code !== ERROR_CODES.OPERATION_CANCELLED || signal?.aborted) throw error; + } + const next = this._pendingTranscriptions.get(cacheKey); + pending = next === pending ? undefined : next; } } @@ -1277,7 +1313,13 @@ class WhisperService extends EventEmitter { // .catch(() => {}) absorption is essential — without it Node logs an // unhandled-rejection warning when the awaiter handles it but the // tap-handler (this finally) is "additional" from the runtime's view. - work.finally(() => this._pendingTranscriptions.delete(cacheKey)).catch(() => {}); + work + .finally(() => { + if (this._pendingTranscriptions.get(cacheKey) === work) { + this._pendingTranscriptions.delete(cacheKey); + } + }) + .catch(() => {}); } return work; } diff --git a/src/main/services/analysisHistory/queries.js b/src/main/services/analysisHistory/queries.js index ebe9aef..50ef295 100644 --- a/src/main/services/analysisHistory/queries.js +++ b/src/main/services/analysisHistory/queries.js @@ -273,17 +273,19 @@ function getAnalysisByDateRange(analysisHistory, analysisIndex, startDate, endDa const start = new Date(startDate); const end = new Date(endDate); - // Get all month keys in range + // Get all month keys in range. The index is keyed by the UTC month of each + // entry's timestamp, so the walk steps through UTC months too: stepping + // local months and reading the UTC key skipped the first or last month of + // the range away from Greenwich. const monthKeys = []; - const current = new Date(start); - current.setDate(1); // Start of month + const current = new Date(Date.UTC(start.getUTCFullYear(), start.getUTCMonth(), 1)); while (current <= end) { const key = current.toISOString().substring(0, 7); // YYYY-MM if (analysisIndex.dateIndex[key]) { monthKeys.push(key); } - current.setMonth(current.getMonth() + 1); + current.setUTCMonth(current.getUTCMonth() + 1); } // Collect entry IDs from relevant months diff --git a/src/main/services/mediaAnalyzerAdapter.js b/src/main/services/mediaAnalyzerAdapter.js index 7457d24..4db9433 100644 --- a/src/main/services/mediaAnalyzerAdapter.js +++ b/src/main/services/mediaAnalyzerAdapter.js @@ -52,6 +52,34 @@ function buildSkipResult(filePath) { }; } +/** + * A file longer than the transcoder accepts. Recorded as a format the build + * cannot read: nothing about it changes on a retry, and each attempt decodes + * the audio up to the limit before failing. + * @param {string} filePath + * @param {number} maxDurationSeconds + * @returns {any} + */ +function buildTooLongResult(filePath, maxDurationSeconds) { + const kind = classifyMediaFile(filePath) || 'audio'; + const hours = Math.round((maxDurationSeconds / 3600) * 10) / 10; + const limit = hours >= 1 ? `${hours} hours` : `${Math.round(maxDurationSeconds)} seconds`; + return { + ...buildSkipResult(filePath), + keywords: [kind, 'too-long'], + extractionMethod: 'format_unsupported', + purpose: `Media is longer than the supported ${limit}; not transcribed.` + }; +} + +function isTooLongError(/** @type {any} */ err) { + return ( + Boolean(err) && + err.code === ERROR_CODES.INVALID_INPUT && + Number.isFinite(err.maxDurationSeconds) + ); +} + function isWhisperMissingError(/** @type {any} */ err) { if (!err) return false; if (err.code === ERROR_CODES.LLAMA_MODEL_NOT_FOUND) return true; @@ -135,6 +163,10 @@ function combineSignals(sources) { * "skipped" result so downstream code (recordAnalysisResult, embedding * queue, vector store) flows unchanged. `SERVICE_UNAVAILABLE` from * in-flight shutdown still propagates. + * + * A file longer than the transcoder's duration limit gets a + * `format_unsupported` result the same way, so it is recorded once and not + * retried. */ /** * Whether to caption video keyframes for this analysis. The same switch @@ -207,6 +239,13 @@ function createMediaAnalyzerAdapter({ whisperService, captionKeyframes } = {}) { }); return buildSkipResult(filePath); } + if (isTooLongError(err)) { + logger.warn('[MediaAdapter] Media longer than the transcription limit; not transcribed', { + file: path.basename(filePath || ''), + maxDurationSeconds: err.maxDurationSeconds + }); + return buildTooLongResult(filePath, err.maxDurationSeconds); + } throw err; } finally { // Tear down the cancel-poll interval whether we resolved or threw. diff --git a/src/renderer/components/NavigationBar.jsx b/src/renderer/components/NavigationBar.jsx index a115fe6..9219a02 100644 --- a/src/renderer/components/NavigationBar.jsx +++ b/src/renderer/components/NavigationBar.jsx @@ -331,12 +331,16 @@ const VIEW_MODE_OPTIONS = [ { mode: 'dashboard', label: 'Dashboard', Icon: LayoutGrid, title: 'Dashboard overview' } ]; +const DASHBOARD_WAITS_FOR_ANALYSIS = + 'Analysis is still running. The dashboard opens when it finishes.'; + /** * Segmented control for switching between the guided phased flow and the * dashboard overview. Subscribes to ui.viewMode directly and dispatches - * setViewMode, so it stays interactive during operations — switching surfaces - * is a presentation choice, not a phase transition, and must never be gated by - * isBlockedByOperation the way the phase tabs are. + * setViewMode, so it is not gated by isBlockedByOperation the way the phase + * tabs are: the way back to Guided is always open. The way into the dashboard + * waits for a running analysis, because Discover owns the run and App.js + * unmounts Discover to show the dashboard, which would stop it. */ const ViewModeToggle = memo(function ViewModeToggle() { const dispatch = useAppDispatch(); @@ -349,6 +353,7 @@ const ViewModeToggle = memo(function ViewModeToggle() { const dashboardAvailable = useAppSelector((state) => hasDestinationFolder(state.files.smartFolders) ); + const isAnalyzing = useAppSelector((state) => Boolean(state.analysis?.isAnalyzing)); // Single stable handler reads the target mode off the button's data-* attr, // avoiding a fresh inline closure per option in the render loop. @@ -370,13 +375,17 @@ const ViewModeToggle = memo(function ViewModeToggle() { > {VIEW_MODE_OPTIONS.map(({ mode, label, Icon, title }) => { const isActive = viewMode === mode; - const isDisabled = mode === 'dashboard' && !dashboardAvailable; + const needsFolder = mode === 'dashboard' && !dashboardAvailable; + const waitsForAnalysis = mode === 'dashboard' && !isActive && isAnalyzing; + const isDisabled = needsFolder || waitsForAnalysis; // One descriptive string drives both the hover tooltip and the // accessible name, so the control is self-describing at every width — // including when the visible text label is collapsed to icon-only. - const describedLabel = isDisabled + const describedLabel = needsFolder ? 'Add a smart folder in Setup to open the dashboard.' - : title; + : waitsForAnalysis + ? DASHBOARD_WAITS_FOR_ANALYSIS + : title; return (