Skip to content

Add general .NET vectorization skill - #1045

Merged
AbhitejJohn merged 4 commits into
mainfrom
tannergooding-port-vectorization-skill
Aug 26, 2026
Merged

Add general .NET vectorization skill#1045
AbhitejJohn merged 4 commits into
mainfrom
tannergooding-port-vectorization-skill

Conversation

@tannergooding

Copy link
Copy Markdown
Member

Replaces #1044, recreated from a branch hosted directly in dotnet/skills so the full CI workflow can run.

Ports dotnet/runtime's vectorization guidance into the general dotnet-advanced plugin and retires the experimental SIMD skill.

The port keeps portable API selection, safe span loads and stores, self-contained width dispatch, vectorized remainder handling, fallback validation, and measured wider-width decisions aligned with the official .NET guidance. It also adds ten stimuli covering review, authoring, framework reuse, portability, memory safety, and routing boundaries.

Targeted local model runs exposed several places where concise guidance was not enough:

  • Claude Opus 5 produced the intended safe span and vector-tail shape, while GPT-5.5 initially used LoadUnsafe/StoreUnsafe with a scalar epilogue. Adding the small safe inner-loop template changed the GPT-5.5 result to Vector128.Create/CopyTo with a preloaded overlapping vector tail.
  • Claude Opus 5, GPT-5.5, Gemini 3.1 Pro, and MAI Code 1.1 Flash initially used the naive Vector256 && Length >= Count followed by Vector128 && Length >= Count cascade. The skill now explicitly defers to the official comprehensive templates and requires each widest-first hardware block to select its vector helper or dedicated small-input helper and return. Gemini and MAI reruns then produced that self-contained dispatch shape.
  • Claude Opus 5, GPT-5.5, and MAI migrated the broken SumValues fixture from managed-reference loading to safe span creation with a masked vector tail. Gemini retained MemoryMarshal.GetReference/LoadUnsafe despite marking its own result as passing, which motivated deterministic graders for safe loading and fallback guards rather than relying only on a judge rubric.
  • Review probes also showed that some initial rubrics would have rewarded repairing handwritten SIMD when an existing accelerated span operation was the better answer. Those rubrics now prefer framework reuse first and grade an explicit-loop repair only when it remains necessary.

These were targeted local stimulus runs rather than the official Vally comparison; the PR evaluation remains the source of truth for measured baseline delta.

Closes #894.

Note

This pull request description was drafted with GitHub Copilot.

tannergooding and others added 2 commits August 24, 2026 13:12
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings August 24, 2026 20:40
@github-actions

github-actions Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Skill Coverage Report

Plugin Skill Covered Coverage
dotnet-experimental exp-simd-vectorization error
dotnet-advanced csharp-scripts 7/22 31.8%
dotnet-advanced dotnet-pinvoke 8/28 28.6%
Uncovered: dotnet-advanced/csharp-scripts
  • [Validation] dotnet --version reports 10.0 or later (or fallback path is used) (line 274)
  • [Validation] If the app uses #:include, #:exclude, or transitive directives from included files, dotnet --version reports SDK 10.0.300 or later (line 275)
  • [Validation] The app compiles without errors (can be checked explicitly with dotnet build <file>.cs) (line 276)
  • [Validation] Multi-file apps include every required helper file and exclude unintended matches (line 278)
  • [Validation] App files and cached artifacts are cleaned up after the session (line 279)
  • [Pitfall] #:package without a version (line 286)
  • [Pitfall] #:property with wrong syntax (line 287)
  • [Pitfall] Directives placed after C# code (line 288)
  • [Pitfall] Helper file is not compiled (line 289)
  • [Pitfall] Shared file needs an assembly boundary (line 290)
  • [Pitfall] Broad include pulls in unrelated files (line 291)
  • [Pitfall] Duplicate directives in included files (line 292)
  • [Pitfall] Reflection-based JSON serialization fails (line 293)
  • [WorkflowStep] Step 5: Clean up (line 199)
  • [CodePattern] [MSBuild] (line 93)
