-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: implement read_file history deduplication (#6279) #6306
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 3 commits
311ef20
1d68210
9fb16de
76bca9d
44f4bd5
c98889c
935677a
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 | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -224,8 +224,8 @@ export class AwsBedrockHandler extends BaseProvider implements SingleCompletionH | |||||||||
|
|
||||||||||
| if (this.options.awsUseApiKey && this.options.awsApiKey) { | ||||||||||
| // Use API key/token-based authentication if enabled and API key is set | ||||||||||
| clientConfig.token = { token: this.options.awsApiKey } | ||||||||||
| clientConfig.authSchemePreference = ["httpBearerAuth"] // Otherwise there's no end of credential problems. | ||||||||||
| ;(clientConfig as any).token = { token: this.options.awsApiKey } | ||||||||||
| ;(clientConfig as any).authSchemePreference = ["httpBearerAuth"] // Otherwise there's no end of credential problems. | ||||||||||
|
Comment on lines
+227
to
+228
|
||||||||||
| ;(clientConfig as any).token = { token: this.options.awsApiKey } | |
| ;(clientConfig as any).authSchemePreference = ["httpBearerAuth"] // Otherwise there's no end of credential problems. | |
| clientConfig.token = { token: this.options.awsApiKey } | |
| clientConfig.authSchemePreference = ["httpBearerAuth"] // Otherwise there's no end of credential problems. |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -329,6 +329,110 @@ export class Task extends EventEmitter<ClineEvents> { | |||||
| return readApiMessages({ taskId: this.taskId, globalStoragePath: this.globalStoragePath }) | ||||||
| } | ||||||
|
|
||||||
| public async deduplicateReadFileHistory(): Promise<void> { | ||||||
| // Check if the experimental feature is enabled | ||||||
| const state = await this.providerRef.deref()?.getState() | ||||||
| if (!state?.experiments || !experiments.isEnabled(state.experiments, EXPERIMENT_IDS.READ_FILE_DEDUPLICATION)) { | ||||||
| return | ||||||
| } | ||||||
|
|
||||||
| const cacheWindowMs = 5 * 60 * 1000 // 5 minutes | ||||||
| const now = Date.now() | ||||||
| const seenFiles = new Map<string, { messageIndex: number; blockIndex: number }>() | ||||||
| const blocksToRemove = new Map<number, Set<number>>() // messageIndex -> Set of blockIndexes to remove | ||||||
|
|
||||||
| // Process messages in reverse order (newest first) to keep the most recent reads | ||||||
| for (let i = this.apiConversationHistory.length - 1; i >= 0; i--) { | ||||||
| const message = this.apiConversationHistory[i] | ||||||
|
|
||||||
| // Only process user messages | ||||||
| if (message.role !== "user") { | ||||||
| continue | ||||||
| } | ||||||
|
|
||||||
| // Skip messages within the cache window | ||||||
| if (message.ts && now - message.ts < cacheWindowMs) { | ||||||
| continue | ||||||
| } | ||||||
|
|
||||||
| // Process content blocks | ||||||
| if (Array.isArray(message.content)) { | ||||||
| for (let j = 0; j < message.content.length; j++) { | ||||||
| const block = message.content[j] | ||||||
| if (block.type === "text" && typeof block.text === "string") { | ||||||
| // Check for read_file results in text blocks | ||||||
| const readFileMatch = block.text.match(/\[read_file(?:\s+for\s+'([^']+)')?.*?\]\s*Result:/i) | ||||||
|
||||||
| const readFileMatch = block.text.match(/\[read_file(?:\s+for\s+'([^']+)')?.*?\]\s*Result:/i) | |
| const readFileMatch = block.text.match(READ_FILE_REGEX) |
Copilot
AI
Jul 28, 2025
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.
The regex pattern /<file>\s*<path>([^<]+)<\/path>/g is a magic string that could be extracted as a constant with a descriptive name for better maintainability.
| const xmlFileMatches = resultContent.matchAll(/<file>\s*<path>([^<]+)<\/path>/g) | |
| const xmlFileMatches = resultContent.matchAll(XML_FILE_PATH_REGEX) |
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.
Using
as anytype assertions should be avoided. Consider properly typing the clientConfig or using a more specific type assertion that maintains type safety.