Skip to content

Commit 55ddd84

Browse files
eavanvalkenburgCopilot
eavanvalkenburg
and
Copilot
authored
Python: Fix Python pyright package scoping and typing remediation (#4426)
* Fix Python pyright package scoping and typing remediation Implements issue #4407 by removing the root pyright include, adding package-level pyright includes, and resolving pyright/mypy typing issues across Python packages. Also cleans unnecessary casts and applies line-level, rule-specific ignores where external libraries are too dynamic. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Reduce pyright cost in handoff cloning Simplify cloned_options construction in HandoffAgentExecutor to avoid expensive TypedDict narrowing/inference in _handoff.py, which was causing pyright to spend a long time in orchestrations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix types * Fix lint and type-check regressions Resolve current Python package check failures across lint, pyright, and mypy after recent code changes, including purview/declarative pyright issues and multiple ruff simplification findings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fixed hooks * Stabilize package tests and test tasks Resolve cross-package non-integration test failures, simplify streaming type flow, harden locale/culture handling, and standardize package test poe tasks to exclude integration tests where applicable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * lots of small fixes * Fix current Python test regressions Address current failing unit tests in azure-ai, bedrock, and azure-cosmos while keeping Bedrock parsing logic inline (no new static helper methods). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * small fixes * small fixes * removed pydantic from json * final updates * fix core * fix tests * fix obser --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 4a043c6 commit 55ddd84

122 files changed

Lines changed: 2329 additions & 2408 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎python/CODING_STANDARD.md‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,12 @@ Public modules must include a module-level docstring, including `__init__.py` fi
2727

2828
## Type Annotations
2929

30+
We use typing as a helper, it is not a goal in and of itself, so be pragmatic about where and when to strictly type, versus when to use a targetted cast or ignore.
31+
In general, the public interfaces of our classes, are important to get right, internally it is okay to have loosely typed code, as long as tests cover the code itself.
32+
This includes making a conscious choice when to program defensively, you can always do `getattr(item, 'attribute')` but that might end up causing you issues down the road
33+
because the type of `item` in this case, should have that attribute and if it doesn't it points to a larger issue, so if the type is expected to have that attribute, you should
34+
use `item.attribute` to ensure it fails at that point, rather then somewhere downstream where a value is expected but none was found.
35+
3036
### Future Annotations
3137

3238
> **Note:** This convention is being adopted. See [#3578](https://github.com/microsoft/agent-framework/issues/3578) for progress.
@@ -79,6 +85,21 @@ def process_config(config: MutableMapping[str, Any]) -> None:
7985
...
8086
```
8187

88+
### Typing Ignore and Cast Policy
89+
90+
Use typing as a helper first and suppressions as a last resort:
91+
92+
- **Prefer explicit typing before suppression**: Start with clearer type annotations, helper types, overloads,
93+
protocols, or refactoring dynamic code into typed helpers. Prioritize performance over completeness of typing, but make a good-faith effort to reduce uncertainty with typing before ignoring. Prefer to use a cast over a typeguard function since that does add overhead.
94+
- **Avoid redundant casts**: Do not add `cast(...)` if the type already matches; casts should be reserved for
95+
unavoidable narrowing where the runtime contract is known, we will use mypy's check on redundant casts to enforce this.
96+
- **Avoid multiple assignments**: Avoid assigning multiple variables just to get typing to pass, that has performance impact while typing should not have that.
97+
- **Line-level pyright ignores only**: If suppression is still required, use a line-level rule-specific ignore
98+
(`# pyright: ignore[reportGeneralTypeIssues]`), file-level is allowed if there is a compelling reason for it, that should be documented right beneath the ignore.
99+
Never change the global suppression flags for mypy and pyright unless the dev team okays it.
100+
- **Private usage boundary**: Accessing private members across `agent_framework*` packages can be acceptable for this
101+
codebase, but private member usage for non-Agent Framework dependencies should remain flagged.
102+
82103
## Function Parameter Guidelines
83104

84105
To make the code easier to use and maintain:

‎python/packages/a2a/agent_framework_a2a/_agent.py‎

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import re
88
import uuid
99
from collections.abc import AsyncIterable, Awaitable, Sequence
10-
from typing import Any, Final, Literal, overload
10+
from typing import Any, Final, Literal, TypeAlias, overload
1111

1212
import httpx
1313
from a2a.client import Client, ClientConfig, ClientFactory, minimal_agent_card
@@ -19,9 +19,11 @@
1919
FileWithBytes,
2020
FileWithUri,
2121
Task,
22+
TaskArtifactUpdateEvent,
2223
TaskIdParams,
2324
TaskQueryParams,
2425
TaskState,
26+
TaskStatusUpdateEvent,
2527
TextPart,
2628
TransportProtocol,
2729
)
@@ -70,6 +72,9 @@ class A2AContinuationToken(ContinuationToken):
7072
TaskState.auth_required,
7173
]
7274

