-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: add tool result compression to prevent context window exhaustion #6466
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,162 @@ | ||
| import { describe, it, expect } from "vitest" | ||
| import { | ||
| compressToolResult, | ||
| shouldCompressToolResult, | ||
| getCompressionLimitsForContextWindow, | ||
| DEFAULT_TOOL_RESULT_CHARACTER_LIMIT, | ||
| DEFAULT_TOOL_RESULT_LINE_LIMIT, | ||
| } from "../compressToolResult" | ||
|
|
||
| describe("compressToolResult", () => { | ||
| describe("compressToolResult function", () => { | ||
| it("should return original result when within limits", () => { | ||
| const shortResult = "This is a short result" | ||
| const compressed = compressToolResult(shortResult) | ||
| expect(compressed).toBe(shortResult) | ||
| }) | ||
|
|
||
| it("should return empty string for empty input", () => { | ||
| expect(compressToolResult("")).toBe("") | ||
| expect(compressToolResult(null as any)).toBe(null) | ||
| expect(compressToolResult(undefined as any)).toBe(undefined) | ||
| }) | ||
|
|
||
| it("should compress result when exceeding character limit", () => { | ||
| const longResult = "A".repeat(60000) // Exceeds default 50000 char limit | ||
| const compressed = compressToolResult(longResult) | ||
|
|
||
| expect(compressed).not.toBe(longResult) | ||
| expect(compressed.length).toBeLessThan(longResult.length) | ||
| expect(compressed).toContain("[Tool result compressed:") | ||
| expect(compressed).toContain("characters omitted") | ||
| }) | ||
|
|
||
| it("should compress result when exceeding line limit", () => { | ||
| const manyLines = Array(1500).fill("line").join("\n") // Exceeds default 1000 line limit | ||
| const compressed = compressToolResult(manyLines) | ||
|
|
||
| expect(compressed).not.toBe(manyLines) | ||
| expect(compressed.split("\n").length).toBeLessThan(manyLines.split("\n").length) | ||
| expect(compressed).toContain("[Tool result compressed:") | ||
| expect(compressed).toContain("lines omitted") | ||
| }) | ||
|
|
||
| it("should use custom limits when provided", () => { | ||
| const result = "A".repeat(200) | ||
| const compressed = compressToolResult(result, 100, 10) // Custom limits | ||
|
|
||
| expect(compressed).not.toBe(result) | ||
| expect(compressed).toContain("[Tool result compressed:") | ||
| }) | ||
|
|
||
| it("should preserve structure with compression note at beginning", () => { | ||
| const longResult = "A".repeat(60000) | ||
| const compressed = compressToolResult(longResult) | ||
|
|
||
| expect(compressed.startsWith("[Tool result compressed:")).toBe(true) | ||
| expect(compressed).toContain("Original 60000 characters") | ||
| }) | ||
|
|
||
| it("should handle mixed character and line limits", () => { | ||
| // Create content that exceeds both limits | ||
| const longLines = Array(1500).fill("A".repeat(100)).join("\n") | ||
| const compressed = compressToolResult(longLines) | ||
|
|
||
| expect(compressed).not.toBe(longLines) | ||
| expect(compressed).toContain("[Tool result compressed:") | ||
| }) | ||
| }) | ||
|
|
||
| describe("shouldCompressToolResult function", () => { | ||
| it("should return false for short results", () => { | ||
| const shortResult = "Short result" | ||
| expect(shouldCompressToolResult(shortResult)).toBe(false) | ||
| }) | ||
|
|
||
| it("should return true for results exceeding character limit", () => { | ||
| const longResult = "A".repeat(60000) | ||
| expect(shouldCompressToolResult(longResult)).toBe(true) | ||
| }) | ||
|
|
||
| it("should return true for results exceeding line limit", () => { | ||
| const manyLines = Array(1500).fill("line").join("\n") | ||
| expect(shouldCompressToolResult(manyLines)).toBe(true) | ||
| }) | ||
|
|
||
| it("should return false for empty results", () => { | ||
| expect(shouldCompressToolResult("")).toBe(false) | ||
| expect(shouldCompressToolResult(null as any)).toBe(false) | ||
| expect(shouldCompressToolResult(undefined as any)).toBe(false) | ||
| }) | ||
|
|
||
| it("should respect custom limits", () => { | ||
| const result = "A".repeat(200) | ||
| expect(shouldCompressToolResult(result, 100, 10)).toBe(true) | ||
| expect(shouldCompressToolResult(result, 300, 10)).toBe(false) | ||
| }) | ||
| }) | ||
|
|
||
| describe("getCompressionLimitsForContextWindow function", () => { | ||
| it("should return appropriate limits for small context windows", () => { | ||
| const limits = getCompressionLimitsForContextWindow(8000) // Small context window | ||
|
|
||
| expect(limits.characterLimit).toBeGreaterThanOrEqual(DEFAULT_TOOL_RESULT_CHARACTER_LIMIT) | ||
| expect(limits.lineLimit).toBeGreaterThanOrEqual(DEFAULT_TOOL_RESULT_LINE_LIMIT) | ||
| }) | ||
|
|
||
| it("should return larger limits for large context windows", () => { | ||
| const smallLimits = getCompressionLimitsForContextWindow(8000) | ||
| const largeLimits = getCompressionLimitsForContextWindow(200000) // Large context window | ||
|
|
||
| expect(largeLimits.characterLimit).toBeGreaterThanOrEqual(smallLimits.characterLimit) | ||
| expect(largeLimits.lineLimit).toBeGreaterThanOrEqual(smallLimits.lineLimit) | ||
| }) | ||
|
|
||
| it("should never return limits below defaults", () => { | ||
| const limits = getCompressionLimitsForContextWindow(1000) // Very small context window | ||
|
|
||
| expect(limits.characterLimit).toBeGreaterThanOrEqual(DEFAULT_TOOL_RESULT_CHARACTER_LIMIT) | ||
| expect(limits.lineLimit).toBeGreaterThanOrEqual(DEFAULT_TOOL_RESULT_LINE_LIMIT) | ||
| }) | ||
|
|
||
| it("should scale limits proportionally with context window", () => { | ||
| const limits1 = getCompressionLimitsForContextWindow(50000) | ||
| const limits2 = getCompressionLimitsForContextWindow(100000) | ||
|
|
||
| // Larger context window should allow larger tool results | ||
| expect(limits2.characterLimit).toBeGreaterThanOrEqual(limits1.characterLimit) | ||
| }) | ||
|
|
||
| it("should cap limits at reasonable maximums", () => { | ||
| const limits = getCompressionLimitsForContextWindow(1000000) // Extremely large context window | ||
|
|
||
| // Should not exceed 2x the default limits | ||
| expect(limits.characterLimit).toBeLessThanOrEqual(DEFAULT_TOOL_RESULT_CHARACTER_LIMIT * 2) | ||
| }) | ||
| }) | ||
|
|
||
| describe("integration with truncateOutput", () => { | ||
| it("should preserve beginning and end of content", () => { | ||
| const longResult = "START" + "A".repeat(60000) + "END" | ||
| const compressed = compressToolResult(longResult) | ||
|
|
||
| // Should contain compression note plus truncated content | ||
| expect(compressed).toContain("[Tool result compressed:") | ||
| // The truncated content should preserve structure from truncateOutput | ||
| expect(compressed).toContain("START") | ||
| expect(compressed).toContain("END") | ||
| }) | ||
|
|
||
| it("should handle line-based truncation", () => { | ||
| const lines = Array(1500) | ||
| .fill(0) | ||
| .map((_, i) => `Line ${i + 1}`) | ||
| const longResult = lines.join("\n") | ||
| const compressed = compressToolResult(longResult) | ||
|
|
||
| expect(compressed).toContain("[Tool result compressed:") | ||
| expect(compressed).toContain("Line 1") // Should preserve beginning | ||
| expect(compressed).toContain("lines omitted") // Should indicate truncation | ||
| }) | ||
| }) | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| import { truncateOutput } from "../../integrations/misc/extract-text" | ||
| import { DEFAULT_TERMINAL_OUTPUT_CHARACTER_LIMIT } from "@roo-code/types" | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This import is unused and should be removed. The constant isn't referenced anywhere in this file, creating unnecessary coupling to the types package. |
||
|
|
||
| /** | ||
| * Default character limit for tool results to prevent context window exhaustion | ||
| * This is set to be conservative to ensure tool results don't consume too much context | ||
| */ | ||
| export const DEFAULT_TOOL_RESULT_CHARACTER_LIMIT = 50000 | ||
|
|
||
| /** | ||
| * Default line limit for tool results | ||
| */ | ||
| export const DEFAULT_TOOL_RESULT_LINE_LIMIT = 1000 | ||
|
|
||
| /** | ||
| * Compresses a tool result if it exceeds the specified limits. | ||
| * Uses the same truncation logic as terminal output compression to maintain consistency. | ||
| * | ||
| * @param result The tool result string to potentially compress | ||
| * @param characterLimit Maximum number of characters allowed (defaults to DEFAULT_TOOL_RESULT_CHARACTER_LIMIT) | ||
| * @param lineLimit Maximum number of lines allowed (defaults to DEFAULT_TOOL_RESULT_LINE_LIMIT) | ||
| * @returns The original result if within limits, or a compressed version with truncation indicators | ||
| */ | ||
| export function compressToolResult( | ||
| result: string, | ||
| characterLimit: number = DEFAULT_TOOL_RESULT_CHARACTER_LIMIT, | ||
| lineLimit: number = DEFAULT_TOOL_RESULT_LINE_LIMIT, | ||
| ): string { | ||
| // If result is empty or null, return as-is | ||
| if (!result || result.length === 0) { | ||
| return result | ||
| } | ||
|
|
||
| // Check if compression is needed | ||
| const needsCharacterCompression = characterLimit > 0 && result.length > characterLimit | ||
| const needsLineCompression = lineLimit > 0 && result.split("\n").length > lineLimit | ||
|
|
||
| // If no compression is needed, return original result | ||
| if (!needsCharacterCompression && !needsLineCompression) { | ||
| return result | ||
| } | ||
|
|
||
| // Use the existing truncateOutput function which handles both character and line limits | ||
| // and provides intelligent truncation with context preservation | ||
| const compressedResult = truncateOutput( | ||
| result, | ||
| lineLimit > 0 ? lineLimit : undefined, | ||
| characterLimit > 0 ? characterLimit : undefined, | ||
| ) | ||
|
|
||
| // Add a note about compression if the result was actually truncated | ||
| if (compressedResult !== result) { | ||
| const originalLength = result.length | ||
| const originalLines = result.split("\n").length | ||
| const compressedLength = compressedResult.length | ||
| const compressedLines = compressedResult.split("\n").length | ||
|
|
||
| // Add compression info at the beginning to make it clear to the model | ||
| const compressionNote = `[Tool result compressed: Original ${originalLength} characters, ${originalLines} lines → Compressed to ${compressedLength} characters, ${compressedLines} lines to prevent context window exhaustion]\n\n` | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The compression note format uses arrow symbol and specific wording that differs from the existing truncateOutput function's format. Should we align these for consistency across the codebase? |
||
|
|
||
| return compressionNote + compressedResult | ||
| } | ||
|
|
||
| return compressedResult | ||
| } | ||
|
|
||
| /** | ||
| * Determines if a tool result should be compressed based on its size and the model's context window. | ||
| * This can be used to make compression decisions before actually compressing. | ||
| * | ||
| * @param result The tool result to check | ||
| * @param characterLimit Character limit threshold | ||
| * @param lineLimit Line limit threshold | ||
| * @returns true if the result exceeds the limits and should be compressed | ||
| */ | ||
| export function shouldCompressToolResult( | ||
| result: string, | ||
| characterLimit: number = DEFAULT_TOOL_RESULT_CHARACTER_LIMIT, | ||
| lineLimit: number = DEFAULT_TOOL_RESULT_LINE_LIMIT, | ||
| ): boolean { | ||
| if (!result || result.length === 0) { | ||
| return false | ||
| } | ||
|
|
||
| const exceedsCharacterLimit = characterLimit > 0 && result.length > characterLimit | ||
| const exceedsLineLimit = lineLimit > 0 && result.split("\n").length > lineLimit | ||
|
|
||
| return exceedsCharacterLimit || exceedsLineLimit | ||
| } | ||
|
|
||
| /** | ||
| * Gets appropriate compression limits based on the model's context window size. | ||
| * Larger context windows can accommodate larger tool results. | ||
| * | ||
| * @param contextWindow The model's context window size in tokens | ||
| * @returns Object with characterLimit and lineLimit appropriate for the context window | ||
| */ | ||
| export function getCompressionLimitsForContextWindow(contextWindow: number): { | ||
| characterLimit: number | ||
| lineLimit: number | ||
| } { | ||
| // Conservative approach: tool results should not exceed a small percentage of context window | ||
| // Assuming roughly 4 characters per token on average | ||
| const maxToolResultTokens = Math.floor(contextWindow * 0.1) // 10% of context window | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing edge case handling - what happens if contextWindow <= 0 or NaN? This could cause unexpected behavior with Math.floor() and Math.min() operations. |
||
| const characterLimit = Math.min(maxToolResultTokens * 4, DEFAULT_TOOL_RESULT_CHARACTER_LIMIT * 2) // Cap at 2x default | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider using named constants for these magic numbers. Something like TOOL_RESULT_CONTEXT_PERCENTAGE = 0.1 and COMPRESSION_CAP_MULTIPLIER = 2 would make the logic clearer. |
||
|
|
||
| // Line limit scales with character limit | ||
| const lineLimit = Math.floor(characterLimit / 50) // Assume ~50 chars per line on average | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The assumption of ~50 characters per line might not hold for all content types. Consider making this configurable or using a more robust calculation based on actual content analysis. |
||
|
|
||
| return { | ||
| characterLimit: Math.max(characterLimit, DEFAULT_TOOL_RESULT_CHARACTER_LIMIT), // Never go below default | ||
| lineLimit: Math.max(lineLimit, DEFAULT_TOOL_RESULT_LINE_LIMIT), // Never go below default | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code block (lines 253-256) is duplicated below at lines 269-271. Could we extract this into a helper function to follow DRY principles? Something like getCompressionLimitsForModel would eliminate the duplication.