feat(website, backend): Sequence citations #6491
Conversation
…d exposing doiPrefix for use outside of the service
…, added periodic task to update citations with crossref results
… citing sources, with source ID that can be either a DOI or URL if a manual citation does not have a DOI
…us into sequence-citations
|
Claude finished @tombch's task in 2m 57s —— View job Code Review
The feature is well-structured — the backend query logic (using Four things worth addressing before merge (inline comments added):
One minor UI nit on |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
@codex review |
| </div> | ||
| )} | ||
|
|
||
| {hasSequenceCitations && <hr className='my-8 border-t-2 border-gray-200' />} |
There was a problem hiding this comment.
I wonder if it could just be folded into the sections above, with no horizontal rule?
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e710bbee7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const sequenceCitationsPromise = SeqSetCitationClient.create().call('getSequenceCitations', { | ||
| params: { accession }, // Display citations across all accession versions | ||
| }); |
There was a problem hiding this comment.
When SeqSets are disabled, the existing /seqsets routes are hidden/rewritten via seqSetsAreEnabled(), but this sequence page still unconditionally waits for getSequenceCitations before rendering. In seqset-disabled deployments that adds a failing backend request to every sequence detail load, and if the backend route is still enabled it can render Cited in links to pages the website intentionally disables; gate this request, and the matching details.json request used by previews, on the SeqSets feature flag.
Useful? React with 👍 / 👎.
| @Schema(description = "A citation of a SeqSet.") | ||
| data class SeqSetCitation(val source: CitationSource) | ||
|
|
||
| data class SeqSetCitingSequence(val seqSetAccession: String, val sequenceAccession: String) |
There was a problem hiding this comment.
we might have a specific type for loculus accessions and seqSetAccessions in the backend
| } | ||
|
|
||
| fun getSequenceCitations(accession: String, version: Long?): List<SequenceCitation> { | ||
| log.info { "Get sequence citations for accession $accession, version $version" } |
There was a problem hiding this comment.
we should probably not log " version " when version is empty
| // Otherwise, return all citations for the accession, regardless of version, including unversioned | ||
| val accessionQuery: Op<Boolean> = if (version != null) { | ||
| val accessionVersion = AccessionVersion(accession, version) | ||
| SeqSetRecordsTable.accession eq accessionVersion.displayAccessionVersion() |
There was a problem hiding this comment.
maybe you could leave a comment somewhere that accession can be accession and accessionVersion - I wonder if we should actually refactor the table to have an accession and version column instead of having to parse the contents of the accession column
|
|
||
| return SeqSetCitationSourceTable.innerJoin( | ||
| SeqSetToCitationSourceTable, | ||
| ).innerJoin( |
There was a problem hiding this comment.
we discussed that the seqsettorecordstable can actually be dropped and the seqsetrecordstable refactored as:
object SeqSetRecordsTable : Table("seqset_to_records") {
val seqSetRecordId = long("seqset_record_id").autoIncrement()
val accession = varchar("accession", 255)
val type = varchar("type", 255)
val isFocal = bool("is_focal").default(true)
val seqSetId = varchar("seqset_id", 255) references SeqSetsTable.seqSetId
val seqSetVersion = long("seqset_version") references SeqSetsTable.seqSetVersion
override val primaryKey = PrimaryKey(seqSetRecordId, seqSetId, seqSetVersion)
}
4 participants
Summary
Backend Changes
get-sequence-citationsendpoint, which returns citing sources for seqsets that include the sequence entry.accessionfor the sequence entry, withversionas an optional parameter. If only the accession is provided, the endpoint returns citing sources for seqsets which use any version{accession}.*, as well as unversioned{accession}, of the sequence entry. If both anaccessionandversionare provided, the endpoint returns citing sources which reference that exact{accession}.{version}.Website Changes
CitationListcomponent to display the first N citations (in this case, currently three) directly on the sequence details page, and adds a 'view all citations modal', clickable if they exceed this number.Screenshot
PR Checklist
🚀 Preview: Add
previewlabel to enable