diff --git a/frontend/editor/public/locales/en-US/translation.toml b/frontend/editor/public/locales/en-US/translation.toml index 33cf0ee8e1..feb42149f7 100644 --- a/frontend/editor/public/locales/en-US/translation.toml +++ b/frontend/editor/public/locales/en-US/translation.toml @@ -6203,6 +6203,7 @@ ssn = "Social Security numbers" [policy] badgeEnforcing = "{{name}} enforcing..." badgeRan = "{{name}} policy ran on this file" +badgeRunning = "{{name}} running..." blockingAction = "{{action}} blocked while enforcing policy, please wait..." dismiss = "Dismiss overlay" enforcingTitle = "Enforcing policy..." diff --git a/frontend/editor/src/core/components/fileEditor/FileEditorThumbnail.module.css b/frontend/editor/src/core/components/fileEditor/FileEditorThumbnail.module.css index a148cc9fa8..1201402bf7 100644 --- a/frontend/editor/src/core/components/fileEditor/FileEditorThumbnail.module.css +++ b/frontend/editor/src/core/components/fileEditor/FileEditorThumbnail.module.css @@ -279,3 +279,16 @@ opacity: 0.5; pointer-events: auto; } + +/* Non-blocking policy run (e.g. classification tagging): small top-right pill + * with the policy's icon + a loader. Colour is set inline to the policy accent. */ +.backgroundPolicyPill { + display: inline-flex; + align-items: center; + gap: 4px; + padding: 4px 7px; + border-radius: 8px; + background: color-mix(in srgb, currentColor 14%, var(--c-surface)); + box-shadow: var(--shadow-md); + pointer-events: auto; +} diff --git a/frontend/editor/src/core/components/fileEditor/FileEditorThumbnail.tsx b/frontend/editor/src/core/components/fileEditor/FileEditorThumbnail.tsx index ccf07e9dea..1a55c53a90 100644 --- a/frontend/editor/src/core/components/fileEditor/FileEditorThumbnail.tsx +++ b/frontend/editor/src/core/components/fileEditor/FileEditorThumbnail.tsx @@ -22,6 +22,7 @@ import { dropTargetForElements, } from "@atlaskit/pragmatic-drag-and-drop/element/adapter"; import { StirlingFileStub } from "@app/types/fileContext"; +import { policyCategoryIcon } from "@app/components/policies/policyCategoryIcon"; import { PolicyBadges, type FileItemPolicyRef, @@ -31,7 +32,10 @@ import { zipFileService } from "@app/services/zipFileService"; import styles from "@app/components/fileEditor/FileEditorThumbnail.module.css"; import { useFileContext } from "@app/contexts/FileContext"; -import { useFileState } from "@app/contexts/file/fileHooks"; +import { + useFileSelector, + useFileSelectors, +} from "@app/contexts/file/fileHooks"; import { FileId } from "@app/types/file"; import ToolChain from "@app/components/shared/ToolChain"; import HoverActionMenu, { @@ -90,7 +94,7 @@ const FileEditorThumbnail = ({ actions: fileActions, openEncryptedUnlockPrompt, } = useFileContext(); - const { state, selectors } = useFileState(); + const selectors = useFileSelectors(); const isMobile = useIsMobile(); const actualFile = useMemo( @@ -101,7 +105,7 @@ const FileEditorThumbnail = ({ const isZipFile = zipFileService.isZipFileStub(file); - const hasError = state.ui.errorFileIds.includes(file.id); + const hasError = useFileSelector((s) => s.ui.errorFileIds.includes(file.id)); const pageCount = file.processedFile?.totalPages || 0; const { isEncrypted, @@ -296,9 +300,12 @@ const FileEditorThumbnail = ({ const [showVersionHistory, setShowVersionHistory] = useState(false); const policyEnforcing = policies.some((p) => p.enforcing); - // Accent of the policy currently enforcing, so the overlay's icon/spinner match - // that policy's badge instead of a fixed blue. - const enforcingAccent = policies.find((p) => p.enforcing)?.accentColor; + // The policy currently enforcing, so the overlay's icon/spinner match that + // policy's badge instead of a fixed blue. + const enforcingPolicy = policies.find((p) => p.enforcing); + // A non-blocking run (e.g. classification tagging) — indicated by a small + // top-right chip instead of the blocking overlay. + const backgroundPolicy = policies.find((p) => p.background && !p.enforcing); const hoverActions = useMemo(() => { const uploadLabel = isUploaded @@ -543,7 +550,8 @@ const FileEditorThumbnail = ({ {/* Thumbnail image or loading state */} @@ -568,6 +576,27 @@ const FileEditorThumbnail = ({ }} /> + {backgroundPolicy && ( + + + + {policyCategoryIcon(backgroundPolicy.id, { + fontSize: 14, + })} + + + + + )} + {/* Badges — top-left: version, pin, ownership, encrypted */}
diff --git a/frontend/editor/src/core/components/layout/Workbench.tsx b/frontend/editor/src/core/components/layout/Workbench.tsx index fc5dd77226..96eb6b8d6f 100644 --- a/frontend/editor/src/core/components/layout/Workbench.tsx +++ b/frontend/editor/src/core/components/layout/Workbench.tsx @@ -2,7 +2,7 @@ import { useEffect, useState, Suspense, lazy } from "react"; import { Box, Loader, Center } from "@mantine/core"; import { useToolWorkflow } from "@app/contexts/ToolWorkflowContext"; import { useFileHandler } from "@app/hooks/useFileHandler"; -import { useFileState } from "@app/contexts/FileContext"; +import { useAllFiles } from "@app/contexts/FileContext"; import { useNavigationState, useNavigationActions, @@ -41,11 +41,10 @@ export default function Workbench() { useCookieConsent({ analyticsEnabled: config?.enableAnalytics === true }); // Use context-based hooks to eliminate all prop drilling - const { selectors } = useFileState(); + const { files: activeFiles } = useAllFiles(); const { workbench: currentView } = useNavigationState(); const { actions: navActions } = useNavigationActions(); const setCurrentView = navActions.setWorkbench; - const activeFiles = selectors.getFiles(); const { previewFile, pageEditorFunctions, diff --git a/frontend/editor/src/core/components/pageEditor/hooks/usePageEditorDropdownState.ts b/frontend/editor/src/core/components/pageEditor/hooks/usePageEditorDropdownState.ts index ac26c142dc..94ed6590ce 100644 --- a/frontend/editor/src/core/components/pageEditor/hooks/usePageEditorDropdownState.ts +++ b/frontend/editor/src/core/components/pageEditor/hooks/usePageEditorDropdownState.ts @@ -1,6 +1,6 @@ import { useMemo } from "react"; import { usePageEditor } from "@app/contexts/PageEditorContext"; -import { useFileState } from "@app/contexts/FileContext"; +import { shallowEqual, useFileSelector } from "@app/contexts/FileContext"; import { FileId } from "@app/types/file"; import { useFileColorMap } from "@app/components/pageEditor/hooks/useFileColorMap"; @@ -24,24 +24,32 @@ const isPdf = (name?: string | null) => typeof name === "string" && name.toLowerCase().endsWith(".pdf"); export function usePageEditorDropdownState(): PageEditorDropdownState { - const { state, selectors } = useFileState(); + const selectedFileIds = useFileSelector((s) => s.ui.selectedFileIds); const { toggleFileSelection, reorderFiles, fileOrder } = usePageEditor(); + // Subscribe to the stubs for the files in view so name/version changes + // re-render the dropdown. Reading via useFileSelectors() during render would + // not subscribe, so the displayed name/version could go stale. + const orderedStubs = useFileSelector( + (s) => fileOrder.map((fileId) => s.files.byId[fileId]), + shallowEqual, + ); + const pageEditorFiles = useMemo(() => { return fileOrder - .map((fileId) => { - const stub = selectors.getStirlingFileStub(fileId); + .map((fileId, index) => { + const stub = orderedStubs[index]; if (!isPdf(stub?.name)) return null; return { fileId, name: stub?.name || "", versionNumber: stub?.versionNumber, - isSelected: state.ui.selectedFileIds.includes(fileId), + isSelected: selectedFileIds.includes(fileId), }; }) .filter((file): file is PageEditorDropdownFile => file !== null); - }, [fileOrder, selectors, state.ui.selectedFileIds]); + }, [fileOrder, orderedStubs, selectedFileIds]); const fileColorMap = useFileColorMap( pageEditorFiles.map((file) => file.fileId), diff --git a/frontend/editor/src/core/components/shared/DismissAllErrorsButton.tsx b/frontend/editor/src/core/components/shared/DismissAllErrorsButton.tsx index 5f3f4ada2a..ea28b78c5e 100644 --- a/frontend/editor/src/core/components/shared/DismissAllErrorsButton.tsx +++ b/frontend/editor/src/core/components/shared/DismissAllErrorsButton.tsx @@ -2,7 +2,7 @@ import React from "react"; import { Group } from "@mantine/core"; import { Button } from "@app/ui/Button"; import { useTranslation } from "react-i18next"; -import { useFileState } from "@app/contexts/FileContext"; +import { useFileSelector } from "@app/contexts/FileContext"; import { useFileActions } from "@app/contexts/file/fileHooks"; import { Z_INDEX_TOAST } from "@app/styles/zIndex"; @@ -14,11 +14,11 @@ const DismissAllErrorsButton: React.FC = ({ className, }) => { const { t } = useTranslation(); - const { state } = useFileState(); + const errorFileIds = useFileSelector((s) => s.ui.errorFileIds); const { actions } = useFileActions(); // Check if there are any files in error state - const hasErrors = state.ui.errorFileIds.length > 0; + const hasErrors = errorFileIds.length > 0; // Don't render if there are no errors if (!hasErrors) { @@ -45,7 +45,7 @@ const DismissAllErrorsButton: React.FC = ({ }} > {t("error.dismissAllErrors", "Dismiss All Errors")} ( - {state.ui.errorFileIds.length}) + {errorFileIds.length}) ); diff --git a/frontend/editor/src/core/components/shared/FileSidebar.tsx b/frontend/editor/src/core/components/shared/FileSidebar.tsx index 7d09261226..742334015c 100644 --- a/frontend/editor/src/core/components/shared/FileSidebar.tsx +++ b/frontend/editor/src/core/components/shared/FileSidebar.tsx @@ -81,9 +81,10 @@ const EXPANDED_WIDTH = "16.25rem"; // ~260px const WATCHED_FOLDER_VIEW_ID = "watchedFolder"; const WATCHED_FOLDER_WORKBENCH_ID = "custom:watchedFolder"; -// Stable empty props for rows without folders, so the memoized FileItem -// isn't re-rendered by a fresh `?? []` identity on every list render. +// Stable empty props for rows without folders/policies, so the memoized +// FileItem isn't re-rendered by a fresh `?? []` identity on every list render. const NO_FOLDERS: never[] = []; +const NO_POLICIES: never[] = []; /** Only surface the "Adding files…" progress row for drops big enough that the * pre-dispatch scan is user-visible; small adds finish before it would paint. */ @@ -790,7 +791,7 @@ const FileSidebar = forwardRef( onDragStart={handleWatchedFolderDragStart} folders={memberFolders} onFolderClick={openWatchedFolder} - policies={policyFileBadges.get(stub.id as string) ?? []} + policies={policyFileBadges.get(stub.id as string) ?? NO_POLICIES} onDelete={isWatchedFoldersActive ? undefined : handleSidebarDelete} onSaveToCloud={isWatchedFoldersActive ? undefined : handleSaveToCloud} canSaveToCloud={storageEnabled && fileOrigin !== "shared-with-me"} diff --git a/frontend/editor/src/core/components/shared/FileSidebarFileItem.tsx b/frontend/editor/src/core/components/shared/FileSidebarFileItem.tsx index a289a245f1..c1a93594c5 100644 --- a/frontend/editor/src/core/components/shared/FileSidebarFileItem.tsx +++ b/frontend/editor/src/core/components/shared/FileSidebarFileItem.tsx @@ -167,7 +167,9 @@ export interface FileItemProps { const MAX_VISIBLE_FOLDER_TAGS = 2; -export function FileItem({ +// Memoized: sidebar rows bail out unless THEIR props change, so one file's +// update (e.g. a new version landing) re-renders one row, not the whole list. +export const FileItem = React.memo(function FileItem({ fileId, name, size, @@ -509,4 +511,4 @@ export function FileItem({ )} ); -} +}); diff --git a/frontend/editor/src/core/components/shared/PolicyBadges.css b/frontend/editor/src/core/components/shared/PolicyBadges.css index 3a919e24c9..8526de44c6 100644 --- a/frontend/editor/src/core/components/shared/PolicyBadges.css +++ b/frontend/editor/src/core/components/shared/PolicyBadges.css @@ -41,22 +41,3 @@ transform: rotate(360deg); } } - -.policy-badge--recent { - animation: policy-badge-pulse 4.5s ease-in-out forwards; -} -@keyframes policy-badge-pulse { - 0%, - 24%, - 48% { - box-shadow: 0 0 0 0 transparent; - } - 12%, - 36% { - box-shadow: 0 0 5px 2px currentColor; - } - 60%, - 100% { - box-shadow: 0 0 0 0 transparent; - } -} diff --git a/frontend/editor/src/core/components/shared/PolicyBadges.stories.tsx b/frontend/editor/src/core/components/shared/PolicyBadges.stories.tsx index 81969bf615..c3646f774a 100644 --- a/frontend/editor/src/core/components/shared/PolicyBadges.stories.tsx +++ b/frontend/editor/src/core/components/shared/PolicyBadges.stories.tsx @@ -2,10 +2,14 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; import { PolicyBadges } from "@app/components/shared/PolicyBadges"; import type { FileItemPolicyRef } from "@app/components/shared/PolicyBadges"; +// Real catalog category ids, so each badge renders its own shared glyph +// (policyCategoryIcon) rather than the unknown-category fallback. Accents mirror +// policyAccentVar's mapping — that lives in the proprietary layer, which a core +// story can't import. const mockPolicies: FileItemPolicyRef[] = [ - { id: "policy-1", name: "Redact PII", accentColor: "#e03131", recent: true }, - { id: "policy-2", name: "Sanitize", accentColor: "#2f9e44", recent: false }, - { id: "policy-3", name: "Watermark", accentColor: "#4263eb", recent: false }, + { id: "security", name: "Redact PII", accentColor: "var(--color-purple)" }, + { id: "compliance", name: "Sanitize", accentColor: "var(--color-green)" }, + { id: "ingestion", name: "Watermark", accentColor: "var(--color-blue)" }, ]; const meta = { @@ -22,15 +26,25 @@ export const Default: Story = { }, }; +/** A blocking policy mid-run: spinner, and the file's exit points are gated. */ export const Enforcing: Story = { + args: { + policies: [ + { ...mockPolicies[0], enforcing: true }, + ...mockPolicies.slice(1), + ], + }, +}; + +/** A non-blocking run (classification tagging): same spinner, nothing gated. */ +export const Background: Story = { args: { policies: [ { - id: "policy-1", - name: "Redact PII", - accentColor: "#e03131", - recent: false, - enforcing: true, + id: "classification", + name: "Classification", + accentColor: "var(--color-orange)", + background: true, }, ...mockPolicies.slice(1), ], diff --git a/frontend/editor/src/core/components/shared/PolicyBadges.tsx b/frontend/editor/src/core/components/shared/PolicyBadges.tsx index f8661b9176..1fa57ca70a 100644 --- a/frontend/editor/src/core/components/shared/PolicyBadges.tsx +++ b/frontend/editor/src/core/components/shared/PolicyBadges.tsx @@ -1,6 +1,6 @@ import { Tooltip } from "@mantine/core"; -import ShieldOutlinedIcon from "@mui/icons-material/ShieldOutlined"; import AutorenewIcon from "@mui/icons-material/Autorenew"; +import { policyCategoryIcon } from "@app/components/policies/policyCategoryIcon"; import { useTranslation } from "react-i18next"; import "@app/components/shared/PolicyBadges.css"; @@ -10,20 +10,20 @@ export interface FileItemPolicyRef { name: string; /** CSS colour for the badge (matches the policy's accent). */ accentColor: string; - /** True only just after the policy was applied — drives the one-off glow, so - * it doesn't replay on every reload of an already-enforced file. */ - recent: boolean; - /** True while the policy run is actively in-flight on this file. */ + /** True while a BLOCKING policy run is in-flight on this file (gates actions). */ enforcing?: boolean; + /** True while a non-blocking run (e.g. classification) is in-flight — shows + * the same spinner but never gates anything. */ + background?: boolean; } const MAX_VISIBLE = 3; /** - * The canonical policy badge row: one accent-tinted shield per policy that has - * run on a file, spinning while a run is in flight, glowing briefly after it - * lands. Every surface that shows per-file policy badges (file sidebar, file - * editor thumbnails, files page) renders this so they stay identical. + * The canonical policy badge row: one accent-tinted category icon per policy + * that has run on a file, spinning while a run is in flight. Every surface that + * shows per-file policy badges (file sidebar, file editor thumbnails, files + * page) renders this so they stay identical. */ export function PolicyBadges({ policies, @@ -40,33 +40,40 @@ export function PolicyBadges({ className={`policy-badges${className ? ` ${className}` : ""}`} data-no-select > - {policies.slice(0, MAX_VISIBLE).map((policy) => ( - - { + const running = policy.enforcing || policy.background; + return ( + - {policy.enforcing ? ( - - ) : ( - - )} - - - ))} + + {running ? ( + + ) : ( + policyCategoryIcon(policy.id, { fontSize: "0.7rem" }) + )} + + + ); + })} ); } diff --git a/frontend/editor/src/core/components/shared/PolicyEnforcingOverlay.tsx b/frontend/editor/src/core/components/shared/PolicyEnforcingOverlay.tsx index cb9e931e80..9d5d2475b3 100644 --- a/frontend/editor/src/core/components/shared/PolicyEnforcingOverlay.tsx +++ b/frontend/editor/src/core/components/shared/PolicyEnforcingOverlay.tsx @@ -4,6 +4,8 @@ export function PolicyEnforcingOverlay(_props: { zIndex?: number; /** CSS colour var for the enforcing policy's accent; tints the icon/spinner. */ accentVar?: string; + /** Category of the enforcing policy — picks its icon in the real overlay. */ + categoryId?: string; }) { return null; } diff --git a/frontend/editor/src/core/components/shared/WorkbenchBar.tsx b/frontend/editor/src/core/components/shared/WorkbenchBar.tsx index 5cc1e6cabd..55ca01d307 100644 --- a/frontend/editor/src/core/components/shared/WorkbenchBar.tsx +++ b/frontend/editor/src/core/components/shared/WorkbenchBar.tsx @@ -19,7 +19,8 @@ import { } from "@app/components/filesPage/filesPageReturnRoute"; import { useWorkbenchBar } from "@app/contexts/WorkbenchBarContext"; import { - useFileState, + useAllFiles, + useFileSelectors, useFileSelection, useFileActions, } from "@app/contexts/FileContext"; @@ -121,10 +122,10 @@ export default function WorkbenchBar({ const { sharingEnabled } = useSharingEnabled(); const viewerContext = React.useContext(ViewerContext); - const { selectors } = useFileState(); + const selectors = useFileSelectors(); const { selectedFiles, selectedFileIds } = useFileSelection(); const { actions: fileActions } = useFileActions(); - const activeFiles = selectors.getFiles(); + const { files: activeFiles } = useAllFiles(); const { activeFileId, setActiveFileId } = useViewer(); const policyFileBadges = usePolicyFileBadges(); // Block print/export while any file the export would touch is under active diff --git a/frontend/editor/src/core/components/tools/convert/ConvertSettings.tsx b/frontend/editor/src/core/components/tools/convert/ConvertSettings.tsx index cc33ccbbb4..fe4a6f8e48 100644 --- a/frontend/editor/src/core/components/tools/convert/ConvertSettings.tsx +++ b/frontend/editor/src/core/components/tools/convert/ConvertSettings.tsx @@ -11,7 +11,7 @@ import { } from "@app/utils/convertUtils"; import { getConversionEndpoints } from "@app/data/toolsTaxonomy"; import { useFileSelection } from "@app/contexts/FileContext"; -import { useFileState } from "@app/contexts/FileContext"; +import { useFileSelector, useFileSelectors } from "@app/contexts/FileContext"; import { detectFileExtension } from "@app/utils/fileUtils"; import { usePreferences } from "@app/contexts/PreferencesContext"; import { useConversionCloudStatus } from "@app/hooks/useConversionCloudStatus"; @@ -62,8 +62,8 @@ const ConvertSettings = ({ const { t } = useTranslation(); const theme = useMantineTheme(); const { setSelectedFiles } = useFileSelection(); - const { state, selectors } = useFileState(); - const activeFiles = state.files.ids; + const selectors = useFileSelectors(); + const activeFiles = useFileSelector((s) => s.files.ids); const { preferences } = usePreferences(); const allEndpoints = useMemo(() => { diff --git a/frontend/editor/src/core/components/tools/shared/ReviewToolStep.tsx b/frontend/editor/src/core/components/tools/shared/ReviewToolStep.tsx index 8c89029913..7918bac7ca 100644 --- a/frontend/editor/src/core/components/tools/shared/ReviewToolStep.tsx +++ b/frontend/editor/src/core/components/tools/shared/ReviewToolStep.tsx @@ -11,7 +11,7 @@ import { Tooltip } from "@app/components/shared/Tooltip"; import { useFileActionTerminology } from "@app/hooks/useFileActionTerminology"; import { useFileActionIcons } from "@app/hooks/useFileActionIcons"; import { saveOperationResults } from "@app/services/operationResultsSaveService"; -import { useFileActions, useFileState } from "@app/contexts/FileContext"; +import { useFileActions, useFileSelectors } from "@app/contexts/FileContext"; import { FileId } from "@app/types/fileContext"; import i18n from "@app/i18n"; @@ -40,7 +40,7 @@ function ReviewStepContent({ const DownloadIcon = icons.download; const stepRef = useRef(null); const { actions: fileActions } = useFileActions(); - const { selectors } = useFileState(); + const selectors = useFileSelectors(); const handleUndo = async () => { try { diff --git a/frontend/editor/src/core/components/viewer/EmbedPdfViewer.tsx b/frontend/editor/src/core/components/viewer/EmbedPdfViewer.tsx index cc4a04598e..be4a4a9971 100644 --- a/frontend/editor/src/core/components/viewer/EmbedPdfViewer.tsx +++ b/frontend/editor/src/core/components/viewer/EmbedPdfViewer.tsx @@ -12,7 +12,12 @@ import { ActionIcon } from "@app/ui/ActionIcon"; import CloseIcon from "@mui/icons-material/Close"; import LockIcon from "@mui/icons-material/Lock"; -import { useFileState, useFileActions } from "@app/contexts/FileContext"; +import { + useAllFiles, + useFileSelector, + useFileSelectors, + useFileActions, +} from "@app/contexts/FileContext"; import { useFileWithUrl } from "@app/hooks/useFileWithUrl"; import { useViewer } from "@app/contexts/ViewerContext"; import { LocalEmbedPDF } from "@app/components/viewer/LocalEmbedPDF"; @@ -259,9 +264,9 @@ const EmbedPdfViewerContent = ({ const redactionTrackerRef = useRef(null); // Get current file from FileContext - const { selectors } = useFileState(); + const selectors = useFileSelectors(); const { actions } = useFileActions(); - const activeFiles = selectors.getFiles(); + const { files: activeFiles } = useAllFiles(); const activeFilesRef = useRef(activeFiles); activeFilesRef.current = activeFiles; const activeFileIds = activeFiles.map((f) => f.fileId); @@ -392,11 +397,11 @@ const EmbedPdfViewerContent = ({ }, [previewFile, fileWithUrl]); // Check if the current file is encrypted (gate the viewer to prevent PDFium crash) - const isCurrentFileEncrypted = React.useMemo(() => { - if (!currentFile || !isStirlingFile(currentFile)) return false; - const stub = selectors.getStirlingFileStub(currentFile.fileId); - return stub?.processedFile?.isEncrypted === true; - }, [currentFile, selectors]); + const isCurrentFileEncrypted = useFileSelector((s) => + currentFile && isStirlingFile(currentFile) + ? s.files.byId[currentFile.fileId]?.processedFile?.isEncrypted === true + : false, + ); const bookmarkCacheKey = React.useMemo(() => { if (currentFile && isStirlingFile(currentFile)) { diff --git a/frontend/editor/src/core/components/viewer/NonPdfViewer.tsx b/frontend/editor/src/core/components/viewer/NonPdfViewer.tsx index ed4a2e2263..bd2704963a 100644 --- a/frontend/editor/src/core/components/viewer/NonPdfViewer.tsx +++ b/frontend/editor/src/core/components/viewer/NonPdfViewer.tsx @@ -4,7 +4,7 @@ import { Button } from "@app/ui/Button"; import ArticleIcon from "@mui/icons-material/Article"; import PictureAsPdfIcon from "@mui/icons-material/PictureAsPdf"; -import { useFileState } from "@app/contexts/FileContext"; +import { useAllFiles } from "@app/contexts/FileContext"; import { useViewer } from "@app/contexts/ViewerContext"; import { useToolWorkflow } from "@app/contexts/ToolWorkflowContext"; import { @@ -126,8 +126,7 @@ export function NonPdfViewer({ file }: NonPdfViewerProps) { // ─── Wrapper that resolves the active file from FileContext ─────────────────── export function NonPdfViewerWrapper(props: ViewerProps) { - const { selectors } = useFileState(); - const activeFiles = selectors.getFiles(); + const { files: activeFiles } = useAllFiles(); const { activeFileIndex } = useViewer(); const file = diff --git a/frontend/editor/src/core/components/viewer/Viewer.tsx b/frontend/editor/src/core/components/viewer/Viewer.tsx index 08103ca1f5..36a1080dbb 100644 --- a/frontend/editor/src/core/components/viewer/Viewer.tsx +++ b/frontend/editor/src/core/components/viewer/Viewer.tsx @@ -5,7 +5,7 @@ import { NonPdfViewerWrapper, type ViewerProps, } from "@app/components/viewer/NonPdfViewer"; -import { useFileState } from "@app/contexts/FileContext"; +import { useAllFiles } from "@app/contexts/FileContext"; import { useViewer } from "@app/contexts/ViewerContext"; import { isStirlingFile } from "@app/types/fileContext"; import { isPdfFile } from "@app/utils/fileUtils"; @@ -26,8 +26,7 @@ type SignatureOverlayPassThrough = Pick< >; const Viewer = (props: ViewerProps & SignatureOverlayPassThrough) => { - const { selectors } = useFileState(); - const activeFiles = selectors.getFiles(); + const { files: activeFiles } = useAllFiles(); const { activeFileId } = useViewer(); // Determine the active file — previewFile takes priority, then look up by stable ID diff --git a/frontend/editor/src/core/components/viewer/ViewerAnnotationControls.tsx b/frontend/editor/src/core/components/viewer/ViewerAnnotationControls.tsx index 32b4b1ce11..fda6c2d2a5 100644 --- a/frontend/editor/src/core/components/viewer/ViewerAnnotationControls.tsx +++ b/frontend/editor/src/core/components/viewer/ViewerAnnotationControls.tsx @@ -5,7 +5,11 @@ import { ActionIcon } from "@app/ui/ActionIcon"; import { Tooltip } from "@app/components/shared/Tooltip"; import { ViewerContext } from "@app/contexts/ViewerContext"; import { useSignature } from "@app/contexts/SignatureContext"; -import { useFileState, useFileContext } from "@app/contexts/FileContext"; +import { + useAllFiles, + useFileSelectors, + useFileContext, +} from "@app/contexts/FileContext"; import { createStirlingFilesAndStubs } from "@app/services/fileStubHelpers"; import { useNavigationState, @@ -39,9 +43,9 @@ export default function ViewerAnnotationControls({ const { historyApiRef, isPlacementMode } = useSignature(); // File state for save functionality - const { state, selectors } = useFileState(); + const selectors = useFileSelectors(); + const { files: activeFiles, fileIds } = useAllFiles(); const { actions: fileActions } = useFileContext(); - const activeFiles = selectors.getFiles(); // Check if we're in sign mode or redaction mode const { selectedTool } = useNavigationState(); @@ -83,7 +87,7 @@ export default function ViewerAnnotationControls({ !historyApiRef?.current?.canUndo() ) return; - if (activeFiles.length === 0 || state.files.ids.length === 0) return; + if (activeFiles.length === 0 || fileIds.length === 0) return; try { const arrayBuffer = await viewerContext.exportActions.saveAsCopy(); @@ -92,7 +96,7 @@ export default function ViewerAnnotationControls({ const file = new File([new Blob([arrayBuffer])], activeFiles[0].name, { type: "application/pdf", }); - const parentStub = selectors.getStirlingFileStub(state.files.ids[0]); + const parentStub = selectors.getStirlingFileStub(fileIds[0]); if (!parentStub) return; const { stirlingFiles, stubs } = await createStirlingFilesAndStubs( @@ -100,11 +104,7 @@ export default function ViewerAnnotationControls({ parentStub, "redact", ); - await fileActions.consumeFiles( - [state.files.ids[0]], - stirlingFiles, - stubs, - ); + await fileActions.consumeFiles([fileIds[0]], stirlingFiles, stubs); // Clear unsaved changes flags after successful save setHasUnsavedChanges(false); diff --git a/frontend/editor/src/core/components/viewer/ViewerShareButton.tsx b/frontend/editor/src/core/components/viewer/ViewerShareButton.tsx index f063cbffe2..4ef4b30cda 100644 --- a/frontend/editor/src/core/components/viewer/ViewerShareButton.tsx +++ b/frontend/editor/src/core/components/viewer/ViewerShareButton.tsx @@ -9,7 +9,7 @@ import CloudUploadIcon from "@mui/icons-material/CloudUpload"; import { Tooltip } from "@app/components/shared/Tooltip"; import ShareManagementModal from "@app/components/shared/ShareManagementModal"; import { useViewer } from "@app/contexts/ViewerContext"; -import { useFileState, useFileActions } from "@app/contexts/FileContext"; +import { useAllFiles, useFileActions } from "@app/contexts/FileContext"; import { uploadHistoryChain } from "@app/services/serverStorageUpload"; import { fileStorage } from "@app/services/fileStorage"; import { alert } from "@app/components/toast"; @@ -39,7 +39,7 @@ export default function ViewerShareButton({ }: ViewerShareButtonProps) { const { t } = useTranslation(); const { activeFileId } = useViewer(); - const { selectors } = useFileState(); + const { fileStubs } = useAllFiles(); const { actions } = useFileActions(); const [confirmOpen, setConfirmOpen] = useState(false); const [saving, setSaving] = useState(false); @@ -49,7 +49,7 @@ export default function ViewerShareButton({ // Resolve strictly to the file shown in the viewer. Never fall back to an // arbitrary file — sharing the wrong document would be worse than not // sharing. If there's no active file, the button is disabled (see isDisabled). - const stubs = selectors.getStirlingFileStubs(); + const stubs = fileStubs; const stub = activeFileId ? stubs.find((s) => s.id === activeFileId) : undefined; diff --git a/frontend/editor/src/core/components/viewer/ZoomAPIBridge.tsx b/frontend/editor/src/core/components/viewer/ZoomAPIBridge.tsx index 2501ddfb79..0cb11837cc 100644 --- a/frontend/editor/src/core/components/viewer/ZoomAPIBridge.tsx +++ b/frontend/editor/src/core/components/viewer/ZoomAPIBridge.tsx @@ -3,7 +3,7 @@ import { useZoom, ZoomMode } from "@embedpdf/plugin-zoom/react"; import { useSpread, SpreadMode } from "@embedpdf/plugin-spread/react"; import { useViewer } from "@app/contexts/ViewerContext"; import { useActiveDocumentId } from "@app/components/viewer/useActiveDocumentId"; -import { useFileState } from "@app/contexts/FileContext"; +import { useAllFiles } from "@app/contexts/FileContext"; import { determineAutoZoom, DEFAULT_FALLBACK_ZOOM, @@ -36,7 +36,7 @@ function ZoomAPIBridgeInner({ documentId }: { documentId: string }) { const { provides: zoom, state: zoomState } = useZoom(documentId); const { spreadMode } = useSpread(documentId); const { registerBridge, triggerImmediateZoomUpdate } = useViewer(); - const { selectors } = useFileState(); + const { fileStubs } = useAllFiles(); const hasSetInitialZoom = useRef(false); const lastSpreadMode = useRef(spreadMode ?? SpreadMode.None); @@ -62,7 +62,7 @@ function ZoomAPIBridgeInner({ documentId }: { documentId: string }) { } }, []); - const stubs = selectors.getStirlingFileStubs(); + const stubs = fileStubs; const firstFileStub = stubs[0]; const firstFileId = firstFileStub?.id; diff --git a/frontend/editor/src/core/components/viewer/useViewerReadAloud.ts b/frontend/editor/src/core/components/viewer/useViewerReadAloud.ts index c2ccdec1be..5352ffc748 100644 --- a/frontend/editor/src/core/components/viewer/useViewerReadAloud.ts +++ b/frontend/editor/src/core/components/viewer/useViewerReadAloud.ts @@ -1,6 +1,6 @@ import { useCallback, useEffect, useRef, useState } from "react"; import { computeReadAloudHighlightRect } from "@app/components/viewer/readAloudHighlight"; -import { useFileState } from "@app/contexts/FileContext"; +import { useFileSelectors } from "@app/contexts/FileContext"; import { useViewer } from "@app/contexts/ViewerContext"; import { useStopReadAloudOnNavigation } from "@app/components/viewer/useStopReadAloudOnNavigation"; import { pdfWorkerManager } from "@app/services/pdfWorkerManager"; @@ -60,7 +60,7 @@ function createHighlightElement( export function useViewerReadAloud(defaultLanguage?: string) { const viewer = useViewer(); - const { selectors } = useFileState(); + const selectors = useFileSelectors(); const [isReadingAloud, setIsReadingAloud] = useState(false); const [speechRate, setSpeechRate] = useState(1); diff --git a/frontend/editor/src/core/contexts/FileContext.tsx b/frontend/editor/src/core/contexts/FileContext.tsx index d0808665cd..a982c347ca 100644 --- a/frontend/editor/src/core/contexts/FileContext.tsx +++ b/frontend/editor/src/core/contexts/FileContext.tsx @@ -16,6 +16,7 @@ import { useReducer, useCallback, useEffect, + useLayoutEffect, useRef, useMemo, useState, @@ -23,7 +24,6 @@ import { import { FileContextProviderProps, FileContextSelectors, - FileContextStateValue, FileContextActionsValue, FileContextActions, FileId, @@ -36,6 +36,7 @@ import { import { fileContextReducer, initialFileContextState, + withReducerIdentityGuard, } from "@app/contexts/file/FileReducer"; import { createFileSelectors } from "@app/contexts/file/fileSelectors"; import { @@ -49,8 +50,9 @@ import { } from "@app/contexts/file/fileActions"; import { FileLifecycleManager } from "@app/contexts/file/lifecycle"; import { - FileStateContext, + FileStoreContext, FileActionsContext, + type FileStateStore, } from "@app/contexts/file/contexts"; import { IndexedDBProvider, @@ -75,10 +77,13 @@ function FileContextInner({ children, enablePersistence = true, }: FileContextProviderProps) { - const [state, dispatch] = useReducer( - fileContextReducer, - initialFileContextState, + // Guarded in dev: warns if a reducer case reallocates a slice without changing + // it, which would silently defeat the selector-subscription bail-out. + const guardedReducer = useMemo( + () => withReducerIdentityGuard(fileContextReducer), + [], ); + const [state, dispatch] = useReducer(guardedReducer, initialFileContextState); // Always call the hook unconditionally to satisfy React's rules of hooks. // IndexedDB context is only used when enablePersistence is true. @@ -657,14 +662,28 @@ function FileContextInner({ ], ); - // Split context values to minimize re-renders - const stateValue = useMemo( + // Subscription store bridge: the context value is STABLE, so consumers only + // re-render when the slice they select (via useFileSelector) changes — not on + // every state change. Listeners are notified after each committed state. + const listenersRef = useRef void>>(new Set()); + const store = useMemo( () => ({ - state, + getState: () => stateRef.current, + subscribe: (listener) => { + listenersRef.current.add(listener); + return () => { + listenersRef.current.delete(listener); + }; + }, selectors, }), - [state, selectors], + [selectors], ); + // Layout effect (not passive): subscribers re-render before the browser + // paints, so a state change can never show a frame with stale consumers. + useLayoutEffect(() => { + for (const listener of listenersRef.current) listener(); + }, [state]); const actionsValue = useMemo( () => ({ @@ -698,7 +717,7 @@ function FileContextInner({ }, [lifecycleManager]); return ( - + {children} - + ); } @@ -758,6 +777,10 @@ export function FileContextProvider({ export { useFileState, useFileActions, + useFileSelector, + useFileSelectors, + useFileIndex, + shallowEqual, useCurrentFile, useFileSelection, useFileManagement, diff --git a/frontend/editor/src/core/contexts/ViewerContext.tsx b/frontend/editor/src/core/contexts/ViewerContext.tsx index 22d95606c1..5e5d08dbb4 100644 --- a/frontend/editor/src/core/contexts/ViewerContext.tsx +++ b/frontend/editor/src/core/contexts/ViewerContext.tsx @@ -9,7 +9,11 @@ import React, { useCallback, } from "react"; import { useNavigation } from "@app/contexts/NavigationContext"; -import { useFileState } from "@app/contexts/FileContext"; +import { + useFileIndex, + useFileSelector, + useFileSelectors, +} from "@app/contexts/FileContext"; import { isStirlingFile } from "@app/types/fileContext"; import type { FileId } from "@app/types/file"; import { enforceExportPolicies } from "@app/services/policyExport"; @@ -244,25 +248,21 @@ export const ViewerProvider: React.FC = ({ children }) => { const [activeFileId, setActiveFileId] = useState(null); // activeFileIndex is derived from activeFileId so they can never desync. - // ViewerProvider sits inside FileContextProvider so useFileState is valid here. - const { selectors, state } = useFileState(); + // ViewerProvider sits inside FileContextProvider so these hooks are valid here. + const selectors = useFileSelectors(); + const fileIds = useFileSelector((s) => s.files.ids); // Clear activeFileId when its file is removed from the workbench. // Dep on state.files.ids so the effect re-runs on every add/remove. useEffect(() => { if (!activeFileId) return; - const stillInWorkbench = state.files.ids.some( + const stillInWorkbench = fileIds.some( (id) => (id as string) === activeFileId, ); if (!stillInWorkbench) setActiveFileId(null); - }, [activeFileId, state.files.ids]); + }, [activeFileId, fileIds]); - const activeFileIndex = useMemo(() => { - if (!activeFileId) return 0; - const files = selectors.getFiles(); - const idx = files.findIndex((f) => f.fileId === activeFileId); - return idx >= 0 ? idx : 0; - }, [activeFileId, selectors]); + const activeFileIndex = useFileIndex(activeFileId); const setActiveFileIndex = useCallback( (index: number) => { const files = selectors.getFiles(); diff --git a/frontend/editor/src/core/contexts/file/FileReducer.ts b/frontend/editor/src/core/contexts/file/FileReducer.ts index 91bd62cf4d..2697974823 100644 --- a/frontend/editor/src/core/contexts/file/FileReducer.ts +++ b/frontend/editor/src/core/contexts/file/FileReducer.ts @@ -425,3 +425,83 @@ export function fileContextReducer( return state; } } + +// ── Dev-only structural-sharing guard ────────────────────────────────────── +// +// The file hooks bail a consumer out of re-rendering when the slice it selects +// keeps its object identity across a dispatch. That optimisation silently +// breaks if a reducer case returns a NEW identity for a slice it didn't +// actually change (e.g. an unnecessary `{ ...state.files }`): every consumer of +// that slice re-renders for nothing, with no test failure. This wrapper warns +// when that happens. No-op in production. + +function idsUnchanged(a: FileId[], b: FileId[]): boolean { + return a.length === b.length && a.every((id, i) => id === b[i]); +} + +function byIdUnchanged( + a: Record, + b: Record, +): boolean { + const keysA = Object.keys(a); + return ( + keysA.length === Object.keys(b).length && + keysA.every((id) => a[id as FileId] === b[id as FileId]) + ); +} + +function uiUnchanged( + a: FileContextState["ui"], + b: FileContextState["ui"], +): boolean { + return (Object.keys(a) as Array).every( + (k) => a[k] === b[k], + ); +} + +function setUnchanged(a: Set, b: Set): boolean { + if (a.size !== b.size) return false; + for (const v of a) if (!b.has(v)) return false; + return true; +} + +/** + * Wrap a reducer so, outside production, it warns when an action reallocates a + * top-level state slice without changing its contents — which would defeat the + * selector-subscription bail-out in the file hooks. + */ +export function withReducerIdentityGuard( + reducer: (s: FileContextState, a: FileContextAction) => FileContextState, +): (s: FileContextState, a: FileContextAction) => FileContextState { + if (process.env.NODE_ENV === "production") return reducer; + return (state, action) => { + const next = reducer(state, action); + if (next === state) return next; + if ( + next.files !== state.files && + idsUnchanged(next.files.ids, state.files.ids) && + byIdUnchanged(next.files.byId, state.files.byId) + ) { + console.error( + `[FileReducer] '${action.type}' reallocated state.files without changing it — ` + + "this re-renders every file consumer for nothing. Return the existing slice unchanged.", + ); + } + if (next.ui !== state.ui && uiUnchanged(next.ui, state.ui)) { + console.error( + `[FileReducer] '${action.type}' reallocated state.ui without changing it — ` + + "this re-renders every UI consumer for nothing. Return the existing slice unchanged.", + ); + } + if ( + next.pinnedFiles !== state.pinnedFiles && + setUnchanged(next.pinnedFiles, state.pinnedFiles) + ) { + console.error( + `[FileReducer] '${action.type}' reallocated state.pinnedFiles without changing it — ` + + "this re-renders every pinned-files consumer for nothing.", + ); + } + return next; + }; +} diff --git a/frontend/editor/src/core/contexts/file/classificationToolRace.test.ts b/frontend/editor/src/core/contexts/file/classificationToolRace.test.ts new file mode 100644 index 0000000000..faf6501a0c --- /dev/null +++ b/frontend/editor/src/core/contexts/file/classificationToolRace.test.ts @@ -0,0 +1,167 @@ +import { describe, it, expect } from "vitest"; +import { fileContextReducer } from "@app/contexts/file/FileReducer"; +import type { + FileContextAction, + FileContextState, + StirlingFileStub, +} from "@app/types/fileContext"; +import type { FileId } from "@app/types/file"; + +/** + * Classification is non-blocking: while it runs, the user can manually run a + * tool on the same file. Classification's only write is a metadata-only, + * shallow-merged UPDATE_FILE_RECORD stamping `classificationLabels`; a manual + * tool run produces a NEW document via CONSUME_FILES (new id + version). These + * tests drive the REAL reducer through every interleaving (classification lands + * before / during / after the tool run) and prove the invariant the design + * relies on: the tool's output document is byte-for-byte what the tool produced, + * regardless of when classification lands. (Label PLACEMENT in the mid-run race + * is the orchestration's job — usePolicyAutoRun resolves targets at write time; + * see usePolicyAutoRun.race.test.tsx. Here we lock the reducer backstop.) + */ + +const stub = ( + id: string, + extra: Partial = {}, +): StirlingFileStub => + ({ + id: id as FileId, + name: "doc.pdf", + versionNumber: 1, + ...extra, + }) as StirlingFileStub; + +function stateWith(...stubs: StirlingFileStub[]): FileContextState { + return { + files: { + ids: stubs.map((s) => s.id), + byId: Object.fromEntries(stubs.map((s) => [s.id, s])) as Record< + FileId, + StirlingFileStub + >, + }, + pinnedFiles: new Set(), + ui: { + selectedFileIds: [], + selectedPageNumbers: [], + isProcessing: false, + processingProgress: 0, + hasUnsavedChanges: false, + errorFileIds: [], + }, + }; +} + +const LABELS = ["Invoice"]; + +// A manual tool run on `inputId` producing a new versioned document `outputId`. +// Mirrors what useToolOperation dispatches: the reducer stamps provenance +// (derivedFromTool, sourceFileIds) and inherits labels itself. +const toolRun = (inputId: string, outputId: string): FileContextAction => ({ + type: "CONSUME_FILES", + payload: { + inputFileIds: [inputId as FileId], + outputStirlingFileStubs: [stub(outputId, { versionNumber: 2 })], + silent: false, + }, +}); + +// Classification stamping labels onto a target id (the reducer merges shallowly). +const classify = (targetId: string): FileContextAction => ({ + type: "UPDATE_FILE_RECORD", + payload: { + id: targetId as FileId, + updates: { classificationLabels: LABELS }, + }, +}); + +describe("classification landing vs a manually-run tool", () => { + it("PRE: classification lands first — tool output is correct AND inherits the label", () => { + let s = stateWith(stub("orig")); + s = fileContextReducer(s, classify("orig")); + s = fileContextReducer(s, toolRun("orig", "out")); + + const out = s.files.byId["out" as FileId]; + expect(out).toBeDefined(); + expect(out.versionNumber).toBe(2); // the document the tool produced + expect(s.files.byId["orig" as FileId]).toBeUndefined(); // input consumed + // Label carried forward onto the tool's new version. + expect(out.classificationLabels).toEqual(LABELS); + }); + + it("POST: classification lands after the tool run, targeting the new leaf — output untouched, label applied, nothing else clobbered", () => { + let s = stateWith(stub("orig")); + s = fileContextReducer(s, toolRun("orig", "out")); + + const before = s.files.byId["out" as FileId]; + // classificationLabelTargets resolves the run's descendants: "out" matches + // because its sourceFileIds includes "orig". + expect(before.sourceFileIds).toContain("orig" as FileId); + + s = fileContextReducer(s, classify("out")); + const after = s.files.byId["out" as FileId]; + + // The label write is a shallow merge: ONLY classificationLabels changes. + expect(after.classificationLabels).toEqual(LABELS); + expect({ ...after, classificationLabels: undefined }).toEqual({ + ...before, + classificationLabels: undefined, + }); + expect(after.versionNumber).toBe(2); + }); + + it("MID (the race): a label write aimed at an already-consumed id no-ops — output document is CORRECT, nothing is resurrected", () => { + // In production this stale-id write no longer happens: usePolicyAutoRun + // resolves the label targets AT WRITE TIME, so the labels land on the live + // leaf instead (see usePolicyAutoRun.race.test.tsx). This test locks the + // reducer-level BACKSTOP behind that: even if a stale id does get written, + // it cannot corrupt or resurrect anything. + let s = stateWith(stub("orig")); + + const staleTargetId = "orig"; + + // During that window the user runs a tool: orig -> out. orig had no labels + // yet, so the new leaf inherits none. + s = fileContextReducer(s, toolRun("orig", "out")); + const out = s.files.byId["out" as FileId]; + expect(out.versionNumber).toBe(2); + expect(out.classificationLabels).toBeUndefined(); + + // Classification's write finally lands — on the now-consumed snapshot id. + const beforeWrite = s; + s = fileContextReducer(s, classify(staleTargetId)); + + // No-op on a missing record: reducer returns the SAME state reference, so no + // zombie "orig" record is resurrected and nothing is corrupted. + expect(s).toBe(beforeWrite); + expect(s.files.byId["orig" as FileId]).toBeUndefined(); + + // The tool's output document is intact and exactly what the tool produced. + const finalOut = s.files.byId["out" as FileId]; + expect(finalOut.versionNumber).toBe(2); + expect(finalOut.sourceFileIds).toContain("orig" as FileId); + // At the reducer level the stale write leaves the leaf unlabelled — which + // is why the orchestration resolves targets at write time instead. The + // DOCUMENT is unaffected either way. + expect(finalOut.classificationLabels).toBeUndefined(); + }); + + it("classification can never overwrite a tool output's document fields (only the label)", () => { + // Tool output already carries its own state; classification must not disturb it. + let s = stateWith( + stub("out", { + versionNumber: 7, + thumbnailUrl: "blob:thumb", + isPinned: true, + } as Partial), + ); + + s = fileContextReducer(s, classify("out")); + const after = s.files.byId["out" as FileId]; + + expect(after.versionNumber).toBe(7); + expect(after.thumbnailUrl).toBe("blob:thumb"); + expect((after as { isPinned?: boolean }).isPinned).toBe(true); + expect(after.classificationLabels).toEqual(LABELS); + }); +}); diff --git a/frontend/editor/src/core/contexts/file/contexts.ts b/frontend/editor/src/core/contexts/file/contexts.ts index c17a178043..6bc74e11ae 100644 --- a/frontend/editor/src/core/contexts/file/contexts.ts +++ b/frontend/editor/src/core/contexts/file/contexts.ts @@ -4,14 +4,28 @@ import { createContext } from "react"; import { + FileContextState, + FileContextSelectors, FileContextStateValue, FileContextActionsValue, } from "@app/types/fileContext"; -// Split contexts for performance -export const FileStateContext = createContext< - FileContextStateValue | undefined ->(undefined); +/** + * Subscription store for file state. The context VALUE is stable — consumers + * subscribe and select slices (see useFileSelector), re-rendering only when + * their selected slice changes, instead of on every state change. + */ +export interface FileStateStore { + getState: () => FileContextState; + subscribe: (listener: () => void) => () => void; + /** Stable selector API (reads live state via refs). */ + selectors: FileContextSelectors; +} + +export const FileStoreContext = createContext( + undefined, +); + export const FileActionsContext = createContext< FileContextActionsValue | undefined >(undefined); diff --git a/frontend/editor/src/core/contexts/file/fileActions.ts b/frontend/editor/src/core/contexts/file/fileActions.ts index 77447003f6..db99e1c673 100644 --- a/frontend/editor/src/core/contexts/file/fileActions.ts +++ b/frontend/editor/src/core/contexts/file/fileActions.ts @@ -370,32 +370,57 @@ export async function addFiles( // Collect hydrations to schedule after dispatch so updateStirlingFileStub finds files in state. const pendingHydrations: Array<() => Promise> = []; + // Per-chunk persistence promises (kicked off as chunks flush, awaited before + // return). See flushChunk — we stream writes instead of one batch at the end. + const persistPromises: Array> = []; - // Stream the batch into the workspace in chunks. The per-file pre-scan below - // (dedupe, encryption sniff — which reads each PDF's bytes) takes real time - // for a big folder drop; a single end-of-loop dispatch would leave the UI - // frozen-looking for seconds and then dump hundreds of rows in one render. - // Chunked dispatch keeps rows (and their thumbnail hydrations) streaming in, - // and the progress store drives the sidebar's "Adding files…" indicator. - const DISPATCH_CHUNK = 25; + // Dispatch stubs in chunks so rows (and thumbnail hydrations) stream in + // rather than dumping the whole drop in one render. + const DISPATCH_CHUNK = 5; let flushedStubs = 0; let flushedHydrations = 0; - const flushChunk = () => { - if ( - !options.skipWorkspaceDispatch && - stirlingFileStubs.length > flushedStubs - ) { - dispatch({ - type: "ADD_FILES", - payload: { stirlingFileStubs: stirlingFileStubs.slice(flushedStubs) }, - }); + // Flushes the pending chunk and returns this chunk's persistence promises, + // so the caller can await the writes (see the loop's yield) before the policy + // auto-run tries to read the file back from storage. + const flushChunk = (): Array> => { + const chunkWrites: Array> = []; + if (stirlingFileStubs.length > flushedStubs) { + const from = flushedStubs; + const newStubs = stirlingFileStubs.slice(from); flushedStubs = stirlingFileStubs.length; + if (!options.skipWorkspaceDispatch) { + dispatch({ + type: "ADD_FILES", + payload: { stirlingFileStubs: newStubs }, + }); + } + // Persist each chunk as it flushes, not one batch at the end: the policy + // auto-run reads files from IndexedDB with no in-memory fallback. + if (enablePersistence) { + const newFiles = stirlingFiles.slice(from); + for (let i = 0; i < newFiles.length; i++) { + const sf = newFiles[i]; + const stub = newStubs[i]; + const write = fileStorage + .storeStirlingFile(sf, stub) + .catch((error) => { + console.error( + "Failed to persist file to storage:", + sf.name, + error, + ); + }); + chunkWrites.push(write); + persistPromises.push(write); + } + } } // Hydrations only after their chunk is dispatched, so // updateStirlingFileStub finds the files in state. while (flushedHydrations < pendingHydrations.length) { scheduleMetadataHydration(pendingHydrations[flushedHydrations++]); } + return chunkWrites; }; reportBulkAddProgress(0, filesToProcess.length); @@ -554,38 +579,25 @@ export async function addFiles( reportBulkAddProgress(++scannedCount, filesToProcess.length); if (stirlingFileStubs.length - flushedStubs >= DISPATCH_CHUNK) { - flushChunk(); + const chunkWrites = flushChunk(); + // Yield a MACROTASK so React commits this chunk and runs its effects + // (incl. the policy-enforcement dispatch) before the next chunk scans. + // The per-file awaits above are only microtasks, which don't give React + // a turn — without this, all dispatches batch and processing can't begin + // until the whole drop is scanned. Awaiting the chunk's writes first means + // the auto-run finds each file's bytes already committed in storage. + await Promise.all(chunkWrites); + await new Promise((resolve) => setTimeout(resolve)); } } // Flush the remainder (also the sole dispatch for small batches). flushChunk(); - // Persist to storage if enabled using fileStorage service - if (enablePersistence && stirlingFiles.length > 0) { - await Promise.all( - stirlingFiles.map(async (stirlingFile, index) => { - try { - // Get corresponding stub with all metadata - const fileStub = stirlingFileStubs[index]; - - // Store using the cleaner signature - pass StirlingFile + StirlingFileStub directly - await fileStorage.storeStirlingFile(stirlingFile, fileStub); - - if (DEBUG) - console.log( - `📄 addFiles: Stored file ${stirlingFile.name} with metadata:`, - fileStub, - ); - } catch (error) { - console.error( - "Failed to persist file to storage:", - stirlingFile.name, - error, - ); - } - }), - ); + // Wait for the per-chunk writes (streamed in flushChunk) to commit, so + // addFiles only resolves once every file is durably stored. + if (enablePersistence && persistPromises.length > 0) { + await Promise.all(persistPromises); } if (!options.skipUploadTracking && stirlingFiles.length > 0) { diff --git a/frontend/editor/src/core/contexts/file/fileHooks.selector.test.tsx b/frontend/editor/src/core/contexts/file/fileHooks.selector.test.tsx new file mode 100644 index 0000000000..5a8f3243f9 --- /dev/null +++ b/frontend/editor/src/core/contexts/file/fileHooks.selector.test.tsx @@ -0,0 +1,223 @@ +import { describe, it, expect, vi } from "vitest"; +import { render, act } from "@testing-library/react"; +import { useEffect } from "react"; +import { MantineProvider } from "@mantine/core"; +import { FileContextProvider } from "@app/contexts/FileContext"; +import { + useAllFiles, + useFileContext, + useFileSelection, + useFileSelectors, + useStirlingFileStub, + useFileActions, +} from "@app/contexts/file/fileHooks"; +import type { StirlingFileStub } from "@app/types/fileContext"; +import type { FileId } from "@app/types/file"; +import type { FileContextAction } from "@app/types/fileContext"; + +/** + * Proves the selector-subscription contract: a consumer re-renders only when + * the slice it selects changes — a single file's update doesn't re-render + * other files' consumers, and selection changes don't re-render list consumers. + */ + +const stub = (id: string): StirlingFileStub => + ({ + id: id as FileId, + name: `${id}.pdf`, + type: "application/pdf", + size: 1, + lastModified: 0, + }) as StirlingFileStub; + +const renders: Record = {}; +let dispatchRef: React.Dispatch | null = null; + +function Controller() { + const { dispatch } = useFileActions(); + dispatchRef = dispatch; + return null; +} + +function StubWatcher({ fileId }: { fileId: string }) { + useStirlingFileStub(fileId as FileId); + renders[`stub-${fileId}`] = (renders[`stub-${fileId}`] ?? 0) + 1; + return null; +} + +function ListWatcher() { + useAllFiles(); + renders.list = (renders.list ?? 0) + 1; + return null; +} + +function SelectionWatcher() { + useFileSelection(); + renders.selection = (renders.selection ?? 0) + 1; + return null; +} + +function setup() { + for (const key of Object.keys(renders)) delete renders[key]; + dispatchRef = null; + render( + + + + + + + + + , + ); + act(() => { + dispatchRef!({ + type: "ADD_FILES", + payload: { stirlingFileStubs: [stub("a"), stub("b")] }, + }); + }); + return { ...renders }; +} + +describe("file hooks — selector subscriptions", () => { + it("updating one file re-renders that file's consumer, not the other's", () => { + const before = setup(); + act(() => { + dispatchRef!({ + type: "UPDATE_FILE_RECORD", + payload: { id: "b" as FileId, updates: { name: "renamed.pdf" } }, + }); + }); + expect(renders["stub-b"]).toBeGreaterThan(before["stub-b"]); + expect(renders["stub-a"]).toBe(before["stub-a"]); + }); + + it("selection changes don't re-render file-list or per-file consumers", () => { + const before = setup(); + act(() => { + dispatchRef!({ + type: "SET_SELECTED_FILES", + payload: { fileIds: ["a" as FileId] }, + }); + }); + expect(renders.selection).toBeGreaterThan(before.selection); + expect(renders.list).toBe(before.list); + expect(renders["stub-a"]).toBe(before["stub-a"]); + expect(renders["stub-b"]).toBe(before["stub-b"]); + }); + + it("file-list changes don't re-render selection-only consumers", () => { + const before = setup(); + act(() => { + dispatchRef!({ + type: "UPDATE_FILE_RECORD", + payload: { id: "a" as FileId, updates: { name: "x.pdf" } }, + }); + }); + expect(renders.selection).toBe(before.selection); + }); +}); + +describe("useFileSelectors — render-phase misuse guard", () => { + const guardErrors = (spy: ReturnType) => + spy.mock.calls.filter((args) => + String(args[0]).includes("[useFileSelectors]"), + ); + + function RenderTimeMisuse() { + const selectors = useFileSelectors(); + selectors.getAllFileIds(); // during render — must be flagged + return null; + } + + function EffectTimeUse() { + const selectors = useFileSelectors(); + useEffect(() => { + selectors.getAllFileIds(); // after commit — legitimate + }, [selectors]); + return null; + } + + it("flags a selector invoked during render", () => { + const spy = vi.spyOn(console, "error").mockImplementation(() => {}); + render( + + + + + , + ); + expect(guardErrors(spy).length).toBeGreaterThan(0); + spy.mockRestore(); + }); + + it("does not flag selector reads from effects", () => { + const spy = vi.spyOn(console, "error").mockImplementation(() => {}); + render( + + + + + , + ); + expect(guardErrors(spy)).toHaveLength(0); + spy.mockRestore(); + }); +}); + +describe("useFileContext — render-phase misuse guard", () => { + // useFileContext subscribes to files + pinnedFiles only, so a render-time read + // of the SELECTION slice through its exposed selectors would silently go + // stale. The guard covers exactly those selectors and nothing else. + const guardErrors = (spy: ReturnType) => + spy.mock.calls.filter((args) => + String(args[0]).includes("[useFileContext]"), + ); + + const renderWithGuard = (node: React.ReactNode) => { + const spy = vi.spyOn(console, "error").mockImplementation(() => {}); + render( + + {node} + , + ); + const errors = guardErrors(spy); + spy.mockRestore(); + return errors; + }; + + function SelectionReadDuringRender() { + const { selectors } = useFileContext(); + selectors.getSelectedFiles(); // unsubscribed slice — must be flagged + return null; + } + + function FilesReadDuringRender() { + const { selectors } = useFileContext(); + selectors.getStirlingFileStubs(); // files slice IS subscribed — legitimate + return null; + } + + function SelectionReadFromEffect() { + const { selectors } = useFileContext(); + useEffect(() => { + selectors.getSelectedFiles(); // after commit — legitimate + }, [selectors]); + return null; + } + + it("flags a selection read during render", () => { + expect( + renderWithGuard().length, + ).toBeGreaterThan(0); + }); + + it("does not flag reads of a slice it subscribes to", () => { + expect(renderWithGuard()).toHaveLength(0); + }); + + it("does not flag selection reads from effects", () => { + expect(renderWithGuard()).toHaveLength(0); + }); +}); diff --git a/frontend/editor/src/core/contexts/file/fileHooks.ts b/frontend/editor/src/core/contexts/file/fileHooks.ts index fcd3ba7ef7..f5c1bfc460 100644 --- a/frontend/editor/src/core/contexts/file/fileHooks.ts +++ b/frontend/editor/src/core/contexts/file/fileHooks.ts @@ -1,27 +1,187 @@ /** - * Performant file hooks - Clean API using FileContext + * Performant file hooks — selector subscriptions over the FileStateStore. + * Each hook re-renders its consumer only when the slice it selects changes, + * not on every file-state change. */ -import { useContext, useMemo } from "react"; +import { useContext, useLayoutEffect, useMemo, useRef } from "react"; +import { useSyncExternalStoreWithSelector } from "use-sync-external-store/shim/with-selector.js"; import { - FileStateContext, + FileStoreContext, FileActionsContext, + FileStateStore, FileContextStateValue, FileContextActionsValue, } from "@app/contexts/file/contexts"; -import { StirlingFileStub, StirlingFile } from "@app/types/fileContext"; +import { + StirlingFileStub, + StirlingFile, + FileContextState, + FileContextSelectors, +} from "@app/types/fileContext"; import { FileId } from "@app/types/file"; +const GUARD_MISUSE = process.env.NODE_ENV !== "production"; + +/** Shallow equality over object/array slices assembled by selectors. */ +export function shallowEqual(a: unknown, b: unknown): boolean { + if (Object.is(a, b)) return true; + if ( + typeof a !== "object" || + a === null || + typeof b !== "object" || + b === null + ) { + return false; + } + const keysA = Object.keys(a); + if (keysA.length !== Object.keys(b).length) return false; + return keysA.every((key) => + Object.is( + (a as Record)[key], + (b as Record)[key], + ), + ); +} + +function useFileStore(): FileStateStore { + const store = useContext(FileStoreContext); + if (!store) { + throw new Error("File hooks must be used within a FileContextProvider"); + } + return store; +} + +/** + * Subscribe to a slice of file state. The component re-renders only when the + * selected value changes (Object.is by default; pass shallowEqual for slices + * assembled into fresh objects/arrays). + */ +export function useFileSelector( + selector: (state: FileContextState) => T, + isEqual?: (a: T, b: T) => boolean, +): T { + const store = useFileStore(); + return useSyncExternalStoreWithSelector( + store.subscribe, + store.getState, + store.getState, + selector, + isEqual, + ); +} + +/** Selectors that read `ui.selectedFileIds`. A hook that doesn't subscribe to + * that slice must not let consumers call these during render. */ +const SELECTION_SELECTORS: ReadonlyArray = [ + "getSelectedFiles", + "getSelectedStirlingFileStubs", +]; + +/** Wrap selectors so a call made during render logs loudly (dev/test only). + * Render-time vs event-time isn't statically lintable, so this is the guard. + * `keys` limits the wrap to the selectors whose slice the calling hook does NOT + * subscribe to — the rest are safe to read during render and pass through. */ +function guardSelectors( + selectors: FileContextSelectors, + isRendering: () => boolean, + hookName: string, + keys?: ReadonlyArray, +): FileContextSelectors { + const guardedKeys = + keys ?? (Object.keys(selectors) as Array); + const guarded: Record = { ...selectors }; + for (const key of guardedKeys) { + const original = selectors[key] as unknown as ( + ...args: unknown[] + ) => unknown; + guarded[key] = (...args: unknown[]) => { + if (isRendering()) { + console.error( + `[${hookName}] ${key}() was called during render. This read doesn't ` + + "subscribe to the state it depends on, so the UI can go stale — use " + + "useFileSelector / useFileSelection / useAllFiles for render-time data.", + ); + } + return original(...args); + }; + } + return guarded as unknown as FileContextSelectors; +} + +/** + * Wrap a hook's exposed selectors in the render-phase misuse guard (no-op in + * production). `keys` names the selectors the calling hook doesn't subscribe to; + * omit it to guard every selector (for hooks that subscribe to nothing). + */ +function useGuardedSelectors( + selectors: FileContextSelectors, + hookName: string, + keys?: ReadonlyArray, +): FileContextSelectors { + // True exactly while this consumer is rendering: set on every render, cleared + // by the layout effect once that render commits. + const renderPhase = useRef(false); + renderPhase.current = GUARD_MISUSE; + useLayoutEffect(() => { + renderPhase.current = false; + }); + return useMemo( + () => + GUARD_MISUSE + ? guardSelectors(selectors, () => renderPhase.current, hookName, keys) + : selectors, + [selectors, hookName, keys], + ); +} + +/** + * Stable selector API with NO state subscription — never re-renders. For + * event-time reads (callbacks/effects), which see live state when invoked. + * Render-time reads need a reactive hook (useAllFiles/useFileSelector) or + * they go stale — calling one during render logs an error outside production. + */ +export function useFileSelectors(): FileContextSelectors { + const { selectors } = useFileStore(); + return useGuardedSelectors(selectors, "useFileSelectors"); +} + +/** + * Position of `fileId` in the resolved file list — the SAME array useAllFiles() + * returns, which drops ids whose bytes haven't hydrated into memory yet, so the + * index lines up with what consumers actually index into. 0 when unset/absent. + * + * Selects a NUMBER, so the consumer re-renders only when the index actually + * moves. useAllFiles() would do the job too, but it re-renders on every + * unrelated stub update (thumbnail hydration, labels, …) — too costly for a + * high-level provider whose context value isn't memoized. + */ +export function useFileIndex(fileId: string | null | undefined): number { + // Raw (unguarded) selectors: the read below runs inside the subscription + // selector, so it IS reactive and the render-phase guard doesn't apply. + const { selectors } = useFileStore(); + return useFileSelector((s) => { + if (!fileId) return 0; + const index = selectors + .getFiles(s.files.ids) + .findIndex((file) => file.fileId === fileId); + return index >= 0 ? index : 0; + }); +} + /** * Hook for accessing file state (will re-render on any state change) * Use individual selector hooks below for better performance */ export function useFileState(): FileContextStateValue { - const context = useContext(FileStateContext); - if (!context) { - throw new Error("useFileState must be used within a FileContextProvider"); - } - return context; + const store = useFileStore(); + const state = useFileSelector((s) => s); + // Selectors are exposed unguarded on purpose: this hook subscribes to the + // WHOLE state, so a render-time selector read can't go stale. + return useMemo( + () => ({ state, selectors: store.selectors }), + [state, store.selectors], + ); } /** @@ -39,21 +199,21 @@ export function useFileActions(): FileContextActionsValue { * Hook for current/primary file (first in list) */ export function useCurrentFile(): { file?: File; record?: StirlingFileStub } { - const { state, selectors } = useFileState(); - - const primaryFileId = state.files.ids[0]; - const primaryFileRecord = primaryFileId - ? state.files.byId[primaryFileId] - : undefined; + const { selectors } = useFileStore(); + const { primaryFileId, record } = useFileSelector( + (s) => ({ + primaryFileId: s.files.ids[0], + record: s.files.ids[0] ? s.files.byId[s.files.ids[0]] : undefined, + }), + shallowEqual, + ); return useMemo( () => ({ file: primaryFileId ? selectors.getFile(primaryFileId) : undefined, - record: primaryFileId - ? selectors.getStirlingFileStub(primaryFileId) - : undefined, + record, }), - [primaryFileId, primaryFileRecord, selectors], + [primaryFileId, record, selectors], ); } @@ -61,27 +221,35 @@ export function useCurrentFile(): { file?: File; record?: StirlingFileStub } { * Hook for file selection state and actions */ export function useFileSelection() { - const { state, selectors } = useFileState(); + const { selectors } = useFileStore(); const { actions } = useFileActions(); + const selectedFileIds = useFileSelector((s) => s.ui.selectedFileIds); + const selectedPageNumbers = useFileSelector((s) => s.ui.selectedPageNumbers); + // Only the SELECTED files' records — an unrelated file's update never + // re-renders selection consumers. + const selectedStubs = useFileSelector( + (s) => s.ui.selectedFileIds.map((id) => s.files.byId[id]), + shallowEqual, + ); // Memoize selected files to avoid recreating arrays const selectedFiles = useMemo(() => { return selectors.getSelectedFiles(); - }, [state.ui.selectedFileIds, state.files.byId, selectors]); + }, [selectedFileIds, selectedStubs, selectors]); return useMemo( () => ({ selectedFiles, - selectedFileIds: state.ui.selectedFileIds, - selectedPageNumbers: state.ui.selectedPageNumbers, + selectedFileIds, + selectedPageNumbers, setSelectedFiles: actions.setSelectedFiles, setSelectedPages: actions.setSelectedPages, clearSelections: actions.clearSelections, }), [ selectedFiles, - state.ui.selectedFileIds, - state.ui.selectedPageNumbers, + selectedFileIds, + selectedPageNumbers, actions.setSelectedFiles, actions.setSelectedPages, actions.clearSelections, @@ -111,57 +279,64 @@ export function useFileManagement() { * Hook for UI state */ export function useFileUI() { - const { state } = useFileState(); const { actions } = useFileActions(); + const ui = useFileSelector( + (s) => ({ + isProcessing: s.ui.isProcessing, + processingProgress: s.ui.processingProgress, + hasUnsavedChanges: s.ui.hasUnsavedChanges, + }), + shallowEqual, + ); return useMemo( () => ({ - isProcessing: state.ui.isProcessing, - processingProgress: state.ui.processingProgress, - hasUnsavedChanges: state.ui.hasUnsavedChanges, + ...ui, setProcessing: actions.setProcessing, setUnsavedChanges: actions.setHasUnsavedChanges, }), - [state.ui, actions], + [ui, actions], ); } /** - * Hook for specific file by ID (optimized for individual file access) + * Hook for specific file by ID (optimized for individual file access): + * re-renders only when THAT file's record changes. */ export function useStirlingFileStub(fileId: FileId): { file?: File; record?: StirlingFileStub; } { - const { state, selectors } = useFileState(); - const fileRecord = state.files.byId[fileId]; + const { selectors } = useFileStore(); + const record = useFileSelector((s) => s.files.byId[fileId]); return useMemo( () => ({ file: selectors.getFile(fileId), - record: selectors.getStirlingFileStub(fileId), + record, }), - [fileId, fileRecord, selectors], + [fileId, record, selectors], ); } /** - * Hook for all files (use sparingly - causes re-renders on file list changes) + * Hook for all files: re-renders on file-list changes only (not selection/UI). */ export function useAllFiles(): { files: StirlingFile[]; fileStubs: StirlingFileStub[]; fileIds: FileId[]; } { - const { state, selectors } = useFileState(); + const { selectors } = useFileStore(); + const files = useFileSelector((s) => s.files); return useMemo( () => ({ - files: selectors.getFiles(), - fileStubs: selectors.getStirlingFileStubs(), - fileIds: state.files.ids, + files: selectors.getFiles(files.ids), + fileStubs: selectors.getStirlingFileStubs(files.ids), + fileIds: files.ids, }), - [state.files.ids, state.files.byId, selectors], + [files, selectors], ); } @@ -173,30 +348,47 @@ export function useSelectedFiles(): { selectedFileStubs: StirlingFileStub[]; selectedFileIds: FileId[]; } { - const { state, selectors } = useFileState(); + const { selectors } = useFileStore(); + const selectedFileIds = useFileSelector((s) => s.ui.selectedFileIds); + // Only the SELECTED files' records — see useFileSelection. + const selectedStubs = useFileSelector( + (s) => s.ui.selectedFileIds.map((id) => s.files.byId[id]), + shallowEqual, + ); return useMemo( () => ({ selectedFiles: selectors.getSelectedFiles(), selectedFileStubs: selectors.getSelectedStirlingFileStubs(), - selectedFileIds: state.ui.selectedFileIds, + selectedFileIds, }), - [state.ui.selectedFileIds, state.files.byId, selectors], + [selectedFileIds, selectedStubs, selectors], ); } -// Navigation management removed - moved to NavigationContext - /** - * Primary API hook for file context operations - * Used by tools for core file context functionality + * Primary API hook for file context operations. Used by tools for core file + * context functionality. Re-renders only when the slices it exposes reactively + * (files, pinned files) change — not on selection/UI changes. */ export function useFileContext() { - const { state, selectors } = useFileState(); + const store = useFileStore(); const { actions } = useFileActions(); + const { files, pinnedFiles } = useFileSelector( + (s) => ({ files: s.files, pinnedFiles: s.pinnedFiles }), + shallowEqual, + ); + // This hook subscribes to files + pinnedFiles, so those selectors are safe to + // read during render; the SELECTION ones aren't (no subscription to + // ui.selectedFileIds), so they carry the misuse guard. + const selectors = useGuardedSelectors( + store.selectors, + "useFileContext", + SELECTION_SELECTORS, + ); - return useMemo( - () => ({ + return useMemo(() => { + return { // Lifecycle management trackBlobUrl: actions.trackBlobUrl, scheduleCleanup: actions.scheduleCleanup, @@ -213,10 +405,11 @@ export function useFileContext() { _operationId: string, _error: string, ) => {}, // Operation tracking not implemented - // File ID lookup + // File ID lookup (reads live state at call time) findFileId: (file: File) => { - return state.files.ids.find((id) => { - const record = state.files.byId[id]; + const { files: liveFiles } = store.getState(); + return liveFiles.ids.find((id) => { + const record = liveFiles.byId[id]; return ( record && record.name === file.name && @@ -227,19 +420,18 @@ export function useFileContext() { }, // Pinned files - pinnedFiles: state.pinnedFiles, + pinnedFiles, pinFile: actions.pinFile, unpinFile: actions.unpinFile, isFilePinned: selectors.isFilePinned, // Active files - activeFiles: selectors.getFiles(), + activeFiles: selectors.getFiles(files.ids), openEncryptedUnlockPrompt: actions.openEncryptedUnlockPrompt, // Direct access to actions and selectors (for advanced use cases) actions, selectors, - }), - [state, selectors, actions], - ); + }; + }, [files, pinnedFiles, actions, store, selectors]); } diff --git a/frontend/editor/src/core/contexts/file/reducerIdentityGuard.test.ts b/frontend/editor/src/core/contexts/file/reducerIdentityGuard.test.ts new file mode 100644 index 0000000000..c250c3e4dc --- /dev/null +++ b/frontend/editor/src/core/contexts/file/reducerIdentityGuard.test.ts @@ -0,0 +1,78 @@ +import { describe, it, expect, vi, afterEach } from "vitest"; +import { withReducerIdentityGuard } from "@app/contexts/file/FileReducer"; +import type { + FileContextState, + FileContextAction, + StirlingFileStub, +} from "@app/types/fileContext"; +import type { FileId } from "@app/types/file"; + +const stub = (id: string): StirlingFileStub => + ({ id: id as FileId, name: `${id}.pdf` }) as StirlingFileStub; + +function baseState(): FileContextState { + return { + files: { ids: ["a" as FileId], byId: { ["a" as FileId]: stub("a") } }, + pinnedFiles: new Set(), + ui: { + selectedFileIds: [], + selectedPageNumbers: [], + isProcessing: false, + processingProgress: 0, + hasUnsavedChanges: false, + errorFileIds: [], + }, + }; +} + +const guardErrors = (spy: ReturnType) => + spy.mock.calls.filter((a) => String(a[0]).includes("[FileReducer]")); + +afterEach(() => vi.restoreAllMocks()); + +describe("withReducerIdentityGuard", () => { + it("warns when a slice is reallocated but unchanged", () => { + const spy = vi.spyOn(console, "error").mockImplementation(() => {}); + // Bad reducer: rebuilds `files` (new ref) with identical contents. + const guarded = withReducerIdentityGuard((s) => ({ + ...s, + files: { ids: [...s.files.ids], byId: { ...s.files.byId } }, + })); + guarded(baseState(), { type: "NOOP" } as unknown as FileContextAction); + expect(guardErrors(spy)).toHaveLength(1); + expect(String(guardErrors(spy)[0][0])).toContain("state.files"); + }); + + it("stays quiet when a slice genuinely changes", () => { + const spy = vi.spyOn(console, "error").mockImplementation(() => {}); + const guarded = withReducerIdentityGuard((s) => ({ + ...s, + files: { + ids: [...s.files.ids, "b" as FileId], + byId: { ...s.files.byId, ["b" as FileId]: stub("b") }, + }, + })); + guarded(baseState(), { type: "NOOP" } as unknown as FileContextAction); + expect(guardErrors(spy)).toHaveLength(0); + }); + + it("stays quiet when the reducer returns the same state reference", () => { + const spy = vi.spyOn(console, "error").mockImplementation(() => {}); + const guarded = withReducerIdentityGuard((s) => s); + const state = baseState(); + expect( + guarded(state, { type: "NOOP" } as unknown as FileContextAction), + ).toBe(state); + expect(guardErrors(spy)).toHaveLength(0); + }); + + it("flags a needless ui reallocation", () => { + const spy = vi.spyOn(console, "error").mockImplementation(() => {}); + const guarded = withReducerIdentityGuard((s) => ({ + ...s, + ui: { ...s.ui }, + })); + guarded(baseState(), { type: "NOOP" } as unknown as FileContextAction); + expect(String(guardErrors(spy)[0][0])).toContain("state.ui"); + }); +}); diff --git a/frontend/editor/src/core/contexts/file/useFileIndex.test.tsx b/frontend/editor/src/core/contexts/file/useFileIndex.test.tsx new file mode 100644 index 0000000000..e10bfed517 --- /dev/null +++ b/frontend/editor/src/core/contexts/file/useFileIndex.test.tsx @@ -0,0 +1,128 @@ +import { describe, it, expect } from "vitest"; +import { render, act } from "@testing-library/react"; +import { + FileStoreContext, + type FileStateStore, +} from "@app/contexts/file/contexts"; +import { useFileIndex } from "@app/contexts/file/fileHooks"; +import type { + FileContextSelectors, + FileContextState, +} from "@app/types/fileContext"; +import type { FileId } from "@app/types/file"; + +/** + * useFileIndex replaced a render-time `selectors.getFiles()` read in + * ViewerContext, which never re-subscribed and so survived a file-list change + * that moved the active file. These tests drive a hand-built store (the real one + * needs IndexedDB to populate its File map) and lock the two properties the fix + * depends on: the index tracks the RESOLVED file list, and the consumer + * re-renders only when the index actually moves. + */ + +function makeStore(ids: string[], resolved: string[]) { + let state: FileContextState = { + files: { ids: ids as FileId[], byId: {} }, + } as FileContextState; + let resolvedIds = new Set(resolved); + const listeners = new Set<() => void>(); + + // Mirrors createFileSelectors.getFiles: maps ids through the in-memory File + // map and DROPS the ones whose bytes haven't landed yet. + const selectors = { + getFiles: (requested?: FileId[]) => + (requested ?? state.files.ids) + .filter((id) => resolvedIds.has(id as string)) + .map((id) => ({ fileId: id })), + } as unknown as FileContextSelectors; + + const store: FileStateStore = { + getState: () => state, + subscribe: (listener) => { + listeners.add(listener); + return () => listeners.delete(listener); + }, + selectors, + }; + + const update = (nextIds: string[], nextResolved: string[] = nextIds) => { + act(() => { + state = { + files: { ids: nextIds as FileId[], byId: {} }, + } as FileContextState; + resolvedIds = new Set(nextResolved); + listeners.forEach((listener) => listener()); + }); + }; + + return { store, update }; +} + +function setup(ids: string[], resolved: string[], fileId: string | null) { + const { store, update } = makeStore(ids, resolved); + let renders = 0; + let index = -1; + + function Probe() { + index = useFileIndex(fileId); + renders++; + return null; + } + + render( + + + , + ); + + return { update, get: () => index, renderCount: () => renders }; +} + +describe("useFileIndex", () => { + it("reports the active file's position in the resolved list", () => { + const { get } = setup(["a", "b", "c"], ["a", "b", "c"], "c"); + expect(get()).toBe(2); + }); + + it("skips ids whose bytes haven't hydrated, matching what consumers index into", () => { + // "a" has no File yet, so getFiles() yields [b, c] — "c" sits at 1, not 2. + const { get } = setup(["a", "b", "c"], ["b", "c"], "c"); + expect(get()).toBe(1); + }); + + it("updates when the list reorders under a stable active file", () => { + // The regression this fixes: activeFileId never changed, so the old + // useMemo kept returning the pre-reorder index. + const { get, update } = setup(["a", "b", "c"], ["a", "b", "c"], "c"); + expect(get()).toBe(2); + update(["c", "a", "b"]); + expect(get()).toBe(0); + }); + + it("updates when a file ahead of the active one is removed", () => { + const { get, update } = setup(["a", "b", "c"], ["a", "b", "c"], "c"); + update(["b", "c"]); + expect(get()).toBe(1); + }); + + it("falls back to 0 when the active file leaves the list", () => { + const { get, update } = setup(["a", "b"], ["a", "b"], "b"); + update(["a"]); + expect(get()).toBe(0); + }); + + it("returns 0 with no active file", () => { + const { get } = setup(["a", "b"], ["a", "b"], null); + expect(get()).toBe(0); + }); + + it("does not re-render when a store change leaves the index alone", () => { + // Selecting a NUMBER is the point: appending after the active file, or any + // unrelated stub churn, must not re-render the consumer. + const { get, update, renderCount } = setup(["a", "b"], ["a", "b"], "a"); + const before = renderCount(); + update(["a", "b", "c"]); + expect(get()).toBe(0); + expect(renderCount()).toBe(before); + }); +}); diff --git a/frontend/editor/src/core/tools/Convert.tsx b/frontend/editor/src/core/tools/Convert.tsx index de29e4ae94..b1d214f1a6 100644 --- a/frontend/editor/src/core/tools/Convert.tsx +++ b/frontend/editor/src/core/tools/Convert.tsx @@ -1,7 +1,7 @@ import { useEffect, useRef } from "react"; import { useTranslation } from "react-i18next"; import { useEndpointEnabled } from "@app/hooks/useEndpointConfig"; -import { useFileState } from "@app/contexts/FileContext"; +import { useAllFiles } from "@app/contexts/FileContext"; import { useViewScopedFiles } from "@app/hooks/tools/shared/useViewScopedFiles"; import { createToolFlow } from "@app/components/tools/shared/createToolFlow"; @@ -14,8 +14,7 @@ import { BaseToolProps, ToolComponent } from "@app/types/tool"; const Convert = ({ onPreviewFile, onComplete, onError }: BaseToolProps) => { const { t } = useTranslation(); - const { selectors } = useFileState(); - const activeFiles = selectors.getFiles(); + const { files: activeFiles } = useAllFiles(); const selectedFiles = useViewScopedFiles(); const scrollContainerRef = useRef(null); diff --git a/frontend/editor/src/desktop/hooks/useExitWarning.ts b/frontend/editor/src/desktop/hooks/useExitWarning.ts index 1e6b4423fc..4a5b16fb8f 100644 --- a/frontend/editor/src/desktop/hooks/useExitWarning.ts +++ b/frontend/editor/src/desktop/hooks/useExitWarning.ts @@ -1,14 +1,14 @@ import { useEffect, useRef } from "react"; import { getCurrentWindow } from "@tauri-apps/api/window"; import { message } from "@tauri-apps/plugin-dialog"; -import { useFileState, useFileActions } from "@app/contexts/FileContext"; +import { useFileSelectors, useFileActions } from "@app/contexts/FileContext"; import { downloadFile } from "@app/services/downloadService"; import type { StirlingFileStub } from "@app/types/fileContext"; import { useTranslation } from "react-i18next"; export function useExitWarning() { const { t } = useTranslation(); - const { selectors } = useFileState(); + const selectors = useFileSelectors(); const { actions: fileActions } = useFileActions(); const selectorsRef = useRef(selectors); const isClosingRef = useRef(false); diff --git a/frontend/editor/src/desktop/hooks/useSaveShortcut.ts b/frontend/editor/src/desktop/hooks/useSaveShortcut.ts index 927d5f06cf..b3faa64bb7 100644 --- a/frontend/editor/src/desktop/hooks/useSaveShortcut.ts +++ b/frontend/editor/src/desktop/hooks/useSaveShortcut.ts @@ -1,5 +1,9 @@ import { useEffect } from "react"; -import { useFileState, useFileActions } from "@app/contexts/FileContext"; +import { + useFileSelector, + useFileSelectors, + useFileActions, +} from "@app/contexts/FileContext"; // Save through the export gateway so a "run on export" policy enforces before // the file is written out (no-op when no such policy is active). import { downloadFileWithPolicy as downloadFile } from "@app/services/exportWithPolicy"; @@ -10,7 +14,8 @@ import { downloadFileWithPolicy as downloadFile } from "@app/services/exportWith * Matches WorkbenchBar button behavior: saves selected files if any, otherwise all files */ export function useSaveShortcut() { - const { selectors, state } = useFileState(); + const selectors = useFileSelectors(); + const currentSelectedFileIds = useFileSelector((s) => s.ui.selectedFileIds); const { actions: fileActions } = useFileActions(); useEffect(() => { @@ -20,7 +25,7 @@ export function useSaveShortcut() { event.preventDefault(); // Get selected files or all files if nothing selected - const selectedFileIds = state.ui.selectedFileIds; + const selectedFileIds = currentSelectedFileIds; const filesToSave = selectedFileIds.length > 0 ? selectors.getFiles(selectedFileIds) @@ -63,5 +68,5 @@ export function useSaveShortcut() { document.addEventListener("keydown", handleKeyDown); return () => document.removeEventListener("keydown", handleKeyDown); - }, [selectors, state.ui.selectedFileIds, fileActions]); + }, [selectors, currentSelectedFileIds, fileActions]); } diff --git a/frontend/editor/src/proprietary/components/policies/classificationLabelTargets.test.ts b/frontend/editor/src/proprietary/components/policies/classificationLabelTargets.test.ts new file mode 100644 index 0000000000..74baf59006 --- /dev/null +++ b/frontend/editor/src/proprietary/components/policies/classificationLabelTargets.test.ts @@ -0,0 +1,44 @@ +import { describe, it, expect } from "vitest"; +import { classificationLabelTargetStubs } from "@app/components/policies/usePolicyAutoRun"; +import type { StirlingFileStub } from "@app/types/fileContext"; + +// Loosely-typed builder: FileId is a branded string, so accept plain string ids +// in tests and cast — classificationLabelTargetStubs only reads id/parent/sources. +const stub = (s: { + id: string; + parentFileId?: string; + sourceFileIds?: string[]; +}): StirlingFileStub => s as unknown as StirlingFileStub; + +const ids = (stubs: StirlingFileStub[]) => stubs.map((s) => s.id as string); + +describe("classificationLabelTargetStubs", () => { + it("targets the run's own file when it's still the leaf", () => { + const stubs = [stub({ id: "a" }), stub({ id: "b" })]; + expect(ids(classificationLabelTargetStubs("a", stubs))).toEqual(["a"]); + }); + + it("targets a descendant leaf when the file was edited during the run", () => { + // "a" was consumed into leaf "a2" (edit forked a new version mid-run). + const stubs = [stub({ id: "a2", sourceFileIds: ["a"] })]; + expect(ids(classificationLabelTargetStubs("a", stubs))).toEqual(["a2"]); + }); + + it("targets a direct child via parentFileId", () => { + const stubs = [stub({ id: "a2", parentFileId: "a" })]; + expect(ids(classificationLabelTargetStubs("a", stubs))).toEqual(["a2"]); + }); + + it("returns the stubs themselves, so the caller can see what's already tagged", () => { + const target = stub({ id: "a" }); + expect(classificationLabelTargetStubs("a", [target])[0]).toBe(target); + }); + + it("is empty when the document has left the workspace (file closed)", () => { + // No fallback to the run's own id: stamping a consumed id would no-op + // anyway, and an empty result lets the caller settle without downloading. + expect(classificationLabelTargetStubs("a", [stub({ id: "z" })])).toEqual( + [], + ); + }); +}); diff --git a/frontend/editor/src/proprietary/components/policies/dispatchSemaphore.test.ts b/frontend/editor/src/proprietary/components/policies/dispatchSemaphore.test.ts new file mode 100644 index 0000000000..355d8b5526 --- /dev/null +++ b/frontend/editor/src/proprietary/components/policies/dispatchSemaphore.test.ts @@ -0,0 +1,47 @@ +import { describe, it, expect, beforeEach } from "vitest"; +import { + acquireDispatchSlot, + releaseDispatchSlot, + resetDispatchSemaphoreForTests, +} from "@app/components/policies/dispatchSemaphore"; + +// Drain the microtask queue so an acquire's await-resume AND the caller's .then +// have both run. +const flush = () => new Promise((r) => setTimeout(r, 0)); + +beforeEach(() => resetDispatchSemaphoreForTests()); + +describe("dispatchSemaphore", () => { + it("lets up to 4 acquire without waiting, then blocks the 5th", async () => { + for (let i = 0; i < 4; i++) await acquireDispatchSlot(); + let fifthAcquired = false; + void acquireDispatchSlot().then(() => { + fifthAcquired = true; + }); + await flush(); + expect(fifthAcquired).toBe(false); + releaseDispatchSlot(); + await flush(); + expect(fifthAcquired).toBe(true); + }); + + it("serves a priority (chained) waiter before earlier normal waiters", async () => { + for (let i = 0; i < 4; i++) await acquireDispatchSlot(); // pool full + const order: string[] = []; + // Two normal (new-file) dispatches queue first… + void acquireDispatchSlot(false).then(() => order.push("normal-1")); + void acquireDispatchSlot(false).then(() => order.push("normal-2")); + // …then a chained dispatch arrives — it must jump ahead. + void acquireDispatchSlot(true).then(() => order.push("chained")); + await flush(); + + releaseDispatchSlot(); + await flush(); + releaseDispatchSlot(); + await flush(); + releaseDispatchSlot(); + await flush(); + + expect(order).toEqual(["chained", "normal-1", "normal-2"]); + }); +}); diff --git a/frontend/editor/src/proprietary/components/policies/dispatchSemaphore.ts b/frontend/editor/src/proprietary/components/policies/dispatchSemaphore.ts new file mode 100644 index 0000000000..d5119e5923 --- /dev/null +++ b/frontend/editor/src/proprietary/components/policies/dispatchSemaphore.ts @@ -0,0 +1,42 @@ +/** + * Bounded concurrency for policy run-dispatch uploads. + * + * Each dispatch POSTs a file's bytes; firing a whole drop at once saturates the + * browser's per-origin connection pool, so status polls and output downloads of + * already-running files queue behind the pending uploads and nothing visibly + * progresses. A small window keeps connections free. + * + * `priority` (a chained/downstream dispatch) jumps to the FRONT of the queue, so + * a file already mid-chain finishes its whole policy flow before a brand-new + * file's first policy starts. Without it a chained dispatch would sit behind the + * entire first-policy wave (FIFO) — e.g. classification wouldn't start on any + * file until security had finished on all of them. + */ +const MAX_CONCURRENT_DISPATCHES = 4; + +let slotsInUse = 0; +const waiters: Array<() => void> = []; + +export async function acquireDispatchSlot(priority = false): Promise { + if (slotsInUse < MAX_CONCURRENT_DISPATCHES) { + slotsInUse++; + return; + } + await new Promise((resolve) => { + if (priority) waiters.unshift(resolve); + else waiters.push(resolve); + }); +} + +export function releaseDispatchSlot(): void { + const next = waiters.shift(); + // Hand the slot straight to the next waiter, else free it. + if (next) next(); + else slotsInUse--; +} + +/** Test-only: reset module state between cases. */ +export function resetDispatchSemaphoreForTests(): void { + slotsInUse = 0; + waiters.length = 0; +} diff --git a/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.batch.test.tsx b/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.batch.test.tsx index 5837bfb285..bc4fa54dff 100644 --- a/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.batch.test.tsx +++ b/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.batch.test.tsx @@ -2,21 +2,8 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { renderHook, act } from "@testing-library/react"; /** - * Batch integration test for the policy auto-run orchestration, at the scale the - * user hit the bug: 61 files uploaded at once, two active upload policies - * (Classification → Security) chained. Drives the REAL policyRunStore + the REAL - * hook effects (dispatch → poll → import → chain), mocking only the IO boundaries - * (network, storage, thumbnail/stub creation). - * - * Proves the invariants the user asked for: - * - 61 files ⇒ exactly 122 runs (61 classification, then 61 security). - * - Delivery is SILENT + in place (consumeFiles called with { silent: true }), - * never adding a second copy — the workspace never grows past 61. - * - No runaway: if the loop guard regressed, the run count would blow past 122 - * (or the test would time out), so an exact 122 is a hard regression gate. - * - Closing all files mid-run does NOT re-open them: with the workspace emptied, - * outputs are delivered to storage (persistVersionedOutputs), never re-added - * to the workspace via consumeFiles. + * Batch integration test (61 files, two chained upload policies) driving the real + * store + hook effects, IO mocked. Classification is forced last (see the sort). */ const FILE_COUNT = 61; @@ -25,13 +12,15 @@ const FILE_COUNT = 61; // the workbench, mirrored into useAllFiles. consumeFiles mutates it in place // (input id → output id) exactly as the real silent reducer would. const mocks = vi.hoisted(() => ({ - workspace: [] as Array<{ id: string }>, + workspace: [] as Array<{ id: string; classificationLabels?: string[] }>, consumeSilentCalls: 0, consumeNonSilentCalls: 0, persistCalls: 0, addFilesCalls: 0, stubCounter: 0, backendOutCounter: 0, + dispatchInFlight: 0, + maxDispatchInFlight: 0, bumpRevision: vi.fn(), runStoredPolicy: vi.fn(), getPolicyRun: vi.fn(), @@ -66,7 +55,8 @@ vi.mock("@app/contexts/IndexedDBContext", () => ({ vi.mock("@app/hooks/usePolicies", () => ({ usePolicies: () => ({ policies: { - // Classification runs first (order 0), Security second (order 1). + // Classification is configured first (order 0) but is FORCED to run last + // by the orchestrator; Security (order 1) therefore runs first. classification: { configured: true, status: "active", @@ -107,7 +97,9 @@ vi.mock("@app/services/fileStubHelpers", () => ({ createStirlingFilesAndStubs: mocks.createStirlingFilesAndStubs, })); vi.mock("@app/services/fileClassification", () => ({ - readClassificationLabelsFromFile: vi.fn().mockResolvedValue(null), + // Classification always resolves labels here, so the metadata-only import path + // stamps them onto the stub. + readClassificationLabelsFromFile: vi.fn().mockResolvedValue(["Invoice"]), })); import { usePolicyAutoRun } from "@app/components/policies/usePolicyAutoRun"; @@ -141,6 +133,8 @@ beforeEach(() => { mocks.addFilesCalls = 0; mocks.stubCounter = 0; mocks.backendOutCounter = 0; + mocks.dispatchInFlight = 0; + mocks.maxDispatchInFlight = 0; mocks.workspace = Array.from({ length: FILE_COUNT }, (_, i) => ({ id: `file-${i}`, @@ -155,15 +149,31 @@ beforeEach(() => { mocks.persistVersionedOutputs.mockImplementation(async () => { mocks.persistCalls += 1; }); - mocks.updateFileMetadata.mockResolvedValue(false); + mocks.updateFileMetadata.mockResolvedValue(true); mocks.downloadPolicyOutput.mockResolvedValue( new Blob(["x"], { type: "application/pdf" }), ); + // Apply stub updates to the shared workspace, as the real reducer does — the + // label stamp's second pass reads them back to stay idempotent. + mocks.updateStirlingFileStub.mockImplementation( + (id: string, updates: Record) => { + const stub = mocks.workspace.find((s) => s.id === id); + if (stub) Object.assign(stub, updates); + }, + ); // Each dispatch gets a unique run id; the run's single backend output likewise. - mocks.runStoredPolicy.mockImplementation( - async () => `run-${mocks.stubCounter++}`, - ); + // Takes real time so overlapping dispatches are measurable (the upload window). + mocks.runStoredPolicy.mockImplementation(async () => { + mocks.dispatchInFlight++; + mocks.maxDispatchInFlight = Math.max( + mocks.maxDispatchInFlight, + mocks.dispatchInFlight, + ); + await new Promise((resolve) => setTimeout(resolve, 2)); + mocks.dispatchInFlight--; + return `run-${mocks.stubCounter++}`; + }); mocks.getPolicyRun.mockImplementation(async (runId: string) => ({ runId, policyId: null, @@ -225,8 +235,8 @@ async function runUntilSettled(expectedRuns: number) { }); } -describe("policy auto-run — 61-file batch through a Classification → Security chain", () => { - it("produces exactly 122 runs (61 classification, then 61 security)", async () => { +describe("policy auto-run — 61-file batch through a Security → Classification chain", () => { + it("produces exactly 122 runs (61 security, then 61 classification)", async () => { await runUntilSettled(FILE_COUNT * 2); const classification = latestRuns.filter( @@ -239,15 +249,26 @@ describe("policy auto-run — 61-file batch through a Classification → Securit expect(latestRuns).toHaveLength(FILE_COUNT * 2); }); - it("delivers every output SILENTLY in place — workspace never grows past 61", async () => { + it("bounds concurrent dispatch uploads so polls/downloads keep connections", async () => { + await runUntilSettled(FILE_COUNT * 2); + expect(mocks.maxDispatchInFlight).toBeGreaterThan(1); // still parallel… + expect(mocks.maxDispatchInFlight).toBeLessThanOrEqual(4); // …but windowed + }); + + it("versions on Security in place, tags on Classification — workspace never grows past 61", async () => { await runUntilSettled(FILE_COUNT * 2); - // 122 deliveries, all silent (background), none via the disruptive path. - expect(mocks.consumeSilentCalls).toBe(FILE_COUNT * 2); + // Only the 61 Security runs fork a version, and every one silently in place. + expect(mocks.consumeSilentCalls).toBe(FILE_COUNT); expect(mocks.consumeNonSilentCalls).toBe(0); + // Classification never forks a version — it only stamps labels onto the stub. + expect(mocks.updateStirlingFileStub).toHaveBeenCalledTimes(FILE_COUNT); + for (const call of mocks.updateStirlingFileStub.mock.calls) { + expect(call[1]).toEqual({ classificationLabels: ["Invoice"] }); + } // Never added as brand-new files either. expect(mocks.addFilesCalls).toBe(0); - // In-place versioning: each file replaced twice, count unchanged. + // In-place versioning + metadata-only tagging: count unchanged. expect(mocks.workspace).toHaveLength(FILE_COUNT); }); @@ -273,8 +294,8 @@ describe("policy auto-run — 61-file batch through a Classification → Securit ); }); - // Still fully processed (chain intact), but delivered to STORAGE, never - // re-added to the workbench — the workspace stays empty. + // Still fully processed (chain intact), but Security's versions went to + // STORAGE, never re-added to the workbench — the workspace stays empty. expect(latestRuns).toHaveLength(FILE_COUNT * 2); expect(mocks.workspace).toHaveLength(0); expect(mocks.consumeSilentCalls).toBe(0); diff --git a/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.race.test.tsx b/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.race.test.tsx new file mode 100644 index 0000000000..b67648e7fc --- /dev/null +++ b/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.race.test.tsx @@ -0,0 +1,293 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { renderHook, act } from "@testing-library/react"; + +/** + * Mid-run race: classification is in flight (its labelled output is still + * downloading) when the user manually runs a tool on the same file — e.g. + * quickly redacting it — which consumes the input and forks a new leaf. + * + * The label targets must be resolved AT WRITE TIME (after the download/parse + * window), not snapshotted at run completion: a stale snapshot points at the + * consumed id, no-ops, and silently loses the labels — the file then shows the + * classification badge (provenance-resolved) but never gets its labels. + */ + +const mocks = vi.hoisted(() => ({ + workspace: [] as Array<{ + id: string; + sourceFileIds?: string[]; + derivedFromTool?: boolean; + classificationLabels?: string[]; + }>, + runStoredPolicy: vi.fn(), + getPolicyRun: vi.fn(), + listPolicyRuns: vi.fn(), + downloadPolicyOutput: vi.fn(), + getStirlingFile: vi.fn(), + getStirlingFileStub: vi.fn(), + persistVersionedOutputs: vi.fn(), + updateFileMetadata: vi.fn(), + createStirlingFilesAndStubs: vi.fn(), + addFiles: vi.fn(), + updateStirlingFileStub: vi.fn(), + consumeFiles: vi.fn(), + bumpRevision: vi.fn(), +})); + +// Classification chains server-side only when the AI engine is on (else it runs +// client-side); this race is in the server import path, so force the engine on. +vi.mock("@app/hooks/useAiEngineEnabled", () => ({ + useAiEngineEnabled: () => true, +})); +vi.mock("@app/contexts/FileContext", () => ({ + useAllFiles: () => ({ fileStubs: mocks.workspace }), + useFileManagement: () => ({ + addFiles: mocks.addFiles, + updateStirlingFileStub: mocks.updateStirlingFileStub, + }), + useFileContext: () => ({ consumeFiles: mocks.consumeFiles }), +})); +vi.mock("@app/contexts/IndexedDBContext", () => ({ + useIndexedDB: () => ({ bumpRevision: mocks.bumpRevision }), +})); +vi.mock("@app/hooks/usePolicies", () => ({ + usePolicies: () => ({ + policies: { + classification: { + configured: true, + status: "active", + backendId: "backend-classification", + runOn: "upload", + order: 0, + outputMode: "new_version", + outputName: "", + }, + }, + }), +})); +vi.mock("@app/services/policyApi", () => ({ + runStoredPolicy: mocks.runStoredPolicy, + getPolicyRun: mocks.getPolicyRun, + listPolicyRuns: mocks.listPolicyRuns, + downloadPolicyOutput: mocks.downloadPolicyOutput, + resolvePolicyRunTarget: () => "saas", +})); +vi.mock("@app/services/fileStorage", () => ({ + fileStorage: { + getStirlingFile: mocks.getStirlingFile, + getStirlingFileStub: mocks.getStirlingFileStub, + persistVersionedOutputs: mocks.persistVersionedOutputs, + updateFileMetadata: mocks.updateFileMetadata, + }, +})); +vi.mock("@app/services/fileStubHelpers", () => ({ + createStirlingFilesAndStubs: mocks.createStirlingFilesAndStubs, +})); +vi.mock("@app/services/fileClassification", () => ({ + readClassificationLabelsFromFile: vi.fn().mockResolvedValue(["Invoice"]), +})); + +import { usePolicyAutoRun } from "@app/components/policies/usePolicyAutoRun"; +import { + usePolicyRuns, + resetPolicyRuns, +} from "@app/components/policies/policyRunStore"; +import type { PolicyRunRecord } from "@app/components/policies/policyRunStore"; + +let latestRuns: PolicyRunRecord[] = []; +function Harness() { + usePolicyAutoRun(); + latestRuns = usePolicyRuns(); + return null; +} + +function deferred() { + let resolve!: (value: T) => void; + const promise = new Promise((r) => { + resolve = r; + }); + return { promise, resolve }; +} + +beforeEach(() => { + localStorage.clear(); + resetPolicyRuns(); + vi.clearAllMocks(); + + mocks.workspace = [{ id: "file-0" }]; + + mocks.listPolicyRuns.mockResolvedValue([]); + mocks.getStirlingFile.mockResolvedValue( + new File(["x"], "doc.pdf", { type: "application/pdf" }), + ); + mocks.getStirlingFileStub.mockResolvedValue(null); + mocks.updateFileMetadata.mockResolvedValue(true); + // Apply stub updates to the shared workspace, as the real reducer does — the + // label stamp's second pass reads them back to stay idempotent. + mocks.updateStirlingFileStub.mockImplementation( + (id: string, updates: Record) => { + const stub = mocks.workspace.find((s) => s.id === id); + if (stub) Object.assign(stub, updates); + }, + ); + mocks.runStoredPolicy.mockResolvedValue("run-0"); + mocks.getPolicyRun.mockResolvedValue({ + runId: "run-0", + policyId: null, + status: "COMPLETED", + currentStep: 1, + stepCount: 1, + error: null, + outputs: [{ fileId: "backend-out-0", fileName: "doc.pdf" }], + }); +}); + +async function settleImport(timeout = 8000) { + await act(async () => { + await vi.waitFor( + () => { + expect(latestRuns.filter((r) => r.imported)).toHaveLength(1); + }, + { timeout, interval: 20 }, + ); + }); +} + +describe("classification vs a mid-run manual tool edit", () => { + it("labels land on the forked leaf when a tool consumes the file during the label download", async () => { + // The classified output's download hangs until we release it — this is the + // async window the user's edit slips into. + const download = deferred(); + mocks.downloadPolicyOutput.mockReturnValue(download.promise); + + const { rerender } = renderHook(() => Harness()); + + // Run dispatched, completed, import started — now hanging in the window. + await act(async () => { + await vi.waitFor( + () => expect(mocks.downloadPolicyOutput).toHaveBeenCalled(), + { timeout: 8000, interval: 20 }, + ); + }); + + // User quickly redacts: the tool consumes file-0 and forks a new leaf. + // (derivedFromTool + sourceFileIds are what CONSUME_FILES stamps.) + act(() => { + mocks.workspace = [ + { + id: "file-0~redacted", + sourceFileIds: ["file-0"], + derivedFromTool: true, + }, + ]; + rerender(); + }); + + // The download finally lands. + download.resolve(new Blob(["x"], { type: "application/pdf" })); + await settleImport(); + + // Labels stamped onto the LIVE leaf, not no-oped on the consumed id. + const stampedIds = mocks.updateStirlingFileStub.mock.calls.map((c) => c[0]); + expect(stampedIds).toEqual(["file-0~redacted"]); + expect(mocks.updateStirlingFileStub).toHaveBeenCalledWith( + "file-0~redacted", + { classificationLabels: ["Invoice"] }, + ); + // Badge persists on the leaf: the run's outputFileIds are the tagged files. + expect(latestRuns[0].outputFileIds).toEqual(["file-0~redacted"]); + }); + + it("control: with no mid-run edit, labels land on the original file", async () => { + mocks.downloadPolicyOutput.mockResolvedValue( + new Blob(["x"], { type: "application/pdf" }), + ); + + renderHook(() => Harness()); + await settleImport(); + + expect(mocks.updateStirlingFileStub).toHaveBeenCalledWith("file-0", { + classificationLabels: ["Invoice"], + }); + expect(latestRuns[0].outputFileIds).toEqual(["file-0"]); + }); + + it("stamps the forked leaf when the consume lands in the same frame as the first stamp", async () => { + // Tighter than the case above: the consume is dispatched but hasn't rendered + // when the labels are stamped, so the workspace snapshot still shows file-0 + // and that stamp no-ops against the real reducer. The post-commit second pass + // is what saves the labels. + mocks.downloadPolicyOutput.mockResolvedValue( + new Blob(["x"], { type: "application/pdf" }), + ); + mocks.updateStirlingFileStub.mockImplementation((id: string) => { + // file-0 is already consumed, so its stamp is lost (no Object.assign) and + // the forked leaf only becomes visible afterwards. Mutate the workspace in + // place: the hook holds it by ref, which is what the second pass re-reads. + if (id === "file-0") { + mocks.workspace.splice(0, mocks.workspace.length, { + id: "file-0~redacted", + sourceFileIds: ["file-0"], + }); + return; + } + const stub = mocks.workspace.find((s) => s.id === id); + if (stub) Object.assign(stub, { classificationLabels: ["Invoice"] }); + }); + + renderHook(() => Harness()); + await settleImport(); + + const stampedIds = mocks.updateStirlingFileStub.mock.calls.map((c) => c[0]); + expect(stampedIds).toEqual(["file-0", "file-0~redacted"]); + // The leaf becoming visible also queues its own classification run, so pick + // the settled one rather than assuming an index. + const imported = latestRuns.find((r) => r.imported); + expect(imported?.outputFileIds).toContain("file-0~redacted"); + }); +}); + +// The label read backs off between attempts (2s, then 4s), so these run on fake +// timers — sleeping for real would hold a worker long enough to starve the suite. +describe("classification label-read failures", () => { + beforeEach(() => vi.useFakeTimers()); + afterEach(() => vi.useRealTimers()); + + async function settleOnFakeTime(maxMs = 30_000) { + for (let elapsed = 0; elapsed < maxMs; elapsed += 250) { + if (latestRuns.some((r) => r.imported)) return; + await act(async () => { + await vi.advanceTimersByTimeAsync(250); + }); + } + throw new Error("classification run never settled"); + } + + it("retries a transient failure instead of leaving the run unsettled", async () => { + // The import effect only re-runs when the run store changes, so bailing out + // on a transient failure would leave this run "running" forever. + mocks.downloadPolicyOutput + .mockRejectedValueOnce(new Error("network blip")) + .mockResolvedValue(new Blob(["x"], { type: "application/pdf" })); + + renderHook(() => Harness()); + await settleOnFakeTime(); + + expect(mocks.downloadPolicyOutput).toHaveBeenCalledTimes(2); + expect(mocks.updateStirlingFileStub).toHaveBeenCalledWith("file-0", { + classificationLabels: ["Invoice"], + }); + }); + + it("settles a run whose labels never become readable", async () => { + // Permanent failure: give up after the retry budget and settle unlabelled, + // rather than spinning the file's "running" pill indefinitely. + mocks.downloadPolicyOutput.mockRejectedValue(new Error("network down")); + + renderHook(() => Harness()); + await settleOnFakeTime(); + + expect(mocks.updateStirlingFileStub).not.toHaveBeenCalled(); + expect(latestRuns.find((r) => r.imported)?.outputFileIds).toEqual([]); + }); +}); diff --git a/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.retry.test.tsx b/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.retry.test.tsx index c25f80ce65..3c54e5cc42 100644 --- a/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.retry.test.tsx +++ b/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.retry.test.tsx @@ -69,8 +69,17 @@ afterEach(() => vi.useRealTimers()); describe("auto-run queue-rejection retry", () => { it("relabels a queue-rejected run as retrying, then re-dispatches it in place", async () => { - // The polled run comes back queue-rejected; the retry resolves the file + fires a fresh run. - getRunApi.mockResolvedValue(queueFullView); + // The polled run comes back queue-rejected once; the retry resolves the file + // and fires a fresh run, whose own polls then see it genuinely running. + getRunApi.mockResolvedValueOnce(queueFullView).mockResolvedValue({ + runId: "run-2", + status: "RUNNING", + currentStep: 1, + stepCount: 2, + error: null, + errorCode: null, + outputs: [], + } as never); getFile.mockResolvedValue({ size: 1234 } as never); runStored.mockResolvedValue("run-2"); @@ -92,19 +101,20 @@ describe("auto-run queue-rejection retry", () => { return usePolicyRuns(); }); - // First poll (2s cadence) sees the rejection → relabel as a soft "retrying" row. + // First poll sees the rejection → relabel as a soft "retrying" row. await act(async () => { await vi.advanceTimersByTimeAsync(2000); }); expect(getRun("run-1")?.retrying).toBe(true); expect(runStored).not.toHaveBeenCalled(); - // After the first backoff window (BASE 4s) the rejected record is dropped and a fresh run fires. + // After the first backoff window (BASE 4s) the rejected record is dropped and + // a fresh run fires; its own first poll shows it genuinely running. await act(async () => { - await vi.advanceTimersByTimeAsync(4000); + await vi.advanceTimersByTimeAsync(6000); }); expect(runStored).toHaveBeenCalledWith("backend-1", [{ size: 1234 }]); expect(getRun("run-1")).toBeUndefined(); - expect(getRun("run-2")?.status).toBe("PENDING"); + expect(getRun("run-2")?.status).toBe("RUNNING"); }); }); diff --git a/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.ts b/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.ts index ca096b6792..e475d784ea 100644 --- a/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.ts +++ b/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.ts @@ -39,6 +39,11 @@ import { dispatchPaygLimitReached } from "@app/services/usageLimitBridge"; import type { FileId } from "@app/types/file"; import { createStirlingFilesAndStubs } from "@app/services/fileStubHelpers"; import { readClassificationLabelsFromFile } from "@app/services/fileClassification"; +import { isClassificationCategory } from "@app/data/policyCategories"; +import { + acquireDispatchSlot, + releaseDispatchSlot, +} from "@app/components/policies/dispatchSemaphore"; import type { StirlingFile, StirlingFileStub } from "@app/types/fileContext"; import type { PoliciesByCategory } from "@app/types/policies"; import { usePolicies } from "@app/hooks/usePolicies"; @@ -59,6 +64,10 @@ import { /** Status poll cadence. */ const POLL_MS = 2000; +/** First poll fires early so a fresh run shows real progress quickly instead of + * sitting on an indeterminate spinner for a full poll interval. */ +const FIRST_POLL_MS = 500; + /** The server aborts any single tool step that runs longer than its internal-API * read timeout, then fails the run — so a run can legitimately stay in flight * for up to this long per step. The client must keep polling at least that long, @@ -175,7 +184,14 @@ export function usePolicyAutoRun(): void { // Classification policy out of the server chain when the AI engine is off. !(id === "classification" && !aiEnabled), ) - .sort(([, a], [, b]) => (a.order ?? 0) - (b.order ?? 0)) + // Classification runs last: it's non-blocking, so an enforcement policy + // running after it would fork a new version and drop the user's edits. + .sort(([idA, a], [idB, b]) => { + const ca = isClassificationCategory(idA) ? 1 : 0; + const cb = isClassificationCategory(idB) ? 1 : 0; + if (ca !== cb) return ca - cb; + return (a.order ?? 0) - (b.order ?? 0); + }) .map(([id]) => id), [policies, aiEnabled], ); @@ -310,6 +326,7 @@ export function usePolicyAutoRun(): void { backendId, outputId as FileId, run.fileName, + true, // chained → jump the dispatch queue ahead of new files ).catch(() => {}); } } @@ -330,15 +347,33 @@ export function usePolicyAutoRun(): void { // so the enforced file appears in the app rather than only on the backend. useEffect(() => { for (const run of runs) { + const classification = isClassificationCategory(run.categoryId); if ( run.status !== "COMPLETED" || run.imported || - !run.outputs?.length || - importing.current.has(run.runId) + importing.current.has(run.runId) || + // Classification settles even with no outputs (nothing to tag); other + // policies need an output to import. + (!run.outputs?.length && !classification) ) { continue; } importing.current.add(run.runId); + // Classification is metadata-only: stamp labels onto the current leaf of + // the file it ran on (no version fork). See importClassificationLabels. + if (classification) { + // Targets are resolved by importClassificationLabels AT WRITE TIME (not + // snapshotted here): its download/parse is an async window during which + // a manual tool run can consume the input and fork a new leaf, and a + // stale snapshot would no-op on the dead id and lose the labels. + void importClassificationLabels( + run, + () => + classificationLabelTargetStubs(run.fileId, fileStubsRef.current), + { updateStirlingFileStub, bumpRevision }, + ).finally(() => importing.current.delete(run.runId)); + continue; + } // Honour the policy's output mode: a new file, or a new version of the // input file it ran on (needs that input's stub, still in the workspace). const outputMode = policies[run.categoryId]?.outputMode ?? "new_version"; @@ -501,6 +536,142 @@ function categoryForPolicy( )?.[0]; } +interface ClassificationImportContext { + updateStirlingFileStub: ( + fileId: FileId, + updates: Partial, + ) => void; + bumpRevision: () => void; +} + +/** Workspace stubs to tag with a classification run's labels: the file it ran + * on plus any live descendants, so an edit made during the async run (which + * forks a new leaf) still shows the tags. Empty once the document has left the + * workspace (closed, or a reconciled run with no local input link). */ +export function classificationLabelTargetStubs( + runFileId: string, + stubs: ReadonlyArray, +): StirlingFileStub[] { + return stubs.filter( + (s) => + (s.id as string) === runFileId || + s.parentFileId === runFileId || + s.sourceFileIds?.includes(runFileId as FileId), + ); +} + +/** Attempts to read a completed run's labels before giving up, and the backoff + * between them (delay × attempt). The import effect only re-runs when the run + * store changes, so a transient read failure has to be retried HERE: bailing + * out would leave the run unsettled and the file's "running" pill spinning + * until unrelated policy activity happened to nudge the effect. */ +const LABEL_READ_ATTEMPTS = 3; +const LABEL_READ_RETRY_MS = 2000; + +/** + * Read classification labels out of a completed run's output PDF. A 404 means + * that output aged out, so it's skipped; any other failure is transient and + * retried with backoff. Returns null when there are genuinely no labels to + * apply (including a run with no outputs), so the caller can settle the run. + */ +async function readRunLabels(run: PolicyRunRecord): Promise { + for (let attempt = 0; attempt < LABEL_READ_ATTEMPTS; attempt++) { + if (attempt > 0) await delay(LABEL_READ_RETRY_MS * attempt); + let transientFailure = false; + for (const out of run.outputs) { + try { + const blob = await downloadPolicyOutput(out.fileId, run.target); + const file = new File([blob], out.fileName ?? run.fileName, { + type: blob.type || "application/pdf", + }); + const labels = await readClassificationLabelsFromFile(file); + if (labels && labels.length > 0) return labels; + } catch (err) { + if (!isNotFoundError(err)) transientFailure = true; + } + } + // Every output was read (or had aged out): there are no labels to apply. + if (!transientFailure) return null; + } + // Out of attempts. Settle the run unlabelled rather than spin forever; the + // file keeps its classification badge, just without tags. + return null; +} + +/** + * Stamp `labels` onto the run's live descendants in place (workspace + storage) + * — no versioned child, no history entry, only tags. Returns the tagged ids. + * + * Runs twice, because `resolveTargets` reads a rendered snapshot of the + * workspace: a CONSUME_FILES that was dispatched but not yet rendered when the + * first pass ran leaves its target already gone by the time UPDATE_FILE_RECORD + * is processed, so that stamp no-ops and the labels would be silently lost. The + * second pass sees the forked leaf and tags it. Each id is stamped at most once + * across both passes, so the pass costs nothing when no consume raced. + */ +async function stampClassificationLabels( + labels: string[], + resolveTargets: () => StirlingFileStub[], + ctx: ClassificationImportContext, +): Promise { + const updates = { classificationLabels: labels }; + const tagged = new Set(); + + for (let pass = 0; pass < 2; pass++) { + // Resolve and stamp the store in one synchronous block — no await between + // them, so a target can't be consumed in between. A consume AFTER the stamp + // is safe too: the CONSUME_FILES reducer carries classificationLabels onto + // the new leaf. + const fresh = resolveTargets().filter((s) => !tagged.has(s.id)); + for (const stub of fresh) { + tagged.add(stub.id); + ctx.updateStirlingFileStub(stub.id, updates); + } + + let mutated = false; + for (const stub of fresh) { + if (await fileStorage.updateFileMetadata(stub.id, updates)) + mutated = true; + } + if (mutated) ctx.bumpRevision(); + + // Yield a macrotask so React processes this pass's stamps (and any consume + // that raced them) before the next pass re-resolves. + if (pass === 0) await new Promise((resolve) => setTimeout(resolve)); + } + return Array.from(tagged); +} + +/** + * Deliver a classification run: read its labels and tag the live document with + * them. Metadata-only — nothing is versioned. + */ +async function importClassificationLabels( + run: PolicyRunRecord, + resolveTargets: () => StirlingFileStub[], + ctx: ClassificationImportContext, +): Promise { + if (resolveTargets().length === 0) { + // The document left the workspace (closed, or a server-reconciled run with + // no local input link) — nothing to tag. + updateRun(run.runId, { imported: true }); + return; + } + const labels = await readRunLabels(run); + const targetIds = + labels && labels.length > 0 + ? await stampClassificationLabels(labels, resolveTargets, ctx) + : []; + // Settle either way so it stops re-importing. outputFileIds are the TAGGED + // workspace files (no forked version), so their policy badge persists. Safe + // to chain-key on: classification is always last, so nothing chains off it. + updateRun(run.runId, { + imported: true, + importedFileIds: run.outputs.map((o) => o.fileId), + outputFileIds: targetIds, + }); +} + /** * Fetch a completed run's not-yet-imported output files and deliver them to the * workspace. Per-output, via allSettled: each output is tracked once delivered, @@ -737,6 +908,9 @@ async function runPolicyOnFile( backendId: string, fileId: FileId, fileName: string, + // Chained (downstream) dispatch — jumps the dispatch queue so a file mid-chain + // finishes its flow before new files start (see acquireDispatchSlot). + priority = false, ): Promise { // A freshly-uploaded file's bytes are written to IndexedDB asynchronously, so // its stub can appear in the file list a beat before getStirlingFile resolves @@ -762,6 +936,9 @@ async function runPolicyOnFile( markDispatched(categoryId, fileId); return; } + // Bounded upload window — see MAX_CONCURRENT_DISPATCHES. Only the POST is + // gated; the IDB wait above never holds a slot. + await acquireDispatchSlot(priority); try { const target = resolvePolicyRunTarget(); const runId = await runStoredPolicy(backendId, [file]); @@ -783,6 +960,8 @@ async function runPolicyOnFile( // the absent run simply won't appear in the activity feed. If the backend did // start a run we never recorded, reconcileServerRuns rediscovers it. markDispatched(categoryId, fileId); + } finally { + releaseDispatchSlot(); } } @@ -803,8 +982,10 @@ export async function poll( // would quit while a long step is still legitimately running. let budgetMs = DEFAULT_STEP_COUNT * STEP_TIMEOUT_MS + POLL_GRACE_MS; const startedAt = Date.now(); + let nextDelayMs = FIRST_POLL_MS; while (Date.now() - startedAt < budgetMs) { - await delay(POLL_MS); + await delay(nextDelayMs); + nextDelayMs = POLL_MS; let view; try { view = await getPolicyRun(runId); diff --git a/frontend/editor/src/proprietary/components/shared/PolicyEnforcingOverlay.tsx b/frontend/editor/src/proprietary/components/shared/PolicyEnforcingOverlay.tsx index 45dea92070..048bca15ac 100644 --- a/frontend/editor/src/proprietary/components/shared/PolicyEnforcingOverlay.tsx +++ b/frontend/editor/src/proprietary/components/shared/PolicyEnforcingOverlay.tsx @@ -11,6 +11,7 @@ import { import { ActionIcon } from "@app/ui/ActionIcon"; import ShieldOutlinedIcon from "@mui/icons-material/ShieldOutlined"; import CloseIcon from "@mui/icons-material/Close"; +import { policyCategoryIcon } from "@app/components/policies/policyCategoryIcon"; import { useTranslation } from "react-i18next"; interface PolicyEnforcingOverlayProps { @@ -23,6 +24,9 @@ interface PolicyEnforcingOverlayProps { /** CSS colour var of the enforcing policy's accent (e.g. `var(--color-orange)`), * so the icon/spinner match that policy's badge instead of a fixed blue. */ accentVar?: string; + /** Category of the enforcing policy — picks its shared icon (shield for + * security, label for classification, …); generic shield when unknown. */ + categoryId?: string; } /** @@ -35,6 +39,7 @@ export function PolicyEnforcingOverlay({ zIndex = 200, onDismiss, accentVar, + categoryId, }: PolicyEnforcingOverlayProps) { const { t } = useTranslation(); if (!enforcing) return null; @@ -87,7 +92,11 @@ export function PolicyEnforcingOverlay({ : undefined } > - + {categoryId ? ( + policyCategoryIcon(categoryId, { fontSize: 26 }) + ) : ( + + )} {t("policy.enforcingTitle", "Enforcing policy…")} diff --git a/frontend/editor/src/proprietary/components/viewer/PolicyEnforcementOverlay.tsx b/frontend/editor/src/proprietary/components/viewer/PolicyEnforcementOverlay.tsx index 75fa9cb952..71d5294f2d 100644 --- a/frontend/editor/src/proprietary/components/viewer/PolicyEnforcementOverlay.tsx +++ b/frontend/editor/src/proprietary/components/viewer/PolicyEnforcementOverlay.tsx @@ -71,6 +71,7 @@ export function PolicyEnforcementOverlay({ runs }: Props) { progress={progress} onDismiss={() => setDismissed(true)} accentVar={policyAccentVar(inFlight.categoryId)} + categoryId={inFlight.categoryId} /> ); } diff --git a/frontend/editor/src/proprietary/components/viewer/Viewer.tsx b/frontend/editor/src/proprietary/components/viewer/Viewer.tsx index e1d0cd96ab..1f04ea22ca 100644 --- a/frontend/editor/src/proprietary/components/viewer/Viewer.tsx +++ b/frontend/editor/src/proprietary/components/viewer/Viewer.tsx @@ -8,6 +8,7 @@ import { usePolicyRuns, type PolicyRunRecord, } from "@app/components/policies/policyRunStore"; +import { isClassificationCategory } from "@app/data/policyCategories"; import { PolicyEnforcementOverlay } from "@app/components/viewer/PolicyEnforcementOverlay"; type SignatureOverlayPassThrough = Pick< @@ -29,6 +30,8 @@ const Viewer = (props: ViewerProps & SignatureOverlayPassThrough) => { ? allRuns.filter( (r: PolicyRunRecord) => r.fileId === activeFileId && + // Classification runs async and must never block the viewer. + !isClassificationCategory(r.categoryId) && (POLICY_IN_FLIGHT_STATUSES.includes(r.status) || r.retrying === true), ) : []; diff --git a/frontend/editor/src/proprietary/data/policyCategories.test.ts b/frontend/editor/src/proprietary/data/policyCategories.test.ts new file mode 100644 index 0000000000..4b4fca2b7e --- /dev/null +++ b/frontend/editor/src/proprietary/data/policyCategories.test.ts @@ -0,0 +1,41 @@ +import { describe, it, expect } from "vitest"; +import { + isClassificationCategory, + pinClassificationLast, +} from "@app/data/policyCategories"; + +describe("isClassificationCategory", () => { + it("recognises the classification category and nothing else", () => { + expect(isClassificationCategory("classification")).toBe(true); + expect(isClassificationCategory("security")).toBe(false); + expect(isClassificationCategory("")).toBe(false); + }); +}); + +describe("pinClassificationLast", () => { + it("moves classification to the end, preserving other order", () => { + expect( + pinClassificationLast(["classification", "security", "compliance"]), + ).toEqual(["security", "compliance", "classification"]); + }); + + it("leaves an order without classification untouched", () => { + expect(pinClassificationLast(["security", "compliance"])).toEqual([ + "security", + "compliance", + ]); + }); + + it("is a no-op when classification is already last", () => { + expect(pinClassificationLast(["security", "classification"])).toEqual([ + "security", + "classification", + ]); + }); + + it("handles classification as the only policy", () => { + expect(pinClassificationLast(["classification"])).toEqual([ + "classification", + ]); + }); +}); diff --git a/frontend/editor/src/proprietary/data/policyCategories.ts b/frontend/editor/src/proprietary/data/policyCategories.ts new file mode 100644 index 0000000000..820a93366b --- /dev/null +++ b/frontend/editor/src/proprietary/data/policyCategories.ts @@ -0,0 +1,21 @@ +/** The classification policy's catalog category id. */ +export const CLASSIFICATION_CATEGORY_ID = "classification"; + +/** + * Classification is metadata-only: it runs async (never blocks), never forks a + * version, and always runs last. This predicate gates that special handling. + */ +export function isClassificationCategory(categoryId: string): boolean { + return categoryId === CLASSIFICATION_CATEGORY_ID; +} + +/** + * Move classification to the end of an execution order (others keep their order), + * so a persisted/displayed order can't place it anywhere but last. + */ +export function pinClassificationLast(orderedCategoryIds: string[]): string[] { + return [ + ...orderedCategoryIds.filter((id) => !isClassificationCategory(id)), + ...orderedCategoryIds.filter((id) => isClassificationCategory(id)), + ]; +} diff --git a/frontend/editor/src/proprietary/hooks/usePolicies.ts b/frontend/editor/src/proprietary/hooks/usePolicies.ts index 8043e6b66a..389f98cd65 100644 --- a/frontend/editor/src/proprietary/hooks/usePolicies.ts +++ b/frontend/editor/src/proprietary/hooks/usePolicies.ts @@ -35,6 +35,7 @@ import { removePolicy, } from "@app/services/policyBackend"; import { reorderPolicies as reorderBackendPolicies } from "@app/services/policyApi"; +import { pinClassificationLast } from "@app/data/policyCategories"; import type { PolicyToStore } from "@app/services/policyPipeline"; import type { PoliciesByCategory, @@ -326,9 +327,12 @@ export function usePolicies() { * first for an instant re-render; the next reconcile re-reads the server order. */ const reorderPolicies = useCallback((orderedCategoryIds: string[]) => { - persistPolicyOrder(orderedCategoryIds); + // Pin classification last so the persisted/server order matches execution + // (it always runs last — see usePolicyAutoRun). + const ordered = pinClassificationLast(orderedCategoryIds); + persistPolicyOrder(ordered); const current = loadPolicies(); - const backendIds = orderedCategoryIds + const backendIds = ordered .map((categoryId) => current[categoryId]?.backendId) .filter((id): id is string => !!id); if (backendIds.length > 0) { diff --git a/frontend/editor/src/proprietary/hooks/usePolicyFileBadges.test.ts b/frontend/editor/src/proprietary/hooks/usePolicyFileBadges.test.ts index c5d8b9952b..2364321213 100644 --- a/frontend/editor/src/proprietary/hooks/usePolicyFileBadges.test.ts +++ b/frontend/editor/src/proprietary/hooks/usePolicyFileBadges.test.ts @@ -1,11 +1,14 @@ import { describe, it, expect } from "vitest"; -import { buildPolicyBadgeMap } from "@app/hooks/usePolicyFileBadges"; +import { + buildPolicyBadgeMap, + reusePolicyBadgeArrays, +} from "@app/hooks/usePolicyFileBadges"; import type { PolicyRunRecord } from "@app/components/policies/policyRunStore"; -const NOW = 1_000_000; const labels = new Map([ ["security", "Security"], ["watermark", "Watermark"], + ["classification", "Classification"], ]); function run(overrides: Partial): PolicyRunRecord { @@ -20,29 +23,24 @@ function run(overrides: Partial): PolicyRunRecord { outputs: [], outputFileIds: ["out"], error: null, - startedAt: NOW - 1_000, // recent by default + startedAt: 0, ...overrides, }; } describe("buildPolicyBadgeMap — badge follows the document onto derived files", () => { - it("badges a policy's direct output, and marks it recent within the window", () => { - const map = buildPolicyBadgeMap([run({})], [{ id: "out" }], labels, NOW); - const badges = map.get("out") ?? []; - expect(badges.map((b) => b.id)).toEqual(["security"]); - expect(badges[0].recent).toBe(true); + it("badges a policy's direct output", () => { + const map = buildPolicyBadgeMap([run({})], [{ id: "out" }], labels); + expect((map.get("out") ?? []).map((b) => b.id)).toEqual(["security"]); }); - it("a versioned edit inherits the badge via parentFileId (never glows)", () => { + it("a versioned edit inherits the badge via parentFileId", () => { const map = buildPolicyBadgeMap( [run({})], [{ id: "out" }, { id: "edit", parentFileId: "out" }], labels, - NOW, ); - const edit = map.get("edit") ?? []; - expect(edit.map((b) => b.id)).toEqual(["security"]); - expect(edit[0].recent).toBe(false); + expect((map.get("edit") ?? []).map((b) => b.id)).toEqual(["security"]); }); it("SPLIT parts inherit the badge via sourceFileIds, though they have no parent", () => { @@ -56,11 +54,9 @@ describe("buildPolicyBadgeMap — badge follows the document onto derived files" { id: "part2", sourceFileIds: ["out"] }, ], labels, - NOW, ); expect((map.get("part1") ?? []).map((b) => b.id)).toEqual(["security"]); expect((map.get("part2") ?? []).map((b) => b.id)).toEqual(["security"]); - expect((map.get("part1") ?? [])[0].recent).toBe(false); }); it("resolves transitively when an intermediate edit was consumed/removed", () => { @@ -70,7 +66,6 @@ describe("buildPolicyBadgeMap — badge follows the document onto derived files" [run({})], [{ id: "part", sourceFileIds: ["editGone", "out"] }], labels, - NOW, ); expect((map.get("part") ?? []).map((b) => b.id)).toEqual(["security"]); }); @@ -83,7 +78,6 @@ describe("buildPolicyBadgeMap — badge follows the document onto derived files" ], [{ id: "merged", sourceFileIds: ["a", "b"] }], labels, - NOW, ); expect((map.get("merged") ?? []).map((b) => b.id).sort()).toEqual([ "security", @@ -96,24 +90,33 @@ describe("buildPolicyBadgeMap — badge follows the document onto derived files" [run({})], [{ id: "out" }, { id: "unrelated", sourceFileIds: ["someUpload"] }], labels, - NOW, ); expect(map.has("unrelated")).toBe(false); }); - it("inherited badges never glow even when the source run is recent", () => { + it("a completed classification run badges the files it tagged", () => { + // Classification is metadata-only: its outputFileIds are the tagged + // workspace files (no forked version), so the label badge persists there. const map = buildPolicyBadgeMap( - [run({ startedAt: NOW })], // maximally recent - [{ id: "out" }, { id: "part", sourceFileIds: ["out"] }], + [ + run({ + categoryId: "classification", + fileId: "in", + outputFileIds: ["in"], + imported: true, + }), + ], + [{ id: "in" }], labels, - NOW, ); - expect((map.get("out") ?? [])[0].recent).toBe(true); - expect((map.get("part") ?? [])[0].recent).toBe(false); + const badges = map.get("in") ?? []; + expect(badges.map((b) => b.id)).toEqual(["classification"]); + expect(badges[0].enforcing).toBeUndefined(); + expect(badges[0].background).toBeUndefined(); }); }); -describe("buildPolicyBadgeMap — enforcing spinner while a run is in flight", () => { +describe("buildPolicyBadgeMap — in-flight indicators", () => { const enforcingOn = ( map: Map, id: string, @@ -124,7 +127,6 @@ describe("buildPolicyBadgeMap — enforcing spinner while a run is in flight", ( [run({ status: "RUNNING", outputFileIds: [] })], [{ id: "in" }], labels, - NOW, ); expect(enforcingOn(map, "in")).toBe(true); }); @@ -136,7 +138,6 @@ describe("buildPolicyBadgeMap — enforcing spinner while a run is in flight", ( [run({ status: "COMPLETED" })], [{ id: "in" }], labels, - NOW, ); expect(enforcingOn(before, "in")).toBe(true); @@ -144,7 +145,6 @@ describe("buildPolicyBadgeMap — enforcing spinner while a run is in flight", ( [run({ status: "COMPLETED", imported: true })], [{ id: "in" }], labels, - NOW, ); expect(enforcingOn(after, "in")).toBe(false); }); @@ -155,7 +155,6 @@ describe("buildPolicyBadgeMap — enforcing spinner while a run is in flight", ( [run({ status, outputFileIds: [] })], [{ id: "in" }], labels, - NOW, ); expect(enforcingOn(map, "in")).toBe(false); } @@ -166,7 +165,6 @@ describe("buildPolicyBadgeMap — enforcing spinner while a run is in flight", ( [run({ status: "FAILED", retrying: true, outputFileIds: [] })], [{ id: "in" }], labels, - NOW, ); expect(enforcingOn(map, "in")).toBe(true); }); @@ -176,8 +174,97 @@ describe("buildPolicyBadgeMap — enforcing spinner while a run is in flight", ( [run({ status: "RUNNING", fileId: "", outputFileIds: [] })], [{ id: "in" }], labels, - NOW, ); expect(enforcingOn(map, "in")).toBe(false); }); + + it("an in-flight classification run is background, never enforcing", () => { + // Non-blocking: shows a spinner but must never trip the enforcing flag + // that gates actions and overlays. + const map = buildPolicyBadgeMap( + [ + run({ + categoryId: "classification", + status: "RUNNING", + outputFileIds: [], + }), + ], + [{ id: "in" }], + labels, + ); + const badges = map.get("in") ?? []; + expect(badges.map((b) => b.id)).toEqual(["classification"]); + expect(badges[0].background).toBe(true); + expect(enforcingOn(map, "in")).toBe(false); + }); +}); + +describe("reusePolicyBadgeArrays — per-file identity across rebuilds", () => { + // buildPolicyBadgeMap allocates fresh arrays every call and the run store hands + // back a new `runs` array on every status poll, so without this the memoized + // sidebar rows get a new `policies` prop for EVERY badged file on each tick. + const build = (runs: PolicyRunRecord[], stubs: { id: string }[]) => + buildPolicyBadgeMap(runs, stubs, labels); + + const twoFiles = [{ id: "a" }, { id: "b" }]; + // Settled + imported, so the badge is a plain one (a COMPLETED run keeps + // `enforcing` until its outputs land — see the in-flight tests above). + const settled = (id: string) => + run({ + runId: `r${id}`, + fileId: id, + outputFileIds: [id], + status: "COMPLETED", + imported: true, + }); + const bothSettled = () => [settled("a"), settled("b")]; + + it("returns the same map when nothing changed", () => { + const first = build(bothSettled(), twoFiles); + const second = reusePolicyBadgeArrays( + first, + build(bothSettled(), twoFiles), + ); + expect(second).toBe(first); + }); + + it("keeps the untouched file's array identity when another file changes", () => { + const first = build(bothSettled(), twoFiles); + // "a" goes in-flight; "b" is unaffected and must keep its exact array. + const next = build( + [ + run({ + runId: "ra", + fileId: "a", + outputFileIds: ["a"], + status: "RUNNING", + }), + settled("b"), + ], + twoFiles, + ); + const second = reusePolicyBadgeArrays(first, next); + expect(second).not.toBe(first); + expect(second.get("b")).toBe(first.get("b")); + expect(second.get("a")).not.toBe(first.get("a")); + expect((second.get("a") ?? [])[0].enforcing).toBe(true); + expect((first.get("a") ?? [])[0].enforcing).toBeUndefined(); + }); + + it("a new badged file doesn't disturb the existing files' arrays", () => { + const first = build(bothSettled(), twoFiles); + const next = build( + [...bothSettled(), settled("c")], + [...twoFiles, { id: "c" }], + ); + const second = reusePolicyBadgeArrays(first, next); + expect(second.get("a")).toBe(first.get("a")); + expect(second.get("b")).toBe(first.get("b")); + expect((second.get("c") ?? []).map((b) => b.id)).toEqual(["security"]); + }); + + it("passes the fresh map straight through on the first build", () => { + const map = build(bothSettled(), twoFiles); + expect(reusePolicyBadgeArrays(null, map)).toBe(map); + }); }); diff --git a/frontend/editor/src/proprietary/hooks/usePolicyFileBadges.ts b/frontend/editor/src/proprietary/hooks/usePolicyFileBadges.ts index 40fcd0399e..52779b9cf8 100644 --- a/frontend/editor/src/proprietary/hooks/usePolicyFileBadges.ts +++ b/frontend/editor/src/proprietary/hooks/usePolicyFileBadges.ts @@ -1,17 +1,12 @@ -import { useMemo } from "react"; +import { useMemo, useRef } from "react"; import { usePolicyRuns } from "@app/components/policies/policyRunStore"; import type { PolicyRunRecord } from "@app/components/policies/policyRunStore"; import { useAllFiles } from "@app/contexts/FileContext"; import { loadPolicyCatalog } from "@app/services/policyCatalog"; import { policyAccentVar } from "@app/components/policies/policyStatus"; +import { isClassificationCategory } from "@app/data/policyCategories"; import type { FileItemPolicyRef } from "@app/components/shared/PolicyBadges"; -/** How long after a run a badge counts as "recent" (drives the one-off glow). - * Measured from run start — must exceed the longest realistic policy wall-clock - * time so the glow still fires after a slow run completes and imports. Old or - * reloaded runs fall outside this window, suppressing the glow on page reload. */ -const RECENT_MS = 5 * 60 * 1000; - /** Minimal provenance shape needed to resolve a file's inherited badges. */ type LineageStub = { id: string; @@ -19,14 +14,10 @@ type LineageStub = { sourceFileIds?: string[]; }; -/** Merge a ref into a list, deduping by policy id. A direct (recent) hit wins - * the glow over an inherited one for the same policy. */ +/** Merge a ref into a list, deduping by policy id. */ function mergeRef(list: FileItemPolicyRef[], ref: FileItemPolicyRef): void { - const existing = list.find((p) => p.id === ref.id); - if (!existing) { + if (!list.some((p) => p.id === ref.id)) { list.push(ref); - } else if (ref.recent) { - existing.recent = true; } } @@ -42,21 +33,18 @@ function mergeRef(list: FileItemPolicyRef[], ref: FileItemPolicyRef): void { * from: its transitive `sourceFileIds` (recorded at the consume boundary, so it * covers split/merge/convert too) plus, defensively, its `parentFileId`. * Because `sourceFileIds` is transitive, a flat lookup suffices — no chain walk, - * and it survives a consumed intermediate. Inherited badges never glow - * (recent=false): only the original application does. + * and it survives a consumed intermediate. */ export function buildPolicyBadgeMap( runs: ReadonlyArray, stubs: ReadonlyArray, labelById: ReadonlyMap, - now: number, ): Map { // Direct badges: a file that IS a policy run's output. const directByFile = new Map(); for (const run of runs) { const name = labelById.get(run.categoryId); if (!name) continue; - const recent = now - run.startedAt < RECENT_MS; for (const fileId of run.outputFileIds ?? []) { const list = directByFile.get(fileId) ?? []; if (!list.some((p) => p.id === run.categoryId)) { @@ -64,7 +52,6 @@ export function buildPolicyBadgeMap( id: run.categoryId, name, accentColor: policyAccentVar(run.categoryId), - recent, }); directByFile.set(fileId, list); } @@ -86,7 +73,7 @@ export function buildPolicyBadgeMap( // from. `sourceFileIds` is the transitive provenance set (so a flat lookup // catches even ancestors whose intermediate edits were consumed), and // `parentFileId` is included defensively for any child not created via a - // consume. Inherited badges are marked recent=false (carried, not applied). + // consume. for (const stub of stubs) { const sources = new Set(stub.sourceFileIds ?? []); if (stub.parentFileId) sources.add(stub.parentFileId); @@ -94,14 +81,17 @@ export function buildPolicyBadgeMap( const srcBadges = directByFile.get(src); if (!srcBadges?.length) continue; const list = result.get(stub.id) ?? []; - for (const ref of srcBadges) mergeRef(list, { ...ref, recent: false }); + for (const ref of srcBadges) mergeRef(list, { ...ref }); result.set(stub.id, list); } } // In-flight pass: add (or upgrade) a badge on the input file for any run that // is currently being processed, so the sidebar shows a spinning indicator - // while the policy is actively enforcing — not just after it completes. + // while the policy is actively running — not just after it completes. + // Blocking policies set `enforcing` (which gates actions/overlays); + // classification is non-blocking, so it sets `background` instead — same + // spinner, but nothing is ever gated on it. // Keep the spinner until `imported` is true: the status reaches COMPLETED // before the output files are imported into the workspace, so gating on // status alone would drop the badge during that async gap. @@ -112,17 +102,19 @@ export function buildPolicyBadgeMap( if (settled && !run.retrying) continue; const name = labelById.get(run.categoryId); if (!name) continue; + const inFlightFlag = isClassificationCategory(run.categoryId) + ? ("background" as const) + : ("enforcing" as const); const list = result.get(run.fileId) ?? []; const existing = list.find((p) => p.id === run.categoryId); if (existing) { - existing.enforcing = true; + existing[inFlightFlag] = true; } else { list.push({ id: run.categoryId, name, accentColor: policyAccentVar(run.categoryId), - recent: false, - enforcing: true, + [inFlightFlag]: true, }); result.set(run.fileId, list); } @@ -131,20 +123,70 @@ export function buildPolicyBadgeMap( return result; } +/** Field-wise equality for a badge ref — the whole shape `PolicyBadges` renders. */ +function sameRef(a: FileItemPolicyRef, b: FileItemPolicyRef): boolean { + return ( + a.id === b.id && + a.name === b.name && + a.accentColor === b.accentColor && + !!a.enforcing === !!b.enforcing && + !!a.background === !!b.background + ); +} + +function sameRefs(a: FileItemPolicyRef[], b: FileItemPolicyRef[]): boolean { + return a.length === b.length && a.every((ref, i) => sameRef(ref, b[i])); +} + +/** + * Carry the previous map's array references over to files whose badges didn't + * change, and return the previous MAP itself when none did. + * + * {@link buildPolicyBadgeMap} allocates a fresh array per badged file on every + * call, and the run store hands back a new `runs` array on every status poll — + * so without this, one file's poll tick gives EVERY badged file a new `policies` + * identity, and the memoized sidebar rows can never bail out (the case the + * memoization exists for). `NO_POLICIES` in FileSidebar only covers the rows + * with no badges at all. + */ +export function reusePolicyBadgeArrays( + previous: Map | null, + next: Map, +): Map { + if (!previous) return next; + let changed = previous.size !== next.size; + for (const [fileId, refs] of next) { + const before = previous.get(fileId); + if (before && sameRefs(before, refs)) next.set(fileId, before); + else changed = true; + } + return changed ? next : previous; +} + /** * Distinct policies that have produced each file, keyed by fileId, derived from * the reactive policy run store. Drives the file sidebar's shield badges. The * badge follows a document down its tool-edit chain — see * {@link buildPolicyBadgeMap}. Shadows the core stub via the {@code @app/*} * alias cascade. + * + * Per-file array identity is preserved across rebuilds so memoized consumers + * (the sidebar rows) only re-render for the file that actually changed — see + * {@link reusePolicyBadgeArrays}. */ export function usePolicyFileBadges(): Map { const runs = usePolicyRuns(); const { fileStubs } = useAllFiles(); + const previous = useRef | null>(null); return useMemo(() => { const labelById = new Map( loadPolicyCatalog().categories.map((c) => [c.id, c.label]), ); - return buildPolicyBadgeMap(runs, fileStubs, labelById, Date.now()); + const map = reusePolicyBadgeArrays( + previous.current, + buildPolicyBadgeMap(runs, fileStubs, labelById), + ); + previous.current = map; + return map; }, [runs, fileStubs]); } diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 970d6703cd..7db904ddf6 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -84,6 +84,7 @@ "signature_pad": "^5.0.4", "smol-toml": "^1.4.2", "tailwindcss": "^4.1.13", + "use-sync-external-store": "^1.6.0", "web-vitals": "^5.1.0" }, "devDependencies": { @@ -109,6 +110,7 @@ "@types/node": "^24.5.2", "@types/react": "^19.2.17", "@types/react-dom": "^19.1.9", + "@types/use-sync-external-store": "^0.0.6", "@typescript-eslint/eslint-plugin": "^8.65.0", "@typescript-eslint/parser": "^8.65.0", "@typescript/native": "npm:typescript@^7.0.2", diff --git a/frontend/package.json b/frontend/package.json index 1763a2126a..dfcfa2842c 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -81,6 +81,7 @@ "signature_pad": "^5.0.4", "smol-toml": "^1.4.2", "tailwindcss": "^4.1.13", + "use-sync-external-store": "^1.6.0", "web-vitals": "^5.1.0" }, "scripts": { @@ -131,6 +132,7 @@ "@types/node": "^24.5.2", "@types/react": "^19.2.17", "@types/react-dom": "^19.1.9", + "@types/use-sync-external-store": "^0.0.6", "@typescript-eslint/eslint-plugin": "^8.65.0", "@typescript-eslint/parser": "^8.65.0", "@typescript/native": "npm:typescript@^7.0.2",