Skip to content
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

Only show the top contexts to the agent for the specific question #912

Merged
merged 3 commits into from
Mar 13, 2025

Conversation

mskarlin
Copy link
Collaborator

If multiple gather evidence iterations are performed in a question, the agent sees the top contexts for all submitted questions currently. We should only show the agent the top contexts for the particular question being asked of gather_evidence at that time. This PR fixes that and adds a test.

@mskarlin mskarlin requested review from jamesbraza, maykcaldas, a team and Copilot March 13, 2025 18:20
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. enhancement New feature or request labels Mar 13, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR ensures that only the top contexts for the currently asked question are shown to the agent when evidence is gathered. Key changes include:

  • Updating the test to use a new question for evidence gathering and verifying that only relevant contexts are returned.
  • Adding a question field to the Context model in paperqa/types.py.
  • Filtering contexts by question in the evidence gathering tool to return only the top relevant ones.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
tests/test_agents.py Updated the test to use a new question and verify evidence filtering
paperqa/types.py Added and propagated the “question” field for context differentiation
paperqa/agents/tools.py Updated filtering logic in gather_evidence to filter contexts by question
paperqa/core.py Passed the question value to the Context constructor

Copy link
Collaborator

@maykcaldas maykcaldas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's this test which uses Context and question is not set.

Specifically for the test, it doesn't matter. But do we want to add the question to the context just to be consistent?

Besides that, it lgtm!

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Mar 13, 2025
@mskarlin mskarlin enabled auto-merge (squash) March 13, 2025 19:31
@mskarlin mskarlin disabled auto-merge March 13, 2025 19:42
@mskarlin mskarlin merged commit d0b56a3 into main Mar 13, 2025
4 of 5 checks passed
@mskarlin mskarlin deleted the only-show-top-contexts-for-question branch March 13, 2025 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants