Skip to content

Python: docs: correct three README errors found by following them - #8822

Merged
Eduard van Valkenburg (eavanvalkenburg) merged 1 commit into
microsoft:mainfrom
feiiiiii5:fix/readme-env-and-sample-refs
Sep 29, 2026
Merged

Eduard van Valkenburg (eavanvalkenburg) merged 1 commit into
microsoft:mainfrom
feiiiiii5:fix/readme-env-and-sample-refs

Conversation

@feiiiiii5

Copy link
Copy Markdown

Motivation & Context

Found by installing the published package in a clean environment and following the docs verbatim, in the order a new user would. Three README errors, none of which need a code change. Details and reproduction in #8821.

  1. README.md:131 reads FOUNDRY_MODEL_DEPLOYMENT_NAME. FoundryChatClient reads FOUNDRY_MODEL (_chat_client.py:211 sets env_prefix="FOUNDRY_"; :220 raises ValueError: Model is required. Set via 'model' parameter or 'FOUNDRY_MODEL'). The wrong name is a real variable in .github/workflows/dotnet-integration-tests.yml:112, which is likely why it survived — it is the pipeline's own, not the client's. python/README.md:58 already has it right, so the two READMEs disagree.
  2. python/samples/01-get-started/README.md marked FOUNDRY_MODEL "optional, defaults to gpt-4o". There is no default; unset, the constructor raises. The code is deliberately so — test_foundry_chat_client.py:345 pins it with pytest.raises(ValueError, match="Model is required"). I removed only the wrong comment.
  3. The same file referred to a Sample 08 that does not exist in this directory (the table lists 1–7, and there is no 08_* under python/samples/). The hosting samples moved to the durable-extension repo, which the same file already links further down, so I deleted the stale line rather than rewording it.

Description & Review Guide

  • What are the major changes? Three lines of documentation: one environment variable name, one incorrect "optional" comment, one stale sample reference. No code, no test, no packaging change.
  • What is the impact of these changes? A reader who copies the root quickstart gets ValueError: Model is required instead of a working first run. A reader who believes FOUNDRY_MODEL is optional and skips it also gets that error — which is how 05_end_to_end.py fails, since it passes no model of its own.
  • What do you want reviewers to focus on? Whether deleting the Sample 08 line is right, as opposed to restoring the sample or rewording the line. That is the only judgement call; the other two are corrections with the code as the authority.

How this was verified. Clean venv on Python 3.13, uv pip install agent-framework → 1.19.0, then the root quickstart in order. The ValueError for the unset model reproduces twice; with FOUNDRY_MODEL set the same snippet runs. I have not executed samples 01–05 end to end — they stop at CredentialUnavailableError('Azure CLI not found on path'), the documented az login prerequisite.

I have not run the Python test suite for this change, since it touches only two Markdown files.

Related Issue

Fixes #8821

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 is linked to an issue and there is no other open PR for this issue (see Related Issue above).
  • 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.

The root quickstart reads FOUNDRY_MODEL_DEPLOYMENT_NAME, which is a
variable in our dotnet integration workflow but not one
FoundryChatClient reads -- it uses FOUNDRY_MODEL, as python/README.md
already says. Uncommenting the line as written raises "Model is
required".

01-get-started marks FOUNDRY_MODEL "optional, defaults to gpt-4o". It
has no default; the constructor raises when it is unset, which
test_foundry_chat_client.py pins. Only the comment is wrong.

01-get-started also refers to a Sample 08 that does not exist here. The
hosting samples moved to the durable-extension repo, which the same
file already links.
Copilot AI balanced review requested due to automatic review settings September 29, 2026 07:30
@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 Sep 29, 2026
@github-actions github-actions Bot changed the title docs: correct three README errors found by following them Python: docs: correct three README errors found by following them Sep 29, 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.

Copilot review overview

🟢 Approval recommended

All documentation changes align with the client implementation, tests, and current sample layout.

Review effort: Balanced
Findings: None

What changed in this PR

Corrects Python quickstart documentation to match FoundryChatClient behavior and current sample structure.

Changes:

  • Uses the correct FOUNDRY_MODEL environment variable.
  • Removes an incorrect default-model claim and stale Sample 08 reference.
File Description
README.md Corrects the Foundry model environment variable.
python/​samples/​01-get-started/​README.md Corrects prerequisites and removes an obsolete sample reference.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

fei (@feiiiiii5) Thanks — the documentation corrections match the current implementation, the dedicated review found no issues, and all reported checks are green.

Merged via the queue into microsoft:main with commit 1a2b4fb Sep 29, 2026
49 of 50 checks passed

This branch was successfully deployed

1 active deployment
github-app-auth — 625fbe86 Deployed Sep 29, 2026 by feiiiiii5 via team_check #5463
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.

.NET: Three README errors in the Python quickstart path

3 participants