Skip to content
This repository was archived by the owner on May 15, 2026. It is now read-only.

Commit bfb0d5f

Browse files
daniel-lxshannesrudolph
authored andcommitted
refactor: improve code quality in condense module
- Convert summarizeConversation to use options object instead of 11 positional params - Extract duplicated getFilesReadByRoo error handling into helper method - Remove unnecessary re-export of generateFoldedFileContext - Update all test files to use new options object pattern
1 parent 0f4e7b7 commit bfb0d5f

7 files changed

Lines changed: 264 additions & 193 deletions

File tree

src/core/condense/__tests__/condense.spec.ts

Lines changed: 49 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,13 @@ Line 2
136136
{ role: "user", content: "Ninth message" },
137137
]
138138

139-
const result = await summarizeConversation(messages, mockApiHandler, "System prompt", taskId, false)
139+
const result = await summarizeConversation({
140+
messages,
141+
apiHandler: mockApiHandler,
142+
systemPrompt: "System prompt",
143+
taskId,
144+
isAutomaticTrigger: false,
145+
})
140146

141147
// Verify we have a summary message with role "user" (fresh start model)
142148
const summaryMessage = result.messages.find((msg) => msg.isSummary)
@@ -164,7 +170,13 @@ Line 2
164170
{ role: "user", content: "Fifth message" },
165171
]
166172

167-
const result = await summarizeConversation(messages, mockApiHandler, "System prompt", taskId, false)
173+
const result = await summarizeConversation({
174+
messages,
175+
apiHandler: mockApiHandler,
176+
systemPrompt: "System prompt",
177+
taskId,
178+
isAutomaticTrigger: false,
179+
})
168180

169181
// All original messages should be tagged with condenseParent
170182
const taggedMessages = result.messages.filter((msg) => !msg.isSummary)
@@ -193,7 +205,13 @@ Line 2
193205
{ role: "user", content: "Ninth message" },
194206
]
195207

196-
const result = await summarizeConversation(messages, mockApiHandler, "System prompt", taskId, false)
208+
const result = await summarizeConversation({
209+
messages,
210+
apiHandler: mockApiHandler,
211+
systemPrompt: "System prompt",
212+
taskId,
213+
isAutomaticTrigger: false,
214+
})
197215

198216
const summaryMessage = result.messages.find((msg) => msg.isSummary)
199217
expect(summaryMessage).toBeTruthy()
@@ -227,7 +245,13 @@ Line 2
227245
{ role: "user", content: "Perfect!" },
228246
]
229247

230-
const result = await summarizeConversation(messages, mockApiHandler, "System prompt", taskId, false)
248+
const result = await summarizeConversation({
249+
messages,
250+
apiHandler: mockApiHandler,
251+
systemPrompt: "System prompt",
252+
taskId,
253+
isAutomaticTrigger: false,
254+
})
231255

232256
// Effective history should contain only the summary (fresh start)
233257
const effectiveHistory = getEffectiveApiHistory(result.messages)
@@ -239,7 +263,13 @@ Line 2
239263
it("should return error when not enough messages to summarize", async () => {
240264
const messages: ApiMessage[] = [{ role: "user", content: "Only one message" }]
241265

242-
const result = await summarizeConversation(messages, mockApiHandler, "System prompt", taskId, false)
266+
const result = await summarizeConversation({
267+
messages,
268+
apiHandler: mockApiHandler,
269+
systemPrompt: "System prompt",
270+
taskId,
271+
isAutomaticTrigger: false,
272+
})
243273

244274
// Should return an error since we have only 1 message
245275
expect(result.error).toBeDefined()
@@ -253,7 +283,13 @@ Line 2
253283
{ role: "user", content: "Previous summary", isSummary: true },
254284
]
255285

256-
const result = await summarizeConversation(messages, mockApiHandler, "System prompt", taskId, false)
286+
const result = await summarizeConversation({
287+
messages,
288+
apiHandler: mockApiHandler,
289+
systemPrompt: "System prompt",
290+
taskId,
291+
isAutomaticTrigger: false,
292+
})
257293

258294
// Should return an error due to recent summary with no substantial messages after
259295
expect(result.error).toBeDefined()
@@ -286,7 +322,13 @@ Line 2
286322
{ role: "user", content: "Seventh" },
287323
]
288324

289-
const result = await summarizeConversation(messages, emptyHandler, "System prompt", taskId, false)
325+
const result = await summarizeConversation({
326+
messages,
327+
apiHandler: emptyHandler,
328+
systemPrompt: "System prompt",
329+
taskId,
330+
isAutomaticTrigger: false,
331+
})
290332

291333
expect(result.error).toBeDefined()
292334
expect(result.messages).toEqual(messages)

src/core/condense/__tests__/foldedFileContext.spec.ts

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -298,19 +298,15 @@ describe("foldedFileContext", () => {
298298
const filesReadByRoo = ["src/user.ts", "src/api.ts"]
299299
const cwd = "/test/project"
300300

301-
const result = await summarizeConversation(
301+
const result = await summarizeConversation({
302302
messages,
303-
mockApiHandler,
304-
"System prompt",
303+
apiHandler: mockApiHandler,
304+
systemPrompt: "System prompt",
305305
taskId,
306-
false,
307-
undefined, // customCondensingPrompt
308-
undefined, // metadata
309-
undefined, // environmentDetails
306+
isAutomaticTrigger: false,
310307
filesReadByRoo,
311308
cwd,
312-
undefined, // rooIgnoreController
313-
)
309+
})
314310

315311
// Verify generateFoldedFileContext was called with the right arguments
316312
expect(mockedGenerateFoldedFileContext).toHaveBeenCalledWith(filesReadByRoo, {
@@ -367,19 +363,15 @@ describe("foldedFileContext", () => {
367363
// Reset the mock to ensure clean state
368364
mockedGenerateFoldedFileContext.mockClear()
369365

370-
const result = await summarizeConversation(
366+
const result = await summarizeConversation({
371367
messages,
372-
mockApiHandler,
373-
"System prompt",
368+
apiHandler: mockApiHandler,
369+
systemPrompt: "System prompt",
374370
taskId,
375-
false,
376-
undefined, // customCondensingPrompt
377-
undefined, // metadata
378-
undefined, // environmentDetails
379-
[], // Empty filesReadByRoo array
380-
"/test/project",
381-
undefined, // rooIgnoreController
382-
)
371+
isAutomaticTrigger: false,
372+
filesReadByRoo: [],
373+
cwd: "/test/project",
374+
})
383375

384376
// generateFoldedFileContext should NOT be called when filesReadByRoo is empty
385377
expect(mockedGenerateFoldedFileContext).not.toHaveBeenCalled()

0 commit comments

Comments
 (0)