refactor: update skill catalog injection logic and add corresponding tests

This commit is contained in:
Costin Stroie 2026-05-06 12:34:42 +03:00
parent 52af9748a2
commit 48f636e343
2 changed files with 54 additions and 5 deletions

View file

@ -690,12 +690,16 @@ func (cb *ContextBuilder) BuildMessagesFromPrompt(req PromptBuildRequest) []prov
}, &providers.CacheControl{Type: "ephemeral"}), }, &providers.CacheControl{Type: "ephemeral"}),
} }
// Skip the skill catalog on tool-call continuations: the LLM already saw // Inject the skill catalog only when the LLM needs to (re)discover available skills:
// it in the initial turn request and doesn't need it re-sent for every // - Turn 1: no history yet, LLM hasn't seen the catalog.
// intermediate tool round-trip. This saves significant tokens on providers // - After compaction: history was summarized; early turns (including the original
// without prompt caching (OpenAI-compat). // catalog injection) are gone, so the LLM must see it again.
// Skip it on tool-call continuations (mid-turn round-trips) and on ordinary
// subsequent turns where the catalog is already in the LLM's context window.
isToolContinuation := len(req.History) > 0 && req.History[len(req.History)-1].Role == "tool" isToolContinuation := len(req.History) > 0 && req.History[len(req.History)-1].Role == "tool"
if !isToolContinuation { isFirstTurn := len(req.History) == 0
isAfterCompaction := req.Summary != ""
if !isToolContinuation && (isFirstTurn || isAfterCompaction) {
if skillsSummary := cb.skillsLoader.BuildSkillsSummary(); skillsSummary != "" { if skillsSummary := cb.skillsLoader.BuildSkillsSummary(); skillsSummary != "" {
catalogPart := PromptPart{ catalogPart := PromptPart{
ID: "capability.skill_catalog", ID: "capability.skill_catalog",

View file

@ -614,6 +614,51 @@ description: delete-me-v1
} }
} }
// TestSkillCatalogInjectionPolicy verifies that the catalog is included only
// when the LLM needs to (re)discover skills: turn 1 and after compaction.
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)
cb := NewContextBuilder(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")
}
// Turn 1: no history — catalog must appear.
if !contains(cb.BuildMessagesFromPrompt(PromptBuildRequest{})) {
t.Error("turn 1 (no history): catalog should be included")
}
// Tool continuation: last message is a tool result — catalog must be skipped.
if contains(cb.BuildMessagesFromPrompt(PromptBuildRequest{
History: []providers.Message{userMsg, assistantMsg, toolMsg},
})) {
t.Error("tool continuation: catalog should be skipped")
}
// Turn > 1, no compaction: catalog must be skipped.
if contains(cb.BuildMessagesFromPrompt(PromptBuildRequest{
History: []providers.Message{userMsg, assistantMsg},
})) {
t.Error("turn > 1, no summary: catalog should be skipped")
}
// After compaction (summary present): catalog must be re-injected.
if !contains(cb.BuildMessagesFromPrompt(PromptBuildRequest{
History: []providers.Message{userMsg, assistantMsg},
Summary: "prior conversation summary",
})) {
t.Error("after compaction (summary present): catalog should be re-injected")
}
}
// TestConcurrentBuildSystemPromptWithCache verifies that multiple goroutines // TestConcurrentBuildSystemPromptWithCache verifies that multiple goroutines
// can safely call BuildSystemPromptWithCache concurrently without producing // can safely call BuildSystemPromptWithCache concurrently without producing
// empty results, panics, or data races. // empty results, panics, or data races.