Skip to content

Commit 4451d34

Browse files
fix!(prepro): Make host field curatable for INSDC-ingested sequences (#6499)
## BREAKING CHANGE A prepro function has changed its name for clarity, please change ``` preprocessing: function: validate_host ``` to: ``` preprocessing: function: resolve_host_taxon_id ``` ### Summary For INSDC ingested sequences, we currently skip host validation and just take the `hostTaxonId` as-is, if it's provided. This was done because we did not want to overwrite INSDC-ingested information during preprocessing, as it would be a type of curation. If a hypothetical INSDC sequence has a `hostTaxonId` pointing to organism A, but a scientific name pointing to organism B, our preprocessing would overwrite this so both fields point to organism A. While this may be correct, it would mean that this sequence looks different on our end versus in INSDC. However, we're now running into issues where we can't perform desired curations on INSDC-ingested data, specifically for sequences where a `hostNameScientific` is present but the `hostTaxonId` is missing. This PR aims to correct this. ### PR Checklist - [ ] All necessary documentation has been adapted. - [ ] The implemented feature is covered by appropriate, automated tests. - [x] Any manual testing that has been done is documented (i.e. what exactly was tested?) I signed in as superuser and confirmed I could revise/curate a sequence by editing the host field: <img width="2958" height="692" alt="image" src="https://github.com/user-attachments/assets/f2df1a02-614e-4385-b533-d983151ad65c" /> 🚀 Preview: https://remove-insdc-special-case.loculus.org --------- Co-authored-by: Anna (Anya) Parker <50943381+anna-parker@users.noreply.github.com>
1 parent 52b2947 commit 4451d34

4 files changed

Lines changed: 43 additions & 53 deletions

File tree

kubernetes/loculus/values.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1250,7 +1250,7 @@ defaultOrganismConfig: &defaultOrganismConfig
12501250
noInput: true
12511251
orderOnDetailsPage: 1540
12521252
preprocessing:
1253-
function: validate_host
1253+
function: resolve_host_taxon_id
12541254
inputs:
12551255
host: host
12561256
hostNameScientific: hostNameScientific

preprocessing/nextclade/src/loculus_preprocessing/processing_functions.py

Lines changed: 23 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1589,7 +1589,7 @@ def replace_identifier(values, replacement):
15891589
)
15901590

