Skip to content

Python: Fix schema handling. Fix function result return for type list. #6370

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 23, 2024

Conversation

moonbox3
Copy link
Contributor

@moonbox3 moonbox3 commented May 22, 2024

Motivation and Context

Building the tools json payload from the kernel parameter metadata wasn't properly including an object of type array.

Description

Correctly include the object type array so that the tool call doesn't return a bad request. Add unit tests.

Contribution Checklist

@moonbox3 moonbox3 requested a review from a team as a code owner May 22, 2024 18:30
@markwallace-microsoft markwallace-microsoft added the python Pull requests for the Python Semantic Kernel label May 22, 2024
@markwallace-microsoft
Copy link
Member

markwallace-microsoft commented May 22, 2024

Py3.10 Test Coverage

Python 3.10 Test Coverage Report •
FileStmtsMissCoverMissing
semantic_kernel/connectors/ai/open_ai/services
   open_ai_chat_completion_base.py2199855%98, 118, 143–147, 171, 175, 191–196, 213–241, 244–255, 273–280, 291–299, 315–322, 343, 351, 357–363, 375–381, 412, 451, 453–454, 459–464, 468–475, 479–491, 513, 522–531
   utils.py311165%20–30, 41, 60
semantic_kernel/connectors/openapi_plugin
   openapi_manager.py582164%92–125
   openapi_parser.py84495%69, 92, 95, 126
   openapi_runner.py996435%53–55, 61–62, 68–82, 86–102, 105–107, 110–112, 115–118, 126–169
semantic_kernel/connectors/openapi_plugin/models
   rest_api_operation.py1245258%72–78, 81–98, 101–103, 106–119, 122–133, 136–148, 175, 191, 204, 238, 243, 247, 259, 275
   rest_api_operation_run_options.py6267%11–12
   rest_api_uri.py9367%14, 17–18
semantic_kernel/functions
   function_result.py30970%49, 51–52, 62–67
   kernel_function_from_method.py85693%129, 148–149, 155, 158–159
TOTAL607886986% 

Python 3.10 Unit Test Overview

Tests Skipped Failures Errors Time
1374 1 💤 0 ❌ 0 🔥 15.312s ⏱️

moonbox3 added 2 commits May 22, 2024 20:51
… plugins. Remove unused kernel json schema class.
…on result return for type string if not kernel content. Add retry logic on FC stepwise int test. Add unit test for function str return type.
@moonbox3 moonbox3 changed the title Python: include the correct schema for an array type. Add unit tests. Python: Fix schema handling. Fix function result return for type string. May 23, 2024
@moonbox3 moonbox3 changed the title Python: Fix schema handling. Fix function result return for type string. Python: Fix schema handling. Fix function result return for type list. May 23, 2024
@moonbox3 moonbox3 added this pull request to the merge queue May 23, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 23, 2024
@moonbox3 moonbox3 added this pull request to the merge queue May 23, 2024
Merged via the queue into microsoft:main with commit 0c95173 May 23, 2024
25 checks passed
@moonbox3 moonbox3 deleted the fix_list_schema branch May 23, 2024 18:10
LudoCorporateShark pushed a commit to LudoCorporateShark/semantic-kernel that referenced this pull request Aug 25, 2024
microsoft#6370)

### Motivation and Context

Building the tools json payload from the kernel parameter metadata
wasn't properly including an object of type `array`.

<!-- Thank you for your contribution to the semantic-kernel repo!
Please help reviewers and future users, providing the following
information:
  1. Why is this change required?
  2. What problem does it solve?
  3. What scenario does it contribute to?
  4. If it fixes an open issue, please link to the issue here.
-->

### Description

Correctly include the object type `array` so that the tool call doesn't
return a bad request. Add unit tests.
- Closes microsoft#6367 
- Closes microsoft#6360
- Fixes the FunctionResult return for a type string -- if the
FunctionResult is of type KernelContent then return the first element of
the list, otherwise return the complete list.
- Fix the kernel function from method to include the proper type_object
for the return parameter so that the schema can be created properly.
- Add retry logic for a sometimes flaky function calling stepwise
planner integration test.
- Add a check during function calling that makes sure the model is
returning the proper number of arguments based on how many function
arguments are required.

<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [X] The code builds clean without any errors or warnings
- [X] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [X] All unit tests pass, and I have added new tests where possible
- [X] I didn't break anyone 😄
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
4 participants