Uncovered: dotnet-advanced/dotnet-pinvoke
  • [Validation] Calling convention specified if targeting Windows x86; omitted otherwise (line 424)
  • [Validation] String encoding is explicit — no reliance on defaults or CharSet.Auto (line 425)
  • [Validation] Memory ownership is documented and matched (who allocates, who frees, with what) (line 426)
  • [Validation] SafeHandle used for all native handles (no raw IntPtr escaping the interop layer) (line 427)
  • [Validation] Delegates passed as callbacks are rooted to prevent GC collection (line 428)
  • [Validation] SetLastError/SetLastPInvokeError set for APIs that use OS error codes (line 429)
  • [Validation] Struct layout matches native (packing, alignment, field order) (line 430)
  • [Validation] CLong/CULong used for C long/unsigned long in cross-platform code (line 431)
  • [Validation] If using CLong/CULong with LibraryImport, [assembly: DisableRuntimeMarshalling] is applied (line 432)
  • [Validation] No bool without explicit MarshalAs — always specify UnmanagedType.Bool (4-byte) or UnmanagedType.U1 (1-byte) to ensure normalization across the language boundary. (line 433)
  • [WorkflowStep] Step 5: Establish Memory Ownership (line 168)
  • [WorkflowStep] Step 6: Use SafeHandle for Native Handles (line 224)
  • [WorkflowStep] Step 7: Handle Errors (line 261)
  • [CodePattern] [256] (line 176)
  • [CodePattern] [UnmanagedFunctionPointer] (line 300)
  • [CodePattern] [Out] (line 144)
  • [CodePattern] [UnmanagedCallConv] (line 111)
  • [CodePattern] [MarshalAs] (line 144)
  • [CodePattern] sealed (line 228)
  • [CodePattern] [UnmanagedCallersOnly] (line 281)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR retires the experimental exp-simd-vectorization skill/eval and introduces a new general-purpose .NET SIMD vectorization skill under the dotnet-advanced plugin, along with a new evaluation suite and fixtures to cover authoring and review scenarios.

Changes:

  • Removed the dotnet-experimental/exp-simd-vectorization skill and its associated eval + fixtures.
  • Added dotnet-advanced/vectorization skill content with detailed guidance aligned to the official .NET SIMD guidance/templates.
  • Added a new tests/dotnet-advanced/vectorization eval with a set of targeted fixtures/projects, and updated CODEOWNERS + plugin README indexing.
Show a summary per file
File Description
tests/dotnet-experimental/exp-simd-vectorization/fixtures/simd-tensor-primitives-product.cs Removed experimental fixture as part of retiring the eval.
tests/dotnet-experimental/exp-simd-vectorization/fixtures/simd-tensor-primitives-minmax.cs Removed experimental fixture as part of retiring the eval.
tests/dotnet-experimental/exp-simd-vectorization/fixtures/simd-no-opportunity-catalog.cs Removed experimental fixture as part of retiring the eval.
tests/dotnet-experimental/exp-simd-vectorization/fixtures/simd-bit-reverser.cs Removed experimental fixture as part of retiring the eval.
tests/dotnet-experimental/exp-simd-vectorization/eval.yaml Removed the experimental eval definition.
plugins/dotnet-experimental/skills/exp-simd-vectorization/SKILL.md Removed the experimental SIMD skill content.
tests/dotnet-advanced/vectorization/fixtures/ZeroBytes.cs Added a new fixture for vectorization review/edge-case detection.
tests/dotnet-advanced/vectorization/fixtures/TailSearch.cs Added a new fixture for tail-handling correctness/memory-safety review.
tests/dotnet-advanced/vectorization/fixtures/LastMatch.cs Added a new fixture for managed-reference safety review.
tests/dotnet-advanced/vectorization/fixtures/AsciiLetters.cs Added a new fixture for portability/runtime-correctness review.
tests/dotnet-advanced/vectorization/fixtures/ContainsZero.cs Added a new fixture for architecture-consistent behavior review.
tests/dotnet-advanced/vectorization/fixtures/sum-values/SumValues.csproj Added runnable project fixture for a reduction repair task.
tests/dotnet-advanced/vectorization/fixtures/sum-values/SumValues.cs Added reduction fixture source for a repair task.
tests/dotnet-advanced/vectorization/fixtures/sum-values/Program.cs Added runnable validation harness for the reduction fixture.
tests/dotnet-advanced/vectorization/fixtures/product/Product.csproj Added runnable project fixture for TensorPrimitives-based optimization.
tests/dotnet-advanced/vectorization/fixtures/product/Product.cs Added product implementation fixture for optimization.
tests/dotnet-advanced/vectorization/fixtures/product/Program.cs Added runnable validation harness for product fixture behavior.
tests/dotnet-advanced/vectorization/fixtures/conditional-increment/ConditionalIncrement.csproj Added runnable project fixture for conditional increment authoring task.
tests/dotnet-advanced/vectorization/fixtures/conditional-increment/Program.cs Added runnable validation harness for conditional increment behavior.
tests/dotnet-advanced/vectorization/fixtures/conditional-increment/ConditionalIncrement.cs Updated fixture implementation shape (static class, formatting) for the new eval.
tests/dotnet-advanced/vectorization/fixtures/widen-clamp/WidenClamp.csproj Added runnable project fixture for widening dispatch task.
tests/dotnet-advanced/vectorization/fixtures/widen-clamp/Program.cs Added runnable validation harness for widening clamp behavior.
tests/dotnet-advanced/vectorization/fixtures/widen-clamp/ClampNegative.cs Added baseline Vector128 implementation fixture for widening-to-Vector256 task.
tests/dotnet-advanced/vectorization/eval.yaml Added the new vectorization evaluation with multiple targeted stimuli + graders.
plugins/dotnet-advanced/skills/vectorization/SKILL.md Added the new general vectorization skill guidance under dotnet-advanced.
plugins/dotnet-advanced/README.md Indexed the new vectorization skill in the plugin README.
.github/CODEOWNERS Reassigned ownership from experimental paths to the new dotnet-advanced skill/test paths.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 27/27 changed files
  • Comments generated: 0
  • Review effort level: Lite

