Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/gold-radios-cover.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"kilo-code": patch
---

The rate limiter no longer generates timeouts longer than the configured limit.
13 changes: 11 additions & 2 deletions src/core/task/Task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2699,10 +2699,19 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
// Use the shared timestamp so that subtasks respect the same rate-limit
// window as their parent tasks.
if (Task.lastGlobalApiRequestTime) {
const now = Date.now()
const now = performance.now() // kilocode_change
const timeSinceLastRequest = now - Task.lastGlobalApiRequestTime
const rateLimit = apiConfiguration?.rateLimitSeconds || 0
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can try to fix it more aggressively by not waiting if rateLimit is zero anyway.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That does make sense, i mean than we kan skip this altogether right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, in fact if rateLimitDelay>rateLimit I think we're in trouble.

maybe Task.lastGlobalApiRequestTime being a global also causes problem.

rateLimitDelay = Math.ceil(Math.max(0, rateLimit * 1000 - timeSinceLastRequest) / 1000)

// kilocode_change start
if (rateLimitDelay > rateLimit) {
console.warn(
`rateLimitDelay ${rateLimitDelay}s is larger than the configured rateLimit ${rateLimit}s; this makes no sense`,
)
rateLimitDelay = rateLimit
}
// kilocode_change end
}

// Only show rate limiting message if we're not retrying. If retrying, we'll include the delay there.
Expand All @@ -2717,7 +2726,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {

// Update last request time before making the request so that subsequent
// requests — even from new subtasks — will honour the provider's rate-limit.
Task.lastGlobalApiRequestTime = Date.now()
Task.lastGlobalApiRequestTime = performance.now() // kilocode_change

const systemPrompt = await this.getSystemPrompt()
this.lastUsedInstructions = systemPrompt
Expand Down
10 changes: 6 additions & 4 deletions src/core/task/__tests__/Task.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1114,9 +1114,11 @@ describe("Cline", () => {
await parentIterator.next()

// Simulate time passing (more than rate limit)
const originalDateNow = Date.now
const mockTime = Date.now() + (mockApiConfig.rateLimitSeconds + 1) * 1000
Date.now = vi.fn(() => mockTime)
// kilocode_change start: use performance instead of Date
const originalDateNow = performance.now
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename

const mockTime = performance.now() + (mockApiConfig.rateLimitSeconds + 1) * 1000
performance.now = vi.fn(() => mockTime)
// kilocode_change end

// Create a subtask after time has passed
const child = new Task({
Expand All @@ -1139,7 +1141,7 @@ describe("Cline", () => {
expect(mockDelay).not.toHaveBeenCalled()

// Restore Date.now
Date.now = originalDateNow
performance.now = originalDateNow // kilocode_change
})

it("should share rate limiting across multiple subtasks", async () => {
Expand Down
Loading