From b02521f6dff1af198eb9156d68b12b967aa83624 Mon Sep 17 00:00:00 2001 From: Florin Senoner <23100806+FlorinSenoner@users.noreply.github.com> Date: Sat, 1 Aug 2026 18:42:55 +0200 Subject: [PATCH 1/5] fix(protspace): derive missing length from fasta --- .../src/protspace/data/annotations/manager.py | 26 +++++++ .../tests/test_annotation_manager.py | 40 ++++++++++ .../fix-fasta-sequence-length/.openspec.yaml | 2 + .../fix-fasta-sequence-length/README.md | 3 + .../fix-fasta-sequence-length/design.md | 75 +++++++++++++++++++ .../fix-fasta-sequence-length/proposal.md | 32 ++++++++ .../specs/fasta-sequence-metadata/spec.md | 26 +++++++ .../fix-fasta-sequence-length/tasks.md | 16 ++++ 8 files changed, 220 insertions(+) create mode 100644 openspec/changes/fix-fasta-sequence-length/.openspec.yaml create mode 100644 openspec/changes/fix-fasta-sequence-length/README.md create mode 100644 openspec/changes/fix-fasta-sequence-length/design.md create mode 100644 openspec/changes/fix-fasta-sequence-length/proposal.md create mode 100644 openspec/changes/fix-fasta-sequence-length/specs/fasta-sequence-metadata/spec.md create mode 100644 openspec/changes/fix-fasta-sequence-length/tasks.md diff --git a/apps/protspace/src/protspace/data/annotations/manager.py b/apps/protspace/src/protspace/data/annotations/manager.py index 26c3dbc75..b1b724f95 100644 --- a/apps/protspace/src/protspace/data/annotations/manager.py +++ b/apps/protspace/src/protspace/data/annotations/manager.py @@ -132,6 +132,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 +200,31 @@ 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: + sequence = self.sequences.get(protein.identifier, "") + if protein.annotations.get("length") or not sequence: + result.append(protein) + continue + + result.append( + ProteinAnnotations( + identifier=protein.identifier, + annotations={ + **protein.annotations, + "length": str(len(sequence)), + }, + ) + ) + return result + def _fetch_uniprot(self, failed_sources: list) -> list[ProteinAnnotations]: """Fetch UniProt annotations.""" try: diff --git a/apps/protspace/tests/test_annotation_manager.py b/apps/protspace/tests/test_annotation_manager.py index d7dc308bf..6799bc6fd 100644 --- a/apps/protspace/tests/test_annotation_manager.py +++ b/apps/protspace/tests/test_annotation_manager.py @@ -439,6 +439,46 @@ 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.TaxonomyRetriever") @patch("src.protspace.data.annotations.manager.UniProtRetriever") def test_to_pd_complete_workflow( 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..a7e37e793 --- /dev/null +++ b/openspec/changes/fix-fasta-sequence-length/design.md @@ -0,0 +1,75 @@ +## 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. + +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. +- Protect both fallback and precedence behavior with focused tests. + +**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. + +## 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. + +## 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..d4e70e620 --- /dev/null +++ b/openspec/changes/fix-fasta-sequence-length/proposal.md @@ -0,0 +1,32 @@ +## 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. +- 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. + +## 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`. +- Adds focused tests in `apps/protspace/tests/test_annotation_manager.py`. +- 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..8b0e6692b --- /dev/null +++ b/openspec/changes/fix-fasta-sequence-length/specs/fasta-sequence-metadata/spec.md @@ -0,0 +1,26 @@ +## 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: 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: 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..46308f1b2 --- /dev/null +++ b/openspec/changes/fix-fasta-sequence-length/tasks.md @@ -0,0 +1,16 @@ +## 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. From 0e000cbf764cff159f2e7e62cffa4c3f9835f942 Mon Sep 17 00:00:00 2001 From: Florin Senoner <23100806+FlorinSenoner@users.noreply.github.com> Date: Sat, 1 Aug 2026 23:06:55 +0200 Subject: [PATCH 2/5] fix(annotations): enrich complete cache lengths from fasta --- .../src/protspace/data/annotations/manager.py | 21 ++++++-- .../src/protspace/data/processors/pipeline.py | 21 +++++++- apps/protspace/tests/test_pipeline_utils.py | 52 +++++++++++++++++++ .../fix-fasta-sequence-length/design.md | 25 ++++++++- .../fix-fasta-sequence-length/proposal.md | 6 ++- .../specs/fasta-sequence-metadata/spec.md | 13 +++++ .../fix-fasta-sequence-length/tasks.md | 9 ++++ 7 files changed, 140 insertions(+), 7 deletions(-) diff --git a/apps/protspace/src/protspace/data/annotations/manager.py b/apps/protspace/src/protspace/data/annotations/manager.py index b1b724f95..8f15ad01e 100644 --- a/apps/protspace/src/protspace/data/annotations/manager.py +++ b/apps/protspace/src/protspace/data/annotations/manager.py @@ -39,6 +39,16 @@ 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 "" + return length if length or not sequence else str(len(sequence)) + + class ProteinAnnotationManager: """Orchestrator for protein annotation extraction workflow.""" @@ -209,8 +219,13 @@ def _fill_missing_fasta_lengths( result = [] for protein in proteins: - sequence = self.sequences.get(protein.identifier, "") - if protein.annotations.get("length") or not sequence: + length = protein.annotations.get("length") + resolved_length = _resolve_fasta_sequence_length( + protein.identifier, + length, + self.sequences, + ) + if resolved_length == length: result.append(protein) continue @@ -219,7 +234,7 @@ def _fill_missing_fasta_lengths( identifier=protein.identifier, annotations={ **protein.annotations, - "length": str(len(sequence)), + "length": resolved_length, }, ) ) 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_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/openspec/changes/fix-fasta-sequence-length/design.md b/openspec/changes/fix-fasta-sequence-length/design.md index a7e37e793..ea248ced6 100644 --- a/openspec/changes/fix-fasta-sequence-length/design.md +++ b/openspec/changes/fix-fasta-sequence-length/design.md @@ -7,6 +7,11 @@ 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 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. @@ -18,6 +23,7 @@ FASTA sequence is available. - 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 to complete cache hits without refetching API annotations. - Protect both fallback and precedence behavior with focused tests. **Non-Goals:** @@ -54,6 +60,20 @@ 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. +### 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. + ## Risks / Trade-offs - **[Identifier mismatch prevents fallback]** → Continue using the pipeline's @@ -62,7 +82,10 @@ canonical-versus-construct reconciliation is outside this change. - **[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. + 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. ## Migration Plan diff --git a/openspec/changes/fix-fasta-sequence-length/proposal.md b/openspec/changes/fix-fasta-sequence-length/proposal.md index d4e70e620..ef4d56e0b 100644 --- a/openspec/changes/fix-fasta-sequence-length/proposal.md +++ b/openspec/changes/fix-fasta-sequence-length/proposal.md @@ -8,6 +8,7 @@ to display the sequence length as unavailable. ## What Changes - Derive a missing sequence length from the matching local FASTA sequence. +- Apply the same fallback when a complete annotation cache satisfies the run. - Preserve a non-empty length returned by UniProt. - Keep the existing missing-value behavior when neither source provides a sequence length. @@ -27,6 +28,7 @@ None. ## Impact - Affects the Python annotation orchestration in - `apps/protspace/src/protspace/data/annotations/manager.py`. -- Adds focused tests in `apps/protspace/tests/test_annotation_manager.py`. + `apps/protspace/src/protspace/data/annotations/manager.py` and the complete-cache + branch in `apps/protspace/src/protspace/data/processors/pipeline.py`. +- Adds focused manager and warm-cache pipeline tests. - 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 index 8b0e6692b..6752b7e85 100644 --- 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 @@ -19,6 +19,19 @@ provide a sequence length. FASTA sequence - **THEN** the output retains the UniProt sequence length +#### 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: No source provides a sequence length - **WHEN** a protein's UniProt annotation has an empty sequence length and no diff --git a/openspec/changes/fix-fasta-sequence-length/tasks.md b/openspec/changes/fix-fasta-sequence-length/tasks.md index 46308f1b2..2dc3b8e14 100644 --- a/openspec/changes/fix-fasta-sequence-length/tasks.md +++ b/openspec/changes/fix-fasta-sequence-length/tasks.md @@ -14,3 +14,12 @@ - [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. From 5cab3198d6ee059a65502879437b5d1bbc546c91 Mon Sep 17 00:00:00 2001 From: Florin Senoner <23100806+FlorinSenoner@users.noreply.github.com> Date: Wed, 5 Aug 2026 14:00:26 +0200 Subject: [PATCH 3/5] fix(annotate): pass fasta sequences to annotation manager --- apps/protspace/docs/annotations.md | 8 ++-- apps/protspace/src/protspace/cli/annotate.py | 10 +++- apps/protspace/tests/test_annotate_cli.py | 47 +++++++++++++++++++ docs/guide/annotations.md | 2 +- docs/scripts/annotation-details.ts | 2 +- .../fix-fasta-sequence-length/design.md | 22 +++++++++ .../fix-fasta-sequence-length/proposal.md | 11 +++-- .../specs/fasta-sequence-metadata/spec.md | 7 +++ .../fix-fasta-sequence-length/tasks.md | 10 ++++ 9 files changed, 109 insertions(+), 10 deletions(-) create mode 100644 apps/protspace/tests/test_annotate_cli.py diff --git a/apps/protspace/docs/annotations.md b/apps/protspace/docs/annotations.md index 2bf6e60f5..07a96526e 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 (e.g., `NCBI|...`, custom IDs), other accession-dependent annotations will be empty. Sequence-dependent annotations and the missing-length fallback can still work if you provide the original FASTA file with `-f`. ## 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) | `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/tests/test_annotate_cli.py b/apps/protspace/tests/test_annotate_cli.py new file mode 100644 index 000000000..561316c9b --- /dev/null +++ b/apps/protspace/tests/test_annotate_cli.py @@ -0,0 +1,47 @@ +import pandas as pd +from typer.testing import CliRunner + +from protspace.cli.app import app +from protspace.data.annotations.retrievers.uniprot_retriever import ( + ProteinAnnotations, +) + + +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.""" + from protspace.data.annotations.retrievers.uniprot_retriever import ( + UniProtRetriever, + ) + + 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/docs/guide/annotations.md b/docs/guide/annotations.md index c32ce7097..a893251fc 100644 --- a/docs/guide/annotations.md +++ b/docs/guide/annotations.md @@ -144,7 +144,7 @@ Keywords are a hierarchical controlled vocabulary, mostly assigned by curators, Length of the protein sequence in amino acids. -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; 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..9c02c149c 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; 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/design.md b/openspec/changes/fix-fasta-sequence-length/design.md index ea248ced6..9761da926 100644 --- a/openspec/changes/fix-fasta-sequence-length/design.md +++ b/openspec/changes/fix-fasta-sequence-length/design.md @@ -12,6 +12,11 @@ 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. @@ -23,8 +28,11 @@ FASTA sequence is available. - 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. - Apply the fallback to complete cache hits without refetching API annotations. - Protect both fallback and precedence behavior with focused tests. +- Keep the annotation references synchronized with the fallback and precedence + behavior. **Non-Goals:** @@ -74,6 +82,17 @@ 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. + ## Risks / Trade-offs - **[Identifier mismatch prevents fallback]** → Continue using the pipeline's @@ -86,6 +105,9 @@ the current output, preserving existing cache lifecycle semantics. - **[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. ## Migration Plan diff --git a/openspec/changes/fix-fasta-sequence-length/proposal.md b/openspec/changes/fix-fasta-sequence-length/proposal.md index ef4d56e0b..d0bb3dd25 100644 --- a/openspec/changes/fix-fasta-sequence-length/proposal.md +++ b/openspec/changes/fix-fasta-sequence-length/proposal.md @@ -8,11 +8,14 @@ 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. - Apply the same fallback when a complete annotation cache satisfies the run. - 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 @@ -28,7 +31,9 @@ None. ## Impact - Affects the Python annotation orchestration in - `apps/protspace/src/protspace/data/annotations/manager.py` and the complete-cache - branch in `apps/protspace/src/protspace/data/processors/pipeline.py`. -- Adds focused manager and warm-cache pipeline tests. + `apps/protspace/src/protspace/data/annotations/manager.py`, the standalone + annotation command, 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 index 6752b7e85..dfb31e195 100644 --- 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 @@ -19,6 +19,13 @@ provide a sequence length. 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 diff --git a/openspec/changes/fix-fasta-sequence-length/tasks.md b/openspec/changes/fix-fasta-sequence-length/tasks.md index 2dc3b8e14..8c9606a01 100644 --- a/openspec/changes/fix-fasta-sequence-length/tasks.md +++ b/openspec/changes/fix-fasta-sequence-length/tasks.md @@ -23,3 +23,13 @@ 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. From b5845824d737ad627a0d3cf87e93be16feb7df7e Mon Sep 17 00:00:00 2001 From: tsenoner Date: Thu, 6 Aug 2026 11:42:11 +0200 Subject: [PATCH 4/5] test(annotate): hoist UniProtRetriever import to module level - Merge the function-local UniProtRetriever import in test_annotate_fasta_derives_missing_length_from_normalized_sequence into the existing module-level import from the same module; the deferred import was not load-bearing since monkeypatch targets a class attribute. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016qoU16kDQxz6U3H2UWbbm2 --- apps/protspace/tests/test_annotate_cli.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/apps/protspace/tests/test_annotate_cli.py b/apps/protspace/tests/test_annotate_cli.py index 561316c9b..f491d776f 100644 --- a/apps/protspace/tests/test_annotate_cli.py +++ b/apps/protspace/tests/test_annotate_cli.py @@ -4,6 +4,7 @@ from protspace.cli.app import app from protspace.data.annotations.retrievers.uniprot_retriever import ( ProteinAnnotations, + UniProtRetriever, ) @@ -11,10 +12,6 @@ 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.""" - from protspace.data.annotations.retrievers.uniprot_retriever import ( - UniProtRetriever, - ) - fasta = tmp_path / "input.fasta" fasta.write_text(">custom|custom_protein|description\nMPEPTIDE\n") output = tmp_path / "annotations.parquet" From 5826d8d9c9512af8660bec3ac2e853b52da995f6 Mon Sep 17 00:00:00 2001 From: Florin Senoner <23100806+FlorinSenoner@users.noreply.github.com> Date: Thu, 6 Aug 2026 14:17:28 +0200 Subject: [PATCH 5/5] fix(annotations): handle fasta length edge cases --- apps/protspace/docs/annotations.md | 4 +- apps/protspace/src/protspace/cli/prepare.py | 5 ++- .../src/protspace/data/annotations/manager.py | 12 ++++-- .../tests/test_annotation_manager.py | 37 +++++++++++++++++++ apps/protspace/tests/test_backend_switch.py | 37 +++++++++++++++++++ docs/guide/annotations.md | 4 +- docs/scripts/annotation-details.ts | 2 +- .../fix-fasta-sequence-length/design.md | 25 +++++++++++++ .../fix-fasta-sequence-length/proposal.md | 7 +++- .../specs/fasta-sequence-metadata/spec.md | 19 ++++++++++ .../fix-fasta-sequence-length/tasks.md | 12 ++++++ .../src/visualization/annotation-metadata.ts | 2 +- 12 files changed, 154 insertions(+), 12 deletions(-) diff --git a/apps/protspace/docs/annotations.md b/apps/protspace/docs/annotations.md index 07a96526e..581b2f534 100644 --- a/apps/protspace/docs/annotations.md +++ b/apps/protspace/docs/annotations.md @@ -25,7 +25,7 @@ Annotation sources have different requirements for protein identifiers: \* `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 (e.g., `NCBI|...`, custom IDs), other accession-dependent annotations will be empty. Sequence-dependent annotations and the missing-length fallback can still work if you provide the original FASTA file with `-f`. +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 @@ -77,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 (UniProt-preferred; FASTA fallback when missing) | `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/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 8f15ad01e..89ab849e4 100644 --- a/apps/protspace/src/protspace/data/annotations/manager.py +++ b/apps/protspace/src/protspace/data/annotations/manager.py @@ -46,7 +46,9 @@ def _resolve_fasta_sequence_length( ) -> object: """Return a FASTA-derived length only when the existing value is empty.""" sequence = sequences.get(identifier, "") if sequences else "" - return length if length or not sequence else str(len(sequence)) + if length or not sequence: + return length + return str(sum(character not in "*-" for character in sequence)) class ProteinAnnotationManager: @@ -251,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/tests/test_annotation_manager.py b/apps/protspace/tests/test_annotation_manager.py index 6799bc6fd..9c309c254 100644 --- a/apps/protspace/tests/test_annotation_manager.py +++ b/apps/protspace/tests/test_annotation_manager.py @@ -479,6 +479,43 @@ def test_preserves_uniprot_length_when_fasta_sequence_is_available( 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/docs/guide/annotations.md b/docs/guide/annotations.md index a893251fc..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. -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; 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). +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 9c02c149c..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: - "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; 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).", + "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/design.md b/openspec/changes/fix-fasta-sequence-length/design.md index 9761da926..280898f67 100644 --- a/openspec/changes/fix-fasta-sequence-length/design.md +++ b/openspec/changes/fix-fasta-sequence-length/design.md @@ -29,7 +29,9 @@ FASTA sequence is available. - 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. @@ -68,6 +70,10 @@ 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 @@ -93,6 +99,21 @@ the reduction pipeline. The command will pass that map to 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 @@ -108,6 +129,10 @@ duplicating fallback logic or changing FASTA parsing and normalization policy. - **[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 diff --git a/openspec/changes/fix-fasta-sequence-length/proposal.md b/openspec/changes/fix-fasta-sequence-length/proposal.md index d0bb3dd25..2a0858e0f 100644 --- a/openspec/changes/fix-fasta-sequence-length/proposal.md +++ b/openspec/changes/fix-fasta-sequence-length/proposal.md @@ -10,7 +10,10 @@ to display the sequence length as unavailable. - 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. @@ -32,8 +35,8 @@ None. - Affects the Python annotation orchestration in `apps/protspace/src/protspace/data/annotations/manager.py`, the standalone - annotation command, and the complete-cache branch in - `apps/protspace/src/protspace/data/processors/pipeline.py`. + 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 index dfb31e195..87480b915 100644 --- 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 @@ -13,6 +13,11 @@ provide a sequence length. - **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 @@ -39,6 +44,20 @@ provide a sequence length. 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 diff --git a/openspec/changes/fix-fasta-sequence-length/tasks.md b/openspec/changes/fix-fasta-sequence-length/tasks.md index 8c9606a01..8aa7ce182 100644 --- a/openspec/changes/fix-fasta-sequence-length/tasks.md +++ b/openspec/changes/fix-fasta-sequence-length/tasks.md @@ -33,3 +33,15 @@ - [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: {