@github-actions github-actions Bot added the pr-state/ready-for-eval PR is mergeable and awaiting evaluation label Aug 24, 2026
github-actions Bot added a commit that referenced this pull request Aug 24, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📊 Skill Evaluation Results

2 model/skill results across 1 skill and 2 models — ✅ 2 improved, ➖ 0 not proven improved, ⚠️ 0 invalid or underpowered, 📉 0 preference losses (report only).

Measurement identity: evaluated commit 021c7df89a150043f4bb7b51b2e4067ebb63aeec; 2 judge models.

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 p ≤ 0.05. Repeated runs measure reliability only.

Skill Model Verdict Gate evidence Overfit Warnings Next action
vectorization claude-sonnet-4.6 ✅ Improved n=10; 8W/1T/1L; d=9; p=0.020; net +70.0% 🟡 0.43 Activation: isolated 5/9; plugin 4/9 Fix activation gaps; Review overfit evidence.
vectorization gpt-5.6-luna ✅ Improved n=10; 8W/2T/0L; d=8; p=0.004; net +80.0% 🟡 0.32 Activation: isolated 9/9; plugin 5/9 Fix activation gaps; Review overfit evidence.
ℹ️ How to read this report
  • ✅ Improved — the result passed both the statistical gate and the 20% practical net-win floor.
  • ➖ Not proven improved — the result is valid but did not pass both gates. This is not automatically a regression.
  • ⚠️ Invalid / underpowered — the gate withheld a quality verdict. Fix the measurement before judging the skill.
  • 📉 Preference loss — the LLM judge credibly preferred baseline. It is report-only, not objective completion proof.
  • Gate evidencen distinct-stimulus votes, W/T/L stimulus votes, d discordant votes, exact one-sided p, and net win. The p value applies to one model/skill result; no matrix-wide multiple-comparison correction is applied.
  • Overfit — overfitting-judge severity (✅ Low, 🟡 Moderate, 🔴 High, — none) and score.
  • Warnings — activation, timeout, retry recovery, or unresolved comparison conditions that need attention.
  • Do not add repeated runs to increase statistical power. Do not add stimuli after seeing a near-pass unless the new breadth is predeclared for a new experiment.
✅ Improved — vectorization (claude-sonnet-4.6)

Why: Net win +70.0% (8W/1T/1L over 10 stimulus vote(s), sign test p=0.020), mean preference +40.0% across 10 paired run(s) — credibly better

Next action: Fix activation gaps; Review overfit evidence.

State: VALID_PASS (credible_preference_improvement)

Gate evidence: n=10; 8W/1T/1L; d=9; p=0.020; net +70.0%

Warnings: Activation: isolated 5/9; plugin 4/9

Overfit: Moderate (score 0.43)

Repeated-run reliability (not used by the gate): 10 paired runs (8W/1T/1L).

Weak or warning scenarios:

Scenario Net win Δ Pref Runs (W/T/L)
▲ Detect architecture-specific behavior change +100.0% +40.0% 1/0/0
▲ Detect empty-span reference access +100.0% +40.0% 1/0/0
▲ Detect out-of-range backwards reference +100.0% +100.0% 1/0/0
= Detect unsigned tail offset underflow +0.0% +0.0% 0/1/0
▲ Detect unsupported vector element type +100.0% +40.0% 1/0/0
▼ Preserve product empty-input contract -100.0% -100.0% 0/0/1

Illustrative judge evidence:

  • Detect unsigned tail offset underflow: Position-swap inconsistent (forward: A, reverse: B). Defaulting to tie.

