Spike: instruction resolver with most-specificity rule

pkg/github/instructions.go

@@ -3,9 +3,164 @@ package github
 import (
 	"os"
 	"slices"
+	"sort"
 	"strings"
 )
 
+// =============================================================================
+// SPIKE: Tool-based instruction specificity rules
+// =============================================================================
+
+// InstructionRule associates an instruction with a set of tools.
+// Rules with more specific tool sets (supersets) take precedence over
+// rules with fewer tools when the same tools are involved.
+type InstructionRule struct {
+	// ID is a unique identifier for the rule (for debugging/logging)
+	ID string
+	// ToolNames is the set of tool names this rule applies to.
+	// The rule is active when ALL of these tools are present in the active set.
+	ToolNames map[string]bool
+	// Instruction is the instruction text for this rule.
+	Instruction string
+}
+
+// NewInstructionRule creates an InstructionRule from a list of tool names.
+func NewInstructionRule(id string, instruction string, toolNames ...string) InstructionRule {
+	tools := make(map[string]bool, len(toolNames))
+	for _, name := range toolNames {
+		tools[name] = true
+	}
+	return InstructionRule{
+		ID:          id,
+		ToolNames:   tools,
+		Instruction: instruction,
+	}
+}
+
+// InstructionResolver resolves which instructions apply based on the
+// most-specificity rule: when multiple rules match, rules that are
+// supersets of other matching rules shadow those smaller rules.
+type InstructionResolver struct {
+	rules []InstructionRule
+}
+
+// NewInstructionResolver creates a new resolver with the given rules.
+func NewInstructionResolver(rules []InstructionRule) *InstructionResolver {
+	return &InstructionResolver{rules: rules}
+}
+
+// isSubset returns true if a is a subset of b (all elements of a are in b).
+func isSubset(a, b map[string]bool) bool {
+	for key := range a {
+		if !b[key] {
+			return false
+		}
+	}
+	return true
+}
+
+// isProperSubset returns true if a is a proper subset of b (a ⊂ b, not equal).
+func isProperSubset(a, b map[string]bool) bool {
+	return len(a) < len(b) && isSubset(a, b)
+}
+
+// matchingRule represents a rule that matched the active tools.
+type matchingRule struct {
+	rule     InstructionRule
+	shadowed bool
+}
+
+// ResolveInstructions returns the instructions that apply to the given active tools,
+// using the most-specificity rule to eliminate shadowed rules.
+//
+// A rule is "shadowed" if another matching rule's ToolNames is a proper superset
+// of this rule's ToolNames. The idea is that the more specific rule (covering more
+// tools) should take precedence.
+func (r *InstructionResolver) ResolveInstructions(activeTools []string) []string {
+	// Build active tools set for O(1) lookup
+	activeSet := make(map[string]bool, len(activeTools))
+	for _, tool := range activeTools {
+		activeSet[tool] = true
+	}
+
+	// Find all rules where ALL required tools are present in activeSet
+	var matches []matchingRule
+	for _, rule := range r.rules {
+		if isSubset(rule.ToolNames, activeSet) {
+			matches = append(matches, matchingRule{rule: rule})
+		}
+	}
+
+	// Apply shadowing: mark rules as shadowed if another rule is a proper superset
+	for i := range matches {
+		for j := range matches {
+			if i == j {
+				continue
+			}
+			// If rule j's tools are a proper superset of rule i's tools,
+			// then rule i is shadowed by rule j
+			if isProperSubset(matches[i].rule.ToolNames, matches[j].rule.ToolNames) {
+				matches[i].shadowed = true
+				break // Once shadowed, no need to check further
+			}
+		}
+	}
+
+	// Collect non-shadowed instructions, sorted by rule ID for determinism
+	var result []string
+	for _, m := range matches {
+		if !m.shadowed {
+			result = append(result, m.rule.Instruction)
+		}
+	}
+
+	// Sort for deterministic output (by instruction content as a simple approach)
+	sort.Strings(result)
+
+	return result
+}
+
+// MatchingRuleIDs returns the IDs of rules that match and are not shadowed.
+// Useful for debugging/testing.
+func (r *InstructionResolver) MatchingRuleIDs(activeTools []string) []string {
+	activeSet := make(map[string]bool, len(activeTools))
+	for _, tool := range activeTools {
+		activeSet[tool] = true
+	}
+
+	var matches []matchingRule
+	for _, rule := range r.rules {
+		if isSubset(rule.ToolNames, activeSet) {
+			matches = append(matches, matchingRule{rule: rule})
+		}
+	}
+
+	for i := range matches {
+		for j := range matches {
+			if i == j {
+				continue
+			}
+			if isProperSubset(matches[i].rule.ToolNames, matches[j].rule.ToolNames) {
+				matches[i].shadowed = true
+				break
+			}
+		}
+	}
+
+	var ids []string
+	for _, m := range matches {
+		if !m.shadowed {
+			ids = append(ids, m.rule.ID)
+		}
+	}
+	sort.Strings(ids)
+	return ids
+}
+
+// =============================================================================
+// END SPIKE
+// =============================================================================
+
 // GenerateInstructions creates server instructions based on enabled toolsets
 func GenerateInstructions(enabledToolsets []string) string {
 	// For testing - add a flag to disable instructions

pkg/github/instructions_test.go

@@ -184,3 +184,211 @@ func TestGetToolsetInstructions(t *testing.T) {
 		})
 	}
 }
