Skip to content

Retrying Context creation once#1083

Merged
jamesbraza merged 8 commits intomainfrom
retrying-parse
Sep 11, 2025
Merged

Retrying Context creation once#1083
jamesbraza merged 8 commits intomainfrom
retrying-parse

Conversation

@jamesbraza
Copy link
Copy Markdown
Collaborator

This PR fixes an issue where, if the LLM poses bad JSON, it would crash gather_evidence. Now:

  1. First the Context creation is retried, with details on the prior failure
  2. Upon second failure, the Context is ditched, with a helpful logger.exception

It also takes care to index all LLM costs across retries.

@jamesbraza jamesbraza self-assigned this Sep 10, 2025
Copilot AI review requested due to automatic review settings September 10, 2025 22:12
@jamesbraza jamesbraza added the bug Something isn't working label Sep 10, 2025
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Sep 10, 2025
@dosubot
Copy link
Copy Markdown

dosubot bot commented Sep 10, 2025

Related Documentation

Checked 1 published document(s). No updates required.

How did I do? Any feedback?  Join Discord

Copy link
Copy Markdown
Contributor

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 implements retry logic for Context creation when LLMs produce malformed JSON, preventing crashes in the gather_evidence function.

  • Adds retry mechanism for Context creation with failure details passed to second attempt
  • Creates proper error handling that abandons Context creation after two failures with helpful logging
  • Ensures all LLM costs are tracked across retry attempts

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/paperqa/docs.py Updates result handling to process lists of LLM results and filter out None contexts
src/paperqa/core.py Implements retry logic with new exception class and refactored function structure

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@jamesbraza jamesbraza force-pushed the retrying-parse branch 2 times, most recently from f7bde6f to 61ab567 Compare September 10, 2025 22:28
Copy link
Copy Markdown
Collaborator

@whitead whitead left a comment

Choose a reason for hiding this comment

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

Any ideas for a test? Maybe a mock llm that throws bad JSON on first call and valid JSON on second?

jamesbraza and others added 2 commits September 10, 2025 16:08
@jamesbraza
Copy link
Copy Markdown
Collaborator Author

Any ideas for a test? Maybe a mock llm that throws bad JSON on first call and valid JSON on second?

Sure, I just created patches to test this in both non-JSON and JSON mode in test_paperqa.py, both JSON failures and score failures

@jamesbraza jamesbraza requested a review from whitead September 10, 2025 23:46
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Sep 11, 2025
@jamesbraza jamesbraza merged commit 589557e into main Sep 11, 2025
4 of 5 checks passed
@jamesbraza jamesbraza deleted the retrying-parse branch September 11, 2025 19:16
@dosubot
Copy link
Copy Markdown

dosubot bot commented Sep 11, 2025

Documentation Updates

Checked 1 published document(s). No updates required.

How did I do? Any feedback?  Join Discord

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants