Skip to content

feat(Harvest): add -- Paper Search and Storage #27

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.

Sign up for GitHub

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

Merged
CJBshuosi merged 3 commits into from
Feb 11, 2026
Merged

feat(Harvest): add -- Paper Search and Storage #27

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
04e1326
feat(Harvest): add -- Paper Search and Storage
CJBshuosi Feb 10, 2026
29a00dd
feat(Harvest): add -- Paper Search and Storage
CJBshuosi Feb 11, 2026
02c765a
[Fix] Fix security issues introduced by the harvest module
CJBshuosi Feb 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions alembic/versions/0003_paper_registry.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,11 @@ def _create_index(name: str, table: str, cols: list[str]) -> None:


def upgrade() -> None:
# NOTE: The papers table may also be created by 0007_paper_harvest_tables with a different schema.
# Only create this version if the table doesn't exist.
created_table = False
if _is_offline() or not _has_table("papers"):
created_table = True
op.create_table(
"papers",
sa.Column("id", sa.Integer(), primary_key=True, autoincrement=True),
Expand All @@ -75,14 +79,19 @@ def upgrade() -> None:
sa.UniqueConstraint("doi", name="uq_papers_doi"),
)

# Only create indexes for columns that exist in this schema version
# These indexes are always safe (columns exist in both schemas):
_create_index("ix_papers_arxiv_id", "papers", ["arxiv_id"])
_create_index("ix_papers_doi", "papers", ["doi"])
_create_index("ix_papers_title", "papers", ["title"])
_create_index("ix_papers_source", "papers", ["source"])
_create_index("ix_papers_published_at", "papers", ["published_at"])
_create_index("ix_papers_first_seen_at", "papers", ["first_seen_at"])
_create_index("ix_papers_created_at", "papers", ["created_at"])
_create_index("ix_papers_updated_at", "papers", ["updated_at"])

# These indexes are only for this schema (not in harvest schema):
if _is_offline() or created_table:
_create_index("ix_papers_title", "papers", ["title"])
_create_index("ix_papers_source", "papers", ["source"])
_create_index("ix_papers_published_at", "papers", ["published_at"])
_create_index("ix_papers_first_seen_at", "papers", ["first_seen_at"])
_create_index("ix_papers_updated_at", "papers", ["updated_at"])


def downgrade() -> None:
Expand Down
138 changes: 138 additions & 0 deletions alembic/versions/0007_paper_harvest_tables.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
"""paper harvest tables

Revision ID: 0007_paper_harvest_tables
Revises: 0006_newsletter_subscribers
Create Date: 2026-02-06

Adds:
- papers: harvested paper metadata with multi-source deduplication
- harvest_runs: harvest execution tracking and audit
"""

from __future__ import annotations

import sqlalchemy as sa
from alembic import context, op

revision = "0007_paper_harvest_tables"
down_revision = "0006_newsletter_subscribers"
branch_labels = None
depends_on = None


def _is_offline() -> bool:
try:
return bool(context.is_offline_mode())
except Exception:
return False


def _insp():
return sa.inspect(op.get_bind())


def _has_table(name: str) -> bool:
return _insp().has_table(name)


def _get_indexes(table: str) -> set[str]:
idx = set()
for i in _insp().get_indexes(table):
idx.add(str(i.get("name") or ""))
return idx


def _create_index(name: str, table: str, cols: list[str]) -> None:
if _is_offline():
op.create_index(name, table, cols)
return
if name in _get_indexes(table):
return
op.create_index(name, table, cols)


def upgrade() -> None:
_upgrade_create_tables()
_upgrade_create_indexes()
Comment thread
coderabbitai[bot] marked this conversation as resolved.


