-
Notifications
You must be signed in to change notification settings - Fork 14
fix: JsonSchema required #288
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
Conversation
Hi @lvluoyue, thanks for taking care of that! How did you run into it? I was wondering before if that's needed or only with stricr mode |
Parameters should be optional when specifying a default value public function searchSong(
string $keyword,
#[With(minimum: 1, maximum: 60)]
int $limit = 10,
#[With(minimum: 1)]
int $page = 1
): string {
return R::mcp($this->tencentService->searchSong($keyword, $page, $limit));
} |
Sounds good, yes! Do you mind sharing what's that |
The full name of R is response, and it is used to encapsulate various response formats into a unified format |
you can run the failing tests locally with:
|
Maybe I get it wrong but in your example the parameter is required but with a default value, so null is not supported here, or am I missing sth? |
I was negligent, sorry. I've modified to the correct code and modified the unit tests |
Are you using OpenAI? |
Thanks, I'm using OpenAI, and I'm writing an mcp server with |
if you revert commit b6007f4 the pipeline should be fine - i personally don't see real value in those |
174af0b
to
b8a7687
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @lvluoyue! Looks good to me now 👍
No description provided.