75+
A2AClientEvent: TypeAlias = tuple[Task, TaskStatusUpdateEvent | TaskArtifactUpdateEvent | None]
76+
A2AStreamItem: TypeAlias = A2AMessage | A2AClientEvent
77+
7378

7479
def _get_uri_data(uri: str) -> str:
7580
match = URI_PATTERN.match(uri)
@@ -260,7 +265,9 @@ def run(
260265
When stream=True: A ResponseStream of AgentResponseUpdate items.
261266
"""
262267
if continuation_token is not None:
263-
a2a_stream: AsyncIterable[Any] = self.client.resubscribe(TaskIdParams(id=continuation_token["task_id"]))
268+
a2a_stream: AsyncIterable[A2AStreamItem] = self.client.resubscribe(
269+
TaskIdParams(id=continuation_token["task_id"])
270+
)
264271
else:
265272
normalized_messages = normalize_messages(messages)
266273
a2a_message = self._prepare_message_for_a2a(normalized_messages[-1])
@@ -276,7 +283,7 @@ def run(
276283

277284
async def _map_a2a_stream(
278285
self,
279-
a2a_stream: AsyncIterable[Any],
286+
a2a_stream: AsyncIterable[A2AStreamItem],
280287
*,
281288
background: bool = False,
282289
) -> AsyncIterable[AgentResponseUpdate]:
@@ -300,14 +307,12 @@ async def _map_a2a_stream(
300307
response_id=str(getattr(item, "message_id", uuid.uuid4())),
301308
raw_representation=item,
302309
)
303-
elif isinstance(item, tuple) and len(item) == 2: # ClientEvent = (Task, UpdateEvent)
310+
elif isinstance(item, tuple) and len(item) == 2 and isinstance(item[0], Task):
304311
task, _update_event = item
305-
if isinstance(task, Task):
306-
for update in self._updates_from_task(task, background=background):
307-
yield update
312+
for update in self._updates_from_task(task, background=background):
313+
yield update
308314
else:
309-
msg = f"Only Message and Task responses are supported from A2A agents. Received: {type(item)}"
310-
raise NotImplementedError(msg)
315+
raise NotImplementedError("Only Message and Task responses are supported")
311316

312317
# ------------------------------------------------------------------
313318
# Task helpers
@@ -396,6 +401,8 @@ def _prepare_message_for_a2a(self, message: Message) -> A2AMessage:
396401
for content in message.contents:
397402
match content.type:
398403
case "text":
404+
if content.text is None:
405+
raise ValueError("Text content requires a non-null text value")
399406
parts.append(
400407
A2APart(
401408
root=TextPart(
@@ -414,6 +421,8 @@ def _prepare_message_for_a2a(self, message: Message) -> A2AMessage:
414421
)
415422
)
416423
case "uri":
424+
if content.uri is None:
425+
raise ValueError("URI content requires a non-null uri value")
417426
parts.append(
418427
A2APart(
419428
root=FilePart(
@@ -426,18 +435,22 @@ def _prepare_message_for_a2a(self, message: Message) -> A2AMessage:
426435
)
427436
)
428437
case "data":
438+
if content.uri is None:
439+
raise ValueError("Data content requires a non-null uri value")
429440
parts.append(
430441
A2APart(
431442
root=FilePart(
432443
file=FileWithBytes(
433-
bytes=_get_uri_data(content.uri), # type: ignore[arg-type]
444+
bytes=_get_uri_data(content.uri),
434445
mime_type=content.media_type,
435446
),
436447
metadata=content.additional_properties,
437448
)
438449
)
439450
)
440451
case "hosted_file":
452+
if content.file_id is None:
453+
raise ValueError("Hosted file content requires a non-null file_id value")
441454
parts.append(
442455
A2APart(
443456
root=FilePart(

‎python/packages/a2a/pyproject.toml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ omit = [
6161

6262
[tool.pyright]
6363
extends = "../../pyproject.toml"
64+
include = ["agent_framework_a2a"]
6465

6566
[tool.mypy]
6667
plugins = ['pydantic.mypy']
@@ -86,7 +87,7 @@ include = "../../shared_tasks.toml"
8687

8788
[tool.poe.tasks]
8889
mypy = "mypy --config-file $POE_ROOT/pyproject.toml agent_framework_a2a"
89-
test = "pytest --cov=agent_framework_a2a --cov-report=term-missing:skip-covered tests"
90+
test = "pytest -m \"not integration\" --cov=agent_framework_a2a --cov-report=term-missing:skip-covered tests"
9091

9192
[build-system]
9293
requires = ["flit-core >= 3.11,<4.0"]

‎python/packages/ag-ui/pyproject.toml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ warn_unused_configs = true
6464
disallow_untyped_defs = false
6565

6666
[tool.pyright]
67+
include = ["agent_framework_ag_ui"]
6768
exclude = ["tests", "tests/ag_ui", "examples"]
6869
typeCheckingMode = "basic"
6970

@@ -73,4 +74,4 @@ include = "../../shared_tasks.toml"
7374

7475
[tool.poe.tasks]
7576
mypy = "mypy --config-file $POE_ROOT/pyproject.toml agent_framework_ag_ui"
76-
test = "pytest --cov=agent_framework_ag_ui --cov-report=term-missing:skip-covered -n auto --dist worksteal tests/ag_ui"
77+
test = "pytest -m \"not integration\" --cov=agent_framework_ag_ui --cov-report=term-missing:skip-covered -n auto --dist worksteal tests/ag_ui"

‎python/packages/anthropic/agent_framework_anthropic/_chat_client.py‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
import logging
66
import sys
7-
from collections.abc import AsyncIterable, Awaitable, Callable, Mapping, MutableMapping, Sequence
7+
from collections.abc import AsyncIterable, Awaitable, Callable, Mapping, Sequence
88
from typing import Any, ClassVar, Final, Generic, Literal, TypedDict
99

1010
from agent_framework import (
@@ -302,15 +302,18 @@ class MyOptions(AnthropicChatOptions, total=False):
302302
env_file_encoding=env_file_encoding,
303303
)
304304

305+
api_key_secret = anthropic_settings.get("api_key")
306+
model_id_setting = anthropic_settings.get("chat_model_id")
307+
305308
if anthropic_client is None:
306-
if not anthropic_settings["api_key"]:
309+
if api_key_secret is None:
307310
raise ValueError(
308311
"Anthropic API key is required. Set via 'api_key' parameter "
309312
"or 'ANTHROPIC_API_KEY' environment variable."
310313
)
311314

312315
anthropic_client = AsyncAnthropic(
313-
api_key=anthropic_settings["api_key"].get_secret_value(),
316+
api_key=api_key_secret.get_secret_value(),
314317
default_headers={"User-Agent": AGENT_FRAMEWORK_USER_AGENT},
315318
)
316319

@@ -324,7 +327,7 @@ class MyOptions(AnthropicChatOptions, total=False):
324327
# Initialize instance variables
325328
self.anthropic_client = anthropic_client
326329
self.additional_beta_flags = additional_beta_flags or []
327-
self.model_id = anthropic_settings["chat_model_id"]
330+
self.model_id = model_id_setting
328331
# streaming requires tracking the last function call ID, name, and content type
329332
self._last_call_id_name: tuple[str, str] | None = None
330333
self._last_call_content_type: str | None = None
@@ -785,18 +788,22 @@ def _prepare_tools_for_anthropic(self, options: Mapping[str, Any]) -> dict[str,
785788
"description": tool.description,
786789
"input_schema": tool.parameters(),
787790
})
788-
elif isinstance(tool, MutableMapping) and tool.get("type") == "mcp":
791+
elif isinstance(tool, Mapping) and tool.get("type") == "mcp": # type: ignore[reportUnknownMemberType]
789792
# MCP servers must be routed to separate mcp_servers parameter
790793
server_def: dict[str, Any] = {
791794
"type": "url",
792-
"name": tool.get("server_label", ""),
793-
"url": tool.get("server_url", ""),
795+
"name": tool.get("server_label", ""), # type: ignore[reportUnknownMemberType]
796+
"url": tool.get("server_url", ""), # type: ignore[reportUnknownMemberType]
794797
}
795-
if allowed_tools := tool.get("allowed_tools"):
796-
server_def["tool_configuration"] = {"allowed_tools": list(allowed_tools)}
797-
headers = tool.get("headers")
798-
if isinstance(headers, dict) and (auth := headers.get("authorization")):
799-
server_def["authorization_token"] = auth
798+
allowed_tools = tool.get("allowed_tools") # type: ignore[reportUnknownMemberType]
799+
if isinstance(allowed_tools, Sequence) and not isinstance(allowed_tools, str):
800+
server_def["tool_configuration"] = {
801+
"allowed_tools": [str(item) for item in allowed_tools] # pyright: ignore[reportUnknownArgumentType,reportUnknownVariableType]
802+
}
803+
headers = tool.get("headers") # type: ignore[reportUnknownMemberType]
804+
authorization = headers.get("authorization") if isinstance(headers, Mapping) else None # pyright: ignore[reportUnknownMemberType,reportUnknownVariableType]
805+
if isinstance(authorization, str):
806+
server_def["authorization_token"] = authorization
800807
mcp_server_list.append(server_def)
801808
else:
802809
# Pass through all other tools (dicts, SDK types) unchanged

‎python/packages/anthropic/pyproject.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ include = "../../shared_tasks.toml"
8787

8888
[tool.poe.tasks]
8989
mypy = "mypy --config-file $POE_ROOT/pyproject.toml agent_framework_anthropic"
90-
test = "pytest --cov=agent_framework_anthropic --cov-report=term-missing:skip-covered -n auto --dist worksteal tests"
90+
test = "pytest -m \"not integration\" --cov=agent_framework_anthropic --cov-report=term-missing:skip-covered -n auto --dist worksteal tests"
9191

9292
[build-system]
9393
requires = ["flit-core >= 3.11,<4.0"]

‎python/packages/azure-ai-search/agent_framework_azure_ai_search/_context_provider.py‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -456,10 +456,10 @@ async def _semantic_search(self, query: str) -> list[Message]:
456456
elif self.embedding_function:
457457
if isinstance(self.embedding_function, SupportsGetEmbeddings):
458458
embeddings = await self.embedding_function.get_embeddings([query]) # type: ignore[reportUnknownVariableType]
459-
query_vector: list[float] = embeddings[0].vector # type: ignore[reportUnknownVariableType]
459+
query_vector = embeddings[0].vector # type: ignore[reportUnknownVariableType]
460460
else:
461-
query_vector = await self.embedding_function(query)
462-
vector_queries = [VectorizedQuery(vector=query_vector, k=vector_k, fields=self.vector_field_name)]
461+
query_vector = await self.embedding_function(query) # type: ignore[reportUnknownVariableType]
462+
vector_queries = [VectorizedQuery(vector=query_vector, k=vector_k, fields=self.vector_field_name)] # type: ignore[reportUnknownArgumentType]
463463

464464
search_params: dict[str, Any] = {"search_text": query, "top": self.top_k}
465465
if vector_queries:
@@ -632,6 +632,8 @@ def _prepare_messages_for_kb_search(messages: list[Message]) -> list[KnowledgeBa
632632
image=KnowledgeBaseMessageImageContentImage(url=content.uri),
633633
)
634634
)
635+
case _:
636+
pass
635637
elif msg.text:
636638
kb_content.append(KnowledgeBaseMessageTextContent(text=msg.text))
637639
if kb_content:

‎python/packages/azure-ai-search/pyproject.toml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ omit = [
6262

6363
[tool.pyright]
6464
extends = "../../pyproject.toml"
65+
include = ["agent_framework_azure_ai_search"]
6566
exclude = ['tests']
6667

6768
[tool.mypy]
@@ -88,7 +89,7 @@ include = "../../shared_tasks.toml"
8889

8990
[tool.poe.tasks]
9091
mypy = "mypy --config-file $POE_ROOT/pyproject.toml agent_framework_azure_ai_search"
91-
test = "pytest --cov=agent_framework_azure_ai_search --cov-report=term-missing:skip-covered tests"
92+
test = "pytest -m \"not integration\" --cov=agent_framework_azure_ai_search --cov-report=term-missing:skip-covered tests"
9293

9394
[build-system]
9495
requires = ["flit-core >= 3.11,<4.0"]

0 commit comments

Comments
 (0)