Reject ambiguous eval identities

Fail closed on missing or duplicate comparison slots, reject duplicate stimulus names before execution, and clarify how the practical floor changes power at its boundary.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 1c6716a2-172b-461a-adb5-127b1ba7e96b

.agents/skills/create-skill-test/SKILL.md

@@ -127,6 +127,8 @@ own value rather than defaulting it.
   cued prompts inflate the overfit score and bias the baseline.
 - Each stimulus should discriminate a **different** property of the skill. Five stimuli covering one
   property give arithmetic, not evidence.
+- Give every stimulus a stable, unique `name`. Vally pairs comparison trajectories by
+  `(stimulus name, trial index)`; duplicate names make slot identity ambiguous.
 - Include a boundary / no-op stimulus for any skill that migrates or rewrites code, proving it
   leaves already-correct input alone.
 
@@ -299,10 +301,11 @@ EXPERIMENT_FILE=my-agent.experiment.yaml ./eng/run-skill-evals.sh <plugin>
 
 Read the trajectories rather than the verdict — there is no sign-test result for an agent eval.
 
-`check_eval_quality.py` blocks ten structural defect classes that each already cost a real result:
+`check_eval_quality.py` blocks eleven structural defect classes that can corrupt a result:
 missing or untracked fixtures, self-contradicting coverage fixtures, empty grader configs, dormancy
-guards with `reject_skills`, sub-floor stimulus counts, duplicate YAML keys, and `config:`/`defaults:`
-collisions. Do not add a new eval to `eng/eval-quality/underpowered-allowlist.txt` — the gate rejects
+guards with `reject_skills`, sub-floor stimulus counts, duplicate YAML keys or stimulus names, and
+`config:`/`defaults:` collisions. Do not add a new eval to
+`eng/eval-quality/underpowered-allowlist.txt` — the gate rejects
 allowlist entries that are new relative to the base branch.
 
 For the official run, submit a PR review containing `/evaluate` so it binds to the reviewed commit.
@@ -312,7 +315,7 @@ For the official run, submit a PR review containing `/evaluate` so it binds to t
 - [ ] Directory is `tests/<plugin>/<skill-name>/` or `tests/<plugin>/agent.<agent-name>/`
 - [ ] Spec uses `stimuli:` / `graders:`, and exactly one of `defaults:` or `config:`
 - [ ] For a skill eval, at least 5 distinct stimuli exist, with more for the effect and tie rate that must be detected (agent evals are exempt)
-- [ ] Each stimulus discriminates a different property
+- [ ] Each stimulus discriminates a different property and has a stable, unique name
 - [ ] Prompts never name the skill, the agent, or its vocabulary
 - [ ] Every referenced fixture exists and is tracked by `git ls-files`
 - [ ] Every fixture behaves as its stimulus assumes — healthy ones build, deliberately broken ones fail only for the stated reason
@@ -338,6 +341,7 @@ For the official run, submit a PR review containing `/evaluate` so it binds to t
 | `expect_tools: [bash]` on an advisory question | Drop it; it causes timeouts, not quality |
 | Timeout too short for code generation | Use ~360s; empty output fails every grader |
 | Duplicate YAML key left behind by an edit | It overwrites the next stimulus field by field — delete the stray block |
+| Duplicate stimulus names | Vally uses names as comparison identity — give every stimulus a stable, unique name |
 | Direct activation-graded eval for a `disable-model-invocation: true` skill | Cover it through a consumer skill, or grade the answer content as `filter-syntax` does |
 | Agent eval sized for the stimulus floor | `agent.*` evals get no verdict; size them for scenario coverage instead |
 | Agent eval "run" with `./eng/run-skill-evals.sh` | The glob drops it — use a widened `EXPERIMENT_FILE` |

.agents/skills/improve-skill-quality/SKILL.md

@@ -90,7 +90,7 @@ See [references/eval-triage.md](references/eval-triage.md) for the full catalogu
 
 ### Step 4: Verify the fixtures before touching the skill
 
