diff --git a/.github/instructions/python.instructions.md b/.github/instructions/python.instructions.md index 096015c4320..0e3a45ce60b 100644 --- a/.github/instructions/python.instructions.md +++ b/.github/instructions/python.instructions.md @@ -16,3 +16,25 @@ for task-specific architecture and workflow guidance. performance, or usability impact. - Do not report `MAY` choices as violations. - Do not duplicate purely mechanical findings that Ruff, Pyright, or another required CI check will report. + +## Semantic API Review + +For public API and integration changes, apply the standards in this order: + +1. Start from public imports and the smallest realistic initialization example. Check the common path and each + materially different execution mode before reading implementation details. +2. Trace parameters through construction, factories, request preparation, execution, persistence, and cleanup. + Look for accepted-but-unused settings, mutually exclusive parameter combinations, and differences between execution + paths. Check that invalid combinations are rejected before execution or external side effects when the required + information is available. +3. Check whether names expose required inputs, data direction, and ownership without relying on constructor conditions. +4. Identify resource, history, continuation, cache, approval, and checkpoint ownership where applicable. Check + cross-request sharing and failure windows. +5. Review observable compatibility, including defaults, wire shapes, option precedence, retention, and failure + behavior, not only signature-checker results. +6. Match evidence to the claim. Offline tests do not establish live deployment behavior. An upstream fix needs + release evidence. An investigation result is not approval for its proposed implementation. + +Findings should identify the affected behavior, a concrete scenario, and its consequence. Do not prescribe a preferred +architecture as a correctness finding when multiple designs satisfy the contract. Treat unresolved alternatives as +design questions. Review author-supplied focus notes as context, not a limit on review scope. diff --git a/.github/skills/pull-requests/SKILL.md b/.github/skills/pull-requests/SKILL.md index 6b968a288dc..a727e075686 100644 --- a/.github/skills/pull-requests/SKILL.md +++ b/.github/skills/pull-requests/SKILL.md @@ -32,6 +32,18 @@ prompts: reviewers only**. Automated/AI reviewers must ignore it and review the entire change rather than narrowing scope to it. +For material Python public-API changes, include the following within this section: +- A short before/after usage example with public imports and initialization. +- The affected execution modes and their intentional differences. +- Changes to defaults, precedence, ownership, persistence, or compatibility. +- The agreed decision or investigation that supports the direction. An investigation + supplies evidence; its proposed implementation still needs maintainer agreement. + +Use an options matrix only when interacting settings make it useful. Do not add a +separate mandatory section or repeat the canonical +[Python coding standards](../../../python/CODING_STANDARD.md). For semantic review, +follow the [Python review guidance](../../instructions/python.instructions.md). + ### `### Related Issue` Link the issue the PR fixes using a GitHub closing keyword (`Fixes #123` / `Closes #123`) so it closes automatically on merge. For a non-trivial change, @@ -40,6 +52,10 @@ agreed with the direction in an issue comment. A PR with no agreed issue may be closed without detailed review. Before opening, confirm there is no other open PR for the same issue; if there is, explain how this PR differs. +A team-only issue, assignment, investigation, `/plan` comment, or suggested option +does not authorize implementation. Check the agreed scope and readiness; team-only +work remains unavailable for community pickup unless maintainers explicitly open it. + For a trivial change that does not require an issue, replace the `Fixes #` placeholder with `N/A - trivial change` and briefly explain why the change is an obvious correction whose desired result is not reasonably in dispute. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 74b7c71d926..dca225eca5f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -100,11 +100,12 @@ DO's: - **DO** follow the standard coding conventions - [.NET](https://learn.microsoft.com/dotnet/csharp/fundamentals/coding-style/coding-conventions) - - [Python](https://pypi.org/project/black/) + - [Python](./python/CODING_STANDARD.md) -- **DO** give priority to the current style of the project or file you're changing - if it diverges from the general guidelines. -- **DO** use the pre-commit hooks for python to ensure proper formatting. +- **DO** follow the repository's language-specific standards when local file style + diverges. Do not copy non-conforming legacy patterns. +- **DO** follow the [Python code-quality guidance](./python/.github/skills/python-code-quality/SKILL.md) + for formatting, linting, and repository hooks. - **DO** include tests when adding new features. When fixing bugs, start with adding a test that highlights how the current behavior is broken. - **DO** keep the discussions focused. When a new or related topic comes up @@ -128,8 +129,9 @@ DON'Ts: ### Breaking Changes Contributions must maintain API signature and behavioral compatibility. Contributions -that include breaking changes will be rejected. Please file an issue to discuss -your idea or change if you believe that a breaking change is warranted. +that include breaking changes require explicit maintainer approval and documentation. +Please file an issue and agree on the direction before implementing a breaking change. +A breaking-change label or title declares intent; it does not grant design approval. #### Python Public API Compatibility diff --git a/python/.github/skills/pull-requests/SKILL.md b/python/.github/skills/pull-requests/SKILL.md index 9d8395cdabe..b887bba384c 100644 --- a/python/.github/skills/pull-requests/SKILL.md +++ b/python/.github/skills/pull-requests/SKILL.md @@ -32,6 +32,9 @@ prompts: reviewers only**. Automated/AI reviewers must ignore it and review the entire change rather than narrowing scope to it. +For material Python public-API changes, follow the additional guidance in the +[root PR skill's Description & Review Guide section](../../../../.github/skills/pull-requests/SKILL.md#-description--review-guide). + ### `### Related Issue` Link the issue the PR fixes using a GitHub closing keyword (`Fixes #123` / `Closes #123`) so it closes automatically on merge. For a non-trivial change, @@ -40,6 +43,9 @@ agreed with the direction in an issue comment. A PR with no agreed issue may be closed without detailed review. Before opening, confirm there is no other open PR for the same issue; if there is, explain how this PR differs. +For team-only work and investigation or planning outcomes, follow the approval +boundaries in the [root PR skill's Related Issue section](../../../../.github/skills/pull-requests/SKILL.md#-related-issue). + For a trivial change that does not require an issue, replace the `Fixes #` placeholder with `N/A - trivial change` and briefly explain why the change is an obvious correction whose desired result is not reasonably in dispute. diff --git a/python/CODING_STANDARD.md b/python/CODING_STANDARD.md index 9dfdea16cf5..1ba76dfebb6 100644 --- a/python/CODING_STANDARD.md +++ b/python/CODING_STANDARD.md @@ -179,6 +179,26 @@ def create_agent( - The API MUST document who owns the mapping, where it is forwarded, and whether values are copied or mutated. - Wrappers MUST NOT accept or forward generic keyword arguments they do not use. +### Execution Modes and Configuration + +- Public APIs SHOULD make supported execution modes and their required inputs apparent from signatures and types. +- When modes require substantially different parameters, lifecycle rules, or state contracts, authors SHOULD compare + separate entry points, typed alternatives, and a combined signature. +- A long list of conditional parameter requirements SHOULD trigger an API-design review. Additional documentation + alone does not resolve an unclear API. Configuration classes must still satisfy the Import-Light Common Paths rules. +- Invalid combinations MUST fail before execution or external side effects when the required information is already + available. Factory-dependent validation MUST occur before running the resolved object. + +### Names, Direction, and Ownership + +- Names SHOULD identify the operation's direction and the object or layer it affects. +- Related settings SHOULD use the same term for the same concept. A different term requires a meaningful semantic + distinction. +- Wrappers SHOULD distinguish framework behavior, hosting-platform behavior, and downstream-service behavior. +- Public renames MUST follow the compatibility policy; improved terminology alone does not authorize a breaking change. +- Related APIs across protocols or languages SHOULD use consistent concepts and terminology, without forcing identical + signatures where their contracts or language conventions differ. + ### Interfaces and Reuse - Behavioral contracts SHOULD use `Protocol`. @@ -187,6 +207,15 @@ def create_agent( - Composition and inheritance are both acceptable. Choose based on substitutability, lifecycle, and clarity rather than a blanket preference. +### Compatibility and Feature Lifecycle + +- Compatibility review MUST cover behavior as well as signatures, including defaults, accepted combinations, wire + formats, persistence, and resource ownership. +- Upstream dependency stability MUST NOT be treated as evidence that an integration feature is stable or that a + required fix is available. +- Dependency-specific capability claims MUST be supported by the required dependency version and relevant behavioral + evidence. + ## Asynchronous Code and Resource Ownership - Use asynchronous APIs for I/O and concurrency. Keep purely synchronous work synchronous. @@ -198,6 +227,27 @@ def create_agent( - Do not close caller-injected resources. - Use synchronous or asynchronous context managers when lifecycle management is part of correct use. +### Factories and Identity-Dependent Resources + +- Factory APIs MUST document invocation frequency, access to request context, and ownership of returned objects and + their resources. +- State required across requests MUST NOT depend on request-owned object identity unless that lifetime is explicitly + guaranteed. +- Identity-dependent caches and connections MUST have an explicit isolation scope and invalidation or rebinding + contract. +- Examples MUST follow the same ownership contract as package code; sample brevity does not justify unsafe sharing. + +## State and Continuation Contracts + +- APIs that manage state MUST identify the authority for each kind of state and the purpose of each continuation + identifier. +- Independently configurable state layers MUST document their interaction, including duplicate persistence, history + divergence, branching restrictions, and unsupported combinations where applicable. +- State transitions with externally visible consequences MUST define when ownership is claimed, when state becomes + durable, and what happens after failure or retry. +- Checkpointing or retry support MUST NOT be described as exactly-once execution unless that guarantee also covers + external side effects. + ## Exception Hierarchy The authoritative classes live in @@ -304,6 +354,18 @@ Package-governance decisions and detailed validation commands live in ## Documentation +### Adapter and Option Contracts + +- Public adapter parameters MUST document whether the adapter interprets, translates, forwards, overrides, or ignores + their values, including unsupported or ineffective settings. +- Documentation for upstream SDK types SHOULD link to the upstream contract and explain adapter-specific behavior + or limitations. +- APIs with multiple configuration sources MUST document precedence and distinguish construction defaults from + per-run overrides. +- Option hooks MUST document their inputs, return contract, invocation timing, supported execution paths, mutation + behavior, and validation applied after the hook. +- Examples SHOULD include public imports and show the smallest complete initialization for the scenario. + ### Module Docstrings - Every public module, including public `__init__.py` modules, MUST have a concise module docstring focused on its