Stabilize skill menu and evaluation retries - #1072
Conversation
Reduce the dotnet-test skill menu below the runtime budget and retry only transient baseline/skilled session.idle timeouts without replacing successful evidence. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Skill Coverage Report
Uncovered:
|
There was a problem hiding this comment.
Pull request overview
This pull request stabilizes evaluation runs by adding a narrowly-scoped retry for transient Vally executor session.idle timeouts (baseline + isolated-skilled only) while keeping the measurement gate fail-closed, and reduces the dotnet-test skill-menu footprint by tightening the test-tagging routing description.
Changes:
- Tighten
plugins/dotnet-test/skills/test-tagging/SKILL.mddescription to reduce rendered menu size under the 15,000 character runtime limit. - Add
eng/vally-adapter/retry-executor-timeouts.mjs(+ tests) to rerun only affected eval/variant groups once and merge only recovered slots. - Wire the recovery step into
.github/workflows/evaluation-run.ymland update investigation docs/tests to reflect the new retry behavior.
Show a summary per file
| File | Description |
|---|---|
| plugins/dotnet-test/skills/test-tagging/SKILL.md | Shortens routing description to reduce skill-menu footprint. |
| eng/vally-adapter/retry-executor-timeouts.mjs | Implements bounded retry + atomic merge of recovered executor-timeout records. |
| eng/vally-adapter/retry-executor-timeouts.test.mjs | Adds unit + CLI-level coverage for retry grouping and merge behavior. |
| eng/vally-adapter/README.md | Documents the new executor-timeout retry step in the pipeline. |
| eng/vally-adapter/InvestigatingResults.md | Adds guidance for diagnosing executor-timeout retries and artifacts. |
| eng/skill-validator/src/docs/InvestigatingResults.md | Notes the new Vally retry behavior in the legacy investigation doc header. |
| eng/evaluation/test_token_failover.py | Updates workflow-behavior assertions to require the retry step to run before adaptation. |
| .github/workflows/evaluation-run.yml | Adds the retry step to rerun/merge transient executor timeouts before adaptation. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 8/8 changed files
- Comments generated: 2
- Review effort level: Lite
|
👋 @AbhitejJohn — this PR has 2 unresolved review thread(s). When you're ready, please address the feedback and push an update; the triage bot will pick up the next state automatically. (Add the |
Normalize eval paths before retry matching and require stable shardKey identities so recovery fails closed when record identity is uncertain. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4c7484eb-1fa5-4332-841e-e924031c75a2
|
@Evangelink, could you please review this stability change when you have time? |
📊 Skill Evaluation Results2 model/skill results across 1 skill and 2 models — ✅ 1 improved, ➖ 1 not proven improved, Measurement identity: evaluated commit Measurement health: 2 expected / 2 observed / 2 written; 0 missing, 0 unexpected, 0 invalid; 0 recovered comparison error slots and 0 unresolved comparison error slots. Objective completion gate: not enabled. Aggregate completion transitions are telemetry only, so this report does not claim that zero objective regressions were proven. A result passes only when the aggregate net win across distinct-stimulus votes is at least 20% and an exact one-sided sign-test result of
ℹ️ How to read this report
➖ Not proven improved — test-tagging (gpt-5.6-luna)Why: Net win +41.7% (6W/5T/1L over 12 stimulus vote(s), sign test p=0.063), mean preference +31.7% across 12 paired run(s) — not credible (sign test p=0.063 > 0.05) Next action: Inspect tied or lost stimuli and fix inconsistent skill behavior. State: Gate evidence: n=12; 6W/5T/1L; d=7; p=0.063; net +41.7% Overfit: Low (score 0.15) Repeated-run reliability (not used by the gate): 12 paired runs (6W/5T/1L). Weak or warning scenarios:
Illustrative judge evidence:
This is one example, not the aggregate verdict. Open Full Results for every judgment. ✅ Improved — test-tagging (claude-sonnet-4.6)Why: Net win +66.7% (9W/2T/1L over 12 stimulus vote(s), sign test p=0.011), mean preference +46.7% across 12 paired run(s) — credibly better Next action: Review overfit evidence. State: Gate evidence: n=12; 9W/2T/1L; d=10; p=0.011; net +66.7% Overfit: Moderate (score 0.48) Repeated-run reliability (not used by the gate): 12 paired runs (9W/2T/1L). Weak or warning scenarios:
Illustrative judge evidence:
This is one example, not the aggregate verdict. Open Full Results for every judgment. 🔍 Full Results - all metrics and investigation details
▶ Sessions Visualisation -- interactive replay of all evaluation sessions |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f9a03f2d-8e7b-400e-bf97-809c8c3c6299
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f9a03f2d-8e7b-400e-bf97-809c8c3c6299
There was a problem hiding this comment.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
plugins/dotnet-test/skills/test-tagging/SKILL.md:9
- The frontmatter
descriptionis what the runtime uses for skill routing, and repo guidance says sibling skills should be partitioned with explicit matching exclusions. This shortenedDO NOT USEclause no longer names the owning sibling skills (e.g.,test-anti-patterns,coverage-analysis,run-tests,crap-score,test-gap-analysis), which can make routing ambiguous even though the body text is detailed.
Classifies existing tests by standard traits and reports their distribution.
MUST USE to categorize/tag/label tests, compare happy vs error paths, audit
the test mix, or describe coverage shape by test type. Read bodies when names
mislead. Apply canonical attributes; otherwise report only. DO NOT USE for
test-quality audits, executed coverage or CRAP, behavioral gaps, writing
- Files reviewed: 9/9 changed files
- Comments generated: 0 new
- Review effort level: Lite
📊 Skill Evaluation Results2 model/skill results across 1 skill and 2 models — ✅ 1 improved, ➖ 1 not proven improved, Measurement identity: evaluated commit Measurement health: 2 expected / 2 observed / 2 written; 0 missing, 0 unexpected, 0 invalid; 0 recovered comparison error slots and 0 unresolved comparison error slots. Objective completion gate: not enabled. Aggregate completion transitions are telemetry only, so this report does not claim that zero objective regressions were proven. A result passes only when the aggregate net win across distinct-stimulus votes is at least 20% and an exact one-sided sign-test result of
ℹ️ How to read this report
➖ Not proven improved — test-tagging (gpt-5.6-luna)Why: Net win +33.3% (5W/6T/1L over 12 stimulus vote(s), sign test p=0.109), mean preference +28.3% across 12 paired run(s) — not credible (sign test p=0.109 > 0.05) Next action: Inspect tied or lost stimuli and fix inconsistent skill behavior. State: Gate evidence: n=12; 5W/6T/1L; d=6; p=0.109; net +33.3% Overfit: Low (score 0.16) Repeated-run reliability (not used by the gate): 12 paired runs (5W/6T/1L). Weak or warning scenarios:
Illustrative judge evidence:
This is one example, not the aggregate verdict. Open Full Results for every judgment. ✅ Improved — test-tagging (claude-sonnet-4.6)Why: Net win +75.0% (9W/3T/0L over 12 stimulus vote(s), sign test p=0.002), mean preference +60.0% across 12 paired run(s) — credibly better Next action: Review overfit evidence. State: Gate evidence: n=12; 9W/3T/0L; d=9; p=0.002; net +75.0% Overfit: Moderate (score 0.38) Repeated-run reliability (not used by the gate): 12 paired runs (9W/3T/0L). Weak or warning scenarios:
Illustrative judge evidence:
This is one example, not the aggregate verdict. Open Full Results for every judgment. 🔍 Full Results - all metrics and investigation details
▶ Sessions Visualisation -- interactive replay of all evaluation sessions |
📊 Skill Evaluation Results2 model/skill results across 1 skill and 2 models — ✅ 2 improved, ➖ 0 not proven improved, Measurement identity: evaluated commit Measurement health: 2 expected / 2 observed / 2 written; 0 missing, 0 unexpected, 0 invalid; 0 recovered comparison error slots and 0 unresolved comparison error slots. Objective completion gate: not enabled. Aggregate completion transitions are telemetry only, so this report does not claim that zero objective regressions were proven. A result passes only when the aggregate net win across distinct-stimulus votes is at least 20% and an exact one-sided sign-test result of
ℹ️ How to read this report
✅ Improved — test-tagging (claude-sonnet-4.6)Why: Net win +83.3% (11W/0T/1L over 12 stimulus vote(s), sign test p=0.003), mean preference +63.3% across 12 paired run(s) — credibly better Next action: Review overfit evidence. State: Gate evidence: n=12; 11W/0T/1L; d=12; p=0.003; net +83.3% Overfit: Moderate (score 0.44) Repeated-run reliability (not used by the gate): 12 paired runs (11W/0T/1L). Weak or warning scenarios:
Illustrative judge evidence:
This is one example, not the aggregate verdict. Open Full Results for every judgment. Routine passing details for 1 result are in Full Results. 🔍 Full Results - all metrics and investigation details ▶ Sessions Visualisation -- interactive replay of all evaluation sessions |
Summary
dotnet-testrendered skill-menu footprint from 15,063 to 14,598 characters by tightening only thetest-taggingrouting descriptionsession.idleexecutor timeouts once for required baseline and isolated-skilled armsshardKeyvaluesmeasurementInvalidEvalCount === 0gate for persistent, systemic, non-timeout, missing, duplicate, or ambiguous comparison evidenceRoot causes
PR #1051 expanded several routing descriptions and moved
dotnet-testfrom 14,421 rendered characters to 15,063, above the unchanged Copilot runtime limit of 15,000. PR #1059 did not cause that regression.In full-corpus runs 32989467349 and 32992064149, every expected result file was written, but one required skilled-arm trial timed out waiting for
session.idle. The paired baseline record then had no treatment match, so the adapter correctly classified the eval asunmatched_trajectoriesand measurement-invalid. Exact-commit retries moved the timeout to a different plugin/model job, which showed a transient executor failure rather than dashboard or corpus accounting failure.Stability behavior
Recovery is narrow and bounded:
session.idletimeout records qualifyevalFileandshardKeyon both attemptsValidation
skill-validator check: 98 skills, 16 agents, 16 plugins passeddotnet-testvalidator check passed at 14,598 / 15,000 rendered characters.github/workflows/evaluation-run.yml