Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,6 @@ mod tests {
assert!(result_for_assistant
.as_deref()
.unwrap_or_default()
.contains("already loaded in the current conversation"));
.contains("already loaded in the current context"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,12 @@ fn get_tool_spec_load_observation(message: &Message) -> Option<GetToolSpecLoadOb
#[cfg(test)]
mod tests {
use super::{collect_product_loaded_deferred_tool_specs, ProductLoadedDeferredToolSpecs};
use crate::agentic::core::{Message, ToolResult};
use crate::agentic::core::{Message, ToolCall, ToolResult};
use crate::agentic::session::ContextCompressor;
use openbitfun_agent_tools::{
resolve_get_tool_spec_execution_plan, validate_deferred_tool_usage,
GetToolSpecExecutionPlan, GET_TOOL_SPEC_TOOL_NAME,
};
use serde_json::json;

fn loaded_spec(tool_name: &str) -> openbitfun_agent_tools::LoadedDeferredToolSpec {
Expand All @@ -77,6 +82,108 @@ mod tests {
}
}

#[test]
fn compaction_requires_reload_only_when_the_full_spec_leaves_the_context() {
let deferred_tools = vec!["Cron".to_string()];
let input = json!({ "tool_name": "Cron" });
let spec_result = Message::tool_result(ToolResult {
tool_id: "load-cron".to_string(),
tool_name: GET_TOOL_SPEC_TOOL_NAME.to_string(),
effective_tool_name: None,
result: json!({
"tool_name": "Cron",
"catalog_generation": 42,
"description": "Manage scheduled jobs.",
"input_schema": { "type": "object", "properties": { "action": { "enum": ["list"] } } },
}),
result_for_assistant: None,
is_error: false,
duration_ms: None,
image_attachments: None,
});
let history = vec![
Message::user("Check the scheduled jobs.".to_string()),
Message::assistant_with_tools(
String::new(),
vec![ToolCall {
tool_id: "load-cron".to_string(),
tool_name: GET_TOOL_SPEC_TOOL_NAME.to_string(),
arguments: input.clone(),
raw_arguments: None,
is_error: false,
parse_error: None,
recovered_from_truncation: false,
repair_kind: Default::default(),
}],
),
spec_result.clone(),
];
let compressor = ContextCompressor::new();

// The same summary may be generated with or without a retained tool
// result. Only the actual retained result is an execution receipt.
for (recent_tokens, retains_spec) in [(0, false), (10_000, true)] {
let plan = compressor
.plan_compression("session", &history, 128_000, recent_tokens)
.unwrap()
.unwrap();
let mut compressed = compressor
.compress_plan_with_contract(
"session",
plan,
None,
"The Cron definition was loaded with GetToolSpec earlier.".to_string(),
)
.unwrap()
.messages;
let loaded = collect_product_loaded_deferred_tool_specs(&compressed, &deferred_tools);
let admission = validate_deferred_tool_usage(
"Cron",
&deferred_tools,
&loaded,
42,
GET_TOOL_SPEC_TOOL_NAME,
);
let names = loaded
.iter()
.map(|spec| spec.tool_name.clone())
.collect::<Vec<_>>();
let reload_plan = resolve_get_tool_spec_execution_plan(&input, &names).unwrap();

if retains_spec {
assert_eq!(loaded, vec![loaded_spec("Cron")]);
assert!(admission.is_ok());
assert!(matches!(
reload_plan,
GetToolSpecExecutionPlan::DuplicateLoad(_)
));
} else {
assert!(loaded.is_empty());
assert!(admission
.unwrap_err()
.to_string()
.contains("reload it even if the summary says it was loaded"));
assert!(matches!(
reload_plan,
GetToolSpecExecutionPlan::LoadDetail { tool_name: "Cron" }
));

// Reading the definition again restores normal admission.
compressed.push(spec_result.clone());
let reloaded =
collect_product_loaded_deferred_tool_specs(&compressed, &deferred_tools);
validate_deferred_tool_usage(
"Cron",
&deferred_tools,
&reloaded,
42,
GET_TOOL_SPEC_TOOL_NAME,
)
.expect("a fresh spec must unlock Cron after compaction");
}
}
}

#[test]
fn product_loaded_spec_state_collects_visible_get_tool_spec_results() {
let visible_get_tool_spec_result = Message::tool_result(ToolResult {
Expand Down
2 changes: 1 addition & 1 deletion src/crates/execution/agent-runtime/src/prompt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const DIRECT_TOOL_LISTING_GUIDANCE: &str = r#"Their definitions are already avai
Each entry below is a directly callable tool name."#;
const DEFERRED_TOOL_LISTING_TITLE: &str = "## Deferred tools";
const DEFERRED_TOOL_LISTING_GUIDANCE: &str = r#"Their definitions are not loaded at the start of the conversation.
You must obtain the tool definition using GetToolSpec before you first invoke a deferred tool. Once its definition is available in the conversation, you can call it through CallDeferredTool.
Use GetToolSpec to read a deferred tool's full definition before invoking it through CallDeferredTool. Reuse it while the successful GetToolSpec result remains in the current context. If compaction or truncation removed that result, load it again; a summary or a past call does not keep the definition loaded.
Each entry below is a deferred tool name with an optional short description."#;

pub fn render_direct_tool_listing_body<'a>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,17 +70,20 @@ fn tool_listing_sections_render_only_present_sections() {
assert!(deferred_tool_listing
.contains("Their definitions are not loaded at the start of the conversation."));
assert!(deferred_tool_listing
.contains("You must obtain the tool definition using GetToolSpec before you first invoke a deferred tool."));
.contains("Use GetToolSpec to read a deferred tool's full definition before invoking it through CallDeferredTool."));
assert!(deferred_tool_listing.contains(
"Once its definition is available in the conversation, you can call it through CallDeferredTool."
"Reuse it while the successful GetToolSpec result remains in the current context."
));
assert!(deferred_tool_listing.contains(
"If compaction or truncation removed that result, load it again; a summary or a past call does not keep the definition loaded."
));
assert!(deferred_tool_listing
.contains("Each entry below is a deferred tool name with an optional short description."));
assert!(deferred_tool_listing.contains(
"## Direct tools\nTheir definitions are already available. You can call them directly.\nEach entry below is a directly callable tool name.\n\n<direct_tools>\n- Read\n- GetToolSpec\n- CallDeferredTool\n</direct_tools>"
));
assert!(deferred_tool_listing.contains(
"## Deferred tools\nTheir definitions are not loaded at the start of the conversation.\nYou must obtain the tool definition using GetToolSpec before you first invoke a deferred tool. Once its definition is available in the conversation, you can call it through CallDeferredTool."
"## Deferred tools\nTheir definitions are not loaded at the start of the conversation.\nUse GetToolSpec"
));
assert!(deferred_tool_listing.ends_with("Search: summary"));
}
Expand Down
3 changes: 2 additions & 1 deletion src/crates/execution/tool-contracts/src/deferred_tool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ pub fn call_deferred_tool_input_schema() -> Value {
"properties": {
"tool_name": {
"type": "string",
"description": "Exact deferred tool name previously loaded with GetToolSpec."
"description": "Exact deferred tool name whose full GetToolSpec result is still visible in the current context."
},
"args": {
"type": "object",
Expand All @@ -80,6 +80,7 @@ pub fn call_deferred_tool_short_description() -> String {
pub fn call_deferred_tool_description() -> String {
r#"Call a deferred tool after reading its full schema with GetToolSpec.

The full GetToolSpec result must still be visible in the current context. If compaction or truncation removed it, reload it with GetToolSpec first; a summary or a past call is not a loaded definition.
Pass the exact deferred tool name in tool_name and put only that tool's arguments inside args.
The order is important. ALWAYS output tool_name first, then args."#
.to_string()
Expand Down
11 changes: 8 additions & 3 deletions src/crates/execution/tool-contracts/src/framework.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ impl fmt::Display for DeferredToolUsageError {
get_tool_spec_tool_name,
} => write!(
formatter,
"Tool '{tool_name}' is deferred. Call {get_tool_spec_tool_name} first with {{\"tool_name\":\"{tool_name}\"}} to read its full usage instructions and input schema before invoking it."
"Tool '{tool_name}' has no loaded definition in the current context. Call {get_tool_spec_tool_name} with {{\"tool_name\":\"{tool_name}\"}} to read its full usage instructions and input schema before invoking it. If compaction removed an earlier definition, reload it even if the summary says it was loaded."
),
Self::StaleSpec {
tool_name,
Expand Down Expand Up @@ -371,7 +371,8 @@ pub fn get_tool_spec_short_description() -> String {
pub fn build_get_tool_spec_description() -> String {
r#"Read the full schema before first calling a deferred tool through CallDeferredTool.

Do not call GetToolSpec again for a tool whose definition is already loaded in the current conversation."#
Do not call GetToolSpec again while its successful result with the full definition is still visible in the current context.
If compaction or truncation removed that result, call GetToolSpec again before using CallDeferredTool. A summary mentioning a previously loaded tool or a past successful call does not load its definition. Reload also when the runtime reports a stale definition."#
.to_string()
}

Expand Down Expand Up @@ -439,7 +440,7 @@ pub fn validate_get_tool_spec_input(input: &Value) -> ValidationResult {

pub fn build_get_tool_spec_duplicate_load_hint(tool_name: &str) -> String {
format!(
"Tool '{}' is already loaded in the current conversation. Do not call GetToolSpec again for it. Use CallDeferredTool with tool_name '{}' and put the tool arguments inside args.",
"Tool '{}' is already loaded in the current context. Use CallDeferredTool with tool_name '{}' and put the tool arguments inside args. Reload with GetToolSpec only if its full definition leaves the context or the runtime reports it stale.",
tool_name, tool_name
)
}
Expand Down Expand Up @@ -2688,6 +2689,10 @@ mod tests {

assert!(description.contains("Read the full schema"));
assert!(description.contains("Do not call GetToolSpec again"));
assert!(description.contains("full definition is still visible in the current context"));
assert!(description
.contains("If compaction or truncation removed that result, call GetToolSpec again"));
assert!(description.contains("does not load its definition"));
}

#[test]
Expand Down
30 changes: 16 additions & 14 deletions src/crates/execution/tool-contracts/tests/tool_contracts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ fn call_deferred_tool_contract_uses_nested_object_arguments() {

assert!(call_deferred_tool_description()
.contains("The order is important. ALWAYS output tool_name first, then args."));
assert!(call_deferred_tool_description()
.contains("If compaction or truncation removed it, reload it with GetToolSpec first"));
assert_eq!(schema["additionalProperties"], false);
assert_eq!(schema["required"], json!(["tool_name", "args"]));
assert_eq!(schema["properties"]["args"]["type"], "object");
Expand Down Expand Up @@ -1470,7 +1472,7 @@ fn deferred_tool_usage_gate_preserves_get_tool_spec_unlock_contract() {
.expect_err("deferred tool should require GetToolSpec unlock");
assert_eq!(
err.to_string(),
"Tool 'WebFetch' is deferred. Call GetToolSpec first with {\"tool_name\":\"WebFetch\"} to read its full usage instructions and input schema before invoking it."
"Tool 'WebFetch' has no loaded definition in the current context. Call GetToolSpec with {\"tool_name\":\"WebFetch\"} to read its full usage instructions and input schema before invoking it. If compaction removed an earlier definition, reload it even if the summary says it was loaded."
);

let loaded_deferred_tool_specs = vec![LoadedDeferredToolSpec {
Expand Down Expand Up @@ -2030,7 +2032,7 @@ fn get_tool_spec_contract_escapes_assistant_detail_for_xml_sections() {
fn get_tool_spec_contract_preserves_duplicate_load_hint() {
assert_eq!(
build_get_tool_spec_duplicate_load_hint("WebFetch"),
"Tool 'WebFetch' is already loaded in the current conversation. Do not call GetToolSpec again for it. Use CallDeferredTool with tool_name 'WebFetch' and put the tool arguments inside args."
"Tool 'WebFetch' is already loaded in the current context. Use CallDeferredTool with tool_name 'WebFetch' and put the tool arguments inside args. Reload with GetToolSpec only if its full definition leaves the context or the runtime reports it stale."
);
}

Expand All @@ -2052,7 +2054,7 @@ fn get_tool_spec_contract_builds_duplicate_load_result() {
assert_eq!(
result_for_assistant.as_deref(),
Some(
"Tool 'WebFetch' is already loaded in the current conversation. Do not call GetToolSpec again for it. Use CallDeferredTool with tool_name 'WebFetch' and put the tool arguments inside args."
"Tool 'WebFetch' is already loaded in the current context. Use CallDeferredTool with tool_name 'WebFetch' and put the tool arguments inside args. Reload with GetToolSpec only if its full definition leaves the context or the runtime reports it stale."
)
);
assert_eq!(image_attachments, None);
Expand Down Expand Up @@ -2127,7 +2129,7 @@ fn get_tool_spec_contract_plans_duplicate_load_without_core_context() {
assert!(result_for_assistant
.as_deref()
.unwrap_or_default()
.contains("already loaded in the current conversation"));
.contains("already loaded in the current context"));
assert_eq!(image_attachments, None);
}

Expand Down Expand Up @@ -3058,7 +3060,7 @@ async fn get_tool_spec_detail_resolver_preserves_contextual_detail_contract() {
.expect("collapsed WebFetch detail");

assert_eq!(detail.tool_name, "WebFetch");
assert_eq!(detail.description, "WebFetch description for agentic");
assert_eq!(detail.description, "WebFetch description for Standard");
assert_eq!(
detail.input_schema["properties"]["agent"]["const"],
"Standard"
Expand All @@ -3067,7 +3069,7 @@ async fn get_tool_spec_detail_resolver_preserves_contextual_detail_contract() {
detail.to_value(),
json!({
"tool_name": "WebFetch",
"description": "WebFetch description for agentic",
"description": "WebFetch description for Standard",
"input_schema": {
"type": "object",
"properties": {
Expand Down Expand Up @@ -3118,7 +3120,7 @@ async fn get_tool_spec_catalog_provider_preserves_runtime_catalog_contract() {
.await
.expect("provider-backed detail");
assert_eq!(detail.tool_name, "WebFetch");
assert_eq!(detail.description, "WebFetch description for agentic");
assert_eq!(detail.description, "WebFetch description for Standard");
}

#[tokio::test]
Expand Down Expand Up @@ -3153,7 +3155,7 @@ async fn get_tool_spec_provider_execution_returns_duplicate_result_without_detai
assert!(result_for_assistant
.as_deref()
.unwrap_or_default()
.contains("already loaded in the current conversation"));
.contains("already loaded in the current context"));
assert_eq!(image_attachments, None);
}

Expand Down Expand Up @@ -3189,15 +3191,15 @@ async fn get_tool_spec_provider_execution_returns_detail_result_from_provider()
};

assert_eq!(data["tool_name"], "WebFetch");
assert_eq!(data["description"], "WebFetch description for agentic");
assert_eq!(data["description"], "WebFetch description for Standard");
assert_eq!(
data["input_schema"]["properties"]["agent"]["const"],
"Standard"
);
let assistant = result_for_assistant.expect("assistant detail");
assert!(assistant.contains("<description>\nWebFetch description for agentic"));
assert!(assistant.contains("<description>\nWebFetch description for Standard"));
assert!(assistant.contains("\"agent\""));
assert!(assistant.contains("\"agentic\""));
assert!(assistant.contains("\"Standard\""));
assert_eq!(image_attachments, None);
}

Expand Down Expand Up @@ -3267,7 +3269,7 @@ async fn get_tool_spec_runtime_facade_owns_execution_path() {
panic!("expected normal tool result");
};
assert_eq!(data["tool_name"], "WebFetch");
assert_eq!(data["description"], "WebFetch description for agentic");
assert_eq!(data["description"], "WebFetch description for Standard");
assert_eq!(
data["input_schema"]["properties"]["agent"]["const"],
"Standard"
Expand Down Expand Up @@ -3306,7 +3308,7 @@ async fn get_tool_spec_runtime_facade_owns_tool_result_vector_adapter_shape() {
assert_eq!(data["tool_name"], "WebFetch");
assert!(result_for_assistant
.expect("assistant detail")
.contains("<description>\nWebFetch description for agentic"));
.contains("<description>\nWebFetch description for Standard"));
assert_eq!(image_attachments, None);

let duplicate_runtime =
Expand Down Expand Up @@ -3339,7 +3341,7 @@ async fn get_tool_spec_runtime_facade_owns_tool_result_vector_adapter_shape() {
assert_eq!(
result_for_assistant.as_deref(),
Some(
"Tool 'WebFetch' is already loaded in the current conversation. Do not call GetToolSpec again for it. Use CallDeferredTool with tool_name 'WebFetch' and put the tool arguments inside args."
"Tool 'WebFetch' is already loaded in the current context. Use CallDeferredTool with tool_name 'WebFetch' and put the tool arguments inside args. Reload with GetToolSpec only if its full definition leaves the context or the runtime reports it stale."
)
);
assert!(image_attachments.is_none());
Expand Down
Loading
Loading