mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-26 07:30:11 +00:00
fix: sort tool_definitions() for deterministic LLM tool ordering (#582)
* fix: sort tool_definitions() for deterministic LLM tool ordering HashMap iteration order is non-deterministic, causing the LLM to receive tools in different orders across calls. Sort alphabetically by name to eliminate position bias in tool selection. Closes #566 Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * refactor: use sort_unstable_by for tool definitions ordering Stable sort is unnecessary since tool names are unique. Unstable sort avoids the overhead of preserving equal-element order. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: repair bad merge in registry.rs (missing closing brace and test attribute) The merge of main into fix/sort-tool-definitions dropped the closing `}` of test_tool_definitions_sorted_alphabetically and the `#[tokio::test]` attribute on test_retain_only_filters_tools, causing an unclosed delimiter parse error that failed all CI jobs. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
9ae04f14e3
commit
c87525d81f
+51
-2
@@ -195,7 +195,8 @@ impl ToolRegistry {
|
||||
|
||||
/// Get tool definitions for LLM function calling.
|
||||
pub async fn tool_definitions(&self) -> Vec<ToolDefinition> {
|
||||
self.tools
|
||||
let mut defs: Vec<ToolDefinition> = self
|
||||
.tools
|
||||
.read()
|
||||
.await
|
||||
.values()
|
||||
@@ -204,7 +205,9 @@ impl ToolRegistry {
|
||||
description: tool.description().to_string(),
|
||||
parameters: tool.parameters_schema(),
|
||||
})
|
||||
.collect()
|
||||
.collect();
|
||||
defs.sort_unstable_by(|a, b| a.name.cmp(&b.name));
|
||||
defs
|
||||
}
|
||||
|
||||
/// Get tool definitions for specific tools.
|
||||
@@ -760,6 +763,52 @@ mod tests {
|
||||
assert_ne!(desc, "EVIL SHADOW");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_tool_definitions_sorted_alphabetically() {
|
||||
// Create tools with names that would NOT be alphabetical if inserted in this order.
|
||||
struct ToolZ;
|
||||
struct ToolA;
|
||||
struct ToolM;
|
||||
|
||||
macro_rules! impl_tool {
|
||||
($ty:ident, $name:expr) => {
|
||||
#[async_trait::async_trait]
|
||||
impl Tool for $ty {
|
||||
fn name(&self) -> &str {
|
||||
$name
|
||||
}
|
||||
fn description(&self) -> &str {
|
||||
$name
|
||||
}
|
||||
fn parameters_schema(&self) -> serde_json::Value {
|
||||
serde_json::json!({})
|
||||
}
|
||||
async fn execute(
|
||||
&self,
|
||||
_: serde_json::Value,
|
||||
_: &crate::context::JobContext,
|
||||
) -> Result<crate::tools::tool::ToolOutput, crate::tools::tool::ToolError> {
|
||||
unreachable!()
|
||||
}
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
impl_tool!(ToolZ, "zebra");
|
||||
impl_tool!(ToolA, "alpha");
|
||||
impl_tool!(ToolM, "middle");
|
||||
|
||||
let registry = ToolRegistry::new();
|
||||
// Register in non-alphabetical order
|
||||
registry.register(Arc::new(ToolZ)).await;
|
||||
registry.register(Arc::new(ToolA)).await;
|
||||
registry.register(Arc::new(ToolM)).await;
|
||||
|
||||
let defs = registry.tool_definitions().await;
|
||||
let names: Vec<&str> = defs.iter().map(|d| d.name.as_str()).collect();
|
||||
assert_eq!(names, vec!["alpha", "middle", "zebra"]);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_retain_only_filters_tools() {
|
||||
let registry = ToolRegistry::new();
|
||||
|
||||
Reference in New Issue
Block a user