Skip to content

Context id updates#1089

Merged
mskarlin merged 4 commits intomainfrom
context-id-improvements
Sep 16, 2025
Merged

Context id updates#1089
mskarlin merged 4 commits intomainfrom
context-id-improvements

Conversation

@mskarlin
Copy link
Copy Markdown
Collaborator

We had some scenarios where our formatted answer could miss context-ids. This implements a more universal matching solution along with tests.

@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. bug Something isn't working labels Sep 16, 2025
@dosubot
Copy link
Copy Markdown

dosubot bot commented Sep 16, 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 a more robust solution for handling context ID parsing and replacement in formatted answers. It addresses scenarios where context IDs could be missed in the previous implementation.

  • Replaces regex-based parenthetical matching with a proper parser
  • Updates get_citation_ids to preserve order and handle duplicates
  • Adds comprehensive test coverage for various edge cases in context ID parsing

Reviewed Changes

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

File Description
src/paperqa/utils.py Adds get_parenthetical_substrings function and updates get_citation_ids to return list with preserved order
src/paperqa/types.py Updates context ID replacement logic to use new parsing functions and fix parenthetical replacement
tests/test_paperqa.py Adds comprehensive test cases for context ID parsing edge cases

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

Copy link
Copy Markdown
Collaborator

@jamesbraza jamesbraza left a comment

Choose a reason for hiding this comment

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

Nice work

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Sep 16, 2025
Copy link
Copy Markdown

@cursor cursor bot left a 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.

Comment @cursor review or bugbot run to trigger another review on this PR

@mskarlin mskarlin enabled auto-merge (squash) September 16, 2025 21:51
@mskarlin mskarlin merged commit 026d5c7 into main Sep 16, 2025
5 checks passed
@mskarlin mskarlin deleted the context-id-improvements branch September 16, 2025 22:06
@dosubot
Copy link
Copy Markdown

dosubot bot commented Sep 16, 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.

3 participants