Skip to content

Commit ab481ea

Browse files
committed
fix: remove alphabetical sorting that breaks version-numbered file sequences
- Remove problematic alphabetical sort in formatAndCombineResults function - This allows formatFilesList to handle sorting with numeric awareness - Fixes issue where version files like v3.25.1.mdx were excluded due to alphabetical sorting placing them beyond the 200-file limit - Add comprehensive tests for version-numbered file sorting scenarios Fixes #6515
1 parent 5041880 commit ab481ea

File tree

2 files changed

+130
-9
lines changed

2 files changed

+130
-9
lines changed

src/services/glob/__tests__/list-files.spec.ts

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -559,3 +559,131 @@ describe("buildRecursiveArgs edge cases", () => {
559559
expect(args).not.toContain("--no-ignore")
560560
})
561561
})
562+
563+
describe("version-numbered file sorting", () => {
564+
beforeEach(() => {
565+
vi.clearAllMocks()
566+
})
567+
568+
it("should properly sort version-numbered files with numeric awareness", async () => {
569+
const mockSpawn = vi.mocked(childProcess.spawn)
570+
const mockProcess = {
571+
stdout: {
572+
on: vi.fn((event, callback) => {
573+
if (event === "data") {
574+
// Simulate version-numbered files that would be problematic with alphabetical sorting
575+
// These files would be sorted incorrectly with basic alphabetical sort:
576+
// v3.25.1.mdx would come after v3.25.mdx but before v3.3.mdx
577+
const versionFiles =
578+
[
579+
"v3.24.mdx",
580+
"v3.25.mdx",
581+
"v3.25.1.mdx",
582+
"v3.25.2.mdx",
583+
"v3.25.3.mdx",
584+
"v3.25.4.mdx",
585+
"v3.3.mdx",
586+
"v3.30.mdx",
587+
].join("\n") + "\n"
588+
setTimeout(() => callback(versionFiles), 10)
589+
}
590+
}),
591+
},
592+
stderr: {
593+
on: vi.fn(),
594+
},
595+
on: vi.fn((event, callback) => {
596+
if (event === "close") {
597+
setTimeout(() => callback(0), 20)
598+
}
599+
}),
600+
kill: vi.fn(),
601+
}
602+
mockSpawn.mockReturnValue(mockProcess as any)
603+
604+
// Mock fs operations
605+
vi.mocked(fs.promises.access).mockRejectedValue(new Error("File not found"))
606+
vi.mocked(fs.promises.readdir).mockResolvedValue([])
607+
608+
// Call listFiles with a limit that would cut off some version files with alphabetical sorting
609+
const [results, limitReached] = await listFiles("/test/docs", false, 200)
610+
611+
// The results should be passed through without the problematic alphabetical sort
612+
// The actual sorting will be handled by formatFilesList with numeric awareness
613+
expect(results.length).toBeGreaterThan(0)
614+
expect(limitReached).toBe(false)
615+
616+
// Verify that all version files are included in the raw results
617+
// (the numeric sorting will happen later in formatFilesList)
618+
const versionFiles = results.filter((file) => file.includes("v3.25"))
619+
expect(versionFiles.length).toBeGreaterThan(0)
620+
621+
// The key fix: formatAndCombineResults should NOT apply alphabetical sorting
622+
// that would misplace version-numbered files
623+
const hasV3_25_1 = results.some((file) => file.includes("v3.25.1.mdx"))
624+
const hasV3_25_2 = results.some((file) => file.includes("v3.25.2.mdx"))
625+
const hasV3_25_3 = results.some((file) => file.includes("v3.25.3.mdx"))
626+
627+
expect(hasV3_25_1).toBe(true)
628+
expect(hasV3_25_2).toBe(true)
629+
expect(hasV3_25_3).toBe(true)
630+
})
631+
632+
it("should not apply alphabetical sorting that breaks version sequences", async () => {
633+
const mockSpawn = vi.mocked(childProcess.spawn)
634+
635+
// Create a scenario with exactly 200+ files where version files would be cut off
636+
const files: string[] = []
637+
638+
// Add 195 regular files that come alphabetically before version files
639+
for (let i = 1; i <= 195; i++) {
640+
files.push(`file${i.toString().padStart(3, "0")}.txt`)
641+
}
642+
643+
// Add version files that would be problematic with alphabetical sorting
644+
files.push("v3.25.mdx") // Position ~196
645+
files.push("v3.25.1.mdx") // Would be position ~203 with alphabetical sort
646+
files.push("v3.25.2.mdx") // Would be position ~204 with alphabetical sort
647+
files.push("v3.25.3.mdx") // Would be position ~205 with alphabetical sort
648+
files.push("v3.25.4.mdx") // Would be position ~199 with alphabetical sort
649+
files.push("v3.3.mdx") // Would be position ~206 with alphabetical sort
650+
651+
const mockProcess = {
652+
stdout: {
653+
on: vi.fn((event, callback) => {
654+
if (event === "data") {
655+
setTimeout(() => callback(files.join("\n") + "\n"), 10)
656+
}
657+
}),
658+
},
659+
stderr: {
660+
on: vi.fn(),
661+
},
662+
on: vi.fn((event, callback) => {
663+
if (event === "close") {
664+
setTimeout(() => callback(0), 20)
665+
}
666+
}),
667+
kill: vi.fn(),
668+
}
669+
mockSpawn.mockReturnValue(mockProcess as any)
670+
671+
// Mock fs operations
672+
vi.mocked(fs.promises.access).mockRejectedValue(new Error("File not found"))
673+
vi.mocked(fs.promises.readdir).mockResolvedValue([])
674+
675+
// Call with 200 file limit - this would cut off version files with alphabetical sorting
676+
const [results, limitReached] = await listFiles("/test/docs", false, 200)
677+
678+
expect(limitReached).toBe(true)
679+
expect(results.length).toBe(200)
680+
681+
// With the fix (no alphabetical sorting in formatAndCombineResults),
682+
// the version files should be preserved in their original order from ripgrep
683+
// and the 200-file limit should be applied to the unsorted list
684+
const versionFileCount = results.filter((file) => file.includes("v3.25")).length
685+
686+
// All version files should be included since they appear early in the ripgrep output
687+
expect(versionFileCount).toBeGreaterThan(0)
688+
})
689+
})

src/services/glob/list-files.ts

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -601,15 +601,8 @@ function formatAndCombineResults(files: string[], directories: string[], limit:
601601
const uniquePathsSet = new Set(allPaths)
602602
const uniquePaths = Array.from(uniquePathsSet)
603603

604-
// Sort to ensure directories come first, followed by files
605-
uniquePaths.sort((a: string, b: string) => {
606-
const aIsDir = a.endsWith("/")
607-
const bIsDir = b.endsWith("/")
608-
609-
if (aIsDir && !bIsDir) return -1
610-
if (!aIsDir && bIsDir) return 1
611-
return a.localeCompare(b)
612-
})
604+
// Note: Sorting is handled by formatFilesList with numeric-aware algorithm
605+
// to properly handle version-numbered files (e.g., v3.25.1, v3.25.2, etc.)
613606

614607
const trimmedPaths = uniquePaths.slice(0, limit)
615608
return [trimmedPaths, trimmedPaths.length >= limit]

0 commit comments

Comments
 (0)