Skip to content

fix: batch release of #406, #545 and #546 (TMbed payloads, N/A styles, shared constants) - #547

Merged
tsenoner merged 36 commits into
mainfrom
fix/missing-values-batch
Oct 15, 2026
Merged

tsenoner merged 36 commits into
mainfrom
fix/missing-values-batch

Conversation

@tsenoner

@tsenoner tsenoner commented Oct 15, 2026 •

Copy link
Copy Markdown
Owner

Why one PR

#543 shipped on its own as 4.16.6. This batch carries the three follow-ups so they ship as one patch release (4.16.7) instead of three: every push to main touching apps/protspace/** cuts a release.

Merge with a merge commit, never squash. Squashing collapses every commit into one that touches every path, which breaks semantic-release's per-commit monorepo scoping.

What's in it

PR What it does
#406 An empty or malformed Biocentral TMbed payload is missing for both derived columns. main still wrote signal peptide False for an empty one. InterPro looks up UniProt's canonical sequence, while Biocentral predicts the FASTA's. Annotation cache semantics v4 refreshes TMbed columns cached before this change. Contract fixture rows cover it across the Python → TypeScript boundary.
#545 protspace style stores every N/A spelling under the web's one __NA__ key: colours, shapes, pinned and hidden values.
#546 The constants Python and the web share (missing-value tokens, __NA__, its swatch, Kelly's colours, palette ids, shapes) are generated from contracts/shared-constants.json. CI checks the generated modules against it.

Each was merged into this branch with a merge commit. The one conflict was in settings_converter.py: #546's generated imports were kept alongside #545's is_missing_value, and the _NA_LABELS set #545 replaced was dropped.

