-
Notifications
You must be signed in to change notification settings - Fork 71
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.
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?
fix: sort tools alphabetically while generating hash #522
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a parameter normalization step for hashing by sorting tools in model parameters, integrates it into request hash generation, updates imports, and adjusts a comment accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant Plugin
participant Normalizer as normalizeParamsForHashing
participant Hasher as Hash Generator
Caller->>Plugin: generateRequestHash(req)
Plugin->>Normalizer: Normalize req.Params (sort tools by function name)
alt Params is nil or ≤1 tool
Normalizer-->>Plugin: Return original/unchanged params
else Multiple tools
Normalizer-->>Plugin: Return params with tools sorted
end
Plugin->>Hasher: Compute hash over normalized params + other inputs
Hasher-->>Plugin: Hash value
Plugin-->>Caller: Return request hash
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugins/semanticcache/utils.go
(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
plugins/semanticcache/utils.go (1)
core/schemas/bifrost.go (4)
RequestInput
(117-123)ModelParameters
(295-312)Tool
(331-335)Function
(324-328)
sort.Slice(sortedTools, func(i, j int) bool { | ||
return sortedTools[i].Function.Name < sortedTools[j].Function.Name | ||
}) |
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.
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.
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.
Summary
Improve semantic cache consistency by normalizing tool order in request parameters before hashing.
Changes
normalizeParamsForHashing
function that sorts tools by function name before hashinggenerateRequestHash
to use the normalized parametersType of change
Affected areas
How to test
Breaking changes
Related issues
Fixes inconsistent cache hits when tools are provided in different orders.
Security considerations
No security implications as this only affects internal hash generation.
Checklist
docs/contributing/README.md
and followed the guidelines