Skip to content

fix(analysis): the run outlives leaving Discover; a queued run is not cancelled by auto-advance #115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
iLevyTate merged 1 commit into from
Nov 11, 2026
+580 −208
Merged

fix(analysis): the run outlives leaving Discover; a queued run is not cancelled by auto-advance #115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 14 additions & 15 deletions src/renderer/components/NavigationBar.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ const NavTab = memo(function NavTab({
title={
!canNavigate && !isActive
? blockReason ||
'Navigation is paused while analysis or file moves run. Wait for the step to finish, or cancel from the active phase.'
'Navigation is paused while files are being moved. Wait for the move to finish, or cancel from the active phase.'
: `Step ${(metadata?.step ?? 0) + 1} of ${PHASE_ORDER.length} • ${metadata?.description || metadata?.title}`
}
style={{ WebkitAppRegion: 'no-drag' }}
Expand Down Expand Up @@ -339,8 +339,9 @@ const DASHBOARD_WAITS_FOR_ANALYSIS =
* dashboard overview. Subscribes to ui.viewMode directly and dispatches
* 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.
* waits for a running analysis: the run lives in AnalysisRunProvider inside
* PhaseRenderer, and App.js replaces PhaseRenderer with the dashboard, which
* would stop it. (Phase tabs do not wait: the run outlives a phase change.)
*/
const ViewModeToggle = memo(function ViewModeToggle() {
const dispatch = useAppDispatch();
Expand Down Expand Up @@ -842,9 +843,10 @@ function NavigationBar() {
};
}, []);

// Keep top-level navigation responsive; analysis state can become stale.
const isBlockedByOperation =
isOrganizing || isLoading || Boolean(isAnalyzing) || Boolean(isUndoRedoExecuting);
// File moves, loading and undo/redo pause the step tabs. A running analysis
// does not: the run lives in AnalysisRunProvider above the phases, so the
// user can organize finished files while the rest keep analyzing.
const isBlockedByOperation = isOrganizing || isLoading || Boolean(isUndoRedoExecuting);

