feat(prepro): Add deacon

[maverbiest]

Partially resolves #6758

Summary

This PR incorporates Deacon into the preprocessing pipeline. Deacon is used to check submitted raw reads files for host contamination and rejects files where the proportion of raw reads above a threshold configured via preprocessing.configFile.deacon_max_host_proportion in the values.yaml.

Details

• Memory cap for preprocessing pods is raised to accomodate loading the 3.1G panhuman index into RAM (set it at 8G for now)
• FileIdAndName -> FileIdAndNameAndReadUrl (same as the backend type). This is because prepro now needs to download raw reads from S3 so it needs the URLs
• At startup, preprocessing downloads the panhuman-1.k31w15.idx that's needed for host depletion
• When preprocessing gets a batch of sequences to process, a deacon server is started. The server is shut down again after a batch is processed. This prevents the 3.1G index from sitting in RAM when it's not used. Downside is that it will need to be read from disk for each batch of sequences (100 seqs for most organisms), which takes ~6 seconds
• Deacon filter is run on each (pair of) raw reads file(s), a summary of the run is output as json and loaded in to a DeaconSummary. This information is used for deciding to reject a file or not
• A new test file is added to cover the new functionality (Deacon calls are mocked, however)

Manual testing

I monitored the prepro logs in Grafana to confirm that:

• Index gets fetched once at prepro startup
• Deacon server starts/stops as expected when a batch of unprocessed sequences is received from the backend
• Deacon filter runs

I also started a preview to check that:

• Raw reads files without host sequences are not rejected
• Raw reads files with too many host sequences get rejected
• Raw reads file is attached to submission and downloadable from the sequence details page after release

Here, the first submission has file ERR17072040.fastq.gz, which is just west-nile sequences. The second submission (with the error) has the same file where I added 10000 reads sourced from the human reference genome:

When the sequence is released, the raw reads file is accessible on the sequence details page:

PR Checklist

- [ ] All necessary documentation has been adapted. -> will be added in #6760

• The implemented feature is covered by appropriate, automated tests.
• Any manual testing that has been done is documented (i.e. what exactly was tested?)

🚀 Preview: https://add-deacon.loculus.org

[maverbiest]

@claude review

[claude]

Claude finished @maverbiest's task in 4m 0s —— View job

---

Claude finished reviewing

• Gather context (diff against origin/raw-reads)
• Understand the request
• Review changed files
• Post review feedback

CI is green (preprocessing-tests, which includes ruff + pytest, passed), so this is a design/logic-level review rather than a lint/test-failure review.

Findings

1. Debug-only preview overrides are still in the diff
kubernetes/loculus/values_preview_server.yaml (+21/-1) adds a defaultOrganisms block disabling every organism except west-nile and bumps preprocessing.limits.memory from 3Gi to 8Gi. This matches your own 9f90c06 TO REVERT: ... commit — flagging so it isn't accidentally left in when this PR is merged into raw-reads/main. Might be worth adding this explicitly to the PR checklist.

2. Possible race between starting the Deacon server and using it
start_deacon_server() (file_processing_functions.py:177-185) does a non-blocking subprocess.Popen(...) and returns immediately; prepro.py:752-756 calls it and then immediately calls process_all(...), which can invoke deacon --use-server filter right away. If the server takes any nonzero time to bind before it's ready to accept --use-server calls, the first filter call in a batch could fail with a connection error. Since stdout/stderr of the server process are both sent to DEVNULL, such a startup failure would also be silent and hard to diagnose. Worth double-checking whether deacon's client retries/blocks until the server socket exists, or whether an explicit readiness check/wait is needed here.

3. Only HTTPError is special-cased around the download step; Deacon-filter failures aren't caught at all
In validate_raw_reads_submission (file_processing_functions.py:112-136), only requests.HTTPError from download_file gets a friendly per-file ProcessingAnnotation. Other request failures (timeout, connection reset, etc.) and any failure from run_deacon_filter (which raises a bare Exception on non-zero exit, line 173) aren't caught here — they'll propagate up and be handled by the generic except Exception in process_all's per-entry loop, which just marks the whole entry as failed with a generic message rather than the specific file-related annotation the rest of this function produces. If that's acceptable (i.e., "this should basically never happen") it's fine, but worth confirming it's intentional rather than an oversight, since it's an inconsistent error-handling story within the same function.

4. Duplicated “magic” Deacon index identifiers
config.py's DEACON_INDEX = "panhuman-1.k31w15.idx" and file_processing_functions.py's run_deacon_fetch's hardcoded "panhuman-1" argument are two independent magic strings that must stay in sync (the .k31w15.idx suffix encodes the k/w params deacon chooses for that index name). Not a bug today, but if the index profile ever changes, it's easy for these to silently drift out of sync. Consider deriving one from the other, or at least a comment noting the coupling.

5. Minor: raw-reads-pipeline coverage moved out of test_nextclade_preprocessing.py without a replacement full-pipeline check
The removed cases in test_nextclade_preprocessing.py (e.g. "with file") verified that files survive end-to-end into ProcessedData. The new test_file_processing.py thoroughly covers validate_raw_reads_submission/process_submitted_files in isolation (nice test suite, good use of the mock_deacon fixture), but nothing currently exercises that a validated raw-reads file mapping actually flows through process_single/process_all into the final processed output. Probably fine given the isolated unit tests, just flagging the coverage gap.

Nits

• run_deacon_filter raises a bare Exception(message) (file_processing_functions.py:173) while run_deacon_fetch raises RuntimeError for the analogous failure (line 149) — consider using the same exception type for consistency.
• The PR description's TODO list ("Implement running Deacon in server mode…", "Implement Deacon server start -> process batch -> Deacon server stop…") both look already implemented by the latest commits (4618873, 319db4d) — might be worth updating the PR body/checklist so reviewers know what's actually still outstanding (currently only "Test that contaminated file gets rejected" seems to remain, and that is covered by test_host_contamination_exceeds_threshold).

Overall this is solid, well-tested work — the new test_file_processing.py suite in particular is thorough (paired reads, extension validation, threshold edge cases, missing-URL handling). The main things worth a second look before merging into raw-reads are the preview-only k8s overrides (item 1) and the server-readiness question (item 2).

[corneliusroemer]

This would disable all preview organisms but west nile, which we don't want.

So needs to be reverted before going on main

[anna-parker]

we probably need to configure two thresholds.

1. Which percentage of reads are flagged
2. How many bp are flagged

[anna-parker]

its also important to know how many bp were flagged

[anna-parker]

It might actually be good to create a little deacon index and example raw reads for testing. We also do that with nextclade as I noticed if we dont actually run nextclade in tests we miss things