Skip to content
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

Add taskId and checkpointNumber parameters to completePrompt and rela… #43

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

hakt0r
Copy link

@hakt0r hakt0r commented Mar 14, 2025

Context

This PR adds idempotency key support for API requests to Anthropic's Claude models. Idempotency keys ensure that duplicate requests don't result in duplicate operations, which is particularly important for maintaining consistency when retrying API calls or resuming from checkpoints.

Implementation

The implementation adds support for generating and using idempotency keys in API requests to Anthropic's Claude models:

Modified the API interfaces to accept taskId and checkpointNumber parameters:

Updated SingleCompletionHandler.completePrompt() to accept these parameters
Updated ApiHandler.createMessage() to accept these parameters
Updated BaseProvider abstract class to include these parameters
Added idempotency key generation in the KiloCodeHandler class:

Created a private getIdempotencyKey() method that generates a deterministic key based on task ID and checkpoint number
Added logic to include the idempotency key in request headers when making API calls
Updated the call chain to pass these parameters:

Modified Cline.ts to pass the current task ID and checkpoint number to API calls
Updated ClineProvider.ts to pass these parameters to the API
Updated single-completion-handler.ts to support the new parameters
Refactored the request options handling in KiloCodeHandler:

Extracted the headers preparation logic into a separate code block
Improved code organization for better readability and maintainability
Updated tests to accommodate the new parameters in API calls

The implementation follows the pattern described in the settings documentation, ensuring proper type safety and parameter passing throughout the application.

How to Test

Create a new task in Kilo Code
Make some API requests that create checkpoints
Verify in network monitoring tools that requests include the idempotency-key header with a value in the format {taskId}-{checkpointNumber}
Try retrying a request with the same idempotency key and verify that it returns the same response without duplicating the operation

@janpaul123
Copy link
Collaborator

PR description is AI slop. Please write a concise description with key points.

(acc, msg, index) => (msg.role === "user" ? [...acc, index] : acc),
[] as number[],
)
// Use a for loop instead of reduce with spread to avoid linting error
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need for this comment. This could've been a self-comment on this PR instead.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also: If you convert this to a for .. of loop it's modern JS best practice compliant, better readable and removes the need for the comment

(() => {
// prompt caching: https://x.com/alexalbert__/status/1823751995901272068
// https://github.com/anthropics/anthropic-sdk-typescript?tab=readme-ov-file#default-headers
// https://github.com/anthropics/anthropic-sdk-typescript/commit/c920b77fc67bd839bfeb6716ceab9d7c9bbe7393
Copy link
Collaborator

Choose a reason for hiding this comment

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

These comments seem useful. Preserve them?

// Add idempotency key if task_id and checkpoint number are provided
if (taskId && checkpointNumber !== undefined) {
requestOptions.headers["idempotency-key"] = this.getIdempotencyKey(taskId, checkpointNumber)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe write this a bit more compactly? It feels like "a lot" in the way it's spread over so many lines, while in reality almost nothing is happening here. Code "weight" should feel proportional to the weight of the content.

const checkpointNumber = this.clineMessages.filter(({ say }) => say === "checkpoint_saved").length

// Pass task_id and checkpoint number to the API for idempotency key generation
const stream = this.api.createMessage(systemPrompt, cleanConversationHistory, this.taskId, checkpointNumber)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same point about weight. Remove those comments.

Copy link
Collaborator

@janpaul123 janpaul123 left a comment

Choose a reason for hiding this comment

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

Please heavily de-slop.


const lastUserMsgIndex = userMsgIndices[userMsgIndices.length - 1] ?? -1
const secondLastMsgUserIndex = userMsgIndices[userMsgIndices.length - 2] ?? -1

// Prepare request options with headers
const requestOptions: { headers: Record<string, string> } = (() => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Either this typing here or the one down on 64 is redundant, making it harder to maintain.

// Add idempotency key if task_id and checkpoint number are provided
if (taskId && checkpointNumber !== undefined) {
headers["idempotency-key"] = this.getIdempotencyKey(taskId, checkpointNumber)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is the AI way of doing it, the more efficient (preventing multiple object mutations) and readable way (imho) to do it would be like this:

const headers = {
  'anthropic-beta': betas.length > 0 ? betas.join(",") : undefined,
  'idempotency-key': taskId && checkpointNumber !== undefined ? this.getIdempotencyKey(taskId, checkpointNumber),
}

@kevinvandijk
Copy link
Collaborator

kevinvandijk commented Mar 18, 2025

What's the status of this PR @hakt0r? Is it still relevant?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants