Add Vally skill-effectiveness experiments and Python test scenarios - #383
Open
LarryOsterman wants to merge 58 commits into
Open
Add Vally skill-effectiveness experiments and Python test scenarios#383LarryOsterman wants to merge 58 commits into
LarryOsterman wants to merge 58 commits into
Conversation
Split from #375: adds the Vally skill-effectiveness experiment framework (run-skill-experiments.ps1, run-all-evals.ps1, generate-skill-experiments.ps1, compare-experiment.mjs), standardized vally eval/experiment yaml and scenarios.yaml for ~40 Python skills, shared Python grader tooling, and harness runner script/dependency updates. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9429ad33-6f31-4ec5-ba00-0f11b5afb82e
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a standardized Vally “skill effectiveness” experiment setup for Python skills under tests/scenarios/, including shared Python grader tooling and updated test-runner dependencies/ignores. This PR is part of the split from #375 and focuses on the Python scenario/eval content (not harness/CI).
Changes:
- Adds shared Node-based graders for Python syntax + basic idiomatic checks under
tests/scenarios/_shared/vally/tools/. - Introduces per-skill Vally
eval.yaml+skill_effectiveness_experiment.yaml(and, for some,skill_effectiveness_eval.yaml) plus updates manyscenarios.yamlmodel defaults. - Updates
tests/package.json(Vally version + package manager metadata) and expandstests/.gitignoreto ignore evaluation output folders.
Reviewed changes
Copilot reviewed 176 out of 178 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/package.json | Bumps @microsoft/vally and adds packageManager metadata for tests tooling. |
| tests/.gitignore | Ignores additional eval output directories under tests/. |
| tests/scenarios/_shared/vally/tools/check-python-syntax.mjs | New shared Python syntax validation grader (supports optional exclude patterns config). |
| tests/scenarios/_shared/vally/tools/check-python-idiomatic.mjs | New shared heuristic Python “idiomatic” checks grader. |
| tests/scenarios/_shared/vally/azure-sdk-python-skill-evaluations.yaml | Aggregates many Python skill evals into a single suite file. |
| tests/scenarios/agent-framework-azure-ai-py/vally/skill_effectiveness_experiment.yaml | Adds skill-vs-baseline experiment definition for this skill. |
| tests/scenarios/agent-framework-azure-ai-py/vally/eval.yaml | Adds Vally eval suite for this skill’s scenarios. |
| tests/scenarios/agent-framework-azure-ai-py/vally/skill_effectiveness_eval.yaml | Adds the skill-effectiveness eval suite definition for this skill. |
| tests/scenarios/agent-framework-azure-ai-py/scenarios.yaml | Sets scenario model config for this skill. |
| tests/scenarios/agent-framework-azure-ai-py/test-output.txt | Adds a captured output artifact for this scenario directory. |
| tests/scenarios/azure-search-documents-py/vally/skill_effectiveness_experiment.yaml | Adds skill-vs-baseline experiment definition for this skill. |
| tests/scenarios/azure-search-documents-py/vally/eval.yaml | Adds Vally eval suite for this skill’s scenarios. |
| tests/scenarios/azure-search-documents-py/scenarios.yaml | Updates scenario model config for this skill. |
| tests/scenarios/azure-eventgrid-py/vally/skill_effectiveness_experiment.yaml | Adds skill-vs-baseline experiment definition for this skill. |
| tests/scenarios/azure-eventgrid-py/vally/eval.yaml | Adds Vally eval suite for this skill’s scenarios. |
| tests/scenarios/azure-eventgrid-py/scenarios.yaml | Updates scenario model config for this skill. |
| tests/scenarios/azure-messaging-webpubsubservice-py/vally/skill_effectiveness_experiment.yaml | Adds skill-vs-baseline experiment definition for this skill. |
| tests/scenarios/azure-messaging-webpubsubservice-py/vally/eval.yaml | Adds Vally eval suite for this skill’s scenarios. |
| tests/scenarios/azure-messaging-webpubsubservice-py/scenarios.yaml | Updates scenario model config for this skill. |
| tests/scenarios/azure-monitor-opentelemetry-py/vally/skill_effectiveness_experiment.yaml | Adds skill-vs-baseline experiment definition for this skill. |
| tests/scenarios/azure-monitor-opentelemetry-py/vally/eval.yaml | Adds Vally eval suite for this skill’s scenarios. |
| tests/scenarios/azure-monitor-opentelemetry-py/scenarios.yaml | Updates scenario model config for this skill. |
| tests/scenarios/pydantic-models-py/vally/syntax-check-config.json | Adds per-scenario syntax-check exclusion config. |
| tests/scenarios/pydantic-models-py/vally/skill_effectiveness_experiment.yaml | Adds skill-vs-baseline experiment definition for this skill. |
| tests/scenarios/pydantic-models-py/scenarios.yaml | Updates scenario model config for this skill. |
| tests/scenarios/fastapi-router-py/vally/syntax-check-config.json | Adds per-scenario syntax-check exclusion config. |
| tests/scenarios/fastapi-router-py/vally/skill_effectiveness_experiment.yaml | Adds skill-vs-baseline experiment definition for this skill. |
| tests/scenarios/azure-storage-queue-py/vally/skill_effectiveness_experiment.yaml | Adds skill-vs-baseline experiment definition for this skill. |
| tests/scenarios/azure-storage-queue-py/vally/eval.yaml | Adds Vally eval suite for this skill’s scenarios. |
| tests/scenarios/azure-storage-queue-py/vally/skill_effectiveness_eval.yaml | Adds the skill-effectiveness eval suite definition for this skill. |
| tests/scenarios/azure-storage-queue-py/scenarios.yaml | Updates scenario model config for this skill. |
| tests/scenarios/m365-agents-py/vally/skill_effectiveness_experiment.yaml | Adds skill-vs-baseline experiment definition for this skill. |
| tests/scenarios/m365-agents-py/vally/eval.yaml | Adds Vally eval suite for this skill’s scenarios. |
| tests/scenarios/m365-agents-py/vally/skill_effectiveness_eval.yaml | Adds the skill-effectiveness eval suite definition for this skill. |
| tests/scenarios/m365-agents-py/scenarios.yaml | Updates scenario model config for this skill. |
| tests/scenarios/azure-ai-language-conversations-py/scenarios.yaml | Adds scenario definitions and mock responses for this skill. |
| tests/scenarios/azure-ai-language-conversations-py/acceptance-criteria.md | Expands/clarifies acceptance criteria with correct/incorrect examples. |
Files not reviewed (1)
- tests/pnpm-lock.yaml: Generated file
This was referenced Jul 15, 2026
Member
Author
…lude-pattern support
richardpark-msft
approved these changes
Jul 16, 2026
richardpark-msft
left a comment
Member
There was a problem hiding this comment.
Some stuff I think is worth fixing:
- Let's use a real python linter (pylint?). Our skills should produce clean code anyways.
- Can you add in a README.md too? There's some handy little workflows in here, hard to discover without a small roadmap.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 178 out of 181 changed files in this pull request and generated 13 comments.
Files not reviewed (1)
- tests/pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (6)
tests/run-all-evals.ps1:314
- The output directory is keyed only by the scenario directory, but
*eval.yamlselects botheval.yamlandskill_effectiveness_eval.yamlfrom the same Rustvally/folder. With the default parallel workers, those evaluations write concurrently to the same directory and each worker can select the other evaluation's newest run as its summary. Include the eval basename in this directory key.
$safeOutDirName = $relativeDir -replace '[\\/:*?"<>|]', "_"
$scenarioOutDir = Join-Path $resolvedResultsRoot $safeOutDirName
tests/compare-experiment.mjs:84
results.jsonlcontains typed records, but this code averages every record as though it were a trial;run-skill-experiments.ps1:1161-1163correctly filters fortrial-result. It also parses the entirerun-summary.jsonlas one JSON value even though the runner reads the last JSONL record. This can dilute scores with zero-valued non-trial records or fail when the summary has multiple lines.
const rows = fs.readFileSync(path.join(dir, 'results.jsonl'), 'utf8')
.split('\n').filter(l => l.trim())
.map(l => JSON.parse(l));
const summary = JSON.parse(fs.readFileSync(path.join(dir, 'run-summary.jsonl'), 'utf8').trim());
tests/scenarios/azure-eventhub-rust/acceptance-criteria.md:184
- This “correct” producer example passes an owned
String, but the producer builder takes the event hub name as&str; the updated skill explicitly records that distinction at.github/plugins/azure-sdk-rust/skills/azure-eventhub-rust/SKILL.md:159(only the consumer takesString). Keepeventhubborrowed here or this acceptance example will not compile.
tests/run-skill-experiments.ps1:7 - The PR description promises a new
generate-skill-experiments.ps1, but no file with that name exists in the repository. Add the announced generator or remove it from the stated framework scope so users are not directed to missing tooling.
Executes skill_effectiveness_experiment.yaml files for all or specified skills.
tests/scenarios/azure-storage-blob-rust/vally/skill_effectiveness_eval.yaml:129
- The stimulus requires
cargo clippy -- -D warnings, but the grader runs non-strictcargo clippy; warning-only code therefore receives a passing command result. Run the same strict command that the prompt requires.
tests/scenarios/azure-keyvault-certificates-rust/vally/eval.yaml:172 - The prompt requires a strict
cargo clippy -- -D warningsrun, but this grader invokes non-strict Clippy and will pass warning-only code. Align the command with the stated completion criterion.
…run-skill-experiments.ps1
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 178 out of 181 changed files in this pull request and generated 3 comments.
Files not reviewed (1)
- tests/pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (3)
tests/compare-experiment.mjs:84
run-summary.jsonlis JSON Lines, but this parses the entire file as one JSON value. Any valid file containing more than one summary record throws before comparison; read nonblank lines and parse the latest record, consistent withrun-all-evals.ps1:354-359.
const summary = JSON.parse(fs.readFileSync(path.join(dir, 'run-summary.jsonl'), 'utf8').trim());
tests/scenarios/azure-eventhub-rust/acceptance-criteria.md:15
- These commands add
azure_coreand immediately remove it. The newly added consumer example later importsazure_core::credentials::TokenCredential, so following this dependency gate leaves that example uncompilable; use the direct credential pattern documented by the updated skill instead.
tests/scenarios/azure-eventhub-rust/acceptance-criteria.md:116 - This trait-object annotation unnecessarily introduces a direct
azure_coredependency that the dependency gate removes.DeveloperToolsCredential::new(None)?already returns anArcaccepted byConsumerClient::open; this also matches.github/plugins/azure-sdk-rust/skills/azure-eventhub-rust/SKILL.md:31,158-160.
…in evals The base_create_response_indb stimulus was failing Python syntax and idiomatic checks because the LLM generated oversized Python files (with helpers and section comments) that got truncated during writing. Changes: - Reduce template.py from 124 to 48 lines using concise ConfigDict pattern, inline Field calls, and brief docstrings - Update SKILL.md examples to use ConfigDict (modern Pydantic v2) - Remove verbose section separators and boilerplate comments
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 180 out of 183 changed files in this pull request and generated 16 comments.
Files not reviewed (1)
- tests/pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (6)
tests/run-all-evals.ps1:168
- This glob also selects
skill_effectiveness_eval.yaml, so every Rust scenario with both files is run twice even though this script and the scenarios guide defineeval.yamlas canonical. Those duplicate evaluations can also share one output/JUnit directory. Restrict discovery to the canonical filename; experiments are already handled byrun-skill-experiments.ps1.
$evalFiles = Get-ChildItem -Path $resolvedScenariosRoot -Filter "*eval.yaml" -File -Recurse |
tests/compare-experiment.mjs:84
run-summary.jsonlis JSON Lines, but parsing the entire file as one JSON value throws as soon as it contains more than one summary record.run-all-evals.ps1:354-359already handles this by selecting one non-empty line. Parse the file line-by-line here as well.
const summary = JSON.parse(fs.readFileSync(path.join(dir, 'run-summary.jsonl'), 'utf8').trim());
tests/run-skill-experiments.ps1:7
- The PR description promises a new
generate-skill-experiments.ps1, but that script is absent and there are no repository references to it. Add the generator or update the stated scope so the advertised experiment-generation workflow is actually available.
tests/scenarios/azure-eventhub-rust/scenarios.yaml:85 - This mock now introduces an unnecessary
azure_coredependency and contradicts the skill's own guidance at.github/plugins/azure-sdk-rust/skills/azure-eventhub-rust/SKILL.md:158:DeveloperToolsCredential::new(None)?already returns anArcthat can be passed directly. Keep the mock aligned with the public pattern.
tests/scenarios/azure-eventhub-rust/acceptance-criteria.md:116 - This “correct” example requires a direct
azure_coredependency solely for an unnecessary trait-object annotation, while the updated skill explicitly says to passDeveloperToolsCredential::new(None)?directly (SKILL.md:158). It is also inconsistent with the dependency section's subsequentcargo remove azure_core. Use the direct credential pattern.
tests/scenarios/azure-eventhub-rust/vally/eval.yaml:162 - The new post-build exception masks real
E0277compilation failures involvingOption<String>, so broken generated Event Hubs code can pass the build and clippy graders. Remove the exception.
| ## camelCase Aliases | ||
|
|
||
| ```python | ||
| from pydantic import BaseModel, ConfigDict, Field |
Comment on lines
35
to
+36
| class MyModel(BaseModel): | ||
| model_config = ConfigDict(populate_by_name=True) |
Comment on lines
+109
to
+112
| - error_code: E0277 | ||
| message_pattern: "Option<std::string::String>" | ||
| action: ignore_for_fail | ||
| - name: Post prompt Rust Cargo Build Failure |
Comment on lines
+155
to
+157
| - error_code: E0277 | ||
| message_pattern: "Option<std::string::String>" | ||
| action: ignore_for_fail |
Comment on lines
+109
to
+112
| error_exceptions: | ||
| - error_code: E0277 | ||
| message_pattern: "Option<std::string::String>" | ||
| action: ignore_for_fail |
Comment on lines
+149
to
+152
| error_exceptions: | ||
| - error_code: E0277 | ||
| message_pattern: "Option<std::string::String>" | ||
| action: ignore_for_fail |
Comment on lines
+157
to
+160
| error_exceptions: | ||
| - error_code: E0277 | ||
| message_pattern: "Option<std::string::String>" | ||
| action: ignore_for_fail |
Comment on lines
+167
to
+170
| error_exceptions: | ||
| - error_code: E0277 | ||
| message_pattern: "Option<std::string::String>" | ||
| action: ignore_for_fail |
Comment on lines
+163
to
+166
| error_exceptions: | ||
| - error_code: E0277 | ||
| message_pattern: "Option<std::string::String>" | ||
| action: ignore_for_fail |
Comment on lines
+71
to
+72
| [Parameter(HelpMessage = "Number of parallel evaluation workers")] | ||
| [int]$Workers = 5, |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 180 out of 183 changed files in this pull request and generated 3 comments.
Files not reviewed (1)
- tests/pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (4)
tests/run-all-evals.ps1:314
*eval.yamldiscovers botheval.yamlandskill_effectiveness_eval.yaml, but this output key uses only their shared parent directory. Those evaluations can run concurrently into the same directory and then select each other's latest run summary. Include the eval filename in the output key.
$relativeDir = [System.IO.Path]::GetRelativePath($resolvedScenariosRoot, $evalFile.DirectoryName).TrimStart('\', '/')
if ([string]::IsNullOrWhiteSpace($relativeDir)) { $relativeDir = "root" }
$safeOutDirName = $relativeDir -replace '[\\/:*?"<>|]', "_"
$scenarioOutDir = Join-Path $resolvedResultsRoot $safeOutDirName
tests/scenarios/azure-storage-blob-rust/vally/skill_effectiveness_eval.yaml:123
- This exception makes a final
cargo buildfailure pass whenever all compiler errors are E0277 messages containingOption<std::string::String>. That is a real non-compiling outcome (for example, formatting optionalblob.name) and directly contradicts this eval's compile-quality goal. Remove the exception from the post-prompt build and Clippy graders; intermediate trajectory errors can be handled separately if needed.
tests/scenarios/azure-eventhub-rust/acceptance-criteria.md:116 - This standalone example imports
azure_core::credentials::TokenCredential, but the dependency instructions immediately removeazure_core. It will not compile after following those instructions, and the skill already documents thatDeveloperToolsCredential::new(None)?can be passed directly. Avoid the unnecessary trait-object annotation.
tests/scenarios/azure-eventhub-rust/acceptance-criteria.md:184 ProducerClient::builder().opentakes the hub name as&str, but this “correct” example passes an ownedString. This contradicts the updated skill guidance and will not match the producer signature.
Comment on lines
+16
to
+20
| sonnet_baseline: | ||
| overrides: | ||
| model: claude-sonnet-4.6 | ||
| environment: | ||
| skills: [] |
Comment on lines
+3
to
+7
| evals: | ||
| - ../../agent-framework-azure-ai-py/vally/eval.yaml | ||
| - ../../azure-ai-contentsafety-py/vally/eval.yaml | ||
| - ../../azure-ai-contentunderstanding-py/vally/eval.yaml | ||
| - ../../azure-ai-language-conversations-py/vally/eval.yaml |
| @@ -0,0 +1,43 @@ | |||
| name: azure-sdk-python-skill-evaluations | |||
…xamples - pydantic-models-py eval: add file save instruction to base_create_response_indb prompt so agents write a .py file, fixing "No Python files found" grader failure - azure-ai-translation-document-py eval: add file save instructions to both stimuli for deterministic grader behavior - azure-ai-translation-document-py skill: rewrite Basic Document Translation section with complete self-contained example including try/except HttpResponseError; remove invalid require_envvar=True parameter from DefaultAzureCredential()
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 181 out of 184 changed files in this pull request and generated 2 comments.
Files not reviewed (1)
- tests/pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (5)
tests/compare-experiment.mjs:83
results.jsonlcontains multiple record types, but this loader keeps every row andaggregate()counts them all as stimuli. The experiment runner inrun-skill-experiments.ps1:1161-1163explicitly filters totrial-result; without the same filter here, average score and pass rate are diluted by non-trial records.
// results.jsonl may be multi-line (one record per stimulus); aggregate them
const rows = fs.readFileSync(path.join(dir, 'results.jsonl'), 'utf8')
.split('\n').filter(l => l.trim())
.map(l => JSON.parse(l));
tests/scenarios/azure-eventhub-rust/acceptance-criteria.md:116
- This “correct” example contradicts the canonical skill guidance at
.github/plugins/azure-sdk-rust/skills/azure-eventhub-rust/SKILL.md:31,158:DeveloperToolsCredential::new(None)?can be passed directly, so the trait-object annotation and directazure_coredependency are unnecessary. Keeping both sources aligned avoids teaching and evaluating different dependency patterns.
tests/scenarios/azure-eventhub-rust/acceptance-criteria.md:184 - The canonical skill documents that
ProducerClient::builder().open()takes the event hub name as&str(.github/plugins/azure-sdk-rust/skills/azure-eventhub-rust/SKILL.md:159). Converting it toStringhere applies the consumer signature to the producer and makes this advertised pattern fail to type-check.
tests/scenarios/azure-eventhub-rust/scenarios.yaml:81 - These imports introduce an unnecessary
azure_coretrait-object binding even though the canonical skill says the credential returned byDeveloperToolsCredential::new(None)?can be passed directly (SKILL.md:31,158). Keep the scenario oracle consistent with the skill so generated direct-credential solutions are not treated as divergent.
tests/run-skill-experiments.ps1:1 - The PR description lists
generate-skill-experiments.ps1as a new script, but that file is absent from the repository and no references to it exist. Add the promised generator or update the PR scope so consumers are not directed to a missing entry point.
Comment on lines
+155
to
+157
| if (files.length === 0) { | ||
| console.error("No Python files found to validate."); | ||
| process.exit(1); |
Contributor
There was a problem hiding this comment.
Fixed in 53b86dd. Added Save the code to <filename>.py in the current workspace. to all 29 stimuli across 15 eval files that were missing explicit file output paths (agent-framework-azure-ai-py, azure-ai-contentsafety-py, azure-ai-contentunderstanding-py, azure-ai-ml-py, azure-ai-projects-py, azure-ai-textanalytics-py, azure-ai-transcription-py, azure-ai-translation-text-py, azure-ai-vision-imageanalysis-py, azure-ai-voicelive-py, azure-containerregistry-py, azure-cosmos-db-py, fastapi-router-py, m365-agents-py, azure-storage-queue-py).
Comment on lines
+310
to
+315
| $relativeDir = [System.IO.Path]::GetRelativePath($resolvedScenariosRoot, $evalFile.DirectoryName).TrimStart('\', '/') | ||
| if ([string]::IsNullOrWhiteSpace($relativeDir)) { $relativeDir = "root" } | ||
|
|
||
| $safeOutDirName = $relativeDir -replace '[\\/:*?"<>|]', "_" | ||
| $scenarioOutDir = Join-Path $resolvedResultsRoot $safeOutDirName | ||
| New-Item -ItemType Directory -Path $scenarioOutDir -Force | Out-Null |
Contributor
There was a problem hiding this comment.
Fixed in 53b86dd. Added $evalBaseName = [System.IO.Path]::GetFileNameWithoutExtension($evalFile.Name) and changed the key to ($relativeDir + "/" + $evalBaseName) -replace '[\\/:*?"<>|]', "_" so each eval file gets its own output directory (e.g., azure-storage-queue-py_vally_eval vs azure-storage-queue-py_vally_skill_effectiveness_eval).
… key collision - Thread 97: Add 'Save the code to <file>.py in the current workspace.' to all eval stimuli that use Python graders but lacked explicit file output paths, preventing deterministic grader failures when models respond without saving files. Affected: agent-framework-azure-ai-py, azure-ai-contentsafety-py, azure-ai-contentunderstanding-py, azure-ai-ml-py, azure-ai-projects-py, azure-ai-textanalytics-py, azure-ai-transcription-py, azure-ai-translation-text-py, azure-ai-vision-imageanalysis-py, azure-ai-voicelive-py, azure-containerregistry-py, azure-cosmos-db-py, fastapi-router-py, m365-agents-py, azure-storage-queue-py - Thread 98: Include eval filename in output key in run-all-evals.ps1 so that eval.yaml and skill_effectiveness_eval.yaml in the same vally/ folder write to separate output directories, preventing JUnit file and run-dir lookup collisions
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 181 out of 184 changed files in this pull request and generated 6 comments.
Files not reviewed (1)
- tests/pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (21)
tests/compare-experiment.mjs:83
- Filter
results.jsonltotrial-resultrecords before aggregating. The companion PowerShell parser does this explicitly; without it, any metadata/summary rows contribute zero scores and inflate the denominator, producing incorrect averages and pass rates.
const rows = fs.readFileSync(path.join(dir, 'results.jsonl'), 'utf8')
.split('\n').filter(l => l.trim())
.map(l => JSON.parse(l));
tests/scenarios/azure-monitor-query-py/vally/eval.yaml:61
- The run-command graders only inspect Python files in the workspace, but this prompt does not request one. Add an explicit filename so a valid code-only response is not deterministically failed as “No Python files found.”
tests/scenarios/azure-cosmos-py/vally/eval.yaml:62 - The syntax and idiomatic graders require a
.pyfile, but this prompt permits a response-only code sample. Require a filename so the graders evaluate the requested solution rather than fail on an empty workspace.
tests/scenarios/azure-eventhub-rust/acceptance-criteria.md:184 - This “correct” producer example passes an owned
String, but the updated skill documents thatProducerClient::opentakes the hub name as&str; only the consumer overload requiresString. As written, this acceptance example does not compile and teaches the opposite of the skill guidance.
tests/run-skill-experiments.ps1:380 Invoke-CopilotNarrativeis never called; narrative generation goes directly throughNew-VariantNarrativeFromDigestandNew-ComparisonNarrativeFromDigests. Keeping this large unused execution path (plus its CLI discovery/fallback behavior) makes the new runner substantially harder to maintain and leaves misleading Copilot warnings. Remove it or wire it into the documented flow.
tests/scenarios/azure-storage-queue-rust/vally/skill_effectiveness_eval.yaml:122- Do not apply this exception to the post-prompt build/clippy checks. The grader treats a nonzero cargo command as successful when all compiler errors match
ignore_for_fail, so final code that fails with this E0277 can pass both compilation graders. Keep the exception only on the trajectory-only grader if transient failures are intentionally ignored.
tests/scenarios/azure-storage-queue-rust/vally/eval.yaml:157 - The final build and clippy graders should not ignore this compiler error.
ignore_for_failconverts a matching nonzero cargo result into a pass, allowing uncompilable final output to score as build-clean; reserve this exception for trajectory analysis only.
tests/scenarios/azure-storage-blob-rust/vally/skill_effectiveness_eval.yaml:123 - Applying
ignore_for_failto post-execution cargo build/clippy masks an uncompilable final workspace whenever E0277 mentionsOption<String>. Remove this exception from both final checks; it is only defensible for recording transient trajectory failures.
tests/scenarios/azure-keyvault-secrets-rust/vally/skill_effectiveness_eval.yaml:171 ignore_for_failshould not be configured on final build/clippy commands: the custom grader suppresses the nonzero command failure when all extracted errors match, so uncompilable output can pass. Limit this exception to trajectory-only analysis if needed.
tests/scenarios/azure-keyvault-secrets-rust/vally/eval.yaml:169- This post-execution exception suppresses a real cargo build failure, and the clippy grader has the same exception. A final workspace with the matching E0277 is consequently graded as compilable; remove the exception from final checks.
tests/scenarios/azure-keyvault-keys-rust/vally/skill_effectiveness_eval.yaml:180 - The final cargo graders must fail on compiler errors. With this
ignore_for_failrule, a nonzero build containing only this E0277 is marked successful, and the following clippy grader repeats the masking. Keep exceptions off post-prompt checks.
tests/scenarios/azure-keyvault-keys-rust/vally/eval.yaml:179 - This rule causes a matching E0277 to be ignored even when the final
cargo buildexits nonzero; the clippy block does the same. That defeats these post-prompt compilation checks and can award passing grades to uncompilable code.
tests/scenarios/azure-keyvault-certificates-rust/vally/eval.yaml:166 - Do not ignore compiler errors in the post-prompt build and clippy graders. The plugin treats a matching nonzero cargo result as successful, so final certificate code can fail E0277 yet receive passing compilation scores.
tests/scenarios/azure-identity-rust/vally/skill_effectiveness_eval.yaml:170 - This exception is unsafe on a final cargo command:
ignore_for_failsuppresses the nonzero build result when the matching E0277 is the only extracted error. The duplicated clippy exception then masks it again, allowing uncompilable output to pass.
tests/scenarios/azure-identity-rust/vally/eval.yaml:168 - The post-prompt build/clippy graders should report final compilation failures, but this exception makes a matching nonzero cargo command count as successful. Remove it from both final checks and retain it only for trajectory diagnostics if intentional.
tests/scenarios/azure-eventhub-rust/vally/skill_effectiveness_eval.yaml:163 - A matching E0277 causes this final nonzero cargo build to be ignored by the custom grader, and the subsequent clippy check repeats the exception. This can classify uncompilable Event Hubs code as passing; final checks must not use
ignore_for_fail.
tests/scenarios/azure-eventhub-rust/vally/eval.yaml:162 - This exception masks a real final cargo failure whenever the only compiler error matches E0277/
Option<String>; the clippy block duplicates it. Remove these exceptions from post-prompt checks so uncompilable output cannot pass.
tests/scenarios/azure-cosmos-rust/vally/skill_effectiveness_eval.yaml:171 - Using
ignore_for_failon post-execution cargo checks converts the matching nonzero build into a pass, and clippy repeats the suppression. Final Cosmos code can therefore remain uncompilable while receiving passing build scores.
tests/scenarios/azure-cosmos-rust/vally/eval.yaml:170 - The final build and clippy checks should never ignore compiler failures. With this rule, the custom grader marks a matching nonzero cargo result successful, allowing final generated code with E0277 to pass both compilation graders.
tests/scenarios/azure-eventhub-rust/acceptance-criteria.md:116 - This “correct” snippet now requires a direct
azure_coredependency even though section 0 removes that crate, and the updated skill explicitly saysDeveloperToolsCredential::new(None)?can be passed directly. Following the acceptance criteria end-to-end therefore leaves this import unresolved. Use the concrete credential binding instead.
tests/scenarios/azure-eventhub-rust/scenarios.yaml:85 - The mock response introduces a direct
azure_coreimport solely for an unnecessary trait-object annotation. The skill now documents thatDeveloperToolsCredential::new(None)?already returns a usableArc, and the acceptance setup removesazure_core; this mock can therefore fail when used with the documented dependencies.
| # Note: vally experiment run expects to be run from the scenario directory | ||
| Push-Location $vallyDir | ||
| try { | ||
| $output = & pnpm --dir $TestsRoot exec vally experiment run skill_effectiveness_experiment.yaml --output-dir $skillOutputRoot 2>&1 |
Comment on lines
+16
to
+17
| Create a python application which shows how to use LogsQueryClient to query a Log Analytics workspace. | ||
| Use DefaultAzureCredential and a relative timespan. |
Comment on lines
+16
to
+18
| Create a python application which demonstrates creating a basic Cosmos DB NoSQL. | ||
| Get a database and container, upsert an item, and read it back. | ||
| Use token credentials, not connection strings or SAS tokens. |
Comment on lines
+16
to
+18
| Create a Python example that uses ConversationAnalysisClient with DefaultAzureCredential. | ||
| Build a valid task payload for Conversation analysis and print the top intent. | ||
| Use a context manager for the client. |
Comment on lines
+16
to
+18
| Create a python application which uses ShareServiceClient using a connection string and demonstrate | ||
| creating a share, listing shares, and deleting the share. | ||
|
|
Comment on lines
162
to
164
| - error_code: E0277 | ||
| message_pattern: "Option<std::string::String>" | ||
| action: ignore_for_fail |
JasonYeMSFT
reviewed
Jul 27, 2026
JasonYeMSFT
reviewed
Jul 27, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 178 out of 181 changed files in this pull request and generated 6 comments.
Files not reviewed (1)
- tests/pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (6)
tests/scenarios/azure-eventhub-rust/acceptance-criteria.md:184
ProducerClient::builder().open()takes the Event Hub name as&str; converting it to an ownedStringmakes this “correct” example fail type checking. The updated skill itself demonstrates the required borrowed argument at.github/plugins/azure-sdk-rust/skills/azure-eventhub-rust/SKILL.md:63-67.
tests/scenarios/azure-monitor-query-py/vally/eval.yaml:17- This stimulus does not require a workspace file, while its run-command graders only scan the workspace and exit with “No Python files found” when none exists (
check-python-syntax.mjs:143-157). Require a concrete.pyoutput so otherwise valid inline answers do not deterministically fail both static graders.
tests/scenarios/azure-eventhub-py/vally/eval.yaml:64 - This prompt does not require a workspace file, so an inline response causes both run-command graders to report that no Python files were found. Specify an output file as the other working Python eval stimuli do.
tests/scenarios/azure-eventgrid-py/vally/eval.yaml:62 - This stimulus does not direct the agent to save code, while both static graders fail on an empty Python workspace. Add an explicit
.pydestination so inline code is not graded as missing.
tests/scenarios/azure-cosmos-py/vally/eval.yaml:18 - The run-command graders scan only generated workspace files and fail when no
.pyfile exists, but this prompt does not require one. Add a concrete output path so valid inline-only responses are not automatically failed.
tests/scenarios/azure-ai-language-conversations-py/vally/eval.yaml:18 - Unlike the second stimulus, this one does not request a workspace file. The two run-command graders cannot inspect assistant output and fail when no
.pyfile is generated, so require a concrete output path.
|
|
||
| 6. **Always verify package versions using crates.io.** Before using a package, check its version on [crates.io](https://crates.io/) to ensure you are using a stable and supported release. | ||
|
|
||
| 7. **Future-proof `#[non_exhaustive]` model structs and enums.** Azure Rust SDK request/response models derive `Default` and are frequently `#[non_exhaustive]`. When constructing an SDK model struct, always end the initializer with `..Default::default()` (even if every currently known field is set), suppressing the lint locally with `#[allow(clippy::needless_update)]` when needed. When matching an SDK enum, include a wildcard (`_`) arm so future service-added variants do not break the match. If a skill documents model construction, its code examples MUST demonstrate this pattern. See `references/azure-sdk-patterns.md` (Model Types) for the full example. |
| - azure-monitor-query-py | ||
| - name: logs_timespan_tuple | ||
| prompt: |- | ||
| Write a python application that shows how to use a LogsQueryClient example that uses a time range tuple with timezone-aware datetimes. |
Comment on lines
+16
to
+18
| Create a synchronous Event Hub producer in Python that authenticates with | ||
| DefaultAzureCredential, creates a batch, handles batch size limits, | ||
| and sends the batch. |
Comment on lines
+16
to
+17
| Write a Python application which shows how to publish a single CloudEvent to an Event Grid custom topic. | ||
| Use DefaultAzureCredential, environment variables, and proper imports. |
Comment on lines
+61
to
+62
| Create a python application using a CosmosClient with a connection string which retrieve | ||
| an existing database. |
Member
Author
|
Fixed all 6 suppressed comments from review #pullrequestreview-4791599641 in commit 5a119cd:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 178 out of 181 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- tests/pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (22)
tests/run-skill-experiments.ps1:1079
- Rust experiments can select
*-rust, but this invocation neither passes the sharedrust-cargo-build-failureplugin nor ensures itsdistbuild exists. Those experiments will fail to resolve the custom grader; mirror the plugin preparation and--grader-pluginargument used byrun-all-evals.ps1.
tests/scenarios/azure-eventhub-rust/scenarios.yaml:85 - The mock now introduces a direct
azure_coredependency and trait-object annotation even though the updated canonical skill saysDeveloperToolsCredential::new(None)?can be passed directly (SKILL.md:31,158). Keep the fixture aligned with the skill so the harness does not normalize unnecessary dependencies.
tests/scenarios/azure-eventhub-rust/acceptance-criteria.md:15 - This “correct” setup adds
azure_coreand immediately removes it. The updated skill only requires that crate for direct imports, so the dependency gate should leave the basic Event Hubs dependencies installed without contradictory commands.
tests/scenarios/azure-eventhub-rust/acceptance-criteria.md:116 - This example contradicts the updated skill by requiring an explicit
Arc<dyn TokenCredential>and directazure_coreimport. Use the credential value returned byDeveloperToolsCredential::newdirectly so acceptance criteria matchSKILL.md:31,158.
tests/scenarios/azure-eventhub-py/vally/eval.yaml:18 - This stimulus invokes workspace-only syntax and pylint graders but never requires a
.pyfile. If the agent returns code in its response, both graders exit with “No Python files found”; explicitly name an output file.
tests/scenarios/azure-cosmos-py/vally/eval.yaml:62 - The run-command graders only inspect
.pyfiles in the workspace, but this prompt does not request one. Add an explicit filename so response-only code does not produce “No Python files found.”
tests/scenarios/azure-eventgrid-py/vally/eval.yaml:17 - This prompt can yield response-only code, while both run-command graders require a workspace
.pyfile and otherwise fail. Require a concrete output filename.
tests/scenarios/azure-monitor-query-py/vally/eval.yaml:61 - The syntax and pylint graders fail when no
.pyfile is present, but this stimulus does not tell the agent to create one. Add an explicit workspace filename.
tests/scenarios/azure-storage-queue-rust/vally/skill_effectiveness_eval.yaml:122 - This exception makes a nonzero final
cargo buildpass whenever its only compiler error is E0277 mentioningOption<String>; the custom grader explicitly treats all-ignored errors as success. That is still uncompilable output, so keep this exception on trajectory analysis only.
tests/scenarios/azure-storage-queue-rust/vally/eval.yaml:157 - This exception can turn a failed final
cargo buildinto a passing grade when E0277/Option<String>is the only error. Since the generated application remains uncompilable, reserve the exception for trajectory grading and let the final build report failure.
tests/scenarios/azure-storage-blob-rust/vally/skill_effectiveness_eval.yaml:123 - Ignoring this error on the final build causes the custom grader to mark an uncompilable application successful when E0277/
Option<String>is its only compiler error. Apply the exception only to trajectory diagnostics.
tests/scenarios/azure-keyvault-secrets-rust/vally/skill_effectiveness_eval.yaml:171 - The custom grader treats a failed command as successful when every parsed error matches this exception. On the post-prompt build that masks genuinely uncompilable E0277/
Option<String>output; retain the exception only for trajectory analysis.
tests/scenarios/azure-keyvault-secrets-rust/vally/eval.yaml:169 - The final build can now pass despite returning nonzero if E0277 mentioning
Option<String>is the only compiler error. That hides uncompilable generated code, so do not apply the trajectory exception to this post-prompt command.
tests/scenarios/azure-keyvault-keys-rust/vally/skill_effectiveness_eval.yaml:180 - Applying this exception to the final build lets the grader convert a nonzero cargo result into success when all errors match E0277/
Option<String>. The output is still uncompilable; limit the exception to the trajectory grader.
tests/scenarios/azure-keyvault-keys-rust/vally/eval.yaml:179 - This post-prompt exception can approve code that
cargo buildcannot compile, because the plugin treats a failure with only ignored errors as success. Remove it from the final build and keep it only for trajectory analysis.
tests/scenarios/azure-keyvault-certificates-rust/vally/eval.yaml:166 - A final cargo failure consisting only of E0277/
Option<String>is marked successful by this exception, even though the generated code does not compile. This suppression belongs only on the trajectory grader.
tests/scenarios/azure-identity-rust/vally/skill_effectiveness_eval.yaml:170 - The plugin marks nonzero commands successful when all errors are ignored, so this final-build exception can report an uncompilable E0277/
Option<String>result as passing. Keep the exception on trajectory collection only.
tests/scenarios/azure-identity-rust/vally/eval.yaml:168 - This suppression can make the post-prompt
cargo buildgrader pass code that still fails compilation with E0277/Option<String>. Do not carry the trajectory exception into the final build check.
tests/scenarios/azure-eventhub-rust/vally/skill_effectiveness_eval.yaml:163 - The final build grader treats an all-ignored failure as success, so this exception can approve generated code that remains uncompilable. Restrict E0277/
Option<String>suppression to trajectory analysis.
tests/scenarios/azure-eventhub-rust/vally/eval.yaml:162 - A nonzero final build with only E0277/
Option<String>errors is converted to success by this exception. Since the output cannot compile, remove the exception from the post-prompt build and retain it only for trajectory diagnostics.
tests/scenarios/azure-cosmos-rust/vally/skill_effectiveness_eval.yaml:171 - This exception causes the final cargo grader to pass a failed build when E0277/
Option<String>is the only parsed error. That masks uncompilable output; trajectory collection is the appropriate place for this suppression.
tests/scenarios/azure-cosmos-rust/vally/eval.yaml:170 - The custom grader considers a failed command successful when all errors match this exception, so the final build can approve uncompilable E0277/
Option<String>code. Remove the exception from this post-prompt command.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 178 out of 181 changed files in this pull request and generated 13 comments.
Files not reviewed (1)
- tests/pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (19)
tests/run-skill-experiments.ps1:1079
- This invocation does not register the shared
rust-cargo-build-failure-checkplugin, although the discovered Rust experiments use that custom grader. Those experiments will fail to resolve the grader; mirrorrun-all-evals.ps1by building the plugin when needed and appending its directory via--grader-plugin.
tests/scenarios/azure-storage-blob-py/vally/eval.yaml:63 - This stimulus uses graders that exit unsuccessfully when no workspace
.pyfile exists, but the prompt does not request one. Explicitly name an output file so the evaluation consistently supplies an artifact to grade.
tests/scenarios/azure-servicebus-py/vally/eval.yaml:63 - This prompt does not request a workspace file, but both run-command graders return failure when they cannot find any
.pyfile. Require a named artifact so this stimulus can be graded deterministically.
tests/scenarios/azure-monitor-query-py/vally/eval.yaml:62 - The run-command graders used below fail with “No Python files found” unless the prompt causes a
.pyartifact to be written. This stimulus currently permits an inline-only response, so explicitly require a workspace file.
tests/scenarios/azure-storage-file-datalake-py/vally/eval.yaml:60 - This stimulus does not require writing a
.pyartifact, so both run-command graders can fail with “No Python files found.” Require a named workspace file.
tests/scenarios/azure-storage-file-share-py/vally/eval.yaml:14 - This stimulus does not require writing a
.pyartifact, so its run-command graders can fail before evaluating the response. Require a named workspace file.
tests/scenarios/azure-monitor-opentelemetry-exporter-py/vally/eval.yaml:59 - This stimulus omits a required
.pyoutput file, while both static graders fail when the workspace has none. Add a named workspace artifact.
tests/scenarios/azure-messaging-webpubsubservice-py/vally/eval.yaml:59 - The prompt permits an inline-only answer, but the run-command graders require a
.pyfile and otherwise fail. Require a named output file.
tests/scenarios/azure-cosmos-py/vally/eval.yaml:60 - This stimulus has no workspace-file requirement, so the static graders can fail with “No Python files found” even for valid inline code. Add a named
.pyoutput.
tests/scenarios/azure-search-documents-py/vally/eval.yaml:60 - This prompt does not require a
.pyartifact, although both run-command graders require one. Add an explicit workspace filename.
tests/scenarios/azure-eventhub-py/vally/eval.yaml:14 - Unlike the second stimulus, this prompt does not request a
.pyworkspace file, so the run-command graders can fail before scoring. Add a named output file.
tests/scenarios/azure-mgmt-fabric-py/vally/eval.yaml:59 - This stimulus does not require writing a
.pyfile, but both static graders fail when none exists. Add a named workspace artifact.
tests/scenarios/azure-mgmt-apicenter-py/vally/eval.yaml:60 - This stimulus does not require a
.pyworkspace artifact, so its run-command graders can fail before scoring. Add a named output file.
tests/scenarios/azure-mgmt-botservice-py/vally/eval.yaml:59 - The prompt allows inline-only code, but both static graders fail when no
.pyfile exists. Require a named workspace output.
tests/scenarios/azure-monitor-opentelemetry-py/vally/eval.yaml:59 - This stimulus omits a workspace filename, although the run-command graders require a
.pyartifact. Add a named output file.
tests/scenarios/azure-data-tables-py/vally/eval.yaml:59 - This prompt does not require a
.pyartifact, so both static graders can fail with “No Python files found.” Add an explicit workspace filename.
tests/scenarios/azure-eventgrid-py/vally/eval.yaml:14 - Unlike the second stimulus, this prompt does not request a
.pyfile, so the run-command graders can fail before scoring. Add a named workspace output.
tests/scenarios/azure-keyvault-py/vally/eval.yaml:60 - This stimulus omits a required
.pyworkspace artifact, while both run-command graders fail if none exists. Add a named output file.
tests/scenarios/azure-mgmt-apimanagement-py/vally/eval.yaml:59 - This stimulus does not require writing a
.pyfile, so the static graders can fail even for valid inline code. Add a named workspace output.
Comment on lines
+16
to
+18
| Create a basic Python application which authenticates with | ||
| DefaultAzureCredential and initializes a BlobServiceClient. Then | ||
| create a container client and create the container. |
Comment on lines
+16
to
+18
| Create a sync example that sends a message to a queue and receives it. | ||
| Use DefaultAzureCredential and context managers. | ||
| Use sender.send_messages(...) to send messages and handle ServiceBusError exceptions. |
| model: claude-sonnet-4.6 | ||
| judge_model: gpt-5.5 | ||
| stimuli: | ||
| - name: basic_service_client_and_filesystem |
| model: claude-sonnet-4.6 | ||
| judge_model: gpt-5.5 | ||
| stimuli: | ||
| - name: trace_exporter_basic |
| model: claude-sonnet-4.6 | ||
| judge_model: gpt-5.5 | ||
| stimuli: | ||
| - name: connection_string_broadcast |
| model: claude-sonnet-4.6 | ||
| judge_model: gpt-5.5 | ||
| stimuli: | ||
| - name: basic_bot_creation |
| model: claude-sonnet-4.6 | ||
| judge_model: gpt-5.5 | ||
| stimuli: | ||
| - name: one_line_setup |
| model: claude-sonnet-4.6 | ||
| judge_model: gpt-5.5 | ||
| stimuli: | ||
| - name: basic_table_service_client |
| model: claude-sonnet-4.6 | ||
| judge_model: gpt-5.5 | ||
| stimuli: | ||
| - name: secret_crud_basic |
| model: claude-sonnet-4.6 | ||
| judge_model: gpt-5.5 | ||
| stimuli: | ||
| - name: basic_apim_client_creation |
…t, package/workflow versions, dead params, exit code tracking
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 180 out of 183 changed files in this pull request and generated 3 comments.
Files not reviewed (1)
- tests/pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (19)
.github/plugins/azure-sdk-python/skills/pydantic-models-py/SKILL.md:33
- The snippet now presents an import block but still uses
datetimewithout importing it. Executing the documented class definition raisesNameError, so include the datetime import in this same fence.
from pydantic import BaseModel, ConfigDict, Field
tests/scenarios/azure-storage-blob-py/vally/eval.yaml:18
- Both run-command graders fail when the workspace contains no
.pyfiles, but this stimulus only asks for an application and never instructs the agent to write one. A normal answer containing a code block therefore fails the static graders regardless of quality. Require a concrete output file, as the other Python eval stimuli do.
tests/scenarios/azure-servicebus-py/vally/eval.yaml:18 - The static graders only inspect
.pyfiles in the workspace and fail when none exist, but this prompt does not tell the agent to save its example. Require a filename so valid answer-only code is not deterministically failed.
tests/scenarios/azure-monitor-query-py/vally/eval.yaml:62 - Unlike the first stimulus, this one never asks the agent to write a
.pyfile. The syntax and idiomatic run-command graders inspect only workspace files and fail when none are present, so answer-only code receives a deterministic failure. Specify an output filename.
tests/scenarios/azure-eventhub-rust/acceptance-criteria.md:15 - The newly documented “correct” setup immediately removes
azure_core, but the updated consumer example importsazure_core::credentials::TokenCredential(acceptance-criteria.md:112-118andscenarios.yaml:80-85). Following these commands leaves an undeclared direct dependency and the example will not compile. Keepazure_coreinstalled while that import is documented.
tests/scenarios/azure-storage-queue-rust/vally/skill_effectiveness_eval.yaml:122 - This exception is attached to the final
cargo buildand Clippy checks, so the grader marks a failed command as passing when all errors match it. A real E0277 such as formattingOption<String>with{}can therefore be reported as build-clean. Keep exceptions on trajectory analysis only; final-workspace checks must fail on compiler errors.
tests/scenarios/azure-storage-queue-rust/vally/eval.yaml:157 - The post-execution graders use this exception to treat matching compiler failures as success. That can let final code which does not compile (for example, an E0277 involving
Option<String>) pass the build/Clippy checks. Restrict the exception to trajectory diagnostics and let final cargo commands fail normally.
tests/scenarios/azure-storage-blob-rust/vally/skill_effectiveness_eval.yaml:123 - Applying
ignore_for_failto finalcargo build/Clippy means a nonzero command is graded as passing when its compiler errors match this broad E0277 substring. This masks genuinely uncompilable final submissions. Keep this allowance only for trajectory analysis, not post-execution validation.
tests/scenarios/azure-keyvault-secrets-rust/vally/skill_effectiveness_eval.yaml:171 - This suppresses matching E0277 failures from the final build and Clippy runs, causing the grader to report success for code that still fails to compile. Error exceptions may be useful for intermediate trajectory attempts, but final-workspace cargo checks should not ignore compiler errors.
tests/scenarios/azure-keyvault-secrets-rust/vally/eval.yaml:169 - The final cargo graders treat a failed command as successful when every error matches this exception. An E0277 mentioning
Option<String>is a normal compile failure, not an infrastructure false positive, so this can approve broken generated code. Remove the exception from post-execution checks.
tests/scenarios/azure-keyvault-keys-rust/vally/skill_effectiveness_eval.yaml:180 - Because this exception is also configured on final
cargo buildand Clippy, matching E0277 compiler failures are counted as successful commands. That defeats the final compilation gate. Retain the exception only for trajectory scoring and run post-prompt commands without it.
tests/scenarios/azure-keyvault-keys-rust/vally/eval.yaml:179 - This causes post-execution build failures to be ignored whenever all extracted errors are E0277 messages containing
Option<String>. Such errors can be genuine mistakes in the final code, so the build gate can return a false pass. Do not apply trajectory exceptions to final cargo checks.
tests/scenarios/azure-keyvault-certificates-rust/vally/eval.yaml:166 - The post-prompt build/Clippy checks should validate the final workspace, but this exception converts matching E0277 compile failures into passes. That can score uncompilable certificate code as clean. Keep the exception on trajectory collection only and remove it from final cargo commands.
tests/scenarios/azure-identity-rust/vally/skill_effectiveness_eval.yaml:170 - This exception is used by the final cargo commands, where it causes a failed build to pass if every error matches E0277 plus
Option<String>. Those are genuine compiler errors in generated code, so final validation becomes inaccurate. Apply exceptions only to intermediate trajectory failures.
tests/scenarios/azure-identity-rust/vally/eval.yaml:168 - A matching E0277 makes the grader ignore a nonzero final
cargo build/Clippy result, so broken final code can be labeled build-clean. The exception belongs, if needed, on trajectory attempts only; post-execution checks must surface every compiler error.
tests/scenarios/azure-eventhub-rust/vally/skill_effectiveness_eval.yaml:163 - Configuring
ignore_for_failon final cargo commands lets a real E0277 involvingOption<String>pass both final build and lint grading despite a nonzero exit. This hides precisely the SDK usage errors the eval should detect. Remove the exception from post-prompt checks.
tests/scenarios/azure-eventhub-rust/vally/eval.yaml:162 - The final build and Clippy graders treat matching E0277 compiler failures as successful because of this exception. For example, formatting an optional SDK field incorrectly can remain uncompilable yet pass these gates. Keep any exception limited to trajectory analysis, not final-workspace validation.
tests/scenarios/azure-cosmos-rust/vally/skill_effectiveness_eval.yaml:171 - This broad E0277 exception also governs post-execution
cargo buildand Clippy. When all errors match, the grader records the failed command as passing, allowing uncompilable final Cosmos code through. Final cargo checks should run without trajectory exceptions.
tests/scenarios/azure-cosmos-rust/vally/eval.yaml:170 - Because
ignore_for_failis applied to final cargo commands, a genuine E0277 mentioningOption<String>makes the failed build/Clippy command count as passing. This produces false-positive eval results for uncompilable code. Remove these exceptions from post-prompt checks.
Comment on lines
+62
to
+63
| Create a Python application which uploads a local file to Azure Blob Storage using BlobClient. | ||
| Use overwrite=True when uploading. |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 182 out of 185 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- tests/pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (23)
tests/run-skill-experiments.ps1:1066
- This experiment runner invokes Rust evals that use
rust-cargo-build-failure-checkwithout loading the shared grader plugin. Unlikerun-all-evals.ps1and the CI workflow, this command supplies no--grader-pluginand does not ensuredist/index.jsis built, so Rust skill experiments fail with an unknown grader instead of producing effectiveness results. Reuse the plugin build/load preflight before launching experiments.
tests/scenarios/azure-storage-blob-py/vally/eval.yaml:18 - This stimulus does not require writing a
.pyfile, but both run-command graders scan only workspace files and exit with “No Python files found” when none exist. Require a concrete output filename so a valid code response is not deterministically failed by the static graders.
tests/scenarios/azure-storage-blob-py/vally/eval.yaml:63 - This stimulus does not require writing a
.pyfile, so the syntax and pylint run-command graders can fail with “No Python files found” even when the assistant returns correct code. Specify a workspace output file.
tests/scenarios/azure-servicebus-py/vally/eval.yaml:18 - The run-command graders require a generated
.pyfile, but this prompt only asks for an example and does not direct the agent to save one. Add an output filename to prevent correct inline responses from failing both static graders.
tests/scenarios/azure-servicebus-py/vally/eval.yaml:63 - The syntax and idiomatic graders exit when no
.pyfile exists, while this stimulus does not require creating one. Give the agent a concrete workspace filename so the graders evaluate the requested implementation.
tests/scenarios/azure-monitor-query-py/vally/eval.yaml:62 - This stimulus can be answered inline because it never requires a workspace file, but its run-command graders fail when no
.pyfile is present. Require a named output file as the other stimulus in this eval does.
.github/plugins/azure-sdk-python/skills/pydantic-models-py/SKILL.md:33 - The import block is still incomplete: this snippet uses
datetimebelow, and the following snippets useOptional, but neither name is imported. Copying the documented examples therefore raisesNameErrorwhile defining the models. Include both standard-library imports with the newly added Pydantic imports.
from pydantic import BaseModel, ConfigDict, Field
tests/scenarios/azure-storage-queue-rust/vally/skill_effectiveness_eval.yaml:122
- This post-execution build grader now treats an
E0277involvingOption<String>as success even thoughcargo buildexited nonzero. That is a real compilation failure (for example, formatting an optional SDK field with{}), so the eval can award a passing build score to code that does not compile. Remove this exception here and from the corresponding Clippy check.
tests/scenarios/azure-storage-queue-rust/vally/eval.yaml:157 - Ignoring this
E0277converts a nonzerocargo buildinto a passing post-execution grader result.Option<String>display errors are genuine source errors, not benign SDK noise, so remove this exception here and from the Clippy grader.
tests/scenarios/azure-storage-blob-rust/vally/skill_effectiveness_eval.yaml:123 - The build grader will ignore a nonzero
cargo buildwhen its only compiler error is thisOption<String>E0277, allowing uncompilable blob code to pass. This is exactly the optional-field formatting error the skill now warns against; remove the exception here and from Clippy.
tests/scenarios/azure-storage-blob-rust/vally/eval.yaml:164 - This exception makes the final build grader pass code that failed to compile with an
Option<String>formatting/type error. Final-workspace build failures must remain failures; remove this exception here and from the Clippy grader.
tests/scenarios/azure-keyvault-secrets-rust/vally/skill_effectiveness_eval.yaml:171 - A post-execution
cargo buildfailure with thisE0277is still an uncompilable submission, butignore_for_failturns it into a passing grader result. Remove this exception here and from the paired Clippy check.
tests/scenarios/azure-keyvault-secrets-rust/vally/eval.yaml:169 - This configuration masks a real final-workspace compilation failure: a lone
E0277mentioningOption<String>causes the grader to mark the failedcargo buildas successful. Remove the exception here and from Clippy.
tests/scenarios/azure-keyvault-keys-rust/vally/skill_effectiveness_eval.yaml:180 - The final build check should not ignore
E0277forOption<String>; it is a genuine compile error and currently allows a failedcargo buildto receive a passing score. Remove this exception here and from the Clippy check.
tests/scenarios/azure-keyvault-keys-rust/vally/eval.yaml:179 - With
ignore_for_fail, code that fails the finalcargo builddue to anOption<String>E0277is graded as compiling. Remove this exception here and from Clippy so build correctness remains deterministic.
tests/scenarios/azure-keyvault-certificates-rust/vally/eval.yaml:166 - This exception masks a real final
cargo buildfailure and can award a passing build grade to uncompilable certificate code. AnOption<String>E0277must be fixed by the generated code, not ignored; remove this block and the equivalent Clippy exception.
tests/scenarios/azure-identity-rust/vally/skill_effectiveness_eval.yaml:170 - A failed final
cargo buildis treated as successful when its only diagnostic matches thisOption<String>E0277. That produces false-positive experiment results; remove this exception here and from the Clippy grader.
tests/scenarios/azure-identity-rust/vally/eval.yaml:168 - This
ignore_for_failrule lets an uncompilable final workspace pass the build grader whencargo buildreports anOption<String>E0277. Remove it here and from Clippy so final compilation failures remain failures.
tests/scenarios/azure-eventhub-rust/vally/skill_effectiveness_eval.yaml:163 - The post-execution build check can now pass despite a nonzero
cargo buildif the diagnostic is thisOption<String>E0277. Since that is a real source-level compile error, remove the exception here and from the Clippy check.
tests/scenarios/azure-eventhub-rust/vally/eval.yaml:162 - Ignoring this diagnostic turns a failed final
cargo buildinto a passing grader result.Option<String>E0277errors indicate invalid generated code and should be fixed rather than exempted; remove this block and the matching Clippy exception.
tests/scenarios/azure-cosmos-rust/vally/skill_effectiveness_eval.yaml:171 - This rule masks an actual post-execution compile failure:
cargo buildcan exit nonzero for anOption<String>E0277while the grader reports success. Remove the exception here and from Clippy to preserve build-result accuracy.
tests/scenarios/azure-cosmos-rust/vally/eval.yaml:170 - The final build grader should fail on this
E0277; it represents uncompilable generated code, butignore_for_failcurrently marks the failed command as successful. Remove this exception here and from the Clippy grader.
tests/run-skill-experiments.ps1:380 - This function and its process-launch helper are never called anywhere in the script, while narrative generation below is fully deterministic. Keeping roughly 200 lines of retry, npx, platform, and loader-fallback logic adds an untested execution path (including
--allow-all-tools) without affecting output. Remove both unused functions rather than carrying speculative complexity.
…x blob-rust ignore_for_fail, fix skill-creator Rust guidance - Add 'Save the code to file.py' instructions to all eval.yaml stimuli missing them (31 stimuli across 18 Python scenario files) so the workspace-only syntax/pylint graders can find the .py files - Remove error_exceptions from Post-prompt graders in azure-storage-blob-rust/vally/eval.yaml (keep only on trajectory analysis), preventing genuine E0277 compilation failures from being masked as successful builds - Fix #[non_exhaustive] guidance in skill-creator/SKILL.md: distinguish between externally constructible structs (use ..Default::default()) and truly non_exhaustive structs (use constructor/builder or default-then-mutate) to avoid E0639 compile errors
…ample, fix invalid credential param - Replace "Explicit Configuration" section with "Explicit Connection String" section showing connection_string=os.environ["APPLICATIONINSIGHTS_CONNECTION_STRING"] with try/except - Remove invalid DefaultAzureCredential(require_envvar=True) parameter (not a valid API) - Update capabilities.md to reflect renamed section This fixes the azure-monitor-opentelemetry-py-eval explicit_connection_string scenario which was failing Python syntax and idiomatic checks because the skill lacked a canonical example for the connection_string parameter pattern.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 184 out of 187 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- tests/pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (3)
tests/run-skill-experiments.ps1:1066
- This runner selects
*-rustscenarios, but it never builds or passes the sharedrust-cargo-build-failuregrader plugin. Rust experiment specs use that custom grader, so those runs will fail with an unknown grader type. Mirror the plugin build/--grader-pluginhandling already implemented inrun-all-evals.ps1.
tests/run-all-evals.ps1:324 - Vally 0.10 defines
--junitas a boolean flag; the report location is controlled by--output-dir. Supplyingresults.xmlas a following argument makes it an unexpected positional argument and breaks every run with-JUnit. The workflow already uses the supported--junit --output-dir ...form.
.github/plugins/azure-sdk-python/skills/pydantic-models-py/SKILL.md:33 - This standalone example still cannot execute because the
datetimeannotation is evaluated whenMyModelis defined, butdatetimeis not imported. Add the standard-library import so users copying the documented snippet do not getNameError.
from pydantic import BaseModel, ConfigDict, Field
| # Note: vally experiment run expects to be run from the scenario directory | ||
| Push-Location $vallyDir | ||
| try { | ||
| $output = & pnpm --dir $TestsRoot exec vally experiment run skill_effectiveness_experiment.yaml --output-dir $skillOutputRoot 2>&1 |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 184 out of 187 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- tests/pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (16)
tests/run-skill-experiments.ps1:1066
- This command runs Rust experiments without registering or building
rust-cargo-build-failure-check. Every Rust experiment references that custom grader, so the advertised-SkillPattern '*-rust'path fails with an unknown grader; mirrorrun-all-evals.ps1by ensuringdist/index.jsexists and passing the shared directory via--grader-plugin.
tests/scenarios/README.md:18 - The documented prerequisite allows Node 20.17, but
tests/package.jsonnow requires Node >=22 and the graders usenode:path.matchesGlob, which is unavailable in Node 20. Following this setup guide can therefore fail before evaluations start; align it with the package engine and CI version.
tests/scenarios/azure-storage-queue-rust/vally/skill_effectiveness_eval.yaml:122 - Applying this exception to the final
cargo buildandcargo clippygraders makes an output with the matching E0277 compiler error pass both compile checks. Because the previous raw Cargo graders were removed, a non-compiling solution can now pass the eval. Keep the exception only on the trajectory grader and require both post-prompt commands to compile cleanly.
tests/scenarios/azure-storage-queue-rust/vally/eval.yaml:157 - Applying this exception to the final
cargo buildandcargo clippygraders makes an output with the matching E0277 compiler error pass both compile checks. Because the previous Cargo checks were removed, a non-compiling solution can now pass the eval. Keep the exception only on the trajectory grader and require both post-prompt commands to compile cleanly.
tests/scenarios/azure-storage-blob-rust/vally/skill_effectiveness_eval.yaml:123 - Applying this exception to the final
cargo buildandcargo clippygraders makes an output with the matching E0277 compiler error pass both compile checks. The replacement removed the raw Cargo graders, so this can let a non-compiling solution pass. Keep the exception only on the trajectory grader.
tests/scenarios/azure-keyvault-secrets-rust/vally/skill_effectiveness_eval.yaml:171 - Applying this exception to the final
cargo buildandcargo clippygraders makes an output with the matching E0277 compiler error pass both compile checks. Since these replace the prior Cargo commands, non-compiling generated code can satisfy the eval. Keep the exception only on the trajectory grader.
tests/scenarios/azure-keyvault-secrets-rust/vally/eval.yaml:169 - Applying this exception to the final
cargo buildandcargo clippygraders makes an output with the matching E0277 compiler error pass both compile checks. Since these replace the prior Cargo commands, non-compiling generated code can satisfy the eval. Keep the exception only on the trajectory grader.
tests/scenarios/azure-keyvault-keys-rust/vally/skill_effectiveness_eval.yaml:180 - Applying this exception to the final
cargo buildandcargo clippygraders makes an output with the matching E0277 compiler error pass both compile checks. Since these replace the prior Cargo commands, non-compiling generated code can satisfy the eval. Keep the exception only on the trajectory grader.
tests/scenarios/azure-keyvault-keys-rust/vally/eval.yaml:179 - Applying this exception to the final
cargo buildandcargo clippygraders makes an output with the matching E0277 compiler error pass both compile checks. Since these replace the prior Cargo commands, non-compiling generated code can satisfy the eval. Keep the exception only on the trajectory grader.
tests/scenarios/azure-keyvault-certificates-rust/vally/eval.yaml:166 - Applying this exception to the final
cargo buildandcargo clippygraders makes an output with the matching E0277 compiler error pass both compile checks. Since these replace the prior Cargo commands, non-compiling generated code can satisfy the eval. Keep the exception only on the trajectory grader.
tests/scenarios/azure-identity-rust/vally/skill_effectiveness_eval.yaml:170 - Applying this exception to the final
cargo buildandcargo clippygraders makes an output with the matching E0277 compiler error pass both compile checks. Since these replace the prior Cargo commands, non-compiling generated code can satisfy the eval. Keep the exception only on the trajectory grader.
tests/scenarios/azure-identity-rust/vally/eval.yaml:168 - Applying this exception to the final
cargo buildandcargo clippygraders makes an output with the matching E0277 compiler error pass both compile checks. Since these replace the prior Cargo commands, non-compiling generated code can satisfy the eval. Keep the exception only on the trajectory grader.
tests/scenarios/azure-eventhub-rust/vally/skill_effectiveness_eval.yaml:163 - Applying this exception to the final
cargo buildandcargo clippygraders makes an output with the matching E0277 compiler error pass both compile checks. Since these replace the prior Cargo commands, non-compiling generated code can satisfy the eval. Keep the exception only on the trajectory grader.
tests/scenarios/azure-eventhub-rust/vally/eval.yaml:162 - Applying this exception to the final
cargo buildandcargo clippygraders makes an output with the matching E0277 compiler error pass both compile checks. Since these replace the prior Cargo commands, non-compiling generated code can satisfy the eval. Keep the exception only on the trajectory grader.
tests/scenarios/azure-cosmos-rust/vally/skill_effectiveness_eval.yaml:171 - Applying this exception to the final
cargo buildandcargo clippygraders makes an output with the matching E0277 compiler error pass both compile checks. Since these replace the prior Cargo commands, non-compiling generated code can satisfy the eval. Keep the exception only on the trajectory grader.
tests/scenarios/azure-cosmos-rust/vally/eval.yaml:170 - Applying this exception to the final
cargo buildandcargo clippygraders makes an output with the matching E0277 compiler error pass both compile checks. Since these replace the prior Cargo commands, non-compiling generated code can satisfy the eval. Keep the exception only on the trajectory grader.
… in experiments - run-all-evals.ps1: Remove filename arg from --junit flag; Vally 0.10 treats it as boolean - pydantic-models-py/SKILL.md: Add missing 'from datetime import datetime' import in camelCase Aliases example - run-skill-experiments.ps1: Mirror rust-cargo-build-failure grader plugin detection/build/pass from run-all-evals.ps1
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 184 out of 187 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- tests/pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (6)
tests/scenarios/README.md:18
- The prerequisite conflicts with
tests/package.json, which declares Node>=22.0.0. Following this README with Node 20.17 can therefore makepnpm installreject the environment before any evaluation runs.
tests/compare-experiment.mjs:54 - The documented fallback says variants are selected alphabetically, but
readdirSync()order is not guaranteed and this array is never sorted. For variant names withoutbaseline/skill, the same results directory can therefore be interpreted differently across filesystems.
const runDirs = fs.readdirSync(experimentDir)
.filter(name => {
const full = path.join(experimentDir, name);
return fs.statSync(full).isDirectory() &&
fs.existsSync(path.join(full, 'results.jsonl')) &&
fs.existsSync(path.join(full, 'run-summary.jsonl'));
});
tests/scenarios/azure-eventhub-rust/acceptance-criteria.md:116
- This “correct” snippet requires a direct
azure_coredependency, but the dependency gate above immediately removesazure_core. It also contradicts the canonical skill guidance at.github/plugins/azure-sdk-rust/skills/azure-eventhub-rust/SKILL.md:97-108,158, whereDeveloperToolsCredential::new(None)?is passed directly. As written, users following the gate and this example get an unresolvedazure_coreimport.
tests/scenarios/azure-eventhub-rust/scenarios.yaml:85 - The mock response introduces a direct
azure_coreimport and explicit trait-object binding even though the skill's supported example passes the credential directly (SKILL.md:97-108,158). Because this scenario does not require addingazure_core, treating this response as canonical can produce code that fails to compile when copied into the generated project.
.github/plugins/azure-sdk-python/skills/pydantic-models-py/SKILL.md:38 - The skill now correctly teaches Pydantic v2
ConfigDict, but the existing scenario validator still requires literal v1 strings such asfrom pydantic import BaseModel, Fieldandpopulate_by_name = True(tests/scenarios/pydantic-models-py/scenarios.yaml:17,24,77-80; matching uses literalincludesintests/harness/runner.ts:368-370). An answer following this new example will consequently be marked as missing expected patterns. Update the Pydantic scenarios and their v1class Configmock responses alongside this guidance.
from pydantic import BaseModel, ConfigDict, Field
class MyModel(BaseModel):
model_config = ConfigDict(populate_by_name=True)
tests/compare-experiment.mjs:301
- When this branch is reached, both deltas are negative, so the report prints wording such as “-20% fewer tokens,” which reverses the intended meaning for readers. Describe the signed values as changes, or render their absolute values before saying “fewer.”
? `✅ Skill variant passes with ${tokenDelta} fewer tokens and ${timeDelta} less wall time → SKILL IS BETTER`
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Split from #375 (5 of 5).
Adds the Vally skill-effectiveness experiment framework and Python test scenarios:
run-skill-experiments.ps1,run-all-evals.ps1,compare-experiment.mjsvally/eval + experiment yaml andscenarios.yamlfor ~40 Python skillstests/scenarios/_shared/run-harness-by-language.ps1and test dependency manifests