diff --git a/docs/adr/45834-surface-metric-and-resolve-eval-references-in-experiments-analyze.md b/docs/adr/45834-surface-metric-and-resolve-eval-references-in-experiments-analyze.md new file mode 100644 index 00000000000..b9da35c2488 --- /dev/null +++ b/docs/adr/45834-surface-metric-and-resolve-eval-references-in-experiments-analyze.md @@ -0,0 +1,49 @@ +# ADR-45834: Surface Metric and Resolve Eval References in experiments analyze + +**Date**: 2026-07-15 +**Status**: Proposed +**Deciders**: PR author (@copilot), gh-aw maintainers + +--- + +### Context + +The `experiments analyze` command loads `ExperimentConfig` from workflow frontmatter, which includes a `metric` field that names the primary success metric for an experiment (e.g., `"effective_tokens"` or `"evals.builds"`). This field was parsed but never returned to callers, so users running `experiments analyze` had no way to see which metric an experiment was optimizing for. When the metric is an eval-backed reference (`evals.` or `eval:`), the reference also points to a declared eval question that carries human-readable context (e.g., `"Does the generated code compile?"`), which was likewise silently dropped. Surfacing this data requires threading `EvalsConfig` through the analysis pipeline alongside the existing `ExperimentConfig` map. + +Two operational constraints shaped the implementation. First, the normal first-run experiment state contains only the single selected variant, so the analysis code must preserve config metadata even when it cannot yet compute statistics for two or more observed variants. Second, both local and remote workflow frontmatter loaders need to parse evals consistently so `experiments analyze --repo ...` does not lose metric-question context that is available for local workflows. + +### Decision + +We will introduce an `experimentFrontmatterResult` struct that bundles both `ExperimentConfigs` and `EvalsConfig` returned from frontmatter loaders, thread it through `computeExperimentAnalyses` and `computeExperimentAnalysis`, and resolve eval-backed metric references to their question text at analysis time. Config metadata extraction happens before the "fewer than two observed variants" early return so first-run output still includes the metric and resolved question. Remote frontmatter loading parses evals on the same best-effort basis as local loading, even if no experiment configs are ultimately returned from that file. The resolved `metric` and `metric_question` fields are added to `ExperimentAnalysis` and surfaced in both human-readable and JSON output. Eval resolution is best-effort: a missing `EvalsConfig`, an unrecognized eval ID, or a parse error leaves `MetricQuestion` empty without failing the command. + +### Alternatives Considered + +#### Alternative 1: Eager resolution at parse time + +Resolve the eval question text immediately when parsing the frontmatter config (inside `loadLocalExperimentConfigs` / `loadRemoteExperimentConfigs`), storing the resolved string directly on `ExperimentConfig.Metric` or a new field. This would avoid threading `EvalsConfig` through the analysis pipeline. It was rejected because it conflates parsing with resolution and requires the loader to know about eval lookup semantics; the loader's responsibility is to extract structured data, not to resolve cross-references. Keeping `EvalsConfig` separate preserves the existing separation of concerns and makes it easier to test resolution logic independently. + +#### Alternative 2: Display raw metric string only, no eval resolution + +Add `Metric string` to `ExperimentAnalysis` and populate it from `cfg.Metric`, but skip resolving eval references to question text. Users would see `evals.builds` in the output without knowing what that question asks. This was rejected because the question text is the primary human-readable meaning of an eval-backed metric, and omitting it forces users to look up the question definition manually in the workflow file—exactly the information the output should provide. + +### Consequences + +#### Positive +- `experiments analyze` output now shows the experiment's primary metric, giving users immediate context about what the experiment is measuring. +- When the metric is an eval reference, the resolved question text (`"Does the generated code compile?"`) is displayed alongside the reference (`evals.builds`), making the output self-documenting without requiring users to consult the workflow file. +- Eval resolution is nil-safe and non-fatal; the command remains functional when evals are absent, the ID is unknown, or the evals block fails to parse. +- First-run analyses with a single observed variant still show the metric and resolved question, which gives users useful context before statistical analysis is possible. +- The `metric` and `metric_question` fields appear in JSON output (`--json`), enabling downstream tooling to consume them. + +#### Negative +- `EvalsConfig` must be plumbed through three function call layers (`loadXxxExperimentConfigs` → `computeExperimentAnalyses` → `computeExperimentAnalysis`), increasing the parameter surface of internal functions and requiring all existing test call sites to be updated. +- Metadata extraction now runs even for degenerate single-variant states, so the analysis path does a small amount of extra work before returning early. +- Two new public symbols are exported from `pkg/workflow` (`ParseExperimentMetricEvalReference`, `ParseEvalsFromFrontmatter`), widening the package's API surface and creating a cross-package dependency on eval parsing from the CLI layer. + +#### Neutral +- The `experimentFrontmatterResult` struct is an internal type (unexported) that bundles the two returned values; it replaces a `map[string]*workflow.ExperimentConfig` return type with a struct return type, which is a mechanical but pervasive change across the loader functions and their callers. +- Eval resolution performs a linear scan over `evals.Questions` for each experiment; for typical experiment counts this is negligible, but it is not indexed. + +--- + +*ADR reviewed and completed by @copilot from the generated draft.* diff --git a/pkg/cli/experiments_analyze_statistics.go b/pkg/cli/experiments_analyze_statistics.go index 107ffd4b14c..6b9ca49d149 100644 --- a/pkg/cli/experiments_analyze_statistics.go +++ b/pkg/cli/experiments_analyze_statistics.go @@ -33,6 +33,15 @@ type ExperimentAnalysis struct { // (t_test, mann_whitney, proportion_test, bayesian_ab). AnalysisType string `json:"analysis_type,omitempty"` + // Metric is the primary metric string declared in the experiment config + // (e.g. "effective_tokens" or "evals.builds"). + Metric string `json:"metric,omitempty"` + + // MetricQuestion is the resolved eval question text when Metric references a declared + // eval question ID (e.g. "evals.builds" resolves to "Does the generated code compile?"). + // Empty when Metric is absent or does not reference an eval. + MetricQuestion string `json:"metric_question,omitempty"` + // MinSamples is the minimum runs per variant required before analysis is reliable. // Defaults to 20 when not declared in the experiment config (R-STAT-007). MinSamples int `json:"min_samples"` @@ -97,7 +106,8 @@ type GuardrailStatus struct { // computeExperimentAnalysis computes the statistical analysis for a single named experiment. // cfg may be nil when no workflow frontmatter is available, in which case defaults are used. -func computeExperimentAnalysis(exp ExperimentVariantStats, cfg *workflow.ExperimentConfig) ExperimentAnalysis { +// evals provides the eval definitions for resolving eval-backed metric references; may be nil. +func computeExperimentAnalysis(exp ExperimentVariantStats, cfg *workflow.ExperimentConfig, evals *workflow.EvalsConfig) ExperimentAnalysis { experimentsStatsLog.Printf("Computing analysis for experiment %q: %d variant(s), %d total runs", exp.Name, len(exp.Variants), exp.Total) a := ExperimentAnalysis{ ExperimentName: exp.Name, @@ -105,15 +115,6 @@ func computeExperimentAnalysis(exp ExperimentVariantStats, cfg *workflow.Experim MinSamples: defaultMinSamples, } - // Degenerate: fewer than 2 variants cannot be meaningfully analysed. - if len(exp.Variants) < 2 { - experimentsStatsLog.Printf("Experiment %q has fewer than 2 variants; skipping analysis", exp.Name) - a.IsBalanced = true - a.Recommendation = "EXTEND" - a.Rationale = "experiment has fewer than 2 variants; cannot perform statistical analysis" - return a - } - // Extract metadata from config when available. if cfg != nil { a.Hypothesis = cfg.Hypothesis @@ -127,6 +128,28 @@ func computeExperimentAnalysis(exp ExperimentVariantStats, cfg *workflow.Experim Threshold: g.Threshold, }) } + // Populate metric and resolve any eval reference to its question text. + if cfg.Metric != "" { + a.Metric = cfg.Metric + evalID, isEval := workflow.ParseExperimentMetricEvalReference(cfg.Metric) + if isEval && evalID != "" && evals != nil { + for _, q := range evals.Questions { + if q.ID == evalID { + a.MetricQuestion = q.Question + break + } + } + } + } + } + + // Degenerate: fewer than 2 variants cannot be meaningfully analysed. + if len(exp.Variants) < 2 { + experimentsStatsLog.Printf("Experiment %q has fewer than 2 variants; skipping analysis", exp.Name) + a.IsBalanced = true + a.Recommendation = "EXTEND" + a.Rationale = "experiment has fewer than 2 variants; cannot perform statistical analysis" + return a } // Collect variant names in alphabetical order for deterministic output. @@ -290,6 +313,13 @@ func printOneExperimentAnalysis(a ExperimentAnalysis) { if a.AnalysisType != "" { fmt.Fprintf(os.Stderr, " Test type : %s\n", a.AnalysisType) } + if a.Metric != "" { + if a.MetricQuestion != "" { + fmt.Fprintf(os.Stderr, " Metric : %s — %s\n", a.Metric, a.MetricQuestion) + } else { + fmt.Fprintf(os.Stderr, " Metric : %s\n", a.Metric) + } + } fmt.Fprintf(os.Stderr, " Min samples: %d per variant\n", a.MinSamples) // Per-variant progress table. diff --git a/pkg/cli/experiments_analyze_statistics_test.go b/pkg/cli/experiments_analyze_statistics_test.go index 6a0f7db0100..e3ded98c1d2 100644 --- a/pkg/cli/experiments_analyze_statistics_test.go +++ b/pkg/cli/experiments_analyze_statistics_test.go @@ -180,7 +180,7 @@ func TestComputeExperimentAnalysis(t *testing.T) { Variants: map[string]int{"concise": 5, "detailed": 5}, Total: 10, } - a := computeExperimentAnalysis(exp, nil) + a := computeExperimentAnalysis(exp, nil, nil) assert.Equal(t, "style", a.ExperimentName, "experiment name") assert.Equal(t, defaultMinSamples, a.MinSamples, "default min_samples") @@ -203,7 +203,7 @@ func TestComputeExperimentAnalysis(t *testing.T) { Variants: []string{"formal", "casual"}, MinSamples: 5, } - a := computeExperimentAnalysis(exp, cfg) + a := computeExperimentAnalysis(exp, cfg, nil) assert.Equal(t, 5, a.MinSamples, "min_samples from config") assert.Equal(t, "READY_FOR_ANALYSIS", a.Recommendation, "count >= min_samples → READY") for _, v := range a.Variants { @@ -223,7 +223,7 @@ func TestComputeExperimentAnalysis(t *testing.T) { AnalysisType: "t_test", MinSamples: 20, } - a := computeExperimentAnalysis(exp, cfg) + a := computeExperimentAnalysis(exp, cfg, nil) assert.Equal(t, "H0: no change. H1: short reduces tokens by 15%.", a.Hypothesis, "hypothesis from config") assert.Equal(t, "t_test", a.AnalysisType, "analysis_type from config") assert.Equal(t, "READY_FOR_ANALYSIS", a.Recommendation, "count >= min_samples → READY") @@ -242,7 +242,7 @@ func TestComputeExperimentAnalysis(t *testing.T) { {Name: "empty_output_rate", Threshold: "==0"}, }, } - a := computeExperimentAnalysis(exp, cfg) + a := computeExperimentAnalysis(exp, cfg, nil) require.Len(t, a.Guardrails, 2, "should have two guardrails") assert.Equal(t, "success_rate", a.Guardrails[0].Name, "first guardrail name") assert.Equal(t, ">=0.95", a.Guardrails[0].Threshold, "first guardrail threshold") @@ -255,7 +255,7 @@ func TestComputeExperimentAnalysis(t *testing.T) { Variants: map[string]int{"A": 5, "B": 5, "C": 5}, Total: 15, } - a := computeExperimentAnalysis(exp, nil) + a := computeExperimentAnalysis(exp, nil, nil) assert.Len(t, a.Variants, 3, "three variants") // α_adjusted = 0.05 / (K − 1) = 0.05 / 2 = 0.025 assert.InDelta(t, 0.025, a.BonferroniAlpha, 0.0001, "Bonferroni alpha for K=3") @@ -268,7 +268,7 @@ func TestComputeExperimentAnalysis(t *testing.T) { Variants: map[string]int{"A": 5, "B": 5, "C": 5, "D": 5}, Total: 20, } - a := computeExperimentAnalysis(exp, nil) + a := computeExperimentAnalysis(exp, nil, nil) // α_adjusted = 0.05 / (4 − 1) = 0.05 / 3 ≈ 0.0167 assert.InDelta(t, 0.05/3.0, a.BonferroniAlpha, 0.0001, "Bonferroni alpha for K=4") }) @@ -280,7 +280,7 @@ func TestComputeExperimentAnalysis(t *testing.T) { Variants: map[string]int{"A": 19, "B": 1}, Total: 20, } - a := computeExperimentAnalysis(exp, nil) + a := computeExperimentAnalysis(exp, nil, nil) assert.False(t, a.IsBalanced, "extreme imbalance should not be balanced") assert.Less(t, a.PValue, balanceSignificanceThreshold, "p < 0.05 for extreme imbalance") }) @@ -291,7 +291,7 @@ func TestComputeExperimentAnalysis(t *testing.T) { Variants: map[string]int{"A": 0, "B": 0}, Total: 0, } - a := computeExperimentAnalysis(exp, nil) + a := computeExperimentAnalysis(exp, nil, nil) assert.Equal(t, "EXTEND", a.Recommendation, "zero runs → EXTEND") assert.True(t, a.IsBalanced, "insufficient data → default to balanced") assert.InDelta(t, 0.0, a.ChiSquare, 1e-10, "no chi-square for zero total") @@ -303,7 +303,7 @@ func TestComputeExperimentAnalysis(t *testing.T) { Variants: map[string]int{"z_last": 3, "a_first": 7}, Total: 10, } - a := computeExperimentAnalysis(exp, nil) + a := computeExperimentAnalysis(exp, nil, nil) require.Len(t, a.Variants, 2, "two variants") assert.Equal(t, "a_first", a.Variants[0].Name, "first variant alphabetically") assert.Equal(t, "z_last", a.Variants[1].Name, "second variant alphabetically") @@ -320,19 +320,131 @@ func TestComputeExperimentAnalysis(t *testing.T) { Variants: []string{"control", "variant"}, Weight: []int{70, 30}, } - a := computeExperimentAnalysis(exp, cfg) + a := computeExperimentAnalysis(exp, cfg, nil) assert.True(t, a.IsBalanced, "70/30 split with 70/30 weights should be balanced") // Expected proportions: control=0.7, variant=0.3 require.Len(t, a.Variants, 2, "two variants") assert.InDelta(t, 70.0, a.Variants[0].ExpectedPct, 0.1, "control expected 70%") assert.InDelta(t, 30.0, a.Variants[1].ExpectedPct, 0.1, "variant expected 30%") }) + + t.Run("metric from config (plain)", func(t *testing.T) { + exp := ExperimentVariantStats{ + Name: "perf", + Variants: map[string]int{"A": 5, "B": 5}, + Total: 10, + } + cfg := &workflow.ExperimentConfig{ + Variants: []string{"A", "B"}, + Metric: "effective_tokens", + } + a := computeExperimentAnalysis(exp, cfg, nil) + assert.Equal(t, "effective_tokens", a.Metric, "metric should be set") + assert.Empty(t, a.MetricQuestion, "MetricQuestion empty for plain metric") + }) + + t.Run("metric resolves eval question when evals provided", func(t *testing.T) { + exp := ExperimentVariantStats{ + Name: "quality", + Variants: map[string]int{"A": 5, "B": 5}, + Total: 10, + } + cfg := &workflow.ExperimentConfig{ + Variants: []string{"A", "B"}, + Metric: "evals.builds", + } + evals := &workflow.EvalsConfig{ + Questions: []workflow.EvalDefinition{ + {ID: "builds", Question: "Does the generated code compile?"}, + {ID: "tests", Question: "Do the tests pass?"}, + }, + } + a := computeExperimentAnalysis(exp, cfg, evals) + assert.Equal(t, "evals.builds", a.Metric, "metric set to original reference") + assert.Equal(t, "Does the generated code compile?", a.MetricQuestion, "eval question resolved") + }) + + t.Run("metric eval reference with eval: prefix resolves question", func(t *testing.T) { + exp := ExperimentVariantStats{ + Name: "coverage", + Variants: map[string]int{"A": 5, "B": 5}, + Total: 10, + } + cfg := &workflow.ExperimentConfig{ + Variants: []string{"A", "B"}, + Metric: "eval:tests", + } + evals := &workflow.EvalsConfig{ + Questions: []workflow.EvalDefinition{ + {ID: "tests", Question: "Do the tests pass?"}, + }, + } + a := computeExperimentAnalysis(exp, cfg, evals) + assert.Equal(t, "eval:tests", a.Metric, "metric set to original reference") + assert.Equal(t, "Do the tests pass?", a.MetricQuestion, "eval question resolved via eval: prefix") + }) + + t.Run("eval reference with unknown id leaves MetricQuestion empty", func(t *testing.T) { + exp := ExperimentVariantStats{ + Name: "x", + Variants: map[string]int{"A": 5, "B": 5}, + Total: 10, + } + cfg := &workflow.ExperimentConfig{ + Variants: []string{"A", "B"}, + Metric: "evals.unknown_id", + } + evals := &workflow.EvalsConfig{ + Questions: []workflow.EvalDefinition{ + {ID: "builds", Question: "Does the generated code compile?"}, + }, + } + a := computeExperimentAnalysis(exp, cfg, evals) + assert.Equal(t, "evals.unknown_id", a.Metric, "metric preserved") + assert.Empty(t, a.MetricQuestion, "MetricQuestion empty when eval id not found") + }) + + t.Run("eval reference with nil evals leaves MetricQuestion empty", func(t *testing.T) { + exp := ExperimentVariantStats{ + Name: "y", + Variants: map[string]int{"A": 5, "B": 5}, + Total: 10, + } + cfg := &workflow.ExperimentConfig{ + Variants: []string{"A", "B"}, + Metric: "evals.builds", + } + a := computeExperimentAnalysis(exp, cfg, nil) + assert.Equal(t, "evals.builds", a.Metric, "metric preserved") + assert.Empty(t, a.MetricQuestion, "MetricQuestion empty when evals is nil") + }) + + t.Run("degenerate experiment still includes metric metadata", func(t *testing.T) { + exp := ExperimentVariantStats{ + Name: "first_run", + Variants: map[string]int{"A": 1}, + Total: 1, + } + cfg := &workflow.ExperimentConfig{ + Variants: []string{"A", "B"}, + Metric: "evals.builds", + } + evals := &workflow.EvalsConfig{ + Questions: []workflow.EvalDefinition{ + {ID: "builds", Question: "Does the generated code compile?"}, + }, + } + a := computeExperimentAnalysis(exp, cfg, evals) + assert.Equal(t, "evals.builds", a.Metric, "metric preserved for single-variant state") + assert.Equal(t, "Does the generated code compile?", a.MetricQuestion, "metric question resolved before degenerate return") + assert.Equal(t, "EXTEND", a.Recommendation, "single-variant state still extends") + }) } // TestComputeExperimentAnalyses tests the bulk analysis function. func TestComputeExperimentAnalyses(t *testing.T) { t.Run("empty experiments returns nil", func(t *testing.T) { - result := computeExperimentAnalyses(nil, nil) + result := computeExperimentAnalyses(nil, nil, nil) assert.Nil(t, result, "nil experiments should return nil") }) @@ -341,7 +453,7 @@ func TestComputeExperimentAnalyses(t *testing.T) { {Name: "exp1", Variants: map[string]int{"A": 5, "B": 5}, Total: 10}, {Name: "exp2", Variants: map[string]int{"X": 3, "Y": 7}, Total: 10}, } - analyses := computeExperimentAnalyses(experiments, nil) + analyses := computeExperimentAnalyses(experiments, nil, nil) require.Len(t, analyses, 2, "should produce one analysis per experiment") assert.Equal(t, "exp1", analyses[0].ExperimentName, "first analysis name") assert.Equal(t, "exp2", analyses[1].ExperimentName, "second analysis name") @@ -359,7 +471,7 @@ func TestComputeExperimentAnalyses(t *testing.T) { MinSamples: 20, }, } - analyses := computeExperimentAnalyses(experiments, configs) + analyses := computeExperimentAnalyses(experiments, configs, nil) require.Len(t, analyses, 1, "one analysis") assert.Equal(t, "test hypothesis", analyses[0].Hypothesis, "hypothesis from config") assert.Equal(t, "proportion_test", analyses[0].AnalysisType, "analysis type from config") @@ -384,7 +496,7 @@ func TestExperimentAnalysisJSONOutput(t *testing.T) { }, } - a := computeExperimentAnalysis(exp, cfg) + a := computeExperimentAnalysis(exp, cfg, nil) jsonBytes, err := json.MarshalIndent(a, "", " ") require.NoError(t, err, "should marshal analysis to JSON") @@ -413,7 +525,7 @@ func TestExperimentAnalysisBonferroniAbsent(t *testing.T) { Variants: map[string]int{"yes": 10, "no": 10}, Total: 20, } - a := computeExperimentAnalysis(exp, nil) + a := computeExperimentAnalysis(exp, nil, nil) assert.InDelta(t, 0.0, a.BonferroniAlpha, 1e-10, "BonferroniAlpha should be zero for K=2") jsonBytes, err := json.MarshalIndent(a, "", " ") @@ -432,7 +544,7 @@ func TestMinSamplesDefaultApplied(t *testing.T) { Variants: map[string]int{"A": 10, "B": 10}, Total: 20, } - a := computeExperimentAnalysis(exp, nil) + a := computeExperimentAnalysis(exp, nil, nil) assert.Equal(t, defaultMinSamples, a.MinSamples, "default min_samples should be 20") } @@ -469,7 +581,7 @@ func TestObservedPctSumsToHundred(t *testing.T) { Variants: map[string]int{"A": 7, "B": 13}, Total: 20, } - a := computeExperimentAnalysis(exp, nil) + a := computeExperimentAnalysis(exp, nil, nil) total := 0.0 for _, v := range a.Variants { total += v.ObservedPct @@ -499,7 +611,7 @@ func TestExpectedPctSumsToHundred(t *testing.T) { total += c } exp := ExperimentVariantStats{Name: "e", Variants: tt.variants, Total: total} - a := computeExperimentAnalysis(exp, tt.cfg) + a := computeExperimentAnalysis(exp, tt.cfg, nil) sum := 0.0 for _, v := range a.Variants { sum += v.ExpectedPct @@ -520,7 +632,7 @@ func TestReadyForAnalysisAllAboveMinSamples(t *testing.T) { Variants: []string{"X", "Y"}, MinSamples: 20, } - a := computeExperimentAnalysis(exp, cfg) + a := computeExperimentAnalysis(exp, cfg, nil) assert.Equal(t, "READY_FOR_ANALYSIS", a.Recommendation, "all variants above min_samples → READY") assert.Contains(t, a.Rationale, "min_samples", "rationale should mention min_samples") @@ -540,7 +652,7 @@ func TestPartiallyBelowMinSamples(t *testing.T) { Variants: []string{"above", "below"}, MinSamples: 20, } - a := computeExperimentAnalysis(exp, cfg) + a := computeExperimentAnalysis(exp, cfg, nil) assert.Equal(t, "EXTEND", a.Recommendation, "one variant below threshold → EXTEND") assert.Contains(t, a.Rationale, "1 of 2", "rationale should count variants below threshold") @@ -559,7 +671,7 @@ func TestChiSquarePerfectBalance(t *testing.T) { Variants: map[string]int{"A": 10, "B": 10, "C": 10}, Total: 30, } - a := computeExperimentAnalysis(exp, nil) + a := computeExperimentAnalysis(exp, nil, nil) assert.InDelta(t, 0.0, a.ChiSquare, 1e-10, "chi² should be 0 for perfectly balanced sample") assert.InDelta(t, 1.0, a.PValue, 1e-10, "p should be 1.0 for chi²=0") assert.True(t, a.IsBalanced, "perfectly balanced → is_balanced") @@ -642,7 +754,7 @@ func TestAnalysisWithNilConfig(t *testing.T) { Variants: map[string]int{"on": 8, "off": 12}, Total: 20, } - a := computeExperimentAnalysis(exp, nil) + a := computeExperimentAnalysis(exp, nil, nil) assert.Equal(t, "no_config", a.ExperimentName, "experiment name preserved") assert.Empty(t, a.Hypothesis, "no hypothesis without config") assert.Empty(t, a.AnalysisType, "no analysis type without config") @@ -663,7 +775,7 @@ func TestComputeExperimentAnalysisDegenerateVariants(t *testing.T) { Variants: map[string]int{}, Total: 0, } - a := computeExperimentAnalysis(exp, nil) + a := computeExperimentAnalysis(exp, nil, nil) assert.Equal(t, "EXTEND", a.Recommendation, "zero variants → EXTEND") assert.True(t, a.IsBalanced, "degenerate case defaults to balanced") assert.Empty(t, a.Variants, "no variant entries") @@ -676,7 +788,7 @@ func TestComputeExperimentAnalysisDegenerateVariants(t *testing.T) { Variants: map[string]int{"only": 10}, Total: 10, } - a := computeExperimentAnalysis(exp, nil) + a := computeExperimentAnalysis(exp, nil, nil) assert.Equal(t, "EXTEND", a.Recommendation, "single variant → EXTEND") assert.True(t, a.IsBalanced, "degenerate case defaults to balanced") assert.Empty(t, a.Variants, "no variant entries emitted for degenerate case") diff --git a/pkg/cli/experiments_command.go b/pkg/cli/experiments_command.go index cc14d633f68..55ae47a1d49 100644 --- a/pkg/cli/experiments_command.go +++ b/pkg/cli/experiments_command.go @@ -235,19 +235,20 @@ func RunExperimentsAnalyze(config ExperimentsAnalyzeConfig) error { branchName := experimentsBranchPrefix + config.ExperimentName - // Load experiment configs from the workflow frontmatter to enrich the statistical output - // with hypothesis text, analysis_type, min_samples, and guardrail thresholds. + // Load experiment configs and evals from the workflow frontmatter to enrich the statistical + // output with hypothesis text, analysis_type, min_samples, guardrail thresholds, and resolved + // eval metric questions. // Config loading is best-effort: failures are silently ignored and analysis falls back to // defaults (min_samples=20, equal expected proportions, no hypothesis displayed). // This ensures the command remains functional even when the workflow .md file is absent // (e.g., when analysing experiments from a remote repository without the workflow checked out). - var experimentConfigs map[string]*workflow.ExperimentConfig + var frontmatterResult experimentFrontmatterResult if config.RepoOverride != "" { - experimentConfigs = loadRemoteExperimentConfigs(config.RepoOverride, config.ExperimentName) + frontmatterResult = loadRemoteExperimentConfigs(config.RepoOverride, config.ExperimentName) } else { - experimentConfigs = loadLocalExperimentConfigs(config.ExperimentName) + frontmatterResult = loadLocalExperimentConfigs(config.ExperimentName) } - experimentsLog.Printf("Loaded %d experiment config(s) for %s", len(experimentConfigs), config.ExperimentName) + experimentsLog.Printf("Loaded %d experiment config(s) for %s", len(frontmatterResult.ExperimentConfigs), config.ExperimentName) var details *ExperimentDetails var err error @@ -264,7 +265,7 @@ func RunExperimentsAnalyze(config ExperimentsAnalyzeConfig) error { } // Compute statistical analyses for each named experiment. - details.Analyses = computeExperimentAnalyses(details.Experiments, experimentConfigs) + details.Analyses = computeExperimentAnalyses(details.Experiments, frontmatterResult.ExperimentConfigs, frontmatterResult.Evals) if config.JSONOutput { jsonBytes, err := json.MarshalIndent(details, "", " ") @@ -281,7 +282,8 @@ func RunExperimentsAnalyze(config ExperimentsAnalyzeConfig) error { // computeExperimentAnalyses computes statistical analyses for all named experiments. // configs maps experiment names to their configuration; values may be nil. -func computeExperimentAnalyses(experiments []ExperimentVariantStats, configs map[string]*workflow.ExperimentConfig) []ExperimentAnalysis { +// evals provides the eval definitions for resolving eval-backed metric references; may be nil. +func computeExperimentAnalyses(experiments []ExperimentVariantStats, configs map[string]*workflow.ExperimentConfig, evals *workflow.EvalsConfig) []ExperimentAnalysis { if len(experiments) == 0 { return nil } @@ -291,22 +293,29 @@ func computeExperimentAnalyses(experiments []ExperimentVariantStats, configs map if configs != nil { cfg = configs[exp.Name] } - analyses = append(analyses, computeExperimentAnalysis(exp, cfg)) + analyses = append(analyses, computeExperimentAnalysis(exp, cfg, evals)) } return analyses } +// experimentFrontmatterResult holds both the experiment configs and evals config parsed +// from a workflow's frontmatter. +type experimentFrontmatterResult struct { + ExperimentConfigs map[string]*workflow.ExperimentConfig + Evals *workflow.EvalsConfig +} + // loadLocalExperimentConfigs reads the workflow .md file for the given experiment name -// and returns the ExperimentConfig map from its frontmatter. +// and returns the ExperimentConfig map and EvalsConfig from its frontmatter. // experimentName is the sanitized workflow ID (the part after "experiments/" in the branch name). -// Returns nil when the workflow file cannot be found or parsed. -func loadLocalExperimentConfigs(experimentName string) map[string]*workflow.ExperimentConfig { +// Returns a zero-value result when the workflow file cannot be found or parsed. +func loadLocalExperimentConfigs(experimentName string) experimentFrontmatterResult { experimentsLog.Printf("Loading local experiment configs for %s", experimentName) filePath := findWorkflowFileForExperiment(experimentName) if filePath == "" { experimentsLog.Printf("No workflow file found for experiment %s", experimentName) - return nil + return experimentFrontmatterResult{} } // Verify that the resolved path is within .github/workflows/ to prevent path traversal. @@ -315,43 +324,52 @@ func loadLocalExperimentConfigs(experimentName string) map[string]*workflow.Expe absFilePath, err := filepath.Abs(filePath) if err != nil { experimentsLog.Printf("Failed to resolve absolute path for %s: %v", filePath, err) - return nil + return experimentFrontmatterResult{} } workflowsDir, err := filepath.Abs(getWorkflowsDir()) if err != nil { experimentsLog.Printf("Failed to resolve workflows dir: %v", err) - return nil + return experimentFrontmatterResult{} } if !strings.HasPrefix(absFilePath, workflowsDir+string(filepath.Separator)) { experimentsLog.Printf("Refusing to read workflow file outside .github/workflows/: %s", absFilePath) - return nil + return experimentFrontmatterResult{} } content, err := os.ReadFile(absFilePath) // #nosec G304 — path confirmed within .github/workflows/ if err != nil { experimentsLog.Printf("Failed to read workflow file %s: %v", absFilePath, err) - return nil + return experimentFrontmatterResult{} } result, err := parser.ExtractFrontmatterFromContent(string(content)) if err != nil { experimentsLog.Printf("Failed to parse frontmatter from %s: %v", filePath, err) - return nil + return experimentFrontmatterResult{} } cfg, err := workflow.ParseFrontmatterConfig(result.Frontmatter) if err != nil { experimentsLog.Printf("Failed to parse frontmatter config from %s: %v", filePath, err) - return nil + return experimentFrontmatterResult{} } - return cfg.ExperimentConfigs + evals, err := workflow.ParseEvalsFromFrontmatter(result.Frontmatter) + if err != nil { + experimentsLog.Printf("Failed to parse evals config from %s: %v", filePath, err) + // Non-fatal: continue without evals resolution. + } + + return experimentFrontmatterResult{ + ExperimentConfigs: cfg.ExperimentConfigs, + Evals: evals, + } } // loadRemoteExperimentConfigs fetches the workflow .md file from the repository default branch -// via the GitHub API and returns the ExperimentConfig map from its frontmatter. -// Returns nil when the file cannot be fetched or parsed. -func loadRemoteExperimentConfigs(repoOverride, experimentName string) map[string]*workflow.ExperimentConfig { +// via the GitHub API and returns the ExperimentConfig map and EvalsConfig from its frontmatter. +// Returns a zero-value result when the file cannot be fetched or parsed. +func loadRemoteExperimentConfigs(repoOverride, experimentName string) experimentFrontmatterResult { experimentsLog.Printf("Loading remote experiment configs for %s from %s", experimentName, repoOverride) // Build the candidate list. First, use the directory listing to find the exact filename @@ -394,14 +412,23 @@ func loadRemoteExperimentConfigs(repoOverride, experimentName string) map[string continue } + evals, err := workflow.ParseEvalsFromFrontmatter(result.Frontmatter) + if err != nil { + experimentsLog.Printf("Failed to parse evals config from %s: %v", apiPath, err) + // Non-fatal: continue without evals resolution. + } + if len(cfg.ExperimentConfigs) > 0 { experimentsLog.Printf("Loaded remote configs from %s", apiPath) - return cfg.ExperimentConfigs + return experimentFrontmatterResult{ + ExperimentConfigs: cfg.ExperimentConfigs, + Evals: evals, + } } } experimentsLog.Printf("No remote workflow file found for experiment %s", experimentName) - return nil + return experimentFrontmatterResult{} } // findRemoteWorkflowFilenameForExperiment lists .md files in .github/workflows/ via the diff --git a/pkg/workflow/compiler_experiments.go b/pkg/workflow/compiler_experiments.go index 2ee6acea5b0..d7bab265999 100644 --- a/pkg/workflow/compiler_experiments.go +++ b/pkg/workflow/compiler_experiments.go @@ -322,13 +322,13 @@ func extractIntSlice(raw any) []int { return nil } -// parseExperimentMetricEvalReference returns the referenced eval question ID when metric +// ParseExperimentMetricEvalReference returns the referenced eval question ID when metric // declares an eval-backed success metric. // Supported forms: // - eval: // - evals. // - evals.. (suffix reserved for future derived metrics) -func parseExperimentMetricEvalReference(metric string) (string, bool) { +func ParseExperimentMetricEvalReference(metric string) (string, bool) { trimmed := strings.TrimSpace(metric) if trimmed == "" { return "", false @@ -367,7 +367,7 @@ func validateExperimentMetricReferences(configs map[string]*ExperimentConfig, ev if cfg == nil { continue } - referencedEvalID, referencesEval := parseExperimentMetricEvalReference(cfg.Metric) + referencedEvalID, referencesEval := ParseExperimentMetricEvalReference(cfg.Metric) if !referencesEval { continue } diff --git a/pkg/workflow/compiler_experiments_test.go b/pkg/workflow/compiler_experiments_test.go index 697e45681b5..ba7329e3b39 100644 --- a/pkg/workflow/compiler_experiments_test.go +++ b/pkg/workflow/compiler_experiments_test.go @@ -526,7 +526,7 @@ func TestParseExperimentMetricEvalReference(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - gotID, gotMatch := parseExperimentMetricEvalReference(tt.metric) + gotID, gotMatch := ParseExperimentMetricEvalReference(tt.metric) assert.Equal(t, tt.wantID, gotID) assert.Equal(t, tt.wantMatch, gotMatch) }) diff --git a/pkg/workflow/evals_config.go b/pkg/workflow/evals_config.go index d26dbde8440..5e4a87732d8 100644 --- a/pkg/workflow/evals_config.go +++ b/pkg/workflow/evals_config.go @@ -196,3 +196,11 @@ func validateEvals(cfg *EvalsConfig) error { } return nil } + +// ParseEvalsFromFrontmatter extracts and validates the evals configuration from the +// raw frontmatter map. Returns nil when the evals field is absent or invalid. +// This is a public standalone convenience wrapper around the compiler method. +func ParseEvalsFromFrontmatter(frontmatter map[string]any) (*EvalsConfig, error) { + var c Compiler + return c.parseEvalsFromFrontmatter(frontmatter) +}