Skip to content

Commit 337cb28

Browse files
MnehmosMnehmos
committed
#4009 per KJ7LNW's suggestions implemented changes to
1 parent e3f9f12 commit 337cb28

File tree

6 files changed

+537
-116
lines changed

6 files changed

+537
-116
lines changed

‎src/core/services/__tests__/fileReadCacheService.spec.ts‎

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
import { vi, describe, it, expect, beforeEach } from "vitest"
2+
import {
3+
processAndFilterReadRequest,
4+
subtractRange,
5+
subtractRanges,
6+
ConversationMessage,
7+
} from "../fileReadCacheService"
8+
import { stat } from "fs/promises"
9+
10+
vi.mock("fs/promises", () => ({
11+
stat: vi.fn(),
12+
}))
13+
14+
const mockedStat = vi.mocked(stat)
15+
16+
describe("fileReadCacheService", () => {
17+
describe("subtractRange", () => {
18+
it("should return the original range if there is no overlap", () => {
19+
const original = { start: 1, end: 10 }
20+
const toRemove = { start: 11, end: 20 }
21+
expect(subtractRange(original, toRemove)).toEqual([original])
22+
})
23+
24+
it("should return an empty array if the range is completely removed", () => {
25+
const original = { start: 1, end: 10 }
26+
const toRemove = { start: 1, end: 10 }
27+
expect(subtractRange(original, toRemove)).toEqual([])
28+
})
29+
30+
it("should subtract from the beginning", () => {
31+
const original = { start: 1, end: 10 }
32+
const toRemove = { start: 1, end: 5 }
33+
expect(subtractRange(original, toRemove)).toEqual([{ start: 6, end: 10 }])
34+
})
35+
36+
it("should subtract from the end", () => {
37+
const original = { start: 1, end: 10 }
38+
const toRemove = { start: 6, end: 10 }
39+
expect(subtractRange(original, toRemove)).toEqual([{ start: 1, end: 5 }])
40+
})
41+
42+
it("should subtract from the middle, creating two new ranges", () => {
43+
const original = { start: 1, end: 10 }
44+
const toRemove = { start: 4, end: 6 }
45+
expect(subtractRange(original, toRemove)).toEqual([
46+
{ start: 1, end: 3 },
47+
{ start: 7, end: 10 },
48+
])
49+
})
50+
})
51+
52+
describe("subtractRanges", () => {
53+
it("should subtract multiple ranges from a single original range", () => {
54+
const originals = [{ start: 1, end: 20 }]
55+
const toRemoves = [
56+
{ start: 1, end: 5 },
57+
{ start: 15, end: 20 },
58+
]
59+
expect(subtractRanges(originals, toRemoves)).toEqual([{ start: 6, end: 14 }])
60+
})
61+
})
62+
63+
describe("processAndFilterReadRequest", () => {
64+
const MOCK_FILE_PATH = "/test/file.txt"
65+
const CURRENT_MTIME = 1000
66+
67+
beforeEach(() => {
68+
mockedStat.mockResolvedValue({ mtime: { getTime: () => CURRENT_MTIME } } as any)
69+
})
70+
71+
it("should allow all when history is empty", async () => {
72+
const requestedRanges = [{ start: 1, end: 10 }]
73+
const result = await processAndFilterReadRequest(MOCK_FILE_PATH, requestedRanges, [])
74+
expect(result.status).toBe("ALLOW_ALL")
75+
expect(result.rangesToRead).toEqual(requestedRanges)
76+
})
77+
78+
it("should reject all when a full cache hit occurs", async () => {
79+
const requestedRanges = [{ start: 1, end: 10 }]
80+
const conversationHistory: ConversationMessage[] = [
81+
{
82+
files: [
83+
{
84+
fileName: MOCK_FILE_PATH,
85+
mtime: CURRENT_MTIME,
86+
loadedRanges: [{ start: 1, end: 10 }],
87+
},
88+
],
89+
},
90+
]
91+
const result = await processAndFilterReadRequest(MOCK_FILE_PATH, requestedRanges, conversationHistory)
92+
expect(result.status).toBe("REJECT_ALL")
93+
expect(result.rangesToRead).toEqual([])
94+
})
95+
96+
it("should allow partial when a partial cache hit occurs", async () => {
97+
const requestedRanges = [{ start: 1, end: 20 }]
98+
const conversationHistory: ConversationMessage[] = [
99+
{
100+
files: [
101+
{
102+
fileName: MOCK_FILE_PATH,
103+
mtime: CURRENT_MTIME,
104+
loadedRanges: [{ start: 1, end: 10 }],
105+
},
106+
],
107+
},
108+
]
109+
const result = await processAndFilterReadRequest(MOCK_FILE_PATH, requestedRanges, conversationHistory)
110+
expect(result.status).toBe("ALLOW_PARTIAL")
111+
expect(result.rangesToRead).toEqual([{ start: 11, end: 20 }])
112+
})
113+
114+
it("should allow all when mtime is older in history", async () => {
115+
const requestedRanges = [{ start: 1, end: 10 }]
116+
const conversationHistory: ConversationMessage[] = [
117+
{
118+
files: [
119+
{
120+
fileName: MOCK_FILE_PATH,
121+
mtime: CURRENT_MTIME - 100, // Older mtime
122+
loadedRanges: [{ start: 1, end: 10 }],
123+
},
124+
],
125+
},
126+
]
127+
const result = await processAndFilterReadRequest(MOCK_FILE_PATH, requestedRanges, conversationHistory)
128+
expect(result.status).toBe("ALLOW_ALL")
129+
expect(result.rangesToRead).toEqual(requestedRanges)
130+
})
131+
132+
it("should allow all for a file not in history", async () => {
133+
const requestedRanges = [{ start: 1, end: 10 }]
134+
const conversationHistory: ConversationMessage[] = [
135+
{
136+
files: [
137+
{
138+
fileName: "/another/file.txt",
139+
mtime: CURRENT_MTIME,
140+
loadedRanges: [{ start: 1, end: 10 }],
141+
},
142+
],
143+
},
144+
]
145+
const result = await processAndFilterReadRequest(MOCK_FILE_PATH, requestedRanges, conversationHistory)
146+
expect(result.status).toBe("ALLOW_ALL")
147+
expect(result.rangesToRead).toEqual(requestedRanges)
148+
})
149+
150+
it("should correctly use the most recent valid history entry", async () => {
151+
const requestedRanges = [{ start: 1, end: 20 }]
152+
const conversationHistory: ConversationMessage[] = [
153+
{
154+
// Older, incorrect mtime
155+
files: [
156+
{
157+
fileName: MOCK_FILE_PATH,
158+
mtime: CURRENT_MTIME - 100,
159+
loadedRanges: [{ start: 1, end: 20 }],
160+
},
161+
],
162+
},
163+
{
164+
// Newer, correct mtime but only partial coverage
165+
files: [
166+
{
167+
fileName: MOCK_FILE_PATH,
168+
mtime: CURRENT_MTIME,
169+
loadedRanges: [{ start: 1, end: 5 }],
170+
},
171+
],
172+
},
173+
]
174+
const result = await processAndFilterReadRequest(MOCK_FILE_PATH, requestedRanges, conversationHistory)
175+
expect(result.status).toBe("ALLOW_PARTIAL")
176+
expect(result.rangesToRead).toEqual([{ start: 6, end: 20 }])
177+
})
178+
})
179+
})

