Skip to content

Python: Fix Agent Framework tool parameter defaults - #14346

Open
ump45nose wants to merge 2 commits into
microsoft:mainfrom
ump45nose:fix/agent-framework-tool-default
Open

ump45nose wants to merge 2 commits into
microsoft:mainfrom
ump45nose:fix/agent-framework-tool-default

Conversation

@ump45nose

@ump45nose ump45nose commented Aug 27, 2026 •

Copy link
Copy Markdown

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 😄

@ump45nose
ump45nose requested a review from a team as a code owner August 27, 2026 16:39
Copilot AI lite review requested due to automatic review settings August 27, 2026 16:39

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.

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.

Comment thread python/semantic_kernel/functions/kernel_function.py
Comment thread python/tests/unit/functions/test_kernel_function_from_method.py

@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)): 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
ump45nose force-pushed the fix/agent-framework-tool-default branch from 424431c to f58bb39 Compare September 4, 2026 07:13
@ump45nose

Copy link
Copy Markdown
Author

已将分支变基到当前 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 辅助完成。

@ump45nose
ump45nose force-pushed the fix/agent-framework-tool-default branch 2 times, most recently from 5cb90de to c7e75d3 Compare September 7, 2026 06:24
@ump45nose
ump45nose force-pushed the fix/agent-framework-tool-default branch from b91001f to ff7fbf2 Compare October 4, 2026 16:37
@ump45nose
ump45nose deployed to github-app-auth October 4, 2026 16:37 — with GitHub Actions Active
@semantic-kernel-automation semantic-kernel-automation Bot added the python Pull requests for the Python Semantic Kernel label Oct 4, 2026
@ump45nose
ump45nose deployed to github-app-auth October 11, 2026 16:00 — with GitHub Actions Active

This branch was successfully deployed

1 active deployment
github-app-auth — 54033487 Deployed Oct 11, 2026 by ump45nose via add_label #29316
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Pull requests for the Python Semantic Kernel

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants