mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
Python: Fix prek runner duplication and add skills (#3791)
* Python: fix prek runner running fmt/lint in all packages on core change When a core package file changed, run_tasks_in_changed_packages.py ran fmt, lint, and pyright in ALL 22 packages (66 tasks). Only type-checking tasks (pyright, mypy) need to propagate to all packages since type changes in core affect downstream packages. File-local tasks (fmt, lint) only need to run in packages with actual file changes. This reduces a core-only change from 66 tasks to 24 tasks (2 local + 22 pyright). Also adds no-commit-to-branch builtin hook to protect the main branch from direct commits. * Python: add agent skills extracted from AGENTS.md and coding standards Add 5 skills to python/.github/skills/ following the Agent Skills format: - python-development: coding standards, type annotations, docstrings, logging - python-testing: test structure, fixtures, running tests, async mode - python-code-quality: linting, formatting, type checking, prek hooks, CI - python-package-management: monorepo structure, lazy loading, versioning - python-samples: sample structure, PEP 723, documentation guidelines * Python: deduplicate AGENTS.md and instructions with agent skills * updated skills * fixes from review * Python: increase timeout for web search integration test
This commit is contained in:
committed by
GitHub
Unverified
parent
56603ab472
commit
8ad66637d8
+3
-8
@@ -1,11 +1,6 @@
|
||||
---
|
||||
applyTo: '**/agent-framework/python/**'
|
||||
applyTo: 'python/**'
|
||||
---
|
||||
|
||||
See [AGENTS.md](../../AGENTS.md) for project structure, commands, and conventions.
|
||||
|
||||
Additional guidance:
|
||||
- Review existing tests and samples to understand coding style before creating new ones
|
||||
- When verifying logic, run only related tests, not the entire suite
|
||||
- Resolve all errors and warnings before running code
|
||||
- Use print statements for debugging, then remove them when done
|
||||
See [AGENTS.md](../../AGENTS.md) for project structure and package documentation.
|
||||
Detailed conventions are in the agent skills under `.github/skills/`.
|
||||
|
||||
@@ -0,0 +1,85 @@
|
||||
---
|
||||
name: python-code-quality
|
||||
description: >
|
||||
Code quality checks, linting, formatting, and type checking commands for the
|
||||
Agent Framework Python codebase. Use this when running checks, fixing lint
|
||||
errors, or troubleshooting CI failures.
|
||||
---
|
||||
|
||||
# Python Code Quality
|
||||
|
||||
## Quick Commands
|
||||
|
||||
All commands run from the `python/` directory:
|
||||
|
||||
```bash
|
||||
# Format code (ruff format, parallel across packages)
|
||||
uv run poe fmt
|
||||
|
||||
# Lint and auto-fix (ruff check, parallel across packages)
|
||||
uv run poe lint
|
||||
|
||||
# Type checking
|
||||
uv run poe pyright # Pyright (parallel across packages)
|
||||
uv run poe mypy # MyPy (parallel across packages)
|
||||
uv run poe typing # Both pyright and mypy
|
||||
|
||||
# All package-level checks in parallel (fmt + lint + pyright + mypy)
|
||||
uv run poe check-packages
|
||||
|
||||
# Full check (packages + samples + tests + markdown)
|
||||
uv run poe check
|
||||
|
||||
# Samples only
|
||||
uv run poe samples-lint # Ruff lint on samples/
|
||||
uv run poe samples-syntax # Pyright syntax check on samples/
|
||||
|
||||
# Markdown code blocks
|
||||
uv run poe markdown-code-lint
|
||||
```
|
||||
|
||||
## Pre-commit Hooks (prek)
|
||||
|
||||
Prek hooks run automatically on commit. They check only changed files and run
|
||||
package-level checks in parallel for affected packages only.
|
||||
|
||||
```bash
|
||||
# Install hooks
|
||||
uv run poe prek-install
|
||||
|
||||
# Run all hooks manually
|
||||
uv run prek run -a
|
||||
|
||||
# Run on last commit
|
||||
uv run prek run --last-commit
|
||||
```
|
||||
|
||||
When core package changes, type-checking (mypy, pyright) runs across all packages
|
||||
since type changes propagate. Format and lint only run in changed packages.
|
||||
|
||||
## Ruff Configuration
|
||||
|
||||
- Line length: 120
|
||||
- Target: Python 3.10+
|
||||
- Auto-fix enabled
|
||||
- Rules: ASYNC, B, CPY, D, E, ERA, F, FIX, I, INP, ISC, Q, RET, RSE, RUF, SIM, T20, TD, W, T100, S
|
||||
- Scripts directory is excluded from checks
|
||||
|
||||
## Pyright Configuration
|
||||
|
||||
- Strict mode enabled
|
||||
- Excludes: tests, .venv, packages/devui/frontend
|
||||
|
||||
## Parallel Execution
|
||||
|
||||
The task runner (`scripts/task_runner.py`) executes the cross-product of
|
||||
(package × task) in parallel using ThreadPoolExecutor. Single items run
|
||||
in-process with streaming output.
|
||||
|
||||
## CI Workflow
|
||||
|
||||
CI splits into 4 parallel jobs:
|
||||
1. **Pre-commit hooks** — lightweight hooks (SKIP=poe-check)
|
||||
2. **Package checks** — fmt/lint/pyright via check-packages
|
||||
3. **Samples & markdown** — samples-lint, samples-syntax, markdown-code-lint
|
||||
4. **Mypy** — change-detected mypy checks
|
||||
+109
@@ -0,0 +1,109 @@
|
||||
---
|
||||
name: python-development
|
||||
description: >
|
||||
Coding standards, conventions, and patterns for developing Python code in the
|
||||
Agent Framework repository. Use this when writing or modifying Python source
|
||||
files in the python/ directory.
|
||||
---
|
||||
|
||||
# Python Development Standards
|
||||
|
||||
## File Header
|
||||
|
||||
Every `.py` file must start with:
|
||||
|
||||
```python
|
||||
# Copyright (c) Microsoft. All rights reserved.
|
||||
```
|
||||
|
||||
## Type Annotations
|
||||
|
||||
- Always specify return types and parameter types
|
||||
- Use `Type | None` instead of `Optional[Type]`
|
||||
- Use `from __future__ import annotations` to enable postponed evaluation
|
||||
- Use suffix `T` for TypeVar names: `ChatResponseT = TypeVar("ChatResponseT", bound=ChatResponse)`
|
||||
- Use `Mapping` instead of `MutableMapping` for read-only input parameters
|
||||
- Prefer `# type: ignore[...]` over unnecessary casts, or `isinstance` checks, when these are internally called and executed methods
|
||||
But make sure the ignore is specific for both mypy and pyright so that we don't miss other mistakes
|
||||
|
||||
## Function Parameters
|
||||
|
||||
- Positional parameters: up to 3 fully expected parameters
|
||||
- Use keyword-only arguments (after `*`) for optional parameters
|
||||
- Provide string-based overrides to avoid requiring extra imports:
|
||||
|
||||
```python
|
||||
def create_agent(name: str, tool_mode: Literal['auto', 'required', 'none'] | ChatToolMode) -> Agent:
|
||||
if isinstance(tool_mode, str):
|
||||
tool_mode = ChatToolMode(tool_mode)
|
||||
```
|
||||
|
||||
- Avoid shadowing built-ins (use `next_handler` instead of `next`)
|
||||
- Avoid `**kwargs` unless needed for subclass extensibility; prefer named parameters
|
||||
|
||||
## Docstrings
|
||||
|
||||
Use Google-style docstrings for all public APIs:
|
||||
|
||||
```python
|
||||
def equal(arg1: str, arg2: str) -> bool:
|
||||
"""Compares two strings and returns True if they are the same.
|
||||
|
||||
Args:
|
||||
arg1: The first string to compare.
|
||||
arg2: The second string to compare.
|
||||
|
||||
Returns:
|
||||
True if the strings are the same, False otherwise.
|
||||
|
||||
Raises:
|
||||
ValueError: If one of the strings is empty.
|
||||
"""
|
||||
```
|
||||
|
||||
- Always document Agent Framework specific exceptions
|
||||
- Explicitly use `Keyword Args` when applicable
|
||||
- Only document standard Python exceptions when the condition is non-obvious
|
||||
|
||||
## Import Structure
|
||||
|
||||
```python
|
||||
# Core
|
||||
from agent_framework import ChatAgent, ChatMessage, tool
|
||||
|
||||
# Components
|
||||
from agent_framework.observability import enable_instrumentation
|
||||
|
||||
# Connectors (lazy-loaded)
|
||||
from agent_framework.openai import OpenAIChatClient
|
||||
from agent_framework.azure import AzureOpenAIChatClient
|
||||
```
|
||||
|
||||
## Public API and Exports
|
||||
|
||||
Define `__all__` in each module. Avoid `from module import *` in `__init__.py` files:
|
||||
|
||||
```python
|
||||
__all__ = ["ChatAgent", "ChatMessage", "ChatResponse"]
|
||||
|
||||
from ._agents import ChatAgent
|
||||
from ._types import ChatMessage, ChatResponse
|
||||
```
|
||||
|
||||
## Performance Guidelines
|
||||
|
||||
- Cache expensive computations (e.g., JSON schema generation)
|
||||
- Prefer `match/case` on `.type` attribute over `isinstance()` in hot paths
|
||||
- Avoid redundant serialization — compute once, reuse
|
||||
|
||||
## Style
|
||||
|
||||
- Line length: 120 characters
|
||||
- Format only files you changed, not the entire codebase
|
||||
- Prefer attributes over inheritance when parameters are mostly the same
|
||||
- Async by default — assume everything is asynchronous
|
||||
|
||||
## Naming Conventions for Connectors
|
||||
|
||||
- `_prepare_<object>_for_<purpose>` for methods that prepare data for external services
|
||||
- `_parse_<object>_from_<source>` for methods that process data from external services
|
||||
@@ -0,0 +1,103 @@
|
||||
---
|
||||
name: python-package-management
|
||||
description: >
|
||||
Guide for managing packages in the Agent Framework Python monorepo, including
|
||||
creating new connector packages, versioning, and the lazy-loading pattern.
|
||||
Use this when adding, modifying, or releasing packages.
|
||||
---
|
||||
|
||||
# Python Package Management
|
||||
|
||||
## Monorepo Structure
|
||||
|
||||
```
|
||||
python/
|
||||
├── pyproject.toml # Root package (agent-framework)
|
||||
├── packages/
|
||||
│ ├── core/ # agent-framework-core (main package)
|
||||
│ ├── azure-ai/ # agent-framework-azure-ai
|
||||
│ ├── anthropic/ # agent-framework-anthropic
|
||||
│ └── ... # Other connector packages
|
||||
```
|
||||
|
||||
- `agent-framework-core` contains core abstractions and OpenAI/Azure OpenAI built-in
|
||||
- Provider packages extend core with specific integrations
|
||||
- Root `agent-framework` depends on `agent-framework-core[all]`
|
||||
|
||||
## Dependency Management
|
||||
|
||||
Uses [uv](https://github.com/astral-sh/uv) for dependency management and
|
||||
[poethepoet](https://github.com/nat-n/poethepoet) for task automation.
|
||||
|
||||
```bash
|
||||
# Full setup (venv + install + prek hooks)
|
||||
uv run poe setup
|
||||
|
||||
# Install/update all dependencies
|
||||
uv run poe install
|
||||
|
||||
# Create venv with specific Python version
|
||||
uv run poe venv --python 3.12
|
||||
```
|
||||
|
||||
## Lazy Loading Pattern
|
||||
|
||||
Provider folders in core use `__getattr__` to lazy load from connector packages:
|
||||
|
||||
```python
|
||||
# In agent_framework/azure/__init__.py
|
||||
_IMPORTS: dict[str, tuple[str, str]] = {
|
||||
"AzureAIAgentClient": ("agent_framework_azure_ai", "agent-framework-azure-ai"),
|
||||
}
|
||||
|
||||
def __getattr__(name: str) -> Any:
|
||||
if name in _IMPORTS:
|
||||
import_path, package_name = _IMPORTS[name]
|
||||
try:
|
||||
return getattr(importlib.import_module(import_path), name)
|
||||
except ModuleNotFoundError as exc:
|
||||
raise ModuleNotFoundError(
|
||||
f"The package {package_name} is required to use `{name}`. "
|
||||
f"Install it with: pip install {package_name}"
|
||||
) from exc
|
||||
```
|
||||
|
||||
## Adding a New Connector Package
|
||||
|
||||
**Important:** Do not create a new package unless approved by the core team.
|
||||
|
||||
### Initial Release (Preview)
|
||||
|
||||
1. Create directory under `packages/` (e.g., `packages/my-connector/`)
|
||||
2. Add the package to `tool.uv.sources` in root `pyproject.toml`
|
||||
3. Include samples inside the package (e.g., `packages/my-connector/samples/`)
|
||||
4. Do **NOT** add to `[all]` extra in `packages/core/pyproject.toml`
|
||||
5. Do **NOT** create lazy loading in core yet
|
||||
|
||||
### Promotion to Stable
|
||||
|
||||
1. Move samples to root `samples/` folder
|
||||
2. Add to `[all]` extra in `packages/core/pyproject.toml`
|
||||
3. Create provider folder in `agent_framework/` with lazy loading `__init__.py`
|
||||
|
||||
## Versioning
|
||||
|
||||
- All non-core packages declare a lower bound on `agent-framework-core`
|
||||
- When core version bumps with breaking changes, update the lower bound in all packages
|
||||
- Non-core packages version independently; only raise core bound when using new core APIs
|
||||
|
||||
## Installation Options
|
||||
|
||||
```bash
|
||||
pip install agent-framework-core # Core only
|
||||
pip install agent-framework-core[all] # Core + all connectors
|
||||
pip install agent-framework # Same as core[all]
|
||||
pip install agent-framework-azure-ai # Specific connector (pulls in core)
|
||||
```
|
||||
|
||||
## Maintaining Documentation
|
||||
|
||||
When changing a package, check if its `AGENTS.md` needs updates:
|
||||
- Adding/removing/renaming public classes or functions
|
||||
- Changing the package's purpose or architecture
|
||||
- Modifying import paths or usage patterns
|
||||
+77
@@ -0,0 +1,77 @@
|
||||
---
|
||||
name: python-samples
|
||||
description: >
|
||||
Guidelines for creating and modifying sample code in the Agent Framework
|
||||
Python codebase. Use this when writing new samples or updating existing ones.
|
||||
---
|
||||
|
||||
# Python Samples
|
||||
|
||||
## File Structure
|
||||
|
||||
Every sample file follows this order:
|
||||
|
||||
1. PEP 723 inline script metadata (if external dependencies needed)
|
||||
2. Copyright header: `# Copyright (c) Microsoft. All rights reserved.`
|
||||
3. Required imports
|
||||
4. Module docstring: `"""This sample demonstrates..."""`
|
||||
5. Helper functions
|
||||
6. Main function(s) demonstrating functionality
|
||||
7. Entry point: `if __name__ == "__main__": asyncio.run(main())`
|
||||
|
||||
## External Dependencies
|
||||
|
||||
Use [PEP 723](https://peps.python.org/pep-0723/) inline script metadata for
|
||||
external packages not in the dev environment:
|
||||
|
||||
```python
|
||||
# /// script
|
||||
# requires-python = ">=3.10"
|
||||
# dependencies = [
|
||||
# "some-external-package",
|
||||
# ]
|
||||
# ///
|
||||
# Run with: uv run samples/path/to/script.py
|
||||
|
||||
# Copyright (c) Microsoft. All rights reserved.
|
||||
```
|
||||
|
||||
Do **not** add sample-only dependencies to the root `pyproject.toml` dev group.
|
||||
|
||||
## Syntax Checking
|
||||
|
||||
```bash
|
||||
# Check samples for syntax errors and missing imports
|
||||
uv run poe samples-syntax
|
||||
|
||||
# Lint samples
|
||||
uv run poe samples-lint
|
||||
```
|
||||
|
||||
## Documentation
|
||||
|
||||
Samples should be over-documented:
|
||||
|
||||
1. Include a README.md in each set of samples
|
||||
2. Add a summary docstring under imports explaining the purpose and key components
|
||||
3. Mark code sections with numbered comments:
|
||||
```python
|
||||
# 1. Create the client instance.
|
||||
...
|
||||
# 2. Create the agent with the client.
|
||||
...
|
||||
```
|
||||
4. Include expected output at the end of the file:
|
||||
```python
|
||||
"""
|
||||
Sample output:
|
||||
User:> Why is the sky blue?
|
||||
Assistant:> The sky is blue due to Rayleigh scattering...
|
||||
"""
|
||||
```
|
||||
|
||||
## Guidelines
|
||||
|
||||
- **Incremental complexity** — start simple, build up (step1, step2, ...)
|
||||
- **Getting started naming**: `step<number>_<name>.py`
|
||||
- When modifying samples, update associated README files
|
||||
+84
@@ -0,0 +1,84 @@
|
||||
---
|
||||
name: python-testing
|
||||
description: >
|
||||
Guidelines for writing and running tests in the Agent Framework Python
|
||||
codebase. Use this when creating, modifying, or running tests.
|
||||
---
|
||||
|
||||
# Python Testing
|
||||
|
||||
We strive for at least 85% test coverage across the codebase, with a focus on core packages and critical paths. Tests should be fast, reliable, and maintainable.
|
||||
When adding new code, check that the relevant sections of the codebase are covered by tests, and add new tests as needed. When modifying existing code, update or add tests to cover the changes.
|
||||
We run tests in two stages, for a PR each commit is tested with `RUN_INTEGRATION_TESTS=false` (unit tests only), and the full suite with `RUN_INTEGRATION_TESTS=true` is run when merging.
|
||||
|
||||
## Running Tests
|
||||
|
||||
```bash
|
||||
# Run tests for all packages in parallel
|
||||
uv run poe test
|
||||
|
||||
# Run tests for a specific package
|
||||
uv run --directory packages/core poe test
|
||||
|
||||
# Run all tests in a single pytest invocation (faster, uses pytest-xdist)
|
||||
uv run poe all-tests
|
||||
|
||||
# With coverage
|
||||
uv run poe all-tests-cov
|
||||
```
|
||||
|
||||
## Test Configuration
|
||||
|
||||
- **Async mode**: `asyncio_mode = "auto"` is enabled — do NOT use `@pytest.mark.asyncio`, but do mark tests with `async def` and use `await` for async calls
|
||||
- **Timeout**: Default 60 seconds per test
|
||||
- **Import mode**: `importlib` for cross-package isolation
|
||||
|
||||
## Test Directory Structure
|
||||
|
||||
Test directories must NOT contain `__init__.py` files.
|
||||
|
||||
Non-core packages must place tests in a uniquely-named subdirectory:
|
||||
|
||||
```
|
||||
packages/anthropic/
|
||||
├── tests/
|
||||
│ └── anthropic/ # Unique subdirectory matching package name
|
||||
│ ├── conftest.py
|
||||
│ └── test_client.py
|
||||
```
|
||||
|
||||
Core package can use `tests/` directly with topic subdirectories:
|
||||
|
||||
```
|
||||
packages/core/
|
||||
├── tests/
|
||||
│ ├── conftest.py
|
||||
│ ├── core/
|
||||
│ │ └── test_agents.py
|
||||
│ └── openai/
|
||||
│ └── test_client.py
|
||||
```
|
||||
|
||||
## Fixture Guidelines
|
||||
|
||||
- Use `conftest.py` for shared fixtures within a test directory
|
||||
- Before adding new fixtures, check if existing ones can be reused or extended
|
||||
- Use descriptive names: `mapper`, `test_request`, `mock_client`
|
||||
|
||||
## File Naming
|
||||
|
||||
- Files starting with `test_` are test files — do not use this prefix for helpers
|
||||
- Use `conftest.py` for shared utilities
|
||||
|
||||
## Integration Tests
|
||||
|
||||
Tests marked with `@skip_if_..._integration_tests_disabled` require:
|
||||
- `RUN_INTEGRATION_TESTS=true` environment variable
|
||||
- Appropriate API keys in environment or `.env` file
|
||||
|
||||
## Best Practices
|
||||
|
||||
- Run only related tests, not the entire suite
|
||||
- Review existing tests to understand coding style before creating new ones
|
||||
- Use print statements for debugging, then remove them when done
|
||||
- Resolve all errors and warnings before committing
|
||||
Reference in New Issue
Block a user