gadfly/cmd/gadfly/specialists.go

package main

import (
	"fmt"
	"os"
	"path/filepath"
	"sort"
	"strings"

	"gopkg.in/yaml.v3"
)

// Specialist is one review lens: a focused instruction appended to the base
// system prompt, run as its own review (+recheck) pass and rendered as its own
// section in the consolidated comment.
type Specialist struct {
	Name  string // lowercase id, used in GADFLY_SPECIALISTS and section anchors
	Title string // human label for the section header
	Focus string // lens-specific instruction appended to the base prompt
}

// builtinSpecialists are the lenses Gadfly ships with. The default suite (below)
// is the "biggies"; the rest are opt-in by name via GADFLY_SPECIALISTS, a
// .gadfly.yml `specialists:` list, or `all`.
var builtinSpecialists = map[string]Specialist{
	"security": {"security", "🔒 Security", "" +
		"Authn/authz gaps, injection (SQL/command/path/template), SSRF, unsafe deserialization, " +
		"secret/credential leakage, missing input validation, unsafe handling of untrusted data, " +
		"and insecure defaults. Trace tainted input to where it's used."},
	"correctness": {"correctness", "🎯 Correctness", "" +
		"Logic bugs and incorrect behavior introduced by the change. Pay special attention to " +
		"SEMANTIC / domain correctness: do NOT trust a constant, conversion factor, formula, unit, " +
		"or threshold because it looks reasonable — independently RE-DERIVE the expected value from " +
		"first principles (units, dimensions, boundary values) and compare. A plausible-looking magic " +
		"number is exactly where real bugs hide."},
	"maintainability": {"maintainability", "🧹 Code cleanliness & maintainability", "" +
		"Readability and structure: dead/duplicated code, confusing names, overly long or deeply " +
		"nested functions, leaky abstractions, copy-paste that should be shared, and changes that " +
		"don't follow the patterns the surrounding code already uses. Prefer concrete, low-churn fixes."},
	"performance": {"performance", "⚡ Performance", "" +
		"Efficiency regressions: N+1 queries, unnecessary allocations or copies, work inside hot loops, " +
		"unbounded growth, missing pagination/limits, blocking calls on hot paths, and avoidable " +
		"quadratic behavior. Only flag impact you can justify, not micro-optimizations."},
	"error-handling": {"error-handling", "🧯 Error handling & edge cases", "" +
		"Ignored or swallowed errors, missing cleanup/rollback/`defer`, panics on bad input, and " +
		"unhandled edge cases: nil/null, empty collections, zero/negative, integer overflow, and " +
		"boundary (off-by-one) conditions. Check the unhappy paths the diff introduces."},

	// --- opt-in (not in the default suite) ---
	"tests": {"tests", "🧪 Tests", "" +
		"Whether the change is adequately tested: new logic without tests, changed behavior whose " +
		"tests weren't updated, missing edge-case/error-path coverage, and brittle or assertion-free tests."},
	"docs": {"docs", "📝 Docs", "" +
		"Documentation drift: public APIs/flags/config changed without updating doc comments, README, " +
		"or examples; stale comments now contradicting the code; missing docs for new exported surface."},
	"conventions": {"conventions", "📐 Conventions", "" +
		"Adherence to THIS repo's own conventions. Discover them — read any README / CONTRIBUTING / " +
		"CLAUDE.md / AGENTS.md / lint config the repo ships, and hold the change to the documented and " +
		"surrounding-code patterns."},
	"improvements": {"improvements", "💡 Improvements (optional)", "" +
		"OPTIONAL, non-blocking suggestions only. Surface at most 1–2 genuinely high-value blind spots " +
		"or simplifications the author likely wants to know about. Be quiet: if nothing rises to that " +
		"bar, reply exactly 'No suggestions.' Never nitpick, never pad. These never affect the verdict."},
}

// defaultSuite is used when nothing selects specialists explicitly.
var defaultSuite = []string{"security", "correctness", "maintainability", "performance", "error-handling"}

// fileConfig is the optional .gadfly.yml schema.
type fileConfig struct {
	// Specialists selects which run (names). Empty => the default suite.
	Specialists []string `yaml:"specialists"`
	// Define adds or overrides specialist definitions.
	Define []struct {
		Name  string `yaml:"name"`
		Title string `yaml:"title"`
		Focus string `yaml:"focus"`
	} `yaml:"define"`
}