Added on this branch

  • fix(style): --generate-template pre-fills N/A with the web's default grey (#DDDDDD) instead of its own #C0C0C0, so a filled-in template matches the default legend. NA_COLOR had no other use and is removed.
  • fix(annotations): a cache refresh refetches its sources whole, so a stale column of a source it refetches anyway is now refreshed instead of dropped. This includes UniProt when it is fetched only to supply sequences. The sources are computed once, before the discard. Before, asking for one column could drop its stale siblings, and the next run paid that source again.
  • fix(style): stored hiddenValues fold their N/A spellings too, not only overridden ones.
  • refactor: small cleanups from /simplify: one TMbed payload test instead of two, and a corrected Kelly's palette note in the contract.
  • docs(openspec): fix(annotations): preserve negative transmembrane predictions #406's change is archived, as AGENTS.md requires before merge. Its proposal now says what it adds on top of main, which shipped the label in fix(protspace): stop annotation retrieval from silently losing data #495.
  • docs(openspec): fix-cache-ownership (fix(protspace): own every retained cache by what produced it #404) and parquetbundle-v3 (feat(bundle): parquetbundle v3, 2x faster 1/3 memory usage #477) are archived too. Both had shipped but were never archived. parquetbundle-v3's 5.0.0 follow-ups now point to Remove the features deprecated in 4.x when we release protspace 5.0 #544, which keeps protspace convert reading v1/v2 in 5.0. The six new capability specs have their Purpose filled in.
  • fix(annotations): cache semantics v4 also covers every InterPro column. InterPro now looks up UniProt's canonical sequence first, so a v3 cache refetches InterPro once instead of keeping a FASTA-based match.
  • refactor: the default categorical palette (kellys) is now DEFAULT_CATEGORICAL_PALETTE_ID in contracts/shared-constants.json. The generator checks that it names a categorical palette, and every default-palette literal in Python and the web reads it. Recorded in the archived interpro-cache-and-default-palette OpenSpec change.
  • fix(style): --generate-template no longer writes selectedPaletteId for a numeric column. A numeric column reads that key as its gradient, so the categorical default reset any gradient chosen in the web app to batlow when a filled-in template was applied.

Verification

All local:

  • pnpm precommit passes, including shared-constants:check.
  • pnpm format:check passes.
  • pytest -m "not slow" (apps/protspace): 1670 passed.
  • apps/prep test suite passes.
  • Bundle contract suite: 20 passed.
  • pnpm test:ci: utils 431, app 471, core 2107 passed.
  • openspec validate --all --strict: 45 passed.
  • The live Biocentral TMbed canary passed earlier against the real server with the new payload validation.
  • Each PR's own CI was green before batching.

Known gap, left as is

Closes #339

🤖 Generated with Claude Code

FlorinSenoner and others added 30 commits August 1, 2026 19:04
- Replace the re.search([Hh])/([Bb]) topology probes in
  BiocentralPredictionRetriever._extract_transmembrane with plain
  substring membership tests, and drop the now-unused `import re`.
- Re-pad the Biocentral annotation table in apps/protspace/docs/annotations.md
  so every Description cell is 47 characters wide again; apps/protspace/ is
  prettier-ignored, so the column alignment has to be maintained by hand.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016qoU16kDQxz6U3H2UWbbm2
- Add parse_fasta_by_identifier and use it in annotate, the pipeline and
  embed_fasta instead of three copies of the header remap.
- Build the annotate command's AnnotationConfiguration once and gate the
  FASTA parse on a new AnnotationConfiguration.needs_sequence property.
- Replace the _build_sequence_map prefer_uniprot flag with a
  _uniprot_sequences helper; each fetcher states precedence by merge order.
- Flatten _extract_tmbed_topology.
- Tests: share one annotate CLI runner, build real Prediction models
  everywhere, fold the none/empty payload cases into the parametrized test.
- Contract fixture: derive both TMbed columns from one payload list so row
  indices cannot drift, assert via the shared NA_VALUE, and rename the
  manifest key to missingTmbedIndex.

No behavior change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ansmembrane

Main shipped most of this fix through PR 495: TMbed negatives are already
labelled `non-transmembrane`, an empty payload already stays missing for the
transmembrane annotation, and `protspace annotate` already passes FASTA
sequences (always, since it also derives missing lengths from them) via
`parse_fasta_normalized`.

Resolution keeps what this branch still adds on top of main:
- `_extract_tmbed_topology` validation, so an empty or malformed TMbed payload
  is missing for both derived annotations (main still turned an empty payload
  into signal peptide "False"); restores the `re` import main still needs.
- InterPro prefers UniProt's sequence, Biocentral the local FASTA one, alongside
  main's `_starved_by_uniprot` check.
- The source-precedence and malformed-payload tests, and the contract fixture's
  TMbed rows, now asserted inside main's shared `expectAnnotationContract`.

Takes main's annotate command, FASTA helpers, configuration and annotate CLI
tests, dropping this branch's needs_sequence gate (it would skip main's length
fallback) and its duplicate parse_fasta_by_identifier helper. The legacy v2
contract variant selects its original four columns before the rename.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
protspace style keeps its own exact-match list of N/A spellings, so a
style naming N/A by any other spelling never reaches the web's __NA__
category, and a restyle of N/A loses to the stored colour.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The web draws every missing cell as one legend entry and looks its style
up under __NA__. protspace style decided what is N/A with its own
exact-match list (`""`, `<NA>`, `NaN`, `__NA__`, `None`), kept twice, so:

- a column whose missing cells say `N/A`, `null` or `none` was styled
  under that spelling, which the web never reads, and an `__NA__` key
  against such data was refused outright; `hiddenValues` and
  `pinnedValues` spelled that way were lost the same way;
- restyling N/A on a bundle that already had an `__NA__` colour kept the
  old colour: the new one went under the data's spelling and `__NA__`
  overwrote it in the settings.