-Run `python eng/eval-quality/check_eval_quality.py` — it blocks ten defect classes that each already
+Run `python eng/eval-quality/check_eval_quality.py` — it blocks eleven defect classes that can
 cost a real result here. Then confirm by hand:
 
 - every fixture behaves as its stimulus assumes — a fixture meant to be healthy builds, and one
@@ -204,5 +204,5 @@ result, confirm the skill payload actually changed — reruns on byte-identical
 
 - [references/writing-for-baseline-delta.md](references/writing-for-baseline-delta.md) — content patterns that beat the unskilled model
 - [references/eval-triage.md](references/eval-triage.md) — symptom, cause and fix catalogue with PR citations
-- [eng/eval-quality/README.md](../../../eng/eval-quality/README.md) — the ten structural gate checks and why each exists
+- [eng/eval-quality/README.md](../../../eng/eval-quality/README.md) — the eleven structural gate checks and why each exists
 - [eng/vally-adapter/InvestigatingResults.md](../../../eng/vally-adapter/InvestigatingResults.md) — downloading artifacts and reading `results.json`. This is the current guide; the similarly-named `eng/skill-validator/src/docs/InvestigatingResults.md` documents the retired `skill-validator evaluate` schema and does not describe today's results.

AGENTS.md

@@ -13,9 +13,9 @@ Use the repository's own authoring skills under `.agents/skills/` instead of imp
   Classify the failure before editing skill content; broken fixtures, underpowered trial counts and
   harness errors routinely masquerade as skill regressions.
 
-Before pushing eval changes, run `python eng/eval-quality/check_eval_quality.py`. It blocks ten
-structural defect classes documented in `eng/eval-quality/README.md`, each of which has already cost
-a real evaluation result here.
+Before pushing eval changes, run `python eng/eval-quality/check_eval_quality.py`. It blocks eleven
+structural defect classes documented in `eng/eval-quality/README.md` that can corrupt a real
+evaluation result.
 
 The distilled quality rules — what makes a skill beat its own baseline — live in the "Quality bar"
 section of `CONTRIBUTING.md`.

eng/eval-quality/README.md

@@ -1,10 +1,8 @@
 # Eval quality gate
 
-`check_eval_quality.py` blocks defect classes that have each already cost a real
-evaluation result on this repo. Every one of them was invisible to the existing
-checks: the eval specs parsed, `skill-validator` passed, and the damage only
-showed up as a skill mysteriously losing to its own baseline — or as a skill
-winning every trial and failing anyway.
+`check_eval_quality.py` blocks structural defects that can corrupt an eval
+result. Most were first found only after an eval mysteriously lost to its own
+baseline or won every trial and still failed.
 
 Run it from the repository root:
 
@@ -16,7 +14,7 @@ python eng/eval-quality/selftest_eval_quality.py       # prove the gate still fi
 
 ## Failing checks
 
-All ten are **structural** — they inspect file existence, git state, declared
+All eleven are **structural** — they inspect file existence, git state, declared
 numbers, or YAML shape/keys. None of them interprets prose, so they cannot fire
 spuriously on a well-written eval.
 
@@ -198,6 +196,12 @@ stimuli because they do not enter the test. Eight stimuli are enough for 80%
 power only for a near-deterministic 90% conditional win rate. A non-pass is not
 proof of no effect.
 
+The table gives **sign-test power**, before the 20% practical floor is applied.
+At a true 60% conditional win rate, the floor is exactly at the expected effect:
+with 158 votes the sign test has 80.6% power, but the combined gate passes about
+52.2% of records and approaches 50% as the sample grows. The gate is designed to
+certify effects above its practical threshold, not effects that only equal it.
+
 Repeated runs still matter. Keep Vally's recommended run counts where the cost
 allows, and read `comparisonTrialEvidence` plus per-stimulus run W/T/L for
 reliability. Do not use those runs to clear the distinct-stimulus floor.
@@ -272,6 +276,14 @@ So the one actionable signal points away from the cause, and the suggested fix
 re-runs a spec that can never load. Replace `config:` with one `defaults:` block
 that carries all settings.
 
