Add python skills and test.#375
Conversation
…mprovements to skill-experiments script
…et available from crates.io
Reducing confusion on vally scope notifications.
richardpark-msft
left a comment
There was a problem hiding this comment.
Still looking through it, but found some things worth addressing now.
| with: | ||
| fetch-depth: 0 | ||
|
|
||
| - uses: actions/setup-node@v4 |
There was a problem hiding this comment.
You can use the latest setup-node action here instead of v4. v6 (currently v6.5.0) has been stable for months; I'd stick with v6 rather than v7, which only just shipped. Apply this to move up a major version:
| - uses: actions/setup-node@v4 | |
| - uses: actions/setup-node@v6 |
One thing to watch: since v5, setup-node auto-caches when package.json has a packageManager field. This step just does a global npm install, so it should be a non-issue, but set package-manager-cache: false if you see odd caching.
| fi | ||
| elif [ "${{ github.event_name }}" = "pull_request" ]; then | ||
| collect_from_changed_dirs "${{ github.event.pull_request.base.sha }}" "${{ github.sha }}" | sort -u > "$specs_file" | ||
| elif [ "${{ github.event_name }}" = "pull_request" ] || [ "${{ github.event_name }}" = "pull_request_target" ]; then |
There was a problem hiding this comment.
You didn't end up using pull_request_target, so you can probably just revert this line (and any other ones related to it).
| echo "::notice::Running Vally evaluations for same-repo PR from ${{ github.event.pull_request.head.repo.full_name }}." | ||
|
|
||
| - name: Vally PR scope notice (cross-repo) | ||
| if: ${{ steps.specs.outputs.has_specs == 'true' && github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository }} |
There was a problem hiding this comment.
I was going back and forth on this with copilot - the checks (for PR vs non-PR) are a bit noisy but apparently you can break it apart a bit, which I think would help. So your overall purpose (everybody gets linted) happens easily enough and eval is a bit more special, but now it's confined to it's own job.
jobs:
lint:
runs-on: ubuntu-latest
# runs for everyone, no token
steps:
- uses: actions/checkout@v6
# resolve specs + schema-only lint
eval:
needs: lint
runs-on: ubuntu-latest
if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository
permissions:
contents: read
copilot-requests: write
env:
GH_TOKEN: ${{ github.token }}
steps:
- uses: actions/checkout@v6
# build grader plugin + run vally eval| [string]$SkillPattern = "*-py", | ||
|
|
||
| [Parameter(Mandatory = $false)] | ||
| [ValidateRange(1, 128)] |
There was a problem hiding this comment.
Please get me the 128 core machine...
| @@ -0,0 +1 @@ | |||
| Error: Experiment config file not found: Q:\src\skills\tests\scenarios\agent-framework-azure-ai-py\skill_effectiveness_experiment.yaml | |||
There was a problem hiding this comment.
Is this file supposed to be checked in? Also, Q:\ is not universal....
| @@ -0,0 +1,94 @@ | |||
| Running experiment 'azure-monitor-ingestion-py-skill-experiment' | |||
| Output: Q:\src\skills\tests\scenarios\azure-monitor-ingestion-py\vally\vally-experiment-results\2026-06-30T23-35-17-490Z | |||
There was a problem hiding this comment.
Another one, not sure if this file should be checked in. Maybe do a scrub and then add in a .gitignore file?
| $npmLoaderPath = $null | ||
| $loaderCandidates = @( | ||
| (Join-Path $copilotDir "node_modules\@github\copilot\npm-loader.js"), | ||
| "q:\.tools\.npm-global\node_modules\@github\copilot\npm-loader.js" |
There was a problem hiding this comment.
Might be worth doing a pass, making sure you don't have other paths hardcoded from your dev setup.
| [bool]$EnableNodeLoaderFallback = $true | ||
| ) | ||
|
|
||
| $copilotCommand = Get-Command copilot -ErrorAction SilentlyContinue |
There was a problem hiding this comment.
There's a lot of code in here trying to be accomodating to all sorts of scenarios around copilot, but it's probably okay to just say 'use the copilot in the path, and if it's not t here, then bail' and then you can delete a bunch of code.
|
This PR is being split into 5 smaller, independently reviewable PRs (all targeting
Together these cover all 361 changed files in this PR, with no overlap. |
* Add capability and non-hero references for Python skills Split from #375: adds references/capabilities.md and references/non-hero-scenarios.md across ~30 azure-sdk-python skills, updates their SKILL.md files, and refreshes .github/skills symlinks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9429ad33-6f31-4ec5-ba00-0f11b5afb82e * Fix create_analyzer to begin_create_analyzer LRO in contentunderstanding non-hero-scenarios * Fix get_supported_languages to use TextTranslationClient in translation-document non-hero-scenarios * Clarify TextTranslationClient default endpoint in supported languages example * Fix instrumentation_options API and supported libraries in azure-monitor-opentelemetry-py non-hero-scenarios * Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Apply remaining changes * Fix invalid token acquisition example in m365 capabilities reference * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Replace placeholder non-hero scenario files with actionable content Add retry, timeout, pagination, cleanup, and async examples for: - azure-ai-transcription-py - azure-messaging-webpubsubservice-py - azure-ai-language-conversations-py * Fix variable naming and list iteration in new non-hero scenarios * Replace Azure SDK boilerplate checklist with Pydantic-specific checks in capabilities.md * Refine Pydantic checklist: focus v2 patterns item on new code * Fix fastapi-router-py capabilities checklist: replace Azure SDK boilerplate with FastAPI-specific checks * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Fix remaining open review comments: eventgrid event var, transcription/speech auth, fabric RpSku, keyvault/botservice credential exposure * Fix Authorization header wording in speech-to-text-rest SKILL.md * Polish wording per code review: fabric None guard, transcription key comment, speech-to-text auth reference * Address all open PR review comments - Fix pnpm-workspace.yaml: resolve esbuild allowBuilds placeholder - Fix azure-mgmt-apicenter non-hero: add missing closing parenthesis - Fix azure-monitor-opentelemetry SKILL.md: flush() → force_flush() - Fix azure-monitor-opentelemetry-exporter SKILL.md: flush() → force_flush() - Fix azure-storage-queue non-hero: re-raise instead of silently passing exception - Fix azure-identity non-hero: log and re-raise auth errors instead of silently passing - Fix azure-ai-contentunderstanding non-hero: AnalysisInput → AnalyzeInput, fix model imports - Fix azure-ai-transcription SKILL.md + non-hero: use AzureKeyCredential wrapper - Fix m365-agents capabilities.md: complete CopilotClient example end-to-end * Polish m365-agents capabilities: improve variable names and context manager comment * Fix remaining review comments and CI pipeline failure - Delete tests/pnpm-workspace.yaml (caused CI 'packages field missing' error) - Delete azure-cosmos-py/scripts/setup_cosmos_container.py (outside PR scope) - Fix pydantic-models-py: clarify model_validate vs direct construction - Fix m365-agents-py: remove activity.conversation.id (UnboundLocalError on empty stream) - Fix azure-messaging-webpubsubservice-py: use close_user_connections, don't log bearer token URL - Fix azure-messaging-webpubsubservice-py acceptance-criteria: update connection close example - Fix azure-ai-transcription-py: correct LROPoller timeout pattern (done()+deadline loop) - Fix azure-ai-transcription-py acceptance-criteria: remove DefaultAzureCredential anti-pattern - Fix azure-ai-translation-text-py: use InputTextItem instead of raw strings in all bodies - Fix azure-ai-ml-py: remove unused Pipeline import - Fix azure-ai-language-conversations-py: fix orchestration intents (dict keyed by name) * Move import time to top of transcription timeout example * Fix transcription acceptance criteria: add DefaultAzureCredential as valid auth, remove incorrect anti-pattern --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Update Vally test harness and CI workflows Split from #375: harness output capture and runner updates, new copilot-client tests, and CI workflow updates (rename vally-evaluation to run-vally-evaluations, test-harness tweaks, actions-lock). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9429ad33-6f31-4ec5-ba00-0f11b5afb82e * Add missing .github/skills symlinks for azure-sdk-python plugin * fix: address workflow review suggestions --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
) * Add lifecycle guidance and Azure SDK patterns to skill-creator Split from #375: updates skill-creator SKILL.md with skill lifecycle guidance and adds references/azure-sdk-patterns.md. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9429ad33-6f31-4ec5-ba00-0f11b5afb82e * Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * skill-creator: remove sync/async preference from guidance Azure SDK guidance should not express a preference for either sync or async clients. Updated SKILL.md and azure-sdk-patterns.md to present both forms as first-class options with equal priority, removing the "pick one mode" directives while retaining the rule against mixing sync and async within a single call path. * skill-creator: replace inaccurate benchmark skill descriptions The previous benchmark table referenced azure-ai-ml-py and other Python skills with descriptions that didn't match their actual content (e.g., azure-ai-ml-py covers workspaces/data/models/compute/jobs/pipelines, not a single train+register workflow). Replace with verified Rust skills from this repository that actually demonstrate the one-workflow/1-2-example criteria. Also drop the misleading 'Tokens Δ' column which implied historical optimization comparisons not applicable to new skills. * skill-creator: fix remaining review issues from f2d1873 review * skill-creator: resolve outstanding review comments * skill-creator: resolve remaining PR review comments * skill-creator: fix remaining review guidance issues * Apply remaining changes * fix: restore valid pnpm workspace config for tests * Address latest PR review feedback * fix: resolve latest skill-creator review feedback --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Adds Python skill-effectiveness experiments, expands skill reference documentation, and updates the Vally test harness.
Changes:
Adds Python Vally experiments and standardizes test models.
Adds capability/non-hero references and lifecycle guidance.
Updates Vally/Rust graders, harness output capture, and CI checks.
Reviewed Changes