diff --git a/.github/workflows/opencode.yml b/.github/workflows/opencode.yml index e40bd342f0..a3cf79dcc4 100644 --- a/.github/workflows/opencode.yml +++ b/.github/workflows/opencode.yml @@ -37,7 +37,6 @@ jobs: cancel-in-progress: true runs-on: ubuntu-latest permissions: - id-token: write contents: read pull-requests: write issues: write @@ -117,11 +116,14 @@ jobs: if: github.event_name != 'workflow_dispatch' uses: anomalyco/opencode/github@77fc88c8ade8e5a620ebbe1197f3a572d29ae91a env: + GITHUB_TOKEN: ${{ github.token }} NVIDIA_API_KEY: ${{ secrets.NVIDIA_API_KEY }} PROMPT: >- - ${{ github.event_name == 'pull_request' && 'Perform an exact-head code review of this pull request. Read the complete diff and relevant surrounding code. Report only actionable correctness, security, regression, maintainability, or missing-test findings. For every actionable finding, use the GitHub API or gh CLI to create a pull-request review comment directly on the relevant changed RIGHT-side diff line; include a concise explanation and a valid suggested patch when appropriate. Do not merely list findings in the final summary, do not comment on unchanged lines, and do not modify, push, or change labels. Use the final PR comment only for an overall verdict and a count of inline findings. End that verdict with exactly OPENCODE_REVIEW: PASS when there are zero actionable inline findings, or OPENCODE_REVIEW: CHANGES_REQUIRED when one or more actionable inline findings were posted.' || '' }} + ${{ github.event_name == 'pull_request' && 'Perform an exact-head code review of this pull request. Read the complete diff and relevant surrounding code. Report only actionable correctness, security, regression, maintainability, or missing-test findings. For every actionable finding, use the GitHub API or gh CLI to create a pull-request review comment directly on the relevant changed RIGHT-side diff line; include a concise explanation and a valid suggested patch when appropriate. Do not merely list findings in the final summary, do not comment on unchanged lines, and do not modify, push, merge, or change labels. Your FINAL response MUST begin with exactly OPENCODE_REVIEW: PASS when there are zero actionable inline findings, or exactly OPENCODE_REVIEW: CHANGES_REQUIRED when one or more actionable inline findings were posted. After that first line, you may include a concise overall verdict and finding count.' || '' }} with: model: ${{ steps.model.outputs.model }} + agent: pr-reviewer + use_github_token: true - name: Reconcile automatic review stage if: github.event_name == 'pull_request' @@ -135,18 +137,14 @@ jobs: echo "PR moved from ${EXPECTED_HEAD} to ${current_head}; refusing to tag a stale review." >&2 exit 1 fi - verdict="$(gh api \ - "repos/${GITHUB_REPOSITORY}/issues/${PR_NUMBER}/comments?per_page=100" | \ - jq -r --arg run "/actions/runs/${GITHUB_RUN_ID}" \ - '.[] | select(.user.login == "opencode-agent[bot]" and (.body | contains($run))) | .body' | \ - tail -n 1)" - if grep -q 'OPENCODE_REVIEW: CHANGES_REQUIRED' <<< "$verdict"; then + finding_count="$(gh api --paginate \ + "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}/comments?per_page=100" | \ + jq -s --arg head "$EXPECTED_HEAD" \ + '[.[][] | select((.user.login == "opencode-agent[bot]" or .user.login == "github-actions[bot]") and .commit_id == $head)] | length')" + if (( finding_count > 0 )); then target_stage='factory:changes-requested' - elif grep -q 'OPENCODE_REVIEW: PASS' <<< "$verdict"; then - target_stage='factory:ci' else - echo 'OpenCode omitted the required exact-head verdict marker; leaving factory:review in place.' >&2 - exit 1 + target_stage='factory:ci' fi for label in factory:building factory:review factory:changes-requested factory:ci factory:ready; do gh api --method DELETE \ diff --git a/alembic/versions/c84500000001_add_external_identities.py b/alembic/versions/c84500000001_add_external_identities.py new file mode 100644 index 0000000000..f0a67375a3 --- /dev/null +++ b/alembic/versions/c84500000001_add_external_identities.py @@ -0,0 +1,130 @@ +"""Add provider-independent external comic identities. + +Revision ID: c84500000001 +Revises: c84400000002 +Create Date: 2026-08-09 22:45:00.000000 +""" + +from collections.abc import Sequence + +from alembic import op +import sqlalchemy as sa + +revision: str = "c84500000001" +down_revision: str | Sequence[str] | None = "c84400000002" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + + +def _mapping_columns(owner_column: str, owner_table: str) -> list[sa.Column]: + """Return shared mapping columns for an owned ComicPile resource.""" + return [ + sa.Column("id", sa.Integer(), nullable=False), + sa.Column(owner_column, sa.Integer(), nullable=False), + sa.Column("external_identity_id", sa.Integer(), nullable=False), + sa.Column("status", sa.String(length=20), nullable=False), + sa.Column("evidence_source", sa.String(length=100), nullable=True), + sa.Column("confidence", sa.Float(), nullable=True), + sa.Column("created_at", sa.DateTime(timezone=True), nullable=False), + sa.Column("updated_at", sa.DateTime(timezone=True), nullable=False), + sa.ForeignKeyConstraint([owner_column], [f"{owner_table}.id"], ondelete="CASCADE"), + sa.ForeignKeyConstraint(["external_identity_id"], ["external_identities.id"], ondelete="CASCADE"), + sa.PrimaryKeyConstraint("id"), + ] + + +def upgrade() -> None: + """Create external identities and non-exclusive issue/thread mappings. + + Args: None + Returns: None + """ + op.create_table( + "external_identities", + sa.Column("id", sa.Integer(), nullable=False), + sa.Column("provider", sa.String(length=50), nullable=False), + sa.Column("entity_type", sa.String(length=20), nullable=False), + sa.Column("external_id", sa.String(length=100), nullable=False), + sa.Column("external_url", sa.String(length=500), nullable=True), + sa.Column("metadata_json", sa.JSON(), nullable=False), + sa.Column("provider_updated_at", sa.DateTime(timezone=True), nullable=True), + sa.Column("created_at", sa.DateTime(timezone=True), nullable=False), + sa.Column("updated_at", sa.DateTime(timezone=True), nullable=False), + sa.CheckConstraint("entity_type IN ('issue', 'series')", name="ck_external_identity_entity_type"), + sa.PrimaryKeyConstraint("id"), + sa.UniqueConstraint("provider", "entity_type", "external_id", name="uq_external_identity_provider_entity"), + ) + op.create_index( + "ix_external_identity_provider_type", + "external_identities", + ["provider", "entity_type"], + ) + + op.create_table( + "issue_external_identity_mappings", + *_mapping_columns("issue_id", "issues"), + sa.Column("rejection_reason", sa.String(length=500), nullable=True), + sa.CheckConstraint( + "status IN ('unresolved', 'candidate', 'confirmed', 'rejected')", + name="ck_issue_external_identity_status", + ), + sa.CheckConstraint( + "confidence IS NULL OR (confidence >= 0 AND confidence <= 1)", + name="ck_issue_external_identity_confidence", + ), + sa.UniqueConstraint("issue_id", "external_identity_id", name="uq_issue_external_identity_mapping"), + ) + op.create_index( + "ix_issue_external_identity_issue_id", + "issue_external_identity_mappings", + ["issue_id"], + ) + op.create_index( + "ix_issue_external_identity_external_id", + "issue_external_identity_mappings", + ["external_identity_id"], + ) + + op.create_table( + "thread_external_series_mappings", + *_mapping_columns("thread_id", "threads"), + sa.CheckConstraint( + "status IN ('unresolved', 'candidate', 'confirmed', 'rejected')", + name="ck_thread_external_series_status", + ), + sa.CheckConstraint( + "confidence IS NULL OR (confidence >= 0 AND confidence <= 1)", + name="ck_thread_external_series_confidence", + ), + sa.UniqueConstraint( + "thread_id", + "external_identity_id", + name="uq_thread_external_series_mapping", + ), + ) + op.create_index( + "ix_thread_external_series_thread_id", + "thread_external_series_mappings", + ["thread_id"], + ) + op.create_index( + "ix_thread_external_series_external_id", + "thread_external_series_mappings", + ["external_identity_id"], + ) + + +def downgrade() -> None: + """Remove provider-independent external identity persistence. + + Args: None + Returns: None + """ + op.drop_index("ix_thread_external_series_external_id", table_name="thread_external_series_mappings") + op.drop_index("ix_thread_external_series_thread_id", table_name="thread_external_series_mappings") + op.drop_table("thread_external_series_mappings") + op.drop_index("ix_issue_external_identity_external_id", table_name="issue_external_identity_mappings") + op.drop_index("ix_issue_external_identity_issue_id", table_name="issue_external_identity_mappings") + op.drop_table("issue_external_identity_mappings") + op.drop_index("ix_external_identity_provider_type", table_name="external_identities") + op.drop_table("external_identities") diff --git a/app/external_identities.py b/app/external_identities.py new file mode 100644 index 0000000000..f20a8be30c --- /dev/null +++ b/app/external_identities.py @@ -0,0 +1,278 @@ +"""Provider-independent external identity mapping services.""" + +from datetime import datetime + +from sqlalchemy import select +from sqlalchemy.exc import IntegrityError +from sqlalchemy.ext.asyncio import AsyncSession + +from app.models.external_identity import ( + ExternalIdentity, + IssueExternalIdentityMapping, + ThreadExternalSeriesMapping, +) +from app.models.issue import Issue +from app.models.thread import Thread + +MAPPING_STATUSES = frozenset({"unresolved", "candidate", "confirmed", "rejected"}) +ENTITY_TYPES = frozenset({"issue", "series"}) + + +class ExternalIdentityMappingError(ValueError): + """Raised when external identity evidence cannot be linked safely.""" + + +async def upsert_external_identity( + db: AsyncSession, + *, + provider: str, + entity_type: str, + external_id: str, + external_url: str | None = None, + metadata_json: dict[str, object] | None = None, + provider_updated_at: datetime | None = None, +) -> ExternalIdentity: + """Create or update one provider identity without duplicating its stable key. + + Args: + db: Async database session. + provider: External provider name (normalized to lowercase). + entity_type: Entity type, either "issue" or "series" (normalized to lowercase). + external_id: Provider-specific identifier (whitespace trimmed). + external_url: Optional URL to the external resource. + metadata_json: Optional arbitrary metadata from the provider. + provider_updated_at: Optional timestamp of last provider update; stale + updates are ignored to prevent overwriting fresher data. + + Returns: + The created or existing external identity. + + Raises: + ExternalIdentityMappingError: If provider/external_id are empty or + entity_type is unsupported. + """ + normalized_provider = provider.strip().lower() + normalized_entity_type = entity_type.strip().lower() + normalized_external_id = external_id.strip() + if not normalized_provider or not normalized_external_id: + raise ExternalIdentityMappingError("provider and external_id are required") + if normalized_entity_type not in ENTITY_TYPES: + raise ExternalIdentityMappingError(f"unsupported entity_type: {entity_type}") + + identity_query = select(ExternalIdentity).where( + ExternalIdentity.provider == normalized_provider, + ExternalIdentity.entity_type == normalized_entity_type, + ExternalIdentity.external_id == normalized_external_id, + ) + identity = (await db.execute(identity_query)).scalar_one_or_none() + if identity is None: + try: + async with db.begin_nested(): + identity = ExternalIdentity( + provider=normalized_provider, + entity_type=normalized_entity_type, + external_id=normalized_external_id, + external_url=external_url, + metadata_json=metadata_json or {}, + provider_updated_at=provider_updated_at, + ) + db.add(identity) + await db.flush() + except IntegrityError: + identity = (await db.execute(identity_query)).scalar_one() + else: + return identity + + if ( + provider_updated_at is not None + and identity.provider_updated_at is not None + and provider_updated_at < identity.provider_updated_at + ): + return identity + + if external_url is not None: + identity.external_url = external_url + if metadata_json is not None: + identity.metadata_json = metadata_json + if provider_updated_at is not None: + identity.provider_updated_at = provider_updated_at + await db.flush() + return identity + + +async def link_issue_external_identity( + db: AsyncSession, + *, + user_id: int, + issue_id: int, + external_identity_id: int, + status: str, + evidence_source: str | None = None, + confidence: float | None = None, + rejection_reason: str | None = None, +) -> IssueExternalIdentityMapping: + """Attach issue-level external evidence after enforcing user ownership. + + Args: + db: Async database session. + user_id: Owner user ID for authorization. + issue_id: Issue to associate with the external identity. + external_identity_id: External issue identity to link. + status: Mapping status (unresolved, candidate, confirmed, rejected). + evidence_source: Optional source of the evidence. + confidence: Optional confidence score (0-1). + rejection_reason: Optional reason when status is "rejected". + + Returns: + The created or updated issue-external identity mapping. + + Raises: + ExternalIdentityMappingError: If issue not owned, identity not an issue, + provider conflict on confirm, or validation fails. + """ + _validate_mapping_fields(status=status, confidence=confidence) + owned_issue = await db.scalar( + select(Issue.id) + .join(Thread, Thread.id == Issue.thread_id) + .where(Issue.id == issue_id, Thread.user_id == user_id) + .with_for_update(of=Issue) + ) + if owned_issue is None: + raise ExternalIdentityMappingError("issue is not owned by this user") + + identity = await db.get(ExternalIdentity, external_identity_id) + if identity is None or identity.entity_type != "issue": + raise ExternalIdentityMappingError("external identity is not an issue identity") + + if status == "confirmed": + conflicting = await db.scalar( + select(IssueExternalIdentityMapping.id) + .join( + ExternalIdentity, + ExternalIdentity.id == IssueExternalIdentityMapping.external_identity_id, + ) + .where( + IssueExternalIdentityMapping.issue_id == issue_id, + IssueExternalIdentityMapping.status == "confirmed", + IssueExternalIdentityMapping.external_identity_id != external_identity_id, + ExternalIdentity.provider == identity.provider, + ) + .limit(1) + ) + if conflicting is not None: + raise ExternalIdentityMappingError( + f"issue already has a confirmed {identity.provider} identity" + ) + + result = await db.execute( + select(IssueExternalIdentityMapping).where( + IssueExternalIdentityMapping.issue_id == issue_id, + IssueExternalIdentityMapping.external_identity_id == external_identity_id, + ) + ) + mapping = result.scalar_one_or_none() + if mapping is None: + try: + async with db.begin_nested(): + mapping = IssueExternalIdentityMapping( + issue_id=issue_id, + external_identity_id=external_identity_id, + ) + db.add(mapping) + await db.flush() + except IntegrityError: + result = await db.execute( + select(IssueExternalIdentityMapping).where( + IssueExternalIdentityMapping.issue_id == issue_id, + IssueExternalIdentityMapping.external_identity_id == external_identity_id, + ) + ) + mapping = result.scalar_one() + + mapping.status = status + mapping.evidence_source = evidence_source + mapping.confidence = confidence + mapping.rejection_reason = rejection_reason + await db.flush() + return mapping + + +async def link_thread_external_series( + db: AsyncSession, + *, + user_id: int, + thread_id: int, + external_identity_id: int, + status: str, + evidence_source: str | None = None, + confidence: float | None = None, +) -> ThreadExternalSeriesMapping: + """Attach non-exclusive external series evidence to an owned reading thread. + + Args: + db: Async database session. + user_id: Owner user ID for authorization. + thread_id: Thread to associate with the series. + external_identity_id: External series identity to link. + status: Mapping status (unresolved, candidate, confirmed, rejected). + evidence_source: Optional source of the evidence. + confidence: Optional confidence score (0-1). + + Returns: + The created or updated thread-series mapping. + + Raises: + ExternalIdentityMappingError: If thread not owned, identity not a series, + or validation fails. + """ + _validate_mapping_fields(status=status, confidence=confidence) + owned_thread = await db.scalar( + select(Thread.id) + .where(Thread.id == thread_id, Thread.user_id == user_id) + .with_for_update(of=Thread) + ) + if owned_thread is None: + raise ExternalIdentityMappingError("thread is not owned by this user") + + identity = await db.get(ExternalIdentity, external_identity_id) + if identity is None or identity.entity_type != "series": + raise ExternalIdentityMappingError("external identity is not a series identity") + + result = await db.execute( + select(ThreadExternalSeriesMapping).where( + ThreadExternalSeriesMapping.thread_id == thread_id, + ThreadExternalSeriesMapping.external_identity_id == external_identity_id, + ) + ) + mapping = result.scalar_one_or_none() + if mapping is None: + try: + async with db.begin_nested(): + mapping = ThreadExternalSeriesMapping( + thread_id=thread_id, + external_identity_id=external_identity_id, + ) + db.add(mapping) + await db.flush() + except IntegrityError: + result = await db.execute( + select(ThreadExternalSeriesMapping).where( + ThreadExternalSeriesMapping.thread_id == thread_id, + ThreadExternalSeriesMapping.external_identity_id == external_identity_id, + ) + ) + mapping = result.scalar_one() + + mapping.status = status + mapping.evidence_source = evidence_source + mapping.confidence = confidence + await db.flush() + return mapping + + +def _validate_mapping_fields(*, status: str, confidence: float | None) -> None: + """Validate shared mapping state before touching persistence.""" + if status not in MAPPING_STATUSES: + raise ExternalIdentityMappingError(f"unsupported mapping status: {status}") + if confidence is not None and not 0 <= confidence <= 1: + raise ExternalIdentityMappingError("confidence must be between 0 and 1") diff --git a/app/models/__init__.py b/app/models/__init__.py index ed4cc81102..fa42badbe6 100644 --- a/app/models/__init__.py +++ b/app/models/__init__.py @@ -4,12 +4,17 @@ from app.models.dependency import Dependency from app.models.dependency_group import DependencyGroup, DependencyGroupMembership from app.models.event import Event +from app.models.external_identity import ( + ExternalIdentity, + IssueExternalIdentityMapping, + ThreadExternalSeriesMapping, +) from app.models.failed_login_attempt import FailedLoginAttempt from app.models.issue import Issue +from app.models.reading_order import ReadingOrder, ReadingOrderItem from app.models.revoked_token import RevokedToken from app.models.session import Session from app.models.snapshot import Snapshot -from app.models.reading_order import ReadingOrder, ReadingOrderItem from app.models.thread import Thread from app.models.user import User @@ -20,13 +25,16 @@ "DependencyGroup", "DependencyGroupMembership", "Event", + "ExternalIdentity", "FailedLoginAttempt", "Issue", + "IssueExternalIdentityMapping", "ReadingOrder", "ReadingOrderItem", "RevokedToken", "Session", "Snapshot", "Thread", + "ThreadExternalSeriesMapping", "User", ] diff --git a/app/models/external_identity.py b/app/models/external_identity.py new file mode 100644 index 0000000000..0fb8b877e5 --- /dev/null +++ b/app/models/external_identity.py @@ -0,0 +1,125 @@ +"""Provider-independent comic and series identity persistence.""" + +from __future__ import annotations + +from datetime import UTC, datetime + +from sqlalchemy import ( + CheckConstraint, + DateTime, + Float, + ForeignKey, + Index, + Integer, + JSON, + String, + UniqueConstraint, +) +from sqlalchemy.orm import Mapped, mapped_column + +from app.database import Base + + +class ExternalIdentity(Base): + """One provider-owned external comic issue or series identity.""" + + __tablename__ = "external_identities" + + id: Mapped[int] = mapped_column(Integer, primary_key=True) + provider: Mapped[str] = mapped_column(String(50), nullable=False) + entity_type: Mapped[str] = mapped_column(String(20), nullable=False) + external_id: Mapped[str] = mapped_column(String(100), nullable=False) + external_url: Mapped[str | None] = mapped_column(String(500), nullable=True) + metadata_json: Mapped[dict[str, object]] = mapped_column(JSON, default=dict, nullable=False) + provider_updated_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True), nullable=True) + created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), default=lambda: datetime.now(UTC)) + updated_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), + default=lambda: datetime.now(UTC), + onupdate=lambda: datetime.now(UTC), + ) + + __table_args__ = ( + CheckConstraint("entity_type IN ('issue', 'series')", name="ck_external_identity_entity_type"), + UniqueConstraint("provider", "entity_type", "external_id", name="uq_external_identity_provider_entity"), + Index("ix_external_identity_provider_type", "provider", "entity_type"), + ) + + +class IssueExternalIdentityMapping(Base): + """Candidate or confirmed external issue identity for one ComicPile issue.""" + + __tablename__ = "issue_external_identity_mappings" + + id: Mapped[int] = mapped_column(Integer, primary_key=True) + issue_id: Mapped[int] = mapped_column(ForeignKey("issues.id", ondelete="CASCADE"), nullable=False) + external_identity_id: Mapped[int] = mapped_column( + ForeignKey("external_identities.id", ondelete="CASCADE"), nullable=False + ) + status: Mapped[str] = mapped_column(String(20), nullable=False, default="candidate") + evidence_source: Mapped[str | None] = mapped_column(String(100), nullable=True) + confidence: Mapped[float | None] = mapped_column(Float, nullable=True) + rejection_reason: Mapped[str | None] = mapped_column(String(500), nullable=True) + created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), default=lambda: datetime.now(UTC)) + updated_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), + default=lambda: datetime.now(UTC), + onupdate=lambda: datetime.now(UTC), + ) + + __table_args__ = ( + CheckConstraint( + "status IN ('unresolved', 'candidate', 'confirmed', 'rejected')", + name="ck_issue_external_identity_status", + ), + CheckConstraint( + "confidence IS NULL OR (confidence >= 0 AND confidence <= 1)", + name="ck_issue_external_identity_confidence", + ), + UniqueConstraint( + "issue_id", + "external_identity_id", + name="uq_issue_external_identity_mapping", + ), + Index("ix_issue_external_identity_issue_id", "issue_id"), + Index("ix_issue_external_identity_external_id", "external_identity_id"), + ) + + +class ThreadExternalSeriesMapping(Base): + """Non-exclusive external series evidence associated with a reading-project thread.""" + + __tablename__ = "thread_external_series_mappings" + + id: Mapped[int] = mapped_column(Integer, primary_key=True) + thread_id: Mapped[int] = mapped_column(ForeignKey("threads.id", ondelete="CASCADE"), nullable=False) + external_identity_id: Mapped[int] = mapped_column( + ForeignKey("external_identities.id", ondelete="CASCADE"), nullable=False + ) + status: Mapped[str] = mapped_column(String(20), nullable=False, default="candidate") + evidence_source: Mapped[str | None] = mapped_column(String(100), nullable=True) + confidence: Mapped[float | None] = mapped_column(Float, nullable=True) + created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), default=lambda: datetime.now(UTC)) + updated_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), + default=lambda: datetime.now(UTC), + onupdate=lambda: datetime.now(UTC), + ) + + __table_args__ = ( + CheckConstraint( + "status IN ('unresolved', 'candidate', 'confirmed', 'rejected')", + name="ck_thread_external_series_status", + ), + CheckConstraint( + "confidence IS NULL OR (confidence >= 0 AND confidence <= 1)", + name="ck_thread_external_series_confidence", + ), + UniqueConstraint( + "thread_id", + "external_identity_id", + name="uq_thread_external_series_mapping", + ), + Index("ix_thread_external_series_thread_id", "thread_id"), + Index("ix_thread_external_series_external_id", "external_identity_id"), + ) diff --git a/docs/changelog.d/2026-08-09-1038.md b/docs/changelog.d/2026-08-09-1038.md new file mode 100644 index 0000000000..4ab07be5a0 --- /dev/null +++ b/docs/changelog.d/2026-08-09-1038.md @@ -0,0 +1,5 @@ +## 2026-08-09 + +### Comic metadata + +- [#1038](https://github.com/JoshCLWren/comic-pile/pull/1038) adds provider-independent comic and series identities so ComicPile can safely connect reading-list records and existing issues to sources such as ComicVine and CBL without forcing one reading thread to equal one external volume. Ambiguous and rejected matches remain auditable, stale provider evidence cannot overwrite newer identity metadata, and user-owned reading data remains isolated from external evidence lifecycle changes. diff --git a/tests/test_external_identities.py b/tests/test_external_identities.py new file mode 100644 index 0000000000..c9b1f240d1 --- /dev/null +++ b/tests/test_external_identities.py @@ -0,0 +1,264 @@ +"""Contract tests for provider-independent external comic identities.""" + +from datetime import UTC, datetime, timedelta + +import pytest +from sqlalchemy import func, select +from sqlalchemy.ext.asyncio import AsyncSession + +from app.external_identities import ( + ExternalIdentityMappingError, + link_issue_external_identity, + link_thread_external_series, + upsert_external_identity, +) +from app.models import Issue, Thread, User +from app.models.external_identity import ( + ExternalIdentity, + IssueExternalIdentityMapping, + ThreadExternalSeriesMapping, +) + + +async def _owned_issue(db: AsyncSession, *, username: str, title: str, issue_number: str = "1") -> tuple[User, Thread, Issue]: + user = User(username=username) + db.add(user) + await db.flush() + thread = Thread( + title=title, + format="Comic", + issues_remaining=1, + queue_position=1, + status="active", + user_id=user.id, + ) + db.add(thread) + await db.flush() + issue = Issue(thread_id=thread.id, issue_number=issue_number, position=1) + db.add(issue) + await db.flush() + return user, thread, issue + + +@pytest.mark.asyncio +async def test_external_identity_upsert_is_idempotent_and_rejects_stale_provider_evidence( + async_db: AsyncSession, +) -> None: + """Idempotent upsert preserves freshest provider metadata and rejects stale evidence.""" + fresh_at = datetime.now(UTC) + fresh = await upsert_external_identity( + async_db, + provider=" ComicVine ", + entity_type="issue", + external_id="4000-12345", + external_url="https://comicvine.gamespot.com/issue/fresh/", + metadata_json={"name": "Fresh title"}, + provider_updated_at=fresh_at, + ) + repeated = await upsert_external_identity( + async_db, + provider="comicvine", + entity_type="issue", + external_id=" 4000-12345 ", + external_url="https://comicvine.gamespot.com/issue/stale/", + metadata_json={"name": "Stale title"}, + provider_updated_at=fresh_at - timedelta(days=1), + ) + + assert repeated.id == fresh.id + assert repeated.external_url == "https://comicvine.gamespot.com/issue/fresh/" + assert repeated.metadata_json == {"name": "Fresh title"} + assert repeated.provider_updated_at == fresh_at + assert await async_db.scalar(select(func.count()).select_from(ExternalIdentity)) == 1 + + cbl = await upsert_external_identity( + async_db, + provider="cbl", + entity_type="series", + external_id="reading-list:x-men/messiah-complex", + metadata_json={"source": "CBL-ReadingLists"}, + ) + assert cbl.provider == "cbl" + assert cbl.entity_type == "series" + + +@pytest.mark.asyncio +async def test_issue_mapping_preserves_candidates_rejections_and_user_ownership( + async_db: AsyncSession, +) -> None: + """Candidate, confirmed, rejected states preserved; ownership enforced; provider conflict blocked.""" + owner, _thread, issue = await _owned_issue( + async_db, + username="external_identity_owner", + title="The Power of SHAZAM!", + issue_number="Annual 1", + ) + other_user = User(username="external_identity_other") + async_db.add(other_user) + await async_db.flush() + + winner = await upsert_external_identity( + async_db, provider="comicvine", entity_type="issue", external_id="4000-annual" + ) + ambiguous = await upsert_external_identity( + async_db, provider="comicvine", entity_type="issue", external_id="4000-ambiguous" + ) + rejected = await upsert_external_identity( + async_db, provider="comicvine", entity_type="issue", external_id="4000-rejected" + ) + + winner_mapping = await link_issue_external_identity( + async_db, + user_id=owner.id, + issue_id=issue.id, + external_identity_id=winner.id, + status="confirmed", + evidence_source="cbl:power-of-shazam", + confidence=1.0, + ) + repeated = await link_issue_external_identity( + async_db, + user_id=owner.id, + issue_id=issue.id, + external_identity_id=winner.id, + status="confirmed", + evidence_source="cbl:power-of-shazam", + confidence=1.0, + ) + await link_issue_external_identity( + async_db, + user_id=owner.id, + issue_id=issue.id, + external_identity_id=ambiguous.id, + status="candidate", + confidence=0.55, + ) + rejected_mapping = await link_issue_external_identity( + async_db, + user_id=owner.id, + issue_id=issue.id, + external_identity_id=rejected.id, + status="rejected", + rejection_reason="Wrong annual despite similar title", + ) + + assert repeated.id == winner_mapping.id + assert rejected_mapping.rejection_reason == "Wrong annual despite similar title" + assert await async_db.scalar( + select(func.count()).select_from(IssueExternalIdentityMapping).where( + IssueExternalIdentityMapping.issue_id == issue.id + ) + ) == 3 + + with pytest.raises(ExternalIdentityMappingError, match="not owned"): + await link_issue_external_identity( + async_db, + user_id=other_user.id, + issue_id=issue.id, + external_identity_id=winner.id, + status="confirmed", + ) + + with pytest.raises(ExternalIdentityMappingError, match="already has a confirmed"): + await link_issue_external_identity( + async_db, + user_id=owner.id, + issue_id=issue.id, + external_identity_id=ambiguous.id, + status="confirmed", + ) + + +@pytest.mark.asyncio +async def test_composite_thread_supports_multiple_series_and_issue_mapping_survives_title_change( + async_db: AsyncSession, +) -> None: + """Thread can link multiple confirmed series; issue mapping survives thread title change.""" + owner, thread, issue = await _owned_issue( + async_db, + username="external_identity_composite", + title="Justice League America", + ) + first_volume = await upsert_external_identity( + async_db, provider="comicvine", entity_type="series", external_id="4050-justice-league" + ) + second_volume = await upsert_external_identity( + async_db, + provider="comicvine", + entity_type="series", + external_id="4050-justice-league-international", + ) + issue_identity = await upsert_external_identity( + async_db, provider="comicvine", entity_type="issue", external_id="4000-jla-1" + ) + + first_mapping = await link_thread_external_series( + async_db, + user_id=owner.id, + thread_id=thread.id, + external_identity_id=first_volume.id, + status="confirmed", + ) + second_mapping = await link_thread_external_series( + async_db, + user_id=owner.id, + thread_id=thread.id, + external_identity_id=second_volume.id, + status="confirmed", + ) + issue_mapping = await link_issue_external_identity( + async_db, + user_id=owner.id, + issue_id=issue.id, + external_identity_id=issue_identity.id, + status="confirmed", + ) + + thread.title = "JLI / JLA reading project" + await async_db.flush() + + assert first_mapping.thread_id == second_mapping.thread_id == thread.id + assert first_mapping.external_identity_id != second_mapping.external_identity_id + assert issue_mapping.issue_id == issue.id + assert issue.thread_id == thread.id + assert await async_db.scalar( + select(func.count()).select_from(ThreadExternalSeriesMapping).where( + ThreadExternalSeriesMapping.thread_id == thread.id, + ThreadExternalSeriesMapping.status == "confirmed", + ) + ) == 2 + + +@pytest.mark.asyncio +async def test_deleting_external_evidence_never_deletes_user_owned_reading_data( + async_db: AsyncSession, +) -> None: + """Deleting external identity cascades only mappings; user threads/issues remain intact.""" + owner, thread, issue = await _owned_issue( + async_db, + username="external_identity_delete_safety", + title="B.P.R.D.: War on Frogs", + issue_number="Revival", + ) + identity = await upsert_external_identity( + async_db, provider="comicvine", entity_type="issue", external_id="4000-revival" + ) + await link_issue_external_identity( + async_db, + user_id=owner.id, + issue_id=issue.id, + external_identity_id=identity.id, + status="confirmed", + ) + identity_id = identity.id + + await async_db.delete(identity) + await async_db.flush() + + assert await async_db.get(Thread, thread.id) is not None + assert await async_db.get(Issue, issue.id) is not None + assert await async_db.scalar( + select(func.count()).select_from(IssueExternalIdentityMapping).where( + IssueExternalIdentityMapping.external_identity_id == identity_id + ) + ) == 0