diff --git a/apps/protspace/docs/annotations.md b/apps/protspace/docs/annotations.md index 2bf6e60f5..581b2f534 100644 --- a/apps/protspace/docs/annotations.md +++ b/apps/protspace/docs/annotations.md @@ -20,10 +20,12 @@ Annotation sources have different requirements for protein identifiers: | Requirement | Sources | Works with `-f` FASTA? | | ----------- | ------- | ---------------------- | -| **UniProt accession** | UniProt, Taxonomy, TED | No — accession needed | +| **UniProt accession** | UniProt, Taxonomy, TED | No* — accession needed | | **Protein sequence** | InterPro, Biocentral, Pfam CLANS | Yes — provide `-f` | -If your H5 keys are not valid UniProt accessions (e.g., `NCBI|...`, custom IDs), accession-dependent annotations will be empty. Sequence-dependent annotations can still work if you provide the original FASTA file with `-f`. +\* `length` is the exception: when UniProt does not provide a sequence length, ProtSpace derives it from a matching FASTA sequence. A non-empty UniProt length remains authoritative, even when a FASTA sequence is available. + +If your H5 keys are not valid UniProt accessions, other accession-dependent annotations will be empty. Sequence-dependent annotations and the missing-length fallback can still work when the H5 keys match the identifiers parsed from the FASTA file supplied with `-f` (for example, the same bare accession or custom ID). ## Group Presets @@ -75,7 +77,7 @@ With `--keep-tmp`, only API-fetched annotations are cached; the CSV is always re | `go_cc` | GO — Cellular Component | `nucleus\|IDA;cytoplasm\|IEA` | | `go_mf` | GO — Molecular Function | `DNA binding\|IDA;protein binding\|IEA` | | `keyword` | UniProt keywords | `KW-0002 (3D-structure);KW-0025 (Alternative splicing)` | -| `length` | Sequence length (amino acids) | `393` | +| `length` | Sequence length (UniProt-preferred; FASTA fallback when missing; `*` and `-` are not counted as residues) | `393` | | `protein_existence` | Evidence level for protein existence | `Evidence at protein level` | | `protein_families` | First protein family | `Protein kinase superfamily\|ISS` | | `reviewed` | Swiss-Prot / TrEMBL | `true` / `false` | diff --git a/apps/protspace/src/protspace/cli/annotate.py b/apps/protspace/src/protspace/cli/annotate.py index e2e99fecb..adf77d512 100644 --- a/apps/protspace/src/protspace/cli/annotate.py +++ b/apps/protspace/src/protspace/cli/annotate.py @@ -57,14 +57,19 @@ def annotate( from protspace.data.annotations.encoding import stamp_format_version from protspace.data.annotations.manager import ProteinAnnotationManager - from protspace.data.io.fasta import is_fasta_file - from protspace.data.loaders.h5 import EMBEDDING_EXTENSIONS + from protspace.data.io.fasta import is_fasta_file, parse_fasta + from protspace.data.loaders.h5 import EMBEDDING_EXTENSIONS, parse_identifier # Extract identifiers from input + sequences = None if is_fasta_file(input): from protspace.data.loaders.query import extract_identifiers_from_fasta headers = extract_identifiers_from_fasta(input) + sequences = { + parse_identifier(header): sequence + for header, sequence in parse_fasta(input).items() + } elif input.suffix.lower() in EMBEDDING_EXTENSIONS: from protspace.data.loaders.h5 import _collect_datasets @@ -100,6 +105,7 @@ def annotate( headers=headers, annotations=annotations_list, output_path=None, + sequences=sequences, ).to_pd() if not scores: diff --git a/apps/protspace/src/protspace/cli/prepare.py b/apps/protspace/src/protspace/cli/prepare.py index a538756dc..6a0eb0343 100644 --- a/apps/protspace/src/protspace/cli/prepare.py +++ b/apps/protspace/src/protspace/cli/prepare.py @@ -492,7 +492,10 @@ def prepare( if not h5s: logger.warning(f"No embedding files in: {path}") continue - embedding_sets.append(load_h5(h5s, name_override=name_override)) + emb_set = load_h5(h5s, name_override=name_override) + if fasta_for_similarity: + emb_set.fasta_path = fasta_for_similarity + embedding_sets.append(emb_set) elif path.suffix.lower() in EMBEDDING_EXTENSIONS: emb_set = load_h5([path], name_override=name_override) # Attach FASTA path from -f flag if provided (for sequence reuse) diff --git a/apps/protspace/src/protspace/data/annotations/manager.py b/apps/protspace/src/protspace/data/annotations/manager.py index 26c3dbc75..89ab849e4 100644 --- a/apps/protspace/src/protspace/data/annotations/manager.py +++ b/apps/protspace/src/protspace/data/annotations/manager.py @@ -39,6 +39,18 @@ logger = logging.getLogger(__name__) +def _resolve_fasta_sequence_length( + identifier: str, + length: object, + sequences: dict[str, str] | None, +) -> object: + """Return a FASTA-derived length only when the existing value is empty.""" + sequence = sequences.get(identifier, "") if sequences else "" + if length or not sequence: + return length + return str(sum(character not in "*-" for character in sequence)) + + class ProteinAnnotationManager: """Orchestrator for protein annotation extraction workflow.""" @@ -132,6 +144,7 @@ def to_pd(self) -> pd.DataFrame: if self.sources_to_fetch["uniprot"] else cached_uniprot ) + uniprot_annotations = self._fill_missing_fasta_lengths(uniprot_annotations) taxonomy_annotations = ( self._fetch_taxonomy(uniprot_annotations, failed_sources) if self.sources_to_fetch["taxonomy"] @@ -199,6 +212,36 @@ def to_pd(self) -> pd.DataFrame: return df + def _fill_missing_fasta_lengths( + self, proteins: list[ProteinAnnotations] + ) -> list[ProteinAnnotations]: + """Fill empty sequence lengths from matching local FASTA sequences.""" + if not self.sequences: + return proteins + + result = [] + for protein in proteins: + length = protein.annotations.get("length") + resolved_length = _resolve_fasta_sequence_length( + protein.identifier, + length, + self.sequences, + ) + if resolved_length == length: + result.append(protein) + continue + + result.append( + ProteinAnnotations( + identifier=protein.identifier, + annotations={ + **protein.annotations, + "length": resolved_length, + }, + ) + ) + return result + def _fetch_uniprot(self, failed_sources: list) -> list[ProteinAnnotations]: """Fetch UniProt annotations.""" try: @@ -210,9 +253,13 @@ def _fetch_uniprot(self, failed_sources: list) -> list[ProteinAnnotations]: except Exception as e: failed_sources.append(f"UniProt ({str(e)})") logger.warning(f"Failed to retrieve UniProt annotations: {e}") - # Create minimal annotation set with just identifiers + # Preserve the same row schema as normal UniProt responses so later + # formatting cannot drop columns based on which protein comes first. return [ - ProteinAnnotations(identifier=header, annotations={"organism_id": ""}) + ProteinAnnotations( + identifier=header, + annotations=dict.fromkeys(UNIPROT_ANNOTATIONS, ""), + ) for header in self.headers ] diff --git a/apps/protspace/src/protspace/data/processors/pipeline.py b/apps/protspace/src/protspace/data/processors/pipeline.py index cf1821b63..40215f1d2 100644 --- a/apps/protspace/src/protspace/data/processors/pipeline.py +++ b/apps/protspace/src/protspace/data/processors/pipeline.py @@ -350,7 +350,10 @@ def _fetch_annotations( self, headers: list[str], embedding_sets: list[EmbeddingSet] = None ) -> pd.DataFrame: """Fetch annotations from APIs with incremental caching support.""" - from protspace.data.annotations.manager import ProteinAnnotationManager + from protspace.data.annotations.manager import ( + ProteinAnnotationManager, + _resolve_fasta_sequence_length, + ) # Extract sequences from FASTA files (if available) to avoid re-fetching sequences = self._extract_sequences(embedding_sets) if embedding_sets else {} @@ -419,6 +422,22 @@ def _fetch_annotations( else: api_df = cached_df + if "length" in api_df.columns and sequences: + identifier_col = api_df.columns[0] + api_df = api_df.copy() + api_df["length"] = [ + _resolve_fasta_sequence_length( + identifier, + length, + sequences, + ) + for identifier, length in zip( + api_df[identifier_col], + api_df["length"], + strict=True, + ) + ] + # Warn if cached annotations are all empty data_cols = [c for c in api_df.columns if c != "identifier"] if data_cols: diff --git a/apps/protspace/tests/test_annotate_cli.py b/apps/protspace/tests/test_annotate_cli.py new file mode 100644 index 000000000..f491d776f --- /dev/null +++ b/apps/protspace/tests/test_annotate_cli.py @@ -0,0 +1,44 @@ +import pandas as pd +from typer.testing import CliRunner + +from protspace.cli.app import app +from protspace.data.annotations.retrievers.uniprot_retriever import ( + ProteinAnnotations, + UniProtRetriever, +) + + +def test_annotate_fasta_derives_missing_length_from_normalized_sequence( + tmp_path, monkeypatch +): + """The FASTA-backed CLI path must supply sequences to the annotation manager.""" + fasta = tmp_path / "input.fasta" + fasta.write_text(">custom|custom_protein|description\nMPEPTIDE\n") + output = tmp_path / "annotations.parquet" + + monkeypatch.setattr( + UniProtRetriever, + "fetch_annotations", + lambda self: [ + ProteinAnnotations( + identifier="custom_protein", + annotations={"length": ""}, + ) + ], + ) + + result = CliRunner().invoke( + app, + [ + "annotate", + "-i", + str(fasta), + "-a", + "length", + "-o", + str(output), + ], + ) + + assert result.exit_code == 0, result.output + assert pd.read_parquet(output).loc[0, "length"] == "8" diff --git a/apps/protspace/tests/test_annotation_manager.py b/apps/protspace/tests/test_annotation_manager.py index d7dc308bf..9c309c254 100644 --- a/apps/protspace/tests/test_annotation_manager.py +++ b/apps/protspace/tests/test_annotation_manager.py @@ -439,6 +439,83 @@ def test_transform_protein_families_with_semicolon(self): class TestIntegration: """Integration tests for complete workflows.""" + @patch("src.protspace.data.annotations.manager.UniProtRetriever") + def test_uses_fasta_sequence_length_when_uniprot_length_is_missing( + self, mock_uniprot_retriever + ): + mock_uniprot_retriever.return_value.fetch_annotations.return_value = [ + ProteinAnnotations( + identifier="custom_protein", + annotations={"length": ""}, + ) + ] + extractor = ProteinAnnotationExtractor( + headers=["custom_protein"], + annotations=["length"], + sequences={"custom_protein": "MPEPTIDE"}, + ) + + result = extractor.to_pd() + + assert result.loc[0, "length"] == "8" + + @patch("src.protspace.data.annotations.manager.UniProtRetriever") + def test_preserves_uniprot_length_when_fasta_sequence_is_available( + self, mock_uniprot_retriever + ): + mock_uniprot_retriever.return_value.fetch_annotations.return_value = [ + ProteinAnnotations( + identifier="P01308", + annotations={"length": "110"}, + ) + ] + extractor = ProteinAnnotationExtractor( + headers=["P01308"], + annotations=["length"], + sequences={"P01308": "MPEPTIDE"}, + ) + + result = extractor.to_pd() + + assert result.loc[0, "length"] == "110" + + @patch("src.protspace.data.annotations.manager.UniProtRetriever") + def test_counts_residues_not_fasta_gap_or_terminator_markers( + self, mock_uniprot_retriever + ): + mock_uniprot_retriever.return_value.fetch_annotations.return_value = [ + ProteinAnnotations( + identifier="custom_protein", + annotations={"length": ""}, + ) + ] + extractor = ProteinAnnotationExtractor( + headers=["custom_protein"], + annotations=["length"], + sequences={"custom_protein": "M-PEP*"}, + ) + + result = extractor.to_pd() + + assert result.loc[0, "length"] == "4" + + @patch("src.protspace.data.annotations.manager.UniProtRetriever") + def test_retains_length_column_when_uniprot_request_fails( + self, mock_uniprot_retriever + ): + mock_uniprot_retriever.return_value.fetch_annotations.side_effect = ( + RuntimeError("offline") + ) + extractor = ProteinAnnotationExtractor( + headers=["no_sequence", "custom_protein"], + annotations=["length"], + sequences={"custom_protein": "MPEPTIDE"}, + ) + + result = extractor.to_pd() + + assert result["length"].tolist() == ["", "8"] + @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_backend_switch.py b/apps/protspace/tests/test_backend_switch.py index c00899a35..726495b9c 100644 --- a/apps/protspace/tests/test_backend_switch.py +++ b/apps/protspace/tests/test_backend_switch.py @@ -206,3 +206,40 @@ def test_embed_cli_rejects_unknown_backend(tmp_path): ) assert result.exit_code != 0 + + +def test_prepare_directory_h5_attaches_fasta_to_embedding_set(tmp_path, monkeypatch): + h5_dir = tmp_path / "embeddings" + h5_dir.mkdir() + with h5py.File(h5_dir / "model.h5", "w") as h5_file: + h5_file.attrs["model_name"] = "prot_t5" + h5_file.create_dataset("custom_protein", data=np.ones(4, dtype=np.float32)) + + fasta = tmp_path / "input.fasta" + fasta.write_text(">custom_protein\nMPEPTIDE\n") + captured = {} + + def capture_run(self, embedding_sets): + captured["fasta_path"] = embedding_sets[0].fasta_path + return self.config.output_path + + monkeypatch.setattr( + "protspace.data.processors.pipeline.ReductionPipeline.run", capture_run + ) + + result = CliRunner().invoke( + app, + [ + "prepare", + "-i", + str(h5_dir), + "-f", + str(fasta), + "-o", + str(tmp_path / "out"), + "--no-log", + ], + ) + + assert result.exit_code == 0, result.output + assert captured["fasta_path"] == fasta diff --git a/apps/protspace/tests/test_pipeline_utils.py b/apps/protspace/tests/test_pipeline_utils.py index 5ec6fc296..9f7b8ce74 100644 --- a/apps/protspace/tests/test_pipeline_utils.py +++ b/apps/protspace/tests/test_pipeline_utils.py @@ -3,6 +3,7 @@ from collections import Counter import numpy as np +import pandas as pd import pytest from protspace.data.loaders.embedding_set import ( @@ -345,6 +346,57 @@ def test_tsv_path(self): assert self._resolve(["data.tsv", "ec"]) == (["ec"], "data.tsv") +# --------------------------------------------------------------------------- +# complete annotation cache +# --------------------------------------------------------------------------- + + +class TestCompleteAnnotationCache: + def test_fills_only_missing_cached_lengths_from_fasta(self, tmp_path): + fasta_path = tmp_path / "input.fasta" + fasta_path.write_text(">custom_protein\nMPEPTIDE\n>cached_protein\nMPEPTIDE\n") + cache_path = tmp_path / "all_annotations.parquet" + cached = pd.DataFrame( + [ + { + "identifier": "custom_protein", + "length": "", + "gene_name": "", + "protein_name": "", + "uniprot_kb_id": "", + }, + { + "identifier": "cached_protein", + "length": "110", + "gene_name": "", + "protein_name": "", + "uniprot_kb_id": "", + }, + ] + ) + cached.to_parquet(cache_path, index=False) + embedding_set = EmbeddingSet( + name="test", + data=np.zeros((2, 2), dtype=np.float32), + headers=["custom_protein", "cached_protein"], + fasta_path=fasta_path, + ) + pipeline = ReductionPipeline( + PipelineConfig( + methods=[MethodSpec("pca", 2)], + output_path=tmp_path / "out.zip", + keep_tmp=True, + intermediate_dir=tmp_path, + annotations=["length"], + ) + ) + + result = pipeline._fetch_annotations(embedding_set.headers, [embedding_set]) + + assert result["length"].tolist() == ["8", "110"] + assert pd.read_parquet(cache_path)["length"].tolist() == ["", "110"] + + # --------------------------------------------------------------------------- # _validate_headers # --------------------------------------------------------------------------- diff --git a/docs/guide/annotations.md b/docs/guide/annotations.md index c32ce7097..dd48fa7a1 100644 --- a/docs/guide/annotations.md +++ b/docs/guide/annotations.md @@ -142,9 +142,9 @@ Keywords are a hierarchical controlled vocabulary, mostly assigned by curators, **Sequence length** -Length of the protein sequence in amino acids. +UniProt sequence length, or a matching local FASTA length when missing. -This is the number of amino acid residues in the entry's canonical sequence and is the most direct measure of protein size. Values are positive integers, ranging from a few dozen residues for short peptides to tens of thousands for the largest proteins such as titin. Because sequence length influences how a pLM pools its per-residue representation, colouring by length can reveal whether apparent embedding structure tracks protein size. See [UniProt: Sequences](https://www.uniprot.org/help/sequences). +When UniProt provides a length, this is the number of amino acid residues in the entry's canonical sequence. If UniProt does not provide a length and a matching FASTA sequence is available, ProtSpace derives the value from that local sequence, excluding `*` terminator and `-` gap markers; a non-empty UniProt length always takes precedence. Values are positive integers, ranging from a few dozen residues for short peptides to tens of thousands for the largest proteins such as titin. Because sequence length influences how a pLM pools its per-residue representation, colouring by length can reveal whether apparent embedding structure tracks protein size. See [UniProt: Sequences](https://www.uniprot.org/help/sequences). ### `protein_existence` {#protein_existence} diff --git a/docs/scripts/annotation-details.ts b/docs/scripts/annotation-details.ts index dc2aff2b4..7d31bd085 100644 --- a/docs/scripts/annotation-details.ts +++ b/docs/scripts/annotation-details.ts @@ -94,7 +94,7 @@ export const ANNOTATION_DETAILS: Record = { }, length: { detailsMarkdown: - "This is the number of amino acid residues in the entry's canonical sequence and is the most direct measure of protein size. Values are positive integers, ranging from a few dozen residues for short peptides to tens of thousands for the largest proteins such as titin. Because sequence length influences how a pLM pools its per-residue representation, colouring by length can reveal whether apparent embedding structure tracks protein size. See [UniProt: Sequences](https://www.uniprot.org/help/sequences).", + "When UniProt provides a length, this is the number of amino acid residues in the entry's canonical sequence. If UniProt does not provide a length and a matching FASTA sequence is available, ProtSpace derives the value from that local sequence, excluding `*` terminator and `-` gap markers; a non-empty UniProt length always takes precedence. Values are positive integers, ranging from a few dozen residues for short peptides to tens of thousands for the largest proteins such as titin. Because sequence length influences how a pLM pools its per-residue representation, colouring by length can reveal whether apparent embedding structure tracks protein size. See [UniProt: Sequences](https://www.uniprot.org/help/sequences).", sourceUrl: 'https://www.uniprot.org/help/sequences', }, protein_existence: { diff --git a/openspec/changes/fix-fasta-sequence-length/.openspec.yaml b/openspec/changes/fix-fasta-sequence-length/.openspec.yaml new file mode 100644 index 000000000..5849c2dbf --- /dev/null +++ b/openspec/changes/fix-fasta-sequence-length/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-08-01 diff --git a/openspec/changes/fix-fasta-sequence-length/README.md b/openspec/changes/fix-fasta-sequence-length/README.md new file mode 100644 index 000000000..d5fd53514 --- /dev/null +++ b/openspec/changes/fix-fasta-sequence-length/README.md @@ -0,0 +1,3 @@ +# fix-fasta-sequence-length + +Derive missing sequence length from the local FASTA sequence when UniProt has no mapping. diff --git a/openspec/changes/fix-fasta-sequence-length/design.md b/openspec/changes/fix-fasta-sequence-length/design.md new file mode 100644 index 000000000..280898f67 --- /dev/null +++ b/openspec/changes/fix-fasta-sequence-length/design.md @@ -0,0 +1,145 @@ +## Context + +The reduction pipeline already parses every referenced FASTA file into an +identifier-to-sequence map and passes that map to `ProteinAnnotationManager`. +The manager currently consults local sequences only when preparing InterPro and +Biocentral requests. Its primary annotation rows still come from UniProt, so an +unmapped identifier retains UniProt's empty `length` value through merge, +transformation, and bundle output. + +When every requested annotation column is present in `all_annotations.parquet`, +the reduction pipeline returns that cached DataFrame before constructing the +manager. Therefore the manager-only fallback does not repair empty lengths on +normal warm-cache reruns. + +The standalone `protspace annotate` command, which is also the annotation path +used by the hosted preparation service, extracts normalized identifiers from a +FASTA input but does not pass its sequences to the manager. The manager fallback +therefore cannot run on that path even though the sequence data is available. + +The fix must remain scoped to missing length metadata. Existing UniProt values +are authoritative, and the annotation manager must continue to work when no +FASTA sequence is available. + +## Goals / Non-Goals + +**Goals:** + +- Fill an empty sequence length from the matching local FASTA sequence. +- Preserve non-empty UniProt sequence lengths. +- Apply the fallback before annotation rows are merged and formatted. +- Apply the fallback when `protspace annotate` receives FASTA input. +- Preserve the supplied FASTA path for directory-based HDF5 input. +- Apply the fallback to complete cache hits without refetching API annotations. +- Exclude FASTA terminator and gap markers from the derived residue count. +- Protect both fallback and precedence behavior with focused tests. +- Keep the annotation references synchronized with the fallback and precedence + behavior. + +**Non-Goals:** + +- Reconcile differences between mapped UniProt and local FASTA sequences. +- Change FASTA parsing or identifier normalization. +- Populate other missing UniProt annotations from FASTA. +- Alter bundle schemas or frontend missing-value rendering. + +## Decisions + +### Enrich the primary annotation rows in the manager + +Immediately after UniProt annotations are fetched or loaded from cache, the +manager will copy each row whose `length` is empty and whose identifier has a +non-empty local sequence, setting `length` to the decimal string form of +`len(sequence)`. Rows that need no fallback remain unchanged. + +This keeps the fallback at the first boundary that owns both inputs and makes +the corrected value available to every downstream merge and output path. + +Alternatives considered: + +- A separate FASTA annotation source would require unnecessary merger and + precedence machinery for one field. +- Passing local sequences into `UniProtRetriever` would couple an external API + client to local-file data and obscure the retriever's responsibility. +- Filling the DataFrame after formatting would fix only one output path and + leave earlier consumers with inconsistent annotation rows. + +### Treat FASTA length strictly as a fallback + +Any non-empty UniProt `length` value remains unchanged, even if it differs from +the local sequence length. The issue concerns unmapped proteins; defining +canonical-versus-construct reconciliation is outside this change. + +The derived value counts amino-acid residues. FASTA `*` terminator markers and +`-` alignment-gap markers are accepted input syntax but are not residues, so +they are excluded from the fallback count. + +### Enrich complete cache hits before returning them + +When the cache contains all requested columns and annotation refetching is not +requested, the pipeline will apply the same missing-only length rule to the +selected cached DataFrame before merging custom CSV annotations. This preserves +the cache-hit fast path and avoids API calls while ensuring warm-cache output +matches cold-cache output. + +The manager and pipeline cache branch will share the scalar precedence rule so +that an empty value is filled from a matching non-empty sequence and every +non-empty cached or UniProt value is retained. The cached Parquet file itself is +not rewritten on a read-only complete-cache hit; the derived value is local to +the current output, preserving existing cache lifecycle semantics. + +### Supply sequences at the standalone annotation boundary + +When `protspace annotate` receives FASTA input, it will parse the file into an +identifier-to-sequence map and normalize each key with the same +`parse_identifier` policy already used to extract annotation identifiers and by +the reduction pipeline. The command will pass that map to +`ProteinAnnotationManager`; HDF5 input continues to provide no local sequences. + +This repairs both direct CLI usage and the hosted preparation service without +duplicating fallback logic or changing FASTA parsing and normalization policy. + +### Preserve FASTA access for directory-based HDF5 input + +`protspace prepare -i -f ` loads the directory into one +embedding set. Attach the supplied FASTA path to that set exactly as the +single-HDF5 branch already does so sequence extraction and annotation fallback +have identical inputs in both supported HDF5 forms. + +### Keep failed UniProt rows schema-uniform + +If the top-level UniProt request fails, create empty rows with the complete +UniProt annotation schema, matching the retriever's invalid-identifier and +batch-failure paths. Downstream formatters derive their columns from the first +row, so uniform keys prevent FASTA-derived lengths from being retained or +dropped based on row order. + +## Risks / Trade-offs + +- **[Identifier mismatch prevents fallback]** → Continue using the pipeline's + existing `parse_identifier` normalization and require an exact key match in + the manager; do not add a second normalization policy. +- **[Mutation leaks into retriever or cache data]** → Return copied + `ProteinAnnotations` values only for rows that receive the fallback. +- **[Mapped lengths are accidentally overwritten]** → Add a focused + precedence test alongside the regression test, including a warm-cache row. +- **[Warm-cache fix triggers network work or rewrites cache]** → Keep the + complete-cache early-return branch and enrich a copy of its selected DataFrame + without constructing the annotation manager or persisting the derived value. +- **[Standalone identifiers and sequence keys diverge]** → Normalize parsed + FASTA keys with the existing `parse_identifier` helper before passing them to + the manager. +- **[FASTA syntax inflates the derived length]** → Exclude `*` terminator and + `-` gap markers from the residue count. +- **[UniProt failure creates heterogeneous rows]** → Reuse the complete + `UNIPROT_ANNOTATIONS` key set for top-level failure rows. + +## Migration Plan + +No data or configuration migration is required. Deploy the Python package with +the fallback; rollback consists of reverting the manager enrichment and its +tests. + +## Open Questions + +None. diff --git a/openspec/changes/fix-fasta-sequence-length/proposal.md b/openspec/changes/fix-fasta-sequence-length/proposal.md new file mode 100644 index 000000000..2a0858e0f --- /dev/null +++ b/openspec/changes/fix-fasta-sequence-length/proposal.md @@ -0,0 +1,42 @@ +## Why + +Protein sequences that cannot be mapped to UniProt currently export an empty +`length` annotation even when the preparation pipeline already parsed their +sequence from FASTA. This discards reliable local metadata and causes ProtSpace +to display the sequence length as unavailable. + +## What Changes + +- Derive a missing sequence length from the matching local FASTA sequence. +- Supply normalized FASTA sequences to the standalone `protspace annotate` + command used by the hosted preparation service. +- Preserve FASTA access when `protspace prepare` loads a directory of HDF5 + embeddings with `-f`. +- Apply the same fallback when a complete annotation cache satisfies the run. +- Count amino-acid residues rather than FASTA terminator or gap markers. +- Preserve a non-empty length returned by UniProt. +- Keep the existing missing-value behavior when neither source provides a + sequence length. +- Add regression coverage for the fallback and precedence behavior. +- Document the missing-only FASTA fallback and UniProt precedence. + +## Capabilities + +### New Capabilities + +- `fasta-sequence-metadata`: Defines how FASTA-derived sequence metadata fills + gaps left by external annotation sources. + +### Modified Capabilities + +None. + +## Impact + +- Affects the Python annotation orchestration in + `apps/protspace/src/protspace/data/annotations/manager.py`, the standalone + annotation command, the HDF5-directory preparation path, and the + complete-cache branch in `apps/protspace/src/protspace/data/processors/pipeline.py`. +- Adds focused manager, standalone annotation, and warm-cache pipeline tests. +- Updates both annotation references to describe FASTA fallback behavior. +- Does not change public APIs, file formats, dependencies, or UniProt precedence. diff --git a/openspec/changes/fix-fasta-sequence-length/specs/fasta-sequence-metadata/spec.md b/openspec/changes/fix-fasta-sequence-length/specs/fasta-sequence-metadata/spec.md new file mode 100644 index 000000000..87480b915 --- /dev/null +++ b/openspec/changes/fix-fasta-sequence-length/specs/fasta-sequence-metadata/spec.md @@ -0,0 +1,65 @@ +## ADDED Requirements + +### Requirement: Missing sequence length falls back to FASTA + +The annotation pipeline SHALL derive a protein's sequence length from its +matching local FASTA sequence when the primary annotation source does not +provide a sequence length. + +#### Scenario: Unmapped protein has a FASTA sequence + +- **WHEN** a protein's UniProt annotation has an empty sequence length and a + matching non-empty FASTA sequence is available +- **THEN** the output length equals the number of residues in that FASTA + sequence + +#### Scenario: FASTA sequence contains non-residue markers + +- **WHEN** a matching FASTA sequence contains `*` terminator or `-` gap markers +- **THEN** those markers are excluded from the derived residue count + +#### Scenario: UniProt provides a sequence length + +- **WHEN** a protein has both a non-empty UniProt sequence length and a matching + FASTA sequence +- **THEN** the output retains the UniProt sequence length + +#### Scenario: Standalone annotation command receives FASTA input + +- **WHEN** `protspace annotate` receives a FASTA file containing a protein whose + UniProt annotation has an empty sequence length +- **THEN** the output length equals the number of residues in the matching FASTA + sequence + +#### Scenario: Complete annotation cache has a missing sequence length + +- **WHEN** a complete annotation cache has an empty sequence length and a + matching non-empty FASTA sequence is available +- **THEN** the warm-cache output length equals the number of residues in that + FASTA sequence without refetching annotations + +#### Scenario: Complete annotation cache has an existing sequence length + +- **WHEN** a complete annotation cache has a non-empty sequence length and a + matching FASTA sequence is available +- **THEN** the warm-cache output retains the cached sequence length + +#### Scenario: Directory-based HDF5 input supplies a FASTA file + +- **WHEN** `protspace prepare` receives a directory of HDF5 files and a FASTA + file through `-f` +- **THEN** the annotation pipeline can use matching sequences from that FASTA + file for the missing-length fallback + +#### Scenario: UniProt retrieval fails before producing rows + +- **WHEN** UniProt retrieval fails and a matching local FASTA sequence is + available for only some proteins +- **THEN** the output retains a uniform length column and fills the matching + proteins independently of row order + +#### Scenario: No source provides a sequence length + +- **WHEN** a protein's UniProt annotation has an empty sequence length and no + matching non-empty FASTA sequence is available +- **THEN** the output sequence length remains missing diff --git a/openspec/changes/fix-fasta-sequence-length/tasks.md b/openspec/changes/fix-fasta-sequence-length/tasks.md new file mode 100644 index 000000000..8aa7ce182 --- /dev/null +++ b/openspec/changes/fix-fasta-sequence-length/tasks.md @@ -0,0 +1,47 @@ +## 1. Regression Coverage + +- [x] 1.1 Add a focused manager test for deriving an empty UniProt length from a matching FASTA sequence. +- [x] 1.2 Run the focused test and record the expected RED failure against the current implementation. + +## 2. Root-Cause Fix + +- [x] 2.1 Enrich only missing primary annotation lengths from matching local FASTA sequences before downstream merging. +- [x] 2.2 Add coverage proving that an existing UniProt length is preserved. +- [x] 2.3 Run the focused tests and record GREEN results. + +## 3. Verification + +- [x] 3.1 Re-run the original unmapped-protein reproduction and confirm the output contains the FASTA-derived length. +- [x] 3.2 Run the affected Python package's lint, format, and non-slow test checks. +- [x] 3.3 Run the repository-mandated `pnpm precommit` gate before publishing. + +## 4. Review Follow-up: Complete Cache Hits + +- [x] 4.1 Add a warm-cache pipeline regression for an empty cached length with + a matching FASTA sequence and record the expected RED failure. +- [x] 4.2 Apply the same missing-only FASTA fallback on the complete-cache path + without refetching annotations or rewriting the cache. +- [x] 4.3 Prove that a non-empty cached length remains authoritative. +- [x] 4.4 Run strict OpenSpec validation and all affected/full verification gates. + +## 5. Review Follow-up: Standalone Annotation and Documentation + +- [x] 5.1 Add a CLI regression proving normalized FASTA sequence data supplies a + missing length and record the expected RED failure. +- [x] 5.2 Pass normalized FASTA identifier-to-sequence data through + `protspace annotate` to the existing manager fallback. +- [x] 5.3 Update both annotation references with missing-only FASTA fallback and + UniProt-over-FASTA precedence. +- [x] 5.4 Run focused, package, docs, OpenSpec, and repository verification gates. + +## 6. Review Follow-up: Edge Cases + +- [x] 6.1 Add regressions for FASTA marker counting, UniProt failure schema + uniformity, and directory-HDF5 FASTA propagation; record the expected RED + failures. +- [x] 6.2 Exclude `*` and `-` from FASTA-derived residue counts. +- [x] 6.3 Preserve the complete UniProt schema on top-level retrieval failure. +- [x] 6.4 Attach `-f` FASTA input to directory-loaded HDF5 embedding sets. +- [x] 6.5 Synchronize user-facing annotation metadata and identifier-matching + documentation. +- [x] 6.6 Run focused, package, docs, OpenSpec, and repository verification gates. diff --git a/packages/utils/src/visualization/annotation-metadata.ts b/packages/utils/src/visualization/annotation-metadata.ts index eb8ad9d20..ac53a361d 100644 --- a/packages/utils/src/visualization/annotation-metadata.ts +++ b/packages/utils/src/visualization/annotation-metadata.ts @@ -147,7 +147,7 @@ export const ANNOTATION_METADATA: Record = { label: 'Sequence length', source: 'UniProt', isPredicted: false, - description: 'Length of the protein sequence in amino acids.', + description: 'UniProt sequence length, or a matching local FASTA length when missing.', docsUrl: docs('length'), }, protein_existence: {