Skip to content

Skill eval: API throttling masquerades as a held-out regression (infra failure scored as regression, not error) #920

Closed
#925
Closed
Skill eval: API throttling masquerades as a held-out regression (infra failure scored as regression, not error)#920
#925
Labels
bugBug reportsdev-leadFor dev-lead agent pickupeval-healthSkill-eval holdout health tracking

Description

@don-petry
don-petry
opened on Jun 25, 2026

Summary

The held-out skill eval cannot distinguish "the model never answered" (an infra/throttle failure) from "the model gave a wrong answer" (a genuine quality regression). Both collapse to a failing case and exit code 1, which the workflow maps to outcome=regression — so a transient Anthropic API throttle opens/updates a false eval-health regression tracker against a skill that is actually fine.

This is a live false alarm right now: #911 (triage holdout, 0/5) was opened by a throttled run, even though the triage prompt fix (#897) scored a clean 5/5 hours earlier.

Evidence

Scheduled run 28020121086 (triage), job eval-report:

  • Whole run took 9 seconds for 5 model-backed cases — the models never actually answered.
  • Scorer log shows every model in the fallback chain throttled:
##[warning][claude] model claude-haiku-4-5-20251001 throttled (rc=1) — trying next in chain
##[warning][claude] model claude-sonnet-4-6   throttled (rc=1) — trying next in chain
   …repeated for all 5 cases…
##[error]Process completed with exit code 1.

Root cause

In scripts/evals/run-eval.sh:

  • Line 242 swallows the engine's exit status, discarding the only signal that the call failed for infra reasons:

    raw="$("$EVAL_ENGINE_CMD" "$work_prompt")" || raw=""

    A throttle (engine exits non-zero after exhausting its fallback chain) and a model that genuinely returned wrong/garbage output (engine exits 0, output unparseable) both end up as raw=""/actual=null → indistinguishable downstream.

  • Lines 262-263 exit 1 whenever any case failed, regardless of cause:

    failed="$(jq -s 'map(select(.pass | not)) | length' "$results")"
[ "$failed" -eq 0 ]

The exit-code contract (lines 52-55) already reserves 2 for hard/infra errors, and .github/workflows/skill-eval-report.yml already maps exit 2 → outcome=error (vs exit 1 → regression). The throttle path simply never reaches exit 2.

