Skip to content

fix: sort tools alphabetically while generating hash #522

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: sort tools alphabetically while generating hash #522

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 31 additions & 2 deletions plugins/semanticcache/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/json"
"fmt"
"maps"
"sort"
"strings"
"time"

Expand Down Expand Up @@ -87,14 +88,14 @@ func (plugin *Plugin) generateEmbedding(ctx context.Context, text string) ([]flo
// - string: Hexadecimal representation of the xxhash
// - error: Any error that occurred during request normalization or hashing
func (plugin *Plugin) generateRequestHash(req *schemas.BifrostRequest, requestType schemas.RequestType) (string, error) {
// Create a hash input structure that includes both input and parameters
// Create a hash input structure with normalized data
hashInput := struct {
Input schemas.RequestInput `json:"input"`
Params *schemas.ModelParameters `json:"params,omitempty"`
Stream bool `json:"stream,omitempty"`
}{
Input: *plugin.getInputForCaching(req),
Params: req.Params,
Params: plugin.normalizeParamsForHashing(req.Params),
Stream: plugin.isStreamingRequest(requestType),
}

Expand Down Expand Up @@ -459,3 +460,31 @@ func (plugin *Plugin) isConversationHistoryThresholdExceeded(req *schemas.Bifros
return false
}
}

// normalizeParamsForHashing creates a normalized copy of parameters with sorted tools.
// Returns nil if the input params are nil to maintain the same structure.
func (plugin *Plugin) normalizeParamsForHashing(params *schemas.ModelParameters) *schemas.ModelParameters {
if params == nil {
return nil
}

// Only create a copy if tools need sorting
if params.Tools == nil || len(*params.Tools) <= 1 {
return params
}

// Create a shallow copy of params
normalized := *params

// Create and sort tools copy
sortedTools := make([]schemas.Tool, len(*params.Tools))
copy(sortedTools, *params.Tools)

// Use Go's built-in sort for efficiency
sort.Slice(sortedTools, func(i, j int) bool {
return sortedTools[i].Function.Name < sortedTools[j].Function.Name
})
Comment on lines +484 to +486
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Ensure the sort canonicalizes tools with identical names

Line [485] compares only Function.Name. When two requests carry the same set of tools but the tools share a name (common for different versions of the same function), the comparator treats them as equal and leaves their original order untouched. That means requests ["foo(v1)", "foo(v2)"] and ["foo(v2)", "foo(v1)"] still hash differently, so the cache remains inconsistent for precisely the class of collisions we’re trying to fix. Please introduce deterministic tie-breakers (e.g., fall back to type/ID and a serialized representation of the tool) so equal-name tools always end up in the same order.

You can address it like this:

-	sort.Slice(sortedTools, func(i, j int) bool {
-		return sortedTools[i].Function.Name < sortedTools[j].Function.Name
-	})
+	sort.Slice(sortedTools, func(i, j int) bool {
+		left := sortedTools[i]
+		right := sortedTools[j]
+
+		if left.Function.Name != right.Function.Name {
+			return left.Function.Name < right.Function.Name
+		}
+		if left.Type != right.Type {
+			return left.Type < right.Type
+		}
+		if (left.ID != nil) != (right.ID != nil) {
+			return left.ID != nil
+		}
+		if left.ID != nil && right.ID != nil && *left.ID != *right.ID {
+			return *left.ID < *right.ID
+		}
+
+		leftKey, _ := json.Marshal(left)
+		rightKey, _ := json.Marshal(right)
+		return string(leftKey) < string(rightKey)
+	})
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
sort.Slice(sortedTools, func(i, j int) bool {
return sortedTools[i].Function.Name < sortedTools[j].Function.Name
})
sort.Slice(sortedTools, func(i, j int) bool {
left := sortedTools[i]
right := sortedTools[j]
if left.Function.Name != right.Function.Name {
return left.Function.Name < right.Function.Name
}
if left.Type != right.Type {
return left.Type < right.Type
}
if (left.ID != nil) != (right.ID != nil) {
return left.ID != nil
}
if left.ID != nil && right.ID != nil && *left.ID != *right.ID {
return *left.ID < *right.ID
}
leftKey, _ := json.Marshal(left)
rightKey, _ := json.Marshal(right)
return string(leftKey) < string(rightKey)
})
🤖 Prompt for AI Agents 
In plugins/semanticcache/utils.go around lines 484-486, the sort comparator only
compares Function.Name which leaves tools with identical names in their original
(non-deterministic) order; update the comparator to canonicalize ties by
performing deterministic fallbacks: first compare Function.Name, then
Function.Type (or Go type), then a stable Function.ID (if present), and finally
a serialized representation (e.g., JSON or a stable string) of the tool/function
as the last tie-breaker so tools with the same name always sort the same way
across requests.


normalized.Tools = &sortedTools
return &normalized
}