This is one example, not the aggregate verdict. Open Full Results for every judgment.

✅ Improved — vectorization (gpt-5.6-luna)

Why: Net win +80.0% (8W/2T/0L over 10 stimulus vote(s), sign test p=0.004), mean preference +44.0% across 10 paired run(s) — credibly better

Next action: Fix activation gaps; Review overfit evidence.

State: VALID_PASS (credible_preference_improvement)

Gate evidence: n=10; 8W/2T/0L; d=8; p=0.004; net +80.0%

Warnings: Activation: isolated 9/9; plugin 5/9

Overfit: Moderate (score 0.32)

Repeated-run reliability (not used by the gate): 10 paired runs (8W/2T/0L).

Weak or warning scenarios:

Scenario Net win Δ Pref Runs (W/T/L)
= Detect architecture-specific behavior change +0.0% +0.0% 0/1/0
▲ Detect empty-span reference access +100.0% +40.0% 1/0/0
▲ Detect out-of-range backwards reference +100.0% +100.0% 1/0/0
▲ Detect unsigned tail offset underflow +100.0% +40.0% 1/0/0
= Ignore unrelated parser performance request +0.0% +0.0% 0/1/0

Illustrative judge evidence:

  • Detect architecture-specific behavior change: Position-swap inconsistent (forward: A, reverse: tie). Defaulting to tie.

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
📊 Session Analytics (preview) -- aggregated metrics across evaluation sessions

@github-actions github-actions Bot added waiting-on-review PR state label and removed pr-state/ready-for-eval PR is mergeable and awaiting evaluation labels Aug 24, 2026
@github-actions

Copy link
Copy Markdown
Contributor

✅ Evaluation passed for 021c7df. cc @webreidi @AbhitejJohn @jeffschw @artl93 — please review.

@AbhitejJohn

Copy link
Copy Markdown
Collaborator

Thanks a bunch for the thorough skill and eval coverage here. I reviewed the skill, fixtures, and eval design across Claude, GPT, and Gemini. The SIMD guidance is strong overall. I found three areas worth adjusting:

  1. Managed-reference rule and LastMatch fixture

    SKILL.md currently says not to create a managed reference past the end of an object, and the LastMatch rubric treats its one-past-end reference as inherently unsafe.

    ECMA-335 permits a managed reference to the address where an element just past the end of an array would be stored, provided it isn't dereferenced. The .NET runtime addendum states this directly:

    "Managed pointers which point at the address just past the end of an object, or the address where an element just past the end of an array would be stored, are permitted but not dereferenceable."

    Sources: ECMA-335 II.14.4.2 and the runtime specification addendum.

    Since LastMatch accepts an arbitrary ReadOnlySpan<int>, span-end and array-end aren't always the same address. Could we avoid depending on that distinction and make the fixture demonstrate the unambiguous defect: subtracting from element zero and creating a managed reference before the start of the span? The rubric could then ask the agent to identify that final subtraction and recommend subtracting only when another element remains.

  2. The evals sometimes verify an exact implementation instead of the required behavior

    Several hard graders require or reject specific source tokens:

    • ConditionalSelect requires one tail strategy for SumValues.
    • ToZeroSmall requires a private helper to have one exact name.
    • Checks for Vector128.Create and CopyTo reject other correct load/store forms.
    • Negative checks for LoadUnsafe, MemoryMarshal, System.Runtime.Intrinsics.X86, and Vector256 can reject valid, bounds-safe implementations. They can also match comments rather than executable code.

    For example, a solution can vectorize every full block and use a correct scalar epilogue without using ConditionalSelect. It would satisfy the prompt's requirement to retain SIMD, pass the boundary and overflow tests, and still fail the token grader. Similarly, renaming ToZeroSmall to HandleSmallInput would fail without changing behavior.

    This also conflicts with the repo's eval-authoring guidance in .agents/skills/create-skill-test/SKILL.md:

    "Test outcomes, not methods"
    "Accept any valid approach."

    The existing run-command graders already provide stronger evidence. They execute exact-width, nonmultiple, empty, large, hardware-disabled, and AVX2-disabled paths. Could we remove the technique and private-name checks, then use the prompt rubric for the remaining outcomes: bounds safety, scalar equivalence, portable fallback, no double processing, and retained SIMD for full-vector inputs?

    I'd also combine conditional rubric pairs such as "prefer an existing span API" and "if retaining explicit SIMD..." into one independently evaluable item that accepts either valid result.

  3. Make the framework-first rule decisive

    In the Product scenario, a skilled trajectory selected custom SIMD while another trajectory used TensorPrimitives. The skill already recommends high-level APIs, but the current wording still leaves room to continue into handwritten vector code after a matching framework operation is found.

    Could we make that rule explicit: use the highest-level API that preserves the contract, then stop? For explicit SIMD, it would also help to require the response to name the framework APIs considered and explain why none preserves the required behavior. Wider Vector256<T>, Vector512<T>, or Vector<T> paths should remain measurement-driven.

