[WIP] feat(messages): add Anthropic Messages API extension#272
[WIP] feat(messages): add Anthropic Messages API extension#272Menci wants to merge 5 commits intomoeru-ai:mainfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request integrates the Anthropic Messages API by introducing a dedicated extension package. This new package provides a structured and efficient way to leverage Anthropic's message-based models, offering robust support for streaming interactions, managing conversational turns, and facilitating tool usage within AI applications. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new package, @xsai-ext/messages, to provide an extension for the Anthropic Messages API. This is a significant feature addition that includes the core streaming logic, type definitions, utility functions, and tests. My review focuses on improving code correctness and maintainability. I've identified a few areas for improvement, including moving a dev dependency to dependencies, simplifying some complex type definitions, refactoring for better state management, and adding error handling for JSON parsing in a stream. Overall, this is a solid foundation for the new package.
| transform: async (chunk, controller) => { | ||
| const event = JSON.parse(chunk.data) as StreamingEvent | ||
| controller.enqueue(event) | ||
| }, |
There was a problem hiding this comment.
The JSON.parse call is not wrapped in a try-catch block. If the chunk.data from the event stream is not valid JSON, this will throw an unhandled exception and crash the stream. It's safer to wrap this in a try-catch block and call controller.error() on failure.
transform: async (chunk, controller) => {
try {
const event = JSON.parse(chunk.data) as StreamingEvent
controller.enqueue(event)
} catch (error) {
controller.error(error)
}
},| "devDependencies": { | ||
| "@standard-schema/spec": "catalog:", | ||
| "zod": "catalog:schema-dev" | ||
| } |
There was a problem hiding this comment.
The @standard-schema/spec package is used for types in the public API of this package (e.g., in src/types/anthropic-tool.ts). Therefore, it should be listed under dependencies instead of devDependencies to ensure its types are available to consumers of this package.
| import type { Usage } from './usage' | ||
|
|
||
| export interface AnthropicAssistantMessageParam { | ||
| content: (AnthropicRedactedThinkingBlock | AnthropicTextBlockParam | AnthropicThinkingBlockParam | AnthropicToolUseBlock)[] | AnthropicRedactedThinkingBlock[] | AnthropicTextBlockParam[] | AnthropicThinkingBlockParam[] | AnthropicToolUseBlock[] | string |
There was a problem hiding this comment.
The union type for content is unnecessarily verbose. A type like (A | B)[] | A[] | B[] can be simplified to (A | B)[]. Applying this simplification will make the type definition cleaner and easier to read.
| content: (AnthropicRedactedThinkingBlock | AnthropicTextBlockParam | AnthropicThinkingBlockParam | AnthropicToolUseBlock)[] | AnthropicRedactedThinkingBlock[] | AnthropicTextBlockParam[] | AnthropicThinkingBlockParam[] | AnthropicToolUseBlock[] | string | |
| content: (AnthropicRedactedThinkingBlock | AnthropicTextBlockParam | AnthropicThinkingBlockParam | AnthropicToolUseBlock)[] | string |
| } | ||
|
|
||
| export interface AnthropicUserMessageParam { | ||
| content: (AnthropicDocumentBlockParam | AnthropicImageBlockParam | AnthropicTextBlockParam | AnthropicToolResultBlockParam)[] | AnthropicDocumentBlockParam[] | AnthropicImageBlockParam[] | AnthropicTextBlockParam[] | AnthropicToolResultBlockParam[] | string |
There was a problem hiding this comment.
The union type for content is unnecessarily verbose. A type like (A | B)[] | A[] | B[] can be simplified to (A | B)[]. Applying this simplification will make the type definition cleaner and easier to read.
| content: (AnthropicDocumentBlockParam | AnthropicImageBlockParam | AnthropicTextBlockParam | AnthropicToolResultBlockParam)[] | AnthropicDocumentBlockParam[] | AnthropicImageBlockParam[] | AnthropicTextBlockParam[] | AnthropicToolResultBlockParam[] | string | |
| content: (AnthropicDocumentBlockParam | AnthropicImageBlockParam | AnthropicTextBlockParam | AnthropicToolResultBlockParam)[] | string |
| const handleMessageStop = async ({ conversation, currentStep, steps, tools, totalUsage }: HandleMessageStopOptions): Promise<ProcessEventResult> => { | ||
| const message = finalizeCurrentMessage(currentStep) | ||
| const stepUsage = message.usage | ||
| const nextUsage = stepUsage | ||
| const nextTotalUsage = addUsage(totalUsage, stepUsage) | ||
| const step = createStep(message, []) | ||
|
|
||
| conversation.push(createAssistantMessageParam(message)) | ||
| steps.push(step) | ||
|
|
||
| const shouldContinue = message.stop_reason === 'tool_use' && step.toolUses.length > 0 | ||
|
|
||
| if (shouldContinue) { | ||
| const results = await Promise.all( | ||
| step.toolUses.map(async toolUse => executeTool({ tools, toolUse })), | ||
| ) | ||
|
|
||
| for (const { toolResult } of results) { | ||
| step.toolResults.push(toolResult) | ||
| } | ||
|
|
||
| conversation.push({ | ||
| content: step.toolResults, | ||
| role: 'user', | ||
| }) | ||
| } | ||
|
|
||
| return { | ||
| currentStep: undefined, | ||
| shouldContinue, | ||
| totalUsage: nextTotalUsage, | ||
| usage: nextUsage, | ||
| } | ||
| } |
There was a problem hiding this comment.
The step object is mutated by pushing to step.toolResults after it has already been pushed to the steps array. This can make the code harder to reason about. It would be cleaner to gather all information for the step before creating it, and then push the immutable step object to the array.
const handleMessageStop = async ({ conversation, currentStep, steps, tools, totalUsage }: HandleMessageStopOptions): Promise<ProcessEventResult> => {
const message = finalizeCurrentMessage(currentStep)
const stepUsage = message.usage
const nextUsage = stepUsage
const nextTotalUsage = addUsage(totalUsage, stepUsage)
conversation.push(createAssistantMessageParam(message))
const toolUses = getToolUses(message)
const shouldContinue = message.stop_reason === 'tool_use' && toolUses.length > 0
let toolResults: AnthropicToolResultBlockParam[] = []
if (shouldContinue) {
const results = await Promise.all(
toolUses.map(async toolUse => executeTool({ tools, toolUse })),
)
toolResults = results.map(({ toolResult }) => toolResult)
conversation.push({
content: toolResults,
role: 'user',
})
}
const step = createStep(message, toolResults)
steps.push(step)
return {
currentStep: undefined,
shouldContinue,
totalUsage: nextTotalUsage,
usage: nextUsage,
}
}
|
Unfortunately, I currently do not plan to support the Anthropic format. I support the Chat Completion API due to its widespread usage, and the Responses API because it is based on an open standard. (https://openresponses.org) |
2 participants
No description provided.