Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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.<id>` or `eval:<id>`), 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.*
50 changes: 40 additions & 10 deletions pkg/cli/experiments_analyze_statistics.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down Expand Up @@ -97,23 +106,15 @@ 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,
TotalRuns: exp.Total,
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
Expand All @@ -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 {
Comment on lines +131 to +135
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.
Expand Down Expand Up @@ -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.
Expand Down
Loading
Loading