Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
20 changes: 20 additions & 0 deletions .github/workflows/evaluation-run.yml
Original file line number Diff line number Diff line change
Expand Up @@ -765,6 +765,26 @@ jobs:
exit 1
fi

# A single agent session can time out transiently while every other
# trial succeeds. Retry only that eval/variant once, then replace only
# the matching failed slots. Persistent or non-timeout failures remain
# in the original JSONL and fail the measurement-validity gate below.
set +e
timeout --signal=TERM --kill-after=30s 30m \
node "$RUNNER_TEMP/trusted-validator-src/eng/vally-adapter/retry-executor-timeouts.mjs" \
--experiment-file "$EXPERIMENT_FILE" \
--experiment-dir "$RUN_DIR" \
--retry-output-dir "$RESULTS_DIR/_executor-retry" \
--summary "$RESULTS_DIR/executor-retry-summary.json" \
--vally "vally" \
--workers 5 \
--max-groups 3
EXECUTOR_RETRY_STATUS=$?
set -e
if [ "$EXECUTOR_RETRY_STATUS" -ne 0 ]; then
echo "::warning::Executor timeout recovery exited $EXECUTOR_RETRY_STATUS; adapting original and any atomically recovered records"
fi

# Run the retained skill-validator overfitting judge over the same
# skills this leg evaluated, before adapting. It reads GITHUB_TOKEN
# (already exported above) for its single LLM call per skill. Scope it
Expand Down
23 changes: 23 additions & 0 deletions eng/evaluation/test_token_failover.py
Original file line number Diff line number Diff line change
Expand Up @@ -693,6 +693,29 @@ def test_fork_checkout_is_blocked_and_adapter_code_is_trusted(self) -> None:
"Vally comparison watchdog expired after 45 minutes",
run_script,
)
self.assertIn(
"retry-executor-timeouts.mjs",
run_script,
)
self.assertIn(
'--max-groups 3',
run_script,
)
self.assertIn(
'EXECUTOR_RETRY_STATUS=$?',
run_script,
)
self.assertIn(
'if [ "$EXECUTOR_RETRY_STATUS" -ne 0 ]',
run_script,
)
self.assertLess(
run_script.index("retry-executor-timeouts.mjs"),
run_script.index(
'node "$RUNNER_TEMP/trusted-validator-src/'
'eng/vally-adapter/adapt.mjs"'
),
)
summary_script = by_name["Write summary"]["run"]
self.assertIn('ICON="➖"', summary_script)
self.assertNotIn('ICON="❌"', summary_script)
Expand Down
5 changes: 5 additions & 0 deletions eng/skill-validator/src/docs/InvestigatingResults.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
# Investigating Evaluation Results

> **⚠️ Skill evaluations now run on the Vally harness.** As of the Vally migration, the LLM eval pipeline (`evaluation.yml`) no longer uses `skill-validator evaluate`; it runs Vally via `eng/vally-adapter/` and uploads `vally-results-*` artifacts. For investigating current eval failures, use the guide at `eng/vally-adapter/InvestigatingResults.md` in the repository root instead. This document describes the legacy `skill-validator evaluate` schema and is retained for historical results and reference. (The `skill-validator check` **linter** is unaffected and still runs via `skill-check.yml`.)
>
> The current Vally workflow makes one targeted recovery attempt for executor
> `session.idle` timeouts before adaptation. See
> `executor-retry-summary.json` in the result artifact and the current guide for
> the bounded retry and fail-closed rules.

> **Current Vally schema:** `state` is authoritative:
> `VALID_PASS`, `VALID_REGRESSION`, `VALID_NO_CHANGE`, or
Expand Down
11 changes: 11 additions & 0 deletions eng/vally-adapter/InvestigatingResults.md
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,17 @@ Work top-down; earlier categories often cause later ones.
### 1. Errored or missing trials (`state == "INVALID_INCONCLUSIVE"`)
The agent crashed, the model was unavailable, evidence was missing, or the comparison judge failed. Check `stateReason`, `errors[]`, `adapter-summary.json`, and the variant's `results.jsonl`/session logs. These are invalid measurements, not skill regressions. If a required variant produced no records, the adapter writes an explicit invalid result with `missing_baseline_records` or `missing_skilled_records`.

The workflow retries only required baseline or isolated-skilled executor records
whose exact failure is a `session.idle` timeout. It reruns the affected eval and
variant once, preserves all successful first-attempt slots, and replaces only
matching failed `shardKey` slots from the same normalized eval path that
succeed. Records without a `shardKey` remain invalid. Check
`executor-retry-summary.json` and the raw record's `executorRetry` field for
recovered attempts. Persistent timeouts, other executor failures, or more than
three affected eval/variant groups remain measurement-invalid and keep the
matrix leg red. The optional whole-plugin arm is report-only telemetry and is
not retried.

If Vally writes a JSON record that cannot satisfy the comparison schema, the
adapter emits `comparison_report_invalid` for that eval and continues the batch.
This preserves exact result accounting without treating malformed evidence as a
Expand Down
23 changes: 20 additions & 3 deletions eng/vally-adapter/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,24 @@ metadata. The adapter keeps the raw report available for diagnosis. It does not
replace Vally's evidence; it adds repository-specific validity and decision
fields.

### 5. Retry only failed comparison-judge slots
### 5. Retry transient executor timeouts once

If a required baseline or isolated-skilled trial fails with
`Timeout after ... waiting for session.idle`, the workflow reruns only that eval
and variant once. The recovery step merges only successful records with matching
stable `shardKey` values after normalizing each eval path. A record without a
`shardKey` fails closed instead of using another field as an unproven identity.
The recovery never replaces successful first-attempt records. A persistent
timeout or a different executor error stays in the original JSONL and remains
measurement-invalid. The optional whole-plugin telemetry arm is not retried and
remains outside the baseline-versus-skilled measurement gate.

The retry is limited to three affected eval/variant groups per matrix leg. More
groups indicate a systemic failure, so the workflow skips recovery and fails
closed instead of multiplying load. `executor-retry-summary.json` and each
recovered record's `executorRetry` field retain the attempt evidence.

### 6. Retry only failed comparison-judge slots

The paired identity is `(stimulusName, trialIndex)`. If a comparison judge
times out or its organization is disabled, the adapter builds a retry report
Expand All @@ -203,7 +220,7 @@ The merged report keeps retry diagnostics even when no slot recovers.
Retry is bounded to one additional attempt. Remaining judge errors make the
result invalid. They are not counted as skill losses.

### 6. Convert repeated trials into independent stimulus votes
### 7. Convert repeated trials into independent stimulus votes

Repeated runs answer "does this task behave consistently?" They do not answer
"does this work on more kinds of tasks?" The adapter therefore gives each
Expand All @@ -230,7 +247,7 @@ flowchart LR
This prevents a four-task eval with three repetitions from pretending to have
twelve independent test cases.

### 7. Apply the repository decision rule
### 8. Apply the repository decision rule

For one model and skill's baseline-versus-isolated comparison:

Expand Down
1 change: 1 addition & 0 deletions eng/vally-adapter/adapt.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -1669,6 +1669,7 @@ export {
classifyComparisonError,
mergeComparisonReports,
loadExpectedEvalFiles,
normalizeEvalFile,
VERDICT_STATES,
MIN_CREDIBLE_STIMULI,
MIN_PRACTICAL_NET_WIN,
Expand Down
Loading