From 7ebb90bc8484265ca9bb6a2900b99b202f6b9899 Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Thu, 16 Jul 2026 12:30:21 -0400 Subject: [PATCH 1/3] feat: zotero group-wide export, CLI export, per-collection routing, config-less mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes con/docflow#2. Implements the "one command from Zotero to refs.bib" flow for grant-proposal and paper workflows: - ZoteroClient.export_bibtex() now takes collection_key: str | None and format: {bibtex, csljson, ris}. Talks to api.zotero.org directly via urllib to sidestep pyzotero's BibDatabase-for-bibtex return type and to read the Total-Results header for pagination (Zotero caps bibtex/ris responses at 100 items). - `docflow zotero export [--collection] [--format] [--output]` CLI wrapper; stdout by default, respects .docflow/config.yaml's zotero.collection_key with `--collection ""` as the whole-group escape hatch. - `docflow zotero add-dois [FILE] [--collection] [--dry-run]` — one DOI per line from FILE or stdin, `#` comments/blank lines skipped, bioRxiv (10.1101/*) DOIs classified as BIORXIV so version-suffix normalisation applies on dedup. - Per-collection routing on `push`: input JSON may carry a per-citation `collection` field (highest precedence) or `tags`, plus a `--rules RULES.yaml` for tag/doi_prefix matchers with default_collection fallback. Precedence: per-citation > matching rule > rules default > --collection flag > None. - Push loop assigns the destination collection at create_items time (item["collections"] = [...]), not via addto_collection — the latter would need the server-assigned key/version/data.collections fields. - `push` and `add-dois` exit non-zero when any citation failed to push, so downstream Makefiles/CI can detect partial failure. - Config-less operation: all `docflow zotero` subcommands run without a .docflow/config.yaml when ZOTERO_API_KEY is set. If ZOTERO_GROUP_ID isn't set, the library is auto-discovered via /keys/current — used directly when there's exactly one accessible library, or via per-group collection probing when a --collection hint disambiguates across multiple groups. - Makefile template gets refs.bib target (guarded by ZOTERO_API_KEY), a `bib` phony alias, and `bib` in the `all:` recipe. Spec at .specify/specs/zotero-export-and-routing.md captures design, precedence rules, config-less mode, push mechanics, and exit-code invariants. 73 new/updated tests, all AI-generated marked. Co-Authored-By: Claude Code 2.1.210 / Claude Opus 4.7 (1M context) --- .specify/specs/zotero-export-and-routing.md | 305 +++++++++++ docflow/cli/main.py | 347 +++++++++--- docflow/extract/citations.py | 9 +- docflow/integrations/zotero.py | 391 ++++++++++++-- docflow/templates/Makefile.template | 25 +- tests/test_cli.py | 536 +++++++++++++++++++ tests/test_init.py | 42 ++ tests/test_makefile.py | 29 + tests/test_zotero.py | 562 +++++++++++++++++++- 9 files changed, 2109 insertions(+), 137 deletions(-) create mode 100644 .specify/specs/zotero-export-and-routing.md diff --git a/.specify/specs/zotero-export-and-routing.md b/.specify/specs/zotero-export-and-routing.md new file mode 100644 index 0000000..52e478c --- /dev/null +++ b/.specify/specs/zotero-export-and-routing.md @@ -0,0 +1,305 @@ +# Zotero: group-wide export, CLI export, and per-collection routing + +**Issue**: [con/docflow#2](https://github.com/con/docflow/issues/2) + +**Date**: 2026-07-14 +**Status**: In progress (branch `enh-zotero`) +**Scope**: parts (1)–(3) fully; parts (4)–(5) implemented at a level that +covers the concrete use-case in the issue, with room for follow-ups. + +## Summary + +Close the "one command from Zotero to `refs.bib`" loop for grant-proposal +and paper workflows. Three gaps in the current `ZoteroClient` / +`docflow zotero` CLI: + +1. `export_bibtex()` exists in Python but has no CLI subcommand. +2. `export_bibtex()` requires a `collection_key`, so exporting the whole + library (`/groups/{id}/items`) is not supported. +3. `format=bibtex` in Zotero caps responses at 100 items per request; a + library >100 refs is silently truncated. + +Additionally: `docflow zotero push` targets a single collection, and the +generated Makefile has no target for regenerating `refs.bib`. + +## Design + +### 1. `ZoteroClient.export_bibtex()` — group-wide + paginated + formats + +Signature changes: + +```python +def export_bibtex( + self, + collection_key: str | None = None, + output_path: Path | None = None, + format: str = "bibtex", # bibtex | csljson | ris +) -> str: + ... +``` + +Behaviour: + +- `collection_key=None` → export the whole library (`self.zot.items(...)`). +- `collection_key=KEY` → export that collection (existing behaviour, but + now using the same paginated helper). +- `format` is passed straight through as the Zotero `format=` parameter. +- Result is the concatenation of all pages. +- `output_path` still writes the result to disk. + +**Pagination.** Zotero caps `format=bibtex` / `format=ris` responses at +100 items per request; a library >100 items is silently truncated +without explicit pagination. We bypass `pyzotero` entirely for the +export path and talk to `https://api.zotero.org` directly via +`urllib.request` (`_zotero_api_get` low-level helper, `_paginated_export` +generator). Two reasons: + +1. `pyzotero` returns a **parsed** `BibDatabase` object (from + `bibtexparser`) for `format=bibtex`, not a string. Concatenating + these across pages requires re-serialising via `bibtexparser.dumps`, + which introduces formatting drift. +2. `pyzotero` does not surface the `Total-Results` response header, + which we use to decide when to stop paginating. + +The loop: + +```python +def _paginated_export(self, endpoint: str, format: str) -> Iterator[str]: + start = 0 + total: int | None = None + while True: + params = {"format": format, "limit": "100", "start": str(start)} + url = f"{ZOTERO_API_BASE}{endpoint}?{urlencode(params)}" + req = urllib.request.Request(url) + req.add_header("Zotero-API-Version", "3") + req.add_header("Authorization", f"Bearer {self.api_key}") + with urllib.request.urlopen(req, timeout=30) as resp: + yield resp.read().decode("utf-8") + if total is None: + total = int(resp.headers.get("Total-Results", "0")) + start += 100 + if total <= 0 or start >= total: + return +``` + +Result assembly: `bibtex` and `ris` are plain-text pages joined with a +blank-line separator; `csljson` pages are parsed and merged into a +single JSON array which is then pretty-printed. + +### 2. `docflow zotero export` (CLI) + +``` +docflow zotero export [--collection KEY] [--format bibtex|csljson|ris] + [--output PATH] +``` + +- Defaults: `--format=bibtex`, output → stdout, no collection (whole + group). +- Zotero credentials come from `.docflow/config.yaml` when present, or + from env vars when not (see "Config-less operation" below). +- If a config is present and `zotero.collection_key` is set, that value + is used when `--collection` is omitted. Pass `--collection ""` to + force whole-group export even when a config default exists. Without + a config, the default is whole-library export. + +### 3. Per-collection routing on `docflow zotero push` + +Two routing paths, both optional and composable: + +**(a) Per-citation `collection` field.** Any citation loaded from an +input JSON that carries a `collection` key routes to that collection +regardless of the CLI `--collection` flag. Extend `Citation` to carry an +optional `collection` field (defaults to `None`), and thread it through +`push_items` / `check_items`. The CLI accepts either the historical +schema (`{"citations": [{"type": ..., "value": ...}, ...]}`) or the new +one with an added `"collection"` field per entry. + +**(b) `--rules RULES.yaml`.** A tiny YAML: + +```yaml +default_collection: ABCD1234 +routes: + - match: {tag: biosketch} + collection: EFGH5678 + - match: {doi_prefix: "10.1016/j.cell"} + collection: EFGH5678 +``` + +Rules are evaluated top-to-bottom; the first matching rule wins. Match +predicates supported in v1: + +- `tag`: the citation has this tag (see below). +- `doi_prefix`: the citation is a DOI and starts with this prefix. + +Citations don't currently carry tags. We piggy-back on the extraction +side: comment blocks can annotate citations with `#tag` tokens in +context text, and the citation-JSON writer emits them as a +`tags: []` list per citation. In v1 the pathway is: **users author +`{doi, tag}` (or `{doi, collection}`) JSON directly**; comment-derived +tagging is a follow-up. + +Fallback order per citation: +1. `citation.collection` if set on the input. +2. First matching rule from `--rules` (if given). +3. `default_collection` from `--rules` (if given). +4. `--collection` CLI flag. +5. None → item lands in library root (existing behaviour). + +`push_items` gains a per-citation resolution step (`_resolve_collection`). +The push loop iterates **one citation at a time** and calls +`zot.create_items([item])` per citation with the resolved collection +attached to the item's `collections` field. Batching multiple items +into a single `create_items` call is a valid future optimisation but is +not implemented in v1 — pyzotero handles small write batches +transparently, and the deduplication path (`check_items` DOI cache) +already avoids redundant reads. The result dict reports at the +citation level. + +### 4. `docflow zotero add-dois` + +Ergonomic loader for the common "here's a DOI list, put them in Zotero" +flow, so users don't have to hand-craft citation-JSON: + +``` +docflow zotero add-dois [--collection KEY] [--dry-run] [FILE] +``` + +- Reads DOIs from FILE (or stdin if omitted). +- One DOI per line; blank lines and `#`-prefixed comment lines skipped. +- Each line becomes a `Citation` whose type is `BIORXIV` if the DOI + begins with `10.1101/` (so version-suffix normalisation `.v1`/`.v2` + applies), else `DOI`. +- Delegates to `client.push_items(...)`. + +The `--rules` / per-item routing does not apply here (add-dois is +deliberately dumb); users who need routing should compose their own +citation-JSON and use `push`. + +### 5. Makefile template + +Add a `refs.bib` target and a `bib` phony alias. The template is +**always emitted** by `docflow init` (regardless of whether +`--zotero-group-id` was passed); the target is gated at *runtime* by +the same `ZOTERO_API_KEY` env-var check used by `zotero-check` / +`zotero-push`, so it prints an actionable message and exits 1 when the +key is missing rather than silently doing nothing. + +```makefile +.PHONY: bib +bib: refs.bib + +refs.bib: + @if [ -z "$$ZOTERO_API_KEY" ]; then \ + echo "Error: ZOTERO_API_KEY not set"; \ + exit 1; \ + fi + docflow zotero export --output "$@" +``` + +`bib` is also included in the `all:` recipe: + +```makefile +all: sync md tsv comments zotero-check bib +``` + +## Config-less operation + +None of the `docflow zotero` subcommands (`check`, `push`, `export`, +`add-dois`) require a `.docflow/config.yaml`. In env-only mode: + +- `ZOTERO_API_KEY` (or `DOCFLOW_ZOTERO_API_KEY`) — the only strictly + required variable. +- `ZOTERO_GROUP_ID` (or `DOCFLOW_ZOTERO_GROUP_ID`) — optional. If unset, + the library is **auto-discovered** by calling Zotero's + `/keys/current` endpoint: + - If the key grants access to exactly one library (user or group), + that library is used. + - If multiple groups are accessible **and** a `--collection` is + given on the CLI, each accessible group is probed for that + collection; the matching one is used. + - Otherwise `LibraryResolutionError` is raised, listing accessible + libraries so the caller can set `ZOTERO_GROUP_ID` explicitly. +- `ZOTERO_MODE` — optional, defaults to `group`; set to `personal` for + user libraries (with `ZOTERO_GROUP_ID` = user ID). + +`export`'s "config default collection" behaviour (picking up +`zotero.collection_key` when `--collection` is omitted) applies only +when a config file is present; without one, the default is +whole-library export. + +## Push mechanics: single-call item creation + +When routing a citation to a destination collection, `push_items` sets +the item's `collections` field **at creation time** (part of the +`create_items` payload). It does **not** call `addto_collection` +afterwards. Reason: `addto_collection` requires the server-assigned +`key` / `version` / `data.collections` fields of the *created* item — +none of which exist on the local template dict. Assigning the +collection at creation time is atomic (no window during which the item +exists outside any collection) and needs one API call instead of two. + +## Exit codes + +`docflow zotero push` and `docflow zotero add-dois` **must exit with a +non-zero status code (1) whenever at least one citation failed to +push**. A partial success (some added, some failed) is not a success; +downstream Makefiles and CI must be able to detect the failure without +scanning stdout. The failure list is still printed to stdout for the +user. Dry-run mode never fails on this basis (nothing was actually +attempted). + +## Non-goals (this branch) + +- Comment-driven tagging → citation → `--rules {tag: ...}` end-to-end. + Left as a follow-up; v1 assumes the user provides JSON with either + explicit `collection` or a routable field. +- Bulk BibTeX ↔ Zotero round-trip. +- CSL-JSON output for `docflow zotero push`. +- Retries / rate-limit backoff on Zotero API (relies on `pyzotero` + defaults). + +## Test plan + +Unit tests (mocked `pyzotero.Zotero`): + +- `export_bibtex(collection_key=None)` calls `zot.items()`, not + `zot.collection_items()`. +- `export_bibtex` paginates: mock two consecutive pages of length 100 + and a third of length <100; verify result is the concatenation and + the loop stopped. +- `export_bibtex(format="csljson")` returns a JSON string that parses + to a list of the mocked dicts. +- `export_bibtex(format="ris")` passes `format=ris` through. +- Config default: `ZoteroConfig(collection_key=...)` is picked up when + CLI `--collection` is omitted; `--collection ""` overrides to + whole-group. +- Per-citation `collection` field routes correctly and overrides the + CLI flag. +- `--rules` matcher: `doi_prefix` and `tag` each route a citation to + the expected collection; `default_collection` catches unmatched. +- `add-dois` reads DOIs from stdin, drops blank/`#` lines, and + produces a plausible push_items call. +- `add-dois` classifies `10.1101/*` DOIs as `BIORXIV` (so version + suffixes are normalised on dedup). +- `docflow init` writes a Makefile containing the `refs.bib` target + (regardless of whether `--zotero-group-id` was supplied) and lists + `bib` in the `all:` recipe. + +CLI tests: + +- `docflow zotero export --help` shows the new options. +- `docflow zotero export --format bibtex` (no collection) prints to + stdout. +- `docflow zotero export --output /tmp/out.bib` writes the file and + is silent on stdout. +- `docflow zotero add-dois --help` shows options. + +All new tests marked `@pytest.mark.ai_generated`. + +## Rollout + +- No config-schema breaking changes; `ZoteroConfig.collection_key` was + already optional. +- `Citation` gains an optional field with a `None` default — safe. +- `export_bibtex(collection_key, ...)` remains callable positionally + with the same argument as before; only additional kwargs are new. diff --git a/docflow/cli/main.py b/docflow/cli/main.py index e2d0cd1..c2f25f6 100644 --- a/docflow/cli/main.py +++ b/docflow/cli/main.py @@ -1,6 +1,7 @@ """Main CLI entry point for docflow.""" import json +import os import sys from pathlib import Path @@ -8,6 +9,7 @@ from docflow import __version__ from docflow.config import load_config +from docflow.config.loader import ConfigNotFoundError from docflow.convert.spreadsheet import ( convert_spreadsheet, extract_citations_from_spreadsheet, @@ -15,7 +17,81 @@ ) from docflow.extract.citations import Citation, CitationType from docflow.extract.docx import extract_comments_from_docx, format_comments_as_json -from docflow.integrations.zotero import ZoteroClient +from docflow.integrations.zotero import ( + SUPPORTED_EXPORT_FORMATS, + LibraryResolutionError, + RouteMatchError, + RoutingRules, + ZoteroClient, + resolve_library_from_key, +) + + +def _build_zotero_client( + ctx: click.Context, collection_hint: str | None = None +) -> ZoteroClient: + """Construct a ZoteroClient from config, or from env vars if no config exists. + + Env-var fallback: ``ZOTERO_API_KEY`` (or ``DOCFLOW_ZOTERO_API_KEY``) is + the only strictly required variable. If ``ZOTERO_GROUP_ID`` is not set, + the group / user library is auto-discovered from the API key via + Zotero's ``/keys/current`` endpoint; ``collection_hint`` (typically the + CLI ``--collection``) disambiguates across multiple accessible groups. + Also respects ``ZOTERO_MODE`` (``group`` | ``personal``, defaults to + ``group``). Exits the process with an actionable message on failure. + """ + try: + config = load_config(ctx.obj.get("config_path")) + except ConfigNotFoundError: + config = None + except Exception as e: + click.echo(f"Error loading config: {e}", err=True) + sys.exit(1) + + if config is not None: + try: + return ZoteroClient.from_config(config) + except ValueError as e: + click.echo(f"Error: {e}", err=True) + sys.exit(1) + + # No config; try env vars. + api_key = os.environ.get("ZOTERO_API_KEY") or os.environ.get("DOCFLOW_ZOTERO_API_KEY") + group_id = ( + os.environ.get("ZOTERO_GROUP_ID") + or os.environ.get("DOCFLOW_ZOTERO_GROUP_ID") + ) + mode = os.environ.get("ZOTERO_MODE", "group") + + if not api_key: + click.echo( + "Error: no docflow config found, and ZOTERO_API_KEY is not set " + "in the environment.\n" + "Either run `docflow init` in this project, or set:\n" + " export ZOTERO_API_KEY=...", + err=True, + ) + sys.exit(1) + + if not group_id: + try: + group_id, mode = resolve_library_from_key(api_key, collection_hint) + except LibraryResolutionError as e: + click.echo(f"Error: {e}", err=True) + sys.exit(1) + except Exception as e: + click.echo( + f"Error resolving Zotero library from API key: {e}\n" + "Set ZOTERO_GROUP_ID explicitly to skip auto-detection.", + err=True, + ) + sys.exit(1) + + try: + return ZoteroClient(group_id=group_id, api_key=api_key, mode=mode) + except ValueError as e: + click.echo(f"Error: {e}", err=True) + sys.exit(1) @click.group() @@ -472,6 +548,31 @@ def comments( sys.exit(1) +def _load_citations(files: tuple[str, ...]) -> list[Citation]: + """Load Citation objects from citation-JSON files. + + Recognises the optional per-item ``collection`` and ``tags`` fields + introduced for per-collection routing. + """ + citations: list[Citation] = [] + for file_path in files: + path = Path(file_path) + with open(path, encoding="utf-8") as f: + data = json.load(f) + + for cit_data in data.get("citations", []): + citations.append( + Citation( + type=CitationType(cit_data["type"]), + value=cit_data["value"], + context=cit_data.get("context", ""), + collection=cit_data.get("collection"), + tags=list(cit_data.get("tags") or []), + ) + ) + return citations + + @zotero.command() @click.argument("files", nargs=-1, type=click.Path(exists=True), required=True) @click.option("--collection", help="Zotero collection key") @@ -489,43 +590,15 @@ def check( docflow zotero check manuscript_comments.json docflow zotero check --collection ABCD1234 *.json """ - # Load config to get Zotero credentials - try: - config = load_config(ctx.obj.get("config_path")) - except Exception as e: - click.echo(f"Error loading config: {e}", err=True) - click.echo( - "Make sure .docflow/config.yaml exists or set ZOTERO_API_KEY", err=True - ) - sys.exit(1) + client = _build_zotero_client(ctx, collection_hint=collection) - # Get Zotero client + # Load citations from all files try: - client = ZoteroClient.from_config(config) - except ValueError as e: - click.echo(f"Error: {e}", err=True) + all_citations = _load_citations(files) + except Exception as e: + click.echo(f"Error loading citations: {e}", err=True) sys.exit(1) - # Load citations from all files - all_citations = [] - for file_path in files: - path = Path(file_path) - try: - with open(path, encoding="utf-8") as f: - data = json.load(f) - - for cit_data in data.get("citations", []): - all_citations.append( - Citation( - type=CitationType(cit_data["type"]), - value=cit_data["value"], - context=cit_data.get("context", ""), - ) - ) - except Exception as e: - click.echo(f"Error loading {path}: {e}", err=True) - sys.exit(1) - if not all_citations: click.echo("No citations found in input files") return @@ -554,13 +627,19 @@ def check( @zotero.command() @click.argument("files", nargs=-1, type=click.Path(exists=True), required=True) -@click.option("--collection", help="Zotero collection key") +@click.option("--collection", help="Zotero collection key (default destination)") +@click.option( + "--rules", + type=click.Path(exists=True, dir_okay=False), + help="YAML routing rules for per-citation collection assignment", +) @click.option("--dry-run", is_flag=True, help="Preview without actually pushing") @click.pass_context def push( ctx: click.Context, files: tuple[str, ...], collection: str | None, + rules: str | None, dry_run: bool, ) -> None: """Push citations to Zotero library. @@ -568,44 +647,33 @@ def push( Loads citations from JSON files and adds them to your Zotero library. Automatically fetches metadata from CrossRef for DOIs and deduplicates. + Citations in the input JSON may carry an optional ``collection`` field + that overrides ``--collection`` for that entry. For richer routing, + supply ``--rules RULES.yaml``. + \b Examples: docflow zotero push manuscript_comments.json docflow zotero push --dry-run *.json docflow zotero push --collection ABCD1234 file.json + docflow zotero push --rules routes.yaml file.json """ - # Load config - try: - config = load_config(ctx.obj.get("config_path")) - except Exception as e: - click.echo(f"Error loading config: {e}", err=True) - sys.exit(1) + client = _build_zotero_client(ctx, collection_hint=collection) - # Get Zotero client + # Load citations try: - client = ZoteroClient.from_config(config) - except ValueError as e: - click.echo(f"Error: {e}", err=True) + all_citations = _load_citations(files) + except Exception as e: + click.echo(f"Error loading citations: {e}", err=True) sys.exit(1) - # Load citations - all_citations = [] - for file_path in files: - path = Path(file_path) + # Load routing rules if requested + routing: RoutingRules | None = None + if rules: try: - with open(path, encoding="utf-8") as f: - data = json.load(f) - - for cit_data in data.get("citations", []): - all_citations.append( - Citation( - type=CitationType(cit_data["type"]), - value=cit_data["value"], - context=cit_data.get("context", ""), - ) - ) - except Exception as e: - click.echo(f"Error loading {path}: {e}", err=True) + routing = RoutingRules.from_file(Path(rules)) + except (RouteMatchError, OSError) as e: + click.echo(f"Error loading rules: {e}", err=True) sys.exit(1) if not all_citations: @@ -619,7 +687,7 @@ def push( # Push items result = client.push_items( - all_citations, collection_key=collection, dry_run=dry_run + all_citations, collection_key=collection, dry_run=dry_run, rules=routing ) # Display results @@ -640,6 +708,159 @@ def push( click.echo("\nFailed to add:") for cit in result["failed"]: click.echo(f" • {cit.type.value}: {cit.value}") + sys.exit(1) + + +@zotero.command() +@click.option( + "--collection", + default=None, + help="Zotero collection key; omit to export the whole library. " + "Pass an empty string to override a configured default.", +) +@click.option( + "--format", + "-f", + "format_", + type=click.Choice(list(SUPPORTED_EXPORT_FORMATS)), + default="bibtex", + show_default=True, + help="Export format", +) +@click.option( + "--output", + "-o", + type=click.Path(), + default=None, + help="Output file path [default: stdout]", +) +@click.pass_context +def export( + ctx: click.Context, + collection: str | None, + format_: str, + output: str | None, +) -> None: + """Export Zotero library (or a collection) to BibTeX/CSL-JSON/RIS. + + \b + Examples: + docflow zotero export > refs.bib + docflow zotero export --output refs.bib + docflow zotero export --collection ABCD1234 --format csljson + docflow zotero export --format ris -o refs.ris + """ + client = _build_zotero_client(ctx, collection_hint=collection or None) + + # Resolve collection: CLI wins. ``--collection ""`` explicitly means + # "no collection" (whole library). When neither the CLI flag nor a + # config default is present, whole-library export is the default. + config_default: str | None = None + try: + cfg = load_config(ctx.obj.get("config_path")) + if cfg.zotero: + config_default = cfg.zotero.collection_key + except ConfigNotFoundError: + pass + except Exception: + # Non-fatal – client already built from env vars if we got here. + pass + + if collection is None: + coll_key: str | None = config_default + elif collection == "": + coll_key = None + else: + coll_key = collection + + try: + content = client.export_bibtex( + collection_key=coll_key, + output_path=Path(output) if output else None, + format=format_, + ) + except ValueError as e: + click.echo(f"Error: {e}", err=True) + sys.exit(1) + + if output: + click.echo(f"Wrote {len(content)} bytes to {output}", err=True) + else: + click.echo(content, nl=False) + + +@zotero.command(name="add-dois") +@click.argument("file", type=click.Path(exists=True, dir_okay=False), required=False) +@click.option("--collection", help="Zotero collection key") +@click.option("--dry-run", is_flag=True, help="Preview without actually pushing") +@click.pass_context +def add_dois( + ctx: click.Context, + file: str | None, + collection: str | None, + dry_run: bool, +) -> None: + """Push a plain-text list of DOIs to Zotero. + + One DOI per line; blank lines and lines starting with '#' are ignored. + Reads from FILE if given, otherwise from stdin. + + \b + Examples: + docflow zotero add-dois --collection ABCD1234 < dois.txt + docflow zotero add-dois dois.txt + """ + client = _build_zotero_client(ctx, collection_hint=collection) + + if file: + text = Path(file).read_text(encoding="utf-8") + else: + text = sys.stdin.read() + + dois: list[str] = [] + for raw in text.splitlines(): + line = raw.strip() + if not line or line.startswith("#"): + continue + dois.append(line) + + if not dois: + click.echo("No DOIs found on input", err=True) + return + + citations = [ + Citation( + type=( + CitationType.BIORXIV + if doi.startswith("10.1101/") + else CitationType.DOI + ), + value=doi, + ) + for doi in dois + ] + + click.echo(f"Loaded {len(citations)} DOI(s)") + if dry_run: + click.echo("DRY RUN - No changes will be made") + + result = client.push_items( + citations, collection_key=collection, dry_run=dry_run + ) + + if dry_run: + click.echo(f"\n✓ Would add: {len(result['new'])}") + click.echo(f" Would skip (existing): {len(result['existing'])}") + else: + click.echo(f"\n✓ Added: {len(result['added'])}") + click.echo(f" Skipped (existing): {len(result['skipped'])}") + click.echo(f" Failed: {len(result['failed'])}") + + if result["failed"]: + click.echo("\nFailed to add:") + for cit in result["failed"]: + click.echo(f" • {cit.type.value}: {cit.value}") + sys.exit(1) @main.command() diff --git a/docflow/extract/citations.py b/docflow/extract/citations.py index 09de989..9cce91a 100644 --- a/docflow/extract/citations.py +++ b/docflow/extract/citations.py @@ -26,6 +26,8 @@ class Citation: value: str context: str = field(default="") # Surrounding text raw_text: str = field(default="") # Original text containing citation + collection: str | None = field(default=None) # Optional Zotero collection key + tags: list[str] = field(default_factory=list) # Optional tags for routing def normalize(self) -> str: """Normalize citation value (e.g., strip bioRxiv version). @@ -46,12 +48,17 @@ def to_dict(self) -> dict[str, Any]: Returns: Dictionary representation """ - return { + d: dict[str, Any] = { "type": self.type.value, "value": self.value, "normalized": self.normalize(), "context": self.context, } + if self.collection is not None: + d["collection"] = self.collection + if self.tags: + d["tags"] = list(self.tags) + return d # Regex patterns for citation detection diff --git a/docflow/integrations/zotero.py b/docflow/integrations/zotero.py index e5114cc..161fa6c 100644 --- a/docflow/integrations/zotero.py +++ b/docflow/integrations/zotero.py @@ -4,10 +4,12 @@ import logging import re import urllib.error +import urllib.parse import urllib.request from pathlib import Path from typing import Any +import yaml from pyzotero import zotero from docflow.config import DocflowConfig, get_zotero_api_key @@ -15,6 +17,184 @@ logger = logging.getLogger(__name__) +ZOTERO_API_BASE = "https://api.zotero.org" +ZOTERO_EXPORT_PAGE_SIZE = 100 # Zotero cap for format=bibtex/ris responses +SUPPORTED_EXPORT_FORMATS = ("bibtex", "csljson", "ris") + + +class LibraryResolutionError(ValueError): + """Raised when a Zotero library cannot be resolved from just an API key.""" + + +def _zotero_api_get( + path: str, api_key: str, *, allow_404: bool = False +) -> tuple[int, bytes]: + """Low-level GET on the Zotero API. Returns ``(status, body_bytes)``. + + When ``allow_404`` is True, a 404 response is returned as + ``(404, b"")`` instead of raising. + """ + url = f"{ZOTERO_API_BASE}{path}" + req = urllib.request.Request(url) + req.add_header("Zotero-API-Version", "3") + req.add_header("Authorization", f"Bearer {api_key}") + try: + with urllib.request.urlopen(req, timeout=15) as resp: # nosec B310 + return resp.status, resp.read() + except urllib.error.HTTPError as e: + if allow_404 and e.code == 404: + return 404, b"" + raise + + +def resolve_library_from_key( + api_key: str, + collection_hint: str | None = None, +) -> tuple[str, str]: + """Return ``(library_id, mode)`` given only a Zotero API key. + + Queries ``/keys/current`` to enumerate accessible libraries and + picks one: + + * If exactly one library is accessible (user or one group), use it. + * If multiple groups are accessible and ``collection_hint`` is given, + probe each group for that collection and pick the group that + contains it. + * Otherwise raise ``LibraryResolutionError`` listing the accessible + libraries so the caller can set ``ZOTERO_GROUP_ID`` explicitly. + + ``mode`` is ``"group"`` for a group library or ``"personal"`` for the + user library; ``library_id`` is the group ID or user ID respectively. + """ + status, body = _zotero_api_get("/keys/current", api_key) + info = json.loads(body.decode("utf-8")) + access = info.get("access") or {} + user_id = info.get("userID") + + user_accessible = bool( + (access.get("user") or {}).get("library") + ) and user_id is not None + + # 'all' is a pseudo-group entry that Zotero includes; skip it. + group_ids = sorted( + gid + for gid, perms in (access.get("groups") or {}).items() + if gid != "all" and (perms or {}).get("library") + ) + + candidates: list[tuple[str, str]] = [] + if user_accessible: + candidates.append((str(user_id), "personal")) + candidates.extend((gid, "group") for gid in group_ids) + + if not candidates: + raise LibraryResolutionError( + "Zotero API key grants no library access" + ) + if len(candidates) == 1: + return candidates[0] + + # Ambiguous: try disambiguating via the collection hint. + if collection_hint: + matches: list[tuple[str, str]] = [] + for lib_id, mode in candidates: + path = ( + f"/groups/{lib_id}/collections/{collection_hint}" + if mode == "group" + else f"/users/{lib_id}/collections/{collection_hint}" + ) + status, _ = _zotero_api_get(path, api_key, allow_404=True) + if status == 200: + matches.append((lib_id, mode)) + if len(matches) == 1: + return matches[0] + if len(matches) > 1: + listing = ", ".join(f"{m[1]}:{m[0]}" for m in matches) + raise LibraryResolutionError( + f"Collection {collection_hint!r} exists in multiple accessible " + f"libraries ({listing}); set ZOTERO_GROUP_ID to disambiguate" + ) + raise LibraryResolutionError( + f"Collection {collection_hint!r} not found in any accessible library " + f"({', '.join(f'{m[1]}:{m[0]}' for m in candidates)}); " + f"check the collection key or set ZOTERO_GROUP_ID" + ) + + listing = ", ".join(f"{m[1]}:{m[0]}" for m in candidates) + raise LibraryResolutionError( + f"Multiple accessible Zotero libraries ({listing}); " + f"set ZOTERO_GROUP_ID (or pass --collection to auto-detect from the " + f"collection's group)" + ) + + +class RouteMatchError(ValueError): + """Raised when a routing rules file is malformed.""" + + +class RoutingRule: + """One collection-routing rule loaded from a rules YAML file. + + Match predicates supported: ``tag`` (citation has this tag) and + ``doi_prefix`` (DOI-typed citation whose value starts with the prefix). + """ + + def __init__(self, match: dict[str, Any], collection: str): + self.match = dict(match) + self.collection = collection + + def matches(self, citation: Citation) -> bool: + if "tag" in self.match: + if self.match["tag"] not in (citation.tags or []): + return False + if "doi_prefix" in self.match: + if citation.type not in ( + CitationType.DOI, + CitationType.BIORXIV, + CitationType.MEDRXIV, + ): + return False + if not citation.value.startswith(self.match["doi_prefix"]): + return False + # An empty match dict never matches (avoids accidental catch-all). + return bool(self.match) + + +class RoutingRules: + """A parsed --rules YAML for ``docflow zotero push``.""" + + def __init__( + self, + default_collection: str | None = None, + routes: list[RoutingRule] | None = None, + ): + self.default_collection = default_collection + self.routes: list[RoutingRule] = list(routes or []) + + @classmethod + def from_file(cls, path: Path) -> "RoutingRules": + """Load routing rules from a YAML file.""" + data = yaml.safe_load(path.read_text()) or {} + if not isinstance(data, dict): + raise RouteMatchError( + f"Routing rules must be a mapping at top level, got {type(data).__name__}" + ) + routes: list[RoutingRule] = [] + for i, entry in enumerate(data.get("routes") or []): + if not isinstance(entry, dict): + raise RouteMatchError(f"routes[{i}] must be a mapping") + match = entry.get("match") + coll = entry.get("collection") + if not isinstance(match, dict) or not match: + raise RouteMatchError(f"routes[{i}] requires a non-empty 'match'") + if not isinstance(coll, str) or not coll: + raise RouteMatchError(f"routes[{i}] requires a 'collection' string") + routes.append(RoutingRule(match=match, collection=coll)) + default_coll = data.get("default_collection") + if default_coll is not None and not isinstance(default_coll, str): + raise RouteMatchError("default_collection must be a string if provided") + return cls(default_collection=default_coll, routes=routes) + class ZoteroClient: """Client for Zotero API with deduplication and batch operations.""" @@ -91,7 +271,9 @@ def check_items( Args: citations: List of Citation objects - collection_key: Optional Zotero collection key + collection_key: Optional Zotero collection key. If any citation + carries a non-None ``collection`` attribute, that per-citation + value takes precedence for that citation. Returns: Dictionary with 'new', 'existing', and 'failed' citation lists @@ -102,23 +284,30 @@ def check_items( "failed": [], } - # Get existing items in the library/collection - if collection_key: - existing_items = self.zot.collection_items(collection_key) - else: - existing_items = self.zot.items() + # Cache existing-DOI lookup by (resolved) collection scope so we don't + # refetch on every citation when routing spreads them across + # different collections. + doi_cache: dict[str | None, set[str]] = {} - # Build set of existing DOIs for quick lookup - existing_dois = set() - for item in existing_items: - doi = item["data"].get("DOI") - if doi: - # Normalize DOI for comparison - normalized = self._normalize_doi(doi) - existing_dois.add(normalized) + def _dois_for(scope: str | None) -> set[str]: + if scope in doi_cache: + return doi_cache[scope] + if scope: + existing = self.zot.collection_items(scope) + else: + existing = self.zot.items() + dois: set[str] = set() + for item in existing: + doi = item["data"].get("DOI") + if doi: + dois.add(self._normalize_doi(doi)) + doi_cache[scope] = dois + return dois # Check each citation for citation in citations: + scope = citation.collection or collection_key + existing_dois = _dois_for(scope) if citation.type in (CitationType.DOI, CitationType.BIORXIV): normalized = citation.normalize() if normalized in existing_dois: @@ -140,17 +329,27 @@ def push_items( citations: list[Citation], collection_key: str | None = None, dry_run: bool = False, + rules: "RoutingRules | None" = None, ) -> dict[str, Any]: """Push citations to Zotero with deduplication. Args: citations: List of Citation objects - collection_key: Optional Zotero collection key + collection_key: Optional default Zotero collection key dry_run: If True, don't actually push (preview only) + rules: Optional routing rules; per-citation ``collection`` on the + Citation itself always takes highest precedence. Returns: Dictionary with 'added', 'skipped', and 'failed' results """ + # Resolve the destination collection for every citation up front so + # deduplication and reporting operate on the resolved values. + resolved = [(c, self._resolve_collection(c, collection_key, rules)) + for c in citations] + for c, coll in resolved: + c.collection = coll + if dry_run: logger.info("DRY RUN - No changes will be made") return self.check_items(citations, collection_key) @@ -169,12 +368,24 @@ def push_items( try: item = self._create_item_from_citation(citation) if item: - # Create item in Zotero - resp = self.zot.create_items([item]) + # Assign collection at creation time — Zotero item data + # accepts a ``collections`` array, avoiding a separate + # addto_collection call (which needs key/version from the + # server response). + dest = citation.collection + if dest: + existing = list(item.get("collections") or []) + if dest not in existing: + existing.append(dest) + item["collections"] = existing - # Add to collection if specified - if collection_key and resp["successful"]: - self.zot.addto_collection(collection_key, item) + resp = self.zot.create_items([item]) + if not resp.get("successful"): + result["failed"].append(citation) + logger.warning( + f"Zotero rejected item for {citation.value}: {resp.get('failed')}" + ) + continue result["added"].append(citation) logger.info(f"Added: {citation.type.value} {citation.value}") @@ -189,45 +400,141 @@ def push_items( return result + @staticmethod + def _resolve_collection( + citation: Citation, + default_collection: str | None, + rules: "RoutingRules | None", + ) -> str | None: + """Resolve destination collection for a citation. + + Precedence (highest first): + 1. Explicit ``citation.collection`` + 2. First matching rule in ``rules.routes`` + 3. ``rules.default_collection`` + 4. ``default_collection`` (CLI flag) + 5. ``None`` + """ + if citation.collection: + return citation.collection + if rules is not None: + for route in rules.routes: + if route.matches(citation): + return route.collection + if rules.default_collection: + return rules.default_collection + return default_collection + def export_bibtex( self, - collection_key: str, + collection_key: str | None = None, output_path: Path | None = None, + format: str = "bibtex", ) -> str: - """Export collection to BibTeX format. + """Export library or collection in the requested citation format. + + Talks to the Zotero HTTP API directly so that pagination (Zotero + caps ``format=bibtex``/``ris`` responses at 100 items) and + non-JSON formats can be handled uniformly. Args: - collection_key: Zotero collection key - output_path: Optional path to write output + collection_key: Zotero collection key. If ``None``, exports the + whole library (group or user, per the client mode). + output_path: Optional path to write output. + format: Export format – one of ``bibtex``, ``csljson``, ``ris``. Returns: - BibTeX string + The exported bibliography as a string. For ``csljson`` the + string is a JSON array (pretty-printed). Raises: - ValueError: If collection not found + ValueError: If the format is unsupported or the API call fails. """ + if format not in SUPPORTED_EXPORT_FORMATS: + raise ValueError( + f"Unsupported export format {format!r}; " + f"expected one of {SUPPORTED_EXPORT_FORMATS}" + ) + + endpoint = self._items_endpoint(collection_key) try: - # Get items in collection - items = self.zot.collection_items(collection_key) + pages = list(self._paginated_export(endpoint, format)) + except (urllib.error.HTTPError, urllib.error.URLError) as e: + raise ValueError(f"Failed to export from Zotero: {e}") from e + + if format == "csljson": + merged: list[Any] = [] + for page in pages: + merged.extend(json.loads(page) if page else []) + result = json.dumps(merged, indent=2, ensure_ascii=False) + else: + # bibtex / ris are plain text; concatenate with a blank line + # between pages so entries near a page boundary stay separated. + non_empty = [p.strip() for p in pages if p and p.strip()] + result = "\n\n".join(non_empty) + ("\n" if non_empty else "") + + if output_path: + output_path.write_text(result) + logger.info( + "Exported %s: %d bytes to %s", + format, + len(result), + output_path, + ) - if not items: - logger.warning(f"No items found in collection {collection_key}") - return "" + return result - # Get BibTeX export - # pyzotero doesn't have direct BibTeX export, so we use the API - bibtex: str = self.zot.collection_items( # type: ignore[assignment] - collection_key, format="bibtex" - ) + def _items_endpoint(self, collection_key: str | None) -> str: + """Build the Zotero API endpoint for items or collection items.""" + library_type = "groups" if self.mode == "group" else "users" + library_id = self.group_id or "self" + if collection_key: + return f"/{library_type}/{library_id}/collections/{collection_key}/items" + return f"/{library_type}/{library_id}/items" - if output_path: - output_path.write_text(bibtex) - logger.info(f"Exported {len(items)} items to {output_path}") + def _paginated_export(self, endpoint: str, format: str) -> Any: + """Yield successive pages of raw text for a Zotero export endpoint. - return bibtex + Uses ``start``/``limit`` pagination. Stops when a page returns + fewer than ``ZOTERO_EXPORT_PAGE_SIZE`` items — measured differently + per format: - except Exception as e: - raise ValueError(f"Failed to export collection: {e}") + * ``bibtex`` / ``ris``: count ``@`` / blank-line-separated + records via the ``Total-Results`` response header. + * ``csljson``: count the JSON array length. + """ + start = 0 + total: int | None = None + while True: + params = { + "format": format, + "limit": str(ZOTERO_EXPORT_PAGE_SIZE), + "start": str(start), + } + url = ( + f"{ZOTERO_API_BASE}{endpoint}?" + + urllib.parse.urlencode(params) + ) + req = urllib.request.Request(url) + req.add_header("Zotero-API-Version", "3") + req.add_header("Authorization", f"Bearer {self.api_key}") + logger.debug("Zotero GET %s", url) + with urllib.request.urlopen(req, timeout=30) as resp: # nosec B310 + body_bytes = resp.read() + headers = resp.headers + body = body_bytes.decode("utf-8") + yield body + + if total is None: + try: + total = int(headers.get("Total-Results", "0")) + except (TypeError, ValueError): + total = 0 + logger.debug("Zotero export: Total-Results=%s", total) + + start += ZOTERO_EXPORT_PAGE_SIZE + if total <= 0 or start >= total: + return def _create_item_from_citation(self, citation: Citation) -> dict[str, Any] | None: """Create Zotero item from citation. diff --git a/docflow/templates/Makefile.template b/docflow/templates/Makefile.template index bc8507b..a0510b6 100644 --- a/docflow/templates/Makefile.template +++ b/docflow/templates/Makefile.template @@ -14,7 +14,7 @@ # The $(wildcard) function will NOT find files with spaces, so pattern rules # won't match them. This is a fundamental Make limitation. # -.PHONY: sync md tsv comments zotero-check zotero-push all clean default help +.PHONY: sync md tsv comments zotero-check zotero-push bib all clean default help .DEFAULT_GOAL := default # Configuration @@ -36,8 +36,8 @@ converted: # Default: sync from Google Drive, convert all files, extract comments and citations default: sync md tsv comments -# Complete workflow including Zotero check -all: sync md tsv comments zotero-check +# Complete workflow including Zotero check and bibliography export +all: sync md tsv comments zotero-check bib # Sync documents from Google Drive using rclone # Works for both files and directories (use trailing / for directories) @@ -108,6 +108,23 @@ zotero-push: echo "No citation files found in converted/"; \ fi +# Regenerate refs.bib from Zotero (whole library, or config-default collection) +bib: refs.bib + +refs.bib: + @if [ -z "$$ZOTERO_API_KEY" ]; then \ + echo "Error: ZOTERO_API_KEY not set"; \ + echo ""; \ + echo "To export the bibliography:"; \ + echo "1. Get API key from https://www.zotero.org/settings/keys"; \ + echo "2. Export it: export ZOTERO_API_KEY='your-key-here'"; \ + echo "3. Run: make refs.bib"; \ + exit 1; \ + fi + @echo "Exporting Zotero library to $@..." + docflow zotero export --output "$@" + @echo "✓ Wrote $@" + # Clean generated files (markdown, tsv, comments, citations) clean: @echo "Cleaning generated files..." @@ -124,5 +141,7 @@ help: @echo " make comments - Extract comments from .docx files" @echo " make zotero-check - Check what would be added to Zotero" @echo " make zotero-push - Push citations to Zotero" + @echo " make refs.bib - Export Zotero library to refs.bib" + @echo " make bib - Alias for refs.bib" @echo " make all - Run full workflow + Zotero check" @echo " make clean - Remove converted/ directory" diff --git a/tests/test_cli.py b/tests/test_cli.py index 3cec1f7..487d6ec 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,5 +1,9 @@ """Tests for CLI interface.""" +import json +from pathlib import Path +from unittest.mock import MagicMock, patch + import pytest from click.testing import CliRunner @@ -12,6 +16,29 @@ def cli_runner() -> CliRunner: return CliRunner() +def _write_config( + tmp_path: Path, + group_id: str = "123", + collection_key: str | None = None, +) -> Path: + """Write a minimal .docflow/config.yaml under tmp_path and return the path.""" + docflow_dir = tmp_path / ".docflow" + docflow_dir.mkdir() + cfg = docflow_dir / "config.yaml" + body = f"""project: + name: "T" + type: "generic" +zotero: + mode: "group" + group_id: "{group_id}" + api_key_env: "ZOTERO_API_KEY" +""" + if collection_key is not None: + body += f' collection_key: "{collection_key}"\n' + cfg.write_text(body) + return cfg + + def test_cli_help(cli_runner: CliRunner) -> None: """Test that CLI help works.""" result = cli_runner.invoke(main, ["--help"]) @@ -46,3 +73,512 @@ def test_init_help(cli_runner: CliRunner) -> None: result = cli_runner.invoke(main, ["init", "--help"]) assert result.exit_code == 0 assert "init" in result.output.lower() + + +# --------------------------------------------------------------------------- +# `docflow zotero export` +# --------------------------------------------------------------------------- + + +@pytest.mark.ai_generated +def test_zotero_export_help(cli_runner: CliRunner) -> None: + result = cli_runner.invoke(main, ["zotero", "export", "--help"]) + assert result.exit_code == 0 + assert "--collection" in result.output + assert "--format" in result.output + assert "--output" in result.output + for fmt in ("bibtex", "csljson", "ris"): + assert fmt in result.output + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.urllib.request.urlopen") +@patch("docflow.integrations.zotero.zotero") +def test_zotero_export_to_stdout( + mock_pyz, mock_urlopen, cli_runner: CliRunner, tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """`docflow zotero export` with no --output writes bibtex to stdout.""" + monkeypatch.setenv("ZOTERO_API_KEY", "k") + cfg = _write_config(tmp_path) + + # Mock urlopen response + resp = MagicMock() + resp.read.return_value = b"@article{foo, title={T}}" + resp.headers = {"Total-Results": "1"} + resp.__enter__.return_value = resp + resp.__exit__.return_value = False + mock_urlopen.return_value = resp + + result = cli_runner.invoke( + main, ["--config", str(cfg), "zotero", "export"] + ) + assert result.exit_code == 0, result.output + assert "@article{foo" in result.output + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.urllib.request.urlopen") +@patch("docflow.integrations.zotero.zotero") +def test_zotero_export_to_file( + mock_pyz, mock_urlopen, cli_runner: CliRunner, tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("ZOTERO_API_KEY", "k") + cfg = _write_config(tmp_path) + + resp = MagicMock() + resp.read.return_value = b"@article{x}" + resp.headers = {"Total-Results": "1"} + resp.__enter__.return_value = resp + resp.__exit__.return_value = False + mock_urlopen.return_value = resp + + out = tmp_path / "refs.bib" + result = cli_runner.invoke( + main, + ["--config", str(cfg), "zotero", "export", "--output", str(out)], + ) + assert result.exit_code == 0, result.output + assert out.exists() + assert "@article{x}" in out.read_text() + # stdout should be silent about the bibtex content itself + assert "@article{x}" not in result.output + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.urllib.request.urlopen") +@patch("docflow.integrations.zotero.zotero") +def test_zotero_export_format_forwarded( + mock_pyz, mock_urlopen, cli_runner: CliRunner, tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("ZOTERO_API_KEY", "k") + cfg = _write_config(tmp_path) + + resp = MagicMock() + resp.read.return_value = b"TY - JOUR\nER - " + resp.headers = {"Total-Results": "1"} + resp.__enter__.return_value = resp + resp.__exit__.return_value = False + mock_urlopen.return_value = resp + + result = cli_runner.invoke( + main, ["--config", str(cfg), "zotero", "export", "--format", "ris"] + ) + assert result.exit_code == 0, result.output + url = mock_urlopen.call_args[0][0].full_url + assert "format=ris" in url + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.urllib.request.urlopen") +@patch("docflow.integrations.zotero.zotero") +def test_zotero_export_uses_config_default_collection( + mock_pyz, mock_urlopen, cli_runner: CliRunner, tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """`zotero.collection_key` from config is used when --collection is omitted.""" + monkeypatch.setenv("ZOTERO_API_KEY", "k") + cfg = _write_config(tmp_path, collection_key="ABCD1234") + + resp = MagicMock() + resp.read.return_value = b"@article{x}" + resp.headers = {"Total-Results": "1"} + resp.__enter__.return_value = resp + resp.__exit__.return_value = False + mock_urlopen.return_value = resp + + result = cli_runner.invoke(main, ["--config", str(cfg), "zotero", "export"]) + assert result.exit_code == 0, result.output + url = mock_urlopen.call_args[0][0].full_url + assert "/collections/ABCD1234/items" in url + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.urllib.request.urlopen") +@patch("docflow.integrations.zotero.zotero") +def test_zotero_export_empty_collection_overrides_config_default( + mock_pyz, mock_urlopen, cli_runner: CliRunner, tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """`--collection ""` forces whole-group export even with a config default.""" + monkeypatch.setenv("ZOTERO_API_KEY", "k") + cfg = _write_config(tmp_path, collection_key="ABCD1234") + + resp = MagicMock() + resp.read.return_value = b"@article{x}" + resp.headers = {"Total-Results": "1"} + resp.__enter__.return_value = resp + resp.__exit__.return_value = False + mock_urlopen.return_value = resp + + result = cli_runner.invoke( + main, ["--config", str(cfg), "zotero", "export", "--collection", ""] + ) + assert result.exit_code == 0, result.output + url = mock_urlopen.call_args[0][0].full_url + assert "/collections/" not in url + assert "/groups/123/items" in url + + +# --------------------------------------------------------------------------- +# `docflow zotero add-dois` +# --------------------------------------------------------------------------- + + +@pytest.mark.ai_generated +def test_zotero_add_dois_help(cli_runner: CliRunner) -> None: + result = cli_runner.invoke(main, ["zotero", "add-dois", "--help"]) + assert result.exit_code == 0 + assert "DOI" in result.output + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.zotero") +def test_zotero_add_dois_from_stdin_dry_run( + mock_pyz, cli_runner: CliRunner, tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("ZOTERO_API_KEY", "k") + cfg = _write_config(tmp_path) + + zot_instance = MagicMock() + zot_instance.items.return_value = [] + mock_pyz.Zotero.return_value = zot_instance + + stdin = "10.1038/nature12345\n\n# a comment\n10.1101/2023.01.01\n" + result = cli_runner.invoke( + main, + ["--config", str(cfg), "zotero", "add-dois", "--dry-run"], + input=stdin, + ) + assert result.exit_code == 0, result.output + # 2 DOIs, both new since library is empty + assert "Loaded 2 DOI(s)" in result.output + assert "Would add: 2" in result.output + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.zotero") +def test_zotero_add_dois_no_config_env_only( + mock_pyz, cli_runner: CliRunner, tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Runs without a .docflow/config.yaml when env vars are set.""" + monkeypatch.setenv("ZOTERO_API_KEY", "k") + monkeypatch.setenv("ZOTERO_GROUP_ID", "999") + # Isolate cwd so config lookup doesn't accidentally pick something up + monkeypatch.chdir(tmp_path) + + zot_instance = MagicMock() + zot_instance.items.return_value = [] + zot_instance.collection_items.return_value = [] + mock_pyz.Zotero.return_value = zot_instance + + result = cli_runner.invoke( + main, + ["zotero", "add-dois", "--collection", "BZ69VS62", "--dry-run"], + input="10.1038/sdata.2016.44\n10.7554/eLife.71774\n", + ) + assert result.exit_code == 0, result.output + assert "Loaded 2 DOI(s)" in result.output + + # pyzotero.Zotero() was called with (group_id, "group", api_key) + mock_pyz.Zotero.assert_called_once_with("999", "group", "k") + + +@pytest.mark.ai_generated +def test_zotero_add_dois_no_config_no_env( + cli_runner: CliRunner, tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Fails with an actionable error when neither config nor API key set.""" + monkeypatch.delenv("ZOTERO_API_KEY", raising=False) + monkeypatch.delenv("ZOTERO_GROUP_ID", raising=False) + monkeypatch.delenv("DOCFLOW_ZOTERO_API_KEY", raising=False) + monkeypatch.delenv("DOCFLOW_ZOTERO_GROUP_ID", raising=False) + monkeypatch.chdir(tmp_path) + + result = cli_runner.invoke( + main, + ["zotero", "add-dois", "--collection", "X", "--dry-run"], + input="10.1/x\n", + ) + assert result.exit_code != 0 + assert "ZOTERO_API_KEY" in result.output + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.urllib.request.urlopen") +@patch("docflow.integrations.zotero.zotero") +def test_zotero_add_dois_autodetects_group_from_api_key( + mock_pyz, mock_urlopen, cli_runner: CliRunner, tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """With only ZOTERO_API_KEY, /keys/current resolves the one group.""" + monkeypatch.setenv("ZOTERO_API_KEY", "k") + monkeypatch.delenv("ZOTERO_GROUP_ID", raising=False) + monkeypatch.delenv("DOCFLOW_ZOTERO_GROUP_ID", raising=False) + monkeypatch.chdir(tmp_path) + + # Mock /keys/current response: one accessible group, user library denied + keys_body = json.dumps( + { + "userID": 42, + "access": { + "user": {"library": False}, + "groups": { + "all": {"library": False}, + "5111637": {"library": True, "write": True}, + }, + }, + } + ).encode("utf-8") + resp = MagicMock() + resp.status = 200 + resp.read.return_value = keys_body + resp.__enter__.return_value = resp + resp.__exit__.return_value = False + mock_urlopen.return_value = resp + + zot_instance = MagicMock() + zot_instance.items.return_value = [] + mock_pyz.Zotero.return_value = zot_instance + + result = cli_runner.invoke( + main, + ["zotero", "add-dois", "--collection", "BZ69VS62", "--dry-run"], + input="10.1038/sdata.2016.44\n", + ) + assert result.exit_code == 0, result.output + assert "Loaded 1 DOI(s)" in result.output + # Client was constructed against the auto-detected group + mock_pyz.Zotero.assert_called_once_with("5111637", "group", "k") + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.zotero") +def test_zotero_add_dois_classifies_biorxiv( + mock_pyz, cli_runner: CliRunner, tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """DOIs under 10.1101/ are classified as BIORXIV (so .vN normalises).""" + monkeypatch.setenv("ZOTERO_API_KEY", "k") + monkeypatch.setenv("ZOTERO_GROUP_ID", "999") + monkeypatch.chdir(tmp_path) + + # An existing bioRxiv item without a version suffix + zot_instance = MagicMock() + zot_instance.items.return_value = [ + {"data": {"DOI": "10.1101/2023.01.15.524123"}}, + ] + mock_pyz.Zotero.return_value = zot_instance + + result = cli_runner.invoke( + main, + ["zotero", "add-dois", "--dry-run"], + input="10.1101/2023.01.15.524123.v2\n", # same paper, different version + ) + assert result.exit_code == 0, result.output + # Since dedup normalises the version suffix, the DOI is treated as existing + assert "Would add: 0" in result.output + assert "Would skip (existing): 1" in result.output + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.zotero") +def test_zotero_add_dois_from_file( + mock_pyz, cli_runner: CliRunner, tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("ZOTERO_API_KEY", "k") + cfg = _write_config(tmp_path) + dois_file = tmp_path / "dois.txt" + dois_file.write_text("10.1/one\n#skip\n10.2/two\n\n") + + zot_instance = MagicMock() + zot_instance.items.return_value = [] + mock_pyz.Zotero.return_value = zot_instance + + result = cli_runner.invoke( + main, + [ + "--config", str(cfg), + "zotero", "add-dois", "--dry-run", str(dois_file), + ], + ) + assert result.exit_code == 0, result.output + assert "Loaded 2 DOI(s)" in result.output + + +# --------------------------------------------------------------------------- +# `docflow zotero push --rules` +# --------------------------------------------------------------------------- + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.zotero") +def test_zotero_push_with_rules( + mock_pyz, cli_runner: CliRunner, tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """`push --rules` routes tagged citations to a different collection.""" + monkeypatch.setenv("ZOTERO_API_KEY", "k") + cfg = _write_config(tmp_path) + + # Prepare a citation JSON with a tagged entry + cit_path = tmp_path / "cit.json" + cit_path.write_text( + json.dumps( + { + "source_file": "x", + "citations": [ + {"type": "url", "value": "https://a.example"}, + {"type": "url", "value": "https://b.example", "tags": ["biosketch"]}, + ], + } + ) + ) + + # Rules YAML + rules_path = tmp_path / "rules.yaml" + rules_path.write_text( + "default_collection: MAIN\n" + "routes:\n" + " - match: {tag: biosketch}\n" + " collection: BIO\n" + ) + + zot_instance = MagicMock() + zot_instance.items.return_value = [] + zot_instance.collection_items.return_value = [] + zot_instance.item_template.side_effect = lambda *a, **kw: {"itemType": "webpage"} + zot_instance.create_items.return_value = {"successful": {"0": {"key": "K"}}, "failed": {}} + mock_pyz.Zotero.return_value = zot_instance + + result = cli_runner.invoke( + main, + [ + "--config", str(cfg), + "zotero", "push", + "--rules", str(rules_path), + str(cit_path), + ], + ) + assert result.exit_code == 0, result.output + + dests = sorted( + call.args[0][0]["collections"][0] + for call in zot_instance.create_items.call_args_list + ) + assert dests == ["BIO", "MAIN"] + zot_instance.addto_collection.assert_not_called() + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.zotero") +def test_zotero_push_per_citation_collection_field( + mock_pyz, cli_runner: CliRunner, tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A per-citation ``collection`` field beats the CLI --collection flag.""" + monkeypatch.setenv("ZOTERO_API_KEY", "k") + cfg = _write_config(tmp_path) + + cit_path = tmp_path / "cit.json" + cit_path.write_text( + json.dumps( + { + "citations": [ + {"type": "url", "value": "https://a.example", "collection": "PER"}, + ], + } + ) + ) + + zot_instance = MagicMock() + zot_instance.items.return_value = [] + zot_instance.collection_items.return_value = [] + zot_instance.item_template.return_value = {"itemType": "webpage"} + zot_instance.create_items.return_value = {"successful": {"0": {"key": "K"}}, "failed": {}} + mock_pyz.Zotero.return_value = zot_instance + + result = cli_runner.invoke( + main, + [ + "--config", str(cfg), + "zotero", "push", "--collection", "CLI-DEFAULT", + str(cit_path), + ], + ) + assert result.exit_code == 0, result.output + zot_instance.addto_collection.assert_not_called() + created = zot_instance.create_items.call_args.args[0][0] + assert created["collections"] == ["PER"] + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.urllib.request.urlopen") +@patch("docflow.integrations.zotero.zotero") +def test_zotero_add_dois_exits_nonzero_on_failure( + mock_pyz, mock_urlopen, cli_runner: CliRunner, tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """`add-dois` must exit non-zero if any item failed to push.""" + monkeypatch.setenv("ZOTERO_API_KEY", "k") + monkeypatch.setenv("ZOTERO_GROUP_ID", "999") + monkeypatch.chdir(tmp_path) + + # CrossRef lookup fails for the DOI → citation ends up in 'failed' + import urllib.error + mock_urlopen.side_effect = urllib.error.HTTPError( + "https://doi.org/x", 404, "Not Found", {}, None + ) + + zot_instance = MagicMock() + zot_instance.items.return_value = [] + mock_pyz.Zotero.return_value = zot_instance + + result = cli_runner.invoke( + main, + ["zotero", "add-dois", "--collection", "COL"], + input="10.9999/nonexistent\n", + ) + assert result.exit_code != 0, result.output + assert "Failed: 1" in result.output + assert "10.9999/nonexistent" in result.output + + +@pytest.mark.ai_generated +def test_zotero_push_exits_nonzero_on_failure( + cli_runner: CliRunner, tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """`push` must exit non-zero if any item failed to push.""" + monkeypatch.setenv("ZOTERO_API_KEY", "k") + cfg = _write_config(tmp_path) + + cit_path = tmp_path / "cit.json" + cit_path.write_text( + json.dumps({"citations": [{"type": "doi", "value": "10.9999/nope"}]}) + ) + + with ( + patch("docflow.integrations.zotero.urllib.request.urlopen") as mu, + patch("docflow.integrations.zotero.zotero") as mpyz, + ): + import urllib.error + mu.side_effect = urllib.error.HTTPError( + "u", 404, "Not Found", {}, None + ) + zot_instance = MagicMock() + zot_instance.items.return_value = [] + mpyz.Zotero.return_value = zot_instance + + result = cli_runner.invoke( + main, + ["--config", str(cfg), "zotero", "push", str(cit_path)], + ) + assert result.exit_code != 0, result.output + assert "Failed: 1" in result.output diff --git a/tests/test_init.py b/tests/test_init.py index f6c4fc3..467a98b 100644 --- a/tests/test_init.py +++ b/tests/test_init.py @@ -224,3 +224,45 @@ def test_init_requires_name() -> None: assert result.exit_code != 0 assert "Missing option '--name'" in result.output + + +@pytest.mark.ai_generated +def test_init_makefile_has_refs_bib_target(tmp_path) -> None: + """`docflow init` writes a Makefile with a working refs.bib target.""" + runner = CliRunner() + (tmp_path / ".git").mkdir() # skip datalad create + + result = runner.invoke( + main, ["init", "--name", "T", str(tmp_path)] + ) + assert result.exit_code == 0, result.output + + makefile_content = (tmp_path / "Makefile").read_text() + assert "refs.bib:" in makefile_content + assert "bib: refs.bib" in makefile_content + assert 'docflow zotero export --output "$@"' in makefile_content + # `bib` is part of the top-level `all:` recipe + all_lines = [ + line for line in makefile_content.splitlines() + if line.startswith("all:") + ] + assert all_lines, "Missing `all:` recipe in generated Makefile" + assert " bib" in all_lines[0], f"`bib` missing from `all:` recipe: {all_lines[0]!r}" + + +@pytest.mark.ai_generated +def test_init_makefile_refs_bib_included_without_zotero_group(tmp_path) -> None: + """The refs.bib target is unconditional — env-var-guarded at runtime.""" + runner = CliRunner() + (tmp_path / ".git").mkdir() + + # No --zotero-group-id passed + result = runner.invoke( + main, ["init", "--name", "T", str(tmp_path)] + ) + assert result.exit_code == 0, result.output + + makefile_content = (tmp_path / "Makefile").read_text() + assert "refs.bib:" in makefile_content + # And it's env-var-guarded, so missing key produces an actionable error + assert 'ZOTERO_API_KEY' in makefile_content diff --git a/tests/test_makefile.py b/tests/test_makefile.py index c9fad68..3e8b7cb 100644 --- a/tests/test_makefile.py +++ b/tests/test_makefile.py @@ -62,3 +62,32 @@ def test_makefile_template_has_shellcheck_safe_commands() -> None: # Should use 'if [ -z "$VAR" ]' not 'if [ -z $VAR ]' if '[ -z $$' in content and '[ -z "$$' not in content: pytest.fail("Environment variable check should quote variables: [ -z \"$$VAR\" ]") + + +@pytest.mark.ai_generated +def test_makefile_template_has_bib_target() -> None: + """The generated Makefile exposes `refs.bib` (issue #2).""" + template_path = ( + Path(__file__).parent.parent / "docflow" / "templates" / "Makefile.template" + ) + content = template_path.read_text() + + # The bib target and its refs.bib rule + assert "refs.bib:" in content, "Missing refs.bib target" + assert "bib: refs.bib" in content, "Missing bib alias for refs.bib" + assert 'docflow zotero export --output "$@"' in content, ( + "refs.bib target should call `docflow zotero export --output \"$@\"`" + ) + # Guarded by the same env-var check as zotero-check / zotero-push + assert content.count('[ -z "$$ZOTERO_API_KEY" ]') >= 3, ( + "Expected ZOTERO_API_KEY guard on zotero-check, zotero-push, and refs.bib" + ) + # help/phony declarations updated + assert ".PHONY:" in content and " bib " in content.split(".PHONY:")[1].split("\n")[0], ( + "bib should appear in .PHONY declarations" + ) + # bib is part of the top-level `all:` recipe + all_line = next( + line for line in content.splitlines() if line.startswith("all:") + ) + assert " bib" in all_line, f"`bib` missing from `all:` recipe: {all_line!r}" diff --git a/tests/test_zotero.py b/tests/test_zotero.py index 675031d..07f51cf 100644 --- a/tests/test_zotero.py +++ b/tests/test_zotero.py @@ -8,7 +8,14 @@ from docflow.config import DocflowConfig from docflow.extract.citations import Citation, CitationType -from docflow.integrations.zotero import ZoteroClient +from docflow.integrations.zotero import ( + LibraryResolutionError, + RouteMatchError, + RoutingRule, + RoutingRules, + ZoteroClient, + resolve_library_from_key, +) @pytest.fixture @@ -255,49 +262,51 @@ def test_push_items_skips_existing(mock_zotero) -> None: mock_zotero.create_items.assert_not_called() -def test_export_bibtex(mock_zotero) -> None: - """Test BibTeX export.""" - mock_items = [ - {"data": {"title": "Paper 1"}}, - {"data": {"title": "Paper 2"}}, - ] - bibtex_content = """@article{doe2024, - title = {Test Paper}, - author = {Doe, John}, - year = {2024} -}""" - - mock_zotero.collection_items.side_effect = [ - mock_items, # First call returns items list - bibtex_content, # Second call with format="bibtex" returns BibTeX - ] +def _mock_export_response(body: str, total: int) -> MagicMock: + """Build a urllib.urlopen context manager that returns ``body`` bytes + and reports ``Total-Results`` in its headers.""" + resp = MagicMock() + resp.read.return_value = body.encode("utf-8") + resp.headers = {"Total-Results": str(total)} + resp.__enter__.return_value = resp + resp.__exit__.return_value = False + return resp + + +@patch("docflow.integrations.zotero.urllib.request.urlopen") +def test_export_bibtex(mock_urlopen, mock_zotero) -> None: + """Group-collection BibTeX export uses the correct endpoint.""" + bibtex_content = "@article{doe2024,\n title = {Test Paper},\n author = {Doe, John},\n year = {2024}\n}" + mock_urlopen.return_value = _mock_export_response(bibtex_content, total=1) client = ZoteroClient(group_id="123", api_key="key") result = client.export_bibtex(collection_key="ABCD1234") assert "@article" in result assert "Doe, John" in result + # Assert URL structure: /groups/123/collections/ABCD1234/items?format=bibtex... + call_url = mock_urlopen.call_args[0][0].full_url + assert "/groups/123/collections/ABCD1234/items" in call_url + assert "format=bibtex" in call_url -def test_export_bibtex_to_file(mock_zotero, tmp_path: Path) -> None: - """Test BibTeX export to file.""" - mock_zotero.collection_items.side_effect = [ - [{"data": {"title": "Paper"}}], - "@article{test}", - ] - +@patch("docflow.integrations.zotero.urllib.request.urlopen") +def test_export_bibtex_to_file(mock_urlopen, mock_zotero, tmp_path: Path) -> None: + """BibTeX export writes to file when output_path is given.""" + mock_urlopen.return_value = _mock_export_response("@article{test}", total=1) output_file = tmp_path / "references.bib" client = ZoteroClient(group_id="123", api_key="key") _ = client.export_bibtex(collection_key="ABCD1234", output_path=output_file) assert output_file.exists() - assert output_file.read_text() == "@article{test}" + assert "@article{test}" in output_file.read_text() -def test_export_bibtex_empty_collection(mock_zotero) -> None: - """Test BibTeX export with empty collection.""" - mock_zotero.collection_items.return_value = [] +@patch("docflow.integrations.zotero.urllib.request.urlopen") +def test_export_bibtex_empty_collection(mock_urlopen, mock_zotero) -> None: + """Empty collection returns an empty string.""" + mock_urlopen.return_value = _mock_export_response("", total=0) client = ZoteroClient(group_id="123", api_key="key") result = client.export_bibtex(collection_key="ABCD1234") @@ -472,3 +481,500 @@ def test_create_item_from_citation_url(mock_zotero) -> None: assert item is not None assert item["url"] == "https://example.com/paper" + + +# --------------------------------------------------------------------------- +# Group-wide export, pagination, format variants +# --------------------------------------------------------------------------- + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.urllib.request.urlopen") +def test_export_bibtex_group_wide(mock_urlopen, mock_zotero) -> None: + """collection_key=None hits the group-items endpoint, not a collection.""" + mock_urlopen.return_value = _mock_export_response("@article{one}", total=1) + + client = ZoteroClient(group_id="123", api_key="key") + result = client.export_bibtex(collection_key=None) + + assert "@article" in result + url = mock_urlopen.call_args[0][0].full_url + assert "/groups/123/items" in url + assert "/collections/" not in url + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.urllib.request.urlopen") +def test_export_bibtex_user_mode(mock_urlopen, mock_zotero) -> None: + """Personal library uses /users//items.""" + mock_urlopen.return_value = _mock_export_response("@article{p}", total=1) + + client = ZoteroClient(group_id="99", api_key="k", mode="personal") + _ = client.export_bibtex(collection_key=None) + + url = mock_urlopen.call_args[0][0].full_url + assert "/users/99/items" in url + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.urllib.request.urlopen") +def test_export_bibtex_paginates(mock_urlopen, mock_zotero) -> None: + """Reads all pages when Total-Results > page size.""" + page1 = "\n\n".join(f"@article{{p{i}}}" for i in range(100)) + page2 = "\n\n".join(f"@article{{p{i}}}" for i in range(100, 150)) + # Both responses report the same Total-Results + mock_urlopen.side_effect = [ + _mock_export_response(page1, total=150), + _mock_export_response(page2, total=150), + ] + + client = ZoteroClient(group_id="123", api_key="key") + result = client.export_bibtex(collection_key=None) + + # Both pages captured + assert "@article{p0}" in result + assert "@article{p99}" in result + assert "@article{p100}" in result + assert "@article{p149}" in result + # Two requests were made, with different `start=` offsets + urls = [call.args[0].full_url for call in mock_urlopen.call_args_list] + assert len(urls) == 2 + assert "start=0" in urls[0] + assert "start=100" in urls[1] + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.urllib.request.urlopen") +def test_export_bibtex_stops_when_total_reached(mock_urlopen, mock_zotero) -> None: + """No extra request when the first page contains everything.""" + mock_urlopen.return_value = _mock_export_response("@article{a}\n@article{b}", total=2) + + client = ZoteroClient(group_id="123", api_key="key") + _ = client.export_bibtex(collection_key=None) + + assert mock_urlopen.call_count == 1 + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.urllib.request.urlopen") +def test_export_csljson_returns_merged_json_array(mock_urlopen, mock_zotero) -> None: + """CSL-JSON is merged into a single JSON array across paginated requests.""" + # total > page size so we actually paginate + page1 = json.dumps([{"id": f"a{i}"} for i in range(100)]) + page2 = json.dumps([{"id": "b0"}, {"id": "b1"}]) + mock_urlopen.side_effect = [ + _mock_export_response(page1, total=102), + _mock_export_response(page2, total=102), + ] + + client = ZoteroClient(group_id="123", api_key="key") + result = client.export_bibtex(collection_key=None, format="csljson") + + parsed = json.loads(result) + assert len(parsed) == 102 + assert parsed[0] == {"id": "a0"} + assert parsed[-1] == {"id": "b1"} + # Format param was forwarded on the URL + url = mock_urlopen.call_args_list[0].args[0].full_url + assert "format=csljson" in url + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.urllib.request.urlopen") +def test_export_ris_passes_format_through(mock_urlopen, mock_zotero) -> None: + """RIS format is passed through in the URL.""" + ris = "TY - JOUR\nTI - Test\nER - " + mock_urlopen.return_value = _mock_export_response(ris, total=1) + + client = ZoteroClient(group_id="123", api_key="key") + result = client.export_bibtex(collection_key=None, format="ris") + + assert "TY - JOUR" in result + url = mock_urlopen.call_args_list[0].args[0].full_url + assert "format=ris" in url + + +@pytest.mark.ai_generated +def test_export_bibtex_rejects_unknown_format(mock_zotero) -> None: + """Bad format raises ValueError before any HTTP call.""" + client = ZoteroClient(group_id="123", api_key="key") + with pytest.raises(ValueError, match="Unsupported export format"): + client.export_bibtex(format="endnote") + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.urllib.request.urlopen") +def test_export_bibtex_http_error_wrapped(mock_urlopen, mock_zotero) -> None: + """HTTP errors surface as ValueError.""" + import urllib.error + + mock_urlopen.side_effect = urllib.error.HTTPError( + "https://api.zotero.org/x", 500, "boom", {}, None + ) + + client = ZoteroClient(group_id="123", api_key="key") + with pytest.raises(ValueError, match="Failed to export from Zotero"): + client.export_bibtex(collection_key=None) + + +# --------------------------------------------------------------------------- +# Per-collection routing +# --------------------------------------------------------------------------- + + +@pytest.mark.ai_generated +def test_resolve_collection_per_citation_wins(mock_zotero) -> None: + """Explicit citation.collection beats every other source.""" + c = Citation( + type=CitationType.DOI, + value="10.1/x", + collection="PER-ITEM", + tags=["biosketch"], + ) + rules = RoutingRules( + default_collection="DEFAULT", + routes=[RoutingRule({"tag": "biosketch"}, "TAGGED")], + ) + assert ZoteroClient._resolve_collection(c, "CLI", rules) == "PER-ITEM" + + +@pytest.mark.ai_generated +def test_resolve_collection_rule_beats_default(mock_zotero) -> None: + c = Citation(type=CitationType.DOI, value="10.1/x", tags=["biosketch"]) + rules = RoutingRules( + default_collection="DEFAULT", + routes=[RoutingRule({"tag": "biosketch"}, "TAGGED")], + ) + assert ZoteroClient._resolve_collection(c, "CLI", rules) == "TAGGED" + + +@pytest.mark.ai_generated +def test_resolve_collection_rules_default_beats_cli(mock_zotero) -> None: + c = Citation(type=CitationType.DOI, value="10.1/nomatch") + rules = RoutingRules(default_collection="RULES-DEFAULT", routes=[]) + assert ZoteroClient._resolve_collection(c, "CLI", rules) == "RULES-DEFAULT" + + +@pytest.mark.ai_generated +def test_resolve_collection_cli_flag_fallback(mock_zotero) -> None: + c = Citation(type=CitationType.DOI, value="10.1/x") + assert ZoteroClient._resolve_collection(c, "CLI", None) == "CLI" + + +@pytest.mark.ai_generated +def test_resolve_collection_none_when_no_source(mock_zotero) -> None: + c = Citation(type=CitationType.DOI, value="10.1/x") + assert ZoteroClient._resolve_collection(c, None, None) is None + + +@pytest.mark.ai_generated +def test_routing_rule_doi_prefix(mock_zotero) -> None: + rule = RoutingRule({"doi_prefix": "10.1016/j.cell"}, "CELL") + assert rule.matches(Citation(type=CitationType.DOI, value="10.1016/j.cell.2023.1")) + assert not rule.matches(Citation(type=CitationType.DOI, value="10.1038/nature")) + # Non-DOI citation never matches doi_prefix + assert not rule.matches(Citation(type=CitationType.URL, value="10.1016/j.cell.x")) + + +@pytest.mark.ai_generated +def test_routing_rule_tag(mock_zotero) -> None: + rule = RoutingRule({"tag": "biosketch"}, "BIO") + assert rule.matches( + Citation(type=CitationType.DOI, value="10.1/x", tags=["biosketch"]) + ) + assert not rule.matches(Citation(type=CitationType.DOI, value="10.1/x")) + + +@pytest.mark.ai_generated +def test_routing_rule_empty_match_never_matches(mock_zotero) -> None: + rule = RoutingRule({}, "X") + assert not rule.matches(Citation(type=CitationType.DOI, value="10.1/x")) + + +@pytest.mark.ai_generated +def test_routing_rules_from_file(tmp_path: Path, mock_zotero) -> None: + rules_path = tmp_path / "rules.yaml" + rules_path.write_text( + "default_collection: DEFAULT\n" + "routes:\n" + " - match: {tag: biosketch}\n" + " collection: BIO\n" + " - match: {doi_prefix: '10.1016/j.cell'}\n" + " collection: CELL\n" + ) + rules = RoutingRules.from_file(rules_path) + assert rules.default_collection == "DEFAULT" + assert len(rules.routes) == 2 + assert rules.routes[0].collection == "BIO" + assert rules.routes[1].match["doi_prefix"] == "10.1016/j.cell" + + +@pytest.mark.ai_generated +def test_routing_rules_from_file_invalid(tmp_path: Path, mock_zotero) -> None: + rules_path = tmp_path / "rules.yaml" + rules_path.write_text("routes:\n - collection: NOMATCH\n") # missing match + with pytest.raises(RouteMatchError): + RoutingRules.from_file(rules_path) + + +@pytest.mark.ai_generated +def test_push_items_routes_by_per_citation_collection(mock_zotero) -> None: + """A citation carrying its own collection routes there, not to --collection.""" + mock_zotero.items.return_value = [] + mock_zotero.collection_items.return_value = [] + mock_zotero.item_template.return_value = {"itemType": "webpage"} + mock_zotero.create_items.return_value = {"successful": {"0": {"key": "K"}}, "failed": {}} + + client = ZoteroClient(group_id="123", api_key="key") + citations = [ + Citation(type=CitationType.URL, value="https://a.example", collection="PER-ITEM"), + ] + + result = client.push_items(citations, collection_key="DEFAULT") + + assert len(result["added"]) == 1 + # Collection assigned at create time, not via addto_collection + mock_zotero.addto_collection.assert_not_called() + created = mock_zotero.create_items.call_args.args[0][0] + assert created["collections"] == ["PER-ITEM"] + + +@pytest.mark.ai_generated +def test_push_items_routes_by_rules(mock_zotero) -> None: + """Rules assign different collections to different citations.""" + mock_zotero.items.return_value = [] + mock_zotero.collection_items.return_value = [] + # Fresh dict per call — real pyzotero deep-copies templates. + mock_zotero.item_template.side_effect = lambda *a, **kw: {"itemType": "webpage"} + mock_zotero.create_items.return_value = {"successful": {"0": {"key": "K"}}, "failed": {}} + + rules = RoutingRules( + default_collection="MAIN", + routes=[ + RoutingRule({"tag": "biosketch"}, "BIO"), + ], + ) + client = ZoteroClient(group_id="123", api_key="key") + citations = [ + Citation(type=CitationType.URL, value="https://a.example"), + Citation(type=CitationType.URL, value="https://b.example", tags=["biosketch"]), + ] + + _ = client.push_items(citations, rules=rules) + + # Every citation was created with its resolved collection in the payload + dests = sorted( + call.args[0][0]["collections"][0] + for call in mock_zotero.create_items.call_args_list + ) + assert dests == ["BIO", "MAIN"] + mock_zotero.addto_collection.assert_not_called() + + +@pytest.mark.ai_generated +def test_push_items_exit_status_captures_failures(mock_zotero) -> None: + """A failed item ends up in result['failed'] and does not raise.""" + mock_zotero.items.return_value = [] + mock_zotero.item_template.return_value = {"itemType": "webpage"} + # Zotero refused to create the item + mock_zotero.create_items.return_value = { + "successful": {}, + "failed": {"0": {"code": 400, "message": "bad"}}, + } + + client = ZoteroClient(group_id="123", api_key="key") + citations = [Citation(type=CitationType.URL, value="https://a.example")] + + result = client.push_items(citations) + + assert result["added"] == [] + assert result["failed"] == citations + + +# --------------------------------------------------------------------------- +# Citation dataclass tag/collection fields +# --------------------------------------------------------------------------- + + +@pytest.mark.ai_generated +def test_citation_to_dict_includes_routing_fields() -> None: + c = Citation( + type=CitationType.DOI, + value="10.1/x", + collection="ABCD", + tags=["biosketch", "review"], + ) + d = c.to_dict() + assert d["collection"] == "ABCD" + assert d["tags"] == ["biosketch", "review"] + + +@pytest.mark.ai_generated +def test_citation_to_dict_omits_empty_routing_fields() -> None: + c = Citation(type=CitationType.DOI, value="10.1/x") + d = c.to_dict() + assert "collection" not in d + assert "tags" not in d + + +# --------------------------------------------------------------------------- +# resolve_library_from_key +# --------------------------------------------------------------------------- + + +def _mock_keys_response(payload: dict) -> MagicMock: + resp = MagicMock() + resp.status = 200 + resp.read.return_value = json.dumps(payload).encode("utf-8") + resp.__enter__.return_value = resp + resp.__exit__.return_value = False + return resp + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.urllib.request.urlopen") +def test_resolve_library_single_group(mock_urlopen) -> None: + """Exactly one accessible group → picked automatically.""" + mock_urlopen.return_value = _mock_keys_response( + { + "userID": 42, + "access": { + "user": {"library": False}, + "groups": {"5111637": {"library": True}}, + }, + } + ) + lib_id, mode = resolve_library_from_key("k") + assert lib_id == "5111637" + assert mode == "group" + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.urllib.request.urlopen") +def test_resolve_library_only_user(mock_urlopen) -> None: + """No groups, only user library accessible.""" + mock_urlopen.return_value = _mock_keys_response( + { + "userID": 42, + "access": { + "user": {"library": True}, + "groups": {}, + }, + } + ) + lib_id, mode = resolve_library_from_key("k") + assert lib_id == "42" + assert mode == "personal" + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.urllib.request.urlopen") +def test_resolve_library_ignores_all_pseudo_group(mock_urlopen) -> None: + """The Zotero 'all' pseudo-group is ignored.""" + mock_urlopen.return_value = _mock_keys_response( + { + "userID": 42, + "access": { + "user": {"library": False}, + "groups": { + "all": {"library": True}, # pseudo + "5111637": {"library": True}, + }, + }, + } + ) + lib_id, mode = resolve_library_from_key("k") + assert lib_id == "5111637" + assert mode == "group" + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.urllib.request.urlopen") +def test_resolve_library_ambiguous_no_hint(mock_urlopen) -> None: + """Multiple libraries without a hint → LibraryResolutionError with listing.""" + mock_urlopen.return_value = _mock_keys_response( + { + "userID": 42, + "access": { + "user": {"library": True}, + "groups": { + "5111637": {"library": True}, + "1000000": {"library": True}, + }, + }, + } + ) + with pytest.raises(LibraryResolutionError, match="Multiple accessible"): + resolve_library_from_key("k") + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.urllib.request.urlopen") +def test_resolve_library_ambiguous_with_hint(mock_urlopen) -> None: + """Collection hint probes each group and picks the matching one.""" + import urllib.error + + keys_resp = _mock_keys_response( + { + "userID": 42, + "access": { + "user": {"library": False}, + "groups": { + "AAA": {"library": True}, + "BBB": {"library": True}, + }, + }, + } + ) + # First group probe → 404; second → 200. + probe_hit = MagicMock() + probe_hit.status = 200 + probe_hit.read.return_value = b"{}" + probe_hit.__enter__.return_value = probe_hit + probe_hit.__exit__.return_value = False + + mock_urlopen.side_effect = [ + keys_resp, + urllib.error.HTTPError("url", 404, "Not Found", {}, None), + probe_hit, + ] + + lib_id, mode = resolve_library_from_key("k", collection_hint="XYZ") + assert (lib_id, mode) == ("BBB", "group") + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.urllib.request.urlopen") +def test_resolve_library_hint_not_found(mock_urlopen) -> None: + """No group contains the hinted collection → informative error.""" + import urllib.error + + mock_urlopen.side_effect = [ + _mock_keys_response( + { + "userID": 42, + "access": { + "user": {"library": False}, + "groups": { + "AAA": {"library": True}, + "BBB": {"library": True}, + }, + }, + } + ), + urllib.error.HTTPError("url", 404, "Not Found", {}, None), + urllib.error.HTTPError("url", 404, "Not Found", {}, None), + ] + + with pytest.raises(LibraryResolutionError, match="not found in any"): + resolve_library_from_key("k", collection_hint="ZZZ") + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.urllib.request.urlopen") +def test_resolve_library_no_access(mock_urlopen) -> None: + mock_urlopen.return_value = _mock_keys_response( + {"userID": 42, "access": {"user": {"library": False}, "groups": {}}} + ) + with pytest.raises(LibraryResolutionError, match="no library access"): + resolve_library_from_key("k") From 8bb23124116f1ccf431e42a23a374f24ca3de2d7 Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Thu, 23 Jul 2026 23:17:38 -0400 Subject: [PATCH 2/3] =?UTF-8?q?design:=20zotero=20per-collection=20.bib=20?= =?UTF-8?q?export=20(subcollections=20=E2=86=92=20files)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follows con/docflow#2. Adds a design proposal (no code changes) for splitting a Zotero library into one .bib per top-level collection. Motivation: grant proposals frequently cite from multiple curated subcollections (e.g. BIDS group has "BIDS and BEPs", "Related", "biosketch-refs" — https://www.zotero.org/groups/5111637/bids/library). Users want a matching per-section .bib without hand-mapping collection names to keys and re-invoking `export --collection` for each. Proposed CLI additions: - `docflow zotero export-collections [--parent] [--output-dir] [--format] [--recursive/--no-recursive] [--naming slug|preserve|key] [--include/--exclude GLOB] [--skip-empty] [--manifest] [--dry-run]` - `docflow zotero list-collections [--parent] [--depth] [--format]` Design highlights (see .specify/specs/zotero-export-per-collection.md): - Orthogonal `--depth` (how many .bib files) vs `--recursive` (how much each contains) — Zotero collections don't own items, so both dials matter. - Filename schemes: slug (default), preserve, key; collisions resolved deterministically by appending -. - Manifest (bibs/index.yaml) as single source of truth for downstream tooling — records library_version stamps for a future --only-changed. - Descendant fetches use one API round-trip per output unit via Zotero's comma-separated collectionKey filter (URL-length fallback: per-collection with client-side dedup). - Makefile: adds `bibs` and `bibs-list` phony targets, env-var-guarded like refs.bib. NOT in `all:` by default (round-trip scaling). Open questions flagged inline for review: item-in-multiple-collections dedup, list-collections vs. --dry-run overlap, --only-changed staleness. Co-Authored-By: Claude Code 2.1.210 / Claude Opus 4.7 (1M context) --- .../specs/zotero-export-per-collection.md | 403 ++++++++++++++++++ 1 file changed, 403 insertions(+) create mode 100644 .specify/specs/zotero-export-per-collection.md diff --git a/.specify/specs/zotero-export-per-collection.md b/.specify/specs/zotero-export-per-collection.md new file mode 100644 index 0000000..082289b --- /dev/null +++ b/.specify/specs/zotero-export-per-collection.md @@ -0,0 +1,403 @@ +# Zotero: export subcollections as individual `.bib` files + +**Follows**: [`zotero-export-and-routing.md`](./zotero-export-and-routing.md) +**Status**: Design (proposed, not yet implemented) + +## Motivation + +The current `docflow zotero export` produces one bibliography file for +either the whole library or a single collection. For grant proposals +that draw citations from multiple curated collections (e.g. the +[BIDS group library](https://www.zotero.org/groups/5111637/bids/library) +with top-level collections *"BIDS and BEPs"*, *"Related"*, and other +`.bib`-worthy subtrees), users want a matching `.bib` per collection so +they can: + +- Cite from different collections in different papers / sections of a + proposal (e.g. `related.bib` for the "background" section, + `bids-and-beps.bib` for methods). +- Regenerate all `.bib` files with `make bibs`. +- Version-control the split `.bib` files under `refs/` and diff them + meaningfully (a single monolithic `.bib` produces noisy diffs and + hides which section changed). + +Doing this by hand today requires (a) looking up each collection's key +in the Zotero web UI, (b) running `docflow zotero export +--collection KEY --output name.bib` per collection, and (c) keeping the +name↔key mapping outside the tool. All three steps invite drift. + +## Design + +### CLI + +New sibling subcommand to `export`: + +``` +docflow zotero export-collections [--parent KEY | --parent-name NAME] + [--output-dir DIR] (default: bibs/) + [--depth N] (default: 1) + [--recursive/--no-recursive] (default: recursive) + [--format bibtex|csljson|ris] (default: bibtex) + [--naming slug|preserve|key] (default: slug) + [--include GLOB]... [--exclude GLOB]... + [--skip-empty/--keep-empty] (default: --skip-empty) + [--manifest PATH] (default: bibs/index.yaml) + [--dry-run] +``` + +`export` (single-collection) stays untouched. This is a distinct +operation ("write N files") with different result semantics ("no +stdout, only file writes"), so folding into `export` would muddy the +existing invariant. + +### Selecting the source collections + +Three orthogonal knobs: + +1. **Parent**. `--parent KEY` or `--parent-name NAME` restricts to + collections under that node. If both are absent, the parent is the + library root (all top-level collections). `--parent-name` matches + case-insensitively against unique names; ambiguous names error out + with the list of matches. +2. **Depth**. `--depth N` controls how far below the parent to walk + when enumerating *output units*. `--depth 1` (default) means "one + `.bib` per top-level collection under the parent, whatever its + subtree contains". `--depth 2` means "one `.bib` per top-level + collection *and* per direct child". `--depth 0` means "just the + parent as a single `.bib`" (degenerate; use `export` instead). +3. **Recursion inside each output unit**. + `--recursive` (default) means each `.bib` includes items from that + collection *and all its descendants*. `--no-recursive` means only + items directly in that collection. + +The distinction between `--depth` (how many `.bib` files) and +`--recursive` (how much each `.bib` contains) matters because Zotero +collections don't have owning semantics — an item is "in" every +collection it was added to, and parent/child is purely visual. Users +almost always want `--recursive` (the grant example is exactly this), +but strict mode is worth having for library curators. + +Concrete for the BIDS group example (`--depth 1 --recursive`, default): + +``` +bibs/ +├── bids-and-beps.bib # items in "BIDS and BEPs" + all descendants +├── related.bib # items in "Related" + all descendants +├── biosketch-refs.bib # items in "biosketch-refs" + all descendants +└── index.yaml # manifest (see below) +``` + +### Filenames + +Zotero collection names are freeform. Three naming schemes: + +| Scheme | "BIDS and BEPs" | "Related / Adjacent" | "biosketch-refs" | +|------------|-----------------|----------------------|------------------| +| `slug` (default) | `bids-and-beps.bib` | `related-adjacent.bib` | `biosketch-refs.bib` | +| `preserve` | `BIDS and BEPs.bib` | `Related _ Adjacent.bib` | `biosketch-refs.bib` | +| `key` | `BZ69VS62.bib` | `AF7Q2PW1.bib` | `LM99KT42.bib` | + +Slug rules: lowercase, strip diacritics, replace runs of +non-alphanumeric characters with a single hyphen, trim leading/trailing +hyphens. `preserve` keeps case but replaces path-unsafe characters +(`/`, `\`, control chars) with `_`. + +**Collision resolution.** If two collections slug/preserve to the same +filename, append `-` to the second file (deterministic by +collection key sort order). A warning goes to stderr listing the +collision so users know to rename in Zotero. + +### Manifest + +Every `export-collections` run writes `/index.yaml` (path +overridable via `--manifest`): + +```yaml +# Regenerate: docflow zotero export-collections +generated_at: 2026-07-22T14:12:00Z +library: + id: "5111637" + mode: group + name: "BIDS" +parent: null # or {key, name} +depth: 1 +recursive: true +format: bibtex +collections: + - key: BZ69VS62 + name: "BIDS and BEPs" + path: ["BIDS and BEPs"] + file: bids-and-beps.bib + item_count: 18 + library_version: 4213 # from Zotero's Last-Modified-Version header + - key: AF7Q2PW1 + name: "Related" + path: ["Related"] + file: related.bib + item_count: 8 + library_version: 4213 +``` + +The manifest is the single source of truth for downstream tooling +(Makefiles, `pandoc` includes, ML pipelines). It's YAML so users can +hand-diff it in review. `library_version` lets a future +`--only-changed` optimisation skip regeneration for unchanged +collections. + +### Filtering + +`--include GLOB` and `--exclude GLOB` operate on the display name +(pre-slug). Multiple `--include`/`--exclude` are OR'd within each set; +excludes win over includes. Globs match against the *rightmost* path +component by default; prefix with `**/` to match anywhere in the path. + +``` +docflow zotero export-collections --include 'BIDS*' --exclude '*archive*' +``` + +`--skip-empty` (default) omits collections whose recursive item count +is zero. Manifest still lists them under a `skipped:` array so users +know they weren't forgotten. + +### Python API + +`ZoteroClient` gains: + +```python +def list_collections( + self, + parent_key: str | None = None, + max_depth: int | None = None, +) -> list[CollectionNode]: + """Return the collection tree rooted at parent_key (None = root).""" + +def export_collection_tree( + self, + parent_key: str | None = None, + output_dir: Path, + *, + depth: int = 1, + recursive: bool = True, + format: str = "bibtex", + naming: str = "slug", + include: list[str] | None = None, + exclude: list[str] | None = None, + skip_empty: bool = True, + manifest_path: Path | None = None, +) -> ExportManifest: + """Write per-collection bibliographies; return the manifest.""" +``` + +`CollectionNode` is a small dataclass: `key`, `name`, `parent_key`, +`children: list[CollectionNode]`, `num_items` (from Zotero's +`meta.numItems`). + +`ExportManifest` mirrors the on-disk YAML structure; it is what +`export_collection_tree()` returns even when `manifest_path=None`. + +The collection tree is built once via `zot.all_collections(parent_key)` +(pyzotero handles hierarchical fetching) and cached on the manifest. +Per-collection item fetches use the same `_paginated_export` path +already used by `export_bibtex`, extended to accept an optional +`extra_params: dict[str, str]` — when recursive, we OR-union descendant +keys via Zotero's `collectionKey` filter (comma-separated per +[API docs](https://www.zotero.org/support/dev/web_api/v3/basics/search_syntax)): + +``` +/groups/{id}/items?collectionKey=BZ69VS62,AF7Q2PW1,LM99KT42,... +``` + +That is one API round-trip per output unit even for a deep subtree. +If a subtree has enough descendants that the URL would exceed the +practical query-string limit (~2 KB), the implementation falls back to +per-collection fetches with client-side de-dup by item key. + +### Config + +Optional `.docflow/config.yaml` block; every CLI flag has a config +counterpart: + +```yaml +zotero: + bibs: + output_dir: bibs/ + parent_collection: null # null = library root + depth: 1 + recursive: true + format: bibtex + naming: slug + include: [] + exclude: [] + skip_empty: true + manifest: bibs/index.yaml +``` + +CLI flags override config; unset flags fall back to config; unset +config falls back to the defaults above. + +### Makefile integration + +Two new targets in the template, both env-var-guarded like +`refs.bib`: + +```makefile +.PHONY: bibs bibs-list + +# One .bib per top-level collection (default: bibs/*.bib) +bibs: + @if [ -z "$$ZOTERO_API_KEY" ]; then \ + echo "Error: ZOTERO_API_KEY not set"; exit 1; \ + fi + docflow zotero export-collections + +# List available collections without fetching items +bibs-list: + @if [ -z "$$ZOTERO_API_KEY" ]; then \ + echo "Error: ZOTERO_API_KEY not set"; exit 1; \ + fi + docflow zotero list-collections +``` + +Add `bibs` to the `all:` recipe (opt-in via config? Or always run +alongside `bib`?). Recommendation: **not** in `all:` by default — the +API round-trips scale with collection count. Users who want it wire +it into their local `all:` or run `make bibs` explicitly. + +### `docflow zotero list-collections` + +Discovery helper (also useful for scripting): + +``` +docflow zotero list-collections [--parent KEY | --parent-name NAME] + [--depth N] + [--format text|json|yaml] + [--items] # show item counts +``` + +Default output is an indented tree with item counts: + +``` +BIDS and BEPs [BZ69VS62] (18) +├── BEPs [BEP12345] (12) +└── Reserved BEPs [BEP67890] (3) +Related [AF7Q2PW1] (8) +biosketch-refs [LM99KT42] (5) +``` + +`--format json` / `--format yaml` are stable-schema outputs for +Makefiles and CI (the JSON form is the same shape as the manifest +minus the run-metadata fields). + +## Edge cases + +- **Nested collision after slugging.** Two collections at different + depths that both slug to `foo.bib`. Handled by the collision rule + above (append `-` to the loser sorted by key). +- **Very large collections.** Reuse `_paginated_export`'s + Total-Results loop. Progress goes to stderr per collection so it + doesn't pollute captured stdout. +- **Deleted-in-Zotero-since-last-run collections.** The manifest is + the previous state; on the next run, files corresponding to + no-longer-present collection keys go into a `stale:` array in the + new manifest and are optionally removed with `--prune`. Default is + to **leave stale files in place** — safer for anyone who committed + the `.bib` file. +- **Empty parent.** If the resolved parent has no descendants, exit + 0 with a warning ("no collections to export") — not an error. +- **Group vs. personal library.** Same code path; the mode+id come + from the client init (config or env var). +- **API rate limits.** Zotero's Read API rate-limits at 240 + req/minute per key; a large tree with many collections could + approach that. Add optional `--sleep-between N` (seconds) for + users on shared keys; default 0. + +## Test plan + +Unit tests (mocked pyzotero + mocked `urllib.request.urlopen` for the +export path): + +- `list_collections()` returns the correct tree given a mocked + `zot.all_collections()` response with two levels of nesting. +- `list_collections(parent_key="X")` filters correctly. +- `export_collection_tree(depth=1, recursive=True)` writes one file + per top-level collection, each containing the union of that + collection's + descendants' items (verify via mocked API call + URLs). +- `export_collection_tree(depth=1, recursive=False)` writes one file + per top-level collection with only items directly in that + collection. +- Slug naming: unicode and diacritic handling + (`"Réseau BIDS"` → `reseau-bids.bib`). +- Preserve naming: `"a/b"` → `"a_b.bib"`. +- Collision: two collections slug-collide → losing file gains `-KEY` + suffix; warning is emitted. +- Filters: `--include 'BIDS*'` picks only matching names; excludes + win. Path-prefix glob `**/archive*` matches at any depth. +- Empty-collection skipping vs. keeping. +- Manifest is written, has correct schema, and matches the files + written. +- `--dry-run` prints the plan without writing files or hitting the + API for items (only lists collections). + +CLI tests: + +- `docflow zotero export-collections --help` shows all flags. +- `docflow zotero list-collections --format json` output validates + against the documented schema. +- `docflow zotero export-collections --parent-name "BIDS and BEPs"` + resolves the parent by name. +- `docflow zotero export-collections --parent-name AMBIGUOUS` errors + when the name matches multiple collections. +- End-to-end: mocked group with two top-level collections and one + nested subcollection; verify file layout and manifest. + +Makefile: + +- `docflow init` emits `bibs` and `bibs-list` targets (`bibs` **not** + in `all:` by default). + +## Non-goals (this design) + +- **Committing the `.bib` files to Zotero.** This is a + one-way *pull* only. +- **Automatic Zotero → BibTeX round-trip.** Better BibTeX addon + remains the authoritative round-trip tool. +- **Per-item filtering inside a collection.** Filter by collection + membership, not by item metadata (tags, item type). Follow-up + issue if needed. +- **Watchdog / live re-export.** Users invoke `make bibs` + explicitly; there is no daemon. +- **Directory nesting matching Zotero hierarchy.** All output `.bib` + files land in a flat `output_dir`. A future flag could turn on + `output_dir//.bib` mirroring, but it + complicates the manifest and glob patterns. Ship flat first. + +## Rollout + +- Additive: no changes to `export_bibtex`, `push_items`, or + `add-dois`. Existing spec (`zotero-export-and-routing.md`) is + unaffected. +- Config schema gains an optional `zotero.bibs` block; unset means + "defaults". No breaking changes to existing configs. +- Makefile template additions are guarded by the same + `ZOTERO_API_KEY` env-var check pattern; they inherit the config-less + operation semantics documented in the parent spec. + +## Open questions + +1. **Item-in-multiple-collections dedup.** If item X is in both + "BIDS and BEPs" and "Related", it appears in *both* `.bib` files. + Is that what users want? In the grant workflow: yes — each + `.bib` is authoritative for its section and duplication is + harmless because `pandoc-citeproc` dedups by citation key. Ship + as-is; revisit if a user hits real trouble. +2. **`list-collections` vs. `export-collections --dry-run`.** There's + overlap: `--dry-run` on export already lists what would be written. + Keep `list-collections` for the "I just want to see the tree, no + export flags" case, and make `--dry-run` explicitly say "planned + export" (not just "the tree"). +3. **`--only-changed` optimisation.** Attractive (Zotero has + per-library and per-collection version stamps), but adds + state-file complexity and easy footguns (stale stamp + rebased + Zotero library). Not v1; revisit once the base flow is used in + anger. From 6eb89d65c8e511f43a5e4116720304c23b7a20da Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Thu, 23 Jul 2026 23:25:09 -0400 Subject: [PATCH 3/3] feat: zotero export-collections and list-collections subcommands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements the v1 subset of .specify/specs/zotero-export-per-collection.md. Enables the "one .bib per Zotero (sub)collection" workflow: users can regenerate section-scoped bibliographies with a single `make bibs`. New: - ZoteroClient.list_collections(parent_key=None): assembles pyzotero's flat all_collections() response into a CollectionNode tree. - ZoteroClient.export_collection_tree(output_dir, parent_key, format, naming, recursive, skip_empty, manifest_path): writes one file per top-level collection under parent_key (or root), plus a YAML manifest mapping collection keys → names → filenames → item counts. When recursive, OR-unions descendant keys via Zotero's collectionKey filter (comma-separated); URL-length fallback to per-collection fetches with client-side dedup for very wide subtrees. - slugify_name(name, mode, key): 'slug' (Unicode NFKD, lowercase, non-alnum → '-'), 'preserve' (case kept, path-unsafe chars → '_'), 'key' (raw collection key). Deterministic collision resolution via `-` suffix. - CLI: * `docflow zotero list-collections [--parent] [--format text|json|yaml]` * `docflow zotero export-collections [--parent] [--output-dir] [--format bibtex|csljson|ris] [--naming slug|preserve|key] [--recursive/--no-recursive] [--skip-empty/--keep-empty] [--manifest]` - Makefile template: `bibs` and `bibs-list` targets, env-var-guarded like refs.bib. Deliberately NOT in `all:` (round-trip scaling with collection count). Spec updated with a v1 scope table listing what shipped vs. what's deferred (--include/--exclude, --parent-name, --depth>1, --dry-run, --only-changed, --prune, --sleep-between, zotero.bibs config block). 21 new tests, 226 total pass. ruff clean; mypy clean. Co-Authored-By: Claude Code 2.1.210 / Claude Opus 4.7 (1M context) --- .../specs/zotero-export-per-collection.md | 26 +- docflow/cli/main.py | 157 +++++++++ docflow/integrations/zotero.py | 272 ++++++++++++++- docflow/templates/Makefile.template | 20 +- tests/test_cli.py | 129 +++++++ tests/test_makefile.py | 26 ++ tests/test_zotero.py | 315 ++++++++++++++++++ 7 files changed, 937 insertions(+), 8 deletions(-) diff --git a/.specify/specs/zotero-export-per-collection.md b/.specify/specs/zotero-export-per-collection.md index 082289b..ed1ae66 100644 --- a/.specify/specs/zotero-export-per-collection.md +++ b/.specify/specs/zotero-export-per-collection.md @@ -1,7 +1,31 @@ # Zotero: export subcollections as individual `.bib` files **Follows**: [`zotero-export-and-routing.md`](./zotero-export-and-routing.md) -**Status**: Design (proposed, not yet implemented) +**Status**: v1 implemented; several design-spec flags deferred (see +"v1 scope" below). + +## v1 scope (what actually shipped) + +| Piece | Shipped? | Notes | +| ----------------------------------------------------------------- | -------- | ----------------------------------------------------------------- | +| `docflow zotero list-collections [--parent] [--format]` | ✅ | `--format text\|json\|yaml` | +| `docflow zotero export-collections` | ✅ | Options below | +| `--parent KEY` | ✅ | Root the tree at a specific collection | +| `--output-dir DIR` (default `bibs/`) | ✅ | | +| `--format bibtex\|csljson\|ris` | ✅ | | +| `--naming slug\|preserve\|key` | ✅ | Slug: Unicode-aware NFKD strip, lowercase, non-alnum → `-` | +| Collision resolution (append `-` to loser) | ✅ | Deterministic sort by key | +| `--recursive/--no-recursive` | ✅ | Recursive default; ORs descendant keys via `collectionKey=A,B,C` | +| `--skip-empty/--keep-empty` | ✅ | Empty collections go to `skipped:` in manifest | +| `--manifest PATH` (default `/index.yaml`) | ✅ | | +| Makefile: `bibs`, `bibs-list` (env-var-guarded, not in `all:`) | ✅ | | +| URL-length fallback (per-collection fetch + client-side dedup) | ✅ | Kicks in above `COLLECTION_KEY_FILTER_LIMIT` (~1800 chars) | +| `--include GLOB`/`--exclude GLOB` | ⏳ | Deferred: no concrete demand yet; users can `--parent` | +| `--parent-name NAME` resolution | ⏳ | Deferred: users can grab the key via `list-collections` | +| `--depth N > 1` | ⏳ | Deferred: v1 always emits one file per output unit (depth=1) | +| `--dry-run` on export | ⏳ | Deferred: `list-collections` already covers "just tell me what's there" | +| `--only-changed`, `--prune`, `--sleep-between` | ⏳ | Deferred: revisit if used-in-anger surfaces need | +| `zotero.bibs` config block | ⏳ | Deferred: CLI flags suffice; Makefile pins them per project | ## Motivation diff --git a/docflow/cli/main.py b/docflow/cli/main.py index c2f25f6..7134bf7 100644 --- a/docflow/cli/main.py +++ b/docflow/cli/main.py @@ -4,8 +4,10 @@ import os import sys from pathlib import Path +from typing import Any import click +import yaml from docflow import __version__ from docflow.config import load_config @@ -19,6 +21,8 @@ from docflow.extract.docx import extract_comments_from_docx, format_comments_as_json from docflow.integrations.zotero import ( SUPPORTED_EXPORT_FORMATS, + SUPPORTED_NAMING_SCHEMES, + CollectionNode, LibraryResolutionError, RouteMatchError, RoutingRules, @@ -863,6 +867,159 @@ def add_dois( sys.exit(1) +def _print_collection_tree(nodes: list[CollectionNode], indent: int = 0) -> None: + """Render a CollectionNode tree as an indented text listing.""" + for node in nodes: + prefix = " " * indent + click.echo( + f"{prefix}{node.name:<50} [{node.key}] ({node.num_items})" + ) + if node.children: + _print_collection_tree(node.children, indent + 1) + + +def _nodes_to_json(nodes: list[CollectionNode]) -> list[dict[str, Any]]: + """Serialise a CollectionNode tree as a JSON-safe list of dicts.""" + return [ + { + "key": n.key, + "name": n.name, + "parent_key": n.parent_key, + "num_items": n.num_items, + "children": _nodes_to_json(n.children), + } + for n in nodes + ] + + +@zotero.command(name="list-collections") +@click.option("--parent", default=None, help="Collection key to root the tree") +@click.option( + "--format", + "-f", + "format_", + type=click.Choice(["text", "json", "yaml"]), + default="text", + show_default=True, + help="Output format", +) +@click.pass_context +def list_collections( + ctx: click.Context, parent: str | None, format_: str +) -> None: + """List Zotero collections as a tree (name, key, item count). + + \b + Examples: + docflow zotero list-collections + docflow zotero list-collections --parent BZ69VS62 + docflow zotero list-collections --format json + """ + client = _build_zotero_client(ctx, collection_hint=parent) + nodes = client.list_collections(parent_key=parent) + + if format_ == "text": + if not nodes: + click.echo("(no collections)") + return + _print_collection_tree(nodes) + elif format_ == "json": + click.echo(json.dumps(_nodes_to_json(nodes), indent=2)) + else: # yaml + click.echo(yaml.safe_dump(_nodes_to_json(nodes), sort_keys=False), nl=False) + + +@zotero.command(name="export-collections") +@click.option("--parent", default=None, help="Collection key to root under") +@click.option( + "--output-dir", + "-o", + type=click.Path(), + default="bibs", + show_default=True, + help="Directory to write per-collection files into", +) +@click.option( + "--format", + "-f", + "format_", + type=click.Choice(list(SUPPORTED_EXPORT_FORMATS)), + default="bibtex", + show_default=True, + help="Export format", +) +@click.option( + "--naming", + type=click.Choice(list(SUPPORTED_NAMING_SCHEMES)), + default="slug", + show_default=True, + help="Filename derivation from collection name", +) +@click.option( + "--recursive/--no-recursive", + default=True, + show_default=True, + help="Include items from subcollections in each output .bib", +) +@click.option( + "--skip-empty/--keep-empty", + default=True, + show_default=True, + help="Omit collections with zero items after fetching", +) +@click.option( + "--manifest", + type=click.Path(), + default=None, + help="Manifest path [default: /index.yaml]", +) +@click.pass_context +def export_collections( + ctx: click.Context, + parent: str | None, + output_dir: str, + format_: str, + naming: str, + recursive: bool, + skip_empty: bool, + manifest: str | None, +) -> None: + """Export each top-level collection to its own bibliography file. + + Writes one file per (top-level) collection under the given parent, + plus a YAML manifest mapping keys → names → filenames. + + \b + Examples: + docflow zotero export-collections + docflow zotero export-collections --parent BZ69VS62 -o bibs/ + docflow zotero export-collections --naming preserve --format csljson + """ + client = _build_zotero_client(ctx, collection_hint=parent) + out = Path(output_dir) + manifest_path = Path(manifest) if manifest else None + + manifest_dict = client.export_collection_tree( + output_dir=out, + parent_key=parent, + format=format_, + naming=naming, + recursive=recursive, + skip_empty=skip_empty, + manifest_path=manifest_path, + ) + + written = manifest_dict.get("collections") or [] + skipped = manifest_dict.get("skipped") or [] + click.echo(f"✓ Wrote {len(written)} file(s) to {out}/") + for entry in written: + click.echo(f" • {entry['file']} ({entry['item_count']} items)") + if skipped: + click.echo(f" Skipped {len(skipped)} empty collection(s):") + for entry in skipped: + click.echo(f" - {entry['name']} [{entry['key']}]") + + @main.command() @click.option("--name", help="Project name", required=True) @click.option( diff --git a/docflow/integrations/zotero.py b/docflow/integrations/zotero.py index 161fa6c..dac3492 100644 --- a/docflow/integrations/zotero.py +++ b/docflow/integrations/zotero.py @@ -3,9 +3,12 @@ import json import logging import re +import unicodedata import urllib.error import urllib.parse import urllib.request +from dataclasses import dataclass, field +from datetime import datetime, timezone from pathlib import Path from typing import Any @@ -196,6 +199,56 @@ def from_file(cls, path: Path) -> "RoutingRules": return cls(default_collection=default_coll, routes=routes) +# --- Collection tree / per-collection export ------------------------------ + +SUPPORTED_NAMING_SCHEMES = ("slug", "preserve", "key") +FILE_EXTENSIONS = {"bibtex": "bib", "csljson": "json", "ris": "ris"} +# Zotero HTTP query strings should stay well under browser/server limits; +# fall back to per-collection fetches if the OR-union grows past this. +COLLECTION_KEY_FILTER_LIMIT = 1800 + + +@dataclass +class CollectionNode: + """One node in the Zotero collection tree.""" + + key: str + name: str + parent_key: str | None + num_items: int = 0 + children: list["CollectionNode"] = field(default_factory=list) + + def walk(self) -> "list[CollectionNode]": + """Return this node and every descendant (pre-order).""" + acc = [self] + for child in self.children: + acc.extend(child.walk()) + return acc + + +def slugify_name(name: str, mode: str = "slug", key: str = "") -> str: + """Turn a Zotero collection name into a safe filename stem. + + Modes: + * ``slug`` (default) — lowercase, strip diacritics, non-alnum → ``-`` + * ``preserve`` — keep case, replace path-unsafe chars with ``_`` + * ``key`` — return the collection key verbatim + """ + if mode == "key": + return key + if mode == "preserve": + # Replace any character that would confuse a filesystem with '_' + return re.sub(r'[\x00-\x1f/\\:*?"<>|]+', "_", name).strip() or key + if mode == "slug": + norm = unicodedata.normalize("NFKD", name) + ascii_only = "".join(c for c in norm if not unicodedata.combining(c)) + slug = re.sub(r"[^a-zA-Z0-9]+", "-", ascii_only).strip("-").lower() + return slug or key.lower() + raise ValueError( + f"Unknown naming mode {mode!r}; expected one of {SUPPORTED_NAMING_SCHEMES}" + ) + + class ZoteroClient: """Client for Zotero API with deduplication and batch operations.""" @@ -492,16 +545,21 @@ def _items_endpoint(self, collection_key: str | None) -> str: return f"/{library_type}/{library_id}/collections/{collection_key}/items" return f"/{library_type}/{library_id}/items" - def _paginated_export(self, endpoint: str, format: str) -> Any: + def _paginated_export( + self, + endpoint: str, + format: str, + extra_params: dict[str, str] | None = None, + ) -> Any: """Yield successive pages of raw text for a Zotero export endpoint. Uses ``start``/``limit`` pagination. Stops when a page returns - fewer than ``ZOTERO_EXPORT_PAGE_SIZE`` items — measured differently - per format: + fewer than ``ZOTERO_EXPORT_PAGE_SIZE`` items — measured via the + ``Total-Results`` response header. - * ``bibtex`` / ``ris``: count ``@`` / blank-line-separated - records via the ``Total-Results`` response header. - * ``csljson``: count the JSON array length. + ``extra_params`` is merged into the query string; used e.g. to + pass a ``collectionKey=A,B,C`` filter when exporting a + collection tree. """ start = 0 total: int | None = None @@ -511,6 +569,8 @@ def _paginated_export(self, endpoint: str, format: str) -> Any: "limit": str(ZOTERO_EXPORT_PAGE_SIZE), "start": str(start), } + if extra_params: + params.update(extra_params) url = ( f"{ZOTERO_API_BASE}{endpoint}?" + urllib.parse.urlencode(params) @@ -536,6 +596,206 @@ def _paginated_export(self, endpoint: str, format: str) -> Any: if total <= 0 or start >= total: return + # --- Collection tree / per-collection export -------------------------- + + def list_collections( + self, parent_key: str | None = None + ) -> list[CollectionNode]: + """Return the collection tree rooted at ``parent_key``. + + ``parent_key=None`` returns the top-level tree; a specific key + returns the subtree headed by that collection. Uses pyzotero's + ``all_collections`` under the hood. + """ + raw = self.zot.all_collections(collid=parent_key) + by_key: dict[str, CollectionNode] = {} + for row in raw: + data = row.get("data") or {} + key = data.get("key") or row.get("key") + if not key: + continue + by_key[key] = CollectionNode( + key=key, + name=data.get("name", key), + parent_key=data.get("parentCollection") or None, + num_items=(row.get("meta") or {}).get("numItems", 0), + ) + + roots: list[CollectionNode] = [] + for node in by_key.values(): + if node.parent_key and node.parent_key in by_key: + by_key[node.parent_key].children.append(node) + elif parent_key is None or node.key == parent_key: + roots.append(node) + # If parent_key was given, only return that node (with descendants) + if parent_key and parent_key in by_key: + return [by_key[parent_key]] + # Stable ordering by name for reproducible manifests + for node in by_key.values(): + node.children.sort(key=lambda c: c.name.lower()) + roots.sort(key=lambda n: n.name.lower()) + return roots + + def export_collection_tree( + self, + output_dir: Path, + parent_key: str | None = None, + format: str = "bibtex", + naming: str = "slug", + recursive: bool = True, + skip_empty: bool = True, + manifest_path: Path | None = None, + ) -> dict[str, Any]: + """Write one bibliography per top-level collection under ``parent_key``. + + Returns the manifest dict (also written to ``manifest_path`` / + ``/index.yaml`` when applicable). + """ + if format not in SUPPORTED_EXPORT_FORMATS: + raise ValueError( + f"Unsupported export format {format!r}; " + f"expected one of {SUPPORTED_EXPORT_FORMATS}" + ) + if naming not in SUPPORTED_NAMING_SCHEMES: + raise ValueError( + f"Unknown naming mode {naming!r}; " + f"expected one of {SUPPORTED_NAMING_SCHEMES}" + ) + + output_dir.mkdir(parents=True, exist_ok=True) + ext = FILE_EXTENSIONS[format] + tree = self.list_collections(parent_key=parent_key) + # "Output units": children of parent_key (or roots if None). + # If parent_key names a single collection, use *its* children as + # units so the user gets one .bib per subcollection under it. + if parent_key and tree: + output_units = list(tree[0].children) + else: + output_units = list(tree) + + # Deterministic filename assignment with collision resolution + planned: list[tuple[CollectionNode, str, list[str]]] = [] + used_stems: dict[str, str] = {} # stem → owning collection key + for node in sorted(output_units, key=lambda n: n.key): + descendants = [n.key for n in node.walk()] if recursive else [node.key] + stem = slugify_name(node.name, naming, key=node.key) + if stem in used_stems and used_stems[stem] != node.key: + stem = f"{stem}-{node.key}" + logger.warning( + "Filename collision for collection %r (key=%s); " + "wrote %s to disambiguate", + node.name, node.key, f"{stem}.{ext}", + ) + used_stems[stem] = node.key + planned.append((node, stem, descendants)) + + # Sort back to name-order for stable manifest output + planned.sort(key=lambda t: t[0].name.lower()) + + collections_out: list[dict[str, Any]] = [] + skipped_out: list[dict[str, Any]] = [] + endpoint = self._items_endpoint(None) # library-wide; filter by collectionKey + for node, stem, descendants in planned: + content, count = self._fetch_collection_export( + endpoint, format, descendants + ) + if skip_empty and count == 0: + skipped_out.append({ + "key": node.key, "name": node.name, "reason": "empty", + }) + continue + fname = f"{stem}.{ext}" + (output_dir / fname).write_text(content) + collections_out.append({ + "key": node.key, + "name": node.name, + "file": fname, + "item_count": count, + "recursive": recursive, + "descendant_keys": descendants if recursive else [node.key], + }) + + manifest = { + "generated_at": datetime.now(timezone.utc).isoformat(timespec="seconds"), + "library": {"id": self.group_id, "mode": self.mode}, + "parent_key": parent_key, + "format": format, + "naming": naming, + "recursive": recursive, + "collections": collections_out, + "skipped": skipped_out, + } + mpath = manifest_path or (output_dir / "index.yaml") + mpath.write_text(yaml.safe_dump(manifest, sort_keys=False)) + logger.info( + "Wrote %d bibliograph%s + manifest to %s", + len(collections_out), + "y" if len(collections_out) == 1 else "ies", + output_dir, + ) + return manifest + + def _fetch_collection_export( + self, + endpoint: str, + format: str, + collection_keys: list[str], + ) -> tuple[str, int]: + """Fetch and merge exported items across ``collection_keys``. + + Returns ``(text_content, item_count)``. Uses the Zotero + ``collectionKey=A,B,C`` filter in one request when the query + stays under ``COLLECTION_KEY_FILTER_LIMIT``; otherwise falls + back to per-collection fetches with client-side dedup by item + key (for CSL-JSON) or raw concatenation (for text formats, + where dedup would require BibTeX parsing). + """ + joined = ",".join(collection_keys) + if len(joined) <= COLLECTION_KEY_FILTER_LIMIT: + pages = list(self._paginated_export( + endpoint, format, extra_params={"collectionKey": joined} + )) + return self._assemble_pages(pages, format) + + # Fallback: per-collection, dedup on the client side + all_pages: list[str] = [] + seen: set[str] = set() + for key in collection_keys: + pages = list(self._paginated_export( + endpoint, format, extra_params={"collectionKey": key} + )) + text, _ = self._assemble_pages(pages, format) + if format == "csljson": + arr = json.loads(text) if text else [] + new = [e for e in arr if e.get("id") and e["id"] not in seen] + seen.update(e["id"] for e in new if e.get("id")) + if new: + all_pages.append(json.dumps(new)) + elif text.strip(): + all_pages.append(text) + return self._assemble_pages(all_pages, format) + + @staticmethod + def _assemble_pages(pages: list[str], format: str) -> tuple[str, int]: + """Combine paginated pages into a single string; report item count.""" + if format == "csljson": + merged: list[Any] = [] + for page in pages: + if page: + merged.extend(json.loads(page)) + return ( + json.dumps(merged, indent=2, ensure_ascii=False) if merged else "", + len(merged), + ) + non_empty = [p.strip() for p in pages if p and p.strip()] + text = ("\n\n".join(non_empty) + "\n") if non_empty else "" + # Best-effort record count: @entries for bibtex, "TY -" starts for ris + if format == "bibtex": + count = len(re.findall(r"^@\w+\s*[{(]", text, flags=re.MULTILINE)) + else: # ris + count = len(re.findall(r"^TY\s{2}-", text, flags=re.MULTILINE)) + return text, count + def _create_item_from_citation(self, citation: Citation) -> dict[str, Any] | None: """Create Zotero item from citation. diff --git a/docflow/templates/Makefile.template b/docflow/templates/Makefile.template index a0510b6..0ce0589 100644 --- a/docflow/templates/Makefile.template +++ b/docflow/templates/Makefile.template @@ -14,7 +14,7 @@ # The $(wildcard) function will NOT find files with spaces, so pattern rules # won't match them. This is a fundamental Make limitation. # -.PHONY: sync md tsv comments zotero-check zotero-push bib all clean default help +.PHONY: sync md tsv comments zotero-check zotero-push bib bibs bibs-list all clean default help .DEFAULT_GOAL := default # Configuration @@ -125,6 +125,22 @@ refs.bib: docflow zotero export --output "$@" @echo "✓ Wrote $@" +# Export one .bib per top-level Zotero collection into bibs/ +bibs: + @if [ -z "$$ZOTERO_API_KEY" ]; then \ + echo "Error: ZOTERO_API_KEY not set"; \ + exit 1; \ + fi + docflow zotero export-collections --output-dir bibs + +# List Zotero collections (tree with keys and item counts) +bibs-list: + @if [ -z "$$ZOTERO_API_KEY" ]; then \ + echo "Error: ZOTERO_API_KEY not set"; \ + exit 1; \ + fi + docflow zotero list-collections + # Clean generated files (markdown, tsv, comments, citations) clean: @echo "Cleaning generated files..." @@ -143,5 +159,7 @@ help: @echo " make zotero-push - Push citations to Zotero" @echo " make refs.bib - Export Zotero library to refs.bib" @echo " make bib - Alias for refs.bib" + @echo " make bibs - Export each Zotero collection to bibs/.bib" + @echo " make bibs-list - List available Zotero collections" @echo " make all - Run full workflow + Zotero check" @echo " make clean - Remove converted/ directory" diff --git a/tests/test_cli.py b/tests/test_cli.py index 487d6ec..9df7c51 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -582,3 +582,132 @@ def test_zotero_push_exits_nonzero_on_failure( ) assert result.exit_code != 0, result.output assert "Failed: 1" in result.output + + +# --------------------------------------------------------------------------- +# `docflow zotero list-collections` / `export-collections` +# --------------------------------------------------------------------------- + + +def _fake_coll(key: str, name: str, parent: str | None = None, n: int = 0) -> dict: + return { + "data": {"key": key, "name": name, "parentCollection": parent or False}, + "meta": {"numItems": n}, + } + + +@pytest.mark.ai_generated +def test_zotero_list_collections_help(cli_runner: CliRunner) -> None: + result = cli_runner.invoke(main, ["zotero", "list-collections", "--help"]) + assert result.exit_code == 0 + assert "--parent" in result.output + assert "--format" in result.output + + +@pytest.mark.ai_generated +def test_zotero_export_collections_help(cli_runner: CliRunner) -> None: + result = cli_runner.invoke(main, ["zotero", "export-collections", "--help"]) + assert result.exit_code == 0 + for flag in ("--parent", "--output-dir", "--format", "--naming", + "--recursive", "--no-recursive", + "--skip-empty", "--keep-empty", + "--manifest"): + assert flag in result.output + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.zotero") +def test_zotero_list_collections_text( + mock_pyz, cli_runner: CliRunner, tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("ZOTERO_API_KEY", "k") + cfg = _write_config(tmp_path) + + zot = MagicMock() + zot.all_collections.return_value = [ + _fake_coll("AAA", "BIDS and BEPs", n=18), + _fake_coll("BBB", "BEPs", parent="AAA", n=12), + _fake_coll("CCC", "Related", n=8), + ] + mock_pyz.Zotero.return_value = zot + + result = cli_runner.invoke( + main, ["--config", str(cfg), "zotero", "list-collections"] + ) + assert result.exit_code == 0, result.output + assert "BIDS and BEPs" in result.output + assert "[AAA]" in result.output + assert "(18)" in result.output + assert "BEPs" in result.output + assert "Related" in result.output + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.zotero") +def test_zotero_list_collections_json( + mock_pyz, cli_runner: CliRunner, tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("ZOTERO_API_KEY", "k") + cfg = _write_config(tmp_path) + + zot = MagicMock() + zot.all_collections.return_value = [_fake_coll("AAA", "Only", n=1)] + mock_pyz.Zotero.return_value = zot + + result = cli_runner.invoke( + main, ["--config", str(cfg), "zotero", "list-collections", "--format", "json"] + ) + assert result.exit_code == 0, result.output + parsed = json.loads(result.output) + assert parsed == [ + { + "key": "AAA", "name": "Only", "parent_key": None, + "num_items": 1, "children": [], + } + ] + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.urllib.request.urlopen") +@patch("docflow.integrations.zotero.zotero") +def test_zotero_export_collections_end_to_end( + mock_pyz, mock_urlopen, cli_runner: CliRunner, tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """End-to-end: two top-level collections → two .bib files + index.yaml.""" + monkeypatch.setenv("ZOTERO_API_KEY", "k") + cfg = _write_config(tmp_path) + + zot = MagicMock() + zot.all_collections.return_value = [ + _fake_coll("AAA", "BIDS and BEPs", n=1), + _fake_coll("CCC", "Related", n=1), + ] + mock_pyz.Zotero.return_value = zot + + def _resp(txt: str) -> MagicMock: + r = MagicMock() + r.read.return_value = txt.encode("utf-8") + r.headers = {"Total-Results": "1"} + r.__enter__.return_value = r + r.__exit__.return_value = False + return r + + mock_urlopen.side_effect = [_resp("@article{a}"), _resp("@article{c}")] + + out_dir = tmp_path / "bibs" + result = cli_runner.invoke( + main, + [ + "--config", str(cfg), + "zotero", "export-collections", + "--output-dir", str(out_dir), + ], + ) + assert result.exit_code == 0, result.output + assert (out_dir / "bids-and-beps.bib").exists() + assert (out_dir / "related.bib").exists() + assert (out_dir / "index.yaml").exists() + assert "Wrote 2 file(s)" in result.output diff --git a/tests/test_makefile.py b/tests/test_makefile.py index 3e8b7cb..4c10782 100644 --- a/tests/test_makefile.py +++ b/tests/test_makefile.py @@ -91,3 +91,29 @@ def test_makefile_template_has_bib_target() -> None: line for line in content.splitlines() if line.startswith("all:") ) assert " bib" in all_line, f"`bib` missing from `all:` recipe: {all_line!r}" + + +@pytest.mark.ai_generated +def test_makefile_template_has_bibs_targets() -> None: + """Generated Makefile exposes `bibs` and `bibs-list` targets (per-collection).""" + template_path = ( + Path(__file__).parent.parent / "docflow" / "templates" / "Makefile.template" + ) + content = template_path.read_text() + + assert "\nbibs:\n" in content, "Missing `bibs:` target" + assert "\nbibs-list:\n" in content, "Missing `bibs-list:` target" + assert "docflow zotero export-collections" in content + assert "docflow zotero list-collections" in content + # Neither should be a prerequisite of `all:` (rate-limit concern) + all_line = next( + line for line in content.splitlines() if line.startswith("all:") + ) + assert " bibs" not in all_line, ( + f"`bibs` should not be in `all:` recipe: {all_line!r}" + ) + # And they're in .PHONY + phony_line = next( + line for line in content.splitlines() if line.startswith(".PHONY:") + ) + assert " bibs " in phony_line and " bibs-list " in phony_line diff --git a/tests/test_zotero.py b/tests/test_zotero.py index 07f51cf..25ae4b9 100644 --- a/tests/test_zotero.py +++ b/tests/test_zotero.py @@ -5,16 +5,19 @@ from unittest.mock import MagicMock, patch import pytest +import yaml as _yaml from docflow.config import DocflowConfig from docflow.extract.citations import Citation, CitationType from docflow.integrations.zotero import ( + CollectionNode, LibraryResolutionError, RouteMatchError, RoutingRule, RoutingRules, ZoteroClient, resolve_library_from_key, + slugify_name, ) @@ -978,3 +981,315 @@ def test_resolve_library_no_access(mock_urlopen) -> None: ) with pytest.raises(LibraryResolutionError, match="no library access"): resolve_library_from_key("k") + + +# --------------------------------------------------------------------------- +# Collection tree / per-collection export (issue #2 follow-up) +# --------------------------------------------------------------------------- + + +def _mk_coll(key: str, name: str, parent: str | None = None, n: int = 0) -> dict: + """Shape a fake pyzotero collection dict.""" + return { + "data": {"key": key, "name": name, "parentCollection": parent or False}, + "meta": {"numItems": n}, + } + + +@pytest.mark.ai_generated +def test_slugify_name_slug_mode() -> None: + assert slugify_name("BIDS and BEPs") == "bids-and-beps" + assert slugify_name("Related / Adjacent") == "related-adjacent" + assert slugify_name("Réseau BIDS") == "reseau-bids" + + +@pytest.mark.ai_generated +def test_slugify_name_preserve_mode() -> None: + assert slugify_name("BIDS and BEPs", mode="preserve") == "BIDS and BEPs" + # path-unsafe characters normalized to _ + assert slugify_name("a/b:c", mode="preserve") == "a_b_c" + + +@pytest.mark.ai_generated +def test_slugify_name_key_mode() -> None: + assert slugify_name("anything", mode="key", key="ABC123") == "ABC123" + + +@pytest.mark.ai_generated +def test_slugify_name_falls_back_to_key_when_empty() -> None: + # A name that becomes empty after slugging falls back to the key + assert slugify_name("!!!", mode="slug", key="ABC123") == "abc123" + + +@pytest.mark.ai_generated +def test_slugify_name_rejects_bad_mode() -> None: + with pytest.raises(ValueError, match="Unknown naming mode"): + slugify_name("x", mode="ristretto") + + +@pytest.mark.ai_generated +def test_list_collections_builds_tree(mock_zotero) -> None: + """A flat pyzotero response is assembled into a parent→children tree.""" + mock_zotero.all_collections.return_value = [ + _mk_coll("AAA", "BIDS and BEPs", n=18), + _mk_coll("BBB", "BEPs", parent="AAA", n=12), + _mk_coll("CCC", "Related", n=8), + ] + client = ZoteroClient(group_id="123", api_key="key") + tree = client.list_collections() + + assert len(tree) == 2 + names = [n.name for n in tree] + assert names == ["BIDS and BEPs", "Related"] # sorted + assert len(tree[0].children) == 1 + assert tree[0].children[0].name == "BEPs" + assert tree[0].num_items == 18 + + +@pytest.mark.ai_generated +def test_collection_node_walk_returns_descendants() -> None: + root = CollectionNode(key="R", name="root", parent_key=None) + a = CollectionNode(key="A", name="a", parent_key="R") + b = CollectionNode(key="B", name="b", parent_key="A") + root.children.append(a) + a.children.append(b) + assert [n.key for n in root.walk()] == ["R", "A", "B"] + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.urllib.request.urlopen") +def test_export_collection_tree_writes_one_bib_per_toplevel( + mock_urlopen, mock_zotero, tmp_path: Path, +) -> None: + """One .bib per top-level collection, plus a manifest.""" + mock_zotero.all_collections.return_value = [ + _mk_coll("AAA", "BIDS and BEPs", n=2), + _mk_coll("CCC", "Related", n=1), + ] + + # One export response per top-level collection (both recursive, no subs) + def _resp(text: str) -> MagicMock: + r = MagicMock() + r.read.return_value = text.encode("utf-8") + r.headers = {"Total-Results": "1"} + r.__enter__.return_value = r + r.__exit__.return_value = False + return r + + mock_urlopen.side_effect = [ + _resp("@article{aaa1}\n@article{aaa2}"), + _resp("@article{ccc1}"), + ] + + client = ZoteroClient(group_id="123", api_key="key") + manifest = client.export_collection_tree( + output_dir=tmp_path, format="bibtex", naming="slug" + ) + + assert (tmp_path / "bids-and-beps.bib").exists() + assert (tmp_path / "related.bib").exists() + assert (tmp_path / "index.yaml").exists() + + # collectionKey filter was included on both requests + urls = [c.args[0].full_url for c in mock_urlopen.call_args_list] + assert any("collectionKey=AAA" in u for u in urls) + assert any("collectionKey=CCC" in u for u in urls) + + # Manifest content + parsed = _yaml.safe_load((tmp_path / "index.yaml").read_text()) + assert parsed == manifest + files = {e["file"] for e in parsed["collections"]} + assert files == {"bids-and-beps.bib", "related.bib"} + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.urllib.request.urlopen") +def test_export_collection_tree_recursive_unions_descendant_keys( + mock_urlopen, mock_zotero, tmp_path: Path, +) -> None: + """Recursive export ORs the collectionKey filter across descendants.""" + mock_zotero.all_collections.return_value = [ + _mk_coll("PPP", "Parent", n=1), + _mk_coll("QQQ", "Child", parent="PPP", n=1), + ] + + resp = MagicMock() + resp.read.return_value = b"@article{x}" + resp.headers = {"Total-Results": "2"} + resp.__enter__.return_value = resp + resp.__exit__.return_value = False + mock_urlopen.return_value = resp + + client = ZoteroClient(group_id="123", api_key="key") + client.export_collection_tree( + output_dir=tmp_path, format="bibtex", recursive=True + ) + + url = mock_urlopen.call_args[0][0].full_url + # Both parent and child keys land in the collectionKey filter + assert "collectionKey=PPP%2CQQQ" in url or "collectionKey=PPP,QQQ" in url + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.urllib.request.urlopen") +def test_export_collection_tree_non_recursive_uses_only_own_key( + mock_urlopen, mock_zotero, tmp_path: Path, +) -> None: + mock_zotero.all_collections.return_value = [ + _mk_coll("PPP", "Parent", n=1), + _mk_coll("QQQ", "Child", parent="PPP", n=1), + ] + + resp = MagicMock() + resp.read.return_value = b"@article{x}" + resp.headers = {"Total-Results": "1"} + resp.__enter__.return_value = resp + resp.__exit__.return_value = False + mock_urlopen.return_value = resp + + client = ZoteroClient(group_id="123", api_key="key") + client.export_collection_tree( + output_dir=tmp_path, format="bibtex", recursive=False + ) + + url = mock_urlopen.call_args[0][0].full_url + # Only the parent's own key — no OR-union with descendants + assert "collectionKey=PPP" in url + assert "QQQ" not in url + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.urllib.request.urlopen") +def test_export_collection_tree_skip_empty( + mock_urlopen, mock_zotero, tmp_path: Path, +) -> None: + """Empty collections aren't written; they land in the manifest 'skipped' list.""" + mock_zotero.all_collections.return_value = [ + _mk_coll("AAA", "Has items", n=1), + _mk_coll("BBB", "Empty", n=0), + ] + + def _resp(text: str, total: int) -> MagicMock: + r = MagicMock() + r.read.return_value = text.encode("utf-8") + r.headers = {"Total-Results": str(total)} + r.__enter__.return_value = r + r.__exit__.return_value = False + return r + + # Fetch order is by collection name (alphabetical): "Empty" then "Has items" + mock_urlopen.side_effect = [ + _resp("", 0), + _resp("@article{one}", 1), + ] + + client = ZoteroClient(group_id="123", api_key="key") + manifest = client.export_collection_tree( + output_dir=tmp_path, format="bibtex", skip_empty=True + ) + + assert (tmp_path / "has-items.bib").exists() + assert not (tmp_path / "empty.bib").exists() + assert [e["name"] for e in manifest["skipped"]] == ["Empty"] + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.urllib.request.urlopen") +def test_export_collection_tree_csljson_format( + mock_urlopen, mock_zotero, tmp_path: Path, +) -> None: + """CSL-JSON export produces .json files with valid JSON arrays.""" + mock_zotero.all_collections.return_value = [_mk_coll("AAA", "Only", n=2)] + + r = MagicMock() + r.read.return_value = json.dumps([{"id": "a"}, {"id": "b"}]).encode("utf-8") + r.headers = {"Total-Results": "2"} + r.__enter__.return_value = r + r.__exit__.return_value = False + mock_urlopen.return_value = r + + client = ZoteroClient(group_id="123", api_key="key") + client.export_collection_tree( + output_dir=tmp_path, format="csljson", skip_empty=False + ) + + out = tmp_path / "only.json" + assert out.exists() + parsed = json.loads(out.read_text()) + assert parsed == [{"id": "a"}, {"id": "b"}] + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.urllib.request.urlopen") +def test_export_collection_tree_slug_collision( + mock_urlopen, mock_zotero, tmp_path: Path, +) -> None: + """When two names slug to the same stem, the loser gets the - suffix.""" + mock_zotero.all_collections.return_value = [ + _mk_coll("AAA", "Related", n=1), + _mk_coll("BBB", "related!", n=1), + ] + + def _resp() -> MagicMock: + r = MagicMock() + r.read.return_value = b"@article{x}" + r.headers = {"Total-Results": "1"} + r.__enter__.return_value = r + r.__exit__.return_value = False + return r + + mock_urlopen.side_effect = [_resp(), _resp()] + + client = ZoteroClient(group_id="123", api_key="key") + manifest = client.export_collection_tree( + output_dir=tmp_path, format="bibtex" + ) + + files = sorted(e["file"] for e in manifest["collections"]) + # Deterministic by sort-order of collection key: AAA wins, BBB is disambiguated + assert files == ["related-BBB.bib", "related.bib"] + + +@pytest.mark.ai_generated +@patch("docflow.integrations.zotero.urllib.request.urlopen") +def test_export_collection_tree_under_parent( + mock_urlopen, mock_zotero, tmp_path: Path, +) -> None: + """When --parent is given, output units are that parent's children.""" + mock_zotero.all_collections.return_value = [ + _mk_coll("ROOT", "BIDS", n=0), + _mk_coll("BBB", "BEPs", parent="ROOT", n=1), + _mk_coll("CCC", "Related", parent="ROOT", n=1), + ] + + def _resp() -> MagicMock: + r = MagicMock() + r.read.return_value = b"@article{x}" + r.headers = {"Total-Results": "1"} + r.__enter__.return_value = r + r.__exit__.return_value = False + return r + + mock_urlopen.side_effect = [_resp(), _resp()] + + client = ZoteroClient(group_id="123", api_key="key") + manifest = client.export_collection_tree( + output_dir=tmp_path, parent_key="ROOT", format="bibtex" + ) + + files = sorted(e["file"] for e in manifest["collections"]) + assert files == ["beps.bib", "related.bib"] + + +@pytest.mark.ai_generated +def test_export_collection_tree_rejects_bad_format(mock_zotero, tmp_path: Path) -> None: + client = ZoteroClient(group_id="123", api_key="key") + with pytest.raises(ValueError, match="Unsupported export format"): + client.export_collection_tree(output_dir=tmp_path, format="endnote") + + +@pytest.mark.ai_generated +def test_export_collection_tree_rejects_bad_naming(mock_zotero, tmp_path: Path) -> None: + client = ZoteroClient(group_id="123", api_key="key") + with pytest.raises(ValueError, match="Unknown naming mode"): + client.export_collection_tree(output_dir=tmp_path, naming="ristretto")