Skip to content

Python: strengthen API design and PR review guidance - #10038

Open
Eduard van Valkenburg (eavanvalkenburg) wants to merge 1 commit into
mainfrom
foundry-hosting-review
Open

Eduard van Valkenburg (eavanvalkenburg) wants to merge 1 commit into
mainfrom
foundry-hosting-review

Conversation

@eavanvalkenburg

Copy link
Copy Markdown
Member

Motivation & Context

The Python Foundry Hosting design review exposed gaps in how we assess initialization, execution modes, state authority, adapter settings, and upstream capability claims. Capture the reusable lessons in the canonical standards and contributor/reviewer guidance, without turning unresolved Foundry API alternatives into repository policy.

Description & Review Guide

  • What are the major changes? Extend python/CODING_STANDARD.md with execution-mode discoverability, semantic naming, behavioral compatibility, factory/resource lifetime, state/continuation, and adapter/hook contracts. Add an ordered semantic API review checklist. Update both PR skills with import-to-initialization examples and approval boundaries. Replace stale Python formatting references in CONTRIBUTING.md and align its breaking-change text with explicit maintainer approval.
  • What is the impact of these changes? Documentation and contributor/review instructions only. No package APIs, runtime behavior, dependency manifests, or PR-template structure change. The guidance does not mandate separate host classes, choose new names, decide approval storage or background support, set Invocations status, or approve the MCP redesign. git diff --check passed; the referenced local guidance files were checked to exist. Builds, unit tests, and Markdown code-block checks were not run because no executable code or code blocks changed. The network Markdown-link workflow was not run locally.
  • What do you want reviewers to focus on? Whether the requirement levels are proportionate, the rules generalize beyond Foundry Hosting, and the checklist improves semantic review without prescribing one architecture or adding mandatory template sections. The two PR skills contain equivalent additions with location-correct links.

Related Issue

Related to #10009. This PR implements the scoped guidance follow-up, not the parent issue's package implementation work, so it intentionally does not close the parent or any sub-issue. Maintainer approval for this documentation scope is recorded in #10009 (comment). No competing open PR was found for this follow-up.

AI Assistance

  • No material AI assistance was used.
  • This is an AI-assisted contribution. I reviewed, understood, and verified all submitted content and accept responsibility for it.

AI assistance details: Copilot helped analyze the review findings, draft and edit the standards and review guidance, and prepare this PR description.

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR links to an agreed issue with no competing open PR, or the Related Issue section documents a trivial-change or repository-automation exception.
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

Copilot AI balanced review requested due to automatic review settings November 3, 2026 13:03
@agent-framework-automation agent-framework-automation Bot added documentation Usage: [Issues, PRs], Target: documentation in the code base and learn docs python Usage: [Issues, PRs], Target: Python labels Nov 3, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues were identified for this documentation-only update to Python API standards and contributor guidance.

No open issues 🎉

What changed in this PR

This documentation-only PR captures reusable lessons from the Python Foundry Hosting review in Agent Framework’s API standards and contributor guidance, without deciding unresolved hosting designs or changing runtime behavior.

Changes:

  • Expand Python standards for execution modes, naming, compatibility, resource ownership, state, and adapter contracts.
  • Add an ordered semantic API review checklist and align both PR skills on usage examples and maintainer approval boundaries.
  • Replace outdated Python formatting guidance and clarify approval requirements for breaking changes.
File Description
python/​CODING_STANDARD.md Defines additional API design, lifecycle, state, and documentation requirements.
python/​.github/​skills/​pull-requests/​SKILL.md Adds Python API review context and implementation-approval guidance.
CONTRIBUTING.md Links canonical Python standards and clarifies breaking-change approval.
.github/​skills/​pull-requests/​SKILL.md Mirrors the Python PR guidance with root-relative links.
.github/​instructions/​python.instructions.md Adds a semantic API review checklist and evidence requirements.

Review effort: Balanced

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MAF Automated Review — Iteration 1

Result: No findings
Scope: full PR (1 commit(s)): 65fdeb371d5d
Model: gpt-5.6-sol

Overview

This documentation-only PR adds ordered semantic API review guidance and expands the Python standards around execution modes, compatibility, resource ownership, state, and adapter contracts. Its strongest guardrails preserve architectural choice, require concrete evidence and explicit maintainer approval, keep the PR template unchanged, and use location-correct links that resolve in the frozen checkout. No Critical, High, or Medium defect remains after reconciling the proposed policy wording with the explicit exception and existing approval rules.

Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
No publishable findings remained after source verification for this scope.

This branch was successfully deployed

1 active deployment
github-app-auth — 65fdeb37 Deployed Nov 3, 2026 by eavanvalkenburg via team_check #8579
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Usage: [Issues, PRs], Target: documentation in the code base and learn docs python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants