Skip to content

feat(website, config): create a customDisplay for reference with variant information#6133

Open
anna-parker wants to merge 2 commits intomainfrom
customDisplayReference
Open

feat(website, config): create a customDisplay for reference with variant information#6133
anna-parker wants to merge 2 commits intomainfrom
customDisplayReference

Conversation

@anna-parker
Copy link
Contributor

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

This adds an additional custom display, similar to the custom display for length and completeness, but in this case for assigned reference and variant strain, where variant strain is a boolean based on the number of private mutations a sequence has - this is currently in use on the private virus preview and has been enabled for cchf multi-ref for testing purposes.

Screenshot

image

PR Checklist

  • All necessary documentation has been adapted.
  • 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

🚀 Preview: Add preview label to enable

@anna-parker anna-parker changed the title feat(website, config): create a customDisplay for reference with vari… feat(website, config): create a customDisplay for reference with variant information Mar 13, 2026
@anna-parker anna-parker added the preview Triggers a deployment to argocd label Mar 13, 2026
@claude claude bot added website Tasks related to the web application deployment Code changes targetting the deployment infrastructure preprocessing Issues related to the preprocessing component labels Mar 13, 2026
@anna-parker anna-parker marked this pull request as ready for review March 13, 2026 16:16
@claude
Copy link
Contributor

claude bot commented Mar 13, 2026

Claude finished @anna-parker's task in 2m 27s —— View job


Code Review

  • Gather context and read changed files
  • Review code quality and logic
  • Check tests
  • Post review feedback

The overall approach is clean. The customDisplay: variantReference type groups the reference and variant backend fields into a single UI row showing the closest reference optionally annotated with "(variant)". Propagating referenceGenomesInfo down through DataTable → DataTableEntry → DataTableEntryValue is the right design. The preprocessing change (extracting the error message into a variable before both logging and adding it to the annotation) is a nice small improvement.

One real bug to fix:

VariantReferenceComponent iterates over segmentDisplayNames (line 101), but segmentDisplayNames is only populated for segments that have an explicit displayName set in the config (see toReferenceGenomes). For any organism — including all single-segment organisms — where a segment has no displayName, the loop body never executes and the component always renders "N/A". Iterating over segmentReferenceGenomes instead avoids this. See inline comment.

The test file covers the important cases well, but there's no test that would catch the above bug — see inline comment on the test file.

@theosanderson
Copy link
Member

@codex review

Copy link

@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: a3674d37a4

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

@theosanderson
Copy link
Member

@codex review

Copy link

@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: 842956664f

ℹ️ 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 force-pushed the customDisplayReference branch from 87788f6 to 96840fb Compare March 13, 2026 18:58
Copy link
Member

Choose a reason for hiding this comment

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

Personally I don't find this intuitive - if I see

Closest reference: A (variant)

I would assume that the closest reference is a variant of A, rather than that this sequence is a variant. But it's not my area

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a different display you would find more intuitive? Also tagging @rneher as he requested this. Perhaps closest reference should be changed? We could call it L segment lineage: A (variant) would that be clearer?

Copy link
Member

Choose a reason for hiding this comment

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

I don't have a great instinct - but yes, I guess that would be clearer but then I'm not sure if that naming is actually accurate - would this use case be called a lineage in the cases we're interested in?

@anna-parker anna-parker force-pushed the customDisplayReference branch from 3325c51 to b293a42 Compare March 20, 2026 10:37
Base automatically changed from segmentDisplayName to main March 20, 2026 11:36
@anna-parker anna-parker force-pushed the customDisplayReference branch from b293a42 to 6620987 Compare March 20, 2026 13:09
@anna-parker anna-parker removed the preview Triggers a deployment to argocd label Mar 20, 2026
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 preprocessing Issues related to the preprocessing component website Tasks related to the web application

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants