-
Notifications
You must be signed in to change notification settings - Fork 9
feat: optimize client conversion performance #162
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
base: main
Are you sure you want to change the base?
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 |
|---|---|---|
|
|
@@ -4,33 +4,137 @@ | |
| */ | ||
|
|
||
| import type { Tool } from "../tools/base"; | ||
| import type { Message, LLMResponse } from "../schema"; | ||
| import type { | ||
| Message, | ||
| LLMResponse, | ||
| FunctionResponse, | ||
| Content, | ||
| } from "../schema"; | ||
|
|
||
| export class RetryExhaustedError extends Error { | ||
| public lastException: Error; | ||
| public attempts: number; | ||
|
|
||
| constructor(lastException: Error, attempts: number) { | ||
| super( | ||
| `Retry failed after ${attempts} attempts. Last error: ${lastException.message}`, | ||
| ); | ||
| this.name = "RetryExhaustedError"; | ||
| this.lastException = lastException; | ||
| this.attempts = attempts; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Elegant retry mechanism module | ||
| * | ||
| * Provides decorators and utility functions to support retry logic for async functions. | ||
| * | ||
| * Features: | ||
| * - Supports exponential backoff strategy | ||
| * - Configurable retry count and intervals | ||
| * - Supports specifying retryable exception types | ||
| * - Detailed logging | ||
| * - Fully decoupled, non-invasive to business code | ||
| */ | ||
| export class RetryConfig { | ||
| constructor( | ||
| /** | ||
| * Whether to enable retry mechanism | ||
| */ | ||
| public readonly enabled: boolean = true, | ||
| /** | ||
| * Maximum number of retries | ||
| */ | ||
| public readonly max_retries: number = 3, | ||
| /** | ||
| * Initial delay time (seconds) | ||
| */ | ||
| public readonly initial_delay: number = 1.0, | ||
| /** | ||
| * Maximum delay time (seconds) | ||
| */ | ||
| public readonly max_delay: number = 60.0, | ||
| /** | ||
| * Exponential backoff base | ||
| */ | ||
| public readonly exponential_base: number = 2.0, | ||
| /** | ||
| * Retryable exception types | ||
| */ | ||
| // public readonly retryable_exceptions: Array<typeof Error> = [Error], | ||
| ) {} | ||
| } | ||
|
|
||
| export function isAbortError(error: unknown): boolean { | ||
| if (!(error instanceof Error)) { | ||
| return false; | ||
| } | ||
| if (error.name === "AbortError") { | ||
| return true; | ||
| } | ||
| return error.message.toLowerCase().includes("abort"); | ||
| } | ||
|
Comment on lines
+28
to
+77
|
||
|
|
||
| /** | ||
| * Abstract base class for LLM clients. | ||
| * | ||
| * This class defines the interface that all LLM clients must implement, | ||
| * regardless of the underlying API protocol (Anthropic, OpenAI, etc.). | ||
| */ | ||
| export abstract class LLMClientBase { | ||
| export abstract class LLMClientBase<M = any> { | ||
| retryCallback: ((exception: Error, attempt: number) => void) | undefined = | ||
| undefined; | ||
|
|
||
| abstract adaptTools(tools: Tool[]): any[]; | ||
|
|
||
| abstract adaptMessages(messages: Message[]): any[]; | ||
| abstract appendMessage(history: M[], message: Message): M[]; | ||
|
|
||
| abstract makeApiRequest( | ||
| apiMessages: any[], | ||
| history: MessageHistory<M>, | ||
| apiTools?: any[], | ||
| systemPrompt?: string, | ||
| signal?: AbortSignal, | ||
| ): Promise<any>; | ||
|
|
||
| abstract generate( | ||
| messages: Message[], | ||
| tools?: Tool[], | ||
| systemPrompt?: string, | ||
| signal?: AbortSignal, | ||
| ): Promise<LLMResponse>; | ||
| } | ||
|
|
||
| /** | ||
| * Holds the messages for the specific LLM provider. | ||
| */ | ||
| export class MessageHistory<M = any> { | ||
| messages: Message[] = []; | ||
| private apiMessages: M[] = []; | ||
|
|
||
|
Comment on lines
+104
to
+107
|
||
| constructor(private readonly client: LLMClientBase<M>) {} | ||
|
|
||
| getApiMessagesForClient(client: LLMClientBase<M>): M[] { | ||
| if (client !== this.client) { | ||
| // this ensures that we always give correct message format to the client | ||
| throw new Error( | ||
| `Client mismatch: converted to ${this.client.constructor.name} while expected ${client.constructor.name}`, | ||
| ); | ||
| } | ||
| return this.apiMessages; | ||
| } | ||
|
|
||
| /** Adds a user message to context. */ | ||
| addUserMessage(contents: Content[]): void { | ||
| this.appendMessage({ role: "user", contents: contents }); | ||
| } | ||
|
|
||
| /** Adds an model message to context. */ | ||
| addModelMessage(response: LLMResponse): void { | ||
| this.appendMessage(response.message); | ||
| } | ||
|
|
||
| /** Adds a tool result message to context. */ | ||
| addToolMessage(contents: FunctionResponse[]): void { | ||
| this.appendMessage({ role: "user", contents: contents }); | ||
| } | ||
|
|
||
| appendMessage(message: Message): this { | ||
| this.messages.push(message); | ||
| this.apiMessages = this.client.appendMessage(this.apiMessages, message); | ||
| return this; | ||
| } | ||
| } | ||
|
Comment on lines
+101
to
+140
|
||
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.
ContextManager.messagesrebuildshistoryfrom the passed array, but later mutations happen viahistory.addModelMessage/addToolMessagewithout updating_state.messages. This splits state between two histories and makesgetHistory()/state.messagesinaccurate after the loop starts. Recommend keeping only one canonical store (preferablyhistory.messages) and derivingmessagesfrom it, or ensuring all append operations go through a single API that updates both.