[BACKUP, do not merge] Multi-space cartography — groups 1-9, Gates A-E - #2
Open
mrubash1 wants to merge 67 commits into
Open
[BACKUP, do not merge] Multi-space cartography — groups 1-9, Gates A-E#2mrubash1 wants to merge 67 commits into
mrubash1 wants to merge 67 commits into
Conversation
Design documentation only. No code, no behavior change.
Nine ADRs covering the decisions the following commits implement:
0001 Block/Space/View, and why the seam is at the representation
0002 Fusion taxonomy and the normalization contract
0003 The `fusable` flag, and why it must not be helpfully removed
0004 Block storage format and the O(N^2) ceiling
0005 Frontend delivery: one static self-contained HTML file
0006 How optional and licensed dependencies stay out of CI
0007 Every labeled-matrix load asserts index alignment
0008 Cohort selection is a choice, not a given
0009 Censoring semantics: a zero is a missing measurement
Two of these rest on measurements against a production 2530-protein run
rather than on reading the code alone, and both were independently
re-derived before being written up:
- 0007: the column order of `all_by_all_tmscore_pivoted.tsv` is a
permutation of the row order. Only 2 of 2530 columns sit in their row
position, so reading the matrix positionally returns the wrong cell
99.92% of the time. Reading it by label is exact. PR Arcadia-Science#106 fixes the
producer; the assertion is what catches the next one.
- 0009: 60.48% of that matrix is the literal string "0.0", written by
`get_line_for_protid` for any pair absent from the Foldseek output.
Not one cell is a measured zero. The cause is `foldseek search
--max-seqs`, default 1000, and the cap predicts the observed zero
fraction to within 145 cells. Of the censored cells, 925,435 have a
measured mirror, with a median true score of 0.772 -- while the
lowest score Foldseek reports anywhere in the matrix is 0.0549. The
fill is not the low end of the distribution; it is a value Foldseek
never produces.
`docs/REVIEW_LOG.md` records the adversarial gates. Gate A ran against
Phase 0.5 and passed, reproducing every quantitative result from an
independent second derivation. It contributed six refinements, of which
one materially shrinks a later commit: the censoring mask can be
rebuilt from the string form of any existing matrix file, so it does not
need reconstructing from the raw .m8 alignments as originally planned.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
Pure addition. No existing file is touched and nothing calls this yet; the consumers arrive with the blocks in a later commit. `all_by_all_tmscore_pivoted.tsv` has two properties that a bare `pd.read_csv` cannot see, and both of them silently produce wrong answers rather than errors. Index alignment. `pivot_foldseek_results` writes the header from one iteration and the rows from another. Before PR Arcadia-Science#106 the header came from an unsorted `set`, and Python salts string hashing per process, so the column order was a per-run permutation of the row order. Measured on a production 2530-protein matrix: 2 of 2530 columns sit in their row position. Reading it positionally returns the wrong cell 99.92% of the time; reading it by label gives a diagonal that is exactly 1.0 in all 2530 rows. The data is sound -- only the naive read is broken. PR Arcadia-Science#106 fixes the writer; this loader is what catches the next one, and what protects analyses pointed at output written before the fix. `load_labeled_matrix` therefore asserts that the column labels are the row labels in the same order, and refuses the file otherwise. The error names the first divergent position and reports what fraction of columns are misplaced, because "slightly out of order" and "essentially random" call for different responses. `repair=True` reorders columns to row order in memory and warns, naming PR Arcadia-Science#106. Censoring. `get_line_for_protid` writes the literal string "0.0" for any pair absent from the Foldseek output, and Foldseek's scores arrive in %.3E form. The two are perfectly separable in the file and identical after pandas coerces both to float. On the same production matrix, 60.48% of cells are the fill and not one is a measured zero -- the lowest score Foldseek reports anywhere in it is 0.0549, so "0.0" is a value Foldseek never produces. Those cells are missing measurements, not measured dissimilarity. The loader builds the censoring mask by comparing the raw token before float conversion, which is the only point at which the distinction still exists. It also counts cells that read as zero without being the fill, so a file where `values == 0` is not a valid censoring test says so rather than being quietly assumed away. `LabeledMatrix` carries protids and columns alongside the values, and has no accessor that returns a bare array. Discarding the labels has to be deliberate. Four defects found by the adversarial review are fixed here, and each has a regression test naming it (docs/REVIEW_LOG.md, gate B): - Duplicate labels are refused on both axes. Previously the repair path reordered via a {label: position} dict, which keeps only the last duplicate: a 2x3 matrix with columns [A,B,B] silently became 2x2, one column was dropped, and cell (A,B) reported 0.7 when its true value was 0.4 -- with is_aligned reporting True. Set equality is not enough; length and uniqueness are checked first. - float32 underflow is detected and warned. 1e-46 is nonzero in double and zero in float32, so it was neither the fill token nor caught by the measured-zero check -- a zero invisible to both. Overflow to inf is caught the same way. - A row/column length mismatch raises MatrixAlignmentError rather than TypeError, so `except MatrixAlignmentError` no longer crashes. - Per-query cap detection now requires the columns *not* to show the same pile-up. A matrix with one censored cell per row is uniform on both axes and is not a cap; it was previously reported as one. - repair=True with require_alignment=False is refused rather than silently ignored. Verified against the production matrix: rejects it by default, and with require_alignment=False reproduces every measurement above -- 2/2530 columns in place, positional diagonal 2/2530 versus label-aligned 2530/2530, 3,871,045 censored cells at 60.4766%, zero measured zeros, and an inferred per-query cap of 1000 with 97.8% of rows sitting on it, predicting a 60.4743% zero fraction against 60.4766% observed. 2.2s for 40.8MB. 42 tests, including permuted-column and duplicate-label fixtures as positive controls on the assertions themselves -- a check nobody has watched fire is a check nobody knows works. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
The design from ADR 0001, with no consumer. Nothing in the existing pipeline calls any of it and the default run is unchanged. That is deliberate: it lets the design be reviewed on its own, before the commit that migrates the TM path onto it has to be evaluated at the same time. ProteinCartography/index.py canonical protid index ProteinCartography/config_schema.py validated config + legacy bridge ProteinCartography/spaces/base.py BlockSpec/BlockResult/SpaceSpec ProteinCartography/spaces/registry.py entry-point discovery ProteinCartography/spaces/store.py .npy block store ProteinCartography/spaces/manifest.py provenance and cache keys index.py exists because pandas makes misalignment easy and silent. `df.reindex(other)` fills absent labels with NaN and returns happily, and a NaN row survives all the way to a coordinate for a protein that was never measured. `ProteinIndex.align` raises instead, naming the missing protids. Extra protids are dropped, because a block computed over a superset is fine; only absences are an error. Per-cell annotations are named channels rather than one anonymous mask. `censored` (not measured, recoverable), `absent` (no value exists), and `confidence` each have a fixed polarity and dtype, and unknown names are rejected. A single `mask` field had no declared polarity -- numpy.ma reads True as invalid, pandas and sklearn read True as valid -- and no declared meaning, so the three providers that need a per-cell annotation would each have filled the same slot with a different idea. The array is written to disk, where that ambiguity becomes permanent. Pairwise blocks must declare how they were symmetrized, and `pairwise_directed` keeps both directions. TM-score is length-normalized per query, so TM(a->b) != TM(b->a) whenever lengths differ; on production data only 40% of both-measured pairs are exactly equal, with a maximum disagreement of 0.67. Collapsing that to one number is a modelling choice, and previously the condensed-triangle shape check made it a silent one. The config validator enforces ADR 0003. A `fusable: false` block in a multi-block space is rejected with the reason stated in the error, not a generic type complaint -- the reason is the mechanism, since a future maintainer meeting a bare rejection will read it as an obstacle and remove it. The defaults are keyed on `provider`, not on the block id: keyed on the id, the table protected `taxonomy:` and missed `tax:`, `Taxonomy:` and `lineage:`, which is to say it protected exactly the users who already knew about it. An explicit `fusable: true` override is allowed but must carry a written justification, which is recorded. `representation: direct` is gated behind verified alignment (ADR 0007), and the gate now requires a real boolean: `alignment_verified: "false"` is a truthy string and used to switch the gate off, the opposite of what it says. `from_legacy()` turns an existing config.yml into one `tmscore` block in one `structure` space, with `plotting_modes` as that space's reducers. A test loads the repo's actual config.yml through it. The cohort default is named `as_filtered`, not `accession`. Reading `fetch_uniprot_metadata` showed that Arcadia-Science#106's sort does not survive the round-trip through UniProt -- the list `download_pdbs` truncates is in UniProt response order -- so calling the current behavior "accession" would repeat the same mistake. ADR 0008 is corrected accordingly. `accession` and `significance` are available and opt-in, because both change which proteins reach the map. Validation is hand-rolled on frozen dataclasses rather than pydantic. ADR 0010 has the argument: the validator runs inside the snakemake driver environment, that environment is closed to new dependencies for this work, and pydantic v2 is a compiled dependency in the one environment that has to keep working. Further defects found by the adversarial review and fixed here, each with a regression test naming it (docs/REVIEW_LOG.md, gate B): - `ProteinIndex.align` refuses duplicate labels in the *source*. It previously built {label: position} over them, keeping the last, so aligning ['A','B','A'] to index ('A','B') silently returned source row 2 for A. The length check still passed. - `write_block` no longer mutates the caller's manifest, and output-derived facts live in `derived`, outside the cache key. The key previously included the stored values' digest, so it could only be reproduced by a caller who had already computed the block -- the cache could never hit across processes. The two tests that appeared to prove otherwise were passing only because of the mutation. - `values_digest` hashes the array as stored (float32), so it can verify the file it describes. - The store swaps the new directory in before removing the old, rather than deleting first. - `BlockResult` rejects object-dtype features at construction instead of failing later inside np.save, and rejects a NaN that no channel explains. - `with_protids` refuses a different-length or duplicated label list. It renames rows and never reorders them, so a permuted list would have attached every protein's values to a different protein. setup.py gains the new subpackage, plus a test that every subpackage with an __init__.py is listed. `setup()` does not recurse, so an unlisted subpackage is silently absent from an installed copy while working fine in a source checkout. The guard was checked by removing the entry and confirming it fails. 185 tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
Pure addition, nothing wired. Reports what the per-query hit cap costs a map, using the mask that matrix_io recovers. Phase 0.5 confirmed that a zero in `all_by_all_tmscore_pivoted.tsv` means "this pair lost the per-query top-1000 cut", not "these proteins are dissimilar" (ADR 0009). On a production 2530-protein run that is 60.5% of the matrix, and not one of those cells is a measured zero. This module turns that into numbers a reader can act on. Four of the five reports are bookkeeping. The fifth is not: cross_cluster_edge_retention() compares how often within-cluster pairs survived the cut against how often between-cluster pairs did. Between-cluster pairs carry the weakest scores, so they fall off the per-query list first. At high censoring almost every surviving edge is within a cluster, which means the clusters look *crisper* as censoring worsens while their arrangement relative to one another decays into noise. The map becomes more convincing and less true at the same time, and nothing about looking at it reveals that. Hence a number. asymmetry_report() reports three denominators rather than picking one. The same asymmetry is 36.6% or 53.6% depending on whether you divide by ordered cells or unordered pairs, and the maximum disagreement between directions is 0.99 or 0.67 depending on whether censoring fills are counted as data. Only the last of those is about asymmetry at all -- comparing a measured value against a fill measures the fill. Both figures were computed during Phase 0.5 under different denominators and were briefly taken for a discrepancy, so each is now reported with its denominator named. Everything here reads the matrix through labels, never positions, so it gives the same answer on a pre-Arcadia-Science#106 permuted matrix as on a fixed one. There is a test asserting exactly that. The report ends with plain-language interpretation, because a QC number nobody reads is a QC number nobody acts on. It warns when a per-query cap is detected, when censoring exceeds half the matrix, when a measured zero makes `values == 0` an invalid censoring test, and when cross-cluster retention has fallen far enough that the arrangement of clusters should be trusted less than the clusters. Per-protein censoring rate is emitted as a tidy table. It is overlay-only (ADR 0003): it describes how well a protein was measured, not the protein, and it correlates with length. 24 tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
The refactor this whole design was for, and the only commit here with any
real risk. Its one job is to prove it changed nothing.
The parity test is the evidence, and it is written to be hard to satisfy
by accident:
1. run the baseline (tag multispace-base) twice and compare, to
establish empirically which files are inherently nondeterministic;
2. run this branch twice and assert it is no less deterministic;
3. assert everything outside that floor is byte-identical.
The floor is measured, not declared, so the test cannot be weakened by
quietly adding an exclusion. It compares every file in the output tree
rather than an allowlist, because an allowlist silently ignores files
nobody thought to list, and a new output that differs should break the
test rather than slip past it. On top of that,
`assert_critical_outputs_compared` names the eleven artifacts that carry
the promise and checks they were actually reached -- a parity test can be
hollowed out one reasonable-looking exclusion at a time.
Measured result: 90 files compared, 87 byte-identical, 3 identical once
Plotly's random figure uuid is normalized, 0 differing. That includes
aggregated_features.tsv, the pivoted matrix, both embeddings, the Leiden
and StruCluster assignments, every per-protein feature table, and every
downloaded structure.
Arriving at the exclusion list took three attempts and the middle one is
worth recording. Excluding foldseek's `temp/` by assumption was too
coarse -- it silenced 113 files and left only 41 compared. Removing the
exclusion entirely then failed, because foldseek names its scratch
subdirectory with a random integer, so the *set* of files differs
between runs, and which shard a record lands in depends on thread
scheduling, so the self-diff floor could not stabilize it either. The
exclusion is back, now with the measurement behind it in the docstring.
What actually moved:
spaces/reducers/core.py the numerical core of PCA/UMAP/t-SNE
blocks/tmscore.py the existing representation, as a block
dim_reduction.py -112/+51, now a shim over that core
dim_reduction.py keeps its CLI, its filenames and its return types
exactly. The point of the shim is that the legacy map and any new
space's map are now produced by the same code and cannot drift; two
implementations of "the pipeline's PCA" would eventually disagree, and
the disagreement would surface as a scientific difference with no
obvious cause.
Three behaviors were preserved deliberately because they look like
mistakes and are not. PCA columns are zero-based (PC0) while UMAP and
t-SNE are one-based; renaming either renames a column in
aggregated_features.tsv. svd_solver stays pinned to "full". And below
N=3 the UMAP fallback reuses the input's existing PC columns rather than
running PCA on PCA output -- I dropped that on the first pass and caught
it re-reading the original, which would have changed coordinates
silently in the one regime nobody looks at.
The tmscore block offers `profile` and `direct`. `profile` is what the
pipeline has always done -- each protein's row as its feature vector --
and is invariant to a consistent column permutation, which is the only
reason the shipped UMAP is not garbage on pre-Arcadia-Science#106 output. There is a
test asserting that invariance rather than assuming it. `direct` reads
the matrix as a matrix, is wrong on 99.92% of cells for a permuted one,
and is gated behind both the alignment assertion and a config flag. It
must also declare its symmetrization, since TM-score is length-
normalized per query and the three rules provably give different
numbers.
The Snakefile is untouched. Generalizing {plotting_mode} to
{space_id}/{reducer} is the remaining piece and is left for review: it
changes DAG structure, and the wildcard constraints interact with path
separators in a way that deserves a second reader. So the blocks and
spaces machinery is present, tested, and not yet wired into the DAG --
the same posture the abstractions landed in, for the same reason.
Also here: `mutation_check.py`, which deliberately breaks the pipeline
eight ways and checks the parity test notices. A parity test that passes
because it is not really comparing anything is worse than no test.
conftest.py gains a `slow` marker and --runslow; the repo had neither,
and the parity suite takes four pipeline runs. CI will run it on every
pull request.
test_optional_dependencies.py makes ADR 0006 mechanical: the test
environment has no scikit-learn, umap-learn, scipy, or torch, and every
core module must still import. That guarantee was previously a promise;
now it fails in CI if broken. It also guards itself -- if the test
environment ever gains scikit-learn, it says so rather than passing
while proving nothing.
254 unit tests pass, 6 skipped; 11 parity tests pass under --runslow.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
Gate C ran mutation testing against the parity test and it failed. This
is the fix, and the finding is worth more than the fix.
Eight deliberate defects, four survived. Not because the comparison was
weak -- because the 11-protein fixture cannot express them:
PCA n_components 30 -> 20 both clamp to min(shape) = 11
UMAP n_neighbors 80 -> 40 both clamp to n - 1 = 10
Leiden n_pcs 30 -> 10 both clamp to min(n-1, n_vars-1) = 10
censoring fill "0.0"->"0.00" all 121 pairs are measured, so the fill
token is never emitted at all
PLAN.md predicted this for the PCA solver and insisted the N>500 fixture
was load-bearing rather than a nicety. It is worse than predicted: the
same fixture also hides three other parameters and the entire censoring
mechanism. An end-to-end parity test on the demo data is necessary and
demonstrably not sufficient.
Running the pipeline at N=750 would mean synthesizing 750 PDB files and
a Foldseek run. But the port only touched the reduction step, and that
step consumes a matrix, not structures. So `synthetic_matrix()`
generates one directly and the reducers run on it from both checkouts.
The generated matrix reproduces the production matrix's measured shape:
60.00% censoring against the real 60.48%, rows uniform at exactly the
per-query cap while columns vary (256-347 at N=750), an exact 1.0 label
diagonal, Foldseek's %.3E formatting, and no measured zeros. It can
reproduce the Arcadia-Science#106 column permutation on demand. It is seeded and
procedural -- the seed is the fixture -- which also keeps it clear of the
publishability question hanging over the real-data slice.
At N=750, all five reducer mutations are detected, including
svd_solver="full" -> "auto". That one undoes PR Arcadia-Science#106's determinism fix
and is invisible at N=11 because the randomized solver only engages
above 500 rows. It is the single clearest demonstration of why the small
fixture cannot stand alone.
Two harness defects, both of which made the suite report better than it
was:
- A mutation whose anchor text had moved was scored as DETECTED,
because the resulting exception was caught in the same branch as a
pipeline crash. Nothing was mutated, so nothing could have differed.
Outcomes are now detected / survived / did-not-apply, and the last
fails the run.
- `n_components=30` appears twice in main(), once per branch, and the
harness patched only the first -- the pca_tsne branch, which a
pca_umap run never executes. Two other mutations targeted reducer
defaults that the caller always overrides, and one ran under a mode
that never reaches t-SNE. All three looked exactly like holes in the
parity test from the outside. `_patched` now checks the occurrence
count against a declared expectation and replaces every match; an
ambiguous anchor is an error. Mutations declare their mode.
Final: 12 mutations, 8 detected, 4 survived with the fixture limitation
that explains each one recorded in the harness, 0 unexplained holes, 0
non-applying. Every expected survival has a corresponding N=750 mutation
that is detected. mutation_check.py exits non-zero on any unexplained
hole, so it is a check and not a report.
263 tests pass, 14 skipped.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
The remaining piece of the port. The legacy rules are untouched, so
parity holds by construction rather than by test -- though the test
still runs.
Two new rules, `compute_block` and `reduce_space`, plus the scripts they
call. Neither is reachable unless the config defines `spaces`:
`MULTISPACE_TARGETS` is empty otherwise, and the `block_id`/`space_id`
wildcard constraints are set to a pattern that matches nothing. The
cluster-mode DAG is still exactly 16 rules; with
demo/multispace/config.yml it is 15, adding one `compute_block` and two
`reduce_space` jobs and dropping `run_foldseek` because that demo leaves
`key_protids` empty.
Rather than replacing `{plotting_mode}` with `{space_id}/{reducer}` and
writing a compatibility layer, the legacy rule stays exactly as it is and
the new rules are added beside it. The plan called for the former; this
is a smaller diff with the same result, and the property that actually
matters -- that the two paths cannot produce different numbers -- comes
from both calling `spaces/reducers/core.py`, not from sharing a rule.
The wildcard constraints are the subtle part and the reason this was held
back for review. Snakemake wildcards are regexes, and an unconstrained
`{space_id}` matches greedily across `/`, so a path like
`spaces/a/embedding_b.tsv` can resolve with `space_id="a/embedding_b"`.
Both new wildcards are pinned to the ids the config defines.
The Snakefile now imports `config_schema`, which needs the package
directory on `sys.path` -- the modules import each other flat, which
works when snakemake runs them as scripts but not when the snakemake
process imports one itself. Three lines, with the reason.
Validating at parse time is deliberate: a config that tries to fuse an
overlay-only signal fails before any work happens rather than four hours
in, which is the whole point of ADR 0003's enforcement.
`reduce_space` refuses multi-block spaces rather than reducing one block
and calling the result the space. Fusion is a later commit, and a map
that looks correct and is not is the worst possible failure here.
A block whose provider is unavailable records a SKIPPED.json and returns
zero, so a missing optional dependency costs that block and nothing else
(ADR 0006). A space that needs a skipped block fails with the skip reason
quoted.
263 tests pass, 14 skipped. ruff, ruff-format and snakefmt clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
The Gate C write-up said "every expected survival has a corresponding N=750 mutation that is detected". That was false for two of the four. The N=750 suite runs the *reducer*, which starts from a matrix, so it never exercised the pivot that creates the censoring fill, and never touched Leiden, which forks from the matrix independently. Those two mutations were covered by nothing at all. Two component runners close it. `run_pivot` drives `pivot_foldseek_results` from a synthetic raw pair list -- the fill only exists downstream of a pair list, so a matrix fixture can never test it. `run_leiden` drives Leiden on the N=750 matrix. Both mutations are now detected. Getting the Leiden one to fire took two further corrections, and both are more interesting than the fix. First: the generated matrix was uniform noise. Its censoring rate, cap signature, diagonal and number formatting all matched production, so it looked realistic -- but it had no cluster structure, so a clustering parameter had nothing to bite on. Leiden at 30 principal components and at 10 returned the same partition of noise. The generator now plants contiguous groups, within-cluster 0.70-0.95 and between-cluster 0.10-0.45; measured on the fixture, 0.825 against 0.275. A fixture can be statistically faithful and still test nothing: matching the marginal distributions is not the same as matching the structure the code looks for. Second: the mutation was anchored on `def scanpy_leiden_cluster(..., n_pcs=30)`, a default that `main()` always overrides from argparse. That is the third time in this suite I anchored on a default the caller passes over, and every time it presents as "survived" -- indistinguish- able from a genuine hole in the test. The `Mutation` docstring now warns about it: anchor on the value the executed path actually reads. 14 mutations across three scales: 10 detected, 4 survived as expected with the fixture limitation recorded against each, 0 unexplained holes. All four N=11 survivors now have a counterpart that fires. 263 tests pass, 14 skipped. ruff, ruff-format and snakefmt clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
Which proteins reach the map is decided by `accessions[:max_structures]` in `download_pdbs`, with no log line and no record. ADR 0008 makes that a named, configurable choice; this commit is the library half, so the wiring commit that follows is small enough to read as a diff. Two things here are deliberate and easy to get wrong in the other direction. The default rule, `as_filtered`, does not sort and does not deduplicate. Both would look like tidying up and both would select a different set of proteins. A duplicate counts against the cap today, so it counts against the cap here. The rule is named `as_filtered` rather than `accession` because the sort PR Arcadia-Science#106 added does not survive the round trip through UniProt -- naming it "accession" would repeat exactly the assumption that made Arcadia-Science#106 incomplete. Significance carries a declared polarity. Lower is better for an e-value and higher is better for a TM-score, so a bare score mapping is a sign error waiting to happen -- one that inverts the cohort to the *worst* hits while looking entirely normal. SIGNIFICANCE_MEASURES names each measure and its direction, the same shape as CHANNEL_SEMANTICS in spaces/base.py, and the sort is derived from that table rather than from an argument the caller supplies. The taxonomic comparison reports per lineage *term*, not per rank. Ranks are not comparable across kingdoms and choosing a depth would impose a vocabulary; a term is a string both sets either carry or do not, so the comparison needs no taxonomy of its own and degrades to "no data" instead of to a wrong answer. It reports a max absolute proportion difference named for its denominator rather than a "divergence" -- proteins carry many terms at once, so the proportions do not sum to one and no distance is defined over them. No pandas and no file formats, so it imports in the minimal python 3.9 environment `download_pdbs` runs in (verified) and tests without fixtures. 43 tests. Nothing imports this yet. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
`download_pdbs` truncates the hit list to `max_structures` and says nothing. This wires cohort.py in behind that cut, so the rule is named and the run reports what it discarded. The default rule is `as_filtered`, which is the same prefix of the same list in the same order, and parity proves it: 41 files compared, 38 byte-identical, 3 identical after Plotly-uuid normalization, 0 differing. On the repo's own demo fixture the report is not a formality. 24 hits, 20 surviving the metadata filter, 10 admitted -- half the candidates dropped -- and the discarded half is taxonomically different from the kept half: every Chiroptera hit discarded (0% retained vs 40% discarded), 4 of 5 Artiodactyla kept. Any clade claim from that run is conditioned on a difference no file previously recorded. The two new inputs to the checkpoint are read only for the report and are already ancestors of it through `filter_aggregated_hits`, so they add an edge the DAG already had and no job. The cluster-mode DAG is unchanged at 16 rules; cluster mode never runs this rule, because the user supplies the structures and there is no cohort decision to make. The parity harness needed a fourth category. Until now every file was compared, excluded by rule, or nondeterministic; cohort_report.json is none of those -- it is *new*, and the baseline has no equivalent. ADDITIVE_OUTPUTS lists it by exact path with its reason, and the allowance is deliberately narrow: it applies only to the side named as the baseline, so a file that goes missing from the branch is still a failure, and between two runs of the same code it does not apply at all. `compare_trees` now requires the caller to name which side is the baseline rather than inferring it. The first version assumed the new tree was the second argument; the parity test passes it first, so the real run failed with "only in A: cohort_report.json". Inferring the direction would have silently accepted a *deleted* output in every call site that passes the other order -- mutation_check passes reference-first. A test now pins both orders. Two mutations added, both detected: the default rule quietly sorting (the exact change ADR 0008 rejected), and an off-by-one at the truncation point. A third, inverting the significance polarity, survives and is recorded as a deliberate hole -- the default config never executes that path, and making it do so would mean changing the default cohort. It is covered by unit tests that assert the e-value and TM-score directions against each other. 17 mutations: 12 detected, 5 survived-as-expected, 0 unexplained holes. Parity: 28 slow tests pass. 327 unit tests. Cluster DAG still 16 rules. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
Completes group 3. `selection: significance` previously raised an error naming a rule that did not exist; this builds it. **ADR 0008 asked for a TM-score ranking and that is not achievable.** Search mode queries the Foldseek *web API*, whose .m8 output has 21 columns and no alignment TM-score -- verified against a recorded response, not inferred from the column list. The TM-scores this pipeline is built around come from the local all-versus-all run, which operates on the downloaded structures. Ranking the cohort by TM-score therefore needs the structures the ranking exists to choose. No ordering of the DAG resolves that; it is where the measurement comes from. So the measure is the e-value the API does report, best across every query that found the hit, with the bit score alongside. The ADR is corrected in place, including the honest consequence: an e-value is alignment significance, not structural similarity, so `significance` selects confidently-detected hits rather than structurally close ones. Weaker than the draft promised, and still the only rule that is both reproducible and principled. Three things this needed: `map_refseq_ids` now optionally writes the from/to pairs it already computes and discards. Without that correspondence a BLAST e-value -- reported against a RefSeq accession -- cannot be keyed to a candidate, which is keyed on UniProt. Written sorted and deduplicated, because it lands in the run directory and an unordered dump of an API response is the shape of defect PR Arcadia-Science#106 had to fix. `aggregate_hit_significance` stays out of the DAG unless asked for. The default rule needs no scores, so the default search DAG is unchanged at 25 rules and gains a 26th only under `selection: significance`. Cluster mode is unchanged at 16; the multispace demo at 15. An unknown measure is now rejected at config-parse time. Where it is *used* is after the searches have run, which is hours too late to learn about a typo. SIGNIFICANCE_MEASURES moved to config_schema for that -- it is configuration vocabulary, and it lives next to SELECTION_RULES. **Bug found in group 2's `from_legacy`, fixed here.** The legacy branch rebuilt the cohort mapping from scratch and copied only `max_structures` across, so a `cohort:` block in a config with no `blocks`/`spaces` keys -- which is every existing config -- was discarded. `selection:` parsed, validated, and did nothing; the DAG silently used the default. Found by writing a config with `selection: significance` and watching the rule count not change. Both branches now share one helper, with a regression test on each. Verified end to end under mocks: 2289 scored accessions, cohort report reads rule=significance / reproducible=true, and the "not reproducible" warning is correctly absent. Most downloads fail in that run because the fixture only carries AlphaFold responses for the ten proteins the *default* rule picks -- which is itself confirmation that the rule selects a different cohort. Parity: 30 slow tests pass, 0 differing. 349 unit tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
Two defects in already-reviewed code, one corrected ADR, and the number the demo fixture turned out to produce. The `from_legacy` finding is the one worth reading. A `cohort:` block in a plain config.yml was discarded, and it parsed and validated on the way to being discarded, so nothing reported it. It survived Gate B because `minimal()` in test_config_schema.py defines blocks and spaces -- the entire cohort section of that file exercised the branch almost no user takes. A test helper that always takes the same branch is a blind spot with a name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
`foldseek_apiquery.py` accepts `--mode tmalign`. A live query against afdb-swissprot with the demo actin structure shows the server returns the same 21 columns in the same positions as 3diaa, with different meanings: the column constants.FOLDSEEK_COLUMN_NAMES calls `evalue` holds a TM-score, and `bits` holds roughly TM times 100. 938 hits, e-value column running 0.402 to 0.9999 with everything inside [0, 1], actin at the top and an unrelated GTP pyrophosphatase at the bottom. Nothing renames and nothing errors. Only the polarity inverts. Two consequences. `extract_foldseek_hits.py` filters `evalue < 0.01` and would keep 0 of those 938 hits -- pre-existing and latent, since the Snakefile never passes --mode, recorded as FOLLOWUPS #25. And hit_significance.py, written two commits ago, ranks `evalue` ascending: it would have put the pyrophosphatase (TM 0.402) above actin (TM 0.9999). That is precisely the inversion SIGNIFICANCE_MEASURES exists to prevent, arriving through the data instead of through the code. hit_significance.py now refuses rather than guesses. Two conditions, not one: values bounded in [0, 1], never small, *and* bit scores at TM-score scale. A weak 3Di-AA search really can return only e-values near 1, so the second condition is what makes a false positive unlikely, and the check errs toward not firing. Verified against both real files -- it fires on the live tmalign response and stays quiet on the recorded 3diaa fixture, which still scores 810 accessions. Refusing is deliberate and temporary. The right fix is to record the mode next to the results, after which either mode reads unambiguously. That is scoped in PLAN.md rather than done here, because exposing the mode changes which hits come back, hence the cohort, hence the map. **This also re-corrects ADR 0008, whose first correction was mine and was wrong.** I claimed a TM-score was unobtainable because the pipeline's TM-scores come from the local all-versus-all run on downloaded structures. That circularity is real for the *matrix* -- but cohort ranking needs one score per candidate against the queries, not the matrix, and tmalign mode supplies exactly that before anything is downloaded. I generalized from one verified fact about the recorded fixture to a claim about the whole API, and stated the inference with the confidence of the measurement. The tell was two lines above the code I had already read: SET_MODES = ["3diaa", "tmalign"]. The decision is unchanged -- the default measure is still the e-value -- but the reason is now "blocked on an unrecorded mode and a column collision" rather than "impossible", which turns a dead end into scoped work. ADR 0007 says never index a labeled matrix positionally, because columns move. This is that defect inverted: a column name is not a contract when its meaning depends on an unrecorded run mode. The columns stay exactly where they are and mean something else. 354 unit tests. Lint clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
They never had. `compute_block` and `reduce_space` were wired into the Snakefile in e72f87d and verified by DAG resolution only; the first time either was executed -- while building the 3Di block -- both failed at import with `ModuleNotFoundError: No module named 'yaml'`. They run in envs/analysis.yml, which has no PyYAML. This affected `tmscore` too, so no block had ever been computed by the pipeline. Adding PyYAML to envs/analysis.yml is the obvious fix and the wrong one. That changes the environment's hash, which forces a fresh solve of the one environment whose package versions decide the pipeline's numeric output, and this repository has already been bitten once by a fresh solve pulling a numpy-2-built matplotlib in beside a pinned numpy 1.23.5. Rebuilding that environment to gain a config parser is a large risk for a small convenience. So the parser goes away instead. A `multispace_config` rule writes the config as JSON, config_io.load_config reads JSON with the standard library, and PyYAML is imported lazily for the path where somebody runs these scripts by hand. The rule uses `run:` so it needs no conda environment of its own, and it is only reachable when the config defines `spaces`. Second defect, found immediately after: `write_block` discarded the manifest its caller had just built. It rebuilt a minimal one from the spec, so every block on disk had `inputs: {}` -- the provider's recorded input digests, seed, and stats never reached the file. `tmscore`'s censoring summary was among the casualties. That is not only lost provenance. `inputs` feeds `cache_key`, so a changed input matrix produced an unchanged key and a stale block looked fresh. `write_block` now prefers the manifest carried on the result, falls back to a rebuild for direct callers, and still stamps its own output-derived facts on top. Six tests, including the cache-key property stated directly. Both defects share a cause worth naming: a rule can resolve in the DAG, pass `snakemake -n`, and never have run. Dry runs check the graph, not the jobs. Verified: the multispace demo now completes 18/18 steps and writes two blocks with populated manifests. Parity unaffected -- 31 slow tests, 0 differing. Cluster DAG still 16 rules. 398 unit tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
First of the free blocks (group 6), and the first thing in this work that gives the map a second opinion. Foldseek already encodes each residue's local tertiary environment as a letter of a 20-state alphabet -- it is how foldseek searches at all -- and the pipeline already depends on foldseek. So this block costs no new dependency and no new computation beyond one extraction pass. Why it is worth having next to `tmscore`: TM-score is a global superposition score, so two proteins that share a domain but arrange their domains differently score poorly, because no single rigid superposition fits both. A 3Di k-mer profile has no such notion -- a shared domain contributes the same k-mers whatever the hinge angle. The two blocks are built to disagree, and ADR 0001 says the disagreement is the product. The provider computes nothing. Extraction needs the foldseek binary, which is in a different conda environment from the one the providers run in, so `foldseek structureto3didescriptor` is its own rule and the provider reads its TSV -- the same arrangement `tmscore` has with the pivoted matrix. It also keeps the module free of subprocesses, so the whole thing tests from a string. One hazard in the descriptor format, and it is the same shape as the tmalign column collision two commits ago: fields 2 and 3 are the amino-acid sequence and the 3Di string, **both uppercase letter strings of exactly the same length**. Read the wrong one and you get a sequence profile labelled as a structural one, with nothing in the output to indicate it. The reader asserts what it can -- four fields, equal lengths, and the two sequences not being identical -- and names the column index rather than inlining it. Frequencies rather than counts by default, because counts scale with protein length and would make the dominant axis of any reduction "how long is this protein". True, and not what the block measures. The test states that as a limit -- the gap between two same-composition sequences shrinks as both grow -- rather than as an arbitrary threshold, which is what the first version did and it was wrong by 0.011. The vocabulary is the observed k-mers, sorted, not the full 20**k grid: at k=3 that grid is 8000 columns and the demo cohort uses 486 of them. It is recorded in the manifest when small enough to read. Verified on real foldseek output, not a fixture: 11 demo structures, 3Di strings matching their sequence lengths exactly, and pairwise distances spanning 0.020 to 0.118 with the longest and most divergent structure as the outlier in both farthest pairs. The shipped multispace demo now builds two co-registered spaces over one protein index and completes 19/19 steps. Default DAGs unchanged: the extraction rule enters only when a block actually uses this provider. Cluster mode 16 rules. 39 new tests, 398 total. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
Two defects in already-reviewed code and one format hazard designed against. The one worth reading is G6.1: `compute_block` and `reduce_space` had never executed. They passed `snakemake -n` and left the cluster DAG at 16 rules, and both of those checks were satisfied while the rules were broken. The standing consequence is written down, because every remaining group adds rules. G6.3 is the second time in three commits that two adjacent columns of the same type and different meanings turned out to be a trap -- tmalign's e-value/TM-score first, now the 3Di file's amino-acid/3Di pair. Recorded as a pattern rather than as two incidents. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
`tmscore` is global shape and `threedi` is local shape. This block is not shape at all. Hydropathy, charge and isoelectric point separate membrane proteins from soluble ones and acidic from basic ones, and they do it for proteins whose structures are too different for any superposition to relate -- which is the argument for co-registering a third geometry rather than fusing a third feature set (ADR 0001). No new dependency and no new fetch. The sequences are already in `uniprot_features.tsv`, which both modes produce, and the descriptors are computed from published constants rather than from Biopython. Biopython is already in `envs/web_apis.yml`, but `compute_block` runs in `envs/analysis.yml`, and adding it there would change that environment's hash and force a fresh solve of the environment whose pins exist precisely because a fresh solve once installed a numpy-2-built matplotlib beside `numpy=1.23.5`. ADR 0006 also requires every block named by the default config to work with zero optional dependencies installed, which this now does. The constants are Biopython's, so the two agree, and the tests check that against the real library whenever it happens to be installed. That is not decoration: the first draft used the EMBOSS pKa set from memory instead of the Bjellqvist set Biopython uses, and every charge and pI was wrong by a plausible-looking amount. Comparing the tables directly -- rather than only values derived from them -- is what named the cause. The same cross-check found arginine's weight transcribed as 174.2017 against Biopython's 174.201, a difference too small for the derived-value tolerance to see. Every default descriptor is intensive: a per-residue mean, a fraction, or a pH. `molecular_weight` is available and deliberately not a default, because MW is about 110 Da per residue and nothing else, so fusing it makes protein length a principal axis of a biology map -- the exact argument ADR 0003 uses to keep pLDDT out of a geometry. Asking for it is recorded in the manifest and warned about on stderr. Two pieces of plumbing come with it: - `--provider-input NAME=PATH` on `compute_block`, because the features table is in the output directory in search mode and in the user's input directory in cluster mode. It reaches the provider through `ctx.extras` and never enters the manifest params, so a machine-specific path cannot get into a cache key. Only the file's digest is recorded. - `test_compute_block.py`, because the entry point had no tests at all. On the demo's eleven actins the block reproduces human beta-actin's published pI of 5.29, and separates the one outlier -- a 507-residue fungal actin-histone fusion -- at pI 8.7 with a positive net charge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
`BlockConfig.normalization` defaulted to `unit_mean_distance`, and `compute_block`
passed that default down to the provider as though the user had asked for it:
params.setdefault("normalization", block.normalization)
`block.normalization` was never None, so the `setdefault` always won and every
provider's own `params.get("normalization", ...)` default was unreachable. The
providers that existed both wanted `unit_mean_distance`, so the two agreed by
coincidence and nothing showed.
`biophys` does not agree. Its columns are a pH beside a per-residue charge beside a
dimensionless fraction; pI ranges over 4 to 12 while charge per residue ranges over
about -0.1 to 0.1, so an unnormalized euclidean distance between them is the isoelectric
point and nothing else. It asks for `zscore_within`, and it was getting
`unit_mean_distance` -- written into its manifest, which is the record a reader would
later trust.
`normalization` is now optional, and None means "ask the provider". Nothing else
changes: both existing providers default to `unit_mean_distance`, `from_legacy` sets it
explicitly, and `to_spec` -- which builds a spec without a provider to ask -- falls back
to the same historical value. The field is still validated when it is given.
The reason this needed a new test file rather than a new assertion is worth recording.
`test_the_block_standardizes_its_columns_by_default` already existed, already asserted
`zscore_within`, and already passed, because it calls the provider directly. The bug
lived in the caller. It is the same failure mode as a mutation anchored on a default the
caller overrides: a default is dead code if its caller always passes the parameter, and
no test of the callee can tell you so.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
The other three blocks measure these proteins. This one reads what a curated database
says about them, which is why it is worth co-registering against them rather than fusing
into them. Pfam and InterPro assignments come from HMMs built on alignments far wider
than any cohort here, so two proteins can share a family while being too distant for
TM-score to relate at all. Agreement with `structure` is confirmation; disagreement is
either a remote homolog the structural map missed or an annotation artifact, and both
are findings (ADR 0001).
No fetch and no new dependency: the Pfam and InterPro columns already arrive with the
UniProt metadata the pipeline downloads.
Three things are deliberate.
**A missing annotation is not an absent domain.** Same distinction ADR 0009 draws for
the similarity matrix's `0.0` fill, and it bites harder here because the bias is not
random -- well-studied proteins are better annotated, so an unannotated row means
"nobody has looked". Unannotated proteins land at the origin, where they resemble each
other exactly; that resemblance is an artifact of research attention. The manifest
records `proteins_without_domains` and `annotated_fraction` so the artifact is visible
instead of arriving as a cluster. A cohort with no annotations at all is an error that
says the domain space cannot be built for this run, rather than a block with zero
columns.
**The reader checks the accession shape rather than trusting the header.** `Pfam` and
`InterPro` are adjacent columns of semicolon-separated uppercase accessions and are
indistinguishable by shape, so reading the wrong one yields a plausible block of the
wrong thing. `PF\d{5}` and `IPR\d{6}` tell them apart, and tests swap them to prove the
check fires. Same hazard as the 3Di descriptor file's two sequence columns, same
treatment.
**`jaccard` is refused rather than accepted and ignored.** It is the natural distance
between two sets and it is in `spaces.base.METRICS`, but `reduce_space` feeds features
into a euclidean PCA without consulting `spec.metric`, so declaring it would write a
claim into the manifest that nothing honors. Euclidean distance on binary presence
vectors is the square root of the number of families two proteins differ on -- usable,
and weighting a heavily annotated protein more than Jaccard would. The error says all of
that; a metric-aware reducer is followup work.
The demo now builds four co-registered geometries over one protein index, 23/23. Its
`families` space is nearly degenerate, because eleven actins nearly all carry PF00022
alone -- the honest result for a cohort this homogeneous, and worth the demo showing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
G6.4 through G6.8, closing out group 6. Two are defects that produced wrong values and were caught only because a new block disagreed with an old assumption; three are design decisions worth their reasoning. - **G6.4** — every provider's `normalization` default had been unreachable since group 2, because `compute_block` filled the parameter in unconditionally. Two providers happened to want the same value, so it looked correct. A default is dead code if its caller always passes the parameter, and no test of the callee can tell you so. - **G6.5** — the first draft of `biophys` used the EMBOSS pKa table instead of the Bjellqvist table Biopython uses. Sixteen derived cross-checks failed at once and none named the cause; comparing the constant *tables* named it immediately, and was the only check that caught arginine's mass being wrong below the derived test's tolerance. - **G6.6** — why the default descriptor set excludes molecular weight, and how the intensive/extensive flag keeps that reasoning attached to the descriptor. - **G6.7** — why `jaccard` is refused rather than declared and ignored. A setting nothing reads is worse than a setting that does not exist, because it reads as a decision. - **G6.8** — where the "every numeric component gets a mutation entry" rule stops, stated rather than left to erode. None of the three new blocks is in the default config, so a mutation in any of them would survive for a reason already recorded once. Verification for the completed group: demo runs 23/23 with four co-registered geometries; `biophys` reproduces human beta-actin's published pI of 5.29 on real data and isolates the one fungal actin-histone fusion at pI 8.7; parity 31 slow tests and 0 differing files against the new post-rebase baseline; mutation harness exits 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
`BASELINE_TAG = "multispace-base"` was created on one machine and pushed
nowhere, so the skip message told a contributor to run
git worktree add ../pc-baseline multispace-base
which cannot succeed for anyone else. The baseline is not a tag anyway: it
is the commit this branch forked from upstream, and that is directly
resolvable.
`baseline_commit` runs `git merge-base HEAD upstream/main` and returns the
sha, or None when there is no `upstream` remote to resolve against -- the
normal case for a fresh clone, and not an error, since the value is only
used to build a suggestion. The skip message prints the concrete sha when
it has one and the shell expression when it does not, so it is
copy-pasteable either way.
`docs/ARCHITECTURE.md` invariant 6 named the same tag and is corrected with
it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
Co-registration is the default product (ADR 0001): each kind of evidence keeps its own geometry, and the same proteins sit in all of them. Nothing enforced the second half. The four blocks draw their protein sets from three different files -- the similarity matrix, the 3Di descriptor table, and the UniProt features table -- written by different rules at different points in the run. Two spaces over slightly different sets still reduce, still plot, and still look co-registered; every per-protein comparison between them would be quietly conditioned on an overlap nobody chose. Recorded as FOLLOWUPS Arcadia-Science#30 against this group. `shared_index` takes the intersection in a named reference space's order and returns a report naming every protein each space lost reaching it. Intersecting rather than refusing is deliberate: a provider legitimately has no data for some protein -- no UniProt record, no foldable structure -- and that is a fact about the cohort. What must not happen is losing it silently, so the loss is enumerated rather than counted. Two hard errors: an empty intersection, where there is nothing to co-register at all, and a duplicate protid within one space, which doubles that protein's weight and which set arithmetic would have swallowed. Nothing calls this yet; the entry point and the cross-space metrics follow. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
Neighborhood Jaccard at k is the principled replacement for `calculate_concordance.py`, which subtracts a fraction sequence identity from a TM-score. Both are numbers in [0, 1] and they are not the same scale, so the difference has no unit and no meaningful zero. Jaccard over neighbor sets asks what was meant -- do these two kinds of evidence put the same proteins next to this one -- in a quantity comparable across proteins, spaces and runs. `calculate_concordance.py` is untouched and keeps its column; removing a maintainer's feature inside a large PR is an unforced fight. Rank correlation extends that from the head of the ordering to all of it, and Procrustes disparity asks the weaker question the other two cannot: whether a reader could have superimposed the two plots by eye. Three things worth the reviewer's attention: - All numpy. scipy has one-liners for Spearman, Procrustes and neighbor search, and ADR 0006 requires the default config to run with it absent. Both implementations are cross-checked against scipy where it happens to be installed, and agree to 3e-16 -- including Spearman on a 60%-censored matrix, where tie handling is the common case rather than an edge case. - Ties are reported, not just broken. A k-th-neighbor tie is resolved by protein order, which is reproducible and arbitrary; a mean Jaccard over mostly-tied neighborhoods is partly measuring the protein order, so the count travels with the score. - The distances are euclidean over unnormalized features, because that is what `reduce_space` actually reduces. `spec.metric` is never consulted (FOLLOWUPS Arcadia-Science#29) and `spec.normalization` is applied nowhere. Honoring either here and not there would describe a geometry no map is drawn from, so instead both are stated in `geometry_caveats` on every comparison. Procrustes allows reflections. UMAP output has no canonical handedness, so forbidding them would report two identical maps as maximally different. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
`coregister.py` reads every space named in `coregistration.compare` out of the block store, aligns them all to one protein index, and writes three kinds of file: the shared index and how each space reached it, a per-pair summary, and a per-protein table per pair. Opt-in twice over. The rule is unreachable without `spaces`, and unreachable again unless `compare` names two of them, so asking for spaces does not silently buy a comparison. The cluster and search DAGs are unchanged at 16 and 25 rules; the multispace demo goes 23 to 24. Procrustes needs both layouts in one coordinate system, so the Snakefile picks the first reducer every compared space ran and passes each embedding as `--embedding SPACE_ID=PATH`. Named, not positional, for the reason `--provider-input` is: snakemake hands the shell a bare list of paths, and working out which is which by position is the defect ADR 0007 exists to prevent. When no reducer is shared the disparity is empty rather than fabricated. Running it on the demo found nothing broken and two things worth knowing, both of which the diagnostics surfaced and neither of which is visible in the scores: - The `families` space has two distinct points across eleven proteins -- ten actins carry one Pfam family and the fusion carries two -- so all eleven rows tie at the k-th neighbor and its Jaccard against `structure` is largely measuring protein index order. `boundary_ties` says so. - Four of the demo's eleven proteins have byte-identical 375-residue sequences under four accessions, so every sequence-derived space is degenerate on them too. Real conservation, not a fixture defect. `_features_for` is renamed to `features_for` because `coregister` needs the same block-loading path `reduce_space` uses; two of them would drift. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
ADR 0011 covers the four sub-decisions ADR 0001 left open, each of which has a wrong answer that produces numbers looking fine: intersect and enumerate rather than refuse or fill; Jaccard over neighbor sets rather than a difference of incommensurable scores; report ties rather than silently break them; and describe the geometry that is actually drawn rather than the one the spec declares. `calculate_concordance.py` gains a docstring saying it is superseded and why it is still here. It subtracts a fraction sequence identity from a TM-score -- two numbers in [0, 1] that are not on the same scale, so the difference has no unit and no meaningful zero -- and it is a column in the final results. Keeping it is invariant I6; marking it is the least that can be done. Docstring only, no behavior change. This takes pre-existing files touched from 9 to 10. REVIEW_LOG G7.1-G7.6. Three are worth a reviewer's time beyond the group: - G7.3, the third instance in two groups of a recorded value that no caller consults. `spec.normalization` is validated, written into every manifest, and read by nothing. Not fixed here, deliberately: applying it only in co-registration would make the disagreement metrics describe a geometry no map is drawn from. - G7.4, a negative audit result. PLAN required auditing every comparison using `struclusters` as a structural reference, now that claim B is confirmed. There is no such comparison -- the two live uses are overlays, which ADR 0003 permits, and the fusion path is already blocked. Recording that it was checked and found clean is what stops the next reader re-deriving it. - G7.6, why the three new numeric components get no mutation entry despite the standing rule that they must. The harness measures whether the parity test notices a change, and the parity test compares against a baseline that has no co-registration output at all. Every entry would survive, for a structural reason, and would be indistinguishable from a real hole. The general form is worth more than the exemption: the parity test cannot see anything the baseline does not produce, so everything additive is invisible to it and needs direct tests instead. Verified: parity 33 slow tests / 0 differing files, mutation harness exit 0, all five commits pass alone in a detached worktree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
The demo cohort cannot test cluster enrichment. Eleven proteins, four of them byte-identical in sequence, and a Pfam column with two distinct values across the whole set: a p-value computed on that is arithmetic, not evidence, and a test asserting one would pass for no reason. So the fixture lands first, on its own, with its ground truth checked before anything consumes it. `annotated_cohort` plants named terms in named clusters at a stated marginal rate and shifts named columns by a stated effect size, so a statistic is checkable in both directions -- it has to find the planted signal and stay quiet everywhere else. Only the second half catches a test that passes because everything came back significant. Three properties are copied from `uniprot_features.tsv` rather than invented, each because it has its own way of breaking a parser: the two different multi-value encodings that table carries in one row (a Python list repr for Lineage, a semicolon-terminated run for Pfam), ordinary missingness, and a nested taxonomy whose terms are therefore not independent hypotheses. Four pathologies are planted deliberately -- a universal term, a singleton term, a cluster with no measurements at all, and a constant column -- because each occurs in real data and each has an answer that is easy to get wrong. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
What a cluster is made of is a different question from where it sits, and the pipeline answers it only as a picture: plot_cluster_distributions runs a Mann-Whitney per numeric column and draws a star on a violin plot. The numbers are never written down, so nothing can sort or join them, and the categorical annotations -- taxon, protein family -- are not tested at all, though they are the columns a biologist asks about. Two tests, because there are two kinds of annotation. Continuous columns get the two-sided Mann-Whitney U with the tie-corrected normal approximation, which is what the plotting script already does. Categorical columns get the exact one-sided hypergeometric tail; one-sided is a decision, and the observed and expected counts travel with every row so depletion stays readable. Benjamini-Hochberg over both. All four are numpy and standard-library lgamma/erfc, per ADR 0006, and all four are cross-checked against scipy behind an importorskip: Mann-Whitney to 2e-16 including on heavily tied and 60%-censored input, the hypergeometric to a relative 6e-12 down to p=1e-94, Fisher's one-sided exact test to 8e-15, and Benjamini-Hochberg to 2e-16. A test that could not run is not a test that found nothing. Every result carries a note, and when it is set the p-value is NaN and the row stays out of the correction family -- counting it would deflate every q-value in the table with nothing on the blank row to say so. This is the branch remove_nans takes the other way, substituting a synthetic 0.0 for a cluster with no measurements and testing it against real distributions (FOLLOWUPS Arcadia-Science#34). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
`enrich_clusters` joins a cluster table to an annotation table, tests every (cluster, annotation) pair, corrects within each annotation column, and writes one tidy row per hypothesis with the counts it was computed from -- so a row is auditable without rerunning anything. The clusters come from `leiden_features.tsv`, which is the pipeline's only clustering. Spaces emit coordinates and nothing else, so this describes the structure space rather than the multi-space map however many spaces the run built. The module takes the cluster table by path and names the clustering in every row, so per-space clustering later is a wiring change rather than a rewrite. Gated on the `enrichment` key alone rather than on `spaces`, because a legacy cluster-mode run already produces both tables it needs. Cluster and search DAGs are unchanged at 16 and 25; multispace goes 24 -> 25 and runs 25/25. Running it end to end found what no unit test could, twice over. The crash: `aggregated_features.tsv` already carries `LeidenCluster`, because it is built by joining `leiden_features.tsv` into everything else, and the fixture had written an annotation table with that column dropped -- a shape the pipeline never produces. The two inputs are now reconciled, and disagreement between them is an error rather than a silent preference. The finding: the demo's two Leiden clusters separate perfectly on mean pLDDT and almost perfectly on length, in opposite directions, and the two correlate at r = -0.86. The table has rediscovered from the other end the confound NOT_FUSABLE_REASONS already refuses to fuse. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
ADR 0012 covers the five sub-decisions the phase description does not answer: that enrichment takes a cluster table by path and names the clustering in every row rather than presenting the TM-score Leiden clustering as the multi-space map's; that continuous columns get a two-sided Mann-Whitney and categorical columns the one-sided hypergeometric tail, with depletion recoverable from the counts; that the universe is the annotated background; that the correction family is one annotation column; and that a test which could not run is a row with a reason rather than an absence. REVIEW_LOG G7.7-G7.12 records what the work found. The fixture-first order paid before the statistic existed, catching two generator defects that would have presented later as missed signal. The entry point crashed on a table shape no unit test had built, which is the third sighting of "test the caller's path". The demo's two clusters separate perfectly on mean pLDDT and almost perfectly on length, in opposite directions, at r = -0.86 -- the table rediscovering from the other end the confound NOT_FUSABLE_REASONS already refuses to fuse. And an importorskip test can be written, be correct, and never run: the env with scipy had no pytest, so the gated cross-checks are now executed somewhere they do not skip. `enrich_clusters` joins the ADR 0006 import guard alongside `enrichment`. Gates: 714 pass / 59 skipped; parity 33 tests, 0 differing files; mutation harness exits 0 with its one recorded survivor; each commit verified alone at 596 / 675 / 713; DAGs 16 / 25 / 25; pre-existing files touched still 10. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
Fusion's failure mode is not a crash. It is a map that looks fine and is one block's map wearing four blocks' labels -- the scale failure ADR 0002 is written about. Neither the demo cohort nor any shape assertion can see that, so the fixture comes first, as it did for enrichment in 173549a. `tests/fusion_cohort.py` generates blocks over one protein index where what a fused geometry should contain is fixed by construction: - Two crossed partitions, one per block. `fold` (4 groups) is visible only in the `wide` block and `chemistry` (3 groups) only in `narrow`. The crossing is exact -- all twelve cells hold 20 proteins -- so neither partition carries information about the other, and "this fusion recovered `fold` and stayed blind to `chemistry`" is a statement about the fusion rather than the draw. - Scale and dimensionality deliberately incommensurate: 200 columns at scale 10 against 4 at scale 0.01, a measured 7523x gap in mean pairwise distance, while the two blocks separate their own partitions about equally well (3.43 and 3.71). Any strategy that favours one is responding to units and column count rather than to information. Four pathology blocks, each with an answer that is easy to get wrong: `narrow_rescaled` (identical information in different units, so late fusion must give an identical geometry), `noise` (no partition, but a contribution share that must still be reported and must still sum), `degenerate` (every distance zero, so mean-distance normalization divides by zero), and `constant_column` (where per-block standardization does). The fixture's own measuring stick is a between-over-within distance ratio, implemented by explicit broadcasting rather than through the Gram identity production code uses: a measuring stick sharing an implementation with the thing it measures cannot catch that implementation being wrong. 21 tests, all on the construction rather than on any statistic.
Phase 5's three remaining diagnostics, as statistics. The clustering they need lands next; everything here is numpy over a distance matrix and a label vector, so it is testable in an environment with no scanpy in it -- which is the environment the unit suite runs in. diagnostics/stability.py -- item 3. Per-protein kNN Jaccard between the data and a perturbation, over replicates. Two perturbations, both modelling something the pipeline does: resample the retrieved cohort (without replacement, because duplicates sit at distance zero and would occupy the whole neighborhood), and add Gaussian score noise scaled to a fraction of the median pairwise distance, so it means the same thing on a TM-score block and on a physicochemistry block. The reference neighbor set is recomputed inside each subsample rather than taken from the full cohort. That makes resampling a genuine null -- both sides lose the same proteins and both promote the same replacement -- so a noise-free replicate scores exactly 1.0 and every departure is attributable to the noise term alone. It also keeps subsample_fraction live rather than decorative: a smaller cohort has fewer candidates competing for the k-th slot. k is clamped to what a subsample can supply, round(f*N) - 1, with the request kept and warned about. Group 8b shipped DEFAULT_K = 15 against an 11-protein demo and broke all seven spaces while every unit test passed at N=240; the ceiling here is tighter still, so the test for it is written at N=11. diagnostics/partition.py -- items 7 and 8. Adjusted Rand index and silhouette, plus a resolution sweep that looks for a plateau and a negative-control report. A plateau means a range of resolutions all recover the same grouping, so the grouping is in the data; uniformly low adjacent agreement means the cluster count is a property of the parameter and nothing else. Both statistics agree with scikit-learn to 1e-12 relative, including the degenerate cases where its answer is a convention rather than a formula: ARI of two all-in-one-cluster partitions is 1.0 by definition and 0/0 by the formula, and the pipeline reaches that case whenever leiden_clustering short-circuits below three proteins. embedding.py's _ordering becomes public as neighbor_ordering, so both diagnostics derive "the k nearest" from one sort. Two that reach it separately agree only where there are no tied distances, and censored TM-scores arrive as exact zeros in their thousands. 62 tests, 0 skipped in the full-stack env. One of them is the uncomfortable half of item 8, demonstrated rather than asserted: k-means on a block with no partition in it returns clusters whose silhouette is higher than the *correct* partition of that same block scores. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
Items 7 and 8 are diagnostics about a partition, and no space had one: reduce_space emits coordinates, and the pipeline's only clustering is Leiden over the TM-score matrix in the legacy path. This is the missing capability, and how it is supplied is the decision (ADR 0015 next commit). It is scanpy's Leiden, not a reimplementation, against this work's own habit of hand-writing numpy. Three things decide it. It adds no dependency: scanpy=1.9.3 and leidenalg=0.9.1 are already pinned in envs/analysis.yml and already run in the default DAG, and the rule that will consume this declares that same environment. ADR 0006 governs optional and licensed dependencies; this is neither. A diagnostic about a partition must be about the partition that ships. A hand-rolled clusterer would make item 7 sweep the resolution of an algorithm that never produces leiden_features.tsv. Being approximately the pipeline's clustering is worse than not clustering at all, because the numbers would look comparable and not be. And Leiden is not SNF. Group 8a hand-rolled similarity network fusion in sixty lines because it has a closed form; Leiden has local-moving, refinement and aggregation phases, and the refinement guarantee is the whole reason it beats Louvain. An approximation carrying the name would be worse than none. The cost is that the reference implementation is the implementation, so nothing cross-checks the arithmetic. What replaces that is a cross-*path* check: this module and leiden_clustering.scanpy_leiden_cluster must label the same matrix identically, asserted at N=250 and again above 500 where scanpy's graph build changes. They do -- identical labels, not merely ARI 1.0. clustering._clamped duplicates the legacy clamping rather than importing it, because importing would pull scanpy in at module scope, and that agreement test is what keeps the duplication honest. The bare environment found a real ordering defect while this was written: argument validation and the below-three-proteins short circuit sat behind the availability check, so a two-protein space needed scanpy installed to be told it has one cluster. Both now come first. test_optional_dependencies.py gains clustering, diagnostics.stability and diagnostics.partition -- and diagnostics.embedding and .redundancy, which group 8b added and did not list. 16 tests: 9 run with no scanpy at all, 15 run where it exists. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
…demo The last three of Phase 5's nine diagnostics reach the entry point, and the four dead DiagnosticsConfig fields come alive. NOT_YET_CONSUMED is now empty, which finishes FOLLOWUPS #36. Spaces cluster in their own right, so `diagnose_space` prefers a space's own partition over the legacy structural one and writes it to clusters.tsv. That is FOLLOWUPS #41: cross-cluster edge retention was computed for every space from the `structure` space's Leiden, which is the right partition for one space and the wrong one for the other six. When a space cannot be clustered the legacy partition is still used and the report says so in `partition.caveat` rather than leaving the reader to infer it. No Snakefile change and no new rule: the DAGs stay 16 / 25 / 35. clusters.tsv is deliberately not a declared output, because whether it exists depends on scanpy being importable and on the space having three proteins, and a rule that promises a file it cannot always write fails the run instead of degrading it. **Running the demo found two defects that 1061 unit tests could not.** Sixth sighting of the rule, and the first one where the statistic was not wrong but vacuous. At eleven proteins k clamps to 8 and an 80% subsample holds 9, so every protein's eight nearest are all the others and the Jaccard is 1.0 whatever the noise. All seven spaces reported perfect stability under a sigma half the size of the data. A statistic with no room left to be wrong in reporting 1.000 is the most confident possible way to say nothing, so NeighborhoodStability gained `informative` and a warning naming the fraction, and the demo now reports `informative: false` for every space. FOLLOWUPS #43 records that trustworthiness has the same exposure at k=6 of 11 and was left alone deliberately. And `fused_early`'s random-distance control silently vanished: its random matrix clusters into one group, which has no silhouette, so the report showed a shorter control list than its six neighbours. A reader cannot tell "ran and found nothing" from "never ran", and the shorter list reads as the former. Requested controls that could not be produced are now named with their reason. Two config keys are validated the way fusion params are: a sweep of one resolution is refused because it has no adjacent pair to compare, and an unknown control name is refused because a control named in a config and implemented nowhere is silently skipped. The known set is enumerated in diagnostics/partition.py and imported by the validator, the way STRATEGY_PARAMS is from fusion. test_diagnostics_config's consumption check now parses instead of grepping. It fired twice in this group on prose -- a docstring naming the key to raise, a comment naming the keys a validator enumerates -- and the pressure that creates is to word documentation around the test. FOLLOWUPS #42 records something the demo exposed and did not cause: the Leiden partition is not reproducible across two conda environments at N=11, which agree on scanpy, leidenalg, igraph, numpy and scikit-learn and differ only in scipy 1.13.1 against 1.15.2. arpack decides a degenerate tie on a nearly complete graph. At N=250 they agree exactly, and so does the legacy path. The pre-existing leiden_clustering rule has the same exposure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
ADR 0015, review log G8c.1 through G8c.9, and two new architecture contracts. The clustering question §0.5 posed was numpy-only against optional-scanpy, and the dependency framing turned out to be wrong on the facts: scanpy and leidenalg are already pinned in envs/analysis.yml, already run in the default 16-rule DAG, and rule diagnose_space already declares that environment. There was nothing to add. What settles it is a meaning argument -- a diagnostic about a partition must be about the partition that ships -- and the ADR leads with that rather than with the cost. Checking the environments before deciding also corrected CLAUDE.md, which described the full-stack test env as "pytest + scipy + sklearn + umap + pandas". It also carries scanpy and leidenalg at exactly the pinned versions, so the claim that a scanpy-backed clustering would be untestable here was false -- and it would have been the strongest argument against it. Contract 9 is new and general: a diagnostic says when it could not discriminate. Reporting a number is not reporting evidence, and the failure that matters is not a wrong value but a confident one computed where nothing could have come out differently. Contract 10: a partition-dependent number says which partition it is about. G8c.3 records the fixture inverting the answer before any statistic existed -- the fourth group running where building the fixture first changed the design. G8c.5 records the two defects the demo found that 1061 unit tests could not. G8c.8 records a reproducibility limit this work exposed and did not cause, and notes that it is a fifth cause for a mutation to survive: the parity test runs both sides in one environment, so it cannot see an environment-dependent output. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
Gate D's brief is to make the system produce a confidently wrong answer. It produced three, and the first is the serious one. **A constant block can receive a 46.7% contribution share.** ADR 0002 promises a clear error for a block whose every pairwise distance is zero, and test_fusion has asserted that since group 8a. The guard tests `mean <= 0.0`. But `pairwise_distances` uses the Gram identity for the memory reason its own docstring gives, and that identity loses about sqrt(eps) relative precision near zero: a block whose rows are *bitwise identical* comes back with a mean distance of 4.21e-08, not 0. Dividing by that amplifies cancellation noise into a unit-mean geometry, and `late` and `graph` both hand it nearly half the fused map with a straight face. Whether it happens depends on the magnitude of the block's values, which is why the existing test passed: at the fixture's N=240 the residue cancels to exactly 0.0 and at N=60 it does not. The test is parameterized over four cohort sizes now, because the size was load-bearing and was a fixture constant. The fix is a noise floor at 1e-6 of the block's own largest absolute value. That number is not chosen from float64: blocks are stored as float32 (ADR 0004), whose epsilon is 1.2e-07, so variation below roughly 1e-6 of a block's magnitude is not representable on disk and cannot be geometry whatever the arithmetic says. Both branches are reachable and they say different things -- "every pairwise distance is zero" against "its mean pairwise distance is 4.21e-08, which is below 1e-06 of the block's own scale" -- because a "this is zero" message about 4e-08 would send a reader looking for the wrong thing. **A NaN produces a plausible faithfulness score.** `argsort` sorts NaN last rather than raising, so one non-finite feature makes one protein's distances all NaN, that protein sorts last for everybody, and trustworthiness comes back 0.489 -- indistinguishable from "this is a mediocre map". `require_finite` now guards the three diagnostics that rank distances, and lives in one place so they cannot disagree about it. **`jaccard_rows` is silently wrong on a repeated index**, returning 1.0 where the sets give 0.667, because the 2k - shared identity assumes each row is a set. No caller can violate that -- they all pass `neighbor_ordering` output -- but the function is exported and the wrong answer was silent. Everything else Gate D attacked held. A `fusable: false` block cannot reach a multi-block space, and cannot be let in by an explicit `fusable: true` without a recorded justification. Blocks with mismatched protid order are aligned by label through the shared index. Contribution shares sum to exactly 1 under all four strategies. `matrix_io` refuses a permuted matrix with a quantified diagnosis. Duplicate protids are refused by `ProteinIndex`, which every real path builds. A singleton cluster scores 0 rather than 1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
Twenty executable probes across groups 6 through 8c, not a reading pass. Every "blocked" in the log is a call that was made and an exception or a correct answer that came back. Three defects, recorded as GD.1 through GD.3 with the fixes already in 1367611, and a fourth entry on what the gate says about gates. The observation worth keeping is GD.4. All five of the gate's own named targets held on the first attempt -- they were written during Phase 1, before groups 7 through 8c existed. All three defects came from probes invented while attacking. And two of them are invisible in a diff: one is a floating-point residue whose existence depends on a random draw, the other is numpy's documented NaN sort order doing exactly what it documents. Neither would have been found by a reviewer reasoning about the code, including the one who wrote it -- group 8a wrote both the constant-block guard and the test asserting it works, and the test passed for three commit groups while the guard did not hold. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
The last unbuilt item in Phase 6, and group 8c made it cheap. It was blocked on spaces not having partitions; they have one now, and diagnostics.partition.adjusted_rand_index is already written and agrees with scikit-learn to 1e-12 including the degenerate cases. What was left was wiring. coregister computes each space's partition rather than reading the clusters.tsv that diagnose_space writes, because that file is deliberately not a declared snakemake output -- whether it exists depends on scanpy and on cohort size -- so this rule cannot order itself after the one that writes it. That is only safe if the two agree, so a test asserts they do rather than the comment claiming it. A missing partition gives cluster_ari = None, the rule Procrustes already follows: absent, not fabricated. **Running the demo found the reason this needed more than wiring.** The pair `families vs fused_late` came out at ARI 1.000 beside a neighborhood Jaccard of 0.291. Both spaces put all eleven proteins in one cluster, and the adjusted Rand index of two single-cluster partitions is 1.0 by convention -- so the table read "these two spaces agree perfectly" when neither had found any structure. One degenerate side is no better: 0.0 against a real partition reads as complete disagreement rather than as nothing to compare. So the value is withheld whenever either side has fewer than two clusters, with the cluster counts and the reason recorded on the pair. The withholding is in the reporting layer; adjusted_rand_index keeps matching scikit-learn and keeps being tested against it. Seven of the demo's ten pairs are now blank and the three that survive are the ones where both spaces actually clustered. Seventh time running end to end has caught something a passing suite could not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
…proves ADR 0006 Four documents, one of them executable. docs/INTERPRETING.md is the one Phase 10 calls the thing that stops the new capability from generating confident nonsense, and it is written as a list of what is licensed and what is not. Cluster compactness survives projection and is licensed; ratios of UMAP distances are not; "these two are close so they share a function" is specifically wrong in a TM geometry rather than merely unsupported, because protease clans sit above TM 0.7 across entirely different specificities. Then each of the nine diagnostics with what it licenses -- including the ones that exist to say *do not read this*, which is most of what the demo reports. docs/EXTENDING.md is the worked version of the eight lines ARCHITECTURE.md sketches: declare an entry point, write four members, and nothing in this repo changes. The useful half is §3, five rules that are each a defect this project already shipped -- identity lives in labels (only 2 of 2530 columns sat in their row position on real data), say whether your zeros are measurements, do not put length in the geometry by accident, cohort-scoped vocabularies are not comparable across runs, and a value written to a manifest is not thereby honored. The README section appends and cross-links rather than rewriting the existing overview, per Phase 10 item 1. .github/workflows/multispace.yml is the job ADR 0006 rule 4 has been promising. It runs the multi-space demo end to end in an environment with zero optional dependencies, then checks all seven spaces produced diagnostics and that the cluster and search DAGs still resolve to 16 and 25. It opens by asserting the driver env still lacks sklearn/umap/scipy/scanpy/torch, because if that env ever gains them the whole job passes while proving nothing. Its second job runs test_determinism.py with --runslow, and fails if anything skipped. That guard has existed since group 8b and CI has never executed it: `make test` collects the file and runs none of it, because the tests are marked slow. A guard that never runs is not a guard. **The workflow is a draft.** Its YAML parses and nothing else about it is verified -- it cannot be run locally. Treat a first red run on GitHub as expected rather than as a regression. docs/MODELS.md is deliberately not written. It records optional models with their licences and this PR ships none; `plm` and `localization` are Phase 8. An empty document about models that do not exist is padding. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
Phase 10 item 5's walkthrough, written around the demo's actual measured output rather than around its configuration. The demo's headline result is a verdict against itself: eleven proteins cannot support these maps, and all nine diagnostics say so in numbers. Stability reports informative:false for all seven spaces, 4 to 10 of the 11 proteins have positions that should not be read, seven of ten space pairs cannot report a cluster ARI, and the two blocks feeding the fused spaces correlate at Spearman 0.883. That is the behaviour worth demonstrating -- the same fields appear on a real cohort and you read them the same way. The section that earns its place is "Things this demo cannot show you", because the alternative is a reader assuming otherwise: censoring never binds at N=11, the reducer determinism bug cannot occur below ~500 proteins, no partition here is worth trusting, and the clustering is not reproducible across scipy versions at this cohort size. FOLLOWUPS Arcadia-Science#22 is discharged by docs/INTERPRETING.md §3, which states that cluster mode writes no cohort report because it makes no cohort decision -- the absence was reading as an oversight rather than as a statement. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
382 s to 154.9 s on an idle box, 33 passed either way. Every change is a cache, a shared fixture, or the removal of a sleep against a mock. Nothing changes how the parity suite executes, because it is the evidence for this branch's central promise. **The finding that dominates: `foldseek_apiquery.py:169` sets `sleep_time = 30` for the public server, and the parity runs hit it while polling a mocked Foldseek that answers instantly.** About 150 s of the original 382 s was spent asleep against a mock. Measured: `run_foldseek` 31.2 s -> 1.33 s, the four-pipeline `runs` fixture 218.4 s -> 98.4 s, the significance run 56.9 s -> 25.3 s. It is removed without editing any source file, by handing the child processes a throwaway user-site directory holding a `usercustomize` module that shortcuts `time.sleep` only when `sys.argv[0]` ends in `foldseek_apiquery.py`. snakemake's `--use-conda` job environment strips exactly R_LIBS, PYTHONPATH, PERLLIB and PERL5LIB, so PYTHONUSERBASE reaches each rule's interpreter intact. Two properties decided that mechanism over the obvious alternatives: It is **symmetric**. `../pc-baseline` is a checkout of a commit that predates this branch and cannot be taught to read a new environment variable. Two of the four runs are the baseline's, so an asymmetric speedup would be close to worthless -- and an env-var flag added to `foldseek_apiquery.py` would also make it the twelfth pre-existing file this PR touches, purely for test speed. It **cannot redirect an import**. Putting the same shim on PYTHONPATH would put this checkout's `ProteinCartography` package on the baseline's import path, and a parity test that compares HEAD against HEAD passes vacuously. That is the worst available outcome and it would look like success. The failure mode is silence: if an environment is ever built with `site.ENABLE_USER_SITE = False`, the hook is never imported and every run quietly costs 30 s more. So `run_pipeline` does not merely set the variable, it reads `benchmarks/*.run_foldseek.txt` afterwards and fails above 15 s. Missing benchmarks are a failure rather than "nothing to check", because the silent mode is otherwise satisfied by silence. The rest: the two self-diffs are computed once rather than three times; `normalize_bytes` is memoised on (path, size, mtime_ns) -- as an LRU with a 128 MB budget, not an unbounded dict, because an output tree carries 21 MB of Plotly HTML and `mutation_check.py` builds fourteen trees in one process; `package_versions` is memoised behind an immutable tuple, so no two manifests can share one mutable dict; the raw entry-point scan is cached at the bare call and deliberately not at `_iter_entry_points`, which two tests monkeypatch; module fixtures replace repeated `fuse_graph`, enrichment and `diagnose_space` runs; and a session-scoped `synthetic_matrix_750` fixture replaces three identical 203 ms generations. Also fixed while in the file: the `runs` fixture docstring said three pipeline runs and it makes four, which matters because the fourth exists to measure the nondeterminism floor rather than to declare it. **Two things were refused rather than done.** Memoising `parity.synthetic_matrix` itself would gut `test_the_synthetic_fixture_is_reproducible`, which compares the bytes of two calls with the same arguments -- a memo reduces that to comparing one cached object with itself. And no fixture was shrunk: N=750 is load-bearing below sklearn's solver switch, N=240 and N=60 both matter after Gate D, and an AST comparison of all 995 test functions found zero identical bodies, so there was no duplicate-test win to take. The search-mode integration test gets the same sleep fix, **gated on mocks**. The cluster-mode test deliberately does not: it has no mocking fixture at all and its `key_protids` pulls `run_foldseek` into the DAG, so its 30 s wait is politeness toward a shared public server rather than dead time. Coverage is unchanged, and the mutation harness is the evidence rather than the claim. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
Phase 7. One static self-contained HTML file, all data embedded, no server, no
CDN, no build step (ADR 0005). Plotly is inlined rather than linked -- 3.6 MB of
JavaScript into the file -- because a CDN link breaks offline use and breaks the
day the CDN version changes, which defeats the archival property that motivated
the decision. `plot_interactive.py` keeps working and keeps its filenames.
**The interesting part is what it refuses to draw.** Nine diagnostics exist now
and several of them say *do not read this*. An explorer that renders those
identically to a trustworthy space would undo group 8c, so there are three
levels of refusal and each is decided in `explorer/payload.py` rather than in
the markup:
- a space the diagnostics call unreadable gets a red rule, a headline and its
points drawn hollow -- you can see where they are and you cannot mistake the
picture for a result;
- an individual protein whose position is not faithful is hollow in every
panel, even inside an otherwise trustworthy space;
- a withheld cross-space number renders as the word "withheld" with its
reason, never as a blank cell, because a blank reads as zero.
No threshold is re-declared here. The bands come from the modules that compute
the statistics -- DISTORTED_THRESHOLD, COIN_FLIP_THRESHOLD -- so the explorer
and docs/INTERPRETING.md cannot drift apart, and a test asserts that by reading
the source.
On the shipped demo it reports 7 of 7 spaces unreadable and says so on stderr:
"That is the diagnostics doing their job, not a failure."
**Running it end to end found two defects that eighteen passing tests did not.**
Eighth sighting of that rule.
The per-protein readable mask read `trustworthiness` and `continuity` arrays out
of diagnostics.json. Those keys do not exist -- `EmbeddingFaithfulness.to_dict`
carries only the means and the per-protein values go to
`faithfulness_{reducer}.tsv` -- so every protein came back readable and the
entire mechanism was silently inert while passing every shape check. It now
reads the TSV, and a missing table marks the space unreadable rather than
drawing every point as trustworthy on the strength of a file that is not there.
And the overlay table was looked for at `aggregated_features.tsv`, where it is
not; it is `final_results/{analysis_name}_aggregated_features.tsv`. The explorer
shipped with zero overlays, which looks exactly like a run whose features table
was empty. Now 24.
Two smaller things the machine caught rather than me: the rule referenced
`rules.aggregate_features` from above its definition, which snakemake resolves
at parse time -- the same ordering constraint that moved `diagnose_space` below
`leiden_clustering` in group 8b, and now the second sighting. And
`test_packaging.py` failed until `explorer` was added to setup.py, which is
exactly the guard it was written to be.
Verified: two runs produce byte-identical files (there is deliberately no
generation timestamp -- it would be the one field guaranteeing two runs of the
same inputs never agree), `node --check` passes on the emitted JavaScript, the
payload round-trips through JSON, and the demo DAG goes 35 -> 36 while cluster
stays 16 and search stays 25.
**Not yet verified: the page in an actual browser.** The JavaScript parses and
the data is right, but nobody has clicked on it. Linked selection and the
disagreement toggle are unexercised.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
The test-speed commit claimed coverage was unchanged and the mutation harness
agreed. Four independent reviewers were then asked to prove the claim false, and
three of them did. All four findings were demonstrated, not argued.
**A test lost its detector.** test_graph_parameters_reach_the_algorithm asserts
that graph parameters change the answer rather than being recorded and ignored.
Its control used to come from the `fuse` dispatcher; a module fixture replaced
it with a direct `fuse_graph` call, on the reasoning that `fuse` with empty
params *is* `fuse_graph(inputs)`. That is true today and it is precisely the
assumption under test. A mutation making `fuse` substitute its own defaults was
caught before the change and missed after -- and no test anywhere else calls
`fuse("graph", ...)` with empty params, which is the case any config naming
`strategy: graph` without parameters actually takes. Both sides go through the
dispatcher again, and the reasoning is in the test so the next speedup pass does
not repeat it.
**The normalization memo could call two different files identical.** It keyed on
(path, size, mtime_ns). Rewrite a file with a real payload change at the same
byte length, restore its mtime, and `compare_trees` reported the trees as
matching -- the single failure that module must never produce. Triggering it
needed `os.utime`; 2000 rapid rewrites here produced 2000 distinct mtimes. But
`cp -p`, `rsync -a`, `tar -x` and coarse-mtime filesystems all preserve mtime,
and "no caller currently does that" is a convention, not a property. The key is
now a content hash: ~50 ms of sha256 against the ~940 ms `re.sub` it avoids, so
the saving survives and the staleness class is gone by construction.
**Two safeguards were untested.** `package_versions` argues at length that
returning a shared mutable dict would let one manifest rewrite every other
manifest's provenance -- and replacing it with exactly that shared dict left the
whole suite green, because the existing test rebinds rather than mutates. The
memo's staleness key was likewise untested: reducing it to the bare path passed
everything. Both now have tests, and both tests were confirmed to fail against
the defect and pass against the fix.
The reviewer named the shape and it is worth keeping: this speedup **added three
process-level caches and in the same commit removed the in-process repetition
that would have exposed a bad one** -- `enrich_clusters.main()` went from 21
calls to 8, `_run()` in test_diagnose_space from 27 to 22.
**And one claim in my own commit message was false.** It said a session-scoped
`synthetic_matrix_750` fixture "replaces three identical 203 ms generations".
Nothing requested it -- none of those three files is even in that diff, and all
three still generate their own matrix. The fixture is removed rather than wired
up: the saving is 0.4 s in the `--runslow` path only, and touching two more test
files to collect it is a worse trade than deleting dead code and saying so.
Also confirmed, and the reason the exercise was worth running: **the sleep hook
is output-neutral.** Parity passing could not show that, because both sides get
the hook -- a symmetric change would sail through. Running the pipeline hooked
and unhooked and comparing the trees does: 40 byte-identical, 3 identical after
normalization, and the 1 differing file is the known nondeterminism floor.
1106 passed, parity 35, lint clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
…ness The adversarial pass on the speedup returned eight findings. Four were fixed in 9a28b08; this is the rest. **The worst one is mine, and it is the fourth instance of the same pattern.** `test_pipeline_in_search_mode.py` popped PYTHONNOUSERSITE unconditionally while gating only the redirect -- three lines below a comment reading "Gated on mocks, and that is not a detail." Under `--no-mocks` that re-enabled the machine's real ~/.local site-packages on sys.path for every snakemake rule interpreter, and any usercustomize.py living there auto-executes. That is the isolation "optional deps stay optional, CI must pass with zero of them installed" depends on, in the run closest to production and the one most likely used to check whether envs/*.yml is complete. Both halves are inside the gate now. A comment that states an invariant does not enforce it. This is the fourth time in this work, and the second time the comment was three lines from the code that broke it. **The sleep guard passed on a header-only benchmark file** -- silence wearing a file, which is exactly the mode its own docstring says it refuses to accept. It counts data rows now and raises on zero. It also gained the tests it never had: missing file, header-only, fast run, slow run. A guard shipped without a test of its own is a guard nobody has watched fail. **Two findings are recorded rather than fixed**, both in docs/FOLLOWUPS.md. Arcadia-Science#44 is a correction to this work's own claims. `mutation_check.py` never runs pytest -- it runs pipeline steps and diffs output trees, and all 17 mutations name six pipeline files. Zero of them touch fusion.py, diagnose_space.py, enrich_clusters.py or diagnostics/*, which are exactly the modules whose test files the speedup consolidated. So an identical "12 detected, 5 survived, 0 unexplained holes" before and after was guaranteed by construction, and citing it as evidence that the speedup changed no coverage was wrong. The harness is fine; the citation was not. Gate E finding 1 is the proof -- it reported zero holes while a real hole was open. #45 records that the only detector for a never-completing Foldseek ticket was wall clock, and removing 30 s sleeps removed it: with the mock returning RUNNING forever, run_pipeline now returns cleanly in 52 s and the guard is satisfied. The fix shape is a poll counter rather than a second timing bound. It also surfaced an upstream defect worth its own PR: foldseek_apiquery.py tests `elapsed > FOLDSEEK_SERVER_TIMEOUT` with elapsed reaching exactly 1800, so a ticket that never completes exits the loop and downloads anyway with no error. The hang was masking it. 1109 passed, lint clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
`download_pdbs` is a checkpoint, and this branch added `cohort_report.json`
to its outputs. That looked additive and was not.
On an output tree produced before this branch -- structures present, report
absent -- no job requests the report, so snakemake never re-runs the
checkpoint to produce it. `checkpoints.download_pdbs.get()` then raises,
`get_pdb_filepaths` contributes no `copy_pdb` job, and the run proceeds
*without the query proteins*. It does not fail; it finishes and reports
success on a map that is missing the protein the search was about.
Reproduced in isolation against snakemake 7.25.3, same tree both ways: a
single-output checkpoint plans 3 jobs including `copy_pdb`, the two-output
version plans 2, and the surviving job receives the directory in place of the
file list.
Neither the rule count nor the parity suite can see this. 16/25/36 is
unchanged and correct on a fresh run, and parity measures a fresh run from an
empty directory, where the claim holds. The break lives entirely in the
resumed run, which is how anyone with a multi-thousand-protein search uses
this pipeline.
The fix is to declare each new output only when the rule that reads it is in
the DAG, so neither is ever an orphan: `cohort_report` under
`MULTISPACE_ENABLED`, whose `diagnose_space` reads it, and `refseq_mapping`
under `COHORT_NEEDS_SIGNIFICANCE`, whose `aggregate_hit_significance` reads
it. `aggregate_hit_significance`'s reference to that output needs the same
guard, because input blocks are evaluated at parse time for every config.
Two consequences worth naming. The default output tree is now byte-identical
file-for-file, not "byte-identical plus two new files" -- README's claim
becomes literally true. And the `mapping_args` params is a function of
`output` rather than a formatted string, because a params string reaches the
shell verbatim and a `{protid}` written there would never expand.
Three tests, each shown to fail against the unfixed Snakefile.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
`conftest.py` stated that the parity suite "is the evidence behind the backwards-compatibility claim, so CI runs it on every pull request". No workflow passed `--runslow`. `make test` collects the 13 slow tests in `test_parity.py` -- including `test_default_output_is_unchanged_from_the_baseline` -- and runs none of them, and no job anywhere created the `../pc-baseline` worktree the fixtures need. The byte-identical promise was a one-time manual assertion on one laptop, and everything resting on it inherited that. `multispace.yml` names this exact failure mode in its own header comment -- "a guard that never executes is not a guard" -- and fixes it for the determinism test while leaving parity out. The new `parity` job checks out at full depth, adds the upstream remote, creates the baseline worktree at the merge-base, and shares the same `.snakemake/conda` cache as the other two jobs so the baseline does not rebuild it. The second step is the one that matters. `baseline_repo` and `conda_prefix` both *skip* rather than fail when their prerequisite is missing, and a skipped parity suite is indistinguishable from a passing one in the summary line. The job greps for both skip reasons and for the pytest summary, so a green result means the comparison ran. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
Four findings, all of the same shape -- a check or a document that is one step
narrower than the thing it describes.
GE.9. `CORE_MODULES` parameterizes the zero-optional-dependency guard over 25
names and omitted seven, four of them entry points the Snakefile invokes:
`compute_block`, `reduce_space`, `diagnose_space`, `build_explorer`, plus
`cohort`, `config_io` and `hit_significance`. The list already carried
`coregister` and `enrich_clusters`, so these were omissions by its own
convention. All seven import cleanly today; the guard simply was not asking.
GE.10. `test_every_subpackage_is_listed_in_setup_py` globbed `*/__init__.py`,
non-recursive, so it never checked `ProteinCartography.spaces.reducers` -- the
one nested subpackage, which happens to be listed, so the test passed for the
right reason by luck. Made recursive, and the expected name is now built from
the whole relative path: a leaf-name version would have looked for
`ProteinCartography.reducers` and failed on a package that is correctly listed.
GE.11. `from ProteinCartography import dim_reduction` stopped working. The
module now imports `spaces.reducers.core` flat, which resolves only with
`ProteinCartography/` itself on `sys.path` -- true when snakemake runs it as a
script, false for anyone importing the package. It worked at the merge-base,
the README advertises the package as importable, and this module carries an
explicit `__all__`, so it is the likeliest library import. A fallback restores
it. The new test runs in a subprocess from outside the repository, because a
test started from the repo root has both paths and cannot tell the forms apart.
GE.4. `docs/EXTENDING.md` documented an extension contract that does not
exist, and disagreed with itself. `spec_schema` is declared by all four
providers and called by nothing -- validation happens inside `compute()`,
after the run has started, which is the failure mode the doc claimed to
prevent. `REDUCER_GROUP` and `FUSION_GROUP` are defined and resolved nowhere,
so §1's "two sibling groups that work the same way" contradicted §7's "not
extensible by entry point". And `version` is not in the cache key: it lands in
`derived`, which `Manifest.cache_key` excludes by design, so a bump invalidates
nothing. The docs and the two docstrings now say what is true; making `version`
actually invalidate is a code change across eleven manifest call sites and is
recorded rather than rushed.
Also corrected from measurement, not reading: ARCHITECTURE.md's module tree
(the explorer is built, nine entry points were missing, biophys does not use
Biopython) and its on-disk layout (five wrong entries -- the store writes
`channel_censored.npy`, `manifest_{reducer}.json`, and `coregistration/`, and
neither `distance.npy` nor `neighbors_k{K}.parquet` is produced at all);
INTERPRETING's mutation claim, which needed the FOLLOWUPS Arcadia-Science#44 narrowing; and
the demo README, which said 35 rules for a 36-rule DAG, "nine diagnostics per
space" for a seven-section file, and "50/50" for a space that splits 73/27.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
…ible
Two defects in one section of one file, found by opening the built HTML and
looking rather than by reading the code.
The provenance footer collected nothing. `_provenance` read
`spaces/{id}/manifest.json`, a filename the store has never written -- it
writes `manifest_{reducer}.json` and `manifest_diagnostics.json`. So
`"manifests": {}` in every explorer ever built, and the footer rendered an
empty `<ul>`. Nothing failed. The only symptom was a blank section in a 3.7 MB
page, which is why five review passes over the source did not see it. This is
the third time in this work that the explorer has read a key or a path that
does not exist, and all three were reachable only by running it.
ADR 0002 says a block's contribution share is "computed, written to the
manifest, logged at runtime, and rendered on the panel", and that "no fused map
renders without it visible". The first three were true. `contribution` occurred
zero times in the HTML. The number was in `extra.fusion.contributions` the
whole time, one file away from the panel that promised to show it -- the
manifest-versus-honored pattern one level up, where the thing not honoring the
value is a decision record.
The panel now shows both the asked and the realized share, because they differ
and the difference is the point: `fused_late` asks 50/50 and realizes 34/66,
`fused_graph` asks 50/50 and realizes 50.4/49.6, and `fused_early` asks 50/50
and realizes 73/27 because `early` concatenates features and `tmscore` brings
eleven columns to `biophys`'s four. Showing only the request would misreport
the map. A single-block space apportions nothing and gets no row -- "tmscore
100%" on four of seven panels is the noise half of the rule that a diagnostic
which always fires says nothing.
Six tests. The template one asserts both that the number reaches the page and
that the panel reads it, because the payload can be right while the panel
ignores it, which is exactly the state being fixed.
Verified on a rebuilt demo: seven manifests, shares on the three fused spaces
and none of the four unfused ones, and 72 KB of JavaScript still clean under
`node --check`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
The first CI run of the new `parity` job did what it was added to do and failed, on four tests the previous commit broke and that no local loop would have caught -- they are `slow`, so `make test` and the fast suite both collect them and run nothing. The four asserted the cohort report on the *default* run tree, where it no longer lands. They now use `cohort_report_run`, one HEAD run with a minimal `spaces:` key, which is the configuration whose `diagnose_space` reads the report; the significance test gets the same key, at no extra cost since it already ran its own pipeline. The determinism test is replaced rather than moved. Its subject -- byte-stability of the report -- is already covered by `test_cohort.py::test_the_written_report_is_byte_stable`, which does not need a pipeline run to say it. What replaces it asserts the GE.2 invariant on a real run rather than on a dry-run plan: the default tree must *not* carry the report. The fixture docstring records why, because this is the part that will look like an omission later. Making the report a demanded target would also fix GE.2, by forcing the checkpoint to re-run -- and snakemake deletes a `directory()` output wholesale before re-running its rule, so a resumed search would re-download every structure from AlphaFold. Executed side by side on the same tree: the orphan output loses the query protein, the demanded output keeps it and re-downloads everything, and declaring it only when a space reads it costs neither. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
…ended Gate E ran as a four-reviewer battery and then a second pass that re-derived every blocker before touching anything. That second pass earned its keep: GE.2 was worse than reported, and the first fix for it was wrong in a way only execution showed. The log records fourteen findings, what was attacked and held, and two things the gate says about gates. Gate D found that probes beat reading. Gate E is the inverse and belongs beside it: every blocker was found by grep or a dry-run diff, and not one is a wrong number. The engineering survived four reviewers -- the `dim_reduction` port could not be made to differ in a single coordinate at seven cohort sizes. What failed was everything around it. Three of the four blockers are statements the repository makes about itself that are not true. The pattern this work has hit five times, that a comment stating an invariant does not enforce it, has a sibling: a document describing a mechanism does not create it, and unlike a comment beside code it never fails a test. The ADRs in this commit are the three that described an intended end state in the present indicative: - **0005** promised a preset switcher and a cluster inspector. Neither is built and both now say so; "six presets shipped" read as a description of the page. - **0006** described three env files, `docs/MODELS.md`, `make fetch-models`, a `plm` provider, an import smoke job and a scheduled fresh-solve job. All eight belong to Phases 8 and 9, which this PR defers. Marked deferred rather than reworded: the decision stands, only its tense was wrong. - **0014** was the only one of fifteen with neither "Alternatives rejected" nor "Consequences", against the README's claim that each has them, and the only one with a non-standard Status line. Both sections written, header normalized. Two comments in production modules named `docs/FOLLOWUPS.md` by path, a file the PR will not contain. Reworded to carry their own reason. The other ten references are trailing pointers after a self-contained sentence and are left alone -- whether the local-only documents ship is one decision (FOLLOWUPS #33), not 92 edits, and it is Matt's. Left deliberately undone, with the reasoning in GE.16: §0.8 E's "vestigial" refs are none of them reachable from HEAD, so deleting them orphans commits rather than tidying pointers. "Vestigial" was decided when they were reachable and the rebase changed that. Verified after the fixes: 1126 unit tests, lint clean, DAGs 16/25/36, parity 38 passed with 0 differing files, mutation harness exit 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
Found by the job's own first execution. `pytest … | tee /tmp/parity.txt` runs
under `shell: micromamba-shell {0}`, which does not inherit the job's
`bash -leo pipefail` default, so the pipeline's status was `tee`'s and four
genuinely failing tests produced a green step. Only the follow-up "must have
run, not skipped" guard caught it -- a check written to catch a *vacuous* pass
caught a *masked failure* instead, which is luck rather than design.
Redirect and replay instead of piping, so the step exits with pytest's status.
The guard stays, because it still covers the case it was written for, and its
message no longer says "no summary line" when what it means is "no *passing*
summary line" -- the failing run did print one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
Consequence of GE.2, noticed while re-reading the file rather than by a failure. Both entries in `ADDITIVE_OUTPUTS` describe files that a default run no longer produces, so neither can match in any comparison the suite performs. Kept rather than deleted -- the mechanism is still exercised on synthetic trees by five tests, and the next genuinely additive output needs the list to exist -- but an allowance that can never fire is the decoration half of the rule this work keeps applying, so it says so in place rather than waiting to be rediscovered. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
mrubash1
changed the title
92 references to the five local-only documents when the gate measured them; 107 after this log entry existed. Writing up the finding added fifteen to it. Worth the two lines it costs. `docs/REVIEW_LOG.md` is the largest single holder of references to files the PR will not contain, it is unlinked from the README, and it describes how the work was produced rather than what the code does -- which is the argument for shipping FOLLOWUPS.md and dropping this file, now recorded as the third option under FOLLOWUPS #33. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
The `multispace` workflow is green on all three jobs after the fixes, and the parity job is not green by skipping -- its log carries the resolved merge-base `36a38c71072e…`, from an `upstream` remote the runner adds itself, and `38 passed in 481.86s`. That is the first time the byte-identical claim has been checked by anything other than a person on a laptop. All eight Gate E commits also verified alone in detached worktrees: 1112 → 1126, monotonic, lint clean and cluster DAG 16 at every one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
The explorer has never been opened in a browser. Attempted twice, blocked both times on browser selection rather than on anything about the page. Recorded with what the check is actually for, because "open it and look" is not a testable instruction: the question is whether an `unreadable` space is unmistakably different from a readable one at a glance. That is ADR 0005 item 5's entire design, and parsing the payload cannot answer it -- which is also why the two defects Gate E did find there were found by reading the built file rather than by any test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
Found by opening the page and pressing the button, which is the only thing
that could have found it.
ADR 0005 item 4 calls disagreement mode a headline feature -- colour by
cross-space neighbourhood Jaccard, "one click, not buried in a menu". Every
point in all seven panels coloured `null`.
The template averages each protein's Jaccard across the pairs it appears in,
reading `row.per_protein` off each comparison row. The payload builds those rows
from `coregistration/summary.tsv`, which is aggregate only -- one row per pair,
`jaccard_mean` and friends. No row had ever carried `per_protein`, so
`if (!row.per_protein) continue` fired ten times out of ten and the map was
empty.
The data was one file away the whole time:
`coregistration/{a}__vs__{b}.tsv` carries `protid` and `neighborhood_jaccard`,
eleven rows per pair. The payload read the summary and never the details.
After: 10 of 10 rows carry it, 11 of 11 proteins get a value, nine distinct
values spanning 0.431 to 0.548, and the gradient renders -- the same protein the
same colour in every panel, which is correct, because disagreement is a property
of the protein and not of the space.
NaN is dropped rather than carried as zero. Averaging a missing measurement in
at 0.0 would report maximal disagreement for a pair that was never measured,
which is FOLLOWUPS Arcadia-Science#34's substituted-zero defect in a new place.
Four tests, three of which fail against the previous payload. The fourth is the
one that generalizes: render the template and assert the key the JavaScript
reads is the key the Python wrote. This is the fourth defect of this exact
family in the explorer -- a reader and a writer each individually reasonable and
never compared -- and no type checker or import graph crosses that boundary.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
Eight features exercised in a browser. Seven worked; the eighth was GE.17. Linked selection lights exactly the selected protids in all seven panels, the overlay selector offers 27 options including the readable mask, the layout switch repositions every point, clear selection restores all eleven, and there were no console errors at any point. The finding worth keeping is about the demo rather than the explorer. **All seven demo spaces are `unreadable`**, because at N=11 every one of them fails the stability band -- so the demo renders seven identical red panels and a reader never sees what a good one looks like. The artifact most people will open cannot demonstrate its own central design. Answered by rendering a synthetic payload with one space at each verdict level. The three are unmistakable and deliberately over-determined: border colour, banner colour and wording, and point fill. Three redundant channels, so the distinction survives a colour-blind reader or a greyscale print. ADR 0005 item 5 holds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
The decision this work has been deferring since group 4. `docs/FOLLOWUPS.md` now ships with the PR; `docs/REVIEW_LOG.md` does not. Shipping the follow-up list is right because it is the evidence that problems were noticed and deferred deliberately rather than missed. Forty-six entries, each naming a file, a reason and what unblocks it -- a normal artifact for work this size, and the thing a maintainer inheriting this branch would most want. Dropping the review log is right for the opposite reasons. At 2,323 lines it was 52% of all 107 references by itself, it is unlinked from the README, it describes how the work was produced rather than what the code does, and it is the one document in this repository nobody would ever update. It is preserved in full in the private notes repo, so nothing is lost -- the gate findings that matter to the code are already in the code, as tests and as comments. `PLAN.md`, `CLAUDE.md`, `docs/EXPLORATION.md` and `docs/PR_NARRATIVE.md` stay local. They are working documents about how the work was run. Thirty-six references would have dangled. Every one is rewritten to carry its own reason rather than a pointer: "(REVIEW_LOG G8.4)" becomes nothing, because the sentence before it already said the thing; "see PLAN §0.4" becomes the sentence it was standing in for. Nothing lost a reason, only a cross-reference to a file the reader would not have. The `FOLLOWUPS #N` references that remain now resolve, because this file ships. Verified by grep over `git ls-files`: the only surviving mention of a non-shipping document is #33 itself, which names them to record what was deliberately left out. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21
mrubash1
added a commit
that referenced
this pull request
Aug 24, 2026
…n numbers `envs/cartography_dev.yml` presented itself as the one environment a contributor needs, and it was a drifted duplicate of `envs/analysis.yml`: scikit-learn 1.3.2 where this branch's determinism guarantee is specific to 1.2.2, scanpy 1.9.6 against 1.9.3, and no matplotlib pin at all where `analysis.yml:13-16` documents one as mandatory. Following `CONTRIBUTING.md` gave you an environment that ran the code and disagreed with it. No snakemake rule names this file in a `conda:` directive, so nothing solved it and the drift had no way to surface. Deleting it closes FOLLOWUPS #2, which had been circling this since 2026-08-17: `ruff`, `snakefmt` and `pre-commit` were declared here and nowhere else, so `make lint` could not run from the tidy env. `CONTRIBUTING.md` now installs the three with `pip` at `lint.yml`'s own pins, which keeps the Makefile's promise that a passing `make lint` means a passing lint workflow. They are deliberately NOT added to `envs/cartography_tidy.yml`. A rule environment's content IS its snakemake identity -- change a line and every rule re-solves -- and `multispace.yml:210-214` already records that reasoning for pytest in the analysis env. A linter is not a pipeline dependency. `CONTRIBUTING.md` also stops telling contributors to add every new dependency to two files. It now says one: the environment for the rule that needs it. README's environment list loses the `cartography_dev` bullet. FOLLOWUPS Arcadia-Science#69 cited this file as one of four declaring `arcadia_pycolor`; it is three now, and the entry says so rather than going quietly stale. `git grep cartography_dev` returns nothing. WS1 of PC-036 phase 1. Upstream files, kept as their own commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SM7RDuLF3HaUcbbf5gchSo
mrubash1
added a commit
that referenced
this pull request
Aug 24, 2026
Two of this ADR's statements were true when written and were made false by
commits 180 and 183, in the same session. Both are the kind that read as
settled record and quietly stop being checkable.
`:29` and `:83` both said the CI conda cache key is `hashFiles('envs/*.yml')`.
Commit 180 scoped it to the six env files a `conda:` directive actually names.
The ADR's argument is unaffected -- unchanged env files are still never
re-solved, which is still why drift went undetected -- and the record now says
so explicitly, along with why the other four were dropped: snakemake does not
solve them, so hashing them only meant that editing a developer env evicted
every pipeline environment.
`:42` said "`envs/` is untouched by this branch", which was rule 1's evidence.
Commit 183 deleted `envs/cartography_dev.yml`. The clause now states the
exception and points at FOLLOWUPS #2, and says why it does not weaken rule 1:
rule 1 governs what ENTERS an existing env file, and nothing entered one.
Recording this as its own commit rather than folding it into 180 or 183,
because the failure being corrected is a document outliving its own evidence
-- which is what CLAUDE.md's "check the plan's claims about the code against
the code" rule is for, running in the other direction.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SM7RDuLF3HaUcbbf5gchSo
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
ProteinCartography builds one map from one representation: an all-versus-all
Foldseek TM-score matrix, PCA-30, then UMAP or t-SNE, with metadata joined at the
end as color overlays. It answers one question well — does global fold structure
organize this family? — and cannot answer several that people keep asking of it:
fold?
per-query hit cap?
Every one of those needs more than one representation over the same proteins.
This PR generalizes from one map to many co-registered maps over one
protidindex, plus explicit, auditable fusion when a single geometry is genuinely
wanted. The existing pipeline is untouched, and there is a test that proves
it — running in CI, on every pull request.
Two things Phase 0 measured that changed the design
Both were checked against a production 2530-protein run, and both were
independently re-derived before anything was built on them.
1. The matrix's column order was a permutation of its row order. Only 2 of
2530 columns sat in their row position. Reading that matrix positionally
returns the wrong cell 99.92% of the time; reading it by label gives a
diagonal that is exactly 1.0 in all 2530 rows. The data was always sound — only
the naive read was broken. PR Arcadia-Science#106 fixes the writer. This PR adds the assertion,
because the assertion is what catches the next one and what protects analyses
pointed at archived output.
2. 60.5% of that matrix is not a measurement.
get_line_for_protidwritesthe literal string
"0.0"for any pair Foldseek did not report, and Foldseek'sown scores arrive in
%.3Eform. Of 3,871,045 zero cells, zero are measuredzeros. The cause is
foldseek search --max-seqs, default 1000, and the cappredicts the observed zero fraction to within 145 cells out of 6.4 million.
The sharpest version: 925,435 of those censored cells have a measured mirror,
so their true value is knowable from the same file — median 0.772, with 96.4%
above 0.5. Meanwhile the lowest score Foldseek reports anywhere in the matrix is
0.0549.
This is already producing wrong answers in shipped outputs:
concordance_v_*evaluates to
0 − fidentfor censored pairs, and the cluster-similarityheatmaps average over raw cells, so at high censoring their between-cluster
means are mostly fill.
What is new
Plus fifteen ADRs in
docs/adr/,docs/ARCHITECTURE.md,docs/INTERPRETING.md,docs/EXTENDING.md,docs/FOLLOWUPS.md, and a runnabledemo/multispace/withits own walkthrough.
102 new files, 33,820 insertions and 129 deletions. The last number is the
one that matters: almost nothing was removed, because almost nothing was changed.
What changed in existing files
Twelve files, and the deletion column is the whole argument.
Snakefilespaces:key. Six deleted lines. The default DAG is still 16 rules in cluster mode and 25 in search mode — unchanged, and checked in CI.ProteinCartography/download_pdbs.pyProteinCartography/dim_reduction.pyspaces/reducers/core.py, so the legacy map and any new space are produced by the same code and cannot drift apart. CLI, filenames and return types unchanged. A net deletion of 46 lines.ProteinCartography/tests/test_packaging.pysetup.py, and thatdim_reductionstill imports as a package moduleProteinCartography/map_refseq_ids.pyREADME.mdProteinCartography/tests/test_pipeline_in_search_mode.py--no-mocksthis test polls the real public server, where the 30 s wait is politeness rather than dead timeconftest.pyslowmarker and--runslow; neither existedProteinCartography/calculate_concordance.pyconfig.ymlsetup.pysetup()does not recursepyproject.tomlpythonpathTwelve files, 129 deleted lines in total, and one of those files is a
docstring-only change. Everything else is new files in new directories, which
is what makes groups 1–4 individually extractable if you would rather take this
in pieces.
What is unchanged
The default configuration produces byte-identical output. The evidence is
ProteinCartography/tests/test_parity.py, which:upstream/main, currently36a38c7,which anyone can materialize with
git worktree add ../pc-baseline $(git merge-base HEAD upstream/main)—twice, and establishes empirically which files are inherently
nondeterministic;
The floor is measured, not declared, so the test cannot be weakened by quietly
adding an exclusion. Measured result: every scientific output is byte-identical —
aggregated_features.tsv, the pivoted matrix, the embeddings, the Leiden andStruCluster assignments, all per-protein feature tables, and all downloaded
structures. Three of the four HTML outputs are byte-identical once Plotly's
single random figure uuid is normalized.
The default tree is byte-identical file for file — not "byte-identical plus
some new files". It produces exactly the same set of paths it produced before.
The only differences are representational and each is excluded with its reason
recorded in
parity.py: benchmark timings, foldseek's internal shard databases,PDF/SVG creation timestamps, the raw unsorted pair lists (whose sorted
derivatives are compared), and the wordcloud's stochastic layout.
This suite runs in CI, in the
parityjob of.github/workflows/multispace.yml: 38 tests, 0 differing files, ~185 s locallyand ~460 s on an ubuntu runner. The job creates the baseline worktree itself from
a merge-base it resolves at runtime, and then greps its own log for both skip
reasons and for the pytest summary line — because a skipped parity suite and a
passing one look identical in a green checkmark.
How to review this
Commit by commit, in order. The branch is rebased onto
upstream/mainat36a38c7, so every commit in the diff is this work's — nothing upstream iscarried along, and there are no merge commits. Sixty-seven commits, in the
twelve groups below.
matrix_io.pythreedi,biophys,domains; no new required dependencyGroups 1–4 are individually extractable if you would rather take this in
pieces; none depends on a later one.
Each group has the same internal shape, and it is worth knowing because it is
the reason to trust the numbers: a fixture with a planted answer first, then
the statistic, then the wiring, then the ADR. Building the fixture first
changed the design before the code existed four times — most sharply in group 8c,
where the first stability fixture separated its groups by 100× and thereby
inverted the answer, scoring the tight well-separated group below the
structureless one. Nothing was wrong with the statistic.
Verified mechanically: every commit passes the unit suite alone in a clean
worktree, with
ruff check,ruff format --checkandsnakefmt --checkcleanat each one, and the cluster DAG at 16 rules throughout.
Five adversarial gates, four of which failed first
The work was reviewed at five points by passes briefed to attack it rather than
to read it. Four found real defects. This is the summary; each finding has a
commit and a regression test naming it.
Gate A re-derived both Phase 0 findings independently and reproduced every
number, contributing six refinements. One materially shrank a later commit: the
censoring mask can be rebuilt from the string form of any existing matrix file,
so it does not need reconstructing from raw
.m8alignments.Gate B attacked the abstractions and found fifteen defects, two of which
returned wrong numbers and reported success — a
repair=Truepath thatsilently discarded a column and mis-assigned cells on duplicate labels, and an
alignthat silently took the last duplicate from its source labels. It alsofound a test that was passing because of the bug it should have caught.
That gate is the argument for the commit ordering: the abstractions landed with
nothing depending on them, were attacked, and were fixed before four block
implementations were written against them.
Gate C mutation-tested the parity test itself and it failed. Eight
deliberate defects, four survived — not because the comparison was weak, but
because the 11-protein demo fixture cannot express them. At that size the PCA
component count, the UMAP neighbour count and Leiden's
n_pcsall clamp to thesame value whatever the config says, and the censoring fill token is never
emitted because every pair is measured.
The fix is a component-level parity test at N=750 on a seeded matrix reproducing
the production matrix's measured shape — 60.00% censoring against the real
60.48%, rows uniform at the per-query cap while columns vary. Final score:
17 mutations, 12 detected, 5 survived-as-expected with the reason recorded
against each, 0 unexplained holes. The harness exits non-zero on any
unexplained hole, so it is a check rather than a report.
Gate D attacked fusion and the diagnostics as twenty executable probes
rather than as a reading pass, and found three defects. The serious one: a block
whose rows are bitwise identical — carrying no information at all — took a
46.7% contribution share of a fused map. ADR 0002 promises a clear error
there and a test had asserted it since group 8a. But distances are computed by
the Gram identity for a memory reason, that identity loses about
sqrt(eps)near zero, and a constant block came back with a mean distance of
4.21e-08rather than
0. The guard testedmean <= 0.0and missed it.The existing test passed the whole time, and why it passed is the useful part:
at its fixture's N=240 the residue cancels to exactly zero and at N=60 it does
not. A random draw decided whether anyone saw it. The test is parameterised over
four cohort sizes now.
Gate E, which is the one a maintainer should read
Gate E ran before this PR was opened: four reviewers with disjoint briefs and no
shared context, told to treat the project's own planning documents as untrusted,
followed by a second pass that re-derived every finding before anything was
changed. Three blockers and eleven smaller findings, and not one of them was a
wrong number. The numerics survived; the packaging did not.
1. The evidence for the central claim had never executed, and a committed file
said it did.
conftest.pystated that the parity suite "is the evidence behindthe backwards-compatibility claim, so CI runs it on every pull request". No
workflow passed
--runslow.make testcollects every slow test intest_parity.py— including the byte-identical one — and runs none of them, andno job anywhere created the baseline worktree the fixtures need. The
byte-identical promise was a one-time manual assertion on one laptop, and every
gate before this one had inherited it. The sharpest part: the workflow's own
header comment names this failure mode — "a guard that never executes is not a
guard" — and fixes it for determinism while leaving parity out. The
parityjobnow exists, and it failed on its first run for a real reason, catching a
regression that no local loop runs.
2. A checkpoint output that nothing reads silently dropped the query protein.
download_pdbsis a snakemake checkpoint, and this branch addedcohort_report.jsonto its outputs — additive in appearance only. On an outputtree produced before this branch, no job requests the report, so snakemake never
re-runs the checkpoint to produce it;
checkpoints.download_pdbs.get()thenraises, the
copy_pdbjobs vanish from the DAG, and the run proceeds withoutthe query proteins. It does not fail. It finishes and reports success on a map
missing the protein the search was about.
Reproduced in isolation against snakemake 7.25.3 — three variants over one
identical tree, executed rather than dry-run:
HIT1.pdb HIT2.pdb QUERY.pdbstructures— the bare directoryrule allNEW.pdb QUERY.pdbNeither of this work's two instruments could see it. The rule count cannot:
16/25/36 is unchanged and correct on a fresh run, which is what CI asserts.
Parity cannot either: it measures a fresh run from an empty directory, where
the claim is true. The break lives entirely in the resumed run, which is how
anyone with a multi-thousand-protein search actually uses this pipeline. Both new
outputs are now declared only when a consumer is in the DAG.
The third row of that table is why the first fix was wrong. Making the report a
demanded target also stops the drop and keeps the report on the default path —
but snakemake deletes a
directory()output wholesale before re-running itsrule, so a resumed search would re-download every structure from AlphaFold.
Better to lose a report than a cohort. Stated plainly:
cohort_report.jsonisno longer produced on the default search path. ADR 0008's truncation diagnostic
is available only to someone who enables a space. That is a real reduction and
the least bad of the three options; the table is in the test fixture's docstring
so it is not re-derived.
3. The explorer's headline feature had never worked, and only a browser could
say so. ADR 0005 names disagreement mode — colour every protein by its
cross-space neighbourhood Jaccard — as a one-click feature. Opening the page and
pressing it coloured every point in all seven panels
null. The template readsrow.per_proteinoff each comparison row; the payload built those rows fromcoregistration/summary.tsv, which is aggregate only. The per-protein dataexisted the whole time in a sibling file the payload never read.
This is the fourth defect of one family in the explorer, after a readable
mask reading from the wrong file, a wrong aggregated-features path, and a
provenance footer that read
manifest.json— a filename the store has neverwritten, so every explorer ever built shipped an empty provenance section. Every
one is a reader and a writer that are each individually reasonable and were never
compared. The regression test that generalizes: render the template and assert
that the key the JavaScript reads is the key the Python wrote. That is a type
check across a language boundary, where no type checker can help.
And it answers whether opening the page in a browser was worth doing. Payload
parsing,
node --check, the 1126 unit tests that existed at the time and a greenCI run all passed over it.
What found it was pressing the button.
What Gate E says about gates. Gate D found that probes beat reading. Gate E
is the inverse: every blocker here was found by grep or by a dry-run diff, and
none was a wrong number. Three of the four blockers are statements the
repository makes about itself that are not true — a CI docstring, an ADR, an
extension guide. The rule this work hit five times — a comment stating an
invariant does not enforce it — has a sibling: a document describing a
mechanism does not create it, and unlike a comment beside code, it never fails
a test.
What Gate E did not check, stated so it is not assumed: no real interrupted
run at scale (the checkpoint mechanism was proven in isolation, on a synthesized
tree); no run against the 2530-protein production matrix — everything here is the
11-protein demo, the 240- and 750-protein fixtures, and dry-runs; the networked
path is mocked throughout; and local development is arm64 macOS, though CI now
covers ubuntu.
Deliberately out of scope
docs/FOLLOWUPS.mdships with this PR and holds 46 items — it is the evidencethat problems were noticed and deferred deliberately rather than missed. The
notable ones:
struclustersis mislabeled. It is a structurally-gated graph clustered onamino-acid identity —
foldseek clustreceives the alignment database andcannot even accept the TM-score database. So the pipeline computes TM-scores,
uses them for the embedding coordinates, then colors those points with an
amino-acid-identity-weighted label, in a rule named
plot_similarity_strucluster. The axes and the colors come from differentsimilarity measures. This is a documentation and naming problem in upstream,
independent of this work, and deserves its own small PR.
max_structurestruncation order still depends on UniProt's response order.N, and this is true of the existing
leiden_clusteringrule. Twoenvironments agreeing on scanpy, leidenalg, igraph, numpy and scikit-learn and
differing only in scipy 1.13.1 against 1.15.2 return different two-cluster
memberships at N=11, and identical ones at N=250.
envs/analysis.ymldoes notpin scipy. Found by this work, not caused by it (#42).
spec.metricandspec.normalizationare recorded on every block andhonored by nothing. Both are declared in every manifest on disk and applied
nowhere, because the reducer path feeds features straight into a euclidean
PCA. Fixing either changes the default map, so neither can land under the
byte-identical requirement without a decision about what the default should
be (Init nextflow scaffold Arcadia-Science/ProteinCartography#29, #32).
remove_nanssubstitutes a synthetic zero for a cluster with no data, so acluster whose structures all failed to download reads as significantly lower
pLDDT rather than as no measurement. Pre-existing; the new
enrichment.pytakes the other branch deliberately and reports
untestedwith the reason(Add parameter for user to define hits must be certain % of the length of the reference protein to include in results Arcadia-Science/ProteinCartography#34).
foldseek_apiquery.pydownloads anyway on a ticket that never completes.The poll loop tests elapsed time rather than the returned status, so a job
stuck in
RUNNINGfalls out of the loop and proceeds to download with noerror. Pre-existing in
upstream/mainand a clean standalone fix (#45).matrix_iois deliberately not retrofitted into the five existingraw-matrix consumers: all five were audited and are already label-safe, so the
retrofit would touch five existing files to buy assertion coverage rather than
a bug fix.
What this does not claim
Worth stating explicitly, because a large PR invites the assumption that
everything in it is finished.
the fused maps recover known biology is a later phase and is not here. A clean
review log is not a scientific result.
fixed. This PR adds a way to see them; it does not correct them.
biology.
docs/INTERPRETING.mdis explicit about which claims each numberlicenses, and the honest answer for the shipped eleven-protein demo is that it
licenses very few.
every one of its seven spaces fails the stability band, so all seven render as
unreadable and a reader never sees what a good one looks like. The
three-level distinction — border colour, banner wording and point fill, three
redundant channels so it survives a colour-blind reader or a greyscale print —
was verified against a synthetic payload instead. That is the demo's scale, not
a defect in the explorer, but it is the first thing most people will look at.
Handoff
67 commits on
multispace/main. Nothing has been pushed toArcadia-Science.ruff check,ruff format --check,snakefmt --checkclean, 125 filesupstream/mainat36a38c7— and it runs in CImultispaceworkflow's three jobs —end-to-end-with-no-optional-dependencies,determinism,parity— plus thelintandtestworkflowsDecisions owed by Matt:
fixture derived from an internal Arcadia run was built and is not used —
the procedural N=750 fixture is used instead, and reproduces the production
matrix's measured censoring shape well enough that publishing real data was
never necessary. Nothing depends on the decision; it is recorded so it is not
rediscovered.
real public Foldseek server today. Its 30-second poll interval is politeness
toward a shared resource, not dead time, so it should not simply be
shortened.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Gv8XcYwawycxfLGk4bnz21