Skip to content

Conversation

jamesbraza
Copy link
Collaborator

See PR title

@jamesbraza jamesbraza self-assigned this Jun 13, 2025
@Copilot Copilot AI review requested due to automatic review settings June 13, 2025 20:01
@jamesbraza jamesbraza added the enhancement New feature or request label Jun 13, 2025
@dosubot dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Jun 13, 2025
Copy link
Contributor

@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 introduces and utilizes a new Environment.get_id method to retrieve the configured question ID (or error if none). It also bumps the fhaviary dependency to enable this feature and updates tests accordingly.

  • Bump fhaviary[llm] to version ≥0.20 in pyproject.toml
  • Implement async def get_id in paperqa/agents/env.py with proper error handling
  • Convert test_env_from_name to async and add a ValueError test for missing ID

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
tests/test_agents.py Marked test async and added a pytest.raises block for get_id
pyproject.toml Updated fhaviary[llm] version to ≥0.20
paperqa/agents/env.py Added get_id method with error raising for unconfigured IDs
Comments suppressed due to low confidence (4)

paperqa/agents/env.py:342

  • [nitpick] Consider rephrasing the error message to be more descriptive, e.g. "No question ID configured; please provide an explicit question ID or object."
raise ValueError(f"No question ID was configured{details}.")

paperqa/agents/env.py:331

  • Add a docstring for get_id to explain its purpose, return value, and the conditions under which it raises a ValueError.
async def get_id(self) -> str:

tests/test_agents.py:1106

  • Add a test case to verify that env.get_id() returns the correct question ID when an environment is created with a non-default question object.
with subtests.test(msg="env-kwargs"):

paperqa/agents/env.py:334

  • Ensure that MultipleChoiceQuestion is imported in this module; otherwise, referencing it here will cause a NameError at runtime.
or self._query.question_id

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jun 13, 2025
@jamesbraza jamesbraza merged commit 462f775 into main Jun 13, 2025
5 checks passed
@jamesbraza jamesbraza deleted the env-id branch June 13, 2025 21:01
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:S This PR changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants