Skip to content

feat(website): use segment displayName and collapse sequences container if an organism is muti-segmented#6106

Merged
anna-parker merged 16 commits intomainfrom
segmentDisplayName
Mar 20, 2026
Merged

feat(website): use segment displayName and collapse sequences container if an organism is muti-segmented#6106
anna-parker merged 16 commits intomainfrom
segmentDisplayName

Conversation

@anna-parker
Copy link
Copy Markdown
Contributor

@anna-parker anna-parker commented Mar 10, 2026

resolves #

Screenshot

Use segment displayName in search UI, details page, download UI:

Collapse sequences dropdown:
image

PR Checklist

  • The implemented feature is covered by appropriate, automated tests.
  • Any manual testing that has been done is documented (i.e. what exactly was tested?) -> tested on preview, used in private virus 4 preview

🚀 Preview: https://segmentdisplayname.loculus.org

@anna-parker anna-parker added the preview Triggers a deployment to argocd label Mar 10, 2026
@claude claude bot added website Tasks related to the web application deployment Code changes targetting the deployment infrastructure labels Mar 10, 2026
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Mar 10, 2026

This PR may be related to: #3557 (Improve how we display segment names: main isn't a thing that we should ever show to users)

@anna-parker anna-parker changed the title feat: use segment displayName and collapse sequences container if 3 or more segments in an organism feat(website): use segment displayName and collapse sequences container if 3 or more segments in an organism Mar 10, 2026
@anna-parker anna-parker marked this pull request as ready for review March 10, 2026 11:42
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Mar 10, 2026

Claude finished @anna-parker's task in 4m 28s —— View job


Code Review

  • Read changed files
  • Review code quality
  • Check testing coverage
  • Review Kubernetes templates
  • Provide final feedback

Good PR overall — the displayName plumbing is clean and the collapsed-tab UX for highly-segmented organisms is a real improvement. A few things to address:

Bug: 2-segment tabs still use name not displayName

The collapsed dropdown (SegmentDropdown) correctly uses segment.displayName ?? segment.name, but the per-segment tab labels rendered when segments.length === 2 (i.e., useCollapsedTab is false) still hardcode segmentName.name:

SequencesContainer.tsx line 216:

label={`${segmentName.name} (unaligned)`}

SequencesContainer.tsx line 265:

label={`${segmentName.name} (aligned)`}

These should be segmentName.displayName ?? segmentName.name to be consistent. For any 2-segment organism that has display names configured, the tabs would still show the raw segment identifier. Fix this →

lapisNameToDisplayName not updated

sequenceTypeHelpers.ts line 286 maps lapis names to segmentName (not displayName):

map.set(refData.lapisName, referenceGenomesInfo.isMultiSegmented ? segmentName : undefined);

This function is used in getTableData.ts to populate segment labels in the mutations/insertions tables on the sequence details page. For organisms with display names, those tables would still show L, M, S instead of Large, Medium, Small. Worth fixing here too: referenceGenomesInfo.segmentDisplayNames[segmentName] ?? segmentName. Fix this →

Minor: Unnecessary dependency in useMemo

DownloadForm.tsx line 69:

const referenceSelected = useMemo(() => nucleotideSegmentInfos.length !== 0, [nucleotideSegmentInfos, geneInfos]);

geneInfos is not used inside the memo — it can be removed from the dependency array.

Testing

The new THREE_SEG_SINGLE_REF_REFERENCEGENOMES fixture is used in SequencesContainer.spec.tsx and correctly verifies that display names appear in the dropdown. The toReferenceGenomes tests in referenceGenomes.spec.ts cover the existing schema conversion cases well. One gap: there's no THREE_SEG_SINGLE_REF_REFERENCEGENOMES_SCHEMA variant to verify the schema→info conversion path includes segmentDisplayNames for 3-segment organisms (though MULTI_SEG_MULTI_REF covers the multi-segment display name mapping).

Kubernetes templates

The _merged-reference-genomes.tpl and _common-metadata.tpl changes look correct — displayNames is now extracted from the segment data and propagated through loculus.getNucleotideSegmentNames and loculus.generateWebsiteMetadata, making per-segment field headers use the display name ($segmentDisplayName) rather than the raw segment identifier.

@anna-parker anna-parker changed the title feat(website): use segment displayName and collapse sequences container if 3 or more segments in an organism feat(website): use segment displayName and collapse sequences container if an organism is muti-segmented Mar 10, 2026
@anna-parker anna-parker removed the preview Triggers a deployment to argocd label Mar 13, 2026
@theosanderson
Copy link
Copy Markdown
Member

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a35ff062ef

ℹ️ 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".

@anna-parker anna-parker added the preview Triggers a deployment to argocd label Mar 13, 2026
Copy link
Copy Markdown
Member

@theosanderson theosanderson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@anna-parker anna-parker merged commit 0ca91f3 into main Mar 20, 2026
45 of 46 checks passed
@anna-parker anna-parker deleted the segmentDisplayName branch March 20, 2026 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deployment Code changes targetting the deployment infrastructure preview Triggers a deployment to argocd website Tasks related to the web application

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants