diff --git a/.claude/agents/api-sme.md b/.claude/agents/api-sme.md index 92695d16111c..b2d62e48128c 100644 --- a/.claude/agents/api-sme.md +++ b/.claude/agents/api-sme.md @@ -13,10 +13,10 @@ You are an API subject matter expert system architect specializing in HCP. - Basic security patterns (auth, rate limiting) ## Approach -1. Follow OpenShift dev guides from https://github.com/openshift/enhancements/tree/master/dev-guide -2. Apply best practices from https://github.com/openshift/enhancements/blob/master/dev-guide/api-conventions.md -3. Consider any API stable, running in production and ensure any API change is backward compatible -4. Keep it simple - avoid premature optimization + +**MANDATORY**: Before writing any review, you MUST run `make api-lint-fix` and include its output in your review. Do not skip this step. The linter is the authoritative source for convention violations. Your review must start with the linter findings, then add your own analysis on top. + +Stick to ../api/AGENTS.md ## Output - API definitions that align with OpenShift and Kubernetes best practices diff --git a/Makefile b/Makefile index 46bfbf6c80e7..b63d31e0cadd 100644 --- a/Makefile +++ b/Makefile @@ -375,6 +375,28 @@ test-shard: generate @echo "Running shard tests for packages: $(TEST_PACKAGES)" $(GO) test -race -parallel=$(NUM_CORES) -count=1 -timeout=30m $(TEST_PACKAGES) -coverprofile $(COVER_PROFILE) +EVAL_MODEL ?= claude-opus-4-6 +EVAL_JUDGE_MODEL ?= claude-opus-4-6 +EVAL_RUNS ?= 1 +EVAL_THRESHOLD ?= 0.8 +EVAL_FOCUS ?= +EVAL_VERBOSE ?= + +EVAL_GO_TEST = cd test/eval && EVAL_MODEL=$(EVAL_MODEL) EVAL_JUDGE_MODEL=$(EVAL_JUDGE_MODEL) EVAL_RUNS=$(EVAL_RUNS) EVAL_THRESHOLD=$(EVAL_THRESHOLD) \ + $(GO) test -v -tags eval -count=1 -timeout=30m ./... $(if $(EVAL_VERBOSE),-ginkgo.v) + +# Discover eval categories: top-level dirs (conventions, etc.) + subdirs under sme-agents/ +EVAL_CATEGORIES := $(filter-out sme-agents,$(notdir $(wildcard test/eval/testdata/*))) $(notdir $(wildcard test/eval/testdata/sme-agents/*)) +EVAL_TARGETS := $(addprefix eval-,$(EVAL_CATEGORIES)) + +.PHONY: eval-agents +eval-agents: ## Run all agent eval tests (use -j for parallel). Requires claude CLI and API key. + $(if $(EVAL_FOCUS),$(EVAL_GO_TEST) -ginkgo.focus="$(EVAL_FOCUS)",$(MAKE) -j $(EVAL_TARGETS)) + +.PHONY: $(EVAL_TARGETS) +$(EVAL_TARGETS): eval-%: + $(EVAL_GO_TEST) -ginkgo.focus="$*" + # OCP envtest index for downstream kubebuilder assets ENVTEST_OCP_INDEX := https://raw.githubusercontent.com/openshift/api/master/envtest-releases.yaml # OCP version to Kubernetes version mapping (OCP 4.x -> K8s 1.(x+13)) diff --git a/api/AGENTS.md b/api/AGENTS.md index 9dc4ee7f33e5..a3e692c672d9 100644 --- a/api/AGENTS.md +++ b/api/AGENTS.md @@ -15,17 +15,6 @@ For conventions, always trust the kube-api-linter (`make api-lint-fix`). Do not - Use feature gates for experimental functionality - CRD generation via controller-gen with OpenShift-specific tooling -Key make targets for API work: - -```bash -make api # Regenerate all CRDs, deepcopy, clients -make api-lint-fix # Run API linter and auto-fix violations -make verify-api-deps # Verify API dependencies -make verify # Full verification (includes api, fmt, vet, lint) -make update # Full update (api-deps, workspace-sync, deps, api, api-docs, clients) -ENVTEST_OCP_K8S_VERSIONS=1.35.0 make test-envtest-ocp # Run envtest for CEL validations -``` - ### API Dependencies It is imperative that the imported dependencies are kept minimal. Use `make verify-api-deps` to verify that the dependencies are allowed. @@ -57,6 +46,16 @@ To avoid introducing new dependencies, do not add utils or methods to the API ty ## API Type Change Guidelines +### Best Practices and Patterns + +Use api/karpenter/v1beta1/karpenter_types.go and api/hypershift/v1beta1/etcdbackup_types.go as examples of best practices and patterns. + +Don't use the other existing APIs as examples as they might have many legacy constraints. + +### Field Grouping + +**When multiple fields on a spec share a common prefix or relate to the same feature, they MUST be grouped into a dedicated struct.** Top-level specs like HostedClusterSpec and NodePoolSpec should only contain fields that are independently meaningful. If removing one field would make another field meaningless, they belong together in a sub-struct. A common signal is fields that share a name prefix (e.g., `BarEndpoint`, `BarConfig`, `BarID` all relate to "Bar" and should be a single `Bar` field with a `BarSpec` struct). + ### N-1 and N+1 Compatibility Every change to an API type must be safe for both: @@ -85,3 +84,15 @@ See `api/hypershift/v1beta1/nodepool_types_test.go` for an example of this patte All API CEL validations must be covered with envtests, see test/envtest/README.md for details +#### Key make targets for API work: + +```bash +make api # Regenerate all CRDs, deepcopy, clients +make api-lint-fix # Run API linter and auto-fix violations +make verify-api-deps # Verify API dependencies +make verify # Full verification (includes api, fmt, vet, lint) +make update # Full update (api-deps, workspace-sync, deps, api, api-docs, clients) +ENVTEST_OCP_K8S_VERSIONS=1.35.0 make test-envtest-ocp # Run envtest for CEL validations +``` + +All these must pass for any change before creating a PR diff --git a/test/eval/README.md b/test/eval/README.md new file mode 100644 index 000000000000..9ac0f7338dbb --- /dev/null +++ b/test/eval/README.md @@ -0,0 +1,100 @@ +# Agent & Convention Evals + +Evaluation framework for testing Claude Code agent definitions and +AGENTS.md conventions. Each scenario sends a prompt to an agent (or +base Claude), then uses an LLM judge to check the output against +expected issues. + +## Prerequisites + +- `claude` CLI installed and authenticated +- Go 1.25+ + +## Quick Start + +```bash +# Run all scenarios in parallel +make eval-agents + +# Run a single agent +make eval-api-sme + +# Run with verbose output +make eval-agents EVAL_FOCUS=api-sme EVAL_VERBOSE=1 + +# Multiple runs with pass-rate threshold +make eval-agents EVAL_RUNS=5 EVAL_THRESHOLD=0.6 +``` + +## Environment Variables + +| Variable | Default | Description | +|----------|---------|-------------| +| `EVAL_MODEL` | `claude-opus-4-6` | Model for agent invocation | +| `EVAL_JUDGE_MODEL` | `claude-opus-4-6` | Model for judging | +| `EVAL_RUNS` | `1` | Number of trials per scenario | +| `EVAL_THRESHOLD` | `0.8` | Minimum pass rate (0.0-1.0) | +| `EVAL_FOCUS` | | Ginkgo focus filter (substring match) | +| `EVAL_VERBOSE` | | Set to `1` for verbose agent output | + +## Directory Structure + +``` +test/eval/ + eval_test.go # Test harness + testdata/ + sme-agents/ # Agent scenarios (uses --agent flag) + / + / + prompt.txt # Input prompt + expected.txt # Expected issues, one per line + patch.diff # Optional: applied before run + conventions/ # Convention tests (no agent) + / + prompt.txt + expected.txt +``` + +## Adding a Scenario + +1. Create a directory under `sme-agents//` or `conventions/` +2. Add `prompt.txt` with the input prompt +3. Add `expected.txt` with expected issues, one per line +4. Optionally add `patch.diff` to apply code changes before the run +5. Run it: `make eval-agents EVAL_FOCUS= EVAL_VERBOSE=1` +6. Iterate on `expected.txt` until the pass rate is stable + +The make target is auto-discovered — no Makefile changes needed. + +## How It Works + +1. **Discovery**: scans `testdata/` for scenarios with `prompt.txt` + and `expected.txt` +2. **Patch** (optional): applies `patch.diff` to the repo so agents + can run tools against real code (e.g., `make api-lint-fix`) +3. **Agent invocation**: runs `claude --agent -p ` + with tools enabled if a patch is present, disabled otherwise +4. **Judge**: a separate Claude call checks the agent output against + expected issues using semantic matching +5. **Pass rate**: runs N trials (`EVAL_RUNS`), asserts the pass rate + meets the threshold (`EVAL_THRESHOLD`) +6. **Cleanup**: reverts any patches applied + +## Scenario Types + +### SME Agent Scenarios (`sme-agents/`) + +Test specific agent definitions (`.claude/agents/.md`). The +agent is invoked with `--agent `. Use `patch.diff` to place +code in the repo for the agent to review with its tools. + +### Convention Scenarios (`conventions/`) + +Test that base Claude (no agent) follows AGENTS.md conventions. +Useful for validating that code style rules, naming conventions, +and other repo-wide policies are applied. + +## Cost + +Each scenario costs ~$0.50-2.00 (agent + judge). A full run of all +6 scenarios costs ~$5-15 depending on how much the agent reads. diff --git a/test/eval/eval_test.go b/test/eval/eval_test.go new file mode 100644 index 000000000000..15682eca28e7 --- /dev/null +++ b/test/eval/eval_test.go @@ -0,0 +1,408 @@ +//go:build eval + +package eval + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "testing" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +const ( + claudeTimeout = 10 * time.Minute + testdataDir = "testdata" + promptFile = "prompt.txt" + expectedFile = "expected.txt" + patchFile = "patch.diff" + + sonnetModel = "claude-sonnet-4-6" + opusModel = "claude-opus-4-6" + haikuModel = "claude-haiku-4-5-20251001" + + defaultModel = opusModel + defaultJudgeModel = opusModel + defaultThreshold = 0.8 + + judgePromptTemplate = `You are a judge evaluating an agent output against expected criteria. + +Agent output: +%s + +Expected criteria (one per line): +%s + +Each criterion is a REQUIREMENT that the agent output must satisfy. A criterion can be: +- An issue the agent must identify (e.g., "missing validation markers") +- A behavior the agent must follow (e.g., "uses gomega for assertions") +- A recommendation the agent must make (e.g., "suggests grouping fields into a struct") + +Compare using SEMANTIC matching. A criterion is COVERED only if the agent output actually satisfies it — not merely mentions the topic. For example: +- "uses gomega matchers" is COVERED only if the output actually uses gomega (Expect, BeTrue, etc.), NOT if it mentions gomega while using something else +- "test names use Gherkin syntax" is COVERED only if the output contains test names like "When X it should Y", NOT if it discusses Gherkin while using a different style + +A criterion counts as covered if satisfied ANYWHERE in the output — in code, prose, tables, or examples. It does NOT need to be a standalone finding. Bundling and expanding are OK. + +The output must NOT report issues that are entirely unrelated to any expected criterion. However, expanding on a criterion is OK (e.g., adding MaxLength when the criterion is about validation). + +If the expected criteria list is EMPTY, return pass=true only if the output has no significant problems. + +You MUST respond with ONLY a raw JSON object. Do NOT wrap in markdown code blocks. Do NOT include any other text. +{ + "pass": true or false, + "issues": [ + {"issue": "expected issue text", "covered": true, "reason": "how it was covered"}, + {"issue": "expected issue text", "covered": false, "reason": "why it was not covered"} + ] +} +pass is true only if ALL issues have covered=true AND no entirely unrelated issues were reported.` +) + +type testCase struct { + Agent string + Name string + Prompt string + Patch []byte + ExpectedIssues string +} + +type testCaseResult struct { + Name string + Passed int + Runs int + Rate float64 + Failures []string +} + +type claudeOutput struct { + Type string `json:"type"` + Result string `json:"result"` + TotalCostUSD float64 `json:"total_cost_usd"` +} + +type issueVerdict struct { + Issue string `json:"issue"` + Covered bool `json:"covered"` + Reason string `json:"reason"` +} + +type judgeResult struct { + Pass bool `json:"pass"` + Issues []issueVerdict `json:"issues"` +} + +var ( + repoRoot string + evalModel string + judgeModel string + evalRuns int + evalThreshold float64 + totalAgentCost float64 + totalJudgeCost float64 + allResults []testCaseResult +) + +func TestEval(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Agent Eval Suite") +} + +func envOrDefault(key, defaultVal string) string { + if val, ok := os.LookupEnv(key); ok { + return val + } + return defaultVal +} + +var _ = BeforeSuite(func() { + evalModel = envOrDefault("EVAL_MODEL", defaultModel) + judgeModel = envOrDefault("EVAL_JUDGE_MODEL", defaultJudgeModel) + + var err error + evalRuns, err = strconv.Atoi(envOrDefault("EVAL_RUNS", "1")) + Expect(err).NotTo(HaveOccurred(), "EVAL_RUNS must be an integer") + Expect(evalRuns).To(BeNumerically(">", 0), "EVAL_RUNS must be positive") + + evalThreshold, err = strconv.ParseFloat(envOrDefault("EVAL_THRESHOLD", fmt.Sprintf("%g", defaultThreshold)), 64) + Expect(err).NotTo(HaveOccurred(), "EVAL_THRESHOLD must be a float") + + repoRoot, err = filepath.Abs(filepath.Join("..", "..")) + Expect(err).NotTo(HaveOccurred()) + + By("verifying agents directory exists") + _, err = os.Stat(filepath.Join(repoRoot, ".claude", "agents")) + Expect(err).NotTo(HaveOccurred(), ".claude/agents/ must exist in repository root") +}) + +var _ = AfterSuite(func() { + if len(allResults) > 0 { + fmt.Printf("\n=== Eval Results (threshold: %.0f%%) ===\n\n", evalThreshold*100) + for _, r := range allResults { + status := "PASS" + if r.Rate < evalThreshold { + status = "FAIL" + } + fmt.Printf(" - [%s] %s — %d/%d passed (%.0f%%)\n", status, r.Name, r.Passed, r.Runs, r.Rate*100) + for _, f := range r.Failures { + fmt.Printf(" - %s\n", f) + } + } + fmt.Println() + } + + fmt.Printf("Total Cost: $%.4f (Agent: $%.4f, Judge: $%.4f)\n", + totalAgentCost+totalJudgeCost, totalAgentCost, totalJudgeCost) +}) + +func loadScenario(dir, name, agent string) testCase { + prompt, err := os.ReadFile(filepath.Join(dir, promptFile)) + Expect(err).NotTo(HaveOccurred(), "prompt.txt missing in %s", name) + + expected, err := os.ReadFile(filepath.Join(dir, expectedFile)) + Expect(err).NotTo(HaveOccurred(), "expected.txt missing in %s", name) + + var patch []byte + if data, err := os.ReadFile(filepath.Join(dir, patchFile)); err == nil { + patch = data + } + + return testCase{ + Agent: agent, + Name: name, + Prompt: strings.TrimSpace(string(prompt)), + Patch: patch, + ExpectedIssues: strings.TrimSpace(string(expected)), + } +} + +func discoverTestCases(baseDir string) []testCase { + topDirs, err := os.ReadDir(baseDir) + Expect(err).NotTo(HaveOccurred(), "failed to read testdata directory") + + var cases []testCase + for _, topEntry := range topDirs { + if !topEntry.IsDir() { + continue + } + topName := topEntry.Name() + topPath := filepath.Join(baseDir, topName) + + if topName == "sme-agents" { + // sme-agents/// — three levels, agent from dir name + agentDirs, err := os.ReadDir(topPath) + Expect(err).NotTo(HaveOccurred()) + for _, agentEntry := range agentDirs { + if !agentEntry.IsDir() { + continue + } + agentName := agentEntry.Name() + scenarioDirs, err := os.ReadDir(filepath.Join(topPath, agentName)) + Expect(err).NotTo(HaveOccurred()) + for _, scenarioEntry := range scenarioDirs { + if !scenarioEntry.IsDir() { + continue + } + name := fmt.Sprintf("%s/%s/%s", topName, agentName, scenarioEntry.Name()) + dir := filepath.Join(topPath, agentName, scenarioEntry.Name()) + cases = append(cases, loadScenario(dir, name, agentName)) + } + } + } else { + // // — two levels, no agent + scenarioDirs, err := os.ReadDir(topPath) + Expect(err).NotTo(HaveOccurred()) + for _, scenarioEntry := range scenarioDirs { + if !scenarioEntry.IsDir() { + continue + } + name := fmt.Sprintf("%s/%s", topName, scenarioEntry.Name()) + dir := filepath.Join(topPath, scenarioEntry.Name()) + cases = append(cases, loadScenario(dir, name, "")) + } + } + } + return cases +} + +func createWorktree(patch []byte) string { + By("creating git worktree") + dir, err := os.MkdirTemp("", "eval-worktree-*") + Expect(err).NotTo(HaveOccurred()) + + cmd := exec.Command("git", "worktree", "add", "--detach", dir, "HEAD") + cmd.Dir = repoRoot + output, err := cmd.CombinedOutput() + Expect(err).NotTo(HaveOccurred(), "git worktree add failed: %s", string(output)) + + By("applying patch in worktree") + cmd = exec.Command("git", "apply", "-") + cmd.Dir = dir + cmd.Stdin = bytes.NewReader(patch) + output, err = cmd.CombinedOutput() + Expect(err).NotTo(HaveOccurred(), "git apply failed in worktree: %s", string(output)) + + return dir +} + +func removeWorktree(dir string) { + By("removing git worktree") + cmd := exec.Command("git", "worktree", "remove", "--force", dir) + cmd.Dir = repoRoot + cmd.CombinedOutput() + os.RemoveAll(dir) +} + +func runAgent(tc testCase, model, workDir string) (string, float64) { + By(fmt.Sprintf("running agent %s via Claude (%s)", tc.Agent, model)) + ctx, cancel := context.WithTimeout(context.Background(), claudeTimeout) + defer cancel() + + args := []string{ + "--print", + "--model", model, + "--output-format", "json", + "--no-session-persistence", + "-p", tc.Prompt, + } + + if tc.Agent != "" { + args = append(args, "--agent", tc.Agent) + } + + if tc.Patch != nil { + args = append(args, "--allowed-tools", "Bash,Read,Grep,Glob") + } else { + args = append(args, "--allowed-tools", "Read,Grep,Glob") + } + + cmd := exec.CommandContext(ctx, "claude", args...) + cmd.Dir = workDir + + output, err := cmd.CombinedOutput() + Expect(err).NotTo(HaveOccurred(), "claude command failed: %s", string(output)) + + var parsed claudeOutput + err = json.Unmarshal(output, &parsed) + Expect(err).NotTo(HaveOccurred(), "failed to parse claude output: %s", string(output)) + + totalAgentCost += parsed.TotalCostUSD + return parsed.Result, parsed.TotalCostUSD +} + +func stripMarkdownCodeBlock(s string) string { + s = strings.TrimSpace(s) + s = strings.TrimPrefix(s, "```json") + s = strings.TrimPrefix(s, "```") + s = strings.TrimSuffix(s, "```") + return strings.TrimSpace(s) +} + +func runJudge(model, agentOutput, expectedIssues string) (judgeResult, float64) { + By(fmt.Sprintf("judging output with Claude (%s)", model)) + ctx, cancel := context.WithTimeout(context.Background(), claudeTimeout) + defer cancel() + + prompt := fmt.Sprintf(judgePromptTemplate, agentOutput, expectedIssues) + cmd := exec.CommandContext(ctx, "claude", + "--print", + "--model", model, + "--output-format", "json", + "--no-session-persistence", + "-p", prompt, + ) + cmd.Dir = repoRoot + + output, err := cmd.CombinedOutput() + Expect(err).NotTo(HaveOccurred(), "claude judge command failed: %s", string(output)) + + var parsed claudeOutput + err = json.Unmarshal(output, &parsed) + Expect(err).NotTo(HaveOccurred(), "failed to parse judge output: %s", string(output)) + + totalJudgeCost += parsed.TotalCostUSD + + var result judgeResult + jsonStr := stripMarkdownCodeBlock(parsed.Result) + err = json.Unmarshal([]byte(jsonStr), &result) + Expect(err).NotTo(HaveOccurred(), "failed to parse judge response as JSON: %s", parsed.Result) + return result, parsed.TotalCostUSD +} + +func runTestCase(tc testCase) { + result := testCaseResult{Name: tc.Name, Runs: evalRuns} + + workDir := repoRoot + if tc.Patch != nil { + workDir = createWorktree(tc.Patch) + DeferCleanup(func() { removeWorktree(workDir) }) + } + + for i := range evalRuns { + By(fmt.Sprintf("run %d/%d", i+1, evalRuns)) + + agentOutput, agentCost := runAgent(tc, evalModel, workDir) + + GinkgoWriter.Printf("\n--- Agent Output (run %d/%d) ---\n%s\n--- End Agent Output ---\n\n", + i+1, evalRuns, agentOutput) + + judge, judgeCost := runJudge(judgeModel, agentOutput, tc.ExpectedIssues) + + GinkgoWriter.Printf("Run %d/%d: pass=%v, Agent=$%.4f, Judge=$%.4f\n", + i+1, evalRuns, judge.Pass, agentCost, judgeCost) + for _, iv := range judge.Issues { + status := "COVERED" + if !iv.Covered { + status = "MISSED" + } + GinkgoWriter.Printf(" [%s] %s — %s\n", status, iv.Issue, iv.Reason) + } + + if judge.Pass { + result.Passed++ + } else { + var missed []string + for _, iv := range judge.Issues { + if !iv.Covered { + missed = append(missed, fmt.Sprintf("[MISSED] %s — %s", iv.Issue, iv.Reason)) + } + } + result.Failures = append(result.Failures, fmt.Sprintf("run %d:\n %s", i+1, strings.Join(missed, "\n "))) + } + } + + result.Rate = float64(result.Passed) / float64(result.Runs) + allResults = append(allResults, result) + + GinkgoWriter.Printf("Result: %d/%d passed (%.0f%%), threshold: %.0f%%\n", + result.Passed, result.Runs, result.Rate*100, evalThreshold*100) + + failureList := "" + for _, f := range result.Failures { + failureList += fmt.Sprintf(" - %s\n", f) + } + Expect(result.Rate).To(BeNumerically(">=", evalThreshold), + "pass rate %.0f%% below threshold %.0f%% for %s.\nFailures:\n%s", + result.Rate*100, evalThreshold*100, tc.Name, failureList) +} + +var _ = Describe("Agent Evaluation", func() { + cwd, _ := os.Getwd() + cases := discoverTestCases(filepath.Join(cwd, testdataDir)) + + for _, tc := range cases { + tc := tc + It(tc.Name, func() { + runTestCase(tc) + }) + } +}) diff --git a/test/eval/testdata/conventions/01-go-test-style/expected.txt b/test/eval/testdata/conventions/01-go-test-style/expected.txt new file mode 100644 index 000000000000..abb9d1843e94 --- /dev/null +++ b/test/eval/testdata/conventions/01-go-test-style/expected.txt @@ -0,0 +1,2 @@ +test names use Gherkin syntax with "When... it should..." pattern +uses gomega matchers for assertions (Expect, BeTrue, BeFalse, HaveOccurred, etc.) diff --git a/test/eval/testdata/conventions/01-go-test-style/prompt.txt b/test/eval/testdata/conventions/01-go-test-style/prompt.txt new file mode 100644 index 000000000000..caa95ac8cd85 --- /dev/null +++ b/test/eval/testdata/conventions/01-go-test-style/prompt.txt @@ -0,0 +1,6 @@ +Write a unit test for a function called ParseMaintenanceWindow that +takes a cron string and duration in minutes, and returns a +MaintenanceWindow struct or an error. It should reject empty cron +strings, durations less than 30 minutes, and durations greater than +480 minutes. It should accept valid inputs like "0 2 * * 6" with +duration 120. Just write the test, not the function itself. diff --git a/test/eval/testdata/sme-agents/api-sme/01-api-design-review/expected.txt b/test/eval/testdata/sme-agents/api-sme/01-api-design-review/expected.txt new file mode 100644 index 000000000000..9621daad37e9 --- /dev/null +++ b/test/eval/testdata/sme-agents/api-sme/01-api-design-review/expected.txt @@ -0,0 +1,9 @@ +Foo_IP should use Go PascalCase naming (no underscores) +JSON tags must use lowerCamelCase (not snake_case or PascalCase) +missing omitempty or omitzero on every field +missing IP address format validation (CEL or kubebuilder) +FooConfig should not be a pointer — use value type with omitzero instead +missing +listType marker on slice field for server-side apply +FooID immutability rule is incomplete — self == oldSelf either blocks initial set or allows remove-then-set bypass on optional fields +missing +optional or +required markers on fields +fields sharing a common prefix should be grouped into a dedicated struct rather than scattered on the parent spec diff --git a/test/eval/testdata/sme-agents/api-sme/01-api-design-review/patch.diff b/test/eval/testdata/sme-agents/api-sme/01-api-design-review/patch.diff new file mode 100644 index 000000000000..b0037b3b0e01 --- /dev/null +++ b/test/eval/testdata/sme-agents/api-sme/01-api-design-review/patch.diff @@ -0,0 +1,29 @@ +diff --git a/api/hypershift/v1beta1/hostedcluster_types.go b/api/hypershift/v1beta1/hostedcluster_types.go +index d99f765f09..49ca51d7c3 100644 +--- a/api/hypershift/v1beta1/hostedcluster_types.go ++++ b/api/hypershift/v1beta1/hostedcluster_types.go +@@ -833,6 +833,24 @@ type HostedClusterSpec struct { + // +kubebuilder:default={} + // +kubebuilder:validation:XValidation:rule="self == oldSelf", message="Capabilities is immutable. Changes might result in unpredictable and disruptive behavior." + Capabilities *Capabilities `json:"capabilities,omitempty"` ++ ++ // foo_ip is an IP address. ++ Foo_IP string `json:"foo_ip"` ++ ++ // fooConfig is the foo configuration for the cluster. ++ FooConfig *FooConfig `json:"fooConfig,omitempty"` ++ ++ // fooID is the unique foo identifier for the cluster. ++ // This field is immutable once set. ++ // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="fooID is immutable" ++ // +optional ++ FooID string `json:"fooID,omitempty"` ++} ++ ++// FooConfig foo config. ++type FooConfig struct { ++ // fooDomains is a list of DNS search domains. ++ FooDomains []string `json:"FooDomains,omitempty"` + } + + // OLMCatalogPlacement is an enum specifying the placement of OLM catalog components. diff --git a/test/eval/testdata/sme-agents/api-sme/01-api-design-review/prompt.txt b/test/eval/testdata/sme-agents/api-sme/01-api-design-review/prompt.txt new file mode 100644 index 000000000000..fecff331133a --- /dev/null +++ b/test/eval/testdata/sme-agents/api-sme/01-api-design-review/prompt.txt @@ -0,0 +1,3 @@ +I've added new foo fields to HostedClusterSpec in +api/hypershift/v1beta1/hostedcluster_types.go. The code is already +on disk. Please review the change. diff --git a/test/eval/testdata/sme-agents/cloud-provider-sme/01-kms-integration/expected.txt b/test/eval/testdata/sme-agents/cloud-provider-sme/01-kms-integration/expected.txt new file mode 100644 index 000000000000..accd5f390003 --- /dev/null +++ b/test/eval/testdata/sme-agents/cloud-provider-sme/01-kms-integration/expected.txt @@ -0,0 +1,4 @@ +mentions platform-specific KMS services (AWS KMS and Azure Key Vault) +proposes API-level abstraction for cross-platform KMS configuration +addresses IAM or credential requirements for KMS access +references Kubernetes EncryptionConfiguration or etcd encryption provider mechanism diff --git a/test/eval/testdata/sme-agents/cloud-provider-sme/01-kms-integration/prompt.txt b/test/eval/testdata/sme-agents/cloud-provider-sme/01-kms-integration/prompt.txt new file mode 100644 index 000000000000..c7463c8fc6f2 --- /dev/null +++ b/test/eval/testdata/sme-agents/cloud-provider-sme/01-kms-integration/prompt.txt @@ -0,0 +1,4 @@ +We want to implement customer-managed encryption key support for +etcd data at rest in hosted control planes. The feature should work +across AWS and Azure. How should we design this in HyperShift? +What API changes and controller logic are needed? diff --git a/test/eval/testdata/sme-agents/control-plane-sme/01-ho-cpo-version-skew/expected.txt b/test/eval/testdata/sme-agents/control-plane-sme/01-ho-cpo-version-skew/expected.txt new file mode 100644 index 000000000000..186c696353a4 --- /dev/null +++ b/test/eval/testdata/sme-agents/control-plane-sme/01-ho-cpo-version-skew/expected.txt @@ -0,0 +1,5 @@ +references the cpov2 or controlplane-component framework for deploying the component +version-dependent behavior should be decided in the CPO based on the hosted cluster release version, not in the HO +HO and CPO can run different versions — the HO must not assume which CPO version is running +the CPO image is part of the OCP release payload and matches the hosted cluster version +considers impact on control plane resource footprint (CPU, memory) diff --git a/test/eval/testdata/sme-agents/control-plane-sme/01-ho-cpo-version-skew/prompt.txt b/test/eval/testdata/sme-agents/control-plane-sme/01-ho-cpo-version-skew/prompt.txt new file mode 100644 index 000000000000..762b42835739 --- /dev/null +++ b/test/eval/testdata/sme-agents/control-plane-sme/01-ho-cpo-version-skew/prompt.txt @@ -0,0 +1,13 @@ +We want to add a new control plane component called "policy-engine" +that enforces admission policies on the hosted cluster. The +component needs to behave differently depending on the OCP version +of the hosted control plane — in 4.18+ it should use +ValidatingAdmissionPolicy (native K8s), but in 4.17 and below it +should fall back to a webhook-based approach. + +The HyperShift Operator needs to know which variant to configure +when reconciling the HostedCluster, and the CPO needs to deploy +the right version of the component. + +How should we implement this considering HyperShift's versioning +model and the HO/CPO version skew constraints? diff --git a/test/eval/testdata/sme-agents/data-plane-sme/01-spot-instance-lifecycle/expected.txt b/test/eval/testdata/sme-agents/data-plane-sme/01-spot-instance-lifecycle/expected.txt new file mode 100644 index 000000000000..218f33d188e3 --- /dev/null +++ b/test/eval/testdata/sme-agents/data-plane-sme/01-spot-instance-lifecycle/expected.txt @@ -0,0 +1,4 @@ +discusses NodePool API abstraction for spot across platforms (AWS Spot, Azure Spot VMs, GCP Preemptible/Spot) +addresses instance interruption lifecycle (node drain, workload rescheduling, machine replacement) +considers impact of spot instances on rolling upgrade strategy +references ClusterAPI (CAPI) resources or controllers (MachineSet, MachineDeployment, Machine) diff --git a/test/eval/testdata/sme-agents/data-plane-sme/01-spot-instance-lifecycle/prompt.txt b/test/eval/testdata/sme-agents/data-plane-sme/01-spot-instance-lifecycle/prompt.txt new file mode 100644 index 000000000000..fa34e2770e1b --- /dev/null +++ b/test/eval/testdata/sme-agents/data-plane-sme/01-spot-instance-lifecycle/prompt.txt @@ -0,0 +1,6 @@ +We want to improve spot/preemptible instance support in NodePools. +Currently users can request spot instances on AWS, but we want to +ensure consistent behavior across platforms. How should the NodePool +API and controllers handle instance interruption events, and what +changes are needed for the data plane upgrade flow to account for +spot instance characteristics? diff --git a/test/eval/testdata/sme-agents/hcp-architect-sme/01-architectural-review/expected.txt b/test/eval/testdata/sme-agents/hcp-architect-sme/01-architectural-review/expected.txt new file mode 100644 index 000000000000..4e962d5dbb9f --- /dev/null +++ b/test/eval/testdata/sme-agents/hcp-architect-sme/01-architectural-review/expected.txt @@ -0,0 +1,3 @@ +flags violation of unidirectional communication principle (management to hosted, never reverse) +raises security or tenant isolation concerns +suggests an alternative architecture that respects unidirectional communication diff --git a/test/eval/testdata/sme-agents/hcp-architect-sme/01-architectural-review/prompt.txt b/test/eval/testdata/sme-agents/hcp-architect-sme/01-architectural-review/prompt.txt new file mode 100644 index 000000000000..b29361b0bb4d --- /dev/null +++ b/test/eval/testdata/sme-agents/hcp-architect-sme/01-architectural-review/prompt.txt @@ -0,0 +1,7 @@ +We are considering a design where the hosted cluster's worker +nodes send status updates directly to the hypershift-operator in +the management cluster via a webhook. The worker node would call +a REST endpoint on the hypershift-operator to report node health +metrics. This way we get real-time health data without polling. + +What do you think of this approach?