diff --git a/app/src/components/EditorArea.tsx b/app/src/components/EditorArea.tsx index 0f592fc1..86fccdd6 100644 --- a/app/src/components/EditorArea.tsx +++ b/app/src/components/EditorArea.tsx @@ -18,7 +18,11 @@ import type { RunMode } from '@/lib/project-store'; interface EditorAnalysisState { isAnalyzing: boolean; error: string | null; - runAnalysis: (activeFileContent?: string, activeFilePath?: string) => Promise; + runAnalysis: ( + activeFileContent?: string, + activeFilePath?: string, + runModeOverride?: RunMode + ) => Promise; setError: (error: string | null) => void; } @@ -207,15 +211,9 @@ export function EditorArea({ const handleAnalyzeActiveOnly = useCallback(() => { if (activeFile && currentProject) { - // Temporarily switch to 'current' mode for this run - const originalMode = currentProject.runMode; - setRunMode(currentProject.id, 'current'); - runAnalysis(activeFile.content, activeFile.path).finally(() => { - // Restore original mode after analysis - setRunMode(currentProject.id, originalMode); - }); + runAnalysis(activeFile.content, activeFile.path, 'current'); } - }, [activeFile, currentProject, runAnalysis, setRunMode]); + }, [activeFile, currentProject, runAnalysis]); // Keyboard shortcuts for running analysis const analysisShortcuts = useMemo( diff --git a/app/src/hooks/__tests__/useAnalysis.test.tsx b/app/src/hooks/__tests__/useAnalysis.test.tsx index d0029442..dfe6cf85 100644 --- a/app/src/hooks/__tests__/useAnalysis.test.tsx +++ b/app/src/hooks/__tests__/useAnalysis.test.tsx @@ -6,6 +6,7 @@ import { buildAnalysisCacheKey } from '@/lib/analysis-hash'; import { PROACTIVE_ANALYSIS_CACHE_KEY_MAX_CHARS } from '@/lib/analysis-cache-policy'; import { useAnalysisStore } from '@/lib/analysis-store'; import type { Project } from '@/lib/project-store'; +import { AnalysisError, AnalysisErrorCode } from '@/types'; const lineageActions = vi.hoisted(() => ({ setResult: vi.fn(), @@ -16,21 +17,31 @@ const lineageActions = vi.hoisted(() => ({ let currentProject: Project | null = null; let activeProjectId: string | null = null; +let hideCTEs = false; +let backendSchema: unknown = null; +let showLintIssues = false; vi.mock('@flowscope-react/store', () => ({ - useLineageState: (selector: (state: { hideCTEs: boolean }) => unknown) => - selector({ hideCTEs: false }), + useLineageState: (selector: (state: { hideCTEs: boolean }) => unknown) => selector({ hideCTEs }), useLineageActions: () => lineageActions, })); vi.mock('@/lib/project-store', () => ({ - useProject: () => ({ currentProject, activeProjectId, backendSchema: null }), + useProject: () => ({ currentProject, activeProjectId, backendSchema }), })); vi.mock('@/lib/view-state-store', () => ({ - useViewStateStore: (selector: (state: { getViewState: () => undefined }) => unknown) => - selector({ getViewState: () => undefined }), - getIssuesStateWithDefaults: () => ({ showLintIssues: false }), + useViewStateStore: ( + selector: (state: { + viewStates: Record; + }) => unknown + ) => + selector({ + viewStates: activeProjectId ? { [activeProjectId]: { issues: { showLintIssues } } } : {}, + }), + getIssuesStateWithDefaults: (issues?: { showLintIssues: boolean }) => ({ + showLintIssues: issues?.showLintIssues ?? false, + }), })); import { useAnalysis } from '../useAnalysis'; @@ -92,6 +103,9 @@ function createAdapter(analyze = vi.fn(), type: BackendAdapter['type'] = 'wasm') describe('useAnalysis memory cache', () => { beforeEach(() => { activeProjectId = 'project-1'; + hideCTEs = false; + backendSchema = null; + showLintIssues = false; currentProject = createProject( activeProjectId, 'x'.repeat(PROACTIVE_ANALYSIS_CACHE_KEY_MAX_CHARS + 1) @@ -234,6 +248,579 @@ describe('useAnalysis memory cache', () => { expect(useAnalysisStore.getState().getResult(projectA.id, keyA)).toBeNull(); }); + it('batches rapid edits and project switches before proactive file sync', async () => { + vi.useFakeTimers(); + currentProject = createProject('project-a', 'SELECT 1'); + activeProjectId = currentProject.id; + const adapter = createAdapter(); + const { rerender } = renderHook(() => useAnalysis(true, { adapter })); + + await act(async () => { + await Promise.resolve(); + }); + expect(adapter.syncFiles).toHaveBeenCalledTimes(1); + vi.mocked(adapter.syncFiles).mockClear(); + + act(() => { + currentProject = createProject('project-a', 'SELECT 12'); + rerender(); + currentProject = createProject('project-b', 'SELECT 123'); + activeProjectId = currentProject.id; + rerender(); + currentProject = createProject('project-c', 'SELECT 1234'); + activeProjectId = currentProject.id; + rerender(); + }); + + await vi.advanceTimersByTimeAsync(299); + expect(adapter.syncFiles).not.toHaveBeenCalled(); + + await act(async () => { + await vi.advanceTimersByTimeAsync(1); + }); + expect(adapter.syncFiles).toHaveBeenCalledTimes(1); + expect(adapter.syncFiles).toHaveBeenCalledWith([{ name: 'model.sql', content: 'SELECT 1234' }]); + }); + + it('discards an analysis result when project inputs change in flight', async () => { + const projectBeforeEdit = createProject('project-1', 'SELECT 1'); + const cacheKey = buildProjectCacheKey(projectBeforeEdit); + currentProject = projectBeforeEdit; + activeProjectId = currentProject.id; + + let resolveAnalysis!: (value: { + result: AnalyzeResult; + cacheKey: string; + cacheHit: boolean; + skipped: boolean; + timings: null; + }) => void; + const pendingAnalysis = new Promise[0]>((resolve) => { + resolveAnalysis = resolve; + }); + const adapter = createAdapter(vi.fn(() => pendingAnalysis)); + const { result, rerender } = renderHook(() => useAnalysis(true, { adapter })); + lineageActions.setResult.mockClear(); + + let runPromise!: Promise; + await act(async () => { + runPromise = result.current.runAnalysis(); + await Promise.resolve(); + }); + + act(() => { + currentProject = createProject('project-1', 'SELECT 2'); + rerender(); + }); + + await act(async () => { + resolveAnalysis({ + result: cachedResult, + cacheKey, + cacheHit: false, + skipped: false, + timings: null, + }); + await runPromise; + }); + + expect(lineageActions.setResult).not.toHaveBeenCalledWith(cachedResult); + expect(result.current.isAnalyzing).toBe(false); + }); + + it('stops before analysis when inputs change during the scheduling frame', async () => { + let resumeAnalysis!: FrameRequestCallback; + vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => { + resumeAnalysis = callback; + return 1; + }); + currentProject = createProject('project-1', 'SELECT 1'); + activeProjectId = currentProject.id; + const adapter = createAdapter(vi.fn()); + const { result, rerender } = renderHook(() => useAnalysis(true, { adapter })); + + let runPromise!: Promise; + act(() => { + runPromise = result.current.runAnalysis(); + }); + expect(result.current.isAnalyzing).toBe(true); + + act(() => { + currentProject = createProject('project-1', 'SELECT 2'); + rerender(); + resumeAnalysis(0); + }); + await act(async () => runPromise); + + expect(adapter.analyze).not.toHaveBeenCalled(); + expect(result.current.isAnalyzing).toBe(false); + }); + + it('does not cancel a run for a project metadata-only update', async () => { + let resumeAnalysis!: FrameRequestCallback; + vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => { + resumeAnalysis = callback; + return 1; + }); + currentProject = createProject('project-1', 'SELECT 1'); + activeProjectId = currentProject.id; + const cacheKey = buildProjectCacheKey(currentProject); + const adapter = createAdapter( + vi.fn().mockResolvedValue({ + result: cachedResult, + cacheKey, + cacheHit: false, + skipped: false, + timings: null, + }) + ); + const { result, rerender } = renderHook(() => useAnalysis(true, { adapter })); + + let runPromise!: Promise; + act(() => { + runPromise = result.current.runAnalysis(); + }); + act(() => { + currentProject = { ...currentProject!, name: 'Renamed project' }; + rerender(); + resumeAnalysis(0); + }); + await act(async () => runPromise); + + expect(adapter.analyze).toHaveBeenCalledTimes(1); + expect(lineageActions.setResult).toHaveBeenCalledWith(cachedResult); + }); + + it('supports an active-file-only run without changing the project run mode', async () => { + currentProject = createProject('project-1', 'SELECT 1'); + currentProject.files.push({ + id: 'project-1-other-file', + name: 'other.sql', + path: 'other.sql', + content: 'SELECT 2', + language: 'sql', + }); + activeProjectId = currentProject.id; + const adapter = createAdapter( + vi.fn().mockImplementation(async (payload: { files: Array<{ name: string }> }) => ({ + result: cachedResult, + cacheKey: buildAnalysisCacheKey({ + files: payload.files.map((file) => ({ ...file, content: 'SELECT 1' })), + dialect: 'generic', + schemaSQL: '', + hideCTEs: false, + enableColumnLineage: true, + enableLinting: false, + templateMode: 'raw', + }), + cacheHit: false, + skipped: false, + timings: null, + })) + ); + const { result } = renderHook(() => useAnalysis(true, { adapter })); + + await act(async () => { + await result.current.runAnalysis('SELECT 1', 'model.sql', 'current'); + }); + + expect(currentProject.runMode).toBe('all'); + expect(adapter.analyze).toHaveBeenCalledWith( + expect.objectContaining({ files: [{ name: 'model.sql', content: 'SELECT 1' }] }), + { knownCacheKey: null } + ); + }); + + it('does not let a pending proactive restore overwrite an explicit active-file run', async () => { + const persistentResult = { ...cachedResult, issues: [{ code: 'PERSISTED' }] } as AnalyzeResult; + const activeResult = { ...cachedResult, issues: [{ code: 'ACTIVE' }] } as AnalyzeResult; + currentProject = createProject('project-1', 'SELECT 1'); + currentProject.files.push({ + id: 'project-1-other-file', + name: 'other.sql', + path: 'other.sql', + content: 'SELECT 2', + language: 'sql', + }); + activeProjectId = currentProject.id; + const allFilesCacheKey = buildAnalysisCacheKey({ + files: currentProject.files.map((file) => ({ name: file.path, content: file.content })), + dialect: 'generic', + schemaSQL: '', + hideCTEs: false, + enableColumnLineage: true, + enableLinting: false, + templateMode: 'raw', + }); + const activeFileCacheKey = buildAnalysisCacheKey({ + files: [{ name: 'model.sql', content: 'SELECT 1' }], + dialect: 'generic', + schemaSQL: '', + hideCTEs: false, + enableColumnLineage: true, + enableLinting: false, + templateMode: 'raw', + }); + let resolveRestore!: (value: { + result: AnalyzeResult; + cacheKey: string; + cacheHit: boolean; + skipped: boolean; + timings: null; + }) => void; + const pendingRestore = new Promise[0]>((resolve) => { + resolveRestore = resolve; + }); + const adapter = createAdapter( + vi.fn().mockResolvedValue({ + result: activeResult, + cacheKey: activeFileCacheKey, + cacheHit: false, + skipped: false, + timings: null, + }) + ); + adapter.getCached = vi.fn(() => pendingRestore); + const { result } = renderHook(() => useAnalysis(true, { adapter })); + await act(async () => Promise.resolve()); + expect(adapter.getCached).toHaveBeenCalledTimes(1); + lineageActions.setResult.mockClear(); + + await act(async () => { + await result.current.runAnalysis('SELECT 1', 'model.sql', 'current'); + }); + await act(async () => { + resolveRestore({ + result: persistentResult, + cacheKey: allFilesCacheKey, + cacheHit: true, + skipped: false, + timings: null, + }); + await pendingRestore; + }); + + expect(lineageActions.setResult).toHaveBeenCalledWith(activeResult); + expect(lineageActions.setResult).not.toHaveBeenCalledWith(persistentResult); + }); + + it('invalidates an active-file override when that unselected file changes', async () => { + const staleResult = { ...cachedResult, issues: [{ code: 'STALE' }] } as AnalyzeResult; + currentProject = createProject('project-1', 'SELECT 1'); + const activeFile = currentProject.files[0]; + const selectedFile = { + id: 'project-1-selected-file', + name: 'selected.sql', + path: 'selected.sql', + content: 'SELECT 2', + language: 'sql' as const, + }; + currentProject.files.push(selectedFile); + currentProject.runMode = 'custom'; + currentProject.selectedFileIds = [selectedFile.id]; + activeProjectId = currentProject.id; + const activeFileCacheKey = buildAnalysisCacheKey({ + files: [{ name: activeFile.path, content: activeFile.content }], + dialect: 'generic', + schemaSQL: '', + hideCTEs: false, + enableColumnLineage: true, + enableLinting: false, + templateMode: 'raw', + }); + let resolveAnalysis!: (value: { + result: AnalyzeResult; + cacheKey: string; + cacheHit: boolean; + skipped: boolean; + timings: null; + }) => void; + const adapter = createAdapter( + vi.fn( + () => + new Promise[0]>((resolve) => { + resolveAnalysis = resolve; + }) + ) + ); + const { result, rerender } = renderHook(() => useAnalysis(true, { adapter })); + lineageActions.setResult.mockClear(); + + let runPromise!: Promise; + await act(async () => { + runPromise = result.current.runAnalysis(activeFile.content, activeFile.path, 'current'); + await Promise.resolve(); + }); + act(() => { + currentProject = { + ...currentProject!, + files: [{ ...activeFile, content: 'SELECT 10' }, selectedFile], + }; + rerender(); + }); + await act(async () => { + resolveAnalysis({ + result: staleResult, + cacheKey: activeFileCacheKey, + cacheHit: false, + skipped: false, + timings: null, + }); + await runPromise; + }); + + expect(lineageActions.setResult).not.toHaveBeenCalledWith(staleResult); + expect(result.current.isAnalyzing).toBe(false); + }); + + it('keeps an explicit result authoritative across metadata-only project updates', async () => { + const persistentResult = { ...cachedResult, issues: [{ code: 'PERSISTED' }] } as AnalyzeResult; + const activeResult = { ...cachedResult, issues: [{ code: 'ACTIVE' }] } as AnalyzeResult; + currentProject = createProject('project-1', 'SELECT 1'); + currentProject.files.push({ + id: 'project-1-other-file', + name: 'other.sql', + path: 'other.sql', + content: 'SELECT 2', + language: 'sql', + }); + activeProjectId = currentProject.id; + const allFilesCacheKey = buildAnalysisCacheKey({ + files: currentProject.files.map((file) => ({ name: file.path, content: file.content })), + dialect: 'generic', + schemaSQL: '', + hideCTEs: false, + enableColumnLineage: true, + enableLinting: false, + templateMode: 'raw', + }); + const activeFileCacheKey = buildAnalysisCacheKey({ + files: [{ name: 'model.sql', content: 'SELECT 1' }], + dialect: 'generic', + schemaSQL: '', + hideCTEs: false, + enableColumnLineage: true, + enableLinting: false, + templateMode: 'raw', + }); + const adapter = createAdapter( + vi.fn().mockResolvedValue({ + result: activeResult, + cacheKey: activeFileCacheKey, + cacheHit: false, + skipped: false, + timings: null, + }) + ); + const { result, rerender } = renderHook(() => useAnalysis(true, { adapter })); + await act(async () => Promise.resolve()); + + await act(async () => { + await result.current.runAnalysis('SELECT 1', 'model.sql', 'current'); + }); + lineageActions.setResult.mockClear(); + vi.mocked(adapter.getCached).mockClear(); + vi.mocked(adapter.getCached).mockResolvedValue({ + result: persistentResult, + cacheKey: allFilesCacheKey, + cacheHit: true, + skipped: false, + timings: null, + }); + + act(() => { + currentProject = { ...currentProject!, name: 'Renamed project' }; + rerender(); + }); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 350)); + }); + + expect(adapter.getCached).not.toHaveBeenCalled(); + expect(lineageActions.setResult).not.toHaveBeenCalledWith(persistentResult); + }); + + it('keeps a custom-mode run alive when an unselected file changes', async () => { + let resumeAnalysis!: FrameRequestCallback; + vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => { + resumeAnalysis = callback; + return 1; + }); + currentProject = createProject('project-1', 'SELECT 1'); + const selectedFile = currentProject.files[0]; + currentProject.files.push({ + id: 'project-1-unselected-file', + name: 'unselected.sql', + path: 'unselected.sql', + content: 'SELECT 2', + language: 'sql', + }); + currentProject.runMode = 'custom'; + currentProject.selectedFileIds = [selectedFile.id]; + activeProjectId = currentProject.id; + const cacheKey = buildAnalysisCacheKey({ + files: [{ name: selectedFile.path, content: selectedFile.content }], + dialect: 'generic', + schemaSQL: '', + hideCTEs: false, + enableColumnLineage: true, + enableLinting: false, + templateMode: 'raw', + }); + const adapter = createAdapter( + vi.fn().mockResolvedValue({ + result: cachedResult, + cacheKey, + cacheHit: false, + skipped: false, + timings: null, + }) + ); + const { result, rerender } = renderHook(() => useAnalysis(true, { adapter })); + + let runPromise!: Promise; + act(() => { + runPromise = result.current.runAnalysis(); + }); + act(() => { + currentProject = { + ...currentProject!, + files: [selectedFile, { ...currentProject!.files[1], content: 'SELECT 20' }], + }; + rerender(); + resumeAnalysis(0); + }); + await act(async () => runPromise); + + expect(adapter.analyze).toHaveBeenCalledTimes(1); + expect(lineageActions.setResult).toHaveBeenCalledWith(cachedResult); + }); + + it('discards an analysis result when analysis options change in flight', async () => { + currentProject = createProject('project-1', 'SELECT 1'); + activeProjectId = currentProject.id; + const cacheKey = buildProjectCacheKey(currentProject); + + let resolveAnalysis!: (value: { + result: AnalyzeResult; + cacheKey: string; + cacheHit: boolean; + skipped: boolean; + timings: null; + }) => void; + const adapter = createAdapter( + vi.fn( + () => + new Promise[0]>((resolve) => { + resolveAnalysis = resolve; + }) + ) + ); + const { result, rerender } = renderHook(() => useAnalysis(true, { adapter })); + lineageActions.setResult.mockClear(); + + let runPromise!: Promise; + await act(async () => { + runPromise = result.current.runAnalysis(); + await Promise.resolve(); + }); + act(() => { + hideCTEs = true; + rerender(); + }); + await act(async () => { + resolveAnalysis({ + result: cachedResult, + cacheKey, + cacheHit: false, + skipped: false, + timings: null, + }); + await runPromise; + }); + + expect(lineageActions.setResult).not.toHaveBeenCalledWith(cachedResult); + expect(result.current.isAnalyzing).toBe(false); + }); + + it('discards an analysis result when the backend changes in flight', async () => { + currentProject = createProject('project-1', 'SELECT 1'); + activeProjectId = currentProject.id; + const cacheKey = buildProjectCacheKey(currentProject); + + let resolveAnalysis!: (value: { + result: AnalyzeResult; + cacheKey: string; + cacheHit: boolean; + skipped: boolean; + timings: null; + }) => void; + const oldAdapter = createAdapter( + vi.fn( + () => + new Promise[0]>((resolve) => { + resolveAnalysis = resolve; + }) + ) + ); + let adapter = oldAdapter; + const { result, rerender } = renderHook(() => useAnalysis(true, { adapter })); + lineageActions.setResult.mockClear(); + + let runPromise!: Promise; + await act(async () => { + runPromise = result.current.runAnalysis(); + await Promise.resolve(); + }); + act(() => { + adapter = createAdapter(vi.fn(), 'rest'); + backendSchema = { tables: [] }; + rerender(); + }); + await act(async () => { + resolveAnalysis({ + result: cachedResult, + cacheKey, + cacheHit: false, + skipped: false, + timings: null, + }); + await runPromise; + }); + + expect(lineageActions.setResult).not.toHaveBeenCalledWith(cachedResult); + expect(result.current.isAnalyzing).toBe(false); + }); + + it('forces a full file replacement before retrying missing worker content', async () => { + currentProject = createProject('project-1', 'SELECT 1'); + activeProjectId = currentProject.id; + const cacheKey = buildProjectCacheKey(currentProject); + const analyze = vi + .fn() + .mockRejectedValueOnce( + new AnalysisError(AnalysisErrorCode.MISSING_FILE_CONTENT, 'missing query.sql') + ) + .mockResolvedValueOnce({ + result: cachedResult, + cacheKey, + cacheHit: false, + skipped: false, + timings: null, + }); + const adapter = createAdapter(analyze); + const { result } = renderHook(() => useAnalysis(true, { adapter })); + vi.mocked(adapter.syncFiles).mockClear(); + + await act(async () => result.current.runAnalysis()); + + expect(analyze).toHaveBeenCalledTimes(2); + expect(adapter.syncFiles).toHaveBeenCalledWith([{ name: 'model.sql', content: 'SELECT 1' }], { + forceReplace: true, + }); + expect(lineageActions.setResult).toHaveBeenCalledWith(cachedResult); + }); + it('does not build a canonical cache key for explicit REST analysis', async () => { const charCodeAt = vi.fn(() => { throw new Error('REST content was hashed'); @@ -261,6 +848,7 @@ describe('useAnalysis memory cache', () => { expect(charCodeAt).not.toHaveBeenCalled(); expect(analyze).toHaveBeenCalledTimes(1); + expect(adapter.syncFiles).not.toHaveBeenCalled(); expect(lineageActions.setResult).toHaveBeenCalledWith(cachedResult); }); }); diff --git a/app/src/hooks/__tests__/useBackendFiles.test.tsx b/app/src/hooks/__tests__/useBackendFiles.test.tsx new file mode 100644 index 00000000..e2505997 --- /dev/null +++ b/app/src/hooks/__tests__/useBackendFiles.test.tsx @@ -0,0 +1,49 @@ +import { act, renderHook, waitFor } from '@testing-library/react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('@pondpilot/flowscope-core', () => ({ + VALID_DIALECTS: ['generic', 'ansi'], +})); + +import { useBackendFiles } from '../useBackendFiles'; + +describe('useBackendFiles', () => { + afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); + + it('preserves the file snapshot across unchanged polling responses', async () => { + let sql = 'SELECT 1'; + vi.stubGlobal( + 'fetch', + vi.fn(async (input: RequestInfo | URL) => { + const url = String(input); + const data = url.endsWith('/api/files') + ? [{ name: 'query.sql', content: sql }] + : url.endsWith('/api/config') + ? { dialect: 'generic', watch_dirs: [], has_schema: false } + : null; + return { + ok: true, + json: async () => data, + } as Response; + }) + ); + + const { result, unmount } = renderHook(() => useBackendFiles(true)); + await waitFor(() => + expect(result.current.files).toEqual([{ name: 'query.sql', content: sql }]) + ); + const firstSnapshot = result.current.files; + + await act(async () => result.current.refresh()); + expect(result.current.files).toBe(firstSnapshot); + + sql = 'SELECT 2'; + await act(async () => result.current.refresh()); + expect(result.current.files).not.toBe(firstSnapshot); + expect(result.current.files?.[0].content).toBe('SELECT 2'); + unmount(); + }); +}); diff --git a/app/src/hooks/useAnalysis.ts b/app/src/hooks/useAnalysis.ts index da05cd16..0edea42c 100644 --- a/app/src/hooks/useAnalysis.ts +++ b/app/src/hooks/useAnalysis.ts @@ -1,10 +1,18 @@ -import { useState, useCallback, useEffect, useMemo, useRef, startTransition } from 'react'; +import { + useState, + useCallback, + useEffect, + useLayoutEffect, + useMemo, + useRef, + startTransition, +} from 'react'; import type { AnalyzeResult } from '@pondpilot/flowscope-core'; import { useLineageActions, useLineageState } from '@flowscope-react/store'; import { analyzeWithWorker, getCachedAnalysis, syncAnalysisFiles } from '@/lib/analysis-worker'; import type { BackendAdapter, AnalysisPayload } from '@/lib/backend-adapter'; import { useProject } from '@/lib/project-store'; -import type { Project } from '@/lib/project-store'; +import type { Project, RunMode } from '@/lib/project-store'; import { getAnalysisCacheRestoreDecision, useAnalysisStore, @@ -20,8 +28,45 @@ import { useDebounce } from './useDebounce'; // Maximum retry attempts for file sync errors to prevent infinite loops const MAX_FILE_SYNC_RETRIES = 1; +/** Idle window before proactive cache hashing and worker synchronization. */ const ANALYSIS_CACHE_KEY_DEBOUNCE_MS = 300; +interface ActiveAnalysisRequest { + requestId: number; + projectId: string | null; + runMode: RunMode; + currentFilePath?: string; + payload: AnalysisPayload; + adapter: BackendAdapter | null | undefined; + backendReady: boolean; + backendSchemaIdentity: string | null; +} + +interface ExplicitResultAuthority { + requestId: number; + projectId: string; + configuredPayload: AnalysisPayload; + adapter: BackendAdapter | null | undefined; + backendReady: boolean; + backendSchemaIdentity: string | null; +} + +function analysisPayloadsEqual(left: AnalysisPayload, right: AnalysisPayload): boolean { + return ( + left.dialect === right.dialect && + left.schemaSQL === right.schemaSQL && + left.hideCTEs === right.hideCTEs && + left.enableColumnLineage === right.enableColumnLineage && + left.enableLinting === right.enableLinting && + left.templateMode === right.templateMode && + left.files.length === right.files.length && + left.files.every( + (file, index) => + file.name === right.files[index].name && file.content === right.files[index].content + ) + ); +} + // Debug flag for analysis-related logging - only enabled in development const ANALYSIS_DEBUG = !!(import.meta as { env?: { DEV?: boolean } }).env?.DEV; @@ -53,16 +98,19 @@ export function useAnalysis(backendReady: boolean, options?: UseAnalysisOptions) const actions = useLineageActions(); const hideCTEs = useLineageState((state) => state.hideCTEs); const { getResult, setResult: storeResult, setMetrics } = useAnalysisStore(); - const getViewState = useViewStateStore((s) => s.getViewState); - const enableLinting = activeProjectId - ? getIssuesStateWithDefaults(getViewState(activeProjectId, 'issues')).showLintIssues - : false; + const issuesState = useViewStateStore((store) => + activeProjectId ? store.viewStates[activeProjectId]?.issues : undefined + ); + const enableLinting = getIssuesStateWithDefaults(issuesState).showLintIssues; const [state, setState] = useState({ isAnalyzing: false, error: null, lastAnalyzedAt: null, }); const analysisRequestRef = useRef(0); + const activeAnalysisRequestRef = useRef(null); + const proactiveRestoreRequestRef = useRef(0); + const explicitResultAuthorityRef = useRef(null); const attemptedCacheIdentityRef = useRef(null); const setAnalyzing = useCallback((isAnalyzing: boolean) => { @@ -106,15 +154,20 @@ export function useAnalysis(backendReady: boolean, options?: UseAnalysisOptions) activeFileContent?: string, // Use path (not just basename) for consistency with custom/all modes. // This ensures sourceName matches across all run modes. - activeFilePath?: string + activeFilePath?: string, + runModeOverride?: RunMode ): AnalysisContext | null => { if (!project) return null; let contextDescription = ''; let filesToAnalyze: Array<{ name: string; content: string }> = []; - const runMode = project.runMode; + const runMode = runModeOverride ?? project.runMode; - if (runMode === 'current' && activeFileContent && activeFilePath) { + if ( + runMode === 'current' && + activeFileContent !== undefined && + activeFilePath !== undefined + ) { filesToAnalyze = [{ name: activeFilePath, content: activeFileContent }]; contextDescription = `Analyzing file: ${activeFilePath}`; } else if (runMode === 'custom') { @@ -144,8 +197,18 @@ export function useAnalysis(backendReady: boolean, options?: UseAnalysisOptions) ); const buildAnalysisPayload = useCallback( - (project: Project | null, activeFileContent?: string, activeFilePath?: string) => { - const context = buildAnalysisContext(project, activeFileContent, activeFilePath); + ( + project: Project | null, + activeFileContent?: string, + activeFilePath?: string, + runModeOverride?: RunMode + ) => { + const context = buildAnalysisContext( + project, + activeFileContent, + activeFilePath, + runModeOverride + ); if (!project || !context) { return null; } @@ -221,42 +284,80 @@ export function useAnalysis(backendReady: boolean, options?: UseAnalysisOptions) [adapter?.type, backendSchema] ); - useEffect(() => { - if (!backendReady || !currentProject) { - return; - } - - let cancelled = false; - // Use file.path as name to match how buildAnalysisContext keys files. - // This ensures the worker cache uses consistent keys (paths) across sync and analysis. - const sqlFiles = currentProject.files - .filter((file) => file.name.endsWith('.sql')) - .map((f) => ({ name: f.path, content: f.content })); + // An explicit result remains authoritative while the configured analysis + // inputs are semantically unchanged. This prevents a metadata-only project + // replacement from scheduling the same proactive cache restore and replacing + // an active-file result with the saved all/custom-mode result. + useLayoutEffect(() => { + const authority = explicitResultAuthorityRef.current; + if (!authority) return; - if (ANALYSIS_DEBUG) - console.log(`[useAnalysis] File sync effect triggered (${sqlFiles.length} SQL files)`); - const syncEffectStart = nowMs(); + if ( + authority.projectId !== activeProjectId || + authority.adapter !== adapter || + authority.backendReady !== backendReady || + authority.backendSchemaIdentity !== backendSchemaIdentity || + !currentAnalysisPayload || + !analysisPayloadsEqual(authority.configuredPayload, currentAnalysisPayload.payload) + ) { + explicitResultAuthorityRef.current = null; + } + }, [activeProjectId, adapter, backendReady, backendSchemaIdentity, currentAnalysisPayload]); + + // A late result belongs to the exact inputs captured by runAnalysis, which + // may use a one-off run mode. Rebuild that same mode from the live project so + // unrelated edits and metadata updates do not cancel it, while an edit to an + // explicitly analyzed file always does. + useLayoutEffect(() => { + const activeRequest = activeAnalysisRequestRef.current; + if (!activeRequest) return; + + let liveAnalysisInput: ReturnType = null; + if (activeRequest.runMode === 'current') { + const currentFile = currentProject?.files.find( + (file) => file.path === activeRequest.currentFilePath + ); + if (currentFile) { + liveAnalysisInput = buildAnalysisPayload( + currentProject, + currentFile.content, + currentFile.path, + 'current' + ); + } + } else { + liveAnalysisInput = buildAnalysisPayload( + currentProject, + undefined, + undefined, + activeRequest.runMode + ); + } - const syncFiles = adapter ? adapter.syncFiles(sqlFiles) : syncAnalysisFiles(sqlFiles); + const shouldInvalidate = + activeRequest.projectId !== activeProjectId || + activeRequest.adapter !== adapter || + activeRequest.backendReady !== backendReady || + activeRequest.backendSchemaIdentity !== backendSchemaIdentity || + !liveAnalysisInput || + !analysisPayloadsEqual(activeRequest.payload, liveAnalysisInput.payload); - syncFiles - .then(() => { - if (!cancelled && ANALYSIS_DEBUG) { - console.log( - `[useAnalysis] File sync effect completed in ${(nowMs() - syncEffectStart).toFixed(1)}ms` - ); - } - }) - .catch((error: unknown) => { - if (!cancelled) { - console.warn('Failed to sync analysis files:', error); - } - }); + if (!shouldInvalidate) return; - return () => { - cancelled = true; - }; - }, [currentProject, backendReady, adapter]); + analysisRequestRef.current += 1; + activeAnalysisRequestRef.current = null; + if (explicitResultAuthorityRef.current?.requestId === activeRequest.requestId) { + explicitResultAuthorityRef.current = null; + } + setState((previous) => (previous.isAnalyzing ? { ...previous, isAnalyzing: false } : previous)); + }, [ + activeProjectId, + adapter, + backendReady, + backendSchemaIdentity, + buildAnalysisPayload, + currentProject, + ]); // Restore a result only when switching to a project whose canonical analysis // key matches. Input changes within the active project may keep the current @@ -283,8 +384,18 @@ export function useAnalysis(backendReady: boolean, options?: UseAnalysisOptions) return; } - const cachedResult = getResult(activeProjectId, currentAnalysisCacheKey); const nextIdentity = { projectId: activeProjectId, cacheKey: currentAnalysisCacheKey }; + const explicitAuthority = explicitResultAuthorityRef.current; + if ( + explicitAuthority?.projectId === activeProjectId && + currentAnalysisInput && + analysisPayloadsEqual(explicitAuthority.configuredPayload, currentAnalysisInput.payload) + ) { + attemptedCacheIdentityRef.current = nextIdentity; + return; + } + + const cachedResult = getResult(activeProjectId, currentAnalysisCacheKey); const restoreDecision = getAnalysisCacheRestoreDecision( attemptedCacheIdentityRef.current, nextIdentity, @@ -338,6 +449,14 @@ export function useAnalysis(backendReady: boolean, options?: UseAnalysisOptions) return; } + const explicitAuthority = explicitResultAuthorityRef.current; + if ( + explicitAuthority?.projectId === activeProjectId && + analysisPayloadsEqual(explicitAuthority.configuredPayload, currentAnalysisInput.payload) + ) { + return; + } + const cachedResult = canUseMemoryCache ? getResult(activeProjectId, currentAnalysisInput.cacheKey) : null; @@ -352,37 +471,54 @@ export function useAnalysis(backendReady: boolean, options?: UseAnalysisOptions) } let cancelled = false; + const restoreRequestId = proactiveRestoreRequestRef.current + 1; + proactiveRestoreRequestRef.current = restoreRequestId; const cacheStart = nowMs(); if (ANALYSIS_DEBUG) console.log(`[useAnalysis] Checking IndexedDB cache for ${context.files.length} files`); const syncAndGetCache = adapter ? adapter.syncFiles(context.files).then(() => { + if (cancelled || proactiveRestoreRequestRef.current !== restoreRequestId) return null; if (ANALYSIS_DEBUG) console.log(`[useAnalysis] Files synced, checking cache...`); return adapter.getCached(cachePayload); }) : syncAnalysisFiles(context.files).then(() => { + if (cancelled || proactiveRestoreRequestRef.current !== restoreRequestId) return null; if (ANALYSIS_DEBUG) console.log(`[useAnalysis] Files synced, checking IndexedDB cache...`); - return getCachedAnalysis({ - fileNames: context.files.map((file) => file.name), - dialect: cachePayload.dialect, - schemaSQL: cachePayload.schemaSQL, - hideCTEs: cachePayload.hideCTEs, - enableColumnLineage: cachePayload.enableColumnLineage, - enableLinting: cachePayload.enableLinting, - templateMode: cachePayload.templateMode, - }); + return getCachedAnalysis( + { + fileNames: context.files.map((file) => file.name), + dialect: cachePayload.dialect, + schemaSQL: cachePayload.schemaSQL, + hideCTEs: cachePayload.hideCTEs, + enableColumnLineage: cachePayload.enableColumnLineage, + enableLinting: cachePayload.enableLinting, + templateMode: cachePayload.templateMode, + }, + context.files + ); }); syncAndGetCache .then((cached) => { const durationMs = nowMs() - cacheStart; - if (cancelled) { + if (cancelled || proactiveRestoreRequestRef.current !== restoreRequestId) { if (ANALYSIS_DEBUG) console.log(`[useAnalysis] IndexedDB cache cancelled after ${durationMs.toFixed(1)}ms`); return; } + const latestExplicitAuthority = explicitResultAuthorityRef.current; + if ( + latestExplicitAuthority?.projectId === activeProjectId && + analysisPayloadsEqual( + latestExplicitAuthority.configuredPayload, + currentAnalysisInput.payload + ) + ) { + return; + } if (!cached?.result || cached.cacheKey !== cacheKey) { if (ANALYSIS_DEBUG) console.log(`[useAnalysis] IndexedDB cache MISS after ${durationMs.toFixed(1)}ms`); @@ -436,25 +572,58 @@ export function useAnalysis(backendReady: boolean, options?: UseAnalysisOptions) ]); const runAnalysis = useCallback( - async (activeFileContent?: string, activeFilePath?: string) => { + async (activeFileContent?: string, activeFilePath?: string, runModeOverride?: RunMode) => { if (!backendReady || !currentProject) return; + const analysisInput = buildAnalysisPayload( + currentProject, + activeFileContent, + activeFilePath, + runModeOverride + ); + const runMode = runModeOverride ?? currentProject.runMode; const requestId = analysisRequestRef.current + 1; analysisRequestRef.current = requestId; + activeAnalysisRequestRef.current = analysisInput + ? { + requestId, + projectId: activeProjectId, + runMode, + currentFilePath: runMode === 'current' ? activeFilePath : undefined, + payload: analysisInput.payload, + adapter, + backendReady, + backendSchemaIdentity, + } + : null; + // An explicit user run owns the visible result. Prevent an older + // proactive IndexedDB restore from replacing it after the worker queue + // drains, including active-file runs that do not change project mode. + proactiveRestoreRequestRef.current += 1; + explicitResultAuthorityRef.current = + canUseMemoryCache && activeProjectId && currentAnalysisPayload + ? { + requestId, + projectId: activeProjectId, + configuredPayload: currentAnalysisPayload.payload, + adapter, + backendReady, + backendSchemaIdentity, + } + : null; setAnalyzing(true); setError(null); const analysisStart = performance.now(); + let explicitResultAccepted = false; await new Promise((resolve) => requestAnimationFrame(() => resolve())); - try { - const analysisInput = buildAnalysisPayload( - currentProject, - activeFileContent, - activeFilePath - ); + if (analysisRequestRef.current !== requestId) { + return; + } + try { if (!analysisInput) { setError('No project context available'); return; @@ -463,7 +632,7 @@ export function useAnalysis(backendReady: boolean, options?: UseAnalysisOptions) const { context, payload: adapterPayload } = analysisInput; if (context.files.length === 0) { - if (currentProject.runMode === 'custom') { + if ((runModeOverride ?? currentProject.runMode) === 'custom') { setError('No files selected for analysis.'); return; } @@ -504,6 +673,7 @@ export function useAnalysis(backendReady: boolean, options?: UseAnalysisOptions) activeProjectId && cacheKey ? getResult(activeProjectId, cacheKey) : null; const knownCacheKey = cachedResult ? cacheKey : null; const displayResult = (result: AnalyzeResult) => { + explicitResultAccepted = true; startTransition(() => { actions.setResult(result); actions.setAnalyzedContent( @@ -534,7 +704,7 @@ export function useAnalysis(backendReady: boolean, options?: UseAnalysisOptions) fileSyncRetries < MAX_FILE_SYNC_RETRIES ) { fileSyncRetries++; - await adapter.syncFiles(context.files); + await adapter.syncFiles(context.files, { forceReplace: true }); continue; } throw error; @@ -554,7 +724,10 @@ export function useAnalysis(backendReady: boolean, options?: UseAnalysisOptions) while (true) { try { - analysisResponse = await analyzeWithWorker(workerPayload, { knownCacheKey }); + analysisResponse = await analyzeWithWorker(workerPayload, { + knownCacheKey, + fileSnapshot: context.files, + }); break; } catch (error) { // Handle missing file content by syncing files and retrying. @@ -565,7 +738,7 @@ export function useAnalysis(backendReady: boolean, options?: UseAnalysisOptions) fileSyncRetries < MAX_FILE_SYNC_RETRIES ) { fileSyncRetries++; - await syncAnalysisFiles(context.files); + await syncAnalysisFiles(context.files, { forceReplace: true }); continue; } throw error; @@ -610,6 +783,13 @@ export function useAnalysis(backendReady: boolean, options?: UseAnalysisOptions) console.error(error); } finally { if (analysisRequestRef.current === requestId) { + activeAnalysisRequestRef.current = null; + if ( + !explicitResultAccepted && + explicitResultAuthorityRef.current?.requestId === requestId + ) { + explicitResultAuthorityRef.current = null; + } setAnalyzing(false); } } @@ -626,7 +806,9 @@ export function useAnalysis(backendReady: boolean, options?: UseAnalysisOptions) setAnalyzing, setError, canUseMemoryCache, + currentAnalysisPayload, adapter, + backendSchemaIdentity, actions, ] ); diff --git a/app/src/hooks/useBackendFiles.ts b/app/src/hooks/useBackendFiles.ts index 287efa74..94bd6ef9 100644 --- a/app/src/hooks/useBackendFiles.ts +++ b/app/src/hooks/useBackendFiles.ts @@ -105,6 +105,19 @@ function sanitizeErrorMessage(message: string): string { return withoutPaths; } +function reuseUnchangedFiles(previous: FileSource[] | null, next: FileSource[]): FileSource[] { + if ( + previous && + previous.length === next.length && + previous.every( + (file, index) => file.name === next[index].name && file.content === next[index].content + ) + ) { + return previous; + } + return next; +} + /** * Fetches files and schema from the backend REST API. * @@ -165,7 +178,10 @@ export function useBackendFiles(enabled: boolean, baseUrl = ''): BackendFilesSta } const filesData = (await filesResponse.json()) as FileSource[]; - setFiles(filesData); + // Polling returns freshly allocated arrays even when the watched files + // are unchanged. Preserve the committed snapshot in that case so + // consumers do not cancel in-flight analysis on referential churn. + setFiles((previous) => reuseUnchangedFiles(previous, filesData)); // Schema is optional (may not be configured on backend) if (schemaResponse.ok) { diff --git a/app/src/lib/__tests__/analysis-hash.test.ts b/app/src/lib/__tests__/analysis-hash.test.ts deleted file mode 100644 index 337d426c..00000000 --- a/app/src/lib/__tests__/analysis-hash.test.ts +++ /dev/null @@ -1,34 +0,0 @@ -import { describe, expect, it } from 'vitest'; - -import { buildFileSyncKey } from '../analysis-hash'; - -describe('buildFileSyncKey', () => { - it('changes when same-length file content changes', () => { - const before = buildFileSyncKey({ - files: [{ name: 'query.sql', content: 'SELECT 1' }], - }); - const after = buildFileSyncKey({ - files: [{ name: 'query.sql', content: 'SELECT 2' }], - }); - const restored = buildFileSyncKey({ - files: [{ name: 'query.sql', content: 'SELECT 1' }], - }); - - expect(after).not.toBe(before); - expect(restored).toBe(before); - }); - - it('is stable for unchanged files', () => { - const files = [ - { name: 'models/orders.sql', content: 'SELECT * FROM raw_orders' }, - { name: 'models/customers.sql', content: 'SELECT * FROM raw_customers' }, - ]; - - const first = buildFileSyncKey({ files }); - const second = buildFileSyncKey({ - files: files.map((file) => ({ ...file })), - }); - - expect(second).toBe(first); - }); -}); diff --git a/app/src/lib/__tests__/analysis-worker.test.ts b/app/src/lib/__tests__/analysis-worker.test.ts index a65e6134..54fef963 100644 --- a/app/src/lib/__tests__/analysis-worker.test.ts +++ b/app/src/lib/__tests__/analysis-worker.test.ts @@ -1,10 +1,16 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { syncAnalysisFiles, terminateAnalysisWorker } from '../analysis-worker'; +import { + analyzeWithWorker, + getCachedAnalysis, + syncAnalysisFiles, + terminateAnalysisWorker, +} from '../analysis-worker'; import type { AnalysisWorkerRequest, AnalysisWorkerResponse } from '../../workers/analysis.worker'; class TestWorker { static instances: TestWorker[] = []; + static autoRespond = true; onmessage: ((event: MessageEvent) => void) | null = null; onerror: ((event: ErrorEvent) => void) | null = null; @@ -17,6 +23,15 @@ class TestWorker { postMessage(message: AnalysisWorkerRequest): void { this.messages.push(message); + if (TestWorker.autoRespond) { + this.respond(message); + } + } + + respond(message = this.messages.at(-1)): void { + if (!message) { + throw new Error('No worker message to respond to'); + } this.onmessage?.({ data: { type: 'sync-result', @@ -24,11 +39,32 @@ class TestWorker { }, } as MessageEvent); } + + respondWith( + response: Omit, + message = this.messages.at(-1) + ): void { + if (!message) { + throw new Error('No worker message to respond to'); + } + this.onmessage?.({ + data: { ...response, requestId: message.requestId }, + } as MessageEvent); + } } +const workerPayload = (fileName: string) => ({ + fileNames: [fileName], + dialect: 'generic' as const, + schemaSQL: '', + hideCTEs: false, + enableColumnLineage: true, +}); + describe('syncAnalysisFiles', () => { beforeEach(() => { TestWorker.instances = []; + TestWorker.autoRespond = true; vi.spyOn(console, 'log').mockImplementation(() => undefined); vi.stubGlobal('Worker', TestWorker); vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => { @@ -43,7 +79,7 @@ describe('syncAnalysisFiles', () => { vi.restoreAllMocks(); }); - it('resyncs same-length edits and skips unchanged files', async () => { + it('sends only changed files and skips unchanged snapshots', async () => { const originalFiles = [{ name: 'query.sql', content: 'SELECT 1' }]; await syncAnalysisFiles(originalFiles); @@ -55,8 +91,194 @@ describe('syncAnalysisFiles', () => { (message) => message.type === 'sync-files' ); expect(syncMessages).toHaveLength(2); + expect(syncMessages[0].syncPayload).toEqual({ + files: originalFiles, + deletedFileNames: [], + replace: true, + }); expect(syncMessages[1].syncPayload?.files).toEqual([ { name: 'query.sql', content: 'SELECT 2' }, ]); + expect(syncMessages[1].syncPayload?.replace).toBe(false); + }); + + it('tracks additions, deletions, renames, and project switches incrementally', async () => { + await syncAnalysisFiles([ + { name: 'models/orders.sql', content: 'SELECT 1' }, + { name: 'models/users.sql', content: 'SELECT 2' }, + ]); + await syncAnalysisFiles([ + { name: 'models/orders.sql', content: 'SELECT 10' }, + { name: 'models/customers.sql', content: 'SELECT 3' }, + ]); + await syncAnalysisFiles([{ name: 'other/project.sql', content: 'SELECT 4' }]); + await syncAnalysisFiles([]); + + const syncMessages = TestWorker.instances[0].messages.filter( + (message) => message.type === 'sync-files' + ); + expect(syncMessages).toHaveLength(4); + expect(syncMessages[1].syncPayload).toEqual({ + files: [ + { name: 'models/orders.sql', content: 'SELECT 10' }, + { name: 'models/customers.sql', content: 'SELECT 3' }, + ], + deletedFileNames: ['models/users.sql'], + replace: false, + }); + expect(syncMessages[2].syncPayload).toEqual({ + files: [{ name: 'other/project.sql', content: 'SELECT 4' }], + deletedFileNames: ['models/orders.sql', 'models/customers.sql'], + replace: false, + }); + expect(syncMessages[3].syncPayload).toEqual({ + files: [], + deletedFileNames: ['other/project.sql'], + replace: false, + }); + }); + + it('orders overlapping snapshots instead of racing full replacements', async () => { + const first = syncAnalysisFiles([{ name: 'query.sql', content: 'SELECT 1' }]); + const second = syncAnalysisFiles([{ name: 'query.sql', content: 'SELECT 2' }]); + + await Promise.all([first, second]); + + const syncMessages = TestWorker.instances[0].messages.filter( + (message) => message.type === 'sync-files' + ); + expect(syncMessages.map((message) => message.syncPayload?.replace)).toEqual([true, false]); + expect(syncMessages[1].syncPayload?.files).toEqual([ + { name: 'query.sql', content: 'SELECT 2' }, + ]); + }); + + it('keeps analysis bound to its snapshot when an older cache lookup queues later', async () => { + TestWorker.autoRespond = false; + const filesA = [{ name: 'query.sql', content: 'SELECT 1' }]; + const filesB = [{ name: 'query.sql', content: 'SELECT 2' }]; + const initialSync = syncAnalysisFiles(filesA); + await vi.waitFor(() => expect(TestWorker.instances[0]?.messages).toHaveLength(1)); + const worker = TestWorker.instances[0]; + + const analysisB = analyzeWithWorker(workerPayload('query.sql'), { fileSnapshot: filesB }); + worker.respond(worker.messages[0]); + await initialSync; + const staleLookupA = getCachedAnalysis(workerPayload('query.sql'), filesA); + + await vi.waitFor(() => expect(worker.messages).toHaveLength(2)); + expect(worker.messages[1].type).toBe('sync-files'); + expect(worker.messages[1].syncPayload?.files).toEqual(filesB); + worker.respond(worker.messages[1]); + + await vi.waitFor(() => expect(worker.messages).toHaveLength(3)); + expect(worker.messages[2].type).toBe('analyze'); + worker.respondWith( + { + type: 'analyze-result', + result: { + nodes: [], + edges: [], + statements: [], + issues: [], + summary: { + statementCount: 0, + tableCount: 0, + columnCount: 0, + joinCount: 0, + complexityScore: 0, + issueCount: { errors: 0, warnings: 0, infos: 0 }, + hasErrors: false, + }, + }, + cacheKey: 'snapshot-b', + }, + worker.messages[2] + ); + await expect(analysisB).resolves.toEqual(expect.objectContaining({ cacheKey: 'snapshot-b' })); + + await vi.waitFor(() => expect(worker.messages).toHaveLength(4)); + expect(worker.messages[3].syncPayload?.files).toEqual(filesA); + worker.respond(worker.messages[3]); + await vi.waitFor(() => expect(worker.messages).toHaveLength(5)); + expect(worker.messages[4].type).toBe('get-cache'); + worker.respondWith({ type: 'cache-result' }, worker.messages[4]); + await expect(staleLookupA).resolves.toBeNull(); + }); + + it('replaces the full snapshot after the worker restarts', async () => { + const files = [{ name: 'query.sql', content: 'SELECT 1' }]; + await syncAnalysisFiles(files); + + terminateAnalysisWorker(); + await syncAnalysisFiles(files); + + expect(TestWorker.instances).toHaveLength(2); + expect(TestWorker.instances[1].messages[0].syncPayload).toEqual({ + files, + deletedFileNames: [], + replace: true, + }); + }); + + it('forces a full replacement when worker state is known to be incomplete', async () => { + const files = [{ name: 'query.sql', content: 'SELECT 1' }]; + await syncAnalysisFiles(files); + await syncAnalysisFiles(files, { forceReplace: true }); + + const syncMessages = TestWorker.instances[0].messages.filter( + (message) => message.type === 'sync-files' + ); + expect(syncMessages).toHaveLength(2); + expect(syncMessages[1].syncPayload).toEqual({ + files, + deletedFileNames: [], + replace: true, + }); + }); + + it('invalidates the client snapshot when the worker crashes', async () => { + const files = [{ name: 'query.sql', content: 'SELECT 1' }]; + await syncAnalysisFiles(files); + + TestWorker.instances[0].onerror?.({ message: 'crashed' } as ErrorEvent); + await syncAnalysisFiles(files); + + expect(TestWorker.instances).toHaveLength(2); + expect(TestWorker.instances[0].terminate).toHaveBeenCalledTimes(1); + expect(TestWorker.instances[1].messages[0].syncPayload?.replace).toBe(true); + }); + + it('does not recreate a terminated worker from queued synchronization', async () => { + TestWorker.autoRespond = false; + const first = syncAnalysisFiles([{ name: 'query.sql', content: 'SELECT 1' }]); + const second = syncAnalysisFiles([{ name: 'query.sql', content: 'SELECT 2' }]); + const outcomes = Promise.allSettled([first, second]); + + await vi.waitFor(() => expect(TestWorker.instances[0]?.messages).toHaveLength(1)); + terminateAnalysisWorker(); + + expect(await outcomes).toEqual([ + expect.objectContaining({ status: 'rejected' }), + expect.objectContaining({ status: 'rejected' }), + ]); + expect(TestWorker.instances).toHaveLength(1); + }); + + it('ignores a stale error from a worker that has already been replaced', async () => { + await syncAnalysisFiles([{ name: 'query.sql', content: 'SELECT 1' }]); + const staleWorker = TestWorker.instances[0]; + terminateAnalysisWorker(); + + TestWorker.autoRespond = false; + const replacementSync = syncAnalysisFiles([{ name: 'query.sql', content: 'SELECT 2' }]); + await vi.waitFor(() => expect(TestWorker.instances).toHaveLength(2)); + const replacementWorker = TestWorker.instances[1]; + + staleWorker.onerror?.({ message: 'late crash' } as ErrorEvent); + replacementWorker.respond(); + + await expect(replacementSync).resolves.toBeUndefined(); + expect(replacementWorker.terminate).not.toHaveBeenCalled(); }); }); diff --git a/app/src/lib/__tests__/project-persistence.test.ts b/app/src/lib/__tests__/project-persistence.test.ts new file mode 100644 index 00000000..db3d3581 --- /dev/null +++ b/app/src/lib/__tests__/project-persistence.test.ts @@ -0,0 +1,47 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { + createDebouncedProjectPersistence, + PROJECT_PERSISTENCE_DEBOUNCE_MS, +} from '../project-persistence'; + +describe('createDebouncedProjectPersistence', () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it('persists only the latest snapshot after the idle window', () => { + const persist = vi.fn(); + const persistence = createDebouncedProjectPersistence(persist); + + persistence.schedule(['SELECT 1']); + vi.advanceTimersByTime(PROJECT_PERSISTENCE_DEBOUNCE_MS - 1); + persistence.schedule(['SELECT 12']); + persistence.schedule(['SELECT 123']); + + vi.advanceTimersByTime(PROJECT_PERSISTENCE_DEBOUNCE_MS - 1); + expect(persist).not.toHaveBeenCalled(); + + vi.advanceTimersByTime(1); + expect(persist).toHaveBeenCalledTimes(1); + expect(persist).toHaveBeenCalledWith(['SELECT 123']); + }); + + it('flushes the latest snapshot once and cancels its timer', () => { + const persist = vi.fn(); + const persistence = createDebouncedProjectPersistence(persist); + + persistence.schedule({ files: ['added.sql'] }); + persistence.schedule({ files: [] }); + persistence.flush(); + persistence.flush(); + vi.runAllTimers(); + + expect(persist).toHaveBeenCalledTimes(1); + expect(persist).toHaveBeenCalledWith({ files: [] }); + }); +}); diff --git a/app/src/lib/__tests__/project-store.test.tsx b/app/src/lib/__tests__/project-store.test.tsx new file mode 100644 index 00000000..c47205fa --- /dev/null +++ b/app/src/lib/__tests__/project-store.test.tsx @@ -0,0 +1,206 @@ +import { act, render } from '@testing-library/react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { STORAGE_KEYS } from '../constants'; +import type { Project } from '../project-store'; + +const backendState = vi.hoisted(() => ({ + type: 'wasm' as 'wasm' | 'rest' | null, + files: null as Array<{ name: string; content: string }> | null, + refresh: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock('@pondpilot/flowscope-core', () => ({ + VALID_DIALECTS: ['generic', 'ansi'], +})); + +vi.mock('../backend-context', () => ({ + useBackend: () => ({ backendType: backendState.type }), +})); + +vi.mock('@/hooks/useBackendFiles', () => ({ + useBackendFiles: () => ({ + files: backendState.files, + schema: null, + dialect: 'generic', + watchDirs: [], + templateMode: 'raw', + refresh: backendState.refresh, + }), +})); + +import { ProjectProvider, useProject } from '../project-store'; +import { PROJECT_PERSISTENCE_DEBOUNCE_MS } from '../project-persistence'; + +const projects: Project[] = [ + { + id: 'project-a', + name: 'Project A', + files: [ + { + id: 'a.sql', + name: 'a.sql', + path: 'a.sql', + content: 'SELECT 1', + language: 'sql', + }, + ], + activeFileId: 'a.sql', + dialect: 'generic', + runMode: 'all', + selectedFileIds: [], + schemaSQL: '', + templateMode: 'raw', + }, + { + id: 'project-b', + name: 'Project B', + files: [ + { + id: 'b.sql', + name: 'b.sql', + path: 'b.sql', + content: 'SELECT 2', + language: 'sql', + }, + { + id: 'removed.sql', + name: 'removed.sql', + path: 'removed.sql', + content: 'SELECT 0', + language: 'sql', + }, + ], + activeFileId: 'b.sql', + dialect: 'generic', + runMode: 'all', + selectedFileIds: [], + schemaSQL: '', + templateMode: 'raw', + }, +]; + +let projectApi: ReturnType; + +function ProjectConsumer() { + projectApi = useProject(); + return null; +} + +function renderProjectProvider() { + return render( + + + + ); +} + +describe('ProjectProvider persistence', () => { + beforeEach(() => { + vi.useFakeTimers(); + localStorage.clear(); + localStorage.setItem(STORAGE_KEYS.PROJECTS, JSON.stringify(projects)); + localStorage.setItem(STORAGE_KEYS.ACTIVE_PROJECT_ID, 'project-a'); + backendState.type = 'wasm'; + backendState.files = null; + backendState.refresh.mockClear(); + }); + + afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); + }); + + it('batches edits and deletions while project selection stays immediate', () => { + const setItem = vi.spyOn(localStorage, 'setItem'); + const stringify = vi.spyOn(JSON, 'stringify'); + renderProjectProvider(); + setItem.mockClear(); + stringify.mockClear(); + + act(() => projectApi.selectProject('project-b')); + expect(localStorage.getItem(STORAGE_KEYS.ACTIVE_PROJECT_ID)).toBe('project-b'); + expect(setItem.mock.calls.filter(([key]) => key === STORAGE_KEYS.PROJECTS)).toHaveLength(0); + + act(() => { + projectApi.updateFile('b.sql', 'SELECT 20'); + projectApi.updateFile('b.sql', 'SELECT 200'); + projectApi.deleteFile('removed.sql'); + }); + + act(() => vi.advanceTimersByTime(PROJECT_PERSISTENCE_DEBOUNCE_MS - 1)); + expect(setItem.mock.calls.filter(([key]) => key === STORAGE_KEYS.PROJECTS)).toHaveLength(0); + expect(stringify).not.toHaveBeenCalled(); + + act(() => vi.advanceTimersByTime(1)); + const projectWrites = setItem.mock.calls.filter(([key]) => key === STORAGE_KEYS.PROJECTS); + expect(projectWrites).toHaveLength(1); + const persisted = JSON.parse(projectWrites[0][1]) as Project[]; + expect(stringify).toHaveBeenCalledTimes(1); + expect(persisted[1].files).toEqual([ + expect.objectContaining({ id: 'b.sql', content: 'SELECT 200' }), + ]); + }); + + it('flushes the latest edit when the provider unmounts', () => { + const setItem = vi.spyOn(localStorage, 'setItem'); + const view = renderProjectProvider(); + setItem.mockClear(); + + act(() => projectApi.updateFile('a.sql', 'SELECT before_teardown')); + view.unmount(); + + const projectWrites = setItem.mock.calls.filter(([key]) => key === STORAGE_KEYS.PROJECTS); + expect(projectWrites).toHaveLength(1); + const persisted = JSON.parse(projectWrites[0][1]) as Project[]; + expect(persisted[0].files[0].content).toBe('SELECT before_teardown'); + + act(() => vi.runAllTimers()); + expect(setItem.mock.calls.filter(([key]) => key === STORAGE_KEYS.PROJECTS)).toHaveLength(1); + }); + + it('flushes additions and deletions when the page is hidden', () => { + const setItem = vi.spyOn(localStorage, 'setItem'); + renderProjectProvider(); + setItem.mockClear(); + + act(() => { + projectApi.createFile('added.sql', 'SELECT added'); + projectApi.deleteFile('a.sql'); + }); + act(() => { + window.dispatchEvent(new Event('pagehide')); + }); + + const projectWrites = setItem.mock.calls.filter(([key]) => key === STORAGE_KEYS.PROJECTS); + expect(projectWrites).toHaveLength(1); + const persisted = JSON.parse(projectWrites[0][1]) as Project[]; + expect(persisted[0].files).toEqual([ + expect.objectContaining({ name: 'added.sql', content: 'SELECT added' }), + ]); + + act(() => vi.runAllTimers()); + expect(setItem.mock.calls.filter(([key]) => key === STORAGE_KEYS.PROJECTS)).toHaveLength(1); + }); + + it('never persists the virtual backend project', () => { + const setItem = vi.spyOn(localStorage, 'setItem'); + const view = renderProjectProvider(); + setItem.mockClear(); + + backendState.type = 'rest'; + backendState.files = [{ name: 'server.sql', content: 'SELECT server' }]; + view.rerender( + + + + ); + expect(projectApi.currentProject?.id).toBe('__backend__'); + + act(() => vi.advanceTimersByTime(PROJECT_PERSISTENCE_DEBOUNCE_MS)); + const projectWrites = setItem.mock.calls.filter(([key]) => key === STORAGE_KEYS.PROJECTS); + expect(projectWrites).toHaveLength(1); + const persisted = JSON.parse(projectWrites[0][1]) as Project[]; + expect(persisted.map((project) => project.id)).toEqual(['project-a', 'project-b']); + }); +}); diff --git a/app/src/lib/analysis-hash.ts b/app/src/lib/analysis-hash.ts index 740fa945..a917c356 100644 --- a/app/src/lib/analysis-hash.ts +++ b/app/src/lib/analysis-hash.ts @@ -12,10 +12,6 @@ const HASH_VERSION = 'v5'; const FNV_OFFSET_BASIS = 0xcbf29ce484222325n; const FNV_PRIME = 0x100000001b3n; const FNV_MASK = 0xffffffffffffffffn; -const FNV_OFFSET_HIGH = 0xcbf29ce4; -const FNV_OFFSET_LOW = 0x84222325; -const FNV_PRIME_LOW = 0x1b3; -const UINT32_SIZE = 0x100000000; export interface AnalysisHashInput { files: Array<{ name: string; content: string }>; @@ -27,23 +23,6 @@ export interface AnalysisHashInput { templateMode?: TemplateMode; } -export interface FileSyncInput { - files: Array<{ name: string; content: string }>; -} - -interface FastHashState { - high: number; - low: number; -} - -interface CachedFileSyncDigest { - name: string; - content: string; - digest: string; -} - -let previousFileSyncDigests: CachedFileSyncDigest[] = []; - /** * Update hash with a string value using FNV-1a algorithm. * @see https://en.wikipedia.org/wiki/Fowler%E2%80%93Noll%E2%80%93Vo_hash_function @@ -73,46 +52,6 @@ function updateHashWithField(currentHash: bigint, value: string): bigint { return hash; } -/** - * Update a 64-bit FNV-1a hash using two 32-bit words. - * - * This produces the same hash as the BigInt implementation without performing - * BigInt arithmetic for every character on the browser's main thread. - */ -function updateFastHashWithString(currentHash: FastHashState, value: string): FastHashState { - let { high, low } = currentHash; - - for (let index = 0; index < value.length; index += 1) { - low = (low ^ value.charCodeAt(index)) >>> 0; - - // The 64-bit FNV prime is 2^40 + 0x1b3. Multiplication by 0x1b3 - // stays within JavaScript's exact integer range, so carry can be - // applied to the high word without BigInt. - const lowProduct = low * FNV_PRIME_LOW; - const carry = Math.floor(lowProduct / UINT32_SIZE); - high = (Math.imul(high, FNV_PRIME_LOW) + carry + (low << 8)) >>> 0; - low = lowProduct >>> 0; - } - - return { high, low }; -} - -function updateFastHashWithField(currentHash: FastHashState, value: string): FastHashState { - const hash = updateFastHashWithString(currentHash, `${value.length}:`); - return updateFastHashWithString(hash, value); -} - -function formatFastHash(hash: FastHashState): string { - return `${hash.high.toString(16).padStart(8, '0')}${hash.low.toString(16).padStart(8, '0')}`; -} - -function buildFileDigest(file: FileSyncInput['files'][number]): string { - let hash = { high: FNV_OFFSET_HIGH, low: FNV_OFFSET_LOW }; - hash = updateFastHashWithField(hash, file.name); - hash = updateFastHashWithField(hash, file.content); - return formatFastHash(hash); -} - export function buildAnalysisCacheKey(input: AnalysisHashInput): string { let hash = FNV_OFFSET_BASIS; // Fixed-format fields use updateHashWithString (no collision risk) @@ -132,24 +71,3 @@ export function buildAnalysisCacheKey(input: AnalysisHashInput): string { return hash.toString(16).padStart(16, '0'); } - -export function buildFileSyncKey(input: FileSyncInput): string { - let hash = { high: FNV_OFFSET_HIGH, low: FNV_OFFSET_LOW }; - hash = updateFastHashWithString(hash, `${HASH_VERSION}-files`); - hash = updateFastHashWithString(hash, String(input.files.length)); - - const nextFileSyncDigests: CachedFileSyncDigest[] = []; - for (const [index, file] of input.files.entries()) { - const cached = previousFileSyncDigests[index]; - const digest = - cached && cached.name === file.name && cached.content === file.content - ? cached.digest - : buildFileDigest(file); - - nextFileSyncDigests.push({ ...file, digest }); - hash = updateFastHashWithField(hash, digest); - } - previousFileSyncDigests = nextFileSyncDigests; - - return formatFastHash(hash); -} diff --git a/app/src/lib/analysis-worker.ts b/app/src/lib/analysis-worker.ts index b1551790..68cd87e3 100644 --- a/app/src/lib/analysis-worker.ts +++ b/app/src/lib/analysis-worker.ts @@ -7,7 +7,6 @@ import type { SyncFilesPayload, WorkerErrorCode, } from '../workers/analysis.worker'; -import { buildFileSyncKey } from './analysis-hash'; import { AnalysisError, AnalysisErrorCode } from '../types'; // Debug flag for analysis worker logging - only enabled in development @@ -34,6 +33,7 @@ function mapWorkerErrorCode(code: WorkerErrorCode | undefined): AnalysisErrorCod interface PendingRequest { resolve: (value: AnalysisWorkerResponse) => void; reject: (error: Error) => void; + worker: Worker; } export interface AnalysisWorkerResult { @@ -47,7 +47,25 @@ export interface AnalysisWorkerResult { let workerInstance: Worker | null = null; let requestCounter = 0; const pendingRequests = new Map(); -let lastSyncedFileKey: string | null = null; +let syncedFiles: Map | null = null; +let fileSyncQueue = Promise.resolve(); +let fileSyncGeneration = 0; + +function resetFileSyncState(): void { + syncedFiles = null; + fileSyncGeneration += 1; + fileSyncQueue = Promise.resolve(); +} + +function rejectWorkerRequests(worker: Worker, error: Error): void { + for (const [requestId, pending] of pendingRequests) { + if (pending.worker !== worker) { + continue; + } + pending.reject(error); + pendingRequests.delete(requestId); + } +} function isWorkerSupported(): boolean { return typeof Worker !== 'undefined'; @@ -55,14 +73,15 @@ function isWorkerSupported(): boolean { function getWorker(): Worker { if (!workerInstance) { - workerInstance = new Worker(new URL('../workers/analysis.worker.ts', import.meta.url), { + const worker = new Worker(new URL('../workers/analysis.worker.ts', import.meta.url), { type: 'module', }); + workerInstance = worker; - workerInstance.onmessage = (event: MessageEvent) => { + worker.onmessage = (event: MessageEvent) => { const response = event.data; const pending = pendingRequests.get(response.requestId); - if (!pending) { + if (!pending || pending.worker !== worker) { return; } pendingRequests.delete(response.requestId); @@ -81,11 +100,13 @@ function getWorker(): Worker { pending.resolve(response); }; - workerInstance.onerror = (error) => { - for (const [requestId, pending] of pendingRequests) { - pending.reject(new Error(`Worker error: ${error.message}`)); - pendingRequests.delete(requestId); + worker.onerror = (error) => { + if (workerInstance === worker) { + worker.terminate(); + workerInstance = null; + resetFileSyncState(); } + rejectWorkerRequests(worker, new Error(`Worker error: ${error.message}`)); }; } @@ -103,8 +124,13 @@ function sendRequest( const worker = getWorker(); return new Promise((resolve, reject) => { - pendingRequests.set(requestId, { resolve, reject }); - worker.postMessage({ ...message, requestId }); + pendingRequests.set(requestId, { resolve, reject, worker }); + try { + worker.postMessage({ ...message, requestId }); + } catch (error) { + pendingRequests.delete(requestId); + reject(error instanceof Error ? error : new Error(String(error))); + } }); } @@ -118,42 +144,132 @@ async function yieldToMainThread(): Promise { }); } -export async function syncAnalysisFiles(files: SyncFilesPayload['files']): Promise { - const syncStart = nowMs(); - const nextKey = buildFileSyncKey({ files }); - if (nextKey === lastSyncedFileKey) { - if (ANALYSIS_WORKER_DEBUG) - console.log(`[syncAnalysisFiles] Skipped (cache hit), ${files.length} files`); - return; - } - - if (ANALYSIS_WORKER_DEBUG) - console.log(`[syncAnalysisFiles] Starting sync of ${files.length} files`); +async function sendFileChanges( + files: SyncFilesPayload['files'], + deletedFileNames: string[], + replace: boolean, + generation: number +): Promise { + const assertCurrentGeneration = () => { + if (generation !== fileSyncGeneration) { + throw new Error('Worker terminated'); + } + }; if (files.length === 0) { - await sendRequest({ type: 'clear-files' }); - lastSyncedFileKey = nextKey; - if (ANALYSIS_WORKER_DEBUG) - console.log(`[syncAnalysisFiles] Cleared files in ${(nowMs() - syncStart).toFixed(1)}ms`); + assertCurrentGeneration(); + await sendRequest({ + type: 'sync-files', + syncPayload: { files: [], deletedFileNames, replace }, + }); + assertCurrentGeneration(); return; } const chunkSize = 5; for (let index = 0; index < files.length; index += chunkSize) { - const chunk = files.slice(index, index + chunkSize); + assertCurrentGeneration(); + const isFirstChunk = index === 0; await sendRequest({ type: 'sync-files', syncPayload: { - files: chunk, - replace: index === 0, + files: files.slice(index, index + chunkSize), + deletedFileNames: isFirstChunk ? deletedFileNames : [], + replace: isFirstChunk && replace, }, }); - await yieldToMainThread(); + assertCurrentGeneration(); + if (index + chunkSize < files.length) { + await yieldToMainThread(); + } + } +} + +async function applyFileSnapshot( + files: SyncFilesPayload['files'], + generation: number +): Promise { + const syncStart = nowMs(); + const targetFilesByName = new Map(files.map((file) => [file.name, file])); + const targetFiles = [...targetFilesByName.values()]; + const targetContents = new Map(targetFiles.map((file) => [file.name, file.content])); + + if (ANALYSIS_WORKER_DEBUG) + console.log(`[syncAnalysisFiles] Comparing ${targetFiles.length} files`); + + if (syncedFiles === null) { + await sendFileChanges(targetFiles, [], true, generation); + syncedFiles = targetContents; + if (ANALYSIS_WORKER_DEBUG) + console.log( + `[syncAnalysisFiles] Replaced worker snapshot in ${(nowMs() - syncStart).toFixed(1)}ms` + ); + return; + } + + const changedFiles = targetFiles.filter((file) => syncedFiles?.get(file.name) !== file.content); + const deletedFileNames = [...syncedFiles.keys()].filter( + (fileName) => !targetContents.has(fileName) + ); + + if (changedFiles.length === 0 && deletedFileNames.length === 0) { + if (ANALYSIS_WORKER_DEBUG) console.log(`[syncAnalysisFiles] Skipped unchanged snapshot`); + return; } - lastSyncedFileKey = nextKey; + await sendFileChanges(changedFiles, deletedFileNames, false, generation); + syncedFiles = targetContents; if (ANALYSIS_WORKER_DEBUG) - console.log(`[syncAnalysisFiles] Completed in ${(nowMs() - syncStart).toFixed(1)}ms`); + console.log( + `[syncAnalysisFiles] Applied ${changedFiles.length} updates and ${deletedFileNames.length} deletions in ${(nowMs() - syncStart).toFixed(1)}ms` + ); +} + +export interface SyncAnalysisFilesOptions { + /** Replace the worker snapshot even when the client snapshot appears current. */ + forceReplace?: boolean; +} + +function withFileSnapshot( + files: SyncFilesPayload['files'], + options: SyncAnalysisFilesOptions | undefined, + task: () => Promise +): Promise { + const snapshot = files.map((file) => ({ ...file })); + const generation = fileSyncGeneration; + const operation = fileSyncQueue + .catch(() => undefined) + .then(async () => { + if (generation !== fileSyncGeneration) { + throw new Error('Worker terminated'); + } + if (options?.forceReplace) { + syncedFiles = null; + } + await applyFileSnapshot(snapshot, generation); + if (generation !== fileSyncGeneration) { + throw new Error('Worker terminated'); + } + return task(); + }); + fileSyncQueue = operation.then( + () => undefined, + () => undefined + ); + return operation; +} + +/** + * Synchronize an exact file snapshot to the worker in call order. The first + * snapshot after worker creation/restart replaces all files; later snapshots + * send only added, changed, renamed, or deleted paths. The returned promise is + * a barrier: analysis started after it resolves observes this snapshot. + */ +export function syncAnalysisFiles( + files: SyncFilesPayload['files'], + options?: SyncAnalysisFilesOptions +): Promise { + return withFileSnapshot(files, options, async () => undefined); } export async function initializeAnalysisWorker(): Promise { @@ -167,53 +283,72 @@ export async function clearAnalysisWorkerCache(): Promise { export interface AnalyzeWorkerOptions { cacheMaxBytes?: number; knownCacheKey?: string | null; + /** Exact worker file snapshot that must remain bound to this analysis. */ + fileSnapshot?: SyncFilesPayload['files']; } export async function analyzeWithWorker( payload: AnalysisWorkerPayload, options?: AnalyzeWorkerOptions ): Promise { - const response = await sendRequest({ - type: 'analyze', - payload, - cacheMaxBytes: options?.cacheMaxBytes, - knownCacheKey: options?.knownCacheKey, - }); + const analyze = async () => { + const response = await sendRequest({ + type: 'analyze', + payload, + cacheMaxBytes: options?.cacheMaxBytes, + knownCacheKey: options?.knownCacheKey, + }); - if (!response.cacheKey) { - throw new Error('Worker returned an empty cache key'); - } + if (!response.cacheKey) { + throw new Error('Worker returned an empty cache key'); + } - const skipped = Boolean(response.skipResult); - if (!response.result && !skipped) { - throw new Error('Worker returned an empty analysis result'); - } + const skipped = Boolean(response.skipResult); + if (!response.result && !skipped) { + throw new Error('Worker returned an empty analysis result'); + } - return { - result: response.result ?? null, - cacheKey: response.cacheKey, - cacheHit: Boolean(response.cacheHit), - skipped, - timings: response.timings ?? null, + return { + result: response.result ?? null, + cacheKey: response.cacheKey, + cacheHit: Boolean(response.cacheHit), + skipped, + timings: response.timings ?? null, + }; }; + + if (options?.fileSnapshot) { + return withFileSnapshot(options.fileSnapshot, undefined, analyze); + } + await fileSyncQueue; + return analyze(); } export async function getCachedAnalysis( - payload: AnalysisWorkerPayload + payload: AnalysisWorkerPayload, + fileSnapshot?: SyncFilesPayload['files'] ): Promise { - const response = await sendRequest({ type: 'get-cache', payload }); + const getCached = async () => { + const response = await sendRequest({ type: 'get-cache', payload }); - if (!response.result || !response.cacheKey) { - return null; - } + if (!response.result || !response.cacheKey) { + return null; + } - return { - result: response.result, - cacheKey: response.cacheKey, - cacheHit: Boolean(response.cacheHit), - skipped: false, - timings: response.timings ?? null, + return { + result: response.result, + cacheKey: response.cacheKey, + cacheHit: Boolean(response.cacheHit), + skipped: false, + timings: response.timings ?? null, + }; }; + + if (fileSnapshot) { + return withFileSnapshot(fileSnapshot, undefined, getCached); + } + await fileSyncQueue; + return getCached(); } export async function getAnalysisWorkerVersion(): Promise { @@ -222,11 +357,12 @@ export async function getAnalysisWorkerVersion(): Promise { } export function terminateAnalysisWorker(): void { - if (workerInstance) { - workerInstance.terminate(); + const worker = workerInstance; + if (worker) { + worker.terminate(); workerInstance = null; } - lastSyncedFileKey = null; + resetFileSyncState(); for (const [requestId, pending] of pendingRequests) { pending.reject(new Error('Worker terminated')); pendingRequests.delete(requestId); diff --git a/app/src/lib/backend-adapter.ts b/app/src/lib/backend-adapter.ts index 0a43df9d..5306b4f3 100644 --- a/app/src/lib/backend-adapter.ts +++ b/app/src/lib/backend-adapter.ts @@ -19,6 +19,7 @@ import { clearAnalysisWorkerCache, } from './analysis-worker'; import type { AnalysisWorkerResult, AnalyzeWorkerOptions } from './analysis-worker'; +import type { SyncAnalysisFilesOptions } from './analysis-worker'; /** * Payload for running analysis. @@ -69,7 +70,10 @@ export interface BackendAdapter { getVersion(): Promise; /** Sync files to the backend (for WASM worker file cache) */ - syncFiles(files: Array<{ name: string; content: string }>): Promise; + syncFiles( + files: Array<{ name: string; content: string }>, + options?: SyncAnalysisFilesOptions + ): Promise; /** Clear the analysis cache */ clearCache(): Promise; @@ -167,9 +171,6 @@ export class WasmBackendAdapter implements BackendAdapter { } async analyze(payload: AnalysisPayload, options?: AnalyzeWorkerOptions): Promise { - // Ensure files are synced before analysis - await this.syncFiles(payload.files); - const workerResult: AnalysisWorkerResult = await analyzeWithWorker( { fileNames: payload.files.map((f) => f.name), @@ -180,7 +181,7 @@ export class WasmBackendAdapter implements BackendAdapter { enableLinting: payload.enableLinting, templateMode: payload.templateMode, }, - options + { ...options, fileSnapshot: payload.files } ); return { @@ -193,18 +194,18 @@ export class WasmBackendAdapter implements BackendAdapter { } async getCached(payload: AnalysisPayload): Promise { - // Ensure files are synced before checking cache - await this.syncFiles(payload.files); - - const cached = await getCachedAnalysis({ - fileNames: payload.files.map((f) => f.name), - dialect: payload.dialect, - schemaSQL: payload.schemaSQL, - hideCTEs: payload.hideCTEs, - enableColumnLineage: payload.enableColumnLineage, - enableLinting: payload.enableLinting, - templateMode: payload.templateMode, - }); + const cached = await getCachedAnalysis( + { + fileNames: payload.files.map((f) => f.name), + dialect: payload.dialect, + schemaSQL: payload.schemaSQL, + hideCTEs: payload.hideCTEs, + enableColumnLineage: payload.enableColumnLineage, + enableLinting: payload.enableLinting, + templateMode: payload.templateMode, + }, + payload.files + ); if (!cached) { return null; @@ -223,8 +224,11 @@ export class WasmBackendAdapter implements BackendAdapter { return getAnalysisWorkerVersion(); } - async syncFiles(files: Array<{ name: string; content: string }>): Promise { - await syncAnalysisFiles(files); + async syncFiles( + files: Array<{ name: string; content: string }>, + options?: SyncAnalysisFilesOptions + ): Promise { + await syncAnalysisFiles(files, options); } async clearCache(): Promise { diff --git a/app/src/lib/project-persistence.ts b/app/src/lib/project-persistence.ts new file mode 100644 index 00000000..fa967ac3 --- /dev/null +++ b/app/src/lib/project-persistence.ts @@ -0,0 +1,50 @@ +/** Time without project changes before the latest snapshot is persisted. */ +export const PROJECT_PERSISTENCE_DEBOUNCE_MS = 500; + +export interface DebouncedProjectPersistence { + /** Replace the pending snapshot and restart the idle timer. */ + schedule(value: T): void; + /** Persist the latest pending snapshot immediately, if one exists. */ + flush(): void; +} + +/** + * Keeps serialization out of interactive updates by retaining only the latest + * immutable snapshot until the idle timer expires. Lifecycle owners must call + * `flush` before teardown so the debounce window cannot lose the last change. + */ +export function createDebouncedProjectPersistence( + persist: (value: T) => void, + delayMs = PROJECT_PERSISTENCE_DEBOUNCE_MS +): DebouncedProjectPersistence { + let pendingValue: T | undefined; + let hasPendingValue = false; + let timeoutId: ReturnType | null = null; + + const flush = () => { + if (timeoutId !== null) { + clearTimeout(timeoutId); + timeoutId = null; + } + if (!hasPendingValue) { + return; + } + + const value = pendingValue as T; + pendingValue = undefined; + hasPendingValue = false; + persist(value); + }; + + return { + schedule(value) { + pendingValue = value; + hasPendingValue = true; + if (timeoutId !== null) { + clearTimeout(timeoutId); + } + timeoutId = setTimeout(flush, delayMs); + }, + flush, + }; +} diff --git a/app/src/lib/project-store.tsx b/app/src/lib/project-store.tsx index 6af5a205..cde342e1 100644 --- a/app/src/lib/project-store.tsx +++ b/app/src/lib/project-store.tsx @@ -1,4 +1,12 @@ -import React, { createContext, useContext, useState, useCallback, useEffect, useMemo } from 'react'; +import React, { + createContext, + useContext, + useState, + useCallback, + useEffect, + useLayoutEffect, + useMemo, +} from 'react'; import { VALID_DIALECTS as CORE_VALID_DIALECTS } from '@pondpilot/flowscope-core'; import type { Dialect as CoreDialect, FileSource, SchemaMetadata } from '@pondpilot/flowscope-core'; import { STORAGE_KEYS, FILE_EXTENSIONS, SHARE_LIMITS, DEFAULT_FILE_LANGUAGE } from './constants'; @@ -8,6 +16,10 @@ import type { TemplateMode } from '@/types'; import { DEFAULT_PROJECT, DEFAULT_DBT_PROJECT } from './default-projects'; import { useBackend } from './backend-context'; import { useBackendFiles } from '@/hooks/useBackendFiles'; +import { + createDebouncedProjectPersistence, + PROJECT_PERSISTENCE_DEBOUNCE_MS, +} from './project-persistence'; const uuidv4 = () => crypto.randomUUID(); @@ -219,6 +231,10 @@ export function ProjectProvider({ children }: { children: React.ReactNode }) { const [activeProjectId, setActiveProjectId] = useState(() => loadActiveProjectIdFromStorage(projects) ); + const projectPersistence = useMemo( + () => createDebouncedProjectPersistence(saveProjectsToStorage, PROJECT_PERSISTENCE_DEBOUNCE_MS), + [] + ); // Get backend state const { backendType } = useBackend(); @@ -278,14 +294,19 @@ export function ProjectProvider({ children }: { children: React.ReactNode }) { } }, [isBackendMode]); + const backendProjectFiles = useMemo( + () => backendFiles?.map(fileSourceToProjectFile) ?? null, + [backendFiles] + ); + // Create a virtual project from backend files const backendProject: Project | null = useMemo(() => { - if (!isBackendMode || !backendFiles) return null; + if (!isBackendMode || !backendProjectFiles) return null; return { id: BACKEND_PROJECT_ID, name: 'Server Files', - files: backendFiles.map(fileSourceToProjectFile), + files: backendProjectFiles, activeFileId: backendActiveFileId, dialect: backendDialect, runMode: backendRunMode, @@ -295,7 +316,7 @@ export function ProjectProvider({ children }: { children: React.ReactNode }) { }; }, [ isBackendMode, - backendFiles, + backendProjectFiles, backendDialect, backendTemplateMode, backendActiveFileId, @@ -303,11 +324,29 @@ export function ProjectProvider({ children }: { children: React.ReactNode }) { backendSelectedFileIds, ]); - useEffect(() => { - saveProjectsToStorage(projects); - }, [projects]); + useLayoutEffect(() => { + projectPersistence.schedule(projects); + }, [projectPersistence, projects]); useEffect(() => { + const flushProjects = () => projectPersistence.flush(); + const flushProjectsWhenHidden = () => { + if (document.visibilityState === 'hidden') { + flushProjects(); + } + }; + + window.addEventListener('pagehide', flushProjects); + document.addEventListener('visibilitychange', flushProjectsWhenHidden); + + return () => { + window.removeEventListener('pagehide', flushProjects); + document.removeEventListener('visibilitychange', flushProjectsWhenHidden); + flushProjects(); + }; + }, [projectPersistence]); + + useLayoutEffect(() => { saveActiveProjectIdToStorage(activeProjectId); }, [activeProjectId]); diff --git a/app/src/workers/analysis.worker.ts b/app/src/workers/analysis.worker.ts index da61b1bf..4ed8dadf 100644 --- a/app/src/workers/analysis.worker.ts +++ b/app/src/workers/analysis.worker.ts @@ -28,6 +28,7 @@ export interface AnalysisWorkerPayload { export interface SyncFilesPayload { files: Array<{ name: string; content: string }>; + deletedFileNames?: string[]; replace?: boolean; } @@ -371,6 +372,10 @@ self.onmessage = async (event: MessageEvent) => { fileCache.clear(); } + for (const fileName of syncPayload.deletedFileNames ?? []) { + fileCache.delete(fileName); + } + for (const file of syncPayload.files) { fileCache.set(file.name, file.content); }