Skip to content

Python: Allow custom argument parsing for skill scripts#6817

Merged
giles17 merged 5 commits into
microsoft:mainfrom
giles17:skill-arg-marshaler
Jul 1, 2026
Merged

Python: Allow custom argument parsing for skill scripts#6817
giles17 merged 5 commits into
microsoft:mainfrom
giles17:skill-arg-marshaler

Conversation

@giles17

@giles17 giles17 commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Motivation & Context

vLLM and some other OpenAI-compatible backends send tool-call arguments in a non-conforming shape (for example, as a JSON string "{\"query\":\"hello\"}" instead of a JSON object). Inline skill scripts bind arguments by keyword name and expect a JSON object (a dict), so arguments arriving in another shape cannot be handled and the call fails.

This mirrors the scenario fixed on the .NET side, letting callers plug in their own argument conversion logic so skill scripts work against these backends.

Description & Review Guide

  • What are the major changes?

    • Adds a public SkillScriptArgumentParser type alias: an optional callable (raw args: dict | list[str] | str | None) -> dict | None that converts the raw args value into named keyword arguments before an inline script runs.
    • The parser can be supplied at three levels, mirroring the .NET PR:
      • InlineSkillScript constructor — per script.
      • InlineSkill constructor — default for scripts added via @skill.script.
      • ClassSkill constructor — default for scripts discovered via @ClassSkill.script.
    • In InlineSkillScript.run(), the parser is applied first, before the existing inline list-args guard. A raw str reaching an inline script with no parser configured raises a clear TypeError.
    • Exports the alias from agent_framework and documents it in the core package AGENTS.md.
  • What is the impact of these changes?

    • Opt-in and fully backward compatible: every new parameter defaults to None, leaving existing behavior unchanged (arguments are used as-is, expecting a dict or None).
    • The parser output type is constrained to dict | None, so the type system enforces the inline-script contract (inline scripts bind by keyword name).
    • File-based scripts are unaffected — their SkillScriptRunner already owns argument handling, matching the .NET scoping to inline scripts.
  • What do you want reviewers to focus on?

    • Whether the three-level placement of the hook matches the .NET design, and the chosen parser input/output typing.

Related Issue

Closes #6543

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.

Add an optional argument_marshaler hook so callers can plug in their own argument conversion logic for inline skill scripts. Supplied at the InlineSkillScript, InlineSkill, and ClassSkill levels; when omitted, behavior is unchanged. This supports backends (e.g. vLLM) that send tool-call arguments in a non-conforming shape such as a JSON string.

Port of .NET PR microsoft#6498. Closes microsoft#6543.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 30, 2026 05:03
@moonbox3 moonbox3 added documentation Usage: [Issues, PRs], Target: documentation in the code base and learn docs python Usage: [Issues, PRs], Target: Python labels Jun 30, 2026
@github-actions

github-actions Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Python Test Coverage

Python Test Coverage Report •
FileStmtsMissCoverMissing
packages/core/agent_framework
   _skills.py10413896%313, 586, 1106, 1121, 1123–1124, 1491–1492, 1736, 1765, 2319, 2782–2783, 2880, 2888, 2893, 2896, 2901, 2921, 2933, 2938, 3034, 3042, 3047, 3050, 3055, 3075, 3084, 3089, 3350–3351, 3700, 3927–3928, 3955–3956, 3963–3964
TOTAL43141514488% 

Python Unit Test Overview

Tests Skipped Failures Errors Time
8518 37 💤 0 ❌ 0 🔥 2m 13s ⏱️

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

This PR adds an opt-in customization hook in the Python skills runtime to convert (“marshal”) raw tool-call args into the shape expected by inline skill scripts, enabling compatibility with OpenAI-compatible backends that send arguments in non-object forms (e.g., JSON strings).

Changes:

  • Introduces SkillScriptArgumentMarshaler and wires it through InlineSkillScript, InlineSkill (decorator-added scripts), and ClassSkill (discovered scripts).
  • Applies the marshaler in InlineSkillScript.run() prior to the existing list-args guard.
  • Exports the new alias publicly, adds documentation, and adds unit tests covering the propagation + end-to-end provider flow.

Reviewed changes

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

File Description
python/packages/core/agent_framework/_skills.py Adds the marshaler alias and plumbs it through inline/class skill script creation and execution.
python/packages/core/agent_framework/init.py Exports SkillScriptArgumentMarshaler as part of the public package API.
python/packages/core/AGENTS.md Documents the new marshaling hook and where it can be configured.
python/packages/core/tests/core/test_skills.py Adds tests for marshaling behavior and propagation at script/skill/provider levels.

Comment thread python/packages/core/agent_framework/_skills.py Outdated
Comment thread python/packages/core/agent_framework/_skills.py Outdated
Comment thread python/packages/core/AGENTS.md Outdated

@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.

Automated Code Review

Reviewers: 4 | Confidence: 91% | Result: All clear

Reviewed: Correctness, Security Reliability, Test Coverage, Failure Modes


Automated review by giles17's agents

- Widen InlineSkillScript.run args to accept a raw str (the one place a marshaler-converted value is valid), and drop the now-unneeded type: ignore markers in tests.

- Constrain the SkillScriptArgumentMarshaler output type to dict | None so the type enforces the inline-script contract instead of a docstring note.

- Add a clear TypeError when a str reaches an inline script with no marshaler configured.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@giles17 giles17 marked this pull request as ready for review June 30, 2026 05:35

@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.

Automated Code Review

Reviewers: 5 | Confidence: 87% | Result: All clear

Reviewed: Correctness, Security Reliability, Test Coverage, Failure Modes, Design Approach


Automated review by giles17's agents

Comment thread python/packages/core/agent_framework/_skills.py Outdated
giles17 and others added 2 commits June 30, 2026 08:27
In Python 'marshalling' specifically connotes the stdlib marshal module, so the term is misleading here. Rename the type alias, the argument_parser parameter/attribute, docstrings, exports, and tests accordingly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@giles17 giles17 changed the title Python: Allow custom argument marshaling for skill scripts Python: Allow custom argument parsing for skill scripts Jun 30, 2026
Comment thread python/packages/core/agent_framework/_skills.py Outdated
The skill constructors are fully keyword-only, so name/description/function are already documented under Args. Singling out argument_parser into its own Keyword Args section was inconsistent; merge it into Args for InlineSkillScript, InlineSkill, and ClassSkill.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@giles17 giles17 enabled auto-merge June 30, 2026 22:51
@giles17 giles17 added this pull request to the merge queue Jun 30, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jun 30, 2026
@giles17 giles17 added this pull request to the merge queue Jul 1, 2026
Merged via the queue into microsoft:main with commit d50698b Jul 1, 2026
38 checks passed
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.

Python: Allow custom argument marshaling for skill scripts

6 participants