‎src/core/services/fileReadCacheService.ts‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
import { stat } from "fs/promises"
2+
3+
export interface LineRange {
4+
start: number
5+
end: number
6+
}
7+
8+
export interface FileMetadata {
9+
fileName: string
10+
mtime: number
11+
loadedRanges: LineRange[]
12+
}
13+
14+
export interface ConversationMessage {
15+
files?: FileMetadata[]
16+
}
17+
18+
export interface FilteredReadRequest {
19+
status: "REJECT_ALL" | "ALLOW_PARTIAL" | "ALLOW_ALL"
20+
rangesToRead: LineRange[]
21+
}
22+
23+
/**
24+
* Checks if two ranges overlap.
25+
*/
26+
function rangesOverlap(r1: LineRange, r2: LineRange): boolean {
27+
return r1.start <= r2.end && r1.end >= r2.start
28+
}
29+
30+
/**
31+
* Subtracts one range from another.
32+
* Returns an array of ranges that are in `original` but not in `toRemove`.
33+
*/
34+
export function subtractRange(original: LineRange, toRemove: LineRange): LineRange[] {
35+
if (!rangesOverlap(original, toRemove)) {
36+
return [original]
37+
}
38+
39+
const result: LineRange[] = []
40+
41+
// Part of original before toRemove
42+
if (original.start < toRemove.start) {
43+
result.push({ start: original.start, end: toRemove.start - 1 })
44+
}
45+
46+
// Part of original after toRemove
47+
if (original.end > toRemove.end) {
48+
result.push({ start: toRemove.end + 1, end: original.end })
49+
}
50+
51+
return result
52+
}
53+
54+
/**
55+
* Subtracts a set of ranges from another set of ranges.
56+
*/
57+
export function subtractRanges(originals: LineRange[], toRemoves: LineRange[]): LineRange[] {
58+
let remaining = [...originals]
59+
60+
for (const toRemove of toRemoves) {
61+
remaining = remaining.flatMap((original) => subtractRange(original, toRemove))
62+
}
63+
64+
return remaining
65+
}
66+
67+
export async function processAndFilterReadRequest(
68+
requestedFile: string,
69+
requestedRanges: LineRange[],
70+
conversationHistory: ConversationMessage[],
71+
): Promise<FilteredReadRequest> {
72+
let currentMtime: number
73+
try {
74+
currentMtime = (await stat(requestedFile)).mtime.getTime()
75+
} catch (error) {
76+
// File doesn't exist or other error, so we must read.
77+
return {
78+
status: "ALLOW_ALL",
79+
rangesToRead: requestedRanges,
80+
}
81+
}
82+
83+
let rangesThatStillNeedToBeRead = [...requestedRanges]
84+
85+
for (let i = conversationHistory.length - 1; i >= 0; i--) {
86+
const message = conversationHistory[i]
87+
if (!message.files) {
88+
continue
89+
}
90+
91+
const relevantFileHistory = message.files.find((f) => f.fileName === requestedFile)
92+
93+
if (relevantFileHistory && relevantFileHistory.mtime >= currentMtime) {
94+
rangesThatStillNeedToBeRead = subtractRanges(rangesThatStillNeedToBeRead, relevantFileHistory.loadedRanges)
95+
96+
if (rangesThatStillNeedToBeRead.length === 0) {
97+
return {
98+
status: "REJECT_ALL",
99+
rangesToRead: [],
100+
}
101+
}
102+
}
103+
}
104+
105+
const originalRangesString = JSON.stringify(requestedRanges.sort((a, b) => a.start - b.start))
106+
const finalRangesString = JSON.stringify(rangesThatStillNeedToBeRead.sort((a, b) => a.start - b.start))
107+
108+
if (originalRangesString === finalRangesString) {
109+
return {
110+
status: "ALLOW_ALL",
111+
rangesToRead: requestedRanges,
112+
}
113+
}
114+
115+
return {
116+
status: "ALLOW_PARTIAL",
117+
rangesToRead: rangesThatStillNeedToBeRead,
118+
}
119+
}

0 commit comments

Comments
 (0)