// Phase navigation handler
const handlePhaseChange = useCallback(
Expand All @@ -871,9 +873,7 @@ function NavigationBar() {
showInfo(
isOrganizing
? 'Files are still moving. Stay on this step until the move finishes or cancel from Organize.'
: isAnalyzing
? 'Analysis is still running. You can keep reviewing this step while processing finishes.'
: 'Navigation is paused until the current operation finishes.',
: 'Navigation is paused until the current operation finishes.',
5200
);
return;
Expand All @@ -891,11 +891,11 @@ function NavigationBar() {
showInfo(
isOrganizing
? 'Navigation is paused while files are moving. You can still open Settings from the gear.'
: 'Navigation is paused while analysis is running. You can still open Settings from the gear.',
: 'Navigation is paused until the current operation finishes. You can still open Settings from the gear.',
5200
);
},
[actions, currentPhase, isAnalyzing, isBlockedByOperation, isOrganizing, showInfo]
[actions, currentPhase, isBlockedByOperation, isOrganizing, showInfo]
);

// Settings handler
Expand All @@ -905,7 +905,8 @@ function NavigationBar() {

// Return to the dashboard the user deep-linked out of. A view switch, not a
// phase transition, so moves and undo/redo do not gate it. A running
// analysis does: it belongs to Discover, which unmounts for the dashboard.
// analysis does: its provider lives in PhaseRenderer, which the dashboard
// replaces.
const handleReturnToDashboard = useCallback(() => {
if (isAnalyzing) {
showInfo(DASHBOARD_WAITS_FOR_ANALYSIS, 5200);
Expand Down Expand Up @@ -1052,9 +1053,7 @@ function NavigationBar() {
? executionStatus?.label
? `${executionStatus.label} Step navigation is paused until file moves finish.`
: 'Step navigation is paused while undo/redo moves files.'
: isAnalyzing
? 'Step navigation is paused while analysis is running.'
: 'Step navigation is paused. Please wait for the current operation to finish.'}
: 'Step navigation is paused. Please wait for the current operation to finish.'}
</Text>
</div>
)}
Expand Down
7 changes: 5 additions & 2 deletions src/renderer/components/PhaseRenderer.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { LazyLoadingSpinner } from './ui/LoadingSkeleton';
import { logger } from '../../shared/logger';
import { PHASES, PHASE_METADATA } from '../../shared/constants';
import perfMarks from '../../shared/perfMarks';
import { AnalysisRunProvider } from '../phases/discover/AnalysisRunProvider';

// The settings overlay and useKeyboardShortcuts are deliberately NOT here:
// they live in App.js so they stay active on the dashboard surface, which
Expand Down Expand Up @@ -222,8 +223,10 @@ function PhaseRenderer() {
}
};

// AnalysisRunProvider wraps every phase so an analysis run outlives the
// Discover screen: the user can organize finished files mid-run.
return (
<>
<AnalysisRunProvider>
<PhaseEnterAnnouncement currentPhase={currentPhase} />
<div className="flex flex-col w-full h-full">
<Suspense fallback={<LazyLoadingSpinner message={getPhaseLoadingMessage(currentPhase)} />}>
Expand All @@ -242,7 +245,7 @@ function PhaseRenderer() {
</AnimatePresence>
</Suspense>
</div>
</>
</AnalysisRunProvider>
);
}

Expand Down
134 changes: 18 additions & 116 deletions src/renderer/phases/DiscoverPhase.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,12 @@ import {
AnalysisProgress
} from '../components/discover';
import { FileListSkeleton } from '../components/ui/LoadingSkeleton';
import store from '../store';
import { useAppSelector } from '../store/hooks';
import { selectOrganizedCount } from '../store/selectors';
import { toCanonicalPath } from '../utils/canonicalPath';
import {
useDiscoverState,
useAnalysis,
useAnalysisRun,
useFileHandlers,
useFileActions,
getFileStateDisplayInfo
Expand All @@ -46,13 +45,9 @@ function DiscoverPhase() {
dateFormat,
caseConvention,
separator,
namingSettings,
setSelectedFiles,
addSelectedFiles,
setAnalysisResults,
setIsAnalyzing,
setAnalysisProgress,
setCurrentAnalysisFile,
setNamingConvention,
setDateFormat,
setCaseConvention,
Expand All @@ -61,7 +56,6 @@ function DiscoverPhase() {
updateFileState,
resetAnalysisState,
actions,
getCurrentPhase,
organizedFiles,
organizedPathIndex,
fileStats
Expand All @@ -72,7 +66,6 @@ function DiscoverPhase() {
const { openSearchModal } = useFloatingSearch();

const [showNamingSettings, setShowNamingSettings] = useState(false);
const [totalAnalysisFailure, setTotalAnalysisFailure] = useState(false);
const [showEmbeddingPrompt, setShowEmbeddingPrompt] = useState(false);
const [isRebuildingEmbeddings, setIsRebuildingEmbeddings] = useState(false);
const isMountedRef = useRef(true);
Expand Down Expand Up @@ -280,45 +273,17 @@ function DiscoverPhase() {
// and it's never set in this context, so use a stable empty object
const stablePhaseData = useRef({}).current;

const extendedActions = useMemo(
() => ({
...actions,
setPhaseData: (key, value) => {
if (key === 'totalAnalysisFailure') {
setTotalAnalysisFailure(value);
} else {
actions.setPhaseData(key, value);
}
}
}),
[actions]
);

// The run lives in AnalysisRunProvider (PhaseRenderer), not here, so it
// keeps going when the user moves to Organize or Complete.
const {
analyzeFiles,
cancelAnalysis,
clearAnalysisQueue,
retryFailedFiles,
pruneBatchResultsForPath
} = useAnalysis({
selectedFiles,
fileStates,
analysisResults,
isAnalyzing,
analysisProgress,
namingSettings,
setters: {
setIsAnalyzing,
setAnalysisProgress,
setCurrentAnalysisFile,
setAnalysisResults,
setFileStates
},
updateFileState,
addNotification,
actions: extendedActions,
getCurrentPhase
});
pruneBatchResultsForPath,
totalAnalysisFailure,
setTotalAnalysisFailure
} = useAnalysisRun();

const {
isScanning,
Expand Down Expand Up @@ -354,7 +319,6 @@ function DiscoverPhase() {
);

const { isDragging, dragProps } = useDragAndDrop(handleFileDrop);
const initialStuckCheckRef = useRef(false);

useSettingsSubscription(
useCallback(
Expand Down Expand Up @@ -447,67 +411,7 @@ function DiscoverPhase() {
};
}, []);

const analysisProgressRef = useRef(analysisProgress);
analysisProgressRef.current = analysisProgress;

useEffect(() => {
if (initialStuckCheckRef.current) return;
initialStuckCheckRef.current = true;

if (isAnalyzing) {
const lastActivity = analysisProgress?.lastActivity || Date.now();
const timeSinceActivity = Date.now() - lastActivity;
const isStuck = timeSinceActivity > 2 * 60 * 1000;

if (isStuck) {
logger.info('Detected stuck analysis state on mount, resetting');
resetAnalysisState('Stuck analysis detected on mount');
}
}
}, [isAnalyzing, analysisProgress, resetAnalysisState]);

useEffect(() => {
if (!isAnalyzing) return;

const checkStalled = () => {
// Use Redux store directly to prevent stale reads when the window is minimized
// (React suspends rendering when minimized, so refs bound to props become stale)
const state = store.getState();
const isActuallyAnalyzing = state.analysis.isAnalyzing;

if (!isActuallyAnalyzing) return; // Already finished, ignore

const progress = state.analysis.analysisProgress || analysisProgressRef.current;
const lastActivity = progress?.lastActivity || Date.now();
const timeSinceActivity = Date.now() - lastActivity;
const current = progress?.current || 0;
const total = progress?.total || 0;

if (current === 0 && total > 0 && timeSinceActivity > TIMEOUTS.STUCK_ANALYSIS_CHECK) {
addNotification('Analysis paused. Restarting...', 'info', 3000, 'analysis-stalled');
resetAnalysisState('Analysis stalled with no progress after 2 minutes');
return;
}

if (timeSinceActivity > TIMEOUTS.ANALYSIS_LOCK) {
addNotification('Analysis timed out. Ready to retry.', 'info', 3000, 'analysis-auto-reset');
resetAnalysisState('Stuck analysis state after 5 minutes of inactivity');
}
};

// Defer state updates so the interval handler returns quickly (avoids Long Task violations)
const checkStalledDeferred = () => {
setTimeout(checkStalled, 0);
};

// Check immediately on dependency change
checkStalled();

// Also check periodically in case dependencies stop updating (the exact stall scenario).
// Defer so the interval handler returns quickly (avoids Long Task violations).
const intervalId = setInterval(checkStalledDeferred, TIMEOUTS.SERVICE_STARTUP);
return () => clearInterval(intervalId);
}, [isAnalyzing, addNotification, resetAnalysisState]);
// Stuck-run checks live in AnalysisRunProvider with the run itself.

const fileStatesRef = useRef(fileStates);
fileStatesRef.current = fileStates;
Expand Down Expand Up @@ -938,27 +842,25 @@ function DiscoverPhase() {
// one analyzed file, so there would be nothing to organize there.
// Organized history also opens Organize, the same rule as the tab
// (uiSlice getPhaseNavigation), so a returning user can reach it.
// A running analysis does not hold this back: the run lives above
// the phases (AnalysisRunProvider), so files already analyzed can be
// organized while the rest keep analyzing.
const canContinue = visibleReadyCount > 0 || organizedCount > 0;
const disabledBecauseAnalyzing = isAnalyzing;
const disabledBecauseNoReady = !canContinue;

let disabledReason = '';
if (disabledBecauseAnalyzing) {
disabledReason = 'Analysis is in progress.';
} else if (disabledBecauseNoReady) {
disabledReason =
visibleFailedCount > 0
if (!canContinue) {
disabledReason = isAnalyzing
? 'Organize opens as soon as the first file is analyzed.'
: visibleFailedCount > 0
? 'All files failed analysis. Retry them, or clear the queue and try again.'
: 'Analyze at least one file first.';
} else if (isAnalyzing) {
disabledReason = 'Analysis keeps running while you organize the files that are ready.';
}

return (
<Button
onClick={() => {
if (isAnalyzing) {
addNotification('Please wait for analysis to complete', 'warning', 3000);
return;
}
if (!canContinue) {
addNotification(
visibleFailedCount > 0
Expand All @@ -977,7 +879,7 @@ function DiscoverPhase() {
variant="primary"
size="md"
className="w-full sm:w-auto min-w-[180px]"
disabled={isAnalyzing || !canContinue}
disabled={!canContinue}
title={disabledReason || undefined}
>
Continue to Organize →
Expand Down
Loading
Loading