Commit Graph

9 Commits

Author SHA1 Message Date
YuliiaKovalova 76a5ec8cb3 Decouple agent runs from judging in skill-validator (--no-judge + cross-dir rejudge) (#769)
* Decouple agent runs from judging in skill-validator

Allow baseline and treatment agent runs to execute in one parallel pool, with
judging deferred to a final step, removing the serial precompute-baseline ->
evaluate barrier.

evaluate --no-judge: runs the requested agent arms and persists sessions.db +
metrics, but skips all judging and requires no baseline file (exits 0 on
successful runs). Forces session persistence; mutually exclusive with
--baseline-out/--baseline-from; requires --results-dir. Only the agent model is
validated (judge model recorded as metadata); warns when zero runs persist.

rejudge --baseline-dir: cross-directory judging that pairs each treatment run
with its baseline run by the BaselineStore key (prompt SHA + target SHA,
persisted in a new sessions.db baseline_key column), runs the same
pairwise/independent judges and pass/fail gates evaluate uses, and writes
verdicts/reports. Baseline and treatment must share --model (rejected if either
side mixes models); effective judge model resolves to --judge-model, else the
treatment DB's, else the baseline DB's, rejecting a persisted mismatch. The
inline per-run judging is extracted into a shared JudgeRunGroup helper reused by
both the same-dir and cross-dir paths (no duplicated judging logic).

Adds tests for baseline_key round-trip + legacy migration, ComputeScenarioKey,
cross-dir pairing, and model/judge-model compatibility. Updates README and
InvestigatingResults docs.

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

* Hoist ComputeScenarioKey to scenario level to avoid per-run fixture hashing

Compute the baseline key once per scenario instead of once per run, so
multi-run evaluations (--runs N) don't re-hash fixture files N times.
The key is threaded into ExecuteRun/ExecuteAgentRun as a parameter.

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

* Memoize scenario-key fixture hashing for the duration of an evaluation

Route the persisted baseline_key through an evaluation-scoped BaselineStore
cache (ComputeScenarioKeyCached) instead of the static ComputeScenarioKey, so
a scenario's setup fixtures are hashed at most once across every run, arm, and
scenario of an evaluation. When a baseline store already exists (--baseline-from
/--baseline-out) its warm input-hash cache is reused; otherwise a dedicated,
side-effect-free cache (BaselineStore.ForKeyCache) is threaded through.

Addresses the PR review comment about ComputeScenarioKey re-hashing large
fixture directories. Adds a test asserting the cached variant equals the
canonical static computation.

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

* Bump session schema version to 3 for the baseline_key column

The baseline_key column was added without bumping schema_info.version, so
external tools keying off the version couldn't detect the new column. Introduce
a single SchemaVersion constant (= 3), stamp it after migrations run, and drop
the drift-prone version literal from the raw CREATE SQL so SetSchemaInfo is the
sole source of truth. Update the schema-info and legacy-upgrade tests to expect
version 3.

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

* Fail fast on judging-dependent options under --no-judge; fix README prefix

Under --no-judge the noise test and overfitting fix cannot run (they require
judging), so reject --noise-skills-dir and --overfitting-fix up front instead
of silently ignoring them, mirroring the existing --baseline-out/--baseline-from
guard. Add EvaluateCommandTests covering these early rejections (they short-
circuit before any model/network call).

Also prefix the decoupled-runs README example with 'skill-validator' for
consistency with the other CLI invocations in the doc.

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-06-16 13:35:33 +02:00
YuliiaKovalova bcaa91882c Add reusable shared baseline to skill-validator evaluate (--baseline-out / --baseline-from) (#754)
* Add shared/precomputed baseline reuse to skill-validator evaluate (#751)

Add --baseline-out and --baseline-from options to the evaluate command so the
no-skill/no-agent baseline arm can be computed once and reused as a shared
control group across multiple skill/agent evaluations. This eliminates redundant
baseline runs and removes baseline run-to-run variance from cross-config
comparisons.

- New BaselineStore + BaselineFile/BaselineScenarioEntry models, keyed per
  scenario on SHA-256(prompt) with a header recording version, model,
  validator version and runs. Load validates version + model and fails fast on
  mismatch or missing scenarios.
- Register the new serializable types in SkillValidatorJsonContext (AOT
  source-gen).
- Wire two mutually-exclusive CLI options into ValidatorConfig; thread an
  optional BaselineStore through both execution paths.
- On reuse, skip the baseline agent run, its assertions/constraints/
  task-completion/judging, and attribute no extra pairwise tokens to the
  baseline; report the scenario with the baseline-reused session phase and a
  reused status. In write mode, record each scenario's averaged baseline and
  persist it after the run.
- Add unit tests for BaselineStore and document the feature in README and
  InvestigatingResults.

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

* Bind reused baseline to fixture identity (targetSha), not just prompt

Align baseline reuse with the (prompt, model, targetSha) identity contract from
the upstream eval-harness design: previously the per-scenario reuse key was the
prompt SHA + model only, so two scenarios that share an identical prompt but
feed the agent different input artifacts (e.g. a different build.binlog) would
collide and silently reuse the wrong baseline.

- Add BaselineScenarioEntry.TargetSha: a SHA-256 over the scenario's materialized
  inputs — files auto-copied via copy_test_files, explicit setup files (inline
  content or copied sources), and the setup command recipe. The reuse key is now
  (promptSha, targetSha); both must match. Bump the on-disk schema to version 2.
- Memoize target hashing per process via a cheap, file-I/O-free setup signature
  to avoid re-hashing large fixtures across the N runs.
- Thread the originating eval.yaml path into Record/TryGetBaseline/
  FindMissingScenarios so inputs can be fingerprinted.
- Tests: target SHA is stable and content-sensitive; same-prompt/different-fixture
  scenarios do not reuse each other's baseline and are surfaced by
  FindMissingScenarios. Update README and InvestigatingResults.

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

* Harden baseline reuse identity after review

Address rubber-duck review findings on the shared-baseline feature:

- Fix Sha256Hex 32-byte bug: a 32-byte input was treated as an
  already-computed digest and not hashed. Split into Sha256Hex (always
  hash) + HexDigest (encode existing digest).
- Broaden reuse identity: the cached baseline RunResult depends on the
  judge model and on per-scenario evaluation criteria (rubric,
  assertions, expect/reject tools, turn/token/timeout limits). Add
  JudgeModel to the baseline header (validated on load) and fold the
  criteria into the per-scenario targetSha so changing them invalidates
  reuse instead of silently serving a stale result.
- Mirror AgentRunner.SetupWorkDir exactly when hashing copied fixtures:
  exclude only the top-level eval.yaml (nested eval.yaml files are
  copied, so they must be hashed).
- Make the target-SHA cache instance-scoped (memoizing only the
  expensive fixture-input hashing) so it can't serve stale hashes or
  leak across evaluations/tests; hash inline file Content in the cache
  key instead of embedding it.
- Deterministic Save ordering (Name, PromptSha, TargetSha); guard Load
  against null Scenarios; enrich FindMissingScenarios output with the
  eval path.
- Document that setup commands are fingerprinted by recipe, so reuse
  assumes they are deterministic/hermetic.
- Tests + docs updated; add judge-model-mismatch and criteria-identity
  tests (562 pass).

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

* Address PR review comments on baseline reuse

- Mirror AgentRunner.SetupWorkDir exactly when hashing copied fixtures:
  enumerate only the files actually copied (top-level siblings except
  eval.yaml, recursing into directories) and skip reparse points and
  out-of-root junctions, instead of blindly hashing every file under the
  eval directory. This keeps the fixture identity restricted to the
  intentionally-copied set so stray output/log files can't poison reuse.
- Stream baseline JSON to/from disk (File.OpenRead/File.Create with
  JsonSerializer) so large baselines never materialize as one giant
  in-memory string.
- Enrich the fail-fast 'missing scenario' output with the eval path and
  short prompt/target SHA prefixes so it is actionable when scenario
  names collide across eval files.
- Add a test locking in recursive (nested-directory) fixture hashing.

563 tests pass.

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

* Address second round of PR review comments

- Skip top-level reparse points in EnumerateCopiedFixtures (not just
  nested ones) so a top-level symlink/junction can't cause hashing of
  data outside the eval directory; code now matches the docstring.
- Record uses first-writer-wins (TryAdd) instead of overwriting, so a
  scenario identity recorded by multiple parallel targets yields a
  deterministic --baseline-out regardless of completion order.
- Persist the baseline judge result to the session DB even when the
  baseline is reused, so the registered 'baseline-reused' session record
  is complete for downstream investigation tooling (pairwise was already
  saved; the judge result was incorrectly gated on a fresh run).
- Add first-writer-wins test (564 pass).

Note: BaselineStore stays internal — the test project already has
InternalsVisibleTo, so it compiles.

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

* Pin MessagePack to 2.5.301 to fix NU1903 vulnerability

The transitive MessagePack 2.5.198 (via GitHub.Copilot.SDK -> StreamJsonRpc)
has a high-severity vulnerability (GHSA-hv8m-jj95-wg3x) that fails the build
under TreatWarningsAsErrors. Pin a direct reference to the patched 2.5.301.

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

* Address review: bare-filename fixture hashing, clone reused baseline, consistent token attribution

- ComputeInputsSha: normalize evalPath via Path.GetFullPath so a bare filename
  still hashes sibling fixtures (avoids TargetSha collisions / unsafe reuse).
- RunMetrics.Clone(): per-run copy with fresh collections; reuse paths now clone
  the cached baseline so concurrent evaluations never share a mutable instance.
- Pairwise judge tokens attributed to both compared runs in every mode (the
  baseline clone makes this safe), keeping token deltas comparable across
  --baseline-from modes.
- Reword Record first-writer-wins doc to describe the within-run stabilization
  guarantee rather than order-independence.
- Add tests for bare-filename fixture hashing and clone isolation (566 pass).

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-06-12 16:18:46 +02:00
Jan Krivanek 92954153dd Bump GitHub.Copilot.SDK to 0.3.0 and address breaking changes (#659)
* Bump GitHub.Copilot.SDK to 0.3.0 and address breaking changes

- GitHub.Copilot.SDK 0.2.2 → 0.3.0
- YamlDotNet 16.3.0 → 17.1.0
- Vecc.YamlDotNet.Analyzers.StaticGenerator 16.3.0 → 17.0.0
- Nerdbank.MessagePack pinned to 1.1.62 (GHSA-2cwq-pwfr-wcw3)

Breaking changes addressed:
- LocalSessionFsHandler now extends abstract SessionFsProvider
  (simplified method signatures with raw parameters)
- McpServers config uses McpStdioServerConfig instead of Dictionary<string,object>
- SessionFsSetProviderRequestConventions → SessionFsSetProviderConventions
- PermissionRequestResultKind.DeniedByRules → .UserNotAvailable (obsolete)
- Mtime/Birthtime now DateTimeOffset instead of string
- Entry/EntryType → SessionFsReaddirWithTypesEntry/EntryType

* Address review: validate MCP server type, suppress YamlDotNet trim warning

- Reject non-stdio MCP server types with a clear error message instead
  of silently treating them as stdio.
- Suppress IL2104 (YamlDotNet 17.x not fully trim-compatible) so that
  PublishAot native builds succeed.
2026-05-18 14:47:49 +02:00
Jan Krivanek b9d343f3b8 Isolate failures during eval and judging (#611) 2026-05-05 14:40:46 +00:00
Viktor Hofer 3fff1f2573 Skill-validator: perf, cancellation, and reliability improvements (#482)
* Skill-validator: perf, cancellation, and reliability improvements

Performance:
- Cache skill-validator-dist.tar.gz in CI (evaluation.yml and skill-check.yml)
  to skip .NET SDK setup, build, and tar on cache hit. Evaluate and comment-on-pr
  jobs restore from cache with artifact download fallback.
- Replace SemaphoreSlim with Lock in SessionDatabase for lighter-weight
  synchronous locking.

Cancellation:
- Thread CancellationToken from System.CommandLine entry point through the
  entire evaluate call chain: Run -> EvaluateTarget -> EvaluateSkill/EvaluateAgent
  -> ExecuteScenario/ExecuteAgentScenario -> ExecuteRun/ExecuteAgentRun, plus
  EvaluateSkillNoise -> ExecuteNoiseTest.
- Add CancellationToken parameter to AgentRunner.RunAgent, Judge.JudgeRun,
  PairwiseJudge.Judge, and OverfittingJudge.Analyze, forwarding to
  RetryHelper and LlmSession.SendAsync respectively.
- Pass CancellationToken to all ConcurrencyLimiter.RunAsync call sites so
  semaphore waits are cancellable.
- Re-throw OperationCanceledException at the top-level skill evaluation loop
  so Ctrl+C / CI cancellation propagates instead of being swallowed as a
  skill failure.

Reliability:
- Smart timeline truncation in Judge and PairwiseJudge: error events
  (session.error, runner.error) are always preserved during truncation
  regardless of position, with a summary marker showing event type breakdown.
- Add jitter (±25%) to RetryHelper exponential backoff to prevent thundering
  herd on rate-limit recovery.
- Add CancellationToken parameter to ConcurrencyLimiter.RunAsync<T>.
- Add ConcurrencyLimiter tests (6 tests) and RetryHelper jitter tests (2 tests).

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

* Address PR review feedback

- RetryHelper: re-clamp delay after jitter so it never exceeds the
  remaining budget or MaxSingleDelayMs.
- Judge/PairwiseJudge: handle edge case where error count alone exceeds
  MaxTimelineEvents by trimming errors with a summary instead of
  unconditionally appending all of them.

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

* Add high-variance detection and refactor FailureKind to enum

- Add Statistics.CoefficientOfVariation() for measuring score consistency
- Flag scenarios with CV > 0.5 as high-variance in ScenarioComparison
- Show console warning and markdown footnote for high-variance scenarios
- Include high-variance scenario names in Comparator verdict reason
- Refactor FailureKind from magic strings to strongly-typed enum with
  JsonStringEnumMemberName attributes for backwards-compatible JSON output
- Add 6 new CV unit tests

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

---------

Co-authored-by: Viktor Hofer <vihofer@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-03-31 13:58:28 +02:00
Copilot d8d3f3d369 Fix duplicate entries in Skills Loaded (and Agents Invoked) columns of evaluation table (#462)
* Initial plan

* Fix duplicate entries in Skills Loaded column of evaluation table

Agent-Logs-Url: https://github.com/dotnet/skills/sessions/2430176d-bf62-461d-bb79-781e34d7d084

Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>

* Remove accidentally committed .nuget binary

Agent-Logs-Url: https://github.com/dotnet/skills/sessions/2430176d-bf62-461d-bb79-781e34d7d084

Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>
2026-03-30 10:56:58 +00:00
Jan Krivanek c94bc5cdc3 Add Ability to author agent evals (#436)
* Add ability to author agent evals

* Adjust
2026-03-25 08:08:58 +01:00
Jan Krivanek 22542a5d75 Track and show subagents invocations (#427)
* Show subagents invocations

* Address PR feedback: fix XML doc and sanitize agent names in markdown

- Update ExtractSubagentActivation XML doc to accurately describe that
  all subagent event types are counted (not just Started/Completed)
- Add markdown table cell sanitization for agent names in FormatSubagentCell
  to prevent injection via pipe chars or newlines in plugin-controlled content
- Extract SanitizeMarkdownTableCell helper for reuse
2026-03-24 11:01:20 +00:00
Viktor Hofer b92dde1854 Refactor skill-validator to vertical slice architecture (#429)
Reorganize the skill-validator project from layer-based grouping
(Commands/, Services/, Models/, Utilities/) to feature-based slices
(Check/, Evaluate/, Consolidate/, Shared/).

Key changes:
- Check/: CheckCommand, SkillProfiler, AgentProfiler, PluginValidator,
  ExternalDependencyChecker, ReferenceScanner
- Evaluate/: EvaluateCommand, RejudgeCommand (now a subcommand of
  evaluate), AgentRunner, Judge, PairwiseJudge, and other eval services
- Consolidate/: ConsolidateCommand
- Shared/: Models, SkillDiscovery, PluginParser (extracted from
  PluginValidator), Reporter, and utilities

Split PluginValidator: parsing helpers (ParsePluginJson,
TryGetSafeSubdirectory) moved to Shared/PluginParser.cs; validation
logic stays in Check/PluginValidator.cs.

Move RawFrontmatter and RawAgentFrontmatter types from EvalSchema into
SkillDiscovery since they are only consumed by discovery.

Wire RejudgeCommand as subcommand: skill-validator evaluate rejudge.

Co-authored-by: Viktor Hofer <vihofer@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-03-24 09:41:55 +00:00