+### 11. Duplicate stimulus names
+
+Vally pairs baseline and treatment trajectories by `(stimulus name, trial
+index)`. Two stimuli with the same name therefore create ambiguous comparison
+slots even when their prompts differ. The authoring gate requires every
+stimulus name in one eval to be unique; the runtime adapter also rejects missing
+or duplicate comparison slot identities.
+
 ## Why the gate scores direction, not magnitude
 
 Worth recording, because the check above is only half of what went wrong.

eng/eval-quality/check_eval_quality.py

@@ -1,8 +1,8 @@
 #!/usr/bin/env python3
 """Eval quality gate.
 
-Codifies defect classes that have each cost a real evaluation result, so they
-cannot silently recur in any plugin.
+Codifies structural defect classes that can corrupt an evaluation result, so
+they cannot silently recur in any plugin.
 
 FAILS on unambiguous bugs:
   1. Referenced fixture missing on disk. The scenario fails at setup, which
@@ -41,6 +41,8 @@ FAILS on unambiguous bugs:
      alias for `defaults`; vally's loader throws on a spec carrying both, the
      evaluate job then produces no verdicts, and CI misreports that as a
      transient infrastructure failure.
+ 11. Duplicate stimulus names. Vally pairs comparison trajectories by stimulus
+     name and trial index, so names are slot identity, not display text.
 
 Every failing check above is structural — it inspects file existence, git
 state, declared numbers, or YAML shape/keys — so it cannot fire spuriously on
@@ -249,6 +251,23 @@ def check_spec_shape(spec: str, doc: dict, raw: str) -> None:
             f"failure. Merge them into one 'defaults:' block")
 
 
+def check_stimulus_names(spec: str, doc: dict) -> None:
+    """Require unique names because Vally uses them as comparison slot identity."""
+    seen: set[str] = set()
+    for index, stimulus in enumerate(doc.get("stimuli") or []):
+        if not isinstance(stimulus, dict):
+            continue
+        name = stimulus.get("name")
+        if not isinstance(name, str) or not name:
+            continue  # Vally schema validation owns missing or malformed names.
+        if name in seen:
+            errors.append(
+                f"{spec}: duplicate stimulus name {name!r} at stimuli[{index}]. "
+                f"Vally pairs trajectories by (stimulus name, trial index), so every "
+                f"stimulus name must be unique.")
+        seen.add(name)
+
+
 def check_dormancy_guards(spec: str, doc: dict) -> None:
     for stim in doc.get("stimuli") or []:
         if stim.get("expect_activation") is not False:
@@ -687,6 +706,7 @@ def main() -> int:
         check_fixtures(spec, doc, tracked)
         check_graders(spec, doc)
         check_spec_shape(spec, doc, raw)
+        check_stimulus_names(spec, doc)
         check_dormancy_guards(spec, doc)
 
     check_cobertura()

eng/eval-quality/selftest_eval_quality.py

@@ -248,6 +248,14 @@ def config_and_defaults_together(d):
         f.write("config:\n  timeout: 5m\n")
 
 
+def duplicate_stimulus_names(d):
+    path = EV(d)
+    with open(path) as f:
+        raw = f.read()
+    with open(path, "w") as f:
+        f.write(raw.replace("name: Does the edge thing", "name: Does the thing", 1))
+
+
 def grandfathered_reports_its_arithmetic(d):
     # The gate's job for a grandfathered eval is to tell the contributor what to
     # change, so the report must separate distinct stimuli from repeated runs.
@@ -427,6 +435,8 @@ results = [
     case("grader with an empty config enforces nothing", empty_grader_config, expect_fail=True),
     case("duplicate key silently overwrites a scenario", duplicate_stimulus_keys, expect_fail=True),
     case("spec declares both config: and defaults:", config_and_defaults_together, expect_fail=True),
+    case("duplicate stimulus names make slot identity ambiguous",
+         duplicate_stimulus_names, expect_fail=True),
     case("dormancy guard also sets reject_skills", guard_with_reject_skills, expect_fail=True),
     case("well-formed dormancy guard", guard_ok, expect_fail=False),
     output_case("reference skill carrying a direct-activation eval",

eng/skill-validator/src/README.md

@@ -466,4 +466,6 @@ Results include bootstrap confidence intervals computed across individual runs.
 - **not significant**: the CI crosses zero — could be noise
 - **g=**: normalized gain, controlling for ceiling effects (a skill improving a strong baseline is harder than improving a weak one)
 
-The default of 5 runs provides sufficient precision for significance testing (validated by [SkillsBench](https://arxiv.org/abs/2602.12670)).
+These intervals describe the legacy run-pooled report. The repository's current
+Vally gate does not treat repeated runs as independent task samples: runs measure
+within-stimulus reliability, while distinct stimuli supply the statistical votes.

eng/vally-adapter/README.md

@@ -126,6 +126,12 @@ These are planning values, not Vally requirements. Ties require more total
 stimuli because they do not enter the sign test. A non-pass is therefore not
 proof that the skill has no effect.
 
+The table is sign-test power before the 20% practical floor. At a true 60%
+conditional win rate, 158 votes give the sign test 80.6% power, but the combined
+gate passes about 52.2% of records because the expected effect sits exactly on
+the floor. The gate is intended to certify effects above that practical
+threshold.
+
 ## Objective completion contract
 
 `VALID_REGRESSION` stays reserved until the harness can compute this tri-state

eng/vally-adapter/adapt.mjs

@@ -569,8 +569,53 @@ function classifyComparisonError(evidence) {
 
 function comparisonTrialKey(stimulusName, trial) {
   const trialIndex = trial?.trialIndex;
-  if (trialIndex == null) return null;
-  return JSON.stringify([stimulusName ?? "", trialIndex]);
+  if (!stimulusName || !Number.isInteger(trialIndex) || trialIndex < 0) return null;
+  return JSON.stringify([stimulusName, trialIndex]);
+}
+
+function comparisonTrialIdentityErrors(report) {
+  const seen = new Set();
+  const duplicates = new Set();
+  const errors = [];
+  for (const stimulus of report.stimuli ?? []) {
+    for (const trial of stimulus.trials ?? []) {
+      const key = comparisonTrialKey(stimulus.stimulusName, trial);
+      if (key === null) {
+        errors.push({
+          phase: "comparison_pairing",
+          kind: "permanent",
+          code: "comparison_trial_identity_missing",
+          message: "Comparison trial is missing trialIndex",
+          stimulusName: stimulus.stimulusName,
+          trialIndex: null,
+        });
+      } else if (seen.has(key) && !duplicates.has(key)) {
+        duplicates.add(key);
+        errors.push({
+          phase: "comparison_pairing",
+          kind: "permanent",
+          code: "comparison_trial_identity_duplicate",
+          message: "Comparison report contains a duplicate (stimulusName, trialIndex) slot",
+          stimulusName: stimulus.stimulusName,
+          trialIndex: trial.trialIndex,
+        });
+      } else {
+        seen.add(key);
+      }
+    }
+  }
+  return errors;
+}
+
+function comparisonTrialKeyCounts(report) {
+  const counts = new Map();
+  for (const stimulus of report.stimuli ?? []) {
+    for (const trial of stimulus.trials ?? []) {
+      const key = comparisonTrialKey(stimulus.stimulusName, trial);
+      if (key !== null) counts.set(key, (counts.get(key) ?? 0) + 1);
+    }
+  }
+  return counts;
 }
 
 function summarizeComparisonTrials(report, invalidateInterval = false) {
@@ -611,11 +656,13 @@ function summarizeComparisonTrials(report, invalidateInterval = false) {
  */
 function mergeComparisonReports(primaryReport, retryReport) {
   const report = structuredClone(primaryReport);
+  const primaryKeyCounts = comparisonTrialKeyCounts(primaryReport);
+  const retryKeyCounts = comparisonTrialKeyCounts(retryReport);
   const retryTrials = new Map();
   for (const stimulus of retryReport?.stimuli ?? []) {
     for (const trial of stimulus.trials ?? []) {
       const key = comparisonTrialKey(stimulus.stimulusName, trial);
-      if (key !== null) retryTrials.set(key, trial);
+      if (key !== null && retryKeyCounts.get(key) === 1) retryTrials.set(key, trial);
     }
   }
 
@@ -633,7 +680,9 @@ function mergeComparisonReports(primaryReport, retryReport) {
       retriedSlots++;
       const error = classifyComparisonError(trial.evidence);
       const retryKey = comparisonTrialKey(stimulus.stimulusName, trial);
-      const retry = retryKey === null ? undefined : retryTrials.get(retryKey);
+      const primaryIdentityIsUnique =
+        retryKey !== null && primaryKeyCounts.get(retryKey) === 1;
+      const retry = primaryIdentityIsUnique ? retryTrials.get(retryKey) : undefined;
       if (retry && !retry.errored) {
         recoveredErrors.push({
           stimulusName: stimulus.stimulusName,
@@ -648,14 +697,34 @@ function mergeComparisonReports(primaryReport, retryReport) {
         };
       }
 
-      const retryError = retry?.errored
-        ? classifyComparisonError(retry.evidence)
-        : {
-            phase: "comparison_judge",
-            kind: "unknown",
-            code: "retry_result_missing",
-            message: "Comparison retry did not return the planned trial slot",
-          };
+      const retryError = !primaryIdentityIsUnique
+        ? {
+            phase: "comparison_pairing",
+            kind: "permanent",
+            code:
+              retryKey === null
+                ? "comparison_trial_identity_missing"
+                : "comparison_trial_identity_duplicate",
+            message:
+              retryKey === null
+                ? "Original comparison trial is missing trialIndex"
+                : "Original comparison report contains a duplicate trial slot",
+          }
+        : retryKey !== null && (retryKeyCounts.get(retryKey) ?? 0) > 1
+          ? {
+              phase: "comparison_pairing",
+              kind: "permanent",
+              code: "retry_result_ambiguous",
+              message: "Comparison retry returned a duplicate trial slot",
+            }
+          : retry?.errored
+            ? classifyComparisonError(retry.evidence)
+            : {
+                phase: "comparison_judge",
+                kind: "unknown",
+                code: "retry_result_missing",
+                message: "Comparison retry did not return the planned trial slot",
+              };
       persistentErrors.push({
         stimulusName: stimulus.stimulusName,
         trialIndex: trial.trialIndex ?? index,
@@ -809,6 +878,7 @@ function comparisonToVerdict(report, identity, roles, nonActivationStims) {
   const unmatchedBaseline = report.unmatchedBaseline ?? [];
   const unmatchedTreatment = report.unmatchedTreatment ?? [];
   const unmatchedTrialCount = unmatchedBaseline.length + unmatchedTreatment.length;
+  const identityErrors = comparisonTrialIdentityErrors(report);
 
   // Raw paired trials remain authoritative for report-integrity checks, retry
   // accounting, and within-stimulus reliability. They are not independent task
@@ -836,7 +906,11 @@ function comparisonToVerdict(report, identity, roles, nonActivationStims) {
     );
   }
 
-  const conclusive = s.erroredCount === 0 && unmatchedTrialCount === 0 && summaryAgrees;
+  const conclusive =
+    s.erroredCount === 0 &&
+    unmatchedTrialCount === 0 &&
+    summaryAgrees &&
+    identityErrors.length === 0;
 
   // Collapse repeated runs to one vote per stimulus. A stimulus votes in the
   // direction supported by more of its successful runs; an even split or all
@@ -1074,11 +1148,13 @@ function comparisonToVerdict(report, identity, roles, nonActivationStims) {
   if (!conclusive) {
     state = VERDICT_STATES.INVALID_INCONCLUSIVE;
     stateReason =
-      unresolvedErrors.length > 0
-        ? { code: "comparison_judge_error", phase: "comparison_judge" }
-        : unmatchedTrialCount > 0
-          ? { code: "unmatched_trajectories", phase: "comparison_pairing" }
-          : { code: "comparison_summary_mismatch", phase: "adapter" };
+      identityErrors.length > 0
+        ? { code: identityErrors[0].code, phase: "comparison_pairing" }
+        : unresolvedErrors.length > 0
+          ? { code: "comparison_judge_error", phase: "comparison_judge" }
+          : unmatchedTrialCount > 0
+            ? { code: "unmatched_trajectories", phase: "comparison_pairing" }
+            : { code: "comparison_summary_mismatch", phase: "adapter" };
   } else if (underpowered) {
     state = VERDICT_STATES.INVALID_INCONCLUSIVE;
     stateReason = { code: "underpowered", phase: "eval_design" };
@@ -1168,7 +1244,7 @@ function comparisonToVerdict(report, identity, roles, nonActivationStims) {
       recoveredErrors: [],
       persistentErrors: [],
     },
-    errors: unresolvedErrors,
+    errors: [...identityErrors, ...unresolvedErrors],
     recoveredErrors,
     scenarios,
     reason,

eng/vally-adapter/adapt.test.mjs

@@ -533,7 +533,64 @@ test("does not pair retry slots by array position when trialIndex is absent", ()
   assert.equal(merged.retrySummary.recoveredSlots, 0);
   assert.equal(
     merged.retrySummary.persistentErrors[0].attemptHistory[1].code,
-    "retry_result_missing",
+    "comparison_trial_identity_missing",
+  );
+});
+
+test("does not recover an ambiguous duplicate retry slot", () => {
+  const primary = reportFromRepeatedScores([null]);
+  primary.stimuli[0].trials[0] = {
+    trialIndex: 0,
+    score: 0,
+    winner: "tie",
+    errored: true,
+    evidence: "Comparison judge failed: Timeout after 120000ms waiting for session.idle",
+  };
+  primary.summary.trialCount = 0;
+  primary.summary.erroredCount = 1;
+  primary.summary.wins = 0;
+
+  const retry = reportFromRepeatedScores([0.4, 0.4]);
+  retry.stimuli[0].trials[1].trialIndex = 0;
+  const merged = mergeComparisonReports(primary, retry);
+
+  assert.equal(merged.summary.erroredCount, 1);
+  assert.equal(merged.retrySummary.recoveredSlots, 0);
+  assert.equal(
+    merged.retrySummary.persistentErrors[0].attemptHistory[1].code,
+    "retry_result_ambiguous",
+  );
+});
+
+test("missing or duplicate comparison slot identities invalidate a verdict", () => {
+  const missing = reportFromScores([0.4, 0.4, 0.4, 0.4, 0.4]);
+  delete missing.stimuli[0].trials[0].trialIndex;
+  const missingVerdict = comparisonToVerdict(
+    missing,
+    IDENTITY,
+    EMPTY_ROLES,
+    new Set(),
+  );
+  assert.equal(missingVerdict.conclusive, false);
+  assert.equal(missingVerdict.state, VERDICT_STATES.INVALID_INCONCLUSIVE);
+  assert.equal(
+    missingVerdict.stateReason.code,
+    "comparison_trial_identity_missing",
+  );
+
+  const duplicate = reportFromStimulusRuns([[0.4, 0.4], [0.4], [0.4], [0.4]]);
+  duplicate.stimuli[0].trials[1].trialIndex = 0;
+  const duplicateVerdict = comparisonToVerdict(
+    duplicate,
+    IDENTITY,
+    EMPTY_ROLES,
+    new Set(),
+  );
+  assert.equal(duplicateVerdict.conclusive, false);
+  assert.equal(duplicateVerdict.state, VERDICT_STATES.INVALID_INCONCLUSIVE);
+  assert.equal(
+    duplicateVerdict.stateReason.code,
+    "comparison_trial_identity_duplicate",
   );
 });