diff --git a/lib/components/fabro-agent/src/context_window.rs b/lib/components/fabro-agent/src/context_window.rs index 902056764..0a5d1e362 100644 --- a/lib/components/fabro-agent/src/context_window.rs +++ b/lib/components/fabro-agent/src/context_window.rs @@ -12,6 +12,7 @@ use fabro_types::{ }; use crate::memory::MemoryDocument; +use crate::native_tool::NativeTool; use crate::skills::{Skill, format_skills_prompt_section}; use crate::tool_registry::{ToolDefinitionWithSource, ToolSource}; @@ -221,7 +222,7 @@ fn memory_prompt_suffix(memory: &[MemoryDocument]) -> String { } fn skills_prompt_suffix(skills: &[Skill]) -> String { - let section = format_skills_prompt_section(skills); + let section = format_skills_prompt_section(skills, NativeTool::UseSkill.canonical_name()); if section.is_empty() { String::new() } else { diff --git a/lib/components/fabro-agent/src/native_tool.rs b/lib/components/fabro-agent/src/native_tool.rs index 64d4dc387..c2f9c8ff1 100644 --- a/lib/components/fabro-agent/src/native_tool.rs +++ b/lib/components/fabro-agent/src/native_tool.rs @@ -109,9 +109,16 @@ impl NativeTool { Self::Glob => "Glob", Self::WebSearch => "WebSearch", Self::WebFetch => "FetchURL", - // Kimi Code has no counterpart with these semantics: its - // TodoList replaces a whole list rather than mutating tasks, - // and it has no equivalent of the remaining tools. + Self::UseSkill => "Skill", + // Deliberately unmapped. Kimi Code's `Agent` launches a + // subagent and returns its result; fabro's spawn_agent returns + // a handle that send_input, wait, and close_agent then drive. + // Borrowing the name without the semantics would promise a + // result the tool does not return -- the same mistake as + // exposing incremental task tools under a whole-list name. + Self::SpawnAgent | Self::SendInput | Self::Wait | Self::CloseAgent => { + self.canonical_name() + } other => other.canonical_name(), }, } diff --git a/lib/components/fabro-agent/src/profiles/kimi.rs b/lib/components/fabro-agent/src/profiles/kimi.rs index 5b18bb127..a02d28ef4 100644 --- a/lib/components/fabro-agent/src/profiles/kimi.rs +++ b/lib/components/fabro-agent/src/profiles/kimi.rs @@ -1,8 +1,6 @@ use std::sync::Arc; -use fabro_llm::types::ToolDefinition; use fabro_model::{AgentProfileKind, Catalog, ProviderId}; -use strum::VariantArray; use super::EnvContext; use crate::agent_profile::AgentProfile; @@ -13,7 +11,7 @@ use crate::sandbox::Sandbox; use crate::skills::Skill; use crate::todo_runtime::TodoRuntime; use crate::todo_tools::make_todo_list_tool; -use crate::tool_registry::{RegisteredTool, ToolRegistry}; +use crate::tool_registry::ToolRegistry; use crate::tools::{WebFetchSummarizer, make_edit_file_tool, register_core_tools}; const CORE_PROMPT: &str = include_str!("prompts/kimi.md.j2"); @@ -39,48 +37,6 @@ files that have not been read, and the call will fail. Prefer Edit for any incre change: Write replaces the entire file, so using it to make a small edit discards \ everything you did not restate."; -/// Expose every registered built-in tool under Kimi Code's names. -/// -/// Only the exposed name changes: executors, schemas, and the identity that -/// permissions and telemetry resolve through are untouched, because -/// `canonical_tool_name` maps every vocabulary back to the same -/// [`NativeTool`]. -fn apply_vocabulary(registry: &mut ToolRegistry, vocabulary: ToolVocabulary) { - for tool in NativeTool::VARIANTS { - let exposed = tool.name(vocabulary); - if exposed == tool.canonical_name() { - continue; - } - let Some(registered) = registry.unregister(tool.canonical_name()) else { - continue; - }; - registry.register(RegisteredTool { - definition: ToolDefinition { - name: exposed.to_string(), - ..registered.definition - }, - ..registered - }); - } -} - -/// Replace a registered tool's description, keeping its executor and schema. -/// -/// Profiles own their registries, so tailoring wording per model is a local -/// change and does not affect what other profiles expose. -fn redescribe(registry: &mut ToolRegistry, tool: NativeTool, description: &str) { - let Some(tool) = registry.unregister(tool.canonical_name()) else { - return; - }; - registry.register(RegisteredTool { - definition: ToolDefinition { - description: description.to_string(), - ..tool.definition - }, - ..tool - }); -} - pub struct KimiProfile { base: BaseProfile, } @@ -97,12 +53,14 @@ impl KimiProfile { options: &NativeToolOptions, summarizer: Option, ) -> Self { - let mut registry = ToolRegistry::new(); + // The registry carries the vocabulary, so tools registered later + // (subagent tools, skills) are renamed too. + let mut registry = ToolRegistry::with_vocabulary(ToolVocabulary::KimiCode); register_core_tools(&mut registry, options, summarizer); registry.register(make_edit_file_tool()); - redescribe(&mut registry, NativeTool::EditFile, EDIT_FILE_DESCRIPTION); - redescribe(&mut registry, NativeTool::WriteFile, WRITE_FILE_DESCRIPTION); + registry.redescribe(NativeTool::EditFile, EDIT_FILE_DESCRIPTION); + registry.redescribe(NativeTool::WriteFile, WRITE_FILE_DESCRIPTION); // Kimi Code drives todos with one replace-whole-list call. The // Anthropic task tools model the opposite interaction -- incremental @@ -111,9 +69,6 @@ impl KimiProfile { let todo_runtime = Arc::new(TodoRuntime::new()); registry.register(make_todo_list_tool(todo_runtime)); - // Applied last so every built-in registered above is renamed. - apply_vocabulary(&mut registry, ToolVocabulary::KimiCode); - Self { base: BaseProfile { profile_kind: AgentProfileKind::Kimi, @@ -175,7 +130,8 @@ impl AgentProfile for KimiProfile { user_instructions: Option<&str>, skills: &[Skill], ) -> String { - let template = EmbeddedPrompt::new("kimi.md.j2", CORE_PROMPT); + let template = EmbeddedPrompt::new("kimi.md.j2", CORE_PROMPT) + .with_vocabulary(self.base.registry.vocabulary()); profiles::assemble_system_prompt( template, @@ -194,6 +150,8 @@ mod tests { use fabro_types::AgentToolCategory; use super::*; + use crate::skills::make_use_skill_tool; + use crate::subagent::{SessionFactory, SubAgentSupervisor}; use crate::test_support::MockSandbox; use crate::tool_permissions::{known_tool_category, tool_category}; @@ -278,10 +236,32 @@ mod tests { "{canonical} should have been renamed" ); } - // No Kimi Code counterpart, so these keep fabro's names. assert!(names.contains(&"TodoList".to_string())); } + /// Tools registered after the profile is constructed must also land in the + /// Kimi vocabulary, or the model sees a mixed-case tool set. + #[test] + fn post_construction_tools_also_use_kimi_names() { + let mut profile = KimiProfile::new("kimi-k3"); + let factory: SessionFactory = Arc::new(|| panic!("unused")); + profile.register_subagent_tools(SubAgentSupervisor::new(3), factory, 0); + profile + .tool_registry_mut() + .register(make_use_skill_tool(Arc::new(vec![Skill { + name: "demo".into(), + description: "d".into(), + template: "t".into(), + }]))); + + let names = profile.tool_registry().names(); + assert!(names.contains(&"Skill".to_string()), "got {names:?}"); + assert!(!names.contains(&"use_skill".to_string()), "got {names:?}"); + // Deliberately not renamed to Kimi Code's `Agent`: fabro's subagent + // tools are a supervisor model, not a call-and-return one. + assert!(names.contains(&"spawn_agent".to_string()), "got {names:?}"); + } + #[test] fn edit_and_write_descriptions_drill_read_before_write() { let profile = KimiProfile::new("kimi-k3"); diff --git a/lib/components/fabro-agent/src/profiles/mod.rs b/lib/components/fabro-agent/src/profiles/mod.rs index adc9d1a96..617e008c7 100644 --- a/lib/components/fabro-agent/src/profiles/mod.rs +++ b/lib/components/fabro-agent/src/profiles/mod.rs @@ -15,6 +15,7 @@ pub use openai::OpenAiProfile; use crate::agent_profile::AgentProfile; use crate::config::{NativeToolOptions, ToolSecrets}; +use crate::native_tool::{NativeTool, ToolVocabulary}; use crate::sandbox::Sandbox; use crate::skills::{Skill, format_skills_prompt_section}; use crate::tool_registry::ToolRegistry; @@ -125,9 +126,11 @@ pub struct EnvContext { /// The environment block is supplied by [`assemble_system_prompt`] and cannot /// be overridden by callers. pub struct EmbeddedPrompt { - name: &'static str, - source: &'static str, - inputs: HashMap, + name: &'static str, + source: &'static str, + inputs: HashMap, + /// Vocabulary the surrounding prompt sections should name tools in. + vocabulary: ToolVocabulary, } impl EmbeddedPrompt { @@ -137,9 +140,17 @@ impl EmbeddedPrompt { name, source, inputs: HashMap::new(), + vocabulary: ToolVocabulary::Fabro, } } + /// Name tools in `vocabulary` in the generated sections. + #[must_use] + pub fn with_vocabulary(mut self, vocabulary: ToolVocabulary) -> Self { + self.vocabulary = vocabulary; + self + } + #[must_use] pub fn with_string(mut self, name: &'static str, value: impl Into) -> Self { self.inputs @@ -184,6 +195,7 @@ pub fn assemble_system_prompt( skills: &[Skill], ) -> String { let env_block = build_env_context_block_with(env, env_context); + let skill_tool = NativeTool::UseSkill.name(template.vocabulary); let prompt = template.render(env_block); let docs_section = if memory.is_empty() { @@ -192,7 +204,7 @@ pub fn assemble_system_prompt( format!("\n\n{}", memory.join("\n\n")) }; let skills_section = { - let s = format_skills_prompt_section(skills); + let s = format_skills_prompt_section(skills, skill_tool); if s.is_empty() { String::new() } else { diff --git a/lib/components/fabro-agent/src/skills.rs b/lib/components/fabro-agent/src/skills.rs index fe380bb28..026464d75 100644 --- a/lib/components/fabro-agent/src/skills.rs +++ b/lib/components/fabro-agent/src/skills.rs @@ -196,16 +196,22 @@ pub fn make_use_skill_tool(skills: Arc>) -> RegisteredTool { } } -pub fn format_skills_prompt_section(skills: &[Skill]) -> String { +/// Render the skills section of a system prompt. +/// +/// `skill_tool` is the name the skill tool is exposed under, which depends on +/// the profile's vocabulary — telling a model to call a tool it was not given +/// is worse than omitting the guidance. +pub fn format_skills_prompt_section(skills: &[Skill], skill_tool: &str) -> String { if skills.is_empty() { return String::new(); } let mut lines = vec![ "# Available Skills".to_string(), - "When the user's request matches a skill below, call the `use_skill` tool \ - to load its instructions, then follow them." - .to_string(), + format!( + "When the user's request matches a skill below, call the `{skill_tool}` tool \ + to load its instructions, then follow them." + ), ]; for skill in skills { if skill.description.is_empty() { @@ -452,13 +458,13 @@ name: trimmed #[test] fn format_empty() { - assert_eq!(format_skills_prompt_section(&[]), ""); + assert_eq!(format_skills_prompt_section(&[], "use_skill"), ""); } #[test] fn format_lists_skills() { let skills = test_skills(); - let section = format_skills_prompt_section(&skills); + let section = format_skills_prompt_section(&skills, "use_skill"); assert!(section.contains("# Available Skills")); assert!(section.contains("call the `use_skill` tool")); assert!(section.contains("- `commit`: Create a commit")); diff --git a/lib/components/fabro-agent/src/test_support.rs b/lib/components/fabro-agent/src/test_support.rs index fd827ec0c..e9c2d8112 100644 --- a/lib/components/fabro-agent/src/test_support.rs +++ b/lib/components/fabro-agent/src/test_support.rs @@ -15,6 +15,7 @@ use futures::stream; use crate::agent_profile::AgentProfile; use crate::config::SessionOptions; +use crate::native_tool::NativeTool; use crate::profiles::EnvContext; use crate::sandbox::*; use crate::session::Session; @@ -80,7 +81,8 @@ impl AgentProfile for TestProfile { user_instructions: Option<&str>, skills: &[Skill], ) -> String { - let skills_section = format_skills_prompt_section(skills); + let skills_section = + format_skills_prompt_section(skills, NativeTool::UseSkill.canonical_name()); let skills_part = if skills_section.is_empty() { String::new() } else { diff --git a/lib/components/fabro-agent/src/tool_registry.rs b/lib/components/fabro-agent/src/tool_registry.rs index 74af24354..55f06222c 100644 --- a/lib/components/fabro-agent/src/tool_registry.rs +++ b/lib/components/fabro-agent/src/tool_registry.rs @@ -8,6 +8,7 @@ use fabro_types::{AgentToolCategory, AgentToolSource, AgentToolSummary}; use tokio_util::sync::CancellationToken; use crate::config::{ToolAccessPolicy, ToolExposureMode}; +use crate::native_tool::{NativeTool, ToolVocabulary}; use crate::sandbox::Sandbox; use crate::session::ToolEnvProvider; use crate::tool_permissions; @@ -125,21 +126,54 @@ fn agent_tool_source(source: &ToolSource) -> AgentToolSource { } pub struct ToolRegistry { - tools: HashMap, + tools: HashMap, + /// Naming scheme applied to built-in tools as they are registered. + /// + /// Held by the registry rather than applied as a pass after construction, + /// so tools registered later — subagent tools, skills — cannot miss it and + /// leave the model with a mixed-vocabulary tool set. + vocabulary: ToolVocabulary, } impl ToolRegistry { #[must_use] pub fn new() -> Self { + Self::with_vocabulary(ToolVocabulary::Fabro) + } + + /// A registry that exposes built-in tools under `vocabulary`. + #[must_use] + pub fn with_vocabulary(vocabulary: ToolVocabulary) -> Self { Self { tools: HashMap::new(), + vocabulary, } } - pub fn register(&mut self, tool: RegisteredTool) { + #[must_use] + pub fn vocabulary(&self) -> ToolVocabulary { + self.vocabulary + } + + pub fn register(&mut self, mut tool: RegisteredTool) { + if let Some(native) = NativeTool::from_any_name(&tool.definition.name) { + tool.definition.name = native.name(self.vocabulary).to_string(); + } self.tools.insert(tool.definition.name.clone(), tool); } + /// Replace a built-in tool's description, keeping its executor and schema. + /// + /// Resolves through the registry's vocabulary, so callers name the tool by + /// identity rather than by whatever string it is currently exposed under. + pub fn redescribe(&mut self, tool: NativeTool, description: impl Into) { + let exposed = tool.name(self.vocabulary); + if let Some(mut registered) = self.tools.remove(exposed) { + registered.definition.description = description.into(); + self.tools.insert(exposed.to_string(), registered); + } + } + pub fn unregister(&mut self, name: &str) -> Option { self.tools.remove(name) }