From d24135915316eb9106bab23d2f23ca19e2e8b7db Mon Sep 17 00:00:00 2001 From: Saboor Hassan Date: Wed, 31 Dec 2025 01:54:41 +0500 Subject: [PATCH] fix(translator): address PR feedback for tool name sanitization - Pre-compile sanitization regex for better performance. - Optimize SanitizeFunctionName for conciseness and correctness. - Handle 64-char edge cases by truncating before prepending underscore. - Fix bug in Antigravity translator (incorrect join index). - Refactor Gemini translators to avoid redundant sanitization calls. - Add comprehensive unit tests including 64-char edge cases. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- .../claude/antigravity_claude_request.go | 5 ++-- .../claude/gemini-cli_claude_request.go | 5 ++-- .../gemini/claude/gemini_claude_request.go | 5 ++-- internal/util/sanitize_test.go | 1 + internal/util/util.go | 23 ++++++++++--------- 5 files changed, 22 insertions(+), 17 deletions(-) diff --git a/internal/translator/antigravity/claude/antigravity_claude_request.go b/internal/translator/antigravity/claude/antigravity_claude_request.go index cd6c0b6f..afbaf8be 100644 --- a/internal/translator/antigravity/claude/antigravity_claude_request.go +++ b/internal/translator/antigravity/claude/antigravity_claude_request.go @@ -225,11 +225,12 @@ func ConvertClaudeRequestToAntigravity(modelName string, inputRawJSON []byte, _ } else if contentTypeResult.Type == gjson.String && contentTypeResult.String() == "tool_result" { toolCallID := contentResult.Get("tool_use_id").String() if toolCallID != "" { - funcName := util.SanitizeFunctionName(toolCallID) + rawFuncName := toolCallID toolCallIDs := strings.Split(toolCallID, "-") if len(toolCallIDs) > 1 { - funcName = util.SanitizeFunctionName(strings.Join(toolCallIDs[0:len(toolCallIDs)-2], "-")) + rawFuncName = strings.Join(toolCallIDs[0:len(toolCallIDs)-1], "-") } + funcName := util.SanitizeFunctionName(rawFuncName) functionResponseResult := contentResult.Get("content") functionResponseJSON := `{}` diff --git a/internal/translator/gemini-cli/claude/gemini-cli_claude_request.go b/internal/translator/gemini-cli/claude/gemini-cli_claude_request.go index 1c252b0a..505f5956 100644 --- a/internal/translator/gemini-cli/claude/gemini-cli_claude_request.go +++ b/internal/translator/gemini-cli/claude/gemini-cli_claude_request.go @@ -107,11 +107,12 @@ func ConvertClaudeRequestToCLI(modelName string, inputRawJSON []byte, _ bool) [] if toolCallID == "" { return true } - funcName := util.SanitizeFunctionName(toolCallID) + rawFuncName := toolCallID toolCallIDs := strings.Split(toolCallID, "-") if len(toolCallIDs) > 1 { - funcName = util.SanitizeFunctionName(strings.Join(toolCallIDs[0:len(toolCallIDs)-1], "-")) + rawFuncName = strings.Join(toolCallIDs[0:len(toolCallIDs)-1], "-") } + funcName := util.SanitizeFunctionName(rawFuncName) responseData := contentResult.Get("content").Raw part := `{"functionResponse":{"name":"","response":{"result":""}}}` part, _ = sjson.Set(part, "functionResponse.name", funcName) diff --git a/internal/translator/gemini/claude/gemini_claude_request.go b/internal/translator/gemini/claude/gemini_claude_request.go index cefb5c43..d7abb98d 100644 --- a/internal/translator/gemini/claude/gemini_claude_request.go +++ b/internal/translator/gemini/claude/gemini_claude_request.go @@ -100,11 +100,12 @@ func ConvertClaudeRequestToGemini(modelName string, inputRawJSON []byte, _ bool) if toolCallID == "" { return true } - funcName := util.SanitizeFunctionName(toolCallID) + rawFuncName := toolCallID toolCallIDs := strings.Split(toolCallID, "-") if len(toolCallIDs) > 1 { - funcName = util.SanitizeFunctionName(strings.Join(toolCallIDs[0:len(toolCallIDs)-1], "-")) + rawFuncName = strings.Join(toolCallIDs[0:len(toolCallIDs)-1], "-") } + funcName := util.SanitizeFunctionName(rawFuncName) responseData := contentResult.Get("content").Raw part := `{"functionResponse":{"name":"","response":{"result":""}}}` part, _ = sjson.Set(part, "functionResponse.name", funcName) diff --git a/internal/util/sanitize_test.go b/internal/util/sanitize_test.go index 1729492c..06a0ca34 100644 --- a/internal/util/sanitize_test.go +++ b/internal/util/sanitize_test.go @@ -26,6 +26,7 @@ func TestSanitizeFunctionName(t *testing.T) { {"Exactly 64 chars", "this_is_a_very_long_name_that_exactly_reaches_sixty_four_charact", "this_is_a_very_long_name_that_exactly_reaches_sixty_four_charact"}, {"Too long (65 chars)", "this_is_a_very_long_name_that_exactly_reaches_sixty_four_charactX", "this_is_a_very_long_name_that_exactly_reaches_sixty_four_charact"}, {"Very long", "this_is_a_very_long_name_that_exceeds_the_sixty_four_character_limit_for_function_names", "this_is_a_very_long_name_that_exceeds_the_sixty_four_character_l"}, + {"Starts with digit (64 chars total)", "1234567890123456789012345678901234567890123456789012345678901234", "_123456789012345678901234567890123456789012345678901234567890123"}, {"Empty", "", ""}, {"Single character invalid", "@", "_"}, {"Single character valid", "a", "a"}, diff --git a/internal/util/util.go b/internal/util/util.go index 86b807de..ecf95943 100644 --- a/internal/util/util.go +++ b/internal/util/util.go @@ -15,28 +15,29 @@ import ( log "github.com/sirupsen/logrus" ) +var functionNameSanitizer = regexp.MustCompile(`[^a-zA-Z0-9_.:-]`) + // SanitizeFunctionName ensures a function name matches the requirements for Gemini/Vertex AI. // It replaces invalid characters with underscores, ensures it starts with a letter or underscore, // and truncates it to 64 characters if necessary. // Regex Rule: [^a-zA-Z0-9_.:-] replaced with _. func SanitizeFunctionName(name string) string { if name == "" { - return name + return "" } // Replace invalid characters with underscore - re := regexp.MustCompile(`[^a-zA-Z0-9_.:-]`) - sanitized := re.ReplaceAllString(name, "_") + sanitized := functionNameSanitizer.ReplaceAllString(name, "_") // Ensure it starts with a letter or underscore - if len(sanitized) > 0 { - first := sanitized[0] - if !((first >= 'a' && first <= 'z') || (first >= 'A' && first <= 'Z') || first == '_') { - // If it starts with an allowed character but not allowed at the beginning, - // we must prepend an underscore. - sanitized = "_" + sanitized + first := sanitized[0] + if !((first >= 'a' && first <= 'z') || (first >= 'A' && first <= 'Z') || first == '_') { + // If it starts with an allowed character but not allowed at the beginning, + // we must prepend an underscore. + // To stay within the 64-character limit while prepending, we may need to truncate first. + if len(sanitized) >= 64 { + sanitized = sanitized[:63] } - } else { - sanitized = "_" + sanitized = "_" + sanitized } // Truncate to 64 characters