-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Set completion support to false by default #441
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
Set completion support to false by default #441
Conversation
|
Thanks for this fix. If I'm understanding the typescript SDK issue correctly, it seems like typescript servers are most likely not reporting completion correctly in their capabilities? Would it make sense to wait to merge this once that fix is out so we can test with the expected state using latest sdk versus a potentially buggy one? |
|
Yep, agreed, let's wait for a new TS SDK version to be published first with this fix. I'll have a quick look on other language SDK as well in the meantime. |
|
@olaservo both TS and Python fixes have been merged. Should we move forward with this PR? |
|
Thanks @fredericbarthelet ! I was about to merge this, but I'm thinking we might wait until the Python SDK update is actually released, so that we can point people to update their server from the latest SDK version. We should be good on TypeScript since that was added a while back. What do you think? |
|
You're right @olaservo - better to wait for the next Python SDK release :) |
|
Should be good to go now :) OK @olaservo ? |
Change completion support default to false and use server capabilities value to assert support for completion.
Fixes #436
Motivation and Context
Current implementation always tries a first completion request on resource template or prompt usage, even if the server does not advertise support for completions. The silent error fallback currently implemented then switch back the support to false for further calls.
How Has This Been Tested?
Yes, with a server with completion, and with one without
Breaking Changes
None
Types of changes
Checklist
Additional context
I noticed server built using the Typescript SDK were not advertising completions support - even when they do support it - unless manually specified in the server construct arguments. I opened a PR to fix that as well to ensure completions capabilities advertising and use is more coherent. Same thing for Python SDK.