Skip to content

Reconcile experiment variant counts against declared variants before computing balance stats - #58719

Merged
pelikhan merged 4 commits into
mainfrom
copilot/deep-report-fix-variant-reconciliation
Sep 5, 2026
Merged

Reconcile experiment variant counts against declared variants before computing balance stats#58719
pelikhan merged 4 commits into
mainfrom
copilot/deep-report-fix-variant-reconciliation

Conversation

Copilot AI commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Persisted state.Counts[name] keys are never reconciled against a workflow's current experiments.<name>.variants list. When a variant set is renamed (e.g. model identifiers), stale labels from earlier runs stay mixed in with current ones forever, inflating the totals used by the chi-square balance test and skewing it toward "imbalanced" even when the real split is close to even.

Root cause

  • experimentVariantCounts only added missing declared variants with zero counts — it never dropped stale keys no longer in cfg.Variants.
  • The balance test used the raw exp.Total (which includes stale counts) against filtered per-variant counts, so expected vs. observed sums never matched.

Changes

  • pkg/cli/experiments_analyze_statistics.go
    • Added filterDeclaredVariantCounts: drops any variant keys not present in a declared variants list.
    • experimentVariantCounts now always filters stale keys via this helper (previously only added missing declared ones).
    • The chi-square balance test's total is now recomputed from the reconciled counts instead of the raw, possibly stale-inflated exp.Total.
  • pkg/cli/experiments_command.go
    • Added reconcileExperimentDetailsWithConfigs, invoked in RunExperimentsAnalyze, which filters ExperimentDetails.Experiments[*].Variants against the loaded frontmatter configs and recomputes Total — so the displayed variant table and the computed statistics both reflect only currently-declared variants. Stale drops are logged once per experiment.

Example

exp := ExperimentVariantStats{
    Name:     "model_size",
    Variants: map[string]int{"gpt-5.4": 28, "gpt-5.4-mini": 18, "small-agent": 1, "agent": 1},
    Total:    48,
}
cfg := &workflow.ExperimentConfig{Variants: []string{"gpt-5.4", "gpt-5.4-mini"}}

a := computeExperimentAnalysis(exp, cfg, nil, nil)
// Before: p≈0.000 (imbalanced) — stale small-agent/agent counts pollute the total
// After:  28/18 split correctly reported as balanced

Scope note

deriveLastSelectedVariant in audit_report_experiments.go (a legacy fallback heuristic for old state files without a run ledger) was left unchanged — it operates on a single downloaded run's artifact with no access to the workflow's frontmatter config, and plumbing that through would touch many unrelated audit/log call sites disproportionate to this fix.

Copilot AI and others added 3 commits September 5, 2026 04:42
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix stale variant labels in experiment state counts Reconcile experiment variant counts against declared variants before computing balance stats Sep 5, 2026
Copilot AI requested a review from pelikhan September 5, 2026 04:50
@pelikhan
pelikhan marked this pull request as ready for review September 5, 2026 04:50
Copilot AI balanced review requested due to automatic review settings September 5, 2026 04:50
@pelikhan
pelikhan merged commit b1cf3fa into main Sep 5, 2026
34 checks passed
@pelikhan
pelikhan deleted the copilot/deep-report-fix-variant-reconciliation branch September 5, 2026 04:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The analysis still exposes the stale unreconciled total through ExperimentAnalysis.TotalRuns.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Reconciles persisted experiment counts with current variant declarations to prevent stale labels from skewing analysis.

Changes:

  • Filters undeclared variant counts and recomputes totals.
  • Applies reconciliation to displayed details and balance statistics.
  • Adds regression and helper tests.
File summaries
File Description
pkg/cli/experiments_command.go Reconciles loaded experiment details.
pkg/cli/experiments_command_test.go Tests detail reconciliation.
pkg/cli/experiments_analyze_statistics.go Filters counts and recalculates statistical totals.
pkg/cli/experiments_analyze_statistics_test.go Tests stale-count exclusion.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +244 to +248
total := exp.Total
if cfg != nil && len(cfg.Variants) > 0 {
total = sumVariantCounts(variantCounts)
}
a.Variants = buildVariantAnalyses(total, variantCounts, variantNames, expectedPcts, a.MinSamples, graderObservations)
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

🎉 This pull request is included in a new release.

Release: v0.88.5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[deep-report] Experiment state.Counts never reconciled against current variants list — stale labels skew balance/selection stats

3 participants