[ump45nose]

Motivation and Context

KernelFunction.as_agent_framework_tool() currently builds a field with a parameter default and then immediately overwrites it with a required field. As a result, optional Semantic Kernel parameters become required in the Agent Framework tool schema.

Description

• preserve every optional field by using is_required as the source of truth, including explicit None defaults
• add a focused regression test that verifies both default=5 and default=None remain optional

Validation:

• uv run pytest tests/unit/functions/test_kernel_function_from_method.py -q — 37 passed
• uv run ruff check semantic_kernel/functions/kernel_function.py tests/unit/functions/test_kernel_function_from_method.py — passed
• uv run ruff format --check semantic_kernel/functions/kernel_function.py tests/unit/functions/test_kernel_function_from_method.py — passed
• git diff --check — passed

Contribution Checklist

• I added a focused regression test for the corrected behavior
• I reviewed the change against the SK Contribution Guidelines
• I didn't break anyone 😄

[Copilot]

Pull request overview

Fixes Python KernelFunction.as_agent_framework_tool() so Agent Framework tool input models preserve parameter defaults (instead of incorrectly making optional parameters required), and adds a regression test to validate the behavior.

Changes:

• Prevent overwriting Pydantic fields that already include a non-None default when building the Agent Framework tool input model.
• Add a unit test asserting a parameter with default=5 remains optional in the generated tool schema.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
python/semantic_kernel/functions/kernel_function.py	Adjusts field-building logic in as_agent_framework_tool() to avoid overwriting defaulted fields.
python/tests/unit/functions/test_kernel_function_from_method.py	Adds a regression test verifying optionality is preserved when a default value is present.

---

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

[github-actions]

MAF Automated Review — Iteration 1

Result: No findings
Scope: full PR (1 commit(s)): a60b81246da8
Model: claude-opus-4.8

Overview

The PR converts an unconditional field-overwrite into an else branch in
KernelFunction.as_agent_framework_tool(), so a Semantic Kernel parameter carrying a
non-None default is preserved as an optional field instead of being clobbered into a
required one. The change is minimal, API-compatible, and touches no shared state,
serialization, or trust boundary. It is backed by a focused regression test that fails
against the pre-fix code and passes after, and the full test_kernel_function_from_method
suite stays green. No Critical/High/Medium defect introduced by the PR was found.

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.

[ump45nose]

已将分支变基到当前 upstream/main 并安全推送（f58bb39a3）。

本地验证：

• uv run pytest tests/unit/functions/test_kernel_function_from_method.py：37 passed（1 个既有 Pydantic 弃用警告）
• git diff --check upstream/main...HEAD：通过

新提交触发的 Python、.NET、拼写与合并门禁工作流目前均为 action_required，看起来需要维护者批准 fork 的工作流后才能执行。烦请批准运行并复审。

该分支的变基、测试与说明由 AI 辅助完成。

[Eduard van Valkenburg (eavanvalkenburg)]

param.is_required does not distinguish x: str | None from x: str | None = None: the decorator marks both optional. With a nullable annotation but no Python default, this branch now creates Field(default=None), and tool.input_model() accepts the missing required argument. I reproduced this with a decorated def f(x: str | None). Preserve the missing-default distinction from the callable signature (or metadata) and add a regression for nullable-but-required parameters.

[ump45nose]

Fixed in 87a81fd: as_agent_framework_tool now builds Fields from the underlying callable's inspect.signature when available. A parameter annotated str | None with no default stays required (no Field default), while str | None = None remains optional with a real None default. Added a regression that expects ValidationError when the nullable-without-default field is omitted.

(AI-assisted reply.)

[Eduard van Valkenburg (eavanvalkenburg)]

Preserving the None default also needs a nullable field annotation. For the new test function, metadata stores label as type_="str"; the generated model therefore advertises a string-only field with default null, and tool.input_model(label=None) raises ValidationError. The current test checks the default object but never validates an explicit None. Please retain the nullable annotation and cover both omitted and explicitly-null values.

[ump45nose]

Addressed in the same 87a81fd change: the Field annotation now comes from the callable signature (unwrapping Annotated when needed), so a nullable parameter keeps a nullable type in the generated input model instead of collapsing to a string-only field with a null default.

(AI-assisted reply.)