Skip to content

Commit 1418823

Browse files
committed
Improve combineApiRequests performance
1 parent 8d045a4 commit 1418823

File tree

2 files changed

+367
-33
lines changed

2 files changed

+367
-33
lines changed
Lines changed: 323 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,323 @@
1+
import { combineApiRequests } from "../combineApiRequests"
2+
import { ClineMessage } from "../ExtensionMessage"
3+
4+
describe("combineApiRequests", () => {
5+
// Helper function to create a basic api_req_started message
6+
const createStartMessage = (text: string = '{"request":"GET /api/data"}', ts: number = 1000): ClineMessage => ({
7+
type: "say",
8+
say: "api_req_started",
9+
text,
10+
ts,
11+
})
12+
13+
// Helper function to create a basic api_req_finished message
14+
const createFinishMessage = (text: string = '{"cost":0.005}', ts: number = 1001): ClineMessage => ({
15+
type: "say",
16+
say: "api_req_finished",
17+
text,
18+
ts,
19+
})
20+
21+
// Helper function to create a non-API message
22+
const createOtherMessage = (
23+
say: "text" | "task" | "error" | "command" = "text",
24+
text: string = "Hello world",
25+
ts: number = 999,
26+
): ClineMessage => ({
27+
type: "say",
28+
say,
29+
text,
30+
ts,
31+
})
32+
33+
describe("Basic functionality", () => {
34+
it("should combine a pair of api_req_started and api_req_finished messages", () => {
35+
const messages: ClineMessage[] = [createStartMessage(), createFinishMessage()]
36+
37+
const result = combineApiRequests(messages)
38+
39+
// Should have one message (the combined one)
40+
expect(result).toHaveLength(1)
41+
42+
// The combined message should have the properties of the start message
43+
expect(result[0].type).toBe("say")
44+
expect(result[0].say).toBe("api_req_started")
45+
expect(result[0].ts).toBe(1000)
46+
47+
// The text should be a JSON string with combined properties
48+
const parsedText = JSON.parse(result[0].text || "{}")
49+
expect(parsedText).toEqual({
50+
request: "GET /api/data",
51+
cost: 0.005,
52+
})
53+
})
54+
55+
it("should handle multiple pairs of API request messages", () => {
56+
const messages: ClineMessage[] = [
57+
createStartMessage('{"request":"GET /api/data1"}', 1000),
58+
createFinishMessage('{"cost":0.005}', 1001),
59+
createStartMessage('{"request":"GET /api/data2"}', 2000),
60+
createFinishMessage('{"cost":0.007}', 2001),
61+
]
62+
63+
const result = combineApiRequests(messages)
64+
65+
// Should have two messages (the combined ones)
66+
expect(result).toHaveLength(2)
67+
68+
// Check first combined message
69+
const parsedText1 = JSON.parse(result[0].text || "{}")
70+
expect(parsedText1).toEqual({
71+
request: "GET /api/data1",
72+
cost: 0.005,
73+
})
74+
75+
// Check second combined message
76+
const parsedText2 = JSON.parse(result[1].text || "{}")
77+
expect(parsedText2).toEqual({
78+
request: "GET /api/data2",
79+
cost: 0.007,
80+
})
81+
})
82+
83+
it("should preserve non-API messages", () => {
84+
const otherMessage = createOtherMessage()
85+
const messages: ClineMessage[] = [otherMessage, createStartMessage(), createFinishMessage()]
86+
87+
const result = combineApiRequests(messages)
88+
89+
// Should have two messages (the other message and the combined one)
90+
expect(result).toHaveLength(2)
91+
92+
// The first message should be unchanged
93+
expect(result[0]).toEqual(otherMessage)
94+
})
95+
96+
it("should handle interleaved API and non-API messages", () => {
97+
const otherMessage1 = createOtherMessage("text", "Message 1", 999)
98+
const otherMessage2 = createOtherMessage("text", "Message 2", 1500)
99+
100+
const messages: ClineMessage[] = [
101+
otherMessage1,
102+
createStartMessage('{"request":"GET /api/data1"}', 1000),
103+
createFinishMessage('{"cost":0.005}', 1001),
104+
otherMessage2,
105+
createStartMessage('{"request":"GET /api/data2"}', 2000),
106+
createFinishMessage('{"cost":0.007}', 2001),
107+
]
108+
109+
const result = combineApiRequests(messages)
110+
111+
// Should have four messages (two other messages and two combined ones)
112+
expect(result).toHaveLength(4)
113+
114+
// Check the order and content of messages
115+
expect(result[0]).toEqual(otherMessage1)
116+
117+
const parsedText1 = JSON.parse(result[1].text || "{}")
118+
expect(parsedText1).toEqual({
119+
request: "GET /api/data1",
120+
cost: 0.005,
121+
})
122+
123+
expect(result[2]).toEqual(otherMessage2)
124+
125+
const parsedText2 = JSON.parse(result[3].text || "{}")
126+
expect(parsedText2).toEqual({
127+
request: "GET /api/data2",
128+
cost: 0.007,
129+
})
130+
})
131+
})
132+
133+
describe("Edge cases", () => {
134+
it("should handle empty messages array", () => {
135+
const result = combineApiRequests([])
136+
expect(result).toEqual([])
137+
})
138+
139+
it("should keep api_req_started message if no matching api_req_finished is found", () => {
140+
const startMessage = createStartMessage()
141+
const messages: ClineMessage[] = [startMessage]
142+
143+
const result = combineApiRequests(messages)
144+
145+
// Should have one message (the original start message)
146+
expect(result).toHaveLength(1)
147+
expect(result[0]).toEqual(startMessage)
148+
})
149+
150+
it("should handle missing text field in api_req_started", () => {
151+
const startMessage: ClineMessage = {
152+
type: "say",
153+
say: "api_req_started",
154+
ts: 1000,
155+
// text field is missing
156+
}
157+
const finishMessage = createFinishMessage()
158+
159+
const messages: ClineMessage[] = [startMessage, finishMessage]
160+
161+
const result = combineApiRequests(messages)
162+
163+
// Should have one message (the combined one)
164+
expect(result).toHaveLength(1)
165+
166+
// The text should be a JSON string with only the finish message properties
167+
const parsedText = JSON.parse(result[0].text || "{}")
168+
expect(parsedText).toEqual({
169+
cost: 0.005,
170+
})
171+
})
172+
173+
it("should handle missing text field in api_req_finished", () => {
174+
const startMessage = createStartMessage()
175+
const finishMessage: ClineMessage = {
176+
type: "say",
177+
say: "api_req_finished",
178+
ts: 1001,
179+
// text field is missing
180+
}
181+
182+
const messages: ClineMessage[] = [startMessage, finishMessage]
183+
184+
const result = combineApiRequests(messages)
185+
186+
// Should have one message (the combined one)
187+
expect(result).toHaveLength(1)
188+
189+
// The text should be a JSON string with only the start message properties
190+
const parsedText = JSON.parse(result[0].text || "{}")
191+
expect(parsedText).toEqual({
192+
request: "GET /api/data",
193+
})
194+
})
195+
196+
it("should use the first api_req_finished message if multiple matches exist", () => {
197+
const messages: ClineMessage[] = [
198+
createStartMessage('{"request":"GET /api/data"}', 1000),
199+
createFinishMessage('{"cost":0.005}', 1001),
200+
createFinishMessage('{"cost":0.007}', 1002), // This should be ignored
201+
]
202+
203+
const result = combineApiRequests(messages)
204+
205+
// Should have one message (the combined one)
206+
expect(result).toHaveLength(1)
207+
208+
// The text should be a JSON string with combined properties from the first finish message
209+
const parsedText = JSON.parse(result[0].text || "{}")
210+
expect(parsedText).toEqual({
211+
request: "GET /api/data",
212+
cost: 0.005, // Should use the first finish message's cost
213+
})
214+
})
215+
216+
it("should handle multiple start messages with some missing finish messages", () => {
217+
const messages: ClineMessage[] = [
218+
createStartMessage('{"request":"GET /api/data1"}', 1000),
219+
createFinishMessage('{"cost":0.005}', 1001),
220+
createStartMessage('{"request":"GET /api/data2"}', 2000),
221+
// No finish message for the second start message
222+
]
223+
224+
const result = combineApiRequests(messages)
225+
226+
// Should have two messages (one combined and one original start message)
227+
expect(result).toHaveLength(2)
228+
229+
// Check first combined message
230+
const parsedText1 = JSON.parse(result[0].text || "{}")
231+
expect(parsedText1).toEqual({
232+
request: "GET /api/data1",
233+
cost: 0.005,
234+
})
235+
236+
// Check second message (should be the original start message)
237+
expect(result[1].say).toBe("api_req_started")
238+
const parsedText2 = JSON.parse(result[1].text || "{}")
239+
expect(parsedText2).toEqual({
240+
request: "GET /api/data2",
241+
})
242+
})
243+
244+
it("should preserve additional properties in the messages", () => {
245+
const startMessage: ClineMessage = {
246+
type: "say",
247+
say: "api_req_started",
248+
text: '{"request":"GET /api/data"}',
249+
ts: 1000,
250+
reasoning: "This is a test",
251+
partial: false,
252+
}
253+
254+
const finishMessage: ClineMessage = {
255+
type: "say",
256+
say: "api_req_finished",
257+
text: '{"cost":0.005}',
258+
ts: 1001,
259+
}
260+
261+
const messages: ClineMessage[] = [startMessage, finishMessage]
262+
263+
const result = combineApiRequests(messages)
264+
265+
// Should have one message (the combined one)
266+
expect(result).toHaveLength(1)
267+
268+
// The combined message should preserve additional properties from the start message
269+
expect(result[0].reasoning).toBe("This is a test")
270+
expect(result[0].partial).toBe(false)
271+
})
272+
})
273+
274+
describe("Bug verification", () => {
275+
it("should correctly replace api_req_started messages with combined ones", () => {
276+
// Create a scenario where we can verify the replacement logic
277+
const otherMessage = createOtherMessage()
278+
const startMessage1 = createStartMessage('{"request":"GET /api/data1"}', 1000)
279+
const finishMessage1 = createFinishMessage('{"cost":0.005}', 1001)
280+
const startMessage2 = createStartMessage('{"request":"GET /api/data2"}', 2000)
281+
// No finish message for the second start
282+
283+
const messages: ClineMessage[] = [otherMessage, startMessage1, finishMessage1, startMessage2]
284+
285+
const result = combineApiRequests(messages)
286+
287+
// Should have three messages (other, combined, and the orphaned start)
288+
expect(result).toHaveLength(3)
289+
290+
// First message should be unchanged
291+
expect(result[0]).toEqual(otherMessage)
292+
293+
// Second message should be a combined message with the same ts as startMessage1
294+
expect(result[1].ts).toBe(startMessage1.ts)
295+
expect(result[1].say).toBe("api_req_started")
296+
const parsedText1 = JSON.parse(result[1].text || "{}")
297+
expect(parsedText1).toEqual({
298+
request: "GET /api/data1",
299+
cost: 0.005,
300+
})
301+
302+
// Third message should be the original startMessage2
303+
expect(result[2]).toEqual(startMessage2)
304+
})
305+
306+
it("should filter out all api_req_finished messages", () => {
307+
const messages: ClineMessage[] = [
308+
createOtherMessage(),
309+
createStartMessage(),
310+
createFinishMessage(),
311+
createFinishMessage('{"cost":0.007}', 2000), // Orphaned finish message
312+
]
313+
314+
const result = combineApiRequests(messages)
315+
316+
// Should have two messages (other and combined), no finish messages
317+
expect(result).toHaveLength(2)
318+
319+
// No message should have say="api_req_finished"
320+
expect(result.some((msg) => msg.say === "api_req_finished")).toBe(false)
321+
})
322+
})
323+
})

0 commit comments

Comments
 (0)