feat(Harvest): Implement the function to obtain anchor author#117
Conversation
Closes jerry609#116 Signed-off-by: LIU BOYU <oor2020@163.com>
|
@CJBshuosi is attempting to deploy a commit to the Jerry's projects Team on Vercel. A member of the Team first needs to authorize it. |
Summary of ChangesHello @CJBshuosi, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant enhancement to the PaperStore by integrating robust author management capabilities. It enables the system to properly link authors to papers during the data harvesting process, ensuring that author information is consistently stored and associated with the relevant research papers. This change improves data integrity and expands the functionality for managing bibliographic data. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughWalkthroughThe pull request adds author synchronization to the paper storage workflow. After a paper is created or updated, the system now attempts to persist paper-author associations through an AuthorStore instance with graceful error handling. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/paperbot/infrastructure/stores/paper_store.py (1)
466-513:⚠️ Potential issue | 🟠 Major
upsert_papers_batchdoes not sync authors to thepaper_authorstable.The
upsert_papermethod syncs authors viareplace_paper_authors(lines 274–280), butupsert_papers_batchskips this step entirely. Papers ingested throughupsert_papers_batchwill haveauthors_jsonpopulated onPaperModelbut no corresponding rows in thepaper_authorsjoin table. If anchor author discovery queriespaper_authors, these papers will be invisible to that feature.Add author sync after the batch commit by iterating over the flushed papers and calling
replace_paper_authorsfor each, matching the pattern inupsert_paper.
🧹 Nitpick comments (1)
src/paperbot/infrastructure/stores/paper_store.py (1)
104-104: Consider sharing theSessionProviderwithAuthorStoreinstead of creating a duplicate engine.Both
PaperStoreand the embeddedAuthorStoreinstantiate their ownSessionProvideragainst the samedb_url, which means two separate connection pools to the same database. IfAuthorStorecan accept an existingSessionProvideror engine, that would reduce resource usage.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements the functionality to sync author information when a paper is created or updated. A medium-severity vulnerability was identified related to the logging of sensitive information in exception handlers, which could lead to potential data leaks. Additionally, the error handling should be refined to catch more specific exceptions instead of a general Exception to improve robustness and maintainability.
| Logger.warning( | ||
| f"Failed to sync paper authors for paper {row.id}: {e}", | ||
| file=LogFiles.HARVEST, | ||
| ) |
There was a problem hiding this comment.
The application logs the entire exception object e when an error occurs while syncing paper authors. The string representation of a database exception can include sensitive information, such as the full SQL query with its parameters or other internal details. Logging such raw exception data can lead to the exposure of sensitive user information (e.g., author names, IDs) in the log files, which may not have the same security controls as the primary database.
| Logger.warning( | |
| f"Failed to sync paper authors for paper {row.id}: {e}", | |
| file=LogFiles.HARVEST, | |
| ) | |
| Logger.warning( | |
| f"Failed to sync paper authors for paper {row.id}. Reason: An unexpected error occurred.", | |
| file=LogFiles.HARVEST, | |
| ) |
| PaperModel, | ||
| ) | ||
| from paperbot.infrastructure.stores.sqlalchemy_db import SessionProvider, get_db_url | ||
| from paperbot.infrastructure.stores.author_store import AuthorStore |
There was a problem hiding this comment.
To handle database-specific errors gracefully when syncing authors, let's import SQLAlchemyError. This will be used in the try...except block below.
| from paperbot.infrastructure.stores.author_store import AuthorStore | |
| from paperbot.infrastructure.stores.author_store import AuthorStore | |
| from sqlalchemy.exc import SQLAlchemyError |
| paper_id=int(row.id), | ||
| authors=authors, | ||
| ) | ||
| except Exception as e: |
There was a problem hiding this comment.
Using a broad except Exception can mask underlying issues and make debugging harder. It's better practice to catch more specific exceptions that you anticipate, like ValueError (which replace_paper_authors can raise) and database-related errors like SQLAlchemyError.
| except Exception as e: | |
| except (ValueError, SQLAlchemyError) as e: |
1 participant
Closes #116
Summary by CodeRabbit
New Features
Bug Fixes