From 5d8351232a9f351de58cadf8b5add2a2b6934959 Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Sat, 24 Jan 2026 12:05:51 -0700 Subject: [PATCH 1/6] feat(condense): add smart code folding with tree-sitter signatures At context condensation time, use tree-sitter to generate folded code signatures (function definitions, class declarations) for files read during the conversation. Each file is included as its own block in the condensed summary, preserving structural awareness without consuming excessive tokens. - Add getFilesReadByRoo() method to FileContextTracker - Create generateFoldedFileContext() using tree-sitter parsing - Update summarizeConversation() to accept array of file sections - Each file gets its own content block in the summary message - Add comprehensive test coverage (12 tests) --- .../__tests__/foldedFileContext.spec.ts | 335 ++++++++++++++++++ src/core/condense/foldedFileContext.ts | 147 ++++++++ src/core/condense/index.ts | 22 +- .../context-tracking/FileContextTracker.ts | 36 ++ src/core/task/Task.ts | 27 +- 5 files changed, 565 insertions(+), 2 deletions(-) create mode 100644 src/core/condense/__tests__/foldedFileContext.spec.ts create mode 100644 src/core/condense/foldedFileContext.ts diff --git a/src/core/condense/__tests__/foldedFileContext.spec.ts b/src/core/condense/__tests__/foldedFileContext.spec.ts new file mode 100644 index 00000000000..4f9b9a8350c --- /dev/null +++ b/src/core/condense/__tests__/foldedFileContext.spec.ts @@ -0,0 +1,335 @@ +// npx vitest src/core/condense/__tests__/foldedFileContext.spec.ts + +import * as path from "path" +import { Anthropic } from "@anthropic-ai/sdk" +import type { ModelInfo } from "@roo-code/types" +import { TelemetryService } from "@roo-code/telemetry" +import { BaseProvider } from "../../../api/providers/base-provider" + +// Mock the tree-sitter module +vi.mock("../../../services/tree-sitter", () => ({ + parseSourceCodeDefinitionsForFile: vi.fn(), +})) + +import { generateFoldedFileContext } from "../foldedFileContext" +import { parseSourceCodeDefinitionsForFile } from "../../../services/tree-sitter" + +const mockedParseSourceCodeDefinitions = vi.mocked(parseSourceCodeDefinitionsForFile) + +describe("foldedFileContext", () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + describe("generateFoldedFileContext", () => { + it("should return empty content for empty file list", async () => { + const result = await generateFoldedFileContext([], { cwd: "/test" }) + + expect(result.content).toBe("") + expect(result.sections).toEqual([]) + expect(result.filesProcessed).toBe(0) + expect(result.filesSkipped).toBe(0) + expect(result.characterCount).toBe(0) + }) + + it("should generate folded context for a TypeScript file with its own system-reminder block", async () => { + const mockDefinitions = `1--5 | export interface User +7--12 | export function createUser(name: string): User +14--28 | export class UserService` + + mockedParseSourceCodeDefinitions.mockResolvedValue(mockDefinitions) + + const result = await generateFoldedFileContext(["/test/user.ts"], { cwd: "/test" }) + + // Each file should be wrapped in its own block + expect(result.content).toContain("") + expect(result.content).toContain("") + expect(result.content).toContain("## File Context: /test/user.ts") + expect(result.content).toContain("interface User") + expect(result.content).toContain("function createUser") + expect(result.content).toContain("class UserService") + expect(result.filesProcessed).toBe(1) + expect(result.filesSkipped).toBe(0) + }) + + it("should generate folded context for a JavaScript file with its own system-reminder block", async () => { + const mockDefinitions = `1--3 | function greet(name) +5--15 | class Calculator` + + mockedParseSourceCodeDefinitions.mockResolvedValue(mockDefinitions) + + const result = await generateFoldedFileContext(["/test/utils.js"], { cwd: "/test" }) + + expect(result.content).toContain("") + expect(result.content).toContain("## File Context: /test/utils.js") + expect(result.content).toContain("function greet") + expect(result.content).toContain("class Calculator") + expect(result.filesProcessed).toBe(1) + }) + + it("should skip files when parseSourceCodeDefinitions returns undefined", async () => { + // First file succeeds, second returns undefined + mockedParseSourceCodeDefinitions + .mockResolvedValueOnce("1--3 | export const x = 1") + .mockResolvedValueOnce(undefined) + + const result = await generateFoldedFileContext(["/test/existing.ts", "/test/unsupported.txt"], { + cwd: "/test", + }) + + expect(result.filesProcessed).toBe(1) + expect(result.filesSkipped).toBe(1) + }) + + it("should skip files when parseSourceCodeDefinitions throws an error", async () => { + mockedParseSourceCodeDefinitions + .mockResolvedValueOnce("1--3 | export const x = 1") + .mockRejectedValueOnce(new Error("File not found")) + + const result = await generateFoldedFileContext(["/test/existing.ts", "/test/non-existent.ts"], { + cwd: "/test", + }) + + expect(result.filesProcessed).toBe(1) + expect(result.filesSkipped).toBe(1) + }) + + it("should respect character budget limit", async () => { + // Create multiple files that would exceed a small budget + const longDefinitions = `1--3 | export function longFunctionName1() +5--7 | export function longFunctionName2() +9--11 | export function longFunctionName3()` + + mockedParseSourceCodeDefinitions.mockResolvedValue(longDefinitions) + + const result = await generateFoldedFileContext(["/test/file1.ts", "/test/file2.ts", "/test/file3.ts"], { + cwd: "/test", + maxCharacters: 200, // Small budget + }) + + expect(result.characterCount).toBeLessThanOrEqual(200) + // Some files should be skipped due to budget limit + expect(result.filesSkipped).toBeGreaterThan(0) + }) + + it("should handle Python files with its own system-reminder block", async () => { + const mockDefinitions = `1--2 | def greet(name) +4--12 | class Person` + + mockedParseSourceCodeDefinitions.mockResolvedValue(mockDefinitions) + + const result = await generateFoldedFileContext(["/test/person.py"], { cwd: "/test" }) + + expect(result.content).toContain("") + expect(result.content).toContain("## File Context: /test/person.py") + expect(result.content).toContain("def greet") + expect(result.content).toContain("class Person") + expect(result.filesProcessed).toBe(1) + }) + + it("should include file path in the File Context header", async () => { + mockedParseSourceCodeDefinitions.mockResolvedValue("1--3 | export function helper()") + + const result = await generateFoldedFileContext(["/test/src/utils/helpers.ts"], { cwd: "/test" }) + + // The path should appear in the File Context header + expect(result.content).toContain("## File Context: /test/src/utils/helpers.ts") + }) + + it("should generate separate system-reminder blocks for multiple files", async () => { + mockedParseSourceCodeDefinitions + .mockResolvedValueOnce("1--3 | export async function fetchData(url: string): Promise") + .mockResolvedValueOnce("1--4 | export interface DataModel") + + const result = await generateFoldedFileContext(["/test/api.ts", "/test/models.ts"], { cwd: "/test" }) + + // Each file should have its own block + const systemReminderMatches = result.content.match(//g) + expect(systemReminderMatches).toHaveLength(2) + + // sections array should have separate entries for each file + expect(result.sections).toHaveLength(2) + expect(result.sections[0]).toContain("## File Context: /test/api.ts") + expect(result.sections[1]).toContain("## File Context: /test/models.ts") + + expect(result.content).toContain("## File Context: /test/api.ts") + expect(result.content).toContain("## File Context: /test/models.ts") + expect(result.content).toContain("fetchData") + expect(result.content).toContain("interface DataModel") + expect(result.filesProcessed).toBe(2) + }) + + it("should truncate content when approaching character limit", async () => { + // Create a definition that would fit but is close to the limit + const longDefinitions = "1--3 | " + "x".repeat(300) + + mockedParseSourceCodeDefinitions.mockResolvedValue(longDefinitions) + + const result = await generateFoldedFileContext(["/test/file1.ts", "/test/file2.ts"], { + cwd: "/test", + maxCharacters: 350, // First file will fit, second will be truncated + }) + + // Content should include truncation marker if truncation happened + expect(result.filesProcessed + result.filesSkipped).toBe(2) + }) + }) + + describe("summarizeConversation with foldedFileContext", () => { + beforeEach(() => { + if (!TelemetryService.hasInstance()) { + TelemetryService.createInstance([]) + } + }) + + // Mock API handler for testing + class MockApiHandler extends BaseProvider { + createMessage(): any { + const mockStream = { + async *[Symbol.asyncIterator]() { + yield { type: "text", text: "Mock summary of the conversation" } + yield { type: "usage", inputTokens: 100, outputTokens: 50, totalCost: 0.01 } + }, + } + return mockStream + } + + getModel(): { id: string; info: ModelInfo } { + return { + id: "test-model", + info: { + contextWindow: 100000, + maxTokens: 50000, + supportsPromptCache: true, + supportsImages: false, + inputPrice: 0, + outputPrice: 0, + description: "Test model", + }, + } + } + + override async countTokens(content: Array): Promise { + let tokens = 0 + for (const block of content) { + if (block.type === "text") { + tokens += Math.ceil(block.text.length / 4) + } + } + return tokens + } + } + + it("should include folded file context with each file as a separate content block", async () => { + const { summarizeConversation } = await import("../index") + + const mockApiHandler = new MockApiHandler() + const taskId = "test-task-id" + + const messages: any[] = [ + { role: "user", content: "First message" }, + { role: "assistant", content: "Second message" }, + { role: "user", content: "Third message" }, + { role: "assistant", content: "Fourth message" }, + { role: "user", content: "Fifth message" }, + { role: "assistant", content: "Sixth message" }, + { role: "user", content: "Seventh message" }, + ] + + // Folded file context sections - each file in its own block + const foldedFileContextSections = [ + ` +## File Context: src/user.ts +1--5 | export interface User +7--12 | export function createUser(name: string): User +14--28 | export class UserService +`, + ` +## File Context: src/api.ts +1--3 | export async function fetchData(url: string): Promise +`, + ] + + const result = await summarizeConversation( + messages, + mockApiHandler, + "System prompt", + taskId, + false, + undefined, // customCondensingPrompt + undefined, // metadata + undefined, // environmentDetails + foldedFileContextSections, // Array of sections, each file in its own block + ) + + // Verify the summary was created + expect(result.summary).toBeDefined() + expect(result.messages.length).toBeGreaterThan(0) + + // Find the summary message + const summaryMessage = result.messages.find((msg: any) => msg.isSummary) + expect(summaryMessage).toBeDefined() + + // Each file should have its own content block + const contentArray = summaryMessage!.content as any[] + + // Find the content blocks containing file contexts + const userFileBlock = contentArray.find( + (block: any) => block.type === "text" && block.text?.includes("## File Context: src/user.ts"), + ) + const apiFileBlock = contentArray.find( + (block: any) => block.type === "text" && block.text?.includes("## File Context: src/api.ts"), + ) + + expect(userFileBlock).toBeDefined() + expect(apiFileBlock).toBeDefined() + + // Each file block should have its own tags + expect(userFileBlock.text).toContain("") + expect(userFileBlock.text).toContain("export interface User") + + expect(apiFileBlock.text).toContain("") + expect(apiFileBlock.text).toContain("fetchData") + }) + + it("should not include file context section when foldedFileContextSections is empty", async () => { + const { summarizeConversation } = await import("../index") + + const mockApiHandler = new MockApiHandler() + const taskId = "test-task-id-2" + + const messages: any[] = [ + { role: "user", content: "First message" }, + { role: "assistant", content: "Second message" }, + { role: "user", content: "Third message" }, + { role: "assistant", content: "Fourth message" }, + { role: "user", content: "Fifth message" }, + { role: "assistant", content: "Sixth message" }, + { role: "user", content: "Seventh message" }, + ] + + const result = await summarizeConversation( + messages, + mockApiHandler, + "System prompt", + taskId, + false, + undefined, // customCondensingPrompt + undefined, // metadata + undefined, // environmentDetails + [], // Empty foldedFileContextSections array + ) + + // Find the summary message + const summaryMessage = result.messages.find((msg: any) => msg.isSummary) + expect(summaryMessage).toBeDefined() + + // The summary content should NOT contain any file context blocks + const contentArray = summaryMessage!.content as any[] + const fileContextBlock = contentArray.find( + (block: any) => block.type === "text" && block.text?.includes("## File Context"), + ) + expect(fileContextBlock).toBeUndefined() + }) + }) +}) diff --git a/src/core/condense/foldedFileContext.ts b/src/core/condense/foldedFileContext.ts new file mode 100644 index 00000000000..db2ad4d380d --- /dev/null +++ b/src/core/condense/foldedFileContext.ts @@ -0,0 +1,147 @@ +import * as path from "path" +import { parseSourceCodeDefinitionsForFile } from "../../services/tree-sitter" +import { RooIgnoreController } from "../ignore/RooIgnoreController" + +/** + * Result of generating folded file context. + */ +export interface FoldedFileContextResult { + /** The formatted string containing all folded file definitions (joined) */ + content: string + /** Individual file sections, each in its own block */ + sections: string[] + /** Number of files successfully processed */ + filesProcessed: number + /** Number of files that failed or were skipped */ + filesSkipped: number + /** Total character count of the folded content */ + characterCount: number +} + +/** + * Options for generating folded file context. + */ +export interface FoldedFileContextOptions { + /** Maximum total characters for the folded content (default: 50000) */ + maxCharacters?: number + /** The current working directory for resolving relative paths */ + cwd: string + /** Optional RooIgnoreController for file access validation */ + rooIgnoreController?: RooIgnoreController +} + +/** + * Generates folded (signatures-only) file context for a list of files using tree-sitter. + * + * This function takes file paths that were read during a conversation and produces + * a condensed representation showing only function signatures, class declarations, + * and other important structural definitions - hiding implementation bodies. + * + * Each file is wrapped in its own `` block during context condensation, + * allowing the model to retain awareness of file structure without consuming excessive tokens. + * + * @param filePaths - Array of file paths to process (relative to cwd) + * @param options - Configuration options including cwd and max characters + * @returns FoldedFileContextResult with the formatted content and statistics + * + * @example + * ```typescript + * const result = await generateFoldedFileContext( + * ['src/utils/helpers.ts', 'src/api/client.ts'], + * { cwd: '/project', maxCharacters: 30000 } + * ) + * // result.content contains individual blocks for each file: + * // + * // ## File Context: src/utils/helpers.ts + * // 1--15 | export function formatDate(...) + * // 17--45 | export class DateHelper {...} + * // + * // + * // ## File Context: src/api/client.ts + * // ... + * // + * ``` + */ +export async function generateFoldedFileContext( + filePaths: string[], + options: FoldedFileContextOptions, +): Promise { + const { maxCharacters = 50000, cwd, rooIgnoreController } = options + + const result: FoldedFileContextResult = { + content: "", + sections: [], + filesProcessed: 0, + filesSkipped: 0, + characterCount: 0, + } + + if (filePaths.length === 0) { + return result + } + + const foldedSections: string[] = [] + let currentCharCount = 0 + + for (const filePath of filePaths) { + // Resolve to absolute path for tree-sitter + const absolutePath = path.isAbsolute(filePath) ? filePath : path.resolve(cwd, filePath) + + try { + // Get the folded definitions using tree-sitter + const definitions = await parseSourceCodeDefinitionsForFile(absolutePath, rooIgnoreController) + + if (!definitions) { + // File type not supported or no definitions found + result.filesSkipped++ + continue + } + + // Wrap each file in its own block + const sectionContent = ` +## File Context: ${filePath} +${definitions} +` + + // Check if adding this file would exceed the character limit + if (currentCharCount + sectionContent.length > maxCharacters) { + // Would exceed limit - check if we can fit at least a truncated version + const remainingChars = maxCharacters - currentCharCount + if (remainingChars < 200) { + // Not enough room for meaningful content, stop processing + result.filesSkipped++ + continue + } + + // Truncate the definitions to fit within the system-reminder block + const truncatedDefinitions = definitions.substring(0, remainingChars - 100) + "\n... (truncated)" + const truncatedContent = ` +## File Context: ${filePath} +${truncatedDefinitions} +` + foldedSections.push(truncatedContent) + currentCharCount += truncatedContent.length + result.filesProcessed++ + + // Stop processing more files since we've hit the limit + result.filesSkipped += filePaths.length - result.filesProcessed - result.filesSkipped + break + } + + foldedSections.push(sectionContent) + currentCharCount += sectionContent.length + result.filesProcessed++ + } catch (error) { + console.error(`Failed to generate folded context for ${filePath}:`, error) + result.filesSkipped++ + } + } + + if (foldedSections.length > 0) { + result.sections = foldedSections + result.content = foldedSections.join("\n") + result.characterCount = result.content.length + } + + return result +} diff --git a/src/core/condense/index.ts b/src/core/condense/index.ts index 313bfcebb6b..de61b3c018e 100644 --- a/src/core/condense/index.ts +++ b/src/core/condense/index.ts @@ -10,6 +10,10 @@ import { maybeRemoveImageBlocks } from "../../api/transform/image-cleaning" import { findLast } from "../../shared/array" import { supportPrompt } from "../../shared/support-prompt" +// Re-export folded file context utilities +export { generateFoldedFileContext } from "./foldedFileContext" +export type { FoldedFileContextResult, FoldedFileContextOptions } from "./foldedFileContext" + export const MIN_CONDENSE_THRESHOLD = 5 // Minimum percentage of context window to trigger condensing export const MAX_CONDENSE_THRESHOLD = 100 // Maximum percentage of context window to trigger condensing @@ -131,6 +135,7 @@ export type SummarizeResponse = { * - Post-condense, the model sees only the summary (true fresh start) * - All messages are still stored but tagged with condenseParent * - blocks from the original task are preserved across condensings + * - File context (folded code definitions) can be preserved for continuity * * Environment details handling: * - For AUTOMATIC condensing (isAutomaticTrigger=true): Environment details are included @@ -148,6 +153,7 @@ export type SummarizeResponse = { * @param {string} customCondensingPrompt - Optional custom prompt to use for condensing * @param {ApiHandlerCreateMessageMetadata} metadata - Optional metadata to pass to createMessage (tools, taskId, etc.) * @param {string} environmentDetails - Optional environment details string to include in the summary (only used when isAutomaticTrigger=true) + * @param {string[]} foldedFileContextSections - Optional array of folded file context sections (each file in its own block) * @returns {SummarizeResponse} - The result of the summarization operation (see above) */ export async function summarizeConversation( @@ -159,6 +165,7 @@ export async function summarizeConversation( customCondensingPrompt?: string, metadata?: ApiHandlerCreateMessageMetadata, environmentDetails?: string, + foldedFileContextSections?: string[], ): Promise { TelemetryService.instance.captureContextCondensed( taskId, @@ -289,7 +296,7 @@ export async function summarizeConversation( { type: "text", text: `## Conversation Summary\n${summary}` }, ] - // Add command blocks as a separate text block if present + // Add command blocks (active workflows) in their own system-reminder block if present if (commandBlocks) { summaryContent.push({ type: "text", @@ -301,6 +308,19 @@ ${commandBlocks} }) } + // Add folded file context (smart code folding) if present + // Each file gets its own block as a separate content block + if (foldedFileContextSections && foldedFileContextSections.length > 0) { + for (const section of foldedFileContextSections) { + if (section.trim()) { + summaryContent.push({ + type: "text", + text: section, + }) + } + } + } + // Add environment details as a separate text block if provided AND this is an automatic trigger. // For manual condensing, fresh environment details will be injected on the next turn. // For automatic condensing, the API request is already in progress so we need them in the summary. diff --git a/src/core/context-tracking/FileContextTracker.ts b/src/core/context-tracking/FileContextTracker.ts index 5741b62cfc2..4765987c5ab 100644 --- a/src/core/context-tracking/FileContextTracker.ts +++ b/src/core/context-tracking/FileContextTracker.ts @@ -206,6 +206,42 @@ export class FileContextTracker { return files } + /** + * Gets a list of unique file paths that Roo has read during this task. + * Optionally filters to only include files read after a specific timestamp. + * + * @param sinceTimestamp - Optional timestamp to filter files read after this time + * @returns Array of unique file paths that have been read + */ + async getFilesReadByRoo(sinceTimestamp?: number): Promise { + try { + const metadata = await this.getTaskMetadata(this.taskId) + + const readFiles = metadata.files_in_context + .filter((entry) => { + // Only include files that were read by Roo (not user edits) + const isReadByRoo = entry.record_source === "read_tool" || entry.record_source === "file_mentioned" + if (!isReadByRoo) { + return false + } + + // If sinceTimestamp is provided, only include files read after that time + if (sinceTimestamp && entry.roo_read_date) { + return entry.roo_read_date >= sinceTimestamp + } + + return true + }) + .map((entry) => entry.path) + + // Return unique file paths (same file may have multiple entries) + return [...new Set(readFiles)] + } catch (error) { + console.error("Failed to get files read by Roo:", error) + return [] + } + } + getAndClearCheckpointPossibleFile(): string[] { const files = Array.from(this.checkpointPossibleFiles) this.checkpointPossibleFiles.clear() diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 11eebde78fb..9eb8ea3ba23 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -125,7 +125,12 @@ import { checkpointDiff, } from "../checkpoints" import { processUserContentMentions } from "../mentions/processUserContentMentions" -import { getMessagesSinceLastSummary, summarizeConversation, getEffectiveApiHistory } from "../condense" +import { + getMessagesSinceLastSummary, + summarizeConversation, + getEffectiveApiHistory, + generateFoldedFileContext, +} from "../condense" import { MessageQueueService } from "../message-queue/MessageQueueService" import { AutoApprovalHandler, checkAutoApproval } from "../auto-approval" import { MessageManager } from "../message-manager" @@ -1623,6 +1628,25 @@ export class Task extends EventEmitter implements TaskLike { // Generate environment details to include in the condensed summary const environmentDetails = await getEnvironmentDetails(this, true) + // Generate folded file context from files read by Roo during this task + // Each file gets its own block as a separate content block + let foldedFileContextSections: string[] | undefined + try { + const filesReadByRoo = await this.fileContextTracker.getFilesReadByRoo() + if (filesReadByRoo.length > 0) { + const foldedResult = await generateFoldedFileContext(filesReadByRoo, { + cwd: this.cwd, + rooIgnoreController: this.rooIgnoreController, + }) + if (foldedResult.sections.length > 0) { + foldedFileContextSections = foldedResult.sections + } + } + } catch (error) { + console.error("[Task#condenseContext] Failed to generate folded file context:", error) + // Continue without folded context - it's not critical + } + const { messages, summary, @@ -1640,6 +1664,7 @@ export class Task extends EventEmitter implements TaskLike { customCondensingPrompt, // User's custom prompt metadata, // Pass metadata with tools environmentDetails, // Include environment details in summary + foldedFileContextSections, // Include folded file context (each file in its own block) ) if (error) { await this.say( From 3bde3cd9dfc5bf308d460138074a43734aa57114 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Sat, 24 Jan 2026 19:32:42 +0000 Subject: [PATCH 2/6] fix: skip tree-sitter error strings in folded file context - Add isTreeSitterErrorString helper to detect error messages - Skip files that return error strings instead of embedding them - Add test for error string handling --- .../__tests__/foldedFileContext.spec.ts | 26 +++++++++++++++++++ src/core/condense/foldedFileContext.ts | 15 +++++++++-- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/src/core/condense/__tests__/foldedFileContext.spec.ts b/src/core/condense/__tests__/foldedFileContext.spec.ts index 4f9b9a8350c..1238b757604 100644 --- a/src/core/condense/__tests__/foldedFileContext.spec.ts +++ b/src/core/condense/__tests__/foldedFileContext.spec.ts @@ -94,6 +94,32 @@ describe("foldedFileContext", () => { expect(result.filesSkipped).toBe(1) }) + it("should skip files when parseSourceCodeDefinitions returns error strings", async () => { + // Tree-sitter can return error strings for missing or denied files + // These should be treated as skipped, not embedded in the output + mockedParseSourceCodeDefinitions + .mockResolvedValueOnce("1--3 | export const x = 1") + .mockResolvedValueOnce("This file does not exist or you do not have permission to access it.") + .mockResolvedValueOnce("Unsupported file type: /test/file.xyz") + + const result = await generateFoldedFileContext(["/test/valid.ts", "/test/missing.ts", "/test/file.xyz"], { + cwd: "/test", + }) + + // Only the first file should be processed, the other two return error strings + expect(result.filesProcessed).toBe(1) + expect(result.filesSkipped).toBe(2) + + // The content should NOT contain the error messages + expect(result.content).not.toContain("does not exist") + expect(result.content).not.toContain("do not have permission") + expect(result.content).not.toContain("Unsupported file type") + + // But it should contain the valid file's content + expect(result.content).toContain("## File Context: /test/valid.ts") + expect(result.content).toContain("export const x = 1") + }) + it("should respect character budget limit", async () => { // Create multiple files that would exceed a small budget const longDefinitions = `1--3 | export function longFunctionName1() diff --git a/src/core/condense/foldedFileContext.ts b/src/core/condense/foldedFileContext.ts index db2ad4d380d..354c8fa2a86 100644 --- a/src/core/condense/foldedFileContext.ts +++ b/src/core/condense/foldedFileContext.ts @@ -2,6 +2,17 @@ import * as path from "path" import { parseSourceCodeDefinitionsForFile } from "../../services/tree-sitter" import { RooIgnoreController } from "../ignore/RooIgnoreController" +/** + * Checks if a definitions string is actually an error message from tree-sitter + * rather than valid code definitions. These error strings should not be embedded + * in the folded file context - instead, the file should be skipped. + */ +function isTreeSitterErrorString(definitions: string): boolean { + // These are known error messages from parseSourceCodeDefinitionsForFile + const errorPatterns = ["This file does not exist", "do not have permission", "Unsupported file type:"] + return errorPatterns.some((pattern) => definitions.includes(pattern)) +} + /** * Result of generating folded file context. */ @@ -91,8 +102,8 @@ export async function generateFoldedFileContext( // Get the folded definitions using tree-sitter const definitions = await parseSourceCodeDefinitionsForFile(absolutePath, rooIgnoreController) - if (!definitions) { - // File type not supported or no definitions found + if (!definitions || isTreeSitterErrorString(definitions)) { + // File type not supported, no definitions found, or error accessing file result.filesSkipped++ continue } From ef960eb657bd0e80fa5e555a07b969e392da8eca Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Mon, 26 Jan 2026 12:45:09 -0700 Subject: [PATCH 3/6] refactor: move generateFoldedFileContext() inside summarizeConversation() - Update summarizeConversation() to accept filesReadByRoo, cwd, rooIgnoreController instead of pre-generated sections - Move folded file context generation inside summarizeConversation() (lines 319-339) - Update ContextManagementOptions type and manageContext() to pass new parameters - Remove generateFoldedFileContext from Task.ts imports - folding now handled internally - Update all tests to use new parameter signature - Reduces Task.ts complexity by moving folding logic to summarization module --- .../__tests__/foldedFileContext.spec.ts | 48 +++- src/core/condense/index.ts | 39 ++- .../__tests__/context-management.spec.ts | 228 ++++++++++++++++++ src/core/context-management/index.ts | 13 + src/core/task/Task.ts | 46 ++-- 5 files changed, 336 insertions(+), 38 deletions(-) diff --git a/src/core/condense/__tests__/foldedFileContext.spec.ts b/src/core/condense/__tests__/foldedFileContext.spec.ts index 1238b757604..a805d26c75b 100644 --- a/src/core/condense/__tests__/foldedFileContext.spec.ts +++ b/src/core/condense/__tests__/foldedFileContext.spec.ts @@ -11,9 +11,20 @@ vi.mock("../../../services/tree-sitter", () => ({ parseSourceCodeDefinitionsForFile: vi.fn(), })) +// Mock generateFoldedFileContext for summarizeConversation tests +vi.mock("../foldedFileContext", async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + generateFoldedFileContext: vi.fn().mockImplementation(actual.generateFoldedFileContext), + } +}) + import { generateFoldedFileContext } from "../foldedFileContext" import { parseSourceCodeDefinitionsForFile } from "../../../services/tree-sitter" +const mockedGenerateFoldedFileContext = vi.mocked(generateFoldedFileContext) + const mockedParseSourceCodeDefinitions = vi.mocked(parseSourceCodeDefinitionsForFile) describe("foldedFileContext", () => { @@ -262,8 +273,8 @@ describe("foldedFileContext", () => { { role: "user", content: "Seventh message" }, ] - // Folded file context sections - each file in its own block - const foldedFileContextSections = [ + // Mock generateFoldedFileContext to return the expected folded sections + const mockFoldedSections = [ ` ## File Context: src/user.ts 1--5 | export interface User @@ -276,6 +287,17 @@ describe("foldedFileContext", () => { `, ] + mockedGenerateFoldedFileContext.mockResolvedValue({ + content: mockFoldedSections.join("\n"), + sections: mockFoldedSections, + filesProcessed: 2, + filesSkipped: 0, + characterCount: mockFoldedSections.join("\n").length, + }) + + const filesReadByRoo = ["src/user.ts", "src/api.ts"] + const cwd = "/test/project" + const result = await summarizeConversation( messages, mockApiHandler, @@ -285,9 +307,17 @@ describe("foldedFileContext", () => { undefined, // customCondensingPrompt undefined, // metadata undefined, // environmentDetails - foldedFileContextSections, // Array of sections, each file in its own block + filesReadByRoo, + cwd, + undefined, // rooIgnoreController ) + // Verify generateFoldedFileContext was called with the right arguments + expect(mockedGenerateFoldedFileContext).toHaveBeenCalledWith(filesReadByRoo, { + cwd, + rooIgnoreController: undefined, + }) + // Verify the summary was created expect(result.summary).toBeDefined() expect(result.messages.length).toBeGreaterThan(0) @@ -318,7 +348,7 @@ describe("foldedFileContext", () => { expect(apiFileBlock.text).toContain("fetchData") }) - it("should not include file context section when foldedFileContextSections is empty", async () => { + it("should not include file context section when filesReadByRoo is empty", async () => { const { summarizeConversation } = await import("../index") const mockApiHandler = new MockApiHandler() @@ -334,6 +364,9 @@ describe("foldedFileContext", () => { { role: "user", content: "Seventh message" }, ] + // Reset the mock to ensure clean state + mockedGenerateFoldedFileContext.mockClear() + const result = await summarizeConversation( messages, mockApiHandler, @@ -343,9 +376,14 @@ describe("foldedFileContext", () => { undefined, // customCondensingPrompt undefined, // metadata undefined, // environmentDetails - [], // Empty foldedFileContextSections array + [], // Empty filesReadByRoo array + "/test/project", + undefined, // rooIgnoreController ) + // generateFoldedFileContext should NOT be called when filesReadByRoo is empty + expect(mockedGenerateFoldedFileContext).not.toHaveBeenCalled() + // Find the summary message const summaryMessage = result.messages.find((msg: any) => msg.isSummary) expect(summaryMessage).toBeDefined() diff --git a/src/core/condense/index.ts b/src/core/condense/index.ts index de61b3c018e..b31278174ba 100644 --- a/src/core/condense/index.ts +++ b/src/core/condense/index.ts @@ -9,9 +9,11 @@ import { ApiMessage } from "../task-persistence/apiMessages" import { maybeRemoveImageBlocks } from "../../api/transform/image-cleaning" import { findLast } from "../../shared/array" import { supportPrompt } from "../../shared/support-prompt" +import { RooIgnoreController } from "../ignore/RooIgnoreController" // Re-export folded file context utilities -export { generateFoldedFileContext } from "./foldedFileContext" +import { generateFoldedFileContext } from "./foldedFileContext" +export { generateFoldedFileContext } export type { FoldedFileContextResult, FoldedFileContextOptions } from "./foldedFileContext" export const MIN_CONDENSE_THRESHOLD = 5 // Minimum percentage of context window to trigger condensing @@ -153,7 +155,9 @@ export type SummarizeResponse = { * @param {string} customCondensingPrompt - Optional custom prompt to use for condensing * @param {ApiHandlerCreateMessageMetadata} metadata - Optional metadata to pass to createMessage (tools, taskId, etc.) * @param {string} environmentDetails - Optional environment details string to include in the summary (only used when isAutomaticTrigger=true) - * @param {string[]} foldedFileContextSections - Optional array of folded file context sections (each file in its own block) + * @param {string[]} filesReadByRoo - Optional array of file paths read by Roo during the task (will be folded via tree-sitter) + * @param {string} cwd - Optional current working directory for resolving file paths (required if filesReadByRoo is provided) + * @param {RooIgnoreController} rooIgnoreController - Optional controller for file access validation * @returns {SummarizeResponse} - The result of the summarization operation (see above) */ export async function summarizeConversation( @@ -165,7 +169,9 @@ export async function summarizeConversation( customCondensingPrompt?: string, metadata?: ApiHandlerCreateMessageMetadata, environmentDetails?: string, - foldedFileContextSections?: string[], + filesReadByRoo?: string[], + cwd?: string, + rooIgnoreController?: RooIgnoreController, ): Promise { TelemetryService.instance.captureContextCondensed( taskId, @@ -308,16 +314,27 @@ ${commandBlocks} }) } - // Add folded file context (smart code folding) if present + // Generate and add folded file context (smart code folding) if file paths are provided // Each file gets its own block as a separate content block - if (foldedFileContextSections && foldedFileContextSections.length > 0) { - for (const section of foldedFileContextSections) { - if (section.trim()) { - summaryContent.push({ - type: "text", - text: section, - }) + if (filesReadByRoo && filesReadByRoo.length > 0 && cwd) { + try { + const foldedResult = await generateFoldedFileContext(filesReadByRoo, { + cwd, + rooIgnoreController, + }) + if (foldedResult.sections.length > 0) { + for (const section of foldedResult.sections) { + if (section.trim()) { + summaryContent.push({ + type: "text", + text: section, + }) + } + } } + } catch (error) { + console.error("[summarizeConversation] Failed to generate folded file context:", error) + // Continue without folded context - non-critical failure } } diff --git a/src/core/context-management/__tests__/context-management.spec.ts b/src/core/context-management/__tests__/context-management.spec.ts index 2616ea571df..c90ff31d1dd 100644 --- a/src/core/context-management/__tests__/context-management.spec.ts +++ b/src/core/context-management/__tests__/context-management.spec.ts @@ -621,6 +621,9 @@ describe("Context Management", () => { undefined, // customCondensingPrompt undefined, // metadata undefined, // environmentDetails + undefined, // filesReadByRoo + undefined, // cwd + undefined, // rooIgnoreController ) // Verify the result contains the summary information @@ -796,6 +799,9 @@ describe("Context Management", () => { undefined, // customCondensingPrompt undefined, // metadata undefined, // environmentDetails + undefined, // filesReadByRoo + undefined, // cwd + undefined, // rooIgnoreController ) // Verify the result contains the summary information @@ -854,6 +860,228 @@ describe("Context Management", () => { }) }) + /** + * Tests for filesReadByRoo being passed to summarizeConversation + */ + describe("filesReadByRoo parameters", () => { + const createModelInfo = (contextWindow: number, maxTokens?: number): ModelInfo => ({ + contextWindow, + supportsPromptCache: true, + maxTokens, + }) + + const messages: ApiMessage[] = [ + { role: "user", content: "First message" }, + { role: "assistant", content: "Second message" }, + { role: "user", content: "Third message" }, + { role: "assistant", content: "Fourth message" }, + { role: "user", content: "Fifth message" }, + ] + + it("should pass filesReadByRoo, cwd, and rooIgnoreController to summarizeConversation when provided", async () => { + // Mock the summarizeConversation function + const mockSummary = "Summary with folded context" + const mockCost = 0.05 + const mockSummarizeResponse: condenseModule.SummarizeResponse = { + messages: [ + { role: "user", content: "First message" }, + { role: "assistant", content: mockSummary, isSummary: true }, + { role: "user", content: "Last message" }, + ], + summary: mockSummary, + cost: mockCost, + newContextTokens: 100, + } + + const summarizeSpy = vi + .spyOn(condenseModule, "summarizeConversation") + .mockResolvedValue(mockSummarizeResponse) + + const modelInfo = createModelInfo(100000, 30000) + const totalTokens = 70001 // Above threshold + const messagesWithSmallContent = [ + ...messages.slice(0, -1), + { ...messages[messages.length - 1], content: "" }, + ] + + const filesReadByRoo = ["src/test.ts", "src/utils.ts"] + const cwd = "/test/project" + const mockRooIgnoreController = { + filterPaths: vi.fn(), + } as unknown as import("../../ignore/RooIgnoreController").RooIgnoreController + + const result = await manageContext({ + messages: messagesWithSmallContent, + totalTokens, + contextWindow: modelInfo.contextWindow, + maxTokens: modelInfo.maxTokens, + apiHandler: mockApiHandler, + autoCondenseContext: true, + autoCondenseContextPercent: 100, + systemPrompt: "System prompt", + taskId, + profileThresholds: {}, + currentProfileId: "default", + filesReadByRoo, + cwd, + rooIgnoreController: mockRooIgnoreController, + }) + + // Verify summarizeConversation was called with filesReadByRoo, cwd, and rooIgnoreController + expect(summarizeSpy).toHaveBeenCalledWith( + messagesWithSmallContent, + mockApiHandler, + "System prompt", + taskId, + true, // isAutomaticTrigger + undefined, // customCondensingPrompt + undefined, // metadata + undefined, // environmentDetails + filesReadByRoo, + cwd, + mockRooIgnoreController, + ) + + // Verify the result contains the summary information + expect(result).toMatchObject({ + messages: mockSummarizeResponse.messages, + summary: mockSummary, + cost: mockCost, + prevContextTokens: totalTokens, + }) + + // Clean up + summarizeSpy.mockRestore() + }) + + it("should pass undefined filesReadByRoo parameters when not provided", async () => { + // Mock the summarizeConversation function + const mockSummary = "Summary without folded context" + const mockCost = 0.03 + const mockSummarizeResponse: condenseModule.SummarizeResponse = { + messages: [ + { role: "user", content: "First message" }, + { role: "assistant", content: mockSummary, isSummary: true }, + { role: "user", content: "Last message" }, + ], + summary: mockSummary, + cost: mockCost, + newContextTokens: 80, + } + + const summarizeSpy = vi + .spyOn(condenseModule, "summarizeConversation") + .mockResolvedValue(mockSummarizeResponse) + + const modelInfo = createModelInfo(100000, 30000) + const totalTokens = 70001 // Above threshold + const messagesWithSmallContent = [ + ...messages.slice(0, -1), + { ...messages[messages.length - 1], content: "" }, + ] + + const result = await manageContext({ + messages: messagesWithSmallContent, + totalTokens, + contextWindow: modelInfo.contextWindow, + maxTokens: modelInfo.maxTokens, + apiHandler: mockApiHandler, + autoCondenseContext: true, + autoCondenseContextPercent: 100, + systemPrompt: "System prompt", + taskId, + profileThresholds: {}, + currentProfileId: "default", + // filesReadByRoo, cwd, rooIgnoreController are NOT provided + }) + + // Verify summarizeConversation was called with undefined parameters + expect(summarizeSpy).toHaveBeenCalledWith( + messagesWithSmallContent, + mockApiHandler, + "System prompt", + taskId, + true, // isAutomaticTrigger + undefined, // customCondensingPrompt + undefined, // metadata + undefined, // environmentDetails + undefined, // filesReadByRoo should be undefined when not provided + undefined, // cwd should be undefined when not provided + undefined, // rooIgnoreController should be undefined when not provided + ) + + // Verify the result + expect(result).toMatchObject({ + summary: mockSummary, + cost: mockCost, + }) + + // Clean up + summarizeSpy.mockRestore() + }) + + it("should pass empty array filesReadByRoo when provided as empty", async () => { + // Mock the summarizeConversation function + const mockSummary = "Summary with empty file list" + const mockCost = 0.04 + const mockSummarizeResponse: condenseModule.SummarizeResponse = { + messages: [ + { role: "user", content: "First message" }, + { role: "assistant", content: mockSummary, isSummary: true }, + { role: "user", content: "Last message" }, + ], + summary: mockSummary, + cost: mockCost, + newContextTokens: 90, + } + + const summarizeSpy = vi + .spyOn(condenseModule, "summarizeConversation") + .mockResolvedValue(mockSummarizeResponse) + + const modelInfo = createModelInfo(100000, 30000) + const totalTokens = 70001 // Above threshold + const messagesWithSmallContent = [ + ...messages.slice(0, -1), + { ...messages[messages.length - 1], content: "" }, + ] + + const result = await manageContext({ + messages: messagesWithSmallContent, + totalTokens, + contextWindow: modelInfo.contextWindow, + maxTokens: modelInfo.maxTokens, + apiHandler: mockApiHandler, + autoCondenseContext: true, + autoCondenseContextPercent: 100, + systemPrompt: "System prompt", + taskId, + profileThresholds: {}, + currentProfileId: "default", + filesReadByRoo: [], // Empty array + cwd: "/test/project", + }) + + // Verify summarizeConversation was called with empty array + expect(summarizeSpy).toHaveBeenCalledWith( + messagesWithSmallContent, + mockApiHandler, + "System prompt", + taskId, + true, // isAutomaticTrigger + undefined, // customCondensingPrompt + undefined, // metadata + undefined, // environmentDetails + [], // Empty array should be passed through + "/test/project", + undefined, // rooIgnoreController + ) + + // Clean up + summarizeSpy.mockRestore() + }) + }) + /** * Tests for profile-specific thresholds functionality */ diff --git a/src/core/context-management/index.ts b/src/core/context-management/index.ts index 250dbeccbe7..e77e69d3041 100644 --- a/src/core/context-management/index.ts +++ b/src/core/context-management/index.ts @@ -7,6 +7,7 @@ import { ApiHandler, ApiHandlerCreateMessageMetadata } from "../../api" import { MAX_CONDENSE_THRESHOLD, MIN_CONDENSE_THRESHOLD, summarizeConversation, SummarizeResponse } from "../condense" import { ApiMessage } from "../task-persistence/apiMessages" import { ANTHROPIC_DEFAULT_MAX_TOKENS } from "@roo-code/types" +import { RooIgnoreController } from "../ignore/RooIgnoreController" /** * Context Management @@ -222,6 +223,12 @@ export type ContextManagementOptions = { metadata?: ApiHandlerCreateMessageMetadata /** Optional environment details string to include in the condensed summary */ environmentDetails?: string + /** Optional array of file paths read by Roo during the task (will be folded via tree-sitter) */ + filesReadByRoo?: string[] + /** Optional current working directory for resolving file paths (required if filesReadByRoo is provided) */ + cwd?: string + /** Optional controller for file access validation */ + rooIgnoreController?: RooIgnoreController } export type ContextManagementResult = SummarizeResponse & { @@ -252,6 +259,9 @@ export async function manageContext({ currentProfileId, metadata, environmentDetails, + filesReadByRoo, + cwd, + rooIgnoreController, }: ContextManagementOptions): Promise { let error: string | undefined let errorDetails: string | undefined @@ -306,6 +316,9 @@ export async function manageContext({ customCondensingPrompt, metadata, environmentDetails, + filesReadByRoo, + cwd, + rooIgnoreController, ) if (result.error) { error = result.error diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 9eb8ea3ba23..8ac021e345d 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -125,12 +125,7 @@ import { checkpointDiff, } from "../checkpoints" import { processUserContentMentions } from "../mentions/processUserContentMentions" -import { - getMessagesSinceLastSummary, - summarizeConversation, - getEffectiveApiHistory, - generateFoldedFileContext, -} from "../condense" +import { getMessagesSinceLastSummary, summarizeConversation, getEffectiveApiHistory } from "../condense" import { MessageQueueService } from "../message-queue/MessageQueueService" import { AutoApprovalHandler, checkAutoApproval } from "../auto-approval" import { MessageManager } from "../message-manager" @@ -1628,23 +1623,13 @@ export class Task extends EventEmitter implements TaskLike { // Generate environment details to include in the condensed summary const environmentDetails = await getEnvironmentDetails(this, true) - // Generate folded file context from files read by Roo during this task - // Each file gets its own block as a separate content block - let foldedFileContextSections: string[] | undefined + // Get files read by Roo for code folding - the actual folding happens inside summarizeConversation() + let filesReadByRoo: string[] | undefined try { - const filesReadByRoo = await this.fileContextTracker.getFilesReadByRoo() - if (filesReadByRoo.length > 0) { - const foldedResult = await generateFoldedFileContext(filesReadByRoo, { - cwd: this.cwd, - rooIgnoreController: this.rooIgnoreController, - }) - if (foldedResult.sections.length > 0) { - foldedFileContextSections = foldedResult.sections - } - } + filesReadByRoo = await this.fileContextTracker.getFilesReadByRoo() } catch (error) { - console.error("[Task#condenseContext] Failed to generate folded file context:", error) - // Continue without folded context - it's not critical + console.error("[Task#condenseContext] Failed to get files read by Roo:", error) + // Continue without file context - it's not critical } const { @@ -1664,7 +1649,9 @@ export class Task extends EventEmitter implements TaskLike { customCondensingPrompt, // User's custom prompt metadata, // Pass metadata with tools environmentDetails, // Include environment details in summary - foldedFileContextSections, // Include folded file context (each file in its own block) + filesReadByRoo, // Files to fold - folding happens inside summarizeConversation + this.cwd, + this.rooIgnoreController, ) if (error) { await this.say( @@ -4064,6 +4051,18 @@ export class Task extends EventEmitter implements TaskLike { ? await getEnvironmentDetails(this, true) : undefined + // Get files read by Roo for code folding - only when context management will run + // The actual folding will happen inside summarizeConversation() for cleaner architecture + let contextMgmtFilesReadByRoo: string[] | undefined + if (contextManagementWillRun && autoCondenseContext) { + try { + contextMgmtFilesReadByRoo = await this.fileContextTracker.getFilesReadByRoo() + } catch (error) { + console.error("[Task#attemptApiRequest] Failed to get files read by Roo:", error) + // Continue without file context - it's not critical + } + } + try { const truncateResult = await manageContext({ messages: this.apiConversationHistory, @@ -4080,6 +4079,9 @@ export class Task extends EventEmitter implements TaskLike { currentProfileId, metadata: contextMgmtMetadata, environmentDetails: contextMgmtEnvironmentDetails, + filesReadByRoo: contextMgmtFilesReadByRoo, + cwd: this.cwd, + rooIgnoreController: this.rooIgnoreController, }) if (truncateResult.messages !== this.apiConversationHistory) { await this.overwriteApiConversationHistory(truncateResult.messages) From 0f4e7b77de4106f8920af046c315b325308d37ff Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Mon, 26 Jan 2026 13:37:39 -0700 Subject: [PATCH 4/6] fix: prioritize most recently read files in folded context Files are now sorted by roo_read_date descending before folded context generation, so if the character budget runs out, the most relevant (recently read) files are included and older files are skipped. --- .../context-tracking/FileContextTracker.ts | 59 ++++++++++++------- 1 file changed, 38 insertions(+), 21 deletions(-) diff --git a/src/core/context-tracking/FileContextTracker.ts b/src/core/context-tracking/FileContextTracker.ts index 4765987c5ab..4c5640afdfb 100644 --- a/src/core/context-tracking/FileContextTracker.ts +++ b/src/core/context-tracking/FileContextTracker.ts @@ -208,34 +208,51 @@ export class FileContextTracker { /** * Gets a list of unique file paths that Roo has read during this task. - * Optionally filters to only include files read after a specific timestamp. + * Files are sorted by most recently read first, so if there's a character + * budget during folded context generation, the most relevant (recent) files + * are prioritized. * * @param sinceTimestamp - Optional timestamp to filter files read after this time - * @returns Array of unique file paths that have been read + * @returns Array of unique file paths that have been read, most recent first */ async getFilesReadByRoo(sinceTimestamp?: number): Promise { try { const metadata = await this.getTaskMetadata(this.taskId) - const readFiles = metadata.files_in_context - .filter((entry) => { - // Only include files that were read by Roo (not user edits) - const isReadByRoo = entry.record_source === "read_tool" || entry.record_source === "file_mentioned" - if (!isReadByRoo) { - return false - } - - // If sinceTimestamp is provided, only include files read after that time - if (sinceTimestamp && entry.roo_read_date) { - return entry.roo_read_date >= sinceTimestamp - } - - return true - }) - .map((entry) => entry.path) - - // Return unique file paths (same file may have multiple entries) - return [...new Set(readFiles)] + const readEntries = metadata.files_in_context.filter((entry) => { + // Only include files that were read by Roo (not user edits) + const isReadByRoo = entry.record_source === "read_tool" || entry.record_source === "file_mentioned" + if (!isReadByRoo) { + return false + } + + // If sinceTimestamp is provided, only include files read after that time + if (sinceTimestamp && entry.roo_read_date) { + return entry.roo_read_date >= sinceTimestamp + } + + return true + }) + + // Sort by roo_read_date descending (most recent first) + // Entries without a date go to the end + readEntries.sort((a, b) => { + const dateA = a.roo_read_date ?? 0 + const dateB = b.roo_read_date ?? 0 + return dateB - dateA + }) + + // Deduplicate while preserving order (first occurrence = most recent read) + const seen = new Set() + const uniquePaths: string[] = [] + for (const entry of readEntries) { + if (!seen.has(entry.path)) { + seen.add(entry.path) + uniquePaths.push(entry.path) + } + } + + return uniquePaths } catch (error) { console.error("Failed to get files read by Roo:", error) return [] From bfb0d5fce9e3356f0292f2552047a13919e23c6b Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Mon, 26 Jan 2026 16:54:03 -0500 Subject: [PATCH 5/6] 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 --- src/core/condense/__tests__/condense.spec.ts | 56 ++++++- .../__tests__/foldedFileContext.spec.ts | 32 ++-- src/core/condense/__tests__/index.spec.ts | 156 +++++++++++++----- src/core/condense/index.ts | 58 +++---- .../__tests__/context-management.spec.ts | 91 ++++------ src/core/context-management/index.ts | 6 +- src/core/task/Task.ts | 58 +++---- 7 files changed, 264 insertions(+), 193 deletions(-) diff --git a/src/core/condense/__tests__/condense.spec.ts b/src/core/condense/__tests__/condense.spec.ts index 9d3352d01a5..c209fa97243 100644 --- a/src/core/condense/__tests__/condense.spec.ts +++ b/src/core/condense/__tests__/condense.spec.ts @@ -136,7 +136,13 @@ Line 2 { role: "user", content: "Ninth message" }, ] - const result = await summarizeConversation(messages, mockApiHandler, "System prompt", taskId, false) + const result = await summarizeConversation({ + messages, + apiHandler: mockApiHandler, + systemPrompt: "System prompt", + taskId, + isAutomaticTrigger: false, + }) // Verify we have a summary message with role "user" (fresh start model) const summaryMessage = result.messages.find((msg) => msg.isSummary) @@ -164,7 +170,13 @@ Line 2 { role: "user", content: "Fifth message" }, ] - const result = await summarizeConversation(messages, mockApiHandler, "System prompt", taskId, false) + const result = await summarizeConversation({ + messages, + apiHandler: mockApiHandler, + systemPrompt: "System prompt", + taskId, + isAutomaticTrigger: false, + }) // All original messages should be tagged with condenseParent const taggedMessages = result.messages.filter((msg) => !msg.isSummary) @@ -193,7 +205,13 @@ Line 2 { role: "user", content: "Ninth message" }, ] - const result = await summarizeConversation(messages, mockApiHandler, "System prompt", taskId, false) + const result = await summarizeConversation({ + messages, + apiHandler: mockApiHandler, + systemPrompt: "System prompt", + taskId, + isAutomaticTrigger: false, + }) const summaryMessage = result.messages.find((msg) => msg.isSummary) expect(summaryMessage).toBeTruthy() @@ -227,7 +245,13 @@ Line 2 { role: "user", content: "Perfect!" }, ] - const result = await summarizeConversation(messages, mockApiHandler, "System prompt", taskId, false) + const result = await summarizeConversation({ + messages, + apiHandler: mockApiHandler, + systemPrompt: "System prompt", + taskId, + isAutomaticTrigger: false, + }) // Effective history should contain only the summary (fresh start) const effectiveHistory = getEffectiveApiHistory(result.messages) @@ -239,7 +263,13 @@ Line 2 it("should return error when not enough messages to summarize", async () => { const messages: ApiMessage[] = [{ role: "user", content: "Only one message" }] - const result = await summarizeConversation(messages, mockApiHandler, "System prompt", taskId, false) + const result = await summarizeConversation({ + messages, + apiHandler: mockApiHandler, + systemPrompt: "System prompt", + taskId, + isAutomaticTrigger: false, + }) // Should return an error since we have only 1 message expect(result.error).toBeDefined() @@ -253,7 +283,13 @@ Line 2 { role: "user", content: "Previous summary", isSummary: true }, ] - const result = await summarizeConversation(messages, mockApiHandler, "System prompt", taskId, false) + const result = await summarizeConversation({ + messages, + apiHandler: mockApiHandler, + systemPrompt: "System prompt", + taskId, + isAutomaticTrigger: false, + }) // Should return an error due to recent summary with no substantial messages after expect(result.error).toBeDefined() @@ -286,7 +322,13 @@ Line 2 { role: "user", content: "Seventh" }, ] - const result = await summarizeConversation(messages, emptyHandler, "System prompt", taskId, false) + const result = await summarizeConversation({ + messages, + apiHandler: emptyHandler, + systemPrompt: "System prompt", + taskId, + isAutomaticTrigger: false, + }) expect(result.error).toBeDefined() expect(result.messages).toEqual(messages) diff --git a/src/core/condense/__tests__/foldedFileContext.spec.ts b/src/core/condense/__tests__/foldedFileContext.spec.ts index a805d26c75b..3bd9b390f5a 100644 --- a/src/core/condense/__tests__/foldedFileContext.spec.ts +++ b/src/core/condense/__tests__/foldedFileContext.spec.ts @@ -298,19 +298,15 @@ describe("foldedFileContext", () => { const filesReadByRoo = ["src/user.ts", "src/api.ts"] const cwd = "/test/project" - const result = await summarizeConversation( + const result = await summarizeConversation({ messages, - mockApiHandler, - "System prompt", + apiHandler: mockApiHandler, + systemPrompt: "System prompt", taskId, - false, - undefined, // customCondensingPrompt - undefined, // metadata - undefined, // environmentDetails + isAutomaticTrigger: false, filesReadByRoo, cwd, - undefined, // rooIgnoreController - ) + }) // Verify generateFoldedFileContext was called with the right arguments expect(mockedGenerateFoldedFileContext).toHaveBeenCalledWith(filesReadByRoo, { @@ -367,19 +363,15 @@ describe("foldedFileContext", () => { // Reset the mock to ensure clean state mockedGenerateFoldedFileContext.mockClear() - const result = await summarizeConversation( + const result = await summarizeConversation({ messages, - mockApiHandler, - "System prompt", + apiHandler: mockApiHandler, + systemPrompt: "System prompt", taskId, - false, - undefined, // customCondensingPrompt - undefined, // metadata - undefined, // environmentDetails - [], // Empty filesReadByRoo array - "/test/project", - undefined, // rooIgnoreController - ) + isAutomaticTrigger: false, + filesReadByRoo: [], + cwd: "/test/project", + }) // generateFoldedFileContext should NOT be called when filesReadByRoo is empty expect(mockedGenerateFoldedFileContext).not.toHaveBeenCalled() diff --git a/src/core/condense/__tests__/index.spec.ts b/src/core/condense/__tests__/index.spec.ts index c8bc5ee8ef1..75190985dba 100644 --- a/src/core/condense/__tests__/index.spec.ts +++ b/src/core/condense/__tests__/index.spec.ts @@ -710,7 +710,12 @@ describe("summarizeConversation", () => { it("should not summarize when there are not enough messages", async () => { const messages: ApiMessage[] = [{ role: "user", content: "Hello", ts: 1 }] - const result = await summarizeConversation(messages, mockApiHandler, defaultSystemPrompt, taskId) + const result = await summarizeConversation({ + messages, + apiHandler: mockApiHandler, + systemPrompt: defaultSystemPrompt, + taskId, + }) expect(result.messages).toEqual(messages) expect(result.cost).toBe(0) expect(result.summary).toBe("") @@ -730,7 +735,12 @@ describe("summarizeConversation", () => { { role: "user", content: "Tell me more", ts: 7 }, ] - const result = await summarizeConversation(messages, mockApiHandler, defaultSystemPrompt, taskId) + const result = await summarizeConversation({ + messages, + apiHandler: mockApiHandler, + systemPrompt: defaultSystemPrompt, + taskId, + }) // Check that the API was called correctly expect(mockApiHandler.createMessage).toHaveBeenCalled() @@ -784,7 +794,12 @@ describe("summarizeConversation", () => { { role: "user", content: "What's new?", ts: 5 }, ] - const result = await summarizeConversation(messages, mockApiHandler, defaultSystemPrompt, taskId) + const result = await summarizeConversation({ + messages, + apiHandler: mockApiHandler, + systemPrompt: defaultSystemPrompt, + taskId, + }) const summaryMessage = result.messages.find((m) => m.isSummary) expect(summaryMessage).toBeDefined() @@ -807,7 +822,12 @@ describe("summarizeConversation", () => { { role: "user", content: "What's new?", ts: 5 }, ] - const result = await summarizeConversation(messages, mockApiHandler, defaultSystemPrompt, taskId) + const result = await summarizeConversation({ + messages, + apiHandler: mockApiHandler, + systemPrompt: defaultSystemPrompt, + taskId, + }) const summaryMessage = result.messages.find((m) => m.isSummary) expect(summaryMessage).toBeDefined() @@ -844,7 +864,12 @@ describe("summarizeConversation", () => { return messages.map(({ role, content }: { role: string; content: any }) => ({ role, content })) }) - const result = await summarizeConversation(messages, mockApiHandler, defaultSystemPrompt, taskId) + const result = await summarizeConversation({ + messages, + apiHandler: mockApiHandler, + systemPrompt: defaultSystemPrompt, + taskId, + }) // Should return original messages when summary is empty expect(result.messages).toEqual(messages) @@ -865,7 +890,12 @@ describe("summarizeConversation", () => { { role: "user", content: "Tell me more", ts: 7 }, ] - await summarizeConversation(messages, mockApiHandler, defaultSystemPrompt, taskId) + await summarizeConversation({ + messages, + apiHandler: mockApiHandler, + systemPrompt: defaultSystemPrompt, + taskId, + }) // Verify that createMessage was called with the SUMMARY_PROMPT (which contains CRITICAL instructions), messages array, and optional metadata expect(mockApiHandler.createMessage).toHaveBeenCalledWith( @@ -897,7 +927,12 @@ describe("summarizeConversation", () => { { role: "user", content: "Newest", ts: 7 }, ] - await summarizeConversation(messages, mockApiHandler, defaultSystemPrompt, taskId) + await summarizeConversation({ + messages, + apiHandler: mockApiHandler, + systemPrompt: defaultSystemPrompt, + taskId, + }) const mockCallArgs = (maybeRemoveImageBlocks as Mock).mock.calls[0][0] as any[] @@ -935,7 +970,12 @@ describe("summarizeConversation", () => { // Override the mock for this test mockApiHandler.createMessage = vi.fn().mockReturnValue(streamWithUsage) as any - const result = await summarizeConversation(messages, mockApiHandler, systemPrompt, taskId) + const result = await summarizeConversation({ + messages, + apiHandler: mockApiHandler, + systemPrompt, + taskId, + }) // Verify that countTokens was called with system prompt + summary message expect(mockApiHandler.countTokens).toHaveBeenCalled() @@ -970,7 +1010,12 @@ describe("summarizeConversation", () => { // Mock countTokens to return a small value mockApiHandler.countTokens = vi.fn().mockImplementation(() => Promise.resolve(30)) as any - const result = await summarizeConversation(messages, mockApiHandler, defaultSystemPrompt, taskId) + const result = await summarizeConversation({ + messages, + apiHandler: mockApiHandler, + systemPrompt: defaultSystemPrompt, + taskId, + }) // Result contains all messages plus summary expect(result.messages.length).toBe(messages.length + 1) @@ -1010,7 +1055,12 @@ describe("summarizeConversation", () => { const mockError = vi.fn() console.error = mockError - const result = await summarizeConversation(messages, invalidHandler, defaultSystemPrompt, taskId) + const result = await summarizeConversation({ + messages, + apiHandler: invalidHandler, + systemPrompt: defaultSystemPrompt, + taskId, + }) // Should return original messages when handler is invalid expect(result.messages).toEqual(messages) @@ -1035,7 +1085,12 @@ describe("summarizeConversation", () => { { role: "user", content: "Thanks", ts: 5 }, ] - const result = await summarizeConversation(messages, mockApiHandler, defaultSystemPrompt, taskId) + const result = await summarizeConversation({ + messages, + apiHandler: mockApiHandler, + systemPrompt: defaultSystemPrompt, + taskId, + }) const summaryMessage = result.messages.find((m) => m.isSummary) expect(summaryMessage).toBeDefined() @@ -1056,7 +1111,12 @@ describe("summarizeConversation", () => { { role: "user", content: "Thanks", ts: 5 }, ] - const result = await summarizeConversation(messages, mockApiHandler, defaultSystemPrompt, taskId) + const result = await summarizeConversation({ + messages, + apiHandler: mockApiHandler, + systemPrompt: defaultSystemPrompt, + taskId, + }) // Summary should be the last message const lastMessage = result.messages[result.messages.length - 1] @@ -1120,14 +1180,14 @@ describe("summarizeConversation with custom settings", () => { it("should use custom prompt when provided", async () => { const customPrompt = "Custom summarization prompt" - await summarizeConversation( - sampleMessages, - mockMainApiHandler, - defaultSystemPrompt, - localTaskId, - false, - customPrompt, - ) + await summarizeConversation({ + messages: sampleMessages, + apiHandler: mockMainApiHandler, + systemPrompt: defaultSystemPrompt, + taskId: localTaskId, + isAutomaticTrigger: false, + customCondensingPrompt: customPrompt, + }) // Verify the custom prompt was used in the user message content const createMessageCalls = (mockMainApiHandler.createMessage as Mock).mock.calls @@ -1144,7 +1204,14 @@ describe("summarizeConversation with custom settings", () => { */ it("should use default systemPrompt when custom prompt is empty or not provided", async () => { // Test with empty string - await summarizeConversation(sampleMessages, mockMainApiHandler, defaultSystemPrompt, localTaskId, false, " ") + await summarizeConversation({ + messages: sampleMessages, + apiHandler: mockMainApiHandler, + systemPrompt: defaultSystemPrompt, + taskId: localTaskId, + isAutomaticTrigger: false, + customCondensingPrompt: " ", + }) // Verify the default SUMMARY_PROMPT was used (contains CRITICAL instructions) let createMessageCalls = (mockMainApiHandler.createMessage as Mock).mock.calls @@ -1156,14 +1223,13 @@ describe("summarizeConversation with custom settings", () => { // Reset mock and test with undefined vi.clearAllMocks() - await summarizeConversation( - sampleMessages, - mockMainApiHandler, - defaultSystemPrompt, - localTaskId, - false, - undefined, - ) + await summarizeConversation({ + messages: sampleMessages, + apiHandler: mockMainApiHandler, + systemPrompt: defaultSystemPrompt, + taskId: localTaskId, + isAutomaticTrigger: false, + }) // Verify the default SUMMARY_PROMPT was used again (contains CRITICAL instructions) createMessageCalls = (mockMainApiHandler.createMessage as Mock).mock.calls @@ -1178,14 +1244,14 @@ describe("summarizeConversation with custom settings", () => { * Test that telemetry is called for custom prompt usage */ it("should capture telemetry when using custom prompt", async () => { - await summarizeConversation( - sampleMessages, - mockMainApiHandler, - defaultSystemPrompt, - localTaskId, - false, - "Custom prompt", - ) + await summarizeConversation({ + messages: sampleMessages, + apiHandler: mockMainApiHandler, + systemPrompt: defaultSystemPrompt, + taskId: localTaskId, + isAutomaticTrigger: false, + customCondensingPrompt: "Custom prompt", + }) // Verify telemetry was called with custom prompt flag expect(TelemetryService.instance.captureContextCondensed).toHaveBeenCalledWith( @@ -1199,14 +1265,14 @@ describe("summarizeConversation with custom settings", () => { * Test that telemetry is called with isAutomaticTrigger flag */ it("should capture telemetry with isAutomaticTrigger flag", async () => { - await summarizeConversation( - sampleMessages, - mockMainApiHandler, - defaultSystemPrompt, - localTaskId, - true, // isAutomaticTrigger - "Custom prompt", - ) + await summarizeConversation({ + messages: sampleMessages, + apiHandler: mockMainApiHandler, + systemPrompt: defaultSystemPrompt, + taskId: localTaskId, + isAutomaticTrigger: true, + customCondensingPrompt: "Custom prompt", + }) // Verify telemetry was called with isAutomaticTrigger flag expect(TelemetryService.instance.captureContextCondensed).toHaveBeenCalledWith( diff --git a/src/core/condense/index.ts b/src/core/condense/index.ts index b31278174ba..5a65f0a96ff 100644 --- a/src/core/condense/index.ts +++ b/src/core/condense/index.ts @@ -10,10 +10,8 @@ import { maybeRemoveImageBlocks } from "../../api/transform/image-cleaning" import { findLast } from "../../shared/array" import { supportPrompt } from "../../shared/support-prompt" import { RooIgnoreController } from "../ignore/RooIgnoreController" - -// Re-export folded file context utilities import { generateFoldedFileContext } from "./foldedFileContext" -export { generateFoldedFileContext } + export type { FoldedFileContextResult, FoldedFileContextOptions } from "./foldedFileContext" export const MIN_CONDENSE_THRESHOLD = 5 // Minimum percentage of context window to trigger condensing @@ -129,6 +127,20 @@ export type SummarizeResponse = { condenseId?: string // The unique ID of the created Summary message, for linking to condense_context clineMessage } +export type SummarizeConversationOptions = { + messages: ApiMessage[] + apiHandler: ApiHandler + systemPrompt: string + taskId: string + isAutomaticTrigger?: boolean + customCondensingPrompt?: string + metadata?: ApiHandlerCreateMessageMetadata + environmentDetails?: string + filesReadByRoo?: string[] + cwd?: string + rooIgnoreController?: RooIgnoreController +} + /** * Summarizes the conversation messages using an LLM call. * @@ -146,33 +158,21 @@ export type SummarizeResponse = { * - For MANUAL condensing (isAutomaticTrigger=false): Environment details are NOT included * because fresh environment details will be injected on the very next turn via * getEnvironmentDetails() in recursivelyMakeClineRequests(). - * - * @param {ApiMessage[]} messages - The conversation messages - * @param {ApiHandler} apiHandler - The API handler to use for summarization and token counting - * @param {string} systemPrompt - The system prompt for API requests (fallback if customCondensingPrompt not provided) - * @param {string} taskId - The task ID for the conversation, used for telemetry - * @param {boolean} isAutomaticTrigger - Whether the summarization is triggered automatically - * @param {string} customCondensingPrompt - Optional custom prompt to use for condensing - * @param {ApiHandlerCreateMessageMetadata} metadata - Optional metadata to pass to createMessage (tools, taskId, etc.) - * @param {string} environmentDetails - Optional environment details string to include in the summary (only used when isAutomaticTrigger=true) - * @param {string[]} filesReadByRoo - Optional array of file paths read by Roo during the task (will be folded via tree-sitter) - * @param {string} cwd - Optional current working directory for resolving file paths (required if filesReadByRoo is provided) - * @param {RooIgnoreController} rooIgnoreController - Optional controller for file access validation - * @returns {SummarizeResponse} - The result of the summarization operation (see above) */ -export async function summarizeConversation( - messages: ApiMessage[], - apiHandler: ApiHandler, - systemPrompt: string, - taskId: string, - isAutomaticTrigger?: boolean, - customCondensingPrompt?: string, - metadata?: ApiHandlerCreateMessageMetadata, - environmentDetails?: string, - filesReadByRoo?: string[], - cwd?: string, - rooIgnoreController?: RooIgnoreController, -): Promise { +export async function summarizeConversation(options: SummarizeConversationOptions): Promise { + const { + messages, + apiHandler, + systemPrompt, + taskId, + isAutomaticTrigger, + customCondensingPrompt, + metadata, + environmentDetails, + filesReadByRoo, + cwd, + rooIgnoreController, + } = options TelemetryService.instance.captureContextCondensed( taskId, isAutomaticTrigger ?? false, diff --git a/src/core/context-management/__tests__/context-management.spec.ts b/src/core/context-management/__tests__/context-management.spec.ts index c90ff31d1dd..9950ec536b3 100644 --- a/src/core/context-management/__tests__/context-management.spec.ts +++ b/src/core/context-management/__tests__/context-management.spec.ts @@ -612,19 +612,13 @@ describe("Context Management", () => { }) // Verify summarizeConversation was called with the right parameters - expect(summarizeSpy).toHaveBeenCalledWith( - messagesWithSmallContent, - mockApiHandler, - "System prompt", + expect(summarizeSpy).toHaveBeenCalledWith({ + messages: messagesWithSmallContent, + apiHandler: mockApiHandler, + systemPrompt: "System prompt", taskId, - true, // isAutomaticTrigger - undefined, // customCondensingPrompt - undefined, // metadata - undefined, // environmentDetails - undefined, // filesReadByRoo - undefined, // cwd - undefined, // rooIgnoreController - ) + isAutomaticTrigger: true, + }) // Verify the result contains the summary information expect(result).toMatchObject({ @@ -790,19 +784,13 @@ describe("Context Management", () => { }) // Verify summarizeConversation was called with the right parameters - expect(summarizeSpy).toHaveBeenCalledWith( - messagesWithSmallContent, - mockApiHandler, - "System prompt", + expect(summarizeSpy).toHaveBeenCalledWith({ + messages: messagesWithSmallContent, + apiHandler: mockApiHandler, + systemPrompt: "System prompt", taskId, - true, // isAutomaticTrigger - undefined, // customCondensingPrompt - undefined, // metadata - undefined, // environmentDetails - undefined, // filesReadByRoo - undefined, // cwd - undefined, // rooIgnoreController - ) + isAutomaticTrigger: true, + }) // Verify the result contains the summary information expect(result).toMatchObject({ @@ -928,19 +916,16 @@ describe("Context Management", () => { }) // Verify summarizeConversation was called with filesReadByRoo, cwd, and rooIgnoreController - expect(summarizeSpy).toHaveBeenCalledWith( - messagesWithSmallContent, - mockApiHandler, - "System prompt", + expect(summarizeSpy).toHaveBeenCalledWith({ + messages: messagesWithSmallContent, + apiHandler: mockApiHandler, + systemPrompt: "System prompt", taskId, - true, // isAutomaticTrigger - undefined, // customCondensingPrompt - undefined, // metadata - undefined, // environmentDetails + isAutomaticTrigger: true, filesReadByRoo, cwd, - mockRooIgnoreController, - ) + rooIgnoreController: mockRooIgnoreController, + }) // Verify the result contains the summary information expect(result).toMatchObject({ @@ -996,19 +981,13 @@ describe("Context Management", () => { }) // Verify summarizeConversation was called with undefined parameters - expect(summarizeSpy).toHaveBeenCalledWith( - messagesWithSmallContent, - mockApiHandler, - "System prompt", + expect(summarizeSpy).toHaveBeenCalledWith({ + messages: messagesWithSmallContent, + apiHandler: mockApiHandler, + systemPrompt: "System prompt", taskId, - true, // isAutomaticTrigger - undefined, // customCondensingPrompt - undefined, // metadata - undefined, // environmentDetails - undefined, // filesReadByRoo should be undefined when not provided - undefined, // cwd should be undefined when not provided - undefined, // rooIgnoreController should be undefined when not provided - ) + isAutomaticTrigger: true, + }) // Verify the result expect(result).toMatchObject({ @@ -1063,19 +1042,15 @@ describe("Context Management", () => { }) // Verify summarizeConversation was called with empty array - expect(summarizeSpy).toHaveBeenCalledWith( - messagesWithSmallContent, - mockApiHandler, - "System prompt", + expect(summarizeSpy).toHaveBeenCalledWith({ + messages: messagesWithSmallContent, + apiHandler: mockApiHandler, + systemPrompt: "System prompt", taskId, - true, // isAutomaticTrigger - undefined, // customCondensingPrompt - undefined, // metadata - undefined, // environmentDetails - [], // Empty array should be passed through - "/test/project", - undefined, // rooIgnoreController - ) + isAutomaticTrigger: true, + filesReadByRoo: [], + cwd: "/test/project", + }) // Clean up summarizeSpy.mockRestore() diff --git a/src/core/context-management/index.ts b/src/core/context-management/index.ts index e77e69d3041..243d7bd797f 100644 --- a/src/core/context-management/index.ts +++ b/src/core/context-management/index.ts @@ -307,19 +307,19 @@ export async function manageContext({ const contextPercent = (100 * prevContextTokens) / contextWindow if (contextPercent >= effectiveThreshold || prevContextTokens > allowedTokens) { // Attempt to intelligently condense the context - const result = await summarizeConversation( + const result = await summarizeConversation({ messages, apiHandler, systemPrompt, taskId, - true, // automatic trigger + isAutomaticTrigger: true, customCondensingPrompt, metadata, environmentDetails, filesReadByRoo, cwd, rooIgnoreController, - ) + }) if (result.error) { error = result.error errorDetails = result.errorDetails diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 8ac021e345d..82df4e5e5c2 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -1573,6 +1573,15 @@ export class Task extends EventEmitter implements TaskLike { } } + private async getFilesReadByRooSafely(context: string): Promise { + try { + return await this.fileContextTracker.getFilesReadByRoo() + } catch (error) { + console.error(`[Task#${context}] Failed to get files read by Roo:`, error) + return undefined + } + } + public async condenseContext(): Promise { // CRITICAL: Flush any pending tool results before condensing // to ensure tool_use/tool_result pairs are complete in history @@ -1623,14 +1632,7 @@ export class Task extends EventEmitter implements TaskLike { // Generate environment details to include in the condensed summary const environmentDetails = await getEnvironmentDetails(this, true) - // Get files read by Roo for code folding - the actual folding happens inside summarizeConversation() - let filesReadByRoo: string[] | undefined - try { - filesReadByRoo = await this.fileContextTracker.getFilesReadByRoo() - } catch (error) { - console.error("[Task#condenseContext] Failed to get files read by Roo:", error) - // Continue without file context - it's not critical - } + const filesReadByRoo = await this.getFilesReadByRooSafely("condenseContext") const { messages, @@ -1640,19 +1642,19 @@ export class Task extends EventEmitter implements TaskLike { error, errorDetails, condenseId, - } = await summarizeConversation( - this.apiConversationHistory, - this.api, // Main API handler (fallback) - systemPrompt, // Default summarization prompt (fallback) - this.taskId, - false, // manual trigger - customCondensingPrompt, // User's custom prompt - metadata, // Pass metadata with tools - environmentDetails, // Include environment details in summary - filesReadByRoo, // Files to fold - folding happens inside summarizeConversation - this.cwd, - this.rooIgnoreController, - ) + } = await summarizeConversation({ + messages: this.apiConversationHistory, + apiHandler: this.api, + systemPrompt, + taskId: this.taskId, + isAutomaticTrigger: false, + customCondensingPrompt, + metadata, + environmentDetails, + filesReadByRoo, + cwd: this.cwd, + rooIgnoreController: this.rooIgnoreController, + }) if (error) { await this.say( "condense_context_error", @@ -4052,16 +4054,10 @@ export class Task extends EventEmitter implements TaskLike { : undefined // Get files read by Roo for code folding - only when context management will run - // The actual folding will happen inside summarizeConversation() for cleaner architecture - let contextMgmtFilesReadByRoo: string[] | undefined - if (contextManagementWillRun && autoCondenseContext) { - try { - contextMgmtFilesReadByRoo = await this.fileContextTracker.getFilesReadByRoo() - } catch (error) { - console.error("[Task#attemptApiRequest] Failed to get files read by Roo:", error) - // Continue without file context - it's not critical - } - } + const contextMgmtFilesReadByRoo = + contextManagementWillRun && autoCondenseContext + ? await this.getFilesReadByRooSafely("attemptApiRequest") + : undefined try { const truncateResult = await manageContext({ From ddfcc7e3ba038f06994b7d23bf73ae6d69c03d2e Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Mon, 26 Jan 2026 18:44:06 -0700 Subject: [PATCH 6/6] fix: address roomote feedback - batch error logging and early budget exit --- src/core/condense/foldedFileContext.ts | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/core/condense/foldedFileContext.ts b/src/core/condense/foldedFileContext.ts index 354c8fa2a86..360dd7fc76b 100644 --- a/src/core/condense/foldedFileContext.ts +++ b/src/core/condense/foldedFileContext.ts @@ -93,8 +93,10 @@ export async function generateFoldedFileContext( const foldedSections: string[] = [] let currentCharCount = 0 + const failedFiles: string[] = [] - for (const filePath of filePaths) { + for (let i = 0; i < filePaths.length; i++) { + const filePath = filePaths[i] // Resolve to absolute path for tree-sitter const absolutePath = path.isAbsolute(filePath) ? filePath : path.resolve(cwd, filePath) @@ -119,9 +121,9 @@ ${definitions} // Would exceed limit - check if we can fit at least a truncated version const remainingChars = maxCharacters - currentCharCount if (remainingChars < 200) { - // Not enough room for meaningful content, stop processing - result.filesSkipped++ - continue + // Not enough room for meaningful content, stop processing all remaining files + result.filesSkipped += filePaths.length - i + break } // Truncate the definitions to fit within the system-reminder block @@ -143,11 +145,19 @@ ${truncatedDefinitions} currentCharCount += sectionContent.length result.filesProcessed++ } catch (error) { - console.error(`Failed to generate folded context for ${filePath}:`, error) + // Collect failed files for batch logging to reduce noise + failedFiles.push(filePath) result.filesSkipped++ } } + // Log failed files as a single batch summary instead of per-file errors + if (failedFiles.length > 0) { + console.warn( + `Folded context generation: skipped ${failedFiles.length} file(s) due to errors: ${failedFiles.slice(0, 5).join(", ")}${failedFiles.length > 5 ? ` and ${failedFiles.length - 5} more` : ""}`, + ) + } + if (foldedSections.length > 0) { result.sections = foldedSections result.content = foldedSections.join("\n")