-
Notifications
You must be signed in to change notification settings - Fork 795
Add response_schema parameter to Docs's aquery response generation
#1202
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
base: main
Are you sure you want to change the base?
Conversation
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.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
Bug: Synchronous method lacks schema feature parity.
The synchronous query method lacks the new response_schema parameter that was added to aquery, breaking the wrapper pattern where both methods should accept identical parameters. This prevents users of the synchronous method from using structured response schemas and causes a parameter mismatch between the wrapper and the underlying async implementation.
src/paperqa/docs.py#L744-L769
Lines 744 to 769 in de6751f
| def query( | |
| self, | |
| query: PQASession | str, | |
| settings: MaybeSettings = None, | |
| callbacks: Sequence[Callable] | None = None, | |
| llm_model: LLMModel | None = None, | |
| summary_llm_model: LLMModel | None = None, | |
| embedding_model: EmbeddingModel | None = None, | |
| partitioning_fn: Callable[[Embeddable], int] | None = None, | |
| ) -> PQASession: | |
| warnings.warn( | |
| "The synchronous `query` method is being deprecated in favor of the" | |
| " asynchronous `aquery` method, this deprecation will conclude in" | |
| " version 6.", | |
| category=DeprecationWarning, | |
| stacklevel=2, | |
| ) | |
| return get_loop().run_until_complete( | |
| self.aquery( | |
| query, | |
| settings=settings, | |
| callbacks=callbacks, | |
| llm_model=llm_model, | |
| summary_llm_model=summary_llm_model, | |
| embedding_model=embedding_model, |
Comment @cursor review or bugbot run to trigger another review on this PR
| summary_llm_model: LLMModel | None = None, | ||
| embedding_model: EmbeddingModel | None = None, | ||
| partitioning_fn: Callable[[Embeddable], int] | None = None, | ||
| response_schema: BaseModel | None = None, |
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.
I think this should be output_type: type[BaseModel] | TypeAdapter | JSONSchema | None = None, are you sure it works?
Can you add or expand a unit test in tests/test_paperqa.py confirming this?
Corresponds to #700, #446
Note
Add optional
response_schematoDocs.aqueryand pass it tollm_model.call_singleasresponse_formatfor answer generation.Written by Cursor Bugbot for commit de6751f. Configure here.