15911591
@staticmethod
1592-
def validate_host(
1592+
def resolve_host_taxon_id(
15931593
input_data: InputMetadata,
15941594
output_field: str,
15951595
input_fields: list[str],
@@ -1605,23 +1605,6 @@ def validate_host(
16051605
return the tax_id of the most generic taxon (i.e., the one that's closest to
16061606
the root of the taxonomy)
16071607
"""
1608-
# for INSDC-ingested sequences we just return the id INSDC gives us (if any)
1609-
# if we ever want to change this behaviour, we just have to remove this short-circuit,
1610-
# the rest of the function is set up to validate INSDC-ingested data as well
1611-
if args["is_insdc_ingest_group"]:
1612-
tax_id = input_data.get("hostTaxonId")
1613-
if not isinstance(tax_id, str) or not tax_id.isdigit():
1614-
return ProcessingResult(
1615-
datum=None,
1616-
warnings=[],
1617-
errors=[],
1618-
)
1619-
return ProcessingResult(
1620-
datum=tax_id,
1621-
warnings=[],
1622-
errors=[],
1623-
)
1624-
16251608
tax_service = args.get("taxonomy_service_url")
16261609
if not tax_service:
16271610
return missing_taxonomy_service_error(input_fields, output_field)
@@ -1650,10 +1633,10 @@ def validate_host(
16501633

16511634
try:
16521635
response = taxonomy_cache.get_or_fetch(url)
1636+
body = response.json()
16531637
except requests.exceptions.RequestException as e:
16541638
return taxonomy_network_error(unvalidated, "validating", e, input_fields, output_field)
16551639

1656-
body = response.json()
16571640
if response.status_code != requests.codes.ok:
16581641
# an invalid host organism is a warning for INSDC ingested sequences, but an error for everyone else
16591642
message = ProcessingAnnotation.from_fields(
@@ -1663,7 +1646,9 @@ def validate_host(
16631646
message=f"Host validation for '{unvalidated}' failed with code {response.status_code}: {body.get('detail', '')}",
16641647
)
16651648
return ProcessingResult(
1666-
datum=None,
1649+
datum=unvalidated
1650+
if args["is_insdc_ingest_group"] and unvalidated.isdigit()
1651+
else None,
16671652
warnings=[message] if args["is_insdc_ingest_group"] else [],
16681653
errors=[message] if not args["is_insdc_ingest_group"] else [],
16691654
)
@@ -1704,47 +1689,42 @@ def scientific_name_from_id(
17041689
input_fields: list[str],
17051690
args: FunctionArgs,
17061691
) -> ProcessingResult:
1707-
# for INSDC-ingested sequences we just return the name INSDC gives us (if any)
1708-
# if we ever want to change this behaviour, we just have to remove this short-circuit,
1709-
# the rest of the function is set up to validate INSDC-ingested data as well
1710-
if args["is_insdc_ingest_group"]:
1711-
return ProcessingResult(
1712-
datum=input_data.get("hostNameScientific"),
1713-
warnings=[],
1714-
errors=[],
1715-
)
1716-
17171692
tax_service = args.get("taxonomy_service_url")
17181693
if not tax_service:
17191694
return missing_taxonomy_service_error(input_fields, output_field)
17201695

17211696
tax_id: str | None = input_data.get("hostTaxonId")
17221697
if not tax_id:
17231698
return ProcessingResult(
1724-
datum=None,
1699+
datum=input_data.get("hostNameScientific")
1700+
if args["is_insdc_ingest_group"]
1701+
else None,
17251702
warnings=[],
17261703
errors=[],
17271704
)
17281705

17291706
url = f"{tax_service}/taxa/{tax_id}"
17301707
try:
17311708
response = taxonomy_cache.get_or_fetch(url)
1709+
body = response.json()
17321710
except requests.exceptions.RequestException as e:
17331711
return taxonomy_network_error(tax_id, "validating", e, input_fields, output_field)
17341712

1735-
body = response.json()
17361713
if response.status_code != requests.codes.ok:
1714+
message = f"Could not map '{tax_id}' to scientific name. Code {response.status_code}: {body.get('detail', '')}"
1715+
logger.warning(message)
1716+
processing_annotation = ProcessingAnnotation.from_fields(
1717+
input_fields,
1718+
[output_field],
1719+
AnnotationSourceType.METADATA,
1720+
message=message,
1721+
)
17371722
return ProcessingResult(
1738-
datum=None,
1739-
warnings=[],
1740-
errors=[
1741-
ProcessingAnnotation.from_fields(
1742-
input_fields,
1743-
[output_field],
1744-
AnnotationSourceType.METADATA,
1745-
message=f"Internal error: could not map '{tax_id}' to scientific name. Code {response.status_code}: {body.get('detail', '')}",
1746-
)
1747-
],
1723+
datum=input_data.get("hostNameScientific")
1724+
if args["is_insdc_ingest_group"]
1725+
else None,
1726+
warnings=[processing_annotation] if args["is_insdc_ingest_group"] else [],
1727+
errors=[processing_annotation] if not args["is_insdc_ingest_group"] else [],
17481728
)
17491729

17501730
scientific_name = body.get("scientific_name")
@@ -1790,12 +1770,12 @@ def common_name_from_id(
17901770
url = f"{tax_service}/taxa/{tax_id}?find_common_name=true"
17911771
try:
17921772
response = taxonomy_cache.get_or_fetch(url)
1773+
body = response.json()
17931774
except requests.exceptions.RequestException as e:
17941775
return taxonomy_network_error(
17951776
tax_id, "getting common name for", e, input_fields, output_field
17961777
)
17971778

1798-
body = response.json()
17991779
if response.status_code != requests.codes.ok:
18001780
return ProcessingResult(
18011781
datum=None,

preprocessing/nextclade/tests/host_processing_config.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ processing_spec:
1212
hostTaxonId: processed.hostTaxonId
1313
args: *preprocessingTaxonomyTestArgs
1414
hostTaxonId:
15-
function: validate_host
15+
function: resolve_host_taxon_id
1616
inputs:
1717
host: host
1818
hostNameScientific: hostNameScientific

preprocessing/nextclade/tests/test_host_name_validation.py

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,10 @@ def test_host_processing_direct_submission(mock_session: MagicMock) -> None:
7171
assert metadata["hostNameCommon"] == "yellow fever mosquito"
7272
assert result[0].processed_entry.errors == []
7373

74-
# All three fields should have hit the taxonomy service
74+
# Three distinct taxonomy-service URLs are fetched, so no hits in taxonomy_cache:
75+
# 1. resolve_host_taxon_id -> GET /taxa?scientific_name=Aedes+aegypti
76+
# 2. scientific_name_from_id -> GET /taxa/7159
77+
# 3. common_name_from_id -> GET /taxa/7159?find_common_name=true
7578
assert mock_session.get.call_count == 3
7679

7780

@@ -96,14 +99,21 @@ def test_host_processing_insdc(mock_session: MagicMock) -> None:
9699
assert metadata["hostNameCommon"] == "yellow fever mosquito"
97100
assert result[0].processed_entry.errors == []
98101

99-
# For INSDC, only use the taxonomy service for common name
100-
assert mock_session.get.call_count == 1
102+
# Only two URLs are fetched: resolve_host_taxon_id and scientific_name_from_id both build
103+
# GET /taxa/7159, so the second one is served from taxonomy_cache (no extra call):
104+
# 1. resolve_host_taxon_id -> GET /taxa/7159
105+
# 2. scientific_name_from_id -> GET /taxa/7159 -> cache hit, no call
106+
# 3. common_name_from_id -> GET /taxa/7159?find_common_name=true
107+
assert mock_session.get.call_count == 2
101108

102109

103110
@patch.object(processing_functions.taxonomy_cache, "session")
104111
def test_host_processing_invalid_hostname(mock_session: MagicMock) -> None:
105-
"""When hostNameScientific is not found in the taxonomy, hostTaxonId is None,
106-
which causes hostNameScientific and hostNameCommon to also fail."""
112+
"""For an INSDC-ingested sequence whithout a scientific name but with a
113+
taxon id, hostTaxonId should be None but hostNameScientific should be set
114+
to the provided value.
115+
There should also be a warning saying host validation failed
116+
"""
107117
mock_session.get.return_value = make_response(404, {"detail": "not found"})
108118
config = get_config(HOST_PROCESSING_CONFIG, ignore_args=True)
109119

@@ -119,9 +129,9 @@ def test_host_processing_invalid_hostname(mock_session: MagicMock) -> None:
119129
assert metadata["hostNameScientific"] == "not a real species"
120130
assert metadata["hostNameCommon"] is None
121131

122-
# For INSDC, nothing should hit the taxonomy service and no warnings are raised
123-
assert mock_session.get.call_count == 0
124-
assert len(result[0].processed_entry.warnings) == 0
132+
assert mock_session.get.call_count == 1
133+
assert len(result[0].processed_entry.warnings) == 1
134+
assert "Host validation for" in result[0].processed_entry.warnings[0].message
125135
assert result[0].processed_entry.errors == []
126136

127137

0 commit comments

Comments
 (0)