diff --git a/.github/workflows/bundle-contract.yml b/.github/workflows/bundle-contract.yml index 12900fa56..9bcc5314f 100644 --- a/.github/workflows/bundle-contract.yml +++ b/.github/workflows/bundle-contract.yml @@ -27,6 +27,11 @@ name: Bundle format contract (Python ↔ TypeScript) # # Cost: ~1 minute, about half of it the Python generator that writes the fixture # bundles by invoking the real `protspace bundle` CLI. +# +# It also checks the generated shared-constants modules against +# contracts/shared-constants.json, for the same reason: a hand edit to only the +# Python module would skip ci.yml, and one to only the TypeScript module would skip +# protspace-ci.yml. on: push: @@ -85,5 +90,8 @@ jobs: - name: Install Node dependencies run: pnpm install --frozen-lockfile + - name: Check generated shared constants + run: pnpm shared-constants:check + - name: Run bundle format contract run: pnpm test:contract diff --git a/.github/workflows/protspace-ci.yml b/.github/workflows/protspace-ci.yml index 5c0501599..abd2babce 100644 --- a/.github/workflows/protspace-ci.yml +++ b/.github/workflows/protspace-ci.yml @@ -20,15 +20,16 @@ name: protspace CI (Python) # # The same holds for every other file outside apps/protspace that the tests read # (protspace cannot import TypeScript or protspace_prep, so its copies are checked -# against their source): the web's palette ids, prep's -# Biocentral outage patterns, the web-side v3 fixtures the decoder is checked -# against, the served demo bundle, the web catalog whose defaultView each showcase -# example opens on, and the perf manifest stage_perf.py writes. A change to only one -# of them would otherwise never run the test that reads it. A checkout holding only -# apps/protspace, the uv workspace files and the files listed below passes the fast -# suite with no skips; a new test reading outside it belongs in the list. The web's -# missing-value tokens are not here: bundle-contract.yml, which runs on every pull -# request, checks them against the Python ones. +# against their source): prep's Biocentral outage patterns, the web-side v3 fixtures +# the decoder is checked against, the served demo bundle, the web catalog whose +# defaultView each showcase example opens on, and the perf manifest stage_perf.py +# writes. A change to only one of them would otherwise never run the test that reads +# it. A checkout holding only apps/protspace, the uv workspace files and the files +# listed below passes the fast suite with no skips; a new test reading outside it +# belongs in the list. Constants shared with the web (missing-value tokens, palette +# ids, ...) are not here: both sides are generated from +# contracts/shared-constants.json, and bundle-contract.yml, which runs on every pull +# request, checks the generated modules and the contract. on: push: @@ -36,8 +37,6 @@ on: paths: - 'apps/protspace/**' - 'docs/guide/python-cli.md' - - 'packages/utils/src/visualization/color-scheme.ts' - - 'packages/utils/src/visualization/numeric-binning.ts' - 'apps/prep/src/protspace_prep/pipeline.py' - 'packages/core/src/components/data-loader/utils/__fixtures__/v3-sample.parquetbundle' - 'apps/web/tests/fixtures/venom_eat_stats_811_v3.parquetbundle' @@ -50,8 +49,6 @@ on: paths: - 'apps/protspace/**' - 'docs/guide/python-cli.md' - - 'packages/utils/src/visualization/color-scheme.ts' - - 'packages/utils/src/visualization/numeric-binning.ts' - 'apps/prep/src/protspace_prep/pipeline.py' - 'packages/core/src/components/data-loader/utils/__fixtures__/v3-sample.parquetbundle' - 'apps/web/tests/fixtures/venom_eat_stats_811_v3.parquetbundle' diff --git a/AGENTS.md b/AGENTS.md index 03aeb58d6..abcb61db2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -37,10 +37,13 @@ One-time CLI setup is in [CONTRIBUTING.md](CONTRIBUTING.md#openspec-one-time-per ## Before committing Always run `pnpm precommit` before any git commit. It is -`lint-staged && quality && docs:annotations:check && docs:examples:check && docs:build`: +`lint-staged && quality && shared-constants:check && docs:annotations:check && +docs:examples:check && docs:build`: - ESLint `--fix` and Prettier `--write`, on staged files only (lint-staged) - TypeScript typecheck, Knip, and Knip dependency validation (`pnpm quality`) +- `shared-constants:check` — the generated TypeScript and Python modules must match + `contracts/shared-constants.json` (see below) - `docs:annotations:check` — the generated annotation reference must match its source - `docs:examples:check` — the generated Example datasets page must match the example catalog, the bundle manifest and its prose (`docs/scripts/example-details.ts`) @@ -69,6 +72,14 @@ Move these in the same PR — `pnpm precommit` covers none, and all three have s Pin a fact that has to live in two places with a test, not a comment asking the next reader to keep them in step — see `apps/protspace/tests/test_docs_extras_sync.py`. +A **value** both the Python package and the web app hold (a token list, a palette id, a colour) +goes in `contracts/shared-constants.json` instead: `pnpm shared-constants` generates it into +`packages/utils/src/generated/` and `apps/protspace/src/protspace/_generated/`, both committed, +and each side re-exports it under the name its code already uses. Never edit the generated +modules. A **rule** both sides implement (how a cell is matched, which column holds the ids) is +code, and is pinned by running both on the same inputs in the bundle contract suite +(`tests/contract`). [`contracts/README.md`](contracts/README.md) has the details. + ## End-to-end tests (Playwright) `e2e.yml` alone drives the real app in a browser; the unit suites run in jsdom, which has no diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1fd7db43c..507ab7ccd 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -198,6 +198,14 @@ pnpm test:ci # Run the CI test suite locally pnpm docs:build # Verify the documentation build ``` +**Constants shared with the Python package** (missing-value tokens, the N/A key and colour, +Kelly's colours, palette ids, point shapes, boolean labels) live in +`contracts/shared-constants.json`. Edit them there and run `pnpm shared-constants`, which +regenerates `packages/utils/src/generated/shared-constants.ts` and +`apps/protspace/src/protspace/_generated/shared_constants.py`; commit all three. +`pnpm shared-constants:check` (in `pnpm precommit` and `bundle-contract.yml`) fails while they +disagree. See [`contracts/README.md`](contracts/README.md) for what belongs there. + ### Code Quality Standards **Automated CI checks** (all PRs must pass): @@ -209,7 +217,8 @@ pnpm docs:build # Verify the documentation build 5. Test suite execution 6. Documentation build 7. Bundle format contract (`bundle-contract.yml`), `pnpm test:contract` against a real Python-produced - bundle. Deliberately **unfiltered**: it runs on every PR, including PRs that touch no code it covers. + bundle, plus `pnpm shared-constants:check`. Deliberately **unfiltered**: it runs on every PR, + including PRs that touch no code it covers. 8. E2E, Playwright (`e2e.yml`), on PRs touching the web app, the packages it builds on, or the root files that decide what those resolve to — `e2e.yml` owns the exact list — plus nightly on `main`. Outside those paths, dispatch it: `gh workflow run e2e.yml --ref `. diff --git a/apps/prep/tests/test_pipeline.py b/apps/prep/tests/test_pipeline.py index 43b76948d..2276eab8d 100644 --- a/apps/prep/tests/test_pipeline.py +++ b/apps/prep/tests/test_pipeline.py @@ -339,7 +339,7 @@ async def test_pipeline_uses_normalized_fasta_for_embed_and_annotate(ctx): normalized = ctx.output_dir / "input.normalized.fasta" assert seen_inputs["embed"] == str(normalized) assert seen_inputs["annotate"] == str(normalized) - assert ">P12345" in normalized.read_text() + assert normalized.read_text() == ">P12345\nMAAAAAA\n" async def test_embed_failure_with_connection_refused_is_classified_as_biocentral_unavailable( diff --git a/apps/protspace/CLAUDE.md b/apps/protspace/CLAUDE.md index d9d8a9252..4c8770dae 100644 --- a/apps/protspace/CLAUDE.md +++ b/apps/protspace/CLAUDE.md @@ -122,6 +122,8 @@ Model shortcuts are defined in `MODEL_SHORT_KEYS` (the models the client lists i ``` src/protspace/ +├── _generated/ +│ └── shared_constants.py # Values shared with the web app, generated from contracts/shared-constants.json (`pnpm shared-constants`); never edit, re-exported under their old names ├── cli/ │ ├── app.py # Typer app root, shared utilities, setup_logging() │ ├── common_options.py # Shared Typer options @@ -315,7 +317,8 @@ For a live count run `uv run pytest tests/ --collect-only -q`. | `test_biocentral_retriever.py` | Biocentral prediction retriever (TMbed parsing, per-sequence), batches of at most `_BATCH_SIZE`, a failed batch keeps the others and warns clear of `_BIOCENTRAL_DOWN_PATTERNS`, long sequences still sent, no usable server fails the source and the warning says why, a model the client no longer lists fails the source (blank columns would be cached as complete) and every model the retriever names is one the client lists | | `test_taxonomy_annotation_retriever.py` | Taxonomy via UniProt Taxonomy API (mocked + integration) | | `test_config_validation.py` | DimensionReductionConfig parameter validation | -| `test_style_warnings.py` | `protspace style` warnings: numeric-column detection (tsenoner/protspace-legacy#67) + `selectedPaletteId` validation (categorical vs gradient palette, per column type) + pinned palette-catalog contract | +| `test_style_warnings.py` | `protspace style` warnings: numeric-column detection (tsenoner/protspace-legacy#67) + `selectedPaletteId` validation (categorical vs gradient palette, per column type); `kellys` is a categorical palette | +| `test_shared_constants.py` | Every name the package exports for a constant shared with the web app (`MISSING_VALUE_TOKENS`, `NA_INTERNAL`, `NA_PINNED_COLOR`, `KELLYS_COLORS`, the palette ids, `ALLOWED_SHAPES`, `ARROW_BOOLEAN_LABELS`) holds the value generated from `contracts/shared-constants.json`; the generated module is plain data (no imports) | | `test_h5_parse_identifier.py` | HDF5 key parsing, identifier extraction | | `test_base_data_processor.py` | BaseProcessor: reduction (the MDS input actually fed: a similarity matrix converted with `sqrt(max(s) - s)`, a distance matrix passed through), output creation, save (incl. settings in unbundled output) | | `test_ted_retriever.py` | TED domain retriever (mocked AlphaFold API, CATH names), final retry pass for failed lookups (10-in-a-row cut-off, 404 never retried), parallel lookups equal to one at a time under jitter, one session, CATH names loaded once, a slow first-pass lookup not holding up the others, a tripped final-pass breaker stopping lookups still retrying | @@ -326,7 +329,7 @@ For a live count run `uv run pytest tests/ --collect-only -q`. | `test_transfer_cli.py` | Transfer orchestration core (k>1 confidence, cosine metric, self-transfer, a cell spelled missing such as `N/A` filled and never copied) and the `transfer` CLI: bad input (no matching embedding, no query match, both id columns) as a usage error with its message, `--help` rendering, `protein_id` and legacy v1 bundles end to end | | `test_predictions_overlay.py` | Building the per-cell prediction overlay columns | | `test_display_decode.py` | Display-side decoding of encoded values, multi-hit rendering, gated-off passthrough | -| `test_missing_values.py` | One definition of a missing cell (`is_missing_value`, the web's `normalizeMissingValue`; the bundle contract suite pins the two): the Dash plot and `stats` read cased, padded and `__NA__` spellings and pandas nulls as missing; every fixed producer vocabulary (TMbed, signal peptide, PDB, fragment, reviewed, boolean labels) stays a category, hit by hit; no string literal in `data/annotations/` or `data/parsers/` is spelled like a missing value | +| `test_missing_values.py` | One definition of a missing cell (`is_missing_value`, the web's `normalizeMissingValue`; the bundle contract suite pins the two): the Dash plot and `stats` read cased, padded and `__NA__` spellings and pandas nulls as missing; `protspace style` stores an N/A colour, shape, pin or hide named by any spelling under `__NA__`, once, a restyle replacing the stored one, and `--generate-template` lists N/A once; every fixed producer vocabulary (TMbed, signal peptide, PDB, fragment, reviewed, boolean labels) stays a category, hit by hit; no string literal in `data/annotations/` or `data/parsers/` is spelled like a missing value | | `test_toxprot_demo.py` | Signal-peptide bound parsing, mature-FASTA stripping, bundle post-processing (column filter/reorder) | | `test_build_showcase.py` | `scripts/generate_examples/build_showcase.py`, the showcase bundle build: bundle reading and rebuilding (legacy and v3), projection selection, column order from the web catalog's `defaultView`, styles filtering, mature chains and the three-finger-toxins hold-out and EAT gates, provenance stamping read back by `write_manifest.py`, step markers and reruns, fetch release checks, `stage-release` never giving a published name new bytes; one offline end-to-end EAT build (three-finger toxins), run in CI | | `test_write_manifest.py` | `scripts/generate_examples/write_manifest.py`: every record value read from the file (legacy and v3 containers), provenance and the build command's machine paths redacted, a bundle stamped for another example refused, retained files kept one release and never under a current file's name, the Zenodo DOI kept while the bytes are unchanged, `--check` | diff --git a/apps/protspace/scripts/generate_toxprot_demo.py b/apps/protspace/scripts/generate_toxprot_demo.py index e75269ffb..ee211b477 100644 --- a/apps/protspace/scripts/generate_toxprot_demo.py +++ b/apps/protspace/scripts/generate_toxprot_demo.py @@ -5,10 +5,11 @@ the mature peptides with ProtT5 and ESM2-650M, then runs DR + the sequence-independent annotation sources via `protspace prepare`. The sequence-based sources (InterPro, Biocentral) run separately through -`protspace annotate` on the FULL-LENGTH sequences: local FASTA sequences -take priority over UniProt's, so feeding them the mature peptides made -InterPro miss by MD5 (73 % empty Pfam) and made TMbed predict no signal -peptide for secreted toxins. +`protspace annotate` on the FULL-LENGTH sequences: Biocentral predicts the +local FASTA sequence over UniProt's, so feeding it the mature peptides made +TMbed predict no signal peptide for secreted toxins. (InterPro once missed by +MD5 the same way, 73 % empty Pfam; it now prefers UniProt's canonical +sequence, falling back to the FASTA's only without one.) Finally the bundle keeps every annotation column (protein_families first), replaces `length` with the mature length, and patches the settings JSON: diff --git a/apps/protspace/src/protspace/_generated/__init__.py b/apps/protspace/src/protspace/_generated/__init__.py new file mode 100644 index 000000000..4ec3cff4e --- /dev/null +++ b/apps/protspace/src/protspace/_generated/__init__.py @@ -0,0 +1,6 @@ +"""Modules generated from sources outside the package; never edit them by hand. + +``shared_constants`` comes from ``contracts/shared-constants.json`` at the repository +root (``pnpm shared-constants``). It is committed so the wheel needs nothing outside +``apps/protspace``. +""" diff --git a/apps/protspace/src/protspace/_generated/shared_constants.py b/apps/protspace/src/protspace/_generated/shared_constants.py new file mode 100644 index 000000000..0853beab7 --- /dev/null +++ b/apps/protspace/src/protspace/_generated/shared_constants.py @@ -0,0 +1,130 @@ +"""Constants the protspace package shares with the ProtSpace web app. + +Generated by scripts/shared-constants/generate-shared-constants.mts from +contracts/shared-constants.json. Do not edit: change the JSON, run +``pnpm shared-constants`` and commit both generated modules. +``pnpm shared-constants:check`` fails while they disagree. +""" + +# Strings that mean "this annotation value is missing". A cell is missing +# when it is null, a non-finite number, blank, or, trimmed and lower-cased, +# one of these: `normalizeMissingValue` in the web, `is_missing_value` in +# Python. That rule is code on each side, so the bundle contract suite runs +# both on the same probe spellings. +# +# `__na__` reserves the spelling of NA_VALUE: the bundle writer drops that +# token unconditionally (it is the sentinel a missing cell carries), so a +# genuine category literally spelled `__NA__` would be silently deleted on +# export. Reading it as missing at ingestion makes the two agree. +MISSING_VALUE_TOKENS = frozenset( + { + "na", + "n/a", + "nan", + "null", + "none", + "__na__", + } +) + +# Internal key of the N/A category: the legend value a missing cell gets, +# and the key its colour and shape are saved under in a bundle's settings. +# Never displayed; the web shows it as `N/A`. +NA_VALUE = "__NA__" + +# Default swatch colour of the N/A category, which a saved category colour +# overrides (categorical annotations only). Python gives a pinned N/A this +# colour, and `protspace style --generate-template` pre-fills it for N/A, so +# a CLI-styled legend matches the web's default. +NA_DEFAULT_COLOR = "#DDDDDD" + +# Kelly's 21 colours of maximum contrast, in the order categories take them +# (the category in slot i gets colour i). Medium gray (#848482) is left out, +# too close to the greys of Other (#999999) and N/A. Very light and very +# dark gray come last, for the least frequent categories. In order: vivid +# yellow, strong purple, vivid orange, very light blue, vivid red, grayish +# yellow, vivid green, strong purplish pink, strong blue, strong yellowish +# pink, strong violet, vivid orange yellow, strong purplish red, vivid +# greenish yellow, strong reddish brown, vivid yellowish green, deep +# yellowish brown, vivid reddish orange, dark olive green, very light gray, +# very dark gray. +# +# Reference: Kelly, Kenneth L. Twenty-two colors of maximum contrast. +# Color Engineering 3.26-27 (1965). +KELLYS_COLORS = ( + "#F3C300", + "#875692", + "#F38400", + "#A1CAF1", + "#BE0032", + "#C2B280", + "#008856", + "#E68FAC", + "#0067A5", + "#F99379", + "#604E97", + "#F6A600", + "#B3446C", + "#DCD300", + "#882D17", + "#8DB600", + "#654522", + "#E25822", + "#2B3D26", + "#F2F3F4", + "#222222", +) + +# Palettes for categorical annotations: the only ids a bundle's +# `selectedPaletteId` may name. `protspace style` warns on any other id, and +# the web draws a gradient or unknown id with DEFAULT_CATEGORICAL_PALETTE_ID. +# The web's `COLOR_SCHEMES` holds the colours and must list exactly these ids +# plus GRADIENT_PALETTE_IDS (the type check fails otherwise). A new palette +# also goes in docs/guide/styling.md (Color palettes). +CATEGORICAL_PALETTE_IDS = ( + "kellys", + "okabeIto", + "tolBright", + "set2", + "dark2", + "tableau10", +) + +# Sequential palettes for numeric annotations, chosen in the web's numeric +# legend settings rather than through `selectedPaletteId`. +GRADIENT_PALETTE_IDS = ( + "viridis", + "cividis", + "inferno", + "batlow", + "plasma", +) + +# The palette a categorical annotation starts on, and the one the web falls +# back to; one of CATEGORICAL_PALETTE_IDS. +DEFAULT_CATEGORICAL_PALETTE_ID = "kellys" + +# The gradient a numeric annotation starts on; one of GRADIENT_PALETTE_IDS. +DEFAULT_NUMERIC_PALETTE_ID = "batlow" + +# Shapes a category can be drawn with: the `shape` values a bundle's +# settings may hold. The web's `PointShape` type is exactly these. +POINT_SHAPES = ( + "circle", + "square", + "diamond", + "triangle-up", + "triangle-down", + "plus", +) + +# Labels of an Arrow `bool` annotation column, false then true. Lower case, +# as the browser has always shown one (`String(true)`), so legend colours +# saved against a v2 bundle keep matching. Both v3 writers store a bool +# column under these labels, and the web's writer keeps `sourceType: bool` +# only while every label is one of them. Not Python's CANONICAL_BOOLEANS +# (`False`/`True`), which are string cells the pipeline writes itself. +ARROW_BOOLEAN_LABELS = ( + "false", + "true", +) diff --git a/apps/protspace/src/protspace/core/constants.py b/apps/protspace/src/protspace/core/constants.py index 83d89e5cd..53510ae3d 100644 --- a/apps/protspace/src/protspace/core/constants.py +++ b/apps/protspace/src/protspace/core/constants.py @@ -3,14 +3,13 @@ import pandas as pd +# The spellings the web app reads as a missing annotation value, compared against +# the trimmed, lower-cased cell. Both languages generate them from +# contracts/shared-constants.json; the bundle contract suite (tests/contract) checks +# that the two matching rules agree. +from protspace._generated.shared_constants import MISSING_VALUE_TOKENS from protspace.utils.arrow_reader import ArrowReader -# The spellings the web app reads as a missing annotation value: its -# ``MISSING_VALUE_TOKENS`` (packages/utils/src/visualization/missing-values.ts), -# which it compares against the trimmed, lower-cased cell. The bundle contract -# suite (tests/contract) fails when the two sets differ. -MISSING_VALUE_TOKENS = frozenset({"na", "n/a", "nan", "null", "none", "__na__"}) - def is_missing_value(value) -> bool: """Whether the web app reads this annotation cell as missing (N/A). diff --git a/apps/protspace/src/protspace/data/annotations/cache.py b/apps/protspace/src/protspace/data/annotations/cache.py index 6db7013d0..f4d4ae3e9 100644 --- a/apps/protspace/src/protspace/data/annotations/cache.py +++ b/apps/protspace/src/protspace/data/annotations/cache.py @@ -212,7 +212,40 @@ def fetch_annotations( # explicit -a the whole cached frame is emitted, so treat that as # consuming all of them. refresh_columns = stale_columns if annotations is None else stale_columns & required - discard_columns = stale_columns - refresh_columns + sequences_supplied = all(sequences.get(h) for h in headers) + migration_sources: set[str] = set() + widened: set[str] = set() + if refresh_columns: + # Fetched as though the cache lacked them, so a source looked up by a + # key the cache has no column for (InterPro and Biocentral by + # `sequence`, taxonomy by `organism_id`) also fetches that key from + # UniProt. Without it the refresh would find nothing and stamp that as + # current. The keys are never stale, so this can run before the + # discard below. + needed = AnnotationConfiguration.determine_sources_to_fetch( + cached_annotations - refresh_columns, + refresh_columns, + sequences_supplied=sequences_supplied, + ) + migration_sources = {source for source, fetch in needed.items() if fetch} + # A refresh refetches these sources whole: every column of theirs the + # cache holds, not only the ones this run asks for. Otherwise the cache + # is written back without the others and the next run that asks for one + # pays the source again. Only a source the run queries anyway is + # widened: a run without -a asks for no taxonomy or Biocentral column, + # so their stale columns are dropped rather than paid for. An explicit + # --refetch keeps to the columns the run asks for. + widened = { + s + for s in migration_sources - refetch + if s == "uniprot" or required & SOURCE_ANNOTATIONS[s] + } + # A stale column of a widened source is refreshed with it, not dropped. + discard_columns = ( + stale_columns + - refresh_columns + - set().union(*(SOURCE_ANNOTATIONS[s] for s in widened)) + ) if discard_columns: # Unused this run, so drop rather than refetch: they must not ride # along into a cache this run stamps as current. A later run that @@ -270,7 +303,6 @@ def fetch_annotations( return AnnotationFetch(api_df, uniprot_releases=read_release_stamp(cached_df)) - sequences_supplied = all(sequences.get(h) for h in headers) sources = AnnotationConfiguration.determine_sources_to_fetch( cached_annotations, required, sequences_supplied=sequences_supplied ) @@ -281,19 +313,7 @@ def fetch_annotations( logger.info( f"--refetch: re-fetching {', '.join(s for s in _ANN_SOURCES if sources[s])}" ) - migration_sources = set() if refresh_columns: - # Fetched as though the cache lacked them, so a source looked up by a - # key the cache has no column for (InterPro and Biocentral by - # `sequence`, taxonomy by `organism_id`) also fetches that key from - # UniProt. Without it the refresh would find nothing and stamp that as - # current. - needed = AnnotationConfiguration.determine_sources_to_fetch( - cached_annotations - refresh_columns, - refresh_columns, - sequences_supplied=sequences_supplied, - ) - migration_sources = {source for source, fetch in needed.items() if fetch} for source in migration_sources: sources[source] = True logger.warning( @@ -313,19 +333,6 @@ def fetch_annotations( cols_to_drop = set().union( *(cached_by_source[s] for s in _ANN_SOURCES if sources[s]) ) - # A refresh refetches its sources whole: every column of theirs the - # cache holds, not only the ones this run asks for. Otherwise the cache - # is written back without the others, which were still current, and - # the next run that asks for one pays the source again. Only a source - # the run queries anyway is widened: a run without -a asks for no - # taxonomy or Biocentral column, so their stale columns are dropped - # rather than paid for. An explicit --refetch keeps to the columns the - # run asks for. - widened = { - s - for s in migration_sources - refetch - if s == "uniprot" or required & SOURCE_ANNOTATIONS[s] - } refreshed_along = ( set().union(*(cached_by_source[s] for s in widened)) - required ) @@ -343,12 +350,12 @@ def fetch_annotations( if "uniprot" in migration_sources and "uniprot" not in refetch: # Keep what the migration is about to discard. If its refresh # fails, these cached values beat the empty annotations a failed - # fetch produces — except the stale columns themselves, whose - # ambiguity is the whole reason for the migration. + # fetch produces — except the stale columns, requested or refreshed + # along, whose ambiguity is the whole reason for the migration. preserved = [ c for c in cached_by_source["uniprot"] - if c not in refresh_columns and c in cached_df.columns + if c not in stale_columns and c in cached_df.columns ] if preserved: legacy_uniprot = cached_df[["identifier", *preserved]].copy() diff --git a/apps/protspace/src/protspace/data/annotations/encoding.py b/apps/protspace/src/protspace/data/annotations/encoding.py index bfc443832..0863c312a 100644 --- a/apps/protspace/src/protspace/data/annotations/encoding.py +++ b/apps/protspace/src/protspace/data/annotations/encoding.py @@ -20,6 +20,14 @@ import pandas as pd import pyarrow as pa +# Re-exported: the labels an Arrow ``BOOLEAN`` annotation column is stored as in a +# v3 bundle, lower case as the browser shows one, shared with the web through +# contracts/shared-constants.json. Not CANONICAL_BOOLEANS below, which are string +# cells the pipeline writes itself. +from protspace._generated.shared_constants import ( + ARROW_BOOLEAN_LABELS as ARROW_BOOLEAN_LABELS, +) + #: The annotation **cell grammar** version: ``1`` is the legacy raw-text grammar, #: ``2`` the percent-encoded one this module writes. It is stamped under #: :data:`FORMAT_VERSION_KEY` on every v2-shaped annotations table (a legacy @@ -48,6 +56,11 @@ # where v2 kept its deepest unranked clade ("melanogaster subgroup"); # predicted_transmembrane calls a TMbed negative "non-transmembrane", # where v2 wrote "none", a token the CLI and the web app read as missing +# v4: an empty or malformed TMbed payload leaves predicted_signal_peptide and +# predicted_transmembrane empty, where v3 wrote signal peptide "False" +# for an empty one and read a malformed one as a topology; the InterPro +# columns look up UniProt's canonical sequence, where v3 looked up the +# FASTA's whenever one was given # # To record a new semantics change, add an entry — the version is derived from # this table, so the two cannot drift apart. The pipeline reads nothing else to @@ -73,6 +86,8 @@ 1: frozenset({"xref_pdb"}), 2: frozenset({"protein_families"}) | _INTERPRO_COLUMNS, 3: frozenset({"root", "predicted_transmembrane"}), + 4: frozenset({"predicted_signal_peptide", "predicted_transmembrane"}) + | _INTERPRO_COLUMNS, } ANNOTATION_CACHE_VERSION = max(CACHE_SEMANTICS_CHANGES) ANNOTATION_CACHE_VERSION_ATTR = "protspace_annotation_cache_version" @@ -83,12 +98,6 @@ # second pass reinterprets its own output as raw source data. CANONICAL_BOOLEANS = ("False", "True") -# The labels an Arrow ``BOOLEAN`` annotation column is stored as in a v3 bundle: -# lower case, as the browser has always displayed one (``String(true)``), so -# legend colours saved against a v2 bundle keep matching. Not CANONICAL_BOOLEANS, -# which are string cells the pipeline writes itself. -ARROW_BOOLEAN_LABELS = ("false", "true") - # Chars that must be percent-encoded inside any free-text token. _RESERVED = {";", "|", "%"} | {chr(c) for c in range(0x20)} | {chr(0x7F)} _ENCODE_TABLE = str.maketrans({c: f"%{ord(c):02X}" for c in _RESERVED}) diff --git a/apps/protspace/src/protspace/data/annotations/manager.py b/apps/protspace/src/protspace/data/annotations/manager.py index e4fed97d0..09e4e7024 100644 --- a/apps/protspace/src/protspace/data/annotations/manager.py +++ b/apps/protspace/src/protspace/data/annotations/manager.py @@ -858,19 +858,16 @@ def _fetch_taxonomy( # run over a lookup that only concerned the new ones. return dict(cached) - def _build_sequence_map( - self, uniprot_annotations: list[ProteinAnnotations] + @staticmethod + def _uniprot_sequences( + uniprot_annotations: list[ProteinAnnotations], ) -> dict[str, str]: - """Build a mapping from headers to sequences. - - Merges local sequences (from FASTA, priority) with UniProt results (fallback). - """ - sequences = dict(self.sequences) if self.sequences else {} - for protein in uniprot_annotations: - seq = protein.annotations.get("sequence", "") - if seq and protein.identifier not in sequences: - sequences[protein.identifier] = seq - return sequences + """Map identifiers to the non-empty sequences UniProt returned.""" + return { + protein.identifier: seq + for protein in uniprot_annotations + if (seq := protein.annotations.get("sequence")) + } def _starved_by_uniprot( self, source: str, label: str, headers: list[str], sequences: dict[str, str] @@ -909,7 +906,12 @@ def _fetch_interpro( headers = self.headers if headers is None else headers try: - sequences = self._build_sequence_map(uniprot_annotations) + # UniProt's sequence wins: InterPro's precomputed matches are keyed by + # the canonical sequence hash. + sequences = { + **(self.sequences or {}), + **self._uniprot_sequences(uniprot_annotations), + } self._starved_by_uniprot("interpro", "InterPro", headers, sequences) retriever = InterProRetriever( @@ -939,7 +941,11 @@ def _fetch_biocentral( headers = self.headers if headers is None else headers try: - sequences = self._build_sequence_map(uniprot_annotations) + # The local FASTA sequence wins: Biocentral predicts what was submitted. + sequences = { + **self._uniprot_sequences(uniprot_annotations), + **(self.sequences or {}), + } self._starved_by_uniprot("biocentral", "Biocentral", headers, sequences) retriever = BiocentralPredictionRetriever( diff --git a/apps/protspace/src/protspace/data/annotations/retrievers/biocentral_retriever.py b/apps/protspace/src/protspace/data/annotations/retrievers/biocentral_retriever.py index 6d2ce0a92..e68d8dabe 100644 --- a/apps/protspace/src/protspace/data/annotations/retrievers/biocentral_retriever.py +++ b/apps/protspace/src/protspace/data/annotations/retrievers/biocentral_retriever.py @@ -29,6 +29,7 @@ # Unique sequences per prediction request. One request with every sequence is # untested beyond a few thousand, and an example-scale run is 100K-485K. _BATCH_SIZE = 1000 +_TMBED_TOPOLOGY_LABELS = frozenset("BbHhSio.") # The server's per-sequence length limits (the same on v1.2.1 and v2.0.1). It # answers a request holding any sequence outside them with 422 for the whole @@ -370,34 +371,41 @@ def _extract_signal_peptide(predictions: list) -> str: TMbed labels: S = signal peptide, H/h = TM helix, B/b = TM beta, i/o = non-TM """ - for pred in predictions: - if pred.model_name == "TMbed": - topology = str(pred.value) if pred.value else "" - return "True" if "S" in topology else "False" - return "" + topology = BiocentralPredictionRetriever._extract_tmbed_topology(predictions) + if topology is None: + return "" + return "True" if "S" in topology else "False" @staticmethod def _extract_transmembrane(predictions: list) -> str: """Derive transmembrane type from TMbed per-residue output. Returns: 'alpha-helical', 'beta-barrel', or 'non-transmembrane', and - '' when TMbed predicted nothing + '' when TMbed predicted nothing usable """ - for pred in predictions: - if pred.model_name == "TMbed": - topology = str(pred.value) if pred.value else "" - if not topology: - # No topology is no prediction, not a negative one. - return "" - has_helix = bool(re.search(r"[Hh]", topology)) - has_beta = bool(re.search(r"[Bb]", topology)) - if has_helix and has_beta: - return "alpha-helical;beta-barrel" - elif has_helix: - return "alpha-helical" - elif has_beta: - return "beta-barrel" - # Not "none": the CLI and the web app both read that as a - # missing value, which showed every negative prediction as N/A. - return "non-transmembrane" - return "" + topology = BiocentralPredictionRetriever._extract_tmbed_topology(predictions) + if topology is None: + return "" + + has_helix = "H" in topology or "h" in topology + has_beta = "B" in topology or "b" in topology + if has_helix and has_beta: + return "alpha-helical;beta-barrel" + elif has_helix: + return "alpha-helical" + elif has_beta: + return "beta-barrel" + # Not "none": the CLI and the web app both read that as a missing value, + # which showed every negative prediction as N/A. + return "non-transmembrane" + + @staticmethod + def _extract_tmbed_topology(predictions: list) -> str | None: + """Return a supported TMbed topology, or ``None`` when unavailable. + + An empty or malformed payload is no prediction, not a negative one. + """ + value = next((p.value for p in predictions if p.model_name == "TMbed"), None) + if isinstance(value, str) and value and set(value) <= _TMBED_TOPOLOGY_LABELS: + return value + return None diff --git a/apps/protspace/src/protspace/data/io/settings_converter.py b/apps/protspace/src/protspace/data/io/settings_converter.py index 3859b7d44..2b12e6400 100644 --- a/apps/protspace/src/protspace/data/io/settings_converter.py +++ b/apps/protspace/src/protspace/data/io/settings_converter.py @@ -14,41 +14,57 @@ import re -NA_COLOR = "#C0C0C0" # light gray for missing / values -NA_INTERNAL = "__NA__" # frontend internal key for N/A categories -NA_PINNED_COLOR = "#DDDDDD" # lighter gray used for N/A in pinned settings -# Every spelling a missing cell can arrive under. A v3 bundle's missing cell -# decodes as "". "None" is what ``str()`` yields for a parquet NULL, which is -# how a v2 bundle exported by a web build before v3 stored a missing categorical -# cell (it no longer wrote the __NA__ sentinel), so without it a styled N/A group -# lands under the key "None" and the frontend's __NA__ lookup misses it. -_NA_LABELS = {"", "", "NaN", "__NA__", "None"} +# Kelly's colours in the web's slot order, the web's N/A key and its N/A swatch, +# generated from contracts/shared-constants.json. +from protspace._generated.shared_constants import ( + DEFAULT_CATEGORICAL_PALETTE_ID, + KELLYS_COLORS, + NA_DEFAULT_COLOR, + NA_VALUE, +) +from protspace.core.constants import is_missing_value + +NA_INTERNAL = NA_VALUE # frontend internal key for N/A categories +NA_PINNED_COLOR = NA_DEFAULT_COLOR # the web's N/A swatch, used for a pinned N/A REST_MARKER = "__REST__" # auto-fill remaining slots from top values by frequency -# Kelly's 21 Colors of Maximum Contrast (same order as the web frontend) -KELLYS_COLORS = [ - "#F3C300", - "#875692", - "#F38400", - "#A1CAF1", - "#BE0032", - "#C2B280", - "#008856", - "#E68FAC", - "#0067A5", - "#F99379", - "#604E97", - "#F6A600", - "#B3446C", - "#DCD300", - "#882D17", - "#8DB600", - "#654522", - "#E25822", - "#2B3D26", - "#F2F3F4", - "#222222", -] + +def is_na_label(value: str) -> bool: + """Whether a styles or settings key names the web's one N/A category. + + Every spelling the web reads as missing (:func:`is_missing_value`): a v3 + bundle's missing cell decodes as ``""``, a Parquet NULL as ``"None"``, and a + cell may say ``N/A`` or ``null`` outright. Plus ``""``, the ``str()`` of a + pandas missing value, which styles files from earlier templates carry. + """ + return value == "" or is_missing_value(value) + + +def _fold_na(by_key: dict) -> dict: + """Key the entries of every N/A spelling as one, under :data:`NA_INTERNAL`. + + The web draws them all as one legend entry, so one value has to win: + ``__NA__`` itself (what the web stored), then the other spellings in sorted + order. + """ + folded = {k: v for k, v in by_key.items() if not is_na_label(k)} + na_keys = [k for k in by_key if is_na_label(k)] + if na_keys: + winner = min(na_keys, key=lambda k: (k != NA_INTERNAL, k)) + folded[NA_INTERNAL] = by_key[winner] + return folded + + +def _fold_na_list(values: list[str]) -> list[str]: + """*values* with the N/A spellings named :data:`NA_INTERNAL`, once, where the + first of them stood.""" + out: list[str] = [] + for value in values: + if not is_na_label(value): + out.append(value) + elif NA_INTERNAL not in out: + out.append(NA_INTERNAL) + return out def _hex_to_rgba(hex_color: str, alpha: float = 0.8) -> str: @@ -214,7 +230,7 @@ def _sort_values_for_zorder( ) -> list[str]: """Return *values* ordered according to *sort_mode*. - NA-like values (:data:`_NA_LABELS`) are always placed last regardless of + N/A values (:func:`is_na_label`) are always placed last regardless of sort mode. Args: @@ -224,8 +240,8 @@ def _sort_values_for_zorder( frequencies: Mapping ``{value: count}``. Required for size-based modes; ignored for alphabetical / manual modes. """ - regular = sorted(v for v in values if v not in _NA_LABELS) - na_present = sorted(v for v in values if v in _NA_LABELS) + regular = sorted(v for v in values if not is_na_label(v)) + na_present = sorted(v for v in values if is_na_label(v)) if sort_mode in ("size-desc", "size-asc") and frequencies: regular.sort( @@ -277,13 +293,15 @@ def visualization_state_to_settings( settings: dict = {} for annotation_name in all_annotations: - colors = annotation_colors.get(annotation_name, {}) - shapes = marker_shapes.get(annotation_name, {}) + # The web draws every N/A spelling as one category, keyed __NA__, so the + # styles, stored categories and counts of all of them are one entry here. + colors = _fold_na(annotation_colors.get(annotation_name, {})) + shapes = _fold_na(marker_shapes.get(annotation_name, {})) # Start from existing settings if available if existing and annotation_name in existing: ann_settings = dict(existing[annotation_name]) - existing_categories = dict(ann_settings.get("categories", {})) + existing_categories = _fold_na(ann_settings.get("categories", {})) else: ann_settings = { "includeShapes": bool(shapes), @@ -291,7 +309,7 @@ def visualization_state_to_settings( "sortMode": "size-desc", "hiddenValues": [], "enableDuplicateStackUI": False, - "selectedPaletteId": "kellys", + "selectedPaletteId": DEFAULT_CATEGORICAL_PALETTE_ID, } existing_categories = {} @@ -309,6 +327,9 @@ def visualization_state_to_settings( for key, val in style_overrides[annotation_name].items(): if key in _SETTINGS_KEYS: ann_settings[key] = val + # Overridden or already stored: the web hides N/A by its one key. + if ann_settings.get("hiddenValues"): + ann_settings["hiddenValues"] = _fold_na_list(ann_settings["hiddenValues"]) # Pop processing-only keys (not stored in output settings). # "zOrderSort" overrides "sortMode" for zOrder computation only @@ -320,6 +341,11 @@ def visualization_state_to_settings( pinned_values: list[str] = ann_settings.pop("pinnedValues", None) or [] sort_mode = zorder_sort or ann_settings.get("sortMode", "size-desc") freqs = value_frequencies.get(annotation_name) if value_frequencies else None + if freqs: + na_count = sum(n for v, n in freqs.items() if is_na_label(v)) + freqs = {v: n for v, n in freqs.items() if not is_na_label(v)} + if na_count: + freqs[NA_INTERNAL] = na_count # Build categories from colors, shapes, and (if needed) frequency data. # When style_overrides reference an annotation with no pre-existing @@ -330,7 +356,7 @@ def visualization_state_to_settings( all_values = set(freqs.keys()) if pinned_values: - # Resolve pinned values against all_values (with NA alias matching). + # Resolve pinned values against all_values (N/A spellings folded). # ONLY pinned values go into categories — this ensures the # frontend's _visibleValues matches exactly the pinned set. # @@ -340,6 +366,7 @@ def visualization_state_to_settings( max_vis = ann_settings.get("maxVisibleValues", 10) # First pass: collect explicit (non-REST) pinned values + pinned_values = _fold_na_list(pinned_values) explicit_pinned: list[str] = [] has_rest = False for pv in pinned_values: @@ -347,16 +374,11 @@ def visualization_state_to_settings( has_rest = True elif pv in all_values: explicit_pinned.append(pv) - elif pv in _NA_LABELS: - for alias in _NA_LABELS: - if alias in all_values: - explicit_pinned.append(alias) - break if has_rest and freqs: # Auto-fill: pick top values by sort_mode, excluding already-pinned and NA candidates = _sort_values_for_zorder( - all_values - _NA_LABELS - set(explicit_pinned), + all_values - {NA_INTERNAL} - set(explicit_pinned), sort_mode, freqs, ) @@ -372,11 +394,6 @@ def visualization_state_to_settings( valid_pinned.extend(rest_values) elif pv in all_values: valid_pinned.append(pv) - elif pv in _NA_LABELS: - for alias in _NA_LABELS: - if alias in all_values: - valid_pinned.append(alias) - break else: valid_pinned = explicit_pinned @@ -394,7 +411,7 @@ def visualization_state_to_settings( cat["color"] = _rgba_to_hex(color) if "rgba" in str(color) else color elif pinned_values: # Auto-assign Kelly's palette colors for pinned values - if value in _NA_LABELS: + if value == NA_INTERNAL: cat["color"] = NA_PINNED_COLOR else: cat["color"] = KELLYS_COLORS[color_slot % len(KELLYS_COLORS)] @@ -403,15 +420,7 @@ def visualization_state_to_settings( shape = shapes.get(value, cat.get("shape", "circle")) cat["shape"] = shape - # Use __NA__ as the key for NA values (frontend internal format) - key = NA_INTERNAL if value in _NA_LABELS else value - categories[key] = cat - - if not pinned_values: - # Ensure / empty values always get light gray - for na_label in ("", "", "NaN"): - if na_label in categories: - categories[na_label].setdefault("color", NA_COLOR) + categories[value] = cat ann_settings["categories"] = categories # Set maxVisibleValues to actual category count diff --git a/apps/protspace/src/protspace/stats/carriage.py b/apps/protspace/src/protspace/stats/carriage.py index 9affce705..37a257e80 100644 --- a/apps/protspace/src/protspace/stats/carriage.py +++ b/apps/protspace/src/protspace/stats/carriage.py @@ -126,6 +126,9 @@ def build_cluster_legend_settings(report: StatsReport, columns=None) -> dict: that actually landed values in the annotations frame), so a column dropped for an id mismatch is not given a phantom legend. """ + from protspace._generated.shared_constants import ( + DEFAULT_CATEGORICAL_PALETTE_ID, + ) from protspace.data.io.settings_converter import KELLYS_COLORS allowed = None if columns is None else set(columns) @@ -156,7 +159,7 @@ def build_cluster_legend_settings(report: StatsReport, columns=None) -> dict: "sortMode": "size-desc", "hiddenValues": [], "enableDuplicateStackUI": False, - "selectedPaletteId": "kellys", + "selectedPaletteId": DEFAULT_CATEGORICAL_PALETTE_ID, "categories": categories, } return settings diff --git a/apps/protspace/src/protspace/utils/add_annotation_style.py b/apps/protspace/src/protspace/utils/add_annotation_style.py index bcf4b402a..e804ae9fc 100644 --- a/apps/protspace/src/protspace/utils/add_annotation_style.py +++ b/apps/protspace/src/protspace/utils/add_annotation_style.py @@ -3,19 +3,24 @@ from collections.abc import Iterable from pathlib import Path +from protspace._generated.shared_constants import ( + CATEGORICAL_PALETTE_IDS, + DEFAULT_CATEGORICAL_PALETTE_ID, + GRADIENT_PALETTE_IDS, + NA_DEFAULT_COLOR, + POINT_SHAPES, +) from protspace.data.annotations.encoding import to_display_value +from protspace.data.io.settings_converter import ( + NA_INTERNAL, + is_na_label, + visualization_state_to_settings, +) from protspace.utils.arrow_reader import ArrowReader logger = logging.getLogger(__name__) -ALLOWED_SHAPES = [ - "circle", - "square", - "diamond", - "triangle-up", - "triangle-down", - "plus", -] +ALLOWED_SHAPES = list(POINT_SHAPES) # Settings-level keys that are passed through to the bundle settings # (everything except "colors" and "shapes" which are applied via the reader). @@ -31,15 +36,11 @@ "pinnedValues", } -# Built-in palette IDs, mirrored from the web frontend — the source of truth: -# packages/utils/src/visualization/color-scheme.ts (COLOR_SCHEMES) and -# numeric-binning.ts (GRADIENT_COLOR_SCHEME_IDS). `selectedPaletteId` may only be a -# categorical id; numeric gradients are chosen in the UI (see https://protspace.app/docs/guide/styling). -# Keep this list in sync with those files. -_CATEGORICAL_PALETTE_IDS = frozenset( - {"kellys", "okabeIto", "tolBright", "set2", "dark2", "tableau10"} -) -_GRADIENT_PALETTE_IDS = frozenset({"batlow", "viridis", "cividis", "inferno", "plasma"}) +# Built-in palette IDs, shared with the web app through +# contracts/shared-constants.json. `selectedPaletteId` may only be a categorical id; +# numeric gradients are chosen in the UI (see https://protspace.app/docs/guide/styling). +_CATEGORICAL_PALETTE_IDS = frozenset(CATEGORICAL_PALETTE_IDS) +_GRADIENT_PALETTE_IDS = frozenset(GRADIENT_PALETTE_IDS) def load_annotation_styles( @@ -80,26 +81,19 @@ def detect_data_format(input_path: str) -> str: ) -_NA_LABELS = {"", "", "NaN", "__NA__", "None"} - - def _resolve_na(value: str, all_values: set[str]) -> str | None: - """If *value* is an NA-like label, return the matching label in *all_values*. - - Different data sources represent missing values as ``""``, ``""``, - ``"NaN"``, or the frontend's ``"__NA__"`` sentinel; a v3 bundle's missing - cell decodes as ``""``. ``"None"`` is what ``str()`` yields for a parquet - NULL, which is how a v2 bundle exported by a web build before v3 stored a - missing categorical cell. This helper maps between them so styles using - one form still work when the data uses another. Returns *None* when no - match is found. + """If *value* names N/A, return the N/A spelling *all_values* holds. + + The web draws every missing spelling as one N/A category + (:func:`~protspace.data.io.settings_converter.is_na_label`): a v3 bundle's + missing cell decodes as ``""``, a Parquet NULL as ``"None"``, and a cell may + say ``N/A`` outright. A styles file may use any of them. The first spelling + in sorted order, so the answer does not depend on set order; *None* when the + column has no missing cell. """ - if value not in _NA_LABELS: + if not is_na_label(value): return None - for candidate in _NA_LABELS: - if candidate in all_values: - return candidate - return None + return min((v for v in all_values if is_na_label(v)), default=None) def _resolve_numeric(value: str, all_values: set[str]) -> str | None: @@ -253,9 +247,10 @@ def _warn_if_bad_palette(annotation: str, styles: dict) -> None: reason = f"'{palette}' is not a known palette" logger.warning( "Annotation '%s': selectedPaletteId %s; the frontend will fall back to " - "'kellys'. Categorical palettes: %s. See https://protspace.app/docs/guide/styling#color-palettes.", + "'%s'. Categorical palettes: %s. See https://protspace.app/docs/guide/styling#color-palettes.", annotation, reason, + DEFAULT_CATEGORICAL_PALETTE_ID, ", ".join(sorted(_CATEGORICAL_PALETTE_IDS)), ) @@ -264,8 +259,9 @@ def generate_template(input_file: str) -> dict: """Generate a pre-filled styles template from an input file. Reads annotations, computes value frequencies, and outputs a template - with values in frequency-descending order. ```` gets a default - light-gray color. + with values in frequency-descending order. N/A, whatever the data spells + it, is one ``__NA__`` entry, last, with the web's default N/A colour, + ``NA_DEFAULT_COLOR``. """ detect_data_format(input_file) reader = ArrowReader(Path(input_file)) @@ -275,24 +271,27 @@ def generate_template(input_file: str) -> dict: for annotation in sorted(reader.get_all_annotations()): freqs = frequencies.get(annotation, {}) - _warn_if_numeric(annotation, freqs.keys()) + numeric = _warn_if_numeric(annotation, freqs.keys()) # Sort values by frequency descending sorted_values = sorted( freqs.keys(), key=lambda v: freqs.get(v, 0), reverse=True ) - colors: dict[str, str] = {} - for value in sorted_values: - if value in ("", "NaN", ""): - colors[value] = "#C0C0C0" - else: - colors[value] = "" + # Every N/A spelling is the web's one N/A entry: listed once, last, as + # the legend places it, in the colour the web gives an unstyled N/A. + colors: dict[str, str] = {v: "" for v in sorted_values if not is_na_label(v)} + if any(is_na_label(v) for v in sorted_values): + colors[NA_INTERNAL] = NA_DEFAULT_COLOR template[annotation] = { "sortMode": "size-desc", "maxVisibleValues": 10, "shapeSize": 10, - "selectedPaletteId": "kellys", + # A numeric column reads selectedPaletteId as its gradient, where the + # categorical default would reset any gradient chosen in the web app. + **( + {} if numeric else {"selectedPaletteId": DEFAULT_CATEGORICAL_PALETTE_ID} + ), "hiddenValues": [], "colors": colors, "shapes": {}, @@ -314,8 +313,6 @@ def _styled_settings( ``selectedPaletteId``) alongside ``colors`` and ``shapes``; these are forwarded to the settings converter. """ - from protspace.data.io.settings_converter import visualization_state_to_settings - # Existing settings (if any), to preserve extra fields. From the reader, # not the file again: this command reads its input once (and so logs a # legacy-format warning once). @@ -352,15 +349,19 @@ def _styled_settings( if overrides: style_overrides[annotation] = overrides + # An N/A style goes under __NA__, the key the stored settings already + # use, so it replaces the stored N/A style rather than competing with it. if "colors" in styles: for value, color in styles["colors"].items(): resolved = _resolve_style_value(str(value), all_values, annotation) - reader.update_annotation_color(annotation, resolved, color) + key = NA_INTERNAL if is_na_label(resolved) else resolved + reader.update_annotation_color(annotation, key, color) if "shapes" in styles: for value, shape in styles["shapes"].items(): resolved = _resolve_style_value(str(value), all_values, annotation) - reader.update_marker_shape(annotation, resolved, shape) + key = NA_INTERNAL if is_na_label(resolved) else resolved + reader.update_marker_shape(annotation, key, shape) # Convert updated visualization_state back to settings_json viz_state = reader.data.get("visualization_state", {}) diff --git a/apps/protspace/tests/test_annotation_manager.py b/apps/protspace/tests/test_annotation_manager.py index 67f52d7cd..d31b88ff1 100644 --- a/apps/protspace/tests/test_annotation_manager.py +++ b/apps/protspace/tests/test_annotation_manager.py @@ -364,6 +364,41 @@ def test_transform_protein_families_with_semicolon(self): class TestIntegration: """Integration tests for complete workflows.""" + @patch("protspace.data.annotations.manager.BiocentralPredictionRetriever") + @patch("protspace.data.annotations.manager.InterProRetriever") + def test_sequence_precedence_is_source_specific( + self, mock_interpro_retriever, mock_biocentral_retriever + ): + """InterPro hashes canonical sequences; Biocentral predicts submitted ones.""" + manager = ProteinAnnotationManager( + headers=["P12345", "custom-protein"], + annotations=["pfam", "predicted_transmembrane"], + sequences={"P12345": "LOCAL", "custom-protein": "CUSTOM"}, + ) + uniprot_annotations = [ + ProteinAnnotations( + identifier="P12345", annotations={"sequence": "CANONICAL"} + ), + ProteinAnnotations(identifier="custom-protein", annotations={}), + ] + mock_interpro_retriever.return_value.fetch_annotations.return_value = [] + mock_interpro_retriever.return_value.failed_batch_count = 0 + mock_biocentral_retriever.return_value.fetch_annotations.return_value = [] + mock_biocentral_retriever.return_value.prediction_failed = False + + manager._fetch_interpro(uniprot_annotations, []) + manager._fetch_biocentral(uniprot_annotations, []) + + assert manager.incomplete_sources == set() + assert mock_interpro_retriever.call_args.kwargs["sequences"] == { + "P12345": "CANONICAL", + "custom-protein": "CUSTOM", + } + assert mock_biocentral_retriever.call_args.kwargs["sequences"] == { + "P12345": "LOCAL", + "custom-protein": "CUSTOM", + } + @pytest.mark.parametrize( "uniprot_length,sequence,expected", [ diff --git a/apps/protspace/tests/test_biocentral_retriever.py b/apps/protspace/tests/test_biocentral_retriever.py index 3c87dc169..a364eb3a7 100644 --- a/apps/protspace/tests/test_biocentral_retriever.py +++ b/apps/protspace/tests/test_biocentral_retriever.py @@ -5,6 +5,7 @@ import pandas as pd import pytest import requests +from biocentral_api import Prediction from protspace.core.constants import is_missing_value, standardize_missing from protspace.data.annotations.retrievers.biocentral_retriever import ( @@ -15,11 +16,16 @@ def _make_prediction(model_name, value): - """Build a mock Prediction object.""" - pred = MagicMock() - pred.model_name = model_name - pred.value = value - return pred + """Build the generated model returned by biocentral-api. + + The adapter only reads ``model_name`` and ``value``. + """ + return Prediction( + model_name=model_name, + prediction_name="test", + protocol="test", + value=value, + ) class TestBiocentralConstants: @@ -75,6 +81,11 @@ def test_no_transmembrane(self): result = BiocentralPredictionRetriever._extract_transmembrane(preds) assert result == "non-transmembrane" + def test_dot_label_is_non_transmembrane(self): + preds = [_make_prediction("TMbed", "........")] + result = BiocentralPredictionRetriever._extract_transmembrane(preds) + assert result == "non-transmembrane" + def test_lowercase_labels(self): """TMbed uses lowercase h/b for non-TM side of helix/strand.""" preds = [_make_prediction("TMbed", "ooohhHHHHHhhoooo")] @@ -86,17 +97,6 @@ def test_no_tmbed_prediction(self): result = BiocentralPredictionRetriever._extract_transmembrane(preds) assert result == "" - @pytest.mark.parametrize("value", ["", None], ids=["empty", "none"]) - def test_an_empty_payload_is_missing_not_negative(self, value): - """A TMbed entry with no topology predicted nothing. - - Before the negative got a name it came out as `none`, which displays as - N/A; `non-transmembrane` would turn it into a confident negative. - """ - preds = [_make_prediction("TMbed", value)] - result = BiocentralPredictionRetriever._extract_transmembrane(preds) - assert result == "" - class TestNoTransmembraneIsACategory: """A protein without a TM segment is a prediction, not a missing value. @@ -119,6 +119,24 @@ def test_the_web_app_keeps_it(self): assert not is_missing_value(self.LABEL) +class TestTmbedPayloadValidation: + @pytest.mark.parametrize( + "value", + ["", None, 0, [], {}, " ", b"abc", "garbage"], + ids=["empty", "none", "zero", "list", "dict", "blank", "bytes", "labels"], + ) + def test_an_unusable_payload_is_missing_not_negative(self, value): + """A TMbed entry with no usable topology predicted nothing. + + Before the negative got a name it came out as `none`, which displays as + N/A; `non-transmembrane`, or signal peptide `False`, would turn it into a + confident negative. + """ + preds = [_make_prediction("TMbed", value)] + assert BiocentralPredictionRetriever._extract_transmembrane(preds) == "" + assert BiocentralPredictionRetriever._extract_signal_peptide(preds) == "" + + class TestPerSequenceExtraction: """Test per-sequence prediction extraction.""" diff --git a/apps/protspace/tests/test_legacy_cache_refresh.py b/apps/protspace/tests/test_legacy_cache_refresh.py index b24a7c2bd..9caf2e8ab 100644 --- a/apps/protspace/tests/test_legacy_cache_refresh.py +++ b/apps/protspace/tests/test_legacy_cache_refresh.py @@ -226,6 +226,32 @@ def test_pfam_refresh_fetches_sequences_the_cache_lacks( assert calls[1] == ("interpro", {"P01308": "MALWMRLLPL"}) assert result["pfam"].tolist() == [CURRENT_PFAM] + def test_the_uniprot_pass_for_sequences_refreshes_its_stale_family_too( + self, tmp_path, monkeypatch + ): + """UniProt is refetched whole for the sequences, so the cached legacy + ``protein_families`` comes back current rather than being dropped and + costing the next run that asks for it a second UniProt pass. + """ + _write_v1_cache(tmp_path) + cache = _read_cache(tmp_path).drop(columns=["sequence"]) + cache.attrs = {ANNOTATION_CACHE_VERSION_ATTR: 1} + cache.to_parquet(tmp_path / CACHE_NAME, index=False) + calls: list = [] + _serve_uniprot(monkeypatch, calls) + _serve_interpro(monkeypatch, calls) + + result = _pipeline(tmp_path, ["pfam"])._fetch_annotations(["P01308"]) + + assert "protein_families" not in result.columns + assert _read_cache(tmp_path)["protein_families"].tolist() == [CURRENT_FAMILY] + + calls.clear() + again = _pipeline(tmp_path, ["protein_families"])._fetch_annotations(["P01308"]) + + assert calls == [] + assert again["protein_families"].tolist() == [CURRENT_FAMILY] + class TestUnrequestedColumnsAreDropped: def test_a_run_served_from_the_cache_fetches_nothing(self, tmp_path, monkeypatch): @@ -420,9 +446,9 @@ def test_every_interpro_column_is_stale_in_a_v1_cache(column): "predicted_signal_peptide": "False", "predicted_transmembrane": "non-transmembrane", } +# No InterPro column: a v2 cache's are stale under v4 as well (TestVersion4). V3_ANNOTATIONS = [ "gene_name", - "pfam", "ted_domains", "root", "species", @@ -564,7 +590,6 @@ def test_a_v2_cache_refetches_taxonomy_and_biocentral_once_and_nothing_else( assert result["predicted_transmembrane"].tolist() == ["non-transmembrane"] # Every other source's value is the cached one. assert result["gene_name"].tolist() == ["CACHED_GENE"] - assert result["pfam"].tolist() == [CURRENT_PFAM] assert result["ted_domains"].tolist() == ["3.40.50.2000|94.2"] cache = _read_cache(tmp_path) assert read_annotation_cache_version(cache) == ANNOTATION_CACHE_VERSION @@ -824,6 +849,94 @@ def test_a_default_run_refetches_no_source_it_does_not_ask_for( assert not _holds_stale_value_as_current(cache, "predicted_transmembrane") +# --- Cache version 4: empty or malformed TMbed payloads ---------------------- +# +# Version 3 still wrote signal peptide "False" for a TMbed prediction with an +# empty topology, a confident negative where nothing was predicted, and read a +# malformed payload as a topology. Biocentral owns both TMbed columns, so it is +# the only source the refresh refetches. + + +class TestVersion4: + def test_version_4_covers_both_tmbed_columns_and_every_interpro_column(self): + assert CACHE_SEMANTICS_CHANGES[4] == frozenset( + {"predicted_signal_peptide", "predicted_transmembrane"} + ) | frozenset(INTERPRO_ANNOTATIONS) + + def test_a_v3_cache_refetches_interpro_once_and_reuses_uniprot( + self, tmp_path, monkeypatch + ): + """v3 looked InterPro up by the FASTA's sequence when one was given; v4 + looks up UniProt's canonical one, so a cached match may be the wrong one.""" + _write_v1_cache(tmp_path) + cache = _read_cache(tmp_path) + cache.attrs = {ANNOTATION_CACHE_VERSION_ATTR: 3} + cache.to_parquet(tmp_path / CACHE_NAME, index=False) + calls: list = [] + _forbid(monkeypatch, UniProtRetriever, "UniProt") + _serve_interpro(monkeypatch, calls) + annotations = ["gene_name", "pfam"] + + result = _pipeline(tmp_path, annotations)._fetch_annotations(["P01308"]) + + assert calls == [("interpro", {"P01308": "MALWMRLLPL"})] + assert result["pfam"].tolist() == [CURRENT_PFAM] + assert result["gene_name"].tolist() == ["CACHED_GENE"] + cache = _read_cache(tmp_path) + assert read_annotation_cache_version(cache) == ANNOTATION_CACHE_VERSION + + calls.clear() + _pipeline(tmp_path, annotations)._fetch_annotations(["P01308"]) + + assert calls == [] + + def test_a_v3_cache_refetches_biocentral_once_and_nothing_else( + self, tmp_path, monkeypatch + ): + # "True" stands in for whatever v3 derived; the refetch answers "False". + _write_v2_cache(tmp_path, version=3, predicted_signal_peptide="True") + calls: list = [] + _forbid_all_but_taxonomy_and_biocentral(monkeypatch) + _serve_taxonomy(monkeypatch, calls) + _serve_biocentral(monkeypatch, calls) + annotations = ["gene_name", "root", "predicted_signal_peptide"] + + result = _pipeline(tmp_path, annotations)._fetch_annotations(["P02299"]) + + assert calls == [("biocentral", {"P02299": "MARTKQTARK"})] + assert result["predicted_signal_peptide"].tolist() == ["False"] + assert result["gene_name"].tolist() == ["CACHED_GENE"] + cache = _read_cache(tmp_path) + assert read_annotation_cache_version(cache) == ANNOTATION_CACHE_VERSION + + # Stamped current, so the next run reuses it without a request. + calls.clear() + _pipeline(tmp_path, annotations)._fetch_annotations(["P02299"]) + + assert calls == [] + + def test_the_other_tmbed_column_is_refreshed_with_it_not_dropped( + self, tmp_path, monkeypatch + ): + """Biocentral is refetched for one stale column, so the other comes along.""" + _write_v2_cache(tmp_path, version=3, predicted_signal_peptide="True") + calls: list = [] + requested: list = [] + _forbid_all_but_taxonomy_and_biocentral(monkeypatch) + _serve_biocentral(monkeypatch, calls, requested=requested) + + result = _pipeline( + tmp_path, ["gene_name", "predicted_transmembrane"] + )._fetch_annotations(["P02299"]) + + assert _sources(calls) == ["biocentral"] + assert "predicted_signal_peptide" in requested[0] + assert "predicted_signal_peptide" not in result.columns + cache = _read_cache(tmp_path) + assert cache["predicted_signal_peptide"].tolist() == ["False"] + assert read_annotation_cache_version(cache) == ANNOTATION_CACHE_VERSION + + class TestBiocentralNeedsASequence: """Biocentral is looked up by sequence, like InterPro, on any run. diff --git a/apps/protspace/tests/test_missing_values.py b/apps/protspace/tests/test_missing_values.py index c982b00ad..79cd374fc 100644 --- a/apps/protspace/tests/test_missing_values.py +++ b/apps/protspace/tests/test_missing_values.py @@ -18,6 +18,7 @@ import pandas as pd import pytest +from protspace._generated.shared_constants import NA_DEFAULT_COLOR from protspace.core.constants import ( MISSING_VALUE_TOKENS, is_missing_value, @@ -36,8 +37,15 @@ from protspace.data.annotations.transformers.uniprot_transforms import ( UniProtTransformer, ) +from protspace.data.io.bundle import read_settings_from_bundle, write_bundle +from protspace.data.io.settings_converter import NA_INTERNAL, NA_PINNED_COLOR from protspace.data.parsers.uniprot_parser import UniProtEntry +from protspace.data.processors.base_processor import BaseProcessor from protspace.stats.annotation_select import build_annotation_labels +from protspace.utils.add_annotation_style import ( + add_annotation_styles, + generate_template, +) PACKAGE = Path(__file__).resolve().parents[1] / "src" / "protspace" @@ -132,6 +140,115 @@ def test_stats_leaves_every_missing_cell_out_of_the_categories(): assert sorted(labels.values()) == ["a", "a", "b", "b"] +def _bundle(tmp_path, organism: list, settings: dict | None = None) -> Path: + ids = [f"P{i}" for i in range(len(organism))] + tables = BaseProcessor({}, {}).create_output( + pd.DataFrame({"identifier": ids, "organism": organism}), + [ + { + "name": "PCA_2", + "dimensions": 2, + "info": {}, + "data": np.zeros((len(ids), 2)), + } + ], + ids, + ) + path = tmp_path / "in.parquetbundle" + write_bundle( + [ + tables["protein_annotations"], + tables["projections_metadata"], + tables["projections_data"], + ], + path, + settings=settings, + ) + return path + + +def _styled(bundle: Path, styles: dict) -> dict: + """The `organism` settings `protspace style` writes for *styles*.""" + out = bundle.with_name("styled.parquetbundle") + add_annotation_styles(str(bundle), {"organism": styles}, str(out)) + return read_settings_from_bundle(out)["organism"] + + +@pytest.mark.parametrize( + ("cell", "key"), + [("N/A", "N/A"), ("null", NA_INTERNAL), ("none", "none"), (None, "N/A")], + ids=["N/A-as-itself", "null-as-__NA__", "none-as-itself", "null-cell-as-N/A"], +) +def test_style_puts_an_na_colour_on_the_web_na_category(tmp_path, cell, key): + # The web draws every missing cell as one legend entry and looks its style + # up under __NA__; a style stored under the cell's own spelling never applies. + bundle = _bundle(tmp_path, ["A", cell, "B", "A"]) + + settings = _styled(bundle, {"colors": {key: "#FF0000", "A": "#00FF00"}}) + + assert settings["categories"].keys() == {"A", NA_INTERNAL} + assert settings["categories"][NA_INTERNAL]["color"] == "#FF0000" + + +@pytest.mark.parametrize("key", [NA_INTERNAL, "N/A", ""]) +def test_restyling_na_replaces_the_stored_colour(tmp_path, key): + stored = { + "categories": { + NA_INTERNAL: {"color": "#111111", "shape": "circle", "zOrder": 1}, + "A": {"color": "#00FF00", "shape": "circle", "zOrder": 0}, + } + } + bundle = _bundle(tmp_path, ["A", None, "A"], settings={"organism": stored}) + + settings = _styled(bundle, {"colors": {key: "#FF0000"}}) + + assert settings["categories"][NA_INTERNAL]["color"] == "#FF0000" + + +def test_restyling_what_style_wrote_keeps_one_na_category(tmp_path): + bundle = _bundle(tmp_path, ["A", None]) + first = tmp_path / "first.parquetbundle" + add_annotation_styles( + str(bundle), + {"organism": {"colors": {"": "#111111", "A": "#AAAAAA"}}}, + str(first), + ) + + categories = _styled(first, {"colors": {"": "#222222"}})["categories"] + + assert categories[NA_INTERNAL]["color"] == "#222222" + assert sorted(cat["zOrder"] for cat in categories.values()) == [0, 1] + + +def test_style_hides_and_pins_na_by_any_spelling(tmp_path): + bundle = _bundle(tmp_path, ["A", "N/A", "B", "A", "none"]) + + settings = _styled( + bundle, + { + "sortMode": "manual", + "maxVisibleValues": 2, + "pinnedValues": ["A", "N/A", "none"], + "hiddenValues": ["N/A", "", "none"], + }, + ) + + assert settings["hiddenValues"] == [NA_INTERNAL] + categories = settings["categories"] + assert categories.keys() == {"A", NA_INTERNAL} + assert categories[NA_INTERNAL]["zOrder"] == 1 + assert categories[NA_INTERNAL]["color"] == NA_PINNED_COLOR + + +def test_style_template_lists_na_once(tmp_path): + template = generate_template(str(_bundle(tmp_path, ["A", None, "N/A", "null"]))) + + colors = template["organism"]["colors"] + assert list(colors) == ["A", NA_INTERNAL] + # The web's own N/A swatch, so a filled-in template looks like the default. + assert colors[NA_INTERNAL] == NA_DEFAULT_COLOR + + # --------------------------------------------------------------------------- # No producer writes a category spelled like a missing value # --------------------------------------------------------------------------- diff --git a/apps/protspace/tests/test_settings_converter.py b/apps/protspace/tests/test_settings_converter.py index 9d8813292..686341fc3 100644 --- a/apps/protspace/tests/test_settings_converter.py +++ b/apps/protspace/tests/test_settings_converter.py @@ -92,6 +92,11 @@ def test_size_asc(self, values, frequencies): result = _sort_values_for_zorder(values, "size-asc", frequencies) assert result == ["Charlie", "Alpha", "Bravo", ""] + def test_any_missing_spelling_sorts_last(self): + # The web reads every one of these as N/A, in any case. + result = _sort_values_for_zorder({"X", "N/A", "null", "A"}, "alpha-asc", None) + assert result == ["A", "X", "N/A", "null"] + def test_na_always_last(self): values = {"X", "", "", "NaN", "A"} result = _sort_values_for_zorder(values, "alpha-asc", None) @@ -180,7 +185,7 @@ def test_basic_roundtrip(self): assert cat["shape"] == "circle" assert "zOrder" in cat - def test_na_gets_gray(self): + def test_na_color_is_stored_under_the_na_key(self): viz = { "annotation_colors": { "ann": {"val": "rgba(255,0,0,0.8)", "": "rgba(192,192,192,0.8)"} @@ -192,6 +197,56 @@ def test_na_gets_gray(self): # NA values are stored under __NA__ key (frontend internal format) assert cats["__NA__"]["color"] == "#C0C0C0" + def test_every_na_spelling_is_one_na_category(self): + # The web draws all of these as its one N/A entry, keyed __NA__; the + # stored __NA__ wins over a spelling of it. + viz = { + "annotation_colors": { + "ann": { + "A": "#00FF00", + "N/A": "#FF0000", + "": "#0000FF", + "__NA__": "#111111", + "": "#222222", + } + }, + "marker_shapes": {"ann": {"null": "diamond"}}, + } + cats = visualization_state_to_settings(viz)["ann"]["categories"] + assert cats.keys() == {"A", "__NA__"} + assert cats["__NA__"]["color"] == "#111111" + assert cats["__NA__"]["shape"] == "diamond" + assert cats["__NA__"]["zOrder"] == 1 # last + + def test_na_spellings_count_as_one_category_when_sorting_by_size(self): + result = visualization_state_to_settings( + {"annotation_colors": {}, "marker_shapes": {}}, + value_frequencies={"ann": {"A": 3, "B": 2, "N/A": 1, "": 2}}, + style_overrides={"ann": {"sortMode": "size-desc"}}, + ) + cats = result["ann"]["categories"] + assert {k: v["zOrder"] for k, v in cats.items()} == { + "A": 0, + "B": 1, + "__NA__": 2, + } + + def test_hidden_na_is_named_as_the_web_names_it(self): + result = visualization_state_to_settings( + {"annotation_colors": {"ann": {"A": "#00FF00"}}, "marker_shapes": {}}, + style_overrides={"ann": {"hiddenValues": ["A", "N/A", "", "__NA__"]}}, + ) + assert result["ann"]["hiddenValues"] == ["A", "__NA__"] + + def test_stored_hidden_na_is_renamed_when_only_colours_change(self): + # A bundle styled before the fold stored the data's own N/A spelling. + existing = {"ann": {"hiddenValues": ["None", "A"], "categories": {}}} + result = visualization_state_to_settings( + {"annotation_colors": {"ann": {"A": "#00FF00"}}, "marker_shapes": {}}, + existing_settings=existing, + ) + assert result["ann"]["hiddenValues"] == ["__NA__", "A"] + def test_defaults_when_no_existing(self): viz = { "annotation_colors": {"ann": {"A": "rgba(255,0,0,0.8)"}}, @@ -229,6 +284,63 @@ def test_style_overrides(self): assert result["ann"]["maxVisibleValues"] == 5 +class TestMissingSpellingsAreOneCategory: + """The web shows every missing spelling as one N/A category, keyed ``__NA__``.""" + + @pytest.mark.parametrize("spelling", ["N/A", "null", "NA", " none "]) + def test_any_missing_spelling_is_keyed_na(self, spelling): + viz = { + "annotation_colors": {"ann": {"A": "#FF0000", spelling: "#010203"}}, + "marker_shapes": {}, + } + cats = visualization_state_to_settings(viz)["ann"]["categories"] + assert set(cats) == {"A", "__NA__"} + assert cats["__NA__"]["color"] == "#010203" + + def test_several_spellings_give_one_category_with_contiguous_zorder(self): + viz = { + "annotation_colors": {"ann": {"A": "#FF0000", "": "#010101"}}, + "marker_shapes": {"ann": {"N/A": "square", "null": "diamond"}}, + } + cats = visualization_state_to_settings(viz)["ann"]["categories"] + assert set(cats) == {"A", "__NA__"} + assert sorted(cat["zOrder"] for cat in cats.values()) == [0, 1] + assert cats["__NA__"]["color"] == "#010101" + # The first spelling in sorted order wins, never set-iteration order. + assert cats["__NA__"]["shape"] == "square" + + def test_the_stored_na_category_keeps_its_fields(self): + existing = {"ann": {"categories": {"__NA__": {"zOrder": 3, "shape": "square"}}}} + viz = {"annotation_colors": {"ann": {"": "#222222"}}, "marker_shapes": {}} + cats = visualization_state_to_settings(viz, existing_settings=existing)["ann"][ + "categories" + ] + assert cats["__NA__"] == {"zOrder": 0, "color": "#222222", "shape": "square"} + + def test_pinning_na_by_two_spellings_pins_it_once(self): + result = visualization_state_to_settings( + {"annotation_colors": {}, "marker_shapes": {}}, + value_frequencies={"ann": {"A": 3, "B": 2, "": 1, "N/A": 1}}, + style_overrides={"ann": {"pinnedValues": ["A", "null", ""]}}, + ) + cats = result["ann"]["categories"] + assert set(cats) == {"A", "__NA__"} + assert cats["A"]["zOrder"] == 0 + assert cats["__NA__"] == {"zOrder": 1, "color": "#DDDDDD", "shape": "circle"} + + def test_rest_fill_never_counts_na_as_a_value(self): + result = visualization_state_to_settings( + {"annotation_colors": {}, "marker_shapes": {}}, + value_frequencies={"ann": {"A": 3, "N/A": 9, "B": 2}}, + style_overrides={ + "ann": {"pinnedValues": ["__REST__", ""], "maxVisibleValues": 3} + }, + ) + cats = result["ann"]["categories"] + order = sorted(cats, key=lambda key: cats[key]["zOrder"]) + assert order == ["A", "B", "__NA__"] + + # --------------------------------------------------------------------------- # Frontend (nested) settings shape — tsenoner/protspace#303 follow-up # --------------------------------------------------------------------------- diff --git a/apps/protspace/tests/test_shared_constants.py b/apps/protspace/tests/test_shared_constants.py new file mode 100644 index 000000000..6c14fc13c --- /dev/null +++ b/apps/protspace/tests/test_shared_constants.py @@ -0,0 +1,68 @@ +"""The constants shared with the web app come from one generated module. + +``contracts/shared-constants.json`` generates ``protspace._generated.shared_constants`` +and the web's ``packages/utils/src/generated/shared-constants.ts``, and +``pnpm shared-constants:check`` fails when either is stale. These tests pin the rest: +every name the package has always exported for one of those constants still holds +the generated value, so a hand-written copy left behind by a later edit to the JSON +fails here, and the generated module stays plain data that ships inside the package. +""" + +import ast +import importlib +from pathlib import Path + +import pytest + +from protspace._generated import shared_constants as shared + +# (module, exported name, the generated value it must hold) +REEXPORTS = [ + ("protspace.core.constants", "MISSING_VALUE_TOKENS", shared.MISSING_VALUE_TOKENS), + ("protspace.data.io.settings_converter", "NA_INTERNAL", shared.NA_VALUE), + ( + "protspace.data.io.settings_converter", + "NA_PINNED_COLOR", + shared.NA_DEFAULT_COLOR, + ), + ("protspace.data.io.settings_converter", "KELLYS_COLORS", shared.KELLYS_COLORS), + ( + "protspace.utils.add_annotation_style", + "_CATEGORICAL_PALETTE_IDS", + frozenset(shared.CATEGORICAL_PALETTE_IDS), + ), + ( + "protspace.utils.add_annotation_style", + "_GRADIENT_PALETTE_IDS", + frozenset(shared.GRADIENT_PALETTE_IDS), + ), + ( + "protspace.utils.add_annotation_style", + "ALLOWED_SHAPES", + list(shared.POINT_SHAPES), + ), + ( + "protspace.data.annotations.encoding", + "ARROW_BOOLEAN_LABELS", + shared.ARROW_BOOLEAN_LABELS, + ), +] + + +@pytest.mark.parametrize( + ("module", "name", "expected"), + REEXPORTS, + ids=[f"{module}.{name}" for module, name, _ in REEXPORTS], +) +def test_exported_name_holds_the_generated_value(module, name, expected): + assert getattr(importlib.import_module(module), name) == expected + + +def test_generated_module_is_plain_data(): + # No imports: the module cannot read contracts/ (absent from an installed + # wheel) or anything else at run time. + tree = ast.parse(Path(shared.__file__).read_text()) + imports = [ + node for node in ast.walk(tree) if isinstance(node, ast.Import | ast.ImportFrom) + ] + assert imports == [] diff --git a/apps/protspace/tests/test_style_warnings.py b/apps/protspace/tests/test_style_warnings.py index 582440abc..d8398822a 100644 --- a/apps/protspace/tests/test_style_warnings.py +++ b/apps/protspace/tests/test_style_warnings.py @@ -6,17 +6,16 @@ per-value colors/shapes set via the CLI are silently dropped; - the `selectedPaletteId` validation warning (gradient/unknown id → resets to kellys in the frontend); -- a contract keeping the Python palette-id catalog equal to the frontend's, - read from its TypeScript source (see the section below). +- the palette catalog, generated from contracts/shared-constants.json like the + web's (see the section below). """ import logging -import re -from pathlib import Path import pyarrow as pa import pytest +from protspace._generated.shared_constants import DEFAULT_CATEGORICAL_PALETTE_ID from protspace.data.annotations.encoding import stamp_format_version from protspace.data.io.bundle import extract_bundle_to_dir, write_bundle from protspace.utils.add_annotation_style import ( @@ -32,8 +31,6 @@ style_keys, ) -REPO_ROOT = Path(__file__).resolve().parents[3] - def _make_bundle(tmp_path, ids, annotation_columns): """Write a minimal .parquetbundle with the given annotation columns.""" @@ -113,6 +110,23 @@ def test_generate_template_warns_only_on_numeric_column(tmp_path, caplog): assert {"length", "family"} <= set(template) +def test_template_gives_only_a_categorical_column_the_categorical_palette(tmp_path): + """A numeric column reads ``selectedPaletteId`` as its gradient, so the + categorical default would reset a gradient chosen in the web app.""" + bundle = _make_bundle( + tmp_path, + ["P1", "P2", "P3"], + { + "length": ["100", "200", "300"], + "family": ["kinase", "phosphatase", "kinase"], + }, + ) + template = generate_template(str(bundle)) + + assert template["family"]["selectedPaletteId"] == DEFAULT_CATEGORICAL_PALETTE_ID + assert "selectedPaletteId" not in template["length"] + + def test_apply_styles_warns_on_numeric_column(tmp_path, caplog): bundle = _make_bundle(tmp_path, ["P1", "P2"], {"length": ["100", "200"]}) proj_dir = extract_bundle_to_dir(bundle) @@ -190,64 +204,16 @@ def test_apply_styles_skips_palette_warning_for_numeric_gradient(tmp_path, caplo assert not any("kellys" in m for m in messages) -# --- contract: keep the Python palette catalog in sync with the frontend --- +# --- palette catalog -------------------------------------------------------- # -# The source of truth is the web frontend, read here from its source (protspace -# cannot import TypeScript): -# packages/utils/src/visualization/color-scheme.ts (COLOR_SCHEMES) -# packages/utils/src/visualization/numeric-binning.ts -# (GRADIENT_COLOR_SCHEME_IDS, DEFAULT_NUMERIC_PALETTE_ID) -# A palette added, renamed or moved between the sets there fails these tests; -# update add_annotation_style.py and docs/guide/styling.md (Color palettes) to match. - -_WEB_VISUALIZATION = REPO_ROOT / "packages" / "utils" / "src" / "visualization" - - -def _web_source_match(file_name: str, pattern: str) -> re.Match: - path = _WEB_VISUALIZATION / file_name - match = re.search(pattern, path.read_text(), re.S) - assert match, f"{pattern!r} not found in {path}" - return match - - -def _web_palette_ids() -> set[str]: - """Every key of the web's COLOR_SCHEMES.""" - block = _web_source_match( - "color-scheme.ts", r"export const COLOR_SCHEMES = \{(.*?)\} as const;" - ).group(1) - ids = set(re.findall(r"^\s*(\w+):", block, re.M)) - assert ids, f"no palette ids in COLOR_SCHEMES: {block!r}" - return ids - - -def _web_gradient_palette_ids() -> set[str]: - block = _web_source_match( - "numeric-binning.ts", r"GRADIENT_COLOR_SCHEME_IDS = new Set\(\[(.*?)\]\)" - ).group(1) - ids = set(re.findall(r"'(\w+)'", block)) - assert ids, f"no ids in GRADIENT_COLOR_SCHEME_IDS: {block!r}" - return ids - - -def test_gradient_palette_ids_match_the_web_app(): - assert set(_GRADIENT_PALETTE_IDS) == _web_gradient_palette_ids() - - -def test_categorical_palette_ids_match_the_web_app(): - # The web has no categorical list of its own: it is every scheme that is - # not a gradient. - web_categorical = _web_palette_ids() - _web_gradient_palette_ids() - assert set(_CATEGORICAL_PALETTE_IDS) == web_categorical +# The ids come from contracts/shared-constants.json, which also generates the web's; +# the generator checks the sets are disjoint and that the numeric default is a +# gradient. This pins the one id the CLI writes and the web falls back to. -def test_palette_defaults_belong_to_their_sets(): - # 'kellys' is what the CLI writes and the frontend resets to; numeric - # columns default to the web's DEFAULT_NUMERIC_PALETTE_ID. +def test_kellys_is_a_categorical_palette(): assert "kellys" in _CATEGORICAL_PALETTE_IDS - default_numeric = _web_source_match( - "numeric-binning.ts", r"DEFAULT_NUMERIC_PALETTE_ID = '(\w+)'" - ).group(1) - assert default_numeric in _GRADIENT_PALETTE_IDS + assert "kellys" not in _GRADIENT_PALETTE_IDS # --------------------------------------------------------------------------- @@ -283,8 +249,15 @@ def test_resolve_style_key_answers_what_style_would_accept(): assert resolve_style_key("__NA__", {"None", "Human"}) == "None" assert resolve_style_key("100.0", {"100"}) == "100" assert resolve_style_key("Alien", {"Human", "Mouse"}) is None - # Only the NA spellings `style` folds: "none" (TMbed) is a value of its own. - assert resolve_style_key("__NA__", {"none", "Human"}) is None + # Every spelling the web shows as N/A is N/A to `style` too. "none" was kept + # apart while TMbed wrote its negative that way, but the web always drew it + # as N/A, so a style on it never applied. + assert resolve_style_key("__NA__", {"none", "Human"}) == "none" + assert resolve_style_key("N/A", {"null", "Human"}) == "null" + assert resolve_style_key("NULL", {"none", "Human"}) == "none" + assert resolve_style_key("__NA__", {"Human"}) is None # no missing cell + # With several N/A spellings in the data, always the same one. + assert resolve_style_key("N/A", {"None", "", "n/a", "Human"}) == "" def test_style_keys_are_the_display_values_of_every_cell(): diff --git a/contracts/README.md b/contracts/README.md new file mode 100644 index 000000000..437dabf93 --- /dev/null +++ b/contracts/README.md @@ -0,0 +1,32 @@ +# Cross-language contracts + +`shared-constants.json` is the single source of the constants the Python package +(`apps/protspace`) and the web app (`packages/utils`, `packages/core`) must hold the same +value for: the missing-value tokens, the `__NA__` sentinel and its colour, Kelly's colours, +the palette ids, the point shapes and the labels of a boolean column. + +`pnpm shared-constants` writes it into two committed modules, which each side re-exports +under its existing names: + +- `packages/utils/src/generated/shared-constants.ts` +- `apps/protspace/src/protspace/_generated/shared_constants.py`. It lives inside the package + because the PyPI wheel is built from `apps/protspace` alone and cannot read this file. + +`pnpm shared-constants:check` fails when either module differs from what the JSON +generates. `pnpm precommit` runs it, and so does `bundle-contract.yml`, which runs on every +pull request. Never edit the generated modules; edit the JSON and regenerate. + +## What belongs here + +**Values only.** A constant goes here when both languages hold it and a difference would +break a bundle, its settings, or what the user sees. Each entry carries a `doc`: the reason +the value is what it is, written once for both languages. + +**Rules do not.** How a cell is matched against the missing-value tokens, how a column is +judged numeric, or which column holds the protein ids is code, and a shared list cannot +keep two implementations of it in step. Those are pinned by running both sides on the same +inputs in the bundle contract suite (`tests/contract`). + +The generator also checks invariants between values, for example that every missing-value +token is lower-case and that the numeric default is a gradient palette. Add one there when a +value must satisfy something for the code on either side to work. diff --git a/contracts/shared-constants.json b/contracts/shared-constants.json new file mode 100644 index 000000000..b2dcd4202 --- /dev/null +++ b/contracts/shared-constants.json @@ -0,0 +1,138 @@ +{ + "$comment": [ + "Constants the Python package (apps/protspace) and the web app (packages/) must agree on.", + "Edit this file, run `pnpm shared-constants`, and commit it with the two modules it", + "generates; never edit those by hand. See contracts/README.md for what belongs here." + ], + "constants": { + "MISSING_VALUE_TOKENS": { + "kind": "set", + "doc": [ + "Strings that mean \"this annotation value is missing\". A cell is missing", + "when it is null, a non-finite number, blank, or, trimmed and lower-cased,", + "one of these: `normalizeMissingValue` in the web, `is_missing_value` in", + "Python. That rule is code on each side, so the bundle contract suite runs", + "both on the same probe spellings.", + "", + "`__na__` reserves the spelling of NA_VALUE: the bundle writer drops that", + "token unconditionally (it is the sentinel a missing cell carries), so a", + "genuine category literally spelled `__NA__` would be silently deleted on", + "export. Reading it as missing at ingestion makes the two agree." + ], + "value": ["na", "n/a", "nan", "null", "none", "__na__"] + }, + "NA_VALUE": { + "kind": "string", + "doc": [ + "Internal key of the N/A category: the legend value a missing cell gets,", + "and the key its colour and shape are saved under in a bundle's settings.", + "Never displayed; the web shows it as `N/A`." + ], + "value": "__NA__" + }, + "NA_DEFAULT_COLOR": { + "kind": "string", + "doc": [ + "Default swatch colour of the N/A category, which a saved category colour", + "overrides (categorical annotations only). Python gives a pinned N/A this", + "colour, and `protspace style --generate-template` pre-fills it for N/A, so", + "a CLI-styled legend matches the web's default." + ], + "value": "#DDDDDD" + }, + "KELLYS_COLORS": { + "kind": "list", + "doc": [ + "Kelly's 21 colours of maximum contrast, in the order categories take them", + "(the category in slot i gets colour i). Medium gray (#848482) is left out,", + "too close to the greys of Other (#999999) and N/A. Very light and very", + "dark gray come last, for the least frequent categories. In order: vivid", + "yellow, strong purple, vivid orange, very light blue, vivid red, grayish", + "yellow, vivid green, strong purplish pink, strong blue, strong yellowish", + "pink, strong violet, vivid orange yellow, strong purplish red, vivid", + "greenish yellow, strong reddish brown, vivid yellowish green, deep", + "yellowish brown, vivid reddish orange, dark olive green, very light gray,", + "very dark gray.", + "", + "Reference: Kelly, Kenneth L. Twenty-two colors of maximum contrast.", + "Color Engineering 3.26-27 (1965)." + ], + "value": [ + "#F3C300", + "#875692", + "#F38400", + "#A1CAF1", + "#BE0032", + "#C2B280", + "#008856", + "#E68FAC", + "#0067A5", + "#F99379", + "#604E97", + "#F6A600", + "#B3446C", + "#DCD300", + "#882D17", + "#8DB600", + "#654522", + "#E25822", + "#2B3D26", + "#F2F3F4", + "#222222" + ] + }, + "CATEGORICAL_PALETTE_IDS": { + "kind": "list", + "doc": [ + "Palettes for categorical annotations: the only ids a bundle's", + "`selectedPaletteId` may name. `protspace style` warns on any other id, and", + "the web draws a gradient or unknown id with DEFAULT_CATEGORICAL_PALETTE_ID.", + "The web's `COLOR_SCHEMES` holds the colours and must list exactly these ids", + "plus GRADIENT_PALETTE_IDS (the type check fails otherwise). A new palette", + "also goes in docs/guide/styling.md (Color palettes)." + ], + "value": ["kellys", "okabeIto", "tolBright", "set2", "dark2", "tableau10"] + }, + "GRADIENT_PALETTE_IDS": { + "kind": "list", + "doc": [ + "Sequential palettes for numeric annotations, chosen in the web's numeric", + "legend settings rather than through `selectedPaletteId`." + ], + "value": ["viridis", "cividis", "inferno", "batlow", "plasma"] + }, + "DEFAULT_CATEGORICAL_PALETTE_ID": { + "kind": "string", + "doc": [ + "The palette a categorical annotation starts on, and the one the web falls", + "back to; one of CATEGORICAL_PALETTE_IDS." + ], + "value": "kellys" + }, + "DEFAULT_NUMERIC_PALETTE_ID": { + "kind": "string", + "doc": ["The gradient a numeric annotation starts on; one of GRADIENT_PALETTE_IDS."], + "value": "batlow" + }, + "POINT_SHAPES": { + "kind": "list", + "doc": [ + "Shapes a category can be drawn with: the `shape` values a bundle's", + "settings may hold. The web's `PointShape` type is exactly these." + ], + "value": ["circle", "square", "diamond", "triangle-up", "triangle-down", "plus"] + }, + "ARROW_BOOLEAN_LABELS": { + "kind": "list", + "doc": [ + "Labels of an Arrow `bool` annotation column, false then true. Lower case,", + "as the browser has always shown one (`String(true)`), so legend colours", + "saved against a v2 bundle keep matching. Both v3 writers store a bool", + "column under these labels, and the web's writer keeps `sourceType: bool`", + "only while every label is one of them. Not Python's CANONICAL_BOOLEANS", + "(`False`/`True`), which are string cells the pipeline writes itself." + ], + "value": ["false", "true"] + } + } +} diff --git a/docs/guide/styling.md b/docs/guide/styling.md index ee62f1fdc..cb83fb97b 100644 --- a/docs/guide/styling.md +++ b/docs/guide/styling.md @@ -30,7 +30,9 @@ workflow above is meant for categorical columns (e.g. `major_group`, `ec`, `supe it to a **numeric** column (`length`, pLDDT, a score) does not do what you expect: - `--generate-template` lists **every distinct number as its own category**, hundreds to thousands - of rows to hand-color. + of rows to hand-color. It leaves `selectedPaletteId` out of a numeric column's entry: the column + reads it as its gradient, so the categorical default would reset a gradient chosen in the web + app to `batlow`. - `colors` / `shapes` / `pinnedValues` match one existing value at a time, so a range like `"200-300"` or an interpolated `"3.5"` raises `Value '…' does not exist`. A key that is a number is resolved numerically rather than textually, so `"100.0"` still finds the value `100` in a @@ -118,11 +120,16 @@ settings, so editing the per-annotation `shapeSize` of such a bundle has no visi ### N/A values -Missing values (`""`, `""`, `"NaN"`, `"__NA__"`, `"None"`) are normalized automatically, use any -form in the styles file. A missing cell of a v3 bundle, from the web app or from Python, reads back -as `""`. `"None"` is how one reads back from a v2 bundle exported by a web build before v3, which -wrote the cell as a Parquet NULL. In the output bundle N/A is stored with the key `__NA__` (the -frontend's internal format). +Every value the web app shows as N/A is one legend entry, so it is one entry in the styles file +too. Name it by any spelling the web reads as missing (see +[Missing Values](./data-format.md#missing-values): `""`, `"N/A"`, `"None"`, `"null"`, `"NaN"`, +`"__NA__"`, in any case), or `""`, whichever the data uses. This holds for `colors`, `shapes`, +`pinnedValues` and `hiddenValues`. A missing cell of a v3 bundle, from the web app or from Python, +reads back as `""`; `"None"` is how one reads back from a v2 bundle exported by a web build before +v3, which wrote the cell as a Parquet NULL. In the output bundle N/A is stored once, with the key +`__NA__` (the frontend's internal format), and a new N/A style replaces the one already stored. +`--generate-template` lists it as `__NA__`, pre-filled with the web app's default N/A gray +(`#DDDDDD`). ### Example: custom colors and shapes @@ -133,7 +140,7 @@ frontend's internal format). "colors": { "Short-chain": "#63CBE5", "Long-chain": "#24638F", - "": "#C0C0C0" + "": "#DDDDDD" }, "shapes": { "Short-chain": "circle", @@ -207,7 +214,7 @@ with `sortMode: "manual"`. requires `maxVisibleValues: 13`. - Colors are auto-assigned from Kelly's 21 Colors of Maximum Contrast when no explicit `colors` are provided. N/A gets a lighter gray (`#DDDDDD`). -- Use `""` for N/A in `pinnedValues`. +- Name N/A in `pinnedValues` by any of its spellings (`""` or `"__NA__"` work for any data). ### `__REST__` auto-fill marker diff --git a/openspec/changes/fix-cache-ownership/.openspec.yaml b/openspec/changes/archive/2026-10-15-fix-biocentral-transmembrane-sentinel/.openspec.yaml similarity index 100% rename from openspec/changes/fix-cache-ownership/.openspec.yaml rename to openspec/changes/archive/2026-10-15-fix-biocentral-transmembrane-sentinel/.openspec.yaml diff --git a/openspec/changes/archive/2026-10-15-fix-biocentral-transmembrane-sentinel/README.md b/openspec/changes/archive/2026-10-15-fix-biocentral-transmembrane-sentinel/README.md new file mode 100644 index 000000000..5f60931b0 --- /dev/null +++ b/openspec/changes/archive/2026-10-15-fix-biocentral-transmembrane-sentinel/README.md @@ -0,0 +1,3 @@ +# fix-biocentral-transmembrane-sentinel + +Prevent valid no-transmembrane predictions from being rendered as missing. diff --git a/openspec/changes/archive/2026-10-15-fix-biocentral-transmembrane-sentinel/design.md b/openspec/changes/archive/2026-10-15-fix-biocentral-transmembrane-sentinel/design.md new file mode 100644 index 000000000..776266cd9 --- /dev/null +++ b/openspec/changes/archive/2026-10-15-fix-biocentral-transmembrane-sentinel/design.md @@ -0,0 +1,136 @@ +## Context + +The Biocentral TMbed adapter currently serializes a valid negative prediction as the +categorical string `none`. The web bundle reader applies a shared missing-value policy +to all categorical annotations, and that policy intentionally treats `none` +case-insensitively as absent data. Consequently, a successfully computed negative +TMbed prediction is displayed as `N/A`, making it indistinguishable from proteins for +which no prediction was produced. + +This behavior crosses the Python annotation producer, the parquet bundle contract, and +the TypeScript visualization consumer. The fix must preserve the generic importer's +established missing-value semantics while making newly produced TMbed data unambiguous. + +`main` shipped the `non-transmembrane` label, the empty-payload case for +`predicted_transmembrane` and FASTA sequences in `annotate` (PR 495) before this change +merged; the decisions below keep the full record, and the code adds what main lacked. + +## Goals / Non-Goals + +**Goals:** + +- Preserve a successful TMbed negative prediction as a visible categorical value in + the web application. +- Keep genuinely absent Biocentral predictions represented as missing values. +- Keep FASTA-provided sequences available to sequence-backed annotation sources even + when UniProt cannot resolve their identifiers. +- Exercise the producer value through the generated Python-to-TypeScript bundle + contract so future sentinel collisions fail in CI. +- Keep user-facing annotation documentation aligned with the emitted vocabulary. + +**Non-Goals:** + +- Changing the generic web missing-value token set. +- Rewriting or migrating already generated `.parquetbundle` files. +- Changing TMbed topology interpretation for alpha helices, beta barrels, or mixed + predictions. +- Adding contextual bundle-reader rules for one annotation name. + +## Decisions + +### Emit `non-transmembrane` for a successful negative TMbed prediction + +The Biocentral adapter will return `non-transmembrane` when a TMbed result contains +neither an alpha-helical nor a beta-barrel segment. The phrase is explicit, is not in +the consumer's missing-value token set, and does not imply that the protein is soluble +or intracellular. + +Alternatives considered: + +- Keep `none` and remove it from the generic missing-value set. This would change the + interpretation of arbitrary imported datasets and turn commonly used missing + sentinels into visible categories. +- Translate `none` only when reading `predicted_transmembrane`. This would add + domain-specific repair logic to the generic bundle consumer and would still leave + the on-disk producer contract ambiguous. +- Use `soluble`. This is biologically narrower than "no predicted transmembrane + segment" and can mislabel secreted or otherwise non-membrane proteins. + +### Preserve empty-string output for an unavailable or malformed TMbed result + +The adapter will return an empty string when Biocentral provides no TMbed prediction or +provides a TMbed prediction object whose optional `value` payload is `None`, empty, +non-string, blank, or contains unsupported labels. A usable topology is a string made +only from TMbed's `B`, `b`, `H`, `h`, `S`, `i`, `o`, and `.` labels. The payload guard +runs before topology labels are scanned. This retains the distinction at the producer: +a valid topology with no membrane segment is `non-transmembrane`, while an unavailable +or malformed topology remains missing. + +Both `predicted_signal_peptide` and `predicted_transmembrane` derive from that same +optional topology payload. A shared extractor therefore owns the missing-payload check +before either annotation interprets the topology, preventing one annotation from +inventing a completed negative result when the other reports missing data. + +### Pass FASTA sequences to the sources that consume them + +The standalone `annotate` command parses the FASTA into a canonical +identifier-to-sequence map and passes it to `ProteinAnnotationManager`. It does so +for every request, not only for sequence-backed sources, because the missing-length +fallback reads the same map (main shipped this before the change merged it). HDF5 +inputs continue to omit a local sequence map and retain their existing +UniProt-fallback behavior. + +Sequence precedence is source-specific. Biocentral predicts the sequence supplied in +the FASTA, using UniProt only as a fallback for identifiers with no local sequence. +InterPro indexes precomputed matches by sequence hash, so it uses UniProt's canonical +sequence when available and falls back to the FASTA sequence for identifiers UniProt +cannot resolve. This keeps FASTA-only identifiers annotatable without replacing valid +canonical InterPro matches when the submitted sequence differs. + +The hosted prep service already normalizes FASTA headers and supplies that normalized +file to `protspace annotate`; regression coverage pins that normalization preserves the +sequence as well as the canonical identifier. + +### Cover both the adapter and the cross-language seam + +Focused Python regressions will construct the real Biocentral `Prediction` model and +assert the adapter vocabulary for completed and absent payloads. The generated bundle +contract will derive its negative transmembrane and missing TMbed-derived fixture values +from the real adapter and assert that TypeScript preserves the former as a category and +the latter as `N/A`. This avoids duplicated hand-written constants that could allow +producer and contract fixtures to drift apart. + +### Refresh caches written before the payload validation + +An empty TMbed payload used to yield signal peptide `False`, and a malformed one was +read as a topology, so cached values of both TMbed columns written before this change +can hold a negative or a category where nothing usable was predicted. The change is +recorded as annotation cache semantics version 4 covering `predicted_signal_peptide` +and `predicted_transmembrane`, so the established legacy-cache refresh refetches them. +Because a refresh refetches its sources whole, a stale column of a source the refresh +refetches anyway is refreshed with it instead of being dropped; otherwise a run that +asks for only one of the two TMbed columns would lose the other from the cache. + +## Risks / Trade-offs + +- [Existing bundles still contain ambiguous `none` values] → Document that the fix + applies to newly generated annotations; users must regenerate an affected bundle to + recover the distinction. +- [Downstream consumers may expect the old literal] → Treat the value as a categorical + contract correction and document the new vocabulary in both CLI and web metadata. +- [Contract fixture imports more producer code] → Use a minimal synthetic TMbed + prediction and retain the real bundle CLI as the serialization boundary. +- [FASTA and UniProt sequences differ] → Keep the submitted sequence authoritative + for Biocentral predictions and the canonical sequence authoritative for InterPro's + precomputed hash lookup, with the other source as fallback in each case. + +## Migration Plan + +Deploy the producer vocabulary and web documentation together. No storage migration is +performed. Newly annotated bundles will carry `non-transmembrane`; existing bundles can +be regenerated with the updated CLI. Rollback consists of reverting the producer value +and its associated tests/documentation. + +## Open Questions + +None. diff --git a/openspec/changes/archive/2026-10-15-fix-biocentral-transmembrane-sentinel/proposal.md b/openspec/changes/archive/2026-10-15-fix-biocentral-transmembrane-sentinel/proposal.md new file mode 100644 index 000000000..b52d7c1f6 --- /dev/null +++ b/openspec/changes/archive/2026-10-15-fix-biocentral-transmembrane-sentinel/proposal.md @@ -0,0 +1,52 @@ +## Why + +Biocentral emits a valid protein-level `predicted_transmembrane` category named +`none`, but the web reader reserves `none` as a missing-value token. As a result, +proteins with a successful “no transmembrane segment” prediction are displayed as +N/A and cannot be distinguished from proteins whose prediction is actually absent. + +`main` shipped the label (`non-transmembrane`), the empty-payload case for +`predicted_transmembrane`, and FASTA sequences in `protspace annotate` (PR 495) while +this change was in review. What this change still adds: an empty or malformed TMbed +payload is missing for the signal peptide too (main still wrote `False`), a malformed +payload is never read as a topology, each sequence-backed source gets the sequence it +needs, and caches written before are refreshed. + +## What Changes + +- Encode successful TMbed predictions with no membrane-spanning segment using a + non-reserved categorical label (already on main; specified here at the bundle + boundary). +- Preserve absent or malformed Biocentral predictions, including TMbed prediction + objects whose optional payload is `None`, empty, non-string, or contains unsupported + topology labels, as missing values for both derived TMbed annotations. +- Use source-appropriate sequence precedence when both the FASTA and UniProt supply a + sequence: Biocentral predicts the submitted sequence, InterPro looks up the canonical + one. +- Record the TMbed payload change as annotation cache semantics version 4, so caches + written before it are refreshed rather than reused; a stale column of a source the + refresh refetches anyway is refreshed with it instead of being dropped. +- Add regression coverage across the Python annotation producer and TypeScript + bundle consumer boundary. + +## Capabilities + +### New Capabilities + +- `annotation-input`: Require standalone FASTA annotation inputs to retain their + sequences for sequence-backed annotation sources. + +### Modified Capabilities + +- `bundle-format-contract`: Require producer-emitted categorical values to remain + distinguishable from the consumer's missing-value sentinel set. +- `annotation-cache-semantics`: Refresh cached TMbed columns written before the payload + validation. + +## Impact + +- Python FASTA annotation input and TMbed extraction in `apps/protspace`. +- The hosted prep pipeline's normalized-FASTA handoff to `protspace annotate`. +- The generated cross-language bundle fixture and contract assertions. +- The annotation cache's legacy-refresh rules (`data/annotations/cache.py`). +- No dependency, API, or bundle-layout changes. diff --git a/openspec/changes/archive/2026-10-15-fix-biocentral-transmembrane-sentinel/specs/annotation-cache-semantics/spec.md b/openspec/changes/archive/2026-10-15-fix-biocentral-transmembrane-sentinel/specs/annotation-cache-semantics/spec.md new file mode 100644 index 000000000..12fcde3f1 --- /dev/null +++ b/openspec/changes/archive/2026-10-15-fix-biocentral-transmembrane-sentinel/specs/annotation-cache-semantics/spec.md @@ -0,0 +1,24 @@ +## ADDED Requirements + +### Requirement: Caches written before the TMbed payload validation are refreshed + +ProtSpace SHALL NOT reuse cached `predicted_signal_peptide` or `predicted_transmembrane` +values stamped version 3 or earlier. Those versions wrote signal peptide `False` for a +TMbed prediction with an empty topology and read a malformed payload as a topology. A run +that requests such a column SHALL refetch Biocentral once and stamp the rewritten cache as +current. A stale column of a source the refresh refetches anyway SHALL be refreshed with it +rather than dropped. + +#### Scenario: A version-3 cache is asked for the signal peptide + +- **WHEN** a run requests `predicted_signal_peptide` and columns from other sources from a + cache stamped version 3 +- **THEN** ProtSpace refetches Biocentral once and fetches no other source +- **AND** the rewritten cache is stamped current, so the next run fetches nothing + +#### Scenario: Only one TMbed column is requested + +- **WHEN** a run requests `predicted_transmembrane` but not `predicted_signal_peptide` from + a cache that holds both under superseded semantics +- **THEN** the one Biocentral refetch also refreshes `predicted_signal_peptide`, which stays + in the cache and out of the run's result diff --git a/openspec/changes/archive/2026-10-15-fix-biocentral-transmembrane-sentinel/specs/annotation-input/spec.md b/openspec/changes/archive/2026-10-15-fix-biocentral-transmembrane-sentinel/specs/annotation-input/spec.md new file mode 100644 index 000000000..3a765db83 --- /dev/null +++ b/openspec/changes/archive/2026-10-15-fix-biocentral-transmembrane-sentinel/specs/annotation-input/spec.md @@ -0,0 +1,22 @@ +## ADDED Requirements + +### Requirement: FASTA annotation inputs retain their sequences + +The standalone annotation producer SHALL pass sequences supplied in a FASTA input to +sequence-backed annotation sources using the same canonical identifiers as its output. + +#### Scenario: A FASTA identifier is not resolved by UniProt + +- **WHEN** `protspace annotate` receives a FASTA entry whose identifier UniProt cannot + resolve +- **THEN** the entry's FASTA sequence remains available to Biocentral and InterPro +- **AND** the hosted prep pipeline preserves that sequence when it passes its normalized + FASTA to `protspace annotate` + +#### Scenario: A FASTA sequence differs from the UniProt canonical sequence + +- **WHEN** `protspace annotate` receives a FASTA sequence for an identifier that UniProt + resolves to a different canonical sequence +- **THEN** Biocentral receives the FASTA sequence it is being asked to predict +- **AND** InterPro receives the UniProt canonical sequence used by its precomputed + sequence-hash index diff --git a/openspec/changes/archive/2026-10-15-fix-biocentral-transmembrane-sentinel/specs/bundle-format-contract/spec.md b/openspec/changes/archive/2026-10-15-fix-biocentral-transmembrane-sentinel/specs/bundle-format-contract/spec.md new file mode 100644 index 000000000..6f7ea8c99 --- /dev/null +++ b/openspec/changes/archive/2026-10-15-fix-biocentral-transmembrane-sentinel/specs/bundle-format-contract/spec.md @@ -0,0 +1,32 @@ +## ADDED Requirements + +### Requirement: Produced annotation categories do not collide with missing-value sentinels + +The Python annotation producer SHALL serialize a completed categorical prediction using +a value that the TypeScript bundle consumer preserves as a category rather than +normalizing to missing data. + +#### Scenario: Negative TMbed prediction crosses the bundle boundary + +- **WHEN** Biocentral returns a completed TMbed prediction containing neither an + alpha-helical nor a beta-barrel transmembrane segment +- **THEN** the producer emits `non-transmembrane` +- **AND** the TypeScript visualization data exposes `non-transmembrane` as a categorical + value rather than `N/A` + +#### Scenario: Missing TMbed prediction remains missing + +- **WHEN** Biocentral returns no TMbed prediction for a protein +- **THEN** the producer emits the established missing representation +- **AND** the TypeScript visualization data does not invent a transmembrane category + +#### Scenario: Empty or malformed TMbed payload remains missing + +- **WHEN** Biocentral returns a TMbed prediction object whose optional `value` payload + is `None`, empty, non-string, blank, or contains unsupported topology labels +- **THEN** the producer emits the established missing representation before scanning + topology labels +- **AND** the TypeScript visualization data exposes the protein as `N/A` rather than + `non-transmembrane` +- **AND** the derived signal-peptide annotation is also exposed as `N/A` rather than + `False` diff --git a/openspec/changes/archive/2026-10-15-fix-biocentral-transmembrane-sentinel/tasks.md b/openspec/changes/archive/2026-10-15-fix-biocentral-transmembrane-sentinel/tasks.md new file mode 100644 index 000000000..080bc5b2d --- /dev/null +++ b/openspec/changes/archive/2026-10-15-fix-biocentral-transmembrane-sentinel/tasks.md @@ -0,0 +1,112 @@ +## 1. Regression Coverage + +- [x] 1.1 Update the focused Biocentral retriever test to require the unambiguous negative TMbed category and observe it fail. +- [x] 1.2 Extend the generated Python-to-TypeScript bundle contract with a negative TMbed prediction and observe the consumer assertion fail. + +## 2. Producer and Documentation + +- [x] 2.1 Change the Biocentral TMbed adapter to emit `non-transmembrane` for a completed negative prediction. +- [x] 2.2 Update CLI and web annotation descriptions to document the corrected categorical vocabulary. + +## 3. Verification + +- [x] 3.1 Run the focused Python retriever and cross-language contract tests and confirm they pass. +- [x] 3.2 Regenerate annotation documentation and verify generated files are current. +- [x] 3.3 Run the affected Python lint/test gates and the repository-wide `pnpm precommit` gate. +- [x] 3.4 Verify a newly produced bundle displays the negative TMbed category separately from missing values. + +## 4. Review Regression Coverage + +- [x] 4.1 Add focused real-model regressions for `None` and empty TMbed payloads and observe the expected failures. +- [x] 4.2 Extend the generated bundle contract with an adapter-derived missing TMbed payload and observe the expected failure. + +## 5. Missing Payload Fix + +- [x] 5.1 Return the established missing representation before scanning absent TMbed topology payloads. + +## 6. Review Verification + +- [x] 6.1 Run the focused Biocentral and cross-language bundle contract tests and confirm they pass. +- [x] 6.2 Run Ruff, the complete non-slow ProtSpace Python suite, strict OpenSpec validation, and `pnpm precommit`. +- [x] 6.3 Prepare the verified change for the inline review follow-up and CI run. + +## 7. Follow-up Review Regression Coverage + +- [x] 7.1 Add focused real-model regressions proving missing TMbed payloads remain + missing for signal peptide and observe the expected failures. +- [x] 7.2 Extend the generated bundle contract to expose missing signal peptide as + `N/A` and observe the expected failure. +- [x] 7.3 Add CLI regression coverage proving FASTA sequences reach the annotation + manager and strengthen the hosted normalized-FASTA handoff assertion. + +## 8. Follow-up Review Fixes + +- [x] 8.1 Parse and pass FASTA sequences through `protspace annotate` while preserving + HDF5 behavior. +- [x] 8.2 Centralize optional TMbed topology extraction for both derived annotations. + +## 9. Follow-up Review Verification + +- [x] 9.1 Run focused ProtSpace, hosted-prep, and cross-language contract tests. +- [x] 9.2 Run Ruff, the complete relevant Python suites, strict OpenSpec validation, + and `pnpm precommit`. +- [x] 9.3 Prepare the verified change for the review reply and pushed CI run. + +## 10. Malformed Payload and Spec Ownership Regressions + +- [x] 10.1 Add real-model regressions proving malformed TMbed payloads remain missing + for both derived annotations and observe the expected failures. +- [x] 10.2 Add a producer/consumer regression proving a malformed TMbed payload reaches + TypeScript as `N/A` and observe the expected failure. +- [x] 10.3 Move the FASTA sequence requirement from `bundle-format-contract` to its + owning `annotation-input` capability within this change. + +## 11. Malformed Payload Fix + +- [x] 11.1 Restrict the shared TMbed topology extractor to non-blank strings containing + only supported topology labels while preserving all valid topology semantics. + +## 12. Final Review Verification + +- [x] 12.1 Run focused ProtSpace and cross-language contract tests. +- [x] 12.2 Run Ruff, the complete relevant Python and hosted-prep suites, strict OpenSpec + validation, and `pnpm precommit`. +- [x] 12.3 Prepare the verified change for inline review replies and pushed CI. + +## 13. Source Precedence and Optimized-Path Regressions + +- [x] 13.1 Add a manager regression proving InterPro prefers a UniProt canonical + sequence while Biocentral prefers the submitted FASTA sequence, and observe it + fail. +- [x] 13.2 Add a CLI regression proving UniProt-only FASTA annotation does not parse or + retain sequences, and observe it fail. +- [x] 13.3 Extend the optimized bundle conversion contract with the negative and missing + TMbed sentinel assertions. + +## 14. Source-Aware Sequence Handling and Documentation + +- [x] 14.1 Apply source-specific sequence precedence in the annotation manager. +- [x] 14.2 Parse FASTA sequences only when a requested annotation source consumes them. +- [x] 14.3 Document that mixed TMbed predictions expose both transmembrane categories. + +## 15. Latest Review Verification + +- [x] 15.1 Run focused annotation-manager, CLI, and bundle-contract checks. +- [x] 15.2 Run Ruff, the complete relevant Python and hosted-prep suites, strict OpenSpec + validation, and `pnpm precommit`. +- [x] 15.3 Prepare the verified changes and per-item dispositions for the newest review + reply. + +## 16. Merge with main and Code Review + +- [x] 16.1 Merge `main`, which had already shipped the `non-transmembrane` label and + FASTA sequence passing; keep its `annotate` command, which parses the FASTA for + every request (the missing-length fallback reads it), superseding 13.2 and 14.2. +- [x] 16.2 Record the TMbed payload change as annotation cache semantics version 4 and + refresh a stale column of a source the refresh refetches anyway instead of + dropping it. +- [x] 16.3 Run Ruff, the non-slow ProtSpace and hosted-prep suites, the bundle contract, + strict OpenSpec validation, and `pnpm precommit`. +- [x] 16.4 Batch with #545 and #546 for one release, fold stored `hiddenValues` N/A + spellings, compute the refetched sources before the stale-column discard, and + archive this change. diff --git a/openspec/changes/archive/2026-10-15-fix-cache-ownership/.openspec.yaml b/openspec/changes/archive/2026-10-15-fix-cache-ownership/.openspec.yaml new file mode 100644 index 000000000..5849c2dbf --- /dev/null +++ b/openspec/changes/archive/2026-10-15-fix-cache-ownership/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-08-01 diff --git a/openspec/changes/fix-cache-ownership/README.md b/openspec/changes/archive/2026-10-15-fix-cache-ownership/README.md similarity index 100% rename from openspec/changes/fix-cache-ownership/README.md rename to openspec/changes/archive/2026-10-15-fix-cache-ownership/README.md diff --git a/openspec/changes/fix-cache-ownership/design.md b/openspec/changes/archive/2026-10-15-fix-cache-ownership/design.md similarity index 100% rename from openspec/changes/fix-cache-ownership/design.md rename to openspec/changes/archive/2026-10-15-fix-cache-ownership/design.md diff --git a/openspec/changes/fix-cache-ownership/proposal.md b/openspec/changes/archive/2026-10-15-fix-cache-ownership/proposal.md similarity index 100% rename from openspec/changes/fix-cache-ownership/proposal.md rename to openspec/changes/archive/2026-10-15-fix-cache-ownership/proposal.md diff --git a/openspec/changes/fix-cache-ownership/specs/atomic-file-publication/spec.md b/openspec/changes/archive/2026-10-15-fix-cache-ownership/specs/atomic-file-publication/spec.md similarity index 100% rename from openspec/changes/fix-cache-ownership/specs/atomic-file-publication/spec.md rename to openspec/changes/archive/2026-10-15-fix-cache-ownership/specs/atomic-file-publication/spec.md diff --git a/openspec/changes/fix-cache-ownership/specs/intermediate-cache-ownership/spec.md b/openspec/changes/archive/2026-10-15-fix-cache-ownership/specs/intermediate-cache-ownership/spec.md similarity index 100% rename from openspec/changes/fix-cache-ownership/specs/intermediate-cache-ownership/spec.md rename to openspec/changes/archive/2026-10-15-fix-cache-ownership/specs/intermediate-cache-ownership/spec.md diff --git a/openspec/changes/fix-cache-ownership/specs/notebook-generate-output-identity/spec.md b/openspec/changes/archive/2026-10-15-fix-cache-ownership/specs/notebook-generate-output-identity/spec.md similarity index 100% rename from openspec/changes/fix-cache-ownership/specs/notebook-generate-output-identity/spec.md rename to openspec/changes/archive/2026-10-15-fix-cache-ownership/specs/notebook-generate-output-identity/spec.md diff --git a/openspec/changes/fix-cache-ownership/tasks.md b/openspec/changes/archive/2026-10-15-fix-cache-ownership/tasks.md similarity index 100% rename from openspec/changes/fix-cache-ownership/tasks.md rename to openspec/changes/archive/2026-10-15-fix-cache-ownership/tasks.md diff --git a/openspec/changes/archive/2026-10-15-fold-style-na-aliases/.openspec.yaml b/openspec/changes/archive/2026-10-15-fold-style-na-aliases/.openspec.yaml new file mode 100644 index 000000000..a216797d5 --- /dev/null +++ b/openspec/changes/archive/2026-10-15-fold-style-na-aliases/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-10-15 diff --git a/openspec/changes/archive/2026-10-15-fold-style-na-aliases/design.md b/openspec/changes/archive/2026-10-15-fold-style-na-aliases/design.md new file mode 100644 index 000000000..a9bd8a65b --- /dev/null +++ b/openspec/changes/archive/2026-10-15-fold-style-na-aliases/design.md @@ -0,0 +1,64 @@ +## Context + +`protspace style` reads a styles file keyed by display values, resolves each key against the +column's values (`resolve_style_key`), writes the result into the reader's visualization state +keyed by the resolved value, and converts that state into the settings part +(`visualization_state_to_settings`). The converter renames an N/A value to `__NA__` as it +writes each category. Both steps use an exact-match `_NA_LABELS` set, so a spelling outside it +stays a category of its own, and several N/A entries reaching the converter overwrite one +another in sorted order. + +## Goals / Non-Goals + +**Goals:** + +- Every value the web shows as N/A is one category in the settings, `__NA__`, whatever spelling + the data or the styles file uses. +- A new N/A style replaces a stored one. +- One N/A rule in the style path, built on `is_missing_value`. + +**Non-Goals:** + +- Missing spellings inside a multi-hit cell (`A;none`): `_to_display_value` splits hits and keeps + them, the web drops missing hits. Styles key hits, so `none` there now resolves as N/A, which + is what the web shows; no further change. +- The Dash plot's own N/A colour (`` in `plotting.py`). + +## Decisions + +**`is_na_label(value) = value == "" or is_missing_value(value)`, in `settings_converter.py`, +imported by `add_annotation_style.py`.** `` is not a web token: it is what pandas prints for +a missing value, and styles files written from earlier templates carry it. It stays a style-path +alias instead of going into the shared token set, which the contract pins to the web. + +**Canonicalise when writing, fold when converting.** `_styled_settings` writes an N/A style under +`__NA__` directly, the key the stored settings already use, so a restyle overwrites the stored +style in the visualization state and no precedence question arises. The converter then folds +whatever N/A spellings remain (older flat settings keyed `""` or ``, frequencies keyed by data +spellings, a stray `N/A` category from an older run) into `__NA__`, with `__NA__` first and the +rest in sorted order. Folding only in the converter would have to guess which of two colours is +the new one. + +Alternative considered: make `resolve_style_key` return `__NA__`. It is also the showcase build's +"would `style` accept this key" check and documents returning a key the data holds; keeping that +contract and canonicalising one step later changes fewer callers. + +**The `none` exclusion goes.** It kept TMbed's negative category styleable apart from N/A, but the +web has always drawn `none` as N/A, so a style on that "category" never applied. With the label +renamed, `none` is N/A everywhere. + +**Template lists N/A once, as `__NA__`.** The styles file round-trips through `resolve_style_key`, +which accepts `__NA__` whenever the column has a missing cell; several N/A rows with separate +colours could only ever produce one. + +**Remove the dead default-grey loop.** The converter's "N/A gets light grey" loop looks for `""`, +`` and `NaN` keys that the renaming has already turned into `__NA__`, so it never ran. The +web's default N/A colour applies, as it already did. + +## Risks / Trade-offs + +- [A styles file used a non-N/A spelling such as `none` as a real category] → The web never + showed it as one; the style now reaches N/A, which is what that cell displays. +- [Folding changes the zOrder of an annotation whose stored settings held a stray `N/A` + category] → Only for settings an older `protspace style` wrote, where that category was + already invisible to the web. diff --git a/openspec/changes/archive/2026-10-15-fold-style-na-aliases/proposal.md b/openspec/changes/archive/2026-10-15-fold-style-na-aliases/proposal.md new file mode 100644 index 000000000..91a3757ec --- /dev/null +++ b/openspec/changes/archive/2026-10-15-fold-style-na-aliases/proposal.md @@ -0,0 +1,52 @@ +## Why + +The web app draws every missing cell as one N/A legend entry and looks its style up under the +key `__NA__`. `protspace style` decides which values are N/A with its own list (`""`, ``, +`NaN`, `__NA__`, `None`, compared exactly), kept twice, in `settings_converter.py` and +`add_annotation_style.py`. Anything else the web reads as N/A keeps its own spelling: + +- Styling a column whose missing cells say `N/A` (or `null`, `none`, `NA`) stores the colour + under the key `N/A`. The web never looks there, so the style is silently lost. `hiddenValues` + and `pinnedValues` spelled that way are lost the same way. +- Restyling N/A on a bundle that already has an `__NA__` colour keeps the old colour. The new + one is written under the data's spelling (`""` for a v3 missing cell), and when both reach + the settings, `__NA__` overwrites it. + +The styling guide promises any N/A spelling works and lands under `__NA__`. Both cases break +that promise without an error. + +## What Changes + +- One rule for N/A in the style path: a value is N/A when the web reads it as missing + (`is_missing_value`), or is ``, the `str()` of a pandas missing value that older styles + files carry. Both `_NA_LABELS` sets are removed. +- `protspace style` writes a style for any N/A spelling under `__NA__`, so a restyle replaces + the stored N/A style instead of losing to it. +- The settings converter folds every N/A spelling into the one `__NA__` category: colours, + shapes, stored categories, frequencies, `pinnedValues` and `hiddenValues`. `__NA__` itself + wins over a spelling of it. +- `resolve_style_key` resolves an N/A key to the first N/A spelling the data holds in sorted + order, rather than in set order. It also resolves to `none` now, which the web has always + shown as N/A (TMbed's negative, the reason it was excluded, is now `non-transmembrane`). +- `--generate-template` lists N/A once, under `__NA__`, instead of once per spelling. +- The styling guide lists the spellings by reference to the web's rule. + +## Capabilities + +### New Capabilities + +### Modified Capabilities + +- `bundle-format-contract`: adds a requirement that a style naming N/A by any spelling reaches + the web's N/A category. + +## Impact + +- `apps/protspace/src/protspace/data/io/settings_converter.py`, + `apps/protspace/src/protspace/utils/add_annotation_style.py`. +- Tests: `test_missing_values.py` (style end to end), `test_settings_converter.py`, + `test_style_warnings.py` (the `none` assertion flips). +- Docs: `docs/guide/styling.md` (N/A values). +- Behaviour: N/A styles that were silently dropped now apply. A settings part from an older + `protspace style` that holds a stray `N/A` (or similar) category is folded into `__NA__` on + the next restyle. The web reader is unchanged. diff --git a/openspec/changes/archive/2026-10-15-fold-style-na-aliases/specs/bundle-format-contract/spec.md b/openspec/changes/archive/2026-10-15-fold-style-na-aliases/specs/bundle-format-contract/spec.md new file mode 100644 index 000000000..aa33275c0 --- /dev/null +++ b/openspec/changes/archive/2026-10-15-fold-style-na-aliases/specs/bundle-format-contract/spec.md @@ -0,0 +1,24 @@ +## ADDED Requirements + +### Requirement: A style for N/A reaches the web's N/A category + +`protspace style` SHALL store every style that names a value the web reads as missing under the +web's N/A key, `__NA__`, whatever spelling the data or the styles file uses. This covers colours, +shapes, `pinnedValues` and `hiddenValues`. The settings it writes SHALL hold at most one N/A +category, and a newly given N/A style SHALL replace a stored one. + +#### Scenario: Missing cells spelled in a way the old alias list missed + +- **WHEN** a column's missing cells are spelled `N/A` and the styles file colours `N/A` +- **THEN** the settings hold that colour under `__NA__` and no `N/A` category + +#### Scenario: Restyling N/A + +- **WHEN** a bundle's settings already colour `__NA__` and `protspace style` is given a new N/A + colour under any N/A spelling +- **THEN** the written settings hold the new colour under `__NA__` + +#### Scenario: Hiding or pinning N/A by its spelling + +- **WHEN** `hiddenValues` or `pinnedValues` name N/A as `N/A`, `""` or `none` +- **THEN** the written settings name it `__NA__`, once diff --git a/openspec/changes/archive/2026-10-15-fold-style-na-aliases/tasks.md b/openspec/changes/archive/2026-10-15-fold-style-na-aliases/tasks.md new file mode 100644 index 000000000..0e138712e --- /dev/null +++ b/openspec/changes/archive/2026-10-15-fold-style-na-aliases/tasks.md @@ -0,0 +1,16 @@ +## 1. Failing tests + +- [x] 1.1 Style end to end: `N/A` cells styled as `N/A` land under `__NA__`; restyling a stored `__NA__` colour takes the new one; `hiddenValues` / `pinnedValues` spelled `N/A` land as `__NA__` +- [x] 1.2 Converter: N/A spellings in the visualization state, stored categories and frequencies fold into one `__NA__`, `__NA__` winning +- [x] 1.3 Resolver: `__NA__` resolves to `none`; an N/A key resolves to the data's first N/A spelling in sorted order; the template lists N/A once + +## 2. One N/A rule in the style path + +- [x] 2.1 Add `is_na_label` to `settings_converter.py` and remove both `_NA_LABELS` sets +- [x] 2.2 Fold N/A in `visualization_state_to_settings` (colours, shapes, categories, frequencies, pinned, hidden) and drop the dead default-grey loop +- [x] 2.3 Write N/A styles under `__NA__` in `_styled_settings`; make `_resolve_na` deterministic; list N/A once in `generate_template` + +## 3. Docs and verification + +- [x] 3.1 Update `docs/guide/styling.md` (N/A values, pinned N/A) and `apps/protspace/CLAUDE.md` +- [x] 3.2 Run ruff, `uv run pytest -m "not slow"`, `pnpm test:contract`, `pnpm precommit` diff --git a/openspec/changes/archive/2026-10-15-interpro-cache-and-default-palette/.openspec.yaml b/openspec/changes/archive/2026-10-15-interpro-cache-and-default-palette/.openspec.yaml new file mode 100644 index 000000000..a216797d5 --- /dev/null +++ b/openspec/changes/archive/2026-10-15-interpro-cache-and-default-palette/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-10-15 diff --git a/openspec/changes/archive/2026-10-15-interpro-cache-and-default-palette/proposal.md b/openspec/changes/archive/2026-10-15-interpro-cache-and-default-palette/proposal.md new file mode 100644 index 000000000..ec073351f --- /dev/null +++ b/openspec/changes/archive/2026-10-15-interpro-cache-and-default-palette/proposal.md @@ -0,0 +1,34 @@ +## Why + +Two gaps the batch review of #406, #545 and #546 left open: + +- #406 makes InterPro look up UniProt's canonical sequence before the FASTA's, which changes what + a cached InterPro column means for a protein whose FASTA differs from the canonical sequence, + but recorded no cache semantics change for it. A resumed run kept the FASTA-based result while + a fresh run returned the canonical one. +- #546 shares the default numeric palette between Python and the web, but not the categorical + one: `kellys` stayed a literal in both languages, where a change on one side would drift. + +## What Changes + +- Cache semantics version 4, not yet released, also covers every InterPro column, so caches + written before it refetch InterPro once. +- `DEFAULT_CATEGORICAL_PALETTE_ID` joins `contracts/shared-constants.json`, validated to be a + categorical palette id, and every default-palette literal in both languages reads it. + +## Capabilities + +### Modified Capabilities + +- `annotation-cache-semantics`: InterPro columns cached before the canonical-sequence lookup are + refreshed. +- `shared-constants`: the default categorical palette is a shared constant, and the generator + checks it names a categorical palette. + +## Impact + +- `apps/protspace`: `encoding.py`'s cache table, the template and settings defaults of + `protspace style`, and the statistics legend defaults. +- `packages/utils` and `packages/core`: the legend, the settings validator and the bundle + readers' default colouring read the shared default. +- No bundle-layout or dependency change. A v3 cache refetches InterPro once. diff --git a/openspec/changes/archive/2026-10-15-interpro-cache-and-default-palette/specs/annotation-cache-semantics/spec.md b/openspec/changes/archive/2026-10-15-interpro-cache-and-default-palette/specs/annotation-cache-semantics/spec.md new file mode 100644 index 000000000..38335412f --- /dev/null +++ b/openspec/changes/archive/2026-10-15-interpro-cache-and-default-palette/specs/annotation-cache-semantics/spec.md @@ -0,0 +1,15 @@ +## ADDED Requirements + +### Requirement: Caches written before InterPro's canonical-sequence lookup are refreshed + +ProtSpace SHALL NOT reuse cached InterPro columns stamped version 3 or earlier. Those versions +looked InterPro up by the FASTA's sequence whenever one was given, where InterPro now takes +UniProt's canonical sequence first, so a cached match, or a cached absence of one, may belong to +a different sequence. A run that requests such a column SHALL refetch InterPro once and stamp the +rewritten cache as current. + +#### Scenario: A version-3 cache is asked for pfam + +- **WHEN** a run requests `pfam` and a UniProt column from a cache stamped version 3 +- **THEN** ProtSpace refetches InterPro once, reuses the cached UniProt values, and stamps the + cache current, so the next run fetches nothing diff --git a/openspec/changes/archive/2026-10-15-interpro-cache-and-default-palette/specs/shared-constants/spec.md b/openspec/changes/archive/2026-10-15-interpro-cache-and-default-palette/specs/shared-constants/spec.md new file mode 100644 index 000000000..bcb03cab2 --- /dev/null +++ b/openspec/changes/archive/2026-10-15-interpro-cache-and-default-palette/specs/shared-constants/spec.md @@ -0,0 +1,56 @@ +## MODIFIED Requirements + +### Requirement: Shared constants have one source + +Every constant both sides must agree on SHALL be defined once, in the shared source +`contracts/shared-constants.json`, together with the reason it is shared. This covers the +missing-value tokens, the `__NA__` sentinel, the N/A swatch colour, Kelly's colours in slot +order, the categorical and gradient palette ids, the default categorical and numeric palettes, +the labels of an Arrow boolean column, and the point shapes. Neither language SHALL hold a hand-written +copy of these values. Each SHALL read them from its generated module, and the names existing +code imports SHALL stay importable from where they were. + +#### Scenario: A missing-value spelling is added + +- **WHEN** a contributor adds a spelling to `MISSING_VALUE_TOKENS` in + `contracts/shared-constants.json` and regenerates +- **THEN** the Python package's `protspace.core.constants.MISSING_VALUE_TOKENS` and the web's + `MISSING_VALUE_TOKENS` both contain it, and no other file is edited + +#### Scenario: A palette is added to one language only + +- **WHEN** `COLOR_SCHEMES` in `color-scheme.ts` gains a palette whose id + `contracts/shared-constants.json` does not list, or loses one it does list +- **THEN** the TypeScript type check fails + +#### Scenario: A Python re-export is replaced by a literal + +- **WHEN** one of the Python names re-exported from the generated module (for example + `KELLYS_COLORS` in `settings_converter.py`) is redefined as a hand-written value +- **THEN** the Python test suite fails, naming the constant + +### Requirement: The source is validated before generation + +The generator SHALL refuse to write when the source is malformed (an unknown kind, a value of +the wrong type, a duplicate entry, a name that is not upper snake case, a doc line over 80 +characters) or breaks an invariant. The invariants are: every missing-value token is trimmed +and lower-case; `NA_VALUE`, lower-cased, is a missing-value token; no palette id is both +categorical and gradient; `DEFAULT_CATEGORICAL_PALETTE_ID` is a categorical palette id; +`DEFAULT_NUMERIC_PALETTE_ID` is a gradient palette id; +`ARROW_BOOLEAN_LABELS` is exactly the false label then the true label. + +#### Scenario: An upper-case token is added + +- **WHEN** `MISSING_VALUE_TOKENS` gains `N/A` +- **THEN** the generator exits non-zero, says that the matcher compares lower-cased cells, + and writes nothing + +#### Scenario: The numeric default is not a gradient + +- **WHEN** `DEFAULT_NUMERIC_PALETTE_ID` names a categorical palette +- **THEN** the generator exits non-zero and writes nothing + +#### Scenario: The categorical default is not a categorical palette + +- **WHEN** `DEFAULT_CATEGORICAL_PALETTE_ID` names a gradient or unknown palette +- **THEN** the generator exits non-zero and writes nothing diff --git a/openspec/changes/archive/2026-10-15-interpro-cache-and-default-palette/tasks.md b/openspec/changes/archive/2026-10-15-interpro-cache-and-default-palette/tasks.md new file mode 100644 index 000000000..a0845c601 --- /dev/null +++ b/openspec/changes/archive/2026-10-15-interpro-cache-and-default-palette/tasks.md @@ -0,0 +1,15 @@ +## 1. InterPro cache version + +- [x] 1.1 Add the InterPro columns to cache semantics version 4 in `encoding.py` +- [x] 1.2 Test that a version-3 cache refetches InterPro once and reuses UniProt, and keep the + version-3 tests to the columns that version changed + +## 2. Shared categorical default + +- [x] 2.1 Add `DEFAULT_CATEGORICAL_PALETTE_ID` to `contracts/shared-constants.json` and the + generator invariant that it is a categorical palette id; regenerate both modules +- [x] 2.2 Read it in place of every default `kellys` literal in Python and TypeScript + +## 3. Verification + +- [x] 3.1 Run the Python and web test suites, the bundle contract and `pnpm precommit` diff --git a/openspec/changes/parquetbundle-v3/.openspec.yaml b/openspec/changes/archive/2026-10-15-parquetbundle-v3/.openspec.yaml similarity index 100% rename from openspec/changes/parquetbundle-v3/.openspec.yaml rename to openspec/changes/archive/2026-10-15-parquetbundle-v3/.openspec.yaml diff --git a/openspec/changes/parquetbundle-v3/design.md b/openspec/changes/archive/2026-10-15-parquetbundle-v3/design.md similarity index 99% rename from openspec/changes/parquetbundle-v3/design.md rename to openspec/changes/archive/2026-10-15-parquetbundle-v3/design.md index cf44c5fd8..8155740ac 100644 --- a/openspec/changes/parquetbundle-v3/design.md +++ b/openspec/changes/archive/2026-10-15-parquetbundle-v3/design.md @@ -368,6 +368,8 @@ deprecated alongside legacy read support. and unit fixtures stay v1/v2 until 5.0.0 removes the reader they test. 4. In 5.0.0, remove the legacy readers, `extractRowsFromParquetBundle` and the notice, and make `protspace convert` point to the last 4.x release for anyone still holding v1/v2 files. + (Revised in issue #544: `convert` keeps its own legacy reader in 5.0 and upgrades v1/v2 + files, so nobody is stranded.) Rollback is a revert of the merge. Files written as v3 in the meantime then need a 4.x build that reads v3, which is why the reader lands in the same release as the writer. diff --git a/openspec/changes/parquetbundle-v3/proposal.md b/openspec/changes/archive/2026-10-15-parquetbundle-v3/proposal.md similarity index 100% rename from openspec/changes/parquetbundle-v3/proposal.md rename to openspec/changes/archive/2026-10-15-parquetbundle-v3/proposal.md diff --git a/openspec/changes/parquetbundle-v3/specs/bundle-conversion/spec.md b/openspec/changes/archive/2026-10-15-parquetbundle-v3/specs/bundle-conversion/spec.md similarity index 100% rename from openspec/changes/parquetbundle-v3/specs/bundle-conversion/spec.md rename to openspec/changes/archive/2026-10-15-parquetbundle-v3/specs/bundle-conversion/spec.md diff --git a/openspec/changes/parquetbundle-v3/specs/bundle-format-contract/spec.md b/openspec/changes/archive/2026-10-15-parquetbundle-v3/specs/bundle-format-contract/spec.md similarity index 100% rename from openspec/changes/parquetbundle-v3/specs/bundle-format-contract/spec.md rename to openspec/changes/archive/2026-10-15-parquetbundle-v3/specs/bundle-format-contract/spec.md diff --git a/openspec/changes/parquetbundle-v3/specs/imported-dataset-persistence/spec.md b/openspec/changes/archive/2026-10-15-parquetbundle-v3/specs/imported-dataset-persistence/spec.md similarity index 100% rename from openspec/changes/parquetbundle-v3/specs/imported-dataset-persistence/spec.md rename to openspec/changes/archive/2026-10-15-parquetbundle-v3/specs/imported-dataset-persistence/spec.md diff --git a/openspec/changes/parquetbundle-v3/specs/parquetbundle-v3/spec.md b/openspec/changes/archive/2026-10-15-parquetbundle-v3/specs/parquetbundle-v3/spec.md similarity index 100% rename from openspec/changes/parquetbundle-v3/specs/parquetbundle-v3/spec.md rename to openspec/changes/archive/2026-10-15-parquetbundle-v3/specs/parquetbundle-v3/spec.md diff --git a/openspec/changes/parquetbundle-v3/specs/point-visibility/spec.md b/openspec/changes/archive/2026-10-15-parquetbundle-v3/specs/point-visibility/spec.md similarity index 100% rename from openspec/changes/parquetbundle-v3/specs/point-visibility/spec.md rename to openspec/changes/archive/2026-10-15-parquetbundle-v3/specs/point-visibility/spec.md diff --git a/openspec/changes/parquetbundle-v3/tasks.md b/openspec/changes/archive/2026-10-15-parquetbundle-v3/tasks.md similarity index 89% rename from openspec/changes/parquetbundle-v3/tasks.md rename to openspec/changes/archive/2026-10-15-parquetbundle-v3/tasks.md index a5eea382e..cf8af332f 100644 --- a/openspec/changes/parquetbundle-v3/tasks.md +++ b/openspec/changes/archive/2026-10-15-parquetbundle-v3/tasks.md @@ -196,24 +196,30 @@ either); keep the e2e fixtures and `v2-sample` legacy as the legacy reader's test data; `scripts/landing-data` reads v3; `encode_legacy_cell` splits a hit at its last pipe -- [ ] 5.4 Merge the PR with a merge commit, never a squash (it touches `apps/protspace/`) - -## 6. Follow-ups for protspace 5.0.0 (not in this change) - -- [ ] 6.1 Python: drop the legacy branch of `bundle._parse_bundle` (3 to 5 parts), the deprecation - warning and `LEGACY_REMOVAL_VERSION`, and the `warn_legacy` plumbing of the readers and writers -- [ ] 6.2 Python: `protspace convert` stops reading v1/v2 and names the last 4.x release that can; - `style` and `transfer` no longer upgrade a legacy input; the v1 cell migration - (`migrate_legacy_annotation_table`, `encode_legacy_cell`) and the v2 browser's id-column - fallback go with it -- [ ] 6.3 Browser: remove the legacy path of `decodeParquetBundle`, `extractRowsFromParquetBundle` - and the bundle-only parts of the row-based conversion it feeds (plain `.parquet` imports keep - `convertParquetToVisualizationDataOptimized`), the `formatVersion` below 3 handling and the - legacy-format notice in `apps/web/src/explore/notifications.ts` -- [ ] 6.4 Tests and data: retire the v1 e2e fixtures under `apps/web/tests/fixtures/`, the - `v2-sample` unit fixture, the Python legacy tests and the legacy-cell path of - `scripts/landing-data` -- [ ] 6.5 Docs: the legacy layout and v1/v2 sections of `docs/guide/data-format.md`, the deprecated - `extractRowsFromParquetBundle` entry in `docs/developers/api/index.md`, and the deprecation - notes in `docs/guide/python-cli.md` and `apps/protspace/CLAUDE.md`; release with a - `BREAKING CHANGE` footer +- [x] 5.4 Merge the PR with a merge commit, never a squash (it touches `apps/protspace/`) + (#477, merged 2026-10-02 as `952646cf`) + +## 6. Follow-ups for protspace 5.0.0 (tracked in #544) + +Tracked in issue #544, which supersedes this list in one respect: `protspace convert` keeps +its own legacy reader in 5.0, so v1/v2 files can still be upgraded. Not tasks of this change, so +listed without checkboxes: + +- Python: drop the legacy branch of `bundle._parse_bundle` (3 to 5 parts), the deprecation + warning and `LEGACY_REMOVAL_VERSION`, and the `warn_legacy` plumbing of the readers and writers +- Python: ~~`protspace convert` stops reading v1/v2 and names the last 4.x release that can~~ + (superseded by #544: `convert` keeps reading them); + `style` and `transfer` no longer upgrade a legacy input; the v1 cell migration + (`migrate_legacy_annotation_table`, `encode_legacy_cell`) and the v2 browser's id-column + fallback go with it +- Browser: remove the legacy path of `decodeParquetBundle`, `extractRowsFromParquetBundle` + and the bundle-only parts of the row-based conversion it feeds (plain `.parquet` imports keep + `convertParquetToVisualizationDataOptimized`), the `formatVersion` below 3 handling and the + legacy-format notice in `apps/web/src/explore/notifications.ts` +- Tests and data: retire the v1 e2e fixtures under `apps/web/tests/fixtures/`, the + `v2-sample` unit fixture, the Python legacy tests and the legacy-cell path of + `scripts/landing-data` +- Docs: the legacy layout and v1/v2 sections of `docs/guide/data-format.md`, the deprecated + `extractRowsFromParquetBundle` entry in `docs/developers/api/index.md`, and the deprecation + notes in `docs/guide/python-cli.md` and `apps/protspace/CLAUDE.md`; release with a + `BREAKING CHANGE` footer diff --git a/openspec/changes/archive/2026-10-15-shared-constants-codegen/.openspec.yaml b/openspec/changes/archive/2026-10-15-shared-constants-codegen/.openspec.yaml new file mode 100644 index 000000000..a216797d5 --- /dev/null +++ b/openspec/changes/archive/2026-10-15-shared-constants-codegen/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-10-15 diff --git a/openspec/changes/archive/2026-10-15-shared-constants-codegen/design.md b/openspec/changes/archive/2026-10-15-shared-constants-codegen/design.md new file mode 100644 index 000000000..4e0f3d1da --- /dev/null +++ b/openspec/changes/archive/2026-10-15-shared-constants-codegen/design.md @@ -0,0 +1,171 @@ +## Context + +`apps/protspace` (Python, shipped to PyPI) writes the bundles and settings that +`@protspace/utils` and `@protspace/core` (TypeScript) read. Several constants are written out +by hand on both sides, and three mechanisms keep them in step today: + +1. **Regex over TypeScript source.** `test_style_warnings.py` parses `COLOR_SCHEMES`, + `GRADIENT_COLOR_SCHEME_IDS` and `DEFAULT_NUMERIC_PALETTE_ID` out of `color-scheme.ts` and + `numeric-binning.ts`. A prettier reflow or a refactor of those files breaks the test + without any value changing. The test runs only when `protspace-ci.yml` runs, which is + why that workflow watches two TypeScript files. +2. **The bundle contract suite** (`tests/contract`, unfiltered `bundle-contract.yml`). + Python writes its missing-value tokens and its verdicts on probe spellings into the + manifest, and vitest asserts that TypeScript agrees. It catches drift, but only after both + files have been edited and CI has run. +3. **Comments**: "same order as the web frontend" on `KELLYS_COLORS`, "frontend internal key" + on `NA_INTERNAL`, and no pin at all on `NA_PINNED_COLOR`, `ARROW_BOOLEAN_LABELS` or + `ALLOWED_SHAPES`. + +The repo already generates committed files and checks them with `--check` +(`docs/scripts/generate-annotations.mts` → `docs/guide/annotations.md`, run by +`pnpm precommit` and `ci.yml`). This change applies the same pattern to code. + +Constraint: the wheel and sdist are built from `apps/protspace` alone, so the Python package +cannot read `packages/` or a repo-root file at build time or at run time. + +## Goals / Non-Goals + +**Goals:** + +- Each shared constant is written once, with its rationale, in a language-neutral file. +- Each language reads it from a committed, generated module. A stale or hand-edited module + fails `pnpm precommit` and a CI job that runs on every pull request. +- No import site changes. The existing names stay importable from where they are today. +- The regex-over-TypeScript tests are removed. + +**Non-Goals:** + +- Behaviour rules: how a cell is matched against the tokens, numeric inference, hit + splitting, and id-column selection. These are code, and the contract suite keeps pinning + them by running both implementations on the same inputs. +- Format-defining bundle constants: the part delimiter, the footer keys, and the container + version. Every contract fixture already depends on them end to end, and changing one is a + format-version event, not an edit. +- Changing any value. This is a pure refactor. + +## Decisions + +### Inventory: what moves and what stays + +| Constant (Python name / TS name) | Moves? | Why | +| ------------------------------------------------------------------------------------------------------- | ------ | ----------------------------------------------------------------------------------------------------------------------------------------- | +| `MISSING_VALUE_TOKENS` / same | yes | Wire: both readers' missing spellings | +| `NA_INTERNAL` / `NA_VALUE` (`__NA__`) | yes | Wire: the settings key for the N/A category | +| `NA_PINNED_COLOR` / `NA_DEFAULT_COLOR` (`#DDDDDD`) | yes | UI: Python colours a pinned N/A as the web would | +| `KELLYS_COLORS` / same | yes | UI: Python assigns the web's slot colours, in order | +| `_CATEGORICAL_PALETTE_IDS`, `_GRADIENT_PALETTE_IDS` / `COLOR_SCHEMES` keys, `GRADIENT_COLOR_SCHEME_IDS` | yes | Wire: `selectedPaletteId` values | +| (none) / `DEFAULT_NUMERIC_PALETTE_ID` | yes | Must be a gradient id. Python pinned this by regex; the generator now checks it | +| `ARROW_BOOLEAN_LABELS` / literal `'true'`/`'false'` in `bundle-writer.ts` | yes | Wire: the v3 label spelling of an Arrow `bool` column | +| `ALLOWED_SHAPES` / `PointShape` | yes | Wire: settings `shape` values (not in the original list, but the same kind of contract) | +| `CANONICAL_BOOLEANS` (`False`/`True`) | no | Python-only producer vocabulary. The web reads these as ordinary category strings | +| `NA_COLOR` (`#C0C0C0`), `_NA_LABELS`, `REST_MARKER` | no | Python/Dash-only. (`NA_COLOR`'s `setdefault` branch cannot run, since those labels are already rekeyed to `__NA__`; noted as a follow-up) | +| `EVIDENCE_CODE_RE`, `_LEGACY_ID_CANDIDATES`, `ID_CANDIDATES`, sort modes | no | Reader rules, not values. A regex shared as a string also invites dialect bugs | +| `PARQUET_BUNDLE_DELIMITER`, footer keys, container version | no | Format-defining, and pinned end to end by the contract fixtures | + +Of the roughly 19 "mirrors the web" comments under `src/protspace`, most describe behaviour +mirrored from `conversion.ts`/`bundle.ts`, which is the contract suite's territory. The data +ones are the rows marked "yes". + +### Source format: JSON with a per-constant `kind` and `doc` + +```json +"MISSING_VALUE_TOKENS": { "kind": "set", "doc": ["…"], "value": ["na", "…"] } +``` + +`kind` is `string`, `list` or `set`. That covers every shared constant and maps onto both +languages without ambiguity: + +| kind | Python | TypeScript | +| -------- | ----------- | ------------------------- | +| `string` | `str` | literal `const` | +| `list` | `tuple` | `readonly [...] as const` | +| `set` | `frozenset` | `ReadonlySet` | + +`doc` is a list of pre-wrapped lines. It is emitted as `#` comments in Python and as JSDoc in +TypeScript, so the rationale (for example why `__na__` is a token) lives next to the value, +not in one of the two copies. + +Alternatives considered: TypeScript as the source, generating only Python. The web would +import the source directly, but the Python module would be generated from code that needs a +TS evaluator, and the source would not be neutral. YAML or TOML would need a parser +dependency, while JSON is native to both Node and Python. + +### Palette ids are lists; colours stay in TypeScript except Kelly's + +The source holds `CATEGORICAL_PALETTE_IDS` and `GRADIENT_PALETTE_IDS` as ordered lists, plus +`KELLYS_COLORS`. The other palettes' colours are used only by the web, so they stay in +`color-scheme.ts`. `COLOR_SCHEMES` is declared +`as const satisfies Record`, so TypeScript rejects both a +missing palette and an id the source does not list. The categorical/gradient split is now +explicit. The web used to define categorical as "every scheme that is not a gradient". + +Alternative considered: moving every palette's colours into the source. That would put +about 100 web-only colour values into the Python package and lose the per-colour notes in +`color-scheme.ts`, without pinning anything else that is shared. + +### One name per constant; existing names are re-exports + +Both generated modules use the same names, taken from the web where the two sides differ +(`NA_VALUE`, `NA_DEFAULT_COLOR`). The old modules keep their names as re-exports +(`NA_INTERNAL = NA_VALUE`, `_GRADIENT_PALETTE_IDS = frozenset(GRADIENT_PALETTE_IDS)`, +`GRADIENT_COLOR_SCHEME_IDS = new Set(GRADIENT_PALETTE_IDS)`, +`type PointShape = (typeof POINT_SHAPES)[number]`). No consumer changes its imports. +`tests/test_shared_constants.py` pins the Python re-exports to the generated module, so a +hand-written copy cannot quietly come back. + +### Generator validates content, then writes; `--check` compares + +`scripts/shared-constants/generate-shared-constants.mts` (run with `tsx`, linted and +type-checked with the other scripts): + +1. Validates the structure: names are `UPPER_SNAKE`, kinds are known, values are strings or + string arrays with no duplicates, and doc lines are at most 80 characters. +2. Validates invariants: tokens are trimmed and lower-case (the matcher compares + trimmed, lower-cased cells, so anything else could never match); `NA_VALUE` lower-cased + is a token; the palette lists are disjoint; `DEFAULT_NUMERIC_PALETTE_ID` is a gradient; + `ARROW_BOOLEAN_LABELS` has exactly two entries (false, true). +3. Renders TypeScript and runs it through Prettier, using the repo config for that path. + Renders Python in a fixed, ruff-format-stable style: collections are always exploded + with a trailing comma, and strings are double-quoted. +4. Writes both files. With `--check`, it compares them instead and exits 1 naming the stale + file and the command to run. + +### Where the check runs + +- `pnpm precommit` gains `pnpm shared-constants:check` next to `docs:annotations:check`. +- CI: a step in `bundle-contract.yml`. It is the one workflow with no path filter (it is + meant to be a required check), so a hand edit to only the Python module fails there. + `ci.yml` would miss that edit, because it ignores `apps/protspace/**`. `protspace-ci.yml` + would miss an edit to only the TypeScript module. +- `protspace-ci.yml` drops `color-scheme.ts` and `numeric-binning.ts` from its paths, since + no Python test reads them any more. A change to the JSON source changes the generated + Python module, which is under `apps/protspace/**`, so Python CI still runs on it. + +### The contract suite keeps its token-set comparison + +That check now compares two values generated from one source, so it cannot fail on a +generator-clean tree. It stays anyway because it is the only check on the values each side +actually uses: a re-export replaced by a local literal would pass `--check` and still be +caught there. The verdict table, which tests behaviour, stays for the reason given under +Non-Goals. + +## Risks / Trade-offs + +- [Generated Python drifts from ruff format as ruff evolves] → The emitted style (exploded + collections, magic trailing commas, double quotes) is ruff-format's fixed point, and + `protspace-ci.yml`'s lint job runs `ruff format --check` on it, so a ruff change that + disagrees fails loudly on the next Python PR instead of drifting. +- [A contributor edits a generated file] → `--check` fails in precommit and in the + unfiltered CI job, naming the command to run. Each generated file starts with a + do-not-edit header naming its source. +- [Someone adds a value to a re-export site instead of the source] → TypeScript re-exports + are bindings, not copies. Python re-exports are pinned by `test_shared_constants.py`, and + the missing tokens also by the contract suite. +- [The generator becomes the place for behaviour] → `contracts/README.md` states the rule: + data only, and rules go to `tests/contract`. + +## Migration Plan + +This is a pure refactor and needs no data or user migration. Rollback is a revert. +Branched on #543, so it lands after it, rebased onto `main`. diff --git a/openspec/changes/archive/2026-10-15-shared-constants-codegen/proposal.md b/openspec/changes/archive/2026-10-15-shared-constants-codegen/proposal.md new file mode 100644 index 000000000..b9a7b53ae --- /dev/null +++ b/openspec/changes/archive/2026-10-15-shared-constants-codegen/proposal.md @@ -0,0 +1,75 @@ +## Why + +The Python package and the web app have to agree on a family of constants: the spellings +that mean "missing", the `__NA__` sentinel, the N/A swatch colour, Kelly's colours in slot +order, the palette ids a styles file may name, the labels of a boolean column, and the +point shapes. Today each of these is written out by hand in both languages. Some are pinned +by Python tests that pull values out of the TypeScript source with regexes +(`_web_palette_ids` and friends in `test_style_warnings.py`). Those tests break when the +source is reformatted, and they only run when Python files change, because +`protspace-ci.yml` is path-filtered. Others are not pinned at all and rely on a "same order +as the web frontend" comment. PR #543 added a contract-suite check for the missing-value +tokens. That check catches drift, but every change still means editing two files and +waiting for CI to say whether they match. + +## What Changes + +- New source of truth: `contracts/shared-constants.json` holds every shared constant once, + with its rationale. Constants that have different names in the two languages get one + name here. +- New generator `scripts/shared-constants/generate-shared-constants.mts`. It validates the + source and writes two committed modules: `packages/utils/src/generated/shared-constants.ts` + and `apps/protspace/src/protspace/_generated/shared_constants.py`. The Python module lives + inside the package, so the wheel and sdist need nothing from outside `apps/protspace`. +- `pnpm shared-constants:check` fails when either committed module differs from what the + source generates. It runs in `pnpm precommit` and in the unfiltered `bundle-contract.yml` + job, which runs on every pull request whatever paths it touches. +- The existing names stay where they are, re-exported from the generated modules, so no + import site changes. Python keeps `MISSING_VALUE_TOKENS`, `NA_INTERNAL`, + `NA_PINNED_COLOR`, `KELLYS_COLORS`, `_CATEGORICAL_PALETTE_IDS`, `_GRADIENT_PALETTE_IDS`, + `ARROW_BOOLEAN_LABELS` and `ALLOWED_SHAPES`. TypeScript keeps `MISSING_VALUE_TOKENS`, + `NA_VALUE`, `NA_DEFAULT_COLOR`, `KELLYS_COLORS`, `DEFAULT_NUMERIC_PALETTE_ID`, + `GRADIENT_COLOR_SCHEME_IDS` and `PointShape`. The web's `COLOR_SCHEMES` is typed against + the generated palette ids, so a palette with no colours, or colours under an id the source + does not list, is a compile error. +- The web's v3 bundle writer checks a `bool` column's labels against the shared + `ARROW_BOOLEAN_LABELS` instead of a literal `'true'`/`'false'`. +- Removed: the regex-over-TypeScript palette tests in `test_style_warnings.py`, and the two + TypeScript files `protspace-ci.yml` watched only for them. +- Kept: the bundle contract suite's behavioural checks, such as the missing-value verdict + table. Matching rules are code, not data, so a shared list cannot pin them. The suite also + keeps its token-set comparison, which compares the values each side actually uses, not + the generated files. +- No value changes, so this ships as `refactor:`/`chore:`/`test:` commits with no release. + +## Capabilities + +### New Capabilities + +- `shared-constants`: one source for the constants the Python package and the web app must + agree on, generated into a committed module per language and checked on every pull + request. + +### Modified Capabilities + +None. `bundle-format-contract` already requires that the two sides read the same cells as +missing, and that requirement still holds: the contract suite keeps comparing both the +token sets and the per-spelling verdicts. This change only alters how the token values reach +each side. + +## Impact + +- New: `contracts/shared-constants.json`, `contracts/README.md`, the generator, and the two + generated modules, plus the `apps/protspace/src/protspace/_generated/` package. +- Python: `core/constants.py`, `data/io/settings_converter.py`, + `utils/add_annotation_style.py` and `data/annotations/encoding.py` re-export from the + generated module. `tests/test_style_warnings.py` loses its TypeScript regexes. New + `tests/test_shared_constants.py`. +- TypeScript (`@protspace/utils`): `visualization/missing-values.ts`, `color-scheme.ts`, + `numeric-binning.ts`, `types.ts` and `parquet/bundle-writer.ts`. +- Tooling: `package.json` (`shared-constants`, `shared-constants:check`, `precommit`), + `.github/workflows/bundle-contract.yml` (new check step), and + `.github/workflows/protspace-ci.yml` (two TS paths dropped). +- Contract suite: a comment in `tests/contract/bundle.contract.test.ts` on why the token-set + comparison stays. +- Docs: `AGENTS.md`, `CONTRIBUTING.md` and `apps/protspace/CLAUDE.md`. diff --git a/openspec/changes/archive/2026-10-15-shared-constants-codegen/specs/shared-constants/spec.md b/openspec/changes/archive/2026-10-15-shared-constants-codegen/specs/shared-constants/spec.md new file mode 100644 index 000000000..617c4f1d6 --- /dev/null +++ b/openspec/changes/archive/2026-10-15-shared-constants-codegen/specs/shared-constants/spec.md @@ -0,0 +1,102 @@ +## ADDED Requirements + +### Requirement: Shared constants have one source + +Every constant both sides must agree on SHALL be defined once, in the shared source +`contracts/shared-constants.json`, together with the reason it is shared. This covers the +missing-value tokens, the `__NA__` sentinel, the N/A swatch colour, Kelly's colours in slot +order, the categorical and gradient palette ids, the default numeric palette, the labels of +an Arrow boolean column, and the point shapes. Neither language SHALL hold a hand-written +copy of these values. Each SHALL read them from its generated module, and the names existing +code imports SHALL stay importable from where they were. + +#### Scenario: A missing-value spelling is added + +- **WHEN** a contributor adds a spelling to `MISSING_VALUE_TOKENS` in + `contracts/shared-constants.json` and regenerates +- **THEN** the Python package's `protspace.core.constants.MISSING_VALUE_TOKENS` and the web's + `MISSING_VALUE_TOKENS` both contain it, and no other file is edited + +#### Scenario: A palette is added to one language only + +- **WHEN** `COLOR_SCHEMES` in `color-scheme.ts` gains a palette whose id + `contracts/shared-constants.json` does not list, or loses one it does list +- **THEN** the TypeScript type check fails + +#### Scenario: A Python re-export is replaced by a literal + +- **WHEN** one of the Python names re-exported from the generated module (for example + `KELLYS_COLORS` in `settings_converter.py`) is redefined as a hand-written value +- **THEN** the Python test suite fails, naming the constant + +### Requirement: Generated modules are committed and checked + +The generator SHALL write `packages/utils/src/generated/shared-constants.ts` and +`apps/protspace/src/protspace/_generated/shared_constants.py`, and both SHALL be committed. +`pnpm shared-constants:check` SHALL fail, naming the stale file and the command that +regenerates it, whenever either committed module differs from what the source generates. The +check SHALL run in `pnpm precommit` and in a CI job that runs on every pull request with no +path filter. + +#### Scenario: A generated module is edited by hand + +- **WHEN** a pull request changes only `apps/protspace/src/protspace/_generated/shared_constants.py` +- **THEN** the unfiltered CI job fails on `pnpm shared-constants:check` + +#### Scenario: The source changes without regenerating + +- **WHEN** `contracts/shared-constants.json` is edited and the generated modules are not + regenerated +- **THEN** `pnpm precommit` fails and names `pnpm shared-constants` + +#### Scenario: The generated modules pass their language's formatter + +- **WHEN** the generator has run +- **THEN** `prettier --check` passes on the TypeScript module and `ruff format --check` and + `ruff check` pass on the Python module + +### Requirement: The Python package needs nothing outside its own directory + +The Python package SHALL ship its shared constants inside `protspace` itself, so that a wheel +or sdist built from `apps/protspace` imports and runs without the repository's `contracts/` +or `packages/` directories. + +#### Scenario: The package is installed from its wheel + +- **WHEN** the protspace wheel is built from `apps/protspace` and installed outside the + repository +- **THEN** `protspace._generated.shared_constants` is importable and every module that + re-exports from it imports + +### Requirement: The source is validated before generation + +The generator SHALL refuse to write when the source is malformed (an unknown kind, a value of +the wrong type, a duplicate entry, a name that is not upper snake case, a doc line over 80 +characters) or breaks an invariant. The invariants are: every missing-value token is trimmed +and lower-case; `NA_VALUE`, lower-cased, is a missing-value token; no palette id is both +categorical and gradient; `DEFAULT_NUMERIC_PALETTE_ID` is a gradient palette id; +`ARROW_BOOLEAN_LABELS` is exactly the false label then the true label. + +#### Scenario: An upper-case token is added + +- **WHEN** `MISSING_VALUE_TOKENS` gains `N/A` +- **THEN** the generator exits non-zero, says that the matcher compares lower-cased cells, + and writes nothing + +#### Scenario: The numeric default is not a gradient + +- **WHEN** `DEFAULT_NUMERIC_PALETTE_ID` names a categorical palette +- **THEN** the generator exits non-zero and writes nothing + +### Requirement: Behaviour stays pinned by running both implementations + +Behavioural rules SHALL NOT be moved into the shared source: how a value is used, such as +how a cell is matched against the missing-value tokens, is code. The bundle contract suite +SHALL keep checking them by running the Python and TypeScript implementations on the same +probe inputs. + +#### Scenario: One side stops lower-casing before matching + +- **WHEN** the TypeScript `normalizeMissingValue` stops lower-casing, while the shared tokens + are unchanged +- **THEN** `pnpm shared-constants:check` still passes and the bundle contract suite fails diff --git a/openspec/changes/archive/2026-10-15-shared-constants-codegen/tasks.md b/openspec/changes/archive/2026-10-15-shared-constants-codegen/tasks.md new file mode 100644 index 000000000..851cab826 --- /dev/null +++ b/openspec/changes/archive/2026-10-15-shared-constants-codegen/tasks.md @@ -0,0 +1,45 @@ +## 1. Source and generator + +- [x] 1.1 Write `contracts/shared-constants.json` with every constant in the design's inventory (values copied unchanged, each with its `doc`), and `contracts/README.md` stating what belongs there (data only; rules go to `tests/contract`) +- [x] 1.2 Write `scripts/shared-constants/generate-shared-constants.mts`: structural validation, invariants, TypeScript render through Prettier, ruff-stable Python render, write and `--check` modes +- [x] 1.3 Add `shared-constants` and `shared-constants:check` to `package.json`; generate both modules and add the hand-written `apps/protspace/src/protspace/_generated/__init__.py` +- [x] 1.4 Confirm the generated Python passes `ruff check` and `ruff format --check`, and the generated TypeScript passes `prettier --check` and ESLint +- [x] 1.5 Confirm each validation fails as specified (upper-case token, numeric default not a gradient, duplicate, unknown kind, overlong doc line) and writes nothing + +## 2. Python re-exports + +- [x] 2.1 `core/constants.py`: `MISSING_VALUE_TOKENS` from the generated module, comment pointing at the source +- [x] 2.2 `data/io/settings_converter.py`: `NA_INTERNAL`, `NA_PINNED_COLOR`, `KELLYS_COLORS` from the generated module +- [x] 2.3 `utils/add_annotation_style.py`: `_CATEGORICAL_PALETTE_IDS`, `_GRADIENT_PALETTE_IDS`, `ALLOWED_SHAPES` from the generated module +- [x] 2.4 `data/annotations/encoding.py`: `ARROW_BOOLEAN_LABELS` from the generated module +- [x] 2.5 `tests/test_style_warnings.py`: delete `_web_source_match`, `_web_palette_ids`, `_web_gradient_palette_ids` and the tests built on them; keep the `kellys`-is-categorical assertion as a plain Python test +- [x] 2.6 New `tests/test_shared_constants.py`: every Python re-export equals (or is derived from) the generated value; the generated module imports nothing + +## 3. TypeScript re-exports + +- [x] 3.1 `visualization/missing-values.ts`: `MISSING_VALUE_TOKENS`, `NA_VALUE`, `NA_DEFAULT_COLOR` from the generated module; module comment points at the source +- [x] 3.2 `visualization/color-scheme.ts`: `KELLYS_COLORS` re-exported; `COLOR_SCHEMES` `satisfies Record` over the generated ids +- [x] 3.3 `visualization/numeric-binning.ts`: `DEFAULT_NUMERIC_PALETTE_ID` re-exported, `GRADIENT_COLOR_SCHEME_IDS` built from `GRADIENT_PALETTE_IDS` +- [x] 3.4 `types.ts`: `PointShape` derived from `POINT_SHAPES` +- [x] 3.5 `parquet/bundle-writer.ts`: a `bool` column's labels checked against `ARROW_BOOLEAN_LABELS` +- [x] 3.6 Mutation check: removing a palette id from the source makes `COLOR_SCHEMES` fail the type check + +## 4. Wiring + +- [x] 4.1 `pnpm precommit` runs `pnpm shared-constants:check` +- [x] 4.2 `bundle-contract.yml` runs `pnpm shared-constants:check` (header comment updated) +- [x] 4.3 `protspace-ci.yml`: drop `color-scheme.ts` / `numeric-binning.ts` from `paths:` and fix the header comment +- [x] 4.4 Contract suite: update the comments that call Python's token set a copy of the web's; keep the token-set and verdict assertions + +## 5. Docs + +- [x] 5.1 `AGENTS.md`: precommit list gains the check; "pin a fact that lives in two places" names `contracts/shared-constants.json` for constants +- [x] 5.2 `CONTRIBUTING.md`: how to change a shared constant +- [x] 5.3 `apps/protspace/CLAUDE.md`: `_generated/` in the package structure, `test_style_warnings.py` row updated, `test_shared_constants.py` row added + +## 6. Verification + +- [x] 6.1 `uv run pytest -m "not slow"` and `uv run ruff check` / `ruff format --check` in `apps/protspace` +- [x] 6.2 `pnpm test:ci`, `pnpm test:contract`, `pnpm format:check`, `pnpm precommit` +- [x] 6.3 Build the wheel and import the generated module from it in a clean environment outside the repo +- [x] 6.4 Reread proposal and design against the final diff; archive the change as the last commit diff --git a/openspec/specs/annotation-cache-semantics/spec.md b/openspec/specs/annotation-cache-semantics/spec.md index 074076314..3cb032c97 100644 --- a/openspec/specs/annotation-cache-semantics/spec.md +++ b/openspec/specs/annotation-cache-semantics/spec.md @@ -389,3 +389,40 @@ the old `none` reads as a missing value. - **THEN** no `none` value is written back under a current stamp, and the cached values of the other Biocentral columns are kept - **AND** a later run fetches Biocentral again + +### Requirement: Caches written before the TMbed payload validation are refreshed + +ProtSpace SHALL NOT reuse cached `predicted_signal_peptide` or `predicted_transmembrane` +values stamped version 3 or earlier. Those versions wrote signal peptide `False` for a +TMbed prediction with an empty topology and read a malformed payload as a topology. A run +that requests such a column SHALL refetch Biocentral once and stamp the rewritten cache as +current. A stale column of a source the refresh refetches anyway SHALL be refreshed with it +rather than dropped. + +#### Scenario: A version-3 cache is asked for the signal peptide + +- **WHEN** a run requests `predicted_signal_peptide` and columns from other sources from a + cache stamped version 3 +- **THEN** ProtSpace refetches Biocentral once and fetches no other source +- **AND** the rewritten cache is stamped current, so the next run fetches nothing + +#### Scenario: Only one TMbed column is requested + +- **WHEN** a run requests `predicted_transmembrane` but not `predicted_signal_peptide` from + a cache that holds both under superseded semantics +- **THEN** the one Biocentral refetch also refreshes `predicted_signal_peptide`, which stays + in the cache and out of the run's result + +### Requirement: Caches written before InterPro's canonical-sequence lookup are refreshed + +ProtSpace SHALL NOT reuse cached InterPro columns stamped version 3 or earlier. Those versions +looked InterPro up by the FASTA's sequence whenever one was given, where InterPro now takes +UniProt's canonical sequence first, so a cached match, or a cached absence of one, may belong to +a different sequence. A run that requests such a column SHALL refetch InterPro once and stamp the +rewritten cache as current. + +#### Scenario: A version-3 cache is asked for pfam + +- **WHEN** a run requests `pfam` and a UniProt column from a cache stamped version 3 +- **THEN** ProtSpace refetches InterPro once, reuses the cached UniProt values, and stamps the + cache current, so the next run fetches nothing diff --git a/openspec/specs/annotation-input/spec.md b/openspec/specs/annotation-input/spec.md new file mode 100644 index 000000000..eb2d352c4 --- /dev/null +++ b/openspec/specs/annotation-input/spec.md @@ -0,0 +1,30 @@ +# annotation-input Specification + +## Purpose + +What the annotation producer takes from its input besides identifiers: the sequences a FASTA +supplies, and which sequence each sequence-backed source (Biocentral, InterPro) is given when +the FASTA and UniProt disagree. + +## Requirements + +### Requirement: FASTA annotation inputs retain their sequences + +The standalone annotation producer SHALL pass sequences supplied in a FASTA input to +sequence-backed annotation sources using the same canonical identifiers as its output. + +#### Scenario: A FASTA identifier is not resolved by UniProt + +- **WHEN** `protspace annotate` receives a FASTA entry whose identifier UniProt cannot + resolve +- **THEN** the entry's FASTA sequence remains available to Biocentral and InterPro +- **AND** the hosted prep pipeline preserves that sequence when it passes its normalized + FASTA to `protspace annotate` + +#### Scenario: A FASTA sequence differs from the UniProt canonical sequence + +- **WHEN** `protspace annotate` receives a FASTA sequence for an identifier that UniProt + resolves to a different canonical sequence +- **THEN** Biocentral receives the FASTA sequence it is being asked to predict +- **AND** InterPro receives the UniProt canonical sequence used by its precomputed + sequence-hash index diff --git a/openspec/specs/atomic-file-publication/spec.md b/openspec/specs/atomic-file-publication/spec.md new file mode 100644 index 000000000..46afefc41 --- /dev/null +++ b/openspec/specs/atomic-file-publication/spec.md @@ -0,0 +1,53 @@ +# atomic-file-publication Specification + +## Purpose + +How a file the package publishes becomes visible: complete or not at all, and with ordinary +new-file permissions. + +## Requirements + +### Requirement: A file this package publishes appears complete or not at all + +A file written for a user or a later run to read SHALL be staged beside its +destination and renamed into place, so an interrupted write leaves either the +previous content or nothing. This covers the bundle, the statistics and +annotation tables `stats` rewrites in place, and a retained query FASTA. A +half-written file at the final path is indistinguishable from a complete one: +the bundle overwrite workflow documents `-b results.parquetbundle -o +results.parquetbundle`, and a retained FASTA's existence is what the next run +reads as a cache hit. + +#### Scenario: A write is interrupted + +- **WHEN** an interruption or error occurs while writing a published file +- **THEN** the destination keeps its previous content, or stays absent if there was none +- **AND** the staged temporary file is removed + +#### Scenario: A download is truncated + +- **WHEN** a query download ends before its compressed stream is complete +- **THEN** decompression fails and nothing is published at the retained path +- **AND** a previously retained complete FASTA is left in place + +#### Scenario: A write completes + +- **WHEN** the staged file is written in full +- **THEN** it replaces the destination in one rename + +### Requirement: A published file carries ordinary new-file permissions + +A file published by staging and rename SHALL carry the permissions a normal new +file would receive under the process umask. Private temporary files are created +owner-only, so publishing one by rename hands the user a file their own group +and other tooling cannot read, which a direct write would never have done. + +#### Scenario: A bundle is written under a permissive umask + +- **WHEN** a bundle is written by a process whose umask allows group or other read +- **THEN** the published bundle is readable accordingly rather than owner-only + +#### Scenario: A retained FASTA is published under a restrictive umask + +- **WHEN** a retained query FASTA is published by a process with a restrictive umask +- **THEN** the published file respects that umask diff --git a/openspec/specs/bundle-conversion/spec.md b/openspec/specs/bundle-conversion/spec.md new file mode 100644 index 000000000..392ac4d93 --- /dev/null +++ b/openspec/specs/bundle-conversion/spec.md @@ -0,0 +1,196 @@ +# bundle-conversion Specification + +## Purpose + +Upgrading a v1 or v2 bundle to v3 without changing what it shows: the `protspace convert` +command, `protspace style` on a legacy input, and the web app's re-export of a loaded legacy +bundle. + +## Requirements + +### Requirement: `protspace convert` SHALL rewrite a v1 or v2 bundle as v3 + +The CLI SHALL provide `protspace convert INPUT [OUTPUT] [--in-place]`, which reads a v1 or v2 +bundle and writes it as a format v3 container carrying the same proteins, annotations and +projections. An unstamped (v1) annotations table SHALL be migrated to the v2 cell grammar before it +is encoded, so its labels keep their parsed hit structure. + +#### Scenario: A v2 bundle is converted + +- **WHEN** `protspace convert old.parquetbundle new.parquetbundle` runs on a v2 bundle +- **THEN** `new.parquetbundle` is a v3 container, and decoding it gives back the tables read from + `old.parquetbundle`, up to the canonical spellings listed in the data-format guide + +#### Scenario: A v1 label contains a percent sign + +- **WHEN** a v1 bundle has the annotation label `50% identity` and is converted +- **THEN** the converted bundle shows the label `50% identity` in the browser and in Python + +#### Scenario: A v1 cell has a semicolon inside parentheses + +- **WHEN** a v1 cell reads `Membrane (single-pass; type I)` +- **THEN** the converted bundle holds it as one label, not two hits + +### Requirement: A legacy bundle's protein ids SHALL be read as the v2 browser read them + +When converting, styling or transferring onto a v1 or v2 bundle, the package SHALL key its +annotation rows as the v2 browser reader did, logging a warning for each departure from the v3 +rules; `protspace transfer` keys them before it transfers, so it runs over the same rows. +Without a `protein_id` or `identifier` column, the id column SHALL be the first column whose name +contains `protein_id`, `identifier`, `id`, `uniprot` or `entry` (case-insensitive, tried in that +order), else the first column, and it SHALL be written as `protein_id`. A row with a null id SHALL +be dropped, and of the rows sharing an id only the last SHALL be kept. + +#### Scenario: The id column is named `id` + +- **WHEN** a v2 bundle whose annotations key their rows by a column `id` is converted +- **THEN** the conversion succeeds, and the v3 bundle's `protein_id` holds that column's values + +#### Scenario: A v1 id column holding FASTA-header ids + +- **WHEN** a v1 bundle whose annotations key their rows by a column `Entry` holding + `sp|P1|A_HUMAN` is converted +- **THEN** the v3 bundle's `protein_id` holds `sp|P1|A_HUMAN` unescaped, so the protein keeps + its annotations: the id column is never migrated as a label + +#### Scenario: Two rows share an id + +- **WHEN** a v2 bundle's annotations hold two rows for `P2` and one row with a null id +- **THEN** the converted bundle has one `P2`, carrying the later row's annotations, and no row + for the null id + +#### Scenario: Transfer on a bundle with a repeated id + +- **WHEN** `protspace transfer` rewrites a v2 bundle whose annotations hold two rows for `P2`, + the earlier one labelled, and one row with a null id +- **THEN** the command succeeds, the earlier `P2` row's label is never a reference, and the v3 + bundle has one `P2` and no row for the null id + +#### Scenario: A legacy bundle v3 cannot hold + +- **WHEN** a v2 bundle whose projections metadata and data name different projections is + converted, styled or transferred onto +- **THEN** the command exits with a usage error that says why, without a traceback, and writes + nothing + +### Requirement: `protspace convert` SHALL NOT overwrite its input unless explicitly asked + +The command SHALL require either an `OUTPUT` path or `--in-place`. It SHALL write over `INPUT` only +when `OUTPUT` is the same path as `INPUT` or `--in-place` is given. + +#### Scenario: Neither an output nor the in-place flag + +- **WHEN** `protspace convert old.parquetbundle` runs +- **THEN** it exits non-zero with a usage error naming `OUTPUT` and `--in-place`, and + `old.parquetbundle` is unchanged + +#### Scenario: In-place conversion + +- **WHEN** `protspace convert old.parquetbundle --in-place` runs on a v2 bundle +- **THEN** `old.parquetbundle` is replaced by its v3 conversion + +#### Scenario: The output names the input + +- **WHEN** `protspace convert old.parquetbundle old.parquetbundle` runs on a v2 bundle +- **THEN** `old.parquetbundle` is replaced by its v3 conversion + +### Requirement: `protspace convert` SHALL preserve settings and statistics + +The converted bundle SHALL carry the input's settings and its statistics part. Settings SHALL be +kept as stored, including the web app's envelope, and statistics SHALL be kept byte for byte. + +#### Scenario: A five-part bundle with settings and statistics + +- **WHEN** a v2 bundle with legend settings and a statistics part is converted +- **THEN** the v3 output's settings equal the input's, and its statistics part is byte-identical + +#### Scenario: A bundle with statistics but no settings + +- **WHEN** a v2 bundle with a zero-byte settings slot and a statistics part is converted +- **THEN** the v3 output has no settings and the same statistics + +### Requirement: `protspace convert` SHALL leave a v3 input untouched + +When `INPUT` is already a v3 container, the command SHALL report that the file is already current, +exit successfully and write nothing. + +#### Scenario: Converting a v3 bundle + +- **WHEN** `protspace convert current.parquetbundle out.parquetbundle` runs on a v3 bundle +- **THEN** it exits `0`, says the file is already v3, `current.parquetbundle` is byte-identical + and `out.parquetbundle` is not created + +### Requirement: `protspace convert` SHALL write its output atomically + +The converted bundle SHALL be written through the package's atomic file writer, so a failed or +interrupted conversion leaves the destination as it was. + +#### Scenario: Encoding fails part-way + +- **WHEN** encoding fails during an in-place conversion +- **THEN** the command exits non-zero, the input file is byte-identical to before, and no temporary + file is left next to it + +### Requirement: `protspace style` SHALL write a v1 or v2 input as v3 + +`protspace style` SHALL write its output as a v3 container whatever the input's format. A v1 or v2 +input SHALL be encoded as `protspace convert` would encode it, with its statistics part kept and the +new settings written, and the command SHALL log one warning that names the input's format version +and says the output is v3. A v3 input SHALL keep every part other than the settings byte for byte. + +#### Scenario: Styling a v2 bundle + +- **WHEN** `protspace style old.parquetbundle styled.parquetbundle --annotation-styles styles.json` + runs on a v2 bundle +- **THEN** `styled.parquetbundle` is a v3 container whose core and payload parts equal those + `protspace convert` writes for `old.parquetbundle`, it carries the new settings, one warning + says the output is v3, and `old.parquetbundle` is unchanged + +#### Scenario: Styling a v1 bundle + +- **WHEN** `protspace style` runs on a v1 bundle +- **THEN** its cells are migrated to the v2 grammar once, as by `protspace convert`, and the output + is v3 + +#### Scenario: A legacy input that `protspace convert` refuses + +- **WHEN** `protspace style` runs on a v2 bundle that `protspace convert` refuses +- **THEN** it exits with a usage error that gives convert's reason, without a traceback, and + writes no output + +#### Scenario: Styling a v3 bundle + +- **WHEN** `protspace style` runs on a v3 bundle +- **THEN** the output's parts other than the settings are byte-identical to the input's, and no + format warning is logged + +### Requirement: Exporting a loaded v1 or v2 bundle from the web app SHALL write v3 + +The web app SHALL write a v3 bundle when a user exports a dataset that was loaded from a v1 or v2 +file, carrying its settings and statistics as for any other export, so the app upgrades a file +without a Python install. + +#### Scenario: Re-exporting a legacy import + +- **WHEN** a user imports a v1 bundle and exports it as a `.parquetbundle` +- **THEN** the exported file is v3, and importing it shows the same proteins, projections, + annotation values and legend settings without the format notice + +### Requirement: A web export SHALL say which proteins of the loaded file it leaves out + +A `.parquetbundle` export from the web app SHALL hold the proteins the app loaded, which leaves +out every protein the source file holds without a coordinate in any projection. When the source +file held such proteins, the export's notification SHALL say how many it leaves out and name +`protspace convert`, which keeps them. + +#### Scenario: Exporting a bundle that holds an annotation-only protein + +- **WHEN** a user loads a bundle whose part 1 holds protein `Q`, which no projection covers, and + exports it as a `.parquetbundle` +- **THEN** the exported file does not hold `Q`, and the export notification says it leaves out 1 + protein without coordinates and names `protspace convert` + +#### Scenario: Exporting a bundle whose every protein is placed + +- **WHEN** a user exports a dataset loaded from a bundle whose every protein has a coordinate +- **THEN** the export notification names only the file diff --git a/openspec/specs/bundle-format-contract/spec.md b/openspec/specs/bundle-format-contract/spec.md index ddab6844d..ae0684799 100644 --- a/openspec/specs/bundle-format-contract/spec.md +++ b/openspec/specs/bundle-format-contract/spec.md @@ -23,12 +23,22 @@ Every `.parquetbundle` used by the contract suite SHALL be produced during the t ### Requirement: Bundles are produced through the real bundle CLI -The generator SHALL invoke the `protspace bundle` command as a subprocess rather than calling `write_bundle` directly, so that the CLI's `identifier` to `protein_id` rename and its format-version stamping are inside the tested surface. A non-zero exit from the subprocess SHALL fail the suite with the subprocess's captured stderr included in the failure message. +The generator SHALL invoke the `protspace bundle` command as a subprocess rather than calling `write_bundle` directly, so that the CLI's `identifier` to `protein_id` rename, its cell-grammar decision and its v3 encoding are inside the tested surface. The generator's stand-in for `protspace annotate` output SHALL be stamped as v2 cell grammar, as `annotate` stamps it. A non-zero exit from the subprocess SHALL fail the suite with the subprocess's captured stderr included in the failure message. -#### Scenario: Format-version stamping is removed from the CLI +#### Scenario: The producer stops writing v3 -- **WHEN** the annotations table is no longer stamped before bundling -- **THEN** the contract suite fails because the reader reports format version `1` instead of `2` +- **WHEN** the CLI writes a container other than the six-part layout, or part 1 stops declaring `protspace_container_version` `3`, or starts carrying `protspace_format_version` +- **THEN** the contract suite fails on the layout assertion + +#### Scenario: The CLI loses the input's cell grammar + +- **WHEN** the CLI stops reading the input's grammar stamp before its rename drops it, so the encoder is handed an unstamped table +- **THEN** the bundle subprocess fails and the suite reports its stderr + +#### Scenario: The CLI treats `annotate` output as plain text + +- **WHEN** the CLI migrates the stamped `annotate` stand-in as if it were v1, escaping its cells a second time +- **THEN** the contract suite fails because the reader surfaces a percent-encoded label still escaped #### Scenario: The bundle subprocess exits non-zero @@ -37,36 +47,36 @@ The generator SHALL invoke the `protspace bundle` command as a subprocess rather ### Requirement: The reader accepts every layout the producer can write -The web reader SHALL accept bundles of three, four, and five parts. Parts beyond the settings part SHALL be ignored rather than parsed or rejected. A zero-byte settings part SHALL be reported as absent settings. +The web reader SHALL accept six-part format v3 bundles, and SHALL keep accepting legacy bundles of three, four, and five parts until legacy read support is removed. A zero-byte settings slot SHALL be reported as absent settings. A statistics part SHALL be carried through unmodified and parsed into statistics rows, without leaking into the settings. A file with more parts than the format defines SHALL be rejected. -#### Scenario: A three-part bundle is read +#### Scenario: A bundle without settings or statistics is read -- **WHEN** a bundle without settings or statistics is read -- **THEN** extraction succeeds and settings are reported as absent +- **WHEN** a bundle written without settings or statistics is read +- **THEN** extraction succeeds and settings and statistics are reported as absent -#### Scenario: A four-part bundle is read +#### Scenario: A bundle with settings is read - **WHEN** a bundle with a settings part is read - **THEN** extraction succeeds and the settings are normalized through the shared settings normalizer -#### Scenario: A five-part bundle with settings and statistics is read +#### Scenario: A bundle with settings and statistics is read - **WHEN** a bundle written with both settings and statistics is read -- **THEN** extraction succeeds, the settings are returned, and the statistics part is ignored +- **THEN** extraction succeeds, the settings are returned, and the statistics are returned as the original bytes and as rows whose columns match the producer's statistics schema -#### Scenario: A five-part bundle carries the zero-byte settings sentinel +#### Scenario: A bundle carries the zero-byte settings sentinel - **WHEN** a bundle written with statistics but without settings is read -- **THEN** extraction succeeds, settings are reported as absent, and the statistics part is ignored +- **THEN** extraction succeeds, settings are reported as absent without entering the settings parser, and the statistics are returned #### Scenario: A bundle carries more parts than the format defines -- **WHEN** a file with more than five parts is read -- **THEN** extraction fails with an error naming the observed part count +- **WHEN** a file with more than six parts is read +- **THEN** extraction fails with an error naming the observed delimiter count ### Requirement: The contract payload exercises the annotation encoding -The generated bundle SHALL carry annotation and projection values that distinguish a correct reader from one that only parses the part layout: at least one percent-encoded label, at least one multi-hit cell using the reserved delimiter, at least one numeric annotation column containing a null, at least one three-dimensional projection, and projection metadata whose JSON contains a value that arrives from parquet as a big integer. +The generated bundle SHALL carry annotation and projection values that distinguish a correct reader from one that only parses the part layout: at least one percent-encoded label, at least one multi-hit cell using the reserved delimiter, at least one numeric annotation column containing a null, at least one boolean annotation column, at least one three-dimensional projection, at least one protein that one projection does not cover, and projection metadata whose JSON contains a value that arrives from parquet as a big integer. #### Scenario: A percent-encoded label round-trips @@ -83,29 +93,25 @@ The generated bundle SHALL carry annotation and projection values that distingui - **WHEN** a numeric annotation is missing for a protein - **THEN** the reader reports that protein as having no value for the annotation rather than a zero or a not-a-number value -#### Scenario: A three-dimensional projection is read - -- **WHEN** a projection declares three dimensions -- **THEN** the reader exposes its `z` coordinates and reports the dimension count from the projection metadata - -#### Scenario: Projection metadata contains a big integer +#### Scenario: A boolean annotation column is read -- **WHEN** projection metadata JSON carries a value parsed from parquet as a big integer -- **THEN** extraction completes without a serialization failure +- **WHEN** the producer writes an annotation column of Arrow type `BOOLEAN` +- **THEN** the reader reports its values as `true` and `false` -### Requirement: The contract covers both conversion implementations +#### Scenario: A three-dimensional projection is read -The reader routes datasets past a row-count threshold through a separate, optimized conversion implementation. The contract SHALL exercise both that implementation and the small-data one, asserting the same annotation encoding contract through each. +- **WHEN** a projection's rows carry `z` coordinates +- **THEN** the reader exposes its `z` coordinates and reports three dimensions -#### Scenario: A dataset large enough to take the optimized path is read +#### Scenario: A protein is missing from one projection -- **WHEN** a generated bundle exceeds the reader's optimized-path row threshold -- **THEN** the optimized conversion entry point decodes its percent-encoded labels, splits its multi-hit cells, and reports its missing numeric values exactly as the small-data path does +- **WHEN** the producer writes a protein with coordinates in one projection and none in another +- **THEN** the reader reports NaN coordinates for it in the projection that does not cover it, and keeps it in the protein list -#### Scenario: Only the optimized implementation regresses +#### Scenario: Projection metadata contains a big integer -- **WHEN** annotation decoding breaks in the optimized implementation but not the small-data one -- **THEN** the contract suite fails +- **WHEN** projection metadata JSON carries a value parsed from parquet as a big integer +- **THEN** extraction completes without a serialization failure ### Requirement: Contract assertions fail when the behavior they name regresses @@ -153,10 +159,11 @@ The contract suite SHALL be a standalone test project that is not part of the de ### Requirement: A bundle never carries the internal lookup columns Every path that writes a bundle's annotations part SHALL omit the internal lookup columns -`organism_id` and `sequence`: `prepare`, `bundle`, `transfer` and the Python bundle-writing -functions. ProtSpace fetches these only to drive the taxonomy and sequence-based lookups. The -annotation-metadata registry and no bundle reader use them. In a bundle they show up in the web -app as meaningless near-unique categorical columns, and the sequences inflate the file. +`organism_id` and `sequence`: `prepare`, `bundle`, `transfer`, `convert`, `style` on a v1 or v2 +input, and the Python bundle-writing functions. ProtSpace fetches these only to drive the taxonomy +and sequence-based lookups. The annotation-metadata registry and no bundle reader use them. In a +bundle they show up in the web app as meaningless near-unique categorical columns, and the +sequences inflate the file. #### Scenario: A table holding internal columns is bundled @@ -171,6 +178,12 @@ app as meaningless near-unique categorical columns, and the sequences inflate th input bundle carried `organism_id` or `sequence` - **THEN** the output bundle carries neither column +#### Scenario: A legacy bundle that carries them is rewritten as v3 + +- **WHEN** `protspace convert`, or `protspace style`, rewrites a v1 or v2 bundle whose annotations + carry `organism_id` or `sequence` +- **THEN** the v3 output carries neither column, and every other annotation column is kept + #### Scenario: The pipeline writes a bundle - **WHEN** `protspace prepare` writes a bundle @@ -226,3 +239,72 @@ annotation producer modules spelled like a missing value. - **WHEN** a module under the annotation retrievers, transformers or parsers gains a string literal spelled like a missing value - **THEN** the Python test suite fails, naming the file and line + +### Requirement: A style for N/A reaches the web's N/A category + +`protspace style` SHALL store every style that names a value the web reads as missing under the +web's N/A key, `__NA__`, whatever spelling the data or the styles file uses. This covers colours, +shapes, `pinnedValues` and `hiddenValues`. The settings it writes SHALL hold at most one N/A +category, and a newly given N/A style SHALL replace a stored one. + +#### Scenario: Missing cells spelled in a way the old alias list missed + +- **WHEN** a column's missing cells are spelled `N/A` and the styles file colours `N/A` +- **THEN** the settings hold that colour under `__NA__` and no `N/A` category + +#### Scenario: Restyling N/A + +- **WHEN** a bundle's settings already colour `__NA__` and `protspace style` is given a new N/A + colour under any N/A spelling +- **THEN** the written settings hold the new colour under `__NA__` + +#### Scenario: Hiding or pinning N/A by its spelling + +- **WHEN** `hiddenValues` or `pinnedValues` name N/A as `N/A`, `""` or `none` +- **THEN** the written settings name it `__NA__`, once + +### Requirement: Produced annotation categories do not collide with missing-value sentinels + +The Python annotation producer SHALL serialize a completed categorical prediction using +a value that the TypeScript bundle consumer preserves as a category rather than +normalizing to missing data. + +#### Scenario: Negative TMbed prediction crosses the bundle boundary + +- **WHEN** Biocentral returns a completed TMbed prediction containing neither an + alpha-helical nor a beta-barrel transmembrane segment +- **THEN** the producer emits `non-transmembrane` +- **AND** the TypeScript visualization data exposes `non-transmembrane` as a categorical + value rather than `N/A` + +#### Scenario: Missing TMbed prediction remains missing + +- **WHEN** Biocentral returns no TMbed prediction for a protein +- **THEN** the producer emits the established missing representation +- **AND** the TypeScript visualization data does not invent a transmembrane category + +#### Scenario: Empty or malformed TMbed payload remains missing + +- **WHEN** Biocentral returns a TMbed prediction object whose optional `value` payload + is `None`, empty, non-string, blank, or contains unsupported topology labels +- **THEN** the producer emits the established missing representation before scanning + topology labels +- **AND** the TypeScript visualization data exposes the protein as `N/A` rather than + `non-transmembrane` +- **AND** the derived signal-peptide annotation is also exposed as `N/A` rather than + `False` + +### Requirement: The contract covers the reader at production scale + +The contract SHALL read, besides the small bundles, a generated bundle large enough that shortcuts sized from the first rows (a label dictionary, a CSR payload, a hit count) would show and that spans several data pages per column, SHALL assert the same annotation encoding contract on it as on the small bundles, and SHALL check its labels, hits, scores, numeric values and coordinates row by row. + +#### Scenario: A dataset of production scale is read + +- **WHEN** the large generated bundle is read +- **THEN** its protein count matches the producer's, and it decodes percent-encoded labels, splits multi-hit cells and reports missing numeric values exactly as the small bundles do + +#### Scenario: Only large payloads regress + +- **WHEN** the reader mishandles a payload only past its first rows, or a chunk that starts past + row 0 +- **THEN** the contract suite fails diff --git a/openspec/specs/imported-dataset-persistence/spec.md b/openspec/specs/imported-dataset-persistence/spec.md new file mode 100644 index 000000000..f8f7fae93 --- /dev/null +++ b/openspec/specs/imported-dataset-persistence/spec.md @@ -0,0 +1,30 @@ +# imported-dataset-persistence Specification + +## Purpose + +How the web app keeps a file the user imports: its bytes are stored in OPFS before its +render starts. + +## Requirements + +### Requirement: An imported file's bytes SHALL be in OPFS before its render starts + +When a user imports a bundle, the web app SHALL finish writing both the file's bytes and its +pending-load record to OPFS before it starts rendering the dataset, so the load-recovery banner can +offer the same file again after a crash during that render. A failure to persist SHALL be reported +as a warning notice and SHALL NOT stop the render. + +#### Scenario: The tab dies during the first render + +- **WHEN** a user imports a bundle and the tab is closed or crashes before the render finishes +- **THEN** on the next visit the recovery banner offers that bundle, and retrying loads it from OPFS + +#### Scenario: The render begins + +- **WHEN** the dataset controller starts rendering an imported dataset +- **THEN** the OPFS write of that file's bytes has already completed or failed + +#### Scenario: OPFS is unavailable + +- **WHEN** writing the imported file to OPFS fails +- **THEN** a persistence-failure warning is shown and the dataset still renders diff --git a/openspec/specs/intermediate-cache-ownership/spec.md b/openspec/specs/intermediate-cache-ownership/spec.md new file mode 100644 index 000000000..45f6bba2c --- /dev/null +++ b/openspec/specs/intermediate-cache-ownership/spec.md @@ -0,0 +1,155 @@ +# intermediate-cache-ownership Specification + +## Purpose + +What a retained projection, embedding, query FASTA or annotation row is owned by, and when it +may be reused, so a cache is never served for data it was not computed from. + +## Requirements + +### Requirement: A cached projection belongs to the data it was computed from + +Cached projection coordinates SHALL be reused only when the embedding matrix and +its identifier order are the same as the run that produced them, in addition to +the existing method, dimension count, and reducer parameters. The logical +embedding name is not evidence of data identity: a resumed embedding cache, a +re-embedded input, a different intersection, or a reordered input all keep the +name while changing the matrix. + +#### Scenario: Input data changes without changing its logical embedding name + +- **WHEN** an embedding set carries the same name, method, and reducer parameters as an earlier run but a different matrix +- **THEN** the reducer runs against the current matrix +- **AND** the earlier coordinates are not reused + +#### Scenario: The same proteins arrive in a different order + +- **WHEN** an embedding set holds the same matrix rows as an earlier run under a different identifier order +- **THEN** the reducer runs again rather than pairing the cached coordinates with the reordered identifiers + +#### Scenario: An input grows between runs + +- **WHEN** proteins are added to an input that keeps its embedding name +- **THEN** the projection is recomputed for every protein in the current run +- **AND** no projection is emitted whose row count differs from the current identifier count + +#### Scenario: Nothing about the input changed + +- **WHEN** a run repeats with the same matrix, identifier order, method, dimensions, and parameters +- **THEN** the cached coordinates are reused without running the reducer + +#### Scenario: Projections are explicitly refreshed + +- **WHEN** a run requests the `projections` refetch stage +- **THEN** cached coordinates are ignored regardless of data identity + +### Requirement: An embedding cache is owned by the backend and model that produced it + +An embedding HDF5 SHALL record the backend and resolved model that produced it, +and a run SHALL refuse to resume from a file another producer wrote. Both +backends resume by identifier alone, so without this a Local-produced vector +satisfies a Biocentral run's resume check and the two are silently mixed in one +dataset. + +#### Scenario: A different backend resumes from the same file + +- **WHEN** a run embeds into an HDF5 recorded as produced by the other backend +- **THEN** the run fails with an error naming the file, the recorded producer, and the remedies (select that backend, choose another output, or refetch the embeddings) +- **AND** the existing file is left untouched rather than extended or deleted + +#### Scenario: The same producer resumes + +- **WHEN** a run embeds into an HDF5 recorded as produced by the same backend and model +- **THEN** the existing identifiers are reused and only outstanding sequences are embedded + +#### Scenario: A file predates producer tracking + +- **WHEN** a run resumes from an HDF5 that records no producer +- **THEN** the run adopts the file, records the current producer, and reports that it did so + +### Requirement: A cached embedding belongs to the sequence it was computed from + +An embedding HDF5 SHALL record, per protein, a digest of the residues the vector +was computed from, and a run SHALL re-embed a protein whose current residues +differ from that digest. Resume matches on identifier alone, so an identifier +whose sequence changed otherwise keeps a vector of the previous residues. + +#### Scenario: A sequence changes while its identifier does not + +- **WHEN** a run embeds a FASTA whose residues for an already-embedded identifier changed +- **THEN** that protein is embedded again and its stored vector and digest are replaced +- **AND** proteins whose residues are unchanged are not embedded again + +#### Scenario: A protein predates sequence digests + +- **WHEN** a stored protein carries no residue digest +- **THEN** it is reused as before and the run does not fail + +### Requirement: An embedding load returns only the proteins that were requested + +Embedding a FASTA SHALL return only that FASTA's proteins, even when the +embedding cache holds more. A cache shared by successive inputs accumulates +every protein it has ever embedded, and returning the accumulation silently +unions unrelated datasets into one bundle. + +#### Scenario: A disjoint input reuses the same embedding cache + +- **WHEN** a FASTA is embedded into a cache that already holds a disjoint input's proteins +- **THEN** the returned embedding set holds only the current FASTA's proteins +- **AND** the retained cache keeps the proteins it already had + +### Requirement: A retained query FASTA is owned by its query text + +A retained query FASTA SHALL be addressed by the query that produced it, for +every caller that retains one. A single shared file name reuses one query's +sequences for a different query whenever both write to the same output +directory. + +#### Scenario: A second query reuses the same output directory + +- **WHEN** a run downloads sequences for one query and a later run requests a different query with the same retained cache directory +- **THEN** the second run downloads its own sequences instead of reusing the first query's FASTA + +#### Scenario: The same query runs again + +- **WHEN** a run repeats a query whose retained FASTA is present and complete +- **THEN** the retained FASTA is reused without downloading it again + +### Requirement: Annotation reuse is decided per identifier and per source + +Annotation retrieval SHALL fetch a source only for the identifiers whose values +the retained cache cannot supply, and SHALL reuse cached values for the rest. +Requesting annotations for identifiers a cache does not hold is the routine +"added sequences to an existing run" case, and re-fetching every source for +every identifier can cost hours at Swiss-Prot scale. + +#### Scenario: The cache covers some of the requested identifiers + +- **WHEN** a run requests annotations for identifiers the retained cache only partly covers +- **THEN** each source is fetched only for the identifiers missing from the cache +- **AND** cached values supply the identifiers the cache already holds +- **AND** the returned annotations cover every requested identifier + +#### Scenario: Taxonomy is reused for organisms already cached + +- **WHEN** identifiers missing from the cache resolve to organisms the cached taxonomy already covers +- **THEN** no taxonomy lookup is made for those organisms +- **AND** organisms the cache does not cover are looked up + +#### Scenario: The cache holds none of the requested identifiers + +- **WHEN** a retained cache describes an entirely different input +- **THEN** every source is fetched for the current identifiers +- **AND** the cached rows are not returned as the current run's annotations + +#### Scenario: A cached superset survives a run that fetches + +- **WHEN** a run fetches annotations for identifiers missing from a cache that also holds identifiers outside the request +- **AND** the run's columns are the columns the cache already holds +- **THEN** the retained cache keeps its other identifiers' rows alongside the newly fetched ones + +#### Scenario: A source fails while filling in missing identifiers + +- **WHEN** a source does not complete for the identifiers being filled in +- **THEN** that source's columns are not written to the cache as empty values for those identifiers +- **AND** the run still returns everything it did retrieve diff --git a/openspec/specs/notebook-generate-output-identity/spec.md b/openspec/specs/notebook-generate-output-identity/spec.md new file mode 100644 index 000000000..7dbdcde49 --- /dev/null +++ b/openspec/specs/notebook-generate-output-identity/spec.md @@ -0,0 +1,36 @@ +# notebook-generate-output-identity Specification + +## Purpose + +How the notebook's Generate action makes its output and parameter scope legible: each run +produces a distinguishable bundle, and the panel states which parameters affect which +methods. + +## Requirements + +### Requirement: Each Generate action produces a distinguishable bundle + +The Preparation notebook SHALL name each generated bundle so that bundles from +different Generate actions are told apart after download. A fixed name makes a +browser file the user opens ambiguous — a second download lands beside the first +as a copy, and opening the earlier file shows the earlier coordinates, which +reads as a stale projection. + +#### Scenario: A second Generate action downloads another bundle + +- **WHEN** a user runs Generate twice in one session +- **THEN** the second bundle's file name differs from the first +- **AND** the notebook reports the name it wrote + +### Requirement: The panel states which parameters affect which methods + +The Preparation notebook SHALL state that the reducer parameter controls apply +only to the methods that consume them. PCA and MDS take no neighbourhood or +perplexity parameter, so changing a slider and regenerating returns a +bit-identical PCA view — the default first projection — which reads as a cache +that ignored the change. + +#### Scenario: A user changes a parameter that the selected method ignores + +- **WHEN** the parameter controls are shown +- **THEN** the panel names the methods each parameter group applies to diff --git a/openspec/specs/parquetbundle-v3/spec.md b/openspec/specs/parquetbundle-v3/spec.md new file mode 100644 index 000000000..5b4b6ca7f --- /dev/null +++ b/openspec/specs/parquetbundle-v3/spec.md @@ -0,0 +1,500 @@ +# parquetbundle-v3 Specification + +## Purpose + +The v3 `.parquetbundle` container that both the Python package and the web app write: its +parts and recorded versions, what a missing coordinate means and which proteins the browser +shows, how dimensions, booleans and list columns are encoded, read support for v1 and v2 +bundles (deprecated until protspace 5.0.0), and the public `decodeParquetBundle` entry point. + +## Requirements + +### Requirement: Every bundle ProtSpace writes SHALL be a format v3 container + +A written bundle SHALL be a six-part container whose first part carries +`protspace_container_version` `"3"` and a `protspace_v3_manifest` in its Parquet key-value +metadata, with the physical layout documented in `docs/guide/data-format.md`. This applies to every bundle written by the Python +package (`prepare`, `bundle`, `transfer`, `style`, `convert` and any other command that writes a +bundle) and by the web app's bundle export. + +#### Scenario: The Python pipeline writes a bundle + +- **WHEN** `protspace bundle` writes a bundle +- **THEN** the file has six parts and part 1's footer declares container version `3` with a + manifest + +#### Scenario: The web app exports a bundle + +- **WHEN** a user exports a loaded dataset as a `.parquetbundle` from the web app +- **THEN** the file has six parts, part 1's footer declares container version `3`, and + `decodeParquetBundle` reads it back to the same protein IDs, projections, annotation values, + settings and statistics that were exported + +#### Scenario: A web export is read by Python + +- **WHEN** a bundle exported from the web app is read with the Python package +- **THEN** it decodes without error and without a legacy-format warning + +### Requirement: v1 and v2 bundles SHALL remain readable until protspace 5.0.0 + +The Python package and the browser SHALL keep reading format v1 and v2 bundles (three to five +parts), producing the same data they did before v3 existed except where this change sets a new +rule for missing coordinates. Support for reading them is deprecated and planned for removal in +protspace 5.0.0. + +#### Scenario: A v1 bundle loads in the browser + +- **WHEN** a three-part bundle with neither `protspace_container_version` nor + `protspace_format_version` in part 1 is loaded +- **THEN** it renders with the legacy parser, and percent-encoded sequences are not decoded + +#### Scenario: A v2 bundle is read in Python + +- **WHEN** `read_tables` is called on a five-part v2 bundle +- **THEN** it returns the stored tables, settings and statistics as before + +### Requirement: Reading a v1 or v2 bundle in Python SHALL log a deprecation warning + +Every read of a v1 or v2 bundle through the Python package SHALL log exactly one warning that says +reading v1/v2 bundles is deprecated, that support will be removed in protspace 5.0.0, and that +`protspace convert` rewrites the file as v3. Reading a v3 bundle SHALL NOT log it. + +#### Scenario: A legacy bundle is read twice + +- **WHEN** a script reads the same v2 bundle twice in one process +- **THEN** the warning is logged twice, once per read, and each names `protspace convert` and + `5.0.0` + +#### Scenario: A v3 bundle is read + +- **WHEN** a v3 bundle is read +- **THEN** no deprecation warning is logged + +### Requirement: The web app SHALL show a non-blocking notice when a user loads a v1 or v2 bundle + +When a user-provided bundle that is format v1 or v2 finishes loading, the web app SHALL show a +notice through its existing notification mechanism that says the file uses an older format whose +support will end in protspace 5.0.0, and that re-exporting it from the app or running +`protspace convert` upgrades it. When the file holds proteins that no projection places, the +notice SHALL instead name only `protspace convert` and say how many proteins an export from the +app would leave out. The notice SHALL NOT block or delay rendering. Datasets the app serves itself +SHALL NOT trigger it. + +#### Scenario: A user imports a v2 bundle + +- **WHEN** a user imports a v2 bundle +- **THEN** the dataset renders and a dismissible notice names re-export and `protspace convert` + +#### Scenario: A user imports a v2 bundle holding annotation-only proteins + +- **WHEN** a user imports a v2 bundle whose annotations part holds 3 proteins no projection places +- **THEN** the notice names `protspace convert` and not re-export, and says an export from the app + leaves out 3 proteins without coordinates + +#### Scenario: A user imports a v3 bundle + +- **WHEN** a user imports a v3 bundle +- **THEN** no format notice is shown + +#### Scenario: The default dataset loads + +- **WHEN** the app loads its bundled default dataset +- **THEN** no format notice is shown, whatever the dataset's format version + +### Requirement: A missing coordinate SHALL be NaN, never zero + +A protein that a projection does not cover SHALL have NaN for every coordinate of that projection: +the v3 encoder SHALL write NaN into part 3, the browser v3 reader SHALL keep NaN, and the browser +legacy reader SHALL produce NaN rather than the zero a fresh `Float32Array` holds. Python's +`decode_v3` SHALL emit a long-format projection row only for a protein whose coordinates in that +projection are finite. + +#### Scenario: The encoder writes a protein absent from one projection + +- **WHEN** a bundle is written where protein `P` has rows in projection `A` but not in projection + `B` +- **THEN** part 3 holds finite values in `A__x` and `A__y` for `P`, and NaN in `B__x` and `B__y` + +#### Scenario: The browser reads that bundle + +- **WHEN** the browser decodes it +- **THEN** `P`'s coordinates in `B` are NaN, not `0` + +#### Scenario: A part 3 shorter than part 1 + +- **WHEN** the browser decodes a v3 bundle whose part 3 holds fewer rows than part 1 +- **THEN** it rejects the file, as Python does, rather than placing the unread proteins at `0` + +#### Scenario: A footer declaring rows its data does not hold + +- **WHEN** the browser decodes a v3 bundle whose part 1 and part 3 footers both declare more rows + than their row groups or pages hold +- **THEN** it rejects the file rather than adding proteins with empty ids at `0` + +#### Scenario: Row groups that split a page + +- **WHEN** the browser decodes a v3 bundle whose part 1 or part 3 row groups declare sizes that + add up to the footer's row count but split a page, so the decoded rows overlap +- **THEN** it rejects the file rather than overwriting one protein with the next and leaving the + last row empty + +#### Scenario: The legacy reader reads the same gap + +- **WHEN** the browser decodes a v2 bundle whose projection `B` has no row for `P` +- **THEN** `P`'s coordinates in `B` are NaN, not `0` + +#### Scenario: A v3 round trip in Python + +- **WHEN** Python decodes that v3 bundle back into tables +- **THEN** the projections table has a row for `P` in `A` and none for `P` in `B` + +#### Scenario: A projection that places no protein + +- **WHEN** Python decodes a v3 bundle whose projection `B` has no finite coordinates for any + protein +- **THEN** the projections table has no row for `B`, and the projections metadata does not list + `B` either, so the decoded tables can be written back + +### Requirement: The browser protein set SHALL be the proteins with a finite coordinate + +The browser reader SHALL include a protein in `protein_ids`, and in every annotation array built +over them, only when it has at least one finite coordinate in at least one projection. A v3 file +SHALL still store annotation-only proteins in part 1, so Python decodes them back. + +#### Scenario: An annotation-only protein in a v3 file + +- **WHEN** a v3 bundle's part 1 holds protein `Q`, which no projection covers +- **THEN** the browser does not list, count, colour or search `Q` +- **AND** Python's decoded annotations table still contains `Q` + +#### Scenario: A label only an unplaced protein carries + +- **WHEN** a v3 bundle's part 1 holds placed proteins labelled `A` and `B` and unplaced proteins + labelled `C`, or with a missing value, in the same column +- **THEN** the browser's labels for that column are `A` and `B` with the order, colours and + dataset hash a bundle without the unplaced proteins gives, with no `C` and no N/A entry + +#### Scenario: A column numeric only over the placed proteins + +- **WHEN** a v2 bundle's text column `size` holds numbers for the placed proteins and `unknown` + for an annotation-only protein, and is converted to v3 +- **THEN** the browser shows `size` as a numeric column with the values and the dataset hash the + v2 file gave, and Python still decodes `unknown` for the annotation-only protein + +#### Scenario: A fractional value only on an unplaced protein + +- **WHEN** a numeric column holds integral values for the placed proteins and `2.5` for an + annotation-only protein, and is written as v3 +- **THEN** the manifest declares the column `numericType` `int`, as the v2 browser inferred it +- **AND** Python still decodes `2.5` for the annotation-only protein + +#### Scenario: A column numeric only on an unplaced protein + +- **WHEN** a v2 bundle's text column is missing on every placed protein and holds `5` for an + annotation-only protein, and is converted to v3 +- **THEN** the browser shows it as a categorical column whose every protein is N/A, as the v2 + file gave it + +#### Scenario: A protein covered by one projection of two + +- **WHEN** protein `P` has coordinates in projection `A` only +- **THEN** `P` is in `protein_ids` and is shown in `A` + +### Requirement: Part 1 SHALL list the proteins in the v2 browser's order + +The v3 encoder SHALL write part 1's rows in the order the v2 browser listed the proteins: by first +appearance of their identifier in the projection rows, then the proteins no projection covers in +the annotations table's order, so that frequency ties in every label dictionary break as they did +in v2. + +#### Scenario: Annotations sorted differently from the projections + +- **WHEN** a bundle is written from annotations in the order `P4, P3, P2, P1`, with `kingdom` + `y, y, x, x`, and projection rows in the order `P1, P2, P3, P4` +- **THEN** part 1 lists `P1, P2, P3, P4`, the `kingdom` dictionary is `x, y`, and the browser + shows the colours and dataset hash the same data gave in v2 + +### Requirement: The manifest SHALL list the projections in the v2 browser's order + +The v3 encoder SHALL list the projections in the manifest, and their columns in part 3, in order +of first appearance of their name in the projection rows, which is the order the v2 browser +listed them in and opened the first of, whatever order the projection metadata gives. Part 2 +SHALL keep the order it was given. + +#### Scenario: Metadata ordered differently from the projection rows + +- **WHEN** a bundle is written whose projection metadata lists `UMAP, PCA` and whose projection + rows start with `PCA` +- **THEN** the manifest lists `PCA, UMAP`, so the browser opens on `PCA` as the v2 browser did, + and part 2 still lists `UMAP, PCA` + +### Requirement: The browser SHALL fold missing-value spellings per hit, as v2 did + +The browser v3 reader SHALL fold a hit whose label is a missing-value spelling (`none`, `NA`, +`null`, ...) into the N/A category only when the hit carries neither a score nor an evidence code, +which is how the v2 reader treated the whole hit, and SHALL keep such a label, with its scores and +evidence, for the hits that carry one. + +#### Scenario: A scored `none` hit and a bare one + +- **WHEN** a v2 bundle's column holds the cells `none|0.5`, `NA|IEA` and `PF1|0.1;none|0.9;none` and + is converted to v3 +- **THEN** the browser shows `none` with score `0.5`, `NA` with evidence `IEA`, and `PF1` and + `none` with their scores for the third protein, whose bare `none` is dropped, as for the v2 file + +### Requirement: The encoder SHALL add projection identifiers missing from the annotations + +The v3 encoder SHALL add an identifier that the projections data names but the annotations table +does not contain as an annotations row whose every annotation is missing, rather than raising. + +#### Scenario: A projected protein without annotations + +- **WHEN** a bundle is written where protein `R` has coordinates but no annotations row +- **THEN** the write succeeds and the browser shows `R` with N/A for every annotation + +### Requirement: A projection's dimension SHALL be derived from its data + +The v3 encoder SHALL record a projection as three-dimensional when any of its rows has a finite +`z`, and as two-dimensional otherwise; a NaN `z` counts as missing, as a null one does. A `dimensions` value in the projection metadata that +disagrees SHALL be ignored, with a warning naming the projection and both values, and part 2 SHALL +be written with the derived value so the projection metadata agrees with the manifest and part 3. +Python's `decode_v3` SHALL return the manifest's dimension in the metadata it decodes. + +#### Scenario: Metadata declares 2 for a projection with z values + +- **WHEN** a projection's rows carry finite `z` values and its metadata says `dimensions` 2 +- **THEN** the manifest records dimension 3, part 3 holds its `__z` column, part 2's `dimensions` + is 3, and a warning is logged + +#### Scenario: Metadata declares 3 for a projection without z values + +- **WHEN** a projection's `z` is null in every row and its metadata says `dimensions` 3 +- **THEN** the manifest records dimension 2, part 3 has no `__z` column, part 2's `dimensions` is + 2, and a warning is logged + +#### Scenario: A z column holding only NaN + +- **WHEN** a projection's `z` is NaN, not null, in every row +- **THEN** the manifest records dimension 2 and part 3 has no `__z` column, so its points are + drawn as the legacy readers drew them rather than culled as missing + +#### Scenario: Metadata agrees with the data + +- **WHEN** the metadata dimension matches the data +- **THEN** no warning is logged and part 2 is written as given + +### Requirement: Boolean annotations SHALL read as `true` and `false` + +An annotation column of Arrow type `BOOLEAN` SHALL be encoded with the labels `true` and `false`, +the spelling the v2 browser reader displayed, by both the Python and the web encoder. + +#### Scenario: A boolean column written by Python + +- **WHEN** a bundle is written from an annotations table with a `BOOLEAN` column +- **THEN** the browser's legend for that column shows `true` and `false`, not `True` and `False` + +#### Scenario: Saved legend settings keep matching + +- **WHEN** a v2 bundle with a `BOOLEAN` column and legend colours saved for `true` and `false` is + converted to v3 +- **THEN** the converted bundle's legend applies the same colours to the same proteins + +### Requirement: A list annotation column SHALL be encoded as a multi-valued column + +The Python v3 encoder SHALL write an annotation column of an Arrow list type as a multi-valued +column with one hit per non-empty element, each element taken literally as its label, and SHALL +refuse a column that has no text form (a struct, a map, a list of lists) with an error that names +the column. + +#### Scenario: GO terms kept as a list column + +- **WHEN** `protspace bundle -a` is given a table whose `go_terms` column is `list` with + the cells `[GO:1, GO:2]` and `[GO:3]` +- **THEN** the bundle is written, the first protein has the two hits `GO:1` and `GO:2`, and Python + decodes the column as the cells `GO:1;GO:2` and `GO:3` + +#### Scenario: A list column holding one number per cell + +- **WHEN** a table's `cluster` column is `list` with the cells `[1]`, `[2]` and `[3]` +- **THEN** the column is written with the labels `1`, `2` and `3`, one hit per protein, and not + as a numeric column + +#### Scenario: That list column through `protspace transfer` + +- **WHEN** `protspace transfer` rewrites a bundle holding that `cluster` column, which it hands + back decoded as the text cells `1`, `2` and `3` +- **THEN** `cluster` is still written with the labels `1`, `2` and `3`, and not as a numeric + column + +#### Scenario: A struct column + +- **WHEN** `protspace bundle -a` is given a table with a struct column +- **THEN** it exits with a usage error that names the column, without a traceback, and writes no + bundle + +### Requirement: A web re-export SHALL keep the column types Python wrote + +The browser reader SHALL keep each v3 manifest column's `sourceType` on the loaded annotation, and +the web exporter SHALL write that `sourceType` back for a column that still fits it, so the Python +package decodes a web re-export of a Python-written bundle to the same column types as the +original. A column that no longer fits its recorded type SHALL be written with the exporter's +inferred type. + +#### Scenario: A boolean column round-trips through the web app + +- **WHEN** a bundle written by Python with a `BOOLEAN` annotation column is loaded in the web app + and exported +- **THEN** Python decodes that column of the export as `bool`, with the same values as the original + +#### Scenario: A boolean column with EAT predictions round-trips through the web app + +- **WHEN** a bundle whose `BOOLEAN` column `protspace transfer` predicted (the prediction labels + spelled `True` / `False`) is loaded in the web app and exported +- **THEN** Python decodes that column of the export as `bool`: the fit is judged on the labels + written to the column, and the predicted rows travel in the companion columns + +#### Scenario: A float column of whole numbers round-trips through the web app + +- **WHEN** a bundle written by Python with a `double` column whose values are all whole numbers is + loaded in the web app and exported +- **THEN** Python decodes that column of the export as `double`, not `int64` + +#### Scenario: EAT companion columns round-trip through the web app + +- **WHEN** a bundle written by `protspace transfer`, whose `__pred_confidence` column is + `float`, is loaded in the web app and exported +- **THEN** Python decodes the export's `__pred_confidence` as `float`, not `double`, and its + `__pred_value` and `__pred_source` columns as `string` + +#### Scenario: A 64-bit hash column round-trips through the web app + +- **WHEN** a bundle written by Python with an `int64` column holding a value beyond ±2^53, which + the encoder stores as exact decimal labels, is loaded in the web app and exported +- **THEN** Python decodes that column of the export as `int64`, with the same values and nulls as + the original + +#### Scenario: An integer column at the ±2^53 edge round-trips through the web app + +- **WHEN** a bundle written by Python with an `int64` column holding `2^53` and `-2^53` is loaded + in the web app and exported +- **THEN** Python decodes that column of the export as `int64`, not `double` + +#### Scenario: A column that no longer fits its recorded type + +- **WHEN** a column recorded as `int32` holds a value outside the `int32` range when it is exported +- **THEN** the export records the exporter's inferred type for it instead + +### Requirement: The container version and the cell grammar SHALL be recorded under separate keys + +A v3 part 1 SHALL declare its container version under `protspace_container_version` and SHALL NOT +carry `protspace_format_version`. `protspace_format_version` SHALL mean only the annotation cell +grammar (`2`, or absent for v1), on legacy parts and on v2-shaped tables. Both readers SHALL +detect a v3 container from `protspace_container_version` alone, SHALL read a file without it as +legacy, and SHALL reject a file whose part count disagrees with that key. + +#### Scenario: A written v3 bundle's footer + +- **WHEN** Python or the web app writes a bundle +- **THEN** part 1's footer has `protspace_container_version` `"3"` and no + `protspace_format_version` + +#### Scenario: Python decodes a v3 bundle + +- **WHEN** `read_tables` decodes a v3 bundle +- **THEN** the annotations table is stamped `protspace_format_version` `2` and carries no + `protspace_container_version` + +#### Scenario: Six parts with only the grammar key + +- **WHEN** a six-part file whose part 1 carries `protspace_format_version` `"3"` but no + `protspace_container_version` is read, in Python or in the browser +- **THEN** it is rejected with an error naming `protspace_container_version`, not read as legacy + +#### Scenario: A container key in a legacy-sized file + +- **WHEN** a three to five part file whose part 1 carries `protspace_container_version` is read +- **THEN** it is rejected rather than read as legacy cells + +#### Scenario: The legacy notice reports the version + +- **WHEN** a user imports a v2 bundle +- **THEN** `decodeParquetBundle` reports format version `2`, read from `protspace_format_version` + +### Requirement: The v3 encoder SHALL refuse an annotations table of undeclared cell grammar + +The Python v3 encoder SHALL refuse an annotations table that is not stamped with +`protspace_format_version` `2`, rather than migrating it as v1. A caller that holds v1 cells +SHALL migrate them explicitly before encoding. `protspace bundle -a` SHALL take the grammar from +its input's stamp, read before any operation drops it, SHALL read the pipeline's own annotation +cache as v2 whether or not it is stamped, and SHALL treat any other unstamped input table as +legacy v1 plain text. + +#### Scenario: An already-v2 table that lost its stamp + +- **WHEN** `write_bundle` is given an annotations table with v2 cells and no stamp +- **THEN** it raises an error naming `protspace_format_version`, writes nothing, and no cell is + escaped a second time + +#### Scenario: `annotate` output is bundled + +- **WHEN** `protspace bundle -a` is given `protspace annotate` output, stamped v2, whose id column + is `identifier` +- **THEN** the cells in the bundle are the input's cells, not migrated again + +#### Scenario: The prepare annotation cache is bundled + +- **WHEN** `protspace bundle -a` is given the `tmp/all_annotations.parquet` cache `prepare` kept, + whose cells hold `Membrane%3B single-pass`, stamped or written before the cache was stamped +- **THEN** the bundle shows the label `Membrane; single-pass`, not `Membrane%3B single-pass` + +#### Scenario: A hand-made annotations table is bundled + +- **WHEN** `protspace bundle -a` is given an unstamped table with the cells `50% identity` and + `Membrane (single-pass; type I)` +- **THEN** the bundle shows the labels `50% identity` and `Membrane (single-pass; type I)`, the + second as one label + +#### Scenario: A hand-made table with a pandas category column + +- **WHEN** `protspace bundle -a` is given an unstamped table whose column, saved from a pandas + `category` column, reads back as a dictionary of strings holding + `Membrane (single-pass; type I)` and `x%41y` +- **THEN** its cells are migrated as the plain string column's are, and the bundle shows those + two labels unchanged + +#### Scenario: Transfer on a v1 bundle + +- **WHEN** `protspace transfer` rewrites a v1 bundle +- **THEN** the v1 cells are migrated to v2 once and the output is a v3 bundle + +### Requirement: `decodeParquetBundle` SHALL be the public entry point for reading a bundle + +`@protspace/core` SHALL export `decodeParquetBundle(arrayBuffer)`, which reads a bundle of any +supported format version, returns its visualization data and settings, and reports the container +format version it read. `extractRowsFromParquetBundle` SHALL be documented as v1/v2-only and +deprecated, and SHALL reject a v3 bundle with an error that names `decodeParquetBundle`. + +#### Scenario: An embedding page reads any bundle + +- **WHEN** a page imports `decodeParquetBundle` from `@protspace/core` and passes it a v2 or a v3 + bundle +- **THEN** it receives the same visualization data the web app renders, and the format version + +#### Scenario: A Node process decodes a legacy bundle + +- **WHEN** a Node script awaits `decodeParquetBundle` on a v1 or v2 bundle large enough for the + chunked conversion, which yields to the event loop between chunks +- **THEN** the promise resolves and the decode leaves no MessagePort or other handle open, so the + process exits when the script ends + +#### Scenario: The row extractor is handed a v3 bundle + +- **WHEN** `extractRowsFromParquetBundle` is called on a v3 bundle +- **THEN** it throws an error that points to `decodeParquetBundle` rather than returning rows built + from integer codes + +#### Scenario: The developer docs + +- **WHEN** a reader follows `docs/developers/embedding.md` or the API reference +- **THEN** the examples use `decodeParquetBundle`, and `extractRowsFromParquetBundle` is marked + deprecated and v1/v2-only diff --git a/openspec/specs/point-visibility/spec.md b/openspec/specs/point-visibility/spec.md index 494cb768c..e3c4fc1a1 100644 --- a/openspec/specs/point-visibility/spec.md +++ b/openspec/specs/point-visibility/spec.md @@ -10,7 +10,7 @@ The single model that decides what a plotted point looks like and whether it can The system SHALL compute per-point display state (tier `hidden | faded | base | selected`, numeric opacity, base opacity, interactivity) in exactly one pure module, and -every consumer (style getters, quadtree construction, hover, click, brush, lasso) SHALL +every consumer (style getters, point grid construction, hover, click, brush, lasso) SHALL read that module rather than re-deriving visibility. Category focus (Shift + hover) SHALL be an input to that module, not a separate opacity path. @@ -18,7 +18,7 @@ be an input to that module, not a separate opacity path. - **WHEN** any combination of legend-hide, selection, highlight, and category-focus state is active -- **THEN** the opacity used for rendering, the points indexed by the quadtree, and the +- **THEN** the opacity used for rendering, the points indexed by the point grid, and the points accepted by hover/click/brush/lasso hit-tests are all derived from the same model and can never disagree @@ -51,7 +51,7 @@ axes do not re-fit). The system SHALL assign opacity exactly `0` (not merely a small value) to annotation-hidden points: consumers gate with a mix of comparisons (`=== 0` at export -culling and hover/click, `> 0` at tracking and the quadtree, `< 0.001` at the shader +culling and hover/click, `> 0` at tracking and the point grid, `< 0.001` at the shader discard) that agree on "invisible and non-interactive" only at exactly `0`. #### Scenario: Hidden value yields exact zero @@ -134,7 +134,7 @@ a category focus is active; selected and highlighted points get the selected opa The system SHALL define interactivity as `opacity > 0` using the configured opacity values (a `fadedOpacity` of `0` makes faded points non-interactive), and SHALL evaluate it against current inputs at event time so hit-testing is correct even while the -quadtree rebuild is rAF-deferred. The renderer-capacity gate (`isPointRendered`) remains +point grid rebuild is rAF-deferred. The renderer-capacity gate (`isPointRendered`) remains a separate check outside the model. #### Scenario: Faded points are clickable under default config @@ -145,8 +145,8 @@ a separate check outside the model. #### Scenario: Hidden points are not clickable - **WHEN** a point's opacity is `0` -- **THEN** it is excluded from the quadtree and rejected by hover/click/brush/lasso even - during the one-frame quadtree staleness window +- **THEN** it is excluded from the point grid and rejected by hover/click/brush/lasso even + during the one-frame point grid staleness window ### Requirement: Depth is independent of hidden state @@ -271,3 +271,63 @@ opacity), then focus. - **THEN** the renderer draws in its selection-active mode (out-of-focus points flat without density blending, in-focus points blended on top), and depth follows base opacity so in-focus points render above out-of-focus points + +### Requirement: Points without finite coordinates SHALL be culled when plot data is built + +The system SHALL remove every point whose coordinates in the selected projection are not all +finite, at the single step that builds plot data from a projection, alongside the query-filter and +isolation culls. No consumer SHALL draw, hit-test, lasso, brush, contour, stack, depth-sort, export or include in the scale domains a point with a non-finite +coordinate, and none SHALL depend on a finiteness check of its own to achieve this. A culled protein SHALL stay in the +dataset (`protein_ids`, annotation arrays, legend counts, search), and in isolation mode it SHALL +stay in the isolated subset: isolation is membership in the isolation layers, never the set of +points the selected projection places. + +#### Scenario: A protein missing from the selected projection + +- **WHEN** protein `P` has NaN coordinates in the selected projection +- **THEN** `P` is absent from the plot data, is not drawn, is not returned by hover, click, brush or + lasso, and does not contribute to the density contours + +#### Scenario: Scale domains ignore the missing point + +- **WHEN** plot data is built for a projection where some proteins have NaN coordinates +- **THEN** the scale domains are computed from the finite points only, and no point is drawn at the + position of `(0, 0)` on their behalf + +#### Scenario: Switching between a complete and an incomplete projection + +- **WHEN** the user switches from projection `A`, where `P` has coordinates, to projection `B`, + where it does not, and back +- **THEN** `P` disappears in `B` and reappears in `A`, and no stale coordinate from the other + projection is drawn for it + +#### Scenario: Exports omit the missing point + +- **WHEN** an image is exported while `P` is culled +- **THEN** the exported image contains no marker for `P` + +#### Scenario: A culled protein stays in the dataset + +- **WHEN** `P` is culled from the selected projection but has coordinates in another +- **THEN** `P` is still counted in its legend category and can be found by search + +#### Scenario: An isolated protein the selected projection does not place + +- **WHEN** `P` and `Q` are isolated in projection `A` and the user switches to projection `B`, + which does not place `P` +- **THEN** the current data (the `.parquetbundle` export and the legend counts) still holds `P`, + with its annotations and its coordinates in `A`, and an isolated subset of which `B` places no + point is still that subset, not the whole dataset + +#### Scenario: Isolating a selection the selected projection does not fully place + +- **WHEN** `P` and `Q` are selected in projection `A`, the user switches to `B`, which does not + place `P`, and isolates the selection +- **THEN** the isolation layer holds `P` and `Q`, and `P` is drawn again on the way back to `A` + +#### Scenario: Isolating proteins of which the selected projection places one + +- **WHEN** `P`, `Q` and `R` are selected in projection `A`, the user switches to `B`, which + places only `R`, and isolates the selection +- **THEN** selection mode stays available, since the isolated subset holds three proteins, and + the isolation reports that size diff --git a/openspec/specs/shared-constants/spec.md b/openspec/specs/shared-constants/spec.md new file mode 100644 index 000000000..7fb4a43dc --- /dev/null +++ b/openspec/specs/shared-constants/spec.md @@ -0,0 +1,118 @@ +# shared-constants Specification + +## Purpose + +The Python package and the web app must hold the same values for a family of constants +(missing-value tokens, the N/A key and colour, Kelly's colours, palette ids, point shapes, +boolean labels). This capability keeps them in step by construction: one source file, +generated into a committed module per language and checked on every pull request, while the +rules both sides implement stay pinned by running both on the same inputs. + +## Requirements + +### Requirement: Shared constants have one source + +Every constant both sides must agree on SHALL be defined once, in the shared source +`contracts/shared-constants.json`, together with the reason it is shared. This covers the +missing-value tokens, the `__NA__` sentinel, the N/A swatch colour, Kelly's colours in slot +order, the categorical and gradient palette ids, the default categorical and numeric palettes, +the labels of an Arrow boolean column, and the point shapes. Neither language SHALL hold a hand-written +copy of these values. Each SHALL read them from its generated module, and the names existing +code imports SHALL stay importable from where they were. + +#### Scenario: A missing-value spelling is added + +- **WHEN** a contributor adds a spelling to `MISSING_VALUE_TOKENS` in + `contracts/shared-constants.json` and regenerates +- **THEN** the Python package's `protspace.core.constants.MISSING_VALUE_TOKENS` and the web's + `MISSING_VALUE_TOKENS` both contain it, and no other file is edited + +#### Scenario: A palette is added to one language only + +- **WHEN** `COLOR_SCHEMES` in `color-scheme.ts` gains a palette whose id + `contracts/shared-constants.json` does not list, or loses one it does list +- **THEN** the TypeScript type check fails + +#### Scenario: A Python re-export is replaced by a literal + +- **WHEN** one of the Python names re-exported from the generated module (for example + `KELLYS_COLORS` in `settings_converter.py`) is redefined as a hand-written value +- **THEN** the Python test suite fails, naming the constant + +### Requirement: Generated modules are committed and checked + +The generator SHALL write `packages/utils/src/generated/shared-constants.ts` and +`apps/protspace/src/protspace/_generated/shared_constants.py`, and both SHALL be committed. +`pnpm shared-constants:check` SHALL fail, naming the stale file and the command that +regenerates it, whenever either committed module differs from what the source generates. The +check SHALL run in `pnpm precommit` and in a CI job that runs on every pull request with no +path filter. + +#### Scenario: A generated module is edited by hand + +- **WHEN** a pull request changes only `apps/protspace/src/protspace/_generated/shared_constants.py` +- **THEN** the unfiltered CI job fails on `pnpm shared-constants:check` + +#### Scenario: The source changes without regenerating + +- **WHEN** `contracts/shared-constants.json` is edited and the generated modules are not + regenerated +- **THEN** `pnpm precommit` fails and names `pnpm shared-constants` + +#### Scenario: The generated modules pass their language's formatter + +- **WHEN** the generator has run +- **THEN** `prettier --check` passes on the TypeScript module and `ruff format --check` and + `ruff check` pass on the Python module + +### Requirement: The Python package needs nothing outside its own directory + +The Python package SHALL ship its shared constants inside `protspace` itself, so that a wheel +or sdist built from `apps/protspace` imports and runs without the repository's `contracts/` +or `packages/` directories. + +#### Scenario: The package is installed from its wheel + +- **WHEN** the protspace wheel is built from `apps/protspace` and installed outside the + repository +- **THEN** `protspace._generated.shared_constants` is importable and every module that + re-exports from it imports + +### Requirement: The source is validated before generation + +The generator SHALL refuse to write when the source is malformed (an unknown kind, a value of +the wrong type, a duplicate entry, a name that is not upper snake case, a doc line over 80 +characters) or breaks an invariant. The invariants are: every missing-value token is trimmed +and lower-case; `NA_VALUE`, lower-cased, is a missing-value token; no palette id is both +categorical and gradient; `DEFAULT_CATEGORICAL_PALETTE_ID` is a categorical palette id; +`DEFAULT_NUMERIC_PALETTE_ID` is a gradient palette id; +`ARROW_BOOLEAN_LABELS` is exactly the false label then the true label. + +#### Scenario: An upper-case token is added + +- **WHEN** `MISSING_VALUE_TOKENS` gains `N/A` +- **THEN** the generator exits non-zero, says that the matcher compares lower-cased cells, + and writes nothing + +#### Scenario: The numeric default is not a gradient + +- **WHEN** `DEFAULT_NUMERIC_PALETTE_ID` names a categorical palette +- **THEN** the generator exits non-zero and writes nothing + +#### Scenario: The categorical default is not a categorical palette + +- **WHEN** `DEFAULT_CATEGORICAL_PALETTE_ID` names a gradient or unknown palette +- **THEN** the generator exits non-zero and writes nothing + +### Requirement: Behaviour stays pinned by running both implementations + +Behavioural rules SHALL NOT be moved into the shared source: how a value is used, such as +how a cell is matched against the missing-value tokens, is code. The bundle contract suite +SHALL keep checking them by running the Python and TypeScript implementations on the same +probe inputs. + +#### Scenario: One side stops lower-casing before matching + +- **WHEN** the TypeScript `normalizeMissingValue` stops lower-casing, while the shared tokens + are unchanged +- **THEN** `pnpm shared-constants:check` still passes and the bundle contract suite fails diff --git a/package.json b/package.json index 4fa97ce7a..9bf376778 100644 --- a/package.json +++ b/package.json @@ -12,6 +12,8 @@ "docs:annotations:check": "tsx docs/scripts/generate-annotations.mts --check", "docs:examples": "tsx docs/scripts/generate-examples.mts", "docs:examples:check": "tsx docs/scripts/generate-examples.mts --check", + "shared-constants": "tsx scripts/shared-constants/generate-shared-constants.mts", + "shared-constants:check": "tsx scripts/shared-constants/generate-shared-constants.mts --check", "examples:fetch": "tsx scripts/examples/fetch.mts", "docs:build": "NODE_ENV=production vitepress build docs", "docs:preview": "vitepress preview docs", @@ -38,7 +40,7 @@ "format:check": "prettier --check \"**/*.{ts,tsx,js,jsx,css,scss,md,json,yml,yaml,mjs,cjs}\"", "knip": "knip", "knip:fix": "knip --fix", - "precommit": "pnpm lint-staged && pnpm quality && pnpm docs:annotations:check && pnpm docs:examples:check && pnpm docs:build", + "precommit": "pnpm lint-staged && pnpm quality && pnpm shared-constants:check && pnpm docs:annotations:check && pnpm docs:examples:check && pnpm docs:build", "prepare": "husky", "landing:data": "tsx scripts/landing-data/build-landing-data.mts" }, diff --git a/packages/core/src/components/data-loader/utils/bundle-v3.ts b/packages/core/src/components/data-loader/utils/bundle-v3.ts index 3f1391713..bad26cc44 100644 --- a/packages/core/src/components/data-loader/utils/bundle-v3.ts +++ b/packages/core/src/components/data-loader/utils/bundle-v3.ts @@ -46,6 +46,7 @@ import { type CsrEvidence, type CsrScores, type Projection, + DEFAULT_CATEGORICAL_PALETTE_ID, rankByFrequency, remapCsr, V3_EVIDENCE_DICT_NAME, @@ -1104,7 +1105,10 @@ export async function readV3Bundle( const drop = missingLabels(labels, keep); const remap = rowsDropped || keep ? rankByHits(storage, labels, drop) : compactLabels(labels, drop); - const { colors, shapes } = generateColorsAndShapes('kellys', labels.length); + const { colors, shapes } = generateColorsAndShapes( + DEFAULT_CATEGORICAL_PALETTE_ID, + labels.length, + ); if (storage instanceof Int32Array) { if (remap) { diff --git a/packages/core/src/components/data-loader/utils/conversion.ts b/packages/core/src/components/data-loader/utils/conversion.ts index e35d00012..33990746c 100644 --- a/packages/core/src/components/data-loader/utils/conversion.ts +++ b/packages/core/src/components/data-loader/utils/conversion.ts @@ -7,6 +7,7 @@ import type { } from '@protspace/utils'; import { COLOR_SCHEMES, + DEFAULT_CATEGORICAL_PALETTE_ID, getEatConfidenceAnnotationKey, getProteinAnnotationIndices, getProteinEvidence, @@ -1083,7 +1084,10 @@ function convertBundleFormatData( const valueToIndex = new Map(); uniqueValues.forEach((value, idx) => valueToIndex.set(value, idx)); - const { colors, shapes } = generateColorsAndShapes('kellys', uniqueValues.length); + const { colors, shapes } = generateColorsAndShapes( + DEFAULT_CATEGORICAL_PALETTE_ID, + uniqueValues.length, + ); const annotationDataArray = uniqueProteinIds.map((proteinId) => { const value = annotationMap.get(proteinId); @@ -1356,7 +1360,10 @@ function convertLegacyFormatData( const valueToIndex = new Map(); uniqueValues.forEach((value, idx) => valueToIndex.set(value, idx)); - const { colors, shapes } = generateColorsAndShapes('kellys', uniqueValues.length); + const { colors, shapes } = generateColorsAndShapes( + DEFAULT_CATEGORICAL_PALETTE_ID, + uniqueValues.length, + ); const annotationDataArray = labelsByRow.map((valueArray) => valueArray.map((v) => valueToIndex.get(v) ?? -1), @@ -1482,7 +1489,7 @@ const SUPPORTED_SHAPES = [ * `colors[i % colors.length]` and `shapes[i % shapes.length]` to handle * categories beyond the cap (they wrap around to the beginning of the cycle). * - * @param paletteId - Key of the palette in COLOR_SCHEMES (falls back to 'kellys') + * @param paletteId - Key of the palette in COLOR_SCHEMES (falls back to DEFAULT_CATEGORICAL_PALETTE_ID) * @param count - Number of (color, shape) pairs to generate */ export function generateColorsAndShapes( @@ -1656,7 +1663,10 @@ async function extractAnnotationsByProtein( const valueToIndex = new Map(); uniqueValues.forEach((val, idx) => valueToIndex.set(val, idx)); - const { colors, shapes } = generateColorsAndShapes('kellys', uniqueValues.length); + const { colors, shapes } = generateColorsAndShapes( + DEFAULT_CATEGORICAL_PALETTE_ID, + uniqueValues.length, + ); // === Pass 2: map cached labels → dictionary indices. Use Int32Array for // strict single-valued columns to avoid the per-protein number[] cliff. === diff --git a/packages/core/src/components/legend/controllers/persistence-controller.test.ts b/packages/core/src/components/legend/controllers/persistence-controller.test.ts index 41c87add6..94e66afbb 100644 --- a/packages/core/src/components/legend/controllers/persistence-controller.test.ts +++ b/packages/core/src/components/legend/controllers/persistence-controller.test.ts @@ -16,6 +16,7 @@ function createTestItem(value: string, zOrder: number): LegendItem { } vi.mock('@protspace/utils', () => ({ + DEFAULT_CATEGORICAL_PALETTE_ID: 'kellys', generateDatasetHash: vi.fn((ids: string[]) => `hash_${ids.join('_')}`), buildStorageKey: vi.fn( (prefix: string, hash: string, annotation: string) => `${prefix}_${hash}_${annotation}`, diff --git a/packages/core/src/components/legend/legend-helpers.ts b/packages/core/src/components/legend/legend-helpers.ts index bbb566728..386417424 100644 --- a/packages/core/src/components/legend/legend-helpers.ts +++ b/packages/core/src/components/legend/legend-helpers.ts @@ -1,3 +1,4 @@ +import { DEFAULT_CATEGORICAL_PALETTE_ID } from '@protspace/utils'; import type { LegendItem, OtherItem, @@ -147,7 +148,7 @@ export function createDefaultSettings(selectedAnnotation: string): LegendPersist hiddenValues: [], categories: {}, enableDuplicateStackUI: LEGEND_DEFAULTS.enableDuplicateStackUI, - selectedPaletteId: 'kellys', + selectedPaletteId: DEFAULT_CATEGORICAL_PALETTE_ID, }; } diff --git a/packages/core/src/components/legend/legend.ts b/packages/core/src/components/legend/legend.ts index cd8f61b6e..5eafd3191 100644 --- a/packages/core/src/components/legend/legend.ts +++ b/packages/core/src/components/legend/legend.ts @@ -3,6 +3,7 @@ import { property, query, state } from 'lit/decorators.js'; import { customElement } from '../../utils/safe-custom-element'; import { COLOR_SCHEMES, + DEFAULT_CATEGORICAL_PALETTE_ID, DEFAULT_NUMERIC_PALETTE_ID, DEFAULT_NUMERIC_STRATEGY, getNumericBinLabelMap, @@ -252,7 +253,7 @@ export class ProtspaceLegend extends LitElement { @state() private _colorPickerItem: string | null = null; @state() private _colorPickerPosition: { x: number; y: number } | null = null; @state() private _showShapePicker = false; - @state() private _selectedPaletteId = 'kellys'; + @state() private _selectedPaletteId: string = DEFAULT_CATEGORICAL_PALETTE_ID; @state() private _numericSettingsByAnnotation: NumericAnnotationDisplaySettingsMap = {}; @state() private _numericManualOrderIdsByAnnotation: Record = {}; @state() private _eatCounts: EatPopulationCounts | null = null; @@ -348,7 +349,7 @@ export class ProtspaceLegend extends LitElement { shapeSize: LEGEND_DEFAULTS.symbolSize, enableDuplicateStackUI: false, annotationSortModes: {}, - selectedPaletteId: 'kellys', + selectedPaletteId: DEFAULT_CATEGORICAL_PALETTE_ID, numericStrategy: DEFAULT_NUMERIC_STRATEGY, reverseGradient: false, }; @@ -1204,7 +1205,7 @@ export class ProtspaceLegend extends LitElement { this._showSettingsDialog = false; this._showOtherDialog = false; this._colorPickerItem = null; - this._selectedPaletteId = 'kellys'; + this._selectedPaletteId = DEFAULT_CATEGORICAL_PALETTE_ID; this._numericSettingsByAnnotation = {}; this._numericManualOrderIdsByAnnotation = {}; this._eatCounts = null; @@ -1969,7 +1970,7 @@ export class ProtspaceLegend extends LitElement { if (isNumericAnnotation) { this._applyNumericDisplayLabels(); this._applyDerivedNumericColors(); - } else if (this._selectedPaletteId !== 'kellys') { + } else if (this._selectedPaletteId !== DEFAULT_CATEGORICAL_PALETTE_ID) { this._applyPaletteColors(this._selectedPaletteId); } @@ -2175,7 +2176,7 @@ export class ProtspaceLegend extends LitElement { private _normalizeCategoricalPaletteId(paletteId: string | undefined | null): string { if (!paletteId || isGradientPalette(paletteId)) { - return 'kellys'; + return DEFAULT_CATEGORICAL_PALETTE_ID; } return paletteId; } @@ -2658,7 +2659,9 @@ export class ProtspaceLegend extends LitElement { this._clearShapeSize(); const isNumericAnnotation = this._isCurrentAnnotationNumeric(); - this._selectedPaletteId = isNumericAnnotation ? DEFAULT_NUMERIC_PALETTE_ID : 'kellys'; + this._selectedPaletteId = isNumericAnnotation + ? DEFAULT_NUMERIC_PALETTE_ID + : DEFAULT_CATEGORICAL_PALETTE_ID; if (isNumericAnnotation) { this._numericSettingsByAnnotation = { ...this._numericSettingsByAnnotation, diff --git a/packages/utils/src/generated/shared-constants.ts b/packages/utils/src/generated/shared-constants.ts new file mode 100644 index 000000000..584be1d76 --- /dev/null +++ b/packages/utils/src/generated/shared-constants.ts @@ -0,0 +1,136 @@ +// Generated by scripts/shared-constants/generate-shared-constants.mts from +// contracts/shared-constants.json. Do not edit: change the JSON, run +// `pnpm shared-constants` and commit both generated modules. +// `pnpm shared-constants:check` fails while they disagree. + +/** + * Strings that mean "this annotation value is missing". A cell is missing + * when it is null, a non-finite number, blank, or, trimmed and lower-cased, + * one of these: `normalizeMissingValue` in the web, `is_missing_value` in + * Python. That rule is code on each side, so the bundle contract suite runs + * both on the same probe spellings. + * + * `__na__` reserves the spelling of NA_VALUE: the bundle writer drops that + * token unconditionally (it is the sentinel a missing cell carries), so a + * genuine category literally spelled `__NA__` would be silently deleted on + * export. Reading it as missing at ingestion makes the two agree. + */ +export const MISSING_VALUE_TOKENS: ReadonlySet = new Set([ + 'na', + 'n/a', + 'nan', + 'null', + 'none', + '__na__', +]); + +/** + * Internal key of the N/A category: the legend value a missing cell gets, + * and the key its colour and shape are saved under in a bundle's settings. + * Never displayed; the web shows it as `N/A`. + */ +export const NA_VALUE = '__NA__'; + +/** + * Default swatch colour of the N/A category, which a saved category colour + * overrides (categorical annotations only). Python gives a pinned N/A this + * colour, and `protspace style --generate-template` pre-fills it for N/A, so + * a CLI-styled legend matches the web's default. + */ +export const NA_DEFAULT_COLOR = '#DDDDDD'; + +/** + * Kelly's 21 colours of maximum contrast, in the order categories take them + * (the category in slot i gets colour i). Medium gray (#848482) is left out, + * too close to the greys of Other (#999999) and N/A. Very light and very + * dark gray come last, for the least frequent categories. In order: vivid + * yellow, strong purple, vivid orange, very light blue, vivid red, grayish + * yellow, vivid green, strong purplish pink, strong blue, strong yellowish + * pink, strong violet, vivid orange yellow, strong purplish red, vivid + * greenish yellow, strong reddish brown, vivid yellowish green, deep + * yellowish brown, vivid reddish orange, dark olive green, very light gray, + * very dark gray. + * + * Reference: Kelly, Kenneth L. Twenty-two colors of maximum contrast. + * Color Engineering 3.26-27 (1965). + */ +export const KELLYS_COLORS = [ + '#F3C300', + '#875692', + '#F38400', + '#A1CAF1', + '#BE0032', + '#C2B280', + '#008856', + '#E68FAC', + '#0067A5', + '#F99379', + '#604E97', + '#F6A600', + '#B3446C', + '#DCD300', + '#882D17', + '#8DB600', + '#654522', + '#E25822', + '#2B3D26', + '#F2F3F4', + '#222222', +] as const; + +/** + * Palettes for categorical annotations: the only ids a bundle's + * `selectedPaletteId` may name. `protspace style` warns on any other id, and + * the web draws a gradient or unknown id with DEFAULT_CATEGORICAL_PALETTE_ID. + * The web's `COLOR_SCHEMES` holds the colours and must list exactly these ids + * plus GRADIENT_PALETTE_IDS (the type check fails otherwise). A new palette + * also goes in docs/guide/styling.md (Color palettes). + */ +export const CATEGORICAL_PALETTE_IDS = [ + 'kellys', + 'okabeIto', + 'tolBright', + 'set2', + 'dark2', + 'tableau10', +] as const; + +/** + * Sequential palettes for numeric annotations, chosen in the web's numeric + * legend settings rather than through `selectedPaletteId`. + */ +export const GRADIENT_PALETTE_IDS = ['viridis', 'cividis', 'inferno', 'batlow', 'plasma'] as const; + +/** + * The palette a categorical annotation starts on, and the one the web falls + * back to; one of CATEGORICAL_PALETTE_IDS. + */ +export const DEFAULT_CATEGORICAL_PALETTE_ID = 'kellys'; + +/** + * The gradient a numeric annotation starts on; one of GRADIENT_PALETTE_IDS. + */ +export const DEFAULT_NUMERIC_PALETTE_ID = 'batlow'; + +/** + * Shapes a category can be drawn with: the `shape` values a bundle's + * settings may hold. The web's `PointShape` type is exactly these. + */ +export const POINT_SHAPES = [ + 'circle', + 'square', + 'diamond', + 'triangle-up', + 'triangle-down', + 'plus', +] as const; + +/** + * Labels of an Arrow `bool` annotation column, false then true. Lower case, + * as the browser has always shown one (`String(true)`), so legend colours + * saved against a v2 bundle keep matching. Both v3 writers store a bool + * column under these labels, and the web's writer keeps `sourceType: bool` + * only while every label is one of them. Not Python's CANONICAL_BOOLEANS + * (`False`/`True`), which are string cells the pipeline writes itself. + */ +export const ARROW_BOOLEAN_LABELS = ['false', 'true'] as const; diff --git a/packages/utils/src/parquet/bundle-writer.ts b/packages/utils/src/parquet/bundle-writer.ts index b3829d6ab..d05767bcf 100644 --- a/packages/utils/src/parquet/bundle-writer.ts +++ b/packages/utils/src/parquet/bundle-writer.ts @@ -54,6 +54,7 @@ import { rankByFrequency, remapCsr } from '../visualization/csr.js'; import { getProteinEvidence, getProteinScores } from '../visualization/plot-data-accessors.js'; import { getEatCompanionColumn, getPredictedCellValues } from '../visualization/eat-overlay.js'; import { isNAValue } from '../visualization/missing-values.js'; +import { ARROW_BOOLEAN_LABELS } from '../generated/shared-constants'; /** * The names Python's encoder takes the id column from, in its order of preference @@ -66,6 +67,9 @@ const FALLBACK_ID_COLUMN = 'protein_id'; const ENCODER = new TextEncoder(); +/** The labels a `bool` column is stored under, shared with Python's decoder. */ +const BOOLEAN_LABELS: ReadonlySet = new Set(ARROW_BOOLEAN_LABELS); + /** * One manifest column. `sourceType` is Python-private and the browser ignores it: it is the * Arrow type `decode_v3` restores the column to (a numeric type, or `bool` for the `true` / @@ -467,9 +471,7 @@ function fitsSourceType( if (sourceType === 'bool') { return ( entry.kind === 'categorical' && - labels.every( - (label) => label == null || isNAValue(label) || label === 'true' || label === 'false', - ) + labels.every((label) => label == null || isNAValue(label) || BOOLEAN_LABELS.has(label)) ); } return true; diff --git a/packages/utils/src/parquet/settings-validation.ts b/packages/utils/src/parquet/settings-validation.ts index 2e0deea1c..7580f93ff 100644 --- a/packages/utils/src/parquet/settings-validation.ts +++ b/packages/utils/src/parquet/settings-validation.ts @@ -1,3 +1,4 @@ +import { DEFAULT_CATEGORICAL_PALETTE_ID } from '../generated/shared-constants'; import type { BundleSettings, ExportOptionsMap, @@ -270,7 +271,10 @@ function sanitizeLegendSettingsEntry(obj: unknown): LegendPersistedSettings | nu hiddenValues: s.hiddenValues, categories, enableDuplicateStackUI: s.enableDuplicateStackUI, - selectedPaletteId: typeof s.selectedPaletteId === 'string' ? s.selectedPaletteId : 'kellys', + selectedPaletteId: + typeof s.selectedPaletteId === 'string' + ? s.selectedPaletteId + : DEFAULT_CATEGORICAL_PALETTE_ID, numericSettings, }; } diff --git a/packages/utils/src/types.ts b/packages/utils/src/types.ts index 07e77bc86..5a758d3a3 100644 --- a/packages/utils/src/types.ts +++ b/packages/utils/src/types.ts @@ -1,3 +1,5 @@ +import type { POINT_SHAPES } from './generated/shared-constants'; + export type AnnotationKind = 'categorical' | 'numeric'; export type NumericAnnotationType = 'int' | 'float'; @@ -306,7 +308,8 @@ export function isValidDensityLayerMode(value: unknown): value is DensityLayerMo return typeof value === 'string' && DENSITY_LAYER_MODES.includes(value as DensityLayerMode); } -export type PointShape = 'circle' | 'square' | 'diamond' | 'triangle-up' | 'triangle-down' | 'plus'; +/** A point's shape; contracts/shared-constants.json lists them for both languages. */ +export type PointShape = (typeof POINT_SHAPES)[number]; // ───────────────────────────────────────────────────────────────── // Legend Persistence Types diff --git a/packages/utils/src/visualization/color-scheme.ts b/packages/utils/src/visualization/color-scheme.ts index 1f36df671..07f07640a 100644 --- a/packages/utils/src/visualization/color-scheme.ts +++ b/packages/utils/src/visualization/color-scheme.ts @@ -1,42 +1,13 @@ +import type { CATEGORICAL_PALETTE_IDS, GRADIENT_PALETTE_IDS } from '../generated/shared-constants'; +import { DEFAULT_CATEGORICAL_PALETTE_ID, KELLYS_COLORS } from '../generated/shared-constants'; import { clamp01 } from './numeric-binning.js'; -/** - * Kelly's 21 Colors of Maximum Contrast - * - * A scientifically-designed palette optimized for maximum visual distinction - * between categories. These colors are chosen to maximize perceptual distance - * and ensure clear differentiation even with many categories. - * - * Medium gray (#848482) is excluded as it's reserved for special categories - * (Others, N/A). Very light gray and very dark gray are included at the end - * for use with less frequent categories. - * - * Reference: Kelly, Kenneth L. "Twenty-two colors of maximum contrast." - * Color Engineering 3.26-27 (1965). - */ -export const KELLYS_COLORS = [ - '#F3C300', // Vivid Yellow - '#875692', // Strong Purple - '#F38400', // Vivid Orange - '#A1CAF1', // Very Light Blue - '#BE0032', // Vivid Red - '#C2B280', // Grayish Yellow - '#008856', // Vivid Green - '#E68FAC', // Strong Purplish Pink - '#0067A5', // Strong Blue - '#F99379', // Strong Yellowish Pink - '#604E97', // Strong Violet - '#F6A600', // Vivid Orange Yellow - '#B3446C', // Strong Purplish Red - '#DCD300', // Vivid Greenish Yellow - '#882D17', // Strong Reddish Brown - '#8DB600', // Vivid Yellowish Green - '#654522', // Deep Yellowish Brown - '#E25822', // Vivid Reddish Orange - '#2B3D26', // Dark Olive Green - '#F2F3F4', // Very Light Gray - '#222222', // Very Dark Gray -] as const; +/** Every palette id; contracts/shared-constants.json lists them for both languages. */ +type PaletteId = (typeof CATEGORICAL_PALETTE_IDS)[number] | (typeof GRADIENT_PALETTE_IDS)[number]; + +// Kelly's colours, in slot order, and the default categorical palette are +// shared with the Python package. +export { DEFAULT_CATEGORICAL_PALETTE_ID, KELLYS_COLORS }; // Color-blind safe export const OKABE_ITO_COLORS = [ @@ -177,7 +148,7 @@ export const COLOR_SCHEMES = { inferno: INFERNO_COLORS, batlow: BATLOW_COLORS, plasma: PLASMA_COLORS, -} as const; +} as const satisfies Record; export type ColorSchemeId = keyof typeof COLOR_SCHEMES; diff --git a/packages/utils/src/visualization/missing-values.ts b/packages/utils/src/visualization/missing-values.ts index 8cd41efcf..1d44d4e5d 100644 --- a/packages/utils/src/visualization/missing-values.ts +++ b/packages/utils/src/visualization/missing-values.ts @@ -5,36 +5,19 @@ * the default display label, the default swatch color, and the normalization * function that runs at the data ingestion boundary — lives here. * - * Downstream code MUST NOT define its own missing-value detection. Add a - * spelling to MISSING_VALUE_TOKENS here and every consumer picks it up. + * Downstream code MUST NOT define its own missing-value detection. The values the + * Python package shares (the missing spellings, the internal token, the swatch + * color) are generated from contracts/shared-constants.json, which documents each: + * add a spelling to MISSING_VALUE_TOKENS there and every consumer, in both + * languages, picks it up. */ +import { MISSING_VALUE_TOKENS, NA_DEFAULT_COLOR, NA_VALUE } from '../generated/shared-constants'; -/** - * Strings that mean "this value is missing". Compared case-insensitively. - * - * `__na__` is here to reserve the spelling of NA_VALUE below: the bundle writer - * drops that token unconditionally (it is the sentinel a missing cell carries), - * so a genuine category literally spelled `__NA__` would be silently deleted on - * export. Normalizing it to missing at ingestion makes the two agree. - */ -export const MISSING_VALUE_TOKENS: ReadonlySet = new Set([ - 'na', - 'n/a', - 'nan', - 'null', - 'none', - '__na__', -]); - -/** Internal identity token for NA after ingestion. Never displayed to users. */ -export const NA_VALUE = '__NA__'; +export { MISSING_VALUE_TOKENS, NA_DEFAULT_COLOR, NA_VALUE }; /** Default display label for NA in legends and tooltips. */ export const NA_DISPLAY = 'N/A'; -/** Default swatch color for NA. Override-able via persisted category color (categorical only). */ -export const NA_DEFAULT_COLOR = '#DDDDDD'; - /** * Normalize a raw cell value to JS `null` if it represents missingness. * Otherwise return the value unchanged. diff --git a/packages/utils/src/visualization/numeric-binning.ts b/packages/utils/src/visualization/numeric-binning.ts index ff3ceec96..7e39a8dab 100644 --- a/packages/utils/src/visualization/numeric-binning.ts +++ b/packages/utils/src/visualization/numeric-binning.ts @@ -1,3 +1,4 @@ +import { DEFAULT_NUMERIC_PALETTE_ID, GRADIENT_PALETTE_IDS } from '../generated/shared-constants'; import { sampleColorSchemeColor } from './color-scheme'; import { NA_VALUE, NA_DEFAULT_COLOR } from './missing-values'; import type { @@ -11,15 +12,9 @@ import type { } from '../types'; import { djb2Hash } from '../storage/data-hash'; -export const DEFAULT_NUMERIC_PALETTE_ID = 'batlow'; +export { DEFAULT_NUMERIC_PALETTE_ID }; export const DEFAULT_NUMERIC_STRATEGY: NumericBinningStrategy = 'quantile'; -export const GRADIENT_COLOR_SCHEME_IDS = new Set([ - 'viridis', - 'cividis', - 'inferno', - 'batlow', - 'plasma', -]); +export const GRADIENT_COLOR_SCHEME_IDS: ReadonlySet = new Set(GRADIENT_PALETTE_IDS); const NUMERIC_COLOR_ALGORITHM_VERSION = 'numeric-gradient-v2'; export interface NumericAnnotationDisplaySettings { diff --git a/scripts/shared-constants/generate-shared-constants.mts b/scripts/shared-constants/generate-shared-constants.mts new file mode 100644 index 000000000..32607e452 --- /dev/null +++ b/scripts/shared-constants/generate-shared-constants.mts @@ -0,0 +1,240 @@ +/** + * Generate the two shared-constants modules from `contracts/shared-constants.json`: + * - `packages/utils/src/generated/shared-constants.ts`, for the web packages; and + * - `apps/protspace/src/protspace/_generated/shared_constants.py`, for the Python package. + * + * Both are committed. The PyPI wheel is built from `apps/protspace` alone, so the Python package + * cannot read the JSON at build or run time. The source is validated first (structure, then the + * invariants the code on either side relies on), and nothing is written when it fails. + * + * Usage: + * tsx scripts/shared-constants/generate-shared-constants.mts # write both modules + * tsx scripts/shared-constants/generate-shared-constants.mts --check # fail if either is stale + */ +import { mkdirSync, readFileSync, writeFileSync } from 'node:fs'; +import { dirname, relative, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { format, resolveConfig } from 'prettier'; + +const ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '../..'); +const SOURCE = resolve(ROOT, 'contracts/shared-constants.json'); +const TS_OUT = resolve(ROOT, 'packages/utils/src/generated/shared-constants.ts'); +const PY_OUT = resolve(ROOT, 'apps/protspace/src/protspace/_generated/shared_constants.py'); + +const KINDS = ['string', 'list', 'set'] as const; +type Kind = (typeof KINDS)[number]; +interface Constant { + kind: Kind; + doc: string[]; + value: string | string[]; +} +type Constants = Map; + +const NAME_RE = /^[A-Z][A-Z0-9]*(?:_[A-Z0-9]+)*$/; +// Both languages' comments carry the doc verbatim; 80 leaves room for the comment prefix +// under prettier's 100 and ruff's 88 columns. +const MAX_DOC_LINE = 80; +const ENTRY_KEYS = new Set(['kind', 'doc', 'value']); + +const rel = (path: string) => relative(ROOT, path); + +function isStringArray(value: unknown): value is string[] { + return Array.isArray(value) && value.every((item) => typeof item === 'string'); +} + +/** The source's constants, in file order, or every structural problem found. */ +function parse(raw: unknown, errors: string[]): Constants { + const constants: Constants = new Map(); + const entries = (raw as { constants?: unknown } | null)?.constants; + if (typeof entries !== 'object' || entries === null || Array.isArray(entries)) { + errors.push('top level: `constants` must be an object'); + return constants; + } + for (const [name, entry] of Object.entries(entries)) { + const where = `constants.${name}`; + if (!NAME_RE.test(name)) errors.push(`${where}: names are UPPER_SNAKE_CASE`); + if (typeof entry !== 'object' || entry === null || Array.isArray(entry)) { + errors.push(`${where}: must be an object with kind, doc and value`); + continue; + } + const { kind, doc, value } = entry as Record; + for (const key of Object.keys(entry)) { + if (!ENTRY_KEYS.has(key)) errors.push(`${where}: unknown key "${key}"`); + } + if (!KINDS.includes(kind as Kind)) { + errors.push(`${where}.kind: must be one of ${KINDS.join(', ')}`); + continue; + } + if (!isStringArray(doc) || doc.length === 0) { + errors.push(`${where}.doc: must be a non-empty list of lines`); + continue; + } + doc.forEach((line, i) => { + if (line.length > MAX_DOC_LINE) { + errors.push(`${where}.doc[${i}]: ${line.length} characters, the limit is ${MAX_DOC_LINE}`); + } + if (line !== line.trimEnd()) errors.push(`${where}.doc[${i}]: trailing whitespace`); + if (line.includes('*/')) errors.push(`${where}.doc[${i}]: "*/" would end the TS comment`); + }); + if (kind === 'string') { + if (typeof value !== 'string') + errors.push(`${where}.value: a string constant needs a string`); + } else if (!isStringArray(value) || value.length === 0) { + errors.push(`${where}.value: a ${kind} constant needs a non-empty list of strings`); + continue; + } else { + const duplicates = value.filter((item, i) => value.indexOf(item) !== i); + if (duplicates.length > 0) errors.push(`${where}.value: duplicates ${duplicates.join(', ')}`); + } + constants.set(name, { kind: kind as Kind, doc, value: value as string | string[] }); + } + return constants; +} + +/** The relationships between values that the code on either side relies on. */ +function checkInvariants(constants: Constants, errors: string[]): void { + const get = (name: string, kind: K) => { + const constant = constants.get(name); + if (constant?.kind === kind) return constant.value as K extends 'string' ? string : string[]; + errors.push(`constants.${name}: required, as a ${kind}`); + return undefined; + }; + + const tokens = get('MISSING_VALUE_TOKENS', 'set'); + for (const token of tokens ?? []) { + if (token === '' || token !== token.trim().toLowerCase()) { + errors.push( + `MISSING_VALUE_TOKENS: "${token}" is not trimmed and lower-case; both readers compare ` + + 'the trimmed, lower-cased cell, so it could never match', + ); + } + } + const naValue = get('NA_VALUE', 'string'); + if (tokens && naValue !== undefined && !tokens.includes(naValue.toLowerCase())) { + errors.push( + `MISSING_VALUE_TOKENS: must hold "${naValue.toLowerCase()}" (NA_VALUE lower-cased), or a ` + + 'category spelled like the sentinel is dropped on export', + ); + } + + const categorical = get('CATEGORICAL_PALETTE_IDS', 'list'); + const gradient = get('GRADIENT_PALETTE_IDS', 'list'); + for (const id of categorical?.filter((item) => gradient?.includes(item)) ?? []) { + errors.push(`palettes: "${id}" is both categorical and gradient`); + } + const categoricalDefault = get('DEFAULT_CATEGORICAL_PALETTE_ID', 'string'); + if ( + categorical && + categoricalDefault !== undefined && + !categorical.includes(categoricalDefault) + ) { + errors.push( + `DEFAULT_CATEGORICAL_PALETTE_ID: "${categoricalDefault}" is not a categorical palette`, + ); + } + const numericDefault = get('DEFAULT_NUMERIC_PALETTE_ID', 'string'); + if (gradient && numericDefault !== undefined && !gradient.includes(numericDefault)) { + errors.push(`DEFAULT_NUMERIC_PALETTE_ID: "${numericDefault}" is not a gradient palette`); + } + + const booleanLabels = get('ARROW_BOOLEAN_LABELS', 'list'); + const shown = [String(false), String(true)]; + if (booleanLabels && booleanLabels.join() !== shown.join()) { + errors.push( + `ARROW_BOOLEAN_LABELS: must be ${JSON.stringify(shown)}, how the web shows a v2 bool cell`, + ); + } +} + +const GENERATED_BY = [ + 'Generated by scripts/shared-constants/generate-shared-constants.mts from', + 'contracts/shared-constants.json. Do not edit: change the JSON, run', + '`pnpm shared-constants` and commit both generated modules.', + '`pnpm shared-constants:check` fails while they disagree.', +]; + +async function renderTypeScript(constants: Constants): Promise { + const blocks = [...constants].map(([name, { kind, doc, value }]) => { + const comment = ['/**', ...doc.map((line) => (line ? ` * ${line}` : ' *')), ' */']; + const items = Array.isArray(value) ? value.map((item) => JSON.stringify(item)).join(', ') : ''; + const declaration = { + string: `export const ${name} = ${JSON.stringify(value)};`, + list: `export const ${name} = [${items}] as const;`, + set: `export const ${name}: ReadonlySet = new Set([${items}]);`, + }[kind]; + return [...comment, declaration].join('\n'); + }); + const header = GENERATED_BY.map((line) => `// ${line}`).join('\n'); + const source = `${header}\n\n${blocks.join('\n\n')}\n`; + return format(source, { ...(await resolveConfig(TS_OUT)), filepath: TS_OUT }); +} + +/** + * Python in the one layout `ruff format` leaves alone: double quotes (JSON string literals are + * valid Python), and every collection exploded with a trailing comma, which ruff keeps exploded. + */ +function renderPython(constants: Constants): string { + const blocks = [...constants].map(([name, { kind, doc, value }]) => { + const comment = doc.map((line) => (line ? `# ${line}` : '#')); + const items = Array.isArray(value) ? value.map((item) => JSON.stringify(item)) : []; + const declaration = { + string: [`${name} = ${JSON.stringify(value)}`], + list: [`${name} = (`, ...items.map((item) => ` ${item},`), ')'], + set: [ + `${name} = frozenset(`, + ' {', + ...items.map((item) => ` ${item},`), + ' }', + ')', + ], + }[kind]; + return [...comment, ...declaration].join('\n'); + }); + const docstring = [ + '"""Constants the protspace package shares with the ProtSpace web app.', + '', + ...GENERATED_BY.map((line) => line.replace(/`/g, '``')), + '"""', + ].join('\n'); + return `${docstring}\n\n${blocks.join('\n\n')}\n`; +} + +const errors: string[] = []; +const constants = parse(JSON.parse(readFileSync(SOURCE, 'utf8')), errors); +if (errors.length === 0) checkInvariants(constants, errors); +if (errors.length > 0) { + console.error( + `✖ ${rel(SOURCE)} is invalid; nothing was written:\n` + + errors.map((error) => ` - ${error}`).join('\n'), + ); + process.exit(1); +} + +const outputs: Array<[string, string]> = [ + [TS_OUT, await renderTypeScript(constants)], + [PY_OUT, renderPython(constants)], +]; + +if (process.argv.includes('--check')) { + const stale = outputs.filter(([path, content]) => { + try { + return readFileSync(path, 'utf8') !== content; + } catch { + return true; + } + }); + if (stale.length > 0) { + console.error( + stale.map(([path]) => `✖ ${rel(path)} is out of date with ${rel(SOURCE)}.`).join('\n') + + '\n Run `pnpm shared-constants` and commit the result.', + ); + process.exit(1); + } + console.log(`✓ The shared-constants modules match ${rel(SOURCE)}.`); +} else { + for (const [path, content] of outputs) { + mkdirSync(dirname(path), { recursive: true }); + writeFileSync(path, content, 'utf8'); + console.log(`✓ Wrote ${rel(path)}`); + } +} diff --git a/tests/contract/bundle.contract.test.ts b/tests/contract/bundle.contract.test.ts index 7d71cf16e..65d5c49aa 100644 --- a/tests/contract/bundle.contract.test.ts +++ b/tests/contract/bundle.contract.test.ts @@ -63,6 +63,9 @@ interface Manifest { }; projectionCount: number; labelWithReservedChar: string; + negativeTransmembraneCategory: string; + missingTmbedIndex: number; + malformedTmbedIndex: number; nullLengthIndex: number; statisticsColumns: string[]; statisticsCategory: string; @@ -279,6 +282,19 @@ function expectAnnotationContract( expect(data.annotations.domains.values).toContain('DomA'); expect(data.annotations.domains.values).toContain('DomB'); + // A TMbed prediction with no membrane segment is a category, while a missing or + // malformed payload is N/A in both TMbed-derived annotations, not a negative. + expect(data.annotations.predicted_transmembrane.values).toContain( + manifest.negativeTransmembraneCategory, + ); + for (const row of [manifest.missingTmbedIndex, manifest.malformedTmbedIndex]) { + for (const annotation of ['predicted_transmembrane', 'predicted_signal_peptide']) { + expect(getProteinAnnotationValues(data, row, annotation), `${annotation}[${row}]`).toEqual([ + NA_VALUE, + ]); + } + } + const lengths = data.numeric_annotation_data?.length; // Assert the length first: an out-of-range index yields `undefined`, which // would satisfy the null check below even if the reader dropped a protein. @@ -555,6 +571,9 @@ describe('bundles the web app exports, read by the Python tooling', () => { // prediction, `none`, came to be displayed as N/A (#339). describe('what counts as a missing annotation value', () => { it('names the same missing-value spellings as the producer', () => { + // Both sets are generated from contracts/shared-constants.json, whose own check covers + // the generated modules. This compares the sets each side actually uses, so a re-export + // replaced by a local literal still fails here. expect([...MISSING_VALUE_TOKENS].sort()).toEqual(manifest.missingValueTokens); }); diff --git a/tests/contract/emit_bundles.py b/tests/contract/emit_bundles.py index 2ad0058e3..2519f746a 100644 --- a/tests/contract/emit_bundles.py +++ b/tests/contract/emit_bundles.py @@ -43,9 +43,13 @@ import pyarrow as pa import pyarrow.parquet as pq +from biocentral_api import Prediction from protspace.core.constants import MISSING_VALUE_TOKENS, is_missing_value from protspace.data.annotations.encoding import encode_field, stamp_format_version +from protspace.data.annotations.retrievers.biocentral_retriever import ( + BiocentralPredictionRetriever, +) from protspace.data.io.bundle import PARQUET_BUNDLE_DELIMITER, create_settings_parquet from protspace.stats.base import STATS_SCHEMA @@ -156,6 +160,38 @@ def length_value(index: int) -> float | None: return None if index == NULL_LENGTH_INDEX else float(100 + index * 10) +def tmbed_predictions(value: object) -> list[Prediction]: + """Build the real generated prediction shape consumed by the adapter.""" + return [ + Prediction( + model_name="TMbed", + prediction_name="topology", + protocol="per_residue", + value=value, + ) + ] + + +# The first proteins' TMbed payloads, each run through the real annotation adapter +# rather than copying its vocabulary into the contract test. A topology of only +# inside/outside labels is a completed prediction with no membrane-spanning +# segment, which must surface as a category. The missing and malformed payloads +# are unavailable predictions, kept as explicit rows apart from the fixture's +# other empty ones so an adapter that invents a negative biological result fails +# before the bundle reader has a chance to normalize the missing representation. +TMBED_PAYLOADS = ["oooooiiiii", None, "garbage"] +NEGATIVE_TMBED_INDEX, MISSING_TMBED_INDEX, MALFORMED_TMBED_INDEX = range(3) +NEGATIVE_TMBED_CATEGORY = BiocentralPredictionRetriever._extract_transmembrane( + tmbed_predictions(TMBED_PAYLOADS[NEGATIVE_TMBED_INDEX]) +) + + +def tmbed_column(extract, count: int) -> list[str]: + """A TMbed-derived annotation column: the payload rows, then empty cells.""" + cells = [extract(tmbed_predictions(value)) for value in TMBED_PAYLOADS] + return (cells + [""] * count)[:count] + + def build_annotations_table(ids: list[str]) -> pa.Table: """Mimic ``protspace annotate`` output: an ``identifier`` column plus annotations. @@ -166,15 +202,22 @@ def build_annotations_table(ids: list[str]) -> pa.Table: Every cell follows one per-row rule at every size (``family_label``, ``domain_hits``, ``length_value``): protein 1 carries the percent-encoded - label and the multi-hit cell, protein 4 the null length, and every other row - its own labels, hit count and scores. The large variant therefore asserts - the same encoding contract as the small one, and can be checked row by row. + label and the multi-hit cell, proteins 1-3 the TMbed payloads, protein 4 the + null length, and every other row its own labels, hit count and scores. The + large variant therefore asserts the same encoding contract as the small one, + and can be checked row by row. """ family = [encode_field(family_label(i)) for i in range(len(ids))] domains = [ ";".join(f"{encode_field(label)}|{score}" for label, score in domain_hits(i)) for i in range(len(ids)) ] + predicted_signal_peptide = tmbed_column( + BiocentralPredictionRetriever._extract_signal_peptide, len(ids) + ) + predicted_transmembrane = tmbed_column( + BiocentralPredictionRetriever._extract_transmembrane, len(ids) + ) # A genuine double column with a null -- distinguishes "missing" from 0 and # from NaN across the language boundary. Real bundles carry both string-typed @@ -187,6 +230,12 @@ def build_annotations_table(ids: list[str]) -> pa.Table: "identifier": pa.array(ids, pa.string()), "family": pa.array(family, pa.string()), "domains": pa.array(domains, pa.string()), + "predicted_signal_peptide": pa.array( + predicted_signal_peptide, pa.string() + ), + "predicted_transmembrane": pa.array( + predicted_transmembrane, pa.string() + ), "length": pa.array(length, pa.float64()), } ) @@ -366,8 +415,11 @@ def write_legacy_v2_bundle(path: Path, ids: list[str]) -> None: ``size`` as numeric, since it inferred over the placed proteins, and the converted bundle has to as well, with the same dataset hash. """ - annotations = build_annotations_table(ids).rename_columns( - ["protein_id", "family", "domains", "length"] + # The legacy file's own columns; the TMbed ones are checked on the v3 variants. + annotations = ( + build_annotations_table(ids) + .select(["identifier", "family", "domains", "length"]) + .rename_columns(["protein_id", "family", "domains", "length"]) ) annotations = annotations.append_column( "size", pa.array([str(10 * i) if i % 3 else "NA" for i in range(len(ids))]) @@ -553,6 +605,9 @@ def convert_legacy() -> None: }, "projectionCount": len(PROJECTIONS), "labelWithReservedChar": LABEL_WITH_RESERVED_CHAR, + "negativeTransmembraneCategory": NEGATIVE_TMBED_CATEGORY, + "missingTmbedIndex": MISSING_TMBED_INDEX, + "malformedTmbedIndex": MALFORMED_TMBED_INDEX, "nullLengthIndex": NULL_LENGTH_INDEX, "statisticsColumns": STATS_SCHEMA.names, "statisticsCategory": STATISTICS_CATEGORY,