One rule now, `is_na_label` (the web's `is_missing_value`, plus `<NA>`,
pandas' printout that older styles files carry). Styles for N/A are
written under `__NA__`, so a restyle replaces the stored one, and the
converter folds whatever spellings remain (colours, shapes, stored
categories, frequencies, pinned, hidden) into one `__NA__` entry, with
`__NA__` itself winning. `--generate-template` lists N/A once, last, as
`__NA__`. `none` resolves as N/A like every other spelling: it was kept
apart for TMbed's old negative label, which the web drew as N/A anyway.
The converter's default-grey loop is removed; it looked for keys that
had already been renamed to `__NA__`, so it never ran.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The branch changed what cached TMbed values mean (an empty payload is no
longer signal peptide "False"; a malformed one is no longer read as a
topology) without recording it, so a v3 annotation cache kept serving them.

- Add cache semantics v4 for predicted_signal_peptide and
  predicted_transmembrane.
- A refresh refetches its sources whole, so a stale column of a source the
  refresh refetches anyway is now refreshed with it instead of dropped;
  otherwise asking for one TMbed column lost the other from the cache.
- Import Prediction from biocentral_api, not its private _generated module.
- Keep the contract fixture's TMbed columns no longer than the protein list.
- Run the source-precedence test on the normal path, not the failure one.
- Toxprot demo docstring: FASTA precedence now holds for Biocentral only.
- OpenSpec: drop the scenario the merged annotate contradicts (it parses
  the FASTA for every request), and add the cache-semantics delta.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…into fix/style-na-aliases

# Conflicts:
#	apps/protspace/CLAUDE.md
#	apps/protspace/tests/test_missing_values.py
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
contracts/shared-constants.json holds every value the Python package and
the web app must agree on (missing-value tokens, the __NA__ key and its
swatch, Kelly's colours, palette ids, point shapes, boolean labels), each
with its rationale. `pnpm shared-constants` validates it and writes a
committed module per language; the Python one sits inside the package so
the wheel needs nothing outside apps/protspace. `--check` runs in
`pnpm precommit`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… module

MISSING_VALUE_TOKENS, NA_INTERNAL, NA_PINNED_COLOR, KELLYS_COLORS, the
palette id sets, ALLOWED_SHAPES and ARROW_BOOLEAN_LABELS keep their names
and values but now come from protspace._generated.shared_constants.
test_style_warnings.py no longer regex-parses color-scheme.ts and
numeric-binning.ts; test_shared_constants.py pins every re-export to the
generated value instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…module

missing-values.ts, color-scheme.ts and numeric-binning.ts re-export
MISSING_VALUE_TOKENS, NA_VALUE, NA_DEFAULT_COLOR, KELLYS_COLORS and
DEFAULT_NUMERIC_PALETTE_ID from it, GRADIENT_COLOR_SCHEME_IDS and
PointShape derive from it, and the v3 writer checks a bool column's
labels against ARROW_BOOLEAN_LABELS. COLOR_SCHEMES now `satisfies` a
record over the generated palette ids, so a palette missing on either
side fails the type check.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
bundle-contract.yml, the one unfiltered workflow, runs
`pnpm shared-constants:check`, so a hand edit to only one generated module
cannot skip the check through ci.yml's or protspace-ci.yml's path filter.
protspace-ci.yml stops watching color-scheme.ts and numeric-binning.ts:
no Python test reads them any more.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The shared-constants session fixed the same style bug independently on
PR #546; that copy is being dropped in favour of this PR. Its tests
covered cases this one did not, so they move here:

- any missing spelling sorts last, and is keyed __NA__ (N/A, null, NA,
  padded none);
- several spellings give one category with contiguous zOrder, the first
  sorted spelling winning;
- a stored __NA__ category keeps its fields when restyled;
- N/A pinned by two spellings is pinned once, with the pinned grey;
- a __REST__ fill never counts N/A as a value;
- an upper-case N/A key resolves;
- restyling what `protspace style` itself wrote keeps one N/A category.

Not ported: folding the Dash display string `<N/A>` (the web shows a
literal `<N/A>` cell as a category), and a template listing every N/A
spelling (this PR lists N/A once as __NA__).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fix(annotations): preserve negative transmembrane predictions

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fix(style): store every N/A style under the web's __NA__ key

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
chore: generate the constants Python and the web share from one file

Conflict in settings_converter.py: keeps #546's generated imports
(KELLYS_COLORS, NA_DEFAULT_COLOR, NA_VALUE) and #545's is_missing_value,
dropping the _NA_LABELS set #545 replaced.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`--generate-template` pre-filled N/A with its own #C0C0C0, while the web
draws an unstyled N/A in NA_DEFAULT_COLOR (#DDDDDD), and a pinned N/A is
#DDDDDD too, so a filled-in template showed N/A in a different grey than
the default legend. The template's one `__NA__` entry now takes the shared
NA_DEFAULT_COLOR, and NA_COLOR, left with no use, is removed.

Re-applies 75c3f66 from #546's dropped style commits onto #545's template.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The rule that a stale column rides along with its refreshed source only
looked at the sources owning a requested stale column, a narrower copy of
the `widened` set the same function computes later. So when UniProt was
refetched just to supply sequences for an InterPro or Biocentral refresh,
its own stale columns (protein_families, xref_pdb) were still dropped, and
the next run that asked for one paid UniProt again.

`migration_sources` and `widened` are now computed before the discard (the
lookup keys they read are never stale), and the discard spares every
column of a widened source. A failed UniProt refresh never restores a stale
column, requested or refreshed along.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- _fold_na takes the winning N/A key with min() instead of sorting them all.
- One parametrized TMbed test covers empty, missing and malformed payloads
  for both derived annotations, instead of two identical bodies.
- Rename test_na_gets_gray: the default grey it was named for is gone.
- The Kelly's palette doc no longer says #848482 is reserved for Other and
  N/A, which the web draws #999999 and #DDDDDD; regenerated both modules.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
visualization_state_to_settings folded N/A spellings in colours, shapes,
stored categories, counts and pinned values, but folded hiddenValues only
when the styles file overrode them. A bundle styled before the fold kept
hiddenValues like ["None"], which the web, hiding N/A by __NA__, never
matches, so restyling only its colours showed the hidden N/A points again.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
tsenoner and others added 6 commits October 15, 2026 14:59
Rereads the proposal and design against the final batch first: main had
already shipped the label and FASTA passing (PR 495), so they now say what
this change adds on top. Fills in the new annotation-input Purpose.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Both shipped (#404, #477) but were never archived, so openspec/specs/
lacked their requirements. Before archiving: ticks parquetbundle-v3's
merge task (#477 landed as merge commit 952646c), turns its 5.0.0
follow-ups into a pointer to issue #544, which keeps `protspace convert`
reading v1/v2 in 5.0, and notes that revision in the design. Fills in the
Purpose of the six capabilities the two changes add.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…l lookup

InterPro now looks 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 no cache semantics change recorded
it: a resumed run kept the FASTA-based match while a fresh run returned
the canonical one. Cache semantics v4 is not released yet, so it now also
covers every InterPro column, and a v3 cache refetches InterPro once.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The default numeric palette was a shared constant, but `kellys`, the
categorical default, stayed a literal in both languages. It is now
DEFAULT_CATEGORICAL_PALETTE_ID in contracts/shared-constants.json, which
the generator checks names a categorical palette, and every default-palette
literal reads it: the style template and settings defaults, the statistics
legend, the legend component, the settings validator and the readers'
default colouring.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Records the InterPro columns in cache semantics v4 and the shared default
categorical palette, the two gaps the batch review left open.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lumn

`--generate-template` wrote selectedPaletteId: "kellys" for every column.
A numeric column reads selectedPaletteId as its gradient, so the web
resolved that categorical id to batlow: applying a filled-in template reset
any gradient chosen in the web app. A numeric column's entry now leaves the
key out, so it keeps its own.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tsenoner
tsenoner merged commit 4bd6432 into main Oct 15, 2026
16 checks passed
@tsenoner
tsenoner deleted the fix/missing-values-batch branch October 15, 2026 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Transmembrane prediction using Biocentral not working

2 participants