diff --git a/CHANGELOG.md b/CHANGELOG.md index 8745b1c..015c7d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ Add an entry here when the project meaningfully changes for users, for example: ## Unreleased +### Added + +- Added a built-in, read-only Zotero Local API integration with exact DOI, arXiv, title, and item-key matching, safe local PDF attachment discovery, `auto` / `off` / `required` resolution policies, and environment diagnostics. + ### Changed - Converted the repository to a dual-stack Claude Code and Codex plugin layout with the canonical skill under `skills/deeppapernote/`. diff --git a/README.md b/README.md index 0417c9d..0423595 100644 --- a/README.md +++ b/README.md @@ -268,14 +268,30 @@ Domain routing is controlled by the editable taxonomy in `skills/deeppapernote/r ### Optional: Zotero for local-library-first workflows DeepPaperNote can work without Zotero. -But if you want the agent to search your local Zotero library first, you should expose a Zotero integration that your agent runtime can actually use. +If you already keep papers in Zotero, DeepPaperNote now has a built-in, read-only integration with the desktop [Zotero Local API](https://www.zotero.org/support/dev/web_api/v3/local_api). It queries only the loopback API at `127.0.0.1:23119`, requires no API key or third-party Python package, and never reads the Zotero SQLite database directly. This is most worth setting up if you already use Zotero as your main paper-management or reading workflow. -Recommended ways to think about it: +To enable it: + +1. Keep the Zotero desktop app running. +2. In Zotero, enable **Settings → Advanced → Allow other applications on this computer to communicate with Zotero**. + +DeepPaperNote probes the Local API automatically during paper resolution. Its `--zotero-mode` policy has three options: + +| Mode | Behavior | +| --- | --- | +| `auto` (default) | Prefer a unique local item; retain web fallback for title, DOI, and arXiv queries when Zotero is unavailable or has no match | +| `off` | Make no Zotero Local API request and preserve the previous resolution path | +| `required` | Stop unless Zotero uniquely resolves the reference; a missing local PDF does not invalidate confirmed Zotero metadata | + +An ambiguous local match always fails closed instead of choosing an arbitrary item. An explicit Zotero key also fails closed when it cannot be verified locally because it has no safe web fallback. Explicit local PDFs and trusted JSON artifacts bypass the lookup in every mode. + +Compatible agent-runtime or MCP integrations remain optional alternatives: | 🧩 Option | 🎯 Best for | 📝 Notes | | --- | --- | --- | +| Built-in Local API | Normal local-library-first use | Recommended path; read-only, offline-capable, no API key, and no extra integration layer | | [kujenga/zotero-mcp](https://github.com/kujenga/zotero-mcp) | Lightweight read access | Closer to a minimal Zotero MCP server for search, metadata, and text access, but it usually still needs some adaptation for your agent runtime | | [54yyyu/zotero-mcp](https://github.com/54yyyu/zotero-mcp) | Richer research workflow features | More feature-rich, but stable use usually still requires some integration work on your side | @@ -290,9 +306,10 @@ Why it matters: Important note: -- DeepPaperNote does **not** require one specific Zotero integration -- for DeepPaperNote, the key capability is that the agent can search Zotero items, inspect metadata, and ideally read local PDF attachments -- the two routes above are **not** always plug-and-play, so stable use may still require some adaptation on your side +- the built-in client sends only read-only `GET` requests and does not expose or forward Zotero's unauthenticated loopback port +- the built-in client currently targets the logged-in user's personal library; group-library select links are rejected instead of silently querying the wrong library +- no local PDF is required for a successful Zotero identity match; DeepPaperNote can keep the confirmed metadata and use its normal PDF acquisition fallback +- third-party runtime integrations are **not** always plug-and-play and are not required by the built-in workflow ### Optional: Semantic Scholar API key @@ -394,7 +411,7 @@ For release-level updates, see [CHANGELOG.md](./CHANGELOG.md). | v0.3.0-alpha | ✅ Released | Major quality upgrade: dedicated innovation section, explicit mechanism flow, stronger workflow discipline, final readability review, math syntax gate, and the new `Original Abstract Translation` front-matter block | | v0.2.0-alpha | ✅ Released | Replication-oriented note-writing upgrade: explicit `note_plan`, equation-aware output, stricter final self-review, bilingual abstract handling, and stronger formatting checks | | v0.1.0-alpha | ✅ Released | First public alpha: evidence-bundle workflow, Zotero-first helpers, placeholder-first figure handling, workspace fallback, OCR fallback, tests, and CI | -| Unreleased | 🕒 No new release-level changes yet | There are currently no additional public release notes beyond v2.0.0 | +| Unreleased | 🛠️ In development | Built-in read-only Zotero Local API resolution, safe attachment reuse, explicit lookup policies, and environment diagnostics | ## ⚙️ Workflow @@ -495,7 +512,7 @@ DeepPaperNote/ | Python 3.10+ | Required | Runs the helper scripts | | PyMuPDF | Required | Core PDF dependency; install it with `python3 -m pip install PyMuPDF` | | Local Obsidian vault | Recommended | Writes directly into a long-term note system; otherwise uses the current workspace's fallback output folder | -| Zotero integration | Optional | Helpful for local-library-first paper workflows | +| Zotero Local API / compatible integration | Optional | Reuses local metadata and PDF attachments before web resolution | | OCR tools | Optional | Improves handling of scanned PDFs | ## 🧭 Design Principles diff --git a/README.zh-CN.md b/README.zh-CN.md index f743cf1..f21c16f 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -263,16 +263,32 @@ setx DEEPPAPERNOTE_PAPERS_DIR "Research/Papers" ### 可选:用于本地文献库优先工作流的 Zotero DeepPaperNote 不依赖 Zotero 才能工作。 -但如果你本来就用 Zotero 做文献管理,配置一个你的 agent 真的能用的 Zotero 集成会很值。 +如果你本来就在 Zotero 里管理论文,DeepPaperNote 现在可以直接使用桌面端的只读 [Zotero Local API](https://www.zotero.org/support/dev/web_api/v3/local_api)。它只访问本机回环地址 `127.0.0.1:23119`,不需要 API key 或额外 Python 包,也不会直接读取 Zotero 的 SQLite 数据库。 它最适合这样的人: - 你本来就用 Zotero 做文献管理 - 你平时主要在 Zotero 里读论文、整理附件和元数据 -可以这样理解不同路线: +启用方法: + +1. 保持 Zotero 桌面应用运行。 +2. 在 Zotero 中启用 **设置 → 高级 → 允许此计算机上的其他应用程序与 Zotero 通信**。 + +DeepPaperNote 会在论文解析时自动探测 Local API。其 `--zotero-mode` 策略有三个选项: + +| 模式 | 行为 | +| --- | --- | +| `auto`(默认) | 优先采用唯一的本地条目;标题、DOI 和 arXiv 查询在 Zotero 不可用或未命中时保留联网回退 | +| `off` | 不请求 Zotero Local API,完整保留以前的解析路径 | +| `required` | 只有 Zotero 唯一解析出论文时才继续;没有本地 PDF 不会否定已经确认的 Zotero 元数据 | + +如果本地出现多个同样可信的候选,解析会明确失败,而不是随意选一个。明确给出的 Zotero key 也必须在本地验证;它没有安全的联网回退。用户明确提供的本地 PDF 和可信 JSON artifact 在所有模式下都会直接使用。 + +agent runtime 或 MCP 提供的兼容集成仍可作为可选替代路线: | 🧩 方案 | 🎯 更适合什么场景 | 📝 说明 | | --- | --- | --- | +| 内置 Local API | 常规的本地文献库优先工作流 | 推荐路线;只读、可离线工作、不需要 API key,也不需要额外集成层 | | [kujenga/zotero-mcp](https://github.com/kujenga/zotero-mcp) | 轻量的只读访问 | 更接近一个最小化 Zotero MCP 服务,适合搜索条目、读元数据、读文本,但通常仍需要你自己做一点适配 | | [54yyyu/zotero-mcp](https://github.com/54yyyu/zotero-mcp) | 更完整的研究工作流能力 | 功能更丰富,但稳定接进你的 agent 环境时通常也需要额外改造 | @@ -287,9 +303,10 @@ DeepPaperNote 不依赖 Zotero 才能工作。 ⚠️需要特别说明的是: -- DeepPaperNote **不强依赖某一个固定的 Zotero 集成仓库** -- 对 DeepPaperNote 来说,需要的关键能力是:让 agent 能搜索 Zotero 条目、查看元数据、最好还能读取本地 PDF 附件 -- 上面提到的两条路线目前都**不一定是即插即用方案**,如果你想稳定使用,通常还需要自己做一层适配或改造 +- 内置客户端只发送只读 `GET` 请求,不会暴露或转发 Zotero 无认证的本机回环端口 +- 内置客户端目前只查询已登录用户的个人文献库;遇到 group library 的选择链接会明确拒绝,不会静默查询错误的库 +- Zotero 身份命中不要求一定存在本地 PDF;DeepPaperNote 可以保留确认过的元数据,再走正常的 PDF 获取回退 +- 第三方 runtime 集成**不一定即插即用**,内置工作流也不依赖它们 ### 可选:Semantic Scholar API Key @@ -385,7 +402,7 @@ python3 -c "import pytesseract; print(pytesseract.get_tesseract_version())" | v0.3.0-alpha | ✅ 已发布 | 一次较大的质量升级:新增固定创新点章节、显式机制流程、更强的整条 workflow 约束、最终可读性质检、公式语法检查,以及新的 `原文摘要翻译` 前置区块 | | v0.2.0-alpha | ✅ 已发布 | 复现级技术笔记写作升级:显式 `note_plan`、公式感知输出、更强的最终自检、摘要中英双写,以及更严格的格式校验 | | v0.1.0-alpha | ✅ 已发布 | 第一个公开 alpha 版:综合证据包流程、Zotero 优先辅助能力、占位优先图表处理、工作区回退输出、OCR 回退、测试与 CI | -| 未发布 | 🕒 暂无新的 release 级变化 | 当前还没有下一版 release 的公开更新内容,最新版本为 v2.0.0 | +| 未发布 | 🛠️ 开发中 | 内置只读 Zotero Local API 解析、安全复用本地附件、显式查询策略和环境诊断 | ## ⚙️ 工作流 @@ -485,7 +502,7 @@ DeepPaperNote/ | Python 3.10+ | 必需 | 运行辅助脚本 | | PyMuPDF | 必需 | 核心 PDF 依赖,可用 `python3 -m pip install PyMuPDF` 安装 | | 本地 Obsidian 库 | 推荐 | 配好后可直接写入长期笔记体系;未配置时使用当前工作区下的回退输出目录 | -| Zotero 集成 | 可选 | 对本地论文库工作流很有帮助 | +| Zotero Local API / 兼容集成 | 可选 | 联网解析前优先复用本地元数据和 PDF 附件 | | OCR 工具 | 可选 | 对扫描版 PDF 更友好 | ## 🧭 设计原则 diff --git a/skills/deeppapernote/SKILL.md b/skills/deeppapernote/SKILL.md index d65dadc..404283d 100644 --- a/skills/deeppapernote/SKILL.md +++ b/skills/deeppapernote/SKILL.md @@ -116,14 +116,15 @@ Prefer the strongest available source in this order: 4. arXiv or open-access PDF sources 5. Semantic Scholar or OpenAlex for metadata backfill -Before resolving the paper, actively check Zotero integration: attempt to call the Zotero MCP tool (for example, search for the paper title or list libraries). If the tool responds without error, Zotero is available and the local-library-first rule below applies. If the call fails or the tool is not present, record "Zotero not available" and proceed without it. Do not skip this check — the check itself determines whether local-library-first applies. +Before web resolution, use the bundled `scripts/resolve_paper.py` Zotero Local API path to check the desktop library. Its default `--zotero-mode auto` prefers a unique local match and falls back to the existing providers when Zotero is unavailable or has no match. An explicit Zotero key has no safe web fallback and must be verified locally. Use `off` to make no Local API request, or `required` when the reference must resolve through Zotero. A trusted JSON artifact or explicit local PDF remains authoritative and bypasses this lookup. A compatible session-scoped Zotero/MCP integration may still provide a trusted input artifact when available, but it is not required for the built-in path. -Local-library-first rule (applies only when the Zotero check above succeeds): -- search the local Zotero library first using the paper title, DOI, or arXiv id +Local-library-first rule: +- search the local Zotero library first using the paper title, DOI, arXiv id, or exact Zotero item key - If Zotero finds the paper, treat that result as the canonical identity resolution step. -- If the attachment path is not exposed by the integration, use `scripts/locate_zotero_attachment.py` with the attachment key and filename to find the local PDF under the user's Zotero storage. +- Prefer the safe local attachment path returned by the built-in Local API. If another compatible integration exposes only an attachment key and filename, use `scripts/locate_zotero_attachment.py` to find the PDF under the user's Zotero storage. - If a local attachment path is available, pass it forward as the preferred PDF source. - If no local attachment is found, still use the library-resolved metadata to avoid title ambiguity, then fall back to network PDF acquisition only for the file itself. +- If multiple local items are equally plausible, fail closed and request a DOI, arXiv id, or exact Zotero key rather than selecting one arbitrarily. - Do not let a weaker title-only internet match override a confident local-library hit. ## Output Rules diff --git a/skills/deeppapernote/references/metadata-sources.md b/skills/deeppapernote/references/metadata-sources.md index 8c46a40..b6ba84a 100644 --- a/skills/deeppapernote/references/metadata-sources.md +++ b/skills/deeppapernote/references/metadata-sources.md @@ -32,8 +32,10 @@ Use the strongest available source first, but backfill aggressively. ## Rules -- If the paper is already in the local Zotero library, treat Zotero as the identity anchor before doing title-based web resolution. +- Use the bundled read-only Zotero Local API client first when the desktop API is enabled; compatible runtime or MCP integrations may provide the same trusted local metadata as an optional alternative. +- If the paper is already in the local Zotero library, treat a unique Zotero match as the identity anchor before doing title-based web resolution. - If Zotero resolves the paper but does not expose a local attachment path, still use the Zotero metadata to avoid title ambiguity. +- Distinguish an ambiguous local identity from an unavailable attachment: never guess between equally plausible items, but keep a uniquely resolved parent item's metadata even when no local PDF is accessible. - Do not let a weaker internet title match override a confident Zotero hit. - Do not invent missing metadata. - If a Chinese title is assistant-generated, mark it as a translation. diff --git a/skills/deeppapernote/scripts/_zotero_local.py b/skills/deeppapernote/scripts/_zotero_local.py new file mode 100644 index 0000000..6b8643f --- /dev/null +++ b/skills/deeppapernote/scripts/_zotero_local.py @@ -0,0 +1,856 @@ +#!/usr/bin/env python3 +"""Read-only client and matching helpers for Zotero's desktop Local API.""" + +from __future__ import annotations + +import json +import os +import re +import socket +import unicodedata +import urllib.error +import urllib.parse +import urllib.request +from dataclasses import dataclass +from pathlib import Path, PurePosixPath, PureWindowsPath +from typing import Any, Callable, Mapping + +from common import ( + DEFAULT_USER_AGENT, + apply_identity_confidence, + extract_arxiv_id, + extract_doi, + infer_source_type, + normalize_whitespace, + paper_id_for_record, +) + +ZOTERO_LOCAL_BASE_URL = "http://127.0.0.1:23119/api" +ZOTERO_API_VERSION = "3" +DEFAULT_TIMEOUT_SECONDS = 1.5 +MAX_JSON_RESPONSE_BYTES = 2 * 1024 * 1024 +MAX_FILE_URL_BYTES = 16 * 1024 +ZOTERO_ITEM_KEY_PATTERN = re.compile(r"^[A-Z0-9]{8}$") +ZOTERO_SELECT_KEY_PATTERN = re.compile( + r"^zotero://select/library/items/([A-Z0-9]{8})$", + flags=re.IGNORECASE, +) +ZOTERO_GROUP_SELECT_PATTERN = re.compile( + r"^zotero://select/groups/\d+/items/[A-Z0-9]{8}$", + flags=re.IGNORECASE, +) + + +@dataclass(frozen=True) +class HttpResult: + """Small transport-neutral HTTP response used by tests and the stdlib client.""" + + status: int + headers: Mapping[str, str] + body: bytes + + +class ZoteroLocalError(RuntimeError): + """Stable error contract for Local API failures.""" + + def __init__( + self, + code: str, + message: str, + *, + http_status: int | None = None, + retryable: bool = False, + ) -> None: + super().__init__(message) + self.code = code + self.http_status = http_status + self.retryable = retryable + + def as_dict(self) -> dict[str, Any]: + payload: dict[str, Any] = { + "code": self.code, + "message": str(self), + "retryable": self.retryable, + } + if self.http_status is not None: + payload["http_status"] = self.http_status + return payload + + +Transport = Callable[[str, dict[str, str], float], HttpResult] + + +class _NoRedirectHandler(urllib.request.HTTPRedirectHandler): + """Keep every Local API transport request on the validated loopback endpoint.""" + + def redirect_request(self, req, fp, code, msg, headers, newurl): # type: ignore[no-untyped-def] + return None + + +def _normalized_headers(headers: Mapping[str, str]) -> dict[str, str]: + return {str(key).lower(): str(value) for key, value in headers.items()} + + +def _read_limited(response: Any, limit: int) -> bytes: + body = response.read(limit + 1) + if len(body) > limit: + raise ZoteroLocalError( + "zotero_invalid_response", + "Zotero Local API returned an unexpectedly large response.", + ) + return body + + +def _default_transport(url: str, headers: dict[str, str], timeout: float) -> HttpResult: + # Local library queries should never inherit a corporate/system HTTP proxy. + opener = urllib.request.build_opener( + urllib.request.ProxyHandler({}), + _NoRedirectHandler(), + ) + request = urllib.request.Request(url, headers=headers, method="GET") + try: + with opener.open(request, timeout=timeout) as response: + return HttpResult( + status=int(getattr(response, "status", 200)), + headers=_normalized_headers(response.headers), + body=_read_limited(response, MAX_JSON_RESPONSE_BYTES), + ) + except urllib.error.HTTPError as exc: + body = b"" + try: + body = _read_limited(exc, MAX_JSON_RESPONSE_BYTES) + except Exception: + pass + return HttpResult( + status=int(exc.code), + headers=_normalized_headers(exc.headers or {}), + body=body, + ) + except urllib.error.URLError as exc: + if isinstance(exc.reason, (socket.timeout, TimeoutError)): + raise ZoteroLocalError( + "zotero_timeout", + "Timed out while connecting to the Zotero Local API.", + retryable=True, + ) from exc + raise ZoteroLocalError( + "zotero_not_running", + "Could not connect to the Zotero Local API on this computer.", + retryable=True, + ) from exc + except (socket.timeout, TimeoutError) as exc: + raise ZoteroLocalError( + "zotero_timeout", + "Timed out while connecting to the Zotero Local API.", + retryable=True, + ) from exc + except OSError as exc: + raise ZoteroLocalError( + "zotero_not_running", + "Could not connect to the Zotero Local API on this computer.", + retryable=True, + ) from exc + + +def _validate_base_url(base_url: str) -> str: + parsed = urllib.parse.urlsplit(base_url.rstrip("/")) + if ( + parsed.scheme != "http" + or parsed.hostname not in {"127.0.0.1", "localhost", "::1"} + or parsed.username is not None + or parsed.password is not None + or parsed.query + or parsed.fragment + or parsed.path.rstrip("/") != "/api" + ): + raise ValueError("Zotero Local API base URL must be a loopback HTTP /api endpoint.") + return urllib.parse.urlunsplit((parsed.scheme, parsed.netloc, parsed.path.rstrip("/"), "", "")) + + +def _error_for_http_status(status: int) -> ZoteroLocalError: + if status == 400: + return ZoteroLocalError( + "zotero_bad_request", + "Zotero Local API rejected the request.", + http_status=status, + ) + if status == 403: + return ZoteroLocalError( + "zotero_api_disabled", + "Zotero Local API access is disabled in Zotero settings.", + http_status=status, + ) + if status == 404: + return ZoteroLocalError( + "zotero_item_not_found", + "The requested Zotero item was not found.", + http_status=status, + ) + if status == 501: + return ZoteroLocalError( + "zotero_unsupported_version", + "This Zotero installation does not support the requested Local API version.", + http_status=status, + ) + if status >= 500: + return ZoteroLocalError( + "zotero_server_error", + "Zotero Local API returned a server error.", + http_status=status, + retryable=True, + ) + return ZoteroLocalError( + "zotero_invalid_response", + f"Zotero Local API returned unexpected HTTP status {status}.", + http_status=status, + ) + + +class ZoteroLocalClient: + """Minimal read-only Zotero Local API v3 client.""" + + def __init__( + self, + *, + timeout: float = DEFAULT_TIMEOUT_SECONDS, + transport: Transport | None = None, + base_url: str = ZOTERO_LOCAL_BASE_URL, + ) -> None: + self.timeout = timeout + self.transport = transport or _default_transport + self.base_url = _validate_base_url(base_url) + self._probe_result: dict[str, Any] | None = None + + def _url(self, path: str = "", params: Mapping[str, Any] | None = None) -> str: + url = f"{self.base_url}/{path.lstrip('/')}" if path else f"{self.base_url}/" + if params: + url = f"{url}?{urllib.parse.urlencode(params)}" + return url + + def _request( + self, + path: str = "", + *, + params: Mapping[str, Any] | None = None, + accept: str = "application/json", + max_bytes: int = MAX_JSON_RESPONSE_BYTES, + ) -> HttpResult: + headers = { + "Accept": accept, + "User-Agent": DEFAULT_USER_AGENT, + "Zotero-API-Version": ZOTERO_API_VERSION, + } + result = self.transport(self._url(path, params), headers, self.timeout) + if not 200 <= result.status < 300: + raise _error_for_http_status(result.status) + if len(result.body) > max_bytes: + raise ZoteroLocalError( + "zotero_invalid_response", + "Zotero Local API returned an unexpectedly large response.", + ) + return result + + def probe(self) -> dict[str, Any]: + if self._probe_result is not None: + return dict(self._probe_result) + result = self._request() + headers = _normalized_headers(result.headers) + api_version = normalize_whitespace(headers.get("zotero-api-version", "")) + if api_version != ZOTERO_API_VERSION: + raise ZoteroLocalError( + "zotero_unsupported_version", + "Zotero Local API did not report compatible API version 3.", + ) + self._probe_result = { + "status": "available", + "reachable": True, + "ready": True, + "base_url": self.base_url, + "api_version": api_version, + "schema_version": normalize_whitespace(headers.get("zotero-schema-version", "")), + } + return dict(self._probe_result) + + def _ensure_ready(self) -> None: + if self._probe_result is None: + self.probe() + + def _request_json( + self, + path: str, + *, + params: Mapping[str, Any] | None = None, + ) -> Any: + self._ensure_ready() + result = self._request(path, params=params) + try: + return json.loads(result.body.decode("utf-8")) + except (UnicodeDecodeError, json.JSONDecodeError) as exc: + raise ZoteroLocalError( + "zotero_invalid_response", + "Zotero Local API returned invalid JSON.", + ) from exc + + def get_item(self, item_key: str) -> dict[str, Any]: + key = normalize_whitespace(item_key).upper() + if not ZOTERO_ITEM_KEY_PATTERN.fullmatch(key): + raise ValueError("Zotero item keys must contain exactly eight letters or digits.") + payload = self._request_json( + f"users/0/items/{key}", + params={"format": "json", "include": "data"}, + ) + if not isinstance(payload, dict): + raise ZoteroLocalError( + "zotero_invalid_response", + "Zotero item response was not a JSON object.", + ) + return payload + + def search_top_items( + self, + query: str, + *, + qmode: str, + ) -> list[dict[str, Any]]: + if qmode not in {"everything", "titleCreatorYear"}: + raise ValueError("Unsupported Zotero quick-search mode.") + payload = self._request_json( + "users/0/items/top", + params={ + "format": "json", + "include": "data", + "itemType": "-attachment", + "q": query, + "qmode": qmode, + }, + ) + if not isinstance(payload, list) or not all(isinstance(item, dict) for item in payload): + raise ZoteroLocalError( + "zotero_invalid_response", + "Zotero search response was not a JSON item list.", + ) + return payload + + def get_children(self, parent_key: str) -> list[dict[str, Any]]: + key = normalize_whitespace(parent_key).upper() + if not ZOTERO_ITEM_KEY_PATTERN.fullmatch(key): + raise ValueError("Zotero item keys must contain exactly eight letters or digits.") + payload = self._request_json( + f"users/0/items/{key}/children", + params={"format": "json", "include": "data"}, + ) + if not isinstance(payload, list) or not all(isinstance(item, dict) for item in payload): + raise ZoteroLocalError( + "zotero_invalid_response", + "Zotero children response was not a JSON item list.", + ) + return payload + + def get_attachment_file_url(self, attachment_key: str) -> str: + key = normalize_whitespace(attachment_key).upper() + if not ZOTERO_ITEM_KEY_PATTERN.fullmatch(key): + raise ValueError("Zotero item keys must contain exactly eight letters or digits.") + self._ensure_ready() + result = self._request( + f"users/0/items/{key}/file/view/url", + accept="text/plain", + max_bytes=MAX_FILE_URL_BYTES, + ) + try: + return result.body.decode("utf-8").strip() + except UnicodeDecodeError as exc: + raise ZoteroLocalError( + "zotero_invalid_response", + "Zotero attachment URL was not UTF-8 text.", + ) from exc + + +def _item_data(raw: Mapping[str, Any]) -> dict[str, Any]: + data = raw.get("data", {}) + return dict(data) if isinstance(data, dict) else {} + + +def zotero_item_key(raw: Mapping[str, Any]) -> str: + data = _item_data(raw) + return normalize_whitespace(str(raw.get("key") or data.get("key") or "")).upper() + + +def _creator_name(creator: Mapping[str, Any]) -> str: + corporate = normalize_whitespace(str(creator.get("name", ""))) + if corporate: + return corporate + return normalize_whitespace(f"{creator.get('firstName', '')} {creator.get('lastName', '')}") + + +def _publication_year(value: str) -> str: + match = re.search(r"(? dict[str, Any]: + """Map a Zotero item wrapper into DeepPaperNote's metadata vocabulary.""" + + data = _item_data(raw) + item_type = normalize_whitespace(str(data.get("itemType", ""))) + key = zotero_item_key(raw) + authors: list[str] = [] + for creator in data.get("creators", []) or []: + if not isinstance(creator, dict): + continue + if creator.get("creatorType") not in {"author", "bookAuthor"}: + continue + name = _creator_name(creator) + if name and name not in authors: + authors.append(name) + + doi = (extract_doi(str(data.get("DOI", ""))) or "").lower() + arxiv_text = " ".join( + str(data.get(field, "")) for field in ("archiveLocation", "extra", "url", "DOI") + ) + arxiv_id = extract_arxiv_id(arxiv_text) or "" + venue = next( + ( + normalize_whitespace(str(data.get(field, ""))) + for field in ( + "publicationTitle", + "conferenceName", + "proceedingsTitle", + "bookTitle", + "websiteTitle", + "repository", + "publisher", + ) + if normalize_whitespace(str(data.get(field, ""))) + ), + "", + ) + source_url = normalize_whitespace(str(data.get("url", ""))) + if urllib.parse.urlsplit(source_url).scheme.lower() not in {"http", "https"}: + source_url = "" + if not source_url and doi: + source_url = f"https://doi.org/{doi}" + + record: dict[str, Any] = { + "status": "ok", + "source_type": "zotero", + "metadata_sources": ["zotero"], + "zotero_key": key, + "zotero_item_type": item_type, + "title": normalize_whitespace(str(data.get("title", ""))), + "authors": authors, + "affiliations": [], + "abstract": normalize_whitespace(str(data.get("abstractNote", ""))), + "venue": venue, + "year": _publication_year(str(data.get("date", ""))), + "doi": doi, + "arxiv_id": arxiv_id, + "source_url": source_url, + } + version = raw.get("version") + if version not in (None, ""): + record["zotero_version"] = version + return record + + +def normalize_zotero_attachment(raw: Mapping[str, Any]) -> dict[str, Any]: + data = _item_data(raw) + return { + "key": zotero_item_key(raw), + "parent_key": normalize_whitespace(str(data.get("parentItem", ""))).upper(), + "item_type": normalize_whitespace(str(data.get("itemType", ""))), + "link_mode": normalize_whitespace(str(data.get("linkMode", ""))), + "title": normalize_whitespace(str(data.get("title", ""))), + "content_type": normalize_whitespace(str(data.get("contentType", ""))).lower(), + "filename": normalize_whitespace(str(data.get("filename", ""))), + "url": normalize_whitespace(str(data.get("url", ""))), + } + + +def is_pdf_attachment(attachment: Mapping[str, Any]) -> bool: + if attachment.get("item_type") != "attachment": + return False + if attachment.get("link_mode") == "linked_url": + return False + return attachment.get("content_type") == "application/pdf" or any( + str(attachment.get(field, "")).lower().endswith(".pdf") for field in ("filename", "title") + ) + + +def _split_attachment_url(raw_url: str) -> urllib.parse.SplitResult: + try: + parsed = urllib.parse.urlsplit(raw_url) + _ = parsed.port, parsed.hostname + except ValueError as exc: + raise ZoteroLocalError( + "zotero_unsafe_file_url", + "Zotero attachment URL contained an invalid network location.", + ) from exc + return parsed + + +def file_url_to_local_path(raw_url: str, *, platform: str | None = None) -> str: + """Convert a safe local ``file://`` URL without touching the filesystem.""" + + value = (raw_url or "").strip() + if not value or "\x00" in value: + raise ZoteroLocalError( + "zotero_unsafe_file_url", + "Zotero returned an empty or invalid attachment file URL.", + ) + parsed = _split_attachment_url(value) + port = parsed.port + if ( + parsed.scheme.lower() != "file" + or parsed.username is not None + or parsed.password is not None + or port is not None + or parsed.query + or parsed.fragment + or parsed.hostname not in {None, "", "localhost"} + ): + raise ZoteroLocalError( + "zotero_unsafe_file_url", + "Zotero attachment URL did not identify a local file.", + ) + try: + decoded = urllib.parse.unquote(parsed.path, encoding="utf-8", errors="strict") + except UnicodeDecodeError as exc: + raise ZoteroLocalError( + "zotero_unsafe_file_url", + "Zotero attachment URL contained invalid UTF-8 path data.", + ) from exc + if "\x00" in decoded: + raise ZoteroLocalError( + "zotero_unsafe_file_url", + "Zotero attachment URL contained an invalid path.", + ) + target_platform = platform or os.name + if target_platform == "nt": + windows_path = decoded.replace("/", "\\") + if windows_path.startswith("\\\\"): + raise ZoteroLocalError( + "zotero_unsafe_file_url", + "Zotero attachment URL identified a remote UNC path.", + ) + if re.match(r"^/[A-Za-z]:/", decoded): + decoded = decoded[1:] + path = PureWindowsPath(decoded.replace("/", "\\")) + else: + if decoded.startswith("//"): + raise ZoteroLocalError( + "zotero_unsafe_file_url", + "Zotero attachment URL identified an ambiguous network path.", + ) + path = PurePosixPath(decoded) + if not path.is_absolute(): + raise ZoteroLocalError( + "zotero_unsafe_file_url", + "Zotero attachment URL was not an absolute local path.", + ) + return str(path) + + +def existing_pdf_from_file_url(raw_url: str) -> str: + path_value = file_url_to_local_path(raw_url) + try: + path = Path(path_value).expanduser().resolve(strict=True) + except (OSError, RuntimeError): + return "" + if not path.is_file() or path.suffix.lower() != ".pdf": + return "" + return str(path) + + +def normalize_lookup_title(title: str) -> str: + normalized = unicodedata.normalize("NFKC", normalize_whitespace(title)).casefold() + normalized = "".join(char if char.isalnum() or char.isspace() else " " for char in normalized) + return normalize_whitespace(normalized) + + +def _match_search_results( + raw_items: list[dict[str, Any]], + *, + match_kind: str, + query: str, +) -> dict[str, Any]: + candidates = [ + normalize_zotero_item(item) + for item in raw_items + if _item_data(item).get("itemType") not in {"", "attachment", "note", "annotation"} + and ZOTERO_ITEM_KEY_PATTERN.fullmatch(zotero_item_key(item)) + ] + if match_kind == "doi": + expected = (extract_doi(query) or "").lower() + matches = [item for item in candidates if item.get("doi", "").lower() == expected] + elif match_kind == "arxiv_id": + expected = extract_arxiv_id(query) or "" + matches = [item for item in candidates if item.get("arxiv_id") == expected] + else: + expected_title = normalize_lookup_title(query) + matches = [ + item + for item in candidates + if normalize_lookup_title(str(item.get("title", ""))) == expected_title + ] + if not matches: + return {"status": "not_found", "match_kind": match_kind, "candidate_count": 0} + if len(matches) > 1: + return { + "status": "ambiguous", + "match_kind": match_kind, + "candidate_count": len(matches), + } + return {"status": "match", "match_kind": match_kind, "record": matches[0]} + + +def _attachment_priority(attachment: Mapping[str, Any]) -> tuple[int, int, int, int]: + label = f"{attachment.get('title', '')} {attachment.get('filename', '')}".casefold() + supplementary = any( + term in label + for term in ("supplementary", "supplemental", "supporting information", "appendix") + ) + link_mode_score = { + "imported_file": 3, + "linked_file": 2, + "imported_url": 1, + }.get(str(attachment.get("link_mode", "")), 0) + explicit_pdf = int(attachment.get("content_type") == "application/pdf") + full_text = int("full text" in label or "pdf" in label) + return (int(not supplementary), link_mode_score, explicit_pdf, full_text) + + +def _attach_local_pdf( + client: ZoteroLocalClient, + record: dict[str, Any], + raw_children: list[dict[str, Any]], + *, + preferred_attachment: Mapping[str, Any] | None = None, +) -> dict[str, Any]: + parent_key = str(record.get("zotero_key", "")) + raw_candidates = list(raw_children) + if preferred_attachment and zotero_item_key(preferred_attachment) not in { + zotero_item_key(item) for item in raw_candidates + }: + raw_candidates.append(dict(preferred_attachment)) + candidates = [normalize_zotero_attachment(item) for item in raw_candidates] + candidates = [ + item + for item in candidates + if is_pdf_attachment(item) + and item.get("parent_key") == parent_key + and ZOTERO_ITEM_KEY_PATTERN.fullmatch(str(item.get("key", ""))) + ] + preferred_key = zotero_item_key(preferred_attachment or {}) + if preferred_key: + candidates = [item for item in candidates if item.get("key") == preferred_key] + if not candidates: + record["zotero_attachment_status"] = "not_found" + return record + + local_candidates: list[tuple[dict[str, Any], str]] = [] + attachment_errors: list[str] = [] + for attachment in sorted(candidates, key=lambda item: str(item.get("key", ""))): + try: + file_url = client.get_attachment_file_url(str(attachment.get("key", ""))) + local_path = existing_pdf_from_file_url(file_url) + except ZoteroLocalError as exc: + attachment_errors.append(exc.code) + continue + if local_path: + local_candidates.append((attachment, local_path)) + + if preferred_key: + preferred = [pair for pair in local_candidates if pair[0].get("key") == preferred_key] + if preferred: + local_candidates = preferred + if local_candidates: + best_priority = max(_attachment_priority(item) for item, _ in local_candidates) + best = [pair for pair in local_candidates if _attachment_priority(pair[0]) == best_priority] + distinct_paths = {path for _, path in best} + if len(distinct_paths) == 1: + attachment, local_path = best[0] + record["zotero_attachment_status"] = "available" + record["zotero_attachment_key"] = attachment["key"] + record["zotero_attachment_filename"] = attachment["filename"] + record["local_pdf_path"] = local_path + return record + record["zotero_attachment_status"] = "ambiguous" + return record + + remote_pdf_urls: set[str] = set() + for item in candidates: + attachment_url = str(item.get("url", "")) + if not attachment_url: + continue + try: + parsed_url = _split_attachment_url(attachment_url) + except ZoteroLocalError as exc: + attachment_errors.append(exc.code) + continue + if parsed_url.scheme == "https" and parsed_url.path.lower().endswith(".pdf"): + remote_pdf_urls.add(attachment_url) + pdf_urls = sorted(remote_pdf_urls) + if len(pdf_urls) == 1: + record["pdf_url"] = pdf_urls[0] + record["zotero_attachment_status"] = "remote_only" + else: + record["zotero_attachment_status"] = "unavailable" + if attachment_errors: + record["zotero_attachment_error"] = attachment_errors[0] + return record + + +def _zotero_key_from_reference(reference: str, *, reference_type: str) -> str: + value = normalize_whitespace(reference) + if ZOTERO_GROUP_SELECT_PATTERN.fullmatch(value): + raise ZoteroLocalError( + "zotero_unsupported_library", + "Zotero group-library select links are not supported by the local user-library client.", + ) + match = ZOTERO_SELECT_KEY_PATTERN.fullmatch(value) + if match: + return match.group(1).upper() + if reference_type == "zotero_key" or ZOTERO_ITEM_KEY_PATTERN.fullmatch(value): + return value.upper() + return "" + + +def _resolve_parent_item( + client: ZoteroLocalClient, + raw_item: dict[str, Any], +) -> tuple[dict[str, Any], dict[str, Any] | None]: + data = _item_data(raw_item) + if data.get("itemType") != "attachment": + return raw_item, None + parent_key = normalize_whitespace(str(data.get("parentItem", ""))).upper() + if not ZOTERO_ITEM_KEY_PATTERN.fullmatch(parent_key): + raise ZoteroLocalError( + "zotero_item_not_bibliographic", + "The Zotero attachment is not linked to a bibliographic parent item.", + ) + return client.get_item(parent_key), raw_item + + +def lookup_zotero_local( + reference: str, + *, + reference_type: str = "", + client: ZoteroLocalClient | None = None, +) -> dict[str, Any]: + """Resolve one scalar reference against the local Zotero library.""" + + local_client = client or ZoteroLocalClient() + source_type = reference_type or infer_source_type(reference) + try: + probe = local_client.probe() + explicit_key = _zotero_key_from_reference(reference, reference_type=source_type) + preferred_attachment: dict[str, Any] | None = None + if explicit_key: + raw_item = local_client.get_item(explicit_key) + if zotero_item_key(raw_item) != explicit_key: + raise ZoteroLocalError( + "zotero_invalid_response", + "Zotero returned a different item than the requested key.", + ) + raw_parent, preferred_attachment = _resolve_parent_item(local_client, raw_item) + record = normalize_zotero_item(raw_parent) + parent_key = str(record.get("zotero_key", "")) + if record.get("zotero_item_type") in {"attachment", "note", "annotation", ""}: + raise ZoteroLocalError( + "zotero_item_not_bibliographic", + "The Zotero item is not a bibliographic parent item.", + ) + if not ZOTERO_ITEM_KEY_PATTERN.fullmatch(parent_key): + raise ZoteroLocalError( + "zotero_invalid_response", + "Zotero bibliographic item did not contain a valid item key.", + ) + match_kind = "zotero_key" + else: + doi = extract_doi(reference) or "" + arxiv_id = extract_arxiv_id(reference) or "" + if source_type in {"doi", "doi_url"} or doi: + match_kind = "doi" + query = doi + qmode = "everything" + elif source_type in {"arxiv_id", "arxiv_url"} or arxiv_id: + match_kind = "arxiv_id" + query = arxiv_id + qmode = "everything" + elif source_type in {"title", "title_query"}: + match_kind = "title" + query = reference + qmode = "titleCreatorYear" + else: + return { + "status": "not_found", + "match_kind": "unsupported_reference", + "probe": probe, + } + raw_items = local_client.search_top_items(query, qmode=qmode) + match = _match_search_results(raw_items, match_kind=match_kind, query=query) + if match["status"] != "match": + match["probe"] = probe + return match + record = dict(match["record"]) + + try: + raw_children = local_client.get_children(str(record.get("zotero_key", ""))) + record = _attach_local_pdf( + local_client, + record, + raw_children, + preferred_attachment=preferred_attachment, + ) + except ZoteroLocalError as exc: + record["zotero_attachment_status"] = "unavailable" + record["zotero_attachment_error"] = exc.code + record["paper_id"] = paper_id_for_record(record) + record = apply_identity_confidence(record) + return { + "status": "match", + "match_kind": match_kind, + "record": record, + "probe": probe, + } + except ZoteroLocalError as exc: + if exc.code == "zotero_item_not_found": + return { + "status": "not_found", + "match_kind": source_type, + "error": exc.as_dict(), + } + return { + "status": "error", + "match_kind": source_type, + "error": exc.as_dict(), + } + + +def probe_zotero_local_api( + *, + client: ZoteroLocalClient | None = None, +) -> dict[str, Any]: + """Return a non-raising environment diagnostic for the Local API.""" + + local_client = client or ZoteroLocalClient() + try: + return local_client.probe() + except ZoteroLocalError as exc: + if exc.code == "zotero_api_disabled": + status = "disabled" + elif exc.code == "zotero_unsupported_version": + status = "incompatible" + elif exc.code in {"zotero_not_running", "zotero_timeout", "zotero_server_error"}: + status = "unavailable" + else: + status = "error" + return { + "status": status, + "reachable": exc.code not in {"zotero_not_running", "zotero_timeout"}, + "ready": False, + "base_url": local_client.base_url, + "api_version": "", + "schema_version": "", + "error": exc.as_dict(), + } diff --git a/skills/deeppapernote/scripts/check_environment.py b/skills/deeppapernote/scripts/check_environment.py index 28b08c0..ac8ccfd 100644 --- a/skills/deeppapernote/scripts/check_environment.py +++ b/skills/deeppapernote/scripts/check_environment.py @@ -9,6 +9,7 @@ import sys from pathlib import Path +from _zotero_local import probe_zotero_local_api from common import emit, env_config_value, runtime_config @@ -68,6 +69,7 @@ def find_local_zotero_hints() -> list[str]: def main() -> None: args = parser().parse_args() config = runtime_config() + zotero_local_api = probe_zotero_local_api() obsidian_vault = str(config.get("obsidian_vault", "")).strip() obsidian_vault_exists = bool(obsidian_vault) and Path(obsidian_vault).expanduser().exists() @@ -98,13 +100,24 @@ def main() -> None: "available": True, "current_working_directory": str(Path.cwd().resolve()), "workspace_output_dir": str(config.get("workspace_output_dir", "DeepPaperNote_output")), - "note": "If no Obsidian vault is configured, DeepPaperNote can still save notes under the current working directory.", + "note": ( + "If no Obsidian vault is configured, DeepPaperNote can still save notes under " + "the current working directory." + ), }, "zotero": { "local_hints": find_local_zotero_hints(), + "local_api": zotero_local_api, + "local_api_available": bool(zotero_local_api.get("ready")), + "local_api_status": str(zotero_local_api.get("status", "error")), + "local_api_version": str(zotero_local_api.get("api_version", "")), + "local_api_schema_version": str(zotero_local_api.get("schema_version", "")), "mcp_available_from_script": False, "session_integration_checked_by_script": False, - "note": "Session-scoped library integrations must be checked by the active agent at runtime, not by this script.", + "note": ( + "The built-in read-only Local API check is reported here. Session-scoped " + "library integrations must still be checked by the active agent at runtime." + ), }, "ocr": { "tesseract_installed": bool(tesseract_path), @@ -117,8 +130,10 @@ def main() -> None: "metadata": { "maintenance_utility": True, "semantic_scholar_api_key_configured": bool( - env_config_value("DEEPPAPERNOTE_SEMANTIC_SCHOLAR_API_KEY", "SEMANTIC_SCHOLAR_API_KEY") - ) + env_config_value( + "DEEPPAPERNOTE_SEMANTIC_SCHOLAR_API_KEY", "SEMANTIC_SCHOLAR_API_KEY" + ) + ), }, } emit(payload, args.output) diff --git a/skills/deeppapernote/scripts/common.py b/skills/deeppapernote/scripts/common.py index f6c6b54..4533e2d 100644 --- a/skills/deeppapernote/scripts/common.py +++ b/skills/deeppapernote/scripts/common.py @@ -1719,6 +1719,51 @@ def choose_best_title_match(title: str, candidates: list[dict[str, Any]]) -> dic return best +def _record_publication_year(record: dict[str, Any]) -> str: + for key in ("year", "published"): + match = re.search(r"(? bool: + paper_id = _string_field(record, "paper_id").lower() + return bool( + extract_doi(_string_field(record, "doi")) + or extract_doi(_string_field(record, "source_url")) + or _record_arxiv_id(record) + or paper_id.startswith(("doi:", "arxiv:")) + ) + + +def _title_enrichment_candidate_is_identity_safe( + base: dict[str, Any], + candidate: dict[str, Any], +) -> bool: + is_zotero_record = _string_field(base, "source_type") == "zotero" or bool( + _string_field(base, "zotero_key") + ) + if ( + not is_zotero_record + or _has_strong_work_identifier(base) + or not _has_strong_work_identifier(candidate) + ): + return True + + author_evidence = _leading_author_status(base, candidate) + base_year = _record_publication_year(base) + candidate_year = _record_publication_year(candidate) + return bool( + title_similarity(_string_field(base, "title"), _string_field(candidate, "title")) + >= 0.82 + and author_evidence + and author_evidence["status"] == "match" + and base_year + and base_year == candidate_year + ) + + def resolve_reference(value: str) -> dict[str, Any]: source_type = infer_source_type(value) stripped = (value or "").strip() @@ -1856,16 +1901,19 @@ def enrich_metadata(record: dict[str, Any]) -> dict[str, Any]: if title: sem = choose_best_title_match(title, search_semantic_scholar(title, limit=5)) - if sem: + if sem and _title_enrichment_candidate_is_identity_safe(base, sem): candidates.append(sem) oa = choose_best_title_match(title, search_openalex_by_title(title, limit=5)) - if oa: + if oa and _title_enrichment_candidate_is_identity_safe(base, oa): candidates.append(oa) cross = choose_best_title_match(title, search_crossref_by_title(title, limit=5)) - if cross: + if cross and _title_enrichment_candidate_is_identity_safe(base, cross): candidates.append(cross) - arxiv = choose_best_title_match(title, safe_fetch_arxiv_entries(search_query=f'ti:"{title}"', max_results=5)) - if arxiv: + arxiv = choose_best_title_match( + title, + safe_fetch_arxiv_entries(search_query=f'ti:"{title}"', max_results=5), + ) + if arxiv and _title_enrichment_candidate_is_identity_safe(base, arxiv): candidates.append(arxiv) merged = merge_metadata_records(*candidates) diff --git a/skills/deeppapernote/scripts/resolve_paper.py b/skills/deeppapernote/scripts/resolve_paper.py index ded3f62..09eabc6 100644 --- a/skills/deeppapernote/scripts/resolve_paper.py +++ b/skills/deeppapernote/scripts/resolve_paper.py @@ -3,19 +3,148 @@ from __future__ import annotations +from typing import Any + +from _zotero_local import lookup_zotero_local from common import ( base_parser, emit, + infer_source_type, maybe_load_json_record, paper_id_for_record, require_ok_input_artifact, resolve_reference, ) +ZOTERO_MODES = ("auto", "off", "required") +ZOTERO_QUERY_SOURCE_TYPES = { + "arxiv_id", + "arxiv_url", + "doi", + "doi_url", + "title", + "title_query", + "zotero_key", +} +ZOTERO_BYPASS_SOURCE_TYPES = {"local_pdf"} + + +class ZoteroResolutionError(RuntimeError): + """Raised when Zotero mode requires the resolver to stop fail-closed.""" + + def __init__(self, message: str, *, lookup: dict[str, Any], failure_class: str) -> None: + super().__init__(message) + self.lookup = lookup + self.failure_class = failure_class + + +def parser(): + p = base_parser(__doc__ or "resolve paper") + p.add_argument( + "--zotero-mode", + choices=ZOTERO_MODES, + default="auto", + help=( + "Local Zotero lookup policy: auto prefers unique local matches, off preserves " + "the web-only path, and required fails unless Zotero resolves the reference." + ), + ) + return p + + +def zotero_lookup_summary(result: dict[str, Any], *, mode: str) -> dict[str, Any]: + summary: dict[str, Any] = { + "mode": mode, + "status": str(result.get("status", "error")), + "match_kind": str(result.get("match_kind", "")), + } + if result.get("candidate_count") is not None: + summary["candidate_count"] = result["candidate_count"] + probe = result.get("probe", {}) + if isinstance(probe, dict) and probe: + summary["api_status"] = str(probe.get("status", "")) + summary["api_version"] = str(probe.get("api_version", "")) + summary["schema_version"] = str(probe.get("schema_version", "")) + error = result.get("error", {}) + if isinstance(error, dict) and error: + summary["error"] = dict(error) + return summary + + +def _zotero_failure(result: dict[str, Any], *, mode: str) -> ZoteroResolutionError: + status = str(result.get("status", "error")) + summary = zotero_lookup_summary(result, mode=mode) + if status == "ambiguous": + return ZoteroResolutionError( + "Zotero returned multiple equally plausible local items; provide a DOI, arXiv ID, " + "or exact Zotero item key.", + lookup=summary, + failure_class="zotero_ambiguous_match", + ) + error = result.get("error", {}) + error_code = str(error.get("code", "")) if isinstance(error, dict) else "" + if str(result.get("match_kind", "")) == "unsupported_reference": + failure_class = "zotero_unsupported_reference" + else: + failure_class = error_code or ( + "zotero_item_not_found" if status == "not_found" else "zotero_lookup_failed" + ) + return ZoteroResolutionError( + "Zotero Local API could not uniquely resolve this reference while " + f"--zotero-mode={mode} is active.", + lookup=summary, + failure_class=failure_class, + ) + + +def resolve_scalar_reference(reference: str, *, zotero_mode: str = "auto") -> dict[str, Any]: + if zotero_mode not in ZOTERO_MODES: + raise ValueError(f"Unsupported Zotero mode: {zotero_mode}") + source_type = infer_source_type(reference) + if zotero_mode == "off" or source_type in ZOTERO_BYPASS_SOURCE_TYPES: + return resolve_reference(reference) + + if source_type not in ZOTERO_QUERY_SOURCE_TYPES: + if zotero_mode == "required": + result = { + "status": "not_found", + "match_kind": "unsupported_reference", + } + raise _zotero_failure(result, mode=zotero_mode) + return resolve_reference(reference) + + lookup = lookup_zotero_local(reference, reference_type=source_type) + lookup_status = str(lookup.get("status", "error")) + explicit_zotero_reference = source_type == "zotero_key" or reference.strip().lower().startswith( + "zotero://select/" + ) + if lookup_status == "match": + record = lookup.get("record", {}) + if not isinstance(record, dict): + lookup = { + "status": "error", + "match_kind": source_type, + "error": { + "code": "zotero_invalid_response", + "message": "Zotero lookup returned an invalid record.", + "retryable": False, + }, + } + else: + resolved = dict(record) + resolved["zotero_lookup"] = zotero_lookup_summary(lookup, mode=zotero_mode) + return resolved + + if lookup_status == "ambiguous" or zotero_mode == "required" or explicit_zotero_reference: + raise _zotero_failure(lookup, mode=zotero_mode) + + resolved = resolve_reference(reference) + resolved["zotero_lookup"] = zotero_lookup_summary(lookup, mode=zotero_mode) + return resolved + -def main() -> None: - parser = base_parser(__doc__ or "resolve paper") - args = parser.parse_args() +def main(argv: list[str] | None = None) -> None: + args = parser().parse_args(argv) if not args.input: raise SystemExit("resolve_paper.py requires --input.") @@ -24,9 +153,25 @@ def main() -> None: if input_record is not None: resolved = dict(require_ok_input_artifact(input_record, "resolve_paper.py")) else: - resolved = resolve_reference(args.input) + try: + resolved = resolve_scalar_reference(args.input, zotero_mode=args.zotero_mode) + except ZoteroResolutionError as exc: + emit( + { + "status": "error", + "run_status": "failed", + "script": "resolve_paper.py", + "failure_class": exc.failure_class, + "failure_summary": str(exc), + "zotero_lookup": exc.lookup, + }, + args.output, + ) + raise SystemExit(str(exc)) from exc - resolved["paper_id"] = args.paper_id or resolved.get("paper_id") or paper_id_for_record(resolved) + resolved["paper_id"] = ( + args.paper_id or resolved.get("paper_id") or paper_id_for_record(resolved) + ) resolved["status"] = resolved.get("status") or "ok" resolved["script"] = "resolve_paper.py" emit(resolved, args.output) diff --git a/skills/deeppapernote/scripts/run_pipeline.py b/skills/deeppapernote/scripts/run_pipeline.py index 08bda82..40a0780 100644 --- a/skills/deeppapernote/scripts/run_pipeline.py +++ b/skills/deeppapernote/scripts/run_pipeline.py @@ -22,6 +22,12 @@ def parser() -> argparse.ArgumentParser: help="Directory for intermediate artifacts.", ) p.add_argument("--prefix", default="run", help="Filename prefix for artifacts.") + p.add_argument( + "--zotero-mode", + choices=("auto", "off", "required"), + default="auto", + help="Local Zotero lookup policy used by the resolve stage.", + ) return p @@ -55,6 +61,8 @@ def main() -> None: str(scripts_dir / "resolve_paper.py"), "--input", args.input, + "--zotero-mode", + args.zotero_mode, "--output", str(resolve_json), ] diff --git a/tests/test_check_environment.py b/tests/test_check_environment.py new file mode 100644 index 0000000..c27753f --- /dev/null +++ b/tests/test_check_environment.py @@ -0,0 +1,99 @@ +from __future__ import annotations + +import json +import sys +from pathlib import Path + +import check_environment +import pytest + + +def run_environment_check( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + local_api: dict, +) -> dict: + output = tmp_path / "environment.json" + monkeypatch.setattr(check_environment, "probe_zotero_local_api", lambda: dict(local_api)) + monkeypatch.setattr(check_environment, "find_obsidian_candidates", lambda: []) + monkeypatch.setattr(check_environment, "find_local_zotero_hints", lambda: []) + monkeypatch.setattr( + check_environment, + "runtime_config", + lambda: { + "obsidian_vault": "", + "papers_dir": "Research/Papers", + "output_dir": "tmp/DeepPaperNote", + "workspace_output_dir": "DeepPaperNote_output", + }, + ) + monkeypatch.setattr(sys, "argv", ["check_environment.py", "--output", str(output)]) + check_environment.main() + return json.loads(output.read_text(encoding="utf-8")) + + +def test_environment_reports_available_zotero_local_api( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + payload = run_environment_check( + tmp_path, + monkeypatch, + { + "status": "available", + "reachable": True, + "ready": True, + "base_url": "http://127.0.0.1:23119/api", + "api_version": "3", + "schema_version": "37", + }, + ) + + assert payload["status"] == "ok" + assert payload["tool_role"] == "maintenance" + assert payload["zotero"]["local_api_available"] is True + assert payload["zotero"]["local_api_status"] == "available" + assert payload["zotero"]["local_api_version"] == "3" + assert payload["zotero"]["local_api_schema_version"] == "37" + assert payload["zotero"]["local_api"]["base_url"].startswith("http://127.0.0.1") + assert payload["zotero"]["mcp_available_from_script"] is False + assert payload["zotero"]["session_integration_checked_by_script"] is False + + +@pytest.mark.parametrize( + ("status", "error_code"), + [ + ("unavailable", "zotero_not_running"), + ("disabled", "zotero_api_disabled"), + ("incompatible", "zotero_unsupported_version"), + ], +) +def test_environment_keeps_full_report_when_zotero_is_not_ready( + status: str, + error_code: str, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + payload = run_environment_check( + tmp_path, + monkeypatch, + { + "status": status, + "reachable": status != "unavailable", + "ready": False, + "base_url": "http://127.0.0.1:23119/api", + "api_version": "", + "schema_version": "", + "error": { + "code": error_code, + "message": "fixture error", + "retryable": status == "unavailable", + }, + }, + ) + + assert payload["status"] == "ok" + assert payload["workspace_fallback"]["available"] is True + assert payload["zotero"]["local_api_available"] is False + assert payload["zotero"]["local_api_status"] == status + assert payload["zotero"]["local_api"]["error"]["code"] == error_code diff --git a/tests/test_contracts_consistency.py b/tests/test_contracts_consistency.py index ffeefcd..0ba81c8 100644 --- a/tests/test_contracts_consistency.py +++ b/tests/test_contracts_consistency.py @@ -692,6 +692,29 @@ def test_pdf_contract_docs_try_supported_acquisition_before_stopping() -> None: assert "标题、DOI、URL、本地 PDF 都可以" in readme_zh_text +def test_zotero_local_api_contract_is_consistent_across_docs() -> None: + skill_text = (PROJECT_ROOT / "skills" / "deeppapernote" / "SKILL.md").read_text( + encoding="utf-8" + ) + metadata_text = ( + PROJECT_ROOT / "skills" / "deeppapernote" / "references" / "metadata-sources.md" + ).read_text(encoding="utf-8") + readme_text = (PROJECT_ROOT / "README.md").read_text(encoding="utf-8") + readme_zh_text = (PROJECT_ROOT / "README.zh-CN.md").read_text(encoding="utf-8") + + for text in (skill_text, readme_text): + assert "Zotero Local API" in text + assert "auto" in text + assert "off" in text + assert "required" in text + + assert "read-only Zotero Local API" in metadata_text + assert "Zotero Local API" in readme_zh_text + assert "`auto`(默认)" in readme_zh_text + assert "ambiguous local match always fails closed" in readme_text + assert "兼容集成仍可作为可选替代路线" in readme_zh_text + + def test_regression_workflow_documents_acquisition_identity_audit_contract() -> None: texts = [ (PROJECT_ROOT / "evals" / "regression-workflow.md").read_text(encoding="utf-8"), diff --git a/tests/test_resolve_paper_zotero.py b/tests/test_resolve_paper_zotero.py new file mode 100644 index 0000000..2013e26 --- /dev/null +++ b/tests/test_resolve_paper_zotero.py @@ -0,0 +1,402 @@ +from __future__ import annotations + +import json +from pathlib import Path + +import common +import pytest +import resolve_paper + + +def local_record() -> dict: + return { + "status": "ok", + "source_type": "zotero", + "metadata_sources": ["zotero"], + "zotero_key": "PARENT01", + "title": "Local Canonical Title", + "authors": ["Local Author"], + "year": "2024", + "venue": "Local Venue", + "doi": "10.5555/local", + "local_pdf_path": "C:/Zotero/storage/ATTACH01/paper.pdf", + "paper_id": "doi:10.5555/local", + "identity_confidence": "high", + "identity_confidence_reasons": ["doi_present", "zotero_key_present"], + } + + +def test_auto_mode_prefers_unique_local_match_without_web_resolution( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr( + resolve_paper, + "lookup_zotero_local", + lambda reference, reference_type: { + "status": "match", + "match_kind": "doi", + "record": local_record(), + "probe": { + "status": "available", + "api_version": "3", + "schema_version": "37", + }, + }, + ) + monkeypatch.setattr( + resolve_paper, + "resolve_reference", + lambda reference: pytest.fail("web resolution must not run after a Zotero match"), + ) + + resolved = resolve_paper.resolve_scalar_reference("10.5555/local") + + assert resolved["title"] == "Local Canonical Title" + assert resolved["zotero_key"] == "PARENT01" + assert resolved["zotero_lookup"] == { + "mode": "auto", + "status": "match", + "match_kind": "doi", + "api_status": "available", + "api_version": "3", + "schema_version": "37", + } + + +@pytest.mark.parametrize("lookup_status", ["not_found", "error"]) +def test_auto_mode_falls_back_when_local_api_has_no_unique_match( + lookup_status: str, + monkeypatch: pytest.MonkeyPatch, +) -> None: + lookup = { + "status": lookup_status, + "match_kind": "doi", + "error": { + "code": "zotero_not_running", + "message": "not running", + "retryable": True, + }, + } + monkeypatch.setattr( + resolve_paper, + "lookup_zotero_local", + lambda reference, reference_type: lookup, + ) + monkeypatch.setattr( + resolve_paper, + "resolve_reference", + lambda reference: { + "status": "ok", + "source_type": "doi", + "doi": reference, + "paper_id": f"doi:{reference}", + }, + ) + + resolved = resolve_paper.resolve_scalar_reference("10.5555/fallback") + + assert resolved["doi"] == "10.5555/fallback" + assert resolved["zotero_lookup"]["status"] == lookup_status + assert resolved["zotero_lookup"]["error"]["code"] == "zotero_not_running" + + +def test_auto_mode_fails_closed_on_ambiguous_local_identity( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr( + resolve_paper, + "lookup_zotero_local", + lambda reference, reference_type: { + "status": "ambiguous", + "match_kind": "title", + "candidate_count": 2, + }, + ) + monkeypatch.setattr( + resolve_paper, + "resolve_reference", + lambda reference: pytest.fail("an ambiguous local identity must not fall through"), + ) + + with pytest.raises(resolve_paper.ZoteroResolutionError) as exc_info: + resolve_paper.resolve_scalar_reference("Ambiguous Paper") + + assert exc_info.value.failure_class == "zotero_ambiguous_match" + assert exc_info.value.lookup["candidate_count"] == 2 + + +def test_required_mode_fails_when_zotero_is_unavailable( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr( + resolve_paper, + "lookup_zotero_local", + lambda reference, reference_type: { + "status": "error", + "match_kind": "zotero_key", + "error": { + "code": "zotero_not_running", + "message": "not running", + "retryable": True, + }, + }, + ) + + with pytest.raises(resolve_paper.ZoteroResolutionError) as exc_info: + resolve_paper.resolve_scalar_reference("PARENT01", zotero_mode="required") + + assert exc_info.value.failure_class == "zotero_not_running" + + +def test_auto_mode_fails_when_an_explicit_zotero_key_cannot_be_verified( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr( + resolve_paper, + "lookup_zotero_local", + lambda reference, reference_type: { + "status": "error", + "match_kind": "zotero_key", + "error": { + "code": "zotero_not_running", + "message": "not running", + "retryable": True, + }, + }, + ) + monkeypatch.setattr( + resolve_paper, + "resolve_reference", + lambda reference: pytest.fail("an unresolved explicit key has no safe web fallback"), + ) + + with pytest.raises(resolve_paper.ZoteroResolutionError) as exc_info: + resolve_paper.resolve_scalar_reference("PARENT01", zotero_mode="auto") + + assert exc_info.value.failure_class == "zotero_not_running" + + +def test_off_mode_preserves_old_path_without_local_api_call( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr( + resolve_paper, + "lookup_zotero_local", + lambda reference, reference_type: pytest.fail("off mode must not query Zotero"), + ) + expected = {"status": "ok", "source_type": "zotero_key", "zotero_key": "PARENT01"} + monkeypatch.setattr(resolve_paper, "resolve_reference", lambda reference: dict(expected)) + + assert resolve_paper.resolve_scalar_reference("PARENT01", zotero_mode="off") == expected + + +def test_required_mode_bypasses_an_explicit_local_pdf( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + pdf_path = tmp_path / "paper.pdf" + pdf_path.write_bytes(b"%PDF-1.4\n") + monkeypatch.setattr( + resolve_paper, + "lookup_zotero_local", + lambda reference, reference_type: pytest.fail("local PDFs must bypass Zotero lookup"), + ) + monkeypatch.setattr( + resolve_paper, + "resolve_reference", + lambda reference: {"status": "ok", "local_pdf_path": reference}, + ) + + resolved = resolve_paper.resolve_scalar_reference(str(pdf_path), zotero_mode="required") + + assert resolved["local_pdf_path"] == str(pdf_path) + + +def test_required_mode_rejects_a_generic_non_pdf_url() -> None: + with pytest.raises(resolve_paper.ZoteroResolutionError) as exc_info: + resolve_paper.resolve_scalar_reference( + "https://example.test/papers/landing-page", + zotero_mode="required", + ) + assert exc_info.value.failure_class == "zotero_unsupported_reference" + assert exc_info.value.lookup["match_kind"] == "unsupported_reference" + + +def test_required_mode_rejects_a_direct_pdf_url_without_querying_zotero( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr( + resolve_paper, + "lookup_zotero_local", + lambda reference, reference_type: pytest.fail( + "a PDF URL has no strong Zotero query identifier" + ), + ) + + with pytest.raises(resolve_paper.ZoteroResolutionError) as exc_info: + resolve_paper.resolve_scalar_reference( + "https://example.test/paper.pdf", + zotero_mode="required", + ) + + assert exc_info.value.failure_class == "zotero_unsupported_reference" + + +def test_main_passes_through_trusted_json_without_querying_zotero( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + artifact = tmp_path / "input.json" + output = tmp_path / "resolved.json" + artifact.write_text( + json.dumps( + { + "status": "ok", + "title": "Trusted Record", + "doi": "10.5555/trusted", + "paper_id": "doi:10.5555/trusted", + } + ), + encoding="utf-8", + ) + monkeypatch.setattr( + resolve_paper, + "lookup_zotero_local", + lambda reference, reference_type: pytest.fail("trusted artifacts must pass through"), + ) + + resolve_paper.main( + [ + "--input", + str(artifact), + "--output", + str(output), + "--zotero-mode", + "required", + "--paper-id", + "manual:override", + ] + ) + + resolved = json.loads(output.read_text(encoding="utf-8")) + assert resolved["title"] == "Trusted Record" + assert resolved["paper_id"] == "manual:override" + assert resolved["script"] == "resolve_paper.py" + + +def test_main_emits_failure_artifact_for_required_lookup_failure( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + output = tmp_path / "resolved.json" + monkeypatch.setattr( + resolve_paper, + "lookup_zotero_local", + lambda reference, reference_type: { + "status": "not_found", + "match_kind": "doi", + }, + ) + + with pytest.raises(SystemExit, match="could not uniquely resolve"): + resolve_paper.main( + [ + "--input", + "10.5555/missing", + "--output", + str(output), + "--zotero-mode", + "required", + ] + ) + + failure = json.loads(output.read_text(encoding="utf-8")) + assert failure["status"] == "error" + assert failure["run_status"] == "failed" + assert failure["failure_class"] == "zotero_item_not_found" + assert failure["zotero_lookup"]["mode"] == "required" + + +def test_web_enrichment_does_not_override_zotero_identity( + monkeypatch: pytest.MonkeyPatch, +) -> None: + record = local_record() + monkeypatch.setattr( + common, + "fetch_crossref_by_doi", + lambda doi: { + "title": "Conflicting Web Title", + "authors": ["Other Author"], + "doi": "10.5555/conflict", + "venue": "Conflicting Venue", + "abstract": "Useful missing abstract.", + "metadata_sources": ["crossref"], + }, + ) + monkeypatch.setattr(common, "fetch_openalex_by_doi", lambda doi: None) + monkeypatch.setattr(common, "search_semantic_scholar", lambda *args, **kwargs: []) + monkeypatch.setattr(common, "search_openalex_by_title", lambda *args, **kwargs: []) + monkeypatch.setattr(common, "search_crossref_by_title", lambda *args, **kwargs: []) + monkeypatch.setattr(common, "safe_fetch_arxiv_entries", lambda *args, **kwargs: []) + + enriched = common.enrich_metadata(record) + + assert enriched["title"] == "Local Canonical Title" + assert enriched["authors"] == ["Local Author"] + assert enriched["doi"] == "10.5555/local" + assert enriched["venue"] == "Local Venue" + assert enriched["local_pdf_path"] == record["local_pdf_path"] + assert enriched["abstract"] == "Useful missing abstract." + assert enriched["metadata_sources"] == ["zotero", "crossref"] + + +def test_title_enrichment_rejects_an_unvalidated_doi_for_a_zotero_item( + monkeypatch: pytest.MonkeyPatch, +) -> None: + record = local_record() + record.pop("doi") + record["paper_id"] = "zotero:PARENT01" + candidate = { + "title": "Local Canonical Title", + "authors": ["Different Author"], + "year": "2023", + "doi": "10.5555/different-paper", + "abstract": "Abstract from the wrong same-title paper.", + "metadata_sources": ["semantic_scholar"], + } + monkeypatch.setattr(common, "search_semantic_scholar", lambda *args, **kwargs: [candidate]) + monkeypatch.setattr(common, "search_openalex_by_title", lambda *args, **kwargs: []) + monkeypatch.setattr(common, "search_crossref_by_title", lambda *args, **kwargs: []) + monkeypatch.setattr(common, "safe_fetch_arxiv_entries", lambda *args, **kwargs: []) + + enriched = common.enrich_metadata(record) + + assert "doi" not in enriched + assert enriched["paper_id"] == "zotero:PARENT01" + assert "abstract" not in enriched + + +def test_title_enrichment_accepts_a_doi_after_zotero_author_and_year_validation( + monkeypatch: pytest.MonkeyPatch, +) -> None: + record = local_record() + record.pop("doi") + record.pop("paper_id") + candidate = { + "title": "Local Canonical Title", + "authors": ["Local Author", "Second Author"], + "year": "2024", + "doi": "10.5555/validated", + "abstract": "Validated enrichment.", + "metadata_sources": ["semantic_scholar"], + } + monkeypatch.setattr(common, "search_semantic_scholar", lambda *args, **kwargs: [candidate]) + monkeypatch.setattr(common, "search_openalex_by_title", lambda *args, **kwargs: []) + monkeypatch.setattr(common, "search_crossref_by_title", lambda *args, **kwargs: []) + monkeypatch.setattr(common, "safe_fetch_arxiv_entries", lambda *args, **kwargs: []) + + enriched = common.enrich_metadata(record) + + assert enriched["doi"] == "10.5555/validated" + assert enriched["paper_id"] == "doi:10.5555/validated" + assert enriched["abstract"] == "Validated enrichment." diff --git a/tests/test_run_pipeline_manifest.py b/tests/test_run_pipeline_manifest.py index e2062de..29c4c9d 100644 --- a/tests/test_run_pipeline_manifest.py +++ b/tests/test_run_pipeline_manifest.py @@ -14,7 +14,6 @@ import run_pipeline - PROJECT_ROOT = Path(__file__).resolve().parents[1] RUN_PIPELINE_SCRIPT = PROJECT_ROOT / "skills" / "deeppapernote" / "scripts" / "run_pipeline.py" @@ -147,36 +146,30 @@ def fake_run(cmd: list[str], check: bool = True, **kwargs) -> object: identity_call = calls[2] fetch_call = calls[3] assert resolve_call[resolve_call.index("--input") + 1] == "paper.pdf" - assert ( - metadata_call[metadata_call.index("--input") + 1] - == str((workdir / "paper_resolve.json").resolve()) + assert resolve_call[resolve_call.index("--zotero-mode") + 1] == "auto" + assert metadata_call[metadata_call.index("--input") + 1] == str( + (workdir / "paper_resolve.json").resolve() ) - assert ( - identity_call[identity_call.index("--input") + 1] - == str((workdir / "paper_metadata.json").resolve()) + assert identity_call[identity_call.index("--input") + 1] == str( + (workdir / "paper_metadata.json").resolve() ) - assert ( - identity_call[identity_call.index("--resolve") + 1] - == str((workdir / "paper_resolve.json").resolve()) + assert identity_call[identity_call.index("--resolve") + 1] == str( + (workdir / "paper_resolve.json").resolve() ) - assert ( - identity_call[identity_call.index("--trace-output") + 1] - == str((workdir / "paper_identity_repair_trace.json").resolve()) + assert identity_call[identity_call.index("--trace-output") + 1] == str( + (workdir / "paper_identity_repair_trace.json").resolve() ) - assert ( - fetch_call[fetch_call.index("--input") + 1] - == str((workdir / "paper_metadata.json").resolve()) + assert fetch_call[fetch_call.index("--input") + 1] == str( + (workdir / "paper_metadata.json").resolve() ) - assert ( - fetch_call[fetch_call.index("--identity") + 1] - == str((workdir / "paper_identity.json").resolve()) + assert fetch_call[fetch_call.index("--identity") + 1] == str( + (workdir / "paper_identity.json").resolve() ) evidence_call = calls[5] assert "--source-manifest" in evidence_call - assert ( - evidence_call[evidence_call.index("--source-manifest") + 1] - == str((workdir / "paper_source_manifest.json").resolve()) + assert evidence_call[evidence_call.index("--source-manifest") + 1] == str( + (workdir / "paper_source_manifest.json").resolve() ) @@ -216,3 +209,34 @@ def fake_run(cmd: list[str], check: bool = True, **kwargs) -> object: "collect_metadata.py", "build_identity_contract.py", ] + + +def test_run_pipeline_forwards_required_zotero_mode_only_to_resolve( + tmp_path: Path, + monkeypatch, +) -> None: + calls: list[list[str]] = [] + + def fake_run(cmd: list[str], check: bool = True, **kwargs) -> object: + calls.append(cmd) + return subprocess.CompletedProcess(cmd, 0) + + monkeypatch.setattr(run_pipeline.subprocess, "run", fake_run) + monkeypatch.setattr( + sys, + "argv", + [ + "run_pipeline.py", + "--input", + "10.5555/local", + "--workdir", + str(tmp_path / "run"), + "--zotero-mode", + "required", + ], + ) + + run_pipeline.main() + + assert calls[0][calls[0].index("--zotero-mode") + 1] == "required" + assert all("--zotero-mode" not in call for call in calls[1:]) diff --git a/tests/test_zotero_local_api.py b/tests/test_zotero_local_api.py new file mode 100644 index 0000000..a7e773b --- /dev/null +++ b/tests/test_zotero_local_api.py @@ -0,0 +1,639 @@ +from __future__ import annotations + +import json +import threading +import urllib.parse +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer +from pathlib import Path +from typing import Any, Callable + +import pytest +from _zotero_local import ( + HttpResult, + ZoteroLocalClient, + ZoteroLocalError, + _match_search_results, + file_url_to_local_path, + lookup_zotero_local, + normalize_zotero_item, + probe_zotero_local_api, +) + + +def json_result( + payload: object, *, status: int = 200, headers: dict[str, str] | None = None +) -> HttpResult: + return HttpResult( + status=status, + headers=headers or {}, + body=json.dumps(payload, ensure_ascii=False).encode("utf-8"), + ) + + +def text_result(payload: str, *, status: int = 200) -> HttpResult: + return HttpResult(status=status, headers={}, body=payload.encode("utf-8")) + + +def root_result(*, api_version: str = "3") -> HttpResult: + return json_result( + {}, + headers={ + "Zotero-API-Version": api_version, + "Zotero-Schema-Version": "37", + }, + ) + + +Route = HttpResult | Callable[[urllib.parse.SplitResult, dict[str, str], float], HttpResult] + + +def routed_transport( + routes: dict[str, Route], +) -> tuple[Callable[..., HttpResult], list[dict[str, Any]]]: + calls: list[dict[str, Any]] = [] + + def transport(url: str, headers: dict[str, str], timeout: float) -> HttpResult: + parsed = urllib.parse.urlsplit(url) + calls.append( + { + "url": url, + "path": parsed.path, + "query": urllib.parse.parse_qs(parsed.query), + "headers": dict(headers), + "timeout": timeout, + } + ) + route = routes.get(parsed.path) + if route is None: + return text_result("missing fixture route", status=404) + return route(parsed, headers, timeout) if callable(route) else route + + return transport, calls + + +def zotero_item( + key: str = "PARENT01", + *, + title: str = "Attention Is All You Need", + doi: str = "10.5555/Example", + extra: str = "arXiv: 1706.03762", + item_type: str = "journalArticle", +) -> dict[str, Any]: + return { + "key": key, + "version": 7, + "data": { + "key": key, + "itemType": item_type, + "title": title, + "creators": [ + { + "creatorType": "author", + "firstName": "Ashish", + "lastName": "Vaswani", + }, + {"creatorType": "author", "name": "Example Research Lab"}, + {"creatorType": "editor", "firstName": "Ignored", "lastName": "Editor"}, + ], + "DOI": doi, + "date": "2017-06-12", + "publicationTitle": "NeurIPS", + "url": "https://example.test/paper", + "extra": extra, + "abstractNote": "A transformer paper.", + }, + } + + +def zotero_attachment( + key: str = "ATTACH01", + *, + parent_key: str = "PARENT01", + filename: str = "paper.pdf", + title: str = "Full Text PDF", + link_mode: str = "imported_file", + url: str = "", +) -> dict[str, Any]: + return { + "key": key, + "data": { + "key": key, + "itemType": "attachment", + "parentItem": parent_key, + "linkMode": link_mode, + "title": title, + "contentType": "application/pdf", + "filename": filename, + "url": url, + }, + } + + +def base_routes() -> dict[str, Route]: + return {"/api/": root_result()} + + +def test_client_rejects_non_loopback_base_url() -> None: + with pytest.raises(ValueError, match="loopback"): + ZoteroLocalClient(base_url="https://example.test/api") + + +def test_probe_checks_api_version_and_is_cached() -> None: + transport, calls = routed_transport(base_routes()) + client = ZoteroLocalClient(transport=transport, timeout=0.25) + + first = client.probe() + second = client.probe() + + assert first == second + assert first == { + "status": "available", + "reachable": True, + "ready": True, + "base_url": "http://127.0.0.1:23119/api", + "api_version": "3", + "schema_version": "37", + } + assert len(calls) == 1 + assert calls[0]["headers"]["Zotero-API-Version"] == "3" + assert calls[0]["headers"]["Accept"] == "application/json" + assert "Authorization" not in calls[0]["headers"] + assert calls[0]["timeout"] == 0.25 + + +def test_probe_reports_disabled_and_incompatible_api() -> None: + disabled_transport, _ = routed_transport({"/api/": text_result("", status=403)}) + disabled = probe_zotero_local_api(client=ZoteroLocalClient(transport=disabled_transport)) + assert disabled["status"] == "disabled" + assert disabled["ready"] is False + assert disabled["error"]["code"] == "zotero_api_disabled" + + old_transport, _ = routed_transport({"/api/": root_result(api_version="2")}) + incompatible = probe_zotero_local_api(client=ZoteroLocalClient(transport=old_transport)) + assert incompatible["status"] == "incompatible" + assert incompatible["error"]["code"] == "zotero_unsupported_version" + + +def test_default_transport_does_not_follow_redirects_off_the_api_endpoint() -> None: + outside_requested = False + + class RedirectHandler(BaseHTTPRequestHandler): + def do_GET(self) -> None: + nonlocal outside_requested + if self.path == "/api/": + self.send_response(302) + self.send_header("Location", "/outside") + self.end_headers() + return + outside_requested = True + self.send_response(200) + self.send_header("Zotero-API-Version", "3") + self.end_headers() + + def log_message(self, format: str, *args: object) -> None: + return None + + server = ThreadingHTTPServer(("127.0.0.1", 0), RedirectHandler) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + client = ZoteroLocalClient( + base_url=f"http://127.0.0.1:{server.server_port}/api", + timeout=1, + ) + with pytest.raises(ZoteroLocalError) as exc_info: + client.probe() + finally: + server.shutdown() + server.server_close() + thread.join(timeout=2) + + assert exc_info.value.code == "zotero_invalid_response" + assert exc_info.value.http_status == 302 + assert outside_requested is False + + +def test_search_encodes_unicode_and_doi_queries_without_api_key() -> None: + routes = base_routes() + routes["/api/users/0/items/top"] = json_result([]) + transport, calls = routed_transport(routes) + client = ZoteroLocalClient(transport=transport) + + client.search_top_items("中文 标题", qmode="titleCreatorYear") + client.search_top_items("10.1000/test(1)/part", qmode="everything") + + assert calls[1]["query"]["q"] == ["中文 标题"] + assert calls[1]["query"]["qmode"] == ["titleCreatorYear"] + assert calls[1]["query"]["itemType"] == ["-attachment"] + assert "limit" not in calls[1]["query"] + assert calls[2]["query"]["q"] == ["10.1000/test(1)/part"] + assert "key" not in calls[1]["query"] + + +def test_client_rejects_invalid_json_and_wrong_search_shape() -> None: + routes = base_routes() + routes["/api/users/0/items/top"] = text_result("not-json") + transport, _ = routed_transport(routes) + with pytest.raises(ZoteroLocalError) as invalid_json: + ZoteroLocalClient(transport=transport).search_top_items("paper", qmode="everything") + assert invalid_json.value.code == "zotero_invalid_response" + + routes["/api/users/0/items/top"] = json_result({"items": []}) + transport, _ = routed_transport(routes) + with pytest.raises(ZoteroLocalError) as wrong_shape: + ZoteroLocalClient(transport=transport).search_top_items("paper", qmode="everything") + assert wrong_shape.value.code == "zotero_invalid_response" + + +def test_normalize_item_maps_bibliographic_fields_without_editors() -> None: + record = normalize_zotero_item(zotero_item()) + + assert record["zotero_key"] == "PARENT01" + assert record["title"] == "Attention Is All You Need" + assert record["authors"] == ["Ashish Vaswani", "Example Research Lab"] + assert record["doi"] == "10.5555/example" + assert record["arxiv_id"] == "1706.03762" + assert record["year"] == "2017" + assert record["venue"] == "NeurIPS" + assert record["abstract"] == "A transformer paper." + assert record["source_type"] == "zotero" + assert record["metadata_sources"] == ["zotero"] + + +def test_normalize_item_does_not_promote_local_file_url_to_source_url() -> None: + item = zotero_item(doi="") + item["data"]["url"] = "file:///tmp/untrusted.pdf" + + record = normalize_zotero_item(item) + + assert record["source_url"] == "" + + +def test_file_url_parser_handles_windows_unicode_spaces_and_literal_plus() -> None: + url = "file:///C:/Users/%E5%BC%A0%E4%B8%89/Zotero/storage/ABC12345/My%20Paper+Notes.pdf" + assert file_url_to_local_path(url, platform="nt") == ( + "C:\\Users\\张三\\Zotero\\storage\\ABC12345\\My Paper+Notes.pdf" + ) + + +def test_file_url_parser_handles_posix_and_rejects_remote_or_web_urls() -> None: + assert file_url_to_local_path("file:///Users/name/My%20Paper.pdf", platform="posix") == ( + "/Users/name/My Paper.pdf" + ) + assert file_url_to_local_path("file://localhost/Users/name/paper.pdf", platform="posix") == ( + "/Users/name/paper.pdf" + ) + for unsafe in ( + "file://server/share/paper.pdf", + "file:////server/share/paper.pdf", + "file://localhost:99/tmp/paper.pdf", + "https://example.test/paper.pdf", + "file:///tmp/paper.pdf?download=1", + "file:///tmp/bad\x00name.pdf", + "file:///tmp/bad%00name.pdf", + "file:///tmp/bad%FFname.pdf", + "file://[invalid/paper.pdf", + ): + with pytest.raises(ZoteroLocalError) as exc: + file_url_to_local_path(unsafe, platform="posix") + assert exc.value.code == "zotero_unsafe_file_url" + + +def test_matcher_prefers_exact_doi_over_similar_titles() -> None: + items = [ + zotero_item("PARENT01", doi="10.5555/exact"), + zotero_item("PARENT02", title="Attention Is Almost All You Need", doi="10.5555/other"), + ] + match = _match_search_results(items, match_kind="doi", query="https://doi.org/10.5555/EXACT") + assert match["status"] == "match" + assert match["record"]["zotero_key"] == "PARENT01" + + +def test_matcher_rejects_duplicate_strong_identifier() -> None: + items = [ + zotero_item("PARENT01", doi="10.5555/duplicate"), + zotero_item("PARENT02", doi="10.5555/DUPLICATE"), + ] + match = _match_search_results(items, match_kind="doi", query="10.5555/duplicate") + assert match == { + "status": "ambiguous", + "match_kind": "doi", + "candidate_count": 2, + } + + +def test_matcher_supports_nfkc_unicode_title_and_detects_ties() -> None: + unique = _match_search_results( + [zotero_item(title="深度学习:方法A")], + match_kind="title", + query="深度学习 方法A", + ) + assert unique["status"] == "match" + + tied = _match_search_results( + [ + zotero_item("PARENT01", title="A Study of Reliable Agents"), + zotero_item("PARENT02", title="A Study of Reliable Agents"), + ], + match_kind="title", + query="A Study of Reliable Agents", + ) + assert tied["status"] == "ambiguous" + + +def test_matcher_rejects_a_non_exact_title_without_independent_identity_evidence() -> None: + match = _match_search_results( + [zotero_item(title="Deep Learning for Dogs")], + match_kind="title", + query="Deep Learning for Cats", + ) + + assert match == { + "status": "not_found", + "match_kind": "title", + "candidate_count": 0, + } + + +def test_lookup_by_key_returns_parent_metadata_and_existing_pdf(tmp_path: Path) -> None: + pdf_path = tmp_path / "论文 附件.pdf" + pdf_path.write_bytes(b"%PDF-1.4\n") + routes = base_routes() + routes["/api/users/0/items/PARENT01"] = json_result(zotero_item()) + routes["/api/users/0/items/PARENT01/children"] = json_result( + [zotero_attachment(filename=pdf_path.name)] + ) + routes["/api/users/0/items/ATTACH01/file/view/url"] = text_result(pdf_path.as_uri()) + transport, calls = routed_transport(routes) + + result = lookup_zotero_local( + "PARENT01", + client=ZoteroLocalClient(transport=transport), + ) + + assert result["status"] == "match" + assert result["match_kind"] == "zotero_key" + assert result["record"]["zotero_key"] == "PARENT01" + assert result["record"]["zotero_attachment_key"] == "ATTACH01" + assert result["record"]["local_pdf_path"] == str(pdf_path.resolve()) + assert result["record"]["paper_id"] == "doi:10.5555/example" + assert result["record"]["identity_confidence"] == "high" + assert [call["path"] for call in calls] == [ + "/api/", + "/api/users/0/items/PARENT01", + "/api/users/0/items/PARENT01/children", + "/api/users/0/items/ATTACH01/file/view/url", + ] + + +@pytest.mark.parametrize("title", ["research", "learning", "ResearcH"]) +def test_lookup_does_not_treat_lowercase_or_mixed_case_title_as_item_key(title: str) -> None: + routes = base_routes() + routes["/api/users/0/items/top"] = json_result([]) + transport, calls = routed_transport(routes) + + result = lookup_zotero_local( + title, + client=ZoteroLocalClient(transport=transport), + ) + + assert result["status"] == "not_found" + assert calls[1]["path"] == "/api/users/0/items/top" + assert calls[1]["query"]["q"] == [title] + + +def test_lookup_by_attachment_key_resolves_bibliographic_parent(tmp_path: Path) -> None: + pdf_path = tmp_path / "paper.pdf" + pdf_path.write_bytes(b"%PDF-1.4\n") + attachment = zotero_attachment() + routes = base_routes() + routes["/api/users/0/items/ATTACH01"] = json_result(attachment) + routes["/api/users/0/items/PARENT01"] = json_result(zotero_item()) + routes["/api/users/0/items/PARENT01/children"] = json_result([]) + routes["/api/users/0/items/ATTACH01/file/view/url"] = text_result(pdf_path.as_uri()) + transport, _ = routed_transport(routes) + + result = lookup_zotero_local( + "ATTACH01", + client=ZoteroLocalClient(transport=transport), + ) + + assert result["status"] == "match" + assert result["record"]["zotero_key"] == "PARENT01" + assert result["record"]["zotero_attachment_key"] == "ATTACH01" + assert result["record"]["local_pdf_path"] == str(pdf_path.resolve()) + + +def test_lookup_by_attachment_key_does_not_substitute_a_sibling_pdf(tmp_path: Path) -> None: + sibling_path = tmp_path / "sibling.pdf" + sibling_path.write_bytes(b"%PDF-1.4\n") + selected = zotero_attachment("ATTACH01", filename="missing.pdf") + sibling = zotero_attachment("ATTACH02", filename=sibling_path.name) + routes = base_routes() + routes["/api/users/0/items/ATTACH01"] = json_result(selected) + routes["/api/users/0/items/PARENT01"] = json_result(zotero_item()) + routes["/api/users/0/items/PARENT01/children"] = json_result([selected, sibling]) + routes["/api/users/0/items/ATTACH01/file/view/url"] = text_result( + (tmp_path / "missing.pdf").as_uri() + ) + routes["/api/users/0/items/ATTACH02/file/view/url"] = text_result(sibling_path.as_uri()) + transport, calls = routed_transport(routes) + + result = lookup_zotero_local( + "ATTACH01", + client=ZoteroLocalClient(transport=transport), + ) + + assert result["status"] == "match" + assert result["record"]["zotero_attachment_status"] == "unavailable" + assert "local_pdf_path" not in result["record"] + assert "/api/users/0/items/ATTACH02/file/view/url" not in {call["path"] for call in calls} + + +def test_lookup_keeps_identity_when_attachment_is_missing(tmp_path: Path) -> None: + missing_path = tmp_path / "missing.pdf" + routes = base_routes() + routes["/api/users/0/items/PARENT01"] = json_result(zotero_item()) + routes["/api/users/0/items/PARENT01/children"] = json_result([zotero_attachment()]) + routes["/api/users/0/items/ATTACH01/file/view/url"] = text_result(missing_path.as_uri()) + transport, _ = routed_transport(routes) + + result = lookup_zotero_local( + "PARENT01", + client=ZoteroLocalClient(transport=transport), + ) + + assert result["status"] == "match" + assert result["record"]["zotero_attachment_status"] == "unavailable" + assert "local_pdf_path" not in result["record"] + + +def test_lookup_does_not_choose_between_equal_local_pdf_attachments(tmp_path: Path) -> None: + first = tmp_path / "first.pdf" + second = tmp_path / "second.pdf" + first.write_bytes(b"%PDF-1.4\n") + second.write_bytes(b"%PDF-1.4\n") + routes = base_routes() + routes["/api/users/0/items/PARENT01"] = json_result(zotero_item()) + routes["/api/users/0/items/PARENT01/children"] = json_result( + [ + zotero_attachment("ATTACH01", filename=first.name), + zotero_attachment("ATTACH02", filename=second.name), + ] + ) + routes["/api/users/0/items/ATTACH01/file/view/url"] = text_result(first.as_uri()) + routes["/api/users/0/items/ATTACH02/file/view/url"] = text_result(second.as_uri()) + transport, _ = routed_transport(routes) + + result = lookup_zotero_local( + "PARENT01", + client=ZoteroLocalClient(transport=transport), + ) + + assert result["status"] == "match" + assert result["record"]["zotero_attachment_status"] == "ambiguous" + assert "local_pdf_path" not in result["record"] + + +def test_lookup_returns_ambiguous_for_duplicate_local_doi() -> None: + routes = base_routes() + routes["/api/users/0/items/top"] = json_result( + [ + zotero_item("PARENT01", doi="10.5555/duplicate"), + zotero_item("PARENT02", doi="10.5555/duplicate"), + ] + ) + transport, _ = routed_transport(routes) + + result = lookup_zotero_local( + "10.5555/duplicate", + client=ZoteroLocalClient(transport=transport), + ) + + assert result["status"] == "ambiguous" + assert result["candidate_count"] == 2 + + +def test_lookup_checks_all_unpaginated_local_results_for_duplicates() -> None: + items = [zotero_item("PARENT01", doi="10.5555/duplicate")] + items.extend( + zotero_item(f"OTHER{index:03d}", doi=f"10.5555/other-{index}") for index in range(2, 11) + ) + items.append(zotero_item("PARENT11", doi="10.5555/duplicate")) + routes = base_routes() + routes["/api/users/0/items/top"] = json_result(items) + transport, calls = routed_transport(routes) + + result = lookup_zotero_local( + "10.5555/duplicate", + client=ZoteroLocalClient(transport=transport), + ) + + assert result["status"] == "ambiguous" + assert result["candidate_count"] == 2 + assert "limit" not in calls[1]["query"] + + +def test_lookup_ignores_search_results_without_valid_item_keys() -> None: + invalid = zotero_item("PARENT01", doi="10.5555/invalid") + invalid["key"] = "" + invalid["data"]["key"] = "" + routes = base_routes() + routes["/api/users/0/items/top"] = json_result([invalid]) + transport, _ = routed_transport(routes) + + result = lookup_zotero_local( + "10.5555/invalid", + client=ZoteroLocalClient(transport=transport), + ) + + assert result["status"] == "not_found" + + +def test_lookup_preserves_parent_metadata_when_children_endpoint_fails() -> None: + routes = base_routes() + routes["/api/users/0/items/PARENT01"] = json_result(zotero_item()) + routes["/api/users/0/items/PARENT01/children"] = text_result("", status=500) + transport, _ = routed_transport(routes) + + result = lookup_zotero_local( + "PARENT01", + client=ZoteroLocalClient(transport=transport), + ) + + assert result["status"] == "match" + assert result["record"]["zotero_attachment_status"] == "unavailable" + assert result["record"]["zotero_attachment_error"] == "zotero_server_error" + + +def test_lookup_preserves_parent_metadata_when_attachment_file_url_is_malformed() -> None: + routes = base_routes() + routes["/api/users/0/items/PARENT01"] = json_result(zotero_item()) + routes["/api/users/0/items/PARENT01/children"] = json_result([zotero_attachment()]) + routes["/api/users/0/items/ATTACH01/file/view/url"] = text_result( + "file://[invalid/paper.pdf" + ) + transport, _ = routed_transport(routes) + + result = lookup_zotero_local( + "PARENT01", + client=ZoteroLocalClient(transport=transport), + ) + + assert result["status"] == "match" + assert result["record"]["title"] == "Attention Is All You Need" + assert result["record"]["paper_id"] == "doi:10.5555/example" + assert result["record"]["zotero_attachment_status"] == "unavailable" + assert result["record"]["zotero_attachment_error"] == "zotero_unsafe_file_url" + + +def test_lookup_preserves_parent_metadata_when_remote_attachment_url_is_malformed( + tmp_path: Path, +) -> None: + routes = base_routes() + routes["/api/users/0/items/PARENT01"] = json_result(zotero_item()) + routes["/api/users/0/items/PARENT01/children"] = json_result( + [zotero_attachment(url="https://[invalid/paper.pdf")] + ) + routes["/api/users/0/items/ATTACH01/file/view/url"] = text_result( + (tmp_path / "missing.pdf").as_uri() + ) + transport, _ = routed_transport(routes) + + result = lookup_zotero_local( + "PARENT01", + client=ZoteroLocalClient(transport=transport), + ) + + assert result["status"] == "match" + assert result["record"]["title"] == "Attention Is All You Need" + assert result["record"]["paper_id"] == "doi:10.5555/example" + assert result["record"]["zotero_attachment_status"] == "unavailable" + assert result["record"]["zotero_attachment_error"] == "zotero_unsafe_file_url" + + +def test_lookup_reports_not_found_for_missing_key() -> None: + routes = base_routes() + routes["/api/users/0/items/MISSING1"] = text_result("", status=404) + transport, _ = routed_transport(routes) + + result = lookup_zotero_local( + "MISSING1", + client=ZoteroLocalClient(transport=transport), + ) + + assert result["status"] == "not_found" + assert result["error"]["code"] == "zotero_item_not_found" + + +def test_lookup_rejects_group_select_link_instead_of_dropping_library_scope() -> None: + transport, calls = routed_transport(base_routes()) + + result = lookup_zotero_local( + "zotero://select/groups/42/items/PARENT01", + client=ZoteroLocalClient(transport=transport), + ) + + assert result["status"] == "error" + assert result["error"]["code"] == "zotero_unsupported_library" + assert [call["path"] for call in calls] == ["/api/"]