def _upgrade_create_tables() -> None:
# Papers table - harvested paper metadata
if _is_offline() or not _has_table("papers"):
op.create_table(
"papers",
sa.Column("id", sa.Integer(), primary_key=True, autoincrement=True),
# Canonical identifiers (for deduplication)
sa.Column("doi", sa.String(length=256), nullable=True),
sa.Column("arxiv_id", sa.String(length=64), nullable=True),
sa.Column("semantic_scholar_id", sa.String(length=64), nullable=True),
sa.Column("openalex_id", sa.String(length=64), nullable=True),
Comment on lines +66 to +69

@gemini-code-assist gemini-code-assist Bot Feb 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The lengths for doi, arxiv_id, semantic_scholar_id, and openalex_id columns in the papers table seem arbitrary. It's best practice to define these lengths based on the maximum possible length of these identifiers from their respective sources to prevent truncation or unnecessary over-allocation. For example, DOIs can be up to 256 characters, arXiv IDs are typically shorter (e.g., 10-15 chars plus optional version), Semantic Scholar IDs are UUID-like (32-36 chars), and OpenAlex IDs are also UUID-like. Consider reviewing the actual maximum lengths from the sources to set more precise limits.

sa.Column("title_hash", sa.String(length=64), nullable=False),
# Core metadata
sa.Column("title", sa.Text(), nullable=False),
sa.Column("abstract", sa.Text(), server_default="", nullable=False),
sa.Column("authors_json", sa.Text(), server_default="[]", nullable=False),
sa.Column("year", sa.Integer(), nullable=True),
sa.Column("venue", sa.String(length=256), nullable=True),
sa.Column("publication_date", sa.String(length=32), nullable=True),
sa.Column("citation_count", sa.Integer(), server_default="0", nullable=False),
# URLs
sa.Column("url", sa.String(length=1024), nullable=True),
sa.Column("pdf_url", sa.String(length=1024), nullable=True),
# Classification
sa.Column("keywords_json", sa.Text(), server_default="[]", nullable=False),
sa.Column("fields_of_study_json", sa.Text(), server_default="[]", nullable=False),
# Source tracking
sa.Column("primary_source", sa.String(length=32), nullable=False),
sa.Column("sources_json", sa.Text(), server_default="[]", nullable=False),
Comment on lines +64 to +87

Copilot AI Feb 11, 2026

Copy link

Choose a reason for hiding this comment

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

The Alembic table definitions don’t match the SQLAlchemy models for PaperModel (e.g. doi length 256 here vs 128 in the model; url/pdf_url 1024 here vs 512 in the model; arxiv_id 64 vs 32). This can cause migration drift and runtime issues across environments. Align the migration column sizes/types with models.py (and ensure unique constraints in the migration match unique=True in the model).

Copilot uses AI. Check for mistakes.
# Timestamps
sa.Column("created_at", sa.DateTime(timezone=True), nullable=True),
sa.Column("updated_at", sa.DateTime(timezone=True), nullable=True),
sa.Column("deleted_at", sa.DateTime(timezone=True), nullable=True),
)
Comment on lines +62 to +92

@coderabbitai coderabbitai Bot Feb 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find PaperModel definition and its unique constraints
rg -n "class PaperModel" --type py -A 50 | head -100

Repository: jerry609/PaperBot

Length of output: 9639

🏁 Script executed:

#!/bin/bash
# Search for UniqueConstraint declarations related to papers
rg -n "UniqueConstraint.*arxiv_id\|UniqueConstraint.*doi" --type py

Repository: jerry609/PaperBot

Length of output: 43

🏁 Script executed:

#!/bin/bash
# Find PaperStore and _find_existing method
rg -n "class PaperStore" --type py -A 5

Repository: jerry609/PaperBot

Length of output: 499

🏁 Script executed:

#!/bin/bash
# Find _find_existing method
rg -n "def _find_existing" --type py -B 2 -A 20

Repository: jerry609/PaperBot

Length of output: 4328

🏁 Script executed:

#!/bin/bash
# Check the migration file more thoroughly to see full context
fd "0003_paper_harvest_tables.py" --type f

Repository: jerry609/PaperBot

Length of output: 106

Add unique constraints to canonical identifier columns in the papers table migration.

The PaperModel declares doi, arxiv_id, semantic_scholar_id, and openalex_id as unique=True, but the migration creates these columns without unique constraints. Without DB-level enforcement, concurrent upserts can bypass the application's deduplication logic in PaperStore._find_existing and create duplicate rows for the same paper.

