Skip to content

bugfix: Workaround for pydantic/#7713#5893

Merged
ekzhu merged 11 commits intomicrosoft:mainfrom
nissa-seru:patch-1
Mar 13, 2025
Merged

bugfix: Workaround for pydantic/#7713#5893
ekzhu merged 11 commits intomicrosoft:mainfrom
nissa-seru:patch-1

Conversation

@nissa-seru
Copy link
Copy Markdown
Contributor

Use of SKChatCompletionAdapter reliably fails with "'MockValSer' object cannot be converted to 'SchemaSerializer'"; can repro with this example: https://microsoft.github.io/autogen/stable/user-guide/core-user-guide/components/model-clients.html#semantic-kernel-adapter

This appears to be related to pydantic/pydantic#7713 - commit uses workaround from pydantic/pydantic#7713 (comment)

Why are these changes needed?

This unblocks use of the Semantic Kernel integration by addressing the above-referenced error, enabling the integration to perform as expected.

Related issue number

N/A, see pydantic/pydantic#7713 for context, though.

Checks

  • I've included any doc changes needed for https://microsoft.github.io/autogen/. See https://github.com/microsoft/autogen/blob/main/CONTRIBUTING.md to build and test documentation locally.
  • None needed, internal only change.
  • I've added tests (if relevant) corresponding to the changes introduced in this PR.
  • None added; this works on my machine, but I'm not clear on the root cause of the issue and have no strong opinion on whether this is the ideal way to fix it long term - simply leaning towards PR`ing a tenative fix instead of raising an issue.
  • I've made sure all auto checks have passed.
  • I am not familiar with these, but assume they will be run during CI.

Use of `SKChatCompletionAdapter` reliably fails with "'MockValSer' object cannot be converted to 'SchemaSerializer'"; can repro with this example: https://microsoft.github.io/autogen/stable/user-guide/core-user-guide/components/model-clients.html#semantic-kernel-adapter

This appears to be related to pydantic/pydantic#7713 - commit uses workaround from pydantic/pydantic#7713 (comment)
@nissa-seru
Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree

@nissa-seru nissa-seru changed the title bugfix: Workaround for pydantic/#7710 bugfix: Workaround for pydantic/#7713 Mar 11, 2025
@ekzhu ekzhu requested a review from lspinheiro March 11, 2025 05:08
@lspinheiro
Copy link
Copy Markdown
Collaborator

@nissa-seru , can you also post an error trace demonstrating the issue? If possible, can you share some minimal code to reproduce? I left one comment on the actual change as well

@nissa-seru
Copy link
Copy Markdown
Contributor Author

Refer to the initial PR for a link to the example - this causes the issue for me when run in a Jupyter notebook running Python 3.12.8. Apologies for failing to capture the error trace originally - I'm a bit busy with other work right now though to roll back the fix locally and recapture it.

@lspinheiro
Copy link
Copy Markdown
Collaborator

@nissa-seru , I was initially concerned with potential loss of data but had a better look and since this is only for logging I don't think there are major concerns. I suggested a fix on the new function. Also make sure to run poe fmt to see if it fixes the CI errors

lspinheiro and others added 2 commits March 12, 2025 10:48
Co-authored-by: Leonardo Pinheiro <leosantospinheiro@gmail.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 12, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.56%. Comparing base (cf13657) to head (08c6c7d).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5893      +/-   ##
==========================================
+ Coverage   70.15%   75.56%   +5.41%     
==========================================
  Files         263      191      -72     
  Lines       15122    13093    -2029     
  Branches      256        0     -256     
==========================================
- Hits        10609     9894     -715     
+ Misses       4313     3199    -1114     
+ Partials      200        0     -200     
Flag Coverage Δ
unittests 75.56% <100.00%> (+5.41%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ekzhu ekzhu enabled auto-merge (squash) March 13, 2025 18:18
@ekzhu ekzhu merged commit 6ae098f into microsoft:main Mar 13, 2025
@nissa-seru nissa-seru deleted the patch-1 branch March 13, 2025 18:53
ekzhu added a commit that referenced this pull request Mar 14, 2025
Use of `SKChatCompletionAdapter` reliably fails with "'MockValSer'
object cannot be converted to 'SchemaSerializer'"; can repro with this
example:
https://microsoft.github.io/autogen/stable/user-guide/core-user-guide/components/model-clients.html#semantic-kernel-adapter

This appears to be related to
pydantic/pydantic#7713 - commit uses
workaround from
pydantic/pydantic#7713 (comment)

## Why are these changes needed?

This unblocks use of the Semantic Kernel integration by addressing the
above-referenced error, enabling the integration to perform as expected.

## Related issue number

N/A, see pydantic/pydantic#7713 for context,
though.

## Checks

- [X] I've included any doc changes needed for
<https://microsoft.github.io/autogen/>. See
<https://github.com/microsoft/autogen/blob/main/CONTRIBUTING.md> to
build and test documentation locally.
 - None needed, internal only change.
- [ ] I've added tests (if relevant) corresponding to the changes
introduced in this PR.
- None added; this works on my machine, but I'm not clear on the root
cause of the issue and have no strong opinion on whether this is the
ideal way to fix it long term - simply leaning towards PR`ing a tenative
fix instead of raising an issue.
- [ ] I've made sure all auto checks have passed.
 - I am not familiar with these, but assume they will be run during CI.

---------

Co-authored-by: Leonardo Pinheiro <leosantospinheiro@gmail.com>
Co-authored-by: Eric Zhu <ekzhu@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants