From a643b9ab67c82264889523a061c5ac73f831a71f Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Thu, 25 Dec 2025 01:53:43 -0800 Subject: [PATCH] Implement advice operators for formula composition (gt-8tmz.2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add Lisp-style advice operators to the formula DSL: - before(target, step) - insert step before target - after(target, step) - insert step after target - around(target, wrapper) - wrap target with before/after Features: - Glob pattern matching for targets (*.implement, shiny.*, etc) - Pointcut matching by type or label - Step reference substitution ({step.id}, {step.title}) - Automatic dependency chaining New types: AdviceRule, AdviceStep, AroundAdvice, Pointcut New functions: ApplyAdvice, MatchGlob, MatchPointcut 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- cmd/bd/cook.go | 5 + internal/formula/advice.go | 234 +++++++++++++++++++++++ internal/formula/advice_test.go | 323 ++++++++++++++++++++++++++++++++ internal/formula/types.go | 70 +++++++ 4 files changed, 632 insertions(+) create mode 100644 internal/formula/advice.go create mode 100644 internal/formula/advice_test.go diff --git a/cmd/bd/cook.go b/cmd/bd/cook.go index d9fa5506..374030af 100644 --- a/cmd/bd/cook.go +++ b/cmd/bd/cook.go @@ -93,6 +93,11 @@ func runCook(cmd *cobra.Command, args []string) { os.Exit(1) } + // Apply advice transformations (gt-8tmz.2) + if len(resolved.Advice) > 0 { + resolved.Steps = formula.ApplyAdvice(resolved.Steps, resolved.Advice) + } + // Apply prefix to proto ID if specified (bd-47qx) protoID := resolved.Formula if prefix != "" { diff --git a/internal/formula/advice.go b/internal/formula/advice.go new file mode 100644 index 00000000..dc378f58 --- /dev/null +++ b/internal/formula/advice.go @@ -0,0 +1,234 @@ +// Package formula provides advice operators for step transformations. +// +// Advice operators are Lisp-style transformations that insert steps +// before, after, or around matching target steps. They enable +// cross-cutting concerns like logging, security scanning, or +// approval gates to be applied declaratively. +// +// Supported patterns: +// - "design" - exact match +// - "*.implement" - suffix match (any step ending in .implement) +// - "shiny.*" - prefix match (any step starting with shiny.) +// - "*" - match all steps +package formula + +import ( + "path/filepath" + "strings" +) + +// MatchGlob checks if a step ID matches a glob pattern. +// Supported patterns: +// - "exact" - exact match +// - "*.suffix" - ends with .suffix +// - "prefix.*" - starts with prefix. +// - "*" - matches everything +// - "prefix.*.suffix" - starts with prefix. and ends with .suffix +func MatchGlob(pattern, stepID string) bool { + // Use filepath.Match for basic glob support + matched, err := filepath.Match(pattern, stepID) + if err == nil && matched { + return true + } + + // Handle additional patterns + if pattern == "*" { + return true + } + + // *.suffix pattern (e.g., "*.implement") + if strings.HasPrefix(pattern, "*.") { + suffix := pattern[1:] // ".implement" + return strings.HasSuffix(stepID, suffix) + } + + // prefix.* pattern (e.g., "shiny.*") + if strings.HasSuffix(pattern, ".*") { + prefix := pattern[:len(pattern)-1] // "shiny." + return strings.HasPrefix(stepID, prefix) + } + + // Exact match + return pattern == stepID +} + +// ApplyAdvice transforms a formula's steps by applying advice rules. +// Returns a new steps slice with advice steps inserted. +// The original steps slice is not modified. +func ApplyAdvice(steps []*Step, advice []*AdviceRule) []*Step { + if len(advice) == 0 { + return steps + } + + result := make([]*Step, 0, len(steps)*2) // Pre-allocate for insertions + + for _, step := range steps { + // Find matching advice rules for this step + var beforeSteps []*Step + var afterSteps []*Step + + for _, rule := range advice { + if !MatchGlob(rule.Target, step.ID) { + continue + } + + // Collect before steps + if rule.Before != nil { + beforeSteps = append(beforeSteps, adviceStepToStep(rule.Before, step)) + } + if rule.Around != nil { + for _, as := range rule.Around.Before { + beforeSteps = append(beforeSteps, adviceStepToStep(as, step)) + } + } + + // Collect after steps + if rule.After != nil { + afterSteps = append(afterSteps, adviceStepToStep(rule.After, step)) + } + if rule.Around != nil { + for _, as := range rule.Around.After { + afterSteps = append(afterSteps, adviceStepToStep(as, step)) + } + } + } + + // Insert before steps + for _, bs := range beforeSteps { + result = append(result, bs) + } + + // Clone the original step and update its dependencies + clonedStep := cloneStep(step) + + // If there are before steps, the original step needs to depend on the last before step + if len(beforeSteps) > 0 { + lastBefore := beforeSteps[len(beforeSteps)-1] + clonedStep.Needs = appendUnique(clonedStep.Needs, lastBefore.ID) + } + + // Chain before steps together + for i := 1; i < len(beforeSteps); i++ { + beforeSteps[i].Needs = appendUnique(beforeSteps[i].Needs, beforeSteps[i-1].ID) + } + + result = append(result, clonedStep) + + // Insert after steps and chain them + for i, as := range afterSteps { + if i == 0 { + // First after step depends on the original step + as.Needs = appendUnique(as.Needs, step.ID) + } else { + // Subsequent after steps chain to previous + as.Needs = appendUnique(as.Needs, afterSteps[i-1].ID) + } + result = append(result, as) + } + + // Recursively apply advice to children + if len(step.Children) > 0 { + clonedStep.Children = ApplyAdvice(step.Children, advice) + } + } + + return result +} + +// adviceStepToStep converts an AdviceStep to a Step. +// Substitutes {step.id} placeholders with the target step's ID. +func adviceStepToStep(as *AdviceStep, target *Step) *Step { + // Substitute {step.id} in ID and Title + id := substituteStepRef(as.ID, target) + title := substituteStepRef(as.Title, target) + if title == "" { + title = id + } + desc := substituteStepRef(as.Description, target) + + return &Step{ + ID: id, + Title: title, + Description: desc, + Type: as.Type, + } +} + +// substituteStepRef replaces {step.id} with the target step's ID. +func substituteStepRef(s string, target *Step) string { + s = strings.ReplaceAll(s, "{step.id}", target.ID) + s = strings.ReplaceAll(s, "{step.title}", target.Title) + return s +} + +// cloneStep creates a shallow copy of a step. +func cloneStep(s *Step) *Step { + clone := *s + // Deep copy slices + if len(s.DependsOn) > 0 { + clone.DependsOn = make([]string, len(s.DependsOn)) + copy(clone.DependsOn, s.DependsOn) + } + if len(s.Needs) > 0 { + clone.Needs = make([]string, len(s.Needs)) + copy(clone.Needs, s.Needs) + } + if len(s.Labels) > 0 { + clone.Labels = make([]string, len(s.Labels)) + copy(clone.Labels, s.Labels) + } + // Don't deep copy children here - ApplyAdvice handles that recursively + return &clone +} + +// appendUnique appends an item to a slice if not already present. +func appendUnique(slice []string, item string) []string { + for _, s := range slice { + if s == item { + return slice + } + } + return append(slice, item) +} + +// MatchPointcut checks if a step matches a pointcut. +func MatchPointcut(pc *Pointcut, step *Step) bool { + // Glob match on step ID + if pc.Glob != "" && !MatchGlob(pc.Glob, step.ID) { + return false + } + + // Type match + if pc.Type != "" && step.Type != pc.Type { + return false + } + + // Label match + if pc.Label != "" { + found := false + for _, l := range step.Labels { + if l == pc.Label { + found = true + break + } + } + if !found { + return false + } + } + + return true +} + +// MatchAnyPointcut checks if a step matches any pointcut in the list. +func MatchAnyPointcut(pointcuts []*Pointcut, step *Step) bool { + if len(pointcuts) == 0 { + return true // No pointcuts means match all + } + for _, pc := range pointcuts { + if MatchPointcut(pc, step) { + return true + } + } + return false +} diff --git a/internal/formula/advice_test.go b/internal/formula/advice_test.go new file mode 100644 index 00000000..28f57509 --- /dev/null +++ b/internal/formula/advice_test.go @@ -0,0 +1,323 @@ +package formula + +import ( + "testing" +) + +func TestMatchGlob(t *testing.T) { + tests := []struct { + pattern string + stepID string + want bool + }{ + // Exact matches + {"design", "design", true}, + {"design", "implement", false}, + {"design", "design.draft", false}, + + // Wildcard all + {"*", "design", true}, + {"*", "implement.draft", true}, + {"*", "", true}, + + // Suffix patterns (*.suffix) + {"*.implement", "shiny.implement", true}, + {"*.implement", "design.implement", true}, + {"*.implement", "implement", false}, + {"*.implement", "shiny.design", false}, + + // Prefix patterns (prefix.*) + {"shiny.*", "shiny.design", true}, + {"shiny.*", "shiny.implement", true}, + {"shiny.*", "shiny", false}, + {"shiny.*", "enterprise.design", false}, + + // Complex patterns + {"*.refine-*", "implement.refine-1", true}, + {"*.refine-*", "implement.refine-2", true}, + {"*.refine-*", "implement.draft", false}, + } + + for _, tt := range tests { + t.Run(tt.pattern+"_"+tt.stepID, func(t *testing.T) { + got := MatchGlob(tt.pattern, tt.stepID) + if got != tt.want { + t.Errorf("MatchGlob(%q, %q) = %v, want %v", tt.pattern, tt.stepID, got, tt.want) + } + }) + } +} + +func TestApplyAdvice_Before(t *testing.T) { + steps := []*Step{ + {ID: "design", Title: "Design"}, + {ID: "implement", Title: "Implement"}, + } + + advice := []*AdviceRule{ + { + Target: "implement", + Before: &AdviceStep{ + ID: "lint-{step.id}", + Title: "Lint before {step.id}", + }, + }, + } + + result := ApplyAdvice(steps, advice) + + if len(result) != 3 { + t.Fatalf("expected 3 steps, got %d", len(result)) + } + + // Check order: design, lint-implement, implement + if result[0].ID != "design" { + t.Errorf("expected first step 'design', got %q", result[0].ID) + } + if result[1].ID != "lint-implement" { + t.Errorf("expected second step 'lint-implement', got %q", result[1].ID) + } + if result[2].ID != "implement" { + t.Errorf("expected third step 'implement', got %q", result[2].ID) + } + + // Check that implement now depends on lint-implement + if !contains(result[2].Needs, "lint-implement") { + t.Errorf("implement should depend on lint-implement, got needs: %v", result[2].Needs) + } +} + +func TestApplyAdvice_After(t *testing.T) { + steps := []*Step{ + {ID: "implement", Title: "Implement"}, + {ID: "submit", Title: "Submit"}, + } + + advice := []*AdviceRule{ + { + Target: "implement", + After: &AdviceStep{ + ID: "test-{step.id}", + Title: "Test after {step.id}", + }, + }, + } + + result := ApplyAdvice(steps, advice) + + if len(result) != 3 { + t.Fatalf("expected 3 steps, got %d", len(result)) + } + + // Check order: implement, test-implement, submit + if result[0].ID != "implement" { + t.Errorf("expected first step 'implement', got %q", result[0].ID) + } + if result[1].ID != "test-implement" { + t.Errorf("expected second step 'test-implement', got %q", result[1].ID) + } + if result[2].ID != "submit" { + t.Errorf("expected third step 'submit', got %q", result[2].ID) + } + + // Check that test-implement depends on implement + if !contains(result[1].Needs, "implement") { + t.Errorf("test-implement should depend on implement, got needs: %v", result[1].Needs) + } +} + +func TestApplyAdvice_Around(t *testing.T) { + steps := []*Step{ + {ID: "implement", Title: "Implement"}, + } + + advice := []*AdviceRule{ + { + Target: "implement", + Around: &AroundAdvice{ + Before: []*AdviceStep{ + {ID: "pre-scan", Title: "Pre-scan"}, + }, + After: []*AdviceStep{ + {ID: "post-scan", Title: "Post-scan"}, + }, + }, + }, + } + + result := ApplyAdvice(steps, advice) + + if len(result) != 3 { + t.Fatalf("expected 3 steps, got %d", len(result)) + } + + // Check order: pre-scan, implement, post-scan + if result[0].ID != "pre-scan" { + t.Errorf("expected first step 'pre-scan', got %q", result[0].ID) + } + if result[1].ID != "implement" { + t.Errorf("expected second step 'implement', got %q", result[1].ID) + } + if result[2].ID != "post-scan" { + t.Errorf("expected third step 'post-scan', got %q", result[2].ID) + } + + // Check dependencies + if !contains(result[1].Needs, "pre-scan") { + t.Errorf("implement should depend on pre-scan, got needs: %v", result[1].Needs) + } + if !contains(result[2].Needs, "implement") { + t.Errorf("post-scan should depend on implement, got needs: %v", result[2].Needs) + } +} + +func TestApplyAdvice_GlobPattern(t *testing.T) { + steps := []*Step{ + {ID: "design", Title: "Design"}, + {ID: "shiny.implement", Title: "Implement"}, + {ID: "shiny.review", Title: "Review"}, + } + + advice := []*AdviceRule{ + { + Target: "shiny.*", + Before: &AdviceStep{ + ID: "log-{step.id}", + Title: "Log {step.id}", + }, + }, + } + + result := ApplyAdvice(steps, advice) + + // Should have: design, log-shiny.implement, shiny.implement, log-shiny.review, shiny.review + if len(result) != 5 { + t.Fatalf("expected 5 steps, got %d", len(result)) + } + + if result[0].ID != "design" { + t.Errorf("expected first step 'design', got %q", result[0].ID) + } + if result[1].ID != "log-shiny.implement" { + t.Errorf("expected second step 'log-shiny.implement', got %q", result[1].ID) + } + if result[2].ID != "shiny.implement" { + t.Errorf("expected third step 'shiny.implement', got %q", result[2].ID) + } +} + +func TestApplyAdvice_NoMatch(t *testing.T) { + steps := []*Step{ + {ID: "design", Title: "Design"}, + {ID: "implement", Title: "Implement"}, + } + + advice := []*AdviceRule{ + { + Target: "nonexistent", + Before: &AdviceStep{ + ID: "lint", + Title: "Lint", + }, + }, + } + + result := ApplyAdvice(steps, advice) + + // No changes expected + if len(result) != 2 { + t.Fatalf("expected 2 steps, got %d", len(result)) + } + + if result[0].ID != "design" || result[1].ID != "implement" { + t.Errorf("steps should be unchanged") + } +} + +func TestApplyAdvice_EmptyAdvice(t *testing.T) { + steps := []*Step{ + {ID: "design", Title: "Design"}, + } + + result := ApplyAdvice(steps, nil) + + if len(result) != 1 || result[0].ID != "design" { + t.Errorf("empty advice should return original steps") + } +} + +func TestMatchPointcut(t *testing.T) { + tests := []struct { + name string + pc *Pointcut + step *Step + want bool + }{ + { + name: "glob match", + pc: &Pointcut{Glob: "*.implement"}, + step: &Step{ID: "shiny.implement"}, + want: true, + }, + { + name: "glob no match", + pc: &Pointcut{Glob: "*.implement"}, + step: &Step{ID: "shiny.design"}, + want: false, + }, + { + name: "type match", + pc: &Pointcut{Type: "bug"}, + step: &Step{ID: "fix", Type: "bug"}, + want: true, + }, + { + name: "type no match", + pc: &Pointcut{Type: "bug"}, + step: &Step{ID: "fix", Type: "task"}, + want: false, + }, + { + name: "label match", + pc: &Pointcut{Label: "security"}, + step: &Step{ID: "audit", Labels: []string{"security", "review"}}, + want: true, + }, + { + name: "label no match", + pc: &Pointcut{Label: "security"}, + step: &Step{ID: "audit", Labels: []string{"review"}}, + want: false, + }, + { + name: "combined match", + pc: &Pointcut{Glob: "*.implement", Type: "task"}, + step: &Step{ID: "shiny.implement", Type: "task"}, + want: true, + }, + { + name: "combined partial fail", + pc: &Pointcut{Glob: "*.implement", Type: "task"}, + step: &Step{ID: "shiny.implement", Type: "bug"}, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := MatchPointcut(tt.pc, tt.step) + if got != tt.want { + t.Errorf("MatchPointcut() = %v, want %v", got, tt.want) + } + }) + } +} + +func contains(slice []string, item string) bool { + for _, s := range slice { + if s == item { + return true + } + } + return false +} diff --git a/internal/formula/types.go b/internal/formula/types.go index 46867a82..0e825851 100644 --- a/internal/formula/types.go +++ b/internal/formula/types.go @@ -86,6 +86,14 @@ type Formula struct { // Compose defines composition/bonding rules. Compose *ComposeRules `json:"compose,omitempty"` + // Advice defines step transformations (before/after/around). + // Applied during cooking to insert steps around matching targets. + Advice []*AdviceRule `json:"advice,omitempty"` + + // Pointcuts defines target patterns for aspect formulas. + // Used with TypeAspect to specify which steps the aspect applies to. + Pointcuts []*Pointcut `json:"pointcuts,omitempty"` + // Source tracks where this formula was loaded from (set by parser). Source string `json:"source,omitempty"` } @@ -227,6 +235,68 @@ type Hook struct { Vars map[string]string `json:"vars,omitempty"` } +// Pointcut defines a target pattern for advice application. +// Used in aspect formulas to specify which steps the advice applies to. +type Pointcut struct { + // Glob is a glob pattern to match step IDs. + // Examples: "*.implement", "shiny.*", "review" + Glob string `json:"glob,omitempty"` + + // Type matches steps by their type field. + // Examples: "task", "bug", "epic" + Type string `json:"type,omitempty"` + + // Label matches steps that have a specific label. + Label string `json:"label,omitempty"` +} + +// AdviceRule defines a step transformation rule. +// Advice operators insert steps before, after, or around matching targets. +type AdviceRule struct { + // Target is a glob pattern matching step IDs to apply advice to. + // Examples: "*.implement", "design", "shiny.*" + Target string `json:"target"` + + // Before inserts a step before the target. + Before *AdviceStep `json:"before,omitempty"` + + // After inserts a step after the target. + After *AdviceStep `json:"after,omitempty"` + + // Around wraps the target with before and after steps. + Around *AroundAdvice `json:"around,omitempty"` +} + +// AdviceStep defines a step to insert via advice. +type AdviceStep struct { + // ID is the step identifier. Supports {step.id} substitution. + ID string `json:"id"` + + // Title is the step title. Supports {step.id} substitution. + Title string `json:"title,omitempty"` + + // Description is the step description. + Description string `json:"description,omitempty"` + + // Type is the issue type (task, bug, etc). + Type string `json:"type,omitempty"` + + // Args are additional context passed to the step. + Args map[string]string `json:"args,omitempty"` + + // Output defines expected outputs from this step. + Output map[string]string `json:"output,omitempty"` +} + +// AroundAdvice wraps a target with before and after steps. +type AroundAdvice struct { + // Before is a list of steps to insert before the target. + Before []*AdviceStep `json:"before,omitempty"` + + // After is a list of steps to insert after the target. + After []*AdviceStep `json:"after,omitempty"` +} + // Validate checks the formula for structural errors. func (f *Formula) Validate() error { var errs []string