diff --git a/python/.github/instructions/python.instructions.md b/python/.github/instructions/python.instructions.md index 6c478a0395..ccd77f3045 100644 --- a/python/.github/instructions/python.instructions.md +++ b/python/.github/instructions/python.instructions.md @@ -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/`. diff --git a/python/.github/skills/python-code-quality/SKILL.md b/python/.github/skills/python-code-quality/SKILL.md new file mode 100644 index 0000000000..9a1ba521b3 --- /dev/null +++ b/python/.github/skills/python-code-quality/SKILL.md @@ -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 diff --git a/python/.github/skills/python-development/SKILL.md b/python/.github/skills/python-development/SKILL.md new file mode 100644 index 0000000000..7b119d13d8 --- /dev/null +++ b/python/.github/skills/python-development/SKILL.md @@ -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__for_` for methods that prepare data for external services +- `_parse__from_` for methods that process data from external services diff --git a/python/.github/skills/python-package-management/SKILL.md b/python/.github/skills/python-package-management/SKILL.md new file mode 100644 index 0000000000..8784aed453 --- /dev/null +++ b/python/.github/skills/python-package-management/SKILL.md @@ -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 diff --git a/python/.github/skills/python-samples/SKILL.md b/python/.github/skills/python-samples/SKILL.md new file mode 100644 index 0000000000..b70862eb8a --- /dev/null +++ b/python/.github/skills/python-samples/SKILL.md @@ -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_.py` +- When modifying samples, update associated README files diff --git a/python/.github/skills/python-testing/SKILL.md b/python/.github/skills/python-testing/SKILL.md new file mode 100644 index 0000000000..d38423b72d --- /dev/null +++ b/python/.github/skills/python-testing/SKILL.md @@ -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 diff --git a/python/.pre-commit-config.yaml b/python/.pre-commit-config.yaml index 24de9fc7a0..dfdf60a61b 100644 --- a/python/.pre-commit-config.yaml +++ b/python/.pre-commit-config.yaml @@ -31,6 +31,9 @@ repos: name: Detect Private Keys - id: check-added-large-files name: Check Added Large Files + - id: no-commit-to-branch + name: Protect main branch + args: [--branch, main] - repo: https://github.com/pre-commit/pre-commit-hooks rev: v6.0.0 hooks: diff --git a/python/AGENTS.md b/python/AGENTS.md index 62d52608a3..1a7e430195 100644 --- a/python/AGENTS.md +++ b/python/AGENTS.md @@ -5,13 +5,20 @@ Instructions for AI coding agents working in the Python codebase. **Key Documentation:** - [DEV_SETUP.md](DEV_SETUP.md) - Development environment setup and available poe tasks - [CODING_STANDARD.md](CODING_STANDARD.md) - Coding standards, docstring format, and performance guidelines +- [samples/SAMPLE_GUIDELINES.md](samples/SAMPLE_GUIDELINES.md) - Sample structure and guidelines + +**Agent Skills** (`.github/skills/`) — detailed, task-specific instructions loaded on demand: +- `python-development` — coding standards, type annotations, docstrings, logging, performance +- `python-testing` — test structure, fixtures, async mode, running tests +- `python-code-quality` — linting, formatting, type checking, prek hooks, CI workflow +- `python-package-management` — monorepo structure, lazy loading, versioning, new packages +- `python-samples` — sample file structure, PEP 723, documentation guidelines ## Maintaining Documentation -When making changes to a package, check if the package's `AGENTS.md` file needs updates. This includes: -- Adding/removing/renaming public classes or functions -- Changing the package's purpose or architecture -- Modifying import paths or usage patterns +When making changes to a package, check if the following need updates: +- The package's `AGENTS.md` file (adding/removing/renaming public APIs, architecture changes, import path changes) +- The agent skills in `.github/skills/` if conventions, commands, or workflows change ## Quick Reference @@ -30,6 +37,7 @@ python/ │ ├── ollama/ # agent-framework-ollama │ └── ... # Other provider packages ├── samples/ # Sample code and examples +├── .github/skills/ # Agent skills for Copilot └── tests/ # Integration tests ``` @@ -39,32 +47,6 @@ python/ - Provider packages (`azure-ai`, `anthropic`, etc.) extend core with specific integrations - Core uses lazy loading via `__getattr__` in provider folders (e.g., `agent_framework/azure/`) -### Import Patterns - -```python -# Core imports -from agent_framework import ChatAgent, ChatMessage, tool - -# Provider imports (lazy-loaded) -from agent_framework.openai import OpenAIChatClient -from agent_framework.azure import AzureOpenAIChatClient, AzureAIAgentClient -``` - -## Key Conventions - -- **Copyright**: `# Copyright (c) Microsoft. All rights reserved.` at top of all `.py` files -- **Types**: Always specify return types and parameter types; use `Type | None` not `Optional` -- **Logging**: `from agent_framework import get_logger` (never `import logging`) -- **Docstrings**: Google-style for public APIs -- **Tests**: Do not use `@pytest.mark.asyncio` (auto mode enabled); run only related tests, not the entire suite -- **Line length**: 120 characters -- **Comments**: Avoid excessive comments; prefer clear code -- **Formatting**: Format only files you changed, not the entire codebase - -## Samples - -See [samples/SAMPLE_GUIDELINES.md](samples/SAMPLE_GUIDELINES.md) for sample structure, external dependency handling (PEP 723), and syntax checking instructions. - ## Package Documentation ### Core diff --git a/python/packages/core/tests/openai/test_openai_responses_client.py b/python/packages/core/tests/openai/test_openai_responses_client.py index dac6bf23e8..88a20285d2 100644 --- a/python/packages/core/tests/openai/test_openai_responses_client.py +++ b/python/packages/core/tests/openai/test_openai_responses_client.py @@ -2259,6 +2259,7 @@ async def test_integration_options( assert "seattle" in response_value["location"].lower() +@pytest.mark.timeout(300) @pytest.mark.flaky @skip_if_openai_integration_tests_disabled async def test_integration_web_search() -> None: diff --git a/python/scripts/run_tasks_in_changed_packages.py b/python/scripts/run_tasks_in_changed_packages.py index 9773e278d4..0c33cb7f83 100644 --- a/python/scripts/run_tasks_in_changed_packages.py +++ b/python/scripts/run_tasks_in_changed_packages.py @@ -8,9 +8,18 @@ from rich import print from task_runner import build_work_items, discover_projects, run_tasks +# Tasks that need to run in all packages when core changes (type info propagates) +TYPE_CHECK_TASKS = {"pyright", "mypy"} -def get_changed_packages(projects: list[Path], changed_files: list[str], workspace_root: Path) -> set[Path]: - """Determine which packages have changed files.""" + +def get_changed_packages( + projects: list[Path], changed_files: list[str], workspace_root: Path +) -> tuple[set[Path], bool]: + """Determine which packages have changed files. + + Returns: + A tuple of (changed_packages, core_package_changed). + """ changed_packages: set[Path] = set() core_package_changed = False @@ -32,20 +41,13 @@ def get_changed_packages(projects: list[Path], changed_files: list[str], workspa # Check if the file is within this project directory abs_path.relative_to(project_abs) changed_packages.add(project) - # Check if the core package was changed if project == Path("packages/core"): core_package_changed = True break except ValueError: - # File is not in this project continue - # If core package changed, check all packages - if core_package_changed: - print("[yellow]Core package changed - checking all packages[/yellow]") - return set(projects) - - return changed_packages + return changed_packages, core_package_changed def main() -> None: @@ -63,17 +65,33 @@ def main() -> None: if not args.files or args.files == ["."]: task_list = ", ".join(args.tasks) print(f"[yellow]No specific files provided, running {task_list} in all packages[/yellow]") - target_packages = sorted(set(projects)) + work_items = build_work_items(sorted(set(projects)), args.tasks) else: - changed_packages = get_changed_packages(projects, args.files, workspace_root) - if changed_packages: - print(f"[cyan]Detected changes in packages: {', '.join(str(p) for p in sorted(changed_packages))}[/cyan]") - else: - print(f"[yellow]No changes detected in any package, skipping[/yellow]") + changed_packages, core_changed = get_changed_packages(projects, args.files, workspace_root) + if not changed_packages: + print("[yellow]No changes detected in any package, skipping[/yellow]") return - target_packages = sorted(changed_packages) - work_items = build_work_items(target_packages, args.tasks) + print(f"[cyan]Detected changes in packages: {', '.join(str(p) for p in sorted(changed_packages))}[/cyan]") + + # File-local tasks (fmt, lint) only run in packages with actual changes. + # Type-checking tasks (pyright, mypy) run in all packages when core changes, + # because type changes in core propagate to downstream packages. + local_tasks = [t for t in args.tasks if t not in TYPE_CHECK_TASKS] + type_tasks = [t for t in args.tasks if t in TYPE_CHECK_TASKS] + + work_items = build_work_items(sorted(changed_packages), local_tasks) + if type_tasks: + if core_changed: + print("[yellow]Core package changed - type-checking all packages[/yellow]") + work_items += build_work_items(sorted(set(projects)), type_tasks) + else: + work_items += build_work_items(sorted(changed_packages), type_tasks) + + if not work_items: + print("[yellow]No matching tasks found in any package[/yellow]") + return + run_tasks(work_items, workspace_root, sequential=args.seq)