Skip to content

Conversation

@fengelniederhammer
Copy link
Contributor

@fengelniederhammer fengelniederhammer commented Oct 8, 2025

resolves #4999

Change the sequence upload on the edit page and on the "Submit single sequence" page such that it doesn't assign segments anymore. This should happen in the preprocessing pipeline (#4847).

  • This will only properly work once the backend is adapted accordingy. Until then, this PR will likely have failing integration tests.
  • We could in principle extract the segment name / fasta header from the uploaded file. But that requires minor code changes in the website that I left for another PR (currently we only get the sequence of the fasta file, the header is discarded).

If we have original data on the edit page, then the segment name is taken from there. In all other cases, we simply enumerate them ("Segment 1", "Segment 2", etc.). That will also be part of the fasta header when submitting / the segment name when uploading edited data.

Screenshot

CCHF "Submit single sequence" (/cchf/submission/2/submit?inputMode=form)

Screencast.from.2025-10-08.13-23-25.mp4

CCHF Edit page for a sequence entry that only has one segment in its original data:

Screencast.from.2025-10-08.13-28-09.mp4

For EV this is rather boring. Either it has a sequence:
image

Or it doesn't:
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?)
    • "Submit single sequence"
      • works for CCHF - although we don't assign segments there anymore. We can only enumerate them.
      • works for EV
      • works for west nile as before
    • edit page
      • works for west nile as before
      • works for CCHF: segments that are present in the uploaded data remain present (but you can remove the files), it's possible to add additional segments until the maximum (3 in this case)
      • works for EV - the existing segment can be modified as for other single segmented viruses.

🚀 Preview: Add preview label to enable

@fengelniederhammer fengelniederhammer added the preview Triggers a deployment to argocd label Oct 8, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR modifies sequence upload functionality to support multi-pathogen organisms by removing automatic segment assignment and instead allowing dynamic addition of sequence segments. The change shifts segment assignment responsibility to the preprocessing pipeline while providing a more flexible interface for uploading sequences.

Key changes:

  • Replaced fixed segment names with dynamic sequence enumeration ("Segment 1", "Segment 2", etc.)
  • Moved EditableSequences class to its own file with improved multi-segment support
  • Updated test interfaces to use arrays instead of key-value pairs for sequence data

Reviewed Changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
website/src/pages/[organism]/submission/edit/[accession]/[version].astro Updated to pass full reference genome schema instead of just segment names
website/src/components/Edit/EditableSequences.ts New file containing refactored EditableSequences class with dynamic segment support
website/src/components/Edit/SequencesForm.tsx Removed EditableSequences class and updated UI to use dynamic labels
website/src/components/Submission/FormOrUploadWrapper.tsx Updated to use new EditableSequences constructor
integration-tests/tests/pages/submission.page.ts Changed sequence data interface from object to array
Multiple test files Updated to use array-based sequence data instead of key-value pairs

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@fengelniederhammer fengelniederhammer changed the base branch from main to preprocessingSegmentAssignment October 13, 2025 08:21
@fengelniederhammer fengelniederhammer marked this pull request as ready for review October 13, 2025 08:25
@fengelniederhammer
Copy link
Contributor Author

This is now targeted to a feature branch where we can merge with red tests to gather changes there until the integration tests are green again.

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.

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

@anna-parker
Copy link
Contributor

Thanks for getting this started! I’ve started reviewing and wanted to flag something we discussed earlier — we had agreed to use the FASTA header as the sequence name shown on the edit page, since it also needs to be passed to the preprocessing pipeline and used in the user-facing error/warning messages. It looks like we’re shifting from records to a set of sequences here, and I’m wondering if that change might require refactoring back to records in order to properly use the FASTA header. Could you clarify the plan for that?

@fengelniederhammer
Copy link
Contributor Author

Thanks for getting this started! I’ve started reviewing and wanted to flag something we discussed earlier — we had agreed to use the FASTA header as the sequence name shown on the edit page, since it also needs to be passed to the preprocessing pipeline and used in the user-facing error/warning messages. It looks like we’re shifting from records to a set of sequences here, and I’m wondering if that change might require refactoring back to records in order to properly use the FASTA header. Could you clarify the plan for that?

On the website there is not restriction regarding that in those changes. We have a "sequence label" around that we use as the FASTA header when submitting and as the object keys when posting edited data. It's still an object.

The only obstacle is that we currently don't get the FASTA header from a file that a user uploads, since the function there only yields a string which contains the sequence (without the header). But we can change that in a follow up PR. Until then, I implemented it such that it simply enumerates the sequences.

@anna-parker anna-parker force-pushed the 4999-multi-pathogen-support-edit-page branch from eb49c79 to 8aaec9d Compare October 18, 2025 12:06
@anna-parker anna-parker force-pushed the 4999-multi-pathogen-support-edit-page branch from 8aaec9d to 0129a69 Compare October 20, 2025 06:14
@fengelniederhammer fengelniederhammer force-pushed the 4999-multi-pathogen-support-edit-page branch from 45bdaa6 to 520c6cd Compare October 28, 2025 09:20
@fengelniederhammer fengelniederhammer changed the base branch from preprocessingSegmentAssignment to main October 28, 2025 09:33
@fengelniederhammer fengelniederhammer changed the base branch from main to preprocessingSegmentAssignment October 28, 2025 09:34
@fengelniederhammer fengelniederhammer changed the title multi pathogen support edit page feat(website): multi pathogen support: edit page: handle preprocessing-assigned segments Nov 3, 2025
@theosanderson theosanderson removed the preview Triggers a deployment to argocd label Nov 5, 2025
@anna-parker anna-parker added the preview Triggers a deployment to argocd label Nov 6, 2025
@anna-parker
Copy link
Contributor

anna-parker commented Nov 6, 2025

When I delete and then reupload a segment in the EVs I get an error:
Screencast from 06.11.2025 15:04:05.webm - this looks like the same backend error I get here: #4850 but here I do actually upload a sequence

@anna-parker
Copy link
Contributor

I started reviewing but decided it was easiest to start #5382 with my suggestions :-)

@fengelniederhammer
Copy link
Contributor Author

Part of #5382 now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Multi pathogen support: edit page

5 participants