🤖 Prompt for AI Agents 
In `@alembic/versions/0003_paper_harvest_tables.py` around lines 65 - 95, The
migration's op.create_table call defines doi, arxiv_id, semantic_scholar_id, and
openalex_id but does not add DB-level uniqueness; add UniqueConstraints (or
unique indexes) for these columns in the same op.create_table call to match
PaperModel's unique=True (e.g., append sa.UniqueConstraint("doi",
name="uq_papers_doi"), sa.UniqueConstraint("arxiv_id",
name="uq_papers_arxiv_id"), sa.UniqueConstraint("semantic_scholar_id",
name="uq_papers_semantic_scholar_id"), sa.UniqueConstraint("openalex_id",
name="uq_papers_openalex_id") so that the op.create_table invocation enforces
uniqueness at the database level and prevents concurrent upsert duplicates).


# Harvest runs table - execution tracking
if _is_offline() or not _has_table("harvest_runs"):
op.create_table(
"harvest_runs",
sa.Column("id", sa.Integer(), primary_key=True, autoincrement=True),
sa.Column("run_id", sa.String(length=64), unique=True, nullable=False),
# Input parameters
sa.Column("keywords_json", sa.Text(), server_default="[]", nullable=False),
sa.Column("venues_json", sa.Text(), server_default="[]", nullable=False),
sa.Column("sources_json", sa.Text(), server_default="[]", nullable=False),
sa.Column("max_results_per_source", sa.Integer(), server_default="100", nullable=False),
# Results
sa.Column("status", sa.String(length=32), server_default="running", nullable=False),
sa.Column("papers_found", sa.Integer(), server_default="0", nullable=False),
sa.Column("papers_new", sa.Integer(), server_default="0", nullable=False),
sa.Column("papers_deduplicated", sa.Integer(), server_default="0", nullable=False),
sa.Column("error_json", sa.Text(), server_default="{}", nullable=False),
# Timestamps
sa.Column("started_at", sa.DateTime(timezone=True), nullable=True),
sa.Column("ended_at", sa.DateTime(timezone=True), nullable=True),
)


def _upgrade_create_indexes() -> None:
# Papers indexes
_create_index("ix_papers_doi", "papers", ["doi"])
_create_index("ix_papers_arxiv_id", "papers", ["arxiv_id"])
_create_index("ix_papers_semantic_scholar_id", "papers", ["semantic_scholar_id"])
_create_index("ix_papers_openalex_id", "papers", ["openalex_id"])
_create_index("ix_papers_title_hash", "papers", ["title_hash"])
_create_index("ix_papers_year", "papers", ["year"])
_create_index("ix_papers_venue", "papers", ["venue"])
_create_index("ix_papers_citation_count", "papers", ["citation_count"])
_create_index("ix_papers_primary_source", "papers", ["primary_source"])
_create_index("ix_papers_created_at", "papers", ["created_at"])

# Harvest runs indexes
_create_index("ix_harvest_runs_run_id", "harvest_runs", ["run_id"])
_create_index("ix_harvest_runs_status", "harvest_runs", ["status"])
_create_index("ix_harvest_runs_started_at", "harvest_runs", ["started_at"])


def downgrade() -> None:
op.drop_table("harvest_runs")
op.drop_table("papers")
Comment on lines +136 to +138

@coderabbitai coderabbitai Bot Feb 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

downgrade() drops tables but doesn't account for the 0003 migration also creating papers.

If 0003 created the papers table and 0007 skipped creation, running downgrade on 0007 will drop a table it didn't create, breaking 0003's expectations. The two migrations need coordinated downgrade logic.

This is a downstream effect of the dual-migration issue flagged in 0003_paper_registry.py.

🤖 Prompt for AI Agents 
In `@alembic/versions/0007_paper_harvest_tables.py` around lines 136 - 138, The
downgrade() in this migration blindly drops "papers" even though that table may
have been created by the earlier 0003 migration; change the downgrade so it only
drops tables this migration actually creates (keep op.drop_table("harvest_runs")
but remove or guard op.drop_table("papers")). Update the downgrade() in
0007_paper_harvest_tables.py to either remove the op.drop_table("papers") call
or wrap it in a runtime check that ensures this migration created the table
(e.g., check current revision or whether this migration added the table) so you
don't break 0003_paper_registry.py's expectations.

Loading
Loading