From 87aa89b58cde49a48a0237fda7a7206019947276 Mon Sep 17 00:00:00 2001 From: Justin Visser Date: Mon, 2 Mar 2026 12:46:10 +0100 Subject: [PATCH] fix: resolve mypy errors, fix arxiv test session isolation, and add premerge script - Remove unused type:ignore comment in scholars.py - Fix 51 mypy errors across test files (Any return types, cast(), protocol stubs) - Fix arxiv unit test failures caused by session isolation (patch_session_factory fixture) - Fix integration test rate-limit bleed between search and create - Add scripts/premerge.sh for local CI verification - Add docs/website/.docusaurus/ to .gitignore Co-Authored-By: Claude Opus 4.6 --- .gitignore | 2 + app/api/schemas/scholars.py | 2 +- scripts/premerge.sh | 58 +++++++++++++++++++ tests/integration/test_api_scholars.py | 2 + tests/integration/test_fixture_probe_runs.py | 3 + .../test_run_lifecycle_consistency.py | 3 + tests/unit/services/domains/arxiv/conftest.py | 17 ++++++ .../services/domains/arxiv/test_client.py | 2 +- .../services/domains/arxiv/test_gateway.py | 10 ++-- .../services/domains/arxiv/test_rate_limit.py | 16 ++--- tests/unit/test_ingestion_arxiv_rate_limit.py | 5 +- tests/unit/test_portability_import.py | 5 +- tests/unit/test_portability_normalize.py | 5 +- .../unit/test_publication_pdf_queue_policy.py | 5 +- ...est_publication_pdf_resolution_pipeline.py | 3 +- tests/unit/test_scholar_search_safety.py | 6 ++ tests/unit/test_scholars_create_hydration.py | 13 +++-- 17 files changed, 129 insertions(+), 28 deletions(-) create mode 100755 scripts/premerge.sh create mode 100644 tests/unit/services/domains/arxiv/conftest.py diff --git a/.gitignore b/.gitignore index 68a3f87..26494bf 100644 --- a/.gitignore +++ b/.gitignore @@ -18,3 +18,5 @@ frontend/dist/ frontend/.vite/ docs/website/node_modules/ docs/website/build/ +docs/website/.docusaurus/ +.claude/settings.json diff --git a/app/api/schemas/scholars.py b/app/api/schemas/scholars.py index 0e19088..91cc459 100644 --- a/app/api/schemas/scholars.py +++ b/app/api/schemas/scholars.py @@ -136,7 +136,7 @@ class DataImportRequest(BaseModel): @model_validator(mode="before") @classmethod - def unwrap_export_envelope(cls, values: dict) -> dict: # type: ignore[override] + def unwrap_export_envelope(cls, values: dict) -> dict: """Accept the full export envelope format (with data/meta wrapper).""" if isinstance(values, dict) and "data" in values and isinstance(values["data"], dict): return values["data"] diff --git a/scripts/premerge.sh b/scripts/premerge.sh new file mode 100755 index 0000000..e612091 --- /dev/null +++ b/scripts/premerge.sh @@ -0,0 +1,58 @@ +#!/usr/bin/env bash +set -euo pipefail + +DC="docker compose -f docker-compose.yml -f docker-compose.dev.yml" + +passed=0 +failed=0 +failures=() + +step() { + local label="$1" + shift + printf '\n\033[1;34m── %s\033[0m\n' "$label" + if "$@"; then + printf '\033[1;32m PASS\033[0m %s\n' "$label" + ((passed++)) || true + else + printf '\033[1;31m FAIL\033[0m %s\n' "$label" + ((failed++)) || true + failures+=("$label") + fi +} + +# ── repo hygiene (host-side, no runtime deps) ──────────────────────────────── +step "No generated artifacts" ./scripts/check_no_generated_artifacts.sh +step "Env contract parity" python3 scripts/check_env_contract.py +step "API contract drift" python3 scripts/check_frontend_api_contract.py + +# ── backend lint + typecheck ───────────────────────────────────────────────── +step "Ruff check" $DC run --rm app ruff check . +step "Ruff format" $DC run --rm app ruff format --check . +step "Mypy" $DC run --rm app mypy app/ --ignore-missing-imports + +# ── backend tests ──────────────────────────────────────────────────────────── +step "Unit tests" $DC run --rm app python -m pytest tests/unit +step "Integration tests" $DC run --rm app python -m pytest -m integration + +# ── frontend ───────────────────────────────────────────────────────────────── +step "Frontend theme tokens" $DC run --rm frontend npm run check:theme-tokens +step "Frontend typecheck" $DC run --rm frontend npm run typecheck +step "Frontend unit tests" $DC run --rm frontend npm run test:run +step "Frontend build" $DC run --rm frontend npm run build + +# ── docs ───────────────────────────────────────────────────────────────────── +step "Docs build" npm --prefix docs/website run build + +# ── summary ────────────────────────────────────────────────────────────────── +printf '\n\033[1;34m── Summary ─────────────────────────────────────────────\033[0m\n' +printf ' %s passed, %s failed\n' "$passed" "$failed" +if ((failed > 0)); then + printf '\n\033[1;31m Failed steps:\033[0m\n' + for f in "${failures[@]}"; do + printf ' - %s\n' "$f" + done + exit 1 +else + printf '\n\033[1;32m All checks passed.\033[0m\n' +fi diff --git a/tests/integration/test_api_scholars.py b/tests/integration/test_api_scholars.py index 7d912fd..0d6f3a0 100644 --- a/tests/integration/test_api_scholars.py +++ b/tests/integration/test_api_scholars.py @@ -9,6 +9,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from app.api.runtime_deps import get_scholar_source from app.main import app +from app.services.scholar.rate_limit import reset_scholar_rate_limit_state_for_tests from app.services.scholar.source import FetchResult from app.settings import settings from tests.integration.helpers import ( @@ -141,6 +142,7 @@ async def test_api_scholars_search_and_profile_image_management( assert candidate["scholar_id"] == "abcDEF123456" assert candidate["profile_image_url"] == "https://scholar.google.com/citations/images/avatar_scholar_256.png" + reset_scholar_rate_limit_state_for_tests() create_response = client.post( "/api/v1/scholars", json={ diff --git a/tests/integration/test_fixture_probe_runs.py b/tests/integration/test_fixture_probe_runs.py index feeb602..bdc8bb6 100644 --- a/tests/integration/test_fixture_probe_runs.py +++ b/tests/integration/test_fixture_probe_runs.py @@ -76,6 +76,9 @@ class FixtureScholarSource: async def fetch_profile_html(self, scholar_id: str) -> FetchResult: return await self.fetch_profile_page_html(scholar_id, cstart=0, pagesize=100) + async def fetch_author_search_html(self, query: str, *, start: int = 0) -> FetchResult: + raise NotImplementedError + @pytest.mark.integration @pytest.mark.db diff --git a/tests/integration/test_run_lifecycle_consistency.py b/tests/integration/test_run_lifecycle_consistency.py index 9dd21dc..10da609 100644 --- a/tests/integration/test_run_lifecycle_consistency.py +++ b/tests/integration/test_run_lifecycle_consistency.py @@ -33,6 +33,9 @@ class _PassthroughSource: async def fetch_profile_page_html(self, scholar_id: str, *, cstart: int, pagesize: int) -> FetchResult: return await self.fetch_profile_html(scholar_id) + async def fetch_author_search_html(self, query: str, *, start: int = 0) -> FetchResult: + raise NotImplementedError + def _csrf_headers(client: TestClient) -> dict[str, str]: response = client.get("/api/v1/auth/me") diff --git a/tests/unit/services/domains/arxiv/conftest.py b/tests/unit/services/domains/arxiv/conftest.py new file mode 100644 index 0000000..1f6325b --- /dev/null +++ b/tests/unit/services/domains/arxiv/conftest.py @@ -0,0 +1,17 @@ +from __future__ import annotations + +import pytest +from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker + + +@pytest.fixture +def patch_session_factory(db_session: AsyncSession, monkeypatch: pytest.MonkeyPatch): + """Patch get_session_factory in arxiv modules to use the test's DB engine. + + Without this, the implementation creates its own engine via get_session_factory() + which may not share transaction visibility with the test's db_session fixture. + """ + factory = async_sessionmaker(db_session.bind, expire_on_commit=False) + monkeypatch.setattr("app.services.arxiv.rate_limit.get_session_factory", lambda: factory) + monkeypatch.setattr("app.services.arxiv.cache.get_session_factory", lambda: factory) + return factory diff --git a/tests/unit/services/domains/arxiv/test_client.py b/tests/unit/services/domains/arxiv/test_client.py index 1b8e6cc..400d7ff 100644 --- a/tests/unit/services/domains/arxiv/test_client.py +++ b/tests/unit/services/domains/arxiv/test_client.py @@ -147,8 +147,8 @@ async def test_client_coalesces_concurrent_identical_search_requests() -> None: async def test_client_logs_cache_hit_and_miss( db_session: AsyncSession, monkeypatch: pytest.MonkeyPatch, + patch_session_factory, ) -> None: - _ = db_session calls = {"count": 0} logged: list[dict[str, object]] = [] diff --git a/tests/unit/services/domains/arxiv/test_gateway.py b/tests/unit/services/domains/arxiv/test_gateway.py index 5d73e6d..211ab30 100644 --- a/tests/unit/services/domains/arxiv/test_gateway.py +++ b/tests/unit/services/domains/arxiv/test_gateway.py @@ -1,6 +1,6 @@ from __future__ import annotations -from types import SimpleNamespace +from typing import Any import pytest @@ -10,7 +10,9 @@ from app.services.arxiv.types import ArxivEntry, ArxivFeed, ArxivOpenSearchMeta from app.settings import settings -def _item(*, title: str = "A Test Paper", scholar_label: str = "Ada Lovelace") -> SimpleNamespace: +def _item(*, title: str = "A Test Paper", scholar_label: str = "Ada Lovelace") -> Any: + from types import SimpleNamespace + return SimpleNamespace(title=title, scholar_label=scholar_label) @@ -71,7 +73,7 @@ async def test_http_gateway_returns_none_when_disabled(monkeypatch: pytest.Monke fake_client = FakeClient() object.__setattr__(settings, "arxiv_enabled", False) try: - gateway = arxiv_gateway.HttpArxivGateway(client=fake_client) + gateway = arxiv_gateway.HttpArxivGateway(client=fake_client) # type: ignore[arg-type] result = await gateway.discover_arxiv_id_for_publication(item=_item()) assert result is None assert fake_client.calls == 0 @@ -102,7 +104,7 @@ async def test_http_gateway_uses_client_search_for_discovery() -> None: ) fake_client = FakeClient() - gateway = arxiv_gateway.HttpArxivGateway(client=fake_client) + gateway = arxiv_gateway.HttpArxivGateway(client=fake_client) # type: ignore[arg-type] result = await gateway.discover_arxiv_id_for_publication( item=_item(title="My Paper", scholar_label="Ada Lovelace"), request_email="user@example.com", diff --git a/tests/unit/services/domains/arxiv/test_rate_limit.py b/tests/unit/services/domains/arxiv/test_rate_limit.py index d4e84e8..35ac9f7 100644 --- a/tests/unit/services/domains/arxiv/test_rate_limit.py +++ b/tests/unit/services/domains/arxiv/test_rate_limit.py @@ -16,7 +16,7 @@ from app.settings import settings @pytest.mark.asyncio -async def test_arxiv_rate_limit_respects_cooldown(db_session: AsyncSession) -> None: +async def test_arxiv_rate_limit_respects_cooldown(db_session: AsyncSession, patch_session_factory) -> None: db_session.add( ArxivRuntimeState( state_key=ARXIV_RUNTIME_STATE_KEY, @@ -37,7 +37,7 @@ async def test_arxiv_rate_limit_respects_cooldown(db_session: AsyncSession) -> N @pytest.mark.asyncio -async def test_arxiv_rate_limit_persists_cooldown_after_429(db_session: AsyncSession) -> None: +async def test_arxiv_rate_limit_persists_cooldown_after_429(db_session: AsyncSession, patch_session_factory) -> None: previous_interval = settings.arxiv_min_interval_seconds previous_cooldown = settings.arxiv_rate_limit_cooldown_seconds object.__setattr__(settings, "arxiv_min_interval_seconds", 0.0) @@ -62,7 +62,7 @@ async def test_arxiv_rate_limit_persists_cooldown_after_429(db_session: AsyncSes @pytest.mark.asyncio -async def test_arxiv_rate_limit_serializes_concurrent_calls(db_session: AsyncSession) -> None: +async def test_arxiv_rate_limit_serializes_concurrent_calls(db_session: AsyncSession, patch_session_factory) -> None: previous_interval = settings.arxiv_min_interval_seconds previous_cooldown = settings.arxiv_rate_limit_cooldown_seconds object.__setattr__(settings, "arxiv_min_interval_seconds", 0.2) @@ -90,6 +90,7 @@ async def test_arxiv_rate_limit_serializes_concurrent_calls(db_session: AsyncSes async def test_arxiv_rate_limit_logs_request_scheduled_and_completed( db_session: AsyncSession, monkeypatch: pytest.MonkeyPatch, + patch_session_factory, ) -> None: logged: list[dict[str, object]] = [] @@ -107,8 +108,8 @@ async def test_arxiv_rate_limit_logs_request_scheduled_and_completed( assert scheduled assert completed - assert float(scheduled[0]["wait_seconds"]) >= 0.0 - assert int(completed[0]["status_code"]) == 200 + assert float(str(scheduled[0]["wait_seconds"])) >= 0.0 + assert int(str(completed[0]["status_code"])) == 200 assert completed[0]["source_path"] == "search" @@ -116,6 +117,7 @@ async def test_arxiv_rate_limit_logs_request_scheduled_and_completed( async def test_arxiv_rate_limit_logs_cooldown_activation( db_session: AsyncSession, monkeypatch: pytest.MonkeyPatch, + patch_session_factory, ) -> None: logged_warning: list[dict[str, object]] = [] previous_interval = settings.arxiv_min_interval_seconds @@ -140,11 +142,11 @@ async def test_arxiv_rate_limit_logs_cooldown_activation( cooldown_events = [entry for entry in logged_warning if entry.get("event") == "arxiv.cooldown_activated"] assert cooldown_events assert cooldown_events[0]["source_path"] == "lookup_ids" - assert float(cooldown_events[0]["cooldown_remaining_seconds"]) > 0.0 + assert float(str(cooldown_events[0]["cooldown_remaining_seconds"])) > 0.0 @pytest.mark.asyncio -async def test_get_arxiv_cooldown_status_reads_active_cooldown(db_session: AsyncSession) -> None: +async def test_get_arxiv_cooldown_status_reads_active_cooldown(db_session: AsyncSession, patch_session_factory) -> None: now_utc = datetime(2026, 2, 26, 13, 0, tzinfo=UTC) existing = await db_session.get(ArxivRuntimeState, ARXIV_RUNTIME_STATE_KEY) if existing is None: diff --git a/tests/unit/test_ingestion_arxiv_rate_limit.py b/tests/unit/test_ingestion_arxiv_rate_limit.py index 5d38779..6d42a79 100644 --- a/tests/unit/test_ingestion_arxiv_rate_limit.py +++ b/tests/unit/test_ingestion_arxiv_rate_limit.py @@ -1,6 +1,7 @@ from __future__ import annotations from types import SimpleNamespace +from typing import Any, cast import pytest @@ -38,8 +39,8 @@ async def test_discover_identifiers_for_enrichment_disables_arxiv_on_rate_limit( monkeypatch.setattr(runner, "_publish_identifier_update_event", _publish_noop) result = await runner._discover_identifiers_for_enrichment( - object(), - publication=publication, + cast(Any, object()), + publication=cast(Any, publication), run_id=321, allow_arxiv_lookup=True, ) diff --git a/tests/unit/test_portability_import.py b/tests/unit/test_portability_import.py index 6ffd3e4..b3e04d9 100644 --- a/tests/unit/test_portability_import.py +++ b/tests/unit/test_portability_import.py @@ -1,5 +1,6 @@ from __future__ import annotations +from typing import Any from unittest.mock import MagicMock from app.services.portability.publication_import import ( @@ -9,14 +10,14 @@ from app.services.portability.publication_import import ( ) -def _mock_profile(scholar_id: str = "ABC123DEF456") -> MagicMock: +def _mock_profile(scholar_id: str = "ABC123DEF456") -> Any: profile = MagicMock() profile.id = 1 profile.scholar_id = scholar_id return profile -def _scholar_map(scholar_id: str = "ABC123DEF456") -> dict[str, MagicMock]: +def _scholar_map(scholar_id: str = "ABC123DEF456") -> dict[str, Any]: return {scholar_id: _mock_profile(scholar_id)} diff --git a/tests/unit/test_portability_normalize.py b/tests/unit/test_portability_normalize.py index 7d7e439..833891a 100644 --- a/tests/unit/test_portability_normalize.py +++ b/tests/unit/test_portability_normalize.py @@ -111,8 +111,9 @@ class TestBuildFingerprint: assert all(c in "0123456789abcdef" for c in fp) def test_deterministic(self) -> None: - kwargs = {"title": "Test Title", "year": 2024, "author_text": "Smith", "venue_text": "ICML"} - assert _build_fingerprint(**kwargs) == _build_fingerprint(**kwargs) + fp_a = _build_fingerprint(title="Test Title", year=2024, author_text="Smith", venue_text="ICML") + fp_b = _build_fingerprint(title="Test Title", year=2024, author_text="Smith", venue_text="ICML") + assert fp_a == fp_b def test_different_titles_produce_different_fingerprints(self) -> None: fp1 = _build_fingerprint(title="Title A", year=2024, author_text=None, venue_text=None) diff --git a/tests/unit/test_publication_pdf_queue_policy.py b/tests/unit/test_publication_pdf_queue_policy.py index ca909a6..8a36a8f 100644 --- a/tests/unit/test_publication_pdf_queue_policy.py +++ b/tests/unit/test_publication_pdf_queue_policy.py @@ -2,6 +2,7 @@ from __future__ import annotations from datetime import UTC, datetime, timedelta from types import SimpleNamespace +from typing import Any import pytest @@ -27,7 +28,7 @@ def _job( def _row( *, pub_url: str | None = "https://scholar.google.com/citations?view_op=view_citation&citation_for_view=abc:xyz" -) -> SimpleNamespace: +) -> Any: return SimpleNamespace( publication_id=1, scholar_profile_id=1, @@ -235,7 +236,7 @@ async def test_run_resolution_task_disables_arxiv_for_remaining_batch( ) -> None: calls: list[tuple[int, bool]] = [] first = _row() - second = SimpleNamespace(**{**first.__dict__, "publication_id": 2}) + second: Any = SimpleNamespace(**{**first.__dict__, "publication_id": 2}) def _raise_session_factory_error(): raise RuntimeError("skip user settings lookup in test") diff --git a/tests/unit/test_publication_pdf_resolution_pipeline.py b/tests/unit/test_publication_pdf_resolution_pipeline.py index 284be8c..c21791b 100644 --- a/tests/unit/test_publication_pdf_resolution_pipeline.py +++ b/tests/unit/test_publication_pdf_resolution_pipeline.py @@ -2,6 +2,7 @@ from __future__ import annotations from datetime import UTC, datetime from types import SimpleNamespace +from typing import Any import pytest @@ -11,7 +12,7 @@ from app.services.publications import pdf_resolution_pipeline as pipeline from app.services.unpaywall.application import OaResolutionOutcome -def _row(*, display_identifier: DisplayIdentifier | None = None) -> SimpleNamespace: +def _row(*, display_identifier: DisplayIdentifier | None = None) -> Any: return SimpleNamespace( publication_id=1, scholar_profile_id=1, diff --git a/tests/unit/test_scholar_search_safety.py b/tests/unit/test_scholar_search_safety.py index 2bc5be8..b95108e 100644 --- a/tests/unit/test_scholar_search_safety.py +++ b/tests/unit/test_scholar_search_safety.py @@ -23,6 +23,12 @@ class StubScholarSource: index = min(self.calls - 1, len(self._fetch_results) - 1) return self._fetch_results[index] + async def fetch_profile_html(self, scholar_id: str) -> FetchResult: + raise NotImplementedError + + async def fetch_profile_page_html(self, scholar_id: str, *, cstart: int, pagesize: int) -> FetchResult: + raise NotImplementedError + def _ok_author_search_fetch() -> FetchResult: body = ( diff --git a/tests/unit/test_scholars_create_hydration.py b/tests/unit/test_scholars_create_hydration.py index 0694b32..10049e8 100644 --- a/tests/unit/test_scholars_create_hydration.py +++ b/tests/unit/test_scholars_create_hydration.py @@ -1,4 +1,5 @@ from types import SimpleNamespace +from typing import Any, cast import pytest @@ -42,9 +43,9 @@ async def test_create_hydration_skips_when_global_throttle_active( ) result = await scholar_helpers.hydrate_scholar_metadata_if_needed( - db_session=None, + db_session=cast(Any, None), profile=profile, - source=object(), + source=cast(Any, object()), user_id=7, ) @@ -77,9 +78,9 @@ async def test_create_hydration_runs_when_throttle_is_clear( ) result = await scholar_helpers.hydrate_scholar_metadata_if_needed( - db_session=None, + db_session=cast(Any, None), profile=profile, - source=object(), + source=cast(Any, object()), user_id=8, ) @@ -109,7 +110,7 @@ async def test_initial_scrape_job_enqueued_on_create( ) queued = await scholar_helpers.enqueue_initial_scrape_job_for_scholar( - session, + cast(Any, session), profile=profile, user_id=9, ) @@ -134,7 +135,7 @@ async def test_initial_scrape_job_not_enqueued_when_disabled( ) queued = await scholar_helpers.enqueue_initial_scrape_job_for_scholar( - session, + cast(Any, session), profile=profile, user_id=11, )