diff --git a/src/renderer/src/components/editor/editor-external-watch-path-index.ts b/src/renderer/src/components/editor/editor-external-watch-path-index.ts index f19dcfbfc45..3ce5f2a432b 100644 --- a/src/renderer/src/components/editor/editor-external-watch-path-index.ts +++ b/src/renderer/src/components/editor/editor-external-watch-path-index.ts @@ -1,5 +1,5 @@ import { joinPath } from '@/lib/path' -import { createFileExplorerWatchPathResolver } from '@/components/right-sidebar/useFileExplorerWatch' +import { getExternalFileChangeRelativePath } from '@/components/right-sidebar/useFileExplorerWatch' import type { OpenFile } from '@/store/slices/editor' import type { FsChangedPayload } from '../../../../shared/filesystem-entry-types' import { @@ -183,7 +183,6 @@ export function indexEditorExternalWatchBatchPaths( const deleteLookup = new IndexedPathLookup(allowAliases) const createOrUpdatePaths = new Map() const changesByRelativePath = new Map() - const watchPathResolver = createFileExplorerWatchPathResolver(scope.worktreePath) for (const event of payload.events) { if (event.kind === 'overflow') { @@ -201,7 +200,8 @@ export function indexEditorExternalWatchBatchPaths( createOrUpdatePaths.set(eventPath.identity.normalizedPath, event.absolutePath) createOrUpdateLookup.add(eventPath, eventPath) } - const relativePath = watchPathResolver.externalFileChangeRelativePath( + const relativePath = getExternalFileChangeRelativePath( + scope.worktreePath, event.absolutePath, event.isDirectory ) diff --git a/src/renderer/src/components/right-sidebar/file-explorer-watch-path.ts b/src/renderer/src/components/right-sidebar/file-explorer-watch-path.ts index a4ee6e0317d..4066bca2d81 100644 --- a/src/renderer/src/components/right-sidebar/file-explorer-watch-path.ts +++ b/src/renderer/src/components/right-sidebar/file-explorer-watch-path.ts @@ -1,65 +1,42 @@ import { joinPath, dirname, normalizeRelativePath } from '@/lib/path' import { - createRelativePathInsideRootResolver, - normalizeRuntimePathForComparison + normalizeRuntimePathForComparison, + relativePathInsideRoot } from '../../../../shared/cross-platform-path' export function normalizeExplorerAbsolutePath(path: string): string { return path === '/' || /^[A-Za-z]:[\\/]$/.test(path) ? path : path.replace(/[\\/]+$/, '') } -export type FileExplorerWatchPathResolver = { - canonicalize: (absolutePath: string) => string | null - externalFileChangeRelativePath: ( - absolutePath: string, - isDirectory: boolean | undefined - ) => string | null -} - -/** Folds the worktree root once per watcher batch rather than once per event in the storm. */ -export function createFileExplorerWatchPathResolver( - worktreePath: string -): FileExplorerWatchPathResolver { - const insideRoot = createRelativePathInsideRootResolver(worktreePath) - const rootPath = normalizeExplorerAbsolutePath(worktreePath) - return { - canonicalize: (absolutePath) => { - const relativePath = insideRoot.resolve(absolutePath) - if (relativePath === null) { - return null - } - return relativePath === '' ? rootPath : joinPath(rootPath, relativePath) - }, - externalFileChangeRelativePath: (absolutePath, isDirectory) => { - if (isDirectory === true) { - return null - } - const relativePath = insideRoot.resolve(absolutePath) - if (relativePath === null || relativePath === '') { - return null - } - // Why: EditorPanel reloads tabs only from a worktree-relative path, not the watcher's absolute one; normalize or contents go stale. - return normalizeRelativePath(relativePath) - } - } -} - export function getExternalFileChangeRelativePath( worktreePath: string, absolutePath: string, isDirectory: boolean | undefined ): string | null { - return createFileExplorerWatchPathResolver(worktreePath).externalFileChangeRelativePath( - absolutePath, - isDirectory - ) + if (isDirectory === true) { + return null + } + + const relativePath = relativePathInsideRoot(worktreePath, absolutePath) + if (relativePath === null || relativePath === '') { + return null + } + + // Why: EditorPanel reloads tabs only from a worktree-relative path, not the watcher's absolute one; normalize or contents go stale. + return normalizeRelativePath(relativePath) } export function canonicalizeFileExplorerWatchPath( worktreePath: string, absolutePath: string ): string | null { - return createFileExplorerWatchPathResolver(worktreePath).canonicalize(absolutePath) + const relativePath = relativePathInsideRoot(worktreePath, absolutePath) + if (relativePath === null) { + return null + } + + const rootPath = normalizeExplorerAbsolutePath(worktreePath) + return relativePath === '' ? rootPath : joinPath(rootPath, relativePath) } export function createCachedDirPathIndex( diff --git a/src/renderer/src/components/right-sidebar/file-explorer-watch-reconcile.ts b/src/renderer/src/components/right-sidebar/file-explorer-watch-reconcile.ts index 56a804adeae..f5491562f52 100644 --- a/src/renderer/src/components/right-sidebar/file-explorer-watch-reconcile.ts +++ b/src/renderer/src/components/right-sidebar/file-explorer-watch-reconcile.ts @@ -11,8 +11,8 @@ import { clearStalePendingReveal } from './file-explorer-watcher-reconcile' import { + canonicalizeFileExplorerWatchPath, createCachedDirPathIndex, - createFileExplorerWatchPathResolver, normalizeExplorerAbsolutePath, parentDirForWatchPath, resolveCachedDirPath @@ -71,7 +71,6 @@ export function processFileExplorerFsPayload(args: ProcessFileExplorerFsPayloadA let cachedDirPathIndex: ReadonlyMap | undefined const cachePathIndex = (): ReadonlyMap => (cachedDirPathIndex ??= createCachedDirPathIndex(cache)) - const watchPathResolver = createFileExplorerWatchPathResolver(currentWorktreePath) const cachedDirsToPurge = new Set() const reconciledRenameSources = new Set() let needsFullRefresh = false @@ -88,7 +87,7 @@ export function processFileExplorerFsPayload(args: ProcessFileExplorerFsPayloadA break } - const normalizedPath = watchPathResolver.canonicalize(evt.absolutePath) + const normalizedPath = canonicalizeFileExplorerWatchPath(currentWorktreePath, evt.absolutePath) if (!normalizedPath) { continue } @@ -140,7 +139,7 @@ export function processFileExplorerFsPayload(args: ProcessFileExplorerFsPayloadA } if (evt.kind === 'rename') { const oldPath = evt.oldAbsolutePath - ? watchPathResolver.canonicalize(evt.oldAbsolutePath) + ? canonicalizeFileExplorerWatchPath(currentWorktreePath, evt.oldAbsolutePath) : null const cachedOldDir = oldPath ? resolveCachedDirPath(cache, oldPath, currentWorktreePath, cachePathIndex) diff --git a/src/renderer/src/components/right-sidebar/useFileExplorerWatch.ts b/src/renderer/src/components/right-sidebar/useFileExplorerWatch.ts index a6b7ff73d4a..d62a2c9581b 100644 --- a/src/renderer/src/components/right-sidebar/useFileExplorerWatch.ts +++ b/src/renderer/src/components/right-sidebar/useFileExplorerWatch.ts @@ -20,7 +20,6 @@ import { processFileExplorerFsPayload } from './file-explorer-watch-reconcile' export { canonicalizeFileExplorerWatchPath, - createFileExplorerWatchPathResolver, getExternalFileChangeRelativePath, resolveCachedDirPath } from './file-explorer-watch-path' diff --git a/src/shared/cross-platform-path-guards.test.ts b/src/shared/cross-platform-path-guards.test.ts index 1ffe695d871..cf6c729b89b 100644 --- a/src/shared/cross-platform-path-guards.test.ts +++ b/src/shared/cross-platform-path-guards.test.ts @@ -94,7 +94,7 @@ function generateRoot(random: () => number, candidate: string): string { // ─── Differential fuzz ─────────────────────────────────────────────── -const FUZZ_ITERATIONS = 200_000 +const FUZZ_ITERATIONS = 20_000 describe('guarded path normalization matches the pre-guard implementation', () => { it(`agrees on every export across ${FUZZ_ITERATIONS} seeded paths`, () => { @@ -192,17 +192,11 @@ describe('guarded path normalization matches the pre-guard implementation', () = ) { record('createNormalizedPathInsideOrEqualMatcher', path, root) } - const expectedRelative = unguarded.relativePathInsideRoot(root, path) - if (guarded.relativePathInsideRoot(root, path) !== expectedRelative) { + if ( + guarded.relativePathInsideRoot(root, path) !== unguarded.relativePathInsideRoot(root, path) + ) { record('relativePathInsideRoot', path, root) } - const resolver = guarded.createRelativePathInsideRootResolver(root) - if (resolver.resolve(path) !== expectedRelative) { - record('createRelativePathInsideRootResolver.resolve', path, root) - } - if (resolver.comparisonRoot !== unguarded.normalizeRuntimePathForComparison(root)) { - record('createRelativePathInsideRootResolver.comparisonRoot', path, root) - } } expect(mismatches).toEqual([]) @@ -250,11 +244,9 @@ describe('guards still do the work when the fast path does not apply', () => { // ─── Regression guards: counted work, not wall clock ───────────────── const originalReplace = String.prototype.replace -const originalNormalize = String.prototype.normalize afterEach(() => { String.prototype.replace = originalReplace - String.prototype.normalize = originalNormalize }) function countReplaceCalls(run: () => void): number { @@ -271,20 +263,6 @@ function countReplaceCalls(run: () => void): number { return calls } -function countNormalizeCalls(run: () => void): number { - let calls = 0 - String.prototype.normalize = function (this: string, ...args: never[]) { - calls++ - return originalNormalize.apply(this, args as never) - } as typeof String.prototype.normalize - try { - run() - } finally { - String.prototype.normalize = originalNormalize - } - return calls -} - const CLEAN_POSIX_PATH = '/Users/nwparker/orca/workspaces/orca/perf/src/renderer/src/components/x.ts' @@ -305,27 +283,34 @@ describe('no-op regex passes stay skipped', () => { }) }) -describe('a fan-out folds its root once, not once per candidate', () => { - const root = '/Users/nwparker/orca/workspaces/orca/perf' - const candidates = Array.from({ length: 50 }, (_, index) => `${root}/src/file-${index}.ts`) +// ─── One root-bound factory, one input contract ────────────────────── - it('normalizes the root a single time across the whole batch', () => { - // 1 root fold + 2 per candidate (comparison key, then the NFC Windows-ness probe). - const calls = countNormalizeCalls(() => { - const resolver = guarded.createRelativePathInsideRootResolver(root) - for (const candidate of candidates) { - resolver.resolve(candidate) - } - }) - expect(calls).toBe(1 + candidates.length * 2) +/** + * `createNormalizedPathInsideOrEqualMatcher` demands an already-normalized candidate because + * `normalizeRuntimePathForComparison` is not idempotent. A sibling factory on the same root that + * took RAW candidates would put two opposite contracts one line apart, and mixing them up returns + * "outside the root" rather than throwing. Hoisting a root out of a loop is worth ~0.2 us/event; + * this is the price. Keep the raw-candidate entry point the plain `relativePathInsideRoot` call. + */ +describe('cross-platform-path exposes a single root-bound factory', () => { + it('has no raw-candidate sibling to the normalized matcher', () => { + expect(Object.keys(guarded).filter((name) => name.startsWith('create'))).toEqual([ + 'createNormalizedPathInsideOrEqualMatcher' + ]) }) - it('costs strictly more when the root is re-folded per candidate', () => { - const perCall = countNormalizeCalls(() => { - for (const candidate of candidates) { - guarded.relativePathInsideRoot(root, candidate) - } - }) - expect(perCall).toBe(candidates.length * 3) + it('shows what mixing the two contracts would cost', () => { + const root = '//wsl.localhost/Ubuntu/Repo' + const candidate = '//wsl.localhost/Ubuntu/Repo/src/App.tsx' + const normalizedCandidate = guarded.normalizeRuntimePathForComparison(candidate) + expect(guarded.normalizeRuntimePathForComparison(normalizedCandidate)).not.toBe( + normalizedCandidate + ) + + const matcher = guarded.createNormalizedPathInsideOrEqualMatcher(root) + expect(matcher(normalizedCandidate)).toBe(true) + // The raw spelling a resolver would accept is silently reported as outside the root. + expect(matcher(candidate)).toBe(false) + expect(guarded.relativePathInsideRoot(root, candidate)).toBe('src/App.tsx') }) }) diff --git a/src/shared/cross-platform-path.ts b/src/shared/cross-platform-path.ts index d96ee7f8546..f173914c789 100644 --- a/src/shared/cross-platform-path.ts +++ b/src/shared/cross-platform-path.ts @@ -192,50 +192,28 @@ export function isPathInsideOrEqual(rootPath: string, candidatePath: string): bo ) } -export type RelativePathInsideRootResolver = { - /** The root's comparison key, so rankers can size it without a second normalize. */ - readonly comparisonRoot: string - resolve: (candidatePath: string) => string | null -} - -/** - * Pre-normalizes the root so a fan-out normalizes it once, not once per candidate. - * - * Unlike `createNormalizedPathInsideOrEqualMatcher`, candidates are passed raw: the returned - * suffix has to be sliced out of the caller's own spelling, so the resolver needs both forms. - */ -export function createRelativePathInsideRootResolver( - rootPath: string -): RelativePathInsideRootResolver { +export function relativePathInsideRoot(rootPath: string, candidatePath: string): string | null { + // Why: decide Windows-ness on the same NFC form the comparison key uses, or the + // two disagree (U+212A folds to 'K', making only one side a drive path) and the + // segment counts desync. Only the branch test sees NFC — the sliced string stays + // raw so the returned suffix remains byte-exact. + const normalizedCandidate = trimRuntimePathTrailingSlash( + isWindowsAbsolutePathLike(candidatePath.normalize('NFC')) + ? normalizeRuntimePathSeparators(candidatePath) + : collapseRuntimePathSlashes(candidatePath) + ) const comparisonRoot = normalizeRuntimePathForComparison(rootPath) + const comparisonCandidate = normalizeRuntimePathForComparison(candidatePath) + + if (comparisonCandidate === comparisonRoot) { + return '' + } const isRoot = comparisonRoot === '/' || /^[a-z]:\/$/i.test(comparisonRoot) const comparisonPrefix = isRoot ? comparisonRoot : `${comparisonRoot}/` - return { - comparisonRoot, - resolve: (candidatePath) => { - const comparisonCandidate = normalizeRuntimePathForComparison(candidatePath) - if (comparisonCandidate === comparisonRoot) { - return '' - } - if (!comparisonCandidate.startsWith(comparisonPrefix)) { - return null - } - // Why: decide Windows-ness on the same NFC form the comparison key uses, or the - // two disagree (U+212A folds to 'K', making only one side a drive path) and the - // segment counts desync. Only the branch test sees NFC — the sliced string stays - // raw so the returned suffix remains byte-exact. - const normalizedCandidate = trimRuntimePathTrailingSlash( - isWindowsAbsolutePathLike(candidatePath.normalize('NFC')) - ? normalizeRuntimePathSeparators(candidatePath) - : collapseRuntimePathSlashes(candidatePath) - ) - return sliceCandidatePastRootSegments(comparisonRoot, normalizedCandidate) - } + if (!comparisonCandidate.startsWith(comparisonPrefix)) { + return null } -} - -export function relativePathInsideRoot(rootPath: string, candidatePath: string): string | null { - return createRelativePathInsideRootResolver(rootPath).resolve(candidatePath) + return sliceCandidatePastRootSegments(comparisonRoot, normalizedCandidate) } /** diff --git a/src/shared/quick-open-filter.ts b/src/shared/quick-open-filter.ts index 823e03aafa5..9d5bebd80b3 100644 --- a/src/shared/quick-open-filter.ts +++ b/src/shared/quick-open-filter.ts @@ -6,7 +6,7 @@ * Centralized to stop local/relay listFiles from drifting on blocklist, ignores, exclusions, * timeouts, and buffering. See docs/design/share-quick-open-file-listing.md. */ -import { createRelativePathInsideRootResolver } from './cross-platform-path' +import { relativePathInsideRoot } from './cross-platform-path' // ─── Hidden-dir blocklist ──────────────────────────────────────────── @@ -85,12 +85,11 @@ export function buildExcludePathPrefixes(rootPath: string, excludePaths?: unknow return [] } const out: string[] = [] - const insideRoot = createRelativePathInsideRootResolver(rootPath) for (const raw of excludePaths) { if (typeof raw !== 'string' || raw.length === 0) { continue } - const relativePath = insideRoot.resolve(raw) + const relativePath = relativePathInsideRoot(rootPath, raw) if (relativePath === null) { continue } diff --git a/src/shared/runtime-workspace-file-owner.ts b/src/shared/runtime-workspace-file-owner.ts index c8deef4a1b3..1edacb07bbc 100644 --- a/src/shared/runtime-workspace-file-owner.ts +++ b/src/shared/runtime-workspace-file-owner.ts @@ -1,5 +1,5 @@ import type { ExecutionHostId } from './execution-host' -import { createRelativePathInsideRootResolver } from './cross-platform-path' +import { normalizeRuntimePathForComparison, relativePathInsideRoot } from './cross-platform-path' export type RuntimeWorkspaceFileRoot = { workspaceId: string @@ -23,12 +23,11 @@ export function findRuntimeWorkspaceFileOwner( if (root.executionHostId !== executionHostId) { continue } - const insideRoot = createRelativePathInsideRootResolver(root.rootPath) - const relativePath = insideRoot.resolve(absolutePath) + const relativePath = relativePathInsideRoot(root.rootPath, absolutePath) if (relativePath === null) { continue } - const rootLength = insideRoot.comparisonRoot.length + const rootLength = normalizeRuntimePathForComparison(root.rootPath).length if ( rootLength > bestRootLength || (rootLength === bestRootLength && diff --git a/src/shared/worktree/ownership.ts b/src/shared/worktree/ownership.ts index 384bc39ac6e..5784b018315 100644 --- a/src/shared/worktree/ownership.ts +++ b/src/shared/worktree/ownership.ts @@ -1,8 +1,4 @@ -import { - createNormalizedPathInsideOrEqualMatcher, - normalizeRuntimePathForComparison, - relativePathInsideRoot -} from '../cross-platform-path' +import { normalizeRuntimePathForComparison, relativePathInsideRoot } from '../cross-platform-path' import { parseWslUncPath } from '../wsl-paths' import { isRuntimePathAbsoluteForRepo, @@ -245,10 +241,9 @@ function isUnderFlatOrUntrustedOrcaRoot( worktreePath: string, knownOrcaLayouts: OrcaWorkspaceLayout[] ): boolean { - // Only containment is asked here, so fold the candidate once instead of once per layout. - const comparisonWorktreePath = normalizeRuntimePathForComparison(worktreePath) for (const layout of knownOrcaLayouts) { - if (!createNormalizedPathInsideOrEqualMatcher(layout.path)(comparisonWorktreePath)) { + const relative = relativePathInsideRoot(layout.path, worktreePath) + if (relative === null) { continue } if (!layout.nestWorkspaces) { @@ -265,9 +260,9 @@ function canClassifyAsExternal( if (knownOrcaLayouts.length === 0) { return false } - const comparisonWorktreePath = normalizeRuntimePathForComparison(worktreePath) for (const layout of knownOrcaLayouts) { - if (!createNormalizedPathInsideOrEqualMatcher(layout.path)(comparisonWorktreePath)) { + const relative = relativePathInsideRoot(layout.path, worktreePath) + if (relative === null) { continue } return layout.nestWorkspaces