feat(documents): add bibliographic citation#1129
Conversation
WalkthroughAdds CSL-based citation export for SONAR documents. Records are mapped to CSL-JSON, rendered through four bundled styles—APA, Chicago, Harvard, and MLA—and exposed through style-discovery and document-citation REST endpoints with access checks. The change adds document type and metadata mapping, citation registry exports, the Estimated code review effort: 5 (Critical) | ~120 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PascalRepond
left a comment
There was a problem hiding this comment.
Some fields missing from the mapping:
- editionStatement.editionDesignation.value --> edition
- series[].name / series[].number --> collection-title / collection-number
- contribution[agent.type=bf:Meeting] (.preferred_name, .place, .date) --> event / event-place / event-date
- extent --> number-of-pages (probably needs a heuristics to convert extent to an
int)
9d2fad7 to
da7f2fb
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
sonar/modules/documents/citations/csl_mapping.py (2)
272-273: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider using
_get_date_partsforevent-dateinstead of year-only.
issueddates use_get_date_parts(full YYYY-MM-DD precision), butevent-dateuses_get_yearwhich truncates to year-only. Meeting dates in SONAR may also carry full-day precision. If consistency is desired,_get_date_partscould be used here as well.♻️ Suggested optional refactor
- if meeting_year := _get_year(meeting_date): - item["event-date"] = {"date-parts": [[int(meeting_year)]]} + if meeting_date_parts := _get_date_parts(meeting_date): + item["event-date"] = {"date-parts": [meeting_date_parts]}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sonar/modules/documents/citations/csl_mapping.py` around lines 272 - 273, Update the event-date mapping to use _get_date_parts with meeting_date instead of _get_year and integer conversion, preserving the existing conditional behavior while retaining available full date precision.
8-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDecouple
_get_urlfrom Flask's request context.
csl_mapping.pyis a data-mapping module, butfrom flask import requestat module level andrequest.host_urlinside_get_urltie it to an active HTTP request. Any caller outside a request context (CLI tools, background jobs, unit tests withouttest_request_context) will hitRuntimeError: Working outside of request contextwhen the record has apid.Consider accepting the host URL (or a pre-built permanent link) as a parameter to
record_to_csl, letting the REST endpoint passrequest.host_urland other callers pass their own value.♻️ Suggested refactor
def _get_url(record, host_url=None): """Return the document's permanent link (ARK if available), if a pid is known.""" if not record.get("pid") or not host_url: return None from sonar.modules.documents.api import DocumentRecord return DocumentRecord.get_permanent_link(host_url, record["pid"]) def record_to_csl(record, lang=None, host_url=None): """Return a CSL-JSON item dict for the given SONAR document record. :param record: Document record dict. :param lang: Optional ISO 639-1 language code used to pick the matching title/subtitle translation. :param host_url: Optional base URL for building the document permalink. """ # ... if url := _get_url(record, host_url=host_url): item["URL"] = urlThen in
rest.py:- return jsonify({"citation": citation_registry.format(record, style, lang=lang)}) + return jsonify({"citation": citation_registry.format(record, style, lang=lang, host_url=request.host_url)})Also applies to: 174-184
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sonar/modules/documents/citations/csl_mapping.py` at line 8, Decouple the citation mapping flow from Flask’s request context by removing the module-level request dependency and updating _get_url to receive a host URL or prebuilt permanent link explicitly. Extend record_to_csl to accept and forward that value, while the REST endpoint supplies request.host_url and non-HTTP callers provide their own value; preserve PID URL generation when a value is supplied.sonar/modules/documents/rest.py (1)
88-117: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueConsider wrapping
citation_registry.format()in error handling.If citeproc-py raises an internal error (e.g.,
AttributeError,UnboundLocalErroron unexpected CSL data), the endpoint returns an unhelpful 500. A targeted catch would let you return a meaningful message to the client.♻️ Suggested optional refactor
return jsonify({"citation": citation_registry.format(record, style, lang=lang)})could become:
+ try: + citation = citation_registry.format(record, style, lang=lang) + except Exception: + return jsonify({"message": "Citation could not be generated for this document."}), 500 + return jsonify({"citation": citation})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sonar/modules/documents/rest.py` around lines 88 - 117, Wrap the citation_registry.format call in citation with targeted error handling for citeproc formatting failures, including unexpected AttributeError and UnboundLocalError cases. Return a meaningful client-facing error response instead of allowing these failures to produce an unhelpful 500, while preserving the existing successful citation response.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/unit/documents/test_citations.py`:
- Around line 397-406: Extract the duplicated BOOK-based record containing the
creator and ctb contribution from test_contributor_role_mapped and
test_contributor_does_not_crash_apa_or_mla into a shared module-level constant
alongside ARTICLE, BOOK, or EDITOR_ONLY. Update both tests to reuse that
constant while preserving their existing assertions and behavior.
- Around line 409-428: Update test_meeting_excluded_from_names so the author
assertion inspects the names within the author dictionaries rather than
comparing the meeting name to the list itself. Preserve the existing checks for
contributor omission and event assignment, and ensure the assertion would fail
if the meeting name appeared in any author entry.
- Around line 490-509: Hoist the duplicated meeting record used by
test_meeting_excluded_from_names and test_csl_mapping_meeting into a shared
module-level constant alongside ARTICLE, BOOK, EDITOR_ONLY, and MULTILANG.
Update both tests to reference that constant while preserving their existing
assertions and behavior.
---
Nitpick comments:
In `@sonar/modules/documents/citations/csl_mapping.py`:
- Around line 272-273: Update the event-date mapping to use _get_date_parts with
meeting_date instead of _get_year and integer conversion, preserving the
existing conditional behavior while retaining available full date precision.
- Line 8: Decouple the citation mapping flow from Flask’s request context by
removing the module-level request dependency and updating _get_url to receive a
host URL or prebuilt permanent link explicitly. Extend record_to_csl to accept
and forward that value, while the REST endpoint supplies request.host_url and
non-HTTP callers provide their own value; preserve PID URL generation when a
value is supplied.
In `@sonar/modules/documents/rest.py`:
- Around line 88-117: Wrap the citation_registry.format call in citation with
targeted error handling for citeproc formatting failures, including unexpected
AttributeError and UnboundLocalError cases. Return a meaningful client-facing
error response instead of allowing these failures to produce an unhelpful 500,
while preserving the existing successful citation response.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0c03d938-a852-4816-9c81-0b7294e65be8
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
pyproject.tomlsonar/modules/documents/citations/__init__.pysonar/modules/documents/citations/csl_mapping.pysonar/modules/documents/citations/registry.pysonar/modules/documents/citations/styles/apa-7th-edition.cslsonar/modules/documents/citations/styles/chicago-author-date-17th-edition.cslsonar/modules/documents/citations/styles/harvard-cite-them-right-12th-edition.cslsonar/modules/documents/citations/styles/modern-language-association-9th-edition.cslsonar/modules/documents/citations/type_mapping.pysonar/modules/documents/rest.pytests/api/documents/test_documents_cite.pytests/unit/documents/test_citations.py
da7f2fb to
202ce29
Compare
* Render citations with citeproc-py against real CSL style files (APA 7, Chicago 17 author-date, MLA 9, Harvard 12) * Vendor the 4 CSL style files under citations/styles/, named with their edition so multiple editions of the same style can coexist; Chicago is pinned to a pre-18th-edition revision to avoid an unfixed citeproc-py crash on cited page ranges * Map SONAR documentType (COAR resource type) to CSL-JSON item types, covering all 49 COAR values (article, book, chapter, thesis, report, conference paper, patent, periodical, broadcast, review, ...) * Map edition, series, bf:Meeting (event/event-place/event-date) and extent (number-of-pages) to their CSL-JSON equivalents * Fall back to partOf.numberingYear then dissertation.date when provisionActivity is missing, and preserve full day precision (year-month-day) when available, since APA/MLA render it for document types where their convention calls for it * Group contribution names by CSL role (author/editor/contributor) instead of a creator-or-editor fallback, so a document can have both at once; exclude bf:Meeting contributions from names and keep corporate/organisation names unsplit * Work around a citeproc-py crash when rendering the CSL "contributor" variable for APA/MLA book-like records with no editor * Add a permanent link back to the document as a CSL URL fallback, passed in as an explicit host_url parameter rather than read from Flask's request context, so the mapping module stays usable outside of a request * Enforce document read permissions on GET /<pid>/citation, denying masked or restricted documents to anonymous/unauthorized users * Return a clear 500 message instead of an unhandled crash when citeproc-py fails to render a citation * Mark citation style labels for translation * Add GET /api/documents/<pid>/citation?style=&lang= endpoint * Add GET /api/documents/citation-styles endpoint * Closes rero#409. Co-Authored-By: Bertrand Zuchuat <bertrand.zuchuat@rero.ch>
202ce29 to
28a48a7
Compare
(APA 7, Chicago 17 author-date, MLA 9, Harvard 12)
their edition so multiple editions of the same style can coexist;
Chicago is pinned to a pre-18th-edition revision to avoid an
unfixed citeproc-py crash on cited page ranges
covering all 49 COAR values (article, book, chapter, thesis,
report, conference paper, patent, periodical, broadcast, review, ...)
extent (number-of-pages) to their CSL-JSON equivalents
provisionActivity is missing, and preserve full day precision
(year-month-day) when available, since APA/MLA render it for
document types where their convention calls for it
instead of a creator-or-editor fallback, so a document can have
both at once; exclude bf:Meeting contributions from names and
keep corporate/organisation names unsplit
"contributor" variable for APA/MLA book-like records with no editor
passed in as an explicit host_url parameter rather than read from
Flask's request context, so the mapping module stays usable outside
of a request
masked or restricted documents to anonymous/unauthorized users
citeproc-py fails to render a citation
<pid>/citation?style=&lang= endpointUI: rero/sonar-ui#485