After these skill and eval changes, a fresh official evaluation would give useful evidence for activation and behavior on the revised content.

(Copilot, commenting on Abhitej's behalf.)

@tannergooding

Copy link
Copy Markdown
Member Author

Managed-reference rule and LastMatch fixture

This is an area where even if the edge is technically allowed, it remains an inherently unsafe and dangerous operation that leads to a large number of bugs and potential security issues, especially where lesser LLMs infer it to be extendable to other cases or make mistakes.

Our official guidance, especially with the work around unsafe evolution and reducing the use of unnecessary unsafe is that it should essentially never be done and so having the skill reject it is ultimately better.

For example, a solution can vectorize every full block and use a correct scalar epilogue without using ConditionalSelect. It would satisfy the prompt's requirement to retain SIMD, pass the boundary and overflow tests, and still fail the token grader. Similarly, renaming ToZeroSmall to HandleSmallInput would fail without changing behavior.

The ToZeroSmall is a case where it should probably be relaxed.

The "scalar epilogue" would be a failure of the skill to correctly vectorize the method (i.e. is invalid). Leaving scalar epilogues is essentially never required and is something that a skill that is explicitly meant to vectorize the code should not be leaving behind.

I'll look at doing cleanup for other cases mentioned as appropriate.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 25, 2026 15:49
@tannergooding

Copy link
Copy Markdown
Member Author

Did some cleanup.

The main thing worth pointing out is that the requirements for Create (over Load), CopyTo (over Store), and the vectorized tails are a matter of safety and overall correctness. This is a known area where models try to do the "wrong thing" because they have wrong historical assumptions (and is something that Gemini and other models were consistently getting wrong without the guidance).

Even where these things are "technically equivalent", it is explicitly unnecessary risk and complexity that we want to push models away from for such an advanced skill. The simpler safe thing is the correct thing and what we want to push as the idiomatic way to handle it, how we want existing code fixed up to be as well to reduce the known risk and bugs that occur.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

  • Files reviewed: 27/27 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@github-actions github-actions Bot added pr-state/ready-for-eval PR is mergeable and awaiting evaluation and removed waiting-on-review PR state label labels Aug 25, 2026
github-actions Bot added a commit that referenced this pull request Aug 25, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📊 Skill Evaluation Results

2 model/skill results across 1 skill and 2 models — ✅ 0 improved, ➖ 2 not proven improved, ⚠️ 0 invalid or underpowered, 📉 0 preference losses (report only).

Measurement identity: evaluated commit 9f20ab0ecd1a42d79142cab8d978b2e05b681e4f; 2 judge models.

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 p ≤ 0.05. Repeated runs measure reliability only.

Skill Model Verdict Gate evidence Overfit Warnings Next action
vectorization claude-sonnet-4.6 ➖ Not proven improved n=10; 3W/2T/5L; d=8; p=0.363; net -20.0% 🟡 0.44 Activation: isolated 5/9; plugin 4/9 Inspect tied or lost stimuli and fix inconsistent skill behavior.
vectorization gpt-5.6-luna ➖ Not proven improved n=10; 6W/3T/1L; d=7; p=0.063; net +50.0% 🟡 0.33 Activation: isolated 7/9; plugin 6/9 Inspect tied or lost stimuli and fix inconsistent skill behavior.
ℹ️ How to read this report
  • ✅ Improved — the result passed both the statistical gate and the 20% practical net-win floor.
  • ➖ Not proven improved — the result is valid but did not pass both gates. This is not automatically a regression.
  • ⚠️ Invalid / underpowered — the gate withheld a quality verdict. Fix the measurement before judging the skill.
  • 📉 Preference loss — the LLM judge credibly preferred baseline. It is report-only, not objective completion proof.
  • Gate evidencen distinct-stimulus votes, W/T/L stimulus votes, d discordant votes, exact one-sided p, and net win. The p value applies to one model/skill result; no matrix-wide multiple-comparison correction is applied.
  • Overfit — overfitting-judge severity (✅ Low, 🟡 Moderate, 🔴 High, — none) and score.
  • Warnings — activation, timeout, retry recovery, or unresolved comparison conditions that need attention.
  • Do not add repeated runs to increase statistical power. Do not add stimuli after seeing a near-pass unless the new breadth is predeclared for a new experiment.
➖ Not proven improved — vectorization (claude-sonnet-4.6)

Why: Net win -20.0% (3W/2T/5L over 10 stimulus vote(s), sign test p=0.363), mean preference -14.0% across 10 paired run(s) — no improvement

Next action: Inspect tied or lost stimuli and fix inconsistent skill behavior.

State: VALID_NO_CHANGE (no_credible_preference_change)

Gate evidence: n=10; 3W/2T/5L; d=8; p=0.363; net -20.0%

Warnings: Activation: isolated 5/9; plugin 4/9

Overfit: Moderate (score 0.44)

Repeated-run reliability (not used by the gate): 10 paired runs (3W/2T/5L).

Weak or warning scenarios:

Scenario Net win Δ Pref Runs (W/T/L)
= Detect architecture-specific behavior change +0.0% +0.0% 0/1/0
▼ Detect empty-span reference access -100.0% -40.0% 0/0/1
▼ Detect out-of-range backwards reference -100.0% -100.0% 0/0/1
▲ Detect unsigned tail offset underflow +100.0% +40.0% 1/0/0
▼ Detect unsupported vector element type -100.0% -40.0% 0/0/1
= Ignore unrelated parser performance request +0.0% +0.0% 0/1/0
▼ Preserve product empty-input contract -100.0% -100.0% 0/0/1
▼ Vectorize conditional increment safely -100.0% -40.0% 0/0/1

Illustrative judge evidence:

  • Detect architecture-specific behavior change: Position-swap inconsistent (forward: B, reverse: A). Defaulting to tie.

This is one example, not the aggregate verdict. Open Full Results for every judgment.

➖ Not proven improved — vectorization (gpt-5.6-luna)

Why: Net win +50.0% (6W/3T/1L over 10 stimulus vote(s), sign test p=0.063), mean preference +26.0% across 10 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: VALID_NO_CHANGE (no_credible_preference_change)

Gate evidence: n=10; 6W/3T/1L; d=7; p=0.063; net +50.0%

Warnings: Activation: isolated 7/9; plugin 6/9

Overfit: Moderate (score 0.33)

Repeated-run reliability (not used by the gate): 10 paired runs (6W/3T/1L).

Weak or warning scenarios:

Scenario Net win Δ Pref Runs (W/T/L)
= Detect architecture-specific behavior change +0.0% +0.0% 0/1/0
▲ Detect empty-span reference access +100.0% +40.0% 1/0/0
= Detect out-of-range backwards reference +0.0% +0.0% 0/1/0
= Detect unsigned tail offset underflow +0.0% +0.0% 0/1/0
▼ Preserve product empty-input contract -100.0% -40.0% 0/0/1

Illustrative judge evidence:

  • Detect architecture-specific behavior change: Position-swap inconsistent (forward: tie, reverse: A). Defaulting to tie.

This is one example, not the aggregate verdict. Open Full Results for every judgment.

🔍 Full Results - all metrics and investigation details

To investigate non-passing or warning results, paste this to your AI coding agent:

For PR 1045 in dotnet/skills, download eval artifacts with gh run download 32872418037 --repo dotnet/skills --pattern "vally-results-*" --dir ./eval-results, then fetch https://raw.githubusercontent.com/dotnet/skills/9f20ab0ecd1a42d79142cab8d978b2e05b681e4f/eng/vally-adapter/InvestigatingResults.md and follow it. Classify each result as measurement-invalid, underpowered, not-proven, preference-loss, or passing-with-warning. Use stateReason, result accounting, weak scenarios, and judge evidence to give the cause and exact next fix.

▶ Sessions Visualisation -- interactive replay of all evaluation sessions
📊 Session Analytics (preview) -- aggregated metrics across evaluation sessions

@github-actions github-actions Bot added waiting-on-review PR state label and removed pr-state/ready-for-eval PR is mergeable and awaiting evaluation labels Aug 25, 2026

@AbhitejJohn AbhitejJohn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the detailed responses and updates. Taking your comment as the authoritative design intent, I agree that the skill can prohibit one-past-end managed references as a conservative policy, even though the runtime permits them when they are not dereferenced.

I think the current text still differs from that intent. SKILL.md says a collection can leave the reference untracked, and the LastMatch rubric requires that explanation. Would you be open to stating this as an intentional safety policy - permitted by the runtime, but prohibited by this guidance because it is fragile and easy to misuse - without attributing it to lost GC tracking?

On the eval checks, I agree that Vector128.IsHardwareAccelerated, Vector128.Create(span), CopyTo(span), and the prohibition on unmeasured wider paths enforce requirements that the skill states explicitly. In particular, the disabled-intrinsics test proves equivalent results, but it cannot prove that the scalar branch ran because Vector128 operations have a correct software fallback. Keeping the Vector128.IsHardwareAccelerated check makes sense.

My remaining eval question is specifically about requiring the literal ConditionalSelect token. A tail can remain fully vectorized and avoid duplicate work by masking the overlapping vector with bitwise operations. Conceptually:

Vector128<int> tail = Vector128.Create(finalFullSpan);
Vector128<int> mask = CreateMaskForUnprocessedLanes(overlap);
sum += tail & mask;

Here, lanes already processed by the main loop are zero in mask, and lanes that belong to the remainder are all bits set. This is equivalent for the reduction to ConditionalSelect(mask, tail, Vector128<int>.Zero). It has no scalar epilogue and does not count overlapping lanes twice.

Would it make sense for the rubric to accept either ConditionalSelect or an equivalent vector masking operation, while the existing nonmultiple-length execution cases verify that no lanes are duplicated?

I also read the latest official result as 0 improved, 2 not proven improved. That does not prove a correctness regression, but it also does not establish an improvement at this head.

(Copilot, commenting on Abhitej's behalf.)

@github-actions github-actions Bot removed the waiting-on-review PR state label label Aug 25, 2026
@github-actions github-actions Bot added the ready-to-merge PR state label label Aug 25, 2026
@github-actions

Copy link
Copy Markdown
Contributor

✅ Approved by @AbhitejJohn. cc @dotnet/skills-merge-approvers — ready to merge.

@tannergooding

Copy link
Copy Markdown
Member Author

I think the current text still differs from that intent. SKILL.md says a collection can leave the reference untracked, and the LastMatch rubric requires that explanation. Would you be open to stating this as an intentional safety policy - permitted by the runtime, but prohibited by this guidance because it is fragile and easy to misuse - without attributing it to lost GC tracking

Have a fix for that up now.

My remaining eval question is specifically about requiring the literal ConditionalSelect token. A tail can remain fully vectorized and avoid duplicate work by masking the overlapping vector with bitwise operations. Conceptually:

The conceptual simplification can actually be worse for codegen, quality, and extensibility. We explicitly recognize and optimize the ConditionalSelect pattern and emit just mask & x if that's allowed and otherwise the general pattern is the one that gets broader and more general lightup for things like embedded masking. We sometimes recognize a bitwise-and as well, but it doesn't have all the same general handling involved and can get morphed in a way that breaks the pattern too. -- Something we'll continue improving over time, but where its a lot simple to push agents down the "golden path".

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 25, 2026 21:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

  • Files reviewed: 27/27 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@github-actions github-actions Bot added pr-state/ready-for-eval PR is mergeable and awaiting evaluation and removed ready-to-merge PR state label labels Aug 25, 2026
github-actions Bot added a commit that referenced this pull request Aug 25, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📊 Skill Evaluation Results

2 model/skill results across 1 skill and 2 models — ✅ 2 improved, ➖ 0 not proven improved, ⚠️ 0 invalid or underpowered, 📉 0 preference losses (report only).

Measurement identity: evaluated commit d4bf2bbf62188496dd63afd4de05d5a0d9f1f0c3; 2 judge models.

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 p ≤ 0.05. Repeated runs measure reliability only.

Skill Model Verdict Gate evidence Overfit Warnings Next action
vectorization claude-sonnet-4.6 ✅ Improved n=10; 7W/2T/1L; d=8; p=0.035; net +60.0% 🟡 0.44 Activation: isolated 4/9; plugin 4/9 Fix activation gaps; Review overfit evidence.
vectorization gpt-5.6-luna ✅ Improved n=10; 6W/4T/0L; d=6; p=0.016; net +60.0% 🟡 0.34 Activation: isolated 8/9; plugin 5/9 Fix activation gaps; Review overfit evidence.
ℹ️ How to read this report
  • ✅ Improved — the result passed both the statistical gate and the 20% practical net-win floor.
  • ➖ Not proven improved — the result is valid but did not pass both gates. This is not automatically a regression.
  • ⚠️ Invalid / underpowered — the gate withheld a quality verdict. Fix the measurement before judging the skill.
  • 📉 Preference loss — the LLM judge credibly preferred baseline. It is report-only, not objective completion proof.
  • Gate evidencen distinct-stimulus votes, W/T/L stimulus votes, d discordant votes, exact one-sided p, and net win. The p value applies to one model/skill result; no matrix-wide multiple-comparison correction is applied.
  • Overfit — overfitting-judge severity (✅ Low, 🟡 Moderate, 🔴 High, — none) and score.
  • Warnings — activation, timeout, retry recovery, or unresolved comparison conditions that need attention.
  • Do not add repeated runs to increase statistical power. Do not add stimuli after seeing a near-pass unless the new breadth is predeclared for a new experiment.
✅ Improved — vectorization (claude-sonnet-4.6)

Why: Net win +60.0% (7W/2T/1L over 10 stimulus vote(s), sign test p=0.035), mean preference +42.0% across 10 paired run(s) — credibly better

Next action: Fix activation gaps; Review overfit evidence.

State: VALID_PASS (credible_preference_improvement)

Gate evidence: n=10; 7W/2T/1L; d=8; p=0.035; net +60.0%

Warnings: Activation: isolated 4/9; plugin 4/9

Overfit: Moderate (score 0.44)

Repeated-run reliability (not used by the gate): 10 paired runs (7W/2T/1L).

Weak or warning scenarios:

Scenario Net win Δ Pref Runs (W/T/L)
= Detect architecture-specific behavior change +0.0% +0.0% 0/1/0
▲ Detect empty-span reference access +100.0% +40.0% 1/0/0
▲ Detect unsigned tail offset underflow +100.0% +40.0% 1/0/0
▼ Detect unsupported vector element type -100.0% -40.0% 0/0/1
= Preserve product empty-input contract +0.0% +0.0% 0/1/0
▲ Reject fragile backwards reference +100.0% +40.0% 1/0/0

Illustrative judge evidence:

  • Detect architecture-specific behavior change: Position-swap inconsistent (forward: A, reverse: B). Defaulting to tie.

This is one example, not the aggregate verdict. Open Full Results for every judgment.

✅ Improved — vectorization (gpt-5.6-luna)

Why: Net win +60.0% (6W/4T/0L over 10 stimulus vote(s), sign test p=0.016), mean preference +30.0% across 10 paired run(s) — credibly better

Next action: Fix activation gaps; Review overfit evidence.

State: VALID_PASS (credible_preference_improvement)

Gate evidence: n=10; 6W/4T/0L; d=6; p=0.016; net +60.0%

Warnings: Activation: isolated 8/9; plugin 5/9

Overfit: Moderate (score 0.34)

Repeated-run reliability (not used by the gate): 10 paired runs (6W/4T/0L).

Weak or warning scenarios:

Scenario Net win Δ Pref Runs (W/T/L)
= Detect empty-span reference access +0.0% +0.0% 0/1/0
= Detect unsigned tail offset underflow +0.0% +0.0% 0/1/0
▲ Detect unsupported vector element type +100.0% +100.0% 1/0/0
= Extend an existing vectorized path +0.0% +0.0% 0/1/0
= Reject fragile backwards reference +0.0% +0.0% 0/1/0

Illustrative judge evidence:

  • Detect empty-span reference access: Both responses reach essentially identical conclusions: correctly identifying the IndexOutOfRangeException from data[0] before the empty check, and correctly noting the vector load is otherwise bounds-safe. Both recommend reordering the empty check. Neither fully satisfies c...

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
📊 Session Analytics (preview) -- aggregated metrics across evaluation sessions

@github-actions github-actions Bot added waiting-on-review PR state label and removed pr-state/ready-for-eval PR is mergeable and awaiting evaluation labels Aug 25, 2026
@AbhitejJohn

Copy link
Copy Markdown
Collaborator

Thanks for the fixes tanner, LGTM. Tagging @artl93 for any additional thoughts. Otherwise, looks good to go.

@github-actions github-actions Bot added ready-to-merge PR state label and removed waiting-on-review PR state label labels Aug 26, 2026
@artl93

artl93 commented Aug 26, 2026

Copy link
Copy Markdown
Member

@AbhitejJohn - If your referring to expertise on vectorization, I cannot comment. I rely on Tanner.

@AbhitejJohn

Copy link
Copy Markdown
Collaborator

@artl93 : Sounds good, merging this in then :) Thanks for the addition Tanner, looking forward to more.

@AbhitejJohn
AbhitejJohn merged commit a3cb4a2 into main Aug 26, 2026
46 checks passed
@AbhitejJohn
AbhitejJohn deleted the tannergooding-port-vectorization-skill branch August 26, 2026 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge PR state label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[skills-eval] dotnet-experimental: 3 skills, 47% pass — 1 to strengthen, 2 keep

4 participants