fix: allow for self-referencing pydantic schema
Describe your changes
This PR introduces a new field related_products in the Products fixture which replicates the failure case mentioned in #155 and makes existing tests fail with a RecursionError as expected. Then we add a new seen set to the function resolve_schema_references which resolves the error and makes the tests pass again.
Issue ticket number and link (if applicable)
Fixes https://github.com/tadata-org/fastapi_mcp/issues/155
Checklist before requesting a review
- [x] Added relevant tests
- [x] Run ruff & mypy
- [x] All tests pass
validated locally on my machine. This solves the recursion issue. Please review and merge this fix in! Important
I have been using it also. It seems to work.
Hello - another passive observer here who would love to see this change included in a future release 🤗
First time using this app, encountered this problem, request merge this PR, please...
@shahar4499 request merge
As a temporary workaround you can copy this function in your code to patch the current implementation:
fastapi_mcp.openapi.utils.resolve_schema_references = resolve_schema_references
Edit: I rewrote my comment because I made the original in a hurry in the end of my day and realized it was basically unreadable.
Hi, thank you very much for your solution. I used it and came across a small mistake, or at least unexpected behavior, it causes for one of my models.
This happens in cases where you have a model that has two or more properties of the same type, like Game has here:
class Player(BaseModel):
name: str
number_of_games: int
teacher: Optional[Player] = None # self-referencing
class Game(BaseModel):
playerA: Player
playerB: Player
I would expect playerA and platerB to be represented by the same structure and only occurrences of Player in deeper nested positions to be replaced by some pointer, stopping the endless chain of recursion.
But since there is one global "seen" set passed around here everything after the first encounter is replaced by a pointer (PlayerB in this case), not only those in nested positions.
This change, fixes the problem for me:
https://github.com/jonvet/fastapi_mcp/pull/1
What is holding this change back from being merged now?
I need this change, and I don't want to keep this weird patch in my code.
What is stopping this fix from being merged?
@jonvet Somehow I got the notification only today. I’m not part of this project, so even if I review your changes I can’t do anything to get this PR merged.