-
Notifications
You must be signed in to change notification settings - Fork 188
feat: add code execution tool support for OpenAI, Anthropic and Gemini #1333
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: graphite-base/1333
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
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -158,23 +158,27 @@ type AnthropicMessage struct { | |
| type AnthropicContent struct { | ||
| ContentStr *string | ||
| ContentBlocks []AnthropicContentBlock | ||
| ContentBlock *AnthropicContentBlock // For "bash_code_execution_tool_result" | ||
| } | ||
|
|
||
| // MarshalJSON implements custom JSON marshalling for AnthropicContent. | ||
| // It marshals either ContentStr or ContentBlocks directly without wrapping. | ||
| func (mc AnthropicContent) MarshalJSON() ([]byte, error) { | ||
| // Validation: ensure only one field is set at a time | ||
| if mc.ContentStr != nil && mc.ContentBlocks != nil { | ||
| return nil, fmt.Errorf("both ContentStr and ContentBlocks are set; only one should be non-nil") | ||
| if mc.ContentStr != nil && mc.ContentBlocks != nil && mc.ContentBlock != nil { | ||
| return nil, fmt.Errorf("both ContentStr, ContentBlocks and ContentBlock are set; only one should be non-nil") | ||
| } | ||
|
|
||
| if mc.ContentStr != nil { | ||
| return sonic.Marshal(*mc.ContentStr) | ||
| } | ||
| if mc.ContentBlocks != nil { | ||
| if mc.ContentBlock != nil && mc.ContentBlocks == nil { | ||
| return sonic.Marshal(*mc.ContentBlock) | ||
| } | ||
| if mc.ContentBlocks != nil && mc.ContentBlock == nil { | ||
| return sonic.Marshal(mc.ContentBlocks) | ||
| } | ||
| // If both are nil, return null | ||
| // If all are nil, return null | ||
| return sonic.Marshal(nil) | ||
| } | ||
|
|
||
|
|
@@ -188,31 +192,40 @@ func (mc *AnthropicContent) UnmarshalJSON(data []byte) error { | |
| return nil | ||
| } | ||
|
|
||
| // Try to unmarshal as a direct ContentBlock | ||
| var contentBlock AnthropicContentBlock | ||
| if err := sonic.Unmarshal(data, &contentBlock); err == nil { | ||
| mc.ContentBlock = &contentBlock | ||
| return nil | ||
| } | ||
|
|
||
| // Try to unmarshal as a direct array of ContentBlock | ||
| var arrayContent []AnthropicContentBlock | ||
| if err := sonic.Unmarshal(data, &arrayContent); err == nil { | ||
| mc.ContentBlocks = arrayContent | ||
| return nil | ||
| } | ||
|
|
||
| return fmt.Errorf("content field is neither a string nor an array of ContentBlock") | ||
| return fmt.Errorf("content field is neither a string, ContentBlock nor an array of ContentBlock") | ||
| } | ||
|
Comment on lines
+195
to
210
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. UnmarshalJSON order may cause incorrect parsing. The unmarshaling logic tries to parse as This could cause arrays of content blocks to be incorrectly parsed as a single Consider checking for array first, or adding a discriminator check (e.g., verify the Proposed fix - reorder to try array first func (mc *AnthropicContent) UnmarshalJSON(data []byte) error {
// First, try to unmarshal as a direct string
var stringContent string
if err := sonic.Unmarshal(data, &stringContent); err == nil {
mc.ContentStr = &stringContent
return nil
}
- // Try to unmarshal as a direct ContentBlock
- var contentBlock AnthropicContentBlock
- if err := sonic.Unmarshal(data, &contentBlock); err == nil {
- mc.ContentBlock = &contentBlock
- return nil
- }
-
// Try to unmarshal as a direct array of ContentBlock
var arrayContent []AnthropicContentBlock
if err := sonic.Unmarshal(data, &arrayContent); err == nil {
mc.ContentBlocks = arrayContent
return nil
}
+ // Try to unmarshal as a direct ContentBlock (single object, not array)
+ var contentBlock AnthropicContentBlock
+ if err := sonic.Unmarshal(data, &contentBlock); err == nil {
+ // Only accept as single ContentBlock if Type is set (to avoid false positives)
+ if contentBlock.Type != "" {
+ mc.ContentBlock = &contentBlock
+ return nil
+ }
+ }
+
return fmt.Errorf("content field is neither a string, ContentBlock nor an array of ContentBlock")
} |
||
|
|
||
| type AnthropicContentBlockType string | ||
|
|
||
| const ( | ||
| AnthropicContentBlockTypeText AnthropicContentBlockType = "text" | ||
| AnthropicContentBlockTypeImage AnthropicContentBlockType = "image" | ||
| AnthropicContentBlockTypeDocument AnthropicContentBlockType = "document" | ||
| AnthropicContentBlockTypeToolUse AnthropicContentBlockType = "tool_use" | ||
| AnthropicContentBlockTypeServerToolUse AnthropicContentBlockType = "server_tool_use" | ||
| AnthropicContentBlockTypeToolResult AnthropicContentBlockType = "tool_result" | ||
| AnthropicContentBlockTypeWebSearchToolResult AnthropicContentBlockType = "web_search_tool_result" | ||
| AnthropicContentBlockTypeWebSearchResult AnthropicContentBlockType = "web_search_result" | ||
| AnthropicContentBlockTypeMCPToolUse AnthropicContentBlockType = "mcp_tool_use" | ||
| AnthropicContentBlockTypeMCPToolResult AnthropicContentBlockType = "mcp_tool_result" | ||
| AnthropicContentBlockTypeThinking AnthropicContentBlockType = "thinking" | ||
| AnthropicContentBlockTypeRedactedThinking AnthropicContentBlockType = "redacted_thinking" | ||
| AnthropicContentBlockTypeText AnthropicContentBlockType = "text" | ||
| AnthropicContentBlockTypeImage AnthropicContentBlockType = "image" | ||
| AnthropicContentBlockTypeDocument AnthropicContentBlockType = "document" | ||
| AnthropicContentBlockTypeToolUse AnthropicContentBlockType = "tool_use" | ||
| AnthropicContentBlockTypeServerToolUse AnthropicContentBlockType = "server_tool_use" | ||
| AnthropicContentBlockTypeToolResult AnthropicContentBlockType = "tool_result" | ||
| AnthropicContentBlockTypeWebSearchToolResult AnthropicContentBlockType = "web_search_tool_result" | ||
| AnthropicContentBlockTypeWebSearchResult AnthropicContentBlockType = "web_search_result" | ||
| AnthropicContentBlockTypeMCPToolUse AnthropicContentBlockType = "mcp_tool_use" | ||
| AnthropicContentBlockTypeMCPToolResult AnthropicContentBlockType = "mcp_tool_result" | ||
| AnthropicContentBlockTypeBashCodeExecutionToolResult AnthropicContentBlockType = "bash_code_execution_tool_result" | ||
| AnthropicContentBlockTypeBashCodeExecutionResult AnthropicContentBlockType = "bash_code_execution_result" | ||
| AnthropicContentBlockTypeThinking AnthropicContentBlockType = "thinking" | ||
| AnthropicContentBlockTypeRedactedThinking AnthropicContentBlockType = "redacted_thinking" | ||
| ) | ||
|
|
||
| // AnthropicContentBlock represents content in Anthropic message format | ||
|
|
@@ -236,6 +249,9 @@ type AnthropicContentBlock struct { | |
| URL *string `json:"url,omitempty"` // For web_search_result content | ||
| EncryptedContent *string `json:"encrypted_content,omitempty"` // For web_search_result content | ||
| PageAge *string `json:"page_age,omitempty"` // For web_search_result content | ||
| StdOut *string `json:"stdout,omitempty"` // For bash_code_execution_result content | ||
| StdErr *string `json:"stderr,omitempty"` // For bash_code_execution_result content | ||
| ReturnCode *int `json:"return_code,omitempty"` // For bash_code_execution_result content | ||
| } | ||
|
|
||
| // AnthropicSource represents image or document source in Anthropic format | ||
|
|
@@ -360,10 +376,12 @@ const ( | |
| type AnthropicToolName string | ||
|
|
||
| const ( | ||
| AnthropicToolNameComputer AnthropicToolName = "computer" | ||
| AnthropicToolNameWebSearch AnthropicToolName = "web_search" | ||
| AnthropicToolNameBash AnthropicToolName = "bash" | ||
| AnthropicToolNameTextEditor AnthropicToolName = "str_replace_based_edit_tool" | ||
| AnthropicToolNameComputer AnthropicToolName = "computer" | ||
| AnthropicToolNameWebSearch AnthropicToolName = "web_search" | ||
| AnthropicToolNameBash AnthropicToolName = "bash" | ||
| AnthropicToolNameTextEditor AnthropicToolName = "str_replace_based_edit_tool" | ||
| AnthropicToolNameBashCodeExecution AnthropicToolName = "bash_code_execution" | ||
| AnthropicToolNameCodeExecution AnthropicToolName = "code_execution" | ||
| ) | ||
|
|
||
| type AnthropicToolComputerUse struct { | ||
|
|
@@ -451,6 +469,7 @@ type AnthropicMessageResponse struct { | |
| Model string `json:"model"` | ||
| StopReason AnthropicStopReason `json:"stop_reason,omitempty"` | ||
| StopSequence *string `json:"stop_sequence,omitempty"` | ||
| Container *AnthropicContainer `json:"container,omitempty"` | ||
| Usage *AnthropicUsage `json:"usage,omitempty"` | ||
| } | ||
|
|
||
|
|
@@ -475,6 +494,11 @@ type AnthropicUsage struct { | |
| OutputTokens int `json:"output_tokens"` | ||
| } | ||
|
|
||
| type AnthropicContainer struct { | ||
| ID string `json:"id"` | ||
| ExpiresAt string `json:"expires_at"` // ISO 8601 timestamp when the container expires (sent by Anthropic) | ||
| } | ||
|
|
||
| type AnthropicUsageCacheCreation struct { | ||
| Ephemeral5mInputTokens int `json:"ephemeral_5m_input_tokens"` | ||
| Ephemeral1hInputTokens int `json:"ephemeral_1h_input_tokens"` | ||
|
|
||
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.
MarshalJSON validation logic is incorrect.
The validation at line 168 checks if all three fields (
ContentStr,ContentBlocks, andContentBlock) are non-nil simultaneously. However, the intended logic should check if more than one field is set (any two), not if all three are set. The current condition only triggers when all three are set, which is unlikely.Additionally, the marshal logic at lines 175-180 has potential issues:
mc.ContentBlock != nil && mc.ContentBlocks == nilmc.ContentBlocks != nil && mc.ContentBlock == nilBut what happens if both
ContentBlockandContentBlocksare set? The first condition fails, the second condition fails, and it falls through to returnnilat line 182, silently losing data.Proposed fix
func (mc AnthropicContent) MarshalJSON() ([]byte, error) { - // Validation: ensure only one field is set at a time - if mc.ContentStr != nil && mc.ContentBlocks != nil && mc.ContentBlock != nil { - return nil, fmt.Errorf("both ContentStr, ContentBlocks and ContentBlock are set; only one should be non-nil") + // Count how many fields are set + setCount := 0 + if mc.ContentStr != nil { + setCount++ } + if mc.ContentBlocks != nil { + setCount++ + } + if mc.ContentBlock != nil { + setCount++ + } + if setCount > 1 { + return nil, fmt.Errorf("multiple content fields are set; only one of ContentStr, ContentBlocks, or ContentBlock should be non-nil") + } if mc.ContentStr != nil { return sonic.Marshal(*mc.ContentStr) } - if mc.ContentBlock != nil && mc.ContentBlocks == nil { + if mc.ContentBlock != nil { return sonic.Marshal(*mc.ContentBlock) } - if mc.ContentBlocks != nil && mc.ContentBlock == nil { + if mc.ContentBlocks != nil { return sonic.Marshal(mc.ContentBlocks) } // If all are nil, return null return sonic.Marshal(nil) }🤖 Prompt for AI Agents