diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 3a81b99..5cb520e 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "deeppapernote", "description": "Academic paper reading and Obsidian note generation skills for building a research WIKI.", - "version": "2.1.0", + "version": "2.1.1", "author": { "name": "917Dhj" }, diff --git a/.codex-plugin/plugin.json b/.codex-plugin/plugin.json index 96fdde0..35f67a2 100644 --- a/.codex-plugin/plugin.json +++ b/.codex-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "deeppapernote", - "version": "2.1.0", + "version": "2.1.1", "description": "Academic paper reading and Obsidian note generation skills for building a research WIKI.", "author": { "name": "917Dhj", diff --git a/CHANGELOG.md b/CHANGELOG.md index c227930..40be981 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,20 @@ Add an entry here when the project meaningfully changes for users, for example: ## Unreleased +## v2.1.1 + +### Added + +- Added a built-in, read-only Zotero Local API integration with local attachment discovery, explicit `auto` / `off` / `required` resolution policies, and environment diagnostics. + +### Fixed + +- Enforced a canonical paper-identity boundary across metadata collection, PDF acquisition, evidence extraction, and synthesis so conflicting or ambiguous identities fail closed. + +### Contributors + +- Added the Zotero Local API integration from PR #13 by [@KumamuKuma](https://github.com/KumamuKuma). + ## v2.1.0 ### Added diff --git a/README.md b/README.md index 13a87e1..7e80726 100644 --- a/README.md +++ b/README.md @@ -132,6 +132,16 @@ None of these are required for ordinary digital PDFs. | Semantic Scholar API | Improves metadata lookup for papers that are difficult to resolve | | OCR tooling | Recovers page text from scanned or low-quality PDFs | +### Zotero Local API + +The built-in, read-only Zotero Local API integration supports three resolution modes: + +- `auto` (default): prefer a unique local item and retain web fallback +- `off`: skip Zotero lookup +- `required`: stop unless Zotero uniquely resolves the reference + +An ambiguous local match always fails closed instead of selecting an arbitrary item. Compatible agent-runtime or MCP integrations remain optional alternatives. + When one of these capabilities is needed, ask your agent to inspect the current environment and guide the setup for that machine. ## 🧭 Inspirations diff --git a/README.zh-CN.md b/README.zh-CN.md index 230655d..2ff50ce 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -132,6 +132,16 @@ export DEEPPAPERNOTE_OBSIDIAN_VAULT="/你的/Obsidian/库/绝对路径" | Semantic Scholar API | 改善部分难以识别论文的元数据获取 | | OCR 工具 | 从扫描版或文本质量较差的 PDF 中恢复页面文字 | +### Zotero Local API + +内置的只读 Zotero Local API 集成提供三种解析模式： + +- `auto`（默认）：优先使用唯一的本地条目，同时保留联网回退 +- `off`：跳过 Zotero 查询 +- `required`：只有 Zotero 唯一解析出目标论文时才继续 + +本地匹配存在歧义时，流程会明确失败，而不是任意选择条目。Agent runtime 或 MCP 提供的兼容集成仍可作为可选替代路线。 + 实际需要其中某项能力时，可以让 Agent 检查当前环境，并针对这台机器引导配置。 ## 🧭 致谢与灵感 diff --git a/pyproject.toml b/pyproject.toml index b93e958..6860bf8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "deeppapernote" -version = "2.1.0" +version = "2.1.1" description = "An agent-neutral skill for turning one research paper into a high-quality Obsidian note." readme = "README.md" requires-python = ">=3.10" 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..ca7054f --- /dev/null +++ b/skills/deeppapernote/scripts/_zotero_local.py @@ -0,0 +1,864 @@ +#!/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 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_identity_title, + 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", ""))) + parent_url_error = "" + if source_url: + try: + parsed_source_url = _split_attachment_url(source_url) + except ZoteroLocalError as exc: + source_url = "" + parent_url_error = exc.code + else: + if parsed_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 + if parent_url_error: + record["zotero_parent_url_error"] = parent_url_error + 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: + return normalize_identity_title(title) + + +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/build_identity_contract.py b/skills/deeppapernote/scripts/build_identity_contract.py index e140c86..ed63436 100644 --- a/skills/deeppapernote/scripts/build_identity_contract.py +++ b/skills/deeppapernote/scripts/build_identity_contract.py @@ -18,30 +18,38 @@ def parser() -> argparse.ArgumentParser: p = argparse.ArgumentParser(description=__doc__ or "build identity contract") p.add_argument("--input", required=True, help="Metadata JSON path or JSON string.") - p.add_argument("--resolve", default="", help="Resolve artifact path for provenance.") + p.add_argument("--resolve", required=True, help="Resolve artifact path for provenance.") p.add_argument("--trace-output", required=True, help="Identity repair trace JSON output path.") p.add_argument("--output", required=True, help="Canonical Identity Artifact JSON output path.") return p -def load_required_record(value: str, consumer: str) -> dict: +def load_required_record(value: str, consumer: str, *, producer: str) -> dict: record = maybe_load_json_record(value) if record is None: raise SystemExit(f"{consumer} requires a JSON acquisition artifact input.") - return dict(require_ok_input_artifact(record, consumer)) - - -def load_optional_record(value: str, consumer: str) -> dict | None: - record = maybe_load_json_record(value) - if record is None: - return None - return dict(require_ok_input_artifact(record, consumer)) + accepted = dict(require_ok_input_artifact(record, consumer)) + if str(accepted.get("script", "")) != producer: + raise SystemExit(f"{consumer} requires a {producer} producer artifact.") + return accepted def main() -> None: args = parser().parse_args() - metadata = load_required_record(args.input, "build_identity_contract.py") - source_record = load_optional_record(args.resolve, "build_identity_contract.py") + metadata = load_required_record( + args.input, + "build_identity_contract.py", + producer="collect_metadata.py", + ) + if not isinstance(metadata.get("identity_observations"), list): + raise SystemExit( + "build_identity_contract.py requires metadata identity_observations." + ) + source_record = load_required_record( + args.resolve, + "build_identity_contract.py", + producer="resolve_paper.py", + ) trace = build_identity_repair_trace( metadata, diff --git a/skills/deeppapernote/scripts/build_synthesis_bundle.py b/skills/deeppapernote/scripts/build_synthesis_bundle.py index f20cf38..3779ddd 100644 --- a/skills/deeppapernote/scripts/build_synthesis_bundle.py +++ b/skills/deeppapernote/scripts/build_synthesis_bundle.py @@ -8,7 +8,12 @@ from pathlib import Path from citation_links import resolve_reference_links -from common import maybe_load_json_record, normalize_whitespace, runtime_config +from common import ( + canonical_identity_acceptance_error, + maybe_load_json_record, + normalize_whitespace, + runtime_config, +) from contracts import ( NOTE_PLAN_REQUIRED_FIELDS, NOTE_REQUIRED_SECTIONS, @@ -239,10 +244,14 @@ def identity_contract_summary(metadata: dict, source_manifest: dict) -> dict: return {} return { "artifact_type": contract.get("artifact_type", ""), + "schema_version": contract.get("schema_version", 1), "paper_id": contract.get("paper_id", ""), "identity_verdict": contract.get("identity_verdict", ""), "work_level_identity": deepcopy(contract.get("work_level_identity", {}) or {}), "source_manifestation": deepcopy(contract.get("source_manifestation", {}) or {}), + "accepted_metadata": deepcopy(contract.get("accepted_metadata", {}) or {}), + "bound_sources": deepcopy(contract.get("bound_sources", []) or []), + "field_provenance": deepcopy(contract.get("field_provenance", {}) or {}), "selected_identity_evidence": deepcopy( contract.get("selected_identity_evidence", []) or [] ), @@ -253,6 +262,22 @@ def identity_contract_summary(metadata: dict, source_manifest: dict) -> dict: } +def accepted_bundle_metadata(metadata: dict, identity_contract: dict) -> dict: + if not identity_contract: + raise ValueError("synthesis requires an accepted canonical identity contract") + error = canonical_identity_acceptance_error(identity_contract) + if error: + raise ValueError(f"synthesis requires an accepted canonical identity: {error}") + accepted = deepcopy(identity_contract.get("accepted_metadata", {}) or {}) + work_identity = identity_contract.get("work_level_identity", {}) or {} + if not accepted: + accepted = deepcopy(work_identity) + for key, value in work_identity.items(): + if value not in ("", None, [], {}): + accepted[key] = deepcopy(value) + return accepted + + def figure_table_manifest( figure_decisions_wrapper: dict, source_manifest: dict, @@ -394,31 +419,43 @@ def bundle( ) source_manifest = source_manifest or {} figure_decisions_wrapper = figure_decisions_wrapper or {} + identity_contract = identity_contract_summary(metadata, source_manifest) + canonical_metadata = accepted_bundle_metadata(metadata, identity_contract) return { "status": "ok", "script": "build_synthesis_bundle.py", - "paper_id": metadata.get("paper_id") or evidence_wrapper.get("paper_id", ""), - "title": metadata.get("title") or evidence_wrapper.get("title", ""), - "identity_contract": identity_contract_summary(metadata, source_manifest), + "paper_id": identity_contract.get("paper_id") + or canonical_metadata.get("paper_id") + or evidence_wrapper.get("paper_id", ""), + "title": canonical_metadata.get("title") or evidence_wrapper.get("title", ""), + "identity_contract": identity_contract, "metadata": { - "title": metadata.get("title", ""), - "translated_title": metadata.get("translated_title", ""), - "authors": metadata.get("authors", []), - "affiliations": metadata.get("affiliations", []), - "year": metadata.get("year", ""), - "venue": metadata.get("venue", ""), - "doi": metadata.get("doi", ""), - "source_url": metadata.get("source_url", ""), - "abstract": metadata.get("abstract", ""), - "arxiv_id": metadata.get("arxiv_id", ""), - "zotero_key": metadata.get("zotero_key", ""), - "metadata_sources": metadata.get("metadata_sources", []), - "identity_confidence": metadata.get("identity_confidence", ""), - "identity_confidence_reasons": metadata.get("identity_confidence_reasons", []) or [], + "title": canonical_metadata.get("title", ""), + "translated_title": canonical_metadata.get("translated_title", ""), + "authors": canonical_metadata.get("authors", []), + "affiliations": canonical_metadata.get("affiliations", []), + "year": canonical_metadata.get("year", ""), + "venue": canonical_metadata.get("venue", ""), + "doi": canonical_metadata.get("doi", ""), + "source_url": canonical_metadata.get("source_url", ""), + "abstract": canonical_metadata.get("abstract", ""), + "arxiv_id": canonical_metadata.get("arxiv_id", ""), + "zotero_key": canonical_metadata.get("zotero_key", ""), + "metadata_sources": canonical_metadata.get("metadata_sources", []), + "identity_confidence": canonical_metadata.get("identity_confidence", ""), + "identity_confidence_reasons": canonical_metadata.get( + "identity_confidence_reasons", [] + ) + or [], }, "evidence_quality": evidence_pack.get("evidence_quality", "unknown"), - "coverage": coverage_summary(evidence_pack, metadata, assets_wrapper, source_manifest), + "coverage": coverage_summary( + evidence_pack, + canonical_metadata, + assets_wrapper, + source_manifest, + ), "source_manifest": { "paper_id": source_manifest.get("paper_id", ""), "title": source_manifest.get("title", ""), 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/collect_metadata.py b/skills/deeppapernote/scripts/collect_metadata.py index ad6b000..e099f4c 100644 --- a/skills/deeppapernote/scripts/collect_metadata.py +++ b/skills/deeppapernote/scripts/collect_metadata.py @@ -5,8 +5,8 @@ from common import ( base_parser, + collect_metadata_observations, emit, - enrich_metadata, maybe_load_json_record, paper_id_for_record, require_ok_input_artifact, @@ -27,7 +27,8 @@ def main() -> None: else: record = resolve_reference(args.input) - metadata = enrich_metadata(record) + metadata = dict(record) + metadata["identity_observations"] = collect_metadata_observations(record) metadata["paper_id"] = args.paper_id or metadata.get("paper_id") or paper_id_for_record(metadata) metadata["status"] = "ok" metadata["script"] = "collect_metadata.py" diff --git a/skills/deeppapernote/scripts/common.py b/skills/deeppapernote/scripts/common.py index f6c6b54..e3ecd10 100644 --- a/skills/deeppapernote/scripts/common.py +++ b/skills/deeppapernote/scripts/common.py @@ -4,19 +4,19 @@ from __future__ import annotations import argparse -from copy import deepcopy import hashlib import json import os import re import ssl +import unicodedata import urllib.parse import urllib.request import xml.etree.ElementTree as ET +from copy import deepcopy from pathlib import Path, PureWindowsPath from typing import Any - ARXIV_NS = { "atom": "http://www.w3.org/2005/Atom", "arxiv": "http://arxiv.org/schemas/atom", @@ -26,6 +26,21 @@ OPENALEX_WORKS_URL = "https://api.openalex.org/works" CROSSREF_WORKS_URL = "https://api.crossref.org/works" DEFAULT_USER_AGENT = "DeepPaperNote/0.1" +PROVENANCE_FIELDS = ( + "title", + "translated_title", + "authors", + "affiliations", + "year", + "venue", + "doi", + "arxiv_id", + "zotero_key", + "abstract", + "source_url", + "pdf_url", + "local_pdf_path", +) SHELL_CONFIG_FILES = [ Path.home() / ".zshenv", Path.home() / ".zprofile", @@ -541,6 +556,40 @@ def _author_key(name: str) -> str: return parts[-1] +def normalize_identity_title(title: str) -> str: + return normalize_whitespace(unicodedata.normalize("NFKC", title).casefold()) + + +def _author_identity_parts(name: str) -> list[str]: + normalized = unicodedata.normalize("NFKC", normalize_whitespace(name)).casefold() + if "," in normalized: + family, given = normalized.split(",", 1) + normalized = f"{given} {family}" + return re.findall(r"[^\W_]+", normalized, flags=re.UNICODE) + + +def _full_leading_author_matches( + source_record: dict[str, Any], + work_record: dict[str, Any], +) -> bool: + source_authors = _dedupe_string_list(source_record.get("authors", [])) + work_authors = _dedupe_string_list(work_record.get("authors", [])) + if not source_authors or not work_authors: + return False + source_parts = _author_identity_parts(source_authors[0]) + work_parts = _author_identity_parts(work_authors[0]) + if len(source_parts) < 2 or len(work_parts) < 2: + return False + if source_parts[-1] != work_parts[-1]: + return False + source_given = source_parts[:-1] + work_given = work_parts[:-1] + return len(source_given) == len(work_given) and all( + left == right + for left, right in zip(source_given, work_given) + ) + + def _leading_author_status( source_record: dict[str, Any], work_record: dict[str, Any], @@ -583,7 +632,7 @@ def _shared_identifier_evidence( if source_doi and work_doi and source_doi == work_doi: shared.append({"kind": "shared_identifier", "value": f"doi:{work_doi}"}) - elif source_doi and work_doi and not (source_arxiv and source_arxiv == work_arxiv): + elif source_doi and work_doi: conflicts.append( { "kind": "identifier", @@ -1027,13 +1076,386 @@ def _warning_scoped_metadata_uncertainty( return warnings +def _identity_observation_record(observation: dict[str, Any]) -> dict[str, Any]: + record = observation.get("record") + if isinstance(record, dict): + return deepcopy(record) + return { + key: deepcopy(value) + for key, value in observation.items() + if key not in {"provider", "retrieved_by", "diagnostics", "relation"} + } + + +def _identity_observation_summary( + observation: dict[str, Any], + *, + status: str, + reason: str, +) -> dict[str, Any]: + record = _identity_observation_record(observation) + summary = { + "provider": _string_field(observation, "provider") + or _string_field(record, "source_type") + or "unknown", + "retrieved_by": deepcopy(observation.get("retrieved_by", {}) or {}), + "status": status, + "reason": reason, + "record": record, + } + for key in ("relation", "diagnostics"): + value = observation.get(key) + if value not in (None, "", [], {}): + summary[key] = deepcopy(value) + return summary + + +def _append_bound_source( + sources: list[dict[str, str]], + *, + kind: str, + value: str, + provider: str, + binding_reason: str, +) -> None: + cleaned = normalize_whitespace(value) + if not cleaned or any(item["kind"] == kind and item["value"] == cleaned for item in sources): + return + sources.append( + { + "kind": kind, + "value": cleaned, + "provider": provider or "unknown", + "binding_reason": binding_reason, + } + ) + + +def _bound_sources_from_record( + record: dict[str, Any], + *, + provider: str, + binding_reason: str, +) -> list[dict[str, str]]: + sources: list[dict[str, str]] = [] + local_pdf = _string_field(record, "local_pdf_path") + if local_pdf: + _append_bound_source( + sources, + kind="local_pdf", + value=_path_string(local_pdf), + provider=provider, + binding_reason=binding_reason, + ) + pdf_url = _string_field(record, "pdf_url") + if pdf_url: + _append_bound_source( + sources, + kind="pdf_url", + value=pdf_url, + provider=provider, + binding_reason=binding_reason, + ) + source_url = _string_field(record, "source_url") + if source_url.lower().endswith(".pdf"): + _append_bound_source( + sources, + kind="pdf_url", + value=source_url, + provider=provider, + binding_reason=binding_reason, + ) + arxiv_id = _record_arxiv_id(record) + if arxiv_id: + _append_bound_source( + sources, + kind="pdf_url", + value=f"https://arxiv.org/pdf/{arxiv_id}.pdf", + provider=provider, + binding_reason="accepted_identifier_derived", + ) + doi = extract_doi(_string_field(record, "doi")) or "" + if doi.lower().startswith("10.3389/"): + _append_bound_source( + sources, + kind="pdf_url", + value=f"https://www.frontiersin.org/articles/{doi}/pdf", + provider=provider, + binding_reason="accepted_identifier_derived", + ) + return sources + + +def _merge_admitted_observation( + accepted: dict[str, Any], + observation: dict[str, Any], + provider: str, +) -> dict[str, Any]: + merged = deepcopy(accepted) + for key, value in observation.items(): + if value in ("", None, [], {}): + continue + if key == "metadata_sources": + sources = _dedupe_string_list(merged.get(key, [])) + for source in _dedupe_string_list(value): + if source not in sources: + sources.append(source) + merged[key] = sources + continue + if merged.get(key) in ("", None, [], {}): + merged[key] = deepcopy(value) + sources = _dedupe_string_list(merged.get("metadata_sources", [])) + if provider != "unknown" and provider not in sources: + sources.append(provider) + if sources: + merged["metadata_sources"] = sources + merged["paper_id"] = _paper_id_for_effective_identity(merged) + return merged + + +def _is_unique_exact_zotero_title_observation( + anchor: dict[str, Any], + item: dict[str, Any], + observation: dict[str, Any], +) -> bool: + relation = item.get("relation") + if not isinstance(relation, dict): + return False + anchor_title = normalize_identity_title(_string_field(anchor, "title")) + observation_title = normalize_identity_title(_string_field(observation, "title")) + return bool( + _string_field(item, "provider").lower() == "zotero" + and _string_field(relation, "kind") == "zotero_lookup" + and _string_field(relation, "match_kind") == "title" + and _string_field(relation, "match_resolution") == "unique_exact" + and _string_field(observation, "zotero_key") + and anchor_title + and anchor_title == observation_title + ) + + +def adjudicate_identity_observations( + anchor: dict[str, Any], + observations: list[Any], +) -> dict[str, Any]: + accepted_metadata = deepcopy(anchor) + accepted_metadata.pop("identity_observations", None) + accepted_observations: list[dict[str, Any]] = [] + rejected_observations: list[dict[str, Any]] = [] + repair_attempts: list[dict[str, Any]] = [] + anchor_provider = _string_field(anchor, "source_type") or "input" + field_provenance = { + field: { + "provider": anchor_provider, + "retrieved_by": {"kind": "input", "value": _string_field(anchor, "paper_id")}, + } + for field in PROVENANCE_FIELDS + if anchor.get(field) not in ("", None, [], {}) + } + bound_sources = _bound_sources_from_record( + anchor, + provider=_string_field(anchor, "source_type") or "input", + binding_reason="trusted_input", + ) + + for item in observations: + if not isinstance(item, dict): + continue + observation = _identity_observation_record(item) + adjudication_anchor = accepted_metadata + shared_identifiers, identifier_conflicts = _shared_identifier_evidence( + adjudication_anchor, + observation, + ) + if identifier_conflicts: + summary = _identity_observation_summary( + item, + status="rejected", + reason="conflicting_strong_identifier", + ) + rejected_observations.append(summary) + repair_attempts.append( + { + "attempt": len(repair_attempts) + 1, + "action": "reject_conflicting_provider_observation", + "status": "rejected", + "evidence_used": _repair_evidence_used(adjudication_anchor), + "rejected_candidate_identity": work_level_identity_from_record( + observation + ), + "accepted_correction": work_level_identity_from_record( + adjudication_anchor + ), + "conflicting_anchors": identifier_conflicts, + } + ) + continue + anchor_year = _record_publication_year(adjudication_anchor) + observation_year = _record_publication_year(observation) + title_author_year_match = bool( + normalize_identity_title(_string_field(adjudication_anchor, "title")) + and normalize_identity_title(_string_field(adjudication_anchor, "title")) + == normalize_identity_title(_string_field(observation, "title")) + and _full_leading_author_matches(adjudication_anchor, observation) + and anchor_year + and anchor_year == observation_year + ) + unique_exact_zotero_title_match = _is_unique_exact_zotero_title_observation( + anchor, + item, + observation, + ) + if not title_author_year_match and not shared_identifiers: + observation_provider = ( + _string_field(item, "provider") + or _string_field(observation, "source_type") + ).lower() + for other_item in observations: + if other_item is item or not isinstance(other_item, dict): + continue + other = _identity_observation_record(other_item) + other_provider = ( + _string_field(other_item, "provider") + or _string_field(other, "source_type") + ).lower() + anchor_title = normalize_identity_title( + _string_field(anchor, "title") + ) + observation_title = normalize_identity_title( + _string_field(observation, "title") + ) + other_title = normalize_identity_title( + _string_field(other, "title") + ) + _, provider_conflicts = _shared_identifier_evidence( + observation, + other, + ) + if ( + observation_provider + and other_provider + and observation_provider != other_provider + and not provider_conflicts + and anchor_title + and anchor_title == observation_title == other_title + and _full_leading_author_matches(observation, other) + and observation_year + and observation_year == _record_publication_year(other) + ): + title_author_year_match = True + break + if ( + not shared_identifiers + and not title_author_year_match + and not unique_exact_zotero_title_match + ): + rejected_observations.append( + _identity_observation_summary( + item, + status="rejected", + reason="insufficient_equivalence_evidence", + ) + ) + continue + + if shared_identifiers: + acceptance_reason = "shared_identifier" + elif unique_exact_zotero_title_match: + acceptance_reason = "unique_exact_zotero_title" + else: + acceptance_reason = "title_author_year" + summary = _identity_observation_summary( + item, + status="accepted", + reason=acceptance_reason, + ) + accepted_observations.append(summary) + provider = summary["provider"] + observation.setdefault("metadata_sources", []) + if provider != "unknown" and provider not in observation["metadata_sources"]: + observation["metadata_sources"].append(provider) + previous_metadata = deepcopy(accepted_metadata) + accepted_metadata = _merge_admitted_observation( + accepted_metadata, + observation, + provider, + ) + retrieved_by = deepcopy(item.get("retrieved_by", {}) or {}) + for field in PROVENANCE_FIELDS: + value = observation.get(field) + if value in ("", None, [], {}): + continue + if accepted_metadata.get(field) != value: + continue + if previous_metadata.get(field) not in ("", None, [], {}): + continue + field_provenance[field] = { + "provider": provider, + "retrieved_by": retrieved_by, + } + for source in _bound_sources_from_record( + observation, + provider=provider, + binding_reason=acceptance_reason, + ): + _append_bound_source(sources=bound_sources, **source) + promoted_identifiers = { + key: _string_field(observation, key) + for key in ("doi", "arxiv_id", "zotero_key") + if not _string_field(previous_metadata, key) + and _string_field(observation, key) + } + if promoted_identifiers: + repair_attempts.append( + { + "attempt": len(repair_attempts) + 1, + "action": "accept_identity_promotion", + "status": "accepted", + "evidence_used": deepcopy(shared_identifiers) + or [ + { + "kind": "title_author_year", + "value": _string_field(observation, "title"), + "source": provider, + } + ], + "rejected_candidate_identity": work_level_identity_from_record( + previous_metadata + ), + "accepted_correction": work_level_identity_from_record( + accepted_metadata + ), + "promoted_identifiers": promoted_identifiers, + "previous_paper_id": _string_field(previous_metadata, "paper_id"), + "accepted_paper_id": _string_field(accepted_metadata, "paper_id"), + } + ) + + return { + "accepted_metadata": apply_identity_confidence(accepted_metadata), + "accepted_observations": accepted_observations, + "rejected_observations": rejected_observations, + "bound_sources": bound_sources, + "repair_attempts": repair_attempts, + "field_provenance": field_provenance, + } + + def identity_repair_decision( metadata: dict[str, Any], *, resolve_record: dict[str, Any] | None = None, ) -> dict[str, Any]: - effective = deepcopy(metadata) - repair_attempts: list[dict[str, Any]] = [] + observations = metadata.get("identity_observations", []) or [] + observation_decision = adjudicate_identity_observations( + resolve_record or metadata, + observations if isinstance(observations, list) else [], + ) + effective = observation_decision["accepted_metadata"] + for diagnostic_key in ("provider_unavailable", "provider_status"): + if metadata.get(diagnostic_key) not in (None, "", False): + effective[diagnostic_key] = deepcopy(metadata[diagnostic_key]) + repair_attempts = list(observation_decision["repair_attempts"]) identity_verdict = "accepted" failure_class = "" unresolved_risks: list[str] = [] @@ -1082,6 +1504,7 @@ def identity_repair_decision( if failure_class: unresolved_risks.append(failure_class) + bound_sources = list(observation_decision["bound_sources"]) return { "record": effective, "identity_verdict": identity_verdict, @@ -1089,6 +1512,11 @@ def identity_repair_decision( "repair_attempts": repair_attempts, "warnings": [], "unresolved_risks": unresolved_risks, + "accepted_metadata": deepcopy(effective), + "accepted_observations": observation_decision["accepted_observations"], + "rejected_observations": observation_decision["rejected_observations"], + "bound_sources": bound_sources, + "field_provenance": observation_decision["field_provenance"], } @@ -1163,7 +1591,10 @@ def identity_contract_state( decision = identity_repair_decision(metadata, resolve_record=source_record) effective = decision["record"] source_record = source_record or effective - paper_id = effective.get("paper_id") or paper_id_for_record(effective) + paper_id = _paper_id_for_effective_identity(effective) + effective["paper_id"] = paper_id + accepted_metadata = deepcopy(decision["accepted_metadata"]) + accepted_metadata["paper_id"] = paper_id equivalence_decision = manifestation_equivalence_decision(effective, source_record) identity_verdict = identity_verdict_from_equivalence(equivalence_decision) warnings = [] @@ -1203,6 +1634,11 @@ def identity_contract_state( "repair_attempts": repair_attempts, "failed": failed, "unresolved_risks": unresolved_risks, + "accepted_metadata": accepted_metadata, + "accepted_observations": decision["accepted_observations"], + "rejected_observations": decision["rejected_observations"], + "bound_sources": decision["bound_sources"], + "field_provenance": decision["field_provenance"], } @@ -1221,11 +1657,15 @@ def build_identity_repair_trace( "run_status": "failed" if failed else "ok", "script": "build_identity_contract.py", "artifact_type": "identity_repair_trace", - "schema_version": 1, + "schema_version": 2, "paper_id": state["paper_id"], "identity_verdict": state["identity_verdict"], "identity_failure_class": failure_class, "repair_attempts": state["repair_attempts"], + "accepted_observations": state["accepted_observations"], + "rejected_observations": state["rejected_observations"], + "bound_sources": state["bound_sources"], + "field_provenance": state["field_provenance"], "selected_identity_evidence": state["selected_identity_evidence"], "equivalence_decision": state["equivalence_decision"], "warnings": state["warnings"], @@ -1257,13 +1697,18 @@ def build_canonical_identity_artifact( "script": "build_identity_contract.py", "producer": "build_identity_contract.py", "artifact_type": "canonical_identity", - "schema_version": 1, + "schema_version": 2, "paper_id": state["paper_id"], "identity_verdict": state["identity_verdict"], "identity_failure_class": failure_class, "failure_summary": identity_failure_summary(failure_class) if failed else "", "work_level_identity": work_level_identity_from_record(effective), "source_manifestation": source_manifestation_from_record(source_record, effective), + "accepted_metadata": state["accepted_metadata"], + "bound_sources": state["bound_sources"], + "accepted_observations": state["accepted_observations"], + "rejected_observations": state["rejected_observations"], + "field_provenance": state["field_provenance"], "selected_identity_evidence": state["selected_identity_evidence"], "equivalence_decision": state["equivalence_decision"], "warnings": state["warnings"], @@ -1281,28 +1726,40 @@ def build_canonical_identity_artifact( } +def canonical_identity_acceptance_error(record: dict[str, Any]) -> str: + artifact_type = _string_field(record, "artifact_type") + if artifact_type != "canonical_identity": + return f"non-canonical identity artifact: {artifact_type}" + if record.get("schema_version") != 2: + return "canonical identity must use schema v2" + verdict = _string_field(record, "identity_verdict").lower().replace("-", "_") + if verdict not in {"accepted", "accepted_with_warnings"}: + return f"unaccepted canonical identity: identity_verdict={verdict}" + return "" + + def require_accepted_canonical_identity(record: dict[str, Any], consumer: str) -> dict[str, Any]: identity = dict(require_ok_input_artifact(record, consumer)) - artifact_type = _string_field(identity, "artifact_type") - verdict = _string_field(identity, "identity_verdict").lower() - if artifact_type != "canonical_identity": - raise SystemExit(f"{consumer} refuses non-canonical identity artifact: {artifact_type}") - normalized_verdict = verdict.replace("-", "_") - if normalized_verdict not in {"accepted", "accepted_with_warnings"}: - raise SystemExit( - f"{consumer} refuses unaccepted canonical identity: identity_verdict={verdict}" - ) + error = canonical_identity_acceptance_error(identity) + if error: + raise SystemExit(f"{consumer} refuses {error}") return identity def canonical_identity_summary(identity: dict[str, Any]) -> dict[str, Any]: return { "artifact_type": _string_field(identity, "artifact_type"), + "schema_version": identity.get("schema_version", 0), "paper_id": _string_field(identity, "paper_id"), "identity_verdict": _string_field(identity, "identity_verdict"), "work_level_identity": deepcopy(identity.get("work_level_identity", {}) or {}), "source_manifestation": deepcopy(identity.get("source_manifestation", {}) or {}), - "selected_identity_evidence": deepcopy(identity.get("selected_identity_evidence", []) or []), + "accepted_metadata": deepcopy(identity.get("accepted_metadata", {}) or {}), + "bound_sources": deepcopy(identity.get("bound_sources", []) or []), + "field_provenance": deepcopy(identity.get("field_provenance", {}) or {}), + "selected_identity_evidence": deepcopy( + identity.get("selected_identity_evidence", []) or [] + ), "equivalence_decision": deepcopy(identity.get("equivalence_decision", {}) or {}), "warnings": deepcopy(identity.get("warnings", []) or []), "repair_trace_path": _string_field(identity, "repair_trace_path"), @@ -1310,6 +1767,24 @@ def canonical_identity_summary(identity: dict[str, Any]) -> dict[str, Any]: } +def require_accepted_fetch_artifact( + record: dict[str, Any], + consumer: str, +) -> dict[str, Any]: + artifact = dict(require_ok_input_artifact(record, consumer)) + if _string_field(artifact, "script") != "fetch_pdf.py": + raise SystemExit(f"{consumer} requires a fetch_pdf.py artifact.") + identity = artifact.get("identity_contract") + if not isinstance(identity, dict): + raise SystemExit(f"{consumer} requires an embedded canonical identity contract.") + require_accepted_canonical_identity(identity, consumer) + canonical_paper_id = _string_field(identity, "paper_id") + artifact_paper_id = _string_field(artifact, "paper_id") + if not canonical_paper_id or artifact_paper_id != canonical_paper_id: + raise SystemExit(f"{consumer} requires fetch paper_id to match canonical identity.") + return artifact + + def fetch_record_from_canonical_identity(identity: dict[str, Any]) -> dict[str, Any]: work_identity = identity.get("work_level_identity", {}) or {} source_manifestation = identity.get("source_manifestation", {}) or {} @@ -1719,6 +2194,24 @@ 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 resolve_reference(value: str) -> dict[str, Any]: source_type = infer_source_type(value) stripped = (value or "").strip() @@ -1747,30 +2240,21 @@ def resolve_reference(value: str) -> dict[str, Any]: return apply_identity_confidence(paper) if source_type == "arxiv_id": arxiv_id = extract_arxiv_id(stripped) or "" - papers = safe_fetch_arxiv_entries(id_list=arxiv_id, max_results=1) - if papers: - paper = papers[0] - paper["paper_id"] = paper_id_for_record(paper) - paper["status"] = "ok" - return apply_identity_confidence(paper) if arxiv_id: return fallback_arxiv_record(arxiv_id, "arxiv_id") if source_type == "arxiv_url": arxiv_id = extract_arxiv_id(stripped) or "" - papers = safe_fetch_arxiv_entries(id_list=arxiv_id, max_results=1) - if papers: - paper = papers[0] - paper["paper_id"] = paper_id_for_record(paper) - paper["status"] = "ok" - return apply_identity_confidence(paper) if arxiv_id: return fallback_arxiv_record(arxiv_id, "arxiv_url", source_url=stripped) if source_type in {"doi", "doi_url"}: doi = extract_doi(stripped) or "" - paper = fetch_crossref_by_doi(doi) or {"doi": doi, "source_url": f"https://doi.org/{doi}"} - paper["source_type"] = "doi" - paper["source_url"] = paper.get("source_url") or f"https://doi.org/{doi}" - paper["status"] = "ok" + paper = { + "status": "ok", + "source_type": "doi", + "source_url": f"https://doi.org/{doi}", + "doi": doi, + "metadata_sources": ["doi"], + } paper["paper_id"] = paper_id_for_record(paper) return apply_identity_confidence(paper) if source_type == "pdf_url": @@ -1809,17 +2293,6 @@ def resolve_reference(value: str) -> dict[str, Any]: return apply_identity_confidence(paper) title = stripped - candidates = ( - search_semantic_scholar(title, limit=5) - + search_crossref_by_title(title, limit=5) - + search_openalex_by_title(title, limit=5) - + safe_fetch_arxiv_entries(search_query=f'ti:"{title}"', max_results=5) - ) - best = choose_best_title_match(title, candidates) - if best: - best = merge_metadata_records({"title": title, "source_type": "title_query", "source_url": "", "metadata_sources": ["title_query"]}, best) - best["status"] = "ok" - return apply_identity_confidence(best) paper = { "status": "ok", "source_type": "title_query", @@ -1831,44 +2304,61 @@ def resolve_reference(value: str) -> dict[str, Any]: return apply_identity_confidence(paper) -def enrich_metadata(record: dict[str, Any]) -> dict[str, Any]: +def collect_metadata_observations(record: dict[str, Any]) -> list[dict[str, Any]]: base = dict(record) - candidates: list[dict[str, Any]] = [base] + observations = [ + deepcopy(item) + for item in base.get("identity_observations", []) or [] + if isinstance(item, dict) + ] doi = normalize_whitespace(str(base.get("doi", ""))) title = normalize_whitespace(str(base.get("title", ""))) arxiv_id = normalize_whitespace(str(base.get("arxiv_id", ""))) + def append(provider: str, kind: str, value: str, candidate: dict[str, Any] | None) -> None: + if not candidate: + return + observation = { + "provider": provider, + "retrieved_by": {"kind": kind, "value": value}, + "record": deepcopy(candidate), + } + if observation not in observations: + observations.append(observation) + if doi: crossref = fetch_crossref_by_doi(doi) - if crossref: - candidates.append(crossref) + append("crossref", "doi", doi, crossref) openalex = fetch_openalex_by_doi(doi) - if openalex: - candidates.append(openalex) + append("openalex", "doi", doi, openalex) sem = choose_best_title_match(title or doi, search_semantic_scholar(doi, limit=3)) - if sem: - candidates.append(sem) + append("semantic_scholar", "doi", doi, sem) if arxiv_id: arxiv = safe_fetch_arxiv_entries(id_list=arxiv_id, max_results=1) if arxiv: - candidates.append(arxiv[0]) + append("arxiv", "arxiv_id", arxiv_id, arxiv[0]) if title: sem = choose_best_title_match(title, search_semantic_scholar(title, limit=5)) - if sem: - candidates.append(sem) + append("semantic_scholar", "title", title, sem) oa = choose_best_title_match(title, search_openalex_by_title(title, limit=5)) - if oa: - candidates.append(oa) + append("openalex", "title", title, oa) cross = choose_best_title_match(title, search_crossref_by_title(title, limit=5)) - if cross: - candidates.append(cross) - arxiv = choose_best_title_match(title, safe_fetch_arxiv_entries(search_query=f'ti:"{title}"', max_results=5)) - if arxiv: - candidates.append(arxiv) + append("crossref", "title", title, cross) + arxiv = choose_best_title_match( + title, + safe_fetch_arxiv_entries(search_query=f'ti:"{title}"', max_results=5), + ) + append("arxiv", "title", title, arxiv) - merged = merge_metadata_records(*candidates) + return observations + + +def enrich_metadata(record: dict[str, Any]) -> dict[str, Any]: + base = dict(record) + observations = collect_metadata_observations(base) + merged = adjudicate_identity_observations(base, observations)["accepted_metadata"] if not merged.get("year") and merged.get("published") and re.match(r"^\d{4}", str(merged["published"])): merged["year"] = str(merged["published"])[:4] if merged.get("doi") and not merged.get("source_url"): @@ -1877,17 +2367,6 @@ def enrich_metadata(record: dict[str, Any]) -> dict[str, Any]: merged["pdf_url"] = f"https://arxiv.org/pdf/{merged['arxiv_id']}.pdf" if merged.get("arxiv_id") and not merged.get("doi"): merged["doi"] = f"10.48550/arXiv.{merged['arxiv_id']}" - if base.get("source_type") == "local_pdf": - if base.get("local_pdf_title_source"): - merged["local_pdf_title_source"] = base["local_pdf_title_source"] - elif is_probable_local_pdf_artifact_title(str(base.get("title", ""))): - merged["local_pdf_title_source"] = "local_pdf_stem_used" - if base.get("local_pdf_artifact_title") or is_probable_local_pdf_artifact_title(str(base.get("title", ""))): - merged["local_pdf_artifact_title"] = True - corrected_title = choose_local_pdf_corrected_title(base, candidates[1:]) - if corrected_title: - merged["title"] = corrected_title - merged["title_corrected_from_external_metadata"] = True merged["paper_id"] = paper_id_for_record(merged) return apply_identity_confidence(merged) diff --git a/skills/deeppapernote/scripts/extract_evidence.py b/skills/deeppapernote/scripts/extract_evidence.py index 457b222..93e2167 100644 --- a/skills/deeppapernote/scripts/extract_evidence.py +++ b/skills/deeppapernote/scripts/extract_evidence.py @@ -11,7 +11,6 @@ from common import ( SECTION_ALIASES, emit, - enrich_metadata, extract_appendix_index, extract_appendix_page_texts, extract_caption_lines, @@ -28,7 +27,7 @@ paper_id_for_record, pick_sentences_by_keywords, pdf_coverage_summary, - resolve_reference, + require_accepted_fetch_artifact, split_sentences, ) from contracts import empty_evidence_pack @@ -49,10 +48,9 @@ def parser() -> argparse.ArgumentParser: p = argparse.ArgumentParser(description=__doc__ or "extract evidence") - p.add_argument("--input", default="", help="Metadata JSON path, fetch_pdf JSON path, JSON string, or raw paper reference.") + p.add_argument("--input", default="", help="Successful fetch JSON path or JSON artifact.") p.add_argument("--source-manifest", default="", help="Source Corpus source_manifest.json path.") p.add_argument("--output", default="", help="Output JSON path.") - p.add_argument("--paper-id", default="", help="Canonical paper id if already known.") p.add_argument("--max-pages", type=int, default=32, help="Maximum number of PDF pages to scan.") p.add_argument("--max-chunks-per-section", type=int, default=12, help="Maximum number of candidate chunks to keep per section.") return p @@ -60,9 +58,9 @@ def parser() -> argparse.ArgumentParser: def ensure_record(input_value: str) -> dict: record = maybe_load_json_record(input_value) - if record is not None: - return dict(record) - return enrich_metadata(resolve_reference(input_value)) + if record is None: + raise SystemExit("extract_evidence.py requires a JSON acquisition artifact.") + return dict(require_accepted_fetch_artifact(record, "extract_evidence.py")) def build_items(sentences: list[str], section: str) -> list[dict]: @@ -834,7 +832,7 @@ def main() -> None: ) or [chunk["text"] for chunk in candidates.get("conclusion", [])[:3]] pack = empty_evidence_pack() - pack["paper_id"] = args.paper_id or record.get("paper_id") or paper_id_for_record(record) + pack["paper_id"] = record.get("paper_id") or paper_id_for_record(record) pack["problem_evidence"] = build_items(problem_sentences, "introduction") pack["task_evidence"] = build_items(task_sentences, "task") pack["data_evidence"] = build_items(data_sentences, "data") diff --git a/skills/deeppapernote/scripts/extract_pdf_assets.py b/skills/deeppapernote/scripts/extract_pdf_assets.py index f176395..25126e5 100644 --- a/skills/deeppapernote/scripts/extract_pdf_assets.py +++ b/skills/deeppapernote/scripts/extract_pdf_assets.py @@ -20,7 +20,14 @@ import re from pathlib import Path -from common import default_assets_dir, emit, enrich_metadata, fitz, maybe_load_json_record, normalize_whitespace, resolve_reference +from common import ( + default_assets_dir, + emit, + fitz, + maybe_load_json_record, + normalize_whitespace, + require_accepted_fetch_artifact, +) try: from PIL import Image # type: ignore @@ -223,7 +230,7 @@ def _classify_caption_kind(label: str) -> str: def parser() -> argparse.ArgumentParser: p = argparse.ArgumentParser(description=__doc__ or "extract pdf assets") - p.add_argument("--input", required=True, help="Fetch JSON path, metadata JSON path, JSON string, or raw paper reference.") + p.add_argument("--input", required=True, help="Successful fetch JSON path or JSON artifact.") p.add_argument("--output", default="", help="Output JSON path.") p.add_argument("--assets-dir", default="", help="Optional explicit assets directory.") p.add_argument("--max-pages", type=int, default=40, help="Maximum pages to scan.") @@ -235,9 +242,9 @@ def parser() -> argparse.ArgumentParser: def ensure_record(input_value: str) -> dict: record = maybe_load_json_record(input_value) - if record is not None: - return dict(record) - return enrich_metadata(resolve_reference(input_value)) + if record is None: + raise SystemExit("extract_pdf_assets.py requires a JSON acquisition artifact.") + return dict(require_accepted_fetch_artifact(record, "extract_pdf_assets.py")) def save_image_bytes(path: Path, data: bytes) -> None: diff --git a/skills/deeppapernote/scripts/extract_source_text.py b/skills/deeppapernote/scripts/extract_source_text.py index 54f8d14..7c6005e 100644 --- a/skills/deeppapernote/scripts/extract_source_text.py +++ b/skills/deeppapernote/scripts/extract_source_text.py @@ -13,7 +13,6 @@ from common import ( clean_pdf_line, emit, - enrich_metadata, ensure_parent, extract_appendix_index, extract_caption_lines, @@ -24,7 +23,7 @@ normalize_whitespace, paper_id_for_record, pdf_coverage_summary, - resolve_reference, + require_accepted_fetch_artifact, stop_section_reason, ) @@ -38,7 +37,7 @@ def parser() -> argparse.ArgumentParser: p.add_argument( "--input", required=True, - help="Fetch JSON path, metadata JSON path, local PDF path, JSON string, or reference.", + help="Successful fetch JSON path or JSON artifact.", ) p.add_argument("--output", default="", help="Source manifest JSON output path.") p.add_argument( @@ -47,7 +46,6 @@ def parser() -> argparse.ArgumentParser: help="Canonical raw sections JSONL output path.", ) p.add_argument("--full-text-output", default="", help="Optional derived Markdown output path.") - p.add_argument("--paper-id", default="", help="Canonical paper id if already known.") p.add_argument( "--max-pages", type=int, @@ -59,17 +57,9 @@ def parser() -> argparse.ArgumentParser: def ensure_record(input_value: str) -> dict[str, Any]: record = maybe_load_json_record(input_value) - if record is not None: - return dict(record) - path = Path(input_value).expanduser() - if path.exists() and path.is_file() and path.suffix.lower() == ".pdf": - return { - "paper_id": f"local:{path.stem}", - "title": path.stem, - "pdf_path": str(path.resolve()), - "source_type": "local_pdf", - } - return enrich_metadata(resolve_reference(input_value)) + if record is None: + raise SystemExit("extract_source_text.py requires a JSON acquisition artifact.") + return dict(require_accepted_fetch_artifact(record, "extract_source_text.py")) def resolve_pdf_path(record: dict[str, Any]) -> Path | None: @@ -359,7 +349,7 @@ def build_manifest( def main() -> None: args = parser().parse_args() record = ensure_record(args.input) - record["paper_id"] = args.paper_id or record.get("paper_id") or paper_id_for_record(record) + record["paper_id"] = record.get("paper_id") or paper_id_for_record(record) pdf_path = resolve_pdf_path(record) if pdf_path is None: raise SystemExit("extract_source_text.py requires a resolvable local PDF path.") diff --git a/skills/deeppapernote/scripts/fetch_pdf.py b/skills/deeppapernote/scripts/fetch_pdf.py index dc82610..4329a89 100644 --- a/skills/deeppapernote/scripts/fetch_pdf.py +++ b/skills/deeppapernote/scripts/fetch_pdf.py @@ -10,15 +10,12 @@ canonical_identity_summary, default_pdf_path, emit, - enrich_metadata, - extract_doi, fetch_record_from_canonical_identity, http_get_bytes, maybe_load_json_record, paper_id_for_record, require_accepted_canonical_identity, require_ok_input_artifact, - resolve_reference, ) @@ -26,7 +23,6 @@ def parser() -> argparse.ArgumentParser: p = argparse.ArgumentParser(description=__doc__ or "fetch pdf") p.add_argument("--input", required=True, help="Metadata JSON path, JSON string, or raw paper reference.") p.add_argument("--output", default="", help="Output path for JSON status.") - p.add_argument("--paper-id", default="", help="Canonical paper id if already known.") p.add_argument("--dest-dir", default="", help="Directory for downloaded PDFs.") p.add_argument( "--identity", @@ -40,83 +36,35 @@ def is_pdf_content(data: bytes) -> bool: return b"%PDF-" in data[:1024] -def frontiers_pdf_url_from_doi(doi: str) -> str: - normalized = extract_doi(doi) or "" - if not normalized.lower().startswith("10.3389/"): - return "" - return f"https://www.frontiersin.org/articles/{normalized}/pdf" - - -def append_candidate(candidates: list[tuple[str, str]], kind: str, value: str) -> None: - cleaned = value.strip() - if cleaned and (kind, cleaned) not in candidates: - candidates.append((kind, cleaned)) - - -def choose_pdf_source(record: dict) -> tuple[str, str]: - candidates = pdf_source_candidates(record) - return candidates[0] if candidates else ("", "") - - -def pdf_source_candidates(record: dict) -> list[tuple[str, str]]: - candidates: list[tuple[str, str]] = [] - local_pdf = str(record.get("local_pdf_path", "")).strip() - if local_pdf and Path(local_pdf).expanduser().exists(): - return [("local_pdf", str(Path(local_pdf).expanduser().resolve()))] - - pdf_url = str(record.get("pdf_url", "")).strip() - if pdf_url: - append_candidate(candidates, "pdf_url", pdf_url) - - source_url = str(record.get("source_url", "")).strip() - if source_url.lower().endswith(".pdf"): - append_candidate(candidates, "pdf_url", source_url) - - arxiv_id = str(record.get("arxiv_id", "")).strip() - if arxiv_id: - append_candidate(candidates, "pdf_url", f"https://arxiv.org/pdf/{arxiv_id}.pdf") - - doi = extract_doi(str(record.get("doi", "")).strip()) - if doi: - if not candidates: - enriched = enrich_metadata({"doi": doi, "title": record.get("title", "")}) - enriched_pdf = str(enriched.get("pdf_url", "")).strip() - if enriched_pdf: - append_candidate(candidates, "pdf_url", enriched_pdf) - frontiers_pdf = frontiers_pdf_url_from_doi(doi) - if frontiers_pdf: - append_candidate(candidates, "pdf_url", frontiers_pdf) - - return candidates - - def main(argv: list[str] | None = None) -> None: args = parser().parse_args(argv) + if not args.identity: + raise SystemExit("fetch_pdf.py requires an accepted --identity artifact.") identity_summary: dict = {} source_manifestation: dict = {} - if args.identity: - input_record = maybe_load_json_record(args.input) - if input_record is not None: - require_ok_input_artifact(input_record, "fetch_pdf.py") - identity_record = maybe_load_json_record(args.identity) - if identity_record is None: - raise SystemExit("fetch_pdf.py requires --identity to be a JSON artifact.") - identity = require_accepted_canonical_identity(identity_record, "fetch_pdf.py") - identity_summary = canonical_identity_summary(identity) - source_manifestation = dict(identity_summary.get("source_manifestation", {}) or {}) - record = fetch_record_from_canonical_identity(identity) - else: - input_record = maybe_load_json_record(args.input) - if input_record is not None: - record = dict(require_ok_input_artifact(input_record, "fetch_pdf.py")) - else: - record = enrich_metadata(resolve_reference(args.input)) - - record["paper_id"] = args.paper_id or record.get("paper_id") or paper_id_for_record(record) - source_candidates = pdf_source_candidates(record) - source_kind, source_value = source_candidates[0] if source_candidates else ("", "") - - if not source_kind: + input_record = maybe_load_json_record(args.input) + if input_record is not None: + require_ok_input_artifact(input_record, "fetch_pdf.py") + identity_record = maybe_load_json_record(args.identity) + if identity_record is None: + raise SystemExit("fetch_pdf.py requires --identity to be a JSON artifact.") + identity = require_accepted_canonical_identity(identity_record, "fetch_pdf.py") + if not isinstance(identity.get("bound_sources"), list): + raise SystemExit("fetch_pdf.py requires canonical identity bound_sources.") + identity_summary = canonical_identity_summary(identity) + source_manifestation = dict(identity_summary.get("source_manifestation", {}) or {}) + record = fetch_record_from_canonical_identity(identity) + + record["paper_id"] = record.get("paper_id") or paper_id_for_record(record) + source_candidates = [ + (str(item.get("kind", "")), str(item.get("value", ""))) + for item in identity.get("bound_sources", []) or [] + if isinstance(item, dict) + and item.get("kind") in {"local_pdf", "pdf_url"} + and str(item.get("value", "")).strip() + ] + + if not source_candidates: payload = { "status": "error", "script": "fetch_pdf.py", @@ -131,28 +79,38 @@ def main(argv: list[str] | None = None) -> None: emit(payload, args.output) raise SystemExit(1) - if source_kind == "local_pdf": - pdf_path = Path(source_value) - payload = { - "status": "ok", - "script": "fetch_pdf.py", - "paper_id": record["paper_id"], - "title": record.get("title", ""), - "pdf_path": str(pdf_path), - "pdf_source": "local_pdf", - "source_url": record.get("source_url", "") or str(pdf_path), - "pdf_url": "", - } - if identity_summary: - payload["identity_contract"] = identity_summary - payload["source_manifestation"] = source_manifestation - emit(payload, args.output) - return - - target_path = default_pdf_path(record, dest_dir=args.dest_dir) attempted_sources: list[dict[str, str]] = [] downloaded: tuple[str, str, bytes] | None = None for candidate_kind, candidate_value in source_candidates: + if candidate_kind == "local_pdf": + pdf_path = Path(candidate_value).expanduser() + if not pdf_path.exists() or not pdf_path.is_file(): + attempted_sources.append( + { + "kind": "local_pdf", + "path": candidate_value, + "status": "missing_file", + } + ) + continue + pdf_path = pdf_path.resolve() + payload = { + "status": "ok", + "script": "fetch_pdf.py", + "paper_id": record["paper_id"], + "title": record.get("title", ""), + "pdf_path": str(pdf_path), + "pdf_source": "local_pdf", + "source_url": record.get("source_url", "") or str(pdf_path), + "pdf_url": "", + } + if identity_summary: + payload["identity_contract"] = identity_summary + payload["source_manifestation"] = source_manifestation + if attempted_sources: + payload["attempted_sources"] = attempted_sources + emit(payload, args.output) + return if candidate_kind != "pdf_url": continue try: @@ -186,7 +144,8 @@ def main(argv: list[str] | None = None) -> None: emit(payload, args.output) raise SystemExit(1) - source_kind, source_value, pdf_bytes = downloaded + target_path = default_pdf_path(record, dest_dir=args.dest_dir) + _, source_value, pdf_bytes = downloaded target_path.write_bytes(pdf_bytes) payload = { "status": "ok", diff --git a/skills/deeppapernote/scripts/resolve_paper.py b/skills/deeppapernote/scripts/resolve_paper.py index ded3f62..a98acba 100644 --- a/skills/deeppapernote/scripts/resolve_paper.py +++ b/skills/deeppapernote/scripts/resolve_paper.py @@ -3,19 +3,170 @@ 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: + if explicit_zotero_reference: + resolved = dict(record) + resolved["zotero_lookup"] = zotero_lookup_summary( + lookup, + mode=zotero_mode, + ) + return resolved + resolved = resolve_reference(reference) + resolved["identity_observations"] = [ + { + "provider": "zotero", + "retrieved_by": {"kind": source_type, "value": reference.strip()}, + "relation": { + "kind": "zotero_lookup", + "match_kind": str(lookup.get("match_kind", source_type)), + "match_resolution": "unique_exact", + }, + "record": 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 +175,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_acquisition_artifacts.py b/tests/test_acquisition_artifacts.py index e084af9..bc9da64 100644 --- a/tests/test_acquisition_artifacts.py +++ b/tests/test_acquisition_artifacts.py @@ -6,6 +6,10 @@ import build_identity_contract import collect_metadata +import common +import extract_evidence +import extract_pdf_assets +import extract_source_text import fetch_pdf import pytest @@ -32,6 +36,8 @@ def build_identity_from_payloads( metadata_payload: dict, expect_failure: bool = False, ) -> tuple[dict, dict]: + metadata_payload = dict(metadata_payload) + metadata_payload.setdefault("identity_observations", []) resolve_path = tmp_path / "paper_resolve.json" metadata_path = tmp_path / "paper_metadata.json" identity_path = tmp_path / "paper_identity.json" @@ -93,15 +99,23 @@ def test_build_identity_contract_emits_accepted_artifact_and_trace( "status": "ok", "script": "collect_metadata.py", "paper_id": "doi:10.1234/example", - "title": "Canonical Metadata Title", - "authors": ["A. Author", "B. Author"], - "year": "2026", - "venue": "Journal of Tests", - "doi": "10.1234/example", - "pdf_url": "https://example.test/paper.pdf", - "source_url": "https://doi.org/10.1234/example", - "identity_confidence": "high", - "identity_confidence_reasons": ["doi_present"], + "identity_observations": [ + { + "provider": "crossref", + "retrieved_by": { + "kind": "doi", + "value": "10.1234/example", + }, + "record": { + "title": "Canonical Metadata Title", + "authors": ["A. Author", "B. Author"], + "year": "2026", + "venue": "Journal of Tests", + "doi": "10.1234/example", + "pdf_url": "https://example.test/paper.pdf", + }, + } + ], } ), encoding="utf-8", @@ -130,9 +144,10 @@ def test_build_identity_contract_emits_accepted_artifact_and_trace( assert identity["status"] == "ok" assert identity["artifact_type"] == "canonical_identity" assert identity["identity_verdict"] == "accepted" - assert identity["work_level_identity"]["title"] == "Canonical Metadata Title" + assert identity["work_level_identity"]["title"] == "Original Resolve Title" assert identity["work_level_identity"]["doi"] == "10.1234/example" - assert identity["source_manifestation"]["pdf_url"] == "https://example.test/paper.pdf" + assert identity["accepted_metadata"]["authors"] == ["A. Author", "B. Author"] + assert identity["bound_sources"][0]["value"] == "https://example.test/paper.pdf" assert identity["warnings"] == [] assert identity["repair_trace_path"] == str(trace_path.resolve()) assert identity["provenance"]["resolve_artifact_path"] == str(resolve_path.resolve()) @@ -185,25 +200,12 @@ def test_build_identity_contract_repairs_noisy_first_page_title( ) assert identity["identity_verdict"] == "accepted" - assert identity["work_level_identity"]["title"] == "Canonical DOI Title For Testing" - assert identity["work_level_identity"]["doi"] == "10.1234/canonical" - assert identity["source_manifestation"]["local_pdf_path"] == str(pdf_path.resolve()) - assert len(trace["repair_attempts"]) == 1 - attempt = trace["repair_attempts"][0] - assert attempt["action"] == "replace_challengeable_identity_anchor" - assert ( - attempt["replacement_reason"] - == "challengeable_first_page_title_replaced_by_stronger_identity_evidence" - ) - assert ( - attempt["rejected_candidate_identity"]["title"] - == "A Noisy Cover Sheet Title For Testing" - ) - assert attempt["accepted_correction"]["title"] == "Canonical DOI Title For Testing" - assert any( - item["kind"] == "doi" and item["value"] == "10.1234/canonical" - for item in attempt["evidence_used"] + assert identity["work_level_identity"]["title"] == ( + "A Noisy Cover Sheet Title For Testing" ) + assert identity["work_level_identity"]["doi"] == "" + assert identity["source_manifestation"]["local_pdf_path"] == str(pdf_path.resolve()) + assert trace["repair_attempts"] == [] def test_build_identity_contract_repairs_filename_only_title_with_arxiv( @@ -246,21 +248,11 @@ def test_build_identity_contract_repairs_filename_only_title_with_arxiv( }, ) - assert identity["work_level_identity"]["title"] == "Authoritative arXiv Repair Title" - assert identity["work_level_identity"]["arxiv_id"] == "2401.00001" - attempt = trace["repair_attempts"][0] - assert ( - attempt["replacement_reason"] - == "challengeable_filename_title_replaced_by_stronger_identity_evidence" - ) - assert ( - attempt["rejected_candidate_identity"]["title"] - == "Smith 等 - 2024 - Noisy Local Filename-123456" - ) - assert any( - item["kind"] == "arxiv_id" and item["value"] == "2401.00001" - for item in attempt["evidence_used"] + assert identity["work_level_identity"]["title"] == ( + "Smith 等 - 2024 - Noisy Local Filename-123456" ) + assert identity["work_level_identity"]["arxiv_id"] == "" + assert trace["repair_attempts"] == [] def test_build_identity_contract_repairs_blank_challengeable_title( @@ -291,17 +283,11 @@ def test_build_identity_contract_repairs_blank_challengeable_title( "identity_confidence": "high", "identity_confidence_reasons": ["doi_present"], }, + expect_failure=True, ) - assert identity["work_level_identity"]["title"] == "Authoritative Metadata Filled Title" - assert identity["work_level_identity"]["doi"] == "10.5555/blank" - attempt = trace["repair_attempts"][0] - assert ( - attempt["replacement_reason"] - == "blank_challengeable_title_filled_by_stronger_identity_evidence" - ) - assert attempt["rejected_candidate_identity"]["title"] == "" - assert attempt["accepted_correction"]["title"] == "Authoritative Metadata Filled Title" + assert identity["identity_failure_class"] == "insufficient_evidence" + assert trace["repair_attempts"][-1]["action"] == "repair_exhausted_fail_closed" def test_build_identity_contract_protects_strong_anchor_from_unrelated_provider( @@ -333,6 +319,19 @@ def test_build_identity_contract_protects_strong_anchor_from_unrelated_provider( "metadata_sources": ["doi", "crossref"], "identity_confidence": "high", "identity_confidence_reasons": ["doi_present"], + "identity_observations": [ + { + "provider": "crossref", + "retrieved_by": {"kind": "doi", "value": "10.1234/original"}, + "record": { + "source_type": "crossref", + "source_url": "https://doi.org/10.9999/unrelated", + "pdf_url": "https://example.test/unrelated.pdf", + "title": "Unrelated Provider Paper", + "doi": "10.9999/unrelated", + }, + } + ], }, ) @@ -342,15 +341,718 @@ def test_build_identity_contract_protects_strong_anchor_from_unrelated_provider( assert identity["source_manifestation"]["pdf_url"] == "" assert len(trace["repair_attempts"]) == 1 attempt = trace["repair_attempts"][0] - assert attempt["action"] == "protect_strong_identity_anchor" - assert ( - attempt["replacement_reason"] - == "strong_identity_anchor_protected_from_unrelated_provider_evidence" - ) + assert attempt["action"] == "reject_conflicting_provider_observation" assert attempt["rejected_candidate_identity"]["doi"] == "10.9999/unrelated" assert attempt["accepted_correction"]["doi"] == "10.1234/original" +def test_build_identity_contract_rejects_conflicting_same_title_provider_observation( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + identity, trace = build_identity_from_payloads( + tmp_path, + monkeypatch, + resolve_payload={ + "status": "ok", + "script": "resolve_paper.py", + "paper_id": "doi:10.1234/trusted", + "source_type": "doi", + "source_url": "https://doi.org/10.1234/trusted", + "title": "A Same Title Paper", + "doi": "10.1234/trusted", + "metadata_sources": ["doi"], + }, + metadata_payload={ + "status": "ok", + "script": "collect_metadata.py", + "paper_id": "doi:10.1234/trusted", + "source_type": "doi", + "source_url": "https://doi.org/10.1234/trusted", + "title": "A Same Title Paper", + "doi": "10.1234/trusted", + "metadata_sources": ["doi", "crossref"], + "identity_confidence": "high", + "identity_confidence_reasons": ["doi_present"], + "identity_observations": [ + { + "provider": "crossref", + "title": "A Same Title Paper", + "doi": "10.9999/conflicting", + "pdf_url": "https://example.test/conflicting.pdf", + } + ], + }, + ) + + assert identity["identity_verdict"] == "accepted" + assert identity["work_level_identity"]["doi"] == "10.1234/trusted" + assert identity["source_manifestation"]["pdf_url"] == "" + assert len(trace["repair_attempts"]) == 1 + attempt = trace["repair_attempts"][0] + assert attempt["action"] == "reject_conflicting_provider_observation" + assert attempt["status"] == "rejected" + assert attempt["rejected_candidate_identity"]["doi"] == "10.9999/conflicting" + assert attempt["accepted_correction"]["doi"] == "10.1234/trusted" + downstream_summary = common.canonical_identity_summary(identity) + assert downstream_summary["schema_version"] == 2 + assert "accepted_observations" not in downstream_summary + assert "rejected_observations" not in downstream_summary + + +def test_build_identity_contract_accepts_shared_identifier_observation_and_binds_pdf( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + identity, trace = build_identity_from_payloads( + tmp_path, + monkeypatch, + resolve_payload={ + "status": "ok", + "script": "resolve_paper.py", + "paper_id": "doi:10.1234/trusted", + "source_type": "doi", + "source_url": "https://doi.org/10.1234/trusted", + "title": "A Shared Identifier Paper", + "doi": "10.1234/trusted", + "metadata_sources": ["doi"], + }, + metadata_payload={ + "status": "ok", + "script": "collect_metadata.py", + "paper_id": "doi:10.1234/trusted", + "source_type": "doi", + "source_url": "https://doi.org/10.1234/trusted", + "title": "A Shared Identifier Paper", + "doi": "10.1234/trusted", + "metadata_sources": ["doi"], + "identity_observations": [ + { + "provider": "crossref", + "retrieved_by": {"kind": "doi", "value": "10.1234/trusted"}, + "title": "A Shared Identifier Paper", + "authors": ["Alice Example"], + "year": "2026", + "doi": "10.1234/trusted", + "abstract": "Accepted provider abstract.", + "pdf_url": "https://example.test/accepted.pdf", + } + ], + }, + ) + + assert identity["schema_version"] == 2 + assert identity["accepted_metadata"]["abstract"] == "Accepted provider abstract." + assert identity["accepted_metadata"]["authors"] == ["Alice Example"] + assert identity["field_provenance"]["abstract"] == { + "provider": "crossref", + "retrieved_by": {"kind": "doi", "value": "10.1234/trusted"}, + } + assert identity["field_provenance"]["doi"]["provider"] == "doi" + assert identity["bound_sources"] == [ + { + "kind": "pdf_url", + "value": "https://example.test/accepted.pdf", + "provider": "crossref", + "binding_reason": "shared_identifier", + } + ] + assert identity["accepted_observations"][0]["provider"] == "crossref" + assert identity["rejected_observations"] == [] + assert trace["schema_version"] == 2 + assert trace["accepted_observations"] == identity["accepted_observations"] + assert trace["rejected_observations"] == [] + assert trace["field_provenance"] == identity["field_provenance"] + + +def test_build_identity_contract_uses_symbol_preserving_title_author_year_equivalence( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + identity, trace = build_identity_from_payloads( + tmp_path, + monkeypatch, + resolve_payload={ + "status": "ok", + "script": "resolve_paper.py", + "paper_id": "zotero:PARENT01", + "source_type": "zotero", + "zotero_key": "PARENT01", + "title": "Reliable C++ Agents", + "authors": ["Alice Example"], + "year": "2026", + "metadata_sources": ["zotero"], + }, + metadata_payload={ + "status": "ok", + "script": "collect_metadata.py", + "paper_id": "zotero:PARENT01", + "source_type": "zotero", + "zotero_key": "PARENT01", + "title": "Reliable C++ Agents", + "authors": ["Alice Example"], + "year": "2026", + "metadata_sources": ["zotero"], + "identity_observations": [ + { + "provider": "crossref", + "retrieved_by": {"kind": "title", "value": "Reliable C++ Agents"}, + "title": "Reliable C Agents", + "authors": ["Alice Example"], + "year": "2026", + "doi": "10.9999/wrong", + "pdf_url": "https://example.test/wrong.pdf", + }, + { + "provider": "openalex", + "retrieved_by": {"kind": "title", "value": "Reliable C++ Agents"}, + "title": "Reliable C++ Agents", + "authors": ["Alice Example"], + "year": "2026", + "doi": "10.1234/correct", + "pdf_url": "https://example.test/correct.pdf", + }, + ], + }, + ) + + assert identity["accepted_metadata"]["doi"] == "10.1234/correct" + assert [item["provider"] for item in identity["accepted_observations"]] == ["openalex"] + assert [item["provider"] for item in identity["rejected_observations"]] == ["crossref"] + assert identity["bound_sources"] == [ + { + "kind": "pdf_url", + "value": "https://example.test/correct.pdf", + "provider": "openalex", + "binding_reason": "title_author_year", + } + ] + assert any( + attempt["action"] == "accept_identity_promotion" + and attempt["accepted_correction"]["doi"] == "10.1234/correct" + for attempt in trace["repair_attempts"] + ) + + +@pytest.mark.parametrize( + ("anchor_title", "candidate_title", "candidate_author", "candidate_year"), + [ + ("Reliable C# Agents", "Reliable C Agents", "Alice Example", "2026"), + ("Na+ Transport Models", "Na Transport Models", "Alice Example", "2026"), + ("Reliable A/B Tests", "Reliable A B Tests", "Alice Example", "2026"), + ("Reliable Agents", "Reliable Agents", "Bob Example", "2026"), + ("Reliable Agents", "Reliable Agents", "Andrew Example", "2026"), + ("Reliable Agents", "Reliable Agents", "A Example", "2026"), + ("Reliable Agents", "Reliable Agents", "Example", "2026"), + ("Reliable Agents", "Reliable Agents", "Alice Example", "2025"), + ], +) +def test_build_identity_contract_rejects_symbol_author_or_year_mismatch( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + anchor_title: str, + candidate_title: str, + candidate_author: str, + candidate_year: str, +) -> None: + identity, _ = build_identity_from_payloads( + tmp_path, + monkeypatch, + resolve_payload={ + "status": "ok", + "script": "resolve_paper.py", + "paper_id": "zotero:PARENT01", + "source_type": "zotero", + "zotero_key": "PARENT01", + "title": anchor_title, + "authors": ["Alice Example"], + "year": "2026", + "metadata_sources": ["zotero"], + }, + metadata_payload={ + "status": "ok", + "script": "collect_metadata.py", + "paper_id": "zotero:PARENT01", + "source_type": "zotero", + "zotero_key": "PARENT01", + "title": anchor_title, + "authors": ["Alice Example"], + "year": "2026", + "metadata_sources": ["zotero"], + "identity_observations": [ + { + "provider": "crossref", + "retrieved_by": {"kind": "title", "value": anchor_title}, + "title": candidate_title, + "authors": [candidate_author], + "year": candidate_year, + "doi": "10.9999/rejected", + "pdf_url": "https://example.test/rejected.pdf", + } + ], + }, + ) + + assert "doi" not in identity["accepted_metadata"] + assert identity["bound_sources"] == [] + assert identity["rejected_observations"][0]["reason"] == ( + "insufficient_equivalence_evidence" + ) + + +def test_build_identity_contract_rejects_a_later_conflicting_identity_promotion( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + identity, _ = build_identity_from_payloads( + tmp_path, + monkeypatch, + resolve_payload={ + "status": "ok", + "script": "resolve_paper.py", + "paper_id": "title:trusted", + "source_type": "title_query", + "title": "Reliable Identity Admission", + "authors": ["Alice Example"], + "year": "2026", + "metadata_sources": ["title_query"], + }, + metadata_payload={ + "status": "ok", + "script": "collect_metadata.py", + "paper_id": "title:trusted", + "source_type": "title_query", + "title": "Reliable Identity Admission", + "authors": ["Alice Example"], + "year": "2026", + "metadata_sources": ["title_query"], + "identity_observations": [ + { + "provider": "crossref", + "retrieved_by": { + "kind": "title", + "value": "Reliable Identity Admission", + }, + "title": "Reliable Identity Admission", + "authors": ["Alice Example"], + "year": "2026", + "doi": "10.1234/accepted", + "abstract": "Accepted first observation.", + }, + { + "provider": "openalex", + "retrieved_by": { + "kind": "title", + "value": "Reliable Identity Admission", + }, + "title": "Reliable Identity Admission", + "authors": ["Alice Example"], + "year": "2026", + "doi": "10.9999/conflicting", + "abstract": "Must not overwrite accepted metadata.", + "pdf_url": "https://example.test/conflicting.pdf", + }, + ], + }, + ) + + assert identity["paper_id"] == "doi:10.1234/accepted" + assert identity["accepted_metadata"]["doi"] == "10.1234/accepted" + assert identity["accepted_metadata"]["abstract"] == "Accepted first observation." + assert [item["provider"] for item in identity["accepted_observations"]] == [ + "crossref" + ] + assert [item["provider"] for item in identity["rejected_observations"]] == [ + "openalex" + ] + assert identity["bound_sources"] == [] + + +def test_build_identity_contract_accepts_two_independent_title_observations( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + observations = [ + { + "provider": provider, + "retrieved_by": {"kind": "title", "value": "Consensus Paper"}, + "record": { + "title": "Consensus Paper", + "authors": ["Alice Example"], + "year": "2026", + "doi": "10.1234/consensus", + }, + } + for provider in ("crossref", "openalex") + ] + identity, _ = build_identity_from_payloads( + tmp_path, + monkeypatch, + resolve_payload={ + "status": "ok", + "script": "resolve_paper.py", + "paper_id": "title:consensus", + "source_type": "title_query", + "title": "Consensus Paper", + "metadata_sources": ["title_query"], + }, + metadata_payload={ + "status": "ok", + "script": "collect_metadata.py", + "paper_id": "title:consensus", + "identity_observations": observations, + }, + ) + + assert identity["paper_id"] == "doi:10.1234/consensus" + assert identity["accepted_metadata"]["authors"] == ["Alice Example"] + assert [item["provider"] for item in identity["accepted_observations"]] == [ + "crossref", + "openalex", + ] + + +def test_build_identity_contract_rejects_provider_consensus_without_titles( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + observations = [ + { + "provider": provider, + "retrieved_by": {"kind": "title", "value": ""}, + "record": { + "authors": ["Alice Example"], + "year": "2026", + "doi": "10.1234/missing-title", + }, + } + for provider in ("crossref", "openalex") + ] + identity, _ = build_identity_from_payloads( + tmp_path, + monkeypatch, + resolve_payload={ + "status": "ok", + "script": "resolve_paper.py", + "paper_id": "title:missing", + "source_type": "title_query", + "title": "", + "metadata_sources": ["title_query"], + }, + metadata_payload={ + "status": "ok", + "script": "collect_metadata.py", + "paper_id": "title:missing", + "identity_observations": observations, + }, + expect_failure=True, + ) + + assert identity["status"] == "error" + assert not identity["paper_id"].startswith("doi:") + assert identity["accepted_observations"] == [] + assert [item["provider"] for item in identity["rejected_observations"]] == [ + "crossref", + "openalex", + ] + + +def test_build_identity_contract_accepts_unique_exact_zotero_title_match( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + local_pdf = tmp_path / "zotero.pdf" + local_pdf.write_bytes(b"%PDF-1.7\n") + zotero_observation = { + "provider": "zotero", + "retrieved_by": { + "kind": "title_query", + "value": "Local Canonical Title", + }, + "relation": { + "kind": "zotero_lookup", + "match_kind": "title", + "match_resolution": "unique_exact", + }, + "record": { + "title": "Local Canonical Title", + "authors": ["Local Author"], + "year": "2024", + "doi": "10.5555/local", + "zotero_key": "PARENT01", + "local_pdf_path": str(local_pdf), + }, + } + identity, _ = build_identity_from_payloads( + tmp_path, + monkeypatch, + resolve_payload={ + "status": "ok", + "script": "resolve_paper.py", + "paper_id": "title:local-canonical-title", + "source_type": "title_query", + "title": "Local Canonical Title", + "metadata_sources": ["title_query"], + "identity_observations": [zotero_observation], + }, + metadata_payload={ + "status": "ok", + "script": "collect_metadata.py", + "paper_id": "title:local-canonical-title", + "identity_observations": [zotero_observation], + }, + ) + + assert identity["paper_id"] == "doi:10.5555/local" + assert identity["accepted_observations"][0]["reason"] == ( + "unique_exact_zotero_title" + ) + assert identity["bound_sources"] == [ + { + "kind": "local_pdf", + "value": str(local_pdf), + "provider": "zotero", + "binding_reason": "unique_exact_zotero_title", + } + ] + + +def test_fetch_consumers_have_no_paper_id_override() -> None: + for parser in (extract_source_text.parser(), extract_evidence.parser()): + assert "--paper-id" not in { + option + for action in parser._actions + for option in action.option_strings + } + + +@pytest.mark.parametrize( + ("resolve_script", "metadata_script"), + [ + ("raw_metadata", "collect_metadata.py"), + ("resolve_paper.py", "raw_metadata"), + ], +) +def test_build_identity_contract_rejects_nonproducer_artifacts( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + resolve_script: str, + metadata_script: str, +) -> None: + with pytest.raises(SystemExit, match="producer"): + build_identity_from_payloads( + tmp_path, + monkeypatch, + resolve_payload={ + "status": "ok", + "script": resolve_script, + "paper_id": "doi:10.1234/raw", + "doi": "10.1234/raw", + }, + metadata_payload={ + "status": "ok", + "script": metadata_script, + "identity_observations": [], + }, + ) + + +def test_build_identity_contract_records_zotero_key_promotion( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + identity, trace = build_identity_from_payloads( + tmp_path, + monkeypatch, + resolve_payload={ + "status": "ok", + "script": "resolve_paper.py", + "paper_id": "title:zotero", + "source_type": "title_query", + "title": "Zotero Promotion Paper", + "authors": ["Alice Example"], + "year": "2026", + "metadata_sources": ["title_query"], + }, + metadata_payload={ + "status": "ok", + "script": "collect_metadata.py", + "paper_id": "title:zotero", + "identity_observations": [ + { + "provider": "zotero", + "retrieved_by": { + "kind": "title", + "value": "Zotero Promotion Paper", + }, + "record": { + "title": "Zotero Promotion Paper", + "authors": ["Alice Example"], + "year": "2026", + "zotero_key": "PARENT01", + }, + } + ], + }, + ) + + assert identity["paper_id"] == "zotero:PARENT01" + promotion = trace["repair_attempts"][0] + assert promotion["promoted_identifiers"] == {"zotero_key": "PARENT01"} + assert promotion["previous_paper_id"] == "title:zotero" + assert promotion["accepted_paper_id"] == "zotero:PARENT01" + + +def test_build_identity_contract_recomputes_a_stale_anchor_paper_id( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + identity, _ = build_identity_from_payloads( + tmp_path, + monkeypatch, + resolve_payload={ + "status": "ok", + "script": "resolve_paper.py", + "paper_id": "doi:10.9999/stale", + "source_type": "doi", + "doi": "10.1234/actual", + "metadata_sources": ["doi"], + }, + metadata_payload={ + "status": "ok", + "script": "collect_metadata.py", + "identity_observations": [], + }, + ) + + assert identity["paper_id"] == "doi:10.1234/actual" + assert identity["accepted_metadata"]["paper_id"] == "doi:10.1234/actual" + + +def test_build_identity_contract_derives_a_bound_source_from_accepted_frontiers_doi( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + identity, _ = build_identity_from_payloads( + tmp_path, + monkeypatch, + resolve_payload={ + "status": "ok", + "script": "resolve_paper.py", + "paper_id": "doi:10.3389/fpubh.2019.00399", + "source_type": "doi", + "source_url": "https://doi.org/10.3389/fpubh.2019.00399", + "doi": "10.3389/fpubh.2019.00399", + "metadata_sources": ["doi"], + }, + metadata_payload={ + "status": "ok", + "script": "collect_metadata.py", + "paper_id": "doi:10.3389/fpubh.2019.00399", + "source_type": "doi", + "source_url": "https://doi.org/10.3389/fpubh.2019.00399", + "doi": "10.3389/fpubh.2019.00399", + "metadata_sources": ["doi"], + "identity_observations": [], + }, + ) + + assert identity["bound_sources"] == [ + { + "kind": "pdf_url", + "value": ( + "https://www.frontiersin.org/articles/" + "10.3389/fpubh.2019.00399/pdf" + ), + "provider": "doi", + "binding_reason": "accepted_identifier_derived", + } + ] + + +def test_build_identity_contract_ignores_unadjudicated_top_level_metadata( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + identity, _ = build_identity_from_payloads( + tmp_path, + monkeypatch, + resolve_payload={ + "status": "ok", + "script": "resolve_paper.py", + "paper_id": "doi:10.1234/trusted", + "source_type": "doi", + "doi": "10.1234/trusted", + "metadata_sources": ["doi"], + }, + metadata_payload={ + "status": "ok", + "script": "collect_metadata.py", + "paper_id": "doi:10.1234/trusted", + "source_type": "doi", + "doi": "10.1234/trusted", + "abstract": "Unadjudicated legacy field.", + "pdf_url": "https://example.test/unadjudicated.pdf", + "metadata_sources": ["doi", "legacy_merge"], + }, + ) + + assert "abstract" not in identity["accepted_metadata"] + assert all( + source["value"] != "https://example.test/unadjudicated.pdf" + for source in identity["bound_sources"] + ) + + +def test_build_identity_contract_requires_the_observation_envelope( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + resolve_path = tmp_path / "resolve.json" + metadata_path = tmp_path / "metadata.json" + resolve_path.write_text( + json.dumps( + { + "status": "ok", + "script": "resolve_paper.py", + "paper_id": "doi:10.1234/trusted", + "source_type": "doi", + "doi": "10.1234/trusted", + } + ), + encoding="utf-8", + ) + metadata_path.write_text( + json.dumps( + { + "status": "ok", + "script": "collect_metadata.py", + "doi": "10.9999/raw", + "pdf_url": "https://example.test/raw.pdf", + } + ), + encoding="utf-8", + ) + monkeypatch.setattr( + sys, + "argv", + [ + "build_identity_contract.py", + "--input", + str(metadata_path), + "--resolve", + str(resolve_path), + "--trace-output", + str(tmp_path / "trace.json"), + "--output", + str(tmp_path / "identity.json"), + ], + ) + + with pytest.raises(SystemExit, match="requires metadata identity_observations"): + build_identity_contract.main() + + def test_build_identity_contract_accepts_equivalent_arxiv_and_published_manifestations( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, @@ -393,6 +1095,29 @@ def test_build_identity_contract_accepts_equivalent_arxiv_and_published_manifest "arxiv_id": "2401.00001", "identity_confidence": "high", "identity_confidence_reasons": ["doi_present", "arxiv_id_present"], + "identity_observations": [ + { + "provider": "crossref", + "retrieved_by": { + "kind": "arxiv_id", + "value": "2401.00001", + }, + "record": { + "source_type": "doi", + "source_url": "https://doi.org/10.1234/published", + "title": "DeepPaperNote: Evidence-First Reading", + "authors": ["Alice Smith", "Bob Jones"], + "abstract": ( + "We introduce an evidence-first reading workflow " + "for a single paper." + ), + "year": "2026", + "venue": "Journal of Paper Systems", + "doi": "10.1234/published", + "arxiv_id": "2401.00001", + }, + } + ], } ), encoding="utf-8", @@ -419,7 +1144,9 @@ def test_build_identity_contract_accepts_equivalent_arxiv_and_published_manifest identity = json.loads(identity_path.read_text(encoding="utf-8")) trace = json.loads(trace_path.read_text(encoding="utf-8")) assert identity["identity_verdict"] == "accepted" - assert identity["work_level_identity"]["title"] == "DeepPaperNote: Evidence-First Reading" + assert identity["work_level_identity"]["title"] == ( + "DeepPaperNote: Evidence First Reading" + ) assert identity["work_level_identity"]["doi"] == "10.1234/published" assert identity["source_manifestation"]["source_kind"] == "arxiv_id" assert identity["source_manifestation"]["title"] == "DeepPaperNote: Evidence First Reading" @@ -471,30 +1198,35 @@ def test_build_identity_contract_marks_safe_metadata_uncertainty_as_warning_scop "arxiv_id": "2401.00001", "identity_confidence": "high", "identity_confidence_reasons": ["doi_present", "arxiv_id_present"], + "identity_observations": [ + { + "provider": "crossref", + "retrieved_by": { + "kind": "arxiv_id", + "value": "2401.00001", + }, + "record": { + "title": "DeepPaperNote: Evidence-First Reading", + "authors": ["Alice Smith", "Bob Jones"], + "year": "2026", + "venue": "Journal of Paper Systems", + "doi": "10.1234/published", + "arxiv_id": "2401.00001", + }, + } + ], }, ) - assert identity["identity_verdict"] == "accepted_with_warnings" + assert identity["identity_verdict"] == "accepted" assert identity["equivalence_decision"]["status"] == "equivalent" - assert identity["work_level_identity"]["year"] == "2026" - assert identity["work_level_identity"]["venue"] == "Journal of Paper Systems" + assert identity["work_level_identity"]["year"] == "2024" + assert identity["work_level_identity"]["venue"] == "arXiv" + assert identity["work_level_identity"]["doi"] == "10.1234/published" assert identity["source_manifestation"]["year"] == "2024" assert identity["source_manifestation"]["venue"] == "arXiv" - assert { - item["reason"]: (item["scope"], item["impact"]) - for item in identity["warnings"] - } == { - "source_manifestation_year_differs_from_work_identity": ( - "metadata", - "avoid_over_specific_year_claims", - ), - "source_manifestation_venue_differs_from_work_identity": ( - "metadata", - "avoid_over_specific_venue_claims", - ), - } - assert trace["identity_verdict"] == "accepted_with_warnings" - assert trace["warnings"] == identity["warnings"] + assert identity["warnings"] == [] + assert trace["identity_verdict"] == "accepted" def test_build_identity_contract_fails_closed_for_competing_manifestations_after_repair( @@ -531,6 +1263,22 @@ def test_build_identity_contract_fails_closed_for_competing_manifestations_after "abstract": "We improve legal reasoning with efficient language models.", "identity_confidence": "medium", "identity_confidence_reasons": ["external_metadata_title_match"], + "identity_observations": [ + { + "provider": "semantic_scholar", + "retrieved_by": { + "kind": "title", + "value": "Efficient Vision Transformers for Medical Images", + }, + "record": { + "title": "Efficient Language Models for Legal Reasoning", + "authors": ["Mallory Text"], + "abstract": ( + "We improve legal reasoning with efficient language models." + ), + }, + } + ], } ), encoding="utf-8", @@ -560,21 +1308,18 @@ def test_build_identity_contract_fails_closed_for_competing_manifestations_after trace = json.loads(trace_path.read_text(encoding="utf-8")) assert identity["status"] == "error" assert identity["run_status"] == "failed" - assert identity["identity_verdict"] == "ambiguous" - assert identity["identity_failure_class"] == "ambiguous_competing_identities" + assert identity["identity_verdict"] == "failed" + assert identity["identity_failure_class"] == "insufficient_evidence" assert "stronger identifier" in identity["failure_summary"] assert "Efficient Vision Transformers" not in identity["failure_summary"] assert "Efficient Language Models" not in identity["failure_summary"] - assert identity["equivalence_decision"]["status"] == "ambiguous" - assert identity["equivalence_decision"]["reason"] == "competing_identity_evidence" - assert any( - item["kind"] == "leading_author" and item["status"] == "conflict" - for item in identity["equivalence_decision"]["evidence"] + assert identity["rejected_observations"][0]["reason"] == ( + "insufficient_equivalence_evidence" ) assert trace["status"] == "error" assert trace["run_status"] == "failed" - assert trace["identity_verdict"] == "ambiguous" - assert trace["identity_failure_class"] == "ambiguous_competing_identities" + assert trace["identity_verdict"] == "failed" + assert trace["identity_failure_class"] == "insufficient_evidence" assert trace["repair_attempts"][-1]["action"] == "repair_exhausted_fail_closed" assert trace["repair_attempts"][-1]["status"] == "failed" assert trace["equivalence_decision"] == identity["equivalence_decision"] @@ -626,56 +1371,6 @@ def test_build_identity_contract_fails_closed_for_competing_manifestations_after }, "provider_unavailable", ), - ( - { - "status": "ok", - "script": "resolve_paper.py", - "paper_id": "doi:10.1234/original", - "source_type": "doi", - "title": "Original DOI Paper", - "doi": "10.1234/original", - "metadata_sources": ["doi"], - }, - { - "status": "ok", - "script": "collect_metadata.py", - "paper_id": "doi:10.1234/original", - "source_type": "doi", - "title": "Original DOI Paper", - "doi": "10.1234/original", - "metadata_sources": ["doi", "crossref"], - "identity_contradictions": ["crossref_title_author_conflict"], - }, - "metadata_contradiction", - ), - ( - { - "status": "ok", - "script": "resolve_paper.py", - "paper_id": "title:local-pdf", - "source_type": "local_pdf", - "source_url": "/tmp/mismatched.pdf", - "local_pdf_path": "/tmp/mismatched.pdf", - "title": "Local PDF About Vision Models", - "authors": ["Alice Vision"], - "abstract": "We classify images with compact vision transformers.", - "metadata_sources": ["local_pdf"], - }, - { - "status": "ok", - "script": "collect_metadata.py", - "paper_id": "doi:10.9999/legal-language", - "source_type": "local_pdf", - "source_url": "/tmp/mismatched.pdf", - "local_pdf_path": "/tmp/mismatched.pdf", - "title": "Legal Language Models For Contract Review", - "authors": ["Mallory Text"], - "abstract": "We improve contract review with legal language models.", - "doi": "10.9999/legal-language", - "metadata_sources": ["local_pdf", "crossref"], - }, - "source_pdf_mismatch", - ), ], ) def test_build_identity_contract_classifies_repair_exhausted_failures( @@ -712,10 +1407,13 @@ def test_collect_metadata_refuses_non_ok_input_artifact( output = tmp_path / "metadata.json" write_error_artifact(artifact, "resolve_paper.py") - def fail_enrich_metadata(record: dict) -> dict: + def fail_collect_metadata_observations(record: dict) -> list[dict]: raise AssertionError("non-ok acquisition artifacts must fail before enrichment") - monkeypatch.setattr("collect_metadata.enrich_metadata", fail_enrich_metadata) + monkeypatch.setattr( + "collect_metadata.collect_metadata_observations", + fail_collect_metadata_observations, + ) monkeypatch.setattr( sys, "argv", @@ -735,9 +1433,113 @@ def fail_enrich_metadata(record: dict) -> dict: assert not output.exists() -def test_fetch_pdf_uses_accepted_identity_contract_for_source_selection( +def test_collect_metadata_preserves_provider_result_as_identity_observation( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, +) -> None: + resolve_path = tmp_path / "paper_resolve.json" + metadata_path = tmp_path / "paper_metadata.json" + resolve_path.write_text( + json.dumps( + { + "status": "ok", + "script": "resolve_paper.py", + "paper_id": "doi:10.1234/trusted", + "source_type": "doi", + "source_url": "https://doi.org/10.1234/trusted", + "title": "Trusted Input Paper", + "doi": "10.1234/trusted", + "metadata_sources": ["doi"], + } + ), + encoding="utf-8", + ) + monkeypatch.setattr( + common, + "fetch_crossref_by_doi", + lambda doi: { + "source_type": "crossref", + "title": "Trusted Input Paper", + "doi": doi, + "abstract": "Provider abstract.", + "pdf_url": "https://example.test/provider.pdf", + "metadata_sources": ["crossref"], + }, + ) + monkeypatch.setattr(common, "fetch_openalex_by_doi", lambda doi: None) + monkeypatch.setattr(common, "search_semantic_scholar", lambda query, limit=5: []) + monkeypatch.setattr(common, "search_crossref_by_title", lambda title, limit=5: []) + monkeypatch.setattr(common, "search_openalex_by_title", lambda title, limit=5: []) + monkeypatch.setattr(common, "safe_fetch_arxiv_entries", lambda **kwargs: []) + monkeypatch.setattr( + sys, + "argv", + [ + "collect_metadata.py", + "--input", + str(resolve_path), + "--output", + str(metadata_path), + ], + ) + + collect_metadata.main() + + metadata = json.loads(metadata_path.read_text(encoding="utf-8")) + assert metadata["doi"] == "10.1234/trusted" + assert "abstract" not in metadata + assert "pdf_url" not in metadata + assert metadata["identity_observations"] == [ + { + "provider": "crossref", + "retrieved_by": {"kind": "doi", "value": "10.1234/trusted"}, + "record": { + "source_type": "crossref", + "title": "Trusted Input Paper", + "doi": "10.1234/trusted", + "abstract": "Provider abstract.", + "pdf_url": "https://example.test/provider.pdf", + "metadata_sources": ["crossref"], + }, + } + ] + + +@pytest.mark.parametrize( + "module", + [extract_source_text, extract_evidence, extract_pdf_assets], +) +def test_downstream_extractors_reject_scalar_identity_bypasses(module: object) -> None: + assert not hasattr(module, "enrich_metadata") + with pytest.raises(SystemExit, match="JSON acquisition artifact"): + module.ensure_record("Reliable C++ Agents") + + +@pytest.mark.parametrize( + "module", + [extract_source_text, extract_evidence, extract_pdf_assets], +) +def test_downstream_extractors_reject_non_fetch_json_bypasses( + module: object, + tmp_path: Path, +) -> None: + artifact = tmp_path / "metadata.json" + artifact.write_text( + json.dumps( + { + "status": "ok", + "script": "collect_metadata.py", + "pdf_path": str(tmp_path / "bypass.pdf"), + } + ), + encoding="utf-8", + ) + with pytest.raises(SystemExit, match="requires a fetch_pdf.py artifact"): + module.ensure_record(str(artifact)) + + +def test_fetch_pdf_uses_accepted_identity_contract_for_source_selection( + tmp_path: Path, ) -> None: metadata_path = tmp_path / "metadata.json" identity_path = tmp_path / "identity.json" @@ -764,6 +1566,7 @@ def test_fetch_pdf_uses_accepted_identity_contract_for_source_selection( "status": "ok", "script": "build_identity_contract.py", "artifact_type": "canonical_identity", + "schema_version": 2, "paper_id": "paper:canonical", "identity_verdict": "accepted", "work_level_identity": { @@ -776,6 +1579,14 @@ def test_fetch_pdf_uses_accepted_identity_contract_for_source_selection( "source_url": str(canonical_pdf), "title": "Canonical Identity Title", }, + "bound_sources": [ + { + "kind": "local_pdf", + "value": str(canonical_pdf), + "provider": "local_pdf", + "binding_reason": "trusted_input", + } + ], "selected_identity_evidence": [], "warnings": [], "repair_trace_path": str(tmp_path / "trace.json"), @@ -783,14 +1594,6 @@ def test_fetch_pdf_uses_accepted_identity_contract_for_source_selection( ), encoding="utf-8", ) - captured_records: list[dict] = [] - - def fake_pdf_source_candidates(record: dict) -> list[tuple[str, str]]: - captured_records.append(dict(record)) - return [("local_pdf", str(canonical_pdf))] - - monkeypatch.setattr("fetch_pdf.pdf_source_candidates", fake_pdf_source_candidates) - fetch_pdf.main( [ "--input", @@ -802,12 +1605,6 @@ def fake_pdf_source_candidates(record: dict) -> list[tuple[str, str]]: ] ) - assert captured_records - assert captured_records[0]["paper_id"] == "paper:canonical" - assert captured_records[0]["title"] == "Canonical Identity Title" - assert captured_records[0]["local_pdf_path"] == str(canonical_pdf) - assert captured_records[0]["local_pdf_path"] != str(stale_pdf) - payload = json.loads(output.read_text(encoding="utf-8")) assert payload["status"] == "ok" assert payload["paper_id"] == "paper:canonical" @@ -819,7 +1616,6 @@ def fake_pdf_source_candidates(record: dict) -> list[tuple[str, str]]: def test_fetch_pdf_refuses_unaccepted_identity_contract_before_candidate_selection( tmp_path: Path, - monkeypatch: pytest.MonkeyPatch, ) -> None: metadata_path = tmp_path / "metadata.json" identity_path = tmp_path / "identity.json" @@ -834,17 +1630,14 @@ def test_fetch_pdf_refuses_unaccepted_identity_contract_before_candidate_selecti "status": "ok", "script": "build_identity_contract.py", "artifact_type": "canonical_identity", + "schema_version": 2, "identity_verdict": "repairable", + "bound_sources": [], } ), encoding="utf-8", ) - def fail_pdf_source_candidates(record: dict) -> list[tuple[str, str]]: - raise AssertionError("unaccepted identity must fail before PDF candidate selection") - - monkeypatch.setattr("fetch_pdf.pdf_source_candidates", fail_pdf_source_candidates) - with pytest.raises(SystemExit) as exc_info: fetch_pdf.main( [ @@ -877,6 +1670,7 @@ def test_fetch_pdf_allows_accepted_with_warnings_identity_contract(tmp_path: Pat "status": "ok", "script": "build_identity_contract.py", "artifact_type": "canonical_identity", + "schema_version": 2, "paper_id": "paper:warning", "identity_verdict": "accepted_with_warnings", "work_level_identity": {"title": "Warning Scoped Paper"}, @@ -886,6 +1680,14 @@ def test_fetch_pdf_allows_accepted_with_warnings_identity_contract(tmp_path: Pat "source_url": str(canonical_pdf), "title": "Warning Scoped Paper", }, + "bound_sources": [ + { + "kind": "local_pdf", + "value": str(canonical_pdf), + "provider": "local_pdf", + "binding_reason": "trusted_input", + } + ], "selected_identity_evidence": [], "warnings": ["metadata_year_missing"], "repair_trace_path": str(tmp_path / "trace.json"), @@ -913,20 +1715,39 @@ def test_fetch_pdf_allows_accepted_with_warnings_identity_contract(tmp_path: Pat def test_fetch_pdf_refuses_non_ok_input_artifact_before_candidate_selection( tmp_path: Path, - monkeypatch: pytest.MonkeyPatch, ) -> None: artifact = tmp_path / "metadata.json" + identity = tmp_path / "identity.json" output = tmp_path / "fetch.json" write_error_artifact(artifact, "collect_metadata.py") - - def fail_pdf_source_candidates(record: dict) -> list[tuple[str, str]]: - message = "non-ok acquisition artifacts must fail before PDF candidate selection" - raise AssertionError(message) - - monkeypatch.setattr("fetch_pdf.pdf_source_candidates", fail_pdf_source_candidates) + identity.write_text( + json.dumps( + { + "status": "ok", + "script": "build_identity_contract.py", + "artifact_type": "canonical_identity", + "schema_version": 2, + "paper_id": "doi:10.1234/example", + "identity_verdict": "accepted", + "work_level_identity": {"doi": "10.1234/example"}, + "source_manifestation": {}, + "bound_sources": [], + } + ), + encoding="utf-8", + ) with pytest.raises(SystemExit) as exc_info: - fetch_pdf.main(["--input", str(artifact), "--output", str(output)]) + fetch_pdf.main( + [ + "--input", + str(artifact), + "--identity", + str(identity), + "--output", + str(output), + ] + ) assert "non-ok input artifact" in str(exc_info.value) assert not output.exists() diff --git a/tests/test_build_synthesis_bundle_contract.py b/tests/test_build_synthesis_bundle_contract.py index ee72f46..7fac8d2 100644 --- a/tests/test_build_synthesis_bundle_contract.py +++ b/tests/test_build_synthesis_bundle_contract.py @@ -2,10 +2,22 @@ import json +import pytest + from build_synthesis_bundle import bundle from contracts import WRITING_CONTRACT_RULES +def test_bundle_refuses_raw_metadata_without_an_identity_contract() -> None: + with pytest.raises(ValueError, match="accepted canonical identity contract"): + bundle( + metadata={"title": "Raw Candidate"}, + evidence_wrapper={"evidence_pack": {}}, + figures_wrapper={}, + assets_wrapper={}, + ) + + def test_bundle_compact_writing_contract_keeps_depth_rules_without_old_bundle_fields() -> None: synthesis = bundle( metadata={"title": "Contract Paper"}, @@ -14,6 +26,14 @@ def test_bundle_compact_writing_contract_keeps_depth_rules_without_old_bundle_fi assets_wrapper={}, source_manifest={ "paper_id": "paper:contract", + "identity_contract": { + "artifact_type": "canonical_identity", + "schema_version": 2, + "paper_id": "paper:contract", + "identity_verdict": "accepted", + "work_level_identity": {"title": "Contract Paper"}, + "accepted_metadata": {"title": "Contract Paper"}, + }, "coverage": {"total_pages": 8, "text_truncated": False}, "sections": [ { @@ -98,6 +118,14 @@ def test_bundle_truncation_warnings_use_source_manifest_not_evidence_pack() -> N figures_wrapper={}, assets_wrapper={}, source_manifest={ + "identity_contract": { + "artifact_type": "canonical_identity", + "schema_version": 2, + "paper_id": "paper:coverage", + "identity_verdict": "accepted", + "work_level_identity": {"title": "Coverage Paper"}, + "accepted_metadata": {"title": "Coverage Paper"}, + }, "coverage": { "total_pages": 4, "text_truncated": False, @@ -123,6 +151,7 @@ def test_bundle_preserves_identity_equivalence_and_source_manifestation() -> Non source_manifest={ "identity_contract": { "artifact_type": "canonical_identity", + "schema_version": 2, "paper_id": "doi:10.1234/published", "identity_verdict": "accepted", "work_level_identity": { @@ -161,6 +190,108 @@ def test_bundle_preserves_identity_equivalence_and_source_manifestation() -> Non assert identity_contract["equivalence_decision"]["location_binding"] == "source_manifestation" +def test_bundle_uses_only_canonical_accepted_metadata_for_model_input() -> None: + synthesis = bundle( + metadata={ + "paper_id": "doi:10.9999/rejected", + "title": "Rejected Candidate", + "doi": "10.9999/rejected", + "abstract": "Rejected abstract must not reach the model.", + "pdf_url": "https://example.test/rejected.pdf", + }, + evidence_wrapper={"evidence_pack": {}}, + figures_wrapper={}, + assets_wrapper={}, + source_manifest={ + "paper_id": "doi:10.1234/accepted", + "title": "Accepted Paper", + "identity_contract": { + "artifact_type": "canonical_identity", + "schema_version": 2, + "schema_version": 2, + "paper_id": "doi:10.1234/accepted", + "identity_verdict": "accepted", + "work_level_identity": { + "title": "Accepted Paper", + "doi": "10.1234/accepted", + }, + "accepted_metadata": { + "paper_id": "doi:10.1234/accepted", + "title": "Accepted Paper", + "authors": ["Alice Example"], + "year": "2026", + "doi": "10.1234/accepted", + "abstract": "Accepted abstract.", + "metadata_sources": ["zotero", "crossref"], + }, + "source_manifestation": { + "source_kind": "local_pdf", + "local_pdf_path": "/tmp/accepted.pdf", + }, + "bound_sources": [], + "accepted_observations": [], + "rejected_observations": [ + { + "provider": "crossref", + "reason": "conflicting_strong_identifier", + "record": { + "doi": "10.9999/rejected", + "pdf_url": "https://example.test/rejected.pdf", + }, + } + ], + "selected_identity_evidence": [], + "equivalence_decision": {"status": "equivalent"}, + "warnings": [], + }, + }, + ) + + assert synthesis["paper_id"] == "doi:10.1234/accepted" + assert synthesis["title"] == "Accepted Paper" + assert synthesis["metadata"] == { + "title": "Accepted Paper", + "translated_title": "", + "authors": ["Alice Example"], + "affiliations": [], + "year": "2026", + "venue": "", + "doi": "10.1234/accepted", + "source_url": "", + "abstract": "Accepted abstract.", + "arxiv_id": "", + "zotero_key": "", + "metadata_sources": ["zotero", "crossref"], + "identity_confidence": "", + "identity_confidence_reasons": [], + } + serialized_model_input = json.dumps(synthesis, ensure_ascii=False) + assert "Rejected abstract must not reach the model" not in serialized_model_input + assert "https://example.test/rejected.pdf" not in serialized_model_input + + +def test_bundle_rejects_noncanonical_artifact_disguised_as_schema_v2() -> None: + with pytest.raises(ValueError, match="canonical identity"): + bundle( + metadata={"title": "Untrusted Paper"}, + evidence_wrapper={"evidence_pack": {}}, + figures_wrapper={}, + assets_wrapper={}, + source_manifest={ + "identity_contract": { + "artifact_type": "raw_metadata", + "schema_version": 2, + "paper_id": "doi:10.9999/untrusted", + "identity_verdict": "accepted", + "accepted_metadata": { + "title": "Untrusted Paper", + "doi": "10.9999/untrusted", + }, + } + }, + ) + + def test_bundle_preserves_identity_warnings_without_body_writing_instruction() -> None: warning = { "reason": "source_manifestation_year_differs_from_work_identity", @@ -175,6 +306,7 @@ def test_bundle_preserves_identity_warnings_without_body_writing_instruction() - source_manifest={ "identity_contract": { "artifact_type": "canonical_identity", + "schema_version": 2, "paper_id": "doi:10.1234/published", "identity_verdict": "accepted_with_warnings", "work_level_identity": { 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_citation_links.py b/tests/test_citation_links.py index 558e246..cbbf158 100644 --- a/tests/test_citation_links.py +++ b/tests/test_citation_links.py @@ -4,7 +4,6 @@ import citation_links import build_synthesis_bundle -from build_synthesis_bundle import bundle from citation_links import extract_reference_candidates_from_pdf, resolve_reference_links @@ -284,7 +283,7 @@ def test_bundle_exposes_reference_candidates_under_references(monkeypatch) -> No lambda: {"obsidian_vault": "", "papers_dir": "Research/Papers"}, ) - synthesis = bundle( + synthesis = build_synthesis_bundle.bundle( metadata={"title": "Citation Paper"}, evidence_wrapper={ "evidence_pack": { @@ -298,6 +297,16 @@ def test_bundle_exposes_reference_candidates_under_references(monkeypatch) -> No }, figures_wrapper={}, assets_wrapper={}, + source_manifest={ + "identity_contract": { + "artifact_type": "canonical_identity", + "schema_version": 2, + "paper_id": "paper:citation-test", + "identity_verdict": "accepted", + "work_level_identity": {"title": "Citation Paper"}, + "accepted_metadata": {"title": "Citation Paper"}, + } + }, ) candidates = synthesis["references"]["candidates"] diff --git a/tests/test_common.py b/tests/test_common.py index 24fc084..2e38447 100644 --- a/tests/test_common.py +++ b/tests/test_common.py @@ -939,7 +939,7 @@ def test_fetch_arxiv_entries_returns_empty_on_invalid_xml(monkeypatch) -> None: assert fetch_arxiv_entries(search_query='ti:"test"', max_results=1) == [] -def test_resolve_reference_title_survives_arxiv_failure(monkeypatch) -> None: +def test_resolve_reference_title_defers_provider_selection(monkeypatch) -> None: semantic_match = { "title": "Example Paper", "authors": ["Alice Example"], @@ -957,9 +957,9 @@ def test_resolve_reference_title_survives_arxiv_failure(monkeypatch) -> None: assert resolved["status"] == "ok" assert resolved["title"] == "Example Paper" - assert "semantic_scholar" in (resolved.get("metadata_sources") or []) - assert resolved["identity_confidence"] == "medium" - assert "external_metadata_title_match" in resolved["identity_confidence_reasons"] + assert resolved["metadata_sources"] == ["title_query"] + assert resolved["identity_confidence"] == "low" + assert "title_query_unmatched" in resolved["identity_confidence_reasons"] def test_resolve_reference_doi_sets_high_identity_confidence(monkeypatch) -> None: @@ -1031,7 +1031,7 @@ def test_resolve_reference_local_pdf_artifact_stem_sets_low_identity_confidence( assert "local_pdf_stem_used" in resolved["identity_confidence_reasons"] -def test_enrich_metadata_survives_arxiv_failure(monkeypatch) -> None: +def test_enrich_metadata_rejects_unlinked_doi_when_arxiv_provider_fails(monkeypatch) -> None: semantic_match = { "title": "Example Paper", "authors": ["Alice Example", "Bob Example"], @@ -1049,10 +1049,10 @@ def test_enrich_metadata_survives_arxiv_failure(monkeypatch) -> None: enriched = enrich_metadata({"title": "Example Paper", "arxiv_id": "2501.00001", "metadata_sources": ["seed_record"]}) assert enriched["title"] == "Example Paper" - assert enriched["doi"] == "10.1000/example" - assert enriched["venue"] == "ExampleConf" - assert enriched["year"] == "2025" - assert enriched["abstract"] == "Strong abstract" + assert enriched["doi"] == "10.48550/arXiv.2501.00001" + assert "venue" not in enriched + assert "year" not in enriched + assert "abstract" not in enriched def test_normalize_openalex_keeps_landing_page_out_of_pdf_url() -> None: @@ -1075,7 +1075,7 @@ def test_normalize_openalex_keeps_landing_page_out_of_pdf_url() -> None: assert normalized["source_url"] == "https://doi.org/10.3389/fpubh.2019.00399" -def test_enrich_metadata_local_pdf_corrects_artifact_title_and_fills_arxiv(monkeypatch) -> None: +def test_enrich_metadata_does_not_promote_filename_only_local_pdf(monkeypatch) -> None: semantic_match = { "title": "LLaMA: Open and Efficient Foundation Language Models", "authors": ["Hugo Touvron", "Thibaut Lavril"], @@ -1102,15 +1102,15 @@ def test_enrich_metadata_local_pdf_corrects_artifact_title_and_fills_arxiv(monke } ) - assert enriched["title"] == "LLaMA: Open and Efficient Foundation Language Models" - assert enriched["doi"] == "10.48550/arXiv.2302.13971" - assert enriched["arxiv_id"] == "2302.13971" - assert "semantic_scholar" in enriched["metadata_sources"] - assert enriched["identity_confidence"] == "high" - assert "arxiv_id_present" in enriched["identity_confidence_reasons"] + assert enriched["title"].startswith("Touvron 等 - 2023") + assert "doi" not in enriched + assert "arxiv_id" not in enriched + assert enriched["metadata_sources"] == ["local_pdf"] -def test_enrich_metadata_local_pdf_corrected_title_sets_medium_identity_confidence(monkeypatch) -> None: +def test_enrich_metadata_keeps_low_confidence_without_independent_local_pdf_evidence( + monkeypatch, +) -> None: semantic_match = { "title": "A Strong External Metadata Title For Testing", "authors": ["Alice Example"], @@ -1137,12 +1137,14 @@ def test_enrich_metadata_local_pdf_corrected_title_sets_medium_identity_confiden } ) - assert enriched["title"] == "A Strong External Metadata Title For Testing" - assert enriched["identity_confidence"] == "medium" - assert "external_metadata_title_match" in enriched["identity_confidence_reasons"] + assert enriched["title"].startswith("Li 等 - 2025") + assert enriched["identity_confidence"] == "low" + assert "local_pdf_artifact_title" in enriched["identity_confidence_reasons"] -def test_enrich_metadata_local_pdf_prefers_published_doi_over_preprint(monkeypatch) -> None: +def test_enrich_metadata_does_not_choose_published_doi_from_filename_only_match( + monkeypatch, +) -> None: published = { "title": "Identifying psychiatric manifestations in outpatients with depression and anxiety: a large language model-based approach", "authors": ["Shihao Xu"], @@ -1179,9 +1181,9 @@ def test_enrich_metadata_local_pdf_prefers_published_doi_over_preprint(monkeypat } ) - assert enriched["title"] == "Identifying psychiatric manifestations in outpatients with depression and anxiety: a large language model-based approach" - assert enriched["doi"] == "10.1038/s44184-025-00175-1" - assert enriched["venue"] == "npj Mental Health Research" + assert enriched["title"].startswith("Xu 等 - 2025") + assert "doi" not in enriched + assert "venue" not in enriched def test_enrich_metadata_backfills_arxiv_doi_when_missing(monkeypatch) -> None: diff --git a/tests/test_contracts_consistency.py b/tests/test_contracts_consistency.py index ffeefcd..7bf02ce 100644 --- a/tests/test_contracts_consistency.py +++ b/tests/test_contracts_consistency.py @@ -4,7 +4,7 @@ import re from pathlib import Path -from build_synthesis_bundle import bundle +from build_synthesis_bundle import bundle as build_synthesis_bundle from contracts import ( NOTE_PLAN_LIST_FIELDS, NOTE_PLAN_REQUIRED_FIELDS, @@ -51,6 +51,19 @@ ) +def bundle(**kwargs: object) -> dict: + source_manifest = dict(kwargs.pop("source_manifest", {}) or {}) + source_manifest["identity_contract"] = { + "artifact_type": "canonical_identity", + "schema_version": 2, + "paper_id": "paper:contract-test", + "identity_verdict": "accepted", + "work_level_identity": {"paper_id": "paper:contract-test"}, + "accepted_metadata": {"paper_id": "paper:contract-test"}, + } + return build_synthesis_bundle(source_manifest=source_manifest, **kwargs) + + def test_deleted_reference_routers_stay_removed() -> None: references = SKILL_ROOT / "references" @@ -692,6 +705,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_extract_evidence.py b/tests/test_extract_evidence.py index b5aacf7..0513d4f 100644 --- a/tests/test_extract_evidence.py +++ b/tests/test_extract_evidence.py @@ -13,7 +13,7 @@ except ImportError: # pragma: no cover fitz = None -from build_synthesis_bundle import bundle +from build_synthesis_bundle import bundle as build_synthesis_bundle from common import extract_caption_lines from contracts import NOTE_REQUIRED_SECTIONS import extract_evidence @@ -23,6 +23,25 @@ EXTRACT_EVIDENCE_SCRIPT = PROJECT_ROOT / "skills" / "deeppapernote" / "scripts" / "extract_evidence.py" +def bundle(*, metadata: dict, source_manifest: dict | None = None, **kwargs) -> dict: + manifest = dict(source_manifest or {}) + if "identity_contract" not in manifest: + title = str(metadata.get("title", "")) + manifest["identity_contract"] = { + "artifact_type": "canonical_identity", + "schema_version": 2, + "paper_id": str(metadata.get("paper_id", "paper:test")), + "identity_verdict": "accepted", + "work_level_identity": {"title": title}, + "accepted_metadata": dict(metadata), + } + return build_synthesis_bundle( + metadata=metadata, + source_manifest=manifest, + **kwargs, + ) + + def write_test_pdf(path: Path, pages: list[str]) -> None: if fitz is None: pytest.skip("PyMuPDF is required for PDF coverage integration tests.") @@ -45,6 +64,20 @@ def write_json(path: Path, payload: dict) -> Path: return path +def write_fetch_json(path: Path, payload: dict) -> Path: + artifact = dict(payload) + artifact["status"] = "ok" + artifact["script"] = "fetch_pdf.py" + artifact["identity_contract"] = { + "artifact_type": "canonical_identity", + "schema_version": 2, + "paper_id": artifact.get("paper_id", "paper:test"), + "identity_verdict": "accepted", + "work_level_identity": {"title": artifact.get("title", "")}, + } + return write_json(path, artifact) + + def write_jsonl(path: Path, records: list[dict]) -> Path: path.write_text( "\n".join(json.dumps(record, ensure_ascii=False) for record in records) + "\n", @@ -199,7 +232,7 @@ def test_extract_evidence_outputs_ablation_evidence(tmp_path: Path) -> None: } input_path = tmp_path / "input.json" output_path = tmp_path / "evidence.json" - input_path.write_text(json.dumps(input_payload, ensure_ascii=False), encoding="utf-8") + write_fetch_json(input_path, input_payload) result = subprocess.run( [ @@ -353,7 +386,7 @@ def test_extract_evidence_outputs_pdf_coverage_for_truncated_pdf(tmp_path: Path) } input_path = tmp_path / "input.json" output_path = tmp_path / "evidence.json" - input_path.write_text(json.dumps(input_payload, ensure_ascii=False), encoding="utf-8") + write_fetch_json(input_path, input_payload) result = subprocess.run( [ @@ -421,17 +454,14 @@ def test_extract_evidence_keeps_later_main_result_captions(tmp_path: Path) -> No input_path = tmp_path / "input.json" output_path = tmp_path / "evidence.json" - input_path.write_text( - json.dumps( - { - "paper_id": "paper:many-figures", - "title": "Many Figures Paper", - "abstract": "We study a system.", - "pdf_path": str(pdf_path), - }, - ensure_ascii=False, - ), - encoding="utf-8", + write_fetch_json( + input_path, + { + "paper_id": "paper:many-figures", + "title": "Many Figures Paper", + "abstract": "We study a system.", + "pdf_path": str(pdf_path), + }, ) result = subprocess.run( @@ -496,17 +526,14 @@ def test_extract_evidence_default_scans_short_pdf_without_truncation(tmp_path: P input_path = tmp_path / "input.json" output_path = tmp_path / "evidence.json" - input_path.write_text( - json.dumps( - { - "paper_id": "paper:default-coverage", - "title": "Default Coverage Paper", - "abstract": "We propose a coverage-aware extraction test.", - "pdf_path": str(pdf_path), - }, - ensure_ascii=False, - ), - encoding="utf-8", + write_fetch_json( + input_path, + { + "paper_id": "paper:default-coverage", + "title": "Default Coverage Paper", + "abstract": "We propose a coverage-aware extraction test.", + "pdf_path": str(pdf_path), + }, ) subprocess.run( @@ -542,17 +569,14 @@ def test_extract_evidence_default_truncates_after_32_pages(tmp_path: Path) -> No input_path = tmp_path / "input.json" output_path = tmp_path / "evidence.json" - input_path.write_text( - json.dumps( - { - "paper_id": "paper:default-truncated", - "title": "Default Truncated Paper", - "abstract": "We propose a coverage-aware extraction test.", - "pdf_path": str(pdf_path), - }, - ensure_ascii=False, - ), - encoding="utf-8", + write_fetch_json( + input_path, + { + "paper_id": "paper:default-truncated", + "title": "Default Truncated Paper", + "abstract": "We propose a coverage-aware extraction test.", + "pdf_path": str(pdf_path), + }, ) subprocess.run( @@ -611,7 +635,7 @@ def test_extract_evidence_outputs_appendix_index_and_selective_evidence(tmp_path } input_path = tmp_path / "input.json" output_path = tmp_path / "evidence.json" - input_path.write_text(json.dumps(input_payload, ensure_ascii=False), encoding="utf-8") + write_fetch_json(input_path, input_payload) subprocess.run( [ diff --git a/tests/test_extract_pdf_assets_quality.py b/tests/test_extract_pdf_assets_quality.py index 297d220..0fb4531 100644 --- a/tests/test_extract_pdf_assets_quality.py +++ b/tests/test_extract_pdf_assets_quality.py @@ -45,15 +45,34 @@ def write_test_pdf(path: Path, pages: list[str]) -> None: doc.close() +def write_fetch_input(path: Path, pdf_path: Path, *, title: str) -> None: + path.write_text( + json.dumps( + { + "status": "ok", + "script": "fetch_pdf.py", + "paper_id": "paper:test", + "title": title, + "pdf_path": str(pdf_path), + "identity_contract": { + "artifact_type": "canonical_identity", + "schema_version": 2, + "paper_id": "paper:test", + "identity_verdict": "accepted", + "work_level_identity": {"title": title}, + }, + } + ), + encoding="utf-8", + ) + + def test_extract_pdf_assets_emits_asset_coverage(tmp_path: Path) -> None: pdf_path = tmp_path / "paper.pdf" write_test_pdf(pdf_path, ["Page 1", "Page 2", "Page 3"]) input_path = tmp_path / "input.json" output_path = tmp_path / "assets.json" - input_path.write_text( - json.dumps({"paper_id": "paper:test", "title": "Coverage Paper", "pdf_path": str(pdf_path)}), - encoding="utf-8", - ) + write_fetch_input(input_path, pdf_path, title="Coverage Paper") subprocess.run( [ @@ -85,10 +104,7 @@ def test_extract_pdf_assets_default_scans_short_pdf_without_truncation(tmp_path: write_test_pdf(pdf_path, ["Page 1", "Page 2", "Page 3"]) input_path = tmp_path / "input.json" output_path = tmp_path / "assets.json" - input_path.write_text( - json.dumps({"paper_id": "paper:test", "title": "Coverage Paper", "pdf_path": str(pdf_path)}), - encoding="utf-8", - ) + write_fetch_input(input_path, pdf_path, title="Coverage Paper") subprocess.run( [ @@ -118,10 +134,7 @@ def test_extract_pdf_assets_default_truncates_after_40_pages(tmp_path: Path) -> write_test_pdf(pdf_path, [f"Page {index}" for index in range(1, 42)]) input_path = tmp_path / "input.json" output_path = tmp_path / "assets.json" - input_path.write_text( - json.dumps({"paper_id": "paper:test", "title": "Long Coverage Paper", "pdf_path": str(pdf_path)}), - encoding="utf-8", - ) + write_fetch_input(input_path, pdf_path, title="Long Coverage Paper") subprocess.run( [ diff --git a/tests/test_extract_source_text.py b/tests/test_extract_source_text.py index 54959a6..d34f1f7 100644 --- a/tests/test_extract_source_text.py +++ b/tests/test_extract_source_text.py @@ -31,6 +31,18 @@ def write_test_pdf(path: Path, pages: list[str]) -> None: def run_extract_source(input_path: Path, output_path: Path, *extra: str) -> dict: + payload = json.loads(input_path.read_text(encoding="utf-8")) + if "identity_contract" not in payload: + payload["status"] = "ok" + payload["script"] = "fetch_pdf.py" + payload["identity_contract"] = { + "artifact_type": "canonical_identity", + "schema_version": 2, + "paper_id": payload.get("paper_id", "paper:test"), + "identity_verdict": "accepted", + "work_level_identity": {"title": payload.get("title", "")}, + } + input_path.write_text(json.dumps(payload), encoding="utf-8") subprocess.run( [ sys.executable, @@ -162,6 +174,7 @@ def test_extract_source_text_binds_locations_to_source_manifestation(tmp_path: P "pdf_path": str(pdf_path), "identity_contract": { "artifact_type": "canonical_identity", + "schema_version": 2, "paper_id": "doi:10.1234/published", "identity_verdict": "accepted_with_warnings", "work_level_identity": { diff --git a/tests/test_fetch_pdf.py b/tests/test_fetch_pdf.py index 521a88a..50b2e24 100644 --- a/tests/test_fetch_pdf.py +++ b/tests/test_fetch_pdf.py @@ -1,5 +1,6 @@ from __future__ import annotations +import json from pathlib import Path import pytest @@ -7,39 +8,12 @@ import fetch_pdf -def test_pdf_source_candidates_add_frontiers_pdf_from_doi(monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.setattr("fetch_pdf.enrich_metadata", lambda record: {"pdf_url": ""}) - - candidates = fetch_pdf.pdf_source_candidates( - { - "doi": "10.3389/fpubh.2019.00399", - "title": "The Effectiveness of Crisis Line Services: A Systematic Review", - } - ) - - assert ("pdf_url", "https://www.frontiersin.org/articles/10.3389/fpubh.2019.00399/pdf") in candidates - - -def test_pdf_source_candidates_reuses_existing_pdf_url_without_enrichment( - monkeypatch: pytest.MonkeyPatch, -) -> None: - def fail_enrich_metadata(record: dict) -> dict: - raise AssertionError("metadata artifact already has a direct PDF candidate") - - monkeypatch.setattr("fetch_pdf.enrich_metadata", fail_enrich_metadata) - - candidates = fetch_pdf.pdf_source_candidates( - { - "doi": "10.3389/fpubh.2019.00399", - "title": "Crisis Lines", - "pdf_url": "https://doi.org/10.3389/fpubh.2019.00399", - } - ) - - assert candidates == [ - ("pdf_url", "https://doi.org/10.3389/fpubh.2019.00399"), - ("pdf_url", "https://www.frontiersin.org/articles/10.3389/fpubh.2019.00399/pdf"), - ] +def test_fetch_pdf_has_no_paper_id_override() -> None: + assert "--paper-id" not in { + option + for action in fetch_pdf.parser()._actions + for option in action.option_strings + } def test_fetch_pdf_rejects_html_and_falls_back_to_frontiers_pdf( @@ -47,7 +21,43 @@ def test_fetch_pdf_rejects_html_and_falls_back_to_frontiers_pdf( monkeypatch: pytest.MonkeyPatch, ) -> None: output = tmp_path / "fetch.json" - monkeypatch.setattr("fetch_pdf.enrich_metadata", lambda record: {"pdf_url": ""}) + metadata = tmp_path / "metadata.json" + identity = tmp_path / "identity.json" + metadata.write_text( + json.dumps({"status": "ok", "script": "collect_metadata.py"}), + encoding="utf-8", + ) + identity.write_text( + json.dumps( + { + "status": "ok", + "script": "build_identity_contract.py", + "artifact_type": "canonical_identity", + "schema_version": 2, + "paper_id": "doi:10.3389/fpubh.2019.00399", + "identity_verdict": "accepted", + "work_level_identity": { + "title": "Crisis Lines", + "doi": "10.3389/fpubh.2019.00399", + }, + "source_manifestation": {"source_kind": "doi"}, + "bound_sources": [ + { + "kind": "pdf_url", + "value": "https://doi.org/10.3389/fpubh.2019.00399", + }, + { + "kind": "pdf_url", + "value": ( + "https://www.frontiersin.org/articles/" + "10.3389/fpubh.2019.00399/pdf" + ), + }, + ], + } + ), + encoding="utf-8", + ) requested_urls: list[str] = [] def fake_http_get_bytes(url: str) -> bytes: @@ -63,7 +73,9 @@ def fake_http_get_bytes(url: str) -> bytes: fetch_pdf.main( [ "--input", - '{"doi":"10.3389/fpubh.2019.00399","title":"Crisis Lines","pdf_url":"https://doi.org/10.3389/fpubh.2019.00399"}', + str(metadata), + "--identity", + str(identity), "--dest-dir", str(tmp_path), "--output", @@ -77,3 +89,188 @@ def fake_http_get_bytes(url: str) -> bytes: ] saved_pdf = next((tmp_path / "pdfs").glob("*.pdf")) assert saved_pdf.read_bytes().startswith(b"%PDF-") + + +def test_fetch_pdf_uses_only_identity_bound_sources( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + metadata_path = tmp_path / "metadata.json" + identity_path = tmp_path / "identity.json" + output_path = tmp_path / "fetch.json" + metadata_path.write_text( + json.dumps( + { + "status": "ok", + "script": "collect_metadata.py", + "paper_id": "doi:10.9999/wrong", + "title": "Wrong Candidate", + "pdf_url": "https://example.test/wrong.pdf", + } + ), + encoding="utf-8", + ) + identity_path.write_text( + json.dumps( + { + "status": "ok", + "script": "build_identity_contract.py", + "artifact_type": "canonical_identity", + "schema_version": 2, + "paper_id": "doi:10.1234/trusted", + "identity_verdict": "accepted", + "work_level_identity": { + "title": "Trusted Paper", + "doi": "10.1234/trusted", + }, + "source_manifestation": {"source_kind": "doi"}, + "bound_sources": [ + { + "kind": "pdf_url", + "value": "https://example.test/trusted.pdf", + "provider": "crossref", + "binding_reason": "shared_identifier", + } + ], + } + ), + encoding="utf-8", + ) + requested_urls: list[str] = [] + + def fake_http_get_bytes(url: str) -> bytes: + requested_urls.append(url) + return b"%PDF-1.7\ntrusted" + + monkeypatch.setattr(fetch_pdf, "http_get_bytes", fake_http_get_bytes) + + fetch_pdf.main( + [ + "--input", + str(metadata_path), + "--identity", + str(identity_path), + "--dest-dir", + str(tmp_path), + "--output", + str(output_path), + ] + ) + + assert not hasattr(fetch_pdf, "enrich_metadata") + assert requested_urls == ["https://example.test/trusted.pdf"] + payload = json.loads(output_path.read_text(encoding="utf-8")) + assert payload["paper_id"] == "doi:10.1234/trusted" + assert payload["title"] == "Trusted Paper" + assert payload["pdf_url"] == "https://example.test/trusted.pdf" + + +def test_fetch_pdf_refuses_legacy_identity_without_bound_sources( + tmp_path: Path, +) -> None: + metadata_path = tmp_path / "metadata.json" + identity_path = tmp_path / "identity.json" + metadata_path.write_text( + json.dumps( + { + "status": "ok", + "script": "collect_metadata.py", + "pdf_url": "https://example.test/raw-bypass.pdf", + } + ), + encoding="utf-8", + ) + identity_path.write_text( + json.dumps( + { + "status": "ok", + "script": "build_identity_contract.py", + "artifact_type": "canonical_identity", + "schema_version": 1, + "paper_id": "doi:10.1234/trusted", + "identity_verdict": "accepted", + "work_level_identity": {"doi": "10.1234/trusted"}, + "source_manifestation": {}, + } + ), + encoding="utf-8", + ) + + with pytest.raises(SystemExit, match="schema v2"): + fetch_pdf.main( + [ + "--input", + str(metadata_path), + "--identity", + str(identity_path), + ] + ) + + +def test_fetch_pdf_skips_a_missing_bound_local_file_and_tries_the_next_source( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + metadata_path = tmp_path / "metadata.json" + identity_path = tmp_path / "identity.json" + output_path = tmp_path / "fetch.json" + metadata_path.write_text( + json.dumps({"status": "ok", "script": "collect_metadata.py"}), + encoding="utf-8", + ) + identity_path.write_text( + json.dumps( + { + "status": "ok", + "script": "build_identity_contract.py", + "artifact_type": "canonical_identity", + "schema_version": 2, + "paper_id": "doi:10.1234/trusted", + "identity_verdict": "accepted", + "work_level_identity": { + "title": "Trusted Paper", + "doi": "10.1234/trusted", + }, + "source_manifestation": {}, + "bound_sources": [ + { + "kind": "local_pdf", + "value": str(tmp_path / "missing.pdf"), + }, + { + "kind": "pdf_url", + "value": "https://example.test/trusted.pdf", + }, + ], + } + ), + encoding="utf-8", + ) + monkeypatch.setattr( + fetch_pdf, + "http_get_bytes", + lambda url: b"%PDF-1.7\ntrusted", + ) + + fetch_pdf.main( + [ + "--input", + str(metadata_path), + "--identity", + str(identity_path), + "--dest-dir", + str(tmp_path), + "--output", + str(output_path), + ] + ) + + payload = json.loads(output_path.read_text(encoding="utf-8")) + assert payload["pdf_url"] == "https://example.test/trusted.pdf" + assert payload["attempted_sources"] == [ + { + "kind": "local_pdf", + "path": str(tmp_path / "missing.pdf"), + "status": "missing_file", + } + ] diff --git a/tests/test_resolve_paper_zotero.py b/tests/test_resolve_paper_zotero.py new file mode 100644 index 0000000..06f2325 --- /dev/null +++ b/tests/test_resolve_paper_zotero.py @@ -0,0 +1,509 @@ +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_resolve_title_keeps_input_as_anchor_without_provider_selection( + monkeypatch: pytest.MonkeyPatch, +) -> None: + def fail_provider(*args: object, **kwargs: object) -> object: + raise AssertionError("resolve stage must not query metadata providers") + + monkeypatch.setattr(common, "search_semantic_scholar", fail_provider) + monkeypatch.setattr(common, "search_crossref_by_title", fail_provider) + monkeypatch.setattr(common, "search_openalex_by_title", fail_provider) + monkeypatch.setattr(common, "safe_fetch_arxiv_entries", fail_provider) + + resolved = resolve_paper.resolve_scalar_reference( + "Reliable C++ Agents", + zotero_mode="off", + ) + + assert resolved["source_type"] == "title_query" + assert resolved["title"] == "Reliable C++ Agents" + assert resolved["metadata_sources"] == ["title_query"] + assert "doi" not in resolved + assert "pdf_url" not in resolved + + +@pytest.mark.parametrize( + ("reference", "expected_source_type", "expected_field", "expected_value"), + [ + ("10.1234/trusted", "doi", "doi", "10.1234/trusted"), + ("2401.00001", "arxiv_id", "arxiv_id", "2401.00001"), + ], +) +def test_resolve_strong_reference_keeps_input_as_anchor_without_provider_selection( + monkeypatch: pytest.MonkeyPatch, + reference: str, + expected_source_type: str, + expected_field: str, + expected_value: str, +) -> None: + def fail_provider(*args: object, **kwargs: object) -> object: + raise AssertionError("resolve stage must not query metadata providers") + + monkeypatch.setattr(common, "fetch_crossref_by_doi", fail_provider) + monkeypatch.setattr(common, "safe_fetch_arxiv_entries", fail_provider) + + resolved = resolve_paper.resolve_scalar_reference(reference, zotero_mode="off") + + assert resolved["source_type"] == expected_source_type + assert resolved[expected_field] == expected_value + assert resolved["paper_id"].endswith(expected_value) + + +def test_auto_mode_preserves_input_anchor_and_emits_zotero_observation( + 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: { + "status": "ok", + "source_type": "doi", + "doi": reference, + "paper_id": f"doi:{reference}", + "metadata_sources": ["doi"], + }, + ) + + resolved = resolve_paper.resolve_scalar_reference("10.5555/local") + + assert resolved["source_type"] == "doi" + assert resolved["doi"] == "10.5555/local" + assert "title" not in resolved + assert resolved["identity_observations"] == [ + { + "provider": "zotero", + "retrieved_by": {"kind": "doi", "value": "10.5555/local"}, + "relation": { + "kind": "zotero_lookup", + "match_kind": "doi", + "match_resolution": "unique_exact", + }, + "record": local_record(), + } + ] + assert resolved["zotero_lookup"] == { + "mode": "auto", + "status": "match", + "match_kind": "doi", + "api_status": "available", + "api_version": "3", + "schema_version": "37", + } + + +def test_required_title_lookup_still_emits_an_observation_for_adjudication( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr( + resolve_paper, + "lookup_zotero_local", + lambda reference, reference_type: { + "status": "match", + "match_kind": "title", + "record": local_record(), + }, + ) + monkeypatch.setattr( + resolve_paper, + "resolve_reference", + lambda reference: { + "status": "ok", + "source_type": "title_query", + "title": reference, + "paper_id": "title:query", + "metadata_sources": ["title_query"], + }, + ) + + resolved = resolve_paper.resolve_scalar_reference( + "Local Canonical Title", + zotero_mode="required", + ) + + assert resolved["source_type"] == "title_query" + assert resolved["identity_observations"][0]["provider"] == "zotero" + assert resolved["identity_observations"][0]["relation"] == { + "kind": "zotero_lookup", + "match_kind": "title", + "match_resolution": "unique_exact", + } + + +@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 "abstract" not in enriched + assert enriched["metadata_sources"] == ["zotero"] + + +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..9c4de8f --- /dev/null +++ b/tests/test_zotero_local_api.py @@ -0,0 +1,676 @@ +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="深度学习：方法Ａ")], + 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, + } + + symbol_difference = _match_search_results( + [zotero_item(title="Reliable C Agents")], + match_kind="title", + query="Reliable C++ Agents", + ) + assert symbol_difference == { + "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_identity_when_parent_url_is_malformed( + tmp_path: Path, +) -> None: + pdf_path = tmp_path / "paper.pdf" + pdf_path.write_bytes(b"%PDF-1.4\ntrusted") + parent = zotero_item() + parent["data"]["url"] = "https://[invalid/paper" + routes = base_routes() + routes["/api/users/0/items/PARENT01"] = json_result(parent) + routes["/api/users/0/items/PARENT01/children"] = json_result([zotero_attachment()]) + routes["/api/users/0/items/ATTACH01/file/view/url"] = text_result(pdf_path.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"]["source_url"] == "https://doi.org/10.5555/example" + assert result["record"]["local_pdf_path"] == str(pdf_path.resolve()) + assert result["record"]["zotero_parent_url_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/"]