diff --git a/apps/prep/tests/test_pipeline.py b/apps/prep/tests/test_pipeline.py index 8b992e87b..896364d85 100644 --- a/apps/prep/tests/test_pipeline.py +++ b/apps/prep/tests/test_pipeline.py @@ -309,7 +309,7 @@ async def fake_create(*args, **kwargs): normalized = ctx.output_dir / "input.normalized.fasta" assert seen_inputs["embed"] == str(normalized) assert seen_inputs["annotate"] == str(normalized) - assert ">P12345" in normalized.read_text() + assert normalized.read_text() == ">P12345\nMAAAAAA\n" async def test_embed_failure_with_connection_refused_is_classified_as_biocentral_unavailable( diff --git a/apps/protspace/docs/annotations.md b/apps/protspace/docs/annotations.md index 2bf6e60f5..8474cb0f0 100644 --- a/apps/protspace/docs/annotations.md +++ b/apps/protspace/docs/annotations.md @@ -192,12 +192,12 @@ Structure-based domain annotations from [TED (The Encyclopedia of Domains)](http Per-protein predictions from the [Biocentral API](https://biocentral.rostlab.org/) using pre-trained models. Requires protein sequences (fetched automatically from UniProt). -| Name | Model | Description | -| -------------------------------- | ------------------------------------ | ------------------------------------------ | -| `predicted_subcellular_location` | LightAttention | 10-class subcellular localization | -| `predicted_membrane` | LightAttention | Membrane / Soluble | -| `predicted_signal_peptide` | TMbed | True / False (derived from topology) | -| `predicted_transmembrane` | TMbed | none / alpha-helical / beta-barrel | +| Name | Model | Description | +| -------------------------------- | ------------------------------------ | ----------------------------------------------- | +| `predicted_subcellular_location` | LightAttention | 10-class subcellular localization | +| `predicted_membrane` | LightAttention | Membrane / Soluble | +| `predicted_signal_peptide` | TMbed | True / False (derived from topology) | +| `predicted_transmembrane` | TMbed | non-transmembrane / alpha-helical / beta-barrel | **Data source**: Batch predictions via Biocentral API (`api.predict()`). TMbed provides per-residue topology labels (`H`=TM helix, `B`=TM beta strand, `S`=signal peptide); signal peptide and transmembrane type are summarized from these labels. diff --git a/apps/protspace/src/protspace/cli/annotate.py b/apps/protspace/src/protspace/cli/annotate.py index e2e99fecb..b1e060d6c 100644 --- a/apps/protspace/src/protspace/cli/annotate.py +++ b/apps/protspace/src/protspace/cli/annotate.py @@ -61,7 +61,9 @@ def annotate( from protspace.data.loaders.h5 import EMBEDDING_EXTENSIONS # Extract identifiers from input - if is_fasta_file(input): + input_is_fasta = is_fasta_file(input) + sequences = None + if input_is_fasta: from protspace.data.loaders.query import extract_identifiers_from_fasta headers = extract_identifiers_from_fasta(input) @@ -82,10 +84,10 @@ def annotate( logger.info(f"Found {len(headers)} protein identifiers") # Resolve annotation names + from protspace.data.annotations.configuration import AnnotationConfiguration + annotations_list = None if annotations: - from protspace.data.annotations.configuration import AnnotationConfiguration - names = [] for item in annotations: for part in item.split(","): @@ -95,11 +97,23 @@ def annotate( if names: annotations_list = AnnotationConfiguration(names).user_annotations + if input_is_fasta: + config = AnnotationConfiguration(annotations_list) + if config.interpro_annotations or config.biocentral_annotations: + from protspace.data.io.fasta import parse_fasta + from protspace.data.loaders.h5 import parse_identifier + + sequences = { + parse_identifier(header): sequence + for header, sequence in parse_fasta(input).items() + } + # Fetch annotations df = ProteinAnnotationManager( headers=headers, annotations=annotations_list, output_path=None, + sequences=sequences, ).to_pd() if not scores: diff --git a/apps/protspace/src/protspace/data/annotations/manager.py b/apps/protspace/src/protspace/data/annotations/manager.py index 26c3dbc75..94268c0bd 100644 --- a/apps/protspace/src/protspace/data/annotations/manager.py +++ b/apps/protspace/src/protspace/data/annotations/manager.py @@ -241,16 +241,20 @@ def _fetch_taxonomy( return {} def _build_sequence_map( - self, uniprot_annotations: list[ProteinAnnotations] + self, + uniprot_annotations: list[ProteinAnnotations], + *, + prefer_uniprot: bool = False, ) -> dict[str, str]: """Build a mapping from headers to sequences. - Merges local sequences (from FASTA, priority) with UniProt results (fallback). + Local FASTA sequences take priority by default. InterPro can request UniProt + priority because its precomputed matches are keyed by canonical sequence hash. """ sequences = dict(self.sequences) if self.sequences else {} for protein in uniprot_annotations: seq = protein.annotations.get("sequence", "") - if seq and protein.identifier not in sequences: + if seq and (prefer_uniprot or protein.identifier not in sequences): sequences[protein.identifier] = seq return sequences @@ -262,7 +266,9 @@ def _fetch_interpro( return [] try: - sequences = self._build_sequence_map(uniprot_annotations) + sequences = self._build_sequence_map( + uniprot_annotations, prefer_uniprot=True + ) retriever = InterProRetriever( headers=self.headers, diff --git a/apps/protspace/src/protspace/data/annotations/retrievers/biocentral_retriever.py b/apps/protspace/src/protspace/data/annotations/retrievers/biocentral_retriever.py index 0946eaa4f..3b3d99a93 100644 --- a/apps/protspace/src/protspace/data/annotations/retrievers/biocentral_retriever.py +++ b/apps/protspace/src/protspace/data/annotations/retrievers/biocentral_retriever.py @@ -1,7 +1,6 @@ """Biocentral API prediction retriever for per-protein annotations.""" import logging -import re import warnings from protspace.data.annotations.retrievers.base_retriever import BaseAnnotationRetriever @@ -24,6 +23,7 @@ } _BATCH_SIZE = 1000 +_TMBED_TOPOLOGY_LABELS = frozenset("BbHhSio.") class BiocentralPredictionRetriever(BaseAnnotationRetriever): @@ -202,28 +202,42 @@ def _extract_signal_peptide(predictions: list) -> str: TMbed labels: S = signal peptide, H/h = TM helix, B/b = TM beta, i/o = non-TM """ - for pred in predictions: - if pred.model_name == "TMbed": - topology = str(pred.value) if pred.value else "" - return "True" if "S" in topology else "False" - return "" + topology = BiocentralPredictionRetriever._extract_tmbed_topology(predictions) + if topology is None: + return "" + return "True" if "S" in topology else "False" @staticmethod def _extract_transmembrane(predictions: list) -> str: """Derive transmembrane type from TMbed per-residue output. - Returns: 'alpha-helical', 'beta-barrel', or 'none' + Returns: 'alpha-helical', 'beta-barrel', or 'non-transmembrane' """ + topology = BiocentralPredictionRetriever._extract_tmbed_topology(predictions) + if topology is None: + return "" + + has_helix = "H" in topology or "h" in topology + has_beta = "B" in topology or "b" in topology + if has_helix and has_beta: + return "alpha-helical;beta-barrel" + elif has_helix: + return "alpha-helical" + elif has_beta: + return "beta-barrel" + return "non-transmembrane" + + @staticmethod + def _extract_tmbed_topology(predictions: list) -> str | None: + """Return a supported TMbed topology, or ``None`` when unavailable.""" for pred in predictions: if pred.model_name == "TMbed": - topology = str(pred.value) if pred.value else "" - has_helix = bool(re.search(r"[Hh]", topology)) - has_beta = bool(re.search(r"[Bb]", topology)) - if has_helix and has_beta: - return "alpha-helical;beta-barrel" - elif has_helix: - return "alpha-helical" - elif has_beta: - return "beta-barrel" - return "none" - return "" + value = pred.value + if ( + not isinstance(value, str) + or not value + or not set(value) <= _TMBED_TOPOLOGY_LABELS + ): + return None + return value + return None diff --git a/apps/protspace/tests/test_annotate_cli.py b/apps/protspace/tests/test_annotate_cli.py new file mode 100644 index 000000000..598dadf80 --- /dev/null +++ b/apps/protspace/tests/test_annotate_cli.py @@ -0,0 +1,88 @@ +"""Tests for the standalone ``protspace annotate`` command.""" + +import pandas as pd +from typer.testing import CliRunner + +from protspace.cli.app import app + + +def test_fasta_sequences_are_passed_to_annotation_manager(tmp_path, monkeypatch): + """FASTA-only proteins must reach sequence-backed annotation sources.""" + import protspace.data.annotations.manager as manager_module + + fasta = tmp_path / "input.fasta" + fasta.write_text( + ">sp|P12345|KNOWN Known protein\nMKV\n>custom-protein description\nAAAG\n" + ) + captured: dict[str, object] = {} + + class FakeManager: + def __init__(self, *args, **kwargs): + captured.update(kwargs) + + def to_pd(self): + return pd.DataFrame({"identifier": captured["headers"]}) + + monkeypatch.setattr(manager_module, "ProteinAnnotationManager", FakeManager) + + output = tmp_path / "annotations.parquet" + result = CliRunner().invoke( + app, + [ + "annotate", + "-i", + str(fasta), + "-a", + "biocentral", + "-o", + str(output), + ], + ) + + assert result.exit_code == 0, result.output + assert captured["headers"] == ["P12345", "custom-protein"] + assert captured["sequences"] == { + "P12345": "MKV", + "custom-protein": "AAAG", + } + + +def test_fasta_is_not_parsed_for_uniprot_only_annotations(tmp_path, monkeypatch): + """Sequence-free sources must not materialize an unused FASTA sequence map.""" + import protspace.data.annotations.manager as manager_module + import protspace.data.io.fasta as fasta_module + + fasta = tmp_path / "input.fasta" + fasta.write_text(">sp|P12345|KNOWN Known protein\nMKV\n") + captured: dict[str, object] = {} + + class FakeManager: + def __init__(self, *args, **kwargs): + captured.update(kwargs) + + def to_pd(self): + return pd.DataFrame({"identifier": captured["headers"]}) + + def fail_if_parsed(*args, **kwargs): + raise AssertionError("parse_fasta must not run for UniProt-only annotations") + + monkeypatch.setattr(manager_module, "ProteinAnnotationManager", FakeManager) + monkeypatch.setattr(fasta_module, "parse_fasta", fail_if_parsed) + + output = tmp_path / "annotations.parquet" + result = CliRunner().invoke( + app, + [ + "annotate", + "-i", + str(fasta), + "-a", + "uniprot", + "-o", + str(output), + ], + ) + + assert result.exit_code == 0, result.output + assert captured["headers"] == ["P12345"] + assert captured["sequences"] is None diff --git a/apps/protspace/tests/test_annotation_manager.py b/apps/protspace/tests/test_annotation_manager.py index d7dc308bf..749d14253 100644 --- a/apps/protspace/tests/test_annotation_manager.py +++ b/apps/protspace/tests/test_annotation_manager.py @@ -439,6 +439,38 @@ def test_transform_protein_families_with_semicolon(self): class TestIntegration: """Integration tests for complete workflows.""" + @patch("src.protspace.data.annotations.manager.BiocentralPredictionRetriever") + @patch("src.protspace.data.annotations.manager.InterProRetriever") + def test_sequence_precedence_is_source_specific( + self, mock_interpro_retriever, mock_biocentral_retriever + ): + """InterPro hashes canonical sequences; Biocentral predicts submitted ones.""" + manager = ProteinAnnotationManager( + headers=["P12345", "custom-protein"], + annotations=["pfam", "predicted_transmembrane"], + sequences={"P12345": "LOCAL", "custom-protein": "CUSTOM"}, + ) + uniprot_annotations = [ + ProteinAnnotations( + identifier="P12345", annotations={"sequence": "CANONICAL"} + ), + ProteinAnnotations(identifier="custom-protein", annotations={}), + ] + mock_interpro_retriever.return_value.fetch_annotations.return_value = [] + mock_biocentral_retriever.return_value.fetch_annotations.return_value = [] + + manager._fetch_interpro(uniprot_annotations, []) + manager._fetch_biocentral(uniprot_annotations, []) + + assert mock_interpro_retriever.call_args.kwargs["sequences"] == { + "P12345": "CANONICAL", + "custom-protein": "CUSTOM", + } + assert mock_biocentral_retriever.call_args.kwargs["sequences"] == { + "P12345": "LOCAL", + "custom-protein": "CUSTOM", + } + @patch("src.protspace.data.annotations.manager.TaxonomyRetriever") @patch("src.protspace.data.annotations.manager.UniProtRetriever") def test_to_pd_complete_workflow( diff --git a/apps/protspace/tests/test_biocentral_retriever.py b/apps/protspace/tests/test_biocentral_retriever.py index 82d91754c..8487f78ee 100644 --- a/apps/protspace/tests/test_biocentral_retriever.py +++ b/apps/protspace/tests/test_biocentral_retriever.py @@ -2,6 +2,9 @@ from unittest.mock import MagicMock, patch +import pytest +from biocentral_api._generated import Prediction + from src.protspace.data.annotations.retrievers.biocentral_retriever import ( BIOCENTRAL_ANNOTATIONS, BiocentralPredictionRetriever, @@ -16,6 +19,16 @@ def _make_prediction(model_name, value): return pred +def _make_real_tmbed_prediction(value): + """Build the generated model returned by biocentral-api.""" + return Prediction( + model_name="TMbed", + prediction_name="topology", + protocol="per_residue", + value=value, + ) + + class TestBiocentralConstants: def test_biocentral_annotations(self): expected = [ @@ -45,6 +58,16 @@ def test_no_tmbed_prediction(self): result = BiocentralPredictionRetriever._extract_signal_peptide(preds) assert result == "" + def test_none_payload_is_missing(self): + preds = [_make_real_tmbed_prediction(None)] + result = BiocentralPredictionRetriever._extract_signal_peptide(preds) + assert result == "" + + def test_empty_payload_is_missing(self): + preds = [_make_real_tmbed_prediction("")] + result = BiocentralPredictionRetriever._extract_signal_peptide(preds) + assert result == "" + class TestTransmembraneExtraction: """Test TMbed → transmembrane type derivation.""" @@ -67,7 +90,22 @@ def test_both_types(self): def test_no_transmembrane(self): preds = [_make_prediction("TMbed", "oooooooooiiiiiiiiiii")] result = BiocentralPredictionRetriever._extract_transmembrane(preds) - assert result == "none" + assert result == "non-transmembrane" + + def test_dot_label_is_non_transmembrane(self): + preds = [_make_real_tmbed_prediction("........")] + result = BiocentralPredictionRetriever._extract_transmembrane(preds) + assert result == "non-transmembrane" + + def test_none_payload_is_missing(self): + preds = [_make_real_tmbed_prediction(None)] + result = BiocentralPredictionRetriever._extract_transmembrane(preds) + assert result == "" + + def test_empty_payload_is_missing(self): + preds = [_make_real_tmbed_prediction("")] + result = BiocentralPredictionRetriever._extract_transmembrane(preds) + assert result == "" def test_lowercase_labels(self): """TMbed uses lowercase h/b for non-TM side of helix/strand.""" @@ -81,6 +119,19 @@ def test_no_tmbed_prediction(self): assert result == "" +class TestTmbedPayloadValidation: + @pytest.mark.parametrize( + "value", + [0, [], {}, " ", b"abc", "garbage"], + ids=["zero", "list", "dict", "blank", "bytes", "unsupported-labels"], + ) + def test_malformed_payload_is_missing_for_derived_annotations(self, value): + preds = [_make_real_tmbed_prediction(value)] + + assert BiocentralPredictionRetriever._extract_signal_peptide(preds) == "" + assert BiocentralPredictionRetriever._extract_transmembrane(preds) == "" + + class TestPerSequenceExtraction: """Test per-sequence prediction extraction.""" diff --git a/docs/guide/annotations.md b/docs/guide/annotations.md index c32ce7097..c5778ca47 100644 --- a/docs/guide/annotations.md +++ b/docs/guide/annotations.md @@ -58,9 +58,9 @@ LightAttention is a lightweight neural network that uses softmax-weighted aggreg **Transmembrane** · ⚡ Predicted -Transmembrane type (none / alpha-helical / beta-barrel) predicted by TMbed. +Transmembrane type (non-transmembrane / alpha-helical / beta-barrel) predicted by TMbed. -From the same TMbed per-residue topology (H = transmembrane helix, B = transmembrane beta strand, S = signal peptide), ProtSpace summarizes the membrane-spanning segments into a single protein-level category. Values are `alpha-helical` when transmembrane helices (H) are predicted, `beta-barrel` when transmembrane beta strands (B) are predicted, and `none` when neither is present. See [Bernhofer & Rost, BMC Bioinformatics 2022](https://doi.org/10.1186/s12859-022-04873-x). +From the same TMbed per-residue topology (H = transmembrane helix, B = transmembrane beta strand, S = signal peptide), ProtSpace summarizes the membrane-spanning segments into protein-level categories. Values are `alpha-helical` when transmembrane helices (H) are predicted, `beta-barrel` when transmembrane beta strands (B) are predicted, and `non-transmembrane` when neither is present. A protein with both segment types carries both the `alpha-helical` and `beta-barrel` categories. See [Bernhofer & Rost, BMC Bioinformatics 2022](https://doi.org/10.1186/s12859-022-04873-x). ## UniProt diff --git a/docs/scripts/annotation-details.ts b/docs/scripts/annotation-details.ts index dc2aff2b4..df972ea0a 100644 --- a/docs/scripts/annotation-details.ts +++ b/docs/scripts/annotation-details.ts @@ -57,7 +57,7 @@ export const ANNOTATION_DETAILS: Record = { }, predicted_transmembrane: { detailsMarkdown: - 'From the same TMbed per-residue topology (H = transmembrane helix, B = transmembrane beta strand, S = signal peptide), ProtSpace summarizes the membrane-spanning segments into a single protein-level category. Values are `alpha-helical` when transmembrane helices (H) are predicted, `beta-barrel` when transmembrane beta strands (B) are predicted, and `none` when neither is present. See [Bernhofer & Rost, BMC Bioinformatics 2022](https://doi.org/10.1186/s12859-022-04873-x).', + 'From the same TMbed per-residue topology (H = transmembrane helix, B = transmembrane beta strand, S = signal peptide), ProtSpace summarizes the membrane-spanning segments into protein-level categories. Values are `alpha-helical` when transmembrane helices (H) are predicted, `beta-barrel` when transmembrane beta strands (B) are predicted, and `non-transmembrane` when neither is present. A protein with both segment types carries both the `alpha-helical` and `beta-barrel` categories. See [Bernhofer & Rost, BMC Bioinformatics 2022](https://doi.org/10.1186/s12859-022-04873-x).', sourceUrl: 'https://doi.org/10.1186/s12859-022-04873-x', }, diff --git a/openspec/changes/fix-biocentral-transmembrane-sentinel/.openspec.yaml b/openspec/changes/fix-biocentral-transmembrane-sentinel/.openspec.yaml new file mode 100644 index 000000000..5849c2dbf --- /dev/null +++ b/openspec/changes/fix-biocentral-transmembrane-sentinel/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-08-01 diff --git a/openspec/changes/fix-biocentral-transmembrane-sentinel/README.md b/openspec/changes/fix-biocentral-transmembrane-sentinel/README.md new file mode 100644 index 000000000..5f60931b0 --- /dev/null +++ b/openspec/changes/fix-biocentral-transmembrane-sentinel/README.md @@ -0,0 +1,3 @@ +# fix-biocentral-transmembrane-sentinel + +Prevent valid no-transmembrane predictions from being rendered as missing. diff --git a/openspec/changes/fix-biocentral-transmembrane-sentinel/design.md b/openspec/changes/fix-biocentral-transmembrane-sentinel/design.md new file mode 100644 index 000000000..3481c7613 --- /dev/null +++ b/openspec/changes/fix-biocentral-transmembrane-sentinel/design.md @@ -0,0 +1,121 @@ +## Context + +The Biocentral TMbed adapter currently serializes a valid negative prediction as the +categorical string `none`. The web bundle reader applies a shared missing-value policy +to all categorical annotations, and that policy intentionally treats `none` +case-insensitively as absent data. Consequently, a successfully computed negative +TMbed prediction is displayed as `N/A`, making it indistinguishable from proteins for +which no prediction was produced. + +This behavior crosses the Python annotation producer, the parquet bundle contract, and +the TypeScript visualization consumer. The fix must preserve the generic importer's +established missing-value semantics while making newly produced TMbed data unambiguous. + +## Goals / Non-Goals + +**Goals:** + +- Preserve a successful TMbed negative prediction as a visible categorical value in + the web application. +- Keep genuinely absent Biocentral predictions represented as missing values. +- Keep FASTA-provided sequences available to sequence-backed annotation sources even + when UniProt cannot resolve their identifiers. +- Exercise the producer value through the generated Python-to-TypeScript bundle + contract so future sentinel collisions fail in CI. +- Keep user-facing annotation documentation aligned with the emitted vocabulary. + +**Non-Goals:** + +- Changing the generic web missing-value token set. +- Rewriting or migrating already generated `.parquetbundle` files. +- Changing TMbed topology interpretation for alpha helices, beta barrels, or mixed + predictions. +- Adding contextual bundle-reader rules for one annotation name. + +## Decisions + +### Emit `non-transmembrane` for a successful negative TMbed prediction + +The Biocentral adapter will return `non-transmembrane` when a TMbed result contains +neither an alpha-helical nor a beta-barrel segment. The phrase is explicit, is not in +the consumer's missing-value token set, and does not imply that the protein is soluble +or intracellular. + +Alternatives considered: + +- Keep `none` and remove it from the generic missing-value set. This would change the + interpretation of arbitrary imported datasets and turn commonly used missing + sentinels into visible categories. +- Translate `none` only when reading `predicted_transmembrane`. This would add + domain-specific repair logic to the generic bundle consumer and would still leave + the on-disk producer contract ambiguous. +- Use `soluble`. This is biologically narrower than "no predicted transmembrane + segment" and can mislabel secreted or otherwise non-membrane proteins. + +### Preserve empty-string output for an unavailable or malformed TMbed result + +The adapter will return an empty string when Biocentral provides no TMbed prediction or +provides a TMbed prediction object whose optional `value` payload is `None`, empty, +non-string, blank, or contains unsupported labels. A usable topology is a string made +only from TMbed's `B`, `b`, `H`, `h`, `S`, `i`, `o`, and `.` labels. The payload guard +runs before topology labels are scanned. This retains the distinction at the producer: +a valid topology with no membrane segment is `non-transmembrane`, while an unavailable +or malformed topology remains missing. + +Both `predicted_signal_peptide` and `predicted_transmembrane` derive from that same +optional topology payload. A shared extractor therefore owns the missing-payload check +before either annotation interprets the topology, preventing one annotation from +inventing a completed negative result when the other reports missing data. + +### Pass FASTA sequences to the sources that consume them + +When a requested source consumes sequences, the standalone `annotate` command will +parse the FASTA into a canonical identifier-to-sequence map and pass it to +`ProteinAnnotationManager`. UniProt-only and taxonomy-only requests will retain the +streaming identifier scan without materializing the full sequence map. HDF5 inputs +continue to omit a local sequence map and retain their existing UniProt-fallback +behavior. + +Sequence precedence is source-specific. Biocentral predicts the sequence supplied in +the FASTA, using UniProt only as a fallback for identifiers with no local sequence. +InterPro indexes precomputed matches by sequence hash, so it uses UniProt's canonical +sequence when available and falls back to the FASTA sequence for identifiers UniProt +cannot resolve. This keeps FASTA-only identifiers annotatable without replacing valid +canonical InterPro matches when the submitted sequence differs. + +The hosted prep service already normalizes FASTA headers and supplies that normalized +file to `protspace annotate`; regression coverage pins that normalization preserves the +sequence as well as the canonical identifier. + +### Cover both the adapter and the cross-language seam + +Focused Python regressions will construct the real Biocentral `Prediction` model and +assert the adapter vocabulary for completed and absent payloads. The generated bundle +contract will derive its negative transmembrane and missing TMbed-derived fixture values +from the real adapter and assert that TypeScript preserves the former as a category and +the latter as `N/A`. This avoids duplicated hand-written constants that could allow +producer and contract fixtures to drift apart. + +## Risks / Trade-offs + +- [Existing bundles still contain ambiguous `none` values] → Document that the fix + applies to newly generated annotations; users must regenerate an affected bundle to + recover the distinction. +- [Downstream consumers may expect the old literal] → Treat the value as a categorical + contract correction and document the new vocabulary in both CLI and web metadata. +- [Contract fixture imports more producer code] → Use a minimal synthetic TMbed + prediction and retain the real bundle CLI as the serialization boundary. +- [FASTA and UniProt sequences differ] → Keep the submitted sequence authoritative + for Biocentral predictions and the canonical sequence authoritative for InterPro's + precomputed hash lookup, with the other source as fallback in each case. + +## Migration Plan + +Deploy the producer vocabulary and web documentation together. No storage migration is +performed. Newly annotated bundles will carry `non-transmembrane`; existing bundles can +be regenerated with the updated CLI. Rollback consists of reverting the producer value +and its associated tests/documentation. + +## Open Questions + +None. diff --git a/openspec/changes/fix-biocentral-transmembrane-sentinel/proposal.md b/openspec/changes/fix-biocentral-transmembrane-sentinel/proposal.md new file mode 100644 index 000000000..546adf648 --- /dev/null +++ b/openspec/changes/fix-biocentral-transmembrane-sentinel/proposal.md @@ -0,0 +1,40 @@ +## Why + +Biocentral emits a valid protein-level `predicted_transmembrane` category named +`none`, but the web reader reserves `none` as a missing-value token. As a result, +proteins with a successful “no transmembrane segment” prediction are displayed as +N/A and cannot be distinguished from proteins whose prediction is actually absent. + +## What Changes + +- Encode successful TMbed predictions with no membrane-spanning segment using a + non-reserved categorical label. +- Preserve absent or malformed Biocentral predictions, including TMbed prediction + objects whose optional payload is `None`, empty, non-string, or contains unsupported + topology labels, as missing values for both derived TMbed annotations. +- Preserve FASTA sequences through the standalone `annotate` command when a requested + source needs them, using source-appropriate precedence when UniProt also supplies a + canonical sequence. +- Add regression coverage across the Python annotation producer and TypeScript + bundle consumer boundary. +- Update generated annotation documentation to describe the corrected category. + +## Capabilities + +### New Capabilities + +- `annotation-input`: Require standalone FASTA annotation inputs to retain their + sequences for sequence-backed annotation sources. + +### Modified Capabilities + +- `bundle-format-contract`: Require producer-emitted categorical values to remain + distinguishable from the consumer's missing-value sentinel set. + +## Impact + +- Python FASTA annotation input and TMbed extraction in `apps/protspace`. +- The hosted prep pipeline's normalized-FASTA handoff to `protspace annotate`. +- The generated cross-language bundle fixture and contract assertions. +- Annotation metadata/documentation describing `predicted_transmembrane` values. +- No dependency, API, or bundle-layout changes. diff --git a/openspec/changes/fix-biocentral-transmembrane-sentinel/specs/annotation-input/spec.md b/openspec/changes/fix-biocentral-transmembrane-sentinel/specs/annotation-input/spec.md new file mode 100644 index 000000000..b87929169 --- /dev/null +++ b/openspec/changes/fix-biocentral-transmembrane-sentinel/specs/annotation-input/spec.md @@ -0,0 +1,28 @@ +## ADDED Requirements + +### Requirement: FASTA annotation inputs retain their sequences + +The standalone annotation producer SHALL pass sequences supplied in a FASTA input to +sequence-backed annotation sources using the same canonical identifiers as its output. + +#### Scenario: A FASTA identifier is not resolved by UniProt + +- **WHEN** `protspace annotate` receives a FASTA entry whose identifier UniProt cannot + resolve +- **THEN** the entry's FASTA sequence remains available to Biocentral and InterPro +- **AND** the hosted prep pipeline preserves that sequence when it passes its normalized + FASTA to `protspace annotate` + +#### Scenario: A FASTA sequence differs from the UniProt canonical sequence + +- **WHEN** `protspace annotate` receives a FASTA sequence for an identifier that UniProt + resolves to a different canonical sequence +- **THEN** Biocentral receives the FASTA sequence it is being asked to predict +- **AND** InterPro receives the UniProt canonical sequence used by its precomputed + sequence-hash index + +#### Scenario: Requested annotations do not consume sequences + +- **WHEN** `protspace annotate` receives a FASTA input and the requested annotation + sources are UniProt-only or taxonomy-only +- **THEN** the command extracts identifiers without materializing the FASTA sequences diff --git a/openspec/changes/fix-biocentral-transmembrane-sentinel/specs/bundle-format-contract/spec.md b/openspec/changes/fix-biocentral-transmembrane-sentinel/specs/bundle-format-contract/spec.md new file mode 100644 index 000000000..6f7ea8c99 --- /dev/null +++ b/openspec/changes/fix-biocentral-transmembrane-sentinel/specs/bundle-format-contract/spec.md @@ -0,0 +1,32 @@ +## ADDED Requirements + +### Requirement: Produced annotation categories do not collide with missing-value sentinels + +The Python annotation producer SHALL serialize a completed categorical prediction using +a value that the TypeScript bundle consumer preserves as a category rather than +normalizing to missing data. + +#### Scenario: Negative TMbed prediction crosses the bundle boundary + +- **WHEN** Biocentral returns a completed TMbed prediction containing neither an + alpha-helical nor a beta-barrel transmembrane segment +- **THEN** the producer emits `non-transmembrane` +- **AND** the TypeScript visualization data exposes `non-transmembrane` as a categorical + value rather than `N/A` + +#### Scenario: Missing TMbed prediction remains missing + +- **WHEN** Biocentral returns no TMbed prediction for a protein +- **THEN** the producer emits the established missing representation +- **AND** the TypeScript visualization data does not invent a transmembrane category + +#### Scenario: Empty or malformed TMbed payload remains missing + +- **WHEN** Biocentral returns a TMbed prediction object whose optional `value` payload + is `None`, empty, non-string, blank, or contains unsupported topology labels +- **THEN** the producer emits the established missing representation before scanning + topology labels +- **AND** the TypeScript visualization data exposes the protein as `N/A` rather than + `non-transmembrane` +- **AND** the derived signal-peptide annotation is also exposed as `N/A` rather than + `False` diff --git a/openspec/changes/fix-biocentral-transmembrane-sentinel/tasks.md b/openspec/changes/fix-biocentral-transmembrane-sentinel/tasks.md new file mode 100644 index 000000000..5c17af02d --- /dev/null +++ b/openspec/changes/fix-biocentral-transmembrane-sentinel/tasks.md @@ -0,0 +1,98 @@ +## 1. Regression Coverage + +- [x] 1.1 Update the focused Biocentral retriever test to require the unambiguous negative TMbed category and observe it fail. +- [x] 1.2 Extend the generated Python-to-TypeScript bundle contract with a negative TMbed prediction and observe the consumer assertion fail. + +## 2. Producer and Documentation + +- [x] 2.1 Change the Biocentral TMbed adapter to emit `non-transmembrane` for a completed negative prediction. +- [x] 2.2 Update CLI and web annotation descriptions to document the corrected categorical vocabulary. + +## 3. Verification + +- [x] 3.1 Run the focused Python retriever and cross-language contract tests and confirm they pass. +- [x] 3.2 Regenerate annotation documentation and verify generated files are current. +- [x] 3.3 Run the affected Python lint/test gates and the repository-wide `pnpm precommit` gate. +- [x] 3.4 Verify a newly produced bundle displays the negative TMbed category separately from missing values. + +## 4. Review Regression Coverage + +- [x] 4.1 Add focused real-model regressions for `None` and empty TMbed payloads and observe the expected failures. +- [x] 4.2 Extend the generated bundle contract with an adapter-derived missing TMbed payload and observe the expected failure. + +## 5. Missing Payload Fix + +- [x] 5.1 Return the established missing representation before scanning absent TMbed topology payloads. + +## 6. Review Verification + +- [x] 6.1 Run the focused Biocentral and cross-language bundle contract tests and confirm they pass. +- [x] 6.2 Run Ruff, the complete non-slow ProtSpace Python suite, strict OpenSpec validation, and `pnpm precommit`. +- [x] 6.3 Prepare the verified change for the inline review follow-up and CI run. + +## 7. Follow-up Review Regression Coverage + +- [x] 7.1 Add focused real-model regressions proving missing TMbed payloads remain + missing for signal peptide and observe the expected failures. +- [x] 7.2 Extend the generated bundle contract to expose missing signal peptide as + `N/A` and observe the expected failure. +- [x] 7.3 Add CLI regression coverage proving FASTA sequences reach the annotation + manager and strengthen the hosted normalized-FASTA handoff assertion. + +## 8. Follow-up Review Fixes + +- [x] 8.1 Parse and pass FASTA sequences through `protspace annotate` while preserving + HDF5 behavior. +- [x] 8.2 Centralize optional TMbed topology extraction for both derived annotations. + +## 9. Follow-up Review Verification + +- [x] 9.1 Run focused ProtSpace, hosted-prep, and cross-language contract tests. +- [x] 9.2 Run Ruff, the complete relevant Python suites, strict OpenSpec validation, + and `pnpm precommit`. +- [x] 9.3 Prepare the verified change for the review reply and pushed CI run. + +## 10. Malformed Payload and Spec Ownership Regressions + +- [x] 10.1 Add real-model regressions proving malformed TMbed payloads remain missing + for both derived annotations and observe the expected failures. +- [x] 10.2 Add a producer/consumer regression proving a malformed TMbed payload reaches + TypeScript as `N/A` and observe the expected failure. +- [x] 10.3 Move the FASTA sequence requirement from `bundle-format-contract` to its + owning `annotation-input` capability within this change. + +## 11. Malformed Payload Fix + +- [x] 11.1 Restrict the shared TMbed topology extractor to non-blank strings containing + only supported topology labels while preserving all valid topology semantics. + +## 12. Final Review Verification + +- [x] 12.1 Run focused ProtSpace and cross-language contract tests. +- [x] 12.2 Run Ruff, the complete relevant Python and hosted-prep suites, strict OpenSpec + validation, and `pnpm precommit`. +- [x] 12.3 Prepare the verified change for inline review replies and pushed CI. + +## 13. Source Precedence and Optimized-Path Regressions + +- [x] 13.1 Add a manager regression proving InterPro prefers a UniProt canonical + sequence while Biocentral prefers the submitted FASTA sequence, and observe it + fail. +- [x] 13.2 Add a CLI regression proving UniProt-only FASTA annotation does not parse or + retain sequences, and observe it fail. +- [x] 13.3 Extend the optimized bundle conversion contract with the negative and missing + TMbed sentinel assertions. + +## 14. Source-Aware Sequence Handling and Documentation + +- [x] 14.1 Apply source-specific sequence precedence in the annotation manager. +- [x] 14.2 Parse FASTA sequences only when a requested annotation source consumes them. +- [x] 14.3 Document that mixed TMbed predictions expose both transmembrane categories. + +## 15. Latest Review Verification + +- [x] 15.1 Run focused annotation-manager, CLI, and bundle-contract checks. +- [x] 15.2 Run Ruff, the complete relevant Python and hosted-prep suites, strict OpenSpec + validation, and `pnpm precommit`. +- [x] 15.3 Prepare the verified changes and per-item dispositions for the newest review + reply. diff --git a/packages/utils/src/visualization/annotation-metadata.ts b/packages/utils/src/visualization/annotation-metadata.ts index eb8ad9d20..fc506d9ce 100644 --- a/packages/utils/src/visualization/annotation-metadata.ts +++ b/packages/utils/src/visualization/annotation-metadata.ts @@ -354,7 +354,8 @@ export const ANNOTATION_METADATA: Record = { label: 'Transmembrane', source: 'Biocentral', isPredicted: true, - description: 'Transmembrane type (none / alpha-helical / beta-barrel) predicted by TMbed.', + description: + 'Transmembrane type (non-transmembrane / alpha-helical / beta-barrel) predicted by TMbed.', docsUrl: docs('predicted_transmembrane'), }, }; diff --git a/tests/contract/bundle.contract.test.ts b/tests/contract/bundle.contract.test.ts index 4f741b70c..323624c19 100644 --- a/tests/contract/bundle.contract.test.ts +++ b/tests/contract/bundle.contract.test.ts @@ -27,6 +27,7 @@ import { // working tree, not built package output. (The vitest config still aliases // `@protspace/utils` — packages/core's own sources import it that way.) import { BUNDLE_DELIMITER_BYTES } from '../../packages/utils/src/parquet/constants'; +import { getProteinAnnotationValues } from '../../packages/utils/src/visualization/plot-data-accessors'; const REPO_ROOT = resolve(__dirname, '../..'); @@ -40,6 +41,9 @@ interface Manifest { largeProteinCount: number; projectionCount: number; labelWithReservedChar: string; + negativeTransmembraneCategory: string; + missingTransmembraneIndex: number; + malformedTmbedIndex: number; nullLengthIndex: number; statisticsColumns: string[]; statisticsCategory: string; @@ -227,6 +231,41 @@ describe('annotation encoding across the language boundary', () => { expect(data.annotations.domains.values).toContain('DomB'); }); + it('preserves a negative TMbed prediction as a category', () => { + expect(data.annotations.predicted_transmembrane.values).toContain( + manifest.negativeTransmembraneCategory, + ); + }); + + it('preserves a missing TMbed payload as N/A', () => { + expect( + getProteinAnnotationValues( + data, + manifest.missingTransmembraneIndex, + 'predicted_transmembrane', + ), + ).toEqual(['__NA__']); + }); + + it('preserves a missing TMbed payload as N/A for signal peptide', () => { + expect( + getProteinAnnotationValues( + data, + manifest.missingTransmembraneIndex, + 'predicted_signal_peptide', + ), + ).toEqual(['__NA__']); + }); + + it('preserves a malformed TMbed payload as N/A', () => { + expect( + getProteinAnnotationValues(data, manifest.malformedTmbedIndex, 'predicted_transmembrane'), + ).toEqual(['__NA__']); + expect( + getProteinAnnotationValues(data, manifest.malformedTmbedIndex, 'predicted_signal_peptide'), + ).toEqual(['__NA__']); + }); + it('reports a missing numeric value as missing rather than zero', () => { const lengths = data.numeric_annotation_data?.length; // Assert the length first: an out-of-range index yields `undefined`, which @@ -286,6 +325,16 @@ describe('the optimized conversion path real datasets take', () => { expect(data.annotations.family.values.join('|')).not.toContain('%3B'); expect(data.annotations.domains.values).toContain('DomA'); expect(data.annotations.domains.values).toContain('DomB'); + expect(data.annotations.predicted_transmembrane.values).toContain( + manifest.negativeTransmembraneCategory, + ); + expect( + getProteinAnnotationValues( + data, + manifest.missingTransmembraneIndex, + 'predicted_transmembrane', + ), + ).toEqual(['__NA__']); const lengths = data.numeric_annotation_data?.length; expect(lengths).toHaveLength(manifest.largeProteinCount); diff --git a/tests/contract/emit_bundles.py b/tests/contract/emit_bundles.py index 4785ad0e5..77218f64b 100644 --- a/tests/contract/emit_bundles.py +++ b/tests/contract/emit_bundles.py @@ -40,8 +40,12 @@ import pyarrow as pa import pyarrow.parquet as pq +from biocentral_api._generated import Prediction from protspace.data.annotations.encoding import encode_field +from protspace.data.annotations.retrievers.biocentral_retriever import ( + BiocentralPredictionRetriever, +) from protspace.stats.base import STATS_SCHEMA # Small enough to eyeball a failure, big enough for a category to have members. @@ -78,6 +82,47 @@ def protein_ids(count: int) -> list[str]: MULTI_HIT_CELL = f"{encode_field('DomA')}|0.91;{encode_field('DomB')}|0.82" +def tmbed_predictions(value: object) -> list[Prediction]: + """Build the real generated prediction shape consumed by the adapter.""" + return [ + Prediction( + model_name="TMbed", + prediction_name="topology", + protocol="per_residue", + value=value, + ) + ] + + +# Derive this fixture value from the real annotation adapter rather than copying +# its vocabulary into the contract test. A topology with only inside/outside +# labels is a completed TMbed prediction with no membrane-spanning segment. +NEGATIVE_TMBED_CATEGORY = BiocentralPredictionRetriever._extract_transmembrane( + tmbed_predictions("oooooiiiii") +) + +# Keep one explicit missing-payload row separate from the fixture's other empty +# rows. This catches an adapter that invents a negative biological result before +# the bundle reader has a chance to normalize the missing representation. +MISSING_TMBED_INDEX = 1 +MISSING_TMBED_VALUE = BiocentralPredictionRetriever._extract_transmembrane( + tmbed_predictions(None) +) +MISSING_SIGNAL_PEPTIDE_VALUE = BiocentralPredictionRetriever._extract_signal_peptide( + tmbed_predictions(None) +) + +# Malformed payloads are unavailable predictions, not completed negatives. Keep +# one explicit row so this producer decision is exercised through bundle ingestion. +MALFORMED_TMBED_INDEX = 2 +MALFORMED_TMBED_VALUE = BiocentralPredictionRetriever._extract_transmembrane( + tmbed_predictions("garbage") +) +MALFORMED_SIGNAL_PEPTIDE_VALUE = BiocentralPredictionRetriever._extract_signal_peptide( + tmbed_predictions("garbage") +) + + def build_annotations_table(ids: list[str]) -> pa.Table: """Mimic ``protspace annotate`` output: an ``identifier`` column plus annotations. @@ -94,6 +139,16 @@ def build_annotations_table(ids: list[str]) -> pa.Table: encode_field("Hydrolase") ] * rest domains = [MULTI_HIT_CELL] + [f"{encode_field('DomB')}|0.75"] * rest + predicted_transmembrane = [ + NEGATIVE_TMBED_CATEGORY, + MISSING_TMBED_VALUE, + MALFORMED_TMBED_VALUE, + ] + [""] * (rest - 2) + predicted_signal_peptide = [ + "False", + MISSING_SIGNAL_PEPTIDE_VALUE, + MALFORMED_SIGNAL_PEPTIDE_VALUE, + ] + [""] * (rest - 2) # A genuine double column with a null -- distinguishes "missing" from 0 and # from NaN across the language boundary. Real bundles carry both string-typed @@ -106,6 +161,8 @@ def build_annotations_table(ids: list[str]) -> pa.Table: "identifier": pa.array(ids, pa.string()), "family": pa.array(family, pa.string()), "domains": pa.array(domains, pa.string()), + "predicted_signal_peptide": pa.array(predicted_signal_peptide, pa.string()), + "predicted_transmembrane": pa.array(predicted_transmembrane, pa.string()), "length": pa.array(length, pa.float64()), } ) @@ -342,6 +399,9 @@ def emit(item: tuple[str, tuple[int, list[str]]]) -> None: "largeProteinCount": LARGE_PROTEIN_COUNT, "projectionCount": len(PROJECTIONS), "labelWithReservedChar": LABEL_WITH_RESERVED_CHAR, + "negativeTransmembraneCategory": NEGATIVE_TMBED_CATEGORY, + "missingTransmembraneIndex": MISSING_TMBED_INDEX, + "malformedTmbedIndex": MALFORMED_TMBED_INDEX, "nullLengthIndex": NULL_LENGTH_INDEX, "statisticsColumns": STATS_SCHEMA.names, "statisticsCategory": STATISTICS_CATEGORY,