chore: add dereference_json_schema#1192
Conversation
907fa6d to
672ba7b
Compare
|
This would also solve the same problem with Claude Code, which I asked about on #1159. This seems like a pragmatic solution. @vl-kp Thank you for this! I'm wondering why leave Although, it seems unfortunate to modify core to accommodate offending clients and lead to lot of configuration options. So I wondered if it was possible using the new middleware feature. It's not at present, because initialization requests are not exposed to middleware. Also, client identification occurs only in the |
There are some corner cases i am not sure it will happen or not:
|
d995567 to
d9c31a8
Compare
Ok, if I understand, this is when
I see, thanks! I guess I would add $def only if max_depth is exceeded. I forgot to ask about |
|
Hi @vl-kp, I have some hesitation about introducing compromises because of client failures; obviously that's not a sustainable way for us to build great features, and folks opening subsequent issues will only deepen the rabbit hole. I do recognize the problem however and I'm intrigued by @richardkmichael's suggestion that MCP Middleware is an excellent way to solve this. I think it is a fantastic motivation for the work in #1215. Then instead of exposing this as a global configuration it could be bundled as e.g. |
I agree, i updated to only add $def when corner case happens
i did a perf test under my Apple M4 Pro(24GB), looks like it doesn't impact too much performance, so i updated to depth to 50, it should meet most of the use cases |
bf98669 to
906c7b6
Compare
sure, we could wait for middleware feature implemented |
|
When I ran into this with Gemini I implemented https://pypi.org/project/jsonref/ with some luck Might be worth considering vs hand rolling the dereferencing |
|
@strawgate Thank you! (FastMCP Agents looks quite handy, glad you mentioned it.) jsonref has a few perhaps notable issues, although none are my use-case. However, their existence speaks to occasional difficulties with specific schema, which I think makes more of a case for externalizing this from FastMCP. As opposed to an internal implementation, jsonref or hand-rolled, needing to be bug fixed in FastMCP. For example, I'm unsure if a wrapped function used by FastMCP could generate a circular schema.
@vl-kp Thanks for the benchmark. They are mostly < 1 ms, so it's not a concern for me. Note also, jsonref has no max-depth. |
|
I'm going to close this PR for now since I don't think this is strategically an approach we want to take, but can open a new PR for an alternative like middleware that would more user control |
|
follow up PR #1641 |
In order to resolve issue #1193