// resolveSpecialists builds the active specialist list from (in increasing
// precedence) the built-ins, a repo .gadfly.yml, and GADFLY_SPECIALIST_<NAME>
// env definitions. The active SELECTION is GADFLY_SPECIALISTS (csv, "all", or
// "auto"), else the file's `specialists:`, else the default suite. Unknown
// names are reported as errors and skipped; valid ones still run.
//
// It also returns the full registry (the catalog of every defined specialist)
// and an auto flag: when the selection is "auto", specs is nil and the caller
// runs the dynamic selector (see auto.go) against the registry instead.
func resolveSpecialists(repoDir string) (specs []Specialist, registry map[string]Specialist, auto bool, errs []error) {

	// 1. registry seeded with built-ins.
	registry = make(map[string]Specialist, len(builtinSpecialists))
	for k, v := range builtinSpecialists {
		registry[k] = v
	}

	// 2. config file: definitions + (maybe) the selection.
	var fileSelection []string
	if cfg, ok, err := loadFileConfig(repoDir); err != nil {
		errs = append(errs, err)
	} else if ok {
		for _, d := range cfg.Define {
			name := strings.ToLower(strings.TrimSpace(d.Name))
			if name == "" || strings.TrimSpace(d.Focus) == "" {
				errs = append(errs, fmt.Errorf(".gadfly.yml: specialist define needs non-empty name and focus"))
				continue
			}
			registry[name] = Specialist{Name: name, Title: titleOr(d.Title, name), Focus: d.Focus}
		}
		fileSelection = cfg.Specialists
	}

	// 3. env definitions: GADFLY_SPECIALIST_<NAME>="focus".
	for _, kv := range os.Environ() {
		key, val, _ := strings.Cut(kv, "=")
		if !strings.HasPrefix(key, "GADFLY_SPECIALIST_") || len(key) == len("GADFLY_SPECIALIST_") {
			continue
		}
		name := strings.ToLower(strings.TrimPrefix(key, "GADFLY_SPECIALIST_"))
		if strings.TrimSpace(val) == "" {
			errs = append(errs, fmt.Errorf("%s: empty specialist focus", key))
			continue
		}
		existing, ok := registry[name]
		title := titleOr("", name)
		if ok {
			title = existing.Title
		}
		registry[name] = Specialist{Name: name, Title: title, Focus: val}
	}

	// 4. selection: env > file > default suite.
	var names []string
	switch {
	case strings.TrimSpace(os.Getenv("GADFLY_SPECIALISTS")) != "":
		names = splitCSV(os.Getenv("GADFLY_SPECIALISTS"))
	case len(fileSelection) > 0:
		names = fileSelection
	default:
		names = defaultSuite
	}
	if len(names) == 1 && strings.EqualFold(names[0], "auto") {
		// Dynamic selection: caller runs the selector against the registry.
		return nil, registry, true, errs
	}
	if len(names) == 1 && strings.EqualFold(names[0], "all") {
		names = sortedKeys(registry)
	}

	// 5. resolve names -> specialists.
	var out []Specialist
	seen := map[string]bool{}
	for _, raw := range names {
		name := strings.ToLower(strings.TrimSpace(raw))
		if name == "" || seen[name] {
			continue
		}
		seen[name] = true
		sp, ok := registry[name]
		if !ok {
			errs = append(errs, fmt.Errorf("unknown specialist %q (built-ins: %s; or define it via .gadfly.yml / GADFLY_SPECIALIST_%s)",
				name, strings.Join(sortedKeys(builtinSpecialists), ", "), strings.ToUpper(name)))
			continue
		}
		out = append(out, sp)
	}
	return out, registry, false, errs
}

// suiteFromRegistry resolves names against the registry, skipping any missing.
// Used as the auto-mode fallback (the default suite always exists in builtins).
func suiteFromRegistry(registry map[string]Specialist, names []string) []Specialist {
	var out []Specialist
	for _, n := range names {
		if sp, ok := registry[n]; ok {
			out = append(out, sp)
		}
	}
	return out
}

// specialistNamesOf returns the names of a specialist slice (for logging).
func specialistNamesOf(specs []Specialist) []string {
	names := make([]string, len(specs))
	for i, s := range specs {
		names[i] = s.Name
	}
	return names
}

// composeSpecialistPrompt appends a specialist's lens to the base system prompt.
func composeSpecialistPrompt(base string, sp Specialist) string {
	return strings.TrimRight(base, "\n") +
		"\n\n## Your assigned lens — " + sp.Title + "\n" +
		"Review the change specifically and ONLY through this lens. Scrutinize it for:\n" +
		sp.Focus +
		"\n\nStay in this lane: other lenses (correctness, security, performance, etc.) are reviewed " +
		"separately, so don't duplicate their findings here. If nothing in your lens is materially " +
		"wrong, reply with the \"No material issues found\" verdict for this lens — do not reach for " +
		"another lens's issue just to have something to say."
}

func loadFileConfig(repoDir string) (fileConfig, bool, error) {
	candidates := []string{}
	if p := strings.TrimSpace(os.Getenv("GADFLY_CONFIG")); p != "" {
		if !filepath.IsAbs(p) {
			p = filepath.Join(repoDir, p)
		}
		candidates = append(candidates, p)
	} else {
		candidates = append(candidates, filepath.Join(repoDir, ".gadfly.yml"), filepath.Join(repoDir, ".gadfly.yaml"))
	}
	for _, p := range candidates {
		data, err := os.ReadFile(p)
		if err != nil {
			continue // not found / unreadable: try next
		}
		var cfg fileConfig
		if err := yaml.Unmarshal(data, &cfg); err != nil {
			return fileConfig{}, false, fmt.Errorf("parse %s: %w", filepath.Base(p), err)
		}
		return cfg, true, nil
	}
	return fileConfig{}, false, nil
}

func titleOr(title, name string) string {
	if strings.TrimSpace(title) != "" {
		return title
	}
	return name
}

func splitCSV(s string) []string {
	var out []string
	for _, p := range strings.Split(s, ",") {
		if t := strings.TrimSpace(p); t != "" {
			out = append(out, t)
		}
	}
	return out
}

func sortedKeys(m map[string]Specialist) []string {
	keys := make([]string, 0, len(m))
	for k := range m {
		keys = append(keys, k)
	}
	sort.Strings(keys)
	return keys
}