From 6df8997c3cd2e98b5f90427b3dffb2d25f334fab Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 5 Aug 2026 17:11:28 -0700 Subject: [PATCH] fix(file-explorer): sort numbered file names naturally across every listing surface (#11576) * fix(file-explorer): sort numbered file names naturally The File Explorer compared names with bare localeCompare, so numbered files listed 100, 200 before 99. Hoist the numeric collator Source Control file rows already use (#10850) into src/shared and apply it to the local and runtime directory listings, the name-filtered view, and Source Control directory nodes, which were inconsistent with the file rows one line below (#11426). * fix(file-explorer): natural sort on SSH funnels, relay, and pickers Adversarial-review round 1 rework: - Both readDir funnels short-circuited to the SSH filesystem provider before the patched sort, so SSH workspaces kept lexicographic order; re-sort locally after the provider returns (the remote relay may be an older build), and fix the relay's own comparator for relay-native consumers. - sortDirEntries (shared, unit-tested) owns the directories-first + natural-order listing contract used by every funnel. - compareFileNames breaks numeric-collation ties ('2' vs '02') by code units so sibling order stays total instead of readdir order, and pins the collator locale to 'en' so every host produces one order. - The SSH folder browser and runtime server dir picker now match the Explorer they browse into. - Ordering pinned by tests at the relay, source-control tree, and shared helper. * fix(mobile): natural sort in the mobile file explorer Mobile re-sorted host readDir results with bare localeCompare, undoing the host funnel's natural order (round-2 review). Reuse the shared comparator and pin the order in the mobile suite. * fix(file-explorer): natural sort at the renderer choke point and remaining ties Round-3 review: the remote-runtime RPC and paired-web routes return the host's order verbatim, so re-sort in readFileExplorerDirectory where every desktop route converges; pin the SSH funnel with a handler-level test; and route Source Control path compares through compareFileNames so numeric-collation ties share one total order with the Explorer. * docs(file-name-sort): state the real perf baseline in the hoist comment * refactor(source-control): drop the dead collator export; pin the test oracle locale * fix(file-listings): cover remaining natural-sort surfaces --- mobile/src/files/file-tree.test.ts | 12 +++ mobile/src/files/file-tree.ts | 3 +- src/main/ipc/filesystem.test.ts | 24 ++++++ src/main/ipc/filesystem.ts | 12 ++- src/main/ipc/ssh-browse.ts | 10 +-- src/main/runtime/orca-runtime-files.ts | 12 ++- src/main/runtime/orca-runtime.ts | 8 +- src/relay/fs-handler.test.ts | 18 +++-- src/relay/fs-handler.ts | 8 +- .../src/components/quick-open-search.test.ts | 50 ++++++++---- .../src/components/quick-open-search.ts | 78 ++++++++++++------- .../right-sidebar/SourceControl.tsx | 4 +- .../file-explorer-directory-listing.test.ts | 32 ++++++++ .../file-explorer-directory-listing.ts | 6 +- .../file-explorer-name-filter-projection.ts | 3 +- .../source-control-file-filter.test.ts | 21 +++++ .../source-control-file-filter.ts | 10 +++ .../source-control-status-sort.test.ts | 4 +- .../source-control-status-sort.ts | 12 +-- .../right-sidebar/source-control-tree.test.ts | 18 +++++ .../right-sidebar/source-control-tree.ts | 9 ++- .../runtime-server-directory-browser.test.ts | 14 +++- .../runtime-server-directory-browser.ts | 4 +- src/shared/file-name-sort.test.ts | 62 +++++++++++++++ src/shared/file-name-sort.ts | 30 +++++++ .../quick-open-directory-reader.test.ts | 23 ++++++ src/shared/quick-open-directory-reader.ts | 3 +- 27 files changed, 389 insertions(+), 101 deletions(-) create mode 100644 src/renderer/src/components/right-sidebar/file-explorer-directory-listing.test.ts create mode 100644 src/shared/file-name-sort.test.ts create mode 100644 src/shared/file-name-sort.ts diff --git a/mobile/src/files/file-tree.test.ts b/mobile/src/files/file-tree.test.ts index bdd9e9225cb..a11690047b0 100644 --- a/mobile/src/files/file-tree.test.ts +++ b/mobile/src/files/file-tree.test.ts @@ -34,6 +34,18 @@ describe('file-tree', () => { ]) }) + it('orders numbered names naturally, matching the desktop explorer', () => { + const cache: DirectoryCache = { + '': { entries: [entry('100 - b.txt'), entry('9 - c.txt'), entry('99 - a.txt')] } + } + + expect(flattenDirectoryCache(cache, new Set()).map((row) => row.id)).toEqual([ + 'file:9 - c.txt', + 'file:99 - a.txt', + 'file:100 - b.txt' + ]) + }) + it('mirrors desktop default browse exclusions while keeping dotfiles visible', () => { const cache: DirectoryCache = { '': { diff --git a/mobile/src/files/file-tree.ts b/mobile/src/files/file-tree.ts index a9283cfa457..71f26ca155b 100644 --- a/mobile/src/files/file-tree.ts +++ b/mobile/src/files/file-tree.ts @@ -1,5 +1,6 @@ // Pure tree projection for the mobile file explorer. Mobile mirrors desktop // browse semantics by flattening cached files.readDir results as folders open. +import { compareFileNames } from '../../../src/shared/file-name-sort' export type MobileDirEntry = { name: string @@ -118,7 +119,7 @@ function compareDirectoryEntries(a: MobileDirEntry, b: MobileDirEntry): number { if (a.isDirectory !== b.isDirectory) { return a.isDirectory ? -1 : 1 } - return a.name.localeCompare(b.name) + return compareFileNames(a.name, b.name) } export function shouldIncludeMobileFileExplorerEntry(entry: MobileDirEntry): boolean { diff --git a/src/main/ipc/filesystem.test.ts b/src/main/ipc/filesystem.test.ts index d1c2ee312d0..ad68010cbb7 100644 --- a/src/main/ipc/filesystem.test.ts +++ b/src/main/ipc/filesystem.test.ts @@ -366,6 +366,30 @@ describe('registerFilesystemHandlers', () => { lstatMock.mockRejectedValue(Object.assign(new Error('missing'), { code: 'ENOENT' })) }) + it('re-sorts SSH provider listings directories-first in natural order', async () => { + // Why: the remote relay may be an older build that still sorts lexicographically. + getSshFilesystemProviderMock.mockReturnValueOnce({ + readDir: vi.fn().mockResolvedValue([ + { name: '100 - b.txt', isDirectory: false, isSymlink: false }, + { name: '9 - c.txt', isDirectory: false, isSymlink: false }, + { name: '10 - dir', isDirectory: true, isSymlink: false }, + { name: '99 - a.txt', isDirectory: false, isSymlink: false } + ]) + }) + registerFilesystemHandlers(store as never) + + const result = (await handlers.get('fs:readDir')!(null, { + dirPath: '/remote/repo', + connectionId: 'ssh-1' + })) as { name: string }[] + expect(result.map((e) => e.name)).toEqual([ + '10 - dir', + '9 - c.txt', + '99 - a.txt', + '100 - b.txt' + ]) + }) + it('returns an actionable reconnect error when the SSH filesystem provider is unavailable', async () => { registerFilesystemHandlers(store as never) diff --git a/src/main/ipc/filesystem.ts b/src/main/ipc/filesystem.ts index bf4aff54f04..4ca9547b77c 100644 --- a/src/main/ipc/filesystem.ts +++ b/src/main/ipc/filesystem.ts @@ -30,6 +30,7 @@ import type { } from '../../shared/types' import type { GitHistoryOptions, GitHistoryResult } from '../../shared/git-history' import type { SshMutationExpectation } from '../../shared/ssh-types' +import { sortDirEntries } from '../../shared/file-name-sort' import { assertSshMutationExpectation } from '../ssh/ssh-connection-generation' import { buildRgArgs, @@ -540,7 +541,9 @@ export function registerFilesystemHandlers( if (args.connectionId) { throwSite = 'ssh-provider' const provider = requireSshFilesystemProvider(args.connectionId) - return await provider.readDir(args.dirPath) + // Why: re-sort locally — the remote relay may be an older build with + // lexicographic ordering. + return sortDirEntries(await provider.readDir(args.dirPath)) } throwSite = 'authorize' const dirPath = await resolveAuthorizedPath(args.dirPath, store) @@ -555,12 +558,7 @@ export function registerFilesystemHandlers( isSymlink: entry.isSymbolicLink() })) ) - return mapped.sort((a, b) => { - if (a.isDirectory !== b.isDirectory) { - return a.isDirectory ? -1 : 1 - } - return a.name.localeCompare(b.name) - }) + return sortDirEntries(mapped) } catch (error: unknown) { recordCrashBreadcrumb( 'fs_readdir_error', diff --git a/src/main/ipc/ssh-browse.ts b/src/main/ipc/ssh-browse.ts index 2356de614b0..f2a779ee412 100644 --- a/src/main/ipc/ssh-browse.ts +++ b/src/main/ipc/ssh-browse.ts @@ -3,6 +3,7 @@ import type { SshConnectionManager } from '../ssh/ssh-connection-manager' import type { SshExecOptions } from '../ssh/ssh-connection-utils' import { powerShellCommand, powerShellLiteral } from '../ssh/ssh-remote-powershell' import type { FilesystemPathFlavor } from '../../shared/types' +import { sortDirEntries } from '../../shared/file-name-sort' export type RemoteDirEntry = { name: string @@ -220,13 +221,8 @@ async function runBrowseCommand( } } - // Sort: directories first, then alphabetical - entries.sort((a, b) => { - if (a.isDirectory !== b.isDirectory) { - return a.isDirectory ? -1 : 1 - } - return a.name.localeCompare(b.name) - }) + // Sort: directories first, then natural name order (matches the Explorer) + sortDirEntries(entries) resolveOnce({ entries, resolvedPath, pathFlavor }) } diff --git a/src/main/runtime/orca-runtime-files.ts b/src/main/runtime/orca-runtime-files.ts index 5cefa574248..68000f1b297 100644 --- a/src/main/runtime/orca-runtime-files.ts +++ b/src/main/runtime/orca-runtime-files.ts @@ -38,6 +38,7 @@ import { resolveRuntimePath } from '../../shared/cross-platform-path' import { PhysicalExitTracker } from '../../shared/physical-exit-tracker' +import { sortDirEntries } from '../../shared/file-name-sort' import type { RuntimeFileListResult, RuntimeFileOpenResult, @@ -1292,7 +1293,9 @@ export class RuntimeFileCommands { if (!provider) { throw new Error(SSH_FILESYSTEM_PROVIDER_UNAVAILABLE_MESSAGE) } - return provider.readDir(target.path) + // Why: re-sort locally — the remote relay may be an older build with + // lexicographic ordering. + return sortDirEntries(await provider.readDir(target.path)) } const dirPath = await resolveAuthorizedPath(target.path, this.host.requireStore()) @@ -1307,12 +1310,7 @@ export class RuntimeFileCommands { } }) ) - return mapped.sort((a, b) => { - if (a.isDirectory !== b.isDirectory) { - return a.isDirectory ? -1 : 1 - } - return a.name.localeCompare(b.name) - }) + return sortDirEntries(mapped) } async watchFileExplorer( diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index b85bb01b463..750cd59a0b3 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -10,6 +10,7 @@ import { normalizeTerminalTitle } from '../../shared/agent-detection' import { extractOscTitleScanTail } from '../../shared/osc-title-scan-tail' +import { sortDirEntries } from '../../shared/file-name-sort' import { isServerDriveListRequest, listWindowsDrives } from './windows-drive-listing' import { extractLastOsc7Uri, extractOscScanTail } from '../daemon/osc7-uri-extraction' import { parseFileUriPathParts } from '../daemon/osc7-file-uri' @@ -18250,12 +18251,7 @@ export class OrcaRuntimeService { isDirectory: entry.isDirectory(), isSymlink: entry.isSymbolicLink() })) - mapped.sort((a, b) => { - if (a.isDirectory !== b.isDirectory) { - return a.isDirectory ? -1 : 1 - } - return a.name.localeCompare(b.name) - }) + sortDirEntries(mapped) return { resolvedPath: dirPath, entries: mapped, diff --git a/src/relay/fs-handler.test.ts b/src/relay/fs-handler.test.ts index 23dd28c35cb..9dc4812ef98 100644 --- a/src/relay/fs-handler.test.ts +++ b/src/relay/fs-handler.test.ts @@ -164,19 +164,23 @@ describe('FsHandler', () => { await expect(dispatcher.callRequest('fs.tempDir')).resolves.toBe(tmpdir()) }) - it('readDir returns sorted entries with directories first', async () => { + it('readDir returns entries directories-first in natural name order', async () => { mkdirSync(path.join(tmpDir, 'subdir')) - writeFileSync(path.join(tmpDir, 'file.txt'), 'hello') - writeFileSync(path.join(tmpDir, 'aaa.txt'), 'world') + for (const name of ['file.txt', '100 - b.txt', '99 - a.txt', '9 - c.txt']) { + writeFileSync(path.join(tmpDir, name), 'x') + } const result = (await dispatcher.callRequest('fs.readDir', { dirPath: tmpDir })) as { name: string isDirectory: boolean }[] - expect(result[0].name).toBe('subdir') - expect(result[0].isDirectory).toBe(true) - expect(result.find((e) => e.name === 'file.txt')).toBeDefined() - expect(result.find((e) => e.name === 'aaa.txt')).toBeDefined() + expect(result[0]).toMatchObject({ name: 'subdir', isDirectory: true }) + expect(result.slice(1).map((e) => e.name)).toEqual([ + '9 - c.txt', + '99 - a.txt', + '100 - b.txt', + 'file.txt' + ]) }) it('readDir reports symlinked directories as directories', async () => { diff --git a/src/relay/fs-handler.ts b/src/relay/fs-handler.ts index 9c146af1450..d07e2bf397f 100644 --- a/src/relay/fs-handler.ts +++ b/src/relay/fs-handler.ts @@ -5,6 +5,7 @@ import { execFile } from 'node:child_process' import { tmpdir } from 'node:os' import { join } from 'node:path' import type { RelayDispatcher, RequestContext } from './dispatcher' +import { sortDirEntries } from '../shared/file-name-sort' import type { RelayContext } from './context' // Why: RelayContext is accepted in the constructor for protocol back-compat // (see docs/relay-fs-allowlist-removal.md), but no longer consulted on FS ops. @@ -148,12 +149,7 @@ export class FsHandler { isSymlink: entry.isSymbolicLink() })) ) - return mapped.sort((a, b) => { - if (a.isDirectory !== b.isDirectory) { - return a.isDirectory ? -1 : 1 - } - return a.name.localeCompare(b.name) - }) + return sortDirEntries(mapped) } private async readFile(params: Record) { diff --git a/src/renderer/src/components/quick-open-search.test.ts b/src/renderer/src/components/quick-open-search.test.ts index b38f2b1c44a..be15b7ee45c 100644 --- a/src/renderer/src/components/quick-open-search.test.ts +++ b/src/renderer/src/components/quick-open-search.test.ts @@ -9,11 +9,33 @@ import { } from './quick-open-search' describe('quick-open-search', () => { - it('returns the first 50 paths with score 0 for an empty query', () => { - const files = Array.from({ length: 75 }, (_, index) => `src/file-${index}.ts`) + it('orders numbered paths naturally for empty queries and fuzzy-score ties', () => { + const files = prepareQuickOpenFiles([ + 'songs/100 - b.txt', + 'songs/9 - c.txt', + 'songs/99 - a.txt' + ]) + + expect(rankQuickOpenFiles('', files).map((item) => item.path)).toEqual([ + 'songs/9 - c.txt', + 'songs/99 - a.txt', + 'songs/100 - b.txt' + ]) + expect(rankQuickOpenFiles('songs', files).map((item) => item.path)).toEqual([ + 'songs/9 - c.txt', + 'songs/99 - a.txt', + 'songs/100 - b.txt' + ]) + }) + + it('returns the first 50 naturally sorted paths with score 0 for an empty query', () => { + const files = Array.from({ length: 75 }, (_, index) => `src/file-${74 - index}.ts`) expect(rankQuickOpenFiles('', prepareQuickOpenFiles(files))).toEqual( - files.slice(0, QUICK_OPEN_RESULT_LIMIT).map((path) => ({ path, score: 0 })) + Array.from({ length: QUICK_OPEN_RESULT_LIMIT }, (_, index) => ({ + path: `src/file-${index}.ts`, + score: 0 + })) ) }) @@ -35,8 +57,8 @@ describe('quick-open-search', () => { ).toEqual(['src/components/Button.tsx', 'button-area/deep/path/file.tsx']) }) - it('keeps first-seen order for tie-heavy results at the limit boundary', () => { - const files = Array.from({ length: 10 }, (_, index) => `src/path-${index}.bin`) + it('uses natural order for tie-heavy results at the limit boundary', () => { + const files = Array.from({ length: 10 }, (_, index) => `src/path-${9 - index}.bin`) expect(rankQuickOpenFiles('s', prepareQuickOpenFiles(files), 4)).toEqual([ { path: 'src/path-0.bin', score: 0 }, @@ -85,9 +107,9 @@ describe('quick-open-search', () => { it('indexes normalized relative paths without changing path semantics', () => { const files = [ 'src/renderer/src/components/QuickOpen.tsx', + 'legacy\\provider\\raw-path.ts', 'packages/windows-origin/src/App.tsx', - 'single-file.ts', - 'legacy\\provider\\raw-path.ts' + 'single-file.ts' ] expect(prepareQuickOpenFiles(files)).toEqual([ @@ -97,22 +119,22 @@ describe('quick-open-search', () => { lowerFilename: 'quickopen.tsx', inputIndex: 0 }, + { + path: 'legacy\\provider\\raw-path.ts', + lowerPath: 'legacy/provider/raw-path.ts', + lowerFilename: 'raw-path.ts', + inputIndex: 1 + }, { path: 'packages/windows-origin/src/App.tsx', lowerPath: 'packages/windows-origin/src/app.tsx', lowerFilename: 'app.tsx', - inputIndex: 1 + inputIndex: 2 }, { path: 'single-file.ts', lowerPath: 'single-file.ts', lowerFilename: 'single-file.ts', - inputIndex: 2 - }, - { - path: 'legacy\\provider\\raw-path.ts', - lowerPath: 'legacy/provider/raw-path.ts', - lowerFilename: 'raw-path.ts', inputIndex: 3 } ]) diff --git a/src/renderer/src/components/quick-open-search.ts b/src/renderer/src/components/quick-open-search.ts index 5413c151193..db13042c8fa 100644 --- a/src/renderer/src/components/quick-open-search.ts +++ b/src/renderer/src/components/quick-open-search.ts @@ -1,4 +1,5 @@ import { isClipboardTextByteLengthOverLimit } from '../../../shared/clipboard-text' +import { compareFileNames } from '../../../shared/file-name-sort' export const QUICK_OPEN_RESULT_LIMIT = 50 export const QUICK_OPEN_QUERY_MAX_BYTES = 2 * 1024 @@ -51,7 +52,11 @@ export function rankQuickOpenFiles( // still naturally type backslashes in path queries. const normalizedQuery = query.trim().replace(/\\/g, '/').toLowerCase() if (!normalizedQuery) { - return files.slice(0, limit).map((file) => ({ path: file.path, score: 0 })) + const results: QuickOpenRankedResult[] = [] + for (const file of files) { + retainTopResult(results, { path: file.path, score: 0, inputIndex: file.inputIndex }, limit) + } + return finalizeResults(results) } const results: QuickOpenRankedResult[] = [] @@ -61,10 +66,10 @@ export function rankQuickOpenFiles( continue } - insertTopResult(results, { path: file.path, score, inputIndex: file.inputIndex }, limit) + retainTopResult(results, { path: file.path, score, inputIndex: file.inputIndex }, limit) } - return results.map(({ path, score }) => ({ path, score })) + return finalizeResults(results) } function fuzzyMatchIndexedFile(query: string, file: QuickOpenIndexedFile): number { @@ -106,42 +111,63 @@ type QuickOpenRankedResult = QuickOpenSearchResult & { inputIndex: number } -function insertTopResult( - results: QuickOpenRankedResult[], +function retainTopResult( + heap: QuickOpenRankedResult[], candidate: QuickOpenRankedResult, limit: number ): void { - const worst = results.at(-1) - if (results.length === limit && worst && compareRankedResult(candidate, worst) >= 0) { + if (heap.length === limit && compareRankedResult(candidate, heap[0]) >= 0) { return } - const insertAt = findInsertionIndex(results, candidate) - results.splice(insertAt, 0, candidate) - if (results.length > limit) { - results.pop() + if (heap.length < limit) { + heap.push(candidate) + siftResultUp(heap, heap.length - 1) + return + } + + heap[0] = candidate + siftResultDown(heap) +} + +function siftResultUp(heap: QuickOpenRankedResult[], startIndex: number): void { + let index = startIndex + while (index > 0) { + const parentIndex = Math.floor((index - 1) / 2) + if (compareRankedResult(heap[index], heap[parentIndex]) <= 0) { + return + } + ;[heap[index], heap[parentIndex]] = [heap[parentIndex], heap[index]] + index = parentIndex } } -function findInsertionIndex( - results: readonly QuickOpenRankedResult[], - candidate: QuickOpenRankedResult -): number { - let low = 0 - let high = results.length - - while (low < high) { - const mid = Math.floor((low + high) / 2) - if (compareRankedResult(candidate, results[mid]) < 0) { - high = mid - } else { - low = mid + 1 +function siftResultDown(heap: QuickOpenRankedResult[]): void { + let index = 0 + while (true) { + const leftIndex = index * 2 + 1 + if (leftIndex >= heap.length) { + return } + const rightIndex = leftIndex + 1 + const worseChildIndex = + rightIndex < heap.length && compareRankedResult(heap[rightIndex], heap[leftIndex]) > 0 + ? rightIndex + : leftIndex + if (compareRankedResult(heap[worseChildIndex], heap[index]) <= 0) { + return + } + ;[heap[index], heap[worseChildIndex]] = [heap[worseChildIndex], heap[index]] + index = worseChildIndex } +} - return low +function finalizeResults(results: QuickOpenRankedResult[]): QuickOpenSearchResult[] { + return results + .sort(compareRankedResult) + .map(({ path, score }): QuickOpenSearchResult => ({ path, score })) } function compareRankedResult(a: QuickOpenRankedResult, b: QuickOpenRankedResult): number { - return a.score - b.score || a.inputIndex - b.inputIndex + return a.score - b.score || compareFileNames(a.path, b.path) || a.inputIndex - b.inputIndex } diff --git a/src/renderer/src/components/right-sidebar/SourceControl.tsx b/src/renderer/src/components/right-sidebar/SourceControl.tsx index 9b223cd6a4f..13c94afebe4 100644 --- a/src/renderer/src/components/right-sidebar/SourceControl.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControl.tsx @@ -125,8 +125,8 @@ import { buildActiveOpenRowKeys } from './source-control-active-open-file-keys' import { + filterAndSortSourceControlPathEntries, filterSourceControlGroupedPathEntries, - filterSourceControlPathEntries, getSourceControlFileFilterState } from './source-control-file-filter' import { getCommitMessageTextareaRows } from './source-control-commit-message-rows' @@ -1822,7 +1822,7 @@ function SourceControlInner(): React.JSX.Element { ) const filteredBranchEntries = useMemo( - () => filterSourceControlPathEntries(branchEntries, fileFilterState), + () => filterAndSortSourceControlPathEntries(branchEntries, fileFilterState), [branchEntries, fileFilterState] ) diff --git a/src/renderer/src/components/right-sidebar/file-explorer-directory-listing.test.ts b/src/renderer/src/components/right-sidebar/file-explorer-directory-listing.test.ts new file mode 100644 index 00000000000..a0e40f2ef12 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/file-explorer-directory-listing.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, it, vi } from 'vitest' + +const readRuntimeDirectory = vi.fn() +vi.mock('@/runtime/runtime-file-client', () => ({ + readRuntimeDirectory: (...args: unknown[]) => readRuntimeDirectory(...args) +})) +vi.mock('./file-explorer-operation-owner', () => ({ + getFileExplorerOperationOwner: () => ({ kind: 'local' }), + getFileExplorerOperationRoute: () => ({ settings: null, connectionId: null }), + getFileExplorerOwnerUnresolvedMessage: () => 'unresolved' +})) + +import { readFileExplorerDirectory } from './file-explorer-directory-listing' + +describe('readFileExplorerDirectory', () => { + it('re-sorts backend order — remote-runtime and paired-web routes return the host order verbatim', async () => { + readRuntimeDirectory.mockResolvedValueOnce([ + { name: '100 - b.txt', isDirectory: false, isSymlink: false }, + { name: '9 - c.txt', isDirectory: false, isSymlink: false }, + { name: '10 - dir', isDirectory: true, isSymlink: false }, + { name: '99 - a.txt', isDirectory: false, isSymlink: false } + ]) + + const { entries } = await readFileExplorerDirectory('wt-1', '/w', '/w/dir') + expect(entries.map((e) => e.name)).toEqual([ + '10 - dir', + '9 - c.txt', + '99 - a.txt', + '100 - b.txt' + ]) + }) +}) diff --git a/src/renderer/src/components/right-sidebar/file-explorer-directory-listing.ts b/src/renderer/src/components/right-sidebar/file-explorer-directory-listing.ts index 13742226c9a..eb8f7142846 100644 --- a/src/renderer/src/components/right-sidebar/file-explorer-directory-listing.ts +++ b/src/renderer/src/components/right-sidebar/file-explorer-directory-listing.ts @@ -1,5 +1,6 @@ import { joinPath, normalizeRelativePath } from '@/lib/path' import type { DirEntry } from '../../../../shared/types' +import { sortDirEntries } from '../../../../shared/file-name-sort' import { readRuntimeDirectory } from '@/runtime/runtime-file-client' import type { FileExplorerOperationOwner, TreeNode } from './file-explorer-types' import { shouldIncludeFileExplorerEntry } from './file-explorer-entries' @@ -56,5 +57,8 @@ export async function readFileExplorerDirectory( }, dirPath ) - return { entries, operationOwner } + // Why: remote-runtime and paired-web routes return the host's order verbatim, + // and an older host may still sort lexicographically; re-sorting an already + // sorted local listing is near-free (adaptive sort). + return { entries: sortDirEntries(entries), operationOwner } } diff --git a/src/renderer/src/components/right-sidebar/file-explorer-name-filter-projection.ts b/src/renderer/src/components/right-sidebar/file-explorer-name-filter-projection.ts index ba057328924..9070472ed0c 100644 --- a/src/renderer/src/components/right-sidebar/file-explorer-name-filter-projection.ts +++ b/src/renderer/src/components/right-sidebar/file-explorer-name-filter-projection.ts @@ -1,5 +1,6 @@ import { joinPath, normalizeRelativePath } from '@/lib/path' import { isClipboardTextByteLengthOverLimit } from '../../../../shared/clipboard-text' +import { compareFileNames } from '../../../../shared/file-name-sort' import type { FileExplorerOperationOwner, TreeNode } from './file-explorer-types' import { createFileExplorerRowProjectionFromParts, @@ -232,7 +233,7 @@ function appendNameFilteredEntries( if (a.node.isDirectory !== b.node.isDirectory) { return a.node.isDirectory ? -1 : 1 } - return a.node.name.localeCompare(b.node.name) + return compareFileNames(a.node.name, b.node.name) }) for (const entry of sortedEntries) { visibleFlatRows.push(entry.node) diff --git a/src/renderer/src/components/right-sidebar/source-control-file-filter.test.ts b/src/renderer/src/components/right-sidebar/source-control-file-filter.test.ts index 33ef043e7af..927f0c45328 100644 --- a/src/renderer/src/components/right-sidebar/source-control-file-filter.test.ts +++ b/src/renderer/src/components/right-sidebar/source-control-file-filter.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest' import { SOURCE_CONTROL_FILE_FILTER_QUERY_MAX_BYTES, + filterAndSortSourceControlPathEntries, filterSourceControlGroupedPathEntries, filterSourceControlPathEntries, getSourceControlFileFilterState, @@ -9,6 +10,26 @@ import { } from './source-control-file-filter' describe('source-control-file-filter', () => { + it('naturally orders committed branch rows without mutating store input', () => { + const entries = [ + { path: 'migrations/100.sql' }, + { path: 'migrations/9.sql' }, + { path: 'migrations/99.sql' } + ] + + expect( + filterAndSortSourceControlPathEntries(entries, { + normalizedFilter: '', + tooLarge: false + }).map((entry) => entry.path) + ).toEqual(['migrations/9.sql', 'migrations/99.sql', 'migrations/100.sql']) + expect(entries.map((entry) => entry.path)).toEqual([ + 'migrations/100.sql', + 'migrations/9.sql', + 'migrations/99.sql' + ]) + }) + it('normalizes bounded queries and filters entries by path', () => { const filter = getSourceControlFileFilterState(' SRC/button ') diff --git a/src/renderer/src/components/right-sidebar/source-control-file-filter.ts b/src/renderer/src/components/right-sidebar/source-control-file-filter.ts index fe84061d5bc..4a4e765c889 100644 --- a/src/renderer/src/components/right-sidebar/source-control-file-filter.ts +++ b/src/renderer/src/components/right-sidebar/source-control-file-filter.ts @@ -1,4 +1,5 @@ import { isClipboardTextByteLengthOverLimit } from '../../../../shared/clipboard-text' +import { compareFileNames } from '../../../../shared/file-name-sort' export const SOURCE_CONTROL_FILE_FILTER_QUERY_MAX_BYTES = 2 * 1024 @@ -48,6 +49,15 @@ export function filterSourceControlPathEntries return entries.filter((entry) => entry.path.toLowerCase().includes(filter.normalizedFilter)) } +export function filterAndSortSourceControlPathEntries( + entries: T[], + filter: SourceControlFileFilterState +): T[] { + return [...filterSourceControlPathEntries(entries, filter)].sort((a, b) => + compareFileNames(a.path, b.path) + ) +} + export function filterSourceControlGroupedPathEntries( grouped: SourceControlGroupedPathEntries, filter: SourceControlFileFilterState diff --git a/src/renderer/src/components/right-sidebar/source-control-status-sort.test.ts b/src/renderer/src/components/right-sidebar/source-control-status-sort.test.ts index 506e49c2949..595ba62fe34 100644 --- a/src/renderer/src/components/right-sidebar/source-control-status-sort.test.ts +++ b/src/renderer/src/components/right-sidebar/source-control-status-sort.test.ts @@ -22,7 +22,9 @@ function referenceCompare(a: GitStatusEntry, b: GitStatusEntry): number { } return 2 } - return rank(a) - rank(b) || a.path.localeCompare(b.path, undefined, { numeric: true }) + // 'en' pinned to match the shared collator — an undefined locale would make + // this oracle environment-dependent (sv/cs/da collate differently). + return rank(a) - rank(b) || a.path.localeCompare(b.path, 'en', { numeric: true }) } describe('compareGitStatusEntries', () => { diff --git a/src/renderer/src/components/right-sidebar/source-control-status-sort.ts b/src/renderer/src/components/right-sidebar/source-control-status-sort.ts index f5ee065bc70..ccedb093ffb 100644 --- a/src/renderer/src/components/right-sidebar/source-control-status-sort.ts +++ b/src/renderer/src/components/right-sidebar/source-control-status-sort.ts @@ -1,14 +1,10 @@ import type { GitStatusEntry } from '../../../../shared/types' - -// Why hoisted: localeCompare with an options object resolves a fresh ICU collator -// on every comparison, so a changed-file sort paid for one per O(n log n) step. -export const sourceControlPathCollator = new Intl.Collator(undefined, { numeric: true }) +import { compareFileNames } from '../../../../shared/file-name-sort' export function compareGitStatusEntries(a: GitStatusEntry, b: GitStatusEntry): number { - return ( - getConflictSortRank(a) - getConflictSortRank(b) || - sourceControlPathCollator.compare(a.path, b.path) - ) + // Why: compareFileNames (not the raw collator) so numeric-collation ties + // ("2" vs "02") stay a total order shared with the File Explorer. + return getConflictSortRank(a) - getConflictSortRank(b) || compareFileNames(a.path, b.path) } function getConflictSortRank(entry: GitStatusEntry): number { diff --git a/src/renderer/src/components/right-sidebar/source-control-tree.test.ts b/src/renderer/src/components/right-sidebar/source-control-tree.test.ts index f585c1c35ef..421b5f3cd06 100644 --- a/src/renderer/src/components/right-sidebar/source-control-tree.test.ts +++ b/src/renderer/src/components/right-sidebar/source-control-tree.test.ts @@ -39,6 +39,24 @@ describe('buildSourceControlTree', () => { ]) }) + it('orders numbered sibling directories naturally, matching the file rows', () => { + const tree = buildSourceControlTree('unstaged', [ + entry({ path: 'migrations/100 - b/a.sql' }), + entry({ path: 'migrations/9 - a/a.sql' }), + entry({ path: 'migrations/99 - c/a.sql' }) + ]) + + expect(labels(flattenSourceControlTree(tree, new Set()))).toEqual([ + 'directory:migrations', + 'directory:migrations/9 - a', + 'file:migrations/9 - a/a.sql', + 'directory:migrations/99 - c', + 'file:migrations/99 - c/a.sql', + 'directory:migrations/100 - b', + 'file:migrations/100 - b/a.sql' + ]) + }) + it('keeps conflict rows ahead of normal rows within a directory', () => { const tree = buildGitStatusSourceControlTree('unstaged', [ entry({ path: 'src/a.ts' }), diff --git a/src/renderer/src/components/right-sidebar/source-control-tree.ts b/src/renderer/src/components/right-sidebar/source-control-tree.ts index 6bf94893b07..2ecca32c0bd 100644 --- a/src/renderer/src/components/right-sidebar/source-control-tree.ts +++ b/src/renderer/src/components/right-sidebar/source-control-tree.ts @@ -1,7 +1,8 @@ import { normalizeRelativePath } from '@/lib/path' import type { GitStatusEntry, GitStagingArea } from '../../../../shared/types' +import { compareFileNames } from '../../../../shared/file-name-sort' import { splitPathSegments } from './path-tree' -import { compareGitStatusEntries, sourceControlPathCollator } from './source-control-status-sort' +import { compareGitStatusEntries } from './source-control-status-sort' export type SourceControlTreeArea = Extract // Why: committed branch rows share the same path tree but do not carry @@ -51,7 +52,7 @@ type MutableDirectoryNode( @@ -90,7 +91,9 @@ function finalizeDirectoryNode a.name.localeCompare(b.name)) + // Why: keep directory ordering consistent with the numeric path collator used + // for the file rows one line below. + directories.sort((a, b) => compareFileNames(a.name, b.name)) files.sort((a, b) => compareEntries(a.entry, b.entry)) const fileCount = files.length + directories.reduce((count, directory) => count + directory.fileCount, 0) diff --git a/src/renderer/src/runtime/runtime-server-directory-browser.test.ts b/src/renderer/src/runtime/runtime-server-directory-browser.test.ts index 07740c37726..6c7579bc7a2 100644 --- a/src/renderer/src/runtime/runtime-server-directory-browser.test.ts +++ b/src/renderer/src/runtime/runtime-server-directory-browser.test.ts @@ -29,7 +29,12 @@ beforeEach(() => { result: { resolvedPath: '/home/me', pathFlavor: 'posix', - entries: [{ name: 'repo', isDirectory: true, isSymlink: false }] + entries: [ + { name: '100 - file.txt', isDirectory: false, isSymlink: false }, + { name: '9 - file.txt', isDirectory: false, isSymlink: false }, + { name: '10 - repo', isDirectory: true, isSymlink: false }, + { name: '99 - file.txt', isDirectory: false, isSymlink: false } + ] }, _meta: { runtimeId: 'remote-runtime' } }) @@ -48,7 +53,12 @@ describe('runtime server directory browser', () => { await expect(browseRuntimeServerDirectory('env-1', '~')).resolves.toEqual({ resolvedPath: '/home/me', pathFlavor: 'posix', - entries: [{ name: 'repo', isDirectory: true, isSymlink: false }] + entries: [ + { name: '10 - repo', isDirectory: true, isSymlink: false }, + { name: '9 - file.txt', isDirectory: false, isSymlink: false }, + { name: '99 - file.txt', isDirectory: false, isSymlink: false }, + { name: '100 - file.txt', isDirectory: false, isSymlink: false } + ] }) expect(runtimeEnvironmentCall).toHaveBeenLastCalledWith({ diff --git a/src/renderer/src/runtime/runtime-server-directory-browser.ts b/src/renderer/src/runtime/runtime-server-directory-browser.ts index 5d7c0a13ade..b28e7755f16 100644 --- a/src/renderer/src/runtime/runtime-server-directory-browser.ts +++ b/src/renderer/src/runtime/runtime-server-directory-browser.ts @@ -1,4 +1,5 @@ import type { DirEntry, FilesystemPathFlavor } from '../../../shared/types' +import { sortDirEntries } from '../../../shared/file-name-sort' import { callRuntimeRpc } from './runtime-rpc-client' export type RuntimeServerDirectoryListing = { @@ -11,10 +12,11 @@ export async function browseRuntimeServerDirectory( environmentId: string, path: string ): Promise { - return callRuntimeRpc( + const listing = await callRuntimeRpc( { kind: 'environment', environmentId }, 'files.browseServerDir', { path }, { timeoutMs: 15_000 } ) + return { ...listing, entries: sortDirEntries(listing.entries) } } diff --git a/src/shared/file-name-sort.test.ts b/src/shared/file-name-sort.test.ts new file mode 100644 index 00000000000..b9bd1c52473 --- /dev/null +++ b/src/shared/file-name-sort.test.ts @@ -0,0 +1,62 @@ +import { describe, expect, it } from 'vitest' +import { compareFileNames, sortDirEntries } from './file-name-sort' + +describe('compareFileNames', () => { + it('sorts numeric name segments naturally instead of lexicographically', () => { + const names = [ + '1 - item.txt', + '100 - item.txt', + '2 - item.txt', + '200 - item.txt', + '409 - item.txt', + '41 - item.txt', + '410 - item.txt', + '9 - item.txt', + '99 - item.txt' + ] + expect([...names].sort(compareFileNames)).toEqual([ + '1 - item.txt', + '2 - item.txt', + '9 - item.txt', + '41 - item.txt', + '99 - item.txt', + '100 - item.txt', + '200 - item.txt', + '409 - item.txt', + '410 - item.txt' + ]) + }) + + it('sorts embedded numbers naturally within alphabetical order', () => { + expect(['b1.md', 'a10.md', 'a2.md', 'a1.md'].sort(compareFileNames)).toEqual([ + 'a1.md', + 'a2.md', + 'a10.md', + 'b1.md' + ]) + }) + + it('breaks numeric-collation ties deterministically instead of readdir order', () => { + // numeric collation ties "2" with "02"; the code-unit fallback keeps a total order. + expect(['2.txt', '02.txt'].sort(compareFileNames)).toEqual(['02.txt', '2.txt']) + expect(['02.txt', '2.txt'].sort(compareFileNames)).toEqual(['02.txt', '2.txt']) + expect(compareFileNames('a.txt', 'a.txt')).toBe(0) + }) +}) + +describe('sortDirEntries', () => { + it('keeps directories first, each group in natural order', () => { + const entries = [ + { name: '10 - notes', isDirectory: false }, + { name: '2 - src', isDirectory: true }, + { name: '9 - docs.txt', isDirectory: false }, + { name: '10 - assets', isDirectory: true } + ] + expect(sortDirEntries(entries).map((e) => e.name)).toEqual([ + '2 - src', + '10 - assets', + '9 - docs.txt', + '10 - notes' + ]) + }) +}) diff --git a/src/shared/file-name-sort.ts b/src/shared/file-name-sort.ts new file mode 100644 index 00000000000..0d2de903fd1 --- /dev/null +++ b/src/shared/file-name-sort.ts @@ -0,0 +1,30 @@ +// Why hoisted: localeCompare with an options object resolves a fresh ICU collator +// on every comparison (~14x this form). Numeric collation itself costs ~5x over +// bare localeCompare — accepted for the feature; don't "optimize" the hoist away. +// numeric: true orders "99 - a" before "100 - b", matching Finder/Explorer. +// Locale pinned to 'en' so local main, renderer, relay, and remote runtime hosts +// all produce one order regardless of each process's LANG (precedent: +// skill-freshness pins 'en' for canonical ordering). +export const fileNameCollator = new Intl.Collator('en', { numeric: true }) + +export function compareFileNames(a: string, b: string): number { + const primary = fileNameCollator.compare(a, b) + if (primary !== 0) { + return primary + } + // Why: numeric collation ties distinct names ("2" vs "02"); fall back to code + // units so sibling order stays a total order instead of readdir order. + return a < b ? -1 : a > b ? 1 : 0 +} + +/** Directories-first, then natural name order — the File Explorer listing contract. */ +export function sortDirEntries( + entries: T[] +): T[] { + return entries.sort((a, b) => { + if (a.isDirectory !== b.isDirectory) { + return a.isDirectory ? -1 : 1 + } + return compareFileNames(a.name, b.name) + }) +} diff --git a/src/shared/quick-open-directory-reader.test.ts b/src/shared/quick-open-directory-reader.test.ts index fcd11985523..33f9cb46a9b 100644 --- a/src/shared/quick-open-directory-reader.test.ts +++ b/src/shared/quick-open-directory-reader.test.ts @@ -22,6 +22,29 @@ beforeEach(() => { }) describe('quick-open streaming directory reader', () => { + it('orders numbered entries naturally before the recursive walk', async () => { + opendirMock.mockResolvedValue({ + async *[Symbol.asyncIterator]() { + for (const name of ['100 - notes', '9 - notes', '99 - notes']) { + yield { + name, + isDirectory: () => true, + isFile: () => false, + isSymbolicLink: () => false + } + } + } + }) + + const entries = await readQuickOpenDirectoryEntries({ + absPath: '/numbered', + allowSymlinkedRoot: false, + budget: createQuickOpenReaddirBudget() + }) + + expect(entries.map((entry) => entry.name)).toEqual(['9 - notes', '99 - notes', '100 - notes']) + }) + it('stops a huge directory one entry beyond the exact cap and closes its iterator', async () => { let produced = 0 let closed = false diff --git a/src/shared/quick-open-directory-reader.ts b/src/shared/quick-open-directory-reader.ts index cc53d8d6438..68a6dd66acf 100644 --- a/src/shared/quick-open-directory-reader.ts +++ b/src/shared/quick-open-directory-reader.ts @@ -1,4 +1,5 @@ import { lstat, opendir } from 'node:fs/promises' +import { compareFileNames } from './file-name-sort' import { isFileListingCancellation, throwIfFileListingCancelled } from './file-listing-cancellation' import { isQuickOpenReadableDirectory } from './quick-open-directory-validation' import { @@ -46,7 +47,7 @@ export async function readQuickOpenDirectoryEntries(opts: { : 'other' }) } - entries.sort((left, right) => (left.name < right.name ? -1 : left.name > right.name ? 1 : 0)) + entries.sort((left, right) => compareFileNames(left.name, right.name)) // Why: discard buffered names if the path became a symlink while its // directory handle was open; descendants must never escape the root.