-
Notifications
You must be signed in to change notification settings - Fork 315
Add Vally skill-effectiveness experiments and Python test scenarios #383
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
LarryOsterman
wants to merge
11
commits into
main
Choose a base branch
from
larryo/split/vally-python-scenarios
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
ffce92d
Add Vally skill-effectiveness experiments and Python test scenarios
LarryOsterman 3f410d7
Update vally version
LarryOsterman e573b16
Update package info
LarryOsterman 6e8b2e8
Fix review comments: delete artifact, fix exception handling, add exc…
Copilot f7b82c6
Fix review 4708288806 comments: exceptions, duplicate files, tab chec…
Copilot 3efe77a
Removed experiment specific evaluations; cleaned up existing eval.yam…
LarryOsterman 3a38b55
Removed bogus files
LarryOsterman 9304c2f
Merge remote-tracking branch 'origin/main' into larryo/split/vally-py…
LarryOsterman 093b72c
test: cover python static graders
Copilot 75af211
fix: address all 10 comments from review 4739490318
Copilot 999107e
fix: rename grader_model to judge_model in all Python eval specs (Val…
Copilot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
107 changes: 56 additions & 51 deletions
107
.github/plugins/azure-sdk-python/skills/fastapi-router-py/assets/template.py
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,142 +1,147 @@ | ||
| """ | ||
| {{ResourceName}} Router | ||
| ResourceName Router | ||
|
|
||
| Handles CRUD operations for {{resource_name}} resources. | ||
| Handles CRUD operations for resource_name resources. | ||
|
|
||
| Template placeholders to replace: | ||
| - ResourceName (PascalCase) | ||
| - resource_name (snake_case) | ||
| - resource_plural (plural snake_case) | ||
| """ | ||
|
|
||
| from typing import Optional | ||
| from fastapi import APIRouter, Depends, HTTPException, Query, status | ||
|
|
||
| from app.auth.jwt import get_current_user, get_current_user_required | ||
| from app.models.user import User | ||
| from app.models.{{resource_name}} import ( | ||
| {{ResourceName}}, | ||
| {{ResourceName}}Create, | ||
| {{ResourceName}}Update, | ||
| from app.models.resource_name import ( | ||
| ResourceName, | ||
| ResourceNameCreate, | ||
| ResourceNameUpdate, | ||
| ) | ||
| from app.services.{{resource_name}}_service import {{ResourceName}}Service | ||
| from app.services.resource_name_service import ResourceNameService | ||
|
|
||
| router = APIRouter(prefix="/api", tags=["{{resource_plural}}"]) | ||
| router = APIRouter(prefix="/api", tags=["resource_plural"]) | ||
|
|
||
|
|
||
| # ============================================================================ | ||
| # Dependencies | ||
| # ============================================================================ | ||
|
|
||
|
|
||
| def get_service() -> {{ResourceName}}Service: | ||
| def get_service() -> ResourceNameService: | ||
| """Dependency to get service instance.""" | ||
| return {{ResourceName}}Service() | ||
| return ResourceNameService() | ||
|
|
||
|
|
||
| # ============================================================================ | ||
| # Endpoints | ||
| # ============================================================================ | ||
|
|
||
|
|
||
| @router.get("/{{resource_plural}}", response_model=list[{{ResourceName}}]) | ||
| async def list_{{resource_plural}}( | ||
| @router.get("/resource_plural", response_model=list[ResourceName]) | ||
| async def list_resource_plural( | ||
| limit: int = Query(default=50, ge=1, le=100), | ||
| offset: int = Query(default=0, ge=0), | ||
| current_user: Optional[User] = Depends(get_current_user), | ||
| service: {{ResourceName}}Service = Depends(get_service), | ||
| ) -> list[{{ResourceName}}]: | ||
| service: ResourceNameService = Depends(get_service), | ||
| ) -> list[ResourceName]: | ||
| """ | ||
| List all {{resource_plural}}. | ||
| List all resource_plural. | ||
|
|
||
| - **limit**: Maximum number of items to return (1-100) | ||
| - **offset**: Number of items to skip | ||
| """ | ||
| return await service.list_{{resource_plural}}(limit=limit, offset=offset) | ||
| return await service.list_resource_plural(limit=limit, offset=offset) | ||
|
|
||
|
|
||
| @router.get("/{{resource_plural}}/{{{resource_name}}_id}", response_model={{ResourceName}}) | ||
| async def get_{{resource_name}}( | ||
| {{resource_name}}_id: str, | ||
| @router.get("/resource_plural/{resource_name_id}", response_model=ResourceName) | ||
| async def get_resource_name( | ||
| resource_name_id: str, | ||
| current_user: Optional[User] = Depends(get_current_user), | ||
| service: {{ResourceName}}Service = Depends(get_service), | ||
| ) -> {{ResourceName}}: | ||
| service: ResourceNameService = Depends(get_service), | ||
| ) -> ResourceName: | ||
| """ | ||
| Get a specific {{resource_name}} by ID. | ||
| Get a specific resource_name by ID. | ||
|
|
||
| Raises 404 if not found. | ||
| """ | ||
| result = await service.get_{{resource_name}}_by_id({{resource_name}}_id) | ||
| result = await service.get_resource_name_by_id(resource_name_id) | ||
| if result is None: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_404_NOT_FOUND, | ||
| detail="{{ResourceName}} not found", | ||
| detail="ResourceName not found", | ||
| ) | ||
| return result | ||
|
|
||
|
|
||
| @router.post( | ||
| "/{{resource_plural}}", | ||
| response_model={{ResourceName}}, | ||
| "/resource_plural", | ||
| response_model=ResourceName, | ||
| status_code=status.HTTP_201_CREATED, | ||
| ) | ||
| async def create_{{resource_name}}( | ||
| data: {{ResourceName}}Create, | ||
| async def create_resource_name( | ||
| data: ResourceNameCreate, | ||
| current_user: User = Depends(get_current_user_required), | ||
| service: {{ResourceName}}Service = Depends(get_service), | ||
| ) -> {{ResourceName}}: | ||
| service: ResourceNameService = Depends(get_service), | ||
| ) -> ResourceName: | ||
| """ | ||
| Create a new {{resource_name}}. | ||
| Create a new resource_name. | ||
|
|
||
| Requires authentication. | ||
| """ | ||
| return await service.create_{{resource_name}}(data, current_user.id) | ||
| return await service.create_resource_name(data, current_user.id) | ||
|
|
||
|
|
||
| @router.patch("/{{resource_plural}}/{{{resource_name}}_id}", response_model={{ResourceName}}) | ||
| async def update_{{resource_name}}( | ||
| {{resource_name}}_id: str, | ||
| data: {{ResourceName}}Update, | ||
| @router.patch("/resource_plural/{resource_name_id}", response_model=ResourceName) | ||
| async def update_resource_name( | ||
| resource_name_id: str, | ||
| data: ResourceNameUpdate, | ||
| current_user: User = Depends(get_current_user_required), | ||
| service: {{ResourceName}}Service = Depends(get_service), | ||
| ) -> {{ResourceName}}: | ||
| service: ResourceNameService = Depends(get_service), | ||
| ) -> ResourceName: | ||
| """ | ||
| Update an existing {{resource_name}}. | ||
| Update an existing resource_name. | ||
|
|
||
| Requires authentication and ownership. | ||
| """ | ||
| # Verify ownership | ||
| existing = await service.get_{{resource_name}}_by_id({{resource_name}}_id) | ||
| existing = await service.get_resource_name_by_id(resource_name_id) | ||
| if existing is None: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_404_NOT_FOUND, | ||
| detail="{{ResourceName}} not found", | ||
| detail="ResourceName not found", | ||
| ) | ||
|
|
||
| # Optional: Check ownership | ||
| # if existing.author_id != current_user.id: | ||
| # raise HTTPException( | ||
| # status_code=status.HTTP_403_FORBIDDEN, | ||
| # detail="Not authorized to update this {{resource_name}}", | ||
| # detail="Not authorized to update this resource_name", | ||
| # ) | ||
|
|
||
| return await service.update_{{resource_name}}({{resource_name}}_id, data) | ||
| return await service.update_resource_name(resource_name_id, data) | ||
|
|
||
|
|
||
| @router.delete( | ||
| "/{{resource_plural}}/{{{resource_name}}_id}", | ||
| "/resource_plural/{resource_name_id}", | ||
| status_code=status.HTTP_204_NO_CONTENT, | ||
| ) | ||
| async def delete_{{resource_name}}( | ||
| {{resource_name}}_id: str, | ||
| async def delete_resource_name( | ||
| resource_name_id: str, | ||
| current_user: User = Depends(get_current_user_required), | ||
| service: {{ResourceName}}Service = Depends(get_service), | ||
| service: ResourceNameService = Depends(get_service), | ||
| ) -> None: | ||
| """ | ||
| Delete a {{resource_name}}. | ||
| Delete a resource_name. | ||
|
|
||
| Requires authentication and ownership. | ||
| """ | ||
| existing = await service.get_{{resource_name}}_by_id({{resource_name}}_id) | ||
| existing = await service.get_resource_name_by_id(resource_name_id) | ||
| if existing is None: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_404_NOT_FOUND, | ||
| detail="{{ResourceName}} not found", | ||
| detail="ResourceName not found", | ||
| ) | ||
|
|
||
| await service.delete_{{resource_name}}({{resource_name}}_id) | ||
| await service.delete_resource_name(resource_name_id) |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| """Validate Python syntax across skill files and Python fenced snippets. | ||
|
|
||
| This script checks: | ||
| 1) Python files under .github/plugins/*/skills and .github/skills | ||
| 2) Python code fences in SKILL.md files under those trees | ||
|
|
||
| It exits non-zero if any syntax error is found. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import ast | ||
| import re | ||
| import sys | ||
| from pathlib import Path | ||
|
|
||
|
|
||
| PY_FENCE_RE = re.compile(r"```(?:python|py)\n(.*?)\n```", re.S) | ||
|
|
||
|
|
||
|
|
||
| def check_python_file(path: Path, repo_root: Path, errors: list[str]) -> int: | ||
| try: | ||
| ast.parse(path.read_text(encoding="utf-8")) | ||
| return 1 | ||
| except SyntaxError as exc: | ||
| rel = path.relative_to(repo_root).as_posix() | ||
| line = exc.lineno or 0 | ||
| text = (exc.text or "").strip() | ||
| errors.append(f"{rel}:{line}: {exc.msg} :: {text}") | ||
| return 0 | ||
|
|
||
|
|
||
| def check_markdown_fences(path: Path, repo_root: Path, errors: list[str]) -> int: | ||
| text = path.read_text(encoding="utf-8") | ||
| checked = 0 | ||
|
|
||
| for block_idx, match in enumerate(PY_FENCE_RE.finditer(text), 1): | ||
| code = match.group(1) | ||
| checked += 1 | ||
| try: | ||
| ast.parse(code) | ||
| except SyntaxError as exc: | ||
| start_line = text[: match.start()].count("\n") + 1 | ||
| line_in_block = exc.lineno or 0 | ||
| approx_file_line = start_line + line_in_block | ||
| rel = path.relative_to(repo_root).as_posix() | ||
| snippet = (exc.text or "").strip() | ||
| errors.append( | ||
| f"{rel}:block {block_idx} (~L{approx_file_line}): {exc.msg} :: {snippet}" | ||
| ) | ||
|
|
||
| return checked | ||
|
|
||
|
|
||
| def main() -> int: | ||
| repo_root = Path(__file__).resolve().parents[2] | ||
| roots = [repo_root / ".github" / "plugins", repo_root / ".github" / "skills"] | ||
|
|
||
| errors: list[str] = [] | ||
| py_checked = 0 | ||
| fence_checked = 0 | ||
|
|
||
| for root in roots: | ||
| if not root.exists(): | ||
| continue | ||
|
|
||
| if root.name == "plugins": | ||
| py_files = root.rglob("skills/**/*.py") | ||
|
|
||
| md_files = root.rglob("skills/**/SKILL.md") | ||
| else: | ||
| py_files = root.rglob("**/*.py") | ||
| md_files = root.rglob("**/SKILL.md") | ||
|
|
||
| for py_path in py_files: | ||
| py_checked += check_python_file(py_path, repo_root, errors) | ||
|
|
||
| for md_path in md_files: | ||
| fence_checked += check_markdown_fences(md_path, repo_root, errors) | ||
|
|
||
| print(f"Checked Python files: {py_checked}") | ||
| print(f"Checked Python code fences: {fence_checked}") | ||
|
|
||
| if errors: | ||
| print("\nPython syntax errors detected:") | ||
| for err in errors: | ||
| print(f"- {err}") | ||
| return 1 | ||
|
|
||
| print("All checked Python files and snippets are syntactically valid.") | ||
| return 0 | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| sys.exit(main()) | ||
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR description has been updated to reflect the actual scope: 145 changed files spanning
tests/scenarios, plugin skill content under.github/plugins/, repository validators, and a CI workflow update under.github/. The original "178 files undertests/" count was from an earlier draft before files were reorganized.