Note there is prior art for the "null everywhere" failure mode: the extract_json fence-stripping (line 129, ref #762) and the raw_excerpt capture (lines 156-160) were both added to make blind null-everywhere failures diagnosable — this issue is the remaining half: classifying an all-infra-failure run as error rather than regression.

Proposed fix

Capture the engine exit status per case and use it to distinguish infra failure from quality miss:

raw=""; eng_rc=0
raw="$("$EVAL_ENGINE_CMD" "$work_prompt")" || eng_rc=$?

Then, when assembling the outcome: if no case produced a successful model response (every failing case is attributable to a non-zero engine exit — i.e. throttle/infra, not a parseable-but-wrong answer), exit 2 (hard error → outcome=error) instead of 1 (regression). A genuine quality miss (engine exits 0, output wrong or unparseable) must still score as a case failure and exit 1.

Correspondingly, notify-eval-health.sh should treat outcome=error distinctly from regression: do not open/update a "held-out regression" tracker on a pure infra error (optionally surface a separate, clearly-labeled infra-error signal so an outage is still visible without implying the skill regressed).

Acceptance criteria

  • A run where every case's engine call exits non-zero (e.g. all models throttled) produces exit 2 / outcome=error, and does not open or update an eval-health regression issue.
  • A genuine quality miss (engine exits 0, output wrong/unparseable) still scores as a case failure → exit 1 → regression, unchanged.
  • A mixed run (some cases answered, some throttled) is handled deterministically per the rule above and documented in the script header.
  • tests/ gains bats coverage for: all-throttled → error; genuine regression → regression (unchanged); mixed → defined outcome. Use the existing EVAL_ENGINE_CMD stub injection (no network).
  • No changes to evals/** held-out case data.

References

Activity

  1. don-petry
    added
    bugBug reports
    eval-healthSkill-eval holdout health tracking
    dev-leadFor dev-lead agent pickup
    on Jun 25, 2026

  2. don-petry commented on Jun 25, 2026

    @don-petry
    don-petry
    on Jun 25, 2026
    CollaboratorAuthor

    Dev-Lead Implementation Plan

    Issue: #920 — Skill eval: API throttling masquerades as a held-out regression (infra failure scored as regression, not error)

    Scope

    Capture the engine's per-case exit status in run-eval.sh so an all-throttled (infra) run exits 2 → outcome=error instead of 1 → regression, and split notify-eval-health.sh so an error outcome opens a separate, clearly-labeled infra-error tracker rather than touching the held-out regression tracker.

    Implementation Checklist

    • scripts/evals/run-eval.sh — capture eng_rc per case (raw=""; eng_rc=0; raw="$(...)" || eng_rc=$?); embed engine_rc in each case result (both deterministic + llm-judge scorers)
    • scripts/evals/run-eval.sh — replace the final [ "$failed" -eq 0 ] exit logic: if every failing case is an infra failure (engine exited non-zero, no parseable-but-wrong answer) → exit 2; a genuine quality miss (engine exited 0) keeps exit 1; document the rule (incl. mixed runs) in the script header
    • scripts/evals/notify-eval-health.sh — give error its own stable title + eval-infra label; error must never open/update/close the eval-health regression tracker; pass closes whichever tracker(s) are open (regression and/or infra)
    • No changes to evals/** held-out case data (read-only)

    Tests to Write

    • tests/test_skill_evals.bats — all-throttled stub → exit 2; mixed (one answered, one throttled) → exit 2; genuine wrong answer (engine rc 0) → exit 1 unchanged; unparseable-but-rc-0 → exit 1; assert per-case engine_rc
    • tests/test_eval_health_notify.bats — error opens a separate eval-infra tracker, never the eval-health regression tracker (even when one is open); error de-dupes its own infra tracker; regression path unchanged; pass closes an open infra tracker

    Test Command

    npx bats tests/test_skill_evals.bats tests/test_eval_health_notify.bats (full suite + bash .dev-lead/scripts/dev-lead-lint.sh)

  3. don-petry commented on Jun 25, 2026

    @don-petry
    don-petry
    on Jun 25, 2026
    CollaboratorAuthor

    Dev-Lead: Implementation Complete

    Plan Execution

    • scripts/evals/run-eval.sh — capture per-case engine exit status (raw=""; eng_rc=0; raw="$(...)" || eng_rc=$?) and embed engine_rc in every case result (deterministic + llm-judge scorers)
    • scripts/evals/run-eval.sh — replaced the final [ "$failed" -eq 0 ] with the infra/quality classifier: every failing case attributable to a non-zero engine exit → exit 2 (outcome=error); any quality miss (engine exited 0, wrong/unparseable) keeps exit 1 (regression). Rule (incl. mixed runs) documented in the header.
    • scripts/evals/notify-eval-health.sh — error now opens/updates a separate eval-infra tracker and never touches the eval-health regression tracker; pass closes whichever tracker(s) (regression and/or infra) are open.
    • No changes to evals/** held-out case data.

    Acceptance Criteria

    • All-throttled run (every engine call exits non-zero) → exit 2 / outcome=error, does not open/update a regression issue.
    • Genuine quality miss (engine exits 0, wrong/unparseable) → exit 1 / regression, unchanged.
    • Mixed run (some answered, some throttled) → deterministic: exit 2 (no parseable-but-wrong answer to call a regression); documented in the script header.
    • tests/ bats coverage added for all-throttled → error, genuine regression → regression, unparseable-but-clean-exit → regression, and mixed → error, all via EVAL_ENGINE_CMD stub injection (no network).
    • No evals/** changes.

    Test Results

    1..45  (tests/test_skill_evals.bats + tests/test_eval_health_notify.bats)
... all 45 ok ...
ok 6 all-throttled run (every engine call exits non-zero) is an infra error -> exit 2 (#920)
ok 7 mixed run (one answered, one throttled) classifies as infra error -> exit 2 (#920)
ok 8 a genuine quality miss (engine exits 0, wrong answer) stays a regression -> exit 1 (#920)
ok 9 unparseable output with a clean engine exit is a regression, not infra -> exit 1 (#920)
ok 36 error opens a SEPARATE infra tracker (eval-infra label), not the regression tracker (#920)
ok 38 error never edits or closes an open held-out REGRESSION tracker (#920)
ok 42 pass closes an open infra tracker too (outage recovery, #920)

    Full suite: 825 passing. One unrelated failure (tests/test_batch_fallback.bats: "Gemini trust error (55) falls back to Copilot") is pre-existing — it fails identically on the clean base (verified via git stash) and references neither changed script. bash .dev-lead/scripts/dev-lead-lint.sh → all checks passed.

    Files Changed

    • scripts/evals/run-eval.sh: per-case engine_rc capture + embed; exit-code classifier (all-infra → 2, else 1); header docs the rule.
    • scripts/evals/notify-eval-health.sh: separate eval-infra tracker for error; _recover / _open_or_update helpers; pass closes both trackers.
    • tests/test_skill_evals.bats: throttle + mixed stubs; 5 new classification tests; engine_rc assertion in llm-judge.
    • tests/test_eval_health_notify.bats: infra-tracker title/helper; reworked error tests + new separation/recovery tests.

    Notes

    • Bonus correctness: scripts/evals/gate.sh already treats run-eval exit 2 as a hard error (rc <= 1 is success), so a throttle during the strict-improvement gate now dies instead of comparing bogus 0-scores — an improvement, with all gate tests still green.
    • The eval-infra label is auto-created (color fbca04) on first use, mirroring the existing eval-health label bootstrap.
    • The workflow (skill-eval-report.yml) needed no change: it already maps exit 2 → outcome=error. Its "Annotate outcome" text still says "hard-errored" for an error; left as-is to avoid out-of-scope workflow edits (the tracking-issue body now describes the infra cause accurately).
  4. don-petry
    added a commit that references this issue on Jun 25, 2026

    feat: implement issue #920 — Skill eval: API throttling masquerades a…

    6768fed
  5. don-petry
    closed this as completedin #925on Jun 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugBug reportsdev-leadFor dev-lead agent pickupeval-healthSkill-eval holdout health tracking

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions