diff --git a/Makefile b/Makefile index 3fa41bc24..48a9a8a85 100644 --- a/Makefile +++ b/Makefile @@ -242,14 +242,14 @@ build-whatsapp-native: generate @echo "Building for multiple platforms..." @mkdir -p $(BUILD_DIR) GOOS=linux GOARCH=amd64 $(GO) build -tags $(GO_BUILD_TAGS),whatsapp_native -ldflags "$(LDFLAGS)" -o $(BUILD_DIR)/$(BINARY_NAME)-linux-amd64 ./$(CMD_DIR) - GOOS=linux GOARCH=arm GOARM=7 $(GO) build -tags $(GO_BUILD_TAGS),whatsapp_native -ldflags "$(LDFLAGS)" -o $(BUILD_DIR)/$(BINARY_NAME)-linux-arm ./$(CMD_DIR) - GOOS=linux GOARCH=arm64 $(GO) build -tags $(GO_BUILD_TAGS),whatsapp_native -ldflags "$(LDFLAGS)" -o $(BUILD_DIR)/$(BINARY_NAME)-linux-arm64 ./$(CMD_DIR) - GOOS=linux GOARCH=loong64 $(GO) build -tags $(GO_BUILD_TAGS),whatsapp_native -ldflags "$(LDFLAGS)" -o $(BUILD_DIR)/$(BINARY_NAME)-linux-loong64 ./$(CMD_DIR) - GOOS=linux GOARCH=riscv64 $(GO) build -tags $(GO_BUILD_TAGS),whatsapp_native -ldflags "$(LDFLAGS)" -o $(BUILD_DIR)/$(BINARY_NAME)-linux-riscv64 ./$(CMD_DIR) - GOOS=linux GOARCH=mipsle GOMIPS=softfloat $(GO) build -tags $(GO_BUILD_TAGS_NO_GOOLM),whatsapp_native -ldflags "$(LDFLAGS)" -o $(BUILD_DIR)/$(BINARY_NAME)-linux-mipsle ./$(CMD_DIR) - $(call PATCH_MIPS_FLAGS,$(BUILD_DIR)/$(BINARY_NAME)-linux-mipsle) - GOOS=darwin GOARCH=arm64 $(GO) build -tags $(GO_BUILD_TAGS),whatsapp_native -ldflags "$(LDFLAGS)" -o $(BUILD_DIR)/$(BINARY_NAME)-darwin-arm64 ./$(CMD_DIR) - GOOS=windows GOARCH=amd64 $(GO) build -tags $(GO_BUILD_TAGS),whatsapp_native -ldflags "$(LDFLAGS)" -o $(BUILD_DIR)/$(BINARY_NAME)-windows-amd64.exe ./$(CMD_DIR) + #GOOS=linux GOARCH=arm GOARM=7 $(GO) build -tags $(GO_BUILD_TAGS),whatsapp_native -ldflags "$(LDFLAGS)" -o $(BUILD_DIR)/$(BINARY_NAME)-linux-arm ./$(CMD_DIR) + #GOOS=linux GOARCH=arm64 $(GO) build -tags $(GO_BUILD_TAGS),whatsapp_native -ldflags "$(LDFLAGS)" -o $(BUILD_DIR)/$(BINARY_NAME)-linux-arm64 ./$(CMD_DIR) + #GOOS=linux GOARCH=loong64 $(GO) build -tags $(GO_BUILD_TAGS),whatsapp_native -ldflags "$(LDFLAGS)" -o $(BUILD_DIR)/$(BINARY_NAME)-linux-loong64 ./$(CMD_DIR) + #GOOS=linux GOARCH=riscv64 $(GO) build -tags $(GO_BUILD_TAGS),whatsapp_native -ldflags "$(LDFLAGS)" -o $(BUILD_DIR)/$(BINARY_NAME)-linux-riscv64 ./$(CMD_DIR) + #GOOS=linux GOARCH=mipsle GOMIPS=softfloat $(GO) build -tags $(GO_BUILD_TAGS_NO_GOOLM),whatsapp_native -ldflags "$(LDFLAGS)" -o $(BUILD_DIR)/$(BINARY_NAME)-linux-mipsle ./$(CMD_DIR) + #$(call PATCH_MIPS_FLAGS,$(BUILD_DIR)/$(BINARY_NAME)-linux-mipsle) + #GOOS=darwin GOARCH=arm64 $(GO) build -tags $(GO_BUILD_TAGS),whatsapp_native -ldflags "$(LDFLAGS)" -o $(BUILD_DIR)/$(BINARY_NAME)-darwin-arm64 ./$(CMD_DIR) + #GOOS=windows GOARCH=amd64 $(GO) build -tags $(GO_BUILD_TAGS),whatsapp_native -ldflags "$(LDFLAGS)" -o $(BUILD_DIR)/$(BINARY_NAME)-windows-amd64.exe ./$(CMD_DIR) ## @$(GO) build $(GOFLAGS) -tags whatsapp_native -ldflags "$(LDFLAGS)" -o $(BINARY_PATH) ./$(CMD_DIR) @echo "Build complete" ## @ln -sf $(BINARY_NAME)-$(PLATFORM)-$(ARCH) $(BUILD_DIR)/$(BINARY_NAME) diff --git a/docs/guides/configuration.md b/docs/guides/configuration.md index 64a61f241..65ee331b2 100644 --- a/docs/guides/configuration.md +++ b/docs/guides/configuration.md @@ -126,6 +126,41 @@ For advanced/test setups, you can override the builtin skills root with: export PICOCLAW_BUILTIN_SKILLS=/path/to/skills ``` +### Skill Catalog Token Optimization + +By default, the skill catalog (the list of available skills shown to the LLM) is included in every LLM request. On providers without prompt caching (most OpenAI-compatible APIs), this costs tokens on every call including intermediate tool round-trips within a single turn. + +Two opt-in flags under `agents.defaults.skill_catalog` reduce this cost: + +```json +"agents": { + "defaults": { + "skill_catalog": { + "skip_on_tools": false, + "skip_on_subsequent": false + } + } +} +``` + +| Field | Default | Effect when `true` | +|---|---|---| +| `skip_on_tools` | `false` | Omits the catalog from tool-call continuation requests (mid-turn LLM round-trips). The LLM already received the catalog on the initial turn request. | +| `skip_on_subsequent` | `false` | Omits the catalog on turns after the first in a session. The catalog is automatically re-injected after context compaction, so the LLM always rediscovers skills when its history is summarized. | + +Both flags default to `false` so existing deployments are unaffected. Enable both for maximum token savings: + +```json +"skill_catalog": { + "skip_on_tools": true, + "skip_on_subsequent": true +} +``` + +Environment variable equivalents: +- `PICOCLAW_AGENTS_DEFAULTS_SKILL_CATALOG_SKIP_ON_TOOLS=true` +- `PICOCLAW_AGENTS_DEFAULTS_SKILL_CATALOG_SKIP_ON_SUBSEQUENT=true` + ### Using Skills From Chat Channels Once skills are installed, and MCP servers are configured, you can inspect and force them directly from a chat channel: diff --git a/pkg/agent/context.go b/pkg/agent/context.go index 87bdd6b41..2fc64c046 100644 --- a/pkg/agent/context.go +++ b/pkg/agent/context.go @@ -22,12 +22,13 @@ import ( ) type ContextBuilder struct { - workspace string - skillsLoader *skills.SkillsLoader - memory *MemoryStore - splitOnMarker bool - agentDiscovery func(agentID string) []AgentDescriptor - promptRegistry *PromptRegistry + workspace string + skillsLoader *skills.SkillsLoader + memory *MemoryStore + splitOnMarker bool + skillCatalogCfg config.SkillCatalogConfig + agentDiscovery func(agentID string) []AgentDescriptor + promptRegistry *PromptRegistry // Cache for system prompt to avoid rebuilding on every call. // This fixes issue #607: repeated reprocessing of the entire context. @@ -67,6 +68,11 @@ func (cb *ContextBuilder) WithSplitOnMarker(enabled bool) *ContextBuilder { return cb } +func (cb *ContextBuilder) WithSkillCatalogConfig(cfg config.SkillCatalogConfig) *ContextBuilder { + cb.skillCatalogCfg = cfg + return cb +} + func (cb *ContextBuilder) WithAgentDiscovery( agentID string, discover func(agentID string) []AgentDescriptor, @@ -221,25 +227,6 @@ func (cb *ContextBuilder) BuildSystemPromptParts() []PromptPart { }) } - // Skills - show summary, AI can read full content with read_file tool - skillsSummary := cb.skillsLoader.BuildSkillsSummary() - if skillsSummary != "" { - add(PromptPart{ - ID: "capability.skill_catalog", - Layer: PromptLayerCapability, - Slot: PromptSlotSkillCatalog, - Source: PromptSource{ID: PromptSourceSkillCatalog, Name: "skill:index"}, - Title: "skill catalog", - Content: fmt.Sprintf(`# Skills - -The following skills extend your capabilities. To use a skill, read its SKILL.md file using the read_file tool. - -%s`, skillsSummary), - Stable: true, - Cache: PromptCacheEphemeral, - }) - } - // Memory context memoryContext := cb.memory.GetMemoryContext() if memoryContext != "" { @@ -333,6 +320,13 @@ func (cb *ContextBuilder) EstimateSystemTokens(summary string, activeSkills []st totalChars := utf8.RuneCountInString(staticPrompt) + dynamicContextChars + // Skill catalog is no longer in the static prompt; add it to the estimate + // (EstimateSystemTokens assumes a non-continuation turn). + if skillsSummary := cb.skillsLoader.BuildSkillsSummary(); skillsSummary != "" { + totalChars += utf8.RuneCountInString(skillsSummary) + 80 // header overhead + totalChars += 7 // separator + } + if skillsText := cb.buildActiveSkillsContext(activeSkills); skillsText != "" { totalChars += utf8.RuneCountInString(skillsText) totalChars += 7 // separator \n\n---\n\n @@ -722,6 +716,37 @@ func (cb *ContextBuilder) BuildMessagesFromPrompt(req PromptBuildRequest) []prov }, &providers.CacheControl{Type: "ephemeral"}), } + // Determine whether to inject the skill catalog. + // Both skip behaviors are opt-in via config (default: always include). + isToolContinuation := len(req.History) > 0 && req.History[len(req.History)-1].Role == "tool" + isFirstTurn := len(req.History) == 0 + isAfterCompaction := req.Summary != "" + skipForTools := cb.skillCatalogCfg.SkipOnTools && isToolContinuation + skipForSubsequent := cb.skillCatalogCfg.SkipOnSubsequent && + !isFirstTurn && !isAfterCompaction && !isToolContinuation + if !skipForTools && !skipForSubsequent { + if skillsSummary := cb.skillsLoader.BuildSkillsSummary(); skillsSummary != "" { + catalogPart := PromptPart{ + ID: "capability.skill_catalog", + Layer: PromptLayerCapability, + Slot: PromptSlotSkillCatalog, + Source: PromptSource{ID: PromptSourceSkillCatalog, Name: "skill:index"}, + Title: "skill catalog", + Content: fmt.Sprintf( + "# Skills\n\nThe following skills extend your capabilities. To use a skill, read its SKILL.md file using the read_file tool.\n\n%s", + skillsSummary, + ), + Stable: true, + Cache: PromptCacheEphemeral, + } + stringParts = append(stringParts, catalogPart.Content) + contentBlocks = append( + contentBlocks, + promptContentBlock(catalogPart, &providers.CacheControl{Type: "ephemeral"}), + ) + } + } + promptParts := append([]PromptPart(nil), req.Overlays...) promptParts = append(promptParts, cb.buildActiveSkillsPromptParts(req.ActiveSkills)...) if contributedParts, err := cb.promptRegistryOrDefault().Collect(context.Background(), req); err != nil { diff --git a/pkg/agent/context_cache_test.go b/pkg/agent/context_cache_test.go index ef5e6c5de..c54422d60 100644 --- a/pkg/agent/context_cache_test.go +++ b/pkg/agent/context_cache_test.go @@ -8,6 +8,7 @@ import ( "testing" "time" + "github.com/sipeed/picoclaw/pkg/config" "github.com/sipeed/picoclaw/pkg/providers" ) @@ -31,6 +32,16 @@ func setupWorkspace(t *testing.T, files map[string]string) string { return tmpDir } +// systemPromptFromMessages extracts the Content of the first system message. +func systemPromptFromMessages(msgs []providers.Message) string { + for _, m := range msgs { + if m.Role == "system" { + return m.Content + } + } + return "" +} + // TestSingleSystemMessage verifies that BuildMessages always produces exactly one // system message regardless of summary/history variations. // Fix: multiple system messages break Anthropic (top-level system param) and @@ -468,8 +479,9 @@ description: global-v1 } cb := NewContextBuilder(tmpDir) - sp1 := cb.BuildSystemPromptWithCache() - if !strings.Contains(sp1, "global-v1") { + // Skill catalog is injected per-request, not in the static cache; check via BuildMessagesFromPrompt. + sysMsg1 := systemPromptFromMessages(cb.BuildMessagesFromPrompt(PromptBuildRequest{})) + if !strings.Contains(sysMsg1, "global-v1") { t.Fatal("expected initial prompt to contain global skill description") } @@ -493,11 +505,11 @@ description: global-v2 t.Fatal("sourceFilesChangedLocked() should detect global skill file content change") } - sp2 := cb.BuildSystemPromptWithCache() - if !strings.Contains(sp2, "global-v2") { + sysMsg2 := systemPromptFromMessages(cb.BuildMessagesFromPrompt(PromptBuildRequest{})) + if !strings.Contains(sysMsg2, "global-v2") { t.Error("rebuilt prompt should contain updated global skill description") } - if sp1 == sp2 { + if sysMsg1 == sysMsg2 { t.Error("cache should be invalidated when global skill file content changes") } } @@ -528,8 +540,9 @@ description: builtin-v1 } cb := NewContextBuilder(tmpDir) - sp1 := cb.BuildSystemPromptWithCache() - if !strings.Contains(sp1, "builtin-v1") { + // Skill catalog is injected per-request, not in the static cache; check via BuildMessagesFromPrompt. + sysMsg1 := systemPromptFromMessages(cb.BuildMessagesFromPrompt(PromptBuildRequest{})) + if !strings.Contains(sysMsg1, "builtin-v1") { t.Fatal("expected initial prompt to contain builtin skill description") } @@ -553,11 +566,11 @@ description: builtin-v2 t.Fatal("sourceFilesChangedLocked() should detect builtin skill file content change") } - sp2 := cb.BuildSystemPromptWithCache() - if !strings.Contains(sp2, "builtin-v2") { + sysMsg2 := systemPromptFromMessages(cb.BuildMessagesFromPrompt(PromptBuildRequest{})) + if !strings.Contains(sysMsg2, "builtin-v2") { t.Error("rebuilt prompt should contain updated builtin skill description") } - if sp1 == sp2 { + if sysMsg1 == sysMsg2 { t.Error("cache should be invalidated when builtin skill file content changes") } } @@ -575,8 +588,9 @@ description: delete-me-v1 defer os.RemoveAll(tmpDir) cb := NewContextBuilder(tmpDir) - sp1 := cb.BuildSystemPromptWithCache() - if !strings.Contains(sp1, "delete-me-v1") { + // Skill catalog is injected per-request, not in the static cache; check via BuildMessagesFromPrompt. + sysMsg1 := systemPromptFromMessages(cb.BuildMessagesFromPrompt(PromptBuildRequest{})) + if !strings.Contains(sysMsg1, "delete-me-v1") { t.Fatal("expected initial prompt to contain skill description") } @@ -592,15 +606,117 @@ description: delete-me-v1 t.Fatal("sourceFilesChangedLocked() should detect deleted skill file") } - sp2 := cb.BuildSystemPromptWithCache() - if strings.Contains(sp2, "delete-me-v1") { + sysMsg2 := systemPromptFromMessages(cb.BuildMessagesFromPrompt(PromptBuildRequest{})) + if strings.Contains(sysMsg2, "delete-me-v1") { t.Error("rebuilt prompt should not contain deleted skill description") } - if sp1 == sp2 { + if sysMsg1 == sysMsg2 { t.Error("cache should be invalidated when skill file is deleted") } } +// TestSkillCatalogInjectionPolicy verifies catalog inclusion under various +// config combinations. +func TestSkillCatalogInjectionPolicy(t *testing.T) { + tmpDir := setupWorkspace(t, map[string]string{ + "skills/demo/SKILL.md": "---\nname: demo\ndescription: \"demo skill\"\n---\n# Demo", + }) + defer os.RemoveAll(tmpDir) + + userMsg := providers.Message{Role: "user", Content: "hello"} + assistantMsg := providers.Message{Role: "assistant", Content: "hi"} + toolMsg := providers.Message{Role: "tool", Content: "result", ToolCallID: "tc1"} + + contains := func(msgs []providers.Message) bool { + return strings.Contains(systemPromptFromMessages(msgs), "demo skill") + } + + newCB := func(skipOnTools, skipOnSubsequent bool) *ContextBuilder { + return NewContextBuilder(tmpDir).WithSkillCatalogConfig(config.SkillCatalogConfig{ + SkipOnTools: skipOnTools, + SkipOnSubsequent: skipOnSubsequent, + }) + } + + t.Run("default (both false): catalog always included", func(t *testing.T) { + cb := newCB(false, false) + for _, req := range []PromptBuildRequest{ + {}, + {History: []providers.Message{userMsg, assistantMsg}}, + {History: []providers.Message{userMsg, assistantMsg, toolMsg}}, + {History: []providers.Message{userMsg, assistantMsg}, Summary: "summary"}, + } { + if !contains(cb.BuildMessagesFromPrompt(req)) { + t.Error("catalog should always be included when both flags are false") + } + } + }) + + t.Run("skip_on_tools: skips tool continuations only", func(t *testing.T) { + cb := newCB(true, false) + if !contains(cb.BuildMessagesFromPrompt(PromptBuildRequest{})) { + t.Error("turn 1: catalog should be included") + } + if !contains(cb.BuildMessagesFromPrompt(PromptBuildRequest{ + History: []providers.Message{userMsg, assistantMsg}, + })) { + t.Error("turn > 1 (no tool): catalog should be included") + } + if contains(cb.BuildMessagesFromPrompt(PromptBuildRequest{ + History: []providers.Message{userMsg, assistantMsg, toolMsg}, + })) { + t.Error("tool continuation: catalog should be skipped") + } + }) + + t.Run("skip_on_subsequent: skips turns > 1, re-injects after compaction", func(t *testing.T) { + cb := newCB(false, true) + if !contains(cb.BuildMessagesFromPrompt(PromptBuildRequest{})) { + t.Error("turn 1: catalog should be included") + } + if contains(cb.BuildMessagesFromPrompt(PromptBuildRequest{ + History: []providers.Message{userMsg, assistantMsg}, + })) { + t.Error("turn > 1, no summary: catalog should be skipped") + } + if !contains(cb.BuildMessagesFromPrompt(PromptBuildRequest{ + History: []providers.Message{userMsg, assistantMsg}, + Summary: "prior conversation summary", + })) { + t.Error("after compaction: catalog should be re-injected") + } + // tool continuation is NOT skipped when only skip_on_subsequent is set + if !contains(cb.BuildMessagesFromPrompt(PromptBuildRequest{ + History: []providers.Message{userMsg, assistantMsg, toolMsg}, + })) { + t.Error("tool continuation (skip_on_subsequent only): catalog should be included") + } + }) + + t.Run("both true: skips tool turns and subsequent turns, re-injects after compaction", func(t *testing.T) { + cb := newCB(true, true) + if !contains(cb.BuildMessagesFromPrompt(PromptBuildRequest{})) { + t.Error("turn 1: catalog should be included") + } + if contains(cb.BuildMessagesFromPrompt(PromptBuildRequest{ + History: []providers.Message{userMsg, assistantMsg, toolMsg}, + })) { + t.Error("tool continuation: catalog should be skipped") + } + if contains(cb.BuildMessagesFromPrompt(PromptBuildRequest{ + History: []providers.Message{userMsg, assistantMsg}, + })) { + t.Error("turn > 1, no summary: catalog should be skipped") + } + if !contains(cb.BuildMessagesFromPrompt(PromptBuildRequest{ + History: []providers.Message{userMsg, assistantMsg}, + Summary: "prior conversation summary", + })) { + t.Error("after compaction: catalog should be re-injected") + } + }) +} + // TestConcurrentBuildSystemPromptWithCache verifies that multiple goroutines // can safely call BuildSystemPromptWithCache concurrently without producing // empty results, panics, or data races. diff --git a/pkg/agent/instance.go b/pkg/agent/instance.go index 4ed713035..f4e2870ef 100644 --- a/pkg/agent/instance.go +++ b/pkg/agent/instance.go @@ -134,7 +134,8 @@ func NewAgentInstance( mcpDiscoveryActive && cfg.Tools.MCP.Discovery.UseBM25, mcpDiscoveryActive && cfg.Tools.MCP.Discovery.UseRegex, ). - WithSplitOnMarker(cfg.Agents.Defaults.SplitOnMarker) + WithSplitOnMarker(cfg.Agents.Defaults.SplitOnMarker). + WithSkillCatalogConfig(cfg.Agents.Defaults.SkillCatalog) agentID := routing.DefaultAgentID agentName := "" diff --git a/pkg/config/config.go b/pkg/config/config.go index 53eaef059..741fd55f8 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -375,6 +375,17 @@ type ToolFeedbackConfig struct { SeparateMessages bool `json:"separate_messages" env:"PICOCLAW_AGENTS_DEFAULTS_TOOL_FEEDBACK_SEPARATE_MESSAGES"` } +type SkillCatalogConfig struct { + // SkipOnTools omits the skill catalog from tool-call continuation requests + // (mid-turn LLM round-trips). The LLM already received the catalog on the + // initial turn request. Default false (catalog always included). + SkipOnTools bool `json:"skip_on_tools" env:"PICOCLAW_AGENTS_DEFAULTS_SKILL_CATALOG_SKIP_ON_TOOLS"` + // SkipOnSubsequent omits the skill catalog on turns after the first in a + // session. The catalog is still re-injected after context compaction. + // Default false (catalog always included). + SkipOnSubsequent bool `json:"skip_on_subsequent" env:"PICOCLAW_AGENTS_DEFAULTS_SKILL_CATALOG_SKIP_ON_SUBSEQUENT"` +} + type AgentDefaults struct { Workspace string `json:"workspace" env:"PICOCLAW_AGENTS_DEFAULTS_WORKSPACE"` RestrictToWorkspace bool `json:"restrict_to_workspace" env:"PICOCLAW_AGENTS_DEFAULTS_RESTRICT_TO_WORKSPACE"` @@ -397,6 +408,7 @@ type AgentDefaults struct { SubTurn SubTurnConfig `json:"subturn" envPrefix:"PICOCLAW_AGENTS_DEFAULTS_SUBTURN_"` ToolFeedback ToolFeedbackConfig `json:"tool_feedback,omitempty"` SplitOnMarker bool `json:"split_on_marker" env:"PICOCLAW_AGENTS_DEFAULTS_SPLIT_ON_MARKER"` // split messages on <|[SPLIT]|> marker + SkillCatalog SkillCatalogConfig `json:"skill_catalog,omitempty"` ContextManager string `json:"context_manager,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_CONTEXT_MANAGER"` ContextManagerConfig json.RawMessage `json:"context_manager_config,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_CONTEXT_MANAGER_CONFIG"` MaxLLMRetries int `json:"max_llm_retries,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_MAX_LLM_RETRIES"`