+
+// =============================================================================
+// SPIKE TESTS: InstructionResolver with most-specificity rule
+// =============================================================================
+
+func TestInstructionResolver_BasicMatching(t *testing.T) {
+	resolver := NewInstructionResolver([]InstructionRule{
+		NewInstructionRule("rule-a", "Instruction A", "tool1"),
+		NewInstructionRule("rule-b", "Instruction B", "tool2"),
+		NewInstructionRule("rule-c", "Instruction C", "tool3"),
+	})
+
+	tests := []struct {
+		name         string
+		activeTools  []string
+		expectedIDs  []string
+		expectedInst []string
+	}{
+		{
+			name:         "single tool matches single rule",
+			activeTools:  []string{"tool1"},
+			expectedIDs:  []string{"rule-a"},
+			expectedInst: []string{"Instruction A"},
+		},
+		{
+			name:         "two tools match two rules",
+			activeTools:  []string{"tool1", "tool2"},
+			expectedIDs:  []string{"rule-a", "rule-b"},
+			expectedInst: []string{"Instruction A", "Instruction B"},
+		},
+		{
+			name:         "no matching tools",
+			activeTools:  []string{"tool4"},
+			expectedIDs:  []string{},
+			expectedInst: []string{},
+		},
+		{
+			name:         "all tools match all rules",
+			activeTools:  []string{"tool1", "tool2", "tool3"},
+			expectedIDs:  []string{"rule-a", "rule-b", "rule-c"},
+			expectedInst: []string{"Instruction A", "Instruction B", "Instruction C"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ids := resolver.MatchingRuleIDs(tt.activeTools)
+			instructions := resolver.ResolveInstructions(tt.activeTools)
+
+			if len(ids) != len(tt.expectedIDs) {
+				t.Errorf("Expected %d rule IDs, got %d: %v", len(tt.expectedIDs), len(ids), ids)
+			}
+			for i, id := range tt.expectedIDs {
+				if i >= len(ids) || ids[i] != id {
+					t.Errorf("Expected rule ID %s at position %d, got %v", id, i, ids)
+				}
+			}
+
+			if len(instructions) != len(tt.expectedInst) {
+				t.Errorf("Expected %d instructions, got %d: %v", len(tt.expectedInst), len(instructions), instructions)
+			}
+		})
+	}
+}
+
+func TestInstructionResolver_SupersetShadowing(t *testing.T) {
+	// Rule with more tools (superset) shadows rules with fewer tools
+	resolver := NewInstructionResolver([]InstructionRule{
+		NewInstructionRule("issues-read", "Read issues instruction", "get_issue", "list_issues"),
+		NewInstructionRule("issues-all", "All issues instruction", "get_issue", "list_issues", "create_issue"),
+		NewInstructionRule("create-only", "Create only instruction", "create_issue"),
+	})
+
+	tests := []struct {
+		name        string
+		activeTools []string
+		expectedIDs []string
+		description string
+	}{
+		{
+			name:        "superset rule shadows subset rules",
+			activeTools: []string{"get_issue", "list_issues", "create_issue"},
+			expectedIDs: []string{"issues-all"},
+			description: "issues-all shadows issues-read (superset) and create-only (superset)",
+		},
+		{
+			name:        "no shadowing when superset rule doesn't match",
+			activeTools: []string{"get_issue", "list_issues"},
+			expectedIDs: []string{"issues-read"},
+			description: "issues-all doesn't match (missing create_issue), so issues-read is not shadowed",
+		},
+		{
+			name:        "single tool rule not shadowed when superset doesn't match",
+			activeTools: []string{"create_issue"},
+			expectedIDs: []string{"create-only"},
+			description: "Only create-only matches",
+		},
+		{
+			name:        "extra active tools don't affect shadowing",
+			activeTools: []string{"get_issue", "list_issues", "create_issue", "get_me", "other_tool"},
+			expectedIDs: []string{"issues-all"},
+			description: "Extra tools in activeTools don't prevent shadowing",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ids := resolver.MatchingRuleIDs(tt.activeTools)
+
+			if len(ids) != len(tt.expectedIDs) {
+				t.Errorf("%s: Expected %d rule IDs %v, got %d: %v",
+					tt.description, len(tt.expectedIDs), tt.expectedIDs, len(ids), ids)
+				return
+			}
+			for i, id := range tt.expectedIDs {
+				if i >= len(ids) || ids[i] != id {
+					t.Errorf("%s: Expected rule ID %s at position %d, got %v",
+						tt.description, id, i, ids)
+				}
+			}
+		})
+	}
+}
+
+func TestInstructionResolver_PartialOverlapNoShadowing(t *testing.T) {
+	// Rules with partial overlap (neither is superset of other) should both apply
+	resolver := NewInstructionResolver([]InstructionRule{
+		NewInstructionRule("rule-ab", "AB instruction", "tool_a", "tool_b"),
+		NewInstructionRule("rule-bc", "BC instruction", "tool_b", "tool_c"),
+	})
+
+	// When all three tools are active, both rules match
+	// Neither is a superset of the other, so neither is shadowed
+	ids := resolver.MatchingRuleIDs([]string{"tool_a", "tool_b", "tool_c"})
+
+	if len(ids) != 2 {
+		t.Errorf("Expected 2 rules (partial overlap, no shadowing), got %d: %v", len(ids), ids)
+	}
+}
+
+func TestInstructionResolver_ComplexHierarchy(t *testing.T) {
+	// Create a hierarchy: rule1 ⊂ rule2 ⊂ rule3
+	resolver := NewInstructionResolver([]InstructionRule{
+		NewInstructionRule("level1", "Level 1 instruction", "t1"),
+		NewInstructionRule("level2", "Level 2 instruction", "t1", "t2"),
+		NewInstructionRule("level3", "Level 3 instruction", "t1", "t2", "t3"),
+	})
+
+	tests := []struct {
+		name        string
+		activeTools []string
+		expectedIDs []string
+	}{
+		{
+			name:        "only level1 active",
+			activeTools: []string{"t1"},
+			expectedIDs: []string{"level1"},
+		},
+		{
+			name:        "level1 and level2 tools active",
+			activeTools: []string{"t1", "t2"},
+			expectedIDs: []string{"level2"}, // shadows level1
+		},
+		{
+			name:        "all three levels active",
+			activeTools: []string{"t1", "t2", "t3"},
+			expectedIDs: []string{"level3"}, // shadows level1 and level2
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ids := resolver.MatchingRuleIDs(tt.activeTools)
+			if len(ids) != len(tt.expectedIDs) {
+				t.Errorf("Expected %v, got %v", tt.expectedIDs, ids)
+			}
+		})
+	}
+}
+
+func TestInstructionResolver_EmptyRules(t *testing.T) {
+	resolver := NewInstructionResolver([]InstructionRule{})
+
+	ids := resolver.MatchingRuleIDs([]string{"tool1", "tool2"})
+	if len(ids) != 0 {
+		t.Errorf("Expected no matches for empty resolver, got %v", ids)
+	}
+
+	instructions := resolver.ResolveInstructions([]string{"tool1"})
+	if len(instructions) != 0 {
+		t.Errorf("Expected no instructions for empty resolver, got %v", instructions)
+	}
+}
+
+func TestInstructionResolver_EmptyActiveTools(t *testing.T) {
+	resolver := NewInstructionResolver([]InstructionRule{
+		NewInstructionRule("rule1", "Instruction 1", "tool1"),
+	})
+
+	ids := resolver.MatchingRuleIDs([]string{})
+	if len(ids) != 0 {
+		t.Errorf("Expected no matches for empty active tools, got %v", ids)
+	}
+}
+
+// =============================================================================
+// END SPIKE TESTS
+// =============================================================================